diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 1edb0bdb4761..a2e5dfbd4fe4 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -625,6 +625,29 @@ jobs: echo "::warning::Passwordless sudo unavailable; XCTest will use its default automation-mode setup" fi + - name: Run bundled command PATH regression + if: ${{ matrix.shard == fromJSON(env.CMUX_APP_HOST_FOCUSED_REGRESSION_SHARD) }} + run: | + # The tolerant full-suite step accepts ordinary Swift Testing failures. + # Keep the shell-resolution integration test non-tolerant so losing + # cmux's bundled commands from PATH cannot pass a shard. + set -euo pipefail + SOURCE_PACKAGES_DIR="$PWD/.ci-source-packages" + if ! command -v fish >/dev/null 2>&1; then + HOMEBREW_NO_AUTO_UPDATE=1 brew install fish + fi + command -v fish >/dev/null 2>&1 + scripts/ci/run-in-console-session.sh \ + scripts/ci/run-app-host-xcodebuild.sh \ + -project cmux.xcodeproj -scheme cmux-unit -configuration Debug \ + -derivedDataPath "$CMUX_DERIVED_DATA_PATH" \ + -clonedSourcePackagesDirPath "$SOURCE_PACKAGES_DIR" \ + -disableAutomaticPackageResolution \ + -destination "platform=macOS" \ + CMUX_SKIP_ZIG_BUILD=1 \ + -only-testing:cmuxTests/CmuxBundledBinPathIntegrationTests \ + test + - name: Run agent chat transcript lifecycle regressions if: ${{ matrix.shard == fromJSON(env.CMUX_APP_HOST_FOCUSED_REGRESSION_SHARD) }} run: | diff --git a/Resources/shell-integration/cmux-bash-integration.bash b/Resources/shell-integration/cmux-bash-integration.bash index c2af5696ec53..40c197ba3dc4 100644 --- a/Resources/shell-integration/cmux-bash-integration.bash +++ b/Resources/shell-integration/cmux-bash-integration.bash @@ -1798,9 +1798,12 @@ _cmux_install_prompt_command() { # Contents/MacOS entry so the GUI cmux binary cannot shadow the CLI cmux. # Shell init (.bashrc/.bash_profile) may prepend other dirs after launch. _cmux_fix_path() { - if [[ -n "${GHOSTTY_BIN_DIR:-}" ]]; then - local gui_dir="${GHOSTTY_BIN_DIR%/}" - local bin_dir="${gui_dir%/MacOS}/Resources/bin" + local integration_dir="${CMUX_SHELL_INTEGRATION_DIR:-}" + integration_dir="${integration_dir%/}" + if [[ "$integration_dir" == */Resources/shell-integration ]]; then + local resources_dir="${integration_dir%/shell-integration}" + local gui_dir="${resources_dir%/Resources}/MacOS" + local bin_dir="$resources_dir/bin" if [[ -d "$bin_dir" ]]; then PATH="$(_cmux_path_prepend_unique_directory "$bin_dir" "${PATH-}" "$gui_dir")" fi diff --git a/Resources/shell-integration/cmux-zsh-integration.zsh b/Resources/shell-integration/cmux-zsh-integration.zsh index 5f2cdb090498..9790910e2e2c 100644 --- a/Resources/shell-integration/cmux-zsh-integration.zsh +++ b/Resources/shell-integration/cmux-zsh-integration.zsh @@ -1891,9 +1891,12 @@ _cmux_precmd() { # We fix this once on first prompt (after all init files have run), and # reinstall cmux-owned wrapper functions in case user startup replaced them. _cmux_fix_path() { - if [[ -n "${GHOSTTY_BIN_DIR:-}" ]]; then - local gui_dir="${GHOSTTY_BIN_DIR%/}" - local bin_dir="${gui_dir%/MacOS}/Resources/bin" + local integration_dir="${CMUX_SHELL_INTEGRATION_DIR:-}" + integration_dir="${integration_dir%/}" + if [[ "$integration_dir" == */Resources/shell-integration ]]; then + local resources_dir="${integration_dir%/shell-integration}" + local gui_dir="${resources_dir%/Resources}/MacOS" + local bin_dir="$resources_dir/bin" if [[ -d "$bin_dir" ]]; then PATH="$(_cmux_path_prepend_unique_directory "$bin_dir" "${PATH-}" "$gui_dir")" fi diff --git a/Resources/shell-integration/fish/config.fish b/Resources/shell-integration/fish/config.fish index d44476321d24..010e19b4f30b 100644 --- a/Resources/shell-integration/fish/config.fish +++ b/Resources/shell-integration/fish/config.fish @@ -305,16 +305,28 @@ if test "$_cmux_integration_enabled" != 0 _cmux_relay_rpc_bg surface.ports_kick "$params" end - function _cmux_path_prepend_unique_directory --argument-names directory + function _cmux_path_prepend_unique_directory --argument-names directory skipped_directory test -n "$directory"; or return 0 set -l next_path "$directory" for entry in $PATH test "$entry" = "$directory"; and continue + test -n "$skipped_directory"; and test "$entry" = "$skipped_directory"; and continue set -a next_path "$entry" end set -gx PATH $next_path end + function _cmux_fix_path + set -q CMUX_SHELL_INTEGRATION_DIR; and test -n "$CMUX_SHELL_INTEGRATION_DIR"; or return 0 + set -l integration_dir (string trim -r -c / -- "$CMUX_SHELL_INTEGRATION_DIR") + string match -q '*/Resources/shell-integration' -- "$integration_dir"; or return 0 + set -l resources_dir (string replace -r '/shell-integration$' '' -- "$integration_dir") + set -l gui_dir (string replace -r '/Resources$' '/MacOS' -- "$resources_dir") + set -l bin_dir "$resources_dir/bin" + test -d "$bin_dir"; or return 0 + _cmux_path_prepend_unique_directory "$bin_dir" "$gui_dir" + end + function _cmux_install_cli_command_shim --argument-names command_name wrapper_path set -l surface_component "$fish_pid" if set -q CMUX_SURFACE_ID; and test -n "$CMUX_SURFACE_ID" @@ -533,3 +545,9 @@ if not set -q CMUX_FISH_USER_CONFIG_ALREADY_LOADED; and test -n "$_cmux_user_con source "$_cmux_user_config" end end + +# Run after the user's config so cmux's bundled commands keep precedence even +# when shell startup replaced PATH. Remote integrations do not use the bundled +# `shell-integration` layout, so the helper leaves their PATH unchanged. +functions -q _cmux_fix_path; and _cmux_fix_path +functions -e _cmux_fix_path diff --git a/cmux.xcodeproj/project.pbxproj b/cmux.xcodeproj/project.pbxproj index de14077befe9..487b82aa3b18 100644 --- a/cmux.xcodeproj/project.pbxproj +++ b/cmux.xcodeproj/project.pbxproj @@ -613,6 +613,7 @@ C0DE71B10000000000000001 /* AppDelegate+AgentChatNotifications.swift in Sources 53750003A0B1C2D3E4F50003 /* CmuxAuthRuntime in Frameworks */ = {isa = PBXBuildFile; productRef = 53750002A0B1C2D3E4F50002 /* CmuxAuthRuntime */; }; D7032A060000000000000001 /* CmuxBrowser in Frameworks */ = {isa = PBXBuildFile; productRef = D7032A060000000000000002 /* CmuxBrowser */; }; E3B7A30000000000000000C3 /* CmuxBrowser in Frameworks */ = {isa = PBXBuildFile; productRef = E3B7A30000000000000000C2 /* CmuxBrowser */; }; + 947194719471947194719472 /* CmuxBundledBinPathIntegrationTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 947194719471947194719471 /* CmuxBundledBinPathIntegrationTests.swift */; }; CA52A003CA52A003CA52A003 /* CmuxCanvas in Frameworks */ = {isa = PBXBuildFile; productRef = CA52A005CA52A005CA52A005 /* CmuxCanvas */; }; CA52A007CA52A007CA52A007 /* CmuxCanvas in Frameworks */ = {isa = PBXBuildFile; productRef = CA52A005CA52A005CA52A005 /* CmuxCanvas */; }; CA53A003CA53A003CA53A003 /* CmuxCanvasUI in Frameworks */ = {isa = PBXBuildFile; productRef = CA53A005CA53A005CA53A005 /* CmuxCanvasUI */; }; @@ -3340,6 +3341,7 @@ C0DE71B10000000000000002 /* AppDelegate+AgentChatNotifications.swift */ = {isa = 875200000000000000000002 /* cmuxApp+SurfaceNavigationMenu.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = "cmuxApp+SurfaceNavigationMenu.swift"; sourceTree = ""; }; 80410000000000000000000E /* cmuxApp+WorkspaceCommandHelpers.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = "cmuxApp+WorkspaceCommandHelpers.swift"; sourceTree = ""; }; A5001011 /* cmuxApp.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = cmuxApp.swift; sourceTree = ""; }; + 947194719471947194719471 /* CmuxBundledBinPathIntegrationTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = CmuxBundledBinPathIntegrationTests.swift; sourceTree = ""; }; 61AD48A9E6C3F1BE2B547FFE /* CMUXCLI+AgentHookCatalog.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = "CMUXCLI+AgentHookCatalog.swift"; sourceTree = ""; }; B9000062A1B2C3D4E5F60719 /* CMUXCLI+AgentHookDefinitions.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = "CMUXCLI+AgentHookDefinitions.swift"; sourceTree = ""; }; 918100000000000000000010 /* CMUXCLI+AgentHookFailureReporting.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = "CMUXCLI+AgentHookFailureReporting.swift"; sourceTree = ""; }; @@ -7447,6 +7449,7 @@ C0DE71B10000000000000002 /* AppDelegate+AgentChatNotifications.swift */ = {isa = 1718C0DE1718C0DE17180001 /* GhosttyCommandShiftForwardingTests.swift */, D3284002A1B2C3D4E5F60718 /* TraditionalChineseIMENumpadRegressionTests.swift */, C0F15A000000000000000002 /* FishShellIntegrationTests.swift */, + 947194719471947194719471 /* CmuxBundledBinPathIntegrationTests.swift */, C0F16A000000000000000002 /* ShellStartupMatrixTests.swift */, 831E0F65E3D945CE8970852E /* ShellIntegrationSendTransportTests.swift */, 8126A1B2C3D4E5F60718293A /* RemoteShellPromptRelayTests.swift */, @@ -10693,6 +10696,7 @@ C0DE71B10000000000000002 /* AppDelegate+AgentChatNotifications.swift */ = {isa = C75740010000000000000001 /* CloudVMMenuItemMetricsTests.swift in Sources */, C0DE43000000000000000009 /* CmuxAgentChatConfigTests.swift in Sources */, 8295A0058295A0058295A005 /* CmuxAlertContentTests.swift in Sources */, + 947194719471947194719472 /* CmuxBundledBinPathIntegrationTests.swift in Sources */, 0CB4E9797AD54D3BB9CF06F9 /* CMUXCLI+AutoNaming.swift in Sources */, C0DE31390000000000000105 /* CMUXCLIErrorOutputRegressionTests.swift in Sources */, 906900000000000000000004 /* CMUXCLIMemoryAttributionTests.swift in Sources */, diff --git a/cmuxTests/CmuxBundledBinPathIntegrationTests.swift b/cmuxTests/CmuxBundledBinPathIntegrationTests.swift new file mode 100644 index 000000000000..1a6295784d11 --- /dev/null +++ b/cmuxTests/CmuxBundledBinPathIntegrationTests.swift @@ -0,0 +1,178 @@ +import Foundation +import Testing + +private let cmuxBundledBinFishExecutablePath = [ + "/opt/homebrew/bin/fish", + "/usr/local/bin/fish", + "/usr/bin/fish", + "/bin/fish", +].first { FileManager.default.isExecutableFile(atPath: $0) } + +@Suite(.serialized) +struct CmuxBundledBinPathIntegrationTests { + enum Shell: String, CustomTestStringConvertible, Sendable { + case bash + case fish + case zsh + + var testDescription: String { rawValue } + } + + private struct ProcessResult { + let status: Int32 + let stdout: String + let stderr: String + } + + private struct ResolutionError: Error, CustomStringConvertible { + let description: String + } + + /// Regression for #9471: cmux's bundled commands must not depend on + /// Ghostty's optional helper environment to remain first on `PATH`. + @Test(arguments: [Shell.bash, .zsh]) + func bundledOpenWinsWithoutGhosttyBinEnvironment(shell: Shell) throws { + try assertBundledOpenWinsWithoutGhosttyBinEnvironment(shell: shell) + } + + @Test(.enabled(if: cmuxBundledBinFishExecutablePath != nil)) + func bundledOpenWinsWithoutGhosttyBinEnvironmentInFish() throws { + try assertBundledOpenWinsWithoutGhosttyBinEnvironment(shell: .fish) + } + + private func assertBundledOpenWinsWithoutGhosttyBinEnvironment(shell: Shell) throws { + guard let executable = Self.executable(for: shell) else { + throw ResolutionError(description: "\(shell.rawValue) is not installed") + } + + let fixture = try makeAppBundleFixture() + defer { try? FileManager.default.removeItem(at: fixture.root) } + + let result = run( + shell: shell, + executable: executable, + integrationDirectory: fixture.integrationDirectory, + home: fixture.root + ) + + guard result.status == 0 else { + throw ResolutionError(description: "\(shell.rawValue) stderr: \(result.stderr)") + } + let resolvedOpen = result.stdout.trimmingCharacters(in: .whitespacesAndNewlines) + guard resolvedOpen == fixture.openShim.path else { + throw ResolutionError( + description: "\(shell.rawValue) resolved \(resolvedOpen) instead of \(fixture.openShim.path); " + + "stderr: \(result.stderr)" + ) + } + } + + private func makeAppBundleFixture() throws -> ( + root: URL, + integrationDirectory: URL, + openShim: URL + ) { + let fileManager = FileManager.default + let root = fileManager.temporaryDirectory + .appending(path: "cmux issue 9471 \(UUID().uuidString)", directoryHint: .isDirectory) + let resources = root + .appending(path: "cmux.app/Contents/Resources", directoryHint: .isDirectory) + let integrationDirectory = resources + .appending(path: "shell-integration", directoryHint: .isDirectory) + let binDirectory = resources.appending(path: "bin", directoryHint: .isDirectory) + let openShim = binDirectory.appending(path: "open", directoryHint: .notDirectory) + let repositoryRoot = URL(fileURLWithPath: #filePath) + .deletingLastPathComponent() + .deletingLastPathComponent() + let shippedIntegration = repositoryRoot + .appending(path: "Resources/shell-integration", directoryHint: .isDirectory) + + try fileManager.createDirectory(at: binDirectory, withIntermediateDirectories: true) + try fileManager.copyItem(at: shippedIntegration, to: integrationDirectory) + try fileManager.createSymbolicLink(at: openShim, withDestinationURL: URL(fileURLWithPath: "/usr/bin/true")) + return (root, integrationDirectory, openShim) + } + + private func run( + shell: Shell, + executable: String, + integrationDirectory: URL, + home: URL + ) -> ProcessResult { + let process = Process() + let stdout = Pipe() + let stderr = Pipe() + process.executableURL = URL(fileURLWithPath: executable) + process.arguments = Self.arguments(for: shell, integrationDirectory: integrationDirectory) + process.environment = [ + "CMUX_FISH_USER_CONFIG_ALREADY_LOADED": "1", + "CMUX_SHELL_INTEGRATION": "1", + "CMUX_SHELL_INTEGRATION_DIR": integrationDirectory.path, + "HOME": home.path, + "PATH": "/usr/bin:/bin", + "SHELL": executable, + "TERM": "xterm-256color", + "USER": NSUserName(), + ] + process.standardInput = FileHandle.nullDevice + process.standardOutput = stdout + process.standardError = stderr + + do { + try process.run() + process.waitUntilExit() + } catch { + return ProcessResult(status: -1, stdout: "", stderr: error.localizedDescription) + } + + return ProcessResult( + status: process.terminationStatus, + stdout: String(data: stdout.fileHandleForReading.readDataToEndOfFile(), encoding: .utf8) ?? "", + stderr: String(data: stderr.fileHandleForReading.readDataToEndOfFile(), encoding: .utf8) ?? "" + ) + } + + private static func executable(for shell: Shell) -> String? { + let candidates: [String] + switch shell { + case .bash: + candidates = ["/bin/bash", "/usr/bin/bash"] + case .fish: + return cmuxBundledBinFishExecutablePath + case .zsh: + candidates = ["/bin/zsh", "/usr/bin/zsh"] + } + return candidates.first { FileManager.default.isExecutableFile(atPath: $0) } + } + + private static func arguments(for shell: Shell, integrationDirectory: URL) -> [String] { + switch shell { + case .bash: + return [ + "--noprofile", + "--norc", + "-c", + // The file path is positional so spaces stay literal. + "source \"$1\"; command -v open", + "bash", + integrationDirectory.appending(path: "cmux-bash-integration.bash").path, + ] + case .fish: + return [ + "--no-config", + "--command", + // Sourcing the integration exercises the same PATH repair without + // keeping Fish's interactive event loop alive after the command. + "source \"$argv[1]\"; command -s open; exit", + integrationDirectory.appending(path: "fish/config.fish").path, + ] + case .zsh: + return [ + "-dfc", + "source \"$1\"; _cmux_fix_path; whence -p open", + "zsh", + integrationDirectory.appending(path: "cmux-zsh-integration.zsh").path, + ] + } + } +} diff --git a/tests/test_claude_wrapper_user_binary_resolution.py b/tests/test_claude_wrapper_user_binary_resolution.py index f62b5ec28519..fca1b901ddb5 100644 --- a/tests/test_claude_wrapper_user_binary_resolution.py +++ b/tests/test_claude_wrapper_user_binary_resolution.py @@ -185,7 +185,11 @@ def test_shell_integration_preserves_empty_path_components(failures: list[str]) surface_id = "surface-path-test" shim_root = tmpdir / "cmux-cli-shims" / surface_id - expected_path = f"{shim_root}::{first}::{last}:" + bundled_bin = SHELL_INTEGRATION_DIR.parent / "bin" + expected_paths = { + "bash": f"{bundled_bin}:{shim_root}::{first}::{last}:", + "zsh": f"{shim_root}:{bundled_bin}::{first}::{last}:", + } input_path = f":{first}::{shim_root}:{last}:" base_env = minimal_env("/usr/bin:/bin", tmpdir) @@ -209,6 +213,7 @@ def test_shell_integration_preserves_empty_path_components(failures: list[str]) "-c", 'PATH="$CMUX_TEST_INPUT_PATH"; ' 'source "$CMUX_SHELL_INTEGRATION_DIR/cmux-zsh-integration.zsh"; ' + '_cmux_fix_path; ' 'printf "%s\\n" "$PATH"', ], ] @@ -221,6 +226,7 @@ def test_shell_integration_preserves_empty_path_components(failures: list[str]) f"{shell_name} path preservation exited {result.returncode}: " f"{(result.stdout + result.stderr).strip()}" ) + expected_path = expected_paths[shell_name] if output != expected_path: failures.append(f"{shell_name} expected PATH {expected_path!r}, got {output!r}")