Skip to content

fix: supervise managed experiment rentals remotely - #717

Open
sox8502 wants to merge 9 commits into
mainfrom
daniel/experiment-rental-leases
Open

sox8502 wants to merge 9 commits into
mainfrom
daniel/experiment-rental-leases

Conversation

@sox8502

@sox8502 sox8502 commented Oct 20, 2026 •

Copy link
Copy Markdown
Contributor

Summary

Managed experiments currently rely on a supervisor on the user's machine to delete their compute. Laptop sleep or SSH failure can strand the rental and continue billing.

Run the payload supervisor on the experiment machine, with a scoped billing capability, 30-second heartbeats and five-minute expiry. Final logs and exit status flow directly to the user machine; release follows its acknowledgment or 60 seconds of billed collection. A released rental whose final outcome was never received becomes “Outcome unavailable.” Standalone instances and other backends retain their existing lifecycle.

Requires companion service #657 and its migration before release. Keeps the existing package version; merging does not trigger a release. The remote supervisor must be included in the next client release before using the normal remote installer. Logs left only on a deleted machine remain best effort and can be lost.

Test plan

  • Rust format, clippy, locked build and full test suite
  • UI build, localization/style checks and 364 UI tests
  • Actual in-app synthetic Lost-state check: list, detail, history and lineage
  • Real child success/failure, empty billing payloads, duplicate-launch fencing, activation-time cancellation, private token storage, unknown cancellation outcomes, shell handshake and 60-second no-ACK expiry tests
  • Lost-run agent wakeups and capability redaction in modern/legacy run JSON; synthetic-token check against running API
  • Real RunPod preview flow: simulated laptop sleep (per-run SIGSTOP), broken SSH, remote supervisor loss and connected result collection; all instances deleted
  • Final CI on 1fea532: Ubuntu 24.04 checks, Linux desktop (x86_64/aarch64), Windows build/tests and version sanity
  • Greptile 5/5 on 1fea532 with unchanged version and pinned CI runner; no unresolved review threads

Real-provider verification

The connected control collected both log markers and exit code 0, then released about 6 seconds after its 120-second payload ended. Suspended-client and broken-SSH/failing-payload retests released after the 60-second collection window. Killing the remote supervisor with the client suspended triggered the natural five-minute lease expiry. Direct RunPod inventory confirmed all seven test instances absent; a capacity rejection created no instance and incurred zero charges.

The initial connected control exposed an SSH pipe retention bug in the background shell list; fixed by 81bf49c and covered by a captured-pipe regression and fresh live retests. The laptop itself stayed awake. Unpublished remote binary staged from reviewed CI; public release installation and provider-API outage remain untested live. Temporary test credentials revoked/removed.

CI follow-up pins the general Linux check to Ubuntu 24.04 LTS after ubuntu-latest moved to 26.04 and failed in SQLite/supervisor tests. Test timeouts retain fixture supervisor stderr and metadata for diagnosis; no tests are disabled. The service lease protocol is opt-in, so service-first deployment preserves older client behavior. Older clients do not gain the new cleanup guarantees.

@sox8502

sox8502 commented Oct 20, 2026

Copy link
Copy Markdown
Contributor Author

@greptileai review

@greptile-apps

greptile-apps Bot commented Oct 20, 2026 •

Copy link
Copy Markdown
Contributor

RetriggerView in GreptileConfidence Score: 5/5

[High impact] The updated head appears safe to merge, with no new blocking issues found.

Summary

Managed experiments now supervise payloads and release rentals from the remote machine. The client collects final logs and exit codes, or records “Outcome unavailable.”

  • Managed experiment rentals now release from the experiment machine.
  • Runs with missing final results appear as “Outcome unavailable.”
  • OpenResearch runs no longer send start and finish telemetry events.

Diagram

sequenceDiagram
  participant Client as User machine
  participant Remote as Experiment machine
  participant Service as Billing service
  Client->>Service: Create rental
  Service-->>Client: Scoped rental capability
  Client->>Remote: Stage payload and start supervisor
  Remote->>Service: Activate rental
  loop While payload runs
    Remote->>Service: Heartbeat
  end
  Remote-->>Client: Final logs and exit code
  opt Final data saved
    Client->>Remote: Acknowledge collection
  end
  Remote->>Service: Release after acknowledgment or 60 seconds
  Note over Service: Missing heartbeats expire the rental
Loading

Reviews (9) · Last reviewed commit: "ci: pin Ubuntu LTS and keep the current ..." · Reviewed by Greptile

Comment thread src/jobs/rental.rs Outdated
Comment thread src/jobs/rental.rs Outdated
Comment thread src/commands/supervise.rs Outdated
Comment thread ui/src/api.ts
@sox8502

sox8502 commented Oct 20, 2026

Copy link
Copy Markdown
Contributor Author

@greptileai review

@sox8502

sox8502 commented Oct 20, 2026

Copy link
Copy Markdown
Contributor Author

@greptileai review

Please review current head 7eae840c2bcf55cdcc67a6458313c6a6dd998d54. The four reported findings are addressed: explicit Bash for installation; durable cancellation marker plus waiting for PID/terminal acknowledgment; first observed exit code retained across log draining; Lost excluded from active hover counts. Added real-shell and activation-time cancellation tests, and verified the narrow-panel Lost layout in the app. Full local Rust and UI gates pass.

Comment thread src/local/openresearch.rs
Comment thread src/commands/supervise.rs
@sox8502

