refactor(scenario): remove validateCode — assertions live in execCode
- drop validateCode column, DTOs, service mappings, and scheduler branch - remove parseValidateResult helper and ValidateResult interface - inject playwright expect into code executor for direct use in execCode - strip validateCode from MCP tool schemas, client types, and UI forms
This commit is contained in:
@@ -27,6 +27,7 @@
|
||||
"@nestjs/schedule": "^6.1.1",
|
||||
"@nestjs/swagger": "^11.2.6",
|
||||
"@nestjs/typeorm": "^11.0.1",
|
||||
"@playwright/test": "^1.59.1",
|
||||
"acorn": "^8.16.0",
|
||||
"better-sqlite3": "^12.8.0",
|
||||
"class-transformer": "^0.5.1",
|
||||
|
||||
@@ -6,6 +6,7 @@ import {
|
||||
import { TraceLogger } from "../common/trace-logger";
|
||||
import { parse } from "acorn";
|
||||
import type { Page, BrowserContext } from "playwright";
|
||||
import { expect as playwrightExpect } from "@playwright/test";
|
||||
import { dumpDom } from "./dom-helpers";
|
||||
import type { EnvironmentData } from "../environment/environment.entity";
|
||||
|
||||
@@ -123,19 +124,28 @@ export class CodeExecutorService {
|
||||
"helpers",
|
||||
"console",
|
||||
"snippetArgs",
|
||||
"expect",
|
||||
`return (async (page, context, helpers, ...args) => { ${snippetCode} })(page, context, helpers, ...snippetArgs)`,
|
||||
);
|
||||
return snippetFn(page, context, pageHelpers, fakeConsole, args);
|
||||
return snippetFn(
|
||||
page,
|
||||
context,
|
||||
pageHelpers,
|
||||
fakeConsole,
|
||||
args,
|
||||
playwrightExpect,
|
||||
);
|
||||
},
|
||||
};
|
||||
|
||||
// Passing `console` as a named parameter shadows the global in the script scope.
|
||||
// Passing `console` and `expect` as named parameters exposes them in script scope.
|
||||
const fn = new Function(
|
||||
"page",
|
||||
"context",
|
||||
"helpers",
|
||||
"console",
|
||||
"result",
|
||||
"expect",
|
||||
`return (async (page, context, helpers) => { ${code} })(page, context, helpers)`,
|
||||
);
|
||||
this.logger.debug("Executing user code");
|
||||
@@ -145,6 +155,7 @@ export class CodeExecutorService {
|
||||
pageHelpers,
|
||||
fakeConsole,
|
||||
result,
|
||||
playwrightExpect,
|
||||
);
|
||||
return { result: execResult };
|
||||
} catch (err) {
|
||||
|
||||
@@ -505,10 +505,6 @@ export class McpService {
|
||||
.string()
|
||||
.optional()
|
||||
.describe("Playwright JS code to execute (exec steps)"),
|
||||
validateCode: z
|
||||
.string()
|
||||
.optional()
|
||||
.describe("Validation JS code returning { success, description }"),
|
||||
},
|
||||
},
|
||||
async ({ scenarioId, ...dto }) => {
|
||||
@@ -569,7 +565,6 @@ export class McpService {
|
||||
.describe("New step type"),
|
||||
title: z.string().optional().describe("New step title"),
|
||||
execCode: z.string().optional().describe("New exec code"),
|
||||
validateCode: z.string().optional().describe("New validation code"),
|
||||
},
|
||||
},
|
||||
async ({ scenarioId, stepId, ...dto }) => {
|
||||
@@ -785,11 +780,6 @@ export class McpService {
|
||||
.nullable()
|
||||
.optional()
|
||||
.describe("Exec/sign code"),
|
||||
validateCode: z
|
||||
.string()
|
||||
.nullable()
|
||||
.optional()
|
||||
.describe("Validation code"),
|
||||
}),
|
||||
)
|
||||
.describe("Ordered list of steps"),
|
||||
|
||||
@@ -13,12 +13,4 @@ export class CreateScenarioStepDto {
|
||||
@IsNotEmpty()
|
||||
execCode?: string;
|
||||
|
||||
@ApiPropertyOptional({
|
||||
example:
|
||||
'return { success: result !== null, description: "title present" };',
|
||||
})
|
||||
@IsOptional()
|
||||
@IsString()
|
||||
@IsNotEmpty()
|
||||
validateCode?: string;
|
||||
}
|
||||
|
||||
@@ -23,11 +23,6 @@ export class ScenarioStepExportDto {
|
||||
@IsNotEmpty()
|
||||
execCode: string | null;
|
||||
|
||||
@ApiPropertyOptional()
|
||||
@IsOptional()
|
||||
@IsString()
|
||||
@IsNotEmpty()
|
||||
validateCode: string | null;
|
||||
}
|
||||
|
||||
export class ScenarioExportDto {
|
||||
|
||||
@@ -19,9 +19,4 @@ export class UpdateScenarioStepDto {
|
||||
@IsNotEmpty()
|
||||
execCode?: string;
|
||||
|
||||
@ApiPropertyOptional()
|
||||
@IsOptional()
|
||||
@IsString()
|
||||
@IsNotEmpty()
|
||||
validateCode?: string;
|
||||
}
|
||||
|
||||
@@ -17,11 +17,6 @@ import { ScenarioService } from "./scenario.service";
|
||||
import { SnippetService } from "../snippet/snippet.service";
|
||||
import { EnvironmentEntity, EnvironmentData } from "../environment/environment.entity";
|
||||
|
||||
interface ValidateResult {
|
||||
success: boolean;
|
||||
description?: string;
|
||||
}
|
||||
|
||||
interface BrowserHandle {
|
||||
browser: Browser;
|
||||
context: BrowserContext;
|
||||
@@ -188,25 +183,6 @@ export class ScenarioSchedulerService {
|
||||
snips,
|
||||
);
|
||||
|
||||
if (step.validateCode) {
|
||||
this.codeExecutor.validate(step.validateCode);
|
||||
const { result } = await this.codeExecutor.execute(
|
||||
page,
|
||||
context,
|
||||
step.validateCode,
|
||||
this.stepLogger(stepRun.id, stepRun.runId),
|
||||
getStepOutput,
|
||||
creds,
|
||||
env,
|
||||
snips,
|
||||
execOutput,
|
||||
);
|
||||
const vr = this.parseValidateResult(result);
|
||||
if (!vr.success) throw new Error(vr.description ?? "Validation failed");
|
||||
await this.passStepRun(stepRun, vr.description ?? null, execOutput);
|
||||
return;
|
||||
}
|
||||
|
||||
await this.passStepRun(stepRun, null, execOutput);
|
||||
} catch (err) {
|
||||
const msg = (err as Error).message ?? String(err);
|
||||
@@ -250,21 +226,6 @@ export class ScenarioSchedulerService {
|
||||
}
|
||||
}
|
||||
|
||||
// ── Validation helper ──────────────────────────────────────────────────────
|
||||
|
||||
private parseValidateResult(raw: unknown): ValidateResult {
|
||||
if (typeof raw === "boolean") return { success: raw };
|
||||
if (raw && typeof raw === "object") {
|
||||
const r = raw as Record<string, unknown>;
|
||||
return {
|
||||
success: Boolean(r["success"]),
|
||||
description:
|
||||
r["description"] != null ? String(r["description"]) : undefined,
|
||||
};
|
||||
}
|
||||
return { success: Boolean(raw) };
|
||||
}
|
||||
|
||||
// ── Pass / fail helpers ────────────────────────────────────────────────────
|
||||
|
||||
private async passStepRun(
|
||||
|
||||
@@ -32,9 +32,6 @@ export class ScenarioStepEntity {
|
||||
@Column({ type: "text", nullable: true })
|
||||
execCode: string | null;
|
||||
|
||||
@Column({ type: "text", nullable: true })
|
||||
validateCode: string | null;
|
||||
|
||||
@CreateDateColumn()
|
||||
createdAt: Date;
|
||||
|
||||
|
||||
@@ -124,7 +124,6 @@ export class ScenarioService {
|
||||
scenarioId,
|
||||
title: dto.title ?? null,
|
||||
execCode: dto.execCode ?? null,
|
||||
validateCode: dto.validateCode ?? null,
|
||||
}),
|
||||
);
|
||||
await this.normalizeStepOrder(scenarioId);
|
||||
@@ -156,7 +155,6 @@ export class ScenarioService {
|
||||
order: step.order,
|
||||
title: dto.title ?? step.title,
|
||||
execCode: dto.execCode ?? step.execCode,
|
||||
validateCode: dto.validateCode ?? step.validateCode,
|
||||
});
|
||||
await this.stepRepo.save(step);
|
||||
|
||||
@@ -404,7 +402,6 @@ export class ScenarioService {
|
||||
steps: scenario.steps.map((s) => ({
|
||||
title: s.title,
|
||||
execCode: s.execCode,
|
||||
validateCode: s.validateCode,
|
||||
})),
|
||||
};
|
||||
}
|
||||
@@ -436,7 +433,6 @@ export class ScenarioService {
|
||||
order: index,
|
||||
title: s.title ?? null,
|
||||
execCode: s.execCode ?? null,
|
||||
validateCode: s.validateCode ?? null,
|
||||
}),
|
||||
);
|
||||
await this.stepRepo.save(steps);
|
||||
|
||||
@@ -477,7 +477,6 @@ describe("ScenarioController", () => {
|
||||
order: 1,
|
||||
sessionName: "s",
|
||||
execCode: "return 1;",
|
||||
validateCode: "return true;",
|
||||
});
|
||||
|
||||
const res = await request(app.getHttpServer())
|
||||
@@ -536,25 +535,6 @@ describe("ScenarioController", () => {
|
||||
expect(step).not.toHaveProperty("updatedAt");
|
||||
});
|
||||
|
||||
it("exports null validateCode as null", async () => {
|
||||
const sc = await createScenario("export-null-validate");
|
||||
await createStep(sc.id, {
|
||||
order: 0,
|
||||
sessionName: "s",
|
||||
execCode: "return 1;",
|
||||
});
|
||||
|
||||
const res = await request(app.getHttpServer())
|
||||
.get(`/scenarios/${sc.id}/export`)
|
||||
.expect(200);
|
||||
|
||||
const exported = yamlParse(res.text) as {
|
||||
steps: Array<{ validateCode: string | null }>;
|
||||
};
|
||||
|
||||
expect(exported.steps[0].validateCode).toBeNull();
|
||||
});
|
||||
|
||||
it("returns 404 for unknown scenario", async () => {
|
||||
await request(app.getHttpServer())
|
||||
.get("/scenarios/00000000-0000-0000-0000-000000000001/export")
|
||||
@@ -572,17 +552,14 @@ describe("ScenarioController", () => {
|
||||
{
|
||||
sessionName: "s",
|
||||
execCode: '{"keyId":"k","environmentName":"e"}',
|
||||
validateCode: null,
|
||||
},
|
||||
{
|
||||
sessionName: "s",
|
||||
execCode: "return 1;",
|
||||
validateCode: "return true;",
|
||||
},
|
||||
{
|
||||
sessionName: "s",
|
||||
execCode: '{"keyId":"k"}',
|
||||
validateCode: null,
|
||||
},
|
||||
],
|
||||
};
|
||||
@@ -622,7 +599,6 @@ describe("ScenarioController", () => {
|
||||
order: 0,
|
||||
sessionName: "rs",
|
||||
execCode: "return 42;",
|
||||
validateCode: "return true;",
|
||||
});
|
||||
|
||||
const exportRes = await request(app.getHttpServer())
|
||||
@@ -630,7 +606,7 @@ describe("ScenarioController", () => {
|
||||
.expect(200);
|
||||
|
||||
const exported = yamlParse(exportRes.text) as {
|
||||
steps: Array<{ execCode: string; validateCode: string | null }>;
|
||||
steps: Array<{ execCode: string }>;
|
||||
};
|
||||
|
||||
const importRes = await request(app.getHttpServer())
|
||||
@@ -643,9 +619,6 @@ describe("ScenarioController", () => {
|
||||
expect(importRes.body.steps[0].execCode).toBe(
|
||||
exported.steps[0].execCode,
|
||||
);
|
||||
expect(importRes.body.steps[0].validateCode).toBe(
|
||||
exported.steps[0].validateCode,
|
||||
);
|
||||
});
|
||||
|
||||
it("imports with empty steps array", async () => {
|
||||
|
||||
Reference in New Issue
Block a user