Skip to content

fix(conversations): respawn agent ptys after exit#2298

Merged
jschwxrz merged 10 commits into
mainfrom
jona/eng-1456-restarting-claude-after-exit-does-not-work
May 30, 2026
Merged

fix(conversations): respawn agent ptys after exit#2298
jschwxrz merged 10 commits into
mainfrom
jona/eng-1456-restarting-claude-after-exit-does-not-work

Conversation

@jschwxrz
Copy link
Copy Markdown
Collaborator

  • restore agent conversation respawn when the underlying pty exits
  • replace backend ptys under a stable logical session id so the renderer stays attached
  • reset the renderer pty view when the backend starts a replacement process
  • guard stale pty exits so old processes cannot unregister newer replacements
  • simplify conversation replacement state and remove unused runtime events
  • cover local and ssh replacement behavior with focused integration-style tests

@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented May 30, 2026

Greptile Summary

This PR restores agent conversation respawn by replacing the old respawnCounts/suppressedExitPtys approach with a new ConversationSessionSupervisor that tracks spawn tokens and a one-replacement-per-failure-window policy. A global epoch counter on ptySessionRegistry lets the renderer's PtySession detect backend respawns via ptyStartedChannel and swap its FrontendPty without tearing down the logical session.

  • Backend: ConversationSessionSupervisor guards stale exits with generation tokens, delegates resize tracking to ptySessionRegistry.resize(), and picks up the last observed terminal size when spawning a replacement.
  • Renderer: PtySession subscribes to ptyStartedChannel in its constructor and recreates FrontendPty on epoch advancement; dispose() / destroy() are split so the epoch listener survives temporary disconnects.
  • Events: AgentSessionExited payload is trimmed to { conversationId, taskId }; ptyStartedChannel gains an epoch field.

Confidence Score: 4/5

Safe to merge with one fix: ptyExitChannel is no longer emitted for agent conversation PTY exits, which breaks the usePty onExit callback for conversation terminals.

The new isCurrentPty guard in the registry onExit handler silently suppresses ptyExitChannel for all normal conversation exits because the conversation handler (registered before ptySessionRegistry.register()) calls unregister() first. The usePty hook subscribes to ptyExitChannel and previously relied on it firing for agent exits. The rest of the change is well-structured and test-covered.

src/main/core/pty/pty-session-registry.ts and the onExit handler registration order in src/main/core/conversations/impl/local-conversation.ts and src/main/core/conversations/impl/ssh-conversation.ts

Important Files Changed

Filename Overview
src/main/core/pty/pty-session-registry.ts Adds epoch tracking, pending-flush map, lastSizes, and isCurrentPty guard — but the isCurrentPty guard prevents ptyExitChannel from being emitted for agent PTY exits because conversation handlers call unregister() before the registry's own onExit fires.
src/main/core/conversations/conversation-session-supervisor.ts New class managing spawn lifecycle, stale-exit guards, and the one-replacement-per-failure-window policy — logic is correct and well-tested.
src/main/core/conversations/impl/local-conversation.ts Replaces respawnCounts/suppressedExitPtys with ConversationSessionSupervisor, correctly threads spawnToken through the async spawn path, and collects last terminal size for replacements.
src/main/core/conversations/impl/ssh-conversation.ts Mirrors local-conversation refactor with added scheduleShellRefreshRetry path; the 127-exit retry now correctly checks isDesired() before refreshing and spawning.
src/renderer/lib/pty/pty-session.ts Adds handleBackendStarted to swap FrontendPty on backend respawn; epoch/version guards cover rapid respawns and the initial-connect race correctly.
src/shared/events/agentEvents.ts AgentSessionExited simplified to only conversationId + taskId, removing projectId, sessionId, and exitCode fields.
src/shared/events/appEvents.ts Adds monotonic epoch field to ptyStartedChannel payload to disambiguate backend respawns on the renderer side.
src/main/core/pty/controller.ts Delegates resize to ptySessionRegistry.resize() to record last dimensions, replacing the direct pty.resize() call.

