Keep background attention from raising Stage Manager windows - #9776
Merged
Merged
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds main-actor terminal bell playback, surface-scoped unread tracking, Dock-aware attention routing, fail-closed focus behavior, and expanded lifecycle and routing tests. ChangesTerminal bell and explicit input
Unread state and Dock navigation
Feed attention lifecycle
Validation
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related issues
Possibly related PRs
Suggested reviewers: Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (3 errors, 1 warning, 1 inconclusive)
✅ Passed checks (20 passed)
✨ Finishing Touches 💡 1📝 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@cmuxTests/GhosttyEnsureFocusWindowActivationTests.swift`:
- Around line 69-102: The test backgroundAgentAttentionStaysInsideCmux currently
bypasses the blocking ingest path and does not verify AppKit attention behavior.
Update it to submit the permission request through the blocking event ingestion
API, inject or use a spy/stub for NSApplication.requestUserAttention, and assert
that no attention request occurs while preserving the existing sidebar,
lifecycle, and cleanup assertions.
In `@Sources/AppDelegate`+DockAttentionRouting.swift:
- Around line 71-79: Make terminal-bell attention routing fail closed when no
originating surface is provided: in
Sources/AppDelegate+DockAttentionRouting.swift lines 71-79, remove the
fallbackTarget-based surface/tab derivation and require surfaceID before
resolving notificationSurfaceOwner; in Sources/GhosttyTerminalView.swift lines
3104-3107, preserve audio playback for app-target bells but skip pane-attention
requests when the bell lacks a surface identifier.
🪄 Autofix
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 Plus
Run ID: e6039567-7b66-4215-b6a6-23bf199fb7c6
📒 Files selected for processing (6)
Packages/macOS/CmuxTerminal/Sources/CmuxTerminal/Services/Bell/TerminalBellService.swiftPackages/macOS/CmuxTerminal/Tests/CmuxTerminalTests/Services/Bell/TerminalBellServiceTests.swiftSources/AppDelegate+DockAttentionRouting.swiftSources/Feed/FeedCoordinator.swiftSources/GhosttyTerminalView.swiftcmuxTests/GhosttyEnsureFocusWindowActivationTests.swift
Contributor
Author
|
@coderabbitai Re-evaluated the pre-merge “test/debug seam” error against the current code and repository policy. This is not a test-only production hook: the public production |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
service="Packages/macOS/CmuxTerminal/Sources/CmuxTerminal/Services/Bell/TerminalBellService.swift"
tests="Packages/macOS/CmuxTerminal/Tests/CmuxTerminalTests/Services/Bell/TerminalBellServiceTests.swift"
ast-grep outline "$service" --items all --view expanded
printf '\n--- service ---\n'
cat -n "$service"
printf '\n--- tests ---\n'
cat -n "$tests"
printf '\n--- relevant references ---\n'
rg -n -C 3 --glob '*.swift' \
'TerminalBellService\s*\(|requestUserAttention|NSSound\.(beep|contentsOfFile)' \
Packages/macOS/CmuxTerminal Sources cmuxTests 2>/dev/null || trueLength of output: 127887
I withdraw the “test/debug seam” finding. 🐇 ✏️ Learnings added
You are interacting with an AI system. |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?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/TerminalNotificationStore.swift`:
- Around line 696-706: Replace the computed manualUnreadSurfaceKeys rebuild with
a maintained flat Set<SidebarSurfaceUnreadKey> owned alongside
manualUnreadSurfaceIdsByWorkspaceId. Update both indexes together in the manual
unread mutation methods, removing keys when indicators are deleted, and pass the
maintained set directly from refreshUnreadPresentation() to sidebarUnread.apply.
Follow the algorithmic-complexity guidance to avoid scanning all workspaces and
surfaces during refreshes.
🪄 Autofix
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 Plus
Run ID: 1d624d50-83f4-41f9-8e9a-81f53549a4fe
📒 Files selected for processing (10)
Packages/macOS/CmuxNotifications/Sources/CmuxNotifications/NotificationDismissalHosting.swiftPackages/macOS/CmuxNotifications/Sources/CmuxNotifications/NotificationDismissalModel.swiftPackages/macOS/CmuxNotifications/Tests/CmuxNotificationsTests/NotificationDismissalModelTests.swiftPackages/macOS/CmuxTerminal/Tests/CmuxTerminalTests/Services/Bell/TerminalBellServiceTests.swiftSources/AppDelegate+DockAttentionRouting.swiftSources/GhosttyTerminalView.swiftSources/TabManager+NotificationDismissalHosting.swiftSources/TerminalNotificationStore.swiftcmuxTests/DockRuntimeParityTests.swiftcmuxTests/GhosttyEnsureFocusWindowActivationTests.swift
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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 `@cmuxTests/DockRuntimeParityTests.swift`:
- Line 530: Update the panel-close step in DockRuntimeParityTests to assert that
destinationDock.closePanel succeeds and that
notificationStore.hasManualUnread(forTabId: destinationWindowID, surfaceId:
panel.id) is false after closing the panel.
In
`@Packages/macOS/CmuxNotifications/Tests/CmuxNotificationsTests/NotificationNavigationCoordinatorTests.swift`:
- Around line 206-218: The failedWindowDockUnreadFallsThrough test currently
lacks a workspace fallback target. Add an unread workspace target to FakeStore,
invoke jumpToLatestUnread, assert that openRouting recorded routing to the
workspace target after the Dock open fails, and retain the assertion that
clearedWindowDockTargets remains empty.
In `@Sources/Workspace`+AttentionFlashRouting.swift:
- Around line 20-26: In Sources/Workspace+AttentionFlashRouting.swift#L20-L26
and Sources/DockSplitStore+AttentionRouting.swift#L60-L74, keep the
manual-unread guard limited to skipping duplicate unread-state writes; remove
its early return so each notification always reaches
mountedTerminal.triggerFlash(reason: .notificationArrival), allowing
triggerFlash to coalesce overlapping animations.
🪄 Autofix
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 Plus
Run ID: ba2f908f-9b93-4b6a-a559-f723a16a881a
📒 Files selected for processing (17)
Packages/macOS/CmuxNotifications/Sources/CmuxNotifications/NotificationNavigationCoordinator.swiftPackages/macOS/CmuxNotifications/Sources/CmuxNotifications/NotificationNavigationStoreReading.swiftPackages/macOS/CmuxNotifications/Sources/CmuxNotifications/NotificationOpenRouting.swiftPackages/macOS/CmuxNotifications/Sources/CmuxNotifications/WindowDockUnreadTarget.swiftPackages/macOS/CmuxNotifications/Tests/CmuxNotificationsTests/FakeOpenRouting.swiftPackages/macOS/CmuxNotifications/Tests/CmuxNotificationsTests/NotificationNavigationCoordinatorTests.swiftPackages/macOS/CmuxTerminal/Sources/CmuxTerminal/Surface/TerminalSurface.swiftSources/AppDelegate+NotificationNavSeams.swiftSources/DockSplitStore+AttentionRouting.swiftSources/DockSplitStore+SurfaceTransfer.swiftSources/DockSplitStore.swiftSources/GhosttyTerminalView.swiftSources/TerminalNotificationStore.swiftSources/Workspace+AttentionFlashRouting.swiftSources/Workspace.swiftcmuxTests/DockRuntimeParityTests.swiftcmuxTests/GhosttyEnsureFocusWindowActivationTests.swift
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 `@cmuxTests/GhosttyEnsureFocusWindowActivationTests.swift`:
- Around line 160-202: The tests mutate process-global application state without
isolation. In cmuxTests/GhosttyEnsureFocusWindowActivationTests.swift:160-202,
make terminalBellInNonKeyCmuxWindowMarksPaneUnread async and execute setup,
assertions, and cleanup inside AppContextSerialGate.withExclusiveAppContext; in
cmuxTests/DockSessionPersistenceTests.swift:308-368, likewise make the affected
test async and place its setup, restoration, assertions, and cleanup inside the
gate. Do not rely on .serialized for this isolation.
🪄 Autofix
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 Plus
Run ID: 6a39cba6-e1d2-4a38-aa09-2a24f6bfdd90
📒 Files selected for processing (4)
Packages/macOS/CmuxNotifications/Tests/CmuxNotificationsTests/NotificationNavigationCoordinatorTests.swiftcmuxTests/DockRuntimeParityTests.swiftcmuxTests/DockSessionPersistenceTests.swiftcmuxTests/GhosttyEnsureFocusWindowActivationTests.swift
Contributor
Author
|
@coderabbitai review |
✅ Action performedReview finished.
|
…ger-raise # Conflicts: # Sources/DockSplitStore+RestoredAgentLifecycle.swift # Sources/DockSplitStore+SessionRestore.swift # Sources/DockSplitStore+SessionSnapshot.swift # Sources/DockSplitStore+SurfaceTransfer.swift # Sources/DockSplitStore.swift
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 372ec73. Configure here.
|
Thank you!!! |
This was referenced Aug 13, 2026
4 tasks done
This was referenced Aug 19, 2026
This was referenced Sep 25, 2026
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
2 participants
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.
Summary
shouldFocus: falseRoot cause and architecture
Both
FeedCoordinator.surfaceBlockingDecisionAttentionandGhosttyApp.ringBell()calledNSApplication.requestUserAttention(.informationalRequest)for background activity. On macOS 26 with Stage Manager, that process-level request can promote cmux's entire window set even though the user is working in another application.This removes process-level AppKit attention from both background paths. Feed attention is lifecycle/sidebar/workspace state. Terminal BEL is split into an audio-only
TerminalBellServiceand a cmux-owned route that resolves the live surface owner, marks inactive panes unread, and flashes without changing selection or focus. The invalid Stage Manager state is unavailable at those boundaries instead of being conditionally suppressed by OS version or focus timing.Regression evidence and verification
c2e9a50145executed the focused suite and failed because the pre-fix Feed path issued one AppKit attention request.1d7626f5a0passed the same selected suite.NSApplicationswizzling because Swift Testing suites run in parallel; the historical red run records the pre-fix AppKit call.cmux-policy-check --mode branch --base origin/mainis clean ate0bdf48c5a.TerminalBellService.swifttypechecked standalone with warnings treated as errors.xcodebuildtests or XCUITests were run, per task constraints. Standalone SwiftPM testing is blocked by the checkout's invalid local GhosttyKit artifact; GitHub Actions owns the app/package test proof.Audit
Fixes #9466
Need help on this PR? Tag
@codesmith-botwith what you need. Autofix is disabled.Note
Medium Risk
Broad changes to notification navigation, unread projections, Dock focus/open, and terminal input attribution across core macOS UX paths; well-covered by package tests but high interaction surface for regressions.
Overview
Removes reliance on process-level AppKit attention for background activity and keeps bells, unread, and navigation inside cmux-owned state so Stage Manager does not promote the whole window set.
Terminal bells split into audio-only
TerminalBellService/TerminalBellPresentationand host-routed visual attention viaonVisualBell; explicit terminal input is attributed only after acceptance (onExplicitInput), including copy mode, mobile gestures, and queued socket input, while workspace font-size shortcuts no longer fan out as per-surface explicit input.Notifications and unread treat per-window Dock surfaces as distinct owners (
WindowDockUnreadTarget,FocusedNotificationTargetworkspace vswindowDock); jump/mark-oldest and open paths can reveal a Dock, exclude an exact Dock target, and returndockUnavailablewhen reveal fails.SidebarUnreadModelis refactored with owner-scoped surface projections, summary/surface observers, and synchronous observation teardown; dismissal can clear store-level manual unread on specific surfaces.liveSurfaceOwnerseparates live ownership from renderable notification opens; Dock session restore/autosave wiresnotificationStoreand manual-unread fingerprints.CI: e2e executed-test parsing sums every XCTest/Swift Testing summary so a later zero-test host teardown cannot zero out a real run.
Reviewed by Cursor Bugbot for commit af914dd. Bugbot is set up for automated code reviews on this repo. Configure here.
Summary by cubic
Stops Stage Manager from promoting cmux windows by removing process-level AppKit attention and keeping all background attention inside cmux. Terminal bells are audio-only; visual BEL/unread route to the exact live owner (workspace or per‑window Dock) with fail‑closed navigation, precise owner‑scoped open/scroll restore, and clear Dock‑availability errors (fixes #9466).
Bug Fixes
unavailable/dockUnavailablewith a localized “Dock could not be revealed”; notification opens target the exact per‑window Dock viaopenWindowDockUnreadand clear only that surface’s manual unread; hidden workspace‑Dock notifications are ignored; manual unread stays constant‑time, survives owner transfer and snapshot/autosave; precise window‑Dock scroll capture/restore; socket focus results includedockUnavailable.CmuxTerminal(TerminalBellService+TerminalBellPresentation); route visual BEL viaonVisualBellto the live owner (incl. projected/mirrored terminals and sidebar focus); suppress when already focused; bound/coalesced notification‑style flashes.onExplicitInput) for copy‑mode, mobile actions, and queued socket input; workspace font‑size shortcuts no longer count as explicit input.Refactors
liveSurfaceOwner);FocusedNotificationTargetdistinguishes workspace vs window‑Dock owners; introducedWindowDockUnreadTarget; focused jump can exclude a specific Dock target; navigation/store seams add window‑Dock unread routes and clearing.SidebarUnreadModel, owner‑keyedSidebarSurfaceUnreadProjection,SurfaceAttentionModel, andWorkspacePanelUnreadModel, with synchronous teardown viaObservationDeliveryLifetime; snapshots carry latest notification id/time; mobile derives from unread snapshots.FeedAttentionTarget; attention lives in cmux lifecycle/sidebar state—noNSApplication.requestUserAttention.Written for commit af914dd. Summary will update on new commits.
Summary by CodeRabbit
New Features
Bug Fixes