Keep rapid settings edits from reverting each other

apps/web/src/hooks/pendingServerSettings.test.ts

@@ -0,0 +1,118 @@
+import {
+  DEFAULT_SERVER_SETTINGS,
+  EnvironmentId,
+  ProviderDriverKind,
+  ProviderInstanceId,
+} from "@t3tools/contracts";
+import { afterEach, describe, expect, it } from "vite-plus/test";
+
+import {
+  __resetPendingServerPatchesForTests,
+  applyPendingServerPatches,
+  getPendingServerPatches,
+  releasePendingServerPatch,
+  retainPendingServerPatch,
+  subscribePendingServerPatches,
+} from "./pendingServerSettings";
+
+const environmentId = EnvironmentId.make("environment-1");
+const codexId = ProviderInstanceId.make("codex");
+const claudeId = ProviderInstanceId.make("claudeAgent");
+
+const providerInstance = (driver: string, enabled: boolean) => ({
+  driver: ProviderDriverKind.make(driver),
+  enabled,
+});
+
+afterEach(() => {
+  __resetPendingServerPatchesForTests();
+});
+
+describe("pendingServerSettings", () => {
+  it("has no overlay before a write is dispatched", () => {
+    expect(getPendingServerPatches(environmentId)).toEqual([]);
+    expect(getPendingServerPatches(null)).toEqual([]);
+    expect(applyPendingServerPatches(DEFAULT_SERVER_SETTINGS, [])).toBe(DEFAULT_SERVER_SETTINGS);
+  });
+
+  it("keeps a second provider toggle from reverting the first one", () => {
+    // Disabling codex is dispatched from the server's own (empty) map.
+    const disableCodex = {
+      providerInstances: { [codexId]: providerInstance("codex", false) },
+    };
+    retainPendingServerPatch(environmentId, disableCodex);
+
+    // Before the echo lands, the panel must already see codex disabled so the
+    // next whole-map replacement it builds carries that edit forward.
+    const optimistic = applyPendingServerPatches(
+      DEFAULT_SERVER_SETTINGS,
+      getPendingServerPatches(environmentId),
+    );
+    expect(optimistic.providerInstances[codexId]?.enabled).toBe(false);
+
+    const disableClaude = {
+      providerInstances: {
+        ...optimistic.providerInstances,
+        [claudeId]: providerInstance("claudeAgent", false),
+      },
+    };
+    retainPendingServerPatch(environmentId, disableClaude);
+
+    const both = applyPendingServerPatches(
+      DEFAULT_SERVER_SETTINGS,
+      getPendingServerPatches(environmentId),
+    );
+    expect(both.providerInstances[codexId]?.enabled).toBe(false);
+    expect(both.providerInstances[claudeId]?.enabled).toBe(false);
+  });
+
+  it("retains the overlay until the last outstanding write settles", () => {
+    const patch = { providerInstances: { [codexId]: providerInstance("codex", false) } };
+    retainPendingServerPatch(environmentId, patch);
+    retainPendingServerPatch(environmentId, patch);
+
+    releasePendingServerPatch(environmentId);
+    expect(getPendingServerPatches(environmentId)).toHaveLength(2);
+
+    releasePendingServerPatch(environmentId);
+    expect(getPendingServerPatches(environmentId)).toEqual([]);
+  });
+
+  it("drops the overlay for a failed write so the server value wins again", () => {
+    retainPendingServerPatch(environmentId, {
+      providerInstances: { [codexId]: providerInstance("codex", false) },
+    });
+    releasePendingServerPatch(environmentId);
+
+    const settings = applyPendingServerPatches(
+      DEFAULT_SERVER_SETTINGS,
+      getPendingServerPatches(environmentId),
+    );
+    expect(settings).toBe(DEFAULT_SERVER_SETTINGS);
+  });
+
+  it("scopes pending writes to their own environment", () => {
+    const otherEnvironmentId = EnvironmentId.make("environment-2");
+    retainPendingServerPatch(environmentId, {
+      providerInstances: { [codexId]: providerInstance("codex", false) },
+    });
+
+    expect(getPendingServerPatches(otherEnvironmentId)).toEqual([]);
+    releasePendingServerPatch(otherEnvironmentId);
+    expect(getPendingServerPatches(environmentId)).toHaveLength(1);
+  });
+
+  it("notifies subscribers when the pending set changes", () => {
+    let notifications = 0;
+    const unsubscribe = subscribePendingServerPatches(() => {
+      notifications += 1;
+    });
+
+    retainPendingServerPatch(environmentId, { enableAssistantStreaming: false });
+    releasePendingServerPatch(environmentId);
+    unsubscribe();
+    retainPendingServerPatch(environmentId, { enableAssistantStreaming: false });
+
+    expect(notifications).toBe(2);
+  });
+});

