fix(ssh): survive csh/tcsh login shells when running multiline relay commands

[xianjianlf2]

Fixes #8701

Problem

Adding an SSH remote whose login shell is csh/tcsh (common on EDA/HPC farms, often centrally managed) fails with Node.js not found on remote host, even with Node 18+ installed and on PATH.

Root cause

sshd hands every exec command string to the user's login shell for parsing. Orca wraps remote commands as exec /bin/sh -c '<script>' (wrapRemoteCommandForPosixShell), which is the right idea — but csh/tcsh cannot keep a single-quoted multiline argument intact. Each line is re-parsed as its own csh command (Unmatched '''. / for: Command not found.), so /bin/sh never runs. Both Node-detection strategies were multiline, and the wrapper is used for all relay commands, so any multiline relay command broke the same way. Reproduced locally: /bin/csh -c "<wrapped multiline>" fails exactly as the issue reports; the single-line form succeeds.

A second, independent break: the login-shell fallback ran "$SHELL" -lc 'command -v node'. csh/tcsh reject the combined -lc flag outright (Unknown option: '-lc'), and have no command builtin either.

Fix

1. wrapRemoteCommandForPosixShell — when the command contains a newline, collapse it to one line: encode backslashes and newlines, and let /bin/sh rebuild the script with eval "$(printf %b '<encoded>')". printf is a POSIX sh builtin (no base64 dependency), eval keeps stdin free for commands that stream data (file transfer), and exit codes propagate. Single-line commands keep the exact previous form, so existing behavior on POSIX/fish/nushell login shells is untouched.
2. Login-shell Node probe — csh/tcsh get -c 'which node' instead of -lc 'command -v node': which is the csh-family PATH resolver, and non-login csh still reads .cshrc, which is where EDA/HPC farms set PATH.

Testing

• New ssh-remote-command-wrapping.test.ts runs the wrapped output through real login shells (sh, bash, zsh, dash, csh, tcsh — skipping ones not installed), exactly how sshd invokes them: multiline scripts with quotes/backslashes/expansions, exit-code propagation, and stdin passthrough. The csh/tcsh cases fail on the old wrapper and pass with the fix.
• New unit tests pin -c 'which node' for csh/tcsh in the login-shell fallback; single-line wrapper output is asserted byte-identical to the previous form.
• Verified end-to-end: the actual Strategy-1 probe script through the new wrapper under /bin/csh and /bin/tcsh finds the same node candidates as under bash.
• Full src/main/ssh/ suite passes (the two pre-existing failures — node:sqlite availability and an ssh-config glob-order case — fail identically on a clean tree).

🤖 Generated with Claude Code

[coderabbitai]

📝 Walkthrough

Walkthrough

Remote command wrapping now preserves direct handling for single-line commands and encodes multiline scripts into newline-free /bin/sh wrappers that reconstruct and evaluate the original script. Tests cover quoting, expansions, exit status, stdin, and multiple shells. Node resolution now uses which node with -c for csh and tcsh, with tests covering both shell types.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Description check	⚠️ Warning	The description covers summary and testing, but it omits required Screenshots, AI Review Report, Security Audit, and Notes sections.	Add the missing template sections, including Screenshots, AI Review Report, Security Audit, and Notes, and fill Screenshots with 'No visual change' if applicable.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly matches the main SSH fix and correctly calls out csh/tcsh multiline relay command handling.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
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.

---

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.}

[coderabbitai]

Actionable comments posted: 1

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: 641a4c8c-9917-4730-8f79-a58087b64f43

📥 Commits

Reviewing files that changed from the base of the PR and between 31f643c and 0708cb8.

📒 Files selected for processing (4)

• src/main/ssh/ssh-connection-utils.ts
• src/main/ssh/ssh-remote-command-wrapping.test.ts
• src/main/ssh/ssh-remote-node-resolution.test.ts
• src/main/ssh/ssh-remote-node-resolution.ts

[coderabbitai]

📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

Use Node path utilities instead of assuming path separators.

As per coding guidelines, use Node path utilities for file paths and never assume / or \. Since this function operates on remote POSIX shell paths, path.posix.basename() is the proper utility and correctly handles edge cases (like trailing slashes) that manual splitting might miss.

Note: Please ensure path is imported at the top of the file (e.g., import path from 'node:path') if it isn't already.

♻️ Proposed refactor

-function isCshFamilyShell(shell: string): boolean {
-  const shellName = shell.split('/').at(-1)
+function isCshFamilyShell(shell: string): boolean {
+  const shellName = path.posix.basename(shell)
   return shellName === 'csh' || shellName === 'tcsh'
 }
 
 function buildCommandInShell(shell: string, command: string): string {
-  const shellName = shell.split('/').at(-1)
+  const shellName = path.posix.basename(shell)
   // Why: dash and POSIX sh do not require `-l`; when $SHELL falls back to
   // /bin/sh, prefer a portable command over login-shell semantics. csh/tcsh
   // reject the combined `-lc` flag outright (`#8701`); their non-login shells
   // still read .cshrc, where EDA/HPC farms set PATH.
   const mode = shellName === 'sh' || shellName === 'dash' || isCshFamilyShell(shell) ? '-c' : '-lc'

📝 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.

Suggested change

	function isCshFamilyShell(shell: string): boolean {
	const shellName = shell.split('/').at(-1)
	return shellName === 'csh' || shellName === 'tcsh'
	}
	function buildCommandInShell(shell: string, command: string): string {
	const shellName = shell.split('/').at(-1)
	// Why: dash and POSIX sh do not require `-l`; when $SHELL falls back to
	// /bin/sh, prefer a portable command over login-shell semantics.
	const mode = shellName === 'sh' || shellName === 'dash' ? '-c' : '-lc'
	// /bin/sh, prefer a portable command over login-shell semantics. csh/tcsh
	// reject the combined `-lc` flag outright (#8701); their non-login shells
	// still read .cshrc, where EDA/HPC farms set PATH.
	const mode = shellName === 'sh' || shellName === 'dash' || isCshFamilyShell(shell) ? '-c' : '-lc'
	function isCshFamilyShell(shell: string): boolean {
	const shellName = path.posix.basename(shell)
	return shellName === 'csh' || shellName === 'tcsh'
	}
	function buildCommandInShell(shell: string, command: string): string {
	const shellName = path.posix.basename(shell)
	// Why: dash and POSIX sh do not require `-l`; when $SHELL falls back to
	// /bin/sh, prefer a portable command over login-shell semantics. csh/tcsh
	// reject the combined `-lc` flag outright (`#8701`); their non-login shells
	// still read .cshrc, where EDA/HPC farms set PATH.
	const mode = shellName === 'sh' || shellName === 'dash' || isCshFamilyShell(shell) ? '-c' : '-lc'

Source: Coding guidelines