fix(coding-agent): make a plan decision a session fact

apps/cli/src/main.ts

@@ -885,7 +885,10 @@ function readCredentialUid(credential: unknown): string | undefined {
 async function dispatchStepAppMode(prep: Extract<MainPreparation, { kind: "dispatch" }>): Promise<void> {
 	switch (prep.appMode) {
 		case "rpc": {
-			await runRpcMode(prep.runtimeHost, { includeLiveEvents: prep.parsed.includeLiveEvents });
+			await runRpcMode(prep.runtimeHost, {
+				includeLiveEvents: prep.parsed.includeLiveEvents,
+				planReviewSurface: prep.parsed.planReviewSurface,
+			});
 			break;
 		}
 		case "interactive": {

apps/cli/src/ui/interactive-mode.ts

@@ -2241,6 +2241,9 @@ export class InteractiveMode {
 		await this.session.bindExtensions({
 			uiContext,
 			mode: "tui",
+			// The terminal answers a plan review inside the tool call, so it never
+			// leaves a proposal waiting for a decision in the session.
+			planReviewSurface: false,
 			abortHandler: () => {
 				this.restoreQueuedMessagesToEditor({ abort: true });
 			},
@@ -2461,6 +2464,9 @@ export class InteractiveMode {
 			ui: this.createExtensionUIContext(),
 			mode: "tui",
 			hasUI: true,
+			// Same host as `bindCurrentSessionExtensions`, stated again because this
+			// context is built by hand rather than by the runner.
+			planReviewSurface: false,
 			cwd: this.sessionManager.getCwd(),
 			sessionManager: this.sessionManager,
 			modelRegistry: extensionRunner.getModelRegistry(),

apps/cli/test/plan-review-tool-row.test.ts

@@ -47,6 +47,9 @@ function exitPlanModeDefinition(): ToolDefinition<any, any, any> {
 		isPlanModeActive: () => true,
 		exitPlanMode: () => {},
 		resolvePlanFilePath: () => "/tmp/plan.md",
+		recordPendingProposal: () => {},
+		handOffProposal: () => {},
+		clearPendingProposal: () => {},
 	} satisfies StepPlanModeController);
 	return tools.get("exit_plan_mode") as ToolDefinition<any, any, any>;
 }

docs/plan-approval.md

@@ -9,10 +9,21 @@ active and steers the notes into the next response; dismissal keeps it active an
 ends the turn. There is no deadline on that decision. The dialog closes when the
 run is cancelled, and the tool then reports the review as aborted.
 
-Headless runs and RPC hosts cannot obtain approval. They exit plan mode and hand
+A host declares which of the two it is. One that can show a proposal and collect a
+decision — the web and desktop apps — sets `planReviewSurface` when it binds the
+session; plan mode then stays on, the turn ends, and the proposal is recorded as
+`pendingProposalHash` in the `step-plan` entry. `mode` and `hasUI` cannot answer
+this: every rpc host passes a dialog bridge, including clients with no review
+surface.
+
+An rpc client states it at startup: `--plan-review-surface` (or
+`RpcModeOptions.planReviewSurface`) for a host that draws the proposal and
+collects the answer. Without the flag a wire client keeps the handoff behavior,
+because the transport says nothing about what the client can draw.
+
+A host without that surface cannot obtain approval. It exits plan mode and hands
 the proposal to the caller, but the turn ends immediately: leaving the mode does
-not authorize execution. RPC hosts are always external, even when their dialog
-bridge reports `hasUI: true`. The caller must approve the exact snapshot before
+not authorize execution. The caller must approve the exact snapshot before
 starting an implementation turn. The host reports the proposal in place of the
 answer it does not have: print text mode writes the handoff to stdout, and the
 SDK result and the RPC `get_last_assistant_text` reply carry the handoff rather
@@ -84,13 +95,15 @@ Both interactive and external results carry structured `details`:
 ```
 
 `outcome: "external_approval_required"` identifies a handoff awaiting the
-caller's decision. `planModeExited: true` describes the resulting mode state; it
-does not mean approval was granted. External handoffs end the turn: the result
-sets `stopTurn` in addition to `terminate`.
+caller's decision. `planModeExited` describes the resulting mode state: `true` for
+a caller that left the mode, `false` for a review-surface host that keeps planning
+until the user answers. Neither means approval was granted. Every handoff ends the
+turn: the result sets `stopTurn` in addition to `terminate`.
 
 `approvalSource` identifies who owns the decision: `"user"` for the TUI review,
-including feedback and dismissal, and `"external"` for the headless / RPC
-caller's approval flow. It does not indicate that approval has occurred. Source
+including feedback and dismissal, and for a review-surface host whose person
+answers in the app; `"external"` for the headless / RPC caller's approval flow.
+It does not indicate that approval has occurred. Source
 and outcome are independent: a future caller-owned `"approved"` outcome can keep
 `approvalSource: "external"` without claiming the user clicked the TUI. Native
 headless/RPC exits currently emit `"external_approval_required"` only.
@@ -112,7 +125,9 @@ if (details?.outcome === "external_approval_required") {
   const expectedHash = createHash("sha256")
     .update(details.planContents, "utf8")
     .digest("hex");
-  if (details.planModeExited !== true || details.planHash !== expectedHash) {
+  // `planModeExited` is `true` for a caller that left the mode, `false` for a
+  // review-surface host that keeps planning until its user answers.
+  if (details.planHash !== expectedHash) {
     throw new Error("Incomplete or inconsistent plan handoff");
   }
   // Present details.planContents and bind the caller's decision to details.planHash.
@@ -144,7 +159,7 @@ Native outcomes keep their existing behavior:
 | `approved` | `user` | `true` | The user selected execution; restore tools and continue. |
 | `feedback` | `user` | `false` | Keep planning and steer the user's notes into the next response. |
 | `dismissed` | `user` | `false` | Keep planning and stop the turn to wait for the user. |
-| `external_approval_required` | `external` | `true` | Headless / RPC handoff; the turn ends and the caller must gate execution. |
+| `external_approval_required` | `external` / `user` | `true` / `false` | The turn ends. `true` for a caller that left the mode (`external`); `false` for a review-surface host that keeps planning until its user answers (`user`). |
 
 The TUI still selects execution by default, and Enter confirms that selection.
 Interactive result details also include the hash of the reviewed snapshot.
@@ -177,29 +192,76 @@ the loop after the batch even when a sibling result does not set `terminate`:
 - Earlier calls in the batch are not rolled back.
 
 The barrier ends that Agent Loop invocation. The session may still start another
-turn for input queued after `agent_end` or for automatic context maintenance, so
-a host that must not proceed without approval keeps that gate on its own side —
-which is why the headless and RPC handoffs are gated by the caller.
+turn for input queued after `agent_end` or for automatic context maintenance. The
+proposal that a handoff left waiting is still recorded in the session, so the
+guard below holds those turns too; a caller that owns the approval also gates
+execution on its own side.
 
 Inside the session, the plan extension registers one automatic-turn guard with
-`ctx.registerAutomaticTurnGuard`: a handoff that no user message has answered yet.
+`ctx.registerAutomaticTurnGuard`, and it asks one question: is a proposal still
+waiting for its decision.
+The answer is the session fact `pendingProposalHash`, not the shape of the
+transcript. A decision clears it — `/plan approve`, a request for changes, or
+leaving plan mode — while a question typed past the proposal does not: on a host
+with a review surface the user is still deciding, and on a caller handoff the
+next turn owns the decision.
 Guards belong to the extension generation that registered them, so a reload drops
 the stale ones along with the runner whose context they capture.
 The session asks that guard before every turn it would start on the user's
 behalf — a feature's `triggerTurn` message, a goal continuation, a cron delivery,
-a background lane waking its parent, and post-run recovery. A feature therefore
+a background lane waking its parent — and post-run recovery. A feature therefore
 does not need to know the other starters to respect the decision, and adding a
 new starter cannot silently bypass it.
 
+Post-run recovery is the exception that proves the rule: overflow and truncation
+recovery repair the transcript so the next request fits. A decision keeps them
+from starting anything new, but the repair itself still runs; threshold compaction
+stays deferred to the next request either way.
+
 A feature that can defer its own trigger asks `ctx.hasPendingUserDecision()` and
 retries later, instead of firing into a queue that would replay work after the
 decision: cron keeps the job due, and a goal continuation stays pending.
 
 A user's own prompt always runs: that message is how the decision gets answered.
 A host bridge forwarding a real person's input reports its own `source`, so the
-message is not mistaken for extension-generated text.
-The guard only recognizes *some* user message, because the approval belongs to
-the caller and the session must not guess at its wording. A host that needs a
-real decision records it on the tool result (the `tool_result` hook) or uses its
-own approval channel; a follow-up question typed by the user also clears the
-gate.
+message is not mistaken for extension-generated text. The same holds while a run
+is in flight: input a person typed waits in the queue, and when the turn submits
+a proposal, that input is still what the next request delivers — as that request's
+input, not after a request the model already took. The gate reads the message the
+agent would deliver next, not whether the queue contains one — work a feature
+queued ahead of the user's words still waits.
+The wait is settled where the message is delivered, not where a prompt is
+submitted: a message queued behind the submitting turn arrives without a new
+`prompt`, and that delivery is the only channel a caller handoff has.
+Context an extension queued (`sendCustomMessage`) is held the same way: it is
+flushed once the decision is answered, not into a question typed while it waits.
+The guard never guesses from wording. On a host without a review surface the
+decision belongs to the caller, and the only channel it has is a message; the
+first message after the handoff therefore settles the wait. A caller that wants
+the wait ended without writing a message sends `/plan off` (leave without
+implementing) or `/plan approve` (accept the proposal), and `/plan approve
+<hash>` binds the decision to the snapshot the caller actually read.
+
+`approve` claims the whole input, so a malformed hash is refused rather than read
+as a task: answering nothing would drop the wait and hand the broken text to the
+model as something to plan. `/plan approve` alone answers whatever is waiting;
+`/plan approve <hash>` must still match the reviewed snapshot after rereading the
+file. Verbs are matched without case, since a host forwards what its user typed,
+and a task whose text starts with one of them has to be rephrased.
+
+Approving, or answering with a message, delivers what the decision held. Leaving
+without approving releases the gate but starts nothing, so held work runs on the
+next turn that starts.
+
+Leaving is a stop, not only a state change: `/plan off` aborts the run in flight.
+The Default announcement reaches the model's next request, so a streaming run
+would otherwise keep going with the plan boundary already lifted and start
+implementing the proposal the user just rejected.
+
+A session written before the decision fact recorded its wait only in the
+transcript. `restorePlanState` migrates it once, from the still-unanswered
+handoff at the tail of the branch, so upgrading does not lift the gate on a
+proposal nobody answered. An entry that already carries `pendingProposalHash`
+keeps it under any version: a build that recorded the fact before this one did
+so without bumping the entry, and the transcript scan would drop a wait the
+branch still reports.

packages/agent-core/README.md

@@ -174,6 +174,17 @@ await agent.continue();
 
 The last message in context must be `user` or `toolResult` (not `assistant`).
 
+Pass `deliverQueued` when the next turn should start from queued input rather than
+from the transcript. The drained message becomes that request's prompt, which is
+what a caller returning after a decision needs: a follow-up queued behind the
+finishing turn would otherwise wait for a request the model takes without seeing
+it. A retry or a context repair must leave it off — those resume the same work,
+and the message the person queued is meant to arrive after it finishes.
+
+```typescript
+await agent.continue({ deliverQueued: true });
+```
+
 ### Event Types
 
 | Event | Description |
@@ -389,6 +400,17 @@ agent.clearAllQueues();
 
 Use clearSteeringQueue, clearFollowUpQueue, or clearAllQueues to drop queued messages.
 
+A delivery filter withholds individual messages. `setQueueDeliveryFilter` is asked
+per message whenever the loop takes queued input; a refused message stays queued in
+its original position, so a caller can hold part of the queue without dropping or
+reordering what it releases. In `one-at-a-time` mode the delivered message is the
+oldest one the filter allows.
+
+```typescript
+agent.setQueueDeliveryFilter((message) => isFromPerson(message));
+agent.hasDeliverableQueuedMessage((message) => isFromPerson(message));
+```
+
 When steering messages are detected after a turn completes:
 1. All tool calls from the current assistant message have already finished
 2. Steering messages are injected

packages/agent-core/src/agent.ts

@@ -144,19 +144,45 @@ class PendingMessageQueue {
 		return this.messages.length > 0;
 	}
 
-	drain(): AgentMessage[] {
+	/**
+	 * Take the messages that may be delivered now.
+	 *
+	 * `allow` is asked per message, and a refused one stays queued: a caller can
+	 * hold part of the queue without dropping what it holds or reordering what it
+	 * releases. With no predicate this drains everything the mode allows.
+	 */
+	drain(allowed?: (message: AgentMessage) => boolean): AgentMessage[] {
+		// Wrapped so a predicate that also declares (index, array) cannot be handed
+		// them by findIndex/some.
+		const allow = allowed === undefined ? undefined : (message: AgentMessage): boolean => allowed(message);
+		if (!allow) {
+			if (this.mode === "all") {
+				const drained = this.messages.slice();
+				this.messages = [];
+				return drained;
+			}
+			const first = this.messages[0];
+			if (!first) return [];
+			this.messages = this.messages.slice(1);
+			return [first];
+		}
 		if (this.mode === "all") {
-			const drained = this.messages.slice();
-			this.messages = [];
-			return drained;
+			const released: AgentMessage[] = [];
+			const held: AgentMessage[] = [];
+			for (const message of this.messages) (allow(message) ? released : held).push(message);
+			this.messages = held;
+			return released;
 		}
+		const index = this.messages.findIndex(allow);
+		if (index === -1) return [];
+		const [first] = this.messages.splice(index, 1);
+		return [first!];
+	}
 
-		const first = this.messages[0];
-		if (!first) {
-			return [];
-		}
-		this.messages = this.messages.slice(1);
-		return [first];
+	/** Whether any queued message would pass `allowed`. */
+	hasDeliverable(allowed?: (message: AgentMessage) => boolean): boolean {
+		if (!allowed) return this.messages.length > 0;
+		return this.messages.some((message: AgentMessage) => allowed(message));
 	}
 
 	clear(): void {
@@ -324,6 +350,29 @@ export class Agent {
 		return this.steeringQueue.hasItems() || this.followUpQueue.hasItems();
 	}
 
+	/**
+	 * Whether either queue holds a message that may be delivered now.
+	 *
+	 * A caller that has to decide whether continuing is safe asks this rather than
+	 * `hasQueuedMessages`: work the predicate refuses does not justify a turn.
+	 */
+	hasDeliverableQueuedMessage(allow: (message: AgentMessage) => boolean): boolean {
+		return this.steeringQueue.hasDeliverable(allow) || this.followUpQueue.hasDeliverable(allow);
+	}
+
+	/**
+	 * Filter consulted whenever the loop takes a queued message.
+	 *
+	 * Holding a message is the only way to keep automatic work out of a turn while
+	 * the user's own input still runs: the queues are drained by the loop, one
+	 * batch at a time, so a check made before starting the run cannot govern what
+	 * it delivers later.
+	 */
+	private queueDeliveryFilter: ((message: AgentMessage) => boolean) | undefined;
+	setQueueDeliveryFilter(filter: ((message: AgentMessage) => boolean) | undefined): void {
+		this.queueDeliveryFilter = filter;
+	}
+
 	/** Active abort signal for the current run, if any. */
 	get signal(): AbortSignal | undefined {
 		return this.activeRun?.abortController.signal;
@@ -371,8 +420,16 @@ export class Agent {
 		await this.runPromptMessages(messages);
 	}
 
-	/** Continue from the current transcript. The last message must be a user or tool-result message. */
-	async continue(): Promise<void> {
+	/**
+	 * Continue from the current transcript.
+	 *
+	 * `deliverQueued` asks for the queued input that may be delivered now to become
+	 * this turn's prompt, instead of continuing from the transcript. A caller that
+	 * is resuming after a retry or a context repair must leave it false: those
+	 * resume the same work, and a follow-up the person queued is meant to arrive
+	 * after it finishes, not to replace it.
+	 */
+	async continue(options: { deliverQueued?: boolean } = {}): Promise<void> {
 		if (this.activeRun) {
 			throw new Error("Agent is already processing. Wait for completion before continuing.");
 		}
@@ -383,13 +440,13 @@ export class Agent {
 		}
 
 		if (lastMessage.role === "assistant") {
-			const queuedSteering = this.steeringQueue.drain();
+			const queuedSteering = this.steeringQueue.drain(this.queueDeliveryFilter);
 			if (queuedSteering.length > 0) {
 				await this.runPromptMessages(queuedSteering, { skipInitialSteeringPoll: true });
 				return;
 			}
 
-			const queuedFollowUps = this.followUpQueue.drain();
+			const queuedFollowUps = this.followUpQueue.drain(this.queueDeliveryFilter);
 			if (queuedFollowUps.length > 0) {
 				await this.runPromptMessages(queuedFollowUps);
 				return;
@@ -398,6 +455,23 @@ export class Agent {
 			throw new Error("Cannot continue from message role: assistant");
 		}
 
+		// `runContinuation` only polls steering before its first request, so a queued
+		// follow-up would otherwise wait behind a request the model takes without ever
+		// seeing it. Delivering it here makes it that request's input.
+		if (options.deliverQueued) {
+			const queuedSteering = this.steeringQueue.drain(this.queueDeliveryFilter);
+			if (queuedSteering.length > 0) {
+				await this.runPromptMessages(queuedSteering, { skipInitialSteeringPoll: true });
+				return;
+			}
+
+			const queuedFollowUps = this.followUpQueue.drain(this.queueDeliveryFilter);
+			if (queuedFollowUps.length > 0) {
+				await this.runPromptMessages(queuedFollowUps);
+				return;
+			}
+		}
+
 		await this.runContinuation();
 	}
 
@@ -495,9 +569,9 @@ export class Agent {
 					skipInitialSteeringPoll = false;
 					return [];
 				}
-				return this.steeringQueue.drain();
+				return this.steeringQueue.drain(this.queueDeliveryFilter);
 			},
-			getFollowUpMessages: async () => this.followUpQueue.drain(),
+			getFollowUpMessages: async () => this.followUpQueue.drain(this.queueDeliveryFilter),
 		};
 	}
 

packages/agent-core/src/types.ts

@@ -46,6 +46,9 @@ export type ToolExecutionMode = "sequential" | "parallel";
  *
  * - "all": drain and inject every queued message at that point.
  * - "one-at-a-time": drain and inject only the oldest queued message, leaving the rest queued for later drain points.
+ *
+ * A delivery filter can withhold messages: "oldest" then means the oldest one it
+ * allows, and withheld messages stay queued in their original order.
  */
 export type QueueMode = "all" | "one-at-a-time";