fix(coding-agent): reject empty advisor stop completions

packages/coding-agent/CHANGELOG.md

@@ -31,6 +31,7 @@
 
 - Fixed visible per-keystroke lag while searching in the `/resume` session picker. Literal matches now rank synchronously from a cached per-session haystack, fuzzy scoring runs in bounded background chunks that converge to the same ranking (large listings previously rebuilt a fuzzy index per token per session on every keystroke), and the prompt-history SQLite lookup — an FTS query plus a LIKE scan over every stored prompt — is debounced off the keystroke path.
 - Fixed compiled Linux binary extension loading when bundled web-search header generation cannot read `header-generator` data files from the build-time path. ([#5178](https://github.com/can1357/oh-my-pi/issues/5178))
+- Fixed the built-in advisor treating empty `stop` completions without advice as successful reviews, so silent provider failures now enter the advisor retry/drop path. ([#5212](https://github.com/can1357/oh-my-pi/issues/5212))
 
 ## [16.4.4] - 2026-07-11
 

packages/coding-agent/src/advisor/__tests__/advisor.test.ts

@@ -1294,6 +1294,110 @@ describe("advisor", () => {
 			expect(failures).toHaveLength(2);
 		});
 
+		it("treats an empty stop turn without advise as a failed advisor turn", async () => {
+			const promptInputs: string[] = [];
+			const turnErrors: unknown[] = [];
+			const failures: unknown[] = [];
+			const adviceNotes: string[] = [];
+			const rollbackCalls: number[] = [];
+			const lengthsBeforePrompt: number[] = [];
+			const events: string[] = [];
+			const state: { messages: AgentMessage[]; error?: string } = { messages: [] };
+			let promptCalls = 0;
+			const agent: AdvisorAgent = {
+				prompt: async input => {
+					promptCalls++;
+					promptInputs.push(input);
+					lengthsBeforePrompt.push(state.messages.length);
+					events.push(`prompt:${promptCalls}`);
+					state.messages.push({ role: "user", content: input, timestamp: promptCalls * 2 - 1 } as AgentMessage);
+					state.messages.push({
+						role: "assistant",
+						content: [],
+						api: "mock",
+						provider: "mock",
+						model: "mock-advisor",
+						usage: {
+							input: 0,
+							output: 0,
+							cacheRead: 0,
+							cacheWrite: 0,
+							totalTokens: 0,
+						},
+						stopReason: "stop",
+						timestamp: promptCalls * 2,
+					} as unknown as AgentMessage);
+					state.error = undefined;
+				},
+				abort: () => {},
+				reset: () => {
+					state.messages.length = 0;
+					state.error = undefined;
+				},
+				rollbackTo: count => {
+					rollbackCalls.push(count);
+					state.messages.length = count;
+					state.error = undefined;
+				},
+				state,
+			};
+			const messages: AgentMessage[] = [{ role: "user", content: "aaa", timestamp: 1 } as AgentMessage];
+			const host: AdvisorRuntimeHost = {
+				snapshotMessages: () => messages,
+				enqueueAdvice: note => adviceNotes.push(note),
+				onTurnError: error => {
+					turnErrors.push(error);
+					events.push(`hook:${error instanceof Error ? error.message : String(error)}`);
+				},
+				notifyFailure: error => {
+					failures.push(error);
+					events.push(`notify:${error instanceof Error ? error.message : String(error)}`);
+				},
+			};
+			const runtime = new AdvisorRuntime(agent, host, 0);
+
+			runtime.onTurnEnd(messages);
+			await runtime.waitForCatchup(1000, 1);
+
+			expect(promptInputs).toHaveLength(3);
+			expect(promptInputs[0]).toContain("aaa");
+			expect(promptInputs[1]).toBe(promptInputs[0]);
+			expect(promptInputs[2]).toBe(promptInputs[0]);
+			expect(lengthsBeforePrompt).toEqual([0, 0, 0]);
+			expect(rollbackCalls).toEqual([0, 0, 0]);
+			expect(turnErrors).toHaveLength(3);
+			const turnErrorMessages = turnErrors.map(error =>
+				(error instanceof Error ? error.message : String(error)).toLowerCase(),
+			);
+			for (const message of turnErrorMessages) {
+				expect(message).toContain("advisor");
+				expect(message).toContain("empty");
+				expect(message).toContain("response");
+			}
+			expect(failures).toHaveLength(1);
+			const failure = failures[0];
+			if (!(failure instanceof Error)) throw new Error("expected advisor failure error");
+			expect(failure.message.toLowerCase()).toContain("advisor");
+			expect(failure.message.toLowerCase()).toContain("empty");
+			expect(failure.message.toLowerCase()).toContain("response");
+			expect(events).toHaveLength(7);
+			expect(events[0]).toBe("prompt:1");
+			expect(events[1].startsWith("hook:")).toBe(true);
+			expect(events[1].toLowerCase()).toContain("empty");
+			expect(events[2]).toBe("prompt:2");
+			expect(events[3].startsWith("hook:")).toBe(true);
+			expect(events[3].toLowerCase()).toContain("empty");
+			expect(events[4]).toBe("prompt:3");
+			expect(events[5].startsWith("hook:")).toBe(true);
+			expect(events[5].toLowerCase()).toContain("empty");
+			expect(events[6].toLowerCase()).toContain("empty");
+			expect(events[6].startsWith("notify:")).toBe(true);
+			expect(adviceNotes).toEqual([]);
+			expect(state.messages).toHaveLength(0);
+			expect(state.error).toBeUndefined();
+			expect(runtime.backlog).toBe(0);
+		});
+
 		it("calls onTurnError with state.error before retrying the batch", async () => {
 			const promptInputs: string[] = [];
 			const turnErrors: unknown[] = [];

packages/coding-agent/src/advisor/runtime.ts

@@ -352,6 +352,10 @@ export class AdvisorRuntime {
 					// successful empty cycle.
 					const promptError = this.agent.state.error;
 					if (promptError) throw new Error(promptError);
+					const emptyResponseError = getAdvisorEmptyResponseError(
+						this.agent.state.messages.slice(messageSnapshot),
+					);
+					if (emptyResponseError) throw emptyResponseError;
 					success = true;
 					this.#consecutiveFailures = 0;
 					this.#failureNotified = false;
@@ -405,6 +409,38 @@ export class AdvisorRuntime {
 	}
 }
 
+function getAdvisorEmptyResponseError(messages: readonly AgentMessage[]): Error | undefined {
+	let sawAssistant = false;
+	for (const message of messages) {
+		if (message.role !== "assistant") continue;
+		sawAssistant = true;
+		if (message.stopReason !== "stop") return undefined;
+		if (hasAdvisorResponseContent(message)) return undefined;
+	}
+	if (sawAssistant) return new Error("Advisor turn returned an empty stop response without advice");
+	if (messages.length > 0) return new Error("Advisor turn ended without an assistant response");
+	return undefined;
+}
+
+function hasAdvisorResponseContent(message: AssistantMessage): boolean {
+	return message.content.some(block => {
+		switch (block.type) {
+			case "text":
+				return block.text.trim().length > 0;
+			case "thinking":
+				return block.thinking.trim().length > 0;
+			case "redactedThinking":
+				return block.data.length > 0;
+			case "toolCall":
+				return block.name.trim().length > 0;
+			case "fallback":
+				return false;
+			default:
+				return false;
+		}
+	});
+}
+
 type TextualContent = string | readonly (TextContent | ImageContent)[];
 
 function obfuscateTextualContent(obfuscator: SecretObfuscator, content: TextualContent): TextualContent {