Repository navigation
fix(runtime): a paired runtime's frame no longer drops another runtime's terminals in a shared workspace - #33414
Merged
Merged
Conversation
…e republishes a shared workspace Desktop B can mirror one SSH workspace from two paired runtimes: the host's server, which runs its terminals, and desktop A's app, which hosts a browser tab A opened there. Any frame from A's app replaced every mirrored terminal row in the workspace, so B dropped the server's terminals until the server's next frame re-created them. A frame now replaces only the mirrored terminals and chats its own runtime owns, matching how browser and editor rows were already scoped. Fixes #31983 Reported-by: amara-black
…ing host's replay A tombstone now retires only its own host's rows, so the visibility-resume repair no longer has to hold it back until every surviving host has a frame to replay over the wiped workspace.
…me may retire Bindings first, then the frames that last published the row, then the workspace's own server; owners that were removed no longer count, so any runtime's frame can retire their rows.
Drop the workspace-owner guess for rows with no evidence, let a frame that publishes the same chat take its row over, and refocus a surviving runtime's terminal when the active one is retired.
…time claims it Union pane bindings and last-frame mappings into one claim set, so a row two runtimes share survives one runtime's retraction. Drop the chat ownership extension: chats are shared across runtimes by session id, and the reported bug is terminals only.
A runtime that stopped publishing a terminal no longer claims it through a cached pane binding; bindings only speak before any frame this session. Keep the visibility-resume repair's survivor deferral as it was.
…me; chats follow the same rule A shared terminal whose bound runtime drops it is retired and rebuilt by the other runtime's next frame, so it never streams from a runtime that left. Chats another live runtime owns survive this frame unless it publishes or replaces them.
A workspace can now keep terminals another runtime publishes, so a close follows the runtime the tab's own PTY names before the workspace catalog.
Contributor
|
Warning Review limit reachedYou'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 24 seconds. View limit details
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. Comment |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
ELI5
Some setups show one workspace on a desktop using tabs from two different paired Orca runtimes. Example: the SSH host's server runs the terminals, and another desktop's Orca app hosts a browser tab that was opened there. Each time the second runtime sent an update, the desktop threw away every terminal tab in that workspace, then put them back seconds later. Now each runtime's update only touches the tabs that runtime owns.
What Changed
Before: desktop B is paired to an SSH host's managed server and also to desktop A's app. When A opened or reloaded a browser tab in the SSH workspace, or B closed A's browser tab, B dropped all of that workspace's terminal tabs. They came back a few seconds later as new rows (same PTYs, new
createdAt). Sometimes they stayed gone until B was reloaded. The agents kept running the whole time; only B's view lost them.After: B keeps the server's terminals and chats no matter what A's app publishes for that workspace.
Mechanism:
shouldReplaceTerminalTabreplaced every mirrored terminal row on any runtime's frame. It now asks one ownership rule,web-session-tabs-sync/mirror-row-ownership.ts:executionHostId. A frame that publishes or replaces (replacesSessionId) a chat takes that row over, so one chat never gets two rows.Why
Browser and editor rows already followed "a runtime's frame speaks only for its own rows". Terminals and chats didn't. Giving them the same per-source ownership fixes the whole class with one rule.
Alternative considered: ignore frames from any runtime that isn't the workspace's owner. Rejected, because it would also hide browser tabs another desktop hosts, which the mirror supports on purpose.
Two expectations in
web-session-tabs-sync-visibility-collision.test.tsxencoded the old behavior, where the last publisher wiped the other runtime's terminals. They now show each host's rows staying until that host's own omission is applied. The visibility-resume repair logic itself is unchanged.ref-oss check (this took more than 3 review rounds): per-source ownership, where each backend keeps and retires only its own terminals, is the standard pattern for clients that aggregate several remote backends. The design moved from separate guards to one ownership rule and dropped a coordinator change.
Linked Issue
Fixes #31983
Thanks to @amara-black for the detailed traces, the profile-state analysis and the follow-up tests that pinned this down.
Visual Proof
N/A. This is a renderer state reconciliation fix with no UI change; the visible effect (terminal tabs no longer disappearing) is covered by the unit reproductions below.
Testing
I manually tested these changes locally
Automated tests added/updated, or explained why not below
New
web-session-tabs-sync-cross-runtime-worktree.test.tsruns B's real snapshot-apply pipeline with two runtimes publishing the same workspace. It covers:createdAtstaying stable;The core cases fail on origin/main.
A close-routing case was added to
terminal-tab-actions-pinned-server-close.test.ts.Single-worker runs of every test file under
src/renderer/src/runtimethat imports the changed modules (212 files), the importers outside that folder (192 files) and the close-route importers all pass.The full static job passes locally, except two steps that can't run on this machine: the Node 18 process-host smoke (no Node 18 installed) and the orcad terminal smoke (it needs a mobile-web build). This PR touches neither.
Not verified live: no two-desktop run. The headless e2e harness can't pair one client to two runtimes that publish the same workspace.
AI Disclosure
Review
Codex review is clean on the final head (base pinned at 407e9eb). An independent review found no P0 or P1 issues.
Agent skill upstream boundary
Notes
Checklist
N/Awith reasonpnpm lint,pnpm typecheck,pnpm test, andpnpm buildpass (or CI will cover; local preferred)