From 6ea99f66075dfa212e7099eb0630b429eaaa3697 Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Fri, 7 Aug 2026 21:11:01 -0700 Subject: [PATCH] fix(runtime): isolate same-path folder workspace PTY identity (#12474) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Folder-project workspace ids (`repoId::/path::workspace:`) were compared via the suffix-stripping `splitWorktreeIdForFilesystem`, so every workspace sharing one directory compared equal at ~35 runtime call sites — PTYs leaked between siblings and paired/mobile clients hung on "Loading terminal". Both identity helpers now use the suffix-preserving `splitWorktreeId`, `stopTerminalsForWorktree` routes through the shared helper, and `findResolvedWorktreeIdForPath` gains a `targetWorktreeId` tie-break. Co-authored-by: dgk-dev --- .../folder-workspace-pty-identity.test.ts | 228 ++++++++++++++++++ src/main/runtime/orca-runtime.test.ts | 44 ++++ src/main/runtime/orca-runtime.ts | 45 ++-- 3 files changed, 300 insertions(+), 17 deletions(-) create mode 100644 src/main/runtime/folder-workspace-pty-identity.test.ts diff --git a/src/main/runtime/folder-workspace-pty-identity.test.ts b/src/main/runtime/folder-workspace-pty-identity.test.ts new file mode 100644 index 000000000..71c18b941 --- /dev/null +++ b/src/main/runtime/folder-workspace-pty-identity.test.ts @@ -0,0 +1,228 @@ +import { describe, expect, it } from 'vitest' +import { getDefaultWorkspaceSession } from '../../shared/constants' +import type { RuntimeClientEvent } from '../../shared/runtime-client-events' +import { makePaneKey } from '../../shared/stable-pane-id' +import type { WorkspaceSessionState } from '../../shared/types' +import { FOLDER_WORKSPACE_INSTANCE_SEPARATOR } from '../../shared/worktree-id' +import { OrcaRuntimeService } from './orca-runtime' + +// Folder projects back several workspaces with ONE directory; only the +// `::workspace:` suffix separates them, so runtime PTY identity must keep it. +const REPO_ID = 'repo-1' +const FOLDER_PATH = '/tmp/folder-project' +const ROOT_ID = `${REPO_ID}::${FOLDER_PATH}` +const WORKSPACE_A = `${ROOT_ID}${FOLDER_WORKSPACE_INSTANCE_SEPARATOR}aaaaaaaa-aaaa-4aaa-8aaa-aaaaaaaaaaaa` +const WORKSPACE_B = `${ROOT_ID}${FOLDER_WORKSPACE_INSTANCE_SEPARATOR}bbbbbbbb-bbbb-4bbb-8bbb-bbbbbbbbbbbb` +const PTY_A = `${WORKSPACE_A}@@pty-a` +const PTY_B = `${WORKSPACE_B}@@pty-b` +const LEAF_B = 'cccccccc-cccc-4ccc-8ccc-cccccccccccc' + +const REPO = { + id: REPO_ID, + path: FOLDER_PATH, + displayName: 'folder-project', + badgeColor: 'blue', + addedAt: 1, + kind: 'folder' +} as const + +type RuntimeInternals = { + buildResolvedWorktreeFromId: (worktreeId: string) => unknown + refreshPtyWorktreeRecordsWithControllerInventory: ( + resolvedWorktrees: unknown[], + targetWorktreeId?: string | null + ) => Promise<{ livePtyIds: Set } | null> + getLivePtyIdsForWorktree: (worktreeId: string) => Set + hasExactPersistedTerminalSurfaceIdentity: (expected: { + worktreeId: string + tabId: string + leafId: string + ptyId: string + incarnationId: string + }) => boolean + ptysById: Map + recordPtyWorktree: (ptyId: string, worktreeId: string, state?: Record) => unknown + onClientEvent: (listener: (event: RuntimeClientEvent) => void) => () => void + onPtyExit: (ptyId: string, exitCode: number) => void + sleepTerminalsForWorktree: (worktreeSelector: string) => Promise + acquireWorktreeTerminalSpawn: (worktreeId?: string) => Promise<() => void> + getTerminalSleepClientEventSnapshot: () => RuntimeClientEvent[] +} + +const OWNED_CONTROLLER_SESSIONS = [ + { id: PTY_A, worktreeId: WORKSPACE_A, cwd: FOLDER_PATH, title: 'a' }, + { id: PTY_B, worktreeId: WORKSPACE_B, cwd: FOLDER_PATH, title: 'b' } +] + +function createRuntimeInternals( + options: { + session?: WorkspaceSessionState + sessions?: unknown[] + // Consecutive controller inventories, so a sleep sees its PTY then sees it gone. + processLists?: unknown[][] + } = {} +): RuntimeInternals { + const meta: Record> = { + [WORKSPACE_A]: { hostId: 'local' }, + [WORKSPACE_B]: { hostId: 'local' } + } + const store = { + getRepos: () => [REPO], + getRepo: (id: string) => (id === REPO_ID ? REPO : undefined), + getAllWorktreeMeta: () => meta, + getWorktreeMeta: (worktreeId: string) => meta[worktreeId], + setWorktreeMeta: (worktreeId: string, patch: Record) => { + meta[worktreeId] = { ...meta[worktreeId], ...patch } + return meta[worktreeId] + }, + getWorkspaceSession: () => options.session ?? getDefaultWorkspaceSession(), + setWorkspaceSession: () => {}, + flushOrThrow: () => {} + } as never + const runtime = new OrcaRuntimeService(store) + runtime.setPtyController({ + write: () => true, + kill: () => true, + stopAndWait: async (ptyId: string) => { + runtime.onPtyExit(ptyId, -1) + return true + }, + getForegroundProcess: async () => null, + listProcesses: async () => + options.processLists + ? (options.processLists.shift() ?? []) + : (options.sessions ?? OWNED_CONTROLLER_SESSIONS) + } as never) + return runtime as unknown as RuntimeInternals +} + +/** + * Resolves to 'blocked' only if `pending` has not settled once the microtask queue drains — + * an uncontended mutation lease is pure-promise, so this never waits on wall-clock time. + */ +async function raceAgainstMicrotaskDrain(pending: Promise): Promise { + return await Promise.race([ + pending.then(() => 'acquired'), + new Promise((resolve) => setTimeout(() => resolve('blocked'), 0)) + ]) +} + +describe('folder workspaces sharing one directory', () => { + it('keeps each workspace instance bound to its own controller PTY', async () => { + const internals = createRuntimeInternals() + const resolvedWorktrees = [WORKSPACE_A, WORKSPACE_B].map((id) => + internals.buildResolvedWorktreeFromId(id) + ) + + await internals.refreshPtyWorktreeRecordsWithControllerInventory(resolvedWorktrees) + + expect(internals.ptysById.get(PTY_A)?.worktreeId).toBe(WORKSPACE_A) + expect(internals.ptysById.get(PTY_B)?.worktreeId).toBe(WORKSPACE_B) + }) + + it('selects only the targeted workspace instance from a controller inventory', async () => { + const internals = createRuntimeInternals() + const resolvedWorktrees = [WORKSPACE_A, WORKSPACE_B].map((id) => + internals.buildResolvedWorktreeFromId(id) + ) + + const inventory = await internals.refreshPtyWorktreeRecordsWithControllerInventory( + resolvedWorktrees, + WORKSPACE_B + ) + + expect([...(inventory?.livePtyIds ?? [])]).toEqual([PTY_B]) + }) + + it('reports live PTYs per workspace instance and for the folder root separately', () => { + const internals = createRuntimeInternals() + internals.recordPtyWorktree(PTY_A, WORKSPACE_A, { connected: true }) + internals.recordPtyWorktree(PTY_B, WORKSPACE_B, { connected: true }) + internals.recordPtyWorktree('pty-root', ROOT_ID, { connected: true }) + + expect([...internals.getLivePtyIdsForWorktree(WORKSPACE_A)]).toEqual([PTY_A]) + expect([...internals.getLivePtyIdsForWorktree(WORKSPACE_B)]).toEqual([PTY_B]) + expect([...internals.getLivePtyIdsForWorktree(ROOT_ID)]).toEqual(['pty-root']) + }) + + it('resolves a persisted terminal surface while a sibling instance also has tabs', () => { + const paneKey = makePaneKey('tab-b', LEAF_B) + const session: WorkspaceSessionState = { + ...getDefaultWorkspaceSession(), + tabsByWorktree: { + [WORKSPACE_A]: [{ id: 'tab-a', worktreeId: WORKSPACE_A, title: 'A' }], + [WORKSPACE_B]: [{ id: 'tab-b', worktreeId: WORKSPACE_B, title: 'B' }] + } as never, + terminalLayoutsByTabId: { + 'tab-b': { root: null, activeLeafId: LEAF_B, ptyIdsByLeafId: { [LEAF_B]: PTY_B } } + } as never, + terminalPtyIncarnationsByPaneKey: { [paneKey]: 'inc-b' } + } + const internals = createRuntimeInternals({ session }) + + expect( + internals.hasExactPersistedTerminalSurfaceIdentity({ + worktreeId: WORKSPACE_B, + tabId: 'tab-b', + leafId: LEAF_B, + ptyId: PTY_B, + incarnationId: 'inc-b' + }) + ).toBe(true) + }) + + it('attributes an unowned cwd-only PTY to the targeted instance, not a sibling', async () => { + const internals = createRuntimeInternals({ + sessions: [{ id: 'legacy-cwd-pty', cwd: `${FOLDER_PATH}/src`, title: 'legacy' }] + }) + const resolvedWorktrees = [WORKSPACE_A, WORKSPACE_B].map((id) => + internals.buildResolvedWorktreeFromId(id) + ) + + const inventory = await internals.refreshPtyWorktreeRecordsWithControllerInventory( + resolvedWorktrees, + WORKSPACE_B + ) + + expect([...(inventory?.livePtyIds ?? [])]).toEqual(['legacy-cwd-pty']) + expect(internals.ptysById.get('legacy-cwd-pty')?.worktreeId).toBe(WORKSPACE_B) + }) + + it('leaves a sleeping instance asleep when a sibling instance spawns a terminal', async () => { + const internals = createRuntimeInternals({ + processLists: [[{ id: PTY_A, worktreeId: WORKSPACE_A, cwd: FOLDER_PATH, title: 'a' }], []] + }) + const events: RuntimeClientEvent[] = [] + internals.onClientEvent((event) => events.push(event)) + + await internals.sleepTerminalsForWorktree(`id:${WORKSPACE_A}`) + const release = await internals.acquireWorktreeTerminalSpawn(WORKSPACE_B) + release() + + // A shared identity key would wake A's stopped terminals on B's spawn and tell clients so. + expect( + events.filter( + (event) => event.type === 'worktreeTerminalSleepState' && event.phase === 'woken' + ) + ).toEqual([]) + expect(internals.getTerminalSleepClientEventSnapshot()).toEqual([ + expect.objectContaining({ worktreeId: WORKSPACE_A, phase: 'committed', ptyIds: [PTY_A] }) + ]) + }) + + it('does not serialize a sibling instance behind this instance held terminal mutation', async () => { + const internals = createRuntimeInternals() + const releaseA = await internals.acquireWorktreeTerminalSpawn(WORKSPACE_A) + + const spawnB = internals.acquireWorktreeTerminalSpawn(WORKSPACE_B) + const spawnSecondA = internals.acquireWorktreeTerminalSpawn(WORKSPACE_A) + + expect(await raceAgainstMicrotaskDrain(spawnB)).toBe('acquired') + // Control: same-instance mutations must still queue, so 'acquired' above is not a free pass. + expect(await raceAgainstMicrotaskDrain(spawnSecondA)).toBe('blocked') + + releaseA() + ;(await spawnB)() + ;(await spawnSecondA)() + }) +}) diff --git a/src/main/runtime/orca-runtime.test.ts b/src/main/runtime/orca-runtime.test.ts index 90b7bb45f..b3e3155c8 100644 --- a/src/main/runtime/orca-runtime.test.ts +++ b/src/main/runtime/orca-runtime.test.ts @@ -3541,6 +3541,50 @@ describe('OrcaRuntimeService', () => { expect(terminals.terminals[0]?.worktreePath).toBe(TEST_FOLDER_WORKSPACE_PATH) }) + it('keeps same-path folder workspace instance PTYs scoped to their exact ids', async () => { + const firstWorktreeId = `${TEST_REPO_ID}::${TEST_FOLDER_WORKSPACE_PATH}${FOLDER_WORKSPACE_INSTANCE_SEPARATOR}11111111-1111-4111-8111-111111111111` + const secondWorktreeId = `${TEST_REPO_ID}::${TEST_FOLDER_WORKSPACE_PATH}${FOLDER_WORKSPACE_INSTANCE_SEPARATOR}22222222-2222-4222-8222-222222222222` + vi.mocked(listWorktrees).mockClear() + vi.mocked(listWorktrees).mockRejectedValue( + new Error('folder explicit-id fallback should not rescan worktrees') + ) + const runtime = createRuntime() + runtime.setPtyController({ + write: () => true, + kill: () => true, + getForegroundProcess: async () => null, + listProcesses: async () => [ + { + id: 'first-folder-pty', + cwd: TEST_FOLDER_WORKSPACE_PATH, + title: 'first', + worktreeId: firstWorktreeId + }, + { + id: 'second-folder-pty', + cwd: TEST_FOLDER_WORKSPACE_PATH, + title: 'second', + worktreeId: secondWorktreeId + } + ] + }) + + const terminals = await runtime.listTerminals(`id:${secondWorktreeId}`) + + expect(listWorktrees).not.toHaveBeenCalled() + expect(terminals.terminals).toHaveLength(1) + expect(terminals.terminals[0]).toMatchObject({ + ptyId: 'second-folder-pty', + worktreeId: secondWorktreeId, + worktreePath: TEST_FOLDER_WORKSPACE_PATH + }) + const internals = runtime as unknown as { + ptysById: Map + } + expect(internals.ptysById.get('first-folder-pty')).toBeUndefined() + expect(internals.ptysById.get('second-folder-pty')?.worktreeId).toBe(secondWorktreeId) + }) + it('routes PTY output through the PTY leaf index in large terminal graphs', () => { const runtime = new OrcaRuntimeService(store) const liveLeafCount = 2773 diff --git a/src/main/runtime/orca-runtime.ts b/src/main/runtime/orca-runtime.ts index e6c7ced50..38ece989d 100644 --- a/src/main/runtime/orca-runtime.ts +++ b/src/main/runtime/orca-runtime.ts @@ -27302,18 +27302,9 @@ export class OrcaRuntimeService { return { stopped: 0 } } // Preserve folder-instance suffixes while normalizing cross-platform path spelling. - const parsedTarget = splitWorktreeId(worktree.id) const ownsWorktree = options.resolvedWorktreeId - ? (candidate: string | undefined): boolean => { - if (!candidate) { - return false - } - const parsedCandidate = splitWorktreeId(candidate) - return parsedCandidate && parsedTarget - ? parsedCandidate.repoId === parsedTarget.repoId && - runtimePathsEqual(parsedCandidate.worktreePath, parsedTarget.worktreePath) - : candidate === worktree.id - } + ? (candidate: string | undefined): boolean => + candidate ? runtimeWorktreeIdsEqual(candidate, worktree.id) : false : (candidate: string | undefined): boolean => candidate === worktree.id const ownsHost = (ptyId: string, connectionId?: string | null): boolean => { if (options.resolvedRuntimeEnvironmentId !== undefined) { @@ -29365,7 +29356,7 @@ export class OrcaRuntimeService { : (session.worktreeId ?? persistedWorktree?.id ?? inferredWorktreeId ?? - findResolvedWorktreeIdForPath(resolvedWorktrees, session.cwd)) + findResolvedWorktreeIdForPath(resolvedWorktrees, session.cwd, targetWorktreeId)) const persistedSurface = persistedIndexes.surfaceByPtyId.get(session.id) const restoresExactSurface = persistedSurface && @@ -37003,9 +36994,16 @@ function runtimePathsEqual(left: string, right: string): boolean { return normalizeRuntimePathForComparison(left) === normalizeRuntimePathForComparison(right) } +/** + * Why: runtime identity is per *workspace*, not per checkout dir. Folder projects back + * several independent workspaces with one directory, separated only by the + * `::workspace:` suffix that filesystem callers must strip; stripping it here + * instead lets one session steal a sibling's PTYs. Normalize only path spelling, so + * Windows/WSL/SSH ids still match themselves across hosts. + */ function runtimeWorktreeIdsEqual(left: string, right: string): boolean { - const parsedLeft = splitWorktreeIdForFilesystem(left) - const parsedRight = splitWorktreeIdForFilesystem(right) + const parsedLeft = splitWorktreeId(left) + const parsedRight = splitWorktreeId(right) return parsedLeft && parsedRight ? parsedLeft.repoId === parsedRight.repoId && runtimePathsEqual(parsedLeft.worktreePath, parsedRight.worktreePath) @@ -37013,7 +37011,8 @@ function runtimeWorktreeIdsEqual(left: string, right: string): boolean { } function runtimeWorktreeIdentityKey(worktreeId: string): string { - const parsed = splitWorktreeIdForFilesystem(worktreeId) + // Same suffix rule: this keys PTY refresh, sleep, and mutation-queue state per session. + const parsed = splitWorktreeId(worktreeId) return parsed ? `${parsed.repoId}\0${normalizeRuntimePathForComparison(parsed.worktreePath)}` : worktreeId @@ -37298,7 +37297,8 @@ function includeTargetResolvedWorktree( function findResolvedWorktreeIdForPath( resolvedWorktrees: ResolvedWorktree[], - cwd: string + cwd: string, + targetWorktreeId?: string | null ): string | null { if (!cwd) { return null @@ -37306,7 +37306,18 @@ function findResolvedWorktreeIdForPath( const matches = resolvedWorktrees .filter((worktree) => isPathInsideOrEqual(worktree.path, cwd)) .sort((left, right) => right.path.length - left.path.length) - return matches[0]?.id ?? null + // Why: a cwd cannot distinguish folder-workspace siblings, which all share one + // directory. Break that tie toward the caller's target instead of store order, + // so an unattributed PTY still lands in the workspace being listed. Only ties at + // the deepest path qualify — a nested worktree must still beat its parent. + const deepest = matches.filter((worktree) => worktree.path.length === matches[0]?.path.length) + return ( + (deepest.length > 1 + ? deepest.find((worktree) => worktree.id === targetWorktreeId)?.id + : undefined) ?? + matches[0]?.id ?? + null + ) } function getLeafWorktreeStatus(