apps/web/src/hooks/pendingServerSettings.ts

@@ -0,0 +1,107 @@
+/**
+ * Optimistic overlay for server settings writes that are still in flight.
+ *
+ * Server settings only change locally once the server echoes a
+ * `settingsUpdated` broadcast back over the websocket. Until that round trip
+ * lands, readers still see pre-edit settings, so a second edit issued inside
+ * the window is computed from stale state — and because keys such as
+ * `providerInstances` are whole-value replacements rather than deep merges,
+ * that second write silently reverts the first one. (Toggle one provider off,
+ * toggle a second off a moment later, and the first provider's switch snaps
+ * back on once its own write is overwritten.)
+ *
+ * Each in-flight patch is retained here and replayed on top of the server value
+ * through the same `applyServerSettingsPatch` the server runs, so both reads and
+ * follow-up patches are built from the edit the user just made. Retained patches
+ * are dropped once no write is outstanding for that environment, at which point
+ * the server value is authoritative again — including when a write failed.
+ *
+ * @module hooks/pendingServerSettings
+ */
+import type { EnvironmentId, ServerSettings, ServerSettingsPatch } from "@t3tools/contracts";
+import { applyServerSettingsPatch } from "@t3tools/shared/serverSettings";
+
+interface PendingServerPatches {
+  readonly patches: ReadonlyArray<ServerSettingsPatch>;
+  /** Writes dispatched for this environment that have not settled yet. */
+  readonly inFlight: number;
+}
+
+export const NO_PENDING_SERVER_PATCHES: ReadonlyArray<ServerSettingsPatch> = [];
+
+const pendingByEnvironment = new Map<EnvironmentId, PendingServerPatches>();
+const listeners = new Set<() => void>();
+
+function emitChange(): void {
+  for (const listener of listeners) {
+    listener();
+  }
+}
+
+export function subscribePendingServerPatches(listener: () => void): () => void {
+  listeners.add(listener);
+  return () => {
+    listeners.delete(listener);
+  };
+}
+
+/**
+ * Snapshot accessor for `useSyncExternalStore`: the returned array is stable
+ * until the environment's pending set actually changes.
+ */
+export function getPendingServerPatches(
+  environmentId: EnvironmentId | null,
+): ReadonlyArray<ServerSettingsPatch> {
+  if (environmentId === null) return NO_PENDING_SERVER_PATCHES;
+  return pendingByEnvironment.get(environmentId)?.patches ?? NO_PENDING_SERVER_PATCHES;
+}
+
+/** Record a patch that has just been dispatched to the server. */
+export function retainPendingServerPatch(
+  environmentId: EnvironmentId,
+  patch: ServerSettingsPatch,
+): void {
+  const pending = pendingByEnvironment.get(environmentId);
+  pendingByEnvironment.set(environmentId, {
+    patches: [...(pending?.patches ?? NO_PENDING_SERVER_PATCHES), patch],
+    inFlight: (pending?.inFlight ?? 0) + 1,
+  });
+  emitChange();
+}
+
+/**
+ * Mark one dispatched write as settled. Patches are only forgotten when the
+ * last outstanding write settles: releasing them one at a time would expose the
+ * server value before its later echo arrived, flickering the UI back to a state
+ * the user has already edited away from.
+ */
+export function releasePendingServerPatch(environmentId: EnvironmentId): void {
+  const pending = pendingByEnvironment.get(environmentId);
+  if (pending === undefined) return;
+  if (pending.inFlight <= 1) {
+    pendingByEnvironment.delete(environmentId);
+  } else {
+    pendingByEnvironment.set(environmentId, {
+      patches: pending.patches,
+      inFlight: pending.inFlight - 1,
+    });
+  }
+  emitChange();
+}
+
+/** Replay in-flight patches over an authoritative server settings value. */
+export function applyPendingServerPatches(
+  settings: ServerSettings,
+  patches: ReadonlyArray<ServerSettingsPatch>,
+): ServerSettings {
+  if (patches.length === 0) return settings;
+  return patches.reduce<ServerSettings>(
+    (current, patch) => applyServerSettingsPatch(current, patch),
+    settings,
+  );
+}
+
+export function __resetPendingServerPatchesForTests(): void {
+  pendingByEnvironment.clear();
+  listeners.clear();
+}

