feat(daemon-cli): mcpdo — experimental connection CLI client (#1432)

[BobDickinson]

Closes #1432

Summary

Adds clients/daemon-cli, an experimental connection CLI published as the mcpdo bin: connect to an MCP server once, then run many commands against that named connection (ssh-agent style). Connections are held by an implicit local Unix-socket daemon that mcpdo starts on demand and talks to over token-authenticated NDJSON.

mcpdo connect test-stdio --config path/to/mcp.json
mcpdo tools/list
mcpdo tools/call echo message:=hi
mcpdo --conn other tools/list      # or --connection
mcpdo logging/tail                 # long-lived stream; Ctrl-C to stop
mcpdo connections/list && mcpdo daemon status
eval "$(mcpdo private)"            # optional per-shell private daemon

• Daemon security model: per-daemon bearer token always required (generated at startup, published 0600 as daemon.token, or supplied via env for private mode), 0700 socket dirs under $TMPDIR/mcp-conn-<uid>/, socket-path length validated up front, O_EXCL pid lock with dead-pid reclaim (no takeover of a live daemon), 1 MiB NDJSON request-line cap, daemon stderr to a 0600 daemon.log.
• Output safety: terminal-bound text (results, elicitation prompts, daemon errors) is control-character sanitized; OSC 8 URIs validated; --format json stays verbatim.
• Stdio correctness: connect always sends an absolute cwd (defaults to the caller's), bare command names are resolved against the caller's PATH client-side, the default-inherited environment (PATH/HOME/SHELL…) is snapshotted from the caller's shell rather than the daemon's, and the daemon chdirs away from its spawn directory.
• Auth: shared oauth.json with the other Inspector clients; connect-time OAuth on this CLI (--relogin, --stored-auth-only); elicitation bridging for form/URL prompts (non-interactive callers — --format json or no TTY — get the elicitation parked and answer it via elicitation/respond; URL mode never auto-accepts).
• Era support: negotiates legacy/modern via core InspectorClient; --era legacy|auto|modern on connect.
• Reuses clients/cli handlers / error-handler / OAuth helpers via a temporary build-time @inspector/cli alias — chore(mcpi): replace the temporary @inspector/cli source alias with a real shared surface #2461 tracks promoting that surface to a shared area.
• Wired into monorepo validate / build / coverage / verify:bundle-externals; documented in AGENTS.md, clients/daemon-cli/README.md, and specification/v2_cli_v2.md. Adds a top-level skills/mcpdo end-user skill (teaches an agent to drive mcpdo), distinct from the .claude/skills/ repo procedures.

Packaging

Published by this PR (maintainer-approved): root bin.mcpdo → clients/daemon-cli/build/mcp-bin.js; files adds clients/daemon-cli/build and skills/mcpdo. Adds ~199 KB compressed (~770 KB unpacked, 16%) to the tarball. The daemon is inert unless mcpdo is invoked. The bin was renamed from mcpi to mcpdo to avoid the existing unrelated mcpi npm package.

Naming

Reviewer-visible rename since the last review: the client moved from clients/mcpi to clients/daemon-cli, the bin is mcpdo, and user-facing vocabulary moved from "session" to "connection" (connections/list|show|use, --connection/--conn) — modern MCP is session-less and the daemon-held thing's lifecycle is the connection's.

Test plan

• npm run coverage:daemon-cli — 417 tests, per-file coverage gate ≥90 on all four dimensions (only the two true bootstraps src/mcp-bin.ts / src/daemon/run.ts excluded)
• npm run verify:bundle-externals (daemon-cli enrolled, 4 bundles)
• npm run local:gate from repo root — green on macOS
• Manual: connect/tools/resources/logging-tail against test-servers over stdio + HTTP, OAuth + EMA connects, shared and mcpdo private daemons

[BobDickinson]

mcpi work summary — PR #1783 (2026-09-13 → 2026-09-14)

Branch: v2/mcpi-client in /Users/bob/Documents/GitHub/inspector-trees/v2-mcpi-client
Repo: modelcontextprotocol/inspector
PR: #1783 — all pushed; CI (build, coverage) green as of 7fd4af59.

Organized by what actually changed functionally, not commit order.

---

1. Modern protocol-era support (era + skills primitives)

Gave mcpi first-class awareness of MCP's protocol eras (legacy vs. modern/
task-capable) and filled in missing skills primitives:

• --era override on connect (f2fc1a2c): force which protocol era an
  ad-hoc session negotiates as, instead of only auto-detecting.
• sessions/show replaces initialize (9550b32c): the session-info RPC
  now reports era details directly (protocol version, task support, etc.)
  instead of the old bare initialize response.
• protocolEra surfaced everywhere (380dd3e7): every session listing
  (sessions/list, not just sessions/show) now reports era at a glance.
• tasks/update (22d5c9f4): implemented to resume paused "modern"
  (task-capable) MCP tasks, with success/error-path tests.
• skills/list and skills/get (40b4f441): implemented the RPCs
  (previously stubbed/missing), supporting positional and --uri argument
  forms, plus a --verify flag on skills/list.
• Docs (2a61f427): documented --era, sessions/show, and
  tasks/update end-to-end.

2. Elicitation features (legacy URL-mode and modern/MRTR form-mode)

Built out MCP's elicitation flow, covering both eras' mechanisms:

• URL-mode (legacy elicitation) (ac7e4bf1): when a server elicits via a
  URL, mcpi prompts to confirm/open it and waits for completion.
• Form-mode (modern/MRTR structured elicitation) (258d789f): when a
  server elicits structured form data (JSON-schema-driven, per the newer
  request-response/MRTR-style pattern), mcpi walks the user through each
  field interactively with a review step before submitting.
• --elicit capability override (98a41510): lets a caller declare
  elicitation support explicitly, for ad-hoc/non-standard clients.

3. Making mcpi agent-friendly

Everything else — reframing and hardening mcpi so an AI agent driving it
non-interactively gets the same guarantees a human at a terminal gets:

• Packaging (29193232): bundled mcpi into the published
  @modelcontextprotocol/inspector npm package so it actually ships.
• mcpi agent-help + skills/mcpi/SKILL.md (9ecd2647): a discoverable,
  self-contained reference for agents on how to drive mcpi non-interactively.
• OAuth without a TTY (0096e2c7): OAuth's URL-prompt-and-wait flow no
  longer requires an interactive terminal; message reframed for an
  agent-attended flow ("The user needs to navigate to this link to
  authenticate: <url>"). Added clean SIGINT/SIGTERM cancellation so a user
  (or agent) can break out of the ~15-minute OAuth wait if they decide not to
  auth or auth fails, instead of it being a hard, uninterruptible block. Also
  addressed the daemon idle-timeout interacting with long OAuth waits.
  Live-tested with a real, non-TTY OAuth flow.
• Non-TTY elicitation (7cf45384): removed the TTY gate on elicitation
  entirely — both URL-mode and form-mode now work non-interactively, since
  the underlying readline-based prompting was never actually TTY-dependent,
  just gated by policy. Closed the one real risk this exposed (stdin EOF/close
  could hang readline.question() forever) by racing every prompt against a
  "stdin closed" signal. Live end-to-end tested against a real MCP test
  server, including a piped-EOF instant-decline case and a live-FIFO
  simulated-agent-relayed-answer case.
• Final non-TTY audit + SIGINT cleanup (7fd4af59): audited all
  remaining isTTY gates; confirmed auth/clear --all and
  requireExplicitSession()'s explicit-session requirement are intentional
  (see MRU note below), fixed a stale doc comment, and extended clean
  SIGINT/SIGTERM cancellation from the two streaming RPCs to the general
  rpc path so Ctrl-C during any blocking call (e.g. tools/call, an
  elicitation wait) cancels cleanly instead of killing the process.

Key design note (MRU): the daemon is a single shared process, so MRU
("most recently used" session) state is global, not per-terminal.
requireExplicitSession() gates on stdin, not stdout, so a human piping
output (mcpi tools/list | jq) still gets MRU convenience; a truly
non-interactive caller (agent/script/CI) must pass --session/@name
explicitly, since there's no live human to catch a wrong guess.
MCP_ALLOW_DEFAULT_SESSION=1 opts back into MRU for scripts that want it.

---

Non-functional maintenance (excluded from the above as "not changes")

These kept the branch buildable/green but didn't change behavior:

• e79dea7f, fd64afff — restored build:dev tooling/build config after a
  v2/main merge broke it.
• 54d00b91 — brought mcpi's validate scripts into parity with the rest of
  the repo's guards.
• aabc19fa, 12353bea — closed CI coverage/build gaps (including one
  caused by the agent-help commit itself shipping without tests) — pure
  test-coverage backfill, no functional change.

[cliffhall]

Mergeability verdict: ❌ Not mergeable — changes requested

CI is green and the feature works end to end. I connected, listed and called tools, and checked sessions and daemon status against a stdio test server. The test suite is large and mostly good. Four things block the merge:

1. Private-mode daemons can be taken over, and live sessions orphaned (security, reproduced).
2. Server-controlled terminal escape sequences reach the user's terminal raw (security, reproduced).
3. Stdio connect resolves relative commands against the daemon's cwd, not the caller's, so it can run a different file than the one the user named (reproduced).
4. Repo-rule violations: dependency placement, coverage-gate exclusions, no DCO signoff on any of the 30 commits, and a test that fails on stock macOS so local:gate cannot go green on a Mac.

There is also a scope question maintainers need to decide explicitly, not by drift: this PR now publishes a global mcpi bin and a background daemon to every installer of @modelcontextprotocol/inspector, while the PR description still says it does not.

Everything below was checked in a clean worktree of 22c97b09 (npm install && npm run build, then validate:guards, coverage:mcpi and verify:bundle-externals) on macOS 15, with an isolated MCP_STORAGE_DIR / MCP_INSPECTOR_DAEMON_DIR.

---

1. Security

Most of the risk comes from what mcpi adds on top of the one-shot CLI: a detached, long-lived process that accepts connect requests carrying an arbitrary serverConfig (including a stdio command) over a Unix socket, and then spawns that command. The one-shot CLI's exposure ends when the process exits. The daemon's does not: it stays up for as long as any session is open, because the idle timer only arms at zero sessions.

The stated trust model is same-UID filesystem trust (shared mode), plus an IPC token in private mode. The findings below are measured against that model.

1a. 🔴 A wrong or missing token replaces a live private daemon (blocker)

ensureDaemon() (clients/mcpi/src/daemon/ensure.ts) treats "socket reachable but ping failed" as "stale socket". It unlinks the socket and spawns a new daemon. ping also fails on daemon_auth_failed, so any caller holding the wrong token, or no token, deletes the live daemon's socket and installs its own daemon in its place.

Reproduced:

# user starts a private daemon
MCP_INSPECTOR_DAEMON_TOKEN=goodtoken mcpi connect --session a node server.js   → daemon 90433

# any same-UID process without the token
mcpi connect --session evil node server.js                                      → spawns daemon 90441 (NO token)

# the legitimate user, still presenting goodtoken
MCP_INSPECTOR_DAEMON_TOKEN=goodtoken mcpi sessions/list
Sessions (1):
* `@evil` (MRU) — node server.js [legacy]

Three consequences:

• Private mode's guarantee is void. The user's token-bearing client is silently served by an unauthenticated daemon that someone else started. The replacement's @evil session is now the user's MRU default, so the user's next bare mcpi tools/call … goes to a server the other party chose. The token exists to separate same-UID callers; this is the one thing it currently fails to do.
• Orphaned processes. The original daemon, 90433 above, and its stdio server keep running with no reachable socket. They never exit, because idle reaping is only armed at zero sessions. The same happens with a wrong token (MCP_INSPECTOR_DAEMON_TOKEN=wrong also produced a second daemon). A typo therefore leaks processes.
• Lock-out. With a wrong token, the legitimate user then gets daemon_auth_failed against the replacement.

Fix: on daemon_auth_failed (or any structured error reply), ensureDaemon must fail loudly and leave the socket alone. Only a socket that refuses connections (ECONNREFUSED / ENOENT) is stale. daemon.lock is written but never used as a lock. Make it one: store pid plus start time, use an O_EXCL create or proper-lockfile, which is already a root dependency, and check liveness with process.kill(pid, 0) before unlinking anything.

1b. 🔴 Terminal escape injection from server-controlled text (blocker)

The human formatter (clients/mcpi/src/session/format-human.ts, the default --format text) writes server-supplied strings straight to the terminal: tool results, descriptions, resource text, elicitation messages, and URIs embedded in OSC 8 hyperlinks (clients/cli/src/style.ts). I found no control-character stripping anywhere under clients/mcpi/src.

Reproduced with the echo test tool (od -c of stdout):

E c h o :   h i 033 ] 5 2 ; c ; c H d u Z W Q = \a 033 ] 0 ; S P O O F E D - T I T L E \a

