fix(mcp): read official registry responses to EOF instead of one chunk by chenmingwei23 · Pull Request #4814 · kirodotdev/KiroCrew

chenmingwei23 · 2026-08-21T02:28:22Z

Problem / Motivation

Searching for MCP servers in Agent Capabilities -> Connections -> MCP Servers -> Add Server returns No servers found for queries that do match entries in the official registry. aws, slack and github all come back empty while a direct request to the same endpoint returns 17+ entries, and the UI shows no error -- a provider failure is indistinguishable from a genuinely empty result.

The failure depends on how the response is chunked, not on the query text: a zero-hit query returns a small body that fits in one chunk and parses fine, which is why the feature looks like it works until you search for something popular.

Why it matters

Add Server is the only in-product path to the official MCP registry, so this makes registry discovery unusable for exactly the queries a user is most likely to type. Because ProviderRegistry.search() isolates provider faults by design, the failure is silent: the user concludes the registry has no aws server rather than that the lookup broke, and nothing in the UI suggests otherwise. The only trace is a WARNING in the gateway log.

What changed (motivation → approach → change)

_fetch_json() read the whole body with a single call:

body = await resp.content.read(_MAX_RESPONSE_BYTES + 1)

aiohttp.StreamReader.read(n) returns up to n bytes -- it resolves as soon as any data is buffered rather than waiting for EOF. The registry streams search pages with no Content-Length, so the body arrives in several chunks and that single call returns only the first one (~8-12 KiB here). json.loads then fails with Unterminated string, _fetch_json converts the JSONDecodeError into ProviderUnavailableError, and ProviderRegistry.search() catches it for fault isolation and returns an empty list for that provider -- which the dashboard renders as an ordinary 0-result search.

The fix streams the response to EOF in bounded chunks via resp.content.iter_chunked() -- the mechanism already used at nine sites in this repo -- and enforces the existing size cap against the accumulated total, so an oversized body is still refused mid-stream rather than buffered whole. The 404 -> None and transport-failure -> ProviderUnavailableError contracts are unchanged.

