Repository navigation
Return tab to plain terminal icon when a live agent session exits - #7834
austinywang wants to merge 4 commits into
Conversation
…minal icon Covers #7822 for Claude, Codex, and OpenCode through the real Workspace state path: - quitting a live agent (dead recorded runtime + title revert to the shell cwd) must clear the restored-snapshot brand icon even when no shell-activity prompt event arrives - a session persist running after the agent already exited (idle prompt, no live runtime) must not adopt the indexed session and repaint the brand icon on a plain shell tab - guards: persist-time adoption while the agent command runs keeps working, and app-relaunch manual-resume / pending auto-resume panels keep their brand mark The exit-path expectations fail without the fix (next commit). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
) Root cause: a panel's restored (resumable) agent snapshot — the icon resolver's final fallback — had two unguarded lifecycle gaps, so a tab kept its agent brand icon (Claude, Codex, OpenCode, all kinds) after the user quit the agent back to a plain shell: - Session persists adopt the hook-index session into restoredAgentSnapshotsByPanelId without any evidence the agent is still running, so a persist after the exit repainted the brand icon onto an idle prompt as .manualResumeAvailable. - The only exit signal that invalidated the snapshot was the shell-activity state machine, which needs a report_shell_state event to arrive after adoption; the proven-dead-PID prune (#7740) cleared live runtime but left the snapshot driving the icon. Fix (Workspace+RestoredAgentLifecycle.swift): - adoptIndexedRestorableAgentSnapshot gates fresh persist-time adoption on recorded agent runtime or a running foreground command, and keeps honoring invalidated fingerprints. - invalidateRestoredAgentSnapshotForProvenAgentExit treats a stale-agent prune (title update, submit-action refresh, or the 30s PID sweep) as an observed agent exit and invalidates the snapshot, exactly like the shell-activity machine does. Pending auto-resume, hibernated panels, and remote workspaces are exempt, and app-relaunch restores never record PIDs, so seeded manual-resume icons are untouched. The invariant now holds for every agent kind: a plain-shell tab with no live agent and no pending restore shows the plain terminal icon. Closes #7822 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughChangesRestored agent lifecycle
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant SessionRestore
participant Workspace
participant PanelLifecycle
participant TerminalTab
SessionRestore->>Workspace: provide indexed restorable snapshot
Workspace->>Workspace: adopt snapshot and derive resume state
Workspace->>TerminalTab: synchronize agent brand icon
PanelLifecycle->>Workspace: prune stale agent PID ownership
Workspace->>Workspace: invalidate restored snapshot for proven exit
Workspace->>TerminalTab: restore plain terminal icon
Possibly related issues
Possibly related PRs
🚥 Pre-merge checks | ✅ 24 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (24 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Greptile SummaryThis PR fixes terminal tab agent icons after a live agent exits. The main changes are:
Confidence Score: 4/5This is close, but one lifecycle case should be fixed before merging.
Sources/Workspace.swift Important Files Changed
Reviews (3): Last reviewed commit: "Keep exited agent sessions resumable; ga..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| restoredAgentResumeStatesByPanelId[panelId] != .awaitingAutoResumeCommand, | ||
| (panels[panelId] as? TerminalPanel)?.isAgentHibernated != true else { |
There was a problem hiding this comment.
When a manually resumable restored panel also has a stale recorded PID, the new prune path treats that dead PID as proof of a live-agent exit and clears the restored snapshot. That can remove the intended manual-resume brand icon and resume state during title churn or the PID sweep, even though pending auto-resume and hibernated panels are protected from the same stale-runtime case.
There was a problem hiding this comment.
This is the designed invalidation, not a regression — the case where .manualResumeAvailable coexists with recorded agent runtime can only arise when an agent actually ran live in this panel during this app run and its process died:
- App-relaunch restore and hibernation never record agent PIDs (
seedSessionRestoredAgentIconStateseeds no runtime;enterAgentHibernationclears all PIDs before setting.manualResumeAvailable), so the intended manual-resume brand icon has no recorded PID to prune — covered byrelaunchRestoredManualResumePanelKeepsBrandIcon. - If a recorded PID exists and is proven dead, the agent ran and exited here. The shell-activity machine already invalidates
.manualResumeAvailableon any observedcommandRunning(Workspace.swift,updateRestoredAgentResumeState), so pruning on proven process death applies the same semantics through the runtime signal — this is the Tab keeps Claude icon after quitting to plain shell — restored-agent snapshot fallback survives #7740 #7822 fix itself (mid-run adoption can park the state at.manualResumeAvailablewhen shell integration is absent). .awaitingAutoResumeCommandstays protected because there the recorded runtime provably belongs to a previous run, never to the queued resume — covered bypendingAutoResumePanelKeepsBrandIconThroughStaleRuntimePrune.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
Sources/Workspace+PanelLifecycle.swift (1)
170-188: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winGate snapshot invalidation on proven process exit
The
guard let pidbranch clears ownership and setsdidChangewithout checking process liveness. This state is reachable becauseadoptDetachedAgentRuntimeStateintentionally restoresagentPIDKeysthat may have no corresponding PID. Line 188 then invalidates the restored snapshot even though no exit was proven, potentially removing a valid manual-restore snapshot and its brand icon.Track
didProveAgentExitseparately and set it only when a non-nil PID failsisRecordedAgentPIDLive.As per path instructions, correctness-critical exit decisions must use authoritative running/exited evidence and fail closed when that signal is missing.
Proposed fix
let keys = agentPIDKeysByPanelId[panelId] ?? [] var didChange = false + var didProveAgentExit = false for key in keys { guard let pid = agentPIDs[key] else { if clearAgentPID(key: key, panelId: panelId, clearStatus: true, refreshPorts: false) { didChange = true } continue } if !isRecordedAgentPIDLive(key: key, pid: pid), clearAgentPID(key: key, panelId: panelId, clearStatus: true, refreshPorts: false) { didChange = true + didProveAgentExit = true } } if didChange { if refreshPorts { refreshTrackedAgentPorts() } AppDelegate.shared?.notificationStore?.clearNotifications(forTabId: id, surfaceId: panelId) - invalidateRestoredAgentSnapshotForProvenExit(panelId: panelId) + if didProveAgentExit { + invalidateRestoredAgentSnapshotForProvenExit(panelId: panelId) + } }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Sources/Workspace`+PanelLifecycle.swift around lines 170 - 188, Gate restored-snapshot invalidation in clearStaleAgentPIDs on proven process exit: add a separate didProveAgentExit flag, set it only when a non-nil PID fails isRecordedAgentPIDLive, and leave it false when the guard let pid branch clears ownership without liveness evidence. Continue tracking didChange for cleanup and port refresh, but call invalidateRestoredAgentSnapshotForProvenAgentExit only when didProveAgentExit is true.Source: Path instructions
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@Sources/Workspace`+PanelLifecycle.swift:
- Around line 170-188: Gate restored-snapshot invalidation in
clearStaleAgentPIDs on proven process exit: add a separate didProveAgentExit
flag, set it only when a non-nil PID fails isRecordedAgentPIDLive, and leave it
false when the guard let pid branch clears ownership without liveness evidence.
Continue tracking didChange for cleanup and port refresh, but call
invalidateRestoredAgentSnapshotForProvenAgentExit only when didProveAgentExit is
true.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 496b5ede-87bd-48ff-918e-ae8c627bcf51
📒 Files selected for processing (5)
Sources/Workspace+PanelLifecycle.swiftSources/Workspace+RestoredAgentLifecycle.swiftSources/Workspace.swiftcmux.xcodeproj/project.pbxprojcmuxTests/TerminalTabAgentIconRestoredSnapshotTests.swift
| restoredAgentResumeStatesByPanelId[panelId] != .awaitingAutoResumeCommand, | ||
| (panels[panelId] as? TerminalPanel)?.isAgentHibernated != true else { |
There was a problem hiding this comment.
This still clears manually resumable restored sessions when they also carry a stale recorded PID. seedSessionRestoredAgentIconState(... willRunStartupCommand: false, willRunStartupInput: false) marks app-relaunch manual-resume panels as .manualResumeAvailable, but this guard only exempts .awaitingAutoResumeCommand and hibernation. If restored runtime state includes a leftover PID from the previous run, the next title update or PID sweep prunes that stale PID and reaches this path, clearing the restored snapshot and removing the intended manual-resume icon and resume state. The fix needs to distinguish restore-seeded manual-resume snapshots from live snapshots adopted during the current run, or otherwise preserve the manual-resume stale-runtime case without keeping the post-exit live-session icon.
Rule Used: Flag correctness-critical detection/identity deriv... (source)
There was a problem hiding this comment.
Addressed in 42c29a2 by reworking the mechanism: the prune no longer clears the snapshot at all. It downgrades the resume state to the new .recordedSessionOnly, which keeps the session recorded — persisted for relaunch, manually resumable, forkable — and only stops it from branding the tab icon. AgentSessionAutoResumeSettingsTests (exited-session persistence) and the updated regression tests (quittingLiveAgentReturnsTabToPlainTerminalIcon now asserts the snapshot survives with .recordedSessionOnly) pin both halves. Seeded manual-resume panels remain untouched: restore and hibernation never record agent PIDs, so the prune cannot fire on them.
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit a1fc84b. Configure here.
The first cut of the #7822 fix blocked persist-time snapshot adoption at an idle prompt and cleared snapshots on proven agent exit. That broke designed behavior pinned by AgentSessionAutoResumeSettingsTests: an exited agent session must stay persisted for manual resume after relaunch and remain forkable — only auto-resume (wasAgentRunning=false) and, per #7822, the tab brand icon may not follow it. Rework: RestoredAgentResumeState gains .recordedSessionOnly — the session is recorded and resumable but the agent is known not to be running in this panel. Persist-time adoption assigns it whenever the agent is not the running foreground command, and the proven-dead-PID prune downgrades .manualResumeAvailable/.observedAgentCommandRunning to it instead of clearing the snapshot. The icon resolvers (Workspace and Dock transfer paths) skip the restored-snapshot fallback in that state; everything else (persistence, fork, manual resume, hibernation eligibility, shell-activity state machine parity) behaves exactly as before. Regression tests updated to pin both halves: the icon reverts to the plain terminal icon on exit AND the session snapshot survives for manual resume. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
| case .some(.autoResumeCommandRunning), .some(.observedAgentCommandRunning): | ||
| break | ||
| case .some(.manualResumeAvailable), nil: | ||
| case .some(.manualResumeAvailable), .some(.recordedSessionOnly), nil: | ||
| invalidateRestoredAgentSnapshot(panelId: panelId, restoredAgent: restoredAgent) |
There was a problem hiding this comment.
.recordedSessionOnly, the snapshot is supposed to stay available for manual resume, relaunch persistence, and forking while no longer branding the tab. This branch still sends that state through invalidateRestoredAgentSnapshot on the next .commandRunning shell report. A normal flow can hit this after the agent exits, the stale PID prune downgrades the panel, and the user later runs any shell command; the saved restorable session is then removed entirely instead of only hiding its icon.
| case .some(.autoResumeCommandRunning), .some(.observedAgentCommandRunning): | |
| break | |
| case .some(.manualResumeAvailable), nil: | |
| case .some(.manualResumeAvailable), .some(.recordedSessionOnly), nil: | |
| invalidateRestoredAgentSnapshot(panelId: panelId, restoredAgent: restoredAgent) | |
| case .some(.autoResumeCommandRunning), .some(.observedAgentCommandRunning), | |
| .some(.recordedSessionOnly): | |
| break | |
| case .some(.manualResumeAvailable), nil: | |
| invalidateRestoredAgentSnapshot(panelId: panelId, restoredAgent: restoredAgent) |
lawrencecchen
added
the
stale-revisit
This branch was successfully deployed
3 participants
Closes #7822
Bug
Quitting a live agent session (Claude, Codex, OpenCode — any recognized agent) so the terminal returns to a plain shell left the tab still showing the agent brand icon. The title reverted to the shell cwd, the icon never did. Reproduced on nightly 0.64.17, which already includes #7740, so this was a remaining gap in that fix.
Root cause
A panel's restored (resumable) agent snapshot —
restoredAgentSnapshotsByPanelId, the icon resolver's final fallback — had two unguarded lifecycle gaps:sessionPanelSnapshot) with no evidence the agent is still running. A persist running after the user quit the agent re-adopted the (still resumable) session at an idle prompt as.manualResumeAvailableand repainted the brand icon on a plain shell tab.report_shell_stateevent to arrive after adoption. The proven-dead-PID prune (Fix stale terminal agent tab icons #7740's title path) cleared live agent runtime but left the snapshot driving the icon.Fix
New
Sources/Workspace+RestoredAgentLifecycle.swiftcentralizes the snapshot's lifecycle rules:adoptIndexedRestorableAgentSnapshotgates fresh persist-time adoption on evidence the session is current: recorded agent runtime or a running foreground command. Invalidated fingerprints keep clearing instead of re-adopting.invalidateRestoredAgentSnapshotForProvenAgentExittreats a stale-agent prune (title update, submit-action refresh, or the 30s PID sweep — all proofs the agent process died) as an observed agent exit, invalidating the snapshot exactly like the shell-activity machine does. Pending auto-resume (.awaitingAutoResumeCommand), hibernated panels, and remote workspaces are exempt.App-relaunch restores never record agent PIDs, so seeded manual-resume/hibernation brand icons are untouched by construction. The invariant now holds for every agent kind: a plain-shell tab with no live agent and no pending restore shows the plain terminal icon. This also un-sticks the text box's agent-bound submit context after an exit (same underlying map).
Tests
Two-commit structure so CI proves the tests catch the bug — commit 1 adds
cmuxTests/TerminalTabAgentIconRestoredSnapshotTests.swift(wired into pbxproj) and should be red, commit 2 adds the fix:quittingLiveAgentReturnsTabToPlainTerminalIcon(parameterized: Claude/Codex/OpenCode) — dead recorded runtime + populated restored snapshot +updatePanelTitletitle revert must clear the icon with no shell-activity event. Fails without the fix.sessionPersistAfterAgentExitDoesNotPaintAgentIconOnPlainShell— realsessionSnapshotpersist over an on-disk claude hook-store fixture at an idle prompt must not adopt/repaint. Fails without the fix.Localization audit: no user-facing strings added or changed (state/icon plumbing only). Swift file budget respected (
Workspace.swiftshrank by 20 lines; new files are 73/211 lines); no budget TSV touched; no new warnings in touched files.🤖 Generated with Claude Code
Need help on this PR? Tag
/codesmithwith what you need. Autofix is disabled.Note
Medium Risk
Touches terminal tab icon resolution and restored-agent resume state across persist, title updates, and PID pruning; behavior is guarded by targeted tests but spans several lifecycle edge cases (auto-resume, relaunch, Dock).
Overview
Fixes #7822: after a live agent quits to a plain shell, the tab icon returns to the generic terminal mark instead of keeping the agent brand, while the resumable session snapshot stays for manual resume, relaunch, and forking.
Introduces
RestoredAgentResumeState.recordedSessionOnlyso “session recorded” and “tab should show agent branding” are separate. Persist-time adoption inWorkspace+RestoredAgentLifecyclenow sets.observedAgentCommandRunningonly when the shell reports a running foreground command; otherwise adoption uses.recordedSessionOnlyso a post-exit persist does not repaint the brand on an idle prompt.downgradeRestoredAgentSnapshotForProvenAgentExitruns when stale agent PIDs are pruned (including viaupdatePanelTitle) and moves eligible panels to.recordedSessionOnlywithout dropping the snapshot; pending auto-resume, hibernated panels, and remote workspaces are exempt.TerminalTabAgentIcon(workspace and Dock) ignores the restored snapshot for icon resolution when resume state is.recordedSessionOnly. Regression coverage lives inTerminalTabAgentIconRestoredSnapshotTests.Reviewed by Cursor Bugbot for commit 42c29a2. Bugbot is set up for automated code reviews on this repo. Configure here.
Summary by cubic
Fixes #7822. When a live agent exits, the tab icon now reverts to the plain terminal icon even without a shell-activity event, while keeping the exited session resumable. Applies to Claude, Codex, and OpenCode.
Bug Fixes
RestoredAgentResumeState.recordedSessionOnlyand updated icon resolvers to ignore restored snapshots in this state, so plain-shell tabs don’t show agent branding..recordedSessionOnlyso manual resume/relaunch/fork stay intact without repainting the icon..recordedSessionOnlyand syncs the icon; pending auto-resume, hibernated panels, and remote workspaces are exempt. Centralized inWorkspace+RestoredAgentLifecycle.swiftviaadoptIndexedRestorableAgentSnapshotanddowngradeRestoredAgentSnapshotForProvenAgentExit. Added regression tests for exit, persist-after-exit, live persist, manual resume, and pending auto-resume.Refactors
project.pbxprojentry ordering.Written for commit 42c29a2. Summary will update on new commits.
Summary by CodeRabbit