Skip to content

Keep new browser tabs on SSH routing after managed conversion - #26570

Merged
OrcaWin merged 2 commits into
mainfrom
nwparker/ssh-browser-route-after-conversion
Oct 9, 2026
Merged

OrcaWin merged 2 commits into
mainfrom
nwparker/ssh-browser-route-after-conversion

Conversation

@nwparker

@nwparker nwparker commented Oct 8, 2026 •

Copy link
Copy Markdown
Contributor
Files Added Deleted Net
Test 7 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​349 $\color{#cf222e}{\Huge{\mathbf{−}}}$​2 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​347
Prod 10 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​108 $\color{#cf222e}{\Huge{\mathbf{−}}}$​32 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​76

ELI5

After converting an SSH workspace to a managed Remote host, new browser tabs tried to reach private websites from the desktop. They now keep using the original SSH connection and browser login.

What Changed

The existing browser route resolver recognizes a managed host’s registered SSH deployment. It keeps the same target/profile browser partition and reads the desktop’s connection status for that target. Preparing a route also checks the deployment registration generation, so a stale registration cannot silently route through a replaced target. Global and per-target browser routing settings still apply.

The real conversion test opens a new tab through the normal workspace action, reaches a remote-only website with its existing login cookie, reconnects SSH, and reaches it again. Its Docker test is assigned to the managed-server template CI lane.

Why

Reusing the existing SSH route and browser partition preserves the verified connection and cookie jar. A new tunnel or copied cookies would add another lifecycle to maintain. Only an explicit managed deployment establishes this association; a generic SSH gateway does not prove which host executes the workspace.

Linked Issue

Follow-up to the SSH consolidation integration in #26483; independently based on OrcaWin/phase35.

Visual Proof

Before: the new browser cannot reach the remote-only origin after conversion.

Before

After: the new browser reaches the same origin with its original login.

After

Baseline failure and origin assertion · Passing conversion, cookie, reconnect, and stale-registration evidence

Testing

  • I manually tested these changes locally
  • Automated tests added/updated, or explained why not below

126 affected tests pass, covering route eligibility, connection phases, browser mount gates, registration validation, SOCKS routing, IPC, and CI selection. The CI-discovered deferred-browser fixture omission was corrected; all 23 lifecycle/route/gate tests and web typecheck pass. Full pnpm tc, affected oxlint, changed-code quality, and a fresh Electron E2E build pass.

Real hidden Electron checks on macOS with an owned Linux SSH host reproduce the baseline failure and pass with the fix. Host request records verify the original cookie and exact browser partition, including after reconnect. The host uses the unchanged installed managed-server template. Windows validation is left to CI.

Review

Self-reviewed target provenance, stale-registration handling, reconnect behavior, settings opt-outs, and compatibility of the optional local IPC field.

Agent skill upstream boundary

  • Not applicable, or this change copies or mechanically translates no upstream skill-installer source, tests, fixtures, registry entries, path tables, comments, or documentation.

Notes

An older retained browser tab with unspecified placement can still be reassigned to an unsupported server browser during conversion. This PR fixes new and explicitly local browser pages; #26572 is the tested follow-up for retained pages and is stacked on this PR. No remote wire format changes are required.

Checklist

  • This PR is small and focused
  • I explained what changed and why (ELI5, the user-facing before/after, the mechanism, and why over the alternatives)
  • Before/after screenshots or videos attached for UI changes, or N/A with reason
  • Self-reviewed for correctness, security, and performance
  • Cross-platform, SSH/remote, and path/shortcut impact considered (or N/A)
  • pnpm lint, pnpm typecheck, pnpm test, and pnpm build pass (or CI will cover; local preferred)

@OrcaWin
OrcaWin force-pushed the nwparker/ssh-browser-route-after-conversion branch from 26bcbff to 890075a Compare October 9, 2026 02:12
@OrcaWin
OrcaWin changed the base branch from OrcaWin/phase35 to main October 9, 2026 02:12
@coderabbitai

coderabbitai Bot commented Oct 9, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

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: 758ba67d-12d1-479b-bedb-a33882606fe1
📥 Commits

Reviewing files that changed from the base of the PR and between d1586c1 and ae8c3b5.

📒 Files selected for processing (4)
  • .github/workflows/e2e.yml
  • config/scripts/ci-e2e-job-selection.mjs
  • config/scripts/ci-e2e-job-selection.test.mjs
  • config/scripts/pr-e2e-source-routing.mjs

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 5 remain after this review.


📝 Walkthrough

Walkthrough

The renderer can resolve SSH browser routes from managed Orcad runtime deployments and track the associated SSH target generation. Partition preparation passes that generation to the main process, which validates the target and generation. A new end-to-end test covers routing and cookie continuity across conversion, stale-generation rejection, and reconnect behavior. CI source routing and the Docker conversion job now include the test.

Priority: ➖ Normal

Merge Risk: ⚪ Minimal · up to ae8c3

The PR aims to preserve SSH-routed browser sessions through managed conversion. The supplied context reports successful conversion testing, and CI routes resolver changes to the dedicated Docker test; no actionable merge-blocking risk is established.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage Warning Docstring coverage is 28.57% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 14 functions across 17 files. (1 skipped:… 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 describes the primary user-facing change: preserving SSH routing for new browser tabs after managed conversion.
Description check Passed The description follows the repository template and explains the user impact, implementation, rationale, issue reference, visual proof, testing, limitations, and checklist status.
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.
Full details: Docstring Coverage

Explanation

Docstring coverage is 28.57% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 14 functions across 17 files. (1 skipped: 1 unsupported.)

  • Fix all pre-merge checks with AI
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Commit to this branch
  • Create a new PR
  • Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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.

@OrcaWin
OrcaWin force-pushed the nwparker/ssh-browser-route-after-conversion branch from 890075a to d1586c1 Compare October 9, 2026 03:51
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