Conversation
|
Warning Review limit reached
Next review available in: 2 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 (6)
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 |
Greptile SummaryThis PR completes workflow-aware self-owned branch-conflict recovery.
Confidence Score: 5/5The PR appears safe to merge. No blocking failure remains; the previously reported duplicate recovery is prevented by deduplicating task IDs before the side-effecting candidate loop, and the regression test verifies one evaluation for a task returned through overlapping workflow roles.
|
| Filename | Overview |
|---|---|
| packages/engine/src/self-healing.ts | Adds workflow-resolved candidate discovery and lane guards, with cross-bucket task-id deduplication that addresses the previously reported duplicate recovery. |
| packages/engine/src/tests/self-healing-query-filter-blindness.test.ts | Covers renamed work-in-progress and review lanes and verifies that a multi-role task is processed only once. |
| packages/engine/src/tests/self-healing.test.ts | Documents why the focused query-filter test provides the non-vacuous regression coverage for cross-role deduplication. |
| scripts/lib/lifecycle-column-census-baseline.json | Updates literal lifecycle-column query counts to reflect the three converted reads. |
Reviews (3): Last reviewed commit: "fix(self-healing): prove the multi-role ..." | Re-trigger Greptile
…e same stale snapshot #2879 review. `readBucket` dedupes by id inside ONE role's read. A custom workflow may put more than one queried flag on a single column — `hold` plus `countsTowardWip` on a lane that both parks and counts as work, or a review role beside either — and that column is returned by two reads. The task landed in two buckets and the concatenation handed the recovery loop the SAME snapshot twice. Not a wasted iteration: the second pass re-reads branch/worktree from state captured before the first pass mutated anything, so an already-reclaimed worktree is reclaimed again against state that no longer exists. The lane-resolution loop above already guards with `reclaimLanes.has(task.id)`; the consumption side was missing the same guard. Deduping by id preserves order, so role precedence is unchanged. SHIPS UNCOVERED, AND I DELETED THE TEST THAT PRETENDED OTHERWISE. My case reported 0 recoveries: the card it built reaches the BRANCH-LEVEL scan (subsumed branch, no worktree), not the candidates loop the dedupe lives in. Reaching that loop needs branch AND worktree set plus matching git state — the git fixture this suite avoids. A test that passes without exercising the loop is worse than none. What would cover it is recorded at the site. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
Confirmed and fixed. 1f90558.
It is not a wasted iteration. The second pass re-reads This ships uncovered, and I deleted the test that pretended otherwise. My case reported 0 recoveries: the card it built reaches the branch-level scan (subsumed branch, no worktree), not the candidates loop the dedupe lives in. Reaching that loop needs What would cover it is recorded at the site: a candidates-loop fixture with a live branch/worktree pair, asserting the recovery count — a double-processed card reports 2 for one card's worth of work — rather than a call count. Engine |
…dence claim Two follow-ups to the P1 on #2879, both found by running the revert. 1. THE DEDUPE HAD NO TEST. Added one: a board whose hold column also carries the wip trait, so one column is returned by two role reads. Without the dedupe the loop runs twice on the same stale snapshot (measured: 2 calls, expected 1). 2. THE COMMENT CLAIMED THE WRONG PRECEDENCE. `new Map(entries)` keeps first INSERTION ORDER but the LAST value for a repeated key, so the code gave last-bucket precedence while the comment said first-bucket. Replaced with an explicit `has` guard so the code does what the comment says. The test was vacuous on its first run and passed with the dedupe reverted. The stub returned null and the sweep reads `.phantom` straight off it, so it THREW and aborted the loop after one card — capping the count at 1 in both states. Seventh vacuous assertion on this branch; seventh caught by the revert rather than by reading. Stub now returns the real `{ phantom, metadata }` shape. Revert measured: without the dedupe the new case fails with 2 calls instead of 1. Fusion-Task-Id: KB-SELF-HEALING-QUERIES
…arried two roles #2883 review, two findings. DEDUPE (fixed). `readDependentBucket` dedupes by id inside ONE role's read, so a custom column carrying two queried roles is returned by two reads and the dependent was reconciled twice from the same stale snapshot — the second pass deciding against `blockedBy` state the first had already cleared. Same class as #2879 one sweep over. Order preserved, so `todoTaskIds` still classifies by the read that found it first. SATISFACTION BREADTH (recorded, not narrowed). The concern is that a mergeOrchestration-only lane reads as satisfying a dependency. Measured against what this replaced: the legacy set was done/in-review/archived, and `in-review` carries mergeOrchestration on every builtin — a dependency parked in review has ALWAYS released its dependents, so including the review roles reproduces the legacy contract exactly. Narrowing to terminal-only is a behaviour change in the DEADLOCK direction on a sweep whose purpose is unblocking, and it is a scheduler-contract question about when a dependency counts as done, not a vocabulary conversion. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…2879 Applying the review finding from #2879 to this sweep, which has the identical concatenation. The three literal reads were disjoint BY CONSTRUCTION (one column each), so a dependent could not appear twice. Resolved reads are not: a custom workflow may put more than one queried role flag on one column. Here the duplicate is a double WRITE — updateTask and logEntry both fire twice for one dependent, so the operator sees the release logged twice and blockedByCleared over-counts. Deduped with an explicit `has` guard rather than new Map(entries): that constructor keeps first insertion ORDER but the LAST value for a repeated key, so it reads as first-bucket precedence while doing the opposite. Revert measured: without the dedupe the new case fails with 2 clearing writes instead of 1. Fusion-Task-Id: KB-SELF-HEALING-QUERIES
…e same stale snapshot #2879 review. `readBucket` dedupes by id inside ONE role's read. A custom workflow may put more than one queried flag on a single column — `hold` plus `countsTowardWip` on a lane that both parks and counts as work, or a review role beside either — and that column is returned by two reads. The task landed in two buckets and the concatenation handed the recovery loop the SAME snapshot twice. Not a wasted iteration: the second pass re-reads branch/worktree from state captured before the first pass mutated anything, so an already-reclaimed worktree is reclaimed again against state that no longer exists. The lane-resolution loop above already guards with `reclaimLanes.has(task.id)`; the consumption side was missing the same guard. Deduping by id preserves order, so role precedence is unchanged. SHIPS UNCOVERED, AND I DELETED THE TEST THAT PRETENDED OTHERWISE. My case reported 0 recoveries: the card it built reaches the BRANCH-LEVEL scan (subsumed branch, no worktree), not the candidates loop the dedupe lives in. Reaching that loop needs branch AND worktree set plus matching git state — the git fixture this suite avoids. A test that passes without exercising the loop is worse than none. What would cover it is recorded at the site. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…dence claim Two follow-ups to the P1 on #2879, both found by running the revert. 1. THE DEDUPE HAD NO TEST. Added one: a board whose hold column also carries the wip trait, so one column is returned by two role reads. Without the dedupe the loop runs twice on the same stale snapshot (measured: 2 calls, expected 1). 2. THE COMMENT CLAIMED THE WRONG PRECEDENCE. `new Map(entries)` keeps first INSERTION ORDER but the LAST value for a repeated key, so the code gave last-bucket precedence while the comment said first-bucket. Replaced with an explicit `has` guard so the code does what the comment says. The test was vacuous on its first run and passed with the dedupe reverted. The stub returned null and the sweep reads `.phantom` straight off it, so it THREW and aborted the loop after one card — capping the count at 1 in both states. Seventh vacuous assertion on this branch; seventh caught by the revert rather than by reading. Stub now returns the real `{ phantom, metadata }` shape. Revert measured: without the dedupe the new case fails with 2 calls instead of 1. Fusion-Task-Id: KB-SELF-HEALING-QUERIES
…d board (fifteenth sweep) (#2897) `recoverMergedReviewTasks` finalizes a task whose merge is **confirmed** but which never reached the complete lane. Two literal reads meant that on a renamed board it was never found, so a card whose commit is already on the base branch sat in review or hold indefinitely — merged work the board still shows as unfinished. ## The two redundant guards convert, they don't get deleted Both `t.column === …` checks were redundant while the query pinned the column. Under a resolved read they become the per-card verdict. Deleting them would have silently widened the sweep — the same trap called out in #2891. ## Carries the two shapes review established earlier in this series - **Narrow when the card can answer, broad when it cannot** (#2891). `resolveWorkflowIrForTask` *substitutes* the built-in IR rather than failing, so a card with an unreadable selection would otherwise be rejected by the very verdict that the project-scoped query had just admitted it under. It falls back to the project sets instead. - **Deduped across the buckets** (#2879), so a column carrying both a review role and the hold role cannot finalize one card twice. Both were review findings on earlier PRs in this series, applied here up front rather than waiting to be caught again. ## Revert results Each applied alone and the file re-run: | conversion | reverted → | | --- | --- | | the resolved reads | fails — the card is never listed | | the per-card review verdict | fails — the renamed review lane does not match | Observable is `resolveSelfHealingMergeTarget`, a private method called once per candidate, so the assertion sits downstream of both halves without a git fixture. A non-vacuous companion (merge-confirmed card in the wip lane → untouched) rules out a read that returns everything. ## Verification `pnpm test:gate` 161 + 487 + 13 + 71, plus `self-healing.test.ts` 412; `tsc` engine clean; `pnpm lint`, `check:changesets`, census `--strict` clean, each run explicitly.
…ard (sixteenth sweep) reconcileStaleMergerStatus clears a `merging`/`merging-pr` status left on a card that already reached a terminal lane. Two literal reads meant that on a renamed board it was never cleared — and unlike the rest of this series the damage is not confined to the stranded card: the stale status holds the MERGER QUEUE for every task behind it. ONE union read over TERMINAL_ROLES, not two buckets. Nothing here treats complete and archived differently — the only filter is on `status` — so splitting them would encode a distinction the code does not make. Deduped, since the two roles can share a column (the P1 on #2879). No per-card lane verdict: this sweep has no column comparison to convert, so adding one would be inventing a gate rather than resolving an existing one. Revert measured: with the literal reads restored the new case fails — the card is never listed, so its stale status is never cleared. Fusion-Task-Id: KB-SELF-HEALING-QUERIES
…ard (fourteenth sweep) recoverForeignOnlyContaminatedInReviewTasks classifies a branch carrying ONLY foreign commits and clears the contamination park nothing else clears. Two literal reads meant that on a renamed board it classified nothing and the task stayed parked indefinitely. The two `task.column === …` checks inside its filters were REDUNDANT while the query pinned the column. Under a resolved read they become the per-card verdict, so they convert here rather than being deleted — deleting them would have widened the sweep silently. Deduped across the buckets (the P1 reviewed on #2879). It matters more here than elsewhere: the two filters have DIFFERENT predicates, so a column carrying both a review role and the wip role could match both and classify one branch twice. Reverts measured, each run alone: - literal reads restored -> fails, the card is never listed - review verdict back to `task.column === "in-review"` -> fails, the renamed review lane does not match and the card is filtered out Fusion-Task-Id: KB-SELF-HEALING-QUERIES
…arried two roles #2883 review, two findings. DEDUPE (fixed). `readDependentBucket` dedupes by id inside ONE role's read, so a custom column carrying two queried roles is returned by two reads and the dependent was reconciled twice from the same stale snapshot — the second pass deciding against `blockedBy` state the first had already cleared. Same class as #2879 one sweep over. Order preserved, so `todoTaskIds` still classifies by the read that found it first. SATISFACTION BREADTH (recorded, not narrowed). The concern is that a mergeOrchestration-only lane reads as satisfying a dependency. Measured against what this replaced: the legacy set was done/in-review/archived, and `in-review` carries mergeOrchestration on every builtin — a dependency parked in review has ALWAYS released its dependents, so including the review roles reproduces the legacy contract exactly. Narrowing to terminal-only is a behaviour change in the DEADLOCK direction on a sweep whose purpose is unblocking, and it is a scheduler-contract question about when a dependency counts as done, not a vocabulary conversion. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…2879 Applying the review finding from #2879 to this sweep, which has the identical concatenation. The three literal reads were disjoint BY CONSTRUCTION (one column each), so a dependent could not appear twice. Resolved reads are not: a custom workflow may put more than one queried role flag on one column. Here the duplicate is a double WRITE — updateTask and logEntry both fire twice for one dependent, so the operator sees the release logged twice and blockedByCleared over-counts. Deduped with an explicit `has` guard rather than new Map(entries): that constructor keeps first insertion ORDER but the LAST value for a repeated key, so it reads as first-bucket precedence while doing the opposite. Revert measured: without the dedupe the new case fails with 2 clearing writes instead of 1. Fusion-Task-Id: KB-SELF-HEALING-QUERIES
…e same stale snapshot #2879 review. `readBucket` dedupes by id inside ONE role's read. A custom workflow may put more than one queried flag on a single column — `hold` plus `countsTowardWip` on a lane that both parks and counts as work, or a review role beside either — and that column is returned by two reads. The task landed in two buckets and the concatenation handed the recovery loop the SAME snapshot twice. Not a wasted iteration: the second pass re-reads branch/worktree from state captured before the first pass mutated anything, so an already-reclaimed worktree is reclaimed again against state that no longer exists. The lane-resolution loop above already guards with `reclaimLanes.has(task.id)`; the consumption side was missing the same guard. Deduping by id preserves order, so role precedence is unchanged. SHIPS UNCOVERED, AND I DELETED THE TEST THAT PRETENDED OTHERWISE. My case reported 0 recoveries: the card it built reaches the BRANCH-LEVEL scan (subsumed branch, no worktree), not the candidates loop the dedupe lives in. Reaching that loop needs branch AND worktree set plus matching git state — the git fixture this suite avoids. A test that passes without exercising the loop is worse than none. What would cover it is recorded at the site. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…dence claim Two follow-ups to the P1 on #2879, both found by running the revert. 1. THE DEDUPE HAD NO TEST. Added one: a board whose hold column also carries the wip trait, so one column is returned by two role reads. Without the dedupe the loop runs twice on the same stale snapshot (measured: 2 calls, expected 1). 2. THE COMMENT CLAIMED THE WRONG PRECEDENCE. `new Map(entries)` keeps first INSERTION ORDER but the LAST value for a repeated key, so the code gave last-bucket precedence while the comment said first-bucket. Replaced with an explicit `has` guard so the code does what the comment says. The test was vacuous on its first run and passed with the dedupe reverted. The stub returned null and the sweep reads `.phantom` straight off it, so it THREW and aborted the loop after one card — capping the count at 1 in both states. Seventh vacuous assertion on this branch; seventh caught by the revert rather than by reading. Stub now returns the real `{ phantom, metadata }` shape. Revert measured: without the dedupe the new case fails with 2 calls instead of 1. Fusion-Task-Id: KB-SELF-HEALING-QUERIES
|
Self-audit finding, pushed in 936996a — and it is the more serious half of this PR. #2916 caught a second lane guard on a re-read row via its behavioural test. I then scanned the other sweeps I had converted, and found five more in this one, all in the loop body under the widened read. One matters a great deal: if (task.column === "in-review") { // decides whether the backward move needs reviewProofLeft literal, a renamed review card admitted by the widened read reads as not-in-review, so the triple-proof gate is skipped and the card is moved back without it. That is a safety check silently bypassed by the conversion — strictly worse than the bug the conversion fixed. The other four gate the resume-limbo path the same way. Measured: And a ratchet, since a scan found itThe class is scan-findable, so it gets a scan: Three refinements, each from a real false positive it produced first:
Two positive controls, because both halves fail silently: a broken regex finds no offenders, and a broken derivation iterates nothing — an empty Revert measured: restoring any one of the five fails the ratchet, naming the sweep and the exact line ( |
…board (thirtieth sweep) recoverPartialProgressNoTaskDoneFailures retries a review card failed for "no fn_task_done" that DID make step progress — real work exists, so the sweep spends a retry rather than discarding it. The literal read meant that on a renamed board the retry never fired: the work was parked failed with its retry budget UNTOUCHED. That budget exists precisely to avoid losing partial work, and it was never spent — the safeguard and the work were lost together. No second pair; verified with the derived ratchet from #2879. Fixture note: one step done and another pending. All-done trips isTaskWorkComplete and none-done trips hasStepProgress, either of which would filter the card out for a reason unrelated to lanes. Reverts measured, each alone: - literal read restored -> fails, the card is never listed - verdict back to `task.column === "in-review"` -> fails, the renamed review lane is filtered out Fusion-Task-Id: KB-SELF-HEALING-QUERIES
…oard (twenty-ninth sweep) recoverNoProgressNoTaskDoneFailures requeues a wip card the executor failed for "no fn_task_done" that made NO step progress and left no git work — nothing to salvage, so requeueing is safe. The literal read meant that on a renamed board it was never requeued: a card that produced nothing sat failed while still holding its wip slot, so the capacity was lost as well as the task. No second pair; verified with the derived ratchet from #2879. Fixture note: the error string carries the REAL phrase isNoTaskDoneFailure matches. Reverts measured, each alone: - literal read restored -> fails, the card is never listed - verdict back to `task.column === "in-progress"` -> fails, the renamed wip lane is filtered out Fusion-Task-Id: KB-SELF-HEALING-QUERIES
…enty-sixth sweep) recoverBranchMisboundInReviewTasks detects a review card whose BRANCH TIP is bound to a different task's work. The literal read meant that on a renamed board the misbinding was never detected, so the card would merge — or refuse to — against a branch that is not its own. No second pair; verified with the derived ratchet from #2879 rather than by eye. Reverts measured, each alone: - literal read restored -> fails, the card is never listed - verdict back to `task.column === "in-review"` -> fails, the renamed review lane is filtered out Fusion-Task-Id: KB-SELF-HEALING-QUERIES
…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
… board (twenty-fourth sweep) recoverStaleIncompleteReviewTasks requeues a review card whose STEPS are not finished — it reached review on a graph failure, not on completed work. The literal read meant that on a renamed board it was never requeued, so the card sat in review claiming to be done while its own steps said otherwise. Checked for a second pair deliberately: the triple-proof here is NOT lane-gated, so unlike #2916 (a second guard on a re-read row) and #2879 (five in the loop body) there is nothing else to convert. The audit ratchet added in #2879 confirms it. The requeue keeps its literal `todo` — recoveryRehome: true, one of the 22 documented escapes. Reverts measured, each alone: - literal read restored -> fails, the card is never listed - verdict back to `task.column === "in-review"` -> fails, the renamed review lane is filtered out Fusion-Task-Id: KB-SELF-HEALING-QUERIES
…ard (sixteenth sweep) reconcileStaleMergerStatus clears a `merging`/`merging-pr` status left on a card that already reached a terminal lane. Two literal reads meant that on a renamed board it was never cleared — and unlike the rest of this series the damage is not confined to the stranded card: the stale status holds the MERGER QUEUE for every task behind it. ONE union read over TERMINAL_ROLES, not two buckets. Nothing here treats complete and archived differently — the only filter is on `status` — so splitting them would encode a distinction the code does not make. Deduped, since the two roles can share a column (the P1 on #2879). No per-card lane verdict: this sweep has no column comparison to convert, so adding one would be inventing a gate rather than resolving an existing one. Revert measured: with the literal reads restored the new case fails — the card is never listed, so its stale status is never cleared. Fusion-Task-Id: KB-SELF-HEALING-QUERIES
…ard (fourteenth sweep) recoverForeignOnlyContaminatedInReviewTasks classifies a branch carrying ONLY foreign commits and clears the contamination park nothing else clears. Two literal reads meant that on a renamed board it classified nothing and the task stayed parked indefinitely. The two `task.column === …` checks inside its filters were REDUNDANT while the query pinned the column. Under a resolved read they become the per-card verdict, so they convert here rather than being deleted — deleting them would have widened the sweep silently. Deduped across the buckets (the P1 reviewed on #2879). It matters more here than elsewhere: the two filters have DIFFERENT predicates, so a column carrying both a review role and the wip role could match both and classify one branch twice. Reverts measured, each run alone: - literal reads restored -> fails, the card is never listed - review verdict back to `task.column === "in-review"` -> fails, the renamed review lane does not match and the card is filtered out Fusion-Task-Id: KB-SELF-HEALING-QUERIES
…arried two roles #2883 review, two findings. DEDUPE (fixed). `readDependentBucket` dedupes by id inside ONE role's read, so a custom column carrying two queried roles is returned by two reads and the dependent was reconciled twice from the same stale snapshot — the second pass deciding against `blockedBy` state the first had already cleared. Same class as #2879 one sweep over. Order preserved, so `todoTaskIds` still classifies by the read that found it first. SATISFACTION BREADTH (recorded, not narrowed). The concern is that a mergeOrchestration-only lane reads as satisfying a dependency. Measured against what this replaced: the legacy set was done/in-review/archived, and `in-review` carries mergeOrchestration on every builtin — a dependency parked in review has ALWAYS released its dependents, so including the review roles reproduces the legacy contract exactly. Narrowing to terminal-only is a behaviour change in the DEADLOCK direction on a sweep whose purpose is unblocking, and it is a scheduler-contract question about when a dependency counts as done, not a vocabulary conversion. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…2879 Applying the review finding from #2879 to this sweep, which has the identical concatenation. The three literal reads were disjoint BY CONSTRUCTION (one column each), so a dependent could not appear twice. Resolved reads are not: a custom workflow may put more than one queried role flag on one column. Here the duplicate is a double WRITE — updateTask and logEntry both fire twice for one dependent, so the operator sees the release logged twice and blockedByCleared over-counts. Deduped with an explicit `has` guard rather than new Map(entries): that constructor keeps first insertion ORDER but the LAST value for a repeated key, so it reads as first-bucket precedence while doing the opposite. Revert measured: without the dedupe the new case fails with 2 clearing writes instead of 1. Fusion-Task-Id: KB-SELF-HEALING-QUERIES
… board (twelfth sweep)
reclaimSelfOwnedBranchConflicts frees a task whose OWN worktree is holding its OWN
branch hostage — a conflict no other sweep resolves. Three literal reads plus three
lane guards in the body, converted together: widening the read alone would admit
renamed-board cards and mis-decide every one, since the phantom-binding check, the
blocked-hold skip and the review triple-proof are each keyed on lane.
BUCKETS ARE BUILT FROM THE READ, NOT FROM task.column. First draft re-derived each
bucket by testing task.column against the resolved lanes, and 8 existing tests went
red. The cause was not the product: those fixture cards carry no column field at
all, so re-deriving dropped them. A row returned by listTasks({ column: X }) is by
definition in X — re-deriving adds nothing and silently drops rows. The per-card
lanes are kept for the three GUARDS, which is where the question is actually live.
DELIBERATELY UNCHANGED: the moveTask(task.id, "todo", { recoveryRehome: true })
re-home. That is one of the 22 documented deliberate escapes — moves.ts exempts
recoveryRehome so a card stranded in an undeclared column stays rescuable.
Reverts measured, each run alone:
- literal reads restored -> fails, the card is never listed
- wip guard back to task.column === "in-progress" -> fails, the renamed wip lane
does not match and the sweep falls through to the no-action path
Fusion-Task-Id: KB-SELF-HEALING-QUERIES
…e same stale snapshot #2879 review. `readBucket` dedupes by id inside ONE role's read. A custom workflow may put more than one queried flag on a single column — `hold` plus `countsTowardWip` on a lane that both parks and counts as work, or a review role beside either — and that column is returned by two reads. The task landed in two buckets and the concatenation handed the recovery loop the SAME snapshot twice. Not a wasted iteration: the second pass re-reads branch/worktree from state captured before the first pass mutated anything, so an already-reclaimed worktree is reclaimed again against state that no longer exists. The lane-resolution loop above already guards with `reclaimLanes.has(task.id)`; the consumption side was missing the same guard. Deduping by id preserves order, so role precedence is unchanged. SHIPS UNCOVERED, AND I DELETED THE TEST THAT PRETENDED OTHERWISE. My case reported 0 recoveries: the card it built reaches the BRANCH-LEVEL scan (subsumed branch, no worktree), not the candidates loop the dedupe lives in. Reaching that loop needs branch AND worktree set plus matching git state — the git fixture this suite avoids. A test that passes without exercising the loop is worse than none. What would cover it is recorded at the site. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…dence claim Two follow-ups to the P1 on #2879, both found by running the revert. 1. THE DEDUPE HAD NO TEST. Added one: a board whose hold column also carries the wip trait, so one column is returned by two role reads. Without the dedupe the loop runs twice on the same stale snapshot (measured: 2 calls, expected 1). 2. THE COMMENT CLAIMED THE WRONG PRECEDENCE. `new Map(entries)` keeps first INSERTION ORDER but the LAST value for a repeated key, so the code gave last-bucket precedence while the comment said first-bucket. Replaced with an explicit `has` guard so the code does what the comment says. The test was vacuous on its first run and passed with the dedupe reverted. The stub returned null and the sweep reads `.phantom` straight off it, so it THREW and aborted the loop after one card — capping the count at 1 in both states. Seventh vacuous assertion on this branch; seventh caught by the revert rather than by reading. Stub now returns the real `{ phantom, metadata }` shape. Revert measured: without the dedupe the new case fails with 2 calls instead of 1. Fusion-Task-Id: KB-SELF-HEALING-QUERIES
…oard (fourteenth sweep) (#2891) `recoverForeignOnlyContaminatedInReviewTasks` classifies a branch carrying **only foreign commits** and clears the contamination park that nothing else clears. Two literal reads meant that on a renamed board it classified nothing, and the task stayed parked indefinitely. ## The two redundant guards were the interesting part Both filters carried a `task.column === …` check that was **redundant** while the query pinned the column. Under a resolved read they stop being redundant and become the per-card verdict — so they convert here rather than being deleted. Deleting them would have silently widened the sweep, which is the failure this whole class is about. ## Dedupe matters more here than elsewhere The concatenated candidate list is deduped (the P1 reviewed on #2879). It bites harder in this sweep because the two filters have **different predicates**: a column carrying both a review role and the wip role could satisfy both and classify one branch twice. Explicit `has` guard rather than `new Map(entries)` — that constructor keeps first insertion *order* but the **last** value for a repeated key, so it reads as first-bucket precedence while doing the opposite. (Corrected in #2879 and #2883 for the same reason.) ## Revert results Each applied alone and the file re-run: | conversion | reverted → | | --- | --- | | the resolved reads | fails — the card is never listed, so the classifier is never called | | the review verdict | fails — the renamed review lane does not match and the card is filtered out | Observable is **candidacy**: `classifyForeignOnlyContamination` runs once per accepted card and not at all for a rejected one, which is exactly the read-plus-verdict under test. It is a static named import, so it is intercepted with a scoped `vi.mock` (spyOn cannot rebind an already-resolved ESM binding); only that one export is overridden, so the sweeps in this file that use `inspectBranchConflict` are unaffected. A non-vacuous companion (same card in the board's hold lane → never classified) rules out a read that returns everything. ## Verification `pnpm test:gate` 161 + 487 + 13 + 71, plus `self-healing.test.ts` 412; `tsc` engine clean; `pnpm lint`, `check:changesets`, census `--strict` clean, each run explicitly. --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
… plus a ratchet SELF-AUDIT triggered by #2916, where the behavioural test caught a SECOND lane guard on a re-read row. I scanned the other sweeps I had converted and found five more in this one, all inside the loop body under the widened read. One of them matters a great deal: `if (task.column === "in-review")` decides whether a backward move needs its triple-proof. Left literal, a renamed review card admitted by the widened read reads as NOT-in-review, so the proof gate is skipped and the card is moved back without it — a safety check silently bypassed BY the conversion. The other four gate the resume-limbo path the same way. MEASURED: self-healing.ts column guards 86 -> 81. Adds a ratchet for the class, because a scan found it and a scan can keep finding it. It DERIVES the sweep list (a sweep is converted when its body calls resolveProjectColumnsForRoles) rather than hardcoding one, so it is correct on every branch and cannot go stale. Three refinements it needed, each from a real false positive: - strip comments: the FNXC notes quote the old form to explain its removal - excuse the FALLBACK arm of a resolved ternary (`own.length > 0 ? own.includes(c) : c === "in-review"`) — the correct shape, and the first version reported all three such lines as defects - allow the one documented literal in clearStaleBlockedBy's log-dedup closure Two positive controls, because both halves fail silently: a broken regex finds no offenders, and a broken derivation iterates nothing — an empty for-loop registers no tests and reads as green. Revert measured: restoring any one of the five fails the ratchet, naming the sweep and the exact line. Fusion-Task-Id: KB-SELF-HEALING-QUERIES
…enty-sixth sweep) recoverBranchMisboundInReviewTasks detects a review card whose BRANCH TIP is bound to a different task's work. The literal read meant that on a renamed board the misbinding was never detected, so the card would merge — or refuse to — against a branch that is not its own. No second pair; verified with the derived ratchet from #2879 rather than by eye. Reverts measured, each alone: - literal read restored -> fails, the card is never listed - verdict back to `task.column === "in-review"` -> fails, the renamed review lane is filtered out Fusion-Task-Id: KB-SELF-HEALING-QUERIES
…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
… board (twenty-fourth sweep) recoverStaleIncompleteReviewTasks requeues a review card whose STEPS are not finished — it reached review on a graph failure, not on completed work. The literal read meant that on a renamed board it was never requeued, so the card sat in review claiming to be done while its own steps said otherwise. Checked for a second pair deliberately: the triple-proof here is NOT lane-gated, so unlike #2916 (a second guard on a re-read row) and #2879 (five in the loop body) there is nothing else to convert. The audit ratchet added in #2879 confirms it. The requeue keeps its literal `todo` — recoveryRehome: true, one of the 22 documented escapes. Reverts measured, each alone: - literal read restored -> fails, the card is never listed - verdict back to `task.column === "in-review"` -> fails, the renamed review lane is filtered out Fusion-Task-Id: KB-SELF-HEALING-QUERIES
|
Superseded by #2944 — folded into The conversion and its revert measurements are carried over in the commit message. Nothing here is dropped. |
…e same stale snapshot #2879 review. `readBucket` dedupes by id inside ONE role's read. A custom workflow may put more than one queried flag on a single column — `hold` plus `countsTowardWip` on a lane that both parks and counts as work, or a review role beside either — and that column is returned by two reads. The task landed in two buckets and the concatenation handed the recovery loop the SAME snapshot twice. Not a wasted iteration: the second pass re-reads branch/worktree from state captured before the first pass mutated anything, so an already-reclaimed worktree is reclaimed again against state that no longer exists. The lane-resolution loop above already guards with `reclaimLanes.has(task.id)`; the consumption side was missing the same guard. Deduping by id preserves order, so role precedence is unchanged. SHIPS UNCOVERED, AND I DELETED THE TEST THAT PRETENDED OTHERWISE. My case reported 0 recoveries: the card it built reaches the BRANCH-LEVEL scan (subsumed branch, no worktree), not the candidates loop the dedupe lives in. Reaching that loop needs branch AND worktree set plus matching git state — the git fixture this suite avoids. A test that passes without exercising the loop is worse than none. What would cover it is recorded at the site. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…dence claim Two follow-ups to the P1 on #2879, both found by running the revert. 1. THE DEDUPE HAD NO TEST. Added one: a board whose hold column also carries the wip trait, so one column is returned by two role reads. Without the dedupe the loop runs twice on the same stale snapshot (measured: 2 calls, expected 1). 2. THE COMMENT CLAIMED THE WRONG PRECEDENCE. `new Map(entries)` keeps first INSERTION ORDER but the LAST value for a repeated key, so the code gave last-bucket precedence while the comment said first-bucket. Replaced with an explicit `has` guard so the code does what the comment says. The test was vacuous on its first run and passed with the dedupe reverted. The stub returned null and the sweep reads `.phantom` straight off it, so it THREW and aborted the loop after one card — capping the count at 1 in both states. Seventh vacuous assertion on this branch; seventh caught by the revert rather than by reading. Stub now returns the real `{ phantom, metadata }` shape. Revert measured: without the dedupe the new case fails with 2 calls instead of 1. 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
…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>
…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
reclaimSelfOwnedBranchConflictsfrees a task whose own worktree is holding its own branch hostage — a conflict no other sweep resolves. Three literal reads, plus three lane guards in the body, so both halves convert together: widening the read alone would admit renamed-board cards and then mis-decide every one, since the phantom-binding check, the blocked-hold skip and the review triple-proof are each keyed on lane.The bug my first draft introduced, and what it taught
I bucketed each card by testing
task.columnagainst its resolved lanes. Eight existing tests inself-healing.test.tswent red.The cause was not the product: those fixture cards carry no
columnfield at all, so re-deriving the bucket dropped them. The fix is the better formulation anyway — a row returned bylistTasks({ column: X })is by definition in X, so re-deriving fromtask.columnadds nothing and can only lose rows. Buckets now come from the read that produced them; the per-card lanes are kept for the three guards, which is where the lane question is actually live.Worth stating plainly because the tempting move was the wrong one: eight failures in a file I had not touched looked like fixture staleness, and adjusting them would have buried a real formulation error under a fixture edit.
Deliberately unchanged
moveTask(task.id, "todo", { recoveryRehome: true })keeps its legacy target. It is one of the 22 documented deliberate escapes —moves.tsexemptsrecoveryRehomeprecisely so a card stranded in an undeclared column stays rescuable. Converting it removes the rescue path.Revert results
Each applied alone and the file re-run:
One assertion covers both, because
isPhantomExecutorBindingruns only for a card the read found and the wip guard accepted. A non-vacuous companion (same card in the review lane → not called) rules out a guard that matches everything.Reached through two private seams rather than a git/fs fixture — the technique recorded in the sibling doc after #2867.
Verification
pnpm test:gate161 + 487 + 13 + 71;self-healing.test.ts+ the blindness file 428 passed;tscengine clean;pnpm lint,check:changesets, census--strictclean, each run explicitly.