Skip to content

fix(tasks): clear running status when agent session exits#2377

Open
janburzinski wants to merge 1 commit into
mainfrom
jan/eng-1521-sidebar-task-stays-marked-running-after-agent-closes
Open

fix(tasks): clear running status when agent session exits#2377
janburzinski wants to merge 1 commit into
mainfrom
jan/eng-1521-sidebar-task-stays-marked-running-after-agent-closes

Conversation

@janburzinski

Copy link
Copy Markdown
Collaborator

Description

  • emit agent session exit event when local/ssh sessions are detached/closed
  • clear sidebar task running state when underlying agent session closes

Screenshot/Recording (if applicable)

https://streamable.com/eaqqlm

Checklist
  • I kept this PR small and focused
  • I ran a self-review before opening this PR
  • I ran the relevant local checks or explained why not
  • I updated docs when behavior or setup changed
  • I added or updated tests when behavior changed, or explained why not
  • I only added comments where the logic is not obvious
  • I used Conventional Commits for commit
    messages and, when possible, the PR title

@greptile-apps

greptile-apps Bot commented Jun 4, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR fixes a UX bug where the sidebar task "running" indicator would stay stuck after an agent session was detached or forcibly closed. It emits a new agentSessionExitedChannel event from both LocalConversationProvider and SshConversationProvider whenever a session is detached or stopped, and the renderer's ConversationManagerStore listens for this event to call clearWorking() on the matching conversation.

  • detachPty is changed from void to boolean so callers know if a real PTY was found; the exit event is then conditionally emitted in detachSession, stopSession, destroyAll, and detachAll, carefully guarded by tmux vs. non-tmux mode to avoid double-emits.
  • ConversationManagerStore.listenToSessionExited is wired up in the constructor and cleaned up in dispose, and tests are added covering both the respawn/tmux stop path and the renderer status-clearing path.

Confidence Score: 5/5

Safe to merge — the exit event is correctly guarded to avoid double-emits across all code paths, and clearWorking is a no-op for statuses other than working and awaiting-input, limiting blast radius.

The change is narrow and well-contained: two symmetric provider files gain guarded event emissions, and the renderer-side handler only resets status when it is actively running. The tmux/non-tmux guards correctly prevent duplicate events in every call path (detach→stop, destroyAll, detachAll). Tests cover the two critical new paths.

No files require special attention. Both provider implementations are symmetric and their new event-emission paths are covered by the added tests.

Important Files Changed

Filename Overview
src/main/core/conversations/impl/local-conversation.ts Adds agentSessionExitedChannel emission across detachSession, stopSession, destroyAll, and detachAll; tmux vs non-tmux guards prevent double-emits; logic is symmetric with ssh-conversation.ts.
src/main/core/conversations/impl/ssh-conversation.ts Mirror of local-conversation.ts changes for the SSH provider; identical structure and guards.
src/renderer/features/tasks/conversations/conversation-manager.test.ts Adds a proper IPC event mock that supports handler registration/deregistration, plus a test verifying taskStatus clears to null when the session-exited event fires.
src/main/core/conversations/impl/conversation-provider-respawn.test.ts Adds mockClear between detachSession and stopSession and asserts the exit event is emitted on stopSession for the tmux/respawn path.

Sequence Diagram

sequenceDiagram
    participant User
    participant Provider as Local/SshConversationProvider
    participant Events as IPC Events
    participant Renderer as ConversationManagerStore

    User->>Provider: detachSession(conversationId)
    Provider->>Provider: detachPty() returns bool
    alt non-tmux and PTY was found
        Provider->>Events: "emit(agentSessionExitedChannel, {conversationId, taskId})"
        Events->>Renderer: listenToSessionExited handler
        Renderer->>Renderer: "conversationStore.clearWorking() → status = idle"
    end

    User->>Provider: stopSession(conversationId)
    Provider->>Provider: supervisor.stop() / sessions.get()
    alt PTY found OR (tmux and wasKnownSession)
        Provider->>Events: "emit(agentSessionExitedChannel, {conversationId, taskId})"
        Events->>Renderer: listenToSessionExited handler
        Renderer->>Renderer: "conversationStore.clearWorking() → status = idle"
    end

    User->>Provider: destroyAll()
    Provider->>Provider: detachAll() emits per session if non-tmux
    alt tmux mode
        Provider->>Events: emit(agentSessionExitedChannel) per knownSessionId
        Events->>Renderer: listenToSessionExited handler
        Renderer->>Renderer: "conversationStore.clearWorking() → status = idle"
    end
Loading

Reviews (1): Last reviewed commit: "fix(tasks): clear running status when ag..." | Re-trigger Greptile

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