Skip to content

fix(remote): keep editor focus after unfocused HTML side preview (STA-5001) - #15716

Merged
AmethystLiang merged 1 commit into
mainfrom
brennanb2025/sta-5001-remote-preview-tab-type
Aug 23, 2026
Merged

AmethystLiang merged 1 commit into
mainfrom
brennanb2025/sta-5001-remote-preview-tab-type

Conversation

@brennanb2025

@brennanb2025 brennanb2025 commented Aug 21, 2026 •

Copy link
Copy Markdown
Contributor
Files Added Deleted Net
Test 4 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​370 $\color{#cf222e}{\Huge{\mathbf{−}}}$​1 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​369
Prod 4 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​56 $\color{#cf222e}{\Huge{\mathbf{−}}}$​5 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​51

ELI5

Opening an HTML preview beside the editor on a paired remote client left the client thinking a terminal was focused. The preview split was created empty and marked active, then a host snapshot still saying "terminal is active" stole focus. The preview is supposed to sit to the side and only take focus after you click it.

What Changed

  • Remote "Open Preview to the Side" no longer activates the empty right-hand split. Local previews still activate so the preview takes focus once the tab exists.
  • Session-tab snapshot apply: if the active group is an empty reserved preview split, keep the sibling editor as the visible tab instead of following the host's still-active terminal.
  • Empty terminal splits are unchanged: they still activate and still follow a host terminal snapshot when they are not a reserved preview group.

Why

This is a real bug, not a flake.

tests/e2e/paired-remote-html-browser-focus.spec.ts failed on main at 4e058d4a52 (e2e 4-of-10) with activeTabType 'terminal' instead of 'editor'. The spec was introduced in #13876 (2026-08-12). A full 40-run log sweep of every e2e 4-of-10 job timed out, so I did not get a complete main-branch fail rate. Code inspection plus a deterministic unit repro is enough: the preview tab is created on the host with activate: false, and the paired client must keep the source editor as the focused surface until a click.

Root cause (file:line):

  1. openFilePreviewToSide called createEmptySplitGroup, which always set activeGroupIdByWorktree to the new empty group (src/renderer/src/store/slices/tabs.ts).
  2. createWebRuntimeSessionBrowserTab({ focusOnCreate: false }) then asked the host to create the browser without activating it, so the host snapshot still had activeTabType: 'terminal'.
  3. applyWebSessionTabsSnapshot saw the empty active group (resolveWebSessionVisibleTabId returns null) and fell through to the host terminal (src/renderer/src/runtime/web-session-tabs-sync.ts).

That is an ordering/synchronization bug on a paired remote surface. Snapshot timing makes the e2e look flaky, but the settled state after an empty-group snapshot is wrong.

Remote wire compatibility

No wire change.

  • Rule 1: no new JSON field.
  • Rule 2: no new stream opcode; nothing to capability-negotiate.
  • Rule 3: the host still publishes the same snapshot (terminal remains active after browser.tabCreate with activate: false). This PR only changes how a new client interprets that existing snapshot when it has a reserved empty preview split. Old clients keep the previous behavior. Old hosts against new clients are fine because the host content is unchanged.

Linked Issue

STA-5001. No GitHub issue was found for this e2e failure.

https://linear.app/stably/issue/STA-5001/bug-active-tab-type-remains-terminal-after-opening-remote-html-preview

Visual Proof

N/A. This is paired session-tab focus sync, not a chrome/layout change. The proof is the unit repro of the e2e assertion (group + active tab + activeTabType).

Testing

Acceptance test (must be able to fail):
src/renderer/src/runtime/web-session-tabs-sync-html-preview-focus.test.ts
"keeps the source editor focused when the empty preview split is still the active group"

  • Pre-fix / neutered fix FAILS:
    AssertionError: expected { activeGroupId: 'host-group-1', … } to deeply equal { … }
    -   "activeGroupId": "client-editor-group",
    -   "activeTabId": "local-html-editor",
    +   "activeGroupId": "host-group-1",
    +   "activeTabId": "web-terminal-host-terminal",
        "activeTabType": "editor",
    
  • Post-fix PASSES (3 tests in that file).
  • Neutered by removing reservedEmptyPreviewFallbackTabId from nextActiveUnifiedTabId; the same test went red again, then the fix was restored.

Also ran:

  • pnpm typecheck
  • node config/scripts/check-changed-code-quality.mjs
  • node config/scripts/check-react-doctor-changed.mjs
  • pnpm vitest run --config config/vitest.config.ts on the affected suites (file-preview, web-session-tabs-sync-*, web-runtime-session-browser-create-*, tabs-empty-split-activation, tabs-pane-layout-operations, tabs-unread-and-focus, editor-tab-placement, web-session-focus-intent-visible-tab)

Not run: the full Playwright spec tests/e2e/paired-remote-html-browser-focus.spec.ts (paired Electron, 240s). I did not have a CI-like paired-host environment here. Say so rather than claim that spec is green.

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

Review

Agent skill upstream boundary

  • Not applicable, or this change follows docs/reference/agent-skill-sharing-upstream-boundary.md and copies or mechanically translates no upstream skill-installer source, tests, fixtures, registry entries, path tables, comments, or documentation.

Checklist

  • This PR is small and focused
  • I explained what changed and why (including ELI5)
  • 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)

…-5001)

