diff --git a/src/tools/look-at/tools.test.ts b/src/tools/look-at/tools.test.ts index 3299aba2e..8f97ea238 100644 --- a/src/tools/look-at/tools.test.ts +++ b/src/tools/look-at/tools.test.ts @@ -1,4 +1,4 @@ -import { describe, expect, test } from "bun:test" +import { describe, expect, test, mock } from "bun:test" import type { ToolContext } from "@opencode-ai/plugin/tool" import { normalizeArgs, validateArgs, createLookAt } from "./tools" @@ -111,16 +111,15 @@ describe("look-at tool", () => { }) describe("createLookAt error handling", () => { - // given promptAsync throws error + // given sync prompt throws and no messages available // when LookAt tool executed - // then returns error string immediately (no message fetch) - test("returns error immediately when promptAsync fails", async () => { + // then returns no-response error (fetches messages after catching prompt error) + test("returns no-response error when prompt fails and no messages exist", async () => { const mockClient = { session: { get: async () => ({ data: { directory: "/project" } }), create: async () => ({ data: { id: "ses_test_prompt_fail" } }), - promptAsync: async () => { throw new Error("Network connection failed") }, - status: async () => ({ data: {} }), + prompt: async () => { throw new Error("Network connection failed") }, messages: async () => ({ data: [] }), }, } @@ -146,51 +145,10 @@ describe("look-at tool", () => { toolContext, ) expect(result).toContain("Error") - expect(result).toContain("Network connection failed") + expect(result).toContain("multimodal-looker") }) - // given promptAsync succeeds but status API fails (polling degrades gracefully) - // when LookAt tool executed - // then still attempts to fetch messages (graceful degradation) - test("fetches messages even when status API fails", async () => { - const mockClient = { - session: { - get: async () => ({ data: { directory: "/project" } }), - create: async () => ({ data: { id: "ses_test_poll_timeout" } }), - promptAsync: async () => ({}), - status: async () => ({ error: new Error("status unavailable") }), - messages: async () => ({ - data: [ - { info: { role: "assistant", time: { created: 1 } }, parts: [{ type: "text", text: "partial result" }] }, - ], - }), - }, - } - - const tool = createLookAt({ - client: mockClient, - directory: "/project", - } as any) - - const toolContext: ToolContext = { - sessionID: "parent-session", - messageID: "parent-message", - agent: "sisyphus", - directory: "/project", - worktree: "/project", - abort: new AbortController().signal, - metadata: () => {}, - ask: async () => {}, - } - - const result = await tool.execute( - { file_path: "/test/file.png", goal: "analyze" }, - toolContext, - ) - expect(result).toBe("partial result") - }) - - // given promptAsync succeeds and session becomes idle + // given sync prompt succeeds // when LookAt tool executed and no assistant message found // then returns error about no response test("returns error when no assistant message after successful prompt", async () => { @@ -198,8 +156,7 @@ describe("look-at tool", () => { session: { get: async () => ({ data: { directory: "/project" } }), create: async () => ({ data: { id: "ses_test_no_msg" } }), - promptAsync: async () => ({}), - status: async () => ({ data: {} }), + prompt: async () => ({}), messages: async () => ({ data: [] }), }, } @@ -236,8 +193,7 @@ describe("look-at tool", () => { session: { get: async () => ({ data: { directory: "/project" } }), create: async () => ({ error: "Internal server error" }), - promptAsync: async () => ({}), - status: async () => ({ data: {} }), + prompt: async () => ({}), messages: async () => ({ data: [] }), }, } @@ -270,8 +226,8 @@ describe("look-at tool", () => { describe("createLookAt model passthrough", () => { // given multimodal-looker agent has resolved model info // when LookAt tool executed - // then model info should be passed to promptAsync - test("passes multimodal-looker model to promptAsync when available", async () => { + // then model info should be passed to sync prompt + test("passes multimodal-looker model to sync prompt when available", async () => { let promptBody: any const mockClient = { @@ -289,11 +245,10 @@ describe("look-at tool", () => { session: { get: async () => ({ data: { directory: "/project" } }), create: async () => ({ data: { id: "ses_model_passthrough" } }), - promptAsync: async (input: any) => { + prompt: async (input: any) => { promptBody = input.body return { data: {} } }, - status: async () => ({ data: {} }), messages: async () => ({ data: [ { info: { role: "assistant", time: { created: 1 } }, parts: [{ type: "text", text: "done" }] }, @@ -330,10 +285,154 @@ describe("look-at tool", () => { }) }) + describe("createLookAt sync prompt (race condition fix)", () => { + // given look_at needs response immediately after prompt returns + // when tool is executed + // then must use synchronous prompt (session.prompt), NOT async (session.promptAsync) + test("uses synchronous prompt to avoid race condition with polling", async () => { + const syncPrompt = mock(async () => ({})) + const asyncPrompt = mock(async () => ({})) + const statusFn = mock(async () => ({ data: {} })) + + const mockClient = { + app: { + agents: async () => ({ data: [] }), + }, + session: { + get: async () => ({ data: { directory: "/project" } }), + create: async () => ({ data: { id: "ses_sync_test" } }), + prompt: syncPrompt, + promptAsync: asyncPrompt, + status: statusFn, + messages: async () => ({ + data: [ + { info: { role: "assistant", time: { created: 1 } }, parts: [{ type: "text", text: "result" }] }, + ], + }), + }, + } + + const tool = createLookAt({ + client: mockClient, + directory: "/project", + } as any) + + const toolContext: ToolContext = { + sessionID: "parent-session", + messageID: "parent-message", + agent: "sisyphus", + directory: "/project", + worktree: "/project", + abort: new AbortController().signal, + metadata: () => {}, + ask: async () => {}, + } + + const result = await tool.execute( + { file_path: "/test/file.png", goal: "analyze" }, + toolContext, + ) + + expect(result).toBe("result") + expect(syncPrompt).toHaveBeenCalledTimes(1) + expect(asyncPrompt).not.toHaveBeenCalled() + expect(statusFn).not.toHaveBeenCalled() + }) + + // given sync prompt throws (JSON parse error even on success) + // when tool is executed + // then catches error gracefully and still fetches messages + test("catches sync prompt errors and still fetches messages", async () => { + const mockClient = { + app: { + agents: async () => ({ data: [] }), + }, + session: { + get: async () => ({ data: { directory: "/project" } }), + create: async () => ({ data: { id: "ses_sync_error" } }), + prompt: async () => { throw new Error("JSON parse error") }, + promptAsync: async () => ({}), + status: async () => ({ data: {} }), + messages: async () => ({ + data: [ + { info: { role: "assistant", time: { created: 1 } }, parts: [{ type: "text", text: "result despite error" }] }, + ], + }), + }, + } + + const tool = createLookAt({ + client: mockClient, + directory: "/project", + } as any) + + const toolContext: ToolContext = { + sessionID: "parent-session", + messageID: "parent-message", + agent: "sisyphus", + directory: "/project", + worktree: "/project", + abort: new AbortController().signal, + metadata: () => {}, + ask: async () => {}, + } + + const result = await tool.execute( + { file_path: "/test/file.png", goal: "analyze" }, + toolContext, + ) + + expect(result).toBe("result despite error") + }) + + // given sync prompt throws and no messages available + // when tool is executed + // then returns error about no response + test("returns no-response error when sync prompt fails and no messages", async () => { + const mockClient = { + app: { + agents: async () => ({ data: [] }), + }, + session: { + get: async () => ({ data: { directory: "/project" } }), + create: async () => ({ data: { id: "ses_sync_no_msg" } }), + prompt: async () => { throw new Error("Connection refused") }, + promptAsync: async () => ({}), + status: async () => ({ data: {} }), + messages: async () => ({ data: [] }), + }, + } + + const tool = createLookAt({ + client: mockClient, + directory: "/project", + } as any) + + const toolContext: ToolContext = { + sessionID: "parent-session", + messageID: "parent-message", + agent: "sisyphus", + directory: "/project", + worktree: "/project", + abort: new AbortController().signal, + metadata: () => {}, + ask: async () => {}, + } + + const result = await tool.execute( + { file_path: "/test/file.png", goal: "analyze" }, + toolContext, + ) + + expect(result).toContain("Error") + expect(result).toContain("multimodal-looker") + }) + }) + describe("createLookAt with image_data", () => { // given base64 image data is provided // when LookAt tool executed - // then should send data URL to promptAsync + // then should send data URL to sync prompt test("sends data URL when image_data provided", async () => { let promptBody: any @@ -344,11 +443,10 @@ describe("look-at tool", () => { session: { get: async () => ({ data: { directory: "/project" } }), create: async () => ({ data: { id: "ses_image_data_test" } }), - promptAsync: async (input: any) => { + prompt: async (input: any) => { promptBody = input.body return { data: {} } }, - status: async () => ({ data: {} }), messages: async () => ({ data: [ { info: { role: "assistant", time: { created: 1 } }, parts: [{ type: "text", text: "analyzed" }] }, @@ -398,11 +496,10 @@ describe("look-at tool", () => { session: { get: async () => ({ data: { directory: "/project" } }), create: async () => ({ data: { id: "ses_raw_base64_test" } }), - promptAsync: async (input: any) => { + prompt: async (input: any) => { promptBody = input.body return { data: {} } }, - status: async () => ({ data: {} }), messages: async () => ({ data: [ { info: { role: "assistant", time: { created: 1 } }, parts: [{ type: "text", text: "analyzed" }] }, diff --git a/src/tools/look-at/tools.ts b/src/tools/look-at/tools.ts index dcb920f08..0d5c1c0b7 100644 --- a/src/tools/look-at/tools.ts +++ b/src/tools/look-at/tools.ts @@ -3,8 +3,7 @@ import { pathToFileURL } from "node:url" import { tool, type PluginInput, type ToolDefinition } from "@opencode-ai/plugin" import { LOOK_AT_DESCRIPTION, MULTIMODAL_LOOKER_AGENT } from "./constants" import type { LookAtArgs } from "./types" -import { log, promptWithModelSuggestionRetry } from "../../shared" -import { pollSessionUntilIdle } from "./session-poller" +import { log, promptSyncWithModelSuggestionRetry } from "../../shared" import { extractLatestAssistantText } from "./assistant-message-extractor" import type { LookAtArgsWithAlias } from "./look-at-arguments" import { normalizeArgs, validateArgs } from "./look-at-arguments" @@ -106,9 +105,9 @@ Original error: ${createResult.error}` const { agentModel, agentVariant } = await resolveMultimodalLookerAgentMetadata(ctx) - log(`[look_at] Sending async prompt with ${isBase64Input ? "base64 image" : "file"} to session ${sessionID}`) + log(`[look_at] Sending prompt with ${isBase64Input ? "base64 image" : "file"} to session ${sessionID}`) try { - await promptWithModelSuggestionRetry(ctx.client, { + await promptSyncWithModelSuggestionRetry(ctx.client, { path: { id: sessionID }, body: { agent: MULTIMODAL_LOOKER_AGENT, @@ -127,15 +126,7 @@ Original error: ${createResult.error}` }, }) } catch (promptError) { - log(`[look_at] promptAsync error:`, promptError) - return `Error: Failed to send prompt to multimodal-looker agent: ${promptError instanceof Error ? promptError.message : String(promptError)}` - } - - log(`[look_at] Polling session ${sessionID} until idle...`) - try { - await pollSessionUntilIdle(ctx.client, sessionID, { pollIntervalMs: 500, timeoutMs: 120_000 }) - } catch (pollError) { - log(`[look_at] Polling error (will still try to fetch messages):`, pollError) + log(`[look_at] Prompt error (ignored, will still fetch messages):`, promptError) } log(`[look_at] Fetching messages from session ${sessionID}...`)