Skip to content

PDX-474/475: feat(mcp): add depth guard and token attribution middleware#169

Merged
mrdailey99 merged 4 commits into
developfrom
feature/pdx-474-475-middleware
May 14, 2026
Merged

PDX-474/475: feat(mcp): add depth guard and token attribution middleware#169
mrdailey99 merged 4 commits into
developfrom
feature/pdx-474-475-middleware

Conversation

@mrdailey99
Copy link
Copy Markdown
Collaborator

@mrdailey99 mrdailey99 commented May 13, 2026

Summary

  • PDX-474PROVAR_MCP_MAX_TOOL_DEPTH env var: per-session agentic loop depth guard. Caps tool calls at N (default 50) per MCP session; returns TOOL_BUDGET_EXCEEDED with callsMade/limit/suggestion fields once the limit is hit. provardx_ping is exempt.
  • PDX-475PROVAR_MCP_EMIT_TOKEN_META env var: per-call token attribution. When true, appends a _meta block to structuredContent with tool, detailLevel, and estimatedTokens; budget-exceeded errors also include sessionTotalEstimatedTokens.
  • New src/mcp/utils/tokenMeta.ts with wrapWithDepthGuard, attachMeta, estimateTokens, and createDepthGuardState.
  • patchWithMiddleware in server.ts monkey-patches registerTool after provardx_ping registration so all Provar tools are wrapped but ping is exempt.
  • docs/mcp.md — new Performance Tuning section with env var reference.

Implementation notes

  • patchWithMiddleware casts through unknown (not any) to avoid @typescript-eslint/no-unsafe-member-access; the runtime overload shape is safe because McpServer.registerTool always accepts (name, config, handler).
  • Session IDs without a sessionId from the MCP SDK get a unique anon-${randomUUID()} per call, so anonymous calls never share state.
  • The in-process session map is capped at 1000 entries with LRU-style eviction of the oldest key.

Test plan

  • 24 unit + integration tests in test/unit/mcp/tokenMeta.test.ts — all passing
  • TypeScript --noEmit — clean
  • ESLint on changed files — zero errors
  • Verify provardx_ping is exempt: depth guard patch runs AFTER ping registration
  • PROVAR_MCP_MAX_TOOL_DEPTH=0 — every tool call returns TOOL_BUDGET_EXCEEDED immediately
  • PROVAR_MCP_EMIT_TOKEN_META=truestructuredContent._meta present; content[0].text unchanged

Merge order

This PR touches src/mcp/server.ts. Merge after Thread A (PDX-468/469/471/473) if that PR also modifies server.ts, to resolve conflicts cleanly.

🤖 Generated with Claude Code

Copilot AI review requested due to automatic review settings May 13, 2026 15:04
@github-actions
Copy link
Copy Markdown

github-actions Bot commented May 13, 2026

Quality Orchestrator

🟢 LOW · 14 / 100 · Touches: utils. All changed files have mapped tests.


🧪 Tests to Run · Running 2 of 48 tests

  • unit/mcp/server.test.ts
  • unit/mcp/tokenMeta.test.ts
▶ Run command
npx vitest run \
  unit/mcp/server.test.ts \
  unit/mcp/tokenMeta.test.ts

⚡ quality-orchestrator  ·  /qo stub <file>  ·  qo analyze-local

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds two MCP server middlewares to improve agent-loop safety and observability: a per-session tool-call depth guard (PROVAR_MCP_MAX_TOOL_DEPTH) and optional per-call token attribution metadata (PROVAR_MCP_EMIT_TOKEN_META). This is implemented by monkey-patching McpServer.registerTool after provardx_ping is registered so ping remains exempt.

Changes:

  • Introduces src/mcp/utils/tokenMeta.ts with depth-guard state, tool handler wrapping, and _meta attachment utilities.
  • Wraps MCP tool registration in src/mcp/server.ts to enforce per-session call limits (default 50) and emit token metadata when enabled.
  • Adds documentation for both env vars and a new unit/integration test suite covering depth guard + meta behavior.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

File Description
test/unit/mcp/tokenMeta.test.ts Adds unit + integration tests for depth guard and token meta attachment.
src/mcp/utils/tokenMeta.ts Implements depth-guard tracking, budget-exceeded error shaping, token estimation, and _meta attachment.
src/mcp/server.ts Patches registerTool to wrap all tools (except provardx_ping) with the depth guard middleware.
docs/mcp.md Documents performance tuning env vars and expected response/meta shapes.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/mcp/server.ts Outdated

// ── Depth-guard middleware (PDX-474) ─────────────────────────────────────────
const rawLimit = parseInt(process.env['PROVAR_MCP_MAX_TOOL_DEPTH'] ?? '50', 10);
const depthLimit = Number.isNaN(rawLimit) ? 50 : rawLimit;
Comment on lines +88 to +94
entry.calls++;
const result = await handler(args, extra);
const tokens = estimateTokens(result);
entry.totalEstimatedTokens += tokens;

const detailLevel = typeof args['detail'] === 'string' ? args['detail'] : 'standard';
return attachMeta(result, toolName, detailLevel);
mrdailey99 and others added 3 commits May 14, 2026 14:57
Req: Agent tools must be preventable from running unchecked agentic loops that exhaust context without returning to the user. Observability tooling also needs per-call token cost signals to track LLM usage across sessions.
Fix: PROVAR_MCP_MAX_TOOL_DEPTH caps tool calls per MCP session (default 50) with TOOL_BUDGET_EXCEEDED errors; PROVAR_MCP_EMIT_TOKEN_META appends a _meta token-attribution block to structuredContent (PDX-475).

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…on when meta disabled

Req: Negative values for PROVAR_MCP_MAX_TOOL_DEPTH produced a nonsensical negative limit in TOOL_BUDGET_EXCEEDED responses. Token estimation via JSON.stringify ran on every tool call even when PROVAR_MCP_EMIT_TOKEN_META was disabled, adding avoidable overhead.
Fix: Treat negative parsed values the same as NaN (fall back to 50). Guard token estimation and accumulation behind the PROVAR_MCP_EMIT_TOKEN_META=true check so it is skipped entirely when meta is disabled.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Req: Depth limit of 0 caused every tool call to fail immediately with TOOL_BUDGET_EXCEEDED because the guard condition is entry.calls >= limit.
Fix: Change the negative-value guard from rawLimit < 0 to rawLimit <= 0 so zero is treated as invalid and falls back to the default of 50.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@mrdailey99 mrdailey99 force-pushed the feature/pdx-474-475-middleware branch from 5cd1014 to 50c8cf8 Compare May 14, 2026 20:01
…tdio transports

RCA: wrapWithDepthGuard fell back to `anon-${randomUUID()}` when extra.sessionId was absent, generating a fresh sessionId for every call. Each call created a new SessionEntry with calls=0, so entry.calls >= limit never tripped on stdio clients (Claude Desktop, Cursor, etc.) that don't supply a sessionId.
Fix: Fall back to the literal string 'anon' so all sessionless callers share one bucket and the budget actually limits runaway tool use. Remove the now-unused crypto.randomUUID import. Flip the prior test ("assigns a unique anon session UUID per call when sessionId is absent") to assert the corrected behavior — second anon call returns TOOL_BUDGET_EXCEEDED — and add a sanity test that named sessions remain independent from the anon bucket.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@mrdailey99 mrdailey99 merged commit c327717 into develop May 14, 2026
4 of 12 checks passed
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