From 9f0508496915ca6909e8c81d6afe7215107b1089 Mon Sep 17 00:00:00 2001 From: Aaron Coville Date: Mon, 24 Aug 2026 20:11:33 -0400 Subject: [PATCH 1/3] feat(T-071): restore desks and positions across a GL-loss scene rebuild MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Chromium evicts the oldest WebGL context when terminals push past the per-process cap, and the office floor's is always the oldest (built at startup). glRecovery rebuilds the scene through the ordinary mount path — which is the recovery working — but every Character was constructed cold, so agents filed in through the door again and desks were re-dealt in rebuild order. Add sceneRestore.ts: the effect cleanup snapshots each agent's seat index and tile into a ref (the effect re-runs, so a closure would not survive), and addCharacter seeds from it instead of the cold-start entry path. Agents reappear at their own desks. This does not stop the eviction. The two halves degrade independently, a theme change discards the snapshot outright, and seat 0 stays Michael's by rule. No Pixi, no map renderer, no React in the module — same headless seam glRecovery uses. The capture shares the cleanup that uninstalls the context-loss listener, so the uninstall runs first and unconditionally and the capture is total: a half-destroyed character is skipped, never thrown. A listener left on a dead scene is worse than the reset this fixes. --- docs/investigations/T-071-floor-reset.md | 96 ++++++++++++ src/renderer/src/scene/office/OfficeFloor.tsx | 41 ++++- src/renderer/src/scene/office/sceneRestore.ts | 125 ++++++++++++++++ test/office-scene-restore.test.cjs | 141 ++++++++++++++++++ 4 files changed, 401 insertions(+), 2 deletions(-) create mode 100644 docs/investigations/T-071-floor-reset.md create mode 100644 src/renderer/src/scene/office/sceneRestore.ts create mode 100644 test/office-scene-restore.test.cjs diff --git a/docs/investigations/T-071-floor-reset.md b/docs/investigations/T-071-floor-reset.md new file mode 100644 index 000000000..d3b321b4f --- /dev/null +++ b/docs/investigations/T-071-floor-reset.md @@ -0,0 +1,96 @@ +# T-071 — the office floor resets when you switch agents + +## What was happening + +Settled before this change, and unchanged by it. Chromium caps live WebGL +contexts per renderer process (~16) and **evicts the oldest** when a new one +pushes past. The office floor's Pixi context is created at app startup, so it is +always the oldest alive. Every terminal xterm opens takes another context via +`@xterm/addon-webgl`. Switching agents opens terminals, crosses the cap, and the +floor's context is the one that goes. + +Pixi reports nothing, so `glRecovery.ts` catches `webglcontextlost`, calls +`preventDefault()` (without it the context never comes back) and bumps +`glGeneration` — a dep of the scene effect at `OfficeFloor.tsx:1740`. The whole +scene is torn down and rebuilt through the ordinary mount path. **That rebuild +is the recovery working correctly**, and the workers never stop running; the +problem was purely visual. Because every `Character` was constructed cold, each +one reappeared at the office door and walked to a desk, and desks were re-dealt +in whatever order the agents happened to be rebuilt in — so an agent could come +back to somebody else's chair. + +## What changed + +`src/renderer/src/scene/office/sceneRestore.ts` (new) — a placement snapshot +that crosses the rebuild. + +- **Capture.** The effect cleanup records, per agent, its seat index and the + tile it was actually standing on, into a `useRef`. A ref because the effect + *re-runs* on a rebuild: anything in its closure dies with the scene. +- **Restore.** `addCharacter` seeds each agent from that snapshot instead of the + cold-start path — same desk, and `spawnTile` is where it already was rather + than the door. `applyState` then snaps it into its seat (`walkToDeskAndSit` + short-circuits when you are already on the desk tile) instead of walking it + there. + +The two halves degrade independently: a desk already claimed by an earlier agent +this mount still leaves the position worth restoring, and a remembered tile that +is not walkable on the current map still leaves the desk worth restoring. A +theme change discards the snapshot outright — seat indices and tiles are indices +into *one* map, and replaying them onto another would seat agents inside walls. + +Seat 0 is Michael's room by rule, so `agent.isGod` always goes through +`claimSeat` and is never restored. + +The module takes plain numbers and predicates — no Pixi, no map renderer, no +React — so the whole restore policy is testable against a `Map` literal with no +browser and no GPU. Same seam `glRecovery.ts` already used. + +## What this does NOT fix + +- **It does not stop the eviction.** The floor still loses its context whenever + enough terminals are open, and the scene is still fully rebuilt. This removes + the annoyance, not the cause. +- Everything except desk and position still resets: the coffee economy (clean-cup + stock, mugs parked on desks), break-room and errand state, in-flight message + envelopes, thought bubbles, and the task-board notes until the next 5s poll. +- Options deliberately *not* taken, because they are Aaron's call, not a worker's: + dropping `@xterm/addon-webgl` (a global change to every user's terminal), or + trying to make the floor's context not be the oldest. + +## Residual jank + +- **A ~1.5s blank floor.** `DEFAULT_REBUILD_DELAY_MS` deliberately waits out the + eviction storm — claiming a context straight back just loses it to the next + terminal in the same burst. Not touched. +- **A 0.5s fade-in.** `Character.show()` always fades from alpha 0 + (`Character.ts:502`). Agents now *materialise at their desks* rather than + walking in, but they still fade. Removing that means changing `show()` for + every caller, which is not surgical enough to be worth it here. +- An agent that was on a coffee break is restored at the café and then walks to + its desk — a short walk, not the walk-in. + +## Un-regressing the recovery + +The rebuild cap and the give-up-loudly path are the thing that must not break: +going blank forever is far worse than an ugly rebuild. They are untouched, and +`test/office-gl-recovery.test.cjs` (6 tests) still passes unchanged. + +The one new risk this change introduces is the capture itself: it runs inside +the same cleanup that uninstalls the context-loss listener, against characters +that are being destroyed. If it threw, it would take the cleanup down with it +and leave a listener on a dead scene — a scene that can resurrect itself, which +is strictly worse than the reset being fixed. So the uninstall runs **first and +unconditionally**, the capture is wrapped, and `captureSceneSnapshot` is total by +construction: a runtime with no character, a getter that throws, or a `NaN` +position is skipped rather than propagated. `test/office-scene-restore.test.cjs` +pins that. + +## Not verifiable here + +This was implemented in a headless worktree with no GPU and no running Electron +app, so the *live* behaviour — actual WebGL context counts, and how the restored +floor looks after a real eviction — was not measured. What would measure it: run +`npm run dev`, open enough agent terminals to cross the cap (~16 contexts; +DevTools logs `WARNING: Too many active WebGL contexts`), and confirm the floor +comes back with agents at their desks instead of filing in through the door. diff --git a/src/renderer/src/scene/office/OfficeFloor.tsx b/src/renderer/src/scene/office/OfficeFloor.tsx index 0c3422fd7..3056e866f 100644 --- a/src/renderer/src/scene/office/OfficeFloor.tsx +++ b/src/renderer/src/scene/office/OfficeFloor.tsx @@ -13,6 +13,7 @@ import { pickSoloLine, pickExchange, type BreakSpot } from './cafeteriaLines'; import { colors } from '@/design/tokens'; import { loadTheme, resolveThemeMap, themeTilesetUrls } from './themeLoader'; import { installContextLossRecovery } from './glRecovery'; +import { captureSceneSnapshot, restorePlacement, type SceneSnapshot } from './sceneRestore'; import type { Tile, Facing, ErrandKind, ErrandSpot } from './themeRegistry'; // The map, tileset atlases, desk-claim order, errand spots, coffee-economy @@ -160,6 +161,11 @@ export function OfficeFloor() { const hostRef = useRef(null); const appRef = useRef(null); const mountIdRef = useRef(0); + // Where everyone was standing when the last scene was torn down. A ref, not + // state: the effect below RE-RUNS on a rebuild, so anything held in its + // closure dies with the scene — only a ref crosses the gap. Seeded by the + // cleanup, read by addCharacter. See sceneRestore.ts. + const sceneSnapshotRef = useRef(null); // Bumped when the WebGL context is evicted; a dep of the effect below, so the // whole scene is torn down and rebuilt through the existing mount path rather // than through a second, parallel recovery routine. @@ -1375,7 +1381,27 @@ export function OfficeFloor() { const addCharacter = async (agent: Agent) => { const charName = theme.cast.byName[agent.character] ? agent.character : theme.cast.defaultCharacter; const member = theme.cast.byName[charName]; - const seatIndex = claimSeat(agent); + // A rebuild (a lost GL context, or a remount on the same theme) puts + // everyone back at the desk they had, standing where they were, instead + // of dealing out fresh desks and marching everybody in through the door + // again. Michael's room is not restorable — seat 0 is his by rule, and + // claimSeat is the one place that rule lives. + const restored = agent.isGod + ? null + : restorePlacement(sceneSnapshotRef.current, officeTheme, agent.id, { + seatCount: seatTiles.length, + isSeatFree: (i) => !seatClaims.has(i), + isWalkable: (x, y) => mapRenderer.isWalkable(x, y), + }); + // Claimed synchronously, BEFORE the await below: two agents mounting at + // once must not both see the same chair as free. + let seatIndex: number | null; + if (restored?.seatIndex != null) { + seatIndex = restored.seatIndex; + seatClaims.add(seatIndex); + } else { + seatIndex = claimSeat(agent); + } const seatTile: Tile = (seatIndex != null ? seatTiles[seatIndex] : undefined) ?? mapRenderer.getSpawnPoint('entrance') ?? { x: 2, y: 2 }; @@ -1393,7 +1419,10 @@ export function OfficeFloor() { frames, seatTile, seatDirection: facingForSeat(seatTile), - spawnTile: entrance, // walk in from the office door + // Walk in from the office door on a genuine first appearance; on a + // rebuild, reappear where this agent already was. applyState below + // then snaps it straight into its seat rather than walking it there. + spawnTile: restored?.spawnTile ?? entrance, glowColor: hexNum(colors.accent[agent.accent]) ?? hexToNumber(member.shirt), onClick: (id) => useStore.getState().select(id), }); @@ -1727,7 +1756,15 @@ export function OfficeFloor() { mountIdRef.current++; const a = appRef.current; if (a) { + // The recovery uninstall goes FIRST and unconditionally: a listener left + // on a dead scene can resurrect it, which is worse than any reset. (a as any).__glRecovery?.(); + // Then remember where everyone was, so the next mount can put them back. + try { + sceneSnapshotRef.current = captureSceneSnapshot(officeTheme, runtimes); + } catch (err) { + console.warn('[OfficeFloor] could not snapshot the floor:', err); + } (a as any).__resize?.disconnect?.(); try { (a as any).__unsub?.(); } catch { /* noop */ } try { (a as any).__offMessage?.(); } catch { /* noop */ } diff --git a/src/renderer/src/scene/office/sceneRestore.ts b/src/renderer/src/scene/office/sceneRestore.ts new file mode 100644 index 000000000..7bda030f4 --- /dev/null +++ b/src/renderer/src/scene/office/sceneRestore.ts @@ -0,0 +1,125 @@ +/** + * Rebuilding the scene without replaying the walk-in. + * + * glRecovery.ts explains WHY the scene is rebuilt: Chromium evicts the oldest + * WebGL context in the process, that is always the office floor's, and the only + * fix Pixi leaves us is to tear the scene down and build it again. The rebuild + * goes through the ordinary mount path, which is exactly what makes it safe — + * and also what makes it visible. Every Character is constructed cold, so each + * one appears at the office door and walks to a desk, and desks are re-claimed + * in whatever order the agents happen to be rebuilt in, so an agent can come + * back to somebody else's chair. Switching agents opens terminals, terminals + * take contexts, and the floor visibly resets. + * + * This module is the memory across that gap: a placement snapshot taken at + * teardown and read back when the characters are recreated. It is deliberately + * free of Pixi, the map renderer and React — it takes plain numbers and + * predicates — so the whole restore policy is testable against a Map literal + * with no browser and no GPU, the same seam glRecovery.ts uses. + * + * It does NOT stop the eviction and it is not meant to; it removes the + * annoyance, not the cause. See docs/investigations/T-071-floor-reset.md. + */ + +export interface SnapshotTile { x: number; y: number } + +export interface AgentPlacement { + /** Index into the mount's ordered seat list, or null if it never got a desk. */ + seatIndex: number | null; + /** The tile the avatar actually occupied when the scene was torn down. */ + tile: SnapshotTile; +} + +export interface SceneSnapshot { + /** The theme these placements were measured against. Seat indices and tiles + * are indices into ONE map; replaying them onto another map would seat + * agents inside walls, so a theme change discards the whole snapshot. */ + theme: string; + agents: Record; +} + +/** What the capture reads off a live runtime — structural, so the real + * Runtime/Character satisfy it without either module importing the other. */ +export interface PlacedRuntime { + seatIndex: number | null; + character: { getTilePosition(): SnapshotTile } | null | undefined; +} + +/** What the restore needs to know about the floor being built right now. */ +export interface FloorProbe { + /** How many seats this mount's ordered seat list has. */ + seatCount: number; + /** Has this seat not already been claimed by an earlier agent this mount? */ + isSeatFree(seatIndex: number): boolean; + /** Can an avatar stand here on the current map? */ + isWalkable(x: number, y: number): boolean; +} + +function finiteInt(n: unknown): n is number { + return typeof n === 'number' && Number.isFinite(n); +} + +/** + * Snapshot where everyone is, for the next mount to read. + * + * Runs inside the effect cleanup, alongside the context-loss uninstall, against + * characters that are about to be destroyed — so it must be total. A runtime + * that is half torn down (no character, a getter that throws, a NaN position) + * is SKIPPED rather than allowed to throw: taking the cleanup down with it + * would leave the loss listener installed on a dead scene, which is a worse bug + * than the reset this exists to fix. + */ +export function captureSceneSnapshot( + theme: string, + runtimes: Iterable<[string, PlacedRuntime]> +): SceneSnapshot { + const agents: Record = {}; + for (const [id, rt] of runtimes) { + try { + const tile = rt?.character?.getTilePosition?.(); + if (!tile || !finiteInt(tile.x) || !finiteInt(tile.y)) continue; + agents[id] = { + seatIndex: finiteInt(rt.seatIndex) ? rt.seatIndex : null, + // Copy: the scene keeps ticking while it is being torn down. + tile: { x: tile.x, y: tile.y } + }; + } catch { + // A character mid-destroy has nothing useful to say about its position. + continue; + } + } + return { theme, agents }; +} + +/** + * Where this agent should be seeded on the mount being built, or `null` to fall + * back to the cold path (claim the next free desk, walk in through the door). + * + * Each half degrades on its own: a desk taken by an earlier agent this mount + * still leaves the position worth restoring, and a remembered tile that is not + * walkable on this map still leaves the desk worth restoring. + */ +export function restorePlacement( + snapshot: SceneSnapshot | null | undefined, + theme: string, + agentId: string, + floor: FloorProbe +): { seatIndex: number | null; spawnTile: SnapshotTile | null } | null { + if (!snapshot || snapshot.theme !== theme || !snapshot.agents) return null; + const placement = snapshot.agents[agentId]; + if (!placement) return null; + + const seat = placement.seatIndex; + const seatIndex = + finiteInt(seat) && seat >= 0 && seat < floor.seatCount && floor.isSeatFree(seat) + ? seat + : null; + + const t = placement.tile; + const spawnTile = + t && finiteInt(t.x) && finiteInt(t.y) && floor.isWalkable(t.x, t.y) + ? { x: t.x, y: t.y } + : null; + + return { seatIndex, spawnTile }; +} diff --git a/test/office-scene-restore.test.cjs b/test/office-scene-restore.test.cjs new file mode 100644 index 000000000..61034010b --- /dev/null +++ b/test/office-scene-restore.test.cjs @@ -0,0 +1,141 @@ +/** + * The floor resets when you switch agents. + * + * Switching agents opens terminals; each xterm takes a WebGL context + * (@xterm/addon-webgl), Chromium crosses its per-process cap and evicts the + * OLDEST context — which is always the office floor's, built at app startup. + * glRecovery catches that and bumps `glGeneration`, a dep of the scene effect, + * so the whole scene is torn down and rebuilt. That rebuild is the recovery + * working; the visible cost is that every agent is constructed cold again and + * replays the walk-in from the office door, and seats are re-claimed in + * whatever order the agents happen to be rebuilt in. + * + * These tests pin the seam that removes the annoyance: the placement of every + * agent is snapshotted before teardown and seeded back on rebuild, so agents + * reappear AT their desks. It does NOT stop the eviction — see + * docs/investigations/T-071-floor-reset.md. + */ +const test = require('node:test'); +const assert = require('node:assert'); +const load = require('./load-ts.cjs'); + +const { captureSceneSnapshot, restorePlacement } = + load('src/renderer/src/scene/office/sceneRestore.ts'); + +/** A runtimes map stand-in — the two fields the capture actually reads. */ +function runtimes(entries) { + return new Map(entries.map(([id, seatIndex, x, y]) => [ + id, { seatIndex, character: { getTilePosition: () => ({ x, y }) } } + ])); +} +/** A floor with `seats` seats, all free, and everything walkable. */ +function floor(seats = 8, claimed = []) { + const taken = new Set(claimed); + return { + seatCount: seats, + isSeatFree: (i) => !taken.has(i), + isWalkable: () => true + }; +} + +test('a snapshot carries each agent’s desk and where it was standing', () => { + const snap = captureSceneSnapshot('office', runtimes([['a', 3, 10, 4], ['b', 5, 2, 9]])); + assert.equal(snap.theme, 'office'); + assert.deepEqual(snap.agents.a, { seatIndex: 3, tile: { x: 10, y: 4 } }); + assert.deepEqual(snap.agents.b, { seatIndex: 5, tile: { x: 2, y: 9 } }); +}); + +test('on rebuild an agent gets its OWN desk back, not the next free one', () => { + const snap = captureSceneSnapshot('office', runtimes([['a', 3, 10, 4]])); + const p = restorePlacement(snap, 'office', 'a', floor()); + assert.equal(p.seatIndex, 3, 'agent was reseated somewhere else — desks shuffled on rebuild'); +}); + +test('the agent reappears AT its desk instead of replaying the walk-in', () => { + const snap = captureSceneSnapshot('office', runtimes([['a', 3, 10, 4]])); + const p = restorePlacement(snap, 'office', 'a', floor()); + assert.deepEqual(p.spawnTile, { x: 10, y: 4 }, 'spawned somewhere other than where it stood'); +}); + +test('a genuine cold start still walks in through the door', () => { + assert.equal(restorePlacement(null, 'office', 'a', floor()), null, + 'no snapshot must mean no restore — the first-ever mount is meant to walk in'); +}); + +test('an agent that joined after the teardown walks in normally', () => { + const snap = captureSceneSnapshot('office', runtimes([['a', 3, 10, 4]])); + assert.equal(restorePlacement(snap, 'office', 'newcomer', floor()), null); +}); + +test('a theme change discards the snapshot — different map, meaningless tiles', () => { + const snap = captureSceneSnapshot('office', runtimes([['a', 3, 10, 4]])); + assert.equal(restorePlacement(snap, 'spaceship', 'a', floor()), null, + 'seat indices and tiles from another map would seat agents inside walls'); +}); + +test('a desk already claimed this mount is never double-claimed', () => { + const snap = captureSceneSnapshot('office', runtimes([['a', 3, 10, 4]])); + const p = restorePlacement(snap, 'office', 'a', floor(8, [3])); + assert.equal(p.seatIndex, null, 'two agents were put in the same chair'); + assert.deepEqual(p.spawnTile, { x: 10, y: 4 }, 'position is still worth restoring'); +}); + +test('a seat index the current floor no longer has falls back', () => { + const snap = captureSceneSnapshot('office', runtimes([['a', 9, 10, 4]])); + assert.equal(restorePlacement(snap, 'office', 'a', floor(8)).seatIndex, null); +}); + +test('an unwalkable remembered tile falls back to the door', () => { + const snap = captureSceneSnapshot('office', runtimes([['a', 3, 10, 4]])); + const p = restorePlacement(snap, 'office', 'a', { ...floor(), isWalkable: () => false }); + assert.equal(p.spawnTile, null, 'would have spawned an agent inside a wall'); + assert.equal(p.seatIndex, 3, 'the desk is still restorable'); +}); + +test('an agent that never got a desk restores its position only', () => { + const snap = captureSceneSnapshot('office', runtimes([['a', null, 10, 4]])); + const p = restorePlacement(snap, 'office', 'a', floor()); + assert.equal(p.seatIndex, null); + assert.deepEqual(p.spawnTile, { x: 10, y: 4 }); +}); + +test('the snapshot is a copy — the live scene keeps moving while it is torn down', () => { + // The tile object handed back is the one the snapshot must NOT alias: the + // ticker is still running during teardown, and a scene destroyed mid-walk + // would otherwise rewrite the placement we are about to restore from. + const pos = { x: 10, y: 4 }; + const live = { seatIndex: 3, character: { getTilePosition: () => pos } }; + const snap = captureSceneSnapshot('office', new Map([['a', live]])); + pos.x = 0; pos.y = 0; + live.seatIndex = 99; + assert.deepEqual(snap.agents.a, { seatIndex: 3, tile: { x: 10, y: 4 } }); +}); + +/** + * DO NOT REGRESS THE RECOVERY. The capture runs inside the same effect cleanup + * that uninstalls the context-loss listener. If a half-destroyed character + * throws while being read, an unguarded capture takes the whole cleanup down + * with it and the listener is never removed — a torn-down scene that can still + * resurrect itself, which is strictly worse than the reset we are fixing. + */ +test('a broken character cannot take the teardown down with it', () => { + const map = new Map([ + ['ok', { seatIndex: 1, character: { getTilePosition: () => ({ x: 5, y: 5 }) } }], + ['throws', { seatIndex: 2, character: { getTilePosition: () => { throw new Error('destroyed'); } } }], + ['gone', { seatIndex: 3, character: null }], + ['nan', { seatIndex: 4, character: { getTilePosition: () => ({ x: NaN, y: 2 }) } }] + ]); + let snap; + assert.doesNotThrow(() => { snap = captureSceneSnapshot('office', map); }); + assert.deepEqual(snap.agents.ok, { seatIndex: 1, tile: { x: 5, y: 5 } }); + for (const bad of ['throws', 'gone', 'nan']) { + assert.equal(snap.agents[bad], undefined, `${bad} should have been skipped, not snapshotted`); + assert.equal(restorePlacement(snap, 'office', bad, floor()), null); + } +}); + +test('a garbage snapshot restores nothing rather than seating agents in walls', () => { + for (const bad of [undefined, {}, { theme: 'office' }, { agents: {} }]) { + assert.equal(restorePlacement(bad, 'office', 'a', floor()), null); + } +}); From 5629d42f240a63f1fbf4712990fe4aa9f8058322 Mon Sep 17 00:00:00 2001 From: Aaron Coville Date: Mon, 24 Aug 2026 20:24:54 -0400 Subject: [PATCH 2/3] =?UTF-8?q?test(T-071):=20QA=20the=20GL-rebuild=20scen?= =?UTF-8?q?e=20restore=20=E2=80=94=20four=20risk=20paths=20probed?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The author could not exercise these from a headless worktree and neither can this branch exercise the GPU, so the probe goes as deep as sceneRestore.ts's Pixi-free seam reaches and says plainly where it stops. Adds 16 tests beside the author's 13. The capture is total for every per-entry shape a Character can be broken into while it is being destroyed — a throwing `character` GETTER, not just a throwing getTilePosition; a non-callable method; a primitive runtime; a key that throws when coerced. A floor of nothing but wreckage still yields a well-formed empty snapshot, which matters because the cleanup assigns the result to the ref unconditionally. The uninstall-before- capture ordering is source structure rather than behaviour, so it is pinned by a source-order assertion that reds when the lines are swapped or the try goes. Seats survive contention: three restore orders, a chair freed mid-mount, and a desk taken by a snapshot-less agent. Seat 0 holds — not because this module has a rule, but because claimSeat deals from 1 and the god never enters the restore path. Both facts are now asserted, so either one being dropped goes red. Nothing stale leaks: every teardown overwrites the ref, checked across an office/spaceship/office round trip. Both halves failing lands the agent on the ordinary cold-start path, not at 0,0. Verdict: correct but untested, now tested. No production code changed. Six findings are written up rather than fixed — the two worth a decision are that Michael still replays the walk-in on every rebuild (the god is excluded from the POSITION restore, which the seat-0 rule does not require) and that an eviction storm can clobber the snapshot with an empty one. glRecovery.ts re-verified at 0 changed lines. 623 of 624; the one failure is the known provider-config picker, fixed in PR #9. --- docs/investigations/T-071-qa.md | 166 ++++++++++++++++ test/office-scene-restore-qa.test.cjs | 267 ++++++++++++++++++++++++++ 2 files changed, 433 insertions(+) create mode 100644 docs/investigations/T-071-qa.md create mode 100644 test/office-scene-restore-qa.test.cjs diff --git a/docs/investigations/T-071-qa.md b/docs/investigations/T-071-qa.md new file mode 100644 index 000000000..ff1742662 --- /dev/null +++ b/docs/investigations/T-071-qa.md @@ -0,0 +1,166 @@ +# T-071 QA — the GL-rebuild scene restore, probed + +QA of `9f050849` on `agent/worker-t071-retry`. Adds `test/office-scene-restore-qa.test.cjs` +(16 tests) alongside the author's `test/office-scene-restore.test.cjs` (13). No production +code changed: **this branch is test-and-docs only.** + +Baseline is `main` @ `5300285f` = 594 of 595 (`provider-config.test.cjs` "model picker options +stay provider-specific", already fixed in PR #9). This branch: **623 of 624, same single +failure.** Typecheck 0 errors. `glRecovery.ts` re-verified untouched: 0 changed lines +`5300285f..9f050849`, so the rebuild cap and the give-up-loudly path are un-regressed. + +The verdict on the four paths is **correct but untested, now tested** — every one behaves as +the author claimed. What follows is what is now pinned, six findings that are real but were +not defects to be manufactured, and an honest list of what a headless worktree cannot settle. + +## 1. The capture is total — the highest-severity risk + +The capture shares the effect cleanup with the context-loss uninstall, and the author is right +that a listener stranded on a dead scene is worse than the reset. Both halves hold, and they +hold independently: + +**The ordering.** `(a as any).__glRecovery?.()` is the first statement in the cleanup and is +unconditional; the capture follows it, inside its own `try`. Nothing downstream of the +uninstall can strand a listener. This is source structure, not behaviour, so it is pinned by a +source-order assertion — it goes red when the two are swapped and when the `try` is removed +(mutations M11, M12 below). A runtime proof would need OfficeFloor mounted under a test +renderer with Pixi stubbed. + +**The totality.** Tried hard to make `captureSceneSnapshot` throw. Every per-entry shape is +skipped cleanly: a `character` **getter** that throws (not just a throwing `getTilePosition`), +a throwing `seatIndex` getter, a throwing `tile.x` getter, `getTilePosition` that is not +callable, a character with no such method, a runtime that is `null`/`undefined`/a primitive, a +null tile, a string coordinate, an `Infinity` coordinate, and a map key that throws when +coerced to a property name. A floor of nothing but wreckage still returns a well-formed +`{ theme, agents: {} }` — which matters, because the cleanup assigns the result to the ref +unconditionally. + +## 2. Seat collision + +No ordering of restores double-claims a desk: three agents with remembered seats land on +exactly their own desks in all three orders tried, with the claims set never exceeding three. +`isSeatFree` is consulted live rather than cached, so a chair freed part-way through a mount +is restorable by the agent that remembers it. A desk taken by a snapshot-less agent is not +stolen back — the rememberer gives up the desk and keeps the position, which is the documented +independent degradation. + +**Seat 0 holds, and it is worth knowing exactly why.** `restorePlacement` has no seat-0 rule of +its own — it will hand seat 0 to anyone who remembers it. The invariant survives on two +caller-side facts: `claimSeat` only ever deals seats from index 1, so no ordinary agent's +snapshot can contain seat 0 to begin with; and OfficeFloor keeps the god off the restore path +entirely. Both are now asserted, and both mutations (`i = 0`, dropping the `agent.isGod` guard) +go red. + +## 3. Snapshot lifetime + +Nothing stale ever leaks forward. Each mount builds its own `runtimes` Map and its cleanup +closes over that one, so every teardown overwrites the ref; a snapshot from rebuild N cannot +reach rebuild N+2. An `office → spaceship → office` round trip was checked explicitly: the only +snapshot alive when office is rebuilt describes the spaceship map, and the theme tag rejects it. +An agent archived between capture and rebuild leaves a dead entry that is never consulted +(`syncAgents` only builds agents still in the store) and holds no claim, so its chair is simply +free. An agent added between them has no entry and walks in cold. + +## 4. Degradation + +Both halves failing returns `{ seatIndex: null, spawnTile: null }` — an object, not `null`, and +the caller treats it identically to a cold start: `claimSeat(agent)` for the desk and +`restored?.spawnTile ?? entrance` for the position. So the both-fail case lands the agent at the +office door with a freshly dealt desk. Not `0,0`, not off-map. A restored spawn tile is never +one the floor called unwalkable, and it is a fresh object rather than an alias into the +snapshot, so the first agent to move cannot corrupt the placement later agents restore from. + +## Findings + +None of these is a reason to hold the branch. F1 and F2 are the two worth a decision. + +**F1 — Michael still replays the walk-in on every rebuild. (medium; scope, not correctness.)** +`agent.isGod ? null : restorePlacement(...)` excludes the god from *both* halves of the restore. +Keeping seat 0 Michael's only requires excluding the **seat**; the **position** is orthogonal +and safe to restore. So on every eviction the one agent most likely to be on screen marches in +through the door — the exact jank this task exists to remove. Roughly: + +```ts +const restored = restorePlacement(sceneSnapshotRef.current, officeTheme, agent.id, { ... }); +// seat 0 is Michael's by rule, and claimSeat is the one place that rule lives +const restoredSeat = agent.isGod ? null : restored?.seatIndex; +``` + +Left unapplied: it is a behaviour change, and the integrator should make it. + +**F2 — an eviction storm clobbers the snapshot with an empty one. (low-medium; narrow window.)** +Characters are built asynchronously (`await theme.cast.getFrames`). If a second eviction tears +the mount down before those resolve, the cleanup captures a `runtimes` Map that is still empty +and writes `{ theme, agents: {} }` over the good snapshot — so the *next* rebuild is a full cold +start and everyone walks in. This is a **loss**, not a leak; nothing stale survives. The window +is narrow (`DEFAULT_REBUILD_DELAY_MS` is 1500ms and frames are usually cached) but this is +precisely the storm the feature targets. Merging instead of replacing would close it, and is +safe because a placement is only ever consulted for an agent still in the store: + +```ts +const fresh = captureSceneSnapshot(officeTheme, runtimes); +sceneSnapshotRef.current = fresh.theme === sceneSnapshotRef.current?.theme + ? { theme: fresh.theme, agents: { ...sceneSnapshotRef.current.agents, ...fresh.agents } } + : fresh; +``` + +**F3 — a newcomer can be dealt a desk someone else remembers. (low; degrades gracefully.)** +`claimSeat` does not know which seats are spoken for, so a snapshot-less agent built before a +rememberer can take that rememberer's chair; the rememberer then falls back to a different desk +but does keep its position. Closing it means reserving remembered seats before any +`addCharacter` runs and releasing the unused ones afterwards — more than a one-liner, and the +current behaviour is defensible. + +**F4 — `finiteInt` does not check integrality. (low; unreachable today.)** A `seatIndex` of +`1.5` passes every guard; the caller would then `seatClaims.add(1.5)` and index `seatTiles[1.5]` +as `undefined`, silently making the entrance the agent's "desk". Unreachable because `claimSeat` +only ever yields integers. `Number.isInteger` for the seat index would make the name true. + +**F5 — the totality guarantee is per-entry, not per-iterable. (info; not a defect.)** The +`for (const [id, rt] of runtimes)` header sits outside the `try`, so a throwing iterator or a +non-destructurable entry would escape. Unreachable — the caller always passes a real +`Map` — and harmless even then, because the uninstall has already run and the +call site wraps the capture in `try/catch`. Deliberately **not** asserted with `assert.throws`: +such a test would go red if someone later widened the guard, i.e. it would forbid an +improvement. + +**F6 — an agent id of `__proto__` is dropped and repoints the prototype of `snapshot.agents`. +(info; verified benign.)** Every inherited-key lookup yields a value whose `seatIndex`/`tile` +fail the finite guards, so the caller falls through to `claimSeat` + entrance. Pinned by a test +so it stays benign. + +## What a headless worktree cannot settle + +- **That the rebuild looks right** — that an agent materialises at its desk rather than at the + door, through the deliberate ~1.5s blank gap and the 0.5s `Character.show()` fade. Needs a + running Electron with a GPU: open terminals until Chromium evicts the floor's context and + watch the floor. This is the only thing that closes the task's actual acceptance criterion. +- **The uninstall/capture ordering at runtime** rather than in source (guarded above, but by + reading the file). Needs OfficeFloor mounted under a test renderer with Pixi stubbed. +- **That a remembered tile is genuinely re-occupiable** — that `mapRenderer.isWalkable` agrees + with what a real `Character` reports from `getTilePosition()` mid-walk, and that two agents + cannot be restored onto the same tile (the restore checks walkability, not occupancy; two + agents standing on one tile is cosmetic, but it is unchecked). Needs the real map and Character. +- **Whether F2 and F3 bite in practice.** Needs the running app: force two evictions inside the + rebuild delay for F2, and spawn an agent during a rebuild for F3. + +## Break-it record + +Every test was proved to bite by mutating the thing it protects. 14 mutations, each reverted: + +| # | mutation | red | +|---|---|---| +| M1 | capture: drop the `try/catch` | totality (2 tests) | +| M2 | capture: drop the `!tile` guard | totality | +| M3 | capture: alias the tile instead of copying | author's "the snapshot is a copy" | +| M4 | restore: drop `isSeatFree` | 3 seat-collision tests | +| M5 | restore: drop the theme check | theme round trip | +| M6 | restore: drop `isWalkable` | both-fail, wall spawn | +| M7 | restore: alias the spawn tile | "the restored tile is a copy" | +| M8 | restore: both-fail lands at `0,0` | both-fail, Object-key | +| M9 | restore: move the seat-0 rule into the module | "no seat-0 rule of its own" | +| M10 | restore: a missing / empty placement returns a default | 3 lifetime tests | +| M11 | caller: capture moved BEFORE the uninstall | source-order guard | +| M12 | caller: capture no longer inside a `try` | source-order guard | +| M13 | caller: god no longer excluded from the restore | seat-0 contention | +| M14 | caller: `claimSeat` deals from seat 0 | seat-0 contention | diff --git a/test/office-scene-restore-qa.test.cjs b/test/office-scene-restore-qa.test.cjs new file mode 100644 index 000000000..8fe66f060 --- /dev/null +++ b/test/office-scene-restore-qa.test.cjs @@ -0,0 +1,267 @@ +/** + * T-071 QA — the paths the author could not exercise. + * + * sceneRestore.ts is deliberately Pixi-free, React-free and map-renderer-free so + * the whole restore POLICY can be driven from a Map literal. These tests use that + * seam to probe the four things a headless worktree can still reach: + * + * 1. the capture is total, because it shares a cleanup with the context-loss + * uninstall and a throw there would strand a listener on a dead scene; + * 2. seats cannot be double-claimed no matter what order agents restore in; + * 3. a snapshot cannot outlive the mount it describes; + * 4. when both halves fail the agent lands on the ordinary cold-start path, + * not at 0,0 and not off-map. + * + * What is NOT reachable from here — and what would settle it — is written up in + * docs/investigations/T-071-qa.md. Nothing here touches glRecovery.ts. + */ +const test = require('node:test'); +const assert = require('node:assert'); +const fs = require('node:fs'); +const path = require('node:path'); +const load = require('./load-ts.cjs'); + +const { captureSceneSnapshot, restorePlacement } = + load('src/renderer/src/scene/office/sceneRestore.ts'); + +/** A live runtime the capture can read cleanly. */ +const rt = (seatIndex, x, y) => ({ seatIndex, character: { getTilePosition: () => ({ x, y }) } }); +/** A floor whose claims are a live Set, so a claim made mid-restore is seen. */ +function floor(seats = 8, claims = new Set(), walkable = () => true) { + return { seatCount: seats, isSeatFree: (i) => !claims.has(i), isWalkable: walkable, claims }; +} +/** What OfficeFloor's addCharacter does with a restore result, seat half only: + * take the remembered desk if it was offered, else leave it to claimSeat. */ +function takeSeat(p, f) { + if (p?.seatIndex != null) { f.claims.add(p.seatIndex); return p.seatIndex; } + return null; +} + +// ─────────────────────────────────────────────────────────────────────────── +// (1) The capture is TOTAL — it must never take the teardown down with it. +// ─────────────────────────────────────────────────────────────────────────── + +/** + * Every way a Character can be broken while it is being destroyed. The author's + * suite covers a throwing getTilePosition, a null character and a NaN tile; these + * are the shapes it does not — a property ACCESS that throws rather than a call, + * a runtime that is not an object at all, and a key that throws on coercion. + */ +test('no shape of half-destroyed runtime can make the capture throw', () => { + const throwing = () => { throw new Error('mid-destroy'); }; + const garbage = [ + ['character getter throws', { seatIndex: 1, get character() { throwing(); } }], + ['seatIndex getter throws', { get seatIndex() { throwing(); }, character: { getTilePosition: () => ({ x: 1, y: 1 }) } }], + ['tile.x getter throws', { seatIndex: 1, character: { getTilePosition: () => ({ get x() { throwing(); }, y: 1 }) } }], + ['getTilePosition is not callable', { seatIndex: 1, character: { getTilePosition: 42 } }], + ['character has no getTilePosition', { seatIndex: 1, character: {} }], + ['runtime is null', null], + ['runtime is undefined', undefined], + ['runtime is a primitive', 5], + ['tile is null', { seatIndex: 1, character: { getTilePosition: () => null } }], + ['tile x is a string', { seatIndex: 1, character: { getTilePosition: () => ({ x: '3', y: 1 }) } }], + ['tile y is Infinity', { seatIndex: 1, character: { getTilePosition: () => ({ x: 1, y: Infinity }) } }] + ]; + for (const [what, broken] of garbage) { + let snap; + assert.doesNotThrow( + () => { snap = captureSceneSnapshot('office', new Map([['ok', rt(1, 5, 5)], ['bad', broken]])); }, + `${what}: the capture threw — the context-loss listener would be stranded on a dead scene` + ); + assert.equal(snap.agents.bad, undefined, `${what}: was snapshotted anyway`); + assert.deepEqual(snap.agents.ok, { seatIndex: 1, tile: { x: 5, y: 5 } }, + `${what}: took a healthy agent down with it`); + } +}); + +test('a key that throws when coerced is skipped, not fatal', () => { + const hostile = { toString() { throw new Error('id'); } }; + let snap; + assert.doesNotThrow(() => { + snap = captureSceneSnapshot('office', new Map([[hostile, rt(1, 2, 3)], ['ok', rt(2, 4, 5)]])); + }); + assert.deepEqual(snap.agents.ok, { seatIndex: 2, tile: { x: 4, y: 5 } }); +}); + +test('a floor of nothing but wreckage still yields a well-formed snapshot', () => { + // The cleanup assigns the result to a ref unconditionally. If a total loss + // could produce something other than an empty, correctly-themed snapshot, the + // NEXT mount would read it and seat agents from garbage. + const snap = captureSceneSnapshot('office', new Map([['a', null], ['b', 7], ['c', { character: null }]])); + assert.deepEqual(snap, { theme: 'office', agents: {} }); + assert.equal(restorePlacement(snap, 'office', 'a', floor()), null); +}); + +test('an agent id that collides with an Object key restores nothing, not a bogus desk', () => { + // `agents` is a bare object literal, so ids like `constructor` or `toString` + // resolve up the prototype chain. Whatever comes back must not read as a + // placement — an inherited function must not put anyone in a chair. + const snap = captureSceneSnapshot('office', new Map([['a', rt(3, 1, 2)]])); + for (const id of ['constructor', 'toString', 'hasOwnProperty', 'valueOf', '__proto__']) { + const p = restorePlacement(snap, 'office', id, floor()); + assert.ok(p === null || (p.seatIndex === null && p.spawnTile === null), + `id "${id}" produced a placement out of Object.prototype: ${JSON.stringify(p)}`); + } +}); + +/** + * SOURCE-ORDER GUARD, and the author's own highest-severity risk: the capture + * lives in the same effect cleanup as the context-loss uninstall. The capture + * being total (above) is the first defence and the cleanup's try/catch is the + * second, but the guarantee that actually holds is ORDER — the uninstall runs + * first and unconditionally, so no failure downstream of it can strand a + * listener. Nothing else in a headless worktree can see that ordering; this + * asserts it against the source and goes red the moment the lines are swapped. + */ +test('the context-loss uninstall runs BEFORE the snapshot capture', () => { + const src = fs.readFileSync( + path.join(__dirname, '..', 'src/renderer/src/scene/office/OfficeFloor.tsx'), 'utf8'); + const uninstall = src.indexOf('__glRecovery?.()'); + const capture = src.indexOf('captureSceneSnapshot(officeTheme'); + assert.ok(uninstall > 0, 'the context-loss uninstall is gone from the cleanup'); + assert.ok(capture > 0, 'the snapshot capture is gone from the cleanup'); + assert.ok(uninstall < capture, + 'the capture was moved ahead of the uninstall — a throw there now strands the ' + + 'loss listener on a dead scene, which is worse than the reset this fixes'); + assert.match(src.slice(capture - 260, capture), /try\s*\{[\s\S]*$/, + 'the capture is no longer inside a try — the cleanup after it would be skipped'); +}); + +// ─────────────────────────────────────────────────────────────────────────── +// (2) Seat collision — restoring remembered seats while others claim seats. +// ─────────────────────────────────────────────────────────────────────────── + +test('agents get their own desks back whatever order they restore in', () => { + const snap = captureSceneSnapshot('office', new Map([['a', rt(3, 1, 1)], ['b', rt(5, 2, 2)], ['c', rt(4, 3, 3)]])); + for (const order of [['a', 'b', 'c'], ['c', 'b', 'a'], ['b', 'a', 'c']]) { + const f = floor(); + const got = {}; + for (const id of order) got[id] = takeSeat(restorePlacement(snap, 'office', id, f), f); + assert.deepEqual(got, { a: 3, b: 5, c: 4 }, `order ${order.join('>')} shuffled the desks`); + assert.equal(f.claims.size, 3, 'a desk was claimed twice or not at all'); + } +}); + +test('a desk taken by an agent with no snapshot is not stolen back', () => { + // The newcomer went through claimSeat and got seat 3 — the desk `a` remembers. + // `a` must not sit on top of it; it gives up the desk and keeps its position. + const snap = captureSceneSnapshot('office', new Map([['a', rt(3, 10, 4)]])); + const f = floor(); + f.claims.add(3); + const p = restorePlacement(snap, 'office', 'a', f); + assert.equal(p.seatIndex, null, 'two agents were put in the same chair'); + assert.deepEqual(p.spawnTile, { x: 10, y: 4 }, 'the desk was lost, the position should not be'); +}); + +test('a seat freed mid-restore is restorable by the agent that remembers it', () => { + // isSeatFree is read at restore time, not cached at snapshot time: an agent + // removed part-way through a rebuild releases its chair and the rightful + // owner, restored later in the same mount, can still take it. + const snap = captureSceneSnapshot('office', new Map([['a', rt(3, 1, 1)]])); + const f = floor(); + f.claims.add(3); + assert.equal(restorePlacement(snap, 'office', 'a', f).seatIndex, null); + f.claims.delete(3); + assert.equal(restorePlacement(snap, 'office', 'a', f).seatIndex, 3, + 'the probe was consulted once and cached — a freed chair stayed unusable'); +}); + +test('restorePlacement has no seat-0 rule of its own — Michael is protected by the caller', () => { + // Documents WHERE the invariant lives. claimSeat is the only place that knows + // seat 0 is the god desk, and OfficeFloor keeps the god off this path entirely + // (`agent.isGod ? null : restorePlacement(...)`). This module will hand seat 0 + // to anyone who remembers it, so that caller-side guard is load-bearing. + const snap = captureSceneSnapshot('office', new Map([['someone', rt(0, 4, 4)]])); + assert.equal(restorePlacement(snap, 'office', 'someone', floor()).seatIndex, 0, + 'the seat-0 rule moved into this module — update the caller-side guard note'); +}); + +test('seat 0 survives contention because no ordinary agent can ever remember it', () => { + // The other half of the invariant: claimSeat only ever deals seats from 1, so + // a non-god snapshot cannot contain seat 0 to begin with. Assert the source + // rather than a restatement of it — this is the rule the restore relies on. + const src = fs.readFileSync( + path.join(__dirname, '..', 'src/renderer/src/scene/office/OfficeFloor.tsx'), 'utf8'); + assert.match(src, /for \(let i = 1; i < seatTiles\.length; i\+\+\)/, + 'claimSeat no longer starts at 1 — a non-god can now hold seat 0 and restore into it'); + assert.match(src, /agent\.isGod\s*\n?\s*\?\s*null\s*\n?\s*:\s*restorePlacement/, + 'the god is no longer excluded from the restore path — seat 0 is contestable'); +}); + +// ─────────────────────────────────────────────────────────────────────────── +// (3) Snapshot lifetime — it must not outlive the mount it describes. +// ─────────────────────────────────────────────────────────────────────────── + +test('an agent archived between capture and rebuild leaves its desk free', () => { + // Its entry stays in the snapshot but is never consulted (syncAgents only + // builds agents that are still in the store), and it holds no claim — so the + // chair is simply available to whoever claims it next. + const snap = captureSceneSnapshot('office', new Map([['killed', rt(3, 1, 1)], ['alive', rt(5, 2, 2)]])); + const f = floor(); + takeSeat(restorePlacement(snap, 'office', 'alive', f), f); + assert.ok(f.isSeatFree(3), 'a dead agent kept its chair reserved across the rebuild'); + assert.equal(restorePlacement(snap, 'office', 'newcomer', f), null, + 'an agent that joined after the teardown must walk in cold'); +}); + +test('a capture taken before the rebuild finished building restores nobody', () => { + // THE EVICTION STORM. Characters are built asynchronously (`await getFrames`), + // so a second eviction landing before those resolve tears the mount down with + // `runtimes` still empty. The capture that runs then is empty — correct in + // itself, and it is what the ref is left holding. + const mid = captureSceneSnapshot('office', new Map()); + assert.deepEqual(mid, { theme: 'office', agents: {} }); + assert.equal(restorePlacement(mid, 'office', 'a', floor()), null, + 'an empty snapshot must read as a cold start, never as a placement'); +}); + +test('a theme round trip never replays the first theme’s tiles', () => { + // office -> spaceship -> office. Each teardown overwrites the ref, so the only + // snapshot alive when office is rebuilt describes the SPACESHIP map, and its + // theme tag is what rejects it. Nothing from two mounts ago can leak through. + const first = captureSceneSnapshot('office', new Map([['a', rt(3, 10, 4)]])); + const second = captureSceneSnapshot('spaceship', new Map([['a', rt(7, 99, 99)]])); + assert.equal(restorePlacement(first, 'spaceship', 'a', floor(64)), null); + assert.equal(restorePlacement(second, 'office', 'a', floor(64)), null, + 'tiles measured on another map were replayed — agents would be seated in walls'); +}); + +// ─────────────────────────────────────────────────────────────────────────── +// (4) Degradation — the both-fail case must land on the cold-start path. +// ─────────────────────────────────────────────────────────────────────────── + +test('both halves failing degrades to the ordinary cold start, not to 0,0', () => { + const snap = captureSceneSnapshot('office', new Map([['a', rt(3, 10, 4)]])); + const p = restorePlacement(snap, 'office', 'a', floor(8, new Set([3]), () => false)); + assert.equal(p.seatIndex, null, 'seatIndex must be null so the caller falls through to claimSeat'); + assert.equal(p.spawnTile, null, + 'spawnTile must be null so the caller falls through to `?? entrance` — a 0,0 or ' + + 'an off-map tile here would drop the agent outside the office'); + assert.ok(!('x' in p), 'the result must not be mistakable for a tile'); +}); + +test('a restored spawn tile is never one the floor called unwalkable', () => { + const walls = new Set(['0,0', '10,4', '3,3']); + const snap = captureSceneSnapshot('office', + new Map([['a', rt(1, 10, 4)], ['b', rt(2, 3, 3)], ['c', rt(3, 6, 6)], ['d', rt(4, 0, 0)]])); + const f = floor(8, new Set(), (x, y) => !walls.has(`${x},${y}`)); + for (const id of ['a', 'b', 'c', 'd']) { + const p = restorePlacement(snap, 'office', id, f); + if (p?.spawnTile) { + assert.ok(f.isWalkable(p.spawnTile.x, p.spawnTile.y), + `${id} was spawned into a wall at ${p.spawnTile.x},${p.spawnTile.y}`); + } + } + assert.equal(restorePlacement(snap, 'office', 'c', f).spawnTile.x, 6, 'a walkable tile was dropped'); +}); + +test('the restored tile is a copy — one agent’s spawn cannot rewrite another’s', () => { + // The caller hands spawnTile to a Character, which owns and mutates position. + // If it aliased the snapshot, the first agent to move would corrupt the + // placement every later agent in the same mount is restored from. + const snap = captureSceneSnapshot('office', new Map([['a', rt(3, 10, 4)]])); + const first = restorePlacement(snap, 'office', 'a', floor()).spawnTile; + first.x = -1; first.y = -1; + assert.deepEqual(restorePlacement(snap, 'office', 'a', floor()).spawnTile, { x: 10, y: 4 }, + 'the restore handed out the snapshot’s own tile object'); +}); From 575705b55fbd55ad3275045de3612228bfe467e3 Mon Sep 17 00:00:00 2001 From: Aaron Coville Date: Mon, 24 Aug 2026 20:33:42 -0400 Subject: [PATCH 3/3] fix(T-071): restore the god's position, and merge snapshots instead of replacing MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two behaviour gaps QA found on 5629d42f and correctly left for the integrator. F1 — Michael still walked in. addCharacter read `agent.isGod ? null : restorePlacement(...)`, which excluded the god from BOTH halves of the restore. Only the SEAT is his by rule: seat 0 is the god desk and claimSeat is the one place that rule lives. The POSITION is orthogonal, so on every eviction the agent most likely to be on screen marched in through the door — the exact jank T-071 exists to remove, on the worst possible subject. Add seedPlacement(): it runs restorePlacement for everyone and nulls only the god's seatIndex, so his tile comes back and his desk is still dealt by claimSeat. The seat-0 rule is not moved or weakened; it now rests on claimSeat dealing from index 1 and on the god's seat never being restored, both asserted. F2 — an eviction storm could clobber a good snapshot with an empty one. Characters build asynchronously (await theme.cast.getFrames), so a second eviction inside the ~1.5s rebuild delay tears the mount down while `runtimes` is still empty; the cleanup then wrote `{ theme, agents: {} }` over the good snapshot and the next rebuild was a full cold start with everyone walking in. A loss, not a leak — and precisely the storm the feature targets. Add mergeSceneSnapshot(): fold the fresh capture onto the remembered one so an empty or partial capture keeps what it could not see, while live placements still win. Safe because a placement is only ever consulted for an agent still in the store. A THEME CHANGE still discards outright, including when the fresh capture is empty — merging across themes would seat agents inside walls. The capture still runs after the context-loss uninstall, inside its try. glRecovery.ts: 0 changed lines. QA's F3-F6 deliberately not applied. 18 tests in test/office-scene-restore-f1f2.test.cjs, each proved to bite by mutation (god's seat restored; god's position dropped; the isGod exclusion put back; claimSeat dealing from 0; the capture assigned rather than merged; the theme check dropped; empty-capture-wins across themes; stale placement winning; the merge mutating in place). The QA suite's source guard on the old `agent.isGod ? null :` wiring is retightened onto seedPlacement. --- docs/investigations/T-071-qa.md | 20 +- src/renderer/src/scene/office/OfficeFloor.tsx | 28 ++- src/renderer/src/scene/office/sceneRestore.ts | 52 ++++ test/office-scene-restore-f1f2.test.cjs | 226 ++++++++++++++++++ test/office-scene-restore-qa.test.cjs | 10 +- 5 files changed, 317 insertions(+), 19 deletions(-) create mode 100644 test/office-scene-restore-f1f2.test.cjs diff --git a/docs/investigations/T-071-qa.md b/docs/investigations/T-071-qa.md index ff1742662..b0364df54 100644 --- a/docs/investigations/T-071-qa.md +++ b/docs/investigations/T-071-qa.md @@ -33,7 +33,8 @@ callable, a character with no such method, a runtime that is `null`/`undefined`/ null tile, a string coordinate, an `Infinity` coordinate, and a map key that throws when coerced to a property name. A floor of nothing but wreckage still returns a well-formed `{ theme, agents: {} }` — which matters, because the cleanup assigns the result to the ref -unconditionally. +unconditionally. (Since F2 it is *merged* onto the ref, so a well-formed empty capture is now +absorbed rather than destructive — but it must still be well-formed.) ## 2. Seat collision @@ -47,9 +48,10 @@ independent degradation. **Seat 0 holds, and it is worth knowing exactly why.** `restorePlacement` has no seat-0 rule of its own — it will hand seat 0 to anyone who remembers it. The invariant survives on two caller-side facts: `claimSeat` only ever deals seats from index 1, so no ordinary agent's -snapshot can contain seat 0 to begin with; and OfficeFloor keeps the god off the restore path -entirely. Both are now asserted, and both mutations (`i = 0`, dropping the `agent.isGod` guard) -go red. +snapshot can contain seat 0 to begin with; and the god's remembered *seat* is never restored. +Both are now asserted, and both mutations (`i = 0`, restoring the god's seat) go red. Before the +F1 follow-up the second fact was the stronger "the god is off the restore path entirely"; F1 +narrowed it to the seat so the god's position could be restored, which is all seat 0 ever needed. ## 3. Snapshot lifetime @@ -86,7 +88,9 @@ const restored = restorePlacement(sceneSnapshotRef.current, officeTheme, agent.i const restoredSeat = agent.isGod ? null : restored?.seatIndex; ``` -Left unapplied: it is a behaviour change, and the integrator should make it. +**APPLIED** in the T-071 follow-up as `seedPlacement` in `sceneRestore.ts` — the god now goes +through the restore and only his *seat* is nulled. Covered by `test/office-scene-restore-f1f2.test.cjs`; +the seat-0 source guard in §2 above was retightened onto the new call site. **F2 — an eviction storm clobbers the snapshot with an empty one. (low-medium; narrow window.)** Characters are built asynchronously (`await theme.cast.getFrames`). If a second eviction tears @@ -104,6 +108,12 @@ sceneSnapshotRef.current = fresh.theme === sceneSnapshotRef.current?.theme : fresh; ``` +**APPLIED** in the T-071 follow-up as `mergeSceneSnapshot` in `sceneRestore.ts`, called from the +cleanup (still *after* the context-loss uninstall). A theme change discards outright, including +when the fresh capture is empty. + +F3-F6 remain deliberately unapplied: documented, unreachable today. + **F3 — a newcomer can be dealt a desk someone else remembers. (low; degrades gracefully.)** `claimSeat` does not know which seats are spoken for, so a snapshot-less agent built before a rememberer can take that rememberer's chair; the rememberer then falls back to a different desk diff --git a/src/renderer/src/scene/office/OfficeFloor.tsx b/src/renderer/src/scene/office/OfficeFloor.tsx index 3056e866f..3733a22a7 100644 --- a/src/renderer/src/scene/office/OfficeFloor.tsx +++ b/src/renderer/src/scene/office/OfficeFloor.tsx @@ -13,7 +13,7 @@ import { pickSoloLine, pickExchange, type BreakSpot } from './cafeteriaLines'; import { colors } from '@/design/tokens'; import { loadTheme, resolveThemeMap, themeTilesetUrls } from './themeLoader'; import { installContextLossRecovery } from './glRecovery'; -import { captureSceneSnapshot, restorePlacement, type SceneSnapshot } from './sceneRestore'; +import { captureSceneSnapshot, mergeSceneSnapshot, seedPlacement, type SceneSnapshot } from './sceneRestore'; import type { Tile, Facing, ErrandKind, ErrandSpot } from './themeRegistry'; // The map, tileset atlases, desk-claim order, errand spots, coffee-economy @@ -1384,15 +1384,14 @@ export function OfficeFloor() { // A rebuild (a lost GL context, or a remount on the same theme) puts // everyone back at the desk they had, standing where they were, instead // of dealing out fresh desks and marching everybody in through the door - // again. Michael's room is not restorable — seat 0 is his by rule, and - // claimSeat is the one place that rule lives. - const restored = agent.isGod - ? null - : restorePlacement(sceneSnapshotRef.current, officeTheme, agent.id, { - seatCount: seatTiles.length, - isSeatFree: (i) => !seatClaims.has(i), - isWalkable: (x, y) => mapRenderer.isWalkable(x, y), - }); + // again. Michael's ROOM is not restorable — seat 0 is his by rule and + // claimSeat is the one place that rule lives — but his POSITION is: + // seedPlacement drops the god's seat and keeps his tile. + const restored = seedPlacement(sceneSnapshotRef.current, officeTheme, agent, { + seatCount: seatTiles.length, + isSeatFree: (i) => !seatClaims.has(i), + isWalkable: (x, y) => mapRenderer.isWalkable(x, y), + }); // Claimed synchronously, BEFORE the await below: two agents mounting at // once must not both see the same chair as free. let seatIndex: number | null; @@ -1760,8 +1759,15 @@ export function OfficeFloor() { // on a dead scene can resurrect it, which is worse than any reset. (a as any).__glRecovery?.(); // Then remember where everyone was, so the next mount can put them back. + // MERGED, not assigned: an eviction storm can tear this mount down before + // its characters finished building, and that capture is empty — writing + // it over a good snapshot would cold-start the next rebuild. A theme + // change still discards outright; mergeSceneSnapshot handles both. try { - sceneSnapshotRef.current = captureSceneSnapshot(officeTheme, runtimes); + sceneSnapshotRef.current = mergeSceneSnapshot( + sceneSnapshotRef.current, + captureSceneSnapshot(officeTheme, runtimes) + ); } catch (err) { console.warn('[OfficeFloor] could not snapshot the floor:', err); } diff --git a/src/renderer/src/scene/office/sceneRestore.ts b/src/renderer/src/scene/office/sceneRestore.ts index 7bda030f4..eb6c346e3 100644 --- a/src/renderer/src/scene/office/sceneRestore.ts +++ b/src/renderer/src/scene/office/sceneRestore.ts @@ -123,3 +123,55 @@ export function restorePlacement( return { seatIndex, spawnTile }; } + +/** + * The placement to seed a character with on this mount, with the god rule + * applied — what `addCharacter` actually calls. + * + * The seat and the position are separate rules and only the SEAT is Michael's. + * Seat 0 is the god desk and `claimSeat` is the one place that rule lives, so + * the god's remembered seat is dropped here and re-dealt there rather than + * being restored (which would put a second copy of the rule on this path). His + * POSITION is orthogonal, and restoring it is the whole point of the feature on + * the agent most likely to be on screen: excluding the god from both halves + * meant Michael marched in through the office door on every single eviction. + */ +export function seedPlacement( + snapshot: SceneSnapshot | null | undefined, + theme: string, + agent: { id: string; isGod?: boolean }, + floor: FloorProbe +): { seatIndex: number | null; spawnTile: SnapshotTile | null } | null { + const restored = restorePlacement(snapshot, theme, agent.id, floor); + if (!restored) return null; + return agent.isGod ? { seatIndex: null, spawnTile: restored.spawnTile } : restored; +} + +/** + * Fold a fresh capture into the one already remembered. + * + * Characters are built asynchronously (`await theme.cast.getFrames`), so an + * eviction STORM — a second context loss inside the ~1.5s rebuild delay — can + * tear a mount down before any of its characters exist. Capturing that mount + * yields `{ theme, agents: {} }`, and assigning it straight onto the ref would + * write an empty snapshot over a good one: the next rebuild would then be a full + * cold start with everyone walking in through the door, which is exactly the + * jank T-071 exists to remove. It is a LOSS, not a leak — and the storm is + * precisely the case the feature targets, so it matters more than its narrow + * window suggests. + * + * Keeping the older entries is safe because a placement is only ever consulted + * for an agent still in the store: an agent that left leaves a dead entry that + * `syncAgents` never looks up and that holds no seat claim. + * + * A THEME CHANGE still discards outright. Seat indices and tiles index ONE map, + * so merging across themes would resurrect office placements onto the spaceship + * and seat agents inside walls — including when the new capture is empty. + */ +export function mergeSceneSnapshot( + previous: SceneSnapshot | null | undefined, + fresh: SceneSnapshot +): SceneSnapshot { + if (!previous || previous.theme !== fresh.theme || !previous.agents) return fresh; + return { theme: fresh.theme, agents: { ...previous.agents, ...fresh.agents } }; +} diff --git a/test/office-scene-restore-f1f2.test.cjs b/test/office-scene-restore-f1f2.test.cjs new file mode 100644 index 000000000..eeb629ca6 --- /dev/null +++ b/test/office-scene-restore-f1f2.test.cjs @@ -0,0 +1,226 @@ +/** + * T-071 follow-up — the two behaviour gaps QA found and correctly did not apply. + * + * F1: the god was excluded from BOTH halves of the restore, so Michael — the + * agent most likely to be on screen — marched in through the door on every + * eviction. Only his SEAT is protected (seat 0 is his by rule and claimSeat + * is the one place that rule lives); his POSITION is orthogonal. + * F2: an eviction storm could tear a mount down before its characters finished + * building (`await theme.cast.getFrames`), capturing a still-empty runtimes + * map and writing `{}` OVER a good snapshot. The next rebuild was then a + * full cold start — the exact jank T-071 exists to remove. + * + * Both are driven through sceneRestore.ts's Pixi-free seam, plus source-wiring + * guards for the one thing a headless worktree cannot execute: that OfficeFloor + * actually calls them. See docs/investigations/T-071-qa.md for QA's reasoning. + */ +const test = require('node:test'); +const assert = require('node:assert'); +const fs = require('node:fs'); +const path = require('node:path'); +const load = require('./load-ts.cjs'); + +const { captureSceneSnapshot, restorePlacement, seedPlacement, mergeSceneSnapshot } = + load('src/renderer/src/scene/office/sceneRestore.ts'); + +const SRC = path.join(__dirname, '..', 'src/renderer/src/scene/office/OfficeFloor.tsx'); +const officeFloorSource = () => fs.readFileSync(SRC, 'utf8'); + +/** A live runtime the capture can read cleanly. */ +const rt = (seatIndex, x, y) => ({ seatIndex, character: { getTilePosition: () => ({ x, y }) } }); +/** A floor whose claims are a live Set, so a claim made mid-restore is seen. */ +function floor(seats = 8, claims = new Set(), walkable = () => true) { + return { seatCount: seats, isSeatFree: (i) => !claims.has(i), isWalkable: walkable, claims }; +} + +// ─────────────────────────────────────────────────────────────────────────── +// F1 — the god keeps his position; only his seat is re-dealt. +// ─────────────────────────────────────────────────────────────────────────── + +test('the god reappears where he was standing instead of walking in again', () => { + // The whole point of the feature, on its most visible subject. Before this, + // OfficeFloor read `agent.isGod ? null : restorePlacement(...)`, which threw + // away the position along with the seat and sent Michael back to the door. + const snap = captureSceneSnapshot('office', new Map([['god-1', rt(0, 11, 7)]])); + const seeded = seedPlacement(snap, 'office', { id: 'god-1', isGod: true }, floor()); + assert.deepEqual(seeded.spawnTile, { x: 11, y: 7 }, + 'the god lost his remembered position — he walks in through the door on every eviction'); +}); + +test('the god does NOT get his remembered seat back — claimSeat re-deals it', () => { + // Seat 0 is Michael's by rule and claimSeat is the one place that rule lives. + // Restoring it here would put a second copy of that rule in the restore path. + const snap = captureSceneSnapshot('office', new Map([['god-1', rt(0, 11, 7)]])); + const seeded = seedPlacement(snap, 'office', { id: 'god-1', isGod: true }, floor()); + assert.equal(seeded.seatIndex, null, + 'the god restored a seat — the seat-0 rule now lives in two places'); +}); + +test('an ordinary agent still gets both halves of its placement back', () => { + // The god rule must not leak onto everybody else. + const snap = captureSceneSnapshot('office', new Map([['a', rt(3, 4, 9)]])); + const seeded = seedPlacement(snap, 'office', { id: 'a', isGod: false }, floor()); + assert.deepEqual(seeded, { seatIndex: 3, spawnTile: { x: 4, y: 9 } }); +}); + +test('an agent with no isGod flag is treated as an ordinary agent', () => { + // `isGod` is optional on Agent; absent must mean "not the god", not "unknown". + const snap = captureSceneSnapshot('office', new Map([['a', rt(3, 4, 9)]])); + assert.equal(seedPlacement(snap, 'office', { id: 'a' }, floor()).seatIndex, 3, + 'a missing isGod flag suppressed an ordinary agent\'s desk'); +}); + +test('a god with no snapshot still falls through to the cold start', () => { + // A genuine first appearance, and the both-halves-failed case: the caller + // must see the same `null` it saw before, so it walks him in from the door. + assert.equal(seedPlacement(null, 'office', { id: 'god-1', isGod: true }, floor()), null); + const snap = captureSceneSnapshot('office', new Map([['someone-else', rt(3, 1, 1)]])); + assert.equal(seedPlacement(snap, 'office', { id: 'god-1', isGod: true }, floor()), null); +}); + +test('a theme change discards the god restore too', () => { + const snap = captureSceneSnapshot('spaceship', new Map([['god-1', rt(0, 11, 7)]])); + assert.equal(seedPlacement(snap, 'office', { id: 'god-1', isGod: true }, floor()), null, + 'a spaceship tile was replayed onto the office map'); +}); + +test('the god does not consume a desk claim on the restore path', () => { + // OfficeFloor only calls seatClaims.add() when the restore OFFERS a seat. + // If the god were ever offered seat 0 here it would be claimed twice — once + // by this path and once by claimSeat. + const snap = captureSceneSnapshot('office', new Map([['god-1', rt(0, 11, 7)]])); + const f = floor(); + const seeded = seedPlacement(snap, 'office', { id: 'god-1', isGod: true }, f); + if (seeded.seatIndex != null) f.claims.add(seeded.seatIndex); + assert.equal(f.claims.size, 0, 'the god claimed a desk before claimSeat ran'); +}); + +// ── the seat-0 rule, un-regressed ────────────────────────────────────────── + +test('no ordinary agent can land on seat 0, before or after the god restore', () => { + // The invariant survives on two caller-side facts, both still true: + // (a) claimSeat only ever deals seats from index 1 to a non-god, so no + // ordinary agent's snapshot can contain seat 0 to begin with; + // (b) the god's seat is never restored, so seat 0 is only ever dealt by + // claimSeat, to the god. + const src = officeFloorSource(); + assert.match(src, /for \(let i = 1; i < seatTiles\.length; i\+\+\)/, + 'claimSeat no longer starts at 1 — a non-god can now hold seat 0 and restore into it'); + assert.match(src, /if \(agent\.isGod\) \{ seatClaims\.add\(GOD_SEAT\); return GOD_SEAT; \}/, + 'claimSeat no longer reserves seat 0 for the god'); + // And behaviourally: a non-god that somehow DID remember seat 0 is not the + // thing that protects it — this module hands it over, which is exactly why + // (a) above is load-bearing and asserted against the source. + const rogue = captureSceneSnapshot('office', new Map([['a', rt(0, 1, 1)]])); + assert.equal(restorePlacement(rogue, 'office', 'a', floor()).seatIndex, 0, + 'the seat-0 rule moved into sceneRestore — update the caller-side guard notes'); +}); + +test('OfficeFloor seeds characters through seedPlacement, god included', () => { + // The wiring a headless worktree cannot execute. Goes red if someone puts the + // `agent.isGod ? null :` exclusion back, or drops the god rule entirely by + // calling restorePlacement directly from addCharacter. + const src = officeFloorSource(); + assert.match(src, /const restored = seedPlacement\(\s*sceneSnapshotRef\.current,\s*officeTheme,\s*agent,/, + 'addCharacter no longer seeds through seedPlacement — the god rule is bypassed'); + assert.doesNotMatch(src, /agent\.isGod\s*\n?\s*\?\s*null\s*\n?\s*:\s*(seedPlacement|restorePlacement)/, + 'the god is excluded from the restore again — Michael walks in on every eviction'); + assert.match(src, /spawnTile: restored\?\.spawnTile \?\? entrance/, + 'the restored position is no longer used to seed the character'); +}); + +// ─────────────────────────────────────────────────────────────────────────── +// F2 — an eviction storm must not clobber a good snapshot with an empty one. +// ─────────────────────────────────────────────────────────────────────────── + +test('a capture of a floor that never finished building keeps the old placements', () => { + // The storm: eviction 1 tears down a populated floor and captures it. The + // rebuild starts, `await theme.cast.getFrames` has not resolved, so `runtimes` + // is still empty when eviction 2 tears THAT mount down. Assigning its capture + // would write `{}` over the good snapshot and the next rebuild would be a full + // cold start — everyone walks in, which is the jank T-071 exists to remove. + const good = captureSceneSnapshot('office', new Map([['a', rt(3, 4, 9)], ['b', rt(5, 1, 2)]])); + const empty = captureSceneSnapshot('office', new Map()); + const merged = mergeSceneSnapshot(good, empty); + assert.deepEqual(merged.agents, good.agents, + 'an empty capture destroyed the previous placements — the next rebuild is a cold start'); + assert.equal(merged.theme, 'office'); +}); + +test('a partial capture keeps the placements it could not see', () => { + // The same storm, one frame later: some characters resolved, some did not. + const good = captureSceneSnapshot('office', new Map([['a', rt(3, 4, 9)], ['b', rt(5, 1, 2)]])); + const partial = captureSceneSnapshot('office', new Map([['a', rt(3, 7, 7)]])); + const merged = mergeSceneSnapshot(good, partial); + assert.deepEqual(merged.agents.b, { seatIndex: 5, tile: { x: 1, y: 2 } }, + 'the agent that had not finished building lost its placement'); +}); + +test('a fresh placement wins over the one it remembers', () => { + // Merging is a floor, not a freeze: an agent that actually moved must be + // remembered where it ended up, not where it was two rebuilds ago. + const older = captureSceneSnapshot('office', new Map([['a', rt(3, 4, 9)]])); + const newer = captureSceneSnapshot('office', new Map([['a', rt(6, 12, 2)]])); + assert.deepEqual(mergeSceneSnapshot(older, newer).agents.a, { seatIndex: 6, tile: { x: 12, y: 2 } }, + 'a stale placement outranked the live one'); +}); + +test('a theme change DISCARDS the previous placements rather than merging them', () => { + // Seat indices and tiles index ONE map. Merging across themes would resurrect + // office placements onto the spaceship and seat agents inside walls — the one + // thing the theme tag exists to prevent. + const office = captureSceneSnapshot('office', new Map([['a', rt(3, 4, 9)]])); + const ship = captureSceneSnapshot('spaceship', new Map([['b', rt(1, 0, 0)]])); + const merged = mergeSceneSnapshot(office, ship); + assert.equal(merged.theme, 'spaceship'); + assert.deepEqual(Object.keys(merged.agents), ['b'], + 'an office placement survived into the spaceship snapshot'); +}); + +test('a theme change to an EMPTY capture still discards — it does not resurrect', () => { + // The nastiest combination of F2 and the theme rule: a storm during a theme + // switch. The empty spaceship capture must win anyway; keeping the office + // placements "because the new one is empty" would replay them onto the new map. + const office = captureSceneSnapshot('office', new Map([['a', rt(3, 4, 9)]])); + const merged = mergeSceneSnapshot(office, captureSceneSnapshot('spaceship', new Map())); + assert.deepEqual(merged, { theme: 'spaceship', agents: {} }, + 'office placements survived a theme change because the new capture was empty'); +}); + +test('the first capture of the session merges into nothing', () => { + const fresh = captureSceneSnapshot('office', new Map([['a', rt(3, 4, 9)]])); + assert.deepEqual(mergeSceneSnapshot(null, fresh), fresh); + assert.deepEqual(mergeSceneSnapshot(undefined, fresh), fresh); +}); + +test('the merge does not mutate the snapshot it is merging into', () => { + // The ref is swapped, not edited: a placement handed out to a restore that is + // still running must not change under it. + const good = captureSceneSnapshot('office', new Map([['a', rt(3, 4, 9)]])); + const merged = mergeSceneSnapshot(good, captureSceneSnapshot('office', new Map([['b', rt(5, 1, 2)]]))); + assert.deepEqual(Object.keys(good.agents), ['a'], 'the previous snapshot was edited in place'); + assert.notEqual(merged.agents, good.agents); +}); + +test('a merged placement is still restorable end to end', () => { + // The point of keeping it: the agent the storm could not see still comes back + // to its own desk on the rebuild after. + const good = captureSceneSnapshot('office', new Map([['a', rt(3, 4, 9)]])); + const merged = mergeSceneSnapshot(good, captureSceneSnapshot('office', new Map())); + assert.deepEqual(seedPlacement(merged, 'office', { id: 'a' }, floor()), + { seatIndex: 3, spawnTile: { x: 4, y: 9 } }, + 'the placement survived the merge but could not be restored from'); +}); + +test('OfficeFloor merges the capture into the ref instead of replacing it', () => { + // The wiring a headless worktree cannot execute. Goes red if the cleanup goes + // back to `sceneSnapshotRef.current = captureSceneSnapshot(...)`. + const src = officeFloorSource(); + assert.match(src, /sceneSnapshotRef\.current = mergeSceneSnapshot\(\s*sceneSnapshotRef\.current,\s*captureSceneSnapshot\(officeTheme, runtimes\)\s*,?\s*\)/, + 'the cleanup assigns the capture straight onto the ref — an eviction storm ' + + 'can still write an empty snapshot over a good one'); + // F2 must not have moved the capture ahead of the context-loss uninstall. + const uninstall = src.indexOf('__glRecovery?.()'); + assert.ok(uninstall > 0 && uninstall < src.indexOf('= mergeSceneSnapshot('), + 'the merge was placed ahead of the uninstall — a throw there strands the loss listener'); +}); diff --git a/test/office-scene-restore-qa.test.cjs b/test/office-scene-restore-qa.test.cjs index 8fe66f060..31320fad7 100644 --- a/test/office-scene-restore-qa.test.cjs +++ b/test/office-scene-restore-qa.test.cjs @@ -169,7 +169,8 @@ test('a seat freed mid-restore is restorable by the agent that remembers it', () test('restorePlacement has no seat-0 rule of its own — Michael is protected by the caller', () => { // Documents WHERE the invariant lives. claimSeat is the only place that knows // seat 0 is the god desk, and OfficeFloor keeps the god off this path entirely - // (`agent.isGod ? null : restorePlacement(...)`). This module will hand seat 0 + // (seedPlacement nulls the god's seat before the caller sees it). This module + // will hand seat 0 // to anyone who remembers it, so that caller-side guard is load-bearing. const snap = captureSceneSnapshot('office', new Map([['someone', rt(0, 4, 4)]])); assert.equal(restorePlacement(snap, 'office', 'someone', floor()).seatIndex, 0, @@ -184,8 +185,11 @@ test('seat 0 survives contention because no ordinary agent can ever remember it' path.join(__dirname, '..', 'src/renderer/src/scene/office/OfficeFloor.tsx'), 'utf8'); assert.match(src, /for \(let i = 1; i < seatTiles\.length; i\+\+\)/, 'claimSeat no longer starts at 1 — a non-god can now hold seat 0 and restore into it'); - assert.match(src, /agent\.isGod\s*\n?\s*\?\s*null\s*\n?\s*:\s*restorePlacement/, - 'the god is no longer excluded from the restore path — seat 0 is contestable'); + // T-071 follow-up (F1): the god is no longer excluded from the restore + // WHOLESALE — he keeps his position — but seedPlacement still drops his seat, + // so seat 0 is only ever dealt by claimSeat. See office-scene-restore-f1f2. + assert.match(src, /const restored = seedPlacement\(/, + 'the god rule left seedPlacement — a restored god seat would make seat 0 contestable'); }); // ───────────────────────────────────────────────────────────────────────────