Follow-up hardening: MCP init retry, safe HTTP retry semantics, MCP server protocol compliance

[alxxjohn]

Summary

Follow-up to #27, fixing the remaining medium-severity items from the reliability audit. Stacked on improve — retarget to main (or let GitHub auto-retarget) once #27 merges.

MCP adapter: transient init failure no longer poisons the run (cleanr/adapters/mcp.go)

The initialize handshake ran under a sync.Once that cached the first error forever — one expired deadline or a briefly-restarting server made every remaining scenario fail instantly with the stale error. Failed attempts now retry on the next Invoke; a mutex preserves the old serialization of concurrent first invokes.

Safe retry semantics in doWithRetry (cleanr/adapters/http.go, shared by HTTP/OpenAI/Anthropic adapters)

• Closed-body bug: when a 429/503's Retry-After exceeded the remaining context budget, the response was returned with an already-closed body — callers saw read on closed response body instead of the actual rate-limit response. The body is now closed only after committing to another attempt, and drained first so the keep-alive connection is reused rather than re-handshaking against an already-degraded server.
• Non-idempotent replays: transport errors were retried for every method, but a connection can die after the request was delivered — replaying a POST risks duplicate writes or double LLM charges. Transport-error retries are now limited to GET/HEAD/OPTIONS; 429/503 responses still retry for all methods (the server explicitly rejected the request, so a replay cannot double-apply it).

MCP server protocol compliance (internal/mcpserver)

• Ids are kept as raw JSON so id: 0 / id: "" round-trip exactly (omitempty silently dropped them, breaking client correlation), and parse errors carry the spec-required id: null.
• A final request without a trailing newline (one-shot piped clients) is answered at EOF instead of silently dropped.
• Tool execution failures return isError: true results the calling model can read and self-correct from; unknown tools are -32602; contained panics remain -32603. Previously every failure was a blanket -32603, which some clients treat as a broken server.

Behavior changes to be aware of

• POSTs no longer retry on transport errors (connection refused/reset). Targets behind flaky networks will surface those errors instead of silently retrying — intentional, since the alternative is potential duplicate mutations/charges. 429/503 handling is unchanged.
• MCP clients that matched on -32603 for argument errors will now see isError tool results (spec-conformant).

Test plan

• cleanr-dev check (gofiles, fmt, vet, go test -race -shuffle=on ./...) — 431 passed, 0 failed
• New regression tests: init retried after transient failure; 429-over-budget body readable; POST not replayed on transport error; GET transport retry + POST 503 retry still work; unterminated final line answered; falsy id echo; id: null on parse error; isError vs -32602 vs -32603 failure modes

🤖 Generated with Claude Code