Repository navigation
Resolve agent notification targets from live identity at delivery time - #7946
Merged
Merged
Conversation
…7939) Two Claude agents in different workspaces must never see a turn-complete notification, unread ring, or status pill land on the other agent's pane: - CLI: stop must prefer the live agent-pid target over a polluted session record (#7391 drift) and heal the record; a moved pane's notification must follow the surface to its current workspace (#5781); SessionStart must not be poisoned by a stale debug.terminals tty row; legacy routing must survive an app without the resolver method. - App: a queued or synchronously delivered notification addressed with a stale workspace id but a live surface id must be retargeted to the surface's current workspace at delivery time instead of being dropped (async) or misfiled (sync). These tests fail on main; the fix lands in the follow-up commit. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
#7939) One authoritative hook-event -> live surface -> current pane/workspace resolution, shared by the CLI and the app: App: new `agent.resolve_delivery_target` control method backed by AgentDeliveryTargetResolution.swift. A `{pid}` probe resolves the pane that owns the agent process RIGHT NOW from two independent live signals (the process's controlling tty matched against surface pty devices, and the process's own stable CMUX_SURFACE_ID environment re-homed through workspaceContainingPanel); disagreement or ambiguity refuses to guess. A `{surface_id}` probe returns the workspace that currently hosts a known surface. The same resolver now retargets every in-app delivery path: queued notifications follow their surface to its current workspace instead of being dropped on a stale workspace claim, the sync notify path records under the surface's current workspace instead of misfiling, and notification click-through re-homes at click time. CLI: Claude hook routing goes through resolveClaudeHookDeliveryTarget, which puts live process identity above every persisted or spawn-time claim: live pid target first (beats a polluted session record - the issue #7391 resume/tty drift class - and heals it via the existing upserts and active-pointer self-heal), then the #7228 legacy chain unchanged, then moved-pane re-home (issue #5781 class) when the identity surface is no longer listed in the resolved workspace. Explicit --workspace/--surface flags bypass the probes, per-tool PreToolUse skips them for cheapness, and an app without the method degrades to the legacy chain exactly as before. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
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:
📝 WalkthroughWalkthroughClaude hooks now resolve live workspace and surface targets through PID and surface probes. Notification delivery, clearing, policy handling, navigation, sidebar mutations, push payloads, and persisted routing metadata preserve live-owner or source-confined behavior. ChangesLive delivery-target routing
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 (4 errors, 1 warning)
✅ Passed checks (20 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 |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
Contributor
Greptile SummaryThis PR resolves agent notification targets from live pane identity at delivery time. The main changes are:
Confidence Score: 5/5This looks safe to merge.
Important Files Changed
Reviews (51): Last reviewed commit: "Merge remote-tracking branch 'origin/mai..." | Re-trigger Greptile |
![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: 5
🤖 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 `@CLI/CMUXCLI`+ClaudePushNotificationHook.swift:
- Around line 35-46: Update the unresolved-target acknowledgment in the Claude
push notification hook to print the exact literal “OK” instead of using
localized output. Keep the existing telemetry handling and early return
unchanged.
In `@Sources/AgentDeliveryTargetResolution.swift`:
- Around line 159-161: Update the AppDelegate.shared guard in the delivery
target resolution flow to replace the implementation-specific “AppDelegate not
available” message with product-facing recovery text indicating that delivery
target resolution is unavailable and should be retried after cmux finishes
starting, while preserving the existing error code and data.
- Around line 162-163: Update the PID handling in the delivery-target resolution
flow around v2Int and liveAgentDeliveryTarget: require an exact, safely
representable pid_t value before routing, avoiding the trapping pid_t(pid)
conversion. When a PID parameter is supplied but cannot be represented exactly
or is invalid, return invalid_params instead of allowing fallback routing;
preserve normal routing for valid positive PIDs.
In `@Sources/TerminalNotificationQueue.swift`:
- Around line 417-420: Update the target resolution in the queued notification
delivery flow around agentNotificationDeliveryTarget to fail closed when it
returns nil. Remove the fallback to the claimed tabId and surfaceId, and skip
the notification instead; preserve delivery when a live target is returned and
match the existing queued-delivery behavior.
- Line 421: Update the notification discard logic around
TerminalMutationBus.shared.discardPendingNotifications to also discard the
claimed pending key from the original pane location, in addition to target.tabId
and target.surfaceId. Ensure both the old key and the rehomed target key are
removed before delivering the synchronous notification.
🪄 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: d0b375e6-3794-43e8-a112-31c24f2ee423
📒 Files selected for processing (11)
CLI/CMUXCLI+ClaudeHookDeliveryTarget.swiftCLI/CMUXCLI+ClaudePushNotificationHook.swiftCLI/cmux.swiftSources/AgentDeliveryTargetResolution.swiftSources/AppDelegate+NotificationOpen.swiftSources/TerminalController.swiftSources/TerminalNotificationQueue.swiftcmux.xcodeproj/project.pbxprojcmuxTests/AgentNotificationLiveRetargetTests.swiftcmuxTests/ClaudeHookLiveDeliveryTargetTestSupport.swiftcmuxTests/ClaudeHookLiveDeliveryTargetTests.swift
…pace rehome - agent.resolve_delivery_target: convert the caller-supplied pid with pid_t(exactly:) so an out-of-range 64-bit value degrades to the surface/workspace probes instead of trapping (socket-reachable crash). - Sync notification delivery: discard superseded pending notifications by their canonical identity (the surface) so an entry queued under a stale claimed workspace key cannot survive retargeting and duplicate/replace the newer notification. - Claude hook rehome: apply the app's identity-surface ownership answer even when the owning workspace is unchanged — a confirmed identity surface outranks the focused-surface fallback in the same workspace. - Regression tests for all three. Review findings from codex autoreview, Cursor Bugbot, Greptile, CodeRabbit on #7946. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…onEnd cleanup
- Skip the live agent-pid probe on relay-backed socket connections: a
hook running on an SSH/cloud host carries a remote pid that must not be
resolved against the Mac's local process table. UUID-based probes and
the legacy chain still apply.
- Split allowsLiveProbe into allowsPidProbe: PreToolUse still skips the
per-tool pid/tty scan, but the cheap {surface_id} re-home probe stays
enabled so a mid-turn pane move cannot make PreToolUse mutate (and
re-record via upsert) the old workspace's focused pane.
- Route SessionEnd cleanup through the live target resolver: clear
status/pid/notifications on the workspace that owns the pane NOW, not
the consumed record's stale workspace (which also wiped unrelated
panes' notifications there). Fork-parent cleanup uses the shared
resolver too.
- Harness regression tests for the SessionEnd and PreToolUse paths.
Round-2 codex autoreview findings on #7946.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
Sources/AgentDeliveryTargetResolution.swift (1)
27-30: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winMark
AgentDeliveryTargetCandidateasnonisolated.This pure value-model struct with
UUIDfields is returned by thenonisolatedfunctionagentDeliveryTargetMatchingTTYDevice. Per the project's Swift 6 coding guidelines, pure value-model structs should be explicitly markednonisolatedto avoid implicit@MainActorisolation.♻️ Proposed fix
-struct AgentDeliveryTargetCandidate: Equatable { +nonisolated struct AgentDeliveryTargetCandidate: Equatable { let workspaceId: UUID let surfaceId: UUID }🤖 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/AgentDeliveryTargetResolution.swift` around lines 27 - 30, Mark the AgentDeliveryTargetCandidate struct as nonisolated while preserving its Equatable conformance and UUID fields, so the value model can be returned by the nonisolated agentDeliveryTargetMatchingTTYDevice function without implicit actor isolation.Source: Coding guidelines
🤖 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 `@CLI/cmux.swift`:
- Around line 24619-24643: Update the clearAgentSurfaceResumeBinding call to
pass cleanupSurfaceId instead of consumedSession.surfaceId. Preserve the
existing workspaceId selection and the matched live-authoritative fallback
behavior used by sendClaudeFeedTelemetry and the surrounding cleanup flow.
---
Outside diff comments:
In `@Sources/AgentDeliveryTargetResolution.swift`:
- Around line 27-30: Mark the AgentDeliveryTargetCandidate struct as nonisolated
while preserving its Equatable conformance and UUID fields, so the value model
can be returned by the nonisolated agentDeliveryTargetMatchingTTYDevice function
without implicit actor 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: ec689d36-d872-4cda-b6db-0d9cf09d2f70
📒 Files selected for processing (4)
CLI/CMUXCLI+ClaudeHookDeliveryTarget.swiftCLI/cmux.swiftSources/AgentDeliveryTargetResolution.swiftcmuxTests/ClaudeHookLiveDeliveryTargetTests.swift
…oints - notification.create_for_caller: a preferred surface that moved out of the (stale, spawn-time) preferred workspace follows the surface to its current owner instead of falling back to the old workspace's focused pane — plain `cmux notify` from a moved pane hit the wrong pane. - notification.create_for_target / create_for_surface: resolve the surface's current owner before rejecting a stale workspace claim, so moved-pane deliveries retarget instead of erroring (matches the v1 notify_target guard fixed earlier; shared-behavior policy). - Pending-notification discard for a surface now always uses the canonical surface identity: a surface-scoped clear that raced the queue drain could leave a stale-keyed entry that re-delivered (resurrected) the notification right after the user dismissed it. - Regression tests for all three. Round-3 codex autoreview findings on #7946. Note: extends the fix to two sibling entrypoint files (TerminalNotificationCallerResolver.swift, TerminalController+ControlNotificationContext.swift) per the repo's shared-behavior policy — same bug class, same resolver path. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…rehome - Revert live re-homing in notification.create_for_target: it is relay- reachable (RemoteDaemonProxyTunnel pins workspace_id to the relay's owner workspace), and the app-side membership guard is what confines a VM to its authorized workspace — a global surface lookup would allow cross-workspace injection from a leaked pane UUID. Moved-pane re-homing for relayed notifications is deferred until a trusted surface binding exists. Regression test pins the boundary. - notification.create_for_surface (local-only, not relay-reachable): re-home before BOTH rejects, including when the claimed routing workspace was closed, not just when it no longer lists the surface. - Restore exact enqueue-key semantics for the (tabId, surfaceId) discard used by rebindSurfaceNotifications: a surface-wide discard could drop a newer notification legitimately queued under the destination key during a pane move. Surface-scoped CLEARS now use the canonical by-surface discard through a dedicated helper (net-zero growth in the hard-capped store file). Regression tests for both semantics. Round-4 codex autoreview findings on #7946. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
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/TerminalController+ControlNotificationContext.swift (1)
41-86: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick winExtract shared "rehome and deliver" helper to remove duplication.
controlNotificationCreateForSurface(Lines 54-71) andcontrolNotificationCreateForTarget'srehomedDelivery()(Lines 103-120) each independently callAppDelegate.shared?.workspaceContainingPanel(...), thendeliverNotificationSynchronously(...), then build an identical.delivered(workspaceID:surfaceID:windowID:)result. This is the same behavior wired through two separate code paths within one file.♻️ Proposed consolidation
+ private func rehomedNotificationDelivery( + surfaceID: UUID, + excludingWorkspaceID: UUID? = nil, + title: String, + subtitle: String, + body: String + ) -> ControlNotificationTargetedDeliveryResolution? { + guard let owner = AppDelegate.shared?.workspaceContainingPanel( + panelId: surfaceID, + preferredWorkspaceId: excludingWorkspaceID + ), owner.workspace.id != excludingWorkspaceID else { return nil } + deliverNotificationSynchronously( + tabId: owner.workspace.id, + surfaceId: surfaceID, + title: title, + subtitle: subtitle, + body: body + ) + return .delivered( + workspaceID: owner.workspace.id, + surfaceID: surfaceID, + windowID: AppDelegate.shared?.windowId(for: owner.tabManager) + ) + }Then both
controlNotificationCreateForSurface'sguard ws.panels[surfaceID] != nil else { ... }andcontrolNotificationCreateForTarget'srehomedDelivery()nested closure call this one helper instead of duplicating the lookup/deliver/build-result sequence.As per coding guidelines, "Do not wire the same behavior separately through multiple surfaces; use one shared action path." This is a fresh duplication introduced by this diff (two near-identical new branches), not pre-existing debt.
Also applies to: 88-141
🤖 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/TerminalController`+ControlNotificationContext.swift around lines 41 - 86, Extract the shared rehome-and-deliver sequence into a helper used by controlNotificationCreateForSurface and controlNotificationCreateForTarget’s rehomedDelivery(). Have the helper resolve the current owner via workspaceContainingPanel, synchronously deliver the notification, and construct the delivered result including workspace, surface, and window identifiers. Replace both duplicated branches with this helper while preserving their existing fallback behavior when no owner is found.Source: Coding guidelines
🤖 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/TerminalController`+ControlNotificationContext.swift:
- Around line 41-86: Extract the shared rehome-and-deliver sequence into a
helper used by controlNotificationCreateForSurface and
controlNotificationCreateForTarget’s rehomedDelivery(). Have the helper resolve
the current owner via workspaceContainingPanel, synchronously deliver the
notification, and construct the delivered result including workspace, surface,
and window identifiers. Replace both duplicated branches with this helper while
preserving their existing fallback behavior when no owner is found.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 69babdd2-5cf3-41e2-a6c5-07ac9d28695e
📒 Files selected for processing (4)
Sources/TerminalController+ControlNotificationContext.swiftSources/TerminalNotificationCallerResolver.swiftSources/TerminalNotificationQueue.swiftcmuxTests/AgentNotificationLiveRetargetTests.swift
…End scoping - PreToolUse AskUserQuestion/ExitPlanMode: when the resolver returned an authoritative live target, use its surface for the upsert, lifecycle, and needs-input notification instead of re-preferring the persisted session surface — a stale/closed record surface would re-pollute the record and pin the blocking prompt on the wrong pane. - SessionEnd: clear the resume binding on the live pane (and also on the record's surface when it differs, so a misfiled binding cannot survive), and scope the re-homed notification clear to the moved pane instead of wiping sibling panes in the destination workspace. - Regression tests: needs-input uses the resolved surface; re-homed SessionEnd clear is panel-scoped. Round-5 codex autoreview findings on #7946. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…tive The inherited CMUX_SURFACE_ID is spawn-time evidence that can be leaked from the operator's focused pane (documented by testCodexHookOverridesLeakedEnvSurfaceWithProcessTTYBinding). When the controlling-tty lookup fails, promoting an env-only answer to an authoritative pid resolution could override a valid session record and recreate the wrong-pane bug. The env signal now only corroborates the unique kernel-tty match (extracted into the pure agentDeliveryTargetCombining with unit coverage); env-only refuses and the caller falls back to the legacy chain and re-home probes. Round-6 codex autoreview finding on #7946. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ome priority - The async clear_notifications --tab --panel path (enqueueClearNotifications) still discarded pending deliveries by their enqueue-time key, leaving a stale-keyed entry to retarget and resurrect the cleared notification; it now discards by canonical surface identity like the store clear. Regression test added. - When the session record's workspace has died and the legacy chain falls back to the caller-tty workspace, the record surface's current owner now outranks that fallback: a stale tty row could otherwise mark an unrelated pane authoritative and skip the identity-surface re-home. Greptile PR review findings on #7946. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…g cleaned A record polluted to another agent's pane (#7391) whose own session is active there made SessionEnd's shouldApplyClaudeHookVisibleMutation gate look stale, skipping cleanup of the REAL pane — stranding its ring and status after exit. The gate now checks the live-resolved cleanup target (workspace + surface) instead of the consumed record's address; the two only differ in exactly the pollution case, where live is correct. Regression test sessionEndPollutedRecordStillClearsLivePane (foreign active session on the polluted record surface; cleanup must land on the live pid target). The SessionEnd/PreToolUse lifecycle tests move to a new ClaudeHookLifecycleCleanupTests.swift (wired into cmuxTests in project.pbxproj) to keep both suites under the 500-line file budget. Cursor Bugbot finding 3567171933 on #7946. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ive owner A pending async notification queued under a stale claimed tabId DRAINS into the live workspace that owns its surface (#7939 retargeting), so a workspace-wide clear that matched only the enqueue key missed it and the notification reappeared right after the clear. Workspace-wide clears now discard by live delivery target: phase 1 snapshots the pending addresses (sequence + claimed key) under the bus lock, phase 2 resolves each entry's delivery target on the main actor (the same agentNotificationDeliveryTarget used at drain) and discards exactly the snapshotted sequences — entries enqueued between the phases are newer than the clear and deliberately survive. The v1 async clear_notifications --tab path stays enqueue-ordered: FIFO drains the stale entry into the live workspace BEFORE the clear barrier wipes it, so its end state was already clean (regression-pinned). The delivery extensions move from TerminalNotificationQueue.swift into TerminalNotificationLiveRetargetDelivery.swift (wired into the app target) to stay inside the file-length budget. Cursor Bugbot finding "Tab-wide clear misses stale queue" on #7946. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Bugbot is paused — on-demand spend limit reachedBugbot uses usage-based billing for this team and has hit its on-demand spend limit. A team admin can raise the spend limit in the Cursor dashboard, or wait for the next billing cycle to continue. |
…ests All four app-host unit shards failed compiling the test file: WorkspaceRemoteConfiguration lives in the CmuxCore package and the file imported only CmuxControlSocket/Darwin/Foundation/Testing plus the app module (sibling tests that use the type import CmuxCore). Also pin the confined in-flight clear semantics Greptile asked about: an authorized-workspace tab-wide clear cancels a confined in-flight relay delivery even though the request carries a surfaceId (authorizedWorkspaceClearCancelsConfinedInFlightRelayDelivery). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Bugbot is paused — on-demand spend limit reachedBugbot uses usage-based billing for this team and has hit its on-demand spend limit. A team admin can raise the spend limit in the Cursor dashboard, or wait for the next billing cycle to continue. |
app-host shard 4/4 flaked on "Clearing policy work terminates its hook subprocess": the 2s marker wait lost to subprocess spawn + SIGTERM propagation + marker write on a loaded CI runner (passed locally, 76/77 green). Raise the marker/file/notification wait deadlines to 15s — the behavior under test is unchanged and a longer deadline only slows the failure path — and drop the one explicit 2s override. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ntics - The "terminates its hook subprocess" regression asserted subprocess signal delivery, which is not reliable under the CI xctest harness (deterministic 15s timeout there, green locally). The test now asserts the guaranteed contract instead: a cleared in-flight request's result is discarded — the hook is marker-gated to complete strictly AFTER the clear, and its late result must record nothing. Termination stays best-effort and unasserted. - PreToolUse surface re-home no longer skips relay-backed connections: pids are host-local (that restriction stays in liveAgentPidDeliveryTarget), but surface UUIDs are valid across the relay. Over the restricted cloud CLI bridge the resolver method is denied, which classifies as .failed and stays fail-closed exactly as before; relay transports that authorize the resolver now re-home. Greptile findings "Allow relay surface re-home" and CI shard 4/4 on #7946. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Comment on lines
+80
to
+81
| guard let requestSurfaceId = request.surfaceId else { | ||
| if request.tabId == tabId { idsToDiscard.append(id) } |
Contributor
There was a problem hiding this comment.
A workspace-wide clear compares a retargetable request only with its original request.tabId. If a request starts under W1, its surface moves to W2, and W2 is cleared while policy evaluation is running, the request survives because its stored tab is still W1. Final delivery then resolves the surface to W2 and records the notification after the clear. Workspace clears must compare retargetable requests with their current surface owner while keeping confined requests scoped to their authorized workspace.
…on-wrong-pane # Conflicts: # CLI/CMUXCLI+ClaudePushNotificationHook.swift # CLI/cmux.swift # cmux.xcodeproj/project.pbxproj
austinywang
force-pushed
the
issue-7939-notification-wrong-pane
branch
from
July 13, 2026 22:51
9e5c1f4 to
4c6ceb4
Compare
Main replaced the claude-hook success output "OK" with the structured
ack "{}" (printClaudeHookAck). The three hook test helpers added on
this branch still asserted stdout == "OK\n", which failed CI shard 4/4
after merging main. Expect "{}\n" to match the merged convention.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…on-wrong-pane # Conflicts: # .github/workflows/ci.yml
Bugbot is paused — on-demand spend limit reachedBugbot uses usage-based billing for this team and has hit its on-demand spend limit. A team admin can raise the spend limit in the Cursor dashboard, or wait for the next billing cycle to continue. |
hhsw2015
pushed a commit
to hhsw2015/cmux
that referenced
this pull request
Jul 16, 2026
manaflow-ai#7946) * Add failing regression tests for wrong-pane notification attribution (manaflow-ai#7939) Two Claude agents in different workspaces must never see a turn-complete notification, unread ring, or status pill land on the other agent's pane: - CLI: stop must prefer the live agent-pid target over a polluted session record (manaflow-ai#7391 drift) and heal the record; a moved pane's notification must follow the surface to its current workspace (manaflow-ai#5781); SessionStart must not be poisoned by a stale debug.terminals tty row; legacy routing must survive an app without the resolver method. - App: a queued or synchronously delivered notification addressed with a stale workspace id but a live surface id must be retargeted to the surface's current workspace at delivery time instead of being dropped (async) or misfiled (sync). These tests fail on main; the fix lands in the follow-up commit. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * Resolve agent notification targets from live identity at delivery time (manaflow-ai#7939) One authoritative hook-event -> live surface -> current pane/workspace resolution, shared by the CLI and the app: App: new `agent.resolve_delivery_target` control method backed by AgentDeliveryTargetResolution.swift. A `{pid}` probe resolves the pane that owns the agent process RIGHT NOW from two independent live signals (the process's controlling tty matched against surface pty devices, and the process's own stable CMUX_SURFACE_ID environment re-homed through workspaceContainingPanel); disagreement or ambiguity refuses to guess. A `{surface_id}` probe returns the workspace that currently hosts a known surface. The same resolver now retargets every in-app delivery path: queued notifications follow their surface to its current workspace instead of being dropped on a stale workspace claim, the sync notify path records under the surface's current workspace instead of misfiling, and notification click-through re-homes at click time. CLI: Claude hook routing goes through resolveClaudeHookDeliveryTarget, which puts live process identity above every persisted or spawn-time claim: live pid target first (beats a polluted session record - the issue manaflow-ai#7391 resume/tty drift class - and heals it via the existing upserts and active-pointer self-heal), then the manaflow-ai#7228 legacy chain unchanged, then moved-pane re-home (issue manaflow-ai#5781 class) when the identity surface is no longer listed in the resolved workspace. Explicit --workspace/--surface flags bypass the probes, per-tool PreToolUse skips them for cheapness, and an app without the method degrades to the legacy chain exactly as before. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * Address review findings: pid_t overflow, stale queue keys, same-workspace rehome - agent.resolve_delivery_target: convert the caller-supplied pid with pid_t(exactly:) so an out-of-range 64-bit value degrades to the surface/workspace probes instead of trapping (socket-reachable crash). - Sync notification delivery: discard superseded pending notifications by their canonical identity (the surface) so an entry queued under a stale claimed workspace key cannot survive retargeting and duplicate/replace the newer notification. - Claude hook rehome: apply the app's identity-surface ownership answer even when the owning workspace is unchanged — a confirmed identity surface outranks the focused-surface fallback in the same workspace. - Regression tests for all three. Review findings from codex autoreview, Cursor Bugbot, Greptile, CodeRabbit on manaflow-ai#7946. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * Address round-2 review: relay pid namespace, PreToolUse rehome, SessionEnd cleanup - Skip the live agent-pid probe on relay-backed socket connections: a hook running on an SSH/cloud host carries a remote pid that must not be resolved against the Mac's local process table. UUID-based probes and the legacy chain still apply. - Split allowsLiveProbe into allowsPidProbe: PreToolUse still skips the per-tool pid/tty scan, but the cheap {surface_id} re-home probe stays enabled so a mid-turn pane move cannot make PreToolUse mutate (and re-record via upsert) the old workspace's focused pane. - Route SessionEnd cleanup through the live target resolver: clear status/pid/notifications on the workspace that owns the pane NOW, not the consumed record's stale workspace (which also wiped unrelated panes' notifications there). Fork-parent cleanup uses the shared resolver too. - Harness regression tests for the SessionEnd and PreToolUse paths. Round-2 codex autoreview findings on manaflow-ai#7946. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * Address round-3 review: shared live retargeting for all notify entrypoints - notification.create_for_caller: a preferred surface that moved out of the (stale, spawn-time) preferred workspace follows the surface to its current owner instead of falling back to the old workspace's focused pane — plain `cmux notify` from a moved pane hit the wrong pane. - notification.create_for_target / create_for_surface: resolve the surface's current owner before rejecting a stale workspace claim, so moved-pane deliveries retarget instead of erroring (matches the v1 notify_target guard fixed earlier; shared-behavior policy). - Pending-notification discard for a surface now always uses the canonical surface identity: a surface-scoped clear that raced the queue drain could leave a stale-keyed entry that re-delivered (resurrected) the notification right after the user dismissed it. - Regression tests for all three. Round-3 codex autoreview findings on manaflow-ai#7946. Note: extends the fix to two sibling entrypoint files (TerminalNotificationCallerResolver.swift, TerminalController+ControlNotificationContext.swift) per the repo's shared-behavior policy — same bug class, same resolver path. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * Address round-4 review: relay auth boundary, rebind discard, surface rehome - Revert live re-homing in notification.create_for_target: it is relay- reachable (RemoteDaemonProxyTunnel pins workspace_id to the relay's owner workspace), and the app-side membership guard is what confines a VM to its authorized workspace — a global surface lookup would allow cross-workspace injection from a leaked pane UUID. Moved-pane re-homing for relayed notifications is deferred until a trusted surface binding exists. Regression test pins the boundary. - notification.create_for_surface (local-only, not relay-reachable): re-home before BOTH rejects, including when the claimed routing workspace was closed, not just when it no longer lists the surface. - Restore exact enqueue-key semantics for the (tabId, surfaceId) discard used by rebindSurfaceNotifications: a surface-wide discard could drop a newer notification legitimately queued under the destination key during a pane move. Surface-scoped CLEARS now use the canonical by-surface discard through a dedicated helper (net-zero growth in the hard-capped store file). Regression tests for both semantics. Round-4 codex autoreview findings on manaflow-ai#7946. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * Address round-5 review: authoritative surface in needs-input, SessionEnd scoping - PreToolUse AskUserQuestion/ExitPlanMode: when the resolver returned an authoritative live target, use its surface for the upsert, lifecycle, and needs-input notification instead of re-preferring the persisted session surface — a stale/closed record surface would re-pollute the record and pin the blocking prompt on the wrong pane. - SessionEnd: clear the resume binding on the live pane (and also on the record's surface when it differs, so a misfiled binding cannot survive), and scope the re-homed notification clear to the moved pane instead of wiping sibling panes in the destination workspace. - Regression tests: needs-input uses the resolved surface; re-homed SessionEnd clear is panel-scoped. Round-5 codex autoreview findings on manaflow-ai#7946. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * Address round-6 review: env-only pid resolution must not be authoritative The inherited CMUX_SURFACE_ID is spawn-time evidence that can be leaked from the operator's focused pane (documented by testCodexHookOverridesLeakedEnvSurfaceWithProcessTTYBinding). When the controlling-tty lookup fails, promoting an env-only answer to an authoritative pid resolution could override a valid session record and recreate the wrong-pane bug. The env signal now only corroborates the unique kernel-tty match (extracted into the pure agentDeliveryTargetCombining with unit coverage); env-only refuses and the caller falls back to the legacy chain and re-home probes. Round-6 codex autoreview finding on manaflow-ai#7946. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * Address PR review: async clear canonical identity, dead-workspace rehome priority - The async clear_notifications --tab --panel path (enqueueClearNotifications) still discarded pending deliveries by their enqueue-time key, leaving a stale-keyed entry to retarget and resurrect the cleared notification; it now discards by canonical surface identity like the store clear. Regression test added. - When the session record's workspace has died and the legacy chain falls back to the caller-tty workspace, the record surface's current owner now outranks that fallback: a stale tty row could otherwise mark an unrelated pane authoritative and skip the identity-surface re-home. Greptile PR review findings on manaflow-ai#7946. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * Address Cursor review: SessionEnd staleness gate judges the pane being cleaned A record polluted to another agent's pane (manaflow-ai#7391) whose own session is active there made SessionEnd's shouldApplyClaudeHookVisibleMutation gate look stale, skipping cleanup of the REAL pane — stranding its ring and status after exit. The gate now checks the live-resolved cleanup target (workspace + surface) instead of the consumed record's address; the two only differ in exactly the pollution case, where live is correct. Regression test sessionEndPollutedRecordStillClearsLivePane (foreign active session on the polluted record surface; cleanup must land on the live pid target). The SessionEnd/PreToolUse lifecycle tests move to a new ClaudeHookLifecycleCleanupTests.swift (wired into cmuxTests in project.pbxproj) to keep both suites under the 500-line file budget. Cursor Bugbot finding 3567171933 on manaflow-ai#7946. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * Add regression coverage for live-owner workspace clears * Address Cursor review: tab-wide clears resolve each pending entry's live owner A pending async notification queued under a stale claimed tabId DRAINS into the live workspace that owns its surface (manaflow-ai#7939 retargeting), so a workspace-wide clear that matched only the enqueue key missed it and the notification reappeared right after the clear. Workspace-wide clears now discard by live delivery target: phase 1 snapshots the pending addresses (sequence + claimed key) under the bus lock, phase 2 resolves each entry's delivery target on the main actor (the same agentNotificationDeliveryTarget used at drain) and discards exactly the snapshotted sequences — entries enqueued between the phases are newer than the clear and deliberately survive. The v1 async clear_notifications --tab path stays enqueue-ordered: FIFO drains the stale entry into the live workspace BEFORE the clear barrier wipes it, so its end state was already clean (regression-pinned). The delivery extensions move from TerminalNotificationQueue.swift into TerminalNotificationLiveRetargetDelivery.swift (wired into the app target) to stay inside the file-length budget. Cursor Bugbot finding "Tab-wide clear misses stale queue" on manaflow-ai#7946. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com> (cherry picked from commit d64f605)
This was referenced Aug 9, 2026
4 tasks done
This was referenced Sep 25, 2026
This branch was successfully deployed
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 #7939
Symptom
With multiple Claude Code agents in different workspaces, a turn-complete notification (and its unread ring / status pill) intermittently lands on the wrong Claude pane in a different workspace.
Root cause
Hook → pane attribution trusted, in priority order: the persisted session record, a tty-name binding, and spawn-time
CMUX_WORKSPACE_ID/CMUX_SURFACE_IDenv — none verified against where the agent process actually lives at delivery time:debug.terminalstty rows after hibernation auto-resume and by resume-binding drift (Claude auto-resume can write a session's resume binding to the wrong workspace after restored TTY cache drift #7391), and go stale on pane moves (Notifications route to the stale workspace after a pane is moved to another workspace (CMUX_WORKSPACE_ID captured at spawn, never re-resolved) #5781).Fix — one authoritative live resolution at delivery time
App (
Sources/AgentDeliveryTargetResolution.swift, newagent.resolve_delivery_targetcontrol method):{pid}probe: the pane that owns the agent process right now, from two independent live signals — the process's controlling tty device matched against every surface's pty device (unique match only), cross-checked against the process's ownCMUX_SURFACE_IDenvironment (a panel UUID is stable pane identity for the process lifetime; only the workspace binding can go stale) re-homed throughworkspaceContainingPanel. Disagreement or ambiguity refuses to answer rather than guessing.{surface_id}probe: the workspace that currently hosts a known surface (re-homes moved panes).notify_target(sync) records under the surface's current workspace instead of misfiling/erroring, and notification click-through re-homes at click time (Notification click navigates to wrong tab when multiple tabs have Claude Code #2792 class).CLI (
CLI/CMUXCLI+ClaudeHookDeliveryTarget.swift): all Claude hook subcommands (session-start, prompt-submit, stop, notification, push-notification; PreToolUse without probes for per-tool cheapness) route throughresolveClaudeHookDeliveryTarget:Explicit
--workspace/--surfaceflags bypass the probes, and an app without the new method degrades to the legacy chain exactly as before (covered by a regression test).Related issues addressed by the shared fix
Tests (two-commit red/green)
Commit 1 adds failing tests, commit 2 the fix:
cmuxTests/ClaudeHookLiveDeliveryTargetTests.swift(+ harness): stop prefers the live pid target over a polluted record and heals it; stop follows a moved pane to its current workspace; SessionStart is not poisoned by a staledebug.terminalstty row; legacy routing survives an app without the resolver method.cmuxTests/AgentNotificationLiveRetargetTests.swift: queued and sync deliveries retarget to the surface's current workspace (ring lands on the owning pane only); pure unique-tty-match decision table.Also manually verified the three CLI scenarios end-to-end against the built binary with a mock control server (polluted record → live pane + healed store; moved pane → new workspace; method-less app → legacy routing).
Not cleanly testable here: the pid→tty kernel probe (
proc_pidinfoe_tdev vs. live surface pty devices) needs a real agent process inside a real pane; it's covered indirectly by the pure-function decision-table test and the mocked v2 protocol tests, not by an end-to-end unit test.Notes
CLI/cmux.swiftshrinks by ~30 lines;Sources/TerminalController.swiftandSources/TerminalNotificationQueue.swiftare at or below their base line counts. No TSV changes.🤖 Generated with Claude Code
Need help on this PR? Tag
/codesmithwith what you need. Autofix is disabled.Note
High Risk
Touches agent hook routing, notification delivery/clear semantics, and cross-workspace navigation—high user-visible impact if resolution or fail-closed paths regress; relay boundary changes need careful review.
Overview
Fixes wrong-workspace Claude notifications (#7939) by resolving where an agent lives now at delivery time instead of trusting stale session records, tty rows, or spawn env.
App: Adds
agent.resolve_delivery_target(live PID → controlling tty vs pane pty, plus surface → current workspace). Queued/sync notification delivery, clears, and pending-queue discard follow a surface to its current workspace; relaycreate_for_targetstays workspace-bound (retargetsToLiveSurfaceOwner: false). OS notification open/nav carries that flag so clicks can re-home when allowed.CLI: New
resolveClaudeHookDeliveryTargetunifies Claude hooks: prefer live PID probe (skipped on relay / PreToolUse), legacy chain, then surface re-home; SessionEnd cleanup and pane-scopedclear_notificationsuse the live pane.Tests: Broad coverage for live routing, lifecycle cleanup, PID auth, and move/clear races.
Reviewed by Cursor Bugbot for commit 7c5e5c1. Bugbot is set up for automated code reviews on this repo. Configure here.
Summary by cubic
Resolve agent notification targets from live identity at delivery time so rings, status, opens, and clears land on the right pane/workspace across macOS, iOS, and web. Adds a provenance flag so trusted notifications can re‑home while confined ones stay workspace‑scoped; fixes #7939.
agent.resolve_delivery_target; queued/sync deliveries retarget to the surface’s current workspace. Clears use live ownership with generation barriers so pending work can’t resurrect; tab/workspace clears resolve each queued entry’s live target. macOS open routing propagatesretargetsToLiveSurfaceOwner; pid_t overflow guarded; adds localized delivery‑target error messages.PreToolUsekeeps cheap routing but re‑homes by surface;SessionEndcleanup and clears act on the live pane only. Explicit flags bypass; PID auth tightened; legacy path preserved when the app lacks the new method. Success ack aligned to structured{}.retargetsToLiveSurfaceOwner. Confined/relay payloads omitsurfaceId; iOS taps honor the flag and do not cross workspaces; web route policy parses it; macOS open fallback uses it.Written for commit 47b45a1. Summary will update on new commits.
Summary by CodeRabbit
retargetsToLiveSurfaceOwnerto control whether terminal notifications can retarget to the live surface owner across opening and mobile push/tap.