fix(remote): show SSH authentication output - #3609
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Team Run ID: 📒 Files selected for processing (2)
Included review availability: Your plan provides up to 10 included reviews per hour; 9 remain after this review. 📝 WalkthroughWalkthroughRemote shell helpers now forward child stderr to the local stderr in real time while retaining the bytes in the returned Suggested reviewers: Merge Risk: ⚪ Minimal · up to Remote SSH authentication messages and URLs are now visible while waiting, while later error context remains available. The covered behavior is ready to merge. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
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 review |
|
✅ Action performedReview finished.
|
Greptile SummaryThis PR changes remote SSH setup commands to relay stderr while commands are still running and retain the same bytes for contextual errors.
Confidence Score: 5/5The PR appears safe to merge, with no concrete changed-code defect established. The relay drains SSH stderr concurrently, restores it to the returned process output, and the new integration test covers both immediate authentication notices and subsequent contextual failures.
|
| Filename | Overview |
|---|---|
| src/remote/attach.rs | Introduces concurrent SSH stderr forwarding while preserving captured output for existing parsing and contextual error handling. |
| tests/remote_attach.rs | Adds an end-to-end Unix regression test covering pre-completion authentication output and retained failure context. |
Sequence Diagram
sequenceDiagram
participant H as Herdr
participant S as SSH process
participant T as Local stderr
participant R as Remote host
H->>S: Start setup command
S->>R: Execute probe or setup script
R-->>S: Authentication notice on stderr
S-->>H: Stream stderr bytes
H-->>T: Relay bytes immediately
R-->>S: Command stdout and exit status
S-->>H: Complete command
H->>H: Return Output with captured stderr
Reviews (1): Last reviewed commit: "fix(remote): show SSH authentication out..." | Re-trigger Greptile
Issue
A Tailscale SSH check makes
herdr --remoteappear to hang because the browser authentication URL is hidden until SSH exits.Problem
Remote setup collected SSH error output only after each command finished. Authentication notices written while SSH was waiting were therefore unavailable to the user.
How did we fix it?
Herdr now relays SSH setup error output as it arrives. It also retains that output so existing contextual failures and authentication hints still work. SSH commands, connection reuse, and parsed standard output are unchanged.
Verification
The production-binary regression test timed out waiting for the Tailscale notice before the change. It now sees the notice and URL before simulated approval, then confirms setup continues and later failures retain their context. Remote tests and Windows lint pass. The full suite passed 3,089 of 3,091 tests; the uid-0 permission test and an unrelated live-handoff replacement-server test still fail on this runner.
refs #3606