Skip to content

self-healing: falsely-failed tasks stayed failed on a renamed board (twenty-fifth sweep) - #2919

Closed
gsxdsm wants to merge 1 commit into
mainfrom
shq36
Closed

self-healing: falsely-failed tasks stayed failed on a renamed board (twenty-fifth sweep)#2919
gsxdsm wants to merge 1 commit into
mainfrom
shq36

Conversation

@gsxdsm

@gsxdsm gsxdsm commented Jul 31, 2026

Copy link
Copy Markdown
Collaborator

recoverMisclassifiedFailures clears a failure the executor parked for "without calling fn_task_done" on a task whose steps are all actually done — the failure is a misclassification, not real work left undone. The literal read meant that on a renamed board it was never cleared, so finished work stayed visibly failed and never entered normal review.

No second pair

Verified with the derived ratchet added in #2879 rather than by eye. That instrument exists because #2916 found a second lane guard on a re-read row, and the follow-up audit found five more in another sweep — checking is now cheap, so it is not skipped.

Fixture note

The error string carries the real phrase isNoTaskDoneFailure matches ("without calling fn_task_done"). Invented prose is filtered out one line later, and the case would then pass with the fix reverted. Third time this exact trap has come up in this series, so it is now the first thing I check when writing one of these fixtures.

Revert results

Each applied alone and the file re-run:

conversion reverted →
the resolved read fails — the card is never listed
the per-card verdict fails — the renamed review lane is filtered out

A non-vacuous companion (same card in the wip lane → untouched) rules out a read that returns everything: a card still in wip has not handed off, so clearing its failure would hide a live problem rather than fix a stale one.

Verification

pnpm test:gate 161 + 487 + 13 + 71, plus self-healing.test.ts 412; tsc engine clean; pnpm lint, check:changesets, census --strict and check-sql-column-literals clean, each run explicitly.

@coderabbitai

coderabbitai Bot commented Jul 31, 2026

Copy link
Copy Markdown
Contributor

Warning

Review limit reached

@gsxdsm, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 25 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

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 configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 7c2adeae-f6f7-47ab-b6ff-b854d042481a

📥 Commits

Reviewing files that changed from the base of the PR and between c6767cb and 438b905.

📒 Files selected for processing (4)
  • .changeset/self-healing-misclassified-failures-query.md
  • packages/engine/src/__tests__/self-healing-query-filter-blindness.test.ts
  • packages/engine/src/self-healing.ts
  • scripts/lib/lifecycle-column-census-baseline.json

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

…wenty-fifth sweep)

recoverMisclassifiedFailures clears a failure the executor parked for "without
calling fn_task_done" on a task whose steps are ALL actually done — the failure is a
misclassification, not real work left undone. The literal read meant that on a
renamed board it was never cleared, so finished work stayed visibly failed and never
entered normal review.

No second pair here; verified with the derived ratchet from #2879 rather than by eye.

Fixture note: the error string carries the REAL phrase isNoTaskDoneFailure matches
("without calling fn_task_done"). Invented prose is filtered out one line later and
the case would pass with the fix reverted.

Reverts measured, each alone:
  - literal read restored -> fails, the card is never listed
  - verdict back to `t.column === "in-review"` -> fails, the renamed review lane is
    filtered out

Fusion-Task-Id: KB-SELF-HEALING-QUERIES
@gsxdsm

gsxdsm commented Jul 31, 2026

Copy link
Copy Markdown
Collaborator Author

Superseded by #2944 — folded into batch-self-healing-renamed-boards with the other 22 renamed-board sweeps. Same root cause, same file; 23 CI runs for one file was the queue jam.

The conversion and its revert measurements are carried over in the commit message. Nothing here is dropped.

