Skip to content

Report browser placement support from the installed provider - #26559

Merged
OrcaWin merged 2 commits into
mainfrom
nwparker/orcad-browser-capability-truth
Oct 9, 2026
Merged

OrcaWin merged 2 commits into
mainfrom
nwparker/orcad-browser-capability-truth

Conversation

@nwparker

@nwparker nwparker commented Oct 8, 2026 •

Copy link
Copy Markdown
Contributor
Files Added Deleted Net
Test 6 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​184 $\color{#cf222e}{\Huge{\mathbf{−}}}$​64 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​120
Prod 7 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​42 $\color{#cf222e}{\Huge{\mathbf{−}}}$​11 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​31

ELI5

A managed SSH host says it can put a browser page on your desktop, then refuses the request. It now advertises that feature only when its browser provider implements it.

What Changed

Browser factories explicitly opt into client page creation. The desktop provider opts in; managed external Chromium and Electron-sidecar providers do not. Runtime status uses that declaration and its live health check instead of publishing client hosting unconditionally. Hosts without any page-creation backend also stop advertising support for caller-supplied page IDs. Network tunneling and the existing refusal responses remain available.

Why

Clients use these capabilities together to choose and stage a client-hosted page. A frontend-only change reproduced browser_unavailable on the real host, so changing tab selection would still fail. Reporting the installed provider’s abilities fixes the false promise without introducing another browser lifecycle. This does not restore a relay browser tab or its login across conversion; that failure remains under investigation.

Linked Issue

Found while validating the SSH consolidation in #26483. Independent fix against OrcaWin/phase35.

Visual Proof

N/A — this changes the runtime capability response, with no layout or styling change. Real before/after SSH responses and test logs:

browser-capability-evidence.log
browser-capability-real-red.log
browser-capability-real-green.log

Testing

The real SSH capability test is registered in the managed-server CI lane that builds its template, excluded from general sharding, and selected by browser-provider/status source changes. 53 job-selection and lane-contract checks pass.

  • Manually exercised a real Linux Docker SSH host with a hidden macOS Electron client. The installed exact adhoc server fails the new capability check; the corrected server passes. Browser requests still return the explicit unavailable error, repository requests work before/after reconnect, and the desktop continues advertising client hosting.
  • Updated automated coverage for an absent provider, server-only provider, supported client provider, and failed health probe. 1,357 runtime/provider tests pass with one existing skip; three real-release browser-placement schema compatibility tests pass.
  • Full pnpm tc, affected-file oxlint, changed-code quality, Node server build and fresh Electron E2E build pass. The fixture uses rebuilt common artifacts with unchanged installed native binaries; all nine template targets verify. Two corrected real SSH runs pass, including the added desktop assertion. Linux/macOS exercised; Windows covered by CI.

Review

Capability strings, RPC methods, and message shapes are unchanged. Older clients already require the complete existing capability set before selecting client hosting. No execution-host, workspace ownership, or terminal lifecycle changes.

Agent skill upstream boundary

  • Not applicable; no upstream skill source copied.

Notes

Browser restoration/login across relay conversion remains unresolved. This PR corrects feature discovery and preserves explicit backend-unavailable responses.

Checklist

  • This PR is small and focused
  • Explained behavior, mechanism, and approach
  • Before/after runtime evidence attached; no visual change
  • Self-reviewed for correctness, security, and performance
  • Cross-platform, SSH/remote, and compatibility considered
  • Local checks pass; CI covers platform builds

@OrcaWin
OrcaWin force-pushed the nwparker/orcad-browser-capability-truth branch from 3ee02fb to 83c1334 Compare October 9, 2026 02:08
@OrcaWin
OrcaWin changed the base branch from OrcaWin/phase35 to main October 9, 2026 02:08
@coderabbitai

coderabbitai Bot commented Oct 9, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

📝 Walkthrough

Walkthrough

The runtime now reports browser client hosting when its browser-command factory supports it. The Electron startup path registers a factory with client hosting enabled. Runtime tests cover capability reporting for supported, server-only, unhealthy, and unavailable providers. A gated SSH/Orcad E2E test checks browser capability and command responses, including after reconnect. CI routes relevant source changes to the dedicated test and runs it in the Docker Orcad lane.

Priority: ➖ Normal

Merge Risk: 🔵 Low · up to 83c13

The change is mergeable with a bounded test-routing gap: future preflight-only changes will not automatically run the dedicated browser-capabilities test. Add preflight to that route.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage Warning Docstring coverage is 16.67% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 6 functions across 11 files. (1 skipped: … Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
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.
Title check Passed The title clearly summarizes the primary change: capability reporting now reflects support from the installed browser provider.
Description check Passed The description is complete and directly aligned with the changes. It explains the user impact, implementation, rationale, issue context, lack of visual changes, testing evidence, compatibility consid…
Full details: Docstring Coverage

Explanation

Docstring coverage is 16.67% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 6 functions across 11 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.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (1)
config/scripts/pr-e2e-source-routing.mjs (1)

49-57: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Route preflight registration changes to the browser-capabilities E2E spec.

When a PR changes only src/main/startup/main-process-preflight.ts, the current routing selects the Windows AppData spec but not ssh-orcad-browser-capabilities.spec.ts. The Windows spec checks startup paths and CLI readiness. It does not assert browser capabilities. Add the preflight file to the browser-capabilities route.

Suggested fix
-      /^src\/main\/(?:runtime\/(?:runtime-browser-commands-factory|orca-runtime-get-status)|host\/electron-browser-commands|orcad\/orcad-browser-)/.test(
+      /^src\/main\/(?:startup\/main-process-preflight|runtime\/(?:runtime-browser-commands-factory|orca-runtime-get-status)|host\/electron-browser-commands|orcad\/orcad-browser-)/.test(

ℹ️ Review info
⚙️ Run configuration
  • Configuration used: Repository UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 002bd012-e5c7-4999-b472-1a86448f219b
📥 Commits

Reviewing files that changed from the base of the PR and between 37833ce and 83c1334.

📒 Files selected for processing (13)
  • .github/workflows/e2e.yml
  • config/scripts/ci-e2e-job-selection.mjs
  • config/scripts/ci-e2e-job-selection.test.mjs
  • config/scripts/pr-e2e-docker-lane-contract.test.mjs
  • config/scripts/pr-e2e-gate-contract.test.mjs
  • config/scripts/pr-e2e-source-routing.mjs
  • src/main/host/electron-browser-commands.ts
  • src/main/runtime/orca-runtime-get-status.ts
  • src/main/runtime/orca-runtime-test-lifecycle.spec.ts
  • src/main/runtime/orca-runtime-tests/browser-capabilities.spec.ts
  • src/main/runtime/runtime-browser-commands-factory.ts
  • src/main/startup/main-process-preflight.ts
  • tests/e2e/ssh-orcad-browser-capabilities.spec.ts
💤 Files with no reviewable changes (1)
  • config/scripts/pr-e2e-gate-contract.test.mjs

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

@OrcaWin
OrcaWin merged commit 1cda323 into main Oct 9, 2026
50 of 70 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.

2 participants