Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion docs/non-compaction-retry-policy.md
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ So: overload/rate/server/network-style failures use this retry policy; context-w

### Responses request-body-read timeout exception

An exact OpenAI Responses HTTP 408 whose error text says `Timed out reading request body` is special only when the provider recorded that the **actual submitted request** was a full replay, not a `previous_response_id` delta. The transport surfaces that full-replay case after the first response. Delta and unknown/legacy request shapes retain ordinary transport retry behavior: mutating history after a delta can force a larger full replay, so automatic local elision must not infer safety from the diagnostic alone. Before any full-replay recovery, the session preserves the normal replay-safety veto and retry budget, requires enabled compaction with `shake` in `compaction.methodOrder`, then performs one conservative, artifact-backed local `shake elide`. The one-shot marker is scoped to the logical prompt sequence; prompt generation remains the cancellation/session-transition fence. A retry occurs only when that operation rewrote eligible history; disabled, no-progress, artifact-save failure, cancellation, an exhausted retry budget, or a second matching error terminates the turn without an unchanged replay.
An exact OpenAI Responses HTTP 408 whose error text says `Timed out reading request body` is special only when the provider recorded that the **actual submitted request** was a full replay, not a `previous_response_id` delta. The transport surfaces that full-replay case after the first response. Delta and unknown/legacy request shapes retain ordinary transport retry behavior: mutating history after a delta can force a larger full replay, so automatic local elision must not infer safety from the diagnostic alone. Before any full-replay recovery, the session preserves the normal replay-safety veto and retry budget, requires enabled compaction with `shake` in `compaction.methodOrder`, then performs one conservative, artifact-backed local `shake elide`. The one-shot marker is scoped to the logical prompt sequence and is cleared by any turn that produced output, so a long prompt that keeps making progress can recover a later timeout over its newly grown history while back-to-back timeouts still get exactly one changed request; prompt generation remains the cancellation/session-transition fence. A retry occurs only when that operation rewrote eligible history; disabled, no-progress, artifact-save failure, cancellation, an exhausted retry budget, or a second matching error terminates the turn without an unchanged replay.

Automatic request-body-timeout recovery elides only eligible tool-result text. It never rewrites assistant/user text, fenced/XML blocks, reasoning, images, or native Responses replay payloads; a session with no eligible tool result terminates rather than submitting another unchanged request.

Expand Down
1 change: 1 addition & 0 deletions packages/coding-agent/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@

### Fixed

