[karl-power]

Summary

First PR for PromQL support in MCP: the source tools now understand PromQL sources and hide them when PromQL is disabled.

• clickstack_describe_source returns a PromQL source's label names and metric names instead of a column schema, from either a ClickHouse TimeSeries table or a Prometheus-compatible endpoint.
• clickstack_list_sources and clickstack_describe_source show each PromQL source's backend. TimeSeries sources keep their table and get a hint for reading it with clickstack_sql through timeSeriesTags / timeSeriesData. Prometheus endpoint sources drop their placeholder database and table.
• With ENABLE_PROMQL off, PromQL sources are hidden from list_sources, describe_source, delete_source and the create_dashboard prompt, and save_source no longer accepts kind: "promql".
• Moved joinPrometheusUpstreamUrl into a new controllers/promql.ts so MCP can reach Prometheus endpoints without importing the Express router.

How to test

1. Run ENABLE_PROMQL=true NEXT_PUBLIC_ENABLE_PROMQL=true yarn dev. The dev collector already writes PromQL metrics to default.metrics_ts.
2. Connect an MCP client to the API.
3. Call clickstack_list_sources. Verify the PromQL source shows "backend": "clickhouse" and "table": "default.metrics_ts", and the output includes promqlUsage.
4. Call clickstack_describe_source with the PromQL source ID. Verify it returns labelNames, metricNames, and a nextSteps.sql hint.
5. Restart without ENABLE_PROMQL. Verify clickstack_list_sources no longer lists the PromQL source or promqlUsage.
6. Call clickstack_describe_source and clickstack_delete_source with the PromQL source ID. Verify both return "not found" and the source still exists in the UI.

References

• Linear Issue: HDX-5302

[vercel]

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

Project	Deployment	Actions	Updated
hyperdx-oss	Ready	Preview	Nov 3, 2026 3:55pm UTC
hyperdx-storybook	Ready	Preview	Nov 3, 2026 3:55pm UTC

[changeset-bot]

🦋 Changeset detected

Latest commit: bee902c

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	Minor
@hyperdx/app	Minor
@hyperdx/otel-collector	Minor

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]

🟡 Tier 3 — Standard

Introduces new logic, modifies core functionality, or touches areas with non-trivial risk.

Why this tier:

• Diff size: 497 production lines changed (Tier 2 max: < 250)
• Touches API routes or data models — hidden complexity risk

Review process: Full human review — logic, architecture, edge cases.
SLA: First-pass feedback within 1 business day.

Stats

• Production files changed: 12
• Production lines changed: 497 (+ 342 in test files, excluded from tier calculation)
• Branch: claude/hdx-5302-mcp-promql
• Author: karl-power

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

