[Atom-oh]

Summary

Part 3 of 3, stacked on #184 (fix/custom-mcp-pinned-transport), which is stacked on #183. Merge last and retarget to dev once #184 merges. This PR holds the web/UI contract and all documentation for the three parts, so every claim is backed by code below it in the stack.

• The datasource list reports instance-specific configuration evidence only (configured, connectionStatus, scope: 'shared'), never a borrowed kind default or a connectivity claim. A Datadog instance's legacy apiKey/appKey pair counts as configured and survives a settings-only PATCH regardless of the row's authType.
• The datasource form separates the server execution limit (Prometheus/Mimir/ClickHouse) from the HTTP response limit (other kinds) and shows the effective limit (55s maximum) — both are enforced by the fix(datasources): bound connector reads and verify real query access #183 connectors. Mimir apiPrefix is validated, persisted and read by the connector.
• Explore passes the selected time window to every kind, rejects unsupported SQL range shapes, and resets the window on instance switch so a 30d window never lands on a 7d kind.
• The test route returns {ok, latencyMs, verified, error?} from the real-query probe. oauth_3lo is rejected at registration and enablement and the form no longer offers it; a historical row fails closed at connect.
• Docs: new docs/reference/external-connections.md (indexed), docs/api-reference.md datasource rows, the datasource guides in ko/en/ja/zh (both the observability and monitoring pages — the latter had still described the old 10s/12s behavior), and the CHANGELOG datasource entries (EN/KO, amended in place).

Review follow-ups (round 1)

The MAJORs about unreachable 55s / unread timeoutS / dead apiPrefix / missing verified / undisclosed truncation were all "connector code lives in another PR" — resolved by stacking on #183, which implements each one. Remaining items fixed here: the monitoring/datasources.md guide (4 locales) and api-reference.md were stale; Datadog legacy keys for the badge and the prune; oauth_3lo removed from the dropdown; CHANGELOG wording now matches the stacked code (it no longer claims anything the stack does not do). The ADR-007 custom-MCP finding was rejected by the chair and is settled by #177.

Tests

• bash scripts/v2/merge-verify.sh on Python 3.12 on this branch (full stack): web vitest PASS (5451 passed), Terraform fmt PASS, deployment Node tests PASS; only the codec-sandbox tests fail for the known environmental reason (same on dev).
• npx tsc --noEmit -p . → 96 errors, identical to the dev baseline (existing test files, none in this diff).

🤖 Generated with Claude Code

[github-actions]

🤖 AI Code Review (two independent reviewers)

_Reviewer responses: codex/ALL claude/ALL _

Status: BLOCKED — CRITICAL/MAJOR issues must be resolved before merge

All verification is complete. Both panels declared image coverage NOT_REQUIRED and the manifest is empty and complete, so no pixel evidence is needed. Here is the synthesized chair review.

---

Chair Review — PR #185: fix(web): show datasource completeness, limits and shared scope

1. Summary

The web-tier hardening in this PR is genuinely good — the test-route credential-spread override fix, the malformed-id guard, the honest configured-vs-connected split, and the explicit window forwarding are all correct and well-tested. However, the PR advertises connector (Lambda) behavior that does not exist in the resulting tree: the connectors in agent/lambda/ are untouched by this diff, so the 55s timeout ceiling, the per-kind HTTP response limit, the Mimir apiPrefix, the verified query-access probe, and the Datadog/Dynatrace/Jaeger partial-result disclosures are all claimed by the UI/CHANGELOG but unimplemented at HEAD. The published datasource guide is also left contradicting the new code.

Stacked-PR note: the CHANGELOG says "Deploy the Lambda producers before the dependent Web changes," implying producer commits elsewhere. The producers live in this tree (agent/lambda/), the diff does not change them, and BASE does not contain the required support (verified directly: HTTP_TIMEOUT = 12, zero readers of apiPrefix, timeoutS consumed only by clickhouse_mcp.py, no health tool emits verified). Judged against resulting code (BASE + patch), these gaps are real.

2. Issues per lens

L2 — Code correctness

• MAJOR (claude flagged; chair-confirmed) — The 55s Prometheus/Mimir ceiling is unreachable. web/app/api/datasources/query/route.ts now sends timeout = min(timeoutS, 55)s as the Prometheus API param, but both connectors issue the query with the default 12s HTTP read deadline (agent/lambda/datasource_http.py:43, prometheus_mcp.py/mimir_mcp.py pass http_timeout only on 3s metadata probes). A saved timeoutS: 35 instance running a >12s query gets a socket timeout and a 502 at 12s, while the form displays "적용 제한: 35s" and metadata.effectiveTimeoutS: 35. The updated test asserting '55s' locks in the unreachable value. BASE's 10s cap existed precisely to stay under the 12s HTTP deadline.
• MAJOR (claude flagged; chair-confirmed) — timeoutS forwarded for every kind, read only by ClickHouse. resolveConnConfig now spreads timeoutS unconditionally, but the only connector consumer is clickhouse_mcp.py:132. Loki/Tempo/Jaeger/Dynatrace/Datadog ignore it and use the fixed 12s deadline — yet the form labels it "HTTP 응답 제한" and renders an "적용 제한: Ns" value that nothing enforces.
• MAJOR (claude flagged; chair-confirmed) — settings.apiPrefix is dead config. Validated, persisted, forwarded, and given a form field — but mimir_mcp.py:21 hardcodes BASE = "/prometheus/api/v1" and no file in the tree reads apiPrefix (grep: zero hits). A root-mounted Mimir follows the form's "leave empty" instruction, stores the empty prefix, and every query still hits /prometheus/api/v1/* → 404; the health test only hits /ready, so Test passes while Explore fails.
• MINOR (both panels, independently — agreement) — Legacy Datadog apiKey/appKey blobs read as unconfigured. getConfiguredDatasourceIds only inspects the generic header fields, while manage/route.ts deliberately preserves these keys — the same instance the manage route protects shows "설정 미확인".
• MINOR (claude) — The manage route's auth-residue prune deletes legacy keys on unrelated PATCHes. KEEP_BY_AUTH lists apiKey/appKey under custom_header only; a settings-only PATCH on a none/bearer Datadog row silently deletes both. Also the final blob prune at manage/route.ts:219 still iterates CRED_KEYS (unreachable today, but asymmetric).
• MINOR (codex; chair-confirmed) — Provider switch can retain an out-of-bound 30d window. rangeWindow persists across datasource selection; switching from Datadog (30d) to Tempo/Jaeger keeps 2592000, which the server now rejects (maxWindow 604800) until the user picks another preset.
• MINOR (claude) — The Mimir "silent settings drop" 400 guard can never fire (settingsPayload() unconditionally emits apiPrefix for mimir); Test-button gating omits databaseInvalid; range: 0/null now 400 where they previously fell through (no in-repo caller affected); the integrations enable path gains an unhandled listIntegrations() DB read.

L3 — Security / AWS mutation safety

Both panels agree: no blocking security finding. ADR-005 boundary intact — only _query/_search/_health tools reachable, no Terraform/IAM surface touched, no hardcoded secrets (fixtures are synthetic and tests assert non-leakage). Three real fixes worth crediting: (1) the test route's creds spread can no longer override the SSRF-validated endpoint/authType (the 169.254.169.254 override case is pinned by a test); (2) a malformed id can no longer silently borrow the default instance's kind-mirror credential; (3) the connected: true-by-default inference is removed.

• MINOR (claude; chair-confirmed) — Status asymmetry on the agent/worker path. load_datasource (datasource_http.py:228-241) still falls back to the kind mirror, so a mirror-only default works for agents while the hub badge says "설정 미확인" — a false-negative the new caption doesn't cover.
• MINOR (claude) — oauth_3lo rejection is half-wired. INTEGRATION_TRANSPORTS still exports it, the migration CHECK still permits it, customization/page.tsx still offers it as a dropdown that now always 400s, and already-enabled rows stay live (only enable is blocked).

L4 — Observability / data-integration correctness

No Steampipe/CIS/AgentCore-diagnosis logic is touched; findings concern the datasource layer.

• MAJOR (claude flagged; chair-confirmed) — verified is a pass-through for data no connector produces. No *_health tool emits a verified field; datadog_health (datadog_mcp.py:103) is an API-key validity probe (GET /api/v1/validate), not a metric-query probe. verified is therefore always [], and the advertised "connection tests validate the actual query operation" is false at HEAD.
• MAJOR (claude flagged; chair-confirmed) — Partial results stay undisclosed exactly where this PR makes them common. Datadog truncates each series at pointlist[:1500] but sets truncated only on series count (datadog_mcp.py:89-91); Dynatrace truncates nested data[:50] undisclosed (dynatrace_mcp.py:95-99); Jaeger's default limit=20 can never trip truncated: len > 50 (jaeger_mcp.py:90-116). This PR newly grants Datadog/Dynatrace 30d windows and Jaeger the full range selector, making silently-capped results the normal case (30d at 1-min resolution is ~43k points → 1500 shown as complete). The web normalizer is ready for the flags; the producers never set them — the new datasource-render.test.ts fixture supplies truncated: true itself, so it is a regression guard, not end-to-end evidence. Codex found no L4 issue here (disagreement); the connector sources confirm claude.
• MINOR (claude) — step and the density cap are decorative for the four search kinds (validated but never sent when spec.range is absent), and metadata.window reports requested, not honored, bounds.
• Correct (chair-verified): the newly forwarded second-granularity start/end round-trip properly through each connector's time parser, and the explicit 1h default matches the connectors' previous implicit default — range: true behavior is preserved.

L5 — Docs / ADR consistency

