fix: prevent false divergence when disabling second PVM

apps/web/e2e/sprint-25-disable-pvm-divergence.spec.ts

@@ -0,0 +1,129 @@
+import { expect, test } from "@playwright/test";
+
+test.describe("Sprint 25 — No divergence after disabling second PVM", () => {
+  /** Load a program and wait for the debugger page to be visible. */
+  async function loadProgram(page: import("@playwright/test").Page) {
+    await page.goto("/#/load");
+    const card = page.getByTestId("example-card-step-test");
+    await expect(card).toBeVisible();
+    await card.click();
+    await expect(page.getByTestId("debugger-page")).toBeVisible({
+      timeout: 15000,
+    });
+  }
+
+  /** Open the settings tab in the bottom drawer. */
+  async function openSettings(page: import("@playwright/test").Page) {
+    await page.getByTestId("drawer-tab-settings").click();
+    await expect(page.getByTestId("settings-tab")).toBeVisible();
+  }
+
+  /**
+   * Attempt to enable the Ananas PVM via settings.
+   * Returns true if both PVM tabs appear, false otherwise.
+   */
+  async function tryEnableAnanas(
+    page: import("@playwright/test").Page,
+  ): Promise<boolean> {
+    await openSettings(page);
+    const ananasSwitch = page.getByTestId("pvm-switch-ananas");
+    await expect(ananasSwitch).toBeVisible();
+    await ananasSwitch.click();
+    try {
+      await expect(page.getByTestId("pvm-tab-ananas")).toHaveRole("tab", {
+        timeout: 15000,
+      });
+      return true;
+    } catch {
+      return false;
+    }
+  }
+
+  test("no divergence after enabling then disabling second PVM", async ({
+    page,
+  }) => {
+    await loadProgram(page);
+
+    // Enable ananas
+    const enabled = await tryEnableAnanas(page);
+    expect(enabled).toBe(true);
+
+    // Wait for both PVMs to be fully loaded (blue dots = paused state)
+    await expect(page.getByTestId("pvm-dot-typeberry")).toHaveClass(
+      /bg-blue-500/,
+      { timeout: 15000 },
+    );
+    await expect(page.getByTestId("pvm-dot-ananas")).toHaveClass(
+      /bg-blue-500/,
+      { timeout: 15000 },
+    );
+
+    // Both freshly loaded PVMs should not diverge
+    await expect(page.getByTestId("divergence-summary")).not.toBeVisible();
+
+    // Disable ananas (settings already open from tryEnableAnanas)
+    const ananasSwitch = page.getByTestId("pvm-switch-ananas");
+    await ananasSwitch.click();
+
+    // Wait for ananas to be removed (reverts to grayed-out span, no role="tab")
+    await expect(page.getByTestId("pvm-tab-ananas")).not.toHaveAttribute(
+      "role",
+      "tab",
+      { timeout: 15000 },
+    );
+
+    // Should NOT show divergence with only one PVM
+    await expect(page.getByTestId("divergence-summary")).not.toBeVisible();
+
+    // Typeberry should still be functional (blue dot = paused)
+    await expect(page.getByTestId("pvm-dot-typeberry")).toHaveClass(
+      /bg-blue-500/,
+      { timeout: 15000 },
+    );
+  });
+
+  test("no divergence after stepping, enabling, then disabling second PVM", async ({
+    page,
+  }) => {
+    await loadProgram(page);
+
+    // Step a few times with just typeberry
+    const nextBtn = page.getByTestId("next-button");
+    await expect(nextBtn).toBeVisible({ timeout: 10000 });
+    await nextBtn.click();
+    await expect(nextBtn).toBeEnabled({ timeout: 5000 });
+    await nextBtn.click();
+    await expect(nextBtn).toBeEnabled({ timeout: 5000 });
+
+    // Enable ananas (this resets the session — both PVMs start fresh)
+    const enabled = await tryEnableAnanas(page);
+    expect(enabled).toBe(true);
+
+    // Wait for both PVMs to load
+    await expect(page.getByTestId("pvm-dot-typeberry")).toHaveClass(
+      /bg-blue-500/,
+      { timeout: 15000 },
+    );
+    await expect(page.getByTestId("pvm-dot-ananas")).toHaveClass(
+      /bg-blue-500/,
+      { timeout: 15000 },
+    );
+
+    // No divergence — both loaded fresh with identical state
+    await expect(page.getByTestId("divergence-summary")).not.toBeVisible();
+
+    // Disable ananas
+    const ananasSwitch = page.getByTestId("pvm-switch-ananas");
+    await ananasSwitch.click();
+
+    // Wait for transition
+    await expect(page.getByTestId("pvm-tab-ananas")).not.toHaveAttribute(
+      "role",
+      "tab",
+      { timeout: 15000 },
+    );
+
+    // No divergence after disabling
+    await expect(page.getByTestId("divergence-summary")).not.toBeVisible();
+  });
+});

apps/web/src/hooks/useOrchestratorState.ts

@@ -59,12 +59,19 @@ export function useOrchestratorState(): OrchestratorReactiveState {
   const pendingPerPvmErrors = useRef<Map<string, string> | null>(null);
   const pendingStepDone = useRef(false);
   const rafId = useRef<number | null>(null);
+  // Tracks the most recently flushed snapshots so post-flush event handlers
+  // don't fall back to the stale `initial` closure captured at subscription time.
+  const lastFlushedSnapshots = useRef<Map<
+    string,
+    { snapshot: MachineStateSnapshot; lifecycle: PvmLifecycle }
+  > | null>(null);
 
   const scheduleFlush = useCallback(() => {
     if (rafId.current !== null) return;
     rafId.current = requestAnimationFrame(() => {
       rafId.current = null;
       if (pendingSnapshots.current) {
+        lastFlushedSnapshots.current = pendingSnapshots.current;
         setSnapshots(pendingSnapshots.current);
         pendingSnapshots.current = null;
       }
@@ -91,6 +98,7 @@ export function useOrchestratorState(): OrchestratorReactiveState {
   useEffect(() => {
     // Discard any buffered data from the previous orchestrator
     pendingSnapshots.current = null;
+    lastFlushedSnapshots.current = null;
     pendingHostCallInfo.current = null;
     pendingPerPvmErrors.current = null;
     pendingSelectedPvmId.current = null;
@@ -121,15 +129,22 @@ export function useOrchestratorState(): OrchestratorReactiveState {
       prev && pvmIds.includes(prev) ? prev : (pvmIds[0] ?? null),
     );
 
-    // Use `initial` (from this orchestrator) as the fallback base for event
-    // accumulation — NOT the stale `snapshots` React state from the closure.
-    // This prevents disabled-PVM data from leaking into the new orchestrator.
+    // Event handlers accumulate into `pendingSnapshots`. After each rAF flush
+    // clears it, the fallback base is `lastFlushedSnapshots` (most recent flush),
+    // then `initial` (pre-loadProgram state from this orchestrator). This chain
+    // prevents two bugs:
+    //   1. Stale React state leaking disabled-PVM data into a new orchestrator
+    //   2. Post-flush events reverting already-loaded PVM state to pre-load defaults
+    //      (the rAF race: PVM A loads → flush → PVM B loads → fallback to `initial`
+    //       would lose A's loaded state without `lastFlushedSnapshots`)
     const onStateChanged = (
       pvmId: string,
       snapshot: MachineStateSnapshot,
       lifecycle: PvmLifecycle,
     ) => {
-      const base = pendingSnapshots.current ?? new Map(initial);
+      const base =
+        pendingSnapshots.current ??
+        new Map(lastFlushedSnapshots.current ?? initial);
       base.set(pvmId, { snapshot, lifecycle });
       pendingSnapshots.current = base;
       pendingSelectedPvmId.current = pvmId;
@@ -169,7 +184,9 @@ export function useOrchestratorState(): OrchestratorReactiveState {
     };
 
     const onTerminated = (pvmId: string, reason: PvmStatus) => {
-      const base = pendingSnapshots.current ?? new Map(initial);
+      const base =
+        pendingSnapshots.current ??
+        new Map(lastFlushedSnapshots.current ?? initial);
       const entry = base.get(pvmId);
       if (entry) {
         base.set(pvmId, {
@@ -187,7 +204,9 @@ export function useOrchestratorState(): OrchestratorReactiveState {
       const isTimeout = /timeout/i.test(error.message);
       const lifecycle: PvmLifecycle = isTimeout ? "timed_out" : "failed";
 
-      const base = pendingSnapshots.current ?? new Map(initial);
+      const base =
+        pendingSnapshots.current ??
+        new Map(lastFlushedSnapshots.current ?? initial);
       const entry = base.get(pvmId);
       if (entry) {
         base.set(pvmId, { snapshot: entry.snapshot, lifecycle });

apps/web/src/pages/DebuggerPage.tsx

@@ -44,8 +44,17 @@ function DebuggerPageInner() {
     snapshotVersion,
     perPvmErrors,
   } = useOrchestratorState();
-  const { summary: divergenceSummary, details: divergenceDetails } =
+  const { summary: rawDivergenceSummary, details: rawDivergenceDetails } =
     useDivergenceCheck(snapshots, selectedPvmId, snapshotVersion);
+  // Suppress divergence during PVM reload — workers respond at different
+  // times so there's a brief window where one PVM has loaded state and the
+  // other still has defaults, producing a false divergence flash.
+  const divergenceSummary = isReloadingRef.current
+    ? null
+    : rawDivergenceSummary;
+  const divergenceDetails = isReloadingRef.current
+    ? null
+    : rawDivergenceDetails;
   const instructions = useDisassembly(envelope);
 
   const { activeTab, openToTab, setActiveTab } = useDrawer();

spec/ui/sprint-25-divergence-detection.md

@@ -14,6 +14,8 @@ When multiple PVMs are active, detect and surface divergences between them. Show
 - No divergence output when fewer than 2 PVMs are active
 - Failed/timed-out PVMs show inline red error text
 - Divergence clears after reset
+- No false divergence when enabling or disabling the second PVM
+- No divergence flash during PVM reload (workers responding at different times)
 
 ## Prior Sprint Dependencies
 
@@ -23,8 +25,11 @@ When multiple PVMs are active, detect and surface divergences between them. Show
 
 ```
 apps/web/src/hooks/useDivergenceCheck.ts
-apps/web/src/components/debugger/PvmTabs.tsx    (extend with divergence display)
+apps/web/src/hooks/useOrchestratorState.ts      (lastFlushedSnapshots ref)
+apps/web/src/pages/DebuggerPage.tsx              (isReloadingRef divergence gate)
+apps/web/src/components/debugger/PvmTabs.tsx     (extend with divergence display)
 apps/web/e2e/sprint-25-divergence.spec.ts
+apps/web/e2e/sprint-25-disable-pvm-divergence.spec.ts
 ```
 
 ## Divergence Contract
@@ -73,6 +78,8 @@ Implementation pitfall: failed/timed-out sessions do not emit `pvmStateChanged`;
 - divergence clears after reset
 - error text appears inline for failed PVMs
 - no divergence shown with single PVM
+- no divergence after enabling then disabling second PVM
+- no divergence after stepping, enabling, then disabling second PVM
 ```
 
 ## Acceptance Criteria
@@ -81,6 +88,8 @@ Implementation pitfall: failed/timed-out sessions do not emit `pvmStateChanged`;
 - Summary is concise; tooltip has full details.
 - Failed/timed-out PVMs show inline error text.
 - Divergence clears after reset.
+- No false divergence when enabling or disabling PVMs.
+- No divergence flash during PVM reload.
 - `cd apps/web && npx vite build` succeeds.
 - E2E tests pass.
 
@@ -94,9 +103,18 @@ Implementation pitfall: failed/timed-out sessions do not emit `pvmStateChanged`;
 - Multi-PVM E2E tests depend on PVM switching via settings, which has a pre-existing timing issue from sprint-24 (orchestrator reload clears program state briefly). These tests are conditionally skipped with `test.skip` when PVM switching doesn't stabilize.
 - Unit tests (22 total) thoroughly cover: all divergence field types, concise summary formatting, edge cases (empty snapshots, missing PVM ID), error display for failed/timed-out PVMs, and tooltip content.
 
+### False divergence on PVM enable/disable
+
+Two bugs caused false divergence when toggling the second PVM:
+
+1. **Stale-closure race in rAF flush** (`useOrchestratorState`): The `onStateChanged` handler accumulated into `pendingSnapshots`, using `new Map(initial)` as fallback after each rAF flush cleared `pendingSnapshots.current`. The `initial` variable was captured before `loadProgram` completed, so when two PVM workers responded across different animation frames, the second event reverted the first PVM's loaded state to pre-load defaults (e.g., gas=0 instead of gas=10000). Fix: a `lastFlushedSnapshots` ref tracks the most recently flushed snapshot map, providing an up-to-date fallback base. The chain is: `pendingSnapshots.current ?? new Map(lastFlushedSnapshots.current ?? initial)`.
+
+2. **Divergence flash during reload** (`DebuggerPage`): Even with the above fix, PVM workers respond at different speeds during `loadProgram`. For one render frame, one PVM has loaded state while the other still has defaults. Fix: divergence results are gated behind `isReloadingRef`, which is already `true` for the duration of `loadProgram` in `onPvmChange`. Since snapshot updates (which trigger renders) arrive while the ref is still `true`, the flash is suppressed. When `loadProgram` finishes and the ref becomes `false`, both PVMs have matching state so divergence is null anyway.
+
 ## Verification
 
 ```bash
 cd apps/web && npx vite build
 npx playwright test e2e/sprint-25-divergence.spec.ts
+npx playwright test e2e/sprint-25-disable-pvm-divergence.spec.ts
 ```