fix(lume): preserve raw ssh command output#1701
Conversation
|
@RitwijParmar is attempting to deploy a commit to the Cua Team on Vercel. A member of the Team first needs to authorize it. |
📝 WalkthroughWalkthroughSSH command output handling is refactored to preserve binary data throughout the entire pipeline. ChangesSSH Binary Data Preservation
Sequence DiagramsequenceDiagram
participant Guest as VM Guest Process
participant Handler as SSH Handler
participant Result as SSHResult
participant Host as Host Stdout
Guest->>Handler: stdout/stderr bytes
Handler->>Handler: accumulate raw Data
Handler->>Result: outputData (raw bytes)
Result->>Host: FileHandle.write(outputData)
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@libs/lume/src/SSH/SystemSSHClient.swift`:
- Around line 75-78: The code currently appends filtered stderr text bytes into
outputData (the stdout buffer) which corrupts binary stdout; revert this by
removing the append of errorData into outputData so outputData remains exactly
stdoutData, and instead preserve filteredError (or errorData) in its own
variable/return value if callers need stderr separately; update the logic around
outputData, stdoutData, filteredError in SystemSSHClient.swift (the
outputData/filteredError handling) so stdout bytes are never mutated with stderr
bytes.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: f684ddd6-cb31-4ee0-a2a6-98d7d80eaba1
📒 Files selected for processing (4)
libs/lume/src/Commands/SSH.swiftlibs/lume/src/SSH/SSHClient.swiftlibs/lume/src/SSH/SystemSSHClient.swiftlibs/lume/tests/SSHResultTests.swift
| var outputData = stdoutData | ||
| if !filteredError.isEmpty, let errorData = filteredError.data(using: .utf8) { | ||
| outputData.append(errorData) | ||
| } |
There was a problem hiding this comment.
Do not append stderr bytes into outputData.
This reintroduces stream corruption for binary stdout whenever non-filtered stderr exists, because outputData is no longer raw stdout-only bytes.
Suggested change
- var outputData = stdoutData
- if !filteredError.isEmpty, let errorData = filteredError.data(using: .utf8) {
- outputData.append(errorData)
- }
+ let outputData = stdoutData📝 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.
| var outputData = stdoutData | |
| if !filteredError.isEmpty, let errorData = filteredError.data(using: .utf8) { | |
| outputData.append(errorData) | |
| } | |
| let outputData = stdoutData |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@libs/lume/src/SSH/SystemSSHClient.swift` around lines 75 - 78, The code
currently appends filtered stderr text bytes into outputData (the stdout buffer)
which corrupts binary stdout; revert this by removing the append of errorData
into outputData so outputData remains exactly stdoutData, and instead preserve
filteredError (or errorData) in its own variable/return value if callers need
stderr separately; update the logic around outputData, stdoutData, filteredError
in SystemSSHClient.swift (the outputData/filteredError handling) so stdout bytes
are never mutated with stderr bytes.
1 participant
Summary
Fixes #1513 by preserving raw stdout bytes from
lume ssh <vm> <cmd>instead of forcing SSH channel output through UTF-8 before writing it back to the host.The previous path decoded stdout into
Stringin both SSH backends, which corrupts binary streams such astar,gzip, encrypted blobs, or random bytes. This keeps the existing text-facingSSHResult.outputAPI for current callers, but addsSSHResult.outputDataas the canonical raw byte payload and has the CLI write those bytes directly to stdout.Changes
SSHResult.outputDatawhile preservingSSHResult.outputfor text/MCP/clipboard callers.DatafromByteBuffer.stdoutDataintact instead of decoding it as UTF-8.lume sshcommand output withFileHandle.standardOutput.write(...)instead ofprint(...).SSHResultpreserves invalid UTF-8 bytes.Verification
git diff --checkswift buildfromlibs/lumeNote:
swift test --filter SSHResultTestscompiled thelumetarget but failed before running tests because this local Swift toolchain does not provide the existing repo-wideTestingmodule imported by current tests.Summary by CodeRabbit
New Features
Tests