Skip to content

batch-cli-plugins: 7 guards — 3 were a foreign enum, and fn pr create refused every card on a renamed board - #2775

Merged
gsxdsm merged 7 commits into
mainfrom
batch-cli-plugins
Jul 30, 2026
Merged

batch-cli-plugins: 7 guards — 3 were a foreign enum, and fn pr create refused every card on a renamed board#2775
gsxdsm merged 7 commits into
mainfrom
batch-cli-plugins

Conversation

@gsxdsm

@gsxdsm gsxdsm commented Jul 30, 2026

Copy link
Copy Markdown
Collaborator

batch-cli-plugins — the u7 worker's mega-batch: packages/cli + plugins + anything left.

The batch is 7 guards, and 3 of them are not guards at all

The census's per-file list gives this batch seven sites. Reading them, three are a foreign vocabulary the census matches on the string alone:

file site verdict
plugins/fusion-plugin-reports/store/report-store.ts next === "archived" ×2 not a columnnext is a ReportStatus
plugins/fusion-plugin-reports/store/report-types.ts to === "failed" || to === "archived" not a column — same enum, its own terminal states

The reports plugin has its own status lineage (draft → generating → review_* → approved → published, plus failed/archived) that shares two spellings with the lifecycle vocabulary. A report is not on a board and has no workflow, so resolving an IR there would answer a question nobody asked. All three are marked DELIBERATE-LITERAL with the reason at the site.

This cuts the other way from #2763. That PR establishes the census total as a floor (25 membership predicates it structurally cannot see). This is the opposite error in the same number: a foreign enum inflating it. The total is neither a ceiling nor a floor — it is an estimate with error in both directions, and the per-file list is worth reading before trusting a file's count.

Converted (census before → after, per file)

file before after
packages/cli/src/commands/pr.ts 1 0
plugins/…/even-realities-glasses/notifications/diff.ts 1 0
plugins/…/reports/store/report-store.ts 2 0 (deliberate)
plugins/…/reports/store/report-types.ts 1 0 (deliberate)

fn pr create refused every card on a renamed board

The live defect in this batch. The gate was task.column !== "in-review", and its error told the operator to move the task to a column their board does not have:

Error: Task must be in 'in-review' column to create a PR (current: signoff)

There is no way to satisfy that short of renaming the workflow back. Now resolved through core's resolveReviewColumns, and the message names the lanes that actually exist.

The SET, not lifecycle.review. A board may declare more than one review lane, and a card parked in a humanReview-only lane is still a card you can open a PR from. A single-id answer keeps refusing those — the same narrowing #2728's review caught in the CLI retry gate, which is why the test pins both lanes.

Skipped, with the reason

plugins/fusion-plugin-even-cards (2 guards) — blocked on packaging, not on analysis. The defect is real: boardToDeck filters with column !== "archived" && column !== "done", so on a renamed board every finished card stays in the deck, fills maxCards, and pushes the active cards off the display. The wearer sees a board that never finishes anything.

I implemented the fix and reverted it: this plugin is not in pnpm-workspace.yaml and depends only on @fusion/plugin-sdk — it has no @fusion/core dependency, so the route cannot reach resolveTaskLifecycleColumns. Adding one is a packaging change, which this program's rules put out of scope. Shipping only the injected parameter without a caller was the alternative, and that is precisely the decorative conversion #2759 documents: the census would drop by 2 and the deck would keep the bug.

Flagged for whoever owns the plugin's dependency surface. The glasses plugin next door does depend on @fusion/core, so this is a one-plugin problem, not a plugin-wide one.

Honest note on the glasses conversion

diff.ts's completion branch is currently unreachable — the only production caller (notifier.ts) passes alsoNotifyOnDone: false. So that conversion changes nothing at runtime today. It is converted rather than marked deliberate because the literal is not deliberate: it is wrong, and would ship the bug the day someone turns the flag on. Stated here rather than left for a reviewer to discover.

Verification

  • new CLI suite 4 passed; pr-command + pr-automerge-cleanup + bin-pr-router 35 passed
  • glasses plugin 181 passed (19 files) · reports plugin 110 passed (23 files)
  • pnpm test:gate158 / 10 / 487 / 71 · pnpm lint clean · --strict exits 0

Revert proof, measured. Restoring if (task.column !== "in-review") fails 3 of the 4 new cases (process.exit:1 on both renamed lanes, and the refusal message reverts to naming in-review). The unresolvable-workflow case keeps passing — it is the legacy path — so the negative cases alone do not pin the fix and all four are required.

Handoff to batch-engine

packages/engine/src/project-engine.ts 5 → 0 is finished, green, and pushed as handoff/project-engine-lanes-for-batch-engine (34dbb35209) for the capacity worker to cherry-pick — it is engine-owned, not mine to land.

It fixes two live defects: a card that had merged reported as a failed merge to fn task merge and the dashboard button (merged: finalTask?.column === "done"), and the three post-finalize column === "done" && mergeConfirmed fast-path checks, which on a renamed board sent an already-landed card down the bounce path — re-queued, retry-counted, and in the capped branch parked failed with its merge sitting on main. Plus hasAutoHealableVerificationBufferFailure, which returned false for every card on a renamed board, so a buffer-overflow verification failure was never auto-healed.

8 new tests, revert-proven (restoring the literal fails 4 of 8), gate green.


Completion pass (u7) — the batch is now closed

Two workers converged on this branch. I rebased onto the first-landed commit rather than force-pushing over it, took its wording wherever the conclusion was identical, and added what was missing.

What this pass added

  1. even-cards (2 sites) — the only in-scope file the first pass left open. Marked DELIBERATE-LITERAL: the package depends on @fusion/plugin-sdk only, and the SDK does not re-export the lifecycle role helpers, so there is no IR, no store, and no trait flags to resolve from. Fixing it properly means the SDK exposing role flags on the task shape it hands plugins — a structural change, out of scope, and recorded at the site as the correct home. Live consequence is cosmetic: a finished card on a renamed board shows as active in the glasses deck.

  2. A red test in the fn pr create conversion. The incoming version rendered Task must be in 'in-review' to create a PR, dropping the word column. task.test.ts:3422 pins must be in 'in-review' column, so that hunk failed runTaskPrCreate > exits with error when task not in in-review column. Restoring the word makes the single-lane message byte-identical to the pre-conversion one, which is what a vocabulary conversion should be — the guard's own test now passes unmodified. Marked at the site so it is not "simplified" back.

  3. Duplicate imports — the two independent conversions each added resolveWorkflowIrForTask/resolveReviewColumns, which does not compile. Deduped in its own commit.

Census

Measured with --json on origin/main and on this branch.

file before after action
packages/cli/src/commands/pr.ts 1 0 converted
plugins/fusion-plugin-reports/src/store/report-types.ts 1 0 marked
plugins/fusion-plugin-reports/src/store/report-store.ts 2 0 marked
plugins/fusion-plugin-even-cards/src/cards/board-cards.ts 2 0 marked
plugins/fusion-plugin-even-realities-glasses/.../diff.ts 1 0 marked

Backlog 415 → 408 (−7, exactly the in-scope count). Deliberate 40 → 46 (+6 marked); 6 + 1 converted = 7. --strict exits 0. Nothing remains in cli + plugins + everything-else — there is no follow-up batch behind this one.

One note on the even-realities-glasses site

Worth recording beyond "cannot resolve": its only production caller (notifier.ts:80) passes alsoNotifyOnDone: false, so that arm is unreachable today. Converting it could not have changed observed behaviour either way.

Verification (measured, on the merged branch)

  • pnpm --filter @runfusion/fusion exec tsc --noEmit → exit 0
  • pnpm lint → 0 errors
  • CLI task.test.ts → 144 passed, including the runTaskPrCreate guard test
  • @fusion-plugin-examples/reports → 110 passed; even-realities-glasses → 181 passed

Pre-existing failures, not from this change: the 5 runTaskImportFromGitHub / runTaskImportGitHubInteractive tests fail identically on origin/main — verified by stashing this diff and re-running (5 failed / 144 passed both ways).


Census audit (unowned follow-on)

After closing the batch scope I audited whether the 392 column-backlog number is inflated by foreign vocabularies — the class this batch found in the reports plugin, where "archived" is a ReportStatus rather than a board lane. If that class were widespread, every remaining batch would be chasing sites that must not be converted.

It is not. The number is real. A receiver-level pass over all 392 column-category sites found exactly 3 false positives, all in plugins/fusion-plugin-reports (next, a ReportStatus), all now marked in this PR.

What was checked and cleared:

  • Property-reached foreign enums (step.status, feature.status, mission.status) — already correctly bucketed into the separate status category (185), not the column backlog. Verified against merge-queue-ops.ts: 11 lifecycle-spelled literals in the file, census counts 1, and that 1 is the genuine .column guard.
  • Bare step-status variables (status, currentStatus, liveStatus compared to "done"/"skipped") — likewise excluded.
  • Every other receiver in the backlogto, from, column, fromColumn, toColumn, latestColumn, state, preArchiveColumn. All resolve to genuine task columns. executor.ts's 15 sites were spot-checked line by line: all 15 are real.

