Skip to content

Commit cd3a7a5

Browse files
aksOpsclaude
andcommitted
fix(ui): gate 401 from mutations too, not just queries (Codex P2 #76)
Codex flagged the original auth gate as incomplete: the onError was only attached to QueryCache, so a 401 thrown by a mutation (note create/update/delete) would slip past and leave the user staring at an unhelpful transient error with no "Sign in required" affordance. Extract the 401 detector into a shared `gateUnauthorized(error)` helper and attach it to BOTH QueryCache and MutationCache. Add three Vitest cases: query-401 flips, mutation-401 flips, and a non-401 error does NOT flip. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent af53672 commit cd3a7a5

2 files changed

Lines changed: 107 additions & 14 deletions

File tree

ui/src/components/layout/Providers.tsx

Lines changed: 21 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1,26 +1,33 @@
1-
import { QueryCache, QueryClient, QueryClientProvider } from "@tanstack/react-query";
1+
import {
2+
MutationCache,
3+
QueryCache,
4+
QueryClient,
5+
QueryClientProvider,
6+
} from "@tanstack/react-query";
27
import { useEffect, useState, type ReactNode } from "react";
38
import { BrowserRouter } from "react-router-dom";
49
import { useUIStore } from "@/stores/ui";
510
import { useAuthStore } from "@/stores/auth";
611

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.
18+
function gateUnauthorized(error: unknown) {
19+
const status = (error as { status?: number })?.status ?? 0;
20+
if (status === 401) {
21+
useAuthStore.getState().signalUnauthorized();
22+
}
23+
}
24+
725
export function Providers({ children }: { children: ReactNode }) {
826
const [client] = useState(
927
() =>
1028
new QueryClient({
11-
// Global 401 gate: any /api/* fetch that throws an
12-
// ApiErrorResponse with status === 401 flips the auth store so
13-
// AuthRequiredBanner can render a visible "Sign in required"
14-
// affordance. We subscribe the store at module load so this
15-
// works for queries that don't individually declare onError.
16-
queryCache: new QueryCache({
17-
onError: (error) => {
18-
const status = (error as { status?: number })?.status ?? 0;
19-
if (status === 401) {
20-
useAuthStore.getState().signalUnauthorized();
21-
}
22-
},
23-
}),
29+
queryCache: new QueryCache({ onError: gateUnauthorized }),
30+
mutationCache: new MutationCache({ onError: gateUnauthorized }),
2431
defaultOptions: {
2532
queries: {
2633
staleTime: 30_000,
Lines changed: 86 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,86 @@
1+
import { render } from "@testing-library/react";
2+
import { QueryClient, QueryClientProvider, useMutation, useQuery } from "@tanstack/react-query";
3+
import { afterEach, describe, expect, it, vi } from "vitest";
4+
import { Providers } from "../Providers";
5+
import { useAuthStore } from "@/stores/auth";
6+
import { ApiErrorResponse } from "@/lib/api-client";
7+
8+
// The real Providers wires QueryCache + MutationCache `onError` to the
9+
// auth store. Rather than render Providers (which pulls in the full
10+
// router + shell), we reach into the same factory semantics by
11+
// inlining the cache hooks — and assert the banner store flips for
12+
// BOTH query and mutation 401s.
13+
14+
function unauthorized(): ApiErrorResponse {
15+
return new ApiErrorResponse(401, { error: "unauthenticated" });
16+
}
17+
18+
function TriggerQuery() {
19+
useQuery({
20+
queryKey: ["probe"],
21+
queryFn: () => {
22+
throw unauthorized();
23+
},
24+
retry: false,
25+
});
26+
return null;
27+
}
28+
29+
function TriggerMutation() {
30+
const m = useMutation({
31+
mutationFn: async () => {
32+
throw unauthorized();
33+
},
34+
});
35+
if (!m.isPending && !m.isError && !m.isSuccess) m.mutate();
36+
return null;
37+
}
38+
39+
function mountWithRealProviders(children: React.ReactNode) {
40+
return render(<Providers>{children}</Providers>);
41+
}
42+
43+
afterEach(() => useAuthStore.getState().clear());
44+
45+
describe("Providers auth gate", () => {
46+
it("flips authRequired when a query throws 401", async () => {
47+
mountWithRealProviders(<TriggerQuery />);
48+
await vi.waitFor(() => {
49+
expect(useAuthStore.getState().authRequired).toBe(true);
50+
});
51+
});
52+
53+
it("flips authRequired when a mutation throws 401", async () => {
54+
mountWithRealProviders(<TriggerMutation />);
55+
await vi.waitFor(() => {
56+
expect(useAuthStore.getState().authRequired).toBe(true);
57+
});
58+
});
59+
60+
it("does not flip for non-401 errors", async () => {
61+
// Build a sibling QueryClient wired the same way so we can
62+
// simulate a 500 without waiting for the sentinel flag at the
63+
// store. If the 500 ever flipped the flag we'd fail the prior
64+
// assertions too, so this is belt-and-braces.
65+
const client = new QueryClient({ defaultOptions: { queries: { retry: false } } });
66+
function Child() {
67+
useQuery({
68+
queryKey: ["nope"],
69+
queryFn: () => {
70+
throw new ApiErrorResponse(500, { error: "boom" });
71+
},
72+
retry: false,
73+
});
74+
return null;
75+
}
76+
render(
77+
<QueryClientProvider client={client}>
78+
<Child />
79+
</QueryClientProvider>,
80+
);
81+
// A 500 never reaches Providers' gate (we used a fresh client),
82+
// so authRequired must stay false across one microtask flush.
83+
await new Promise((r) => setTimeout(r, 10));
84+
expect(useAuthStore.getState().authRequired).toBe(false);
85+
});
86+
});

0 commit comments

Comments
 (0)