@gsxdsm gsxdsm closed this Jul 31, 2026
gsxdsm added a commit that referenced this pull request Jul 31, 2026
…olds 23 PRs) (#2944)

**Consolidation of 23 open PRs into one.** Every one shared a single
root cause and mostly touched a single file; 23 CI runs for that was
indefensible.

Folds and supersedes: #2867 #2869 #2876 #2879 #2883 #2891 #2899 #2901
#2902 #2905 #2906 #2914 #2916 #2918 #2919 #2920 #2922 #2927 #2929 #2932
#2934 #2937 #2939.
(#2865, #2882, #2897, #2909, #2912 already merged and are not
re-folded.)

## The root cause

A self-healing sweep selects its work with `listTasks({ column:
"in-review" })`. On a board whose lanes are renamed that returns
**nothing**, so the sweep never runs — no error, no log line, no failed
task. Several sweeps had already had their *predicates* converted to
resolved lanes, which dropped a census count and changed nothing,
because the query above the loop had already returned an empty list.

**26 sweeps converted.** Each one: read the project's columns for the
role, then decide each card against **its own** workflow, with the
legacy ids unioned so a board mid-rename is never skipped.

## What each sweep stops silently failing to do

| | |
| --- | --- |
| stale merger status | one finished card held the **merge queue** for
everything behind it |
| stale `blockedBy` / completed-task release | dependents stayed blocked
on work that had already finished — the board stops moving |
| workspace partial lands | a task left with **some repos merged and
some not** |
| mid-merge retry stamp | the card stalled *and* the operator's manual
Retry was gated by the same stamp |
| in-progress limbo / no-progress failures | dead cards held a work slot
forever |
| partial-progress retry | real work parked failed with its **retry
budget unspent** |
| orphaned-execution signal | visibility only — the one signal pointing
at an orphan went silent |
| zero-commit audit | went **half-blind**: the error arm kept working,
the lane arm did not |

Plus: ghost review cards, transient merge failures, misclassified
failures, branch misbinding, missing-worktree failures,
merged-but-unfinished finalization, done-metadata repair, self-owned
branch conflicts, orphan-only scope violations, post-done wedges, idle
assigned agents, PR-conflict worktree ownership, and orphaned workspace
worktrees.

## Two defects the conversion itself introduced, both caught and fixed

1. **Missed pairs.** Widening a read without converting the guards
beneath it is *worse than not converting*: the sweep starts admitting
renamed-board cards and then mis-decides every one. Review caught a
second guard on a re-read row; the audit that triggered found **five
more**, one of which gates the `reviewProof` triple-proof — a renamed
review card would have been moved backward with the safety check
silently skipped. Column guards 86 → 81.
2. **Duplicate processing.** The literal reads were disjoint by
construction; resolved reads are not, so a column carrying two role
flags put one card in two buckets — duplicate moves, duplicate audit
rows, inflated counts.

Both now have ratchets.
`self-healing-converted-sweeps-have-no-literal-lane-guards.test.ts`
**derives** its sweep list (a sweep counts as converted when its body
calls `resolveProjectColumnsForRoles`), so it cannot go stale, and it
carries two positive controls because a broken regex finds no offenders
and a broken derivation iterates nothing — an empty loop registers no
tests and reads green.

## Deliberately unchanged

- 22 `moveTask` destinations carrying `recoveryRehome: true` —
`moves.ts` exempts these so a card stranded in an undeclared column
stays rescuable.
- One literal in `clearStaleBlockedBy`'s log-dedup closure (allowed by
name in the ratchet, with the reason).
- `surfaceInReviewStalls` — hot list-read path, needs a batched
prefetch; that is a performance design decision, not a conversion.
- `scheduler.ts` and `replan-target.ts` — built on
`resolveTaskWorkflowIrSync`, which returns the default IR for every task
in production. Converting there produces inert code.

## The fold itself is worth one note

All 23 branches appended to the **same test file at the same anchor**,
so every automatic strategy — git 3-way, `merge-file --union`, and three
hand-written resolvers — interleaved them mid-block. Two attempts
committed conflict markers before I caught it. The file is therefore
**reconstructed**: head authored once, body assembled as the union of
each branch's own intact top-level segments keyed by test title, with
the nested `already-merged hard blocker` describe appended whole
(flattening it orphaned its helper). Verified by *parsing after every
step* rather than trusting the merge — which is how each interleaving
was caught.

## Verification

`pnpm test:gate` 161 + 487 + 13 + 71. Scoped suites 592 passed
(self-healing, the blindness suite at 68 cases, the ratchet, and the
notification suite). `tsc` engine clean; `pnpm lint`,
`check:changesets`, `lifecycle-column-census --strict` and
`check-sql-column-literals` all clean, each run explicitly.

Each folded conversion was individually revert-proven on its original
branch — the read reverted alone, and the per-card verdict reverted
alone — and those measurements are recorded in the commit messages
carried into this branch.

---------

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant