Fix login-shell probe cleanup

CHANGELOG.md

@@ -8,6 +8,7 @@
 
 ### Fixed
 - 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 and reap session-escaped helpers so repeated refreshes cannot accumulate orphaned processes.
 - 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/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

@@ -741,14 +741,13 @@ public enum ShellCommandLocator {
         }
     }
 
-    // swiftlint:disable cyclomatic_complexity function_body_length
+    // 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],
@@ -856,9 +855,6 @@ public enum ShellCommandLocator {
             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 +889,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 +912,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 +934,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

@@ -951,11 +951,37 @@ enum RPCWireError: Error, LocalizedError {
 
 typealias CodexExecutableResolver = @Sendable (_ environment: [String: String], _ executable: String) -> String?
 
-let defaultCodexExecutableResolver: CodexExecutableResolver = { environment, executable in
-    BinaryLocator.resolveCodexBinary(env: environment)
+func resolveCodexExecutableForRPC(
+    environment: [String: String],
+    executable: String,
+    captureLoginPATH: () -> [String]?) -> String?
+{
+    // An explicit override is authoritative and should never need a shell probe.
+    if let override = environment["CODEX_CLI_PATH"],
+       FileManager.default.isExecutableFile(atPath: override)
+    {
+        return override
+    }
+
+    // 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()
+    return BinaryLocator.resolveCodexBinary(env: environment, loginPATH: loginPATH)
         ?? TTYCommandRunner.which(executable)
 }
 
+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"])
+        })
+}
+
 /// 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)
@@ -1240,7 +1266,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 +1280,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,17 @@
+import Testing
+@testable import CodexBarCore
+
+struct CodexExecutableResolverTests {
+    @Test
+    func `explicit override skips login path capture`() {
+        let resolved = resolveCodexExecutableForRPC(
+            environment: ["CODEX_CLI_PATH": "/usr/bin/true"],
+            executable: "codex",
+            captureLoginPATH: {
+                Issue.record("Explicit Codex override should not capture a login-shell PATH")
+                return nil
+            })
+
+        #expect(resolved == "/usr/bin/true")
+    }
+}

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/ShellCommandLocatorProcessTests.swift

@@ -0,0 +1,68 @@
+#if canImport(Darwin)
+import Darwin
+#else
+import Glibc
+#endif
+import Foundation
+import Testing
+@testable import CodexBarCore
+
+struct ShellCommandLocatorProcessTests {
+    @Test
+    func `shell runner terminates session escaped partial output holders after timeout`() throws {
+        let pidFile = FileManager.default.temporaryDirectory
+            .appendingPathComponent("codexbar-shell-runner-timeout-\(UUID().uuidString)")
+            .path
+        let stdoutPIDFile = "\(pidFile).stdout"
+        let stderrPIDFile = "\(pidFile).stderr"
+        let script = """
+        import os
+        import signal
+        import sys
+        import time
+
+        for stream, suffix in ((1, ".stdout"), (2, ".stderr")):
+            child = os.fork()
+            if child == 0:
+                os.setsid()
+                signal.signal(signal.SIGHUP, signal.SIG_IGN)
+                signal.signal(signal.SIGTERM, signal.SIG_IGN)
+                os.close(2 if stream == 1 else 1)
+                with open(sys.argv[1] + suffix, "w") as handle:
+                    handle.write(str(os.getpid()))
+                while True:
+                    time.sleep(1)
+
+        while not all(os.path.exists(sys.argv[1] + suffix) for suffix in (".stdout", ".stderr")):
+            time.sleep(0.01)
+        time.sleep(1000)
+        """
+
+        let start = Date()
+        let data = ShellCommandLocator.test_runShellCommand(
+            shell: "/usr/bin/python3",
+            arguments: ["-c", script, pidFile],
+            timeout: 0.2)
+        let elapsed = Date().timeIntervalSince(start)
+
+        let pids = try [stdoutPIDFile, stderrPIDFile].map { file in
+            let pidText = try String(contentsOfFile: file, encoding: .utf8)
+                .trimmingCharacters(in: .whitespacesAndNewlines)
+            return try #require(pid_t(pidText))
+        }
+        defer {
+            for pid in pids {
+                kill(pid, SIGKILL)
+            }
+            for file in [stdoutPIDFile, stderrPIDFile] {
+                try? FileManager.default.removeItem(atPath: file)
+            }
+        }
+
+        #expect(data == nil)
+        #expect(elapsed < 3.0, "Timed-out PATH probes should remain bounded")
+        for pid in pids {
+            #expect(kill(pid, 0) != 0)
+        }
+    }
+}