From c8a72792447bb60b34dd61e7b07d5bc796d3eaeb Mon Sep 17 00:00:00 2001 From: Rhuan Barreto Date: Sun, 26 Jul 2026 13:28:39 +0200 Subject: [PATCH 1/3] fix(rules): ARCH-020/glob-scan-dot matches ctx.ast() instead of raw text glob-scan-dot regexed raw source for `.scan(` calls, so a comment or string literal merely mentioning `.scan()` was misreported as a violation (fixes #513). It now walks the ESTree via ctx.ast() / ctx.findAstNodes() for real `.scan(...)` CallExpression nodes, so comments and strings can't match by construction. Line numbers are re-located in the original source (ctx.ast()'s loc isn't trustworthy for "typescript", per ARCH-022), with comments/strings blanked first so the re-location search has the same guarantee. Also documents ctx.findAstNodes() as the preferred replacement for a hand-rolled walk() helper in ARCH-022, since none of its own example rules used it yet (it shipped after they were written). Fire-tested both directions: a genuine scan() without dot:true still fails, and a comment/string containing the literal text no longer does. Signed-off-by: Rhuan Barreto --- .../ARCH-020-glob-scan-include-dotfiles.md | 4 +- ...CH-020-glob-scan-include-dotfiles.rules.ts | 149 ++++++++++++++++-- .../adrs/ARCH-022-ast-aware-rule-context.md | 1 + .../agent-memory/archgate-developer/MEMORY.md | 2 +- .../project_rules_engine_internals.md | 3 +- 5 files changed, 142 insertions(+), 17 deletions(-) diff --git a/.archgate/adrs/ARCH-020-glob-scan-include-dotfiles.md b/.archgate/adrs/ARCH-020-glob-scan-include-dotfiles.md index 36387567..c008f043 100644 --- a/.archgate/adrs/ARCH-020-glob-scan-include-dotfiles.md +++ b/.archgate/adrs/ARCH-020-glob-scan-include-dotfiles.md @@ -55,7 +55,7 @@ Every call to `Bun.Glob#scan()` (`glob.scan(...)`) in source MUST pass `{ dot: t ### Automated -- **Archgate rule** ARCH-020/glob-scan-dot: Scans `src/**/*.ts` for `.scan(` calls and reports any whose argument list does not contain `dot:`. Severity: error. +- **Archgate rule** ARCH-020/glob-scan-dot: Parses `src/**/*.ts` via `ctx.ast()` ([ARCH-022](./ARCH-022-ast-aware-rule-context.md)) and walks the ESTree for real `.scan(...)` `CallExpression` nodes, reporting any whose argument list has no object literal with a `dot` key. Structural, not text-based, so a comment or string that merely mentions `.scan()` cannot be misreported (see [archgate/cli#513](https://github.com/archgate/cli/issues/513)). Severity: error. ### Manual @@ -64,5 +64,7 @@ Code reviewers MUST verify new `Bun.Glob` scans pass `dot: true` and that any in ## References - [archgate/cli#222](https://github.com/archgate/cli/issues/222) — the dotfile-skipping bug this ADR prevents +- [archgate/cli#513](https://github.com/archgate/cli/issues/513) — the companion rule's regex-over-raw-text false positive on comments/strings, fixed by moving to `ctx.ast()` - [`src/engine/runner.ts`](../../src/engine/runner.ts), [`src/engine/git-files.ts`](../../src/engine/git-files.ts) — canonical `scan({ dot: true })` usage - [ARCH-009: Platform Detection Helper](./ARCH-009-platform-detection-helper.md) — related cross-platform correctness governance +- [ARCH-022: AST-Aware Rule Context](./ARCH-022-ast-aware-rule-context.md) — `ctx.ast()` / `ctx.findAstNodes()`, the structural inspection primitive this rule's companion check now uses diff --git a/.archgate/adrs/ARCH-020-glob-scan-include-dotfiles.rules.ts b/.archgate/adrs/ARCH-020-glob-scan-include-dotfiles.rules.ts index eb74c3ab..5a3c1630 100644 --- a/.archgate/adrs/ARCH-020-glob-scan-include-dotfiles.rules.ts +++ b/.archgate/adrs/ARCH-020-glob-scan-include-dotfiles.rules.ts @@ -1,5 +1,109 @@ /// +/** + * ARCH-020 enforcement on top of ctx.ast() / ctx.findAstNodes() (ARCH-022): + * walks the ESTree for real `.scan(...)` CallExpression nodes, so a + * comment or string literal that merely mentions `.scan()` is not a match. + */ + +/** True for `.scan(...)` -- a non-computed `.scan` member call. */ +function isScanCall(node: EsTreeNode): boolean { + if (node.type !== "CallExpression") return false; + const callee = node.callee as EsTreeNode | undefined; + if (callee?.type !== "MemberExpression" || callee.computed === true) { + return false; + } + const property = callee.property as EsTreeNode | undefined; + return property?.type === "Identifier" && property.name === "scan"; +} + +/** Does this call's argument list include an object literal with a `dot` key? */ +function hasDotOption(call: EsTreeNode): boolean { + const args = (call.arguments as EsTreeNode[] | undefined) ?? []; + return args.some((arg) => { + if (arg.type !== "ObjectExpression") return false; + const properties = (arg.properties as EsTreeNode[] | undefined) ?? []; + return properties.some((prop) => { + if (prop.type !== "Property" || prop.computed === true) return false; + const key = prop.key as + | (EsTreeNode & { name?: unknown; value?: unknown }) + | undefined; + if (key?.type === "Identifier") return key.name === "dot"; + if (key?.type === "Literal") return key.value === "dot"; + return false; + }); + }); +} + +/** + * Blank comments and string/template literals to spaces, keeping every + * newline, so line numbers computed against the result match the original + * source. Re-locates a call ctx.ast() already found structurally, since + * `loc` is not trustworthy for `"typescript"` (ARCH-022). Does not track + * regex literals, matching `source-positions.ts`. + */ +function blankNonCode(source: string): string { + let out = ""; + let i = 0; + const n = source.length; + while (i < n) { + const ch = source[i]; + const next = source[i + 1]; + if (ch === "/" && next === "/") { + while (i < n && source[i] !== "\n") { + out += " "; + i++; + } + continue; + } + if (ch === "/" && next === "*") { + out += " "; + i += 2; + while (i < n && !(source[i] === "*" && source[i + 1] === "/")) { + out += source[i] === "\n" ? "\n" : " "; + i++; + } + if (i < n) { + out += " "; + i += 2; + } + continue; + } + if (ch === '"' || ch === "'" || ch === "`") { + const quote = ch; + out += " "; + i++; + while (i < n && source[i] !== quote) { + if (source[i] === "\\" && i + 1 < n) { + out += " "; + i += 2; + continue; + } + out += source[i] === "\n" ? "\n" : " "; + i++; + } + if (i < n) { + out += " "; + i++; + } + continue; + } + out += ch; + i++; + } + return out; +} + +/** 1-based line of the next `.scan(` at or after `fromIndex` in blanked `code`. */ +function nextScanLine( + code: string, + fromIndex: number +): { line: number | undefined; nextIndex: number } { + const idx = code.indexOf(".scan(", fromIndex); + if (idx === -1) return { line: undefined, nextIndex: fromIndex }; + return { line: code.slice(0, idx).split("\n").length, nextIndex: idx + 6 }; +} + export default { rules: { "glob-scan-dot": { @@ -9,32 +113,49 @@ export default { async check(ctx) { const files = ctx.scopedFiles.filter((f) => f.endsWith(".ts")); - // Capture the argument list of each `.scan( ... )` call. The character - // class `[^)]` spans newlines, so multi-line option objects are - // covered, as long as the args contain no nested `)` (true for glob - // option objects: `{ cwd, dot: true }`). - const callPattern = /\.scan\(([^)]*)\)/gu; - const checks = files.map(async (file) => { - let content: string; + let tree: EsTreeProgram; try { - content = await ctx.readFile(file); + tree = await ctx.ast(file, "typescript"); } catch { return; } - for (const match of content.matchAll(callPattern)) { - const args = match[1]; - if (/\bdot\s*:/u.test(args)) continue; + const scanCalls = ctx + .findAstNodes(tree, "CallExpression") + .filter((node) => isScanCall(node)); + if (scanCalls.length === 0) return; + + // Sort by transpiled loc: not source-accurate for "typescript" + // (ARCH-022), but Bun's transpiler only erases type-only syntax, + // never reorders statements, so relative order is preserved -- + // enough to pair each call with its re-located line below. + scanCalls.sort((a, b) => { + const lineDiff = + (a.loc?.start.line ?? 0) - (b.loc?.start.line ?? 0); + if (lineDiff !== 0) return lineDiff; + return (a.loc?.start.column ?? 0) - (b.loc?.start.column ?? 0); + }); + + let source: string; + try { + source = await ctx.readFile(file); + } catch { + return; + } + const code = blankNonCode(source); - const offset = match.index ?? 0; - const line = content.slice(0, offset).split("\n").length; + let cursor = 0; + for (const call of scanCalls) { + const { line, nextIndex } = nextScanLine(code, cursor); + cursor = nextIndex; + if (hasDotOption(call)) continue; ctx.report.violation({ message: "Bun.Glob#scan() must pass { dot: true } or it silently skips dot-prefixed directories on Windows", file, - line, + ...(line === undefined ? {} : { line }), fix: "Add `dot: true` to the scan options, e.g. `glob.scan({ cwd, dot: true })`", }); } diff --git a/.archgate/adrs/ARCH-022-ast-aware-rule-context.md b/.archgate/adrs/ARCH-022-ast-aware-rule-context.md index b7db07ad..20d2a830 100644 --- a/.archgate/adrs/ARCH-022-ast-aware-rule-context.md +++ b/.archgate/adrs/ARCH-022-ast-aware-rule-context.md @@ -112,6 +112,7 @@ Dispatch on `language` MUST be invisible to rule authors. - **Comment-governance rules become structural** — length, style, and content policies can be written against structured comment tokens (`type`/`value`/`loc`) with original-source-accurate positions instead of fragile line/regex heuristics. - **Failure visibility reuses proven machinery** — no new exit code, reporter branch, or error-boundary design; throw-on-failure rides on `runner.ts`'s per-rule isolation and `reporter.ts`'s exit-code-2 category. - **Incremental adoption** — TS/JS support needs no new capability surface beyond what exists internally, and Python/Ruby can follow independently since the guardrail and failure-semantics design is identical for both. +- **`ctx.findAstNodes(tree, ...types)` retires the hand-rolled `walk(node, visit)` helper each rule file previously had to repeat** — it covers "collect every node of type X" for any language's tree shape by matching the `_type`/`type` discriminant in preorder. A custom walk is still needed when a rule must prune a subtree before collecting (e.g. skip descending into nested function bodies), not merely filter by type. ### Negative diff --git a/.claude/agent-memory/archgate-developer/MEMORY.md b/.claude/agent-memory/archgate-developer/MEMORY.md index 860adff3..0cd75147 100644 --- a/.claude/agent-memory/archgate-developer/MEMORY.md +++ b/.claude/agent-memory/archgate-developer/MEMORY.md @@ -39,4 +39,4 @@ Exceptions: minor follow-up tweaks after validation already passed, and non-code - [Docs are forward-only and version-independent](feedback_forward_only_docs.md) — no pinned versions or drift-prone counts; nothing enforces this - [Claude Code hooks config](project_claude_code_hooks_config.md) — the `"shell": "bash"` requirement and the `WorktreeCreate` contract - [PR review thread triage](project_pr_review_thread_triage.md) — REST hides resolution state; use the GraphQL `reviewThreads.isResolved` field -- [Rules engine follow-up](project_rules_engine_internals.md) — the one pending perf item no rule tracks +- [Rules engine follow-up](project_rules_engine_internals.md) — pending perf item, and ARCH-023's unfixed sibling of the #513 regex-over-text bug diff --git a/.claude/agent-memory/archgate-developer/project_rules_engine_internals.md b/.claude/agent-memory/archgate-developer/project_rules_engine_internals.md index af15634a..7e72a7c4 100644 --- a/.claude/agent-memory/archgate-developer/project_rules_engine_internals.md +++ b/.claude/agent-memory/archgate-developer/project_rules_engine_internals.md @@ -1,8 +1,9 @@ --- name: project-rules-engine-internals -description: Open follow-up work in the ADR rules engine that no rule or test tracks +description: Open follow-up work and known unfixed bugs in the ADR rules engine that no rule or test tracks metadata: type: project --- - Rules-file load is still re-parsed per invocation — `runner.ts`'s per-run caches do NOT cover it; deferred, see #345. +- `ARCH-023-engine-file-listing-via-in-memory-git-tracked-matching.rules.ts`'s `scan-confined-to-fallback-modules` check has the same class of bug archgate/cli#513 fixed in ARCH-020's `glob-scan-dot`: it regexes raw source (`/\.scan\(([^)]*)\)/gu`, per its own comment "Same call-site detection as ARCH-020's glob-scan-dot rule") instead of walking `ctx.ast()`, so a comment or string mentioning `.scan(` in a `src/engine/` file outside the allowlist is misreported as a violation. Confirmed live 2026-07-26 by fire-testing a fixture with a `.scan()`-mentioning comment and string literal (no real call) — both false-flagged at their own lines. Not yet fixed; out of #513's scope (that issue named only ARCH-020). If asked to fix rule-authoring text-matching bugs in this repo, check this rule too — the fix is a straight port of ARCH-020's `ctx.findAstNodes`-based approach (see [[feedback_prefer_tests_over_adr_rules]] for the enforcement-layer framing). From 2e007585c68c8b67e981c1c61c5e26b3f0245d95 Mon Sep 17 00:00:00 2001 From: Rhuan Barreto Date: Sun, 26 Jul 2026 15:01:07 +0200 Subject: [PATCH 2/3] fix(rules): ARCH-023/scan-confined-to-fallback-modules matches ctx.ast() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Same bug class as ARCH-020 (#513), confirmed live in this rule too: it regexed raw source for `.scan(` call sites in src/engine/, so a comment or string literal merely mentioning `.scan()` in a non-allowlisted file was misreported as a violation. Ports ARCH-020's fix — walk the ESTree via ctx.ast()/ctx.findAstNodes() for real `.scan(...)` call sites, with the same comment/string-blanking approach for line re-location. Fire-tested both directions: a genuine scan() call outside the two allowlisted modules is still flagged, and a comment/string containing the literal text no longer is. Signed-off-by: Rhuan Barreto --- ...ting-via-in-memory-git-tracked-matching.md | 2 +- ...ia-in-memory-git-tracked-matching.rules.ts | 126 ++++++++++++++++-- .../agent-memory/archgate-developer/MEMORY.md | 2 +- .../project_rules_engine_internals.md | 3 +- 4 files changed, 117 insertions(+), 16 deletions(-) diff --git a/.archgate/adrs/ARCH-023-engine-file-listing-via-in-memory-git-tracked-matching.md b/.archgate/adrs/ARCH-023-engine-file-listing-via-in-memory-git-tracked-matching.md index aea9df4a..d169f370 100644 --- a/.archgate/adrs/ARCH-023-engine-file-listing-via-in-memory-git-tracked-matching.md +++ b/.archgate/adrs/ARCH-023-engine-file-listing-via-in-memory-git-tracked-matching.md @@ -72,7 +72,7 @@ The rules engine (`src/engine/`) MUST list files by matching globs **in memory** ## Compliance and Enforcement -- **Automated:** The companion rule `scan-confined-to-fallback-modules` (this ADR) blocks `Bun.Glob#scan()` call sites in `src/engine/` outside `glob-utils.ts`/`git-files.ts`. ARCH-020's `glob-scan-dot` rule covers `dot: true` on the remaining fallbacks. `archgate check` runs both in CI and pre-push. +- **Automated:** The companion rule `scan-confined-to-fallback-modules` (this ADR) parses `src/engine/**/*.ts` via `ctx.ast()` ([ARCH-022](./ARCH-022-ast-aware-rule-context.md)) and walks the ESTree for real `.scan(...)` call sites, blocking any outside `glob-utils.ts`/`git-files.ts` — structural, not text-based, so a comment or string mentioning `.scan(` cannot be misreported. ARCH-020's `glob-scan-dot` rule covers `dot: true` on the remaining fallbacks. `archgate check` runs both in CI and pre-push. - **Manual:** Reviewers of `src/engine/` changes verify new file listings route through `glob-utils.ts` and that sandbox validation precedes the tracked/scan branch. - **Exceptions:** A new scan call site outside the two fallback modules requires updating this ADR (and its rule's allowlist) with justification approved by the maintainer. diff --git a/.archgate/adrs/ARCH-023-engine-file-listing-via-in-memory-git-tracked-matching.rules.ts b/.archgate/adrs/ARCH-023-engine-file-listing-via-in-memory-git-tracked-matching.rules.ts index ce110afa..7a35443c 100644 --- a/.archgate/adrs/ARCH-023-engine-file-listing-via-in-memory-git-tracked-matching.rules.ts +++ b/.archgate/adrs/ARCH-023-engine-file-listing-via-in-memory-git-tracked-matching.rules.ts @@ -3,13 +3,95 @@ /** * ARCH-023: file listing in src/engine/ must match in memory against the * git-tracked set. Bun.Glob scanning is fallback-only and confined to the - * two modules that implement the fallback. + * two modules that implement the fallback. Walks the ESTree via + * ctx.ast()/ctx.findAstNodes() (ARCH-022) for real `.scan(...)` call sites, + * so a comment or string mentioning `.scan(` is not a match. */ const SCAN_ALLOWED_FILES = new Set([ "src/engine/glob-utils.ts", "src/engine/git-files.ts", ]); +/** True for `.scan(...)` -- a non-computed `.scan` member call. */ +function isScanCall(node: EsTreeNode): boolean { + if (node.type !== "CallExpression") return false; + const callee = node.callee as EsTreeNode | undefined; + if (callee?.type !== "MemberExpression" || callee.computed === true) { + return false; + } + const property = callee.property as EsTreeNode | undefined; + return property?.type === "Identifier" && property.name === "scan"; +} + +/** + * Blank comments and string/template literals to spaces, keeping every + * newline, so line numbers computed against the result match the original + * source. Re-locates a call ctx.ast() already found structurally, since + * `loc` is not trustworthy for `"typescript"` (ARCH-022). Does not track + * regex literals, matching `source-positions.ts`. + */ +function blankNonCode(source: string): string { + let out = ""; + let i = 0; + const n = source.length; + while (i < n) { + const ch = source[i]; + const next = source[i + 1]; + if (ch === "/" && next === "/") { + while (i < n && source[i] !== "\n") { + out += " "; + i++; + } + continue; + } + if (ch === "/" && next === "*") { + out += " "; + i += 2; + while (i < n && !(source[i] === "*" && source[i + 1] === "/")) { + out += source[i] === "\n" ? "\n" : " "; + i++; + } + if (i < n) { + out += " "; + i += 2; + } + continue; + } + if (ch === '"' || ch === "'" || ch === "`") { + const quote = ch; + out += " "; + i++; + while (i < n && source[i] !== quote) { + if (source[i] === "\\" && i + 1 < n) { + out += " "; + i += 2; + continue; + } + out += source[i] === "\n" ? "\n" : " "; + i++; + } + if (i < n) { + out += " "; + i++; + } + continue; + } + out += ch; + i++; + } + return out; +} + +/** 1-based line of the next `.scan(` at or after `fromIndex` in blanked `code`. */ +function nextScanLine( + code: string, + fromIndex: number +): { line: number | undefined; nextIndex: number } { + const idx = code.indexOf(".scan(", fromIndex); + if (idx === -1) return { line: undefined, nextIndex: fromIndex }; + return { line: code.slice(0, idx).split("\n").length, nextIndex: idx + 6 }; +} + export default { rules: { "scan-confined-to-fallback-modules": { @@ -21,28 +103,48 @@ export default { (f) => f.endsWith(".ts") && !SCAN_ALLOWED_FILES.has(f) ); - // Same call-site detection as ARCH-020's glob-scan-dot rule: capture - // the argument list of each scan call. `[^)]` spans newlines, so - // multi-line option objects are covered. - const callPattern = /\.scan\(([^)]*)\)/gu; - const checks = files.map(async (file) => { - let content: string; + let tree: EsTreeProgram; + try { + tree = await ctx.ast(file, "typescript"); + } catch { + return; + } + + const scanCalls = ctx + .findAstNodes(tree, "CallExpression") + .filter((node) => isScanCall(node)); + if (scanCalls.length === 0) return; + + // Sort by transpiled loc: not source-accurate for "typescript" + // (ARCH-022), but relative order survives transpilation, which + // only erases type-only syntax -- enough to pair each call with + // its re-located line below. + scanCalls.sort((a, b) => { + const lineDiff = + (a.loc?.start.line ?? 0) - (b.loc?.start.line ?? 0); + if (lineDiff !== 0) return lineDiff; + return (a.loc?.start.column ?? 0) - (b.loc?.start.column ?? 0); + }); + + let source: string; try { - content = await ctx.readFile(file); + source = await ctx.readFile(file); } catch { return; } + const code = blankNonCode(source); - for (const match of content.matchAll(callPattern)) { - const offset = match.index ?? 0; - const line = content.slice(0, offset).split("\n").length; + let cursor = 0; + for (const _call of scanCalls) { + const { line, nextIndex } = nextScanLine(code, cursor); + cursor = nextIndex; ctx.report.violation({ message: "Bun.Glob#scan() in src/engine/ is fallback-only and confined to glob-utils.ts/git-files.ts — walking the filesystem per rule re-introduces the traversal cost ARCH-023 eliminates", file, - line, + ...(line === undefined ? {} : { line }), fix: "Use listMatchingFiles() or matchTrackedFiles() from src/engine/glob-utils.ts; if a genuine new fallback is required, update ARCH-023 and its allowlist with maintainer approval", }); } diff --git a/.claude/agent-memory/archgate-developer/MEMORY.md b/.claude/agent-memory/archgate-developer/MEMORY.md index 0cd75147..860adff3 100644 --- a/.claude/agent-memory/archgate-developer/MEMORY.md +++ b/.claude/agent-memory/archgate-developer/MEMORY.md @@ -39,4 +39,4 @@ Exceptions: minor follow-up tweaks after validation already passed, and non-code - [Docs are forward-only and version-independent](feedback_forward_only_docs.md) — no pinned versions or drift-prone counts; nothing enforces this - [Claude Code hooks config](project_claude_code_hooks_config.md) — the `"shell": "bash"` requirement and the `WorktreeCreate` contract - [PR review thread triage](project_pr_review_thread_triage.md) — REST hides resolution state; use the GraphQL `reviewThreads.isResolved` field -- [Rules engine follow-up](project_rules_engine_internals.md) — pending perf item, and ARCH-023's unfixed sibling of the #513 regex-over-text bug +- [Rules engine follow-up](project_rules_engine_internals.md) — the one pending perf item no rule tracks diff --git a/.claude/agent-memory/archgate-developer/project_rules_engine_internals.md b/.claude/agent-memory/archgate-developer/project_rules_engine_internals.md index 7e72a7c4..af15634a 100644 --- a/.claude/agent-memory/archgate-developer/project_rules_engine_internals.md +++ b/.claude/agent-memory/archgate-developer/project_rules_engine_internals.md @@ -1,9 +1,8 @@ --- name: project-rules-engine-internals -description: Open follow-up work and known unfixed bugs in the ADR rules engine that no rule or test tracks +description: Open follow-up work in the ADR rules engine that no rule or test tracks metadata: type: project --- - Rules-file load is still re-parsed per invocation — `runner.ts`'s per-run caches do NOT cover it; deferred, see #345. -- `ARCH-023-engine-file-listing-via-in-memory-git-tracked-matching.rules.ts`'s `scan-confined-to-fallback-modules` check has the same class of bug archgate/cli#513 fixed in ARCH-020's `glob-scan-dot`: it regexes raw source (`/\.scan\(([^)]*)\)/gu`, per its own comment "Same call-site detection as ARCH-020's glob-scan-dot rule") instead of walking `ctx.ast()`, so a comment or string mentioning `.scan(` in a `src/engine/` file outside the allowlist is misreported as a violation. Confirmed live 2026-07-26 by fire-testing a fixture with a `.scan()`-mentioning comment and string literal (no real call) — both false-flagged at their own lines. Not yet fixed; out of #513's scope (that issue named only ARCH-020). If asked to fix rule-authoring text-matching bugs in this repo, check this rule too — the fix is a straight port of ARCH-020's `ctx.findAstNodes`-based approach (see [[feedback_prefer_tests_over_adr_rules]] for the enforcement-layer framing). From c560a7f5baa35bf620faa81c8a590b7f01dcd23e Mon Sep 17 00:00:00 2001 From: Rhuan Barreto Date: Sun, 26 Jul 2026 15:09:12 +0200 Subject: [PATCH 3/3] fix(rules): ARCH-020's hasDotOption rejects a literal dot: false CodeRabbit review on PR #533: hasDotOption checked only for the dot key's presence, not its value, so `glob.scan({ cwd, dot: false })` passed the check while reproducing the exact silent-skip bug ARCH-020 exists to prevent. A literal value must now be true; a non-literal value (identifier, expression) can't be resolved statically and stays treated as compliant, avoiding false positives on constants/config. Also updates the ADR's Compliance wording to match, and confirms (by reading resolveScopedFiles/git-files.ts) that a second review finding -- the rule's file scope allegedly not being confined to src/** -- is already false: ctx.scopedFiles is pre-scoped to the ADR's files glob before the rule runs, matching CodeRabbit's own follow-up on the same finding. Fire-tested: dot: false is now flagged; dot: true and a non-literal dot: someConst are not. Signed-off-by: Rhuan Barreto --- .../ARCH-020-glob-scan-include-dotfiles.md | 2 +- ...CH-020-glob-scan-include-dotfiles.rules.ts | 19 +++++++++++++++---- 2 files changed, 16 insertions(+), 5 deletions(-) diff --git a/.archgate/adrs/ARCH-020-glob-scan-include-dotfiles.md b/.archgate/adrs/ARCH-020-glob-scan-include-dotfiles.md index c008f043..c57d7c5b 100644 --- a/.archgate/adrs/ARCH-020-glob-scan-include-dotfiles.md +++ b/.archgate/adrs/ARCH-020-glob-scan-include-dotfiles.md @@ -55,7 +55,7 @@ Every call to `Bun.Glob#scan()` (`glob.scan(...)`) in source MUST pass `{ dot: t ### Automated -- **Archgate rule** ARCH-020/glob-scan-dot: Parses `src/**/*.ts` via `ctx.ast()` ([ARCH-022](./ARCH-022-ast-aware-rule-context.md)) and walks the ESTree for real `.scan(...)` `CallExpression` nodes, reporting any whose argument list has no object literal with a `dot` key. Structural, not text-based, so a comment or string that merely mentions `.scan()` cannot be misreported (see [archgate/cli#513](https://github.com/archgate/cli/issues/513)). Severity: error. +- **Archgate rule** ARCH-020/glob-scan-dot: Parses `src/**/*.ts` via `ctx.ast()` ([ARCH-022](./ARCH-022-ast-aware-rule-context.md)) and walks the ESTree for real `.scan(...)` `CallExpression` nodes, reporting any whose argument list has no `dot` key with a `true` (or non-statically-resolvable) value — a literal `dot: false` is flagged. Structural, not text-based, so a comment or string that merely mentions `.scan()` cannot be misreported (see [archgate/cli#513](https://github.com/archgate/cli/issues/513)). Severity: error. ### Manual diff --git a/.archgate/adrs/ARCH-020-glob-scan-include-dotfiles.rules.ts b/.archgate/adrs/ARCH-020-glob-scan-include-dotfiles.rules.ts index 5a3c1630..ae87d06c 100644 --- a/.archgate/adrs/ARCH-020-glob-scan-include-dotfiles.rules.ts +++ b/.archgate/adrs/ARCH-020-glob-scan-include-dotfiles.rules.ts @@ -17,7 +17,12 @@ function isScanCall(node: EsTreeNode): boolean { return property?.type === "Identifier" && property.name === "scan"; } -/** Does this call's argument list include an object literal with a `dot` key? */ +/** + * Does this call's argument list include a `dot` option that isn't + * disqualified? A non-literal value (identifier, expression) can't be + * resolved statically, so it's treated as compliant; a literal value must + * be `true` -- `dot: false` reproduces the exact bug this ADR prevents. + */ function hasDotOption(call: EsTreeNode): boolean { const args = (call.arguments as EsTreeNode[] | undefined) ?? []; return args.some((arg) => { @@ -28,9 +33,15 @@ function hasDotOption(call: EsTreeNode): boolean { const key = prop.key as | (EsTreeNode & { name?: unknown; value?: unknown }) | undefined; - if (key?.type === "Identifier") return key.name === "dot"; - if (key?.type === "Literal") return key.value === "dot"; - return false; + const isDotKey = + key?.type === "Identifier" + ? key.name === "dot" + : key?.type === "Literal" && key.value === "dot"; + if (!isDotKey) return false; + const value = prop.value as + | (EsTreeNode & { value?: unknown }) + | undefined; + return value?.type !== "Literal" || value.value === true; }); }); }