sox8502 commented Oct 20, 2026

Copy link
Copy Markdown
Contributor Author

@greptileai review

Please review current head 2b1dda7. Both latest findings are fixed: the SQLite data directory and recovery directory are owner-only, recovery files are created 0600, and LOST no longer follows unacknowledged local cancel intent. Regression tests cover existing-directory permission hardening and acknowledged versus unacknowledged cancellation. Current main is integrated. Local checks: 1,155 Rust unit tests, 364 UI tests, clippy, UI build and a runnable CLI build passed. All four prior launch/cancellation/exit-code/live-count issues remain fixed.

Comment thread src/store.rs
@sox8502

sox8502 commented Oct 20, 2026

Copy link
Copy Markdown
Contributor Author

@greptileai review

The latest finding is fixed: lost runs now enter the ready-wakeup query and send waiting agents an explicit Outcome unavailable notification with partial-local-log guidance. Added store regression plus exact wakeup text coverage; all 20 wakeup tests pass, clippy passes. Earlier capability permissions and cancellation fixes remain. Please review the new current head and report any remaining actionable findings.

@sox8502

sox8502 commented Oct 20, 2026

Copy link
Copy Markdown
Contributor Author

@greptileai review

Additional capability hardening: public run serialization now removes the rental object. This covers ApiRun responses and StoredRun JSON/SSE from the legacy serve daemon; private SQLite/recovery descriptor serialization is unchanged. Regression test verifies modern and legacy JSON cannot contain the fixture token while the private descriptor retains it. API serialization test, clippy and locked build passed. Lost-run wakeup tests from the prior commit also passed. Please review the exact new head for remaining issues.

Comment thread src/store.rs Outdated
@sox8502

sox8502 commented Oct 20, 2026

Copy link
Copy Markdown
Contributor Author

@greptileai review

Corrected the latest finding: public_backend removes rentalLease, the actual serialized field. The fixture now instantiates BackendDescriptor, sets its typed rental_lease, and uses to_json; both modern ApiRun and legacy StoredRun JSON/SSE are checked for absence of the token while the private descriptor retains it. Reproduced the previous leak using a synthetic rentalLease against the running preview. Corrected regression, clippy and locked build pass. Please review the exact new current head for any remaining actionable findings.

@sox8502

sox8502 commented Oct 20, 2026

Copy link
Copy Markdown
Contributor Author

@greptileai review

A real RunPod end-to-end connected control found that the async cd && nohup list retained the SSH output pipes until the remote supervisor exited. That prevented healthy clients from collecting and acknowledging final data. The client launcher now performs cd in the foreground with an error guard and backgrounds only nohup. A captured-pipe shell regression fails on the old command and passes in 0.03 seconds on the fix. Full locked Rust tests, clippy, format and build passed. Real-provider sleep, SSH-failure and remote-supervisor-loss cleanup passed; retests with this fix are running. No service or other-backend implementation changed. Please review this exact new head.

@sox8502

sox8502 commented Oct 20, 2026

Copy link
Copy Markdown
Contributor Author

Real-provider E2E results (preview service)

Tested against service revision 3ef7b14f and client fix 81bf49c using real RunPod CPU instances:

  • Connected 120-second success: full logs plus exit code 0 collected locally; rental released at 126.173 seconds after activation.
  • Simulated laptop sleep: SIGSTOP only the per-run client supervisor, kept stopped through provider deletion; remote supervisor released after the 60-second final-collection window (180.463 seconds after activation).
  • Broken SSH with failing payload: remove authentication and freeze existing SSH workers on only the disposable VM; both independent probe and real client lose SSH. Remote heartbeats/release still work (180.496 seconds after activation).
  • Lost remote supervisor with suspended client: payload remains alive; natural lease worker releases at the exact five-minute heartbeat deadline and deletes provider instance.
  • All seven real provider instances absent from direct provider inventory. One capacity rejection created no instance and incurred zero charges.
  • Natural billing tick at 02:18:32.779Z completed 02:18:33.583Z, after the final release at 02:13:56.681Z. All eight sandbox ledger totals and charged-window endpoints unchanged: zero additional charges (6 cents total preview metering).

The first healthy-client control exposed an SSH launcher pipe-retention bug; fixed in client 81bf49c and retested live. No service source change was needed. Temporary preview token revoked, test SSH registration removed and local credential state restored.

Limits: laptop stayed awake; simulated per-process suspension rather than physical OS sleep. Unpublished remote supervisor staged from reviewed CI artifact; public release installer untested. No live provider-API outage injection. Logs/results remain directly between experiment and user machines. No production deployment or merge performed.

@sox8502
sox8502 requested a review from myles332 as a code owner October 20, 2026 02:42
@sox8502

sox8502 commented Oct 20, 2026

Copy link
Copy Markdown
Contributor Author

@greptileai review

Please review exact updated head 1fea532. Removed the package version bump at user request (no release on merge), pinned the general Linux check to Ubuntu 24.04 LTS after ubuntu-latest migrated to 26.04, and retained supervisor fixture stderr/metadata in timeout diagnostics. No tests disabled or assertions weakened. Full local locked Rust tests, build, clippy and formatting passed, including SSH/Slurm supervisor and logs_cli suites. Rental implementation unchanged from your 5/5 review; real-provider evidence remains on this PR.

This branch has not been deployed

No deployments
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.

1 participant