[nwparker]

	Files	Added	Deleted	Net
Test	5	$\color{#1a7f37}{\Huge{\mathbf{+}}}$​283	$\color{#cf222e}{\Huge{\mathbf{−}}}$​8	$\color{#1a7f37}{\Huge{\mathbf{+}}}$​275
Prod	4	$\color{#1a7f37}{\Huge{\mathbf{+}}}$​104	$\color{#cf222e}{\Huge{\mathbf{−}}}$​16	$\color{#1a7f37}{\Huge{\mathbf{+}}}$​88

ELI5

SSH commit and PR generation should find agents installed by the remote user’s bash or zsh startup files.

What Changed

Opt only generation calls into a supported POSIX login shell. Pass cwd, executable and arguments as positional data; fence startup banners from returned output; preserve stdin and exec replacement for cancellation. Reapply explicit environment values and generated guards after startup so a profile cannot overwrite the caller’s chosen PATH or variables.

Why

This reuses the existing framed-shell capture and safe spawn abstractions. The new regression failed in both bash and zsh before the environment fix, and now passes without evaluating caller strings.

Linked Issue

Related to @andy-murr’s #5197; #4720 provides original context.

Visual Proof

Historical hidden-renderer proof from audit baseline 59c0d5585e30. A synthetic agent returns the fixed response through production Source Control UI; no paid agent request. These images illustrate unchanged behavior after the current-main merge.

Before:

After:

Testing

The hook-order parity repair is now also on main as #23090. Latest follow-up 69ac62a406a2 aligns only its provenance comment with that upstream version, removing the merge conflict; assertions and product code are unchanged. The full test file matches main byte-for-byte. All three focused parity tests, normal commit lint/format checks and a local merge check pass. All current-head checks have passed or been skipped as intended, including required PR Checks verification, changed-spec E2E, Docker SSH E2E and real WSL terminal validation.

• Automated regressions added and run
• Historical hidden-renderer fixture evidence attached

57 tests pass, two skip on macOS, with Node typecheck and changed-code quality passing before stacking. The combined actual-source suite (command parsing through relay execution) passes 292 tests with two skipped; it verifies explicit PATH and literal values against conflicting bash/zsh startup files. No authenticated OpenCode call, native Windows/WSL or deployed full SSH UI transport in this pass.

All tests used ORCA_BACKGROUND_LAUNCH=1; no visible test window or real agent account was launched. Full lint, test suite and release packaging remain CI coverage.

Review

After #22427 merged, integrated current main (85ac14e9c263) without rewriting branch history. The PR now targets main directly and contains eight SSH-login source/test files plus the parity-test repair that now matches main; the main integration preserved all eight files from the prior reviewed head 096bf3fb7dd3. Kept main’s updated environment-helper typing when resolving the parent’s squash overlap.

A fresh independent review reproduced one additional edge in real bash and zsh: when the relay already inherited safe credential-prompt settings, the sparse environment override omitted them and a startup profile could reenable prompts. fe3914586569 now reapplies those guards and their complete indexed Git config after startup. The regression also proves inherited caller config survives.

Fresh validation on fe3914586569: 155 focused tests passed, 2 platform skips, Node typecheck passed, and changed-code quality passed with zero new findings across the eight files. All current-head checks have passed or been skipped as intended, including required PR Checks verification, changed-spec E2E, Docker SSH E2E and real WSL terminal validation. Final validation at 69ac62a406a2 completed all of those checks successfully.

Addressed Pullfrog feedback in 0acca9e2748, then merged the parent follow-up into 096bf3fb7dd3: missing agents again return actionable ENOENT errors, ordinary agent exit 127 remains distinct, and stream errors cannot crash the relay. Pullfrog’s review at fe3914586569 was clean and confirmed the new credential-guard regression fails when the fix is removed. Both original threads remain resolved. The parity-only follow-up also passed Pullfrog; the latest comment-only alignment also passed Pullfrog, which verified the unchanged assertions and reported no new issues.

Independently reviewed literal argv/environment handling, host ownership, Windows casing, WSL forwarding, cancellation/cleanup and mixed-version fallback. The original integration base was 996f9cc3068c.

Agent skill upstream boundary

• Not applicable; no upstream skill content copied.

Notes

The environment-prefix change #22427 is merged. This PR is now independent and targets main. Unsupported POSIX shells keep direct execution; old relays ignore the optional flag and retain prior behavior. Interactive startup files that block or consume stdin remain outside this fix.

Checklist

• Scoped before/after behavior and approach explained
• Before/after proof attached
• Correctness, security, performance, platform and remote behavior reviewed
• Current-head CI checks complete, including optional SSH E2E checks

[nwparker]

@coderabbitai review

[coderabbitai]

⚠️ Action not completed

Review rate limited.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

[coderabbitai]

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Advanced

Run ID: 4116eaaa-4fde-4b72-a84f-f248dbc1f40e

📥 Commits

Reviewing files that changed from the base of the PR and between fe39145 and 72fbca5.

📒 Files selected for processing (1)

• src/renderer/src/components/terminal-pane/terminal-pane-hook-order-parity.test.ts

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 0 remain after this review.

---

📝 Walkthrough

Walkthrough

SSH commit-message operations now request login-shell execution. The relay supports this option for Bash and Zsh, preserves selected environment values, processes captured output, and reports missing binaries. The WSL captured-login-shell helper now uses an exported function to build nonce-marked commands. The terminal hook-order parity test updates its expected hook count and digest.

Priority: ➖ Normal

Merge Risk: ⚪ Minimal · up to 72fbc

The SSH login-shell request reaches the relay as intended. No actionable merge-blocking risk is established by the reviewed changes.

Security Architecture Review

Security architecture risk: 🔵 Low · up to 72fbc

The change is limited to opted-in remote generation on supported shells. Arguments remain separate from shell command text, and the existing unattended credential protections are applied before launch. Login startup files do introduce a conditional, user-controlled execution path that merits design review; no exploit is established.

Retained concerns

• Low · security · inferred: Opted-in generation now executes remote user startup files before the agent. Those files can run commands or alter executable selection before the relay reapplies selected environment values. The effective risk depends on who can modify the files and PATH directories; neither is established, and this is not a verified credential-guard bypass.

Security review details

Security Blast Radius

• inferred — The newly selected startup files and PATH affect opted-in generation processes under the remote user’s account, not all relay commands. The evidence does not establish cross-user access or a wider deployment exposure.

Security Findings and Attack Paths

• inferred — A party able to modify startup files or a PATH directory used by them could influence startup execution or agent selection on this new path. The supplied evidence does not show such access by a lower-trust party or a successful attack.

Trust Boundaries and Controls

• observed — The relay does not interpolate caller binary, arguments, or cwd into shell command text. Its credential guard is present in the spawned environment and selected guard values are restored for the agent, although restoration occurs after startup files run.

Resilience and Maintainability Implications

• observed — Cancellation targets the operation lane’s child process, and timeout invokes subprocess-tree termination. Completion removes only the matching lane entry; the record does not establish how arbitrary startup-file subprocesses behave under interruption.

Hardening Proposals

• proposed — If remote workspaces or collaborators can influence startup files or PATH directories, define which executable locations generation may trust and verify that startup-time commands cannot escape the unattended credential policy.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 40.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 5 functions across 9 files.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Title check	✅ Passed	The title clearly identifies the primary change: enabling SSH generation agents to be found through the remote user's login shell.
Description check	✅ Passed	The description is detailed and covers the change, rationale, linked issues, visual proof, testing, platform limitations, review notes, and checklist. It uses related issue references instead of the t…

• Fix all pre-merge checks with AI

✨ Finishing Touches 💡 1

📝 Generate docstrings 💡

• Commit to this branch
• Create a new PR

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[pullfrog]

ℹ️ No critical issues — the login-shell change is sound. One error-classification regression and a small robustness nit are inline.

Reviewed changes

Reviewed the full 8-file diff against nwparker/external-5363-command-env (5170a86).

• Login-shell generation opt-in — executeCommitMessagePlan sends loginShell: true; execNonInteractive deliberately does not. Windows, unsupported POSIX shells, and non-boolean values fall back to the prior direct spawn.
• Fenced login-shell argv builder (agent-exec-login-shell.ts) — runs shell -ilc with cd -- "$1"; exec "$@" through /usr/bin/env, so args and env stay literal argv data and exec keeps cancellation targeting the real agent.
• Relay spawn path (agent-exec-handler.ts) — re-applies explicit env after startup files, spawns via spawnProcess, and fences stdout.
• Fence builder extraction (wsl-login-shell-command.ts) — behavior-preserving split enabling the SSH reuse.
• Tests — real bash/zsh integration coverage for login-PATH discovery, literal argv/env, and cancel; updated spawn-option expectations.

  ^{｜ Fix all ➔ ｜ Fix 👍s ➔ ｜ View workflow run ｜ Using DeepSeek Flash (free via Pullfrog for OSS) ｜ 𝕏}

[pullfrog]

✅ No new issues found.

Reviewed changes

Reviewed the delta since the prior Pullfrog review (23edaa7 → 096bf3f), which merges the parent follow-up at 0acca9e2748 and resolves both prior findings.

• Missing-binary classification restored — a /bin/sh -c guard now command -v checks the resolved binary after explicit env is applied, prints a per-call missingMarker and exits 127; finish maps exitCode === 127 && slicedStdout === missingMarker to spawnError: spawn <binary> ENOENT with empty stdout/stderr, so the client's actionable "not found / Install X there" wording returns. An agent's own exit 127 with output stays an ordinary failure.
• Stream error guards extended — no-op error listeners now cover stdin, stdout and stderr, retained after close, so a late broken pipe cannot crash the relay.
• Regression coverage — the new bash/zsh test asserts the exact missing-binary result and a real agent exiting 127 without spawnError, and the handler test emits stdout/stderr errors (including one after close). Verified locally: those suites pass (18 passed / 3 skipped on this runner).

  ^{｜ View workflow run ｜ Using DeepSeek Flash (free via Pullfrog for OSS) ｜ 𝕏}

[pullfrog]

✅ No new issues found.

Reviewed changes

Reviewed the delta since the prior Pullfrog review (096bf3f → fe39145), which retargets the branch to main after #22427 landed and adds one guard fix.

• Kept inherited credential guards through login startup — loginShellEnv now retains GIT_TERMINAL_PROMPT, GCM_INTERACTIVE, GIT_ASKPASS, SSH_ASKPASS, and every GIT_CONFIG_* entry even when they match the relay's own process.env, so /usr/bin/env re-applies the complete guard after the profile runs.
• Added a real bash/zsh regression — stubs inherited guards and an adversarial profile, then asserts the agent sees the guarded values and the full indexed config (GIT_CONFIG_COUNT=3 plus the two appended entries). Confirmed it fails when the filter clause is removed.
• Reformatted three unrelated test files (594eb1e) to match main; no behavioral change.

  ^{｜ View workflow run ｜ Using DeepSeek Flash (free via Pullfrog for OSS) ｜ 𝕏}

[OrcaWin]

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.

[OrcaWin]

Follow-up: on orcad, commit/PR generation runs on the host, and orcad merges the user's login-shell PATH at startup (#27319, nushell included via shellPathProbe), so agents installed by bash/zsh startup files are found. Closing as fixed.