Fix login-shell probe cleanup

CHANGELOG.md

@@ -11,6 +11,7 @@
 - Linux CLI: bootstrap the configured IANA timezone before Foundation startup on non-FHS systems, preventing SIGILL on NixOS (#2127). Thanks @xikhar!
 - Ollama: release temporary dashboard network sessions after each fetch, preventing repeated refreshes from retaining delegates and URL-cache resources. Thanks @astuteprogrammer!
 - Linux CLI: prevent usage rendering from crashing in Foundation bundle discovery when formatting rate windows. Thanks @thanthi-del!
+- CLI: defer login-shell PATH probes until Codex RPC launch, preserve login PATH for explicit script overrides, and reap session-escaped helpers without cross-probe descriptor inheritance. Thanks @anagnorisis2peripeteia!
 - Menus: keep overview provider-row clicks reliable during live menu rebuilds without stealing nested Copy or plan actions. Thanks @Yuxin-Qiao!
 - Startup: load persisted plan-utilization history away from the main thread so mature histories no longer delay app launch. Thanks @Yuxin-Qiao!
 - Provider cleanup: prevent in-flight usage, status, token-cost, and cached-hydration work from republishing stale state after a provider is disabled, unavailable, or re-enabled. Thanks @Yuxin-Qiao!

Sources/CodexBarCore/CodexExecutableResolver.swift

@@ -0,0 +1,51 @@
+import Foundation
+
+struct CodexExecutableResolution: Sendable {
+    let executable: String
+    let loginPATH: [String]?
+}
+
+typealias CodexExecutableResolver = @Sendable (
+    _ environment: [String: String],
+    _ executable: String) -> CodexExecutableResolution?
+
+private func executableIsScript(_ path: String) -> Bool {
+    guard let handle = FileHandle(forReadingAtPath: path) else { return true }
+    defer { try? handle.close() }
+    return (try? handle.read(upToCount: 2)) == Data("#!".utf8)
+}
+
+func resolveCodexExecutableForRPC(
+    environment: [String: String],
+    executable: String,
+    captureLoginPATH: () -> [String]?) -> CodexExecutableResolution?
+{
+    if let override = environment["CODEX_CLI_PATH"],
+       FileManager.default.isExecutableFile(atPath: override)
+    {
+        // Native overrides can launch directly. Script overrides can depend on a
+        // login-shell PATH through `#!/usr/bin/env node` or helper commands.
+        let loginPATH = executableIsScript(override) ? captureLoginPATH() : nil
+        return CodexExecutableResolution(executable: override, loginPATH: loginPATH)
+    }
+
+    // Capture before the general resolver to preserve login-shell PATH precedence
+    // over bundled fallbacks and to make `/usr/bin/env node` launchers usable.
+    let loginPATH = captureLoginPATH()
+    guard let resolved = BinaryLocator.resolveCodexBinary(env: environment, loginPATH: loginPATH)
+        ?? TTYCommandRunner.which(executable)
+    else { return nil }
+    return CodexExecutableResolution(executable: resolved, loginPATH: loginPATH)
+}
+
+let defaultCodexExecutableResolver: CodexExecutableResolver = { environment, executable in
+    // Resolve the login-shell PATH at the actual RPC launch boundary. Warming it
+    // from UsageFetcher.init was both racy for env-based launchers and unsafe for
+    // short-lived CLI hosts because the fire-and-forget probe could outlive them.
+    resolveCodexExecutableForRPC(
+        environment: environment,
+        executable: executable,
+        captureLoginPATH: {
+            LoginShellPathCache.shared.currentOrCapture(shell: environment["SHELL"])
+        })
+}

Sources/CodexBarCore/Host/Process/SpawnedProcessGroup.swift

@@ -322,6 +322,14 @@ package final class SpawnedProcessGroup: @unchecked Sendable {
         self.startWaiter()
     }
 
+    package static func adopt(
+        pid: pid_t,
+        outputFileDescriptors: [Int32]) -> SpawnedProcessGroup
+    {
+        let outputPipes = Set(outputFileDescriptors.compactMap(OutputPipeIdentity.resolve(fileDescriptor:)))
+        return SpawnedProcessGroup(pid: pid, outputPipes: outputPipes)
+    }
+
     package static func launch(
         binary: String,
         arguments: [String],
@@ -531,7 +539,7 @@ package final class SpawnedProcessGroup: @unchecked Sendable {
         let deadline = Date().addingTimeInterval(max(0, grace))
         var processIdentities = self.currentResidualProcessIdentities(includeDescendants: true)
         processIdentities.formUnion(self.currentProcessGroupMemberIdentities())
-        if let rootIdentity = self.rootIdentity {
+        if self.isRunning, let rootIdentity = self.rootIdentity {
             processIdentities.insert(rootIdentity)
         }
         Self.signal(processIdentities: processIdentities, signal: SIGTERM)
@@ -546,6 +554,8 @@ package final class SpawnedProcessGroup: @unchecked Sendable {
         processIdentities.formUnion(self.currentProcessGroupMemberIdentities())
         if self.isRunning, let rootIdentity = self.rootIdentity {
             processIdentities.insert(rootIdentity)
+        } else if let rootIdentity = self.rootIdentity {
+            processIdentities.remove(rootIdentity)
         }
         Self.signal(processIdentities: processIdentities, signal: SIGKILL)
 

Sources/CodexBarCore/PathEnvironment.swift

@@ -10,6 +10,11 @@ import Musl
 import Security
 #endif
 
+#if os(Linux)
+@_silgen_name("pipe2")
+private func linuxPipe2(_ pipeDescriptors: UnsafeMutablePointer<Int32>, _ flags: Int32) -> Int32
+#endif
+
 public enum PathPurpose: Hashable, Sendable {
     case rpc
     case tty
@@ -641,10 +646,11 @@ public enum CodexLaunchPreflight {
 
 public enum ShellCommandLocator {
     #if canImport(Darwin)
-    private static let shellSpawnFlags = Int16(POSIX_SPAWN_SETSID)
+    private static let shellSpawnFlags = Int16(POSIX_SPAWN_SETSID | POSIX_SPAWN_CLOEXEC_DEFAULT)
     #else
     private static let shellSpawnFlags: Int16 = 0x80 // glibc/musl POSIX_SPAWN_SETSID.
     #endif
+    private static let shellSpawnLock = NSLock()
 
     static func test_runShellCommand(
         shell: String,
@@ -654,6 +660,10 @@ public enum ShellCommandLocator {
         self.runShellCommand(shell: shell, arguments: arguments, timeout: timeout)
     }
 
+    static func test_makeCloseOnExecPipe() -> (read: Int32, write: Int32)? {
+        self.makeCloseOnExecPipe()
+    }
+
     static var test_shellSpawnFlags: Int16 {
         self.shellSpawnFlags
     }
@@ -741,29 +751,58 @@ public enum ShellCommandLocator {
         }
     }
 
-    // swiftlint:disable cyclomatic_complexity function_body_length
+    private static func makeCloseOnExecPipe() -> (read: Int32, write: Int32)? {
+        var fds: (read: Int32, write: Int32) = (-1, -1)
+        #if os(Linux)
+        // Glibc and Musl export pipe2, but their Swift modules do not consistently declare it.
+        guard withUnsafeMutablePointer(to: &fds, {
+            $0.withMemoryRebound(to: Int32.self, capacity: 2) { linuxPipe2($0, O_CLOEXEC) == 0 }
+        }) else { return nil }
+        #else
+        guard withUnsafeMutablePointer(to: &fds, {
+            $0.withMemoryRebound(to: Int32.self, capacity: 2) { pipe($0) == 0 }
+        }) else { return nil }
+
+        for fd in [fds.read, fds.write] {
+            let flags = fcntl(fd, F_GETFD)
+            guard flags >= 0, fcntl(fd, F_SETFD, flags | FD_CLOEXEC) == 0 else {
+                close(fds.read)
+                close(fds.write)
+                return nil
+            }
+        }
+        #endif
+        return fds
+    }
+
+    // swiftlint:disable cyclomatic_complexity
     /// Runs a shell command, draining both stdout and stderr concurrently so that
     /// verbose shell init scripts (oh-my-zsh, nvm, pyenv, etc.) cannot deadlock on
     /// a full pipe buffer.  The child is launched via `posix_spawn` with
     /// `POSIX_SPAWN_SETSID` so it cannot take ownership of the caller's controlling
-    /// terminal. The new session also makes the child its own process-group leader
-    /// before `exec`, which guarantees that subsequent `kill(-pgid, ...)` calls reach
-    /// background helpers spawned by shell init, both after timeout and normal exit.
+    /// terminal. The new session also makes the child its own process-group leader;
+    /// cleanup tracks both that group and helpers retaining the command's output pipes.
     fileprivate static func runShellCommand(
         shell: String,
         arguments: [String],
         timeout: TimeInterval) -> Data?
     {
+        // Darwin needs a lock around raw descriptor creation, close-on-exec flagging,
+        // and spawn. Linux creates close-on-exec descriptors atomically with pipe2.
+        self.shellSpawnLock.lock()
+        var shellSpawnLockHeld = true
+        defer {
+            if shellSpawnLockHeld {
+                self.shellSpawnLock.unlock()
+            }
+        }
+
         // Pipes for stdout/stderr.  stdin is redirected from /dev/null in the child
-        // via posix_spawn_file_actions_addopen below.
-        var stdoutFds: (read: Int32, write: Int32) = (-1, -1)
-        var stderrFds: (read: Int32, write: Int32) = (-1, -1)
-        guard withUnsafeMutablePointer(to: &stdoutFds, {
-            $0.withMemoryRebound(to: Int32.self, capacity: 2) { pipe($0) == 0 }
-        }) else { return nil }
-        guard withUnsafeMutablePointer(to: &stderrFds, {
-            $0.withMemoryRebound(to: Int32.self, capacity: 2) { pipe($0) == 0 }
-        }) else {
+        // via posix_spawn_file_actions_addopen below. Close-on-exec prevents a
+        // concurrently spawned probe from retaining these descriptors and being
+        // mistaken for one of this probe's output holders during cleanup.
+        guard let stdoutFds = self.makeCloseOnExecPipe() else { return nil }
+        guard let stderrFds = self.makeCloseOnExecPipe() else {
             close(stdoutFds.read); close(stdoutFds.write)
             return nil
         }
@@ -850,15 +889,14 @@ public enum ShellCommandLocator {
         // once every descendant in the process group also closes them.
         close(stdoutFds.write)
         close(stderrFds.write)
+        self.shellSpawnLock.unlock()
+        shellSpawnLockHeld = false
 
         guard spawnResult == 0 else {
             close(stdoutFds.read); close(stderrFds.read)
             return nil
         }
 
-        // POSIX_SPAWN_SETSID guarantees the child's session ID and pgid equal its pid.
-        let pgid: pid_t = pid
-
         // Track EOF on each pipe so we can wait for full drain instead of sleeping.
         // The readability handler fires with empty data when every writer end is
         // closed (i.e. the child *and* any inheriting background helpers are gone).
@@ -893,27 +931,18 @@ public enum ShellCommandLocator {
             }
         }
 
-        // Reap the child on a background queue and signal a semaphore on exit.
-        let exitSemaphore = DispatchSemaphore(value: 0)
-        let waitPid = pid
-        DispatchQueue.global(qos: .userInitiated).async {
-            var status: Int32 = 0
-            while waitpid(waitPid, &status, 0) == -1, errno == EINTR {
-                // retry
-            }
-            exitSemaphore.signal()
+        // Adopt the already-spawned session so cleanup can also discover helpers
+        // that escape into a new process group while retaining our output pipes.
+        let process = SpawnedProcessGroup.adopt(
+            pid: pid,
+            outputFileDescriptors: [stdoutFds.read, stderrFds.read])
+        let deadline = Date().addingTimeInterval(timeout)
+        while process.isRunning, Date() < deadline {
+            usleep(10000)
         }
 
-        let finishedInTime = exitSemaphore.wait(timeout: .now() + timeout) == .success
-
-        if !finishedInTime {
-            kill(-pgid, SIGTERM)
-            kill(pid, SIGTERM)
-            if exitSemaphore.wait(timeout: .now() + 0.4) != .success {
-                kill(-pgid, SIGKILL)
-                kill(pid, SIGKILL)
-                _ = exitSemaphore.wait(timeout: .now() + 1.0)
-            }
+        if process.isRunning {
+            process.terminateSynchronously()
             stdoutHandle.readabilityHandler = nil
             stderrHandle.readabilityHandler = nil
             if stdoutDone.fire() {
@@ -925,15 +954,14 @@ public enum ShellCommandLocator {
             return nil
         }
 
-        // Normal completion — clean up any background children spawned by shell init.
-        // Without this, helpers that inherited stdout/stderr keep the pipe write ends
-        // open and we never see EOF on the read ends.
-        kill(-pgid, SIGTERM)
+        // Normal completion — clean up background children spawned by shell init,
+        // including session-escaped helpers that still hold our output pipes open.
+        process.terminateSynchronously()
 
         // Wait for both pipes to deliver EOF so no buffered bytes are lost.
         // Bounded so a stuck handler can't hang the caller indefinitely.
         if drainGroup.wait(timeout: .now() + 0.4) != .success {
-            kill(-pgid, SIGKILL)
+            process.terminateSynchronously(grace: 0)
         }
         if drainGroup.wait(timeout: .now() + 0.6) != .success {
             stdoutHandle.readabilityHandler = nil
@@ -948,7 +976,7 @@ public enum ShellCommandLocator {
         return stdoutCollector.drain()
     }
 
-    // swiftlint:enable cyclomatic_complexity function_body_length
+    // swiftlint:enable cyclomatic_complexity
 
     private static func runShellCapture(_ shell: String?, _ timeout: TimeInterval, _ command: String) -> String? {
         let shellPath = (shell?.isEmpty == false) ? shell! : "/bin/zsh"

Sources/CodexBarCore/UsageFetcher.swift

@@ -949,13 +949,6 @@ enum RPCWireError: Error, LocalizedError {
     }
 }
 
-typealias CodexExecutableResolver = @Sendable (_ environment: [String: String], _ executable: String) -> String?
-
-let defaultCodexExecutableResolver: CodexExecutableResolver = { environment, executable in
-    BinaryLocator.resolveCodexBinary(env: environment)
-        ?? TTYCommandRunner.which(executable)
-}
-
 /// RPC helper used on background tasks; safe because we confine it to the owning task.
 private final class CodexRPCClient: @unchecked Sendable {
     private static let log = CodexBarLog.logger(LogCategories.codexRPC)
@@ -1012,16 +1005,19 @@ private final class CodexRPCClient: @unchecked Sendable {
         }
         self.stdoutLineContinuation = stdoutContinuation
 
-        let resolvedExec = resolveExecutable(environment, executable)
+        let resolution = resolveExecutable(environment, executable)
 
-        guard let resolvedExec else {
+        guard let resolution else {
             Self.log.warning("Codex RPC binary not found", metadata: ["binary": executable])
             throw CodexStatusProbeError.codexNotInstalled
         }
+        let resolvedExec = resolution.executable
         var env = environment
+        let loginPATH = resolution.loginPATH ?? LoginShellPathCache.shared.current
         env["PATH"] = PathBuilder.effectivePATH(
             purposes: [.rpc, .nodeTooling],
-            env: env)
+            env: env,
+            loginPATH: loginPATH)
 
         self.process.environment = env
         self.process.executableURL = URL(fileURLWithPath: "/usr/bin/env")
@@ -1240,7 +1236,6 @@ public struct UsageFetcher: Sendable {
         self.requestTimeoutSeconds = 3.0
         self.codexExecutableResolver = defaultCodexExecutableResolver
         self.codexArguments = ["-s", "read-only", "-a", "untrusted", "app-server"]
-        LoginShellPathCache.shared.captureOnce()
     }
 
     init(
@@ -1255,7 +1250,6 @@ public struct UsageFetcher: Sendable {
         self.requestTimeoutSeconds = requestTimeoutSeconds
         self.codexExecutableResolver = codexExecutableResolver
         self.codexArguments = codexArguments
-        LoginShellPathCache.shared.captureOnce()
     }
 
     public func loadLatestUsage(keepCLISessionsAlive: Bool = false) async throws -> UsageSnapshot {

Tests/CodexBarTests/CodexExecutableResolverTests.swift

@@ -0,0 +1,42 @@
+import Foundation
+import Testing
+@testable import CodexBarCore
+
+struct CodexExecutableResolverTests {
+    @Test
+    func `explicit native override skips login path capture`() {
+        let resolved = resolveCodexExecutableForRPC(
+            environment: ["CODEX_CLI_PATH": "/usr/bin/true"],
+            executable: "codex",
+            captureLoginPATH: {
+                Issue.record("Native override should not capture a login-shell PATH")
+                return nil
+            })
+
+        #expect(resolved?.executable == "/usr/bin/true")
+        #expect(resolved?.loginPATH == nil)
+    }
+
+    @Test
+    func `explicit script override captures login path for env based launchers`() throws {
+        let scriptURL = FileManager.default.temporaryDirectory
+            .appendingPathComponent("codexbar-script-override-\(UUID().uuidString)")
+        try Data("#!/usr/bin/env node\n".utf8).write(to: scriptURL)
+        try FileManager.default.setAttributes([.posixPermissions: 0o700], ofItemAtPath: scriptURL.path)
+        defer { try? FileManager.default.removeItem(at: scriptURL) }
+
+        let loginPATH = ["/custom/node/bin", "/usr/bin"]
+        var captureCount = 0
+        let resolved = resolveCodexExecutableForRPC(
+            environment: ["CODEX_CLI_PATH": scriptURL.path],
+            executable: "codex",
+            captureLoginPATH: {
+                captureCount += 1
+                return loginPATH
+            })
+
+        #expect(resolved?.executable == scriptURL.path)
+        #expect(resolved?.loginPATH == loginPATH)
+        #expect(captureCount == 1)
+    }
+}

Tests/CodexBarTests/PathBuilderTests.swift

@@ -96,7 +96,7 @@ struct PathBuilderTests {
         let escapedMarker = Self.shellSingleQuoted(marker)
         let script = """
         (
-          trap '' TERM
+          trap '' HUP TERM
           touch \(escapedMarker)
           while :; do sleep 1; done
         ) &

Tests/CodexBarTests/ShellCommandForegroundTests.swift

@@ -9,6 +9,7 @@ struct ShellCommandForegroundTests {
         let flags = ShellCommandLocator.test_shellSpawnFlags
 
         #expect(flags & Int16(POSIX_SPAWN_SETSID) != 0)
+        #expect(flags & Int16(POSIX_SPAWN_CLOEXEC_DEFAULT) != 0)
         #expect(flags & Int16(POSIX_SPAWN_SETPGROUP) == 0)
     }
 }