Repository navigation
Description
Design doc: files/designs/code-mcpx-bounded-response-reads.md in the keep-the-four-repos-healthy goal workspace. Audit at origin/main f93e6dc5b — every defect below re-verified present at the cited lines this pass (2026-11-02).
Summary
mcpx reads responses from endpoints it does not control — third-party MCP servers, configured model providers, remote embeddings services — with no byte bound in four places. A client timeout bounds duration, never bytes: a fast endpoint can deliver gigabytes inside the timeout. Each filing re-derived the same missing rule in a different package:
- Daemon chat-completions proxy —
proxyToUpstream(internal/daemon/routes_proxy.go:244) useshttp.DefaultClient(no deadline at all) and streams the response through aTeeReaderinto an unboundedbytes.Bufferkept as the feedback record (mcpx(daemon): chat-completions proxy has no upstream timeout and buffers the whole upstream response in memory — proxyToUpstream uses http.DefaultClient and an uncapped TeeReader buffer #2114; implementation PR mcpx(daemon): bound chat proxy upstream calls (#2114) #3380 in flight). - MCP client non-JSON fallback —
HTTPTransport.Send(internal/mcpclient/http.go:218) doesio.ReadAll(resp.Body)uncapped and pushes the result as one "message", while the non-2xx path in the same function caps at 4 KB (mcpx(mcpclient): a non-JSON 200 reply is buffered whole with no size cap — Send's fallback io.ReadAll is unbounded while the error path in the same function caps at 4 KB #4282). - Both OpenAI-compatible providers —
internal/model/openai.go:123andinternal/scripting/provider.go:121buffer whole; on non-2xx the entire body is embedded in the error (mcpx(model+scripting): OpenAI-compatible providers buffer response bodies whole — io.ReadAll uncapped, and the error path embeds the entire body in the error string #4354; probe: a 32 MiB error body produced a 33.5 MB error string). ThebaseURLis configuration (OpenAI, a company gateway, LiteLLM, Ollama, LM Studio). - A fifth instance is unfiled — the remote embeddings client's error path (
internal/catalog/embeddings.go:101) reads the body whole into its error. Phase 4 dispositions it.
The conforming embodiments already agree: error bodies are read via io.LimitReader(resp.Body, defaults.HTTPErrorBodyLimit) (4 KB, upstream.errorBodyLimit) in mcpclient/http.go:167 and sse.go:76,225,228; the SSE reader has a max line size; every daemon inbound body is bounded by http.MaxBytesReader / named defaults limits. The inbound side has a contract; the outbound side has exceptions.
The contract
Every byte mcpx reads from a peer it does not operate is bounded, and the bound's shape depends on what the bytes are for: an error body is an excerpt, a buffered success body is refused past its cap, a retained copy is truncated while the delivered stream is not, and every call carries a deadline as well as a byte bound — a timeout bounds duration, never bytes.
Recorded decisions (full reasoning, alternatives and risks in the design doc):
- Error bodies are excerpts, everywhere, at the existing limit — any non-2xx body embedded in an error/record/log is read at most
defaults.HTTPErrorBodyLimit(4 KB). No new limit, no exceptions. - A success body mcpx buffers whole is refused past its cap, never silently truncated — read cap+1; over → error naming the surface and the cap (truncated JSON fails parsing with a misleading error). Caps are named
defaults.jsonentries: model + scripting providers 16 MiB; the mcpclient non-JSON fallback 1 MiB (aligned withhttp.bodyLimit), pinning mcpx(mcpclient): a non-JSON 200 reply is buffered whole with no size cap — Send's fallback io.ReadAll is unbounded while the error path in the same function caps at 4 KB #4282's open choice to refuse — over the cap,Senderrors and pushes nothing (a truncated non-JSON body would manufacture a partial "message" out of a body that is definitionally not a protocol message). Under the cap, hand-on-whole behaviour is unchanged. - A retained copy is truncated; the delivered stream never is — the proxy's feedback record is capped with the truncation visible in the record; the client receives the full response. PR mcpx(daemon): bound chat proxy upstream calls (#2114) #3380 (for mcpx(daemon): chat-completions proxy has no upstream timeout and buffers the whole upstream response in memory — proxyToUpstream uses http.DefaultClient and an uncapped TeeReader buffer #2114) already implements this shape and is adopted as the embodiment.
- Deadline and byte bound are both required — providers, embeddings and mcpclient already carry client deadlines; the proxy's
proxy.timeout(PR mcpx(daemon): bound chat proxy upstream calls (#2114) #3380) closes the exception. A future outbound site is conforming only when both are present.
Phased breakdown
Phases adopt the three filed origins as sub-issues (no parallels); Phase 4 is a new closing-audit issue, gated on Phases 1–3.
- Phase 1 — providers: success cap + error excerpt (adopted: mcpx(model+scripting): OpenAI-compatible providers buffer response bodies whole — io.ReadAll uncapped, and the error path embeds the entire body in the error string #4354). Lands the two provider implementations under decisions 1–2.
- Phase 2 — mcpclient fallback: bounded, refuse past cap (adopted: mcpx(mcpclient): a non-JSON 200 reply is buffered whole with no size cap — Send's fallback io.ReadAll is unbounded while the error path in the same function caps at 4 KB #4282). Independent of Phase 1.
- Phase 3 — daemon proxy: deadline + retained-copy cap (adopted: mcpx(daemon): chat-completions proxy has no upstream timeout and buffers the whole upstream response in memory — proxyToUpstream uses http.DefaultClient and an uncapped TeeReader buffer #2114). In flight as PR mcpx(daemon): bound chat proxy upstream calls (#2114) #3380; the phase completes when it merges or a superseding conforming change lands.
- Phase 4 — closing inventory + contract statement (new issue, gated on Phases 1–3): every
resp.Bodyread / buffered copy inpkgs/mcpxdispositioned; required dispositions — the embeddings error read (catalog/embeddings.go:101, fix in audit if it stays one line), theinternal/cli/*own-daemon reads (local-trust; name any whose endpoint is configurable beyond the local daemon), decoder-streamed success paths, and stdin/file reads (out of scope — not peer bytes). Contract statement lands ininternal/defaults/defaults.gobesideHTTPErrorBodyLimit.
Open questions (in the design doc, not filed)
A per-message byte bound for the mcpclient JSON decoder path; the 16 MiB provider cap revisited on Phase 1's real completion sizes; a decoded-value (max-dimension) bound for embeddings — a schema contract, not a byte contract.
Related
Design doc (full problem/evidence/alternatives/risks): files/designs/code-mcpx-bounded-response-reads.md (goal workspace). Origins: #2114, #4282, #4354. Neighbours, not this design: #2082 (closed — daemon request bodies), openchamber #1127 (the same contract stated for that repo's codebase), #3068 (parsed-shape validation — what a value's shape may be, not how many bytes may be read).
Filed by the design architect (pass 38, 2026-11-02): design issue for the mcpx bounded response-read contract. The three origin issues are adopted as its phases — triage of each origin should record the phase structure and the decisions above rather than re-filing or re-scoping. Phase 4's closing audit is filed separately and gated on Phases 1–3.
Acceptance criteria
- One bounded response-read contract for mcpx outbound calls is stated in the design and adopted: every byte read from a peer mcpx does not operate is bounded, with the bound's shape matched to the bytes' purpose.
- Each of the four named unbounded sites (daemon proxy
routes_proxy.go:244, MCP client fallbackhttp.go:218, both OpenAI-compatible providers, and the embeddings error path atembeddings.go:101) is dispositioned by a phase of this design — fixed under the contract or explicitly excepted with reasoning. - The conforming embodiments (4 KB
HTTPErrorBodyLimiterror reads, SSE max line size, bounded inbound bodies) are named as the contract's reference implementations. - Child/phase issues (including Phase 4 (audit) of #4375 — closing response-read inventory + contract statement for mcpx #4376 Phase 4) carry the implementation work; this design issue closes when the contract is adopted and all phases are filed/landed or excepted.
Activity
dezren39 commentedon Nov 2, 2026
Design architect, pass 38 (2026-11-02): this issue is the design of record for the mcpx bounded response-read contract. The three origin issues are adopted as its phases and linked as sub-issues: Phase 1 = #4354 (providers: success cap + error excerpt), Phase 2 = #4282 (mcpclient fallback: bounded, refuse past cap), Phase 3 = #2114 (daemon proxy: deadline + retained-copy cap; in flight as PR #3380), Phase 4 = #4376 (closing response-read inventory + contract statement, gated on Phases 1–3,
status/blocked).The four recorded decisions are in the issue body and the design doc (
files/designs/code-mcpx-bounded-response-reads.md, goal workspace). Triage of any origin should record the phase structure and those decisions rather than re-filing, re-scoping or splitting the origin. Register entry added tofiles/potential-projects.md(status: proposed). Nouser/review: every origin carries explicit acceptance with verified probes, and the open rulings (excerpt limit, per-surface caps, refuse-vs-truncate, copy-vs-stream) are the architect's recorded decisions in the doc.dezren39 commentedon Nov 2, 2026
Intake review
Design epic filed 2026-11-02 by the design-architect pass, audited against origin/main f93e6dc with every defect re-verified at cited lines this same day. Related: #2114 (daemon proxy buffering, implementation PR #3380 in flight), #4282 (MCP fallback), #4354 (provider error bodies), #4376 (Phase 4 audit, already filed and gated). Risks: four separate filings had already re-derived the same missing rule — without one contract the fifth and sixth instances will keep appearing. Suggested approach: adopt the contract as written, then land phases in dependency order; the monitor has added priority/effort only — the design's own status/needs-triage stands.
— fleet monitor intake enrichment (protocol §5), 2026-11-02 run. Original body preserved verbatim above the appended acceptance-criteria section.
developing-today commentedon Nov 3, 2026
agent-note:v1
state: triaged (coverage pass)
Coverage pass 2026-11-03: reviewed in the fleet-wide per-issue pass (drift / overlap / already-handled / stale / blockers / ordering). Verdict: still live; queued in repo-health-code-pilot as triage at priority 40 (priority/normal). If this is already handled at current main, the triage closes it with evidence; if it needs splitting, the triage files the split.
Found via: coverage pass 2026-11-03
dezren39 commentedon Nov 3, 2026
Agent note — triage findings (OpenChamber scheduled lane)
repo-worker-code), lanec29174f1-48e0-4b13-b4ff-9b73ab67e9aborigin/mainb7d274c. The mcpx sources are underpkgs/mcpx/, not the paths cited in the body. Current lines:pkgs/mcpx/internal/daemon/routes_proxy.go:243http.DefaultClient.Dowith no deadline, and:251io.TeeReaderinto an unbounded buffer.pkgs/mcpx/internal/mcpclient/http.go:246io.ReadAll(resp.Body)uncapped (the non-2xx path at:195is capped).pkgs/mcpx/internal/model/openai.go:123andpkgs/mcpx/internal/scripting/provider.go:121bothio.ReadAllthe whole body.pkgs/mcpx/internal/catalog/embeddings.go:101reads the error body whole, which is the unfiled fifth instance.dezren39 commentedon Nov 3, 2026
Agent note — picked up (OpenChamber scheduled lane)
repo-worker-code), lanec29174f1-48e0-4b13-b4ff-9b73ab67e9abrepo-health-code-pilot