Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 1 addition & 2 deletions e2e/mcp-late-dispatch.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -371,7 +371,7 @@ describe("integration — late MCP dispatch", () => {
);

test.serial(
"padded optional MCP arguments reach the server minus the values its schema rejects",
"padded optional arguments to a read MCP tool reach the server without rejected placeholders or null",
async () => {
const received: Record<string, unknown>[] = [];
await withSession(async (session) => {
Expand Down Expand Up @@ -428,7 +428,6 @@ describe("integration — late MCP dispatch", () => {
team: "",
fields: [],
priority: 0,
assignee: null,
release: "v0345-ef53e73667fa",
},
]);
Expand Down
72 changes: 72 additions & 0 deletions src/agent/tool-schema-normalize.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -163,6 +163,78 @@ describe("normalizeToolDefinitionsForProvider", () => {
}
});

describe("optional MCP properties", () => {
const listIssues: ToolDefinition = {
name: "mcp__linear__list_issues",
description: "[linear] list issues",
inputSchema: {
type: "object",
properties: {
release: { type: "string" },
priority: { type: "number" },
},
required: ["release"],
},
};
const builtin: ToolDefinition = {
...listIssues,
name: "list_issues",
};

test.each([
{ providerName: "codex/plus", model: "gpt-5.6-terra" },
{ providerName: "opencode-go", model: "gpt-5.1" },
{ providerName: "codex", model: "codex-mini-latest" },
])("$providerName $model advertises null on MCP tools only", (ctx) => {
const [mcp, own] = normalizeToolDefinitionsForProvider(
[listIssues, builtin],
ctx,
);
expect(mcp?.inputSchema).toEqual({
type: "object",
properties: {
release: { type: "string" },
priority: { type: ["number", "null"] },
},
required: ["release"],
});
expect(own).toBe(builtin);
});

test.each([
{ providerName: "anthropic", model: "claude-sonnet-5-5" },
{ providerName: "moonshot", model: "kimi-k2" },
])("$providerName leaves MCP tools as they are", (ctx) => {
const defs = [listIssues];
expect(normalizeToolDefinitionsForProvider(defs, ctx)).toBe(defs);
});

test("grok sanitizes MCP tools without advertising null", () => {
const [out] = normalizeToolDefinitionsForProvider([listIssues], {
providerName: "xai/default-2",
model: "grok-4.6",
});
expect(out?.inputSchema).toEqual(listIssues.inputSchema);
});

test("returns the same array when no MCP property changes", () => {
const nullable: ToolDefinition = {
...listIssues,
inputSchema: {
type: "object",
properties: { assignee: { type: ["string", "null"] } },
},
};
const defs = [nullable, builtin];
expect(
normalizeToolDefinitionsForProvider(defs, {
providerName: "openai",
model: "gpt-5.6",
}),
).toBe(defs);
});
});

test("grok sanitizes untyped properties and strips $schema", () => {
const untyped: ToolDefinition = {
name: "submit_result",
Expand Down
31 changes: 25 additions & 6 deletions src/agent/tool-schema-normalize.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
import type { ToolDefinition } from "@intx/types/runtime";
import { advertiseNullForOptionalProperties } from "../mcp/placeholder-args.js";
import { isMcpToolName } from "../mcp/tool-name.js";
import {
isGptProvider,
isKimiLeafProvider,
isMuseSparkLeafProvider,
isXaiGrokLeafProvider,
Expand Down Expand Up @@ -317,33 +320,49 @@ function sanitizeToolDefForGrok(def: ToolDefinition): ToolDefinition {
};
}

// GPT pads optional arguments with real values ("priority: 0") where it
// cannot write null; the MCP handler drops the null it writes instead.
function advertiseNullForMcpTool(def: ToolDefinition): ToolDefinition {
if (!isMcpToolName(def.name)) return def;
const inputSchema = advertiseNullForOptionalProperties(def.inputSchema);
return inputSchema === def.inputSchema ? def : { ...def, inputSchema };
}

function needsAcyclicPresentSchema(ctx: NormalizeToolDefsContext): boolean {
return isKimiLeafProvider(ctx) || isMuseSparkLeafProvider(ctx);
}

/**
* Family-gated wire rewrite of tool defs before they reach the
* director / provider. Moonshot/kimi and Muse Spark get a non-recursive
* `present` schema; Grok/xAI get schema sanitization; others pass through.
* Does not alter runtime validation or the canonical `presentDefinition`.
* `present` schema; Grok/xAI get schema sanitization; GPT gets `null` on
* optional MCP properties; others pass through. Does not alter runtime
* validation or the canonical `presentDefinition`. Returns `defs` itself
* when no def changes, so an unchanged toolset keeps its cache identity.
*
* Call at every advertise path that may include `present`; sub-agent
* toolsets omit it, so rewriting is a no-op unless one appears.
* Call wherever a tools array is built for the provider. Text that quotes
* a schema back to the model (tool_search cards) keeps the canonical
* definition.
*/
export function normalizeToolDefinitionsForProvider(
defs: readonly ToolDefinition[],
ctx: NormalizeToolDefsContext,
): ToolDefinition[] {
const acyclicPresent = needsAcyclicPresentSchema(ctx);
const grok = isXaiGrokLeafProvider(ctx);
if (!acyclicPresent && !grok) {
const gpt = isGptProvider(ctx);
if (!acyclicPresent && !grok && !gpt) {
return defs as ToolDefinition[];
}
return defs.map((def) => {
const out = defs.map((def) => {
let next = def;
if (acyclicPresent && def.name === "present")
next = rewritePresentAcyclic(next);
if (grok) next = sanitizeToolDefForGrok(next);
if (gpt) next = advertiseNullForMcpTool(next);
return next;
});
return out.every((def, i) => def === defs[i])
? (defs as ToolDefinition[])
: out;
}
Loading
Loading