Skip directories when resolving provider executables on PATH - #9476
Conversation
FileManager.isExecutableFile(atPath:) returns true for directories on macOS, so a directory named like a provider binary earlier on PATH is selected by the CLI and app PATH walks. These tests fail until the resolvers reject directories. Refs #8743 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
FileManager.isExecutableFile(atPath:) returns true for directories on macOS, so a directory named like a provider binary (~/bin/omx/, ~/bin/claude/) earlier on PATH was selected as the executable and the launch failed at execv with a confusing "Permission denied". Reject directories with fileExists(atPath:isDirectory:) before the executable check in all three PATH walks, mirroring the guard resolveClaudeExecutable already applied to configured candidates. Fixes #8743 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (6)
📝 WalkthroughWalkthroughExecutable resolution now skips directories that share provider executable names. Unit and CLI regression tests verify that later valid executables are selected, and CI runs the new CLI regression test. ChangesPATH executable resolution
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
Suggested reviewers: Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (1 error, 1 warning)
✅ Passed checks (23 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
FileManager.isExecutableFile(atPath:)returns true for directories on macOS. The PATH walks that resolve provider binaries only called that API, so a directory named like the binary (~/bin/omx/,~/bin/claude/) earlier on PATH was picked as the executable and the launch died atexecvwithFailed to launch omx: Permission denied.Three PATH walks now reject directories with
fileExists(atPath:isDirectory:)before the executable check, mirroring the guardresolveClaudeExecutablealready applied to configured candidates:CLI/CMUXCLI+ExecutableResolution.swift—resolveExecutableInSearchPath(claude, codex, opencode, omx, omc, auto-naming summarizers)CLI/cmux.swift—resolveExecutableInPath(bun and the OMO/OMX/OMC helpers)Sources/AgentExecutableResolver.swift— the app-side agent resolver, same bugFixes #8743
Testing
tests/test_issue_8743_path_directory_shadowing.py(new, wired intoci.yml): puts a directory namedomx/omcahead of a real executable on PATH and asserts the real one runs. Before the fix:FAIL ... Failed to launch omx: Permission deniedfor both. After:PASS.cmuxTests/AgentExecutableResolverTests.swift—testSkipsDirectoryNamedLikeExecutableOnSearchPathcovers the app-side resolver../scripts/reload.sh --tag sym8743, then the issue's repro against the tagged CLI: with/tmp/.../shadow/omx(a directory) first on PATH,cmux omx --versionnow printsREAL OMX at /tmp/.../real/omxinstead of failing. Tagged app killed afterwards.🤖 Generated with Claude Code
Need help on this PR? Tag
@codesmith-botwith what you need. Autofix is disabled.Summary by cubic
Skip directories when resolving provider executables on PATH so the CLI and app don’t select a directory named like the binary and fail to launch on macOS. Fixes #8743.
isExecutableFilein all PATH walks:CLI/CMUXCLI+ExecutableResolution.swift,CLI/cmux.swift, andSources/AgentExecutableResolver.swift.tests/test_issue_8743_path_directory_shadowing.py,cmuxTests/AgentExecutableResolverTests.swift) and wired the new test into CI.Written for commit 3e53303. Summary will update on new commits.
Summary by CodeRabbit
Bug Fixes
Tests