Scope is deliberately limited to the reported defect. The same single-read shape survives in three unrelated readers (updates.py:740, feedback.py:179, source_providers.py:2422); each has its own reachability and consequence argument, and folding them in here was already tried and rejected on this code (#2224's updates.py hunk was asked to be reverted as an undeclared change). They are tracked in #4829.

Tests

Six tests in a new TestFetchJsonRead class (test/test_mcp_providers.py), all hermetic -- a fake StreamReader hands out queued chunks and honours the documented "up to n bytes" contract:

test_multi_chunk_body_is_assembled -- the regression: a document split in three parses whole.
test_heavily_fragmented_body_is_assembled -- byte-at-a-time delivery still assembles.
test_oversized_body_rejected_mid_stream -- the cap applies to the accumulated total, and the read aborts instead of draining the rest.
test_body_at_exact_cap_is_accepted -- the boundary is not off by one.
test_404_returns_none_without_reading -- a missing entry stays distinct from an unreachable registry.
test_truncated_body_surfaces_as_provider_unavailable -- a genuinely malformed document is still a provider failure, not a crash.

Reverting the production hunk fails the first four by name; the last two are contract-preservation tests and correctly stay green either way. The test double models iter_chunked and deliberately also keeps read(n), so a single-read implementation is still exercised and the revert proof stays non-vacuous.

Manual verification

Reproduced and re-verified through three probes, before and after the change:

probe	before	after
`_fetch_json` against the live registry (`search=aws&limit=20`, 28,858 bytes, no `Content-Length`)	`ProviderUnavailableError: Unterminated string starting at ... char 7937`	20 servers parsed
`_fetch_json` against a local chunked-stream server (53 KB in 8 KiB chunks)	`Unterminated string ... char 8126`	120 servers parsed
full stack in an isolated instance -- `GET /api/mcp/discover?q=<q>&limit=20`, the request the Add Server dialog issues	`{"results": [], "providers": ["official"]}`, with `WARNING MCP provider official failed for query 'aws'` and the traceback in the log	`aws` 20, `slack` 18, `github` 20 results

Related Issues

Fixes #2222

Same defect as #2224 (open, currently conflicting) and #2232 (closed as a duplicate of it); this change is scoped to _fetch_json and carries the regression tests. Credit to @ayahiro1729 for the report and the diagnosis, and to @ChickenisLegit for independently finding it.

Why no screenshot: backend-only change; the diff touches no frontend path and renders no new UI -- the user-visible effect is the existing Add Server list going from empty to populated, evidenced by the API probe above.

Checklist

Single commit with a Conventional Commits title (feat|fix|docs|refactor|perf|test|chore|ci|build|revert: ...)
Existing tests pass and new tests added for new functionality
Self-review completed; code follows project style guidelines
Documentation updated (if applicable)
No secrets, credentials, or internal references in the diff

github-actions · 2026-08-21T02:31:03Z

GPT 5.6 Review — ✅ no blocking findings

GPT 5.6 completed its review of d224c0d1cd691ef4c73c223b75921e3279581ade and found no blocking issues.

This comment is updated in place on each push.

Review details

No findings.
[GPT-REVIEWED] d224c0d

False positive or not applicable? A repository writer can comment:
/ai-review override gpt d224c0d1cd691ef4c73c223b75921e3279581ade: <one-sentence reason>

github-actions · 2026-08-21T02:31:04Z

Design Review (Fable 5) — ✅ PASS

Design-level review of d224c0d1cd691ef4c73c223b75921e3279581ade — updated in place on each push. A BLOCK verdict blocks PR readiness; PASS/CONCERNS are advisory.

The diff matches the description end-to-end: the root cause (single read(n) vs. streamed body) is fixed at the right layer, the size cap semantics are preserved mid-stream, contracts (404→None, transport→ProviderUnavailableError) are untouched, and the duplicate single-read sites are explicitly deferred to a tracked issue rather than smuggled in. Tests are hermetic and prove the regression. No design-level concerns survive the kill-filter.

Design-Verdict: PASS

Root-cause fix at the correct seam, bounded as before, deliberately scoped, with revert-proving hermetic tests — sound and proportionate.

Suggestions

The silent-failure UX the description documents (provider fault renders as an ordinary empty result) outlives this fix for any future provider error; worth a follow-up issue alongside Three HTTP readers truncate chunked responses via a single StreamReader.read() #4829 if one doesn't exist.

[DESIGN-REVIEWED] d224c0d

github-actions · 2026-08-21T02:31:32Z

Opus 4.8 Review — ✅ no blocking findings

Reviewed d224c0d1cd691ef4c73c223b75921e3279581ade — this comment is updated in place on each push.

Review details

The diff is a small, well-scoped fix: replacing a single resp.content.read(n) (which returns only already-buffered bytes and thus truncates multi-chunk documents) with a bounded iter_chunked accumulation loop.

Verifying the correctness claims independently:

Size cap parity — old raised on len(body) > _MAX; new raises on accumulated total > _MAX. At exactly _MAX both accept; above it both refuse. Consistent.
UTF-8 boundary split — decode happens after b"".join(chunks), so multibyte chars split across chunks reassemble. No decode error introduced.
Empty/truncated/malformed body — json.loads raises JSONDecodeError, caught and re-raised as ProviderUnavailableError, same as before.
404 — early return None before any read; unchanged.
Timeout — ClientTimeout(total=...) still bounds the whole read; a slow-drip stream can't hang indefinitely.
Mid-stream refusal — oversized body refused on crossing the boundary without draining; the untrusted-external-content DoS guard is preserved, not weakened.
Async — iter_chunked is async; no blocking syscall in a loop.

The tests are hermetic (monkeypatch only, no filesystem or child processes). No AUTOSDE rule is weakened. Nothing survives falsification, and I find no additional grounded defect in Step 2.

No findings.

[OPUS-REVIEWED] d224c0d

Verdict parsed from the review's SHA-scoped output markers for commit d224c0d1cd691ef4c73c223b75921e3279581ade.

False positive or not applicable? A repository writer can comment:
/ai-review override fable d224c0d1cd691ef4c73c223b75921e3279581ade: <one-sentence reason>

github-actions · 2026-08-21T02:34:01Z

First Principles Review (Fable 5) — ✅ PASS

Premise-level review of d224c0d1cd691ef4c73c223b75921e3279581ade — why this exists and whether the shipped surface is the smallest honest version. Updated in place on each push. A BLOCK verdict blocks PR readiness; PASS/CONCERNS are advisory.

I've read the contract, the intent file, and the patch, and verified the claims against the repository: the sibling single-read count (3: updates.py:740, feedback.py:183, source_providers.py:2422, grepped \.content\.read\(), the nine pre-existing iter_chunked sites, and the absence of any shared aiohttp-client bounded-read helper (read_bounded_json in _shared.py reads server-side web.Request bodies; skillsh._read_bounded is sync urllib — neither fits).

First-Principles-Verdict: PASS

A one-hunk mechanism-level fix for a reported defect (#2222), with siblings counted, declared, and tracked rather than silently left.

What this change ships

Intent: make MCP registry search in Add Server return the servers the registry actually has. FIX.

Registry search/detail now assembles a multi-chunk response instead of parsing the first chunk — justified (the fix, derived from Official registry MCP search returns 0 results due to truncated response read #2222).
Oversized responses are now refused mid-stream instead of after buffering — justified, declared; preserves the existing 5 MiB ceiling against untrusted external content.
New module constant _HTTP_READ_CHUNK_BYTES — justified; mandated by the documented "every limit has an owning module" rule.
Six hermetic regression tests with a fake stream reader — declared; four fail on revert.

No existing mechanism does this job: _shared.read_bounded_json is server-side (web.Request), skillsh._read_bounded is sync urllib; the inline iter_chunked accumulate-and-cap shape matches the nine existing client-side sites.

Watch

Three counted siblings keep the single-read shape (updates.py:740, feedback.py:183, source_providers.py:2422; grepped \.content\.read\(_MAX). Deferral is grounded — fix(mcp): correctly read chunked responses from official registry #2224's wider hunk was asked to be reverted, and Three HTTP readers truncate chunked responses via a single StreamReader.read() #4829 tracks them — so this is accepted-and-deferred, not a point patch to block on.

[FIRST-PRINCIPLES-REVIEWED] d224c0d

MCP server search in Add Server returned "No servers found" for queries that match entries in the official registry (aws, slack, github). _fetch_json read the body with a single resp.content.read(cap + 1). StreamReader.read(n) returns only the bytes already buffered, so on the registry's chunked HTTP/2 stream (no Content-Length) it yields the first ~8-12 KiB and json.loads fails on the truncated document. That becomes a ProviderUnavailableError, which ProviderRegistry.search swallows for fault isolation, so the dashboard renders a provider failure as an ordinary zero-result search with no error shown. Drain the response in bounded chunks until EOF, enforcing the existing size cap against the accumulated total so an oversized body is still rejected mid-stream rather than buffered whole.

chenmingwei23 · 2026-08-21T03:25:38Z

Dispositions for the First Principles CONCERNS verdict on 348b4f3e35acad89444742c15fa4559387294035. Both items were legitimate; one is fixed in the code, one is deferred to a filed issue.

Subtraction: replace the hand-rolled while True / read / break loop with iter_chunked -- fixed in d224c0d1c.

Replace the hand-rolled while True / read / break loop in _fetch_json with async for chunk in resp.content.iter_chunked(_HTTP_READ_CHUNK_BYTES) -- the existing aiohttp mechanism already used twice in this repo (petdex_import.py:162, link_meta.py:222); drops the manual EOF handling.

Correct, and the idiom is more established than the verdict says: iter_chunked is used at nine sites under src/kiro_crew/, and petdex_import.py::_read_capped already pairs it with exactly this accumulated-total cap. _fetch_json now mirrors that shape, which removes the manual EOF branch and the bytearray accumulator. Semantics are unchanged: the cap is still checked after every chunk, so an oversized body is refused mid-stream rather than buffered whole. Re-verified after the change -- the live registry and a local chunked-stream server both parse whole (20 and 120 entries), prove.py still reports PROVEN, and the same four regression tests still fail by name when the production hunk is reverted. The test double now models iter_chunked and deliberately keeps read(n), so a single-read implementation is still exercised and the revert proof stays non-vacuous.
Watch: point patch with three counted unfixed siblings of the same root cause -- accepted-and-deferred to Three HTTP readers truncate chunked responses via a single StreamReader.read() #4829.

the identical single-call-plus-cap shape survives at feedback.py:183 (_read_capped_text), source_providers.py:2422 (Jira fetch), and updates.py:740 (_fetch_feed_bytes -- whose comment claims an oversized body "is DETECTED rather than silently truncated", which the same read(n) contract falsifies for a chunked feed).

The count is right and the updates.py comment observation is a genuinely new finding -- that site carries a stale guarantee as well as the bug. Deferred rather than folded in, for two reasons. First, each sibling has its own reachability and consequence argument (a release-feed check, an Aperture reply, a Jira fetch), and none of them is the defect Official registry MCP search returns 0 results due to truncated response read #2222 reported, so bundling them would put three unrelated behaviour changes behind one issue's verification. Second, this exact widening was already tried and rejected on this code: fix(mcp): correctly read chunked responses from official registry #2224 included the updates.py hunk and the GPT lane asked for it to be reverted as an undeclared change to an unrelated path. Three HTTP readers truncate chunked responses via a single StreamReader.read() #4829 names all three sites with their consequences, records the stale comment, and raises the question of whether they should share one bounded-read helper -- which is the shape a reviewer would want if all three move at once.

bolichen97

Tier 1 auto-approve: fix (2 files). Criteria: no conflict, no requested changes, security path denylist clean, design-doc gate clean, SAST annotations clean, security checklist all-NO, AI reviewers green. Category: fix(mcp) — streams the official-registry response to EOF via bounded iter_chunked instead of a single read that truncated multi-chunk pages; the _MAX_RESPONSE_BYTES cap is preserved (checked against the accumulated total), no new parsing introduced.

bolichen97

Tier 1 auto-approve: fix (2 files). Criteria: no conflict, no requested changes, security path denylist clean, design-doc gate clean, SAST annotations clean, security checklist all-NO, AI reviewers green. Category: fix — stream the official MCP registry response to EOF in bounded 64KiB chunks so a multi-chunk search page parses whole instead of truncated; the _MAX_RESPONSE_BYTES cap is preserved and enforced mid-stream.

iamwhatever

Tier 1 auto-approve: fix (2 files). Criteria: no conflict, no requested changes, security path denylist clean, design-doc gate clean, SAST annotations clean (Semgrep + CodeQL success, 0 alerts), security checklist all-NO, AI reviewers green. Category: fix(mcp) stream the official-registry response to EOF in bounded chunks instead of a single read(n) that truncated multi-chunk documents; the _MAX_RESPONSE_BYTES cap is preserved and now enforced against the accumulated total. Clear root cause, source + tests only.

…nk (#4829) (#4858) Three dashboard HTTP readers assumed a single StreamReader.read(cap + 1) returns the whole response body. read(n) returns UP TO n bytes, resolving as soon as any data is buffered, so on a chunked response with no Content-Length it returns only the first buffered chunk (~8-12 KiB) and the caller silently works on a truncated body: - updates._fetch_feed_bytes: release feed parsed from a partial document, so an update check can misread or fail on a chunked feed. The comment claiming an oversized body "is DETECTED rather than silently truncated" was false for a chunked feed and is corrected. - feedback._read_capped_text: a truncated Aperture response was decoded and treated as the full reply. - source_providers Jira fetch: json.loads on a partial document surfaced as SourceProviderError, so the issue read as unavailable. Same defect class as the official-registry site fixed in #4814. Since the single-read shape has been reintroduced independently more than once, extract one shared read_capped_response(resp, cap) helper in the dashboard handlers' _shared module (next to the request-side read_bounded_json) that drains iter_chunked chunks to EOF, enforcing the cap against the accumulated total so an oversized body is refused mid-stream, and clamping the return to cap + 1 bytes so every caller keeps its existing over-cap sentinel unchanged. All three sites route through it; cap values and caller semantics are untouched. Tests lock in: a multi-chunk body is read whole at the helper and at all three call sites (proven red against the previous single-read code), an over-cap body still trips each existing cap-exceeded path, reading stops mid-stream rather than buffering an oversized body, and an exact-cap body arrives complete. Closes #4829 Co-authored-by: Joe Guo <zejiangg@amazon.com>

kirodotdev#4814) MCP server search in Add Server returned "No servers found" for queries that match entries in the official registry (aws, slack, github). _fetch_json read the body with a single resp.content.read(cap + 1). StreamReader.read(n) returns only the bytes already buffered, so on the registry's chunked HTTP/2 stream (no Content-Length) it yields the first ~8-12 KiB and json.loads fails on the truncated document. That becomes a ProviderUnavailableError, which ProviderRegistry.search swallows for fault isolation, so the dashboard renders a provider failure as an ordinary zero-result search with no error shown. Drain the response in bounded chunks until EOF, enforcing the existing size cap against the accumulated total so an oversized body is still rejected mid-stream rather than buffered whole.

…nk (kirodotdev#4829) (kirodotdev#4858) Three dashboard HTTP readers assumed a single StreamReader.read(cap + 1) returns the whole response body. read(n) returns UP TO n bytes, resolving as soon as any data is buffered, so on a chunked response with no Content-Length it returns only the first buffered chunk (~8-12 KiB) and the caller silently works on a truncated body: - updates._fetch_feed_bytes: release feed parsed from a partial document, so an update check can misread or fail on a chunked feed. The comment claiming an oversized body "is DETECTED rather than silently truncated" was false for a chunked feed and is corrected. - feedback._read_capped_text: a truncated Aperture response was decoded and treated as the full reply. - source_providers Jira fetch: json.loads on a partial document surfaced as SourceProviderError, so the issue read as unavailable. Same defect class as the official-registry site fixed in kirodotdev#4814. Since the single-read shape has been reintroduced independently more than once, extract one shared read_capped_response(resp, cap) helper in the dashboard handlers' _shared module (next to the request-side read_bounded_json) that drains iter_chunked chunks to EOF, enforcing the cap against the accumulated total so an oversized body is refused mid-stream, and clamping the return to cap + 1 bytes so every caller keeps its existing over-cap sentinel unchanged. All three sites route through it; cap values and caller semantics are untouched. Tests lock in: a multi-chunk body is read whole at the helper and at all three call sites (proven red against the previous single-read code), an over-cap body still trips each existing cap-exceeded path, reading stops mid-stream rather than buffering an oversized body, and an exact-cap body arrives complete. Closes kirodotdev#4829 Co-authored-by: Joe Guo <zejiangg@amazon.com>

chenmingwei23 requested a review from a team as a code owner August 21, 2026 02:28

github-actions Bot added the readiness: checking Automated validation is still running label Aug 21, 2026

chenmingwei23 force-pushed the fix/official-registry-read-eof branch from 348b4f3 to d224c0d Compare August 21, 2026 03:24

github-actions Bot added readiness: checking Automated validation is still running and removed readiness: passed Eligible automated validation passed for the current revision labels Aug 21, 2026

chenmingwei23 mentioned this pull request Aug 21, 2026

Three HTTP readers truncate chunked responses via a single StreamReader.read() #4829

Closed

github-actions Bot added readiness: passed Eligible automated validation passed for the current revision and removed readiness: checking Automated validation is still running labels Aug 21, 2026

bolichen97 enabled auto-merge (squash) August 21, 2026 04:04

bolichen97 approved these changes Aug 21, 2026

View reviewed changes

bolichen97 merged commit 2b45c1b into main Aug 21, 2026
69 of 70 checks passed

bolichen97 deleted the fix/official-registry-read-eof branch August 21, 2026 04:04

github-actions Bot removed the readiness: passed Eligible automated validation passed for the current revision label Aug 21, 2026

bolichen97 approved these changes Aug 21, 2026

View reviewed changes

iamwhatever approved these changes Aug 21, 2026

View reviewed changes

github-actions Bot mentioned this pull request Aug 21, 2026

fix(dashboard): read chunked responses to EOF #4836

Closed

5 tasks

ayahiro1729 mentioned this pull request Aug 21, 2026

Official registry MCP search returns 0 results due to truncated response read #2222

Closed

1 task

iamwhatever mentioned this pull request Aug 21, 2026

fix(dashboard): stream HTTP response bodies to EOF instead of one chunk (#4829) #4858

Merged

5 tasks

Navigation Menu

Uh oh!

fix(mcp): read official registry responses to EOF instead of one chunk - #4814

fix(mcp): read official registry responses to EOF instead of one chunk#4814
bolichen97 merged 1 commit into
mainfrom
fix/official-registry-read-eof

chenmingwei23 commented Aug 21, 2026 •

edited

Loading

Uh oh!

github-actions Bot commented Aug 21, 2026 •

edited

Loading

Uh oh!

github-actions Bot commented Aug 21, 2026 •

edited

Loading

Uh oh!

github-actions Bot commented Aug 21, 2026 •

edited

Loading

Uh oh!

github-actions Bot commented Aug 21, 2026 •

edited

Loading

Uh oh!

chenmingwei23 commented Aug 21, 2026

Uh oh!

bolichen97 left a comment

Uh oh!

Uh oh!

bolichen97 left a comment

Uh oh!

iamwhatever left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Conversation

chenmingwei23 commented Aug 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Problem / Motivation

Why it matters

What changed (motivation → approach → change)

Tests

Manual verification

Related Issues

Checklist

Uh oh!

github-actions Bot commented Aug 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

GPT 5.6 Review — ✅ no blocking findings

Uh oh!

github-actions Bot commented Aug 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Design Review (Fable 5) — ✅ PASS

Suggestions

Uh oh!

github-actions Bot commented Aug 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Opus 4.8 Review — ✅ no blocking findings

Uh oh!

github-actions Bot commented Aug 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

First Principles Review (Fable 5) — ✅ PASS

What this change ships

Watch

Uh oh!

chenmingwei23 commented Aug 21, 2026

Uh oh!

bolichen97 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

bolichen97 left a comment

Choose a reason for hiding this comment

Uh oh!

iamwhatever left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

chenmingwei23 commented Aug 21, 2026 •

edited

Loading

github-actions Bot commented Aug 21, 2026 •

edited

Loading

github-actions Bot commented Aug 21, 2026 •

edited

Loading

github-actions Bot commented Aug 21, 2026 •

edited

Loading

github-actions Bot commented Aug 21, 2026 •

edited

Loading