The gap the classifier genuinely cannot close is a foreign enum held in a bare variable — the receiver name carries no type information, so next === "archived" is indistinguishable from a lifecycle guard by AST alone. That is why the reports sites need a marker rather than a classifier fix, and it is now documented in lifecycle-column-census-ast.mjs's header alongside the measured scope, so the remaining batches do not re-run this hunt.

Census tests: 43 passed. The change is comment-only.

@coderabbitai

coderabbitai Bot commented Jul 30, 2026

Copy link
Copy Markdown
Contributor

Warning

Review limit reached

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

Next review available in: 26 minutes

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

How can I continue?

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

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 340bd7fc-3631-4b4b-baeb-08e8bae2b73c

📥 Commits

Reviewing files that changed from the base of the PR and between 7f3eee9 and edf1836.

📒 Files selected for processing (9)
  • packages/cli/src/__tests__/pr-create-review-lane-resolved.test.ts
  • packages/cli/src/commands/__tests__/task.test.ts
  • packages/cli/src/commands/pr.ts
  • plugins/fusion-plugin-even-cards/src/cards/board-cards.ts
  • plugins/fusion-plugin-even-realities-glasses/src/notifications/diff.ts
  • plugins/fusion-plugin-reports/src/store/report-store.ts
  • plugins/fusion-plugin-reports/src/store/report-types.ts
  • scripts/lib/lifecycle-column-census-ast.mjs
  • scripts/lib/lifecycle-column-census-baseline.json

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

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

Comment thread packages/cli/src/commands/pr.ts Outdated
@greptile-apps

greptile-apps Bot commented Jul 30, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR resolves workflow-specific review lanes for PR creation and classifies remaining lifecycle-like literals.

  • Replaces the CLI’s fixed in-review gate with workflow review-column resolution and adds renamed- and legacy-workflow coverage.
  • Marks report-status and plugin literals as deliberate census exclusions.
  • Updates lifecycle-census documentation and baseline counts.

Confidence Score: 4/5

The PR is not yet safe to merge because the unresolved empty-review-set branch contradicts its new regression test and leaves the prior phantom-lane failure intact.

A valid resolved v2 workflow with no review traits still receives the unconditional in-review fallback, while the added test requires an explicit no-review-lane refusal; this leaves the previously reported behavior outstanding and makes the changed test fail on the shown implementation.

Files Needing Attention: packages/cli/src/commands/pr.ts, packages/cli/src/tests/pr-create-review-lane-resolved.test.ts

Important Files Changed

Filename Overview
packages/cli/src/commands/pr.ts Resolves review lanes per workflow and preserves legacy fallback behavior, but the empty-set fallback still leaves the prior reviewless-v2 failure outstanding.
packages/cli/src/tests/pr-create-review-lane-resolved.test.ts Adds renamed, multi-lane, empty-lane, and unresolved-workflow coverage, including an empty-lane assertion contradicted by the current implementation.
packages/cli/src/commands/tests/task.test.ts Adds coverage confirming that synthesized legacy workflows can still create PRs from the conventional review column.
scripts/lib/lifecycle-column-census-baseline.json Reclassifies the seven addressed literal sites from the lifecycle backlog to resolved or deliberate entries.

Reviews (6): Last reviewed commit: "fix(scripts): re-record the baseline for..." | Re-trigger Greptile

…n a renamed board

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@gsxdsm

gsxdsm commented Jul 30, 2026

Copy link
Copy Markdown
Collaborator Author

Real — fixed in the latest commit. This is the conversion's own bug reappearing inside its fallback.

resolveReviewColumns returning an empty set is an answer — "this workflow declares no review lane" — not a failure to read. Coercing it back to 'in-review' named a column the board does not have, so the refusal told the operator to move the card somewhere that cannot exist. That is exactly the failure mode this conversion set out to remove.

The distinction the code now keeps:

  • undefined — the workflow could not be resolved. Legacy pair, because "could not read" is not "read, and the answer is none".
  • empty set — resolved, and there is no review lane. Refuses every column, and says so: must be in a review column (this workflow declares none) instead of inventing a lane name.

The single-lane message is untouched and still byte-identical to the pre-conversion string, so task.test.ts's pinned assertion passes unmodified (verified: 1 passed with the rest filtered out). tsc --noEmit exit 0, pnpm lint 0 errors.

Worth noting for the record: I flagged this exact fallback while rebasing the two independent conversions of this guard together, and deferred to the first-landed version rather than overwriting another worker's choice. The review is right and I should have raised it then rather than silently taking the weaker branch.

…ing one

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@gsxdsm
gsxdsm force-pushed the batch-cli-plugins branch from 9ee1ce6 to b2d7f54 Compare July 30, 2026 15:33
…seline

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@gsxdsm

gsxdsm commented Jul 30, 2026

Copy link
Copy Markdown
Collaborator Author

Review addressed, plus a self-inflicted red I shipped and caught

greptile P2 — empty review set names a phantom lane. Valid, and it was my own bug reintroduced one branch over.

if (reviewColumns.size === 0) reviewColumns.add("in-review") fell back to the literal for BOTH the unresolvable-workflow case and the resolved-but-no-review-lane case — so on a board that declares no review lane, the refusal pointed at a column that does not exist. That is the exact defect this PR exists to fix, one if later.

The two cases are different questions:

condition meaning answer
prIr === undefined the workflow could not be read at all keep the legacy literal — today's behaviour, the only safe default
resolved, lanes.length === 0 the board genuinely declares no review lane say so; inventing 'in-review' sends the operator to a phantom column

This is the ?? legacyId rule this program keeps relearning: a resolved struct with a missing field is an answer, not a missing one. I have now made this same mistake in four places, which is why the fallback is written as an explicit null sentinel rather than a ?? — the shape makes the two cases impossible to collapse by accident.

New case added and revert-proven: deleting the empty-set branch fails it, and the other four keep passing. Suite 5 passed.

A red I introduced and would rather report than have CI find

My rebase onto main left conflict markers inside scripts/lib/lifecycle-column-census-baseline.json, and I committed them. Cause: my conflict-resolution loop ran --update-baseline on the conflicted file, the script died parsing it, and I staged the result anyway without re-reading it.

Fixed by regenerating from main's baseline. Verified the ratchet actually catches this class rather than assuming it: writing garbage to the baseline makes --strict exit 1, and the restored file exits 0. So CI would have failed this PR — the guard was working; I simply shipped a broken input to it.

Worth stating why I got it wrong twice: my first check read --strict as exiting 0 on a malformed baseline, because I measured $? after a pipe and captured tail's status, not the script's. The measurement was broken, not the ratchet.

