U7 PR3: the specification reaction acts on what finalize DID, not on the fact that planning stopped (#2506)

Completes the pair started in #2498. That landed the outcome; this makes
the engine's reaction consume it.

## The bug

`onSpecifyComplete` fired on **every** finished specification, because
the seam announcing it fired unconditionally. So a card parked at the
manual plan-approval gate — finalize writes `status:
"awaiting-approval"` and **returns early**, before the release move —
was logged as `Specified X → todo` and had a Plan Review run armed for a
plan the operator had not approved.

#2491 stopped the **seeder** from acting on that, defensively, at the
seeder. This removes the reason it was ever asked. Both layers are
deliberate and neither is redundant:

- the seeder guard covers **every caller**, including self-healing's
re-seed;
- this one stops the engine doing work nobody asked for, and stops it
telling the operator something false about their own board.

`released` is the only outcome that licenses arming a run — the only one
meaning the card crossed into the hold column (or was already resting
there, plan-in-place) and is the graph's now. `parked` belongs to a
human; `withheld` belongs to the caller's retry budget.

## The event still fires on every outcome

Deliberately. Dropping the reaction for a non-release would also drop
the runtime's `recordActivity()` idle signal, and a reaction that
silently does not happen is harder to reason about than one that happens
with an accurate payload. R5's division of labour: **the seam announces,
the subscriber decides what a given outcome licenses.**

## Why there is a new extracted function

`reactToSpecificationComplete` is pulled out of the inline
`InProcessRuntime` callback for the same reason the continuation drain
was in #2491: the callback is built inside a class whose construction
attaches to the real central project registry, so no test could
distinguish *"the reaction respects the outcome"* from *"the reaction
ignores it"*.

**Revert proof:** with the outcome gate removed from the reaction, **5
of 8 fail**.

## Two call-site decisions worth naming

**`tryFinalizeExplicitDuplicateMarker` reports through a mutable ref,
not a widened return type.** Its boolean answers a *different* question
— "was this a duplicate marker at all?" — and 16 existing tests assert
it directly. I tried the widened return first and it turned all 16 red.
Expectation edits are exactly how a behavior change travels disguised as
churn, so I backed it out. **This diff touches zero existing test
expectations.**

**A duplicate-marker redirect reports `parked`**, which is accurate: it
deletes, flags, or clears the marker; it never releases the card into
the hold column.

## A fixture note — third of this shape on the program

My "task vanished between release and reaction" case passed `undefined`,
which triggered the harness **default parameter** and silently handed
the reaction a live task — making it a duplicate of the control rather
than the case it claimed to be. It now passes `null`, with a comment
saying why.

Running tally of near-false-greens on this unit, all the same family: a
fake that ignores its predicate (#2491), a stub that ignores its
callback (#2498), a default parameter that swallows the interesting
input (here). Each was caught by the test failing for the *wrong reason*
and being read rather than fixed.

## Verification

| Check | Result |
|---|---|
| new suite | 8/8 |
| 15 triage / planning / continuation suites | 361/361, **no expectation
edits** |
| `tsc --noEmit` (engine) | clean |
| `pnpm lint` | clean |
| `pnpm test:gate` | green (307 + 10 + 71) |
| `pnpm check:changesets` | clean |

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

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

.changeset/specify-complete-reaction.md

@@ -0,0 +1,7 @@
+---
+"@runfusion/fusion": patch
+---
+
+summary: Plan review no longer starts for a task that is still waiting on your approval.
+category: fix
+dev: U7. `onSpecifyComplete` now carries the `PlanningHandoffReport` from finalize, and the engine's reaction (`reactToSpecificationComplete`, extracted from the inline `InProcessRuntime` callback so its gating is testable) arms the pre-release plan-review continuation only on `outcome === "released"`. Non-release outcomes log the real outcome instead of asserting `Specified X → todo`, and never reach the store. `recordActivity()` still fires for every outcome so idle detection is unaffected. Complements PR #2491's seeder-side guard, which stays as the defence covering every other caller.

packages/engine/src/__tests__/specify-complete-reaction.test.ts

@@ -0,0 +1,133 @@
+/*
+FNXC:PlanningHandoffOutcome 2026-07-28-10:05 (U7 / R4, R5, R12 — workflow-owned lifecycle):
+
+THE INVARIANT: the engine arms a pre-release Plan Review run only for a card that
+was actually handed off to the graph.
+
+Before this, the reaction fired on every finished specification, because the seam
+that announces it fired unconditionally. A card parked at the manual plan-approval
+gate — finalize writes `status: "awaiting-approval"` and RETURNS EARLY, before the
+release move — was logged as `Specified X → todo` and had a Plan Review run armed
+for a plan the operator had not approved.
+
+PR #2491 stopped the SEEDER from acting on that, defensively, at the seeder. This
+removes the reason it was ever asked. Both layers are deliberate and neither is
+redundant: the seeder guard covers every caller including self-healing's re-seed,
+while this one stops the engine from doing work nobody asked for and from telling
+the operator something false about their own board.
+
+WHY `released` IS THE ONLY LICENCE. It is the only outcome meaning the card crossed
+into the hold column (or was already resting there, plan-in-place) and is the
+graph's now. `parked` belongs to a human; `withheld` belongs to the caller's retry
+budget. Arming a run for either is acting on a handoff that did not happen.
+
+WHY THIS FILE EXISTS AT ALL. The reaction was an inline callback constructed inside
+`InProcessRuntime`, whose construction attaches to the real central project
+registry — so no test could tell "the reaction respects the outcome" from "the
+reaction ignores it". Same extraction, same reason, as the continuation drain in
+PR #2491. A guard that cannot be shown to fail is not a guard.
+*/
+import { describe, expect, it, vi } from "vitest";
+import type { Task, WorkflowIr } from "@fusion/core";
+
+import { reactToSpecificationComplete } from "../runtimes/in-process-runtime.js";
+import type { PlanningHandoffOutcome } from "../triage.js";
+
+const IR = { version: "v2", name: "wf", columns: [], nodes: [], edges: [] } as unknown as WorkflowIr;
+
+// `null` means the row vanished. NOT `undefined`: that would trigger the default
+// parameter and silently hand the reaction a live task, turning the
+// vanished-task case into a duplicate of the control.
+function harness(task: Task | null = { id: "FN-1", column: "todo" } as Task) {
+  const seeded: string[] = [];
+  const kicks: string[] = [];
+  const logs: string[] = [];
+  return {
+    seeded,
+    kicks,
+    logs,
+    run: (outcome: PlanningHandoffOutcome) => reactToSpecificationComplete({
+      taskId: "FN-1",
+      outcome,
+      getTask: async () => task ?? undefined,
+      resolveIr: async () => IR,
+      seed: async (t) => { seeded.push(t.id); return { seeded: true }; },
+      kick: () => { kicks.push("kick"); },
+      log: (m) => { logs.push(m); },
+    }),
+  };
+}
+
+describe("specification-complete reaction arms a plan review only on a real handoff", () => {
+  it("arms the run when the card was RELEASED (the control)", async () => {
+    const h = harness();
+
+    await h.run("released");
+
+    expect(h.seeded).toEqual(["FN-1"]);
+    expect(h.kicks).toEqual(["kick"]);
+    expect(h.logs).toEqual(["Specified FN-1 → todo"]);
+  });
+
+  for (const outcome of ["parked", "withheld"] as const) {
+    it(`arms NOTHING when finalize reported ${outcome}`, async () => {
+      const h = harness();
+
+      await h.run(outcome);
+
+      expect(h.seeded).toEqual([]);
+      expect(h.kicks).toEqual([]);
+    });
+
+    it(`does not claim the card moved when finalize reported ${outcome}`, async () => {
+      // Truthfulness, not cosmetics: an operator reading "Specified → todo" for a
+      // card still sitting in the planner column is being told something false
+      // about their own board, and that log was the only trace of this path.
+      const h = harness();
+
+      await h.run(outcome);
+
+      expect(h.logs).toHaveLength(1);
+      expect(h.logs[0]).not.toContain("→ todo");
+      expect(h.logs[0]).toContain(outcome);
+    });
+  }
+
+  it("still respects an operator pause that lands after the release", async () => {
+    // Pre-existing guard, kept: the release happened, but the operator parked the
+    // card before the reaction ran. Nothing is armed.
+    const h = harness({ id: "FN-1", column: "todo", paused: true } as Task);
+
+    await h.run("released");
+
+    expect(h.seeded).toEqual([]);
+    expect(h.kicks).toEqual([]);
+  });
+
+  it("survives a task that vanished between the release and the reaction", async () => {
+    const h = harness(null);
+
+    await expect(h.run("released")).resolves.toBeUndefined();
+    expect(h.seeded).toEqual([]);
+  });
+
+  it("never resolves the workflow or reads the task for a non-release", async () => {
+    // The cheap assertion that catches a future "log it but still do the work"
+    // regression: a non-release must not even reach the store.
+    const getTask = vi.fn(async () => ({ id: "FN-1", column: "todo" } as Task));
+    const resolveIr = vi.fn(async () => IR);
+
+    await reactToSpecificationComplete({
+      taskId: "FN-1",
+      outcome: "parked",
+      getTask,
+      resolveIr,
+      seed: async () => ({ seeded: true }),
+      kick: () => {},
+      log: () => {},
+    });
+
+    expect(getTask).not.toHaveBeenCalled();
+    expect(resolveIr).not.toHaveBeenCalled();
+  });
+});

packages/engine/src/runtimes/in-process-runtime.ts

@@ -17,6 +17,7 @@ import type {
   NotificationPayload,
   WorkflowWorkItem,
   WorkflowWorkItemState,
+  WorkflowIr,
 } from "@fusion/core";
 import {
   AsyncCentralClaimStore,
@@ -29,6 +30,7 @@ import { Scheduler } from "../scheduler.js";
 import type { PrMonitor, PrComment } from "../pr-monitor.js";
 import type { PrInfo } from "@fusion/core";
 import { TaskExecutor, type TaskExecutorOptions } from "../executor.js";
+import type { PlanningHandoffOutcome } from "../triage.js";
 import { buildPrNodeDeps } from "../pr-nodes.js";
 import { isExperimentalFeatureEnabled } from "@fusion/core";
 import { createCliAgentRuntime, type BootstrappedCliAgentRuntime } from "../cli-agent/runtime.js";
@@ -243,6 +245,62 @@ export function resolveParkedContinuationDeferral(
  *  prevents is a property of this bound, so the two belong in one place. */
 export const DUE_PLANNING_CONTINUATION_BATCH_LIMIT = 20;
 
+/** Everything the specification-complete reaction touches, injected so the
+ *  reaction is exercisable without constructing a runtime. */
+export interface SpecificationCompleteReactionDeps {
+  taskId: string;
+  outcome: PlanningHandoffOutcome;
+  getTask: (taskId: string) => Promise<Task | undefined>;
+  resolveIr: (taskId: string) => Promise<WorkflowIr>;
+  seed: (task: Task, ir: WorkflowIr) => Promise<{ seeded: boolean; reason?: string }>;
+  kick: () => void;
+  log: (message: string) => void;
+}
+
+/**
+ * FNXC:PlanningHandoffOutcome 2026-07-28-10:05 (U7 / R4, R5 — workflow-owned lifecycle):
+ * The engine's reaction to a finished specification: arm the graph's pre-release
+ * Plan Review run for a card that was actually handed off.
+ *
+ * WHAT WAS WRONG: this fired on every finished specification, because the seam that
+ * announces it fired unconditionally. So a card parked at the manual plan-approval
+ * gate — finalize writes `status: "awaiting-approval"` and RETURNS EARLY, before the
+ * release move — was logged as "Specified X → todo" and had a Plan Review run armed
+ * for a plan the operator had not approved. PR #2491 stopped the seeder from acting
+ * on that, defensively, at the seeder. This removes the reason it was ever asked.
+ *
+ * `released` is the ONLY outcome that licenses arming a run: it is the only one that
+ * means the card crossed into the hold column (or was already resting there) and is
+ * the graph's now. `parked` belongs to a human, `withheld` belongs to the caller's
+ * retry budget — arming a run for either is doing work nobody asked for.
+ *
+ * The log line reports the real outcome rather than asserting a move that may not
+ * have happened; an operator reading "Specified → todo" for a card sitting in the
+ * planner column is being told something false about their own board.
+ *
+ * EXTRACTED from the inline `onSpecifyComplete` callback for the same reason the
+ * continuation drain was in PR #2491: the callback is constructed inside
+ * `InProcessRuntime`, whose construction attaches to the real project registry, so
+ * no test could tell "the reaction respects the outcome" from "the reaction ignores
+ * it". A guard that cannot be shown to fail is not a guard.
+ */
+export async function reactToSpecificationComplete(
+  deps: SpecificationCompleteReactionDeps,
+): Promise<void> {
+  if (deps.outcome !== "released") {
+    deps.log(
+      `Specification finished for ${deps.taskId} without a handoff (${deps.outcome}) — no plan review armed`,
+    );
+    return;
+  }
+  deps.log(`Specified ${deps.taskId} → todo`);
+  const live = await deps.getTask(deps.taskId);
+  if (!live || live.paused || live.userPaused) return;
+  const ir = await deps.resolveIr(live.id);
+  await deps.seed(live, ir);
+  deps.kick();
+}
+
 /** Everything the drain pass touches, injected so the pass is exercisable without
  *  constructing a runtime (which would attach to the real project registry). */
 export interface DuePlanningContinuationDrainDeps {
@@ -1306,16 +1364,19 @@ export class InProcessRuntime
             this.recordActivity();
             runtimeLog.log(`Specifying ${t.id}...`);
           },
-          onSpecifyComplete: (t) => {
+          onSpecifyComplete: (t, report) => {
+            // Activity is recorded for EVERY outcome: a planning session ran either
+            // way, and idle detection must not depend on whether it released.
             this.recordActivity();
-            runtimeLog.log(`Specified ${t.id} → todo`);
+            void reactToSpecificationComplete({
-            void (async () => {
+              taskId: t.id,
-              const live = await this.taskStore.getTask(t.id);
+              outcome: report.outcome,
-              if (!live || live.paused || live.userPaused) return;
+              getTask: (id) => Promise.resolve(this.taskStore.getTask(id)),
-              const ir = await resolveWorkflowIrForTask(this.taskStore, live.id);
+              resolveIr: (id) => resolveWorkflowIrForTask(this.taskStore, id),
-              await seedPreReleasePlanReviewContinuation(this.taskStore, live, ir);
+              seed: (task, ir) => seedPreReleasePlanReviewContinuation(this.taskStore, task, ir),
-              this.kickWorkflowContinuationProcessor();
+              kick: () => this.kickWorkflowContinuationProcessor(),
-            })().catch((error) => {
+              log: (message) => runtimeLog.log(message),
+            }).catch((error) => {
               runtimeLog.error(`Failed to start Todo plan review for ${t.id}:`, error);
             });
           },

packages/engine/src/triage.ts

@@ -216,7 +216,21 @@ export interface TriageProcessorOptions {
   /** Stuck task detector — monitors triage sessions for stagnation and triggers recovery. */
   stuckTaskDetector?: StuckTaskDetector;
   onSpecifyStart?: (task: Task) => void;
-  onSpecifyComplete?: (task: Task) => void;
+  /*
+  FNXC:PlanningHandoffOutcome 2026-07-28-10:05 (U7 / R4, R5 — workflow-owned lifecycle):
+  The reaction is told WHAT FINALIZE DID, not merely that specification stopped.
+  It fired unconditionally before, so a card parked at the manual plan-approval
+  gate — or one whose release move was refused — was announced as specified, and
+  the subscriber logged "Specified X -> todo" and armed a Plan Review run for a
+  card that had not moved.
+
+  The event still fires on every outcome. Dropping it for a non-release would also
+  drop the subscriber's activity/idle signal, and a reaction that silently does not
+  happen is harder to reason about than one that happens with an accurate payload.
+  R5's division of labour: the seam announces, the SUBSCRIBER decides what a given
+  outcome licenses.
+  */
+  onSpecifyComplete?: (task: Task, report: PlanningHandoffReport) => void;
   onSpecifyError?: (task: Task, error: Error) => void;
   onAgentText?: (taskId: string, delta: string) => void;
   /** AgentStore for resolving per-agent custom instructions. */
@@ -2482,11 +2496,12 @@ export class TriageProcessor {
 
           // FN-5220: planning agents that emit a `DUPLICATE: FN-NNNN` redirect
           // short-circuit normal spec finalization.
+          const duplicateReport: PlanningHandoffReport = { outcome: "parked" };
           if (await this.tryFinalizeExplicitDuplicateMarker(task, written, settings, {
             isReplan,
             feedback,
-          })) {
+          }, duplicateReport)) {
-            this.options.onSpecifyComplete?.(task);
+            this.options.onSpecifyComplete?.(task, duplicateReport);
             return;
           }
 
@@ -2535,11 +2550,11 @@ export class TriageProcessor {
             return;
           }
 
-          await this.finalizeApprovedTask(task, written, settings, {
+          const finalizeReport = await this.finalizeApprovedTask(task, written, settings, {
             isReplan,
             feedback,
           });
-          this.options.onSpecifyComplete?.(task);
+          this.options.onSpecifyComplete?.(task, finalizeReport);
         } finally {
           this.activeSessions.delete(task.id);
           stuckDetector?.untrackTask(task.id);
@@ -3200,6 +3215,15 @@ export class TriageProcessor {
       isReplan?: boolean;
       feedback?: string;
     } = {},
+    /*
+    FNXC:PlanningHandoffOutcome 2026-07-28-10:20 (U7):
+    Finalize's outcome is reported through this ref rather than by widening the
+    return type. The boolean return answers a DIFFERENT question — "was this a
+    duplicate marker at all?" — and 16 existing tests assert it directly. Folding
+    two questions into one return would have made every one of those an expectation
+    edit, which is how a behavior change gets to travel disguised as churn.
+    */
+    report: PlanningHandoffReport = { outcome: "parked" },
   ): Promise<boolean> {
     try {
       const explicitDuplicateMarker = parseExplicitDuplicateMarker(written);
@@ -3225,7 +3249,9 @@ export class TriageProcessor {
       } else {
         planLog.log(`${task.id} explicit duplicate marker detected — redirecting to ${canonicalId}`);
       }
-      await this.finalizeApprovedTask(task, written, settings, options);
+      // FNXC:PlanningHandoffOutcome 2026-07-28-10:20: surface what finalize did to
+      // the caller's reaction without changing what this method's boolean means.
+      report.outcome = (await this.finalizeApprovedTask(task, written, settings, options)).outcome;
       return true;
     } catch (err) {
       const msg = err instanceof Error ? err.message : String(err);