That is an OSC 52 clipboard write and an OSC 0 title change, both delivered intact from the server. Depending on the terminal, the same channel allows clipboard poisoning (the next paste into a shell), hiding or overwriting earlier output (CSI cursor moves and erases), and spoofing link targets. A URI containing \a also breaks out of the OSC 8 wrapper. The one-shot CLI emits JSON, where these bytes are escaped, so this is new exposure introduced by this PR. It matters more because the skill in this PR targets agents reading the output.

Fix: sanitize every server-derived string before styling it. Replace C0/C1 controls other than \n and \t, plus DEL, with visible escapes such as \x1b → ␛ or \u001b. Validate or percent-encode URIs before putting them in OSC 8. --format json is already safe.

1c. 🟠 Stdio commands resolve against the daemon's cwd, not the caller's

The daemon is spawned without cwd, so it inherits the directory of whichever mcpi invocation first started it. connect does not default --cwd to the caller's process.cwd() (clients/mcpi/src/session/mcp.ts, serverOptions.cwd). A relative stdio target is therefore resolved in someone else's directory:

(daemon started from the repo root; lsof cwd → /…/mcp-inspector-pr1783)
cd test-servers/build && mcpi connect --session rel node ./test-server-stdio.js
{"error":{"code":"error","message":"Connection closed"}}

Here it failed with an opaque error, but only because the file did not exist in the daemon's cwd. If a file with the same name exists there, mcpi silently runs that one. mcpi connect node ./server.js in project B would execute project A's ./server.js. That is a correctness bug with a real security edge. PATH has the same staleness problem: every later session inherits the first shell's PATH, whether that came from nvm, a venv or anything else.

Fix: the front end should always send an absolute cwd, defaulting to process.cwd(), and should resolve relative command paths before sending. It should also consider forwarding the caller's PATH. The daemon should chdir to its own directory, or to /, at startup so it never pins an arbitrary working directory.

1d. 🟠 The daemon dies silently, and the socket-path limit is unchecked

ensure.ts spawns the daemon with stdio: "ignore", so every startup failure is invisible. The client waits 10s and reports a generic daemon_start_timeout. I hit this at once: my scratch MCP_INSPECTOR_DAEMON_DIR produced a 140-byte socket path, and listen() fails above macOS's 104-byte sun_path limit (108 on Linux). The daemon exited, left a stale daemon.lock behind (mode 0644, because the chmod never ran), and the user saw only a timeout.

The private-mode layout, $HOME/.mcp-inspector/private/<uuid>/daemon.sock, uses 72 fixed bytes, leaving ~32 bytes for $HOME on macOS. This is also why a test fails locally (see §2c).

Fix: validate the socket path length up front with a clear error, and shorten the private layout, e.g. a short id, or $TMPDIR/mcpi-<uid>/<short> with a 0700 dir. Send the daemon's stderr to daemon.log in the daemon dir with mode 0600, and have daemon_start_timeout include its tail.

1e. 🟡 Hardening (not blocking on their own)

