Skip to content

Share SSH monitoring and back off transport failures (OR-558) - #741

Merged
sox8502 merged 23 commits into
mainfrom
daniel/or-558-shared-ssh
Oct 23, 2026
Merged

sox8502 merged 23 commits into
mainfrom
daniel/or-558-shared-ssh

Conversation

@sox8502

@sox8502 sox8502 commented Oct 22, 2026 •

Copy link
Copy Markdown
Contributor

Experiments on the same SSH target share one persistent monitoring connection. Healthy logs/status keep their cadence; failures share durable exponential backoff. Explicit Connect retries immediately without erasing failure history. The remote helper supports Python 3.6+. Per-run supervisors and managed-instance release grace remain intact.

Fixes #713.

Fixes OR-558.

Review fixes cover full-prefix log rewrite detection with bounded caching, connection-scoped legacy guards, password whitespace, harmless SSH config edits, safe persisted-key migration, per-run Slurm exit-file failures, scheduler-only completion, waiting for dead-session teardown before reserving another SSH attempt, and chunking large command inputs through the existing upload path. Identity changes remain pinned and migration preserves retry history. Generated assets are rebuilt from the merged UI.

Validation

  • 6bee71f4 integrates main fe36f742; merge conflicts resolved. Local fmt (including Windows launcher), locked clippy/build/test pass: 1,277 Rust unit tests + 19 integration tests, 3 optional tests ignored. UI build/typecheck and 428 UI tests pass.
  • Meaningful regressions reproduce the addressed failures before their fixes. The teardown regression blocks replacement until cleanup acknowledgement; this is deterministic unit evidence, not a claim that the narrow race was forced through the UI.
  • Recorded focused Linux comparisons reproduce unchanged-suffix rewrites and cross-host legacy blocking on 778114b4; 21906b7d verifies same-size/larger replacement byte equality and cross-host Connect while retaining same-host refusal. Static helper-only 256 MiB catch-up reads exactly 256 MiB; ten unchanged idle polls reread zero bytes. This is not an SSH throughput benchmark.
  • Actual-app SendEnv regression reproduced on ca8fb01e; fixed on 99e8a396 with explicit password reconnect and gap-free completion. Same-data upgrade failure was reproduced, then the same old run recovered on 5cfb903f: 80,164 equal bytes, sequences 1–1484, Done/remote exit zero.
  • Actual-app Slurm baseline: an unreadable exit file blocked its healthy completed neighbor. cde56e50 recovered the neighbor but exposed a second scheduler-only completion bug on the original bad run. Final head fixes that mapping; same-run after-case is pending below.
  • Native Windows spaced-password rejection reproduced on 778114b4; corrected authentication verified on 21906b7d, ca8fb01e and fresh 5cfb903f native processes with server counters and binary provenance.
  • Historical full native Windows CU matrix on 55af17d6: actual Python 3.6.15/3.7.17, 17 runs (13 Done, 4 Cancelled), concurrent real SSH/Slurm, one helper per target, live logs/status, queued/no-log cancellation, outage without authentication, explicit reconnect with byte-equal recovery, dashboard reopen and repeated log rewrites. Revision-specific report.
  • Final-head all-platform CI passes, including Windows builds and older-Python helper coverage.
  • Completed exact-head Greptile 5/5, no findings and zero unresolved review threads.
  • 13b0bdf5 actual-app recovery of the original Slurm job with its exit file still mode 000, using a proven new-head per-run supervisor. Genuine scheduler status is COMPLETED/0:0; local exit-code field remains null.
  • Large-script failure reproduced through actual keyboard-driven SSH and Slurm CLI launches on 13b0bdf5: identical API-saved 819,200-byte harmless script, frame-limit error. The new Rust bounded-frame regression failed before and passes after with >2 MiB byte equality; 14 helper tests pass locally, including multiframe SHA-256 equality and preserved control/status deadlines.
  • 6bee71f4 same-payload large-script SSH and real Slurm completion through keyboard CLI and dashboard. All 14 helper tests pass on actual Python 3.6.15 and 3.7.17, including 2,259,000-byte SHA-256 equality and command-lane deadlines.
  • 6bee71f4 native Windows keyboard Connect/submit/wait/logs all exit zero; same 819,200-byte payload appears exactly once in the real remote script, remote exit is zero, and the 26-byte local/remote marker logs match. Windows report.
  • 13b0bdf5 practical healthy-reset outage: Linux observed 60.955 s healthy, a 5.002061 s retry deadline, and first replacement at 5.096610 s; no earlier SSH/authentication observed. Explicit reconnect recovered the same run with gap-free sequences 1–833 and 42,509 equal log bytes. Native Windows backoff evidence remains separately labelled; this is not deterministic race/MFA proof.
  • Scoped cleanup and restoration verified on Linux and Windows: original project command, private configs/SSH permissions and Slurm retention restored; owned services/fixtures stopped, unrelated services and evidence retained. Idle Linux containers needed forced stop after grace. Linux report and cleanup details.

