Skip to content

fix: finish windows ssh setup after bridge exit - #3719

Merged
Pimpmuckl merged 1 commit into
masterfrom
fix/windows-ssh-cold-start
Sep 12, 2026
Merged

fix: finish windows ssh setup after bridge exit#3719
Pimpmuckl merged 1 commit into
masterfrom
fix/windows-ssh-cold-start

Conversation

@Pimpmuckl

@Pimpmuckl Pimpmuckl commented Sep 7, 2026

Copy link
Copy Markdown
Collaborator

Cold Windows SSH setup starts a persistent server but can wait indefinitely for setup to finish: PowerShell Start-Process -Wait waits for the bridge's descendants too. Wait only for the bridge process and retain its handle so Windows PowerShell 5.1 preserves the exit code.

This is standalone on master. The production change is confined to the Windows streaming command helper used by direct, saved, and API bridge launchers. Unix launch commands, server startup policy, and the remaining Windows feature stack are unchanged.

Validation:

  • Existing native descendant-lifetime regression reproduced the hang on master: the launcher did not exit within ten seconds.
  • Updated launcher and all 79 remote tests passed.
  • Native just check passed: 2,938 Rust tests passed, 4 skipped; maintenance checks and build passed.
  • Local FAST review (two reviewers) and explicit deslop completed with no findings.

refs #3651

@kangal-bot

Copy link
Copy Markdown
Collaborator

@coderabbitai review
@greptileai

@coderabbitai

coderabbitai Bot commented Sep 7, 2026

Copy link
Copy Markdown

Review Change StackReview Change Stack

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Advanced

Run ID: 7bb5faf4-1149-4130-a07f-beaf73cdb7bf

📥 Commits

Reviewing files that changed from the base of the PR and between a5d5f6f and e0ce61c.

📒 Files selected for processing (1)
  • src/remote/attach.rs

Included review availability: Your plan provides up to 10 included reviews per hour; 8 remain after this review.


📝 Walkthrough

Walkthrough

The Windows PowerShell bridge now starts applications with -PassThru, retains the process handle, and calls WaitForExit(). Command expectations use this form. A Windows-only test verifies that the bridge returns exit code 23 while a descendant process remains active.

Suggested reviewers: ogulcancelik

Priority: ⬇️ Low

Merge Risk: ⚪ Minimal · up to e0ce6

The Windows bridge now completes when the launched application exits rather than waiting for its descendants, with regression coverage for the cold-start scenario.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 60.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 5 functions across 1 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly identifies the Windows SSH setup fix caused by the bridge exit behavior.
Description check ✅ Passed The description directly explains the PowerShell process-waiting fix, affected launchers, scope, and validation results.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
  • Fix all pre-merge checks with AI

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@coderabbitai

coderabbitai Bot commented Sep 7, 2026

Copy link
Copy Markdown

@kangal-bot I will review the changes.

✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@greptile-apps

greptile-apps Bot commented Sep 7, 2026

Copy link
Copy Markdown

RetriggerConfidence Score: 5/5

The PR appears safe to merge; no actionable correctness, security, or repository-rule violations remain.

Summary

  • Replaces Start-Process -Wait with an explicit Process.WaitForExit() call.
  • Applies the behavior consistently to direct, saved, and API bridge launchers.
  • Adds a native Windows regression test proving the launcher returns code 23 while a spawned descendant remains alive.
  • Updates command-generation expectations for all affected Windows bridge variants.

Diagram

sequenceDiagram
    participant SSH as SSH setup
    participant PS as Windows PowerShell
    participant Bridge as Herdr bridge
    participant Server as Persistent server descendant

    SSH->>PS: Run encoded streaming command
    PS->>Bridge: Start-Process -PassThru
    Bridge->>Server: Start persistent server
    Bridge-->>PS: Exit with bridge status
    PS->>PS: WaitForExit on bridge handle
    PS-->>SSH: Return bridge exit code
    Note over Server: Continues running independently
Loading

Reviews (5) · Last reviewed commit: "fix: finish windows ssh setup after brid..."

Comment thread src/remote/attach.rs
@Pimpmuckl

Copy link
Copy Markdown
Collaborator Author

@coderabbitai review
@greptileai

@coderabbitai

coderabbitai Bot commented Sep 7, 2026

Copy link
Copy Markdown

@Pimpmuckl I will review the changes.

✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

greptile-apps[bot]
greptile-apps Bot previously approved these changes Sep 7, 2026
@Pimpmuckl

Copy link
Copy Markdown
Collaborator Author

@coderabbitai review
@greptileai

@coderabbitai

coderabbitai Bot commented Sep 7, 2026

Copy link
Copy Markdown

@Pimpmuckl I will review the changes.

✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

Comment thread src/remote/attach.rs Outdated
greptile-apps[bot]
greptile-apps Bot previously approved these changes Sep 7, 2026
@Pimpmuckl
Pimpmuckl force-pushed the issue/3651-windows-live-handoff branch from 9eecb47 to d76fa9f Compare September 7, 2026 18:14
@Pimpmuckl
Pimpmuckl force-pushed the fix/windows-ssh-cold-start branch 2 times, most recently from 0a3b882 to c5c339c Compare September 7, 2026 18:43
@Pimpmuckl
Pimpmuckl force-pushed the issue/3651-windows-live-handoff branch from d76fa9f to a77a207 Compare September 7, 2026 18:43
@Pimpmuckl

Copy link
Copy Markdown
Collaborator Author

@coderabbitai review
@greptileai

@coderabbitai

coderabbitai Bot commented Sep 7, 2026

Copy link
Copy Markdown

@Pimpmuckl I will review the changes.

✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

greptile-apps[bot]
greptile-apps Bot previously approved these changes Sep 7, 2026
@Pimpmuckl
Pimpmuckl force-pushed the fix/windows-ssh-cold-start branch from c5c339c to e0ce61c Compare September 12, 2026 15:17
@Pimpmuckl
Pimpmuckl removed this pull request from stack #3688 September 12, 2026 15:18
@Pimpmuckl
Pimpmuckl changed the base branch from issue/3651-windows-live-handoff to master September 12, 2026 15:19
@Pimpmuckl

Copy link
Copy Markdown
Collaborator Author

@coderabbitai full review
@greptileai

Please review the standalone diff against master at e0ce61c. This replaces the former stacked version with a one-file Windows launcher fix. The native regression reproduced the old hang, all 79 remote tests and native just check passed (2,938 Rust tests), and local FAST plus deslop returned no findings.

@coderabbitai

coderabbitai Bot commented Sep 12, 2026

Copy link
Copy Markdown

@Pimpmuckl I will perform a full review of the standalone diff against e0ce61c9411a06c106a15677501716ed4d3206aa.

✅ Action performed

Full review finished.

@Pimpmuckl
Pimpmuckl merged commit 6cf4bbb into master Sep 12, 2026
7 checks passed
@Pimpmuckl
Pimpmuckl deleted the fix/windows-ssh-cold-start branch September 12, 2026 15:24
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