From 67fd80bb4ad2f14abbe8f6a4058d7d3e7a4760ce Mon Sep 17 00:00:00 2001 From: brennanb2025 Date: Wed, 6 May 2026 16:24:57 -0700 Subject: [PATCH] WIP: Changes before auto-review fixes iteration 3 Co-authored-by: Orca --- 00-review-context.md | 6 ++++-- src/renderer/src/App.tsx | 25 ++++++++++++++----------- 2 files changed, 18 insertions(+), 13 deletions(-) diff --git a/00-review-context.md b/00-review-context.md index ec31a87d737..7413f3e0896 100644 --- a/00-review-context.md +++ b/00-review-context.md @@ -64,11 +64,13 @@ - src/renderer/src/store/slices/terminals-hydration.test.ts:280-286 | Low | "toggles both ways" test — coupled to terminals.ts:105 skip - src/renderer/src/lib/workspace-session.test.ts:144-202 | Low | integration test naming — describes block content - src/renderer/src/store/slices/terminals.ts:98-106 | Low | hydrationSucceeded in TerminalSlice — matches existing convention +- src/renderer/src/App.tsx:397-419 | Medium | fallback setState doesn't stash deferredSshSessionIdsByTabId — recovery-of-recovery path; user is in degraded mode and will restart per toast prompt. Acceptable trade-off +- src/renderer/src/App.tsx:353-364 | Low | toast lacks explicit id for dedup — single-shot path; speculative future-proofing ## Iteration State -Current iteration: 2 -Last completed phase: Iteration 1 fixes complete (typecheck + tests pass) +Current iteration: 3 +Last completed phase: Iteration 2 fixes complete (typecheck + tests pass) Files fixed iteration 1: - src/main/persistence.ts (load() backup fallback, chained writes, rotation-after-write, ENOENT handling) - src/main/persistence.test.ts (updated existing tests for new semantics + 6 new tests) diff --git a/src/renderer/src/App.tsx b/src/renderer/src/App.tsx index 2c4671e7b1f..1862ff20638 100644 --- a/src/renderer/src/App.tsx +++ b/src/renderer/src/App.tsx @@ -176,12 +176,14 @@ function App(): React.JSX.Element { let uiHydrated = false // Why (issue #1158): track whether the success-path call to // reconnectPersistedTerminals started so the catch path doesn't run it a - // second time. Reconnect mutates store state via set() blocks inside its - // loops (assigning ptyIds, draining pendingReconnect* maps); re-entering + // second time. Reconnect mutates store state via per-tab set() blocks + // inside its loops (populating tabsByWorktree / ptyIdsByTabId); re-entering // it on partially-mutated state would double-set ptyIds and drain pending* // maps twice. If the success-path call started and threw mid-loop, those - // set() blocks may have already flipped workspaceSessionReady — but if - // they didn't, we still need to force the flag true so the UI mounts. + // per-tab set() blocks may have populated tabsByWorktree / ptyIdsByTabId + // for some tabs but did NOT reach the tail set() that flips + // workspaceSessionReady — so we still need to force the flag true so the + // UI mounts. let reconnectStarted = false void (async () => { try { @@ -358,7 +360,7 @@ function App(): React.JSX.Element { action: { label: 'Restart now', onClick: () => { - void window.api.app.relaunch?.() + void window.api.app.relaunch() } } }) @@ -404,12 +406,13 @@ function App(): React.JSX.Element { } } else { // Why (issue #1158): the success-path call to - // reconnectPersistedTerminals already started; its set() blocks - // may have flipped workspaceSessionReady before throwing mid-loop. - // Don't re-run reconnect over partially-mutated state (it would - // double-set ptyIds and drain pending* maps twice). Force the - // flag true so the UI mounts even if the partial run never - // reached the line that flips it. The same pending* clear applies + // reconnectPersistedTerminals already started; its per-tab set() + // blocks may have populated tabsByWorktree / ptyIdsByTabId for + // some tabs but did NOT reach the tail set() that flips + // workspaceSessionReady (that runs after the loop completes). + // Don't re-run reconnect over partially-mutated state — doing so + // would double-set ptyIds and drain pending* maps twice. Force + // the flag true so the UI mounts. The same pending* clear applies // here for the same reason as above. useAppStore.setState({ workspaceSessionReady: true,