chore(web): collapse InspectorView's prop wall into domain bundles

[cliffhall]

Closes #2130

Phase 3 of #2126 — the finishing pass on App.tsx.

<InspectorView> took ~130 flat props, which made ~170 of App.tsx's JSX lines a
prop wall rather than a component tree. It now takes 12 domain props, and the
call site is 14 lines:

<InspectorView
  shell={shellProps}
  connection={connectionProps}
  servers={serverListProps}
  tools={toolsPanelProps}
  prompts={promptsPanelProps}
  resources={resourcesPanelProps}
  apps={appsPanelProps}
  tasks={tasksPanelProps}
  logs={logsPanelProps}
  protocol={protocolPanelProps}
  network={networkPanelProps}
  console={consolePanelProps}
/>

Two conventions that keep this reviewable

• No field was renamed. Every field inside a bundle carries the exact name it
  had as a flat prop, so this is a regrouping and the diff can be read as one.
• The bundles stop at InspectorView. It destructures each one back into the
  same locals its body already used and passes the same individual props down, so
  nothing below the view knows the bundles exist. That was the issue's stated
  recommendation, and it is why ~1,000 lines of view body are untouched.

Shapes live in a new InspectorView/types.ts, carrying the JSDoc that was on the
individual props.

Three props that have no single screen

Named explicitly because a reviewer will look for them:

• onCompleteArgument / completionsSupported are read by both the Prompts
  and Resources screens, so they sit in connection rather than being duplicated
  into two bundles.
• malformedListItems is one array that three screens each filter for their own
  entries, so it sits in shell — it genuinely has no domain owner.
• erroredServerId / connectedServerId are connection outcomes that the
  Servers screen renders, so they sit in connection, not servers.

Closures lifted out of the JSX

Every multi-line closure that was declared inline is now a named useCallback
above the return: the server add/import/clone highlight-clearing handlers,
onServerRemove's lookup, and onServerReorder's ~15-line .catch +
notifications.show. The seven void-discarding wrappers
(onToggleConnection, onCallTool, onGetPrompt, onReadResource,
onOpenApp, onCancelTask, onDisconnect) are also named, and carry the
no-floating-promises justification once between them rather than at seven call
sites.

They stay in App.tsx rather than moving into useServerCommands: the issue
suggests the owning hook, but these handlers drive App-local modal state
(configModal, importConfigOpen, removeTarget, highlightedServerIds), and
moving that state into the hook is a separate change from regrouping props. The
"Done when" item this closes is the JSX one.

Note on App.tsx's line count

It goes 1,796 → 1,926. The JSX shrank by ~155 lines; the bundle literals and the
lifted closures add more than that back, because prop values that used to be
inline expressions are now named. The goal here was the legible tree, not the
line count — Phases 0–2 already met the parent issue's size target.

Tests and stories

• InspectorView.test.tsx: makeProps now takes any number of per-bundle
  override layers (makeProps({ tools: { tools: [t] } })), which is what lets
  the connectedHttp / failedHttp scenario helpers supply a base and still
  accept a caller override. All 82 call sites converted; 82 tests pass
  unchanged.
• App.test.tsx: the InspectorView double is now typed with the real
  InspectorViewProps instead of a hand-written structural mirror, so it can no
  longer drift from the component it stands in for. 95 tests pass unchanged.
• InspectorView.stories.tsx: one constant per bundle at module scope, because
  Storybook merges args only at the top level — a story overriding one field
  spreads its bundle (servers: { ...serversArgs, servers: [] }) rather than
  replacing it.

No behavior change.

🤖 Generated with Claude Code

https://claude.ai/code/session_01Quk1hBUtyhkXowwMXzAnAY

[Copilot]

Pull request overview

Refactors InspectorView to replace roughly 130 flat props with 12 domain-oriented bundles while preserving downstream screen interfaces.

Changes:

• Adds typed domain prop bundles and unpacks them at the view boundary.
• Replaces inline App.tsx handlers with named callbacks and concise bundle wiring.
• Updates tests, mocks, fixtures, and Storybook stories for the new API.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
clients/web/src/App.tsx	Assembles domain bundles and named callbacks.
clients/web/src/App.test.tsx	Updates the view mock for bundled props.
clients/web/src/components/views/InspectorView/types.ts	Defines the domain bundle interfaces.
clients/web/src/components/views/InspectorView/InspectorView.tsx	Accepts and destructures the new bundles.
clients/web/src/components/views/InspectorView/InspectorView.test.tsx	Updates fixtures and layered bundle overrides.
clients/web/src/components/views/InspectorView/InspectorView.stories.tsx	Migrates Storybook arguments to bundles.

Suppressed comments (1)

clients/web/src/App.tsx:1565

