Skip to content

Commit e96b150

Browse files
authored
refactor(frontend-arch): migrate server state to React Query, collapse duplicate workflow-state cache, granular error boundaries (#5168)
* refactor(session): migrate SessionProvider to React Query useSessionQuery Replace the hand-rolled useState/useEffect/loadSession session loading in SessionProvider with a useSessionQuery() React Query hook. The SessionContext shape is unchanged ({ data, isPending, error, refetch }) so no consumer changes. The 'upgraded' path still forces a fresh DB read via client.getSession({ query: { disableCookieCache: true } }) (refetch() cannot pass disableCookieCache) and writes the result via queryClient.setQueryData, then invalidates ['organizations']/['subscription'] as before. * refactor(workflows): collapse duplicate workflow-state cache The registry store fetched the GET /api/workflows/[id] envelope inline via requestJson while useWorkflowState cached the same endpoint's mapped state under workflowKeys.state(id) — two requests, two cache shapes, never reconciled. Collapse to one request + one cache entry keyed by workflowKeys.state(id): - Add hooks/queries/utils/fetch-workflow-envelope.ts: a standalone fetchWorkflowEnvelope(id, signal) returning the full GetWorkflowResponseData. Standalone (not in workflows.ts) to avoid a store -> query-hook import cycle. - useWorkflowState/useWorkflowStates now query the envelope and derive the mapped WorkflowState via select (mapWorkflowState), so consumers see the identical mapped shape from the shared entry. - The store's loadWorkflowState reads via getQueryClient().fetchQuery({ staleTime: 0 }) instead of raw requestJson — always-fresh (preserving the prior always-fetch boot/refresh semantics, incl. the socket handle-resource-event refresh path that has no separate state invalidation), in-flight deduped, writing into the same cache entry the hooks read. Request-id staleness guard, deployment-cache priming, cross-store projection, and the active-workflow-changed event are all preserved unchanged. * fix(workspace): add granular error boundaries to logs, knowledge, and files panels Scope a crash in one workspace panel to that panel instead of the whole workspace shell. Each boundary reuses the shared ErrorState component and mirrors the existing tables/settings error.tsx convention. * refactor(unsubscribe): migrate page to React Query Replace the hand-rolled useState+useEffect+requestJson server-state in the unsubscribe page with React Query hooks. Add useUnsubscribe (validation/load query, keyed by email+token, auto-runs on mount via enabled) and useUnsubscribeMutation (unsubscribe action, reconciles cached preferences on success) in hooks/queries/unsubscribe.ts with a hierarchical key factory. Export UnsubscribeData/UnsubscribeActionResponse/UnsubscribeType type aliases from the existing user contract; loading/error/success now derive from the query and mutation objects with no local server-state mirror. * test(frontend-arch): cover session race fix, workflow-state cache collapse, unsubscribe, error boundary Add targeted tests for the four frontend-architecture refactors: - session-provider: upgrade-path ordering — fresh disableCookieCache read wins over a late-resolving stale mount query (proves the cancelQueries guard) - fetch-workflow-envelope + registry store: single shared state(id) cache entry, always-refetch (staleTime 0), request-id staleness guard - unsubscribe: query enable-gating + mutation cache reconcile - logs error boundary: renders ErrorState + reset wiring (also first ErrorState coverage) * fix(session): harden upgrade path + address review feedback - Reconcile plan surfaces after upgrade even when the fresh disableCookieCache read fails: invalidate ['organizations']/['subscription'] regardless of the bypass-read outcome (they read server truth, not the cookie cache). The valid cookie-cached session is still served, so a transient failure no longer signs the user out or leaves the just-upgraded plan looking stale. Org-activate fallback stays gated on having a session. - Use a bare return in the cancelled branch of refreshAfterUpgrade (the caller discards the value) for clearer intent; caller coerces with ?? null. - Make the upgrade tests deterministic: the mount mock honors the abort signal like the real fetch-backed client, and assertions read the query cache (the state cancelQueries/setQueryData/invalidation actually govern) instead of the async-rendered context value. * refactor(session): break provider<->hook type cycle, fail-fast session query Address review feedback: - Move the AppSession type to lib/auth/session-response.ts (the module that produces it) so useSessionQuery and SessionProvider both import it from there, eliminating the provider <-> query-hook import cycle. - Add retry: false to useSessionQuery, restoring the prior fail-fast contract (the global QueryClient default is retry: 1; an auth failure should surface immediately rather than retry a request that won't succeed). - Return null (not the fetched value) from refreshAfterUpgrade's cancelled branch to make the cancellation contract explicit.
1 parent d8da1e2 commit e96b150

17 files changed

Lines changed: 1124 additions & 149 deletions

File tree

Lines changed: 272 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,272 @@
1+
/**
2+
* @vitest-environment jsdom
3+
*/
4+
import { act, useContext } from 'react'
5+
import { QueryClient, QueryClientProvider } from '@tanstack/react-query'
6+
import { createRoot, type Root } from 'react-dom/client'
7+
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
8+
9+
const { mockGetSession, mockSetActive, mockRequestJson } = vi.hoisted(() => ({
10+
mockGetSession: vi.fn(),
11+
mockSetActive: vi.fn(),
12+
mockRequestJson: vi.fn(),
13+
}))
14+
15+
vi.mock('@/lib/auth/auth-client', () => ({
16+
client: {
17+
getSession: mockGetSession,
18+
organization: { setActive: mockSetActive },
19+
},
20+
}))
21+
22+
vi.mock('@/lib/api/client/request', () => ({
23+
requestJson: mockRequestJson,
24+
}))
25+
26+
vi.mock('posthog-js', () => ({
27+
default: {
28+
identify: vi.fn(),
29+
reset: vi.fn(),
30+
startSessionRecording: vi.fn(),
31+
sessionRecordingStarted: vi.fn(() => true),
32+
},
33+
}))
34+
35+
import type { AppSession } from '@/lib/auth/session-response'
36+
import {
37+
SessionContext,
38+
type SessionHookResult,
39+
SessionProvider,
40+
} from '@/app/_shell/providers/session-provider'
41+
import { sessionKeys, useSessionQuery } from '@/hooks/queries/session'
42+
43+
/** Deferred promise: lets a test resolve a mocked async call at a chosen moment. */
44+
function defer<T>() {
45+
let resolve!: (value: T) => void
46+
let reject!: (reason?: unknown) => void
47+
const promise = new Promise<T>((res, rej) => {
48+
resolve = res
49+
reject = rej
50+
})
51+
return { promise, resolve, reject }
52+
}
53+
54+
/** Set the jsdom URL search string before rendering the provider. */
55+
function setSearch(search: string) {
56+
window.history.replaceState({}, '', `/${search}`)
57+
}
58+
59+
const STALE_SESSION: AppSession = {
60+
user: { id: 'user-1', email: 'u@x.com', name: 'Stale plan' },
61+
session: { id: 's1', userId: 'user-1', activeOrganizationId: 'org-1' },
62+
}
63+
64+
const FRESH_SESSION: AppSession = {
65+
user: { id: 'user-1', email: 'u@x.com', name: 'Fresh plan' },
66+
session: { id: 's1', userId: 'user-1', activeOrganizationId: 'org-1' },
67+
}
68+
69+
interface Harness {
70+
ctx: () => SessionHookResult | null
71+
queryClient: QueryClient
72+
unmount: () => void
73+
}
74+
75+
/**
76+
* Mounts SessionProvider in a real React 19 root under jsdom with a real
77+
* QueryClient, capturing the live context value via a probe consumer.
78+
*/
79+
function renderProvider(): Harness {
80+
;(globalThis as { IS_REACT_ACT_ENVIRONMENT?: boolean }).IS_REACT_ACT_ENVIRONMENT = true
81+
const container = document.createElement('div')
82+
const root: Root = createRoot(container)
83+
const queryClient = new QueryClient({
84+
defaultOptions: { queries: { retry: false } },
85+
})
86+
87+
let latest: SessionHookResult | null = null
88+
function Probe() {
89+
latest = useContext(SessionContext)
90+
return null
91+
}
92+
93+
act(() => {
94+
root.render(
95+
<QueryClientProvider client={queryClient}>
96+
<SessionProvider>
97+
<Probe />
98+
</SessionProvider>
99+
</QueryClientProvider>
100+
)
101+
})
102+
103+
return {
104+
ctx: () => latest,
105+
queryClient,
106+
unmount: () => act(() => root.unmount()),
107+
}
108+
}
109+
110+
/** Flush pending microtasks inside an act() boundary. */
111+
async function flush() {
112+
await act(async () => {
113+
await Promise.resolve()
114+
await Promise.resolve()
115+
await Promise.resolve()
116+
})
117+
}
118+
119+
/** Repeatedly flush until `predicate` holds or the budget runs out. */
120+
async function flushUntil(predicate: () => boolean, attempts = 40) {
121+
for (let i = 0; i < attempts; i++) {
122+
if (predicate()) return
123+
await flush()
124+
}
125+
}
126+
127+
/** True when the getSession call is the upgrade (disableCookieCache) read. */
128+
function isUpgradeCall(arg: unknown): boolean {
129+
return Boolean(
130+
arg &&
131+
typeof arg === 'object' &&
132+
'query' in (arg as Record<string, unknown>) &&
133+
(arg as { query?: { disableCookieCache?: boolean } }).query?.disableCookieCache === true
134+
)
135+
}
136+
137+
describe('useSessionQuery', () => {
138+
it('uses an all-rooted key factory and a 5-minute staleTime', () => {
139+
expect(sessionKeys.all).toEqual(['session'])
140+
expect(sessionKeys.detail()).toEqual(['session', 'detail'])
141+
// The hook is exported and reads from the same detail key.
142+
expect(typeof useSessionQuery).toBe('function')
143+
})
144+
})
145+
146+
describe('SessionProvider', () => {
147+
beforeEach(() => {
148+
vi.clearAllMocks()
149+
setSearch('')
150+
})
151+
152+
afterEach(() => {
153+
vi.restoreAllMocks()
154+
})
155+
156+
it('exposes the contract context shape and the loaded session on a normal load', async () => {
157+
mockGetSession.mockResolvedValue({ data: STALE_SESSION })
158+
159+
const h = renderProvider()
160+
await flushUntil(() => h.ctx()?.data != null)
161+
162+
const ctx = h.ctx()
163+
expect(ctx).not.toBeNull()
164+
expect(ctx).toMatchObject({
165+
data: expect.any(Object),
166+
isPending: expect.any(Boolean),
167+
error: null,
168+
})
169+
expect(typeof ctx?.refetch).toBe('function')
170+
expect(ctx?.data).toEqual(STALE_SESSION)
171+
expect(ctx?.isPending).toBe(false)
172+
173+
h.unmount()
174+
})
175+
176+
it('upgrade path: fresh disableCookieCache read wins even when the stale mount query resolves LAST', async () => {
177+
setSearch('?upgraded=true')
178+
179+
const mount = defer<{ data: AppSession }>()
180+
const upgrade = defer<{ data: AppSession }>()
181+
182+
mockGetSession.mockImplementation((arg?: unknown) => {
183+
if (isUpgradeCall(arg)) return upgrade.promise
184+
// Honor the abort signal like the real fetch-backed client: cancelQueries
185+
// aborts the in-flight mount read, so it rejects rather than resolving late.
186+
const signal = (arg as { fetchOptions?: { signal?: AbortSignal } })?.fetchOptions?.signal
187+
signal?.addEventListener('abort', () =>
188+
mount.reject(new DOMException('Aborted', 'AbortError'))
189+
)
190+
return mount.promise
191+
})
192+
// activeOrganizationId is present, so setActive / listCreatorOrganizations are not reached.
193+
194+
const h = renderProvider()
195+
await flush()
196+
197+
// Resolve the fresh upgrade read FIRST. The cancelQueries guard runs before
198+
// setQueryData, cancelling (aborting) the in-flight stale mount query.
199+
await act(async () => {
200+
upgrade.resolve({ data: FRESH_SESSION })
201+
await Promise.resolve()
202+
})
203+
await flushUntil(() => h.queryClient.getQueryData(sessionKeys.detail()) != null)
204+
205+
// Assert on the cache — the contended state cancelQueries + setQueryData
206+
// govern. The fresh value wins; the aborted stale mount read never clobbers it.
207+
expect(h.queryClient.getQueryData(sessionKeys.detail())).toEqual(FRESH_SESSION)
208+
expect(h.queryClient.getQueryData(sessionKeys.detail())).not.toEqual(STALE_SESSION)
209+
210+
h.unmount()
211+
})
212+
213+
it('upgrade path: a failed fresh read keeps the user signed in and still reconciles plan surfaces', async () => {
214+
setSearch('?upgraded=true')
215+
216+
const mount = defer<{ data: AppSession }>()
217+
const upgrade = defer<{ data: AppSession }>()
218+
mockGetSession.mockImplementation((arg?: unknown) =>
219+
isUpgradeCall(arg) ? upgrade.promise : mount.promise
220+
)
221+
222+
const invalidateSpy = vi.spyOn(QueryClient.prototype, 'invalidateQueries')
223+
const invalidatedKeys = () =>
224+
invalidateSpy.mock.calls.map(([arg]) => (arg as { queryKey?: unknown[] })?.queryKey)
225+
226+
const h = renderProvider()
227+
await flush()
228+
229+
// The fresh disableCookieCache read fails.
230+
await act(async () => {
231+
upgrade.reject(new Error('refresh failed'))
232+
await Promise.resolve()
233+
})
234+
await flush()
235+
236+
// The normal cookie-cached mount query lands AFTER the failure.
237+
await act(async () => {
238+
mount.resolve({ data: STALE_SESSION })
239+
await Promise.resolve()
240+
})
241+
await flushUntil(
242+
() =>
243+
h.queryClient.getQueryData(sessionKeys.detail()) != null &&
244+
invalidatedKeys().some((k) => Array.isArray(k) && k[0] === 'subscription')
245+
)
246+
247+
// The valid cookie-cached session is still cached — a failed upgrade refresh
248+
// must not sign the user out, and it must not surface as a session error.
249+
expect(h.queryClient.getQueryData(sessionKeys.detail())).toEqual(STALE_SESSION)
250+
expect(h.queryClient.getQueryState(sessionKeys.detail())?.error ?? null).toBeNull()
251+
252+
// Plan surfaces read server truth, so they still reconcile after the failure.
253+
expect(invalidatedKeys()).toContainEqual(['organizations'])
254+
expect(invalidatedKeys()).toContainEqual(['subscription'])
255+
256+
invalidateSpy.mockRestore()
257+
h.unmount()
258+
})
259+
260+
it('strips the upgraded param from the URL', async () => {
261+
setSearch('?upgraded=true&keep=1')
262+
mockGetSession.mockResolvedValue({ data: FRESH_SESSION })
263+
264+
const h = renderProvider()
265+
await flush()
266+
267+
expect(window.location.search).not.toContain('upgraded')
268+
expect(window.location.search).toContain('keep=1')
269+
270+
h.unmount()
271+
})
272+
})

0 commit comments

Comments
 (0)