Skip to content

fix(browser): a press an SSH host left held across commands is let go of when the host is revoked - #30847

Merged
nwparker merged 2 commits into
mainfrom
nwparker/host-held-input-release-on-revoke
Oct 20, 2026
Merged

nwparker merged 2 commits into
mainfrom
nwparker/host-held-input-release-on-revoke

Conversation

@nwparker

@nwparker nwparker commented Oct 19, 2026 •

Copy link
Copy Markdown
Contributor
Files Added Deleted Net
Test 2 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​225 0 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​225
Prod 8 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​183 $\color{#cf222e}{\Huge{\mathbf{−}}}$​51 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​132

ELI5

An SSH host's orca browser mouse down presses a button on this desktop's page, and the matching mouse up comes later as a separate command. If the host disconnected in between, or the user turned off "Allow this host's orca CLI to control Orca", the page was left with the button stuck down. Now the desktop lets go of it when the host's connection ends.

What Changed

Before: each host command tracked only what it pressed itself, and that tracking ended when the command finished. #30630 lets go of a revoked command's own presses, but nothing covered a press left behind by a command that had already finished. Later moves on that page, including the desktop's own, were sent as drags, because the page still had the button down.

After:

  • One record per page. Every browser input event already passes through one function (trackRemoteCliInput). That function now records, for each page, which host connection last pressed each button, key or touch. A release by anyone clears the record. A press by the desktop or by a different host also clears it, but only after that press is delivered.
  • Release when the host's connection ends. The desktop browser bridge connection's own signal (hostRevoked) is passed down through manager, wiring, executor, browser scope and queue. Unlike the per-command signal, it does not fire when orcad cancels one command or a command times out. When it fires, every press that host still owns is let go of off the page. The release runs in the page's command queue, rechecks ownership at that point, and falls back to a direct release if the queue was torn down.
  • Two smaller corrections.
    • A helper step that arrives on the CDP proxy is attributed to the command whose message it came on, not to whichever command holds the page now.
    • The off-page release reports the buttons still held, so a button someone else holds is not reported as released.

Cancelling one command still lets go of only what that command pressed, as before.

Why

Tying held input to the connection that pressed it, and clearing it when that connection ends, is the usual model for a remote input session.

The first version parked the leftover presses per command and handed them to the host's next command. Reviews found three problems with it:

  • it could end another caller's drag;
  • it ran outside the page's queue;
  • it attributed a re-press to the wrong command.

A single ownership record kept at the one place all input flows through removes all three.

Known limit: real mouse and keyboard input from the user doesn't pass through that function. A user who physically holds the same button when the host disconnects gets a synthetic release off the page. Electron's input events also fire for injected input, so they can't tell the user's own input apart reliably.

Linked Issue

None. Found in a deficiency scan of the remote client surfaces.

Visual Proof

N/A: no UI change; this is input state on a page driven over SSH.

Testing

  • Automated tests added/updated
    • src/main/browser/agent-browser-bridge-host-revocation.test.ts:
      • a host press left between commands is let go of when the host ends (fails on main);
      • it is not let go of again after the host's own mouse up;
      • it survives a later command's cancel (fails on main);
      • it becomes the desktop's, or another host's, after their press;
      • a failed desktop press keeps the host's ownership;
      • the release happens before a desktop press queued after the host ended;
      • the live button mask is kept;
      • the release happens exactly once when the connection ends mid-command.
    • desktop-browser-bridge-manager.test.ts: the host signal survives one command's cancel and fires on close.
    • Ran src/main/browser, src/main/runtime/desktop-browser-bridge and the bridge wiring tests with one worker, the local static-analysis job and smoke:orcad-terminal.
  • Manually tested. A live check needs an SSH host driving a desktop browser; verified only in tests.

Review

  • Codex (gpt-6.1-sol, high):
    • Round 1 findings (stealing another caller's drag, running outside the queue, re-press attribution) were fixed by the redesign.
    • Round 2 findings (a failed desktop press, the chorded-button mask) were fixed.
    • Round 3 asks to also clear ownership on the user's real input. Dismissed as the known limit above: such input can't be told apart from injected input.
  • Opus:
    • Round 1 findings were fixed by the redesign.
    • Round 2 findings (a queued release rejected by session teardown, late proxy-message attribution) were fixed.
    • Round 3 was clean.
  • Design check after round 3: compared with how remote input sessions usually clear what they pressed when the session ends. That matches this design, so the approach did not change.

Agent skill upstream boundary

  • Not applicable.

Notes

  • Mixed versions: desktop-only. No wire change, no new opcode or field.
  • SSH: this is the SSH browser bridge path.
  • Mobile and web: they don't use this path.
  • Windows and Linux: no platform-specific code.

Checklist

  • This PR is small and focused
  • I explained what changed and why
  • N/A for screenshots
  • Self-reviewed for correctness, security, and performance
  • Cross-platform, SSH/remote, and path/shortcut impact considered
  • Static analysis, focused tests and the orcad smoke pass locally

@coderabbitai

coderabbitai Bot commented Oct 19, 2026 •

Copy link
Copy Markdown
Contributor

Warning

Review limit reached

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Next included review available in 1 minute.

Check out review usage here.

View limit details

Limit details: You’ve used all 10 included reviews currently available.

Learn how review limits work.

Review configuration:

⚙️ Run configuration
  • Configuration used: Repository UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 72c5c3d7-4cf5-4124-a87a-8c6f6c4ebdc4
📥 Commits

Reviewing files that changed from the base of the PR and between 575d3b2 and b326cac.

📒 Files selected for processing: 10
  • src/main/browser/agent-browser-bridge-host-revocation.test.ts
  • src/main/browser/agent-browser-bridge-queue.ts
  • src/main/browser/desktop-browser-bridge/desktop-browser-bridge-connection.ts
  • src/main/browser/desktop-browser-bridge/desktop-browser-bridge-executor.ts
  • src/main/browser/desktop-browser-bridge/desktop-browser-bridge-manager.test.ts
  • src/main/browser/desktop-browser-bridge/desktop-browser-bridge-manager.ts
  • src/main/browser/guest-direct-key-input.ts
  • src/main/browser/remote-cli-page-admission.ts
  • src/main/ipc/desktop-browser-bridge-wiring.ts
  • src/main/runtime/desktop-browser-bridge/remote-cli-browser-scope.ts
  • Autofix · 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.

@nwparker
nwparker merged commit d4b65b9 into main Oct 20, 2026
126 of 128 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.

1 participant