From 3fdba65da4cd1b41003a69d67be4ff7f4397723e Mon Sep 17 00:00:00 2001 From: roboomp Date: Wed, 1 Jul 2026 17:24:27 +0000 Subject: [PATCH 1/3] fix(inspect-image): bounded per-request timeout on the vision-model call MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Wraps InspectImageTool.execute's instrumentedCompleteSimple call with an AbortSignal.timeout combined via AbortSignal.any with the caller's abort signal, so a stalled provider fails fast with a ToolError instead of hanging until manual abort. Timeout is configurable via new inspect_image.timeoutMs setting (default 180000ms, 0 disables). Manual abort still surfaces as 'inspect_image request aborted' — the tool distinguishes timeout via timeoutSignal.aborted && !signal.aborted. Fixes #4165 --- packages/coding-agent/CHANGELOG.md | 8 ++ .../src/config/settings-schema.ts | 19 +++++ .../coding-agent/src/tools/inspect-image.ts | 67 ++++++++++----- .../test/tools/inspect-image.test.ts | 81 +++++++++++++++++++ 4 files changed, 153 insertions(+), 22 deletions(-) diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index d1722f2ec..ea6ea37df 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -2,6 +2,14 @@ ## [Unreleased] +### Added + +- Added a configurable per-request timeout for the `inspect_image` tool (`inspect_image.timeoutMs`, default 3 minutes; set to 0 to disable) so a stalled vision-model provider fails fast with a clear error instead of blocking until manual abort ([#4165](https://github.com/can1357/oh-my-pi/issues/4165)). + +### Fixed + +- Fixed `inspect_image` blocking indefinitely when the vision-model API stalls by combining the caller's abort signal with an `AbortSignal.timeout()` and surfacing a distinct timeout `ToolError` (separate from user-triggered abort) ([#4165](https://github.com/can1357/oh-my-pi/issues/4165)). + ## [16.2.12] - 2026-07-01 ### Breaking Changes diff --git a/packages/coding-agent/src/config/settings-schema.ts b/packages/coding-agent/src/config/settings-schema.ts index 29e5b4499..bf66657fe 100644 --- a/packages/coding-agent/src/config/settings-schema.ts +++ b/packages/coding-agent/src/config/settings-schema.ts @@ -3521,6 +3521,25 @@ export const SETTINGS_SCHEMA = { }, }, + "inspect_image.timeoutMs": { + type: "number", + default: 180_000, + ui: { + tab: "tools", + group: "Execution", + label: "Inspect Image Timeout", + description: + "Per-request timeout for the inspect_image vision-model call, in milliseconds. A stalled provider fails fast with a timeout error instead of blocking until manual abort. Set to 0 to disable the timeout.", + options: [ + { value: "0", label: "Disabled" }, + { value: "60000", label: "1 minute" }, + { value: "120000", label: "2 minutes" }, + { value: "180000", label: "3 minutes" }, + { value: "300000", label: "5 minutes" }, + ], + }, + }, + "checkpoint.enabled": { type: "boolean", default: false, diff --git a/packages/coding-agent/src/tools/inspect-image.ts b/packages/coding-agent/src/tools/inspect-image.ts index a270c5dbc..9373ab183 100644 --- a/packages/coding-agent/src/tools/inspect-image.ts +++ b/packages/coding-agent/src/tools/inspect-image.ts @@ -1,6 +1,6 @@ import type { AgentTool, AgentToolContext, AgentToolResult, AgentToolUpdateCallback } from "@oh-my-pi/pi-agent-core"; import { instrumentedCompleteSimple, resolveTelemetry } from "@oh-my-pi/pi-agent-core"; -import { type Api, completeSimple, type ImageContent, type Model, type ToolExample } from "@oh-my-pi/pi-ai"; +import { type Api, type AssistantMessage, completeSimple, type ImageContent, type Model, type ToolExample } from "@oh-my-pi/pi-ai"; import { prompt } from "@oh-my-pi/pi-utils"; import { type } from "arktype"; import { extractTextContent } from "../commit/utils"; @@ -212,32 +212,55 @@ export class InspectImageTool implements AgentTool 0; + const timeoutSignal = hasTimeout ? AbortSignal.timeout(timeoutMs) : undefined; + const effectiveSignal = timeoutSignal + ? signal + ? AbortSignal.any([signal, timeoutSignal]) + : timeoutSignal + : signal; + const timedOut = (): boolean => Boolean(timeoutSignal?.aborted) && !signal?.aborted; + const formatTimeoutMessage = (): string => { + const seconds = timeoutMs % 1000 === 0 ? `${timeoutMs / 1000}` : (timeoutMs / 1000).toFixed(1); + return `inspect_image request timed out after ${seconds}s. Increase inspect_image.timeoutMs (currently ${timeoutMs}ms; 0 disables) or check the vision model provider.`; + }; + + let response: AssistantMessage; + try { + response = await instrumentedCompleteSimple( + model, + { + systemPrompt: [prompt.render(inspectImageSystemPromptTemplate)], + messages: [ + { + role: "user", + content: [ + { type: "image", data: imageInput.data, mimeType: imageInput.mimeType }, + { type: "text", text: params.question }, + ], + timestamp: Date.now(), + }, + ], + }, + { + apiKey: modelRegistry.resolver(model, this.session.getSessionId?.() ?? undefined), + signal: effectiveSignal, + }, + { telemetry, oneshotKind: "inspect_image", completeImpl: this.completeImageRequest }, + ); + } catch (error) { + if (error instanceof Error && (error.name === "AbortError" || error.name === "TimeoutError")) { + if (timedOut()) throw new ToolError(formatTimeoutMessage()); + } + throw error; + } if (response.stopReason === "error") { throw new ToolError(response.errorMessage ?? "inspect_image request failed."); } if (response.stopReason === "aborted") { + if (timedOut()) throw new ToolError(formatTimeoutMessage()); throw new ToolError("inspect_image request aborted."); } diff --git a/packages/coding-agent/test/tools/inspect-image.test.ts b/packages/coding-agent/test/tools/inspect-image.test.ts index 91dfcc661..cb30ff7ae 100644 --- a/packages/coding-agent/test/tools/inspect-image.test.ts +++ b/packages/coding-agent/test/tools/inspect-image.test.ts @@ -121,6 +121,39 @@ function createCompleteSimpleForbiddenStub(): CompleteSimpleStub { return { calls, fn }; } +function createCompleteSimpleHangingStub(): CompleteSimpleStub { + const calls: unknown[][] = []; + const fn = (async (...args: unknown[]) => { + calls.push(args); + const options = args[2] as { signal?: AbortSignal } | undefined; + const stubSignal = options?.signal; + await new Promise(resolve => { + if (!stubSignal) return; + if (stubSignal.aborted) return resolve(); + stubSignal.addEventListener("abort", () => resolve(), { once: true }); + }); + return { + role: "assistant", + api: visionModel.api, + provider: visionModel.provider, + model: visionModel.id, + usage: { + input: 0, + output: 0, + cacheRead: 0, + cacheWrite: 0, + totalTokens: 0, + cost: { input: 0, output: 0, cacheRead: 0, cacheWrite: 0 }, + }, + stopReason: "aborted", + timestamp: Date.now(), + content: [], + }; + }) as typeof completeSimple; + + return { calls, fn }; +} + describe("InspectImageTool", () => { let testDir: string; @@ -401,4 +434,52 @@ describe("InspectImageTool", () => { ); expect(stub.calls).toHaveLength(0); }); + + it("times out with a configured error when the vision-model call stalls", async () => { + const imagePath = path.join(testDir, "screen.png"); + fs.writeFileSync(imagePath, Buffer.from(TINY_PNG_BASE64, "base64")); + + const stub = createCompleteSimpleHangingStub(); + const settings = Settings.isolated({ "inspect_image.timeoutMs": 50 }); + const tool = new InspectImageTool(createSession(testDir, visionModel, "test-key", settings), stub.fn); + + const start = Date.now(); + await expect(tool.execute("call-timeout", { path: imagePath, question: "Anything?" })).rejects.toThrow( + /inspect_image request timed out.*inspect_image\.timeoutMs.*50ms/, + ); + const elapsed = Date.now() - start; + expect(elapsed).toBeLessThan(5000); + expect(stub.calls).toHaveLength(1); + }); + + it("surfaces manual abort as aborted, not as timed out", async () => { + const imagePath = path.join(testDir, "screen.png"); + fs.writeFileSync(imagePath, Buffer.from(TINY_PNG_BASE64, "base64")); + + const stub = createCompleteSimpleHangingStub(); + const settings = Settings.isolated({ "inspect_image.timeoutMs": 60_000 }); + const tool = new InspectImageTool(createSession(testDir, visionModel, "test-key", settings), stub.fn); + const controller = new AbortController(); + + const pending = tool.execute("call-manual-abort", { path: imagePath, question: "Anything?" }, controller.signal); + setTimeout(() => controller.abort(), 25); + await expect(pending).rejects.toThrow(/inspect_image request aborted/); + await expect(pending).rejects.not.toThrow(/timed out/); + expect(stub.calls).toHaveLength(1); + }); + + it("skips the timeout guard when inspect_image.timeoutMs is zero", async () => { + const imagePath = path.join(testDir, "screen.png"); + fs.writeFileSync(imagePath, Buffer.from(TINY_PNG_BASE64, "base64")); + + const stub = createCompleteSimpleSuccessStub("Timeout disabled path"); + const settings = Settings.isolated({ "inspect_image.timeoutMs": 0 }); + const tool = new InspectImageTool(createSession(testDir, visionModel, "test-key", settings), stub.fn); + + const result = await tool.execute("call-timeout-disabled", { path: imagePath, question: "Anything?" }); + expect(result.content).toEqual([{ type: "text", text: "Timeout disabled path" }]); + expect(stub.calls).toHaveLength(1); + const passed = stub.calls[0]?.[2] as { signal?: AbortSignal } | undefined; + expect(passed?.signal).toBeUndefined(); + }); }); From fd9974d9345dc526d3054761ad75e3229b0bc3cd Mon Sep 17 00:00:00 2001 From: roboomp Date: Wed, 1 Jul 2026 17:25:26 +0000 Subject: [PATCH 2/3] style: bun run fix --- packages/coding-agent/src/tools/inspect-image.ts | 9 ++++++++- packages/coding-agent/test/tools/inspect-image.test.ts | 2 +- 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/packages/coding-agent/src/tools/inspect-image.ts b/packages/coding-agent/src/tools/inspect-image.ts index 9373ab183..99a8aeacd 100644 --- a/packages/coding-agent/src/tools/inspect-image.ts +++ b/packages/coding-agent/src/tools/inspect-image.ts @@ -1,6 +1,13 @@ import type { AgentTool, AgentToolContext, AgentToolResult, AgentToolUpdateCallback } from "@oh-my-pi/pi-agent-core"; import { instrumentedCompleteSimple, resolveTelemetry } from "@oh-my-pi/pi-agent-core"; -import { type Api, type AssistantMessage, completeSimple, type ImageContent, type Model, type ToolExample } from "@oh-my-pi/pi-ai"; +import { + type Api, + type AssistantMessage, + completeSimple, + type ImageContent, + type Model, + type ToolExample, +} from "@oh-my-pi/pi-ai"; import { prompt } from "@oh-my-pi/pi-utils"; import { type } from "arktype"; import { extractTextContent } from "../commit/utils"; diff --git a/packages/coding-agent/test/tools/inspect-image.test.ts b/packages/coding-agent/test/tools/inspect-image.test.ts index cb30ff7ae..acb5521c4 100644 --- a/packages/coding-agent/test/tools/inspect-image.test.ts +++ b/packages/coding-agent/test/tools/inspect-image.test.ts @@ -149,7 +149,7 @@ function createCompleteSimpleHangingStub(): CompleteSimpleStub { timestamp: Date.now(), content: [], }; - }) as typeof completeSimple; + }) as unknown as typeof completeSimple; return { calls, fn }; } From d2d5e40bb220b85997062fb37cd8435b72d1df62 Mon Sep 17 00:00:00 2001 From: roboomp Date: Wed, 1 Jul 2026 17:28:17 +0000 Subject: [PATCH 3/3] fix(inspect-image): raise default timeoutMs to 300000 (5 min) Reporter feedback on #4165: the initial 180s default is too short for some legitimate vision-model calls (evidence in the issue showed a successful 146s call). Bump the inspect_image.timeoutMs default to 300000 ms (5 min), matching the highest preset option already listed under the setting; user-tunable and 0 still disables. Fixes #4165 --- packages/coding-agent/CHANGELOG.md | 2 +- packages/coding-agent/src/config/settings-schema.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index ea6ea37df..f4a03955e 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -4,7 +4,7 @@ ### Added -- Added a configurable per-request timeout for the `inspect_image` tool (`inspect_image.timeoutMs`, default 3 minutes; set to 0 to disable) so a stalled vision-model provider fails fast with a clear error instead of blocking until manual abort ([#4165](https://github.com/can1357/oh-my-pi/issues/4165)). +- Added a configurable per-request timeout for the `inspect_image` tool (`inspect_image.timeoutMs`, default 5 minutes; set to 0 to disable) so a stalled vision-model provider fails fast with a clear error instead of blocking until manual abort ([#4165](https://github.com/can1357/oh-my-pi/issues/4165)). ### Fixed diff --git a/packages/coding-agent/src/config/settings-schema.ts b/packages/coding-agent/src/config/settings-schema.ts index bf66657fe..dae902714 100644 --- a/packages/coding-agent/src/config/settings-schema.ts +++ b/packages/coding-agent/src/config/settings-schema.ts @@ -3523,7 +3523,7 @@ export const SETTINGS_SCHEMA = { "inspect_image.timeoutMs": { type: "number", - default: 180_000, + default: 300_000, ui: { tab: "tools", group: "Execution",