Bridge Claude Code PushNotification tool into cmux notifications - #7385
Merged
Merged
Conversation
Claude Code's PushNotification tool delivers via a raw OSC desktop notification and never fires the Notification hook. cmux suppresses raw OSC notifications on surfaces running a hook-integrated agent (suppressesRawTerminalNotification), so inside cmux every PushNotification was silently swallowed. Add a PostToolUse hook (matcher PushNotification) to the wrapper's injected settings and a `cmux hooks claude push-notification` handler that posts the tool's message through notify_target_async. The handler mirrors the tool's own delivery decision via tool_response.localSent (skipped pushes stay skipped) and fails open when an older client omits the structured response. No lifecycle/status change: a mid-turn push must not flip a running pane to "Needs input". 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:
📝 WalkthroughWalkthroughAdds a Claude ChangesPushNotification hook bridge
Estimated code review effort: 4 (Complex) | ~45 minutes Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (1 error)
✅ 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 |
Contributor
Greptile SummaryBridges Claude Code's
Confidence Score: 5/5Safe to merge; the change is additive notification plumbing with no lifecycle or status mutations and conservative skip guards. The new handler follows the established hook pattern exactly, reuses existing workspace/surface resolution and guards, and is covered by seven targeted regression tests now running in CI. The only rough edge is a misleading comment in the wrapper script — the code itself is correct. Resources/bin/cmux-claude-wrapper — the inline comment about localSent being the skip gate is inaccurate and should be corrected to reference disabledReason. Important Files Changed
Sequence Diagram%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
participant CC as Claude Code
participant Wrapper as cmux-claude-wrapper
participant CLI as cmux hooks claude push-notification
participant Socket as cmux daemon
CC->>Wrapper: PushNotification tool call (PostToolUse hook)
Note over Wrapper: async, timeout 10s
Wrapper->>CLI: "stdin: JSON {tool_input.message, tool_response}"
CLI->>CLI: claudePushNotificationMessage() — extract + normalize + cap 240
alt empty message
CLI-->>Wrapper: print OK (breadcrumb: empty)
else "disabledReason == user_present or config_off"
CLI-->>Wrapper: print OK (breadcrumb: skipped)
else staleness / nested-agent guard fails
CLI-->>Wrapper: print OK (breadcrumb: stale / nested-suppressed)
else
CLI->>Socket: notify_target_async workspaceId surfaceId payload
Socket-->>CLI: OK
CLI-->>Wrapper: print OK
end
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
sequenceDiagram
participant CC as Claude Code
participant Wrapper as cmux-claude-wrapper
participant CLI as cmux hooks claude push-notification
participant Socket as cmux daemon
CC->>Wrapper: PushNotification tool call (PostToolUse hook)
Note over Wrapper: async, timeout 10s
Wrapper->>CLI: "stdin: JSON {tool_input.message, tool_response}"
CLI->>CLI: claudePushNotificationMessage() — extract + normalize + cap 240
alt empty message
CLI-->>Wrapper: print OK (breadcrumb: empty)
else "disabledReason == user_present or config_off"
CLI-->>Wrapper: print OK (breadcrumb: skipped)
else staleness / nested-agent guard fails
CLI-->>Wrapper: print OK (breadcrumb: stale / nested-suppressed)
else
CLI->>Socket: notify_target_async workspaceId surfaceId payload
Socket-->>CLI: OK
CLI-->>Wrapper: print OK
end
Reviews (8): 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}
| if let localSent = response["localSent"] as? Bool { | ||
| return localSent | ||
| } | ||
| return response["disabledReason"] == nil |
Contributor
There was a problem hiding this comment.
response["disabledReason"] == nil mishandles a JSON-null disabledReason. Swift's JSONSerialization converts JSON null to NSNull(), so a payload {"disabledReason": null} leaves response["disabledReason"] as Optional<Any>.some(NSNull()), which is not Swift nil. The comparison therefore returns false, suppressing the notification — the opposite of the intended fail-open behaviour when no meaningful disable reason is present. Narrowing to as? String == nil treats both absent keys and non-String values (including NSNull) as "no disable reason".
Suggested change
| return response["disabledReason"] == nil | |
| return response["disabledReason"] as? String == nil |
Contributor
Author
| payload = { | ||
| "session_id": f"sess-{uuid.uuid4().hex}", | ||
| "hook_event_name": "PostToolUse", | ||
| "cwd": "/Users/lawrence/fun", |
Contributor
There was a problem hiding this comment.
cwd field is hardcoded to a developer-specific local path. This does not affect test correctness (cmux only uses cwd for context), but it leaks an author's home directory into the shared test corpus and will look odd in CI diffs. A neutral placeholder is a small, clean fix.
Suggested change
| "cwd": "/Users/lawrence/fun", | |
| "cwd": "/tmp/cmux-test-workspace", |
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
Contributor
Author
There was a problem hiding this comment.
Switched to a neutral /tmp/cmux-test-workspace cwd in fd054f1.
— Claude Code
Comment on lines
+244
to
+257
| # 3. No structured tool_response (older client) -> fail open and bridge. | ||
| proc, commands, workspace_id, surface_id = run_push_notification_hook( | ||
| cli_path, | ||
| push_payload("fallback delivery", None), | ||
| ) | ||
| if proc.returncode != 0: | ||
| print("FAIL: push-notification (fail-open) hook exited nonzero") | ||
| print(f"stdout={proc.stdout!r} stderr={proc.stderr!r} commands={commands!r}") | ||
| return 1 | ||
| expected = f"notify_target_async {workspace_id} {surface_id} Claude Code||fallback delivery" | ||
| if [line for line in commands if line.startswith("notify_target_async ")] != [expected]: | ||
| print("FAIL: missing tool_response should fail open and bridge the message") | ||
| print(f"expected={expected!r} commands={commands!r}") | ||
| return 1 |
Contributor
There was a problem hiding this comment.
disabledReason: null edge
The three existing cases cover localSent: true, localSent: false + disabledReason: string, and no tool_response. There is no case for tool_response present with localSent absent and disabledReason explicitly null in JSON. Due to the NSNull issue in claudePushNotificationWasDelivered, this payload would currently suppress the notification rather than fail open. Adding a fourth assertion here would pin the correct behaviour and catch a regression if the Swift-side fix is ever reverted.
Contributor
Author
![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 `@tests/test_claude_hook_push_notification.py`:
- Line 36: The test is auto-discovering and executing a binary from
world-writable /tmp via the candidate selection logic and later subprocess
execution, which should be avoided. Update the candidate handling in the test
helper and the execution path in the same test to stop globbing /tmp for cmux,
and instead require an explicit binary path such as CMUX_CLI_BIN (or another
explicitly provided build output) before running anything. Keep the existing
executable check, but only on user-supplied paths, and make the test fail fast
if no explicit binary is provided.
🪄 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: 2b58a933-727a-42af-83a2-51c027c44eac
📒 Files selected for processing (6)
.github/workflows/ci.ymlCLI/cmux.swiftResources/bin/cmux-claude-wrapperdocs/agent-hooks.mdtests/test_claude_hook_push_notification.pytests/test_claude_wrapper_hooks.py
Judge note: the missing-message early return was the only guard path without a breadcrumb. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Contributor
Author
|
@codex review |
|
Codex Review: Didn't find any major issues. 🎉 Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
workflow-guard-tests failed the Swift file length budget: cmux.swift was exactly at budget before this branch and the new handler added 104 lines. Pure code motion into CLI/CMUXCLI+ClaudePushNotificationHook.swift (under the 500-line tracking threshold); cmux.swift is back to its budgeted 34499 lines. Widened only the helpers the moved code calls from private to internal. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
JSONSerialization maps JSON null to NSNull, not Swift nil, so claudePushNotificationWasDelivered's disabledReason presence check suppresses a payload with an explicit null reason instead of bridging it. Also drop the /tmp binary glob from the test's CLI resolution (world-writable dir; CI passes CMUX_CLI_BIN) and use a neutral cwd. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Only a string disabledReason marks a skipped push; NSNull and other non-string values fail open so the message is never silently dropped. 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)
CLI/cmux.swift (1)
15821-15839: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winStatic
claude-hook --helpusage text wasn't updated for the newpush-notificationsubcommand.The dynamic help printed by
case "help", "--help", "-h":(line 23787-23789) now listspush-notification, butsubcommandUsage("claude-hook")— shown forcmux claude-hook --helpbefore socket dispatch (seedispatchSubcommandHelp) — still only documentssession-start|active|stop|idle|notification|notify|prompt-submit. This leaves the two help surfaces inconsistent and push-notification undiscoverable via--help.📝 Proposed fix
- Usage: cmux claude-hook <session-start|active|stop|idle|notification|notify|prompt-submit> [flags] + Usage: cmux claude-hook <session-start|active|stop|idle|notification|notify|prompt-submit|push-notification> [flags] Hook for Claude Code integration. Reads JSON from stdin. Subcommands: session-start Signal that a Claude session has started active Alias for session-start stop Signal that a Claude session has stopped idle Alias for stop notification Forward a Claude notification notify Alias for notification prompt-submit Clear notification and set Running on user prompt + push-notification Forward a PushNotification tool call🤖 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 `@CLI/cmux.swift` around lines 15821 - 15839, The static help text returned by subcommandUsage("claude-hook") is missing the new push-notification subcommand, so update the claude-hook usage string to match the dynamic help shown in dispatchSubcommandHelp and the case "help", "--help", "-h" path. Make sure the subcommand list in the claude-hook branch includes push-notification alongside the existing session-start, active, stop, idle, notification, notify, and prompt-submit entries so both help surfaces stay consistent.
🤖 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 `@CLI/cmux.swift`:
- Around line 15821-15839: The static help text returned by
subcommandUsage("claude-hook") is missing the new push-notification subcommand,
so update the claude-hook usage string to match the dynamic help shown in
dispatchSubcommandHelp and the case "help", "--help", "-h" path. Make sure the
subcommand list in the claude-hook branch includes push-notification alongside
the existing session-start, active, stop, idle, notification, notify, and
prompt-submit entries so both help surfaces stay consistent.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: c7711fbb-4134-4cb0-a641-f5f477a1dbe4
📒 Files selected for processing (3)
CLI/CMUXCLI+ClaudePushNotificationHook.swiftCLI/cmux.swiftcmux.xcodeproj/project.pbxproj
Codex review P1: gating the bridge on tool_response.localSent makes it inert whenever Claude's local terminal channel is unavailable or suppressed (mobile-only delivery, or a client honoring the wrapper's notifications_disabled for the tool). Only explicit user-facing skip reasons (user_present, config_off) should suppress the cmux bridge. Also pins config_off as a skip. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Bridge unless disabledReason is user_present or config_off. localSent no longer decides anything: cmux swallows the tool's raw OSC delivery regardless of the local-channel outcome, so keying on it made the bridge inert exactly when the local channel is unavailable (codex review P1). Unknown reasons and unstructured responses still fail open. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Codex review P2: the bridge forwarded tool_input.message unbounded into the notification store, unlike every other hook path which normalizes and caps bodies. Pin the 240-char normalized truncation. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Same bound as every other claude hook body (message-key limit in claudeHookCompactFieldLimit); uses the shared normalizedSingleLine + truncate helpers. 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)
CLI/CMUXCLI+ClaudePushNotificationHook.swift (1)
17-26: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winStale comment contradicts the actual bridging logic.
This comment says bridging mirrors
tool_response.localSent, butclaudePushNotificationShouldBridge(lines 105-127) deliberately does the opposite — it ignoreslocalSentand only suppresses on explicitdisabledReasonvalues (user_present/config_off), per the well-documented rationale directly below it. This looks like a leftover from before the routing refinement described in the commit history; leaving it risks a future maintainer "fixing" the code back to thelocalSent-keyed behavior that was intentionally removed.📝 Proposed fix
- // PostToolUse bridge for Claude Code's PushNotification tool. The - // tool delivers through a raw OSC desktop notification, and cmux - // deliberately drops raw OSC notifications from surfaces running a - // hook-integrated agent (they would duplicate hook notifications), - // so without this bridge every PushNotification is silently - // swallowed inside cmux. The tool's own Notification hook never - // fires for it. Mirror the tool's delivery decision: bridge exactly - // when the tool reports its terminal notification as sent - // (tool_response.localSent); fail open when an older client omits - // the structured response. + // PostToolUse bridge for Claude Code's PushNotification tool. The + // tool delivers through a raw OSC desktop notification, and cmux + // deliberately drops raw OSC notifications from surfaces running a + // hook-integrated agent (they would duplicate hook notifications), + // so without this bridge every PushNotification is silently + // swallowed inside cmux. The tool's own Notification hook never + // fires for it. Bridge unless tool_response.disabledReason is an + // explicit user-facing skip ("user_present"/"config_off"); fail + // open when an older client omits the structured response — see + // claudePushNotificationShouldBridge for the full rationale.🤖 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 `@CLI/CMUXCLI`+ClaudePushNotificationHook.swift around lines 17 - 26, The comment above the Claude PushNotification bridge is stale and no longer matches the logic in claudePushNotificationShouldBridge. Update the comment near CMUXCLI+ClaudePushNotificationHook to describe the current behavior: bridging is based on explicit disabledReason values like user_present and config_off, not on tool_response.localSent. Keep the wording aligned with the rationale in claudePushNotificationShouldBridge so future readers don’t infer the old localSent-based routing.
🤖 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 `@CLI/CMUXCLI`+ClaudePushNotificationHook.swift:
- Around line 17-26: The comment above the Claude PushNotification bridge is
stale and no longer matches the logic in claudePushNotificationShouldBridge.
Update the comment near CMUXCLI+ClaudePushNotificationHook to describe the
current behavior: bridging is based on explicit disabledReason values like
user_present and config_off, not on tool_response.localSent. Keep the wording
aligned with the rationale in claudePushNotificationShouldBridge so future
readers don’t infer the old localSent-based routing.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: c9359da2-0c99-460e-85f4-aed2e120dd34
📒 Files selected for processing (2)
CLI/CMUXCLI+ClaudePushNotificationHook.swifttests/test_claude_hook_push_notification.py
…fication-hook # Conflicts: # cmux.xcodeproj/project.pbxproj
mochiexists
pushed a commit
to mochiexists/cmux-mochi
that referenced
this pull request
Jul 7, 2026
…aflow-ai#7385) * Bridge Claude Code PushNotification tool into cmux notifications Claude Code's PushNotification tool delivers via a raw OSC desktop notification and never fires the Notification hook. cmux suppresses raw OSC notifications on surfaces running a hook-integrated agent (suppressesRawTerminalNotification), so inside cmux every PushNotification was silently swallowed. Add a PostToolUse hook (matcher PushNotification) to the wrapper's injected settings and a `cmux hooks claude push-notification` handler that posts the tool's message through notify_target_async. The handler mirrors the tool's own delivery decision via tool_response.localSent (skipped pushes stay skipped) and fails open when an older client omits the structured response. No lifecycle/status change: a mid-turn push must not flip a running pane to "Needs input". Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * Add breadcrumb on empty push-notification payload Judge note: the missing-message early return was the only guard path without a breadcrumb. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * Move push-notification hook handler out of cmux.swift workflow-guard-tests failed the Swift file length budget: cmux.swift was exactly at budget before this branch and the new handler added 104 lines. Pure code motion into CLI/CMUXCLI+ClaudePushNotificationHook.swift (under the 500-line tracking threshold); cmux.swift is back to its budgeted 34499 lines. Widened only the helpers the moved code calls from private to internal. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * Add failing test: JSON-null disabledReason must fail open JSONSerialization maps JSON null to NSNull, not Swift nil, so claudePushNotificationWasDelivered's disabledReason presence check suppresses a payload with an explicit null reason instead of bridging it. Also drop the /tmp binary glob from the test's CLI resolution (world-writable dir; CI passes CMUX_CLI_BIN) and use a neutral cwd. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * Treat JSON-null disabledReason as fail-open in push-notification bridge Only a string disabledReason marks a skipped push; NSNull and other non-string values fail open so the message is never silently dropped. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * Add failing test: localSent=false without a skip reason must bridge Codex review P1: gating the bridge on tool_response.localSent makes it inert whenever Claude's local terminal channel is unavailable or suppressed (mobile-only delivery, or a client honoring the wrapper's notifications_disabled for the tool). Only explicit user-facing skip reasons (user_present, config_off) should suppress the cmux bridge. Also pins config_off as a skip. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * Gate push-notification bridge on skip reason, not localSent Bridge unless disabledReason is user_present or config_off. localSent no longer decides anything: cmux swallows the tool's raw OSC delivery regardless of the local-channel outcome, so keying on it made the bridge inert exactly when the local channel is unavailable (codex review P1). Unknown reasons and unstructured responses still fail open. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * Add failing test: oversized push-notification body must be truncated Codex review P2: the bridge forwarded tool_input.message unbounded into the notification store, unlike every other hook path which normalizes and caps bodies. Pin the 240-char normalized truncation. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * Normalize and cap push-notification message at 240 chars Same bound as every other claude hook body (message-key limit in claudeHookCompactFieldLimit); uses the shared normalizedSingleLine + truncate helpers. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * ci: retrigger checks on current head The queued CI run for stale head 9ec064c jammed the branch concurrency group; pushes 352471f..625dd8f never got runs. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com> (cherry picked from commit 10a418e)
This was referenced Jul 12, 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.
Claude Code has a
PushNotificationtool the model calls to proactively notify the user ("build failed", "needs your decision"). Inside cmux those pushes were silently swallowed: the tool delivers via a raw OSC desktop notification and never fires theNotificationhook, while cmux suppresses raw OSC notifications on surfaces running a hook-integrated agent (suppressesRawTerminalNotification, https://github.com/manaflow-ai/cmux/blob/main/Sources/Workspace%2BPanelLifecycle.swift). Verified live: aPushNotificationcall reporting "Terminal notification sent" produced nothing incmux list-notifications.Fix: the wrapper's injected settings gain a
PostToolUsehook with matcherPushNotification, calling a newcmux hooks claude push-notificationhandler that posts the tool's message throughnotify_target_asyncwith the usual workspace/surface resolution, stale-session and nested-agent guards. The handler mirrors the tool's own delivery decision via the structuredtool_response(localSent/disabledReason): a push the tool skipped (user active, channel disabled) is not duplicated, and a missing structured response (older clients) fails open so the message is never dropped. No lifecycle or status change, so a mid-turn push cannot flip a running pane to "Needs input".Tests:
tests/test_claude_wrapper_hooks.pynow asserts the PostToolUse matcher and async bridge; newtests/test_claude_hook_push_notification.py(wired into ci.yml) drives the CLI handler against a capturing socket for sent, skipped, and fail-open payloads.🤖 Generated with Claude Code
Need help on this PR? Tag
/codesmithwith what you need. Autofix is disabled.Note
Medium Risk
Changes user-visible notification delivery for Claude in cmux, but reuses existing hook routing/guards and is covered by new regression tests.
Overview
Fixes silent drops of Claude Code’s model-initiated
PushNotificationtool inside cmux: raw OSC delivery is suppressed on hook-integrated surfaces and the tool never hits theNotificationhook, so proactive pushes never reached the notification store.Adds an async
PostToolUsehook (matcherPushNotification) in the Claude wrapper that callscmux hooks claude push-notification. The new handler resolves workspace/surface like other Claude hooks, applies stale-session and nested-agent guards, posts vianotify_target_async, and does not change agent lifecycle or flip a pane to “Needs input”. Bridging is suppressed only whentool_response.disabledReasonisuser_presentorconfig_off; missing or partial responses fail open, andlocalSentis not used as the gate (cmux already swallows OSC either way). Message bodies are normalized and capped at 240 characters.Several CLI helpers are widened to
internal/funcso the new extension can reuse them. CI, wrapper hook tests, a socket-capturing regression suite, and agent-hooks docs are updated.Reviewed by Cursor Bugbot for commit 8a23881. Bugbot is set up for automated code reviews on this repo. Configure here.
Summary by cubic
Bridges Claude Code’s
PushNotificationtool into cmux so model-initiated alerts show as cmux notifications. Delivers unlessdisabledReasonisuser_presentorconfig_off, keeps pane lifecycle untouched, and normalizes + caps bodies at 240 chars.Bug Fixes
PostToolUsematcherPushNotificationand add asynccmux hooks claude push-notification.disabledReasonisuser_present/config_off; ignorelocalSent; fail open on missing/partialtool_response(incl. JSON-null reason); breadcrumb empty payloads.notify_target_asyncwith staleness and nested-agent guards; no “Needs input” flip.localSent=falsewithout reason, and oversized body truncation; wired into CI; docs updated.Refactors
CMUXCLI+ClaudePushNotificationHook.swift; widen minimal helpers for reuse with no behavior change.Written for commit 8a23881. Summary will update on new commits.
Summary by CodeRabbit
push-notificationsubcommand tocmux claude-hook.PushNotificationtool deliveries into cmux notifications via a newPostToolUsepath.PostToolUsepush-notification bridge and skip behavior.