Skip to content

Reuse Windows SSH connections for experiment launch and supervision (OR-543) - #699

Open
devin-ai-integration[bot] wants to merge 7 commits into
mainfrom
devin/1792220930.94284-or543-windows-ssh
Open

devin-ai-integration[bot] wants to merge 7 commits into
mainfrom
devin/1792220930.94284-or543-windows-ssh

Conversation

@devin-ai-integration

@devin-ai-integration devin-ai-integration Bot commented Oct 17, 2026 •

Copy link
Copy Markdown
Contributor

Summary

  • Fix OR-543: reuse one native batch SSH shell per target within each Windows orx process for staging, launch, status and log commands; Unix ControlMaster behavior is unchanged.
  • Serialize binary-safe framed commands/uploads, invalidate dead or reconfigured sessions, drain stderr continuously, and bound configuration probes with cancellation-safe Windows process-tree cleanup using existing Job Object helpers.
  • Retry startup timeouts with 30/60/120-second backoff before command submission, never replay a submitted command. Add ORX_SSH_POLL_INTERVAL_SECS and completion-triggered final log draining.

Test plan

  • Reproduced on unchanged main: manual native SSH succeeded, then public orx exp run … --backend ssh --host or543-rate --no-container failed under the genuine Ubuntu server's 3-new-connections/minute, burst-3 kernel limiter.
  • Final head d64bc705039cfaacdb9c5f53fbc5bd1b328cb838: independent scope Yes, Greptile 5/5 with no findings, all eight CI checks green, including Windows and Linux x86_64/aarch64.
  • Native Windows formatting/clippy, focused framing/lifecycle/configuration-timeout/cancellation/log-drain tests, existing Codex process-tree tests, real-SSH reuse/concurrency test, UI i18n/style/typecheck.
  • Fresh final-head native rebuild, genuine subscription-authenticated Codex/Claude live preflights, restarted isolated backend with inherited credentials, and fresh dashboard Codex reply.
  • Same limiter re-armed: manual SSH and public launch exited 0; detached supervision completed run 8060faac-c8d0-4411-80fe-6c4568a77d07 in 46 seconds. CLI/dashboard preserved final output without a trailing newline. Authenticated SSH count increased by exactly three (manual, launcher, supervisor), with zero drops and no per-poll churn.
  • No stranded SSH/supervisor after completion or backend/listeners after Ctrl+C. The interrupted backend shell returned code 1; this is not a claimed normal backend exit 0.
  • Fresh origin/main fetched and merge-tree check clean; PR remains open for human review, assigned to Daniel based on related SSH sharing/recovery work.

The full local Rust suite has four failures also reproduced on unchanged main (Git identity/remote fixtures and a Git Bash Python shim assertion). Changed-area tests and final-head CI pass.

Bug reproduction on main

Windows Server 2022 / native OpenSSH 9.5p2 / orx 0.2.19 against genuine Ubuntu SSH with kernel rate limiting. Main failed before detached supervision.

Bug reproduction on main

Post-fix functionality

Same controlled Windows environment and limiter, final PR head. Both recordings are animated WebP conversions made with ffmpeg.

Post-fix functionality

Dashboard real Codex reply and completed SSH run

Caveats and remaining limits

  • Background skill-import screening logged Codex (gpt-6.1-sol) request failed: batch file arguments are invalid. The real dashboard chat and SSH flow passed; whether this adjacent failure predates the PR was not investigated.
  • Exact Windows 11/university infrastructure, ProxyJump and live timeout/backoff were not exercised. Framing, cancellation and configuration edge cases have focused test coverage, not recorded end-to-end coverage.
  • Separate orx processes still create separate transports; concurrent launch bursts can exceed server limits.

Automation improvements

  • Add Windows blueprint knowledge for the optional genuine Ubuntu/Docker SSH limiter fixture, re-arm/counter checks without extra SSH traffic, and bounded native ssh -n -T/console paste. This would reduce fixture setup friction; the suggestion could not be saved because this session lacks ManageRepoBlueprints permission.

