feat(gate): ratchet move-target literals at zero — #3150's population had nothing holding it (#3246)
Closes the gap I flagged when re-measuring #3150: that population is at **0**, and nothing was holding it there. ## Why this surface has no gate today The lifecycle census parses **comparisons**. A move destination is a call **argument**: ```ts await store.moveTask(id, "in-review"); // never counted by anything ``` #3150 measured 31 of these across four files. They are now 0 — I verified that on current main before writing this — but the comparison backlog drifted **787 → 854** during the window its own ratchet was unwired, and this population never had one. ## The failure mode is louder than the guards' A wrong lane **guard** silently answers "no". A wrong move **target** is rejected by `moveTaskInternal` with `TransitionRejectionError: unknown-column` — so on a board that renamed its review lane, every task finishing implementation **threw** instead of reaching review. Loud at runtime, invisible to any test on the default board. ## AST, not grep — and that is measured, not stylistic | scan | result | |---|---| | comment-naive grep of `self-healing.ts` | 1 hit — **JSDoc prose**: `* could call moveTask("in-review")` | | #3150's own SQL survey by grep | 37 hits against **12** real sites (25 comments) | Comments are not AST nodes, so that false-positive class cannot occur here in either direction. ## Controls — all four run, because a gate that only reports 0 proves nothing | probe | expected | got | |---|---|---| | real `moveTask(id, "in-review")` injected | fail | **exit 1**, names the file | | identical call as JSDoc prose | pass | **exit 0** (AST ignores comments) | | legacy target + leading `DELIBERATE-LITERAL` | pass | **exit 0** (marker honored) | | probe removed | pass | **exit 0** | The third is the #1411 `recoveryRehome` safe-landing path, where the legacy id genuinely *is* the target. Marker must be **leading** — the census already learned that an inline marker attaches to the wrong node and is silently ignored. ## Ratchet semantics match the census Fails on a **drop** as well as a rise. A stale allowance is a hole a re-added target can return through while the gate stays green — exactly what let the comparison baseline drift. ## Measured | check | result | |---|---| | this gate | scans **1816** files, reports **0** | | `check:lifecycle-columns` / `check:sql-column-literals` | 0 / 0 | | `check:fnxc-future-dates` / `check:lane-wiring` | 0 / 0 | | eslint / `pnpm test:gate` | clean / exit 0 | Wired into `pr-checks.yml` beside the sibling ratchets, named to match ("Move-target ratchet"). ## Scope Gate only — **no production code touched**, and no conversions in this PR. The population was already empty; this makes "31 → 0" an invariant instead of a snapshot, which is the caveat I attached when recommending #3150 for closure. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Quality Improvements** * Added automated validation for task-movement configuration values. * Pull request checks now detect unexpected changes in tracked values. * Added baseline tracking with strict validation to identify both additions and removals. * Added support for explicitly documenting intentional exceptions. * Improved reporting for file-discovery and source-reading failures. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
2
.github/workflows/pr-checks.yml
vendored
2
.github/workflows/pr-checks.yml
vendored
@@ -58,6 +58,8 @@ jobs:
|
||||
run: pnpm check:lifecycle-columns
|
||||
- name: SQL-column ratchet
|
||||
run: pnpm check:sql-column-literals
|
||||
- name: Move-target ratchet
|
||||
run: pnpm check:move-target-literals
|
||||
- name: Inert lane/flag seams
|
||||
run: pnpm check:inert-flag-seams
|
||||
- name: FNXC stamp dates
|
||||
|
||||
@@ -21,6 +21,7 @@
|
||||
"check:changesets": "node scripts/check-changeset-format.mjs",
|
||||
"check:lifecycle-columns": "node scripts/lifecycle-column-census.mjs --strict",
|
||||
"check:sql-column-literals": "node scripts/check-sql-column-literals.mjs",
|
||||
"check:move-target-literals": "node scripts/check-move-target-literals.mjs --strict",
|
||||
"check:inert-flag-seams": "node scripts/check-inert-flag-seams.mjs",
|
||||
"check:inert-sync-lanes": "node scripts/check-inert-sync-lane-conversions.mjs",
|
||||
"check:fnxc-future-dates": "node scripts/check-fnxc-future-dates.mjs",
|
||||
|
||||
162
scripts/check-move-target-literals.mjs
Normal file
162
scripts/check-move-target-literals.mjs
Normal file
@@ -0,0 +1,162 @@
|
||||
#!/usr/bin/env node
|
||||
/*
|
||||
FNXC:MoveTargetLiterals 2026-07-31-17:20 (u12 — #3150's population had no ratchet):
|
||||
|
||||
WHAT THIS GUARDS. A `moveTask(id, "in-review")` DESTINATION is a call argument, not a comparison, so
|
||||
the lifecycle-column census — which parses comparisons — has never counted one. #3150 measured 31 such
|
||||
targets across four files; they are now 0. Nothing held that at 0, so the population could regrow
|
||||
silently, which is exactly how the comparison backlog drifted 787 -> 854 while its own gate was
|
||||
unwired.
|
||||
|
||||
WHY IT MATTERS MORE THAN THE COMPARISON BACKLOG. A wrong lane GUARD silently answers "no". A wrong
|
||||
move TARGET is rejected by `moveTaskInternal` with `TransitionRejectionError: unknown-column`, so on a
|
||||
board that renamed its review lane, every task finishing implementation THREW instead of reaching
|
||||
review. Loud, but only at runtime and only on a renamed board — the shape no unit test on the default
|
||||
board can see.
|
||||
|
||||
AST, NOT GREP, and the reason is measured: a comment-naive scan of `self-healing.ts` reports one
|
||||
remaining hit that is JSDoc prose (`* could call moveTask("in-review")`), and #3150's own SQL count
|
||||
was 37 by grep against 12 real sites — 25 comments. Comments are not AST nodes, so this cannot
|
||||
produce that class of false positive in either direction.
|
||||
|
||||
ESCAPE HATCH. A leading `DELIBERATE-LITERAL` comment on the enclosing statement exempts a site, for
|
||||
the #1411 legacy safe-landing path (`recoveryRehome: true`), where the legacy id is the point rather
|
||||
than an unconverted lane. Marker must be in LEADING comments — the census learned that an inline
|
||||
marker attaches to the wrong node and is silently ignored.
|
||||
|
||||
Report-only by default; `--strict` fails on any change from the baseline, in EITHER direction, so a
|
||||
drop is re-recorded in the same PR that earns it rather than leaving a stale allowance open.
|
||||
*/
|
||||
import { readFileSync, writeFileSync, existsSync } from "node:fs";
|
||||
import { execSync } from "node:child_process";
|
||||
import { join, resolve, dirname } from "node:path";
|
||||
import { fileURLToPath } from "node:url";
|
||||
import ts from "typescript";
|
||||
|
||||
const REPO = resolve(dirname(fileURLToPath(import.meta.url)), "..");
|
||||
const BASELINE_PATH = process.env.FUSION_MOVE_TARGET_BASELINE_PATH
|
||||
?? join(REPO, "scripts", "lib", "move-target-literals-baseline.json");
|
||||
|
||||
const LEGACY_COLUMN_IDS = new Set(["triage", "todo", "in-progress", "in-review", "done", "archived"]);
|
||||
const MOVE_FNS = new Set(["moveTask", "moveTaskInternal"]);
|
||||
const DELIBERATE_MARKER = "DELIBERATE-LITERAL";
|
||||
|
||||
const strict = process.argv.includes("--strict");
|
||||
const updateBaseline = process.argv.includes("--update-baseline");
|
||||
const json = process.argv.includes("--json");
|
||||
|
||||
let files;
|
||||
try {
|
||||
files = execSync(
|
||||
"git ls-files 'packages/*/src/**/*.ts' 'packages/*/src/*.ts' 'packages/*/src/**/*.tsx' 'packages/*/app/**/*.ts' 'packages/*/app/**/*.tsx'",
|
||||
{ encoding: "utf8", maxBuffer: 64 * 1024 * 1024 },
|
||||
).split("\n").map((f) => f.trim()).filter(Boolean)
|
||||
.filter((f) => !f.includes("__tests__") && !/\.(test|spec)\.tsx?$/.test(f));
|
||||
} catch (err) {
|
||||
/* FAIL CLOSED: an unreadable file list means nothing was checked, which must not read as clean. */
|
||||
console.error(`check-move-target-literals: could not list files — ${err?.message ?? err}`);
|
||||
process.exit(1);
|
||||
}
|
||||
if (files.length === 0) {
|
||||
console.error("check-move-target-literals: file list is EMPTY — refusing to report on zero files.");
|
||||
process.exit(1);
|
||||
}
|
||||
|
||||
/** True when the statement enclosing `node` carries a DELIBERATE-LITERAL in its LEADING comments. */
|
||||
function hasDeliberateMarker(node, source) {
|
||||
for (let cur = node; cur; cur = cur.parent) {
|
||||
const ranges = ts.getLeadingCommentRanges(source, cur.getFullStart()) ?? [];
|
||||
for (const r of ranges) {
|
||||
if (source.slice(r.pos, r.end).includes(DELIBERATE_MARKER)) return true;
|
||||
}
|
||||
if (ts.isStatement(cur)) break;
|
||||
}
|
||||
return false;
|
||||
}
|
||||
|
||||
const byFile = {};
|
||||
for (const file of files) {
|
||||
let source;
|
||||
try {
|
||||
source = readFileSync(join(REPO, file), "utf8");
|
||||
} catch (error) {
|
||||
/* FNXC:MoveTargetRatchet 2026-07-31-23:55 (#3246 review): an unreadable tracked file made the
|
||||
count silently short, and this count is a claim about the whole tree. Fail closed. */
|
||||
console.error(`check-move-target-literals: cannot read tracked file ${file}: ${error?.message ?? error}`);
|
||||
console.error("check-move-target-literals: refusing to report a count that may be short.");
|
||||
process.exit(2);
|
||||
}
|
||||
if (!source.includes("moveTask")) continue;
|
||||
const sf = ts.createSourceFile(file, source, ts.ScriptTarget.Latest, true, file.endsWith(".tsx") ? ts.ScriptKind.TSX : ts.ScriptKind.TS);
|
||||
let count = 0;
|
||||
const walk = (node) => {
|
||||
if (ts.isCallExpression(node)) {
|
||||
const callee = ts.isPropertyAccessExpression(node.expression)
|
||||
? node.expression.name.text
|
||||
: ts.isIdentifier(node.expression) ? node.expression.text : "";
|
||||
if (MOVE_FNS.has(callee)) {
|
||||
/* FNXC:MoveTargetRatchet 2026-07-31-23:55 (#3246 review): DESTINATION ARGUMENT ONLY.
|
||||
moveTask(id, toColumn, options?) and moveTaskInternal(id, toColumn, options, internal,
|
||||
currentTask?) both put the destination at index 1. Scanning every argument counted
|
||||
`{ reason: "done" }` in an options bag as a move target — work that does not exist and
|
||||
that no real fix could clear. Backtick form included. */
|
||||
const destination = node.arguments[1];
|
||||
const isLiteral = destination
|
||||
&& (ts.isStringLiteral(destination) || ts.isNoSubstitutionTemplateLiteral(destination));
|
||||
if (isLiteral && LEGACY_COLUMN_IDS.has(destination.text) && !hasDeliberateMarker(node, source)) {
|
||||
count += 1;
|
||||
}
|
||||
}
|
||||
}
|
||||
ts.forEachChild(node, walk);
|
||||
};
|
||||
walk(sf);
|
||||
if (count > 0) byFile[file] = count;
|
||||
}
|
||||
|
||||
const total = Object.values(byFile).reduce((a, b) => a + b, 0);
|
||||
if (json) {
|
||||
console.log(JSON.stringify({ total, byFile }, null, 2));
|
||||
process.exit(0);
|
||||
}
|
||||
|
||||
console.log(`check-move-target-literals: scanned ${files.length} source files`);
|
||||
console.log(` moveTask targets that are legacy column literals: ${total}`);
|
||||
for (const [file, n] of Object.entries(byFile).sort((a, b) => b[1] - a[1])) {
|
||||
console.log(` ${String(n).padStart(4)} ${file}`);
|
||||
}
|
||||
if (total === 0) {
|
||||
console.log(" POPULATION EMPTY — #3150 measured 31 here; a wrong target THROWS rather than no-ops,");
|
||||
console.log(" so keep it empty. Use resolveTaskLifecycleColumns / the role helpers for destinations.");
|
||||
}
|
||||
|
||||
const baseline = existsSync(BASELINE_PATH) ? JSON.parse(readFileSync(BASELINE_PATH, "utf8")) : { byFile: {} };
|
||||
if (updateBaseline) {
|
||||
writeFileSync(BASELINE_PATH, `${JSON.stringify({ byFile }, null, 2)}\n`);
|
||||
console.log("check-move-target-literals: baseline re-recorded.");
|
||||
process.exit(0);
|
||||
}
|
||||
if (!strict) process.exit(0);
|
||||
|
||||
const allowed = baseline.byFile ?? {};
|
||||
const problems = [];
|
||||
for (const [file, n] of Object.entries(byFile)) {
|
||||
const cap = allowed[file] ?? 0;
|
||||
if (n > cap) problems.push(` ${file}: ${n} legacy move target(s), baseline allows ${cap}`);
|
||||
}
|
||||
for (const [file, cap] of Object.entries(allowed)) {
|
||||
const n = byFile[file] ?? 0;
|
||||
if (n < cap) problems.push(` ${file}: ${n} legacy move target(s), baseline allows ${cap} — DROP, re-record it`);
|
||||
}
|
||||
if (problems.length > 0) {
|
||||
console.error("\ncheck-move-target-literals --strict: move-target population DIVERGES from baseline:\n");
|
||||
console.error(problems.join("\n"));
|
||||
console.error("\nA legacy move TARGET is rejected on a renamed board (TransitionRejectionError: unknown-column),");
|
||||
console.error("so this is a runtime throw rather than a silent no-op. Resolve the destination through the role");
|
||||
console.error("helpers, or add a leading DELIBERATE-LITERAL comment if the legacy id is genuinely the point");
|
||||
console.error("(the #1411 recoveryRehome safe-landing path). If a count went DOWN, re-record with");
|
||||
console.error("--strict --update-baseline in the same commit.\n");
|
||||
process.exit(1);
|
||||
}
|
||||
console.log("check-move-target-literals --strict: every file matches its baseline exactly.");
|
||||
process.exit(0);
|
||||
1
scripts/lib/move-target-literals-baseline.json
Normal file
1
scripts/lib/move-target-literals-baseline.json
Normal file
@@ -0,0 +1 @@
|
||||
{"byFile":{}}
|
||||
Reference in New Issue
Block a user