Stop writing one profile's state into another

frontend/src/hooks/useSettings.ts

@@ -68,6 +68,7 @@ import {
   updateTag,
   updateUrlAllowlist,
 } from "../api/client";
+import { setActiveProfile } from "../utils/profileStorage";
 import type { ExternalEditorSettings } from "../api/client";
 import type { AiSettings, TaskState } from "../types";
 
@@ -640,7 +641,13 @@ export function useSwitchProfile() {
   const qc = useQueryClient();
   return useMutation({
     mutationFn: (profile: string) => switchProfile(profile),
-    onSuccess: () => {
+    onSuccess: (_data, profile) => {
+      // Record the new name before the caller reloads.
+      // profileStorage reads this synchronously on the next
+      // load, which is what keeps the reads immediately
+      // after a switch on the right profile rather than on
+      // the previous one until its fetch lands.
+      setActiveProfile(profile);
       void qc.invalidateQueries();
     },
   });

frontend/src/utils/__tests__/profileStorage.test.ts

@@ -0,0 +1,118 @@
+import {
+  afterEach, beforeEach, describe, expect, it, vi,
+} from "vitest";
+
+/** Let every queued microtask and one macrotask run.
+ *  Three Promise.resolve() ticks are not enough for the
+ *  fetch -> json -> setActiveProfile chain. */
+const settle = () =>
+  new Promise((r) => setTimeout(r, 0));
+
+/**
+ * The module fetches the active profile on import and
+ * caches it, so each test needs a fresh module registry
+ * with its own fetch stub.
+ */
+async function loadModule(
+  profile: string,
+  opts: { resolve?: boolean } = {},
+) {
+  const { resolve = true } = opts;
+  let release!: () => void;
+  const gate = new Promise<void>((r) => { release = r; });
+
+  vi.stubGlobal("fetch", vi.fn(async () => {
+    if (!resolve) await gate;
+    return {
+      json: async () => ({ active: profile }),
+    } as Response;
+  }));
+
+  vi.resetModules();
+  const mod = await import("../profileStorage");
+  if (resolve) await settle();
+  return { ...mod, release };
+}
+
+beforeEach(() => {
+  localStorage.clear();
+});
+
+afterEach(() => {
+  vi.unstubAllGlobals();
+});
+
+describe("once the profile is known", () => {
+  it("prefixes reads and writes with it", async () => {
+    const m = await loadModule("org-mode");
+    expect(m.getActiveProfile()).toBe("org-mode");
+
+    m.profileSet("filter", "open");
+    expect(localStorage.getItem("org-mode:filter"))
+      .toBe("open");
+    expect(m.profileGet("filter")).toBe("open");
+
+    m.profileRemove("filter");
+    expect(m.profileGet("filter")).toBeNull();
+  });
+
+  it("keeps two profiles apart", async () => {
+    const a = await loadModule("default");
+    a.profileSet("filter", "aus default");
+
+    const b = await loadModule("org-mode");
+    expect(b.profileGet("filter")).toBeNull();
+    b.profileSet("filter", "aus org-mode");
+
+    expect(localStorage.getItem("default:filter"))
+      .toBe("aus default");
+    expect(localStorage.getItem("org-mode:filter"))
+      .toBe("aus org-mode");
+  });
+
+  it("migrates an unprefixed value once", async () => {
+    localStorage.setItem("filter", "alt");
+    const m = await loadModule("org-mode");
+    expect(m.profileGet("filter")).toBe("alt");
+    expect(localStorage.getItem("org-mode:filter"))
+      .toBe("alt");
+    expect(localStorage.getItem("filter")).toBeNull();
+  });
+});
+
+describe("before the profile is known", () => {
+  it("does not write into the wrong profile", async () => {
+    // The fetch is in flight. A component that renders
+    // now -- task cards read deadline acks during render
+    // -- calls straight through.
+    const m = await loadModule("org-mode", {
+      resolve: false,
+    });
+    m.profileSet("filter", "gehoert zu org-mode");
+
+    m.release();
+    await settle();
+
+    expect(localStorage.getItem("default:filter"))
+      .toBeNull();
+    expect(localStorage.getItem("org-mode:filter"))
+      .toBe("gehoert zu org-mode");
+  });
+
+  it("does not migrate an old value into the wrong profile",
+    async () => {
+      localStorage.setItem("filter", "alt");
+      const m = await loadModule("org-mode", {
+        resolve: false,
+      });
+      m.profileGet("filter");
+
+      m.release();
+      await settle();
+
+      // The unprefixed value belongs to whichever profile
+      // is actually active, not to "default".
+      expect(localStorage.getItem("default:filter"))
+        .toBeNull();
+    });
+});

frontend/src/utils/profileStorage.ts

@@ -5,63 +5,147 @@
  * switching profiles isolates per-profile state (advisor
  * chat history, unread badges, view filters, etc.).
  *
- * The active profile is fetched once on module load and
- * cached. Since the page reloads on every profile switch,
- * this is always current.
+ * The active profile comes from the server, and that is a
+ * fetch, which means there is a window on every page load
+ * where it is not yet known. These helpers are synchronous
+ * and get called during render, so the window is real:
+ * task cards read deadline acks while the request is still
+ * in flight.
+ *
+ * What used to happen in that window was to assume
+ * "default". Two things went wrong with that:
+ *
+ *   - A write landed under ``default:`` no matter which
+ *     profile was active, so one profile's state leaked
+ *     into another's namespace.
+ *   - A read of a legacy unprefixed key MOVED it under
+ *     ``default:`` and deleted the original. The profile
+ *     it actually belonged to lost it for good.
+ *
+ * So: the resolved name is cached in localStorage and read
+ * back synchronously on the next load, which closes the
+ * window entirely except on the first load after a switch.
+ * For that one, writes are buffered until the name
+ * arrives, and the legacy migration waits too. A read in
+ * the window can still return the previous profile's
+ * value, which is wrong on screen for a few milliseconds
+ * and, unlike the other two, undoes itself.
  */
 
-let _profile = "default";
+/** Where the last known profile name is cached. Not
+ *  prefixed: it is what tells us the prefix. */
+const ACTIVE_KEY = "kaisho:activeProfile";
+
+let _profile: string | null = null;
+/** Writes made before the name was known. */
+const pending = new Map<string, string | null>();
+
+function readCached(): string | null {
+  try {
+    return localStorage.getItem(ACTIVE_KEY);
+  } catch {
+    return null;
+  }
+}
+
+_profile = readCached();
+
+/** Apply the buffered writes now that the prefix is
+ *  known, and run any migration that was held back. */
+function flush(): void {
+  for (const [key, value] of pending) {
+    if (value === null) {
+      localStorage.removeItem(prefix(key));
+    } else {
+      localStorage.setItem(prefix(key), value);
+    }
+  }
+  pending.clear();
+}
+
+export function setActiveProfile(name: string): void {
+  _profile = name || "default";
+  try {
+    localStorage.setItem(ACTIVE_KEY, _profile);
+  } catch {
+    // A full or disabled store is not worth failing over;
+    // the fetch will resolve it again next load.
+  }
+  flush();
+}
 
-// Fetch the active profile name eagerly on module load.
-// Falls back to "default" if the fetch fails.
+// Confirm (or correct) the cached name. Always runs, even
+// when the cache was warm, so a profile switched in another
+// window is picked up.
 fetch("/api/settings/profiles")
   .then((r) => r.json())
   .then((data: { active: string }) => {
-    _profile = data.active || "default";
+    setActiveProfile(data.active || "default");
   })
-  .catch(() => {});
+  .catch(() => {
+    // Offline or the backend is not up yet. Keep whatever
+    // the cache said; flush so buffered writes are not
+    // lost, since "default" is the honest guess when there
+    // is nothing better.
+    if (_profile === null) _profile = "default";
+    flush();
+  });
 
 function prefix(key: string): string {
-  return `${_profile}:${key}`;
+  return `${_profile ?? "default"}:${key}`;
 }
 
 /**
  * Get a profile-scoped value from localStorage.
  *
- * Before the profile is loaded, falls back to the
- * unprefixed key for backwards compatibility (first
- * page load before the fetch completes).
+ * A value written in this session but not yet flushed is
+ * returned from the buffer, so a set/get pair inside the
+ * window behaves the way a caller expects.
  */
 export function profileGet(key: string): string | null {
+  if (pending.has(key)) return pending.get(key) ?? null;
+
   const prefixed = localStorage.getItem(prefix(key));
   if (prefixed !== null) return prefixed;
-  // Backwards compat: migrate old unprefixed value
+
   const old = localStorage.getItem(key);
-  if (old !== null) {
+  if (old === null) return null;
+  // Read it, but only move it once the prefix is known.
+  // Migrating under a guessed prefix deletes the original
+  // and files it under the wrong profile.
+  if (_profile !== null) {
     localStorage.setItem(prefix(key), old);
     localStorage.removeItem(key);
-    return old;
   }
-  return null;
+  return old;
 }
 
 /** Set a profile-scoped value in localStorage. */
 export function profileSet(
   key: string,
   value: string,
 ): void {
+  if (_profile === null) {
+    pending.set(key, value);
+    return;
+  }
   localStorage.setItem(prefix(key), value);
 }
 
 /** Remove a profile-scoped value from localStorage. */
 export function profileRemove(key: string): void {
+  if (_profile === null) {
+    pending.set(key, null);
+    return;
+  }
   localStorage.removeItem(prefix(key));
 }
 
 /**
- * Get the current active profile name.
- * Returns "default" before the initial fetch completes.
+ * Get the current active profile name. Returns "default"
+ * while it is still unknown, which is the right guess on a
+ * first-ever load and the only one available.
  */
 export function getActiveProfile(): string {
-  return _profile;
+  return _profile ?? "default";
 }