Conversation
|
Warning Review limit reached
Next review available in: 23 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 (4)
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 SummaryThe PR makes stale dependency cleanup workflow-lane-aware.
Confidence Score: 4/5The PR is not yet safe to merge because renamed V2 workflows with empty lifecycle traits still leave blocked cards invisible to stale dependency cleanup. The new candidate census depends entirely on project-level role resolution, which includes legacy IDs and explicitly traited columns but not renamed columns with empty traits; because per-task classification occurs only after that census, affected cards remain permanently blocked even after their dependency finishes. Files Needing Attention: packages/engine/src/self-healing.ts
|
| Filename | Overview |
|---|---|
| packages/engine/src/self-healing.ts | Converts stale-block candidate discovery to role-resolved reads and per-task classification, but the previously reported traitless-workflow omission remains outstanding. |
| packages/engine/src/tests/self-healing-query-filter-blindness.test.ts | Adds positive and negative renamed-workflow coverage for clearing stale blockers without clearing live ones. |
| .changeset/self-healing-stale-blockedby-query.md | Adds the required patch changeset with operator-facing fix metadata. |
| scripts/lib/lifecycle-column-census-baseline.json | Lowers the self-healing literal lifecycle-query baseline to reflect the three converted reads. |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart TD
Roles[Hold, WIP, and review roles] --> Resolve[Resolve project column IDs]
Resolve --> Read[List tasks in resolved columns]
Read --> Dedupe[Deduplicate blocked candidates]
Dedupe --> PerTask[Resolve each candidate's workflow]
PerTask --> Bucket[Classify into hold, WIP, or review bucket]
Bucket --> Cleanup[Evaluate blocker state and clear stale blockedBy]
Reviews (2): Last reviewed commit: "docs(self-healing): record the traitless..." | Re-trigger Greptile
|
Same finding as #2869, same root cause, and it is now blocking two PRs — so here is a concrete design rather than another deferral. The per-task fallback in these sweeps is correct and cannot help, because it never runs. This is the three-state rule at project scope — the one place the program has not applied it. Inside the merge queue (#2819) the fix was Why a blanket fix to the helper would be wrong, which is the part worth settling before someone writes it: the safe answer differs by caller.
So it has to be opt-in per caller, not a change to what the helper returns by default — something like Also correcting the premise, because it points at the wrong fixture: this is a hand-authored v2 board, not a v1 upgrade. I am not implementing it inside a review response: it touches every sweep, both analytics aggregators and the glasses notifier, and the per-caller opt-in is a contract decision. Flagging with the design so whoever picks it up is not re-deriving it a third time. |
…er than leaving it silent #2876 review, confirmed. `resolveProjectColumnsForRoles` returns its legacy floor plus what workflows DECLARE for the role, so a board that renames its lanes but declares no lifecycle traits contributes nothing — the card never enters `blockedCandidates` and the correct per-card classification below never runs for it. Invisible before the guard is reached. DEFERRED, not ignored, and a blanket fix would be wrong: the safe direction differs by caller. For a sweep, over-inclusion costs extra listTasks calls the per-card check discards; for the analytics aggregators the same widening inflates a number an operator reads. It needs an opt-in on the shared helper, which touches every sweep, both aggregators and the notifier. Premise corrected for whoever writes the fixture: this is a hand-authored V2 board, not a v1 upgrade. synthesizeDefaultColumns emits the DEFAULT ids with traits:[], so a v1-shaped fixture passes vacuously. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
Confirmed and recorded at the site (d010bae), deferred with the design stated — resolving on that basis.
This is the three-state rule at project scope, the one place the program has not applied it. The merge queue got the per-task version in #2819. A blanket fix to the helper would be wrong, which is why this is a note rather than a patch: the safe direction differs by caller. For a sweep, over-inclusion costs extra Premise correction for whoever writes the fixture: this is a hand-authored v2 board, not a v1 upgrade. Same finding is open on #2869; I have cross-referenced both so they are not fixed differently. Engine |
…ep too #2869 review, same class as #2876. A board that renames its review lane but declares no lifecycle traits contributes nothing to the project union, so the card is not in `wedgeById` and the per-card fallback below — including its own legacy degraded answer — never runs for it. A fallback cannot rescue a card the query never returned. Deferred for the reason recorded on the sibling sweep: the safe direction differs by caller, so the shared helper needs an opt-in rather than a changed default. Fixture note included because the reported premise names v1, and a v1-upgraded board cannot express a renamed lane at all. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…ep too #2869 review, same class as #2876. A board that renames its review lane but declares no lifecycle traits contributes nothing to the project union, so the card is not in `wedgeById` and the per-card fallback below — including its own legacy degraded answer — never runs for it. A fallback cannot rescue a card the query never returned. Deferred for the reason recorded on the sibling sweep: the safe direction differs by caller, so the shared helper needs an opt-in rather than a changed default. Fixture note included because the reported premise names v1, and a v1-upgraded board cannot express a renamed lane at all. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…er than leaving it silent #2876 review, confirmed. `resolveProjectColumnsForRoles` returns its legacy floor plus what workflows DECLARE for the role, so a board that renames its lanes but declares no lifecycle traits contributes nothing — the card never enters `blockedCandidates` and the correct per-card classification below never runs for it. Invisible before the guard is reached. DEFERRED, not ignored, and a blanket fix would be wrong: the safe direction differs by caller. For a sweep, over-inclusion costs extra listTasks calls the per-card check discards; for the analytics aggregators the same widening inflates a number an operator reads. It needs an opt-in on the shared helper, which touches every sweep, both aggregators and the notifier. Premise corrected for whoever writes the fixture: this is a hand-authored V2 board, not a v1 upgrade. synthesizeDefaultColumns emits the DEFAULT ids with traits:[], so a v1-shaped fixture passes vacuously. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…th sweep) clearStaleBlockedBy unsticks a card still pointing at a blocker that has since finished. Its BODY was already fully lane-resolved — per-referenced-task lanes, a shared IR cache, legacy ids unioned — and none of it ran, because the three reads above it asked for the literal todo/in-progress/in-review. The clearest instance yet of what this class costs: the expensive half was converted, the count went down, and an operator saw no change because the cheap half above it stayed literal. THREE reads, not one union. The buckets are treated differently downstream (hold cards seed the queued-dependency pass; review cards are exempted when paused), so the union read is followed by a per-card classification against its own workflow. SCOPE HELD: the hold read resolves role `hold` only, not `intake`. The original asked for `todo`; adding intake would newly scan `triage` cards, which is a behavior change riding along in a conversion. Legacy ids are UNIONED into each bucket rather than compared, mirroring lanesOf below — the first draft used `|| task.column === "todo"` and the census correctly scored three new guards for it. MEASURED: self-healing.ts census allowance 37 -> 34; baseline rewritten downward and committed. Revert measured: with the three literal reads restored, the new case fails — the blocked card is never listed, so its stale blockedBy is never cleared. Fusion-Task-Id: KB-SELF-HEALING-QUERIES
…er than leaving it silent #2876 review, confirmed. `resolveProjectColumnsForRoles` returns its legacy floor plus what workflows DECLARE for the role, so a board that renames its lanes but declares no lifecycle traits contributes nothing — the card never enters `blockedCandidates` and the correct per-card classification below never runs for it. Invisible before the guard is reached. DEFERRED, not ignored, and a blanket fix would be wrong: the safe direction differs by caller. For a sweep, over-inclusion costs extra listTasks calls the per-card check discards; for the analytics aggregators the same widening inflates a number an operator reads. It needs an opt-in on the shared helper, which touches every sweep, both aggregators and the notifier. Premise corrected for whoever writes the fixture: this is a hand-authored V2 board, not a v1 upgrade. synthesizeDefaultColumns emits the DEFAULT ids with traits:[], so a v1-shaped fixture passes vacuously. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…ep too #2869 review, same class as #2876. A board that renames its review lane but declares no lifecycle traits contributes nothing to the project union, so the card is not in `wedgeById` and the per-card fallback below — including its own legacy degraded answer — never runs for it. A fallback cannot rescue a card the query never returned. Deferred for the reason recorded on the sibling sweep: the safe direction differs by caller, so the shared helper needs an opt-in rather than a changed default. Fixture note included because the reported premise names v1, and a v1-upgraded board cannot express a renamed lane at all. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
Superseded by #2944 — folded into The conversion and its revert measurements are carried over in the commit message. Nothing here is dropped. |
…er than leaving it silent #2876 review, confirmed. `resolveProjectColumnsForRoles` returns its legacy floor plus what workflows DECLARE for the role, so a board that renames its lanes but declares no lifecycle traits contributes nothing — the card never enters `blockedCandidates` and the correct per-card classification below never runs for it. Invisible before the guard is reached. DEFERRED, not ignored, and a blanket fix would be wrong: the safe direction differs by caller. For a sweep, over-inclusion costs extra listTasks calls the per-card check discards; for the analytics aggregators the same widening inflates a number an operator reads. It needs an opt-in on the shared helper, which touches every sweep, both aggregators and the notifier. Premise corrected for whoever writes the fixture: this is a hand-authored V2 board, not a v1 upgrade. synthesizeDefaultColumns emits the DEFAULT ids with traits:[], so a v1-shaped fixture passes vacuously. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…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>
…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>
clearStaleBlockedByunsticks a card still pointing at a blocker that has since finished. Its body was already fully lane-resolved — per-referenced-task lanes, a shared IR cache, legacy ids unioned, the lot — and none of it ran, because the three reads above it asked for the literaltodo/in-progress/in-review.This is the clearest instance yet of what the query-filter class costs: the expensive half was converted, the census count went down, and an operator saw no change, because the cheap half above it stayed literal. The card sat blocked behind a dependency that finished days earlier.
Three reads, not one union
The buckets are treated differently downstream — hold cards seed the queued-dependency pass, review cards are exempted when paused — so this cannot collapse into a single union read. The union read is followed by a per-card classification against each card's own workflow.
Scope held deliberately
The hold read resolves role
holdonly, notintake. The original asked fortodo; addingintakewould newly scantriagecards, which is a behavior change riding along inside a conversion.The census caught my first draft
I wrote the legacy fallbacks as
|| task.column === "todo"and the census scored three new column guards for it (89 → 92). It was right to: those are exactly the comparisons this program exists to remove. Rewritten to union the legacy ids into each bucket, mirroring thelanesOfhelper twenty lines below that already did it that way.Measured:
self-healing.tscensus allowance 37 → 34; baseline rewritten downward and committed.Revert result
blockedByis never clearedA non-vacuous companion is included: same board, same pair, blocker still in the wip lane → nothing is cleared. Without it, a sweep that cleared every
blockedByit found would satisfy the positive case.Verification
pnpm test:gate161 + 487 + 13 + 71, plusself-healing.test.ts412;tscengine clean;pnpm lint,check:changesets, census--strictclean, each run explicitly.