Skip to content

Commit 8390f61

Browse files
committed
fix(settings): keep optional prefetch failures nonfatal
1 parent e1af030 commit 8390f61

4 files changed

Lines changed: 36 additions & 61 deletions

File tree

.agents/skills/react-query-best-practices/SKILL.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,7 @@ Read these before analyzing:
3838
- Compose caller-controlled `enabled` options with required-param guards (`Boolean(id) && (options?.enabled ?? true)`). Never spread options after an internal guard, because `{ enabled: true }` can silently re-enable an invalid request.
3939
- A disabled query can still report `isPending: true`. Aggregate loading state only for queries that are applicable/enabled, or an optional query can hold the whole surface in a permanent loading state.
4040
- Deferred authorization or policy queries must fail closed. Do not give pending/error data the same fallback as a successfully loaded unrestricted policy; disable guarded actions until the policy query succeeds.
41+
- Server prefetches must call the authorized use case, apply the route presenter/response schema, and reuse the client's exact key, mapper, and stale time. Keep all fallible auth/read/parse work inside `queryFn` so an optional warm cannot fail the page, and never bypass a route that redacts fields.
4142

4243
### Mutations
4344
- Use `onSettled` (not `onSuccess`) for cache reconciliation — it fires on both success and error

apps/sim/app/workspace/[workspaceId]/settings/[section]/page.tsx

Lines changed: 1 addition & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -207,13 +207,7 @@ export default async function WorkspaceSettingsSectionPage({
207207
}
208208
}
209209

210-
/**
211-
* Scoped to the sections that actually read the key. The prefetch has to be awaited — an
212-
* unsettled query is dropped from the dehydrated payload, so firing and forgetting would
213-
* waterfall anyway — which means running it unconditionally charged the other ~25 sections
214-
* a blocking round-trip for a cache entry they never touch. The viewer's profile is seeded
215-
* by the workspace layout under a different key and is not repeated here.
216-
*/
210+
/** Awaiting is required because unsettled queries are omitted from dehydration. */
217211
await sectionPrefetch
218212

