From cf36b70cedd9de6d4a415865bda4b1f5a3f0b6e0 Mon Sep 17 00:00:00 2001 From: Danny Gershman Date: Thu, 11 Jun 2026 13:49:32 -0400 Subject: [PATCH 1/2] Walk PATH and honor defaults.binaries for codex/cursor discovery (CROW-484) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `OpenAICodexAgent.findBinary()` and `CursorAgent.findBinary()` only checked three hardcoded paths (`/opt/homebrew/bin/...`, `/usr/local/bin/...`, `~/.local/bin/...`). Codex is distributed as `npm i -g @openai/codex` — under nvm/Volta/pnpm/asdf the binary never lands in any of those. When discovery returned nil, `AppDelegate.launchMainApp` skipped registration, so the picker silently never showed the agent even though it was installed. This refactors discovery into a default `CodingAgent.findBinary()` impl: explicit `defaults.binaries.` override → PATH walk via `ShellEnvironment.findExecutable` (same way `command -v` resolves) → hardcoded `fallbackCandidates`. Per-agent impls are gone; each agent just declares its fallback list. AppDelegate now loads config before registration, populates `BinaryOverrides.shared`, and logs the resolved path so users can verify which install Crow picked up. 🐦‍⬛ Generated with Claude Code, orchestrated by Crow Co-Authored-By: Claude Crow-Session: CEB4D4CB-61A8-468F-8A3A-0E72FF5610C6 --- .../Sources/CrowClaude/ClaudeCodeAgent.swift | 17 +++--- .../Sources/CrowCodex/OpenAICodexAgent.swift | 18 +++---- .../OpenAICodexAgentTests.swift | 25 +++++++++ .../CrowCore/Agent/BinaryOverrides.swift | 34 ++++++++++++ .../Sources/CrowCore/Agent/CodingAgent.swift | 35 +++++++++++++ .../Sources/CrowCore/Models/AppConfig.swift | 13 ++++- .../Sources/CrowCore/ShellEnvironment.swift | 18 +++++-- .../Tests/CrowCoreTests/AppConfigTests.swift | 32 +++++++++++- .../CrowCoreTests/BinaryOverridesTests.swift | 52 +++++++++++++++++++ .../CrowCoreTests/ShellEnvironmentTests.swift | 32 ++++++++++++ .../Sources/CrowCursor/CursorAgent.swift | 19 +++---- Sources/Crow/App/AppDelegate.swift | 25 +++++---- 12 files changed, 267 insertions(+), 53 deletions(-) create mode 100644 Packages/CrowCore/Sources/CrowCore/Agent/BinaryOverrides.swift create mode 100644 Packages/CrowCore/Tests/CrowCoreTests/BinaryOverridesTests.swift create mode 100644 Packages/CrowCore/Tests/CrowCoreTests/ShellEnvironmentTests.swift diff --git a/Packages/CrowClaude/Sources/CrowClaude/ClaudeCodeAgent.swift b/Packages/CrowClaude/Sources/CrowClaude/ClaudeCodeAgent.swift index f409151b..eec3517e 100644 --- a/Packages/CrowClaude/Sources/CrowClaude/ClaudeCodeAgent.swift +++ b/Packages/CrowClaude/Sources/CrowClaude/ClaudeCodeAgent.swift @@ -16,8 +16,12 @@ public struct ClaudeCodeAgent: CodingAgent { private let launcher: ClaudeLauncher - /// Standard search paths for the Claude CLI binary, in priority order. - static let claudeBinaryCandidates: [String] = [ + /// Last-resort search paths for the `claude` binary, used only when the + /// configured `BinaryOverrides` and a PATH walk both miss. Claude Code + /// installs via its installer to `~/.local/bin/claude` by default, which + /// is the first hit on most setups; the homebrew paths cover users who + /// symlinked the CLI into a system location (CROW-484). + public let fallbackCandidates: [String] = [ FileManager.default.homeDirectoryForCurrentUser.appendingPathComponent(".local/bin/claude").path, "/usr/local/bin/claude", "/opt/homebrew/bin/claude", @@ -32,15 +36,6 @@ public struct ClaudeCodeAgent: CodingAgent { self.launcher = ClaudeLauncher() } - public func findBinary() -> String? { - for path in Self.claudeBinaryCandidates { - if FileManager.default.isExecutableFile(atPath: path) { - return path - } - } - return nil - } - public func autoLaunchCommand( session: Session, worktreePath: String, diff --git a/Packages/CrowCodex/Sources/CrowCodex/OpenAICodexAgent.swift b/Packages/CrowCodex/Sources/CrowCodex/OpenAICodexAgent.swift index 39bd499d..b34108b4 100644 --- a/Packages/CrowCodex/Sources/CrowCodex/OpenAICodexAgent.swift +++ b/Packages/CrowCodex/Sources/CrowCodex/OpenAICodexAgent.swift @@ -18,9 +18,12 @@ public struct OpenAICodexAgent: CodingAgent { private let launcher: CodexLauncher - /// Standard search paths for the `codex` binary, in priority order. - /// Homebrew-cask installs Codex at the first path on macOS. - static let codexBinaryCandidates: [String] = [ + /// Last-resort search paths for the `codex` binary, used only when the + /// configured `BinaryOverrides` and a PATH walk both miss. Most users will + /// resolve through PATH (codex ships via `npm i -g @openai/codex` and + /// lives wherever the user's Node manager puts globals); this list is just + /// the historical hardcoded set we used to check first (CROW-484). + public let fallbackCandidates: [String] = [ "/opt/homebrew/bin/codex", "/usr/local/bin/codex", FileManager.default.homeDirectoryForCurrentUser.appendingPathComponent(".local/bin/codex").path, @@ -35,15 +38,6 @@ public struct OpenAICodexAgent: CodingAgent { self.launcher = CodexLauncher() } - public func findBinary() -> String? { - for path in Self.codexBinaryCandidates { - if FileManager.default.isExecutableFile(atPath: path) { - return path - } - } - return nil - } - public func autoLaunchCommand( session: Session, worktreePath: String, diff --git a/Packages/CrowCodex/Tests/CrowCodexTests/OpenAICodexAgentTests.swift b/Packages/CrowCodex/Tests/CrowCodexTests/OpenAICodexAgentTests.swift index 3adb7024..aa60ce8f 100644 --- a/Packages/CrowCodex/Tests/CrowCodexTests/OpenAICodexAgentTests.swift +++ b/Packages/CrowCodex/Tests/CrowCodexTests/OpenAICodexAgentTests.swift @@ -62,4 +62,29 @@ struct OpenAICodexAgentTests { // accept either outcome and just verify the result type. _ = agent.findBinary() // smoke test: must not crash } + + @Test func findBinaryHonorsBinaryOverride() { + // `defaults.binaries.codex` -> absolute path. The default + // `CodingAgent.findBinary()` impl should consult + // `BinaryOverrides.shared` before walking PATH (CROW-484). + // `/bin/sh` is guaranteed-executable on macOS and clearly distinct + // from any real codex install, so a positive result here means the + // override path was honored. + BinaryOverrides.shared.set(["codex": "/bin/sh"]) + defer { BinaryOverrides.shared.set([:]) } + + #expect(agent.findBinary() == "/bin/sh") + } + + @Test func findBinaryIgnoresOverrideWhenPathMissing() { + // A stale override (binary moved/uninstalled after config edit) must + // not break registration outright — fall through to PATH/fallback + // discovery instead. We can't guarantee codex is installed in the + // test env, so we just assert that the bogus override doesn't get + // returned literally. + BinaryOverrides.shared.set(["codex": "/tmp/this-path-does-not-exist-crow484"]) + defer { BinaryOverrides.shared.set([:]) } + + #expect(agent.findBinary() != "/tmp/this-path-does-not-exist-crow484") + } } diff --git a/Packages/CrowCore/Sources/CrowCore/Agent/BinaryOverrides.swift b/Packages/CrowCore/Sources/CrowCore/Agent/BinaryOverrides.swift new file mode 100644 index 00000000..019d790e --- /dev/null +++ b/Packages/CrowCore/Sources/CrowCore/Agent/BinaryOverrides.swift @@ -0,0 +1,34 @@ +import Foundation + +/// Process-wide explicit overrides for `CodingAgent.findBinary()`. Populated +/// once at app launch from `AppConfig.defaults.binaries` and consulted by the +/// default `findBinary()` impl before falling through to PATH-walk + hardcoded +/// candidates (CROW-484). +/// +/// Keyed by `AgentKind` so a config entry like +/// `{ "defaults": { "binaries": { "codex": "/Users/me/.nvm/.../bin/codex" } } }` +/// pins the codex agent's binary to the explicit path the user named — useful +/// when discovery still fails for some exotic install layout. +public final class BinaryOverrides: @unchecked Sendable { + public static let shared = BinaryOverrides() + + private let lock = NSLock() + private var paths: [AgentKind: String] = [:] + + /// Replace the override map. Keys are `AgentKind.rawValue` strings (matches + /// the JSON config shape and the existing `agentsByKind` keying style). + /// An empty map clears all overrides. + public func set(_ raw: [String: String]) { + lock.lock(); defer { lock.unlock() } + paths = Dictionary(uniqueKeysWithValues: raw.map { (AgentKind(rawValue: $0), $1) }) + } + + /// The user-configured absolute path for `kind`, or `nil` if none is set. + /// Callers must still verify the path is executable before using it — a + /// stale override (e.g. the binary moved after config edit) should fall + /// through to PATH-walk rather than break agent registration. + public func path(for kind: AgentKind) -> String? { + lock.lock(); defer { lock.unlock() } + return paths[kind] + } +} diff --git a/Packages/CrowCore/Sources/CrowCore/Agent/CodingAgent.swift b/Packages/CrowCore/Sources/CrowCore/Agent/CodingAgent.swift index 464b760c..6da2b409 100644 --- a/Packages/CrowCore/Sources/CrowCore/Agent/CodingAgent.swift +++ b/Packages/CrowCore/Sources/CrowCore/Agent/CodingAgent.swift @@ -35,6 +35,12 @@ public protocol CodingAgent: Sendable { /// `AgentStateTransition` values. var stateSignalSource: any StateSignalSource { get } + /// Last-resort hardcoded paths for the agent's CLI binary, checked after + /// the user's explicit `BinaryOverrides` and a PATH walk both miss. Used + /// only when the user's resolved PATH is unusually narrow (CROW-484). An + /// empty list is fine — most agents will resolve through PATH first. + var fallbackCandidates: [String] { get } + /// Resolve this agent's binary on disk, or return `nil` if it isn't /// installed. Drives binary-presence gating for the per-session picker /// and the launch-command builder below. @@ -98,6 +104,35 @@ public protocol CodingAgent: Sendable { } public extension CodingAgent { + /// Default empty `fallbackCandidates` so simple conformers don't have to + /// declare an empty array. Agents with known install locations should + /// override this. + var fallbackCandidates: [String] { [] } + + /// Default binary discovery: explicit `BinaryOverrides` → PATH walk + /// (using the agent's `launchCommandToken` as the binary name) → + /// hardcoded `fallbackCandidates`. Returns the first resolved absolute + /// path, or `nil` if nothing matches. + /// + /// This replaces the per-agent hardcoded-list-only impl that left + /// nvm/Volta/pnpm/asdf installs invisible to Crow (CROW-484). + func findBinary() -> String? { + let fm = FileManager.default + // 1. Explicit user override from `defaults.binaries.`. Verify + // the path is still executable so a stale override falls through + // to discovery rather than breaking registration outright. + if let configured = BinaryOverrides.shared.path(for: kind), + fm.isExecutableFile(atPath: configured) { + return configured + } + // 2. Walk the user's resolved PATH the same way `command -v` does. + if let found = ShellEnvironment.shared.findExecutable(launchCommandToken) { + return found + } + // 3. Hardcoded last-resort fallback covers the exotic-PATH case. + return fallbackCandidates.first { fm.isExecutableFile(atPath: $0) } + } + /// Default Manager launch command: invoke the agent's CLI binary by /// name with no extra flags. The terminal backend (tmux/Ghostty) owns /// the submitting Enter — return the raw command without a trailing diff --git a/Packages/CrowCore/Sources/CrowCore/Models/AppConfig.swift b/Packages/CrowCore/Sources/CrowCore/Models/AppConfig.swift index 221cf0ea..6bd8efac 100644 --- a/Packages/CrowCore/Sources/CrowCore/Models/AppConfig.swift +++ b/Packages/CrowCore/Sources/CrowCore/Models/AppConfig.swift @@ -406,6 +406,12 @@ public struct ConfigDefaults: Codable, Sendable, Equatable { public var excludeReviewRepos: [String] public var excludeTicketRepos: [String] public var ignoreReviewLabels: [String] + /// Explicit absolute-path overrides for `CodingAgent` binary discovery, + /// keyed by `AgentKind.rawValue` (e.g. `"codex"`, `"cursor"`, `"claude-code"`). + /// Consulted before the PATH walk in `CodingAgent.findBinary()` — set this + /// when discovery doesn't find your install for any reason (exotic Node + /// manager, sandboxed PATH, etc.). See CROW-484. + public var binaries: [String: String] /// Characters that are invalid in git ref names (see `git check-ref-format`). private static let invalidBranchChars = CharacterSet(charactersIn: " ~^:?*[\\") @@ -432,7 +438,8 @@ public struct ConfigDefaults: Codable, Sendable, Equatable { excludeDirs: [String] = ["node_modules", ".git", "vendor", "dist", "build", "target"], excludeReviewRepos: [String] = [], excludeTicketRepos: [String] = [], - ignoreReviewLabels: [String] = [] + ignoreReviewLabels: [String] = [], + binaries: [String: String] = [:] ) { self.provider = provider self.cli = cli @@ -441,6 +448,7 @@ public struct ConfigDefaults: Codable, Sendable, Equatable { self.excludeReviewRepos = excludeReviewRepos self.excludeTicketRepos = excludeTicketRepos self.ignoreReviewLabels = ignoreReviewLabels + self.binaries = binaries } public init(from decoder: Decoder) throws { @@ -452,10 +460,11 @@ public struct ConfigDefaults: Codable, Sendable, Equatable { excludeReviewRepos = try container.decodeIfPresent([String].self, forKey: .excludeReviewRepos) ?? [] excludeTicketRepos = try container.decodeIfPresent([String].self, forKey: .excludeTicketRepos) ?? [] ignoreReviewLabels = try container.decodeIfPresent([String].self, forKey: .ignoreReviewLabels) ?? [] + binaries = try container.decodeIfPresent([String: String].self, forKey: .binaries) ?? [:] } private enum CodingKeys: String, CodingKey { - case provider, cli, branchPrefix, excludeDirs, excludeReviewRepos, excludeTicketRepos, ignoreReviewLabels + case provider, cli, branchPrefix, excludeDirs, excludeReviewRepos, excludeTicketRepos, ignoreReviewLabels, binaries } } diff --git a/Packages/CrowCore/Sources/CrowCore/ShellEnvironment.swift b/Packages/CrowCore/Sources/CrowCore/ShellEnvironment.swift index 2ef420d0..365e2d56 100644 --- a/Packages/CrowCore/Sources/CrowCore/ShellEnvironment.swift +++ b/Packages/CrowCore/Sources/CrowCore/ShellEnvironment.swift @@ -29,16 +29,26 @@ public final class ShellEnvironment: Sendable { env.merging(extra) { _, new in new } } - /// Returns `true` if `name` is an executable found in the resolved PATH. - public func hasCommand(_ name: String) -> Bool { + /// Returns the absolute path of `name` if it resolves to an executable on + /// the user's PATH, or `nil` otherwise. Mirrors the behavior of shell + /// `command -v` — walks `resolvedPATH` left-to-right and returns the first + /// hit. Used by every `CodingAgent` to find its CLI binary regardless of + /// where the user's package manager installed it (npm-global / nvm / + /// volta / pnpm / asdf — see CROW-484). + public func findExecutable(_ name: String) -> String? { let fm = FileManager.default for dir in resolvedPATH.split(separator: ":") { let path = "\(dir)/\(name)" if fm.isExecutableFile(atPath: path) { - return true + return path } } - return false + return nil + } + + /// Returns `true` if `name` is an executable found in the resolved PATH. + public func hasCommand(_ name: String) -> Bool { + findExecutable(name) != nil } // MARK: - PATH Resolution diff --git a/Packages/CrowCore/Tests/CrowCoreTests/AppConfigTests.swift b/Packages/CrowCore/Tests/CrowCoreTests/AppConfigTests.swift index 83dbf79e..689dd4c8 100644 --- a/Packages/CrowCore/Tests/CrowCoreTests/AppConfigTests.swift +++ b/Packages/CrowCore/Tests/CrowCoreTests/AppConfigTests.swift @@ -8,7 +8,7 @@ import Testing WorkspaceInfo(name: "TestOrg", provider: "github", cli: "gh", alwaysInclude: ["repo1"]), WorkspaceInfo(name: "GitLabOrg", provider: "gitlab", cli: "glab", host: "gitlab.example.com"), ], - defaults: ConfigDefaults(provider: "gitlab", cli: "glab", branchPrefix: "fix/", excludeDirs: ["vendor"], excludeReviewRepos: ["zarf-dev/zarf", "bmlt-enabled/yap"], excludeTicketRepos: ["org/hidden-repo"]), + defaults: ConfigDefaults(provider: "gitlab", cli: "glab", branchPrefix: "fix/", excludeDirs: ["vendor"], excludeReviewRepos: ["zarf-dev/zarf", "bmlt-enabled/yap"], excludeTicketRepos: ["org/hidden-repo"], binaries: ["codex": "/tmp/codex"]), notifications: NotificationSettings(globalMute: true), sidebar: SidebarSettings(hideSessionDetails: true) ) @@ -27,10 +27,40 @@ import Testing #expect(decoded.defaults.excludeDirs == ["vendor"]) #expect(decoded.defaults.excludeReviewRepos == ["zarf-dev/zarf", "bmlt-enabled/yap"]) #expect(decoded.defaults.excludeTicketRepos == ["org/hidden-repo"]) + #expect(decoded.defaults.binaries == ["codex": "/tmp/codex"]) #expect(decoded.notifications.globalMute == true) #expect(decoded.sidebar.hideSessionDetails == true) } +/// `defaults.binaries` decodes from explicit JSON and is keyed by +/// `AgentKind.rawValue` (CROW-484). +@Test func configDefaultsBinariesDecodesFromJSON() throws { + let json = #""" + { + "defaults": { + "binaries": { + "codex": "/Users/me/.nvm/versions/node/v22/bin/codex", + "cursor": "/Users/me/.bun/bin/agent", + "claude-code": "/Users/me/.local/bin/claude" + } + } + } + """#.data(using: .utf8)! + let config = try JSONDecoder().decode(AppConfig.self, from: json) + + #expect(config.defaults.binaries["codex"] == "/Users/me/.nvm/versions/node/v22/bin/codex") + #expect(config.defaults.binaries["cursor"] == "/Users/me/.bun/bin/agent") + #expect(config.defaults.binaries["claude-code"] == "/Users/me/.local/bin/claude") +} + +/// Missing `binaries` key decodes to an empty map (forward-compat with +/// existing config files written before CROW-484). +@Test func configDefaultsBinariesDefaultsEmpty() throws { + let json = #"{ "defaults": { "branchPrefix": "fix/" } }"#.data(using: .utf8)! + let config = try JSONDecoder().decode(AppConfig.self, from: json) + #expect(config.defaults.binaries.isEmpty) +} + @Test func appConfigDecodeFromEmptyJSON() throws { let json = "{}".data(using: .utf8)! let config = try JSONDecoder().decode(AppConfig.self, from: json) diff --git a/Packages/CrowCore/Tests/CrowCoreTests/BinaryOverridesTests.swift b/Packages/CrowCore/Tests/CrowCoreTests/BinaryOverridesTests.swift new file mode 100644 index 00000000..de0f1009 --- /dev/null +++ b/Packages/CrowCore/Tests/CrowCoreTests/BinaryOverridesTests.swift @@ -0,0 +1,52 @@ +import Foundation +import Testing +@testable import CrowCore + +@Suite("BinaryOverrides") +struct BinaryOverridesTests { + /// Resets the shared override state at the end of the suite so other + /// tests don't see leaked entries. Each test that mutates state is + /// responsible for resetting before returning, but this is belt + braces. + init() { + BinaryOverrides.shared.set([:]) + } + + @Test func emptyByDefault() { + BinaryOverrides.shared.set([:]) + #expect(BinaryOverrides.shared.path(for: .codex) == nil) + #expect(BinaryOverrides.shared.path(for: .cursor) == nil) + #expect(BinaryOverrides.shared.path(for: .claudeCode) == nil) + } + + @Test func setStoresRawKeyedByAgentKind() { + BinaryOverrides.shared.set([ + "codex": "/Users/me/.nvm/versions/node/v22/bin/codex", + "cursor": "/tmp/agent", + ]) + defer { BinaryOverrides.shared.set([:]) } + + #expect(BinaryOverrides.shared.path(for: .codex) == "/Users/me/.nvm/versions/node/v22/bin/codex") + #expect(BinaryOverrides.shared.path(for: .cursor) == "/tmp/agent") + #expect(BinaryOverrides.shared.path(for: .claudeCode) == nil) + } + + @Test func setReplacesPreviousMap() { + BinaryOverrides.shared.set(["codex": "/tmp/codex-a"]) + BinaryOverrides.shared.set(["cursor": "/tmp/agent"]) + defer { BinaryOverrides.shared.set([:]) } + + // Previous codex entry is gone — `set` replaces, not merges. + #expect(BinaryOverrides.shared.path(for: .codex) == nil) + #expect(BinaryOverrides.shared.path(for: .cursor) == "/tmp/agent") + } + + @Test func unknownAgentKindRoundTrips() { + // AgentKind is a RawRepresentable struct so downstream packages can + // register their own kinds. The override map should accept and + // surface arbitrary keys. + BinaryOverrides.shared.set(["custom-agent": "/tmp/custom"]) + defer { BinaryOverrides.shared.set([:]) } + + #expect(BinaryOverrides.shared.path(for: AgentKind(rawValue: "custom-agent")) == "/tmp/custom") + } +} diff --git a/Packages/CrowCore/Tests/CrowCoreTests/ShellEnvironmentTests.swift b/Packages/CrowCore/Tests/CrowCoreTests/ShellEnvironmentTests.swift new file mode 100644 index 00000000..e018595d --- /dev/null +++ b/Packages/CrowCore/Tests/CrowCoreTests/ShellEnvironmentTests.swift @@ -0,0 +1,32 @@ +import Foundation +import Testing +@testable import CrowCore + +@Suite("ShellEnvironment") +struct ShellEnvironmentTests { + /// `sh` is guaranteed present in `/bin/sh` on macOS, and `/bin` is part of + /// any sane PATH (login shell or fallback). The exact resolved path may be + /// `/bin/sh` or a homebrew-installed sh earlier in PATH — we just verify + /// the helper returns *something* executable matching the requested name. + @Test func findExecutableResolvesKnownBinary() { + let resolved = ShellEnvironment.shared.findExecutable("sh") + #expect(resolved != nil) + if let resolved { + #expect(FileManager.default.isExecutableFile(atPath: resolved)) + #expect(resolved.hasSuffix("/sh")) + } + } + + @Test func findExecutableReturnsNilForUnknownBinary() { + // A name no PATH directory should plausibly contain. + let resolved = ShellEnvironment.shared.findExecutable("crow-nonexistent-binary-xyzzy-12345") + #expect(resolved == nil) + } + + @Test func hasCommandAgreesWithFindExecutable() { + // The two helpers must stay in sync — `hasCommand` is implemented as + // `findExecutable(_:) != nil`, but pin the contract with a test. + #expect(ShellEnvironment.shared.hasCommand("sh") == true) + #expect(ShellEnvironment.shared.hasCommand("crow-nonexistent-binary-xyzzy-12345") == false) + } +} diff --git a/Packages/CrowCursor/Sources/CrowCursor/CursorAgent.swift b/Packages/CrowCursor/Sources/CrowCursor/CursorAgent.swift index 2d2d1b96..2ed47028 100644 --- a/Packages/CrowCursor/Sources/CrowCursor/CursorAgent.swift +++ b/Packages/CrowCursor/Sources/CrowCursor/CursorAgent.swift @@ -24,10 +24,12 @@ public struct CursorAgent: CodingAgent { private let launcher: CursorLauncher - /// Standard search paths for the `agent` binary, in priority order. - /// Homebrew-cask installs the Cursor app bundle at the first path on - /// macOS; users who symlink the embedded CLI usually drop it there. - static let cursorBinaryCandidates: [String] = [ + /// Last-resort search paths for the `agent` binary (Cursor's CLI), used + /// only when the configured `BinaryOverrides` and a PATH walk both miss. + /// The Cursor app bundle's embedded CLI is usually symlinked into PATH or + /// installed via the Cursor app's "Install 'cursor' command" action; this + /// list is the historical hardcoded set we used to check first (CROW-484). + public let fallbackCandidates: [String] = [ "/opt/homebrew/bin/agent", "/usr/local/bin/agent", FileManager.default.homeDirectoryForCurrentUser.appendingPathComponent(".local/bin/agent").path, @@ -42,15 +44,6 @@ public struct CursorAgent: CodingAgent { self.launcher = CursorLauncher() } - public func findBinary() -> String? { - for path in Self.cursorBinaryCandidates { - if FileManager.default.isExecutableFile(atPath: path) { - return path - } - } - return nil - } - public func autoLaunchCommand( session: Session, worktreePath: String, diff --git a/Sources/Crow/App/AppDelegate.swift b/Sources/Crow/App/AppDelegate.swift index 93867e36..3db5200f 100644 --- a/Sources/Crow/App/AppDelegate.swift +++ b/Sources/Crow/App/AppDelegate.swift @@ -324,37 +324,42 @@ final class AppDelegate: NSObject, NSApplicationDelegate { private func launchMainApp() { guard let devRoot else { return } + // Load config first so per-agent binary overrides + // (`defaults.binaries.`) are visible to the registration gates + // below — `CodingAgent.findBinary()` consults `BinaryOverrides.shared` + // before walking PATH (CROW-484). + let config = appConfig ?? ConfigStore.loadConfig(devRoot: devRoot) ?? AppConfig() + self.appConfig = config + BinaryOverrides.shared.set(config.defaults.binaries) + // Register the Claude Code agent in the shared registry — always // present, since the Manager terminal and the default-agent picker // both rely on it. AgentRegistry.shared.register(ClaudeCodeAgent()) // Conditionally register the OpenAI Codex agent — only when its - // binary is on disk. Keeps the per-session picker clean for users - // who haven't installed Codex. + // binary resolves on PATH (or via an explicit `defaults.binaries.codex` + // override). Keeps the per-session picker clean for users who haven't + // installed Codex (CROW-484). let codexAgent = OpenAICodexAgent() - if codexAgent.findBinary() != nil { + if let codexPath = codexAgent.findBinary() { AgentRegistry.shared.register(codexAgent) - NSLog("[Crow] OpenAI Codex agent registered") + NSLog("[Crow] OpenAI Codex agent registered at %@", codexPath) } // Conditionally register the Cursor agent on the same gate. The // Cursor CLI installs the binary as `agent` (not `cursor`); when // it's absent the picker silently stays at the two prior agents. let cursorAgent = CursorAgent() - if cursorAgent.findBinary() != nil { + if let cursorPath = cursorAgent.findBinary() { AgentRegistry.shared.register(cursorAgent) - NSLog("[Crow] Cursor agent registered") + NSLog("[Crow] Cursor agent registered at %@", cursorPath) } // Initialize libghostty NSLog("[Crow] Initializing Ghostty") GhosttyApp.shared.initialize() - // Load config before initializing the terminal backend so any future - // backend selection knobs can read it. - let config = appConfig ?? ConfigStore.loadConfig(devRoot: devRoot) ?? AppConfig() - self.appConfig = config NSLog("[Crow] Config loaded (workspaces: %d)", config.workspaces.count) // Configure the tmux backend (#198 → defaulted-on in #301 → the only From 1721dcd03a4dd7e4e864e3c373c4c0bcf3859502 Mon Sep 17 00:00:00 2001 From: Danny Gershman Date: Thu, 11 Jun 2026 13:57:40 -0400 Subject: [PATCH 2/2] Serialize BinaryOverrides test suites; document Cursor agent-name collision risk MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Reviewer (dhilgaertner) caught two issues on #485: 1. (Red) `BinaryOverrides` and `OpenAICodexAgent` suites mutate the process-global `BinaryOverrides.shared` singleton. Swift Testing runs tests in parallel within a suite by default, so the per-test `defer { set([:]) }` resets don't isolate anything — a concurrent `set(...)` clobbers another test's state mid-assertion (the reviewer reproduced 5/6 failures under `--filter`). Marking both suites `.serialized` is sufficient because only these two touch the global, and they live in separate test targets. 2. (Yellow) Cursor's CLI binary is named `agent`, which is generic enough that CI runner installs (Azure DevOps, TeamCity) ship a binary with the same name. The PATH walk in the default `findBinary()` could mis-register Cursor on a build machine. Document the tradeoff at the `launchCommandToken` declaration and point users at `defaults.binaries.cursor` as the escape hatch — the explicit-override step runs before the PATH walk and pins the resolution. 🐦‍⬛ Generated with Claude Code, orchestrated by Crow Co-Authored-By: Claude Crow-Session: CEB4D4CB-61A8-468F-8A3A-0E72FF5610C6 --- .../Tests/CrowCodexTests/OpenAICodexAgentTests.swift | 2 +- .../Tests/CrowCoreTests/BinaryOverridesTests.swift | 2 +- .../CrowCursor/Sources/CrowCursor/CursorAgent.swift | 11 +++++++++++ 3 files changed, 13 insertions(+), 2 deletions(-) diff --git a/Packages/CrowCodex/Tests/CrowCodexTests/OpenAICodexAgentTests.swift b/Packages/CrowCodex/Tests/CrowCodexTests/OpenAICodexAgentTests.swift index aa60ce8f..cd4aa19b 100644 --- a/Packages/CrowCodex/Tests/CrowCodexTests/OpenAICodexAgentTests.swift +++ b/Packages/CrowCodex/Tests/CrowCodexTests/OpenAICodexAgentTests.swift @@ -3,7 +3,7 @@ import Testing @testable import CrowCodex @testable import CrowCore -@Suite("OpenAICodexAgent") +@Suite("OpenAICodexAgent", .serialized) struct OpenAICodexAgentTests { private let agent = OpenAICodexAgent() diff --git a/Packages/CrowCore/Tests/CrowCoreTests/BinaryOverridesTests.swift b/Packages/CrowCore/Tests/CrowCoreTests/BinaryOverridesTests.swift index de0f1009..370876c2 100644 --- a/Packages/CrowCore/Tests/CrowCoreTests/BinaryOverridesTests.swift +++ b/Packages/CrowCore/Tests/CrowCoreTests/BinaryOverridesTests.swift @@ -2,7 +2,7 @@ import Foundation import Testing @testable import CrowCore -@Suite("BinaryOverrides") +@Suite("BinaryOverrides", .serialized) struct BinaryOverridesTests { /// Resets the shared override state at the end of the suite so other /// tests don't see leaked entries. Each test that mutates state is diff --git a/Packages/CrowCursor/Sources/CrowCursor/CursorAgent.swift b/Packages/CrowCursor/Sources/CrowCursor/CursorAgent.swift index 2ed47028..a7561b08 100644 --- a/Packages/CrowCursor/Sources/CrowCursor/CursorAgent.swift +++ b/Packages/CrowCursor/Sources/CrowCursor/CursorAgent.swift @@ -18,6 +18,17 @@ public struct CursorAgent: CodingAgent { public let iconSystemName: String = "cursorarrow.rays" public let supportsRemoteControl: Bool = true /// Cursor's CLI binary is named `agent`, not `cursor`. + /// + /// `agent` is a generic name — CI runner installs (Azure DevOps, TeamCity) + /// also ship a binary called `agent`, so the PATH-walk discovery in + /// `CodingAgent.findBinary()` can in principle resolve a non-Cursor + /// executable on a build machine. If that happens, set + /// `defaults.binaries.cursor` to the absolute path of Cursor's CLI in + /// `{devRoot}/.claude/config.json` — the explicit override is consulted + /// before the PATH walk and pins the resolution. We accept the false- + /// positive risk here (CROW-484) because real workstations don't usually + /// have a competing `agent` on PATH, and the override knob exists for + /// the exotic case. public let launchCommandToken: String = "agent" public let hookConfigWriter: any HookConfigWriter public let stateSignalSource: any StateSignalSource