chore(frontend): upgrade MCP SDK to 1.29.0 + adopt onerror, streaming, OAuth provider

[malinskibeniamin]

What

We upgraded the MCP TypeScript SDK (1.26 → 1.29) and adopted three new capabilities across the Console MCP servers feature and the AI agents path: transport-level error observability, task-based streaming tool calls, and a first-class OAuthClientProvider wrapping the Console JWT. The first consumer is the MCP server inspector tab, which now surfaces tool-call progress live.

Why

• Richer tool call UX. Long-running tool calls now stream incremental progress with a status line rather than appearing as a single opaque spinner — the user sees the tool working.
• Centralized auth. ConsoleJWTOAuthProvider lets the SDK's internal request machinery (retries, auth recovery) drive off the same JWT source as Console's transport, instead of having to be taught each callsite separately.
• Observable transport errors. SSE drops and HTTP failures that happen outside the request/response cycle are now logged with the server URL instead of silently disappearing.

User-visible changes

• The MCP server inspector tab shows a Progress bar and a live status line while a tool call is executing.
• Cancelling a running tool call aborts the underlying request (no orphan fetches against the MCP gateway).
• Tool calls against older MCP servers that do not yet advertise task-streaming continue to work unchanged — the client transparently falls back to the classic non-streaming path.

Hardening

• Capability fallback. Post-connect, we inspect client.getServerCapabilities().tasks?.requests?.tools?.call. Absent → we call client.callTool directly instead of experimental.tasks.callToolStream, so older servers cannot produce a silently-hung mutation.
• Streaming timeout. The stream is wrapped in a 120s composed-signal timeout (configurable per-call via streamTimeoutMs). On timeout, the signal is aborted and the mutation rejects with MCP tool stream timed out after <N>ms.
• Watchdog. If the stream closes without a terminal result or error message, the mutation rejects explicitly with a descriptive error rather than resolving undefined or hanging.
• JWT expiry detection. ConsoleJWTOAuthProvider.tokens() base64url-decodes the payload and reads exp. Already-expired tokens return undefined so the SDK surfaces UnauthorizedError early; malformed or exp-less tokens fail open (advisory, not crypto).
• Transport error logging. transport.onerror is wired before client.connect(), logs to the browser console with the server URL, and deliberately does not toast — the mutation's onError owns toast to avoid double-firing.
• Progress clamping. The inspector clamps the computed percent to [0, 100] and handles NaN / total=0 by going indeterminate, so a misbehaving server cannot push a broken-looking bar.
• Tool-switch reset. Switching the selected tool mid-stream now clears in-flight progress state so stale percentages do not bleed into the new selection.

Risks & mitigations

Original risk	Shipped mitigation
Streaming against an older server could hang the mutation	Capability check + non-streaming fallback + 120s timeout + watchdog
OAuth provider has no expiry awareness	Best-effort exp decode in tokens(), fail-open on any parse failure
transport.onerror could double-fire with mutation onError	Transport callback logs only; contract pinned by a regression test
Manual Authorization header + authProvider could disagree	Both read live config.jwt per-request from the same source, so they agree by construction

Test plan

• bun run type:check
• bun run lint:check (no new violations over the 984-error baseline)
• bun run test (unit + integration)
• bun run test:browser (streaming inspector visual regression)
• Manual smoke: create an MCP server, inspect a tool, run it, observe the Progress bar + status line move
• Manual smoke: rotate the console JWT mid-session, confirm the next MCP call picks up the new token

Follow-ups

• Elicitation UI — SDK 1.29 delivers elicitation through server-initiated requests handled via registered handlers, requiring a new dialog component.
• OAuth refresh flow with backend coordination, once the expiry detection lands and we decide on refresh semantics.
• Streaming adoption in the knowledge-base playground (currently on the non-streaming path).

[malinskibeniamin]

@claude review

[claude]

Claude finished @malinskibeniamin's task in 2m 35s —— View job

---

PR Review

• Read CLAUDE.md and understand repo conventions
• Review changed source files
• Review test files
• Formulate and post review feedback

---

This is a solid, well-scoped upgrade. The architecture choices — capability fallback, composed abort signals, fail-open JWT expiry, transport error segregation — are all correct. Test coverage is thorough and the concurrency suite is particularly good. A few issues worth addressing before merge:

---

Bug: Double toast on non-abort tool errors

useStreamMCPServerToolMutation has a mutation-level onError that calls formatToastErrorMessageGRPC. The component's executeToolRequest also passes a per-call onError that calls toast.error(error.message). TanStack Query fires both callbacks, so a "server blew up" error shows two toasts.

remote-mcp.tsx:475-487 — mutation-level handler calls formatToastErrorMessageGRPC for every non-abort error.

remote-mcp-inspector-tab.tsx:378-382 — per-call handler calls toast.error(error.message) unless the message is the exact string 'Request was cancelled'.

The component-level onError should either be removed (relying entirely on the mutation-level handler) or the mutation-level handler should be removed and error formatting owned by the call site. The mixed strategy causes doubles.

Fix this →

---

Bug: throw message.error when error is undefined

In drainMCPStream (remote-mcp.tsx:422), message.error is typed as Error | undefined. When the stream yields { type: 'error' } with no error payload, throw undefined produces an unhelpful undefined rejection.

// Current:
throw message.error;

// Safer:
throw message.error ?? new Error('MCP stream yielded an error event with no payload');

Fix this →

---

Connection leak: MCP client is never closed

createMCPClientWithSession establishes an SSE/HTTP transport session and returns { client, transport }, but neither the mutation nor the component ever calls client.close() or transport.close(). Every tool invocation holds an open connection until the server times it out. The AbortController cancels the stream but not the underlying transport session.

remote-mcp.tsx:437 — createMCPClientWithSession called but result's transport is never torn down in useStreamMCPServerToolMutation's finally block.

The simplest fix is a finally { await client.close?.() } around the streaming block, or closing the transport after both the streaming and fallback paths complete.

Fix this →

---

Minor: useCallMCPServerToolMutation is now dead code

The component switched to useStreamMCPServerToolMutation but useCallMCPServerToolMutation is still exported from remote-mcp.tsx:319. If no other consumer uses it, remove it to keep the surface clean. If it's kept as a public API, it should be noted.

---

Minor: streamTimeoutMs is silently ignored on the capability-fallback path

When the server doesn't advertise tasks.requests.tools.call, the code falls back to client.callTool and passes the raw signal — but the composed timeout signal is never built for this path. The caller's streamTimeoutMs parameter has no effect on the fallback. Worth either documenting clearly or wiring the same timeout composition.

---

Notes / praise

• buildStreamAbortControl signal composition is correct — the { once: true } listener avoids double-removal and the signal?.aborted pre-check is a nice edge case handle.
• ConsoleJWTOAuthProvider is clean. The fail-open behaviour on malformed tokens is correctly documented and tested.
• Dual auth (authProvider + manual Authorization header in custom fetch) is acknowledged in the PR and both read live config.jwt, so they agree by construction. Fine as a belt-and-suspenders; worth collapsing in a follow-up once the SDK's auth layer is confirmed as the canonical path.
• Test suite is comprehensive: capability fallback, timeout, watchdog, pre-aborted signal, concurrency isolation, onerror-vs-toast contract regression — all covered.

[malinskibeniamin]

@claude review

[claude]

Claude finished @malinskibeniamin's task in 2m 11s —— View job

---

PR Review

• Read CLAUDE.md and understand repo conventions
• Review changed source files (current state after recent fixes)
• Check if prior review issues are resolved
• Identify any remaining or new issues
• Post updated review

---

All four issues raised in the first review pass are now fixed. Details below, plus one new issue exposed by reading the current code.

---

Previous issues — all resolved

Issue	Fix commit
Double toast on non-abort errors	f3fcfdf — component-level toast.error removed; mutation onError is the single toast owner
throw message.error when error is undefined	d8847fa — now throw message.error ?? new Error('MCP stream yielded an error event with no payload')
MCP client never closed in useStreamMCPServerToolMutation	45452ed — outer finally { await client.close?.() } added
streamTimeoutMs ignored on capability-fallback path	8984e2a — composed signal now applied to callTool fallback

Note: useCallMCPServerToolMutation (previously flagged as dead code) is a live consumer — knowledge-base-playground-tab.tsx uses it for the non-streaming knowledge-base path. ✅

---

Bug: listMCPServerTools leaks the MCP client connection

Every tool-list fetch opens a transport session and leaves it open. The MCP client is created but never closed:

remote-mcp.tsx:308-312

export const listMCPServerTools = async (serverUrl: string) => {
  const { client } = await createMCPClientWithSession(serverUrl, 'redpanda-console');
  return client.listTools(); // transport held open until server timeout
};

The same pattern fixed in useStreamMCPServerToolMutation and useCallMCPServerToolMutation applies here. Fix:

export const listMCPServerTools = async (serverUrl: string) => {
  const { client } = await createMCPClientWithSession(serverUrl, 'redpanda-console');
  try {
    return await client.listTools();
  } finally {
    await client.close?.();
  }
};

Fix this →

---

Minor: Empty Mcp-Session-Id header on initial request

In createMCPClientWithSession, the custom fetch wrapper always sends 'Mcp-Session-Id': client?.transport?.sessionId ?? ''. On the initial connect, the session hasn't been established yet, so this sends Mcp-Session-Id: '' (an empty string rather than omitting the header). Some servers may treat an empty session ID differently from an absent one.

remote-mcp.tsx:287-293

headers: {
  ...init?.headers,
  'Content-Type': 'application/json',
  ...(config.jwt && { Authorization: `Bearer ${config.jwt}` }),
  'Mcp-Session-Id': client?.transport?.sessionId ?? '',  // ← always present
},

More defensively:

...(client?.transport?.sessionId && { 'Mcp-Session-Id': client.transport.sessionId }),

This matches the conditional pattern already used for Authorization.

---

Notes / praise

The buildStreamAbortControl composition and drainMCPStream watchdog logic are solid and handle all the edge cases correctly (pre-aborted signal, timeout vs user-cancel disambiguation, empty-error stream). The ConsoleJWTOAuthProvider is clean with correct fail-open semantics. The test suite is thorough — capability fallback, concurrency isolation, onerror-vs-toast contract, and the new streaming inspector visual regression are all well covered.

[malinskibeniamin]

@claude review

[claude]

Claude finished @malinskibeniamin's task in 3m 20s —— View job

---

PR Review (Round 3)

• Read CLAUDE.md and understand repo conventions
• Read current state of changed files
• Check if round 2 issues are resolved
• Identify any remaining or new issues
• Post updated review

---

Both round 2 issues are fixed. Details below, plus two new minor issues.

---

Previous issues — both resolved

Issue	Fix
listMCPServerTools leaks MCP client connection	try/finally { await client.close?.() } added (remote-mcp.tsx:308-316). Two new lifecycle tests added (remote-mcp.test.tsx:982-997).
Empty Mcp-Session-Id on initial request	Changed to conditional spread ...(client?.transport?.sessionId && {...}) (remote-mcp.tsx:290). New test added (remote-mcp.test.tsx:999-1017).

---

Bug: User cancellation of a hanging stream fires an unexpected toast

When the user calls cancelToolRequest() while a stream is in progress, the following happens under the mock (and potentially the real SDK if it returns cleanly rather than throwing on abort):

1. controller.abort() → onUserAbort fires → timeoutController.abort() → composedSignal.aborted = true
2. The generator observes the signal and returns without yielding a terminal message
3. drainMCPStream returns undefined
4. The condition composedSignal.aborted && !signal?.aborted is true && false = false (user signal is also aborted)
5. Falls through to throw new Error('MCP tool stream ended without a terminal result or error message')
6. onError receives this watchdog error — its message doesn't contain 'aborted' and name !== 'AbortError', so a toast fires

The existing test at remote-mcp.test.tsx:939 verifies client close but doesn't assert on toastErrorMock, so the toast fires undetected. In production the real SDK likely throws an AbortError from callToolStream directly (bypassing this path), but the code has no defensive fallback.

remote-mcp.tsx:483-487 — add a user-abort guard before the watchdog throw:

if (composedSignal.aborted && !signal?.aborted) {
  throw new Error(`MCP tool stream timed out after ${streamTimeoutMs}ms`);
}
// NEW: distinguish user cancellation from a genuine watchdog
if (signal?.aborted) {
  throw Object.assign(new Error('Request was cancelled'), { name: 'AbortError' });
}
throw new Error('MCP tool stream ended without a terminal result or error message');

And add a corresponding assertion to the abort test:

expect(toastErrorMock).not.toHaveBeenCalled();

Fix this →

---

Minor: setStreamProgress replaces rather than merges state

Both progress channels call the same onProgress callback with different fields:

• drainMCPStream emits { taskId, status, statusMessage } from task messages
• The SDK onprogress callback emits { progress, total } for numeric progress

In the inspector tab, both paths call setStreamProgress(update) which replaces the full state. If a taskStatus message arrives after an onprogress update, the numeric progress/total fields are cleared, reverting the bar to indeterminate. The reverse also holds.

remote-mcp-inspector-tab.tsx:375:

// Current:
onProgress: (update) => setStreamProgress(update),

// Suggested:
onProgress: (update) => setStreamProgress((prev) => ({ ...prev, ...update })),

---

Minor: No client lifecycle tests for useCallMCPServerToolMutation

listMCPServerTools and useStreamMCPServerToolMutation both have tests confirming client.close() is called on success and failure (remote-mcp.test.tsx:903-997). useCallMCPServerToolMutation has the same try/finally pattern but no equivalent tests.

---

Notes / praise

• Both round 2 fixes are correct and well-tested. The Mcp-Session-Id conditional spread matches the Authorization header pattern exactly.
• The listMCPServerTools lifecycle tests (lines 982-997) cover both the success and rejection paths — good coverage parity with the streaming mutation tests.
• buildStreamAbortControl's { once: true } listener and signal?.aborted pre-check remain solid.
• ConsoleJWTOAuthProvider fail-open semantics (base64url decode with padding tolerance, isJwtExpired returning false on any parse error) are correct and well-documented.

[malinskibeniamin]

