From fa3bd289958b03a33bc5ec20fe6e1c25e2f3f4ca Mon Sep 17 00:00:00 2001 From: Jinwoo Hong <73622457+Jinwoo-H@users.noreply.github.com> Date: Mon, 26 Oct 2026 15:29:06 -0400 Subject: [PATCH] Let Resource Manager End stop orphaned terminals that main is attached to (#34040) --- .../terminal-host-fenced-termination.ts | 1 + src/main/ipc/pty/ipc/renderer-kill.ts | 10 +- src/main/ipc/pty/renderer-fenced-kill.test.ts | 106 ++++++++++++++++++ .../terminal/layout/terminal-view-presence.ts | 5 + ...urce-manager-end-tabless-terminals.spec.ts | 62 ++++++++++ 5 files changed, 183 insertions(+), 1 deletion(-) create mode 100644 tests/e2e/resource-manager-end-tabless-terminals.spec.ts diff --git a/src/main/daemon/terminal-host-fenced-termination.ts b/src/main/daemon/terminal-host-fenced-termination.ts index a771b81f09b..45869c858ea 100644 --- a/src/main/daemon/terminal-host-fenced-termination.ts +++ b/src/main/daemon/terminal-host-fenced-termination.ts @@ -32,6 +32,7 @@ export function beginTerminalHostFencedTermination( if (request.requireNoAgentOwner && deps.owners.listForPty(request.id).length > 0) { return { accepted: false, result: { outcome: 'refused', reason: 'claimed' } } } + // Current mains check viewers themselves; this guards older mains and direct callers. if (request.requireUnattached && session.hasAttachedClients) { return { accepted: false, result: { outcome: 'refused', reason: 'attached' } } } diff --git a/src/main/ipc/pty/ipc/renderer-kill.ts b/src/main/ipc/pty/ipc/renderer-kill.ts index 3755be609f5..b7f295fafa6 100644 --- a/src/main/ipc/pty/ipc/renderer-kill.ts +++ b/src/main/ipc/pty/ipc/renderer-kill.ts @@ -161,7 +161,15 @@ async function stopFencedRendererPty( if (!provider?.killFenced) { return { outcome: 'refused', reason: 'unsupported' } } - const result = await provider.killFenced(args) + // Main attaches what it spawns, so only main sees viewers; best-effort, a later viewer sees the confirmed End. + if (args.requireUnattached && deps.runtime?.hasTerminalViewer(args.id)) { + return { outcome: 'refused', reason: 'attached' } + } + // Without a runtime to ask about viewers, keep the daemon's attached check. + const result = await provider.killFenced({ + ...args, + requireUnattached: args.requireUnattached && !deps.runtime + }) if ( (result.outcome === 'ended' || result.outcome === 'already_exited') && ptyIncarnationById.get(args.id) === args.expectedIncarnationId diff --git a/src/main/ipc/pty/renderer-fenced-kill.test.ts b/src/main/ipc/pty/renderer-fenced-kill.test.ts index 4e0028cb850..e03ea9580bc 100644 --- a/src/main/ipc/pty/renderer-fenced-kill.test.ts +++ b/src/main/ipc/pty/renderer-fenced-kill.test.ts @@ -1,6 +1,10 @@ import { describe, expect, it, vi } from 'vitest' +import { DaemonPtyAdapter } from '../../daemon/daemon-pty-adapter' +import { waitFor } from '../../daemon/daemon-pty-adapter-test-harness' import { startLateExitHarness } from './daemon-late-exit-test-fixture' import { ptyIncarnationById } from './provider/ownership-state' +import { installPtyKillIpcHandler } from './ipc/renderer-kill' +import { shutdownProviderAndDetectExit } from './provider/shutdown-detect' const request = (id: string, expectedIncarnationId: string) => ({ id, @@ -66,6 +70,108 @@ describe('checked renderer stop route', () => { } }) + it('ends a tab-less terminal this run spawned, though main holds its stream', async () => { + const harness = await startLateExitHarness() + try { + await expect( + harness.killFenced({ + ...request(harness.id, harness.result.incarnationId ?? 'missing'), + requireUnattached: true + }) + ).resolves.toEqual({ outcome: 'ended' }) + } finally { + await harness.dispose() + } + }) + + it('keeps the daemon attached check when no runtime can answer for viewers', async () => { + const harness = await startLateExitHarness() + try { + installPtyKillIpcHandler({ + getLocalPtyProviderStartupPromise: () => undefined, + shutdownProviderAndDetectExit, + rememberSyntheticKillExit: harness.session.rememberSyntheticKillExit, + sendPtyExitToRenderer: harness.session.sendPtyExitToRenderer + }) + await expect( + harness.killFenced({ + ...request(harness.id, harness.result.incarnationId ?? 'missing'), + requireUnattached: true + }) + ).resolves.toEqual({ outcome: 'refused', reason: 'attached' }) + } finally { + await harness.dispose() + } + }) + + it('ends a tab-less restart survivor once main attaches it', async () => { + const harness = await startLateExitHarness() + const previousRun = new DaemonPtyAdapter({ + socketPath: harness.socketPath, + tokenPath: harness.tokenPath + }) + try { + const survivorId = 'repo::/tmp/late-exit-audit@@survivor' + const survivor = await previousRun.spawn({ cols: 80, rows: 24, sessionId: survivorId }) + const detached = vi.spyOn(harness.server['attachments'], 'detachClientSessions') + await previousRun.disconnectOnly() + await waitFor(() => detached.mock.calls.length > 0) + await harness.adapter.attach(survivorId) + await expect( + harness.killFenced({ + ...request(survivorId, survivor.incarnationId ?? 'missing'), + requireUnattached: true + }) + ).resolves.toEqual({ outcome: 'ended' }) + } finally { + await harness.dispose() + } + }) + + it('refuses a tab-less end while a pane or remote view uses the terminal', async () => { + const harness = await startLateExitHarness() + try { + const checked = { + ...request(harness.id, harness.result.incarnationId ?? 'missing'), + requireUnattached: true + } + const releaseRemote = harness.runtime.registerRemoteTerminalViewSubscriber(harness.id) + await expect(harness.killFenced(checked)).resolves.toEqual({ + outcome: 'refused', + reason: 'attached' + }) + releaseRemote() + harness.runtime.syncWindowGraph(1, { + tabs: [ + { + tabId: 'tab-1', + worktreeId: 'repo::/tmp/late-exit-audit', + title: '', + activeLeafId: 'leaf-1', + layout: null + } + ], + leaves: [ + { + tabId: 'tab-1', + worktreeId: 'repo::/tmp/late-exit-audit', + leafId: 'leaf-1', + paneRuntimeId: 1, + ptyId: harness.id, + paneTitle: '' + } + ] + }) + await expect(harness.killFenced(checked)).resolves.toEqual({ + outcome: 'refused', + reason: 'attached' + }) + expect(harness.subprocess.forceKill).not.toHaveBeenCalled() + } finally { + await harness.dispose() + } + }) + it('preserves operator-close provenance when the physical exit beats the control response', async () => { const harness = await startLateExitHarness() try { diff --git a/src/main/runtime/terminal/layout/terminal-view-presence.ts b/src/main/runtime/terminal/layout/terminal-view-presence.ts index 8268a31bd14..c7fc2e49fe7 100644 --- a/src/main/runtime/terminal/layout/terminal-view-presence.ts +++ b/src/main/runtime/terminal/layout/terminal-view-presence.ts @@ -82,6 +82,11 @@ export abstract class OrcaRuntimeWithNotifyRemoteTerminalViewPresenceChanged ext return this.terminalViewSubscribers.hasRaw(ptyId) } + /** A pane or remote view uses this PTY; main's own ingestion stream does not count. */ + hasTerminalViewer(ptyId: string): boolean { + return this.getLeavesForPty(ptyId).length > 0 || this.hasRawTerminalViewSubscriber(ptyId) + } + /** Whether a remote view answers this PTY's queries now. */ hasRemoteTerminalViewSubscriber(ptyId: string): boolean { return this.terminalViewSubscribers.hasRemoteAnsweringQueries(ptyId) diff --git a/tests/e2e/resource-manager-end-tabless-terminals.spec.ts b/tests/e2e/resource-manager-end-tabless-terminals.spec.ts new file mode 100644 index 00000000000..d930557e260 --- /dev/null +++ b/tests/e2e/resource-manager-end-tabless-terminals.spec.ts @@ -0,0 +1,62 @@ +import { mkdirSync } from 'node:fs' +import { expect, test } from './helpers/orca-app' +import { makeResourceManagerFixtureVisible } from './helpers/resource-manager-visible-fixture' +import { waitForSessionReady } from './helpers/store' + +test.use({ seedTestRepo: false }) + +test('Resource Manager ends a tab-less terminal this run spawned', async ({ + orcaPage, + electronApp +}, testInfo) => { + const folderPath = testInfo.outputPath('notes') + mkdirSync(folderPath, { recursive: true }) + await waitForSessionReady(orcaPage) + await orcaPage.setViewportSize({ width: 1200, height: 900 }) + const ptyId = await orcaPage.evaluate(async (folderPath) => { + const state = window.__store!.getState() + const group = await window.api.projectGroups.create({ + name: 'Notes', + parentPath: folderPath, + createdFrom: 'folder-scan' + }) + await state.fetchProjectGroups({ runtimeEnvironmentId: null, throwOnError: true }) + if (!group) { + throw new Error('Could not create project group') + } + const folder = await state.createFolderWorkspace( + { projectGroupId: group.id, name: 'Notes', folderPath }, + { runtimeEnvironmentId: null } + ) + if (!folder) { + throw new Error('Could not create folder workspace') + } + const spawned = await window.api.pty.spawn({ + cols: 80, + rows: 24, + cwd: folderPath, + worktreeId: `folder:${folder.id}`, + initiallyHidden: true + }) + return spawned.id + }, folderPath) + + await makeResourceManagerFixtureVisible(electronApp, orcaPage) + await orcaPage.getByRole('button', { name: /^Resource Manager,/ }).click() + await orcaPage.getByRole('tab', { name: 'PTYs', exact: true }).click() + const panel = orcaPage.getByRole('tabpanel', { name: /^PTYs/ }) + await expect(panel.getByText(/1 without a tab/)).toBeVisible() + await panel.getByRole('button', { name: /^End / }).click() + const dialog = orcaPage.getByRole('dialog', { name: /^End .*\?$/ }) + await dialog.getByRole('button', { name: 'End terminal', exact: true }).click() + // The dialog closes only after End returns, so the row state below is End's result. + await expect(dialog).toBeHidden() + await expect(panel.getByRole('button', { name: /^End / })).toHaveCount(0) + await expect + .poll(async () => + (await orcaPage.evaluate(() => window.api.pty.listSessions({ connectionId: null }))).map( + (session) => session.id + ) + ) + .not.toContain(ptyId) +})