Opening a remote HTML preview to the side created an empty split and made it
the active group. The next host session-tab snapshot still had the terminal
active, so the client treated that empty group as a terminal focus change.

Do not activate the empty split for unfocused remote previews, and when a
reserved preview group is still empty, keep the sibling editor as the
visible tab instead of following the host terminal.
@coderabbitai

coderabbitai Bot commented Aug 21, 2026

Copy link
Copy Markdown
Contributor

Warning

Review limit reached

@brennanb2025, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 3 minutes

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

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

How can I continue?

Wait for the limit to reset, then comment @coderabbitai review or push new commits to the PR.

An organization admin can change what happens after included review limits in Billing.

How do review limits work?

CodeRabbit enforces per-developer PR review limits within each organization.

For paid Pro and Pro+ reviews, CodeRabbit uses a developer's included PR review attempts over the past 7 days to set the current hourly allowance. At typical activity levels, the full plan allowance applies. Higher sustained activity can lower the allowance until earlier attempts leave the 7-day window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro Plus

Run ID: ae1efcf0-5a73-4db2-9265-bd817172ce6a

📥 Commits

Reviewing files that changed from the base of the PR and between df7460a and 912f115.

📒 Files selected for processing (8)
  • src/renderer/src/lib/file-preview.test.ts
  • src/renderer/src/lib/file-preview.ts
  • src/renderer/src/runtime/web-session-focus-intent-visible-tab.test.ts
  • src/renderer/src/runtime/web-session-focus-intent.ts
  • src/renderer/src/runtime/web-session-tabs-sync-html-preview-focus.test.ts
  • src/renderer/src/runtime/web-session-tabs-sync.ts
  • src/renderer/src/store/slices/tabs-empty-split-activation.test.ts
  • src/renderer/src/store/slices/tabs.ts

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.

@brennanb2025

Copy link
Copy Markdown
Contributor Author

Coordinator verification

The e2e workflow is skipping on this PR, so the spec that produced STA-5001 never ran in CI. I ran it locally on both sides:

tree tests/e2e/paired-remote-html-browser-focus.spec.ts:14
plain origin/main 1 failed — reproduces STA-5001
this branch 1 passed (28.8s)

So the acceptance test genuinely fails before the fix and passes after.

Also verified:

  • pnpm typecheck clean.
  • 180 tests pass across web-session-tabs-sync, web-session-focus-intent, tabs-empty-split-activation, file-preview.
  • Changed-code quality, type-aware, and React Doctor gates: 0 new findings.
  • Non-vacuous on both halves. Reverting the { activate: false } guard in file-preview.ts fails creates paired-runtime side previews in the requested split; forcing reservedEmptyPreviewFallbackTabId to null fails keeps the source editor focused when the empty preview split is still the active group.

Note for reviewers: this changes what a paired host publishes as the active tab, so docs/reference/remote-wire-compatibility.md applies — the snapshot-side change reaches older clients with no wire change. Worth confirming an old client cannot read the reserved-empty group as a terminal focus change.

