feat(remote-server): POSIX sh compatibility, armv8l mapping, typed unsupported platform errors#10503
feat(remote-server): POSIX sh compatibility, armv8l mapping, typed unsupported platform errors#10503alokedesai wants to merge 2 commits intomasterfrom
Conversation
…supported platform errors - Make preinstall_check.sh and install_remote_server.sh POSIX sh without bash (Alpine, BusyBox, etc.). - Add RemoteShell enum to ssh.rs with run_ssh_script_with_shell() allowing scripts to be piped via sh -s or bash -s. Update ssh_transport.rs to use RemoteShell::Sh for both preinstall and install scripts. - Add armv8l to install script arch case mapping, aligning with Rust-side parse_uname_output that already handled it. - Add PlatformParseError typed error to setup.rs, replacing generic anyhow errors for parse_uname_output. Distinguishes Malformed, UnsupportedOs, and UnsupportedArch so callers can route unsupported platforms to clean Unsupported fallback instead of generic Failed. - Add UnsupportedArch and UnsupportedOs variants to UnsupportedReason enum for the setup state machine. - Update ssh_transport.rs detect_remote_platform to use DetectPlatformError that preserves typed unsupported errors for downstream routing. - Add comprehensive tests: unsupported arch/OS parsing (armv7l, i686, FreeBSD), PlatformParseError display, UnsupportedReason variants, sh tilde expansion, armv8l mapping guard, and POSIX shebang guard. Co-Authored-By: Oz <oz-agent@warp.dev>
Drop PlatformParseError (redundant with transport::Error), adapt RemoteShell + sh invocations to work with new SshCommandError and transport::Error types, update tests to use transport::Error. Co-Authored-By: Oz <oz-agent@warp.dev>
|
I'm starting a first review of this pull request. You can view the conversation on Warp. I completed the review and no human review was requested for this pull request. Comment Powered by Oz |
![oz-for-oss[bot]](https://avatars.githubusercontent.com/in/3450139?s=60&v=4){kind=small}
There was a problem hiding this comment.
Overview
This PR moves the remote-server install and preinstall scripts toward POSIX sh, adds explicit shell selection for SSH script execution, and extends unsupported platform handling for armv8l and unsupported OS/arch cases.
Concerns
- Typed
UnsupportedOs/UnsupportedArcherrors are documented as routeable, but the setup flow still treatsdetect_platform()errors as inconclusive and continues toward binary checks/installation. Unsupported platforms can still surface as install failures instead of taking the clean legacy SSH fallback.
Verdict
Found: 0 critical, 1 important, 0 suggestions
Request changes
Comment /oz-review on this pull request to retrigger a review (up to 3 times on the same pull request).
Powered by Oz
| /// The expected format is `<os> <arch>`, e.g. `Linux x86_64` or `Darwin arm64`. | ||
| /// Takes the last line to skip any shell initialization output. | ||
| /// | ||
| /// Returns a typed [`crate::transport::Error`] so the caller can |
There was a problem hiding this comment.
Error::UnsupportedOs / UnsupportedArch before continuing; otherwise unsupported hosts will keep flowing into binary checks/install instead of the legacy SSH fallback.
1 participant
Problem
Remote-server setup assumed Bash and a narrow set of architecture strings. That meant hosts with only POSIX
shcould fail before the installer ran,armv8lhosts were reported as unsupported even though they should use the aarch64 artifact, and unsupported OS/arch detection could surface as a noisy install failure instead of a clean fallback path.Solution
sh -swhere the scripts do not need Bash.RemoteShellsupport so future callers can explicitly chooseshorbash.armv8lto the aarch64 remote-server artifact consistently in the shell script and Rust platform parser.Proof / validation
No screenshots are applicable because this is remote install plumbing, not a visual UI change. Proof is command and constructed-host output:
cargo test -p remote_serverpassed in the integrated validation run: 136 tests passed.cargo check -p warppassed after the SSH transport changes were integrated.18_missing_bash:RESULT: EXPECTED_EXIT (exit 0)— installer ran throughshwithout Bash in PATH.21_armv8l_supported: beforeorigin/masterexited2withunsupported arch: armv8l; after this change exited0.Co-Authored-By: Oz oz-agent@warp.dev