fix(session): gate writer on hydration success + roll backups#1160
Open
brennanb2025 wants to merge 4 commits intomainfrom
Open
fix(session): gate writer on hydration success + roll backups#1160brennanb2025 wants to merge 4 commits intomainfrom
brennanb2025 wants to merge 4 commits intomainfrom
Conversation
Protects orca-data.json from two silent-data-loss failure modes: - Fix A: add a `hydrationSucceeded` flag. The debounced + shutdown session writers now short-circuit until hydration completed without throwing, so a crash in fetchRepos / fetchAllWorktrees / session.get / hydrateWorkspaceSession can no longer serialize an empty in-memory store over the user's persisted tabs. The App.tsx catch handler no longer calls hydrateWorkspaceSession with empty defaults; it leaves the in-memory state untouched and only flips workspaceSessionReady (via reconnectPersistedTerminals) so the UI can mount. - Fix B: rotate 5 rolling backups (.bak.0 – .bak.4) of orca-data.json before each overwrite, gated to at most one rotation per hour so the debounced tick doesn't churn disk. The current file is copied (not renamed) to .bak.0 so the primary file never temporarily disappears. Defers to follow-ups: broadening the worktree-id filter guard (fix C) and the fail-loud hydration error UI (fix D). Co-authored-by: Orca <help@stably.ai>
Co-authored-by: Orca <help@stably.ai>
Co-authored-by: Orca <help@stably.ai>
Addresses two gaps in the #1158 error path that mirrored the session writer bug the PR fixes: - Guard defaults-hydration in App.tsx catch with a uiHydrated flag so a late-stage startup failure can't overwrite already-loaded ui.json values (sidebar width, sort, filters) with hardcoded defaults. - Wrap reconnectPersistedTerminals() in the catch with a try/catch and force workspaceSessionReady=true via setState on failure, so a crash in the recovery step can't leave the user staring at a blank window. Also adds async-path coverage for the 1-hour backup rotation gate — existing tests only exercised the sync flush path. Co-authored-by: Orca <help@stably.ai>
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
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.
Problem
orca-data.json(the persisted workspace session: tabs, worktree assignments, layouts) can be silently wiped when startup hydration fails:src/renderer/src/App.tsx:308-340wrapped hydration in try/catch. On any throw (fetchRepos,fetchAllWorktrees,session.get(), orhydrateWorkspaceSessionitself) the catch calledhydrateWorkspaceSession({ tabsByWorktree: {}, ... }), zeroing the in-memory tab map. The debounced session writer then fired and overwrote disk with that empty state.src/renderer/src/store/slices/terminals.tsfilterstabsByWorktreeagainst the currently-known worktrees. A partialfetchAllWorktreesresult (slow SSH sync, etc.) drops tabs for unknown worktrees; the next writer tick persists the dropped state.No backups, no logs outside the renderer console — users only found out after they'd already lost work.
Solution
This PR implements fixes A + B from the issue; explicitly defers fix C (broaden filter) and fix D (fail-loud UI) to follow-ups.
Fix A — gate the session writer on hydration success
hydrationSucceededflag (startsfalse) to the terminal slice andsetHydrationSucceeded(true)action.App.tsxflips it totrueonly afterhydrateWorkspaceSession(session)returns without throwing.window.api.session.setand thebeforeunloadsetSync) now short-circuit via a sharedshouldPersistWorkspaceSession(state)helper inworkspace-session.ts. The helper requires bothworkspaceSessionReady && hydrationSucceeded.hydrateWorkspaceSessionwith empty defaults — the in-memory state is left untouched,hydrationSucceededstays false, and the writer stays a no-op for the rest of this process. The UI still mounts becausereconnectPersistedTerminals()continues to flipworkspaceSessionReady.[startup] Workspace session hydration failed; leaving disk state untouched: <msg>) so user reports are diagnosable.Fix B — rolling backups for
orca-data.json.bak.3 → .bak.4,.bak.2 → .bak.3, …,.bak.0 → .bak.1, andorca-data.json → .bak.0. Keeps 5 snapshots.BACKUP_MIN_INTERVAL_MS = 60 * 60 * 1000) so the 300ms debounced tick doesn't replace the ring with near-identical snapshots within minutes..bak.0soorca-data.jsonnever temporarily disappears between rotation and the new write — a crash in that window would otherwise strandload()on defaults.writeToDiskAsync) and sync (writeToDiskSync/flush()) paths rotate consistently.Out of scope (follow-ups)
Test plan
pnpm exec vitest run src/main/persistence.test.ts— 41 tests, all green. New coverage:orca-data.jsondoes not yet exist.bak.0before overwriting.bak.0….bak.4; no.bak.5)pnpm exec vitest run src/renderer/src/lib/workspace-session.test.ts— added theshouldPersistWorkspaceSessiongate matrix + an integration-style test that simulates the App.tsx subscribe pattern and asserts the writer is never invoked after a failed hydration, and fires once flags are both set.pnpm exec vitest run src/renderer/src/store/slices/terminals-hydration.test.ts— addedhydrationSucceededdefault-false, toggle, and "hydrateWorkspaceSession does not flip the flag on its own" tests.pnpm exec vitest run src/renderer/src/store/— 280 passed). Three pre-existing test failures (local-pty-provider.test.ts,pty.test.ts,terminal-attribution.test.ts) fail identically onmainand are unrelated.pnpm run lint— no new warnings.Manual verification to run on merge
orca-data.jsonto force a hydration error (e.g. replaceworkspaceSession.tabsByWorktreewith an invalid shape that throws in the renderer), relaunch, confirm on-disk file is not overwritten and.bak.0appears after the next successful session.~/Library/Application Support/orca/and confirm up to 5 rolling.bak.Nfiles exist.Made with Orca 🐋