fix(mcp): strip empty placeholder arguments before forwarding

Author: bwachman

e2e/mcp-late-dispatch.test.ts

@@ -369,4 +369,61 @@ describe("integration — late MCP dispatch", () => {
       });
     },
   );
+
+  test.serial(
+    "padded optional MCP arguments reach the server as only the meaningful filters",
+    async () => {
+      const received: Record<string, unknown>[] = [];
+      await withSession(async (session) => {
+        session.toolset.dynamicRunner.addTools(
+          mcpClientTools(
+            linearClient({
+              inputSchema: {
+                type: "object",
+                properties: {
+                  customView: { type: "string", minLength: 1 },
+                  cursor: { type: "string" },
+                  query: { type: "string" },
+                  team: { type: "string" },
+                  fields: { type: "array", items: { type: "string" } },
+                  priority: { type: "number" },
+                  assignee: { type: ["string", "null"] },
+                  release: { type: "string" },
+                },
+              },
+              call: async (_toolName, args) => {
+                received.push(args);
+                return "ISSUE-1";
+              },
+            }),
+          ),
+        );
+        // The padding gpt-5.6-terra sent on a real release lookup.
+        replyScript(
+          session,
+          [
+            {
+              name: LATE_MCP,
+              args: {
+                customView: " ",
+                cursor: "",
+                query: "",
+                team: "",
+                fields: [],
+                priority: 0,
+                assignee: null,
+                release: "v0345-ef53e73667fa",
+              },
+            },
+          ],
+          "listed",
+        );
+
+        await runUntilDone(session, "list the issues in this release");
+      });
+      expect(received).toEqual([
+        { priority: 0, assignee: null, release: "v0345-ef53e73667fa" },
+      ]);
+    },
+  );
 });

src/mcp/placeholder-args.test.ts

@@ -0,0 +1,70 @@
+import { describe, expect, test } from "bun:test";
+import { stripPlaceholderArguments } from "./placeholder-args.js";
+
+const schema = {
+  type: "object",
+  properties: {
+    query: { type: "string" },
+    customView: { type: "string", minLength: 1 },
+    fields: { type: "array", items: { type: "string" } },
+    team: { type: "string" },
+    assignee: { type: ["string", "null"] },
+    parentId: { anyOf: [{ type: "string" }, { type: "null" }] },
+    cycle: { type: "string", nullable: true },
+    stage: { type: "string", enum: ["", "planned"] },
+    priority: { type: "number" },
+    includeArchived: { type: "boolean" },
+    id: { type: "string" },
+  },
+  required: ["id"],
+};
+
+describe("stripPlaceholderArguments", () => {
+  test("drops optional empty strings, whitespace, empty arrays, and disallowed null", () => {
+    expect(
+      stripPlaceholderArguments(
+        { id: "CL-1", query: "", customView: " ", fields: [], team: null },
+        schema,
+      ),
+    ).toEqual({ id: "CL-1" });
+  });
+
+  test("never changes required arguments", () => {
+    expect(stripPlaceholderArguments({ id: "" }, schema)).toEqual({ id: "" });
+    expect(stripPlaceholderArguments({ id: null }, schema)).toEqual({
+      id: null,
+    });
+  });
+
+  test("keeps null where the property schema allows it", () => {
+    const args = { assignee: null, parentId: null, cycle: null };
+    expect(stripPlaceholderArguments(args, schema)).toEqual(args);
+  });
+
+  test("keeps zero, false, and enum values", () => {
+    const args = { priority: 0, includeArchived: false, stage: "" };
+    expect(stripPlaceholderArguments(args, schema)).toEqual(args);
+  });
+
+  test("keeps meaningful values and leaves the caller's object intact", () => {
+    const args = { query: "release", fields: ["title"], team: "" };
+    expect(stripPlaceholderArguments(args, schema)).toEqual({
+      query: "release",
+      fields: ["title"],
+    });
+    expect(args).toEqual({ query: "release", fields: ["title"], team: "" });
+  });
+
+  test("treats a key the schema does not declare as optional", () => {
+    expect(
+      stripPlaceholderArguments({ id: "CL-1", extra: "" }, schema),
+    ).toEqual({ id: "CL-1" });
+  });
+
+  test("forwards arguments unchanged when the schema shape is unusable", () => {
+    const args = { query: "" };
+    expect(
+      stripPlaceholderArguments(args, { type: "object", required: "query" }),
+    ).toEqual(args);
+  });
+});

src/mcp/placeholder-args.ts

@@ -0,0 +1,58 @@
+import { type } from "arktype";
+
+const ObjectSchema = type({
+  "properties?": "Record<string, unknown>",
+  "required?": "string[]",
+});
+
+const PropertySchema = type({
+  "type?": "string | string[]",
+  "nullable?": "boolean",
+  "enum?": "unknown[]",
+  "const?": "unknown",
+  "anyOf?": "unknown[]",
+  "oneOf?": "unknown[]",
+});
+
+function allowsNull(property: unknown): boolean {
+  const parsed = PropertySchema(property);
+  if (parsed instanceof type.errors) return false;
+  if (parsed.nullable === true) return true;
+  if (parsed.type === "null") return true;
+  if (Array.isArray(parsed.type) && parsed.type.includes("null")) return true;
+  return [...(parsed.anyOf ?? []), ...(parsed.oneOf ?? [])].some(allowsNull);
+}
+
+// A value the schema names explicitly is a deliberate choice, even when empty.
+function isListedValue(property: unknown, value: unknown): boolean {
+  const parsed = PropertySchema(property);
+  if (parsed instanceof type.errors) return false;
+  return parsed.enum?.includes(value) === true || parsed.const === value;
+}
+
+function isPlaceholder(value: unknown, property: unknown): boolean {
+  if (isListedValue(property, value)) return false;
+  if (typeof value === "string") return value.trim() === "";
+  if (Array.isArray(value)) return value.length === 0;
+  return value === null && !allowsNull(property);
+}
+
+/**
+ * Drops optional arguments a model filled with an empty placeholder (`""`,
+ * whitespace, `[]`, or `null` where the property does not allow it).
+ * Required arguments and any other value are forwarded unchanged.
+ */
+export function stripPlaceholderArguments(
+  args: Record<string, unknown>,
+  inputSchema: Record<string, unknown>,
+): Record<string, unknown> {
+  const schema = ObjectSchema(inputSchema);
+  if (schema instanceof type.errors) return args;
+  const required = new Set(schema.required ?? []);
+  return Object.fromEntries(
+    Object.entries(args).filter(
+      ([key, value]) =>
+        required.has(key) || !isPlaceholder(value, schema.properties?.[key]),
+    ),
+  );
+}

src/mcp/plugin.ts

@@ -18,6 +18,7 @@ import {
   type MCPClient,
   type MCPContentBlock,
 } from "./client.js";
+import { stripPlaceholderArguments } from "./placeholder-args.js";
 import { mcpToolName } from "./tool-name.js";
 import { LOG_NAMESPACE_ROOT } from "../branding.js";
 
@@ -107,9 +108,11 @@ export function mcpClientTools(
         signal: AbortSignal,
       ): Promise<ToolResult> => {
         try {
+          // Some models pad every optional field; servers reject the empty
+          // values (customView minLength) or read them as filters.
           const envelope = await client.callResult(
             tool.name,
-            call.arguments,
+            stripPlaceholderArguments(call.arguments, tool.inputSchema),
             signal,
           );
           const authorizedBlocks = applyPolicyToBlocks(envelope.blocks);