Link to Devin session: https://app.devin.ai/sessions/314bfb3947db4ee88510e83379b34811
Open in Devin Desktop: https://app.devin.ai/desktop/session/314bfb3947db4ee88510e83379b34811?variant=devin

Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
@devin-ai-integration

Copy link
Copy Markdown
Contributor Author

I'll fix CI failures and address comments from users with write access. I'll skip comments containing "(aside)".

  • Disable automatic comment, CI, and merge conflict monitoring

@greptile-apps

greptile-apps Bot commented Oct 17, 2026 •

Copy link
Copy Markdown
Contributor

RetriggerView in GreptileConfidence Score: 5/5

[High impact] The latest changes appear safe to merge; no blocking issue remains.

Summary

This PR reuses Windows SSH connections for experiment launch and supervision. The latest change makes configuration-probe cleanup safe when a request is cancelled.

  • Windows orx commands share one SSH shell per target.
  • Timed-out SSH connections retry before the remote command starts.
  • SSH supervisors can change how often status and logs are checked.

Diagram

%%{init: {'theme': 'neutral'}}%%
flowchart TD
  A[Start SSH suspended] --> B{Assign kill-on-close job}
  B -->|Failure| C[Return error and drop child]
  B -->|Success| D[Resume SSH]
  D --> E[Await configuration output]
  E --> F[Success, error, timeout, or cancellation]
  F --> G[Drop job and stop remaining descendants]
Loading

Reviews (6) · Last reviewed commit: "Make SSH probe process-tree cleanup canc..." · Reviewed by Greptile

Comment thread src/commands/supervise.rs Outdated
Comment thread src/jobs/ssh/session.rs
Comment thread src/jobs/ssh/session.rs Outdated
Comment thread src/jobs/ssh/session.rs Outdated
Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
@devin-ai-integration

Copy link
Copy Markdown
Contributor Author

@greptileai Please review the latest head f55340f. Follow-up commits address the four previous findings and add native Windows framing/draining, diagnostics, effective-config, dead-child and final-log-wakeup regressions.

Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
@devin-ai-integration

Copy link
Copy Markdown
Contributor Author

@greptileai Please review latest head b60aac5. Only change since the previous review request: the opt-in real-SSH test explicitly disables ControlMaster on every platform instead of being excluded on Unix. This resolves Unix dead-code clippy and tests the Windows transport; production code is unchanged.

Comment thread src/jobs/ssh/session.rs Outdated
Comment thread src/jobs/ssh/session.rs Outdated
Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
@devin-ai-integration

Copy link
Copy Markdown
Contributor Author

@greptileai Final test-only follow-up is a65133c: keep the integration test and reconnect call on all platforms, but assert a different TCP connection only on Windows. This preserves Unix ControlMaster behavior without ineffective duplicate options. Native Windows clippy and the real-SSH test pass. Please review this exact latest head.

Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
@devin-ai-integration

Copy link
Copy Markdown
Contributor Author

@greptileai Please review final head cbbf45c. The remaining configuration-probe deadline finding is fixed with a native-SSH regression; Windows also terminates hung Match exec descendants before dropping SSH to release inherited pipes. All five Windows session unit tests, the opt-in real-SSH transport test, formatting and clippy pass.

Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
@devin-ai-integration

Copy link
Copy Markdown
Contributor Author

@greptileai Please review latest head d64bc70. Follow-up makes configuration-probe cleanup cancellation-safe: reuse the existing Windows kill-on-close Job Object helpers, start SSH suspended, enroll before resume, retain the job through output/deadline. Strict enrollment failure stops the probe. Native-SSH regressions cover both timer expiry and cancellation after Match exec starts; all six session tests, existing Codex wrapper teardown tests and Windows clippy pass.

@sox8502 sox8502 added the awaiting review Devin bugfix PR is open and needs human review before merge label Oct 17, 2026
@sox8502 sox8502 self-assigned this Oct 17, 2026

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

awaiting review Devin bugfix PR is open and needs human review before merge

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant