Restore terminal-only tab icons#7824
Merged
Merged
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughRestored-agent lifecycle handling now records completed generations, gates continuation and fork availability, suppresses stale terminal icon fallbacks, preserves completion state across surface transfers, and adds regression coverage for lifecycle, routing, icons, and CI setup. ChangesRestored-agent lifecycle
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related issues
Possibly related PRs
🚥 Pre-merge checks | ✅ 3 | ❌ 22❌ Failed checks (2 warnings, 20 inconclusive)
✅ Passed checks (3 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 |
Contributor
Greptile SummaryThis PR restores generic terminal tab icons while preserving agent-specific behavior outside the tab icon slot. The main changes are:
Confidence Score: 5/5This looks safe to merge.
Important Files Changed
Reviews (26): Last reviewed commit: "Merge remote-tracking branch 'origin/mai..." | Re-trigger Greptile |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
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/Workspace.swift`:
- Around line 433-437: The completed-agent exclusion lacks direct
persistence-level test coverage. Add or extend the `.promptIdle` transition test
to assert that the resulting
`sessionSnapshot(...).panels.first?.terminal?.agent` and relevant `.hibernation`
agent are nil after `completedAgentExit` is set, alongside the existing
resume-state assertion. Use the test’s existing session snapshot and panel
identifiers to verify the stale agent cannot be persisted or restored.
🪄 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: 07376511-0e48-4e74-ab15-4e26376bf360
📒 Files selected for processing (4)
Sources/TerminalTabAgentIcon.swiftSources/Workspace.swiftcmuxTests/AgentSessionAutoResumeSwiftTests.swiftcmuxTests/TerminalTabAgentIconTests.swift
austinywang
changed the title
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 b2e8ec5. Configure here.
Contributor
Author
|
@codex review |
|
To use Codex here, create a Codex account and connect to github. |
@austinywang I can't start this review because your workspace has reached its free monthly review limit. cubic has reviewed 241,631 of the 240,000 allowed lines of code this month. Reviews resume on 1 August 2026 (in 22 days). Paid plans include much higher monthly review limits. Upgrade now to resume reviews. To help optimise your usage, you can tune cubic to get the most out of your usage limits:
|
|
✅ Action performedReview finished.
|
|
To use Codex here, create a Codex account and connect to github. |
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
Sources/Workspace.swift (1)
4586-4614: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
.completedAgentExitalso blocks the live fork path.allowsAgentContinuation(forPanelId:)sits in front of both the restored snapshot lookup and theSharedLiveAgentIndex.sharedfallback, so once a panel reaches.completedAgentExitthe fork/continuation UI stays disabled until some separate seed/clear path rewrites that panel state. Split the restored-session gate from live-agent discovery so a later agent in the same terminal can still be forked.🤖 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.swift` around lines 4586 - 4614, Update allowsAgentContinuation(forPanelId:) so restored-session resume states, including .completedAgentExit, only gate restored snapshot handling and do not block live-agent discovery. Preserve the restored snapshot lookup behavior, but allow the method to fall through to SharedLiveAgentIndex.shared for panels with a later live agent, enabling fork/continuation after the original restored agent exits.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.
Inline comments:
In `@Sources/ContentView`+ForkAgentConversation.swift:
- Around line 154-163: Change the default value of allowsAgentContinuation from
true to false in commandPalettePanelHasForkableAgent,
commandPaletteImmediateForkExecutionSnapshot, and
commandPaletteImmediateForkExecutionSnapshotSelection, or remove the defaults to
require explicit caller decisions; update any affected call sites to pass the
intended value explicitly and preserve fail-closed behavior.
In `@Sources/Workspace`+PanelLifecycle.swift:
- Around line 414-418: Replace the duplicated restored-agent metadata removals
in the panel-close path with a call to clearRestoredAgentSnapshot(panelId:),
while retaining discardAgentRuntimeState as needed. Use the existing helper as
the single source of truth for clearing restored snapshot, resume-state, and
working-directory data.
In `@Sources/Workspace`+TitleOwnership.swift:
- Around line 82-94: In the panel-title update block, avoid calling updateTab
for an unchanged custom-title state as well as an unchanged title. Compare
panelCustomTitles[panelId] != nil with existing.hasCustomTitle, and only invoke
bonsplitController.updateTab when either value differs; pass nil for unchanged
fields where supported, mirroring the no-op guards in installSubscription and
the terminal-title branch.
---
Outside diff comments:
In `@Sources/Workspace.swift`:
- Around line 4586-4614: Update allowsAgentContinuation(forPanelId:) so
restored-session resume states, including .completedAgentExit, only gate
restored snapshot handling and do not block live-agent discovery. Preserve the
restored snapshot lookup behavior, but allow the method to fall through to
SharedLiveAgentIndex.shared for panels with a later live agent, enabling
fork/continuation after the original restored agent exits.
🪄 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: ef7f6852-ebc8-4c19-8226-b165eccfa417
📒 Files selected for processing (20)
Sources/AppDelegate+ResolvedIconRendering.swiftSources/ContentView+ForkAgentConversation.swiftSources/ContentView.swiftSources/DockSplitStore+SurfaceTransfer.swiftSources/DockSplitStore.swiftSources/ResolvedIconAppearanceObserver.swiftSources/TerminalTabAgentIcon.swiftSources/Workspace+AgentLifecycle.swiftSources/Workspace+ForkAgentConversationAvailability.swiftSources/Workspace+PanelLifecycle.swiftSources/Workspace+TitleOwnership.swiftSources/Workspace.swiftSources/WorkspaceContentView.swiftSources/cmuxApp.swiftcmux.xcodeproj/project.pbxprojcmuxTests/CompletedRestoredAgentForkAvailabilityTests.swiftcmuxTests/CompletedRestoredAgentInputRoutingTests.swiftcmuxTests/RemoteTmuxSessionRenameTitleTests.swiftcmuxTests/TerminalTabAgentIconTests.swiftcmuxTests/TerminalTabIconRegressionTests.swift
💤 Files with no reviewable changes (5)
- Sources/AppDelegate+ResolvedIconRendering.swift
- Sources/ResolvedIconAppearanceObserver.swift
- cmuxTests/TerminalTabAgentIconTests.swift
- Sources/cmuxApp.swift
- Sources/TerminalTabAgentIcon.swift
DockTerminalReattachTests names PanelShellActivityState in a helper signature but never imported its defining module, breaking all four app-host unit test shards at compile time. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The Tart macOS-15 runner images that MACOS_RUNNER_15 now routes to have no node on PATH, so the claude wrapper regressions in the CLI no-socket step fail with "exec: node: not found" on every run (main has been red since the runner flip). Pin setup-node for the focused-regression shard so the step is self-sufficient on any runner, matching the nightly.yml precedent for the same failure mode. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
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/RestoredAgentCompletedGeneration.swift`:
- Around line 4-7: Mark the RestoredAgentCompletedGeneration struct as
nonisolated so this Sendable pure value model remains actor-agnostic under Swift
6 default isolation.
🪄 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: ca105e8e-a7b0-438f-8c57-6a045fa627ec
📒 Files selected for processing (20)
.github/workflows/ci.ymlSources/AgentPIDProcessIdentity.swiftSources/ContentView+ForkAgentConversation.swiftSources/ContentView.swiftSources/DockSplitStore+SurfaceTransfer.swiftSources/RestorableAgentSession.swiftSources/RestoredAgentCompletedGeneration.swiftSources/RestoredAgentLifecycleCoordinator.swiftSources/Workspace+AgentLifecycle.swiftSources/Workspace+DetachedSurfaceTransfer.swiftSources/Workspace+ForkAgentConversationAvailability.swiftSources/Workspace+PanelLifecycle.swiftSources/Workspace+TitleOwnership.swiftSources/Workspace.swiftcmux.xcodeproj/project.pbxprojcmuxTests/CommandPaletteSearchEngineTests.swiftcmuxTests/CompletedRestoredAgentGenerationTests.swiftcmuxTests/DockPortalReconcileTests.swiftcmuxTests/DockTerminalReattachTests.swiftcmuxTests/WorkspaceUnitTests.swift
Comment on lines
+4
to
+7
| struct RestoredAgentCompletedGeneration: Sendable { | ||
| let completedAt: TimeInterval | ||
| let processIdentities: Set<AgentPIDProcessIdentity> | ||
| } |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
Mark this Sendable value model nonisolated.
Under Swift 6 default @MainActor isolation, this new pure value-model struct becomes implicitly main-actor-isolated even though it is Sendable and semantically actor-agnostic (it is also carried through DetachedSurfaceTransfer). Declare it nonisolated so it isn't implicitly bound to the main actor.
As per coding guidelines: "In Swift 6, mark Codable, Identifiable, Sendable, and pure value-model structs as nonisolated when they should not be implicitly @MainActor-isolated."
♻️ Proposed change
-/// Records the process generation that a restored terminal already completed.
-struct RestoredAgentCompletedGeneration: Sendable {
+/// Records the process generation that a restored terminal already completed.
+nonisolated struct RestoredAgentCompletedGeneration: Sendable {
let completedAt: TimeInterval
let processIdentities: Set<AgentPIDProcessIdentity>
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| struct RestoredAgentCompletedGeneration: Sendable { | |
| let completedAt: TimeInterval | |
| let processIdentities: Set<AgentPIDProcessIdentity> | |
| } | |
| /// Records the process generation that a restored terminal already completed. | |
| nonisolated struct RestoredAgentCompletedGeneration: Sendable { | |
| let completedAt: TimeInterval | |
| let processIdentities: Set<AgentPIDProcessIdentity> | |
| } |
🤖 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/RestoredAgentCompletedGeneration.swift` around lines 4 - 7, Mark the
RestoredAgentCompletedGeneration struct as nonisolated so this Sendable pure
value model remains actor-agnostic under Swift 6 default isolation.
Source: Coding guidelines
…gent-icon-clear # Conflicts: # .github/workflows/ci.yml
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #7822
Summary
Root cause and decision
Terminal tabs gained several independent agent-icon sources after 0.64.17: live PID state, terminal-title classification, restored-agent snapshots, appearance rerendering, and Dock or transfer propagation. Fixing individual lifecycle edges kept exposing another stale path. The requested invariant is now structural: agent state no longer participates in terminal-tab icon selection.
Testing
dbc9363619adds behavior tests covering live registered agents, an agent-like process title, and a restored agent; the assertions fail against the previous agent-icon implementation.b7df74442eremoves the override system and preservesterminal.fillwith niliconImageDataandiconAsset.fbb60b581e/744ab7e94eprove and fix completed-agent textbox routing.43b5b0278e/b2e8ec5cf7prove and fix completed-agent fork availability across context-menu and command-palette paths.c3a5a8330d/40f398dc7aprove and fix the completed-agent snapshot being deleted by a later ordinary shell command.CommandPaletteSearchEngineTestspassed.40f398dc7awith./scripts/reload.sh --tag issue-7822-restored-agent-icon-clear --launch; the isolated CLI socket was verified../scripts/lint-pbxproj-test-wiring.sh./scripts/check-pbxproj.shgit diff --checkpython3 scripts/swift_file_length_budget.pyreports only the unchanged main-branch baseline overage incmuxUITests/AutomationSocketUITests.swift(551 vs budget 528); that file and budget are absent from this PR diff.Demo Video
40f398dc7a.Review Trigger (Copy/Paste as PR comment)
Checklist
Localization
The only user-visible change is the icon. No user-facing strings, accessibility text, settings, docs, or web messages changed; localization catalogs are untouched.
Note
Medium Risk
Touches restored-agent resume/fork/persistence and tab chrome across Workspace, Dock, and command palette; behavior is intricate but heavily covered by new regression tests.
Overview
Restores terminal-only tab icons by removing agent-brand resolution from tabs (live PIDs, title-derived keys, restored snapshots, Dock/transfer paths, and appearance-driven rerenders). Terminal tabs no longer push custom
iconImageData/iconAssetfrom agent state;updatePanelTitleand related paths only refresh titles.Introduces a
completedAgentExitrestored-agent lifecycle backed byRestoredAgentLifecycleCoordinatorand process-generation tracking (RestoredAgentCompletedGeneration,AgentPIDProcessIdentitynowHashable). When a restored agent finishes (shell prompt idle), the app marks completion instead of wiping snapshots, andallowsAgentContinuation/restoredAgentSnapshotForContinuationgate fork, command palette, persistence, hibernation, and agent-specific textbox routing. A newer live agent process can supersede completion via index reconciliation; stale hook/index updates cannot revive a completed session.Detached surface transfers now carry
shellActivityStateandrestoredAgentCompletedGenerationso completion survives workspace/Dock moves. Session snapshot building uses index entries and reconciles completed agents before accepting restored metadata.Reviewed by Cursor Bugbot for commit 06a7430. Bugbot is set up for automated code reviews on this repo. Configure here.
Summary by CodeRabbit