• MAJOR (claude flagged; chair-confirmed) — The published guide contradicts the new code and is not updated. docs-site/docs/monitoring/datasources.md states in two places that Prometheus/Mimir cap at 10s under the 12s connector HTTP timeout and that the other five kinds' timeout is "저장만 되고 현재 적용되지 않음" — the guide is actually right about the connectors and now contradicts the form's claims; no locale is touched.
• MAJOR (claude flagged; chair-confirmed) — CHANGELOG overclaims unimplemented connector behavior. "Mimir's API prefix is configurable" (nothing reads it), "upstream success/error bytes are bounded before parsing" (_parse(resp.read()) is an unbounded read; only the non-JSON fallback is capped at 4000 chars), "Query-access tests validate the actual operation", "Datadog point caps and Dynatrace nested series caps … remain visibly partial" — all absent from BASE and the patch. "Missing explicit instance credentials fail closed" is implemented only for the list badge, not the query path.
• REJECTED (codex MAJOR — not adopted into the gate): codex claims the CHANGELOG's "Custom MCP retains admin-managed extensibility" contradicts ADR-007/ADR-017, asserting those ADRs permit only curated presets and "explicitly drop arbitrary custom MCP." The ADR texts live in the private upstream repo and cannot be read here; the in-tree evidence contradicts the claim: custom_mcp is a registered kind in BASE's INTEGRATION_KINDS_EGRESS (the declared source of truth, shared with the migration CHECK), the PUT route's enable/disable is commented "custom-only at the SQL level," and the pinning/no-redirect/Host-SNI properties the CHANGELOG cites exist in datasource_http.py. docs/reference/05-agentcore.md describes ADR-007/017 as gating vendor-hosted presets, not prohibiting custom MCP. Unverifiable at best, contradicted by BASE at worst — excluded.
• MINOR (claude): docs/api-reference.md lacks apiPrefix and the new configured/connectionStatus/scope fields, and its "clickhouse 유효 최대 55" qualifier is now wrong; gap-audit entries (docs/v1-gap-audit-2026-07-19.md:86,279) are stale; the oauth_3lo runtime limitation has no doc record while the "source of truth" constant still includes it; three topology-flavored i18n strings land with no call site.
• CHANGELOG conventions (CLAUDE.md): compliant — entries amended in place, no PR/round numbers, English/Korean 1:1.

3. Suggestions

1. Either land the connector-side changes in this PR (extend HTTP_TIMEOUT above the configured bound for Prometheus/Mimir, honor timeoutS as a transport deadline in the other five connectors, read apiPrefix in mimir_mcp.py, emit verified from the health tools, set truncated on Datadog point / Dynatrace nested / Jaeger limit saturation) or strip the UI/CHANGELOG claims down to what HEAD actually does and re-introduce them with the producer PR.
2. Until the connectors honor it, keep the Prometheus/Mimir web cap at 10s and don't render "적용 제한" for kinds whose connectors ignore timeoutS.
3. Teach getConfiguredDatasourceIds and KEEP_BY_AUTH the same notion of Datadog credential material (apiKey/appKey), so the badge and the scrub agree.
4. On datasource-kind switch in ExplorePanel, clamp rangeWindow to the new kind's max preset.
5. Update docs-site/docs/monitoring/datasources.md (all locales), docs/api-reference.md, and remove oauth_3lo from the selectable transports (or document why it stays listed while rejected).

4. Verdict

Confirmed MAJORs in L2 (unreachable/false timeout limits, dead apiPrefix), L4 (unimplemented verified, undisclosed truncation made common by the widened windows), and L5 (guide and CHANGELOG contradict the resulting code). The security direction of the PR is positive, but the completeness/limits story it ships to users is not true at HEAD.

IMAGE_COVERAGE: NOT_REQUIRED

---

Triggered by commit 221d8e917bc026a4b9bb2d810d55f3e3e471512f · workflow: .github/workflows/pr-review.yml

[Atom-oh]

Status note for reviewers:

• Rebased onto the current fix(agent): pin custom MCP connections to the validated address #184 head (which sits on fix(datasources): bound connector reads and verify real query access #183 1cb5d405, i.e. on dev with fix: enforce default-off custom MCP governance #186's default-off custom MCP governance). The two fix: address review feedback commits pushed to this branch earlier today are kept.
• Conflict resolution against fix: enforce default-off custom MCP governance #186: the /api/integrations enable path keeps fix: enforce default-off custom MCP governance #186's attestation gate; this PR's enable-time oauth_3lo check was dropped because that gate only admits api_key/oauth_client_credentials (registration validation and the form still reject oauth_3lo). The reference doc states that posture.
• The docs-site/docs/monitoring/datasources.md (4 locales) correction that fix(datasources): bound connector reads and verify real query access #183's latest review still lists as its remaining MAJOR is carried here, since this PR owns the docs for the stack.
• Like fix(agent): pin custom MCP connections to the validated address #184, this PR gets no CI while its base is a feature branch (pr-review.yml/merge-verify.yml trigger on main/dev bases only); retarget to dev after fix(agent): pin custom MCP connections to the validated address #184 merges and the review re-runs on the current head.

Full web suite on this head: 5532 passed, 275 skipped.