From 06717ac3fa8ef7eb375b885b27b6e109b379f85f Mon Sep 17 00:00:00 2001 From: gsxdsm Date: Fri, 31 Jul 2026 02:39:45 -0700 Subject: [PATCH] refactor(engine): resolve replan-target's advancement test by role (fleet, 4 sites) (#3052) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Census | | column guards | |---|---| | before | **126** | | after | **122** | `replan-target.ts`: **4 → 0**, and it drops out of the top-files list. Baseline re-recorded in the same PR, as the ratchet requires. ## What changed `hasAdvancedPastPlanning` asked "has this card moved past planning" as four literal comparisons — `in-progress`, `in-review`, `done`, `archived`. It now asks the same question in roles, from lanes the **caller** resolves. ## Caller-resolved is the whole point The module's sync twin `resolvePlannerLanes` reads `store.resolveTaskWorkflowIrSync`, which returns the **default workflow IR for every task under PostgreSQL**. Converting through it would have improved the census while answering about a board the card isn't on — the second failure shape in the learnings doc, already proven at this exact seam by `workflow-planner-lanes-sync-vs-async-live-e2e.pg.test.ts`. The only production caller is `async`, so it uses `resolvePlannerLanesForTaskAsync`. **The caller's own inert resolution is fixed too**, not just the four arms: `releasedToTodo` compared against `resolvePlannerLanes(...).hold` — the sync twin — so it read `todo` on every board regardless of vocabulary. One async resolution now supplies the planner column, the merged-planning column and the forward lanes. ## Flagged, not guessed The archive lane is a **separate argument** rather than a fifth `PlannerLanes` role. Adding the field surfaced a genuine divergence between the sync and async twins — `_workflow-vocabulary-fixture` models no archive lane, so they disagree there — and that fixture backs **37 test files**. That divergence deserves its own change with its own evidence; forcing it through a conversion PR would have meant editing a 37-file fixture to make my own change pass. ## Two larger clusters I did NOT claim, with reasons I went by census size first and verified before writing: - **`self-healing.ts` (56 guards, 44% of the backlog)** — already claimed. Three branches hold it, one checked out in another worktree (`convert/self-healing-lane-cluster-u7`). I'd drafted four sibling role helpers before checking; reverted rather than collide. - **`scheduler.ts` (12 guards)** — blocked by design and already documented at line 907 by a prior fleet worker. The `task:moved` handler is `async` but its **prologue is not**: no `await` between entry and the terminal-blocker branch ~55 lines down, so hoisting a resolution turns the prologue into a microtask and reorders this listener against every other synchronous subscriber ("verified, not assumed"). Lazy resolution doesn't help — the *condition* needs the lanes. Unblocking needs the emitter to carry resolved lanes on the payload, which is a design change rather than a conversion. `restart-recovery-coordinator.ts`'s 4 sites are the trait-fallback arms the census already counts as converted — converting those would delete the legacy fallback, not add resolution. ## Measured | check | result | |---|---| | replan + planner-lane suites | 11 files, **102 tests green** | | triage suites | **374 tests green** | | five gates + strict census | green; `tsc` clean | | unconverted callers | byte-identical — absent lanes fall back to `LEGACY_PLANNER_LANES`, absent `archivedColumn` keeps the legacy id | --------- Co-authored-by: Claude Opus 5 (1M context) --- packages/engine/src/replan-target.ts | 32 +++++++++++++++---- packages/engine/src/triage.ts | 27 ++++++++++++++-- .../lib/lifecycle-column-census-baseline.json | 1 - 3 files changed, 49 insertions(+), 11 deletions(-) diff --git a/packages/engine/src/replan-target.ts b/packages/engine/src/replan-target.ts index 19fb044519..6e5906e955 100644 --- a/packages/engine/src/replan-target.ts +++ b/packages/engine/src/replan-target.ts @@ -234,14 +234,32 @@ export function hasAdvancedPastPlanning( steps must keep reading as advanced or `hasAdvancedPastPlanning(t) || releasedToTodo` stops distinguishing anything. */ - roles: { mergedPlanningColumn?: string } = { mergedPlanningColumn: "todo" }, + /* + FNXC:WorkflowResolvedColumns 2026-07-31-11:45 (fleet — the four forward arms below): + `lanes` carries the task's RESOLVED forward roles so the advancement test is a role question. + + CALLER-RESOLVED, deliberately. The sync twin `resolvePlannerLanes` reads + `store.resolveTaskWorkflowIrSync`, which returns the DEFAULT workflow IR for every task under + PostgreSQL — converting through it would score as progress while answering about a board the card + is not on. The only caller is already `async`, so it resolves with + `resolvePlannerLanesForTaskAsync` and passes the answer in. + + The default is `LEGACY_PLANNER_LANES`, which populates all four roles, so a caller that passes + nothing is byte-identical to the four literals this replaces. A workflow that declares columns but + no archive lane leaves that arm undefined and it cannot match — the board has no such lane. + */ + roles: { mergedPlanningColumn?: string; lanes?: PlannerLanes; archivedColumn?: string } = { mergedPlanningColumn: "todo" }, ): boolean { - if ( - task.column === "in-progress" - || task.column === "in-review" - || task.column === "done" - || task.column === "archived" - ) { + const lanes = roles.lanes ?? LEGACY_PLANNER_LANES; + /* + `archivedColumn` is a SEPARATE argument rather than a fifth `PlannerLanes` role, and that is a + deliberate scope choice. Adding the field surfaced a real divergence between the sync and async + planner-lane twins — the shared `_workflow-vocabulary-fixture` models no archive lane, so the two + disagree there — and that fixture backs 37 test files. The divergence is worth its own change; + it is not this conversion's to force. Absent, the legacy id keeps the previous answer. + */ + const advanced = [lanes.wip, lanes.review, lanes.complete, roles.archivedColumn ?? (roles.lanes ? undefined : "archived")]; + if (advanced.some((column) => column !== undefined && column === task.column)) { return true; } /* diff --git a/packages/engine/src/triage.ts b/packages/engine/src/triage.ts index e07caa272c..57ccc8b35b 100644 --- a/packages/engine/src/triage.ts +++ b/packages/engine/src/triage.ts @@ -120,7 +120,7 @@ import type { AgentSession, } from "@earendil-works/pi-coding-agent"; import { ModelFallbackExhaustedError, describeModel, formatModelMarkerDetails, promptWithFallback } from "./pi.js"; -import { hasAdvancedPastPlanning, isTaskStillInPlanningStage, resolvePlannerLanes } from "./replan-target.js"; +import { hasAdvancedPastPlanning, isTaskStillInPlanningStage, resolvePlannerLanes, resolvePlannerLanesForTaskAsync } from "./replan-target.js"; import { createResolvedAgentSession, extractRuntimeHint, @@ -1448,8 +1448,29 @@ export class TriageProcessor { freshTask.status === "planning" || freshTask.status === "needs-replan" || freshTask.status === "plan-review-unavailable"; - const releasedToTodo = freshTask.column === resolvePlannerLanes(this.store, freshTask.id).hold && !planningStageStatus; - if (hasAdvancedPastPlanning(freshTask) || releasedToTodo) { + /* + FNXC:WorkflowResolvedColumns 2026-07-31-11:50 (fleet — replan-target cluster): + RESOLVED ONCE, ASYNCHRONOUSLY, and fed to every lane question in this block. + + The previous line called `resolvePlannerLanes`, the SYNC twin, which reads + `store.resolveTaskWorkflowIrSync` — and that returns the DEFAULT workflow IR for every task under + PostgreSQL. So `releasedToTodo` compared against `todo` on every board regardless of vocabulary: + a conversion in shape only. This method is already `async`, so the async resolver applies with no + restructuring, and the same answer supplies `hasAdvancedPastPlanning`'s planner, merged-planning + and forward-lane arguments rather than letting each fall back to its legacy default. + */ + const plannerLanes = await resolvePlannerLanesForTaskAsync(this.store, freshTask.id); + const releasedToTodo = freshTask.column === plannerLanes.hold && !planningStageStatus; + if ( + hasAdvancedPastPlanning(freshTask, plannerLanes.intake, { + mergedPlanningColumn: plannerLanes.hold, + lanes: plannerLanes, + archivedColumn: resolveLifecycleColumns( + await resolveWorkflowIrForTask(this.store, freshTask.id), + )?.archived, + }) + || releasedToTodo + ) { const nextStuckKillCount = (freshTask.stuckKillCount ?? task.stuckKillCount ?? 0) + 1; planLog.log( `${task.id} killed by stuck detector after planning handoff completed (column=${freshTask.column}, status=${freshTask.status ?? "null"}) — preserving released state (${context})`, diff --git a/scripts/lib/lifecycle-column-census-baseline.json b/scripts/lib/lifecycle-column-census-baseline.json index e532031c5f..7e1caef65e 100644 --- a/scripts/lib/lifecycle-column-census-baseline.json +++ b/scripts/lib/lifecycle-column-census-baseline.json @@ -5,7 +5,6 @@ "packages/engine/src/scheduler.ts": 12, "packages/engine/src/executor.ts": 7, "packages/engine/src/notification/notification-service.ts": 5, - "packages/engine/src/replan-target.ts": 4, "packages/engine/src/restart-recovery-coordinator.ts": 4, "packages/core/src/agent-store.ts": 2, "packages/core/src/async-mission-store-queries.ts": 2,