- Fixed long agent runs stopping on a second OpenAI Responses request-body timeout: the one-shot shake-and-retry recovery is re-armed by any turn that produced output, so a prompt that kept making progress can recover its newly grown history instead of terminating, while back-to-back timeouts still get exactly one changed request ([#12654](https://github.com/can1357/oh-my-pi/pull/12654) by [@hellofrommorgan](https://github.com/hellofrommorgan)).
- Fixed system prompt configuration validation so systemPromptTemplate and customSystemPrompt cannot conflict with a full systemPrompt replacement, including when values are empty.
- Added browser-relay support for listing eligible pages without attaching to or claiming them.
- Fixed Codex compatibility with the sloppy edit tool.
Expand Down
12 changes: 9 additions & 3 deletions packages/coding-agent/src/session/turn-recovery.ts
Original file line number Diff line number Diff line change
Expand Up @@ -427,14 +427,20 @@ export class TurnRecovery {
}

/**
* Records which model produced this turn, marks an active fallback as having
* served, then closes a successful retry saga and annotates recovered
* persisted errors.
* Records which model produced this turn, re-arms the one-shot Responses
* request-body-timeout recovery, marks an active fallback as having served,
* then closes a successful retry saga and annotates recovered persisted
* errors.
*/
async onAssistantSettledSuccessfully(message: AssistantMessage): Promise<void> {
if (!assistantTurnProducedOutput(message)) {
return;
}
// A turn that produced output is forward progress: later history growth is
// new, so the next exact full-replay body-read timeout gets its own single
// shake-and-retry. Back-to-back timeouts never reach this point, so the
// one-shot bound on an unchanged-request loop is unaffected.
this.#requestBodyReadTimeoutRecoveryPromptSequence = undefined;
const model = this.#host.model();
if (model) {
const level = this.#host.thinkingLevel();
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
import * as fs from "node:fs/promises";
import * as path from "node:path";
import { describe, expect, it, vi } from "bun:test";
import { Agent, type AgentMessage } from "@oh-my-pi/pi-agent-core";
import { type } from "@oh-my-pi/omptype";
import { Agent, type AgentMessage, type AgentTool } from "@oh-my-pi/pi-agent-core";
import { DEFAULT_SHAKE_CONFIG } from "@oh-my-pi/pi-agent-core/compaction";
import { streamSimple } from "@oh-my-pi/pi-ai/stream";
import { streamOpenAIResponses } from "@oh-my-pi/pi-ai/providers/openai-responses";
Expand Down Expand Up @@ -31,6 +32,53 @@ function completeResponse(): Response {
);
}

/** Completed Responses turn carrying optional assistant text plus one `probe` call. */
function toolCallResponse(responseId: string, callId: string, leadingText?: string): Response {
const events: Array<Record<string, unknown>> = [
{ type: "response.created", response: { id: responseId, status: "in_progress" } },
];
let outputIndex = 0;
if (leadingText !== undefined) {
const messageItem = { type: "message", id: `msg_${responseId}`, role: "assistant" };
events.push(
{
type: "response.output_item.added",
output_index: outputIndex,
item: { ...messageItem, status: "in_progress", content: [] },
},
{
type: "response.output_text.delta",
output_index: outputIndex,
item_id: messageItem.id,
delta: leadingText,
},
{
type: "response.output_item.done",
output_index: outputIndex,
item: { ...messageItem, status: "completed", content: [{ type: "output_text", text: leadingText }] },
},
);
outputIndex++;
}
const callItem = { type: "function_call", id: `fc_${callId}`, call_id: callId, name: "probe" };
events.push(
{ type: "response.output_item.added", output_index: outputIndex, item: { ...callItem, arguments: "" } },
{ type: "response.output_item.done", output_index: outputIndex, item: { ...callItem, arguments: "{}" } },
{
type: "response.completed",
response: {
id: responseId,
status: "completed",
usage: { input_tokens: 1, output_tokens: 1, total_tokens: 2 },
},
},
);
return new Response(events.map(event => `data: ${JSON.stringify(event)}`).join("\n\n") + "\n\n", {
status: 200,
headers: { "content-type": "text/event-stream" },
});
}

function model(baseUrl: string): Model<"openai-responses"> {
return buildModel({
api: "openai-responses",
Expand All @@ -48,6 +96,9 @@ function model(baseUrl: string): Model<"openai-responses"> {

type ResponseFactory = (body: string, requestNumber: number) => Response | Promise<Response>;

const probeParameters = type({});
type HarnessTool = AgentTool<typeof probeParameters, unknown, unknown>;

type SessionHarnessOptions = {
compactionEnabled?: boolean;
methodOrder?: string[];
Expand All @@ -56,6 +107,7 @@ type SessionHarnessOptions = {
maxRetries?: number;
messages?: AgentMessage[];
respond?: ResponseFactory;
tools?: HarnessTool[];
};

type SessionHarness = {
Expand Down Expand Up @@ -136,7 +188,7 @@ async function createSessionHarness(options: SessionHarnessOptions = {}): Promis
await sessionManager.ensureOnDisk();
const agent = new Agent({
getApiKey: () => "local-test-key",
initialState: { model: activeModel, systemPrompt: ["Test"], tools: [], messages },
initialState: { model: activeModel, systemPrompt: ["Test"], tools: options.tools ?? [], messages },
streamFn: streamSimple,
});
const settings = Settings.isolated({
Expand Down Expand Up @@ -510,6 +562,68 @@ describe("AgentSession Responses request-body timeout recovery", () => {
await harness.cleanup();
}
});

it("recovers a second full-replay timeout in the same prompt after an intervening successful turn", async () => {
const midPromptBulk = "MIDPROMPT_BULK_SENTINEL ".repeat(5_000);
// Distinct chunks: verbatim repetition would trip the thinking-loop guard
// and turn this turn into an unrelated retry.
const progressText = Array.from({ length: 3_000 }, (_, index) => `ASSISTANT_PROGRESS_SENTINEL_${index}`).join(
" ",
);
let probeCalls = 0;
const harness = await createSessionHarness({
tools: [
{
name: "probe",
label: "probe",
description: "probe test tool",
parameters: probeParameters,
execute: async () => {
probeCalls++;
return {
content: [{ type: "text", text: probeCalls === 1 ? midPromptBulk : "small follow-up result" }],
};
},
},
],
respond: (_body, requestNumber) => {
switch (requestNumber) {
// Two exact full-replay timeouts separated by successful turns.
case 1:
case 4:
return timeoutResponse();
case 2:
return toolCallResponse("resp_bulk", "call_bulk");
// Assistant text pushes the fresh tool-result bulk out of shake's
// protected recent window, so the second recovery has real material.
case 3:
return toolCallResponse("resp_progress", "call_small", progressText);
default:
return completeResponse();
}
},
});
try {
await runPrompt(harness);
expect(probeCalls).toBe(2);
expect(harness.requests).toHaveLength(5);
// First recovery elided the seeded history; the mid-prompt bulk did not exist yet.
expect(harness.requests[1]).toContain("artifact://");
expect(harness.requests[1]).not.toContain("historical tool result");
expect(harness.requests[3]).toContain("MIDPROMPT_BULK_SENTINEL");
// Second recovery elided only the new tool-result bulk.
expect(harness.requests[4]).not.toContain("MIDPROMPT_BULK_SENTINEL");
expect(harness.requests[4]).toContain("artifact://");
expect(harness.requests[4]).toContain("ASSISTANT_PROGRESS_SENTINEL");
expect(harness.session.agent.state.messages.at(-1)).toMatchObject({
role: "assistant",
stopReason: "stop",
content: [{ type: "text", text: "Recovered" }],
});
} finally {
await harness.cleanup();
}
}, 30_000);
it("does not rewrite when the recovery artifact cannot be saved", async () => {
const harness = await createSessionHarness();
const allocateArtifactPath = vi
Expand Down
43 changes: 42 additions & 1 deletion packages/coding-agent/test/turn-recovery-replay-unsafe.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -48,9 +48,11 @@ function createHost(
messages?: readonly AgentMessage[];
lastModelChangeRole?: string;
modelRoles?: Record<string, string>;
shakeSucceeds?: boolean;
} = {},
): TurnRecoveryHost {
const settings = Settings.isolated({
"retry.baseDelayMs": 1,
...(options.fallbackChains ? { "retry.fallbackChains": options.fallbackChains } : {}),
...(options.modelRoles ? { modelRoles: options.modelRoles } : {}),
});
Expand All @@ -69,6 +71,8 @@ function createHost(
} as never,
sessionManager: {
getLastModelChangeRole: () => options.lastModelChangeRole,
getBranch: () => [],
getSessionId: () => "test-session",
} as never,
persistedAssistantEntryId: () => undefined,
settings,
Expand Down Expand Up @@ -101,7 +105,7 @@ function createHost(
maybeAutoRedeemCodexReset: async () => false,
runAutoCompaction: async () =>
({ deferredHandoff: false, continuationScheduled: false }) as RecoveryCompactionResult,
shakeForRequestBodyReadTimeout: async () => false,
shakeForRequestBodyReadTimeout: async () => options.shakeSucceeds === true,
withBashBranchTransition: <T>(operation: () => T): T => operation(),
};
}
Expand Down Expand Up @@ -322,6 +326,43 @@ describe("TurnRecovery replay-unsafe output classification", () => {
expect(await recovery.handleResponsesRequestBodyReadTimeout(message)).toBe("not-applicable");
});

describe("full-replay timeout one-shot lifecycle", () => {
const timeoutTurn = (): AssistantMessage => ({
...makeMessage([], model),
api: "openai-responses" as const,
errorStatus: 408,
errorMessage: "Timed out reading request body.",
requestBodyReadTimeoutFullReplay: true,
});
const settledTurn = (content: AssistantMessage["content"]): AssistantMessage => ({
...makeMessage(content, model),
stopReason: "stop" as const,
errorMessage: undefined,
});

it("re-arms the recovery after an intervening turn that produced output", async () => {
const recovery = new TurnRecovery(createHost(model, modelRegistry, { shakeSucceeds: true }));
expect(await recovery.handleResponsesRequestBodyReadTimeout(timeoutTurn())).toBe("handled-retry");
await recovery.onAssistantSettledSuccessfully(
settledTurn([{ type: "toolCall", id: "call-progress", name: "bash", arguments: { command: "pwd" } }]),
);
expect(await recovery.handleResponsesRequestBodyReadTimeout(timeoutTurn())).toBe("handled-retry");
});

it("stays bound to one changed retry while the same prompt makes no progress", async () => {
const recovery = new TurnRecovery(createHost(model, modelRegistry, { shakeSucceeds: true }));
expect(await recovery.handleResponsesRequestBodyReadTimeout(timeoutTurn())).toBe("handled-retry");
expect(await recovery.handleResponsesRequestBodyReadTimeout(timeoutTurn())).toBe("handled-terminal");
});

it("does not re-arm on a settled turn that produced no output", async () => {
const recovery = new TurnRecovery(createHost(model, modelRegistry, { shakeSucceeds: true }));
expect(await recovery.handleResponsesRequestBodyReadTimeout(timeoutTurn())).toBe("handled-retry");
await recovery.onAssistantSettledSuccessfully(settledTurn([]));
expect(await recovery.handleResponsesRequestBodyReadTimeout(timeoutTurn())).toBe("handled-terminal");
});
});

it("does not replay a long OpenCode Go usage limit after committed text", () => {
const openCodeModel = getBundledModel("opencode-go", "deepseek-v4-flash");
if (!openCodeModel) throw new Error("Expected bundled OpenCode Go model");
Expand Down
Loading