Harden SSH LocalCommand fish reconnect regression

[austinywang]

Closes #3533.

Summary

• run the SSH LocalCommand regression through fish to match OpenSSH local-login-shell execution
• rename reconnect/timing helpers to make clear they return POSIX script bodies, while openSSHLocalCommandValue owns the final /bin/sh -c wrapped LocalCommand

Regression-test note

Current main already includes the centralized /bin/sh -c OpenSSH LocalCommand wrapper from #3506, so the new test-hardening commit cannot demonstrate a red-first CI proof window against this base. This PR closes the remaining coverage gap by making the fake OpenSSH harness execute LocalCommand through fish instead of /bin/sh.

Verification

• git diff --check
• python3 scripts/swift_file_length_budget.py --budget .github/swift-file-length-budget.tsv
• swiftc -typecheck CLI command files
• Plain SSH LocalCommand repro: forcing SHELL=$(command -v fish) with bare cmux_localcommand_probe=ok fails with fish: Unsupported use of =
• Plain SSH fixed-shape repro: forcing the same fish context with /bin/sh -c 'cmux_localcommand_probe=ok; ...' prints local-command-ok\n- Tagged dev app: ./scripts/reload.sh --tag issue-3533-ssh-localcommand-fish --launch\n- Tagged dev CLI/socket: CMUX_SOCKET_PATH=/tmp/cmux-debug-issue-3533-ssh-localcommand-fish.sock /tmp/cmux-cli ssh ... austinywang@Austins-MacBook-Pro, then remote command returned CMUX_DEV_SSH_OK user=austinywang host=Austins-MacBook-Pro.local shell=/bin/zsh\n- Tagged dev log: remote.bootstrap.ready, remote.tty.bootstrap.ready, and remote.proxy.ready; no fish: Unsupported use of = or cmux_reconnect_cli= LocalCommand assignment errors\n\n## Not tested\n- Local XCTest suite per repo policy\n- CI still in progress; live dogfood was prioritized per latest request

[vercel]

The latest updates on your projects. Learn more about Vercel for GitHub.

Project	Deployment	Actions	Updated (UTC)
cmux	Ready	Preview, Comment	May 5, 2026 5:35am
cmux-staging	Building	Preview, Comment	May 5, 2026 5:35am

[coderabbitai]

📝 Walkthrough

Walkthrough

The PR updates SSH local command generation in cmux.swift to use "command script" helpers instead of "command" helpers, ensuring scripts are wrapped and executable under POSIX sh regardless of the user's login shell. A test is updated to pass the fish shell executable through an environment variable for proper test isolation.

Changes

Fish Shell SSH LocalCommand Compatibility

Layer / File(s)	Summary
Helper Function Refactor CLI/cmux.swift	deferredRemoteReconnectLocalCommand(...) and sshConnectionTimingLocalCommand(...) are renamed to deferredRemoteReconnectLocalCommandScript(...) and sshConnectionTimingLocalCommandScript(...) respectively, signaling that scripts are wrapped for POSIX sh execution.
Core Wiring Updates CLI/cmux.swift	Calls to the renamed helpers are updated; combinedLocalCommandScript combines both scripts; configuredForegroundAuthToken and auto_connect now key off deferredRemoteReconnectCommandScript == nil instead of deferredRemoteReconnectCommand == nil.
Remote Configuration CLI/cmux.swift	SSH remote configure command's deferredReconnect argument is updated from deferredRemoteReconnectCommand == nil to deferredRemoteReconnectCommandScript == nil.
Test Environment Setup cmuxTests/WorkspaceSSHFishShellTests.swift	fishExecutable is resolved early and propagated via CMUX_TEST_LOCAL_SHELL environment variable; the fake SSH handler executes LocalCommand using the environment variable instead of hard-coded /bin/sh, and redundant later resolution is removed.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Possibly related PRs

• manaflow-ai/cmux#3506: Partial fix for fish shell incompatibility in SSH RemoteCommand; this PR extends the fix to SSH LocalCommand by wrapping it in shell scripts.
• manaflow-ai/cmux#2564: Previous modifications to SSH local-command generation and related auto_connect/foreground-auth logic in the same area of CLI/cmux.swift.

Poem

🐰 Whiskers twitch as fish now swims,
No more POSIX syntax whims,
Scripts wrapped tight in sh -c bliss,
Shell-agnostic—can't resist!

