From f10261f42401f9ecfb792f616ead8f59a56b51c2 Mon Sep 17 00:00:00 2001 From: gsxdsm Date: Fri, 31 Jul 2026 00:27:41 -0700 Subject: [PATCH] fix(scripts): the contamination audit scanned four legacy lanes and claimed it had (#3005) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## An audit that scanned four legacy lanes — and claimed it had Two halves of the same wrong answer. **The query allowlisted the lanes:** ```sql WHERE deleted_at IS NULL AND "column" IN ('triage','todo','in-progress','in-review') ``` On a board whose lanes are named anything else that matches **nothing**, so the audit scans zero rows and reports zero contamination — a clean bill of health from a scan that never happened. `triage` is in that list too, a lane U11 (#2515) deleted. **And the report asserted the coverage it did not have:** ```js scannedColumns: ["triage", "todo", "in-progress", "in-review"], ``` printed regardless of what the query returned. When I first surveyed this script I called that field "the one thing keeping it from being fully silent" — it turns out it was a **claim, not an observation**, so it was not keeping it honest at all. It is now derived from the rows that came back. ## Fix: exclude finished lanes instead of allowlisting active ones Inverted so the default is the safe one — an unrecognised lane is active work by assumption and **is** audited; only lanes that genuinely mean finished drop out. An allowlist fails **closed** (skip everything unknown), a denylist fails **open** (look at it), and for an audit one extra finished branch is a far smaller error than auditing nothing. Filtered in JS rather than by building a dynamic SQL exclusion: it keeps **one** place deciding what "finished" means, and removes the last raw-SQL lane literal from this file. ## Revert proof ``` ✖ scannedColumns reports the board's real lanes, not a fixed legacy claim ✖ reports each scanned lane once, and nothing at all for an empty board ℹ pass 1 ℹ fail 2 ``` ## A demonstration of #3000, for free This PR removes a 4-literal raw-SQL clause, and `check-sql-column-literals` here reports **22, unchanged and green** — because this branch predates #3000 and the gate still walks `packages/` only. That is precisely the blind spot #3000 closes, reproduced a second time. ## Merge order This removes the 4 literals #3000 baselines. Landing this **after** #3000 drops that count and its gate fails on DECREASE — that gate auto-rewrites the baseline and asks for the commit, unlike `check-lane-wiring` which needs an explicit `--update-baseline`. Either order works; one of them needs a re-record, and I am happy to push it. ## Verification (measured) - `node --test` — **3 passed / 0 failed** (1 pre-existing + 2 new) - `node --check`, `eslint` — clean - `lifecycle-column-census --strict`, `check-lane-wiring`, `check-fnxc-future-dates` — green No changeset: root `scripts/` is repo tooling, not part of the published package. ## Territory status This was the last item I know of in `scripts/`. The four operator scripts holding lane assumptions — `recover-stale-blocked-by` (#2992), `reconcile-task-state-consistency` (#2994), `reconcile-leaked-soft-deletes` (#2999) and this one — are now either resolved or, where a script genuinely cannot resolve lanes, made loud rather than silent. --- .../audit-branch-cross-contamination.test.mjs | 41 +++++++++++++++++ scripts/audit-branch-cross-contamination.mjs | 44 +++++++++++++++++-- scripts/lib/sql-column-literals-baseline.json | 3 +- 3 files changed, 82 insertions(+), 6 deletions(-) diff --git a/scripts/__tests__/audit-branch-cross-contamination.test.mjs b/scripts/__tests__/audit-branch-cross-contamination.test.mjs index 6aa85f5390..1c3f81dcbd 100644 --- a/scripts/__tests__/audit-branch-cross-contamination.test.mjs +++ b/scripts/__tests__/audit-branch-cross-contamination.test.mjs @@ -50,3 +50,44 @@ test("flags branch as tainted when foreign task commits are present", () => { fs.rmSync(dir, { recursive: true, force: true }); } }); + +/* +FNXC:OperatorScriptLaneAssumptions 2026-07-31-10:15: +THE INVARIANT: the audit reports the lanes it ACTUALLY scanned, never a fixed claim. + +`scannedColumns` was the literal `["triage","todo","in-progress","in-review"]`, printed regardless of +what the query returned. Paired with a query that allowlisted those same four ids, a renamed board +produced a report asserting coverage of four lanes it had not looked at — and zero contamination, +which reads as a clean bill of health rather than as a scan that never happened. + +That self-disclosure is the only thing standing between this audit and a silent wrong answer, so it +has to be an observation. Reverted, the first case reports the four legacy ids for a board that has +none of them. +*/ +test("scannedColumns reports the board's real lanes, not a fixed legacy claim", () => { + const report = analyzeBranchCrossContamination({ + projectRoot: "/nonexistent", + taskRows: [ + { id: "FN-1", title: "a", branch: null, baseCommitSha: null, columnName: "building" }, + { id: "FN-2", title: "b", branch: null, baseCommitSha: null, columnName: "backlog" }, + ], + }); + + assert.deepEqual(report.scannedColumns, ["backlog", "building"]); + assert.equal(report.scannedTaskCount, 2); +}); + +test("reports each scanned lane once, and nothing at all for an empty board", () => { + const rows = ["todo", "todo", "in-progress"].map((columnName, i) => ({ + id: `FN-${i + 10}`, title: "t", branch: null, baseCommitSha: null, columnName, + })); + + assert.deepEqual( + analyzeBranchCrossContamination({ projectRoot: "/nonexistent", taskRows: rows }).scannedColumns, + ["in-progress", "todo"], + ); + assert.deepEqual( + analyzeBranchCrossContamination({ projectRoot: "/nonexistent", taskRows: [] }).scannedColumns, + [], + ); +}); diff --git a/scripts/audit-branch-cross-contamination.mjs b/scripts/audit-branch-cross-contamination.mjs index 95141f223e..d1cfdc17c6 100644 --- a/scripts/audit-branch-cross-contamination.mjs +++ b/scripts/audit-branch-cross-contamination.mjs @@ -97,12 +97,24 @@ function parseCommits(raw) { } /** Pure analysis over injected task rows ({ id, title, branch, baseCommitSha, columnName }). */ +/* +FNXC:OperatorScriptLaneAssumptions 2026-07-31-10:15: +`scannedColumns` reports what was ACTUALLY scanned, instead of asserting a fixed list. + +This field was the literal array `["triage","todo","in-progress","in-review"]` — a claim, not an +observation. It was printed in the report regardless of what the query returned, so on a renamed +board it stated coverage the audit did not have. That is the one thing keeping a silent audit +honest, so it must be derived rather than declared. + +Derived from the rows themselves: whatever lanes came back are the lanes examined, whether or not +this script has heard of them. +*/ export function analyzeBranchCrossContamination({ projectRoot = process.cwd(), taskRows }) { const report = { generatedAt: new Date().toISOString(), projectRoot, scannedTaskCount: taskRows.length, - scannedColumns: ["triage", "todo", "in-progress", "in-review"], + scannedColumns: [...new Set(taskRows.map((row) => row.columnName).filter(Boolean))].sort(), taintedTaskCount: 0, missingBranchCount: 0, tasks: [], @@ -178,13 +190,37 @@ export async function auditBranchCrossContamination({ projectRoot = process.cwd( const backend = await openBackend(projectRoot); let taskRows; try { - const { asyncLayer, sql } = backend; - taskRows = rowsOf(await asyncLayer.db.execute(sql` + const { core, store, asyncLayer, sql } = backend; + /* + FNXC:OperatorScriptLaneAssumptions 2026-07-31-10:15: + Exclude the board's FINISHED lanes rather than allowlisting four legacy active ones. + + The query said `"column" IN ('triage','todo','in-progress','in-review')`. On a board whose lanes + are named anything else that matches NOTHING, so the audit scans zero rows and reports zero + contamination — a clean bill of health from a scan that never happened. `triage` is in that list + too, a lane U11 (#2515) deleted. + + Inverted so the default is the safe one: an unrecognised lane is active work by assumption and IS + audited; only lanes that genuinely mean finished drop out. An allowlist fails closed (skip + everything unknown), a denylist fails open (look at it), and for an audit looking at one extra + finished branch is a far smaller error than auditing nothing. + + Filtered in JS rather than by building a dynamic SQL exclusion: it keeps ONE place deciding what + "finished" means, and it removes the last raw-SQL lane literal from this file — the shape the + sibling gate could not see until #3000 widened it to `scripts/`. + */ + const terminalLanes = store && core.resolveProjectColumnsForRoles + ? await core.resolveProjectColumnsForRoles(store, core.TERMINAL_ROLES).catch(() => undefined) + : undefined; + /* DELIBERATE-LITERAL — the degraded default when the lanes could not be resolved. */ + const isFinished = (column) => (terminalLanes ? terminalLanes.has(column) : column === "done" || column === "archived"); + const allRows = rowsOf(await asyncLayer.db.execute(sql` SELECT id, title, branch, base_commit_sha AS "baseCommitSha", "column" AS "columnName" FROM project."tasks" - WHERE deleted_at IS NULL AND "column" IN ('triage','todo','in-progress','in-review') + WHERE deleted_at IS NULL ORDER BY id `)); + taskRows = allRows.filter((row) => !isFinished(row.columnName)); } finally { await backend.shutdown().catch(() => {}); } diff --git a/scripts/lib/sql-column-literals-baseline.json b/scripts/lib/sql-column-literals-baseline.json index c393aa8b0f..5afd8218b7 100644 --- a/scripts/lib/sql-column-literals-baseline.json +++ b/scripts/lib/sql-column-literals-baseline.json @@ -12,6 +12,5 @@ "packages/core/src/task-store/task-artifacts-ops.ts": 1, "packages/core/src/task-store/workflow-definitions.ts": 2, "packages/core/src/team-analytics.ts": 3, - "packages/core/src/workflow-analytics.ts": 3, - "scripts/audit-branch-cross-contamination.mjs": 4 + "packages/core/src/workflow-analytics.ts": 3 }