diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index 6b64830f4..5733d87b3 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -352,6 +352,7 @@ - Fixed Cursor-provider turns whose assistant message contains intro text, tool calls, and trailing final text rendering the final answer above the tool output instead of at the transcript tail ([#4871](https://github.com/can1357/oh-my-pi/issues/4871)). - Fixed `omp -p` looking hung while a text-mode prompt is in flight by writing a one-shot working indicator to stderr before awaiting the model response. ([#4901](https://github.com/can1357/oh-my-pi/issues/4901)) - Fixed autolearn auto-continue firing a capture turn after an aborted stop (Esc/cancel): the controller now skips any `agent_end` whose last assistant message has `stopReason: "aborted"`. +- Fixed MCP tools receiving session image attachments as raw `local://...` URIs by resolving them to session-local filesystem paths before `tools/call` is sent ([#4946](https://github.com/can1357/oh-my-pi/issues/4946)). ## [16.3.12] - 2026-07-08 diff --git a/packages/coding-agent/src/extensibility/custom-tools/types.ts b/packages/coding-agent/src/extensibility/custom-tools/types.ts index 166a9820c..7beb91624 100644 --- a/packages/coding-agent/src/extensibility/custom-tools/types.ts +++ b/packages/coding-agent/src/extensibility/custom-tools/types.ts @@ -23,6 +23,7 @@ import type { Settings } from "../../config/settings"; import type { ExecOptions, ExecResult } from "../../exec/exec"; import type { HookUIContext } from "../../extensibility/hooks/types"; import type * as PiCodingAgent from "../../index"; +import type { LocalProtocolOptions } from "../../internal-urls/local-protocol"; import type { Theme } from "../../modes/theme/theme"; import type { ReadonlySessionManager } from "../../session/session-manager"; import type { TodoItem } from "../../tools/todo"; @@ -96,6 +97,8 @@ export interface CustomToolContext { settings?: Settings; /** Fetch implementation for outbound HTTP; defaults to global fetch when omitted. */ fetch?: FetchImpl; + /** Calling session's `local://` root mapping for tools that bridge out of the OMP process. */ + localProtocolOptions?: LocalProtocolOptions; /** Whether to auto-approve all destructive tool operations (--auto-approve CLI flag) */ autoApprove?: boolean; } diff --git a/packages/coding-agent/src/extensibility/extensions/runner.ts b/packages/coding-agent/src/extensibility/extensions/runner.ts index bec6b662c..a26f805a0 100644 --- a/packages/coding-agent/src/extensibility/extensions/runner.ts +++ b/packages/coding-agent/src/extensibility/extensions/runner.ts @@ -7,6 +7,7 @@ import type { KeyId } from "@oh-my-pi/pi-tui"; import { logger } from "@oh-my-pi/pi-utils"; import type { ModelRegistry } from "../../config/model-registry"; import type { Settings } from "../../config/settings"; +import type { LocalProtocolOptions } from "../../internal-urls/local-protocol"; import type { MemoryRuntimeContext } from "../../memory-backend"; import { type Theme, theme } from "../../modes/theme/theme"; import type { SessionManager } from "../../session/session-manager"; @@ -256,6 +257,7 @@ export class ExtensionRunner { private readonly modelRegistry: ModelRegistry, getMemory?: () => MemoryRuntimeContext | undefined, private readonly settings?: Settings, + private readonly localProtocolOptions?: LocalProtocolOptions, ) { this.#uiContext = noOpUIContext; this.#getMemoryFn = getMemory; @@ -538,6 +540,7 @@ export class ExtensionRunner { hasPendingMessages: () => this.#hasPendingMessagesFn(), shutdown: () => this.#shutdownHandler(), getSystemPrompt: () => this.#getSystemPromptFn(), + localProtocolOptions: this.localProtocolOptions, memory: this.#getMemoryFn?.(), }; } diff --git a/packages/coding-agent/src/extensibility/extensions/types.ts b/packages/coding-agent/src/extensibility/extensions/types.ts index 97a1fccbf..551f50af0 100644 --- a/packages/coding-agent/src/extensibility/extensions/types.ts +++ b/packages/coding-agent/src/extensibility/extensions/types.ts @@ -41,6 +41,7 @@ import type { PythonResult } from "../../eval/py/executor"; import type { BashResult } from "../../exec/bash-executor"; import type { ExecOptions, ExecResult } from "../../exec/exec"; import type * as PiCodingAgent from "../../index"; +import type { LocalProtocolOptions } from "../../internal-urls/local-protocol"; import type { MemoryRuntimeContext } from "../../memory-backend"; import type { CustomEditor } from "../../modes/components/custom-editor"; import type { Theme } from "../../modes/theme/theme"; @@ -420,6 +421,8 @@ export interface ExtensionContext { sessionManager: ReadonlySessionManager; /** Model registry for API key resolution */ modelRegistry: ModelRegistry; + /** Calling session's `local://` root mapping for external tool bridges. */ + localProtocolOptions?: LocalProtocolOptions; /** Current model (may be undefined) */ model: Model | undefined; /** Read-only model query facade: list / current / resolve / family. */ diff --git a/packages/coding-agent/src/mcp/tool-bridge.ts b/packages/coding-agent/src/mcp/tool-bridge.ts index 0a64e7446..2a10e4140 100644 --- a/packages/coding-agent/src/mcp/tool-bridge.ts +++ b/packages/coding-agent/src/mcp/tool-bridge.ts @@ -15,8 +15,10 @@ import type { CustomToolResult, RenderResultOptions, } from "../extensibility/custom-tools/types"; +import { resolveLocalUrlToFile } from "../internal-urls/local-protocol"; import type { Theme } from "../modes/theme/theme"; import type { OutputMeta } from "../tools/output-meta"; +import { normalizeLocalScheme } from "../tools/path-utils"; import { ToolAbortError, throwIfAborted } from "../tools/tool-errors"; import { callTool } from "./client"; import { renderMCPCall, renderMCPResult } from "./render"; @@ -104,13 +106,62 @@ function stripHarnessIntent(args: MCPToolArgs, inputSchema: MCPToolDefinition["i return rest; } +async function resolveOutboundLocalUrlArgs( + value: unknown, + context: CustomToolContext, + seen: WeakSet = new WeakSet(), +): Promise { + if (typeof value === "string") { + const normalized = normalizeLocalScheme(value); + if (!normalized.startsWith("local://")) return value; + const localFile = await resolveLocalUrlToFile(normalized, { + cwd: context.sessionManager?.getCwd?.(), + settings: context.settings, + localProtocolOptions: context.localProtocolOptions, + }); + return localFile?.path ?? value; + } + if (typeof value !== "object" || value === null) return value; + if (seen.has(value)) return value; + seen.add(value); + + if (Array.isArray(value)) { + let resolved: unknown[] | undefined; + for (let index = 0; index < value.length; index++) { + const item = value[index]; + const next = await resolveOutboundLocalUrlArgs(item, context, seen); + if (next === item && !resolved) continue; + resolved ??= value.slice(); + resolved[index] = next; + } + return resolved ?? value; + } + + const input = value as Record; + let resolved: Record | undefined; + for (const key in input) { + const item = input[key]; + const next = await resolveOutboundLocalUrlArgs(item, context, seen); + if (next === item && !resolved) continue; + resolved ??= { ...input }; + resolved[key] = next; + } + return resolved ?? value; +} + /** * Normalize raw tool params into the outbound `tools/call` arguments: strip - * the harness intent field, then drop optional empty placeholders the server - * declares but doesn't require. + * the harness intent field, drop optional empty placeholders the server + * declares but doesn't require, then translate session-local files to paths + * external MCP servers can read. */ -function prepareOutboundArgs(params: unknown, inputSchema: MCPToolDefinition["inputSchema"]): MCPToolArgs { - return omitUnusedOptionalArgs(stripHarnessIntent(normalizeToolArgs(params), inputSchema), inputSchema); +async function prepareOutboundArgs( + params: unknown, + inputSchema: MCPToolDefinition["inputSchema"], + context: CustomToolContext, +): Promise { + const args = omitUnusedOptionalArgs(stripHarnessIntent(normalizeToolArgs(params), inputSchema), inputSchema); + return (await resolveOutboundLocalUrlArgs(args, context)) as MCPToolArgs; } /** Details included in MCP tool results for rendering */ @@ -316,7 +367,7 @@ export class MCPTool implements CustomTool { signal?: AbortSignal, ): Promise> { throwIfAborted(signal); - const args = prepareOutboundArgs(params, this.tool.inputSchema); + const args = await prepareOutboundArgs(params, this.tool.inputSchema, _ctx); const provider = this.connection._source?.provider; const providerName = this.connection._source?.providerName; @@ -415,7 +466,7 @@ export class DeferredMCPTool implements CustomTool { signal?: AbortSignal, ): Promise> { throwIfAborted(signal); - const args = prepareOutboundArgs(params, this.tool.inputSchema); + const args = await prepareOutboundArgs(params, this.tool.inputSchema, _ctx); const provider = this.#fallbackProvider; const providerName = this.#fallbackProviderName; diff --git a/packages/coding-agent/src/prompts/system/workflow-notice.md b/packages/coding-agent/src/prompts/system/workflow-notice.md index 74eddee21..3e62ca5b0 100644 --- a/packages/coding-agent/src/prompts/system/workflow-notice.md +++ b/packages/coding-agent/src/prompts/system/workflow-notice.md @@ -51,20 +51,20 @@ Decompose first, then {{#if taskBatch}}batch the independent leaves{{else}}issue {{#if taskBatch}} task( - context: "# Goal\nReview the auth diff...\n# Constraints\nRead-only...\n# Contract\nReturn findings as severity/file/line/fix...", + context: "# Goal\nReview the auth diff…\n# Constraints\nRead-only…\n# Contract\nReturn findings as severity/file/line/fix…", tasks: [ - { id: "AuthOwner", role: "Auth Storage Reviewer", assignment: "# Target\npackages/ai/src/auth-storage.ts\n# Change\nTrace credential selection...\n# Acceptance\nReturn confirmed findings only..." }, - { id: "PromptOwner", role: "Prompt Contract Reviewer", assignment: "# Target\npackages/coding-agent/src/prompts/**\n# Change\nCheck active-tool guidance...\n# Acceptance\nReturn mismatches and exact prompt lines..." }, + { id: "AuthOwner", role: "Auth Storage Reviewer", assignment: "# Target\npackages/ai/src/auth-storage.ts\n# Change\nTrace credential selection…\n# Acceptance\nReturn confirmed findings only…" }, + { id: "PromptOwner", role: "Prompt Contract Reviewer", assignment: "# Target\npackages/coding-agent/src/prompts/**\n# Change\nCheck active-tool guidance…\n# Acceptance\nReturn mismatches and exact prompt lines…" }, ] ) {{else}} task( role: "Auth Storage Reviewer", - assignment: "# Target\npackages/ai/src/auth-storage.ts\n# Change\nReview the auth diff. Shared contract: read-only; return findings as severity/file/line/fix.\n# Acceptance\nReturn confirmed findings only..." + assignment: "# Target\npackages/ai/src/auth-storage.ts\n# Change\nReview the auth diff. Shared contract: read-only; return findings as severity/file/line/fix.\n# Acceptance\nReturn confirmed findings only…" ) task( role: "Prompt Contract Reviewer", - assignment: "# Target\npackages/coding-agent/src/prompts/**\n# Change\nCheck active-tool guidance. Shared contract: read-only; return mismatches and exact prompt lines.\n# Acceptance\nReturn confirmed findings only..." + assignment: "# Target\npackages/coding-agent/src/prompts/**\n# Change\nCheck active-tool guidance. Shared contract: read-only; return mismatches and exact prompt lines.\n# Acceptance\nReturn confirmed findings only…" ) {{/if}} diff --git a/packages/coding-agent/src/sdk.ts b/packages/coding-agent/src/sdk.ts index b8be1ae59..a23f8b29e 100644 --- a/packages/coding-agent/src/sdk.ts +++ b/packages/coding-agent/src/sdk.ts @@ -865,6 +865,7 @@ function createCustomToolContext(ctx: ExtensionContext): CustomToolContext { isIdle: ctx.isIdle, hasQueuedMessages: ctx.hasPendingMessages, abort: ctx.abort, + localProtocolOptions: ctx.localProtocolOptions, }; } @@ -2218,6 +2219,7 @@ export async function createAgentSession(options: CreateAgentSessionOptions = {} modelRegistry, () => (hasSession ? createSessionMemoryRuntimeContext(session, agentDir, cwd) : undefined), settings, + localProtocolOptions, ); credentialDisabledTarget = extensionRunner; @@ -2236,6 +2238,7 @@ export async function createAgentSession(options: CreateAgentSessionOptions = {} session.abort({ reason: USER_INTERRUPT_LABEL }); }, settings, + localProtocolOptions, autoApprove: options.autoApprove ?? false, }); const toolContextStore = new ToolContextStore(getSessionContext); diff --git a/packages/coding-agent/src/session/agent-session.ts b/packages/coding-agent/src/session/agent-session.ts index cb3b1bcf7..a58c67957 100644 --- a/packages/coding-agent/src/session/agent-session.ts +++ b/packages/coding-agent/src/session/agent-session.ts @@ -7242,6 +7242,8 @@ export class AgentSession { abort: () => { this.agent.abort(); }, + settings: this.settings, + localProtocolOptions: this.#localProtocolOptions(), }); for (const customTool of mcpTools) { diff --git a/packages/coding-agent/test/extensions-runner.test.ts b/packages/coding-agent/test/extensions-runner.test.ts index 0a2680515..ff7e5aaa2 100644 --- a/packages/coding-agent/test/extensions-runner.test.ts +++ b/packages/coding-agent/test/extensions-runner.test.ts @@ -72,6 +72,26 @@ describe("ExtensionRunner", () => { }; }; + it("exposes caller localProtocolOptions through extension context", async () => { + const localProtocolOptions = { + getArtifactsDir: () => tempDir.join("artifacts"), + getSessionId: () => "runner-session", + }; + const result = await loadTestExtensions(); + const runner = new ExtensionRunner( + result.extensions, + result.runtime, + tempDir.path(), + sessionManager, + modelRegistry, + undefined, + undefined, + localProtocolOptions, + ); + + expect(runner.createContext().localProtocolOptions).toBe(localProtocolOptions); + }); + describe("shortcut conflicts", () => { it("warns when extension shortcut conflicts with built-in", async () => { const extCode = ` diff --git a/packages/coding-agent/test/mcp-tool-args.test.ts b/packages/coding-agent/test/mcp-tool-args.test.ts index 42d49c299..158bb1431 100644 --- a/packages/coding-agent/test/mcp-tool-args.test.ts +++ b/packages/coding-agent/test/mcp-tool-args.test.ts @@ -1,7 +1,9 @@ import { describe, expect, it } from "bun:test"; +import * as path from "node:path"; import type { CustomToolContext } from "@oh-my-pi/pi-coding-agent/extensibility/custom-tools"; import { DeferredMCPTool, MCPTool, type MCPToolDefinition } from "@oh-my-pi/pi-coding-agent/mcp"; import type { MCPServerConnection } from "@oh-my-pi/pi-coding-agent/mcp/types"; +import { TempDir } from "@oh-my-pi/pi-utils"; import { INTENT_FIELD } from "@oh-my-pi/pi-wire"; import { createMockConnection, createMockTransport } from "./mcp-test-utils"; @@ -40,6 +42,35 @@ function createCapturedConnection(calls: CapturedRequest[]): MCPServerConnection return createMockConnection({ tools: {} }, transport); } +const imageToolDefinition: MCPToolDefinition = { + name: "read_image_with_model", + description: "Read an image from a local filesystem path", + inputSchema: { + type: "object", + properties: { + image_path: { type: "string" }, + }, + required: ["image_path"], + }, +}; + +async function createLocalImageContext( + tempDir: TempDir, +): Promise<{ context: CustomToolContext; expectedPath: string }> { + const artifactsDir = tempDir.join("artifacts"); + const expectedPath = path.join(artifactsDir, "local", "image-issue.png"); + await Bun.write(expectedPath, "png bytes"); + return { + context: { + localProtocolOptions: { + getArtifactsDir: () => artifactsDir, + getSessionId: () => "session-id", + }, + } as CustomToolContext, + expectedPath, + }; +} + describe("MCP tool arguments", () => { it("omits optional empty placeholders before tools/call", async () => { const calls: CapturedRequest[] = []; @@ -154,4 +185,20 @@ describe("MCP tool arguments", () => { expect(calls).toEqual([{ method: "tools/call", params: { name: "echo", arguments: { i: "hello" } } }]); }); + + it("resolves local image arguments before forwarding tools/call", async () => { + using tempDir = TempDir.createSync("@pi-mcp-local-image-"); + const calls: CapturedRequest[] = []; + const { context, expectedPath } = await createLocalImageContext(tempDir); + const tool = new MCPTool(createCapturedConnection(calls), imageToolDefinition); + + await tool.execute("call-1", { image_path: "local://image-issue.png" }, undefined, context, undefined); + + expect(calls).toEqual([ + { + method: "tools/call", + params: { name: "read_image_with_model", arguments: { image_path: expectedPath } }, + }, + ]); + }); });