diff --git a/.changeset/replan-stale-execution-stamp.md b/.changeset/replan-stale-execution-stamp.md new file mode 100644 index 0000000000..816fa43bed --- /dev/null +++ b/.changeset/replan-stale-execution-stamp.md @@ -0,0 +1,7 @@ +--- +"@runfusion/fusion": patch +--- + +summary: Fix cards stranding in Planning after Plan Review asks for changes. +category: fix +dev: `hasAdvancedPastPlanning` treated a rebounded replan card as already-advanced once triage claimed it. Plan Review REVISE rebounds to the planner column with `needs-replan` (a durable park), but triage's claim overwrites that with the TRANSIENT `planning`, which is excluded from `REPLAN_PARK_STATUSES` — so the card fell through to the execution timestamps, which are set on the first pass and never cleared. Every guarded planner write then silently no-opped and the finalize never handed the card off. The stamps are now discriminated by arrival order: a stamp predating `columnMovedAt` belongs to a previous pass, while one written after arrival still means execution won the FN-8361 race. The PR #2360 stranded-advanced class (stamps, no planning status) is unchanged. Also logs a warning when a planning finalize declines to hand off, which is how this strand stayed invisible. diff --git a/packages/engine/src/__tests__/replan-target.test.ts b/packages/engine/src/__tests__/replan-target.test.ts index 7cb0e2b44d..be4a510293 100644 --- a/packages/engine/src/__tests__/replan-target.test.ts +++ b/packages/engine/src/__tests__/replan-target.test.ts @@ -157,6 +157,51 @@ const planningGuardCases: PlanningGuardCase[] = [ }, stillPlanning: false, }, + /* + FNXC:WorkflowReplan 2026-07-26-18:40 (FN-8596 strand): + The stale-stamp case, and the exact shape that stranded a card in production. Triage CLAIMED a + rebounded replan card, overwriting `needs-replan` with the transient `planning`, so the durable- + park escape no longer applied and the card fell through to the execution stamps — which were set + on its FIRST pass and are never cleared. It read as advanced, every guarded planner write silently + no-opped, and the finalize never handed the card off. + The discriminator is arrival order: the stamp PREDATES `columnMovedAt`, so it belongs to the + previous pass. Contrast with the FN-8361 case above, where the stamp lands after arrival (there, + no `columnMovedAt` at all) and execution genuinely won the race. + */ + { + label: "triage replan card claimed by triage, stamps left over from its previous pass", + task: { + column: "triage", + steps: [planStep("step-1")], + status: "planning", + worktree: "/tmp/brave-otter", + executionStartedAt: "2026-07-26T13:50:57.686Z", + firstExecutionAt: "2026-07-26T13:50:57.686Z", + columnMovedAt: "2026-07-26T13:51:33.266Z", + }, + stillPlanning: true, + }, + { + label: "triage planning card whose execution stamp lands AFTER arrival (live claim, FN-8361)", + task: { + column: "triage", + steps: [], + status: "planning", + columnMovedAt: "2026-07-26T13:51:33.266Z", + executionStartedAt: "2026-07-26T13:52:10.000Z", + }, + stillPlanning: false, + }, + { + label: "stranded-advanced triage card with stale stamps but NO planning status (PR #2360)", + task: { + column: "triage", + steps: [planStep("step-1")], + executionStartedAt: "2026-07-26T13:50:57.686Z", + columnMovedAt: "2026-07-26T13:51:33.266Z", + }, + stillPlanning: false, + }, { label: "triage card parked by a reviewer outage after an execution attempt", task: { diff --git a/packages/engine/src/replan-target.ts b/packages/engine/src/replan-target.ts index e0985aaefa..6f8ffaafee 100644 --- a/packages/engine/src/replan-target.ts +++ b/packages/engine/src/replan-target.ts @@ -67,7 +67,10 @@ const REPLAN_PARK_STATUSES = new Set( export function hasAdvancedPastPlanning( task: Pick - & Partial>, + // FNXC:WorkflowReplan 2026-07-26-18:30: `columnMovedAt` is the stale-stamp discriminator (see + // the execution-stamp branch). Optional so existing narrowed callers still compile; absent, the + // branch keeps its prior "stamps mean advanced" answer. + & Partial>, ): boolean { if ( task.column === "in-progress" @@ -106,7 +109,40 @@ export function hasAdvancedPastPlanning( maxTriageConcurrent slot in a claim/skip loop. */ if (task.firstExecutionAt != null || task.executionStartedAt != null) { - return true; + /* + FNXC:WorkflowReplan 2026-07-26-18:30 (FN-8596 strand): + Distinguish a STALE stamp from a LIVE claim before treating the stamps as proof of advancement. + Both look identical in the fields above, and conflating them is what stranded FN-8596: Plan + Review returned REVISE, the graph rebounded the card to `triage` with `needs-replan`, triage + claimed it and overwrote the status to the TRANSIENT `"planning"` (so the durable-park escape + above no longer applied), and the stamps from the card's FIRST pass — never cleared — made the + replanning card read as "advanced past planning" for the rest of the session. Every guarded + planner write then silently no-opped, so the revision wrote PROMPT.md and the finalize never + handed the card off. It sat in triage until an engine restart. + + The discriminator is arrival order: a stamp written BEFORE the card arrived in the planner + column belongs to a previous pass, while a stamp written AFTER it arrived means execution + genuinely won the FN-8361 race and recovery must not clear the status out from under it. + Requiring a planning-stage status too keeps the PR #2360 stranded-advanced class (triage card + with stamps and NO planning status) reading as advanced, so self-healing still owns it. + Missing/unparseable `columnMovedAt` falls through to the prior behavior (advanced), so this can + only ever narrow the strand, never widen the race. + */ + const arrivedAtMs = Date.parse(task.columnMovedAt ?? ""); + const newestStampMs = Math.max( + Date.parse(task.executionStartedAt ?? "") || Number.NEGATIVE_INFINITY, + Date.parse(task.firstExecutionAt ?? "") || Number.NEGATIVE_INFINITY, + ); + const stampPredatesArrival = Number.isFinite(arrivedAtMs) + && Number.isFinite(newestStampMs) + && newestStampMs < arrivedAtMs; + const claimedInPlannerLane = task.column === "triage" + && task.status != null + && PLANNING_STAGE_STATUSES.has(task.status); + if (!(stampPredatesArrival && claimedInPlannerLane)) { + return true; + } + return false; } // The planner column itself is never "advanced" — nothing executes out of triage, and the steps // below belong to the card's previous planning pass. @@ -130,7 +166,7 @@ the "not advanced" answer, and TypeScript could not flag it. */ export function isTaskStillInPlanningStage( task: Pick - & Partial>, + & Partial>, ): boolean { return !hasAdvancedPastPlanning(task); } diff --git a/packages/engine/src/triage.ts b/packages/engine/src/triage.ts index 28acc9996d..16d22a24db 100644 --- a/packages/engine/src/triage.ts +++ b/packages/engine/src/triage.ts @@ -3237,7 +3237,25 @@ export class TriageProcessor { FN-8361 treats every delayed-finalization mutation as a live planning-stage transition. A normal scheduler advance skips and terminates this recovery body. */ - if (!await this.updatePlanningStateIfStillCurrent(task, taskUpdates)) return; + /* + FNXC:TriageFinalizeVisibility 2026-07-26-18:20 (FN-8596 strand): + This guard aborting used to be COMPLETELY silent — a bare `return` with no log, no audit and no + requeue. That is how the FN-8596 strand stayed invisible: the planner wrote PROMPT.md (via the + store tool, which bypasses the guard), the finalize refused here, and the card sat in triage + with `status:"planning"` forever with nothing in any log explaining why. Skipping is a LEGITIMATE + outcome when the scheduler genuinely advanced the card (FN-8024 deliberately does not log that + case), but "the finalize declined to hand off" must be observable — so warn with the live state + that made the decision. Cheap: it fires at most once per finalize attempt, not per poll. + */ + if (!await this.updatePlanningStateIfStillCurrent(task, taskUpdates)) { + const live = await this.store.getTask(task.id).catch(() => null); + planLog.warn( + `${task.id}: planning finalize skipped — task no longer in the planning stage ` + + `(column=${live?.column ?? "unknown"}, status=${live?.status ?? "null"}, ` + + `executionStartedAt=${live?.executionStartedAt ?? "null"}). Handoff NOT performed.`, + ); + return; + } try { const preflightDecision = await Promise.race([