fix(deploy-agent): show ssh's own error when the target cannot be reached - #241
Merged
Merged
Conversation
…ched The preflight discarded ssh's stderr and then asserted the cause was an unauthorised key. The first run against Saruman got exactly that message, when BatchMode had a more specific one to offer — unknown host key, password-only root, or no route — each of which is fixed differently. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
There was a problem hiding this comment.
🟡 Changes recommended
The new preflight writes SSH stderr to a predictable temp filename (symlink/clobber risk) and prefixes ssh: onto output that already typically includes ssh:, reducing safety and clarity.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
This PR improves scripts/deploy-agent.sh’s SSH preflight by preserving and displaying SSH’s actual stderr (instead of a guessed failure cause), and adds a single-line hint mapping common BatchMode failures to likely fixes.
Changes:
- Capture and print
sshstderr during the “hostname” preflight when the connection fails. - Replace the previous generic “key must already be authorised” message with SSH’s own error plus a brief hint mapping.
File summaries
| File | Description |
|---|---|
| scripts/deploy-agent.sh | Improves preflight failure diagnostics by surfacing SSH stderr and providing actionable hints for common non-interactive SSH failures. |
Review details
- Files reviewed: 1/1 changed files
- Comments generated: 1
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
A $$-named file in a shared temp directory can be pre-created or symlinked by another user; mktemp cannot. And OpenSSH already prefixes its lines, so the script's own "ssh:" doubled it. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What changed
scripts/deploy-agent.sh's preflight printed a guessed cause ("the key must already be authorised") over a discarded stderr. It now prints ssh's own message and a one-line map from the three BatchMode failures — unknown host key, permission denied, timeout — to their fixes.Why
Follow-up to #88 / #240. The first run against
Sarumanfrom the Mac hit exactly this and the message was not enough to act on.Blast radius
secrets/*.sops.yamlOne script, local half only; nothing on any host changes.
Verification
bash -ncleannobody@10.0.99.30: printsssh: nobody@10.0.99.30: Permission denied (publickey,password).followed by the mapped hintmake validate— lint and pins only affect this file; CI runs the full set🤖 Generated with Claude Code