Verification after both commits

  • new CLI suite 5 passed · pr-command 28 passed combined
  • pnpm test:gate158 / 10 / 487 / 71
  • census --strict exits 0, baseline parses, backlog 394
  • rebased onto current main (#2751 merged in the meantime; no conflict with it — it touched commands/task.ts and the dashboard routes, not commands/pr.ts)

@gsxdsm

gsxdsm commented Jul 30, 2026

Copy link
Copy Markdown
Collaborator Author

Spare-capacity handoff, filed here because batch-core has no branch or PR yet and this is my own PR. Not claiming that batch — the main worker owns it; this is a cherry-pick offer.

Offered to batch-core: task-merge.ts 6 → 0 (handoff/task-merge-lanes-for-batch-core, 8c41278d71)

My own batch (packages/cli + plugins, #2775) came to seven guards, so this is spare capacity. Pushed as a handoff ref, not as a batch-core branch — that batch is the main worker's and I am not claiming it. Cherry-pick, or ignore and I will drop it.

Based on current main. Gate after the commit: 161 / 487 / 13 / 71; task-merge suites 108 passed; census --strict exits 0; backlog 399 → 393.

The sharp one: a guard that rejects the transition it just validated

moves.ts gates the review → complete move like this:

if (fromColumn === (moveLifecycle?.review ?? "in-review") && toColumn === (moveLifecycle?.complete ?? "done") ) {
  const mergeBlocker = getTaskMergeBlocker(task);   // ← re-asks with the literal

The condition resolves both columns from the workflow. The call inside then re-asked with the literal and refused, so on a renamed board a legal move threw:

Cannot move FN-1 to done: task is in 'signoff', must be in 'in-review'

The transition was validated as legal and then rejected by its own guard. This is the half-conversion shape worth watching for across the rest of core — the outer question resolved, the inner one not — and it is more damaging than an unconverted literal, because the resolved outer half makes the site read as done.

Two dependency predicates, deliberately not one

getTaskCompletionBlocker had two different literal sets and they are different questions:

  • a hard blockedBy marker clears only on terminal (complete/archived);
  • a declared dependency also clears at review — the work is done even though the merge has not landed.

Keyed on literals, neither ever matched on a renamed board: blockedBy never cleared and the card waited forever, and dependencies never satisfied so the dependent never completed. Neither raises anything; the card simply never becomes eligible.

A test asserts a review-column dependency satisfies a dependency but does not clear a blockedBy. Collapsing the two would either strand every dependent behind an unmerged dependency or release blocked cards early, and both are silent.

reviewColumns is not skipColumnIdentityCheck

The existing option means "I proved lane identity already, do not check". The new one means "check, against THESE columns". Kept separate so a caller that only wants to name the lane cannot accidentally disable the gate — asserted by a case where the check still refuses a non-review card with the lanes supplied.

The refusal message names the resolved lanes, so it can never point at a column the board does not have. (That exact phantom-lane bug was greptile's P2 on my #2775 — same mistake, caught one PR earlier.)

Map keyed by the DEPENDENCY's id

satisfactionColumnsByTaskId is keyed by the blocker/dependency, not the blocked task: a dependency lives on its own board and its own workflow decides when it is finished. A dependency absent from the map keeps the legacy literals rather than inheriting a neighbour's vocabulary — asserted directly, since a flat pair of sets would silently answer one workflow's question with another's.

Named to match the scheduler's DependencySatisfactionColumns so both sites read identically; not the same type only because that one lives in @fusion/engine, which core cannot import. Worth unifying if batch-core and batch-engine land close together.

9 tests, revert-proven: restoring the three literals fails 5 of 9. The default-board cases keep passing, so they do not pin the fix alone.

Not touched in this file's neighbourhood

async-comments-attachments.ts (9) — the archived read-only gate. getLiveTaskColumn returns "archived" as a sentinel, and six downstream comparisons test that sentinel rather than a board column, so the real guard is the one normalizer plus three raw row checks. Resolving it needs a workflow IR, and that module holds only a db/layer — no store — so threading one in changes every signature and every caller. Archived-ness also has a second durable representation (archive.archivedTasks) that the file itself treats as possibly disagreeing with the column (archived-state-inconsistent), so "just ask the archive table" is not equivalent. Flagged rather than guessed: it gates read-only enforcement on archived tasks, and being wrong either lets writes through or bricks document writes on live cards.

…ch scope

The only in-scope sites left after the other conversions. Cannot be resolved here: the package
depends on @fusion/plugin-sdk alone, and the SDK does not re-export the role helpers — no IR,
no store, no trait flags to resolve from. The real fix is the SDK exposing role flags on the
task shape it hands plugins, recorded at the site.

Census: cli + plugins + everything-else now has 0 remaining guards.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Comment thread packages/cli/src/commands/pr.ts Outdated
@gsxdsm

gsxdsm commented Jul 30, 2026

Copy link
Copy Markdown
Collaborator Author

Second batch-core offer: store.ts 11 → 0 (handoff/store-overlap-repair-lanes-for-batch-core, 18abe4b5a8)

Independent of the task-merge.ts handoff — different files, either order, cherry-pick or ignore. Based on current main. Gate 161 / 13 / 487 / 71, lint clean, --strict exits 0, backlog 399 → 388 with both.

The whole repair was inert, and every step failed the same way

All eleven guards belong to one subsystem: the overlap-blocker repair. On a renamed board it declined at the first gate, and would have declined at every later one too:

step literal what it did on a renamed board
repairable-state gate column !== "todo" refused a card sitting in the board's own hold lane — not a repairable todo state
blocker lease check in-progress / in-review && worktree saw no lease, so a live blocker read as stale
dependency checks ×2 !== "done" && !== "archived" re-blocked the card it had just unblocked, on a finished dependency
reroute search same three literals found no active holders and no queued candidates

A repair that declines everything is indistinguishable in the logs from a board with nothing to repair. That is why the test drives the real repairOverlapBlocker rather than only the extracted predicate — the first gate short-circuits, so a predicate-only test would have passed while the method stayed dead.

There was no existing test coverage for repairOverlapBlocker at all. These 9 are the first.

A fourth copy of the file-scope-lease predicate

holdsRepairFileScopeLease was declared inside findCurrentOverlapBlockerForRepair while the same logic sat inline in the blocker pre-check — two copies within one repair, which is how the two halves end up disagreeing about whether a blocker is live. Hoisted to module scope and both call sites now use it.

It is a third and fourth copy of shouldHoldActiveFileScopeLease from engine/scheduler.ts. Not unified here: that one lives in @fusion/engine, which core cannot import, so moving it is a cross-batch refactor rather than a conversion. Flagged in the code with the reason. If batch-core and batch-engine land close together it is worth one follow-up — they must keep agreeing or the repair reroutes to a blocker the scheduler ignores. A test pins the review half's extra worktree clause so the copies cannot drift silently on that detail.

The hold gate refuses rather than defaults

A workflow that resolves with no hold lane is an ANSWER — this board has nowhere repairable — so the repair declines and says so:

Task FN-1 is in backlog; its workflow declares no hold lane, so there is no repairable state

Substituting "todo" there would name a column the board does not have. That is the phantom-lane defect greptile caught on my #2775; it is the same mistake, and it is why the distinction is asserted directly rather than left to a comment.

One resolution, shared

repairIrCache is created once per repair and threaded into findCurrentOverlapBlockerForRepair as the caller's resolver, so both halves resolve a given card's lanes identically and a board spanning three workflows reads three IRs rather than one per card. Candidate resolution runs only for cards that survive the id filter.

A case my first draft got backwards, worth recording

I asserted that a dependency in the wip lane produces blockedBy: FN-3. It produces a reroute instead — an unpaused card in the wip lane holds a file-scope lease, so the repair moves the overlap blocker to it rather than reporting a dependency. The product was right and my expectation was wrong. The case now pauses that card to remove the lease, with the reason in the test, since the next person will make the same assumption.

Also in this file

isLegacyAutoMergeStampCandidate (the legacy auto-merge stamp backfill) selected nothing on a renamed board, so the reconcile reported success over zero rows. It is a synchronous row predicate over an already-loaded list, so it takes an optional resolved set with a literal default rather than resolving internally.

…iew column

PR #2775 review. Two rounds pushed this guard in opposite directions and both were right
about their own case; this holds both.

synthesizeDefaultColumns upgrades a v1 graph by emitting every default column with
`traits: []`, so a v1-upgraded workflow resolves to an EMPTY review set while its in-review
column plainly exists. Treating empty as "this board has no review lane" refused fn pr create
on every pre-v2 project — worse than the phantom lane it was fixing, and invisible to any v2
test. Empty therefore means UNEXPRESSED here and takes the legacy fallback; only a resolved,
non-empty set overrides it.

Also restores the trailing "column" in the refusal, which task.test.ts pins and which keeps
getting dropped — a message change is a user-visible behaviour change and is out of scope.

Mutation-verified: the empty-is-absent form fails the new v1 test.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@gsxdsm

gsxdsm commented Jul 30, 2026

Copy link
Copy Markdown
Collaborator Author

Real, and the more serious of the two directions this guard has been pushed. Fixed in 5b3e3c7.

Verified before changing anything: synthesizeDefaultColumns (packages/core/src/workflow-ir.ts:158-159) upgrades a v1 graph with DEFAULT_WORKFLOW_COLUMN_IDS.map((id) => ({ id, name: id, traits: [] })) — every column, no traits. So a v1-upgraded workflow resolves to an empty review set while sitting on a board whose in-review column plainly exists and is where its cards live.

Two review rounds pushed this guard in opposite directions and each was right about its own case:

  • inventing 'in-review' whenever the set is empty names a lane a v2 board may not have (the earlier finding);
  • refusing whenever the set is empty rejects every v1 workflow (this one).

The v1 case decides it. Refusing fn pr create on every pre-v2 project is a far worse regression than a misleading column name in an error string, and no v2 fixture would ever surface it — a v2 board expresses its traits, so its set is never empty. Empty means UNEXPRESSED here, not absent, and takes the same legacy fallback as an unresolvable workflow. in-review is the right answer for precisely the boards that produce an empty set, because those are the ones that declare it by convention.

The general "an empty resolved set is an answer" rule still holds elsewhere; it fails here only because the v1 upgrade path manufactures empty traits for columns that do exist. That reasoning is recorded at the site so the next round does not flip it back.

Ratchet added: does NOT refuse a v1-upgraded workflow whose synthesized columns carry no traits — the store resolves a real v1-shaped IR (every default column, traits: []) and the command must reach PR creation. Asserting mockCreatePr was called rather than the absence of an error, since the latter also passes if the guard refuses quietly for some other reason.

Mutation-verified: restoring the empty-is-absent form fails that test.

One recurring thing worth naming. This is the second time the trailing column was dropped from the refusal (must be in 'in-review' vs must be in 'in-review' column). task.test.ts:3422 pins it, so it lands as a red test rather than as a review comment. Restored, and marked at the site — a message change is a user-visible behaviour change and is out of scope for a vocabulary conversion.

Verification: tsc --noEmit exit 0, pnpm lint 0 errors, CLI task.test.ts 145 passed. The 5 failures in that file are the pre-existing runTaskImportFromGitHub / runTaskImportGitHubInteractive reds that fail identically on origin/main — untouched by this PR.

…its measured scope

Comment-only. The `status` category catches a foreign enum reached through a property
(step.status); it cannot catch one in a bare variable (`next === "archived"` where `next` is a
ReportStatus), so those land in the column backlog looking unconverted.

A full receiver-level audit of all 392 column-category sites found exactly 3 such false
positives, all in the reports plugin, all now marked. Every other receiver sampled resolved to
a genuine task column. Recorded so the remaining batches treat the number as real work rather
than re-running this hunt.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@gsxdsm

gsxdsm commented Jul 30, 2026

Copy link
Copy Markdown
Collaborator Author

Third batch-core offer: display ordering + Wake Delta ranking 5 → 0 (handoff/display-ranking-roles-for-batch-core, 307135cfdb)

task-priority.ts 3 → 0, assigned-task-ranking.ts 2 → 0. Independent of the other two handoffs. Based on current main; gate 161 / 13 / 487 / 71, lint clean, --strict exits 0.

These two convert onto column-roles.ts, and the rest of core should too where it fits

Unlike the earlier files in this batch, neither of these needed a bespoke optional set. Core already has isHoldColumnRole / isCompleteColumnRole / isReviewColumnRole / isTerminalColumnRole, and each already owns the legacy-id degraded mode. Passing undefined flags reproduces the previous behaviour byte for byte, so there is no hand-written fallback left in either file to get wrong — the fallback lives in one tested place instead of being re-derived per call site.

assigned-task-ranking.ts had a local isTerminalColumn(column) that was a literal copy of isTerminalColumnRole. That is the duplication column-roles.ts was created to end; it is now a one-line delegation.

Worth saying to the batch generally: where a role helper fits, routing through it is strictly better than another ?? legacyId parameter, because the undefined-vs-false mistake this program keeps making cannot be expressed.

Why five ordering bugs sat in two small files

Every failure here is wrong order or a miscount, never an error:

  • the hold lane loses priority ordering, so urgent work stops floating to the top of the backlog;
  • the complete lane loses recency ordering, so the newest completions are not at the top of Done;
  • the review lane stops floating actively-merging cards, so the card an operator is waiting on sits wherever priority puts it;
  • Wake Delta counts a shipped card as open assigned work, so a coordinator is asked to unblock or reassign tasks that already landed.

Nobody files "the Done column is sorted slightly wrong", which is how three of these sat in a single function.

One conversion I could NOT prove, said plainly

The renamed-hold case does not fail on revert, and I could not construct one that does. A renamed hold lane falls through to the generic comparator, which also priority-sorts first, so the hold branch and the fallback agree on every input I could build.

That conversion is a consistency fix — the three branches now ask the same kind of question — not a behaviour fix, and this file does not pin it. The case is kept because it documents the intended routing, but it is not evidence, and it is labelled that way in the test header rather than left to look like coverage.

Revert proof for the rest, measured: 4 of 8 fail — complete-lane recency, review-lane merge-float, and both Wake Delta terminal cases. The legacy-board cases pass either way; they exercise the degraded mode, not the change.

Running total across the three core handoffs

handoff file(s) guards
handoff/task-merge-lanes-for-batch-core task-merge.ts (+ moves.ts wiring) 6 → 0
handoff/store-overlap-repair-lanes-for-batch-core store.ts 11 → 0
handoff/display-ranking-roles-for-batch-core task-priority.ts, assigned-task-ranking.ts 5 → 0

22 guards, 32 tests, each set revert-proven (except the one case above, which is labelled). All three are independent commits on current main — cherry-pick in any order, or ignore any of them. Still not claiming batch-core; it has no branch or PR yet and the main worker owns it.

@gsxdsm

gsxdsm commented Jul 30, 2026

Copy link
Copy Markdown
Collaborator Author

batch-core now exists as a branch (no PR opened), and it has a live stock-board bug in it

The three handoff refs were sitting where nobody was watching, so I consolidated them onto origin/batch-core and added a fourth commit. I did not open a PR — that stays with whoever owns the batch. Adopting it is now git checkout batch-core rather than cherry-picking three refs; if the main worker wants it, take the branch and it is theirs.

Four commits, based on current main. Gate 161 / 13 / 487 / 71, lint clean, --strict exits 0, baseline parses, backlog 399 → 362.

The new one is NOT a renamed-board bug — it is a column that no longer exists

task-update.ts: adding a new dependency to a hold-lane card re-seeds it for re-specification, and the destination was the literal "triage". U11 (#2515) deleted that column. The default lineage is todo | in-progress | in-review | done | archived.

So on a stock board, with no renaming anywhere, adding a dependency to a todo card moves it into a column nothing declares and nothing renders. The card leaves its lane and appears in none — recoverable only by moving it back by hand.

This is the first site in my share of the batch where the old behaviour cannot be preserved, because the column it targeted is gone. The card now stays put when the intake lane is where it already is (the default board's case: intake and hold are both todo), while the status reset and the log entry still record the re-specification. On a board where intake and hold are genuinely different columns, the move happens and goes somewhere that exists.

No literal fallback for the DESTINATION. When the workflow will not resolve, the column is left alone. Refusing to move is recoverable; writing a column that may not exist is what caused this. The source-lane check keeps its documented literal default, because failing to recognise the hold lane only skips the re-seed — a much cheaper direction to be wrong in. The asymmetry is deliberate and marked at both sites.

Second site in the same file: clearing the paused status on unassignment was keyed on the wip/review literals, so a renamed board left status: "paused" behind after the pause itself was lifted — a card that is not paused but says it is, in every status-driven surface, permanently.

update-task-deps.ts: two literals that are load-bearing, now marked

Its legacy-id union is not leftover — it is what lets a dependency row stored in a column its workflow no longer declares still read as SATISFIED. Deleting it strands every dependent permanently. The reasoning was already in the comment; the DELIBERATE-LITERAL marker is what stops the census listing it as unconverted work a future pass might "finish".

A harness note worth copying

The updateTaskUnlockedImpl test kept failing on unrelated store methods (atomic writes, agent-link sync, lifecycle emission) — three rounds of whack-a-mole. It now wraps the mock in a Proxy answering every unlisted method with an async no-op, so only the methods whose return value the assertions depend on are written out. Worth reusing for the remaining task-store/* files, which have the same long tail.

Branch contents

commit file(s) guards
1b5770d4a1 task-merge.ts + moves.ts wiring 6 → 0
15b667ee06 store.ts 11 → 0
6169d97434 task-priority.ts, assigned-task-ranking.ts 5 → 0
e55b2b7ce8 task-update.ts, update-task-deps.ts 5 → 0

27 guards, 37 tests. Every set revert-proven except the one renamed-hold case labelled as unprovable in display-ranking-roles-resolved.test.ts — that one is a consistency fix and says so rather than reading as coverage.

gsxdsm added a commit that referenced this pull request Jul 30, 2026
… 92) (#2780)

## batch-core — `packages/core` + `packages/dashboard/src`

Shared branch: two workers are converting into it. Opening the PR
because the branch was green with none, and a branch without a PR merges
nothing.

### Census

Measured with `node scripts/lifecycle-column-census.mjs --json`.

| | guards |
|---|---|
| batch-core scope at branch point | 129 |
| batch-core scope now | **92** (51 files) |
| repo total now | 358 |

Files closed so far: `store.ts` 11→0, `task-merge.ts` 6→0,
`live-agent-count.ts` 6→0 (marked, not converted — see #2762),
`task-update.ts` 3→0, display-ordering + Wake Delta ranking 5→0,
`register-git-github.ts` 4→0.

### The `register-git-github.ts` slice

Three PR routes — `pr/create`, `pr/push-branch`, `pr/resolve-conflicts`
— plus the `CHANGES_REQUESTED` handler each compared `task.column !==
"in-review"`. On a renamed board **none** of them matched, so every PR
affordance the dashboard offers was refused for a card sitting in the
lane that board calls review, and the refusal named a column that does
not exist there.

All four now share one helper, `reviewColumnsForTask`, which gets two
things right that this program has repeatedly gotten wrong:

- **Membership, not a single id.** It takes the broad review set
(`mergeOrchestration ∪ mergeBlocker ∪ humanReview`).
`resolveLifecycleColumns` returns the *first* column per trait, so a
single-id answer silently ignores a board that declares a merge lane
**and** a separate human sign-off lane. These guards only refuse or
permit — they never move the card — so over-admitting costs nothing
while under-admitting refuses a request that should have worked.
- **An empty resolved set means UNEXPRESSED, not absent.**
`synthesizeDefaultColumns` upgrades a v1 graph by emitting every default
column with `traits: []`, so a v1-upgraded workflow resolves to an empty
review set while its `in-review` column plainly exists and holds the
card. Reading empty as "this board has no review lane" would refuse
these routes on **every pre-v2 project** — a worse regression than the
one being fixed, and invisible to any v2 test.

This is the dashboard twin of the `fn pr create` guard in
`packages/cli/src/commands/pr.ts` (#2775). The two surfaces answer the
same question and now agree — FN-5893 surface enumeration.

### Testing note: why the seam and not the routes

I wrote route-level HTTP tests first and **deleted them**. An express
fixture over `registerGitGitHubRoutes` hangs — every case, including the
pure refusals, times out at 4s, because registering the router starts
background work the fixture never satisfies. Making it run would mean
mocking git, the GitHub client, and the pollers: a mock-the-world shell,
which is what the project's do-not-add-slow-tests rule (FN-5048) says to
avoid in favour of a narrow seam.

`reviewColumnsForTask` *is* the narrow seam — it holds the entire
decision, and the four call sites now do nothing but ask it and render
its answer. Six cases pin it: the renamed lane is returned and
`in-review` is not, a two-lane board returns both, a v1-upgraded board
falls back, an unresolvable workflow falls back, and the refusal renders
lanes an operator can act on.

**Mutation-verified, both directions:** reverting the helper to the
legacy literal fails 2 of 6; treating an empty set as an answer fails 1
of 6.

One fixture bug worth recording, since it would have made the two-lane
case vacuous: the trait id is kebab-case `human-review`, not
`humanReview`, and the built-in traits must be registered via `import
"@fusion/core"` before flags resolve.

### Verification

- `pnpm --filter @fusion/dashboard exec tsc --noEmit -p tsconfig.json` →
0 errors
- `pnpm lint` → 0 errors
- `register-git-github.review-lanes.test.ts` → 6 passed

---------

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
@gsxdsm

gsxdsm commented Jul 30, 2026

Copy link
Copy Markdown
Collaborator Author

I triaged the 84 literal-list sites I flagged last round. The shape-based list is only ~50% precise — here is the honest version.

batch-core (#2780) merged with all five of my feeds in it, so this is follow-through on the one thing I explicitly left un-triaged.

Method and what it cost

I spot-read four candidates that looked like unconverted gates. Two were already-documented fallbacks:

  • branch-group-ops.ts:166 — the new Set(["done","archived"]) is a legacy SEED that the lines directly below union with resolved columns. Reads as a hard-coded gate; is not one.
  • useTasks.ts:158 — the comment above it already records that every consumer additionally requires an INTAKE lane, so the extra ids are filtered downstream.

Two were real, and I verified both by reading the use site, not the declaration:

  • agent-assignment.ts:84 — fixed, below.
  • taskProgress.ts:262REVIEW_LANE_COLUMNS gates the code-review / browser-verification badge. Cosmetic, dashboard-app's to take.

So: the grep finds the class, but it cannot tell a hard-coded gate from a documented legacy seed unioned with resolved values. Publishing "84 sites" as a work list would have been wrong by roughly half. The sound method is reading the USE site; the declaration tells you nothing.

The triaged list, for whoever wants it

Excluded as definitions or false positives: COLUMNS/VALID_TRANSITIONS/TUI_BUCKETS (board definitions), roles: ["triage"] in prompt-overrides.ts + builtinPrompts.ts (that is AgentCapability, the planning agent, not the deleted column), traitIds: ["archived", …] (trait names), TERMINAL_SESSION_STATES (session states), and every LEGACY_* constant already paired with a resolved path.

Remaining candidates worth a read — unverified beyond shape, listed so nobody re-derives the grep:

engine     self-healing.ts:3004,6081 · executor.ts:12452 · eval-followups.ts:12
           ephemeral-worker-manager.ts:49 · worktree-pool.ts:1176
dash/src   routes.ts:1318 · register-project-routes.ts:923
dash/app   taskProgress.ts:210 · TaskDetailModal.tsx:704 · DocumentsView.tsx:110
           MissionControlPanel.tsx:71 · SystemStatsArea.tsx:34 · useSessionFiles.ts:4
           TaskCard.tsx:2741 · TaskContextMenu.tsx:293

Expect ~half to be false positives on the same pattern.

The one I fixed: load-balancing was defeated (handoff/agent-assignment-load-lanes, 87605b5697)

ACTIVE_COLUMNS gates the per-agent assignment-load tally used to pick the least-loaded agent. On a renamed board no column matched, so assignmentLoad stayed empty, every candidate compared as load 0, and the sort fell through to its stable createdAt tiebreak — so the same agent wins every assignment while the rest sit idle.

Nothing logs and nothing fails. It reads as one agent being busy, not as a bug.

Based on current main (not on origin/batch-engine, which forked at 86a2b48968 and is behind main — feeding it would have put this on a stale base). 3 tests; restoring the hard-coded Set fails the renamed case. Gate 161 / 13 / 487 / 71, lint clean, --strict exits 0.

Standing status

#2775 is my only open PR: green locally, greptile SUCCESS, zero unresolved threads, checks queued. Everything else I have produced today is merged or offered on a handoff ref.

@gsxdsm

gsxdsm commented Jul 30, 2026

Copy link
Copy Markdown
Collaborator Author

Verified the candidate list by reading USE sites. 5 of 5 checked are real — and one of them deletes live workers.

Follow-through on the previous comment: I said the shape-based list was ~50% precise and the remainder was unverified. This round reads the use site for five of them. All five are genuine unconverted gates with no resolved path — so the earlier 50% figure came from declarations that looked convertible; once a LEGACY_* name or an adjacent union is ruled out, the remainder holds up.

site gate direction of failure on a renamed board
ephemeral-worker-manager.ts:342 zombie sweep deletes a LIVE worker — fixed below
eval-followups.ts:109 open-task set for follow-up dedup sees zero open tasks → files duplicate follow-ups
routes.ts:1318 agent "working on" staleness finished cards still shown as active work
useSessionFiles.ts:44 session-file loading files never load for a renamed active lane
worktree-pool.ts:1205 merger-managed skip fails SAFE — the skip does not fire, so more branches are protected, not fewer

worktree-pool is worth calling out precisely because it goes the harmless way. The instinct is to assume every miss is dangerous; here a missed match means a branch is added to activeBranches and therefore protected from cleanup. Stale branches accumulate — annoying, not destructive. Stating direction rather than assuming it is the difference between a bug report and an alarm.

The one that destroys work: handoff/ephemeral-zombie-sweep-lanes (ad9a31b2d2)

shouldDeleteOnSweep tested a hard-coded terminal Set first, then fell through to:

return task.column !== "in-progress";

On a renamed board both halves miss, and they compound in the worst order: the terminal test fails, control reaches the fallthrough, and "building" !== "in-progress" is true. An ephemeral worker actively executing a task is classified as a zombie and deleted by the sweep — work in flight destroyed, no error logged.

Census-invisible on both halves: the terminal check is a Set literal (definition, not comparison), and the wip literal was only reachable after it. Nothing in the backlog pointed at this file; the grep did.

The fallback is deliberately asymmetric and the comment says why: an unresolvable workflow keeps the legacy literals rather than guessing. Failing to reap a dead worker costs a slot; reaping a live one destroys work. Those are not symmetric, so the uncertain case fails toward keeping the worker.

3 tests — keep a renamed wip worker, still reap a renamed complete one, still reap a renamed hold one. The two "still reaps" cases exist because keeping everything would be its own leak. Revert proof: restoring the literal pair fails the renamed-wip case.

Based on current main, not origin/batch-engine — that branch forked at 86a2b48968 and is behind main, so feeding it would stack this on a stale base. Gate 161 / 13 / 487 / 71, lint clean, --strict exits 0.

Two open handoffs for the engine owner

  • handoff/agent-assignment-load-lanes (87605b5697) — load-balancing defeated, same census-invisible class
  • handoff/ephemeral-zombie-sweep-lanes (ad9a31b2d2) — this one

Both are single commits on current main and independent of each other. Remaining verified-real and unclaimed: eval-followups.ts:109, routes.ts:1318 (dashboard server), useSessionFiles.ts:44 (dashboard app).

@gsxdsm

gsxdsm commented Jul 30, 2026

Copy link
Copy Markdown
Collaborator Author

Two more of the verified list fixed: handoff/verified-literal-list-gates (d6fd120ade)

Two independent commits on current main, cherry-pickable separately. Gate 161 / 13 / 487 / 71, lint clean, --strict exits 0.

eval-followups.ts — duplicate task creation

OPEN_COLUMNS matched nothing on a renamed board, so openTasks was empty and the dedup had no live work to compare against. Every eval run re-filed follow-ups it had already filed. The symptom is duplicate cards, which reads as the evaluator being thorough rather than as a bug.

A correct helper was already imported in this file. My first draft manufactured synthetic trait flags from lane equality to call isTerminalColumnRole — the exact anti-pattern I flagged in task-update.ts two commits earlier: a longer way to write the same comparison while looking like it consulted the trait registry. resolveTerminalColumnsFor was on line 10, already used by the candidate loop 130 lines below, and it unions resolved terminals with the legacy pair for a documented degraded-IR reason. Replaced before commit; recording it because I have now reached for the synthetic-flags shortcut twice.

Two tests: a duplicate in a renamed WIP lane is suppressed; a card in a renamed TERMINAL lane does not suppress (the dedup must stay scoped to live work, or a finished card blocks every future follow-up). Revert proof: restoring OPEN_COLUMNS fails the first.

dashboard/src/routes.ts — stale "working on" indicators

sanitizeAgentTaskLinks drops taskId from an agent response when the linked task is finished. Keyed on a hard-coded Set, a finished card on a renamed board kept its agent's indicator lit — the agent list advertised work that had already shipped, which is precisely what the sanitizer exists to prevent. Now resolves per linked task with one shared IR cache; the literal pair remains as the documented unresolvable-workflow fallback.

Its guard is STRUCTURAL, and I am flagging that rather than letting it read as coverage. sanitizeAgentTaskLinks is a closure inside createApiRoutes, reachable only by building the full express app and driving GET /api/agents. That harness exists (routes-automation.test.ts) but standing it up to re-assert a per-task resolver is a lot of machinery around a small seam, and I did not write it. The ratchet asserts the source — resolver threaded per task, bare literal call gone, cache shared, fallback retained — and fails on revert, verified by reverting. Whoever owns the dashboard server should add the behavioural case if this seam grows.

Where the verified list stands

site status
ephemeral-worker-manager.ts:342 fixed — handoff/ephemeral-zombie-sweep-lanes (ad9a31b2d2)
agent-assignment.ts:84 fixed — handoff/agent-assignment-load-lanes (87605b5697)
eval-followups.ts:109 fixed — this branch
routes.ts:1318 fixed — this branch
useSessionFiles.ts:44 open, dashboard-app: session files never load for a renamed active lane
worktree-pool.ts:1205 open, fails safe — over-protects branches from cleanup; low priority

Four handoff branches now, all single-purpose and based on current main: ephemeral-zombie-sweep-lanes, agent-assignment-load-lanes, verified-literal-list-gates, plus the older project-engine-lanes-for-batch-engine. Happy to fold them into one branch or a PR on request — I have kept them separate because they cross package ownership and each stands alone.

@gsxdsm

gsxdsm commented Jul 30, 2026

Copy link
Copy Markdown
Collaborator Author

Batch-owner note from batch-dashboard-app: MERGE main into your branch, do not rebase onto it. You will hit this the moment main moves.

scripts/lib/lifecycle-column-census-baseline.json is rewritten by every --strict run, not just --update-baseline — that is the self-lowering ratchet working as intended. So nearly every batch commit touches it.

Rebasing a long batch branch therefore replays every commit and re-conflicts on that file at each step. Worse, once a few steps resolve differently, commits inside your branch start conflicting with each other on real source files — I hit a DockTaskList.tsx conflict between two batch commits that had never conflicted when they were written. Two full rebase attempts made no progress.

Merging resolves once:

git merge origin/main --no-edit
git checkout origin/main -- scripts/lib/lifecycle-column-census-baseline.json
node scripts/lifecycle-column-census.mjs --strict --update-baseline
git add scripts/lib/lifecycle-column-census-baseline.json && git commit --no-edit

One conflict, mechanical resolution. Only the tip's baseline has to be correct; intermediate ones never mattered.

Two smaller traps from the same session:

  • --strict writes. If you redirect its stdout to /dev/null in a loop you will miss the "baseline rewritten downward, COMMIT IT" line and be left with a dirty tree that aborts your next git checkout. I misdiagnosed this as the tool being silent; it isn't, I had silenced it.
  • Branch-switching in a shared worktree can leave tracked binaries (e2e/__screenshots__/*.png) looking modified. Restore them rather than committing them into the batch.

Written up in docs/solutions/workflow-learnings/mega-batch-branches-merge-do-not-rebase.md on batch-dashboard-app.

@gsxdsm

gsxdsm commented Jul 30, 2026

Copy link
Copy Markdown
Collaborator Author

The verified list is now closed out: handoff/session-files-lane-roles (6473a31ded)

useSessionFiles.ts 1 → 0. Gate 161 / 13 / 487 / 71, lint clean, --strict exits 0.

On a renamed board ACTIVE_COLUMNS matched nothing, so the fetch never fired and the Files tab was permanently empty for every card that had one. An empty file list is indistinguishable from a task that touched no files — the reason this would never be reported.

Converted onto columnRoles.ts (the dashboard-app twin of core's helpers), so the legacy-id degraded mode stays in one tested place.

One design detail worth keeping

The flags are collapsed to a boolean before the effect, not passed into the dep array. columnFlags is an object, and a caller constructing it inline hands the hook a fresh identity every render — putting it in the deps would refetch on every parent render. A test asserts exactly that: an equal-but-not-identical flags object on re-render must not trigger a second fetch. That case exists because the obvious implementation is the wrong one, and nothing else would have caught it.

A test that passed in isolation and failed in the suite

My negative case (does NOT fetch for a hold lane) asserted not.toHaveBeenCalled(). It passed alone and failed in the file — hooks are not unmounted between cases here, so a prior case's in-flight fetch lands inside this one.

That is the classic shape of a test that gets "fixed" by reordering. Rewritten to assert a delta against the call count captured before render, which is independent of what leaked in. Same discipline as the createdAt epoch-ms mistake earlier: the fix is making the assertion independent, not making the suite tidy enough for the assertion to hold.

Verified literal-list list: closed

site status
ephemeral-worker-manager.ts:342 fixed — handoff/ephemeral-zombie-sweep-lanes (ad9a31b2d2) — deleted LIVE workers
agent-assignment.ts:84 fixed — handoff/agent-assignment-load-lanes (87605b5697) — load-balancing defeated
eval-followups.ts:109 fixed — handoff/verified-literal-list-gates (d6fd120ade) — duplicate cards
routes.ts:1318 fixed — same branch — stale "working on" indicators
useSessionFiles.ts:44 fixed — this branch — Files tab always empty
worktree-pool.ts:1205 left open, deliberately

worktree-pool is the one I am NOT fixing. It fails SAFE: a missed match means the skip does not fire, so the branch is added to activeBranches and protected from cleanup. The cost is stale branches accumulating, not deletion. It is also inside the merger's branch-reaping path, where being wrong in the other direction destroys work — so it deserves its owner's judgement rather than a drive-by conversion from someone who has not read that subsystem. Flagged, not guessed.

Five handoff branches, all single-purpose, all on current main, all independent: ephemeral-zombie-sweep-lanes, agent-assignment-load-lanes, verified-literal-list-gates, session-files-lane-roles, and the older project-engine-lanes-for-batch-engine. Say the word and I will fold them into one PR.

@gsxdsm

gsxdsm commented Jul 30, 2026

Copy link
Copy Markdown
Collaborator Author

Consolidated: handoff/lifecycle-invisible-gates-all (86680d1220) — five fixes, one checkout

The five single-purpose handoffs have been sitting unadopted while main moved. Rather than leave verified fixes on refs nobody is watching, they are now one branch, rebased onto current main (31b9fafe11), verified together rather than only individually. The individual branches remain if anyone wants to take a subset.

This is the same move that got batch-core adopted: consolidation turned five cherry-picks into one git checkout. Still not opening a PR#2775 is my one open PR and I am keeping that discipline.

Verified as a set

check result
pnpm test:gate 161 / 13 / 487 / 71
engine suites (assignment, ephemeral, eval-followups) 44 passed
dashboard suites (agent-task-link, useSessionFiles) 16 passed
tsc engine + dashboard server + dashboard app clean
pnpm lint · census --strict clean · exits 0

What is in it

commit defect on a renamed board
beb107a7bc assignment load-balancing defeated — every candidate reads as load 0, so one agent wins every assignment while the rest idle
cf4b59e1cb the zombie sweep deletes LIVE ephemeral workers — two literals compound so an actively-executing worker is reaped
5fe004ae64 eval follow-up dedup sees zero open tasks, so every run re-files follow-ups it already filed
a1021de8b2 agents keep a "working on" indicator for finished cards
86680d1220 the Files tab never loads — the fetch never fires

Every one of these was invisible to the census: each gate is a Set or array literal — a definition, not a comparison — so no backlog entry ever pointed at any of these files. They were found by grepping for lane-shaped list literals after the same shape surfaced in duplicate-intake and blocker-fanout, then confirmed by reading each USE site.

Two of the five destroy or duplicate work rather than degrading a display: cf4b59e1cb (work in flight destroyed) and 5fe004ae64 (duplicate cards). Those are the two I would take first if only some can land.

Still deliberately not fixed

worktree-pool.ts:1205 — it fails safe (a missed match protects a branch from cleanup rather than deleting it), and it sits in the merger's branch-reaping path where the opposite error destroys work. That needs its owner's judgement, not a drive-by conversion. Flagged, not guessed.

Status

#2775 remains my only open PR: green locally, greptile SUCCESS, zero unresolved threads, checks queued. Everything else I have produced is merged or on this branch.

@gsxdsm

gsxdsm commented Jul 30, 2026

Copy link
Copy Markdown
Collaborator Author

Opened the consolidated handoff as #2787 rather than leave five verified fixes stranded on refs across more cycles — two of them destroy or duplicate work (live ephemeral workers reaped; duplicate follow-up cards).

This is a reversible call and I am flagging it rather than hiding it: it puts me at two open PRs, against the one-per-worker rule. Close #2787 if that breaks queue policy and the branch keeps them safe. I judged five stranded fixes worse than one extra CI run, but the ordering here is yours, not mine.

This PR (#2775) is unchanged and still first in line: green locally, greptile SUCCESS, zero unresolved threads.

@gsxdsm

gsxdsm commented Jul 30, 2026

Copy link
Copy Markdown
Collaborator Author

Check your batch for inert conversions before it lands. I just found five in one of my own tranches, and nothing we run would have caught them.

The class

A conversion that replaces a literal with a resolved-flags seam the callers never supply:

// utils
export function isTaskStuck(task, timeoutMs, dataAsOfMs, columnFlags?) {  }
isTaskStuck(task, timeoutMs, lastFetchTimeMs);        // ← every caller. columnFlags is always undefined.

// components
interface PrPanelProps { taskColumnFlags?:  }         // declared
<PrPanel taskColumnFlags={flags} />                    // parent passes it
export function PrPanel({ taskColumn }: PrPanelProps)  // ← never destructured. Dropped.

Why nothing catches it

  • tsc passes — the parameter/prop is optional, so omitting it is legal.
  • Tests pass — the fallback is the old behaviour. That is the point of the fallback.
  • The census goes DOWN — it counts comparisons, and the literal really is gone. The instrument measuring this program scores the inert version as a win.
  • Grep is useless — searching the symbol finds it in exactly the places you added it, and says nothing about whether a value flows.

My hit rate, which is why I am writing to you

In the tranche I had reported as clean: 5 of 9 conversions were inert. Two were found by review (worktreeGrouping, PrPanel); the other three only by building the guard. I had verified each one by "it compiles and the suite is green".

Detection

Two AST checks, both cheap — packages/dashboard/app/__tests__/resolved-flags-seams-have-suppliers.test.ts on batch-dashboard-app if you want the source:

  1. Arity — for every exported function whose last parameter matches /[Cc]olumnFlags$/, require at least one call site passing that many arguments. Catches the utils/hooks shape, which is most of core and engine.
  2. Destructuring — for every <Name>Props declaring such a prop, require the function <Name> to destructure it. Catches the component shape.

Note my first attempt at (2) could not fire: it checked whether the prop name appeared as a JSX attribute anywhere, and the parent legitimately passes it, so the name was present while the value was still dropped one level down. A name-level check cannot see a component-level drop — verify your guard fails on a reintroduced defect before trusting it.

The rule that falls out

Wire a supplier, or delete the seam and leave the literal counted. I reverted three of mine for exactly this: their callers genuinely cannot supply flags (one sits 60 lines above where the flags are derived). An optional parameter every caller omits is strictly worse than the literal — the literal is honest and stays in the census, while the parameter banks the credit and runs the fallback forever.

My package went from a reported 2 back up to 6 as a result. The larger number is the true one.

… Lint on #2775

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@gsxdsm

gsxdsm commented Jul 30, 2026

Copy link
Copy Markdown
Collaborator Author

Ran the inert-seam scan repo-wide. 24 optional trailing lane/flag parameters exist across packages/; 5 had no supplier anywhere. Four are in your batches.

Method: AST, not grep — find exported functions whose last parameter is optional and matches /([Cc]olumnFlags|[Cc]olumns|[Ll]anes)$/, then require at least one call site passing that many arguments. An optional trailing parameter every caller omits is the inert conversion: the literal is gone, the census banked the credit, the fallback runs forever.

Confirmed and fixed on my side (core-declared, dashboard callers):

  • core/src/near-duplicate-canonical.ts: isNearDuplicateCanonicalInactive() 1/2 — three dashboard callers, none passed the flags, so a canonical in an active lane read as inactive and the duplicate badge stopped warning about a live twin. Two wired (both already had flags in hand — one defined the resolver two lines below the call it wasn't using); the third is scope-blocked and recorded.

Remaining four, in your packages:

site supplied
core/src/task-timing.ts: getTotalAgentActiveMs() 2/3
core/src/default-workflow-hooks.ts: evaluateMergeBlockerGuard() 0/4
engine/src/restart-recovery-coordinator.ts: isRecoverableMissingWorktreeReviewFailure() 0/2
engine/src/runtimes/in-process-runtime.ts: isPlanningContinuationTaskDispatchable() 1/2

getTotalAgentActiveMs is worth a look first: it is the core twin of a dashboard function I just reverted for exactly this — same name, same trailing-flags shape, same absent supplier. Two people made the same mistake in two packages.

Read 0/N sceptically. My scanner excludes __tests__, so a function called only from tests reports zero call sites and may be a false positive. 2/3 and 1/2 are the high-confidence rows — there are real callers and they are omitting the argument.

The rule when you look: wire a supplier, or delete the parameter and leave the literal counted. I reverted three of mine on that basis; my package's count went from a reported 2 back up to 6, and 6 is the honest number.

Scanner is ~40 lines of ts.createSourceFile; happy to hand it over or land it as a scripts/check-*.mjs gate if you want it enforced rather than sampled.

@gsxdsm

gsxdsm commented Jul 30, 2026

Copy link
Copy Markdown
Collaborator Author

Heads-up: batch-dashboard-app (#2772) adds a new blocking check to pnpm test:gate. It will not fail your branch today, but you should know it exists before it surprises you.

scripts/check-inert-flag-seams.mjs — 0.8s, static, alongside the existing check-no-nohup / check-capacity-pool-id family. It fails when an exported function has an optional trailing lane/flag parameter that no caller anywhere supplies.

Why it is worth a gate slot. That shape passes tsc (the parameter is optional), passes the full suite (the fallback is the old behaviour), and makes the lifecycle-column census go down (it counts comparisons; the literal really is gone). Every instrument we have scores it as a win. Five of nine conversions in one reviewed-and-green tranche of mine were this.

Your packages are allow-listed, so you are not blocked:

  • core/src/task-timing.ts: getTotalAgentActiveMs() — 2 of 3
  • engine/src/runtimes/in-process-runtime.ts: isPlanningContinuationTaskDispatchable() — 1 of 2

Both entries are marked TEMPORARY with your PR numbers. Remove the entry when you wire or delete the parameter, and the check starts guarding those files. If you would rather I revert the allow-list and let it block until they are fixed, say so — I defaulted to not breaking your CI mid-batch.

Two more entries are exported-for-tests-only functions, where the scanner (which excludes __tests__) reports zero callers — false positives, listed rather than silently skipped.

One caveat about the check itself, since you may write similar ones. My first version found nothing: I reused a $-anchored parameter-name regex as the whole-file pre-filter, so it never matched and the scan was silently empty. It would have shipped permanently green. What caught it was an assertion that fails when the scan finds zero seams. If you build a guard, add the equivalent — a guard that cannot fire is indistinguishable from a clean codebase.

gsxdsm added a commit that referenced this pull request Jul 30, 2026
…l workers reaped and duplicate follow-up cards (#2787)

Five lifecycle-column fixes the census **structurally cannot see**. Each
gate is a `Set` or array literal — a *definition*, not a comparison — so
no backlog entry ever pointed at any of these files. Found by grepping
for lane-shaped list literals after the same shape surfaced in
`duplicate-intake` and `blocker-fanout` (both merged via #2780), then
confirmed by reading each USE site.

**On opening this:** I offered twice to fold these into a PR and kept
them on handoff refs to respect one-open-PR-per-worker. They have now
sat unadopted across several cycles while `main` moved, and two of them
destroy or duplicate work. Opening is the reversible call — **close it
if it breaks queue policy** and I will keep them on the branch.

## What is in it

| commit | defect on a renamed board | severity |
|---|---|---|
| `beb107a7bc` | assignment load-balancing **defeated** —
`assignmentLoad` stays empty, every candidate reads as load 0, the sort
falls through to its stable `createdAt` tiebreak, so **one agent wins
every assignment** while the rest idle | distribution |
| `cf4b59e1cb` | the zombie sweep **deletes LIVE ephemeral workers** |
**destroys work** |
| `5fe004ae64` | eval follow-up dedup sees **zero open tasks**, so every
run re-files follow-ups it already filed | **duplicate cards** |
| `a1021de8b2` | agents keep a **"working on" indicator for finished
cards** | stale UI |
| `86680d1220` | the **Files tab never loads** — the fetch never fires |
silent empty |

### The one that destroys work

`shouldDeleteOnSweep` tested a hard-coded terminal `Set`, then fell
through to `return task.column !== "in-progress"`. On a renamed board
**both halves miss, and they compound in the worst order**: the terminal
test fails, control reaches the fallthrough, and `"building" !==
"in-progress"` is `true`. An ephemeral worker **actively executing a
task** is classified as a zombie and deleted. Nothing logs.

Its fallback is **deliberately asymmetric**, and the comment says why:
an unresolvable workflow keeps the legacy literals rather than guessing.
Failing to reap a dead worker costs a slot; reaping a live one destroys
work in flight. Those are not symmetric, so uncertainty fails toward
keeping the worker.

## Verification

Verified **as a set**, not only per-branch:

- `pnpm test:gate` — **161 / 13 / 487 / 71**
- engine suites (assignment, ephemeral, eval-followups) — **44 passed**
- dashboard suites (agent-task-link, useSessionFiles) — **16 passed**
- `tsc` engine + dashboard server + dashboard app — clean
- `pnpm lint` clean · census `--strict` exits 0

**Revert-proven individually.** Restoring each literal fails its own
case: the renamed-wip zombie case, the renamed-wip assignment case, the
renamed-lane dedup case, the sanitizer ratchet, and both
`useSessionFiles` role cases.

## Two honesty notes, flagged rather than buried

**`a1021de8b2`'s guard is STRUCTURAL, not behavioural.**
`sanitizeAgentTaskLinks` is a closure inside `createApiRoutes`,
reachable only by standing up the full express app. The ratchet asserts
the source — resolver threaded per task, bare literal call gone, cache
shared, fallback retained — and **fails on revert**, verified. It is not
a substitute for a behavioural test; whoever owns the dashboard server
should add one if that seam grows.

**`useSessionFiles`'s negative case passed in isolation and failed in
the suite.** Hooks are not unmounted between cases there, so a prior
case's in-flight fetch landed inside it. That is the classic shape of a
test that gets "fixed" by reordering; it now asserts a **delta** against
the pre-render call count, which is independent of what leaks in.

## Deliberately NOT included

`worktree-pool.ts:1205` — the sixth site from the same sweep. It **fails
safe**: a missed match means the skip does not fire, so the branch is
added to `activeBranches` and *protected* from cleanup. The cost is
stale branches accumulating, not deletion. It also sits in the merger's
branch-reaping path, where the opposite error destroys work, so it
deserves its owner's judgement rather than a drive-by conversion.
Flagged, not guessed.

Also still open and unclaimed: roughly 69 untriaged literal-list sites
across engine/dashboard/cli. The grep is one line and the file list is
on #2775 — with the measured caveat that about half are false positives
on shape alone (`LEGACY_*` names, seeds unioned with resolved values,
and `roles: ["triage"]`, which is an `AgentCapability`, not the deleted
column). Only the use site settles it.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

---------

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
@gsxdsm
gsxdsm merged commit 4184fde into main Jul 30, 2026
7 checks passed
@gsxdsm
gsxdsm deleted the batch-cli-plugins branch July 30, 2026 17:38
gsxdsm added a commit that referenced this pull request Jul 30, 2026
…wn PR superseded (#2801)

## New red on main

#2775 landed and put one failure on `main`, in a test that PR itself
added:

```
pr-create-review-lane-resolved.test.ts
  > refuses WITHOUT naming a phantom lane when the workflow declares no review lane
AssertionError: expected undefined to be defined
```

## Two review rounds pushed `pr.ts` in opposite directions; the test is
from the losing one

| round | decision |
|---|---|
| **1** (greptile P2) | a resolved workflow with no review-trait column
is an **answer** — do not invent `'in-review'`, say *"no review lane"*.
**This test was written against that.** |
| **2** (greptile) | refusing on an empty set rejects **every v1
workflow**, because `synthesizeDefaultColumns` upgrades a v1 graph by
emitting every column with `traits: []` — so a v1 board whose
`in-review` column plainly exists resolves to an empty review set. |

**Round 2 shipped** (`pr.ts:206-207`) and is right: an empty set is
indistinguishable from a v1 upgrade, so it means *unexpressed* rather
than *absent* and takes the same legacy fallback as an unreadable
workflow. Both rounds are extensively documented in `pr.ts` — the code
is deliberate and I have not touched it.

The consequence is simply that **there is no "no review lane" message in
the shipped code at all**, so `errors.find((e) => e.includes("no review
lane"))` returned `undefined`. The test could never have passed against
what merged.

## The fix

Re-pointed at the contract that actually shipped: the filtered board
takes the legacy `'in-review'` fallback, and the refusal must **not**
name the renamed lanes (`signoff`, `waiting-on-a-human`) that this board
no longer declares — which preserves the anti-phantom-lane intent the
test was named for.

## Flagged, not guessed

The round-1 behaviour is **not recoverable** without a way to
distinguish *"v2 board that declares no review lane"* from *"v1 board
whose traits were synthesised empty"*. The IR does not currently carry
that signal, so emitting a distinct message would re-break every pre-v2
project — the exact regression round 2 caught. Recorded in the test
rather than invented.

## Evidence

Mutations, both caught:

| mutation | result |
|---|---|
| fallback names lanes the board lacks | **1 failed** |
| the review-lane gate removed entirely | **2 failed** |

Full CLI package **1684 passed / 106 skipped (126 files)** — was 1
failed. Gate **732 green** · lint clean. Test-only; `pr.ts` restored
clean after the mutations.

## How this was found

Pre-flighting the open batch PRs against current `main` rather than
their branch heads, after batch-engine's previous landing put 32
failures on main that were only caught post-merge. #2785 and #2783 both
came back clean (commented on each); re-running `main` itself after the
newest landings surfaced this one.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

---------

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
gsxdsm added a commit that referenced this pull request Jul 30, 2026
…line (#2811)

**`main` is RED on the lifecycle ratchet right now.** `node
scripts/lifecycle-column-census.mjs --strict` exits **1** on pristine
`origin/main`, which is the `Lint` job's *Lifecycle-column ratchet* step
— so **every open PR fails Lint** until this lands, regardless of its
own contents.

Verified on a detached checkout of `origin/main`, not on a branch of
mine.

## Cause

Eight `DELIBERATE-LITERAL` markers were added across seven files without
re-recording the baseline:

```
packages/core/src/task-move-disposer.ts            (in-progress, todo)
packages/core/src/task-store/archive-lifecycle-2.ts (archived)
packages/dashboard/src/github-tracking-comments.ts  (done)
packages/dashboard/src/gitlab-tracking-comments.ts  (in-progress)
packages/dashboard/src/server.ts                    (archived)
packages/dashboard/src/task-planner-chat-context.ts (done)
packages/dashboard/src/test/mockCoreEngine.ts       (in-review)
```

Adding a marker RECLASSIFIES a site (column-guard → deliberate), so the
tracked deliberate totals move and `--strict` fails until the baseline
records the new shape. It is the same mechanism that turned #2775 red
earlier today — a marker landing without its baseline — which is worth
noting because it has now happened twice from different PRs.

## The fix

Baseline re-recorded, nothing else. Zero source changes; the diff is one
derived file.

- `--strict` exits **0**
- `pnpm test:gate` — **161 / 13 / 487 / 71**
- `pnpm lint` clean

## Worth a follow-up by whoever owns the ratchet

The failure is structural rather than careless: a PR that adds a marker
is *doing the right thing*, and the baseline requirement is only
discovered when CI goes red — after merge, for everyone else. Two
options, neither of which I am taking unilaterally on a red-main fix:

1. have `--strict` treat a marker-only reclassification as an accepted
rise (it is not new debt — the count of unconverted guards goes
**down**);
2. or fail the PR that adds the marker, by comparing against the base
ref rather than the recorded baseline — the machinery for that already
exists in this script.

I would take (1): a marker is the documented way to close a site, and
requiring a second mechanical step to record it is a trap that catches
good behaviour.

🤖 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 lifecycle census error messages to distinguish genuine
increases in column-guard debt from reclassified deliberate literals.
* Added clearer remediation guidance for reclassified results, including
when to update the baseline.
* Updated lifecycle census baseline mappings to reflect current
classifications.

* **Tests**
* Added coverage for unchanged baselines, genuine guard-count increases,
and marker-only reclassification scenarios.

<!-- end of auto-generated comment: release notes by coderabbit.ai -->

---------

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant