Repository navigation
fix(remote): keep resumed agent tabs stable on headed hosts - #11448
Conversation
📝 WalkthroughWalkthroughRuntime-owned PTYs are tracked through creation, persistence, renderer-tab rescue, mobile terminal assignment, teardown, and exit. Mobile terminals remain listed when renderer snapshots omit runtime-owned tabs and are removed after closure or synchronized absence. Agent status mirroring now remaps tab and worktree attribution, preserves fresher state timestamps, reconciles provider sessions, and includes provider-session equivalence in comparisons. Tests cover attribution repair, epoch updates, and obsolete session clearing. 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
📝 Generate docstrings
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 |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Greptile SummaryThis PR fixes two renderer-side state-reconciliation bugs in
Confidence Score: 5/5Renderer-side reconciliation fix with no tab-lifecycle side effects; the runtime ownership flag has clean set/clear paths and is exercised by new tests. The three changed functions in web-session-tabs-sync.ts have narrow, well-tested scopes and the previously flagged provider-session loss on the fresher-OSC path is now correctly handled by the explicit providerSession assignment. The runtimeSessionOwned flag in orca-runtime.ts has consistent lifecycle coverage across createTerminal, splitTerminal propagation, PTY exit, and explicit tab close. Files Needing Attention: No files require special attention.
|
| Filename | Overview |
|---|---|
| src/renderer/src/runtime/web-session-tabs-sync.ts | Core renderer-side fix: adds tabId remapping in remapHostAgentStatus, correct providerSession preservation in buildMirroredAgentStatusPatch, and providerSession equality check in agentStatusEntryEqual. Logic is sound and the previously-flagged provider-session loss on the fresher-OSC path is addressed. |
| src/renderer/src/runtime/web-session-tabs-sync.test.ts | Significantly expanded test coverage: attribution repair, fresher-OSC identity retention, same-state identity update, and next-turn providerSession isolation are all exercised within the renamed test. |
| src/main/runtime/orca-runtime.ts | Adds runtimeSessionOwned flag to RuntimePtyWorktreeRecord with consistent set/clear lifecycle and integrates the flag into shouldPreserveMobileSessionTab as a new preservation path. |
| src/main/runtime/orca-runtime.test.ts | Adds a test for live headed runtime-owned tab preservation until explicit close, and extends an existing test across multiple syncWindowGraph updates. |
Sequence Diagram
sequenceDiagram
participant Host as Host Runtime
participant Client as Paired Client
participant Sync as web-session-tabs-sync
participant State as AgentStatusByPaneKey
Host->>Client: Snapshot with agentStatus (tabId: host-tab-1, paneKey: host-tab-1:leaf)
Client->>Sync: remapHostAgentStatus(surface)
Note over Sync: paneKey to web-terminal-host-tab-1:leaf, tabId to web-terminal-host-tab-1 (NEW)
Sync->>State: entry with remapped paneKey + tabId
alt OSC 9999 fresher
Note over Sync: Keep existing visible state, take host ownership metadata (NEW)
Sync->>State: nextEntry merges existing state with host paneKey/worktreeId/tabId/providerSession
else Host snapshot is fresher
Sync->>State: "nextEntry = remapped entry"
end
Note over Sync: agentStatusEntryEqual now checks agentProviderSessionsEqual() (NEW)
alt providerSession changed
Sync->>State: Write nextEntry
else providerSession equal
Sync->>State: Skip no-op
end
Reviews (4): Last reviewed commit: "fix(remote): preserve headed runtime age..." | Re-trigger Greptile
OrcaWin
changed the title
brennanb2025
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/main/runtime/orca-runtime.ts (1)
27856-27856: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valuePreservation logic OK; minor duplication between the two ownership-check helpers.
hasLiveRuntimeSessionOwnedPtyBindingandhasLiveOrPersistedServeOrSshOwnedPtyBindingare both consulted here and both walk PTY bindings for the same tab via similar lookups. Not a correctness issue — could be worth folding into one combined preservation predicate later for readability, but not essential now.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 13d0e17e-568e-4f49-ad3b-0be507faedea
📒 Files selected for processing (4)
src/main/runtime/orca-runtime.test.tssrc/main/runtime/orca-runtime.tssrc/renderer/src/runtime/web-session-tabs-sync.test.tssrc/renderer/src/runtime/web-session-tabs-sync.ts
| runtime.syncWindowGraph(1, { | ||
| tabs: [], | ||
| leaves: [ | ||
| { | ||
| tabId: 'tab-renderer', | ||
| worktreeId: TEST_WORKTREE_ID, | ||
| leafId: HEADLESS_SECOND_LEAF_ID, | ||
| paneRuntimeId: 1, | ||
| ptyId: 'pty-renderer', | ||
| paneTitle: null | ||
| } | ||
| ], | ||
| mobileSessionTabs: [ | ||
| { | ||
| worktree: TEST_WORKTREE_ID, | ||
| publicationEpoch: 'epoch-1', | ||
| snapshotVersion: 3, | ||
| activeGroupId: 'group-1', | ||
| activeTabId: null, | ||
| activeTabType: null, | ||
| tabs: [] | ||
| } | ||
| ] | ||
| }) | ||
| runtime.syncWindowGraph(1, { | ||
| tabs: [], | ||
| leaves: [ | ||
| { | ||
| tabId: 'tab-renderer', | ||
| worktreeId: TEST_WORKTREE_ID, | ||
| leafId: HEADLESS_SECOND_LEAF_ID, | ||
| paneRuntimeId: 1, | ||
| ptyId: 'pty-renderer', | ||
| paneTitle: null | ||
| } | ||
| ], | ||
| mobileSessionTabs: [ | ||
| { | ||
| worktree: TEST_WORKTREE_ID, | ||
| publicationEpoch: 'epoch-1', | ||
| snapshotVersion: 4, | ||
| activeGroupId: 'group-1', | ||
| activeTabId: null, | ||
| activeTabType: null, | ||
| tabs: [] | ||
| } | ||
| ] | ||
| }) | ||
|
|
||
| expect((await runtime.listMobileSessionTabs(`id:${TEST_WORKTREE_ID}`)).tabs).toEqual([]) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Assert retention after snapshot version 3.
Without an assertion between versions 3 and 4, immediate pruning at version 3 still passes the final empty-list assertion. Assert the terminal remains after version 3, then verify removal after version 4.
Proposed test addition
runtime.syncWindowGraph(1, {
tabs: [],
leaves: [
{
tabId: 'tab-renderer',
worktreeId: TEST_WORKTREE_ID,
leafId: HEADLESS_SECOND_LEAF_ID,
paneRuntimeId: 1,
ptyId: 'pty-renderer',
paneTitle: null
}
],
mobileSessionTabs: [
{
worktree: TEST_WORKTREE_ID,
publicationEpoch: 'epoch-1',
snapshotVersion: 3,
activeGroupId: 'group-1',
activeTabId: null,
activeTabType: null,
tabs: []
}
]
})
+ expect((await runtime.listMobileSessionTabs(`id:${TEST_WORKTREE_ID}`)).tabs).toEqual([
+ expect.objectContaining({ parentTabId: 'tab-renderer', ptyId: 'pty-renderer' })
+ ])
runtime.syncWindowGraph(1, {Based on PR objectives: renderer-omission handling must preserve tabs until synchronized absence.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| runtime.syncWindowGraph(1, { | |
| tabs: [], | |
| leaves: [ | |
| { | |
| tabId: 'tab-renderer', | |
| worktreeId: TEST_WORKTREE_ID, | |
| leafId: HEADLESS_SECOND_LEAF_ID, | |
| paneRuntimeId: 1, | |
| ptyId: 'pty-renderer', | |
| paneTitle: null | |
| } | |
| ], | |
| mobileSessionTabs: [ | |
| { | |
| worktree: TEST_WORKTREE_ID, | |
| publicationEpoch: 'epoch-1', | |
| snapshotVersion: 3, | |
| activeGroupId: 'group-1', | |
| activeTabId: null, | |
| activeTabType: null, | |
| tabs: [] | |
| } | |
| ] | |
| }) | |
| runtime.syncWindowGraph(1, { | |
| tabs: [], | |
| leaves: [ | |
| { | |
| tabId: 'tab-renderer', | |
| worktreeId: TEST_WORKTREE_ID, | |
| leafId: HEADLESS_SECOND_LEAF_ID, | |
| paneRuntimeId: 1, | |
| ptyId: 'pty-renderer', | |
| paneTitle: null | |
| } | |
| ], | |
| mobileSessionTabs: [ | |
| { | |
| worktree: TEST_WORKTREE_ID, | |
| publicationEpoch: 'epoch-1', | |
| snapshotVersion: 4, | |
| activeGroupId: 'group-1', | |
| activeTabId: null, | |
| activeTabType: null, | |
| tabs: [] | |
| } | |
| ] | |
| }) | |
| expect((await runtime.listMobileSessionTabs(`id:${TEST_WORKTREE_ID}`)).tabs).toEqual([]) | |
| runtime.syncWindowGraph(1, { | |
| tabs: [], | |
| leaves: [ | |
| { | |
| tabId: 'tab-renderer', | |
| worktreeId: TEST_WORKTREE_ID, | |
| leafId: HEADLESS_SECOND_LEAF_ID, | |
| paneRuntimeId: 1, | |
| ptyId: 'pty-renderer', | |
| paneTitle: null | |
| } | |
| ], | |
| mobileSessionTabs: [ | |
| { | |
| worktree: TEST_WORKTREE_ID, | |
| publicationEpoch: 'epoch-1', | |
| snapshotVersion: 3, | |
| activeGroupId: 'group-1', | |
| activeTabId: null, | |
| activeTabType: null, | |
| tabs: [] | |
| } | |
| ] | |
| }) | |
| expect((await runtime.listMobileSessionTabs(`id:${TEST_WORKTREE_ID}`)).tabs).toEqual([ | |
| expect.objectContaining({ parentTabId: 'tab-renderer', ptyId: 'pty-renderer' }) | |
| ]) | |
| runtime.syncWindowGraph(1, { | |
| tabs: [], | |
| leaves: [ | |
| { | |
| tabId: 'tab-renderer', | |
| worktreeId: TEST_WORKTREE_ID, | |
| leafId: HEADLESS_SECOND_LEAF_ID, | |
| paneRuntimeId: 1, | |
| ptyId: 'pty-renderer', | |
| paneTitle: null | |
| } | |
| ], | |
| mobileSessionTabs: [ | |
| { | |
| worktree: TEST_WORKTREE_ID, | |
| publicationEpoch: 'epoch-1', | |
| snapshotVersion: 4, | |
| activeGroupId: 'group-1', | |
| activeTabId: null, | |
| activeTabType: null, | |
| tabs: [] | |
| } | |
| ] | |
| }) | |
| expect((await runtime.listMobileSessionTabs(`id:${TEST_WORKTREE_ID}`)).tabs).toEqual([]) |
Native Windows verification of
|
| Check | Result |
|---|---|
| Resumed tab survives 3 × 15 s switch-away cycles | PASS |
| PTY stays bound across every cycle | PASS |
Agent status attached to the resumed tab (web-terminal-0306131b-…) |
PASS |
| Provider session + transcript path correct | PASS (019fb10c-…) |
| Exactly one agent process spawned by the resume | PASS |
Dwell was 15 s deliberately — CLOSE_INTENT_TTL_MS is 10_000, and the earlier 1.5 s validation passed precisely because it never reached the failure window.
Also re-ran the two changed suites locally: 987 passed.
On the fix itself: it adds a narrow runtimeSessionOwned flag rather than widening hasLiveOrPersistedServeOrSshOwnedPtyBinding, which avoids the tab-resurrection hazard that predicate's comment warns about. All four set-sites are gated on runtime-session provenance — notably ensurePtyBackedMobileSurfaceForRendererTab early-returns unless a runtime-session create is pending, so ordinary renderer tabs never acquire the flag — and it is cleared on renderer close and PTY exit.
…#11448) * fix(remote): keep resumed agent tabs stable * fix(remote): retain resume identity from older snapshots * fix(remote): refresh mirrored resume attribution * fix(remote): preserve headed runtime agent tabs
A ledger of what this branch carries on top of stablyai/orca, with the upstream issue/PR to watch for each entry so a change can be dropped once upstream lands its own fix. Flags the stablyai#11448 cherry-pick as redundant from v1.4.164-rc.0. Claude-Session: https://claude.ai/code/session_01S765eBk3XW7WmsvBxF17V9
Drops the stablyai#11448 entry (upstream's copy arrived with the merge) and the skill entry (local tooling, not application divergence). Folds the agent-status retention fix into the remote-tab entry and records the remote notification gap as known-open, including why it must be built on top of the status fix. Claude-Session: https://claude.ai/code/session_01S765eBk3XW7WmsvBxF17V9
…#11448) * fix(remote): keep resumed agent tabs stable * fix(remote): retain resume identity from older snapshots * fix(remote): refresh mirrored resume attribution * fix(remote): preserve headed runtime agent tabs
Summary
web-terminal-*identity and retain the correct provider identity without restoring an older turnRoot cause
A headed desktop runtime can create a local PTY on behalf of a paired client. The PTY remains live and authoritative in main, but the host renderer can omit its synthetic session tab during reconciliation. The previous preservation predicate only recognized headless,
serve-*, and SSH-owned PTYs, so a live local PTY created through the runtime-session path could be dropped from the publication while its process remained alive.The runtime now records whether a PTY is runtime-session-owned and preserves it only while its exact worktree, parent tab, and leaf binding is still live. Splits inherit ownership. Explicit close, retirement, and exit clear it, so this does not turn stale or rebound PTYs into durable tabs.
The renderer-side reconciliation also keeps the paired client's remapped ownership/provider metadata and prevents an older host
donesnapshot from restoring a previous provider session onto a newer clientworkingturn.Compatibility and performance
serve-*, SSH, and headless preservation remains intact.Validation
Headed Electron paired-runtime validation on final source
Validated commit
59db88d257on macOS with an isolated headed Electron host and a separate paired Electron client. The harness rejected local scope and required:A structured Codex resume fixture reported through Orca's real hook and OSC status paths. The client switched to a second worktree for 15 seconds (past the 10-second close-intent TTL), returned, and confirmed:
web-terminal-*session_id: pr11448-electron-resume)workingstate remains on the resumed tabThis run covers the renderer-backed headed-host topology that previously dropped locally owned runtime PTYs. It is isolated same-machine pairing, not a physical remote-server, Windows, or Linux rerun. The earlier Windows 11 real-Codex validation still covers the original paired-runtime reproduction and renderer identity behavior, but the final ownership fix was not rerun on that rig.
Automated checks
pnpm run typecheck:nodepnpm run typecheck:weboxfmt --checkpnpm run check:max-lines-ratchetpnpm run check:reliability-gatesgit diff --checkScreenshots
Resumed agent working before the worktree round trip:
The same resumed tab and working agent after 15 seconds away and re-entry:
Review history
d644677a8c.a73e37c032.Security
No authentication, authorization, credential, filesystem, shell, or network-routing behavior changed. The new state is an in-memory PTY ownership marker used only by session-tab preservation and cleanup.