fix: prevent UI data loss and blank window on startup hydration failure

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>

00-review-context.md

@@ -1,63 +0,0 @@
-# Review Context
-
-## Branch Info
-
-- Base: origin/main (8e44541c4c53cfdf6bb149dc7dc4524eff24405a)
-- Current: brennanb2025/fix-session-hydration-data-loss
-
-## Changed Files Summary
-
-- M src/main/persistence.test.ts
-- M src/main/persistence.ts
-- M src/renderer/src/App.tsx
-- M src/renderer/src/lib/workspace-session.test.ts
-- M src/renderer/src/lib/workspace-session.ts
-- M src/renderer/src/store/slices/terminals-hydration.test.ts
-- M src/renderer/src/store/slices/terminals.ts
-
-## Changed Line Ranges (PR Scope)
-
-<!-- In scope: issues on these lines OR caused by these changes. Out of scope: unrelated pre-existing issues -->
-
-| File                                                      | Changed Lines                                          |
-| --------------------------------------------------------- | ------------------------------------------------------ |
-| src/main/persistence.test.ts                              | 5, 585-712                                             |
-| src/main/persistence.ts                                   | 5-14, 57-68, 88-93, 99-154, 269-277, 308-316           |
-| src/renderer/src/App.tsx                                  | 31-34, 98, 188-192, 283-296, 316-320, 355, 389, 399-403|
-| src/renderer/src/lib/workspace-session.test.ts            | 1-2, 143-243                                           |
-| src/renderer/src/lib/workspace-session.ts                 | 11-27                                                  |
-| src/renderer/src/store/slices/terminals-hydration.test.ts | 269-309                                                |
-| src/renderer/src/store/slices/terminals.ts                | 98-106, 230-233                                        |
-
-## Review Standards Reference
-
-- Follow /review-code standards
-- Focus on: correctness, security, performance, maintainability
-- Priority levels: Critical > High > Medium > Low
-
-## File Categories
-
-### Electron/Main
-- src/main/persistence.ts
-- src/main/persistence.test.ts
-
-### Frontend/UI
-- src/renderer/src/App.tsx
-- src/renderer/src/lib/workspace-session.ts
-- src/renderer/src/lib/workspace-session.test.ts
-- src/renderer/src/store/slices/terminals.ts
-- src/renderer/src/store/slices/terminals-hydration.test.ts
-
-## Skipped Issues (Do Not Re-validate)
-
-<!-- Issues validated but deemed not worth fixing. Do not re-validate these in future iterations. -->
-<!-- Format: [file:line-range] | [severity] | [reason skipped] | [issue summary] -->
-<!-- NOTE: Skips should be RARE - only purely cosmetic issues with no functional impact -->
-
-[None yet]
-
-## Iteration State
-
-Current iteration: 1
-Last completed phase: Setup
-Files fixed this iteration: []

src/main/persistence.test.ts

@@ -709,5 +709,89 @@ describe('Store', () => {
         vi.useRealTimers()
       }
     })
+
+    // Why (issue #1158): scheduleSave → writeToDiskAsync → rotateBackupsAsync
+    // is the hot path during normal use (every mutation debounces through it);
+    // flush() is only used at shutdown. The 1-hour gate must hold on the async
+    // path too, or routine edits would burn the 5-slot ring in minutes.
+    it('does not rotate on the async write path within the 1-hour window', async () => {
+      vi.useFakeTimers()
+      try {
+        // Seed disk so the first async write has something to snapshot.
+        writeDataFile({
+          schemaVersion: 1,
+          repos: [makeRepo({ id: 'seed' })],
+          worktreeMeta: {},
+          settings: {},
+          ui: {},
+          githubCache: { pr: {}, issue: {} },
+          workspaceSession: {}
+        })
+
+        const store = await createStore()
+
+        // First async write (debounced) — rotates because .bak.0 doesn't exist
+        // yet, so shouldRotateBackups returns true on statSync ENOENT.
+        store.addRepo(makeRepo({ id: 'first-async' }))
+        vi.advanceTimersByTime(300)
+        await store.waitForPendingWrite()
+
+        const bak0AfterFirst = readBackup(0)
+        expect(bak0AfterFirst.repos.map((r) => r.id)).toEqual(['seed'])
+
+        // Advance only 5 minutes, then trigger another async write. The 1-hour
+        // gate must block rotation: .bak.0 should still reflect the seed state,
+        // NOT the "first-async" state from the previous write.
+        vi.setSystemTime(new Date(Date.now() + 5 * 60 * 1000))
+        store.addRepo(makeRepo({ id: 'within-hour-async', path: '/within-async' }))
+        vi.advanceTimersByTime(300)
+        await store.waitForPendingWrite()
+
+        const bak0AfterSecond = readBackup(0)
+        expect(bak0AfterSecond.repos.map((r) => r.id)).toEqual(['seed'])
+      } finally {
+        vi.useRealTimers()
+      }
+    })
+
+    it('rotates on the async write path after the 1-hour window elapses', async () => {
+      vi.useFakeTimers()
+      try {
+        // Seed disk so the first async write has something to snapshot.
+        writeDataFile({
+          schemaVersion: 1,
+          repos: [makeRepo({ id: 'seed' })],
+          worktreeMeta: {},
+          settings: {},
+          ui: {},
+          githubCache: { pr: {}, issue: {} },
+          workspaceSession: {}
+        })
+
+        const store = await createStore()
+
+        // First async write: snapshots "seed" into .bak.0.
+        store.addRepo(makeRepo({ id: 'first-async' }))
+        vi.advanceTimersByTime(300)
+        await store.waitForPendingWrite()
+
+        expect(readBackup(0).repos.map((r) => r.id)).toEqual(['seed'])
+
+        // Jump past BACKUP_MIN_INTERVAL_MS (1h). The next async write should
+        // rotate: the previous on-disk state (seed + first-async) lands in
+        // .bak.0, and the prior .bak.0 ("seed" alone) shifts to .bak.1.
+        vi.setSystemTime(new Date(Date.now() + 61 * 60 * 1000))
+        store.addRepo(makeRepo({ id: 'after-hour-async', path: '/after-async' }))
+        vi.advanceTimersByTime(300)
+        await store.waitForPendingWrite()
+
+        const bak0After = readBackup(0)
+        expect(bak0After.repos.map((r) => r.id).sort()).toEqual(['first-async', 'seed'])
+        expect(existsSync(backupFile(1))).toBe(true)
+        expect(readBackup(1).repos.map((r) => r.id)).toEqual(['seed'])
+      } finally {
+        vi.useRealTimers()
+      }
+    })
   })
 })

src/renderer/src/App.tsx

@@ -167,6 +167,13 @@ function App(): React.JSX.Element {
     // without this, the first (unmounted) pass would keep spawning PTYs.
     const abortController = new AbortController()
 
+    // Why (issue #1158): track whether the real persisted UI hydration
+    // succeeded so the catch path below doesn't overwrite freshly-loaded
+    // ui.json values with hardcoded defaults. The debounced UI writer is
+    // gated only on persistedUIReady, so a default-hydration after a real
+    // one would serialize those defaults back to disk — silently erasing
+    // sidebar width, sort order, filters, etc.
+    let uiHydrated = false
     void (async () => {
       try {
         await actions.fetchRepos()
@@ -181,6 +188,7 @@ function App(): React.JSX.Element {
         await actions.fetchSettings()
         if (!cancelled) {
           actions.hydratePersistedUI(persistedUI)
+          uiHydrated = true
           actions.hydrateWorkspaceSession(session)
           actions.hydrateTabsSession(session)
           actions.hydrateEditorSession(session)
@@ -298,30 +306,53 @@ function App(): React.JSX.Element {
           error
         )
         if (!cancelled) {
-          actions.hydratePersistedUI({
-            lastActiveRepoId: null,
-            lastActiveWorktreeId: null,
-            sidebarWidth: 280,
-            rightSidebarWidth: 350,
-            groupBy: 'none',
-            sortBy: 'name',
-            showActiveOnly: false,
-            filterRepoIds: [],
-            collapsedGroups: [],
-            uiZoomLevel: 0,
-            editorFontZoomLevel: 0,
-            worktreeCardProperties: [...DEFAULT_WORKTREE_CARD_PROPERTIES],
-            statusBarItems: [...DEFAULT_STATUS_BAR_ITEMS],
-            statusBarVisible: true,
-            dismissedUpdateVersion: null,
-            lastUpdateCheckAt: null
-          })
+          // Why (issue #1158): only hydrate UI with defaults if the real
+          // hydration never ran. If fetchRepos/fetchAllWorktrees/session.get/
+          // fetchSettings threw before line 183, persistedUIReady is still
+          // false and the UI cannot mount until we hydrate something. But if
+          // the real hydrate already ran and a later step threw, overwriting
+          // with defaults would flow through the debounced UI writer and
+          // clobber ui.json (sidebar width, sort, filters, etc.).
+          if (!uiHydrated) {
+            actions.hydratePersistedUI({
+              lastActiveRepoId: null,
+              lastActiveWorktreeId: null,
+              sidebarWidth: 280,
+              rightSidebarWidth: 350,
+              groupBy: 'none',
+              sortBy: 'name',
+              showActiveOnly: false,
+              filterRepoIds: [],
+              collapsedGroups: [],
+              uiZoomLevel: 0,
+              editorFontZoomLevel: 0,
+              worktreeCardProperties: [...DEFAULT_WORKTREE_CARD_PROPERTIES],
+              statusBarItems: [...DEFAULT_STATUS_BAR_ITEMS],
+              statusBarVisible: true,
+              dismissedUpdateVersion: null,
+              lastUpdateCheckAt: null
+            })
+          }
           // Why: reconnectPersistedTerminals flips workspaceSessionReady so the
           // UI mounts; auto-tab-creation becomes unblocked. hydrationSucceeded
           // is intentionally NOT set — the session writer must stay a no-op
           // until the user gets a clean restart, so we don't overwrite the
           // on-disk file we failed to load.
-          await actions.reconnectPersistedTerminals()
+          try {
+            await actions.reconnectPersistedTerminals()
+          } catch (reconnectErr) {
+            console.error(
+              '[startup] reconnectPersistedTerminals failed in error path:',
+              reconnectErr
+            )
+            // Why (issue #1158): this is already the recovery path from a
+            // failed hydration. If the recovery itself throws, the async IIFE
+            // rejects as an unhandled promise and workspaceSessionReady never
+            // flips — leaving the user staring at a blank window. Forcing the
+            // flag true lets the app shell mount with an empty session, which
+            // is strictly better than a non-functional UI.
+            useAppStore.setState({ workspaceSessionReady: true })
+          }
         }
       }
       void actions.initGitHubCache()