From a88e1eaa0aa5c9f763c3d8f7a8b5563467893d7e Mon Sep 17 00:00:00 2001 From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Date: Fri, 25 Sep 2026 20:58:33 -0700 Subject: [PATCH] fix(mobile): paired clients re-derive a kept terminal after a cold restore A renderer frame published before a cold-restored terminal's PTY registered was fenced to an empty tab list and recorded as accepted, and the renderer never resends unchanged content. When registerPty binds a surface the accepted frame fenced out, re-merge that frame so the fence reads current state. --- ...ime-fenced-renderer-frame-rederive.test.ts | 174 ++++++++++++++++++ src/main/runtime/orca-runtime-register-pty.ts | 1 + src/main/runtime/orca-runtime-runtime-id.ts | 2 + .../orca-runtime-sync-mobile-session-tabs.ts | 85 ++++++--- 4 files changed, 236 insertions(+), 26 deletions(-) create mode 100644 src/main/runtime/orca-runtime-fenced-renderer-frame-rederive.test.ts diff --git a/src/main/runtime/orca-runtime-fenced-renderer-frame-rederive.test.ts b/src/main/runtime/orca-runtime-fenced-renderer-frame-rederive.test.ts new file mode 100644 index 00000000000..563893cfc95 --- /dev/null +++ b/src/main/runtime/orca-runtime-fenced-renderer-frame-rederive.test.ts @@ -0,0 +1,174 @@ +import { describe, expect, it, vi } from 'vitest' +import { getDefaultWorkspaceSession } from '../../shared/constants' +import type { + RuntimeMobileSessionTabsResult, + RuntimeMobileSessionTabsSnapshot +} from '../../shared/runtime-types' +import type { WorkspaceSessionState } from '../../shared/workspace-session-state-types' +import { OrcaRuntimeService } from './orca-runtime' + +const WORKTREE_ID = 'repo::/worktree' +const REPO_ID = 'repo' +const LIVE_REPO = { + id: REPO_ID, + path: '/worktree', + displayName: 'repo', + badgeColor: 'blue', + addedAt: 1 +} as const +const LEFT = '11111111-1111-4111-8111-111111111111' +const RIGHT = '22222222-2222-4222-8222-222222222222' +const SPLIT_ROOT = { + type: 'split' as const, + direction: 'vertical' as const, + first: { type: 'leaf' as const, leafId: LEFT }, + second: { type: 'leaf' as const, leafId: RIGHT } +} + +// A cold restore: the persisted split survives, and an earlier incarnation change left the repo's +// terminal membership host-authoritative, but no PTY has registered yet. +function makeColdRestoredSession(): WorkspaceSessionState { + return { + ...getDefaultWorkspaceSession(), + tabsByWorktree: { + [WORKTREE_ID]: [ + { + id: 'tab', + ptyId: 'pty-left', + worktreeId: WORKTREE_ID, + title: 'Terminal', + customTitle: null, + color: null, + sortOrder: 0, + createdAt: 1 + } + ] + }, + terminalLayoutsByTabId: { + tab: { + root: SPLIT_ROOT, + activeLeafId: LEFT, + expandedLeafId: null, + ptyIdsByLeafId: { [LEFT]: 'pty-left', [RIGHT]: 'pty-right' } + } + }, + terminalTopologyRevisionByRepoId: { [REPO_ID]: 2 } + } +} + +function makeRendererFrame(): RuntimeMobileSessionTabsSnapshot { + const parentLayout = { + root: SPLIT_ROOT, + activeLeafId: LEFT, + expandedLeafId: LEFT, + ptyIdsByLeafId: { [LEFT]: 'pty-left', [RIGHT]: 'pty-right' } + } + return { + worktree: WORKTREE_ID, + publicationEpoch: 'renderer', + snapshotVersion: 1, + activeGroupId: 'group', + activeTabId: `tab::${LEFT}`, + activeTabType: 'terminal', + tabGroups: [{ id: 'group', activeTabId: 'tab', tabOrder: ['tab'] }], + tabs: [ + { + type: 'terminal', + id: `tab::${LEFT}`, + parentTabId: 'tab', + leafId: LEFT, + ptyId: 'pty-left', + title: 'Left', + parentLayout, + isActive: true + }, + { + type: 'terminal', + id: `tab::${RIGHT}`, + parentTabId: 'tab', + leafId: RIGHT, + ptyId: 'pty-right', + title: 'Right', + parentLayout, + isActive: false + } + ] + } +} + +function publishRendererFrame(runtime: OrcaRuntimeService): void { + runtime.syncWindowGraph(1, { + tabs: [ + { + tabId: 'tab', + worktreeId: WORKTREE_ID, + title: 'Terminal', + activeLeafId: LEFT, + layout: SPLIT_ROOT + } + ], + leaves: [], + mobileSessionTabs: [makeRendererFrame()] + }) +} + +function coldRestoredRuntime(): OrcaRuntimeService { + const session = makeColdRestoredSession() + const runtime = new OrcaRuntimeService({ + getRepos: () => [LIVE_REPO], + getWorkspaceSession: () => session + } as never) + runtime.attachWindow(1) + // The desktop window is live, so main must not rebuild the list from the persisted session. + vi.spyOn(runtime as never, 'getAvailableAuthoritativeWindow').mockReturnValue({} as never) + return runtime +} + +async function listedSurfaces(runtime: OrcaRuntimeService): Promise { + const result = await runtime.listMobileSessionTabs(`id:${WORKTREE_ID}`) + return result.tabs.map((tab) => `${tab.id}:${'status' in tab ? tab.status : ''}`) +} + +describe('a renderer frame fenced before its PTY registered', () => { + it.each([ + ['local', null], + ['SSH', 'ssh-1'] + ])('%s: re-derives the fence once the PTY registers', async (_host, connectionId) => { + const runtime = coldRestoredRuntime() + publishRendererFrame(runtime) + expect(await listedSurfaces(runtime)).toEqual([]) + const published: RuntimeMobileSessionTabsResult[] = [] + const unsubscribe = runtime.onMobileSessionTabsChanged((event) => published.push(event)) + + runtime.registerPty('pty-left', WORKTREE_ID, connectionId, { + tabId: 'tab', + leafId: LEFT, + incarnationId: 'incarnation-restored' + }) + + expect(published.at(-1)?.tabs.map((tab) => tab.id)).toEqual([`tab::${LEFT}`]) + expect(await listedSurfaces(runtime)).toEqual([`tab::${LEFT}:ready`]) + // The renderer's unchanged resend must not undo or churn the re-derived list. + publishRendererFrame(runtime) + expect(await listedSurfaces(runtime)).toEqual([`tab::${LEFT}:ready`]) + unsubscribe() + }) + + it('keeps a surface whose PTY never returns fenced when a sibling registers', async () => { + const runtime = coldRestoredRuntime() + publishRendererFrame(runtime) + + runtime.registerPty('pty-left', WORKTREE_ID, null, { + tabId: 'tab', + leafId: LEFT, + incarnationId: 'incarnation-restored' + }) + runtime.registerPty('pty-other', WORKTREE_ID, null, { + tabId: 'tab-other', + leafId: RIGHT, + incarnationId: 'incarnation-other' + }) + + expect(await listedSurfaces(runtime)).toEqual([`tab::${LEFT}:ready`]) + }) +}) diff --git a/src/main/runtime/orca-runtime-register-pty.ts b/src/main/runtime/orca-runtime-register-pty.ts index 37a5434e5c5..7496f72ed35 100644 --- a/src/main/runtime/orca-runtime-register-pty.ts +++ b/src/main/runtime/orca-runtime-register-pty.ts @@ -144,6 +144,7 @@ export class OrcaRuntimeWithRegisterPty extends OrcaRuntimeWithInvalidateAllHand // mobile create's tab is live; publish its surface main-side (#7587). if (binding && paneKey) { this.ensurePtyBackedMobileSurfaceForRendererTab(worktreeId, binding.tabId) + this.rederiveFencedRendererSurface(worktreeId, ptyId, binding.tabId, binding.leafId) } } diff --git a/src/main/runtime/orca-runtime-runtime-id.ts b/src/main/runtime/orca-runtime-runtime-id.ts index 7b1811d9255..3de51000e7b 100644 --- a/src/main/runtime/orca-runtime-runtime-id.ts +++ b/src/main/runtime/orca-runtime-runtime-id.ts @@ -138,6 +138,8 @@ export class OrcaRuntimeWithRuntimeId { protected acceptedRendererMobileSnapshotByWorktree = new Map< string, { + /** The renderer's frame as received, so its fence can be re-derived when the fence's inputs change. */ + frame: RuntimeMobileSessionTabsSnapshot publicationEpoch: string rendererVersion: number rendererTabCount: number diff --git a/src/main/runtime/orca-runtime-sync-mobile-session-tabs.ts b/src/main/runtime/orca-runtime-sync-mobile-session-tabs.ts index 51997577179..b317d56760f 100644 --- a/src/main/runtime/orca-runtime-sync-mobile-session-tabs.ts +++ b/src/main/runtime/orca-runtime-sync-mobile-session-tabs.ts @@ -155,32 +155,7 @@ export class OrcaRuntimeWithSyncMobileSessionTabs extends OrcaRuntimeWithWriteOr ) { continue } - this.nativeChatDraftResolutions.reconcile(snapshot) - const launchDraftFencedSnapshot = this.nativeChatDraftResolutions.applyFence(snapshot) - const fencedSnapshot = this.applyMobileSessionRetirementFences(launchDraftFencedSnapshot) - this.releaseRuntimeSessionOwnershipForRendererRetiredTabs(fencedSnapshot, existing) - const nextSnapshot = this.mergePreservedHeadlessMobileSessionTabs(fencedSnapshot, existing) - // Why: clients drop same-epoch frames whose version isn't strictly newer, - // and main-local touches may already have emitted a higher version than - // the renderer's counter — keep the stored version strictly monotonic so - // the accepted content is never discarded as stale downstream. - const storedVersion = existing - ? Math.max(nextSnapshot.snapshotVersion, existing.snapshotVersion + 1) - : nextSnapshot.snapshotVersion - this.storeMobileSessionSnapshot( - snapshot.worktree, - storedVersion === nextSnapshot.snapshotVersion - ? nextSnapshot - : { ...nextSnapshot, snapshotVersion: storedVersion } - ) - this.acceptedRendererMobileSnapshotByWorktree.set(snapshot.worktree, { - publicationEpoch: snapshot.publicationEpoch, - rendererVersion: snapshot.snapshotVersion, - rendererTabCount: fencedSnapshot.tabs.length, - rendererTabIdentityKeys: new Set( - fencedSnapshot.tabs.flatMap((tab) => getMobileSessionSnapshotTabIdentityKeys(tab)) - ) - }) + this.mergeRendererMobileSnapshot(snapshot) } for (const [worktreeId, existing] of [...this.mobileSessionTabsByWorktree.entries()]) { if (!nextWorktrees.has(worktreeId)) { @@ -218,4 +193,62 @@ export class OrcaRuntimeWithSyncMobileSessionTabs extends OrcaRuntimeWithWriteOr } return changedWorktreeIds } + + // Why: a surface fenced out of the accepted frame before its PTY registered stays out, because the + // renderer never resends unchanged content; re-merge the frame once that PTY binds. D1 (main as the + // single membership writer) absorbs this gate. + protected rederiveFencedRendererSurface( + worktreeId: string, + ptyId: string, + tabId: string, + leafId: string + ): void { + const accepted = this.acceptedRendererMobileSnapshotByWorktree.get(worktreeId) + if ( + !accepted || + accepted.rendererTabIdentityKeys.has(`${tabId}::${leafId}`) || + !accepted.frame.tabs.some( + (tab) => + tab.type === 'terminal' && + tab.parentTabId === tabId && + tab.leafId === leafId && + tab.ptyId === ptyId + ) + ) { + return + } + this.mergeRendererMobileSnapshot(accepted.frame) + this.notifyMobileSessionTabsChanged(worktreeId) + } + + protected mergeRendererMobileSnapshot(snapshot: RuntimeMobileSessionTabsSnapshot): void { + const existing = this.mobileSessionTabsByWorktree.get(snapshot.worktree) + this.nativeChatDraftResolutions.reconcile(snapshot) + const launchDraftFencedSnapshot = this.nativeChatDraftResolutions.applyFence(snapshot) + const fencedSnapshot = this.applyMobileSessionRetirementFences(launchDraftFencedSnapshot) + this.releaseRuntimeSessionOwnershipForRendererRetiredTabs(fencedSnapshot, existing) + const nextSnapshot = this.mergePreservedHeadlessMobileSessionTabs(fencedSnapshot, existing) + // Why: clients drop same-epoch frames whose version isn't strictly newer, + // and main-local touches may already have emitted a higher version than + // the renderer's counter — keep the stored version strictly monotonic so + // the accepted content is never discarded as stale downstream. + const storedVersion = existing + ? Math.max(nextSnapshot.snapshotVersion, existing.snapshotVersion + 1) + : nextSnapshot.snapshotVersion + this.storeMobileSessionSnapshot( + snapshot.worktree, + storedVersion === nextSnapshot.snapshotVersion + ? nextSnapshot + : { ...nextSnapshot, snapshotVersion: storedVersion } + ) + this.acceptedRendererMobileSnapshotByWorktree.set(snapshot.worktree, { + frame: snapshot, + publicationEpoch: snapshot.publicationEpoch, + rendererVersion: snapshot.snapshotVersion, + rendererTabCount: fencedSnapshot.tabs.length, + rendererTabIdentityKeys: new Set( + fencedSnapshot.tabs.flatMap((tab) => getMobileSessionSnapshotTabIdentityKeys(tab)) + ) + }) + } }