@claude review

[claude]

Claude finished @malinskibeniamin's task in 3m 18s —— View job

---

PR Review (Round 4)

• Read CLAUDE.md and understand repo conventions
• Read current state of changed files
• Check if round 3 issues are resolved
• Identify any remaining or new issues
• Post updated review

---

All three Round 3 issues are fixed. Details below, plus two new minor notes.

---

Previous issues — all resolved

Issue	Fix
User cancellation fires unexpected toast via watchdog path	remote-mcp.tsx:488-490 — if (signal?.aborted) { throw Object.assign(new Error('Request was cancelled'), { name: 'AbortError' }); } added before the watchdog throw
setStreamProgress replaces instead of merges state	remote-mcp-inspector-tab.tsx:375 — changed to setStreamProgress((prev) => ({ ...prev, ...update })). New test added (remote-mcp-inspector-tab.test.tsx:264-287) verifying statusMessage from task messages persists after numeric onprogress fires.
No lifecycle tests for useCallMCPServerToolMutation	Two tests added (remote-mcp.test.tsx:1007-1040) covering success and rejection paths with close() call assertions.

---

Minor: nextListToolsRejection and nextCallToolRejection not reset in beforeEach

beforeEach resets nextConnectRejection = undefined (line 206) but omits the two sibling variables declared on lines 84-85. The mock self-clears them on first use, so in practice there's no observed flakiness — but if a test throws before the mock fires, the stale rejection bleeds into the next test. Worth adding for consistency:

// remote-mcp.test.tsx beforeEach
nextListToolsRejection = undefined;
nextCallToolRejection = undefined;

---

Minor: No abort-path test (no-toast assertion) for useCallMCPServerToolMutation

useStreamMCPServerToolMutation's abort test (remote-mcp.test.tsx:946-971) pins expect(toastErrorMock).not.toHaveBeenCalled(). useCallMCPServerToolMutation has the same AbortError guard in onError (lines 344-346) but its lifecycle tests don't cover the abort path. Parity test would be:

test('does not fire a toast when callTool rejects with AbortError', async () => {
  nextCallToolRejection = Object.assign(new Error('aborted'), { name: 'AbortError' });
  // ...mutateAsync...
  expect(toastErrorMock).not.toHaveBeenCalled();
});

---

Notes / praise

• The abort-guard fix (signal?.aborted check before watchdog throw) is exactly right and well-tested. The mock in remote-mcp.test.tsx correctly simulates the hang-then-signal-abort path: the generator resolves when composedSignal.aborted, the for await exits with no messages, and the new guard throws an AbortError instead of the watchdog error.
• State-merge fix is clean: spreading null is a JS no-op so { ...null, ...update } on the first progress event is safe.
• useCallMCPServerToolMutation lifecycle tests match the parity pattern from the streaming mutation tests.
• Overall the code is solid, all critical bugs from prior rounds are resolved, and the hardening strategy (capability fallback + composed abort + watchdog + fail-open JWT) is correct and well-tested. Ready to merge modulo the two minor test notes above.

[malinskibeniamin]

Fix the screenshot in the PR description, it's unstyled

[malinskibeniamin]

@claude review

[claude]

Claude finished @malinskibeniamin's task in 2m 18s —— View job

---

PR Review (Round 5)

• Read CLAUDE.md and understand repo conventions
• Read current state of changed files
• Check if round 4 issues are resolved
• Identify any remaining or new issues

---

Both Round 4 minor issues are resolved. No new issues found.

Previous issues — both resolved

Issue	Fix
nextListToolsRejection / nextCallToolRejection not reset in beforeEach	Added to beforeEach at remote-mcp.test.tsx:206-207. ✅
No abort-path (no-toast) test for useCallMCPServerToolMutation	Test added at remote-mcp.test.tsx:1043-1061 — covers AbortError rejection, verifies close() called once and toastErrorMock not called. ✅

---

Final state

All six rounds of feedback have been addressed:

Round	Issues raised	Status
1	Double toast; throw undefined; connection leak in streaming mutation; dead code; streamTimeoutMs ignored on fallback	All fixed
2	listMCPServerTools connection leak; empty Mcp-Session-Id on initial request	All fixed
3	User-cancel watchdog fires unexpected toast; setStreamProgress replaces instead of merges; no lifecycle tests for useCallMCPServerToolMutation	All fixed
4	nextList/CallToolRejection not reset in beforeEach; no abort no-toast test for useCallMCPServerToolMutation	All fixed

The hardening strategy (capability fallback + composed abort signal + watchdog + fail-open JWT + transport error segregation) is correct and fully tested. The test suite is comprehensive across all paths: streaming success/failure, timeout, user abort, capability fallback, concurrency isolation, client lifecycle, onerror-vs-toast contract, and progress state merge.

This PR is ready to merge.