diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index e56d3bbba..79022b068 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -31,6 +31,8 @@ ### Fixed - Fixed browser screenshots reporting `0x0` dimensions when `Bun.Image` rejects an image whose PNG/JPEG header still exposes real dimensions. ([#3577](https://github.com/can1357/oh-my-pi/issues/3577)) +- Fixed Anthropic classifier refusals being persisted as assistant dialogue after no fallback handled them; refusal stops are now displayed as errors but pruned from active and saved context before the next prompt. ([#3591](https://github.com/can1357/oh-my-pi/issues/3591)) + - Fixed `/resume ` in the interactive TUI only searching the active cwd's session directory; id-prefix lookup now falls back to sessions from other cwd buckets like CLI `--resume `. - Fixed plan mode rejecting edits to plan artifacts when models refer to them by bare filenames - Fixed absolute paths to session-owned artifacts being incorrectly routed through the editor bridge diff --git a/packages/coding-agent/src/session/agent-session.ts b/packages/coding-agent/src/session/agent-session.ts index e72615d53..7839fb365 100644 --- a/packages/coding-agent/src/session/agent-session.ts +++ b/packages/coding-agent/src/session/agent-session.ts @@ -2665,6 +2665,7 @@ export class AgentSession { if (this.#sessionMessageAlreadyPersisted(message)) return; if (message.role === "assistant") { const assistantMsg = message as AssistantMessage; + if (this.#isClassifierRefusal(assistantMsg)) return; if (assistantMsg.stopReason !== "aborted" && assistantMsg.stopReason !== "error" && assistantMsg.usage) { assistantMsg.contextSnapshot = { promptTokens: calculatePromptTokens(assistantMsg.usage), @@ -3137,7 +3138,6 @@ export class AgentSession { return; } } - // Check for retryable errors first (overloaded, rate limit, server errors) if (this.#isRetryableError(msg)) { const didRetry = await this.#handleRetryableError(msg); if (didRetry) { @@ -3145,6 +3145,14 @@ export class AgentSession { return; } } + // Classifier refusals are persisted-skipped above; also prune the trailing + // stub from active context so the next turn's prompt does not replay it. + // Fall through to the standard error tail so `session_stop` hooks (block, + // continue, telemetry) still fire — matching the pre-fix flow for + // `stopReason === "error"`. + if (this.#isClassifierRefusal(msg)) { + this.#removeAssistantMessageFromActiveContext(msg); + } this.#resolveRetry(); if (!checkedCompaction) { @@ -3178,7 +3186,7 @@ export class AgentSession { return; } } - await this.#emitSessionStopEvent(settledMessages); + await this.#emitSessionStopEvent(settledMessages, msg); await emitAgentEndNotification(); } }; @@ -4169,13 +4177,16 @@ export class AgentSession { await this.#extensionRunner?.emit({ type: "agent_end", messages }); } - async #emitSessionStopEvent(messages: AgentMessage[]): Promise { + async #emitSessionStopEvent( + messages: AgentMessage[], + lastAssistantMessage = this.getLastAssistantMessage(), + ): Promise { if (this.#agentKind === "sub" || !this.#extensionRunner?.hasHandlers("session_stop")) return; const generation = this.#promptGeneration; const result = await this.#extensionRunner.emitSessionStop({ messages, turn_id: Math.max(0, this.#turnIndex - 1), - last_assistant_message: this.getLastAssistantMessage(), + last_assistant_message: lastAssistantMessage, session_id: this.sessionId, session_file: this.sessionFile, stop_hook_active: this.#sessionStopHookActive, @@ -9048,7 +9059,7 @@ export class AgentSession { }); } - #removeEmptyStopFromActiveContext(assistantMessage: AssistantMessage): void { + #removeAssistantMessageFromActiveContext(assistantMessage: AssistantMessage): void { const messages = this.agent.state.messages; const lastMessage = messages[messages.length - 1]; if ( @@ -9057,6 +9068,10 @@ export class AgentSession { ) { this.agent.replaceMessages(messages.slice(0, -1)); } + } + + #removeEmptyStopFromActiveContext(assistantMessage: AssistantMessage): void { + this.#removeAssistantMessageFromActiveContext(assistantMessage); const emptyStopEntry = this.sessionManager .getBranch() @@ -11468,11 +11483,8 @@ export class AgentSession { errorMessage, }); - // Remove error message from agent state (keep in session for history) - const messages = this.agent.state.messages; - if (messages.length > 0 && messages[messages.length - 1].role === "assistant") { - this.agent.replaceMessages(messages.slice(0, -1)); - } + // Remove the failed assistant message from active context before retrying. + this.#removeAssistantMessageFromActiveContext(message); // Wait with exponential backoff (abortable). const retryAbortController = new AbortController(); diff --git a/packages/coding-agent/test/agent-session-retry-fallback.test.ts b/packages/coding-agent/test/agent-session-retry-fallback.test.ts index 5bc86fe97..4ae3b5c1d 100644 --- a/packages/coding-agent/test/agent-session-retry-fallback.test.ts +++ b/packages/coding-agent/test/agent-session-retry-fallback.test.ts @@ -10,6 +10,7 @@ import { getBundledModel } from "@oh-my-pi/pi-catalog/models"; import { ModelRegistry } from "@oh-my-pi/pi-coding-agent/config/model-registry"; import { parseModelPattern } from "@oh-my-pi/pi-coding-agent/config/model-resolver"; import { Settings } from "@oh-my-pi/pi-coding-agent/config/settings"; +import type { ExtensionRunner } from "@oh-my-pi/pi-coding-agent/extensibility/extensions"; import { AgentSession, type AgentSessionEvent } from "@oh-my-pi/pi-coding-agent/session/agent-session"; import { AuthStorage } from "@oh-my-pi/pi-coding-agent/session/auth-storage"; import { SessionManager } from "@oh-my-pi/pi-coding-agent/session/session-manager"; @@ -328,6 +329,103 @@ describe("AgentSession retry fallback", () => { ]); }); + it("drops classifier refusal messages before later prompts", async () => { + const primaryModel = getBundledModel("anthropic", "claude-sonnet-4-5"); + if (!primaryModel) { + throw new Error("Expected bundled test model to exist"); + } + + const mock = createMockModel({ + responses: [ + { + content: ["Classifier declined this turn."], + stopReason: "error", + stopDetails: { + type: "refusal", + category: "bio", + explanation: "Classifier declined this turn.", + }, + errorMessage: "Refusal (bio): Classifier declined this turn.", + }, + context => { + const replayedAssistantText = context.messages + .filter((message): message is AssistantMessage => message.role === "assistant") + .flatMap(message => message.content) + .filter(block => block.type === "text") + .map(block => block.text) + .join("\n"); + return { + content: [replayedAssistantText.includes("Classifier declined this turn.") ? "polluted" : "clean"], + }; + }, + ], + }); + const agent = new Agent({ + getApiKey: model => `${model.provider}-test-key`, + initialState: { + model: primaryModel, + systemPrompt: ["Test"], + tools: [], + messages: [], + }, + streamFn: (model, context, options) => mock.stream(model, context, options), + }); + + const settings = Settings.isolated({ + "compaction.enabled": false, + "retry.baseDelayMs": 5, + "retry.maxRetries": 1, + "retry.modelFallback": false, + }); + settings.setModelRole("default", `${primaryModel.provider}/${primaryModel.id}`); + + const sessionStopCalls: number[] = []; + const sessionStopLastAssistantMessages: Array = []; + const extensionRunner = { + emit: vi.fn().mockResolvedValue(undefined), + emitBeforeAgentStart: vi.fn().mockResolvedValue(undefined), + hasHandlers: vi.fn((eventType: string) => eventType === "session_stop"), + emitSessionStop: vi.fn((event: { last_assistant_message?: AssistantMessage }) => { + sessionStopCalls.push(mock.calls.length); + sessionStopLastAssistantMessages.push(event.last_assistant_message); + return Promise.resolve(undefined); + }), + } as unknown as ExtensionRunner; + + session = new AgentSession({ + agent, + sessionManager: SessionManager.inMemory(), + settings, + modelRegistry, + extensionRunner, + }); + + await session.prompt("Trigger classifier refusal"); + await session.waitForIdle(); + await session.prompt("Next prompt should not replay the refusal"); + await session.waitForIdle(); + + expect(mock.calls).toHaveLength(2); + const replayedAssistantText = mock.calls[1]?.context.messages + .filter((message): message is AssistantMessage => message.role === "assistant") + .flatMap(message => message.content) + .filter(block => block.type === "text") + .map(block => block.text) + .join("\n"); + expect(replayedAssistantText).not.toContain("Classifier declined this turn."); + expect(getLastAssistantMessage(session).content).toEqual([{ type: "text", text: "clean" }]); + // session_stop hooks must fire after each settled turn — including the + // refusal turn (regression: prior to PR #3594's review fix, the refusal + // branch short-circuited before `#emitSessionStopEvent`). + expect(sessionStopCalls).toEqual([1, 2]); + expect(sessionStopLastAssistantMessages[0]?.stopReason).toBe("error"); + expect(sessionStopLastAssistantMessages[0]?.stopDetails).toEqual({ + type: "refusal", + category: "bio", + explanation: "Classifier declined this turn.", + }); + }); + it("does not exceed retry.maxRetries for classifier fallback chains", async () => { const primaryModel = getBundledModel("anthropic", "claude-sonnet-4-5"); const firstFallback = getBundledModel("openai", "gpt-4o-mini");