chore(core): promote the CLI surface mcpdo shares into core/cli (#2461) by cliffhall · Pull Request #2613 · modelcontextprotocol/inspector

cliffhall · 2026-10-07T04:02:44Z

What

clients/mcpdo compiled against clients/cli/src through a temporary build-time @inspector/cli alias. This moves the shared, Node-only closure it reached for into a new core/cli/ area, with the same internal layout, and both clients now import it through the ordinary @inspector/core alias.

Moved to `core/cli/`
`error-handler.ts`	exit codes, `CliExitCodeError`, the error envelope
`cliOAuth.ts`, `cli-oauth-navigation.ts`	interactive OAuth connect flow
`style.ts`, `utils/awaitable-log.ts`	output helpers
`handlers/`	`run-method`, `method-types`, `output-file`, `format-output`, `collect-app-info`, `skills-verify`, `servers-list`, `connect-timeout`

output-file, collect-app-info and skills-verify weren't named in the issue, but run-method/method-types import them, so they're part of the same closure. One-shot-only output (emit-result, consume-outcome, schema-lint-report, servers-write, cli.ts, completion.ts) stays in clients/cli/src.

Also changed

The alias is gone from mcpdo's tsup.config.ts, vitest.config.ts, tsconfig.json and tsconfig.test.json, and noExternal drops /^@inspector\/cli/.
Coverage: core/cli/** is added to clients/cli's coverage include (with allowExternal, since it sits outside that project's root). The cli suites still exercise it, so it stays under the per-file ≥90 gate. Current numbers: core/cli 99.1/95.2/97.7/100 and core/cli/handlers 99.5/95.8/100/99.5 (stmts/branch/funcs/lines). Web's core/* whitelist leaves it out on purpose, because no web test reaches it.
Tests stay in clients/cli/__tests__/. They drive these modules through the cli runner. AGENTS.md's test-placement rule and the project-structure skill now record core/cli/ as the one exception to "core tests live in clients/web/src/test/core/".
CliExitCodeError uses plain fields instead of parameter properties. Everything under core/ is typechecked by web's tsc -b, and web sets erasableSyntaxOnly. Behavior is unchanged.
Inside core/cli/, self-imports are now relative (../mcp/…), matching the rest of core/.
Docs: AGENTS.md (structure tree, coverage note), the project-structure skill, the mcpdo README layout note, cli test docs, and the specs and code comments that pointed at the old paths.

Not in this PR

The issue's note about the TUI reusing this surface is left for a separate pass. The TUI doesn't import any of these modules today, and moving its own copies is a behavior change, not a relocation.

Verification

npm run local:gate: every stage up to the Firefox smoke passed. That covers validate and its guards, coverage on all five clients, verify:build-gate, verify:bundle-externals, and the launcher/cli/mcpdo/tui/web/chromium smokes. I ran local:storybook on its own afterwards and it passed (123 files). Two local environment issues to flag:

smoke:web:firefox couldn't launch Firefox under the sandbox this session ran in (sandbox_extension_issue_file_to_process … Operation not permitted). It's a browser-launch failure, not an app failure, and this diff doesn't touch web. CI runs that smoke.
The tui useCopyKeys "W saves…" test fails on macOS's long default $TMPDIR. That's already tracked in TUI useCopyKeys save-path test fails on macOS's long default TMPDIR #2609, and it fails the same way on unmodified v2/main. The gate passed it with TMPDIR=/tmp.

🤖 Generated with Claude Code

mcpdo compiled against clients/cli/src through a temporary build-time @inspector/cli alias. Move the shared node-only closure it reached for — error-handler, awaitable-log, style, cliOAuth, cli-oauth-navigation and the handlers run-method / method-types / output-file / format-output / collect-app-info / skills-verify / servers-list / connect-timeout — into core/cli/, keeping the same layout, and import it through the ordinary @inspector/core alias from both clients. - Drop the @inspector/cli alias from mcpdo's tsup, vitest and tsconfigs. - Keep core/cli under clients/cli's per-file coverage gate (its suites still exercise it); web's core/* whitelist deliberately omits it. - CliExitCodeError drops parameter properties: core/ is typechecked under web's erasableSyntaxOnly. - Update AGENTS.md, the project-structure skill, the mcpdo README and the specs that pointed at the old paths. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: cliffhall <cliff@futurescale.com>

Copilot

Copilot review overview

🟢 Approval recommended

The relocation, consumer imports, and coverage configuration are consistent, with no blocking defects identified.

Review effort: Balanced
Findings: 1

Open (1)

Update testing guidance for core/cli test location and coverage · New

What changed in this PR

Moves the Node-only functionality shared by the one-shot CLI and mcpdo into core/cli/, removing their dependency on CLI-internal source paths.

Changes:

Relocates shared handlers, OAuth helpers, error handling, and output utilities.
Rewires consumers and tests through @inspector/core, removing @inspector/cli configuration.
Preserves CLI coverage gating and updates ownership documentation.

File	Description
specification/v2_cli_v2.md	Updates shared-module paths and coverage documentation.
specification/v2_auth_mid_session.md	Updates OAuth source paths.
scripts/smoke-cli.mjs	Updates a comment’s source path.
core/mcp/uriTemplate.ts	Updates a comment’s handler path.
core/cli/utils/awaitable-log.ts	Relocates stream-output helpers.
core/cli/style.ts	Relocates styling helpers.
core/cli/handlers/skills-verify.ts	Uses relative core imports.
core/cli/handlers/servers-list.ts	Uses relative core imports.
core/cli/handlers/run-method.ts	Uses relative core imports.
core/cli/handlers/output-file.ts	Uses relative core imports.
core/cli/handlers/method-types.ts	Uses relative core type imports.
core/cli/handlers/format-output.ts	Relocates result formatting.
core/cli/handlers/connect-timeout.ts	Uses relative core imports.
core/cli/handlers/collect-app-info.ts	Uses relative core imports.
core/cli/error-handler.ts	Relocates errors; uses explicit class fields.
core/cli/cliOAuth.ts	Relocates shared OAuth flow.
core/cli/cli-oauth-navigation.ts	Relocates OAuth navigation.
core/auth/node/runner-interactive-oauth.ts	Updates a comment’s error-handler path.
clients/mcpdo/vitest.config.ts	Removes the temporary test alias.
clients/mcpdo/tsup.config.ts	Removes the temporary build alias.
clients/mcpdo/tsconfig.test.json	Removes the temporary test path mapping.
clients/mcpdo/tsconfig.json	Removes the temporary source path mapping.
clients/mcpdo/src/mcp-bin.ts	Imports shared error handling.
clients/mcpdo/src/daemon/stream-client.ts	Imports shared errors.
clients/mcpdo/src/daemon/server.ts	Imports shared handlers and errors.
clients/mcpdo/src/daemon/protocol.ts	Imports shared method types.
clients/mcpdo/src/daemon/ensure.ts	Imports shared errors.
clients/mcpdo/src/daemon/elicitation-park.ts	Imports shared errors.
clients/mcpdo/src/daemon/connections.ts	Imports shared errors.
clients/mcpdo/src/daemon/client.ts	Imports shared errors.
clients/mcpdo/src/daemon/auth.ts	Imports shared errors.
clients/mcpdo/src/connection/stored-auth.ts	Imports shared errors.
clients/mcpdo/src/connection/mcp.ts	Imports the shared CLI surface.
clients/mcpdo/src/connection/format-human.ts	Imports shared styling.
clients/mcpdo/src/connection/format-connection.ts	Imports shared output helpers and types.
clients/mcpdo/src/connection/form-prompt.ts	Imports the shared style type.
clients/mcpdo/src/connection/ema.ts	Imports shared OAuth navigation and errors.
clients/mcpdo/src/connection/elicitation-prompt.ts	Imports the shared style type.
clients/mcpdo/src/connection/dispatch.ts	Imports shared types and styling.
clients/mcpdo/src/connection/authorize.ts	Imports shared OAuth helpers.
clients/mcpdo/src/connection/auth-names.ts	Imports shared catalog types.
clients/mcpdo/src/connection/auth-helper.ts	Imports shared errors.
clients/mcpdo/README.md	Documents shared core ownership.
clients/mcpdo/__tests__/mcp-auth-coverage.test.ts	Updates error imports.
clients/mcpdo/__tests__/helpers/mcp-runner.ts	Updates error-formatter imports.
clients/mcpdo/__tests__/format-connection.test.ts	Updates error and styling imports.
clients/mcpdo/__tests__/form-prompt.test.ts	Updates styling imports.
clients/mcpdo/__tests__/ema.test.ts	Updates error imports.
clients/mcpdo/__tests__/ema-commands.test.ts	Updates styling imports.
clients/mcpdo/__tests__/elicitation-prompt.test.ts	Updates styling imports.
clients/mcpdo/__tests__/daemon-stream.test.ts	Updates error imports.
clients/mcpdo/__tests__/daemon-rpc-abort.test.ts	Retargets the handler mock.
clients/mcpdo/__tests__/daemon-private.test.ts	Updates error imports.
clients/mcpdo/__tests__/daemon-paths.test.ts	Updates formatter imports.
clients/mcpdo/__tests__/daemon-elicitation-park.test.ts	Retargets the handler mock.
clients/mcpdo/__tests__/daemon-coverage.test.ts	Updates error imports.
clients/mcpdo/__tests__/daemon-connections.test.ts	Updates error imports.
clients/mcpdo/__tests__/connection-stored-auth.test.ts	Updates error imports.
clients/mcpdo/__tests__/authorize.test.ts	Retargets OAuth mocks.
clients/mcpdo/__tests__/auth-names.test.ts	Updates catalog type imports.
clients/mcpdo/__tests__/auth-names-commands.test.ts	Updates imports and catalog mocks.
clients/cli/vitest.config.ts	Gates coverage for relocated modules.
clients/cli/src/index.ts	Imports shared error handling.
clients/cli/src/handlers/schema-lint-report.ts	Imports shared output helpers and types.
clients/cli/src/handlers/emit-result.ts	Imports shared result-writing dependencies.
clients/cli/src/handlers/consume-outcome.ts	Imports shared output and error helpers.
clients/cli/src/completion.ts	Imports shared methods and logging.
clients/cli/src/cli.ts	Consumes and re-exports relocated modules.
clients/cli/__tests__/style.test.ts	Targets relocated styling helpers.
clients/cli/__tests__/skills-verify-cli.test.ts	Updates exit-code imports.
clients/cli/__tests__/servers-list.test.ts	Targets relocated catalog helpers.
clients/cli/__tests__/schema-lint-report.test.ts	Updates error imports.
clients/cli/__tests__/run-method.test.ts	Targets the relocated handler.
clients/cli/__tests__/run-method-skills.test.ts	Targets relocated skill handlers.
clients/cli/__tests__/run-method-mocks.test.ts	Targets the relocated handler.
clients/cli/__tests__/README.md	Documents shared-module coverage.
clients/cli/__tests__/output-file.test.ts	Targets relocated file-output helpers.
clients/cli/__tests__/oauth-interactive.test.ts	Targets relocated OAuth helpers.
clients/cli/__tests__/method-types.test.ts	Targets relocated method definitions.
clients/cli/__tests__/helpers/oauth-test-fakes.ts	Updates OAuth type imports.
clients/cli/__tests__/helpers/cli-runner.ts	Updates imports and coverage comments.
clients/cli/__tests__/format-output.test.ts	Targets relocated formatting.
clients/cli/__tests__/error-handler.test.ts	Targets relocated error handling.
clients/cli/__tests__/emit-result.test.ts	Updates error imports.
clients/cli/__tests__/consume-outcome-stream.test.ts	Updates outcome type imports.
clients/cli/__tests__/completion.test.ts	Updates method-definition imports.
clients/cli/__tests__/cliOAuth.test.ts	Targets relocated OAuth helpers.
clients/cli/__tests__/cli-oauth-navigation.test.ts	Targets relocated OAuth navigation.
AGENTS.md	Documents ownership and test-placement rules.
.claude/skills/project-structure/SKILL.md	Documents the shared surface and coverage ownership.

…ion (#2461) Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: cliffhall <cliff@futurescale.com>

cliffhall · 2026-10-07T04:08:52Z

Copilot round 1: one low-severity finding. .claude/skills/testing/SKILL.md didn't mention the core/cli/ test-placement and coverage exception. Fixed in 6197966 and answered in the thread. The suppressed block was empty. Requesting round 2.

Copilot

Copilot review overview

🟢 Approval recommended

The relocation consistently updates consumers, aliases, and coverage ownership, with no unresolved blocking defects identified.

Review effort: Balanced
Findings: None

Resolved since last review (1)

Update testing guidance for core/cli test location and coverage

cliffhall · 2026-10-07T04:13:44Z

Copilot review loop closed. Round 2 came back clean: no findings, no inline comments, and nothing suppressed. The one finding from round 1 shows as resolved. The loop stops on the first clean round.

cliffhall · 2026-10-07T04:36:35Z

✅ Verdict: Mergeable

I smoke tested head 6197966d against real test servers by driving both the one-shot CLI and mcpdo across every surface the moved core/cli/ modules serve. Everything behaved correctly. No regressions, and nothing blocks the merge. CI (build, coverage) is green, merge state is CLEAN, and both commits are signed off.

Automated (fresh detached worktree, clean `npm install` + `npm run build`)

Check	Result
`smoke:cli`	✅ pass
`smoke:mcpdo`	✅ pass (connect → calls → disconnect → daemon stop, socket released)
`verify:bundle-externals`	✅ 4 bundles, nothing externalized was inlined
`clients/cli` `check` + `test:coverage`	✅ 581 passed / 2 skipped; `core/cli` 99.1/95.2/97.7/100, `core/cli/handlers` 99.5/95.8/100/99.5. Every file is ≥90 on all four dimensions
`clients/mcpdo` `check` + `test:coverage`	✅ 490 passed
Stale references	✅ none: no `@inspector/cli` anywhere, no `core/` → `clients/` import, no doc still pointing at the old `clients/cli/src/...` paths
Rename fidelity	✅ all 13 moves are pure relocations. The content diffs are only `@inspector/core/...` → relative imports, plus the documented `CliExitCodeError` parameter-property → field change

Manual: one-shot CLI (`clients/cli/build`, and `mcp-inspector --cli` via the launcher)

stdio and streamable-HTTP: initialize, tools/list (auto-paginates 12 tools), tools/call via --tool-arg and --tool-args-json, resources/list|read, resources/templates/list, an RFC 6570 template read with a percent-encoded reserved char, prompts/list|get, structured output, and logging/setLevel (legacy). The modern era correctly rejects setLevel.
Error envelope / exit codes (error-handler): tool_is_error → 5, tool_not_found → 5, no_app → 2, auth_required → 3, unreachable → 4 (refused, and a blackholed --connect-timeout 1500 cut off at ~1.6s), --strict → 6, skills --verify → 7. Usage errors (unknown method/server, --tool-arg + --tool-args-json, --strict with the wrong method, raw with tools/list) → 1.
output-file: --output json and raw (tool text and resource text); a missing parent dir → output_write_failed.
collect-app-info: --app-info on tools/list (NDJSON) and on a single tool. skills-verify: per-skill reports, and the summary on the skills showcase.
servers-list, --completion zsh, --print-handoff, --list-stored-auth.
Interactive OAuth (cliOAuth + cli-oauth-navigation), driven through a pty with auto-open disabled: DCR → consent → loopback callback → Authorization complete. → the call succeeds. A non-TTY run is correctly refused. --stored-auth-only reuses the persisted token afterwards, and --relogin clears it.

Manual: `mcpdo` (private token-gated daemon)

servers/list|show, then connect of 11 entries (stdio + HTTP, legacy + modern, plus an ad-hoc URL with --era legacy), connections/list|show|use, @name and --conn addressing, and the non-TTY connection_required guard.
tools/list|call (key:=value and a JSON object), with the same error codes and exit codes as the CLI (the shared error-handler). Also resources/*, templates, prompts/get|complete, structured output, --app-info (exit 2 on no_app), logging/setLevel, and logging/tail receiving a send_notification line.
Tasks + elicitation: a modern task with --task completes. modern_input_task parks elicitationPending (form), elicitation/respond … approved:=true completes it, and a stale ID → elicitation_not_found. URL-mode elicitation over stdio → form submitted → --done → Collected value: smoke-value.
roots/set|list, skills/list|get.
OAuth: connect oauth exits 0 with pendingAuth + authUrl. A call while pending → auth_required (3) with the re-show hint. After approval, connections/show completes the sign-in and calls succeed. auth/list shows it ● live, and the one-shot CLI reuses the token mcpdo stored. auth/clear, disconnect (plus the unknown-connection error), and daemon stop all work.

Not exercised

The step-up [y/N] confirmer. oauth-step-up-demo.json's first grant already requests every supported scope, so get_temp never needs step-up. cliOAuth.test.ts covers it, and that suite passes.
EMA (auth/ema-*): it needs an IdP fixture.

Observations (none caused by this PR)

Each of these now has a follow-up issue, linked under the item.

The DCO check isn't running on any v2 PR. dco.yml was added in ci: replace the suspended DCO app with a signoff check we own #2603 and triggers on pull_request_target. GitHub now sources those workflows from the default branch, and dco.yml isn't on main yet. Its only recorded run is a pull_request event on ci: replace the suspended DCO app with a signoff check we own #2603's own branch (v2/chore/2566-dco-signoff-check). No PR since then has a DCO check, including ci: replace the suspended DCO app with a signoff check we own #2603 itself at merge, test(web): subscribe the fetch log before connecting in timeout-diagnostics (#2580) #2612, and this one. It should start working after the next milestone merge into main. Until then the required-check guarantee isn't in force. Both commits here are signed off.

Follow-up: ci: run the DCO check on PRs into v2/main now, plus a post-merge backstop on v2/main #2616, fixed by PR ci: run the DCO check on every v2 PR now, before push, and after merge (#2616) #2619 (the check now runs on every v2 PR, before push, and after merge). The 10 historic unsigned commits it surfaced are tracked in docs: record retroactive DCO signoffs for the 10 unsigned commits on v2/main #2617 (retroactive signoff record, PR docs: retroactive DCO signoffs for the 10 unsigned commits on v2/main (#2617) #2618).
Not from this PR: -q still prints the Wrote N bytes … to <path> status line on stderr when --output is used. The help for -q says status lines are suppressed. output-file.ts moved unchanged, so this predates the PR.

Follow-up: CLI: -q does not suppress the "Wrote N bytes … to <path>" status line when --output is used #2614.
Not from this PR: the CLI's connect-timeout message says to raise the timeout "in Server Settings", which is web UI wording. --connect-timeout is the CLI's knob.

Follow-up: CLI/mcpdo: connect-timeout error tells the user to change "Server Settings", which only the web UI has #2615.

🤖 Generated with Claude Code

cliffhall added the v2 Issues and PRs for v2 label Oct 7, 2026

cliffhall linked an issue Oct 7, 2026 that may be closed by this pull request

chore(mcpdo): replace the temporary @inspector/cli source alias with a real shared surface #2461

Closed

cliffhall requested a balanced review from Copilot October 7, 2026 04:02

Copilot started reviewing on behalf of cliffhall October 7, 2026 04:03 View session

Copilot AI reviewed Oct 7, 2026

View reviewed changes

Comment thread AGENTS.md

docs(testing): record the core/cli test-placement and coverage except…

6197966

…ion (#2461) Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: cliffhall <cliff@futurescale.com>

cliffhall requested a balanced review from Copilot October 7, 2026 04:08

Copilot started reviewing on behalf of cliffhall October 7, 2026 04:09 View session

Copilot AI reviewed Oct 7, 2026

View reviewed changes

cliffhall added 2 commits October 7, 2026 01:47

Merge branch 'v2/main' into v2/chore/2461-shared-cli-surface

ba63ed8

Merge branch 'v2/main' into v2/chore/2461-shared-cli-surface

aa46c2e

cliffhall merged commit 3f1048b into v2/main Oct 7, 2026
7 checks passed

cliffhall deleted the v2/chore/2461-shared-cli-surface branch October 7, 2026 14:35

Navigation Menu

Repository navigation

chore(core): promote the CLI surface mcpdo shares into core/cli (#2461) - #2613

chore(core): promote the CLI surface mcpdo shares into core/cli (#2461)#2613
cliffhall merged 4 commits into
v2/mainfrom
v2/chore/2461-shared-cli-surface

cliffhall commented Oct 7, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

cliffhall commented Oct 7, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

cliffhall commented Oct 7, 2026

Uh oh!

cliffhall commented Oct 7, 2026 •

edited

Loading

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

cliffhall commented Oct 7, 2026

What

Also changed

Not in this PR

Verification

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot review overview

🟢 Approval recommended

Uh oh!

Uh oh!

cliffhall commented Oct 7, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot review overview

🟢 Approval recommended

Uh oh!

cliffhall commented Oct 7, 2026

Uh oh!

cliffhall commented Oct 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

✅ Verdict: Mergeable

Automated (fresh detached worktree, clean npm install + npm run build)

Manual: one-shot CLI (clients/cli/build, and mcp-inspector --cli via the launcher)

Manual: mcpdo (private token-gated daemon)

Not exercised

Observations (none caused by this PR)

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

cliffhall commented Oct 7, 2026 •

edited

Loading

Automated (fresh detached worktree, clean `npm install` + `npm run build`)

Manual: one-shot CLI (`clients/cli/build`, and `mcp-inspector --cli` via the launcher)

Manual: `mcpdo` (private token-gated daemon)