🚥 Pre-merge checks | ✅ 12 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 20.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (12 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title 'Harden SSH LocalCommand fish reconnect regression' clearly summarizes the main change: addressing fish shell compatibility in SSH LocalCommand execution. It accurately reflects the core issue being fixed.
Linked Issues check	✅ Passed	The PR directly addresses issue #3533 by executing the SSH LocalCommand through fish in tests and renaming helpers to clarify script body handling. The changes ensure the reconnect script works under fish shells.
Out of Scope Changes check	✅ Passed	All changes are directly related to fixing the fish shell incompatibility in SSH LocalCommand execution and clarifying helper function responsibilities. No out-of-scope modifications detected.
Cmux Swift Actor Isolation	✅ Passed	No actor isolation violations. Pure utility functions on non-isolated struct returning Sendable types. Tests properly annotated with @MainActor.
Cmux Swift Blocking Runtime	✅ Passed	No blocking/timing primitives introduced in production code. PR refactors string-generating SSH helpers and adds test harness with DispatchSemaphore (allowed test scaffolding).
Cmux Swift Concurrency	✅ Passed	No new legacy async patterns in production code. New helpers are pure string manipulation. Test synchronization is pre-existing and allowed per rules.
Cmux Swift @Concurrent	✅ Passed	New synchronous helper functions have no async/concurrent isolation violations. Simple string builders with no @concurrent annotations needed or added.
Cmux Swift File And Package Boundaries	✅ Passed	Net-neutral changes (+9/-9, +3/-3). Within budget. Clarifying private helper renames. No public API, mixed responsibilities, or material expansion. Focused bug fix exception.
Cmux Swift Logging	✅ Passed	No logging violations. New helper functions contain no NSLog, Logger, debugPrint, dump. Auth tokens properly handled in scripts. All print statements are CLI output (allowed).
Cmux Swiftui State Layout	✅ Passed	PR contains no SwiftUI code. All changes are in CLI foundation code (SSH command helpers and tests) with zero ObservableObject, @Published, @Observable, GeometryReader, or state patterns.
Cmux Architecture Rethink	✅ Passed	Small correctness fix. Renames clarify script producers; centralized wrapping in openSSHLocalCommandValue. No timing repairs or split lifecycle. Test-only sync allowed.
Description check	✅ Passed	The PR description comprehensively covers what changed, why it changed, and provides detailed verification steps including manual testing of the fix.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch issue-3533-ssh-localcommand-fish

---

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 and usage tips.}

[greptile-apps]

Greptile Summary

This PR hardens the SSH LocalCommand fish-shell reconnect path with two changes: the production code renames deferredRemoteReconnectLocalCommand and sshConnectionTimingLocalCommand to the …Script suffix to make clear they return POSIX script bodies (not the final LocalCommand value), and the existing test now threads the LocalCommand through fish (via CMUX_TEST_LOCAL_SHELL) to match how OpenSSH actually invokes it for fish-login-shell users. The fishExecutable lookup is also moved to the top of the test so CI skips early on machines without fish installed, rather than doing most of the setup work first.

Confidence Score: 4/5

Safe to merge — production changes are pure renames with no logic delta, and the test change correctly simulates fish login-shell execution of LocalCommand.

The production code is a mechanical rename with no behavioral change. The test improvement is well-reasoned: moving the fish lookup to the top avoids running expensive setup before discovering fish is absent, and switching the fake SSH runner from /bin/sh to fish closes the gap between the test harness and real OpenSSH behavior. The one note is that the fix for this regression landed in a prior merged PR, so the failing-test-first commit structure required by the repo policy could not be demonstrated in CI.

No files require special attention — cmux.swift changes are rename-only and the test change is straightforward.

Important Files Changed

Filename	Overview
CLI/cmux.swift	Pure rename of two private helpers and all their call sites (deferredRemoteReconnectLocalCommand → ...Script, sshConnectionTimingLocalCommand → ...Script); no logic changes.
cmuxTests/WorkspaceSSHFishShellTests.swift	Moves fishExecutable discovery to the top of the test (early skip if fish is absent), plumbs CMUX_TEST_LOCAL_SHELL into the fake SSH environment, and changes the Python LocalCommand runner from hardcoded /bin/sh to the fish executable — correctly simulating OpenSSH's login-shell execution of LocalCommand.

Sequence Diagram

sequenceDiagram
    participant CLI as cmux CLI
    participant SR as deferredRemoteReconnectLocalCommandScript
    participant ST as sshConnectionTimingLocalCommandScript
    participant CLS as combinedLocalShellScript
    participant LC as openSSHLocalCommandValue
    participant SSH as OpenSSH
    participant Fish as fish (login shell)
    participant Sh as /bin/sh

    CLI->>SR: build POSIX reconnect script body
    SR-->>CLI: String (POSIX snippet)
    CLI->>ST: build timing script body
    ST-->>CLI: String (POSIX snippet)
    CLI->>CLS: join snippets
    CLS-->>CLI: combined POSIX body
    CLI->>LC: wrap with /bin/sh -c + %% escape
    LC-->>CLI: LocalCommand= value
    CLI->>SSH: pass -o LocalCommand=...
    SSH->>Fish: fish -c "/bin/sh -c '...'"
    Fish->>Sh: exec /bin/sh -c '...'
    Sh-->>Fish: exit
    Fish-->>SSH: exit

Loading

_{Reviews (1): Last reviewed commit: "Make SSH LocalCommand script ownership e..." | Re-trigger Greptile}