Limits

Native Windows tests connect to isolated Linux targets inside WSL2 through loopback relays. The reporter’s exact MFA environment, paid-provider E2E, coding-agent authentication and Windows legacy-monitor takeover remain untested. The full 17-run matrix is historical evidence; later focused checks retain their exact revision labels.

Historical Windows footage is playable without annotation overlays. Current Windows recording post-processing failed; the completed Linux MP4 also fails in the delivered web viewer after a normal reattachment attempt. Current acceptance uses preserved raw captures, screenshots and process/server/file evidence; delivered video playback is not claimed. Linux helper provenance was captured after the first SSH completion and before Slurm launch; embedded source/bootstrap matched, but a live helper snapshot before the first completion was not obtained. Windows helper stdin was not independently hashed remotely. Docker was restored through a targeted restart and real-container verification; the initiating hang remains unexplained. Missing-log warnings remain recorded in metadata; the existing UI hides monitoring warnings after terminal status.

Linux reports and recordings · Native Windows evidence

sox8502 commented Oct 22, 2026

Copy link
Copy Markdown
Contributor Author

@greptileai review

Please review the current candidate, 772a115. Cross-platform computer-use testing is in progress; the PR remains draft.

@greptile-apps

greptile-apps Bot commented Oct 22, 2026 •

Copy link
Copy Markdown
Contributor

RetriggerView in GreptileConfidence Score: 5/5

[High impact] The latest changes appear safe to merge, with no new blocking issues found.

Summary

This PR shares SSH monitoring across runs on the same target and saves retry delays across restarts.

  • Experiments on one SSH target share a persistent monitor and retry schedule.
  • Log readers follow file rewrites without losing the new contents.
  • Slurm runs keep reporting status through the shared SSH monitor.
  • The run detail view shows monitoring errors for live runs.
  • SSH services stop cleanly before data directories and development slots move on.

Diagram

%%{init: {'theme': 'neutral'}}%%
flowchart TD
  A[Command input] --> B{Larger than 256 KiB?}
  B -->|No| C[Single exec request]
  B -->|Yes| D[Start upload]
  D --> E[Send bounded chunks in order]
  E --> F[Commit upload and run command]
  E -->|Error| G[Abort upload]
  C --> H[Return command result]
  F --> H
Loading

Reviews (21) · Last reviewed commit: "Stream large SSH command input through b..." · Reviewed by Greptile

Comment thread src/jobs/ssh/service/mod.rs
Comment thread src/commands/supervise.rs Outdated
Comment thread src/jobs/ssh/helper.py Outdated
Comment thread scripts/dev-slot.mjs

sox8502 commented Oct 22, 2026

Copy link
Copy Markdown
Contributor Author

@greptileai review

Addressed all four findings in ee67264:

  • Explicit Connect/attach bypasses the automatic timer while retaining retry history and serialized target attempts.
  • Active logs drain available chunks and replacement publication uses a fixed initial byte boundary, so continued output cannot postpone publication indefinitely.
  • Setup/upload commands retain their prior duration behavior; monitoring deadlines and independent heartbeat remain bounded.
  • Dashboard stop uses optional idle-only service shutdown and preserves active experiment monitoring.

Validation: fmt/clippy/locked build; 1,174 unit + 18 CLI integration tests; 8 helper and 10 dev-slot tests. Added reconnect-under-cooldown, growing replacement, timeout-policy, and busy-service regressions. Actual macOS app replacement logging passed; a real 125-second helper command completed with status responsive at 122 seconds. Exact-revision Linux/Windows computer-use tests are now running; PR remains draft.

sox8502 commented Oct 22, 2026

Copy link
Copy Markdown
Contributor Author

@greptileai review

Please use final current head 97df5cd. The additional dev-slot-only follow-up preserves strict idle shutdown before destructive cleanup or stale-slot reclamation, while ordinary dashboard stop leaves busy monitors alive. The busy-service regression now verifies both behaviors (10 dev-slot tests passed). Actual macOS dashboard stop/restart during an active SSH run passed; its broker stayed alive and logs continued. Rust/product source is unchanged from ee67264.

Comment thread scripts/dev-slot.mjs

sox8502 commented Oct 22, 2026

Copy link
Copy Markdown
Contributor Author

@greptileai review

The latest summary still identifies ee67264 as the reviewed commit. Please review current HEAD 97df5cd. It already fixes the new cleanup finding: cleanupUnlocked awaits stopSshServices(slotPaths, true) before registry/data deletion, stale reservation reclamation does the same before removing directories, and requireIdle sends strict shutdown rather than shutdown_if_idle. The regression verifies ordinary stop preserves busy monitoring while strict cleanup rejects it. All 10 dev-slot tests passed. Please refresh the summary/confidence against this exact commit.

