refactor(web): convert current-platform-assistant hook to Zustand store (LUM-1718)#31433
Conversation
…1718)
Replace the hand-rolled `useSyncExternalStore` + module-level
listener Set in `use-current-platform-assistant` with a Zustand
`persist`-backed store. A custom `StateStorage` adapter keeps the
existing per-org `vellum_current_assistant_id__{orgId}`
localStorage key format so the on-disk shape stays backward
compatible. React Query still owns the platform assistants list;
the hook is now a thin composition of the store + the query.
![vex-assistant-bot[bot]](https://avatars.githubusercontent.com/in/3399303?s=60&v=4){kind=small}
There was a problem hiding this comment.
✦ APPROVE
Checked against all five Migration Review Gates from the Zustand patterns KB.
Gate 1 — Auth/org-switch ✅
storedId is derived inline as orgId ? (byOrg[orgId] ?? null) : null. When org changes, orgId changes → derivation re-evaluates against the right slice of byOrg. No stale-state-on-switch risk. The store is already org-scoped, so there's no cross-user bleed concern (unlike the prechat_native_screen key that PR #31431 just fixed — different problem, handled differently).
Gate 2 — Cleanup completeness ✅
The storage event listener is module-level (registered once at module load), same as the old listeners set in the deleted code. Module-level listeners don't leak — they live for the lifetime of the module. No component-scoped subscription to clean up.
Gate 3 — CONVENTIONS.md — N/A for an intra-domain refactor. No new pattern being established beyond what's already documented.
Gate 4 — Selector usage ✅
const byOrg = useCurrentPlatformAssistantStore.use.byOrg();
const setAssistantIdAction = useCurrentPlatformAssistantStore.use.setAssistantId();Both use .use.field() from createSelectors. No useShallow, no bare useStore(). createSelectors wraps the base store correctly at the bottom of the store file.
Gate 5 — getState() in non-React code ✅
getAssistantId uses get().byOrg[orgId] inside the store factory — that's Zustand's get, not a hook. Correct.
perOrgStorage adapter ✅
The custom StateStorage translates persist's single-name interface into per-org vellum_current_assistant_id__{orgId} keys. The two-loop pattern in writeByOrgToLocalStorage (collect first, then delete/write) is safe — no iteration-while-mutating. partialize: (state) => ({ byOrg: state.byOrg }) correctly excludes actions from serialization. Backward-compatible on-disk format preserved.
Double JSON round-trip — createJSONStorage(() => perOrgStorage) will stringify the state envelope before passing to perOrgStorage.setItem, which then parses it to extract byOrg. Redundant serialize/deserialize but harmless.
Cross-tab sync ✅
if (event.key.startsWith(PLATFORM_ASSISTANT_STORAGE_PREFIX)) {
void useCurrentPlatformAssistantStoreBase.persist.rehydrate();
}Correctly calls .persist.rehydrate() on the base store (not the createSelectors-wrapped export — .persist lives on the raw store). void to suppress the Promise warning. Equivalent behavior to the deleted notify() plumbing, with less code.
setAssistantIdAction in effect deps ✅
Zustand actions have stable references across renders — including this in the useEffect dep array is correct and won't trigger extra effect runs.
delete next[orgId] ✅
The spread { ...state.byOrg } before the delete creates a new object — no mutation of existing state.
CI — 7/7 green. ✅
Net: 69 lines added, 69 lines deleted from the hook (-50 lines net including the new store file is actually +168 / -69). The useSyncExternalStore + module-level pub/sub is gone; the store is cleaner and consistent with organization-store.ts and auth-store.ts.
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
| const setAssistantIdAction = | ||
| useCurrentPlatformAssistantStore.use.setAssistantId(); |
There was a problem hiding this comment.
🟡 Action obtained via .use.setAssistantId() subscription instead of .getState(), violating documented store convention
The hook subscribes to the setAssistantId action via useCurrentPlatformAssistantStore.use.setAssistantId() at line 26 and adds it to the dependency arrays of both useEffect (line 63) and useCallback (line 71). Per docs/STATE_MANAGEMENT.md, the convention table explicitly says actions should be called via useStore.getState().actionName() "anywhere" because "actions are stable references — calling via .getState() is always correct and avoids adding the action to dependency arrays." The create-selectors.ts:11 utility docstring reiterates: .getState().field for event handlers, callbacks, effects. This also creates an unnecessary React subscription to the action function. The fix is to remove the .use.setAssistantId() call and instead call useCurrentPlatformAssistantStore.getState().setAssistantId(orgId, resolvedId) directly inside the useEffect and useCallback bodies.
Prompt for agents
In apps/web/src/domains/settings/hooks/use-current-platform-assistant.ts, the setAssistantId action is obtained via useCurrentPlatformAssistantStore.use.setAssistantId() at the render body level (line 25-26), creating an unnecessary subscription. Per the STATE_MANAGEMENT.md convention, actions should be called via .getState() to avoid subscriptions and dependency array entries.
To fix:
1. Remove lines 25-26 (the const setAssistantIdAction = useCurrentPlatformAssistantStore.use.setAssistantId() assignment)
2. In the useEffect body (line 55), replace setAssistantIdAction(orgId, resolvedId) with useCurrentPlatformAssistantStore.getState().setAssistantId(orgId, resolvedId)
3. Remove setAssistantIdAction from the useEffect dependency array (line 63)
4. In the useCallback body (line 69), replace setAssistantIdAction(orgId, id) with useCurrentPlatformAssistantStore.getState().setAssistantId(orgId, id)
5. Remove setAssistantIdAction from the useCallback dependency array (line 71)
Was this helpful? React with 👍 or 👎 to provide feedback.
| return { byOrg: next }; | ||
| }), | ||
|
|
||
| getAssistantId: (orgId) => get().byOrg[orgId] ?? null, |
There was a problem hiding this comment.
🚩 getAssistantId action is defined but unused — intentional API surface
The getAssistantId action (current-platform-assistant-store.ts:45,140) is defined in the store interface and implementation but never called anywhere in the codebase. The hook at use-current-platform-assistant.ts:28 reads byOrg directly instead. This was considered as potential dead code per the AGENTS.md dead-code-removal rule, but since this is a new store with a public API, getAssistantId serves as a convenience for non-React consumers (e.g., useCurrentPlatformAssistantStore.getState().getAssistantId(orgId)). Leaving it is reasonable, but if no other consumer is planned, removing it would follow the repo's "proactively remove unused code" guideline.
Was this helpful? React with 👍 or 👎 to provide feedback.
![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: af53e38110
ℹ️ 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".
| for (const key of existing) { | ||
| const orgId = key.slice(PLATFORM_ASSISTANT_STORAGE_PREFIX.length); | ||
| if (byOrg[orgId] == null) { | ||
| window.localStorage.removeItem(key); | ||
| } |
There was a problem hiding this comment.
writeByOrgToLocalStorage treats the in-memory byOrg snapshot as authoritative and removes every prefixed localStorage key not present in that snapshot. In multi-tab usage, a stale tab can write before its storage-triggered rehydrate() runs, causing it to delete assistant selections another tab just wrote for different orgs. The prior implementation only touched the active org key, so this introduces a lost-update regression under concurrent tab writes.
Useful? React with 👍 / 👎.
2 participants
Summary
Replaces the hand-rolled
useSyncExternalStore+ module-level listener set inuse-current-platform-assistantwith a Zustandpersist-backed store, matching the rest of the codebase.domains/settings/current-platform-assistant-store.ts— Zustand store withpersistmiddleware, wrapped viacreateSelectors(). State isbyOrg: Record<orgId, assistantId>; actions aresetAssistantId(orgId, id)/getAssistantId(orgId).StateStorageadapter keeps writes split across the existing per-orgvellum_current_assistant_id__{orgId}localStorage keys, so the on-disk format stays backward compatible with the prior implementation.storageevent listener callspersist.rehydrate()on any matching key, replacing the previous module-levellistenersset +notify()plumbing.useCurrentPlatformAssistantis now a thin composition of the store + the list query — nouseSyncExternalStore, no module-level pub/sub.Linear: https://linear.app/vellum/issue/LUM-1718
Test plan
bunx tsc --noEmitpassesbun run lintpasses (no new warnings)vellum_current_assistant_id__{orgId}localStorage entries still apply on first load after upgradeGenerated by Claude Code