Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
168 changes: 168 additions & 0 deletions apps/web/src/domains/settings/current-platform-assistant-store.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,168 @@
/**
* Zustand store for the per-organization "current platform assistant"
* selection. Persists the selected assistant ID for each org to
* localStorage so a reload or new tab restores the prior selection.
*
* **Storage model — one localStorage key per org:**
*
* Each org's selection is stored under
* `vellum_current_assistant_id__{orgId}`. A custom `StateStorage`
* adapter wired into the `persist` middleware reads/writes those keys
* directly, so the on-disk format stays compatible with the prior
* hand-rolled implementation.
*
* **Cross-tab sync:**
*
* The persist middleware doesn't subscribe to `storage` events on its
* own. We listen for any `storage` event whose key carries the
* per-org prefix and trigger `persist.rehydrate()` so the store
* picks up writes from other tabs.
*
* References:
* - {@link https://zustand.docs.pmnd.rs/}
* - {@link https://zustand.docs.pmnd.rs/integrations/persisting-store-data}
*/

import { create } from "zustand";
import {
createJSONStorage,
persist,
type StateStorage,
} from "zustand/middleware";

import { createSelectors } from "@/utils/create-selectors.js";

export const PLATFORM_ASSISTANT_STORAGE_PREFIX =
"vellum_current_assistant_id__";

export interface CurrentPlatformAssistantState {
/** orgId → selected assistant ID. Absent entries mean "no selection yet". */
byOrg: Record<string, string>;
}

export interface CurrentPlatformAssistantActions {
setAssistantId: (orgId: string, id: string | null) => void;
getAssistantId: (orgId: string) => string | null;
}

export type CurrentPlatformAssistantStore = CurrentPlatformAssistantState &
CurrentPlatformAssistantActions;

function readByOrgFromLocalStorage(): Record<string, string> {
if (typeof window === "undefined") return {};
const byOrg: Record<string, string> = {};
try {
for (let i = 0; i < window.localStorage.length; i++) {
const key = window.localStorage.key(i);
if (key && key.startsWith(PLATFORM_ASSISTANT_STORAGE_PREFIX)) {
const orgId = key.slice(PLATFORM_ASSISTANT_STORAGE_PREFIX.length);
const value = window.localStorage.getItem(key);
if (value != null) byOrg[orgId] = value;
}
}
} catch {
// ignore storage failures
}
return byOrg;
}

function writeByOrgToLocalStorage(byOrg: Record<string, string>): void {
if (typeof window === "undefined") return;
try {
const existing: string[] = [];
for (let i = 0; i < window.localStorage.length; i++) {
const key = window.localStorage.key(i);
if (key && key.startsWith(PLATFORM_ASSISTANT_STORAGE_PREFIX)) {
existing.push(key);
}
}
for (const key of existing) {
const orgId = key.slice(PLATFORM_ASSISTANT_STORAGE_PREFIX.length);
if (byOrg[orgId] == null) {
window.localStorage.removeItem(key);
}
Comment on lines +79 to +83
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Avoid deleting other-org keys during persist writes

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 👍 / 👎.

}
for (const [orgId, id] of Object.entries(byOrg)) {
window.localStorage.setItem(
`${PLATFORM_ASSISTANT_STORAGE_PREFIX}${orgId}`,
id,
);
}
} catch {
// ignore storage failures
}
}

/**
* Translates the single-name `name` view that `persist` expects into
* per-org reads and writes against the existing
* `vellum_current_assistant_id__{orgId}` localStorage keys.
*/
const perOrgStorage: StateStorage = {
getItem: () => {
const byOrg = readByOrgFromLocalStorage();
return JSON.stringify({ state: { byOrg }, version: 0 });
},
setItem: (_name, value) => {
let parsed: { state?: { byOrg?: Record<string, string> } };
try {
parsed = JSON.parse(value) as typeof parsed;
} catch {
return;
}
const byOrg = parsed.state?.byOrg ?? {};
writeByOrgToLocalStorage(byOrg);
},
removeItem: () => {
writeByOrgToLocalStorage({});
},
};

const CURRENT_PLATFORM_ASSISTANT_STORE_NAME =
"vellum:current-platform-assistant";

const useCurrentPlatformAssistantStoreBase = create<CurrentPlatformAssistantStore>()(
persist(
(set, get) => ({
byOrg: readByOrgFromLocalStorage(),

setAssistantId: (orgId, id) =>
set((state) => {
const next = { ...state.byOrg };
if (id == null) {
delete next[orgId];
} else {
next[orgId] = id;
}
return { byOrg: next };
}),

getAssistantId: (orgId) => get().byOrg[orgId] ?? null,
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚩 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.

Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

}),
{
name: CURRENT_PLATFORM_ASSISTANT_STORE_NAME,
storage: createJSONStorage(() => perOrgStorage),
partialize: (state) => ({ byOrg: state.byOrg }),
},
),
);

