fleet: async-comments-attachments.ts NOT converted — the archived gate lives in three encodings (52 sites), pinned by a guard that fails on each - #2724
Conversation
|
Warning Review limit reached
Next review available in: 3 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 (1)
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 SummaryAdds a repository-wide test inventory for the TypeScript, Drizzle, and raw-SQL encodings of archived-column checks. Confidence Score: 4/5The PR is not yet safe to merge because the archived-column parity test remains outside the merge gate and can be skipped when production comparisons change. The new test discovers production sites only while it is running, but the curated unit gate does not include it; core source changes delegated to that gate therefore do not reliably execute the intended parity check. Files Needing Attention: packages/core/src/tests/archived-column-gate-parity.test.ts; packages/core/package.json
|
| Filename | Overview |
|---|---|
| packages/core/src/tests/archived-column-gate-parity.test.ts | Adds the archived-column parity inventories and scans all tracked core TypeScript sources for the three encodings. |
Reviews (3): Last reviewed commit: "fix(core): #2724 review — accept element..." | Re-trigger Greptile
…f-conversion I shipped Both greptile P1s on #2744 were real. 1. STALE FLAGS ACROSS TASKS. I passed `workflowMoveMetadata?.currentColumnFlags` raw to TaskReviewTab. The file already has `detailColumnFlags`, which applies `detailFlagsAreForThisTask` (`workflowMoveMetadata?.taskId === task.id`) — because on the render where the modal switches tasks the state still holds the PREVIOUS card's payload. Passing the raw value resolves the review tab's roles from another task's workflow: confidently wrong rather than merely stale. That reasoning was already written six lines above my own change and I reviewed past it. 2. A HALF-CONVERSION. `canStartPrFeedbackAddressing` in utils/prFeedback.ts carries the SAME lane pair the tab asks about, and I converted only the caller. Consequence: on a renamed review or WIP lane, a task with actionable PR feedback but NO loaded display items kept the Address-PR-Feedback action hidden — the caller's role check passed and the helper returned false. Now takes optional flags on the same seam, with both callers (TaskReviewTab, TaskCard) supplying what they already hold. This is the third instance in this program of "both halves or neither": the archived gate (#2724), the tracking pair (#2715), and now the lane pair split between a component and its util. Converting the visible half first is what makes the remaining half invisible. REVERT PROOF: restoring the id pair in `canStartPrFeedbackAddressing` fails the new case. The case deliberately supplies EMPTY review items — with items present the `displayItems.length > 0` arm masks the helper and the bug cannot be seen. Two fixture details the first attempt got wrong and are now recorded in the test: the field read is `lastReviewDecision` (not `reviewDecision`), and `isPrMode` needs review source `pull-request`. Census: prFeedback.ts 2 -> 0 (re-recorded via --update-baseline, which reported exactly "TIGHTENED 1 entry"). Gate 158+10+487+71 GREEN. 463 passed across the review-tab, card, sorting and columnRoles suites; the 2 TaskCard failures are the known pre-existing CSS-var geometry assertions. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…ync-filter class Fleet cluster: packages/dashboard/src/github-tracking-reconciler.ts, and the reference implementation for the `.filter((task) => task.column === "<id>")` shape I flagged across four files while waiting on a decision that was mine to make. THE DECISION: prefetch a resolved map, then filter synchronously. The alternative — async predicates — forces every caller into `for await` and turns a list comprehension into a sequential walk. Prefetching keeps the filters sync, puts the awaits in one bounded place, and lets the IR cache do the job it was built for: "a self-healing pass over 400 cards spanning three workflows must read three IRs, not 400". The cache is instance-scoped and shared across all four passes, so each distinct workflow's IR is read once for the whole run. `resolveLifecycleColumns` is pure and not memoized by that cache, so this still costs one cheap struct build per task — fine in a background reconcile, and stated rather than hidden. WHAT IT COST BEFORE: on a board with renamed terminal lanes every filter matched nothing, so the reconciler closed NO GitHub issues and reported `scanned: 0` — a clean-looking pass that did nothing. WHY THIS IS NOT THE SPLIT BRAIN #2724 DOCUMENTS, checked rather than assumed. That guard covers the archived gate in packages/core, where the same question is answered in TypeScript AND SQL. This file contains zero SQL (measured: no drizzle, no `sql` template, no eq/ne) and calls `listTasks({ includeArchived: true })` — the SQL half has already been told to include archived rows, so this filter SELECTS among rows it was handed rather than deciding liveness a second time. Gate versus consumer. The fourth pass needed its own check: its list comes from `listTasksForGithubTrackingReconcile`, which IS SQL — but that impl filters on `deletedAt IS NOT NULL` and `githubTracking IS NOT NULL`, never on the column, so there is no SQL-side encoding of this question to diverge from. REACHABILITY FINDING, recorded not acted on: in backend mode that pass returns only soft-deleted rows (its own comment says the archived fallback is a separate AsyncArchiveLineage subsystem, skipped there), and `task.deletedAt` is tested first in the stateReason chain — so its archived arm is effectively unreachable today. Converted rather than deleted; whether that fallback should be wired is a separate question from what vocabulary it speaks. REVERT PROOF, both run: restoring the id comparisons fails "closes issues on a RENAMED complete lane" and the renamed archived-heuristic case, each with no setIssueState call. Worth noting why the 33 existing cases stayed green through the conversion: their fake store has no workflow reader, so `resolveTaskLifecycleColumns` returns undefined and they assert the legacy fallback — exactly what they always asserted. None of them could have caught this being wrong. `workflowIr` is now an opt-in on that fake. Census 9 -> 0. Gate 158+10+487+71 GREEN. 35 passed across the three reconciler suites. Dashboard tsc and lint clean; --strict exits 0. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…s told there was none Second application of the sync-filter pattern decided in #2737: prefetch a resolved map with the caller-owned IR cache, keep every predicate synchronous. THE FAILURE. `selectNextTaskForAgentImpl` picks an agent's next task by filtering the board for its WIP lane, then its hold lane, both on `task.column === "<literal>"`. On a renamed board both filters match nothing, so an agent asking for work is told there is none — with its own assigned tasks sitting in the list it just fetched. No error, no log; the agent idles. `pauseTaskImpl` had the same shape: pausing a running card on a renamed board left its status untouched, so the UI kept showing it as working. CONSUMER, NOT A GATE — the #2724 check run rather than assumed. This file's only SQL predicate is `eq(table.projectId, ...)`; nothing here compares a column to a literal in SQL, so there is no second encoding to diverge from. The list arrives from `store.listTasks` and these filters select among rows already in hand. Async predicates were the alternative and would have turned these filter chains into sequential awaits inside the dispatch path. One prefetch, one IR read per distinct workflow, filters stay sync. `pauseTaskImpl` resolves for the single task it holds rather than joining the map — it is a different entry point with one id in scope, and a map would be one entry. REVERT PROOF, both run: restoring the wip literal fails the renamed WIP case with `expected null to be truthy`; restoring the hold literal fails the renamed hold case identically. New test calls the impl directly with a store fake resolving a renamed IR. The existing `selectNextTaskForAgent` coverage drives a real store harness, so a renamed vocabulary there means registering a real custom workflow and moving cards through it — heavier than the question, which is only which lane the filters name. The bind evaluator runs for real; only the store is faked. A third case pins that the hold filter keeps its `userPaused` exclusion, so a filter matching every column would not satisfy the other two. Related coverage checked first: agent-heartbeat-worktree-renamed-hold.test.ts covers the requeue TARGET on a renamed board, not the dispatcher's SELECTION filters — which is why this is a new file rather than a case added there. Census 6 -> 0. Gate 158+10+487+71 GREEN. 20 passed across the routing-policy and new dispatch suites. Core tsc and lint clean; --strict exits 0. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
P1 (parity guard is not gated) — accurate observation, and I am deliberately not acting on it. The finding is factually right: AGENTS is explicit that this is the intended default:
and the gate is described as deliberately thin — "CI blocks PRs on exactly Lint, Typecheck, Build, and Gate". A guard written today, which has never caught a real regression, has no evidence of value yet. Adding it because it feels important is how the thin gate stops being thin, and the file's own history says the gate rotted that way before. The guard's value does not depend on gate membership either: it fails loudly in the full suite, and its failure message names the encoding that changed plus the two real options. Someone doing a partial conversion sees it before merge in review and after merge in the non-blocking run. If it ever fires on a real partial conversion, that is the evidence AGENTS asks for and it should be promoted then, with the incident cited. P2 (AST scan skips equivalent access forms) — correct, and worth a fix. The scan accepts a property access or a bare I will add |
…nd re-record two merged conversions
|
P2 fixed, and the fix produced a more interesting result than expected. Element access is now matched. The widening found no new sites — but the guard failed anyway, for a better reason. The inventory had drifted down: Both are conversions that merged since this guard was written — Header count corrected from 37 to 35 TypeScript comparisons (50 sites total across the three encodings), with the reason recorded inline so the next reader knows the number moved because work landed, not because the scan changed. The P1 (gate admission) reply stands: AGENTS says tests never graduate into the gate by default and this guard has not yet caught a real regression. When it does, that is the evidence for promotion. Verification
|
42bb025 to
005aa0b
Compare
…, not one async-comments-attachments.ts (9) is NOT converted, and this is the evidence for why. No production file changes; the census is unmoved. MEASURED in packages/core — one rule, three encodings, 52 sites: - 37 TypeScript comparisons against "archived", 23 files - 7 Drizzle `ne(tasks.column, 'archived')` predicates, 6 files - 8 raw `sql` template comparisons, 5 files (one a hand-written SELECT string) The SQL encodings decide which rows a query RETURNS; the TypeScript one decides what code does with a row it has. Convert only the TypeScript half and a board with a renamed archived lane splits: getLiveTaskColumn reports the task archived while readLiveTaskRows still returns it as live — a document write rejected by its gate while its parent is listed live. Every builtin workflow spells the column `archived`, so all three agree by accident on every board we ship and no existing test can see the divergence. The SQL sides need the resolved id as a query-build VALUE, including inside the `for update` document/artifact transactions that today receive a db/tx handle and no store. Threading a resolver into the persistence layer, or declaring `archived` a non-renameable system column, are both real decisions. Neither is a conversion. I WAS WRONG ABOUT THE SHAPE: the third encoding was written as an assertion that no raw template exists. It failed on the first run with five files. Nothing in the repo counted them — they are not comparisons (invisible to the census) and not eq/ne calls (invisible to the Drizzle scan). That case is now an inventory, with a note on why asserting absence was the wrong invariant. INJECTION PROOF, all three run: converting one TS comparison fails with "TypeScript encoding changed"; removing one Drizzle predicate fails with "Drizzle encoding changed"; removing one raw template fails with "Raw-sql encoding changed". Each carries the split-brain explanation and the two real options. Scan notes: comment-stripped via the census's shared stripComments — an earlier version audited three JSDoc prose lines as sites. The TS matcher accepts a bare local named `column`, not just a receiver: without it four of the eight sites in async-comments-attachments.ts are missed, measured. Gate 158+10+487+71 GREEN. Census --strict exits 0, baseline untouched. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…nd re-record two merged conversions
005aa0b to
fd7f13b
Compare
…s told there was none Second application of the sync-filter pattern decided in #2737: prefetch a resolved map with the caller-owned IR cache, keep every predicate synchronous. THE FAILURE. `selectNextTaskForAgentImpl` picks an agent's next task by filtering the board for its WIP lane, then its hold lane, both on `task.column === "<literal>"`. On a renamed board both filters match nothing, so an agent asking for work is told there is none — with its own assigned tasks sitting in the list it just fetched. No error, no log; the agent idles. `pauseTaskImpl` had the same shape: pausing a running card on a renamed board left its status untouched, so the UI kept showing it as working. CONSUMER, NOT A GATE — the #2724 check run rather than assumed. This file's only SQL predicate is `eq(table.projectId, ...)`; nothing here compares a column to a literal in SQL, so there is no second encoding to diverge from. The list arrives from `store.listTasks` and these filters select among rows already in hand. Async predicates were the alternative and would have turned these filter chains into sequential awaits inside the dispatch path. One prefetch, one IR read per distinct workflow, filters stay sync. `pauseTaskImpl` resolves for the single task it holds rather than joining the map — it is a different entry point with one id in scope, and a map would be one entry. REVERT PROOF, both run: restoring the wip literal fails the renamed WIP case with `expected null to be truthy`; restoring the hold literal fails the renamed hold case identically. New test calls the impl directly with a store fake resolving a renamed IR. The existing `selectNextTaskForAgent` coverage drives a real store harness, so a renamed vocabulary there means registering a real custom workflow and moving cards through it — heavier than the question, which is only which lane the filters name. The bind evaluator runs for real; only the store is faked. A third case pins that the hold filter keeps its `userPaused` exclusion, so a filter matching every column would not satisfy the other two. Related coverage checked first: agent-heartbeat-worktree-renamed-hold.test.ts covers the requeue TARGET on a renamed board, not the dispatcher's SELECTION filters — which is why this is a new file rather than a case added there. Census 6 -> 0. Gate 158+10+487+71 GREEN. 20 passed across the routing-policy and new dispatch suites. Core tsc and lint clean; --strict exits 0. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…ync-filter class Fleet cluster: packages/dashboard/src/github-tracking-reconciler.ts, and the reference implementation for the `.filter((task) => task.column === "<id>")` shape I flagged across four files while waiting on a decision that was mine to make. THE DECISION: prefetch a resolved map, then filter synchronously. The alternative — async predicates — forces every caller into `for await` and turns a list comprehension into a sequential walk. Prefetching keeps the filters sync, puts the awaits in one bounded place, and lets the IR cache do the job it was built for: "a self-healing pass over 400 cards spanning three workflows must read three IRs, not 400". The cache is instance-scoped and shared across all four passes, so each distinct workflow's IR is read once for the whole run. `resolveLifecycleColumns` is pure and not memoized by that cache, so this still costs one cheap struct build per task — fine in a background reconcile, and stated rather than hidden. WHAT IT COST BEFORE: on a board with renamed terminal lanes every filter matched nothing, so the reconciler closed NO GitHub issues and reported `scanned: 0` — a clean-looking pass that did nothing. WHY THIS IS NOT THE SPLIT BRAIN #2724 DOCUMENTS, checked rather than assumed. That guard covers the archived gate in packages/core, where the same question is answered in TypeScript AND SQL. This file contains zero SQL (measured: no drizzle, no `sql` template, no eq/ne) and calls `listTasks({ includeArchived: true })` — the SQL half has already been told to include archived rows, so this filter SELECTS among rows it was handed rather than deciding liveness a second time. Gate versus consumer. The fourth pass needed its own check: its list comes from `listTasksForGithubTrackingReconcile`, which IS SQL — but that impl filters on `deletedAt IS NOT NULL` and `githubTracking IS NOT NULL`, never on the column, so there is no SQL-side encoding of this question to diverge from. REACHABILITY FINDING, recorded not acted on: in backend mode that pass returns only soft-deleted rows (its own comment says the archived fallback is a separate AsyncArchiveLineage subsystem, skipped there), and `task.deletedAt` is tested first in the stateReason chain — so its archived arm is effectively unreachable today. Converted rather than deleted; whether that fallback should be wired is a separate question from what vocabulary it speaks. REVERT PROOF, both run: restoring the id comparisons fails "closes issues on a RENAMED complete lane" and the renamed archived-heuristic case, each with no setIssueState call. Worth noting why the 33 existing cases stayed green through the conversion: their fake store has no workflow reader, so `resolveTaskLifecycleColumns` returns undefined and they assert the legacy fallback — exactly what they always asserted. None of them could have caught this being wrong. `workflowIr` is now an opt-in on that fake. Census 9 -> 0. Gate 158+10+487+71 GREEN. 35 passed across the three reconciler suites. Dashboard tsc and lint clean; --strict exits 0. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…f-conversion I shipped Both greptile P1s on #2744 were real. 1. STALE FLAGS ACROSS TASKS. I passed `workflowMoveMetadata?.currentColumnFlags` raw to TaskReviewTab. The file already has `detailColumnFlags`, which applies `detailFlagsAreForThisTask` (`workflowMoveMetadata?.taskId === task.id`) — because on the render where the modal switches tasks the state still holds the PREVIOUS card's payload. Passing the raw value resolves the review tab's roles from another task's workflow: confidently wrong rather than merely stale. That reasoning was already written six lines above my own change and I reviewed past it. 2. A HALF-CONVERSION. `canStartPrFeedbackAddressing` in utils/prFeedback.ts carries the SAME lane pair the tab asks about, and I converted only the caller. Consequence: on a renamed review or WIP lane, a task with actionable PR feedback but NO loaded display items kept the Address-PR-Feedback action hidden — the caller's role check passed and the helper returned false. Now takes optional flags on the same seam, with both callers (TaskReviewTab, TaskCard) supplying what they already hold. This is the third instance in this program of "both halves or neither": the archived gate (#2724), the tracking pair (#2715), and now the lane pair split between a component and its util. Converting the visible half first is what makes the remaining half invisible. REVERT PROOF: restoring the id pair in `canStartPrFeedbackAddressing` fails the new case. The case deliberately supplies EMPTY review items — with items present the `displayItems.length > 0` arm masks the helper and the bug cannot be seen. Two fixture details the first attempt got wrong and are now recorded in the test: the field read is `lastReviewDecision` (not `reviewDecision`), and `isPrMode` needs review source `pull-request`. Census: prFeedback.ts 2 -> 0 (re-recorded via --update-baseline, which reported exactly "TIGHTENED 1 entry"). Gate 158+10+487+71 GREEN. 463 passed across the review-tab, card, sorting and columnRoles suites; the 2 TaskCard failures are the known pre-existing CSS-var geometry assertions. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…f-conversion I shipped Both greptile P1s on #2744 were real. 1. STALE FLAGS ACROSS TASKS. I passed `workflowMoveMetadata?.currentColumnFlags` raw to TaskReviewTab. The file already has `detailColumnFlags`, which applies `detailFlagsAreForThisTask` (`workflowMoveMetadata?.taskId === task.id`) — because on the render where the modal switches tasks the state still holds the PREVIOUS card's payload. Passing the raw value resolves the review tab's roles from another task's workflow: confidently wrong rather than merely stale. That reasoning was already written six lines above my own change and I reviewed past it. 2. A HALF-CONVERSION. `canStartPrFeedbackAddressing` in utils/prFeedback.ts carries the SAME lane pair the tab asks about, and I converted only the caller. Consequence: on a renamed review or WIP lane, a task with actionable PR feedback but NO loaded display items kept the Address-PR-Feedback action hidden — the caller's role check passed and the helper returned false. Now takes optional flags on the same seam, with both callers (TaskReviewTab, TaskCard) supplying what they already hold. This is the third instance in this program of "both halves or neither": the archived gate (#2724), the tracking pair (#2715), and now the lane pair split between a component and its util. Converting the visible half first is what makes the remaining half invisible. REVERT PROOF: restoring the id pair in `canStartPrFeedbackAddressing` fails the new case. The case deliberately supplies EMPTY review items — with items present the `displayItems.length > 0` arm masks the helper and the bug cannot be seen. Two fixture details the first attempt got wrong and are now recorded in the test: the field read is `lastReviewDecision` (not `reviewDecision`), and `isPrMode` needs review source `pull-request`. Census: prFeedback.ts 2 -> 0 (re-recorded via --update-baseline, which reported exactly "TIGHTENED 1 entry"). Gate 158+10+487+71 GREEN. 463 passed across the review-tab, card, sorting and columnRoles suites; the 2 TaskCard failures are the known pre-existing CSS-var geometry assertions. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…s told there was none Second application of the sync-filter pattern decided in #2737: prefetch a resolved map with the caller-owned IR cache, keep every predicate synchronous. THE FAILURE. `selectNextTaskForAgentImpl` picks an agent's next task by filtering the board for its WIP lane, then its hold lane, both on `task.column === "<literal>"`. On a renamed board both filters match nothing, so an agent asking for work is told there is none — with its own assigned tasks sitting in the list it just fetched. No error, no log; the agent idles. `pauseTaskImpl` had the same shape: pausing a running card on a renamed board left its status untouched, so the UI kept showing it as working. CONSUMER, NOT A GATE — the #2724 check run rather than assumed. This file's only SQL predicate is `eq(table.projectId, ...)`; nothing here compares a column to a literal in SQL, so there is no second encoding to diverge from. The list arrives from `store.listTasks` and these filters select among rows already in hand. Async predicates were the alternative and would have turned these filter chains into sequential awaits inside the dispatch path. One prefetch, one IR read per distinct workflow, filters stay sync. `pauseTaskImpl` resolves for the single task it holds rather than joining the map — it is a different entry point with one id in scope, and a map would be one entry. REVERT PROOF, both run: restoring the wip literal fails the renamed WIP case with `expected null to be truthy`; restoring the hold literal fails the renamed hold case identically. New test calls the impl directly with a store fake resolving a renamed IR. The existing `selectNextTaskForAgent` coverage drives a real store harness, so a renamed vocabulary there means registering a real custom workflow and moving cards through it — heavier than the question, which is only which lane the filters name. The bind evaluator runs for real; only the store is faked. A third case pins that the hold filter keeps its `userPaused` exclusion, so a filter matching every column would not satisfy the other two. Related coverage checked first: agent-heartbeat-worktree-renamed-hold.test.ts covers the requeue TARGET on a renamed board, not the dispatcher's SELECTION filters — which is why this is a new file rather than a case added there. Census 6 -> 0. Gate 158+10+487+71 GREEN. 20 passed across the routing-policy and new dispatch suites. Core tsc and lint clean; --strict exits 0. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…f-conversion I shipped Both greptile P1s on #2744 were real. 1. STALE FLAGS ACROSS TASKS. I passed `workflowMoveMetadata?.currentColumnFlags` raw to TaskReviewTab. The file already has `detailColumnFlags`, which applies `detailFlagsAreForThisTask` (`workflowMoveMetadata?.taskId === task.id`) — because on the render where the modal switches tasks the state still holds the PREVIOUS card's payload. Passing the raw value resolves the review tab's roles from another task's workflow: confidently wrong rather than merely stale. That reasoning was already written six lines above my own change and I reviewed past it. 2. A HALF-CONVERSION. `canStartPrFeedbackAddressing` in utils/prFeedback.ts carries the SAME lane pair the tab asks about, and I converted only the caller. Consequence: on a renamed review or WIP lane, a task with actionable PR feedback but NO loaded display items kept the Address-PR-Feedback action hidden — the caller's role check passed and the helper returned false. Now takes optional flags on the same seam, with both callers (TaskReviewTab, TaskCard) supplying what they already hold. This is the third instance in this program of "both halves or neither": the archived gate (#2724), the tracking pair (#2715), and now the lane pair split between a component and its util. Converting the visible half first is what makes the remaining half invisible. REVERT PROOF: restoring the id pair in `canStartPrFeedbackAddressing` fails the new case. The case deliberately supplies EMPTY review items — with items present the `displayItems.length > 0` arm masks the helper and the bug cannot be seen. Two fixture details the first attempt got wrong and are now recorded in the test: the field read is `lastReviewDecision` (not `reviewDecision`), and `isPrMode` needs review source `pull-request`. Census: prFeedback.ts 2 -> 0 (re-recorded via --update-baseline, which reported exactly "TIGHTENED 1 entry"). Gate 158+10+487+71 GREEN. 463 passed across the review-tab, card, sorting and columnRoles suites; the 2 TaskCard failures are the known pre-existing CSS-var geometry assertions. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…s told there was none Second application of the sync-filter pattern decided in #2737: prefetch a resolved map with the caller-owned IR cache, keep every predicate synchronous. THE FAILURE. `selectNextTaskForAgentImpl` picks an agent's next task by filtering the board for its WIP lane, then its hold lane, both on `task.column === "<literal>"`. On a renamed board both filters match nothing, so an agent asking for work is told there is none — with its own assigned tasks sitting in the list it just fetched. No error, no log; the agent idles. `pauseTaskImpl` had the same shape: pausing a running card on a renamed board left its status untouched, so the UI kept showing it as working. CONSUMER, NOT A GATE — the #2724 check run rather than assumed. This file's only SQL predicate is `eq(table.projectId, ...)`; nothing here compares a column to a literal in SQL, so there is no second encoding to diverge from. The list arrives from `store.listTasks` and these filters select among rows already in hand. Async predicates were the alternative and would have turned these filter chains into sequential awaits inside the dispatch path. One prefetch, one IR read per distinct workflow, filters stay sync. `pauseTaskImpl` resolves for the single task it holds rather than joining the map — it is a different entry point with one id in scope, and a map would be one entry. REVERT PROOF, both run: restoring the wip literal fails the renamed WIP case with `expected null to be truthy`; restoring the hold literal fails the renamed hold case identically. New test calls the impl directly with a store fake resolving a renamed IR. The existing `selectNextTaskForAgent` coverage drives a real store harness, so a renamed vocabulary there means registering a real custom workflow and moving cards through it — heavier than the question, which is only which lane the filters name. The bind evaluator runs for real; only the store is faked. A third case pins that the hold filter keeps its `userPaused` exclusion, so a filter matching every column would not satisfy the other two. Related coverage checked first: agent-heartbeat-worktree-renamed-hold.test.ts covers the requeue TARGET on a renamed board, not the dispatcher's SELECTION filters — which is why this is a new file rather than a case added there. Census 6 -> 0. Gate 158+10+487+71 GREEN. 20 passed across the routing-policy and new dispatch suites. Core tsc and lint clean; --strict exits 0. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…efect the third one exposed
All three review threads were correct, and all three were the same fault: a comment
claiming something the code did not do.
1. ONE IR CACHE, as the comment already claimed. The dispatch pass allocated
`dispatchIrCache` while the dependency pass allocated `satisfiedIrCache`, so a task
and its dependency in the same workflow resolved that IR twice — the comment asserted
an optimisation the code did not perform. Now a single `lifecycleIrCache` feeds both.
2. FNXC timestamps dated 2026-07-31 while the date is 2026-07-30. A recurrence: I
future-dated six of these earlier in this program and corrected them then.
3. THE BIND-EVALUATOR COVERAGE CLAIM WAS FALSE, and proving it found a real defect.
The test header said "the bind evaluator is exercised for real" while the call omitted
`selectNextTaskForAgentImpl`'s OPTIONAL `agent` argument, so `isBindCompatible` hit its
`if (!agent) return true` short-circuit and `evaluateImplementationTaskBind` never ran.
Passing an agent is not proof it runs — an executor is allowed, so a short-circuit and a
real evaluation look identical. So I asserted a `custom`-role agent is REFUSED an
implementation task on a renamed lane. IT FAILED, and the cause is production:
THE ROLE-ROUTING POLICY IS BYPASSED ENTIRELY ON A RENAMED BOARD.
`isImplementationTask` is Set membership over hardcoded {triage, todo, in-progress, ...}
and `evaluateImplementationTaskBind` short-circuits to `allowed: true` when it is false.
So on a renamed workflow every agent is bind-compatible with every task, and the check
that stops a liaison/custom agent being handed implementation work — the NEXT-871 loop
FN-7851 fixed — does not apply.
WHY THE CENSUS MISSED IT: these are Set MEMBERS, not comparisons. The census scans
`===`/`!==` against a column, so a literal collection is invisible — the same blind spot
that hid the raw-`sql` encoding of the archived gate (#2724). The backlog number is a
floor, not a total.
NOT FIXED HERE. `isImplementationTask` is a sync pure predicate with no store, called
from the gate for agent assignment; resolving a workflow inside it means threading a
resolver through the routing policy and making its callers async. Getting that wrong
either hands implementation work to a liaison or refuses it to a valid executor — a
behaviour change to agent admission, not a vocabulary conversion.
The case is written to CURRENT behaviour and named as documenting a defect, so it does
not sit red; flip the expectation when the policy resolves lanes by role.
Gate 158+10+487+71 GREEN. 21 passed across the dispatch and routing-policy suites. Core
tsc and lint clean; --strict exits 0.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…ass (prefetch a resolved map), and the reconciler closed NO issues on a renamed board (#2737) `github-tracking-reconciler.ts` 9 → **0**, and the reference implementation for the `.filter((task) => task.column === "<id>")` shape I have been flagging across four files. ## I stopped waiting and decided it I flagged this class in #2709, #2696, #2700 and #2715 as "needs one decision" and left ~25 sites unconverted. That decision was mine to make and I should have made it three PRs ago. **Prefetch a resolved map, then filter synchronously.** The alternative — async predicates — forces every caller into `for await` and turns a list comprehension into a sequential walk. Prefetching keeps the filters synchronous, puts the awaits in one bounded place, and lets the IR cache do the job it was explicitly built for: > "A self-healing pass over 400 cards spanning three workflows must read three IRs, not 400." The cache is **instance-scoped and shared across all four passes**, so each distinct workflow's IR is read once for the whole run rather than once per pass. `resolveLifecycleColumns` is pure and *not* memoized by that cache, so this still costs one cheap struct build per task — fine in a background reconcile, and stated rather than hidden. No new abstraction: `resolveTaskLifecycleColumns` already takes a caller-owned cache. The only new code is a local map builder and two named predicates. ## What it cost before On a board with renamed terminal lanes, **every filter here matched nothing**. The reconciler closed **no** GitHub issues and reported `scanned: 0` — a clean-looking pass that did nothing. ## Why this is not the split brain #2724 documents — checked, not assumed #2724 proves the archived gate in `packages/core` is enforced in three encodings, so converting one alone diverges them. I checked whether that applies here before converting: - This file contains **zero SQL** — measured: no drizzle, no `sql` template, no `eq`/`ne`. - It calls `listTasks({ includeArchived: true })`, so the SQL half has already been told to include archived rows. The filter **selects among rows it was handed** rather than deciding liveness a second time. **Gate versus consumer** is the distinction, and a consumer can be converted alone. The fourth pass needed its own check because its list comes from `listTasksForGithubTrackingReconcile`, which *is* SQL — but that impl filters on `deletedAt IS NOT NULL` and `githubTracking IS NOT NULL`, **never on the column**, so there is no SQL-side encoding of this question to diverge from. ## Why the 33 existing tests stayed green through the conversion Their fake store has **no workflow reader**, so `resolveTaskLifecycleColumns` catches and returns `undefined` and every case asserts the legacy fallback — exactly what it always asserted. **None of them could have caught this being wrong.** `workflowIr` is now an opt-in on that fake, which is what makes the new cases real tests rather than restatements. | reverted | result | |---|---| | terminal filter back to the ids | "closes issues on a RENAMED complete lane" fails, no `setIssueState` | | same | renamed archived-heuristic case fails, no `setIssueState` | ## A reachability finding, recorded not acted on In backend mode `reconcileDeletedAndArchived` returns only **soft-deleted** rows — its own comment says the archived-tasks fallback is a separate `AsyncArchiveLineage` subsystem, skipped there — and `task.deletedAt` is tested *first* in the `stateReason` chain. So its archived arm is **effectively unreachable today**. I converted it rather than deleting it: it is the documented FN-5577 done-heuristic, and whether that fallback should be wired here is a separate question from what vocabulary it speaks. ## Verification `pnpm test:gate` **GREEN** (158 + 10 + 487 + 71) · **35 passed** across the three reconciler suites · dashboard `tsc` clean · `pnpm lint` clean · census `--strict` exits 0. Remaining files in this class (`branch-group-ops.ts`, `store.ts`, and the dependency pairs) can now follow this pattern instead of waiting — with the gate-versus-consumer check applied to each, since `branch-group-ops.ts` sits closer to the persistence layer than this one does. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
…f-conversion I shipped Both greptile P1s on #2744 were real. 1. STALE FLAGS ACROSS TASKS. I passed `workflowMoveMetadata?.currentColumnFlags` raw to TaskReviewTab. The file already has `detailColumnFlags`, which applies `detailFlagsAreForThisTask` (`workflowMoveMetadata?.taskId === task.id`) — because on the render where the modal switches tasks the state still holds the PREVIOUS card's payload. Passing the raw value resolves the review tab's roles from another task's workflow: confidently wrong rather than merely stale. That reasoning was already written six lines above my own change and I reviewed past it. 2. A HALF-CONVERSION. `canStartPrFeedbackAddressing` in utils/prFeedback.ts carries the SAME lane pair the tab asks about, and I converted only the caller. Consequence: on a renamed review or WIP lane, a task with actionable PR feedback but NO loaded display items kept the Address-PR-Feedback action hidden — the caller's role check passed and the helper returned false. Now takes optional flags on the same seam, with both callers (TaskReviewTab, TaskCard) supplying what they already hold. This is the third instance in this program of "both halves or neither": the archived gate (#2724), the tracking pair (#2715), and now the lane pair split between a component and its util. Converting the visible half first is what makes the remaining half invisible. REVERT PROOF: restoring the id pair in `canStartPrFeedbackAddressing` fails the new case. The case deliberately supplies EMPTY review items — with items present the `displayItems.length > 0` arm masks the helper and the bug cannot be seen. Two fixture details the first attempt got wrong and are now recorded in the test: the field read is `lastReviewDecision` (not `reviewDecision`), and `isPrMode` needs review source `pull-request`. Census: prFeedback.ts 2 -> 0 (re-recorded via --update-baseline, which reported exactly "TIGHTENED 1 entry"). Gate 158+10+487+71 GREEN. 463 passed across the review-tab, card, sorting and columnRoles suites; the 2 TaskCard failures are the known pre-existing CSS-var geometry assertions. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
… told there was none (#2739) Second application of the sync-filter pattern decided in #2737. `branch-group-ops.ts` 6 → **0**. ## The failure `selectNextTaskForAgentImpl` picks an agent's next task by filtering the board for its WIP lane, then its hold lane — both `task.column === "<literal>"`. On a renamed board **both filters match nothing**, so an agent asking for work is told there is none, with its own assigned tasks sitting in the list it just fetched. No error, no log line. The agent idles. `pauseTaskImpl` had the same shape: pausing a running card on a renamed board left its `status` untouched, so the UI kept showing it as working. ## Consumer, not a gate — checked rather than assumed Applying the #2724 test to this file, since it sits closer to the persistence layer than the reconciler did: its **only** SQL predicate is `eq(table.projectId, ...)`. Nothing here compares a column to a literal in SQL, so there is no second encoding of these questions to diverge from. The list arrives from `store.listTasks` and the filters select among rows already in hand. Async predicates were the alternative and would have turned these filter chains into sequential awaits inside the dispatch path. One prefetch, one IR read per distinct workflow, filters stay synchronous. `pauseTaskImpl` resolves for the single task it holds rather than joining a map — different entry point, one id in scope, and a map would have exactly one entry. ## Revert proof | reverted | result | |---|---| | the wip literal | renamed WIP case fails: `expected null to be truthy` | | the hold literal | renamed hold case fails identically | The new test calls the impl **directly** with a store fake resolving a renamed IR. The existing `selectNextTaskForAgent` coverage drives a real store harness, so exercising a renamed vocabulary there means registering a real custom workflow and moving cards through it — heavier than the question, which is only which lane the filters name. The bind evaluator runs for real; only the store is faked. A third case pins that the hold filter keeps its `userPaused` exclusion, so a filter matching every column would not satisfy the other two. **Related coverage checked before writing a new file:** `agent-heartbeat-worktree-renamed-hold.test.ts` covers the requeue **target** on a renamed board, not the dispatcher's **selection** filters — different branch of the same subsystem, so a case added there would have read as duplicate coverage of the wrong thing. ## Verification `pnpm test:gate` **GREEN** (158 + 10 + 487 + 71) · **20 passed** across the routing-policy and new dispatch suites · core `tsc` clean · `pnpm lint` clean · census `--strict` exits 0. 🤖 Generated with [Claude Code](https://claude.com/claude-code) <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **Bug Fixes** - Improved agent task selection when workflow lane names or IDs have been renamed. - Agents now correctly resume assigned in-progress or queued tasks across customized workflows. - Prevented agents from selecting tasks paused by users, including on boards with renamed lanes. - Updated task pausing behavior to correctly reflect lifecycle stages beyond default lane names. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
…turally cannot see, one a live defect (#2763) Docs only, extending the entry #2748 landed. Opening it because the fleet reads the census total as its completion bar, and that total excludes a whole predicate class — a measurement that should not live in a chat reply. ## Measured on `origin/main` - **47** array/Set literals of two or more lifecycle ids, in 35 files. - **25 are membership predicates against a task's column** — `SET.has(task.column)` / `ARRAY.includes(task.column)` — in 19 files. Two are documented fallbacks behind a resolved primary, so **~23 are unconverted guards**. - The census scans `===` / `!==` against a column. **None of these is a comparison, so none is counted.** | file | constant | | --- | --- | | `cli/src/commands/task.ts` (3) | `retryReviewColumns` | | `dashboard/app/components/TaskCard.tsx` (2) | `TIME_INDICATOR_COLUMNS` | | `engine/src/eval-followups.ts` (2) | `OPEN_COLUMNS` | | `engine/src/merger.ts` (2) | `sourceTerminal` | | `engine/src/task-revert.ts` (2) | `REVERTABLE_COLUMNS` | | `core/src/agent-role-policy.ts` (1) | `IMPLEMENTATION_TASK_COLUMNS` | ## One is a proven live defect `isImplementationTask` is `IMPLEMENTATION_TASK_COLUMNS.has(task.column)`, and `evaluateImplementationTaskBind` short-circuits to `allowed: true` when it returns false. **On a renamed board every agent is bind-compatible with every task** — the role check that stops a liaison being handed implementation work (the NEXT-871 loop FN-7851 fixed) does not apply. It surfaced only because a reviewer questioned a coverage claim in one of my dispatch tests (#2739). Passing an agent wasn't proof the evaluator ran, so I asserted a `custom`-role agent must be *refused* — and that test failed against production. Flagged at the site in #2739, not fixed: `isImplementationTask` is a sync pure predicate with no store, and making the routing policy async is a behaviour change to agent admission. ## What this does and does not argue The census is the right instrument — AST-based, honest about what it measures, and it has caught real drift in both directions (it failed on me in #2724 when merged conversions moved an inventory *down*). This is not an argument against it. It is an argument against reading **"backlog: N" as "N guards remain"**. The same shape already appeared in the archived gate (#2724), where the rule is additionally encoded in Drizzle predicates and raw `sql` templates that no comparison scan can see. Two independent classes now, found the same way — by looking at what the instrument's definition excludes. **Extending the census to count membership predicates is deliberately left to you, not done here.** It would move every worker's number mid-fleet, and deciding which sets are lifecycle guards versus board-config definitions or type unions is exactly the judgement `DELIBERATE-LITERAL` exists for — 47 collections would each need that call. ## Verification `pnpm lint` clean · census `--strict` exits 0 · no code changes. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
…e to every check we have Second known instance (the first was the archived gate in PR #2724), which makes it a pattern rather than an accident: `getInReviewDurationEvents` encodes `in-review` and `done` in a raw SQL fragment, so the Reliability duration metric stays blind on a renamed board while the two counts beside it — fixed in #2861 — start working. Partial blindness is harder to spot than total blindness. Neither the census (scans comparisons) nor the unwired-parameter guard (scans declarations) can see a string inside a `sql` template, so this class is not in the backlog number at all. That is a second, independent reason the total is a floor. Also records the sibling-importer lesson: GitLab's `column: "triage"` was fixed in #2843 and Linear's identical copy survived until an area already declared clean was re-grepped (#2860). Handoff for batch-core; no source change. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…e to every check we have Second known instance (the first was the archived gate in PR #2724), which makes it a pattern rather than an accident: `getInReviewDurationEvents` encodes `in-review` and `done` in a raw SQL fragment, so the Reliability duration metric stays blind on a renamed board while the two counts beside it — fixed in #2861 — start working. Partial blindness is harder to spot than total blindness. Neither the census (scans comparisons) nor the unwired-parameter guard (scans declarations) can see a string inside a `sql` template, so this class is not in the backlog number at all. That is a second, independent reason the total is a floor. Also records the sibling-importer lesson: GitLab's `column: "triage"` was fixed in #2843 and Linear's identical copy survived until an area already declared clean was re-grepped (#2860). Handoff for batch-core; no source change. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…n see (#2877) Docs only. Two findings from this unit that cost real time to derive and would otherwise be re-derived by whoever reaches these files next. ## 1. `=== "archived"` is usually a SENTINEL `packages/core/src/task-store/async-comments-attachments.ts` carries **9** census guards — the second-largest single-file count outside `self-healing.ts`. Reading all nine: **exactly one** is a board-column comparison. The other eight compare against a value `getLiveTaskColumn` *manufactures*: ```ts if (row.column === "archived" || row.deletedAt != null) return "archived"; // ← fabricated return row.column; ``` Converting those eight to `isArchivedColumnRole` would keep passing on the built-in board and start **failing** on a renamed one — a soft-deleted parent's documents would become readable. **The conversion makes the renamed board worse**, which is the opposite of what the census count implies. The rule that separates them: look at where the compared value *came from*, not at its type. From `task.column` or a DB field → a board lane. From a function that *returns* `"archived"` as a documented outcome → a sentinel. Consequence worth stating plainly: **a file's census count is an upper bound on convertible sites, not a work estimate.** ## 2. Lane literals inside raw `sql` are in no total at all The Reliability panel had three inputs. Two were call arguments and converted routinely (#2861). The third encoded its lanes in a `sql` fragment: ```sql metadata->>'to' = 'in-review' OR (metadata->>'from' = 'in-review' AND metadata->>'to' = 'done') ``` The census scans `===`/`!==` comparisons; the unwired-lane-parameter guard scans declarations. **Neither can see a string inside a `sql` template**, so this class is not in the backlog number — a second, independent reason the total is a floor. Second known instance after the archived gate in PR #2724, which makes it a pattern rather than an accident. Fixed in #2875, and the doc says so rather than leaving it described as outstanding — a learnings doc that reports a fixed defect as open sends the next reader to a dead end. `scripts/check-sql-column-literals.mjs` (#2841) is the detector for the class and freezes the surface at 30 sites; the two are complementary. ## 3. Sibling files The GitLab importer's `column: "triage"` was fixed in #2843. The Linear importer — written from the same template, with **two tests pinning the bug** — still had it, and was found only by re-grepping an area I had already declared clean (#2860). When a defect is found in a file that has a sibling, the sibling is the next place to look, and no tool will tell you that. ## Verification `pnpm lint` clean. No source change. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claimed the async-comments-attachments cluster (9) and did not convert it. This PR is the evidence for why, as a guard rather than a note — no production change, census unmoved.
What the cluster actually is
Every other file in the backlog converts on its own: resolve the task's lifecycle columns, compare against the role.
archiveddoesn't, and not by a matter of degree.Measured in
packages/core— one rule, three encodings, 52 sites:=== "archived"ne(tasks.column, 'archived')sql`…column != 'archived'`Convert only the TypeScript half and a board whose archived lane is renamed splits:
getLiveTaskColumncorrectly reports the task archived (it resolved the role) whilereadLiveTaskRowsstill hands it back as live. A document write is rejected by its gate while its parent is listed as live — a state neither gate alone can produce today.And nothing would catch it. Every builtin workflow spells that column
archived, so all three encodings agree by accident on every board we ship.Why the SQL sides aren't just converted too
ne(tasks.column, ...)needs the resolved id as a query-build value, so the IR must be resolved before the query — including inside thefor updatedocument/artifact transactions, which today receive adb/txhandle and no store and no workflow reader. One raw site is a hand-writtenSELECTstring, so its comparison isn't even a Drizzle expression that could take a bound value without rewriting the query.The two real options are: thread a resolver into the persistence layer, or declare
archiveda non-renameable system column and mark all 52 sites deliberate. Both are decisions with blast radius. Neither is a fleet conversion, so I didn't pick one.I was wrong about the shape, and that's the strongest part
I wrote the third case as an assertion that no raw
sqltemplate compares a column to'archived'— I assumed two encodings. It failed on the first run with five files.Nothing in the repo was counting them: they aren't comparisons (invisible to the column census) and aren't
eq/necalls (invisible to the Drizzle scan). A partial conversion doesn't have to miss one encoding — it can miss two. That case is now an inventory, with a note on why asserting absence was the wrong invariant: an absence assertion has to be deleted by whoever adds the next raw template, and deleting a red guard is how a class of sites stops being tracked.Injection proof — all three run
async-comments-attachments.tsasync-lifecycle.tsreads.tsEach failure carries the split-brain explanation and the two real options, so the next worker hits the reason rather than a bare count mismatch.
Two scan bugs I fixed in my own guard
async-archive-lineage.tsthat were prose in a JSDoc block. Now comment-stripped through the census's sharedstripCommentsrather than a second implementation.x.column === "archived") misses the list paths that hold the value in a bare local (if (column === null || column === "archived") return []) — four of the eight sites in the target file, measured. The matcher now accepts a bare identifier namedcolumn.Counts here are per file, not per line: line numbers churn on unrelated edits, and a ratchet that cries wolf gets deleted.
Verification
pnpm test:gateGREEN (158 + 10 + 487 + 71) · new guard 2/2 ·pnpm lintclean · coretscclean · census--strictexits 0 with the baseline untouched — this PR converts nothing and claims nothing.🤖 Generated with Claude Code