apps/web/src/hooks/useSettings.ts

@@ -24,6 +24,14 @@ import {
   type UnifiedSettings,
 } from "@t3tools/contracts/settings";
 import { safeErrorLogAttributes } from "@t3tools/client-runtime/errors";
+import {
+  applyPendingServerPatches,
+  getPendingServerPatches,
+  NO_PENDING_SERVER_PATCHES,
+  releasePendingServerPatch,
+  retainPendingServerPatch,
+  subscribePendingServerPatches,
+} from "./pendingServerSettings";
 import { ensureLocalApi } from "~/localApi";
 import * as Struct from "effect/Struct";
 import { primaryServerSettingsAtom, serverEnvironment } from "~/state/server";
@@ -140,6 +148,17 @@ function persistClientSettings(settings: ClientSettings): void {
     });
 }
 
+function usePendingServerPatches(
+  environmentId: EnvironmentId | null,
+): ReadonlyArray<ServerSettingsPatch> {
+  const getSnapshot = useCallback(() => getPendingServerPatches(environmentId), [environmentId]);
+  return useSyncExternalStore(
+    subscribePendingServerPatches,
+    getSnapshot,
+    () => NO_PENDING_SERVER_PATCHES,
+  );
+}
+
 // ── Key sets for routing patches ─────────────────────────────────────
 
 const SERVER_SETTINGS_KEYS = new Set<string>(Struct.keys(ServerSettings.fields));
@@ -198,14 +217,21 @@ export function mergeEnvironmentSettings(
 }
 
 function useMergedSettings<T>(
+  environmentId: EnvironmentId | null,
   serverSettings: ServerSettings,
   selector: ((settings: UnifiedSettings) => T) | undefined,
 ): T {
   const clientSettings = useClientSettingsValue();
+  const pendingPatches = usePendingServerPatches(environmentId);
+
+  const optimisticServerSettings = useMemo<ServerSettings>(
+    () => applyPendingServerPatches(serverSettings, pendingPatches),
+    [pendingPatches, serverSettings],
+  );
 
   const merged = useMemo<UnifiedSettings>(
-    () => mergeEnvironmentSettings(serverSettings, clientSettings),
-    [clientSettings, serverSettings],
+    () => mergeEnvironmentSettings(optimisticServerSettings, clientSettings),
+    [clientSettings, optimisticServerSettings],
   );
 
   return useMemo(() => (selector ? selector(merged) : (merged as T)), [merged, selector]);
@@ -224,20 +250,21 @@ export function useEnvironmentSettings<T = UnifiedSettings>(
   selector?: (settings: UnifiedSettings) => T,
 ): T {
   const serverSettings = useAtomValue(serverEnvironment.settingsValueAtom(environmentId));
-  return useMergedSettings(serverSettings ?? DEFAULT_SERVER_SETTINGS, selector);
+  return useMergedSettings(environmentId, serverSettings ?? DEFAULT_SERVER_SETTINGS, selector);
 }
 
 /** Primary-only settings access for the settings UI and other explicitly global surfaces. */
 export function usePrimarySettings<T = UnifiedSettings>(
   selector?: (settings: UnifiedSettings) => T,
 ): T {
-  return useMergedSettings(useAtomValue(primaryServerSettingsAtom), selector);
+  const environmentId = usePrimaryEnvironment()?.environmentId ?? null;
+  return useMergedSettings(environmentId, useAtomValue(primaryServerSettingsAtom), selector);
 }
 
 /**
  * Returns an updater that routes each key to the correct backing store.
  *
- * Server keys are optimistically patched in atom-backed server state, then
+ * Server keys are applied optimistically through `./pendingServerSettings` and
  * persisted via RPC. Client keys go through client persistence.
  */
 function useUpdateSettingsTarget(environmentId: EnvironmentId | null) {
@@ -251,9 +278,12 @@ function useUpdateSettingsTarget(environmentId: EnvironmentId | null) {
 
       if (Object.keys(serverPatch).length > 0) {
         if (environmentId) {
+          retainPendingServerPatch(environmentId, serverPatch);
           void persistServerSettings({
             environmentId,
             input: { patch: serverPatch },
+          }).finally(() => {
+            releasePendingServerPatch(environmentId);
           });
         }
       }