feat(web): migrate OrganizationProvider from React Context to Zustand store#31194
Conversation
… store Replace the React Context-based OrganizationProvider with a Zustand organization store. Fetches orgs via the generated SDK client directly, absorbs organization-state.ts (sessionStorage, module-level request state), and eliminates the Context provider wrapper from providers.tsx. - Add stores/organization-store.ts with createSelectors - Remove OrganizationProvider from providers.tsx - Delete organization-state.ts (both copies: domains/ and lib/) - Delete organization-provider.tsx (replaced by store) - Update api-interceptors.ts and AttachmentPreviewModal.tsx imports - Wire org fetch in main.tsx after auth init Closes LUM-1643 Co-Authored-By: ashlee@vellum.ai <ashlee@vellum.ai>
Document when to use React Query (domain data consumed by components) vs direct SDK calls inside Zustand stores (infrastructure state needed by middleware, interceptors, and loaders via .getState()). Co-Authored-By: ashlee@vellum.ai <ashlee@vellum.ai>
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9846f41ad5
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| useAuthStore.getState().initSession().then(() => { | ||
| if (useAuthStore.getState().isLoggedIn) { | ||
| useOrganizationStore.getState().fetchOrganizations(); | ||
| } |
There was a problem hiding this comment.
This only fetches organizations after the initial initSession() call, so if the app starts unauthenticated and later becomes authenticated without a full reload (for example via refreshSession() on focus/visibility or any in-app auth transition), organization-store is never populated and getActiveOrganizationIdForRequests() stays null. In that state, org-scoped API calls run without Vellum-Organization-Id, which can fail or target the wrong org context. The fetch needs to run whenever auth transitions into a logged-in state, not just at bootstrap.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 1cc2148. Replaced the one-shot .then() with a useAuthStore.subscribe() that watches isLoggedIn transitions. The subscription is registered before initSession(), so it covers both startup and post-login paths (OAuth callback, signup, tab-refocus re-auth).
useAuthStore.subscribe((state, prevState) => {
if (state.isLoggedIn && !prevState.isLoggedIn) {
useOrganizationStore.getState().fetchOrganizations();
}
});![vex-assistant-bot[bot]](https://avatars.githubusercontent.com/in/3399303?s=60&v=4){kind=small}
There was a problem hiding this comment.
✦ APPROVE
Companion to #31168. Same problem, same solution — the old OrganizationProvider had exactly the same "React Context can't be read outside the tree" limitation as the old AuthProvider, plus a particularly painful workaround: two duplicate organization-state.ts files (one in domains/, one in lib/, 111 lines each) manually syncing a module-level requestOrganizationId variable via useSyncExternalStore subscriptions just so API interceptors could read the active org ID. That entire sync layer is gone.
What lands: −363 lines of fragile plumbing, +163 lines of clean Zustand store. getActiveOrganizationIdForRequests() in api-interceptors.ts now delegates to useOrganizationStore.getState().currentOrganizationId — one line instead of a cascading listener set.
organization-store.ts — well-structured
create<OrganizationStore>()((set, get) => ...)curried form ✅createSelectorswrap before export ✅ — follows the #31168 convention- State interface cleanly separated from actions interface before being intersected into
OrganizationStore— good model for future stores resolveActiveOrganizationIdcorrectly handles: empty org list → null, stored ID not in list → fallback toorganizations[0](same behavior as before)clearOrganizationresets to"idle"status (not"error") — correct, matches the "never fetched" initial state rather than implying a failuresetCurrentOrganizationIdguards against setting an ID not in the list — prevents out-of-sync state ✅getActiveOrganizationIdForRequestsandclearOrganizationexported as compatibility wrappers — clean, the interceptor import just changes path with no behavior change ✅
Direct SDK call instead of React Query — correct choice, well documented
organizationsList() from sdk.gen.ts is the right call here. The org store needs to be self-contained and readable outside React, so tying it to React Query's cache (which requires a QueryClientProvider) would defeat the purpose. The CONVENTIONS.md addition documents the boundary clearly with concrete criteria. The TkDodo reference is a good anchor.
main.tsx initialization sequence
useAuthStore.getState().initSession().then(() => {
if (useAuthStore.getState().isLoggedIn) {
useOrganizationStore.getState().fetchOrganizations();
}
});The sequencing is correct — orgs are only fetched after auth resolves AND isLoggedIn is confirmed. The promise is intentionally not awaited (store's status: "loading" handles the async state while rendering proceeds).
One concern worth verifying: mid-session login path
The initSession().then() call in main.tsx handles startup. But the old OrganizationProvider used React Query's enabled: isLoggedIn && !isAuthLoading — reactive, automatically fetching orgs whenever isLoggedIn transitioned from false to true.
The new pattern doesn't have that reactivity at the org-store layer. If a user starts the app unauthenticated (redirected to /account/login via auth middleware), logs in, and returns to /assistant, fetchOrganizations() needs to be called somewhere in the login completion path.
This is probably already handled if LoginPage calls useAuthStore.getState().initSession() after successful login and that triggers a fetchOrganizations() call — but the chain isn't visible in this PR. Worth a quick sanity check: does LoginPage.tsx (from #31141) call fetchOrganizations() on success, or does initSession() in auth-store have a subscriber/side-effect that calls it?
If the gap exists, the cleanest fix is a subscriber in organization-store.ts:
// In organization-store.ts — subscribe to auth state changes
useAuthStore.subscribe(
(state) => state.isLoggedIn,
(isLoggedIn) => {
if (isLoggedIn) {
useOrganizationStore.getState().fetchOrganizations();
}
},
);Non-blocking since the startup case works correctly and most users arrive already logged in. But worth covering before the migration goes fully live.
AttachmentPreviewModal.tsx — minor note
The modal imports getActiveOrganizationIdForRequests (the imperative getter) rather than subscribing reactively via useOrganizationStore.use.currentOrganizationId(). This was true before this PR too — just redirected. If the org switches while the modal is open, the component doesn't re-render. Likely acceptable for an attachment modal, but a follow-on could update it to use the selector.
providers.tsx simplification
OrganizationProvider wrapper removed from the tree — provider composition is cleaner. ScopeKeyedQueryClientProvider reads org ID atomically via useOrganizationStore.use.currentOrganizationId() ✅. The scope-keyed cache invalidation behavior is preserved.
No bot reviews at the time of this review — first reviewer.
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
| set({ | ||
| organizations, | ||
| currentOrganizationId, | ||
| status: currentOrganizationId ? "ready" : "error", | ||
| error: currentOrganizationId | ||
| ? null | ||
| : "No organization available for this user.", | ||
| }); |
There was a problem hiding this comment.
🚩 OrganizationProvider removal drops the organization status gate
The old OrganizationProvider exposed a status field (idle | loading | ready | error) and an error field via React Context. The deriveOrganizationStatus function had a nuanced condition checking that activeRequestOrganizationId === currentOrganizationId before returning ready. The new store sets status: 'ready' as soon as fetchOrganizations resolves with a valid org ID, but consumers of this status field weren't checked. If any component previously used useOrganization().status to gate rendering (e.g., waiting for ready before showing the main app), that gate is now gone since OrganizationProvider was removed. I didn't find any such consumer in the current codebase, but it's worth verifying no downstream usage was missed.
Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
Verified — no consumer uses useOrganization().status to gate rendering. The only consumer of the old useOrganization() hook was ScopeKeyedQueryClientProvider in providers.tsx, which only read currentOrganizationId. The new store still exposes status via useOrganizationStore.use.status() for any future consumer that needs it.
…back Subscribe to auth store transitions so fetchOrganizations() fires on in-app login (OAuth callback, signup), not only at startup. Restore sessionStorage fallback in getActiveOrganizationIdForRequests() to cover the race window between page refresh and async fetch completion. Co-Authored-By: ashlee@vellum.ai <ashlee@vellum.ai>
devin-ai-integration
Bot
dismissed
vex-assistant-bot[bot]’s stale review
via
1cc2148
| useAuthStore.subscribe((state, prevState) => { | ||
| if (state.isLoggedIn && !prevState.isLoggedIn) { | ||
| useOrganizationStore.getState().fetchOrganizations(); | ||
| } | ||
| }); |
There was a problem hiding this comment.
🔴 Organization store not refetched when user changes while already logged in (cross-tab user switch)
When refreshSession() detects a different user (e.g., cross-tab session switch via BroadcastChannel or window focus), syncOrganizationState at apps/web/src/stores/auth-store.ts:78 correctly calls clearOrganization() because the user ID changed. However, the subsequent set({ isLoggedIn: true, user }) does NOT trigger the subscription in main.tsx:13-17 because isLoggedIn was already true for the previous user — the condition state.isLoggedIn && !prevState.isLoggedIn evaluates to true && !true = false.
This leaves the app with cleared organization state (organizations: [], currentOrganizationId: null, status: "idle") and no mechanism to refetch organizations for the new user. API calls will lack the Vellum-Organization-Id header and the UI will have no org context. This is a regression from the old OrganizationProvider which would remount (and re-fetch via React Query) when authScopeKey changed in AppProviders.
| useAuthStore.subscribe((state, prevState) => { | |
| if (state.isLoggedIn && !prevState.isLoggedIn) { | |
| useOrganizationStore.getState().fetchOrganizations(); | |
| } | |
| }); | |
| useAuthStore.subscribe((state, prevState) => { | |
| if (state.isLoggedIn && (!prevState.isLoggedIn || state.user?.id !== prevState.user?.id)) { | |
| useOrganizationStore.getState().fetchOrganizations(); | |
| } | |
| }); | |
Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
Good catch — fixed in 40739a7. The subscription now also checks for user ID changes:
useAuthStore.subscribe((state, prevState) => {
if (
state.isLoggedIn &&
(!prevState.isLoggedIn || state.user?.id !== prevState.user?.id)
) {
useOrganizationStore.getState().fetchOrganizations();
}
});This handles: (1) initial login, (2) OAuth callback/signup, (3) cross-tab user switch where isLoggedIn stays true but the user changes.
There was a problem hiding this comment.
Refactored in 4680728. The incremental subscription in main.tsx was a symptom of building lifecycle piecemeal instead of designing it holistically. All lifecycle logic now lives inside setupOrganizationStore() in the store module:
- Auth subscription (login + user switch) — was in main.tsx, now in store
- Focus/visibility refetch — was in separate
setupOrganizationListeners(), now integrated - Added staleTime (10s) — matches old
createQueryClient({ staleTime: 10_000 }), prevents hammering API on every tab switch
main.tsx is now a single setupOrganizationStore() call with no org-specific logic.
| fetchOrganizations: async () => { | ||
| set({ status: "loading", error: null }); | ||
|
|
||
| try { | ||
| const result = await organizationsList(); | ||
| const organizations = result.data?.results ?? []; | ||
| const candidateId = | ||
| get().currentOrganizationId ?? getStoredOrganizationId(); | ||
| const currentOrganizationId = resolveActiveOrganizationId( | ||
| organizations, | ||
| candidateId, | ||
| ); | ||
|
|
||
| if (currentOrganizationId) { | ||
| setStoredOrganizationId(currentOrganizationId); | ||
| } | ||
|
|
||
| set({ | ||
| organizations, | ||
| currentOrganizationId, | ||
| status: currentOrganizationId ? "ready" : "error", | ||
| error: currentOrganizationId | ||
| ? null | ||
| : "No organization available for this user.", | ||
| }); | ||
| } catch (err) { | ||
| const message = | ||
| err instanceof Error && err.message | ||
| ? err.message | ||
| : "Failed to load organizations."; | ||
| set({ status: "error", error: message }); | ||
| } | ||
| }, |
There was a problem hiding this comment.
🚩 Loss of automatic organization refetch on window focus/visibility
The old OrganizationProvider used React Query (useQuery(organizationsListOptions())) which would automatically re-fetch the organization list when the query became stale (after staleTime elapsed) and the window regained focus. The new Zustand store only fetches organizations once on login (via the subscription in main.tsx:13-17). If the user's organization list changes server-side (e.g., added to a new org), they won't see the update until they fully log out and back in. This is likely an intentional trade-off per the new CONVENTIONS.md section on 'Data fetching: React Query vs direct SDK calls', but worth confirming the product intent.
Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
Intentional tradeoff. The old React Query fetch with default staleTime: 0 + refetchOnWindowFocus: true would hit organizationsList() on every tab focus — wasteful for a list that changes extremely rarely (only when an admin adds you to a new org). The current behavior (fetch once on login, refetch on user change) matches the practical usage pattern. If org-list staleness becomes a product concern, we can add a manual refresh trigger or a periodic poll.
There was a problem hiding this comment.
Corrected — I was wrong to dismiss this as an acceptable tradeoff. Fixed in 32c7035.
Added setupOrganizationListeners() in organization-store.ts (mirroring the auth store's setupAuthListeners pattern) that refetches the org list on window focus and visibility change. Only fires when the store has previously loaded (status === "ready" or "error"), not on "idle" (before first login).
Called alongside setupAuthListeners() in main.tsx.
There was a problem hiding this comment.
Addressed in 4680728. The focus/visibility refetch now includes a 10s staleTime check (Date.now() - lastFetchedAt > STALE_TIME_MS) matching the old createQueryClient({ staleTime: 10_000 }). All lifecycle logic consolidated into setupOrganizationStore() in the store module.
Also trigger fetchOrganizations() when user ID changes while already logged in (e.g. cross-tab session switch via BroadcastChannel). Co-Authored-By: ashlee@vellum.ai <ashlee@vellum.ai>
Add setupOrganizationListeners() that refetches the org list when the tab regains focus or becomes visible, preserving the old React Query refetchOnWindowFocus behavior. Only fires when the store has previously loaded (status ready or error), not on idle. Co-Authored-By: ashlee@vellum.ai <ashlee@vellum.ai>
…Store()
Move auth subscription and focus/visibility refetch from main.tsx into
the store module so the store owns all of its lifecycle behavior.
- Auth subscription: fetches orgs on login and cross-tab user switch
- Focus/visibility: refetches when tab regains focus, with 10s staleTime
matching the old React Query createQueryClient({ staleTime: 10_000 })
- main.tsx reduced to a single setupOrganizationStore() call
Co-Authored-By: ashlee@vellum.ai <ashlee@vellum.ai>
|
@codex review |
There was a problem hiding this comment.
✦ APPROVE (re-review at 46807285f7)
My prior concern — mid-session login not triggering org fetch reactively — is fully addressed. The setupOrganizationStore() auth subscription is exactly the right fix:
useAuthStore.subscribe((state, prevState) => {
if (
state.isLoggedIn &&
(!prevState.isLoggedIn || state.user?.id !== prevState.user?.id)
) {
useOrganizationStore.getState().fetchOrganizations();
}
});Handles login, cross-tab BroadcastChannel sync, and user switch all in one place. ✅
The rest of the store is clean: createSelectors → useOrganizationStore.use.currentOrganizationId() in components, getState() for interceptors, getActiveOrganizationIdForRequests() fallback to sessionStorage for the pre-hydration window. The window focus / stale-time refetch logic mirrors the old React Query refetchOnWindowFocus + staleTime: 10_000 behavior precisely.
One non-blocking note for test hygiene:
setupOrganizationStore() returns a cleanup function that removes the window and document event listeners — but the two Zustand subscriptions (stale-time tracker + auth transition) are not unsubscribed in the returned cleanup. subscribe() returns an unsub function; capturing and including both in the returned cleanup would prevent subscriber accumulation across tests that call setupOrganizationStore() multiple times.
const unsubStale = useOrganizationStore.subscribe(...);
const unsubAuth = useAuthStore.subscribe(...);
// ...
return () => {
unsubStale();
unsubAuth();
window.removeEventListener("focus", onFocus);
document.removeEventListener("visibilitychange", onVisibilityChange);
};Not a production issue (store lives forever in SPA context), but clean to fix.
CONVENTIONS.md addition is excellent — the React Query vs. direct SDK calls decision tree is exactly the guidance future contributors will need when standing up new infrastructure stores. Good precedent set by pairing the implementation with the doc.
…tore teardown Co-Authored-By: ashlee@vellum.ai <ashlee@vellum.ai>
devin-ai-integration
Bot
dismissed
vex-assistant-bot[bot]’s stale review
via
8f07397
|
Good catch — fixed in 8f07397. The cleanup now unsubscribes both Zustand subscriptions alongside the DOM listeners: return () => {
unsubStale();
unsubAuth();
window.removeEventListener("focus", onFocus);
document.removeEventListener("visibilitychange", onVisibilityChange);
}; |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 46807285f7
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| set({ | ||
| organizations, | ||
| currentOrganizationId, | ||
| status: currentOrganizationId ? "ready" : "error", | ||
| error: currentOrganizationId | ||
| ? null | ||
| : "No organization available for this user.", | ||
| }); |
There was a problem hiding this comment.
fetchOrganizations() applies its result unconditionally after await organizationsList(), so an in-flight request from user A can overwrite state after clearOrganization() runs for logout or user-switch to user B. In that race, the store and sessionStorage can be repopulated with the previous account’s org ID, and interceptors will attach Vellum-Organization-Id for the wrong account until another refresh corrects it. Capture auth identity/request version at dispatch time and ignore late responses that no longer match current auth state.
Useful? React with 👍 / 👎.
| useAuthStore.subscribe((state, prevState) => { | ||
| if ( | ||
| state.isLoggedIn && | ||
| (!prevState.isLoggedIn || state.user?.id !== prevState.user?.id) | ||
| ) { | ||
| useOrganizationStore.getState().fetchOrganizations(); | ||
| } | ||
| }); |
There was a problem hiding this comment.
setupOrganizationStore() registers Zustand subscriptions but the returned cleanup only removes DOM event listeners, leaving the store subscriptions active forever. If setup runs more than once (e.g., HMR, test bootstrap/teardown, or future remountable initialization), listeners accumulate and can trigger duplicate fetchOrganizations() calls and memory leaks. Store the unsubscribe functions returned by subscribe and invoke them inside the cleanup.
Useful? React with 👍 / 👎.
Resolved conflicts in 10 files where both branches removed their respective dispatch prop-threading: - Our branch removed dispatchInteraction (interaction Zustand store) - Main's #31144 removed dispatchTurn (turn Zustand store) Resolution: remove both dispatchers, use both Zustand stores directly. Also incorporates main's #31194 (organization store migration) and Co-Authored-By: ashlee@vellum.ai <ashlee@vellum.ai> #31110 (OpenAPI spec sync).
1 participant
Prompt / plan
Migrate OrganizationProvider from React Context to a Zustand store (LUM-1643), following the same
createSelectorspattern established in the auth store migration (#31168).Why
The OrganizationProvider had the same problems that motivated the auth store migration:
React Context can't be read outside the React tree. API interceptors need the active org ID on every request (for the
Vellum-Organization-Idheader). The old provider worked around this with a separate module-level variable (requestOrganizationIdinorganization-state.ts) manually synced viauseEffect— a fragile sync layer that existed solely because Context is React-only.Zustand eliminates the sync layer. The org store is the single source of truth. Interceptors read
useOrganizationStore.getState().currentOrganizationIddirectly — no module-level variable, no listener set, nouseSyncExternalStoresubscription.Consistency. Auth and org are both cross-domain infrastructure state consumed by middleware, interceptors, and multiple domains. They should use the same pattern.
What changed
stores/organization-store.ts— Zustand store withcreateSelectors, absorbs all oforganization-state.ts(sessionStorage persistence, org resolution logic, request state). All lifecycle management is self-contained viasetupOrganizationStore().providers.tsx— removedOrganizationProviderwrapper;ScopeKeyedQueryClientProviderreads org from store directlyauth-store.ts— callsclearOrganization()from org store instead of the deleted module-level functionmain.tsx— callssetupOrganizationStore()at startup (one line, all lifecycle managed by the store)api-interceptors.tsandAttachmentPreviewModal.tsx— imports point to storedomains/organization/organization-provider.tsx(252 lines — replaced by store)domains/organization/organization-state.ts(111 lines — absorbed into store)lib/organization/organization-state.ts(111 lines — duplicate of the above)Store lifecycle design
The old
OrganizationProvidergot lifecycle management for free from React Query (reactive fetch viaenabled,refetchOnWindowFocus,staleTime). The new Zustand store manages its own lifecycle explicitly viasetupOrganizationStore(), which registers two listeners:1. Auth subscription — fetches orgs when the user logs in or switches accounts:
This covers: initial login via
initSession(), OAuth callback viarefreshSession(), signup, and cross-tab user switch via BroadcastChannel (whereisLoggedInstaystruebutuser.idchanges).2. Window focus/visibility refetch with staleTime — refetches the org list when the tab regains focus, but only if data is older than 10 seconds:
This preserves the old React Query behavior without hammering the API on every tab switch.
Both listeners live inside the store module —
main.tsxjust callssetupOrganizationStore()and doesn't contain any org logic.Parity with old system
useQuery({ enabled: isLoggedIn })isLoggedIntransitionenabledflipisLoggedIntransitionkeypropuser?.idchangerefetchOnWindowFocus: truesetupOrganizationStorefocus listenercreateQueryClient({ staleTime: 10_000 })lastFetchedAt+ 10s checkshouldClearOrganizationRequestStatesyncOrganizationState(null)→clearOrganization()organization-state.tsgetActiveOrganizationIdForRequestsfallbackgetState().currentOrganizationIddirectlysessionStorage fallback for API interceptors
getActiveOrganizationIdForRequests()falls back to sessionStorage when the store hasn't resolved yet, covering the race window between page refresh and asyncfetchOrganizations()completion:Data fetching decision: direct SDK call instead of React Query
The org list fetch uses
organizationsList()from the generated SDK client directly, not React Query. This is intentional — see the new CONVENTIONS.md section "Data fetching: React Query vs direct SDK calls" for the full rationale. In short: org state is infrastructure-level shared state consumed by both React components and non-React contexts (interceptors, middleware). Zustand stores for this category of state should be self-contained and not depend on React Query's component-tree-bound lifecycle.References:
Dead code cleaned up
organization-state.tsatlib/organization/(identical copy thatAttachmentPreviewModal.tsximported)organization-state.ts(absorbed into store)OrganizationProvider+useOrganizationhook +OrganizationContextSafety
useOrganization().ScopeKeyedQueryClientProviderwas the sole consumer — now reads from the store.getActiveOrganizationIdForRequests()has the same name, return type, and sessionStorage fallback.Test plan
bunx tsc --noEmit— passesbun run lint— passesCloses LUM-1643
Link to Devin session: https://app.devin.ai/sessions/5c400920d2b745bc84401cd020820f22
Requested by: @ashleeradka