Settle auto-resolved Codex approval notifications

.github/workflows/test-depot.yml

@@ -165,7 +165,7 @@ jobs:
               suite_output=$(run_unit_suite "-only-testing:cmuxTests/$suite") || suite_status=$?
               printf '%s\n' "$suite_output"
               if [ "$suite_status" -ne 0 ]; then return "$suite_status"; fi
-              if ! printf '%s\n' "$suite_output" | grep -Eq 'Test run with [1-9][0-9]* tests|Executed [1-9][0-9]* tests'; then
+              if ! grep -Eq 'Test run with [1-9][0-9]* tests|Executed [1-9][0-9]* tests' <<< "$suite_output"; then
                 echo "No tests executed for $suite" >&2
                 return 1
               fi

CLI/CMUXCLI+AgentHookPayload.swift

@@ -1,3 +1,4 @@
+import Darwin
 import Foundation
 
 extension CMUXCLI {
@@ -352,46 +353,39 @@ extension CMUXCLI {
         maxBytes: UInt64
     ) -> [String]? {
         let expandedPath = NSString(string: path).expandingTildeInPath
-        guard let handle = try? FileHandle(forReadingFrom: URL(fileURLWithPath: expandedPath)) else {
+        // Open and validate one descriptor. A path-only stat followed by a
+        // separate FileHandle open is a TOCTOU window in which a FIFO/device
+        // could replace the transcript and block the synchronous hook.
+        // `O_NOFOLLOW` rejects a final symlink and `O_NONBLOCK` keeps even a
+        // raced special-file open from waiting before fstat can reject it.
+        let descriptor = Darwin.open(
+            expandedPath,
+            O_RDONLY | O_CLOEXEC | O_NOFOLLOW | O_NONBLOCK
+        )
+        guard descriptor >= 0 else { return nil }
+        defer { _ = Darwin.close(descriptor) }
+        var metadata = stat()
+        guard Darwin.fstat(descriptor, &metadata) == 0,
+              (metadata.st_mode & mode_t(S_IFMT)) == mode_t(S_IFREG) else {
             return nil
         }
-        defer { try? handle.close() }
-
-        func isASCIIWhitespace(_ byte: UInt8) -> Bool {
-            byte == 0x09 || byte == 0x0A || byte == 0x0D || byte == 0x20
-        }
-
-        func hasCompleteLineAfterLeadingBoundary(_ data: Data, readStart: UInt64) -> Bool {
-            guard readStart > 0 else { return true }
-            guard let newline = data.firstIndex(of: 0x0A) else { return false }
-            return data[data.index(after: newline)...].contains { !isASCIIWhitespace($0) }
-        }
+        let handle = FileHandle(fileDescriptor: descriptor, closeOnDealloc: false)
 
         let size: UInt64
         do {
             size = try handle.seekToEnd()
-            var readStart = size > maxBytes ? size - maxBytes : 0
+            let readStart = size > maxBytes ? size - maxBytes : 0
             try handle.seek(toOffset: readStart)
-            guard var data = try handle.readToEnd(), !data.isEmpty else {
+            // Read exactly the bounded window. `readToEnd()` can observe bytes
+            // appended after `seekToEnd()` and silently exceed the rollout
+            // budget on a busy Codex transcript.
+            let initialReadLength = Int(min(maxBytes, UInt64(Int.max)))
+            guard var data = try handle.read(upToCount: initialReadLength), !data.isEmpty else {
                 return nil
             }
-            let maxWindowBytes = maxBytes > UInt64.max / 8 ? UInt64.max : maxBytes * 8
-
-            while !hasCompleteLineAfterLeadingBoundary(data, readStart: readStart), readStart > 0 {
-                let currentWindowBytes = size - readStart
-                guard currentWindowBytes < maxWindowBytes else { break }
-                let remainingWindowBytes = maxWindowBytes - currentWindowBytes
-                let expansionBytes = min(readStart, maxBytes, remainingWindowBytes)
-                guard expansionBytes > 0 else { break }
-
-                readStart -= expansionBytes
-                try handle.seek(toOffset: readStart)
-                guard let expandedData = try handle.readToEnd(), !expandedData.isEmpty else {
-                    return nil
-                }
-                data = expandedData
-            }
-
+            // Keep the hook read strictly bounded. If the window begins in the
+            // middle of a very long line, dropping that partial line is safer
+            // than expanding into an unbounded transcript prefix.
             if readStart > 0, let newline = data.firstIndex(of: 0x0A) {
                 data.removeSubrange(data.startIndex...newline)
             }

CLI/CMUXCLI+CodexFireAndForgetHooks.swift

@@ -271,9 +271,11 @@ extension CMUXCLI {
     }
 
     /// Removes obsolete regular files only when their names prove cmux ownership.
-    /// Live Codex sessions may still hold paths from another tagged build, and
-    /// concurrent launches can briefly overlap script generation, so collection
-    /// waits until no Codex process is running and leaves recent files alone.
+    /// This runs only during an explicit hook install, never on wrapper launch
+    /// or automatic reconciliation. Since an older immutable script may still
+    /// be referenced by a long-lived Codex process, fail closed whenever any
+    /// Codex process is running; the process probe is intentionally outside the
+    /// launch path.
     static func garbageCollectCodexHookScripts(retaining filenames: Set<String>) {
         guard !hasRunningCodexProcess(),
               let directory = codexHookScriptsDirectory(),
@@ -285,8 +287,9 @@ extension CMUXCLI {
             return
         }
 
-        let newestRemovableDate = Date().addingTimeInterval(-60)
-        for url in contents where !filenames.contains(url.lastPathComponent) {
+        let newestRemovableDate = Date().addingTimeInterval(-24 * 60 * 60)
+        let removableCandidates = contents.compactMap { url -> (url: URL, date: Date)? in
+            guard !filenames.contains(url.lastPathComponent) else { return nil }
             let values = try? url.resourceValues(forKeys: [
                 .contentModificationDateKey,
                 .isRegularFileKey,
@@ -295,13 +298,28 @@ extension CMUXCLI {
                   values?.isRegularFile == true,
                   let modificationDate = values?.contentModificationDate,
                   modificationDate < newestRemovableDate else {
-                continue
+                return nil
             }
-            try? FileManager.default.removeItem(at: url)
+            return (url, modificationDate)
+        }
+        // Keep cleanup bounded if a damaged or very old installation has
+        // accumulated an unexpectedly large number of generated files, while
+        // deleting the oldest batch so later explicit installs make progress.
+        let boundedCandidates = removableCandidates
+            .sorted { lhs, rhs in
+                if lhs.date != rhs.date { return lhs.date < rhs.date }
+                return lhs.url.lastPathComponent < rhs.url.lastPathComponent
+            }
+            .prefix(256)
+        for url in boundedCandidates {
+            try? FileManager.default.removeItem(at: url.url)
         }
     }
 
-    /// Conservatively detects sessions that may still reference an older hook generation.
+    /// A running Codex process may have loaded an immutable hook path that is
+    /// absent from the current config. Cleanup is explicit-install-only, so a
+    /// synchronous fail-closed probe protects that process without adding
+    /// launch latency or a background polling task.
     private static func hasRunningCodexProcess() -> Bool {
         let process = Process()
         process.executableURL = URL(fileURLWithPath: "/usr/bin/pgrep")
@@ -319,7 +337,11 @@ extension CMUXCLI {
 
     static func codexFireAndForgetAgentHookShellCommand(_ command: String, for def: AgentHookDef) -> String {
         let routedArguments = command.hasPrefix("cmux ") ? String(command.dropFirst("cmux ".count)) : command
-        let runner = "payload=\"$1\"; shift; \"$@\" <\"$payload\" >/dev/null 2>&1 & child=\"$!\"; ( timer=; trap \"kill \\$timer 2>/dev/null || true; wait \\$timer 2>/dev/null || true; exit 0\" HUP INT TERM; sleep 30 & timer=\"$!\"; wait \"$timer\" 2>/dev/null || true; timer=; kill \"$child\" 2>/dev/null || true ) & watchdog=\"$!\"; wait \"$child\" 2>/dev/null || true; kill \"$watchdog\" 2>/dev/null || true; wait \"$watchdog\" 2>/dev/null || true; rm -f \"$payload\""
+        // Keep timer ownership in one process and stop it through a file
+        // handshake. A signal-based supervisor can receive TERM before its
+        // trap/timer assignment is installed, orphaning the timer for a fast
+        // child; the explicit parent-owned timer is always waited/reaped.
+        let runner = "payload=\"$1\"; shift; timer_stop=\"$payload.timer-stop\"; timer_done=\"$payload.timer-done\"; kill_timer_tree() { timer_root=\"$1\"; for timer_child in $(/usr/bin/pgrep -P \"$timer_root\" 2>/dev/null || true); do kill_timer_tree \"$timer_child\"; done; kill -KILL \"$timer_root\" 2>/dev/null || true; }; rm -f \"$timer_stop\" \"$timer_done\"; ( sleep 30 & timer=\"$!\"; while [ ! -e \"$timer_stop\" ]; do timer_state=$(/bin/ps -o state= -p \"$timer\" 2>/dev/null | /usr/bin/tr -d \"[:space:]\"); case \"$timer_state\" in \"\"|Z*) wait \"$timer\" 2>/dev/null || true; printf done >\"$timer_done\"; exit 0;; esac; /bin/sleep 0.05; done; kill_timer_tree \"$timer\"; wait \"$timer\" 2>/dev/null || true; exit 0 ) & timer_supervisor=\"$!\"; \"$@\" <\"$payload\" >/dev/null 2>&1 & child=\"$!\"; ( while [ ! -s \"$timer_done\" ] && [ ! -e \"$timer_stop\" ]; do /bin/sleep 0.05; done; if [ -s \"$timer_done\" ]; then kill \"$child\" 2>/dev/null || true; fi ) & watchdog=\"$!\"; wait \"$child\" 2>/dev/null || true; : >\"$timer_stop\"; kill \"$watchdog\" 2>/dev/null || true; wait \"$watchdog\" 2>/dev/null || true; wait \"$timer_supervisor\" 2>/dev/null || true; rm -f \"$payload\" \"$timer_stop\" \"$timer_done\""
         let noOp = stdinDrainingHookNoOpShellCommand
         return [
             "cmux_cli=\"${CMUX_BUNDLED_CLI_PATH:-}\"",

CLI/CMUXCLI+SemanticNotifications.swift

@@ -12,6 +12,12 @@ extension CMUXCLI {
         let fields = payload.split(separator: "|", omittingEmptySubsequences: false).map(String.init)
         guard fields.count >= 3 else { throw CLIError(message: String(localized: "cli.notification.invalidPayload", defaultValue: "Invalid notification payload")) }
         let meta = fields.count > 3 ? fields[3].split(separator: ";").map(String.init) : []
+        if source == "codex", kind == .approvalRequested,
+           Self.semanticAttentionContext(rawObject, source: source).requestIdentity == nil,
+           let approval = meta.first(where: { $0.hasPrefix("a=") }),
+           approval.range(of: "^a=[0-9a-f]{24}\\.[0-9a-f]{24}$", options: .regularExpression) != nil {
+            return "notify_target_async \(workspaceId) \(surfaceId) \(payload)"
+        }
         let category = meta.first { $0.hasPrefix("c=") }.map { String($0.dropFirst(2)) }
             ?? (kind == .turnCompleted ? "turn-complete" : "other")
         let notification = AgentJournalNotification(title: fields[0], subtitle: fields[1], body: fields[2],
@@ -26,7 +32,7 @@ extension CMUXCLI {
         kind: AgentJournalEventKind, rawObject: [String: Any]?, notification: AgentJournalNotification,
         pendingWork: Bool = false, isSubagent: Bool = false
     ) throws -> String {
-        var context = Self.semanticAttentionContext(rawObject)
+        var context = Self.semanticAttentionContext(rawObject, source: source)
         var notification = notification
         switch kind {
         case .errorReported, .messagePublished: notification.category = "other"
@@ -47,7 +53,7 @@ extension CMUXCLI {
         return "agent_journal_append \(String(decoding: data, as: UTF8.self))"
     }
 
-    static func semanticAttentionContext(_ object: [String: Any]?) -> AgentAttentionContext {
+    static func semanticAttentionContext(_ object: [String: Any]?, source: String? = nil) -> AgentAttentionContext {
         func identifier(_ keys: [String]) -> String? {
             for key in keys {
                 if let value = object?[key] as? String, !value.isEmpty { return value }
@@ -59,7 +65,8 @@ extension CMUXCLI {
         return AgentAttentionContext(
             eventIdentity: identifier(["event_id", "eventId", "message_id", "uuid"]),
             turnIdentity: identifier(["turn_id", "turnId"]),
-            requestIdentity: identifier(["tool_use_id", "toolUseId", "toolUseID", "tool_call_id", "toolCallId", "request_id", "requestId"]))
+            requestIdentity: (source == "codex" ? CodexApprovalNotificationIdentity.nativeRequestID(in: object) : nil)
+                ?? identifier(["tool_use_id", "toolUseId", "toolUseID", "tool_call_id", "toolCallId", "request_id", "requestId"]))
     }
 
     static func semanticOccurredAtMs(_ object: [String: Any]?) -> Int64? {

CLI/FeedEventClassifier.swift

@@ -36,21 +36,16 @@ struct FeedEventClassification: Equatable {
     /// A tool COMPLETED for an agent whose approval prompts notify via
     /// ``notifiesNativeApprovalPrompt`` — execution strictly follows any
     /// approval, so the prompt resolved (approved by the user or by the
-    /// agent's own auto-reviewer) and the bridge clears the pane's stale
-    /// notifications. Only tool COMPLETION qualifies: pre-tool events fire
+    /// agent's own auto-reviewer) and the bridge resolves the correlated
+    /// approval notification. Only tool COMPLETION qualifies: pre-tool events fire
     /// when the agent intends to run a tool, with no ordering guarantee
     /// against the approval-prompt hook, so clearing there could erase a
     /// just-raised prompt while the agent is still blocked.
     ///
-    /// The clear is deliberately pane-wide and uncorrelated with any single
-    /// request: notifications carry no request identity anywhere in cmux,
-    /// and every agent integration clears the same way on progress signals —
-    /// Claude's `session-start`/`prompt-submit`/`pre-tool-use` hooks, the
-    /// generic `.approvalResponse` action (Hermes' resolved native
-    /// approvals), and codex's own `prompt-submit` hook (which also clears
-    /// deny-without-further-tools residue at the next turn). Pane
-    /// notifications are attention signals; agent progress in the pane makes
-    /// them stale as a set.
+    /// Newer Codex payloads carry an approval/call id; older payloads fall back
+    /// to a bounded session/turn/tool/input identity. The app-side coordinator
+    /// marks repeated derived identities ambiguous, so one completion cannot
+    /// settle multiple identical requests.
     let clearsNativeApprovalPrompt: Bool
 }
 
@@ -198,8 +193,8 @@ struct FeedEventClassifier {
         case .toolEnd:
             // A completed tool ran, and execution strictly follows any
             // approval — so this is the earliest progress signal that can
-            // safely clear a resolved native approval prompt (approved by
-            // the user or by the agent's own auto-reviewer). Scoped to
+            // safely resolve its correlated native approval prompt (approved
+            // by the user or by the agent's own auto-reviewer). Scoped to
             // sources that raise those prompts so other agents' tool
             // telemetry never touches the notification queue.
             return FeedEventClassification(
@@ -436,14 +431,14 @@ struct FeedEventClassifier {
 
     /// Builds the pane-attention V1 socket command a classified feed event
     /// carries — the `needs-permission`-gated `notify_target_async` for a
-    /// native approval prompt, or the pane-scoped `clear_notifications` for
-    /// a resolved one. Pure so the exact wire command (UUID gating, payload
-    /// shape, gate meta) is unit-testable; the CLI feed hook sends the
+    /// native approval prompt, or the correlated `clear_notifications` for a
+    /// resolved one. Pure so the exact wire command (UUID gating, payload
+    /// shape, gate/correlation meta) is unit-testable; the CLI feed hook sends the
     /// returned line request/response and awaits the app's acknowledgement.
     ///
-    /// Returns `nil` when the classification carries no attention side
-    /// effect or when either identity is missing/not a UUID: the command is
-    /// advisory and must never fail the hook.
+    /// Returns `nil` when the classification carries no attention side effect,
+    /// when targets are invalid, or when an exact Codex completion lacks its
+    /// identity. A legacy prompt may still use the pane-scoped notify form.
     ///
     /// The notification body deliberately names only the TOOL — mirroring
     /// the in-app Feed approval banner (`feed.notification.permission.body`)
@@ -457,7 +452,9 @@ struct FeedEventClassifier {
         workspaceId: String?,
         surfaceId: String?,
         agentID: String = "codex",
-        includeAgentContext: Bool = false
+        includeAgentContext: Bool = false,
+        source: String? = nil,
+        approvalIdentity: CodexApprovalNotificationIdentity? = nil
     ) -> String? {
         guard classification.notifiesNativeApprovalPrompt
                 || classification.clearsNativeApprovalPrompt else { return nil }
@@ -466,7 +463,28 @@ struct FeedEventClassifier {
               let surfaceRaw = surfaceId?.trimmingCharacters(in: .whitespacesAndNewlines),
               let surfaceUUID = UUID(uuidString: surfaceRaw)
         else { return nil }
+        // A Codex completion is correlated by an exact identity. Other native
+        // approval producers retain the historical pane-scoped command shape;
+        // callers that omit `source` are treated as Codex for compatibility
+        // with the strict completion path.
+        let requiresCorrelatedIdentity = source?
+            .trimmingCharacters(in: .whitespacesAndNewlines)
+            .lowercased() == "codex" || source == nil
+        // A hook payload is bounded, but a single tool-name field could still
+        // consume nearly the entire budget and force large socket/UI copies.
+        // Keep attention lines small and predictable on the synchronous path.
+        guard !classification.clearsNativeApprovalPrompt
+                || !requiresCorrelatedIdentity
+                || approvalIdentity != nil else {
+            return nil
+        }
         if classification.clearsNativeApprovalPrompt {
+            if let approvalIdentity {
+                return "clear_notifications --tab=\(workspaceUUID.uuidString) --panel=\(surfaceUUID.uuidString) \(approvalIdentity.resolutionOptions)"
+            }
+            // Legacy non-Codex producers have no exact approval identity and
+            // historically clear their own pane-scoped prompt.
+            guard !requiresCorrelatedIdentity else { return nil }
             return "clear_notifications --tab=\(workspaceUUID.uuidString) --panel=\(surfaceUUID.uuidString)"
         }
         let subtitle = String(
@@ -487,7 +505,14 @@ struct FeedEventClassifier {
             )
         }
         let meta: String?
-        if includeAgentContext {
+        if let approvalIdentity {
+            meta = AgentHookNotifyCategory.needsPermission.metaSegment(
+                pending: false,
+                approvalID: approvalIdentity.approvalID,
+                approvalIDIsDerived: !approvalIdentity.isAuthoritative,
+                approvalSource: "feed"
+            )
+        } else if includeAgentContext {
             meta = AgentHookNotifyCategory.needsPermission.metaSegment(
                 pending: false,
                 agentID: agentID
@@ -507,7 +532,8 @@ struct FeedEventClassifier {
     /// tool names are payload-controlled input, so normalize them the same
     /// way `notificationPayload` sanitizes its fields.
     private static func attentionNotificationField(_ value: String) -> String {
-        value
+        let bounded = String(value.prefix(240))
+        return bounded
             .components(separatedBy: .newlines)
             .joined(separator: " ")
             .trimmingCharacters(in: .whitespacesAndNewlines)