fix(cli): retry transient sandbox sync disconnects - #2258
Conversation
|
Auto-sync is disabled for draft pull requests in this repository. Workflows must be run manually. Contributors can view more details about this message here. |
Signed-off-by: Evan Lezar <elezar@nvidia.com>
64fbab8 to
15fdeab
Compare
elezar
left a comment
elezar
left a comment
There was a problem hiding this comment.
gator-agent
PR Review Status
Validation: This is a focused maintainer-authored CLI reliability fix extracted from #1868, scoped to sandbox sync retry behavior.
Head SHA: 15fdeab3ca3969d2152b5c9bb146066a76b1f0d7
Review findings:
- Two warning-level correctness findings remain. Both are about preserving the remote SSH exit status when local tar streaming fails early, so permanent remote failures are not retried as transient local pipe/EOF errors.
Docs: Not needed; this changes sync resilience and failure classification, not documented CLI syntax or workflow semantics.
Checks: Required status contexts are currently green; E2E is not applied.
Next state: gator:in-review
| return Err(miette::miette!( | ||
| "ssh tar extract exited with status {status}" | ||
| )); | ||
| return Err(Report::new(SshCommandExitError::from_status( |
There was a problem hiding this comment.
gator-agent
Warning: If the remote command fails permanently before consuming stdin, for example a mkdir -p permission failure, the local tar writer can see BrokenPipe before this status path is reached, and the new retry policy treats that as transient. Please capture the archive-write result, always wait for the SSH child, and prefer a non-255 SSH exit status over the retryable pipe error so permanent remote failures are reported once instead of retried four times.
| .unpack(&extract_into) | ||
| .into_diagnostic() | ||
| .wrap_err("failed to extract tar archive from sandbox")?; | ||
| .with_sync_context("failed to extract tar archive from sandbox")?; |
There was a problem hiding this comment.
gator-agent
Warning: The download path has the same failure-ordering issue: archive.unpack() can hit UnexpectedEof before the child status is collected, so a permanent remote tar failure can be retried as a transient stream EOF. Please wait for the SSH child after extraction fails and classify using the SSH status when it is available.
elezar
added
the
gator:in-review
1 participant
Summary
Retry sandbox upload and download operations when a gateway handoff interrupts the underlying transport. Retries rebuild the SSH session for each attempt and remain limited to typed transient failures; validation, local permission, remote command, and other permanent failures still return immediately.
Related Issue
Extracted from #1868 so the generally applicable CLI behavior can be reviewed independently.
Changes
Unavailable, connection transport errors,BrokenPipe,ConnectionReset,UnexpectedEof, and SSH exit 255.Testing
mise run pre-commitpassesgrpc_toolswas unavailable while the new venv was still being populated, and workspace Clippy could not findz3.houtside the Nix environment. Format, license, Helm, and Markdown checks passed.nix develop -c cargo test -p openshell-cli --lib(195 tests)nix develop -c cargo clippy -p openshell-cli --all-targets -- -D warningsChecklist