From 1d480a59e26e67f46852e90ff9da2e2c7a739f41 Mon Sep 17 00:00:00 2001 From: Cameron Beeley Date: Mon, 13 Jul 2026 12:48:19 +0100 Subject: [PATCH 1/3] Fix login-shell probe cleanup --- CHANGELOG.md | 1 + .../Host/Process/SpawnedProcessGroup.swift | 12 +++- Sources/CodexBarCore/PathEnvironment.swift | 50 +++++--------- Sources/CodexBarCore/UsageFetcher.swift | 32 +++++++-- .../CodexExecutableResolverTests.swift | 17 +++++ Tests/CodexBarTests/PathBuilderTests.swift | 2 +- .../ShellCommandLocatorProcessTests.swift | 68 +++++++++++++++++++ 7 files changed, 144 insertions(+), 38 deletions(-) create mode 100644 Tests/CodexBarTests/CodexExecutableResolverTests.swift create mode 100644 Tests/CodexBarTests/ShellCommandLocatorProcessTests.swift diff --git a/CHANGELOG.md b/CHANGELOG.md index 9b28898da3..4c4548f107 100644 --- a/CHANGELOG.md +++ b/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! diff --git a/Sources/CodexBarCore/Host/Process/SpawnedProcessGroup.swift b/Sources/CodexBarCore/Host/Process/SpawnedProcessGroup.swift index ecfb190d8b..96acc3d262 100644 --- a/Sources/CodexBarCore/Host/Process/SpawnedProcessGroup.swift +++ b/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) diff --git a/Sources/CodexBarCore/PathEnvironment.swift b/Sources/CodexBarCore/PathEnvironment.swift index 631ac045df..cb3289c057 100644 --- a/Sources/CodexBarCore/PathEnvironment.swift +++ b/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" diff --git a/Sources/CodexBarCore/UsageFetcher.swift b/Sources/CodexBarCore/UsageFetcher.swift index 843934fbd6..cc0414a457 100644 --- a/Sources/CodexBarCore/UsageFetcher.swift +++ b/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 { diff --git a/Tests/CodexBarTests/CodexExecutableResolverTests.swift b/Tests/CodexBarTests/CodexExecutableResolverTests.swift new file mode 100644 index 0000000000..2b8a5d6874 --- /dev/null +++ b/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") + } +} diff --git a/Tests/CodexBarTests/PathBuilderTests.swift b/Tests/CodexBarTests/PathBuilderTests.swift index 7a459308db..6c989256bc 100644 --- a/Tests/CodexBarTests/PathBuilderTests.swift +++ b/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 ) & diff --git a/Tests/CodexBarTests/ShellCommandLocatorProcessTests.swift b/Tests/CodexBarTests/ShellCommandLocatorProcessTests.swift new file mode 100644 index 0000000000..0271392385 --- /dev/null +++ b/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) + } + } +} From ffe09726cecd7789dd39aabf3675c41ddb4a28af Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Tue, 14 Jul 2026 00:29:21 +0100 Subject: [PATCH 2/3] fix: harden login PATH probe cleanup --- CHANGELOG.md | 2 +- .../CodexExecutableResolver.swift | 51 +++++++++++++++++ Sources/CodexBarCore/PathEnvironment.swift | 57 +++++++++++++++---- Sources/CodexBarCore/UsageFetcher.swift | 42 ++------------ .../CodexExecutableResolverTests.swift | 31 +++++++++- .../ShellCommandForegroundTests.swift | 1 + .../ShellCommandLocatorProcessTests.swift | 15 +++++ 7 files changed, 149 insertions(+), 50 deletions(-) create mode 100644 Sources/CodexBarCore/CodexExecutableResolver.swift diff --git a/CHANGELOG.md b/CHANGELOG.md index 4c4548f107..2f8ecb91b5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,7 +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. +- 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! diff --git a/Sources/CodexBarCore/CodexExecutableResolver.swift b/Sources/CodexBarCore/CodexExecutableResolver.swift new file mode 100644 index 0000000000..dacb02f68d --- /dev/null +++ b/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"]) + }) +} diff --git a/Sources/CodexBarCore/PathEnvironment.swift b/Sources/CodexBarCore/PathEnvironment.swift index cb3289c057..7b2330100d 100644 --- a/Sources/CodexBarCore/PathEnvironment.swift +++ b/Sources/CodexBarCore/PathEnvironment.swift @@ -641,10 +641,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 +655,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,6 +746,29 @@ public enum ShellCommandLocator { } } + private static func makeCloseOnExecPipe() -> (read: Int32, write: Int32)? { + var fds: (read: Int32, write: Int32) = (-1, -1) + #if canImport(Darwin) + 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 + } + } + #else + guard withUnsafeMutablePointer(to: &fds, { + $0.withMemoryRebound(to: Int32.self, capacity: 2) { pipe2($0, O_CLOEXEC) == 0 } + }) else { 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 @@ -753,16 +781,23 @@ public enum ShellCommandLocator { arguments: [String], timeout: TimeInterval) -> Data? { + // Darwin lacks pipe2(O_CLOEXEC). Keep raw descriptor creation, flagging, + // and spawn inside one lock so another PATH probe cannot exec during the + // pipe-to-fcntl window. Linux gets atomic close-on-exec pipe creation too. + 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 } @@ -849,6 +884,8 @@ 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) diff --git a/Sources/CodexBarCore/UsageFetcher.swift b/Sources/CodexBarCore/UsageFetcher.swift index cc0414a457..1c2f4c9b7c 100644 --- a/Sources/CodexBarCore/UsageFetcher.swift +++ b/Sources/CodexBarCore/UsageFetcher.swift @@ -949,39 +949,6 @@ enum RPCWireError: Error, LocalizedError { } } -typealias CodexExecutableResolver = @Sendable (_ environment: [String: String], _ executable: String) -> String? - -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) @@ -1038,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") diff --git a/Tests/CodexBarTests/CodexExecutableResolverTests.swift b/Tests/CodexBarTests/CodexExecutableResolverTests.swift index 2b8a5d6874..72117dfd96 100644 --- a/Tests/CodexBarTests/CodexExecutableResolverTests.swift +++ b/Tests/CodexBarTests/CodexExecutableResolverTests.swift @@ -1,17 +1,42 @@ +import Foundation import Testing @testable import CodexBarCore struct CodexExecutableResolverTests { @Test - func `explicit override skips login path capture`() { + func `explicit native 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") + Issue.record("Native override should not capture a login-shell PATH") return nil }) - #expect(resolved == "/usr/bin/true") + #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) } } diff --git a/Tests/CodexBarTests/ShellCommandForegroundTests.swift b/Tests/CodexBarTests/ShellCommandForegroundTests.swift index 1880a78aa1..691593dd37 100644 --- a/Tests/CodexBarTests/ShellCommandForegroundTests.swift +++ b/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) } } diff --git a/Tests/CodexBarTests/ShellCommandLocatorProcessTests.swift b/Tests/CodexBarTests/ShellCommandLocatorProcessTests.swift index 0271392385..7736323586 100644 --- a/Tests/CodexBarTests/ShellCommandLocatorProcessTests.swift +++ b/Tests/CodexBarTests/ShellCommandLocatorProcessTests.swift @@ -8,6 +8,21 @@ import Testing @testable import CodexBarCore struct ShellCommandLocatorProcessTests { + @Test + func `shell probe pipe descriptors close across unrelated execs`() throws { + let fds = try #require(ShellCommandLocator.test_makeCloseOnExecPipe()) + defer { + close(fds.read) + close(fds.write) + } + + for fd in [fds.read, fds.write] { + let flags = fcntl(fd, F_GETFD) + #expect(flags >= 0) + #expect(flags & FD_CLOEXEC != 0) + } + } + @Test func `shell runner terminates session escaped partial output holders after timeout`() throws { let pidFile = FileManager.default.temporaryDirectory From bc40e4bf374732a81b809dbc1f4ef1bf2309409c Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Tue, 14 Jul 2026 00:55:13 +0100 Subject: [PATCH 3/3] fix: restore atomic Linux probe pipes --- Sources/CodexBarCore/PathEnvironment.swift | 21 +++++++++++++-------- 1 file changed, 13 insertions(+), 8 deletions(-) diff --git a/Sources/CodexBarCore/PathEnvironment.swift b/Sources/CodexBarCore/PathEnvironment.swift index 7b2330100d..e7ae0eff95 100644 --- a/Sources/CodexBarCore/PathEnvironment.swift +++ b/Sources/CodexBarCore/PathEnvironment.swift @@ -10,6 +10,11 @@ import Musl import Security #endif +#if os(Linux) +@_silgen_name("pipe2") +private func linuxPipe2(_ pipeDescriptors: UnsafeMutablePointer, _ flags: Int32) -> Int32 +#endif + public enum PathPurpose: Hashable, Sendable { case rpc case tty @@ -748,7 +753,12 @@ public enum ShellCommandLocator { private static func makeCloseOnExecPipe() -> (read: Int32, write: Int32)? { var fds: (read: Int32, write: Int32) = (-1, -1) - #if canImport(Darwin) + #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 } @@ -761,10 +771,6 @@ public enum ShellCommandLocator { return nil } } - #else - guard withUnsafeMutablePointer(to: &fds, { - $0.withMemoryRebound(to: Int32.self, capacity: 2) { pipe2($0, O_CLOEXEC) == 0 } - }) else { return nil } #endif return fds } @@ -781,9 +787,8 @@ public enum ShellCommandLocator { arguments: [String], timeout: TimeInterval) -> Data? { - // Darwin lacks pipe2(O_CLOEXEC). Keep raw descriptor creation, flagging, - // and spawn inside one lock so another PATH probe cannot exec during the - // pipe-to-fcntl window. Linux gets atomic close-on-exec pipe creation too. + // 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 {