chore: replace main's tree with v2 - #1830

* docs(AGENTS): board tracks issues only, never PRs

Reconcile a contradiction in the board workflow: line 109 said every board
item is a real issue, but the "created" callout and the add-to-board bullet
said "issue or PR." Make it consistent — only issues go on board #28; a PR
still gets the v2 label but is tracked through its linked issue's card via
Closes #N.

Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
Claude-Session: https://claude.ai/code/session_01Cb5xVMkUUgpxwBsmZSQyRm

* docs(AGENTS): use <issue-url> placeholder in board one-liner recipe

Address PR review nit: make the one-liner's placeholder <issue-url> to
match the two-step recipe, reinforcing that only issues go on the board.

Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
Claude-Session: https://claude.ai/code/session_01Cb5xVMkUUgpxwBsmZSQyRm

---------

Co-authored-by: Claude Opus 4.8 (1M context) <[email protected]>

)

Give the CLI a stable machine-readable failure surface:

- EXIT_CODES map (0 ok / 1 usage / 2 no-app / 3 auth-required /
  4 unreachable / 5 tool-error)
- ErrorEnvelope interface + CliExitCodeError for pre-classified exits
- pure classifyError() (status/cause/pattern heuristics) and
  formatErrorOutput()/handleError() that emit one JSON line on stderr
- in-process cli-runner mirrors the binary via formatErrorOutput so tests
  observe the real exit code and envelope
- document the exit-code map + envelope shape in clients/cli/README.md

Wave 1 foundation of the PR #1510 decomposition; shapes kept clean for
reuse by --app-info (exit 2), stored-auth (exit 3), and --format json.

Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
Claude-Session: https://claude.ai/code/session_01S3fTN8H3R8YV4yUGvZjYnX

Redact Authorization, Cookie, Set-Cookie, Proxy-Authorization, X-Api-Key,
and x-mcp-remote-auth to [REDACTED] in createFetchTracker before any
request/response entry reaches the in-memory log, pino logger, or
persisted session storage. Comparison is case-insensitive; original key
casing is preserved and the live outbound request is untouched.

Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
Claude-Session: https://claude.ai/code/session_01S3fTN8H3R8YV4yUGvZjYnX

…#1556)

Add a machine-readable summary of an MCP App's UI metadata: the AppInfo
interface plus extractAppInfo(), which merges a tool's _meta.ui
(resourceUri, visibility) with the linked UI resource's _meta.ui (csp,
permissions, domain, prefersBorder, mimeType). Content-block matching is
exact-URI first, then a lowercase + trailing-slash normalized match, then
a single-block fallback since resources/read returns the requested
resource by definition. Shared plumbing for the programmatic-review path
(CLI --app-info, integration tests) without rendering the app.

Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
Claude-Session: https://claude.ai/code/session_01S3fTN8H3R8YV4yUGvZjYnX

…Uri, isHttpUrl (closes #1560)

Extend the browser download library beyond JSON so the Apps host's
ui/download-file support can download arbitrary embedded resources and
open http(s) links safely, with no UI wiring in this change:

- downloadBlob(): temp-anchor download core with a setTimeout(...,0)-
  deferred revokeObjectURL so a synchronous revoke can't abort the
  scheduled download (Firefox/Safari, intermittently Chrome).
- downloadJsonFile(): refactored on top of downloadBlob; callers unchanged.
- fileNameFromUri(): last path segment sanitized (control/format chars
  stripped, disallowed filename chars -> _, capped at 255, "download"
  fallback).
- isHttpUrl(): parse-and-allowlist http(s) only, else null.

Re-implementation of the downloadFile.ts slice of PR #1510 as Wave 1 of
the #1579 decomposition.

Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
Claude-Session: https://claude.ai/code/session_01S3fTN8H3R8YV4yUGvZjYnX

…closes #1557)

Add an `mcp_app_demo` tool/resource preset — a self-contained MCP App widget
that exercises size-changed, ui/message, log notifications, and host-context
rendering with no external server. Requires plumbing an optional `_meta` field
through the composable test server's `ToolDefinition` and `ResourceDefinition`
so clients can read tool-level `_meta.ui.resourceUri` and resource-level
`_meta.ui.csp`.

- composable-test-server: optional `_meta` on ToolDefinition/ResourceDefinition,
  passed through to registerTool config and the resource content item.
- test-server-fixtures: `createMcpAppDemoTool()`, `createMcpAppDemoResource()`,
  inline `MCP_APP_DEMO_HTML` widget; wired into getDefaultServerConfig.
- preset-registry: `mcp_app_demo` tool + `mcp_app_demo_widget` resource presets.
- integration test asserting the `_meta` plumbing end-to-end.

Wave 1 of PR #1510 decomposition.

Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
Claude-Session: https://claude.ai/code/session_01S3fTN8H3R8YV4yUGvZjYnX

)

Add clients/web/src/components/elements/AppRenderer/hostContext.ts — a pure
utilities module for the MCP Apps host:

- currentTheme(): read the resolved Mantine color scheme from the DOM
- currentStyles(): map the inspector's Mantine/-inspector design tokens to a
  McpUiHostStyles snapshot via STYLE_VARIABLE_SOURCES
- measureContainerDimensions(): whole-pixel container measurement
- snapshotHostContext(): assemble the initial McpUiHostContext seed

Wave 1 of the PR #1510 decomposition (#1579). Pure extraction only — the
AppRenderer/AppsScreen/createAppBridgeFactory rewrites belong to Wave 2.
Full unit coverage (100% lines/statements/functions/branches).

Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
Claude-Session: https://claude.ai/code/session_01S3fTN8H3R8YV4yUGvZjYnX

Add a pure, DOM-free CSP-builder library at
clients/web/src/lib/sandbox-csp.ts that validates app-supplied
`_meta.ui.csp` requests, filters them to safe sources, builds the
locked-down Content-Security-Policy string, and wraps untrusted widget
HTML in a fixed host shell whose first <head> child is the CSP meta.

- SAFE_CSP_SOURCE: strict regex that rejects directive/attribute
  breakouts (`;`, `"`, `<`, `>`, whitespace).
- approveCspSources(): drops unsafe entries (with a warning) and omits
  empty keys; echoes back only what the host will enforce.
- buildSandboxCspPolicy(): `default-src 'none'` catch-all, `form-action
  'none'`, source-allowlist filtering, `'unsafe-inline'` for the app's
  own inline script/style.
- escapeHtmlAttr() / wrapSandboxedHtml(): defense-in-depth so untrusted
  bytes never precede the applied policy.

Re-implements the sandbox-csp portion of PR #1510 (Wave 1 of #1579) as a
standalone library; wiring into the app bridge is a follow-up issue.
Adds full unit tests (100% lines/statements/functions/branches) and
lists the file in the web coverage `include` so it is gated (the
untested legacy src/lib siblings stay out until they get tests).

Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
Claude-Session: https://claude.ai/code/session_01S3fTN8H3R8YV4yUGvZjYnX

…oses #1562)

generateOAuthState() now throws when crypto.getRandomValues is unavailable
instead of silently degrading to a non-cryptographic Math.random() fallback —
OAuth state is a CSRF token and must be unpredictable.

The web /oauth/callback handler now rejects a returned `state` param that does
not parse to the expected 64-char-hex authId shape (a forgery indicator),
surfacing a clear error toast instead of proceeding with an undefined session.

Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
Claude-Session: https://claude.ai/code/session_01S3fTN8H3R8YV4yUGvZjYnX

…rt (closes #1563)

Wrap the Node transport's fetch with an undici EnvHttpProxyAgent dispatcher
when a standard proxy env var is set, so the CLI and web backend can reach
remote MCP servers through corporate proxies with zero new flags. undici is
imported lazily (only when a proxy var is present) and added as a dependency
at the repo root and in the CLI client; a missing undici raises an actionable
error. Documents the behavior and dependency in the CLI README.

Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
Claude-Session: https://claude.ai/code/session_01S3fTN8H3R8YV4yUGvZjYnX

…ge parity) (closes #1548)

Migrate the web client's OAuth storage from BrowserOAuthStorage
(sessionStorage) to RemoteOAuthStorage, which POSTs through the backend's
/api/storage/oauth route to ~/.mcp-inspector/storage/oauth.json (mode 0600).
A credential obtained in the browser is now the same blob the TUI/CLI read
on the same host, giving web ⇄ TUI ⇄ CLI parity (Wave 1 of #1579).

Core (shared with TUI/CLI):
- OAuthStorageBase now drives a single async hydration (`ready()`,
  `getHydrationError()`); every post-redirect read (getClientInformation,
  getTokens, getCodeVerifier, getServerMetadata, getIdpSession) and every
  save awaits it so a late hydration merge cannot clobber a fresh write.
  getCodeVerifier/getServerMetadata become async; the store is created with
  skipHydration:true so there is no auto-hydration to race the explicit one.
- store.ts adds normalizeServerUrl so a token saved under
  https://Example.com/mcp is found when the CLI asks for
  https://example.com/mcp/; getServerState falls back to the raw key.
- remote-storage adapter POSTs with keepalive so the write survives the
  OAuth authorize redirect, surfaces swallowed persist failures, and gives
  richer read/write error messages.
- generateOAuthState throws instead of silently degrading to Math.random.
- NodeOAuthStorage honors MCP_INSPECTOR_OAUTH_STATE_PATH for isolated
  fixtures. Provider codeVerifier()/getServerMetadata() + the EMA idpOidc and
  connection-state readers await the now-async storage.

Web:
- environmentFactory + a new shared lib/remoteOAuthStorage accessor
  (memoized per {baseUrl, authToken}) wire RemoteOAuthStorage into the
  connection path, the EMA IdP hook, and the per-server "clear OAuth" action
  so all three share one in-memory view of oauth.json.

Tests: async-hydration + normalizeServerUrl suite, keepalive/error-path
coverage, and the sync→async migration across existing storage/provider/EMA
tests. Web validate + coverage gate + integration suite green; CLI/TUI/
launcher validate green.

Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
Claude-Session: https://claude.ai/code/session_01S3fTN8H3R8YV4yUGvZjYnX

Address @claude review of #1583:

- statusOf() now only treats a numeric `.code` as an HTTP status when it
  is in the 100-599 range, so an MCP SDK McpError JSON-RPC code (e.g.
  -32601 MethodNotFound) is no longer leaked into the envelope `status`
  or misclassified as AUTH_REQUIRED (it falls back to USAGE).
- causeOf() now carries a depth cap (MAX_CAUSE_DEPTH) so a cyclic/
  self-referential `error.cause` chain terminates instead of recursing
  infinitely.
- Add tests for both: a -32601 McpError-shaped error (status undefined,
  exit USAGE) and a self-referential cause chain (finite string).

Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
Claude-Session: https://claude.ai/code/session_01S3fTN8H3R8YV4yUGvZjYnX

Address code-review DRY note on PR #1587: MCP_APP_DEMO_URI was defined
in both the fixture and the integration test. Export it from the fixture
and import it in the test so the two stay in lockstep.

Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
Claude-Session: https://claude.ai/code/session_01S3fTN8H3R8YV4yUGvZjYnX

…asymmetry, tighten test

- Soften the OAuth-callback-rejected toast: the shape guard is
  defense-in-depth, not full CSRF prevention (PKCE remains primary), so the
  message no longer overstates ("rejecting to prevent a cross-site request" ->
  "did not originate from this session").
- Document the intentional asymmetry between a present-but-malformed `state`
  (rejected) and a wholly absent `state` (accepted, matched via
  OAUTH_PENDING_SERVER_KEY) — rejecting the null case would mask real provider
  error redirects that omit `state`.
- Make the "valid state is not rejected" test assert the specific downstream
  "could not be matched" toast (seeding OAUTH_PENDING_SERVER_KEY) instead of an
  indirect "some toast fired" check.

Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
Claude-Session: https://claude.ai/code/session_01S3fTN8H3R8YV4yUGvZjYnX

…edence

Address round-1 review nits on apps.ts:
- readUiMeta now accepts `unknown` and narrows internally, removing the
  three `as WithUiMeta` casts at its call sites (readability win).
- Add a comment documenting that the content-block-then-result precedence
  for `_meta.ui` is intentional (posture lives on the content block; no
  shallow merge).
- Add a TODO to switch to the named McpUiToolMeta/McpUiResourceMeta types
  once the upstream extensionless re-export is fixed for NodeNext.

Coverage: apps.ts 100% lines / 97.05% branch / 100% funcs — clears the gate.

Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
Claude-Session: https://claude.ai/code/session_01S3fTN8H3R8YV4yUGvZjYnX

undici 8.5.0 requires Node >= 22.19.0 (its declared engines), so CI's
Node 20.x could not load undici's CacheStorage (missing
webidl.util.markAsUncloneable) and the proxy integration tests failed.
Bump the CI setup-node to 22.x to match the repo's Node floor.

Reconcile the declared floor with undici's real requirement: bump root
engines.node to >=22.19.0 and correct the CLI README note (undici 8.5.0
needs >=22.19.0, not >=22.7.5). Also document proxy support in the web
README and clarify that a caller-supplied eventSourceInit.fetch bypasses
the proxy wrapper by design.

Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
Claude-Session: https://claude.ai/code/session_01S3fTN8H3R8YV4yUGvZjYnX

Address round-1 @claude review of #1588 (no behavior change to the
enforced policy):

- approveCspSources / buildSandboxCspPolicy: document that
  resourceDomains intentionally feeds script-src/style-src per the
  McpUiResourceCsp contract, and that approve screens injection-safety
  only (not breadth — a bare `*` is accepted; safe under the
  opaque-origin sandbox).
- CSP_KEYS: replace the `satisfies` (proves listed keys valid) with an
  exhaustiveCspKeys() helper that ALSO fails to compile if the upstream
  ext-apps type gains a domain key CSP_KEYS omits, so a requested
  restriction can never be silently dropped.
- SAFE_CSP_SOURCE: accept case-insensitive schemes (URL schemes are
  case-insensitive); previously over-rejected `HTTPS://…`. Adds tests.

Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
Claude-Session: https://claude.ai/code/session_01S3fTN8H3R8YV4yUGvZjYnX

…n path

Address round-2 code-review note on PR #1587: the _meta passthrough was
wired only into the regular registerTool path, so an App-flavored task
tool would not surface tool-level _meta. Add _meta to TaskToolDefinition
and a matching conditional spread on both registerToolTask overload
configs so the two registration paths behave symmetrically.

Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
Claude-Session: https://claude.ai/code/session_01S3fTN8H3R8YV4yUGvZjYnX

…palive/memo caveats

Address round-1 @claude review of #1592:

- OAuthStorageBase.clear* no longer mutate the pre-hydration (empty) store —
  a clear issued before hydration is deferred until it lands, so it merges
  onto the real persisted state instead of (a) being resurrected by the late
  rehydrate merge or (b) persisting a near-empty blob that clobbers every
  other server's on-disk credential. Already-hydrated clears stay synchronous.
- store.setServerState migrates a pre-normalization raw-key blob onto the
  canonical key on first partial write (was: shadowed it with a fresh
  canonical entry, orphaning the raw blob's other fields). clearServerState
  now also drops the raw-key orphan.
- Document the keepalive 64KB combined-body ceiling on the remote-storage
  write, the getRemoteOAuthStorage memo key intentionally omitting fetchFn,
  and the real ordering invariant behind the sync getScope.
- Tests for the clear-before-hydration and raw-key-migration paths.

Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
Claude-Session: https://claude.ai/code/session_01S3fTN8H3R8YV4yUGvZjYnX

The root package.json engines.node was bumped to >=22.19.0 but the root
package entry in package-lock.json still declared >=22.7.5. Regenerate so
the lockfile matches package.json for npm ci.

Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
Claude-Session: https://claude.ai/code/session_01S3fTN8H3R8YV4yUGvZjYnX

Address round-3 code-review note on PR #1587: resource `_meta` is applied
only by the default read handler; a customHandler from onRegisterResource
replaces the contents wholesale and would drop it. Document this on
ResourceDefinition._meta so a future App UI resource with a custom read
handler knows to re-add _meta itself.

Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
Claude-Session: https://claude.ai/code/session_01S3fTN8H3R8YV4yUGvZjYnX

Round-2 @claude review of #1592 caught a seventh member of the clear
family that the first pass missed: clearClientInformation still wrote to
the store synchronously, carrying the same pre-hydration clobber risk the
clearAfterHydration helper was written for (a clear before hydration lands
persists a near-empty blob, overwriting the whole on-disk oauth.json).
Wrap its mutation the same way and add a clear-before-hydration test
proving a sibling token (and thus every other server's blob) survives.

Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
Claude-Session: https://claude.ai/code/session_01S3fTN8H3R8YV4yUGvZjYnX

… getScope

Round-3 @claude review of #1592 (optional nit): getClientRegistrationKind is
the other synchronous getter that, like getScope, is safe only because its
sole caller (buildOAuthConnectionState) awaits getClientInformation/
getServerMetadata first, flushing hydration before this runs on the
post-redirect callback path. Add a docstring mirroring getScope's so a future
refactor doesn't read it without a preceding awaited storage read.

Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
Claude-Session: https://claude.ai/code/session_01S3fTN8H3R8YV4yUGvZjYnX

Completes requirement #2 of #1517: a table mapping each active version to
its base branch, project board, and version label, plus a "Label by
version" note mirroring AGENTS.md.

Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
Claude-Session: https://claude.ai/code/session_01S3fTN8H3R8YV4yUGvZjYnX

…1596)

Root-cause and deterministically fix the recurring 5s-timeout flakes that
surfaced under v8-instrumented, concurrent load. All fixes are test-only; no
product source changed. No test was skipped or disabled, and the per-file
coverage gate (>=90 on all four dims) still holds.

Common root cause across the modal/form/screen suites: userEvent.setup()
schedules a real setTimeout between keystrokes. Under CPU contention those
yields balloon, so multi-field interactions blew past the 5s per-test
timeout. Fix: userEvent.setup({ delay: null }) removes the per-keystroke
real-timer dependence (typing dispatches synchronously), making the suites
load-independent.

Per file:
- ServerConfigModal / ServerImportConfigModal / ServerImportJsonModal /
  ResourcesScreen / InspectorView: userEvent.setup({ delay: null }).
- PromptArgumentsForm (completions): replaced wall-clock sleeps sized just
  past the 300ms completion debounce (await setTimeout(400)) — which race
  the debounce timer under load — with awaited conditions (findBy/waitFor)
  on the real rendered outcome, plus delay:null. Negative assertions now
  lean on the component's synchronous state/timer teardown instead of a
  timed window, so they are deterministic regardless of machine speed.
- inspectorClient integration "tracks stderr logs": the child's stderr is
  piped out-of-band from the tool's JSON-RPC response, so reading the log
  synchronously after callTool raced the stderr chunk. Wrapped the assertion
  in vi.waitFor so it polls until the line lands.

Verified: web validate + 6x test:coverage (4 sequential + 2 concurrent
under load) + standalone test:integration, all green (229 files / 3237
tests per coverage run; 799 integration tests), zero intermittent failures.

Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
Claude-Session: https://claude.ai/code/session_01S3fTN8H3R8YV4yUGvZjYnX

…ound 1)

The "cancels a pending debounce timer when the input is re-focused" test
passed trivially after the delay:null change: synchronous typing meant the
assertion ran at t≈0, before the 300ms debounce could ever fire, so it
passed whether or not handleFocus's clearTimeout worked.

Rewrote it with fake timers so the debounce window elapses deterministically
(advance the clock by 400ms directly). The interaction is driven with
fireEvent rather than userEvent because userEvent's async internals deadlock
under vitest fake timers with the Mantine/happy-dom stack. Verified
load-bearing: removing the clearTimeout in handleFocus makes it fail
(expected 1 to be 0), and it passes once restored.

Also tightened the sibling-context comment (with delay:null the sibling is
reliably "es"); the tolerant /^es?$/ regex is left as-is.

Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
Claude-Session: https://claude.ai/code/session_01S3fTN8H3R8YV4yUGvZjYnX

