From 4e2f52ce8fbd7a54886fd8806d5e188b40832754 Mon Sep 17 00:00:00 2001 From: gsxdsm Date: Fri, 31 Jul 2026 14:56:07 -0700 Subject: [PATCH] =?UTF-8?q?feat(gate):=20ratchet=20move-target=20literals?= =?UTF-8?q?=20at=20zero=20=E2=80=94=20#3150's=20population=20had=20nothing?= =?UTF-8?q?=20holding=20it=20(#3246)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. ## 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. --------- Co-authored-by: Claude Opus 5 (1M context) --- .github/workflows/pr-checks.yml | 2 + package.json | 1 + scripts/check-move-target-literals.mjs | 162 ++++++++++++++++++ .../lib/move-target-literals-baseline.json | 1 + 4 files changed, 166 insertions(+) create mode 100644 scripts/check-move-target-literals.mjs create mode 100644 scripts/lib/move-target-literals-baseline.json diff --git a/.github/workflows/pr-checks.yml b/.github/workflows/pr-checks.yml index 447ec2627a..9e10fa64b2 100644 --- a/.github/workflows/pr-checks.yml +++ b/.github/workflows/pr-checks.yml @@ -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 diff --git a/package.json b/package.json index a844197437..9156ff558f 100644 --- a/package.json +++ b/package.json @@ -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", diff --git a/scripts/check-move-target-literals.mjs b/scripts/check-move-target-literals.mjs new file mode 100644 index 0000000000..6f72500db7 --- /dev/null +++ b/scripts/check-move-target-literals.mjs @@ -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); diff --git a/scripts/lib/move-target-literals-baseline.json b/scripts/lib/move-target-literals-baseline.json new file mode 100644 index 0000000000..df3bc9cdde --- /dev/null +++ b/scripts/lib/move-target-literals-baseline.json @@ -0,0 +1 @@ +{"byFile":{}}