Repository navigation
(restore): add Restore on click startup mode - #441
Conversation
devsuitup
left a comment
There was a problem hiding this comment.
Thanks for this — lazy restore is a real memory win when many sessions were open. A few things need to change before it can land:
Blocking
- Base is 180 commits behind main (forked at 9ae4b9a, v0.0.58); the diff no longer applies (
public/app.js,public/sidebar.js). Restore on main now goes throughcreateRestorePlanner/tickRestorePlanner(), which waits for indexing and shows the "not in the index" notice (#376). The lazy branch belongs insidetickRestorePlanner, afterplan.actionis resolved; otherwise sessions not yet indexed at startup never become dormant, and the nextpersistWorkingSet()drops them fromopenWorkingSet. - The dormant marker is cleared at the top of
openSession. On main,openSessionfirst callsguardResume(a session live in another process asks before resuming). If the user declines, oropenTerminalfails, the entry has already left the dormant set and falls out of the persisted working set. Clear it only after a successful open, whereskippedWorkingSetEntriesis cleared (or reuse that map). - The tests re-implement app.js instead of loading it. With the lazy branch disabled, the persist loop removed, the delete neutered and the sidebar filter removed all at once, the suite stays green. Please test the shipped code with
loadAppFunctionsfromtest/app-source.js(see.ai/shared-guidelines.md, Testing), and add a case where the click is refused because the session is live elsewhere.
Also
- A dormant entry can only be dismissed by opening it (which spawns
claude) and then closing it, and it escapes the visible-count / max-age limits across restarts. A way to dismiss it, or an age cap, would help. - A
CHANGELOG.mdentry under## Unreleased(seedocs/changelog.md). - The rationale comment at
public/app.js:123belongs in.ai/contexts/session-state.md(a short "lazy restore" section): code keeps at most a one-line pointer. - Worth stating in the docs: in lazy mode the grid starts empty and the last active session is not selected.
Checked and fine: the eslint.config.js global, the settings wording, SETTING_DEFAULTS for the default, and nothing spawns from persisted data without a click.
CI has not run on this head yet; it will once the branch is rebased.
93bec92 to
4b8d98e
Compare
devsuitup
left a comment
There was a problem hiding this comment.
Thanks for the rebase. The lazy branch now sits in tickRestorePlanner, after the plan resolves, and dormant entries are spliced back in persistWorkingSet. Point 1 is resolved. Two things still block:
Blocking
-
The dormant marker is still cleared before the open succeeds (
public/app.js:1303-1305, at the top ofopenSession, beforeguardResumeand theresult.okcheck). Scenario:- a dormant session is live in another process;
- the user clicks it and cancels the confirm;
- the entry is now neither dormant nor open, and the next persist drops it from the working set for good.
The same loss happens when
openTerminalreturnsok:false, because the entry becomesclosed. Fix: movedormantWorkingSet.delete(sessionId)and the class removal next toskippedWorkingSetEntries.delete(sessionId), afterresult.ok. -
The refusal case is not tested. In
test/restore-lazy.test.js,getSessionLiveElsewherealways returns null, so moving the delete either way leaves the suite green. Please add a case where it returns a live session andconfirmreturns false. Then assert that the session is still dormant and still in the persisted set.
Non-blocking
- Function extraction. The test slices functions with its own brace matcher.
extractFunction/loadAppFunctionsfromtest/app-source.jsskip braces inside strings and regexes; please use them. - No way to get rid of a dormant entry. It still bypasses the visible-count and max-age limits (
public/sidebar.js:729), and dismissing it means opening it, which spawnsclaude. A dismiss action or an age cap would help. - Sidebar untested. No test loads the sidebar changes (the
.dormantclass, the visibility bypass). - Docs. Please mention that in lazy mode the grid starts empty and the last active session is not selected.
CI is green on 4b8d98e. The diff against current main merges without textual conflicts. The tests were not run on the merged result.
Review of devsuitup#441: - openSession clears the dormant marker only after openTerminal succeeds, so a resume refused by guardResume or a failed open keeps the entry dormant; persistWorkingSet keeps a dormant or skipped entry whose open entry is closed. - "Don't restore" in a dormant session's context menu removes it from the saved set without spawning claude. - restore-lazy tests load the shipped functions with loadAppFunctions next to the real sidebar.js, and cover the refused resume, the failed open, the .dormant class, the visibility bypass and the dismiss. - docs/session-restore.md and docs/settings.md describe Restore on click, including the empty grid and no selected session at launch.
4b8d98e to
7c64d17
Compare
|
Thanks for the second pass. Rebased on current main (b03875b) and addressed everything in 7c64d17: Blocking
Non-blocking
Each of these mutations turns the suite red: delete back at the top of |
Review of devsuitup#441: - openSession clears the dormant marker only after openTerminal succeeds, so a resume refused by guardResume or a failed open keeps the entry dormant; persistWorkingSet keeps a dormant or skipped entry whose open entry is closed. - "Don't restore" in a dormant session's context menu removes it from the saved set without spawning claude. - restore-lazy tests load the shipped functions with loadAppFunctions next to the real sidebar.js, and cover the refused resume, the failed open, the .dormant class, the visibility bypass and the dismiss. - docs/session-restore.md and docs/settings.md describe Restore on click, including the empty grid and no selected session at launch.
devsuitup
left a comment
There was a problem hiding this comment.
Review at 7c64d17, which is now rebased on current main. Thanks for that, and for the earlier changes: the marker is cleared only after a successful open, the refusal case is tested through the shipped code, there is a way to dismiss a dormant entry, the docs and the CHANGELOG are in. Two points still block. Both were reproduced against the shipped code by the reviewer, and I re-checked them by reading.
Blocking
- Sessions awaiting the index are lost from the saved set (
public/app.js,persistWorkingSet). The loop adds the open sessions plus the skipped and dormant entries. A saved session that the planner has not marked yet is in neither list. If anything persists during the cold index (a click, a close),openWorkingSetis overwritten without it. When the planner later marks it dormant, nothing persists again, so the next restart no longer has it. Fix: keep the saved entries that are not yet resolved when persisting, or persist again after the planner marks them. Test: persist while a saved entry is still unindexed, then check the stored set. - A renderer reload opens the remembered active session before the lazy check (
public/app.js, theloadProjects().thenblock).openSession(session, undefined, { automatic: true })runs foractiveSessionIdregardless of the mode, so a reload starts a session without a click, including one that had exited.
Non-blocking
- Deleting a dormant session does not clear its dormant entry, so its saved record stays: only
dismissDormantSessionandopenSessionremove it fromdormantWorkingSet. - The age-based project collapse still hides dormant rows (
public/sidebar.js). - Dormant rows cannot be selected or opened with the existing keyboard navigation (
public/sidebar.js,public/grid-view.js).
87 existing tests pass locally on Node 24. Not verified: Node 20/22 with c8.
|
Tested on Linux (Ubuntu, GNOME on Wayland), PR head 7c64d17. Setup. An isolated instance, with a throwaway HOME and its own data directory. The fixture is one git project with four synthetic transcripts. A stub Working
Blocking: "sessions awaiting the index are lost from the saved set." Reproduces.
Blocking: "a renderer reload opens the remembered active session." Reproduces. Session 1 was opened from its dormant row, then ended with Ctrl-C. Seen alongside, not caused by this PR. While the scan worker is blocked, quitting hangs in main. The test FIFO is a forced condition, not a real large history. |
Review of devsuitup#441: - openSession clears the dormant marker only after openTerminal succeeds, so a resume refused by guardResume or a failed open keeps the entry dormant; persistWorkingSet keeps a dormant or skipped entry whose open entry is closed. - "Don't restore" in a dormant session's context menu removes it from the saved set without spawning claude. - restore-lazy tests load the shipped functions with loadAppFunctions next to the real sidebar.js, and cover the refused resume, the failed open, the .dormant class, the visibility bypass and the dismiss. - docs/session-restore.md and docs/settings.md describe Restore on click, including the empty grid and no selected session at launch.
7c64d17 to
f4bd11b
Compare
|
Thanks, and thanks @jbr-sekoia for reproducing both points. Rebased on current main (eaf59ff) and addressed in f4bd11b. Blocking
Non-blocking
Changed upstream tests. These read the reload path and
Docs: Each fix was removed in turn, and each removal fails a test. |
devsuitup
left a comment
There was a problem hiding this comment.
Review at f4bd11b. Thanks: pending-index persistence, the already-exited reload, deleting a dormant session, the initial age collapse and keyboard navigation are fixed, and the changelog ends with (#441). The branch is level with main and its interplay with #485 and #486 holds. 160 tests in 13 related files pass locally on Node 24. One point still blocks.
Blocking
- A PTY that exits after the live snapshot can be spawned again without a click (
public/app.js,reopenActiveSessionAfterReload~277-280). In lazy mode the function readsgetActiveSessions()once, finds the session live, then callsopenSession(session, undefined, { automatic: true }). If the PTY exits between the snapshot and the open,openSessionstarts a newclaude --resumeon its own, which is exactly what the lazy mode promises not to do. The test checks the snapshot, not the race. Enforce it where the spawn happens: make the automatic open reattach-only in the main process (refuse to spawn when there is no live PTY), and return a result the renderer handles. Test: snapshot says live, the PTY is gone at open time, and nothing is spawned.
Non-blocking
- After the planner settles, a recreated project or worktree group can age-collapse its dormant rows again (
public/app.js~327,public/sidebar.js~981). - Design comments in
test/restore-lazy.test.js(~4) andpublic/grid-view.js(~555) go beyond the one-line pointer allowed in code.
Not run: Linux Node 20/22 with c8, lint, Electron and live-process scenarios, fresh CI. CI on this head is not approved yet.
jbr-sekoia
left a comment
There was a problem hiding this comment.
Review at f4bd11b, rebased on current main (eaf59ff). Both blocking points of the last review and both issues reproduced on Linux are resolved:
- Sessions awaiting the index.
persistWorkingSetnow holdspendingRestoreEntries()next to the dormant ones, and in lazy mode the planner ignoressessionOpenedOutsideRestore, so a session opened from "+" during the cold index no longer settles the planner and drops the rest. Removing either of these failstest/restore-lazy.test.js("saved sessions not indexed yet…"; the first removal also fails four tests intest/restore-pending-persist.test.js). - Renderer reload.
reopenActiveSessionAfterReloadreopensactiveSessionIdin lazy mode only whenget-active-sessionslists it, and that list excludes exited PTYs, so the Ctrl-C-then-reload case no longer spawns. Dropping the lazy check, or never reattaching, fails the reload test. - The three non-blocking points (delete, age collapse, keyboard navigation) are addressed, and each change fails a test when it is reverted.
Mutation check: 20 single inversions run against the restore, resume and open-session suites (108 tests). 15 turn them red. The 5 that stay green are listed under Non-blocking. CI is green on f4bd11b.
Blocking
-
The previous/next session keys can resume the same dormant session several times (
public/grid-view.js,navigateSession).navigateSessioncallsopenSessionon a dormant row without awaiting it, and the handler does not filtere.repeat.activeSessionIdchanges only whenshowSessionruns, afterguardResumeandopenTerminalreturn. Until then, each further press computes the samenextand callsopenSessionfor it again.openSessionchecksopenSessions.hasbefore its first await, so every call goes through. Scenario:xis open andb, the next row, is dormant;- the user presses Ctrl+Shift+] twice quickly, or holds it down;
openTerminalis called forbtwice. Observed in therestore-lazyharness:openTerminalcalls["x","b","b"].
In main,
open-terminalawaitsstartMcpServerbeforeactiveSessions.set(sessionId, …)(main.js, on by default). The second request can therefore pass theactiveSessions.hasreattach check, so twoclaude --resume brun against one transcript and the first PTY is no longer tracked. This main-side part was traced by reading, not run. A double click on a sidebar row could already do this on main; the held key makes it a one-key action, and it lands on a feature whose promise is that nothing starts without a deliberate action. Fix: keep a set of sessions being opened, checked and filled at the top ofopenSessionand cleared in afinally, or at least ignorenavigateSessionwhile the target has an open in flight. Test: twonavigateSession(1)without awaiting, then oneopenTerminalcall.
Non-blocking
- A reload marks sessions that are still running as dormant (
public/app.js,tickRestorePlanner). After a reloadopenSessionsis empty apart from the reattached active session. The planner therefore marks every other saved session dormant, even when its PTY still runs, and the row shows the "not started" outline. A click reattaches, so nothing spawns. But Don't restore on such a row removes the session from the set while itsclaudekeeps running. Skipping the ids thatget-active-sessionslists (or reattaching them) would match the docs. - Untested branches. Each of these mutations leaves the suite green:
hasDormant = falsefor slug groups (public/sidebar.js), so a dormant session inside an old slug group is folded under "+ N older";- dropping the
slug-groupbranch ofrevealDormantSessions; - dropping the
worktree-sessionsbranch ofrevealDormantSessions; - dropping the
.dormantclass removal inopenSession.
- Vacuous assertion (
test/restore-lazy.test.js, "Don't restore"). The final check that a non-dormant row has no Don't restore item passes even when the item is shown on every row (if (true)). After the re-render,n1has nooncontextmenu, so no menu opens at all. Assert that the menu exists before checking its items.
Lint: 0 errors. Not verified: a live run of the navigation race, and Node 22 with c8.
Review of devsuitup#441: - openSession clears the dormant marker only after openTerminal succeeds, so a resume refused by guardResume or a failed open keeps the entry dormant; persistWorkingSet keeps a dormant or skipped entry whose open entry is closed. - "Don't restore" in a dormant session's context menu removes it from the saved set without spawning claude. - restore-lazy tests load the shipped functions with loadAppFunctions next to the real sidebar.js, and cover the refused resume, the failed open, the .dormant class, the visibility bypass and the dismiss. - docs/session-restore.md and docs/settings.md describe Restore on click, including the empty grid and no selected session at launch.
f4bd11b to
d8b210b
Compare
|
Thanks to both of you. Rebased on 805d064, the parent of #499: my token cannot push a branch that contains its workflow change. The branch merges cleanly with current main (75ab0ad), including #501's scheduled slug groups in Blocking
Non-blocking
Other changes
Testing
|
devsuitup
left a comment
There was a problem hiding this comment.
Review at d8b210b. Thanks for this round: both blocking points from the last reviews and all the non-blocking ones are addressed, and CI is green on every job. Merged into current main (75ab0ad) in a scratch copy there is no textual conflict, and 255 tests across the restore, resume, open-session, sidebar, grid and restore-plan files pass on the result. Removing any of 11 of the PR's lines one at a time turns a test red.
Previous findings
- PTY exiting after the live snapshot: resolved.
main.js:2559refuses areattachOnlyopen after the live-PTY reattach branch, before the remote attach and any spawn;get-active-sessionsand the exit handler (~2402-2425) read the sameactiveSessions; the renderer passes the flag (public/app.js:279,:338,:1412) and handlesnotLive(:1424). - Same dormant session resumed twice by prev/next: resolved,
openingSessions(public/app.js:1380-1447) checked before the first await and cleared infinally. - Reload marking running sessions dormant: resolved (
public/app.js:333-342). - Group recreated after marking: resolved,
revealDormantSessions(newSidebar)before morphdom (public/sidebar.js:1031). - The four surviving mutants: resolved. Vacuous "Don't restore" assertion: resolved (
test/restore-lazy.test.js:230-231). Design comments: resolved.
Major
- The CHANGELOG entry lands in the v0.0.90 section once merged. Based on 805d064, the
(#441)line ends up under## v0.0.90 — 2026-10-09 → ### Newin the merge, and the changelog check stays green. Since the rebase is blocked by #499's workflow change on your side, we can move it under## Unreleasedby a maintainer edit on the branch or at merge — tell us which you prefer. - Holding the prev/next key resumes one dormant session after another (
public/grid-view.js:567-568, handler:642). Once an open finishes,activeSessionIdhas moved, so the next auto-repeatkeydownresumes the following dormant row, and the list wraps. Holding Ctrl+Shift+] for a couple of seconds with 15 saved sessions in Restore on click starts severalclaude --resume, the cost this mode exists to avoid. Passe.repeatfromhandleSessionNavKeytonavigateSessionand do not resume a dormant target on a repeat; test: twonavigateSession(1, { repeat: true })across dormant rows, zeroopenTerminalcalls. - The in-flight guard also fixes a bug on main, which needs its own regression. Today a double click on a sidebar row calls
openSessiontwice (public/sidebar.js:1433-1439) and both pass theopenSessions.hascheck before the first await, so twoopen-terminalrequests go out, in every restore mode. The new test covers it only throughnavigateSession. Please add a test through the row's click handler (two clicks, oneopenTerminal). For bug fixes we ask for the regression test pushed alone first and seen failing in CI, then the fix; splitting the guard and its click test into a small PR would satisfy that and make the fix easy to find later.
Minor
4. public/app.js:1385: a duplicate call returns undefined, which runRestore and the lazy loop (:338, !== false) read as success. In auto mode, a click on a saved session still awaiting getSessionLiveElsewhere, then runRestore reaching it, then a declined "live elsewhere" confirm leaves it neither open nor skipped, so the next persist drops it. Keep openingSessions as a Map of id → promise and return the in-flight promise.
5. public/app.js:333-341: the lazy branch does not hold its candidates while it awaits reattaches (tick() already removed them, public/restore-plan.js:61-62), so a persist during one of the awaits (Don't restore, delete, or #479's exit flush) writes a set without the unprocessed candidates, and the branch never persists at the end. Put them in restoreInFlight for the loop, or persist at the end.
6. public/grid-view.js:568: navigateSession drops the openSession promise, so a throwing openTerminal is an unhandled rejection; a logging .catch is enough.
Suggestion
7. The CHANGELOG says each session "is resumed only when you click it", but prev/next also resume; the docs already say so.
Note: #479 conflicts with this branch in persistWorkingSet (the held line), test/restore-pending-persist.test.js and test/schedule-live-resume.test.js, and the two treat a session closed during a restore differently; whichever lands second reconciles.
Not verified: Electron and live-process runs, the held-key scenario live.
Changes requested: 3 major, 3 minor, 1 suggestion.
devsuitup
left a comment
There was a problem hiding this comment.
Review of 85ebb24 (rebased on 33e8e10). Verdict: changes requested: 0 blocker, 0 major, 2 minor.
CI on 85ebb24: all green (Test 38550846930, Build & Release 38550846911). Locally (Node 22): 19 related files, 289/289 pass; restore-lazy.test.js 51/51 on 85ebb24, and the 4 new tests fail on assertions with 9d9c462's app.js.
Previous points: both closed
- The tick no longer counts its own in-flight candidate as open (
public/app.js:486). The new tests fail on 9d9c462 and pass on 85ebb24. - A second click or the session keys no longer clear the row before the member's open settles (
public/app.js:1728-1740). Same red/green evidence.
Findings
1. MINOR, confirmed by probe: a saved, still-running member is dropped while the row head's open is in flight. public/app.js:484-485 with :1734.
- The tick treats the reattach as done whenever
openSession(...)does not returnfalse. Two other outcomes returnundefined:- the early return at
:1734when the awaited open fails; - a reattach that ends with
ok: falsewithoutnotLive.
- the early return at
- Production rows carry
headSessionId(session-cache.js:656,683), sorowOpenIdon a member also looks at the head.setupMergedin the tests does not set it, so the suite cannot see this. - Scenario:
- Row with members
[B, C], head B, saved[B, C, x]. Only C's terminal is still running after a reload. - The user clicks the row during
getActiveSessions, which starts resuming B. - The tick's reattach of C awaits B's open.
- B fails and the call returns
undefined.
- Result: saved becomes
[B, x]and C is lost. - If B opens instead, C's terminal is never reattached. That behaviour predates this round.
- Row with members
- Suggested fix:
reattached = result !== false && openSessions.get(id) && !openSessions.get(id).closed, so that C falls through to the existing branches: clear it when the row is open, mark it otherwise.- Add
headSessionIdtosetupMerged, or a case that sets it. - Add a test with a running non-head member while the head is opening.
2. MINOR, confirmed by probe: an open the user starts during the marking is undone at the end of the tick. public/app.js:477, :501.
- The lazy branch saves
shown = activeSessionIdbefore its awaits. It callsshowSession(shown)afterwards whenever the active session changed. It cannot tell its own reattach-onlyshowSessioncalls from the user's click. - Since 85ebb24 also waits for the user's open in finding 1's scenario, the user's session opens before the tick ends and is then hidden again.
- Probe, with
n1open and active before the tick:- Plain
[a, x, b]withbrunning, and the user opensaduring the marking: calls["a","b"], active session afterwardsn1. - Finding 1's row with the head opening and succeeding: active session afterwards
n1.
- Plain
- Suggested fix:
- Do not let automatic reattaches change the active session, instead of restoring it afterwards: pass
activate: falseto the tick's reattach-onlyopenSession, and make the early return at:1741-1743respectactivate. - Or skip the restore when a user open ran during the tick.
- Do not let automatic reattaches change the active session, instead of restoring it afterwards: pass
Checked, no finding
- A click that ends without opening: this happens only when the awaited open was the user's own open of the same row and it failed, and that open has already shown its error.
- Double
claude --resume: none found. The exception branch registers inopeningSessionsbefore resuming (:1737) and goes throughguardResumewithautomatic: false. - Rebase:
git diff 33e8e10 prh/441 -- public/app.jsremoves no main behaviour. - Hygiene: no
t.waitFor. The CHANGELOG entry is under## Unreleased.
Restoring the working set resumes a claude process per session at startup, which costs a lot of memory. The new lazy mode marks last run's sessions in the sidebar (outlined dot, always visible) and resumes each one only when it is opened. Unopened sessions stay in the saved working set across restarts; "Don't restore" drops one without starting it. A reload reattaches only to a PTY that still runs and never spawns.
…while reattaching A held previous/next key resumed one dormant session after another: an auto-repeat now cycles the open sessions only. The lazy branch keeps its candidates in restoreInFlight while it awaits the reattaches, so a persist in that window keeps them, and a failed open from the keyboard is logged instead of left as an unhandled rejection. The changelog entry moves under Unreleased and names the keys.
…rking, held keys A dormant session whose conversation continues under another id must not stay marked under the old id; a session opened while the marking awaits a reattach must not be marked; a click during a reattach-only open of the same id must resume it; the session keys must pass e.repeat; a reattach-only open must not follow a continuation.
…ions opened during the marking A click resolving a dormant session to another id moved the saved index and the skipped entry but not the dormant one, so the old id came back marked on every start. The marking now skips a candidate the user opened while it awaited a reattach, and reattach-only opens no longer follow a continuation.
…now on main A fresh entry (a session cleared by /clear, no transcript) has no row to click, so the marking drops it. Test harnesses declare the state the merged exit flush and /clear restore read.
…d during the marking
…, and skip a candidate opened under its continuation The remap moved the dormant entry onto the continuation id, and the already-open branch returned before deleting it, so a closed continuation came back at every start. The remap also leaves restoreInFlight, so the marking no longer marks a session the user opened under a continuation.
… and a save during the marking writes each session once
…t until that open settles A click that resolves a candidate the marking has not reached to a continuation used to drop it from restoreInFlight at once, so a refused or failed continuation left it neither dormant nor saved. In Restore on click mode the entry now moves to the continuation id and, when the open settles, becomes dormant there unless the continuation opened. The marking also takes a candidate out of restoreInFlight once it is dormant, so a save during a later reattach writes it once. Other modes keep main's handling.
… the held entries and the continuation settle A saved member of a merged row has no sessionMap entry of its own: the marking, the row click, the session keys, "Don't restore", Delete and the slug-group fold must all find it through its row.
…ession row The marking, the reattach and the session keys resolve a member id through its row (indexedSessionById). A row click resumes the row's dormant member, and opening or showing any member clears every dormant id of the row, since the row is then open. Row marks go through sessionItemEl and follow every id of the row. "Don't restore", Delete and the slug-group fold use rowSessionIds. The changelog entry moves to Unreleased.
… member, whatever the saved order After a reload, a later saved member of a row whose other member was reattached is still marked, and the session keys then resume it beside the running one. The result also depends on the saved order.
…ked, and the session keys show that member The marking now clears a candidate whose row already has an open member, so the result no longer depends on the saved order, and navigateSession opens the whole row for such a member instead of resuming it beside the running one. indexedRowOf maps a member to its row through localMergedMemberRows instead of scanning sessionMap.
…s in flight, and a later member clears the members marked earlier
… still in flight A member opened during the marking counted as open before its openTerminal settled, so a failed open lost the other saved members of its row.
…nd answer the in-app confirm
…d open of its row, keeps the saved sessions when it fails
… or by a second open of its row The marking now marks a candidate whose open is still in flight, so a failed open leaves it saved; the open's own clearDormantRow drops it on success. openSessionNow waits for an in-flight open of the row's member before clearing the row, and resumes that member when the open it waited for was an automatic reattach that found no PTY.
85ebb24 to
e52abdf
Compare
|
Checked e52abdf (rebase onto 1168203). Thanks for keeping up with main. Rebase: clean. Still open — the two MINORs from the 85ebb24 review, both reproduced again at e52abdf:
Verdict: changes requested (0 blocker, 0 major, 2 minor). |
…open is kept when it fails, and a session opened during the marking stays active Production rows carry headSessionId, so a member's reattach looks at the head's open; setupMerged can now set it.
…, and count only a session left open A reattach that returned early after another member's open failed was taken as done, dropping a running member from the saved set; and restoring the active session afterwards hid a session the user opened during the marking.
devsuitup
left a comment
There was a problem hiding this comment.
Checked 9ede0f2. Both MINORs from the 85ebb24 review are fixed.
- Saved running member while the head's open is in flight: row
[B,C]withheadSessionId: 'B', saved[B,C,x], only C live, row clicked duringgetActiveSessions. B fails: e52abdf saved[B,x], 9ede0f2 keeps[B,C,x]. Test 50 is red with e52abdf'sapp.jsand green here; removing the open check atpublic/app.js:485turns it red again. - User open during the marking: the active session ends on the user's
a(wasn1). Test 51 is red on e52abdf, green here; removingactivate: falsefails tests 16 and 51. restore-lazy.test.js53/53; the 41 restore-related test files pass locally on Node 22 (734 tests); CI green on all jobs. Not.waitFor; the changelog diff is only the(#441)line.
Nit, not blocking: the mayShow() guard on openSessionNow's two early returns (:1817, :1861) has no test. Reverting both to a plain showSession(...) keeps every test green, yet the case is reachable: during the marking the user opens b (PTY live) then switches to n1; the tick's reattach of b hits the already-open return. Here the active session stays n1; with the guard reverted it becomes b. A test for it would lock that in.
Verdict: approve.
|
Rebased on main 1168203 (backup of the reviewed head: Tests 665059b pushed alone: Test 38701105223 red on assertions, only the two new tests (Build & Release 38701105143 cancelled). Fix 9ede0f2: Test 38702167436, Build & Release 38702167380 green. 1. Running member dropped while the head's open is in flight. The tick counts a reattach only when 2. User's open undone at the end of the tick. The tick's reattaches pass Docs: |
Based on main 421f77f, which carries #539 (the in-flight open guard), #477, #479, #547/#551's merged web-session rows and #558's restore of a merged member; this PR has only its own commits.
Why
Restoring the working set at startup resumes one
claudeprocess per session, which costs a lot of memory when many sessions were open.What
New Restore Sessions on Startup option: Restore on click (
restoreOnStartup: 'lazy').reattachOnly; main answersnotLiveinstead of spawning).openSessionpath; the marker clears only once the open succeeds. A held key never resumes marked sessions one after another. A marked session whose conversation continued under another id is resumed there, and the new id replaces it in the saved set; if that continuation is declined or fails to start, it stays marked and saved under the new id, also when it was opened before the marking reached it. A merged web-session row is marked when any of its sessions is; a click on it resumes its marked member (or shows the member already open) and clears the row's marks, and the session keys resume a marked member under its own id.openWorkingSet, so they survive further restarts.Default stays "Ask on startup".
Tests
test/restore-lazy.test.jsloads the shipped functions andsidebar.js.mainin my sandboxed environment (the CLI-state canary, 12test-prisolation tests).🤖 Generated with Claude Code