fix(pty): prevent session ID collision for non-worktree tasks

[Simonstorms]

Summary

Fixes "Session ID ... is already in use" error when creating a new task without a worktree.

New non-worktree tasks were entering the multi-chat session discovery branch, which scans ~/.claude/projects/ for existing .jsonl session files. If any of those files belonged to an active Claude process, the discovered UUID was passed via --session-id and Claude rejected it as already in use.

The fix gates that discovery branch on isResume so new tasks always get a fresh session UUID instead of trying to adopt one from disk.

Screenshot

Type of change

• Bug fix

Checklist

• pnpm run format
• pnpm run lint
• pnpm run type-check
• pnpm exec vitest run (638 tests passing)

🤖 Generated with Claude Code

[vercel]

@Simonstorms is attempting to deploy a commit to the General Action Team on Vercel.

A member of the Team first needs to authorize it.

[greptile-apps]

Greptile Summary

This PR fixes a "Session ID already in use" error by gating the multi-chat session-discovery branch in applySessionIsolation on isResume, so new (non-resume) tasks always receive a freshly generated session UUID instead of attempting to adopt one that may belong to an active Claude process.

• Fix is correct and minimal – adding isResume && to the existing condition is the narrowest possible change; it preserves the intended resume/multi-chat adoption behaviour while eliminating the collision path for new tasks.
• Decision-tree JSDoc is now stale – Step 3 in the function comment doesn't mention the new isResume prerequisite, which could mislead future readers.
• No regression test added – the test suite covers resume scenarios but lacks a test for the specific bug path (isResume=false, other same-provider sessions exist), leaving room for a silent regression.

Confidence Score: 4/5

• Safe to merge — the one-line fix is correct and well-scoped with no regressions observed.
• The change is a single boolean guard that closes a clearly described bug path. Logic for all other branches (resume, additional chat, first-time creation) is unaffected. The only gaps are a stale JSDoc comment and an absent regression test — neither blocks merging.
• No files require special attention beyond the minor comment and test coverage notes in src/main/services/ptyManager.ts.

Important Files Changed

Filename	Overview
src/main/services/ptyManager.ts	One-line guard isResume && added to the multi-chat discovery branch; correctly prevents new tasks from adopting an already-active session UUID. Decision-tree comment and regression test coverage are not updated.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[applySessionIsolation called] --> B{knownSession\nin map?}
    B -- Yes --> C{Session file\nexists & cwd\nmatches?}
    C -- Yes --> D["push --resume uuid\nreturn true"]
    C -- No --> E[Clear stale entry\nfall through]
    B -- No --> F{isAdditionalChat?}
    E --> F
    F -- Yes --> G["push --session-id uuid\n(new chat)\nreturn true"]
    F -- No --> H{"isResume &&\nhasOtherSameProviderSessions?"}
    H -- "Yes (resume only)" --> I[discoverExistingClaudeSession]
    I --> J{Session\nfound?}
    J -- Yes --> K["push --session-id discoveredUuid\nreturn true"]
    J -- No --> L["push --session-id freshUuid\nreturn true"]
    H -- "No (new task → fixed path)" --> M{"!isResume?"}
    M -- Yes --> N["push --session-id freshUuid\nreturn true"]
    M -- No --> O["return false\n(caller uses generic -c -r)"]

    style H fill:#d4edda,stroke:#28a745
    style N fill:#d4edda,stroke:#28a745

Loading

Comments Outside Diff (2)

1. src/main/services/ptyManager.ts, line 467-474 (link)

   Decision-tree comment no longer reflects the isResume guard

   Step 3 in the JSDoc decision tree still describes the multi-chat transition without mentioning the new isResume prerequisite. Keeping the comment in sync prevents future readers from being surprised when they see isResume && in the actual code.

2. src/main/services/ptyManager.ts, line 528-541 (link)

   Missing regression test for the fixed scenario

   The PR fixes the bug where a new non-worktree task with isResume=false would incorrectly enter the discovery branch and receive an already-in-use session UUID. However, no test was added to cover this exact path. Without a dedicated regression test, a future refactor could re-introduce the bug silently.

   Consider adding a test such as:

   it('assigns fresh session UUID for new task when other same-provider sessions exist', () => {
     // Simulate another task's session being present for the same cwd
     const otherPtyId = 'claude-main-task456';
     const sessionMap = {
       [otherPtyId]: { uuid: 'some-other-uuid', cwd: TEST_CWD },
     };
     fsReadFileSyncMock.mockReturnValue(JSON.stringify(sessionMap));
     // File exists on disk (simulates an active session)
     fsExistsSyncMock.mockReturnValue(true);
   
     const cliArgs: string[] = [];
     // isResume = false — brand-new task, not a resume
     const result = applySessionIsolation(cliArgs, claudeProvider, PTY_ID, TEST_CWD, false);
   
     expect(result).toBe(true);
     // Must NOT reuse the existing session UUID
     expect(cliArgs).not.toContain('--resume');
     expect(cliArgs).not.toContain('some-other-uuid');
     // Must have received a fresh --session-id
     expect(cliArgs).toContain('--session-id');
   });

   _{Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!}

_{Last reviewed commit: b6d3053}

[arnestrickmann]

Good catch, @Simonstorms. And thanks for opening the PR.
Merging.