executor: orphaned tasks were never resumed after a restart on a renamed board - #2947
Conversation
…enamed board A MISSED PAIR, the class ratcheted in #2879 — found in executor.ts this time. `listWipLaneTasks()` already resolved the wip lane by role. The filter beneath it did not: it re-asserted the literal `in-progress` on the rows that read returned. So on a renamed board the read found the orphans and the filter discarded every one. This is the worse half of the pattern, and the reason the sibling STRUCTURAL test (executor-resume-query-lanes) was not enough: the read looks converted, the census scores only the comparison, and the sweep silently does nothing. `resumeOrphaned` is the single path that recovers tasks after a crash or restart, so the failure surfaces only when an operator is already investigating a crash and has every reason to blame that instead. The rows come from `listTasks({ column })` per resolved column, so a row is in that column by definition; the re-assert only ever had value as a stale-snapshot guard, which membership preserves. Revert measured: with the filter back on the literal, the new case fails — the renamed card is dropped and the sweep returns before touching it. Fusion-Task-Id: KB-SELF-HEALING-QUERIES
Generalises the self-healing ratchet from #2879 after the same defect turned up in executor.ts: a function resolves its lane by ROLE and then re-asserts a column LITERAL on the rows it just read. Why this class needs a scan rather than review attention: - the read LOOKS converted, so scanning for `listTasks({ column: "…" })` finds nothing - the census scores only the comparison, so the backlog number moves the WRONG WAY - a STRUCTURAL test pinning "the read asks for resolved lanes" passes — resumeOrphaned had exactly such a test, green the whole time the sweep was dead Excludes the fallback arm of a resolved ternary (`lanes ? lanes.has(c) : c === "done"`), which is the correct shape, and allowlists four files whose literals are deliberate with the reason recorded: ephemeral-worker-manager (unresolvable-workflow default), triage (the U11 orphan case), scheduler and replan-target (sync listeners on the inert sync IR reader, pinned by sync-workflow-ir-is-always-default.pg.test.ts). Carries a positive control: a wrong source path would make every case pass by scanning nothing. Revert measured: restoring the executor.ts filter to the literal fails it, naming the function and line (`resumeOrphaned: executor.ts:5974`). Fusion-Task-Id: KB-SELF-HEALING-QUERIES
Two things the rebase onto the merged #2944 surfaced. SCOPE. The new engine-wide ratchet flagged self-healing.ts's log-dedup literal in clearStaleBlockedBy — a site the SIBLING ratchet already owns and already allows by name, with a derived sweep list that is strictly more precise. Adding a second allowance here would have given one fact two owners free to drift apart, which is the failure this program keeps hitting. The engine-wide scan now skips files that have a dedicated ratchet instead. BASELINE. executor.ts drops 8 -> 7 column guards from the resumeOrphaned fix; re-recorded downward. Revert re-measured after the narrowing: restoring the executor filter fails BOTH the behavioural case and the ratchet. Fusion-Task-Id: KB-SELF-HEALING-QUERIES
|
Warning Review limit reached
Next review available in: 22 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (5)
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 |
|
Expanding a finding I recorded on #2944, because I attempted the fix and the attempt proved the scope is bigger than I said. I claimed the last unconverted query — Three sibling signals decorate the same row, and they disagree about which lane it is in.
So on one decorated row,
Why I backed out rather than landing it. The fix is a membership What a correct change looks like, for whoever owns
No code in this PR — the engine change here stands on its own. Recorded so the next person starts from the real shape instead of my earlier, narrower description. |
…NXC gate (#2949) Two things, and the second is why the first does not ship alone. ## The opt-in `resolveProjectColumnsForRoles` gains `untraitedProject: "declared-columns"`. When **no** workflow in the project expresses **any** lifecycle trait, every declared column id joins the answer. This is the three-state rule at **project** scope — the last item on the deferred list, recorded at three self-healing call sites (#2869, #2876). A board that renames its lanes and declares no traits contributes nothing today, so its cards are **absent from every role-keyed query**, and the correct per-card fallback downstream never runs for them. A fallback cannot rescue a card the query never returned. **Not "no workflow declares this role."** A project that expresses traits and has no review lane has *answered*; widening there would invent lanes it deliberately lacks. Mutation-verified both directions — widening unconditionally fails 1 of 12, making the option a no-op fails 1 of 12. **Opt-in, not default**, because the safe direction differs per caller — the finding in `project-union-versus-per-task-lanes.md`: | caller | over-inclusion costs | |---|---| | sweep | nothing — the per-card check discards the extra rows | | aggregator | an inflated number an operator reads (#2864, #2866) | | action site | a card routed or notified under a vocabulary that is not its own (#2852, #2891) | Making it the default moves all three at once, in the one direction two of them must not. Verified byte-identical without the option, so this lands with **no caller changes** and each site adopts it on its own reasoning. ## Main was red, and my own gate caught me first I dated the new comments `2026-07-31` while today is `2026-07-30` — **the exact defect `check-fnxc-future-dates` exists to prevent, committed while writing the feature.** The gate I added yesterday failed my own commit. Correcting mine surfaced that the merged sentinel batch, #2947, and three engine test files carried future-dated stamps too, so **the gate was failing on `main` for everyone**, not just here. All corrected to real dates rather than raising the ceiling. The stamps were simply wrong, and a baseline bump would have recorded the error as permitted — which is the failure mode that ratchet exists to prevent. Core and engine `tsc` clean, `pnpm lint` clean, census `--strict` 0, FNXC gate 0 (469 known, none added), gate green (161/487/13/71). Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
resumeOrphanedis the only path that recovers tasks after a crash or restart. On a board with renamed columns it recovered nothing.A missed pair, not an unconverted read
The read was already resolved. The filter directly beneath it re-asserted the literal on the rows that read returned, so the sweep found the orphans and threw them all away.
This is the worse half of the class, and it hid well:
listTasks({ column: "…" })finds nothing;executor-resume-query-lanes.test.ts— and it was green the entire time the sweep was dead. A test asserting the read exists says nothing about the filter beneath it.The failure only surfaces after a crash, when an operator is already investigating the crash and has every reason to blame that instead.
The ratchet, generalised
#2944 ratcheted this class inside
self-healing.tsafter review found one instance and a follow-up audit found five more. This generalises it to every engine source: a function that resolves lanes and compares a column id in the same body is a pair.Excluded, deliberately:
lanes ? lanes.has(c) : c === "done") — the correct shape;ephemeral-worker-manager(unresolvable-workflow default),triage(the U11 orphan case),schedulerandreplan-target(sync listeners on the inert sync IR reader, already pinned bysync-workflow-ir-is-always-default.pg.test.ts);self-healing.ts, because it has a dedicated ratchet that is strictly more precise. Two ratchets allowlisting the same site is one fact with two owners, free to drift — the exact failure mode this program keeps hitting. One file, one ratchet.It carries a positive control: a wrong source path would make every case pass by scanning nothing.
I swept the rest of the engine with it and executor.ts was the only genuine hit — everything else is documented-deliberate or blocked on the inert sync reader.
Revert results
Each measured by restoring the literal filter and re-running:
resumeOrphaned: executor.ts:5974A non-vacuous companion (card in the review lane → not resumed) rules out a filter that matches everything: a card in review has no session to resume, and re-dispatching it would restart finished work.
Measured:
executor.tscolumn guards 8 → 7; baseline re-recorded downward.Verification
pnpm test:gate161 + 487 + 13 + 71; executor prompt/soft-delete/resume suites plus the new ratchet, 357 passed;tscengine clean;pnpm lint,check:changesets, census--strictandcheck-sql-column-literalsclean, each run explicitly.