[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 114 in total (1.40%).
Run yarn dupes locally to reproduce, or yarn dupes:all for the full picture.

[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.

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

3 posted as inline comment(s) on the changed lines. 1 listed below.

Findings outside the changed lines

1 minor

• 🔵 packages/api/src/mcp/__tests__/promqlSources.int.test.ts:566 — global.fetch mock is never reset; assertions read calls accumulated across tests and a stubbed rejection leaks forward → The packages/api Jest config sets no clearMocks/resetMocks. The test asserts that mockFetch.mock.calls are exactly two URLs and reads urls[0], so it breaks if any earlier test in the file calls fetch. mockRejectedValue in the next test also persists for every later test. Add mockFetch.mockReset() in beforeEach, as routers/api/__tests__/prometheus.int.test.ts:77 does.

---

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

[greptile-apps]

Confidence Score: 5/5

[Medium impact] The PR appears safe to merge; both previous findings are fixed and no new blocking issue was found.

Summary

Adds PromQL source descriptions to MCP and hides PromQL sources when the feature is disabled.

• MCP descriptions show the labels and metric names in a PromQL source.
• Source lists show where PromQL data lives and how to query it.
• MCP keeps PromQL sources out of workflows when PromQL is disabled.
• Prometheus requests send connection credentials in a Basic auth header.

_{Reviews (2) · Last reviewed commit: "fix: Send Prometheus host credentials as..." · Reviewed by Greptile}

[github-actions]

Deep Review

✅ No critical issues found.

No P0/P1 ship-blockers. Credential handling and the PromQL visibility gate are implemented carefully and backed by integration tests: credentials are stripped from the URL into a Basic auth header before fetch, error strings use a userinfo-free redactedTarget, and isHiddenSource is applied on every MCP path (list, describe, delete, create-dashboard prompt) with save_source dropping the promql kind when PromQL is disabled. All three prior review findings (authenticated-description failure, label-query cancellation, describe timeout) are verified addressed in the current code.

🟡 P2 -- recommended

• packages/api/src/controllers/promql.ts:55 -- takeBasicAuthHeader runs decodeURIComponent on url.username/url.password, which throws URIError when a credential contains a bare % (the WHATWG URL parser leaves it un-encoded in userinfo); in proxyToPrometheus the call sits outside the function's try/catch, and in fetchPrometheusData the throw is mislabeled as Failed to reach Prometheus, so a validly-configured connection silently fails auth with a confusing error.
  • Fix: wrap the decodeURIComponent calls in a fallback to the raw value, or move the takeBasicAuthHeader(url) call inside proxyToPrometheus's existing try block.
  • _{reliability, testing}
• packages/api/src/mcp/tools/sources/describePromqlSource.ts:49 -- the truncation branch (per-list .slice caps, the note field, and getPromqlLabelCatalog's limit + 1 "there is more" signal) is never exercised because every fixture stays under the 100/200 caps.
  • Fix: add a describe test with a catalog exceeding MAX_LABEL_NAMES/MAX_METRIC_NAMES asserting the lists are sliced and the note field is emitted.

🔵 P3 nitpicks (4)

• packages/api/src/mcp/tools/types.ts:22 -- context.promqlEnabled is only ever set in tests; app.ts builds McpContext without it, so production always resolves through the ?? config.IS_PROMQL_ENABLED fallback and the per-context override is a dead path.
  • Fix: set promqlEnabled where McpContext is built in app.ts, or drop the field and read config directly while injecting config in tests.
• packages/api/src/mcp/tools/sources/listSources.ts:240 -- PromQL backend is computed two different ways: listSources prefetches a Set of Prometheus-endpoint connection IDs while describePromqlSource reads it from getPromqlLabelCatalog, so the two can drift.
  • Fix: extract a single shared helper that resolves a PromQL source's backend and reuse it in both tools.
• packages/api/src/mcp/tools/sources/describePromqlSource.ts:40 -- limit = Math.max(MAX_LABEL_NAMES, MAX_METRIC_NAMES) overloads one number for two independent caps (over-fetching label names ~2x) with the "is-truncated" logic split across two files.
  • Fix: pass the two caps explicitly, or return a per-list hasMore flag from getPromqlLabelCatalog.
• packages/api/src/mcp/tools/sources/schemas.ts:22 -- NON_PROMQL_SOURCE_KINDS has no compile-time link to SOURCE_KINDS/SourceKind (unlike the asserted SOURCE_KINDS), so a future source kind would silently be rejected by save_source whenever PromQL is disabled.
  • Fix: derive it from Exclude<(typeof SOURCE_KINDS)[number], 'promql'> or add a mirror of the _assertSourceKindsMatchEnum assertion.

---

Reviewers (6): previous-comments, security, testing, maintainability, reliability, kieran-typescript. (correctness and adversarial were dispatched but did not return before synthesis; their focus area -- the decodeURIComponent throw path -- was independently analyzed and corroborated by reliability.)

Testing gaps:

• No direct unit test for controllers/promql.ts helpers (takeBasicAuthHeader with percent-encoded or malformed credentials, getPromqlLabelCatalog, fetchPrometheusData); integration tests only use clean credentials and small fixtures.
• abort_signal threading into the ClickHouse DISTINCT label query is unverified -- describeSourceTimeout.test.ts mocks getPromqlLabelCatalog wholesale.
• The create-dashboard prompt's new isHiddenSource filter and buildMcpSaveSourceSchema(false) stripping the promql kind are not asserted at the unit level.

_{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 • 459 passed • 1 skipped • 1698s

Status	Count
✅ Passed	459
❌ Failed	0
⚠️ Flaky	3
⏭️ Skipped	1

Tests ran across 4 shards in parallel.

View full report →

[knudtty]

I expect we might have to tune this on another iteration. For some set with hundreds of thousands of series, the values could be tens to hundreds of megabytes.

[karl-power]

Good call, yes. I'll add a PR to cap this somehow at the end of the stack

[karl-power]

Addressed here:
#3541

[knudtty]

Do you think it makes sense to reuse the new fetchPrometheusData function in prometheus queries like this?

[karl-power]

fetchPrometheusData buffers and parses the whole body and throws on errors, which suits the MCP here. But the router streams responses and passes the status and error body through, so swapping it in would change that behaviour. I have a PR (3rd in the stack) almost ready where the backend choice is moved into controllers/promql.ts. I'll refactor a bit to dedupe some of the request building and move into a shared helper.