Skip to content

Add cmux wait — block until notification arrives #1

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
shaunr0b wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Add cmux wait — block until notification arrives #1

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
87 changes: 87 additions & 0 deletions CLI/cmux.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1299,6 +1299,70 @@ struct CMUXCLI {
let response = try sendV1Command("clear_notifications", client: client)
print(response)

case "wait":
let timeoutStr = optionValue(commandArgs, name: "--timeout")
let batchStr = optionValue(commandArgs, name: "--batch")
let wsFlag = optionValue(commandArgs, name: "--workspace")
let includeSelf = hasFlag(commandArgs, name: "--include-self")

let overallTimeout: Double = {
guard let s = timeoutStr, let v = Double(s), v > 0 else { return 0 }
return v
}()
let batchMs: Int = {
guard let s = batchStr, let v = Int(s) else { return 200 }
return max(0, v)
}()
Comment on lines +1308 to +1315

@coderabbitai coderabbitai Bot Feb 28, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Validate --timeout/--batch strictly to avoid silent infinite waits.

On Line [1309] and Line [1313], invalid numeric input silently changes behavior (--timeout abc becomes wait-forever). This is a high-impact CLI correctness issue.

Suggested fix 
-            let overallTimeout: Double = {
-                guard let s = timeoutStr, let v = Double(s), v > 0 else { return 0 }
-                return v
-            }()
-            let batchMs: Int = {
-                guard let s = batchStr, let v = Int(s) else { return 200 }
-                return max(0, v)
-            }()
+            let overallTimeout: Double
+            if let timeoutStr {
+                guard let v = Double(timeoutStr), v >= 0 else {
+                    throw CLIError(message: "--timeout must be a non-negative number")
+                }
+                overallTimeout = v
+            } else {
+                overallTimeout = 0
+            }
+
+            let batchMs: Int
+            if let batchStr {
+                guard let v = Int(batchStr), v >= 0 else {
+                    throw CLIError(message: "--batch must be a non-negative integer")
+                }
+                batchMs = v
+            } else {
+                batchMs = 200
+            }
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@CLI/cmux.swift` around lines 1308 - 1315, When parsing timeoutStr and
batchStr for overallTimeout and batchMs, reject invalid numeric input instead of
silently falling back: if timeoutStr is non-nil but cannot be converted to a
positive Double (or is <= 0) print a clear error and exit non-zero (do not treat
as "wait forever"); likewise, if batchStr is non-nil but cannot be converted to
an Int (or is negative) print a clear error and exit; keep the existing default
behavior only when the flag is absent (timeoutStr == nil or batchStr == nil).
Update the parsing code that computes overallTimeout and batchMs to validate the
presence of timeoutStr/batchStr and fail fast on invalid values (referencing
overallTimeout, batchMs, timeoutStr, batchStr).


let workspaceId: String? = wsFlag != nil ? try resolveWorkspaceId(wsFlag, client: client) : nil
let callerSurfaceId: String? = includeSelf ? nil : ProcessInfo.processInfo.environment["CMUX_SURFACE_ID"]

// Install SIGINT handler to print [] and exit cleanly
signal(SIGINT) { _ in
print("[]")
exit(0)
}
Comment on lines +1321 to +1324

@coderabbitai coderabbitai Bot Feb 28, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

find . -name "cmux.swift" -type f

Repository: LegendsOfLearning/cmux

Length of output: 82

🏁 Script executed:

# Find the file and examine the context around lines 1321-1324
if [ -f "CLI/cmux.swift" ]; then
  sed -n '1310,1330p' CLI/cmux.swift | cat -n
fi

Repository: LegendsOfLearning/cmux

Length of output: 1069

🌐 Web query:

Swift signal() handler async-signal-safe print() Darwin POSIX safety

💡 Result:

In a POSIX/Darwin signal()/sigaction() handler, you must only call async-signal-safe functions; anything that might allocate, lock, touch stdio buffers, or run Swift/ObjC runtime code is undefined behavior. (docs.oracle.com)

Is Swift print() async-signal-safe?

No. print() goes through Swift’s standard library I/O machinery and (directly or indirectly) can use non-reentrant facilities (buffering/locks/allocation). It is not async-signal-safe, so calling it in a signal handler is unsafe. (docs.oracle.com)

What can you do to “print” from a signal handler?

If you absolutely must emit something from inside the handler, stick to write(2) to a file descriptor (for example STDERR_FILENO), using a prebuilt/static byte buffer (don’t allocate or build a String in the handler). write() is on the POSIX async-signal-safe list. (docs.oracle.com)

Recommended pattern in Swift on Darwin

Do almost nothing in the handler; instead, notify normal code and do logging/printing there:

  • Set a volatile sig_atomic_t flag in the handler and poll it elsewhere. (cocoadev.github.io)
  • Or, on Apple platforms, prefer DispatchSourceSignal (GCD) to receive signals on a dispatch queue and then safely call Swift/print() there. (blog.smittytone.net)

Also avoid in handlers (common gotchas)

  • Don’t create Swift concurrency work (like Task { ... }) from a handler. (forums.swift.org)
  • Don’t call exit() from a crashing-signal handler; use _exit() if you truly must terminate (and for crash reporting, Apple recommends restoring default handling and returning instead). (developer.apple.com)

Citations:

Replace unsafe print() in signal handler with a safer pattern.

The print() call on line 1322 is not async-signal-safe and can cause crashes or deadlocks. Use one of these patterns instead:

  1. Flag-based approach (simplest): Set a volatile sig_atomic_t flag in the handler and check/handle output in the main loop.
  2. DispatchSourceSignal (preferred on Apple platforms): Receive the signal on a dispatch queue where print() is safe.

Also use _exit() instead of exit() to avoid flushing buffers in the handler.

🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@CLI/cmux.swift` around lines 1321 - 1324, The signal handler for SIGINT
currently calls print() and exit(), both unsafe in a signal handler; replace
this by either (preferred) creating a DispatchSourceSignal for SIGINT and
perform printing and clean shutdown on its dispatch queue, or (simpler) make the
handler set a volatile sig_atomic_t flag (e.g., shutdownRequested) and return,
then check that flag in the main run loop to call print() and perform graceful
shutdown; also replace exit() with _exit() if any immediate process termination
is required inside a handler. Ensure you update the SIGINT setup (the closure
currently passed to signal(SIGINT) { ... }) and reference the new
DispatchSourceSignal or the sig_atomic_t flag and the location where the main
loop observes it.


let overallDeadline: Date? = overallTimeout > 0 ? Date().addingTimeInterval(overallTimeout) : nil

while true {
// Calculate server-side poll time (10s default, clamped to remaining time)
var pollMs = 10000
if let deadline = overallDeadline {
let remaining = deadline.timeIntervalSinceNow
if remaining <= 0 {
print("[]")
break
}
pollMs = min(pollMs, max(100, Int(remaining * 1000)))
}

var params: [String: Any] = [
"poll_timeout_ms": pollMs,
"batch_ms": batchMs,
]
if let ws = workspaceId {
params["workspace_id"] = ws
}
if let sf = callerSurfaceId {
params["exclude_surface_id"] = sf
}

let result = try client.sendV2(method: "notification.wait", params: params)
let timedOut = result["timed_out"] as? Bool ?? true

if !timedOut, let notifications = result["notifications"] as? [[String: Any]], !notifications.isEmpty {
print(jsonString(notifications))
break
}

// Server lease timed out, retry unless overall deadline reached
if let deadline = overallDeadline, Date() >= deadline {
print("[]")
break
}
}

case "claude-hook":
cliTelemetry.breadcrumb("claude-hook.dispatch")
do {
Expand Down Expand Up @@ -4183,6 +4247,28 @@ struct CMUXCLI {

Clear all queued notifications.
"""
case "wait":
return """
Usage: cmux wait [--timeout <seconds>] [--batch <ms>] [--workspace <id|ref>] [--include-self]

Block until a new notification arrives. Prints a JSON array of notifications
and exits with code 0. If --timeout is reached with no notifications, prints
an empty array ([]) and exits with code 0.

Flags:
--timeout <seconds> Overall wait timeout (default: 0 = wait forever)
--batch <ms> After first notification, wait this long to collect
more before returning (default: 200)
--workspace <id|ref> Only wait for notifications in this workspace
--include-self Include notifications from the caller's own surface
Comment on lines +4252 to +4263

@coderabbitai coderabbitai Bot Feb 28, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Update wait help text to include index handles.

Line [4252], Line [4262], and Line [6366] under-document accepted workspace formats. The command accepts index too, not just id/ref.

Suggested doc fix 
-            Usage: cmux wait [--timeout <seconds>] [--batch <ms>] [--workspace <id|ref>] [--include-self]
+            Usage: cmux wait [--timeout <seconds>] [--batch <ms>] [--workspace <id|ref|index>] [--include-self]
@@
-              --workspace <id|ref>   Only wait for notifications in this workspace
+              --workspace <id|ref|index>   Only wait for notifications in this workspace
@@
-          wait [--timeout <seconds>] [--batch <ms>] [--workspace <id|ref>] [--include-self]
+          wait [--timeout <seconds>] [--batch <ms>] [--workspace <id|ref|index>] [--include-self]

Also applies to: 6366-6366

🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@CLI/cmux.swift` around lines 4252 - 4263, The help text for the "cmux wait"
command currently says "--workspace <id|ref>" but the command also accepts an
index; update the usage/help strings to reflect this by changing occurrences of
"<id|ref>" to "<id|ref|index>" (or similar) in the wait help block and the other
documented location mentioned (the string used for the wait command help and any
duplicated help at the other occurrence), ensuring the displayed Flags section
for "--workspace" now lists the accepted formats including index so both help
outputs (the one around the cmux wait usage and the duplicated one) are
consistent.

(by default, notifications from $CMUX_SURFACE_ID
are excluded)

Examples:
cmux wait --timeout 30
cmux wait --workspace workspace:1 --timeout 60
cmux wait --timeout 10 --include-self
"""
case "set-status":
return """
Usage: cmux set-status <key> <value> [flags]
Expand Down Expand Up @@ -6277,6 +6363,7 @@ struct CMUXCLI {
notify --title <text> [--subtitle <text>] [--body <text>] [--workspace <id|ref>] [--surface <id|ref>]
list-notifications
clear-notifications
wait [--timeout <seconds>] [--batch <ms>] [--workspace <id|ref>] [--include-self]
claude-hook <session-start|stop|notification> [--workspace <id|ref>] [--surface <id|ref>]

# sidebar metadata commands
Expand Down
95 changes: 95 additions & 0 deletions Sources/TerminalController.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1157,6 +1157,8 @@ class TerminalController {
return v2Ok(id: id, result: self.v2NotificationList())
case "notification.clear":
return v2Result(id: id, self.v2NotificationClear())
case "notification.wait":
return v2Result(id: id, self.v2NotificationWait(params: params))

// App focus
case "app.focus_override.set":
Expand Down Expand Up @@ -1473,6 +1475,7 @@ class TerminalController {
"notification.create_for_target",
"notification.list",
"notification.clear",
"notification.wait",
"app.focus_override.set",
"app.simulate_active",
"browser.open_split",
Expand Down Expand Up @@ -4708,6 +4711,98 @@ class TerminalController {
return .ok([:])
}

private func v2NotificationWait(params: [String: Any]) -> V2CallResult {
let pollTimeoutMs = min(max(v2Int(params, "poll_timeout_ms") ?? 10000, 100), 12000)
let batchMs = min(max(v2Int(params, "batch_ms") ?? 200, 0), 5000)
let workspaceFilter = v2UUID(params, "workspace_id")
let excludeSurface = v2UUID(params, "exclude_surface_id")
Comment on lines +4717 to +4718

@coderabbitai coderabbitai Bot Feb 28, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Validate filter IDs instead of silently widening the query scope.

On Line 4717 and Line 4718, invalid workspace_id / exclude_surface_id values currently fall back to nil, which can unexpectedly return unfiltered notifications.

🔧 Proposed fix 
 private func v2NotificationWait(params: [String: Any]) -> V2CallResult {
     let pollTimeoutMs = min(max(v2Int(params, "poll_timeout_ms") ?? 10000, 100), 12000)
     let batchMs = min(max(v2Int(params, "batch_ms") ?? 200, 0), 5000)
     let workspaceFilter = v2UUID(params, "workspace_id")
     let excludeSurface = v2UUID(params, "exclude_surface_id")
+    if params["workspace_id"] != nil && workspaceFilter == nil {
+        return .err(code: "invalid_params", message: "Missing or invalid workspace_id", data: nil)
+    }
+    if params["exclude_surface_id"] != nil && excludeSurface == nil {
+        return .err(code: "invalid_params", message: "Missing or invalid exclude_surface_id", data: nil)
+    }
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@Sources/TerminalController.swift` around lines 4717 - 4718, The current use
of v2UUID(params, "workspace_id") and v2UUID(params, "exclude_surface_id") lets
invalid IDs silently become nil and widen the query; change this so that when
the params dictionary contains "workspace_id" or "exclude_surface_id" but v2UUID
returns nil, the handler rejects the request with a validation error (e.g., 400)
instead of treating it as absent. Locate the code that assigns workspaceFilter
and excludeSurface and add a check: if params.keys.contains("workspace_id") &&
workspaceFilter == nil (and similarly for exclude_surface_id) then return/throw
a validation/bad-request response with a clear message about the invalid UUID.
Ensure you reference the same v2UUID helper and preserve existing behavior only
for truly absent keys.


// Snapshot current notification IDs
let existingIds: Set<UUID> = DispatchQueue.main.sync {
Set(TerminalNotificationStore.shared.notifications.map { $0.id })
}

let pollInterval: TimeInterval = 0.05
let deadline = Date().addingTimeInterval(Double(pollTimeoutMs) / 1000.0)
var firstSeen: Date?

while Date() < deadline {
let newNotifications: [TerminalNotification] = DispatchQueue.main.sync {
TerminalNotificationStore.shared.notifications.filter { n in
guard !existingIds.contains(n.id) else { return false }
if let ws = workspaceFilter, n.tabId != ws { return false }
if let ex = excludeSurface, n.surfaceId == ex { return false }
return true
}
}

if !newNotifications.isEmpty {
if firstSeen == nil {
firstSeen = Date()
}
// Continue polling for batch_ms to collect more
if Date().timeIntervalSince(firstSeen!) >= Double(batchMs) / 1000.0 {
// Mark returned notifications as read and build response
let items: [[String: Any]] = DispatchQueue.main.sync {
let ids = newNotifications.map { $0.id }
for id in ids {
TerminalNotificationStore.shared.markRead(id: id)
}
let formatter = ISO8601DateFormatter()
return newNotifications.map { n in
[
"id": n.id.uuidString,
"workspace_id": n.tabId.uuidString,
"surface_id": v2OrNull(n.surfaceId?.uuidString),
"title": n.title,
"subtitle": n.subtitle,
"body": n.body,
"created_at": formatter.string(from: n.createdAt),
"is_read": true,
]
}
}
return .ok(["notifications": items, "timed_out": false])
}
}

_ = RunLoop.current.run(mode: .default, before: Date().addingTimeInterval(pollInterval))
}

// If we collected notifications during the batch window but hit the poll deadline
if firstSeen != nil {
let finalNotifications: [[String: Any]] = DispatchQueue.main.sync {
let current = TerminalNotificationStore.shared.notifications.filter { n in
guard !existingIds.contains(n.id) else { return false }
if let ws = workspaceFilter, n.tabId != ws { return false }
if let ex = excludeSurface, n.surfaceId == ex { return false }
return true
}
let formatter = ISO8601DateFormatter()
for n in current {
TerminalNotificationStore.shared.markRead(id: n.id)
}
return current.map { n in
[
"id": n.id.uuidString,
"workspace_id": n.tabId.uuidString,
"surface_id": v2OrNull(n.surfaceId?.uuidString),
"title": n.title,
"subtitle": n.subtitle,
"body": n.body,
"created_at": formatter.string(from: n.createdAt),
"is_read": true,
]
}
}
if !finalNotifications.isEmpty {
return .ok(["notifications": finalNotifications, "timed_out": false])
}
}

return .ok(["notifications": [] as [Any], "timed_out": true])
}

// MARK: - V2 App Focus Methods

private func v2AppFocusOverride(params: [String: Any]) -> V2CallResult {
Expand Down