@AmethystLiang
AmethystLiang merged commit 9ea2636 into main Aug 23, 2026
48 checks passed
paidaxingyo666 pushed a commit to paidaxingyo666/Manta that referenced this pull request Aug 23, 2026
…-5001) (stablyai#15716)

Opening a remote HTML preview to the side created an empty split and made it
the active group. The next host session-tab snapshot still had the terminal
active, so the client treated that empty group as a terminal focus change.

Do not activate the empty split for unfocused remote previews, and when a
reserved preview group is still empty, keep the sibling editor as the
visible tab instead of following the host terminal.

(cherry picked from commit 9ea2636)
CodeHourra pushed a commit to CodeHourra/orca that referenced this pull request Aug 23, 2026
…-5001) (stablyai#15716)

Opening a remote HTML preview to the side created an empty split and made it
the active group. The next host session-tab snapshot still had the terminal
active, so the client treated that empty group as a terminal focus change.

Do not activate the empty split for unfocused remote previews, and when a
reserved preview group is still empty, keep the sibling editor as the
visible tab instead of following the host terminal.
@brennanb2025

Copy link
Copy Markdown
Contributor Author

Follow-up audit found a three-or-more-group focus bug in the merged fallback: an empty pending preview could select an unrelated editor by group creation order. The client-local narrowing is in #16129, with a regression that fails on current main and paired Electron validation.

Jinwoo-H added a commit that referenced this pull request Aug 24, 2026
Textual conflicts (7 files) resolved as semantic merges — both sides kept in
every case; no side was dropped.

Beyond the textual conflicts, main added four gates and one behaviour that this
branch's new code violated. Fixed in this merge:

- runtime-electron ratchet (#15968): the branch's `rpc/methods/browser-network-tunnel.ts`
  reached `ipc/ssh.ts`, dragging 20 Electron modules back into the runtime graph the
  orcad work had just emptied. The connection-manager accessor moves to
  `ssh/ssh-target-registry.ts` (the same extraction #15927 did for the other five
  accessors) and the dispatch reads it there; `wsl-browser-network-relay-launch.ts`
  reads the app root through the AppEnvironment port. Baseline back to 0 entries.

- HTML side-preview focus (STA-5001, #15716): main keeps the source editor focused
  when the reserved preview split is still empty, by steering the visible tab. This
  branch's client-owned placement reconciler decides the active group on its own and
  bypassed that, refocusing the preview. It now takes the same fallback tab.

- child_process / windowsHide / wsl.exe census gates: the WSL browser-network relay
  launcher moves onto `spawnProcess`, and the two Electron test launchers now spawn
  with `windowsHide`. The four sites that cannot migrate today are allowlisted with
  their reason (detached process-group kill, a non-all-pipes stdio shape, and a
  long-lived streaming wsl.exe child that `runWslProcess` cannot express).

- max-lines: both sides grew `package-electron-runtime-contract.test.mjs`; the union
  crossed 600. This branch's two additions move to
  `config/scripts/client-hosted-browser-package-coverage.test.mjs`.

Also ports four browser-create test files onto main's
`applyWebSessionTabsSnapshot`/`decideWebSessionTabsSnapshot` split (#15644).

Gates: typecheck 0 errors, `pnpm lint` exit 0, 6316/6369 test files green. The 12
remaining failures are pre-existing on one or both parents (5 packaging tests need a
real `pnpm install`, global-fetch audit and 4 others fail on both parents) or flaky.
dallascrilley pushed a commit to dallascrilley/orca that referenced this pull request Aug 27, 2026
…-5001) (stablyai#15716)

Opening a remote HTML preview to the side created an empty split and made it
the active group. The next host session-tab snapshot still had the terminal
active, so the client treated that empty group as a terminal focus change.

Do not activate the empty split for unfocused remote previews, and when a
reserved preview group is still empty, keep the sibling editor as the
visible tab instead of following the host terminal.
enrick-dev pushed a commit to enrick-dev/orca that referenced this pull request Oct 15, 2026
…-5001) (stablyai#15716)

Opening a remote HTML preview to the side created an empty split and made it
the active group. The next host session-tab snapshot still had the terminal
active, so the client treated that empty group as a terminal focus change.

Do not activate the empty split for unfocused remote previews, and when a
reserved preview group is still empty, keep the sibling editor as the
visible tab instead of following the host terminal.
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