feat(network): add Network screen surfacing backend fetch log (#1355)

[cliffhall]

Summary

Closes #1355.

Adds a new Network tab to the v2 web client that routes the existing FetchRequestLogState to a screen. Each entry shows method, URL, status, duration, and category (auth vs transport), with an expandable detail view for request/response headers and body (or a placeholder when the body is unreadable).

New surfaces

• Screen: clients/web/src/components/screens/NetworkScreen/
• Groups: NetworkControls (search + category filter), NetworkStreamPanel (Clear / Export / list), NetworkEntry (per-entry display + expand)
• Theme: added ThemeText with a monoBreak variant so long header values wrap inside the table cell without inline styles
• Wiring: App.tsx now consumes useFetchRequestLog and dispatches onClearNetwork → FetchRequestLogState.clearFetchRequests() and onExportNetwork → JSON download via the existing downloadJsonFile helper
• Tab gating: the Network tab only appears when the active server uses SSE / streamable-HTTP. Stdio servers (no HTTP traffic) keep their previous tab list.

Search filter

The sidebar search matches against method, URL, status code, status text, request/response headers, request body, response body, and error message — case-insensitive.

Response body capture

The fetch tracker used to silently drop the body for every streamable-HTTP POST because the response content-type is text/event-stream. In practice those streams are bounded — the server sends the JSON-RPC reply (possibly preceded by progress events) and closes — so we can clone the response and read it. Only GET + SSE / ndjson stays uncaptured (genuinely long-lived push channel).

Reading the body inline would block the transport from delivering progress notifications in real time (it broke the resetTimeoutOnProgress integration test). The body is therefore read in the background and emitted via a new fetchRequestBodyUpdate event that FetchRequestLogState patches into the matching entry by id. The plumbing flows through core/mcp/node/transport, RemoteSession, RemoteClientTransport, and the remote-event type union so it works for both direct (in-browser) and remoted (dev backend) transports.

When the body genuinely isn't captured (long-lived GET stream) the entry header gets an orange SSE badge to the left of the status badge, and the Response Body section renders Long-lived stream — body not captured. Non-streaming responses with no body render (empty). Inline preview limit bumped to 100 KB so typical tools/list responses render rather than tripping the "too large" notice.

Test plan

• npm run validate (format, lint, build, unit + integration coverage) — 1786 tests pass
• npm run test:storybook — 320 stories pass
• resetTimeoutOnProgress integration test (which would have regressed if body reading blocked the transport) passes
• New per-file coverage above the gate: NetworkScreen / NetworkControls / NetworkStreamPanel at 100%; NetworkEntry at 97% lines / 100% functions
• Manual verification with Playwright against a NightRiderHttp streamable-HTTP server: tools/list, prompts/list, resources/list responses now render in the Network tab; long-lived GET /mcp keeps its "not captured" placeholder + new SSE badge; Network tab is absent for stdio connections

[cliffhall]

@claude review

[claude]

Claude encountered an error after 0s —— View job

---

I'll analyze this and get back to you.

[claude]

Claude encountered an error after 0s —— View job

---

I'll analyze this and get back to you.

[claude]

Claude finished @cliffhall's task in 3m 31s —— View job

---

Code Review

Solid PR overall — the new Network screen is well-structured, tests are thorough, and the async body-capture plumbing through the remote-transport layer is cleanly threaded. A few things worth considering before merge.

High-signal findings

1. OAuth response bodies are now captured and rendered

core/mcp/inspectorClient.ts:310-318 wires updateResponseBody into effectiveAuthFetch, which means OAuth token-exchange responses (containing access_token / refresh_token) are now stored in FetchRequestLogState and shown in the Network tab body preview.

Pre-PR these were silently dropped (the body was undefined). Post-PR a developer screen-sharing the Inspector can leak live tokens by clicking Expand on an auth entry. dispatchFetchRequest logs the (still-undefined) body via pino at first dispatch, so disk logs are safe — but the UI is not.

Two options worth considering:

• Redact known sensitive keys (access_token, refresh_token, client_secret) when rendering auth-category body previews
• Add a "redact secrets" toggle in NetworkControls

Probably acceptable for a local inspector tool, but worth a deliberate decision rather than a side-effect of the body-capture refactor.

2. NetworkEntry's prop-driven useEffect clobbers local expand state — clients/web/src/components/groups/NetworkEntry/NetworkEntry.tsx:144-146

useEffect(() => {
  setIsExpanded(isListExpanded);
}, [isListExpanded]);

This is the classic "derive state from prop via effect" anti-pattern. When the user manually expands a single entry, then the parent flips the list-level toggle to the same value the entry already had, the user's intent is silently discarded on the next list-level flip. It's likely the intended behavior (the list-level toggle is supposed to be authoritative), but the implementation also resets state in the no-op case where isListExpanded is re-passed unchanged. Consider either:

• Resetting via key={isListExpanded} on the entry (clean unmount semantics)
• Documenting the intent in a comment so a future refactor doesn't read it as a bug

3. Duplicated isLongLivedStream definition — NetworkEntry.tsx:86-96 vs fetchTracking.ts:118-121

Two copies of the same rule that must stay in lockstep. The comment in NetworkEntry.tsx calls this out, but a shared helper in core/mcp/ (e.g. isLongLivedStreamResponse(method, headers)) would prevent drift. Cheap to extract.

Lower-priority observations

4. NetworkStreamPanel search joins fields with " " and runs .includes(term) — NetworkStreamPanel.tsx:60-73. Cross-field substring matches are technically possible (a search for "foo bar" where one field ends foo and the next begins bar). Unlikely in practice, but a per-field .some(f => f.includes(term)) would be cleaner.

5. Long-lived stream rule is GET-only. The PR description correctly explains why bounded POST SSE is safe to clone, but if a buggy/malicious server returns SSE on POST without closing, cloned.text() will hang indefinitely. Since the read runs in the background and doesn't block the transport, this is a memory leak (stream reader holds a reference) rather than a hang — but worth noting. Could be mitigated with a setTimeout abort on the background read for belt-and-braces.

6. MAX_INLINE_BODY_BYTES = 100_000 — naming suggests bytes but it's body.length (JS string .length, which is UTF-16 code units). For multibyte content the byte count is higher than the limit suggests. Minor cosmetic; consider MAX_INLINE_BODY_CHARS.

7. Fragile test selector — NetworkStreamPanel.test.tsx:124-130 finds the ListToggle via text-content negation across all buttons. A data-testid or an accessible label would survive future button additions to the panel header.

8. ScrollArea.Autosize mah="calc(100vh - var(--app-shell-header-height, 0px) - 150px)" — NetworkStreamPanel.tsx:119. The 150px magic number couples the panel to surrounding chrome. Consider extracting to a CSS custom property or computing from a known token.

Plumbing review

The fetch-body-update plumbing is consistent end-to-end:

• fetchTracking.ts fires updateResponseBody(id, body) after the background cloned.text() resolves
• CreateTransportOptions.onFetchResponseBody propagates from inspectorClient.ts:574 through both node/transport.ts and createRemoteTransport.ts:60
• Remote path serializes via RemoteSession.onFetchResponseBody → SSE fetch_request_body_update event → RemoteClientTransport.consumeEventStream → option callback
• FetchRequestLogState patches the existing entry by id with findIndex and re-emits fetchRequestsChange

InspectorClient retains live closures referencing itself after disconnect(), but the dispatch becomes a no-op since FetchRequestLogState was destroyed and unsubscribed. Confirmed safe.

Tests / coverage

• Unit tests cover the new components comprehensively (filters, categories, expand/collapse, SSE badge, empty body, long-lived placeholder, size limit)
• fetchTracking.test.ts covers async emit, error path, long-lived skip, never-ending stream non-blocking, clone failures
• fetchRequestLogState.test.ts covers append, trim, body-update patch, unknown id ignore
• InspectorView.test.tsx covers Network-tab gating for stdio vs streamable-http

Looks good — tests cover the new event end-to-end and the new placeholder branches.

Summary

No blockers, but the OAuth-body-visibility consideration (finding 1) is worth a deliberate decision rather than letting it ride. Findings 2 + 3 are cleanup items that would make future refactors safer.

---

• Read AGENTS.md / CLAUDE.md for repo conventions
• Review new Network screen components
• Review fetch tracking changes (body capture + async update)
• Review transport/remote plumbing
• Review state store changes
• Review theme + view wiring
• Check test coverage
• Post final review
  · Branch: v2/issue-1355-network-screen

[cliffhall]

Thanks for the review! Responses inline, addressed in 80a2bc1.

#1 OAuth response bodies are captured — Fixed. Removed updateResponseBody from the effectiveAuthFetch tracker so auth-category entries keep their pre-PR behavior (headers + status only, body undefined). Comment in inspectorClient.ts explicitly notes that adding it back requires explicit secret redaction first. Less feature surface, but avoids the screen-share leak vector without committing to a redaction model now.

#2 NetworkEntry useEffect anti-pattern — Documented. Added a comment explaining the parent's list-level toggle is authoritative and per-entry overrides are intentionally discarded on each list toggle. Mirrors HistoryEntry (which has the same pattern). Didn't switch to key={isListExpanded} because that would also discard the per-entry override on every re-render with the same parent prop, which is more invasive than the no-op setState the current effect produces.

#3 Duplicated isLongLivedStream — Fixed. Extracted isLongLivedStreamResponse(method, contentType) to core/mcp/fetchTracking.ts and consume from both the tracker and NetworkEntry. No more drift risk.

#4 Cross-field substring match in search — Fixed. Switched from joined-string .includes to per-field .some(f => f.includes(term)). Regression test added (searching sthttps no longer falsely matches across the POST/URL boundary).

#5 POST SSE memory leak from buggy server — Noted, deferred. Local-only inspector + browser GC will eventually reclaim if the response goes out of scope; if real servers start mis-behaving we can add an AbortController with a timeout on the background read. Not pulling that complexity in now.

#6 MAX_INLINE_BODY_BYTES naming — Fixed. Renamed to MAX_INLINE_BODY_CHARS with a comment noting the unit is JS string .length (UTF-16 code units). User-facing notice now reads "Body too large to preview (N characters)".

#7 Fragile ListToggle selector — Fixed. Added aria-label={compact ? 'Expand all' : 'Collapse all'} to the ListToggle button (both variants). Both an a11y improvement and a stable selector — the panel test now uses getByRole('button', { name: 'Expand all' }) instead of fishing it out by text-content negation.

#8 Magic 150px in ScrollArea — Skipped. Matches the existing pattern in HistoryListPanel and LogStreamPanel; tweaking one without the others would be inconsistent and the value is a layout-chrome budget that doesn't really belong as a public token. Happy to extract across all three panels in a separate PR if you'd prefer.

[cliffhall]

@claude review

[claude]

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

---

Follow-up review on 80a2bc11

Verified the responses to my earlier review. All addressed fixes look correct.

• Progress notifications #1 OAuth body capture — updateResponseBody removed from buildEffectiveAuthFetch in core/mcp/inspectorClient.ts:310-323. Headers + status still tracked. The inline comment about requiring explicit secret redaction before re-enabling is exactly the right note to leave for a future contributor.
• Request logging #2 useEffect documented — NetworkEntry.tsx:144-152 now spells out that the list-level toggle is authoritative and the per-entry override is intentionally discarded on each list toggle. Choosing the comment over key={isListExpanded} is the right call — keying would also unmount on every parent re-render and cost a Collapse animation.
• Completion requests #3 isLongLivedStreamResponse extracted — core/mcp/fetchTracking.ts:12-22 is the new home; consumed by the tracker (fetchTracking.ts:138-141) and the UI (NetworkEntry.tsx:91-96). No more lockstep risk.
• SSE transport support #4 Per-field search — NetworkStreamPanel.tsx:60-75 switched to fields.some(f => f.toLowerCase().includes(term)). New regression test at NetworkStreamPanel.test.tsx:87-94 covers the sthttps cross-boundary case. Good.
• add readme #6 Rename to MAX_INLINE_BODY_CHARS — done in NetworkEntry.tsx:54-58, with a comment justifying the unit. User-facing notice now reads "Body too large to preview (N characters)".
• Tool use #7 aria-label on ListToggle — ListToggle.tsx:16,24,32 adds Expand all / Collapse all. Panel test now uses getByRole('button', { name: 'Expand all' }). Cleaner and an a11y improvement at the same time.

Findings #5 (POST SSE memory leak) and #8 (150px magic number) were correctly deferred with reasoning — fine as-is.

Tiny follow-ups (non-blocking)

1. Stale test comment in HistoryListPanel.test.tsx:232 — "has no accessible name" is no longer true since ListToggle now sets aria-label. The test still works (it finds the empty-text-content button), but it could be simplified to getByRole('button', { name: 'Expand all' }) and the comment removed. Same opportunity in ServerSettingsModal.test.tsx:249 ("first button in the header row"). Drive-by cleanup the next time those files are touched. Fix this →

2. ListToggle.test.tsx doesn't assert the new aria-label — the component tests at ListToggle.test.tsx:7-35 use screen.getByRole("button") without name. Adding name: "Expand all" / name: "Collapse all" to those queries would lock in the new contract so a future refactor can't silently drop the label.

Summary

Review fixes are accurate and complete; the deferrals are well-reasoned. Nothing further blocking from my side. The two non-blockers above are cleanup opportunities, not requirements.
· Branch: v2/issue-1355-network-screen

[cliffhall]

Both follow-ups applied in 9185fdf.

Stale "no accessible name" test selectors — Fixed in HistoryListPanel.test.tsx and ServerSettingsModal.test.tsx. Both now use getByRole('button', { name: 'Expand all' }) / 'Collapse all' instead of fishing the toggle out by empty textContent. Stale comments removed.

ListToggle tests don't assert the aria-label — Fixed. ListToggle.test.tsx now queries by name in every assertion and has an added test for the not-compact branch. The aria-label contract is now locked in — a future refactor that drops the label will fail the unit tests.

[cliffhall]

@claude review

[claude]

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

---

Follow-up review on 9185fdf9

Both follow-ups landed as advertised. Aria-label contract is now locked in by ListToggle.test.tsx, and HistoryListPanel / ServerSettingsModal selectors are clean.

• Stale "no accessible name" selectors — HistoryListPanel.test.tsx:222-237 uses getByRole("button", { name: "Collapse all" }); ServerSettingsModal.test.tsx:246-258 uses Expand all / Collapse all. Stale comments removed.
• ListToggle aria-label assertions — ListToggle.test.tsx:7-46 now queries every button by name, with a new compact={false} case covering the not-compact branch. The contract is enforced — a future refactor that drops the label will fail unit tests.

One small a11y nit (non-blocking)

NetworkControls.tsx:62-67 — the category toggles are UnstyledButtons whose active/inactive state is communicated by background color only (bg={active ? "var(--mantine-primary-color-light)" : undefined}). They behave like toggle buttons but don't expose the toggled state to assistive tech. Adding aria-pressed={active} would make the state programmatically discoverable without changing visuals. Mirrors the kind of fix this PR already made for ListToggle. Fix this →

Sanity-check of the broader diff

Re-read the new plumbing now that it's settled — nothing else worth flagging.

• isLongLivedStreamResponse extraction (core/mcp/fetchTracking.ts:11-22) — consumed by the tracker (fetchTracking.ts:138-141) and the UI (NetworkEntry.tsx:91-96) without duplication.
• buildEffectiveAuthFetch (core/mcp/inspectorClient.ts:310-323) — updateResponseBody deliberately omitted, with the rationale and "redact before re-enabling" guidance inline.
• Remote transport plumbing (createRemoteTransport.ts:60, node/transport.ts:84/104/131, remote-session.ts:100-105, server.ts:480-481, remoteClientTransport.ts:249-257, remote/types.ts:43+57+78) — onFetchResponseBody threads through every layer.
• State patch (fetchRequestLogState.ts:79-100) — findIndex-by-id is safe against clear-between-events (returns -1, silently dropped, no exception); getFetchRequests() returns a fresh array (fetchRequestLogState.ts:168-170) so subscribers see a new reference and React re-renders.
• fetchTracking.test.ts:106-220 — covers the long-lived skip rule, async emit on bounded POST SSE, async emit on JSON, never-ending stream not blocking, and clone-failure no-emit.

Approved from my side — only the aria-pressed nit above remains, and it's a follow-up, not a blocker.
· Branch: v2/issue-1355-network-screen