Skip to content

Don't apply the worktree default when resuming a session - #62

Closed
ymajoros wants to merge 1 commit into
doctly:mainfrom
ymajoros:fix/no-worktree-on-resume-upstream
Closed

Don't apply the worktree default when resuming a session#62
ymajoros wants to merge 1 commit into
doctly:mainfrom
ymajoros:fix/no-worktree-on-resume-upstream

Conversation

@ymajoros

@ymajoros ymajoros commented Jun 1, 2026

Copy link
Copy Markdown
Contributor

Problem

A plain click on a session in the sidebar does nothing — no terminal appears, the session stays "inactive" — yet Resume with config works. So a session can only be interacted with through the config dialog.

Cause

openSession() resumes with resolveDefaultSessionOptions(), which is shared with new-session creation and includes the global worktree: true default. Resuming an existing session with --worktree tries to spin it up in a fresh git worktree and fails to attach, so the terminal never shows.

showResumeSessionDialog() has no worktree control and never sets the flag — which is exactly why "Resume with config" works. Plain click and the dialog should behave the same.

Fix

Strip worktree/worktreeName from the resolved options on the resume path in openSession():

const resumeOptions = customOptions || await resolveDefaultSessionOptions({ projectPath });
if (resumeOptions) { delete resumeOptions.worktree; delete resumeOptions.worktreeName; }

New-session creation is untouched (still honors the worktree default). Worktree is a new-session concept — resuming must reuse the session's existing directory.

Testing

Built locally (Electron 41, Linux) with worktree: true as the default: plain click now resumes in place and attaches, matching the dialog. New-session-in-worktree still works.

resolveDefaultSessionOptions() is shared by new-session creation and resume.
The `worktree: true` global default makes sense only for NEW sessions (start
in a fresh isolated git worktree). On resume it's wrong: a plain click on a
session resolves to {worktree:true}, so it tries to resume the existing
session into a brand-new worktree and fails to attach — the terminal never
shows and the session looks stuck "inactive". "Resume with config" works
only because the resume dialog has no worktree control and never sets it.

Strip worktree/worktreeName from the resolved options on the resume path so
plain click resumes in place, matching the dialog.
@abasiri

abasiri commented Aug 1, 2026

Copy link
Copy Markdown
Contributor

Thanks for this — the bug is real and the diagnosis was spot on. Closing in favour of your other fix for the same issue.

While reviewing this alongside #70, I found you'd written this twice: once here (fix/no-worktree-on-resume-upstream) and once on fix/no-worktree-on-resume-jbr, which reached #70 as 5384778 — "Ignore the worktree option when resuming a session (fix at the source)". I've cherry-picked that one onto main as 10b80e2, with you preserved as the author.

I went with the main-process version because of the gap your own commit message calls out. This PR strips the option inside openSession(), but launchScheduleCreator (public/dialogs.js:32) never goes through it:

const options = await resolveDefaultSessionOptions(project);   // carries worktree: true
await window.api.openTerminal(result.sessionId, project.projectPath, false, options);   // isNew=false → resume

So "Create scheduled task" would have stayed broken on any project with the worktree default enabled. Gating at the open-terminal handler (if (isNew && sessionOptions.worktree)) covers that path, fork, and anything added later, and it doesn't mutate the caller's options object.

One follow-up if you're up for it: neither version ships a test, and now that the decision lives at a single chokepoint in the main process it's straightforward to cover. A regression here is silent — the session just fails to attach — so it's worth pinning down.

Same situation applies to #60: #70 carries your reconcile fix plus d9210b6, which adds a 1s throttle because loadProjects() fires get-projects twice per sidebar paint. I measured that on a 48-folder / 771-session install — roughly 1433 syscalls and ~10ms of synchronous main-process work per paint, halved by the throttle. A PR with that folded in would be very welcome.

@abasiri abasiri closed this Aug 1, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants