cli: Wave 3 — programmatic review path (rollup of #1573–#1575)

[cliffhall]

Closes #1573
Closes #1574
Closes #1575

Wave 3 rollup — CLI programmatic review path

Rollup of the three Wave 3 sub-issues from the #1579 decomposition of PR #1510, so the whole CLI lane can be smoke-tested and merged as one cumulative diff against v2/main. This branch (v2/1575-cli-stored-auth) already contains all three merged down base→tip, each run through the @claude review loop to clean with npm run ci green.

Included (base→tip):

• cli: programmatic ergonomics — --method initialize, --tool-args-json, --connect-timeout, env-var fixes #1573 programmatic ergonomics — --method initialize, --tool-args-json, --connect-timeout, env-var fixes (incl. core getStateFilePath() MCP_STORAGE_DIR support)
• cli: --app-info probe + --format json envelope #1574 --app-info probe + --format json envelope
• cli: stored-auth suite — --use-stored-auth, --wait-for-auth, --list-stored-auth, --print-handoff #1575 stored-auth suite — --use-stored-auth, --wait-for-auth, --list-stored-auth, --print-handoff

Interim/follow-ups tracked separately: --print-handoff deep-link format (#1666, needs #1576) and refresh_token injection (#1665).

The three per-issue PRs (#1651, #1653, #1655) are superseded by this rollup; #1655 is repurposed as the rollup. Per AGENTS.md the Closes lines won't auto-fire on v2/main — the sub-issues will be closed and their board cards moved to Done on merge.

🤖 Generated with Claude Code

[cliffhall]

@claude review

[claude]

Claude finished @cliffhall's task in 2m 20s —— View job

---

Review — PR #1655: CLI stored-auth suite

• Read the PR diff (cli.ts, tests, README)
• Review cli.ts logic (arg parsing, stored-auth injection, wait/list/handoff)
• Check tests & coverage
• Post review feedback

Overall this is a clean, well-scoped addition. The four flags are cohesive, the state-path resolution correctly reuses core getStateFilePath(), the store shape (servers[url].tokens.access_token) matches ServerOAuthState, and the parseArgs → ParseResult union is a tidy way to model the short-circuit modes. Docs are thorough and honest about the interim/blind-injection tradeoffs. No blocking issues found. A handful of notes below.

---

1. --print-handoff deepLink doesn't carry the token the web app reads (interim, worth double-checking)

clients/cli/src/cli.ts:558-559

const params = new URLSearchParams({ serverUrl, transport: "http" });
if (apiToken) params.set("autoConnect", apiToken);

Per CLAUDE.md ("Web backend auth token"), the browser recovers the API token from window.__INSPECTOR_API_TOKEN__, ?MCP_INSPECTOR_API_TOKEN=…, or sessionStorage — not from an autoConnect param. As written, opening this deepLink on a fresh session (no injected token) would hit an unauthenticated /api/* and fail, since autoConnect=<token> isn't one of the three recognized sources. You've flagged this as interim pending #1576, and there's a note when the token is unset — so this is a "confirm the reconciliation" item, not a defect. Worth an explicit // TODO(#1576) on this line so it isn't forgotten. Also transport: "http" is hardcoded even when the real server is SSE — same interim bucket.

2. Blind injection ignores a stored refresh_token

clients/cli/src/cli.ts:498-507 / 957-960 — the token is injected as-is. ServerOAuthState.tokens (OAuthTokens) can also carry a refresh_token, so an expired access token surfaces as a 401 (documented, exit 3) even when a silent refresh was possible. Acceptable for a prototype and clearly documented, but a natural follow-up: fall back to a refresh when the access token is expired (connection-state.ts already has isJwtExpired).

3. Minor test gap: --wait-for-auth without --server-url

clients/cli/src/cli.ts:924-929 shares the missing---server-url guard between both flags via a ternary. Tests cover the --use-stored-auth arm (stored-auth.test.ts:103) but not the --wait-for-auth arm, so the "--wait-for-auth" branch of that message ternary is untested. A one-line test would close it and lock the branch coverage. Fix this →

4. Conflicting short-circuit flags resolve silently

--list-stored-auth is checked before --print-handoff (cli.ts:880-898), so passing both silently runs the listing; likewise both ignore any other flags without complaint. Fine as-is, but a note in the README that these modes are mutually exclusive and take precedence would preempt confusion.

Nits (non-blocking)

• readOAuthServers does a dynamic import("node:fs/promises") on every poll iteration (cli.ts:484). Node caches the module so cost is negligible — just flagging it's re-imported per 500 ms tick.
• waitForStoredToken computes const key only for the timeout message (cli.ts:521); fine, just slightly eager.
• Empty-string MCP_INSPECTOR_OAUTH_STATE_PATH correctly falls through to MCP_STORAGE_DIR (verified against getStateFilePath), and the handoff test relies on this — good, subtle, and covered.

Nice work — the tests exercise real injection over an in-process HTTP server and the URL-normalization matching, which is exactly where this class of feature tends to break.
· branch v2/1575-cli-stored-auth

[cliffhall]

Thanks — applied 3 of the 4 substantive findings, declined #2 as a documented follow-up. Details:

1. --print-handoff deepLink / autoConnect param (interim) — applied a // TODO(#1576). Added an explicit inline TODO(#1576) on the buildHandoff params, calling out both that transport:"http" is hardcoded even for SSE servers and that autoConnect=<token> is not one of the three token sources the web app reads (per CLAUDE.md). Both to be reconciled with #1576's canonical format. The unset-token note already warns the user at runtime.

2. Blind injection ignores refresh_token — declined (documented follow-up). Agreed it's the right eventual behavior, but minting a fresh token from a stored refresh_token pulls in the SDK auth-provider refresh path and is beyond this PR's "reuse an existing token" scope. Recorded it as an explicit follow-up in both the injection-site comment and the README so it isn't lost.

3. Test gap: --wait-for-auth without --server-url — applied. Added a test asserting that arm of the shared guard's message ternary ("--wait-for-auth requires --server-url"), which was previously only covered for the --use-stored-auth arm.

4. Conflicting short-circuit flags resolve silently — documented. Added a README note that --list-stored-auth / --print-handoff are mutually exclusive short-circuit modes and that --list-stored-auth takes precedence. Left the precedence behavior as-is (erroring on the combo felt heavier than clarifying it).

Nits (dynamic import("node:fs/promises") per poll, eager key in waitForStoredToken) — left as-is; both are negligible and you flagged them as non-blocking. Empty-string MCP_INSPECTOR_OAUTH_STATE_PATH fall-through is intentional and covered.

npm run ci is green. (This branch also carries the #1651 and #1653 review fixes via merges from the parent branches.)

[cliffhall]

@claude review

[claude]

Claude finished @cliffhall's task in 53s —— View job

---

Re-review — PR #1655: CLI stored-auth suite

• Read the follow-up commit (133b9041) addressing the prior review
• Review cli.ts stored-auth logic
• Review tests & README changes
• Post review feedback

Verified the follow-up commit against my earlier review. All four substantive items are resolved as described, and I found no regressions or new issues in the delta. This looks ready to merge.

---

Prior findings — verification

1. --print-handoff deepLink / autoConnect param — ✅ resolved. The explicit TODO(#1576) at cli.ts:558-562 now calls out both interim gaps precisely: transport:"http" hardcoded even for SSE, and autoConnect=<token> not being one of the three token sources the web app reads (window.__INSPECTOR_API_TOKEN__ / ?MCP_INSPECTOR_API_TOKEN / sessionStorage, per CLAUDE.md). Cross-referenced against buildHandoff and the runtime note — an unset-token caller is still warned at runtime. Good.

2. Blind injection ignores refresh_token — ✅ recorded as follow-up (correctly declined for scope). The injection-site comment (cli.ts:938-941) and README (clients/cli/README.md:203) now both note that a stored refresh_token is not yet used and that this is the natural next step. Agreed this is beyond "reuse an existing token" scope.

3. Test gap: --wait-for-auth without --server-url — ✅ closed. stored-auth.test.ts:112-121 now asserts the --wait-for-auth arm of the shared guard's message ternary ("--wait-for-auth requires --server-url"), which was previously covered only for the --use-stored-auth arm. The two arms of that ternary are now both exercised.

4. Conflicting short-circuit flags — ✅ documented. clients/cli/README.md:205 adds the "Short-circuit modes" note: both flags print-and-exit without connecting, ignore method/target flags, are mutually exclusive, and --list-stored-auth takes precedence. Behavior left as-is, which matches the check order at cli.ts:885-903.

Nits (dynamic import per poll, eager key in waitForStoredToken) left as-is — as expected, both were flagged non-blocking.

---

Nothing further from me. The delta is comment/test/docs-only with no behavior change, and coverage holds (cli.ts remains 100% stmts/funcs/lines per the PR notes). Nice, tidy close-out.
· branch v2/1575-cli-stored-auth

[cliffhall]

Thanks for the tidy close-out. No further changes on this PR.