refactor(frontend-arch): migrate server state to React Query, collapse duplicate workflow-state cache, granular error boundaries#5168
Conversation
…uery
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.
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.
… 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.
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.
…lapse, 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)
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
PR SummaryMedium Risk Overview Session — Workflow state — One cache entry per workflow on Unsubscribe — Page uses Resilience — Reviewed by Cursor Bugbot for commit 7c92041. Configure here. |
![cursor[bot]](https://avatars-githubusercontent-com.shimmerl.top/in/1210556?s=60&v=4){kind=small}
|
@greptile review |
|
@cursor review |
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
1 issue from previous review remains unresolved.
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit 650db2d. Configure here.
Greptile SummaryThis PR refactors four areas of server-state management toward React Query, eliminates a duplicate HTTP request for workflow state, and adds panel-scoped error boundaries.
Confidence Score: 5/5Safe to merge — the refactor is behavior-preserving, the test suite covers the critical race-condition paths, and no existing consumers of the changed cache entries were left broken. All four migration surfaces (session, workflow state, unsubscribe, error boundaries) are backed by targeted tests. The cancelQueries → setQueryData ordering in the upgrade path is verified by an abort-signal test that confirms the stale mount fetch cannot clobber the fresh session. The workflow-state cache collapse is validated to produce exactly one shared entry. The only asymmetry between the store's staleTime: 0 and the hooks' staleTime: 30s is intentional and works correctly because fetchQuery updates the shared cache entry that hook observers automatically read from. No files require special attention. Important Files Changed
Sequence Diagram%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
participant Browser
participant SessionProvider
participant useSessionQuery
participant QueryClient
participant BetterAuth
Browser->>SessionProvider: "mount (URL has ?upgraded=true)"
SessionProvider->>useSessionQuery: subscribe (stale cookie-cached fetch starts)
useSessionQuery->>BetterAuth: getSession() [cookie-cached, with AbortSignal]
SessionProvider->>SessionProvider: strip ?upgraded from URL
SessionProvider->>BetterAuth: "getSession({disableCookieCache:true})"
BetterAuth-->>SessionProvider: fresh session
SessionProvider->>QueryClient: cancelQueries(sessionKeys.detail()) → abort stale signal
BetterAuth-->>useSessionQuery: AbortError (signal fired)
SessionProvider->>QueryClient: setQueryData(sessionKeys.detail(), fresh)
QueryClient-->>SessionProvider: "context data = fresh session"
SessionProvider->>QueryClient: invalidateQueries(['organizations'])
SessionProvider->>QueryClient: invalidateQueries(['subscription'])
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
sequenceDiagram
participant Browser
participant SessionProvider
participant useSessionQuery
participant QueryClient
participant BetterAuth
Browser->>SessionProvider: "mount (URL has ?upgraded=true)"
SessionProvider->>useSessionQuery: subscribe (stale cookie-cached fetch starts)
useSessionQuery->>BetterAuth: getSession() [cookie-cached, with AbortSignal]
SessionProvider->>SessionProvider: strip ?upgraded from URL
SessionProvider->>BetterAuth: "getSession({disableCookieCache:true})"
BetterAuth-->>SessionProvider: fresh session
SessionProvider->>QueryClient: cancelQueries(sessionKeys.detail()) → abort stale signal
BetterAuth-->>useSessionQuery: AbortError (signal fired)
SessionProvider->>QueryClient: setQueryData(sessionKeys.detail(), fresh)
QueryClient-->>SessionProvider: "context data = fresh session"
SessionProvider->>QueryClient: invalidateQueries(['organizations'])
SessionProvider->>QueryClient: invalidateQueries(['subscription'])
Reviews (5): Last reviewed commit: "refactor(session): break provider<->hook..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars-githubusercontent-com.shimmerl.top/in/867647?s=60&v=4){kind=small}
Greptile SummaryThis PR migrates three server-state islands to React Query — the session provider, the unsubscribe page, and the workflow-state cache — and adds granular Next.js error boundaries for the logs, files, and knowledge panels.
Confidence Score: 4/5The refactor is well-structured and the core cache-collapse and upgrade-path logic are correct; the main watch-out is in the session query, which now silently retries failed auth checks up to three times before surfacing an error. The workflow-state cache collapse, unsubscribe migration, and error boundary additions are clean and well-tested. The session path has two smaller issues: apps/sim/hooks/queries/session.ts and apps/sim/app/_shell/providers/session-provider.tsx warrant a second look — the session query lacks an explicit retry policy and the circular type dependency between the two files should be addressed. Important Files Changed
Sequence Diagram%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
participant Browser
participant SessionProvider
participant RQ as React Query Cache
participant AuthClient as Better Auth Client
Note over Browser,AuthClient: Normal mount (no ?upgraded)
SessionProvider->>RQ: useSessionQuery() subscribes
RQ->>AuthClient: fetchSession() if stale
AuthClient-->>RQ: AppSession data
RQ-->>SessionProvider: data, isPending, error, refetch
Note over Browser,AuthClient: Plan upgrade redirect (?upgraded=true)
Browser->>SessionProvider: "mount with ?upgraded=true in URL"
SessionProvider->>Browser: window.history.replaceState strip param
SessionProvider->>RQ: cancelQueries sessionKeys.detail
SessionProvider->>AuthClient: getSession disableCookieCache true
AuthClient-->>SessionProvider: fresh AppSession
SessionProvider->>RQ: setQueryData fresh session
Note right of RQ: Stale mount query cancelled so fresh data wins
Note over Browser,AuthClient: Workflow state hydration registry store
SessionProvider->>RQ: fetchQuery workflowKeys.state id staleTime 0
RQ->>AuthClient: fetchWorkflowEnvelope id
AuthClient-->>RQ: GetWorkflowResponseData stored in cache
RQ-->>SessionProvider: raw envelope
Note right of RQ: useWorkflowState subscribers get mapWorkflowState via select
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
sequenceDiagram
participant Browser
participant SessionProvider
participant RQ as React Query Cache
participant AuthClient as Better Auth Client
Note over Browser,AuthClient: Normal mount (no ?upgraded)
SessionProvider->>RQ: useSessionQuery() subscribes
RQ->>AuthClient: fetchSession() if stale
AuthClient-->>RQ: AppSession data
RQ-->>SessionProvider: data, isPending, error, refetch
Note over Browser,AuthClient: Plan upgrade redirect (?upgraded=true)
Browser->>SessionProvider: "mount with ?upgraded=true in URL"
SessionProvider->>Browser: window.history.replaceState strip param
SessionProvider->>RQ: cancelQueries sessionKeys.detail
SessionProvider->>AuthClient: getSession disableCookieCache true
AuthClient-->>SessionProvider: fresh AppSession
SessionProvider->>RQ: setQueryData fresh session
Note right of RQ: Stale mount query cancelled so fresh data wins
Note over Browser,AuthClient: Workflow state hydration registry store
SessionProvider->>RQ: fetchQuery workflowKeys.state id staleTime 0
RQ->>AuthClient: fetchWorkflowEnvelope id
AuthClient-->>RQ: GetWorkflowResponseData stored in cache
RQ-->>SessionProvider: raw envelope
Note right of RQ: useWorkflowState subscribers get mapWorkflowState via select
Reviews (2): Last reviewed commit: "test(frontend-arch): cover session race ..." | Re-trigger Greptile |
- 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.
…n 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.
|
@greptile review |
|
@cursor review |
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit 7c92041. Configure here.
|
@greptile review |
1 similar comment
|
@greptile review |
1 participant
Summary
useState/useEffect/loadSessionwith auseSessionQuery()hook. Context shape is byte-identical ({ data, isPending, error, refetch }) so all 34 consumers are untouched. The?upgradedpath still forces a fresh DB read viagetSession({ disableCookieCache: true })+setQueryData(refetch can't pass that flag), now guarded bycancelQueriesso a late-resolving stale mount fetch can't clobber the fresh session.GET /api/workflows/[id]envelope inline whileuseWorkflowStatecached the same endpoint's mapped state separately (two requests, two shapes, never reconciled). Now one request + one cache entry keyed byworkflowKeys.state(id): the store hydrates it viafetchQuery({ staleTime: 0 })(preserving always-refetch, incl. the socket refresh path) and the hooks derive the mappedWorkflowStateviaselect.error.tsxto the logs, knowledge, and files panels so a crash scopes to the panel instead of the whole workspace shell (reuses the sharedErrorState).useState+ effect + inline fetch withuseUnsubscribequery +useUnsubscribeMutation; contract change is additive (named type aliases only).Type of Change
Testing
cancelQueriesguard), workflow-state cache collapse (single shared entry, always-refetch, request-id staleness guard), unsubscribe query gating + mutation cache reconcile, and the logs error boundary (firstErrorStatecoverage).tsc0 errors,bun run lint,check:react-query,check:api-validationall green.Checklist