The ratchet's baseline could not be re-recorded once a file rose — the one state that blocks a correct conversion (#2668)
Unowned (no open PR touches the census CLI — only its baseline JSON) and **live**, since #2654 gates CI on `--strict`. ## The problem `--update-baseline` sat **behind** the rise exit, so the only supported way to re-record was unavailable in exactly the situation that needs it. That matters because **a conversion legitimately adds a literal.** The correct shape for a caller that may have no traits is `flags ? flags.x : columnId === "legacy"`, and each one raises a file's count by one. Measured on current main: `columnRoles.ts` went **0 → 1** from precisely that shape (added by #2647, documented at the site, correct code). So a worker doing the right thing meets a red gate whose only escape is hand-editing the JSON. That is how a ratchet becomes something people route around rather than run — and then it guards nothing. This is the same failure mode as a guard that cannot fire, arrived at from the other side. ## The change `--update-baseline` is an explicit operator action, so it re-records **unconditionally** and prints what it accepted under `ACCEPTED RISES`. Swallowing a rise silently is the real danger; refusing to let anyone re-record is the same danger one step later, wearing a red check nobody trusts. **The rise check is unchanged** and still exits 1 without the flag. **One writer now.** The old second `writeFileSync` behind the rise exit is deleted rather than left unreachable — two writers for one artifact is how they drift. The `!deliberateTracked && updateBaseline` special case went with it, since the unconditional block covers the legacy-shape migration too. ## Exercised end to end On a real rise injected into `live-agent-count.ts`: ``` rise + plain --strict exit 1 (the ratchet still bites) rise + --strict --update-baseline exit 0 "ACCEPTED RISES live-agent-count.ts: 6 -> 7" ``` Four cases assert the CLI's own source, because exit codes are the contract and the pure summarizer cannot express them: the write precedes the rise check, the branches exit 0 and 1 respectively, accepted rises are **named**, and there is exactly **one** writer. ## A note on the revert proof, because it caught me twice My first attempt to move the block back was a **no-op**: the marker I sliced on (`if (regressions.length > 0) {`) also appears *inside* the update block, so the "revert" reassembled the file unchanged and the suite stayed green. **A revert proof that does not go red can mean the guard is vacuous *or* that the revert did not land** — and the second is easy to miss when you are expecting the first. The real revert fails **2 of 27**, and the assertions now verify marker *uniqueness* before slicing on it. ## Verification - 27/27 census suites; `--strict` exits 0; `pnpm test:gate` **71/71**; `pnpm lint` clean - census on this tree: 748 column guards, **4 triage** (all in `moves.ts`'s flag-OFF block, deletion-scheduled with #2655) 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -342,3 +342,136 @@ describe("the summary separates the three classes", () => {
|
||||
expect(summary.byColumnId).toEqual({ todo: 1 });
|
||||
});
|
||||
});
|
||||
|
||||
/*
|
||||
FNXC:LifecycleColumnCensus 2026-07-31-18:40 (the ratchet's one unusable state, now fixed):
|
||||
|
||||
`--update-baseline` MUST RUN EVEN WHEN A FILE ROSE. It could not: the rise check exited before the write,
|
||||
so the only supported way to re-record was unavailable in exactly the situation that needs it.
|
||||
|
||||
That is a live problem, not a tidiness one, because #2654 gates CI on this AND A CONVERSION LEGITIMATELY
|
||||
ADDS A LITERAL — the correct shape for a caller that may have no traits is
|
||||
`flags ? flags.x : columnId === "legacy"`, and each one raises a file's count by one. Measured on main:
|
||||
`columnRoles.ts` went 0 -> 1 from exactly that shape. So a worker doing the right thing met a red gate
|
||||
whose only escape was hand-editing the JSON, which is how a ratchet becomes something people route around.
|
||||
|
||||
Exercised end to end before writing this, on a real rise injected into `live-agent-count.ts`:
|
||||
rise + plain --strict exit 1 (unchanged — the ratchet still bites)
|
||||
rise + --strict --update-baseline exit 0, printing "ACCEPTED RISES live-agent-count.ts: 6 -> 7"
|
||||
|
||||
EXIT CODES ARE THE CONTRACT and the pure summarizer cannot express them, so these assert the CLI's own
|
||||
source: which branch writes, which exits, and — the part that was actually broken — the ORDER. Marker-to-
|
||||
marker slices rather than character windows, and each marker checked for uniqueness first, because a
|
||||
repeated marker is the magic-number problem wearing a name.
|
||||
*/
|
||||
describe("the baseline can always be re-recorded", () => {
|
||||
const cliPath = new URL("../../../../scripts/lifecycle-column-census.mjs", import.meta.url).pathname;
|
||||
|
||||
function cliSource(): string {
|
||||
// eslint-disable-next-line @typescript-eslint/no-require-imports
|
||||
return require("node:fs").readFileSync(cliPath, "utf8") as string;
|
||||
}
|
||||
|
||||
function sliceBetween(cli: string, from: string, to: string): string {
|
||||
const start = cli.indexOf(from);
|
||||
const end = cli.indexOf(to, start + from.length);
|
||||
expect(start).toBeGreaterThan(-1);
|
||||
expect(end).toBeGreaterThan(start);
|
||||
// A repeated marker would make the slice meaningless, so prove uniqueness before trusting it.
|
||||
expect(cli.indexOf(from, start + from.length)).toBe(-1);
|
||||
return cli.slice(start, end);
|
||||
}
|
||||
|
||||
it("writes the baseline BEFORE the rise check can exit", () => {
|
||||
const cli = cliSource();
|
||||
const updateAt = cli.indexOf("if (updateBaseline) {");
|
||||
const riseAt = cli.indexOf("column-guard count ROSE");
|
||||
|
||||
expect(updateAt).toBeGreaterThan(-1);
|
||||
expect(riseAt).toBeGreaterThan(updateAt);
|
||||
});
|
||||
|
||||
it("exits 0 from the update branch and 1 from the rise branch", () => {
|
||||
const cli = cliSource();
|
||||
|
||||
expect(sliceBetween(cli, "if (updateBaseline) {", "column-guard count ROSE")).toContain("process.exit(0)");
|
||||
expect(sliceBetween(cli, "column-guard count ROSE", "baseline is STALE")).toContain("process.exit(1)");
|
||||
});
|
||||
|
||||
/*
|
||||
FNXC:LifecycleColumnCensus 2026-07-31-18:30 (PR #2668 review — greptile):
|
||||
END-TO-END, because every assertion above reads this file's SOURCE TEXT. Substrings,
|
||||
marker ordering and `writeFileSync` counts cannot see control flow: move the exit,
|
||||
reorder the branches, or return before the write, and all of them stay green while
|
||||
the contract is broken.
|
||||
|
||||
The contract is three observable things — the EXIT CODE, what lands in the baseline
|
||||
file, and what is printed. These drive the real CLI against a throwaway baseline via
|
||||
`FUSION_CENSUS_BASELINE_PATH` and assert exactly those, so a control-flow change
|
||||
fails here even when the source still contains every string the tests above look for.
|
||||
*/
|
||||
describe("driven end to end", () => {
|
||||
// eslint-disable-next-line @typescript-eslint/no-require-imports
|
||||
const { execFileSync } = require("node:child_process");
|
||||
// eslint-disable-next-line @typescript-eslint/no-require-imports
|
||||
const fs = require("node:fs");
|
||||
// eslint-disable-next-line @typescript-eslint/no-require-imports
|
||||
const os = require("node:os");
|
||||
// eslint-disable-next-line @typescript-eslint/no-require-imports
|
||||
const path = require("node:path");
|
||||
|
||||
const repoRoot = new URL("../../../..", import.meta.url).pathname;
|
||||
|
||||
function runCli(args: string[], baseline: unknown): { status: number; stdout: string } {
|
||||
const dir = fs.mkdtempSync(path.join(os.tmpdir(), "fusion-census-"));
|
||||
const baselinePath = path.join(dir, "baseline.json");
|
||||
fs.writeFileSync(baselinePath, JSON.stringify(baseline));
|
||||
try {
|
||||
const stdout = execFileSync("node", [cliPath, ...args], {
|
||||
encoding: "utf8",
|
||||
/* The CLI globs with `git ls-files` relative to CWD, and vitest runs from the
|
||||
package dir where that finds nothing — the run then exits on "file list is
|
||||
EMPTY". Without this the rise test would pass for the wrong reason. */
|
||||
cwd: repoRoot,
|
||||
env: { ...process.env, FUSION_CENSUS_BASELINE_PATH: baselinePath },
|
||||
}) as string;
|
||||
return { status: 0, stdout, ...{ baselinePath } } as never;
|
||||
} catch (err) {
|
||||
const e = err as { status?: number; stdout?: string };
|
||||
return { status: e.status ?? -1, stdout: e.stdout ?? "", ...{ baselinePath } } as never;
|
||||
} finally {
|
||||
// Read-back happens in the caller via the returned path; cleanup is per-test.
|
||||
}
|
||||
}
|
||||
|
||||
it("exits 0 and REWRITES the baseline under --update-baseline, even when the count rose", () => {
|
||||
/* The case the ordering bug broke: a rise used to exit before the write, so the
|
||||
one command whose whole job is re-recording could not re-record. */
|
||||
const stale = { totals: { column: 1, role: 0, status: 0, deliberate: 0 }, byFile: { "packages/engine/src/self-healing.ts": 1 }, byColumnId: {}, queryByFile: {} };
|
||||
const r = runCli(["--strict", "--update-baseline"], stale) as unknown as { status: number; stdout: string; baselinePath: string };
|
||||
expect(r.status).toBe(0);
|
||||
const written = JSON.parse(fs.readFileSync(r.baselinePath, "utf8"));
|
||||
expect(written.totals.column).toBeGreaterThan(1);
|
||||
expect(r.stdout).toContain("ACCEPTED RISES");
|
||||
});
|
||||
|
||||
it("exits 1 and LEAVES the baseline alone on a rise without --update-baseline", () => {
|
||||
const stale = { totals: { column: 1, role: 0, status: 0, deliberate: 0 }, byFile: { "packages/engine/src/self-healing.ts": 1 }, byColumnId: {}, queryByFile: {} };
|
||||
const r = runCli(["--strict"], stale) as unknown as { status: number; stdout: string; baselinePath: string };
|
||||
expect(r.status).toBe(1);
|
||||
const after = JSON.parse(fs.readFileSync(r.baselinePath, "utf8"));
|
||||
expect(after.totals.column).toBe(1);
|
||||
});
|
||||
});
|
||||
|
||||
it("names what it accepted instead of swallowing it", () => {
|
||||
// A silent re-record would hide a genuine regression behind a routine command.
|
||||
expect(cliSource()).toContain("ACCEPTED RISES");
|
||||
});
|
||||
|
||||
it("has exactly ONE writer for the baseline artifact", () => {
|
||||
// The old code had a second `writeFileSync` behind the rise exit — unreachable in the case that
|
||||
// needed it, and a second writer for one artifact is how the two drift.
|
||||
expect(cliSource().split("writeFileSync(").length - 1).toBe(1);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -45,7 +45,21 @@ import {
|
||||
} from "./lib/lifecycle-column-census.mjs";
|
||||
|
||||
const HERE = dirname(fileURLToPath(import.meta.url));
|
||||
const BASELINE_PATH = join(HERE, "lib", "lifecycle-column-census-baseline.json");
|
||||
/*
|
||||
FNXC:LifecycleColumnCensus 2026-07-31-18:20 (PR #2668 review — greptile):
|
||||
BASELINE PATH IS OVERRIDABLE so the CLI can be driven END TO END in a test.
|
||||
|
||||
The suite could only assert this file's SOURCE TEXT — substrings, marker ordering,
|
||||
`writeFileSync` call counts — because a test that actually ran the CLI would rewrite
|
||||
the repo's real baseline. Source assertions cannot see control flow: move the exit,
|
||||
reorder the branches, or return before the write, and every one of them still passes.
|
||||
|
||||
An env override is the smallest seam that makes the real contract testable: exit
|
||||
code, what lands in the baseline file, and what is printed. Production never sets it,
|
||||
so the default is unchanged.
|
||||
*/
|
||||
const BASELINE_PATH = process.env.FUSION_CENSUS_BASELINE_PATH
|
||||
?? join(HERE, "lib", "lifecycle-column-census-baseline.json");
|
||||
|
||||
let files;
|
||||
try {
|
||||
@@ -260,6 +274,48 @@ for (const [file, allowed] of baselineQueryByFile) {
|
||||
}
|
||||
|
||||
|
||||
/*
|
||||
FNXC:LifecycleColumnCensus 2026-07-31-18:20:
|
||||
`--update-baseline` MUST RUN EVEN WHEN A FILE ROSE, and it could not: the rise check exited first, so the
|
||||
only supported way to re-record was unavailable in exactly the situation that needs it.
|
||||
|
||||
That is not hypothetical now that #2654 gates CI on this. A CONVERSION LEGITIMATELY ADDS A LITERAL: the
|
||||
correct shape for a caller that may have no traits is `flags ? flags.x : columnId === "legacy"`, and every
|
||||
one of those raises a file's count by one. So a worker doing the right thing hits a red gate whose only
|
||||
escape is hand-editing the JSON — which is how a ratchet becomes something people route around instead of
|
||||
run. Measured on current main: `columnRoles.ts` 0 -> 1 from exactly that shape.
|
||||
|
||||
The flag is an explicit operator action, so it re-records unconditionally and PRINTS what it accepted
|
||||
under `ACCEPTED RISES`. Silently swallowing a rise is the real danger; refusing to let anyone re-record is
|
||||
the same danger one step later, wearing a red check nobody trusts.
|
||||
*/
|
||||
if (updateBaseline) {
|
||||
writeFileSync(
|
||||
BASELINE_PATH,
|
||||
`${JSON.stringify({
|
||||
generatedFrom: "node scripts/lifecycle-column-census.mjs --strict --update-baseline",
|
||||
totals: summary.totals,
|
||||
byColumnId: summary.byColumnId,
|
||||
byFile: Object.fromEntries(summary.byFile),
|
||||
deliberateByFile: Object.fromEntries(summary.deliberateByFile ?? []),
|
||||
properties: summary.properties,
|
||||
queryByColumnId: summary.queryByColumnId,
|
||||
queryByFile: Object.fromEntries(summary.queryByFile),
|
||||
}, null, 2)}\n`,
|
||||
);
|
||||
if (regressions.length > 0) {
|
||||
console.log("\n ACCEPTED RISES (a merge or a conversion added guards here — convert them or they stay in the bar):");
|
||||
for (const r of regressions) {
|
||||
console.log(` ${r.file}${r.kind === "query" ? " (query filter)" : ""}: ${r.allowed} -> ${r.count}`);
|
||||
}
|
||||
}
|
||||
if (stale.length > 0) {
|
||||
console.log(`\n TIGHTENED ${stale.length} entr${stale.length === 1 ? "y" : "ies"} whose counts dropped.`);
|
||||
}
|
||||
console.log("\nlifecycle-column-census: baseline re-recorded.");
|
||||
process.exit(0);
|
||||
}
|
||||
|
||||
if (regressions.length > 0) {
|
||||
console.error("\nlifecycle-column-census --strict: column-guard count ROSE\n");
|
||||
for (const r of regressions) {
|
||||
@@ -273,24 +329,14 @@ if (regressions.length > 0) {
|
||||
process.exit(1);
|
||||
}
|
||||
|
||||
if (stale.length > 0 || (!deliberateTracked && updateBaseline)) {
|
||||
if (updateBaseline) {
|
||||
writeFileSync(
|
||||
BASELINE_PATH,
|
||||
`${JSON.stringify({
|
||||
generatedFrom: "node scripts/lifecycle-column-census.mjs --strict --update-baseline",
|
||||
totals: summary.totals,
|
||||
byColumnId: summary.byColumnId,
|
||||
byFile: Object.fromEntries(summary.byFile),
|
||||
deliberateByFile: Object.fromEntries(summary.deliberateByFile ?? []),
|
||||
properties: summary.properties,
|
||||
queryByColumnId: summary.queryByColumnId,
|
||||
queryByFile: Object.fromEntries(summary.queryByFile),
|
||||
}, null, 2)}\n`,
|
||||
);
|
||||
console.log(`\nlifecycle-column-census --strict: baseline TIGHTENED for ${stale.length} file(s).`);
|
||||
process.exit(0);
|
||||
}
|
||||
/*
|
||||
FNXC:LifecycleColumnCensus 2026-07-31-18:25: the `--update-baseline` branch that lived here is GONE — it
|
||||
now runs above, before the rise exit, so a risen file can be re-recorded. Keeping a second copy here would
|
||||
be two writers for one artifact, and the one behind the rise exit was unreachable in the case that needed
|
||||
it. The `!deliberateTracked && updateBaseline` condition went with it: the unconditional block covers the
|
||||
legacy-shape migration too.
|
||||
*/
|
||||
if (stale.length > 0) {
|
||||
console.error("\nlifecycle-column-census --strict: baseline is STALE — it allows more than the tree has\n");
|
||||
for (const s of stale) {
|
||||
console.error(` ${s.file}: allows ${s.allowed}, tree has ${s.count}`);
|
||||
|
||||
Reference in New Issue
Block a user