fix(scripts): the FN-4000 consistency reconciler failed in BOTH directions on a renamed board - #2994
Merged
Merged
Conversation
…tions on a renamed board `findTaskStateInconsistencies` keyed both of its checks on legacy lane literals, and they broke in opposite directions: `hasDoneTransient` gated on `column === "done"`, so on a board whose complete lane is named anything else it NEVER fires — a finished card still carrying `status:"failed"`, a worktree, a blockedBy or live recovery counters is never reported and never normalized. That stale state is exactly what FN-4000 exists to clear. `failed-status-outside-in-review` gated on `column !== "in-review"`, which fails the OPPOSITE way: no column equals the literal, so EVERY failed task is flagged. A reconciliation report listing the whole board is as useless as one listing nothing, and it is the more dangerous of the two because it looks like the tool is working. Lanes are resolved per task and injected, not resolved inside the loop: doing it inline would drag `importCore()` — and a built `packages/core/dist` — into every unit test of a pure reconciliation loop. `main` wires the real resolver whenever it opened a real backend, so the parameter is not the inert shape this migration keeps finding; a caller injecting its own store (tests) has no staged dist and falls back to the documented legacy literals. `importCore` is now exported from `scripts/lib/backend-db.mjs` so operator scripts reach core helpers through the SAME staged-dist seam `openBackend` already uses, rather than each growing its own dist path — `@fusion/core` is not resolvable from repo-root `scripts/`. The normalization move now targets the card's OWN column. Naming `"done"` was only ever a way of spelling "where it already is", since the move exists to trigger the store's done-normalization. One test expectation of mine was wrong before the code was: a failed card in the complete lane trips BOTH checks, which is correct. The case now isolates the first check so it pins one behaviour rather than blurring two. Reverted, 3 of the 5 new cases fail. Verified: 8 passed here, 7 in the sibling script suites, node --check and eslint clean, census / sql-literal / lane-wiring / fnxc green. No changeset — root `scripts/` is repo tooling, not part of the published package.
Collaborator
Author
|
Verified — both directions of the fix are pinned, and I have one thing to correct before merge. Mutations, one per direction of the original defect:
Each arm has its own failing case, which matters here more than usual: the two checks break in opposite ways (one never fires, one fires on everything), so a single test covering "renamed board" could easily have pinned one arm and left the other free. It doesn't. Three stamps carry an impossible clock timeHour This is systemic, not your slip. Counting what I have found so far: four already on I have extended that gate and normalized the four on The rising hours across PRs (24 → 25 → 26) suggest they are being written as a continuing sequence past midnight rather than read off a clock. Worth knowing that the local calendar is what the gate compares against, so a genuine post-midnight stamp needs tomorrow's date, which is future-dated and will fail for a different reason — clamping to |
Collaborator
Author
Correction to the follow-up note in this PR's descriptionI wrote that
return { core, store: boot.taskStore, asyncLayer, sql, schema, shutdown };and the leaked-soft-deletes script already destructures What that follow-up actually involves, for whoever takes it:
I stopped short of it deliberately: it is a mutation path against operator databases, it moves a gate baseline, and I was at the end of a long session. Rushing that particular change is how a "conversion" corrupts real boards. The design above is the whole of what I worked out, so nobody has to re-derive it. |
Contributor
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughReconciliation now resolves configurable complete and review lanes, preserves ChangesConfigurable lane reconciliation
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant BackendCore
participant LaneResolver
participant Reconciliation
participant TaskStore
BackendCore->>LaneResolver: Resolve lifecycle columns
LaneResolver->>Reconciliation: Return complete and review lanes
Reconciliation->>TaskStore: Inspect task state
Reconciliation->>TaskStore: Move task to resolved complete lane
Possibly related PRs
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
gsxdsm
added a commit
that referenced
this pull request
Jul 31, 2026
…nto boards that do not have one (#2999) ## A repair script that wrote a column the board does not have Under `--apply`, against an operator's live database: ```js await tx.execute(sql`UPDATE project."tasks" SET "column" = 'archived' WHERE id = ${row.id}`); ``` On a board that does not declare `archived`, that is not a mislabel — it parks the row in a column the workflow does not have, **manufacturing exactly the undeclared-column state this migration keeps repairing elsewhere**. The selection was wrong in the same direction, which made the write far worse. "Leaked" meant `column !== "archived"`, so on a renamed board **every** soft-deleted row looked leaked — including the ones resting correctly in that board's own archived lane. The repair then rewrote them. The tool's fix *was* the damage. ## Three changes, because fixing one would have left the others deciding **The SQL pre-filter carried the same literal** (`AND "column" != 'archived'`), so the query and the planner each imposed the legacy vocabulary independently. Dropped it — soft-deleted rows are a small set, so selecting them all and filtering in the pure planner costs nothing and leaves **one** place that decides what "archived" means. **The filter takes the set**; a row resting in *any* of the board's archived lanes is not leaked. **The write resolves per task**, because the destination must be that card's own lane, not a board-wide pick. A row whose archived lane cannot be resolved is **skipped and reported**, never written with a guessed id. A recovery script that declines to act on rows it does not understand is recoverable; one that writes a plausible wrong value is not. Verified rather than assumed — a store that answers nothing resolves to the default lifecycle: ``` lifecycle from unanswering store: {"intake":"todo",…,"archived":"archived"} ``` so a legacy board repairs exactly as before. ## Correcting myself On #2994 I wrote that this follow-up "needs the same `importCore` seam". It doesn't: `openBackend` already returns `{ core, store, … }` and this script already destructures `core`. No new plumbing was required. I posted that correction on #2994 too, since acting on it would have wasted someone's time. ## Revert proof ``` ✖ a soft-deleted row already in the board's RENAMED archived lane is not leaked ✖ a board with several archived lanes treats all of them as resting places ℹ pass 5 ℹ fail 2 ``` The other two new cases pass both ways by design — they guard the legacy meaning and the still-catches-a-real-leak direction — so I am not counting them as coverage of the defect. ## Verification (measured) - `node --test` across all three script suites — **17 passed / 0 failed** - `node --check`, `eslint` — clean - `check-sql-column-literals`, `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. ## Gate blind spot found while verifying this, NOT fixed here I removed a raw-SQL lane literal and expected `check-sql-column-literals` to drop from 22 — its own header says *"a LOWER count fails too so the baseline is ratcheted down"*. It stayed at **22 and green**, because it walks `PACKAGES` only and **never scans `scripts/`**. That is the same shape as the lane-wiring gap #2978 closed (it scanned neither `plugins` nor `dashboard/app`). `scripts/audit-branch-cross-contamination.mjs:185` still holds `WHERE … "column" IN ('triage','todo','in-progress','in-review')`, invisible to the gate. Left as a separate follow-up rather than bundled into a product fix.
This was referenced Jul 31, 2026
gsxdsm
added a commit
that referenced
this pull request
Jul 31, 2026
#3000 widened check-sql-column-literals to scan scripts/, baselining this file's 4-literal IN clause. This branch removes that clause, so the recorded population sits 4 above the tree — a DECREASE, which the gate fails on by design and auto-rewrites. Re-recorded to 22. This is the merge-order hand-off flagged on the PR, done rather than left for the operator. Still blocked on #3008 for the Lint check: main is red on check-fnxc-future-dates because of four hour-26 stamps I wrote in #2994, and CI lints the branch MERGED with main. Nothing in this branch can fix that; #3008 is the fix.
gsxdsm
added a commit
that referenced
this pull request
Jul 31, 2026
#3006) **`main` is currently red on the FNXC gate.** ``` $ node scripts/check-fnxc-future-dates.mjs # on origin/main scripts/reconcile-task-state-consistency.mjs: 2 future-dated FNXC stamp(s), baseline allows 0 scripts/lib/backend-db.mjs: 1 scripts/__tests__/reconcile-task-state-consistency.test.mjs: 1 exit 1 ``` #2994 carried four `2026-07-30-26:10` stamps. I flagged them on that PR before it merged; #2995 (the hour check) landed first, so the merge order turned the warning into a red gate rather than a red PR. Clamped to `23:10` — same rule as the nine before it: hour to `23`, minutes preserved, so ordering within each file survives. This is a normalization with a stated rule, not a claim about the true minute. **Verified:** FNXC gate exit 0, `reconcile-task-state-consistency` 8 pass / 0 fail. Comment-text only. ### Worth fixing at the source Thirteen impossible-hour stamps across six PRs in two days, and the hours climb — `24:40` → `25:30` → `26:10`. They are being written as a continuing sequence past midnight rather than read off a clock, which is a reasonable instinct and produces an invalid stamp every time. The trap is that the honest spelling does not work either: a genuine post-midnight stamp needs *tomorrow's* date, and the gate compares against the **local** calendar — so `2026-07-31-00:40` written from UTC-7 is future-dated and fails for a different reason. Clamping to `23:xx` is currently the only spelling that satisfies both, which is not obvious and is why this keeps recurring. If it recurs again, the fix is probably in the error message rather than more normalization PRs: the gate could name the valid range and the timezone it compares against, so the next author sees the constraint at the moment they hit it. Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
gsxdsm
added a commit
that referenced
this pull request
Jul 31, 2026
#3000 widened check-sql-column-literals to scan scripts/, baselining this file's 4-literal IN clause. This branch removes that clause, so the recorded population sits 4 above the tree — a DECREASE, which the gate fails on by design and auto-rewrites. Re-recorded to 22. This is the merge-order hand-off flagged on the PR, done rather than left for the operator. Still blocked on #3008 for the Lint check: main is red on check-fnxc-future-dates because of four hour-26 stamps I wrote in #2994, and CI lints the branch MERGED with main. Nothing in this branch can fix that; #3008 is the fix.
gsxdsm
added a commit
that referenced
this pull request
Jul 31, 2026
…laimed 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.
gsxdsm
added a commit
that referenced
this pull request
Jul 31, 2026
**`main` is currently red**, so every open PR shows a failing Lint job that is not its own fault. #3004 is how I found it — its gates all pass in isolation and fail against main. ## Cause: an hour that does not exist ``` FNXC:OperatorScriptLaneAssumptions 2026-07-30-26:10 ^^ hour 26 ``` Four of them, across three files, from #2994. ## The part worth fixing is the message, not the stamps This gate counts **two** defects — a date after today, and an impossible clock time — but the failure text only ever explained the first: ``` scripts/reconcile-task-state-consistency.mjs: 2 future-dated FNXC stamp(s), baseline allows 0 A stamp dated after today (2026-07-31) records the change as happening in the future... ``` Every stamp in that file is dated `2026-07-30` or earlier — all valid past dates. So the message sends you to inspect stamps that are fine, and the natural conclusion is *the gate is broken*, not *the stamp is*. I spent several minutes reproducing the regex by hand and getting `future count = 0` before instrumenting the real script and finding `hits += impossibleClockTimes(source)`. A gate that detects the right defect and describes a different one is worse than a slightly less sensitive gate, because it spends the reader's trust. Now: ``` scripts/reconcile-task-state-consistency.mjs FNXC:OperatorScriptLaneAssumptions 2026-07-30-26:10 (impossible clock time) ``` **Mutation-verified**: restoring one `26:10` stamp reproduces the failure, and the message names it. ## The stamps: `2026-07-31-02:10`, not `23:59` Hour 26 on the 30th is the informal spelling of 02:10 the next day. Clamping to `23:59` would keep the file's stamps in a plausible order but silently move the event; this preserves what the author meant. Reversible either way — say the word if you would rather they were clamped. ## The 176-file baseline drop is unrelated `475 -> 183 known`. The clock crossed midnight, so yesterday's stamps are no longer future-dated, and the ratchet auto-lowers on drops by design. It rides along because the gate must leave a baseline matching reality — an allowance nothing occupies is somewhere a real regression can hide. It is not part of the fix. ## Verification - `check:fnxc-future-dates` — exit 0 (was **exit 1 on main**) - `check:lifecycle-columns`, `check:sql-column-literals`, `check:inert-flag-seams`, `check:lane-wiring` — all exit 0 - eslint clean ## Worth someone's attention beyond this PR `#2994` landed four impossible timestamps. The gate caught them, but only after the clock crossed midnight changed which files it reported — meaning the impossible-time check was live but effectively invisible until it collided with an unrelated drop. It is worth asking whether that check has ever produced a message anyone acted on before today. Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
gsxdsm
added a commit
that referenced
this pull request
Jul 31, 2026
…letion date (#3007) The **fifth** instance of the async-memo shape #2998 documents, and the one that survived #3001's sweep. `lifecycleDates` gates its `completed` value on `isCompleteColumn || isArchivedColumn` — both derived from the async `taskColumnFlags` prop — while listing neither: ```js }, [task.createdAt, task.executionCompletedAt, task.archivedAt, task.column, locale, lifecycleNowMs]); ``` First paint runs with the flags undefined, the role helpers fall back to the legacy ids, and on a board whose complete lane is named anything but `done` that answers false. The flags arrive, `task.column` has not changed, nothing recomputes, and the card renders **no "Completed <date>" line at all**. ## Why #3001's sweep called this covered That PR recorded `mergeSignature` as *"the last live site … nine persistent candidates, seven covered transitively or by a dependency that already carries the flags."* This memo was presumably in the covered pile, and the reasoning is nearly right: it **does** list a dependency that changes — `lifecycleNowMs`. But that value is driven by a timer scheduled with `millisecondsUntilNextLocalMidnight` (FN-8561, so compact date labels turn over at the viewer's midnight). **A dependency that changes once a day is not coverage for a value that must be correct on first paint.** The card shows no completion date for the rest of the session. That distinction is worth adding to the doc's property 2: *does a listed dependency change* is the wrong question — *does it change when the resolved value arrives* is the right one. ## Verification | state | result | |---|---| | clean | 2/2 pass | | revert the dep fix | **1 failed / 1 passed** | The control case (a `done` board) passes either way by design, so a failure in the renamed case means "renamed board", not "nothing renders". **One trap worth recording**, since it nearly cost me the finding: my first `completedLine()` used `time[datetime]:last-of-type`. When only the *Created* line renders, that selector returns **that** element — so the pre-resolution absence assertion silently passed against the wrong node. The test now matches on the element's own `Completed` label. A positional selector cannot express "this specific line is missing". `tsc -p tsconfig.app.json` 0 errors, lint clean, 8/8 across all three renamed-lane TaskCard suites. ## Note `main` is currently red on the FNXC gate for an unrelated reason (#2994's impossible-hour stamps landing after #2995); fixed in #3006. Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The FN-4000 consistency reconciler failed in both directions
findTaskStateInconsistencieskeyed both checks on legacy lane literals, and they break in opposite ways:hasDoneTransientstatus:"failed", a worktree, a blockedBy or live recovery counters is never reported and never normalized — precisely the stale state FN-4000 exists to clearfailed-status-outside-in-reviewThe second is the more dangerous of the two: a tool that reports nothing looks broken, but a tool that reports everything looks like it is working.
Wiring, and why the resolver is injected rather than built inline
Lanes are resolved per task (a board can span workflows) and passed in. Resolving inside the loop would drag
importCore()— and therefore a builtpackages/core/dist— into every unit test of a pure reconciliation loop.mainwires the real resolver whenever it opened a real backend, so this is not the inert optional-parameter shape this migration keeps finding. A caller injecting its own store (tests) has no staged dist and falls back to the documented legacy literals, which is exactly today's behaviour.importCoreis now exported fromscripts/lib/backend-db.mjsso operator scripts reach core helpers through the same staged-dist seamopenBackendalready uses, rather than each growing its own dist path —@fusion/coreis not resolvable from repo-rootscripts/, which is what made the obvious import fail.The normalization move now targets the card's own column: naming
"done"was only ever a way of spelling "where it already is", since the move exists to trigger the store's done-normalization.One of my test expectations was wrong before the code was
My first version asserted that a card in a renamed complete lane with
status:"failed"yields only the transient-state finding. It yields both — and that is correct, because a failed card outside the review lane genuinely is flagged. I isolated the case (droppingstatus:"failed", keeping the worktree) so it pins one behaviour instead of blurring two, rather than "fixing" the expectation to match whatever came out.Revert proof
Restoring the four literals:
The remaining two new cases pass both ways by design — "still flags a failed card outside the resolved review lane" and "unresolved lanes keep exactly the legacy behaviour" guard against over-correction, so I am not counting them as coverage of the defect.
Verification (measured)
node --test— 8 passed / 0 failed (3 pre-existing + 5 new)recover-stale-blocked-by,reconcile-leaked-soft-deletes) — 7 passed, unaffected by the shared-lib exportnode --check,eslint— cleanlifecycle-column-census --strict,check-sql-column-literals,check-lane-wiring,check-fnxc-future-dates— greenNo changeset: root
scripts/is repo tooling, not part of the published package.Still not addressed in this territory
reconcile-leaked-soft-deletes.mjscarries a rawUPDATE project."tasks" SET "column" = 'archived'— on a renamed board that writes a column the workflow does not declare, creating the undeclared-column state this migration keeps repairing elsewhere. It holds a raw backend rather than a store, so it needs the sameimportCoreseam this PR exports; left for a follow-up rather than bundled here.Summary by CodeRabbit