From 4f1efc211baddc817b089b9724f30051a067a293 Mon Sep 17 00:00:00 2001 From: Rhuan Barreto Date: Thu, 16 Jul 2026 18:10:05 -0300 Subject: [PATCH 1/3] feat(engine): cache ctx.ast() parse results within a single check run (closes #482) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add an astResults map to RunCaches, mirroring cachedGlob/cachedFileText, keyed on the NUL-joined (absPath, language, rev, comments) tuple. The promise is cached, so concurrent identical calls collapse into one in-flight parse/interpreter spawn. Rejected promises stay cached — a deliberate decision consistent with ctx.ast()'s fail-closed contract: every rule touching the same input fails fast with the identical error. The cheap argument-validation guardrails (path safety, language plausibility, feature guard) still run per call before the cache lookup, preserving ARCH-022's guardrail ordering on cache hits. The python doc-only-edit test now parses its "real edit" in a separate runChecks invocation: a mid-run working-tree edit is deliberately not observable within one run, matching readFile's existing cache semantics. Signed-off-by: Rhuan Barreto --- .../content/docs/nb/reference/rule-api.mdx | 2 + .../content/docs/pt-br/reference/rule-api.mdx | 2 + docs/src/content/docs/reference/rule-api.mdx | 2 + src/engine/ast-support.ts | 19 ++ src/engine/runner.ts | 189 +++++++----- tests/engine/runner-ast-base.test.ts | 54 ++-- tests/engine/runner-ast-cache.test.ts | 284 ++++++++++++++++++ 7 files changed, 448 insertions(+), 104 deletions(-) create mode 100644 tests/engine/runner-ast-cache.test.ts diff --git a/docs/src/content/docs/nb/reference/rule-api.mdx b/docs/src/content/docs/nb/reference/rule-api.mdx index 853d3cbf..a94329bc 100644 --- a/docs/src/content/docs/nb/reference/rule-api.mdx +++ b/docs/src/content/docs/nb/reference/rule-api.mdx @@ -210,6 +210,8 @@ interface AstOptions { Parse en kildefil til dens språknative AST. Stien er relativ til prosjektroten og går gjennom samme sandkasse som `readFile`. TypeScript og JavaScript parses i prosessen; Python og Ruby parses ved å starte systemtolkens egen AST-fasilitet fra standardbiblioteket som en underprosess. Formen på det returnerte treet varierer per språk -- se [AstNode](#astnode). +Parseresultater caches gjennom én enkelt `archgate check`-kjøring, med nøkkel `(path, language, rev, comments)`: gjentatte -- til og med samtidige -- identiske kall på tvers av regler koster én parse (én tolkoppstart for Python/Ruby), og en mislykket parse kaster den samme feilen på nytt til alle kallere. Behandle det returnerte treet som skrivebeskyttet -- det kan være delt med andre regler. + ```typescript const program = await ctx.ast("src/cli.ts", "typescript"); for (const node of program.body) { diff --git a/docs/src/content/docs/pt-br/reference/rule-api.mdx b/docs/src/content/docs/pt-br/reference/rule-api.mdx index 72d1fcee..19803086 100644 --- a/docs/src/content/docs/pt-br/reference/rule-api.mdx +++ b/docs/src/content/docs/pt-br/reference/rule-api.mdx @@ -210,6 +210,8 @@ interface AstOptions { Faz o parse de um arquivo-fonte para sua AST nativa da linguagem. O caminho é relativo à raiz do projeto e passa pelo mesmo sandbox de `readFile`. TypeScript e JavaScript são parseados in-process; Python e Ruby são parseados invocando como subprocesso o recurso de AST da biblioteca padrão do próprio interpretador do sistema. O formato da árvore retornada difere por linguagem -- veja [AstNode](#astnode). +Os resultados de parse são cacheados durante uma única execução de `archgate check`, com chave `(path, language, rev, comments)`: chamadas idênticas repetidas -- até mesmo concorrentes -- entre regras custam um único parse (uma única inicialização do interpretador para Python/Ruby), e um parse que falhou relança o mesmo erro para todos os chamadores. Trate a árvore retornada como somente leitura -- ela pode ser compartilhada com outras regras. + ```typescript const program = await ctx.ast("src/cli.ts", "typescript"); for (const node of program.body) { diff --git a/docs/src/content/docs/reference/rule-api.mdx b/docs/src/content/docs/reference/rule-api.mdx index fe0938c2..f3b4159b 100644 --- a/docs/src/content/docs/reference/rule-api.mdx +++ b/docs/src/content/docs/reference/rule-api.mdx @@ -210,6 +210,8 @@ interface AstOptions { Parse a source file into its language-native AST. The path is relative to the project root and passes through the same sandbox as `readFile`. TypeScript and JavaScript are parsed in-process; Python and Ruby are parsed by invoking the system interpreter's own standard-library AST facility as a subprocess. The returned tree shape differs per language -- see [AstNode](#astnode). +Parse results are cached for the lifetime of a single `archgate check` run, keyed on `(path, language, rev, comments)`: repeated -- even concurrent -- identical calls across rules cost one parse (one interpreter spawn for Python/Ruby), and a failed parse rethrows the same error to every caller. Treat the returned tree as read-only -- it may be shared with other rules. + ```typescript const program = await ctx.ast("src/cli.ts", "typescript"); for (const node of program.body) { diff --git a/src/engine/ast-support.ts b/src/engine/ast-support.ts index 7a32a8ca..d9e9a975 100644 --- a/src/engine/ast-support.ts +++ b/src/engine/ast-support.ts @@ -392,6 +392,25 @@ export function finalizeAstResult( return tree; } +/** + * Cache key for a per-run `ctx.ast()` parse (see `RunCaches.astResults` in + * runner.ts): the full tuple that determines the parse output, NUL-joined — + * NUL cannot appear in a path, so distinct tuples can never collide. + */ +export function astCacheKey( + absPath: string, + language: AstLanguage, + useBase: boolean, + wantComments: boolean +): string { + return [ + absPath, + language, + useBase ? "base" : "working-tree", + wantComments ? "comments" : "no-comments", + ].join("\u0000"); +} + /** Extract a readable message from Bun.Transpiler/meriyah parse errors. */ export function parseErrorMessage(err: unknown): string { if (err instanceof AggregateError && err.errors.length > 0) { diff --git a/src/engine/runner.ts b/src/engine/runner.ts index ff630bed..bf58619e 100644 --- a/src/engine/runner.ts +++ b/src/engine/runner.ts @@ -5,6 +5,7 @@ import { relative, resolve, isAbsolute } from "node:path"; import type { AstLanguage, + AstNode, AstOptions, EsTreeProgram, GrepMatch, @@ -22,6 +23,7 @@ import { PYTHON_AST_WITH_COMMENTS_PROGRAM, RUBY_AST_PROGRAM, RUBY_BASENAMES, + astCacheKey, commentsUnsupportedError, finalizeAstResult, implausibleLanguageError, @@ -104,16 +106,19 @@ const RULE_TIMEOUT_MS = 30_000; * without these caches, 40+ rules each repeat identical filesystem work. * * Values are promises so concurrent rules share in-flight work instead of - * racing to duplicate it. Only immutable results are cached: glob results - * are copied on return, file contents are strings. `readJSON` is deliberately - * NOT cached — rules receive a mutable object, and sharing one instance - * would leak mutations between rules. + * racing to duplicate it. Glob results are copied on return, file contents + * are immutable strings. AST results are cached as shared trees — rules must + * treat them as read-only. `readJSON` is deliberately NOT cached — rules + * receive a mutable object, and sharing one instance would leak mutations + * between rules. */ interface RunCaches { /** Glob results keyed by `tracked:`/`all:` + pattern. */ globResults: Map>; /** File contents keyed by absolute path. */ fileText: Map>; + /** ctx.ast() parses keyed by the NUL-joined (absPath, language, rev, comments) tuple. */ + astResults: Map>; } export interface RuleResult { @@ -207,6 +212,7 @@ function createRuleContext( language: "ruby", opts?: AstOptions ): Promise; + // oxlint-disable-next-line require-await -- async keeps guardrail failures as rejections, never sync throws async function astImpl( path: string, language: AstLanguage, @@ -242,81 +248,106 @@ function createRuleContext( throw commentsUnsupportedError(language, path); } - // In-process branch: TypeScript/JavaScript via the shared meriyah - // parser (js-parser.ts). No subprocess is spawned for these languages. - if (language === "typescript" || language === "javascript") { - const source = useBase - ? await readBaseSourceOrThrow(projectRoot, baseRev, relPath, path) - : await cachedFileText(absPath); - try { - return parseTsOrJsSource(language, path, source, wantComments); - } catch (err) { - throw new Error( - `Failed to parse "${path}" as ${language}: ${parseErrorMessage(err)}` - ); + /** The uncached parse: TS/JS in-process, Python/Ruby via guardrails 3–4. */ + async function parseUncached(): Promise { + // In-process branch: TypeScript/JavaScript via the shared meriyah + // parser (js-parser.ts). No subprocess is spawned for these languages. + if (language === "typescript" || language === "javascript") { + const source = useBase + ? await readBaseSourceOrThrow(projectRoot, baseRev, relPath, path) + : await cachedFileText(absPath); + try { + // Meriyah's Program is ESTree-shaped but lacks the index signature. + const tree = parseTsOrJsSource(language, path, source, wantComments); + return tree as unknown as EsTreeProgram; + } catch (err) { + throw new Error( + `Failed to parse "${path}" as ${language}: ${parseErrorMessage(err)}` + ); + } } - } - // Guardrail 3: interpreter availability probe, cached per check run. - const candidates = interpreterCandidates(language); - let probe = interpreterCache.get(language); - if (!probe) { - probe = probeInterpreter(candidates); - interpreterCache.set(language, probe); - } - const interpreter = await probe; - if (!interpreter) { - throw interpreterNotFoundError(language, candidates, path); - } + // Guardrail 3: interpreter availability probe, cached per check run. + const candidates = interpreterCandidates(language); + let probe = interpreterCache.get(language); + if (!probe) { + probe = probeInterpreter(candidates); + interpreterCache.set(language, probe); + } + const interpreter = await probe; + if (!interpreter) { + throw interpreterNotFoundError(language, candidates, path); + } - // For { rev: "base" }, the interpreter serializers read a file path from - // argv, but the base content is not on disk — materialize it to a throwaway - // temp file and hand that path to the same, unchanged program (and the same - // `-I` isolation). Cleaned up in `finally` regardless of outcome. - const { sourcePath, cleanup } = await materializeAstInput({ - useBase, - absPath, - ext: language === "python" ? ".py" : ".rb", - projectRoot, - baseRev, - relPath, - displayPath: path, - }); - - try { - // Guardrail 4: guarded invocation — array args only, path via argv. - // Python runs in isolated mode (-I): without it, `python -c` puts the - // cwd (the target project root) on sys.path, so a hostile project - // could shadow stdlib modules (ast.py, json.py) and execute arbitrary - // code when the serializer imports them. Ruby is safe as-is — its - // load path has not included the cwd since 1.9.2. - const pyProgram = wantComments - ? PYTHON_AST_WITH_COMMENTS_PROGRAM - : PYTHON_AST_PROGRAM; - const cmd = - language === "python" - ? [interpreter, "-I", "-c", pyProgram, sourcePath] - : [ - interpreter, - "-rripper", - "-rjson", - "-e", - RUBY_AST_PROGRAM, - sourcePath, - ]; - const { exitCode, stdout, stderr } = await runAstSubprocess(cmd); - if (exitCode !== 0) { - const detail = stderr.trim() || `exit code ${exitCode}`; - throw new Error(`Failed to parse "${path}" as ${language}: ${detail}`); + // For { rev: "base" }, the interpreter serializers read a file path from + // argv, but the base content is not on disk — materialize it to a throwaway + // temp file and hand that path to the same, unchanged program (and the same + // `-I` isolation). Cleaned up in `finally` regardless of outcome. + const { sourcePath, cleanup } = await materializeAstInput({ + useBase, + absPath, + ext: language === "python" ? ".py" : ".rb", + projectRoot, + baseRev, + relPath, + displayPath: path, + }); + + try { + // Guardrail 4: guarded invocation — array args only, path via argv. + // Python runs in isolated mode (-I): without it, `python -c` puts the + // cwd (the target project root) on sys.path, so a hostile project + // could shadow stdlib modules (ast.py, json.py) and execute arbitrary + // code when the serializer imports them. Ruby is safe as-is — its + // load path has not included the cwd since 1.9.2. + const pyProgram = wantComments + ? PYTHON_AST_WITH_COMMENTS_PROGRAM + : PYTHON_AST_PROGRAM; + const cmd = + language === "python" + ? [interpreter, "-I", "-c", pyProgram, sourcePath] + : [ + interpreter, + "-rripper", + "-rjson", + "-e", + RUBY_AST_PROGRAM, + sourcePath, + ]; + const { exitCode, stdout, stderr } = await runAstSubprocess(cmd); + if (exitCode !== 0) { + const detail = stderr.trim() || `exit code ${exitCode}`; + throw new Error( + `Failed to parse "${path}" as ${language}: ${detail}` + ); + } + return finalizeAstResult( + parseAstJson(stdout, path, language), + language, + wantComments + ) as AstNode; + } finally { + cleanup?.(); } - return finalizeAstResult( - parseAstJson(stdout, path, language), - language, - wantComments - ); - } finally { - cleanup?.(); } + + // Per-run parse cache, mirroring cachedGlob/cachedFileText: keyed on the + // full tuple that determines the output, NUL-joined (NUL cannot appear in + // a path, so distinct tuples never collide). The PROMISE is cached, so + // concurrent identical calls share one in-flight parse/subprocess spawn. + // Rejected promises stay cached — a deliberate decision: ctx.ast() is + // fail-closed (ARCH-022), so every rule touching the same input fails + // fast with the identical error instead of re-paying the spawn. The + // cheap argument-validation guardrails above (path safety, language + // plausibility, feature guard) still run per call, before this lookup, + // preserving ARCH-022's guardrail ordering on cache hits too. + const cacheKey = astCacheKey(absPath, language, useBase, wantComments); + let hit = caches.astResults.get(cacheKey); + if (!hit) { + hit = parseUncached(); + caches.astResults.set(cacheKey, hit); + } + return hit; } return { @@ -474,9 +505,13 @@ export async function runChecks( // invocation — shared across every ADR and rule in this run. const interpreterCache = new Map>(); - // Per-run glob/file-text caches shared across all rule contexts — rules - // overwhelmingly repeat the same globs and reads (see RunCaches). - const caches: RunCaches = { globResults: new Map(), fileText: new Map() }; + // Per-run glob/file-text/AST caches shared across all rule contexts — rules + // overwhelmingly repeat the same globs, reads, and parses (see RunCaches). + const caches: RunCaches = { + globResults: new Map(), + fileText: new Map(), + astResults: new Map(), + }; // Run ADRs in parallel const adrResults = await Promise.allSettled( diff --git a/tests/engine/runner-ast-base.test.ts b/tests/engine/runner-ast-base.test.ts index 52697e35..d7723233 100644 --- a/tests/engine/runner-ast-base.test.ts +++ b/tests/engine/runner-ast-base.test.ts @@ -406,37 +406,37 @@ describe("runChecks ctx.fileAtBase() / ctx.ast({ rev: 'base' })", () => { // away in the outer scope (TS treats a nested-closure write to a plain // `let` as never happening). const seen: { doc?: boolean; real?: boolean } = {}; - const loaded = makeLoadedAdr({ - rules: { - r: { - description: "python doc-only", - async check(ctx) { - const baseTree = await ctx.ast("src/calc.py", "python", { - rev: "base", - }); - const headTree = await ctx.ast("src/calc.py", "python"); - // A doc-only rule strips docstrings (their value legitimately - // changed) and compares the rest, position-insensitively. - seen.doc = - JSON.stringify(pyStructure(stripDocstrings(baseTree))) === - JSON.stringify(pyStructure(stripDocstrings(headTree))); - - // A real change to the working tree body. - await Bun.write( - join(dir, "src/calc.py"), - "def add(a, b):\n return a - b\n" - ); - const realTree = await ctx.ast("src/calc.py", "python"); - seen.real = - JSON.stringify(pyStructure(stripDocstrings(baseTree))) === - JSON.stringify(pyStructure(stripDocstrings(realTree))); + const compareRule = (key: "doc" | "real") => + makeLoadedAdr({ + rules: { + r: { + description: `python ${key} comparison`, + async check(ctx) { + const baseTree = await ctx.ast("src/calc.py", "python", { + rev: "base", + }); + const headTree = await ctx.ast("src/calc.py", "python"); + // A doc-only rule strips docstrings (their value legitimately + // changed) and compares the rest, position-insensitively. + seen[key] = + JSON.stringify(pyStructure(stripDocstrings(baseTree))) === + JSON.stringify(pyStructure(stripDocstrings(headTree))); + }, }, }, - }, - }); + }); - await runChecks(dir, [loaded], { base: "HEAD" }); + await runChecks(dir, [compareRule("doc")], { base: "HEAD" }); expect(seen.doc).toBe(true); + + // A real change to the working tree body. Parsed in a SEPARATE run: + // AST parses are cached per check invocation, so a mid-run edit is + // deliberately not observable within the same runChecks call. + await Bun.write( + join(dir, "src/calc.py"), + "def add(a, b):\n return a - b\n" + ); + await runChecks(dir, [compareRule("real")], { base: "HEAD" }); expect(seen.real).toBe(false); } ); diff --git a/tests/engine/runner-ast-cache.test.ts b/tests/engine/runner-ast-cache.test.ts new file mode 100644 index 00000000..01c114a6 --- /dev/null +++ b/tests/engine/runner-ast-cache.test.ts @@ -0,0 +1,284 @@ +// SPDX-License-Identifier: Apache-2.0 +// Copyright 2026 Archgate +import { describe, expect, test, beforeEach, afterEach, spyOn } from "bun:test"; +import { mkdtempSync, mkdirSync, writeFileSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; + +import { + interpreterCandidates, + probeInterpreter, +} from "../../src/engine/ast-support"; +import type { LoadResult } from "../../src/engine/loader"; +import { runChecks } from "../../src/engine/runner"; +import type { AdrDocument } from "../../src/formats/adr"; +import type { RuleSet } from "../../src/formats/rules"; +import { git, safeRmSync } from "../test-utils"; + +// Probe once at load time so interpreter-dependent tests can skipIf cleanly. +const pythonInterpreter = await probeInterpreter( + interpreterCandidates("python") +); + +function makeLoadedAdr( + ruleSet: RuleSet, + overrides: Partial = {} +): LoadResult { + return { + type: "loaded", + value: { + adr: { + frontmatter: { + id: "AST-CACHE-001", + title: "AST Cache Test", + domain: "general", + rules: true, + ...overrides, + }, + body: "", + filePath: "/test.md", + }, + ruleSet, + }, + }; +} + +/** Count Bun.spawn calls that are Python AST parses (argv[1] is "-I"). */ +function countAstSpawns(spy: { mock: { calls: unknown[][] } }): number { + return spy.mock.calls.filter((args) => { + const cmd = args[0]; + return Array.isArray(cmd) && cmd[1] === "-I"; + }).length; +} + +describe("runChecks ctx.ast() per-run parse cache", () => { + let tempDir: string; + + beforeEach(() => { + tempDir = mkdtempSync(join(tmpdir(), "archgate-ast-cache-")); + mkdirSync(join(tempDir, "src"), { recursive: true }); + }); + + afterEach(() => safeRmSync(tempDir)); + + test("identical calls across rules share one parse (same tree instance)", async () => { + writeFileSync(join(tempDir, "src", "a.ts"), "export const v = 1;\n"); + + // Each in-process parse produces a fresh object, so instance identity + // across three rules proves exactly one parse ran for the whole run. + const trees: unknown[] = []; + const rule = (name: string) => ({ + description: `parse from rule ${name}`, + async check(ctx: Parameters[0]) { + trees.push(await ctx.ast("src/a.ts", "typescript")); + }, + }); + + const loaded = makeLoadedAdr({ + rules: { one: rule("one"), two: rule("two"), three: rule("three") }, + }); + const result = await runChecks(tempDir, [loaded]); + + expect(result.results.every((r) => r.error === undefined)).toBe(true); + expect(trees).toHaveLength(3); + expect(trees[1]).toBe(trees[0]); + expect(trees[2]).toBe(trees[0]); + }); + + test("concurrent identical calls collapse into one in-flight parse", async () => { + writeFileSync(join(tempDir, "src", "b.ts"), "export const n = 2;\n"); + + let same = false; + const loaded = makeLoadedAdr({ + rules: { + concurrent: { + description: "two overlapping parses of the same file", + async check(ctx) { + const [t1, t2] = await Promise.all([ + ctx.ast("src/b.ts", "typescript"), + ctx.ast("src/b.ts", "typescript"), + ]); + same = t1 === t2; + }, + }, + }, + }); + + const result = await runChecks(tempDir, [loaded]); + expect(result.results[0].error).toBeUndefined(); + expect(same).toBe(true); + }); + + test.skipIf(!pythonInterpreter)( + "python: interpreter spawn count does not scale with rule count", + async () => { + writeFileSync(join(tempDir, "src", "calc.py"), "x = 1\n"); + + const trees: unknown[] = []; + const rule = (name: string) => ({ + description: `parse from rule ${name}`, + async check(ctx: Parameters[0]) { + trees.push(await ctx.ast("src/calc.py", "python")); + }, + }); + + const spawnSpy = spyOn(Bun, "spawn"); + try { + const loaded = makeLoadedAdr({ + rules: { one: rule("one"), two: rule("two"), three: rule("three") }, + }); + const result = await runChecks(tempDir, [loaded]); + + expect(result.results.every((r) => r.error === undefined)).toBe(true); + // Three rules, one subprocess: the parse promise is shared. + expect(countAstSpawns(spawnSpy)).toBe(1); + expect(trees[1]).toBe(trees[0]); + expect(trees[2]).toBe(trees[0]); + } finally { + spawnSpy.mockRestore(); + } + } + ); + + test("comments: true and false/omitted cache independently", async () => { + writeFileSync( + join(tempDir, "src", "c.ts"), + "// a comment\nexport const v = 1;\n" + ); + + const trees: Record = {}; + const loaded = makeLoadedAdr({ + rules: { + variants: { + description: "parse with and without comments", + async check(ctx) { + trees.plain = await ctx.ast("src/c.ts", "typescript"); + trees.withComments = await ctx.ast("src/c.ts", "typescript", { + comments: true, + }); + trees.explicitFalse = await ctx.ast("src/c.ts", "typescript", { + comments: false, + }); + }, + }, + }, + }); + + const result = await runChecks(tempDir, [loaded]); + expect(result.results[0].error).toBeUndefined(); + // Different outputs never collide... + expect(trees.withComments).not.toBe(trees.plain); + expect( + (trees.withComments as { comments?: unknown[] }).comments + ).toBeDefined(); + expect((trees.plain as { comments?: unknown[] }).comments).toBeUndefined(); + // ...while `comments: false` and omitted are the same tuple, so they share. + expect(trees.explicitFalse).toBe(trees.plain); + }); + + test("rev: 'base' and working-tree parses cache independently", async () => { + await git(["init", "--initial-branch=main"], tempDir); + await git(["config", "user.email", "t@t.com"], tempDir); + await git(["config", "user.name", "T"], tempDir); + writeFileSync(join(tempDir, "src", "d.ts"), "export function foo() {}\n"); + await git(["add", "."], tempDir); + await git(["commit", "-m", "base"], tempDir); + writeFileSync(join(tempDir, "src", "d.ts"), "export function bar() {}\n"); + + const trees: Record = {}; + const names: Record = {}; + const loaded = makeLoadedAdr({ + rules: { + revs: { + description: "base and working-tree parses", + async check(ctx) { + trees.base1 = await ctx.ast("src/d.ts", "typescript", { + rev: "base", + }); + trees.head1 = await ctx.ast("src/d.ts", "typescript"); + trees.base2 = await ctx.ast("src/d.ts", "typescript", { + rev: "base", + }); + trees.head2 = await ctx.ast("src/d.ts", "typescript"); + const name = (tree: unknown) => + (tree as { body: { declaration?: { id?: { name?: string } } }[] }) + .body[0]?.declaration?.id?.name ?? ""; + names.base = name(trees.base1); + names.head = name(trees.head1); + }, + }, + }, + }); + + const result = await runChecks(tempDir, [loaded], { base: "HEAD" }); + expect(result.results[0].error).toBeUndefined(); + // Distinct revisions never collide; repeats within a revision share. + expect(trees.base1).not.toBe(trees.head1); + expect(trees.base2).toBe(trees.base1); + expect(trees.head2).toBe(trees.head1); + expect(names.base).toBe("foo"); + expect(names.head).toBe("bar"); + }); + + test("a rejected parse is served from cache: identical error instance", async () => { + writeFileSync(join(tempDir, "src", "broken.ts"), "const = {\n"); + + // Deliberate decision: rejected promises stay cached, so every rule + // touching the same input fails fast with the SAME error (fail-closed, + // consistent with ctx.ast()'s ARCH-022 throw contract). + const errors: unknown[] = []; + const rule = (name: string) => ({ + description: `catch parse failure in rule ${name}`, + async check(ctx: Parameters[0]) { + try { + await ctx.ast("src/broken.ts", "typescript"); + } catch (err) { + errors.push(err); + } + }, + }); + + const loaded = makeLoadedAdr({ + rules: { one: rule("one"), two: rule("two") }, + }); + await runChecks(tempDir, [loaded]); + + expect(errors).toHaveLength(2); + expect(errors[1]).toBe(errors[0]); + expect((errors[0] as Error).message).toContain("Failed to parse"); + }); + + test.skipIf(!pythonInterpreter)( + "python: a rejected parse does not spawn a second interpreter", + async () => { + writeFileSync(join(tempDir, "src", "bad.py"), "def broken(:\n"); + + const errors: unknown[] = []; + const rule = (name: string) => ({ + description: `catch parse failure in rule ${name}`, + async check(ctx: Parameters[0]) { + try { + await ctx.ast("src/bad.py", "python"); + } catch (err) { + errors.push(err); + } + }, + }); + + const spawnSpy = spyOn(Bun, "spawn"); + try { + const loaded = makeLoadedAdr({ + rules: { one: rule("one"), two: rule("two") }, + }); + await runChecks(tempDir, [loaded]); + + expect(countAstSpawns(spawnSpy)).toBe(1); + expect(errors).toHaveLength(2); + expect(errors[1]).toBe(errors[0]); + expect((errors[0] as Error).message).toContain("Failed to parse"); + } finally { + spawnSpy.mockRestore(); + } + } + ); +}); From f66c0cc4c57c8ae5090574ad1cc4125127033491 Mon Sep 17 00:00:00 2001 From: rhuanbarreto <283004+rhuanbarreto@users.noreply.github.com> Date: Thu, 16 Jul 2026 21:11:07 +0000 Subject: [PATCH 2/3] docs: regenerate llms-full.txt Signed-off-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> --- docs/public/llms-full.txt | 2 ++ 1 file changed, 2 insertions(+) diff --git a/docs/public/llms-full.txt b/docs/public/llms-full.txt index 43046691..3e863be2 100644 --- a/docs/public/llms-full.txt +++ b/docs/public/llms-full.txt @@ -5364,6 +5364,8 @@ interface AstOptions { Parse a source file into its language-native AST. The path is relative to the project root and passes through the same sandbox as `readFile`. TypeScript and JavaScript are parsed in-process; Python and Ruby are parsed by invoking the system interpreter's own standard-library AST facility as a subprocess. The returned tree shape differs per language -- see [AstNode](#astnode). +Parse results are cached for the lifetime of a single `archgate check` run, keyed on `(path, language, rev, comments)`: repeated -- even concurrent -- identical calls across rules cost one parse (one interpreter spawn for Python/Ruby), and a failed parse rethrows the same error to every caller. Treat the returned tree as read-only -- it may be shared with other rules. + ```typescript const program = await ctx.ast("src/cli.ts", "typescript"); for (const node of program.body) { From 9afa9df523eb5797753d14288ba1356f65117fd1 Mon Sep 17 00:00:00 2001 From: Rhuan Barreto Date: Thu, 16 Jul 2026 23:37:41 -0300 Subject: [PATCH 3/3] fix(engine): use normalized path in cached ctx.ast() error messages A cached parse rejection is shared by every caller of the same input tuple, but its message interpolated the raw, closure-captured path from whichever caller populated the cache first. Since safePath() collapses aliased spellings (e.g. src/./a.ts and src/a.ts) into one cache entry, a later caller could receive an error mentioning another rule's spelling. Interpolate the normalized repo-relative path in everything thrown from the cached parse closure instead. The per-call guardrail errors ahead of the cache lookup still echo the caller's own spelling. Signed-off-by: Rhuan Barreto --- src/engine/runner.ts | 22 +++++++++++------ tests/engine/runner-ast-cache.test.ts | 35 +++++++++++++++++++++++++++ 2 files changed, 50 insertions(+), 7 deletions(-) diff --git a/src/engine/runner.ts b/src/engine/runner.ts index bf58619e..bfbb3d6b 100644 --- a/src/engine/runner.ts +++ b/src/engine/runner.ts @@ -248,21 +248,29 @@ function createRuleContext( throw commentsUnsupportedError(language, path); } - /** The uncached parse: TS/JS in-process, Python/Ruby via guardrails 3–4. */ + /** + * The uncached parse: TS/JS in-process, Python/Ruby via guardrails 3–4. + * Errors below are CACHED (see the astResults lookup), so they + * interpolate the normalized `relPath` — never the raw `path` — + * otherwise a cached rejection would carry the first caller's path + * spelling (e.g. "src/./a.py") into every later caller's error, since + * aliased spellings resolve to the same cache entry. + */ async function parseUncached(): Promise { // In-process branch: TypeScript/JavaScript via the shared meriyah // parser (js-parser.ts). No subprocess is spawned for these languages. if (language === "typescript" || language === "javascript") { const source = useBase - ? await readBaseSourceOrThrow(projectRoot, baseRev, relPath, path) + ? await readBaseSourceOrThrow(projectRoot, baseRev, relPath, relPath) : await cachedFileText(absPath); try { // Meriyah's Program is ESTree-shaped but lacks the index signature. + // `path` is safe here: it only picks the parse mode by extension. const tree = parseTsOrJsSource(language, path, source, wantComments); return tree as unknown as EsTreeProgram; } catch (err) { throw new Error( - `Failed to parse "${path}" as ${language}: ${parseErrorMessage(err)}` + `Failed to parse "${relPath}" as ${language}: ${parseErrorMessage(err)}` ); } } @@ -276,7 +284,7 @@ function createRuleContext( } const interpreter = await probe; if (!interpreter) { - throw interpreterNotFoundError(language, candidates, path); + throw interpreterNotFoundError(language, candidates, relPath); } // For { rev: "base" }, the interpreter serializers read a file path from @@ -290,7 +298,7 @@ function createRuleContext( projectRoot, baseRev, relPath, - displayPath: path, + displayPath: relPath, }); try { @@ -318,11 +326,11 @@ function createRuleContext( if (exitCode !== 0) { const detail = stderr.trim() || `exit code ${exitCode}`; throw new Error( - `Failed to parse "${path}" as ${language}: ${detail}` + `Failed to parse "${relPath}" as ${language}: ${detail}` ); } return finalizeAstResult( - parseAstJson(stdout, path, language), + parseAstJson(stdout, relPath, language), language, wantComments ) as AstNode; diff --git a/tests/engine/runner-ast-cache.test.ts b/tests/engine/runner-ast-cache.test.ts index 01c114a6..3575f613 100644 --- a/tests/engine/runner-ast-cache.test.ts +++ b/tests/engine/runner-ast-cache.test.ts @@ -248,6 +248,41 @@ describe("runChecks ctx.ast() per-run parse cache", () => { expect((errors[0] as Error).message).toContain("Failed to parse"); }); + test("aliased path spellings share one cache entry and a normalized error message", async () => { + writeFileSync(join(tempDir, "src", "broken2.ts"), "const = {\n"); + + // "src/./broken2.ts" and "src/broken2.ts" resolve to the same absPath, + // so both spellings hit one cache entry. The cached rejection must not + // leak the first caller's raw spelling: the message always carries the + // normalized repo-relative path. + const errors: unknown[] = []; + const rule = (spelling: string) => ({ + description: `parse via spelling ${spelling}`, + async check(ctx: Parameters[0]) { + try { + await ctx.ast(spelling, "typescript"); + } catch (err) { + errors.push(err); + } + }, + }); + + const loaded = makeLoadedAdr({ + rules: { + aliased: rule("src/./broken2.ts"), + canonical: rule("src/broken2.ts"), + }, + }); + await runChecks(tempDir, [loaded]); + + expect(errors).toHaveLength(2); + // One cache entry for both spellings: the very same error instance. + expect(errors[1]).toBe(errors[0]); + const message = (errors[0] as Error).message; + expect(message).toContain('"src/broken2.ts"'); + expect(message).not.toContain("src/./broken2.ts"); + }); + test.skipIf(!pythonInterpreter)( "python: a rejected parse does not spawn a second interpreter", async () => {