Skip to content

ops-sync: Add SSH timeout, improve error detection and reporting - #44

Closed
khamer wants to merge 3 commits into
masterfrom
better-sync-errors
Closed

khamer wants to merge 3 commits into
masterfrom
better-sync-errors

Conversation

@khamer

@khamer khamer commented Sep 28, 2026

Copy link
Copy Markdown
Member

Summary

  • Add OPS_SSH_CONNECT_TIMEOUT config (default 10s) to detect hung SSH connections during ops sync
  • Capture SSH stderr to temp log and report connection failures with helpful context (firewall, VPN, bastion host suggestions)
  • Check for empty database dumps before dropping database to avoid data loss on failed syncs
  • Fix shell portability: replace seq with printf for padding, use arithmetic operators for comparisons
  • Remove OPS_TEST_MODE (undocumented, no longer used)
  • Enforce proper exit codes: unknown commands return 1, missing remote host exits early, ops sync outside project exits 1
  • Redirect error messages to stderr throughout

What changed

ops-sync reliability:

  • SSH connection timeout now detected and reported clearly
  • Database dump failures now surface SSH stderr instead of silently failing
  • Empty dumps are rejected before destructive DROP DATABASE
  • Added validation that OPS_PROJECT_REMOTE_HOST is configured

cmd-run error handling:

  • Unknown commands now print error and return exit code 1
  • Preserves exit code from actual command execution

Shell improvements:

  • Portable padding using printf instead of macOS-incompatible seq
  • Arithmetic comparisons use (( )) instead of [[ ]]

🤖 Generated with Claude Code

@khamer
khamer requested a balanced review from Copilot September 28, 2026 21:45
@khamer

khamer commented Sep 28, 2026

Copy link
Copy Markdown
Member Author

Requesting Copilot, and going to run this branch locally for a bit.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🟡 Changes recommended

Database importer failures are currently masked when SSH succeeds.

Review effort: Balanced
Findings: 1 High severity

Open (1)

Comment thread ops.sh
"ops $OPS_PROJECT_REMOTE_DB_TYPE export $OPS_PROJECT_REMOTE_DB_NAME" | \
"ops $OPS_PROJECT_REMOTE_DB_TYPE export $OPS_PROJECT_REMOTE_DB_NAME" 2>"$ssh_log" | \
$OPS_PROJECT_DB_TYPE-import "$OPS_PROJECT_DB_NAME"
ops-sync-check-ssh "${PIPESTATUS[0]}"
@khamer khamer closed this Sep 29, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants