Skip to content

fix(tabs): keep an open diff focused while agents stream (STA-4697) - #15390

Merged
brennanb2025 merged 2 commits into
mainfrom
brennanb2025/sta-4697-unable-to-open-diff-file
Aug 19, 2026
Merged

brennanb2025 merged 2 commits into
mainfrom
brennanb2025/sta-4697-unable-to-open-diff-file

Conversation

@brennanb2025

@brennanb2025 brennanb2025 commented Aug 18, 2026 •

Copy link
Copy Markdown
Contributor
Files Added Deleted Net
Test 2 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​388 0 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​388
Prod 6 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​57 $\color{#cf222e}{\Huge{\mathbf{−}}}$​17 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​40

ELI5

Open a diff while an agent is running and the diff would flash up, then the app would jump back to the terminal. The diff tab stayed in the tab bar, just not selected — so clicking more files piled up dead tabs.

The code that decides "which tab is the user looking at?" was asking the wrong question. It only knew a coarse answer — "an editor" — and then went hunting for a tab whose type was literally editor. A diff tab's type is diff, so the hunt always failed, and "I found nothing" is the same signal the reconciler uses for "nothing to preserve, go ahead and pick a terminal."

Now it just asks the tab groups what is on screen, which is the thing that actually knows.

What Changed

resolveWebSessionVisibleTabId no longer inverts the coarse address. When group records exist:

  1. Resolve the active group (activeGroupIdByWorktree, else groups[0] — the existing recovery in deriveActiveSurfaceForWorktree)
  2. A focused empty group returns null — authoritative that nothing is visible, rather than resolving into some other group
  3. Otherwise the group's activeTabId, validated with tab.groupId === activeGroup.id
  4. If that id is gone, follow the entity within the same group — reconcile rematerializes local tabs as mirrored ones under new ids
  5. The coarse address is consulted only when there are zero group records, and now projects through toVisibleTabType instead of comparing exactly

Two supporting changes:

  • web-runtime-session.ts — the browser-create focus guard's change filter now observes groupsByWorktree / activeGroupIdByWorktree, which the resolver newly reads. Without this a group-only focus move returns early and never re-evaluates.
  • First commit moves the canonical toVisibleTabType beside the two unions it maps between and drops the two copies already identical to it.

Why

toVisibleTabType is many-to-one: editor | diff | conflict-review | check-details all collapse to 'editor', and activeTabType is produced by it. Inverting that by equality is wrong in principle — 'diff' is just the instance that bit. Fixing only the comparison would leave the inversion in place, so the resolver now works forward from concrete group state.

That also makes it immune to a real drift: activateTab writes group state only, while setActiveFile / setActiveTabType write the legacy records separately, so the two disagree transiently in ordinary flows.

Pre-existing, not a regression. The guard was introduced 2026-06-15 by #5435 (413fb31ab5) matching contentType === 'editor' — blind to diff tabs from day one, which is a little ironic given #5435 is the focus-steal fix this guard's comment cites. #13876 (70ad65fc94, 08-12) hoisted it and preserved the blindness. Present in v1.4.184. No cherry-pick: the 08-15 release shipped as v1.4.183 with v1.4.184 on top, and no release-prep branch or RC is in flight.

Reachability. On main the reducer is reached only via refreshWebRuntimeSessionTabsSnapshot, gated on a bound activeRuntimeEnvironmentId — so remote / SSH / paired / headless-serve workspaces can hit this on shipped builds. The report came from an adhoc build of a side branch that adds a second, ungated caller, which is why it reproduced on a plain local workspace there.

Deliberately not in this PR

  • Focused empty splits. Broken today (the resolver returns a background group's tab and reconciliation follows it) and still broken after (it returns null and a terminal wins). Preserving them needs a tagged focus result, both callers, and group/layout preservation in the reconciler's group rebuild — an unreserved empty client group is discarded during rebuild regardless of what the resolver returns.
  • Simulator coarse type. Simulators get strictly better here — the tab is no longer stolen — but activeTabType still reports 'terminal' on a terminal echo, exactly as today, because currentVisibleTabTypeStillValid has no simulator branch. Fixing that goes with unifying the third toVisibleTabType copy.

Linked Issue

STA-4697

Visual Proof

N/A — reproducing the symptom live needs a bound runtime environment plus agents actively streaming, which does not survive a screenshot. The behavior is pinned by an end-to-end test instead: against the pre-fix resolver it fails with expected 'web-terminal-host-tab-1' to be 'local-diff-tab', which is precisely the reported symptom (diff replaced by the terminal, tab retained).

Testing

macOS.

  • vitest run --config config/vitest.config.ts src/renderer/src/runtime src/renderer/src/store/slices — 374 files, 3473 tests passed.
  • Red-before-green, measured, not assumed: reverted the resolver body and re-ran. 8 of 12 new unit tests go red; the 4 that stay green are property tests and are labeled as such in the file rather than counted as regression coverage. The end-to-end test fails pre-fix with the assertion quoted above.
  • pnpm typecheck — exit 0, and proven live: injecting const x: number = "s" produced TS2322, then reverted. This repo prints nothing on success, so a silent pass is otherwise indistinguishable from a dead gate.
  • oxfmt --check — clean, echoed 8 files so the list actually arrived.
  • pnpm lint — exit 0.

The full-suite run earned its keep: it caught a regression that static review missed. When a local editor tab is rematerialized as a mirrored tab under a new id, the group's pre-reconcile activeTabId no longer resolves, so focus was dropped and the snapshot took over — breaking does not let stale browser intent override a newer editor selection. Step 4 above fixes it, pinned by two tests (follows the entity; does not follow it across groups).

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

Review

Agent skill upstream boundary

  • Not applicable.

Notes

  • Cross-platform: no platform-conditional code; entity and worktree ids are opaque strings here.
  • Remote SSH: this is the remote path. No RPC param, stream opcode, or published-content change — purely client-side reconciliation, so no wire-compatibility work is required.
  • Folder workspaces: keyed by the same generic worktreeId throughout.
  • Backwards compatibility: no persisted-state or schema change. The no-group path preserves today's behavior for fresh slices and the first remote reconcile.
  • Performance: strictly less work in the common case — a group lookup and one find instead of scanning for a type/entity match.

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)

Author

  • X / Twitter: @BrennanKB5

Three copies of toVisibleTabType had drifted: the runtime one omits 'simulator'.
Move the canonical projection next to the two unions it maps between and replace
the two copies that are already identical to it. The runtime copy is left alone
on purpose - unifying it would change behavior, so it goes with the follow-up.
resolveWebSessionVisibleTabId answered 'which tab is the user looking at' by
inverting a many-to-one projection: it compared tab.contentType against the
coarse activeTabType. Diff tabs open with activeTabType 'editor' but carry
contentType 'diff', so the match never succeeded and the guard returned null -
which is the reconciler's signal to fall through and activate a terminal. Every
agent status echo republished the snapshot, so the diff lost focus ~300ms after
opening, once per click. Same for conflict-review and check-details.

Resolve the visible tab from group state instead, which is what is actually on
screen and is the rule deriveActiveSurfaceForWorktree already uses. The coarse
address survives only when there are no group records, now projected rather than
compared exactly.

Also follow the entity within the group when reconcile rematerializes the visible
tab under a new id, and teach the browser-create focus guard to observe the group
records the resolver now reads.
@coderabbitai

coderabbitai Bot commented Aug 18, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro Plus

Run ID: fadec88d-79ba-4df2-be21-d9cbbeadddab

📥 Commits

Reviewing files that changed from the base of the PR and between 1b5cc00 and 3c00e2c.

📒 Files selected for processing (8)
  • src/renderer/src/runtime/web-runtime-session.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-diff-focus-steal.test.ts
  • src/renderer/src/store/slices/tabs.ts
  • src/renderer/src/store/slices/worktrees/listing/worktree-catalog-visibility.ts
  • src/renderer/src/store/slices/worktrees/session/active-worktree-surface.ts
  • src/shared/tab-types.ts
💤 Files with no reviewable changes (1)
  • src/renderer/src/store/slices/worktrees/listing/worktree-catalog-visibility.ts

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


📝 Walkthrough

Walkthrough

The change centralizes visible tab type conversion in src/shared/tab-types.ts. Web-session focus resolution now uses active group state, supports rematerialized tab IDs, and retains no-group fallback behavior. Browser creation reevaluates focus when group state changes. Tests cover grouped resolution, type normalization, and protection against terminal status echoes stealing focus from diff tabs.

Merge Risk: ⚪ Minimal · up to 3c00e

This localized change keeps the visible diff tab focused during agent activity and includes targeted regression coverage and validation; no actionable merge-blocking risk remains beyond normal checks and review.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. 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 main fix: preserving focus on open diff tabs while agents stream.
Description check ✅ Passed The description follows the template, explains the change and rationale, links the issue, documents testing, and addresses compatibility and scope.
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.
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Create stacked PR
  • Commit on current branch

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

STA-4697 SSH / remote-runtime validation

Review-only run against this PR worktree. No code, commit, or PR-text edits.

What I exercised

Paired desktop client (this PR) to a same-worktree hub that owns a throwaway Linux Docker SSH target with a real demo-project repo. Opened sta4697-validation.txt through the visible Source Control row (UI click). Then republished the host session.tabs snapshot twice via the shipped refreshWebRuntimeSessionTabsSnapshot path after the hub terminal title/status became codex [thinking]. The host snapshot still named the terminal as activeTabId / activeTabType: terminal — the pre-fix steal payload.

Identity

  • Client window.api.app.getIdentity(): Orca: brennanb2025/sta-4697-unable-to-open-diff-file, isDev: true, devLabel: sta4697-client, devBranch: brennanb2025/sta-4697-unable-to-open-diff-file, devWorktreeName: ruffe, devRepoRoot: /Users/brennanbenson/orca/workspaces/orca/ruffe
  • Hub identity: same worktree/branch, devLabel: sta4697-hub
  • Isolated ORCA_DEV_USER_DATA_PATH + CDP 9334 (client) / 9335 (hub), renderers 5175 / 5176
  • Client activeRuntimeEnvironmentId=3a506857-434e-4849-9465-5d293a301145 (bound remote runtime)

Target shape

  • Throwaway Linux container orca-ssh-sta4697-1787100025 (Debian 12 bookworm, linux-arm64), not ssh localhost
  • SSH: root@127.0.0.1:2222 (Docker-published port) with a one-off ed25519 key
  • Repo: https://github.com/stablyai/demo-project cloned via local bundle fallback (/work/demo-project, HEAD 141b6ff on main)
  • Relay: pnpm build:relay first; hub log Platform: linux-arm64 then Relay started successfully; container process node relay.js + handshake OK

Visible result

The diff tab stayed the active selected tab after the status-echo snapshot republish. The mirrored terminal retitled to codex [thinking] (snapshot applied) but did not steal focus. Neighboring path: clicking the terminal tab, then refreshing the snapshot, left the terminal focused.

Before republish (diff open, terminal still Terminal 1) After republish (terminal is codex [thinking], diff still selected)
01-diff-open.png 02-after-republish.png

Store after two refreshes: groupActiveTabId remained the local diff tab (sta4697-validation.txt); activeTabType stayed 'editor' (coarse projection); host snapshot activeTabType: 'terminal'.

Remote backing / logs

  • Remote git: ## main...origin/main [ahead 1] + M sta4697-validation.txt
  • Relay log: ORCA-RELAY v0.1.0 READY, handshake OK, no relay crash
  • sshd: accepted the test key; no auth failures
  • Playwright: 0 console errors. Unrelated noise only: Claude usage refresh failed (disposable profile), lsof port-scan timeouts, orchestration terminal_liveness_unavailable, tooltip controlled/uncontrolled, xterm deadline

Gaps

  • Opened a branch diff (contentType: 'diff'), not an unstaged working-tree diff — same tab kind the bug keys on
  • Republish was the exact shipped refreshWebRuntimeSessionTabsSnapshot after a hub title/status echo; a live Codex hook POST from inside the container PTY was not used (OSC title via pty.write did not change the hub tab title)
  • Nested paired-desktop + hub-owned SSH, not orca serve inside the container
  • Paired client shows the expected “Reconnect SSH target” affordance (client does not hold the SSH target)

SSH validation verdict: land. The shipped remote-runtime path kept the clicked diff focused while the host snapshot said the terminal was active.

@brennanb2025

Copy link
Copy Markdown
Contributor Author

Review status: LAND

Completed the independent same-model review loop plus readiness checks at both the start and end of the session. No P0/P1/P2 findings; counsel was not required.

  • Scope: remains focused on STA-4697. The two-commit, eight-file diff resolves visible focus from concrete active-group state and centralizes the existing visible-type projection. No unrelated product scope was added.
  • Correctness: focused tests passed 26/26 and broader relevant tab/editor tests passed 65/65. The implementation handles diff, conflict-review, check-details, split-group ownership, stale group ids, and tab rematerialization without crossing groups.
  • Performance: no added polling, unbounded state, subprocess work, or persistent listener. The new work is bounded per-worktree lookup inside existing reconciliation; the additional observed records are used only by the existing short-lived browser-create focus guard.
  • Compatibility: no persisted-state, mobile-facing, RPC/IPC, stream, or wire contract changed. Folder-workspace and SSH ids remain opaque. Windows packaging, Git compatibility, and cross-version wire checks are green.
  • CI: 46 successful checks, one expected skipped e2e job, zero failures, including Node 24/26 tests, typecheck, static analysis, package, and Windows package.
  • Electron + SSH: full evidence and before/after screenshots. On a disposable Debian 12 linux-arm64 SSH target with a real repo, the diff opened through the visible Source Control UI and remained selected across two shipped session.tabs refreshes while the host snapshot named the terminal active and its title changed to codex [thinking]. Clicking the terminal afterward also remained stable.
  • Cleanup: disposable container, Electron processes, profiles, keys, temp files, and Playwright artifacts were removed. Tracked worktree is clean; only the eight pre-existing untracked reviews/*.md files remain.

No code changes were warranted during this review, so there was nothing additional to commit or push; local and remote PR heads remain 3c00e2ce.

Residual validation gaps are narrow and recorded: the visual proof used a branch diff rather than an unstaged diff (same contentType: 'diff'), invoked the exact shipped refresh after a terminal status/title echo rather than a live hook POST, and used paired desktop + hub-owned SSH rather than orca serve inside the container.

Verdict: land as-is.

@brennanb2025
brennanb2025 merged commit c228030 into main Aug 19, 2026
47 checks passed
paidaxingyo666 pushed a commit to paidaxingyo666/Manta that referenced this pull request Aug 21, 2026
…tablyai#15390)

* refactor(tabs): put the visible-tab-type projection in one place

Three copies of toVisibleTabType had drifted: the runtime one omits 'simulator'.
Move the canonical projection next to the two unions it maps between and replace
the two copies that are already identical to it. The runtime copy is left alone
on purpose - unifying it would change behavior, so it goes with the follow-up.

* fix(tabs): keep an open diff focused while agents stream (STA-4697)

resolveWebSessionVisibleTabId answered 'which tab is the user looking at' by
inverting a many-to-one projection: it compared tab.contentType against the
coarse activeTabType. Diff tabs open with activeTabType 'editor' but carry
contentType 'diff', so the match never succeeded and the guard returned null -
which is the reconciler's signal to fall through and activate a terminal. Every
agent status echo republished the snapshot, so the diff lost focus ~300ms after
opening, once per click. Same for conflict-review and check-details.

Resolve the visible tab from group state instead, which is what is actually on
screen and is the rule deriveActiveSurfaceForWorktree already uses. The coarse
address survives only when there are no group records, now projected rather than
compared exactly.

Also follow the entity within the group when reconcile rematerializes the visible
tab under a new id, and teach the browser-create focus guard to observe the group
records the resolver now reads.
dallascrilley pushed a commit to dallascrilley/orca that referenced this pull request Aug 27, 2026
…tablyai#15390)

* refactor(tabs): put the visible-tab-type projection in one place

Three copies of toVisibleTabType had drifted: the runtime one omits 'simulator'.
Move the canonical projection next to the two unions it maps between and replace
the two copies that are already identical to it. The runtime copy is left alone
on purpose - unifying it would change behavior, so it goes with the follow-up.

* fix(tabs): keep an open diff focused while agents stream (STA-4697)

resolveWebSessionVisibleTabId answered 'which tab is the user looking at' by
inverting a many-to-one projection: it compared tab.contentType against the
coarse activeTabType. Diff tabs open with activeTabType 'editor' but carry
contentType 'diff', so the match never succeeded and the guard returned null -
which is the reconciler's signal to fall through and activate a terminal. Every
agent status echo republished the snapshot, so the diff lost focus ~300ms after
opening, once per click. Same for conflict-review and check-details.

Resolve the visible tab from group state instead, which is what is actually on
screen and is the rule deriveActiveSurfaceForWorktree already uses. The coarse
address survives only when there are no group records, now projected rather than
compared exactly.

Also follow the entity within the group when reconcile rematerializes the visible
tab under a new id, and teach the browser-create focus guard to observe the group
records the resolver now reads.
enrick-dev pushed a commit to enrick-dev/orca that referenced this pull request Oct 15, 2026
…tablyai#15390)

* refactor(tabs): put the visible-tab-type projection in one place

Three copies of toVisibleTabType had drifted: the runtime one omits 'simulator'.
Move the canonical projection next to the two unions it maps between and replace
the two copies that are already identical to it. The runtime copy is left alone
on purpose - unifying it would change behavior, so it goes with the follow-up.

* fix(tabs): keep an open diff focused while agents stream (STA-4697)

resolveWebSessionVisibleTabId answered 'which tab is the user looking at' by
inverting a many-to-one projection: it compared tab.contentType against the
coarse activeTabType. Diff tabs open with activeTabType 'editor' but carry
contentType 'diff', so the match never succeeded and the guard returned null -
which is the reconciler's signal to fall through and activate a terminal. Every
agent status echo republished the snapshot, so the diff lost focus ~300ms after
opening, once per click. Same for conflict-review and check-details.

Resolve the visible tab from group state instead, which is what is actually on
screen and is the rule deriveActiveSurfaceForWorktree already uses. The coarse
address survives only when there are no group records, now projected rather than
compared exactly.

Also follow the entity within the group when reconcile rematerializes the visible
tab under a new id, and teach the browser-create focus guard to observe the group
records the resolver now reads.
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