From 9abec9f2351a7b4fe87f623ccb588668e86da299 Mon Sep 17 00:00:00 2001 From: gsxdsm Date: Fri, 31 Jul 2026 18:07:32 -0700 Subject: [PATCH] =?UTF-8?q?fix(gate):=20the=20fnxc=20check=20WROTE=20to=20?= =?UTF-8?q?the=20tree=20it=20was=20checking=20=E2=80=94=20nine=20PRs=20cha?= =?UTF-8?q?sed=20three=20defects=20(#3287)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Root-cause fix for the duplicate-PR pileup tracked in #3267. **Running the check modified the working tree.** ## Reproduction ``` clean: 0 files dirty $ node scripts/check-fnxc-future-dates.mjs # no flags, no --update-baseline exit 0 after: 1 file dirty → M scripts/lib/fnxc-future-dates-baseline.json ``` `:227` auto-tightened and `writeFileSync`'d on every run. ## Why that produced nine PRs The tightening is **right in substance** — the comment above it explains why banking a stale allowance is worse than re-recording. Doing it as a *side effect of checking* is what hurt: every worker who ran the gate received an identical uncommitted diff they had not written, and reasonably committed it. The clearest evidence is #3283 and #3285 — five minutes apart, `+0/-1` each, both deleting the same baseline line. **Neither author wrote that line.** The gate wrote it, in both of their checkouts. I also mis-attributed my own dirty tree to leftover work while retracting a measurement on #3277/#3278. The dirt was this script. ## The change Still computed, still reported loudly — only **written** under `--update-baseline`: ``` [check-fnxc-future-dates] baseline CAN BE TIGHTENED for 1 file(s): packages/cli/src/__tests__/cli-active-count-lanes.test.ts: 10 -> 5 run `pnpm check:fnxc-future-dates --update-baseline` to record it (one commit, one author) ``` **A plain run stays green rather than failing on a tightening.** Stamps age into the past on their own, so failing would redden main on a clock tick — which is precisely why the auto-write existed. Report, don't enforce. ## Measured, both directions | scenario | result | |---|---| | stale allowance, plain run | reports + hint; baseline **unchanged** (verified still inflated at 10) | | stale allowance, `--update-baseline` | `baseline written: 122 stamp(s) in 63 file(s)`; value reset to 5 | | clean tree, plain run | exit 0, **zero files dirty** | | `census --strict` / eslint | 0 / clean | The first row is the one that matters: I inflated an allowance, ran the check, and confirmed the file was **still inflated afterwards**. Asserting only "exit 0, no diff" would have passed even if the write had silently succeeded and produced no net change. ## Scope One script. CI is unaffected — it never committed the side-effect write, so that write was always discarded there. The only behaviour change is that an interactive run no longer edits your tree. This is a smaller intervention than the claim-protocol I proposed earlier in #3267, and I now think that one was treating a symptom: workers were not colliding because they lacked a protocol, but because the tool handed each of them the same diff. --- scripts/check-fnxc-future-dates.mjs | 36 +++++++++++++++++++++++------ 1 file changed, 29 insertions(+), 7 deletions(-) diff --git a/scripts/check-fnxc-future-dates.mjs b/scripts/check-fnxc-future-dates.mjs index b8650b732b..ad612a6ccc 100644 --- a/scripts/check-fnxc-future-dates.mjs +++ b/scripts/check-fnxc-future-dates.mjs @@ -149,7 +149,8 @@ function scan() { const found = scan(); -if (process.argv.includes("--update-baseline")) { +const updateBaseline = process.argv.includes("--update-baseline"); +if (updateBaseline) { writeFileSync(BASELINE, `${JSON.stringify(found, null, 2)}\n`); const total = Object.values(found).reduce((a, b) => a + b, 0); console.log(`[check-fnxc-future-dates] baseline written: ${total} stamp(s) in ${Object.keys(found).length} file(s)`); @@ -219,14 +220,35 @@ for (const [file, allowed] of Object.entries(baseline)) { const count = found[file] ?? 0; if (count < allowed) tightened.push(` ${file}: ${allowed} -> ${count}`); } +/* +FNXC:FnxcStampHygiene 2026-08-01-00:55 (a CHECK must not modify the tree it is checking): +This block used to rewrite the baseline on every plain run. The tightening itself is right — the +comment above explains why banking a stale allowance is worse — but performing it as a SIDE EFFECT of +checking handed every worker an identical uncommitted diff they had not written. + +Measured cost: on 2026-07-31/08-01 nine PRs chased three defects in this gate's area, and two of them +(#3283, #3285, five minutes apart, `+0/-1` each) deleted the SAME baseline line. Neither author wrote +it; the gate wrote it, in both of their checkouts, and each reasonably committed what they found. I +also mis-attributed my own dirty tree to leftover work and retracted a measurement partly on that +basis. + +So: still computed, still reported loudly, but only WRITTEN under --update-baseline. A plain run is +read-only and stays green — failing on a tightening would redden main every time a stamp simply ages +into the past, which is exactly why the auto-write existed. +*/ if (tightened.length > 0) { - for (const [file, allowed] of Object.entries(baseline)) { - const count = found[file] ?? 0; - if (count < allowed) { if (count === 0) delete baseline[file]; else baseline[file] = count; } - } - writeFileSync(BASELINE, `${JSON.stringify(baseline, null, 2)}\n`); - console.log(`[check-fnxc-future-dates] baseline TIGHTENED for ${tightened.length} file(s):`); + console.log(`[check-fnxc-future-dates] baseline CAN BE TIGHTENED for ${tightened.length} file(s):`); for (const line of tightened.sort()) console.log(line); + if (updateBaseline) { + for (const [file, allowed] of Object.entries(baseline)) { + const count = found[file] ?? 0; + if (count < allowed) { if (count === 0) delete baseline[file]; else baseline[file] = count; } + } + writeFileSync(BASELINE, `${JSON.stringify(baseline, null, 2)}\n`); + console.log("[check-fnxc-future-dates] baseline re-recorded."); + } else { + console.log(" run `pnpm check:fnxc-future-dates --update-baseline` to record it (one commit, one author)."); + } } if (problems.length > 0) {