• onDisconnect uses try/finally, so finalizeExplicitDisconnect() runs but a failed transport close still rejects (useConnectionLifecycle.ts:776-783). Bare void makes that a global unhandled rejection, contrary to the preceding claim that the callee catches failures. Add a terminating .catch(...) that reports the close failure and test the rejected-disconnect path.

  const dispatchDisconnect = useCallback(() => {
    void onDisconnect();
  }, [onDisconnect]);

---

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

[cliffhall]

All three review findings addressed in 37add96 (mirrored here since inline replies get folded away once the fix is pushed).

1 + 2 — bare void on onToggleConnection / onDisconnect (inline, and the suppressed comment on App.tsx:1565). Both valid; verified in useConnectionLifecycle.ts rather than taken on faith. The toggle has three escape paths outside its own try/catch — awaiting initialConfigSettledRef.current?.promise, the setupClientForServer(target) construction, and the already-connected disconnect branch, whose try/finally runs finalizeExplicitDisconnect() and then lets a failed transport close propagate. onDisconnect is that same try/finally on its own. Both now terminate with a .catch that toasts.

The comment above them was the actual defect: it asserted that all seven wrappers own their failures. That is true of the five command handlers (onCallTool, onGetPrompt, onReadResource, onCancelTask, onOpenApp — each ends in a catch), so void stays right for those; it was false for the two lifecycle ones. The comment now splits the groups and explains the distinction.

Worth stating plainly: the unhandled rejection is pre-existing — the old JSX did void onToggleConnection(id) inline, so this PR relocated it rather than introducing it. What the PR did introduce was a comment claiming it was safe, which is exactly the kind of false reassurance that outlives the code it describes. Fixed the behavior as well as the comment.

Tests. Two new cases in the App background command rejections (#2049) block, one per path, asserting both the toast and an empty unhandled-rejection list. I reverted the fix and re-ran to confirm both fail against the old void code — they pin the fix rather than merely passing alongside it. The InspectorView double grew a disconnect button, since the standalone onDisconnect had no route through it.

3 — the PropOverrides doc example. Valid, and self-inflicted: the example was correct as written, but the script I used to convert the 82 makeProps call sites also rewrote the example inside its own JSDoc into a triple-nested shape. Restored, and I swept the file for other comment text the script had touched — this was the only one.

npm run local:gate green end to end after the changes: ≥90 coverage gate, both build gates, all smokes under Chromium and Firefox, 122 Storybook play functions.

[Copilot]

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.

[cliffhall]

Round 2 addressed in 0139fd7 (mirrored here, since inline replies fold away once the fix is pushed). Both findings valid.

1 — void still leaks on four command handlers. Traced and confirmed: onCallTool / onGetPrompt / onReadResource / onOpenApp await handleCommandScopedAuthRecovery from inside their catch blocks, and that helper awaits tryApplyStoredAuthRecovery → client.checkAuthChallengeSatisfied() and client.pushRemoteAuthState(), both uncaught and both reaching the backend. A rejection thrown from a catch is not caught by that same catch, so it escapes the handler entirely.

Rather than patch those four and keep a comment listing which handlers are safe, all seven wrappers now terminate identically through a shared reportDispatchFailure(title). The reason is worth stating plainly: whether a handler "owns its failures" has not proven reliably knowable by reading it. My first pass asserted all seven did; my second, corrected pass asserted five did; both were wrong. So the code no longer depends on getting that judgement right. Handlers that do surface their own failures resolve and never reach the .catch, so the ordinary error paths do not double-toast.

2 — reorder had no App-level coverage. Correct, and self-inflicted: I lifted that closure out of the JSX and gave it no route through the test double. The double now exposes a reorder-servers control, with tests for both forwarding the ordered ids and reporting a rejected reorder.

On verification. All four new rejection tests were confirmed to fail against the bare-void code before being kept — I reverted the fixes and re-ran rather than assuming. That mattered: an earlier revert attempt aborted before writing the file, so the "confirmed failing" result I nearly reported would have been vacuous.

Two things the gate caught that the test run could not:

• My first cut of the reorder tests built the useServers mock by spreading vi.mocked(useServers)(), which does not type-check (the hook takes a required argument). vitest does not typecheck, so npm run test was green while tsc -b rejected it. Now uses a properly typed serversWithReorder helper.
• An intermediate gate run failed on scripts/lib/render-smoke.test.mjs — unrelated to this PR, and the known flake described by render-smoke's under-the-deadline test is timing-marginal, and aborts the whole gate when it flakes #2180 ("timing-marginal, and aborts the whole gate when it flakes"). Its fix had landed on v2/main after the last merge, so v2/main is merged in again here.

npm run local:gate green end to end on the final state: ≥90 coverage gate, both build gates, all smokes under Chromium and Firefox, 122 Storybook play functions.

[Copilot]

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated no new comments.