Repository navigation
Conversation
OpenCode and other agents installed via shell startup files (nvm/fnm/ Homebrew) could not be found when generating commit/PR messages over SSH. The relay runs on the remote host via a non-interactive SSH command whose PATH lacks those install dirs, so the agent binary was not found and generation failed. Opt the SSH commit/PR-generation spawn into login+interactive shell (bash/zsh -ilc) PATH resolution, matching the established pattern in hydrate-shell-path.ts and preflight-handler.ts. The interactive flag is required so .bashrc/.zshrc PATH hooks get sourced. Windows and non-bash/zsh shells are unchanged. Fixes #4720 Supersedes #5197. Co-authored-by: Andy Murray <andym3@illinois.edu> Co-authored-by: Orca <help@stably.ai>
Test & verification evidenceFocused unit tests (the regression guards)New / updated tests proving the fix: This new test asserts the relay spawns:
Windows path unchanged
Lint / format / typecheck (scoped to changed files)Pre-existing unrelated failures (confirmed present on Relay buildBefore / after reasoning
Security self-auditThe shell invocation passes the binary and arguments as separate positional parameters ( |
📝 WalkthroughWalkthrough
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: a3f6a0a2-3f84-41cf-b776-13713e25781f
📒 Files selected for processing (4)
src/main/providers/ssh-git-provider.test.tssrc/main/providers/ssh-git-provider.tssrc/relay/agent-exec-handler.test.tssrc/relay/agent-exec-handler.ts
| const shell = resolveDefaultShell() | ||
| const shellName = basename(shell).toLowerCase() | ||
| if (shellName !== 'bash' && shellName !== 'zsh') { | ||
| return { spawnCmd: binary, spawnArgs: args } | ||
| } | ||
| // Why: SSH relay processes are launched by a non-interactive SSH command, | ||
| // whose PATH often lacks nvm/fnm/Homebrew agent installs. The interactive | ||
| // flag (`-i`) is required so .bashrc/.zshrc — where those PATH hooks live — | ||
| // get sourced; a plain login shell (`-lc`) would skip them. This resolves | ||
| // remote agent commands like the user's Orca terminal shell does. | ||
| return { | ||
| spawnCmd: shell, | ||
| spawnArgs: ['-ilc', 'exec "$@"', '_', binary, ...args] |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Only enable the wrapper when the user's shell is explicitly bash/zsh.
resolveDefaultShell() falls back to /bin/bash//bin/zsh when SHELL is unset, so this branch changes behavior for “unknown shell” hosts instead of preserving the existing direct-spawn path. That means a fish/csh user can still get bash/zsh startup files sourced here, which breaks the bash/zsh-only contract in this PR.
Suggested fix
-import { resolveDefaultShell } from './pty-shell-utils'
...
- const shell = resolveDefaultShell()
- const shellName = basename(shell).toLowerCase()
+ const shell = typeof env.SHELL === 'string' ? env.SHELL : ''
+ const shellName = shell ? basename(shell).toLowerCase() : ''
if (shellName !== 'bash' && shellName !== 'zsh') {
return { spawnCmd: binary, spawnArgs: args }
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const shell = resolveDefaultShell() | |
| const shellName = basename(shell).toLowerCase() | |
| if (shellName !== 'bash' && shellName !== 'zsh') { | |
| return { spawnCmd: binary, spawnArgs: args } | |
| } | |
| // Why: SSH relay processes are launched by a non-interactive SSH command, | |
| // whose PATH often lacks nvm/fnm/Homebrew agent installs. The interactive | |
| // flag (`-i`) is required so .bashrc/.zshrc — where those PATH hooks live — | |
| // get sourced; a plain login shell (`-lc`) would skip them. This resolves | |
| // remote agent commands like the user's Orca terminal shell does. | |
| return { | |
| spawnCmd: shell, | |
| spawnArgs: ['-ilc', 'exec "$@"', '_', binary, ...args] | |
| const shell = typeof env.SHELL === 'string' ? env.SHELL : '' | |
| const shellName = shell ? basename(shell).toLowerCase() : '' | |
| if (shellName !== 'bash' && shellName !== 'zsh') { | |
| return { spawnCmd: binary, spawnArgs: args } | |
| } | |
| // Why: SSH relay processes are launched by a non-interactive SSH command, | |
| // whose PATH often lacks nvm/fnm/Homebrew agent installs. The interactive | |
| // flag (`-i`) is required so .bashrc/.zshrc — where those PATH hooks live — | |
| // get sourced; a plain login shell (`-lc`) would skip them. This resolves | |
| // remote agent commands like the user's Orca terminal shell does. | |
| return { | |
| spawnCmd: shell, | |
| spawnArgs: ['-ilc', 'exec "$@"', '_', binary, ...args] |
🧰 Tools
🪛 ast-grep (0.44.0)
[warning] Importing child_process exposes a command-execution surface; ensure any command/argument built from input is validated, and prefer execFile/spawn with an argument array over exec.
Context: import { exec, spawn, type ChildProcess } from 'child_process'
Note: [CWE-78] Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection').
(detect-child-process-typescript)
|
Follow-up pushed in What changed:
Tradeoff status: not fully zero for the original PATH-missing case. If an SSH agent is only discoverable because Validation:
|
|
Follow-up pushed in What changed:
Tradeoff status: zero shell startup/rc cost for successful normal SSH users, and zero shell-wrapped generation. The only remaining rc execution is the necessary compatibility path-discovery case when the agent genuinely exists only after Validation:
|
|
Closing as obsolete after the merged SSH relay retirement in #27214 and #27215. This PR concerns the relay-backed remote Orca CLI bridge and agent execution. #27214 removed the desktop relay connection/fallback path; #27215 deleted the SSH relay server, deployment assets, and relay-only tests. SSH hosts now use the managed Orca server (orcad), and unsupported hosts fail explicitly, so this relay-specific work is no longer planned. This closes the proposed relay implementation; it does not claim that any independent local, WSL, or managed-server behavior was fixed. |
|
The relay files this PR touched were deleted, so it can't be revived as-is. The symptom still applies on the managed remote server, though, so I've carried it over to #27395. |
2 participants
Description
Fixes #4720 by resolving OpenCode (and other commit/PR agent binaries) through the remote login + interactive shell environment before execution, so SSH uses the PATH the user actually has configured.
Evidence of fix
Before: a safe fake
opencodewas visible from the isolated remote login shell, but Orca's production OpenCode commit-message planner + SSH relay returnedopencode not found on the remote PATH.After: the focused PR resolves the agent through the remote login/interactive shell PATH and the same production planner reaches the discovered executable. The PR's targeted SSH/shell tests, lint, typechecks, and GitHub
verifycheck pass.ELI5
Orca was checking the remote machine's “bare hallway” PATH, while OpenCode lived in the PATH your shell sets up after you log in. The fix asks the logged-in shell where the tool is, then uses that answer.
User-facing trade-offs
Impact
Fixes #4720. On SSH targets, OpenCode and other agent CLIs installed through nvm/fnm/Homebrew shell startup files could appear in Orca settings and terminal agents, but commit-message / PR-description generation could still fail because the relay is launched by a non-interactive SSH command with a shorter PATH.
What Changed
The SSH commit/PR agent path is now direct-first and shell-free for generation:
spawn(binary, args).command -vlookup, captures the shell-initialized PATH, caches the absolute path, then spawns the agent directly by absolute path.<shell> -ilc 'exec "$@"' ....Tradeoff Status
Zero-tradeoff design for normal successful SSH users: agents already on inherited PATH pay no shell startup/rc cost.
The previous open tradeoff, shell-wrapped generation, is eliminated. The remaining rc execution is only a necessary compatibility path-discovery fallback for the original bug class when no direct PATH or preflight cache can identify the agent. If OpenCode exists only because
.bashrc/.zshrcmutates PATH, the relay has no deterministic non-shell source for those dynamic PATH entries. In that case Orca runs only a boundedcommand -vlookup, caches the result, and still launches the agent directly rather than under the shell.Behavior Matrix
command -vlookup, then direct absolute-path spawn with the discovered PATH..cmd/cmd.exepath remains unchanged.execNonInteractive-> unchanged; the shell lookup flag is still only set by SSH commit/PR generation.Testing
pnpm exec vitest run --config config/vitest.config.ts src/relay/preflight-handler.test.ts src/relay/agent-exec-handler.test.ts src/relay/agent-exec-handler-windows.test.ts src/main/providers/ssh-git-provider.test.ts-> 112 passedpnpm exec oxlinton touched relay/provider files -> cleanpnpm exec oxfmt --checkon touched relay/provider files -> cleanpnpm run typecheck:node-> cleanpnpm run build:relay-> built all 6 relay targetsgit merge-tree --write-tree HEAD origin/main-> exit 0Merged current
origin/maininto the PR branch to keep #6559 applying cleanly.Co-authored-by: Andy Murray andym3@illinois.edu
Made with Orca 🐋