Merge pull request #1578 from corbitsdev/cl-11352-drop-and-advertise-null-for-optional-mcp-arguments-to-stop

fix(mcp): keep padded optional arguments from filtering MCP reads

Author: TheGreatAxios

e2e/mcp-late-dispatch.test.ts

@@ -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) => {
@@ -428,7 +428,6 @@ describe("integration — late MCP dispatch", () => {
           team: "",
           fields: [],
           priority: 0,
-          assignee: null,
           release: "v0345-ef53e73667fa",
         },
       ]);

src/agent/tool-schema-normalize.test.ts

@@ -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",

src/agent/tool-schema-normalize.ts

@@ -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,
@@ -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;
 }