fix(scripts): the contamination audit scanned four legacy lanes and claimed it had (#3005)
## 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.
This commit is contained in:
@@ -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,
|
||||
[],
|
||||
);
|
||||
});
|
||||
|
||||
@@ -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(() => {});
|
||||
}
|
||||
|
||||
@@ -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
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user