smashimo/oh-my-pi
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Merge PR #3594: fix(session): prune classifier refusals from context (@roboomp)

Browse Source
This commit is contained in:
can1357
2026-06-26 23:42:15 +02:00
parent 363d5ca6c5 11c96c9ae3
commit c50af68125
3 changed files with 122 additions and 10 deletions
Download Patch File Download Diff File Expand all files Collapse all files
packages/coding-agent/CHANGELOG.md
+2
View File
@@ -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 <session-id>` 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 <session-id>`.
- 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
packages/coding-agent/src/session/agent-session.ts
+22 -10
View File
@@ -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<void> {
async #emitSessionStopEvent(
messages: AgentMessage[],
lastAssistantMessage = this.getLastAssistantMessage(),
): Promise<void> {
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();
packages/coding-agent/test/agent-session-retry-fallback.test.ts
+98
View File
@@ -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<AssistantMessage | undefined> = [];
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");