web+cli+docs: Wave 4 — deep-link auto-connect + programmatic MCP App review (rollup of #1576–#1578, #1665, #1666)

[cliffhall]

Closes #1576
Closes #1577
Closes #1578
Closes #1665
Closes #1666

Wave 4 rollup — deep-link auto-connect + programmatic MCP App review path

Tracking issue: #1675. Rollup of the five Wave 4 sub-issues from the #1579 decomposition of PR #1510, so the whole programmatic-review surface (URL deep-link → rendered widget, stored-auth handoff with refresh) can be smoke-tested and merged as one cumulative diff against v2/main. Each sub-issue shipped and was reviewed on its own branch (@claude review → LGTM), then merged base→tip into this branch.

Included (base→tip)

• web: deep-link auto-connect (?serverUrl&transport&autoConnect) + machine-readable connection status #1576 — web: deep-link auto-connect ?serverUrl&transport&autoConnect. parseDeepLink() (http(s)-only serverUrl; autoConnect CSRF-gated on the per-launch API token; stable deep-link catalog row). App.tsx phase-split ensure→update→connect effect; connectErrorMessage. InspectorView header data-testid="connection-status" with data-status/data-error-message/data-deeplink.
• web: deep-link openApp/appArgs/autoOpen — pre-select and pre-fill an app from the URL #1577 — web: deep-link openApp/appArgs/autoOpen. Once connected + the tool appears, switch to Apps, pre-select, pre-fill appArgs merged over schema defaults; token-gated autoOpen fires "Open App" (AppsScreen owns running, so autoOpen threads to it, fire-once).
• cli: reconcile --print-handoff deep-link with #1576 canonical format #1666 — cli: reconcile --print-handoff with web: deep-link auto-connect (?serverUrl&transport&autoConnect) + machine-readable connection status #1576's canonical deep-link format (derived transport, normalized serverUrl, autoConnect gate; dropped the interim TODO(#1576)).
• cli: stored-auth injection should honor refresh_token, not just access_token #1665 — cli: --use-stored-auth honors refresh_token — SDK refresh grant + rotation persisted via writeStoreFile (0o600), with fallback to the stored access token when refresh fails.
• docs: MCP App review recipe + README updates for new CLI flags, deep-links, and proxy support #1578 — docs: docs/mcp-app-review.md CLI-first → one-shot-web recipe + per-client README updates.

Verification

• npm run ci green on this branch (validate → coverage ≥90 per-file gate → smoke → Storybook). Coverage gate passes for every touched file (deepLink.ts, InspectorView.tsx, AppsScreen.tsx, cli.ts).
• Each sub-PR (web: deep-link auto-connect (?serverUrl&transport&autoConnect) + connection-status (#1576) #1670, web: deep-link openApp/appArgs/autoOpen — pre-select and pre-fill an app from the URL (#1577) #1671, cli: reconcile --print-handoff deep-link with #1576 canonical format (#1666) #1672, cli: stored-auth injection should honor refresh_token, not just access_token (#1665) #1673, docs: MCP App review recipe + README updates (#1578) #1674) went through the @claude review loop to LGTM; those per-issue PRs are superseded by this rollup.

Per AGENTS.md the Closes lines won't auto-fire on v2/main — the sub-issues are closed and their board cards moved to Done on merge of this rollup; #1675 stays In Review behind it.

🤖 Generated with Claude Code

[cliffhall]

@claude review

[claude]

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

---

Review: Wave 4 rollup (#1676)

• Read the PR diff against origin/v2/main
• Review web deep-link (deepLink.ts, App.tsx, InspectorView.tsx, AppsScreen.tsx)
• Review CLI changes (cli.ts, stored-auth refresh)
• Review docs & READMEs
• Post review feedback

---

Reviewed the full cumulative diff. This is a well-executed rollup — the security gating is thoughtfully documented, the CLI refresh path reuses the SDK rather than hand-rolling a token request, and the machine-readable data-* contract is a clean automation surface. Each piece was already LGTM'd on its own branch, and it shows. Comments below are minor/nits; nothing blocking.

Strengths

• parseDeepLink CSRF gating is solid. autoConnect/autoOpen must both equal the per-launch API token; serverUrl is restricted to http(s): via validateServerUrl, and decodeAppArgs fails closed to {} on any atob/JSON error. The security rationale is captured in-code rather than left implicit.
• CLI refresh reuses refreshAuthorization + writeStoreFile (0o600). Persisting rotation through the shared store writer keeps web/CLI on-disk format identical, and the graceful fallback to the stored access token when refresh fails avoids turning a transient auth-server hiccup into a hard failure. The distinct error codes (no_stored_token / no_client_information / refresh_failed) are a nice touch for automated callers.
• onToggleConnection now reads serversRef.current instead of the closure's servers, and drops servers from its deps — this correctly fixes the stale-list race the deep-link IIFE would otherwise hit, and stabilizes the callback identity.
• Test coverage is thorough (transport derivation, refresh end-to-end, fallback, each error code) and the data-* contract is documented in clients/web/README.md with a matching anchor for the docs cross-link.

Minor observations (non-blocking)

1. Deep-link connect is skipped when the deep-link server is already activeServerId but disconnected — App.tsx (connect phase):

deepLinkConnectRef.current = true;
if (activeServerId !== deepLink.serverId) {
  void onToggleConnection(deepLink.serverId).catch(...);
}

If activeServerId is persisted/restored to "deep-link" from a prior session while the connection is disconnected, this marks the ref consumed and never connects, so a reload of the same deep-link URL silently won't auto-connect. The guard is there to avoid toggling off an already-live connection — but the intent is "connect if not connected," not "do nothing if this is the active id." Consider gating on connectionStatus/activeServer.connected rather than on activeServerId identity. Low likelihood (requires activeServerId to survive a reload as deep-link), hence non-blocking.

2. Double snapshot read on the refresh path. In parseArgs, findStoredServerState is computed from readOAuthServers(...), then refreshStoredAuthToken() calls readOAuthSnapshot(...) again internally. Correct (the second read is the authoritative one it writes back), just a redundant file read. Not worth restructuring unless you're already touching it.

3. openApp without autoOpen bypasses the no-input auto-launch. When a deep link pre-selects an app via InspectorView → onAppsUiChange/onSelectApp, it bypasses AppsScreen.handleSelect, whose no-input-app branch auto-launches on selection. So a no-input app deep-linked with autoOpen=false lands pre-selected but not running (the user must click Open App), which differs from clicking it in the sidebar. Arguably fine — but a UX inconsistency worth a comment or a deliberate decision.

4. Doc/verification note. The PR body states npm run ci is green and typecheck passes. I could not independently run the SDK-typed refreshAuthorization call locally (deps not installed in this checkout), so I'm trusting CI on the metadata?: OAuthMetadata | undefined and second-arg shape. Worth a glance that the green run predates the final tip commit of this branch.

None of these rise to a change request. Nice work threading the deep-link phases and the stored-auth refresh together cleanly.
· branch v2/1579-wave4-rollup

[cliffhall]

Thanks for the thorough rollup review. Addressed the one correctness edge case in 7411fc88:

• Progress notifications #1 (reload doesn't reconnect when activeServerId restored to deep-link while disconnected): the connect phase now gates on !(activeServerId === deep-link && connectionStatus === "connected") instead of id identity alone, so a reload of the same deep-link URL connects whenever it isn't already connected — while still never toggling a live connection off.
• Completion requests #3 (no-input app pre-selected but not launched without autoOpen): added a comment documenting this as deliberate and security-aligned — a crafted URL must not fire a tool call against the target server without the token-gated autoOpen, so pre-select-without-launch is the intended behavior even for no-input apps (it bypasses handleSelect's auto-launch on purpose).

Dispositions on the rest:

• Request logging #2 (double snapshot read on refresh): left as-is — the second read inside refreshStoredAuthToken is the authoritative one it writes back, and de-duping would thread state through for a negligible saving, as you noted.
• SSE transport support #4 (verification): npm run ci was run on the fully-merged rollup tip and is green (per-file ≥90 coverage gate incl. deepLink.ts/InspectorView.tsx/AppsScreen.tsx/cli.ts, smoke, Storybook). This tip commit re-runs CI on push. Also manually smoke-tested end-to-end: CLI --print-handoff/--app-info, and a live browser deep-link (?serverUrl&transport&autoConnect&openApp&autoOpen) reaching data-status=connected + data-app-status=ready with zero clicks, plus the wrong-token → data-deeplink=rejected path.

Leaving this rollup unmerged for maintainer review.