fix(codex): timeouts for websockets (#4979)
This commit is contained in:
@@ -7,6 +7,7 @@
|
||||
- Fixed user message transcript rendering to preserve user-authored ordered-list markers ([#5013](https://github.com/earendil-works/pi/issues/5013)).
|
||||
- Fixed self-update commands to bypass npm, pnpm, and Bun minimum release age gates for explicit `pi update` runs ([#4929](https://github.com/earendil-works/pi/issues/4929)).
|
||||
- Fixed context token estimates to count user image attachments consistently with tool result images ([#4983](https://github.com/earendil-works/pi/issues/4983)).
|
||||
- Fixed `httpIdleTimeoutMs` to apply to OpenAI Codex Responses WebSocket idle waits and added `websocketConnectTimeoutMs` for bounded WebSocket connect waits ([#4945](https://github.com/earendil-works/pi/issues/4945)).
|
||||
- Fixed `RpcClient` to reject pending requests and consume stdin pipe errors when the child process exits unexpectedly ([#4764](https://github.com/earendil-works/pi/issues/4764)).
|
||||
- Fixed managed npm extension updates to avoid package managers installing or resolving pi host packages as peer dependencies ([#4907](https://github.com/earendil-works/pi/issues/4907)).
|
||||
- Fixed RPC mode raw stdout writes to retry transient backpressure errors and flush queued protocol output during shutdown ([#4897](https://github.com/earendil-works/pi/issues/4897)).
|
||||
|
||||
@@ -129,7 +129,9 @@ Keep `retry.provider.maxRetries` at `0` unless provider-level retries are explic
|
||||
|---------|------|---------|-------------|
|
||||
| `steeringMode` | string | `"one-at-a-time"` | How steering messages are sent: `"all"` or `"one-at-a-time"` |
|
||||
| `followUpMode` | string | `"one-at-a-time"` | How follow-up messages are sent: `"all"` or `"one-at-a-time"` |
|
||||
| `transport` | string | `"sse"` | Preferred transport for providers that support multiple transports: `"sse"`, `"websocket"`, or `"auto"` |
|
||||
| `transport` | string | `"auto"` | Preferred transport for providers that support multiple transports: `"sse"`, `"websocket"`, `"websocket-cached"`, or `"auto"` |
|
||||
| `httpIdleTimeoutMs` | number | `300000` | HTTP header/body idle timeout in milliseconds, also used by providers with explicit stream idle timeouts. Set to `0` to disable. |
|
||||
| `websocketConnectTimeoutMs` | number | `15000` | WebSocket connect/open handshake timeout in milliseconds for providers that support WebSocket transports. Set to `0` to disable. |
|
||||
|
||||
### Terminal & Images
|
||||
|
||||
|
||||
@@ -340,11 +340,18 @@ export async function createAgentSession(options: CreateAgentSessionOptions = {}
|
||||
throw new Error(auth.error);
|
||||
}
|
||||
const providerRetrySettings = settingsManager.getProviderRetrySettings();
|
||||
const timeoutMs =
|
||||
options?.timeoutMs ??
|
||||
providerRetrySettings.timeoutMs ??
|
||||
(model.api === "openai-codex-responses" ? settingsManager.getHttpIdleTimeoutMs() : undefined);
|
||||
const websocketConnectTimeoutMs =
|
||||
options?.websocketConnectTimeoutMs ?? settingsManager.getWebSocketConnectTimeoutMs();
|
||||
const attributionHeaders = getAttributionHeaders(model, settingsManager, options?.sessionId);
|
||||
return streamSimple(model, context, {
|
||||
...options,
|
||||
apiKey: auth.apiKey,
|
||||
timeoutMs: options?.timeoutMs ?? providerRetrySettings.timeoutMs,
|
||||
timeoutMs,
|
||||
websocketConnectTimeoutMs,
|
||||
maxRetries: options?.maxRetries ?? providerRetrySettings.maxRetries,
|
||||
maxRetryDelayMs: options?.maxRetryDelayMs ?? providerRetrySettings.maxRetryDelayMs,
|
||||
headers:
|
||||
|
||||
@@ -112,6 +112,7 @@ export interface Settings {
|
||||
warnings?: WarningSettings;
|
||||
sessionDir?: string; // Custom session storage directory (same format as --session-dir CLI flag)
|
||||
httpIdleTimeoutMs?: number; // HTTP header/body idle timeout in milliseconds; 0 disables it
|
||||
websocketConnectTimeoutMs?: number; // WebSocket connect/open handshake timeout in milliseconds; 0 disables it
|
||||
}
|
||||
|
||||
/** Deep merge settings: project/overrides take precedence, nested objects merge recursively */
|
||||
@@ -145,6 +146,17 @@ function deepMergeSettings(base: Settings, overrides: Settings): Settings {
|
||||
return result;
|
||||
}
|
||||
|
||||
function parseTimeoutSetting(value: unknown, settingName: string): number | undefined {
|
||||
const timeoutMs = parseHttpIdleTimeoutMs(value);
|
||||
if (timeoutMs !== undefined) {
|
||||
return timeoutMs;
|
||||
}
|
||||
if (value !== undefined) {
|
||||
throw new Error(`Invalid ${settingName} setting: ${String(value)}`);
|
||||
}
|
||||
return undefined;
|
||||
}
|
||||
|
||||
export type SettingsScope = "global" | "project";
|
||||
|
||||
export interface SettingsStorage {
|
||||
@@ -722,15 +734,7 @@ export class SettingsManager {
|
||||
}
|
||||
|
||||
getHttpIdleTimeoutMs(): number {
|
||||
const value = this.settings.httpIdleTimeoutMs;
|
||||
const timeoutMs = parseHttpIdleTimeoutMs(value);
|
||||
if (timeoutMs !== undefined) {
|
||||
return timeoutMs;
|
||||
}
|
||||
if (value !== undefined) {
|
||||
throw new Error(`Invalid httpIdleTimeoutMs setting: ${String(value)}`);
|
||||
}
|
||||
return DEFAULT_HTTP_IDLE_TIMEOUT_MS;
|
||||
return parseTimeoutSetting(this.settings.httpIdleTimeoutMs, "httpIdleTimeoutMs") ?? DEFAULT_HTTP_IDLE_TIMEOUT_MS;
|
||||
}
|
||||
|
||||
setHttpIdleTimeoutMs(timeoutMs: number): void {
|
||||
@@ -750,6 +754,10 @@ export class SettingsManager {
|
||||
};
|
||||
}
|
||||
|
||||
getWebSocketConnectTimeoutMs(): number | undefined {
|
||||
return parseTimeoutSetting(this.settings.websocketConnectTimeoutMs, "websocketConnectTimeoutMs");
|
||||
}
|
||||
|
||||
getHideThinkingBlock(): boolean {
|
||||
return this.settings.hideThinkingBlock ?? false;
|
||||
}
|
||||
|
||||
153
packages/coding-agent/test/sdk-stream-options.test.ts
Normal file
153
packages/coding-agent/test/sdk-stream-options.test.ts
Normal file
@@ -0,0 +1,153 @@
|
||||
import { mkdirSync, mkdtempSync, rmSync } from "node:fs";
|
||||
import { tmpdir } from "node:os";
|
||||
import { join } from "node:path";
|
||||
import {
|
||||
type Api,
|
||||
type AssistantMessage,
|
||||
createAssistantMessageEventStream,
|
||||
type Model,
|
||||
type SimpleStreamOptions,
|
||||
} from "@earendil-works/pi-ai";
|
||||
import { afterEach, beforeEach, describe, expect, it } from "vitest";
|
||||
import { AuthStorage } from "../src/core/auth-storage.ts";
|
||||
import { ModelRegistry } from "../src/core/model-registry.ts";
|
||||
import { createAgentSession } from "../src/core/sdk.ts";
|
||||
import { SessionManager } from "../src/core/session-manager.ts";
|
||||
import { SettingsManager } from "../src/core/settings-manager.ts";
|
||||
|
||||
describe("createAgentSession stream options", () => {
|
||||
let tempDir: string;
|
||||
let cwd: string;
|
||||
let agentDir: string;
|
||||
|
||||
beforeEach(() => {
|
||||
tempDir = mkdtempSync(join(tmpdir(), "pi-sdk-stream-options-"));
|
||||
cwd = join(tempDir, "project");
|
||||
agentDir = join(tempDir, "agent");
|
||||
mkdirSync(cwd, { recursive: true });
|
||||
mkdirSync(agentDir, { recursive: true });
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
if (tempDir) {
|
||||
rmSync(tempDir, { recursive: true, force: true });
|
||||
}
|
||||
});
|
||||
|
||||
function createModel(api: Api): Model<Api> {
|
||||
return {
|
||||
id: "capture-model",
|
||||
name: "Capture Model",
|
||||
api,
|
||||
provider: "capture-provider",
|
||||
baseUrl: "https://capture.invalid/v1",
|
||||
reasoning: false,
|
||||
input: ["text"],
|
||||
cost: { input: 0, output: 0, cacheRead: 0, cacheWrite: 0 },
|
||||
contextWindow: 128000,
|
||||
maxTokens: 4096,
|
||||
};
|
||||
}
|
||||
|
||||
function createDoneStream(api: Api) {
|
||||
const stream = createAssistantMessageEventStream();
|
||||
const message: AssistantMessage = {
|
||||
role: "assistant",
|
||||
content: [{ type: "text", text: "ok" }],
|
||||
api,
|
||||
provider: "capture-provider",
|
||||
model: "capture-model",
|
||||
usage: {
|
||||
input: 0,
|
||||
output: 0,
|
||||
cacheRead: 0,
|
||||
cacheWrite: 0,
|
||||
totalTokens: 0,
|
||||
cost: { input: 0, output: 0, cacheRead: 0, cacheWrite: 0, total: 0 },
|
||||
},
|
||||
stopReason: "stop",
|
||||
timestamp: Date.now(),
|
||||
};
|
||||
stream.end(message);
|
||||
return stream;
|
||||
}
|
||||
|
||||
async function captureStreamOptions(
|
||||
api: Api,
|
||||
settings: { httpIdleTimeoutMs?: number; websocketConnectTimeoutMs?: number },
|
||||
requestOptions: SimpleStreamOptions = {},
|
||||
): Promise<SimpleStreamOptions | undefined> {
|
||||
const model = createModel(api);
|
||||
const settingsManager = SettingsManager.inMemory(settings);
|
||||
|
||||
const authStorage = AuthStorage.create(join(agentDir, "auth.json"));
|
||||
authStorage.setRuntimeApiKey(model.provider, "test-api-key");
|
||||
const modelRegistry = ModelRegistry.create(authStorage, join(agentDir, "models.json"));
|
||||
let capturedOptions: SimpleStreamOptions | undefined;
|
||||
|
||||
modelRegistry.registerProvider(model.provider, {
|
||||
api,
|
||||
streamSimple: (_model, _context, providerOptions) => {
|
||||
capturedOptions = providerOptions;
|
||||
return createDoneStream(api);
|
||||
},
|
||||
});
|
||||
|
||||
const sessionManager = SessionManager.inMemory(cwd);
|
||||
const { session } = await createAgentSession({
|
||||
cwd,
|
||||
agentDir,
|
||||
model,
|
||||
authStorage,
|
||||
modelRegistry,
|
||||
settingsManager,
|
||||
sessionManager,
|
||||
});
|
||||
|
||||
try {
|
||||
await session.agent.streamFn(model, { messages: [] }, requestOptions);
|
||||
return capturedOptions;
|
||||
} finally {
|
||||
session.dispose();
|
||||
modelRegistry.unregisterProvider(model.provider);
|
||||
}
|
||||
}
|
||||
|
||||
it("forwards httpIdleTimeoutMs as timeoutMs for OpenAI Codex", async () => {
|
||||
const options = await captureStreamOptions("openai-codex-responses", { httpIdleTimeoutMs: 1234 });
|
||||
|
||||
expect(options?.timeoutMs).toBe(1234);
|
||||
});
|
||||
|
||||
it("does not default timeoutMs from httpIdleTimeoutMs for other providers", async () => {
|
||||
const options = await captureStreamOptions("openai-completions", { httpIdleTimeoutMs: 1234 });
|
||||
|
||||
expect(options?.timeoutMs).toBeUndefined();
|
||||
});
|
||||
|
||||
it("lets request timeoutMs override httpIdleTimeoutMs for OpenAI Codex", async () => {
|
||||
const options = await captureStreamOptions(
|
||||
"openai-codex-responses",
|
||||
{ httpIdleTimeoutMs: 1234 },
|
||||
{ timeoutMs: 0 },
|
||||
);
|
||||
|
||||
expect(options?.timeoutMs).toBe(0);
|
||||
});
|
||||
|
||||
it("forwards websocketConnectTimeoutMs from settings", async () => {
|
||||
const options = await captureStreamOptions("openai-codex-responses", { websocketConnectTimeoutMs: 1234 });
|
||||
|
||||
expect(options?.websocketConnectTimeoutMs).toBe(1234);
|
||||
});
|
||||
|
||||
it("lets request websocketConnectTimeoutMs override settings", async () => {
|
||||
const options = await captureStreamOptions(
|
||||
"openai-codex-responses",
|
||||
{ websocketConnectTimeoutMs: 1234 },
|
||||
{ websocketConnectTimeoutMs: 0 },
|
||||
);
|
||||
|
||||
expect(options?.websocketConnectTimeoutMs).toBe(0);
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user