From 2779dfeb2b8fabe1e006820ac2a1d3937a202cd4 Mon Sep 17 00:00:00 2001 From: OrcaWin Date: Sun, 11 Oct 2026 12:05:22 -0700 Subject: [PATCH] refactor(pty): drop SSH spawn, liveness and kill branches (#27529) * refactor(pty): drop SSH spawn, liveness and kill branches * test(pty): drive the commit-durability tests through local PTYs only * test(pty): drop the SSH host arm from the exit-durability fixture --------- Co-authored-by: m4air --- .../claude-launch-routing.test.ts | 5 +- .../claude-profile-installed-router.ts | 5 +- .../pty-persisted-incarnation-repair.test.ts | 3 +- src/main/ipc/pty-ssh-undelivered-kill.test.ts | 239 --------------- .../ipc/pty/host-env/fresh-spawn-routing.ts | 3 +- src/main/ipc/pty/ipc/renderer-kill.ts | 21 +- src/main/ipc/pty/ipc/spawn-commit-persist.ts | 36 +-- ...spawn-commit-ssh-lease-cardinality.test.ts | 281 ------------------ src/main/ipc/pty/ipc/spawn-commit.ts | 91 +++--- src/main/ipc/pty/ipc/spawn-env.ts | 21 +- src/main/ipc/pty/ipc/spawn-execute.ts | 49 +-- src/main/ipc/pty/ipc/spawn-preflight.ts | 58 ++-- src/main/ipc/pty/ipc/spawn-state.ts | 2 - .../pty/pane/pane-owner-replacement.test.ts | 52 +--- .../ipc/pty/pane/pane-owner-replacement.ts | 7 +- src/main/ipc/pty/pane/spawn-registration.ts | 9 +- src/main/ipc/pty/pane/ssh-pane-lease-claim.ts | 44 --- src/main/ipc/pty/pane/stable-owner.ts | 43 +-- ...ble-pane-absence-death-certificate.test.ts | 53 +--- ...ts => stable-pane-absence-respawn.test.ts} | 37 ++- src/main/ipc/pty/provider/liveness.ts | 66 +--- src/main/ipc/pty/provider/ownership-state.ts | 11 +- .../provider/registry-host-routing.test.ts | 11 +- src/main/ipc/pty/runtime/controller.ts | 7 - src/main/ipc/pty/runtime/kill.ts | 30 +- src/main/ipc/pty/runtime/spawn-commit.ts | 115 +++---- src/main/ipc/pty/runtime/spawn-execute.ts | 52 +--- src/main/ipc/pty/runtime/spawn-preflight.ts | 75 ++--- src/main/ipc/pty/runtime/spawn-state.ts | 2 - .../ipc/pty/runtime/undelivered-ssh-kill.ts | 68 ----- .../pty-reattach-failure-routing.test.ts | 20 +- .../pty-spawn-exit-durability.test.ts | 31 +- src/main/providers/ssh-pty-errors.ts | 61 ---- ...ss-terminal-dispose-write-ordering.test.ts | 4 +- ...me-maybe-hydrate-headless-from-renderer.ts | 4 +- src/main/runtime/orca-runtime-on-pty-data.ts | 9 +- ...a-runtime-prepare-pty-execution-context.ts | 37 --- ...runtime-stop-explicitly-closed-tab-ptys.ts | 16 +- ...-runtime-terminal-close-continuity.test.ts | 30 -- src/main/runtime/orca-runtime.ts | 1 - ...ation-mailbox-notification-test-harness.ts | 7 +- ...stration-message-delivery-identity.test.ts | 7 +- .../runtime-pty-controller-contract.ts | 3 - .../runtime/runtime-terminal-contracts.ts | 5 - 44 files changed, 258 insertions(+), 1473 deletions(-) delete mode 100644 src/main/ipc/pty-ssh-undelivered-kill.test.ts delete mode 100644 src/main/ipc/pty/ipc/spawn-commit-ssh-lease-cardinality.test.ts delete mode 100644 src/main/ipc/pty/pane/ssh-pane-lease-claim.ts rename src/main/ipc/pty/pane/{stable-pane-relay-absence-respawn.test.ts => stable-pane-absence-respawn.test.ts} (87%) delete mode 100644 src/main/ipc/pty/runtime/undelivered-ssh-kill.ts delete mode 100644 src/main/providers/ssh-pty-errors.ts diff --git a/src/main/claude-accounts/claude-launch-routing.test.ts b/src/main/claude-accounts/claude-launch-routing.test.ts index a9eb8e33b2b..d8fc2a0b3cb 100644 --- a/src/main/claude-accounts/claude-launch-routing.test.ts +++ b/src/main/claude-accounts/claude-launch-routing.test.ts @@ -71,9 +71,8 @@ async function launchHomes(f: ReturnType) { const chatEnv: Record = {} return { terminal: (await f.router.prepareLaunch()).configDir, - terminalEnv: withClaudeProfileTerminalEnv>({}, null, { - runtime: 'host' - }).CLAUDE_CONFIG_DIR, + terminalEnv: withClaudeProfileTerminalEnv>({}, { runtime: 'host' }) + .CLAUDE_CONFIG_DIR, chat: await resolveClaudeStructuredLaunchHome(f.router, chatEnv, '/recorded'), chatEnv: chatEnv.CLAUDE_CONFIG_DIR, chatRecord: resolveStructuredClaudeAccountHomePath({ diff --git a/src/main/claude-accounts/claude-profile-installed-router.ts b/src/main/claude-accounts/claude-profile-installed-router.ts index f622d60e9df..38c986db9a8 100644 --- a/src/main/claude-accounts/claude-profile-installed-router.ts +++ b/src/main/claude-accounts/claude-profile-installed-router.ts @@ -47,13 +47,12 @@ export function getClaudeWslProfileRouter(): ClaudeWslProfileRouter | undefined return installedWsl } -/** A local or WSL pane's env with the routed account's pointer added; SSH panes keep theirs. */ +/** A local or WSL pane's env with the routed account's pointer added. */ export function withClaudeProfileTerminalEnv | undefined>( env: Env, - connectionId: string | null | undefined, target: ClaudeAccountSelectionTarget ): Env | Record { - const profileEnv = connectionId ? undefined : installed?.terminalEnv(target) + const profileEnv = installed?.terminalEnv(target) return profileEnv ? { ...env, ...profileEnv } : env } diff --git a/src/main/ipc/pty-persisted-incarnation-repair.test.ts b/src/main/ipc/pty-persisted-incarnation-repair.test.ts index c057d6ec5d2..8d2ff0b1102 100644 --- a/src/main/ipc/pty-persisted-incarnation-repair.test.ts +++ b/src/main/ipc/pty-persisted-incarnation-repair.test.ts @@ -175,8 +175,7 @@ describe('registerPtyHandlers', () => { ptyId: 'pty-persisted-owner', incarnationId: 'inc-stale-owner' } - }), - undefined + }) ) expect( mainWindow.webContents.send.mock.calls.filter(([channel]) => channel === 'pty:spawned') diff --git a/src/main/ipc/pty-ssh-undelivered-kill.test.ts b/src/main/ipc/pty-ssh-undelivered-kill.test.ts deleted file mode 100644 index 8390c0f5286..00000000000 --- a/src/main/ipc/pty-ssh-undelivered-kill.test.ts +++ /dev/null @@ -1,239 +0,0 @@ -import { describe, expect, it, vi } from 'vitest' -import { setupPtyIpcSuite } from './pty-ipc-test-harness' -import { TerminalIntentionalStops } from '../runtime/terminal-intentional-stops' -import { - registerPtyHandlers, - deletePtyOwnership, - setPtyOwnership, - restorePtyIncarnation -} from './pty' - -vi.mock('electron', () => import('./pty-ipc-mock-registry').then((m) => m.electronModuleMock())) -vi.mock('fs', () => import('./pty-ipc-mock-registry').then((m) => m.fsModuleMock())) -vi.mock('node-pty', () => import('./pty-ipc-mock-registry').then((m) => m.nodePtyModuleMock())) -vi.mock('node:child_process', async (importOriginal) => - (await import('./pty-ipc-mock-registry')).childProcessModuleMock(await importOriginal()) -) -vi.mock('../opencode/hook-service', () => - import('./pty-ipc-mock-registry').then((m) => m.openCodeHookServiceModuleMock()) -) -vi.mock('../mimo/hook-service', () => - import('./pty-ipc-mock-registry').then((m) => m.mimoHookServiceModuleMock()) -) -vi.mock('../agent-hooks/server', () => - import('./pty-ipc-mock-registry').then((m) => m.agentHookServerModuleMock()) -) -vi.mock('../pi/titlebar-extension-service', () => - import('./pty-ipc-mock-registry').then((m) => m.piTitlebarExtensionModuleMock()) -) -vi.mock('../pwsh', () => import('./pty-ipc-mock-registry').then((m) => m.pwshModuleMock())) -vi.mock('../wsl', async (importOriginal) => - (await import('./pty-ipc-mock-registry')).wslModuleMock(await importOriginal()) -) -vi.mock('../telemetry/client', () => - import('./pty-ipc-mock-registry').then((m) => m.telemetryClientModuleMock()) -) -vi.mock('../telemetry/classify-error', () => - import('./pty-ipc-mock-registry').then((m) => m.classifyErrorModuleMock()) -) -vi.mock('../cli/linux-terminal-orca-cli-shim', () => - import('./pty-ipc-mock-registry').then((m) => m.linuxCliShimModuleMock()) -) -vi.mock('../memory/pty-registry', () => - import('./pty-ipc-mock-registry').then((m) => m.ptyRegistryModuleMock()) -) -vi.mock('../agent-hooks/migration-unsupported-pty-state', () => - import('./pty-ipc-mock-registry').then((m) => m.migrationUnsupportedPtyModuleMock()) -) -vi.mock('../codex/codex-pane-account-registry', () => - import('./pty-ipc-mock-registry').then((m) => m.codexPaneAccountRegistryModuleMock()) -) -vi.mock('../codex/codex-state-db-backfill-recovery', () => - import('./pty-ipc-mock-registry').then((m) => m.codexBackfillRecoveryModuleMock()) -) - -const SCOPED_PTY_ID = 'ssh:ssh-1@@pty-7' - -function createKillStore() { - return { - markSshRemotePtyLease: vi.fn(), - recordSshRemotePtyKillIntent: vi.fn(), - clearSshRemotePtyKillIntent: vi.fn() - } -} - -function installController(handlers: Map) { - const runtime = { - setPtyController: vi.fn(), - markPtyStopRequested: vi.fn(), - markPtyLivenessUnverifiable: vi.fn(), - onPtyExit: vi.fn(), - intentionalPtyStops: new TerminalIntentionalStops() - } - handlers.clear() - return { runtime } -} - -describe('undelivered SSH stops', () => { - const { handlers, mainWindow } = setupPtyIpcSuite() - - function install(store: ReturnType): { - kill: (ptyId: string) => boolean - stopAndWait: (ptyId: string, opts?: { keepHistory?: boolean }) => Promise - recordUnconfirmedStop: (ptyId: string) => boolean - runtime: ReturnType['runtime'] - } { - const { runtime } = installController(handlers as never) - registerPtyHandlers( - mainWindow as never, - runtime as never, - undefined, - undefined, - undefined, - store as never - ) - // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: registerPtyHandlers installs the runtime PTY controller, which implements these members. - const controller = runtime.setPtyController.mock.calls[0]?.[0] as { - kill: (ptyId: string) => boolean - stopAndWait: (ptyId: string, opts?: { keepHistory?: boolean }) => Promise - recordUnconfirmedStop: (ptyId: string) => boolean - } - return { - kill: controller.kill, - stopAndWait: controller.stopAndWait, - recordUnconfirmedStop: controller.recordUnconfirmedStop, - runtime - } - } - - // The offline close: no provider is registered, so the relay is never even asked. The lease is - // tombstoned locally, and without the record the remote shell has nothing left to retire it. - it('records the stop when no provider is registered to deliver it', async () => { - const store = createKillStore() - setPtyOwnership(SCOPED_PTY_ID, 'ssh:ssh-1') - restorePtyIncarnation(SCOPED_PTY_ID, 'inc-b') - const { kill } = install(store) - - try { - expect(kill(SCOPED_PTY_ID)).toBe(false) - expect(store.recordSshRemotePtyKillIntent).toHaveBeenCalledWith( - 'ssh-1', - 'pty-7', - expect.objectContaining({ incarnationId: 'inc-b' }) - ) - } finally { - deletePtyOwnership(SCOPED_PTY_ID) - } - }) - - it('records the stop when renderer pty:kill finds no provider to deliver it', async () => { - const store = createKillStore() - setPtyOwnership(SCOPED_PTY_ID, 'ssh:ssh-1') - restorePtyIncarnation(SCOPED_PTY_ID, 'inc-r2') - install(store) - - try { - await handlers.get('pty:kill')!(null, { id: SCOPED_PTY_ID }) - expect(store.recordSshRemotePtyKillIntent).toHaveBeenCalledWith( - 'ssh-1', - 'pty-7', - expect.objectContaining({ incarnationId: 'inc-r2' }) - ) - } finally { - deletePtyOwnership(SCOPED_PTY_ID) - } - }) - - it('records nothing for a local PTY, which has no later host to ask', async () => { - const store = createKillStore() - setPtyOwnership('local-pty', 'local') - restorePtyIncarnation('local-pty', 'inc-d') - const { kill } = install(store) - - try { - kill('local-pty') - await new Promise((resolve) => setTimeout(resolve, 0)) - expect(store.recordSshRemotePtyKillIntent).not.toHaveBeenCalled() - } finally { - deletePtyOwnership('local-pty') - } - }) - - // A `pty2:` id is epoch-scoped, so it names one process even when the incarnation was never - // learned: an offline close across a relaunch still owes the kill. - it('records the stop for an epoch-scoped id whose incarnation was never learned', () => { - const store = createKillStore() - const ptyId = 'ssh:ssh-1@@pty2:epoch-a:4' - setPtyOwnership(ptyId, 'ssh:ssh-1') - const { kill } = install(store) - - try { - expect(kill(ptyId)).toBe(false) - expect(store.recordSshRemotePtyKillIntent).toHaveBeenCalledWith('ssh-1', 'pty2:epoch-a:4', { - requestedAt: expect.any(Number), - attempts: 0 - }) - } finally { - deletePtyOwnership(ptyId) - } - }) - - // One recorder: the explicit-close receipt promises the retry for an epoch-scoped id too. - it("records an explicit close's unconfirmed stop for an epoch-scoped id with no incarnation", () => { - const store = createKillStore() - const ptyId = 'ssh:ssh-1@@pty2:epoch-b:5' - setPtyOwnership(ptyId, 'ssh:ssh-1') - const { recordUnconfirmedStop } = install(store) - - try { - expect(recordUnconfirmedStop(ptyId)).toBe(true) - expect(store.recordSshRemotePtyKillIntent).toHaveBeenCalledTimes(1) - expect(store.recordSshRemotePtyKillIntent).toHaveBeenCalledWith('ssh-1', 'pty2:epoch-b:5', { - requestedAt: expect.any(Number), - attempts: 0 - }) - } finally { - deletePtyOwnership(ptyId) - } - }) - - // An explicit close records its order when the stop goes unconfirmed, before the follow-up kill, - // so its receipt can promise the reconnect retry from the record itself. - it("records an explicit close's unconfirmed stop and says so", () => { - const store = createKillStore() - setPtyOwnership(SCOPED_PTY_ID, 'ssh:ssh-1') - restorePtyIncarnation(SCOPED_PTY_ID, 'inc-g') - const { recordUnconfirmedStop } = install(store) - - try { - expect(recordUnconfirmedStop(SCOPED_PTY_ID)).toBe(true) - expect(store.recordSshRemotePtyKillIntent).toHaveBeenCalledWith( - 'ssh-1', - 'pty-7', - expect.objectContaining({ incarnationId: 'inc-g', attempts: 0 }) - ) - } finally { - deletePtyOwnership(SCOPED_PTY_ID) - } - }) - - it('reports no recorded order for a local or reversibly stopped PTY', () => { - const store = createKillStore() - setPtyOwnership('local-pty', 'local') - restorePtyIncarnation('local-pty', 'inc-h') - setPtyOwnership(SCOPED_PTY_ID, 'ssh:ssh-1') - restorePtyIncarnation(SCOPED_PTY_ID, 'inc-i') - const { recordUnconfirmedStop, runtime } = install(store) - const settleStop = runtime.intentionalPtyStops.mark(SCOPED_PTY_ID, 'reversible', null) - - try { - expect(recordUnconfirmedStop('local-pty')).toBe(false) - expect(recordUnconfirmedStop(SCOPED_PTY_ID)).toBe(false) - expect(store.recordSshRemotePtyKillIntent).not.toHaveBeenCalled() - } finally { - settleStop(false) - deletePtyOwnership('local-pty') - deletePtyOwnership(SCOPED_PTY_ID) - } - }) -}) diff --git a/src/main/ipc/pty/host-env/fresh-spawn-routing.ts b/src/main/ipc/pty/host-env/fresh-spawn-routing.ts index e03c29e79bc..8e99124f7f0 100644 --- a/src/main/ipc/pty/host-env/fresh-spawn-routing.ts +++ b/src/main/ipc/pty/host-env/fresh-spawn-routing.ts @@ -18,11 +18,10 @@ export function routesFreshSpawnsToLocalProvider(provider: IPtyProvider): boolea export function recoverFreshSpawnProviderRouting( provider: IPtyProvider, - connectionId: string | null | undefined, sessionId: string | undefined, isNewSession = sessionId === undefined ): Promise | undefined { - if (connectionId || (!isNewSession && sessionId) || !routesFreshSpawnsToLocalProvider(provider)) { + if ((!isNewSession && sessionId) || !routesFreshSpawnsToLocalProvider(provider)) { return } return provider.recoverFreshSpawnRouting?.() diff --git a/src/main/ipc/pty/ipc/renderer-kill.ts b/src/main/ipc/pty/ipc/renderer-kill.ts index 503efaaeeee..095ffc64cb2 100644 --- a/src/main/ipc/pty/ipc/renderer-kill.ts +++ b/src/main/ipc/pty/ipc/renderer-kill.ts @@ -14,7 +14,6 @@ import { type ResolvedPtyHost } from '../provider/registry' import { finishPtyShutdown, isPtyAlreadyGoneError } from '../provider/liveness' -import { recordUndeliveredSshPtyKill } from '../runtime/undelivered-ssh-kill' export type PtyKillIpcDeps = { store?: Store @@ -30,8 +29,7 @@ export type PtyKillIpcDeps = { } /** `pty:kill` — the route the renderer takes when the user closes a terminal tab, and a separate - * implementation from `killPtyFromRuntimeController`. Both have to record an undelivered SSH stop, - * and this is the one ordinary tab close actually reaches. */ + * implementation from `killPtyFromRuntimeController`. */ export function installPtyKillIpcHandler(deps: PtyKillIpcDeps): void { getPtyIpc().handle('pty:kill', (_event, args: { id: string; keepHistory?: boolean }) => stopRendererOwnedPty(deps, args) @@ -97,24 +95,12 @@ async function stopRendererOwnedPtyProcess( if (startupPromise) { await startupPromise } - // Why stated rather than inferred: this IPC serves both the ordinary tab close and pane - // hibernation, and only hibernation passes keepHistory. Recording a replayable kill for a - // hibernating pane would destroy it on the next handshake. - const reversible = args.keepHistory === true const provider = tryGetProviderForPty(args.id) if (!provider && connectionId) { // Why: detached SSH PTYs intentionally keep ownership after their // provider is unregistered; hydrated app-scoped ids can also arrive // before ownership is rebuilt. Tombstone instead of falling back local. const incarnationId = finishPtyShutdown(args.id, connectionId, store) - // The relay was never asked, so the remote shell is still running. Keep the order. - recordUndeliveredSshPtyKill({ - store, - ptyId: args.id, - connectionId, - reversible, - incarnationId - }) runtime?.markPtyLivenessUnverifiable?.(args.id, SSH_PROVIDER_UNREGISTERED_REASON) runtime?.onPtyExit(args.id, -1, incarnationId) rememberSyntheticKillExit(args.id, incarnationId) @@ -134,10 +120,7 @@ async function stopRendererOwnedPtyProcess( }) } catch (err) { if (!isPtyAlreadyGoneError(err)) { - // Why: a failed shutdown can leave the process alive (SSH relay grace window / local daemon); keep ownership/lease state so the user can retry. - // The renderer has already discarded the tab, so nothing here retries — the durable order - // is what lets the next handshake to this host finish the close. - recordUndeliveredSshPtyKill({ store, ptyId: args.id, connectionId, reversible }) + // Why: a failed shutdown can leave the local daemon's process alive; keep ownership so a retry can reach it. throw err } /* session already dead — cleanup below handles the rest */ diff --git a/src/main/ipc/pty/ipc/spawn-commit-persist.ts b/src/main/ipc/pty/ipc/spawn-commit-persist.ts index 108b4e7c388..03b7ef9e0bd 100644 --- a/src/main/ipc/pty/ipc/spawn-commit-persist.ts +++ b/src/main/ipc/pty/ipc/spawn-commit-persist.ts @@ -1,13 +1,9 @@ -import { - getConnectionExecutionHostId, - toSshExecutionHostId -} from '../../../../shared/execution-host' -import { closeStartupQueryAuthorityForPty, getRelayPtyId } from '../provider/registry' +import { LOCAL_EXECUTION_HOST_ID } from '../../../../shared/execution-host' +import { closeStartupQueryAuthorityForPty } from '../provider/registry' import { createTerminalSessionStateSaveFailureMessage } from '../../../../shared/terminal-session-state-save-failure' import { recordCodexPaneAccountForSpawn } from '../host-env/codex-home' import { persistAdmittedStablePaneBinding } from '../pane/stable-owner' import { swapReplacedPaneBinding } from '../pane/pane-owner-replacement' -import { claimSshPaneLease } from '../pane/ssh-pane-lease-claim' import { pendingByPaneKey, pendingPtyIdBySerializerGeneration, @@ -29,8 +25,7 @@ export async function persistPtyIpcSpawnCommit(ctx: PtyIpcSpawnState): Promise

{ - if (args.connectionId && ctx.deps.store) { - ctx.deps.store.removeSshRemotePtyLease(args.connectionId, relayResultId) - } - }) + await discardUnpersistedPtySpawn(ctx.provider, ctx.result) if (err instanceof Error && err.message === 'terminal_pane_owner_changed') { throw err } @@ -96,7 +83,6 @@ export async function persistPtyIpcSpawnCommit(ctx: PtyIpcSpawnState): Promise

