diff --git a/.github/workflows/pr.yml b/.github/workflows/pr.yml index 4717da652fc..4a7c9d1e39e 100644 --- a/.github/workflows/pr.yml +++ b/.github/workflows/pr.yml @@ -571,6 +571,15 @@ jobs: src/main/runtime/worktree-scan-admin-fingerprint-gate.test.ts src/shared/secure-file-fsync-flags.test.ts + - name: Build Electron app for Windows terminal lifecycle E2E + run: pnpm exec electron-vite build --mode e2e + + - name: Run Windows terminal close lifecycle regression + env: + SKIP_BUILD: '1' + ORCA_E2E_FORWARD_APP_LOGS: '1' + run: pnpm run test:e2e tests/e2e/windows-headless-terminal-close-lifecycle.spec.ts --workers=1 + # Why the :parallel variant: identical to build:release except the three # electron-vite targets overlap instead of running back to back. The Linux package # job already packages and smoke-tests an AppImage built that way. diff --git a/src/main/runtime/orca-runtime.test.ts b/src/main/runtime/orca-runtime.test.ts index 5095cea186c..3589d886636 100644 --- a/src/main/runtime/orca-runtime.test.ts +++ b/src/main/runtime/orca-runtime.test.ts @@ -31475,9 +31475,10 @@ describe('OrcaRuntimeService', () => { }) ) const kill = vi.fn(() => true) + const closeTerminal = vi.fn() const closeTerminalTab = vi.fn(async () => {}) const runtime = new OrcaRuntimeService(runtimeStore as never) - runtime.setNotifier({ closeTerminalTab } as never) + runtime.setNotifier({ closeTerminal, closeTerminalTab } as never) runtime.setPtyController({ write: () => true, kill, @@ -31526,6 +31527,7 @@ describe('OrcaRuntimeService', () => { ptyKilled: true }) expect(kill).toHaveBeenCalledWith(ptyId) + expect(closeTerminal).toHaveBeenCalledWith('host-tab') expect(closeTerminalTab).not.toHaveBeenCalled() expect(getSession().tabsByWorktree[TEST_WORKTREE_ID]).toEqual([]) expect((await runtime.listMobileSessionTabs(`id:${TEST_WORKTREE_ID}`)).tabs).toEqual([]) @@ -31576,6 +31578,113 @@ describe('OrcaRuntimeService', () => { expect((await runtime.listMobileSessionTabs(`id:${TEST_WORKTREE_ID}`)).tabs).not.toEqual([]) }) + it('does not settle a missing mobile projection while the renderer still owns the parent', () => { + const { runtimeStore } = makeRuntimeStoreWithWorkspaceSession(getDefaultWorkspaceSession()) + const runtime = new OrcaRuntimeService(runtimeStore as never) + runtime.syncWindowGraph(1, { + tabs: [ + { + tabId: 'host-tab', + worktreeId: TEST_WORKTREE_ID, + title: 'Headless worker', + activeLeafId: HEADLESS_LEAF_ID, + layout: null + } + ], + leaves: [] + }) + const isAlreadyRetired = ( + runtime as unknown as { + isAlreadyRetiredMobileTerminalSurfaceError: ( + error: Error, + worktreeId: string, + parentTabId: string + ) => boolean + } + ).isAlreadyRetiredMobileTerminalSurfaceError.bind(runtime) + + expect(isAlreadyRetired(new Error('tab_not_found'), TEST_WORKTREE_ID, 'host-tab')).toBe(false) + }) + + it('keeps a split sibling when exit retires the addressed leaf before an unconfirmed stop returns', async () => { + const tabId = '33333333-3333-4333-8333-333333333333' + const leftPtyId = 'headless-split-left' + const rightPtyId = 'headless-split-right' + const layout = makeHeadlessTerminalLayout({ + [HEADLESS_LEAF_ID]: leftPtyId, + [HEADLESS_SECOND_LEAF_ID]: rightPtyId + }) + const { runtimeStore, getSession } = makeRuntimeStoreWithWorkspaceSession( + makeWorkspaceSessionWithHeadlessTerminal({ + activeTabId: tabId, + activeTabIdByWorktree: { [TEST_WORKTREE_ID]: tabId }, + tabsByWorktree: { + [TEST_WORKTREE_ID]: [ + { + id: tabId, + ptyId: leftPtyId, + worktreeId: TEST_WORKTREE_ID, + title: 'Split workers', + customTitle: null, + color: null, + sortOrder: 0, + createdAt: 1 + } + ] + }, + terminalLayoutsByTabId: { [tabId]: layout } + }) + ) + let runtime!: OrcaRuntimeService + let tabIdsAfterExit: string[] = [] + const kill = vi.fn(() => false) + const stopAndWait = vi.fn(async (ptyId: string) => { + runtime.onPtyExit(ptyId, 0) + tabIdsAfterExit = (await runtime.listMobileSessionTabs(`id:${TEST_WORKTREE_ID}`)).tabs.map( + (tab) => tab.id + ) + return false + }) + runtime = new OrcaRuntimeService({ ...runtimeStore, flushOrThrow: vi.fn() } as never) + runtime.setPtyController({ + write: () => true, + kill, + stopAndWait, + getForegroundProcess: async () => null, + listProcesses: async () => [] + }) + runtime.registerPty(leftPtyId, TEST_WORKTREE_ID, null, { + tabId, + leafId: HEADLESS_LEAF_ID + }) + runtime.registerPty(rightPtyId, TEST_WORKTREE_ID, null, { + tabId, + leafId: HEADLESS_SECOND_LEAF_ID + }) + runtime.syncWindowGraph(0, { tabs: [], leaves: [] }) + await runtime.listMobileSessionTabs(`id:${TEST_WORKTREE_ID}`) + const rightHandle = runtime.preAllocateHandleForPty(rightPtyId) + + await expect(runtime.closeTerminal(rightHandle)).resolves.toMatchObject({ + handle: rightHandle, + tabId, + ptyKilled: false, + ptyStopVerdict: 'unverifiable' + }) + + expect(stopAndWait).toHaveBeenCalledWith(rightPtyId, expect.anything()) + expect(tabIdsAfterExit).toEqual([`${tabId}::${HEADLESS_LEAF_ID}`]) + expect(kill).toHaveBeenCalledWith(rightPtyId) + expect(kill).not.toHaveBeenCalledWith(leftPtyId) + expect(getSession().tabsByWorktree[TEST_WORKTREE_ID]).toHaveLength(1) + expect(getSession().terminalLayoutsByTabId[tabId]?.ptyIdsByLeafId).toMatchObject({ + [HEADLESS_LEAF_ID]: leftPtyId + }) + expect( + (await runtime.listMobileSessionTabs(`id:${TEST_WORKTREE_ID}`)).tabs.map((tab) => tab.id) + ).toEqual([`${tabId}::${HEADLESS_LEAF_ID}`]) + }) + it('keeps the renderer close transaction for an adopted runtime-owned tab', async () => { // The renderer pin state can be newer than the debounced session, so once adopted its live close guard must win over stale persisted metadata. const servePtyId = 'serve-adopted-1' diff --git a/src/main/runtime/orca-runtime.ts b/src/main/runtime/orca-runtime.ts index 8de190f392d..d7d6deede5b 100644 --- a/src/main/runtime/orca-runtime.ts +++ b/src/main/runtime/orca-runtime.ts @@ -29161,7 +29161,7 @@ export class OrcaRuntimeService { private findMobileTerminalSurface( worktreeId: string, parentTabId: string, - options: { requireReady?: boolean } = {} + options: { leafId?: string; requireReady?: boolean } = {} ): RuntimeMobileSessionCreateTerminalResult | null { const snapshot = this.mobileSessionTabsByWorktree.get(worktreeId) if (!snapshot) { @@ -29169,7 +29169,10 @@ export class OrcaRuntimeService { } const result = this.toMobileSessionTabsResult(snapshot) const tab = result.tabs.find( - (candidate) => candidate.type === 'terminal' && candidate.parentTabId === parentTabId + (candidate) => + candidate.type === 'terminal' && + candidate.parentTabId === parentTabId && + (options.leafId === undefined || candidate.leafId === options.leafId) ) if (!tab || tab.type !== 'terminal') { return null @@ -29192,7 +29195,9 @@ export class OrcaRuntimeService { ): boolean { // Why: missing persistence is ambiguous while a sibling or stale live publication still owns the surface. return ( - error.message === 'tab_not_found' && !this.findMobileTerminalSurface(worktreeId, parentTabId) + error.message === 'tab_not_found' && + !this.tabs.has(parentTabId) && + !this.findMobileTerminalSurface(worktreeId, parentTabId) ) } @@ -29208,7 +29213,7 @@ export class OrcaRuntimeService { candidate.parentLayout?.ptyIdsByLeafId?.[candidate.leafId] === ptyId) ) return tab?.type === 'terminal' - ? this.findMobileTerminalSurface(worktreeId, tab.parentTabId) + ? this.findMobileTerminalSurface(worktreeId, tab.parentTabId, { leafId: tab.leafId }) : null } @@ -29795,9 +29800,8 @@ export class OrcaRuntimeService { if (pty) { // Why: PTY exit can immediately replace a ready SSH publication with a pending one, so capture its durable HUB surface before killing it. const surface = - (pty.pty.tabId - ? this.findMobileTerminalSurface(pty.pty.worktreeId, pty.pty.tabId) - : null) ?? this.findMobileTerminalSurfaceForPty(pty.pty.worktreeId, pty.pty.ptyId) + this.findMobileTerminalSurfaceForPty(pty.pty.worktreeId, pty.pty.ptyId) ?? + (pty.pty.tabId ? this.findMobileTerminalSurface(pty.pty.worktreeId, pty.pty.tabId) : null) const tabId = surface?.tab.parentTabId ?? pty.pty.tabId ?? pty.record.tabId // Why: relay recovery can leave stale renderer leaves; the persisted HUB layout defines whether closing this PTY closes the whole surface. const siblingCount = surface?.tab.parentLayout @@ -29831,7 +29835,16 @@ export class OrcaRuntimeService { return this.describeTerminalClose(handle, tabId, pty.pty.ptyId, ptyKilled) } const ptyKilled = await this.stopExplicitlyClosedTabPtys([pty.pty.ptyId], pty.pty.ptyId) - if (!ptyKilled || siblingCount <= 1) { + // Why: an exit callback can retire the addressed split leaf before an unconfirmed stop returns; parent fallback would destroy its surviving siblings. + const addressedSplitSurfaceRetired = + siblingCount > 1 && + surface !== null && + !this.mobileSessionSnapshotHasSurface( + pty.pty.worktreeId, + surface.tab.parentTabId, + surface.tab.leafId + ) + if ((!ptyKilled || siblingCount <= 1) && !addressedSplitSurfaceRetired) { if (surface) { // Why: paired viewers keep ended streams mounted until the HUB publishes removal, so explicit close uses the durable host-tab transaction instead of viewer-local exit handling. try {