Repository navigation
Inline notification replies (macOS + iOS) with schema-driven reply shapes and a notification debug mode - #8670
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:
📝 WalkthroughWalkthroughThe PR adds inline terminal replies for macOS and iOS notifications, propagates reply shape through terminal and APNs payloads, adds inline Feed question actions, centralizes question parsing, and adds DEBUG notification diagnostics and emission controls. ChangesNotification reply contracts and mobile delivery
Reply-shape propagation
Feed questions and parsing
Debug notification tooling
Estimated code review effort: 5 (Critical) | ~90 minutes Possibly related PRs
Suggested reviewers: Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (7 errors, 1 warning, 1 inconclusive)
✅ Passed checks (16 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 |
Contributor
Greptile SummaryThis PR adds iMessage-style inline notification replies to cmux on macOS and iOS, driven by a
Confidence Score: 5/5Safe to merge. No new correctness bugs introduced; the two pre-existing concerns (dynamic category race and debug seam) were flagged in prior review threads and are unchanged in this iteration. The reply-shape schema is conservative (unknown values coerce to open-only), the macOS surface.send_text path uses the documented mainThreadCallable route and correctly reads ok:true from the custom protocol response, the iOS pending-reply state machine is well-tested and pure, and all string catalog additions carry both en and ja translations. The only new finding is a dead .ready branch in the iOS applyPendingReplyIfReady initial evaluate call — a clarity concern, not a runtime bug. Prior-thread concerns remain open in Sources/Feed/FeedCoordinator.swift (dynamic category cleanup race in cancelNotification) and Sources/TerminalNotificationCallerResolver.swift (widened stringParam/boolParam visibility plus #if DEBUG debug seam). Neither is introduced or worsened by this iteration. Important Files Changed
Sequence DiagramsequenceDiagram
participant User
participant iOS_Notif as iOS Notification Center
participant CmuxAppDelegate as CmuxAppDelegate (iOS)
participant MobilePushCoordinator
participant PendingReplyState
participant MobileShellStore
participant Mac_Web as Web Relay (APNs)
participant Mac_TerminalController as TerminalController (Mac)
participant TerminalNotificationStore
participant NotificationDeliveryCoordinator
participant NotificationCenter as UNUserNotificationCenter (Mac)
Note over Mac_TerminalController,TerminalNotificationStore: macOS notification delivery
Mac_TerminalController->>TerminalNotificationStore: deliverNotificationSynchronously(replyShape:.text)
TerminalNotificationStore->>NotificationCenter: schedule with textReplyCategoryIdentifier
TerminalNotificationStore->>Mac_Web: PhonePushPayload(replyShape:"text")
Mac_Web->>iOS_Notif: APNs push (category: cmux.terminal.reply)
Note over User,MobileShellStore: iOS inline reply flow
User->>iOS_Notif: Types reply text, taps Send
iOS_Notif->>CmuxAppDelegate: didReceive(UNTextInputNotificationResponse)
CmuxAppDelegate->>MobilePushCoordinator: handleReply(text, workspaceId, surfaceId, macDeviceId)
MobilePushCoordinator->>PendingReplyState: park(PendingReply)
MobilePushCoordinator->>MobilePushCoordinator: applyPendingReplyIfReady()
MobilePushCoordinator->>PendingReplyState: evaluate(isTargetReachable, isChannelAvailable)
alt All prerequisites met
PendingReplyState-->>MobilePushCoordinator: .ready(reply)
MobilePushCoordinator->>MobileShellStore: sendTerminalInput(text+CR, workspaceID, terminalID)
else Prerequisites not met
PendingReplyState-->>MobilePushCoordinator: .waiting
Note over MobilePushCoordinator: Retried on bind(store:) or workspacesDidChange()
end
Note over User,NotificationDeliveryCoordinator: macOS inline reply flow
User->>NotificationCenter: Types reply, taps Send (terminal.reply action)
NotificationCenter->>NotificationDeliveryCoordinator: userNotificationCenter didReceive
NotificationDeliveryCoordinator->>Mac_TerminalController: sendReply(text, tabId, surfaceId) via surface.send_text
Mac_TerminalController-->>NotificationDeliveryCoordinator: ok:true / ok:false
alt Send succeeded
NotificationDeliveryCoordinator->>TerminalNotificationStore: markNotificationRead(id)
else Send failed
NotificationDeliveryCoordinator->>Mac_TerminalController: openTerminalNotification (fallback)
end
Reviews (2): 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}
Comment on lines
1121
to
1131
| let center = UNUserNotificationCenter.current() | ||
| center.removePendingNotificationRequestsOffMain(withIdentifiers: [identifier]) | ||
| center.removeDeliveredNotificationsOffMain(withIdentifiers: [identifier]) | ||
| let categoryId = "CMUXFeedQuestion.\(requestId)" | ||
| center.getNotificationCategories { current in | ||
| let categories = Set(current.filter { $0.identifier != categoryId }) | ||
| center.setNotificationCategories(categories) | ||
| } | ||
| } | ||
| } | ||
|
|
Contributor
There was a problem hiding this comment.
cancelNotification's getNotificationCategories callback calls setNotificationCategories directly on whatever queue UNUserNotificationCenter delivers it (typically main, but as a raw DispatchQueue block). registerQuestionCategoryAndAddIfStillAwaiting wraps its write in Task { @MainActor }. Because a DispatchQueue.main block can run between two MainActor-task continuations, if a question notification is resolved concurrently with another being registered, the cleanup callback captures a snapshot before the new category was written — its setNotificationCategories call then lands last and silently clobbers the freshly minted category. The affected question notification would subsequently appear without its inline option buttons, falling through to applicationActivation.activateApplication() for every action.
Comment on lines
+111
to
+130
| return result | ||
| } | ||
|
|
||
| #if DEBUG | ||
| func notificationDebugCallerTarget(params: [String: Any]) -> NotificationDebugTarget? { | ||
| guard let fallbackTabManager = activeTabManagerForCallerNotification() else { return nil } | ||
| let target = Self.callerNotificationTarget( | ||
| fallback: fallbackTabManager, | ||
| preferredWorkspaceId: v2UUID(params, "preferred_workspace_id"), | ||
| preferredSurfaceId: v2UUID(params, "preferred_surface_id"), | ||
| callerTTY: Self.normalizedTTYName(stringParam(params, "caller_tty")), | ||
| preferTTY: boolParam(params, "prefer_tty") ?? false | ||
| ) | ||
| guard let target else { return nil } | ||
| return NotificationDebugTarget( | ||
| workspaceId: target.workspace.id, | ||
| surfaceId: target.surfaceId | ||
| ) | ||
| } | ||
| #endif |
Contributor
There was a problem hiding this comment.
Sources/
notificationDebugCallerTarget is a #if DEBUG-guarded method added to a production source file with no production caller. To make it compile, stringParam and boolParam are widened from private to internal — exactly the "visibility widened together with a wrapper accessor" pattern the no-test-debug-seam rule prohibits. The canonical fix is to isolate the debug target-resolution logic in a dedicated #if DEBUG extension file (as done with NotificationDebugEmitter.swift) and restore private on the helpers.
Rule Used: Do not add new test/debug seams (ForTesting-styl... (source)
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!
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
🤖 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
`@Packages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/MobilePushCoordinator.swift`:
- Around line 415-499: The applyPendingReplyIfReady flow loses the reply when
sendTerminalInput fails because pendingReplyState is cleared before the RPC
completes. Preserve the ready reply through the send attempt and, on failure,
re-park it with bounded retry/backoff (separate from prerequisite waiting), or
invoke an existing user-visible failure path; ensure retries do not overwrite or
silently discard pending replies during bursts.
- Around line 153-163: The reply labels used by the
UNTextInputNotificationAction in MobilePushCoordinator must have concrete
translations. Update the corresponding mobile.push.reply.action,
mobile.push.reply.send, and mobile.push.reply.placeholder entries in
Localizable.xcstrings with actual English and Japanese values, preserving the
existing localization keys and locale structure.
In `@Resources/Localizable.xcstrings`:
- Around line 137-156: Update the localized values for
debug.notification.error.missingKind and debug.notification.error.missingEnabled
to use actionable product-facing wording, such as prompting users to choose a
notification type and whether notifications are enabled. Keep raw field names
out of these user-facing translations; retain them only in internal diagnostics
if needed.
In `@Sources/AppDelegate`+NotificationDeliverySeams.swift:
- Around line 84-105: The notificationDeliverySendTerminalReply method must
resolve the current delivery target before sending when
retargetsToLiveSurfaceOwner is true. Use
AppDelegate.shared.agentNotificationDeliveryTarget(claimedTabId: tabId,
surfaceId: surfaceId), retain the existing surface fallback otherwise, and
include the resolved tabId in the surface.send_text routing parameters so
retargeted replies reach the live workspace.
In `@Sources/Feed/FeedCoordinator.swift`:
- Around line 982-1050: Add coordinator-owned storage for minted per-request
notification categories, and update
registerQuestionCategoryAndAddIfStillAwaiting and cancelNotification to mutate
that storage and derive the complete desired category set before each
setNotificationCategories call. Remove their dependence on independently fetched
getNotificationCategories snapshots for Feed-question categories, while
preserving unrelated categories and pruning entries for requests that are no
longer live so concurrent mint/cancel operations cannot overwrite one another.
In `@Sources/NotificationDebugEmitter.swift`:
- Around line 169-189: Update emitFeed to return false immediately when target
is nil, before calling feedEvent or starting ingestion. Only construct and emit
the WorkstreamEvent when a focused NotificationDebugTarget exists; preserve
optional surfaceId handling for valid workspace-scoped targets.
- Around line 175-177: Replace the Thread.detachNewThread usage in
NotificationDebugEmitter with an actor- or Task-owned asynchronous ingestion
path, avoiding ingestBlocking and native-thread creation for each feed action.
Reuse or add an async FeedCoordinator ingestion method that preserves event
processing while providing task lifecycle and cancellation ownership.
In `@Sources/TerminalNotificationCallerResolver.swift`:
- Around line 114-130: Move notificationDebugCallerTarget and its supporting
stringParam/boolParam parsing used only by DEBUG socket handling into a
dedicated DEBUG-only extension/file, keeping them excluded from production
builds. Restore the normal resolver parsers to private visibility and update the
debug socket code to use the isolated debug support without exposing production
APIs.
🪄 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: bafd68c2-253c-4fc6-b9be-1bb8d0638b42
📒 Files selected for processing (50)
CLI/cmux.swiftPackages/iOS/CmuxMobileShell/Sources/CmuxMobileShell/MobileShellComposite+ExplicitTerminalInput.swiftPackages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/MobilePushCoordinator.swiftPackages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/PendingReply.swiftPackages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/PendingReplyDecision.swiftPackages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/PendingReplyState.swiftPackages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/Resources/Localizable.xcstringsPackages/iOS/CmuxMobileShellUI/Tests/CmuxMobileShellUITests/PendingReplyStateTests.swiftPackages/macOS/CMUXAgentLaunch/Sources/CMUXAgentLaunch/Workstream/WorkstreamQuestionPrompt+Parsing.swiftPackages/macOS/CMUXAgentLaunch/Sources/CMUXAgentLaunch/Workstream/WorkstreamStore.swiftPackages/macOS/CMUXAgentLaunch/Tests/CMUXAgentLaunchTests/Workstream/WorkstreamQuestionPromptParsingTests.swiftPackages/macOS/CmuxControlSocket/Sources/CmuxControlSocket/Coordinator/Notification/ControlCommandCoordinator+Notification.swiftPackages/macOS/CmuxControlSocket/Sources/CmuxControlSocket/Coordinator/Notification/ControlNotificationContext.swiftPackages/macOS/CmuxControlSocket/Tests/CmuxControlSocketTests/ControlCommandContextTestStubs.swiftPackages/macOS/CmuxNotifications/Sources/CmuxNotifications/NotificationDeliveryActionTitles.swiftPackages/macOS/CmuxNotifications/Sources/CmuxNotifications/NotificationDeliveryCoordinator.swiftPackages/macOS/CmuxNotifications/Sources/CmuxNotifications/NotificationDeliveryResponse.swiftPackages/macOS/CmuxNotifications/Sources/CmuxNotifications/NotificationFeedDecision.swiftPackages/macOS/CmuxNotifications/Sources/CmuxNotifications/NotificationTerminalReplying.swiftPackages/macOS/CmuxNotifications/Sources/CmuxNotifications/TerminalNotificationDeliveryIdentifiers.swiftPackages/macOS/CmuxNotifications/Sources/CmuxNotifications/TerminalNotificationReplyShape.swiftPackages/macOS/CmuxNotifications/Sources/CmuxNotifications/UserNotificationCenterConfiguring.swiftPackages/macOS/CmuxNotifications/Tests/CmuxNotificationsTests/NotificationDeliveryCoordinatorTests.swiftPackages/macOS/CmuxNotifications/Tests/CmuxNotificationsTests/NotificationDismissalModelTests.swiftResources/Localizable.xcstringsSources/AgentNotificationDelivery.swiftSources/AppDelegate+NotificationDeliverySeams.swiftSources/AppDelegate.swiftSources/Cloud/PhonePushClient.swiftSources/Cloud/PhonePushPayload.swiftSources/Feed/FeedCoordinator.swiftSources/IrohTransportDebugMenuButtons.swiftSources/NotificationDebugEmitter.swiftSources/NotificationDebugMenuButtons.swiftSources/NotificationDebugTarget.swiftSources/TerminalController+ControlNotificationContext.swiftSources/TerminalController+DebugMethodNames.swiftSources/TerminalController.swiftSources/TerminalNotification.swiftSources/TerminalNotificationCallerResolver.swiftSources/TerminalNotificationLiveRetargetDelivery.swiftSources/TerminalNotificationPolicy.swiftSources/TerminalNotificationQueue.swiftSources/TerminalNotificationStore.swiftcmux.xcodeproj/project.pbxprojcmuxTests/PhonePushPresenceGateTests.swiftios/cmux/CmuxAppDelegate.swiftweb/services/apns/payload.tsweb/services/apns/routePolicy.tsweb/tests/apns.test.ts
Comment on lines
+137
to
+156
| "debug.notification.error.invalidKindOrTarget": { | ||
| "extractionState": "manual", | ||
| "localizations": { | ||
| "en": { "stringUnit": { "state": "translated", "value": "Unknown kind or no notification target" } }, | ||
| "ja": { "stringUnit": { "state": "translated", "value": "不明な種類、または通知対象がありません" } } | ||
| } | ||
| }, | ||
| "debug.notification.error.missingEnabled": { | ||
| "extractionState": "manual", | ||
| "localizations": { | ||
| "en": { "stringUnit": { "state": "translated", "value": "Missing enabled" } }, | ||
| "ja": { "stringUnit": { "state": "translated", "value": "enabled がありません" } } | ||
| } | ||
| }, | ||
| "debug.notification.error.missingKind": { | ||
| "extractionState": "manual", | ||
| "localizations": { | ||
| "en": { "stringUnit": { "state": "translated", "value": "Missing kind" } }, | ||
| "ja": { "stringUnit": { "state": "translated", "value": "kind がありません" } } | ||
| } |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟡 Minor | ⚡ Quick win
Use product-facing wording for debug errors.
These localized errors expose internal request fields (kind and enabled) directly to users. Replace them with actionable wording such as “Choose a notification type” and “Choose whether notifications are enabled”; retain the raw field names only in internal diagnostics.
As per coding guidelines, user-facing errors and command output must not expose implementation details.
🤖 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 `@Resources/Localizable.xcstrings` around lines 137 - 156, Update the localized
values for debug.notification.error.missingKind and
debug.notification.error.missingEnabled to use actionable product-facing
wording, such as prompting users to choose a notification type and whether
notifications are enabled. Keep raw field names out of these user-facing
translations; retain them only in internal diagnostics if needed.
Source: Coding guidelines
Comment on lines
+175
to
+177
| Thread.detachNewThread { | ||
| _ = FeedCoordinator.shared.ingestBlocking(event: event, waitTimeout: 300) | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift
Replace the unowned blocking thread.
Each feed action creates a native thread and blocks in ingestBlocking; “Emit All” creates several concurrently, with no cancellation or lifecycle owner. Route this through an actor- or task-owned async ingestion path instead.
🤖 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/NotificationDebugEmitter.swift` around lines 175 - 177, Replace the
Thread.detachNewThread usage in NotificationDebugEmitter with an actor- or
Task-owned asynchronous ingestion path, avoiding ingestBlocking and
native-thread creation for each feed action. Reuse or add an async
FeedCoordinator ingestion method that preserves event processing while providing
task lifecycle and cancellation ownership.
Source: Coding guidelines
Comment on lines
+114
to
+130
| #if DEBUG | ||
| func notificationDebugCallerTarget(params: [String: Any]) -> NotificationDebugTarget? { | ||
| guard let fallbackTabManager = activeTabManagerForCallerNotification() else { return nil } | ||
| let target = Self.callerNotificationTarget( | ||
| fallback: fallbackTabManager, | ||
| preferredWorkspaceId: v2UUID(params, "preferred_workspace_id"), | ||
| preferredSurfaceId: v2UUID(params, "preferred_surface_id"), | ||
| callerTTY: Self.normalizedTTYName(stringParam(params, "caller_tty")), | ||
| preferTTY: boolParam(params, "prefer_tty") ?? false | ||
| ) | ||
| guard let target else { return nil } | ||
| return NotificationDebugTarget( | ||
| workspaceId: target.workspace.id, | ||
| surfaceId: target.surfaceId | ||
| ) | ||
| } | ||
| #endif |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Keep DEBUG-only helpers out of the production resolver.
notificationDebugCallerTarget is added inline under Sources/, and stringParam/boolParam are widened solely so DEBUG socket handling in another file can call them. Move this debug-only target/parsing support into a dedicated debug extension/file and keep the resolver’s normal parsers private.
As per coding guidelines, “Production Swift source must not add test/debug-only seams” and “A genuinely unavoidable debug-only facility must be isolated in a dedicated debug file or folder.”
Also applies to: 299-305
🤖 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/TerminalNotificationCallerResolver.swift` around lines 114 - 130,
Move notificationDebugCallerTarget and its supporting stringParam/boolParam
parsing used only by DEBUG socket handling into a dedicated DEBUG-only
extension/file, keeping them excluded from production builds. Restore the normal
resolver parsers to private visibility and update the debug socket code to use
the isolated debug support without exposing production APIs.
Source: Coding guidelines
This was referenced Jul 27, 2026
…lies, iOS reply re-park - FeedCoordinator: all CMUXFeedQuestion.* category get->set round trips now append to one MainActor-serialized chain so a mint racing a mint or a cancel can no longer clobber the other's setNotificationCategories write (Greptile P1, CodeRabbit TOCTOU). - Banner text replies resolve the live surface owner via agentNotificationDeliveryTarget before surface.send_text and route with the resolved workspace_id, failing closed when the surface is gone. - iOS: a failed inline-reply RPC re-parks the reply (original createdAt, so the 120s TTL still bounds retries) instead of dropping it; a newer reply parked mid-send still wins. - Debug: caller-target resolution moved behind the shared production seam resolvedCallerNotificationTarget; DEBUG-only param parsing lives in NotificationDebugTarget.swift and the resolver's helpers are private again. debug.notification.emit fails closed for feed kinds without a resolved target. Missing-param errors now say what to pass (EN+JA). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Collaborator
Author
|
Review findings addressed in b766aca:
Skipped with reason:
|
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
Sources/Feed/FeedCoordinator.swift (1)
868-878: 🚀 Performance & Scalability | 🟠 Major | ⚡ Quick winKeep question and permission JSON parsing off the main actor.
inlineQuestionOptions(for:)callsWorkstreamQuestionPrompt.parse(...)from MainActor notification delivery, andpermissionNotificationCategoryId(for:)also calls permission JSON helpers on this path.toolInputJSONis an arbitrary serialized agent payload without a size bound, andparseusesJSONSerialization.jsonObject(data:). Parse once before the MainActor flow into an immutable option/capability snapshot and reuse it through delivery and action registration.🤖 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/Feed/FeedCoordinator.swift` around lines 868 - 878, Move parsing out of the MainActor notification-delivery path: update the caller of inlineQuestionOptions and permissionNotificationCategoryId to parse toolInputJSON once before entering MainActor flow, build an immutable options/capability snapshot, and pass that snapshot through delivery and action registration. Refactor inlineQuestionOptions and permissionNotificationCategoryId to consume the pre-parsed snapshot without invoking WorkstreamQuestionPrompt.parse or permission JSON helpers.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 `@Resources/Localizable.xcstrings`:
- Around line 147-155: Update the localizations for
debug.notification.error.missingEnabled and debug.notification.error.missingKind
to include every supported catalog locale: ar, bs, da, de, en, es, fr, it, ja,
km, ko, nb, pl, pt-BR, ru, th, tr, uk, zh-Hans, and zh-Hant. Preserve the
existing English and Japanese translations and provide corresponding
translations for the remaining locales.
In `@Sources/Feed/FeedCoordinator.swift`:
- Around line 1044-1047: Change liveCategoryIds in the live
notification-category pruning flow to a Set of the mapped waiter request
identifiers, while preserving the existing CMUXFeedQuestion prefix filtering and
membership behavior in the categories filter.
In `@Sources/NotificationDebugTarget.swift`:
- Around line 17-21: Validate supplied preferred_workspace_id and
preferred_surface_id parameters before calling resolvedCallerNotificationTarget,
distinguishing omitted values from malformed, empty, invalid, or unresolved
identifiers. Return invalid_params for any supplied identifier that fails
validation, and only pass validated UUIDs into the existing fallback resolution
flow in NotificationDebugTarget.
In `@Sources/TerminalController.swift`:
- Around line 2238-2241: Update the notification debug handler around
NotificationDebugEmitter.shared.emit so a present force_banner key whose
notificationDebugBoolParam result is nil returns invalid_params. Preserve false
as the default only when force_banner is absent, and continue passing valid
boolean values to emit.
In `@Sources/TerminalNotificationCallerResolver.swift`:
- Around line 79-81: Update the unavailable guard in
TerminalNotificationCallerResolver to keep the "unavailable" error code while
replacing the implementation-specific message with a localized product-level
message such as “Notification target unavailable.” Add or reuse the
corresponding localization catalog entry rather than hardcoding user-facing
text.
---
Outside diff comments:
In `@Sources/Feed/FeedCoordinator.swift`:
- Around line 868-878: Move parsing out of the MainActor notification-delivery
path: update the caller of inlineQuestionOptions and
permissionNotificationCategoryId to parse toolInputJSON once before entering
MainActor flow, build an immutable options/capability snapshot, and pass that
snapshot through delivery and action registration. Refactor
inlineQuestionOptions and permissionNotificationCategoryId to consume the
pre-parsed snapshot without invoking WorkstreamQuestionPrompt.parse or
permission JSON helpers.
🪄 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 Plus
Run ID: 96e073d9-9f12-4167-8361-9d8d95ebe490
📒 Files selected for processing (8)
Packages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/MobilePushCoordinator.swiftResources/Localizable.xcstringsSources/AppDelegate+NotificationDeliverySeams.swiftSources/Feed/FeedCoordinator.swiftSources/NotificationDebugEmitter.swiftSources/NotificationDebugTarget.swiftSources/TerminalController.swiftSources/TerminalNotificationCallerResolver.swift
Comment on lines
+147
to
+155
| "en": { "stringUnit": { "state": "translated", "value": "Pass enabled=true or enabled=false to turn notification debug mode on or off." } }, | ||
| "ja": { "stringUnit": { "state": "translated", "value": "enabled=true または enabled=false を指定して、通知デバッグモードのオン/オフを切り替えてください。" } } | ||
| } | ||
| }, | ||
| "debug.notification.error.missingKind": { | ||
| "extractionState": "manual", | ||
| "localizations": { | ||
| "en": { "stringUnit": { "state": "translated", "value": "Pass kind=<notification kind> to choose which debug notification to emit." } }, | ||
| "ja": { "stringUnit": { "state": "translated", "value": "kind=<通知の種類> を指定して、発行するデバッグ通知を選んでください。" } } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
catalog="Resources/Localizable.xcstrings"
for key in \
"debug.notification.error.missingEnabled" \
"debug.notification.error.missingKind"
do
echo "== $key =="
jq -r --arg key "$key" \
'.strings[$key].localizations // {} | keys[]' "$catalog" | sort
doneRepository: manaflow-ai/cmux
Length of output: 255
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
catalog="Resources/Localizable.xcstrings"
jq -r '
.strings
| to_entries[]
| select(.key | startswith("debug.notification.error.missing"))
| {key, locales: (.value.localizations // {} | keys | sort)}
' "$catalog"
echo "All locales in catalog:"
jq -r '[.strings | to_entries[].value.localizations // {} | keys[]] | unique | sort | .[]' "$catalog"Repository: manaflow-ai/cmux
Length of output: 436
Add all supported locales for the notification debug strings.
debug.notification.error.missingEnabled and debug.notification.error.missingKind are present, but both only include en and ja instead of the catalog’s supported locales: ar, bs, da, de, en, es, fr, it, ja, km, ko, nb, pl, pt-BR, ru, th, tr, uk, zh-Hans, and zh-Hant.
🤖 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 `@Resources/Localizable.xcstrings` around lines 147 - 155, Update the
localizations for debug.notification.error.missingEnabled and
debug.notification.error.missingKind to include every supported catalog locale:
ar, bs, da, de, en, es, fr, it, ja, km, ko, nb, pl, pt-BR, ru, th, tr, uk,
zh-Hans, and zh-Hant. Preserve the existing English and Japanese translations
and provide corresponding translations for the remaining locales.
Source: Coding guidelines
Comment on lines
+1044
to
+1047
| let liveCategoryIds = self.liveWaiterRequestIds().map { "CMUXFeedQuestion.\($0)" } | ||
| var categories = Set(current.filter { category in | ||
| !category.identifier.hasPrefix("CMUXFeedQuestion.") | ||
| || liveCategoryIds.contains(category.identifier) |
There was a problem hiding this comment.
🚀 Performance & Scalability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== locate file =="
git ls-files | rg '(^|/)FeedCoordinator\.swift$' || true
echo "== outline relevant methods =="
ast-grep outline Sources/Feed/FeedCoordinator.swift --match liveWaiterRequestIds --view expanded || true
ast-grep outline Sources/Feed/FeedCoordinator.swift --view expanded | sed -n '1,220p' || true
echo "== relevant lines with context =="
sed -n '1020,1060p' Sources/Feed/FeedCoordinator.swift
echo "== liveWaiterRequestIds usages =="
rg -n "liveWaiterRequestIds|FeedCoordinator|categories.filter|current:" Sources/Feed Sources 2>/dev/null | head -200Repository: manaflow-ai/cmux
Length of output: 25301
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== inspect surrounding function =="
sed -n '960,1075p' Sources/Feed/FeedCoordinator.swift
echo "== deterministic membership scan simulation =="
python3 - <<'PY'
import random
random.seed(0)
categories = 1000
waiters = 1000
prefix = "CMUXFeedQuestion."
array_ids = list(f"{prefix}{i}" for i in range(waiters))
category_ids = [prefix + str(i) for i in range(categories)] + [f"Other.{i}" for i in range(categories)]
keep = [cid for cid in category_ids if not cid.startswith(prefix) or cid in array_ids]
print("category_count=", len(category_ids), "waiter_count=", len(array_ids))
print("comparisons_for_array_contains_per_keep=", category_count:=len(category_ids), "=>", category_count * len(array_ids))
print("matches=", len(keep))
PYRepository: manaflow-ai/cmux
Length of output: 5023
Use a Set for live category IDs.
liveCategoryIds is an array, so contains(category.identifier) scans live waiter IDs for every dynamic category during notification-category pruning. Keep the mapped IDs in a Set before filtering.
Proposed fix
- let liveCategoryIds = self.liveWaiterRequestIds().map { "CMUXFeedQuestion.\($0)" }
+ let liveCategoryIds = Set(
+ self.liveWaiterRequestIds().map { "CMUXFeedQuestion.\($0)" }
+ )📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| let liveCategoryIds = self.liveWaiterRequestIds().map { "CMUXFeedQuestion.\($0)" } | |
| var categories = Set(current.filter { category in | |
| !category.identifier.hasPrefix("CMUXFeedQuestion.") | |
| || liveCategoryIds.contains(category.identifier) | |
| let liveCategoryIds = Set( | |
| self.liveWaiterRequestIds().map { "CMUXFeedQuestion.\($0)" } | |
| ) | |
| var categories = Set(current.filter { category in | |
| !category.identifier.hasPrefix("CMUXFeedQuestion.") | |
| || liveCategoryIds.contains(category.identifier) |
🤖 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/Feed/FeedCoordinator.swift` around lines 1044 - 1047, Change
liveCategoryIds in the live notification-category pruning flow to a Set of the
mapped waiter request identifiers, while preserving the existing
CMUXFeedQuestion prefix filtering and membership behavior in the categories
filter.
Sources: Coding guidelines, Path instructions
Comment on lines
+17
to
+21
| guard let target = resolvedCallerNotificationTarget( | ||
| preferredWorkspaceId: v2UUID(params, "preferred_workspace_id"), | ||
| preferredSurfaceId: v2UUID(params, "preferred_surface_id"), | ||
| callerTTY: notificationDebugStringParam(params, "caller_tty"), | ||
| preferTTY: notificationDebugBoolParam(params, "prefer_tty") ?? false |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n -C 6 '\bv2UUID\s*\(' Sources
rg -n -C 10 'func resolvedCallerNotificationTarget\s*\(' Sources/TerminalNotificationCallerResolver.swift
rg -n -C 8 'notificationDebugCallerTarget|preferred_workspace_id|preferred_surface_id' SourcesRepository: manaflow-ai/cmux
Length of output: 50373
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== NotificationDebugTarget.swift =="
cat -n Sources/NotificationDebugTarget.swift | sed -n '1,140p'
echo
echo "== TerminalControllerV2ParamParsingSupport.swift =="
cat -n Sources/TerminalControllerV2ParamParsingSupport.swift | sed -n '1,180p'
echo
echo "== TerminalNotificationCallerResolver.swift outline/content =="
wc -l Sources/TerminalNotificationCallerResolver.swift
cat -n Sources/TerminalNotificationCallerResolver.swift | sed -n '1,260p'
echo
echo "== targeted debug/target parameter parsing patterns =="
rg -n -C 3 'v2RawString|v2MainSync|v2String|func v2HasNonNullParam|notificationDebugBoolParam|notificationDebugStringParam|resolvedCallerNotificationTarget' Sources/TerminalControllerV2ParamParsingSupport.swift Sources/NotificationDebugTarget.swift Sources/TerminalNotificationCallerResolver.swiftRepository: manaflow-ai/cmux
Length of output: 36067
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== locate debug notify call sites =="
rg -n -C 8 'notificationDebugCallerTarget|debug\.notification|notificationDebug' Sources
echo
echo "== read-only parser behavior for preferred_workspace_id/surface_id edge cases =="
python3 - <<'PY'
from pathlib import Path
p = Path("Sources/TerminalControllerV2ParamParsingSupport.swift")
text = p.read_text()
for i, line in enumerate(text.splitlines(), start=1):
if i == 5 or i == 64 or i == 87:
j = 0
while line[j].isspace():
j += 1
if line[j:j+22] not in ("nonisolated func v2String", "nonisolated func v2RawString", "nonisolated func v2UUID"):
continue
depth = 1
out = []
idx = i - 1
while depth > 0 and idx < len(text.splitlines()):
out.append(f"{idx+1}: {text.splitlines()[idx]}")
for ch in text.splitlines()[idx]:
if ch == "{":
depth += 1
elif ch == "}":
depth -= 1
idx += 1
print(f"\n== {line[j:j+22]} ==")
print("\n".join(out))
PYRepository: manaflow-ai/cmux
Length of output: 16927
Reject malformed target identifiers before fallback resolution.
v2UUID turns missing, empty, invalid UUID, and unresolved preferred_workspace_id/preferred_surface_id values into nil. At Sources/NotificationDebugTarget.swift:16, that is treated as omitted selection from resolvedCallerNotificationTarget, so debug notifications can target the fallback workspace. For supplied values, parse them first and return invalid_params before resolving.
🤖 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/NotificationDebugTarget.swift` around lines 17 - 21, Validate
supplied preferred_workspace_id and preferred_surface_id parameters before
calling resolvedCallerNotificationTarget, distinguishing omitted values from
malformed, empty, invalid, or unresolved identifiers. Return invalid_params for
any supplied identifier that fails validation, and only pass validated UUIDs
into the existing fallback resolution flow in NotificationDebugTarget.
Sources: Path instructions, Learnings
Comment on lines
+2238
to
+2241
| let emitted = NotificationDebugEmitter.shared.emit( | ||
| kind: kind, | ||
| forceBanner: notificationDebugBoolParam(params, "force_banner") ?? false, | ||
| target: notificationDebugCallerTarget(params: params) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Reject an invalid force_banner value.
Line 2240 converts every invalid but present force_banner value to false. Return invalid_params when the key is present and notificationDebugBoolParam returns nil. Keep false as the default only when the key is absent. Otherwise the RPC reports success while ignoring the caller value.
Based on learnings: a present invalid parameter must return invalid_params instead of silently falling back to a default.
🤖 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.swift` around lines 2238 - 2241, Update the
notification debug handler around NotificationDebugEmitter.shared.emit so a
present force_banner key whose notificationDebugBoolParam result is nil returns
invalid_params. Preserve false as the default only when force_banner is absent,
and continue passing valid boolean values to emit.
Source: Learnings
Comment on lines
+79
to
81
| guard activeTabManagerForCallerNotification() != nil else { | ||
| return .err(code: "unavailable", message: "TabManager not available", data: nil) | ||
| } |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Use a localized product-level unavailable message.
"TabManager not available" exposes an internal implementation type in the notification.create_for_caller API response. Return a localized product message such as “Notification target unavailable.” Keep "unavailable" as the machine-readable error classification.
As per coding guidelines, “User-facing errors, alerts, command output, API bodies, and recovery copy must not expose implementation details” and user-facing text must use localized APIs and matching catalogs.
🤖 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/TerminalNotificationCallerResolver.swift` around lines 79 - 81,
Update the unavailable guard in TerminalNotificationCallerResolver to keep the
"unavailable" error code while replacing the implementation-specific message
with a localized product-level message such as “Notification target
unavailable.” Add or reuse the corresponding localization catalog entry rather
than hardcoding user-facing text.
Source: Coding guidelines
…on settings Reports authorizationStatus, alertStyle, and per-surface settings straight from UNUserNotificationCenter so authorization problems on a dev build are diagnosable over the socket. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@Sources/NotificationDebugTarget.swift`:
- Around line 46-53: Update notificationDebugStatus and the
debug.notification.status handling in v2LegacyMainActorResponse to avoid
DispatchSemaphore-based waiting on MainActor. Use an async continuation or move
the notification-settings lookup to a worker path, and keep the RPC response
connected to that non-blocking operation while preserving the existing payload
and timeout behavior.
🪄 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 Plus
Run ID: 043f9134-16ac-44e1-b08d-5c626b98d88f
📒 Files selected for processing (2)
Sources/NotificationDebugTarget.swiftSources/TerminalController.swift
Comment on lines
+46
to
+53
| /// `debug.notification.status` — the system's actual notification settings | ||
| /// for this bundle id, so authorization/style problems are diagnosable from | ||
| /// the socket instead of screenshot archaeology. Blocks the socket worker | ||
| /// on the settings callback (bounded; DEBUG-only diagnostic). | ||
| nonisolated func notificationDebugStatus() -> [String: Any] { | ||
| let semaphore = DispatchSemaphore(value: 0) | ||
| var payload: [String: Any] = ["available": false] | ||
| UNUserNotificationCenter.current().getNotificationSettings { settings in |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== locate files =="
git ls-files | rg '(^|/)NotificationDebugTarget\.swift$|(^|/)TerminalController\.swift$|\.github/review-bot-rules/.*blocking|reliability-single-source-of-truth\.md|algorithmic-complexity\.md' || true
echo "== NotificationDebugTarget outline and relevant lines =="
if [ -f Sources/NotificationDebugTarget.swift ]; then
wc -l Sources/NotificationDebugTarget.swift
ast-grep outline Sources/NotificationDebugTarget.swift --view expanded || true
sed -n '1,140p' Sources/NotificationDebugTarget.swift | nl -ba
fi
echo "== TerminalController references =="
rg -n "notificationDebugStatus|v2LegacyMainActorResponse|debug\.notification\.status|socket worker|MainActor" Sources TerminalController.swift Sources/TerminalController.swift 2>/dev/null || true
echo "== TerminalController relevant section =="
if [ -f Sources/TerminalController.swift ]; then
wc -l Sources/TerminalController.swift
sed -n '2180,2245p' Sources/TerminalController.swift | nl -ba
fi
echo "== Search call sites =="
rg -n "notificationDebugStatus|getNotificationSettings|getNotificationSettings\(" Sources Tests 2>/dev/null || trueRepository: manaflow-ai/cmux
Length of output: 969
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== NotificationDebugTarget relevant sections =="
sed -n '1,120p' Sources/NotificationDebugTarget.swift
echo
echo "== TerminalController relevant references and sections =="
sed -n '2180,2250p' Sources/TerminalController.swift
rg -n "notificationDebugStatus|v2LegacyMainActorResponse|debug\.notification\.status|dispatch_async_to_worker|dispatchAsyncToWorker|`@MainActor`|Actor|MainActor" Sources/TerminalController.swift Sources --glob '*.swift' 2>/dev/null || true
echo
echo "== fast behavioral facts from source text =="
python3 - <<'PY'
from pathlib import Path
nc = Path('Sources/NotificationDebugTarget.swift').read_text()
tc = Path('Sources/TerminalController.swift').read_text()
for phrase in ['DispatchSemaphore(value: 0)', 'DispatchSemaphore.wait(timeout:', 'notificationDebugStatus() -> [String: Any]', 'nonisolated func notificationDebugStatus()']:
print('NC contains', phrase, phrase in nc)
for phrase in ['v2LegacyMainActorResponse', 'notificationDebugStatus()', 'debug.notification.status']:
hits = []
for i,l in enumerate(tc.splitlines(),1):
if phrase in l: hits.append((i,l.strip()))
print('TC matches for', phrase, hits)
PYRepository: manaflow-ai/cmux
Length of output: 50373
Do not bridge the notification-settings callback onto MainActor with a semaphore.
Sources/NotificationDebugTarget.swift waits on DispatchSemaphore.wait(timeout:) for up to three seconds while invoked from Sources/TerminalController.swift’s v2LegacyMainActorResponse. This suspends the MainActor RPC path until the callback fires or the timeout expires. Use an async continuation or a worker path for debug.notification.status; keep the RPC response tied to that non-blocking operation.
🤖 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/NotificationDebugTarget.swift` around lines 46 - 53, Update
notificationDebugStatus and the debug.notification.status handling in
v2LegacyMainActorResponse to avoid DispatchSemaphore-based waiting on MainActor.
Use an async continuation or move the notification-settings lookup to a worker
path, and keep the RPC response connected to that non-blocking operation while
preserving the existing payload and timeout behavior.
Source: Coding guidelines
# Conflicts: # Packages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/MobilePushCoordinator.swift # Packages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/Resources/Localizable.xcstrings # Sources/Cloud/PhonePushClient.swift # Sources/TerminalController+ControlNotificationContext.swift # Sources/TerminalController.swift # cmux.xcodeproj/project.pbxproj # web/services/apns/routePolicy.ts
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
Sources/Cloud/PhonePushClient.swift (1)
350-356: 🗄️ Data Integrity & Integration | 🔵 Trivial | 💤 Low valueNormalize empty dismissal
replyShapevalues.
PhonePushPayload.init(...)accepts an emptyStringforreplyShape, so.dismissenvelopes currently can carryreplyShape: "". Update the Swift envelope constructor to enforcenone/textor omit the field for dismissals, and add route coverage for missing and unknown values.🤖 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/Cloud/PhonePushClient.swift` around lines 350 - 356, The dismiss envelope construction in Sources/Cloud/PhonePushClient.swift:350-356 must not pass an empty replyShape; normalize it to the supported none/text values or omit the field. Update PhonePushPayload.swift:8-9 and 36-50 so initialization and routing accept missing values safely while rejecting or normalizing unknown values, and add coverage for both missing and unknown replyShape inputs.
♻️ Duplicate comments (1)
Sources/Feed/FeedCoordinator.swift (1)
1288-1292: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winUse a
SetforliveCategoryIds.
liveWaiterRequestIds()returns aSet<String>, but.mapproduces an Array.containsthen performs a linear scan for every dynamic category incurrent, making the prune O(categories × liveWaiters). Keep the mapped identifiers in aSet.♻️ Proposed fix
- let liveCategoryIds = self.liveWaiterRequestIds().map { "CMUXFeedQuestion.\($0)" } + let liveCategoryIds = Set( + self.liveWaiterRequestIds().map { "CMUXFeedQuestion.\($0)" } + )As per coding guidelines: "Avoid repeated full scans, sorting, filtering, or per-item nested scans over scalable collections in production code."
🤖 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/Feed/FeedCoordinator.swift` around lines 1288 - 1292, Change liveCategoryIds in the category-pruning flow to a Set by preserving the mapped identifier values in set form, so contains performs constant-time membership checks while filtering current. Keep the existing identifier prefix and category filtering behavior unchanged.Sources: Coding guidelines, Path instructions
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@Packages/macOS/CMUXAgentLaunch/Tests/CMUXAgentLaunchTests/Workstream/WorkstreamQuestionPromptParsingTests.swift`:
- Around line 28-45: Extend WorkstreamQuestionPrompt parsing tests with coverage
for nil and invalid JSON returning an empty result, and for nested questions
using the snake_case multi_select alias. In the multi-question test, verify
fallback identifiers are indexed as q0 and q1 and that the second question
enables multiSelect.
In
`@Packages/macOS/CmuxNotifications/Sources/CmuxNotifications/NotificationDeliveryCoordinator.swift`:
- Around line 48-52: Replace the independent fire-and-forget Task around
category installation with a single serialized notification-category owner that
exclusively performs read-modify-write mutations for both configuration and
dynamic updates. Update the installation API to enqueue its mutation through
that owner and expose completion so callers can await or otherwise observe
completion, and route FeedCoordinator’s dynamic CMuxFeedQuestion.* updates
through the same owner.
In
`@Packages/macOS/CmuxNotifications/Tests/CmuxNotificationsTests/NotificationDeliveryCoordinatorTests.swift`:
- Around line 136-137: Replace the unreliable Task.yield() synchronization in
the test with a real completion signal from FakeNotificationCenter for
notification category installation. Expose and await a continuation or
expectation that is fulfilled after configureUserNotifications completes its
category read/write, then assert center.categories only after that signal, using
a deadline-bounded wait if an expectation is used.
In `@Resources/Localizable.xcstrings`:
- Around line 4-36: Update the new entries under cli.help.notify.reply and
debug.menu.notification.* to include localizations for every supported catalog
locale: ar, bs, da, de, en, es, fr, it, ja, km, ko, nb, pl, pt-BR, ru, th, tr,
uk, zh-Hans, and zh-Hant. Preserve the existing English and Japanese
translations, and add translated stringUnit values for each remaining locale.
In `@Sources/Feed/FeedCoordinator.swift`:
- Around line 1390-1397: Update cancelNotification around the categoryId cleanup
so enqueueQuestionCategoryUpdate is invoked only when the corresponding
CMUXFeedQuestion.<requestId> category was actually minted. Skip the
notificationCategories read and setNotificationCategories round trip for
permission or exit-plan requests without a per-request category.
In `@Sources/TerminalNotificationCallerResolver.swift`:
- Around line 83-86: Update the parameter handling in
TerminalNotificationCallerResolver to distinguish omitted preferred_workspace_id
and preferred_surface_id values from supplied malformed identifiers. Reuse the
strict identifier parser from NotificationDebugTarget, and return invalid_params
when either supplied value fails validation; preserve fallback target selection
only when the identifiers are omitted.
---
Outside diff comments:
In `@Sources/Cloud/PhonePushClient.swift`:
- Around line 350-356: The dismiss envelope construction in
Sources/Cloud/PhonePushClient.swift:350-356 must not pass an empty replyShape;
normalize it to the supported none/text values or omit the field. Update
PhonePushPayload.swift:8-9 and 36-50 so initialization and routing accept
missing values safely while rejecting or normalizing unknown values, and add
coverage for both missing and unknown replyShape inputs.
---
Duplicate comments:
In `@Sources/Feed/FeedCoordinator.swift`:
- Around line 1288-1292: Change liveCategoryIds in the category-pruning flow to
a Set by preserving the mapped identifier values in set form, so contains
performs constant-time membership checks while filtering current. Keep the
existing identifier prefix and category filtering behavior unchanged.
🪄 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: bb440f3c-ecec-49d8-90ef-5560b2c2c453
📒 Files selected for processing (50)
CLI/cmux.swiftPackages/iOS/CmuxMobileShell/Sources/CmuxMobileShell/MobileShellComposite+ExplicitTerminalInput.swiftPackages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/MobilePushCoordinator.swiftPackages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/PendingReply.swiftPackages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/PendingReplyDecision.swiftPackages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/PendingReplyState.swiftPackages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/Resources/Localizable.xcstringsPackages/iOS/CmuxMobileShellUI/Tests/CmuxMobileShellUITests/PendingReplyStateTests.swiftPackages/macOS/CMUXAgentLaunch/Sources/CMUXAgentLaunch/Workstream/WorkstreamQuestionPrompt+Parsing.swiftPackages/macOS/CMUXAgentLaunch/Sources/CMUXAgentLaunch/Workstream/WorkstreamStore.swiftPackages/macOS/CMUXAgentLaunch/Tests/CMUXAgentLaunchTests/Workstream/WorkstreamQuestionPromptParsingTests.swiftPackages/macOS/CmuxControlSocket/Sources/CmuxControlSocket/Coordinator/Notification/ControlCommandCoordinator+Notification.swiftPackages/macOS/CmuxControlSocket/Sources/CmuxControlSocket/Coordinator/Notification/ControlNotificationContext.swiftPackages/macOS/CmuxControlSocket/Tests/CmuxControlSocketTests/ControlCommandContextTestStubs.swiftPackages/macOS/CmuxNotifications/Sources/CmuxNotifications/NotificationDeliveryActionTitles.swiftPackages/macOS/CmuxNotifications/Sources/CmuxNotifications/NotificationDeliveryCoordinator.swiftPackages/macOS/CmuxNotifications/Sources/CmuxNotifications/NotificationDeliveryResponse.swiftPackages/macOS/CmuxNotifications/Sources/CmuxNotifications/NotificationFeedDecision.swiftPackages/macOS/CmuxNotifications/Sources/CmuxNotifications/NotificationTerminalReplying.swiftPackages/macOS/CmuxNotifications/Sources/CmuxNotifications/TerminalNotificationDeliveryIdentifiers.swiftPackages/macOS/CmuxNotifications/Sources/CmuxNotifications/TerminalNotificationReplyShape.swiftPackages/macOS/CmuxNotifications/Sources/CmuxNotifications/UserNotificationCenterConfiguring.swiftPackages/macOS/CmuxNotifications/Tests/CmuxNotificationsTests/NotificationDeliveryCoordinatorTests.swiftResources/Localizable.xcstringsSources/AgentNotificationDelivery.swiftSources/AppDelegate+NotificationDeliverySeams.swiftSources/AppDelegate.swiftSources/Cloud/PhonePushClient.swiftSources/Cloud/PhonePushPayload.swiftSources/Cloud/PhonePushRequestEnvelope.swiftSources/Feed/FeedCoordinator.swiftSources/IrohTransportDebugMenuButtons.swiftSources/NotificationDebugEmitter.swiftSources/NotificationDebugMenuButtons.swiftSources/NotificationDebugTarget.swiftSources/TerminalController+ControlNotificationContext.swiftSources/TerminalController+DebugMethodNames.swiftSources/TerminalController.swiftSources/TerminalNotification.swiftSources/TerminalNotificationCallerResolver.swiftSources/TerminalNotificationLiveRetargetDelivery.swiftSources/TerminalNotificationPolicy.swiftSources/TerminalNotificationQueue.swiftSources/TerminalNotificationStore.swiftcmux.xcodeproj/project.pbxprojcmuxTests/PhonePushPresenceGateTests.swiftios/cmux/CmuxAppDelegate.swiftweb/services/apns/payload.tsweb/services/apns/routePolicy.tsweb/tests/apns.test.ts
Comment on lines
+28
to
+45
| @Test("parses flat questions and defaults multi-select to false") | ||
| func parsesFlatQuestion() throws { | ||
| let parsed = WorkstreamQuestionPrompt.parse(toolInputJSON: #""" | ||
| { | ||
| "prompt": "Choose", | ||
| "options": ["Alpha", "Beta"] | ||
| } | ||
| """#) | ||
|
|
||
| let question = try #require(parsed.first) | ||
| #expect(question.id == "q0") | ||
| #expect(question.prompt == "Choose") | ||
| #expect(!question.multiSelect) | ||
| #expect(question.options == [ | ||
| .init(id: "opt0", label: "Alpha"), | ||
| .init(id: "opt1", label: "Beta"), | ||
| ]) | ||
| } |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
Add coverage for the empty-result and alias paths.
FeedCoordinator.inlineQuestionOptions treats an empty parse result as "no inline options" and falls back to the shared CMUXFeedQuestion category. That fail-closed path is untested. The multi_select snake_case alias and the multi-question fallback id (q1) are also untested.
🧪 Proposed additional tests
`@Test`("returns no prompts for absent or invalid input")
func parsesInvalidInput() {
`#expect`(WorkstreamQuestionPrompt.parse(toolInputJSON: nil).isEmpty)
`#expect`(WorkstreamQuestionPrompt.parse(toolInputJSON: "not json").isEmpty)
}
`@Test`("honors snake_case multi_select and indexed fallback ids")
func parsesSnakeCaseMultiSelect() throws {
let parsed = WorkstreamQuestionPrompt.parse(toolInputJSON: #"""
{"questions": [{"question": "A"}, {"question": "B", "multi_select": true}]}
"""#)
`#expect`(parsed.count == 2)
`#expect`(parsed[0].id == "q0")
`#expect`(parsed[1].id == "q1")
`#expect`(parsed[1].multiSelect)
}🤖 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
`@Packages/macOS/CMUXAgentLaunch/Tests/CMUXAgentLaunchTests/Workstream/WorkstreamQuestionPromptParsingTests.swift`
around lines 28 - 45, Extend WorkstreamQuestionPrompt parsing tests with
coverage for nil and invalid JSON returning an empty result, and for nested
questions using the snake_case multi_select alias. In the multi-question test,
verify fallback identifiers are indexed as q0 and q1 and that the second
question enables multiSelect.
Comment on lines
+48
to
+52
| Task { @MainActor [weak self] in | ||
| guard let self else { return } | ||
| let current = await center.currentNotificationCategories() | ||
| center.setNotificationCategories(current.union(notificationCategories())) | ||
| } |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
Serialize all notification-category mutations through one owner.
Line 48 starts an independent async read-modify-write operation. FeedCoordinator performs another async read-modify-write operation for dynamic CMUXFeedQuestion.* categories. If both reads complete before either write, the last write can remove terminal reply categories or dynamic question categories.
Use one serialized category-update owner for configuration and dynamic updates. Make category installation expose a completion signal through that owner.
As per coding guidelines, use one explicit owner for state and do not create fire-and-forget Task work with meaningful lifecycle.
🤖 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
`@Packages/macOS/CmuxNotifications/Sources/CmuxNotifications/NotificationDeliveryCoordinator.swift`
around lines 48 - 52, Replace the independent fire-and-forget Task around
category installation with a single serialized notification-category owner that
exclusively performs read-modify-write mutations for both configuration and
dynamic updates. Update the installation API to enqueue its mutation through
that owner and expose completion so callers can await or otherwise observe
completion, and route FeedCoordinator’s dynamic CMuxFeedQuestion.* updates
through the same owner.
Source: Coding guidelines
Comment on lines
+4
to
+36
| "cli.help.notify.reply": { | ||
| "extractionState": "manual", | ||
| "localizations": { | ||
| "en": { "stringUnit": { "state": "translated", "value": "--reply Allow a free-text inline reply" } }, | ||
| "ja": { "stringUnit": { "state": "translated", "value": "--reply 自由入力のインライン返信を許可" } } | ||
| } | ||
| }, | ||
| "debug.menu.notification.all": { | ||
| "extractionState": "manual", | ||
| "localizations": { | ||
| "en": { "stringUnit": { "state": "translated", "value": "Emit All" } }, | ||
| "ja": { "stringUnit": { "state": "translated", "value": "すべて送信" } } | ||
| } | ||
| }, | ||
| "debug.menu.notification.cli": { | ||
| "extractionState": "manual", | ||
| "localizations": { | ||
| "en": { "stringUnit": { "state": "translated", "value": "CLI" } }, | ||
| "ja": { "stringUnit": { "state": "translated", "value": "CLI" } } | ||
| } | ||
| }, | ||
| "debug.menu.notification.cliReply": { | ||
| "extractionState": "manual", | ||
| "localizations": { | ||
| "en": { "stringUnit": { "state": "translated", "value": "CLI Reply" } }, | ||
| "ja": { "stringUnit": { "state": "translated", "value": "CLI 返信" } } | ||
| } | ||
| }, | ||
| "debug.menu.notification.emit": { | ||
| "extractionState": "manual", | ||
| "localizations": { | ||
| "en": { "stringUnit": { "state": "translated", "value": "Emit Notification" } }, | ||
| "ja": { "stringUnit": { "state": "translated", "value": "通知を送信" } } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
catalog="Resources/Localizable.xcstrings"
keys="$(jq -r '
.strings
| to_entries[]
| select(
.key == "cli.help.notify.reply"
or (.key | startswith("debug.menu.notification."))
or (.key | startswith("debug.notification."))
or (.key | startswith("feed.notification."))
or (.key | startswith("terminal.notification."))
)
| .key
' "$catalog" | sort -u)"
catalog_locales="$(jq -r '
[.strings[] | (.localizations // {}) | keys[]]
| unique
| sort[]
' "$catalog")"
while IFS= read -r key; do
key_locales="$(jq -r --arg key "$key" '
.strings[$key].localizations // {}
| keys[]
' "$catalog" | sort)"
missing="$(comm -23 \
<(printf '%s\n' "$catalog_locales") \
<(printf '%s\n' "$key_locales"))"
if [[ -n "$missing" ]]; then
printf '%s is missing: %s\n' "$key" "$(tr '\n' ' ' <<< "$missing")"
fi
done <<< "$keys"Repository: manaflow-ai/cmux
Length of output: 198
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
catalog="Resources/Localizable.xcstrings"
echo "== file exists and first relevant keys =="
python3 - <<'PY'
import json
from pathlib import Path
p = Path("Resources/Localizable.xcstrings")
data = json.loads(p.read_text())
targets = []
for key in data.get("strings", {}):
if key == "cli.help.notify.reply" or \
key.startswith("debug.menu.notification.") or \
key.startswith("debug.notification.") or \
key.startswith("feed.notification.") or \
key.startswith("terminal.notification."):
locs = sorted((data["strings"][key]["localizations"] or {}).keys())
targets.append((key, locs))
catalog_loc_set = set()
for locs in ((data["strings"].get(k, {}) or {}).get("localizations") or {} for k in data.get("strings", {})):
catalog_loc_set.update(locs)
for locs in (data.get("metadata", {}) or {}).get("localizations") or []:
catalog_loc_set.update(locs)
print("catalog_locales", sorted(catalog_loc_set))
for key, locs in targets:
print(key, locs)
missing = sorted(set(catalog_loc_set) - set(locs))
if missing:
print(" MISSING", missing)
PY
echo
echo "== changed files/diff target range summary =="
git diff -- Resources/Localizable.xcstrings | head -n 220Repository: manaflow-ai/cmux
Length of output: 9221
Add localization entries for every supported catalog locale.
Resources/Localizable.xcstrings supports ar, bs, da, de, en, es, fr, it, ja, km, ko, nb, pl, pt-BR, ru, th, tr, uk, zh-Hans, and zh-Hant, but the new strings under the affected ranges define translations only for en and ja. This will show English fallback text in those strings for every other supported locale. Add localizations entries for all supported locales.
🤖 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 `@Resources/Localizable.xcstrings` around lines 4 - 36, Update the new entries
under cli.help.notify.reply and debug.menu.notification.* to include
localizations for every supported catalog locale: ar, bs, da, de, en, es, fr,
it, ja, km, ko, nb, pl, pt-BR, ru, th, tr, uk, zh-Hans, and zh-Hant. Preserve
the existing English and Japanese translations, and add translated stringUnit
values for each remaining locale.
Sources: Coding guidelines, Path instructions, Learnings
Comment on lines
83
to
86
| let preferredWorkspaceId = v2UUID(params, "preferred_workspace_id") | ||
| let preferredSurfaceId = v2UUID(params, "preferred_surface_id") | ||
| let callerTTY = Self.normalizedTTYName(stringParam(params, "caller_tty")) | ||
| let callerTTY = stringParam(params, "caller_tty") | ||
| let preferTTY = boolParam(params, "prefer_tty") ?? false |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Reject malformed preferred target identifiers.
Line 83 and Line 84 convert malformed supplied identifiers to nil. The resolver then selects a fallback target at Line 94. A notification with reply_shape: "text" can therefore create a reply action for the wrong terminal.
Distinguish omitted identifiers from supplied invalid identifiers. Return invalid_params for invalid values. Use the same strict parser in Sources/NotificationDebugTarget.swift.
🤖 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/TerminalNotificationCallerResolver.swift` around lines 83 - 86,
Update the parameter handling in TerminalNotificationCallerResolver to
distinguish omitted preferred_workspace_id and preferred_surface_id values from
supplied malformed identifiers. Reuse the strict identifier parser from
NotificationDebugTarget, and return invalid_params when either supplied value
fails validation; preserve fallback target selection only when the identifiers
are omitted.
Source: Path instructions
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
A DEBUG-only button in Settings > Push Alerts schedules a LOCAL notification carrying the same cmux.terminal.reply category and cmux userInfo schema as a Mac-forwarded push, addressed at the selected workspace/terminal. The response path cannot tell local from remote, so the inline Reply UX, parking, and the terminal.input RPC back to the Mac are verifiable on a device without APNs — dev web deployments have no push service configured. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Dev iPhones register APNs tokens with the staging deployment (the device rig default), so a Debug Mac posting pushes to its tag-local localhost port can never deliver: that origin has no token registry, and every forward died queued. Route /api/notifications/* through a push-specific base that mirrors irohBrokerBaseURL: explicit CMUX_PUSH_API_BASE_URL or VM-API overrides win, Debug defaults to staging, Release keeps the production VM-API origin. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The tag rig bakes a localhost CMUX_VM_API_BASE_URL into every Debug bundle's LSEnvironment, so deferring to that knob re-broke the push lane on every fresh build. Only an explicit CMUX_PUSH_API_BASE_URL (env or ~/.cmux-dev.env) overrides the Debug staging default now. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The queue only logged terminal outcomes, so a failing rig read as opaque invalid_response/retry_exhausted lines with no way to tell a redirect from a decode mismatch from a transport error. Log host, HTTP status, byte count, and classification per attempt (never content). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…iew fixes - Question-category serialization now rides main's UserNotificationCenterServing seam: new bounded notificationCategories() read on the service, register/ cancel merge inside the coordinator-owned serialized chain. - Empty exit-plan Revise… no longer approves the plan: it opens the app at the card, matching the empty question-Other… fallback (cursor High), with a regression test. - A failed iOS inline-reply send arms a bounded, cancellable 5s retry through an injected sleep, so a transient RPC failure with unchanged topology cannot strand the re-parked reply until TTL (cursor High). - Release iOS Settings shows only the Allow Push Alerts toggle; the delivery diagnostics, Mac forwarding controls, and test actions are DEBUG-only. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Main added direct test call sites for the control notification entrypoints that predate the reply-shape parameter; a nil default keeps every legacy caller source-compatible while the socket dispatcher still passes the wire value through. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…parked reply while channel is down - The launch/category (re)install now merges live CMUXFeedQuestion.* categories through the new bounded read instead of replacing the whole set, so a re-configure can no longer strip a live question banner's option buttons. Regression test included. - A reply parked because the RPC channel is unavailable arms the same bounded retry ladder as a failed send, so a channel that recovers without emitting a store event cannot strand the reply until TTL. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
# Conflicts: # ghostty
Collaborator
Author
|
Merging per direct directive. Gate math: GitHub required checks green (Greptile, CodeRabbit, cubic, Socket); merge-gate web-typecheck/react-apps-check/web-db-migrations/remote-daemon-tests/workflow-guard-tests all pass; the remaining app-host unit-test shard reds fail identically on a pure-main baseline run (https://github.com/manaflow-ai/cmux/actions/runs/31287521157 — same |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
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 ea468f9. Configure here.
| request: request, | ||
| requestId: requestId, | ||
| effects: effects | ||
| ) |
There was a problem hiding this comment.
Unregistered question category fallback
Medium Severity
When reading or writing notification categories fails, the AskUserQuestion banner is still posted with its dynamic CMUXFeedQuestion.{requestId} category. That category was never registered, so the banner has no option buttons and also never falls back to the static open-only CMUXFeedQuestion category the comment describes.
Reviewed by Cursor Bugbot for commit ea468f9. Configure here.
This was referenced Aug 10, 2026
This was referenced Aug 14, 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
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.
Adds iMessage-style inline replies to cmux notifications, driven by an explicit reply-shape schema so future notification kinds degrade safely to open-only.
macOS.
TerminalNotificationgainsreplyShape(none|text), derived from the agent hook category (turn-complete and idle-reminder are text-replyable) or set explicitly viareply_shapeon thenotification.create*socket verbs andcmux notify --reply. Text-shape banners use a new category whoseUNTextInputNotificationActionroutes the typed text throughsurface.send_textplus Return into the agent's terminal, then marks the notification read; empty or failed replies fall back to the existing open path. Exit-plan banners gain a fourth "Revise…" text action that resolves the parked hook withexitPlan(.manual, feedback:). AskUserQuestion banners with one single-select question and at most 4 options get a per-request dynamic category with one button per option (labels from the agent payload) plus an "Other…" free-text action; multi-select, multi-question, or larger option sets keep the current open-only banner. Dynamic categories are always unioned with the static set, pruned against live waiters at mint time, and removed when the request resolves.iOS. The Mac forwards
replyShapein phone pushes; the web relay picks APNs categorycmux.terminal.replyfor text-shape pushes (unknown shapes coerce to the plain category). The iOS app registers a reply category with a text action; a submitted reply is parked (latest-wins, 120 s TTL) until the store, target Mac, and RPC channel are ready, then sent via explicit-targetterminal.inputplus Return without changing navigation or selection.Debug mode. DEBUG-only
debug.notification.mode/debug.notification.emitsocket verbs and a matching Debug-menu section emit every notification kind (8 pane-banner kinds, 7 Feed decision variants including every fallback case) through the real store andfeed.pushpipelines, so all presentations and reply paths are testable on demand. Feed debug items use real waiters and time out after 300 s.Verification: CmuxNotifications 73 tests, CMUXAgentLaunch 241, CmuxControlSocket 277, web 31 tests plus typecheck, macOS compile, tagged build
nreply, and live socket round-trips of all three Feed decision replies against debug-emitted items. Live banner-click verification on macOS and phone-side reply are pending dogfood. Follow-up (Feed decision pushes to iPhone plus a notification content extension for multi-select) is specced separately.🤖 Generated with Claude Code
Need help on this PR? Tag
/codesmithwith what you need. Autofix is disabled.Summary by cubic
Adds inline text replies to terminal notifications on macOS and iOS with a schema-driven
replyShape, plus a debug mode and diagnostics to test and inspect notification behavior. Improves reliability with merged dynamic categories on (re)install, live retargeting, bounded iOS reply retries (including channel-down cases), and clearer exit‑plan handling.New Features
TerminalNotification.replyShape(none|text) from agent category or via socketreply_shapeand CLIcmux notify --reply. Text-reply banners use a new category/action; replies live‑retarget to the resolved surface owner and fail closed if missing; exit‑plan adds a “Revise…” text action; eligible single‑select AskUserQuestion (≤4 options) gets inline buttons plus “Other…”.replyShape; web switches APNs category betweenCMUX_APNS_REPLY_CATEGORYandCMUX_APNS_CATEGORY. The app registers both with localized “Reply/Send/placeholder”, parks replies (latest wins, 120s TTL), retries failed sends and channel‑down cases after 5s, re‑parks on failure, and ensures a newer reply mid‑send still wins. Adds a DEBUG-only Settings button to schedule a local reply notification.replyShape; route policy parses it; tests cover category selection.notification.create*acceptreply_shape;cmux notify --replyenables text replies.debug.notification.mode,debug.notification.emit, anddebug.notification.statusexercise all kinds and report system settings. Debug Macs route push via shared staging by default withCMUX_PUSH_API_BASE_URLoverride; per‑attempt push logs include host, HTTP status, byte count, and classification.Bug Fixes
CMUXFeedQuestion.*category get→set round trips serialize on one MainActor chain and use a boundednotificationCategories()read; launch/(re)install now merges existing categories so active question buttons aren’t stripped.replyShapedegrades to open‑only.WorkstreamQuestionPrompt.parseaccepts nested and legacy shapes.ControlNotificationContextentrypoints defaultreplyShapeWiretonilfor legacy callers/tests.Written for commit ea468f9. Summary will update on new commits.
Summary by CodeRabbit
--reply.Note
High Risk
Touches notification delivery, terminal input routing, cloud push payloads, and relay authorization boundaries across macOS and iOS; incorrect retargeting or category merge could mis-deliver agent input or strip live notification actions.
Overview
Adds inline text replies to terminal notifications end-to-end, driven by a
replyShapeschema (none/text) so unsupported kinds stay open-only.macOS stores
replyShapeon notifications (from agent categories like turn-complete / idle-reminder, or explicitreply_shapeon control socket /cmux notify --reply). Text-reply banners use a new notification category; typed text goes to the terminal viasurface.send_textplus Return, with live surface retargeting and open-on-empty/failed reply. Feed exit-plan adds a “Revise…” text action; eligible single-select questions (≤4 options) get per-request dynamic categories with option buttons and “Other…”, serialized mint/cancel so categories aren’t clobbered on reinstall.iOS registers reply APNs categories, parks submitted replies until Mac/workspace/surface/RPC are ready, sends via explicit
terminal.inputwithout changing UI selection, and retries transient failures. Phone push carriesreplyShape; Debug Macs default push relay to staging viapushAPIBaseURL.DEBUG adds socket/menu notification emitters and
debug.notification.*verbs to exercise all notification kinds through real pipelines.Reviewed by Cursor Bugbot for commit ea468f9. Bugbot is set up for automated code reviews on this repo. Configure here.