Repository navigation
fix(T-071): agents return to their desks across a GL-loss rebuild instead of walking back in - #11
Closed
aaroncoville wants to merge 3 commits into
Closed
aaroncoville wants to merge 3 commits into
aaroncoville wants to merge 3 commits into
Conversation
added 3 commits
August 24, 2026 20:11
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.
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.
…f replacing Two behaviour gaps QA found on 5629d42 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.
Owner
Author
|
Closed per Aaron's decision on the restore-point review: behaviour fully preserved (verified by patch-id) in the branch that became PR #15, minus material not wanted upstream. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Your report: "Occasionally when switching agents the whole floor resets and all the workers have to walk in through the door and get back to a desk... the crash of the visual floor is annoying."
You were right that it is purely visual — the workers never stop. Agents now materialise at their own desks across a rebuild instead of replaying the walk-in.
Why it happens (the code documents this in its own header)
Chromium caps 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 permanently the oldest alive. Every terminal xterm opens takes another context via
@xterm/addon-webgl. Switching agents opens terminals, crosses the cap, and the floor is evicted first.Pixi detects nothing — no exception, no rejected promise. Without recovery the office would go blank until restart.
installContextLossRecoverycatches the loss and rebuilds the whole scene. The walk-in was the recovery working correctly; the jarring visual was its price.What changed
New pure
sceneRestore.ts(no Pixi, no React, no mapRenderer — headlessly testable, the same seamglRecovery.tswas built for) snapshots each agent's seat and tile before teardown.OfficeFloorholds it in a ref, not state — the scene effect re-runs, so closures die — and seeds the rebuild from it.Pipeline
toby-t071bdwight-t071qakelly-t071fixF1 — the god still walked in
agent.isGod ? null : restorePlacement(...)excluded Michael from both halves of the restore. But the rule being protected is only the seat — seat 0 is his, owned byclaimSeat. Position is orthogonal. So on every eviction the agent most likely to be on screen marched in through the door: the exact jank this feature exists to remove, on the worst possible subject.New
seedPlacement()nulls onlyseatIndexfor the god and restores his tile.claimSeatuntouched.F2 — an eviction storm could wipe a good snapshot
Characters build asynchronously. A second eviction landing before they resolve captured an empty set and wrote it over the good snapshot — making the next rebuild a full cold start where everyone walks in. A loss, not a leak. Narrow window, but that storm is precisely what this feature targets, so probability is the wrong lens.
mergeSceneSnapshot()folds a fresh capture into the remembered one; a theme change still discards outright.Verification (reproduced independently, not relayed)
restorePlacementalways returns nullglRecovery.tschanged lines642 tests, 641 pass. The one failure is pre-existing on
mainand is fixed by PR #9.npm run typecheck— 0 errors.One correction I owe the record: I briefed a hire that the suite was "607 of 607 green". That number belonged to PR #9's branch, not
main. The hire git-archived HEAD into a temp dir, ran the suite, and told me I was wrong. It was right.What this does NOT fix — worth knowing before you look
DEFAULT_REBUILD_DELAY_MSwaits out the eviction storm) and a 0.5s fade fromCharacter.show(). They no longer walk — they still appear.The root-cause option — dropping
@xterm/addon-webglso terminals stop taking contexts, since xterm already falls back to its DOM renderer — was deliberately not taken. It changes terminal rendering for every user and needs your call, with real numbers first.