Skip to content

Commit e3bb09f

Browse files
fix(ui): route MCP console 401s through shared auth banner (RAN-36) (#79)
Replace raw fetch() in useMCP with mcpRequest() so a 401 on /mcp flips the same auth store /api/* uses. This makes the AuthRequiredBanner render on the MCP route when the session expires, instead of leaving the 401 stuck in local toolsError / per-call error state. - api-client: extract rawRequest helper that defaults Content-Type, attaches credentials, and signals useAuthStore on 401. apiFetch reuses it; new mcpRequest export returns the raw Response so MCP can read Mcp-Session-Id and parse SSE bodies the JSON shape can't expose. - useMCP: rpc() routes through mcpRequest; drops the now-redundant Content-Type / credentials it inherits from the helper. - Providers: keep the QueryCache/MutationCache 401 gate as a defensive layer; comment updated to reflect that the HTTP boundary is now the primary signal (signalUnauthorized is idempotent). - Tests: api-client unit tests cover apiFetch + mcpRequest 401 → store flip; new MCPConsole.test renders the route with /mcp returning 401 and asserts AuthRequiredBanner appears. Co-authored-by: Paperclip <noreply@paperclip.ing>
1 parent 45c44e4 commit e3bb09f

5 files changed

Lines changed: 151 additions & 15 deletions

File tree

ui/src/components/layout/Providers.tsx

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -9,12 +9,13 @@ import { BrowserRouter } from "react-router-dom";
99
import { useUIStore } from "@/stores/ui";
1010
import { useAuthStore } from "@/stores/auth";
1111

12-
// Global 401 gate: any /api/* fetch that throws an ApiErrorResponse
13-
// with status === 401 flips the auth store so AuthRequiredBanner can
14-
// render a visible "Sign in required" affordance. Wired to BOTH
15-
// QueryCache and MutationCache — a 401 on a write action (note
16-
// create/update/delete) must surface the banner just the same as a
17-
// read-path failure.
12+
// Defensive 401 gate. The HTTP boundary (apiFetch / mcpRequest in
13+
// lib/api-client.ts) is the primary signal for AuthRequiredBanner — it
14+
// already flips the auth store on every 401 it sees. This React Query
15+
// hook is kept so any consumer that synthesises an ApiErrorResponse
16+
// outside the fetch path (e.g. tests, future non-HTTP transports) still
17+
// surfaces the banner. signalUnauthorized is idempotent, so the
18+
// double-signal on real /api/* 401s is harmless.
1819
function gateUnauthorized(error: unknown) {
1920
const status = (error as { status?: number })?.status ?? 0;
2021
if (status === 401) {

ui/src/hooks/api/useMCP.ts

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import { useState, useCallback, useRef, useEffect } from "react";
2+
import { mcpRequest } from "@/lib/api-client";
23

34
export interface MCPCallRecord {
45
id: string;
@@ -25,14 +26,15 @@ async function rpc(
2526
body: unknown,
2627
): Promise<{ json: unknown; sessionId: string | null }> {
2728
const headers: Record<string, string> = {
28-
"Content-Type": "application/json",
2929
"Accept": "application/json, text/event-stream",
3030
};
3131
if (sessionId) headers["Mcp-Session-Id"] = sessionId;
3232

33-
const res = await fetch("/mcp", {
33+
// Routed through mcpRequest so 401s flip the shared auth store and
34+
// surface the AuthRequiredBanner on /mcp instead of being swallowed
35+
// into local toolsError / per-call error state.
36+
const res = await mcpRequest("/mcp", {
3437
method: "POST",
35-
credentials: "include",
3638
headers,
3739
body: JSON.stringify(body),
3840
});

ui/src/lib/__tests__/api-client.test.ts

Lines changed: 68 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,15 @@
1-
import { describe, it, expect, vi } from "vitest";
1+
import { afterEach, describe, it, expect, vi } from "vitest";
22
import { http, HttpResponse } from "msw";
33
import { server } from "@/test/msw";
4-
import { apiFetch, ApiErrorResponse, initAuth } from "../api-client";
4+
import { apiFetch, ApiErrorResponse, initAuth, mcpRequest } from "../api-client";
5+
import { useAuthStore } from "@/stores/auth";
6+
7+
afterEach(() => {
8+
// Auth store is module-level; reset after every test or a 401 in one
9+
// case leaks `authRequired = true` into the next assertion. No render
10+
// happens here, so we mutate the store directly without React.act.
11+
useAuthStore.getState().clear();
12+
});
513

614
describe("apiFetch", () => {
715
it("returns parsed json on 200", async () => {
@@ -114,6 +122,17 @@ describe("apiFetch", () => {
114122
}
115123
});
116124

125+
it("flips the shared auth store on a 401 (so AuthRequiredBanner renders)", async () => {
126+
server.use(
127+
http.get("/api/expired", () =>
128+
HttpResponse.json({ error: "no session" }, { status: 401 }),
129+
),
130+
);
131+
expect(useAuthStore.getState().authRequired).toBe(false);
132+
await expect(apiFetch("/api/expired")).rejects.toBeInstanceOf(ApiErrorResponse);
133+
expect(useAuthStore.getState().authRequired).toBe(true);
134+
});
135+
117136
it("does not set Authorization header on data-path fetch even when a key exists in a meta tag", async () => {
118137
const meta = document.createElement("meta");
119138
meta.setAttribute("name", "docsiq-api-key");
@@ -139,3 +158,50 @@ describe("apiFetch", () => {
139158
}
140159
});
141160
});
161+
162+
describe("mcpRequest", () => {
163+
it("returns the raw Response so MCP can read Mcp-Session-Id and SSE bodies", async () => {
164+
server.use(
165+
http.post("/mcp", () =>
166+
HttpResponse.json(
167+
{ jsonrpc: "2.0", id: 1, result: { ok: true } },
168+
{ headers: { "Mcp-Session-Id": "sess-42" } },
169+
),
170+
),
171+
);
172+
const res = await mcpRequest("/mcp", {
173+
method: "POST",
174+
body: JSON.stringify({ jsonrpc: "2.0", id: 1, method: "ping" }),
175+
});
176+
expect(res.status).toBe(200);
177+
expect(res.headers.get("Mcp-Session-Id")).toBe("sess-42");
178+
expect(useAuthStore.getState().authRequired).toBe(false);
179+
});
180+
181+
it("flips the shared auth store on a 401 so the /mcp surface honours AuthRequiredBanner", async () => {
182+
server.use(
183+
http.post("/mcp", () =>
184+
HttpResponse.json({ error: "session expired" }, { status: 401 }),
185+
),
186+
);
187+
expect(useAuthStore.getState().authRequired).toBe(false);
188+
const res = await mcpRequest("/mcp", { method: "POST", body: "{}" });
189+
expect(res.status).toBe(401);
190+
expect(useAuthStore.getState().authRequired).toBe(true);
191+
});
192+
193+
it("sends credentials: 'include' and defaults Content-Type for string bodies", async () => {
194+
const spy = vi.spyOn(globalThis, "fetch").mockResolvedValue(
195+
new Response("{}", { status: 200, headers: { "content-type": "application/json" } }),
196+
);
197+
try {
198+
await mcpRequest("/mcp", { method: "POST", body: JSON.stringify({}) });
199+
const init = (spy.mock.calls[0][1] ?? {}) as RequestInit;
200+
expect(init.credentials).toBe("include");
201+
const hdrs = new Headers(init.headers);
202+
expect(hdrs.get("Content-Type")).toBe("application/json");
203+
} finally {
204+
spy.mockRestore();
205+
}
206+
});
207+
});

ui/src/lib/api-client.ts

Lines changed: 28 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import type { ApiError } from "@/types/api";
2+
import { useAuthStore } from "@/stores/auth";
23

34
// Before cookies are set the first time, the UI may have been shipped a
45
// one-shot bearer via the meta tag (legacy). We exchange it for a cookie
@@ -59,16 +60,29 @@ function isBrowserManagedBody(body: BodyInit): boolean {
5960
return false;
6061
}
6162

62-
export async function apiFetch<T>(
63-
path: string,
64-
init: RequestInit = {},
65-
): Promise<T> {
63+
// Shared low-level request helper. Centralises the auth-required signal
64+
// so a 401 from any HTTP surface (apiFetch on /api/*, mcpRequest on
65+
// /mcp/*) flips the same auth store the AuthRequiredBanner reads from.
66+
// Returns the raw Response so callers that need headers or non-JSON
67+
// framing (e.g. MCP's SSE event-stream + Mcp-Session-Id) can handle it.
68+
async function rawRequest(path: string, init: RequestInit = {}): Promise<Response> {
6669
if (sessionReady) await sessionReady;
6770
const headers = new Headers(init.headers);
6871
if (init.body && !headers.has("Content-Type") && !isBrowserManagedBody(init.body)) {
6972
headers.set("Content-Type", "application/json");
7073
}
7174
const res = await fetch(path, { ...init, headers, credentials: "include" });
75+
if (res.status === 401) {
76+
useAuthStore.getState().signalUnauthorized();
77+
}
78+
return res;
79+
}
80+
81+
export async function apiFetch<T>(
82+
path: string,
83+
init: RequestInit = {},
84+
): Promise<T> {
85+
const res = await rawRequest(path, init);
7286
if (!res.ok) {
7387
let body: ApiError = { error: `HTTP ${res.status}` };
7488
try {
@@ -81,3 +95,13 @@ export async function apiFetch<T>(
8195
if (res.status === 204) return undefined as T;
8296
return res.json() as Promise<T>;
8397
}
98+
99+
// MCP can't use apiFetch() directly because it needs Mcp-Session-Id from
100+
// response headers and may receive an SSE text/event-stream body that the
101+
// generic JSON shape can't expose. mcpRequest mirrors apiFetch's
102+
// session-bootstrap, Content-Type defaulting, credentials, and shared
103+
// 401 → auth-store gate, then hands the caller the raw Response for
104+
// MCP-specific framing.
105+
export async function mcpRequest(path: string, init: RequestInit = {}): Promise<Response> {
106+
return rawRequest(path, init);
107+
}
Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,43 @@
1+
import { afterEach, describe, expect, it } from "vitest";
2+
import { act } from "react";
3+
import { render, screen, waitFor } from "@testing-library/react";
4+
import { MemoryRouter } from "react-router-dom";
5+
import { http, HttpResponse } from "msw";
6+
import { server } from "@/test/msw";
7+
import MCPConsole from "@/routes/MCPConsole";
8+
import { AuthRequiredBanner } from "@/components/layout/AuthRequiredBanner";
9+
import { useAuthStore } from "@/stores/auth";
10+
11+
afterEach(() => {
12+
// Auth store is module-level — without a reset between tests the 401
13+
// banner from a prior assertion leaks into the next render.
14+
act(() => useAuthStore.getState().clear());
15+
});
16+
17+
describe("MCPConsole route", () => {
18+
it("renders the shared AuthRequiredBanner when /mcp returns 401", async () => {
19+
server.use(
20+
http.post("/mcp", () =>
21+
HttpResponse.json(
22+
{ error: "session expired" },
23+
{ status: 401, headers: { "Content-Type": "application/json" } },
24+
),
25+
),
26+
);
27+
28+
render(
29+
<MemoryRouter initialEntries={["/mcp"]}>
30+
<AuthRequiredBanner />
31+
<MCPConsole />
32+
</MemoryRouter>,
33+
);
34+
35+
// The hook fires `initialize` on mount; the 401 must flip the auth
36+
// store via mcpRequest so the shared banner renders on this route.
37+
const banner = await screen.findByTestId("auth-required-banner");
38+
expect(banner).toBeInTheDocument();
39+
expect(screen.getByText(/sign in required/i)).toBeInTheDocument();
40+
41+
await waitFor(() => expect(useAuthStore.getState().authRequired).toBe(true));
42+
});
43+
});

0 commit comments

Comments
 (0)