fix(02): plan revision iter 3 — BLOCKER 3 cross-plan regression + W1/W2

BLOCKER 3 — cross-plan regression: Plans 02-03 and 02-05 BOTH re-author src/sim/garden/commands.ts but had reverted simulateOneTick to the old defective return shape (`return { ...next, lastTickAt: currentTick };`). Wave 1's execution of 02-03 would overwrite 02-02's correct version, breaking the invariant for the entire phase. - 02-03: simulateOneTick return now matches 02-02 line 457 exactly: `return { ...next, tickCount: next.tickCount + 1 };` - 02-05: same fix for the silent-mode update (Step 6). - 02-03 acceptance_criteria: add negative grep (`! grep -E "lastTickAt:\s*(this|currentTick)" src/sim/garden/commands.ts`) and positive grep (`grep -q "tickCount: next.tickCount" ...`). - 02-05 acceptance_criteria: add the same two greps for commands.ts so 02-05's silent-mode edits cannot silently re-introduce the regression. W1 — App.tsx import: 02-05 Step 11 used `useEffect` without importing it. Combined `import { useState }` and `import { useRef }` into a single `import { useState, useEffect, useRef } from 'react';` line. W2 — helper arity divergence: Settings.tsx (one-arg, Date.now() inline) and PhaserGame.tsx (two-arg, clock.now() injected) had two parallel definitions of buildPayloadFromStore / hydrateStoreFromPayload. Fix: - New Step 3.5 introduces `src/save/payload.ts` with the unified two-arg signature: `buildPayloadFromStore(state, nowMs)` and `hydrateStoreFromPayload(state, payload)`. - `src/save/index.ts` re-exports both. - Settings.tsx imports from save barrel; passes Date.now() at the call site (no clock injection on hand). - PhaserGame.tsx imports from save barrel; passes clock.now() (the injected wallClock or FakeClock). - Inline duplicate definitions in both files removed; replaced with a comment pointing to the shared module. - files_modified updated to include src/save/payload.ts. - acceptance_criteria asserts: shared file exists, both helpers exported, both consumers import from save barrel, no inline duplicate definitions remain. VALIDATION.md not updated — no `<automated>` verify command changed; the new greps live inside `<acceptance_criteria>` (executor-checked per task), and VALIDATION.md is not present in the phase dir. All iteration-1 + iteration-2 fixes preserved; no regressions. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-05-09 03:15:39 -04:00
parent d065922cad
commit a641056364
2 changed files with 121 additions and 71 deletions
@@ -605,7 +605,11 @@ export function simulateOneTick(
      next = compost(next, cmd.tileIdx, currentTick);
    }
  }
-  return { ...next, lastTickAt: currentTick };
+  // BLOCKER 3 invariant (matches Plan 02-02 line 457): the sim writes
+  // tickCount (sim-internal counter for STRY-10), NEVER lastTickAt.
+  // lastTickAt is wall-clock ms owned by saveSync. Re-authoring this file
+  // in 02-03 must preserve the increment that 02-02 established.
+  return { ...next, tickCount: next.tickCount + 1 };
 }
 ```

@@ -1038,6 +1042,8 @@ Plan 02-04's authored content will land the actual lines.
    - `grep -q "kind: 'harvest'" src/game/scenes/Garden.ts`
    - `grep -q "kind: 'compost'" src/game/scenes/Garden.ts`
    - `grep -q "setFragmentRevealId" src/game/scenes/Garden.ts` (reveal flow wired)
+    - `! grep -E "lastTickAt:\\s*(this|currentTick)" src/sim/garden/commands.ts` (BLOCKER 3 — 02-03's edits to commands.ts must not re-introduce sim-side lastTickAt writes)
+    - `grep -q "tickCount: next.tickCount" src/sim/garden/commands.ts` (BLOCKER 3 — 02-02's tickCount increment must survive 02-03's harvest/compost edits)
    - `npx vitest run src/ui/journal/` exits 0 with all tests green (≥10 cases across 2 files)
    - `npm run ci` exits 0
  </acceptance_criteria>
@@ -15,6 +15,7 @@ files_modified:
  - src/sim/garden/commands.ts
  - src/save/migrations.ts
  - src/save/index.ts
+  - src/save/payload.ts
  - src/ui/letter/Letter.tsx
  - src/ui/letter/Letter.test.tsx
  - src/ui/letter/letter-renderer.ts
@@ -380,7 +381,10 @@ export function simulateOneTick(state, currentTick, commands, ctx): SimState {
  if (ctx.silent) {
    next = autoHarvestReadyPlants(next, currentTick, ctx);
  }
-  return { ...next, lastTickAt: currentTick };
+  // BLOCKER 3 invariant (matches Plan 02-02 line 457 and Plan 02-03):
+  // the sim NEVER writes lastTickAt. saveSync owns wall-clock ms; the
+  // sim owns the tickCount counter (STRY-10).
+  return { ...next, tickCount: next.tickCount + 1 };
 }
 ```

@@ -697,12 +701,93 @@ export { buildLetterSlots } from './letter-renderer';
 export type { LetterSlots } from './letter-renderer';
 ```

+**Step 3.5 — `src/save/payload.ts`** — shared helpers (W2 fix: deduplicate Settings.tsx and PhaserGame.tsx):
+
+The previous revision left two parallel definitions of `buildPayloadFromStore` /
+`hydrateStoreFromPayload` — one in `Settings.tsx` (one-arg, calling `Date.now()`
+inline) and one in `PhaserGame.tsx` (two-arg, taking the clock-derived `nowMs`).
+That arity divergence is the source of W2. The fix: extract BOTH helpers to a
+single module with the two-arg signature; both call sites import from it.
+
+The `nowMs` parameter is the wall-clock ms to write into `payload.lastTickAt`.
+Settings.tsx (no clock injection on hand) passes `Date.now()`; PhaserGame.tsx
+passes `clock.now()` (the injected clock — wallClock or FakeClock). The
+two-arg surface unifies the contract; the *value* passed differs, which is
+correct.
+
+```typescript
+import type { AppState } from '../store';
+import type { V1Payload } from './schemas';
+
+/**
+ * Build a V1Payload save envelope from the current store state.
+ *
+ * BLOCKER 3 invariants:
+ *   - lastTickAt is wall-clock ms — owned by saveSync (PhaserGame) and the
+ *     Settings export path. The sim NEVER writes lastTickAt.
+ *   - tickCount is the sim-internal monotonic counter (STRY-10) — read from
+ *     the store; the sim writes it via simulateOneTick.
+ *
+ * @param s        Snapshot of the store state (`useAppStore.getState()`).
+ * @param nowMs    Wall-clock milliseconds to record as `lastTickAt`.
+ *                 PhaserGame.tsx passes `clock.now()` (injected clock);
+ *                 Settings.tsx passes `Date.now()` (no clock on hand).
+ */
+export function buildPayloadFromStore(s: AppState, nowMs: number): V1Payload {
+  return {
+    garden: { tiles: s.tiles },
+    plants: [],
+    harvestedFragmentIds: s.harvestedFragmentIds,
+    lastTickAt: nowMs,                    // wall-clock ms
+    tickCount: s.tickCount ?? 0,          // BLOCKER 3 — sim-internal counter
+    unlockedPlantTypes: s.unlockedPlantTypes,
+    luraBeatProgress: s.luraBeatProgress,
+    offlineEvents: null,
+    settings: {
+      musicVolume: 0.7,
+      ambientVolume: 0.5,
+      sfxVolume: 0.8,
+      persistenceToastShown: s.persistenceToastShown,
+    },
+  };
+}
+
+/**
+ * Hydrate the store from a migrated V1Payload. Defensive defaults guard
+ * against partial / older payloads that survived migrate() but with
+ * missing-but-compatible fields.
+ */
+export function hydrateStoreFromPayload(s: AppState, payload: V1Payload): void {
+  s.applyTilesAndUnlocks(
+    payload.garden.tiles ?? new Array(16).fill(null),
+    payload.unlockedPlantTypes ?? [],
+  );
+  s.setHarvested(payload.harvestedFragmentIds ?? []);
+  s.setLuraBeatProgress(
+    payload.luraBeatProgress ?? { arrived: false, mid: false, farewell: false, pending: null },
+  );
+  s.setPersistenceToastShown(payload.settings?.persistenceToastShown ?? false);
+  // BLOCKER 3 — restore tickCount so STRY-10 narrative gating resumes.
+  s.setTickCount?.(payload.tickCount ?? 0);
+}
+```
+
+Update `src/save/index.ts` to re-export the two helpers:
+```typescript
+export { buildPayloadFromStore, hydrateStoreFromPayload } from './payload';
+```
+
 **Step 4 — `src/ui/settings/Settings.tsx`** (D-28 save-management only):

 ```typescript
 import { useState } from 'react';
 import { useAppStore } from '../../store';