# Conflicts:
#	ui/dist/assets/PdfPreview-BqsUVJSr.js
#	ui/dist/assets/PdfPreview-DhN13peK.js
#	ui/dist/assets/PdfPreview-Dn2XGhUv.js
#	ui/dist/assets/index-D6tC-1j6.js
#	ui/dist/assets/index-DHbi4ZWi.js
#	ui/dist/assets/index-fwfO5DTZ.js
#	ui/dist/index.html

sox8502 commented Oct 22, 2026

Copy link
Copy Markdown
Contributor Author

@greptileai review

Please review current HEAD 39cb2e4. Since the clean 5/5 review on 97df5cd, current main 4cecfe4 was merged and actual Linux/Windows computer-use testing found two hidden diagnostics. The follow-up always prints a server error after terminal output and displays the existing selected-run monitoringError above logs. Focused tests cover prompt-then-error and selected-run/recovery behavior; embedded UI rebuilt. All local Rust shipping gates passed (1183 unit + 18 CLI integration), 8 helper, 10 dev-slot, 396 UI tests collectively, style/typecheck/build. Exact-candidate targeted computer-use rechecks are in progress. Please tie the review summary/confidence to this exact head.

Comment thread src/jobs/ssh/prepared.rs

sox8502 commented Oct 22, 2026

Copy link
Copy Markdown
Contributor Author

@greptileai review

Fixed the valid known-hosts list finding in exact HEAD aca6872. OpenSSH's -G dump loses filename boundaries, so quoting split words cannot safely reconstruct the list. The snapshot now leaves UserKnownHostsFile and GlobalKnownHostsFile to the already-included original config, preserving native parsing and strict checking; resolved values remain in connection identity/drift validation. The native-parser regression fails before and passes after. Actual Linux app reproduction on39cb showed strict host verification failing before a password prompt while native controls accepted the verified fixture key; the same UI flow is being rechecked onaca68723. All required local Rust gates passed:1184 unit+18 CLI integration. Please review this exact head and refresh confidence/findings.

@sox8502
sox8502 marked this pull request as ready for review October 22, 2026 07:55
@sox8502
sox8502 requested a review from myles332 as a code owner October 22, 2026 07:55
@sox8502

sox8502 commented Oct 22, 2026

Copy link
Copy Markdown
Contributor Author

@greptileai review

Comment thread src/commands/supervise.rs Outdated
@sox8502

sox8502 commented Oct 22, 2026

Copy link
Copy Markdown
Contributor Author

@greptileai review

Comment thread src/jobs/ssh/helper.py Outdated
@sox8502

sox8502 commented Oct 22, 2026

Copy link
Copy Markdown
Contributor Author

@greptileai review

@sox8502

sox8502 commented Oct 23, 2026

Copy link
Copy Markdown
Contributor Author

@greptileai review

Comment thread src/jobs/ssh/helper.py Outdated
Comment thread src/jobs/ssh/service/mod.rs Outdated
Comment thread src/commands/compute.rs
@sox8502

sox8502 commented Oct 23, 2026

Copy link
Copy Markdown
Contributor Author

@greptileai review

@sox8502

sox8502 commented Oct 23, 2026

Copy link
Copy Markdown
Contributor Author

@greptileai review

Comment thread src/jobs/ssh/helper.py Outdated
Comment thread src/jobs/ssh/helper.py Outdated
@sox8502

sox8502 commented Oct 23, 2026

Copy link
Copy Markdown
Contributor Author

@greptileai review

@sox8502

sox8502 commented Oct 23, 2026

Copy link
Copy Markdown
Contributor Author

@greptileai review

Comment thread src/jobs/ssh/service/state.rs Outdated
@sox8502

sox8502 commented Oct 23, 2026

Copy link
Copy Markdown
Contributor Author

@greptileai review

Comment thread src/jobs/ssh/service/state.rs Outdated
@sox8502

sox8502 commented Oct 23, 2026

Copy link
Copy Markdown
Contributor Author

@greptileai review

Comment thread src/jobs/ssh/helper.py
@sox8502

sox8502 commented Oct 23, 2026

Copy link
Copy Markdown
Contributor Author

@greptileai review

Comment thread src/jobs/ssh/service/transport.rs
@sox8502

sox8502 commented Oct 23, 2026

Copy link
Copy Markdown
Contributor Author

@greptileai review

Comment thread src/jobs/ssh/service/mod.rs Outdated
@sox8502

sox8502 commented Oct 23, 2026

Copy link
Copy Markdown
Contributor Author

@greptileai review

@sox8502
sox8502 merged commit b97c234 into main Oct 23, 2026
17 checks passed
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.

Slurm run supervisors retry ssh in a tight loop, triggering fresh logins and getting the user's HPC account locked

1 participant