219213
return (

apps/sim/app/workspace/[workspaceId]/settings/[section]/prefetch.test.ts

Lines changed: 25 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -64,7 +64,17 @@ describe('credential-groups prefetch', () => {
6464
})
6565

6666
it('hydrates the key the panel subscribes to, through the authorized use case', async () => {
67-
mockExecute.mockResolvedValue({ credentialGroups: [{ id: 'g1' }] })
67+
const credentialGroup = {
68+
id: 'g1',
69+
workspaceId: 'w1',
70+
name: 'Engineering',
71+
description: null,
72+
options: [],
73+
status: 'active',
74+
createdAt: '2026-01-01T00:00:00.000Z',
75+
updatedAt: '2026-01-01T00:00:00.000Z',
76+
}
77+
mockExecute.mockResolvedValue({ credentialGroups: [{ ...credentialGroup, internal: true }] })
6878
const queryClient = new QueryClient()
6979

7080
await SECTION_PREFETCHERS['credential-groups']?.(queryClient, {
@@ -76,12 +86,10 @@ describe('credential-groups prefetch', () => {
7686
principal: { kind: 'session', userId: 'u1', sessionId: 's1' },
7787
input: { workspaceId: 'w1' },
7888
})
79-
expect(queryClient.getQueryData(credentialGroupKeys.list('w1'))).toEqual([{ id: 'g1' }])
89+
expect(queryClient.getQueryData(credentialGroupKeys.list('w1'))).toEqual([credentialGroup])
8090
})
8191

8292
it('leaves the cache empty when the use case denies the viewer', async () => {
83-
// prefetchQuery swallows the rejection, so a denied viewer simply hydrates nothing and the
84-
// client fetch renders the real error rather than a poisoned cache entry.
8593
mockExecute.mockRejectedValue(Object.assign(new Error('forbidden'), { code: 'forbidden' }))
8694
const queryClient = new QueryClient()
8795

@@ -92,4 +100,17 @@ describe('credential-groups prefetch', () => {
92100

93101
expect(queryClient.getQueryData(credentialGroupKeys.list('w1'))).toBeUndefined()
94102
})
103+
104+
it('leaves the cache empty when session authentication fails', async () => {
105+
mockAuthenticate.mockRejectedValue(new Error('unauthenticated'))
106+
const queryClient = new QueryClient()
107+
108+
await SECTION_PREFETCHERS['credential-groups']?.(queryClient, {
109+
workspaceId: 'w1',
110+
userId: 'u1',
111+
})
112+
113+
expect(mockExecute).not.toHaveBeenCalled()
114+
expect(queryClient.getQueryData(credentialGroupKeys.list('w1'))).toBeUndefined()
115+
})
95116
})

apps/sim/app/workspace/[workspaceId]/settings/[section]/prefetch.ts

Lines changed: 9 additions & 50 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import type { QueryClient } from '@tanstack/react-query'
2+
import { listCredentialGroupsContract } from '@/lib/api/contracts/credential-groups'
23
import { internalSessionAuth } from '@/lib/api/server/routes/internal-json-route'
34
import { listCredentialGroupSettings } from '@/lib/credential-groups/application/manage-groups'
45
import { getUserSettings } from '@/lib/users/queries'
@@ -13,20 +14,7 @@ import {
1314
credentialGroupKeys,
1415
} from '@/hooks/queries/utils/credential-group-queries'
1516

16-
/**
17-
* Prefetch general settings server-side via the shared data layer.
18-
*
19-
* Uses the same query key and mapper as the client `useGeneralSettings` hook, so the
20-
* hydrated entry is indistinguishable from one a client fetch produced.
21-
*
22-
* The authenticated caller supplies the viewer ID it already resolved. Re-reading the session
23-
* inside the query would add another dependency to a prefetch that is deliberately started as
24-
* soon as workspace access succeeds.
25-
*
26-
* Callers must await the returned promise before dehydration. Only a settled query is included
27-
* by the current dehydration policy, so dropping the promise would leave the panel to fetch on
28-
* the client as if it had never been prefetched.
29-
*/
17+
/** Prefetches the same key and mapped value as `useGeneralSettings`. */
3018
export function prefetchGeneralSettings(queryClient: QueryClient, userId: string) {
3119
return queryClient.prefetchQuery({
3220
queryKey: generalSettingsKeys.settings(),
@@ -38,33 +26,20 @@ export function prefetchGeneralSettings(queryClient: QueryClient, userId: string
3826
})
3927
}
4028

41-
/**
42-
* Prefetch the workspace's credential groups through the same authorized use case the route
43-
* runs, so the panel paints hydrated instead of blanking until its own fetch returns.
44-
*
45-
* The use case is the authorization boundary, not the route: `listCredentialGroupSettings` is a
46-
* `defineAuthorizedWorkspaceUseCase`, so it resolves canonical context, authorizes the principal
47-
* and asserts the entitlement before reading. Prefetching through it therefore applies exactly
48-
* the checks a client request would. Reaching past it to `listCredentialGroups` would not — most
49-
* settings reads authorize in their route handler, and calling their data layer directly from a
50-
* server component would skip that gate entirely.
51-
*
52-
* A denied or failed prefetch is not fatal: `prefetchQuery` swallows the rejection, nothing is
53-
* dehydrated for the key, and the client fetches normally and renders the real error.
54-
*/
29+
/** Prefetches credential groups through the route's authorization and response boundaries. */
5530
async function prefetchCredentialGroups(
5631
queryClient: QueryClient,
5732
{ workspaceId }: SettingsSectionPrefetchContext
5833
) {
59-
const principal = await internalSessionAuth.authenticate()
6034
return queryClient.prefetchQuery({
6135
queryKey: credentialGroupKeys.list(workspaceId),
6236
queryFn: async () => {
63-
const { credentialGroups } = await listCredentialGroupSettings.execute({
37+
const principal = await internalSessionAuth.authenticate()
38+
const result = await listCredentialGroupSettings.execute({
6439
principal,
6540
input: { workspaceId },
6641
})
67-
return credentialGroups
42+
return listCredentialGroupsContract.response.schema.parse(result).credentialGroups
6843
},
6944
staleTime: CREDENTIAL_GROUP_LIST_STALE_TIME,
7045
})
@@ -76,25 +51,9 @@ export interface SettingsSectionPrefetchContext {
7651
}
7752

7853
/**
79-
* The data a section needs on its first paint, keyed by section.
80-
*
81-
* A settings section otherwise pays three serial hops — the route payload, its lazily-loaded
82-
* chunk, and only then its own queries. Seeding the query cache here collapses the third into
83-
* the first, so the body renders populated the moment its chunk lands rather than blanking
84-
* again while it fetches.
85-
*
86-
* Deliberately sparse, and it should stay that way. Every entry is awaited before dehydration
87-
* (an unsettled query is dropped from the payload), so a prefetch sits in front of the section
88-
* it serves — one that is slow, or that most viewers never open, makes the page slower. Two
89-
* conditions gate entry: the read must go through something that authorizes the viewer itself,
90-
* and the hydrated value must match what the client hook stores under that key, mapper included.
91-
*
92-
* Sections absent here are absent on purpose. Most settings reads authorize in their route
93-
* handler rather than their data layer, so prefetching them means first lifting the check out of
94-
* the route — and a few must never be prefetched naively at all, because the route is also what
95-
* redacts their response (SSO strips the OIDC client secret; MCP withholds credential headers
96-
* from read-only members). Their `general`/`billing`/`admin` entries below cover a switch that
97-
* would otherwise paint its default and visibly flip.
54+
* First-paint prefetches keyed by section. Keep this sparse: each entry blocks dehydration,
55+
* must preserve authorization and route projection, and must match the client hook's cache shape.
56+
* Never bypass a route that redacts sensitive fields.
9857
*/
9958
export const SECTION_PREFETCHERS: Partial<
10059
Record<

0 commit comments

Comments
 (0)