-import { exportToBase64, importFromBase64, listSnapshots, snapshot, openSaveDB, wrap, unwrap, migrate, CURRENT_SCHEMA_VERSION, type V1Payload } from '../../save';
+import {
+  exportToBase64, importFromBase64, listSnapshots, snapshot, openSaveDB,
+  wrap, unwrap, migrate, CURRENT_SCHEMA_VERSION,
+  buildPayloadFromStore, hydrateStoreFromPayload,
+  type V1Payload,
+} from '../../save';
 import { uiStrings } from '../../content';

 /**
@@ -720,7 +805,9 @@ export function Settings({ open, onClose }: { open: boolean; onClose: () => void
      // Build a fresh save envelope from current store state.
      // (Plan 02-05 wires the same payload-build path in src/PhaserGame.tsx for save lifecycle hooks.)
      const state = useAppStore.getState();
-      const payload: V1Payload = buildPayloadFromStore(state);
+      // W2: shared two-arg signature. Settings has no injected clock, so we
+      // pass Date.now() directly — PhaserGame's saveSync passes clock.now().
+      const payload: V1Payload = buildPayloadFromStore(state, Date.now());
      const env = wrap(payload, CURRENT_SCHEMA_VERSION);
      const b64 = await exportToBase64(env);
      navigator.clipboard?.writeText(b64).catch(() => {});
@@ -740,7 +827,7 @@ export function Settings({ open, onClose }: { open: boolean; onClose: () => void
      const env = await importFromBase64(base64Buf);
      const raw = unwrap(env);                                  // verifies CRC
      const { payload } = migrate(raw, env.schemaVersion);      // upgrades schema
-      hydrateStoreFromPayload(payload as V1Payload);
+      hydrateStoreFromPayload(useAppStore.getState(), payload as V1Payload);
      setStatusLine('Restored.');
    } catch (e) {
      setStatusLine('That doesn\'t look like one of yours.');
@@ -759,7 +846,7 @@ export function Settings({ open, onClose }: { open: boolean; onClose: () => void
      // Phase 8 may add a list selector).
      const last = snaps[snaps.length - 1];
      const payload = unwrap(last.envelope);
-      hydrateStoreFromPayload(payload as V1Payload);
+      hydrateStoreFromPayload(useAppStore.getState(), payload as V1Payload);
      setStatusLine('Earlier garden restored.');
    } catch (e) {
      setStatusLine('Nothing earlier could be reached.');
@@ -799,39 +886,10 @@ const btnStyle: React.CSSProperties = {
  fontFamily: 'serif', textAlign: 'left', width: '100%',
 };

-// Helpers — these live here for now; can be extracted to src/save/ if reused.
-// BLOCKER 3 invariants:
-//   - lastTickAt is wall-clock ms (set here at export time via Date.now())
-//   - tickCount is the sim-internal monotonic counter (read from the store;
-//     simAdapter.applyTickCount writes it into the store every Garden.update
-//     so Settings.tsx can read it without coupling to the active scene)
-function buildPayloadFromStore(s: ReturnType<typeof useAppStore.getState>): V1Payload {
-  return {
-    garden: { tiles: s.tiles },
-    plants: [],
-    harvestedFragmentIds: s.harvestedFragmentIds,
-    lastTickAt: Date.now(),
-    tickCount: s.tickCount ?? 0,
-    unlockedPlantTypes: s.unlockedPlantTypes,
-    luraBeatProgress: s.luraBeatProgress,
-    offlineEvents: null,
-    settings: {
-      musicVolume: 0.7, ambientVolume: 0.5, sfxVolume: 0.8,
-      persistenceToastShown: s.persistenceToastShown,
-    },
-  };
-}
-
-function hydrateStoreFromPayload(payload: V1Payload): void {
-  const state = useAppStore.getState();
-  state.applyTilesAndUnlocks(payload.garden.tiles, payload.unlockedPlantTypes);
-  state.setHarvested(payload.harvestedFragmentIds);
-  state.setLuraBeatProgress(payload.luraBeatProgress);
-  state.setPersistenceToastShown(payload.settings.persistenceToastShown);
-  // BLOCKER 3 — restore the sim's tick counter so a returning player resumes
-  // where they left off rather than restarting at tick 0.
-  state.setTickCount?.(payload.tickCount ?? 0);
-}
+// W2: helpers live in `src/save/payload.ts` (Step 3.5) — both Settings.tsx
+// and PhaserGame.tsx import from there. The unified two-arg signature
+// (`(state, nowMs)` for build; `(state, payload)` for hydrate) eliminates
+// the arity divergence the verifier flagged.
 ```

 (Phase 2 minimum-viable Settings — clipboard read/paste UX is acceptable; Phase 8 polishes.)
@@ -921,6 +979,7 @@ import { installFirstInteractionGestureHandler } from './ui/begin';
 import {
  openSaveDB, requestPersistence, wrap, unwrap, migrate,
  CURRENT_SCHEMA_VERSION, registerSaveLifecycleHooks,
+  buildPayloadFromStore, hydrateStoreFromPayload,
  type V1Payload, type SavedRecord,
 } from './save';
 import {
@@ -982,7 +1041,7 @@ export const PhaserGame = forwardRef<IRefPhaserGame, IProps>(function PhaserGame
          const { payload: migratedPayload } = migrate(raw, env.schemaVersion);
          const payload = migratedPayload as V1Payload;
          appStore.getState().dismissBeginGate(); // D-22: skip Begin
-          hydrateStoreFromPayload(payload);
+          hydrateStoreFromPayload(appStore.getState(), payload);

          // Offline catch-up
          const off = computeOfflineCatchup(payload.lastTickAt, nowMs);
@@ -994,7 +1053,7 @@ export const PhaserGame = forwardRef<IRefPhaserGame, IProps>(function PhaserGame
              return simulateOneTick(s as never, (s as { lastTickAt: number }).lastTickAt + 1, [], { ...ctx, silent });
            }, true);
            const finalState = result.state as unknown as V1Payload;
-            hydrateStoreFromPayload(finalState);
+            hydrateStoreFromPayload(appStore.getState(), finalState);

            if (off.cappedMs >= ABSENCE_LETTER_THRESHOLD_MS) {
              appStore.getState().openLetter(finalState.offlineEvents);
@@ -1084,34 +1143,10 @@ function PersistenceToastWrapper(): JSX.Element | null {
  return <ToastModule.PersistenceToast show={true} />;
 }

-// Helpers (mirror Settings.tsx — deduplicate by extracting if reused).
-// BLOCKER 3 invariant: `lastTickAt` is WALL-CLOCK MILLISECONDS (the value of
-// clock.now() at save time). The sim never writes lastTickAt — only saveSync
-// in PhaserGame.tsx and the Settings export path do. computeOfflineCatchup
-// reads the same wall-clock convention.
-function buildPayloadFromStore(s: ReturnType<typeof appStore.getState>, lastTickAt: number): V1Payload {
-  return {
-    garden: { tiles: s.tiles },
-    plants: [],
-    harvestedFragmentIds: s.harvestedFragmentIds,
-    lastTickAt,                                  // wall-clock ms, owned by saveSync
-    tickCount: s.tickCount ?? 0,                 // BLOCKER 3 — sim-internal counter from store
-    unlockedPlantTypes: s.unlockedPlantTypes,
-    luraBeatProgress: s.luraBeatProgress,
-    offlineEvents: null,
-    settings: { musicVolume: 0.7, ambientVolume: 0.5, sfxVolume: 0.8, persistenceToastShown: s.persistenceToastShown },
-  };
-}
-function hydrateStoreFromPayload(payload: V1Payload): void {
-  const state = appStore.getState();
-  state.applyTilesAndUnlocks(payload.garden.tiles ?? new Array(16).fill(null), payload.unlockedPlantTypes ?? []);
-  state.setHarvested(payload.harvestedFragmentIds ?? []);
-  state.setLuraBeatProgress(payload.luraBeatProgress ?? { arrived: false, mid: false, farewell: false, pending: null });
-  state.setPersistenceToastShown(payload.settings?.persistenceToastShown ?? false);
-  // BLOCKER 3 — restore tickCount so the sim's STRY-10 narrative gating
-  // resumes from the correct counter on return.
-  state.setTickCount?.(payload.tickCount ?? 0);
-}
+// W2: helpers live in `src/save/payload.ts` (Step 3.5) — both Settings.tsx
+// and PhaserGame.tsx import from there. The unified two-arg signature
+// eliminates the arity divergence the verifier flagged. saveSync above
+// passes `clock.now()` for lastTickAt; Settings.tsx passes `Date.now()`.
 ```

 (NOTE: The `require` inside `PersistenceToastWrapper` will fail in ESM — replace with a top-level import. Reorganize the file so PersistenceToast imports cleanly. The above is a sketch; the executor finalizes the import order to avoid circular deps. Acceptable approach: move `PersistenceToast` rendering up to App.tsx and pass the `show` boolean via the store — see Step 11 below.)
@@ -1132,8 +1167,7 @@ private get clock(): Clock {
 **Step 11 — Update `src/App.tsx`** to mount Letter, Settings, PersistenceToast, and a SettingsIcon:

 ```typescript
-import { useState } from 'react';
-import { useRef } from 'react';
+import { useState, useEffect, useRef } from 'react';
 import { PhaserGame, type IRefPhaserGame } from './PhaserGame.tsx';
 import { BeginScreen } from './ui/begin';
 import { SeedPicker } from './ui/garden';
@@ -1230,8 +1264,18 @@ export default App;
    - `grep -q "migrate(" src/ui/settings/Settings.tsx` (BLOCKER 2: import path runs migrate)
    - `grep -E "unwrap\\(.*\\)|migrate\\(" src/ui/settings/Settings.tsx | head -5` (BLOCKER 2: both unwrap and migrate appear)
    - `grep -q "lastTickAt: clock.now()" src/PhaserGame.tsx` (BLOCKER 3: saveSync writes wall-clock ms via clock.now)
+    - `test -f src/save/payload.ts` (W2: shared payload helpers extracted)
+    - `grep -q "export function buildPayloadFromStore" src/save/payload.ts`
+    - `grep -q "export function hydrateStoreFromPayload" src/save/payload.ts`
+    - `grep -E "from '\\.\\./save'" src/ui/settings/Settings.tsx` (W2: Settings imports from save barrel — buildPayloadFromStore + hydrateStoreFromPayload)
+    - `grep -E "from '\\./save'" src/PhaserGame.tsx` (W2: PhaserGame imports from save barrel — buildPayloadFromStore + hydrateStoreFromPayload)
+    - `grep -q "buildPayloadFromStore" src/save/index.ts` (W2: barrel re-exports the shared helpers)
+    - `! grep -E "^function buildPayloadFromStore" src/ui/settings/Settings.tsx` (W2: no inline duplicate definition in Settings)
+    - `! grep -E "^function buildPayloadFromStore" src/PhaserGame.tsx` (W2: no inline duplicate definition in PhaserGame)
    - `! grep -q "lastTickAt: this\\." src/sim/garden/Garden.ts || true` (BLOCKER 3: sim does NOT write lastTickAt — Garden lives under src/game/scenes/, not src/sim/, but this guards against accidental sim-side writes)
    - `! grep -E "lastTickAt: this\\.(currentTick|tickCount)" src/game/scenes/Garden.ts` (BLOCKER 3: Garden scene does NOT overwrite lastTickAt with a tick counter — saveSync owns it)
+    - `! grep -E "lastTickAt:\\s*(this|currentTick)" src/sim/garden/commands.ts` (BLOCKER 3 — 02-05's silent-mode edits to commands.ts must not re-introduce sim-side lastTickAt writes)
+    - `grep -q "tickCount: next.tickCount" src/sim/garden/commands.ts` (BLOCKER 3 — tickCount increment from 02-02 must survive 02-05's silent-mode edits)
    - `grep -q "addEventListener('keydown'" src/App.tsx` (W2: D-29 keyboard shortcut wired)
    - `grep -q "tlg:toggle-journal" src/App.tsx` (W2: journal hotkey dispatches the toggle event)
    - `grep -q "lifecycleRef.current?.detach()" src/PhaserGame.tsx` (W5: lifecycle handle reaches outer effect cleanup)