fleet: github-tracking-reconciler 9 → 0 — deciding the sync-filter class (prefetch a resolved map), and the reconciler closed NO issues on a renamed board - #2737
Conversation
Greptile SummaryThe PR makes GitHub tracking reconciliation workflow-aware.
Confidence Score: 5/5The PR appears safe to merge, with the previously reported scan-bounding inefficiency still outstanding. The workflow cache no longer survives between reconciliation passes or periodic sweeps, while lifecycle prefetch can still traverse a full board when qualifying tasks are sparse and remains unbounded in the backfill path; no blocking failure remains. Files Needing Attention: packages/dashboard/src/github-tracking-reconciler.ts
|
| Filename | Overview |
|---|---|
| packages/dashboard/src/github-tracking-reconciler.ts | Adds per-task workflow lifecycle resolution and uses resolved terminal columns across four reconciliation paths. |
| packages/dashboard/src/tests/github-tracking-reconciler.test.ts | Extends the fake store with optional workflow IR support and tests renamed complete and archived lanes. |
| scripts/lib/lifecycle-column-census-baseline.json | Removes the converted reconciler entries and preserves the tracking-comments baseline entry. |
Reviews (3): Last reviewed commit: "chore(census): re-record baseline after ..." | Re-trigger Greptile
|
Warning Review limit reached
Next review available in: 6 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 (3)
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 |
…aim a finished card, and no card was ever 'stale' (#2746) Two core clusters. 6 converted, 2 flagged. ## Two silent failures **No card was ever stale.** `task-age-staleness.ts` applies its signal only to the mid-flight and review lanes — a card in a hold or terminal lane is waiting or finished, not stale. Both lanes were named by id, so on a renamed board the signal returned `undefined` for **every** card and the stale-card warning never appeared anywhere on the board. **An agent could claim a finished card.** `claimTaskForAgent`'s terminal guard was `column === "done" || column === "archived"`. On a renamed board neither matched, so the claim **succeeded** and the agent began work on completed output. ## The threshold selectors are a separate literal, and half-converting is worse than neither `task-age-staleness` has two independent uses of `in-progress`: the **lane gate** that decides whether the signal applies, and the **threshold selectors** that pick which warning/critical numbers to measure against. Converting only the gate admits a renamed-WIP card and then measures it against the **review** threshold — a wrong number, silently. Both are converted, and each is revert-proofed on its own: | reverted | result | |---|---| | the lane gate | 3 of the new cases fail (`expected undefined to be defined`) | | the threshold selectors | the threshold case fails — a renamed WIP card gets the review threshold | No new seam for either: the staleness signal already took a `context` object, and its one production caller (`task-store/reads.ts`) already holds a **per-pass IR cache** for precisely this kind of resolution. ## Cost stated rather than hidden The `reads.ts` resolution is **unconditional**, where the hold-column read directly beside it is gated on `task.paused`. That asymmetry is deliberate: the lanes this needs are exactly what decides whether the signal applies at all, so there is no cheaper gate available ahead of it. With the shared per-pass cache that is a struct build per card, not an IR read. ## Flagged and left counted `formatCurrentTaskLine` is a pure formatter over `Pick<Task, "column">` whose output **prints** the column name for a human reader — same class as `github-tracking-comments.ts:165`. It also degrades gracefully: the "(not active — X)" wording is lost on a renamed board, but "(X)" is still accurate, just less specific. Threading a resolution into a string builder to pick a word is the wrong trade. ## The recurring blind spot, fourth time **None of the 12 existing staleness cases could have caught this** — `lifecycle` is optional and they all omit it, so they assert the legacy fallback. Same for the reconciler's 33 (#2737) and `TaskReviewTab`'s 45 (#2744). This is now a consistent property of the optional-flags seam: **the existing suite stays green through the conversion and through a broken one.** Every file in this program needs at least one case that supplies flags, or the conversion is untested in both directions. Worth making an explicit review criterion rather than something each worker rediscovers. ## Verification `pnpm test:gate` **GREEN** (158 + 10 + 487 + 71) · **31 passed** across staleness / routing-policy / dispatch suites · core `tsc` clean · `pnpm lint` clean · census `--strict` exits 0. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
a5a7d65 to
87426cd
Compare
|
Both findings are real and fixed. The first one is the more embarrassing of the two, because I had already argued the opposite position in a neighbouring file. 1. Workflow cache stays stale (P1) — correct. The IR cache was instance-scoped on a long-lived reconciler, so an operator's workflow edit would never be picked up. In
Then I used one here two PRs later. The cache is now allocated per pass, which is the shape 2. Lifecycle prefetch bypasses the scan limit (P2) — correct. The prefetch resolved for every row
Rows past the cut are left unresolved and absent from the map, which is safe and worth stating explicitly: the only consumers are the terminal predicates, which fall back to the legacy ids for an absent entry — the same degraded answer they give on a store with no workflow reader — and those rows are dropped by the slice anyway. The fourth pass ( VerificationGate GREEN (158 + 10 + 487 + 71) · 35 passed across the three reconciler suites · dashboard |
…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>
…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>
…tch by the scan limit
87426cd to
305f276
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>
…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>
…hanges landed as #2715 and #2737 This PR converted github-tracking-comments.ts and github-tracking-reconciler.ts. Both files were then converted independently while it sat in the queue: #2715 took comments.ts (a literal comment KIND passed to formatTrackingComment where mine derived one) and #2737 took the reconciler (a prefetched lifecycle map where mine bounded a take). Both are on main; I am not contesting either. SO THE SOURCE CHANGES ARE DROPPED AND THE SUITE STAYS, for a specific reason: it passes against THEIR implementations unchanged. Two independent implementations satisfying the same assertions is the strongest available evidence that the assertions describe the invariant rather than one author's shape — and that is worth more here than the conversion I no longer own. It also adds the coverage neither PR has: the resolution-COST cases. #2737's bound is real (its own review caught the same unbounded prefetch greptile caught on mine) but it is asserted only in a comment. Measured against main by disabling the `break`: 600 resolutions for a 600-row history against a 200-row limit, and the case reddens. A performance guard with no test is one refactor from being gone, and the output is identical either way — only counting the work can see it. 9/9 in this suite against main's implementations; 206/206 across the nine github-tracking suites; gate green (10 / 71). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…hanges landed as #2715 and #2737 This PR converted github-tracking-comments.ts and github-tracking-reconciler.ts. Both files were then converted independently while it sat in the queue: #2715 took comments.ts (a literal comment KIND passed to formatTrackingComment where mine derived one) and #2737 took the reconciler (a prefetched lifecycle map where mine bounded a take). Both are on main; I am not contesting either. SO THE SOURCE CHANGES ARE DROPPED AND THE SUITE STAYS, for a specific reason: it passes against THEIR implementations unchanged. Two independent implementations satisfying the same assertions is the strongest available evidence that the assertions describe the invariant rather than one author's shape — and that is worth more here than the conversion I no longer own. It also adds the coverage neither PR has: the resolution-COST cases. #2737's bound is real (its own review caught the same unbounded prefetch greptile caught on mine) but it is asserted only in a comment. Measured against main by disabling the `break`: 600 resolutions for a 600-row history against a 200-row limit, and the case reddens. A performance guard with no test is one refactor from being gone, and the output is identical either way — only counting the work can see it. 9/9 in this suite against main's implementations; 206/206 across the nine github-tracking suites; gate green (10 / 71). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…hanges landed as #2715 and #2737 (#2714) **Claim announced on #2706 before starting.** `github-tracking-comments.ts` + `github-tracking-reconciler.ts` — one coherent subsystem, 9 guards each. **18 → 7 by census, 18 → 0 behaviourally.** The gap is explained at the bottom and it is not hand-waving. ## Both halves failed quietly, in the way that suppresses its own evidence | surface | what a renamed board got | |---|---| | the comment poster | returned early for **every** move — the tracked issue silently stopped receiving both its "in progress" and its "done" comment. The operator sees a linked GitHub issue that never updates. | | the reconciler (3 scan passes) | matched **zero** tasks, so completed work's issues were never closed — and the pass reported a clean `scanned: 0`. | That second one is the shape worth internalising: **the number that would have revealed the problem is the number the bug suppresses.** No error, no warning, a green sweep. ## The comment poster needed a derivation, not a swap `event.to` was **both** compared against the two literals **and** passed into `formatTrackingComment` as its `transition` argument (typed `"in-progress" | "done"`). One value carrying two meanings: a lane id and a comment kind. Eight independent swaps would have had to keep agreeing with each other forever — and a ninth site (the template's own `transition === "done"`) is *not* a column at all, so a mechanical sweep would have converted it wrongly. Resolving the lanes once and deriving the kind separates the two meanings permanently. Log details still print the real column, so the operator reads their own board's name. ## The reconciler Per task through **one shared IR cache per scan**, resolved into a `Set` of terminal ids rather than an async predicate inside `.filter(...)` — `Array.filter` ignores promises, so an async predicate there silently keeps **every** row. That is a trap worth naming for other fleet workers converting list filters. The archived-vs-complete distinction keeps its own resolver rather than reusing the terminal pair: it decides GitHub's `state_reason`, and closing a finished issue as `not_planned` is operator-visible and wrong — as is the reverse. ## Revert proof **5 of 8 new cases redden.** 201/201 across the nine `github-tracking` suites (193 were already there and still pass). ## Why the census says 7 and not 0 The reconciler goes **9 → 0**. The comment poster still reports **7**, and every one of those is `transition === "in-progress" | "done"` — the derived comment **kind**, not a column. There is no lane comparison left in the file. That is exactly the vocabulary-collision class **#2692** is fixing (it already lists five misclassified receivers: an SSE event type, a cache-key mode, an evidence kind, a telemetry event kind, an agent state). **`transition` is a sixth and I have reported it there.** Until that lands the census counts them, so I am reporting both numbers rather than the flattering one. I deliberately did **not** mark them `DELIBERATE-LITERAL` to move the count: that marker means "a lifecycle literal reviewed and kept", and these are not lifecycle literals at all. Using it as a census-silencer would put a wrong reason in the code to make a number look better. ## Verification `pnpm test:gate` **487 / 71** · **201/201** github-tracking suites · `tsc -p packages/dashboard` clean · `pnpm lint` clean · census `--strict` exit 0 (it also tightened three entries other workers' merges left stale — the #2679 auto-tighten working). No changeset: `@fusion/dashboard` is private and this is internal behaviour on renamed boards. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
…umn id to force done-sorting (#2744) Two app-side clusters, 8 → **0**, plus a caller-side hack retired. ## What was broken **`TaskReviewTab.tsx`** — three of its four questions were `task.column === "in-review"`, driving the **Create-PR button**, the **"frozen on entry to review"** auto-merge hint, and **PR-feedback addressing**. On a renamed review lane all three took their non-review branch: the button was absent, and the hint claimed the effective auto-merge value was *not* frozen when it was. **`taskSorting.ts`** — `isReviewColumn` decides whether merging cards float to the top of a lane. Keyed on the id it silently stopped doing that on any renamed review lane, so the operator loses the "what is merging right now" ordering with nothing failing. Both follow the shape this code already established: **caller supplies the trait, default to the legacy id**. `columnFlags` on the review tab is optional and wired from `TaskDetailModal`, which already resolved it for `canEdit` and the actions menu. ## A synthetic column id, retired `Board.tsx` forced done-sorting by passing the **literal `"done"`** as the column argument for any complete-flagged lane: ```ts grouped[column.id] = isWorkflowDoneLikeColumn ? sortTasksForDisplayColumn(grouped[column.id] ?? [], "done", doneSortMode) : sortTasksForDisplayColumn(grouped[column.id] ?? [], column.id as ColumnType, ...); ``` A synthetic id standing in for a trait — so a custom complete lane sorted correctly only because its caller **lied about its name**. Both call sites now pass the real column id and state the trait. (Board's own census count stays at 2: those two literals were the synthetic ids and are gone; the 2 remaining are different sites.) ## Revert proof | reverted | failure | |---|---| | `task.column === "in-review"` on the Create-PR guard | `Unable to find an element by: [data-testid="task-review-create-pr"]` | | same, on the auto-merge hint | `expected 'Effective: Auto-merge off' to contain 'frozen on entry to review'` | A third case pins that the widened test does not treat *every* column as review. **None of the 45 existing `TaskReviewTab` cases could have caught this** — `columnFlags` is optional and they all omit it, so they assert the legacy fallback. That is the same blind spot as the reconciler's 33 in #2737, and it keeps recurring: an optional-flags seam means the existing suite stays green through the conversion *and* through a broken one. ## A process failure worth recording **I lost this conversion once and had to redo it.** I overwrote four files with their `origin/main` versions to check whether a failing test was pre-existing, then "restored" with `git checkout HEAD -- <dir>`. HEAD was still `origin/main` because I had not committed, so that **discarded the work**. Same class as the shared-stash incident two PRs back: an implicit or positional restore reference. The fix is ordering, not care — **commit before any baseline comparison**, so `git checkout HEAD -- <file>` restores my work rather than main's. This PR's commit was created before the comparison for exactly that reason, and the note is in the commit message so the next person hits it there too. ## Verification `pnpm test:gate` **GREEN** (158 + 10 + 487 + 71) · **232 passed** across TaskReviewTab / taskSorting / Board suites · dashboard `tsc -p tsconfig.app.json` clean · `pnpm lint` clean · census `--strict` exits 0. The 1 `board-mobile` failure is **pre-existing** — verified by swapping in clean `origin/main` copies of all four files and reproducing it. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- 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>
github-tracking-reconciler.ts9 → 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 awaitand 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: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.
resolveLifecycleColumnsis 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:
resolveTaskLifecycleColumnsalready 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/coreis enforced in three encodings, so converting one alone diverges them. I checked whether that applies here before converting:sqltemplate, noeq/ne.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 ondeletedAt IS NOT NULLandgithubTracking 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
resolveTaskLifecycleColumnscatches and returnsundefinedand every case asserts the legacy fallback — exactly what it always asserted. None of them could have caught this being wrong.workflowIris now an opt-in on that fake, which is what makes the new cases real tests rather than restatements.setIssueStatesetIssueStateA reachability finding, recorded not acted on
In backend mode
reconcileDeletedAndArchivedreturns only soft-deleted rows — its own comment says the archived-tasks fallback is a separateAsyncArchiveLineagesubsystem, skipped there — andtask.deletedAtis tested first in thestateReasonchain. 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:gateGREEN (158 + 10 + 487 + 71) · 35 passed across the three reconciler suites · dashboardtscclean ·pnpm lintclean · census--strictexits 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, sincebranch-group-ops.tssits closer to the persistence layer than this one does.🤖 Generated with Claude Code