fix(mcp): strip harness intent field at MCP tools/call boundary
OMP injects `INTENT_FIELD` (`i`) into every tool's wire schema. The direct model tool-call path strips it via `extractIntent` in agent-loop, but the eval `tool.*` bridge forwards args verbatim, so strict-schema MCP servers (Linear, anything with `additionalProperties:false` / Zod `.strict()`) rejected every call with `-32602 unrecognized_keys: ["i"]`. The eval bridge surfaced the rejection as `hasError: true` instead of throwing, so batch callers reading the value as success silently mutated nothing. Move the strip to the MCP boundary so it owns the contract regardless of caller: `MCPTool.execute` / `DeferredMCPTool.execute` route params through a new `prepareOutboundArgs` that runs `stripHarnessIntent` before `omitUnusedOptionalArgs`. `stripHarnessIntent` leaves `i` in place when the server's own `inputSchema.properties` declares it, so a server that legitimately uses `i` as a parameter is unaffected. Fixes #3575
This commit is contained in:
@@ -27,6 +27,7 @@
|
||||
- Fixed stale snapcompact archive state being reintroduced at the AgentSession manual and auto LLM compaction save paths; `preserveData.snapcompact` is now stripped after hook- and result-supplied preserve data are merged, so a prior snapcompact pass can no longer leak frames into a later context-full compaction. ([#3561](https://github.com/can1357/oh-my-pi/pull/3561) by [@serverinspector](https://github.com/serverinspector))
|
||||
- Fixed the snapcompact→context-full migration shipping the prior archive's plaintext to the summarization provider without secret redaction. When secrets are configured, the migrated archive's `text`/`textHead`/`textTail` regions are now obfuscated alongside the previous summary, while opaque provider-replay state (OpenAI remote-compaction `encrypted_content`) stays byte-identical. ([#3561](https://github.com/can1357/oh-my-pi/pull/3561))
|
||||
- Fixed fresh interactive launches ignoring `modelRoles.default` when the configured default lives on an extension-registered provider (e.g. an openai-compat plugin's `posthog/claude-opus-4-8`). `createAgentSession` resolved the default role before extension factories registered their providers, so the role-pointed model wasn't visible there; on a fresh launch (no `-c`/`--resume`) the post-extension fallback then went straight to `pickDefaultAvailableModel`, replacing the user's configured default with the first bundled provider default with auth (commonly `openai/gpt-5.5` when `OPENAI_API_KEY` was set). The fallback now retries the default-role lookup against the post-extension allowed-model set — including its explicit thinking selector — before falling back to a bundled provider default. ([#3569](https://github.com/can1357/oh-my-pi/issues/3569))
|
||||
- Fixed the `eval` `tool.*` bridge leaking the harness-internal `i` ("intent") field into MCP `tools/call` requests, so strict-schema servers (Linear, anything with `additionalProperties:false` / Zod `.strict()`) rejected every call with `-32602 unrecognized_keys: ["i"]` while the same call via the direct model tool-call path succeeded. `MCPTool.execute` / `DeferredMCPTool.execute` (`packages/coding-agent/src/mcp/tool-bridge.ts`) now strip `INTENT_FIELD` at the MCP boundary, so an MCP call behaves identically whether issued by the model directly or via the eval `tool.*` bridge; servers that legitimately declare `i` as a real parameter keep it untouched. ([#3575](https://github.com/can1357/oh-my-pi/issues/3575))
|
||||
|
||||
## [16.1.22] - 2026-06-26
|
||||
|
||||
|
||||
@@ -7,6 +7,7 @@ import type { AgentToolUpdateCallback } from "@oh-my-pi/pi-agent-core";
|
||||
import type { TSchema } from "@oh-my-pi/pi-ai";
|
||||
import { normalizeSchemaForMCP } from "@oh-my-pi/pi-ai/utils/schema";
|
||||
import { untilAborted } from "@oh-my-pi/pi-utils";
|
||||
import { INTENT_FIELD } from "@oh-my-pi/pi-wire";
|
||||
import type { SourceMeta } from "../capability/types";
|
||||
import type {
|
||||
CustomTool,
|
||||
@@ -83,6 +84,35 @@ function omitUnusedOptionalArgs(args: MCPToolArgs, inputSchema: MCPToolDefinitio
|
||||
return cleaned ?? args;
|
||||
}
|
||||
|
||||
/**
|
||||
* Drop the harness-internal intent field (`INTENT_FIELD`) before forwarding
|
||||
* args to an MCP server. The harness injects `i` into every tool's wire
|
||||
* schema; the direct model tool-call path strips it via `extractIntent`, but
|
||||
* the `eval` `tool.*` bridge and any other in-process caller forwards args
|
||||
* verbatim. Strict-schema servers (Linear, anything with
|
||||
* `additionalProperties:false` / Zod `.strict()`) reject every call that
|
||||
* carries `i`. The MCP boundary is the authoritative guard so callers don't
|
||||
* have to pre-strip.
|
||||
*
|
||||
* Leaves `i` in place when the server's own `inputSchema.properties` declares
|
||||
* it, so a server that legitimately uses `i` as a parameter is unaffected.
|
||||
*/
|
||||
function stripHarnessIntent(args: MCPToolArgs, inputSchema: MCPToolDefinition["inputSchema"]): MCPToolArgs {
|
||||
if (!Object.hasOwn(args, INTENT_FIELD)) return args;
|
||||
if (inputSchema.properties && Object.hasOwn(inputSchema.properties, INTENT_FIELD)) return args;
|
||||
const { [INTENT_FIELD]: _intent, ...rest } = args;
|
||||
return rest;
|
||||
}
|
||||
|
||||
/**
|
||||
* 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.
|
||||
*/
|
||||
function prepareOutboundArgs(params: unknown, inputSchema: MCPToolDefinition["inputSchema"]): MCPToolArgs {
|
||||
return omitUnusedOptionalArgs(stripHarnessIntent(normalizeToolArgs(params), inputSchema), inputSchema);
|
||||
}
|
||||
|
||||
/** Details included in MCP tool results for rendering */
|
||||
export interface MCPToolDetails {
|
||||
/** Server name */
|
||||
@@ -286,7 +316,7 @@ export class MCPTool implements CustomTool<TSchema, MCPToolDetails> {
|
||||
signal?: AbortSignal,
|
||||
): Promise<CustomToolResult<MCPToolDetails>> {
|
||||
throwIfAborted(signal);
|
||||
const args = omitUnusedOptionalArgs(normalizeToolArgs(params), this.tool.inputSchema);
|
||||
const args = prepareOutboundArgs(params, this.tool.inputSchema);
|
||||
const provider = this.connection._source?.provider;
|
||||
const providerName = this.connection._source?.providerName;
|
||||
|
||||
@@ -385,7 +415,7 @@ export class DeferredMCPTool implements CustomTool<TSchema, MCPToolDetails> {
|
||||
signal?: AbortSignal,
|
||||
): Promise<CustomToolResult<MCPToolDetails>> {
|
||||
throwIfAborted(signal);
|
||||
const args = omitUnusedOptionalArgs(normalizeToolArgs(params), this.tool.inputSchema);
|
||||
const args = prepareOutboundArgs(params, this.tool.inputSchema);
|
||||
const provider = this.#fallbackProvider;
|
||||
const providerName = this.#fallbackProviderName;
|
||||
|
||||
|
||||
@@ -2,6 +2,7 @@ import { describe, expect, it } from "bun:test";
|
||||
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 { INTENT_FIELD } from "@oh-my-pi/pi-wire";
|
||||
import { createMockConnection, createMockTransport } from "./mcp-test-utils";
|
||||
|
||||
type CapturedRequest = {
|
||||
@@ -86,4 +87,71 @@ describe("MCP tool arguments", () => {
|
||||
},
|
||||
]);
|
||||
});
|
||||
|
||||
it("strips the harness intent field before tools/call", async () => {
|
||||
// Regression: the harness injects `i` into every tool's wire schema and
|
||||
// the eval `tool.*` bridge forwards it verbatim. Strict-schema MCP
|
||||
// servers (e.g. Linear) reject every such call with
|
||||
// `unrecognized_keys: ["i"]`. The MCP boundary owns the contract; `i`
|
||||
// must never reach `tools/call`.
|
||||
const calls: CapturedRequest[] = [];
|
||||
const tool = new MCPTool(createCapturedConnection(calls), createSearchToolDefinition());
|
||||
|
||||
await tool.execute(
|
||||
"call-1",
|
||||
{ [INTENT_FIELD]: "looking up Foo", symbol: "Foo", language: "TypeScript", file: "" },
|
||||
undefined,
|
||||
unusedContext,
|
||||
undefined,
|
||||
);
|
||||
|
||||
expect(calls).toEqual([
|
||||
{
|
||||
method: "tools/call",
|
||||
params: { name: "search", arguments: { symbol: "Foo", language: "TypeScript" } },
|
||||
},
|
||||
]);
|
||||
});
|
||||
|
||||
it("strips the harness intent field for deferred MCP tools", async () => {
|
||||
const calls: CapturedRequest[] = [];
|
||||
const connection = createCapturedConnection(calls);
|
||||
const tool = new DeferredMCPTool("intellij-index", createSearchToolDefinition(), async () => connection);
|
||||
|
||||
await tool.execute(
|
||||
"call-1",
|
||||
{ [INTENT_FIELD]: "deferred lookup", symbol: "Bar", language: "TypeScript" },
|
||||
undefined,
|
||||
unusedContext,
|
||||
undefined,
|
||||
);
|
||||
|
||||
expect(calls).toEqual([
|
||||
{
|
||||
method: "tools/call",
|
||||
params: { name: "search", arguments: { symbol: "Bar", language: "TypeScript" } },
|
||||
},
|
||||
]);
|
||||
});
|
||||
|
||||
it("preserves `i` when the server's own schema declares it", async () => {
|
||||
// A server that legitimately exposes `i` as one of its parameters
|
||||
// must receive the caller-supplied value untouched. The boundary
|
||||
// guard checks the server's declared `properties` and steps aside.
|
||||
const calls: CapturedRequest[] = [];
|
||||
const definition: MCPToolDefinition = {
|
||||
name: "echo",
|
||||
description: "Echo a single token",
|
||||
inputSchema: {
|
||||
type: "object",
|
||||
properties: { i: { type: "string" } },
|
||||
required: ["i"],
|
||||
},
|
||||
};
|
||||
const tool = new MCPTool(createCapturedConnection(calls), definition);
|
||||
|
||||
await tool.execute("call-1", { i: "hello" }, undefined, unusedContext, undefined);
|
||||
|
||||
expect(calls).toEqual([{ method: "tools/call", params: { name: "echo", arguments: { i: "hello" } } }]);
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user