Comments Outside Diff (1)

  1. src/main/core/pty/pty-session-registry.ts, line 75-95 (link)

    P1 ptyExitChannel never emitted for agent PTY exits

    In both LocalConversationProvider and SshConversationProvider, pty.onExit is registered before ptySessionRegistry.register(). When the PTY exits, the conversation's handler fires first and calls ptySessionRegistry.unregister(sessionId) — removing the entry from ptyMap. When the registry's own onExit handler fires next, the new isCurrentPty guard evaluates to false (ptyMap entry already deleted) and returns early, so events.emit(ptyExitChannel, info, sessionId) is never reached.

    Previously (no isCurrentPty guard) ptyExitChannel always fired on agent exits. Now it is silently suppressed. The usePty hook subscribes to ptyExitChannel and calls onExitRef.current?.(info) — that callback will never fire for conversation terminals, breaking any UI that relies on it (exit-code display, state-change reactions, etc.).

    The fix is to register the conversation's pty.onExit handler after ptySessionRegistry.register() so the registry's handler runs first and emits ptyExitChannel before the conversation handler calls unregister.

    Prompt To Fix With AI
    This is a comment left during a code review.
    Path: src/main/core/pty/pty-session-registry.ts
    Line: 75-95
    
    Comment:
    **`ptyExitChannel` never emitted for agent PTY exits**
    
    In both `LocalConversationProvider` and `SshConversationProvider`, `pty.onExit` is registered **before** `ptySessionRegistry.register()`. When the PTY exits, the conversation's handler fires first and calls `ptySessionRegistry.unregister(sessionId)` — removing the entry from `ptyMap`. When the registry's own `onExit` handler fires next, the new `isCurrentPty` guard evaluates to `false` (ptyMap entry already deleted) and returns early, so `events.emit(ptyExitChannel, info, sessionId)` is never reached.
    
    Previously (no `isCurrentPty` guard) `ptyExitChannel` always fired on agent exits. Now it is silently suppressed. The `usePty` hook subscribes to `ptyExitChannel` and calls `onExitRef.current?.(info)` — that callback will never fire for conversation terminals, breaking any UI that relies on it (exit-code display, state-change reactions, etc.).
    
    The fix is to register the conversation's `pty.onExit` handler **after** `ptySessionRegistry.register()` so the registry's handler runs first and emits `ptyExitChannel` before the conversation handler calls `unregister`.
    
    How can I resolve this? If you propose a fix, please make it concise.
Prompt To Fix All With AI
Fix the following 1 code review issue. Work through them one at a time, proposing concise fixes.

---

### Issue 1 of 1
src/main/core/pty/pty-session-registry.ts:75-95
**`ptyExitChannel` never emitted for agent PTY exits**

In both `LocalConversationProvider` and `SshConversationProvider`, `pty.onExit` is registered **before** `ptySessionRegistry.register()`. When the PTY exits, the conversation's handler fires first and calls `ptySessionRegistry.unregister(sessionId)` — removing the entry from `ptyMap`. When the registry's own `onExit` handler fires next, the new `isCurrentPty` guard evaluates to `false` (ptyMap entry already deleted) and returns early, so `events.emit(ptyExitChannel, info, sessionId)` is never reached.

Previously (no `isCurrentPty` guard) `ptyExitChannel` always fired on agent exits. Now it is silently suppressed. The `usePty` hook subscribes to `ptyExitChannel` and calls `onExitRef.current?.(info)` — that callback will never fire for conversation terminals, breaking any UI that relies on it (exit-code display, state-change reactions, etc.).

The fix is to register the conversation's `pty.onExit` handler **after** `ptySessionRegistry.register()` so the registry's handler runs first and emits `ptyExitChannel` before the conversation handler calls `unregister`.

Reviews (3): Last reviewed commit: "Merge remote-tracking branch 'origin/mai..." | Re-trigger Greptile

Comment thread src/main/core/conversations/impl/ssh-conversation.ts Outdated
Comment thread src/main/core/conversations/impl/ssh-conversation.ts Outdated
@jschwxrz
Copy link
Copy Markdown
Collaborator Author

@greptileai

@jschwxrz jschwxrz merged commit 98fcf07 into main May 30, 2026
1 check passed
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.

1 participant