Repository navigation
Fix stale terminal agent tab icons - #7740
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughTerminal title updates now prune stale agent runtime PIDs, clear related notifications, and refresh tab icons when runtime or title-derived agent state changes. Tests cover Claude, Codex, and plain-shell transitions. ChangesStale agent cleanup and icon refresh
Estimated code review effort: 2 (Simple) | ~10 minutes Sequence Diagram(s)sequenceDiagram
participant Terminal
participant Workspace
participant NotificationStore
participant BonsplitTab
Terminal->>Workspace: updatePanelTitle(panelId, title)
Workspace->>Workspace: clearStaleAgentPIDs(panelId, refreshPorts: true)
Workspace->>NotificationStore: clearNotifications(forTabId, surfaceId)
Workspace->>BonsplitTab: refresh tab and icon after stale-runtime pruning
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 stale terminal agent tab icons after agent changes in the same terminal. The main changes are:
Confidence Score: 5/5This looks safe to merge.
Important Files Changed
Reviews (5): Last reviewed commit: "Add no-title agent identity regression c..." | Re-trigger Greptile |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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.
Inline comments:
In `@Sources/TerminalTabAgentIcon.swift`:
- Around line 198-207: Add a regression test alongside the existing terminal
title/icon tests covering the didPruneStaleAgentRuntime-only path, using the
relevant Workspace updatePanelTitle and agent PID recording APIs. Start with a
Codex-derived title, record a stale Claude PID, then change to another Codex
title that preserves the derived agent key; assert the stale PID is removed and
both the workspace icon and Bonsplit tab icon update to Codex.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 0379ff03-7297-4e1f-bd87-b4d544c34d86
📒 Files selected for processing (2)
Sources/TerminalTabAgentIcon.swiftcmuxTests/TerminalTabAgentIconTests.swift
| // Update bonsplit tab title only when this panel's title changed, and | ||
| // update the icon when the title-derived agent fallback changes. | ||
| if (didMutatePanelTitle || didMutateTitleDerivedAgent), | ||
| // update the icon when the current agent signal changes. | ||
| if (didMutatePanelTitle || didMutateTitleDerivedAgent || didPruneStaleAgentRuntime), | ||
| let tabId = surfaceIdFromPanelId(panelId), | ||
| let panel = panels[panelId], | ||
| let existing = bonsplitController.tab(tabId) { | ||
| let baseTitle = panelTitles[panelId] ?? panel.displayTitle | ||
| let resolvedTitle = resolvedPanelTitle(panelId: panelId, fallback: baseTitle) | ||
| let titleUpdate: String? = existing.title == resolvedTitle ? nil : resolvedTitle | ||
| let iconAssetUpdate: String?? = didMutateTitleDerivedAgent | ||
| let iconAssetUpdate: String?? = didMutateTitleDerivedAgent || didPruneStaleAgentRuntime |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
Add a test for the didPruneStaleAgentRuntime-only path.
All three test cases transition the title to a different agent (or plain shell), so didMutateTitleDerivedAgent is always true alongside didPruneStaleAgentRuntime. The specific path this fix enables — where didPruneStaleAgentRuntime is true but didMutateTitleDerivedAgent is false (e.g., title changes from "codex --yolo" to "codex --yolo --verbose", both mapping to "codex", while a stale Claude PID is pruned) — is not exercised. Without coverage, a future regression could drop didPruneStaleAgentRuntime from the icon-update condition and the existing tests would still pass.
🧪 Suggested additional test
`@MainActor`
`@Test` func terminalTitleChangePrunesStaleAgentWithoutTitleDerivedKeyChange() throws {
let workspace = Workspace()
let panel = try `#require`(workspace.focusedTerminalPanel)
let tabId = try `#require`(workspace.surfaceIdFromPanelId(panel.id))
// Establish title-derived key as "codex" first
_ = workspace.updatePanelTitle(panelId: panel.id, title: "codex --yolo")
// Record a stale Claude PID
workspace.recordAgentPID(key: "claude_code.old-session", pid: 0, panelId: panel.id, refreshPorts: false)
// Stale Claude icon should be showing (live agent wins over title-derived)
`#expect`(workspace.terminalTabAgentIconAsset(forPanelId: panel.id) == "AgentIcons/Claude")
// Title changes but still maps to "codex" — title-derived key doesn't change
`#expect`(workspace.updatePanelTitle(panelId: panel.id, title: "codex --yolo --verbose"))
// Stale Claude pruned, icon should now show Codex from title-derived key
`#expect`(workspace.terminalTabAgentIconAsset(forPanelId: panel.id) == "AgentIcons/Codex")
`#expect`(workspace.bonsplitController.tab(tabId)?.iconAsset == "AgentIcons/Codex")
`#expect`(workspace.agentPIDs["claude_code.old-session"] == nil)
}🤖 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/TerminalTabAgentIcon.swift` around lines 198 - 207, Add a regression
test alongside the existing terminal title/icon tests covering the
didPruneStaleAgentRuntime-only path, using the relevant Workspace
updatePanelTitle and agent PID recording APIs. Start with a Codex-derived title,
record a stale Claude PID, then change to another Codex title that preserves the
derived agent key; assert the stale PID is removed and both the workspace icon
and Bonsplit tab icon update to Codex.
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)
148-160: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winScope stale PID notification clears to the owning panel(s).
clearNotifications(forTabId:)removes every notification in the tab, butclearStaleAgentPIDs()can be triggered by just one dead PID among multiple panels. UseagentPIDPanelIdsByKeyto callclearNotifications(forTabId:surfaceId:)for the affected panel(s), and only fall back to the tab-wide clear for unowned keys.🤖 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 148 - 160, Update clearStaleAgentPIDs so notification cleanup is scoped to affected panels: track each stale PID’s key, use agentPIDPanelIdsByKey to call clearNotifications(forTabId:surfaceId:) for every owning panel, and only call the existing tab-wide clearNotifications(forTabId:) fallback when a stale key has no owning panel.
🤖 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 148-160: Update clearStaleAgentPIDs so notification cleanup is
scoped to affected panels: track each stale PID’s key, use agentPIDPanelIdsByKey
to call clearNotifications(forTabId:surfaceId:) for every owning panel, and only
call the existing tab-wide clearNotifications(forTabId:) fallback when a stale
key has no owning panel.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 76c2031f-c81d-4976-a13e-1894e7b66578
📒 Files selected for processing (3)
Sources/TabManager.swiftSources/Workspace+PanelLifecycle.swiftcmuxTests/TerminalTabAgentIconTests.swift
…icon-stale # Conflicts: # Sources/TerminalTabAgentIcon.swift # cmuxTests/TerminalTabAgentIconTests.swift
Main reverted #7216 (workspaces-as-todos) and its file splits, so this merge deletes WorkspaceTodoState/WorkspaceTodoPanel/WorkspaceTodoPanelView, AllShortcutsPopover.swift, and VerticalTabsSidebar+EmptyAreasAndFooter.swift, strips the todo fields from the sidebar-immediate and mobile observed snapshots, and folds the returning ContentView sidebar footer/empty-area types back in with the migration transforms applied (@ObservedObject -> let, @EnvironmentObject -> @Environment(TabManager.self)); the footer no longer plumbs modifierKeyMonitor, matching main. Main's stale-agent-icon fix (#7740) merged clean into TerminalTabAgentIcon/Workspace+PanelLifecycle. The file-length budget resolves to union-max per surviving path.
Closes #7731
Root cause
Terminal tab icon state had multiple inputs: PID-derived live agent runtime, title-derived agent fallback, and restored-session metadata. On terminal title changes, cmux updated the title-derived agent key and the tab title, but did not prune dead PID-derived runtime for that same panel before resolving the icon. Because
TerminalTabAgentIconResolvercorrectly gives live runtime precedence over title fallback, a dead Claudeclaude_codePID key could keep winning after the user exited Claude and launched Codex in the same PTY.This was in the cmux app state path, not Bonsplit: Bonsplit already supports clearing or replacing
iconAssetwhen cmux sends the update.Fix
On terminal title updates,
Workspace.updatePanelTitlenow reconciles title-derived agent state and prunes stale per-panel agent PID runtime before resolving the tab icon. That makes the icon reflect the current live-or-title-derived agent signal and covers Claude -> Codex, Codex -> Claude, and agent -> plain shell transitions.Tests
Added behavioral Swift Testing coverage through
Workspace.updatePanelTitlefor:The first commit adds only the regression tests; the second commit adds the fix.
Local verification
git diff --check./scripts/lint-pbxproj-test-wiring.shpython3 scripts/swift_file_length_budget.pycurrently reports pre-existing over-budget files not touched by this PR:Sources/DockSplitStore.swift,Packages/Shared/CmuxAuthRuntime/Sources/CmuxAuthRuntime/Coordinator/AuthCoordinator.swift,Packages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/WorkspaceListView.swift, andSources/Panels/BrowserNavigationDelegate.swift.No local app build or local Xcode tests were run before opening the PR because the issue instructions required GitHub Actions for validation.
Need help on this PR? Tag
/codesmithwith what you need. Autofix is disabled.Note
Medium Risk
Touches workspace agent lifecycle, tab icon sync, and notification cleanup on a hot path (title updates), but changes are localized with strong regression tests.
Overview
Fixes stale terminal agent tab icons when users switch agents in the same PTY (e.g. Claude → Codex) by reconciling icon state on panel title changes.
Workspace.updatePanelTitlenow callsclearStaleAgentPIDs(panelId:)for terminal panels before resolving the tab icon. Dead PID-derived runtime no longer wins over title-derived agent fallback inTerminalTabAgentIconResolver. Bonsplit tab icon updates when stale runtime is pruned even if the title text is unchanged.clearStaleAgentPIDs(workspace-wide and per-panel) now clears stale terminal notifications when pruning succeeds—per-panel cleanup is scoped withsurfaceId.TabManager.sweepStaleAgentPIDsno longer clears notifications itself (that happens inside workspace pruning).Regression tests cover title-only agent identity, Claude↔Codex transitions, plain shell icon clear, port cleanup, and panel-scoped notification removal.
Reviewed by Cursor Bugbot for commit 74f1c5e. Bugbot is set up for automated code reviews on this repo. Configure here.
Summary by cubic
Fixes stale terminal agent tab icons by pruning dead PID runtime during title updates and clearing stale notifications so the icon matches the current agent or a plain shell. Centralizes cleanup and port refresh in
Workspace, and updates the bonsplit icon when the agent signal changes even if the title text doesn’t.Workspace.updatePanelTitle, reconcile title-derived agent, prune per-panel agent PIDs viaclearStaleAgentPIDs(refreshPorts: true), and clear stale notifications (tab-wide or per-panel) before resolving the icon; refresh the bonsplit icon on title, agent-fallback, or pruned-runtime changes.TabManager.sweepStaleAgentPIDs; cleanup now happens inWorkspace, preserving sibling panel notifications. Added tests for Claude↔Codex, agent→shell (icon clears), stale port cleanup, panel-scoped notification cleanup, and that raw titles like "codex --yolo" don’t become agent identity.Written for commit 74f1c5e. Summary will update on new commits.
Summary by CodeRabbit
zsh) correctly clear any lingering agent icon state.