Skip to content

Backfill coverage for InspectorClient and remote-server core paths (#1307 follow-up) #1310

@cliffhall

Description

@cliffhall

Background

#1307 brought all 19 v1.5-ported tests online under a new integration vitest project (node env) and dropped the broad coverage.exclude globs for v1.5-ported source. Coverage now enforces the per-file gate (lines ≥90, statements ≥85, functions ≥80, branches ≥50) on every core module except two, which remain excluded with a TODO(#new-issue) marker pending this follow-up.

Files needing coverage backfill

core/mcp/inspectorClient.ts — 73.48% lines / 72.09% functions / 72.82% statements / 60.7% branches

2184 LOC source file with a 4005-LOC integration test (inspectorClient.test.ts) that already covers most of the happy path. The remaining 153 uncovered lines span 107 disjoint ranges scattered through the file — primarily:

  • OAuth normal-mode and guided-mode methods when oauthManager is unset (getOAuthTokens, isOAuthAuthorized, getOAuthStep, setOAuthConfig).
  • subscribeToResource / unsubscribeFromResource (entire methods uncovered).
  • Error / fall-through paths in pagination, message tracking, and OAuth flows (e.g. getServerUrl() when transport is stdio, OAuth refresh failures, transport reconnect edge cases).
  • Various small `if (!this.client) throw …` guards.

The path-of-least-resistance is probably adding focused unit tests (no real server) for the methods that are pure guard clauses, then a second pass on OAuth flow + subscribe/unsubscribe with a mocked Client.

core/mcp/remote/node/server.ts — 88.15% lines / 78.37% functions / 82.75% statements / 65.44% branches

637 LOC Hono server backing the remote transport. Existing tests cover most happy paths and several error paths added in #1307. The remaining ~27 uncovered lines are concentrated in:

  • The private createTokenAuthProvider helper's auxiliary methods (clientInformation, codeVerifier, state — only tokens() is invoked in practice; the others satisfy the OAuthClientProvider interface).
  • Transport-failure paths in /api/mcp/connect (transport throws sync during start(), markTransportDead).
  • 401 propagation in /api/mcp/send / /api/mcp/connect when the underlying transport reports a 401 via error.status.
  • forwardLogEvent fallback when the level label maps to a non-function on the logger.

These need either:

  • Realistic failure injection in the integration suite (e.g. a test MCP server that exits on startup, or an upstream that returns 401), or
  • Refactoring the private helpers to be testable from a unit test.

Reproduction

Drop the two TODO(#NNNN) exclusions in clients/web/vite.config.ts (under `coverage.exclude`) and run `npm run test:coverage` — both files will fail the per-file gate. The exclusions are the only reason validate currently passes.

Acceptance criteria

  • Both files pass the per-file gate (90/85/80/50) after running `npm run test:coverage`.
  • The two TODO entries are removed from `clients/web/vite.config.ts` (`coverage.exclude`).
  • `npm run validate` still passes.

Out of scope

Metadata

Metadata

Assignees

Labels

v2Issues and PRs for v2

Type

No type
No fields configured for issues without a type.

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions