fix(terminal-core): preserve cache fields dropped on every remount - #31
Merged
Conversation
TerminalEngine.mount()'s end-of-mount cache rebuild (the delete-then-set that reorders the entry for LRU eviction) copied ~25 TerminalCacheEntry fields into a fresh object literal but omitted four the type declares: agentColorLocked, lastSnapshot, lastDataAt, lastInputAt. Every remount (tab switch, pane collapse, etc.) silently reset them to undefined: - agentColorLocked gated the color-OSC guard, so a per-agent color lock broke on any remount and the running program's palette OSCs could start overwriting the assigned scheme. - lastDataAt/lastInputAt are the quiet-period gates for the heal/resync settle checks; undefined short-circuits every "&&" guard as already settled, risking a term.reset() mid-keystroke right after a remount. - lastSnapshot losing its value forces one guaranteed extra full repaint on the next mirror-mode resync. Fix: spread the existing cache entry first, before the explicit field list, so any field TerminalCacheEntry declares survives a remount by default instead of needing to be named here. The explicit keys still come after the spread and win where the rebuild intentionally overwrites/resets a field (terminal, fitAddon, disposables, kbState, win32State, etc.) — spreading first cannot clobber them. Added a cache.test.ts case that sets all four fields, remounts on the same cacheKey, and asserts they survive; confirmed it fails against the prior literal and passes with the spread.
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
2 participants
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.
What
TerminalEngine.mount()ends by rebuilding the terminal cache entry as a fresh object literal — a deliberate delete-then-set so the key moves to the end of the Map for LRU ordering. It copied ~25TerminalCacheEntryfields but omitted four the type declares.Every remount (tab switch, pane collapse, cross-window detach, reload) silently reset them:
agentColorLocked(cache.ts:23)lastDataAt/lastInputAt(cache.ts:51,:58)undefinedshort-circuits every&&guard as already settled, risking aterm.reset()mid-keystroke right after a remountlastSnapshot(cache.ts:50)Fix
Spread the existing entry first, before the explicit field list. This fixes the class of bug: any field added to
TerminalCacheEntryin future survives a remount by default instead of having to be remembered here. Explicit keys still come after the spread and win wherever the rebuild intentionally overwrites or resets a field.One line of production code.
Verification
cache.test.tscase that sets all four fields, remounts on the same cacheKey, and asserts they survive.Expected: true, Received: undefined; with it, it passes.Notes
Found during Canvas Mode Phase 0 design review. Pre-existing and unrelated to Canvas Mode — worth landing on its own.