fix: Recover from missing shell sessions

[charlesvien]

Problem

Restored terminals wedge after an app restart: stale shell session ids are replayed against PTYs that no longer exist, and the resulting write/resize failures were only logged, leaving a dead terminal.

Changes

1. Stop persisting shell session ids; add a migration that clears saved ids
2. Recover missing interactive shells on write/resize and replay the triggering input
3. Emit a shell Exit event (code 130) on explicit destroy so the renderer learns about teardown
4. Add unit tests for recovery, session id persistence and teardown

How did you test this?

Manually

Automatic notifications

• Publish to changelog?
• Alert Sales and Marketing teams?

[charlesvien]

• fix: Recover from missing shell sessions #2465 👈 (View in Graphite)
• main

This stack of pull requests is managed by Graphite. Learn more about stacking.

[greptile-apps]

T-Rex Logs

What T-Rex did

• Generated targeted recovery checks for interactive terminal write recovery, concurrent writes during one recovery, command terminal missing-session handling, unrelated error handling, and resize-triggered recovery.
• Verified the recovery checks completed successfully for the TerminalManager changes.
• Generated store migration checks for persisted terminal state; the local test environment could not resolve an existing renderer dependency used by the store tests.
• Generated TerminalManager.trex.test.ts with 6 targeted tests for recovery edge cases and ran with pnpm vitest run; all 6 passed.
• Demonstrated concurrent write recovery deduplication: 3 writes share one recovery, all 3 retry after recovery completes.
• Confirmed recoverMissingSession calls handleExit instead of re-creating the session during the command terminal test.
• Confirmed isMissingShellSessionError discriminates unrelated errors during the non-recovery error test.
• Generated terminalStore.trex.test.ts with 9 migration edge case tests; blocked by pre-existing @posthog/electron-trpc/renderer resolution failure (same issue affects the existing terminalStore.test.ts).
• Code review of ShellService.destroy() identified a potential duplicate exit event race: processTracking.kill(pid) may fire before disposables are cleaned up, risking two Exit events.
• Read destroy() and onExit() paths to understand possible double-exit scenarios when kill triggers onExit synchronously.
• Generated destroy-race.test.ts with 3 targeted tests; execution blocked by missing @posthog/git/queries in the sandbox (same issue blocks the existing service tests).
• Saved the destroy race analysis to trex-artifacts/destroy-race-analysis.md.

Artifacts

TerminalManager recovery validation log

• Shows the recovery validation results for TerminalManager changes and notes the renderer dependency issue affecting store tests.

Verbose vitest output for 6 TerminalManager recovery edge case tests — all passed

• Contains detailed test run results confirming all six tests passed.

Destroy race analysis TypeScript excerpt

• Source showing destroy() and onExit interaction related to possible double exit.

Destroy race analysis Markdown

• Trex analysis summarizing the destroy race findings.

_{Ran code and verified through T-Rex}

Comments Outside Diff (1)

1. apps/code/src/main/services/shell/service.ts, line 312-318 (link)

   Avoid duplicate exits

   processTracking.kill(pid) can trigger the PTY onExit callback before this method disposes the listener. In that case the onExit callback emits ShellEvent.Exit, then destroy() continues and emits another exit event with code 130. Renderer subscribers can receive two teardown events for one explicit destroy. Mark or remove the session before killing, or guard the exit listener so only one path emits.

   Artifacts

   
   

   Destroy race analysis

   • Contains supporting evidence from the run (text/markdown; charset=utf-8).

   Generated destroy race test

   • Contains supporting evidence from the run (text/typescript; charset=utf-8).

   Destroy exitPromise analysis

   • Contains supporting evidence from the run (text/markdown; charset=utf-8).

   _{Ran code and verified through T-Rex}

   Prompt To Fix With AI

   This is a comment left during a code review.
   Path: apps/code/src/main/services/shell/service.ts
   Line: 312-318
   
   Comment:
   **Avoid duplicate exits**
   
   `processTracking.kill(pid)` can trigger the PTY `onExit` callback before this method disposes the listener. In that case the `onExit` callback emits `ShellEvent.Exit`, then `destroy()` continues and emits another exit event with code `130`. Renderer subscribers can receive two teardown events for one explicit destroy. Mark or remove the session before killing, or guard the exit listener so only one path emits.
   
   <details><summary><strong>Artifacts</strong></summary><br />
   
   **[Destroy race analysis](https://app.greptile.com/trex/artifacts/c75fabd7-b614-4141-8c27-20d7713980fe)**
   
   - Contains supporting evidence from the run (text/markdown; charset=utf-8).
   
   **[Generated destroy race test](https://app.greptile.com/trex/artifacts/3d88f434-8a4d-4b8e-a61d-e6263daa4876)**
   
   - Contains supporting evidence from the run (text/typescript; charset=utf-8).
   
   **[Destroy exitPromise analysis](https://app.greptile.com/trex/artifacts/c75fabd7-b614-4141-8c27-20d7713980fe)**
   
   - Contains supporting evidence from the run (text/markdown; charset=utf-8).
   </details>
   
   <sub><a href="https://www.greptile.com/trex"><img alt="T-Rex" src="https://greptile-static-assets.s3.amazonaws.com/trex/trex_green.svg" height="14" align="absmiddle"></a> Ran code and verified through T-Rex</sub>
   
   How can I resolve this? If you propose a fix, please make it concise.

Prompt To Fix All With AI

Fix the following 2 code review issues. Work through them one at a time, proposing concise fixes.

---

### Issue 1 of 2
apps/code/src/main/services/shell/service.ts:312-318
**Avoid duplicate exits**

`processTracking.kill(pid)` can trigger the PTY `onExit` callback before this method disposes the listener. In that case the `onExit` callback emits `ShellEvent.Exit`, then `destroy()` continues and emits another exit event with code `130`. Renderer subscribers can receive two teardown events for one explicit destroy. Mark or remove the session before killing, or guard the exit listener so only one path emits.

### Issue 2 of 2
apps/code/src/main/services/shell/service.ts:318-321
**Resolve destroyed sessions**

This explicit destroy path now emits an exit event, but it does not resolve the session's `exitPromise`. The only resolver is inside the PTY `onExit` callback, so if that callback does not fire before the listener is disposed, callers holding the `ShellSession` from `createSession()` can wait forever for a session that has already been torn down. Store the resolver on the session or route destroy through the same exit-completion path.

_{Reviews (1): Last reviewed commit: "Recover from missing shell sessions" | Re-trigger Greptile}