From 7fee870886fc20d1de1378def6b3370dc4750526 Mon Sep 17 00:00:00 2001 From: Rhuan Barreto Date: Mon, 6 Jul 2026 14:12:37 -0300 Subject: [PATCH] feat(engine): injectable AST timeout, BOM coverage, .cts support MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Make runAstSubprocess() timeout injectable via optional timeoutMs parameter (defaults to AST_SUBPROCESS_TIMEOUT_MS) so tests can exercise the timeout→kill→throw path deterministically (#453) - Add BOM fixture tests for Python (utf-8-sig) and Ruby (r:bom|utf-8) serializers to guard against encoding regressions (#454) - Parse .cts files with sourceType "script" in the TypeScript AST branch, mirroring the existing .cjs handling for JavaScript (#455) - Consolidate plausibility guardrail tests and merge .cjs/.cts sourceType tests to stay within the 500-line file limit Closes #453, closes #454, closes #455 Claude-Session: https://claude.ai/code/session_01H7XPPVhN1t6HzH8F3xN4DP Signed-off-by: Rhuan Barreto --- src/engine/ast-support.ts | 9 ++-- src/engine/runner.ts | 6 ++- tests/engine/ast-support.test.ts | 52 ++++++++++++++++++++ tests/engine/runner-ast.test.ts | 83 ++++++++++++++++---------------- 4 files changed, 102 insertions(+), 48 deletions(-) diff --git a/src/engine/ast-support.ts b/src/engine/ast-support.ts index dd9b1db8..c9d8cbc0 100644 --- a/src/engine/ast-support.ts +++ b/src/engine/ast-support.ts @@ -156,7 +156,8 @@ export async function probeInterpreter( * large AST output cannot deadlock the pipe buffer. */ export async function runAstSubprocess( - cmd: string[] + cmd: string[], + timeoutMs: number = AST_SUBPROCESS_TIMEOUT_MS ): Promise<{ exitCode: number; stdout: string; stderr: string }> { const proc = Bun.spawn(cmd, { stdin: "ignore", @@ -168,7 +169,7 @@ export async function runAstSubprocess( let timer: ReturnType | undefined; const timeout = new Promise<"timeout">((resolve) => { - timer = setTimeout(() => resolve("timeout"), AST_SUBPROCESS_TIMEOUT_MS); + timer = setTimeout(() => resolve("timeout"), timeoutMs); }); const result = await Promise.race([proc.exited, timeout]).finally(() => { if (timer) clearTimeout(timer); @@ -176,9 +177,7 @@ export async function runAstSubprocess( if (result === "timeout") { proc.kill(); await proc.exited; - throw new Error( - `AST parser subprocess timed out after ${AST_SUBPROCESS_TIMEOUT_MS}ms` - ); + throw new Error(`AST parser subprocess timed out after ${timeoutMs}ms`); } const [stdout, stderr] = await Promise.all([stdoutPromise, stderrPromise]); diff --git a/src/engine/runner.ts b/src/engine/runner.ts index aa57a1a9..fe35d366 100644 --- a/src/engine/runner.ts +++ b/src/engine/runner.ts @@ -199,7 +199,11 @@ function createRuleContext( if (language === "typescript") { const loader = lowerPath.endsWith(".tsx") ? "tsx" : "ts"; const js = new Bun.Transpiler({ loader }).transformSync(source); - return parseJsModule(js) as unknown as EsTreeProgram; + // .cts is TypeScript CommonJS — parse the transpiled JS as a + // sloppy-mode script (mirroring the .cjs handling below). + return parseJsModule(js, { + sourceType: lowerPath.endsWith(".cts") ? "script" : "module", + }) as unknown as EsTreeProgram; } // .cjs cannot legally contain import/export in Node — parse it as // a sloppy-mode script so CommonJS-isms (top-level return, `with`) diff --git a/tests/engine/ast-support.test.ts b/tests/engine/ast-support.test.ts index 470100cd..f0ab09b3 100644 --- a/tests/engine/ast-support.test.ts +++ b/tests/engine/ast-support.test.ts @@ -80,6 +80,15 @@ describe("runAstSubprocess", () => { expect(stderr).toContain("boom-stderr"); expect(stdout).toBe(""); }); + + test("throws after the injected timeout and kills the subprocess", async () => { + await expect( + runAstSubprocess( + [process.execPath, "-e", "setTimeout(() => {}, 30000)"], + 50 + ) + ).rejects.toThrow(/timed out after 50ms/iu); + }); }); describe("parseAstJson", () => { @@ -168,6 +177,26 @@ describe("PYTHON_AST_PROGRAM end-to-end", () => { } ); + test.skipIf(!pythonInterpreter)( + "strips UTF-8 BOM and parses correctly", + async () => { + const interpreter = pythonInterpreter ?? "python"; + const file = join(tempDir, "bom.py"); + writeFileSync(file, "x = 42\n"); + + const { exitCode, stdout } = await runAstSubprocess([ + interpreter, + "-c", + PYTHON_AST_PROGRAM, + file, + ]); + expect(exitCode).toBe(0); + + const tree = JSON.parse(stdout) as { _type: string }; + expect(tree._type).toBe("Module"); + } + ); + test.skipIf(!pythonInterpreter)( "exits 1 with a syntax message for invalid source", async () => { @@ -221,6 +250,29 @@ describe("RUBY_AST_PROGRAM end-to-end", () => { } ); + test.skipIf(!rubyInterpreter)( + "strips UTF-8 BOM and parses correctly", + async () => { + const interpreter = rubyInterpreter ?? "ruby"; + const file = join(tempDir, "bom.rb"); + writeFileSync(file, 'puts "hello"\n'); + + const { exitCode, stdout } = await runAstSubprocess([ + interpreter, + "-rripper", + "-rjson", + "-e", + RUBY_AST_PROGRAM, + file, + ]); + expect(exitCode).toBe(0); + + const sexp = JSON.parse(stdout) as unknown[]; + expect(Array.isArray(sexp)).toBe(true); + expect(sexp[0]).toBe("program"); + } + ); + test.skipIf(!rubyInterpreter)( "exits 1 with 'Ruby syntax error' for invalid source", async () => { diff --git a/tests/engine/runner-ast.test.ts b/tests/engine/runner-ast.test.ts index aa2cc080..0f6686c7 100644 --- a/tests/engine/runner-ast.test.ts +++ b/tests/engine/runner-ast.test.ts @@ -150,27 +150,6 @@ describe("runChecks ctx.ast()", () => { expect(bodyTypes).toEqual(["FunctionDeclaration", "ExpressionStatement"]); }); - test("plausibility guardrail: wrong extension for the language is refused", async () => { - writeFileSync(join(tempDir, "data.json"), "{}"); - - const loaded = makeLoadedAdr( - {}, - { - rules: { - "json-as-python": { - description: "Attempt to parse JSON as Python", - async check(ctx) { - await ctx.ast("data.json", "python"); - }, - }, - }, - } - ); - - const result = await runChecks(tempDir, [loaded]); - expect(result.results[0].error).toContain("does not look like python"); - }); - test("sandbox guardrail: paths escaping the project root are refused", async () => { const loaded = makeLoadedAdr( {}, @@ -377,44 +356,64 @@ describe("runChecks ctx.ast()", () => { expect(programTypes).toEqual(["Program", "Program"]); }); - test(".cjs parses in sloppy script mode while .mjs rejects top-level return", async () => { - // Node permits a top-level `return` in CommonJS files but never in ESM, - // so the same source must parse as .cjs and throw as .mjs — proving the - // .cjs sourceType special-case is real, not incidental. - const source = + test(".cjs/.cts parse as script sourceType; .mjs rejects top-level return", async () => { + // Top-level return is legal in CommonJS (.cjs) but illegal in ESM (.mjs). + const cjsSrc = "if (process.env.ARCHGATE_DISABLED) return;\nmodule.exports = { ok: true };\n"; - writeFileSync(join(tempDir, "src", "legacy.cjs"), source); - writeFileSync(join(tempDir, "src", "modern.mjs"), source); + writeFileSync(join(tempDir, "src", "legacy.cjs"), cjsSrc); + writeFileSync(join(tempDir, "src", "modern.mjs"), cjsSrc); + // .cts is TypeScript CommonJS — mirroring the .cjs handling. + writeFileSync(join(tempDir, "src", "c.cts"), "const x: number = 42;\n"); + writeFileSync(join(tempDir, "src", "m.mts"), "const x: number = 42;\n"); - let cjsSourceType = ""; + const sourceTypes: Record = {}; const loaded = makeLoadedAdr( {}, { rules: { - "cjs-script-mode": { - description: "Top-level return is legal in .cjs", + "cjs-mode": { + description: ".cjs → script", async check(ctx) { - const program = await ctx.ast("src/legacy.cjs", "javascript"); - cjsSourceType = program.sourceType; + sourceTypes.cjs = ( + await ctx.ast("src/legacy.cjs", "javascript") + ).sourceType; }, }, - "mjs-module-mode": { - description: "Top-level return is illegal in .mjs", + "mjs-mode": { + description: ".mjs rejects top-level return", async check(ctx) { await ctx.ast("src/modern.mjs", "javascript"); }, }, + "cts-mts": { + description: ".cts → script, .mts → module", + async check(ctx) { + sourceTypes.cts = ( + await ctx.ast("src/c.cts", "typescript") + ).sourceType; + sourceTypes.mts = ( + await ctx.ast("src/m.mts", "typescript") + ).sourceType; + }, + }, }, } ); const result = await runChecks(tempDir, [loaded]); - const cjs = result.results.find((r) => r.ruleId === "cjs-script-mode"); - const mjs = result.results.find((r) => r.ruleId === "mjs-module-mode"); - expect(cjs?.error).toBeUndefined(); - expect(cjsSourceType).toBe("script"); + const mjs = result.results.find((r) => r.ruleId === "mjs-mode"); + expect( + result.results.find((r) => r.ruleId === "cjs-mode")?.error + ).toBeUndefined(); + expect( + result.results.find((r) => r.ruleId === "cts-mts")?.error + ).toBeUndefined(); + expect(sourceTypes).toEqual({ + cjs: "script", + cts: "script", + mts: "module", + }); expect(mjs?.error).toContain("Failed to parse"); - expect(mjs?.error).toContain("src/modern.mjs"); }); test.skipIf(!pythonInterpreter)( @@ -463,9 +462,9 @@ describe("runChecks ctx.ast()", () => { } ); - test("plausibility guardrail applies per language, not only python", async () => { + test("plausibility guardrail: wrong extension is refused for every language", async () => { writeFileSync(join(tempDir, "data.json"), "{}"); - const languages = ["ruby", "typescript", "javascript"] as const; + const languages = ["python", "ruby", "typescript", "javascript"] as const; const loaded = makeLoadedAdr( {},