Merge remote-tracking branch 'origin/farm/f9161653/fix-prewalk-completion-loop'

packages/coding-agent/src/session/agent-session.ts

@@ -1748,7 +1748,7 @@ export class AgentSession {
 	#prewalk: Prewalk | undefined;
 	/** True once the plan nudge has been queued; scrubbed from context at the switch. */
 	#prewalkPlanInjected = false;
-	/** True until the first assistant turn after the plan nudge completes. */
+	/** Armed by plan/tool progress; consumed by one text-only continuation. */
 	#prewalkContinuePending = false;
 	/** True once any successful `todo` call landed — opens the prewalk
 	 *  trigger gate: the switch fires at the first edit/write AFTER the todo
@@ -2249,22 +2249,31 @@ export class AgentSession {
 		const prewalk = this.#prewalk;
 		if (!prewalk || context?.message.role !== "assistant") return;
 
-		// The plan nudge can produce a prose-only reply, which the agent loop
-		// treats as completion before any implementation starts. Keep the
-		// safety net open only for the turn immediately following that nudge:
-		// once the model calls any tool, later text-only completion is genuine.
-		if (this.#prewalkContinuePending) {
+		const todoCalledThisTurn = context.toolResults.some(result => result.toolName === "todo");
+		if (todoCalledThisTurn) {
+			this.#prewalkTodoSeen = true;
+		}
+
+		// The plan nudge asks for a prose plan before implementation begins,
+		// but the agent loop treats each text-only reply as terminal. Tool
+		// progress re-arms one continuation, allowing split flows such as
+		// plan → todo → prose → read → prose → edit/write. Consuming the arm
+		// before steering also detects completion: two consecutive text-only
+		// replies have no intervening progress, so the second ends naturally
+		// instead of producing the #5551 loop.
+		const hasToolResults = context.toolResults.length > 0;
+		if (this.#prewalkPlanInjected && hasToolResults) {
+			this.#prewalkContinuePending = true;
+		} else if (this.#prewalkContinuePending) {
 			this.#prewalkContinuePending = false;
-			if (context.toolResults.length === 0) {
-				this.agent.steer({
-					role: "custom",
-					customType: PREWALK_CONTINUE_MESSAGE_TYPE,
-					content: prewalkContinuePrompt,
-					attribution: "agent",
-					display: false,
-					timestamp: Date.now(),
-				});
-			}
+			this.agent.steer({
+				role: "custom",
+				customType: PREWALK_CONTINUE_MESSAGE_TYPE,
+				content: prewalkContinuePrompt,
+				attribution: "agent",
+				display: false,
+				timestamp: Date.now(),
+			});
 		}
 
 		// Todo gate: the plan nudge instructs "finish the plan, then init the
@@ -2275,9 +2284,6 @@ export class AgentSession {
 		// ACTIVE tool set, not the registry: a registered-but-deactivated todo
 		// (e.g. a restricted active-tool slate) is uncallable and would
 		// deadlock the switch.
-		if (context.toolResults.some(result => result.toolName === "todo")) {
-			this.#prewalkTodoSeen = true;
-		}
 		const todoGateOpen = this.#prewalkTodoSeen || !this.getActiveToolNames().includes("todo");
 		const action = todoGateOpen
 			? context.toolResults.find(result => PREWALK_ACTION_TOOLS[result.toolName])

packages/coding-agent/test/agent-session-prewalk.test.ts

@@ -292,16 +292,25 @@ describe("AgentSession prewalk", () => {
 		expect(session.model?.id).toBe(target.id);
 	});
 
-	it("does not continue a completed bash-only task after the plan-nudge window closes", async () => {
+	it("bounds a completed bash-only task to a single continuation instead of looping", async () => {
+		// Regression (#5551): with no edit/write ever run, the continuation net
+		// used to re-fire on every text-only reply, looping forever. It must
+		// fire at most once — one "continue" nudge — then let the next text-only
+		// reply end the run. No mock fallback: a stray extra turn rejects.
 		const primary = modelOrThrow("claude-sonnet-4-5");
 		const target = modelOrThrow("claude-sonnet-4-6");
 		const modelRegistry = new ModelRegistry(authStorage, path.join(tempDir.path(), "models.yml"));
 
+		// Turn 1: record (nudge injected after). Turn 2: bash — not an action
+		// tool. Turn 3: prose — the single continuation fires. Turn 4: prose
+		// again — no more continuation, run ends. A 5th call would exhaust the
+		// script and reject.
 		const mock = createMockModel({
 			responses: [
 				toolCall("t1", "record"),
 				toolCall("t2", "bash"),
 				{ content: [{ type: "text", text: "Commit complete." }], stopReason: "stop" },
+				{ content: [{ type: "text", text: "Nothing left to do." }], stopReason: "stop" },
 			],
 		});
 		const requested: string[] = [];
@@ -331,11 +340,75 @@ describe("AgentSession prewalk", () => {
 
 		await session.prompt("commit the current changes");
 
+		// Exactly one continuation: 4 turns, all on the primary (no edit/write,
+		// so no switch), then a clean stop.
 		expect(requested).toEqual([
 			`${primary.provider}/${primary.id}`,
 			`${primary.provider}/${primary.id}`,
 			`${primary.provider}/${primary.id}`,
+			`${primary.provider}/${primary.id}`,
 		]);
+		expect(session.model?.id).toBe(primary.id);
+	});
+
+	it("re-arms continuation after tool progress between prose turns", async () => {
+		// Regression: a normal prewalk can split planning across several turns:
+		// prose plan, todo init, then prose before implementation. Each tool
+		// progress segment must earn one continuation so the second prose turn
+		// cannot end the run before edit/write.
+		const primary = modelOrThrow("claude-sonnet-4-5");
+		const target = modelOrThrow("claude-sonnet-4-6");
+		const modelRegistry = new ModelRegistry(authStorage, path.join(tempDir.path(), "models.yml"));
+
+		// Turn 1: read-only (nudge injected after). Turn 2: prose plan —
+		// bridged. Turn 3: todo — gate opens and re-arms the net. Turn 4:
+		// prose — bridged again. Turn 5: write — switch.
+		const mock = createMockModel({
+			responses: [
+				toolCall("t1", "record"),
+				{ content: [{ type: "text", text: "Here is the plan." }], stopReason: "stop" },
+				toolCall("t3", "todo"),
+				{ content: [{ type: "text", text: "Plan captured, starting now." }], stopReason: "stop" },
+				toolCall("t5", "write"),
+				{ content: ["done"] },
+			],
+		});
+		const requested: string[] = [];
+		const agent = new Agent({
+			getApiKey: () => "test-key",
+			initialState: {
+				model: primary,
+				systemPrompt: ["Test"],
+				tools: [recordTool as AgentTool, writeTool as AgentTool, todoTool as AgentTool],
+				messages: [],
+				thinkingLevel: Effort.Medium,
+			},
+			convertToLlm,
+			streamFn: (model, context, options) => {
+				requested.push(`${model.provider}/${model.id}`);
+				return mock.stream(model, context, options);
+			},
+		});
+		session = new AgentSession({
+			agent,
+			sessionManager: SessionManager.inMemory(),
+			settings: Settings.isolated({ "compaction.enabled": false }),
+			modelRegistry,
+			toolRegistry,
+			prewalk: { target },
+		});
+
+		await session.prompt("do the task");
+
+		expect(requested).toEqual([
+			`${primary.provider}/${primary.id}`,
+			`${primary.provider}/${primary.id}`,
+			`${primary.provider}/${primary.id}`,
+			`${primary.provider}/${primary.id}`,
+			`${primary.provider}/${primary.id}`,
+			`${target.provider}/${target.id}`,
+		]);
+		expect(session.model?.id).toBe(target.id);
 	});
 
 	it("skips the todo gate when todo is registered but not active (subagent-style restricted slates)", async () => {