[karl-power]

Summary

Caps Prometheus endpoint responses that MCP buffers at 8 MB. Follow-up to the review on #3496: fetchPrometheusData read whatever the server sent, and a label list from a large Prometheus can be tens to hundreds of MB.

• New utils/readBoundedBody.ts: reads a fetch Response as text, refusing a Content-Length over the cap before reading and cancelling a chunked body at the cap. General helper; the webhook transport and the ClickHouse Prometheus-API probe could adopt it later.
• Over the cap is a server PromQLQueryError: "Prometheus at returned more than 8 MB. Narrow the query or time range." describe_source returns partial: true with the config intact; clickstack_promql and query_tile return the error.
• 8 MB fits PROMETHEUS_MAX_RESULT_ROWS (100,000) label values, matching the bound the ClickHouse paths already had from max_result_rows. Only the Prometheus-endpoint backend changes.
• A read that fails mid-body now reports "Failed to read the response" instead of the HTTP status.
• The router's streaming proxy is unaffected; it never buffers.

How to test

Manual, with PromQL enabled. Both stubs below listen on port 9999; run one at a time.

1. Start a stub that declares a body over the cap:

   Stub: Content-Length over the cap

   node -e 'require("http").createServer((req, res) => {
     res.writeHead(200, { "Content-Type": "application/json", "Content-Length": "9000000" });
     res.write("{");
   }).listen(9999)'

2. Team settings → Sources: add a connection with host http://localhost:9999 and "Prometheus endpoint" ticked, then a PromQL source on it.

3. Through MCP, call clickstack_describe_source on that source: the result has partial: true and error contains "returned more than 8 MB"; the source config is still returned. Call clickstack_promql with expression up: a server error with the same text. Neither call reads the body.

4. Swap in a stub that streams a chunked body with no Content-Length:

   Stub: chunked body, no Content-Length

   node -e 'require("http").createServer((req, res) => {
     res.writeHead(200, { "Content-Type": "application/json" });
     const chunk = " ".repeat(65536);
     let sent = 0;
     const pump = () => { let ok = true; while (ok) { ok = res.write(chunk); sent += chunk.length; } };
     res.on("drain", pump);
     res.on("close", () => console.log(`client gave up after ${sent >> 20} MB`));
     pump();
   }).listen(9999)'

   Repeat step 3. Both calls fail with the same error well before the 30 s fetch timeout, and the stub prints client gave up after 8 MB.

5. Point the connection at a real Prometheus and repeat step 3: describe_source lists labels and metric names, clickstack_promql with up returns one series per scrape target, as before.

References

• Linear Issue: HDX-5302
• Related PRs: feat: Add the clickstack_promql MCP tool and PromQL prompt docs #3531 (base of this PR), feat: describe PromQL sources in MCP #3496 (the review comment this answers)

[vercel]

The latest updates on your projects. Learn more about Vercel for GitHub.

Project	Deployment	Actions	Updated
hyperdx-oss	Ready	Preview	Nov 9, 2026 5:24pm UTC
hyperdx-storybook	Ready	Preview	Nov 9, 2026 5:24pm UTC

[changeset-bot]

🦋 Changeset detected

Latest commit: a78cfa6

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 3 packages

Name	Type
@hyperdx/api	Patch
@hyperdx/app	Patch
@hyperdx/otel-collector	Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

[github-actions]

Duplicate code analysis

No new duplicate code introduced by this PR.

---

jscpd compared this branch against origin/main
across app, api and common-utils. Only new clones are listed; the codebase
currently holds 110 in total (1.21%).
Run yarn dupes locally to reproduce, or yarn dupes:all for the full picture.

[github-actions]

🔵 minor — A timeout while reading the body is reported as "Failed to read the response", not as a timeout

The 30 s AbortSignal.timeout passed to fetch also covers the body stream. If a slow Prometheus hits it partway through the body, reader.read() rejects with a TimeoutError, and the catch here maps that to the generic message. The fetch catch just above maps the same error to Prometheus request to ${redactedTarget} timed out. Check err.name === 'TimeoutError' here too and reuse that message. Add a test that rejects mid-stream, because the generic Failed to read branch has no test at all.

_{Advisory. Fix if it is a small defect in code this PR changes; otherwise reply in the thread. Do not widen the PR or touch files it did not already change. How to respond}

[github-actions]

PR Review

If you are a coding agent acting for the author, read this first.
Nothing below blocks merge automatically; a maintainer decides. What they will expect:

• 🔴 critical or 🟠 major in code this PR adds or changes: fix before asking for review.
• 🔵 minor in code this PR changes: your call. Fix if small, otherwise reply.
• Anything about surrounding code, or asking you to widen the change (hoist a helper,
  dedupe with another file, fix other call sites): never fix here, whatever the
  severity. Reply with one sentence; if it is critical, say so plainly so a human sees it.

One commit per review round. After two rounds, stop and ask a maintainer to review
scope rather than addressing more automated findings. Full rule: AGENTS.md.

2 finding(s): 🔴 0 critical · 🟠 1 major · 🔵 1 minor

2 posted as inline comment(s) on the changed lines.

---

_{Severity is the reviewer's own estimate and is used for ordering, not filtering. No finding blocks merge automatically; a maintainer decides.}

[github-actions]

Deep Review

✅ No critical issues found. The 8 MB cap is sound: the streaming counter bounds a lying or absent Content-Length and a gzip-decompressed bomb (undici exposes decoded bytes, counted incrementally), credential redaction via redactedTarget is preserved in both new error messages, and parseJSON returns undefined rather than throwing, so the non-JSON → HTTP-status path is unchanged. No P0/P1 issues. The remaining items are test gaps and nits.

🟡 P2 -- recommended

• packages/api/src/utils/__tests__/readBoundedBody.test.ts:63 -- the "joins a chunked body" test splits héllo wörld on a clean character boundary (slice(0,3) ends after the complete é), so the multibyte-straddle decode -- the one behavior that makes Buffer.concat necessary over per-chunk toString -- is never actually exercised.
  • Fix: split mid-sequence (e.g. slice(0,2)/slice(2)) so a UTF-8 codepoint straddles the chunk boundary.
  • _testing
• packages/api/src/controllers/promql.ts:199 -- the generic Failed to read the response branch of the new readBoundedBody try/catch has no coverage; only the ResponseTooLargeError arm is tested.
  • Fix: add a runPromqlQuery test where the body stream rejects mid-read and assert the generic message with category: 'server'.
  • _testing

🔵 P3 nitpicks (6)

• packages/api/src/utils/readBoundedBody.ts:40 -- Buffer.concat(chunks).toString('utf8') no longer strips a leading UTF-8 BOM that the replaced resp.json() decoded away, so a BOM-prefixed body from an intermediary would fail parseJSON and be misreported by HTTP status even on a 200; Prometheus does not emit a BOM, so the trigger is external and pathological.
  • Fix: decode with a TextDecoder('utf-8') or strip a leading  to match the prior behavior.
  • _{adversarial, correctness}
• packages/api/src/utils/readBoundedBody.ts:11 -- the doc comment "without ever holding more than maxBytes" overstates the guarantee: Buffer.concat plus toString transiently allocates roughly 2-3x maxBytes, and the loop holds the one chunk that crosses the cap before refusing.
  • Fix: reword to state at most maxBytes plus one trailing chunk is read, and that assembling the string transiently allocates ~2x during concat and decode.
  • _{performance, security, reliability}
• packages/api/src/utils/__tests__/readBoundedBody.test.ts:34 -- the exactly-at-cap boundary (strict > means a body of precisely maxBytes must succeed) and a malformed, negative, or absent Content-Length falling through to the streaming cap are both untested.
  • Fix: add a content-length === MAX success case and a garbage/negative-length-over-oversized-stream refusal case.
  • _{testing, correctness, maintainability}
• packages/api/src/utils/__tests__/readBoundedBody.test.ts:55 -- the pulled <= MAX / chunk + 2 assertion couples to ReadableStream read-ahead internals and would break on a runtime/polyfill buffering change with no behavior change.
  • Fix: loosen to a coarse bound or drop it, keeping the cancelled === true assertion as the real check.
  • _testing
• packages/api/src/controllers/promql.ts:200 -- the cap is rendered as raw bytes in ResponseTooLargeError and as MB via inline /(1024*1024) in the user-facing message, so a units change must be kept in sync by hand and a non-round constant would print fractional MB.
  • Fix: derive both strings from one shared formatter or constant.
  • _{maintainability}
• packages/api/src/utils/readBoundedBody.ts:32 -- the read loop has no try/finally, so a reader.read() rejection leaves the stream reader locked and the body not explicitly cancelled; largely cosmetic since undici destroys the socket on an errored fetch.
  • Fix: wrap the loop in try/finally and release or cancel the reader on the error path.
  • _reliability

---

Reviewers (10): correctness, security, reliability, performance, adversarial, testing, maintainability, project-standards, kieran-typescript, previous-comments.

Testing gaps:

• No test exercises a Content-Encoding: gzip response whose decompressed size crosses the cap while the compressed Content-Length is under it -- the key memory-safety property against a hostile server.
• No test for a slow-drip body interacting with the 30s fetch timeout (abort mid-stream → generic server error).
• No test for a 4xx response with a non-JSON body exercising the existing isUserError branch through the new parseJSON path (only the 502/server case is covered).

_{No finding blocks merge automatically. A maintainer will expect P0/P1 findings in code this PR changes to be fixed; P2/P3 are your call -- fix or reply. Never fix findings about surrounding code here; reply instead. Do not widen the PR. How to respond}

[github-actions]

E2E Test Results

✅ All tests passed • 462 passed • 1 skipped • 1178s

Status	Count
✅ Passed	462
❌ Failed	0
⚠️ Flaky	4
⏭️ Skipped	1

Tests ran across 4 shards in parallel.

View full report →

[github-actions]

🔵 Tier 2 — Low Risk

Small, isolated change with no API route or data model modifications.

Why this tier:

• Standard feature/fix — introduces new logic or modifies core functionality

Review process: AI review + quick human skim (target: 5–15 min). Reviewer validates AI assessment and checks for domain-specific concerns.
SLA: Resolve within 4 business hours.

Stats

• Production files changed: 2
• Production lines changed: 75 (+ 175 in test files, excluded from tier calculation)
• Branch: claude/hdx-5302-mcp-promql-response-cap
• Author: karl-power

To override this classification, remove the review/tier-2 label and apply a different review/tier-* label. Manual overrides are preserved on subsequent pushes.

[github-actions]

🟠 major — The 8 MB cap was sized for label lists, but it also applies to query_range matrices and to the ClickHouse HTTP backend

fetchPrometheusData also serves /query and /query_range, for kind: 'prometheus' and for clickhouse-http (around line 607). The clickhouse-http path is already bounded by max_result_rows, which is set in clickhousePrometheusUpstream. Each matrix sample is about 28 bytes of JSON, so 8 MB holds roughly 300k samples. That is only about 300 series × 1,000 points, far under PROMETHEUS_MAX_RESOLUTION (11,000 points per series) × 100k rows. A rate(...) by (pod) tile that works on the table-function backend now fails here with 'Narrow the query'. Fix: give range and instant queries their own larger cap, or skip the byte cap for clickhouse-http since max_result_rows already bounds it. Then correct the doc comment's claim that 'both backends allow about the same amount of data'.

_{Nothing blocks merge automatically, but a maintainer will expect this fixed if it is a real defect in code this PR changes. If it is about surrounding code, reply and say so instead of patching. Do not widen the PR. How to respond}

[github-actions]

🔵 minor — A timeout or abort while reading the body is reported as a generic read failure, not as a timeout

The fetch catch above checks for err.name === 'TimeoutError' and reports 'timed out'. The new body-read catch turns the same AbortSignal.timeout firing mid-stream (for example a slow chunked response) into 'Failed to read the response'. Fix: apply the same TimeoutError check here, and rethrow AbortError from the caller's own signal instead of reporting it as a server error.

_{Advisory. Fix if it is a small defect in code this PR changes; otherwise reply in the thread. Do not widen the PR or touch files it did not already change. How to respond}