Restore every window, not just one: per-window sessions + a window registry - #41
Merged
Conversation
Plan 018 Task 1. Rust must know how many windows to create before any webview exists, and localStorage is unreachable from Rust — so the window LIST and geometry live here, while each window's tab/pane payload stays in the renderer keyed by the windowId recorded here. Every failure mode (missing, corrupt, future version) loads as empty: the cost of an empty registry is one window, the cost of an error path is an app that does not start.
Plan 018 Task 2. WindowTracker owns the registry, the live label -> windowId map and the geometry write debounce as one unit, so 'the map and the registry agree' has a single owner. Register on create (persisted immediately — a new window must survive a crash), forget on Destroyed, and update geometry/focus on Moved/Resized/Focused. Moved and Resized fire per frame during a drag, so the disk write is debounced to 500ms while memory tracks every event. id_for_label deliberately returns None rather than falling back to slot 0: windows silently sharing one storage key is the defect this exists to fix.
Plan 018 Task 3. setup() binds slot 0 to the already-created 'main' window and builds slots 1..N with the same configuration open_new_window uses (GPU browser args included — a divergence there is the gpu_preference footgun). Labels are NOT reused from the registry. A saved detach-* label would send the restored window down the detach boot path looking for a payload no process still holds (detach_payloads is in-memory, empty every launch). The stable windowId carries the session, so the label is free to be normalised. Geometry is only reapplied if it is still reachable: monitors get unplugged between sessions, and a window restored onto coordinates that no longer exist is one the user cannot click, drag or close. get_window_session_id errors on an unknown label rather than defaulting to slot 0 — a silent fallback would put two windows on one storage key.
Mirrors the `:pro:alt` script pair: CARGO_TARGET_DIR=target-pro-nightly with TERMFLOW_PROFILE=nightly baked in, so the build defaults to the nightly profile without --profile on every launch. Nothing in Rust special-cases a profile name -- everything mutable is derived from ProfileIdentity -- so the name alone buys config.nightly.json, history.nightly.db, layout.nightly.json, recordings.nightly, its own instance lock, pty-host pipe and host record, API/MCP port claim and TermFlow-nightly.log, plus a window/tray title marked (nightly). It is not the primary instance, so it never claims the machine-wide fabric keypair or peer listener. Both .gitignore files gain /target-pro-nightly/: cargo resolves CARGO_TARGET_DIR relative to src-tauri, which is where the tree actually lands, while the root entry covers a run from the repo root. The guard test that reads the baked profile names back out of package.json was swept into b55493e by a concurrent bulk stage.
Plan 018 Task 4. StateManager's STATE_KEY now carries a window dimension resolved from the backend registry, so each window persists its own tabs instead of blindly overwriting one shared key. Resolved in index.tsx immediately after initProfileScope and before the bridge or App, for the same reason that one must be: App registers unload, visibility and interval saves the moment it mounts, so a late id lands the first saves on slot 0's key. Only the auto-saved SESSION is per-window. Saved layouts and the API token stay per-instance — a named layout is a user library, and fragmenting it per window would hide layouts from the window that did not save them. Slot 0 keeps the original bare key, so an existing single-window session loads byte-identically. The window dimension uses '#' rather than ':' so a window id can never derive another profile's slot-0 key.
Plan 018 Task 5. ?newWindow=1 used to return before StateManager.restoreState. With per-window keys that is both unnecessary and harmful: a fresh window has a fresh id, so restoreState finds nothing and the existing decision table opens a default tab on its own — while the early return left a window that SAVED (the mount effect registers the save hooks for every window regardless) but never restored, so its default tab could replace a session it never read. The ?path= feature is preserved by feeding it into pendingOpenPath. Adds a source tripwire over the boot wiring: both hops it guards fail silently (every window quietly back on slot 0's key), and neither App nor bootstrapApp can be mounted under the root Jest config. Verified by mutation — reintroducing the early return turns it red.
Plan 018 Task 6. Detached windows already get a registry id (Task 2) and the boot loop normalises labels away from detach-* (Task 3), so a torn-off window comes back as an ordinary restorable window and reconstructDetachedWindow's no-payload path falls through to the normal restore. Closes a real race found while wiring this: the id was published AFTER builder.build(), but a webview starts loading the moment it is built and resolves its session id as its first action. The losing side of that race falls back to slot 0 and silently merges the new window into the main window's session — the exact defect this feature removes. Ids are now reserved before build, in all three creation paths (new window, detach, boot restore).
Plan 018 Task 7. The restore sweep prunes history.db down to the terminals in the layout it just restored. That was correct while one window restored; with per-window sessions N windows boot concurrently, so whichever restored first deleted every OTHER window's scrollback before those windows read their own session. StateManager's own comment already required this union. The keep-set now unions every session key belonging to THIS profile — sibling profiles are excluded, since sweeping against another instance's terminals would delete live shells' history. Fails safe: a session blob that will not parse is a window whose terminals cannot be named, so the sweep is skipped entirely rather than run on a partial union. Deleted scrollback is not recoverable.
…ws' sessions Plan 018 Tasks 8 and 9. Task 8 — AppHandle::exit tears the process down without firing CloseRequested for any window, so no renderer gets its beforeunload. Survivable while one shared key held everything; with per-window sessions each window owns data only IT can write, so an unflushed window loses its tabs outright. Quit now asks every window to persist and waits for acks, with a hard 1.5s ceiling — a quit that hangs is worse than one stale window. A second Quit while a flush is in flight exits immediately. The WAIT is split into wait_for_acks so the timeout branch is testable; a happy-path test would never reach it. Task 9 — every closed window used to leave a localStorage blob forever, the same unbounded growth pruneCwds exists to prevent, one level up. Slot 0 now sweeps session keys against the backend registry before the history prune, so a dead window's terminals fall out of the union and its scrollback is actually reclaimed. An empty live-list is treated as 'could not ask' and sweeps nothing, since the alternative deletes every session there is. NOTE: the Rust half is cargo-check clean as of the previous commit, but the new flush_tests module is UNVERIFIED — D: has 1.09GB free and the build fails with os error 112. Renderer half is fully green (883 tests).
Canvas Mode is unmerged on develop and touches 8 of the same files, so the per-window session work is developed on top of it rather than colliding at PR time. Two conflicts, both adjacent-line rather than semantic: lib.rs WindowEvent::Destroyed — both branches add cleanup to the same block. Kept both: canvas node geometry is a per-window renderer projection that must be dropped, while the window registry entry is precisely the thing that must survive to recreate this window's peers. StateManager imports — canvas still imported the profile-only stateKey. Dropped it and kept sessionStateKey: reinstating the profile-only key would put every window back on one blob, which is the defect this branch removes. Canvas's own persistence (sanitizeCanvasState) is untouched and now rides the per-window key, which is right — Canvas Mode is a tab, and tabs are per-window. Also switched the boot tripwire onto canvas's utils/readSource: its assertions span whitespace and would silently stop matching on a CRLF checkout, which the e2e job hits or misses depending on which runner is free. Verified: 130 suites / 2000 tests green, renderer typecheck clean, and all four window-creation sites accounted for (canvas added none). The Rust half is UNVERIFIED — D: has 1.09GB free and cargo fails with os error 112.
Plan 018 Task 10. wait_exit_then_apply_updates was handed Vec::new() as restart_args, so an in-app update relaunched --profile work as the DEFAULT identity: a different config file, a different window registry, and an empty renderer storage scope. The user reads that as the update having eaten their session. Derived from the resolved identity rather than echoed from std::env::args, which would also replay one-shot flags (--path, --headless) that must not survive a restart. Elevation is deliberately not expressed: it is a property of the process token and is re-derived on launch, and the AUTO-selected 'elevated' name comes with it. Passing --profile elevated to a process that came back at medium integrity would mint a third identity (rel.elevated) owning nobody's data — so that one case is left to be re-derived, while an explicitly named profile always carries.
The comment still described the pre-018 behaviour. ?newWindow=1 now means the window has no session saved under its id, not that restore is skipped — and it does save under that id, so it is restored like any other window.
Canvas Mode landed on develop via PR #40, so this branch takes it from there rather than carrying its own copy.
6 tasks
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.
The bug
Quit TermFlow with several windows open and only one came back, carrying one window's tabs.
Root-caused to three independent layers, each sufficient on its own — which is why fixing any one of them in isolation changed nothing observable:
L1 — one key, N writers, blind overwrite.
stateKey()scoped only by profile + elevation; every window of an instance shares a WebView2 origin and therefore onelocalStorage.saveState()read only its own store and did a baresetItem— no read, no merge, no per-writer version. Every window armed the same writers: theuseEffectatApp.tsxregisters beforeunload + a 30s autosave + visibilitychange synchronously, while theisDetachWindow()/?newWindow=1early-returns live inside the asyncinitializeApp()— so they skipped RESTORE, never SAVE. The worst case was deterministic, not racy: closing a secondary window wrote that window's tabs immediately before destroy, so closing a 1-tab window overwrote a 10-tab window's session.L2 — one reader.
restoreStatehad a single call site, reachable only by falling through both early returns — the bootmainwindow.L3 — nothing to restore into. One window declared in
tauri.conf.json, no window-list persistence anywhere insrc-tauri/, and detach payloads in an in-memoryDashMapthat starts empty every launch.The fix
Rust owns the durable window list; the renderer owns each window's payload. That split is forced: Rust cannot read
localStorage, andbeforeunloadcannot await aninvoke.window_registry.rs— profile-scoped registry of windows + geometry, written atomically, debounced against per-frame drag events. Every failure mode (missing, corrupt, future version) loads as empty: the cost of an empty registry is one window, the cost of an error path is an app that does not start.setup()recreates N windows, binding slot 0 to the existingmainand building the rest with the same configurationopen_new_windowuses. Geometry is only reapplied if still reachable — monitors get unplugged, and a window restored off-screen is one the user cannot click.windowScope.tsresolves awindowIdinindex.tsxbefore the bridge orApploads, for the same reasoninitProfileScopemust. Slot 0 keeps the bare key, so an existing single-window session loads byte-identically.?newWindow=1no longer skips restore — a fresh window has a fresh id, finds nothing, and the existing decision table opens a default tab. Skipping restore while still saving is what let a fresh window's default tab replace a session it never read.Collateral fixed, because each would make this look broken
AppHandle::exit(0)(tray Quit,quit_app, last-window confirm) fires noCloseRequested, so no renderer ever saved. Now a flush handshake with a hard 1.5s ceiling — a quit that hangs is worse than one stale window.localStorageblob forever. Slot 0 now sweeps against the backend registry.updater.rspassed emptyrestart_args, so an in-app update relaunched--profile workas default — different config, different registry, empty storage scope. The user reads that as the update eating their session.Two bugs found in this PR's own first draft
builder.build(). A webview starts loading the moment it is built and resolves its id as its first act; losing that race falls back to slot 0 — silently merging the new window into main's session, i.e. the original bug, reintroduced by the fix. Ids are now reserved before build in all three creation paths.:, so a default-profile window with idworkwould deriveauto-terminal-state:work— the work profile's slot-0 key. Now#: unreachable by construction rather than by coincidence.Verification
405 Rust + 2000 renderer tests green;
tsc --noEmitclean; clippy clean (0 errors, no warnings in new files).The boot-wiring tripwire was mutation-tested — reintroducing the
?newWindow=1early return turns it red — and reads source throughutils/readSource, since its assertions span whitespace and would silently stop matching on a CRLF checkout.Manually confirmed on a
nightlyside-by-side instance: the registry is created, slot 0 registers asw0/main, and geometry tracks the live window.Plan:
termflow-fabric/docs/plan/018-multi-window-session-restore-implementation.md