diff --git a/src/main/browser/agent-browser-bridge-execution.ts b/src/main/browser/agent-browser-bridge-execution.ts index 65f64b6fb08..31f5a191ede 100644 --- a/src/main/browser/agent-browser-bridge-execution.ts +++ b/src/main/browser/agent-browser-bridge-execution.ts @@ -12,6 +12,7 @@ import { import { translateResult } from './agent-browser-bridge-result' import { AgentBrowserBridgeTabs } from './agent-browser-bridge-tabs' import { ORCA_TAB_SESSION_PREFIX } from './agent-browser-orphan-sweep' +import { canSkipAgentBrowserSessionReset } from './agent-browser-session-reset' import { STALE_SESSION_CLOSE_TIMEOUT_MS, type AgentBrowserExecOptions, @@ -173,6 +174,15 @@ export abstract class AgentBrowserBridgeExecution extends AgentBrowserBridgeTabs } protected closeStaleAgentBrowserSession(sessionName: string): Promise { + if ( + canSkipAgentBrowserSessionReset({ + ownsSocketDirectory: this.ownsAgentBrowserSocketDirectory, + socketDirectory: this.agentBrowserEnv.AGENT_BROWSER_SOCKET_DIR, + sessionName + }) + ) { + return Promise.resolve() + } return new Promise((resolve, reject) => { let child: ReturnType | null = null let settled = false diff --git a/src/main/browser/agent-browser-bridge-session-lifecycle.test.ts b/src/main/browser/agent-browser-bridge-session-lifecycle.test.ts index 5eab202541c..2955cb66263 100644 --- a/src/main/browser/agent-browser-bridge-session-lifecycle.test.ts +++ b/src/main/browser/agent-browser-bridge-session-lifecycle.test.ts @@ -1,18 +1,26 @@ import { describe, it, expect, vi, beforeEach } from 'vitest' -const { execFileMock, webContentsFromIdMock, existsSyncMock, readFileSyncMock, stdinWrites } = - vi.hoisted(() => ({ - execFileMock: vi.fn(), - webContentsFromIdMock: vi.fn(), - existsSyncMock: vi.fn(() => false), - readFileSyncMock: vi.fn(() => Buffer.from('')), - stdinWrites: [] as string[] - })) +const { + execFileMock, + webContentsFromIdMock, + existsSyncMock, + readFileSyncMock, + lstatSyncMock, + stdinWrites +} = vi.hoisted(() => ({ + execFileMock: vi.fn(), + webContentsFromIdMock: vi.fn(), + existsSyncMock: vi.fn(() => false), + readFileSyncMock: vi.fn(() => Buffer.from('')), + lstatSyncMock: vi.fn(), + stdinWrites: [] as string[] +})) vi.mock('child_process', () => ({ execFile: execFileMock })) vi.mock('fs', () => ({ existsSync: existsSyncMock, readFileSync: readFileSyncMock, + lstatSync: lstatSyncMock, accessSync: vi.fn(), chmodSync: vi.fn(), constants: { X_OK: 1 } @@ -73,6 +81,14 @@ function closeCallCount(): number { describe('AgentBrowserBridge', () => { let bridge: AgentBrowserBridge + // The mocked fs has no mkdirSync, so the constructor never claims a socket directory itself. + function ownSocketDirectory(): void { + Object.assign(bridge, { + ownsAgentBrowserSocketDirectory: true, + agentBrowserEnv: { AGENT_BROWSER_SOCKET_DIR: '/tmp/orca-ab-test' } + }) + } + beforeEach(() => { resetAgentBrowserBridgeMocks({ webContentsFromIdMock, @@ -81,11 +97,29 @@ describe('AgentBrowserBridge', () => { stdinWrites, cdpWsProxyInstances: CdpWsProxyMock.instances }) + // Default to a socket that exists so an unprepared test still takes the reset path. + lstatSyncMock.mockReset() + lstatSyncMock.mockReturnValue({}) bridge = new AgentBrowserBridge(mockBrowserManager()) bridge.setActiveTab(100) }) + it('snapshots a fresh owned session without launching a helper just to close it', async () => { + ownSocketDirectory() + lstatSyncMock.mockImplementation(() => { + throw Object.assign(new Error('No socket'), { code: 'ENOENT' }) + }) + webContentsFromIdMock.mockReturnValue(mockWebContents(100)) + succeedWith({ snapshot: 'ready' }) + + expect(await bridge.snapshot()).toMatchObject({ snapshot: 'ready' }) + expect(closeCallCount()).toBe(0) + expect(lstatSyncMock).toHaveBeenCalledWith('/tmp/orca-ab-test/orca-tab-tab-1.sock') + }) + it('fails closed when stale agent-browser session ownership cannot be reset', async () => { + ownSocketDirectory() + lstatSyncMock.mockReturnValue({}) vi.useFakeTimers() try { const closeKill = vi.fn() diff --git a/src/main/browser/agent-browser-session-reset.test.ts b/src/main/browser/agent-browser-session-reset.test.ts new file mode 100644 index 00000000000..b38822d5175 --- /dev/null +++ b/src/main/browser/agent-browser-session-reset.test.ts @@ -0,0 +1,51 @@ +import { beforeEach, expect, it, vi } from 'vitest' +import { join } from 'node:path' + +const { lstatSync } = vi.hoisted(() => ({ lstatSync: vi.fn() })) +vi.mock('node:fs', () => ({ lstatSync })) +import { canSkipAgentBrowserSessionReset } from './agent-browser-session-reset' + +const owned = { + ownsSocketDirectory: true, + socketDirectory: '/tmp/orca-ab-profile', + sessionName: 'orca-tab-page' +} +const socketPath = join(owned.socketDirectory, 'orca-tab-page.sock') + +beforeEach(() => { + lstatSync.mockReset() +}) + +it('skips an absent owned socket', () => { + lstatSync.mockImplementation(() => { + throw Object.assign(new Error('No socket'), { code: 'ENOENT' }) + }) + expect(canSkipAgentBrowserSessionReset(owned)).toBe(true) + expect(lstatSync).toHaveBeenCalledWith(socketPath) +}) + +it('requires reset when a socket or symlink exists', () => { + lstatSync.mockReturnValue({}) + expect(canSkipAgentBrowserSessionReset(owned)).toBe(false) + expect(lstatSync).toHaveBeenCalledWith(socketPath) +}) + +it.each(['EACCES', 'EIO', 'ENOTDIR'])('requires reset for %s', (code) => { + lstatSync.mockImplementation(() => { + throw Object.assign(new Error('Socket inspection failed'), { code }) + }) + expect(canSkipAgentBrowserSessionReset(owned)).toBe(false) + expect(lstatSync).toHaveBeenCalledWith(socketPath) +}) + +// Windows and inherited socket directories both arrive as ownsSocketDirectory: false. +it.each([ + { ownsSocketDirectory: false }, + { socketDirectory: undefined }, + { sessionName: '../other' }, + { sessionName: 'has space' }, + { sessionName: '' } +])('requires reset without an owned Unix socket address: %j', (override) => { + expect(canSkipAgentBrowserSessionReset({ ...owned, ...override })).toBe(false) + expect(lstatSync).not.toHaveBeenCalled() +}) diff --git a/src/main/browser/agent-browser-session-reset.ts b/src/main/browser/agent-browser-session-reset.ts new file mode 100644 index 00000000000..8c6b7545f02 --- /dev/null +++ b/src/main/browser/agent-browser-session-reset.ts @@ -0,0 +1,30 @@ +import { lstatSync } from 'node:fs' +import { join } from 'node:path' + +// agent-browser's own session-name rule; doubles as a traversal fence for the `join` below. +const SAFE_SESSION_NAME = /^[A-Za-z0-9_-]+$/ + +/** + * True when no daemon can be holding `sessionName`, so closing it would only start one. + * + * Only an Orca-derived socket directory proves that (`ownsSocketDirectory`): it is a + * private per-profile `/tmp` directory, never an inherited one shared with a second + * profile, and never Windows, which uses named pipes and leaves no socket to inspect. + */ +export function canSkipAgentBrowserSessionReset(options: { + ownsSocketDirectory: boolean + socketDirectory: string | undefined + sessionName: string +}): boolean { + const { socketDirectory, sessionName } = options + if (!options.ownsSocketDirectory || !socketDirectory || !SAFE_SESSION_NAME.test(sessionName)) { + return false + } + try { + lstatSync(join(socketDirectory, `${sessionName}.sock`)) + return false + } catch (error) { + // Only a proven-absent socket is safe to skip; permission and other failures prove nothing. + return (error as NodeJS.ErrnoException).code === 'ENOENT' + } +} diff --git a/src/main/daemon/terminal-host-non-agent-foreground.test.ts b/src/main/daemon/terminal-host-non-agent-foreground.test.ts new file mode 100644 index 00000000000..4c50699ff2a --- /dev/null +++ b/src/main/daemon/terminal-host-non-agent-foreground.test.ts @@ -0,0 +1,92 @@ +import { afterEach, describe, expect, it, vi } from 'vitest' +import type { ProcessTableRow } from '../../shared/process-table-snapshot' +import type * as SnapshotReader from '../../shared/process-table-snapshot-reader' +import { inspectTerminalHostProcess } from './terminal-host-process-inspection' +import type { Session } from './session' + +const { readSnapshot } = vi.hoisted(() => ({ readSnapshot: vi.fn() })) +vi.mock('../../shared/process-table-snapshot-reader', async (importOriginal) => ({ + ...(await importOriginal()), + getStrictProcessTableSnapshotWithAge: readSnapshot +})) + +function table(command: string | null): ProcessTableRow[] { + const foregroundPgid = command === null ? 100 : 101 + const shell: ProcessTableRow = { + pid: 100, + ppid: 1, + pgid: 100, + tpgid: foregroundPgid, + tty: 'pts/1', + startTime: 'shell-start', + stat: command === null ? 'Ss+' : 'Ss', + command: '/bin/bash' + } + return command === null + ? [shell] + : [ + shell, + { + ...shell, + pid: 101, + ppid: 100, + pgid: 101, + stat: 'S+', + startTime: 'command-start', + command + } + ] +} + +async function inspect(rawName: string, command: string | null) { + readSnapshot.mockResolvedValue({ rows: table(command), capturedAgeMs: 0 }) + return inspectTerminalHostProcess({ + sessionId: 'busy-tab', + session: { + pid: 100, + incarnationId: 'incarnation-1', + isAlive: true, + getForegroundProcess: () => rawName + } as unknown as Session, + authorityGeneration: 'generation-1', + nextObservationEpoch: () => 1 + }) +} + +afterEach(() => { + vi.restoreAllMocks() + readSnapshot.mockClear() +}) + +describe.each(['linux', 'darwin'] as const)('daemon ordinary foreground on %s', (platform) => { + it.each(['sleep', 'vim', 'node'])( + 'retains the running %s name alongside agent-only evidence', + async (name) => { + vi.spyOn(process, 'platform', 'get').mockReturnValue(platform) + const result = await inspect(name, `${name} 300`) + expect(result).toMatchObject({ + foregroundProcess: name, + hasChildProcesses: true, + foregroundProcessEvidence: { verdict: 'live', processName: null } + }) + expect(readSnapshot).toHaveBeenCalledTimes(1) + } + ) + + it('still clears a stale recognized agent after its process exits', async () => { + vi.spyOn(process, 'platform', 'get').mockReturnValue(platform) + expect(await inspect('claude', null)).toMatchObject({ + foregroundProcess: null, + foregroundProcessEvidence: { verdict: 'live', processName: null } + }) + }) + + it('still reports no foreground command for an idle shell', async () => { + vi.spyOn(process, 'platform', 'get').mockReturnValue(platform) + expect(await inspect('bash', null)).toMatchObject({ + foregroundProcess: null, + hasChildProcesses: false, + foregroundProcessEvidence: { verdict: 'live', processName: null } + }) + }) +}) diff --git a/src/main/daemon/terminal-host-process-inspection.ts b/src/main/daemon/terminal-host-process-inspection.ts index 6810687631e..2f9fb1491d9 100644 --- a/src/main/daemon/terminal-host-process-inspection.ts +++ b/src/main/daemon/terminal-host-process-inspection.ts @@ -1,4 +1,5 @@ import { isShellProcess } from '../../shared/agent-detection' +import { recognizeAgentProcess } from '../../shared/agent-process-recognition' import type { RemoteForegroundEvidence } from '../../shared/foreground-process-evidence' import { getCheapProcessTableSnapshot } from '../../shared/cheap-process-table-snapshot-reader' import { getStrictProcessTableSnapshotWithAge } from '../../shared/process-table-snapshot-reader' @@ -100,9 +101,16 @@ export async function inspectTerminalHostProcess(args: { clearSteadyStateAnchor(session) } } + const nonShellForeground = foregroundProcess !== null && !isShellProcess(foregroundProcess) + // Evidence names recognized agents only, so its null must not erase an ordinary command (#18078). + const ordinaryForeground = + nonShellForeground && !recognizeAgentProcess(foregroundProcess) ? foregroundProcess : null return { - foregroundProcess: evidence.verdict === 'live' ? evidence.processName : foregroundProcess, - hasChildProcesses: foregroundProcess !== null && !isShellProcess(foregroundProcess), + foregroundProcess: + evidence.verdict === 'live' + ? (evidence.processName ?? ordinaryForeground) + : foregroundProcess, + hasChildProcesses: nonShellForeground, foregroundProcessEvidence: evidence } } diff --git a/src/main/persistence-deregistered-repo-residue.test.ts b/src/main/persistence-deregistered-repo-residue.test.ts index 3a7a3372b3c..a7fb4d7353f 100644 --- a/src/main/persistence-deregistered-repo-residue.test.ts +++ b/src/main/persistence-deregistered-repo-residue.test.ts @@ -1,7 +1,5 @@ // Why this file exists: deregistering a project used to strand every row it owned. No sweeper could -// reach them -- the missing-directory prune is gated on the repo still being registered, and a -// paired client's mirror of a remote host's rows is keyed by ids that client never registers, so the -// owning host's removal never reached it (#17776). +// reach them because the missing-directory prune is gated on the repo still being registered. import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest' import { rmSync, mkdtempSync } from 'node:fs' import { join } from 'node:path' @@ -103,7 +101,7 @@ describe('deregistered repo residue', () => { expect(session.sleepingAgentSessionsByPaneKey ?? {}).toEqual({}) }) - it("sweeps a remote host's session partition the owning host's removal can never reach", async () => { + it('keeps a remote session whose repo is not registered on the desktop', async () => { writeDataFile({ schemaVersion: 1, repos: [makeRepo({ id: LIVE_REPO, path: '/workspace/live' })], @@ -117,8 +115,10 @@ describe('deregistered repo residue', () => { store.flush() const partition = store.getWorkspaceSession(RUNTIME_HOST) - expect(partition.tabsByWorktree).toEqual({}) - expect(partition.activeTabTypeByWorktree).toEqual({}) + expect(partition.tabsByWorktree[GONE_WORKTREE]).toHaveLength(1) + expect(partition.activeTabTypeByWorktree).toEqual( + sessionFor(GONE_WORKTREE).activeTabTypeByWorktree + ) }) it('keeps rows for every registered repo, on any execution host', async () => { @@ -204,15 +204,13 @@ describe('deregistered repo residue', () => { schemaVersion: 1, repos: [makeRepo({ id: LIVE_REPO, path: '/workspace/live' })], worktreeMeta: {}, - workspaceSessionsByHostId: { - [RUNTIME_HOST]: { ...getDefaultWorkspaceSession(), ...session } - } + workspaceSession: { ...getDefaultWorkspaceSession(), ...session } }) const store = await createStore() store.flush() - const partition = store.getWorkspaceSession(RUNTIME_HOST) + const partition = store.getWorkspaceSession() expect(partition.activeWorktreeId ?? null).toBeNull() expect(partition.activeWorkspaceKey ?? null).toBeNull() expect(partition.activeWorktreeIdsOnShutdown ?? []).toEqual([]) diff --git a/src/main/persistence-remote-session-startup.test.ts b/src/main/persistence-remote-session-startup.test.ts new file mode 100644 index 00000000000..ddb3f57827b --- /dev/null +++ b/src/main/persistence-remote-session-startup.test.ts @@ -0,0 +1,109 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import { mkdtempSync, rmSync } from 'node:fs' +import { join } from 'node:path' +import { tmpdir } from 'node:os' +import { getDefaultWorkspaceSession } from '../shared/constants' +import type { BrowserPage, BrowserWorkspace } from '../shared/browser-workspace-types' +import { createStore, makeRepo, testState } from './persistence-test-harness' + +vi.mock('./ssh/ssh-config-parser', () => ({ + loadUserSshConfig: vi.fn(), + sshConfigHostsToTargets: vi.fn() +})) +vi.mock('electron', () => ({ + app: { getPath: () => testState.dir }, + safeStorage: { isEncryptionAvailable: () => false } +})) +vi.mock('./telemetry/client', () => ({ track: vi.fn() })) +vi.mock('./telemetry/cohort-classifier', () => ({ getCohortAtEmit: vi.fn().mockReturnValue({}) })) + +const HOST = 'runtime:paired-host' +const REPO = 'remote-repo' +const WORKTREE = `${REPO}::/remote/project` +const PAGE: BrowserPage = { + id: 'page-1', + workspaceId: 'browser-1', + worktreeId: WORKTREE, + url: 'https://example.test/moved', + title: 'Moved page', + loading: false, + canGoBack: true, + canGoForward: false, + faviconUrl: null, + loadError: null, + createdAt: 1, + browserRuntimeEnvironmentId: 'paired-host', + remoteBrowserPageId: 'remote-page-1', + remoteBrowserPageClientHosted: true +} +const BROWSER: BrowserWorkspace = { + id: PAGE.workspaceId, + worktreeId: WORKTREE, + sessionProfileId: null, + activePageId: PAGE.id, + pageIds: [PAGE.id], + url: PAGE.url, + title: PAGE.title, + loading: false, + faviconUrl: null, + canGoBack: true, + canGoForward: false, + loadError: null, + createdAt: 1 +} + +function browserSession() { + return { + ...getDefaultWorkspaceSession(), + browserTabsByWorktree: { [WORKTREE]: [BROWSER] }, + browserPagesByWorkspace: { [BROWSER.id]: [PAGE] }, + activeBrowserTabIdByWorktree: { [WORKTREE]: BROWSER.id } + } +} + +describe('remote session startup ownership', () => { + beforeEach(() => { + testState.dir = mkdtempSync(join(tmpdir(), 'orca-remote-session-')) + }) + afterEach(() => { + rmSync(testState.dir, { recursive: true, force: true }) + }) + + it('keeps a paired browser row and its hosting identity across two Store reloads', () => { + const seed = createStore() + seed.addRepo(makeRepo({ id: 'local-repo', path: join(testState.dir, 'local') })) + seed.setWorkspaceSession(browserSession(), HOST) + seed.flush() + + for (let i = 0; i < 2; i += 1) { + const reloaded = createStore() + expect(reloaded.getWorkspaceSession(HOST).browserPagesByWorkspace).toEqual({ + [BROWSER.id]: [PAGE] + }) + expect(reloaded.sweepDeregisteredRepoResidue()).toEqual([]) + reloaded.flush() + } + }) + + it('retains remote metadata when no session or local catalog row names its repo', () => { + const seed = createStore() + seed.setWorktreeMetaForHost(WORKTREE, HOST, { displayName: 'Remote work' }) + seed.flush() + const reloaded = createStore() + expect(reloaded.getWorktreeMeta(WORKTREE)).toMatchObject({ displayName: 'Remote work' }) + expect(reloaded.sweepDeregisteredRepoResidue()).toEqual([]) + reloaded.flush() + }) + + it('still applies an explicit remote project removal', () => { + const seed = createStore() + seed.setWorkspaceSession(browserSession(), HOST) + seed.flush() + const reloaded = createStore() + // Assert the row survived load first, or an empty partition below would prove nothing. + expect(reloaded.getWorkspaceSession(HOST).browserPagesByWorkspace).not.toEqual({}) + reloaded.removeProjectForHost(REPO, HOST) + reloaded.flush() + expect(createStore().getWorkspaceSession(HOST).browserPagesByWorkspace).toEqual({}) + }) +}) diff --git a/src/main/persistence/loading-store/repo-lifecycle-operations.ts b/src/main/persistence/loading-store/repo-lifecycle-operations.ts index 535108845e1..e5c75c1ff01 100644 --- a/src/main/persistence/loading-store/repo-lifecycle-operations.ts +++ b/src/main/persistence/loading-store/repo-lifecycle-operations.ts @@ -136,11 +136,9 @@ export class RepoLifecycleOperations { /** * Drop every persisted row owned by a repo id that is no longer registered. * - * Runs at load because no removal path can: `removeProject` only fires while the repo is still in - * `state.repos`, and a paired client's mirror of a remote host's rows is keyed by ids that client - * never registers, so the owning host's removal never reaches it (#17776). An orphan has no owner - * that could object, so this ignores the session-ownership and local-execution-host gates the - * missing-directory sweeper needs. + * Runs at load to reach leftover local rows after deregistration. Rows owned by a `runtime:*` + * host are exempt: this runs before pairing, so their absence from the local catalog cannot + * establish deletion. Only an explicit `removeProjectForHost` retires them. */ sweepDeregisteredRepoResidue(): string[] { const state = this[repoLifecycleOperationsContext].runtime.state diff --git a/src/main/persistence/restoring-sessions/session-worktree-ownership.ts b/src/main/persistence/restoring-sessions/session-worktree-ownership.ts index e06e39dc5c5..5c54af92975 100644 --- a/src/main/persistence/restoring-sessions/session-worktree-ownership.ts +++ b/src/main/persistence/restoring-sessions/session-worktree-ownership.ts @@ -209,7 +209,7 @@ export function collectWorkspaceSessionWorktreeOwners( return owners } -function addWorkspaceSessionWorktreeOwners( +export function addWorkspaceSessionWorktreeOwners( session: WorkspaceSessionState, collector: WorktreeOwnerCandidateCollector ): void { diff --git a/src/main/persistence/tracking-repos/deregistered-repo-residue.ts b/src/main/persistence/tracking-repos/deregistered-repo-residue.ts index c52bddbb712..cf97adc11ae 100644 --- a/src/main/persistence/tracking-repos/deregistered-repo-residue.ts +++ b/src/main/persistence/tracking-repos/deregistered-repo-residue.ts @@ -1,19 +1,61 @@ import type { PersistedState } from '../../../shared/persisted-state-types' -import { getWorktreeIdFromHostIdentity } from '../../../shared/worktree/host-qualified-identity' +import { + getExecutionHostIdFromWorktreeHostIdentity, + getWorktreeIdFromHostIdentity +} from '../../../shared/worktree/host-qualified-identity' +import { parseExecutionHostId } from '../../../shared/execution-host' +import { addWorkspaceSessionWorktreeOwners } from '../restoring-sessions/session-worktree-ownership' import { splitWorktreeId } from '../../../shared/worktree/id' import type { WorkspaceSessionState } from '../../../shared/workspace-session-state-types' import { SESSION_FIELDS_PRUNED_BY_OWNER_KEY } from '../../orca-profiles/profile-project-session-field-disposition' import { ownerKeyWorktreeIds } from '../../orca-profiles/profile-project-worktree-identity' +/** A `runtime:*` host addresses a paired Orca desktop's rows, whose catalog lives on that host. */ +const isPairedHost = (hostId: string | null | undefined): boolean => + parseExecutionHostId(hostId)?.kind === 'runtime' + +/** Repo ids an owner key can name, across both readings (see `ownerKeyWorktreeIds`). */ +function ownerKeyRepoIds(ownerKey: string | null | undefined): string[] { + return ownerKey + ? ownerKeyWorktreeIds(ownerKey).flatMap((worktreeId) => { + const repoId = splitWorktreeId(worktreeId)?.repoId + return repoId ? [repoId] : [] + }) + : [] +} + /** * Repo ids that still own persisted rows but no longer appear in `state.repos`. * - * Why nothing else finds them: every other sweeper is gated on the repo still being registered, so - * deregistering a project stranded the rows it owned permanently — including a paired client's - * mirror of a remote host's session partition, which no local repo removal can reach (#17776). + * Rows owned by a `runtime:*` host are held live instead of swept: a paired client mirrors that + * host's sessions without ever registering its repos, and this runs in the Store constructor, + * before pairing, so catalog absence there proves nothing (#17776 read it as proof and deleted + * live sessions). The cost is that residue outliving a removal is no longer swept for those hosts. */ export function collectDeregisteredRepoIds(state: PersistedState): Set { const liveRepoIds = new Set(state.repos.map((repo) => repo.id)) + const retainOwner = (ownerKey: string | null | undefined): void => { + for (const repoId of ownerKeyRepoIds(ownerKey)) { + liveRepoIds.add(repoId) + } + } + // `owners` goes unread: the walker only ever calls `addOwner`. + const retainCollector = { owners: new Set(), addOwner: retainOwner } + for (const [hostId, session] of Object.entries(state.workspaceSessionsByHostId ?? {})) { + if (session && isPairedHost(hostId)) { + addWorkspaceSessionWorktreeOwners(session, retainCollector) + } + } + for (const [worktreeId, meta] of Object.entries(state.worktreeMeta)) { + if (isPairedHost(meta.hostId)) { + retainOwner(worktreeId) + } + } + for (const alias of Object.keys(state.worktreeIdentityAliases ?? {})) { + if (isPairedHost(getExecutionHostIdFromWorktreeHostIdentity(alias))) { + retainOwner(getWorktreeIdFromHostIdentity(alias)) + } + } const orphanRepoIds = new Set() // Only a full `::` locator seeds the set. A bare key -- a folder workspace id, a // repo-keyed topology revision, a test-shaped locator -- cannot be told apart from a repo id, and @@ -30,10 +72,7 @@ export function collectDeregisteredRepoIds(state: PersistedState): Set { * other reading would hand the removal pass -- which accepts either -- a live row to delete. */ const addOwnerKey = (ownerKey: string): void => { - const repoIds = ownerKeyWorktreeIds(ownerKey).flatMap((worktreeId) => { - const repoId = splitWorktreeId(worktreeId)?.repoId - return repoId ? [repoId] : [] - }) + const repoIds = ownerKeyRepoIds(ownerKey) if (repoIds.length > 0 && repoIds.every((repoId) => !liveRepoIds.has(repoId))) { for (const repoId of repoIds) { orphanRepoIds.add(repoId) diff --git a/src/main/providers/agent-foreground-process.test.ts b/src/main/providers/agent-foreground-process.test.ts index fb883d2166c..91b1e992c06 100644 --- a/src/main/providers/agent-foreground-process.test.ts +++ b/src/main/providers/agent-foreground-process.test.ts @@ -221,13 +221,13 @@ describe('resolveAgentForegroundProcess', () => { }) it('confirms a quoted login shell only when its fresh PTY tree contains shells', async () => { - mockPs(['100 99 Ss+ "/bin/zsh" -l', '101 100 S+ /bin/bash'].join('\n')) + mockPs(['100 99 100 100 Ss+ "/bin/zsh" -l', '101 100 101 100 S+ /bin/bash'].join('\n')) await expect(confirmShellForegroundProcess(100, 'zsh')).resolves.toBe(true) }) it('uses spawned-shell identity instead of a lagging foreground child label', async () => { - mockPs(['100 99 Ss+ /bin/zsh -l'].join('\n')) + mockPs(['100 99 100 100 Ss+ /bin/zsh -l'].join('\n')) await expect(confirmShellForegroundProcess(100, '/bin/zsh')).resolves.toBe(true) }) @@ -235,11 +235,11 @@ describe('resolveAgentForegroundProcess', () => { it('confirms the spawned shell behind a login wrapper while prompt hooks run', async () => { mockPs( [ - '100 99 Ss /usr/bin/login -pfl developer /bin/zsh', - '101 100 S+ -zsh', - '102 101 S+ (zsh)', - '103 102 S+ (sed)', - '104 102 R+ (git)' + '100 99 100 101 Ss /usr/bin/login -pfl developer /bin/zsh', + '101 100 101 101 S+ -zsh', + '102 101 101 101 S+ (zsh)', + '103 102 101 101 S+ (sed)', + '104 102 101 101 R+ (git)' ].join('\n') ) @@ -249,10 +249,10 @@ describe('resolveAgentForegroundProcess', () => { it('rejects a foreground nested shell while the spawned shell remains suspended', async () => { mockPs( [ - '100 99 Ss /usr/bin/login -pfl developer /bin/zsh', - '101 100 S -zsh', - '102 101 S+ agent-tui', - '103 102 S+ /bin/zsh -i' + '100 99 100 102 Ss /usr/bin/login -pfl developer /bin/zsh', + '101 100 101 102 S -zsh', + '102 101 102 102 S+ agent-tui', + '103 102 102 102 S+ /bin/zsh -i' ].join('\n') ) @@ -262,9 +262,9 @@ describe('resolveAgentForegroundProcess', () => { it('rejects shell ownership while a TUI and its nested shell remain in the PTY tree', async () => { mockPs( [ - '100 99 Ss /bin/zsh -l', - '101 100 S+ /usr/local/bin/agent-tui', - '102 101 S+ /bin/bash -i' + '100 99 100 101 Ss /bin/zsh -l', + '101 100 101 101 S+ /usr/local/bin/agent-tui', + '102 101 101 101 S+ /bin/bash -i' ].join('\n') ) @@ -272,7 +272,7 @@ describe('resolveAgentForegroundProcess', () => { }) it('rejects shell ownership while a stopped TUI remains resumable', async () => { - mockPs(['100 99 Ss+ /bin/zsh -l', '101 100 T /usr/local/bin/agent-tui'].join('\n')) + mockPs(['100 99 100 100 Ss+ /bin/zsh -l', '101 100 101 100 T agent-tui'].join('\n')) await expect(confirmShellForegroundProcess(100, 'zsh')).resolves.toBe(false) }) diff --git a/src/main/providers/agent-foreground-process.ts b/src/main/providers/agent-foreground-process.ts index 7fface8941e..69276098369 100644 --- a/src/main/providers/agent-foreground-process.ts +++ b/src/main/providers/agent-foreground-process.ts @@ -3,6 +3,7 @@ import { resolveOuterWrapperForegroundProcess } from '../../shared/foreground-wr import type { ProcessTableRow } from '../../shared/process-table-snapshot' import { getFreshProcessTableSnapshot, + getFreshShellForegroundSnapshot, getProcessTableSnapshot } from '../../shared/process-table-snapshot-reader' import { collectDescendantsFromIndex, getProcessTableIndex } from '../../shared/process-table-index' @@ -76,7 +77,7 @@ export async function confirmShellForegroundProcess( } } try { - const index = getProcessTableIndex(await getFreshProcessTableSnapshot()) + const index = getProcessTableIndex(await getFreshShellForegroundSnapshot()) const root = index.byPid.get(shellPid) if (!root) { return false diff --git a/src/main/runtime/browser-session-tab-selection-snapshot.test.ts b/src/main/runtime/browser-session-tab-selection-snapshot.test.ts index a96a5617d3b..f553650323c 100644 --- a/src/main/runtime/browser-session-tab-selection-snapshot.test.ts +++ b/src/main/runtime/browser-session-tab-selection-snapshot.test.ts @@ -2,8 +2,6 @@ import { describe, expect, it } from 'vitest' import type { RuntimeMobileSessionTabsSnapshot } from '../../shared/runtime-types' import { applyBrowserSessionTabSelection } from './browser-session-tab-selection-snapshot' -const EPOCH = 'headless:test' - function makeSnapshot(): RuntimeMobileSessionTabsSnapshot { return { worktree: 'wt-1', @@ -46,8 +44,7 @@ function select(overrides: { focusesHost: boolean; targetGroupId?: string }) { snapshot: makeSnapshot(), tabId: 'page-new', focusesHost: overrides.focusesHost, - ...(overrides.targetGroupId ? { targetGroupId: overrides.targetGroupId } : {}), - publicationEpoch: EPOCH + ...(overrides.targetGroupId ? { targetGroupId: overrides.targetGroupId } : {}) }) } @@ -115,10 +112,11 @@ describe('applyBrowserSessionTabSelection', () => { expect(snapshot.activeGroupId).toBe('group-left') }) - it('republishes under a fresh epoch and a newer version either way', () => { + // Rotating the epoch here retires the renderer's own publication client-side. + it('keeps the publication epoch and advances the version either way', () => { for (const focusesHost of [true, false]) { const { snapshot } = select({ focusesHost }) - expect(snapshot.publicationEpoch).toBe(EPOCH) + expect(snapshot.publicationEpoch).toBe('headless:before') expect(snapshot.snapshotVersion).toBe(5) } }) diff --git a/src/main/runtime/browser-session-tab-selection-snapshot.ts b/src/main/runtime/browser-session-tab-selection-snapshot.ts index aa95c9f2685..e5862e1926d 100644 --- a/src/main/runtime/browser-session-tab-selection-snapshot.ts +++ b/src/main/runtime/browser-session-tab-selection-snapshot.ts @@ -22,7 +22,6 @@ export function applyBrowserSessionTabSelection(args: { tabId: string targetGroupId?: string focusesHost: boolean - publicationEpoch: string }): BrowserSessionTabSelectionResult { const { snapshot, tabId, targetGroupId, focusesHost } = args const groups = snapshot.tabGroups ?? [] @@ -56,7 +55,6 @@ export function applyBrowserSessionTabSelection(args: { placedInTargetGroup, snapshot: { ...snapshot, - publicationEpoch: args.publicationEpoch, snapshotVersion: snapshot.snapshotVersion + 1, ...(placedInTargetGroup && focusesHost ? { activeGroupId: targetGroupId } : {}), ...(focusesHost diff --git a/src/main/runtime/hidden-output-restored-provider-snapshot.test.ts b/src/main/runtime/hidden-output-restored-provider-snapshot.test.ts new file mode 100644 index 00000000000..bcca4491fca --- /dev/null +++ b/src/main/runtime/hidden-output-restored-provider-snapshot.test.ts @@ -0,0 +1,79 @@ +import { describe, expect, it, vi } from 'vitest' +import { createRuntime, syncSinglePty } from './orca-runtime-test-fixtures.spec' + +describe('hidden-output recovery after provider reattach', () => { + it('uses retained provider modes instead of the pre-attach redraw suffix', async () => { + const runtime = createRuntime() + const serializeProviderBuffer = vi.fn(async () => ({ + data: '\x1b[?1049hRetained TUI', + cols: 100, + rows: 30, + seq: 1000, + source: 'headless' as const, + alternateScreen: true + })) + runtime.setPtyController({ + write: () => true, + kill: () => true, + getForegroundProcess: async () => null, + serializeProviderBuffer + }) + syncSinglePty(runtime, 'pty-1') + runtime.onPtyData('pty-1', '\x1b[HRedraw without the original alternate-screen entry', 60) + runtime.synchronizePtyOutputSequenceFromProvider('pty-1', { + value: 1000, + generation: 'continued' + }) + + const snapshot = await runtime.serializeHiddenOutputRecoveryBuffer('pty-1', { + scrollbackRows: 5000 + }) + + expect(snapshot).toMatchObject({ data: '\x1b[?1049hRetained TUI', alternateScreen: true }) + expect(serializeProviderBuffer).toHaveBeenCalledWith('pty-1', { scrollbackRows: 5000 }) + }) + + it('keeps the renderer fallback for providers without retained snapshots', async () => { + const runtime = createRuntime() + runtime.onPtyData('pty-1', 'partial redraw', 14) + runtime.synchronizePtyOutputSequenceFromProvider('pty-1', { + value: 1000, + generation: 'continued' + }) + const serializeBuffer = vi.fn(async () => ({ + data: '\x1b[?1049hRenderer TUI', + cols: 100, + rows: 30 + })) + runtime.setPtyController({ + write: () => true, + kill: () => true, + getForegroundProcess: async () => null, + hasRendererSerializer: () => true, + serializeBuffer + }) + + await expect(runtime.serializeHiddenOutputRecoveryBuffer('pty-1')).resolves.toMatchObject({ + data: '\x1b[?1049hRenderer TUI', + source: 'renderer' + }) + }) + + it('keeps an authoritative main model without polling the provider', async () => { + const runtime = createRuntime() + const serializeProviderBuffer = vi.fn(async () => null) + runtime.setPtyController({ + write: () => true, + kill: () => true, + getForegroundProcess: async () => null, + serializeProviderBuffer + }) + runtime.onPtyData('pty-1', '\x1b[?1049hLive TUI', 20) + + await expect(runtime.serializeHiddenOutputRecoveryBuffer('pty-1')).resolves.toMatchObject({ + alternateScreen: true, + source: 'headless' + }) + expect(serializeProviderBuffer).not.toHaveBeenCalled() + }) +}) diff --git a/src/main/runtime/orca-runtime-close-structured-agent-session-tab.ts b/src/main/runtime/orca-runtime-close-structured-agent-session-tab.ts index bd282b6575d..ba762ef17d8 100644 --- a/src/main/runtime/orca-runtime-close-structured-agent-session-tab.ts +++ b/src/main/runtime/orca-runtime-close-structured-agent-session-tab.ts @@ -206,8 +206,7 @@ export class OrcaRuntimeWithCloseStructuredAgentSessionTab extends OrcaRuntimeWi snapshot, tabId: tab.id, ...(targetGroupId !== undefined ? { targetGroupId } : {}), - focusesHost, - publicationEpoch: `headless:${Date.now().toString(36)}` + focusesHost }) this.storeMobileSessionSnapshot(worktreeId, nextSnapshot) // Why: browser group membership is otherwise live-only; persist it so a diff --git a/src/main/runtime/orca-runtime-create-runtime-owned-mobile-session-terminal.ts b/src/main/runtime/orca-runtime-create-runtime-owned-mobile-session-terminal.ts index dd74cfbfb7a..31fb8a90ab0 100644 --- a/src/main/runtime/orca-runtime-create-runtime-owned-mobile-session-terminal.ts +++ b/src/main/runtime/orca-runtime-create-runtime-owned-mobile-session-terminal.ts @@ -120,7 +120,8 @@ export class OrcaRuntimeWithCreateRuntimeOwnedMobileSessionTerminal extends Orca } const next: RuntimeMobileSessionTabsSnapshot = { worktree: worktreeId, - publicationEpoch: `headless:${Date.now().toString(36)}`, + // Why: a fresh epoch retires the current publisher, so clients drop its later tab updates. + publicationEpoch: existing?.publicationEpoch ?? `headless:${Date.now().toString(36)}`, snapshotVersion: (existing?.snapshotVersion ?? 0) + 1, // Why: activating the new tab also focuses its group, so a "+" targeting a specific split group makes that group active too. activeGroupId: diff --git a/src/main/runtime/orca-runtime-reconcile-headless-mobile-session-browser-tabs.ts b/src/main/runtime/orca-runtime-reconcile-headless-mobile-session-browser-tabs.ts index a9f377e5a7b..ddb74df4dc8 100644 --- a/src/main/runtime/orca-runtime-reconcile-headless-mobile-session-browser-tabs.ts +++ b/src/main/runtime/orca-runtime-reconcile-headless-mobile-session-browser-tabs.ts @@ -25,11 +25,28 @@ export class OrcaRuntimeWithReconcileHeadlessMobileSessionBrowserTabs extends Or worktreeId: string, existing: RuntimeMobileSessionTabsSnapshot ): void { - const liveBrowserTabs = this.buildHeadlessMobileSessionBrowserTabs(worktreeId) - const liveIds = liveBrowserTabs.map((tab) => tab.id) const existingBrowserTabs = existing.tabs.filter( (tab): tab is RuntimeMobileSessionBrowserTab => tab.type === 'browser' ) + const publishedBrowserTabs = this.buildHeadlessMobileSessionBrowserTabs(worktreeId) + // An attached renderer owns its browser rows; the client-page registry cannot retire them. + const rendererBrowserTabs = + this.getAvailableAuthoritativeWindow() && !this.offscreenBrowserBackend + ? existingBrowserTabs.filter((tab) => tab.placement?.kind !== 'client') + : [] + // Keyed by id so no row can publish twice whatever the two sources overlap on; a freshly + // built row wins over the retained one it replaces. + const liveById = new Map( + [...rendererBrowserTabs, ...publishedBrowserTabs].map((tab) => [tab.id, tab]) + ) + // Emit in the order the snapshot already had, because the equality check below compares by + // index: rebuilding renderer-first would read a pure reordering as a change and republish. + const retainedInOrder = existingBrowserTabs.flatMap((tab) => { + const live = liveById.get(tab.id) + return live && liveById.delete(tab.id) ? [live] : [] + }) + const liveBrowserTabs = [...retainedInOrder, ...liveById.values()] + const liveIds = liveBrowserTabs.map((tab) => tab.id) const existingBrowserIds = existingBrowserTabs.map((tab) => tab.id) if (headlessBrowserTabsUnchanged(liveBrowserTabs, existingBrowserTabs)) { return @@ -53,7 +70,6 @@ export class OrcaRuntimeWithReconcileHeadlessMobileSessionBrowserTabs extends Or : (nextTabs.find((tab) => tab.isActive) ?? nextTabs[0] ?? null) this.storeMobileSessionSnapshot(worktreeId, { ...existing, - publicationEpoch: `headless-hydrated:${Date.now().toString(36)}`, snapshotVersion: existing.snapshotVersion + 1, ...(activeStillPresent ? {} diff --git a/src/main/runtime/orca-runtime-serialize-main-terminal-buffer.ts b/src/main/runtime/orca-runtime-serialize-main-terminal-buffer.ts index ed777c1f7d1..6559dbfd349 100644 --- a/src/main/runtime/orca-runtime-serialize-main-terminal-buffer.ts +++ b/src/main/runtime/orca-runtime-serialize-main-terminal-buffer.ts @@ -47,6 +47,10 @@ export class OrcaRuntimeWithSerializeMainTerminalBuffer extends OrcaRuntimeWithA pendingEscapeTailAnsi?: string terminalOwner?: 'shell' } | null> { + const restoredSnapshot = await this.serializePreferredRestoredTerminalBuffer(ptyId, opts) + if (restoredSnapshot) { + return restoredSnapshot + } const headlessSnapshot = await this.serializeHeadlessTerminalBuffer(ptyId, { ...opts, includeEmpty: true diff --git a/src/main/runtime/orca-runtime-serialize-terminal-buffer-from-available-state.ts b/src/main/runtime/orca-runtime-serialize-terminal-buffer-from-available-state.ts index e031dc1b6f5..8969841359b 100644 --- a/src/main/runtime/orca-runtime-serialize-terminal-buffer-from-available-state.ts +++ b/src/main/runtime/orca-runtime-serialize-terminal-buffer-from-available-state.ts @@ -23,18 +23,9 @@ export class OrcaRuntimeWithSerializeTerminalBufferFromAvailableState extends Or kittyKeyboardFlags?: number terminalOwner?: 'shell' } | null> { - if (this.providerSnapshotPreferredPtys.has(ptyId)) { - // Why: pre-attach stream bytes only form a suffix of restored state. A - // sequenced provider snapshot safely reconciles live bytes; renderer is - // the fallback when an older provider cannot expose that boundary. - const providerSnapshot = await this.serializeProviderTerminalBuffer(ptyId, opts) - if (providerSnapshot) { - return providerSnapshot - } - const rendererSnapshot = await this.serializeRendererTerminalBuffer(ptyId, opts) - if (rendererSnapshot) { - return rendererSnapshot - } + const restoredSnapshot = await this.serializePreferredRestoredTerminalBuffer(ptyId, opts) + if (restoredSnapshot) { + return restoredSnapshot } const headlessSnapshot = await this.serializeHeadlessTerminalBuffer(ptyId, opts) if (headlessSnapshot) { @@ -58,6 +49,20 @@ export class OrcaRuntimeWithSerializeTerminalBufferFromAvailableState extends Or : rendererSnapshot } + protected async serializePreferredRestoredTerminalBuffer( + ptyId: string, + opts: { scrollbackRows?: number } = {} + ) { + if (!this.providerSnapshotPreferredPtys.has(ptyId)) { + return null + } + // Pre-attach bytes are only a suffix; older providers can fall back to the renderer. + return ( + (await this.serializeProviderTerminalBuffer(ptyId, opts)) ?? + (await this.serializeRendererTerminalBuffer(ptyId, opts)) + ) + } + async serializeRendererTerminalBuffer( ptyId: string, opts: { scrollbackRows?: number } = {} diff --git a/src/main/runtime/renderer-browser-session-reconciliation.test.ts b/src/main/runtime/renderer-browser-session-reconciliation.test.ts new file mode 100644 index 00000000000..7dc929c231d --- /dev/null +++ b/src/main/runtime/renderer-browser-session-reconciliation.test.ts @@ -0,0 +1,154 @@ +import { expect, it, vi } from 'vitest' +import type { + RuntimeMobileSessionBrowserTab, + RuntimeMobileSessionTabsSnapshot +} from '../../shared/runtime-types' +import { OrcaRuntimeWithCloseStructuredAgentSessionTab } from './orca-runtime-close-structured-agent-session-tab' +import { OrcaRuntimeWithReconcileHeadlessMobileSessionBrowserTabs } from './orca-runtime-reconcile-headless-mobile-session-browser-tabs' + +const rendererPage: RuntimeMobileSessionBrowserTab = { + type: 'browser', + id: 'renderer-tab', + browserWorkspaceId: 'renderer-workspace', + browserPageId: 'renderer-page', + title: 'Server page', + url: 'https://example.com/server', + loading: false, + canGoBack: false, + canGoForward: false, + isActive: false +} +const clientPage: RuntimeMobileSessionBrowserTab = { + ...rendererPage, + id: 'client', + browserWorkspaceId: 'client', + browserPageId: 'client', + placement: { + kind: 'client', + browserHostClientId: 'host', + browserHostGeneration: 1, + pageHostGeneration: 1 + } +} +const snapshot: RuntimeMobileSessionTabsSnapshot = { + worktree: 'wt', + publicationEpoch: 'renderer:1', + snapshotVersion: 1, + activeGroupId: 'group', + activeTabId: 'renderer-tab', + activeTabType: 'browser', + tabs: [rendererPage], + tabGroups: [{ id: 'group', activeTabId: 'renderer-tab', tabOrder: ['renderer-tab'] }] +} + +/** Drives the reconcile against a stub host and returns the published snapshot, if any. */ +function reconcile( + host: { + live?: RuntimeMobileSessionBrowserTab[] + attached?: boolean + offscreen?: boolean + }, + existing: RuntimeMobileSessionTabsSnapshot = snapshot +): RuntimeMobileSessionTabsSnapshot | undefined { + const storeMobileSessionSnapshot = vi.fn() + const runtime = OrcaRuntimeWithReconcileHeadlessMobileSessionBrowserTabs.prototype as unknown as { + reconcileHeadlessMobileSessionBrowserTabs( + worktreeId: string, + existing: RuntimeMobileSessionTabsSnapshot + ): void + } + runtime.reconcileHeadlessMobileSessionBrowserTabs.call( + { + buildHeadlessMobileSessionBrowserTabs: () => host.live ?? [], + getAvailableAuthoritativeWindow: () => (host.attached === false ? null : {}), + offscreenBrowserBackend: host.offscreen === true ? {} : null, + storeMobileSessionSnapshot + }, + 'wt', + existing + ) + return storeMobileSessionSnapshot.mock.calls[0]?.[1] +} + +it('keeps renderer-owned browser pages when refreshing client-hosted pages on an attached desktop', () => { + const published = reconcile({}) ?? snapshot + + expect(published.tabs).toContainEqual(rendererPage) + expect(published.tabGroups?.[0].tabOrder).toContain('renderer-tab') +}) + +it.each([false, true])('retires absent offscreen pages when attached=%s', (attached) => { + expect(reconcile({ attached, offscreen: true })?.tabs).toEqual([]) +}) + +it('removes retired client pages and publishes live ones while retaining renderer rows and group order', () => { + const livePage = { ...clientPage, id: 'live', browserWorkspaceId: 'live', browserPageId: 'live' } + + const published = reconcile( + { live: [livePage] }, + { + ...snapshot, + tabs: [rendererPage, clientPage], + tabGroups: [ + { id: 'group', activeTabId: 'renderer-tab', tabOrder: ['renderer-tab', 'client'] } + ] + } + ) + + expect(published?.tabs).toEqual([rendererPage, livePage]) + expect(published?.tabGroups?.[0].tabOrder).toEqual(['renderer-tab', 'live']) + expect(published?.activeTabId).toBe('renderer-tab') + expect(published?.publicationEpoch).toBe(snapshot.publicationEpoch) + expect(published?.snapshotVersion).toBe(snapshot.snapshotVersion + 1) +}) + +it('never publishes a row twice when the live build reclaims a renderer-owned id', () => { + const reclaimed = { + ...clientPage, + id: rendererPage.id, + browserPageId: rendererPage.browserPageId + } + + const published = reconcile({ live: [reclaimed] }) + + expect(published?.tabs).toEqual([reclaimed]) + expect(published?.tabGroups?.[0].tabOrder).toEqual([rendererPage.id]) +}) + +it('does not republish when a client row merely sits before a renderer row', () => { + const interleaved = { + ...snapshot, + tabs: [clientPage, rendererPage], + tabGroups: [{ id: 'group', activeTabId: 'renderer-tab', tabOrder: ['client', 'renderer-tab'] }] + } + + expect(reconcile({ live: [clientPage] }, interleaved)).toBeUndefined() +}) + +it('keeps the renderer publication epoch when selecting a client-hosted browser tab', () => { + const storeMobileSessionSnapshot = vi.fn() + const runtime = OrcaRuntimeWithCloseStructuredAgentSessionTab.prototype as unknown as { + markHeadlessBrowserSessionTabActive( + worktreeId: string, + browserPageId: string, + options: { focusesHost: boolean } + ): void + } + + runtime.markHeadlessBrowserSessionTabActive.call( + { + offscreenBrowserBackend: {}, + hydrateHeadlessMobileSessionTabsFromWorkspaceSession: () => undefined, + mobileSessionTabsByWorktree: new Map([['wt', snapshot]]), + storeMobileSessionSnapshot, + emitMobileSessionTabsSnapshot: vi.fn() + }, + 'wt', + 'renderer-page', + { focusesHost: false } + ) + + expect(storeMobileSessionSnapshot.mock.calls[0]?.[1].publicationEpoch).toBe( + snapshot.publicationEpoch + ) +}) diff --git a/src/main/runtime/runtime-owned-terminal-publication-lineage.test.ts b/src/main/runtime/runtime-owned-terminal-publication-lineage.test.ts new file mode 100644 index 00000000000..2df5abf00da --- /dev/null +++ b/src/main/runtime/runtime-owned-terminal-publication-lineage.test.ts @@ -0,0 +1,57 @@ +import { expect, it, vi } from 'vitest' +import type { RuntimeMobileSessionTabsResult } from '../../shared/runtime-types' + +// Fragments stay side-effect ordered: mocks, then lifecycle, then fixtures. +const { OrcaRuntimeService } = await import('./orca-runtime-test-mocks.spec') +await import('./orca-runtime-test-lifecycle.spec') +const { store, TEST_WORKTREE_ID } = await import('./orca-runtime-test-fixtures.spec') + +it.each(['renderer:active-generation', 'headless:active-generation'])( + 'keeps %s live when runtime-owned creation supplements its inventory', + async (publicationEpoch) => { + const runtime = new OrcaRuntimeService(store) + runtime.setPtyController({ + spawn: vi.fn().mockResolvedValue({ id: 'pty-runtime-fallback' }), + write: () => true, + kill: () => true, + getForegroundProcess: async () => null + }) + runtime.syncWindowGraph(0, { + tabs: [], + leaves: [], + mobileSessionTabs: [ + { + worktree: TEST_WORKTREE_ID, + publicationEpoch, + snapshotVersion: 7, + activeGroupId: null, + activeTabId: null, + activeTabType: null, + tabs: [] + } + ] + }) + const events: RuntimeMobileSessionTabsResult[] = [] + const unsubscribe = runtime.onMobileSessionTabsChanged( + (snapshot) => events.push(snapshot), + 'paired-client' + ) + try { + const created = await runtime.createMobileSessionTerminal(`id:${TEST_WORKTREE_ID}`, { + activate: false, + select: false, + navigation: 'caller', + clientNavigationId: 'paired-client' + }) + expect(created.tab.status).toBe('ready') + expect(created.publicationEpoch).toBe(publicationEpoch) + expect(created.snapshotVersion).toBeGreaterThan(7) + expect(events.at(-1)).toMatchObject({ + publicationEpoch: `${publicationEpoch}:client-navigation`, + tabs: [expect.objectContaining({ id: created.tab.id, status: 'ready' })] + }) + } finally { + unsubscribe() + } + } +) diff --git a/src/main/wsl-availability-missing-kernel.test.ts b/src/main/wsl-availability-missing-kernel.test.ts new file mode 100644 index 00000000000..3ab7b6d4a26 --- /dev/null +++ b/src/main/wsl-availability-missing-kernel.test.ts @@ -0,0 +1,98 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import { execFile, execFileSync } from 'node:child_process' +import { runProcess, runProcessSync, type ProcessResult } from '../shared/child-process/run-process' +import { + _resetWslAvailabilityCacheForTests, + isWslAvailable, + isWslAvailableAsync +} from './wsl-availability' + +vi.mock('node:child_process', () => ({ execFile: vi.fn(), execFileSync: vi.fn() })) +vi.mock('../shared/child-process/run-process', () => ({ + runProcess: vi.fn(), + runProcessSync: vi.fn() +})) +vi.mock('./wsl-interop-spawn-directory', () => ({ + resolveWslInteropSpawnCwd: () => 'C:\\Windows' +})) + +const originalPlatform = process.platform +const success: ProcessResult = { code: 0, signal: null, stdout: '', stderr: '', timedOut: false } + +beforeEach(() => { + vi.resetAllMocks() + Object.defineProperty(process, 'platform', { value: 'win32' }) + _resetWslAvailabilityCacheForTests() +}) +afterEach(() => { + Object.defineProperty(process, 'platform', { value: originalPlatform }) + _resetWslAvailabilityCacheForTests() +}) + +for (const mode of ['sync', 'async'] as const) { + describe(`${mode} WSL1 availability without WSL2 kernel`, () => { + const probe = () => (mode === 'sync' ? isWslAvailable() : isWslAvailableAsync()) + const guestRunner = () => (mode === 'sync' ? runProcessSync : runProcess) + + function failStatus(code: number): void { + vi.mocked(execFileSync).mockImplementation(() => { + throw { status: code } + }) + vi.mocked(execFile).mockImplementation((...args: unknown[]) => { + const callback = args.at(-1) as (error: unknown) => void + callback({ code }) + return {} as ReturnType + }) + } + function guestResult(result: ProcessResult): void { + vi.mocked(runProcess).mockResolvedValue(result) + vi.mocked(runProcessSync).mockReturnValue(result) + } + + // Node reports the Windows DWORD; the console prints its signed equivalent. + for (const status of [-444, 4_294_966_852]) { + it(`requires guest execution and caches its success for ${status}`, async () => { + failStatus(status) + guestResult(success) + expect(await probe()).toBe(true) + expect(await probe()).toBe(true) + expect(guestRunner()).toHaveBeenCalledTimes(1) + expect(guestRunner()).toHaveBeenCalledWith( + expect.objectContaining({ + program: 'wsl.exe', + args: ['--exec', '/bin/true'], + timeoutMs: 5000, + cwd: 'C:\\Windows' + }) + ) + }) + } + + for (const result of [ + { ...success, code: 1 }, + { ...success, code: null, timedOut: true } + ]) { + it(`keeps a failed guest unavailable: ${JSON.stringify(result)}`, async () => { + failStatus(-444) + guestResult(result) + expect(await probe()).toBe(false) + }) + } + + it('stays unavailable when the guest probe cannot be spawned', async () => { + failStatus(-444) + vi.mocked(runProcess).mockRejectedValue(new Error('EPERM')) + vi.mocked(runProcessSync).mockImplementation(() => { + throw new Error('EPERM') + }) + expect(await probe()).toBe(false) + }) + + it('does not probe a guest for unrelated status failures', async () => { + failStatus(1) + expect(await probe()).toBe(false) + expect(runProcess).not.toHaveBeenCalled() + expect(runProcessSync).not.toHaveBeenCalled() + }) + }) +} diff --git a/src/main/wsl-availability.ts b/src/main/wsl-availability.ts index 1d14f526413..ad5e5645c30 100644 --- a/src/main/wsl-availability.ts +++ b/src/main/wsl-availability.ts @@ -1,4 +1,6 @@ import { execFile, execFileSync } from 'node:child_process' +import { runProcess, runProcessSync, type ProcessSpec } from '../shared/child-process/run-process' +import { buildWslExecArgs } from '../shared/wsl-login-shell-command' import { resolveWslInteropSpawnCwd } from './wsl-interop-spawn-directory' type WslAvailabilityCache = @@ -94,6 +96,55 @@ function cacheWslAvailabilityProbeResult(error: unknown, startedAtGeneration: nu return !error } +// `wsl --status` exits 0x1bc when the WSL2 kernel package is missing -- a package +// a WSL1 distro never needed. Node keeps the Windows DWORD; the console prints the +// signed form, and either spelling can reach us. +function isMissingWsl2KernelStatus(error: unknown): boolean { + const failure = error as { status?: unknown; code?: unknown } | null + return [failure?.status, failure?.code].some((code) => code === -444 || code === 4_294_966_852) +} + +// Cheapest proof the default guest runs: no login shell, no output to parse. +function defaultGuestExecutionProbe(): ProcessSpec { + return { + program: 'wsl.exe', + args: buildWslExecArgs(undefined, ['/bin/true']), + cwd: resolveWslInteropSpawnCwd(), + timeoutMs: WSL_AVAILABILITY_PROBE_TIMEOUT_MS, + maxOutputBytes: 4096 + } +} + +/** + * The `--status` error still worth caching, or null once the guest ran anyway. + * + * Why it returns that error rather than a fresh negative: a guest probe that could not + * spawn means "could not ask", and minting an answer for that is the bug this subsystem + * keeps re-shipping (docs/reference/wsl-probe-failure-semantics.md). + */ +function wslStatusErrorAfterGuestProbe(error: unknown): unknown { + if (!isMissingWsl2KernelStatus(error)) { + return error + } + try { + return runProcessSync(defaultGuestExecutionProbe()).code === 0 ? null : error + } catch { + return error + } +} + +/** Async twin of `wslStatusErrorAfterGuestProbe`; the sync/async pair share one cache. */ +async function wslStatusErrorAfterGuestProbeAsync(error: unknown): Promise { + if (!isMissingWsl2KernelStatus(error)) { + return error + } + try { + return (await runProcess(defaultGuestExecutionProbe())).code === 0 ? null : error + } catch { + return error + } +} + function probeWslStatus(): Promise { return new Promise((resolve, reject) => { execFile( @@ -147,7 +198,10 @@ export function isWslAvailable(): boolean { }) return cacheWslAvailabilityProbeResult(null, startedAtGeneration) } catch (error) { - return cacheWslAvailabilityProbeResult(error, startedAtGeneration) + return cacheWslAvailabilityProbeResult( + wslStatusErrorAfterGuestProbe(error), + startedAtGeneration + ) } } @@ -176,7 +230,12 @@ export function isWslAvailableAsync(): Promise { const startedAtGeneration = wslAvailabilityCacheGeneration wslAvailabilityProbeInFlight = probeWslStatus() .then(() => cacheWslAvailabilityProbeResult(null, startedAtGeneration)) - .catch((error: unknown) => cacheWslAvailabilityProbeResult(error, startedAtGeneration)) + .catch(async (error: unknown) => + cacheWslAvailabilityProbeResult( + await wslStatusErrorAfterGuestProbeAsync(error), + startedAtGeneration + ) + ) .finally(() => { wslAvailabilityProbeInFlight = null }) diff --git a/src/relay/relay-pty-source-activation.ts b/src/relay/relay-pty-source-activation.ts index d4ab4e06726..3898dda6df1 100644 --- a/src/relay/relay-pty-source-activation.ts +++ b/src/relay/relay-pty-source-activation.ts @@ -4,7 +4,10 @@ import type { PtySourceRecoveryResult } from '../shared/pty-source-recovery-contract' import type { PtySourceReceivingActivation } from '../shared/pty-source-receiving-activation' -import type { PtySourceDeliveryIdentity } from '../shared/pty-source-credit-contract' +import type { + PtySourceDeliveryIdentity, + PtySourceDeliverySnapshot +} from '../shared/pty-source-credit-contract' import type { RequestContext } from './dispatcher' import type { RelayPtySourceDeliveryRecord, @@ -12,6 +15,16 @@ import type { } from './relay-pty-source-send-scheduler' import type { SshPtyConsumerSessionAdapter } from './ssh-pty-consumer-session-adapter' +// Takes the post-rotation snapshot: creditedEndSu is the accepted checkpoint and windowSu the +// reconnecting client's window. The pre-rotation snapshot would fence below the checkpoint. +export function boundedPtyRecoveryEnd( + snapshot: Pick +): number { + const { receivedEndSu, creditedEndSu, windowSu } = snapshot + // Oversized quarantine cannot earn credit; fence at the checkpoint and drain it live. + return receivedEndSu - creditedEndSu > windowSu ? creditedEndSu : receivedEndSu +} + export function createPtySourceReceivingActivation( identity: PtySourceDeliveryIdentity, checkpointSourceEndSu: number, diff --git a/src/relay/relay-pty-source-publication.ts b/src/relay/relay-pty-source-publication.ts index 16b6e81c61b..c1959fd24d1 100644 --- a/src/relay/relay-pty-source-publication.ts +++ b/src/relay/relay-pty-source-publication.ts @@ -7,6 +7,7 @@ import type { PtySourceReceivingActivation } from '../shared/pty-source-receivin import { createPtySourceReceivingActivation, pendingPtySourceRecoveryResult, + boundedPtyRecoveryEnd, registerCanceledPtySourceRetirement, registerPtySourceActivationSettlement, samePtySourceRecoveryRequest @@ -66,9 +67,7 @@ export class RelayPtySourcePublication { recovery?: PtySourceRecoveryRequest ): false | 'opened' | 'rotated' | 'existing' | PtySourceRecoveryResult { let current = this.deliveries.get(id) - // A superseded request can find the delivery its own replacement opened: releasing that fence - // resumes a send the replacement is still rotating, and cancelling it blanks the pane that owns - // it. So every bail-out below acts only on a record this caller still owns. + // Only release this caller's delivery; its replacement may still be rotating. const owned = current?.clientId === context?.clientId ? current : undefined if (!context?.onResponseSettled) { this.sender.releaseRotationFence(owned) @@ -141,7 +140,7 @@ export class RelayPtySourcePublication { identity = rotation.identity displayEnd = current.displayEnd recoveryCheckpointSourceEndSu = recovery.acceptedSourceEndSu - recoveryEndSu = snapshot.receivedEndSu + recoveryEndSu = boundedPtyRecoveryEnd(this.session.sourceDeliverySnapshot(identity)) recoveryWasSealed = snapshot.state === 'sealed-unsettled' this.counters.rotated++ } catch (error) { diff --git a/src/relay/relay-pty-source-recovery-window.test.ts b/src/relay/relay-pty-source-recovery-window.test.ts new file mode 100644 index 00000000000..6f3f8186178 --- /dev/null +++ b/src/relay/relay-pty-source-recovery-window.test.ts @@ -0,0 +1,181 @@ +import { afterEach, expect, it } from 'vitest' +import { RelayDispatcher, type RelayClientSessionIdentity } from './dispatcher' +import { boundedPtyRecoveryEnd } from './relay-pty-source-activation' +import { encodeJsonRpcFrame, MessageType } from './protocol' +import { RelayPtySourcePublication } from './relay-pty-source-publication' +import { SshPtyConsumerSessionAdapter } from './ssh-pty-consumer-session-adapter' + +const endpointIdentity: RelayClientSessionIdentity = { + principal: 'endpoint-principal', + authenticated: true, + allowSessionOwner: true, + authenticationKind: 'endpoint-credential' +} + +type Frame = { + id?: number + method?: string + params?: Record + result?: Record +} + +function decode(buffer: Buffer): Frame | null { + return buffer[0] === MessageType.Regular + ? JSON.parse(buffer.subarray(13, 13 + buffer.readUInt32BE(9)).toString('utf8')) + : null +} + +const flushRequests = (): Promise => new Promise((resolve) => setImmediate(resolve)) +let dispatcher: RelayDispatcher | undefined + +afterEach(() => dispatcher?.dispose()) + +it.each([0, 4])( + 'drains a retained tail larger than the window from checkpoint %i', + async (checkpoint) => { + const original: Frame[] = [] + dispatcher = new RelayDispatcher( + (data, settled) => { + const frame = decode(data) + if (frame) { + original.push(frame) + } + settled({ ok: true }) + return true + }, + { supportsWriteCallback: true }, + endpointIdentity + ) + const mux = dispatcher + let publication: RelayPtySourcePublication + const adapter = new SshPtyConsumerSessionAdapter(mux, 'build', undefined, (id) => + publication.onCreditAvailable(id) + ) + publication = new RelayPtySourcePublication(mux, adapter, () => {}) + const open = (clientId: number, id: number, resume?: Record): void => { + mux.feedClient( + clientId, + encodeJsonRpcFrame( + { + jsonrpc: '2.0', + id, + method: 'pty.openClient', + params: { + protocolVersion: 1, + clientInstanceId: 'client', + requestedRole: 'session-owner', + resume, + capabilities: { outputFlowControl: { versions: [1], requestedWindowSu: 4 } } + } + }, + id, + 0 + ) + ) + } + open(1, 1) + await flushRequests() + publication.activate('pty', 'incarnation', { + clientId: 1, + isStale: () => false, + sessionIdentity: endpointIdentity, + onResponseSettled: (settle) => queueMicrotask(() => settle({ ok: true })) + }) + await flushRequests() + expect(publication.publish('pty', { data: 'abcdefghijkl' }, false)).toBe(true) + const oldFrame = original.find((frame) => frame.method === 'pty.data')!.params! + const grant = original.find((frame) => frame.id === 1)!.result! + mux.invalidateClient() + const replacement: Frame[] = [] + const clientId = mux.attachClient( + (data, settled) => { + const frame = decode(data) + if (frame) { + replacement.push(frame) + } + settled({ ok: true }) + return true + }, + { supportsWriteCallback: true }, + endpointIdentity + ) + open(clientId, 2, { ownerGeneration: grant.ownerGeneration, ownerLease: grant.ownerLease }) + await flushRequests() + const recovery = publication.activate( + 'pty', + 'incarnation', + { + clientId, + isStale: () => false, + sessionIdentity: endpointIdentity, + onResponseSettled: (settle) => queueMicrotask(() => settle({ ok: true })) + }, + { + status: 'checkpoint', + clientGeneration: Number(oldFrame.clientGeneration), + ownerGeneration: Number(oldFrame.ownerGeneration), + deliveryToken: String(oldFrame.deliveryToken), + ptyIncarnation: 'incarnation', + acceptedSourceEndSu: checkpoint + } + ) + // The fence lands on the checkpoint itself: the tail drains live rather than behind it. + expect(recovery).toMatchObject({ + status: 'pending', + checkpointSourceEndSu: checkpoint, + recoveryEndSu: checkpoint + }) + await flushRequests() + // The receiver cannot ACK quarantined data until this fence arrives. + expect(replacement.filter((frame) => frame.method === 'pty.recoveryComplete')).toHaveLength(1) + let accepted = checkpoint + let output = '' + for (let turn = 0; accepted < 12 && turn < 4; turn++) { + const frames = replacement.filter( + (frame) => frame.method === 'pty.data' && Number(frame.params!.sourceEndSu) > accepted + ) + expect(frames.length).toBeGreaterThan(0) + for (const frame of frames) { + const params = frame.params! + expect(Number(params.sourceEndSu) - Number(params.sourceLengthSu)).toBe(accepted) + accepted = Number(params.sourceEndSu) + output += String(params.data) + } + expect(publication.getDebugSnapshot().outstandingSourceUnits).toBeLessThanOrEqual(4) + const params = frames.at(-1)!.params! + mux.feedClient( + clientId, + encodeJsonRpcFrame( + { + jsonrpc: '2.0', + method: 'pty.ackData', + params: { + acknowledgements: [ + { + id: 'pty', + clientGeneration: params.clientGeneration, + ownerGeneration: params.ownerGeneration, + deliveryToken: params.deliveryToken, + creditedEndSu: accepted + } + ] + } + }, + 3 + turn, + 0 + ) + ) + await flushRequests() + } + expect(accepted).toBe(12) + expect(output).toBe('abcdefghijkl'.slice(checkpoint)) + expect(publication.getDebugSnapshot().outstandingSourceUnits).toBe(0) + } +) + +it('fences at the checkpoint only once the tail outgrows the window', () => { + const tail = (receivedEndSu: number) => ({ receivedEndSu, creditedEndSu: 4, windowSu: 4 }) + // Exactly one window is still deliverable without credit, so it keeps the ordinary fence. + expect(boundedPtyRecoveryEnd(tail(8))).toBe(8) + expect(boundedPtyRecoveryEnd(tail(9))).toBe(4) +}) diff --git a/src/renderer/src/components/ports/WorkspacePortScanner.test.tsx b/src/renderer/src/components/ports/WorkspacePortScanner.test.tsx index 0ae53fee931..7741368d025 100644 --- a/src/renderer/src/components/ports/WorkspacePortScanner.test.tsx +++ b/src/renderer/src/components/ports/WorkspacePortScanner.test.tsx @@ -602,3 +602,113 @@ describe('WorkspacePortScanner', () => { expect(getPublishedRemoteWorktreePorts()).toBeUndefined() }) }) + +describe('advertised URL refresh bursts', () => { + async function mountLocalScanner(): Promise<() => void> { + useAppStore.setState({ settings: getDefaultSettings('/tmp/orca-workspaces') }) + await act(async () => { + root?.render() + await flushPromises() + }) + localScan.mockClear() + return vi.mocked(window.api.workspacePorts.onAdvertisedUrlChanged).mock + .calls[0][0] as () => void + } + + it('coalesces sequential URL changes into one immediate scan and one settled scan', async () => { + const changed = await mountLocalScanner() + for (let index = 0; index < 5; index++) { + await act(async () => { + changed() + await flushPromises() + await vi.advanceTimersByTimeAsync(100) + }) + } + expect(localScan).toHaveBeenCalledTimes(1) + await act(async () => { + await vi.advanceTimersByTimeAsync(1_000) + }) + expect(localScan).toHaveBeenCalledTimes(2) + await act(async () => { + changed() + await flushPromises() + }) + expect(localScan).toHaveBeenCalledTimes(3) + }) + + it('cancels the settled scan on unmount', async () => { + const changed = await mountLocalScanner() + await act(async () => { + changed() + await flushPromises() + }) + act(() => root?.unmount()) + root = null + await vi.advanceTimersByTimeAsync(2_000) + expect(localScan).toHaveBeenCalledTimes(1) + }) + + it('skips the settled scan while hidden and accepts the next visible URL change', async () => { + let visibility: DocumentVisibilityState = 'visible' + const restore = overrideDocumentVisibilityState(() => visibility) + try { + const changed = await mountLocalScanner() + await act(async () => { + changed() + await flushPromises() + }) + visibility = 'hidden' + await act(async () => { + await vi.advanceTimersByTimeAsync(2_000) + }) + expect(localScan).toHaveBeenCalledTimes(1) + visibility = 'visible' + await act(async () => { + changed() + await flushPromises() + }) + expect(localScan).toHaveBeenCalledTimes(2) + } finally { + restore() + } + }) +}) + +it('releases the URL burst when its leading scan finishes while hidden', async () => { + let visibility: DocumentVisibilityState = 'visible' + const restore = overrideDocumentVisibilityState(() => visibility) + try { + useAppStore.setState({ settings: getDefaultSettings('/tmp/orca-workspaces') }) + await act(async () => { + root?.render() + await flushPromises() + }) + const changed = vi.mocked(window.api.workspacePorts.onAdvertisedUrlChanged).mock + .calls[0][0] as () => void + let finish!: (scan: WorkspacePortScanResult) => void + localScan.mockClear() + localScan.mockImplementationOnce( + () => + new Promise((resolve) => { + finish = resolve + }) + ) + await act(async () => { + changed() + await flushPromises() + }) + visibility = 'hidden' + await act(async () => { + finish(emptyScan) + await flushPromises() + }) + visibility = 'visible' + await act(async () => { + changed() + await flushPromises() + }) + expect(localScan).toHaveBeenCalledTimes(2) + } finally { + restore() + } +}) diff --git a/src/renderer/src/components/ports/WorkspacePortScanner.tsx b/src/renderer/src/components/ports/WorkspacePortScanner.tsx index 2b12bd86cd8..f8f2d11ee40 100644 --- a/src/renderer/src/components/ports/WorkspacePortScanner.tsx +++ b/src/renderer/src/components/ports/WorkspacePortScanner.tsx @@ -280,6 +280,7 @@ export function WorkspacePortScanner({ enabled = true }: { enabled?: boolean }): return } + let burstRefresh: Promise | null = null let eventSequence = 0 let disposed = false let retryTimer: ReturnType | null = null @@ -296,15 +297,24 @@ export function WorkspacePortScanner({ enabled = true }: { enabled?: boolean }): const sequence = eventSequence clearRetryTimer() if (!isWindowVisible()) { + burstRefresh = null return } - void refresh({ force: true, targets: [runtimeTarget] }).finally(() => { - if (disposed || sequence !== eventSequence || !isWindowVisible()) { + // Keep the leading scan through the quiet window so sequential events share it too. + burstRefresh ??= refresh({ force: true, targets: [runtimeTarget] }) + void burstRefresh.finally(() => { + if (disposed || sequence !== eventSequence) { + return + } + if (!isWindowVisible()) { + burstRefresh = null return } // Why: some dev servers print their URL just before the listener is // visible to lsof/netstat. One quiet settle scan catches that startup race. retryTimer = setTimeout(() => { + retryTimer = null + burstRefresh = null if (disposed || sequence !== eventSequence || !isWindowVisible()) { return } diff --git a/src/renderer/src/components/tab-bar/EditorFileTab.test.tsx b/src/renderer/src/components/tab-bar/EditorFileTab.test.tsx index 6eb614550c9..edc15a53f3c 100644 --- a/src/renderer/src/components/tab-bar/EditorFileTab.test.tsx +++ b/src/renderer/src/components/tab-bar/EditorFileTab.test.tsx @@ -345,6 +345,15 @@ function findMenuItemByText(node: unknown, label: string): ReactElementLike { return item } +/** Picks Rename, then fires the close-autofocus that actually opens the input. */ +function selectRenameFromMenu(node: unknown): void { + ;(findMenuItemByText(node, 'Rename').props.onSelect as () => void)() + const content = findElementsByType(node, 'DropdownMenuContent')[0]! + ;(content.props.onCloseAutoFocus as (event: { preventDefault: () => void }) => void)({ + preventDefault: vi.fn() + }) +} + function findSpanByText(node: unknown, label: string): ReactElementLike { const span = findElementsByType(node, 'span').find( (candidate) => @@ -401,7 +410,7 @@ describe('EditorFileTab rename menu', () => { // isUntitled; the tab menu must let users rename the screenshot-style // "untitled-N.md" files directly. expect(renameItem.props.disabled).toBe(false) - ;(renameItem.props.onSelect as () => void)() + selectRenameFromMenu(firstRender) const secondRender = expandNode((await renderEditorFileTab(file, onActivate)).element) const inputs = findElementsByType(secondRender, 'input') @@ -425,9 +434,8 @@ describe('EditorFileTab rename menu', () => { it('ignores IME composition Enter before renaming the editor file tab', async () => { const file = baseFile() const firstRender = expandNode((await renderEditorFileTab(file)).element) - const renameItem = findMenuItemByText(firstRender, 'Rename') - ;(renameItem.props.onSelect as () => void)() + selectRenameFromMenu(firstRender) const secondRender = expandNode((await renderEditorFileTab(file)).element) const input = findElementsByType(secondRender, 'input')[0] @@ -457,9 +465,8 @@ describe('EditorFileTab rename menu', () => { it('does not re-commit when unmounting the rename input emits multiple blur events', async () => { const file = baseFile() const firstRender = expandNode((await renderEditorFileTab(file)).element) - const renameItem = findMenuItemByText(firstRender, 'Rename') - ;(renameItem.props.onSelect as () => void)() + selectRenameFromMenu(firstRender) const secondRender = expandNode((await renderEditorFileTab(file)).element) const input = findElementsByType(secondRender, 'input')[0] diff --git a/src/renderer/src/components/tab-bar/EditorFileTab.tsx b/src/renderer/src/components/tab-bar/EditorFileTab.tsx index 78d17b8b929..064ac1fdb0e 100644 --- a/src/renderer/src/components/tab-bar/EditorFileTab.tsx +++ b/src/renderer/src/components/tab-bar/EditorFileTab.tsx @@ -171,8 +171,8 @@ export default function EditorFileTab({ if (!input) { return } - // Why: Radix closes the context menu after onSelect; defer focus so its - // teardown cannot steal focus back or blur-commit the newly mounted input. + // Why: the tab re-lays out around the input; focus on the next frame so + // that swap has settled before selecting text. renameFocusFrameRef.current = requestAnimationFrame(() => { renameFocusFrameRef.current = null if (renameInputRef.current !== input) { diff --git a/src/renderer/src/components/tab-bar/EditorFileTabContextMenu.test.tsx b/src/renderer/src/components/tab-bar/EditorFileTabContextMenu.test.tsx index 00b2d2a210c..812079563af 100644 --- a/src/renderer/src/components/tab-bar/EditorFileTabContextMenu.test.tsx +++ b/src/renderer/src/components/tab-bar/EditorFileTabContextMenu.test.tsx @@ -202,7 +202,9 @@ function extractText(node: unknown): string { return el.props && 'children' in el.props ? extractText(el.props.children) : '' } -async function renderMenu(): Promise { +async function renderMenu( + overrides: { onActivate?: () => void; onOpenRenameInput?: () => void } = {} +): Promise { const module = await import('./EditorFileTabContextMenu') return module.EditorFileTabContextMenu({ open: true, @@ -238,7 +240,8 @@ async function renderMenu(): Promise { onCloseAll: vi.fn(), onCloseToRight: vi.fn(), onCloseToLeft: vi.fn(), - onOpenMarkdownPreview: vi.fn() + onOpenMarkdownPreview: vi.fn(), + ...overrides }) } @@ -266,6 +269,27 @@ describe('EditorFileTabContextMenu close-all shortcut', () => { vi.unstubAllGlobals() }) + it('opens rename only after menu close releases focus and consumes the request once', async () => { + const onActivate = vi.fn() + const onOpenRenameInput = vi.fn() + const tree = expandNode(await renderMenu({ onActivate, onOpenRenameInput })) + const rename = findElementsByType(tree, 'DropdownMenuItem').find((item) => + extractText(item.props.children).includes('Rename') + )! + const content = findElementsByType(tree, 'DropdownMenuContent')[0]! + ;(rename.props.onSelect as () => void)() + expect(onActivate).not.toHaveBeenCalled() + expect(onOpenRenameInput).not.toHaveBeenCalled() + const preventDefault = vi.fn() + const close = content.props.onCloseAutoFocus as (event: { preventDefault: () => void }) => void + close({ preventDefault }) + expect(preventDefault).toHaveBeenCalledTimes(1) + expect(onActivate).toHaveBeenCalledTimes(1) + expect(onOpenRenameInput).toHaveBeenCalledTimes(1) + close({ preventDefault }) + expect(onOpenRenameInput).toHaveBeenCalledTimes(1) + }) + it('renders assigned shortcuts next to Rename, Close, and Close All Editor Tabs', async () => { const tree = expandNode(await renderMenu()) const menuItems = findElementsByType(tree, 'DropdownMenuItem') diff --git a/src/renderer/src/components/tab-bar/EditorFileTabContextMenu.tsx b/src/renderer/src/components/tab-bar/EditorFileTabContextMenu.tsx index 32e29595738..1813265573f 100644 --- a/src/renderer/src/components/tab-bar/EditorFileTabContextMenu.tsx +++ b/src/renderer/src/components/tab-bar/EditorFileTabContextMenu.tsx @@ -126,6 +126,10 @@ export function EditorFileTabContextMenu({ } skipMenuFocusRestoreRef.current = false event.preventDefault() + // Why: opening the input in onSelect lets the still-closing menu reclaim + // focus, and the resulting blur commits the rename away before the user types. + onActivate() + onOpenRenameInput() }} > { skipMenuFocusRestoreRef.current = true - onActivate() - onOpenRenameInput() }} > diff --git a/src/renderer/src/components/terminal-pane/pane-helpers.test.ts b/src/renderer/src/components/terminal-pane/pane-helpers.test.ts index 8e5987672eb..ee962bd49ae 100644 --- a/src/renderer/src/components/terminal-pane/pane-helpers.test.ts +++ b/src/renderer/src/components/terminal-pane/pane-helpers.test.ts @@ -90,6 +90,7 @@ describe('fitAndFocusPanes', () => { vi.stubGlobal('HTMLElement', FakeHTMLElement) vi.stubGlobal('document', { activeElement, + querySelectorAll: vi.fn(() => []), querySelector: vi.fn((selector: string) => selector === '[data-tab-rename-input="true"]' && renameInputMounted ? (new FakeHTMLElement({ tagName: 'INPUT' }) as unknown as Element) diff --git a/src/renderer/src/components/terminal-pane/pane-helpers.ts b/src/renderer/src/components/terminal-pane/pane-helpers.ts index 84715b241ca..94e4d96a162 100644 --- a/src/renderer/src/components/terminal-pane/pane-helpers.ts +++ b/src/renderer/src/components/terminal-pane/pane-helpers.ts @@ -1,4 +1,5 @@ import type { PaneManager } from '@/lib/pane-manager/pane-manager' +import { focusPanePreservingOverlays } from '@/lib/pane-manager/pane-overlay-focus' export function fitPanes(manager: PaneManager): void { manager.fitAllPanes() @@ -16,7 +17,9 @@ export function focusActivePane(manager: PaneManager): void { } const panes = manager.getPanes() const activePane = manager.getActivePane() ?? panes[0] - activePane?.terminal.focus() + if (activePane) { + focusPanePreservingOverlays(activePane) + } } export function fitAndFocusPanes(manager: PaneManager): void { diff --git a/src/renderer/src/components/terminal-pane/terminal-layout-overlay-focus.test.tsx b/src/renderer/src/components/terminal-pane/terminal-layout-overlay-focus.test.tsx new file mode 100644 index 00000000000..d8684dd5df1 --- /dev/null +++ b/src/renderer/src/components/terminal-pane/terminal-layout-overlay-focus.test.tsx @@ -0,0 +1,97 @@ +// @vitest-environment happy-dom +import { afterEach, describe, expect, it, vi } from 'vitest' +import type { PaneManager } from '@/lib/pane-manager/pane-manager' +import { fitAndFocusPanes } from './pane-helpers' + +function createLayoutFixture() { + const textarea = document.createElement('textarea') + textarea.className = 'xterm-helper-textarea' + document.body.append(textarea) + textarea.focus() + const terminal = { focus: vi.fn(() => textarea.focus()) } + const manager = { + fitAllPanes: vi.fn(), + getActivePane: () => ({ terminal }), + getPanes: () => [{ terminal }] + } as unknown as PaneManager + return { manager, terminal, textarea } +} + +function mountOverlay(role: string) { + const overlay = document.createElement('div') + overlay.setAttribute('role', role) + overlay.tabIndex = -1 + vi.spyOn(overlay, 'getClientRects').mockReturnValue([ + new DOMRect(0, 0, 100, 100) + ] as unknown as DOMRectList) + document.body.append(overlay) + return overlay +} + +afterEach(() => { + document.body.replaceChildren() + vi.restoreAllMocks() +}) + +describe('terminal layout preserves overlay focus', () => { + it.each(['menu', 'dialog', 'alertdialog', 'listbox'])( + 'does not blur an open %s during a queued fit', + (role) => { + const { manager, terminal } = createLayoutFixture() + const overlay = mountOverlay(role) + overlay.focus() + const blurred = vi.fn() + overlay.addEventListener('blur', blurred) + + fitAndFocusPanes(manager) + + expect(manager.fitAllPanes).toHaveBeenCalledOnce() + expect(terminal.focus).not.toHaveBeenCalled() + expect(document.activeElement).toBe(overlay) + expect(blurred).not.toHaveBeenCalled() + } + ) + + it('leaves a mounted menu time to acquire focus', () => { + const { manager, terminal, textarea } = createLayoutFixture() + textarea.blur() + mountOverlay('menu') + + fitAndFocusPanes(manager) + + expect(terminal.focus).not.toHaveBeenCalled() + expect(document.activeElement).toBe(document.body) + }) + + it('allows focus after the menu closes', () => { + const { manager, terminal, textarea } = createLayoutFixture() + const overlay = mountOverlay('menu') + overlay.focus() + overlay.remove() + + fitAndFocusPanes(manager) + + expect(terminal.focus).toHaveBeenCalledOnce() + expect(document.activeElement).toBe(textarea) + }) + + it('hands focus back to a menu that is animating closed', () => { + const { manager, terminal, textarea } = createLayoutFixture() + mountOverlay('menu').setAttribute('data-state', 'closed') + + fitAndFocusPanes(manager) + + expect(terminal.focus).toHaveBeenCalledOnce() + expect(document.activeElement).toBe(textarea) + }) + + it('does not treat the workspace sidebar as a focus-owning overlay', () => { + const { manager, terminal } = createLayoutFixture() + const sidebar = mountOverlay('listbox') + sidebar.setAttribute('data-worktree-sidebar', '') + + fitAndFocusPanes(manager) + + expect(terminal.focus).toHaveBeenCalledOnce() + }) +}) diff --git a/src/renderer/src/components/use-worktree-jump-palette-browser-ownership.test.ts b/src/renderer/src/components/use-worktree-jump-palette-browser-ownership.test.ts new file mode 100644 index 00000000000..c8aea613f03 --- /dev/null +++ b/src/renderer/src/components/use-worktree-jump-palette-browser-ownership.test.ts @@ -0,0 +1,82 @@ +// @vitest-environment happy-dom + +import { cleanup, renderHook } from '@testing-library/react' +import { afterEach, expect, it } from 'vitest' +import { useAppStore } from '@/store' +import type { BrowserPage, BrowserWorkspace } from '../../../shared/browser-workspace-types' +import type { Tab } from '../../../shared/tab-types' +import { makeUnifiedTab, makeWorktree } from './worktree-jump-palette-test-fixtures' +import { useWorktreeJumpPaletteOpenTabs } from './use-worktree-jump-palette-open-tabs' + +afterEach(cleanup) + +it('keeps same-id browser results on their owner with recency, and follows ownership changes', () => { + const worktrees = [ + makeWorktree('same-id', 'Local workspace', { hostId: 'local' }), + makeWorktree('same-id', 'Remote workspace', { hostId: 'runtime:paired' }) + ] + const page: BrowserPage = { + id: 'page', + workspaceId: 'browser', + worktreeId: 'same-id', + url: 'https://example.test/docs', + title: 'Browser proof', + loading: false, + faviconUrl: null, + canGoBack: false, + canGoForward: false, + loadError: null, + createdAt: 1 + } + const workspace: BrowserWorkspace = { + ...page, + id: 'browser', + activePageId: page.id, + pageIds: [page.id] + } + const tab: Tab = { + ...makeUnifiedTab('tab', 'same-id', 'browser', 'Browser proof'), + contentType: 'browser', + executionHostId: 'runtime:paired', + lastFocusedAt: 5_000 + } + type PaletteInput = Parameters[0] + const input: Partial = { + ...useAppStore.getInitialState(), + // The store holds {key, result}; the hook takes the unwrapped result. + workspacePortScan: null, + paletteStatusInputsActive: true, + allWorktrees: worktrees, + browserSortedWorktrees: worktrees, + repoMap: new Map(), + repoByHostIdentity: new Map(), + worktreeOrder: new Map(), + worktreeMatches: [], + hasQuery: true, + deferredQuery: 'Browser proof', + browserTabsByWorktree: { 'same-id': [workspace] }, + browserPagesByWorkspace: { browser: [page] }, + unifiedTabsByWorktree: { 'same-id': [tab] } + } + const { result, rerender } = renderHook( + (props: Partial) => useWorktreeJumpPaletteOpenTabs(props as PaletteInput), + { initialProps: input } + ) + // lastActiveAt rides the same map: without it every browser row sorts as never-focused. + const owners = () => + result.current.browserItems.map(({ result: entry }) => [ + entry.pageId, + entry.executionHostId, + entry.lastActiveAt + ]) + + expect(owners()).toEqual([['page', 'runtime:paired', 5_000]]) + + rerender({ + ...input, + unifiedTabsByWorktree: { + 'same-id': [{ ...tab, executionHostId: 'local' }] + } + }) + expect(owners()).toEqual([['page', 'local', 5_000]]) +}) diff --git a/src/renderer/src/components/use-worktree-jump-palette-open-tabs.ts b/src/renderer/src/components/use-worktree-jump-palette-open-tabs.ts index d6c62711670..42630be7116 100644 --- a/src/renderer/src/components/use-worktree-jump-palette-open-tabs.ts +++ b/src/renderer/src/components/use-worktree-jump-palette-open-tabs.ts @@ -93,6 +93,7 @@ export function useWorktreeJumpPaletteOpenTabs({ worktreeOrder, browserTabsByWorktree, browserPagesByWorkspace, + unifiedTabsByWorktree, activeBrowserTabId, activeWorktreeId, activeWorkspaceExecutionHostId, @@ -109,6 +110,7 @@ export function useWorktreeJumpPaletteOpenTabs({ browserPagesByWorkspace, browserTabsByWorktree, browserSortedWorktrees, + unifiedTabsByWorktree, repoByHostIdentity, repoMap, unifiedTabsByWorktree, diff --git a/src/renderer/src/hooks/ipc-events/terminal-request-ipc-bridge.ts b/src/renderer/src/hooks/ipc-events/terminal-request-ipc-bridge.ts index 1fd3eed2039..a297c751a29 100644 --- a/src/renderer/src/hooks/ipc-events/terminal-request-ipc-bridge.ts +++ b/src/renderer/src/hooks/ipc-events/terminal-request-ipc-bridge.ts @@ -3,6 +3,7 @@ import { getConnectionIdFromState } from '@/lib/connection-context' import { initialAgentTabViewModeProps } from '@/lib/native-chat-initial-view-mode' import { isNativeChatTranscriptLocalReadable } from '@/lib/native-chat-transcript-readability' import { resolveTerminalWorktreeRoute } from '@/lib/terminal-worktree-route' +import { insertUnifiedTabAfterAnchor } from '@/lib/unified-tab-anchor-insertion' import { translate } from '@/i18n/i18n' import { useAppStore } from '../../store' import { @@ -83,30 +84,11 @@ export function registerTerminalRequestIpcBridge(unsubs: (() => void)[]): void { requestBackgroundTerminalWorktreeMount({ worktreeId, tabIds: [tab.id] }) } if (data.afterTabId) { - const createdUnifiedTab = useAppStore + const createdUnifiedTabId = useAppStore .getState() - .unifiedTabsByWorktree[worktreeId]?.find((item) => item.entityId === tab.id) - const anchorUnifiedTab = useAppStore - .getState() - .unifiedTabsByWorktree[worktreeId]?.find((item) => item.id === data.afterTabId) - if ( - createdUnifiedTab && - anchorUnifiedTab && - createdUnifiedTab.groupId === anchorUnifiedTab.groupId - ) { - const group = useAppStore - .getState() - .groupsByWorktree[worktreeId]?.find((item) => item.id === createdUnifiedTab.groupId) - const order = (group?.tabOrder ?? []).filter((id) => id !== createdUnifiedTab.id) - const anchorIndex = order.indexOf(anchorUnifiedTab.id) - order.splice( - anchorIndex === -1 ? order.length : anchorIndex + 1, - 0, - createdUnifiedTab.id - ) - useAppStore.getState().reorderUnifiedTabs(createdUnifiedTab.groupId, order, { - recordInteraction: false - }) + .unifiedTabsByWorktree[worktreeId]?.find((item) => item.entityId === tab.id)?.id + if (createdUnifiedTabId) { + insertUnifiedTabAfterAnchor(worktreeId, createdUnifiedTabId, data.afterTabId) } } if (shouldActivate) { diff --git a/src/renderer/src/lib/pane-manager/pane-manager-pane-creation.ts b/src/renderer/src/lib/pane-manager/pane-manager-pane-creation.ts index c4c3a28c4ca..53ca7f58166 100644 --- a/src/renderer/src/lib/pane-manager/pane-manager-pane-creation.ts +++ b/src/renderer/src/lib/pane-manager/pane-manager-pane-creation.ts @@ -1,3 +1,4 @@ +import { focusPanePreservingOverlays } from './pane-overlay-focus' import type { ManagedPane, ManagedPaneInternal, PaneManagerOptions } from './pane-manager-types' import type { PaneManagerHost } from './pane-manager-host' import { applyPaneOpacity } from './pane-divider' @@ -23,7 +24,7 @@ export function createInitialManagedPane( applyPaneOpacity(host.panes.values(), host.getActivePaneId(), host.getStyleOptions()) if (opts?.focus !== false) { - pane.terminal.focus() + focusPanePreservingOverlays(pane) } host.publishPaneCreated(pane) diff --git a/src/renderer/src/lib/pane-manager/pane-manager.ts b/src/renderer/src/lib/pane-manager/pane-manager.ts index 798d5feaf25..87b06370e02 100644 --- a/src/renderer/src/lib/pane-manager/pane-manager.ts +++ b/src/renderer/src/lib/pane-manager/pane-manager.ts @@ -1,13 +1,11 @@ +import { focusPanePreservingOverlays } from './pane-overlay-focus' import type { PaneManagerOptions, PaneStyleOptions, ManagedPane, ManagedPaneInternal, PaneRenderingDiagnostics, - DropZone, - PaneExternalDropHandler, - PaneExternalDropResolver, - PaneExternalDropTarget + DropZone } from './pane-manager-types' import type { SplitPaneAroundLeafIdsOptions } from './pane-subtree-split' import type { PaneManagerHost } from './pane-manager-host' @@ -68,7 +66,7 @@ export type { PaneExternalDropTarget, PaneExternalDropResolver, PaneExternalDropHandler -} +} from './pane-manager-types' export class PaneManager { private root: HTMLElement @@ -235,7 +233,7 @@ export class PaneManager { applyPaneOpacity(this.panes.values(), this.activePaneId, this.styleOptions) if (opts?.focus !== false) { - pane.terminal.focus() + focusPanePreservingOverlays(pane) } if (changed) { diff --git a/src/renderer/src/lib/pane-manager/pane-overlay-focus.test.ts b/src/renderer/src/lib/pane-manager/pane-overlay-focus.test.ts new file mode 100644 index 00000000000..6b51c1ba449 --- /dev/null +++ b/src/renderer/src/lib/pane-manager/pane-overlay-focus.test.ts @@ -0,0 +1,131 @@ +// @vitest-environment happy-dom +import { afterEach, describe, expect, it, vi } from 'vitest' +import { PaneManager } from './pane-manager' +import { createInitialManagedPane } from './pane-manager-pane-creation' +import type { PaneManagerHost } from './pane-manager-host' +import type { ManagedPaneInternal } from './pane-manager-types' + +vi.mock('./pane-lifecycle', () => ({ + openTerminal: vi.fn(), + createPaneDOM: vi.fn(), + disposePane: vi.fn(), + setLigaturesEnabled: vi.fn() +})) + +afterEach(() => { + document.body.innerHTML = '' +}) + +function fixture() { + const root = document.createElement('div') + document.body.append(root) + const container = document.createElement('div') + const textarea = document.createElement('textarea') + container.append(textarea) + const pane = { + id: 1, + container, + terminal: { focus: vi.fn(() => textarea.focus()) } + } as unknown as ManagedPaneInternal + const panes = new Map([[pane.id, pane]]) + const publishPaneCreated = vi.fn() + const onActivePaneChange = vi.fn() + const manager = Object.create(PaneManager.prototype) as PaneManager + Object.assign(manager, { + panes, + activePaneId: null, + styleOptions: {}, + options: { onActivePaneChange } + }) + const host = { + options: {}, + root, + panes, + createPaneInternal: () => pane, + setActivePaneId: vi.fn(), + getActivePaneId: () => pane.id, + getStyleOptions: () => ({}), + publishPaneCreated + } as unknown as PaneManagerHost + return { root, container, textarea, pane, host, manager, publishPaneCreated, onActivePaneChange } +} + +function overlay(role: string) { + const element = document.createElement('div') + element.setAttribute('role', role) + element.tabIndex = -1 + document.body.append(element) + element.focus() + return element +} + +describe.each(['initial', 'active'] as const)('%s pane focus', (operation) => { + function focus(f: ReturnType, requested = true) { + if (operation === 'initial') { + createInitialManagedPane(f.host, { focus: requested }) + expect(f.publishPaneCreated).toHaveBeenCalledWith(f.pane) + } else { + f.root.append(f.container) + f.manager.setActivePane(f.pane.id, { focus: requested }) + expect(f.manager.getActivePane()?.id).toBe(f.pane.id) + expect(f.onActivePaneChange).toHaveBeenCalledTimes(1) + } + } + + it.each(['menu', 'dialog', 'alertdialog', 'listbox'])('preserves a visible %s', (role) => { + const f = fixture() + const popup = overlay(role) + focus(f) + expect(document.activeElement).toBe(popup) + expect(f.pane.terminal.focus).not.toHaveBeenCalled() + }) + + it('focuses a terminal hosted inside a dialog', () => { + const f = fixture() + overlay('dialog').append(f.root) + focus(f) + expect(document.activeElement).toBe(f.textarea) + }) + + it('preserves a nested popup over a dialog-hosted terminal', () => { + const f = fixture() + const dialog = overlay('dialog') + dialog.append(f.root) + const popup = overlay('menu') + dialog.append(popup) + popup.focus() + focus(f) + expect(document.activeElement).toBe(popup) + }) + + it('allows focus with only persistent sidebar chrome', () => { + const f = fixture() + overlay('listbox').setAttribute('data-worktree-sidebar', '') + focus(f) + expect(document.activeElement).toBe(f.textarea) + }) + + it('allows focus after the overlay closes', () => { + const f = fixture() + overlay('menu').style.display = 'none' + focus(f) + expect(document.activeElement).toBe(f.textarea) + }) + + it('preserves a popup nested inside sidebar chrome', () => { + const f = fixture() + const sidebar = overlay('listbox') + sidebar.setAttribute('data-worktree-sidebar', '') + const popup = overlay('menu') + sidebar.append(popup) + popup.focus() + focus(f) + expect(document.activeElement).toBe(popup) + }) + + it('honors an explicit no-focus request', () => { + const f = fixture() + focus(f, false) + expect(f.pane.terminal.focus).not.toHaveBeenCalled() + }) +}) diff --git a/src/renderer/src/lib/pane-manager/pane-overlay-focus.ts b/src/renderer/src/lib/pane-manager/pane-overlay-focus.ts new file mode 100644 index 00000000000..0dcf5c07ce9 --- /dev/null +++ b/src/renderer/src/lib/pane-manager/pane-overlay-focus.ts @@ -0,0 +1,18 @@ +import { hasVisibleOverlay } from '../visible-overlay' +import type { ManagedPane } from './pane-manager-types' + +export function focusPanePreservingOverlays( + pane: Pick +): void { + if ( + typeof document !== 'undefined' && + hasVisibleOverlay({ + ignoreMatches: '[role="listbox"][data-worktree-sidebar]', + ignoreContaining: pane.container, + ignoreDismissed: true + }) + ) { + return + } + pane.terminal.focus() +} diff --git a/src/renderer/src/lib/unified-tab-anchor-insertion.ts b/src/renderer/src/lib/unified-tab-anchor-insertion.ts new file mode 100644 index 00000000000..2b9055b919f --- /dev/null +++ b/src/renderer/src/lib/unified-tab-anchor-insertion.ts @@ -0,0 +1,22 @@ +import { useAppStore } from '../store' + +/** Move `tabId` to sit immediately after `anchorTabId`; no-op unless both share a group. */ +export function insertUnifiedTabAfterAnchor( + worktreeId: string, + tabId: string, + anchorTabId: string +): void { + if (tabId === anchorTabId) { + return + } + const state = useAppStore.getState() + const group = (state.groupsByWorktree[worktreeId] ?? []).find( + (candidate) => candidate.tabOrder.includes(tabId) && candidate.tabOrder.includes(anchorTabId) + ) + if (!group) { + return + } + const order = group.tabOrder.filter((id) => id !== tabId) + order.splice(order.indexOf(anchorTabId) + 1, 0, tabId) + state.reorderUnifiedTabs(group.id, order, { recordInteraction: false }) +} diff --git a/src/renderer/src/lib/visible-overlay.ts b/src/renderer/src/lib/visible-overlay.ts index 44dc14a514a..c7e4e721069 100644 --- a/src/renderer/src/lib/visible-overlay.ts +++ b/src/renderer/src/lib/visible-overlay.ts @@ -5,12 +5,19 @@ const OVERLAY_SELECTOR = type VisibleOverlayOptions = { /** Overlays inside a match are treated as page content, not as a layer above it. */ ignoreSelector?: string + /** Ignore matching chrome itself while retaining overlays nested within it. */ + ignoreMatches?: string + /** A terminal hosted inside an overlay may still take focus within that overlay. */ + ignoreContaining?: Element + /** An overlay animating out no longer outranks focus that was queued before it closed. */ + ignoreDismissed?: boolean } /** * Whether a dialog, alert dialog, listbox, or menu is on screen. Page-level Escape * handlers ask this before acting: the overlay owns the first Escape, and a page - * that preventDefaults instead vetoes the overlay's own dismissal. + * that preventDefaults instead vetoes the overlay's own dismissal. Terminal focus + * asks the same question: a live overlay outranks a queued pane focus. */ export function hasVisibleOverlay(options?: VisibleOverlayOptions): boolean { return Array.from(document.querySelectorAll(OVERLAY_SELECTOR)).some((element) => { @@ -23,6 +30,19 @@ export function hasVisibleOverlay(options?: VisibleOverlayOptions): boolean { if (options?.ignoreSelector && element.closest(options.ignoreSelector)) { return false } + if (options?.ignoreMatches && element.matches(options.ignoreMatches)) { + return false + } + if (options?.ignoreContaining && element.contains(options.ignoreContaining)) { + return false + } + // Why: overlays stay mounted and painted through their exit animation, so a + // dismissed one would otherwise keep owning a queued focus for ~300ms. Escape + // callers opt out: they run before the attribute flips, so it only ever hides + // a still-open overlay from them. + if (options?.ignoreDismissed && element.getAttribute('data-state') === 'closed') { + return false + } const style = window.getComputedStyle(element) return ( style.display !== 'none' && diff --git a/src/renderer/src/runtime/remote-runtime-terminal-binary-controller.ts b/src/renderer/src/runtime/remote-runtime-terminal-binary-controller.ts index b8abe9da61a..c54c14378de 100644 --- a/src/renderer/src/runtime/remote-runtime-terminal-binary-controller.ts +++ b/src/renderer/src/runtime/remote-runtime-terminal-binary-controller.ts @@ -25,34 +25,28 @@ export abstract class RemoteRuntimeTerminalBinaryController extends RemoteRuntim this.failConnection(new Error('Remote terminal stream received a malformed frame.')) return } + const isOutput = + frame.opcode === TerminalStreamOpcode.Output || + frame.opcode === TerminalStreamOpcode.OutputSpan const stream = this.streams.get(frame.streamId) if (!stream) { - if ( - frame.opcode === TerminalStreamOpcode.Output || - frame.opcode === TerminalStreamOpcode.OutputSpan - ) { + if (isOutput) { // Why: the renderer already disposed this stream; unsubscribe releases server credit that cannot reach a parser. this.sendFrame(frame.streamId, TerminalStreamOpcode.Unsubscribe) } return } - if ( - (frame.opcode === TerminalStreamOpcode.Output || - frame.opcode === TerminalStreamOpcode.OutputSpan) && - shouldDropE2eRemoteTerminalOutput(stream, frame.payload.byteLength) - ) { + if (isOutput && shouldDropE2eRemoteTerminalOutput(stream, frame.payload.byteLength)) { this.queueOutputAcknowledgement(stream, frame.payload.byteLength) return } - stream.watchdog.recordInbound() + // Control frames prove transport activity, not delivery of command output. + stream.watchdog.recordInbound(isOutput) if (frame.opcode === TerminalStreamOpcode.WriteUnavailable) { stream.callbacks.onWriteUnavailable?.() return } - if ( - frame.opcode === TerminalStreamOpcode.Output || - frame.opcode === TerminalStreamOpcode.OutputSpan - ) { + if (isOutput) { this.handleOutputFrame(frame, stream) return } diff --git a/src/renderer/src/runtime/remote-runtime-terminal-response-controller.ts b/src/renderer/src/runtime/remote-runtime-terminal-response-controller.ts index a8f76c78e70..5682a1a3bdd 100644 --- a/src/renderer/src/runtime/remote-runtime-terminal-response-controller.ts +++ b/src/renderer/src/runtime/remote-runtime-terminal-response-controller.ts @@ -40,7 +40,7 @@ export abstract class RemoteRuntimeTerminalResponseController extends RemoteRunt if (!stream) { return } - stream.watchdog.recordInbound() + stream.watchdog.recordInbound(false) if (event.type === 'end' && shouldHoldE2eRemoteTerminalEnd(stream.terminal)) { return } diff --git a/src/renderer/src/runtime/remote-runtime-terminal-stall-recovery.test.ts b/src/renderer/src/runtime/remote-runtime-terminal-stall-recovery.test.ts index c6e64e4e410..b28d6507de1 100644 --- a/src/renderer/src/runtime/remote-runtime-terminal-stall-recovery.test.ts +++ b/src/renderer/src/runtime/remote-runtime-terminal-stall-recovery.test.ts @@ -236,7 +236,28 @@ describe('remote terminal stalled stream recovery', () => { stream.close() }) - it('restarts a stream when the authoritative snapshot advanced without live output', async () => { + // Why: a control frame landing just after Enter is transport activity, not the command's answer. + it.each<[string, (streamId: number) => void]>([ + ['no intervening frame', () => {}], + ['a resize acknowledgement', (id) => emitControlFrame(id, TerminalStreamOpcode.Resized)], + ['a metadata frame', (id) => emitControlFrame(id, TerminalStreamOpcode.Metadata)], + [ + 'a fit-override change', + (id) => + emitStreamEvent({ + type: 'fit-override-changed', + streamId: id, + mode: 'mobile-fit', + cols: 80, + rows: 24 + }) + ], + [ + 'a driver change', + (id) => emitStreamEvent({ type: 'driver-changed', streamId: id, driver: { kind: 'idle' } }) + ], + ['an unsolicited snapshot', (id) => emitSnapshot(id, undefined, 'baseline', 8)] + ])('recovers missing live output despite %s', async (_label, emitIntervening) => { const { getRemoteRuntimeTerminalMultiplexer } = await import('./remote-runtime-terminal-multiplexer') const onTransportClose = vi.fn() @@ -250,8 +271,10 @@ describe('remote terminal stalled stream recovery', () => { sendBinary.mockClear() expect(stream.sendInput('echo missing\r')).toBe(true) + emitIntervening(stream.streamId) await vi.advanceTimersByTimeAsync(REMOTE_TERMINAL_COMMAND_RESPONSE_TIMEOUT_MS) const request = sentFrames(TerminalStreamOpcode.SnapshotRequest)[0] + expect(request).toBeDefined() const payload = request ? decodeTerminalStreamJson<{ requestId: number }>(request.payload) : null @@ -445,6 +468,21 @@ describe('remote terminal stalled stream recovery', () => { ) } + function emitControlFrame(streamId: number, opcode: TerminalStreamOpcode): void { + callbacks?.onBinary( + encodeTerminalStreamFrame({ + opcode, + streamId, + seq: 0, + payload: encodeTerminalStreamJson({ cols: 80, rows: 24 }) + }) + ) + } + + function emitStreamEvent(result: Record): void { + callbacks?.onResponse({ ok: true, result }) + } + function emitSnapshot( streamId: number, requestId: number | undefined, diff --git a/src/renderer/src/runtime/remote-terminal-stream-watchdog.ts b/src/renderer/src/runtime/remote-terminal-stream-watchdog.ts index be9e1b9d404..290a366ecc2 100644 --- a/src/renderer/src/runtime/remote-terminal-stream-watchdog.ts +++ b/src/renderer/src/runtime/remote-terminal-stream-watchdog.ts @@ -13,7 +13,8 @@ export type RemoteTerminalStreamWatchdog = { recordOutputAcknowledged: (bytes: number) => void completeCommandResponseProbe: () => void recordCommandInput: (text: string) => void - recordInbound: () => void + /** Only live output answers a pending command; control frames prove transport activity alone. */ + recordInbound: (carriesOutput: boolean) => void dispose: () => void } @@ -111,9 +112,11 @@ export function createRemoteTerminalStreamWatchdog( REMOTE_TERMINAL_COMMAND_RESPONSE_TIMEOUT_MS ) }, - recordInbound() { + recordInbound(carriesOutput) { lastInboundAtMs = Date.now() - clearResponseTimer() + if (carriesOutput) { + clearResponseTimer() + } }, dispose() { disposed = true diff --git a/src/renderer/src/runtime/web-runtime-session-terminal-legacy-create.test.ts b/src/renderer/src/runtime/web-runtime-session-terminal-legacy-create.test.ts index 6b1ba5e6d7c..76ec9f5e150 100644 --- a/src/renderer/src/runtime/web-runtime-session-terminal-legacy-create.test.ts +++ b/src/renderer/src/runtime/web-runtime-session-terminal-legacy-create.test.ts @@ -131,6 +131,71 @@ describe('createWebRuntimeSessionTerminal', () => { ]) }) + it.each([ + { + agent: 'codex' as const, + predecessor: 'web-terminal-host-tab-1', + afterTabId: 'web-terminal-host-tab-1%3A%3Aleaf-1' + }, + { + agent: undefined, + predecessor: 'web-terminal-host-tab-1', + afterTabId: 'web-terminal-host-tab-1%3A%3Aleaf-1' + }, + { agent: undefined, predecessor: 'local-browser-tab', afterTabId: 'local-browser-tab' }, + { + agent: undefined, + predecessor: 'web-terminal-host-tab-1%3A%3Aleaf-1', + afterTabId: 'web-terminal-host-tab-1%3A%3Aleaf-1' + } + ])( + 'settles after $afterTabId for $agent creation without activating it', + async ({ agent, predecessor, afterTabId }) => { + const successor = 'web-terminal-host-tab-3' + const created = 'web-terminal-host-tab-2' + const reorderUnifiedTabs = vi.fn() + mocks.getState.mockReturnValue({ + ...mocks.getState(), + unifiedTabsByWorktree: { + [WORKTREE_ID]: [predecessor, successor, created].map((id) => ({ + id, + groupId: 'client-group' + })) + }, + groupsByWorktree: { + [WORKTREE_ID]: [{ id: 'client-group', tabOrder: [predecessor, successor, created] }] + }, + reorderUnifiedTabs, + moveUnifiedTabToGroup: mocks.moveUnifiedTabToGroup + }) + const runtimeCall = vi.fn(async (request: { method: string }) => ({ + id: request.method, + ok: true, + result: + request.method === 'session.tabs.createTerminal' + ? { tab: { id: 'host-tab-2::leaf-2' }, publicationEpoch: 'epoch-1', snapshotVersion: 2 } + : makeSnapshot() + })) + vi.stubGlobal('window', { api: { runtimeEnvironments: { call: runtimeCall } } }) + + await expect( + createWebRuntimeSessionTerminal({ + worktreeId: WORKTREE_ID, + afterTabId, + agent, + activate: false + }) + ).resolves.toEqual({ status: 'created' }) + + expect(reorderUnifiedTabs).toHaveBeenCalledExactlyOnceWith( + 'client-group', + [predecessor, created, successor], + { recordInteraction: false } + ) + expect(mocks.moveUnifiedTabToGroup).not.toHaveBeenCalled() + } + ) + it('can create a terminal without selecting the target worktree', async () => { const setStateResults: unknown[] = [] mocks.setState.mockImplementation((updater: (state: unknown) => unknown) => { diff --git a/src/renderer/src/runtime/web-runtime-terminal-create-operation.ts b/src/renderer/src/runtime/web-runtime-terminal-create-operation.ts index 5ca20bdb7f3..601888e5f9b 100644 --- a/src/renderer/src/runtime/web-runtime-terminal-create-operation.ts +++ b/src/renderer/src/runtime/web-runtime-terminal-create-operation.ts @@ -248,20 +248,26 @@ export async function createWebRuntimeSessionTerminalResult( // tab to THIS new terminal, instead of sticky-keeping the prior tab. recordWebSessionFocusIntent(intentOwner, args.worktreeId, createdTabId, createdLeafId) } + const placementTabId = + createdTabId && (args.targetGroupId || args.afterTabId) ? createdTabId : undefined await refreshWebRuntimeSessionTabsSnapshot(environmentId, args.worktreeId, { expectedEnvironmentPairingRevision: intentOwner.pairingRevision, // Why: the publication can beat the RPC response; replay it once after caller intent exists. acceptCurrentSnapshot: - Boolean(createdTabId) && (args.activate !== false || Boolean(args.targetGroupId)), + Boolean(createdTabId) && (args.activate !== false || Boolean(placementTabId)), // Why: a placement record needs a post-create list; a deduped in-flight one can predate it. - ...(args.targetGroupId && createdTabId ? { afterCurrentInFlight: true } : {}) + ...(placementTabId ? { afterCurrentInFlight: true } : {}) }) - if (args.targetGroupId && createdTabId) { + if (placementTabId) { await settleWebRuntimeTerminalPlacement( environmentId, args.worktreeId, - webTerminalPlacementParentTabId(createdTabId), - { groupId: args.targetGroupId, activate: args.activate !== false } + webTerminalPlacementParentTabId(placementTabId), + { + groupId: args.targetGroupId, + afterTabId: args.afterTabId, + activate: args.activate !== false + } ) } return { diff --git a/src/renderer/src/runtime/web-runtime-terminal-placement-settlement.ts b/src/renderer/src/runtime/web-runtime-terminal-placement-settlement.ts index 50cbb551f97..e1b10d8b5b2 100644 --- a/src/renderer/src/runtime/web-runtime-terminal-placement-settlement.ts +++ b/src/renderer/src/runtime/web-runtime-terminal-placement-settlement.ts @@ -1,13 +1,31 @@ +import { insertUnifiedTabAfterAnchor } from '../lib/unified-tab-anchor-insertion' import { useAppStore } from '../store' -import { forgetWebSessionTerminalPlacement } from './web-session-terminal-placement' -import { toWebTerminalSurfaceTabId } from './web-terminal-surface-id' +import { + forgetWebSessionTerminalPlacement, + webTerminalPlacementParentTabId +} from './web-session-terminal-placement' +import { + isWebTerminalSurfaceTabId, + toHostSessionTabId, + toWebTerminalSurfaceTabId +} from './web-terminal-surface-id' + +/** Snapshots key mirrored terminals by the parent tab, so an unknown `parent::leaf` anchor resolves to its parent. */ +function anchorUnifiedTabId(worktreeId: string, afterTabId: string): string { + const known = (useAppStore.getState().unifiedTabsByWorktree[worktreeId] ?? []).some( + (tab) => tab.id === afterTabId + ) + return known || !isWebTerminalSurfaceTabId(afterTabId) + ? afterTabId + : toWebTerminalSurfaceTabId(webTerminalPlacementParentTabId(toHostSessionTabId(afterTabId))) +} /** Settle the placement once the mirrored tab exists (bounded poll), then consume the record. */ export async function settleWebRuntimeTerminalPlacement( environmentId: string, worktreeId: string, hostTabId: string, - placement: { groupId: string; activate: boolean } + placement: { groupId?: string; afterTabId?: string; activate: boolean } ): Promise { const unifiedTabId = toWebTerminalSurfaceTabId(hostTabId) const findTab = () => @@ -20,18 +38,36 @@ export async function settleWebRuntimeTerminalPlacement( await new Promise((resolve) => setTimeout(resolve, 250)) } const tab = findTab() + if (!tab) { + return + } + const anchorId = placement.afterTabId + ? anchorUnifiedTabId(worktreeId, placement.afterTabId) + : undefined const state = useAppStore.getState() - const targetGroupExists = (state.groupsByWorktree[worktreeId] ?? []).some( - (group) => group.id === placement.groupId - ) - if (tab && targetGroupExists && tab.groupId !== placement.groupId) { + const groups = state.groupsByWorktree[worktreeId] ?? [] + // Why: the requested group can be closed while the mirrored tab is still in flight; the + // anchor's own group still expresses where the caller asked for this terminal. + const targetGroup = + groups.find((group) => group.id === placement.groupId) ?? + (anchorId === undefined + ? undefined + : groups.find((group) => group.tabOrder.includes(anchorId))) + if (!targetGroup) { + return + } + if (tab.groupId !== targetGroup.id) { // Why: a snapshot can adopt the tab before the record exists (the publication races the // RPC response); repair through the same client-owned move a user drag takes. - state.moveUnifiedTabToGroup(unifiedTabId, placement.groupId, { + state.moveUnifiedTabToGroup(unifiedTabId, targetGroup.id, { activate: placement.activate, recordInteraction: false }) } + if (anchorId) { + // The create caller owns this insertion; subsequent host snapshots preserve client order. + insertUnifiedTabAfterAnchor(worktreeId, unifiedTabId, anchorId) + } } finally { forgetWebSessionTerminalPlacement({ environmentId, worktreeId, hostTabId }) } diff --git a/src/renderer/src/store/slices/agent-pane-authority.test.ts b/src/renderer/src/store/slices/agent-pane-authority.test.ts index 5e97e3c9064..01e99f27d6f 100644 --- a/src/renderer/src/store/slices/agent-pane-authority.test.ts +++ b/src/renderer/src/store/slices/agent-pane-authority.test.ts @@ -74,6 +74,20 @@ describe('agent pane authority', () => { expect(retirePaneAuthority).toHaveBeenCalledWith(TARGET) }) + it('re-retiring an already-retired pane keeps the retired-key map identity and epochs', () => { + const store = createTestStore() + store.getState().setAgentStatus(TARGET, { state: 'working', prompt: 'target' }) + store.getState().retireAgentPaneAuthority(TARGET) + const before = store.getState() + + store.getState().retireAgentPaneAuthority(TARGET) + + const after = store.getState() + expect(after.recentlyRetiredAgentStatusPaneKeys).toBe(before.recentlyRetiredAgentStatusPaneKeys) + expect(after.agentStatusEpoch).toBe(before.agentStatusEpoch) + expect(after.sortEpoch).toBe(before.sortEpoch) + }) + it('retires the pane activity cutoff with the rest of its pane-owned state', () => { const store = createTestStore() store.setState({ diff --git a/src/renderer/src/store/slices/agent-status-pane-keyed-records.test.ts b/src/renderer/src/store/slices/agent-status-pane-keyed-records.test.ts new file mode 100644 index 00000000000..5b327561637 --- /dev/null +++ b/src/renderer/src/store/slices/agent-status-pane-keyed-records.test.ts @@ -0,0 +1,110 @@ +import { describe, expect, it } from 'vitest' +import { + RECENTLY_CLOSED_AGENT_STATUS_TAB_IDS_MAX, + RECENTLY_RETIRED_AGENT_STATUS_PANE_KEYS_MAX, + boundRecentlyClosedAgentStatusTabIds, + boundRecentlyRetiredAgentStatusPaneKeys +} from './agent-status-pane-keyed-records' + +function keyRecord(keys: readonly string[]): Record { + const record: Record = {} + for (const key of keys) { + record[key] = true + } + return record +} + +function fullRecord(max: number, prefix: string): Record { + return keyRecord(Array.from({ length: max }, (_, i) => `${prefix}${i}`)) +} + +describe('boundRecentlyRetiredAgentStatusPaneKeys', () => { + it('returns the existing record when there is nothing to add', () => { + const existing = keyRecord(['a', 'b']) + expect(boundRecentlyRetiredAgentStatusPaneKeys(existing, [])).toBe(existing) + const empty = keyRecord([]) + expect(boundRecentlyRetiredAgentStatusPaneKeys(empty, [])).toBe(empty) + }) + + it('returns the existing record when the additions already form its tail in order', () => { + const existing = keyRecord(['a', 'b', 'c']) + expect(boundRecentlyRetiredAgentStatusPaneKeys(existing, ['c'])).toBe(existing) + expect(boundRecentlyRetiredAgentStatusPaneKeys(existing, ['b', 'c'])).toBe(existing) + expect(boundRecentlyRetiredAgentStatusPaneKeys(existing, ['a', 'b', 'c'])).toBe(existing) + }) + + // Why: LRU order decides which key the cap evicts next. A key-set match is not a + // no-op when the re-added key is not already at the tail — it must move there. + it('re-retiring an existing non-tail key changes identity and moves it to the tail', () => { + const existing = keyRecord(['a', 'b', 'c']) + const next = boundRecentlyRetiredAgentStatusPaneKeys(existing, ['a']) + expect(next).not.toBe(existing) + expect(Object.keys(next)).toEqual(['b', 'c', 'a']) + expect(Object.keys(existing)).toEqual(['a', 'b', 'c']) + }) + + it('tail keys re-added in a different relative order are rebuilt in the new order', () => { + const existing = keyRecord(['a', 'b', 'c']) + const next = boundRecentlyRetiredAgentStatusPaneKeys(existing, ['c', 'b']) + expect(next).not.toBe(existing) + expect(Object.keys(next)).toEqual(['a', 'c', 'b']) + }) + + it('appends new keys after the existing ones', () => { + const existing = keyRecord(['a']) + const next = boundRecentlyRetiredAgentStatusPaneKeys(existing, ['b', 'c']) + expect(next).not.toBe(existing) + expect(Object.keys(next)).toEqual(['a', 'b', 'c']) + }) + + it('evicts the oldest keys once the cap is exceeded', () => { + const full = fullRecord(RECENTLY_RETIRED_AGENT_STATUS_PANE_KEYS_MAX, 'k') + const next = boundRecentlyRetiredAgentStatusPaneKeys(full, ['fresh']) + const keys = Object.keys(next) + expect(keys).toHaveLength(RECENTLY_RETIRED_AGENT_STATUS_PANE_KEYS_MAX) + expect(keys[0]).toBe('k1') + expect(keys.at(-1)).toBe('fresh') + expect(next.k0).toBeUndefined() + }) + + it('re-retiring the oldest key at the cap keeps it fenced and evicts the next oldest', () => { + const full = fullRecord(RECENTLY_RETIRED_AGENT_STATUS_PANE_KEYS_MAX, 'k') + const bumped = boundRecentlyRetiredAgentStatusPaneKeys(full, ['k0']) + expect(bumped).not.toBe(full) + expect(Object.keys(bumped).at(-1)).toBe('k0') + const afterFresh = boundRecentlyRetiredAgentStatusPaneKeys(bumped, ['fresh']) + expect(afterFresh.k0).toBe(true) + expect(afterFresh.k1).toBeUndefined() + }) + + it('never returns an over-cap record unchanged', () => { + const over = fullRecord(RECENTLY_RETIRED_AGENT_STATUS_PANE_KEYS_MAX + 1, 'k') + const last = `k${RECENTLY_RETIRED_AGENT_STATUS_PANE_KEYS_MAX}` + const next = boundRecentlyRetiredAgentStatusPaneKeys(over, [last]) + expect(next).not.toBe(over) + expect(Object.keys(next)).toHaveLength(RECENTLY_RETIRED_AGENT_STATUS_PANE_KEYS_MAX) + expect(next.k0).toBeUndefined() + }) +}) + +describe('boundRecentlyClosedAgentStatusTabIds', () => { + it('returns the existing record when the tab is already the most recent', () => { + const existing = keyRecord(['t1', 't2']) + expect(boundRecentlyClosedAgentStatusTabIds(existing, 't2')).toBe(existing) + }) + + it('moves a re-closed tab to the tail', () => { + const existing = keyRecord(['t1', 't2']) + const next = boundRecentlyClosedAgentStatusTabIds(existing, 't1') + expect(next).not.toBe(existing) + expect(Object.keys(next)).toEqual(['t2', 't1']) + }) + + it('evicts the oldest tab once the cap is exceeded', () => { + const full = fullRecord(RECENTLY_CLOSED_AGENT_STATUS_TAB_IDS_MAX, 't') + const next = boundRecentlyClosedAgentStatusTabIds(full, 'fresh') + expect(Object.keys(next)).toHaveLength(RECENTLY_CLOSED_AGENT_STATUS_TAB_IDS_MAX) + expect(next.t0).toBeUndefined() + expect(next.fresh).toBe(true) + }) +}) diff --git a/src/renderer/src/store/slices/agent-status-pane-keyed-records.ts b/src/renderer/src/store/slices/agent-status-pane-keyed-records.ts index af5cc4e83c7..f9199140c07 100644 --- a/src/renderer/src/store/slices/agent-status-pane-keyed-records.ts +++ b/src/renderer/src/store/slices/agent-status-pane-keyed-records.ts @@ -3,47 +3,66 @@ export const RECENTLY_RETIRED_AGENT_STATUS_PANE_KEYS_MAX = 1024 // delete-then-set for LRU recency, then evict oldest keys past the cap (Record iterates // insertion order); safe because a status for a tab closed >MAX tabs ago cannot still arrive. -export function boundRecentlyClosedAgentStatusTabIds( +function boundLruKeyRecord( existing: Record, - tabId: string + additions: ReadonlySet, + max: number ): Record { - const next: Record = {} - for (const key of Object.keys(existing)) { - if (key !== tabId) { - next[key] = true - } + if (isLruKeyRecordUnchanged(existing, additions, max)) { + return existing } - next[tabId] = true - const keys = Object.keys(next) - if (keys.length > RECENTLY_CLOSED_AGENT_STATUS_TAB_IDS_MAX) { - for (const stale of keys.slice(0, keys.length - RECENTLY_CLOSED_AGENT_STATUS_TAB_IDS_MAX)) { - delete next[stale] - } - } - return next -} - -export function boundRecentlyRetiredAgentStatusPaneKeys( - existing: Record, - paneKeys: readonly string[] -): Record { - const additions = new Set(paneKeys) const next: Record = {} for (const key of Object.keys(existing)) { if (!additions.has(key)) { next[key] = true } } - for (const paneKey of additions) { - next[paneKey] = true + for (const key of additions) { + next[key] = true } const keys = Object.keys(next) - for (const stale of keys.slice(0, -RECENTLY_RETIRED_AGENT_STATUS_PANE_KEYS_MAX)) { + for (const stale of keys.slice(0, -max)) { delete next[stale] } return next } +// The rebuild is a no-op only when nothing would be evicted and the additions are +// already the tail of `existing` in that same relative order. A matching key SET is +// not enough: re-adding a key moves it to the tail, and that order decides which key +// the cap evicts next, so a stale-order hit would un-fence a recently retired pane. +function isLruKeyRecordUnchanged( + existing: Record, + additions: ReadonlySet, + max: number +): boolean { + const keys = Object.keys(existing) + if (keys.length > max || additions.size > keys.length) { + return false + } + let index = keys.length - additions.size + for (const key of additions) { + if (keys[index++] !== key) { + return false + } + } + return true +} + +export function boundRecentlyClosedAgentStatusTabIds( + existing: Record, + tabId: string +): Record { + return boundLruKeyRecord(existing, new Set([tabId]), RECENTLY_CLOSED_AGENT_STATUS_TAB_IDS_MAX) +} + +export function boundRecentlyRetiredAgentStatusPaneKeys( + existing: Record, + paneKeys: readonly string[] +): Record { + return boundLruKeyRecord(existing, new Set(paneKeys), RECENTLY_RETIRED_AGENT_STATUS_PANE_KEYS_MAX) +} + export function movePaneKeyedRecord( record: Record, fromPaneKey: string, diff --git a/src/renderer/src/store/slices/browser/browser-close-actions.ts b/src/renderer/src/store/slices/browser/browser-close-actions.ts index c70b93f6eb2..3aa77f9e46c 100644 --- a/src/renderer/src/store/slices/browser/browser-close-actions.ts +++ b/src/renderer/src/store/slices/browser/browser-close-actions.ts @@ -12,6 +12,7 @@ import { getFallbackTabTypeForWorktree, isLocalBrowserPageOwner } from './browse import { closeRemoteBrowserPageInOwningEnvironment } from './browser-remote-close' import { releaseDocPreviewGrant } from '@/lib/doc-preview-grants' import { destroyWorkspaceWebviews } from '../browser-webview-cleanup' +import { omitRecordKeys } from '../worktrees/teardown/record-key-omission' export function createBrowserCloseActions( set: BrowserSliceSet, @@ -231,10 +232,15 @@ export function createBrowserCloseActions( destroyWorkspaceWebviews(browserPagesByWorkspace, workspace.id) } set((s) => { - const nextBrowserTabsByWorktree = { ...s.browserTabsByWorktree } - delete nextBrowserTabsByWorktree[worktreeId] - const nextActiveBrowserTabIdByWorktree = { ...s.activeBrowserTabIdByWorktree } - delete nextActiveBrowserTabIdByWorktree[worktreeId] + const removedWorktreeIds = [worktreeId] + const nextBrowserTabsByWorktree = omitRecordKeys( + s.browserTabsByWorktree, + removedWorktreeIds + ) + const nextActiveBrowserTabIdByWorktree = omitRecordKeys( + s.activeBrowserTabIdByWorktree, + removedWorktreeIds + ) // Why: reset the global browser surface only when the shut-down worktree is the active one AND had tabs. const shouldResetGlobalBrowser = s.activeWorktreeId === worktreeId && hadBrowserTabs return { diff --git a/src/renderer/src/store/slices/worktrees/teardown/remove-worktree-store-cleanup.ts b/src/renderer/src/store/slices/worktrees/teardown/remove-worktree-store-cleanup.ts index 09ab88fdfbc..415697b883e 100644 --- a/src/renderer/src/store/slices/worktrees/teardown/remove-worktree-store-cleanup.ts +++ b/src/renderer/src/store/slices/worktrees/teardown/remove-worktree-store-cleanup.ts @@ -35,6 +35,12 @@ export function applyRemoveWorktreeSuccessState( } } const omitByFileId = (m: Record | undefined) => omitRecordKeys(m, removedFileIds) + // Why guarded: a removed worktree usually has no open file, and an unconditional + // filter would hand openFiles a new identity anyway — the sibling purge path + // already does this. + const nextOpenFiles = s.openFiles.some((f) => f.worktreeId === worktreeId) + ? s.openFiles.filter((f) => f.worktreeId !== worktreeId) + : s.openFiles // If the active file belonged to the removed worktree, clear it const activeFileCleared = s.activeFileId ? s.openFiles.some((f) => f.id === s.activeFileId && f.worktreeId === worktreeId) @@ -79,7 +85,7 @@ export function applyRemoveWorktreeSuccessState( ? null : s.activeWorkspaceExecutionHostId, activeTabId: s.activeTabId && tabIds.has(s.activeTabId) ? null : s.activeTabId, - openFiles: s.openFiles.filter((f) => f.worktreeId !== worktreeId), + openFiles: nextOpenFiles, browserTabsByWorktree: omitByWorktree(s.browserTabsByWorktree), // Why: closeBrowserTab records a Cmd+Shift+T undo snapshot, but a deleted worktree's tabs can't be restored; purge it. recentlyClosedBrowserTabsByWorktree: omitByWorktree(s.recentlyClosedBrowserTabsByWorktree), diff --git a/src/renderer/src/store/slices/worktrees/teardown/worktree-teardown-array-identity.test.ts b/src/renderer/src/store/slices/worktrees/teardown/worktree-teardown-array-identity.test.ts new file mode 100644 index 00000000000..5e122d9dbef --- /dev/null +++ b/src/renderer/src/store/slices/worktrees/teardown/worktree-teardown-array-identity.test.ts @@ -0,0 +1,58 @@ +import { describe, expect, it } from 'vitest' +import type { AppState } from '../../../types' +import { applyRemoveWorktreeSuccessState } from './remove-worktree-store-cleanup' + +type OpenFile = AppState['openFiles'][number] + +const REMOVED = 'repo-1::/repos/one/removed' +const KEPT = 'repo-1::/repos/one/kept' + +function fileFor(worktreeId: string, id: string): OpenFile { + return { id, worktreeId, path: `${worktreeId}/f.ts`, name: 'f.ts' } as unknown as OpenFile +} + +function buildState(openFiles: OpenFile[]): AppState { + return { + worktreesByRepo: { 'repo-1': [] }, + tabsByWorktree: { [KEPT]: [] }, + openFiles, + everActivatedWorktreeIds: new Set(), + lastVisitedAtByWorktreeId: {}, + deleteStateByWorktreeId: {}, + sortEpoch: 0 + } as unknown as AppState +} + +function removeWorktree(state: AppState): AppState { + let current = state + applyRemoveWorktreeSuccessState( + (update) => { + const patch = typeof update === 'function' ? update(current) : update + current = { ...current, ...patch } + }, + REMOVED, + new Set() + ) + return current +} + +describe('worktree removal openFiles identity', () => { + it('keeps the openFiles reference when the removed worktree had no open file', () => { + // openFiles is selected whole by the editor panel, file explorer and git-status + // polling, so a fresh array here rerenders all of them for no data change. + const before = buildState([fileFor(KEPT, 'kept-file')]) + + const after = removeWorktree(before) + + expect(after.openFiles).toBe(before.openFiles) + }) + + it('still drops the removed worktree files', () => { + const before = buildState([fileFor(KEPT, 'kept-file'), fileFor(REMOVED, 'gone-file')]) + + const after = removeWorktree(before) + + expect(after.openFiles).not.toBe(before.openFiles) + expect(after.openFiles.map((f) => f.id)).toEqual(['kept-file']) + }) +}) diff --git a/src/renderer/src/store/terminals/terminal-shutdown-guards-identity.test.ts b/src/renderer/src/store/terminals/terminal-shutdown-guards-identity.test.ts new file mode 100644 index 00000000000..c353991c2c1 --- /dev/null +++ b/src/renderer/src/store/terminals/terminal-shutdown-guards-identity.test.ts @@ -0,0 +1,72 @@ +import { describe, expect, it, vi } from 'vitest' +import type { AppState } from '../types' +import { createTerminalShutdownGuardController } from './terminal-shutdown-guards' + +vi.mock('@/components/terminal-pane/pty-transport', () => ({ + restorePtyDataHandlersAfterFailedShutdown: vi.fn(), + unregisterPtyDataHandlers: vi.fn(() => []) +})) +vi.mock('@/components/terminal-pane/terminal-parked-watcher-registry', () => ({ + disposeParkedTerminalWatchersForPtyIds: vi.fn() +})) +vi.mock('@/components/terminal-pane/pty-shutdown-exit-deferral', () => ({ + clearCommittedPtyShutdownSettlements: vi.fn(), + hasCommittedPtyShutdownSettlement: vi.fn(() => false), + markCommittedPtyShutdowns: vi.fn(), + noteCommittedPtyShutdownSettlements: vi.fn(), + settleDeferredPtyShutdownExits: vi.fn() +})) + +function harness(initial: Partial, exitGuardPtyIds: readonly string[]) { + let current = initial as AppState + const set = vi.fn((update: unknown) => { + const patch = + typeof update === 'function' ? (update as (s: AppState) => object)(current) : update + current = { ...current, ...(patch as object) } + }) + const guards = createTerminalShutdownGuardController({ + exitGuardPtyIds, + get: (() => current) as never, + keepIdentifiers: false, + rendererShutdownPtyIds: exitGuardPtyIds, + runtimeEnvironmentId: null, + set: set as never, + tabs: [] + }) + return { guards, set, state: () => current } +} + +describe('markShutdownPending identity', () => { + it('does not write the store when there is nothing to guard', () => { + const { guards, set } = harness({ suppressedPtyExitIds: {}, pendingPtyShutdownIds: {} }, []) + + guards.markShutdownPending() + + expect(set).not.toHaveBeenCalled() + }) + + it('still counts a pending owner when every id is already suppressed', () => { + const suppressedPtyExitIds: Record = { 'pty-1': true } + const { guards, state } = harness( + { suppressedPtyExitIds, pendingPtyShutdownIds: { 'pty-1': 1 } }, + ['pty-1'] + ) + + guards.markShutdownPending() + + expect(state().suppressedPtyExitIds).toBe(suppressedPtyExitIds) + expect(state().pendingPtyShutdownIds).toEqual({ 'pty-1': 2 }) + }) + + it('suppresses the ids that were not yet suppressed', () => { + const { guards, state } = harness( + { suppressedPtyExitIds: { 'pty-1': true }, pendingPtyShutdownIds: {} }, + ['pty-1', 'pty-2'] + ) + + guards.markShutdownPending() + + expect(state().suppressedPtyExitIds).toEqual({ 'pty-1': true, 'pty-2': true }) + expect(state().pendingPtyShutdownIds).toEqual({ 'pty-1': 1, 'pty-2': 1 }) + }) +}) diff --git a/src/renderer/src/store/terminals/terminal-shutdown-guards.ts b/src/renderer/src/store/terminals/terminal-shutdown-guards.ts index 83342bd5041..0eba0fda927 100644 --- a/src/renderer/src/store/terminals/terminal-shutdown-guards.ts +++ b/src/renderer/src/store/terminals/terminal-shutdown-guards.ts @@ -13,6 +13,7 @@ import { settleDeferredPtyShutdownExits } from '@/components/terminal-pane/pty-shutdown-exit-deferral' import type { TerminalStoreGet, TerminalStoreSet } from './terminal-state' +import { copyOnWriteRecord } from '../copy-on-write-record' export type TerminalShutdownGuardController = { commitHandlerSnapshots: () => void @@ -48,18 +49,22 @@ export function createTerminalShutdownGuardController({ let partialRendererStopSettled = false const markShutdownPending = (): void => { + // Why the early return: tearing down a worktree whose panes already exited passes + // no guard ids, and the spreads below would still hand both maps a new identity. + if (exitGuardPtyIds.length === 0) { + return + } set((state) => { const pendingPtyShutdownIds = { ...state.pendingPtyShutdownIds } + // Why copy-on-write: re-guarding an already-suppressed pty writes the same `true`. + const suppressedPtyExitIds = copyOnWriteRecord(state.suppressedPtyExitIds) for (const ptyId of exitGuardPtyIds) { pendingPtyShutdownIds[ptyId] = (pendingPtyShutdownIds[ptyId] ?? 0) + 1 + if (state.suppressedPtyExitIds[ptyId] !== true) { + suppressedPtyExitIds.set(ptyId, true) + } } - return { - suppressedPtyExitIds: { - ...state.suppressedPtyExitIds, - ...Object.fromEntries(exitGuardPtyIds.map((ptyId) => [ptyId, true] as const)) - }, - pendingPtyShutdownIds - } + return { suppressedPtyExitIds: suppressedPtyExitIds.read(), pendingPtyShutdownIds } }) } diff --git a/src/shared/process-table-snapshot-reader.ts b/src/shared/process-table-snapshot-reader.ts index 962a24c7f48..06d3407f3b9 100644 --- a/src/shared/process-table-snapshot-reader.ts +++ b/src/shared/process-table-snapshot-reader.ts @@ -6,7 +6,9 @@ import { PS_ARGS, PS_MAX_BUFFER_BYTES, ProcessTableCaptureError, + SHELL_FOREGROUND_PS_ARGS, parseProcessTableRows, + parseShellForegroundRows, parseStrictProcessTableRows, type ProcessTableRow } from './process-table-snapshot' @@ -250,29 +252,47 @@ async function readLinuxProcessStartTimes( return result } +async function captureProcessTable(args: readonly string[]): Promise { + let stdout: string + try { + ;({ stdout } = await execFile('ps', [...args], { + encoding: 'utf-8', + timeout: PS_TIMEOUT_MS, + maxBuffer: PS_MAX_BUFFER_BYTES + })) + } catch (error) { + // A ceiling hit is truncation, not absence: name it in the domain vocabulary. + if ((error as { code?: unknown } | null)?.code === 'ERR_CHILD_PROCESS_STDIO_MAXBUFFER') { + throw new ProcessTableCaptureError('capture_truncated') + } + throw error + } + return assertWholeCapture(stdout) +} + const processTableReader = createProcessTableSnapshotReader({ runPs: async () => { - let stdout: string - try { - ;({ stdout } = await execFile('ps', [...PS_ARGS], { - encoding: 'utf-8', - timeout: PS_TIMEOUT_MS, - maxBuffer: PS_MAX_BUFFER_BYTES - })) - } catch (error) { - // A ceiling hit is truncation, not absence: name it in the domain vocabulary. - if ((error as { code?: unknown } | null)?.code === 'ERR_CHILD_PROCESS_STDIO_MAXBUFFER') { - throw new ProcessTableCaptureError('capture_truncated') - } - throw error - } - const baseCapture = createProcessTableCapture(assertWholeCapture(stdout)) + const stdout = await captureProcessTable(PS_ARGS) + const baseCapture = createProcessTableCapture(stdout) const startTimesByPid = await readLinuxProcessStartTimes(baseCapture.lenient()) return createProcessTableCapture(stdout, startTimesByPid, process.platform === 'linux') }, now: () => Date.now() }) +// Its own reader, not a column-set flag on the shared one: terminal-name resolution dominates +// macOS capture time, and a shell proof must not queue behind a full capture it cannot use. +const shellForegroundReader = createProcessTableSnapshotReader({ + runPs: async () => parseShellForegroundRows(await captureProcessTable(SHELL_FOREGROUND_PS_ARGS)), + now: () => Date.now() +}) + +export async function getFreshShellForegroundSnapshot(): Promise { + return process.platform === 'darwin' + ? shellForegroundReader.getFreshSnapshot() + : getFreshProcessTableSnapshot() +} + export async function getProcessTableSnapshot(): Promise { return (await processTableReader.getSnapshot()).lenient() } @@ -334,4 +354,5 @@ export async function getStrictProcessTableSnapshotWithAge(): Promise<{ export function resetProcessTableSnapshotForTests(): void { processTableReader.reset() + shellForegroundReader.reset() } diff --git a/src/shared/process-table-snapshot.ts b/src/shared/process-table-snapshot.ts index 3b3236079c4..81a669e6d97 100644 --- a/src/shared/process-table-snapshot.ts +++ b/src/shared/process-table-snapshot.ts @@ -38,6 +38,47 @@ export const CHEAP_PS_ARGS = ( : ['-axo', 'pid=,ppid=,pgid=,tpgid=,stat='] ) as readonly string[] +/** + * Shell-proof tier: job control plus argv, dropping only the columns the shell predicate never + * reads — macOS `tty=` (0.29s of the 0.34s on a 1,900-process Mac) and the start marker. Enough + * to name a pane's foreground process; never enough to correlate a pid across captures. + */ +export const SHELL_FOREGROUND_PS_ARGS = [ + '-axo', + 'pid=,ppid=,pgid=,tpgid=,stat=,command=' +] as readonly string[] + +/** + * Parse a {@link SHELL_FOREGROUND_PS_ARGS} capture, anchored to exactly those columns. + * Not {@link parseProcessTableRows}: with no `tty=` to absorb it, that parser's optional + * tty/start pair eats the head of an argv shaped `python 3 app.py`, and a command-less zombie + * row parses into a garbage pid/stat pair. + * + * Lenient per row like its siblings, but a capture yielding none is unreadable rather than a + * machine with no processes: the shell proof must not read that as "the shell is gone". + */ +export function parseShellForegroundRows(stdout: string): ProcessTableRow[] { + const rows: ProcessTableRow[] = [] + for (const rawLine of stdout.split(/\r?\n/)) { + const match = rawLine.trim().match(/^(\d+)\s+(\d+)\s+(-?\d+)\s+(-?\d+)\s+(\S+)\s+(.+)$/) + const pid = match ? Number(match[1]) : 0 + if (match && Number.isSafeInteger(pid) && pid > 0) { + rows.push({ + pid, + ppid: Number(match[2]), + pgid: Number(match[3]), + tpgid: Number(match[4]), + stat: match[5], + command: match[6] + }) + } + } + if (rows.length === 0) { + throw new ProcessTableCaptureError('empty_capture') + } + return rows +} + export type CheapProcessTableRow = { pid: number ppid: number diff --git a/src/shared/shell-foreground-snapshot.test.ts b/src/shared/shell-foreground-snapshot.test.ts new file mode 100644 index 00000000000..e4cfa5f53a5 --- /dev/null +++ b/src/shared/shell-foreground-snapshot.test.ts @@ -0,0 +1,77 @@ +import { afterEach, beforeEach, expect, it, vi } from 'vitest' + +const { execFileMock } = vi.hoisted(() => ({ execFileMock: vi.fn() })) +vi.mock('node:child_process', () => ({ execFile: execFileMock })) + +import { + getFreshShellForegroundSnapshot, + getProcessTableSnapshot, + resetProcessTableSnapshotForTests +} from './process-table-snapshot-reader' +import { parseShellForegroundRows } from './process-table-snapshot' + +type Callback = (error: Error | null, result: { stdout: string; stderr: string }) => void +const platform = Object.getOwnPropertyDescriptor(process, 'platform')! +const shell = '100 99 100 100 Ss+ /bin/zsh -l' + +beforeEach(() => { + Object.defineProperty(process, 'platform', { value: 'darwin' }) + execFileMock.mockReset() + resetProcessTableSnapshotForTests() +}) +afterEach(() => Object.defineProperty(process, 'platform', platform)) + +it('answers concurrent shell proofs without waiting for a pending full capture', async () => { + let finishFull!: Callback + execFileMock.mockImplementation((_program, args: string[], _options, callback: Callback) => { + if (args[1]?.includes('tty=')) { + finishFull = callback + } else { + expect(args).toEqual(['-axo', 'pid=,ppid=,pgid=,tpgid=,stat=,command=']) + callback(null, { stdout: shell, stderr: '' }) + } + }) + const full = getProcessTableSnapshot() + const [first, second] = await Promise.all([ + getFreshShellForegroundSnapshot(), + getFreshShellForegroundSnapshot() + ]) + expect(first).toEqual([ + { pid: 100, ppid: 99, pgid: 100, tpgid: 100, stat: 'Ss+', command: '/bin/zsh -l' } + ]) + expect(second).toBe(first) + expect(execFileMock).toHaveBeenCalledTimes(2) + finishFull(null, { stdout: shell, stderr: '' }) + await full + await getFreshShellForegroundSnapshot() + expect(execFileMock).toHaveBeenCalledTimes(3) +}) + +it('requires a new capture after an earlier shell proof has started', async () => { + const callbacks: Callback[] = [] + execFileMock.mockImplementation((_program, _args, _options, callback: Callback) => { + callbacks.push(callback) + }) + const first = getFreshShellForegroundSnapshot() + await vi.waitFor(() => expect(callbacks).toHaveLength(1)) + const second = getFreshShellForegroundSnapshot() + callbacks[0]!(null, { stdout: shell, stderr: '' }) + await first + await vi.waitFor(() => expect(callbacks).toHaveLength(2)) + callbacks[1]!(null, { stdout: shell.replace('Ss+', 'Ss'), stderr: '' }) + expect((await second)[0]?.stat).toBe('Ss') +}) + +// With no `tty=` column to absorb them, the shared parser read `python`/`3` as tty/start. +it('keeps an argv whose second token is numeric', () => { + expect(parseShellForegroundRows('101 100 101 101 S+ /usr/bin/python 3 app.py')).toEqual([ + { pid: 101, ppid: 100, pgid: 101, tpgid: 101, stat: 'S+', command: '/usr/bin/python 3 app.py' } + ]) +}) + +it('rejects an unreadable shell capture', async () => { + execFileMock.mockImplementation((_program, _args, _options, callback: Callback) => { + callback(null, { stdout: '', stderr: '' }) + }) + await expect(getFreshShellForegroundSnapshot()).rejects.toThrow('empty_capture') +}) diff --git a/tests/e2e/ssh-docker-transport-drop-recovery.spec.ts b/tests/e2e/ssh-docker-transport-drop-recovery.spec.ts index c64761ede80..cbdf179e82a 100644 --- a/tests/e2e/ssh-docker-transport-drop-recovery.spec.ts +++ b/tests/e2e/ssh-docker-transport-drop-recovery.spec.ts @@ -163,8 +163,7 @@ test.describe('SSH transport drop recovery', () => { } }) - // #18018: local authority-aware recovery still loses the flooded pane's relay channel. - test.fixme('stays bounded when a disconnected shell floods its pty', async ({ + test('stays bounded when a disconnected shell floods its pty', async ({ orcaPage }, testInfo) => { test.slow()