From 24f5ffaffabfde22cfd32f044f3c3db7a9e5ce2b Mon Sep 17 00:00:00 2001 From: gsxdsm Date: Fri, 31 Jul 2026 02:33:28 -0700 Subject: [PATCH] =?UTF-8?q?fleet:=20scheduler.ts=2012=20=E2=86=92=202=20li?= =?UTF-8?q?fecycle-column=20guards=20(#3051)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Claiming `packages/engine/src/scheduler.ts` from the census work order. ## Census before/after | File | Before | After | |---|---:|---:| | `packages/engine/src/scheduler.ts` | 12 | **2** | Measured with `scripts/lifecycle-column-census.mjs` (kind `column` only). ## The file already had the right shape — it just under-answered `resolveTaskParkedColumnsSync` already resolves a task's lanes from its own workflow, **synchronously on purpose**: these run inside `task:moved` / `task:updated` listeners, and its own comment records why an `await` is forbidden there — it would defer everything after it to a microtask and reorder handlers relative to a synchronous emitter. It also already fails soft to the legacy ids. But it only returned `{hold, intake}`, so every *other* lane question in the same listeners was still asked with a literal. Widening it to the full role set converted ten sites with no new abstraction, no new resolution per site, and no change to the event-ordering contract. ## What was silently broken on a renamed board - **PR monitoring never started** (`to === "in-review"`) and **never stopped** (`from === "in-review"`) — a card's PR either untracked, or tracked forever with its buffered comments never drained. - **Terminal cleanup never ran** (`to === "done" || "archived"`). - **The wip → hold failure bookkeeping never recorded** (`from === "in-progress"`). None of these throw. They just stop happening — which is why the census, not a red test, is what found them. ## Remaining 2, deliberately not converted `L1097` (`task.column === "in-progress"`) and `L1171` (`task.column !== "in-review"`) sit outside the listener where `parked` is in scope. They need their own resolution, and resolving per call there is a different cost profile than one-per-event; I flagged rather than guessed, per the fleet rule. ## Verification - census: `scheduler.ts` 12 → 2 - `scheduler-workflow-cutover` + `scheduler` — 42 tests green - `tsc --noEmit` on `@fusion/engine` clean; `pnpm lint` clean Behaviour on an unresolvable workflow is unchanged: the widened helper keeps the same fail-soft legacy defaults the narrow one had. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 5 (1M context) --- packages/engine/src/scheduler.ts | 30 ++++++++++++++++++++---------- 1 file changed, 20 insertions(+), 10 deletions(-) diff --git a/packages/engine/src/scheduler.ts b/packages/engine/src/scheduler.ts index ef6209c594..5065f614fb 100644 --- a/packages/engine/src/scheduler.ts +++ b/packages/engine/src/scheduler.ts @@ -400,12 +400,22 @@ this conversion rather than losing the wake entirely. defers everything after it to a microtask and reorders handlers relative to a synchronous emitter. A file split must not change event ordering, so the resolution uses the store's sync IR path. */ -function resolveTaskParkedColumnsSync(store: TaskStore, taskId: string): { hold: string; intake: string } { +/* +FNXC:WorkflowResolvedColumns 2026-07-31-10:10 (fleet: scheduler lifecycle roles): +Widened from {hold,intake} to the full role set. The listeners below ask the same "which lane is +this?" question about wip, review and the terminal columns, and answered it with literals — so on +a renamed board PR monitoring never started or stopped, failure bookkeeping never recorded, and +terminal cleanup never ran. None of those error; they simply stop happening. One resolution per +event, reusing the SAME sync path and the SAME fail-soft legacy defaults, so event ordering and +unresolvable-workflow behaviour are both unchanged. +*/ +function resolveTaskParkedColumnsSync(store: TaskStore, taskId: string): { hold: string; intake: string; wip: string; review: string; complete: string; archived: string } { + const legacy = { hold: "todo", intake: "triage", wip: "in-progress", review: "in-review", complete: "done", archived: "archived" }; try { - const lifecycle = resolveLifecycleColumns(store.resolveTaskWorkflowIrSync(taskId)); - return { hold: lifecycle?.hold ?? "todo", intake: lifecycle?.intake ?? "triage" }; + const l = resolveLifecycleColumns(store.resolveTaskWorkflowIrSync(taskId)); + return { hold: l?.hold ?? legacy.hold, intake: l?.intake ?? legacy.intake, wip: l?.wip ?? legacy.wip, review: l?.review ?? legacy.review, complete: l?.complete ?? legacy.complete, archived: l?.archived ?? legacy.archived }; } catch { - return { hold: "todo", intake: "triage" }; + return legacy; } } @@ -932,13 +942,13 @@ export class Scheduler { } // PR Monitoring if (this.options.prMonitor) { - if (to === "in-review" && task.prInfo) { + if (to === parked.review && task.prInfo) { // Start monitoring existing PR const repo = getCurrentRepo(this.store.getRootDir()); if (repo) { this.options.prMonitor.startMonitoring(task.id, repo.owner, repo.repo, task.prInfo); } - } else if (from === "in-review" && to !== "in-review") { + } else if (from === parked.review && to !== parked.review) { // If task has a closed/merged PR, drain buffered comments before // stopping monitoring (drainComments needs the tracked PR to still exist) if (task.prInfo && (task.prInfo.status === "closed" || task.prInfo.status === "merged")) { @@ -979,7 +989,7 @@ export class Scheduler { // FN-3895/FN-3924: complement periodic stale-blockedBy self-healing with immediate // blocker reconciliation when a potential blocker reaches a terminal completion column. // Invariant: blockedBy must reference a *current* unresolved blocker, else be null. - if (to === "done" || to === "archived") { + if (to === parked.complete || to === parked.archived) { try { const settings = await this.store.getSettings(); if (!settings.globalPause && !settings.enginePaused) { @@ -1052,13 +1062,13 @@ export class Scheduler { } } - if (from === "in-progress" && to === parked.hold) { + if (from === parked.wip && to === parked.hold) { if (source === "engine") { this.recentEngineTodoRequeues.set(task.id, task.columnMovedAt ?? new Date().toISOString()); } else { this.recentEngineTodoRequeues.delete(task.id); } - } else if (to === "in-review" || to === "done" || to === "archived") { + } else if (to === parked.review || to === parked.complete || to === parked.archived) { this.recentEngineTodoRequeues.delete(task.id); if (task.dispatchStormCount != null || task.lastDispatchAt != null || task.executeRequeueLoopCount != null || task.executeRequeueLoopSignature != null) { void this.store.updateTask(task.id, { @@ -1075,7 +1085,7 @@ export class Scheduler { // Event-driven scheduling: when a task moves to "done" (completion) or "todo" (retry/manual move), // trigger scheduling immediately so waiting tasks can start without waiting // for the next poll interval (up to 15 seconds). - if (to === "done" || to === parked.hold) { + if (to === parked.complete || to === parked.hold) { schedulerLog.log(`Task moved to ${to} — triggering scheduling`); this.schedule(); }