export const useCurrentPlatformAssistantStore = createSelectors(
useCurrentPlatformAssistantStoreBase,
);

// ---------------------------------------------------------------------------
// Cross-tab sync
// ---------------------------------------------------------------------------

if (typeof window !== "undefined") {
window.addEventListener("storage", (event) => {
if (event.key === null) {
void useCurrentPlatformAssistantStoreBase.persist.rehydrate();
return;
}
if (event.key.startsWith(PLATFORM_ASSISTANT_STORAGE_PREFIX)) {
void useCurrentPlatformAssistantStoreBase.persist.rehydrate();
}
});
}
Original file line number Diff line number Diff line change
@@ -1,72 +1,15 @@
import { useQuery } from "@tanstack/react-query";
import { useCallback, useEffect, useSyncExternalStore } from "react";
import { useCallback, useEffect } from "react";

import { assistantsListOptions } from "@/generated/api/@tanstack/react-query.gen.js";
import type { Assistant } from "@/generated/api/types.gen.js";
import { useCurrentPlatformAssistantStore } from "@/domains/settings/current-platform-assistant-store.js";
import { useOrganizationStore } from "@/stores/organization-store.js";

const PLATFORM_ASSISTANT_STORAGE_PREFIX = "vellum_current_assistant_id__";

const PLATFORM_LIST_OPTIONS = assistantsListOptions({
query: { hosting: "platform" },
});

function storageKeyForOrg(orgId: string | null): string | null {
if (!orgId) return null;
return `${PLATFORM_ASSISTANT_STORAGE_PREFIX}${orgId}`;
}

type Listener = () => void;
const listeners = new Set<Listener>();

function notify(): void {
for (const listener of listeners) listener();
}

if (typeof window !== "undefined") {
window.addEventListener("storage", (event: StorageEvent) => {
if (event.key === null) {
notify();
return;
}
if (event.key.startsWith(PLATFORM_ASSISTANT_STORAGE_PREFIX)) {
notify();
}
});
}

function subscribe(listener: Listener): () => void {
listeners.add(listener);
return () => {
listeners.delete(listener);
};
}

function getSnapshot(orgId: string | null): string | null {
const key = storageKeyForOrg(orgId);
if (!key) return null;
try {
return window.localStorage.getItem(key);
} catch {
return null;
}
}

function writeStoredId(orgId: string | null, id: string | null): void {
const key = storageKeyForOrg(orgId);
if (!key) return;
try {
if (id == null) {
window.localStorage.removeItem(key);
} else {
window.localStorage.setItem(key, id);
}
} catch {
// ignore storage failures
}
notify();
}

export interface UseCurrentPlatformAssistantResult {
assistantId: string | null;
assistant: Assistant | null;
Expand All @@ -78,12 +21,11 @@ export interface UseCurrentPlatformAssistantResult {

export function useCurrentPlatformAssistant(): UseCurrentPlatformAssistantResult {
const orgId = useOrganizationStore.use.currentOrganizationId();
const byOrg = useCurrentPlatformAssistantStore.use.byOrg();
const setAssistantIdAction =
useCurrentPlatformAssistantStore.use.setAssistantId();
Comment on lines +25 to +26
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 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)
Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.


const storedId = useSyncExternalStore(
subscribe,
useCallback(() => getSnapshot(orgId), [orgId]),
() => null,
);
const storedId = orgId ? (byOrg[orgId] ?? null) : null;

const listQuery = useQuery(PLATFORM_LIST_OPTIONS);

Expand All @@ -109,16 +51,24 @@ export function useCurrentPlatformAssistant(): UseCurrentPlatformAssistantResult
if (!isListLoaded) return;
if (platformAssistants.length === 0) return;
if (resolvedId === storedId) return;
if (resolvedId != null) {
writeStoredId(orgId, resolvedId);
if (resolvedId != null && orgId) {
setAssistantIdAction(orgId, resolvedId);
}
}, [isListLoaded, platformAssistants.length, resolvedId, storedId, orgId]);
}, [
isListLoaded,
platformAssistants.length,
resolvedId,
storedId,
orgId,
setAssistantIdAction,
]);

const setAssistantId = useCallback(
(id: string | null) => {
writeStoredId(orgId, id);
if (!orgId) return;
setAssistantIdAction(orgId, id);
},
[orgId],
[orgId, setAssistantIdAction],
);

return {
Expand Down