({ - app: { getPath: () => testState.dir }, - safeStorage: { isEncryptionAvailable: () => false } -})) - -const TARGET = 'ssh-1' -const WORKTREE = 'repo1::/worktree' -const TAB = 'tab-1' - -/** Exercises the shipped binding-then-publication order so reconnects retire earlier leases. */ -async function commitSshSpawn( - store: ReturnType, - args: { relayPtyId: string; leafId: string } -): Promise { - const deps = { store } as unknown as PtySpawnIpcDeps - const spawnArgs = { - cols: 80, - rows: 24, - worktreeId: WORKTREE, - tabId: TAB, - leafId: args.leafId, - connectionId: TARGET - } as unknown as PtySpawnIpcArgs - const ctx = createPtyIpcSpawnState(deps, spawnArgs) - ctx.result = { id: toAppSshPtyId(TARGET, args.relayPtyId) } - ctx.validatedLeafId = args.leafId - publishPtyIpcSpawnCommit(ctx, await persistPtyIpcSpawnCommit(ctx)) -} - -/** One pane's layout, so the two host partitions can be given different bindings for one leaf. */ -function sessionBinding(ptyId: string) { - return { - activeRepoId: 'repo1', - activeWorktreeId: WORKTREE, - activeTabId: TAB, - tabsByWorktree: {}, - terminalLayoutsByTabId: { - [TAB]: { - root: { type: 'leaf' as const, leafId: TEST_LEAF_1 }, - activeLeafId: TEST_LEAF_1, - expandedLeafId: null, - ptyIdsByLeafId: { [TEST_LEAF_1]: ptyId } - } - } - } -} - -function bulkReattachPtyIds(store: ReturnType): string[] { - return store - .getSshRemotePtyLeases(TARGET) - .filter(sshRemotePtyLeaseAllowsReattach) - .map((lease) => lease.ptyId) - .sort() -} - -describe('the IPC spawn commit keeps one reattachable lease per SSH pane', () => { - beforeEach(() => { - testState.dir = mkdtempSync(join(tmpdir(), 'orca-test-')) - }) - afterEach(() => { - rmSync(testState.dir, { recursive: true, force: true }) - }) - - // QA's measurement, driven through the real path: five relay restarts, one pane, N+1 leases. - it('holds the reattach set flat across five reconnects of one pane', async () => { - const store = await createStore() - - for (let reconnect = 0; reconnect < 5; reconnect++) { - // A relay renumbers from `pty-1` on every start; a reconnect therefore re-leases the same - // pane under an id it has never used before. - await commitSshSpawn(store, { relayPtyId: `pty-${reconnect}`, leafId: TEST_LEAF_1 }) - } - - expect(bulkReattachPtyIds(store)).toEqual(['pty-4']) - }) - - it('retires each predecessor as `expired` with the winner recorded, never `terminated`', async () => { - const store = await createStore() - - await commitSshSpawn(store, { relayPtyId: 'pty-0', leafId: TEST_LEAF_1 }) - await commitSshSpawn(store, { relayPtyId: 'pty-1', leafId: TEST_LEAF_1 }) - - const predecessor = store.getSshRemotePtyLeases(TARGET).find((entry) => entry.ptyId === 'pty-0') - // `expired`, not `terminated`: losing the lease is not evidence the remote shell died, and the - // process is deliberately left running (docs/reference/ssh-execution-boundary.md). - expect(predecessor).toMatchObject({ state: 'expired', supersededBy: 'pty-1' }) - }) - - // The failure that would be worse than the fan-out: over-superseding strands a live remote - // process behind a pane that can no longer find it. - it('leaves a genuine orphan reattachable while superseding the pane that re-leased', async () => { - const store = await createStore() - - await commitSshSpawn(store, { relayPtyId: 'orphan-pty', leafId: TEST_LEAF_2 }) - // The orphan's client lost its route; nothing observed the shell, so it stays askable. - store.markSshRemotePtyLease(TARGET, 'orphan-pty', 'expired') - - await commitSshSpawn(store, { relayPtyId: 'pty-0', leafId: TEST_LEAF_1 }) - await commitSshSpawn(store, { relayPtyId: 'pty-1', leafId: TEST_LEAF_1 }) - - const orphan = store.getSshRemotePtyLeases(TARGET).find((entry) => entry.ptyId === 'orphan-pty') - expect(orphan?.supersededBy).toBeUndefined() - expect(bulkReattachPtyIds(store)).toEqual(['orphan-pty', 'pty-1']) - }) - - /** - * The shape the Docker lane exposed, and the reason a spawn-time trigger is not enough on its - * own. When the spawn commit writes no binding, the renderer's debounced layout publish does it - * later — so at commit time the pane still names the predecessor and supersession correctly - * declines. Nothing revisited it afterwards, and the predecessor stayed reattachable forever. - * - * Measured rows agreed on target, worktree, tab and leaf and still carried no `supersededBy`. - */ - it('retires a predecessor whose successor bound the pane after the spawn commit', async () => { - const store = await createStore() - - await commitSshSpawn(store, { relayPtyId: 'pty2:aaa:1', leafId: TEST_LEAF_1 }) - // What `handlePtyReattachFailure` writes when a restarted relay disowns the id. - store.markSshRemotePtyLease(TARGET, 'pty2:aaa:1', 'expired') - - // The successor leases without binding the pane; the binding catches up afterwards, exactly as - // the renderer's debounced publish does. - store.upsertSshRemotePtyLease({ - targetId: TARGET, - ptyId: 'pty2:bbb:1', - worktreeId: WORKTREE, - tabId: TAB, - leafId: TEST_LEAF_1, - state: 'attached' - }) - expect(bulkReattachPtyIds(store)).toEqual(['pty2:aaa:1', 'pty2:bbb:1']) - await store.persistPtyBinding({ - worktreeId: WORKTREE, - tabId: TAB, - leafId: TEST_LEAF_1, - ptyId: toAppSshPtyId(TARGET, 'pty2:bbb:1') - }) - - // What the connect path does before reading the set it feeds to `pty.attach`. - store.reconcileSshRemotePtyLeasesForTarget(TARGET) - - expect(bulkReattachPtyIds(store)).toEqual(['pty2:bbb:1']) - }) - - // Reconciliation must not invent evidence: with no binding naming the pane, nothing says which - // shell owns it, so every lease stays askable. - it('leaves leases reattachable when no binding names the pane', async () => { - const store = await createStore() - - store.upsertSshRemotePtyLease({ - targetId: TARGET, - ptyId: 'unbound-a', - worktreeId: WORKTREE, - tabId: TAB, - leafId: TEST_LEAF_1, - state: 'expired' - }) - store.upsertSshRemotePtyLease({ - targetId: TARGET, - ptyId: 'unbound-b', - worktreeId: WORKTREE, - tabId: TAB, - leafId: TEST_LEAF_1, - state: 'expired' - }) - - store.reconcileSshRemotePtyLeasesForTarget(TARGET) - - expect(bulkReattachPtyIds(store)).toEqual(['unbound-a', 'unbound-b']) - }) - - /** - * The measured defect, reduced to its cause. - * - * Main writes an SSH pane's binding to the `ssh:` partition, but a stale copy of the same - * leaf survives in `local`. Reading `local` first named the PREDECESSOR as the pane's current - * PTY, so supersession took an already-expired lease as its winner and returned having marked - * nothing — once per relay restart, forever. Both partitions name the same PTY again once the - * renderer republishes, which is why the finished store looks consistent and hides this. - */ - it('supersedes when the local partition still names the predecessor', async () => { - const store = await createStore() - const hostId = toSshExecutionHostId(TARGET) - const predecessor = toAppSshPtyId(TARGET, 'pty2:old:1') - const successor = toAppSshPtyId(TARGET, 'pty2:new:1') - - store.upsertSshRemotePtyLease({ - targetId: TARGET, - ptyId: 'pty2:old:1', - worktreeId: WORKTREE, - tabId: TAB, - leafId: TEST_LEAF_1, - state: 'expired' - }) - // Both partitions start on the predecessor, as they do before a relay restart. - store.setWorkspaceSession(sessionBinding(predecessor)) - store.setWorkspaceSession(sessionBinding(predecessor), hostId) - store.upsertSshRemotePtyLease({ - targetId: TARGET, - ptyId: 'pty2:new:1', - worktreeId: WORKTREE, - tabId: TAB, - leafId: TEST_LEAF_1, - state: 'attached' - }) - // Production's writer for an SSH pane binding, and the whole point: it updates ONLY the host - // partition, so `local` is left naming the predecessor until the renderer republishes. - await store.persistPtyBinding( - { worktreeId: WORKTREE, tabId: TAB, leafId: TEST_LEAF_1, ptyId: successor }, - hostId - ) - expect( - store.getWorkspaceSession().terminalLayoutsByTabId?.[TAB]?.ptyIdsByLeafId?.[TEST_LEAF_1] - ).toBe(predecessor) - - store.supersedeSshRemotePtyLeasesForBoundPane(TARGET, TEST_LEAF_1) - - const retired = store.getSshRemotePtyLeases(TARGET).find((l) => l.ptyId === 'pty2:old:1') - expect(retired).toMatchObject({ state: 'expired', supersededBy: 'pty2:new:1' }) - expect(bulkReattachPtyIds(store)).toEqual(['pty2:new:1']) - }) - - // The mirror: a live shell the pane is still bound to must never be retired, whichever partition - // names it. Over-superseding strands a running remote process. - it('never retires a live lease the pane is still bound to', async () => { - const store = await createStore() - const live = toAppSshPtyId(TARGET, 'pty2:live:1') - - store.upsertSshRemotePtyLease({ - targetId: TARGET, - ptyId: 'pty2:live:1', - worktreeId: WORKTREE, - tabId: TAB, - leafId: TEST_LEAF_1, - state: 'attached' - }) - // Bound BEFORE the stray lease arrives, which is the order that makes the binding meaningful: - // with no binding at all, an arriving lease is the only evidence there is and does win. - store.setWorkspaceSession(sessionBinding(live)) - store.setWorkspaceSession(sessionBinding(live), toSshExecutionHostId(TARGET)) - store.upsertSshRemotePtyLease({ - targetId: TARGET, - ptyId: 'pty2:other:1', - worktreeId: WORKTREE, - tabId: TAB, - leafId: TEST_LEAF_1, - state: 'attached' - }) - - store.supersedeSshRemotePtyLeasesForBoundPane(TARGET, TEST_LEAF_1) - - const stillLive = store.getSshRemotePtyLeases(TARGET).find((l) => l.ptyId === 'pty2:live:1') - expect(stillLive).toMatchObject({ state: 'attached' }) - expect(stillLive?.supersededBy).toBeUndefined() - }) - - // Panes are independent, and supersession keys on the leaf: a second live pane on the same - // target must survive its neighbour reconnecting. - it('does not touch a sibling pane on the same target', async () => { - const store = await createStore() - - await commitSshSpawn(store, { relayPtyId: 'sibling-pty', leafId: TEST_LEAF_2 }) - await commitSshSpawn(store, { relayPtyId: 'pty-0', leafId: TEST_LEAF_1 }) - await commitSshSpawn(store, { relayPtyId: 'pty-1', leafId: TEST_LEAF_1 }) - - expect(bulkReattachPtyIds(store)).toEqual(['pty-1', 'sibling-pty']) - }) -}) diff --git a/src/main/ipc/pty/ipc/spawn-commit.ts b/src/main/ipc/pty/ipc/spawn-commit.ts index ffcc5d158bb..6ff84a49751 100644 --- a/src/main/ipc/pty/ipc/spawn-commit.ts +++ b/src/main/ipc/pty/ipc/spawn-commit.ts @@ -29,7 +29,7 @@ export async function commitPtyIpcSpawn(ctx: PtyIpcSpawnState): Promise { const args = ctx.args - admitPtyReattachOwnership(ctx.deps.runtime, ctx.result, args.connectionId) + admitPtyReattachOwnership(ctx.deps.runtime, ctx.result) if (ctx.nativeWindowsConptySpawn) { markNativeWindowsConptyPty(ctx.result.id) } @@ -61,7 +61,7 @@ async function commitReservedPtyIpcSpawn(ctx: PtyIpcSpawnState): Promise { @@ -152,32 +150,29 @@ async function commitReservedPtyIpcSpawn(ctx: PtyIpcSpawnState): Promise 0 && - args.worktreeId.length <= 512 - ? args.worktreeId - : null, - sessionId: - typeof args.sessionId === 'string' && - args.sessionId.length > 0 && - args.sessionId.length <= 256 - ? args.sessionId - : null, - paneKey: rememberedPaneKey, - pid: - typeof spawnedPid === 'number' && Number.isFinite(spawnedPid) && spawnedPid > 0 - ? spawnedPid - : null - }) - } + // Why: record the spawn-result pid once here so the memory module needn't reach back into ipc/pty on a hot path (works for in-process and daemon-hosted PTYs). + const spawnedPid = ctx.result.pid ?? null + // Why: args.worktreeId/sessionId arrive as untrusted IPC strings (type unenforced at the boundary); bound them so malformed/oversized values can't pollute registerPty's maps. + registerPty({ + ptyId: ctx.result.id, + worktreeId: + typeof args.worktreeId === 'string' && + args.worktreeId.length > 0 && + args.worktreeId.length <= 512 + ? args.worktreeId + : null, + sessionId: + typeof args.sessionId === 'string' && + args.sessionId.length > 0 && + args.sessionId.length <= 256 + ? args.sessionId + : null, + paneKey: rememberedPaneKey, + pid: + typeof spawnedPid === 'number' && Number.isFinite(spawnedPid) && spawnedPid > 0 + ? spawnedPid + : null + }) // Why: telemetry-plan.md§Agent launch semantics — fire agent_started only after spawn resolved; safeParse each field so a spoofed IPC payload can't poison the event (missing required field skips it). if (args.telemetry && !ctx.stablePaneOwner) { recordPtySpawnTelemetry(args.telemetry) @@ -206,24 +201,22 @@ async function commitReservedPtyIpcSpawn(ctx: PtyIpcSpawnState): Promise { const args = ctx.args // Why: the daemon-backed provider skips LocalPtyProvider's buildSpawnEnv, so assemble the same host-local env here for parity. - // Safety: skip entirely for SSH — every injection is a loopback secret or a local path that leaks or misleads on the remote host. - // Why: forward pane env to SSH only when the relay hook path is enabled, or a newer relay could emit statuses this build can't route. - const sshSourceEnv = stripRemotePaneEnvWhenHooksDisabled(args.connectionId, args.env) - const baseEnvWithAuth = ctx.claudeAuth - ? { ...sshSourceEnv, ...ctx.claudeAuth.envPatch } - : sshSourceEnv + const baseEnvWithAuth = ctx.claudeAuth ? { ...args.env, ...ctx.claudeAuth.envPatch } : args.env const spawnPaneKey = baseEnvWithAuth?.ORCA_PANE_KEY const parsedSpawnPaneKey = parseValidPaneKey(spawnPaneKey) const verifiedPaneKey = @@ -68,7 +61,6 @@ export async function assemblePtyIpcSpawnEnv(ctx: PtyIpcSpawnState): Promise { ctx.snapshotKittyFlagsCoverReconciledSeq = false } } - ensureWslHookRelayForReattach(ctx.result, args.connectionId) + ensureWslHookRelayForReattach(ctx.result) ctx.deps.runtime?.preparePtyExecutionContext?.( ctx.result.id, - args.connectionId - ? null - : ctx.result.wslDistro === undefined - ? ctx.expectedWslDistro - : ctx.result.wslDistro + ctx.result.wslDistro === undefined ? ctx.expectedWslDistro : ctx.result.wslDistro ) ctx.spawnTiming.mark('provider_spawn') } catch (err) { @@ -139,52 +129,19 @@ export async function executePtyIpcSpawn(ctx: PtyIpcSpawnState): Promise { ctx.pendingRegistrationPtyId = null } const spawnError = normalizeNodePtySpawnError(err) - const isIdentityMismatch = - isSshPtyIdentityMismatchError(spawnError) || isSshPtyIdentityMismatchError(rawMessage) - const isExpiredSshSession = - Boolean(args.connectionId) && - (spawnError.message.includes(SSH_SESSION_EXPIRED_ERROR) || - rawMessage.includes(SSH_SESSION_EXPIRED_ERROR)) - // The message alone cannot carry this decision. All three reattach refusals are minted with the - // same `SSH_SESSION_EXPIRED` text, and only one of them observed the process: `restoreRequired` - // means the PTY is LIVE and only its source stream needs rebuilding, which - // `ssh-pty-errors.ts` states outright. Expiring its lease and deleting its ownership erases - // this client's last record of a running remote process, and #9819's sweep reads a PTY it has - // no record of as one it may SIGKILL on the next connect. Only positive host-reported absence - // may reach that bookkeeping; being too strict here merely leaves a dead lease for the next - // reattach to retire on real host evidence. - const relayReportedSessionAbsent = isExpiredSshSession && isSshPtyAbsentFromRelayError(err) const exitedBeforeSpawnReply = ctx.rejectedRegistrationCandidate?.exitedBeforeSpawnReply === true if (ctx.effectiveSessionAppId !== undefined) { if ( ctx.hadSessionSizeBeforeAttach && ctx.sessionSizeBeforeAttach && - (isIdentityMismatch || (!isExpiredSshSession && !exitedBeforeSpawnReply)) + !exitedBeforeSpawnReply ) { ptySizes.set(ctx.effectiveSessionAppId, ctx.sessionSizeBeforeAttach) } else { ptySizes.delete(ctx.effectiveSessionAppId) } } - if ( - args.connectionId && - ctx.effectiveSessionRelayId !== undefined && - relayReportedSessionAbsent - ) { - // Why: expired remote reattach = relay already dropped the PTY; clear the lease so writes can't restore the stale binding. - if (ctx.effectiveSessionAppId !== undefined && !isIdentityMismatch) { - clearProviderPtyState(ctx.effectiveSessionAppId) - deletePtyOwnership(ctx.effectiveSessionAppId) - } - if (!isIdentityMismatch) { - ctx.deps.store?.markSshRemotePtyLease( - args.connectionId, - ctx.effectiveSessionRelayId, - 'expired' - ) - } - } // Why: provider state buildPtyHostEnv materialized for this minted id leaks if spawn failed. if (ctx.isMintedSessionId && ctx.effectiveSessionId !== undefined) { clearProviderPtyState(ctx.effectiveSessionId) diff --git a/src/main/ipc/pty/ipc/spawn-preflight.ts b/src/main/ipc/pty/ipc/spawn-preflight.ts index d04ddf91980..cbc0f2e21d0 100644 --- a/src/main/ipc/pty/ipc/spawn-preflight.ts +++ b/src/main/ipc/pty/ipc/spawn-preflight.ts @@ -15,29 +15,21 @@ import { recoverFreshSpawnProviderRouting, routesFreshSpawnsToLocalProvider } from '../host-env/fresh-spawn-routing' -import { getAppPtyId, getProvider, getRelayPtyId } from '../provider/registry' +import { getProvider } from '../provider/registry' import type { PtyIpcSpawnState } from './spawn-state' import { getConnectionExecutionHostId } from '../../../../shared/execution-host' export async function preparePtyIpcSpawnPreflight(ctx: PtyIpcSpawnState): Promise { const args = ctx.args // Establish daemon identity before the first await so hidden delivery is gated before byte zero. + // Only this machine spawns here: an `ssh:` connection throws, so every line below is local. ctx.provider = getProvider(getConnectionExecutionHostId(args.connectionId)) ctx.isDaemonHostSpawn = - !args.connectionId && - !(ctx.provider instanceof LocalPtyProvider) && - !routesFreshSpawnsToLocalProvider(ctx.provider) + !(ctx.provider instanceof LocalPtyProvider) && !routesFreshSpawnsToLocalProvider(ctx.provider) ctx.isMintedSessionId = args.sessionId === undefined && ctx.isDaemonHostSpawn ctx.effectiveSessionId = args.sessionId ?? (ctx.isDaemonHostSpawn ? mintPtySessionId(args.worktreeId) : undefined) - ctx.effectiveSessionAppId = - ctx.effectiveSessionId !== undefined - ? getAppPtyId(args.connectionId, ctx.effectiveSessionId) - : undefined - ctx.effectiveSessionRelayId = - ctx.effectiveSessionId !== undefined - ? getRelayPtyId(args.connectionId, ctx.effectiveSessionId) - : undefined + ctx.effectiveSessionAppId = ctx.effectiveSessionId ctx.initiallyHidden = args.initiallyHidden === true ctx.preSpawnHiddenMarkId = ctx.initiallyHidden && ctx.isDaemonHostSpawn && ctx.effectiveSessionAppId !== undefined @@ -74,9 +66,8 @@ export async function preparePtyIpcSpawnPreflight(ctx: PtyIpcSpawnState): Promis } } if (!ctx.preAdoptedStablePane) { - // Why: reattach needs exact cwd, SSH cannot probe locally, and successful stable-pane adoption needs no launch preflight. - const requestedMissingCwdFallback = - !args.connectionId && !args.sessionId && args.cwdFallback === 'worktree' + // Why: reattach needs exact cwd, and successful stable-pane adoption needs no launch preflight. + const requestedMissingCwdFallback = !args.sessionId && args.cwdFallback === 'worktree' const isPosixStartupCwd = args.cwd?.startsWith('/') === true const startupWorkspaceCwd = requestedMissingCwdFallback && isPosixStartupCwd @@ -159,25 +150,16 @@ export async function preparePtyIpcSpawnPreflight(ctx: PtyIpcSpawnState): Promis ctx.spawnTiming.mark('preflight') const freshSpawnRecovery = ctx.preAdoptedStablePane ? undefined - : recoverFreshSpawnProviderRouting(ctx.provider, args.connectionId, args.sessionId) + : recoverFreshSpawnProviderRouting(ctx.provider, args.sessionId) if (freshSpawnRecovery) { await freshSpawnRecovery const previousHiddenMarkId = ctx.preSpawnHiddenMarkId ctx.isDaemonHostSpawn = - !args.connectionId && - !(ctx.provider instanceof LocalPtyProvider) && - !routesFreshSpawnsToLocalProvider(ctx.provider) + !(ctx.provider instanceof LocalPtyProvider) && !routesFreshSpawnsToLocalProvider(ctx.provider) ctx.isMintedSessionId = args.sessionId === undefined && ctx.isDaemonHostSpawn ctx.effectiveSessionId = args.sessionId ?? (ctx.isDaemonHostSpawn ? mintPtySessionId(args.worktreeId) : undefined) - ctx.effectiveSessionAppId = - ctx.effectiveSessionId !== undefined - ? getAppPtyId(args.connectionId, ctx.effectiveSessionId) - : undefined - ctx.effectiveSessionRelayId = - ctx.effectiveSessionId !== undefined - ? getRelayPtyId(args.connectionId, ctx.effectiveSessionId) - : undefined + ctx.effectiveSessionAppId = ctx.effectiveSessionId ctx.preSpawnHiddenMarkId = ctx.initiallyHidden && ctx.isDaemonHostSpawn && ctx.effectiveSessionAppId !== undefined ? ctx.effectiveSessionAppId @@ -191,10 +173,9 @@ export async function preparePtyIpcSpawnPreflight(ctx: PtyIpcSpawnState): Promis } } } - ctx.isClaudeLaunch = - !ctx.preAdoptedStablePane && !args.connectionId && isClaudeLaunchCommand(args.command) + ctx.isClaudeLaunch = !ctx.preAdoptedStablePane && isClaudeLaunchCommand(args.command) ctx.terminalRuntimeOptions = - process.platform === 'win32' && !args.connectionId + process.platform === 'win32' ? resolveLocalWindowsTerminalRuntimeOptions({ requestedShellOverride: args.shellOverride, settings: ctx.deps.getSettings?.(), @@ -211,20 +192,19 @@ export async function preparePtyIpcSpawnPreflight(ctx: PtyIpcSpawnState): Promis // Why: daemon host-env setup needs a stable id BEFORE provider.spawn so buildPtyHostEnv hooks/Pi cleanup can run; daemon still honors opts.sessionId ?? mint(). // Note: sessionId is STABLE across daemon restarts by design — do NOT simplify to a fresh UUID per spawn; that orphans reconnectable state. // Why: only clear ids minted in THIS request on failure — a caller-supplied args.sessionId may name an existing PTY we must not clobber. - ctx.expectedWslDistro = !args.connectionId - ? (resolveWslSessionContext({ - cwd: ctx.cwd, - sessionId: ctx.effectiveSessionId, - shellOverride: ctx.terminalRuntimeOptions.shellOverride, - terminalWindowsWslDistro: ctx.terminalRuntimeOptions.terminalWindowsWslDistro - })?.distro ?? null) - : null + ctx.expectedWslDistro = + resolveWslSessionContext({ + cwd: ctx.cwd, + sessionId: ctx.effectiveSessionId, + shellOverride: ctx.terminalRuntimeOptions.shellOverride, + terminalWindowsWslDistro: ctx.terminalRuntimeOptions.terminalWindowsWslDistro + })?.distro ?? null const initialSelectionTarget = getCodexSelectionTargetForPty( initialShellOverride, ctx.cwd, ctx.expectedWslDistro ) - args.env = withClaudeProfileTerminalEnv(args.env, args.connectionId, initialSelectionTarget) + args.env = withClaudeProfileTerminalEnv(args.env, initialSelectionTarget) ctx.claudeAuth = ctx.isClaudeLaunch && ctx.deps.prepareClaudeAuth ? await ctx.deps.prepareClaudeAuth(initialSelectionTarget) diff --git a/src/main/ipc/pty/ipc/spawn-state.ts b/src/main/ipc/pty/ipc/spawn-state.ts index d753ac4a6fd..ed0310d7929 100644 --- a/src/main/ipc/pty/ipc/spawn-state.ts +++ b/src/main/ipc/pty/ipc/spawn-state.ts @@ -49,7 +49,6 @@ export type PtyIpcSpawnState = { isMintedSessionId: boolean effectiveSessionId: string | undefined effectiveSessionAppId: string | undefined - effectiveSessionRelayId: string | undefined expectedWslDistro: string | null baseEnv: Record | undefined effectiveLaunchConfig: SleepingAgentLaunchConfig | undefined @@ -125,7 +124,6 @@ export function createPtyIpcSpawnState( isMintedSessionId: false, effectiveSessionId: undefined, effectiveSessionAppId: undefined, - effectiveSessionRelayId: undefined, expectedWslDistro: null, baseEnv: undefined, effectiveLaunchConfig: undefined, diff --git a/src/main/ipc/pty/pane/pane-owner-replacement.test.ts b/src/main/ipc/pty/pane/pane-owner-replacement.test.ts index 681a48ba616..07ba7d410e3 100644 --- a/src/main/ipc/pty/pane/pane-owner-replacement.test.ts +++ b/src/main/ipc/pty/pane/pane-owner-replacement.test.ts @@ -15,8 +15,8 @@ vi.mock('../../../ssh/ssh-config-parser', () => ({ const worktreeId = 'repo-local::/fixture/local' const tabId = 'restart-tab' -function replaced(ptyId: string, hostId?: string): ReplacedPaneOwner { - return { ptyId, pane: { worktreeId, tabId, leafId: TEST_LEAF_1, hostId } } +function replaced(ptyId: string): ReplacedPaneOwner { + return { ptyId, pane: { worktreeId, tabId, leafId: TEST_LEAF_1, hostId: undefined } } } type FixtureStore = Awaited>['store'] @@ -25,27 +25,23 @@ type FixtureStore = Awaited>['store'] async function expectSwapAfterRendererClear( store: FixtureStore, oldId: string, - newId: string, - hostId?: string + newId: string ): Promise { expect( - store.getWorkspaceSession(hostId).terminalLayoutsByTabId[tabId].ptyIdsByLeafId?.[TEST_LEAF_1] + store.getWorkspaceSession().terminalLayoutsByTabId[tabId].ptyIdsByLeafId?.[TEST_LEAF_1] ).toBeUndefined() const binding = { worktreeId, tabId, leafId: TEST_LEAF_1, ptyId: newId, incarnationId: 'inc-new' } expect( - await store.persistPtyBinding( - { ...binding, expectedBinding: { ptyId: oldId, incarnationId: 'inc-old' } }, - hostId - ) + await store.persistPtyBinding({ + ...binding, + expectedBinding: { ptyId: oldId, incarnationId: 'inc-old' } + }) ).toBe(false) expect( - await store.persistPtyBinding( - swapReplacedPaneBinding(store, binding, replaced(oldId, hostId), hostId), - hostId - ) + await store.persistPtyBinding(swapReplacedPaneBinding(store, binding, replaced(oldId))) ).toBe(true) expect( - store.getWorkspaceSession(hostId).terminalLayoutsByTabId[tabId].ptyIdsByLeafId?.[TEST_LEAF_1] + store.getWorkspaceSession().terminalLayoutsByTabId[tabId].ptyIdsByLeafId?.[TEST_LEAF_1] ).toBe(newId) } @@ -69,38 +65,12 @@ it('swaps a local split pane whose renderer layout patch already dropped the sto ).toBe('sibling') }) -it('swaps an SSH pane whose terminated lease let the renderer clear withdraw the binding', async () => { - const { store } = await fixture() - const hostId = 'ssh:restart' - const oldId = 'ssh:restart@@remote-old' - await store.persistPtyBinding( - { worktreeId, tabId, leafId: TEST_LEAF_1, ptyId: oldId, incarnationId: 'inc-old' }, - hostId - ) - store.upsertSshRemotePtyLease({ - targetId: 'restart', - ptyId: 'remote-old', - worktreeId, - tabId, - leafId: TEST_LEAF_1, - state: 'attached' - }) - // What the replacement stop records (finishPtyShutdown). - store.markSshRemotePtyLease('restart', 'remote-old', 'terminated') - const layouts = structuredClone(store.getWorkspaceSession(hostId).terminalLayoutsByTabId) - layouts[tabId].ptyIdsByLeafId = {} - store.patchWorkspaceSession({ terminalLayoutsByTabId: layouts }, hostId) - await expectSwapAfterRendererClear(store, oldId, 'ssh:restart@@remote-new', hostId) -}) - it('refuses the swap when another owner holds the leaf', async () => { const { store } = await fixture() await store.persistPtyBinding({ worktreeId, tabId, leafId: TEST_LEAF_1, ptyId: 'successor' }) const binding = { worktreeId, tabId, leafId: TEST_LEAF_1, ptyId: 'new' } expect( - await store.persistPtyBinding( - swapReplacedPaneBinding(store, binding, replaced('old'), undefined) - ) + await store.persistPtyBinding(swapReplacedPaneBinding(store, binding, replaced('old'))) ).toBe(false) expect( store.getWorkspaceSession().terminalLayoutsByTabId[tabId].ptyIdsByLeafId?.[TEST_LEAF_1] diff --git a/src/main/ipc/pty/pane/pane-owner-replacement.ts b/src/main/ipc/pty/pane/pane-owner-replacement.ts index 0130ae8b702..cfb577eed63 100644 --- a/src/main/ipc/pty/pane/pane-owner-replacement.ts +++ b/src/main/ipc/pty/pane/pane-owner-replacement.ts @@ -66,15 +66,14 @@ export function excludeReplacedPaneOwner( export function swapReplacedPaneBinding( store: Store, binding: PersistPtyBindingArgs, - replaced: ReplacedPaneOwner, - hostId: string | undefined + replaced: ReplacedPaneOwner ): () => PersistPtyBindingArgs | null { return () => { const bound = - store.getWorkspaceSession(hostId).terminalLayoutsByTabId?.[binding.tabId]?.ptyIdsByLeafId?.[ + store.getWorkspaceSession().terminalLayoutsByTabId?.[binding.tabId]?.ptyIdsByLeafId?.[ binding.leafId ] - // Unbound is ok: the renderer's pre-connect clear can withdraw it (split tab, SSH lease ended). + // Unbound is ok: the renderer's pre-connect clear can withdraw it (split tab). return bound === undefined || bound === replaced.ptyId ? binding : null } } diff --git a/src/main/ipc/pty/pane/spawn-registration.ts b/src/main/ipc/pty/pane/spawn-registration.ts index 54452e5079e..e51e9c62c66 100644 --- a/src/main/ipc/pty/pane/spawn-registration.ts +++ b/src/main/ipc/pty/pane/spawn-registration.ts @@ -12,15 +12,14 @@ import { retirePersistedStablePaneOwner } from './stable-owner' export function admitPtyReattachOwnership( runtime: OrcaRuntimeService | undefined, - result: PtySpawnResult, - connectionId: string | null | undefined + result: PtySpawnResult ): void { if (!result.isReattach && result.agentSessionEnsure?.disposition !== 'adopted') { return } runtime?.assertPtyRegistrationAllowed?.(result.id, result.incarnationId) // A failed local save must not strand a live process already admitted by its host. - setAdoptedPtyOwnership(result.id, connectionId) + setAdoptedPtyOwnership(result.id) if (result.incarnationId) { ptyIncarnationById.set(result.id, result.incarnationId) } @@ -28,8 +27,7 @@ export function admitPtyReattachOwnership( export async function discardUnpersistedPtySpawn( provider: IPtyProvider, - result: PtySpawnResult, - onDiscarded?: () => void + result: PtySpawnResult ): Promise { if ( result.isReattach || @@ -50,7 +48,6 @@ export async function discardUnpersistedPtySpawn( if (isCurrentPtyExit(result)) { clearProviderPtyState(result.id) ptyOwnership.delete(result.id) - onDiscarded?.() } } diff --git a/src/main/ipc/pty/pane/ssh-pane-lease-claim.ts b/src/main/ipc/pty/pane/ssh-pane-lease-claim.ts deleted file mode 100644 index 5746814a5f7..00000000000 --- a/src/main/ipc/pty/pane/ssh-pane-lease-claim.ts +++ /dev/null @@ -1,44 +0,0 @@ -import { isTerminalLeafId } from '../../../../shared/stable-pane-id' -import { getRelayPtyId } from '../provider/registry' -import type { Store } from '../../../persistence' - -/** - * Claim a remote PTY for a pane: record the lease, then retire the pane's predecessors. - * - * The lease keeps the RELAY id, because reconnect calls `pty.attach` with target-local ids, while - * the pane binding keeps the app-facing id used for hydration. - * - * Supersession is a second step rather than something `upsertSshRemotePtyLease` finishes on its own - * because it is fenced on the pane's durable binding — it refuses to retire a predecessor the pane - * is still bound to, which would detach a live pane. A caller that leases BEFORE it binds therefore - * trips that fence on every reconnect and, with the upsert as the only trigger, never re-runs: one - * more reattachable lease, and one more `pty.attach` round trip on every later connect, forever. - * Re-running it here from the binding side is what makes the two writes commute. - */ -export function claimSshPaneLease(args: { - store: Store | undefined - connectionId: string | null | undefined - ptyId: string - worktreeId: string | undefined - tabId: string | undefined - leafId: string | undefined -}): void { - const { store, connectionId } = args - if (!store || !connectionId) { - return - } - const leafId = - typeof args.leafId === 'string' && isTerminalLeafId(args.leafId) ? args.leafId : null - store.upsertSshRemotePtyLease({ - targetId: connectionId, - ptyId: getRelayPtyId(connectionId, args.ptyId), - ...(typeof args.worktreeId === 'string' ? { worktreeId: args.worktreeId } : {}), - ...(typeof args.tabId === 'string' ? { tabId: args.tabId } : {}), - ...(leafId ? { leafId } : {}), - state: 'attached', - lastAttachedAt: Date.now() - }) - if (leafId) { - store.supersedeSshRemotePtyLeasesForBoundPane(connectionId, leafId) - } -} diff --git a/src/main/ipc/pty/pane/stable-owner.ts b/src/main/ipc/pty/pane/stable-owner.ts index 7c474aa0082..e5c6a7b8884 100644 --- a/src/main/ipc/pty/pane/stable-owner.ts +++ b/src/main/ipc/pty/pane/stable-owner.ts @@ -10,11 +10,11 @@ import type { IPtyProvider, PtySpawnOptions, PtySpawnResult } from '../../../pro import { getPtySshConnectionId } from '../provider/registry' import { isDaemonEndpointGoneError, + SessionNotFoundError, TerminalHostGoneError, TerminalSessionOwnerUnverifiedError } from '../../../daemon/daemon-errors' import { ptyIncarnationById, ptyOwnership } from '../provider/ownership-state' -import { isHostReportedPtyAbsenceError, isObservedPtyExitEvidence } from '../provider/liveness' import { clearProviderPtyState } from '../provider/state-cleanup' import { spawnCommitBindingOrigin } from '../../../persistence/loading-store/pty-binding-span' @@ -195,25 +195,21 @@ export async function persistAdmittedStablePaneBinding(args: { result: PtySpawnResult worktreeId: string | undefined startupCwd: string | undefined - connectionId: string | null | undefined }): Promise { const expectedBinding = stablePanePersistenceFence(args.owner) if (!args.store || !args.owner || !args.worktreeId || !expectedBinding) { return false } - const persisted = await args.store.persistPtyBinding( - { - worktreeId: args.worktreeId, - tabId: args.owner.tabId, - leafId: args.owner.leafId, - ptyId: args.result.id, - ...(args.result.incarnationId ? { incarnationId: args.result.incarnationId } : {}), - ...(args.startupCwd ? { startupCwd: args.startupCwd } : {}), - expectedBinding, - origin: spawnCommitBindingOrigin(args.result) - }, - args.connectionId ? toSshExecutionHostId(args.connectionId) : undefined - ) + const persisted = await args.store.persistPtyBinding({ + worktreeId: args.worktreeId, + tabId: args.owner.tabId, + leafId: args.owner.leafId, + ptyId: args.result.id, + ...(args.result.incarnationId ? { incarnationId: args.result.incarnationId } : {}), + ...(args.startupCwd ? { startupCwd: args.startupCwd } : {}), + expectedBinding, + origin: spawnCommitBindingOrigin(args.result) + }) if (persisted === false) { throw new Error('terminal_pane_owner_changed') } @@ -250,7 +246,8 @@ export async function attachStablePaneOwner( if (isDaemonEndpointGoneError(error)) { throw new TerminalHostGoneError() } - if (!isHostReportedPtyAbsenceError(error)) { + // Why: only the process that owns the PTY answers this, so its absence is an observed exit. + if (!(error instanceof SessionNotFoundError)) { throw error } const ownerBeforeRetire = args.resolveOwner?.() @@ -263,17 +260,9 @@ export async function attachStablePaneOwner( ) { throw new Error('terminal_pane_owner_changed') } - // `pty.attach` answers absent both for a pid the relay probed and found gone and for an id its - // session map never had — every id minted before a relay restart, checked against nothing. Only - // the marked half observed the process, so only it may certify a death; the rest publishes the - // stop sentinel its sibling handlePtyReattachFailure publishes, which every reader resolves to - // `stop_unverified` (docs/reference/ssh-execution-boundary.md). - runtime?.onPtyExit( - owner.ptyId, - UNVERIFIED_PROCESS_EXIT_CODE, - owner.incarnationId, - isObservedPtyExitEvidence(error) ? { hostExitConfirmed: true } : {} - ) + runtime?.onPtyExit(owner.ptyId, UNVERIFIED_PROCESS_EXIT_CODE, owner.incarnationId, { + hostExitConfirmed: true + }) clearProviderPtyState(owner.ptyId) ptyOwnership.delete(owner.ptyId) if ( diff --git a/src/main/ipc/pty/pane/stable-pane-absence-death-certificate.test.ts b/src/main/ipc/pty/pane/stable-pane-absence-death-certificate.test.ts index 390bcdd451a..f45eb52afec 100644 --- a/src/main/ipc/pty/pane/stable-pane-absence-death-certificate.test.ts +++ b/src/main/ipc/pty/pane/stable-pane-absence-death-certificate.test.ts @@ -1,36 +1,21 @@ -// `attachStablePaneOwner` is the last reader that synthesised a runtime exit from a reattach -// refusal, and it published code 0 — which `orca-runtime-on-pty-exit` records as a death -// certificate. The refusal it acts on is a union: `pty.attach` answers absent both for a pid the -// relay probed and found gone, and for an id its session map never had, which is every id minted -// before a relay restart. Certifying the union orphans a live remote shell and cold-starts a second -// agent onto its transcript (docs/reference/ssh-execution-boundary.md). -// -// The sibling handlePtyReattachFailure has always refused to certify from that union. These pin the -// same rule here, and pin that the marked half — the one refusal the relay backed with a pid probe -// — still earns the certificate, so a genuinely dead PTY is not left `unverifiable` forever. +// `attachStablePaneOwner` synthesises a runtime exit from a reattach refusal, and code 0 there is a +// death certificate. Only the typed answer of the process registry that owns the PTY may earn it; +// an untyped "not found" string has lost whether the owner answered at all. import { describe, expect, it, vi } from 'vitest' import { withDurableRuntimeStore } from '../../../runtime/runtime-durable-store-fixture' import { getDefaultWorkspaceSession } from '../../../../shared/constants' -import { makePaneKey } from '../../../../shared/stable-pane-id' -import { SSH_EXIT_UNCONFIRMED_REASON } from '../../../../shared/pty-liveness-verdict' import type { WorkspaceSessionState } from '../../../../shared/workspace-session-state-types' import { SessionNotFoundError } from '../../../daemon/daemon-errors' import type { Store } from '../../../persistence' -import { - SSH_SESSION_EXPIRED_ERROR, - SshPtyAbsentFromRelayError, - SshPtyProvenExitedOnRelayError -} from '../../../providers/ssh-pty-errors' import type { IPtyProvider } from '../../../providers/types' import { OrcaRuntimeService } from '../../../runtime/orca-runtime' -import { resolvePersistedStablePaneOwner, spawnForStablePane } from './stable-owner' +import { spawnForStablePane } from './stable-owner' const CONNECTION = 'conn-1' const WORKTREE = 'repo-1::/tmp/pane-absence' const TAB = 'tab-1' const LEAF = '1b3f2c4d-5e6a-4b7c-8d9e-0f1a2b3c4d5e' const SIBLING_LEAF = '2c4d3e5f-6a7b-4c8d-9e0f-1a2b3c4d5e6f' -// Ids carry the relay's per-start mint epoch, so this one names a PTY the CURRENT relay never minted. const PTY_ID = 'ssh:conn-1@@pty2:epoch-a:1' const OWNER = { tabId: TAB, leafId: LEAF, ptyId: PTY_ID, hasPersistedBinding: true as const } @@ -122,33 +107,6 @@ async function adoptAfterAttachRefusal(error: unknown): Promise<{ } describe('a stable pane whose reattach was refused', () => { - it('records no death certificate when the relay merely does not know the id', async () => { - const { runtime, spawn } = await adoptAfterAttachRefusal( - new SshPtyAbsentFromRelayError(`${SSH_SESSION_EXPIRED_ERROR}: pty2:epoch-a:1`) - ) - - expect(spawn).toHaveBeenCalledTimes(2) - // The shell may well still be running under the previous daemon's orphaned process tree, so the - // register must keep saying "we could not observe it" — not "it ended". - expect(runtime.getPtyLivenessVerdict(PTY_ID)).toEqual({ - status: 'unverifiable', - reason: SSH_EXIT_UNCONFIRMED_REASON - }) - }) - - it('still certifies the death the relay proved with a pid probe', async () => { - const { runtime, store, spawn } = await adoptAfterAttachRefusal( - new SshPtyProvenExitedOnRelayError(`${SSH_SESSION_EXPIRED_ERROR}: pty2:epoch-a:1`) - ) - - expect(spawn).toHaveBeenCalledTimes(2) - expect(runtime.getPtyLivenessVerdict(PTY_ID)).toEqual({ status: 'exited' }) - // If nothing ever retired, a proven-dead pane would reattach to a corpse on every adoption. - expect( - resolvePersistedStablePaneOwner(store, makePaneKey(TAB, LEAF), WORKTREE, CONNECTION) - ).toBeNull() - }) - it('certifies an absence reported by the process registry that owns the PTY', async () => { // The daemon (or the in-process map) answering here is the owner of the process, and an // endpoint that had gone raises TerminalHostGoneError above, so this absence is an observation @@ -159,9 +117,6 @@ describe('a stable pane whose reattach was refused', () => { }) it('refuses to abandon the binding on an untyped "not found" string', async () => { - // The relay's raw wire wording. The SSH reattach path types it before any pane sees it, so an - // untyped one reached this gate having lost every distinction the type carries — including - // whether the answer came from the host that owns the process at all. const { store, read } = paneStore() const before = JSON.stringify(read()) const runtime = runtimeOwning(store) diff --git a/src/main/ipc/pty/pane/stable-pane-relay-absence-respawn.test.ts b/src/main/ipc/pty/pane/stable-pane-absence-respawn.test.ts similarity index 87% rename from src/main/ipc/pty/pane/stable-pane-relay-absence-respawn.test.ts rename to src/main/ipc/pty/pane/stable-pane-absence-respawn.test.ts index ce3c578cfa0..2a2addc4adb 100644 --- a/src/main/ipc/pty/pane/stable-pane-relay-absence-respawn.test.ts +++ b/src/main/ipc/pty/pane/stable-pane-absence-respawn.test.ts @@ -1,11 +1,10 @@ import { describe, expect, it, vi } from 'vitest' import { withDurableRuntimeStore } from '../../../runtime/runtime-durable-store-fixture' import type { WorkspaceSessionState } from '../../../../shared/workspace-session-state-types' -import { TerminalSessionOwnerUnverifiedError } from '../../../daemon/daemon-errors' import { - SSH_SESSION_EXPIRED_ERROR, - SshPtyAbsentFromRelayError -} from '../../../providers/ssh-pty-errors' + SessionNotFoundError, + TerminalSessionOwnerUnverifiedError +} from '../../../daemon/daemon-errors' import type { Store } from '../../../persistence' import type { IPtyProvider } from '../../../providers/types' import { spawnForStablePane, type StablePaneOwner } from './stable-owner' @@ -82,7 +81,7 @@ function sessionStore(leaves: string[]): { store: Store; read: () => WorkspaceSe } } -describe('stable pane adoption after the relay reports the PTY absent', () => { +describe('stable pane adoption after the owning host reports the PTY absent', () => { it.each([false, true])( 'reattaches a live pane without launching a provider process (settled worker: %s)', async (settledWorker) => { @@ -126,10 +125,8 @@ describe('stable pane adoption after the relay reports the PTY absent', () => { } ) - it('spawns fresh once the relay has positively answered for that id', async () => { - const { run, spawn } = spawnAfterAttachRejection( - new SshPtyAbsentFromRelayError(`${SSH_SESSION_EXPIRED_ERROR}: pty-1`) - ) + it('spawns fresh once the owning host has positively answered for that id', async () => { + const { run, spawn } = spawnAfterAttachRejection(new SessionNotFoundError(OWNER.ptyId)) const result = await run() @@ -139,10 +136,10 @@ describe('stable pane adoption after the relay reports the PTY absent', () => { expect(result.owner).toBeNull() }) - // A restarted relay renumbers from pty-1, so the message alone cannot distinguish absence from a - // lost link — only the class may authorise abandoning the binding. + // The message alone cannot distinguish absence from a lost link; only the class may authorise + // abandoning the binding. it.each([ - ['an expired session with no relay evidence', `${SSH_SESSION_EXPIRED_ERROR}: pty-1`], + ['an untyped absence message', `Session not found: ${OWNER.ptyId}`], ['a lost link', 'SSH connection lost, reconnecting...'], ['a request timeout', 'Request "pty.attach" timed out after 10000ms'] ])('keeps the binding and refuses to respawn after %s', async (_label, message) => { @@ -158,10 +155,10 @@ describe('stable pane adoption after the relay reports the PTY absent', () => { describe('with persistence actually reached', () => { it('retires only the absent leaf and leaves the tab and its sibling bound', async () => { const { store, read } = sessionStore([LEAF, SIBLING_LEAF]) - const { run, spawn } = spawnAfterAttachRejection( - new SshPtyAbsentFromRelayError(`${SSH_SESSION_EXPIRED_ERROR}: pty-1`), - { store, worktreeId: WORKTREE } - ) + const { run, spawn } = spawnAfterAttachRejection(new SessionNotFoundError(OWNER.ptyId), { + store, + worktreeId: WORKTREE + }) const result = await run() @@ -180,10 +177,10 @@ describe('stable pane adoption after the relay reports the PTY absent', () => { // survival then rests entirely on the renderer. If that ever regresses, this is the tripwire. it('drops the tab when the absent leaf was the only one, leaving re-persistence to the renderer', async () => { const { store, read } = sessionStore([LEAF]) - const { run, spawn } = spawnAfterAttachRejection( - new SshPtyAbsentFromRelayError(`${SSH_SESSION_EXPIRED_ERROR}: pty-1`), - { store, worktreeId: WORKTREE } - ) + const { run, spawn } = spawnAfterAttachRejection(new SessionNotFoundError(OWNER.ptyId), { + store, + worktreeId: WORKTREE + }) const result = await run() diff --git a/src/main/ipc/pty/provider/liveness.ts b/src/main/ipc/pty/provider/liveness.ts index 5c4265bdc6e..ed1758e02cd 100644 --- a/src/main/ipc/pty/provider/liveness.ts +++ b/src/main/ipc/pty/provider/liveness.ts @@ -1,14 +1,7 @@ -import { isRemoteAgentHooksEnabled } from '../../../../shared/agent-hook-relay' import type { AgentSessionOwnerBinding } from '../../../../shared/agent-session-host-authority' import { agentSessionOwnerBindingsEqual } from '../../../../shared/claimed-agent-pty-owner' import { addNodePtyRecoveryHint } from '../../../daemon/node-pty-error-hints' -import { SessionNotFoundError } from '../../../daemon/daemon-errors' import type { Store } from '../../../persistence' -import { - isSshPtyAbsentFromRelayError, - isSshPtyNotFoundError, - isSshPtyProvenExitedOnRelayError -} from '../../../providers/ssh-pty-errors' import type { IPtyProvider } from '../../../providers/types' import { ptyIncarnationById, ptyOwnership } from './ownership-state' import { getRelayPtyId } from './registry' @@ -18,30 +11,6 @@ import { } from '../delivery/visibility-state' import { clearProviderPtyState } from './state-cleanup' -export function stripRemotePaneEnvWhenHooksDisabled( - connectionId: string | null | undefined, - env: Record | undefined -): Record | undefined { - if (!connectionId || isRemoteAgentHooksEnabled()) { - return env - } - if ( - !env || - (!('ORCA_PANE_KEY' in env) && - !('ORCA_TAB_ID' in env) && - !('ORCA_WORKTREE_ID' in env) && - !('ORCA_AGENT_LAUNCH_TOKEN' in env)) - ) { - return env - } - const stripped = { ...env } - delete stripped.ORCA_PANE_KEY - delete stripped.ORCA_TAB_ID - delete stripped.ORCA_WORKTREE_ID - delete stripped.ORCA_AGENT_LAUNCH_TOKEN - return stripped -} - export function normalizeNodePtySpawnError(err: unknown): Error { const rawMessage = err instanceof Error ? err.message : String(err) const hintedMessage = addNodePtyRecoveryHint(rawMessage) @@ -58,40 +27,7 @@ export function normalizeNodePtySpawnError(err: unknown): Error { export function isPtyAlreadyGoneError(err: unknown): boolean { const message = err instanceof Error ? err.message : String(err) - return ( - isSshPtyNotFoundError(err) || - // Why: the reattach path rewrites the relay's wording to SSH_SESSION_EXPIRED and only this - // class preserves that the relay itself answered "absent" rather than the link dropping. - isSshPtyAbsentFromRelayError(err) || - /Session not found/i.test(message) - ) -} - -/** - * Narrower than {@link isPtyAlreadyGoneError}, for the one caller that retires a durable pane - * binding rather than just releasing in-memory state: only a typed answer from the host that owns - * the process may authorise that. The bare `PTY ".+" not found` text is the relay's raw wire - * wording, which the SSH reattach path always types before it reaches a pane; matching the text - * instead would let any untyped string carrying that phrase unbind a live pane - * (docs/reference/ssh-execution-boundary.md). - */ -export function isHostReportedPtyAbsenceError(err: unknown): boolean { - return isSshPtyAbsentFromRelayError(err) || err instanceof SessionNotFoundError -} - -/** - * The half of {@link isHostReportedPtyAbsenceError} that actually observed the process, and so the - * only half that may certify an exit. - * - * The relay's plain absence answer is excluded because `pty.attach` gives it for an id its session - * map never had as readily as for a pid it probed — after a relay restart, every id the previous - * one minted. `SessionNotFoundError` is included because the process answering is the one that owns - * the PTY: the in-process registry itself, or a daemon whose endpoint is live (a gone endpoint - * raises `isDaemonEndpointGoneError` instead), so its absence is an observation rather than a lost - * route (docs/reference/ssh-execution-boundary.md). - */ -export function isObservedPtyExitEvidence(err: unknown): boolean { - return isSshPtyProvenExitedOnRelayError(err) || err instanceof SessionNotFoundError + return /Session not found/i.test(message) } export function delay(ms: number): Promise { diff --git a/src/main/ipc/pty/provider/ownership-state.ts b/src/main/ipc/pty/provider/ownership-state.ts index 3132ff59e4d..d8eae2eb838 100644 --- a/src/main/ipc/pty/provider/ownership-state.ts +++ b/src/main/ipc/pty/provider/ownership-state.ts @@ -22,14 +22,9 @@ export function setPtyOwnership(id: string, hostId: ExecutionHostId): void { ptyOwnership.set(id, hostId) } -/** An adopted PTY that names no SSH target keeps the host already recorded for its id. */ -export function setAdoptedPtyOwnership(id: string, connectionId: string | null | undefined): void { - ptyOwnership.set( - id, - connectionId - ? toSshExecutionHostId(connectionId) - : (ptyOwnership.get(id) ?? LOCAL_EXECUTION_HOST_ID) - ) +/** An adopted PTY keeps the host already recorded for its id. */ +export function setAdoptedPtyOwnership(id: string): void { + ptyOwnership.set(id, ptyOwnership.get(id) ?? LOCAL_EXECUTION_HOST_ID) } export function restorePtyIncarnation(id: string, incarnationId: string): void { diff --git a/src/main/ipc/pty/provider/registry-host-routing.test.ts b/src/main/ipc/pty/provider/registry-host-routing.test.ts index 601f28d9b46..544a7c3534e 100644 --- a/src/main/ipc/pty/provider/registry-host-routing.test.ts +++ b/src/main/ipc/pty/provider/registry-host-routing.test.ts @@ -13,11 +13,8 @@ import { setLocalPtyProvider, tryGetProviderForPty } from './registry' -import { - isHostReportedPtyAbsenceError, - isObservedPtyExitEvidence, - isPtyAlreadyGoneError -} from './liveness' +import { isPtyAlreadyGoneError } from './liveness' +import { SessionNotFoundError } from '../../../daemon/daemon-errors' function answeringProvider(): IPtyProvider { // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: the routed calls below only touch these members. @@ -103,7 +100,7 @@ describe('PTY registry keyed by execution host', () => { } expect(error).toBeInstanceOf(PtyHostNotDispatchableError) expect(isPtyAlreadyGoneError(error)).toBe(false) - expect(isHostReportedPtyAbsenceError(error)).toBe(false) - expect(isObservedPtyExitEvidence(error)).toBe(false) + // Stable-pane adoption certifies an exit only from this class. + expect(error).not.toBeInstanceOf(SessionNotFoundError) }) }) diff --git a/src/main/ipc/pty/runtime/controller.ts b/src/main/ipc/pty/runtime/controller.ts index 9148bd6771b..00cac683ca5 100644 --- a/src/main/ipc/pty/runtime/controller.ts +++ b/src/main/ipc/pty/runtime/controller.ts @@ -32,7 +32,6 @@ import { waitForRendererSerializerFromRuntimeController, writePtyFromRuntimeController } from './operations' -import { recordUnconfirmedExplicitSshStop } from './undelivered-ssh-kill' import { supportsForegroundProcessEvidenceFromRuntimeController } from './foreground-process-evidence-capability' import { listProcessesFromRuntimeController, @@ -66,12 +65,6 @@ export function installPtyRuntimeController(deps: PtyRuntimeControllerDeps): voi retireRejectedPty: (ptyId, stopConfirmed) => retireRejectedPtyFromRuntimeController(deps, ptyId, stopConfirmed), stopAndWait: (ptyId, opts) => stopAndWaitPtyFromRuntimeController(deps, ptyId, opts), - recordUnconfirmedStop: (ptyId) => - recordUnconfirmedExplicitSshStop({ - store: deps.store, - ptyId, - reversible: runtime?.intentionalPtyStops?.isReversibleStopInFlight(ptyId) ?? false - }), getForegroundProcess: (ptyId) => getForegroundProcessFromRuntimeController(ptyId), inspectProcess: (ptyId, options) => inspectProcessFromRuntimeController(ptyId, options), confirmForegroundProcess: (ptyId) => confirmForegroundProcessFromRuntimeController(ptyId), diff --git a/src/main/ipc/pty/runtime/kill.ts b/src/main/ipc/pty/runtime/kill.ts index f35de0e774d..ba0cfb7fe15 100644 --- a/src/main/ipc/pty/runtime/kill.ts +++ b/src/main/ipc/pty/runtime/kill.ts @@ -7,7 +7,6 @@ import { resolvePtyExecutionHost } from '../provider/registry' import { isPtyAlreadyGoneError, delay, verifyPtyStopped } from '../provider/liveness' -import { recordUndeliveredSshPtyKill } from './undelivered-ssh-kill' import type { PtyRuntimeControllerDeps } from './controller-deps' import { errorMessage } from '../../../../shared/error-message' @@ -39,15 +38,6 @@ export function killPtyFromRuntimeController( } = deps runtime?.markPtyStopRequested?.(ptyId) const connectionId = getPtySshConnectionId(ptyId) - const recordUndelivered = (incarnationId?: string): void => { - recordUndeliveredSshPtyKill({ - store, - ptyId, - connectionId, - reversible: runtime?.intentionalPtyStops?.isReversibleStopInFlight(ptyId) ?? false, - incarnationId - }) - } // Why: close runtime tails without clearing provider ownership, so // a retry can still target a PTY that survived the failed shutdown. const reportStopFailure = (err: unknown): void => { @@ -56,9 +46,6 @@ export function killPtyFromRuntimeController( if (retiredRejectedPtyIds.has(ptyId)) { return } - if (connectionId) { - runtime?.markPtyLivenessUnverifiable?.(ptyId, msg) - } runtime?.onPtyExit(ptyId, -1, ptyIncarnationById.get(ptyId)) } const killWithCurrentProvider = (): boolean => { @@ -71,11 +58,8 @@ export function killPtyFromRuntimeController( // provider was unregistered. Tombstone the lease so reconnect does // not revive a terminal the user explicitly closed. const incarnationId = finishPtyShutdown(ptyId, connectionId, store) - // The relay was never asked, so the remote shell is still running. Keep the order. - recordUndelivered(incarnationId) publishSyntheticExit(deps, ptyId, -1, incarnationId) runtime?.markPtyLivenessUnverifiable?.(ptyId, SSH_PROVIDER_UNREGISTERED_REASON) - return false } return false } @@ -98,19 +82,13 @@ export function killPtyFromRuntimeController( return } reportStopFailure(err) - // Outside the `retired` guard: the remote process outlives this client's bookkeeping - // either way, and the intent is what the next handshake replays. - recordUndelivered() }) return true } const startupPromise = getLocalPtyProviderStartupPromise(resolvePtyExecutionHost(ptyId)) if (startupPromise) { // Why: select the provider after the daemon swap; the fallback first can report success while orphaning a daemon PTY. - void startupPromise.then(killWithCurrentProvider).catch((err) => { - reportStopFailure(err) - recordUndelivered() - }) + void startupPromise.then(killWithCurrentProvider).catch(reportStopFailure) return true } return killWithCurrentProvider() @@ -216,9 +194,6 @@ export async function stopAndWaitPtyFromRuntimeController( }) } catch (err) { if (!isPtyAlreadyGoneError(err)) { - if (connectionId) { - runtime?.markPtyLivenessUnverifiable?.(ptyId, errorMessage(err)) - } console.warn(`[pty] Failed to stop PTY ${ptyId}: ${errorMessage(err)}`) return false } @@ -229,9 +204,6 @@ export async function stopAndWaitPtyFromRuntimeController( return false } } catch (err) { - if (connectionId) { - runtime?.markPtyLivenessUnverifiable?.(ptyId, errorMessage(err)) - } console.warn(`[pty] Failed to verify PTY ${ptyId} stopped: ${errorMessage(err)}`) return false } diff --git a/src/main/ipc/pty/runtime/spawn-commit.ts b/src/main/ipc/pty/runtime/spawn-commit.ts index deb361e2ab2..1472a842b44 100644 --- a/src/main/ipc/pty/runtime/spawn-commit.ts +++ b/src/main/ipc/pty/runtime/spawn-commit.ts @@ -22,15 +22,11 @@ import { seedTerminalRestoreRecordsFromSpawnResult } from '../pane/agent-session import { seedHeadlessTerminalFromSpawnResult } from '../pane/terminal-spawn-restore' import { recordPtySpawnTelemetry } from '../pane/spawn-telemetry' import { persistAdmittedStablePaneBinding } from '../pane/stable-owner' -import { claimSshPaneLease } from '../pane/ssh-pane-lease-claim' import { isNativeWindowsLocalPtySpawn, markNativeWindowsConptyPty } from '../../../runtime/terminal-model-query-authority' -import { - getConnectionExecutionHostId, - toSshExecutionHostId -} from '../../../../shared/execution-host' +import { LOCAL_EXECUTION_HOST_ID } from '../../../../shared/execution-host' import { createTerminalSessionStateSaveFailureMessage } from '../../../../shared/terminal-session-state-save-failure' import { resolvePaneSpawnReservation } from '../pane/spawn-reservation' import { admitProviderReattachLaunchIdentity } from '../pane/launch-authority' @@ -49,7 +45,7 @@ export async function commitRuntimePtySpawn(ctx: RuntimePtySpawnState) { async function commitReservedRuntimePtySpawn(ctx: RuntimePtySpawnState) { const args = ctx.args - admitPtyReattachOwnership(ctx.deps.runtime, ctx.result, args.connectionId) + admitPtyReattachOwnership(ctx.deps.runtime, ctx.result) const providerReattachLaunchIdentity = admitProviderReattachLaunchIdentity(ctx.result) if ( isNativeWindowsLocalPtySpawn({ @@ -69,8 +65,7 @@ async function commitReservedRuntimePtySpawn(ctx: RuntimePtySpawnState) { owner: ctx.stablePaneOwner, result: ctx.result, worktreeId: ctx.hostSessionBinding?.worktreeId, - startupCwd: ctx.cwd, - connectionId: args.connectionId + startupCwd: ctx.cwd }) } catch (error) { if (error instanceof Error && error.message === 'terminal_pane_owner_changed') { @@ -82,8 +77,8 @@ async function commitReservedRuntimePtySpawn(ctx: RuntimePtySpawnState) { }) } if (ctx.result.agentSessionEnsure?.disposition === 'adopted') { - // Why: an adoption is an attach to a live owner by definition, but the SSH relay's adopted - // reply omits isReattach; derive it once so the size commit and the reservation agree. + // Why: an adoption is an attach to a live owner by definition, but an adopted reply can omit + // isReattach; derive it once so the size commit and the reservation agree. const adoptedResult = { ...ctx.result, isReattach: true } const owner = ctx.result.agentSessionEnsure.owner const rejectedRegistration = registerPersistedPtySpawn( @@ -91,7 +86,7 @@ async function commitReservedRuntimePtySpawn(ctx: RuntimePtySpawnState) { ctx.hostSessionBinding?.store ?? ctx.deps.store, ctx.result.id, owner.surface.worktreeId, - args.connectionId ?? null, + null, { tabId: owner.surface.tabId, leafId: owner.surface.leafId, @@ -108,22 +103,20 @@ async function commitReservedRuntimePtySpawn(ctx: RuntimePtySpawnState) { ctx.result, ctx.hostSessionBinding?.expectedSourceBinding ) - setAdoptedPtyOwnership(ctx.result.id, args.connectionId) + setAdoptedPtyOwnership(ctx.result.id) ctx.deps.runtime?.registerPreAllocatedHandleForPty(ctx.result.id, owner.surface.terminalHandle) if (ctx.result.incarnationId) { ptyIncarnationById.set(ctx.result.id, ctx.result.incarnationId) } - if (!args.connectionId) { - ctx.deps.options?.onCodexHomePtySpawned?.({ - id: ctx.result.id, - codexHomePath: ctx.selectedCodexHomePath, - reattached: true, - startedAt: ctx.codexHomeLaunchStartedAt, - startedSequence: ctx.codexHomeLaunchStartedSequence, - ...codexReattachedHomeRouteField(ctx.reattachedCodexHomeRoutes, ctx.result.id, true), - ...(ctx.env ? { launchEnv: ctx.env } : {}) - }) - } + ctx.deps.options?.onCodexHomePtySpawned?.({ + id: ctx.result.id, + codexHomePath: ctx.selectedCodexHomePath, + reattached: true, + startedAt: ctx.codexHomeLaunchStartedAt, + startedSequence: ctx.codexHomeLaunchStartedSequence, + ...codexReattachedHomeRouteField(ctx.reattachedCodexHomeRoutes, ctx.result.id, true), + ...(ctx.env ? { launchEnv: ctx.env } : {}) + }) // Why: this branch returns before the normal commit site; without this the cache keeps // whatever the caller requested. commitRuntimePtySize(ctx, adoptedResult) @@ -158,9 +151,7 @@ async function commitReservedRuntimePtySpawn(ctx: RuntimePtySpawnState) { ...(placement ? { placement } : {}), origin: spawnCommitBindingOrigin(ctx.result, expectedSourceBinding) } - const persisted = args.connectionId - ? await store.persistPtyBinding(binding, toSshExecutionHostId(args.connectionId)) - : await store.persistPtyBinding(binding) + const persisted = await store.persistPtyBinding(binding) if (persisted === false) { throw new Error('terminal_split_source_not_found') } @@ -181,7 +172,7 @@ async function commitReservedRuntimePtySpawn(ctx: RuntimePtySpawnState) { ctx.hostSessionBinding?.store ?? ctx.deps.store, ctx.result.id, args.worktreeId, - args.connectionId ?? null, + null, // Why: thread validated pane identity so main can back a pending mobile create even if graph-sync stalls (#7587). typeof args.tabId === 'string' && isValidTerminalTabId(args.tabId) && @@ -195,9 +186,7 @@ async function commitReservedRuntimePtySpawn(ctx: RuntimePtySpawnState) { ...(providerReattachLaunchIdentity ? { providerReattachLaunchIdentity } : {}) } : undefined, - !args.connectionId - ? shouldSkipCodexHomeEnvForWindowsShell(ctx.daemonShellOverride, ctx.cwd) - : undefined + shouldSkipCodexHomeEnvForWindowsShell(ctx.daemonShellOverride, ctx.cwd) ) if (rejectedRegistration) { await rejectedRegistration @@ -215,19 +204,11 @@ async function commitReservedRuntimePtySpawn(ctx: RuntimePtySpawnState) { if (args.preAllocatedHandle && !ctx.stablePaneOwner?.handle) { ctx.deps.runtime?.registerPreAllocatedHandleForPty(ctx.result.id, args.preAllocatedHandle) } - ptyOwnership.set(ctx.result.id, getConnectionExecutionHostId(args.connectionId)) + ptyOwnership.set(ctx.result.id, LOCAL_EXECUTION_HOST_ID) if (ctx.result.incarnationId) { ptyIncarnationById.set(ctx.result.id, ctx.result.incarnationId) } - claimSshPaneLease({ - store: ctx.deps.store, - connectionId: args.connectionId, - ptyId: ctx.result.id, - worktreeId: args.worktreeId, - tabId: args.tabId, - leafId: args.leafId - }) commitRuntimePtySize(ctx, ctx.result) if (ctx.effectiveSessionAppId !== undefined && ctx.effectiveSessionAppId !== ctx.result.id) { ptySizes.delete(ctx.effectiveSessionAppId) @@ -261,38 +242,34 @@ async function commitReservedRuntimePtySpawn(ctx: RuntimePtySpawnState) { if (paneKey && pendingSerializer) { pendingPtyIdBySerializerGeneration.set(pendingSerializer.gen, ctx.result.id) } - if (!args.connectionId) { - registerPty({ - ptyId: ctx.result.id, - worktreeId: args.worktreeId ?? null, - sessionId: ctx.sessionId ?? null, - paneKey, - pid: - typeof ctx.result.pid === 'number' && Number.isFinite(ctx.result.pid) && ctx.result.pid > 0 - ? ctx.result.pid - : null - }) - } + registerPty({ + ptyId: ctx.result.id, + worktreeId: args.worktreeId ?? null, + sessionId: ctx.sessionId ?? null, + paneKey, + pid: + typeof ctx.result.pid === 'number' && Number.isFinite(ctx.result.pid) && ctx.result.pid > 0 + ? ctx.result.pid + : null + }) // Why: runtime-owned/background spawns bypass mounted-pane state, so inventory consumers need an explicit signal. ctx.deps.sendPtySpawnedToRenderer(ctx.result.id) - if (!args.connectionId) { - ctx.deps.options?.onCodexHomePtySpawned?.({ - id: ctx.result.id, - codexHomePath: ctx.selectedCodexHomePath, - startedAt: ctx.codexHomeLaunchStartedAt, - startedSequence: ctx.codexHomeLaunchStartedSequence, - ...codexReattachedHomeRouteField( - ctx.reattachedCodexHomeRoutes, - ctx.result.id, - ctx.result.isReattach === true - ), - ...(ctx.result.isReattach === true - ? { reattached: true } - : ctx.env - ? { launchEnv: ctx.env } - : {}) - }) - } + ctx.deps.options?.onCodexHomePtySpawned?.({ + id: ctx.result.id, + codexHomePath: ctx.selectedCodexHomePath, + startedAt: ctx.codexHomeLaunchStartedAt, + startedSequence: ctx.codexHomeLaunchStartedSequence, + ...codexReattachedHomeRouteField( + ctx.reattachedCodexHomeRoutes, + ctx.result.id, + ctx.result.isReattach === true + ), + ...(ctx.result.isReattach === true + ? { reattached: true } + : ctx.env + ? { launchEnv: ctx.env } + : {}) + }) const response = { id: ctx.result.id, ...(ctx.result.incarnationId ? { incarnationId: ctx.result.incarnationId } : {}), diff --git a/src/main/ipc/pty/runtime/spawn-execute.ts b/src/main/ipc/pty/runtime/spawn-execute.ts index 76d15d8650f..46540384be6 100644 --- a/src/main/ipc/pty/runtime/spawn-execute.ts +++ b/src/main/ipc/pty/runtime/spawn-execute.ts @@ -1,5 +1,5 @@ import type { PtySpawnResult } from '../../../providers/types' -import { ptyIncarnationById, deletePtyOwnership } from '../provider/ownership-state' +import { ptyIncarnationById } from '../provider/ownership-state' import { ptySizes } from '../delivery/visibility-state' import { tryGetProviderForPty } from '../provider/registry' import { ensureWslHookRelayForReattach } from '../../../agent-hooks/wsl-hook-relay-reattach' @@ -11,11 +11,6 @@ import { import { spawnForStablePane, resolveStablePaneOwner } from '../pane/stable-owner' import { clearProviderPtyState } from '../provider/state-cleanup' import { isProviderAgentSessionOwnerLive, normalizeNodePtySpawnError } from '../provider/liveness' -import { - SSH_SESSION_EXPIRED_ERROR, - isSshPtyAbsentFromRelayError, - isSshPtyIdentityMismatchError -} from '../../../providers/ssh-pty-errors' import type { RuntimePtySpawnState } from './spawn-state' import { markRuntimeSpawnHiddenBeforeSpawn } from './spawn-hidden-delivery' @@ -116,8 +111,7 @@ export async function executeRuntimePtySpawn(ctx: RuntimePtySpawnState): Promise isLive: async (owner) => { const ownerProvider = tryGetProviderForPty(owner.ptyId) if (!ownerProvider) { - // Why: a disconnected relay may keep its PTY alive during the - // grace window; missing transport is unknown, never absence. + // Why: an owner on a host this process cannot reach is unknown, never absent. throw new Error('execution_owner_unavailable') } return await isProviderAgentSessionOwnerLive(ownerProvider, owner) @@ -192,14 +186,10 @@ export async function executeRuntimePtySpawn(ctx: RuntimePtySpawnState): Promise ctx.snapshotKittyFlagsCoverReconciledSeq = false } } - ensureWslHookRelayForReattach(ctx.result, args.connectionId) + ensureWslHookRelayForReattach(ctx.result) ctx.deps.runtime?.preparePtyExecutionContext?.( ctx.result.id, - args.connectionId - ? null - : ctx.result.wslDistro === undefined - ? ctx.expectedWslDistro - : ctx.result.wslDistro + ctx.result.wslDistro === undefined ? ctx.expectedWslDistro : ctx.result.wslDistro ) } catch (err) { if ( @@ -225,51 +215,19 @@ export async function executeRuntimePtySpawn(ctx: RuntimePtySpawnState): Promise ctx.pendingRegistrationPtyId = null } const spawnError = normalizeNodePtySpawnError(err) - const isIdentityMismatch = - isSshPtyIdentityMismatchError(spawnError) || isSshPtyIdentityMismatchError(rawMessage) - const isExpiredSshSession = - Boolean(args.connectionId) && - (spawnError.message.includes(SSH_SESSION_EXPIRED_ERROR) || - rawMessage.includes(SSH_SESSION_EXPIRED_ERROR)) - // The message alone cannot carry this decision. All three reattach refusals are minted with the - // same `SSH_SESSION_EXPIRED` text, and only one of them observed the process: `restoreRequired` - // means the PTY is LIVE and only its source stream needs rebuilding, which - // `ssh-pty-errors.ts` states outright. Expiring its lease and deleting its ownership erases - // this client's last record of a running remote process, and #9819's sweep reads a PTY it has - // no record of as one it may SIGKILL on the next connect. Only positive host-reported absence - // may reach that bookkeeping; being too strict here merely leaves a dead lease for the next - // reattach to retire on real host evidence. - const relayReportedSessionAbsent = isExpiredSshSession && isSshPtyAbsentFromRelayError(err) const exitedBeforeSpawnReply = ctx.rejectedRegistrationCandidate?.exitedBeforeSpawnReply === true if (ctx.effectiveSessionAppId !== undefined) { if ( ctx.hadSessionSizeBeforeAttach && ctx.sessionSizeBeforeAttach && - (isIdentityMismatch || (!isExpiredSshSession && !exitedBeforeSpawnReply)) + !exitedBeforeSpawnReply ) { ptySizes.set(ctx.effectiveSessionAppId, ctx.sessionSizeBeforeAttach) } else { ptySizes.delete(ctx.effectiveSessionAppId) } } - if ( - args.connectionId && - ctx.effectiveSessionRelayId !== undefined && - relayReportedSessionAbsent - ) { - if (ctx.effectiveSessionAppId !== undefined && !isIdentityMismatch) { - clearProviderPtyState(ctx.effectiveSessionAppId) - deletePtyOwnership(ctx.effectiveSessionAppId) - } - if (!isIdentityMismatch) { - ctx.deps.store?.markSshRemotePtyLease( - args.connectionId, - ctx.effectiveSessionRelayId, - 'expired' - ) - } - } if (ctx.isNewDaemonSession && ctx.sessionId !== undefined) { clearProviderPtyState(ctx.sessionId) } diff --git a/src/main/ipc/pty/runtime/spawn-preflight.ts b/src/main/ipc/pty/runtime/spawn-preflight.ts index 0e7112619a1..22d33bf708d 100644 --- a/src/main/ipc/pty/runtime/spawn-preflight.ts +++ b/src/main/ipc/pty/runtime/spawn-preflight.ts @@ -5,7 +5,7 @@ import type { PtySpawnResult } from '../../../providers/types' import { LocalPtyProvider } from '../../../providers/local-pty-provider' import { isValidTerminalTabId } from '../../../../shared/terminal-tab-id' import { isTerminalLeafId } from '../../../../shared/stable-pane-id' -import { getAppPtyId, getProvider, getRelayPtyId } from '../provider/registry' +import { getProvider } from '../provider/registry' import { buildPtyHostEnv } from '../host-env/assembly' import { getCompatibleSelectedCodexHomePath, @@ -21,7 +21,6 @@ import { recoverFreshSpawnProviderRouting, routesFreshSpawnsToLocalProvider } from '../host-env/fresh-spawn-routing' -import { stripRemotePaneEnvWhenHooksDisabled } from '../provider/liveness' import { isTuiAgent } from '../../../../shared/tui-agent-config' import { isSafePtySessionId, @@ -54,20 +53,15 @@ export async function prepareRuntimePtySpawn( } } ctx.cwd = ctx.deps.resolvePtySpawnStartupCwd(args.worktreeId, args.cwd) + // Only this machine spawns here: an `ssh:` connection throws, so every line below is local. ctx.provider = getProvider(getConnectionExecutionHostId(args.connectionId)) const freshSpawnRecovery = ctx.preAdoptedStablePane ? undefined - : recoverFreshSpawnProviderRouting( - ctx.provider, - args.connectionId, - args.sessionId, - args.isNewSession - ) + : recoverFreshSpawnProviderRouting(ctx.provider, args.sessionId, args.isNewSession) if (freshSpawnRecovery) { await freshSpawnRecovery } - ctx.isClaudeLaunch = - !ctx.preAdoptedStablePane && !args.connectionId && isClaudeLaunchCommand(args.command) + ctx.isClaudeLaunch = !ctx.preAdoptedStablePane && isClaudeLaunchCommand(args.command) // Why: runtime-created terminals carry no renderer-computed projectRuntime; resolve from worktreeId to honor the project's Windows runtime. // `args.shellOverride` is the per-request pick (`terminal create --shell`), read here the way // the renderer twin (ipc/spawn-preflight.ts) reads a tab's override. Without it a runtime create @@ -75,7 +69,7 @@ export async function prepareRuntimePtySpawn( // default shell with the request typed into it. Still Windows-only: the override names a // Windows shell, and spawn-options applies it under the same platform gate. ctx.terminalRuntimeOptions = - process.platform === 'win32' && !args.connectionId + process.platform === 'win32' ? resolveLocalWindowsTerminalRuntimeOptions({ requestedShellOverride: args.shellOverride, settings: ctx.deps.getSettings?.(), @@ -85,17 +79,12 @@ export async function prepareRuntimePtySpawn( }) : { shellOverride: - args.shellOverride ?? - (process.platform === 'win32' - ? undefined - : ctx.deps.getSettings?.()?.terminalDefaultShell || undefined), + args.shellOverride ?? (ctx.deps.getSettings?.()?.terminalDefaultShell || undefined), terminalWindowsWslDistro: null } ctx.daemonShellOverride = ctx.terminalRuntimeOptions.shellOverride ctx.isDaemonHostSpawn = - !args.connectionId && - !(ctx.provider instanceof LocalPtyProvider) && - !routesFreshSpawnsToLocalProvider(ctx.provider) + !(ctx.provider instanceof LocalPtyProvider) && !routesFreshSpawnsToLocalProvider(ctx.provider) ctx.callerRequestedSessionId = args.sessionId?.trim() ctx.requestedSessionId = ctx.callerRequestedSessionId ?? @@ -105,22 +94,18 @@ export async function prepareRuntimePtySpawn( ctx.sessionId = ctx.requestedSessionId ?? (ctx.isDaemonHostSpawn ? mintPtySessionId(args.worktreeId) : undefined) - ctx.effectiveSessionRelayId = - ctx.sessionId !== undefined ? getRelayPtyId(args.connectionId, ctx.sessionId) : undefined - ctx.effectiveSessionAppId = - ctx.sessionId !== undefined ? getAppPtyId(args.connectionId, ctx.sessionId) : undefined + ctx.effectiveSessionAppId = ctx.sessionId ctx.isNewDaemonSession = !ctx.preAdoptedStablePane && ctx.isDaemonHostSpawn && (ctx.callerRequestedSessionId === undefined || args.isNewSession === true) - ctx.expectedWslDistro = !args.connectionId - ? (resolveWslSessionContext({ - cwd: ctx.cwd, - sessionId: ctx.sessionId, - shellOverride: ctx.terminalRuntimeOptions.shellOverride, - terminalWindowsWslDistro: ctx.terminalRuntimeOptions.terminalWindowsWslDistro - })?.distro ?? null) - : null + ctx.expectedWslDistro = + resolveWslSessionContext({ + cwd: ctx.cwd, + sessionId: ctx.sessionId, + shellOverride: ctx.terminalRuntimeOptions.shellOverride, + terminalWindowsWslDistro: ctx.terminalRuntimeOptions.terminalWindowsWslDistro + })?.distro ?? null ctx.codexSelectionTarget = getCodexSelectionTargetForPty( ctx.daemonShellOverride, ctx.cwd, @@ -142,7 +127,7 @@ export async function prepareRuntimePtySpawn( // Why: the drop still applies here, but this controller's result has no field for // notifyResumeUnavailable — runtime/relay panes start fresh without the notice. ctx.launchCommand = codexResumeLaunch.command - args.env = withClaudeProfileTerminalEnv(args.env, args.connectionId, ctx.codexSelectionTarget) + args.env = withClaudeProfileTerminalEnv(args.env, ctx.codexSelectionTarget) ctx.claudeAuth = ctx.isClaudeLaunch && ctx.deps.prepareClaudeAuth ? await ctx.deps.prepareClaudeAuth(ctx.codexSelectionTarget) @@ -169,8 +154,7 @@ export async function prepareRuntimePtySpawn( ...(args.placement ? { placement: args.placement } : {}) } } - const sshScopedEnv = stripRemotePaneEnvWhenHooksDisabled(args.connectionId, args.env) - ctx.env = ctx.claudeAuth ? { ...sshScopedEnv, ...ctx.claudeAuth.envPatch } : sshScopedEnv + ctx.env = ctx.claudeAuth ? { ...args.env, ...ctx.claudeAuth.envPatch } : args.env ctx.requestedAgentTeamsPath = ctx.env?.ORCA_AGENT_TEAMS_TEAM_ID ? ctx.env[resolvePathEnvKey(ctx.env, process.platform)] : undefined @@ -183,20 +167,19 @@ export async function prepareRuntimePtySpawn( (await ctx.deps.getSelectedCodexHomePath?.(ctx.codexSelectionTarget, ctx.env, { launchesCodex })) ?? null - ctx.selectedCodexHomePath = - !ctx.preAdoptedStablePane && !args.connectionId - ? getCompatibleSelectedCodexHomePath( - ctx.codexSelectionTarget, - codexResumeHome - ? await ctx.deps.reconcileSharedRuntimeResumeHome(codexResumeHome, async () => - getCompatibleSelectedCodexHomePath( - ctx.codexSelectionTarget, - await selectLaunchCodexHome() - ) + ctx.selectedCodexHomePath = !ctx.preAdoptedStablePane + ? getCompatibleSelectedCodexHomePath( + ctx.codexSelectionTarget, + codexResumeHome + ? await ctx.deps.reconcileSharedRuntimeResumeHome(codexResumeHome, async () => + getCompatibleSelectedCodexHomePath( + ctx.codexSelectionTarget, + await selectLaunchCodexHome() ) - : await selectLaunchCodexHome() - ) - : null + ) + : await selectLaunchCodexHome() + ) + : null if ( !ctx.preAdoptedStablePane && args.launchAgent === 'codex' && diff --git a/src/main/ipc/pty/runtime/spawn-state.ts b/src/main/ipc/pty/runtime/spawn-state.ts index 888a51658f5..d4f8472fa09 100644 --- a/src/main/ipc/pty/runtime/spawn-state.ts +++ b/src/main/ipc/pty/runtime/spawn-state.ts @@ -34,7 +34,6 @@ export type RuntimePtySpawnState = { callerRequestedSessionId: string | undefined requestedSessionId: string | undefined sessionId: string | undefined - effectiveSessionRelayId: string | undefined effectiveSessionAppId: string | undefined isNewDaemonSession: boolean expectedWslDistro: string | null @@ -157,7 +156,6 @@ export function createRuntimePtySpawnState( callerRequestedSessionId: undefined, requestedSessionId: undefined, sessionId: undefined, - effectiveSessionRelayId: undefined, effectiveSessionAppId: undefined, isNewDaemonSession: false, expectedWslDistro: null, diff --git a/src/main/ipc/pty/runtime/undelivered-ssh-kill.ts b/src/main/ipc/pty/runtime/undelivered-ssh-kill.ts deleted file mode 100644 index 9edce646456..00000000000 --- a/src/main/ipc/pty/runtime/undelivered-ssh-kill.ts +++ /dev/null @@ -1,68 +0,0 @@ -import type { Store } from '../../../persistence' -import { ptyIncarnationById } from '../provider/ownership-state' -import { getPtySshConnectionId, getRelayPtyId } from '../provider/registry' -import { isEpochScopedRelayPtyId } from '../../../../shared/ssh-pending-pty-kill' - -export type UndeliveredSshPtyKill = { - store: Store | undefined - ptyId: string - connectionId: string | null | undefined - /** Whether this caller may still undo the stop. Every caller states it rather than having it - * inferred, because both directions are bugs: `true` on a real close re-opens the leak, and - * `false` on a reversible one lets a later handshake kill a pane the user went back to using. - * Worktree sleep is the reversible case, on both the runtime and the renderer route. */ - reversible: boolean - /** Pass what `finishPtyShutdown` returned: it clears the live map, so a caller that already ran - * it can no longer look the incarnation up. */ - incarnationId?: string - now?: number -} - -/** Durably records a stop this client asked for on an SSH host and could not confirm, so the next - * handshake to that host can replay it. Without this the remote PTY — a child of the detached - * relay daemon, not of the ssh channel — outlives the failed RPC forever. - * - * The silent no-ops are the safety rules, not defensive padding: - * - **reversible**: see above. - * - **no `connectionId`**: a local PTY's owner is this process, so a failed kill has no later host - * to ask; there is nothing to replay against. - * - **no incarnation on a legacy id**: a legacy relay renumbers from `pty-1` on every start, so - * without the host-minted incarnation the order could never be safely aimed. A `pty2:` id is - * epoch-scoped and names one process, so it is recorded without one. - * - **an id naming another connection**: `getRelayPtyId` throws on those, and this runs inside - * promise `.catch` handlers where that would surface as an unhandled rejection. */ -export function recordUndeliveredSshPtyKill(args: UndeliveredSshPtyKill): boolean { - const { store, ptyId, connectionId } = args - if (!store || !connectionId || args.reversible) { - return false - } - let relayPtyId: string - try { - relayPtyId = getRelayPtyId(connectionId, ptyId) - } catch { - return false - } - const incarnationId = args.incarnationId ?? ptyIncarnationById.get(ptyId) - if (!incarnationId && !isEpochScopedRelayPtyId(relayPtyId)) { - return false - } - store.recordSshRemotePtyKillIntent(connectionId, relayPtyId, { - requestedAt: args.now ?? Date.now(), - ...(incarnationId ? { incarnationId } : {}), - attempts: 0 - }) - return true -} - -/** Records the replay order for an explicit close whose stop went unconfirmed, before its follow-up - * kill is sent, so the close receipt can promise the retry only when an order really exists. */ -export function recordUnconfirmedExplicitSshStop(args: { - store: Store | undefined - ptyId: string - reversible: boolean -}): boolean { - return recordUndeliveredSshPtyKill({ - ...args, - connectionId: getPtySshConnectionId(args.ptyId) - }) -} diff --git a/src/main/persistence/loading-store/pty-reattach-failure-routing.test.ts b/src/main/persistence/loading-store/pty-reattach-failure-routing.test.ts index 75eeef8061d..244f0a6336d 100644 --- a/src/main/persistence/loading-store/pty-reattach-failure-routing.test.ts +++ b/src/main/persistence/loading-store/pty-reattach-failure-routing.test.ts @@ -8,7 +8,6 @@ import { createRuntimePtySpawnState } from '../../ipc/pty/runtime/spawn-state' import { createPtySpawnCommitDependencies } from './pty-spawn-commit-dependencies-fixture' import { clearProviderPtyState } from '../../ipc/pty/provider/state-cleanup' import { ptyIncarnationById, ptyOwnership } from '../../ipc/pty/provider/ownership-state' -import { toSshExecutionHostId } from '../../../shared/execution-host' vi.mock('../../telemetry/client', () => ({ track: vi.fn() })) vi.mock('../../telemetry/cohort-classifier', () => ({ @@ -19,12 +18,12 @@ vi.mock('../../ssh/ssh-config-parser', () => ({ sshConfigHostsToTargets: () => [] })) -const connectionId = 'reattach-host' +const connectionId = null const binding = { worktreeId: 'repo-local::/fixture/local', tabId: 'aaaaaaaa-aaaa-4aaa-8aaa-aaaaaaaaaaaa', leafId: 'bbbbbbbb-bbbb-4bbb-8bbb-bbbbbbbbbbbb', - ptyId: 'ssh:reattach-host@@surviving-pty' + ptyId: 'surviving-pty' } const incarnation = 'cccccccc-cccc-4ccc-8ccc-cccccccccccc' const successorIncarnation = 'dddddddd-dddd-4ddd-8ddd-dddddddddddd' @@ -86,20 +85,14 @@ describe.each(['ipc', 'runtime'])('%s failed reattach routing', (controller) => await runtime.onPtyExit(binding.ptyId, 0, incarnationId, { providerExitObserved: true }) if (outcome === 'replaced') { runtime.onPtySpawned(binding.ptyId, successorIncarnation) - ptyOwnership.set(binding.ptyId, 'ssh:successor-host') + ptyOwnership.set(binding.ptyId, 'local') ptyIncarnationById.set(binding.ptyId, successorIncarnation) } } gate.finish.reject(new Error('disk full')) await pending - expect(ownerWhileSaving).toBe(`ssh:${connectionId}`) - expect(ptyOwnership.get(binding.ptyId)).toBe( - outcome === 'live' - ? `ssh:${connectionId}` - : outcome === 'replaced' - ? 'ssh:successor-host' - : undefined - ) + expect(ownerWhileSaving).toBe('local') + expect(ptyOwnership.get(binding.ptyId)).toBe(outcome === 'exited' ? undefined : 'local') expect(ptyIncarnationById.get(binding.ptyId)).toBe( outcome === 'live' ? incarnationId @@ -109,8 +102,7 @@ describe.each(['ipc', 'runtime'])('%s failed reattach routing', (controller) => ) expect(shutdown).not.toHaveBeenCalled() expect(publish).not.toHaveBeenCalled() - expect(store.getSshRemotePtyLeases(connectionId)).toEqual([]) - const session = readState().workspaceSessionsByHostId[toSshExecutionHostId(connectionId)] + const session = readState().workspaceSession expect( session?.terminalLayoutsByTabId[binding.tabId]?.ptyIdsByLeafId?.[binding.leafId] ).toBeUndefined() diff --git a/src/main/persistence/loading-store/pty-spawn-exit-durability.test.ts b/src/main/persistence/loading-store/pty-spawn-exit-durability.test.ts index b39a5eb01c9..427152b3d1e 100644 --- a/src/main/persistence/loading-store/pty-spawn-exit-durability.test.ts +++ b/src/main/persistence/loading-store/pty-spawn-exit-durability.test.ts @@ -8,7 +8,6 @@ import { commitPtyIpcSpawn } from '../../ipc/pty/ipc/spawn-commit' import { createPtyIpcSpawnState } from '../../ipc/pty/ipc/spawn-state' import type { PtySpawnIpcDeps } from '../../ipc/pty/ipc/spawn-types' import { registerPersistedPtySpawn } from '../../ipc/pty/pane/spawn-registration' -import { toSshExecutionHostId } from '../../../shared/execution-host' import { clearProviderPtyState } from '../../ipc/pty/provider/state-cleanup' import { ptyOwnership, ptyIncarnationById } from '../../ipc/pty/provider/ownership-state' import { ptySizes } from '../../ipc/pty/delivery/visibility-state' @@ -31,26 +30,23 @@ vi.mock('../../ssh/ssh-config-parser', () => ({ it.each( [ - { controller: 'runtime', connectionId: null, exitCode: 0 }, - { controller: 'runtime', connectionId: 'test-host', exitCode: 0 }, - { controller: 'ipc', connectionId: null, exitCode: -1 }, - { controller: 'ipc', connectionId: 'test-host', exitCode: 0 } + { controller: 'runtime', exitCode: 0 }, + { controller: 'ipc', exitCode: -1 } ].flatMap((test) => [ { ...test, stableOwner: false }, { ...test, stableOwner: true } ]) )( - 'retires an exited $controller binding on $connectionId after disk finishes (stable: $stableOwner)', - async ({ controller, connectionId, exitCode, stableOwner }) => { + 'retires an exited $controller binding after disk finishes (stable: $stableOwner)', + async ({ controller, exitCode, stableOwner }) => { + const connectionId = null const { store, authority, readState } = await fixture() const runtime = new OrcaRuntimeService(store) const binding = { worktreeId: 'repo-local::/fixture/local', tabId: 'aaaaaaaa-aaaa-4aaa-8aaa-aaaaaaaaaaaa', leafId: 'bbbbbbbb-bbbb-4bbb-8bbb-bbbbbbbbbbbb', - ptyId: connectionId - ? `ssh:${connectionId}@@pty-exited-during-durable-bind` - : 'pty-exited-during-durable-bind', + ptyId: 'pty-exited-during-durable-bind', incarnationId: 'cccccccc-cccc-4ccc-8ccc-cccccccccccc' } const owner = stableOwner @@ -61,10 +57,7 @@ it.each( } : null if (owner) { - await store.persistPtyBinding( - { ...binding, incarnationId: owner.persistedIncarnationId }, - connectionId ? toSshExecutionHostId(connectionId) : undefined - ) + await store.persistPtyBinding({ ...binding, incarnationId: owner.persistedIncarnationId }) } runtime.onPtySpawned(binding.ptyId, binding.incarnationId) runtime.beginPtyRegistration(binding.ptyId, binding.incarnationId) @@ -115,11 +108,6 @@ it.each( expect(ptyOwnership.has(binding.ptyId)).toBe(false) expect(ptyIncarnationById.has(binding.ptyId)).toBe(false) expect(ptySizes.has(binding.ptyId)).toBe(false) - expect( - store - .getSshRemotePtyLeases(connectionId ?? undefined) - .some((lease) => lease.ptyId.includes('pty-exited-during-durable-bind')) - ).toBe(false) if (reservation) { await expect(reservation.promise).resolves.toMatchObject({ id: binding.ptyId }) const next = await nextReservation @@ -127,10 +115,7 @@ it.each( resolvePaneSpawnReservation(reservationKey, next, { id: 'next-spawn' }) expect(paneSpawnReservationsByOwnerKey.has(reservationKey)).toBe(false) } - const state = readState() - const session = connectionId - ? state.workspaceSessionsByHostId[toSshExecutionHostId(connectionId)] - : state.workspaceSession + const session = readState().workspaceSession expect( session.terminalLayoutsByTabId[binding.tabId]?.ptyIdsByLeafId?.[binding.leafId] ).toBeUndefined() diff --git a/src/main/providers/ssh-pty-errors.ts b/src/main/providers/ssh-pty-errors.ts deleted file mode 100644 index dc4532ed75e..00000000000 --- a/src/main/providers/ssh-pty-errors.ts +++ /dev/null @@ -1,61 +0,0 @@ -import { errorMessage } from '../../shared/error-message' - -export const SSH_SESSION_EXPIRED_ERROR = 'SSH_SESSION_EXPIRED' -export const SSH_PTY_IDENTITY_MISMATCH_ERROR = 'SSH_PTY_IDENTITY_MISMATCH' - -export function isSshPtyNotFoundError(error: unknown): boolean { - const message = errorMessage(error) - return /PTY ".+" not found/i.test(message) -} - -export function isSshPtyIdentityMismatchError(error: unknown): boolean { - const message = errorMessage(error) - return message.includes(SSH_PTY_IDENTITY_MISMATCH_ERROR) || /identity mismatch/i.test(message) -} - -/** - * A reachable relay answered for this exact PTY id and reported it absent, so the client may retire - * its own route to it. Deliberately NOT raised for a transport failure, a request timeout, a - * disposed multiplexer, an identity mismatch (the id names a live PTY belonging to another pane), - * or `restoreRequired` (the PTY is live, only its source stream is not) — none of those observe the - * process, and treating them as absence orphans live remote work. - * - * This is NOT itself a death certificate. `pty.attach` answers absent for an id its session map - * never had as readily as for a pid it probed — and after a relay restart that is every id the - * previous one minted. Certifying `exited` needs {@link SshPtyProvenExitedOnRelayError} - * (docs/reference/ssh-execution-boundary.md). - * - * Carries the same `SSH_SESSION_EXPIRED` message so message-based consumers are unaffected; only - * callers that can act on the stronger verdict test the class. - */ -export class SshPtyAbsentFromRelayError extends Error { - constructor(message: string) { - super(message) - this.name = 'SshPtyAbsentFromRelayError' - } -} - -export function isSshPtyAbsentFromRelayError(error: unknown): boolean { - return error instanceof SshPtyAbsentFromRelayError -} - -/** - * The narrow half of {@link SshPtyAbsentFromRelayError}: the relay probed the pid and found it gone - * before answering absent, so this is the one attach refusal that observed the process and the only - * one that may certify a death. - * - * The parent class is raised for the whole union, which also contains "this session map has no such - * id" — every id minted before a relay restart, checked against nothing. Callers that only release - * client-side bookkeeping keep testing the parent; a caller about to record `exited` must test this - * (docs/reference/ssh-execution-boundary.md). - */ -export class SshPtyProvenExitedOnRelayError extends SshPtyAbsentFromRelayError { - constructor(message: string) { - super(message) - this.name = 'SshPtyProvenExitedOnRelayError' - } -} - -export function isSshPtyProvenExitedOnRelayError(error: unknown): boolean { - return error instanceof SshPtyProvenExitedOnRelayError -} diff --git a/src/main/runtime/headless-terminal-dispose-write-ordering.test.ts b/src/main/runtime/headless-terminal-dispose-write-ordering.test.ts index 13efbb1e80a..c0c271bd792 100644 --- a/src/main/runtime/headless-terminal-dispose-write-ordering.test.ts +++ b/src/main/runtime/headless-terminal-dispose-write-ordering.test.ts @@ -81,7 +81,7 @@ describe('disposeHeadlessTerminal write ordering', () => { const runtime = createRuntime() // Queued but not yet parsed: the chain link is still pending here. runtime.onPtyData(PTY_ID, 'QUEUED-BEFORE-DISPOSE\r\n', 1) - runtime.resetPtyModelAfterMigrationFailure(PTY_ID) + runtime.preparePtyExecutionContext(PTY_ID, null, { resetIncarnation: true }) runtime.onPtyData(PTY_ID, 'ISSUED-AFTER-DISPOSE\r\n', 2) await runtime.serializeHiddenOutputRecoveryBuffer(PTY_ID) @@ -95,7 +95,7 @@ describe('disposeHeadlessTerminal write ordering', () => { it('leaves the post-disposal emulator holding only post-disposal output', async () => { const runtime = createRuntime() runtime.onPtyData(PTY_ID, 'QUEUED-BEFORE-DISPOSE\r\n', 1) - runtime.resetPtyModelAfterMigrationFailure(PTY_ID) + runtime.preparePtyExecutionContext(PTY_ID, null, { resetIncarnation: true }) runtime.onPtyData(PTY_ID, 'ISSUED-AFTER-DISPOSE\r\n', 2) const snapshot = await runtime.serializeHiddenOutputRecoveryBuffer(PTY_ID) diff --git a/src/main/runtime/orca-runtime-maybe-hydrate-headless-from-renderer.ts b/src/main/runtime/orca-runtime-maybe-hydrate-headless-from-renderer.ts index 582d6a92d2a..1cab3801ffe 100644 --- a/src/main/runtime/orca-runtime-maybe-hydrate-headless-from-renderer.ts +++ b/src/main/runtime/orca-runtime-maybe-hydrate-headless-from-renderer.ts @@ -171,7 +171,7 @@ export class OrcaRuntimeWithMaybeHydrateHeadlessFromRenderer extends OrcaRuntime data: string, outputSequence: number, forwardQueryReplies = false - ): Promise { + ): void { const state = this.getOrCreateHeadlessTerminal(ptyId) const completion = state.writeChain.then(async () => { // Why: the ingestion-time ownership decision is closed over this @@ -205,8 +205,6 @@ export class OrcaRuntimeWithMaybeHydrateHeadlessFromRenderer extends OrcaRuntime } state.outputSequence = outputSequence }) - // Legacy callers remain best-effort; bounded SSH admission observes the raw receipt. state.writeChain = completion.catch(() => {}) - return completion } } diff --git a/src/main/runtime/orca-runtime-on-pty-data.ts b/src/main/runtime/orca-runtime-on-pty-data.ts index e665d55ce10..01bf48c8bb8 100644 --- a/src/main/runtime/orca-runtime-on-pty-data.ts +++ b/src/main/runtime/orca-runtime-on-pty-data.ts @@ -31,7 +31,6 @@ export class OrcaRuntimeWithOnPtyData extends OrcaRuntimeWithPreparePtyExecution at: number, sequenceChars = data.length, transformed = false, - captureModelReceipt?: (completion: Promise) => void, sourceRanges?: readonly TerminalOutputSourceRange[] ): number { const outputSequence = (this.ptyOutputSequenceById.get(ptyId) ?? 0) + sequenceChars @@ -65,13 +64,7 @@ export class OrcaRuntimeWithOnPtyData extends OrcaRuntimeWithPreparePtyExecution // applyTrackedPtyTitle) in byte order, superseding main's inline // extractLastOscTitleForPty block (#7880/#7852 title/status semantics are // preserved via the tracker + detectAgentStatusFromTitle path). - const modelCompletion = this.trackHeadlessTerminalData( - ptyId, - data, - outputSequence, - forwardQueryReplies - ) - captureModelReceipt?.(modelCompletion) + this.trackHeadlessTerminalData(ptyId, data, outputSequence, forwardQueryReplies) const pty = this.getOrCreatePtyWorktreeRecord(ptyId) const ptyTailBefore = pty diff --git a/src/main/runtime/orca-runtime-prepare-pty-execution-context.ts b/src/main/runtime/orca-runtime-prepare-pty-execution-context.ts index 3dfcc2cc084..b30742cf1b0 100644 --- a/src/main/runtime/orca-runtime-prepare-pty-execution-context.ts +++ b/src/main/runtime/orca-runtime-prepare-pty-execution-context.ts @@ -1,7 +1,5 @@ // @ts-nocheck -- mechanically split from OrcaRuntimeService; behavior is covered by AST equivalence and characterization tests. import { OrcaRuntimeWithRegisterPty } from './orca-runtime-register-pty' -import type { TerminalOutputSourceRange } from '../../shared/terminal-output-source-range' -import type { RuntimePtyDataAdmission } from './runtime-terminal-contracts' export class OrcaRuntimeWithPreparePtyExecutionContext extends OrcaRuntimeWithRegisterPty { preparePtyExecutionContext( @@ -54,39 +52,4 @@ export class OrcaRuntimeWithPreparePtyExecutionContext extends OrcaRuntimeWithRe this.terminalSpawnCommandsByPtyId.set(ptyId, trimmed) } } - - resetPtyModelAfterMigrationFailure(ptyId: string): void { - this.providerSnapshotPreferredPtys.add(ptyId) - this.disposeHeadlessTerminal(ptyId) - } - - /** - * Handles incoming data from a PTY process, running agent detection, - * updating terminal tail buffers, and triggering foreground agent refreshes. - */ - acceptPtyDataBounded( - ptyId: string, - data: string, - at: number, - sequenceChars = data.length, - transformed = false, - sourceRanges?: readonly TerminalOutputSourceRange[] - ): RuntimePtyDataAdmission { - let completion: Promise | null = null - const sequence = this.onPtyData( - ptyId, - data, - at, - sequenceChars, - transformed, - (receipt) => { - completion = receipt - }, - sourceRanges - ) - if (!completion) { - throw new Error('PTY model admission receipt was not captured') - } - return Object.freeze({ sequence, completion }) - } } diff --git a/src/main/runtime/orca-runtime-stop-explicitly-closed-tab-ptys.ts b/src/main/runtime/orca-runtime-stop-explicitly-closed-tab-ptys.ts index e87a6fd996d..9cdc7b96441 100644 --- a/src/main/runtime/orca-runtime-stop-explicitly-closed-tab-ptys.ts +++ b/src/main/runtime/orca-runtime-stop-explicitly-closed-tab-ptys.ts @@ -8,9 +8,9 @@ import { parsePaneKey } from '../../shared/stable-pane-id' import { errorMessage } from '../../shared/error-message' /** How an explicit close's stop of its addressed PTY ended. */ -type ExplicitCloseStop = { stopped: boolean; pendingKillRecorded: boolean } +type ExplicitCloseStop = { stopped: boolean } -const NO_STOP: ExplicitCloseStop = { stopped: false, pendingKillRecorded: false } +const NO_STOP: ExplicitCloseStop = { stopped: false } export class OrcaRuntimeWithStopExplicitlyClosedTabPtys extends OrcaRuntimeWithFocusTerminal { protected async stopExplicitlyClosedTabPtys( @@ -23,7 +23,6 @@ export class OrcaRuntimeWithStopExplicitlyClosedTabPtys extends OrcaRuntimeWithF this.markPtyStopRequested(ptyId) const expectedIncarnationId = this.ptysById.get(ptyId)?.incarnationId let stopped = false - let pendingKillRecorded = false if (this.ptyController?.stopAndWait) { try { stopped = await this.ptyController.stopAndWait(ptyId, { deadlineMs }) @@ -45,8 +44,6 @@ export class OrcaRuntimeWithStopExplicitlyClosedTabPtys extends OrcaRuntimeWithF verdict?.status === 'unverifiable' && verdict.reason === SSH_PROVIDER_UNREGISTERED_REASON if (!providerAlreadyRetiredPty) { - // Why before the kill: its own failure is recorded only once its RPC settles. - pendingKillRecorded = this.ptyController.recordUnconfirmedStop?.(ptyId) === true this.ptyController.kill(ptyId) if (!verdict || verdict.status === 'live') { this.markPtyLivenessUnverifiable( @@ -60,7 +57,7 @@ export class OrcaRuntimeWithStopExplicitlyClosedTabPtys extends OrcaRuntimeWithF stopped = this.ptyController?.kill(ptyId) ?? false } if (ptyId === addressedPtyId) { - addressedPtyStop = { stopped, pendingKillRecorded } + addressedPtyStop = { stopped } } } return addressedPtyStop @@ -72,12 +69,7 @@ export class OrcaRuntimeWithStopExplicitlyClosedTabPtys extends OrcaRuntimeWithF ptyId: string | null, stop: ExplicitCloseStop ): RuntimeTerminalClose { - const close: RuntimeTerminalClose = { - handle, - tabId, - ptyKilled: stop.stopped, - ...(stop.pendingKillRecorded ? { pendingKillRecorded: true as const } : {}) - } + const close: RuntimeTerminalClose = { handle, tabId, ptyKilled: stop.stopped } if (stop.stopped || !ptyId) { return close } diff --git a/src/main/runtime/orca-runtime-terminal-close-continuity.test.ts b/src/main/runtime/orca-runtime-terminal-close-continuity.test.ts index 5e100dd4dbd..a9a4d6a4fd7 100644 --- a/src/main/runtime/orca-runtime-terminal-close-continuity.test.ts +++ b/src/main/runtime/orca-runtime-terminal-close-continuity.test.ts @@ -368,36 +368,6 @@ describe('terminal close and handle incarnation continuity', () => { expect(harness.kill).toHaveBeenCalledWith(PTY_ID) }) - it.each([true, false])( - 'promises a retry only when the stop recorded a replayable kill (recorded: %s)', - async (recorded) => { - const harness = createHarness() - const [{ handle }] = (await harness.runtime.listTerminals(`id:${WORKTREE_ID}`)).terminals - harness.setVerifiedStopResult(false) - const recordUnconfirmedStop = vi.fn(() => { - // The order must exist before the follow-up kill, whose own failure lands only later. - expect(harness.kill).not.toHaveBeenCalled() - return recorded - }) - const controller = harness.runtime['ptyController'] - if (!controller) { - throw new Error('fixture has no PTY controller') - } - Object.assign(controller, { recordUnconfirmedStop }) - - const closing = harness.runtime.closeTerminal(handle) - await vi.waitFor(() => expect(harness.closeTerminalTab).toHaveBeenCalled()) - harness.retirePersistedTab() - harness.acknowledged.resolve() - - const close = await closing - expect(recordUnconfirmedStop).toHaveBeenCalledWith(PTY_ID) - expect(harness.kill).toHaveBeenCalledWith(PTY_ID) - expect(close.ptyStopVerdict).toBe('unverifiable') - expect(close.pendingKillRecorded).toBe(recorded ? true : undefined) - } - ) - it('leaves a confirmed kill receipt free of any stop verdict', async () => { const harness = createHarness() const [{ handle }] = (await harness.runtime.listTerminals(`id:${WORKTREE_ID}`)).terminals diff --git a/src/main/runtime/orca-runtime.ts b/src/main/runtime/orca-runtime.ts index feb0d7e0629..7f3cbe1744b 100644 --- a/src/main/runtime/orca-runtime.ts +++ b/src/main/runtime/orca-runtime.ts @@ -174,7 +174,6 @@ export type { SubscriptionRegistration } from './runtime-subscription-registry' export type { OrchestrationCompatibilityCallerAuthority, OrchestrationCompatibilityTerminalAuthority, - RuntimePtyDataAdmission, RuntimeTerminalAgentStatusEvent } from './runtime-terminal-contracts' export type { MessageWaitResult } from './runtime-message-waiters' diff --git a/src/main/runtime/orchestration-mailbox-notification-test-harness.ts b/src/main/runtime/orchestration-mailbox-notification-test-harness.ts index 2622416d74f..1199d9bc95e 100644 --- a/src/main/runtime/orchestration-mailbox-notification-test-harness.ts +++ b/src/main/runtime/orchestration-mailbox-notification-test-harness.ts @@ -203,9 +203,10 @@ export function registerSecondPane( export async function driveToLiveIdle(runtime: OrcaRuntimeService): Promise { await runtime.listTerminals() - const working = runtime.acceptPtyDataBounded(PTY_ID, '\x1b]0;Codex working\x07', 1) - const done = runtime.acceptPtyDataBounded(PTY_ID, '\x1b]0;Codex done\x07', 2) - await Promise.all([working.completion, done.completion]) + runtime.onPtyData(PTY_ID, '\x1b]0;Codex working\x07', 1) + runtime.onPtyData(PTY_ID, '\x1b]0;Codex done\x07', 2) + // Why: the snapshot awaits the headless model's write chain, so both chunks are parsed. + await runtime.serializeMainTerminalBuffer(PTY_ID) } export function pointerCount(write: ReturnType): number { diff --git a/src/main/runtime/orchestration-message-delivery-identity.test.ts b/src/main/runtime/orchestration-message-delivery-identity.test.ts index e7d7bd4ce5d..3be6161897e 100644 --- a/src/main/runtime/orchestration-message-delivery-identity.test.ts +++ b/src/main/runtime/orchestration-message-delivery-identity.test.ts @@ -112,9 +112,10 @@ function createRuntime( async function driveToLiveIdle(runtime: OrcaRuntimeService): Promise { await runtime.listTerminals() - const working = runtime.acceptPtyDataBounded(PTY_ID, '\x1b]0;Codex working\x07', 1) - const done = runtime.acceptPtyDataBounded(PTY_ID, '\x1b]0;Codex done\x07', 2) - await Promise.all([working.completion, done.completion]) + runtime.onPtyData(PTY_ID, '\x1b]0;Codex working\x07', 1) + runtime.onPtyData(PTY_ID, '\x1b]0;Codex done\x07', 2) + // Why: the snapshot awaits the headless model's write chain, so both chunks are parsed. + await runtime.serializeMainTerminalBuffer(PTY_ID) } async function check( diff --git a/src/main/runtime/runtime-pty-controller-contract.ts b/src/main/runtime/runtime-pty-controller-contract.ts index 4be82197283..157439f3b43 100644 --- a/src/main/runtime/runtime-pty-controller-contract.ts +++ b/src/main/runtime/runtime-pty-controller-contract.ts @@ -128,9 +128,6 @@ export type RuntimePtyController = { ptyId: string, opts?: { keepHistory?: boolean; deadlineMs?: number } ): Promise - /** Durably records a kill order for an explicit close's unconfirmed stop, replayed when its SSH - * host reconnects. True only when an order was written; local PTYs have no later host to ask. */ - recordUnconfirmedStop?(ptyId: string): boolean getCwd?(ptyId: string): Promise getForegroundProcess(ptyId: string): Promise inspectProcess?( diff --git a/src/main/runtime/runtime-terminal-contracts.ts b/src/main/runtime/runtime-terminal-contracts.ts index ca70eedd030..8f7212f07e5 100644 --- a/src/main/runtime/runtime-terminal-contracts.ts +++ b/src/main/runtime/runtime-terminal-contracts.ts @@ -159,11 +159,6 @@ export type HookLiveAgentRow = Pick< 'payload' | 'updatedAt' | 'evidenceObservedAt' | 'stateStartedAt' | 'worktreeId' > -export type RuntimePtyDataAdmission = Readonly<{ - sequence: number - completion: Promise -}> - export type TerminalHandleRecord = { handle: string runtimeId: string