Skip to content

feat(dashboard): plot MCP tool calls on the request timeline - #13941

Merged
diegosouzapw merged 6 commits into
diegosouzapw:release/v3.8.51from
HouMinXi:feat/timeline-mcp-audit
Sep 24, 2026
Merged

diegosouzapw merged 6 commits into
diegosouzapw:release/v3.8.51from
HouMinXi:feat/timeline-mcp-audit

Conversation

@HouMinXi

@HouMinXi HouMinXi commented Sep 17, 2026 •

Copy link
Copy Markdown
Contributor

Related to #13898. #13898 stays open.

The request timeline only plotted LLM rows from /api/usage/call-logs. MCP tool calls already live in mcp_tool_audit; they never showed on the same axis.

What this PR does

  • Maps mcp_tool_audit rows onto the timeline as kind: mcp (mcp:<id> so they do not collide with call-log ids).
  • Polls both endpoints. A failed MCP fetch keeps the last MCP bars (mcpOk); it does not wipe them.
  • Reuses the logs-grid API-key dropdown. LLM rows go through ?apiKey=; MCP rows through ?apiKeyId= only when the selected value is a known key id. A name-only filter skips the MCP fetch instead of asking for a null id.

Follow-ups already on this branch (the two product holes from review)

  1. Tunnel dashboards. GET /api/mcp/audit and /api/mcp/audit/stats are in LOCAL_ONLY_API_GET_EXEMPTIONS. POST and the rest of /api/mcp/* stay local-only. Without that, a tunnel-served dashboard 403s the poll forever.
  2. Key stamp. logToolCall no longer writes process.env.OMNIROUTE_API_KEY_ID. It uses resolveMcpCallerApiKeyId (HTTP headers first, then the env key lookup used by stdio). Selecting a dashboard key no longer drops every MCP bar.

Changelog lives in changelog.d/features/13941-mcp-timeline.md.

⚠️ base-red inherited: #13866

Maintainer rework (merge-batch 2026-09-24)

  • Merged the current release/v3.8.51 tip (clean, no conflicts).
  • Re-read the routeGuard.ts change line by line: the exemption is exact-match on /api/mcp/audit and /api/mcp/audit/stats only, GET/HEAD/OPTIONS only; /api/mcp/audit/extra, /api/mcp/sse, /api/mcp/stream and POST stay local-only (covered by the tests). Both handlers are GET-only and still gated by requireManagementAuth.
  • Added the reason for the two new entries next to the pinned-membership list in route-guard-version-get-exemption.test.ts, as that test asks for.
  • Validation on the merged head: request-timeline-mcp-audit, route-guard-version-get-exemption, mcp-audit-caller-key 43/43 pass; eslint (with the frozen suppressions) clean on touched files; typecheck:core and check:open-sse-typecheck show only the inherited base errors (cliproxyAccountHealth.ts, auggie.ts); file-size OK.

diegosouzapw added a commit to HouMinXi/OmniRoute that referenced this pull request Sep 17, 2026
@diegosouzapw

Copy link
Copy Markdown
Owner

Arrumei o changelog; duas pendências de produto ficam para o dono.

Movi a linha do CHANGELOG.md para changelog.d/features/13941-mcp-timeline.md — editar o CHANGELOG.md direto é proibido pelo CONTRIBUTING.md:409 e conflitaria na reconciliação da release (três PRs inseriam no mesmo ponto). Não toquei em mais nada.

Pendência 1 — a metade MCP é morta em dashboard via túnel. src/server/authz/routeGuard.ts:33-34 lista "/api/mcp/" em LOCAL_ONLY_API_PREFIXES, e o loopback é imposto antes do auth. Num dashboard servido por túnel, o poll bate 403 a cada intervalo, para sempre. O erro é engolido em RequestTimeline.utils.ts (.catch(() => ({ ok: false … }))) — sem warning, sem back-off, sem aviso de UI. Fica um chip "MCP" na legenda que nunca popula.

Pendência 2 — o filtro por chave é inerte. mcp_tool_audit.api_key_id vem do env de processo OMNIROUTE_API_KEY_ID (open-sse/mcp-server/audit.ts:373), que é NULL em praticamente toda instalação. Efeito: selecionar qualquer API key remove todas as barras MCP. A PR admite isso ("Not in this PR"), mas é metade do que ela anuncia, e é o bloqueio real da #13898.

Menor: ?apiKey= é substring sobre nome ou id (call-logs/route.ts:75-77) enquanto ?apiKeyId= é exato — o mesmo item do dropdown seleciona conjuntos diferentes nos dois eixos. E as 5 chaves novas do timeline entram como inglês literal nos 65 locales; passam o check-new-key-coverage (esse gate não compara valores) mas pesam no ratio de tradução.

Os testes são bons — request-timeline-mcp-audit.test.ts 21/21, comportamentais com fetch stubado.

HouMinXi pushed a commit to HouMinXi/OmniRoute that referenced this pull request Sep 18, 2026
@HouMinXi
HouMinXi force-pushed the feat/timeline-mcp-audit branch from 1a81a32 to cc7726c Compare September 18, 2026 03:13
@HouMinXi

Copy link
Copy Markdown
Contributor Author

Rebased onto release/v3.8.51 (1603c86e06) and added one commit answering the two product gaps.

Tunnel poll. GET /api/mcp/audit and GET /api/mcp/audit/stats join the existing GET exemption set (isLocalOnlyPath exact-match, GET/HEAD/OPTIONS only). SSE, stream, tools, and POST stay loopback-only. Both handlers still start with requireManagementAuth. Tests pin the exemption membership and the sibling paths that must remain blocked.

Per-key filter. logToolCall now stamps api_key_id from resolveMcpCallerApiKeyId (HTTP principal, stdio env-key lookup) instead of OMNIROUTE_API_KEY_ID. An empty string collapses to NULL so a blank header does not become its own filter bucket. Covered by three tests, including an injection that restores ?? null and watches the empty-string case go red.

CI failures on this branch still match the inherited set (API Route Typecheck, Docs Gates, Fast Quality Gates, four unit shards). Merge integrity, Vitest, and ESLint stayed green on the previous head.

HouMinXi pushed a commit to HouMinXi/OmniRoute that referenced this pull request Sep 18, 2026
@HouMinXi
HouMinXi force-pushed the feat/timeline-mcp-audit branch from cc7726c to d46d12b Compare September 18, 2026 11:09
@HouMinXi

Copy link
Copy Markdown
Contributor Author

Rebased onto current release/v3.8.51 after #14078.

Seven locale files conflicted; reviewed translations stayed, MCP timeline keys were re-applied. Tests 43/43.

HouMinXi pushed a commit to HouMinXi/OmniRoute that referenced this pull request Sep 18, 2026
@HouMinXi
HouMinXi force-pushed the feat/timeline-mcp-audit branch from d46d12b to c4613e5 Compare September 18, 2026 14:41
@HouMinXi

Copy link
Copy Markdown
Contributor Author

Rebased onto current release/v3.8.51 (36493a6270). Unique commits unchanged.

HouMinXi and others added 4 commits September 20, 2026 11:28
The request timeline only showed LLM call-logs, so MCP tool
invocations were invisible and there was no per-key slice.
Merge mcp_tool_audit as a distinct row kind and reuse the
logs-grid API-key filter. A failed MCP fetch leaves the LLM
path unchanged.

Related to diegosouzapw#13898. diegosouzapw#13898 stays open.

Signed-off-by: Minxi Hou <houminxi@gmail.com>
Name-only dropdown values must not be sent as MCP apiKeyId. A failed
LLM or MCP fetch must keep the other source's bars, and a filtered
poll must not wipe previously seen keys from the dropdown.

Signed-off-by: Minxi Hou <houminxi@gmail.com>
GET /api/mcp/audit and /stats join the existing GET exemption set so a
dashboard served through a tunnel can plot MCP bars. SSE and stream stay
loopback-only. The handlers still require management auth.

logToolCall now stamps the HTTP caller's API-key id (stdio falls back to
the env-key lookup) instead of OMNIROUTE_API_KEY_ID, which is unset on
almost every install and made the per-key filter drop every MCP bar.

Signed-off-by: Minxi Hou <houminxi@gmail.com>
@HouMinXi
HouMinXi force-pushed the feat/timeline-mcp-audit branch from c4613e5 to 8d3693c Compare September 20, 2026 15:29
@diegosouzapw

Copy link
Copy Markdown
Owner

The MCP-on-timeline mapping is a solid addition, and closing the two product gaps from
review (tunnel GET exemption + the dead per-key filter via resolveMcpCallerApiKeyId) in the
same branch is good follow-through. I confirmed on the current tip that the GET exemption for
/api/mcp/audit genuinely doesn't exist yet, so this isn't redundant work. Given the diff
touches LOCAL_ONLY_API_PREFIXES (a Hard-Rule-15/17 surface), I'd like to see a clean run of
the three touched test files right before merge as a final confirmation — not because anything
looks wrong, just because of the security weight of that file. No other blocking items from my
side.

The pinned-membership test asks every new exemption to carry its reason next
to the list; add it for /api/mcp/audit and /api/mcp/audit/stats.
@diegosouzapw
diegosouzapw merged commit 77ec7c1 into diegosouzapw:release/v3.8.51 Sep 24, 2026
9 of 16 checks passed
HouMinXi pushed a commit to HouMinXi/OmniRoute that referenced this pull request Sep 24, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants