feat: implement lazy session restore feature with UI toggle#93
feat: implement lazy session restore feature with UI toggle#93quarterpound wants to merge 3 commits into
Conversation
|
@gi11es any chance you can take a look at this? |
gi11es
left a comment
There was a problem hiding this comment.
Thanks — this is a well-constructed PR overall. A removed-behavior audit of the restructured restore path found the default (toggle-off) flow otherwise equivalent to the old eager code: tab order, suppressUnseen, onProcessExit wiring, selectedTabIndex clamping, and tmux name preservation all carry over. But verification confirmed one regression on the default path plus two UI issues, so requesting changes:
Confirmed — blocking
-
Orphaned process spawn on the default path (
startPendingTabsProgressively). The oldcreateTabsProgressivelybuilt tabs from saved state one-by-one, so a tab couldn't be closed before it existed. The new code creates allTabItems up front and holds live references inremaining, walking them at 0.05s cadence. Closing a not-yet-started tab (closeTabById → handleSurfaceClosedById) removes it fromworkspace.tabsbut never clearstab.pendingStart, so when its turn comes,startPendingShellIfNeededhappily callssurface.startShell— spawning an orphanedclaude --resume/codex/tmux process attached to no tab; itsonProcessExit → handleSurfaceClosedByIdis a no-op since the tab is gone. Fix options: clearpendingStartinhandleSurfaceClosedById, and/or havestartPendingShellIfNeededverify the tab is still present in some workspace before starting. -
Tab bar shows pending tabs as live. The sidebar learned
filled: !info.pending(hollow, non-pulsing dots), butHorizontalTabViewinTabBarViews.swiftconstructsBadgeShapeViewwithoutfilled:(defaultstrue) and still appliesisBadgeAnimated. Background tabs of the selected workspace are pending yet appear in the tab bar with solid — and potentially pulsing — dots, contradicting the sidebar one pane over. -
Never-started codex tabs flip to "thinking".
pollCodexBadgeStatesbuildscodexTargetsfrom all codex tabs with asessionId— deferred tabs keep their restored id — andparseCodexActivityInfohappily reads a stale on-disk.jsonlwhose last event istask_started, soapplyCodexBadgeStatessets.codexThinkingon a tab whose process was never spawned. Bounded (hollow dot, not persisted), but wrong: excludependingStart != niltabs fromcodexTargets.
Worth addressing
- Eager/lazy start paths have already drifted: the eager branch guards codex discovery with
tab.sessionId == nil || forkSession, the lazy copy drops|| forkSession(andPendingStartdoesn't captureforkSession). Latent today, but the duplication invites divergence — consider making the eager branch setpendingStartand immediately callstartPendingShellIfNeededso there's one start path. startPendingShellIfNeededtriggers a fullrebuildSidebar()on each first-open (defaultrefreshSidebar: truefromshowTab); cycling through N restored tabs costs N full sidebar rebuilds — a targeted badge refresh would do.- Minor:
PendingStartretainsenvVars/initialInputper never-opened tab indefinitely, andworkingDirectoryduplicatesworkspace.path.
One claim I explicitly checked and did NOT flag: closing a never-opened lazy terminal tab kills its tmux session — that matches the app's documented close-tab semantics, and closeWorkspace still takes the detach() branch thanks to the eager tmuxSessionName assignment. 👍
Also needs a rebase — the PR currently shows as conflicting with master.
On launch, Deckard restored every saved session by immediately spawning its process for every tab in every project. With many projects open, that's dozens of forkpty + agent processes fired within seconds of opening the app, plus a burst of TCC prompts and CPU churn for sessions you may never look at.
This PR makes session startup lazy-capable: tabs and their terminal views are still created up front (the sidebar and tab bar look identical), but the underlying shell/agent process can be deferred until a tab is first opened.
"Restore sessions lazily" toggle controls it, defaulting off so existing behavior is preserved.