fix(app): clear per-call panels and log level on disconnect (#1368)

[cliffhall]

Closes #1368.

Problem

The Tools / Prompts / Resources result panels and the Logs screen's level survived a disconnect/reconnect cycle, so after disconnecting from server A and connecting to server B, server B's screens still showed server A's last result (e.g. get_acts → { "acts": [] }).

These pieces of state live in App.tsx, one level above the per-server state managers (managedToolsState etc., which are destroyed in setupClientForServer), so they were only ever cleared by their explicit onClear* handlers.

Change

Extend the existing disconnect listener in App.tsx (the one that already resets activeServerId and the Connection Info modal flag) to also reset:

• toolCallState → undefined
• getPromptState → undefined
• readResourceState → undefined
• currentLogLevel → "info" (the default the next session starts at)

Subscribing to the InspectorClient's own disconnect event covers all three disconnect paths (explicit toggle, header Disconnect button, mid-session transport failure). latencyMs already resets via the connectionStatus effect and is left as-is.

Audited App.tsx for other session-scoped UI state — there is no errorMessage state (the issue's note was speculative); errors surface via transient toasts, not retained state.

Tests

Adds App.test.tsx covering the disconnect → tool-call-panel-cleared and log-level-reset transition. App is a wiring component, so the test mocks the InspectorClient (a fake EventTarget we fire disconnect on), the per-server state managers, the core hooks, and InspectorView (a thin double surfacing the props under test).

Acceptance criteria

• After disconnect A → connect B, the Tools / Prompts / Resources panels show their empty state
• currentLogLevel resets to "info" on disconnect
• Clearing fires for all three disconnect paths (covered by subscribing to the client disconnect event)
• App.test.tsx covers the disconnect → tool-call panel cleared transition

npm run validate, npm run test:storybook all pass (1883 unit/integration + 322 storybook tests).

🤖 Generated with Claude Code

[cliffhall]

@claude review

[claude]

Claude finished @cliffhall's task in 1m 36s —— View job

---

Claude PR Review

• Load review tools
• Read changed files (App.tsx, App.test.tsx)
• Verify diff against origin/v2/main
• Check disconnect path coverage and behavior
• Evaluate test quality and coverage
• Post review feedback

---

Verdict: LGTM with minor suggestions

This is a tight, well-scoped fix. The reasoning in the PR description matches what's actually in the code, the new comment block on the useEffect is genuinely useful, and the bug-coverage claims hold up under audit. Nothing rises to the level of a blocker.

Correctness

• All three disconnect paths are covered. Confirmed in core/mcp/inspectorClient.ts: the client dispatches "disconnect" both on transport closure (line 372, the failure/exit path) and at the end of explicit disconnect() (line 995). Subscribing to the client's event — instead of watching connectionStatus — is the right choice, and the existing comment already explains why (avoids the first-render-clobbers-new-id trap).
• Reset values are correct. setCurrentLogLevel("info") matches the initial-state default at App.tsx:251, so disconnect truly returns to the "fresh session" baseline. The three panel states are reset to undefined, which matches their initial values at lines 257–265.
• latencyMs correctly excluded. The connectionStatus effect at App.tsx:319–326 already clears it on the connected→non-connected edge, and that one needs the connecting-edge ref dance — leaving it where it is is correct.
• Listener lifecycle is sound. The effect re-subscribes whenever inspectorClient changes and the cleanup removes the listener, so there's no stale-handler hazard when the client is replaced or App unmounts.

Test review (App.test.tsx)

• The choice to treat App as a wiring component and mock the InspectorClient as a real EventTarget is the right call — dispatchEvent(new Event("disconnect")) exercises the actual subscription wiring, not a stub of it. This also means the test would catch a future regression where someone swaps to connectionStatus-watching.
• The InspectorView double cleanly surfaces the state under test through data-testids and exposes the handlers as plain buttons. Easy to read.
• Minor gap: the test only directly verifies toolCallState and currentLogLevel. getPromptState and readResourceState are reset by the same three lines but aren't asserted. They share a code path, so the risk of regression-without-detection is low, but extending the InspectorView double with two more data-testid spans and asserting all four pieces of state is a small addition that would make the test fully match the PR's "reset on disconnect" contract. Up to you whether it's worth the churn.
• Tiny nit: the test name "clears the tool-call result panel and resets the log level" undersells the change. If you extend coverage as above, retitle to something like "clears per-call panels and resets the log level on client disconnect."

Style / docs

• The new comment block at App.tsx:353–361 is exemplary — explains why App owns this state, why this is where it gets reset, why latencyMs is excluded, and references the issue. No changes needed.
• Inline comments on each setter call ("Clear the per-call result panels…", "Back to the default level…") are arguably redundant given the block comment above the effect, but they're short and unobtrusive — fine either way.

Things I checked and was happy with

• No new dependencies, no public-API changes.
• Doesn't disturb the existing setActiveServerId(undefined) / setConnectionInfoModalOpen(false) semantics — both still fire first, which is the order any future reader would expect.
• The connection flag the issue speculated about (errorMessage) doesn't exist in App.tsx — the PR description correctly notes this rather than silently leaving an unaddressed item.
• No format/lint concerns in the diff.

Optional follow-up (not for this PR)

If App.tsx accumulates more per-session UI state in the future, the pattern in this effect (handler that resets a list of pieces of state on "disconnect") is going to grow. A small resetSessionScopedUiState() callback colocated with the setters could keep the effect tidy. Not worth doing today — three lines is below the abstraction threshold per the project guidelines.

---

• Branch: fix/1368-clear-session-state-on-disconnect

[cliffhall]

Thanks for the review! Addressed both suggestions in faf5fac:

• Coverage gap (assert all four panels): Extended the InspectorView double with prompt-status / resource-status spans and get-prompt / read-resource buttons. The test now fills all three per-call panels (tool/prompt/resource) plus the log level, then asserts every one resets on disconnect — so the test fully matches the "reset on disconnect" contract rather than relying on the shared code path for the two unasserted ones.
• Test name: Renamed to clears the per-call panels and resets the log level on client disconnect.

On the optional follow-up (a resetSessionScopedUiState() helper): agreed, and agreed it's not for this PR — at three setters it's below the abstraction threshold. If App.tsx grows more per-session state later, that's the natural point to extract it.

npm run validate (1883 tests) and npm run test:storybook (322) both green.

[cliffhall]

@claude review

[claude]

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

---

Claude PR Review (follow-up pass)

• Pull latest branch state and diff against origin/v2/main
• Re-read App.tsx change
• Re-read App.test.tsx (new file)
• Verify the two review-response items from the prior round
• Post updated review

---

Verdict: LGTM — ready to merge

The two suggestions from my prior review were addressed cleanly, and re-auditing the change top-to-bottom turned up nothing new. Nothing blocking, nothing nit-worthy.

What changed since the last review

• Coverage extended to all four pieces of session-scoped state. InspectorView double now surfaces prompt-status / resource-status spans and get-prompt / read-resource buttons (App.test.tsx:191-203). The test fills all three per-call panels plus the log level and asserts every one resets — so the test now matches the PR's "reset on disconnect" contract directly rather than leaning on the shared-code-path argument for the unasserted ones.
• Test renamed to clears the per-call panels and resets the log level on client disconnect (App.test.tsx:221). Accurately describes the contract under test.

Re-audit notes

• Disconnect dispatch is still covered on all three paths. Confirmed dispatchTypedEvent("disconnect") at core/mcp/inspectorClient.ts:372 (transport-closure path → covers mid-session failure / process exit) and :995 (end of explicit disconnect() → covers toggle + header button). Both are guarded by status !== "disconnected" so there's no double-fire on reconnect.
• Reset values still match the initial defaults: setCurrentLogLevel("info") matches App.tsx:251; the three undefined resets match App.tsx:257-265. latencyMs is still correctly handled by the connectionStatus edge effect at App.tsx:317-327.
• Listener lifecycle. Effect re-subscribes when inspectorClient changes and the returned cleanup removes the listener (App.tsx:362-379) — no stale-handler hazard across client swaps or unmount.
• Comment block at App.tsx:353-361 is exemplary. Captures why App owns this state, why it's reset here, and why latencyMs is excluded.

Test quality

• Mocking InspectorClient as a real EventTarget keeps the subscription wiring under test instead of stubbed (App.test.tsx:22-46). A future regression that switched to watching connectionStatus instead of the client event would be caught.
• The test flow — connect → exercise all three handlers → bump log level → dispatch disconnect → assert all four pieces reset — is a clean, end-to-end statement of the contract.
• The mock surface is broad (9 state managers + remote storage + ~10 hooks + environmentFactory + InspectorView) but each mock is minimal and motivated. There's no way to test App's wiring without this much scaffolding; it's the right trade-off.

Optional follow-up (still not for this PR)

Same note as last time: if App.tsx accrues more per-session UI state, a colocated resetSessionScopedUiState() callback becomes worth extracting. Four setters is still below the threshold.

---

• Branch: fix/1368-clear-session-state-on-disconnect

[cliffhall]

Thanks — glad it's ready to merge.

Opened #1394 to track the optional resetSessionScopedUiState() extraction so it isn't lost. As we agreed, it's deliberately not actionable now (four setters is below the abstraction threshold); the issue's trigger is "a 5th+ piece of session-scoped state is added to App.tsx," at which point the extraction becomes worthwhile.