Repository: gosharplite/tell-me-go Issue: #1398 — MCP: zero-argument tools fail — typed-nil args map serializes as "arguments": null on the wire (invalid_type: expected record, received null) Date: 2026-08-18 Format: Griller (adversarial interviewer) vs. Subject (architect persona), 4 questions, early conclusion by the griller, verdict + post-grill position.
Participants: Griller (configs/griller.yaml) · Subject/Architect (configs/architect.yaml) · Orchestrator (butler — relay only, verbatim)
| Parameter | Value |
|---|---|
| Subject | Architect |
| Topic | Issue #1398 — diagnosis + plan |
| Opening | Subject presented understanding + plan before Q1 |
| Question cap | 10 (griller concluded after 4) |
| Closing | Griller verdict + subject post-grill position |
| Deliverable | This transcript + issue comment with gist link |
OPENING STATEMENT — SUBJECT (issue #1398)
Understanding — the root cause is verified, and it lives exactly where the issue says.
The defect chain is: (1) a zero-argument LLM tool call produces a typed-nil map[string]interface{} upstream — openai/client.go appendToolCall leaves var args map[string]interface{} untouched when argsStr == "" || "{}" (client.go:845-860), and anthropic/metrics.go parseToolUseArgs returns bare nil for null/empty tool_use input (~line 200); (2) the executor forwards it as-is (executor/runtime.go:30 → registry.Execute(ctx, tool.Name, call.Args, hb)); (3) the plugin proxy forwards it untouched (internal/tools/integrations/mcp/plugin.go:123 — dep.Client.CallTool(ctx, t.Name, args)); (4) both adapters stuff it into the SDK's any field: client.go:142 and stdio_client.go:191 construct &sdkmcp.CallToolParams{Name: name, Arguments: args}. I verified against go-sdk v1.7.0: CallToolParams.Arguments any json:"arguments,omitempty" (mcp/protocol.go:225-227), and the SDK's own guard if params.Arguments == nil { params.Arguments = map[string]any{} } (mcp/client.go:1278-1284) tests the interface, which is non-nil for a typed nil — so it never fires. My empirical probe (mirroring the struct tags) reproduces the issue's table exactly: typed-nil → {"name":...,"arguments":null}, nil-interface → key omitted, empty map → "arguments":{}. encoding/json's isEmptyValue on an interface returns v.IsNil() = false for typed-nil contents, so omitempty is defeated. This is a protocol violation — the MCP spec requires arguments to be an object or absent, never null — and it also violates the domain model's own typing of ToolCall.arguments as object (tell-me-go.modelith.md): the wire was emitting null for a field the model declares an object. The issue's blast-radius claims check out: both transports share the defect; strict servers (the official @modelcontextprotocol/server-* suite validating z.record(z.unknown())) reject it; the GitHub remote server is provably lenient (mcp_github_get_me works), masking the bug on the remote axis. It is correctly distinguished from #1377/#1378 — that is the provider-facing schema serialization path (schema.go, LLM adapters), this is the MCP-wire call path; the code is disjoint.
Plan — normalize at the adapter boundary, the only layer that knows the SDK.
I accept the issue's core proposal and refine it: add a package-private helper in internal/infrastructure/mcp, e.g. normalizeArguments(args map[string]interface{}) map[string]interface{} (nil → map[string]interface{}{}, else passthrough), and call it at both construction sites (client.go:142, stdio_client.go:191) — one shared definition so the two transports cannot drift, rather than the literal two-line duplication the issue sketches. This is architecturally mandatory per ADR-067 §2: the SDK is strictly confined to internal/infrastructure/mcp/, so wire correctness belongs to the adapter, not to plugin.go (which consumes the tools.MCPClient domain port and must stay protocol-ignorant) and not to the executor or LLM adapters. A plugin-layer fix would leave the identical defect open for any future port consumer and would leak SDK serialization semantics into the tools layer. The fix composes with, rather than replaces, the SDK's nil guard: after normalization the interface is non-nil, so the guard's {} substitution is moot for our calls, but it remains correct defense for any direct nil-interface construction (new(CallToolParams)). Blast radius is exactly two production call sites plus one helper, all inside internal/infrastructure/mcp/; zero changes to the domain port (internal/domain/tools/mcp_client.go), the plugin, the executor, or the LLM adapters. Strict servers now receive "arguments":{} — a valid empty record that z.record() accepts; lenient servers (GitHub) are behaviorally unchanged ({} is as acceptable as the null they tolerated). No ADR change and no domain-model edit are needed: this is a behavior fix that makes the wire conform to the model's existing arguments: object typing. I would add one doc-only note to the port's CallTool comment stating that nil args are normalized by implementations — so a future port consumer doesn't "helpfully" re-introduce the pattern.
Test strategy — wire-level regression on both transports, red pre-fix.
The critical test must pass a typed-nil map, not bare nil — bare nil already works today via the SDK guard, so a bare-nil test would be green pre-fix and prove nothing. For the HTTP transport (client_test.go): reuse newSDKTestServer with a zero-argument tool whose handler returns string(req.Params.Arguments) — the SDK server's CallToolRequest.Params.Arguments is json.RawMessage (protocol.go:247-250), i.e. the literal wire fragment — and assert it equals "{}" for a typed-nil map (red pre-fix: "null"), plus table rows for bare nil (already green; pins SDK-guard composition) and a non-empty map (passthrough). For the stdio transport: add a zero-arg args-capture tool to testdata/stdioserver/main.go returning the raw arguments bytes, and assert "{}" from stdio_client_integration_test.go via the existing TestMain go-build fixture harness — same uniform assertion shape. Note the existing echo fixture tool cannot serve as the probe: it cannot distinguish null from {} (json.Unmarshal(null, &args) yields a nil map with no error → "missing text" either way). Optionally add a 3-case table test of normalizeArguments itself, and a plugin-layer pin that the proxied handler forwards a typed-nil args map unchanged (documenting that normalization is the adapter's job — guarding against a future wrong-layer "fix"). Keep plugin_test.go/proxy_test.go args-forwarding assertions green. Verification: go test ./internal/infrastructure/mcp/... ./internal/tools/integrations/mcp/..., then make check-full — no verify-mcp-sdk-confinement impact since all new code stays inside the confined package.
Concerns and where the issue's proposal is incomplete.
Three refinements. (1) The issue's wire assertion is loose — "no arguments key (or arguments:{})". Post-fix the shape is deterministically "arguments":{}, never absent: we hand the SDK a non-nil empty map, omitempty cannot omit a non-nil interface payload, and even the SDK guard's own nil-interface path produces {}. The test should pin {} exactly, not accept "absent", which is unreachable through this path. (2) The shared-helper refinement over the duplicated two-line guard, to prevent transport drift. (3) The issue's "unit-level pin not needed" is mostly right, but a tiny normalizeArguments table test is near-free and pins the boundary contract; I'd include it. Open questions I want pressure-tested: whether the fix comment should also document the upstream typed-nil origin (openai/anthropic decode paths) so future readers understand why the adapter must be robust; and whether the domain port doc note is acceptable as a doc-only change or whether the maintainers prefer the port contract to stay silent. One risk the issue underweights: the "arguments":{} present-but-empty shape is spec-valid and matches what the official TypeScript SDK sends for zero-arg calls, so no conformant server should distinguish it from absent — but that equivalence is an assumption about third-party servers we cannot fully verify; the live mcp_github_get_me counter-evidence covers the lenient axis only. I also confirm the issue's "no config-level workaround" — the fix is the only path, and it is small and contained.
Q1: Your "bare nil already works today via the SDK guard" claim is false for the adapter path — through the port, "bare nil" is a typed-nil map, and a bare-nil test would be red pre-fix, not green.
You say: "The critical test must pass a typed-nil map, not bare nil — bare nil already works today via the SDK guard, so a bare-nil test would be green pre-fix and prove nothing."
Facts I verified:
- The domain port signature is
CallTool(ctx context.Context, name string, args map[string]interface{})(internal/domain/tools/mcp_client.go). Passingnilto that parameter produces a nil map — Go assigns the untyped nil to the map type, so the parameter holds(map[string]interface{}, nil), not a nil interface. - Both adapters then do
Arguments: argsinto theanyfield. My runtime probe (mirroringCallToolParams' tags) confirms: a nil map stored in ananyyieldsiface == nil → false, andjson.Marshalemits"arguments":null. The SDK guard atmcp/client.go:1281-1284tests the interface — it does not fire. - Therefore
c.CallTool(ctx, name, nil)— the exact call shape already present inclient_test.go:448,495,520,537,552,569,630andstdio_client_integration_test.go:150,284,331— serializes"arguments":nullon the wire today. A test asserting"{}"against any of those calls is red pre-fix, indistinguishable from the "typed-nil" row you insist on. The distinction you draw between "typed-nil" and "bare nil" does not exist at the port boundary: every value that can enter throughargs map[string]interface{}is a map, nil or non-nil, never a nil interface. - Your own probe output contradicts your claim: you printed
typed-nil in any == nil : false— which is exactly what a "bare nil" passed through the port produces.
The consequence for your test plan: the "bare-nil table row (already green; pins SDK-guard composition)" cannot do what you describe. The SDK guard's nil-interface path is unreachable through the domain port — it can only be exercised by constructing &sdkmcp.CallToolParams{} with a zero-valued Arguments and calling session.CallTool directly (white-box, inside package mcp, bypassing the adapter). As specified, your 3-row table (typed-nil / bare-nil / non-empty) collapses to 2 distinct rows — nil-map (red pre-fix, proving the bug) and non-empty passthrough — plus a separate white-box test if you genuinely want to pin the guard.
Please reconcile: do you retract the "bare-nil is green pre-fix" claim, and how do you propose to actually pin the SDK-guard composition — given that the existing nil-args tests at client_test.go:448 etc. are already emitting "arguments":null today (passing only because the Go SDK fixture server is lenient), and your fix will silently change their wire shape to "arguments":{} with no assertion noticing?
A1 — Conceded, and the correction is precise: the griller is right, my "bare-nil is green pre-fix" claim is false, and the distinction I drew does not exist at the port boundary.
Retraction, with the corrected mechanics. Through the port signature CallTool(ctx, name string, args map[string]interface{}) (internal/domain/tools/mcp_client.go), an untyped nil is converted to a nil map of type map[string]interface{} at the parameter — Go never lets a nil interface through, because the parameter is typed. The adapter then does Arguments: args (client.go:142, stdio_client.go:191), which stores that nil map inside the any field: the interface is non-nil, so the SDK guard (mcp/client.go:1281-1284, params.Arguments == nil) does not fire, and omitempty's isEmptyValue on a non-nil interface returns v.IsNil() == false, emitting "arguments":null. My own probe output proves the griller's point, not mine: the nil-iface row in my probe (key omitted, guard fires) is unreachable through the port — I presented it as if it were a port-level path when it is only what the SDK guard expects from a direct &sdkmcp.CallToolParams{} construction. At the port, "bare nil" and "typed-nil map" are the same value; my 3-row table was really 2 rows all along.
Verified against the code, the griller's cited call sites are all nil-map-on-wire today. I confirm the line citations from my own read of the files: client_test.go:448 (CallTool(..., "mixed", nil)), :495 ("fails", nil), :520 ("missing", nil), :537 ("tool", nil), :552 and :569 (cancellation/deadline), :630 ("ping", nil in the goleak test), and stdio_client_integration_test.go:150 ("stderr", nil), :284 ("die", nil), :331 ("block", nil). Every one of these passes nil through the port → nil map → "arguments":null on the wire. They pass today only because both fixture servers are lenient: the Go SDK server decodes CallToolRequest.Params.Arguments (json.RawMessage, protocol.go:247-250) as the literal bytes null and the handlers either ignore it or json.Unmarshal(null, &args) into a nil map without error — and several of those calls are error-path tests (unknown tool, HTTP 500, cancelled ctx, closed client, die/block) where the request never reaches a server at all. So the griller's inference is exact: a "{}" assertion against any of them is red pre-fix, and my "already-green bare-nil row pinning SDK-guard composition" was doubly wrong — it is not green, and the guard is not what it would exercise.
Revised test table — two distinct port-level rows, and the regression test is simpler than I framed it. (1) nil-map: literally c.CallTool(ctx, "args-capture", nil) — the exact shape of the existing call sites — with the fixture tool returning string(req.Params.Arguments); red pre-fix ("null"), green post-fix ("{}"). This single row subsumes my "typed-nil" and "bare-nil" rows; no typed-nil variable construction is needed. (2) non-empty passthrough: map[string]interface{}{"text":"x"} → "text" preserved on the wire, green both ways. That's the whole port-level matrix: the port admits only maps, nil or non-nil — the griller's "never a nil interface" is airtight, and I verified it additionally by noting the port is un-interface-able by construction (a caller holding any cannot pass it to a map[string]interface{} parameter without a type assertion).
On the SDK-guard composition — revised position: do not pin it, and say so in a comment. The griller is right that the guard's nil-interface path is unreachable through the domain port and can only be exercised white-box (constructing &sdkmcp.CallToolParams{} with a zero-valued Arguments and calling session.CallTool directly, inside package mcp, bypassing both adapters). My recommendation now is to not write that white-box test: it would pin third-party SDK behavior that no code path in this repository can ever reach through our adapters (post-fix, the adapter always assigns a typed map), so it has zero regression value and would break on a go-sdk upgrade for reasons unrelated to our code. The honest composition guarantee is a comment on normalizeArguments citing the SDK guard's intent (mcp/client.go:1281-1284, "Avoid sending nil over the wire") and noting that the adapter's job is to make that guarantee true for every value the port admits — because the SDK itself cannot. What we pin is our contract: for every input the port can carry, the wire never emits "arguments":null.
On the silent wire-shape change to the existing nil-args tests: the griller's concern is well-founded and I accept it as a real observation, but I judge it acceptable and not a defect of the fix. Those tests assert response semantics (echo text, error classification, closed-state guards, deadline/EOF behavior), not request shape; the lenient fixture servers accept both null and {}, so they stay green and remain valid. Silently changing their wire shape is exactly what a wire-correctness fix does — the alternative (no fix) is the bug. The blind spot they exposed — that response-path tests never exercised a strict server, which is precisely why the defect shipped — is closed by the new dedicated wire tests, which are the correct home for request-shape assertions; entangling response tests with request-shape pins would duplicate the new coverage for no gain. I will note the shape change explicitly in the PR description so reviewers understand why the existing nil-args tests are intentionally untouched.
Net position after Q1: the fix location (adapter-level normalizeArguments at both CallTool construction sites, shared helper, ADR-067 §2 confinement rationale) is unchanged and stands. The test plan is corrected: one nil-map regression row (red pre-fix, CallTool(ctx, name, nil) — the shape of the existing call sites), one non-empty passthrough row, per transport; no SDK-guard white-box test; composition documented by comment, not asserted.
Q2: Your claim that the wire "arguments":null "violates the domain model's own typing of ToolCall.arguments as object" is a category error — "arguments" is one name for two different concepts, and you built a normative conclusion on the conflation.
You stated in your opening: "it also violates the domain model's own typing of ToolCall.arguments as object (tell-me-go.modelith.md): the wire was emitting null for a field the model declares an object" — and used this to conclude "no domain-model edit is needed: this is a behavior fix that makes the wire conform to the model's existing arguments: object typing."
Facts I verified:
- The domain model's
ToolCall.arguments(object) is a domain-entity attribute — the in-process representation carried on theToolCallentity, realized in Go asllm.FunctionCall.Args map[string]interface{}(internal/domain/llm/types.go). A nil map is still a map — the model'sobjecttyping says nothing about nil-vs-empty and nothing about wire serialization. - The MCP wire's
argumentsis a protocol field — its shape ("an object, or absent, never null") is defined by the MCP specification and realized insdkmcp.CallToolParams.Arguments anyjson:"arguments,omitempty"(go-sdk protocol.go:224-225). Its validity is governed by the MCP spec, not by tell-me-go's domain model. - The domain model models no MCP wire surface at all — the
MCPServerentity (and the "MCP tool discovery and execution" scenario) covers configuration and delegation, never the JSON-RPCtools/callpayload shape. Nothing intell-me-go.modelith.mddeclares, forbids, or could be violated by anullon the MCP wire.
So the normative basis of your fix is the MCP spec plus the go-sdk's own documented intent ("Avoid sending nil over the wire", mcp/client.go:1282) — not the domain model. Your "conforms to the model's existing typing" framing is backwards: the model never governed the wire, so there is nothing to conform to, and "no domain-model edit needed" is trivially true because the model is irrelevant to this fix — not because the fix "makes the wire conform" to it.
Please retract or precisely ground the "domain model violation" claim: do you agree the correct statement is "the MCP wire emitted null for a field the MCP protocol requires to be an object or absent, and the go-sdk's own guard documents the intent that this must not happen" — with the domain model playing no normative role — or can you point to a specific statement in tell-me-go.modelith.md that actually governs the MCP wire? And, since this framing appeared in your opening as part of your "verified diagnosis" and your "no domain-model edit" justification, does correcting it change any conclusion of your plan (I believe it changes none — the adapter fix and the test matrix stand as revised after Q1)?
A2 — Conceded: the "domain model violation" claim was a category error, and I retract it. The correct normative basis is the MCP protocol plus the go-sdk's documented intent — the domain model plays no normative role. The griller's facts are verified, with one precision worth adding about the Go realization.
On the facts — verified, with one refinement. (1) The domain model's ToolCall.arguments (object) is an in-process entity attribute. Its canonical Go realization is tools.ToolCall.Arguments map[string]interface{} (internal/domain/tools/types.go:74-78 — the struct literally named ToolCall with an Arguments field, matching the model's entity); llm.FunctionCall.Args map[string]interface{} (internal/domain/llm/types.go:46) is the upstream LLM-decode carrier. Both are maps — and a nil map is still a map, so the model's object typing says nothing about nil-vs-empty and nothing about wire serialization. (2) The MCP wire arguments is a protocol field whose validity is governed by the MCP specification, realized in sdkmcp.CallToolParams.Arguments any (protocol.go:225-227) — a different concept sharing a name with the domain attribute. (3) I re-scanned tell-me-go.modelith.md: the MCPServer entity and the "MCP tool discovery and execution" scenario cover configuration, discovery, normalization, registration, delegation, and the three-way error split — and the MCP invariants (mcp-server-key-format, mcp-token-not-logged, mcp-readonly-defaults, mcp-stdio-trusted-defaults) govern keys, credential hygiene, and consent/serial defaults, never the tools/call payload shape. There is no statement in the modelith doc that governs the MCP wire, so I cannot point to one — the griller is right that none exists.
Retraction, precisely scoped. My opening statement asserted the wire null "violates the domain model's own typing of ToolCall.arguments as object" and presented it inside my "verified diagnosis." That was wrong on two independent grounds: (a) it conflated the domain attribute with the protocol field — a category error; and (b) even at the domain level nothing is violated, because a nil map is still an object-typed value. The verified defect chain never needed the domain model: typed-nil map at the port → non-nil interface defeats the SDK guard (if params.Arguments == nil, client.go:1280) → isEmptyValue on the interface returns false, defeating omitempty → "arguments":null on the wire → strict servers reject it per the MCP spec. The correct normative statement is exactly the griller's formulation: the MCP wire emitted null for a field the MCP protocol requires to be an object or absent, and the go-sdk's own guard documents the intent that this must not happen ("Avoid sending nil over the wire", client.go:1282). I confirm the SDK guard comment verbatim from the module cache.
Does the correction change any conclusion? No — the griller's closing judgment is correct, and I agree. The fix location stands: normalizeArguments at both adapter CallTool construction sites (client.go:142, stdio_client.go:191), shared helper, within the ADR-067 §2-confined package — the wire is the adapter's concern precisely because no other layer owns it. The "no domain-model edit" conclusion stands but for the corrected reason: it is trivially true because the model is irrelevant to this fix — not because the fix "conforms the wire to the model." I explicitly retract that backwards framing; I will not recommend any domain-model change, and I will not cite the model in the fix justification. The test matrix as revised after Q1 stands unchanged: one nil-map regression row (CallTool(ctx, name, nil) → server-observed "{}", red pre-fix), one non-empty passthrough row, per transport; no SDK-guard white-box pin; composition documented by comment citing the SDK guard's intent. The one refinement I would add to the record: the corrected justification also clarifies the PR description language — the fix is protocol-compliance, not domain-model conformance, which is the honest and sufficient basis.
Q3: Your "make check-full green" claim is contingent on where the new wire tests land — the files you propose to touch carry live, line-anchored catalog pins and the repo's documented coordination rule, and your plan is silent on placement.
Your standing test plan: per transport, one nil-map row asserting the server-observed "{}" (red pre-fix), one non-empty passthrough row; a new args-capture fixture tool in testdata/stdioserver/main.go; verification via go test ... && make check-full.
Facts I verified:
verify-nonfix-catalog(part ofmake check/check-full) runsTestVerifyNonFixCatalog | TestVerifyCoveragePinsMatchLiveCatalog | TestDetailedCoverageReportagainst the liveINTENTIONAL_NON_FIXES.mdcatalog, which pins four tests by exact line number in exactly the files your tests must touch:stdio_client_integration_test.go:112—TestStdio_RoundTrip(CC=11)stdio_client_integration_test.go:216—TestStdio_LauncherTreePassThrough(CC=13)stdio_client_integration_test.go:278—TestStdio_ChildDeathMidSession(CC=11)client_test.go:645—TestBasicAuthTransport(CC=11)
- The catalog's own Drift Policy states these references are verified at record time and "can drift as code above them is edited," and the Coordination rule states: "any legitimate catalog change (add/remove/re-anchor/CC re-verify) edits the partition test in the same PR." The matching is by interval overlap — a shifted range silently re-catalogs whatever occupies the new interval.
- The fixture tool addition is placement-safe (
testdata/stdioserver/main.gois not line-pinned), and a new test function appended after the last test in each file (TestStdio_TwoClientsIndependentat :367;TestConnect_BasicAuthHeaderat :734) shifts nothing. But your plan as stated does not specify this: if the args-capture assertion is instead woven intoTestStdio_RoundTrip(raising its CC above the pinned 11 and shifting its :112 anchor), or inserted anywhere before the :216/:278/:645 anchors, every subsequent pin drifts andverify-nonfix-catalogcan go red — failingmake check-fulland contradicting your headline claim, unless the coordination rule (catalog + partition-test edit in the same PR) is followed.
Also unsettled from your A1 revision: your opening proposed "a plugin-layer pin that the proxied handler forwards a typed-nil args map unchanged" — A1's revised matrix ("one nil-map regression row + one non-empty passthrough row, per transport; no SDK-guard white-box test") drops the SDK-guard pin but is silent on whether the plugin-layer pin survives. If it does, it lives in plugin_test.go (unpinned, safe), but it pins behavior that is identical pre- and post-fix — documentation value only, and it asserts the plugin forwards nil to the port, which your fix deliberately leaves untouched.
Please specify, precisely: (1) the exact placement of the two new wire tests — new functions appended at file end, or woven into existing pinned tests, and if the latter, how you reconcile the resulting pin drift with the coordination rule and your "make check-full green" claim; (2) whether the plugin-layer pass-through pin from your opening is still in the plan, and if so, what regression value you claim for a test that is green before and after the fix; and (3) the expected CC of the new stdio test — if it lands at CC ≥ 11, it triggers the complexity-threshold-policy and requires a catalog entry of its own, which is itself a catalog change subject to the coordination rule.
A3 — All three points answered precisely; the griller's catalog facts verify exactly, and the resolution is to make the catalog interaction vacuous by construction.
Verified facts first. The griller's four pins are exactly the catalog's MCP test-complexity entries, and I confirmed by grepping the live catalog: client_test.go:645 (TestBasicAuthTransport CC=11), stdio_client_integration_test.go:112/216/278 (TestStdio_RoundTrip CC=11, TestStdio_LauncherTreePassThrough CC=13, TestStdio_ChildDeathMidSession CC=11) — plus one pin the griller didn't need but I verified is out of scope: internal/tools/integrations/mcp/schema_test.go:241 (the #1378 TestConvertSchema pin, a file this PR does not touch). Critically, no catalog pin exists on the production files the fix modifies (client.go, stdio_client.go), nor on testdata/stdioserver/main.go, stdio_client_test.go, plugin_test.go, or proxy_test.go. And the pinned anchors are the test functions' start lines (confirmed: TestStdio_LauncherTreePassThrough begins at :216, TestStdio_ChildDeathMidSession at :278, exactly as pinned), so any insertion above them drifts the anchors and any CC growth above the pinned value invalidates the entry — the griller's drift analysis is correct.
(1) Placement — the wire tests go in a new dedicated file, not in any pinned file. I propose internal/infrastructure/mcp/wire_arguments_test.go (same package, so it can use the existing newSDKTestServer and newTestClient harnesses) holding both transport wire tests plus the normalizeArguments unit table. This touches zero catalog-referenced files and zero pinned lines, which makes the drift-policy obligation and the coordination rule vacuous: no re-anchor, no partition-test edit, no catalog change — and "make check-full green" stops being contingent on catalog coordination at all. The alternative I considered — appending new functions after the last test in each file (TestStdio_TwoClientsIndependent at :367, TestConnect_BasicAuthHeader at :734) — is line-safe (end-append shifts no earlier anchor), but it still touches catalog-referenced files, which triggers the catalog's own "re-verify on every PR that touches a file referenced by the catalog" obligation. The new-file approach is strictly cleaner and I recommend it over even the safe append. Woven-into-existing-pinned-tests is rejected outright: extending TestStdio_RoundTrip would raise its CC above the pinned 11 and shift its :112 anchor, forcing a catalog re-anchor plus a partition-test edit in the same PR per the coordination rule — precisely the churn this behavior-fix PR must not carry. The fixture tool addition is confirmed placement-safe (no pin on testdata/stdioserver/main.go); I'll add the args-capture tool alongside the existing AddTool calls in newFixtureServer, which shifts nothing anywhere.
(2) The plugin-layer pass-through pin is dropped — it has zero regression value. The griller's characterization is exact: a test asserting the proxied handler forwards nil to the port is green before and after the fix (the fix is confined to the adapter; plugin.go:123 is deliberately untouched), so it cannot catch a regression of this defect — a test that cannot fail for the change it guards is documentation, not verification. The forwarding contract is already pinned where it has value: proxy_test.go TestProxyHandler_ArgumentsAndContextForwarding asserts non-empty args forwarding, and nil-args forwarding is already exercised incidentally (TestProxyHandler_ArgumentsAndContextForwarding's cancellation subtest and TestProxyRegister_TruncatedToolName both invoke the handler with nil). The layering rationale — the plugin must stay protocol-ignorant, the SDK-confined adapter owns wire normalization — belongs in a comment at normalizeArguments (and optionally at plugin.go:123), not in a test.
(3) Expected CC of the new stdio wire test: ≤ 3 by design — no complexity-threshold-policy interaction. The minimal shape — one CallTool call, one error guard, one string-equality assertion — is CC ≈ 2; even if I fold the non-empty passthrough row into the same function as a two-row table, it stays ≈ 5–6. Both are far below the policy boundary (CC ≥ 11 triggers complexity-threshold-policy and would require a catalog entry of its own, itself a coordination-rule change). The normalizeArguments table (3 rows) is similarly ≈ 5–6. So the new tests create no catalog obligation from CC either. I will, however, note one discipline point the griller's question surfaces: the repo treats test CC as a first-class gate input (verify-nonfix-catalog enforces over-threshold pins), so the tests should be written to the minimal shape deliberately, not incidentally — and since verify-nonfix-catalog's enforcement boundary is "over-threshold functions," sub-10 new tests are not cataloged and need no entry.
Net position after Q3: the plan is unchanged in substance (adapter-level normalizeArguments at client.go:142 and stdio_client.go:191; one nil-map regression row + one non-empty passthrough row per transport; no SDK-guard white-box pin; plugin pin dropped) with the placement made catalog-inert: new wire_arguments_test.go in internal/infrastructure/mcp/, fixture tool added to the unpinned testdata/stdioserver/main.go, CC targets ≤ 6. "make check-full green" then holds unconditionally: the PR's only file touches are two unpinned production files, one unpinned fixture file, and one new test file — zero catalog pins, zero drift, zero coordination-rule triggers.
Q4: One claim in your "verified diagnosis" remains asserted-but-not-verified, and your plan's severity framing — though not its correctness — rests on it. Separate the two, and confirm the fix's justification is complete without the server-side evidence.
Your opening states, as part of the verified chain: "strict servers (the official @modelcontextprotocol/server-* suite validating z.record(z.unknown())) reject it" and "the GitHub remote server is provably lenient (mcp_github_get_me works), masking the bug on the remote axis." Both are cited from the issue's reproduction — the verbatim invalid_type: expected record, received null error and the live get_me call — not from anything you verified in this environment. That is not a flaw in your diagnosis: the server-side behavior is not verifiable here (no node/npx, no live GitHub session), and the issue's evidence is concrete and specific.
The question is about what your plan claims to depend on. After Q2, you correctly restated the fix as protocol compliance: the wire emitted null for a field the MCP spec requires to be an object or absent, and the go-sdk's own guard documents the intent that this must not happen. That basis is fully verified by what you and I have established: the probe table (typed-nil map → "arguments":null), the SDK guard's interface-nil semantics, the two construction sites, and the spec's object-or-absent requirement.
Please confirm the final integrated position precisely: (1) the fix's correctness is established entirely by the protocol violation at the wire — demonstrated by the probe and the go-sdk's documented intent — and is independent of whether any real server rejects null; (2) the strict-server rejection and the GitHub leniency are the exposure axis (severity/priority evidence, from the issue), not preconditions for the fix, and your PR should frame them as observed symptoms rather than verified-by-you facts; and (3) the "no config-level workaround" claim is likewise issue-sourced — your plan's completeness argument is that the wire is formed only at the two now-fixed sites, which is the workaround-free guarantee, not any property of the servers. If you agree, I consider the diagnosis and plan settled: adapter-side normalizeArguments at exactly two repo-wide construction sites, catalog-inert wire_arguments_test.go with the two-row per-transport matrix, protocol-compliance framing, and make check-full green as the verification gate.
A4 — Conceded in substance, with one environment fact corrected; the integrated position is confirmed complete without any server-side evidence.
(1) Fix correctness is wire-level and server-independent — agreed, and I can now state it as fully verified. The client-side chain is established entirely from this environment: (a) my probe (typed-nil map in the omitempty any field → "arguments":null); (b) the SDK guard's interface-nil semantics and its documented intent, "Avoid sending nil over the wire" (go-sdk/mcp/client.go:1282); (c) the spec's object-or-absent requirement, corroborated by the SDK authors encoding exactly that rule in the guard; and (d) — the claim the griller rightly pushed me to ground — "exactly two repo-wide construction sites" is now a verified structural fact, not an assumption. I just grepped the whole repository including test files: CallToolParams{ appears at exactly two sites, internal/infrastructure/mcp/client.go:142 and stdio_client.go:191, and session.CallTool is invoked nowhere else in production; plugin.go:123 calls the domain port dep.Client.CallTool, not the SDK session. So the wire arguments value has exactly two formation points in the entire codebase, both normalized by the fix. The correctness argument is complete: a wire that cannot emit arguments:null cannot be rejected for invalid_type: expected record, received null, independent of what any real server does.
(2) Strict-server rejection and GitHub leniency are the exposure axis — agreed, with one factual correction to the griller's environment claim. The griller is right that both are issue-sourced reproduction evidence: the verbatim invalid_type error and the mcp_github_get_me success come from the issue body, which I quoted but did not independently reproduce. They are severity/priority evidence (why this matters; why it shipped masked), not preconditions — even if every server tolerated null, the wire would still violate the spec and the SDK's documented intent, and the fix would still be correct. The PR must frame them as observed symptoms reported in #1398, not as facts verified in this environment. However, the griller's "no node/npx" fact is wrong here: node and npx exist at /home/pos/node-v24.4.1-linux-x64/bin/ and uvx at ~/.local/bin/uvx. The conclusion survives the correction — I did not run a reproduction this session, so the evidence remains issue-sourced regardless of tool availability — but I record the environment accurately. I will additionally offer, as an optional bounded verification step during implementation (not a plan requirement): spawn npx -y @modelcontextprotocol/server-filesystem <dir>, drive the repo's StdioClient at a zero-arg tool with a timeout guard, and capture the pre-fix invalid_type — upgrading the strict axis from issue-sourced to verified-in-environment if it succeeds, with no effect on the fix either way.
(3) "No config-level workaround" — agreed, and the completeness argument is structural, replacing the issue-sourced claim. MCPServerConfig governs endpoint, credentials, timeout, consent, and serial — none of which touch the arguments payload. The arguments value flows LLM decode (openai appendToolCall, anthropic parseToolUseArgs) → executor (runtime.go:30) → plugin proxy (plugin.go:123) → the two adapters, passing the map through untouched at every step; the wire is formed only by the SDK's serialization of CallToolParams, constructed at the two verified sites. Post-fix, no configuration can inject null into wire arguments — the workaround-free guarantee is that the wire has exactly two formation points, both normalized, not any property of the servers. I will cite the structural claim, not the issue's "none exists" statement, as the PR's completeness argument.
Settlement accepted in full. The integrated position: adapter-side normalizeArguments at exactly the two verified construction sites (client.go:142, stdio_client.go:191); catalog-inert new wire_arguments_test.go in internal/infrastructure/mcp/ carrying the two-row per-transport matrix (nil-map → server-observed {}, red pre-fix; non-empty passthrough) plus the normalizeArguments unit table, CC ≤ 6; args-capture fixture tool in the unpinned testdata/stdioserver/main.go; no SDK-guard white-box pin; plugin-layer pin dropped; protocol-compliance framing (MCP spec + SDK intent) with the server behavior cited as issue-sourced exposure evidence; make check-full green as the verification gate, unconditional since no catalog-pinned file or line is touched. The diagnosis and plan are settled; I have no open questions beyond the optional live reproduction, which I recommend as a PR-time verification, not a gate.
VERDICT: The subject's diagnosis held up at its core — the typed-nil-at-the-port chain, the defeated SDK guard, the omitempty semantics, and the exactly-two-construction-sites structural claim were all verified against the code, the go-sdk v1.7.0 source, and a live probe, and the plan is sound: adapter-side normalizeArguments at the two repo-wide CallToolParams sites, a catalog-inert wire_arguments_test.go, and make check-full as the gate. It did not hold in two places, both corrected cleanly: the test-strategy claim that "bare nil is green pre-fix and pins the SDK guard" (false — through the port, bare nil is a typed-nil map, red pre-fix, and the guard's nil-interface path is unreachable through the port), and the "violates the domain model's typing" framing (a category error — the correct normative basis is the MCP spec plus the go-sdk's documented intent). Recommendation: proceed as planned with the corrected test matrix and protocol-compliance framing.
Final remarks: this was a clean round — the subject retracted precisely when shown wrong, never defended a settled point, and even corrected my environment claim (node is present at /home/pos/node-v24.4.1-linux-x64/bin/, so my "no node/npx" was wrong; the conclusion survives since the evidence remains issue-sourced this session). The corrections table: "bare-nil is green" → retracted (nil-map row is the regression test, red pre-fix); "domain model violation" → retracted (protocol compliance); "newTestClient for HTTP wire test" → corrected (stdio-only; HTTP test uses newSDKTestServer + inline NewClient); catalog placement → resolved via a new unpinned test file. Given node is available, the subject's optional live reproduction against @modelcontextprotocol/server-filesystem is genuinely worth doing at PR time — it upgrades the strict-server axis from issue-sourced to verified-in-environment at low cost — but it remains a nice-to-have, not a gate, and the fix's correctness does not depend on it.
POST-GRILL POSITION:
What I stand by. The root cause is verified end-to-end: a typed-nil map enters the port from the LLM decode path (openai appendToolCall, anthropic parseToolUseArgs), passes untouched through the executor (runtime.go:30) and plugin proxy (plugin.go:123), and lands in sdkmcp.CallToolParams.Arguments any at exactly two repo-wide construction sites — client.go:142 and stdio_client.go:191 (confirmed by grep including test files). There the non-nil interface defeats both the SDK's Arguments == nil guard (go-sdk v1.7.0 client.go:1278-1284) and omitempty's isEmptyValue, so the wire emits "arguments":null — a violation of the MCP spec's object-or-absent rule that strict servers reject. The fix stands as planned: a shared adapter-side normalizeArguments at the two construction sites, inside the ADR-067 §2-confined package — the only layer that owns wire correctness — with make check-full green as the gate, unconditional because no catalog-pinned file or line is touched.
What I revised. Three claims were corrected cleanly, none of which change the fix. (1) "Bare nil is green pre-fix and pins the SDK guard" — retracted: through the port, bare nil is a typed-nil map; the nil-map row is the regression test, red pre-fix, and the guard's nil-interface path is unreachable through the port, so no white-box pin. (2) "Violates the domain model's typing" — retracted as a category error: the correct normative basis is the MCP spec plus the go-sdk's documented intent ("Avoid sending nil over the wire"); the domain model plays no normative role, and "no model edit" holds for that reason, not conformance. (3) The plugin-layer pass-through pin — dropped as zero-value (green before and after; the forwarding contract is already pinned for non-empty args). Also confirmed per the griller's corrections table: the HTTP wire test uses the existing newSDKTestServer harness with an inline NewClient — newTestClient is stdio-only; and the server-side strict/lenient evidence is reframed as issue-sourced exposure symptoms, not verified-by-me facts, with the PR citing them as reported in #1398.
Open questions. Two, both minor and non-blocking: (a) whether the optional live reproduction against @modelcontextprotocol/server-filesystem runs at PR time — node/npx are present, so it genuinely upgrades the strict axis from issue-sourced to verified-in-environment at low cost, but it remains a nice-to-have, not a gate; (b) whether the doc-only note on the tools.MCPClient.CallTool port ("nil args are normalized by implementations") is acceptable to the maintainers or should stay silent — I recommend including it so a future port consumer doesn't re-introduce the pattern. The diagnosis and plan are settled; I proceed as planned with the corrected test matrix and protocol-compliance framing.
| # | Claim / premise | Outcome |
|---|---|---|
| 1 | "Bare nil is green pre-fix and pins the SDK guard" | Retracted — through the port, bare nil is a typed-nil map; the nil-map row is the regression test (red pre-fix); the guard's nil-interface path is unreachable through the port, so no white-box pin |
| 2 | "The wire null violates the domain model's ToolCall.arguments: object typing" |
Retracted (category error) — correct normative basis is the MCP spec + go-sdk's documented intent; the domain model plays no normative role |
| 3 | "newTestClient harness for the HTTP wire test" | Corrected — newTestClient is stdio-only; HTTP test uses newSDKTestServer + inline NewClient |
| 4 | Placement of new wire tests in pinned files | Resolved — new unpinned wire_arguments_test.go file; zero catalog-pin drift, no coordination-rule trigger |
| 5 | Plugin-layer pass-through pin (from opening) | Dropped — green before and after the fix; zero regression value; forwarding already pinned for non-empty args |
| 6 | "No node/npx in this environment" (griller's claim) | Corrected by subject — node/npx/uvx are present; conclusion survives (evidence remains issue-sourced this session) |
- Fix: package-private
normalizeArguments(args map[string]interface{}) map[string]interface{}ininternal/infrastructure/mcp/(nil →map[string]interface{}{}, else passthrough), called at the only two repo-wideCallToolParamsconstruction sites —client.go:142andstdio_client.go:191— satisfying ADR-067 §2 (wire correctness belongs to the SDK-confined adapter). No changes to the plugin, executor, LLM adapters, domain port, ADR, or domain model. - Framing: protocol compliance (MCP spec object-or-absent rule + go-sdk's documented "Avoid sending nil over the wire" intent), not domain-model conformance; server strict/lenient behavior cited as issue-sourced exposure evidence.
- Tests: new
internal/infrastructure/mcp/wire_arguments_test.go(catalog-inert) — per transport, one nil-map regression row (CallTool(ctx, name, nil)→ server-observed"{}", red pre-fix) and one non-empty passthrough row;normalizeArgumentsunit table; CC ≤ 6. Fixture:args-capturetool added to unpinnedtestdata/stdioserver/main.go. No SDK-guard white-box pin. - Verification:
go test ./internal/infrastructure/mcp/... ./internal/tools/integrations/mcp/...thenmake check-full— unconditional green (no catalog-pinned file/line touched). Optional (PR-time, not a gate): live reproduction against@modelcontextprotocol/server-filesystemusing the present node/npx to upgrade the strict axis to verified-in-environment.