fix(settings): guard custom-instructions sync against out-of-order reads

Two reads in flight at once (rapid toggle off/on/off/on) could resolve
out of order, letting the stale read land last and win while sync is
on. A late read resolving after toggle-off could likewise deposit a
latent snapshot. Capture a generation before the await and discard any
read a newer reconcile has superseded. The read is a host round-trip,
so on slower transports than local IPC the window is real.

Covers the contribution with tests: happy path, clear-on-disable, and
both race orderings (verified the race tests fail without the guard).

Generated-By: PostHog Code
Task-Id: b6359217-b9b8-40bb-a748-308fa450a732

Author: pauldambra

packages/ui/src/features/settings/customInstructionsSync.contribution.test.ts

@@ -0,0 +1,114 @@
+import type { HostTrpcClient } from "@posthog/host-router/client";
+import { registerRendererStateStorage } from "@posthog/ui/shell/rendererStorage";
+import { beforeEach, describe, expect, it, vi } from "vitest";
+import { CustomInstructionsSyncContribution } from "./customInstructionsSync.contribution";
+import { useSettingsStore } from "./settingsStore";
+
+registerRendererStateStorage({
+  getItem: vi.fn().mockResolvedValue(null),
+  setItem: vi.fn().mockResolvedValue(undefined),
+  removeItem: vi.fn().mockResolvedValue(undefined),
+});
+
+const query = vi.fn();
+const client = {
+  os: { getUserAgentInstructions: { query } },
+} as unknown as HostTrpcClient;
+
+const staleFile = {
+  path: "/home/u/.claude/CLAUDE.md",
+  displayPath: "~/.claude/CLAUDE.md",
+  content: "stale",
+  truncated: false,
+};
+const freshFile = { ...staleFile, content: "fresh" };
+
+/** Runs queued microtasks so a resolved read's continuation lands. */
+const flush = () => new Promise<void>((resolve) => setTimeout(resolve, 0));
+
+const setSyncEnabled = (enabled: boolean) =>
+  useSettingsStore.getState().setSyncCustomInstructionsFromFile(enabled);
+
+describe("CustomInstructionsSyncContribution", () => {
+  // The contribution subscribes to the module-level store and never
+  // unsubscribes, so one shared instance serves every test.
+  let started = false;
+
+  beforeEach(() => {
+    useSettingsStore.setState({
+      _hasHydrated: true,
+      syncCustomInstructionsFromFile: false,
+      syncedCustomInstructions: null,
+    });
+    query.mockReset();
+    if (!started) {
+      new CustomInstructionsSyncContribution(client).start();
+      started = true;
+    }
+  });
+
+  it("mirrors the file into the store when sync turns on", async () => {
+    query.mockResolvedValueOnce(freshFile);
+
+    setSyncEnabled(true);
+    await flush();
+
+    expect(useSettingsStore.getState().syncedCustomInstructions).toEqual(
+      freshFile,
+    );
+  });
+
+  it("clears the snapshot when sync turns off", async () => {
+    query.mockResolvedValueOnce(freshFile);
+    setSyncEnabled(true);
+    await flush();
+
+    setSyncEnabled(false);
+
+    expect(useSettingsStore.getState().syncedCustomInstructions).toBeNull();
+  });
+
+  it("discards a read that resolves after sync was toggled off", async () => {
+    let resolveRead: (file: typeof staleFile) => void = () => {};
+    query.mockReturnValueOnce(
+      new Promise((resolve) => {
+        resolveRead = resolve;
+      }),
+    );
+
+    setSyncEnabled(true);
+    setSyncEnabled(false);
+    resolveRead(staleFile);
+    await flush();
+
+    expect(useSettingsStore.getState().syncedCustomInstructions).toBeNull();
+  });
+
+  it("keeps the newest read when re-enable reads resolve out of order", async () => {
+    let resolveFirst: (file: typeof staleFile) => void = () => {};
+    let resolveSecond: (file: typeof freshFile) => void = () => {};
+    query
+      .mockReturnValueOnce(
+        new Promise((resolve) => {
+          resolveFirst = resolve;
+        }),
+      )
+      .mockReturnValueOnce(
+        new Promise((resolve) => {
+          resolveSecond = resolve;
+        }),
+      );
+
+    setSyncEnabled(true);
+    setSyncEnabled(false);
+    setSyncEnabled(true);
+    resolveSecond(freshFile);
+    await flush();
+    resolveFirst(staleFile);
+    await flush();
+
+    expect(useSettingsStore.getState().syncedCustomInstructions).toEqual(
+      freshFile,
+    );
+  });
+});

packages/ui/src/features/settings/customInstructionsSync.contribution.ts

@@ -17,6 +17,14 @@ const log = logger.scope("custom-instructions-sync");
  */
 @injectable()
 export class CustomInstructionsSyncContribution implements Contribution {
+  /**
+   * Bumped by every reconcile so an in-flight read that a newer toggle flip
+   * has superseded cannot land its (now stale) snapshot in the store. The
+   * read is a host round-trip, so on slower transports rapid toggling can
+   * genuinely have two reads in flight resolving out of order.
+   */
+  private generation = 0;
+
   constructor(
     @inject(HOST_TRPC_CLIENT)
     private readonly hostClient: HostTrpcClient,
@@ -39,6 +47,7 @@ export class CustomInstructionsSyncContribution implements Contribution {
   }
 
   private async reconcile(enabled: boolean): Promise<void> {
+    const generation = ++this.generation;
     if (!enabled) {
       useSettingsStore.getState().setSyncedCustomInstructions(null);
       return;
@@ -50,6 +59,9 @@ export class CustomInstructionsSyncContribution implements Contribution {
     useSettingsStore.getState().setSyncedCustomInstructions(null);
     try {
       const file = await this.hostClient.os.getUserAgentInstructions.query();
+      // A newer toggle flip started its own reconcile while this read was in
+      // flight; its outcome owns the store now.
+      if (generation !== this.generation) return;
       useSettingsStore.getState().setSyncedCustomInstructions(file);
     } catch (err) {
       // The snapshot was already cleared above, so a transient read failure