…d modules (#1784)

* refactor(web): sharpen src/lib vs src/utils rule and move misfiled modules (#1776)

Codify the empirical rule the web client already followed loosely — utils =
pure functions that compute; lib = things that instantiate, adapt, or touch
the environment — in AGENTS.md and clients/web/README.md, and move the modules
that sat on the wrong side of it.

Moves (source + co-located test):
- lib/sandbox-csp -> utils (pure CSP-string construction; type-only import)
- utils/browserTabVisibility -> lib (registers visibilitychange listeners)
- utils/oauthResume -> lib (reads/writes sessionStorage)
- utils/clearServerOAuthState -> lib (drives live InspectorClient / OAuthStorage)

Borderline audits (kept in place): pendingReauth (pure type declarations),
deepLink / correlateTransportErrors / mcpNetworkHeaders / oauthUx / oauthFlow /
oauthNetworkPhase (pure transforms / type-only core imports) stay in utils;
downloadFile stays whole in lib (dominant side is DOM side effects).

Also relocate the lone src/test/lib/downloadFile.test.ts straggler beside its
source at src/lib/downloadFile.test.ts, and document the test-placement rule
(side-by-side by default; src/test/ only for core/ tests, the integration
project, and shared scaffolding).

Pure move — no behavior change; no build/coverage config keys off the boundary.

Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
Claude-Session: https://claude.ai/code/session_01XNdjEPKLG637X8YmhDiEk5

* refactor(web): address review — fix stale paths, move customHeaders, one-way arrows (#1776)

Responds to the @claude review on PR #1784:

1. Stale path references (fixed): update the four in-repo pointers left at
   pre-move paths — specification/v2_auth_mid_session.md (oauthResume,
   browserTabVisibility → lib/), and the two load-bearing sandbox security
   comments (sandbox_proxy.html, sandbox-controller.ts → src/utils/sandbox-csp).

2. lib/types/customHeaders (moved): it's pure (types + pure transforms), the
   rule's clearest counterexample. Moved lib/types/customHeaders.{ts,test.ts} →
   utils/ (no external importers; only its own test) and removed the
   undocumented lib/types/ subdir. Documented that utils/ is the home for pure
   shared domain types + their constructors — no separate types/ bucket.

3. One-way arrows (item 3): declare OAuthResumeAuthKind in utils/pendingReauth
   and re-export it from lib/oauthResume, so the only cross-directory arrow now
   points lib → utils. Codified that invariant in AGENTS.md.

Wording nit: AGENTS.md + README now state diagnostic logging
(console.warn/error) doesn't count as a side effect for the utils rule.

Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
Claude-Session: https://claude.ai/code/session_01XNdjEPKLG637X8YmhDiEk5

* docs: address review round 2 — stale customHeaders refs, rule wording (#1776)

Doc-only follow-up to the round-2 @claude review on PR #1784 (no code changed):

1. Stale paths (same class as round-1 item 1): the customHeaders move left
   three references to the deleted lib/types/customHeaders.ts path in
   specification/ (v2_ux_interfaces_phase_0_4_inventory.md,
   v2_ux_interfaces_plan.md ×2) — updated to utils/customHeaders.ts.

2. customHeaders anchor: note in AGENTS.md that its shape is staged for
   ServerSettingsForm (per v2_ux_interfaces_plan.md), so its zero-importer
   state reads as intentional rather than rot; kept as the no-types/-bucket
   illustration and led the anchor list with load-bearing modules.

3. Type-only-import clause: split the conflated clause — a type-only import
   (pendingReauth) and re-exporting core's pure functions/constants
   (oauthUx/oauthFlow) are distinct reasons to stay in utils; what makes a
   module lib is wrapping core's stateful runtime. Fixed in both AGENTS.md
   and the web README.

Cosmetic: re-padded the v2_auth_mid_session.md table row to column width.

Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
Claude-Session: https://claude.ai/code/session_01XNdjEPKLG637X8YmhDiEk5

* docs: address review round 3 — scope the "no types/ bucket" claim (#1776)

Doc-only follow-up to the round-3 @claude review on PR #1784 (no code changed):

1. "no separate types/ bucket" overreached: clients/web/src/types/ does exist
   (navigation.ts + react-syntax-highlighter .d.ts ambient stubs wired via
   tsconfig.app.json paths). Scoped the claim in both AGENTS.md and the web
   README to "no types/ sub-bucket INSIDE lib/utils", and noted src/types/ is
   for ambient .d.ts stubs, not domain types.

2. README example list drifted from AGENTS.md's round-2 load-bearing anchors —
   realigned it (inspectorTabs/mcpNetworkHeaders, dropped the zero-importer
   customHeaders from the headline list).

3. Clarified v2_ux_interfaces_plan.md:185 so the v2 path reads as the copy's
   destination, not as the v1.5 source location.

Follow-up filed as #1785 (board Todo): src/types/navigation.ts InspectorTab is
dead and duplicates utils/inspectorTabs.ts — kept out of this pure-move PR.

Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
Claude-Session: https://claude.ai/code/session_01XNdjEPKLG637X8YmhDiEk5

* docs: address review round 4 — reconcile src/types/ claim with #1785 (#1776)

Doc-only follow-up to the round-4 @claude review on PR #1784 (no code changed):

The round-3 scope fix overshot: it asserted src/types/ holds "not domain types
at all", but the dead navigation.ts InspectorTab is a plain-.ts domain type
still living there, and specification/v2_ux_interfaces_plan.md:181-182 actively
prescribes placing InspectorTab there — so AGENTS.md and that spec doc gave
opposite guidance about the same directory.

- AGENTS.md + web README: softened to "not the place for new domain types"
  and added a parenthetical that the one lingering domain type (dead
  navigation.ts) is tracked for removal in #1785; until then a pure domain
  type belongs in utils/, not src/types/.
- v2_ux_interfaces_plan.md:181: marked the tab-enum placement guidance as
  superseded by #1785 (live type is InspectorTabId in utils/inspectorTabs.ts).

Self-heals on merge of #1785.

Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
Claude-Session: https://claude.ai/code/session_01XNdjEPKLG637X8YmhDiEk5

* docs: address review round 5 — fix garbled #1785 clause, restructure utils rule (#1776)

Doc-only follow-up to the round-5 @claude review on PR #1784 (no code changed):

1. Grammar (item 1): the round-4 #1785 clause read "superseding [#1785] will
   remove it" — a participle with no subject. Reworded to "the dead
   navigation.ts InspectorTab, which #1785 will remove".

2. Readability (item 2): the src/utils/ bullet had accreted into a single
   ~1,400-char blob across five review rounds. Promoted the carve-outs to
   scannable sub-bullets (Domain types / Diagnostic logging / Importing from
   core) and lifted the src/types/ note out to its own line, since it's a
   statement about a sibling directory, not about utils.

AGENTS.md only; the README's parallel sentence was already clean.

Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
Claude-Session: https://claude.ai/code/session_01XNdjEPKLG637X8YmhDiEk5

* docs+config: address review round 6 — correct the coverage-glob claim (#1776)

Responds to the round-6 @claude review on PR #1784:

1. "no build/coverage glob" was factually wrong (should-fix): the web coverage
   include in clients/web/vite.config.ts DOES enumerate src/lib/** and
   src/utils/**. The conclusion (a move between them is coverage-neutral)
   survives, but the reason was wrong. Reworded both AGENTS.md and the web
   README to say nothing *enforces* the boundary — no path alias, and the
   include lists both dirs so a move is coverage-neutral — plus the important
   whitelist caveat: a module outside components/lib/utils/server falls out of
   the ≥90 gate entirely.

2. Removed a pre-existing duplicate glob in vite.config.ts (src/lib/**/*.{ts,tsx}
   appeared twice, lines 161 and 163). Inert (vitest de-dupes) but it sat three
   lines from the config the doc now describes.

Nits: gave the README bullet the same round-5 sub-bullet restructure as
AGENTS.md, and made its #1785 reference a real markdown link (bare #N doesn't
autolink in rendered repo markdown).

npm run ci green (config change re-verified through coverage + smokes + Storybook).

Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
Claude-Session: https://claude.ai/code/session_01XNdjEPKLG637X8YmhDiEk5

---------

Co-authored-by: Claude Opus 4.8 (1M context) <[email protected]>

…config.ts, scripts/) (#1788)

* chore(web): format-gate the ungated first-party TS/JS (#1778)

Prettier only covered core/ and each client's src/, leaving the web Node
backend (clients/web/server/**), web top-level configs (vite.config.ts,
tsup.runner.config.ts, eslint.config.js, ...), and the root build/verify
tooling (scripts/**/*.mjs) ungated — so their formatting was never checked
by validate/CI. eslint already reached these via `eslint .`, so this is a
prettier-only gap.

- clients/web format/format:check now cover src, server, and top-level
  *.{ts,js}, not just src.
- New root format:scripts / format:check:scripts for scripts/**/*.mjs,
  folded into the root format and into validate:core.
- Normalized the newly-covered files with prettier --write (pure
  formatting; no runtime change).

Closes #1778

Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
Claude-Session: https://claude.ai/code/session_01XNdjEPKLG637X8YmhDiEk5

* chore: future-proof the scripts format glob (#1778 review)

Widen format:scripts / format:check:scripts from "scripts/**/*.mjs" to
"scripts/**/*.{mjs,js,cjs,ts}" so a future non-.mjs file under scripts/
is auto-gated, matching the future-proof glob style clients/web now uses.
A brace glob is a single pattern, so prettier does not error on the
unmatched branches. Addresses observation #2 from the PR #1788 review.

Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
Claude-Session: https://claude.ai/code/session_01XNdjEPKLG637X8YmhDiEk5

* docs: sync AGENTS.md scripts glob after the widening (#1788 review r2)

d86d8f7 widened the scripts glob to {mjs,js,cjs,ts} and updated the
validate:core bullet, but the format bullet at AGENTS.md:217 still said
scripts/**/*.mjs. Bring it in line so the two mentions agree.

Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
Claude-Session: https://claude.ai/code/session_01XNdjEPKLG637X8YmhDiEk5

---------

Co-authored-by: Claude Opus 4.8 (1M context) <[email protected]>

…1790 #1767) (#1792)

* chore: close a batch of quality-gate & tidy gaps (#1785 #1787 #1789 #1790 #1767)

Bundles five small, related tidy/gate items into one PR:

- #1785: delete the dead src/types/navigation.ts InspectorTab (0 importers;
  duplicated/drifted from utils/inspectorTabs InspectorTabId). src/types/
  now holds only ambient .d.ts stubs.
- #1787: add src/hooks/** to the web coverage `include` (it was silently
  outside the ≥90 gate) and add tests for the two untested hooks,
  useServerJsonImport and useImportClientConfig, bringing every hook to
  ≥90 on lines/statements/functions/branches.
- #1789: widen clients/{cli,tui,launcher} format/format:check to their
  top-level configs; add root format:shared / format:check:shared covering
  test-servers/src/**, vitest.shared.mts, and the repo-root eslint.config.js.
- #1790: pin prettier to an exact version (3.8.4, dropping the ^3.8.1
  caret) in all five package.json files so the format gate's verdict can't
  shift with an in-range patch bump; lockfiles synced.
- #1767: its root-file format+lint gap is closed here (format:shared +
  a new lint:shared via a second files block in the root eslint.config.js;
  web/server/ was already gated in #1788). Its larger cli/tui __tests__
  typecheck piece (76 type errors) is split out to #1791.

Prettier-normalized the newly-covered files (pure formatting). No runtime
behavior change. `npm run ci` green.

Closes #1785
Closes #1787
Closes #1789
Closes #1790
Closes #1767

Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
Claude-Session: https://claude.ai/code/session_01XNdjEPKLG637X8YmhDiEk5

* chore: address PR #1792 review findings

- #1789 gap: widen clients/web format to include .storybook and clients/
  launcher format to include scripts/; reformat the 3 .storybook files
  (were non-conforming under the pinned prettier 3.8.4). Audited all 885
  tracked source files — now fully prettier-clean.
- Export VALIDATE_DEBOUNCE_MS / COLLAPSE_DELAY_MS / HIGHLIGHT_DURATION_MS
  from useServerJsonImport and use them in the test instead of hardcoded
  300/1000/250 (so a tuned constant can't silently desync the test).
- Assert the symmetric renameErrors["alpha"] alongside ["beta"] so a
  one-directional collision-check regression fails.
- Comment the blank-rename test as exercising a UI-unreachable defensive
  branch (canImport gates submit).
- Un-stale the two specification/v2_ux_interfaces_plan.md references to the
  now-removed navigation.ts InspectorTab.

npm run ci green.

Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
Claude-Session: https://claude.ai/code/session_01XNdjEPKLG637X8YmhDiEk5

* docs: sync AGENTS.md format-surface description with the widened globs (#1792 review r2)

1ce3b64 added .storybook to clients/web's format glob and scripts/ to
clients/launcher's, but AGENTS.md:222 still described the pre-fix scope.
Bring it in line and note the non-recursive top-level glob is why nested
dirs are named explicitly. Docs only.

Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
Claude-Session: https://claude.ai/code/session_01XNdjEPKLG637X8YmhDiEk5

* chore: handle PR #1792 round-3 follow-ups in-place (format-coverage guard, uniform globs, theme gate)

Rather than defer these to tickets, fold them into this PR:

- Obs 2: widen every client + root format glob to a uniform
  *.{ts,tsx,mts,cts,js,jsx,mjs,cjs} so a future .mts/.cjs/.tsx top-level
  file can't silently slip the gate.
- Obs 1: add scripts/verify-format-coverage.mjs (wired into `validate`) —
  it parses the format:check globs out of every package.json, enumerates
  all tracked source files, and fails listing any not covered by a glob.
  The durable, standing version of #1789's one-shot audit; negative-tested.
- Obs 3: bring src/theme/** under the coverage gate (21/23 files were
  already ≥90 transitively; add Paper.test.tsx for the one laggard,
  Paper.ts). src/App.tsx (~4.5k lines, ~42% branches) is converted from a
  silent omission into an explicit, documented exclusion (comment on the
  coverage `include` + AGENTS.md) — gating it is a dedicated effort, not a
  whitelist tweak.

npm run ci green (the new guard reports 885/885 tracked source files gated).

Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
Claude-Session: https://claude.ai/code/session_01XNdjEPKLG637X8YmhDiEk5

* chore: address PR #1792 round-4 findings (guard hardening, docs, exclusion list)

- Harden verify-format-coverage: only harvest format:check globs from
  scripts transitively reachable from `validate` (via npm run refs), so an
  unwired/renamed format:check no longer counts as coverage. Negative-tested
  — renaming clients/web format:check to an unreferenced name now fails the
  guard (627 files flagged) instead of passing.
- Name src/main.tsx and src/index.ts (browser + bin bootstraps) alongside
  src/App.tsx in the coverage-include comment and AGENTS.md, so the
  documented exclusion list is complete rather than claiming App.tsx is the
  only one.
- Doc sync: document verify:format-coverage (AGENTS.md + README script table
  + scripts/ layout line), fix the `validate` ordering (verify:format-
  coverage now runs before validate:core) in AGENTS.md and README, and
  update the quoted globs to the uniform *.{ts,tsx,mts,cts,js,jsx,mjs,cjs} /
  test-servers *.{ts,tsx,mts,cts}.

npm run ci green.

Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
Claude-Session: https://claude.ai/code/session_01XNdjEPKLG637X8YmhDiEk5

* chore: address PR #1792 round-5 findings (airtight guard + doc/config sync)

- verify-format-coverage: assert the root `validate` chain invokes each
  client's `validate` (cd <dir> && npm run validate), closing the last
  reachability edge — a client dropped from the root chain now fails the
  guard even though its own globs still exist. Negative-tested.
- eslint.config.js: widen the shared `files` block to
  test-servers/src/**/*.{ts,tsx,mts,cts} to match the widened lint:shared
  glob, so a future .mts/.cts there is lint-gated, not just prettier-gated.
- AGENTS.md: scope the "uniform extension set" claim to client globs
  (core stays {ts,tsx}, shared {ts,tsx,mts,cts}), with verify:format-coverage
  as the backstop — removing the self-contradiction with the line below.

npm run ci green.

Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
Claude-Session: https://claude.ai/code/session_01XNdjEPKLG637X8YmhDiEk5

---------

Co-authored-by: Claude Opus 4.8 (1M context) <[email protected]>

…eak 'window is not defined' (#1786) (#1793)

* test(web): settle real-timer Transitions so the coverage gate can't leak 'window is not defined' (#1786)

The web coverage gate was intermittently failing with an unhandled
`ReferenceError: window is not defined` thrown from a Mantine `Transition`
timer that fired after happy-dom tore down `window` (the #1760 class). The
three `ViewHeader.test.tsx` tests that use `renderWithMantineTransitions`
(real, env="default" timers) each drove their *primary* transition to
completion, but the concurrent *enter* on a sibling cell leaves no unmount to
`waitFor`, so its rAF→setTimeout chain could still be pending at teardown and
fire post-teardown, failing an unrelated later file's run.

Fix: after each such test, flush pending transition timers against the
still-mounted tree via `settleTransitions()` (a wait longer than the 300ms
header duration + rAF slack), so the settling setState targets a live tree and
nothing is left to fire after cleanup unmounts.

Also close the related nit: `renderWithMantine`/`renderWithMantineTransitions`
now accept a `colorScheme` option, so `ViewHeader.test.tsx` no longer
hand-rolls a bare `defaultColorScheme="dark"` MantineProvider (the #1760
anti-pattern). AGENTS.md documents both the option and the settle requirement.

Closes #1786

Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
Claude-Session: https://claude.ai/code/session_01XNdjEPKLG637X8YmhDiEk5

* test(web): address review — export settleTransitions, derive settle from HEADER_ANIM_MS, guard fake timers, correct comments

Review feedback on PR #1793:

1. Derive the settle window from the real animation duration: export
   `HEADER_ANIM_MS` from ViewHeader.tsx and compute
   `TRANSITION_SETTLE_MS = HEADER_ANIM_MS + 200` in the test, so bumping the
   header duration can't silently make the settle insufficient (the invariant
   is now structural, not a comment).

2. Move `settleTransitions` into the shared `renderWithMantine.tsx` helper
   (exported, optional `ms` arg) next to `renderWithMantineTransitions`, since
   AGENTS.md points every future caller at it — shared test scaffolding belongs
   in src/test/, not a single test file. AGENTS.md now references the helper.

3. Guard `settleTransitions` against `vi.useFakeTimers()`: it awaits a real
   setTimeout, so under fake timers it would hang to the test timeout. Throw a
   clear message instead.

4. Correct the diagnosis comments: the hazard isn't a Mantine timer surviving
   unmount (its effect cleanup cancels timers) — it's the React state update
   Mantine's inner rAF schedules outside `act`, resolving post-teardown. Also
   fix the stale setup.ts matchMedia comment: the reduce-motion stub does not
   disable transitions (Mantine defaults respectReducedMotion=false); env="test"
   is the actual #1760 protection.

Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
Claude-Session: https://claude.ai/code/session_01XNdjEPKLG637X8YmhDiEk5

* test(web): tighten the review-trail comments (round 2)

Comment-accuracy follow-ups from PR #1793 round 2 (test infra only, no
behavior change):

- DEFAULT_SETTLE_MS: drop the false "Mantine's default header transitions run
  ~300ms" attribution (300ms is ViewHeader's HEADER_ANIM_MS; Mantine's Transition
  default is 250ms). Justify 500ms generically — a typical few-hundred-ms
  transition plus rAF slack.
- settleTransitions doc: stop asserting a single escape mechanism as fact. State
  what's observed (a post-teardown dispatchSetState throwing from
  resolveUpdatePriority, which runs synchronously inside dispatchSetState), what's
  known (useTransition has an unmount cleanup), and that the exact escape route
  isn't pinned down — the act-wrapped drain fixes it regardless.
- setup.ts: happy-dom v20 does implement matchMedia, so reframe the block as a
  deliberate override (pinning prefers-reduced-motion: reduce) rather than a
  polyfill for a missing API.
- Export MantineRenderOptions so callers can name the options type.

Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
Claude-Session: https://claude.ai/code/session_01XNdjEPKLG637X8YmhDiEk5

* test(web): resolve the last comment-contradiction + tighten the matchMedia override (round 3)

Round-3 review of PR #1793 (test infra only):

- setup.ts finding 1: the inner `value:` comment still said "happy-dom doesn't
  implement it", contradicting the block comment two lines up (which now
  correctly states happy-dom v20 DOES implement matchMedia and this is an
  override). Dropped the false clause; kept the part that actually justifies the
  `as unknown as MediaQueryList` double cast (the stub omits most of the
  interface).
- setup.ts finding 5 (folded in rather than deferred): anchored the `matches`
  regex to `/prefers-reduced-motion:\s*reduce/` so the override no longer also
  answers `true` to `(prefers-reduced-motion: no-preference)`.
- renderWithMantine.tsx nit 4: the `env="test"` rationale sat on `makeWrapper`,
  which also builds the `env="default"` wrappers (opposite purpose). Moved it
  onto `renderWithMantine`, mirroring the `env="default"` rationale on
  `renderWithMantineTransitions`; `makeWrapper` keeps a one-liner.

Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
Claude-Session: https://claude.ai/code/session_01XNdjEPKLG637X8YmhDiEk5

* test(web): make renderWithMantineTransitions own the settle (round 4)

Round-4 review of PR #1793 — implements the note-2 hardening in-PR rather than
deferring it, plus note 1 and the nit:

- Note 2 (settle was a convention AGENTS.md had to police): the real-transitions
  helper now OWNS the settle. `renderWithMantineTransitions` arms an automatic
  post-test drain via a helper-registered `afterEach`, so a real-transitions
  test can no longer leak the #1760 class by forgetting to settle a concurrent
  enter. Verified empirically that the helper's `afterEach` runs BEFORE the
  global cleanup() in setup.ts (afterEach is LIFO; setup.ts's cleanup is a
  setupFile registered first) — the ordering the drain depends on. The three
  explicit `settleTransitions()` calls in ViewHeader.test.tsx are gone; each
  render passes `settleMs: HEADER_ANIM_MS + 200` instead, keeping the window
  duration-derived. The auto-settle skips under fake timers (and no
  transitions test uses them). `settleTransitions` stays exported for the rare
  manual mid-body case.

- Note 1 (settle formula was duration-only): the helper doc + the new
  `settleMs` option doc now say the window must exceed the longest JS chain —
  duration PLUS any enterDelay/exitDelay — plus rAF slack.

- Nit: ViewHeader.test.tsx now states why HEADER_ANIM_MS is exported (the
  settle window must track the animation it settles).

Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
Claude-Session: https://claude.ai/code/session_01XNdjEPKLG637X8YmhDiEk5

* test(web): pin + self-check the auto-settle hook ordering; fix fake-timer doc; keep longest settle (round 5)

Round-5 review of PR #1793:

- Note 2 (load-bearing hook ordering was unpinned/undefended): the
  auto-settle's afterEach must run before setup.ts's cleanup(). Added BOTH
  guards the review suggested. (1) Runtime self-check: renderWithMantineTransitions
  captures the render container; the afterEach throws a clear error if it's
  already detached (cleanup ran first). (2) Pinned `sequence.hooks: "stack"` on
  the unit project. While verifying, found the ordering actually holds in EVERY
  sequence.hooks mode (stack/list/parallel) because setupFile hooks are outer
  relative to the helper's import-registered hook — so the comments now state
  that true mechanism (the pin is defense-in-depth; the container.isConnected
  check is the real enforcement), rather than claiming it "depends on stack".
  Full 3533-test unit suite passes under "stack".

- Note 1 (fake-timer claim was backwards): AGENTS.md no longer says the leak
  "would return" under fake timers (fake clocks park Mantine's setTimeout/rAF
  and are discarded, not drained). The real hazard — the auto-settle silently
  no-ops, so real-clock work is unprotected — is now stated. The afterEach also
  console.warns on the fake-timer skip (was silent; the manual settleTransitions
  throws — this closes that asymmetry).

- Note 3 (arming was last-write-wins): armedSettleMs now keeps the longest
  window via Math.max, so a later short-animation render can't under-settle an
  earlier long one.

- Nit: documented `settleMs: 0` as a working opt-out for a test that drove every
  transition to completion itself (act flush, no wait).

Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
Claude-Session: https://claude.ai/code/session_01XNdjEPKLG637X8YmhDiEk5

* test(web): post-await self-check + correct the hook-ordering mechanism (round 6)

Round-6 review of PR #1793:

- Finding 1 (self-check blind in parallel): added the post-await
  container.isConnected re-check the review suggested (factored both checks into
  assertConnectedForSettle(container, "before"|"after")). While verifying it, I
  established the actual mechanism, which differs from what both prior comments
  and the review assumed: setup.ts's cleanup() is a *setupFile (outer)* hook, and
  Vitest runs inner afterEach hooks to completion before outer ones in EVERY
  sequence.hooks mode — so cleanup never races this settle. Verified: ViewHeader's
  real-transitions tests pass under stack/list/parallel with neither check
  tripping, and a same-suite `afterEach(cleanup)` probe (where the ordering CAN
  break) does trip them (pre-check). So sequence.hooks isn't load-bearing here;
  the comments in renderWithMantine.tsx, vite.config.ts, and AGENTS.md now say
  that. The "stack" pin stays as documentation of intent / defense-in-depth, and
  the two checks are the real guard against a future same-level-cleanup
  regression.

- Nit: documented why last-write-wins on armedContainer is fine (cleanup detaches
  every container at once, so the last is a faithful "cleanup ran" proxy).
- Nit: the fake-timer skip warning now interpolates the current test name so it's
  actionable at teardown.

Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
Claude-Session: https://claude.ai/code/session_01XNdjEPKLG637X8YmhDiEk5

* test(web): disarm the auto-settle on self-unmount; concurrency + message nits (round 7)

Round-7 review of PR #1793:

- Finding 1 (guard misdiagnoses a self-unmount): the detached-tree check fired
  on ANY detachment and blamed a broken cleanup ordering, but a test calling the
  unmount() returned by renderWithMantineTransitions also detaches the container
  (a common pattern here). Fixed at the source: the helper now wraps unmount() to
  disarm the auto-settle (clear armedSettleMs/armedContainer) — once the test has
  unmounted there's nothing to settle against a live tree. The guard message also
  now names the self-unmount cause. Verified: a probe that unmounts a
  real-transitions render no longer trips the guard; ViewHeader unaffected.

- Nit: documented that the module-level arming assumes sequential tests (not safe
  under test.concurrent/describe.concurrent).
- Nit: the fake-timer skip warning falls back to "(unknown test)" when
  currentTestName is undefined.

- Finding 3 (doc weight): trimmed the AGENTS.md bullet to the *rule* (use the
  transitions variant only for mid-flight assertions; pass a duration-derived
  settleMs; no fake timers; use the returned unmount()) and pointed at the
  helper's own comments for the full mechanism, which now carries it at more
  depth.

Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
Claude-Session: https://claude.ai/code/session_01XNdjEPKLG637X8YmhDiEk5

* test(web): downgrade (not disarm) the auto-settle on self-unmount; per-container liveness (round 8)

Round-8 review of PR #1793:

- Finding 1 (round-7 disarm was too blunt + self-contradictory): disarming on
  self-unmount removed the drain entirely for the mid-flight-unmount test that
  most needs #1760 protection, and its "useTransition cleanup applies" rationale
  contradicted this file's own settleTransitions doc (which says that cleanup
  can't be relied on to stop the rAF). Now the wrapped unmount() DOWNGRADES: the
  drain still runs (mechanism-independent — flushes queued rAF/setTimeout while
  window is alive; a setState on the unmounted tree is a React no-op), only the
  liveness assertion for that container is dropped.

- Finding 2 (disarm undid round-5 max-wins arming): arming is now per-container
  (a Set of live containers) instead of all-or-nothing. Unmounting one tree drops
  only its own container; a second still-mounted tree keeps its longer window and
  its liveness check. The isConnected assertions run over the still-live set.

Verified with probes: returned unmount() drains without tripping the guard; a
second still-mounted tree is unaffected when the first self-unmounts; a bare
mid-body cleanup() STILL trips the guard (it's live, not decoration); ViewHeader
unchanged. Comments/AGENTS.md updated so the self-unmount path and the
settleTransitions doc no longer contradict each other.

Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
Claude-Session: https://claude.ai/code/session_01XNdjEPKLG637X8YmhDiEk5

* test(web): make the auto-settle drain best-effort so a guard trip doesn't also leak (round 9)

Round-9 review of PR #1793 (merge-verdict; optional robustness note):

- Finding 1 (pre-settle throw skipped the drain): on the ordering regression the
  guard exists to catch, throwing before the drain meant the maintainer got BOTH
  the explanatory error AND an uncaught post-teardown `window is not defined` in
  an unrelated file. The afterEach now captures a liveness failure, always runs
  the act-wrapped drain (flushing queued rAF/setTimeout while window is alive),
  and throws afterward — so the explanatory error stands alone. Verified: a bare
  mid-body cleanup() still throws the guard error (now after draining); ViewHeader
  unchanged.

- Nits: AGENTS.md now names the reachable bare-detach path (a mid-body cleanup()
  on a still-armed tree) rather than "a bare RTL unmount"; the settleMs doc notes
  arming is per-test, so in a multi-render test the longest window wins (a `0` is
  superseded by a longer sibling render).

Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
Claude-Session: https://claude.ai/code/session_01XNdjEPKLG637X8YmhDiEk5

* test(web): fix the regressed fake-timer doc + guarantee the liveness error survives a drain throw (round 10)

Round-10 review of PR #1793 (merge-verdict):

- Finding 1 (AGENTS.md fake-timer clause regressed in the round-7 doc trim): it
  had reverted to "silently dropping the protection", which round 5 had corrected
  — "silently" is now false (the auto-settle console.warns on the fake-timer
  skip) and "dropping the protection" reinstates a walked-back framing (fake
  clocks park Mantine's timers on a discarded clock, so the no-op doesn't
  obviously reopen #1760). Reworded to match the helper: it warns, and the hazard
  is that real-clock work is then unprotected, so the test depends on which clock
  was installed at teardown.

- Finding 3 (nit): matched the console.warn string to that same corrected
  framing, so AGENTS.md and the warning agree.

- Finding 2 (nit): a drain-time throw could discard the captured liveness error
  (the substitution round 9 set out to prevent, one step further). Now the drain
  error is captured too and the liveness error is preferred, so the explanatory
  error always wins. Verified a bare mid-body cleanup() still throws the liveness
  error after the best-effort drain; lint clean (no throw-in-finally); ViewHeader
  unchanged.

Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
Claude-Session: https://claude.ai/code/session_01XNdjEPKLG637X8YmhDiEk5

* test(web): preserve the drain error as cause + route the guard message to the reachable cause (round 11)

Round-11 review of PR #1793 (merge-verdict; two optional polish items):

- Finding 1: when both a liveness error and a drain-time throw occur, the drain
  error was dropped entirely. It's now attached as the liveness error's `cause`
  (when the liveness error is an Error), so an act-surfaced component error from
  the settling render is preserved without changing which error fails the test.

- Finding 2: the guard's error message pointed the reader first at the
  `sequence.hooks: "stack"` pin, which the file itself documents as
  defense-in-depth and NOT the ordering guarantee. Reworded to name the actually-
  reachable cause first — cleanup moved to a same-level afterEach instead of the
  setup.ts setupFile (outer) hook — and demote the pin to a parenthetical, so a
  maintainer who trips the check isn't sent to investigate a non-causal setting.

Verified: a bare mid-body cleanup() throws the reworded message; eslint clean;
ViewHeader unchanged.

Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
Claude-Session: https://claude.ai/code/session_01XNdjEPKLG637X8YmhDiEk5

* test(web): guard message states observation before candidate causes; log the preserved drain error (round 12)

Round-12 review of PR #1793 (merge-verdict):

- Finding 1: the round-11 rewording had dropped the "Expected cause:" hedge and
  asserted setup.ts's cleanup() as the fact, but the check only observes that a
  still-live armed tree was detached — it can't identify what detached it.
  Reworded to state that observation first, then list candidate causes: (a) a
  bare mid-body cleanup() in the test (the reachable case today), or (b) cleanup
  moved off the setup.ts setupFile hook to a same-level afterEach. No single
  cause asserted as fact.

- Nits 2a/2b: when both a liveness error and a drain error fire, the drain error
  is now also console.error'd (in addition to being attached as `cause`), so it's
  readable regardless of whether the reporter renders `error.cause` and isn't
  silently dropped even if the liveness error somehow isn't an Error.

Verified: a bare mid-body cleanup() throws the reworded message; eslint clean;
ViewHeader unchanged.

Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
Claude-Session: https://claude.ai/code/session_01XNdjEPKLG637X8YmhDiEk5

* test(web): final comment/string polish on the auto-settle guard (round 13)

Round-13 review of PR #1793 (merge-verdict; string/comment polish only):

- Finding 1: the console.error text claimed the drain error was "reported as the
  liveness error's cause" unconditionally, but the cause is only attached when
  the liveness error is an Error — misleading on the very non-Error path the
  hoisted log exists to serve. Dropped that parenthetical so the log is true on
  both paths.
- Nit: trimmed the now-duplicated outer comment to just the priority rationale
  (the inner comment, which mentions the log + instanceof conditional, is the
  accurate one).
- Nit: guard message says "drained against a dead tree" (exact for both the
  before- and during-settle detach) instead of "drained nothing".
- Nit: the message now names renderWithMantine.tsx rather than "the ordering note
  here", so it's self-contained out of context.

eslint clean; ViewHeader unchanged; npm run ci green.

Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
Claude-Session: https://claude.ai/code/session_01XNdjEPKLG637X8YmhDiEk5

* test(web): correct the sequence.hooks default claim (it's "stack" in vitest 4, not "parallel") (round 14)

Round-14 review of PR #1793 (merge-verdict):

- Finding 1: verified against installed vitest 4.1.0 — `resolved.sequence.hooks
  ??= "stack"` (coverage chunk), so the runtime default is "stack"; the CLI
  help-text's "parallel" is stale in vitest itself. The vite.config.ts comment
  claimed the pin overrides a "parallel" default; corrected to say the pin is
  Vitest 4's own default made explicit (documents intent + guards a future
  default change), which is a truer reason to keep it than an override.

- Nit: tied the two additions together — the post-settle check's same-level race
  can only arise if someone changes sequence.hooks to "parallel" (same-level
  hooks run sequentially under "stack"/"list"), so the pin and the post-check now
  reference each other rather than sitting in opposed positions.

ViewHeader unchanged; npm run ci green.

Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
Claude-Session: https://claude.ai/code/session_01XNdjEPKLG637X8YmhDiEk5

* test(web): document the isolate:true dependency (empirically confirmed) + fix pre-check-blindness wording (round 15)

Round-15 review of PR #1793 (merge-verdict; comment-only):

- Finding 1: the auto-settle's module-level afterEach depends on isolate:true
  (the default) to re-register per test file; under isolate:false the module
  evaluates once per worker and the hook binds to only the first importing file,
  so other files arm with no consuming hook — the drain silently never runs and
  the state leaks across files. Reproduced it (vitest --no-isolate --pool=threads
  --no-file-parallelism over two real-transitions files leaves one file's arming
  unconsumed) and documented the assumption + the onTestFinished escape hatch on
  the arming-state comment.

- Nit: the "sequence.hooks is why the pre-check can't see the concurrent detach"
  clause was wrong — the pre-check is blind because it runs synchronously before
  the await (a property of where it sits, not the hook mode). Reworded to state
  that, and that the concurrent case *additionally* needs sequence.hooks:
  "parallel".

ViewHeader unchanged; npm run ci green.

Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
Claude-Session: https://claude.ai/code/session_01XNdjEPKLG637X8YmhDiEk5

---------

Co-authored-by: Claude Opus 4.8 (1M context) <[email protected]>

* feat(cli): catalog listing, auth flags, and OAuth browser UX

Add servers/list and servers/show, --relogin / --stored-auth-only, browser
OAuth navigation with OSC 8 links, and extract method handlers under
clients/cli/src/handlers. Tracks #1781.

Co-authored-by: Cursor <[email protected]>

* fix(cli): address #1782 review — relogin clear, stored-auth-only, docs

Clear both raw and normalized OAuth keys on --relogin; check the shared
store before bailing under --stored-auth-only mid-session; restore the
tests README link target; document servers/show redaction limits; keep
mcpi groundwork with TODO(#1432) markers.

Co-authored-by: Cursor <[email protected]>

* fix(cli): gate OAuth browser open and harden #1782 review follow-ups

Arm auto-open only for CLI-owned interactive OAuth, honor MCP_AUTO_OPEN_ENABLED,
move --relogin clear after validation, skip keychain on servers/list, and cover
secret redaction after rehydrate.

Co-authored-by: Cursor <[email protected]>

* fix(cli): silence disarmed OAuth URLs and round-3 review nits

Suppress authorize URL print when navigation is disarmed or
--stored-auth-only; document MCP_AUTO_OPEN_ENABLED; redact Cookie/auth
headers; keep servers/show unit tests off the OS keychain.

Co-authored-by: Cursor <[email protected]>

* test(cli): cover withArmedAutoOpen for the ≥90 branch gate

CI failed on cliOAuth.ts branch coverage after the auto-open arming
helper landed; exercise the armed path through runCliInteractiveOAuth.

Co-authored-by: Cursor <[email protected]>

* fix(cli): mid-RPC unauthorized recovery and clear round-4 nits

Mirror connect's UnauthorizedError path in withCliAuthRecoveryRetry, let
MCP_AUTO_OPEN_ENABLED=true force open off-TTY, mock open-url in the
in-process runner, reject --relogin on catalog methods, restore the
__tests__ README inventory, and narrow OAuth helpers to structural client types.

Co-authored-by: Cursor <[email protected]>

* fix(cli): reconnect after mid-RPC OAuth and finish round-5 nits

Reconnect before retrying after unauthorized recovery, guard non-OAuth
configs, decouple forceAutoOpen, move open-url mock to vitest setupFiles,
and reject --relogin with stored-auth short-circuits.

Co-authored-by: Cursor <[email protected]>

* fix(cli): fail fast on non-TTY interactive OAuth and round-6 nits

Skip interactive OAuth without a TTY unless MCP_AUTO_OPEN_ENABLED=true;
document --stored-auth-only for CI; redact settings.metadata; reject
--relogin for stdio; clarify AuthRecoveryRequired reconnect comments.

Co-authored-by: Cursor <[email protected]>

* fix(cli): admit interactive OAuth when stdin is a TTY (round-7)

Gate on stdin||stderr so `2>&1 | tee` still works; fold confirm/auto-open/
TTY into CliOAuthConnectOptions; rename authRequiredFailure; document
MCP_AUTO_OPEN_ENABLED=true as non-TTY admit.

Co-authored-by: Cursor <[email protected]>

* fix(cli): fail fast on step-up without stdin (round-8)

Decline [y/N] on readline EOF/close, require a stdin TTY for the default
step-up confirmer (force-open admit is not enough), document stderr-only
browser auto-open vs stdin||stderr flow admit, and drop the alias wrapper.

Co-authored-by: Cursor <[email protected]>

* fix(cli): allow piped step-up answers; bound silent pipes (round-9)

Drop the hard stdin-TTY gate that broke `echo y | …`. Keep EOF decline and
add a short non-TTY timeout so an open pipe that never writes cannot hang.
Clarify isTTY as admit-only; extract the readline mock factory.

Co-authored-by: Cursor <[email protected]>

* fix(cli): keep partial piped step-up answers (round-10)

Capture readline `line` before close so `printf y` without a newline is
not treated as decline; timeout is auth_required with a distinct message;
document the 5s bound and post-confirm callback wait; pin the TTY no-timeout arm.

Co-authored-by: Cursor <[email protected]>

* fix(cli): tighten step-up timeout messaging and TTY test (round-11)

Make the TTY no-timeout assertion use a settled flag (the Promise.race
check was vacuous), say "timed out" instead of "declined" on the pipe
bound, and document that piped answers need a newline or stdin close.

Co-authored-by: Cursor <[email protected]>

---------

Co-authored-by: Cursor <[email protected]>

…ow-list (#1796)

Closes #1795.

Enforce localhost-only binding in code (the "never 0.0.0.0" rule was only a spec note): refuse an all-interfaces HOST — across every spelling the OS binds as the wildcard (inet_aton decimal/octal/hex, IPv6, IPv4-mapped, zone-scoped, IDNA/Unicode) — unless DANGEROUSLY_BIND_ALL_INTERFACES=true (Docker's sanctioned opt-in). Fixes the reported stdio-connect 403 by expanding the default loopback origin allow-list to all three interchangeable forms.

Hardening surfaced across an extended review: closes an IDNA/fullwidth wildcard bypass, a blank-ALLOWED_ORIGINS fail-open, and the CLI/TUI OAuth-callback wildcard bind (loopback-only now, RFC 8252); canonicalizes the origin allow-list the way a browser does; derives frame-ancestors from the real embedder list (IPv6-literal exclusion verified in headless Chromium); strictPort on all three bind points; and preserves the CLI exit-code map + JSON envelope through the mcp-inspector --cli launcher path. Shared host normalization lives in core/node/hostUrl.ts. Docs + tests + smokes throughout.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

* fix(core): register peer-request handlers before connect (#1797)

The roots/sampling/elicitation capabilities are advertised on the SDK
`Client` at construction, so from the moment `connect()` sends
`notifications/initialized` the server may issue any of those requests.
The handlers, however, were registered after the handshake resolved and
after `fetchServerInfo()` — leaving a window in which the SDK answered
`-32601 Method not found`.

`server-filesystem` asks for `roots/list` the instant it is initialized
(that is how it learns its allowed directories) and lost the race, so it
silently fell back to its CLI-argument directories instead of the roots
configured in the Inspector. `server-everything` asks later and did not.

Extract the four peer-request handler blocks into
`registerPeerRequestHandlers()` and call it before `client.connect()`.
None of them depend on server capabilities — only on constructor-set
state. The notification handlers that do gate on `this.capabilities` stay
in `connect()`, after `initialize`.

Regression test drives the race deterministically with a fake transport
that delivers `roots/list` synchronously from inside the `send()` of
`notifications/initialized`; it fails on the pre-fix ordering.

Closes #1797

Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
Claude-Session: https://claude.ai/code/session_011TrCvpunxToe7qnppk6zwr

* review: round-1 fixes — pre-connect roots/list_changed, wider timing test (#1797)

Finding 1: `notifications/roots/list_changed` was the one handler in the
post-handshake block that gates on nothing but constructor state, so it
sat in the same race — a server emitting it right after `initialized` had
it silently dropped (no wire error; the symptom is a missed UI refresh).
Moved to a new `registerPeerNotificationHandlers()` called next to
`registerPeerRequestHandlers()` before `connect()`.

Finding 2: the timing test asserted one of the four moved handler blocks.
It now injects `tasks/list` and the `roots/list_changed` notification in
the same burst as `roots/list`, so moving any of the block back after
`connect()` fails the test. Sampling/elicitation stay out — they park a
pending request awaiting user input and have no reply to assert on.
Dropped the unused `sessionId`/`setProtocolVersion` transport fields.

Verified the added assertions are load-bearing: the notification
assertion fails against the previous commit, the `roots/list` one against
`origin/v2/main`.

Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
Claude-Session: https://claude.ai/code/session_011TrCvpunxToe7qnppk6zwr

* review: round-2 fixes — correct the roots/list_changed premise (#1797)

The round-1 doc comment claimed a server may emit
`notifications/roots/list_changed` at initialize. It can't, normally:
`RootsListChangedNotificationSchema` is a member of
`ClientNotificationSchema` and absent from `ServerNotificationSchema` —
the client sends it (setRoots does, at :4251). The inbound handler is
defensive coverage for a non-conformant server, and dispatches
`rootsChange` with our own already-known roots. Reworded the method doc
and the test's header so neither states the false premise; the move
itself stands (it gates on no server capability, so it belongs with the
request handlers).

Also from review:
- Dropped the vacuous `notificationRejected` flag and its inverted
  comment. The SDK does not throw for an unregistered notification, so
  the try/catch could never observe one — confirmed by bisect, where the
  pre-fix failure was always `rootsChanges`, never the flag. Removing the
  catch also stops it swallowing a genuine throw from the injected
  requests.
- Narrowed `replies` to responses (`"result" in message || "error" in
  message`) so it can't also collect client-originated requests.
- Annotated the unreachable `if (!this.client)` guard with a justified
  `v8 ignore` per the AGENTS.md dead-guard policy.

Re-verified both regression directions: the `roots/list` assertion fails
against `origin/v2/main`, the `rootsChange` one against `55e25b7c`.

Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
Claude-Session: https://claude.ai/code/session_011TrCvpunxToe7qnppk6zwr

* review: round-3 fix — drop stale comments inside the roots handler (#1797)

The four inline comments moved verbatim with the handler contradicted the
method doc reworded one round earlier: they said "server's roots" (they
are ours — it is a ClientNotification) and "we'll just dispatch an empty
array", which is not what `this.roots || []` does when roots are
configured. The new timing test asserts exactly that non-empty dispatch.
Replaced with one accurate line. Comment-only.

Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
Claude-Session: https://claude.ai/code/session_011TrCvpunxToe7qnppk6zwr

* fix(cli): advertise roots so roots/set is actually serviceable (#1797)

Round-4 review found the same -32601 symptom on a second path: the CLI
constructed its InspectorClient with no `roots` option, so
`capabilities.roots` was never advertised and no `roots/list` handler was
registered — yet `--method roots/set` calls `setRoots()`, which announces
`notifications/roots/list_changed` to the server. A server taking up that
invitation got "Method not found", after the CLI reported success.

The review's suggested fix — register `roots/list` unconditionally — is
not implementable: the SDK asserts the client capability inside
`setRequestHandler` ("Client does not support roots capability"), so it
throws during connect for any client built without the option, and
`registerCapabilities` refuses to run after connect. A client that omits
`roots` therefore cannot legally serve `roots/list` at all.

Fixed at the layer that can fix it: `cli.ts` seeds `roots: []`, matching
web (`App.tsx` always passes it). The core registration stays gated on
the constructor value, now with the SDK constraint documented so the next
reader doesn't retry the unconditional version.

`setRoots()`'s doc claimed it "will enable roots capability if it wasn't
already enabled" — it cannot, for the reasons above. Corrected, and
dropped the dead `if (this.roots === undefined) this.roots = []` the very
next line overwrote.

Tests: a CLI test drives `list_roots` over an HTTP test server (verified
load-bearing — without the seed it fails on the -32601 error text), and
the core timing test gains a case pinning that roots set after connect
are served with the current values, not the constructor's.

Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
Claude-Session: https://claude.ai/code/session_011TrCvpunxToe7qnppk6zwr

* fix(cli,tui): advertise the roots configured in mcp.json (#1797)

Round-5 review: seeding `roots: []` fixed the -32601 but not the outcome.
`server-filesystem` discards an empty roots answer (`validatedRootDirs
.length > 0`) and keeps its CLI-argument directories — which is the
symptom this PR set out to fix. The user's roots were already in hand two
lines away, on `serverSettings.roots` (lifted from mcp.json by
`mcpConfigToServerEntries`). Now passed through `cleanRoots`, the same
shared helper web uses, so both clients answer `roots/list` byte-identically
for the same config.

This matters most on the CLI because it is one-shot: `--method roots/set`
sets roots on a connection torn down a few statements later, so the config
file is the only durable way to give a run its roots.

Same gap in the TUI (`App.tsx` built its options with no `roots` key at
all), fixed the same way. The review suggested a follow-up issue for it;
folding it in here instead, since it is the identical line and splitting
it would leave a known-wrong client in tree.

Also from review: hoisted `waiters` up with the other fields, and
`injectRequest` now rejects after 1s naming the method and id rather than
hanging to the vitest timeout.

Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
Claude-Session: https://claude.ai/code/session_011TrCvpunxToe7qnppk6zwr

* review: round-6 fixes — document roots in the CLI README, harden cleanRoots (#1797)

- clients/cli/README.md enumerated the settings lifted from a catalog/config
  file without roots. Roots are the least discoverable member of that list —
  there is no flag, and `--method roots/set` dies with the connection — so
  the file is the only durable way to supply them. Said so.
- cleanRoots() trusted its input's shape. `Root[]` is a compile-time type over
  hand-editable mcp.json, and as of this PR all three clients feed it straight
  from disk, so `"roots": [{"name":"Work"}]` or `"roots": "file:///work"` threw
  at connect. Non-array bails to []; an entry without a string uri is dropped
  with a warning. Hardened in the one shared place rather than three call
  sites. Review suggested a follow-up issue; folded in for the same reason as
  the TUI fix last round.
- Test comment said the CLI passes a `roots: []` seed — true one commit ago.
- injectRequest clears its timeout on reply (an armed timer outliving a test is
  the #1760 teardown class), and setRoots' doc drops the "despite what this
  comment used to claim" archaeology.

Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
Claude-Session: https://claude.ai/code/session_011TrCvpunxToe7qnppk6zwr

* review: round-7 fix — cleanRoots also throws on a non-string name (#1797)

The round-6 hardening guarded `uri` and left `name` throwing one line
later: `r.name?.trim()` guards null/undefined, not "is a string", so
`"roots": [{"uri":"file:///a","name":42}]` still died at connect — the
same crash class, one field over, reachable from all three clients and
the web settings-save path. Reproduced against the shipped function body
before fixing.

A bad `name` now costs the name, not the root (it is optional, and the
uri is still usable), with a warning. The doc no longer overstates what
is validated.

Also from review: the CLI test comment said "`cli.ts` now seeds
`roots: []`" — true only of that ad-hoc test's path; and the
registration comment's `(:588)` line reference is now a durable name.

Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
Claude-Session: https://claude.ai/code/session_011TrCvpunxToe7qnppk6zwr

* fix(core): normalize roots in setRoots() too (#1797)

Round-8 review: this PR made cleanRoots the single normalizer for roots at
connect and at save, but setRoots — the third way roots enter the client —
did a bare `this.roots = [...roots]`. Web cleans at its call site; the CLI
does not, so `--method roots/set --roots-json '[{"name":"Work"}]'` stored a
root with no uri, advertised it in the roots/list reply, and echoed it back
as success. cleanRoots is idempotent, so calling it inside setRoots covers
the CLI without disturbing web's pre-clean, and all three entry points now
agree.

Review filed this as an optional follow-up; folding it in for the same
reason as the TUI and cleanRoots items — it is smaller than the issue
describing it.

Test asserts the malformed entry is dropped and the valid one survives;
verified load-bearing against the bare-spread version.

Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
Claude-Session: https://claude.ai/code/session_011TrCvpunxToe7qnppk6zwr

* review: round-9 fixes — normalize roots in the constructor too (#1797)

The round-8 commit claimed "all three entry points now agree"; review
pointed out the constructor is a fourth, and the only one still raw. All
three clients clean at their call site, so nothing was broken — but the
invariant was enforced by convention rather than by the class, at the very
entry point this whole thread started from. `this.roots` now runs through
cleanRoots, with a ternary so the `undefined`-vs-`[]` distinction that
gates capabilities.roots survives.

Also from review:
- The CLI roots/set test provoked a cleanRoots warning without suppressing
  it (AGENTS.md). Suppressed, and the spy now asserts the drop was
  reported rather than silent.
- `enables roots when previously undefined …` in coverage-backfill was
  named after behaviour round 5 established does not exist. Renamed, with
  a comment on what that client actually can and cannot do.
- setRoots dispatches a copy to rootsChange, as getRoots() already did, so
  a listener can't push into the list we advertise.
- run-method.test.ts's connectStdio now passes `roots: []` like cli.ts, so
  the helper matches the client shape production builds.

Verified load-bearing: the constructor test fails against the raw
assignment. validate / coverage / build-gate / smoke / storybook all green
(run as separate stages — full `npm run ci` kept being killed mid-coverage
by the environment).

Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
Claude-Session: https://claude.ai/code/session_011TrCvpunxToe7qnppk6zwr

* review: round-10 fixes — copy at both rootsChange dispatches (#1797)

Round-9 claimed setRoots' copy closed the "a listener can't push into the
list we advertise" hole. It closed one of two sites: my reply conflated
the notification handler's *dispatch* with the roots/list *request*
handler — the latter is serialized to the wire immediately and is fine,
the former hands listeners the internal array. Both copy now.

Also from review: dropped the `({}) as never` in the new constructor test
for the real factory the sibling tests use (this test never connects, so
it is never invoked), and the needless `async` on it.

Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
Claude-Session: https://claude.ai/code/session_011TrCvpunxToe7qnppk6zwr

* review: round-11 doc fixes; merge v2/main (#1797)

- cleanRoots' opening sentence still framed it as the settings-form
  normalizer, contradicting the paragraph below it that says every client
  feeds it from disk. It has four callers now; says so.
- Marked the two runtime guards `Keep:` with the reason. The `Root[]`
  parameter narrows both to `never`, so "the compiler says this can't
  happen" is the argument someone will use to delete the checks rounds 6-7
  installed. The reason now lives where the deletion would happen.
- Noted the constructor-normalization case in the timing test's header, so
  it is discoverable from a file otherwise about the initialized race.

Also merged origin/v2/main (#1796, loopback binding), which had advanced
by one commit and touched the smoke scripts.

Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
Claude-Session: https://claude.ai/code/session_011TrCvpunxToe7qnppk6zwr

* fix(core): clear queued peer requests when connect() fails (#1797)

Round-12 review found a second-order consequence of the handler move.
Before it, registration happened after every await in connect(), so the
moment a peer request could be queued the connect could no longer fail —
queue and failure path never overlapped. Registering before the handshake
puts both the handshake and the post-connect `setLoggingLevel` inside that
window.

The catch sets status to error and drops the transport but never cleared
pendingSamples/pendingElicitations; only disconnect() does, and connect()
reaches it solely on the connect-timeout path (not the default). So a
server that elicits at `initialized` and then fails the connect left a
live pending-request modal on a dead connection — web derives it from the
queue lengths with no status gate — and the elicitation's cancel() never
fired, hanging any awaitUrlElicitation waiter.

Extracted disconnect()'s clear-and-cancel into clearPendingPeerRequests()
and called it from the catch, dispatching the change events so the UI
drops the modal. CLI/TUI are unaffected either way (sample/elicit off).

Test note: the first version of this test passed with and without the fix
— its `requestedSchema` omitted `properties`, so the SDK rejected the
inbound params and nothing was ever queued. Fixed the fixture; it now
fails without the change with a queued ElicitationCreateMessage.

Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
Claude-Session: https://claude.ai/code/session_011TrCvpunxToe7qnppk6zwr

* fix(core): clear the peer-request queue on a mid-session crash too (#1797)

Round-13 review, two items.

The test asserted only `getPendingElicitations()`/`getPendingSamples()`,
which read the arrays directly — but the arrays are not what removes the
modal. `usePendingClientRequests` tracks its own state off the
pendingSamplesChange/pendingElicitationsChange events, so a regression that
cleared without dispatching would strand a live modal on a dead connection
and stay green. Both tests now assert the events fired with an empty
payload; verified by deleting just the dispatches, which fails them.

The connect-failure fix left the other way a connection ends uncovered:
a mid-session crash goes through the transport's `onclose`, which sets
status and fires `disconnect` but never touched the queues. Same two
hazards, and arguably more reachable. `clearAndAnnouncePendingPeerRequests`
wraps the clear plus its events, guarded on a non-empty queue so the paths
that overlap (a connect failure whose dropCachedTransport also fires
onclose) announce once; called from both. New test queues an elicitation,
fires onclose, and asserts the queue and the event — fails without the
change.

Review flagged the crash path as a follow-up; folding it in, as with the
earlier out-of-scope items — it is the same seam and three lines.

Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
Claude-Session: https://claude.ai/code/session_011TrCvpunxToe7qnppk6zwr

* review: round-14 fixes — make both teardown paths agree (#1797)

- The crash path's comment claimed "cleared before the `disconnect` event
  so a consumer sees an empty queue" while disconnect() cleared *after*
  its own dispatch — an invariant stated as a property and held at one of
  two sites, the same shape as the round-10 finding. Moved disconnect()'s
  clear above its status/disconnect block so both paths agree; its change
  events stay batched with the other teardown dispatches. Full gate green,
  so nothing depended on the old ordering.
- clearPendingPeerRequests' doc still named the `connect()` failure path as
  a caller; that path now goes through clearAndAnnouncePendingPeerRequests.
  Names the two real callers.
- The mid-session test waited on `await Promise.resolve()` — the
  microtask-counting pattern this file replaced with injectRequest two
  rounds ago, working today only because the enqueue happens exactly one
  tick deep. Waits on the client's own `newPendingElicitation` instead.
- The file header still called the constructor case "the last case"; it is
  third of five, and the two teardown cases went undescribed. Rewritten.
- Restored the missing blank line between two tests, and noted why the
  sampling-event assertion holds for a queue that never held anything.

Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
Claude-Session: https://claude.ai/code/session_011TrCvpunxToe7qnppk6zwr

* review: round-15 fixes — scope the parity claim to the arrays (#1797)

Round-14 made the two teardown paths agree on when the queue is cleared,
and the comment then claimed parity flatly. It holds for the arrays but
not the events: the crash path announces before `disconnect`, while
disconnect() batches its change events with the rest of its teardown, so
they land just after. Since the events are what drives the modal, the
distinction matters to the next reader. Scoped the comment rather than
un-batching — un-batching would trade a documented ordering for a real
behaviour change with no gain.

Same rule as round 14, one level down: check the other site, then check
the other axis.

Also gave the new `newPendingElicitation` wait the timeout that
`injectRequest` documents, so a regression that stops the enqueue fails at
the wait instead of hanging to the vitest timeout.

Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
Claude-Session: https://claude.ai/code/session_011TrCvpunxToe7qnppk6zwr

* test: pin disconnect()'s clear-before-announce ordering (#1797)

Round-14 moved clearPendingPeerRequests() above disconnect()'s status
block so a `disconnect` consumer sees an empty queue on both teardown
paths, and round 15 documented it — but nothing enforced it. The existing
`disconnect` listeners only count events, and the two teardown tests cover
the crash and connect-failure paths, neither of which goes through
disconnect(). So the one behavioural change in the stack was untested in
both directions, and the tidy that undoes it (regrouping the clear with
the batched change events) would have stayed green.

Test records the queue length from inside a `disconnect` listener; the
fixture's close() never fires onclose, so it exercises disconnect()'s own
ordering rather than the crash path. Verified against the pre-round-14
position: `expected 1 to be +0`.

Folded the newPendingElicitation wait into a shared helper now that two
tests use it.

Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
Claude-Session: https://claude.ai/code/session_011TrCvpunxToe7qnppk6zwr

* test: pin disconnect()'s announce as well as its clear (#1797)

Round-16 pinned the array half of disconnect()'s teardown and left the
events unpinned — and nothing else in the suite covers them, so deleting
disconnect()'s two change-event dispatches would have left a live modal
after a user-initiated disconnect with the suite green. That is the path
most likely to have a modal open when it runs. Asserted after the await
rather than inside the disconnect listener, since disconnect() batches the
events deliberately; verified by deleting the dispatches (`expected
undefined to be +0`).

Header rewritten by category rather than position: it has now gone stale
twice in three rounds because it counted tests ("the last two") and
enumerated paths that a new test falsified. It names the three teardown
paths and what distinguishes them instead.

Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
Claude-Session: https://claude.ai/code/session_011TrCvpunxToe7qnppk6zwr

* review: round-18 doc fixes — scope the three-path claim, re-attribute cancel() (#1797)

The header I rewrote last round claimed all three teardown paths clear
before announcing. True only if "announcing" means the `disconnect` event:
the connect-failure path emits none at all, and both it and the crash path
clear after their `statusChange`. Scoped to what's true. Same shape as
rounds 10/14/15 — a parity asserted in prose that holds on one of the axes
it could mean — which is now the fourth time, in a sentence I wrote.

ElicitationCreateMessage.cancel()'s doc still credited `disconnect()`
teardown; this PR made clearPendingPeerRequests() its only caller, serving
three paths. Re-attributed, and promoted the no-`onRemove` note to its own
paragraph with the consequence spelled out — it quietly became a
three-caller invariant during this PR, and "don't splice mid-iteration"
reads like a style note until you know it would skip every other entry and
leave those promises unsettled.

Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
Claude-Session: https://claude.ai/code/session_011TrCvpunxToe7qnppk6zwr

* fix(core): gate roots/list on what was advertised, not on this.roots (#1797)

Round-19 review found a latent wedge, reproduced before fixing: a client
constructed without `roots` that later calls setRoots() has this.roots
defined, so registerPeerRequestHandlers tried to register the handler on
every subsequent connect() — and the SDK throws "Client does not support
roots capability" from setRequestHandler when the capability was never
advertised. That throw lands before the handshake, so the client can never
reconnect. Worse than the -32601 this PR started from: the connection is
gone rather than answering one method badly.

Not a regression (the same gate lived after the handshake before) and
unreachable from the three shipped clients, which all pass roots. But
core/ is a library surface, and the failure is unrecoverable.

Gate now reads a construction-time `rootsCapabilityAdvertised` — the thing
the SDK actually asserts on — so this.roots stays free to change. setRoots'
doc says what does happen on such a client: the roots are stored and
readable, but no server can ask for them.

Also scoped the header's claim that the connect-failure path emits no
`disconnect`: true of connect()'s own catch, but on a real transport
dropCachedTransport's close() fires onclose first, which does dispatch one
— making the catch's own call the auth-recovery backstop.

Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
Claude-Session: https://claude.ai/code/session_011TrCvpunxToe7qnppk6zwr

* review: round-20 — derive the roots gate from the advertisement itself (#1797)

Round-19's fix restored the invariant but not the derivation:
rootsCapabilityAdvertised was computed from `options.roots` alongside the
capability block rather than from it, so the two encodings of "did we
advertise roots" could drift. Adding any term to the advertisement
condition would leave the flag true with `capabilities.roots` absent —
the same unrecoverable wedge, reintroduced silently and in the throwing
direction.

Now read off the built capability object after it is assembled, so
divergence is structurally impossible rather than merely absent. Verified
the reconnect test still fails against the old `this.roots` gate.

Also: named the capability-lifetime category in the test header (the new
test was none of the three it listed), moved that test below the teardown
group so the group stays contiguous, and rewrapped the over-long comment
line.

Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
Claude-Session: https://claude.ai/code/session_011TrCvpunxToe7qnppk6zwr

* fix(core): gate elicitation/create on what was advertised too (#1797)

Round-21 review asked round-20's question of the capability next door, and
there the two derivations already disagreed — no future edit needed.
`elicit` is typed `boolean | { form?, url? }`, so `{}` or
`{ form: false, url: false }` is valid, enables no mode, and advertises no
`capabilities.elicitation` — while the registration gated on `this.elicit`
being truthy. Reproduced before fixing:

  CONNECT ERROR: Client does not support elicitation capability
                 (required for elicitation/create)

Since #1797 moved registration before the handshake, that throw means the
client cannot connect at all. Same wedge as the roots one, one capability
over, and reachable through a documented option value rather than a future
edit.

Gated on a new elicitationCapabilityAdvertised, derived from the built
capability object exactly as the roots flag now is. Test constructs with
`{ form: false, url: false }` and asserts connect() resolves; verified
load-bearing.

Also made the four construction-time capability inputs (sample, elicit,
receiverTasks, advertisedExtensions) `readonly`. The round-19 bug was a
mutator making a construction-time fact stale, so a future setter for any
of these should be a compile error rather than a review catch.

Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
Claude-Session: https://claude.ai/code/session_011TrCvpunxToe7qnppk6zwr

* fix(core): derive tasks.requests and the remaining gates from the advertisement (#1797)

capabilities.tasks.requests declares which server→client requests we accept
as tasks, but was built from `receiverTasks` alone — so
`{ receiverTasks: true, elicit: false }` told the server it could send a
task-augmented elicitation/create while advertising no elicitation
capability and (since the last commit) registering no handler. A server
taking the invitation gets -32601: the advertise-then-refuse shape this PR
opened with, and the last place it survived. Now built from
capabilities.sampling / capabilities.elicitation, both decided just above,
with the key omitted entirely if neither applies.

Also made the remaining three registration gates read the built capability
object like the two from the last two rounds — sampling, tasks/*, and the
URL-elicitation completion handler. None could diverge today, but a mixed
convention means the next reader has to re-derive which gates are safe, and
adding one condition to an advertisement block is exactly how elicitation
broke. Five gates, one rule now.

Test asserts a receiverTasks+elicit:false client advertises tasks.requests
with sampling and without elicitation; verified load-bearing.

Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
Claude-Session: https://claude.ai/code/session_011TrCvpunxToe7qnppk6zwr

* review: round-23 — pin the omitted-requests arm, drop the count again (#1797)

The `requests` key is omitted when neither sampling nor elicitation is
advertised, but every `receiverTasks: true` construction in the tree left
one of them enabled, so that arm was unexercised — and it would not have
shown as a coverage failure, since one uncovered branch in a ~4900-line
file sits well inside the per-file gate. Second client in the same test
asserts `tasks` survives (list/cancel are still serviceable) while
`requests` is absent rather than empty; verified against `requests:
taskRequests`.

The header regained a numeral four commits after round 17 rewrote it by
category to stop exactly that, and it was already stale. Dropped, and named
the third category — advertise-only-what-you-serve, which is about the
capability object rather than a registration gate.

Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
Claude-Session: https://claude.ai/code/session_011TrCvpunxToe7qnppk6zwr

* fix(core): settle queued samples on teardown instead of discarding them (#1797)

Round-24 review. Round 13 recorded the samples-vs-elicitations asymmetry
as deliberate on two grounds: nothing internal awaits a sample, and the
transport is gone by then. The first still holds; the second is false on
one path, and it is a path this PR created. `connect()`'s catch drops the
transport only when `!transportHasAuthProvider`, so on the HTTP+OAuth path
— web's ordinary authenticated case — it is retained and the caller
reconnects over it. A sampling request queued during the handshake was
then discarded with the connection still live: no response frame ever
written, and the server waits forever for a reply to a request we
accepted. Accept-then-silence, which is worse for the peer than the
-32601 this PR opened with.

SamplingCreateMessage.cancel() mirrors the elicitation one — settles
(rejects, what the UI's decline path already sends and the SDK turns into
an error response) without calling onRemove, since the caller iterates the
queue and clears it. clearPendingPeerRequests loops it like the
elicitations.

Also corrected two comments that assumed the catch always tore the
transport down, and noted that the retention is gated on
transportHasAuthProvider alone — independently of
isConnectAuthRecoveryError, which gates only the status hold.

Test asserts settlement rather than a wire frame, since on the disconnect()
path the transport is closed before the settle; a later respond() throwing
"already resolved or rejected" is the proof. Verified load-bearing.

Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
Claude-Session: https://claude.ai/code/session_011TrCvpunxToe7qnppk6zwr

* fix(core): mark receiver-task payload promises handled (#1797)

Round-25 review. clearPendingPeerRequests iterates every entry in
pendingSamples, and two shapes live there: the plain server-request one,
whose reject reaches the SDK handler, and the task-augmented one, whose
reject reaches the receiver task's payloadPromise. That promise only gets
a consumer when the server polls tasks/result, so a task sitting in
input_required has no handler attached — and the settle added last commit
surfaced as an unhandled rejection.

Not hypothetical: the repo already worked around it test-side, with a
helper that reaches into private state to attach no-op catches, whose
comment describes this exact race. Fixed at the source instead — one
`void payloadPromise.catch(() => {})` at creation, which also covers the
pre-existing cancelReceiverTask path. Removed the now-redundant helper and
its two call sites. Verified: dropping the source line makes that suite
report two unhandled rejections.

Corrected SamplingCreateMessage.cancel()'s doc — its "no response frame is
ever written" rationale is true of the plain shape only; a task-augmented
sample already answered the wire with a CreateTaskResult, so settling it is
about not leaving the task in input_required limbo.

Nits: generalized the enqueue wait helper to both events so the sampling
wait gets the timeout the elicitation one has; moved the sampling teardown
test back into the contiguous teardown group; named the settle property in
the header; fixed the fixture doc that still claimed to record replies.

Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
Claude-Session: https://claude.ai/code/session_011TrCvpunxToe7qnppk6zwr

* review: round-26 — drop the comment that outlived its call (#1797)

The suppressReceiverPayloadRejections call went last commit; its two-line
comment stayed, describing a pre-attach that no longer happens and
crediting the test for handling a rejection the source now handles.
Removed. Also reworded the surviving .catch() in inspectorClient.test.ts:
it is still needed, but as a value capture for the assertion, not for
unhandled-rejection suppression.

Rewrapped the teardown paragraph in the test header — my round-25 edit
left a 104-char line, and two hand-fixes left it ragged, so it is reflowed
in one pass.

Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
Claude-Session: https://claude.ai/code/session_011TrCvpunxToe7qnppk6zwr

* review: round-27 — skip the roots/list_changed the SDK would refuse (#1797)

Review reported setRoots() emitting roots/list_changed on a client that
never advertised roots, inviting a re-fetch we'd answer -32601. Probed it:
the premise is wrong. The SDK asserts the notification capability too —
`notification()` rejects with "Client does not support roots list changed
notifications" and nothing reaches the wire. No spec violation and no
invitation; what actually happened is that setRoots provoked that
rejection and logged it as "Failed to send", implying a transient failure
where the truth is structural.

Kept the early return on that corrected basis: it stops us provoking a
rejection we know will come, and warns accurately instead. Doc and comment
say the SDK is the enforcer rather than claiming this guard is what keeps
the wire clean.

The test pins the end state (no invitation for roots we can't serve) and
passes either way — recorded as such rather than presented as a regression
guard, since it isn't one.

Also moved the advertise-category test below the two gate cases, matching
the order the header introduces them.

Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
Claude-Session: https://claude.ai/code/session_011TrCvpunxToe7qnppk6zwr

* review: round-28 — gate the notification on the predicate the SDK asserts (#1797)

The roots/list_changed guard read `capabilities.roots !== undefined`, but
the SDK asserts `capabilities.roots.listChanged` for the notification —
wider than the flag. Identical today (roots are only ever advertised as
`{ listChanged: true }`), but a conditional advertisement would un-gate it
silently, which is the round-21 shape again. Split into
rootsListChangedCapabilityAdvertised, mirroring
urlElicitationCapabilityAdvertised, which already reads its assertion's
sub-field; rootsCapabilityAdvertised keeps the roots/list registration,
where presence is what setRequestHandler checks.

Also moved the new test out of the gate pair it had split — it is a
runtime-emission case, not a registration-gate one — and extended the
header's converse-category paragraph to cover notifications, which indexes
it without inventing a category. Reflowed the setRoots doc paragraph in one
pass (111-char line from the last commit, plus a ragged orphan from an
earlier edit).

Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
Claude-Session: https://claude.ai/code/session_011TrCvpunxToe7qnppk6zwr

* review: round-29 — chase the corrected premise into clients/cli (#1797)

Round 27 corrected "setRoots announces roots/list_changed to a server that
then gets -32601" — the SDK refuses the notification from a client that
never declared it, so nothing reached the wire. That reword happened in
core/, but the same premise had been written into the two comments
justifying the CLI's roots seed, and they still asserted it. Both now state
the real pre-fix failure: a server asking for roots on its own got -32601,
and roots/set could not announce at all.

Also fixed the warn message under the round-28 gate: it named the roots
capability while the gate is on roots.listChanged — precisely the pair that
split was made to distinguish, so a client advertising `{}` would have been
told it advertised nothing.

Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
Claude-Session: https://claude.ai/code/session_011TrCvpunxToe7qnppk6zwr

* fix(core): reject in-flight raw-wire requests on a mid-session crash (#1797)

rejectPendingRawWireRequests had one caller — disconnect() — so the crash
path settled the inbound peer-request queue this PR added and left the
outbound raw-wire map alone. That map holds the modern tasks/* frames the
SDK's era gate refuses to route, so the SDK's chained _onclose doesn't know
about them: a Tasks-tab poll in flight when the server died waited out its
own 30s timeout and reported a timeout for what was a crash, on a
connection that had already dispatched `disconnect`.

Pre-existing — both the map and the crash path predate this PR. Fixing it
here because the invariant it violates is one this PR wrote down: "every
path that ends a connection has to clear that queue, announce it, and
settle each entry", in clearAndAnnouncePendingPeerRequests' doc and the
timing test's header. One channel was true of one path out of three.

Test starts a raw-wire request and fires onclose; without the call it
hangs to the vitest timeout, which is the symptom.

Checked the two siblings and left both: taskInputAbortControllers is
already unblocked via the elicitation cancel the crash path performs, and
activeToolCallAbortController rides an ordinary client.request the SDK
settles itself.

Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
Claude-Session: https://claude.ai/code/session_011TrCvpunxToe7qnppk6zwr

* review: round-31 test fixes — justify the cast, drop the no-op tick (#1797)

- The new test's `as unknown as` had no justification, which AGENTS.md
  explicitly prohibits. It is unavoidable — rawWireRequest is private and
  every public route is gated on isTasksExtensionNegotiated(), which needs
  a modern handshake the fixture doesn't perform — so that reasoning is now
  written down instead of left to be re-derived.
- `await Promise.resolve()` was a no-op: rawWireRequest registers its entry
  in the promise executor, synchronously, before any suspension. Dropped,
  which also makes the test pin that ordering rather than paper over it —
  and stops the third recurrence of the microtask-counting pattern this
  file's own helper doc argues against.
- Moved the test into the teardown group and extended the header's teardown
  paragraph to cover the outbound direction, which it framed entirely as
  inbound.

Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
Claude-Session: https://claude.ai/code/session_011TrCvpunxToe7qnppk6zwr

* review: round-32 — separate the outbound teardown claim from the inbound one (#1797)

My round-31 header edit spliced the outbound raw-wire passage into the
middle of the inbound claim, so "There are three: a failed connect(), a
mid-session transport close, and an explicit disconnect()" ended up reading
as scoping the outbound settle — which is two by design, since nothing
populates the map before the handshake. Round 31 established that scoping
so the code wouldn't imply an impossible state; the prose then un-scoped
it. Split into its own paragraph with its own scope clause, and reflowed
both in one pass rather than nudging breaks.

Also raced the raw-wire assertion, for the reason the file's own wait
helper documents: without the teardown rejection the test waited out the
request timeout and reported a bare vitest timeout. It now fails with
"raw-wire request not settled by teardown" — verified.

Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
Claude-Session: https://claude.ai/code/session_011TrCvpunxToe7qnppk6zwr

* review: round-33 — extract the timeout race, mark the failure-path promise handled (#1797)

Three copies of "reject after 1s so a regression fails where it happened"
had accumulated, each re-explaining itself. Extracted `withTimeout`, used
by the enqueue wait and the raw-wire assertion; injectRequest keeps its own
since its timeout also deletes the waiter entry.

On the failing path the raw-wire test left the production 30s timer armed,
so a regressed run got the intended failure plus a stray unhandled
rejection up to 30s later — the armed-timer class the file's own comment
names, on the failure path. `settled` is marked handled before the race.
Verified: with the fix removed the run fails with "raw-wire request not
settled by teardown" and no unhandled rejection.

Recorded why disconnect() drains the SDK's in-flight requests but rejects
the raw-wire map outright: the user asked to disconnect, and a Tasks poll
is not worth delaying that for. Pre-existing and deliberate, now written
down where the asymmetry is visible.

Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
Claude-Session: https://claude.ai/code/session_011TrCvpunxToe7qnppk6zwr

* review: round-34 — drop a no-op guard and the false mechanism it asserted (#1797)

Round 33 claimed the raw-wire test could leave `settled` rejecting
unhandled on a regression run, and round 33's fix encoded that as
`void settled.catch(() => {})` with a comment explaining it. Both were
wrong: `Promise.race` attaches a rejection handler to every input, so
`withTimeout(settled, …)` on the next line already marks it handled.
Verified with a standalone script — no unhandledRejection fires, and the
pre-fix version was equally safe.

Removed the line and the comment. The comment mattered more than the line:
it told the next reader that `Promise.race` does not mark its losers
handled, which is false and would cost them elsewhere.

Kept the true half by setting `timeout: 50` on that test's client, so a
regression leaves no 30s timer armed and fails with `Raw request
"tasks/get" timed out after 50 ms` — more self-describing than the raced
generic message. Verified.

Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
Claude-Session: https://claude.ai/code/session_011TrCvpunxToe7qnppk6zwr

* review: round-35 — the drain comment described a trade-off nobody made (#1797)

Last round's comment said raw-wire requests "get no share of the drain
above: the user asked to disconnect, and a Tasks poll is not worth delaying
that for" — framing a decision. Verified it isn't one: `safeDisconnectTimeout`
defaults to 0 and the only caller anywhere that passes a value is a single
test, so nothing is drained in production; and the drain polls the SDK's
`_responseHandlers`, which never holds raw-wire ids, so those were never
eligible regardless.

Reworded to what is true. The framing came from my own round-33 reply,
which described the drain as running on every disconnect — a comment
faithfully encoding a half-read mechanism, which is the same failure as the
`Promise.race` one it followed.

Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
Claude-Session: https://claude.ai/code/session_011TrCvpunxToe7qnppk6zwr

* review: round-36 — the race comment outlived the mechanism its own commit replaced (#1797)

`timeout: 50` made the request settle in ~50ms, so the 1s race can never
win and a regression fails on the assertion (`timed out after 50 ms` vs the
expected /Connection closed/) — which the commit message said, while the
comment beside it still credited the race. Reworded: the short timeout is
what fails fast and names itself, the race is the backstop for a promise
that never settles at all. Race kept; it covers the case where the
request's own timer is broken too.

Third comment in three rounds encoding a mechanism the same commit changed.

Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
Claude-Session: https://claude.ai/code/session_011TrCvpunxToe7qnppk6zwr

* review: round-37 — scope the timeout comment, align the bypass guards (#1797)

`timeout: 50` is the client-wide request timeout, not a bound on the
raw-wire request; it is inert in that test only because the test issues no
SDK request after connect. Noted, so someone adding one later knows it
would inherit a 50ms budget rather than reading the comment and looking
elsewhere.

The two installReceiverTaskResponseBypass guards read `this.receiverTasks`
while the five gates around them read the advertisement. Installing the
bypass mutates the Protocol's _requestHandlers map, so it is registration —
same as the setRequestHandler it follows. Equivalent today; aligned so the
convention is uniform rather than something the next reader has to
re-derive per site. The genuinely-runtime `receiverTasks` reads are left.

Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
Claude-Session: https://claude.ai/code/session_011TrCvpunxToe7qnppk6zwr

* review: round-38 — one predicate for the whole bypass mechanism (#1797)

Round 37 moved the bypass *install* guard to the advertisement and left the
three branches it controls on the raw option, splitting one immutable fact
across two sources. The dead direction is the harmful one: if the tasks
advertisement ever acquires a condition — the exact edit that produced the
elicitation wedge — then advertised goes false while receiverTasks stays
true, the bypass is not installed, and the handler still returns
`{ task }` into the SDK's validating wrapper, which rejects it. That is the
breakage the bypass exists to prevent, and before round 37 it could not
happen because install and branch read the same field.

All three now read tasksCapabilityAdvertised. `receiverTasks` is left where
it belongs: assigned once, and driving the advertisement.

Also noted that the wrapper's own check is redundant with its install guard
and kept deliberately, since it must agree with the handler branch.

Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
Claude-Session: https://claude.ai/code/session_011TrCvpunxToe7qnppk6zwr

* fix(core): don't carry receiver tasks into the next session (#1797)

Round-39 review found the last member of the enumeration rounds 12-14 and
31 built: receiverTaskRecords was cleared only by disconnect(). The crash
path doesn't clear it, the connect() failure catch doesn't, and connect()
never reset it — so a record created in one session survived into the next
connect() on the same instance, and `tasks/list` is answered from that map.
A new server would be told about a task it never created.

Reachable in web (the only client with receiverTasks): a task-augmented
sampling/createMessage arriving in the handshake window creates a record;
if that connect then fails with a 401, App.tsx retries connect() on the
same instance.

Cleared at the top of connect() rather than only on the crash path — the
auth-recovery retry means ending a session isn't the only way a new one
begins, and starting clean covers every route in. disconnect() keeps its
clear via the same extracted helper.

Pre-existing — the crash path never cleared these — but the handshake
window this PR opened is what lets a receiver task exist before connect
resolves, and this was the one member left outside the invariant the PR
documented. Test verified load-bearing.

Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
Claude-Session: https://claude.ai/code/session_011TrCvpunxToe7qnppk6zwr

* fix(core): reset all session-scoped state on connect, not just receiver tasks (#1797)

Round-40 review: receiverTaskRecords was one of five collections cleared
only by disconnect(). Two of the other four have real symptoms on the same
crash-then-reconnect path:

- subscribedResources — the modern subscribeToResource early-returns on a
  uri already in the set, so after a reconnect the user's Subscribe click
  silently sends nothing to the new server while the UI keeps listing it.
- cancelledTaskIds — a leftover id mislabels a *new* task sharing it as
  cancelled rather than failed, and server task ids are often short.

taskInputAbortControllers leaked un-aborted controllers.

modernLogLevel had the opposite asymmetry: disconnect() cleared it so it
couldn't leak (#1629), but nothing restored it, so a reconnect after an
explicit disconnect silently lost the user's configured level while a
reconnect after a crash kept a mid-session override. It is now recomputed
from serverSettings the way the constructor does, so both routes agree.

clearReceiverTasks became resetSessionState, called from the same place.

Tests cover the two symptomatic siblings and now assert receiver tasks
through the `tasks/list` handler — what a server actually sees, and the
symptom itself — which also removed the unjustified `as unknown as` the
previous commit added. The remaining cast is justified in place, since
cancelledTaskIds has no public reader.

Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
Claude-Session: https://claude.ai/code/session_011TrCvpunxToe7qnppk6zwr

* fix(web): re-seed the Logs control when the client re-seeds its own level (#1797)

Round-41 review caught a consequence of the last commit. `resetSessionState`
restores the client's modern log level from the server setting at connect;
`resetSessionScopedUiState` still blanked the web control to null on every
disconnect. On the one path that reconnects the *same* instance —
auth recovery — the control then read Off while every modern request carried
`_meta` logLevel "debug", the default. A tool whose job is showing what you
send should not disagree with itself about what it sent.

The control now re-seeds from the active server's settings exactly as
setupClientForServer does, instead of blanking.

Also from review:
- Added the tests the restore never had, covering both halves: a mid-session
  override does not survive a reconnect, and a disconnect no longer drops the
  configured level (the #1629 half, and the actual behaviour change).
- resetSessionState's doc claimed "every piece of state scoped to one
  connection", which invites assuming membership rather than checking it.
  Now states the rule: state a new session would misread; what only needs
  settling on the way out stays with the teardown paths.
- Header: session-scoping paragraph moved below the outbound one to match
  test order, and it now names all five members rather than three.

Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
Claude-Session: https://claude.ai/code/session_011TrCvpunxToe7qnppk6zwr

* refactor(core): one derivation for the modern log level (#1797)

Round-42 review: the configured level was computed independently in four
places — twice in InspectorClient (constructor, resetSessionState) and twice
in App.tsx (the seed and the re-seed). The client and the Logs control
agreed only because four sites happened to compute the same expression, and
any drift reproduces exactly the symptom the previous commit fixed: the
control reading one level while every modern request stamps another.

Extracted resolveModernLogLevel into core/mcp/types.ts beside
DEFAULT_MODERN_LOG_LEVEL; all four call sites use it, with the web mapping
undefined→null at its own boundary (display, not a second derivation).

Also from review:
- disconnect()'s comment still credited itself with preventing the #1629
  leak, which resetSessionState now prevents. It says what the line does
  now — read "not opted in" while disconnected — and notes the web control
  deliberately shows the configured level in that window.
- resetSessionState's doc claimed an un-aborted taskInputAbortControllers
  entry "leaks a paused poll loop". Both registration sites release in a
  finally, so nothing leaks permanently; the abort closes the window
  between a crash and the unwind. Softened, and it now has the test it
  lacked — the only member of the five without one.
- Noted in App.tsx that the re-seed depends on activeServerIdRef still
  holding the outgoing id (it syncs in a passive effect), so an eager clear
  would silently drop it to the default.

Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
Claude-Session: https://claude.ai/code/session_011TrCvpunxToe7qnppk6zwr

* fix(core): reset the subscription stream state with the set it derives from (#1797)

Round-43 review: resetSessionState cleared subscribedResources but left
modernStreamState, so a crash-then-reconnect on the same instance held an
empty set alongside `{ active: true }` — a combination the rest of the file
treats as impossible, since onModernSubscriptionClosed and
onModernReconnectFailed both compute `active` from `subscribedResources.size
> 0`, and the comment at :4712 asserts a terminal drop clears both. No
symptom today only because the UI mirror resets itself independently, in
another file — which is the arrangement this PR has spent its rounds
removing. Clearing it here also announces the reset; that set's every other
mutation dispatches.

Also from review:
- The web re-seed treated "no server found" as the default level rather
  than Off, because resolveModernLogLevel returns the default for absent
  *settings*. Reachable via a second disconnect after a crash, and via
  removing the active server. Now distinguishes the two.
- Converged the two injectRequest copies: withTimeout takes an optional
  onTimeout, so both drop their waiter on the timeout path. The newer copy
  had lost exactly the cleanup that was round 33's reason for leaving the
  original alone.

Test seeds the stream state a live subscription would have left; verified
load-bearing (the earlier version passed either way, since the fixture
never activates a stream).

Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
Claude-Session: https://claude.ai/code/session_011TrCvpunxToe7qnppk6zwr

* fix(web,core): correct the re-seed branch; share the subscription-stream reset (#1797)

Round-44 review. My round-43 fix traded one wrong branch for another: it
read `?.settings`, which is undefined both when no server matched *and*
when the server has no settings node — and the second is the common case
(a hand-written mcp.json entry never opened in Server Settings). Those
servers showed Debug while connected and Off after disconnect, which is
exactly the client/UI disagreement rounds 41-42 removed, with no
modernLogLevelChange event to reconcile it. Now branches on the server.

Worth noting no test would have caught it: App.tsx is a documented
exclusion from the coverage gate.

Also from review:
- resetSessionState cleared subscribedResources without announcing it,
  two lines below a comment arguing that every other mutation of that set
  dispatches. And it took two statements of a six-statement cluster
  disconnect() clears together — leaving modernReconnectAttempts (a new
  session's first drop would back off as if it were the old one's nth),
  modernListenGeneration (the bump that makes an in-flight re-listen bail),
  and a stale modernSubscription. One resetSubscriptionStream helper now
  serves both, announcing both axes; disconnect() keeps only the live
  stream's best-effort close. That also removed the file's one assignment
  to modernStreamState that bypassed its setter.
- The stream-state test pinned the value, not the dispatch — the half the
  UI actually tracks. Now asserts both; verified against a bare field
  assignment.
- onModernSubscriptionClosed's comment claimed the disconnect reset always
  clears the set; on a crash it doesn't until the next connect. Says so.

Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
Claude-Session: https://claude.ai/code/session_011TrCvpunxToe7qnppk6zwr

* review: round-45 — reattach the orphaned doc, order the announcements (#1797)

Extracting resetSubscriptionStream last round inserted it *and its doc*
between resetSessionState's doc and resetSessionState — so two doc blocks
stacked, the new helper took the nearest, and the 23-line block explaining
each member's symptom attached to nothing. Dead text where it sat, and the
most load-bearing comment in the batch. Moved, not reworded.

That is round 32's finding in production code, from the same cause:
inserting anything between a doc block and its method is a move that needs
checking, prose or code.

Also from review:
- resetSubscriptionStream announced the cleared set before updating the
  stream state derived from it, so a listener in between saw an empty set
  with an `active` stream — the combination the helper exists to prevent,
  exposed by its own dispatches. Both fields move first, then both are
  announced. The onModernSubscriptionClosed comment now says that window
  is never observable, which is newly true.
- The test header called the log level "cleared"; it is re-seeded, which is
  the #1629 half of the test twenty lines below. Says "reset", with the
  distinction spelled out, and the paragraph reflowed in one pass.

Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
Claude-Session: https://claude.ai/code/session_011TrCvpunxToe7qnppk6zwr

* fix(core): close the listen stream the connect-path reset drops (#1797)

Round-46 review: resetSubscriptionStream's doc claimed disconnect() is the
only caller with a stream worth closing, and the helper did a bare
`modernSubscription = null`. Not guaranteed on the connect path — an
`onerror` without an `onclose` leaves the transport up, connect() reuses
it, and the reset then drops the last reference to a stream still open on
the server. Nothing can close it afterwards: the `closed` handler bails on
the bumped generation, and `closed` never resolves for a live stream.
Before the extraction the surviving reference was eventually closed by
refreshModernSubscription as `previous`, so this was newly unclosable.

The helper now closes it itself, best-effort, after the dispatches so the
round-45 ordering is unaffected; disconnect() drops its two hand-rolled
lines.

Also from review:
- The session test pinned the subscription set's value but not its
  dispatch — the half the UI tracks, and the gap round 43 closed for the
  stream axis one line away. Both now asserted; verified load-bearing.
- resetSessionState's doc said disconnect() "clears all of this", which is
  true by duplication for two of the five members; it now says a sixth has
  to be added in both places. And "the teardown paths" handle the in-flight
  tool call only on the disconnect route — on a crash it is callTool's own
  finally. Named.

Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
Claude-Session: https://claude.ai/code/session_011TrCvpunxToe7qnppk6zwr

* review: round-47 — fix the doc's count, catch the closed-promise chain (#1797)

- resetSessionState's doc said "two members through the same helpers, the
  other two hand-rolled" for a body with five. It is two and three — and
  the member the enumeration dropped is the one that is *paired* rather
  than duplicated: modernLogLevel is re-derived here and blanked in
  disconnect(), deliberately. Both corrected.
- `subscription.closed.then(...)` had no rejection handler, unlike every
  other promise around that stream. Newly reachable: before the last
  commit a connect-path reset dropped the reference with `closed`
  permanently pending, so the chain never settled at all; closing the
  stream is what makes it settle. Added, with the Node-terminates reasoning
  the file already applies to payloadPromise.
- The listen-stream test exercised transport reuse without asserting it —
  the half that makes "still open on the server" true. Counts factory calls
  now, so the premise fails loudly if a future change recreates the
  transport rather than silently ceasing to apply.

Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
Claude-Session: https://claude.ai/code/session_011TrCvpunxToe7qnppk6zwr

* review: round-48 — scope the catch, guard the close, index the test (#1797)

- The .catch comment claimed the connect-path close made a *rejecting*
  `closed` reachable. It doesn't: closing resolves it. What that close newly
  reaches is the handler running at all, where the reference used to be
  dropped with `closed` pending forever. And chaining after `.then` also
  swallowed handler throws — silently abandoning a re-listen on the path
  whose job is recovering a dropped stream. Now the second argument to
  `.then`, scoped to the rejection, with the comment saying which half is
  which.
- `closing?.close()` reaches into third-party code from `disconnect()`'s
  straight-line teardown, none of which is inside a `try` — a synchronous
  throw would skip the raw-wire rejects, the task aborts, the receiver-task
  clear and nine dispatches, silently, since most callers catch a
  disconnect(). Best-effort now against both failure modes.
- Header indexes the listen-stream test: same category, but a release
  obligation rather than a misread one.
- Its comment described the generation guard, which the stub can't exercise;
  marked as production behaviour.

Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
Claude-Session: https://claude.ai/code/session_011TrCvpunxToe7qnppk6zwr

* review: round-49 — pin the best-effort close, unorphan the doc (#1797)

Round 48 made `resetSubscriptionStream`'s close best-effort against both a
rejected promise and a synchronous throw, but nothing exercised either arm:
both `modernSubscription` stubs in the tree resolve. On the `disconnect()`
path an escaping failure would silently skip the raw-wire rejects, the paused
task-input aborts, the tool-call abort and `clearReceiverTasks()` — the
findings rounds 13 and 30 closed, re-opened at once with a green suite. Two
table-driven cases now seed a `close()` that throws and one that rejects,
and assert teardown continued past it.

Also: the `.then` comment gave opposite verdicts on one mechanism (an
unhandled rejection is named unacceptable, then chosen for a handler throw) —
reworded to say the handler cannot throw today and why the shape is
defensive; and `refreshModernSubscription`'s doc block, orphaned two methods
up since before this branch, moved onto the method it describes.

Closes #1797

* review: round-50 — sweep the queues start-clean, unify the close (#1797)

`resetSessionState`'s doc excluded the peer-request queues and the raw-wire
map because "the teardown paths handle them" — true of the paths that run.
An `onerror` without an `onclose` runs none of them: it flips status to
"error" and leaves `baseTransport` cached, so a `connect()` on the same
instance reuses a live transport. That is the route the round-46 stream close
was built on, and it strands these two the same way — the web pending-request
modal is derived from the queue length with no status gate, so it outlives
the session, and answering it writes a response for the old request id onto
the new connection. Both are now swept start-clean beside the reset (both
helpers are idempotent, so the routes that already ran them are unaffected),
the end-clean clears stay put for the `disconnect`-event consumer, and both
docs say which is which.

Also folds the three subscription `close()` sites onto one
`closeSubscriptionBestEffort` helper, so all three absorb a synchronous throw
as well as a rejection — at `refreshModernSubscription`'s first site the
reference is already dropped, so a throw there both leaks a live stream and
aborts the refresh before its replacement is opened.

Closes #1797

* review: round-51 — pin the sweep's ordering, name the category (#1797)

The connect-start sweep must sit after `resetSessionState()`, and the two
calls read as independent. Cancelling a task-augmented peer request settles
it synchronously into the record callback, which ends in `upsertReceiverTask`
— a no-op only because `clearReceiverTasks()` just emptied the map. Hoisted,
it would emit a `notifications/tasks/status` for the outgoing session's task
onto the transport the connect is about to reuse. Stated at the call and
pinned by a test (which fails on the hoist).

Adding that third caller also made five docs stale, all describing the old
set by count or as "teardown" — the class round 23 diagnosed. All five now
name the category: every route out, plus the top of …

* test(cli,tui): typecheck the __tests__ dirs (#1791)

The cli/tui `typecheck` scripts only checked `src` (the src-only tsconfig
excludes `**/*.test.*`), leaving the `__tests__` dirs — which do the most
`as` casting — with no tsc pass. Add a `tsconfig.test.json` per client that
includes `__tests__` with the test-exclusions dropped and the test-only path
aliases (`@modelcontextprotocol/inspector-test-server`, the `@inspector/core/*`
deep paths, express/vitest) resolving what vitest resolves, then fold
`tsc -p tsconfig.test.json` into each `typecheck`.

Fixes the 76 errors it surfaced:
- cli: a wrong `@modelcontextprotocol/inspector-core/...` module specifier
  (should be `@inspector/core/...`; only a type import so vitest never caught
  it), `r.headers` optional-access guards, and partial OAuth mocks replaced
  with typed `makeFakeCliOAuthClient()` / `makeFakeServerSettings()` factories
  instead of `as` casts. Adds `@types/express` (devDep) so the transitively
  aliased test-server source typechecks, mirroring clients/web.
- tui: rest-param signatures on the App OAuth spies (fixes the spread-arg and
  narrow-return errors), a widened callback param (`iss?`), a `makeFakeServer-
  Settings()` factory for the tuiOAuth tests, required OAuth-metadata/client
  fields in AuthTab, and a loosened `entry()` message param for the
  deliberately-malformed wire messages HistoryTab's defensive branches render.

Gate coverage only; no runtime behavior change.

Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
Claude-Session: https://claude.ai/code/session_01XNdjEPKLG637X8YmhDiEk5

* test(cli,tui): address review — real spy signatures, config typecheck, nits

Responds to the @claude review of #1799:

- App.test.tsx: type the OAuth spies against the real InspectorClient method
  signatures (`vi.fn<InspectorClient["…"]>()`) and forward `Parameters<…>`
  tuples from the FakeClient wrappers, so `toHaveBeenCalledWith(...)` on the
  six OAuth-lifecycle spies stays argument-checked under the new gate rather
  than accepting anything via `unknown[]`. `getOAuthState` regains its wide
  `OAuthConnectionState | undefined` return.
- Typecheck the client config files too: add `vitest.config.ts` / `tsup.config.ts`
  (and tui `dev.ts`) to each src `tsconfig.json` include — they were the one
  first-party surface still getting no tsc pass.
- Trim the redundant `src/**/*` from each `tsconfig.test.json` include (tsc
  pulls in the src the tests import; the src-only config already checks it).
- Rename the tui settings helper to `oauth-test-fakes.ts` to match cli's, and
  soften both factories' docstrings to stop naming a loader they don't mirror
  field-for-field.
- Fix a stale `@modelcontextprotocol/inspector-core/...` specifier in a
  createRemoteTransport JSDoc example (the same wrong package name the gate
  caught in the cli tests, surviving in a comment).

Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
Claude-Session: https://claude.ai/code/session_01XNdjEPKLG637X8YmhDiEk5

* test: address round-2 review — correct claims, tighten types, typecheck launcher

Responds to the round-2 @claude review of #1799:

- Correct the toHaveBeenCalledWith claim: vitest doesn't type-check
  `toHaveBeenCalledWith(...)` arguments against the mock's signature, so the
  App.test.tsx comment no longer claims it does — the typed spies buy checked
  implementations/return payloads (the stale-literal fix), not arg-checking.
- authenticate spy now uses the real `InspectorClient["authenticate"]`
  signature (returns a `URL`), so it no longer diverges from production; the
  one mockResolvedValue is updated to a URL.
- makeFakeCliOAuthClient members are typed against the real CliOAuthClient
  signatures (not bare vi.fn()), so a wrong-signature override is rejected.
- HistoryTab: revert the `entry()` `message: unknown` widening (it un-checked
  the ~15 well-formed fixtures) and instead scope a named `MALFORMED` cast to
  the two intentionally-malformed messages.
- Typecheck the launcher `__tests__` too (the same gate hole in the one client
  the PR hadn't touched): new clients/launcher/tsconfig.test.json + a typecheck
  script folded into validate. Completes the "every client's __tests__ is
  typechecked" invariant.
- Doc the new test-typecheck + config-file typecheck in AGENTS.md.

Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
Claude-Session: https://claude.ai/code/session_01XNdjEPKLG637X8YmhDiEk5

* test: address round-3 nits — launcher config typecheck, callTool typed, docs

Responds to the round-3 @claude review of #1799 (all non-blocking):

- launcher: add `vitest.config.ts` to tsconfig.test.json's include (it was in
  neither project — the build config's rootDir: ./src rejects it). Now
  typechecked.
- App.test.tsx: type `callTool` against `InspectorClient["callTool"]` (+ the
  Parameters<…> wrapper) so its `mockResolvedValue` payloads are checked, and
  give `clearOAuthTokens` an `async () => {}` impl so the mock actually returns
  the `Promise<void>` its type promises.
- AGENTS.md: correct the per-client alias-set description (cli's is widest,
  tui's is core+react only, launcher's has no paths) and note launcher's config
  file lives in its test project, not the src project.

Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
Claude-Session: https://claude.ai/code/session_01XNdjEPKLG637X8YmhDiEk5

* test: address round-4 — durable typecheck-coverage guard + launcher/doc nits

Responds to the round-4 @claude review of #1799 (all non-blocking):

- Add `verify:typecheck-coverage` (scripts/verify-typecheck-coverage.mjs), the
  typecheck-coverage analog of verify:format-coverage and the durable guard for
  the invariant this PR establishes: for each Node client it runs the tsconfig
  projects its `typecheck` names with `tsc --listFiles`, unions them, and fails
  on any tracked .ts/.tsx that lands in no project. This closes the class the
  review kept surfacing by hand (launcher/__tests__ in round 3,
  launcher/vitest.config.ts in round 4) — a new top-level file can no longer go
  untypechecked silently. Wired into `npm run ci` and the GitHub workflow.
- launcher: reorder `validate` to run `typecheck` before `build` (matches tui
  and the documented order), and make `typecheck` self-contained — it now runs
  both `tsconfig.json` and `tsconfig.test.json`, so running it standalone means
  the same thing it does in cli/tui.
- makeFakeCliOAuthClient: note on the comment that the typing covers the
  defaults, not `overrides` (a bare vi.fn() override is Mock<(...args)=>any>).
- Docs: README table + AGENTS.md updated for the new guard and the corrected
  launcher typecheck/validate ordering.

Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
Claude-Session: https://claude.ai/code/session_01XNdjEPKLG637X8YmhDiEk5

* test: address round-5 — wire-check the guard, --listFilesOnly, into validate

Responds to the round-5 @claude review of #1799:

- Finding 1 (the pre-merge one): the guard now ports verify:format-coverage's
  reachability checks — it asserts each client's `typecheck` is reachable from
  its own `validate` and that the root chain runs each client's `validate`, so
  it can't stay green while measuring a typecheck nothing invokes. Verified:
  stripping `npm run typecheck` from launcher's validate now fails the guard.
- Nit 3: switch `tsc --listFiles` → `tsc --listFilesOnly` (identical file list,
  ~4× faster: whole guard 23s → ~4s). Now cheap enough for the inner loop, so
  it moves into `validate` (right after verify:format-coverage) instead of a
  standalone ci step — matching its sibling. Removed the separate ci-chain entry
  and GitHub workflow step (validate covers it).
- Nit 4: `npx --no-install tsc` so a missing local tsc fails fast instead of
  reaching for the registry (matches verify-build-gate.mjs).
- Nit 5: corrected projectFiles' JSDoc to describe the filter that's actually
  there (drops repo-external + node_modules paths; core/ + test-servers/ stay
  but are harmless).
- Finding 2: fixed the 2-space misindent on the new root package.json line
  (prettier --write). Docs (README/AGENTS) updated for the validate placement.

Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
Claude-Session: https://claude.ai/code/session_01XNdjEPKLG637X8YmhDiEk5

* chore(scripts): address round-6 guard nits — dedupe wiring, robust diagnostics

Responds to the round-6 @claude review of #1799 (all guard-ergonomics nits):

- Nit 2: extract the shared reachability logic (`reachableScripts`,
  `rootRunsClientValidate`) into scripts/lib/npm-scripts.mjs and consume it from
  BOTH verify-format-coverage.mjs and verify-typecheck-coverage.mjs, so the
  "is this gate actually run?" wiring can't drift between them (same rationale
  as scripts/lib/prod-web-server.mjs).
- Nit 1: typecheckProjects now harvests `-p`/`--project` from every script
  reachable from `typecheck` (not the single string), so a delegating
  `typecheck` (`npm run typecheck:src && …`) is handled instead of failing with
  a false "names no -p project" diagnosis. Also accepts `--project`.
- Nit 3: on a tsc config error, echo the `error TS…` diagnostic (scanning both
  streams — tsc prints config errors to stdout) so a broken tsconfig is
  self-explaining rather than surfacing only as a misleading "file in no
  project" report.
- Nit 4: refreshed the CI workflow's stale "Validate" step name/comment (it now
  runs the coverage guards + validate:core + per-client typecheck).
- Nit 5: reworded the `.d.ts` carve-out comment to read as pre-emptive (there
  are none under cli/tui/launcher today).

Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
Claude-Session: https://claude.ai/code/session_01XNdjEPKLG637X8YmhDiEk5

* chore(scripts): round-7 — guard .mts/.cts too; drop dead export

Responds to the round-7 @claude review of #1799:

- Finding 1: verify-typecheck-coverage now requires a tsc pass for tracked
  `.mts`/`.cts` (not just `.ts`/`.tsx`), matching verify-format-coverage's
  extension set — `.mts` is already the shared-config idiom here, so a client
  `vitest.config.ts` → `.mts` rename can no longer drop out of both the tsconfig
  include and the guard's required set. No-op on today's tree (no client
  `.mts`/`.cts`); confirmed a tracked `.mts` with a type error is now caught.
  Ambient `.d.{ts,mts,cts}` stay excluded.
- Nit 2: drop the unused `export` on `rootReachedCommands` (internal to the lib;
  nothing imports it, and scripts/ isn't eslint-gated to catch dead exports).

Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
Claude-Session: https://claude.ai/code/session_01XNdjEPKLG637X8YmhDiEk5

* test: round-8 — single casts in HistoryTab, uniform wrappers, doc scope

Responds to the round-8 @claude review of #1799:

- Finding 1: reduce the `MALFORMED` helper and `entry()`'s return cast from
  `as unknown as` to a single `as` (both compile clean — the malformed shapes
  are comparable to the JSONRPCMessage union), and delete the false "only
  option" sentence. Fixes the AGENTS.md avoid-double-casts violation.
- Nit 3: write all seven FakeClient OAuth wrappers as
  `(...a: Parameters<InspectorClient["…"]>)` (the three zero-arg ones were
  hand-written `()` forwarders), so the block comment's "forward the same
  Parameters<…> tuple" is literally true and a future added parameter can't be
  silently dropped.
- Nit 2: update the five places that still described the guard as `.ts`/`.tsx`
  only to `.ts`/`.tsx`/`.mts`/`.cts` (script header + trackedSourceFiles JSDoc,
  AGENTS.md ×2, README.md).

Nits 4 (js config files out of the typecheck guard by construction) and the
footnoted callTool/ancestor-tsc items left as agreed.

Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
Claude-Session: https://claude.ai/code/session_01XNdjEPKLG637X8YmhDiEk5

* test: round-9 — drop entry()'s harmful cast; guards vouch for each other

Responds to the round-9 @claude review of #1799:

- Finding 1: remove `entry()`'s `as MessageEntry` return cast in
  HistoryTab.test.tsx. It was unnecessary (the spread of Partial<MessageEntry>
  over complete defaults is directly assignable) and harmful — it silenced type
  errors in the helper's OWN default fields (a `direction`/`method` typo now
  surfaces TS2820/TS2353). MALFORMED's single cast stays (load-bearing). This
  leaves MALFORMED as the file's only cast, matching what the comments describe.
- Nit 2: add `rootReachesScript` to scripts/lib/npm-scripts.mjs and have each
  coverage guard assert its SIBLING is still wired into the root `validate`. A
  guard can't detect being unrun itself, but the two now vouch for each other,
  so dropping either from `validate` is caught by the other (only deleting both
  slips through). Verified: removing verify:typecheck-coverage from validate
  now fails verify:format-coverage, and vice-versa.

Nits 3 (git-index visibility) and 4 (pre-existing as-never/as-unknown casts in
tests — out of scope, none added by this PR) left as agreed.

Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
Claude-Session: https://claude.ai/code/session_01XNdjEPKLG637X8YmhDiEk5

* test: round-10 — reject non-checking typecheck passes; pre/post hooks; docs

Responds to the round-10 @claude review of #1799:

- Finding 1: verify-typecheck-coverage now rejects a `-p <project>` whose own
  command carries `--noCheck` or `--listFilesOnly` — such a pass lists files
  without type-checking them, which previously satisfied both the wiring and
  file-coverage halves while checking nothing. (Splits each script on
  `&&`/`||`/`;` so a flag on one command can't taint another.) Verified a
  `typecheck` using `tsc … --noCheck` now fails with a clear message; `--noCheck`
  is a live idiom here (cli's test-servers:build), so this is a real copy-paste
  hazard.
- Nit 2: reachableScripts (shared lib) now also follows npm's implicit
  `pre<name>`/`post<name>` lifecycle hooks, so moving a gate into e.g.
  `prevalidate` is recognized instead of producing a false "not reachable"
  failure (cli already uses a `pretest` hook). Verified.
- Nit 3: AGENTS.md "Test placement" now states the Node clients (cli/tui/
  launcher) keep all tests in `__tests__/`, enforced by verify:typecheck-coverage
  (a co-located src test lands in no tsconfig project).
- Nit 4: noted on the factory comments that `Partial<…>` overrides admit an
  explicit `undefined` for a required field (exactOptionalPropertyTypes off).

Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
Claude-Session: https://claude.ai/code/session_01XNdjEPKLG637X8YmhDiEk5

* fix: round-11 — launcher build excludes tests; guard catches all noCheck vectors

Responds to the round-11 @claude review of #1799:

- Finding 1 (shipping bug): clients/launcher/tsconfig.json now excludes
  `**/*.test.{ts,tsx}` (matching cli/tui). launcher's `build` is an EMITTING
  tsc over src/**, and clients/launcher/build is in the root `files` allowlist,
  so a co-located `src/**/*.test.ts` would have been compiled into the published
  tarball. Excluding it also makes such a test land in no tsconfig project, so
  verify:typecheck-coverage catches it — making the AGENTS.md placement rule
  literally true for all three clients.
- Finding 2: verify-typecheck-coverage now catches both remaining ways a
  typecheck could check nothing: (a) case-insensitive `--noCheck`/`--nocheck`/
  `--listFilesOnly` on the command, and (b) `noCheck` set in the tsconfig itself
  (detected via `tsc --showConfig`, which surfaces it from the config or its
  extends chain). Verified both fail the guard.
- Nits 3 & 4: restructured into an explicit gate-integrity phase (wiring +
  neutered + config-noCheck + no-projects) that reports under its own accurate
  header and exits BEFORE the file-coverage pass — so an inert gate no longer
  prints file-remediation advice, nor buries the real cause under an
  "in no tsconfig project" line for every file that gate covered.

The one CIMD OAuth E2E integration test that flaked in the coverage run passes
32/32 in isolation and on re-run; unrelated to these script/config changes.

Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
Claude-Session: https://claude.ai/code/session_01XNdjEPKLG637X8YmhDiEk5

* fix: round-12 — exclude .mts/.cts tests from build; drop false integrity line

Responds to the round-12 @claude review of #1799:

- Finding 1 (tarball, again): the round-11 test exclude only covered
  `.ts`/`.tsx`, so a co-located `src/*.test.mts` still compiled into launcher's
  shipped `build/`. All three clients now exclude `**/*.test.*` (one glob,
  covering .ts/.tsx/.mts/.cts) — which is also exactly what AGENTS.md already
  claims. Verified a `src/probe.test.mts` is no longer emitted and now fails the
  guard.
- Finding 2: the gate-integrity phase no longer prints a false "names no
  `-p <project>`" line when config-`noCheck` disables projects — the condition
  now keys off `projects.length` (harvested), not `checking.length` (post-
  filter), so a client whose projects were all config-disabled shows only the
  accurate per-project lines.
- Nit 3: recorded the boundary on the phase comment — it does not detect
  shell-level failure suppression (`… || true`, `; exit 0`).
- Nit 5: the file-coverage remediation now leads with "for a co-located test,
  move it to `__tests__/`" (adding it to the src `include` would reintroduce the
  build emit) before the general add-to-include advice.

Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
Claude-Session: https://claude.ai/code/session_01XNdjEPKLG637X8YmhDiEk5

* chore(scripts): round-13 — implicit-tsconfig + derived clients; diagnostics

Responds to the round-13 @claude review of #1799:

- Finding 1: verify-typecheck-coverage now understands a `tsc` command with no
  `-p`/`-b` project flag — it resolves the implicit `./tsconfig.json` (tsc's own
  default), so the idiomatic `tsc --noEmit` form counts as coverage instead of
  producing false "in no tsconfig project" failures. Also harvests `-b`/`--build`
  project paths. Verified `tsc --noEmit && tsc --noEmit -p tsconfig.test.json`
  now reports OK.
- Finding 2: CLIENTS is no longer hardcoded — it's derived from the `cd
  clients/<x> && npm run validate` entries in the root `validate` chain (minus an
  explicit web exclusion), so a new Node client is auto-required, matching the
  repo-wide reach of its sibling verify-format-coverage. Added a
  guard-would-check-nothing bail if the derivation yields an empty set. Verified
  a new `clients/newclient` is picked up and its uncovered file flagged.
- Nit 3: the phase-2 remediation now leads with the general add-to-`include`
  advice and only appends the "move it to __tests__/" note when a failing path
  is actually a test file (was misdirecting plain files).
- Nit 4: AGENTS.md no longer says the test config includes "only" __tests__
  (launcher's also includes its vitest.config.ts).

(Two different web integration tests each flaked once under parallel load during
the coverage runs — both pass in isolation and on a clean re-run; timing-
sensitive and unrelated to these script/doc-only changes.)

Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
Claude-Session: https://claude.ai/code/session_01XNdjEPKLG637X8YmhDiEk5

* chore(scripts): round-14 — enumerate clients from disk (fail-closed)

Responds to the round-14 @claude review of #1799:

- Finding 1: replace the root-chain `CLIENTS` derivation (round 13) with a
  disk enumeration — every `clients/<name>` whose package.json has a `typecheck`
  script. The chain derivation was fail-open: a root-chain edit that kept
  running a client but didn't match the strict regex silently dropped it from
  the required set (112→53 files, still green). Disk is fail-closed — a client
  can't be silently dropped, a new client with a typecheck is auto-required, and
  web (no typecheck script; builds via `tsc -b`) is excluded on its own, so the
  explicit exclusion is gone. The existing `rootRunsClientValidate` wiring check
  still verifies the root chain runs each — now LOUD when it doesn't. Verified:
  `cd ./clients/tui && …` now hard-fails the integrity phase instead of silently
  shrinking coverage; `cd clients/tui && npm run build && npm run validate` stays
  gated. Removed the now-unused `clientsRunByRootValidate` from the shared lib.
- Nits 2 & 3: documented the two unreachable-today limitations on
  typecheckProjects — a solution-style `tsc -b` (empty `files` + `references`)
  would under-report, and the implicit-tsconfig fallback assumes no file operands.
- Nit 4: refreshed the file header to mention `-b`/`--build` + the implicit
  `./tsconfig.json` (only the JSDoc had been updated).

Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
Claude-Session: https://claude.ai/code/session_01XNdjEPKLG637X8YmhDiEk5

* chore(scripts): round-15 — enroll-or-exempt enumeration; normalize `cd ./`

Responds to the round-15 @claude review of #1799:

- Finding 1: the disk enumeration keyed on the presence of a script named
  `typecheck`, so renaming it silently dropped the client (112→53, still green)
  — contradicting the docstring. Now every `clients/*` with a readable
  package.json must EITHER declare a `typecheck` (→ enrolled) OR be in an
  explicit EXEMPT map (`clients/web`, reason: `tsc -b`); anything else is a hard
  gate-integrity failure. This is fail-closed on the rename axis too, and web's
  skip is a stated decision rather than an accident of which scripts it declares
  — so web gaining a `typecheck` no longer auto-enrolls it into the solution-`-b`
  false failures. Verified: renaming tui's `typecheck` now hard-fails; a `tsc -b`
  on web stays exempt.
- Nit 3: `rootRunsClientValidate` normalizes a leading `./`, so
  `cd ./clients/tui && npm run validate` (which genuinely runs it) is no longer a
  false "no longer runs" alarm.
- Nit 2: README/AGENTS/script-header prose no longer names a fixed
  cli/tui/launcher scope — they describe the disk-discovered "every clients/*
  with a typecheck script (web exempt)" rule.

Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
Claude-Session: https://claude.ai/code/session_01XNdjEPKLG637X8YmhDiEk5

* chore(scripts): round-16 — anchor wiring match; close enumeration axes

Responds to the round-16 @claude review of #1799:

- Finding 1 (only false-PASS): rootRunsClientValidate matched the client dir as
  a bare substring, so a prefix-sibling (`clients/tui` vs `clients/tui-next`)
  satisfied the wiring check for the shorter name — dropping a client from the
  root chain went green if a sibling was present. Now an anchored regex (escaped
  dir + optional leading `./` + a trailing boundary). Verified the sibling case
  now hard-fails.
- Nit 2: a `clients/*` dir holding tracked TS but no readable package.json is
  now a gate-integrity failure (was treated as "not a client"), so the
  "fail-closed on every axis" claim holds. Verified.
- Nit 3: readdirSync is wrapped, so a missing `clients/` dir yields the intended
  "found no clients/* dir" message rather than a raw ENOENT stack.
- Nit 4: EXEMPT's reason is now surfaced in the success line ("clients/web
  exempt: typechecks via tsc -b …"), so the exemption is visible at runtime, not
  only in source.

(Also moved `isRequiredSource` above nodeClients — nit 2's new tracked-TS check
calls trackedSourceFiles at module-init, which referenced it before init.)

Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
Claude-Session: https://claude.ai/code/session_01XNdjEPKLG637X8YmhDiEk5

* chore(scripts): round-17 — share quote-aware tokenizer between guards

Responds to the round-17 @claude review of #1799:

- Finding 1: the two coverage guards parsed shell strings differently —
  verify-format-coverage had a quote-aware tokenize(); verify-typecheck-coverage
  used a raw regex. So a quoted `-p "tsconfig.test.json"` (a form tsc accepts)
  made the typecheck guard report every file that project covers as uncovered,
  and a quoted `cd "clients/tui"` in the root chain tripped a false wiring
  failure. Moved tokenize() (now single+double quote) into the shared lib,
  imported it into both guards, rewrote typecheckProjects to walk tokens
  (`-p`/`--project`/`-b`/`--build` → next token), and strip quotes before the
  `cd` match in rootRunsClientValidate. Verified both quoted forms now pass.
- Nit 2: a stale EXEMPT key (a dir that no longer exists) was asserted in the
  success line unchecked. nodeClients now fails if an EXEMPT key isn't a real
  `clients/*` directory. Verified.
- Nit 3 (build/dist filter belt-and-braces) left as a recorded boundary.

Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
Claude-Session: https://claude.ai/code/session_01XNdjEPKLG637X8YmhDiEk5

* chore(scripts): round-18 — match tsc by basename; strip quotes on both halves

Responds to the round-18 @claude review of #1799:

- Finding 1 (regression from round 17): `tokens.includes("tsc")` didn't match a
  path-invoked binary (`node_modules/.bin/tsc`), which the previous `\btsc\b`
  regex did — so such a `typecheck` was falsely reported as naming no project.
  Now matches on the token basename (`isTsc`), keeping round-17's quoting fix
  while restoring the tolerance. Verified a `node_modules/.bin/tsc` typecheck
  reports OK.
- Nit 2: `rootRunsClientValidate` stripped quotes for the `cd` match but tested
  `npm run validate` against the unstripped string, so `npm run "validate"`
  false-failed. Both halves now test the stripped string.
- Nits 3 & 4: documented on typecheckProjects — the contrived `--noCheck false`
  (checking on) is still treated as disabling, and the `&&`/`||`/`;` split runs
  before tokenizing (a quoted operator inside an arg would split mid-token,
  unreachable for project paths).

Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
Claude-Session: https://claude.ai/code/session_01XNdjEPKLG637X8YmhDiEk5

* test: round-19 — typecheck test-servers' unimported bin; --prefix wiring form

Responds to the round-19 @claude review of #1799:

- Finding 1: `test-servers/src/server-composable.ts` (a `#!/usr/bin/env node`
  bin entry nothing imports) got no tsc pass — the barrel alias covers the other
  13 test-servers/src files transitively, but not this one, and its own build
  runs `tsc --noCheck`, so its errors were emitted into `test-servers/build`
  unchecked. Added it explicitly to `clients/cli/tsconfig.test.json`'s include
  (zero source changes; verified a bad line in it now fails cli's typecheck).
  Softened the AGENTS.md clause that implied the whole test-server source is
  transitively covered.
- Nit 2: `rootRunsClientValidate` now also recognizes the
  `npm --prefix <dir> run validate` invocation form (was `cd`-only).
- Nit 3 (stderr spy drops the completion-callback overload): left as-is —
  latent, not live (no exercised path awaits a stderr drain), and adding typed
  overload params would reintroduce the write-overload complexity the round-1
  vi.spyOn refactor removed.

Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
Claude-Session: https://claude.ai/code/session_01XNdjEPKLG637X8YmhDiEk5

* chore(scripts): round-20 — anchor validate tail; guard shared-source class

Responds to the round-20 @claude review of #1799:

- Finding 1 (false PASS): the wiring matcher's tail `/\brun validate\b/` also
  matched `run validate:fast` (`:` is a word boundary) — a DIFFERENT script that
  may skip typecheck. So the root chain could run a client's `validate:fast`
  while the guard kept measuring its `validate` (unrun), green. Anchored the
  tail to a boundary (`/\brun validate(?=$|[\s&;])/`). Verified `validate:fast`
  now hard-fails.
- Finding 2: durably close the test-servers/src class instead of point-fixing.
  Added a SHARED_ROOTS check — every tracked TS under `test-servers/src` + the
  root `vitest.shared.mts` (the `format:check:shared`/`lint:shared` surface) must
  land in the GLOBAL union of client projects. So a NEW unimported bin entry is
  caught (not just server-composable.ts, named in round 19). Verified a fresh
  `server-orphan-bin.ts` now fails. Zero source changes — the union already
  covers 15/15.
- Nit 4: the integrity message and rootRunsClientValidate docstring now mention
  the `npm --prefix` form too.

Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
Claude-Session: https://claude.ai/code/session_01XNdjEPKLG637X8YmhDiEk5

* chore(scripts): round-21 — deny-by-default non-client coverage

Responds to the round-21 @claude review of #1799 (findings 1 & 2, one fix):

- Replaced the `SHARED_ROOTS` allowlist with a deny-by-default non-client set:
  all tracked TS minus `clients/*` (per-client pass) minus a reasoned `core/`
  exemption (covered by web's `tsc -b`). This closes BOTH round-21 findings at
  once:
  - Finding 1 (stale allowlist fail-open): a rename of `test-servers/src` no
    longer silently drops it — its files reappear in the required "other" set
    and hard-fail (verified: `git mv test-servers/src test-servers/source` now
    fails instead of going 127→113 green). The subtractive `core/` exemption is
    fail-closed on staleness for the same reason.
  - Finding 2 (allowlist vs deny-by-default): a new top-level TS location is now
    auto-required (verified `tools/probe.ts` is flagged), matching the repo-wide
    reach of the sibling verify-format-coverage.
- Nit 3: added the shared-source clause to the README table row (AGENTS got it
  last round).

Header comment updated to describe the non-client coverage.

Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
Claude-Session: https://claude.ai/code/session_01XNdjEPKLG637X8YmhDiEk5

* chore(scripts): round-22 — narrow core/ exemption to what web's tsc -b covers

Responds to the round-22 @claude review of #1799:

- Finding 1: the `core/` exemption was whole-tree, but web's `tsc -b` only
  covers `core/**/*.ts` MINUS co-located `*.test.ts(x)` (its app project's
  include). So a `core/*.tsx`/`*.mts`/`*.cts` or a co-located `core` test got no
  tsc pass from anything. Narrowed the exemption to exactly that shape
  (`isExemptCoreFile`): a core `.ts` that isn't a co-located test. So a core
  `.tsx`/`.mts`/co-located test is now required in some project. Zero-fix today
  (0 such files); verified `core/mcp/probe-widget.tsx` and a co-located core
  `.test.ts` are now flagged.
- Nit 2: `EXEMPT_ROOTS` is now a Map (dir → reason) and its reason is surfaced
  in the success line alongside the client exemption.
- Nit 3: the non-client miss message is now the general "in no client's tsconfig
  project"; the `test-servers` remediation clause is conditional on an actual
  `test-servers/` path (tracked via `nonClientMisses`, not string-sniffing the
  composed message).
- Nit 4: success line says "(N clients + non-client)".

Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
Claude-Session: https://claude.ai/code/session_01XNdjEPKLG637X8YmhDiEk5

* chore(scripts): round-23 — core exemption mirrors both web projects

Responds to the round-23 @claude review of #1799:

- Finding 1: round 22's `isExemptCoreFile` mirrored only web's app project, but
  web's `tsc -b` also builds `tsconfig.test.json`, which includes
  `core/**/__tests__/**/*.{ts,tsx}`. So a `core/**/__tests__/*.test.ts` (a live
  layout — `core/mcp/__tests__/fakeInspectorClient.ts` already sits there) was
  flagged though web checks it. Now mirrors the UNION: under `__tests__/` →
  exempt `.ts`/`.tsx`; otherwise `.ts` minus co-located `*.test.ts(x)`. Verified
  a core `__tests__` test now passes while a core `.tsx` outside `__tests__`
  stays required.
- Finding 2: `EXEMPT_ROOTS` was a Map whose keys nothing read (round 22 replaced
  its filter with hardcoded `isExemptCoreFile`) — a phantom registry. Collapsed
  to a single `CORE_EXEMPT_REASON` const used in the OK line.
- Nit 3: README/AGENTS scope descriptions updated (core is no longer wholesale
  excluded; the non-client set is deny-by-default).

Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
Claude-Session: https://claude.ai/code/session_01XNdjEPKLG637X8YmhDiEk5

* chore(scripts): round-24 — enroll clients/web via tsconfig references

Responds to the round-24 @claude review of #1799:

- Finding 1: `clients/web` was whole-tree EXEMPT, but web's `tsc -b` only reaches
  what its four referenced projects include — a file at web's package root
  (e.g. `playwright.config.ts`) got no tsc pass and passed EVERY gate, fully
  silently. This was the last whole-tree exemption wider than the gate it
  defers to. Now web is ENROLLED: a client with no `typecheck` script but a
  `tsconfig.json` `references` array is measured through those reference
  projects (`tsc -p <ref> --listFilesOnly`), and is wired iff its `validate`
  runs `tsc -b` (`validateRunsTscBuild`). EXEMPT is now the empty escape hatch.
  Verified: a web-root `playwright.config.ts` with a type error is now flagged,
  and dropping `tsc -b` from web's build hard-fails the integrity phase.
  Zero-fix on the tree — the union of web's 4 projects covers all 625 tracked
  web files.
- Nit 2: assert `core/` exists (the OK line unconditionally states its
  exemption) — parity with the EXEMPT stale-key check.
- Nit 3: a `core/` miss now gets core-specific remediation (widen web's
  `tsconfig.app.json` include, not a cli/tui project).
- Nit 4: README/AGENTS scope descriptions corrected — web is enrolled (not
  exempt), and the exact core-exempt shape (app `*.ts` minus co-located tests +
  test `__tests__/**/*.{ts,tsx}`).

Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
Claude-Session: https://claude.ai/code/session_01XNdjEPKLG637X8YmhDiEk5

* chore(scripts): round-25 — drop redundant core exemption; guard tsc -b --noCheck

Responds to the round-25 @claude review of #1799:

- Finding 1: enrolling web (round 24) made the hand-maintained `core/` exemption
  both redundant and fail-open — web's measured `tsc -b` projects already cover
  core, and the mirror could go stale (narrowing web's include let an unimported
  core error pass). Deleted `isExemptCoreFile`, `CORE_EXEMPT_REASON`, the core
  presence assertion, and the exemptNote core clause. `core/` is now required
  like everything else and covered by measurement (127→894 files; a `core/*.tsx`
  web doesn't reach is now caught by measurement, not a hand-model). Kept the
  core-specific remediation clause.
- Finding 2: the reference (`tsc -b`) path skipped the non-inertness check — a
  `tsc -b --noCheck` build gated nothing while the guard stayed green. Turned
  `validateRunsTscBuild` into `tscBuildStatus` (ok/neutered/none); a neutered
  `tsc -b` now hard-fails, matching the typecheck-script path. Factored the
  disabling-flag predicate (`isDisablingFlag`) so both paths share it.
- Nit 3: `clientTsconfigReferences` now strips block comments too (every other
  tsconfig in the repo uses `/* */`).
- Nit 5: README/AGENTS wiring clause covers web's `tsc -b` (not just the
  `typecheck` script), and core is described as measured, not exempt.

Nit 4 resolves itself: EXEMPT is empty and the exemptNote/ternary now collapse
cleanly. Nit 6 (tsc -b bound to a specific tsconfig) left as recorded.

Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
Claude-Session: https://claude.ai/code/session_01XNdjEPKLG637X8YmhDiEk5

* chore(scripts): round-26 — expand solution configs wherever harvested

Responds to the round-26 @claude review of #1799:

- Finding 1: the reference (`tsc -b`) path was selected only by "no typecheck
  script", so adding `typecheck: "tsc -b"` to web (the natural uniformity move)
  flipped it to the typecheck-script path, where the harvested solution config
  lists nothing → 665 false failures. Fixed at the measurement layer:
  projectFiles now expands a solution config (`{"files":[], "references":[…]}`
  — lists nothing under --listFilesOnly) to its references and measures those
  recursively, so a `tsc -b` project is measured no matter which path harvests
  it. Generalized the references reader (tsconfigReferences(path)). Verified web
  with `typecheck: "tsc -b"` now passes. Dropped the stale "web … is out of
  scope" JSDoc clause.
- Nit 2: a `core/**/__tests__/**` miss now points at `tsconfig.test.json` (its
  actual owner), not `tsconfig.app.json`.
- Nit 4: AGENTS.md's one-line summary now mentions the non-client (`core/`,
  `test-servers/src`) coverage, matching the detailed bullet.

Nit 3 (EXEMPT empty, dormant stale-key check) left as the intended escape hatch.

Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
Claude-Session: https://claude.ai/code/session_01XNdjEPKLG637X8YmhDiEk5

* chore(scripts): round-27 — non-inertness follows references on both paths

Responds to the round-27 @claude review of #1799:

- Finding 1: round 26 made COVERAGE follow a solution config's references, but
  the non-inertness (`noCheck`) check on the typecheck-script path still only
  inspected the harvested solution (empty compilerOptions) — so a `noCheck` in
  a REFERENCED project was invisible there (caught only on the reference path).
  Factored a shared `resolveLeafProjects` (a solution → its real leaf projects,
  cached rawProjectFiles) used by BOTH coverage (`projectFiles`) and a shared
  `checkingLeaves` disable-check, so a `noCheck` in a referenced project is now
  caught on either path. Verified: `typecheck: "tsc -b"` + `noCheck` in
  tsconfig.app.json now hard-fails (was green). Caching also cut the guard from
  ~20s to ~11s.
- Nit 2: directory-form references (`{ "path": "./packages/a" }`) now resolve to
  `<dir>/tsconfig.json`, so a nested-solution dir reference is expanded.
- Nit 3: reworded the empty-listFiles comment (it's "solution config OR errored
  config", both of which the reference expansion handles).
- Nit 4: script header notes a `tsc -b` solution is reduced to its references on
  either enrollment path.

Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
Claude-Session: https://claude.ai/code/session_01XNdjEPKLG637X8YmhDiEk5

* chore(scripts): round-28 — dedup shared leaf in checkingLeaves

Responds to the round-28 @claude review of #1799 (which found no new gate hole):

- Finding 1: `checkingLeaves` dedup'd the `checking` array but ran
  projectDisablesChecking + integrity.push on every visit, so a leaf reachable
  from two harvested projects produced a duplicate `noCheck` line (and a
  redundant `tsc --showConfig`). Now tracks visited leaves in a Set and skips an
  already-seen leaf entirely. Verified a duplicate harvested project yields one
  line, not two.
- Nit 3 (record-completeness): noted in AGENTS that the one tier the guard
  structurally can't see — a per-file `// @ts-nocheck` — is owned by
  `@typescript-eslint/ban-ts-comment` across lint:core/lint:shared/each client.

Nit 2 (the ~11s perf claim isn't reproducible on a CI-class runner) and nit 4
(hybrid-solution / composite boundaries, both tsc-rejected → unreachable) left
as recorded.

Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
Claude-Session: https://claude.ai/code/session_01XNdjEPKLG637X8YmhDiEk5

* test(scripts): unit-test the typecheck-coverage guard's parsers

Responds to the round-29 @claude review of #1799 (no new gate hole; the finding
is that the guard itself has no tests):

- Made the guard's pure helpers importable: exported `typecheckProjects`,
  `tscBuildStatus`, `tsconfigReferences`, `clientTsconfigReferences`,
  `resolveLeafProjects`, `isTsc`, `isDisablingFlag`, `isRequiredSource`, and
  moved the module-init + phase execution behind an exported `main()` run only
  when the file is executed directly (importing it for tests no longer runs the
  guard). Behavior when run directly is unchanged (894 files).
- Added table-driven tests (node's built-in `node --test`; the root has no
  vitest harness by design) for the shared lib (`tokenize`, `reachableScripts`
  incl. pre/post hooks, `rootRunsClientValidate` across cd/--prefix/quoted/
  prefix-sibling/`validate:fast`, `rootReachesScript`) and the guard's parsers
  (`isRequiredSource`, `isTsc`, `isDisablingFlag`, `typecheckProjects`,
  `tscBuildStatus`) — one case per rule a review round found (r7/r10/r13/r15/
  r16/r17/r18/r19/r20/r25). 12 tests. Wired `test:scripts` into `validate`.
- Nit 2: a `scripts/*.ts` miss now gets a scripts-specific remediation (keep it
  `.mjs`, or give `scripts/` its own tsconfig project).
- Documented the guard's test suite in AGENTS.md.

Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
Claude-Session: https://claude.ai/code/session_01XNdjEPKLG637X8YmhDiEk5

* test(scripts): make test:scripts self-guarding; fill parser test gaps

Responds to the round-30 @claude review of #1799 (mutation testing killed 5/5;
findings are about the new test gate + suite completeness):

- Finding 1: `test:scripts` lacked the two properties the guard enforces for
  every tsc pass. verify-typecheck-coverage now asserts, in phase 1, that
  `test:scripts` is reachable from the root `validate` (via the already-imported
  rootReachesScript) AND that its tracked `scripts/**/*.{test,spec}.*` file set
  is non-empty and every file is matched by the command's glob (a new exported
  `globToRegExp`). So dropping it from validate, or renaming a test to
  `*.spec.mjs`/`*.test.mts` (which node --test silently skips), now hard-fails.
  Verified both.
- Nit 2: extracted a pure `parseTsconfigReferences(raw)` from tsconfigReferences
  and table-tested the JSONC tolerance (block/line comments, trailing comma, no
  references, malformed, no path) — the r17/r26 rules had no cover.
- Nit 3: added `-b`/`--build`/bare-`tsc -b` and explicit `--project` cases to
  typecheckProjects's test so its title matches its table.
- Nit 4: rewrote the rootRunsClientValidate test to put `cmd` in a script
  reached from validate, and added the reachability cases (an orphan cd-validate
  must NOT count; validate:tui indirection must) — the r5 property.
- Nit 5: added test:scripts to AGENTS's validate ordering + the README table.

15 guard tests now.

Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
Claude-Session: https://claude.ai/code/session_01XNdjEPKLG637X8YmhDiEk5

* chore(scripts): round-31 — glob harvest follows delegation; brace/? globs

Round-31 review findings, both false failures in the new `test:scripts`
self-guard:

1. The glob half read `rootPkg.scripts["test:scripts"]` literally while the
   wiring half two lines above used `reachableScripts`, so a delegating
   `test:scripts` (`npm run test:scripts:lib && npm run test:scripts:guard`)
   — which genuinely runs all 15 tests — reported every tracked test file as
   "not matched by the glob", advising a rename the files already satisfy.
   Now harvested across every script reachable FROM `test:scripts`, the same
   fix round 6 applied to the `typecheck` harvest; `pretest:scripts` counts
   for free.

2. `globToRegExp` escaped `{`/`}` as literals and left `?` as a regex
   quantifier, both divergent from `node --test`'s glob. `*.{test,spec}.mjs`
   is the natural widening here (the guard's own file scan already treats a
   `.spec.` file as a test) and false-failed; `?` silently meant "optional
   previous char" and threw outright when segment-leading. Adds `?` → `[^/]`
   and a brace-alternation pass, with an unbalanced brace falling back to a
   literal.

Nit 3: `resolveLeafProjects`'s two path-resolution rules (the r26
directory-form reference, and refs resolving against the referring config's
dir) extracted as `projectConfigFile`/`refToProject` so they are one-row
tables like the other parsers, rather than untestable behind a `tsc` spawn.

Nit 4: AGENTS.md + README document that the guard now enforces `test:scripts`
on three axes (wired, non-empty, glob-covered).

Verified: probes C and D exit 0; probe B (rename to `*.spec.mjs`) still
caught; `test:scripts` 18/18; `verify:typecheck-coverage` OK 894 files;
`verify:format-coverage` OK 919; `npm run ci` green.

Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
Claude-Session: https://claude.ai/code/session_014vgJ6ZYj9VLhfSjgua48zw

* chore(scripts): round-32 — measure globs with node's own matcher

Round-32 review; both findings point at deleting code.

1. `globToRegExp` hand-modeled `node --test`'s glob and was still one form
   short — character classes (`*[.]test.mjs`, `*.test.[mc]js`) hit the escape
   set and became literals, so the guard false-failed on files node genuinely
   runs. Replaced the whole translator (and its brace balance pre-scan) with
   node's `path.matchesGlob` — 22.5.0+, and the repo floor is >=22.19.0. Same
   *measure, don't model* move round 25 made when it deleted `isExemptCoreFile`
   in favor of measuring web's projects; being exact for every form node
   accepts is the point, since node's matcher is the thing being modeled.

2. The round-31 glob harvest was inline in `main()`, so mutation-reverting it
   to the literal single-script read left the suite green — the commit's
   headline fix was its one untested change. Extracted as `testScriptGlobs`,
   with rows for the direct form, the delegating form (both child globs, not
   `npm`/`run`/`<name>`), a `pretest:scripts` hook, and an unreachable script
   contributing nothing.

Nit 3: `resolveLeafProjects`' JSDoc moved back onto it (the extraction left it
stranded above `projectConfigFile`, which had two stacked doc blocks).

Nit 4a: a `test:scripts` naming no path/glob harvested nothing, and `.some()`
over an empty list blamed every test file with unfollowable advice. Now one
explicit message naming the real problem.

Verified: probes for the direct, delegating, brace, `[.]` and `[mc]` forms all
exit 0 (the last two were false failures before); probe B (rename to
`*.spec.mjs`) still caught; bare `node --test` reports the new single message;
`test:scripts` 18/18; `verify:typecheck-coverage` OK 894; `npm run ci` green.

Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
Claude-Session: https://claude.ai/code/session_014vgJ6ZYj9VLhfSjgua48zw

* chore(scripts): round-33 — scope the glob harvest to `node --test` segments

1. False pass. `testScriptGlobs` kept every non-flag token from every reachable
   script, so a glob belonging to a NEIGHBOURING command was attributed to the
   test runner — and since `.some()` needs only one glob to match, a broader
   one suppressed a real miss rather than adding an inert pattern. A
   `pretest:scripts` of `prettier --check "scripts/**/*.mjs"` (reachable via
   the pre-hook round 31 added) was enough: the probe-B rename to `*.spec.mjs`
   then passed while `node --test` silently ran 12 of 18 tests. Now scoped per
   command segment to the ones invoking `--test`, exactly as
   `typecheckProjects` gates on `tokens.some(isTsc)`. The JSDoc paragraph
   claiming a surplus token can't suppress a miss is corrected — it was false.

2. The gate-integrity block is extracted as `testScriptProblems(scripts,
   testFiles)`, pure and outside `main()`. The nit-4a branch added last round
   was the third consecutive commit whose newest branch sat outside the test
   seam and mutation-survived; extracting the block ends the pattern rather
   than pinning one more condition from the outside.

Verified. The finding-1 probe (prettier pre-hook + `*.spec.mjs` rename) now
exits 1 where it exited 0; the hook alone still exits 0. Mutation testing over
the whole helper — empty-harvest branch to `if (false)`, segment gate removed,
wiring axis removed, non-empty axis removed, segment split removed,
`reachableScripts` to `Object.keys`, `matchesTestGlob` to equality — kills
7/7, including the branch that survived the last three rounds.
`test:scripts` 21/21 · `verify:typecheck-coverage` OK 894 ·
`verify:format-coverage` OK 919 · `npm run ci` green.

Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
Claude-Session: https://claude.ai/code/session_014vgJ6ZYj9VLhfSjgua48zw

* chore(scripts): round-34 — harvest only positional args; normalize `./`

Both findings are the axis round 33 fixed, one level in.

1. False pass. The segment gate stopped a neighbouring COMMAND's glob, but
   within a kept segment the filter was still "any non-flag token", so a
   glob-valued flag's VALUE was harvested as a positional arg.
   `--test-coverage-include "scripts/**/*.mjs"` is broader than the test glob
   and matches a renamed `*.spec.mjs`, so it vouched for the very file the
   runner skips — and it isn't hypothetical: `scripts/` sits outside the ≥90
   coverage gate, so that flag is the obvious next thing added here. Tokens
   preceded by a value-taking `--test-*` flag are now dropped.

2. False failure. `./scripts/**/*.test.mjs` — which `node --test` accepts, and
   the common npm-script idiom — matched nothing, because `git ls-files` emits
   no `./`. Both files were then blamed for a rename that never happened, the
   unfollowable-remediation shape fixed for the bare-runner case last round. A
   leading `./` is normalized off, as `rootRunsClientValidate` already does.

Nit 4: the gate-integrity footer's typecheck-wiring advice is now conditioned
on at least one issue being about the typecheck wiring — a `test:scripts`
failure carries its own per-line remediation, and appending `--noCheck`/`tsc
-b` advice under a renamed-test-file message sends the reader after the wrong
thing.

Verified. The coverage-include probe with the rename now exits 1 where it
exited 0, and exits 0 without the rename; the `./` probe exits 0 where it
exited 1; a `test:scripts`-only failure no longer prints the typecheck footer.
Mutation sweep 9/9 killed, including both new rules. `test:scripts` 22/22 ·
`verify:typecheck-coverage` OK 894 · `verify:format-coverage` OK 919 ·
`npm run ci` green.

Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
Claude-Session: https://claude.ai/code/session_014vgJ6ZYj9VLhfSjgua48zw

* chore(scripts): round-35 — fourth axis: flags that narrow the run

1. False pass on a new axis. The three axes ask whether the tests are run,
   whether there are any, and whether the runner is told about each — none asks
   what it then DOES with them. `--test-shard 1/2` keeps every glob intact and
   executes 6 of 22 tests, exiting 0, with the guard green; `--test-only` runs
   2, as does a non-matching `--test-name-pattern`. This is `--noCheck` for the
   test runner, one domain over. Adds `NARROWING_TEST_FLAGS` and a fourth axis
   in `testScriptProblems`. Three of those flags were already in
   `VALUE_TAKING_TEST_FLAGS` — the guard knew them well enough to drop their
   values but not that they shrink the run. Both `--flag value` and
   `--flag=value` report as the bare flag name.

2. The footer conditioning added last round was itself inside `main()` — the
   pattern the same commit's comment documented, recurring in the commit that
   documented it. Extracted as `integrityAdvice(problems, nonTypecheck)`.

   Doing that fixes nit 3 in the same move: the old condition keyed off
   `testScriptIssues` specifically, so it covered one of three non-typecheck
   categories. A sibling-vouch failure ("the root validate no longer runs
   verify:format-coverage") and client-enrollment problems still got
   `--noCheck`/`tsc -b` advice about a typecheck pass they have nothing to do
   with. All three sets are now excluded.

Verified. `--test-shard 1/2`, `--test-only` and `--test-name-pattern zzz` each
now exit 1 naming the flag, while node runs 6/2/2 of 22. A sibling-vouch
failure no longer prints the typecheck footer. Mutation sweep 12/12 killed,
including all three new rules and the branch this commit replaces.
`test:scripts` 24/24 · `verify:typecheck-coverage` OK 894 ·
`verify:format-coverage` OK 919 · `npm run ci` green.

Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
Claude-Session: https://claude.ai/code/session_014vgJ6ZYj9VLhfSjgua48zw

* chore(scripts): round-36 — discover test files by content, not name

1. False pass on the axis underneath the other four. The required test-file set
   was built with the same name pattern the `test:scripts` glob uses, so a
   rename escaping BOTH was invisible: the file left `testFiles` entirely, so
   nothing asked whether the runner was told about it, and the survivor
   satisfied the non-empty axis. A dot→hyphen slip
   (`npm-scripts.test.mjs` → `npm-scripts-test.mjs`) dropped 6 of 24 tests with
   every gate green — and the file that stops running is the one pinning the
   r16 prefix-sibling and r20 tail anchors, the series' two other false passes.

   Round 30 fixed "renamed to a form the glob misses" by broadening the name
   pattern; a fifth name pattern only moves the boundary again. `node --test`
   decides what a test is by whether it registers tests, so `isScriptsTestFile`
   reads that instead — the same measure-don't-model move as r25 and r32. It
   also covers what no name pattern can: a NEW test file added under a name the
   glob misses, never run and never noticed.

Nit 2: `enrollmentProblems` no longer suppresses the footer. "declares no
`typecheck` script …" IS about the typecheck pass and the footer's first clause
is its canonical fix; excluding it lost the advice for the r15 rename scenario.
The criterion is "is this about the typecheck pass at all", not "does it carry
per-line advice" — several typecheck problems carry their own and still want
the footer. JSDoc corrected accordingly.

Nit 3: the complementary-shard boundary (`1/2` + `2/2` across two reachable
scripts genuinely runs everything, and is flagged anyway) recorded on the
helper, alongside the guard's other documented boundaries.

Nit 4: the three `--test-coverage-{lines,branches,functions}` thresholds added
to VALUE_TAKING_TEST_FLAGS. Their values are numeric and so inert rather than
suppressing, but model completeness is cheaper than re-deriving that later.

Verified. The hyphen rename now exits 1 naming the file; the `.spec.mjs` rename
is still caught; an enrollment failure prints the footer again; the guard is
unchanged at 894 files OK (exactly the 2 tracked test files reference
`node:test`, none of the other 12 `scripts/` files do). Mutation sweep 14/14.
`test:scripts` 25/25 · `verify:format-coverage` OK 919 · `npm run ci` green.

Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
Claude-Session: https://claude.ai/code/session_014vgJ6ZYj9VLhfSjgua48zw

* chore(scripts): round-37 — register-not-mention; guard the discovery read

Both findings were introduced by round 36's content predicate.

1. False failure. The predicate matched a bare `node:test` mention, so a shared
   helper importing the runner's `mock` API — registering no tests — was
   enrolled as a required test file and told to rename itself to `*.test.mjs`,
   which would only make node run an empty file. `scripts/lib/` is already the
   repo's shared-helper location, so this is the plausible next file there.
   Tightened to node's actual criterion: an import/require OF `node:test` AND a
   test registration.

2. Regression. `readFileSync` was passed as an argument, so it ran for every
   tracked `scripts/` file before the predicate's extension gate could reject
   it — a file in the index but not the worktree (`rm` without `git rm`) took
   the guard down with a raw ENOENT stack, aborting phase 1 before any real
   finding printed. Discovery did no reads at all before round 36. Extension is
   now gated first and the read wrapped: an unreadable tracked file warns and
   is skipped, the round-6 precedent where a failed `tsc` echoes its diagnostic
   instead of dying.

Verified: the mock-helper probe exits 0; the tracked-but-deleted probe exits 0
with a one-line skip warning instead of a stack; the dot→hyphen rename is still
caught. Mutation sweep over the predicate 3/3 (import form, registration,
extension). `test:scripts` 25/25 · `verify:typecheck-coverage` OK 894 ·
`verify:format-coverage` OK 919 · `npm run ci` green.

Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
Claude-Session: https://claude.ai/code/session_014vgJ6ZYj9VLhfSjgua48zw

* chore(scripts): round-37 nit 3 — one definition of the JS-extension check

Finding 2's fix hoisted the extension gate into `main()` (it must run before
the read), leaving a second copy of the regex inside the predicate that reads
as protection but can never fire there. Extracted as `isJsFile` and used in
both positions, so the two can't drift.

Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
Claude-Session: https://claude.ai/code/session_014vgJ6ZYj9VLhfSjgua48zw

---------

Co-authored-by: Claude Opus 4.8 (1M context) <[email protected]>

* fix(core): start OAuth on a modern/auto-era connect (#1805)

The SDK's `server/discover` negotiation probe runs before anything else on
`protocolEra: "auto" | "modern"`, and its classifier reports whatever the
transport threw as `SdkError(ERA_NEGOTIATION_FAILED)` with the real error moved
to `data.cause`. That buried both connect-time auth signals: the remote path's
`AuthRecoveryRequiredError` (matched with `instanceof`, and carrying the
authorization URL) and a direct transport's `AuthChallengeError`. An
OAuth-protected server that authorized fine on the legacy era instead produced a
dead-end "Version negotiation probe failed" — no redirect, and the connection
status flipped to `error` despite `isConnectAuthRecoveryError` existing to
prevent exactly that.

- `findNestedAuthError()` walks `cause` / `data.cause` for either typed error.
  Not gated on the SDK's error code or message, so it survives rewording.
- `connect()` unwraps the rejection at the earliest point, ahead of
  `withDirectAuthRecovery` (whose `isAuthChallengeError` check is shallow), the
  outer status guard, and every client's connect error handling — so web, CLI,
  and TUI are all fixed without a client-side change.
- On the direct path, enable `interceptAuthChallenges` for the probing eras even
  with no stored tokens. Without an authProvider the 401 reaches the SDK as a raw
  `SdkHttpError`, and the classifier ignores the HTTP status (it only looks for a
  JSON-RPC error body) — so pin mode rethrew it with the 401 discarded entirely,
  no status and not even a cause for the walk to find.

Verified against a live dual-era server that answers 401 on both `initialize`
and `server/discover`: `auto` and `modern` now reject with
`AuthRecoveryRequiredError` carrying a real authorization URL. Each half of the
fix is load-bearing — reverting either fails the new integration tests.

Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
Claude-Session: https://claude.ai/code/session_013SNjAqoEhUVVkr3UStR6om

* docs(core): mark the era-probe challenge interception as a removable workaround

Per review on #1806: the `|| this.probesProtocolEra()` clause compensates for
an upstream SDK gap (modelcontextprotocol/typescript-sdk#2561, tracked as
#1807). Make its lifespan explicit so whoever picks up #1807 knows this block
is the thing to delete, rather than having to re-derive it.

Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
Claude-Session: https://claude.ai/code/session_016S7h83UKNQ7kvSiFDNksCt

* fix(core): resolve connect() when auth recovery is satisfied silently

Review findings on #1809:

- connect() built its handshake promise once, outside the runConnect closure
  withDirectAuthRecovery retries. On a "satisfied" outcome the recovery runs a
  complete nested connect() via reconnectAfterAuthRecovery(), so the retry leg
  re-awaited the already-rejected promise and rejected a connect() whose client
  was in fact connected. Short-circuit the retry when already connected, and
  skip the post-connect block the nested connect already ran (a duplicate
  `connect` event re-triggers every list-state manager's refresh).
- Document the accepted 403 side effect of enabling interceptAuthChallenges
  with no stored tokens, and why every `{ pin }` counts as probing.
- Replace the private-method double cast in the era-probe test with an
  observable assertion on the transport options, and split the non-auth
  failure case per era instead of a message alternation that passed either way.

Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
Claude-Session: https://claude.ai/code/session_016S7h83UKNQ7kvSiFDNksCt

* fix(core): key the connect retry leg off attempt, bound nested recoveries

Second-pass review findings on #1809:

- The satisfied-recovery short-circuit keyed off live connection status, so an
  onclose landing between the nested connect() and the retry leg fell through
  to the rejected handshake promise and reported the stale 401 as the reason
  the session died. withDirectAuthRecovery now passes `attempt` to its
  operation, so connect() identifies the retry leg structurally and reports the
  teardown when the recovered session is already gone.
- Bound nested silent recoveries. The `attempt >= 1` guard is per-call and a
  satisfied outcome recovers via a nested connect() with a fresh counter, so a
  server that keeps rejecting freshly refreshed tokens recursed without bound.
  Counted across the nesting boundary, capped at 3.
- Strengthen the regression test (server info populated by the nested connect,
  no duplicate statusChange) and add the two missing cases: a failed reconnect
  still rejects, and the recursion bound terminates.

Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
Claude-Session: https://claude.ai/code/session_016S7h83UKNQ7kvSiFDNksCt

* test(core): cover the recovery teardown branch; fix a duplicated case

Third-pass review findings on #1809, both about tests added in the previous
commit:

- The retry leg's teardown error had no coverage, so a regression to the old
  status fall-through would have stayed green. Covered deterministically: the
  nested connect dispatches `connect` after setting "connected", so closing the
  live transport from that listener flips the status before the retry leg runs.
  Asserts the error identity, which is the point of the change.
- "still rejects when the reconnect underneath the recovery fails" used a
  challenging transport for every connect, so it walked the same bounded chain
  as the recursion-bound test rather than covering a non-auth reconnect
  failure, and its comment claimed an "error" status the auth path never
  reaches. Now fails the reconnect with a plain error and asserts both.

Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
Claude-Session: https://claude.ai/code/session_016S7h83UKNQ7kvSiFDNksCt

---------

Co-authored-by: rinormaloku <[email protected]>
Co-authored-by: Claude Opus 5 (1M context) <[email protected]>

…st (#1808) (#1810)

* fix(auth): offer explicit re-authorize when callback state is lost (#1808)

A `/oauth/callback` that arrives with no recorded SEP-2352 discovery state
made the SDK throw `AuthorizationServerMismatchError` and the Inspector
dead-ended, surfacing the raw SDK text with no path forward.

The SDK folds two very different failures into that one error class, so
`core/auth/issuerBinding.ts` classifies them structurally: a recoverable
"lost authorization state" (the SDK puts prose, not an issuer URL, in the
`recordedIssuer` slot) versus a genuine cross-AS `issuer_mismatch`. Only
the first gets a recovery affordance.

Web: the callback failure now raises a dedicated banner ("Authorization
state was lost" / "Authorize again") whose action clears the stale OAuth
state for that server and starts a fresh authorization. A genuine
mismatch stays a red, non-dismissible security notification with both
issuers named and no one-click retry.

Node runners (CLI/TUI): the same classification rewrites the callback
rejection into the shared actionable copy instead of the SDK text.

Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
Claude-Session: https://claude.ai/code/session_016S7h83UKNQ7kvSiFDNksCt

* fix(auth): detect the SDK mismatch error via isInstance (#1808)

The callback-leg classifier gated on `err.mcpBrand`, but the SDK declares
that brand in a `static {}` block, so it lives on the constructor and is
never reachable from an instance. The predicate always returned false, so
`findIssuerBindingFailure()` never matched and the #1808 recovery banner
was inert in production — the callback dead-ended exactly as before.

Use the SDK's own `AuthorizationServerMismatchError.isInstance()`, which is
cross-copy safe by construction (it consults a `Symbol.for()`-keyed brand
set), with a `name` fallback for a serialization boundary. Correct the
module doc, which cited cross-bundle safety as the reason to avoid
`instanceof` — the very thing the SDK's branded `hasInstance` provides.

Every fixture now constructs a real `AuthorizationServerMismatchError`
instead of a hand-rolled object with an own `mcpBrand`, which is why a
green `npm run ci` hid this. Adds a test pinning the brand's placement so
the feature cannot silently go inert again.

Review follow-ups: drop the unused `isLostAuthorizationStateError`, remove
the duplicated banner title default, resolve the banner title/action label
where the banner is raised, keep the clear-failure notification open, and
bound remote-supplied issuers in user-facing copy.

Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
Claude-Session: https://claude.ai/code/session_016S7h83UKNQ7kvSiFDNksCt

---------

Co-authored-by: Claude Opus 5 (1M context) <[email protected]>

* docs: add CONTRIBUTORS.md issues-only policy (#1517)

Document the contribution policy: we accept issues, not pull requests.
Maintainers handle design and implementation through a prompt-driven
workflow. Contributors who have already built a change locally should
share the prompt they used rather than a diff.

Cross-reference the policy from AGENTS.md.

Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>

* docs: add PR template redirecting external authors to issues-only policy (#1517)

GitHub auto-populates pull_request_template.md into every new PR body,
so it steers would-be external PR authors to CONTRIBUTORS.md before they
submit a diff.

Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>

* docs: drop version-board details from CONTRIBUTORS.md (#1517)

The policy takes effect after v2/main merges into main, leaving a single
line of development. Tell contributors to open a well-formed issue rather
than routing them to per-version project boards.

Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>

* docs: add version-board/label table to CONTRIBUTORS.md (closes #1595)

Completes requirement #2 of #1517: a table mapping each active version to
its base branch, project board, and version label, plus a "Label by
version" note mirroring AGENTS.md.

Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
Claude-Session: https://claude.ai/code/session_01S3fTN8H3R8YV4yUGvZjYnX

* docs: drop v1.5 from CONTRIBUTORS.md, invite deeper contribution (#1517)

Review feedback on PR #1604:

- Remove the v1.5 row/bullet from the version table — v1.5 was an internal
  intermediate branch that external contributors never saw, so surfacing it
  in an outward-facing doc is noise. The table is now v1 / v2 only.
- Add a "Want to work on the Inspector with us?" section pointing at the MCP
  Contributor Discord (#inspector-dev) and the community calls, framing the
  issues-only rule as being about unsolicited patches rather than a closed
  door, with supervised work as the path toward maintainership.
- Link that section from "Why this policy exists", and add the same Discord
  pointer to the PR template.

Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
Claude-Session: https://claude.ai/code/session_01Txmv2qqv3yeKgRzoqXytzD

* docs: drop project-board links from CONTRIBUTORS.md (#1517)

The org project boards aren't visible to anyone outside the organization, so
linking them in an outward-facing doc points contributors at a 404. The
version table is now base branch + label only, and the PR template names the
version labels instead of "the appropriate version board".

Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
Claude-Session: https://claude.ai/code/session_01Txmv2qqv3yeKgRzoqXytzD

* docs: v1 takes security fixes only (#1517)

Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
Claude-Session: https://claude.ai/code/session_01Txmv2qqv3yeKgRzoqXytzD

* docs: use post-runbook branch names (v1/main, main) (#1517)

This doc will be read after the branch runbook completes, when v1 lives on
v1/main and v2 is the default main. Update the version table, the label-by-
version line, and the PR template's label hint + CONTRIBUTORS.md links
accordingly.

Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
Claude-Session: https://claude.ai/code/session_01Txmv2qqv3yeKgRzoqXytzD

---------

Co-authored-by: Claude Opus 4.8 (1M context) <[email protected]>

* docs: refresh V2 board Status option IDs in AGENTS.md (#1823)

`V2 Go Live` (`b3a6966e`) was added to the Status field via the web UI on
2026-07-27 for the go-live phases (#1804) and was not in the table; the
`SDK V2 + New Spec` row it replaced in position (`1bbb6f57`) had already
been removed, so following the table failed with "option Id does not
belong to the field". Record the current five options, note that adding
the column via the web UI left the other four IDs untouched (evidence the
UI path is safe, unlike the GraphQL full-replace hazard documented below
it), and list every removed column's dead ID in one place.

Also add the missing recipe for moving a card that is already on the board:
the existing recipes only cover `item-add` for a new item, leaving no
copy-paste path for looking an existing card's item id up by issue number.

Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
Claude-Session: https://claude.ai/code/session_01MvKF2mqi4hoQSUqQpZXNTq

* docs: close the ID-rotation pointer over the new recipe (review #1825)

The hazard note's maintenance pointer lists the hardcoded option ids that
must be updated when they rotate, but the recipe added in the previous
commit hardcodes a third one (159c8a02) that the pointer did not name —
reproducing the staleness class this PR exists to fix. Add it.

Also fold in two adjacent fixes: the "In review" casing left behind two
lines from the "In progress" → "In Progress" fix (labels are matched
exactly when re-fetching ids by name), and a note that --limit must stay
above the board's item count, since exceeding it truncates silently and
surfaces as an opaque node-resolution error from item-edit.

Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
Claude-Session: https://claude.ai/code/session_01MvKF2mqi4hoQSUqQpZXNTq

* docs: move the --limit caveat inside the recipe fence (review #1825)

The section tells the reader to copy the recipes verbatim, so guidance that
lives only in the prose above the fence is lost by anyone who grabs the block
— which is the whole point of the block. The two recipes above it already
carry their context as in-block comments; match that.

Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
Claude-Session: https://claude.ai/code/session_01MvKF2mqi4hoQSUqQpZXNTq

---------

Co-authored-by: Claude Opus 5 (1M context) <[email protected]>

…1814)

* fix: wait for the TUI to exit before removing its temp dir (#1801)

`done()` sent SIGTERM and then immediately `rmSync`'d the work dir, which
doubles as the TUI's HOME. Nothing waited for the child, so the Ink process
could still be writing into the dir while rmSync walked it — a file landing
after a directory was read but before it was removed surfaces as ENOTEMPTY
on macOS, failing `smoke:tui` after a successful render.

Removal now happens only once the child's `exit` event fires: SIGTERM, then
SIGKILL after SMOKE_TUI_EXIT_GRACE_MS (default 5s), then clean up regardless
after twice that. The crash-before-render path short-circuits via a
`childExited` flag set by the pre-registered exit handler. cleanup() also
warns instead of throwing, so a leftover temp dir can never fail a passing
smoke.

Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
Claude-Session: https://claude.ai/code/session_01MvKF2mqi4hoQSUqQpZXNTq

* fix: wait on close, not exit, so spawn failure resolves instantly (review #1814)

A failed spawn emits `error` + `close` and never `exit`, so the previous
`child.once("exit")` wait never fired on that path: done() sent SIGTERM to a
process that never existed and resolved only via the give-up timer — 10s at
the default grace, with a warning blaming a SIGTERM the child never received.
Waiting on `close` covers both normal termination and spawn failure, and
additionally guarantees the stdio pipes are drained.

The short-circuit gate moves with it, `childExited` → `childClosed`: keying it
on `exit` would have skipped the drain on the crash-before-render path, which
is exactly where the quoted output matters. Messages that quote that output are
now thunks rendered in finish(), after the drain, instead of being sliced at
call time. SIGTERM is now sent only to a child that is still running, since on
both the crash and spawn-failure paths there is nothing left to signal.

finish() also gets a `finished` guard so its single-entry invariant is local
rather than inferred from process.exit() being synchronous.

Measured, spawn failure (grace shortened to 500ms): 1.04s + spurious warning
before, 0.02s and silent after. Crash-before-render now prints the child's
stderr detail line that the pre-drain slice could truncate. Happy path 3x,
timeout path, and full `npm run smoke` + `npm run validate` all green.

Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
Claude-Session: https://claude.ai/code/session_01MvKF2mqi4hoQSUqQpZXNTq

* fix: bound the drain, and make the give-up warning true (review #1814 r2)

Three follow-ons from the second review pass:

The give-up warning still said the TUI "did not exit within Xms of SIGTERM",
but after switching the wait to `close` neither clause is reliably true — the
timer's most plausible trigger is now a child that has already exited, and on
the crash and spawn-failure paths no SIGTERM is sent at all. That is the same
defect this PR fixed one round ago (a warning naming a signal never sent), so
the deadline is now re-armed with a message describing what is actually being
awaited.

`close` is bounded by whoever holds the stdio pipes, which can outlive the
direct child: a descendant that inherited them keeps it pending. That is the
one real cost of `exit` → `close`, and it is moot only because the TUI spawns
nothing at boot — a reason that lives entirely outside this file. Once `exit`
has fired the deadline drops to DRAIN_MS, keeping the drain guarantee without
inheriting an unrelated process's lifetime. Measured against a grandchild
holding the pipes for 3s: 0.54s and an accurate warning, instead of 3s.

The quoted output is sliced from the tail rather than the head. The thunk added
last round exists to capture bytes that arrive late, which a head slice then
discards as soon as output exceeds 800 chars — and on the timeout path the last
frame is what diagnoses a stuck render, not the first. Verified: a 2KB stderr
crash now reports its final line. The sibling scripts keep `slice(0, 800)`;
changing them is a deliberate sweep, not a drive-by here.

Also corrected the cleanup() comment — the wait lives in done(), and it is for
`close`, not `exit`.

Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
Claude-Session: https://claude.ai/code/session_01MvKF2mqi4hoQSUqQpZXNTq

* docs: correct the cleanup() and DRAIN_MS comments; tail-slice cleanly (review #1814 r3)

The cleanup() comment claimed its warning "should not fire now that removal
waits for close" — true only of the normal path. Both give-up branches call
finish() without close, and those are precisely the states where something may
still hold the dir, so the warning is expected there. Same defect class as the
two warnings already fixed in this PR (a message asserting what the code no
longer guarantees), relocated into a comment.

The DRAIN_MS comment explained why the cap exists but not what it costs: on
that path the work dir is removed while a descendant holding HOME=work may
still be writing to it — #1801's race, re-entered deliberately. That is only
acceptable because cleanup() warns instead of throwing, so record the two as
load-bearing for each other where the constant is defined.

Tail-slicing an Ink stream can begin mid-CSI-sequence, leading a diagnostic
with an escape-code fragment that mangles the line after it. outputTail() drops
through the first newline when the output was actually truncated, and returns
short output untouched.

Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
Claude-Session: https://claude.ai/code/session_01MvKF2mqi4hoQSUqQpZXNTq

---------

Co-authored-by: Claude Opus 5 (1M context) <[email protected]>

… wording (#1811) (#1812)

Closes #1811

Four README links pointed at files that don't exist on `v2/main`, and the launcher README described catalog seeding as one behavior when it is two.

- `docs/mcp-server-configuration.md` — written fresh for v2 (the v1.5 file predates the `--catalog`/`--config` split)
- `docs/launcher-config-consolidation-plan.md` — restored from `v1.5/main`, path drift corrected, v2 notes appended
- `docs/images/tui-screenshot.png` — regenerated against the current TUI
- launcher README seeding wording — split into the two real behaviors (web seeds two sample servers; cli/tui seed empty)

Review across three rounds also corrected several claims verified against source: the `--` separator is inverted between web and cli, a cli target placed after a flag is silently discarded, `--server` is a no-op on web and an unknown-option error on tui, the `MCP_CATALOG_PATH` ad-hoc guard is cli-only, and the conflict matrix diverges on both `--header` and `--transport stdio`.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

https://claude.ai/code/session_01Txmv2qqv3yeKgRzoqXytzD

Phase 4 of the v2 go-live runbook (#1804 §5). `main` becomes the v2 tree while
keeping its own history: this is a true merge recording `v2/main` as a second
parent, not a reset, so no force-push is needed and the default branch's
history and PR trail survive intact.

Mechanically: `-s ours` to take no tree from the merge, then the index is
replaced wholesale with `v2/main`'s. `git diff HEAD origin/v2/main` is empty
afterwards.

Note the runbook's command needed one addition — `main` and `v2/main` have
genuinely unrelated histories (different root commits, no merge base), so
`--allow-unrelated-histories` is required or the merge refuses outright.

All 125 open PRs previously targeting `main` were re-pointed at `v1/main`
first, so none of them are stranded against a tree that no longer exists.

Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
Claude-Session: https://claude.ai/code/session_01Txmv2qqv3yeKgRzoqXytzD