Skip to content
Merged
56 changes: 55 additions & 1 deletion apps/web/docs/STATE_MANAGEMENT.md
Comment thread
ashleeradka marked this conversation as resolved.
Original file line number Diff line number Diff line change
Expand Up @@ -196,6 +196,60 @@ for when to use each API.

Reference: [Zustand — Auto Generating Selectors](https://zustand.docs.pmnd.rs/learn/guides/auto-generating-selectors)

## Status as one discriminated union, never parallel booleans

Model an async or multi-phase status as a single
[discriminated-union](https://www.typescriptlang.org/docs/handbook/2/narrowing.html#discriminated-unions)
field, not a pair of booleans. A `(isLoading, isLoggedIn)` pair has four
combinations but only three are legal — `(true, true)` ("loading yet already
logged in") is meaningless, and every reader has to remember which combination
means what. A single `sessionStatus: "initializing" | "authenticated" |
"unauthenticated"` makes the illegal state unrepresentable and names each phase
once.

```ts
// Avoid — parallel booleans: 4 states encode 3, (true,true) is illegal
interface AuthState { isLoading: boolean; isLoggedIn: boolean }

// Prefer — one field, every value legal and named
type SessionStatus = "initializing" | "authenticated" | "unauthenticated";
interface AuthState { sessionStatus: SessionStatus }
```

References:
- [TypeScript — Discriminated unions](https://www.typescriptlang.org/docs/handbook/2/narrowing.html#discriminated-unions)
- [Making illegal states unrepresentable](https://fsharpforfunandprofit.com/posts/designing-with-types-making-illegal-states-unrepresentable/)

## Where derivation lives: inline vs shared predicate

Deriving a value from store state (`status === "authenticated"`,
`user?.id`) belongs **inline in the consumer** when it is a one-off — keep the
store's surface minimal and the derivation next to where it's used.

Extract a **shared predicate** when the *same* derivation recurs across
modules. Re-encoding `status === "authenticated"` at six call sites leaks the
field's encoding to every reader: the question "is the user authenticated?"
should be answered in one place, so a change to the encoding touches one line.
Give the owning module two forms:

- a **pure predicate** `isAuthenticated(status)` for imperative readers
(middleware, route resolvers, services) that already hold a value, and
- a **hook** `useIsAuthenticated()` that composes the predicate over an atomic
selector, for reactive components.

```ts
// Pure predicate — owns the encoding, usable anywhere
export const isAuthenticated = (s: SessionStatus) => s === "authenticated";

// Hook — reactive read for components, composes the predicate
export const useIsAuthenticated = () =>
isAuthenticated(useAuthStore.use.sessionStatus());
```

Keep pure predicates in a dependency-free module when modules the store
*depends on* must also read them — `import type` cannot break a cycle through a
runtime value (a predicate), only through a type.

## Reading state: `.use.*` vs `.getState()`

Zustand exposes two ways to read store state. Using the wrong one
Expand All @@ -218,7 +272,7 @@ const handleClick = useCallback(() => {
}, []);

// Middleware — outside React
const { isLoggedIn } = useAuthStore.getState();
const { sessionStatus } = useAuthStore.getState();
```

Zustand's `set()` is synchronous — `.getState()` after an action
Expand Down
9 changes: 4 additions & 5 deletions apps/web/src/assistant/lifecycle-service.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -97,8 +97,7 @@ function makeQueryClient(): QueryClient {
}

const baseInputs = {
isLoggedIn: true,
isLoading: false,
sessionStatus: "authenticated" as const,
isRetired: false,
isNonProduction: false,
hasPlatformSession: true,
Expand Down Expand Up @@ -193,7 +192,7 @@ describe("lifecycleService — server state projection", () => {
});

describe("lifecycleService — bootstrap branches", () => {
test("respondToInputs with isLoggedIn=false clears both stores (safety-net for token-expiry-style auth flips that don't call logout())", async () => {
test("respondToInputs with an unauthenticated session clears both stores (safety-net for token-expiry-style auth flips that don't call logout())", async () => {
// Drive the service into an `active` state through the
// legitimate path so its internal state mirrors the store.
getAssistantMock.mockImplementationOnce(async () => ({
Expand All @@ -215,10 +214,10 @@ describe("lifecycleService — bootstrap branches", () => {
"active",
);

// Now flip isLoggedIn false and reconcile.
// Now flip the session unauthenticated and reconcile.
lifecycleService.setInputs({
...baseInputs,
isLoggedIn: false,
sessionStatus: "unauthenticated",
queryClient: makeQueryClient(),
});
await lifecycleService.respondToInputs();
Expand Down
20 changes: 9 additions & 11 deletions apps/web/src/assistant/lifecycle-service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -50,13 +50,13 @@ import {
isLocalMode,
} from "@/lib/local-mode";
import { setSelfHostedConnection } from "@/lib/self-hosted/connection";
import { isAuthenticated, type SessionStatus } from "@/stores/session-status";

const MAX_HATCH_RETRIES = 3;
const MAX_INITIALIZING_RECOVERIES = 3;

export interface LifecycleServiceInputs {
isLoggedIn: boolean;
isLoading: boolean;
sessionStatus: SessionStatus;
isRetired: boolean;
isNonProduction: boolean;
hasPlatformSession: boolean;
Expand Down Expand Up @@ -105,8 +105,7 @@ class AssistantLifecycleService {
*/
private ready = false;
private inputs: LifecycleServiceInputs = {
isLoggedIn: false,
isLoading: true,
sessionStatus: "initializing",
isRetired: false,
isNonProduction: false,
hasPlatformSession: false,
Expand All @@ -126,9 +125,9 @@ class AssistantLifecycleService {

/**
* Synchronously drop the selection + lifecycle state. Called from
* `auth-store.logout()` before `isLoggedIn` flips, so subscribers
* to either store don't observe a stale id in their first
* re-render after logout. The `respondToInputs` `!isLoggedIn`
* `auth-store.logout()` before `sessionStatus` leaves `authenticated`,
* so subscribers to either store don't observe a stale id in their
* first re-render after logout. The `respondToInputs` not-authenticated
* branch is the safety net for cases where auth flips without
* going through the explicit logout call (e.g. token expiry
* detected by an interceptor and surfaced as a state change
Expand Down Expand Up @@ -158,7 +157,7 @@ class AssistantLifecycleService {
*/
async respondToInputs(): Promise<void> {
if (!this.ready) return;
if (!this.inputs.isLoggedIn || this.inputs.isLoading) {
if (!isAuthenticated(this.inputs.sessionStatus)) {
// Logout / pre-auth boot — same reset as `resetForLogout` but
// reachable from the input-driven path for token-expiry style
// flips that don't call `logout()` explicitly.
Expand Down Expand Up @@ -672,7 +671,7 @@ class AssistantLifecycleService {
/**
* Reset all state to the post-import default. For tests only —
* production code should never call this. (Use `respondToInputs`
* with `isLoggedIn: false` for logout reset.)
* with a non-authenticated `sessionStatus` for logout reset.)
*/
__resetForTesting(): void {
if (this.initializingTimeout) {
Expand All @@ -689,8 +688,7 @@ class AssistantLifecycleService {
this.ready = false;
useAssistantLifecycleStore.setState({ expectingFirstMessage: false });
this.inputs = {
isLoggedIn: false,
isLoading: true,
sessionStatus: "initializing",
isRetired: false,
isNonProduction: false,
hasPlatformSession: false,
Expand Down
16 changes: 6 additions & 10 deletions apps/web/src/assistant/use-lifecycle.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,10 +19,10 @@ import { lifecycleService } from "@/assistant/lifecycle-service";
import { useAssistantQuery } from "@/assistant/queries";
import { isGatewayAuthMode } from "@/lib/auth/gateway-session";
import { isLocalMode } from "@/lib/local-mode";
import { isAuthenticated, type SessionStatus } from "@/stores/session-status";

interface UseAssistantLifecycleOptions {
isLoggedIn: boolean;
isLoading: boolean;
sessionStatus: SessionStatus;
isRetired: boolean;
isNonProduction: boolean;
hasPlatformSession: boolean;
Expand All @@ -40,8 +40,7 @@ interface UseAssistantLifecycleOptions {
}

export function useAssistantLifecycle({
isLoggedIn,
isLoading,
sessionStatus,
isRetired,
isNonProduction,
hasPlatformSession,
Expand All @@ -54,8 +53,7 @@ export function useAssistantLifecycle({
// mode and "local mode without platform session" short-circuit
// to local states without ever calling /assistant/.
const shouldQueryServer =
isLoggedIn &&
!isLoading &&
isAuthenticated(sessionStatus) &&
!isGatewayAuthMode() &&
(hasPlatformSession || !isLocalMode());

Expand All @@ -69,8 +67,7 @@ export function useAssistantLifecycle({
// hook itself.
useEffect(() => {
lifecycleService.setInputs({
isLoggedIn,
isLoading,
sessionStatus,
isRetired,
isNonProduction,
hasPlatformSession,
Expand All @@ -80,8 +77,7 @@ export function useAssistantLifecycle({
});
void lifecycleService.respondToInputs();
}, [
isLoggedIn,
isLoading,
sessionStatus,
isRetired,
isNonProduction,
hasPlatformSession,
Expand Down
10 changes: 5 additions & 5 deletions apps/web/src/components/providers.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ import { QueryClient, QueryClientProvider } from "@tanstack/react-query";
import { TooltipProvider } from "@vellum/design-library";
import { useState, type ReactNode } from "react";

import { useAuthStore } from "@/stores/auth-store";
import { useAuthStore, useIsAuthenticated } from "@/stores/auth-store";
import { useOrganizationStore } from "@/stores/organization-store";

function createQueryClient(): QueryClient {
Expand Down Expand Up @@ -54,12 +54,12 @@ function ScopeKeyedQueryClientProvider({
}: {
children: ReactNode;
}) {
const isLoggedIn = useAuthStore.use.isLoggedIn();
const isAuthenticated = useIsAuthenticated();
const user = useAuthStore.use.user();
const currentOrganizationId =
useOrganizationStore.use.currentOrganizationId();
const scopeKey = `${
isLoggedIn ? `user:${user?.id ?? "unknown"}` : "anonymous"
isAuthenticated ? `user:${user?.id ?? "unknown"}` : "anonymous"
}:org:${currentOrganizationId ?? "none"}`;

return (
Expand All @@ -70,9 +70,9 @@ function ScopeKeyedQueryClientProvider({
}

export function AppProviders({ children }: { children: ReactNode }) {
const isLoggedIn = useAuthStore.use.isLoggedIn();
const isAuthenticated = useIsAuthenticated();
const user = useAuthStore.use.user();
const authScopeKey = isLoggedIn
const authScopeKey = isAuthenticated
? `user:${user?.id ?? "unknown"}`
: "anonymous";

Expand Down
14 changes: 9 additions & 5 deletions apps/web/src/domains/account/pages/account-page.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -6,16 +6,20 @@ import { AccountShell } from "@/components/account/account-shell";
import { PROVIDER_CALLBACK_URL, PROVIDER_ID } from "@/domains/account/login-flow";
import { hardNavigate } from "@/lib/auth/hard-navigate";
import { startAuthFlow } from "@/runtime/native-auth";
import { useAuthStore } from "@/stores/auth-store";
import {
useAuthStore,
useIsAuthenticated,
useIsSessionInitializing,
} from "@/stores/auth-store";
import { routes } from "@/utils/routes";

/**
* Account landing page. Shows a sign-in CTA when unauthenticated,
* or a "Go to your assistant" link + sign-out button when logged in.
*/
export function AccountPage() {
const isLoggedIn = useAuthStore.use.isLoggedIn();
const isLoading = useAuthStore.use.isLoading();
const isAuthenticated = useIsAuthenticated();
const isSessionInitializing = useIsSessionInitializing();
const user = useAuthStore.use.user();
const logout = useAuthStore.use.logout();
const [errorMessage, setErrorMessage] = useState<string | null>(null);
Expand All @@ -33,15 +37,15 @@ export function AccountPage() {
}
};

if (isLoading) {
if (isSessionInitializing) {
return (
<AccountShell>
<AccountHeading title="Loading..." />
</AccountShell>
);
}

if (!isLoggedIn) {
if (!isAuthenticated) {
return (
<AccountShell>
<AccountHeading
Expand Down
5 changes: 3 additions & 2 deletions apps/web/src/domains/account/pages/platform-loopback-page.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -54,8 +54,9 @@ export function PlatformLoopbackPage() {
document.cookie = `sessionid=${sessionToken}; path=/; samesite=lax; max-age=1209600`;

void (async () => {
// Re-run session init now that the cookie is set — this updates
// isLoggedIn so the auth middleware lets navigation through.
// Re-run session init now that the cookie is set — this moves
// sessionStatus to "authenticated" so the auth middleware lets
// navigation through.
await useAuthStore.getState().initSession();

try {
Expand Down
6 changes: 3 additions & 3 deletions apps/web/src/domains/chat/chat-page.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
import { useCallback, useEffect, useRef } from "react";
import * as Sentry from "@sentry/react";

import { useAuthStore } from "@/stores/auth-store";
import { useIsSessionInitializing } from "@/stores/auth-store";
import { lifecycleService } from "@/assistant/lifecycle-service";
import { useAssistantLifecycleStore } from "@/assistant/lifecycle-store";
import { useAssistantSelectionStore } from "@/assistant/selection-store";
Expand All @@ -30,7 +30,7 @@ import { VersionSelectionScreen } from "@/domains/chat/components/version-select
import { ActiveChatView } from "@/domains/chat/active-chat-view";

export function ChatPage() {
const authLoading = useAuthStore.use.isLoading();
const isSessionInitializing = useIsSessionInitializing();
const assistantState = useAssistantLifecycleStore.use.assistantState();
const assistantId = useAssistantSelectionStore.use.activeAssistantId();
const selfHostedChatEnabled = useClientFeatureFlagStore.use.selfHostedAssistant();
Expand Down Expand Up @@ -58,7 +58,7 @@ export function ChatPage() {
// -------------------------------------------------------------------------
// Loading guards — auth / assistant lifecycle not yet resolved
// -------------------------------------------------------------------------
const connectingReason = authLoading
const connectingReason = isSessionInitializing
? "auth_loading"
: assistantState.kind === "loading"
? "assistant_loading"
Expand Down
12 changes: 7 additions & 5 deletions apps/web/src/domains/chat/components/preferences-menu.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -20,20 +20,22 @@ mock.module("@/hooks/use-is-mobile", () => ({
}));

const authRef = {
isLoggedIn: true,
isAuthenticated: true,
user: { id: "u1", email: "user@example.com", isStaff: false, username: null, firstName: "", lastName: "" },
logout: async () => {},
};

mock.module("@/stores/auth-store", () => {
const store = () => null;
store.use = {
isLoggedIn: () => authRef.isLoggedIn,
user: () => authRef.user,
logout: () => authRef.logout,
};
store.getState = () => authRef;
return { useAuthStore: store };
return {
useAuthStore: store,
useIsAuthenticated: () => authRef.isAuthenticated,
};
});

const flagsRef = {};
Expand Down Expand Up @@ -91,14 +93,14 @@ import { PreferencesMenu } from "@/domains/chat/components/preferences-menu";

beforeEach(() => {
isMobileRef.value = false;
authRef.isLoggedIn = true;
authRef.isAuthenticated = true;
authRef.user = { id: "u1", email: "user@example.com", isStaff: false, username: null, firstName: "", lastName: "" };
billingRef.data = undefined;
});

describe("PreferencesMenu", () => {
test("renders nothing when not logged in", () => {
authRef.isLoggedIn = false;
authRef.isAuthenticated = false;
const html = renderToStaticMarkup(createElement(PreferencesMenu));
expect(html).toBe("");
});
Expand Down
Loading