• Shared-mode directory permissions. ensureDaemonDir() creates ~/.mcp-inspector with the default umask (0755 here). The socket is chmod 0600 only after listen(), which leaves a short window. Create the dir 0700 and bind inside it. Then the socket's own mode never matters, and BSD's inconsistent enforcement of socket permissions stops mattering too.
• An exec service outside agent sandboxes. This deserves a paragraph because the PR ships a skill aimed at agents. In shared mode, any same-UID process that can connect() to ~/.mcp-inspector/daemon.sock can have the daemon spawn any command. Same-UID is nominally the same privilege, but agent sandboxes (Claude Code's sandbox, Codex, containers that bind-mount $HOME, Flatpak) often restrict exec and filesystem access and not Unix-socket connects. A daemon started outside a sandbox, by the human, becomes an unsandboxed exec endpoint for any agent inside one. My suggestion: always require a token, including in shared mode, stored in a 0600 file in the 0700 daemon dir. That gives no extra protection against a plain same-UID process, which can read the file anyway. It does give sandbox policies a file-read denial to rely on, which is the control they actually have. It also retires the tokenless path that 1a exploits.
• No request-size limit on the NDJSON reader (readline over the socket). A single unbounded line grows daemon memory without limit. Cap the line length.
• Non-TTY elicitation answered by an agent (7cf4538). This is a real product decision, not a bug. Form-mode elicitation is meant to put a question to the user, and this makes it routine for an agent to answer on the user's behalf. That may be fine for an inspector, but please record it as a decision (spec doc plus README), and keep URL-mode elicitation requiring an explicit human action. skills/mcpi/SKILL.md also still says "running non-interactively (no TTY, scripted, or --format json) auto-declines", which that commit made false. Only --format json declines now.
• core/auth/node/runner-interactive-oauth.ts now installs process-wide SIGINT/SIGTERM listeners for the length of the OAuth wait. They are correctly removed in finally, but this is shared core/ and also runs under the TUI, which owns Ctrl-C through Ink. Please confirm that TUI Ctrl-C during an OAuth wait still behaves as intended, or scope the handler to callers that opt in.

Good things worth keeping: timingSafeEqual token comparison, 0600 socket and lock, 0700 private dirs, randomBytes(32) tokens, the POSIX-safe single-quoting in mcpi private, idle self-reaping, and stdio children exiting when their stdin closes. I SIGKILLed the daemon, and its test server exited with no orphans.

---

2. Repo-rule compliance (AGENTS.md)

2a. 🔴 Dependency placement

• clients/mcpi/package.json re-declares root-owned runtime dependencies: @modelcontextprotocol/{client,core,server,server-legacy}, ajv, atomically, @napi-rs/keyring, pino, undici, zod, commander and open. This breaks "a client declares only what that client alone consumes … clients/cli and clients/launcher therefore declare no runtime dependencies". It re-creates exactly the second copy that #1896 exists to prevent (the 3,387-line clients/mcpi/package-lock.json). server/server-legacy are not runtime dependencies of a client at all.
• The re-declaration is also load-bearing, which is why this matters beyond neatness. tsup auto-externalizes what the client's manifest declares, and mcpi's external list omits undici, zod, ajv and atomically. Deleting the manifest entries today would inline undici and reproduce #2067 (Dynamic require of "assert" is not supported). Fix: delete the client runtime deps and name every root runtime dependency that core/ reaches in clients/mcpi/tsup.config.ts external, mirroring clients/cli/tsup.config.ts and its comments.
• AGENTS.md still says "must also be named in all three bundler external lists (clients/{cli,tui}/tsup.config.ts, clients/web/tsup.runner.config.ts)". With a fourth bundler this rule changes, so update AGENTS.md in the same change, per the maintenance rule.

2b. 🔴 Coverage gate: whole files waved out

clients/mcpi/vitest.config.ts excludes src/daemon/ipc-glue.ts and src/daemon/stream-client.ts from the ≥90 per-file gate as "hard-to-stabilize accept/stream races". The rule is explicit: "A genuinely-unreachable branch is annotated at the source, never waved through by lowering the gate", and a race is fixed with an awaited condition, never with headroom or exclusion (#1596). ipc-glue.ts is the socket accept loop and the elicitation line-consumer, the most security-relevant code in the client. It is the file that most needs gating. Only the true bootstraps (mcp-bin.ts, daemon/run.ts) qualify for exclusion, as with clients/cli's src/index.ts. The AGENTS.md edit that documents these exclusions should be dropped along with them.

2c. 🔴 A test fails locally, so local:gate cannot pass on macOS

coverage:mcpi → daemon-private.test.ts > ensureDaemon spawns a token-gated daemon from env fails on stock macOS: 1 failed | 220 passed, with Timed out waiting for session daemon. The test sets HOME to os.tmpdir()/…, which on macOS is /var/folders/…/T/. The resulting socket path is 142 bytes (§1d). CI passes only because Linux's /tmp is short. local:gate is the mandatory pre-push command, and the PR's own test plan still has npm run ci unchecked. Fixing §1d fixes this too.

2d. 🔴 DCO

None of the 30 commits carries Signed-off-by (git log --format='%(trailers:key=Signed-off-by)' is empty for every one). AGENTS.md: "sign off every commit (git commit -s — the DCO check is a hard merge gate with no partial credit)". A rebase with --signoff fixes it. It is probably best done together with the rebase in §2f.

2e. 🟠 Docs and structure

• The PR description is stale and contradicts the code. It says "Not in the published tarball (files allowlist unchanged)" and "No root bin.mcpi". 2919323 adds "mcpi": "./clients/mcpi/build/mcp-bin.js" to root bin, and adds clients/mcpi/build and skills/mcpi to files. It also still says "Depends on #1782" (merged) and "Retarget … after #1782 merges". Please rewrite it; reviewers and the release notes will read it.
• The new top-level skills/ directory is missing from the AGENTS.md / README Project Structure trees. Its relationship to .claude/skills/ needs one line (end-user skill shipped in the tarball vs. repo procedures). Otherwise the next agent will try to run verify:skills rules against it, or move it.
• Branch name v2/mcpi-client lacks the type/issue segment (v2/feat/1432-mcpi-client). This is minor and not worth a new branch now.

2f. 🟠 Freshness and size

The branch is 124 commits behind v2/main. That includes #2374's Skills registry and -32021 changes and the SDK-v2 client-extension work, which touch the same skills/list / skills/get / era surfaces mcpi wraps. mergeStateStatus says CLEAN, but green CI on a stale base proves little for this surface. Please rebase with --signoff and re-run local:gate. At 16.8k added lines, with two merge commits and features accreted over two months (elicitation, EMA, tasks/update, era, packaging), this is hard to review as a whole. At minimum, the packaging change (2919323) should be split into its own PR so it gets its own decision (see §3).

2g. 🟡 Architecture: the @inspector/cli reach-in

The build-time alias from clients/mcpi into clients/cli/src (handlers, error-handler, OAuth navigation) makes one client's private source another client's API. clients/cli refactors can now break mcpi with no signal in the cli's own gate. fd64aff and aabc19f are both exactly this kind of breakage. The code comments say "temporary"; please file the tracking issue now (move handlers/, error-handler, and cli-oauth-navigation into core/, or a shared Node-runner area) and link it from the tsup comment. Temporary without an issue tends to become permanent.

---

3. Should it ship in the published package? Should it be containerized?

Shipping. Publishing adds a second global bin and a long-lived background daemon to every npm i -g @modelcontextprotocol/inspector install, under a name maintainers haven't signed off on. (mcpi is also an existing, unrelated npm package, a Minecraft-Pi API, which is harmless for a bin but will confuse search and npx mcpi.) The issue and spec still call this experimental. I'd keep it out of the tarball until the security items above are fixed and a maintainer signs off on the bin name, then publish it in a dedicated PR. That was the original plan in this PR's description, and I think it was right.

Containerizing the daemon: should not, and mostly could not usefully. The daemon's whole job needs host resources: the OS keychain (@napi-rs/keyring), the shared oauth.json store, the user's browser for OAuth, a loopback OAuth callback port, and above all local stdio servers that exist to touch the user's files and tools. Putting the daemon in a container breaks keyring and OAuth, turns every stdio server into a mount-and-PATH configuration problem, and on macOS and Windows adds a Linux VM dependency (Docker Desktop, Podman). It also secures the wrong thing. The daemon itself is small, trusted first-party code. The risky parts are (a) the socket as an exec endpoint, which §1a and §1e fix in code, and (b) the MCP servers it runs, which are untrusted third-party code. That is the same risk every MCP host takes, and containers are the right tool for it.

What I'd recommend instead:

1. Fix the socket boundary in code (§1a, §1e). That is the risk the daemon adds.
2. Make server isolation opt-in, per session. Document the recipe that already works today with no code: mcpi connect docker run -i --rm --network none -v "$PWD:/work:ro" <image>. Then consider a first-class --sandbox on connect that wraps the stdio command: docker/podman run -i everywhere, with lighter native options later (bwrap on Linux, sandbox-exec profiles on macOS). That puts isolation where the untrusted code is, lets users choose it per server, and costs nothing when unused.
3. Treat HTTP/SSE targets as needing no process isolation. Their risk is the terminal-output and elicitation surface (§1b, §1e), which sanitization covers.

---

Summary of requested changes

#	Change	Severity
1a	ensureDaemon: never unlink or replace on auth failure; turn daemon.lock into a real pid lock	🔴 security
1b	Sanitize control characters in all server-derived text output; validate OSC 8 URIs	🔴 security
1c	Send an absolute cwd (default process.cwd()) with stdio connects; chdir the daemon away	🟠 security/correctness
1d	Socket-path length check, shorter private layout, daemon stderr to a 0600 log	🟠
1e	0700 daemon dir; token always required (file-backed); cap NDJSON line length; fix SKILL.md elicitation wording; confirm TUI Ctrl-C	🟡
2a	Drop client runtime deps; complete the external list; update AGENTS.md's "three lists" rule	🔴 rules
2b	Gate ipc-glue.ts / stream-client.ts (fix races, v8 ignore only truly unreachable lines)	🔴 rules
2c	Make daemon-private.test.ts pass on macOS (falls out of 1d)	🔴 rules
2d	--signoff every commit	🔴 rules
2e/2f	Rebase on v2/main, rewrite the PR description, document skills/, split out packaging	🟠
2g	File the tracking issue for the @inspector/cli reach-in	🟡

Happy to re-review once the 🔴 items are in. The session model itself works well and I'd like to see it land.

[BobDickinson]

Thanks for the thorough review — reproducing 1a–1c made these easy to fix with confidence. Everything below is on the branch; local:gate is green on macOS (the §2c failure is gone). Two headline changes since your review, both maintainer-approved: the client is renamed (mcpi → mcpdo, clients/mcpi → clients/daemon-cli, "session" → "connection" vocabulary), and packaging is folded back into this PR (details under §3).

§1 Security

• 1a — fixed. ensureDaemon now fails loudly on any structured error reply (including daemon_auth_failed) and never unlinks the socket; only ECONNREFUSED/ENOENT is treated as stale. daemon.lock is a real O_EXCL pid+starttime lock with a kill(pid, 0) liveness check before any reclaim. Your repro sequence now errors instead of replacing the daemon.
• 1b — fixed. All server-derived terminal-bound text goes through a sanitizer (C0/C1 + DEL → visible escapes, \n/\t preserved); OSC 8 URIs are validated before wrapping. Covered by tests including your OSC 52/OSC 0 payloads. --format json unchanged.
• 1c — fixed. connect always sends an absolute cwd, defaulting to the caller's process.cwd(); the daemon chdirs to its own directory at startup. For PATH we went a step further than forwarding: bare command names are resolved client-side against the caller's PATH (which-style) and sent absolute, so no environment crosses the socket at all.
• 1d — fixed. Socket path length is validated up front against the platform sun_path limit with a clear error; the layout is shortened to $TMPDIR/mcp-conn-<uid>/<id>/ (0700); daemon stderr goes to a 0600 daemon.log and start-timeout errors include its tail.
• 1e — all taken. Daemon dir created 0700 before bind; token always required in every mode (0600 daemon.token in the 0700 dir — adopted your reasoning: it gives sandbox policies a file-read denial to enforce and retires the tokenless path from 1a); 1 MiB NDJSON request-line cap; SKILL.md elicitation wording corrected and the non-TTY-agent-may-answer decision is recorded in specification/v2_cli_v2.md (URL mode still requires an explicit answer); the OAuth SIGINT/SIGTERM handlers were a regression introduced by this PR's own first commit — they're now opt-in (handleSignals), so TUI Ctrl-C behavior is unchanged.

§2 Repo rules

• 2a — fixed. Client runtime deps removed; every root runtime dependency core/ reaches is named in clients/daemon-cli/tsup.config.ts external, mirroring clients/cli; AGENTS.md's "three lists" rule updated to four.
• 2b — fixed. ipc-glue.ts and stream-client.ts are in the ≥90 per-file gate; the accept/stream races were fixed with awaited conditions, and only genuinely-unreachable lines carry annotated v8 ignore. Only the two true bootstraps remain excluded. The AGENTS.md exclusion note is gone.
• 2c — fixed (fell out of 1d). local:gate green on stock macOS.
• 2d — fixed. Rebased; every commit carries Signed-off-by.
• 2e — done. PR description rewritten to match the code (including packaging); skills/ documented in the AGENTS.md and README structure trees with the .claude/skills/ distinction. Agreed on leaving the branch name.
• 2f — done, with one deviation. Rebased onto current v2/main and re-ran local:gate. Packaging was initially split to a separate branch as you suggested, then folded back after an explicit maintainer decision to ship it in this PR — see §3.
• 2g — done. Tracking issue chore(mcpi): replace the temporary @inspector/cli source alias with a real shared surface #2461 filed for promoting the @inspector/cli reach-in surface to a shared area; linked from the tsup comment.

§3 Decisions

• Shipping: maintainer-approved to bundle in this PR, under the new conflict-free bin name mcpdo (your mcpi-collision point drove the rename). Data point: the addition is ~199 KB compressed / ~770 KB unpacked (~16% of the tarball), and the daemon is inert unless the bin is invoked. Without bundling there was no reasonable install story for the experimental client (build-from-source + npm link).
• Containerizing: agree — no. Your opt-in per-connection isolation recipe (docker run -i --rm --network none …) is now documented in clients/daemon-cli/README.md; a first-class --sandbox flag is deferred as a possible follow-up.

Ready for re-review whenever you are.

[Copilot]

Copilot review overview

🟡 Changes recommended

The request-size security limit is bypassable, and several correctness and required lint-enforcement issues remain.

Get a fresh assessment by requesting another Copilot review.

Review effort: Balanced
Findings: 2 · 4 · 4

Open (10)

• Concurrent startup callers can use mismatched daemon tokens · New
• Oversized terminated lines bypass the 1 MiB limit · New
• TypeScript lint omits no-floating-promises checks · New
• Schema sanitization mutates protocol data · New
• Reject non-finite numeric inputs · New
• Empty PATH entries incorrectly skip the current directory · New
• Dependency update path references nonexistent clients/mcpdo directory · New
• New client lint permits warnings · New
• Example fails to load the specified JSON config · New
• Advertised initialize command is not registered · New

What changed in this PR

Adds mcpdo, an experimental connection-oriented MCP CLI backed by a token-authenticated Unix-socket daemon.

Changes:

• Implements persistent MCP connections, commands, OAuth, elicitation, streaming, and safe output formatting.
• Adds extensive tests and integrates the client into build, validation, coverage, and packaging.
• Documents the new client and ships an agent-facing mcpdo skill.

File	Description
AGENTS.md	Documents the new client and repository rules.
README.md	Adds mcpdo to the project overview.
clients/​cli/​__tests__/​method-types.test.ts	Updates shared method-list tests.
clients/​cli/​__tests__/​run-method-mocks.test.ts	Updates reusable handler mocks.
clients/​cli/​__tests__/​servers-list.test.ts	Tests reusable server-list behavior.
clients/​cli/​src/​cli-oauth-navigation.ts	Exposes shared OAuth navigation.
clients/​cli/​src/​cliOAuth.ts	Supports reusable OAuth flows.
clients/​cli/​src/​handlers/​consume-outcome.ts	Updates shared outcome handling.
clients/​cli/​src/​handlers/​method-types.ts	Defines connection-compatible methods.
clients/​cli/​src/​handlers/​run-method.ts	Exposes shared MCP method execution.
clients/​cli/​src/​handlers/​servers-list.ts	Generalizes server catalog loading.
clients/​cli/​src/​style.ts	Exposes CLI styling helpers.
clients/​daemon-cli/​README.md	Documents installation and usage.
clients/​daemon-cli/​__tests__/​agent-help.test.ts	Tests agent-help output.
clients/​daemon-cli/​__tests__/​authorize.test.ts	Tests authorization behavior.
clients/​daemon-cli/​__tests__/​connection-stored-auth.test.ts	Tests stored-auth commands.
clients/​daemon-cli/​__tests__/​daemon-connections.test.ts	Tests connection lifecycle.
clients/​daemon-cli/​__tests__/​daemon-coverage.test.ts	Covers daemon edge cases.
clients/​daemon-cli/​__tests__/​daemon-ipc-glue.test.ts	Tests IPC framing and handling.
clients/​daemon-cli/​__tests__/​daemon-paths.test.ts	Tests daemon filesystem paths.
clients/​daemon-cli/​__tests__/​daemon-private.test.ts	Tests private-daemon authentication.
clients/​daemon-cli/​__tests__/​daemon-stream.test.ts	Tests streaming IPC.
clients/​daemon-cli/​__tests__/​dispatch.test.ts	Tests RPC and stream dispatch.
clients/​daemon-cli/​__tests__/​elicitation-bridge.test.ts	Tests daemon elicitation bridging.
clients/​daemon-cli/​__tests__/​elicitation-client.test.ts	Tests elicitation client transport.
clients/​daemon-cli/​__tests__/​elicitation-prompt.test.ts	Tests interactive elicitation.
clients/​daemon-cli/​__tests__/​ema-commands.test.ts	Tests EMA commands.
clients/​daemon-cli/​__tests__/​ema.test.ts	Tests EMA authentication logic.
clients/​daemon-cli/​__tests__/​form-prompt.test.ts	Tests form prompting and validation.
clients/​daemon-cli/​__tests__/​form-schema.test.ts	Tests elicitation schema parsing.
clients/​daemon-cli/​__tests__/​format-connection.test.ts	Tests connection output formatting.
clients/​daemon-cli/​__tests__/​helpers/​mcp-runner.ts	Adds daemon CLI test harness.
clients/​daemon-cli/​__tests__/​hoist-connection.test.ts	Tests connection argument rewriting.
clients/​daemon-cli/​__tests__/​mcp-auth-coverage.test.ts	Covers MCP authentication branches.
clients/​daemon-cli/​__tests__/​mcp-connection.test.ts	Tests CLI connection workflows.
clients/​daemon-cli/​__tests__/​mcp-coverage.test.ts	Covers command-routing edge cases.
clients/​daemon-cli/​__tests__/​parse-tool-args.test.ts	Tests tool argument parsing.
clients/​daemon-cli/​__tests__/​resolve-command.test.ts	Tests executable resolution.
clients/​daemon-cli/​__tests__/​sanitize.test.ts	Tests terminal sanitization.
clients/​daemon-cli/​eslint.config.js	Configures daemon-client linting.
clients/​daemon-cli/​package-lock.json	Locks daemon-client dependencies.
clients/​daemon-cli/​package.json	Defines scripts and package metadata.
clients/​daemon-cli/​src/​connection/​authorize.ts	Implements connect-time OAuth.
clients/​daemon-cli/​src/​connection/​dispatch.ts	Dispatches daemon RPCs and streams.
clients/​daemon-cli/​src/​connection/​elicitation-prompt.ts	Implements elicitation prompts.
clients/​daemon-cli/​src/​connection/​ema.ts	Implements enterprise-managed auth.
clients/​daemon-cli/​src/​connection/​form-prompt.ts	Collects form elicitation input.
clients/​daemon-cli/​src/​connection/​form-schema.ts	Parses elicitation schemas.
clients/​daemon-cli/​src/​connection/​format-connection.ts	Formats command output.
clients/​daemon-cli/​src/​connection/​format-human.ts	Provides human-readable formatting.
clients/​daemon-cli/​src/​connection/​mcp.ts	Defines the mcpdo command surface.
clients/​daemon-cli/​src/​connection/​parse-tool-args.ts	Parses tool-call arguments.
clients/​daemon-cli/​src/​connection/​private-env.ts	Creates private-daemon shell exports.
clients/​daemon-cli/​src/​connection/​resolve-command.ts	Resolves caller-side executables.
clients/​daemon-cli/​src/​connection/​sanitize.ts	Sanitizes terminal-bound data.
clients/​daemon-cli/​src/​connection/​stored-auth.ts	Manages persisted authentication.
clients/​daemon-cli/​src/​daemon/​auth.ts	Implements daemon token authentication.
clients/​daemon-cli/​src/​daemon/​client.ts	Implements request-response IPC.
clients/​daemon-cli/​src/​daemon/​connections.ts	Manages persistent MCP connections.
clients/​daemon-cli/​src/​daemon/​elicitation-bridge.ts	Bridges elicitation over IPC.
clients/​daemon-cli/​src/​daemon/​ensure.ts	Starts and discovers the daemon.
clients/​daemon-cli/​src/​daemon/​framing.ts	Encodes and parses IPC frames.
clients/​daemon-cli/​src/​daemon/​index.ts	Exports daemon APIs.
clients/​daemon-cli/​src/​daemon/​ipc-glue.ts	Accepts and processes socket clients.
clients/​daemon-cli/​src/​daemon/​paths.ts	Defines daemon paths and limits.
clients/​daemon-cli/​src/​daemon/​protocol.ts	Defines the IPC protocol.
clients/​daemon-cli/​src/​daemon/​run.ts	Boots the daemon process.
clients/​daemon-cli/​src/​daemon/​server.ts	Implements daemon lifecycle and routing.
clients/​daemon-cli/​src/​daemon/​stream-client.ts	Implements streaming IPC clients.
clients/​daemon-cli/​src/​mcp-bin.ts	Boots the mcpdo executable.
clients/​daemon-cli/​tsconfig.json	Configures source type-checking.
clients/​daemon-cli/​tsconfig.test.json	Configures test type-checking.
clients/​daemon-cli/​tsup.config.ts	Builds CLI and daemon bundles.
clients/​daemon-cli/​vitest.config.ts	Configures tests and coverage.
clients/​tui/​package-lock.json	Refreshes the TUI dependency lock.
clients/​web/​package-lock.json	Refreshes the web dependency lock.
clients/​web/​src/​test/​core/​auth/​runner-interactive-oauth.test.ts	Tests OAuth signal cancellation.
core/​auth/​node/​runner-interactive-oauth.ts	Adds optional signal handling.
core/​mcp/​serverList.ts	Persists elicitation capability settings.
core/​mcp/​types.ts	Defines elicitation capability types.
package.json	Wires build, validation, packaging, and bin entry.
scripts/​install-clients.mjs	Adds daemon-client installation.
scripts/​lib/​workflow-gate.test.mjs	Updates gate coverage assertions.
scripts/​sdk-watch.mjs	Includes daemon bundle externals guidance.
scripts/​verify-bundle-externals.mjs	Verifies the new multi-entry bundle.
scripts/​verify-format-coverage.mjs	Enrolls daemon-client formatting.
scripts/​verify-test-timeouts.mjs	Enrolls daemon-client timeouts.
scripts/​verify-test-timeouts.test.mjs	Updates timeout guard tests.
skills/​mcpdo/​SKILL.md	Adds agent-facing usage guidance.
specification/​v2_catalog_launch_config.md	Links the as-built CLI specification.
specification/​v2_cli_tui_launcher.md	Documents the additional client surface.
specification/​v2_cli_v2.md	Specifies the implemented connection CLI.

Files not reviewed (3)

• clients/daemon-cli/package-lock.json: Generated file
• clients/tui/package-lock.json: Generated file
• clients/web/package-lock.json: Generated file

---

💡 Add a code-review agent skill for context-aware, tailored reviews. Learn more in the docs.

[Copilot]

Copilot review overview

🟡 Changes recommended

Unvalidated terminal hyperlinks, an unsafe private-daemon parent directory, and potentially truncated stream output must be corrected before approval.

Get a fresh assessment by requesting another Copilot review.

Review effort: Balanced
Findings: 4

Open (4)

• Await pending stdout writes before exiting · New
• Validate URL-elicitation schemes before rendering links · New
• Restrict OSC 8 links to an explicit scheme allowlist · New
• Reject insecure or symlinked temporary parent directories · New

Resolved since last review (10)

• Oversized terminated lines bypass the 1 MiB limit
• Concurrent startup callers can use mismatched daemon tokens
• Empty PATH entries incorrectly skip the current directory
• Reject non-finite numeric inputs
• Schema sanitization mutates protocol data
• TypeScript lint omits no-floating-promises checks
• Advertised initialize command is not registered
• Example fails to load the specified JSON config
• New client lint permits warnings
• Dependency update path references nonexistent clients/mcpdo directory

Files not reviewed (3)

• clients/daemon-cli/package-lock.json: Generated file
• clients/tui/package-lock.json: Generated file
• clients/web/package-lock.json: Generated file

Previously missed (3)

In code that hasn't changed since last review

Add elicitCapability persistence and fallback coverage

core/​mcp/​serverList.ts:74

The new persisted setting adds valid/invalid read branches and default-omission write behavior, but the existing comprehensive serverList.test.ts suite has no elicitCapability case. Add coverage for all accepted literals, an unknown hand-edited value falling back to the default, and round-trip/default omission so this shared catalog behavior cannot regress unnoticed.

Verify the published mcpdo executable and artifacts

package.json:22

This publishes a second executable, but scripts/pack-and-verify.mjs still checks only the installed mcp-inspector bin and web/launcher artifacts. A missing clients/daemon-cli/build, broken bin.mcpdo target, or omitted skills/mcpdo directory would therefore pass the repository's published-tarball verification. Enroll the new files and run the installed mcpdo --help in that check.

Rename the tracking entry to Connection CLI umbrella

specification/​v2_catalog_launch_config.md:515

The PR explicitly renames the user-facing concept from “session” to “connection,” but this updated row still calls #1432 the “Session CLI umbrella.” Use “Connection CLI umbrella” so the tracking table matches the as-built terminology.

[Copilot]

Copilot review overview

🟡 Changes recommended

OAuth helper coordination contains two stale-file races and an unref’ed polling timer that can lose concurrent sign-in flows.

Review effort: Balanced
Findings: 1 · 1

Open (2)

• Unref'ed polling timer lets reservation losers exit early · New
• TOCTOU race can delete a fresh authorization marker · New

Resolved since last review (2)

• Atomically reserve per-server flows before spawning helpers
• Shell permission bypass enables unrestricted host command execution

[Copilot]

Copilot review overview

🟡 Changes recommended

RPC cancellation, caller-environment propagation, and evaluation transcript handling contain unresolved correctness defects.

Review effort: Balanced
Findings: 1

Open (1)

• This catches every parse failure, not just a torn final record, so corruption in the middle of the… · New

Resolved since last review (2)

• Unref'ed polling timer lets reservation losers exit early
• TOCTOU race can delete a fresh authorization marker

Previously missed (2)

In code that hasn't changed since last review

Resolving only the executable does not preserve the invoking shell's stdio environment.…

clients/​daemon-cli/​src/​connection/​mcp.ts:547

Resolving only the executable does not preserve the invoking shell's stdio environment. StdioClientTransport is constructed in the persistent daemon, so its inherited PATH, HOME, SHELL, etc. come from whichever shell first spawned that daemon; a later shell can resolve the right executable here while the server and any subprocesses still receive stale values. Snapshot the SDK's inherited environment client-side at connect time and merge the configured env over it before sending serverConfig.

The PR description says JSON mode auto-declines form elicitation, but this implementation…

clients/​daemon-cli/​src/​connection/​dispatch.ts:110

The PR description says JSON mode auto-declines form elicitation, but this implementation deliberately parks it and returns elicitationPending; the new elicitation test and shipped skill document the parked workflow too. Update the PR description to match the actual JSON contract so users do not assume the server receives a decline automatically.

[BobDickinson]

Re the two body-only "previously missed" items and the headline in the latest review:

• Stale environment for stdio servers (mcp.ts): valid — fixed in 5b92cff. connect now snapshots the SDK's default-inherited environment (PATH/HOME/SHELL/…) from the calling shell and merges it under any configured env before the config crosses to the daemon, completing the existing absolute-cwd/command handoff. The pinning is extracted into pinStdioConfigToCaller with unit tests, and now also treats a type-less config as stdio (the implicit default).
• PR description vs JSON elicitation behavior (dispatch.ts): the description was stale — updated. Non-interactive callers (--format json or no TTY) get the elicitation parked and answer via elicitation/respond; nothing is auto-declined.
• "RPC cancellation" (headline only): mentioned in the overview sentence but appears nowhere as a finding, so there is nothing actionable to evaluate.

[Copilot]

Copilot review overview

🟡 Changes recommended

Form validation mishandles valid Unicode lengths, ticket identifiers can collide, and an architecture document remains inconsistent.

Review effort: Balanced
Findings: 1 · 2 · 1

Open (4)

• Generate ticket IDs from unique per-submission state · New
• Count Unicode code points for entered value length validation · New
• Count Unicode code points for schema string length validation · New
• Update architecture documentation to include daemon-cli · New

Resolved since last review (1)

• This catches every parse failure, not just a torn final record, so corruption in the middle of the…

[Copilot]

Copilot review overview

🟡 Changes recommended

Subscription notifications can be lost, OAuth marker cleanup races with replacement flows, and pending connections report an unnegotiated protocol era.

Review effort: Balanced
Findings: 2

Open (2)

• Subscribe listener is attached too late, losing early updates · New
• Cleanup can delete a replacement helper marker · New

Resolved since last review (4)

• Generate ticket IDs from unique per-submission state
• Count Unicode code points for schema string length validation
• Count Unicode code points for entered value length validation
• Update architecture documentation to include daemon-cli

[Copilot]

Copilot review overview

🔵 Needs a closer look

IPC Unicode corruption, cross-connection elicitation ID collisions, and incorrect shipped guidance must be resolved.

Review effort: Balanced
Findings: None

Resolved since last review (2)

• Cleanup can delete a replacement helper marker
• Subscribe listener is attached too late, losing early updates

Previously missed (1)

In code that hasn't changed since last review

Update docs for parked elicitation behavior

clients/​daemon-cli/​README.md:175

This section still describes the former auto-decline behavior. The implemented dispatch path sets parkElicitations for JSON and no-TTY callers, returns an elicitation-pending payload, and resumes through elicitation/respond, as the PR summary also states. Update these instructions so agents do not expect an automatic decline or try to answer an inline prompt that is never opened.

[BobDickinson]

Re the latest review (no findings, one body-only item):

• README parked-elicitation docs: valid — fixed in 03bb675. The elicitation section now describes the actual behavior: non-interactive callers (--format json or no TTY) get the elicitation parked, the RPC returns an elicitationPending payload, and the answer comes via elicitation/respond (form key:=value/JSON, --done, --decline, --cancel; auto-cancel after 10 minutes). The --elicit off bullet no longer claims requests are auto-declined.
• "IPC Unicode corruption" and "cross-connection elicitation ID collisions" are mentioned only in the overview headline; no finding for either appears anywhere in the review, so there is nothing actionable to evaluate.

[cliffhall]

Mergeability verdict: ❌ Not mergeable yet (close)

The blockers from my previous review (2026-09-22) are fixed, and I re-drove each one against the built mcpdo bin. The client is fully wired into the quality gate, and every stage passes on this head. What blocks the merge now:

1. DCO: 24 of the 60 non-merge commits carry no Signed-off-by (for example fb2a2fb, 427608d, 80b8cc1, b5eee0a, 12e4cef). This is a hard merge gate with no partial credit. git rebase origin/v2/main --signoff fixes it in one pass.
2. Known-vulnerable esbuild (§3, Tool use #7): clients/daemon-cli resolves esbuild 0.27.7, which falls in GHSA-g7r4-m6w7-qqqr. Its own npm audit flags it. web, cli and tui all carry the "esbuild": "^0.28.2" override, and this client does not.
3. Four medium correctness bugs in the daemon and connection code (§3, Progress notifications #1–SSE transport support #4), including one where concurrent first runs can leave every client locked out with daemon_auth_failed.

Everything else below is follow-up or advisory. Once 1–3 are in, this looks mergeable to me.

Checked on head c15cd368 (origin/v2/mcpi-client) in a clean worktree with a full npm install, on macOS 15 / Node 26, with an isolated MCP_INSPECTOR_DAEMON_DIR / MCP_STORAGE_DIR. It merges into v2/main without conflicts (16 commits behind). CI build and coverage pass.

---

1. Previous blockers: all resolved

#	Previous finding	Now	How I checked
1a	A wrong or missing token replaced a live private daemon	✅ Fixed	Private daemon started with a token (pid 94026). A wrong-token connections/list fails loudly with daemon_auth_failed. The socket owner stayed 94026 throughout: nothing was unlinked, replaced or orphaned.
1b	Server-controlled escape sequences reached the terminal	✅ Fixed	tools/call echo with an OSC 52 clipboard write, an OSC 0 title and CSI 2J: 0 raw ESC and 0 BEL bytes on stdout, with and without --plain. The controls render as ␛ / ␇.
1c	Relative stdio commands resolved against the daemon's cwd	✅ Fixed	The daemon's cwd is now its own dir (lsof: /private/tmp/d1783). connect -- node ./server-composable.js from test-servers/build resolved against the caller's cwd and connected.
1d	Silent daemon death; unchecked socket-path length	✅ Fixed	A 127-byte socket path fails in about 1s with "socket path is too long for this platform (127 bytes > 103)". The daemon's stderr goes to a 0600 daemon.log.
1e	Hardening	✅ Mostly	Dirs are 0700, and the NDJSON reader is capped (MAX_REQUEST_LINE_BYTES, 1 MiB).
2a	Dependency placement	✅ Fixed	No runtime deps in clients/daemon-cli/package.json. verify:bundle-externals: OK, 4 bundles. AGENTS.md now reads "all four bundler external lists".
2b	Whole files waved out of coverage	✅ Fixed	Only the two bootstraps (src/mcp-bin.ts, src/daemon/run.ts) are excluded. ipc-glue.ts and stream-client.ts are gated.
2c	A test failed on macOS	✅ Fixed	coverage:daemon-cli on macOS: 399 / 399 pass.
2d	DCO	❌ Still open	24 of 60 commits unsigned (above).
2e	Stale description; skills/ undocumented	✅ Mostly	The description now matches the published bin. The skills/ tree is in AGENTS.md and the README. The test count in the description is stale (259, now 399).
2f	Branch freshness	✅	16 behind, merges clean.

On 1a: a caller with no token and the same MCP_INSPECTOR_DAEMON_DIR gets in, because the client reads the dir's 0600 daemon.token (client.ts:97). It connected @evil, which became the MRU. That matches the same-UID trust model (and the file-backed token I suggested), so I'm not counting it as a finding. But it means private mode isolates daemons from each other; it does not protect them from same-UID processes that know the dir. Worth one line in the docs or SKILL.md.

---

2. Is daemon-cli fully covered by the quality gate?

Yes for every core stage, with three gaps.

Covered, and passing on this head:

• validate / local:validate: validate:daemon-cli runs format:check, lint (--max-warnings 0, type-aware, no-floating-promises at error, globalIgnores(["build","coverage"])) and typecheck of both tsconfig.json and tsconfig.test.json, plus tests. 399 pass.
• coverage: coverage:daemon-cli runs a per-file threshold of ≥90 on lines, statements, functions and branches. It passed, with the aggregate at 98.0 / 93.5 / 97.7 / 98.3.
• CI: main.yml needed no change. build runs npm run validate and the parallel coverage job runs npm run coverage, and both reach the new client through the root scripts.
• Guards: all pass and all count the new client.
  • verify:install-fresh, verify:dep-lockstep and verify:typecheck-coverage discover clients/* dynamically (6 installs, 5 clients).
  • verify:test-timeouts (7 Vitest projects), verify:bundle-externals (4 bundles), verify:format-coverage and install-clients.mjs name it explicitly.

Gaps:

1. No mcpdo smoke in the gate. npm run smoke covers launcher, cli, tui and web, but never runs the mcpdo bin against a daemon. The daemon path is exercised only inside the Vitest suite (5 of the 30 test files spawn a real daemon). pack:verify adds mcpdo --help plus a daemon-free servers/list, but it is not in local:gate. It runs only in CI's release-time package job. A smoke:mcpdo that connects to a stdio test server, runs tools/call, then disconnect and daemon/stop, would close this.
2. The shipped skill is unvalidated. skills/mcpdo/SKILL.md ships in the tarball (files), but verify:skills scans only .claude/skills/. Malformed frontmatter, such as the silent unquoted-#/: truncation AGENTS.md warns about, would pass the gate and ship. Point verify:skills (or a sibling check) at skills/ too. The eval harness (skill-eval-mcpdo.mjs) being ungated is consistent with the skills:eval policy.
3. The two coverage exclusions carry no justification comment. Excluding the bin bootstraps is the same pattern as cli's src/index.ts and is fine, but the web client documents its exclusions on the array. One comment here would match.

---

3. New findings from this review

A high-effort review of the full diff against merge base 1716af17. Verified means I (or the review pass) re-read the code and confirmed the failure path, or reproduced it. Unverified means found by a review agent and not independently re-checked; treat those as leads.

Blocking (medium)

1. disconnectAll stops on the first missing connection. clients/daemon-cli/src/daemon/connections.ts:542 · verified
   • It loops await this.disconnect(name, false) with no try/catch, and disconnect throws connection_not_found if the entry was already removed (for example by a same-name reconnect that outlives the 3s quiesce).
   • One missing name stops the loop, so the remaining connections and their stdio children are never torn down, and clearIdleTimer is skipped.
   • doStop (server.ts:225) then rejects before destroySoon and server.close().
   • Fix: settle each disconnect independently (allSettled, or try/catch per name) and always run the tail of doStop.
2. Multi-byte UTF-8 split across socket chunks is silently corrupted. client.ts:260 and stream-client.ts:226 · verified
   • Both do buffer += String(chunk) without setEncoding("utf8"), so a character split across chunks becomes two U+FFFD. The JSON still parses, so nothing reports it.
   • It hits results over ~64KB containing CJK text or emoji, and long logging/tail streams.
   • Fix: socket.setEncoding("utf8") (or a StringDecoder).
3. Piped form or elicitation answers are dropped. connection/form-prompt.ts:37 (also elicitation-prompt.ts:108) · reproduced with a probe
   • On a non-TTY stdin, readline emits every buffered line in one pass while only the first has a pending rl.question, so the rest are lost and EOF cancels the form.
   • printf 'alice\n30\n\n' | mcpdo tools/call register fills one field, then cancels with "stdin closed".
   • Each elicitation round also opens and closes its own interface, which discards lines already buffered.
   • This matters for exactly the agent-driven, non-TTY use the skill promotes.
4. The lock race can leave every client locked out. daemon/server.ts:843 · verified by reading; narrow window
   • acquireLock treats an empty lock file as stale. That is also what a concurrent starter's lock looks like between openSync("wx") and writeSync(pid).
   • Two concurrent first runs can both win the lock and both write daemon.token. The survivor then serves one token while the file holds the other, so every client gets daemon_auth_failed until someone kills the daemon by hand.
   • Fix: treat an empty or unparsable lock younger than a short grace period as held, and retry.

Security / dependency (blocking; trivial fix)

7. esbuild override missing. clients/daemon-cli/package.json · verified
   • The lockfile resolves esbuild 0.27.7, which is inside GHSA-g7r4-m6w7-qqqr (0.27.3–0.28.0).
   • web, cli and tui carry "overrides": { "esbuild": "^0.28.2" }. Add the same here and refresh the lockfile.
   • verify:dep-lockstep cannot see this, because no manifest declares esbuild.

Should fix (medium, not blocking)

5. logging/tail prints [object Object]. connection/format-human.ts:895 · verified. The text output does String(params.data ?? …), so object-valued log data loses its payload. Render JSON for non-string data.
6. The eval matcher ignores --flag=value. scripts/lib/mcpdo-eval-matchers.mjs:94 · verified
   • parseMcpdoArgv only matches --connection / --conn as a separate token, so --connection=wrong-server parses as no connection and matchCall accepts it as a hit in the multi-server cases.
   • --tool-name=x scores a correct call as a miss.
   • This skews the behavior eval's numbers.
7. A stale parked-elicitation subscriber swallows a new elicitation. daemon/elicitation-park.ts:196,209 with elicitation-bridge.ts:72 · unverified
   • An expired or cancelled park leaves byId straight away, but its closed channel stays as subscribers[0] until the outcome settles.
   • A new tools/call that elicits in that window is auto-cancelled on the dead channel.

Low

9. ping succeeds while the daemon is stopping. server.ts:351, ensure.ts:157 · unverified. The next command fails with "shutting down" instead of respawning. After close(), a respawn blocked on the still-held lock waits the full 10s and ends in daemon_start_timeout.

10. An already-aborted signal leaks the socket. client.ts:249-273, stream-client.ts:215-235 · reproduced with a probe. onAbort destroys the socket, then socket.connect() still runs, which un-destroys it and dials the daemon. The leaked socket keeps the event loop alive and holds an ipcSockets entry. mcp-bin hides this only because it calls process.exit.

11. auth-helper.ts:419 listens for exit, not close. lower confidence. The child's final {"event":"error"} stdout line can be lost, and a generic "exited before producing an authorization URL" message replaces the real cause.

12. The failure path doesn't sanitize C1 controls. format-connection.ts:362,377, auth-helper.ts:414 · lower confidence. Server-derived text flows into CliExitCodeError messages, which formatErrorOutput writes with plain JSON.stringify, and that does not escape U+0080–U+009F. So a raw 8-bit CSI/OSC reaches the terminal on failure, while the success path sanitizes the same text (1b).

13. The eval shim can truncate stdout. scripts/lib/mcpdo-eval-shim.mjs:126 · lower confidence. It calls process.exit(code) right after stdout.write; on macOS pipe writes are async, so large JSON can be cut off.

14. Eval pool teardown. scripts/skill-eval-mcpdo.mjs:773,815 · unverified

    • One inconclusive sample rejects Promise.all, so the other samples' finally never runs and their private daemons and sandboxes leak.
    • If makeBehaviorEnv throws (:593), the sandbox created just before it is never removed.

15. The skills:eval env allowlist drops Bedrock and Vertex credentials. scripts/skill-eval.mjs:715 · verified by reading. The child env passes only the ANTHROPIC_, CLAUDE_ and XDG_ prefixes, so a maintainer running the eval against Bedrock (AWS_*) or Vertex (CLOUD_ML_* / GOOGLE_*) now fails auth.

The core/ changes (runner-interactive-oauth signal handling, elicitCapability persistence) and the clients/cli changes (ref-counted subscribe, tasks/update) came out clean. The daemon-cli external list matches cli's and covers what it needs.

---

Summary of requested changes

#	Change	Blocks merge
DCO	git rebase origin/v2/main --signoff (24 unsigned commits)	Yes
7	Add the esbuild ^0.28.2 override to clients/daemon-cli	Yes
1–4	disconnectAll resilience, UTF-8 decoding, piped prompt answers, the lock race	Yes
5, 6, 8	logging/tail objects, stale elicitation subscriber, --flag=value in the eval matcher	Should fix
Gate 1–2	Add a smoke:mcpdo to npm run smoke; validate skills/mcpdo/SKILL.md	Should fix (or file follow-ups)
9–15, Gate 3	Low-severity items and the exclusion comment	Follow-up

The session model works well end to end, and the fixes since the last round are solid. Once the blocking rows are in, I'm happy to approve.

🤖 Generated with Claude Code

[BobDickinson]

Thanks for the thorough round — every item is now addressed on head 3c3de07d (CI green: build + coverage). All commits gated locally with npm run format + npm run local:gate per commit. Per-item disposition below, with as-built notes where the fix differs from the suggested shape.

Blockers

#	Finding	Commit	Notes
DCO	24 unsigned commits	history rewrite, head c4e713ab → since extended	Used a targeted filter-branch over only the 24 unsigned commits rather than rebase --signoff: the range contains GPG-signed upstream merge commits that a rebase would have rewritten/unsigned. All 67 non-merge commits on the branch now carry Signed-off-by. Note: the DCO check will still show "queued" — the DCO app appears inert repo-wide (check suites never execute on any PR), so verification is by trailer inspection.
1	disconnectAll stops on first missing connection	45c45af1	Per-name try/catch; doStop's tail always runs.
2	UTF-8 split across socket chunks	45c45af1	setEncoding("utf8") on both clients.
3	Piped form/elicitation answers dropped	a606e39d	As-built: a persistent PromptReader shared across rounds (one readline interface for the whole command), so buffered lines survive between questions and across elicitation rounds. Your printf repro is now a test.
4	Empty-lock race	45c45af1	Empty/unparsable lock younger than a grace period is treated as held, with retry; constants named and tested.
7	esbuild GHSA-g7r4-m6w7-qqqr	e3b37226	"esbuild": "^0.28.2" override added, lockfile refreshed; resolves 0.28.x.

Should fix

#	Finding	Commit	Notes
5	logging/tail prints [object Object]	2cabc86c	Non-string data renders as JSON.
6	Stale parked-elicitation subscriber	138dd59b	Verified real. As-built: park teardown owns the unwire — ParkedCall carries the unwire handle, and expiry, cancel(), and cancelForConnection (same hole, not cited) detach the subscriber immediately rather than waiting on the outcome's finally. Regression test: an expired park's dead channel can no longer swallow a new call's elicitation.
8	Eval matcher ignores --flag=value	8250d0bd	Inline = split before value/variadic handling; spelling matrix test extended.
Gate 1	No mcpdo smoke	3c3de07d	npm run smoke:mcpdo, wired into npm run smoke (so local:gate + CI run it). Goes beyond the sketch: hermetic sandbox (private daemon dir/token/storage/catalog), connect → tools/call → elicitation park + elicitation/respond round-trip → daemon status → disconnect → daemon stop with verified daemon exit.
Gate 2	skills/mcpdo/SKILL.md unvalidated	8250d0bd	verify:skills now validates shipped skills/*/SKILL.md frontmatter (the unquoted-#/: truncation class) — frontmatter only; the eval/budget rules stay scoped to .claude/skills/.

Follow-up tier — all done anyway

#	Finding	Commit	Notes
9	ping while stopping → daemon_start_timeout	138dd59b	As-built to a deliberate UX rule: the daemon is conceptually always "up", so ping always succeeds and now carries a stopping flag (daemon status shows "(shutting down)"). ensureDaemon, on stopping: true, waits for the old process to exit (pid-based kill-0 poll — the socket closes before the lock is released, so socket-reachability is the wrong signal) and then spawns fresh. Observing a shutdown never restarts the daemon. Integration test covers the full wait-then-respawn path.
10	Already-aborted signal leaks the socket	45c45af1	Abort checked before connect(); no un-destroy.
11	auth-helper exit vs close	2cabc86c	Listens on close; final buffered stdout line is read.
12	C1 controls on the failure path	2cabc86c	sanitizeText() applied to server-influenced text in failure paths (auth-helper event messages, tool-name interpolations); error-path test added.
13	Eval shim process.exit truncation	8250d0bd	process.exitCode + stdin.destroy() instead of process.exit().
14	Eval pool teardown	3c3de07d	pool() takes an optional onError mapper: the behavior section records an errored sample as a scored miss and keeps going; without onError the pool stops taking new items, lets in-flight cleanup finish, then rethrows. makeBehaviorEnv throw now reclaims the sandbox. The trigger section deliberately keeps rethrow semantics: faking invoked: false for an errored sample would falsely pass negative cases, and its finally already reclaims the shared sandbox. Pool semantics unit-tested.
15	Bedrock/Vertex env allowlist	8250d0bd	AWS_, GOOGLE_, CLOUD_ML_ prefixes pass through; the test that asserted the opposite was updated.
Gate 3	Coverage-exclusion comment	2cabc86c	Justification comment on the array, matching web's pattern.
1a note	Private-mode trust model doc line	8250d0bd	README now states it separates daemons from each other, not from same-UID processes that know the dir.

Also refreshed: the PR description's test count (now 417 Vitest tests across 30+ files, plus the eval-harness node:test units and the new smoke), and the SKILL.md elicitation wording (the command returns immediately; the tool call stays parked — never wait on or time-box the command itself).

Commits, oldest first: e3b37226 (esbuild), 45c45af1 (1/2/4/10), a606e39d (3), 2cabc86c (5/11/12/Gate 3), 8250d0bd (8/13/15/Gate 2/doc lines), 138dd59b (6/9), 3c3de07d (14/Gate 1).

[Copilot]

Copilot review overview

🟡 Changes recommended

Windows evaluation drops mixed-case environment keys, and several shipped usage and design documents contradict actual CLI behavior.

Review effort: Balanced
Findings: 1 · 3

Open (4)

• Preserve mixed-case Windows environment keys · New
• Pass container options after the connect command separator · New
• Remove the completed RPC serialization to-do · New
• Update obsolete elicitation behavior documentation · New

[Copilot]

Copilot review overview

🔵 Needs a closer look

The shipped agent skill recommends --decline for URL elicitations even though the daemon rejects that action, and the architecture guide retains retired session terminology.

Review effort: Balanced
Findings: None

Resolved since last review (4)

• Preserve mixed-case Windows environment keys
• Update obsolete elicitation behavior documentation
• Remove the completed RPC serialization to-do
• Pass container options after the connect command separator

Previously missed (1)

In code that hasn't changed since last review

Architecture guide still uses outdated session terminology

AGENTS.md:4

The PR explicitly renamed this surface and its user-facing vocabulary from “session” to “connection,” but the repository’s primary architecture guide still introduces mcpdo as the session CLI. This makes the canonical terminology internally inconsistent.

[BobDickinson]

Copilot round-38 overview advisories (no threads to reply into): both fixed in 205f6f4 — the mcpdo skill now ends a URL-mode elicitation with --done or --cancel (the daemon rejects --decline there), and AGENTS.md introduces mcpdo as the connection CLI, matching the session→connection rename.

[Copilot]

Copilot review overview

🔵 Needs a closer look

OAuth helpers can contend for the fixed callback port, and one evaluation assertion is currently ineffective.

Review effort: Balanced
Findings: None

Previously missed (1)

In code that hasn't changed since last review

Serialize OAuth helpers sharing the same callback endpoint

clients/​daemon-cli/​src/​connection/​auth-helper.ts:334

The reservation is scoped per server URL, but every helper uses the same default loopback callback (127.0.0.1:6276). Concurrent non-TTY connects to two different OAuth servers (or from shared/private daemons) therefore acquire different locks, start two helpers, and one fails to bind the callback port. Serialize helpers by callback endpoint in a user-shared location (while keeping URL-specific markers), or allocate distinct callback ports where registration permits.

[BobDickinson]

Copilot round-39 came back with no findings, which closes out the automated review loop (rounds 37–39: four findings fixed in bee94ef, two doc advisories fixed in 205f6f4, then clean).

One overview-only advisory in the final round — serializing concurrent OAuth helpers that share the default loopback callback port (auth-helper.ts) — is declined as won't-fix: it only bites when two non-TTY connects to two different OAuth servers run their browser flows simultaneously on one machine, and serializing helpers by callback endpoint is a design change beyond this PR's scope. The eval harness already avoids the collision with ephemeral callback ports.

@cliffhall this is ready for another look — every item from your 2026-09-30 round is addressed (per-item disposition in the comment above), head is 205f6f49, CI green.