mirror of
https://github.com/stablyai/orca.git
synced 2026-10-19 00:03:20 +00:00
fix: fence terminal teardown ownership
This commit is contained in:
@@ -3474,12 +3474,12 @@
|
||||
"providers": ["local", "daemon", "ssh", "remote-runtime"],
|
||||
"coveredPlatforms": ["macos"],
|
||||
"coveredProviders": ["local", "ssh"],
|
||||
"coverageNotes": "A macOS unit harness loads the installed patched node-pty WindowsTerminal prototype without constructing native state, proving pre-ready kill/destroy dispatch immediately. A relay artifact harness reproduces vanilla npm node-pty retaining one deferred kill with zero native close calls, then proves both Windows relay artifacts hash, ship, apply, include the local ConPTY console-list fallback, and idempotently verify the fix. Relay tests simulate win32 shutdown, repeated stale-spawn native failure, one-shot ownership, and retry after native failure without deleting the only handle before close is proven. Relay boot UUIDs scope recycled pty-N counters through initial spawn, durable-lease persistence, reattach, and SSH target-id migration; mismatched generations expire before raw attach and release their exact in-memory retry owner, while pane identity protects legacy leases. Main-process retry owners persist local daemon and remote-runtime teardown intent across renderer and app restarts, coalesce by exact provider identity, use one shared one-shot timer per owner, and fence SSH relay plus remote-runtime generation reuse before close. Local Windows shutdown retains provider records and listeners after native throw unless onExit independently proves death. A disconnected SSH provider rejects shutdown without tombstoning its attachable lease. Paired/mobile headless close refuses to depersist a terminal until the controller accepts durable retry ownership. Project removal keeps local or remote ownership until provider-verified shutdown and successful VM cleanup; runtime repo removal drains pre-spawn trust/team/command preparation plus create-first worktree creation, headless, renderer-backed mobile, renderer split, and selector-less renderer terminal admissions before fresh ownership/liveness snapshots, stop, and metadata mutation. Desktop and duplicate-host repo removal route through the same transaction; legacy worktree creation, renderer PTY spawn, and every local/SSH/folder worktree removal outcome—including Windows stale registration and orphan-compatible fallback—are fenced and verified before destructive mutation or metadata purge. Hidden local/SSH and remote-runtime background-launch cleanup retain exact PTY/handle retry ownership with bounded one-shot scheduling. Crash-durable teardown intent uses an O(1) revisioned append journal with exact SSH set/remove records, compacted to one checkpoint after a successful full-state save, instead of synchronously serializing every retained owner for each mutation. Targeted terminal.stop probes skip unrelated sessions. The same relay contract serves paired web/mobile clients; no live Windows ConPTY or Windows-over-SSH run is recorded yet.",
|
||||
"coverageNotes": "A macOS unit harness loads the installed patched node-pty WindowsTerminal prototype without constructing native state, proving pre-ready kill/destroy dispatch immediately. A relay artifact harness reproduces vanilla npm node-pty retaining one deferred kill with zero native close calls, then proves both Windows relay artifacts hash, ship, apply, include the local ConPTY console-list fallback, and idempotently verify the fix. Relay tests simulate win32 shutdown, repeated stale-spawn native failure, one-shot ownership, and retry after native failure without deleting the only handle before close is proven. Relay boot UUIDs scope recycled pty-N counters through initial spawn, durable-lease persistence, reattach, and SSH target-id migration; mismatched generations expire before raw attach and release their exact in-memory retry owner, while pane identity protects legacy leases. Main-process retry owners persist local daemon and remote-runtime teardown intent across renderer and app restarts, coalesce by exact provider identity, use one shared one-shot timer per owner, and fence SSH relay plus remote-runtime generation reuse before close. Local and daemon providers retain pane/tab identity and reject stale teardown before native or daemon kill; daemon protocol v22 carries the identity across process boundaries, while legacy adapters fail closed unless main observed the exact pane. Lease-less SSH retry owners survive transport replacement and re-drive the replacement provider. Local Windows shutdown retains provider records and listeners after native throw unless onExit independently proves death. A disconnected SSH provider rejects shutdown without tombstoning its attachable lease. Paired/mobile headless close refuses to depersist a terminal until the controller accepts durable retry ownership. Project removal keeps local or remote ownership until provider-verified shutdown and successful VM cleanup; runtime repo removal drains pre-spawn trust/team/command preparation plus create-first worktree creation, headless, renderer-backed mobile, renderer split, and selector-less renderer terminal admissions before fresh ownership/liveness snapshots, stop, and metadata mutation. Desktop and duplicate-host repo removal route through the same transaction; unresolved terminal creation may continue for an unrelated host while the removed host remains fenced. Legacy worktree creation, renderer PTY spawn, and every local/SSH/folder worktree removal outcome—including Windows stale registration and orphan-compatible fallback—are fenced and verified before destructive mutation or metadata purge. Hidden local/SSH and remote-runtime background-launch cleanup retain exact PTY/handle retry ownership with bounded one-shot scheduling. Crash-durable teardown intent uses an O(1) revisioned append journal with exact SSH set/remove records, compacted to one checkpoint after a successful full-state save, instead of synchronously serializing every retained owner for each mutation. Targeted terminal.stop probes skip unrelated sessions. The same relay contract serves paired web/mobile clients; no live Windows ConPTY or Windows-over-SSH run is recorded yet.",
|
||||
"motivatingLinks": [
|
||||
"https://github.com/stablyai/orca/issues/8260",
|
||||
"https://github.com/stablyai/orca/pull/8279"
|
||||
],
|
||||
"invariant": "A Windows PTY closed before its first output executes exactly one no-argument node-pty shutdown immediately; Orca never deletes ownership without accepted native shutdown or an observed exit, and a later retry can target only the same relay generation and pane while project removal fences both worktree and terminal creation.",
|
||||
"invariant": "A Windows PTY closed before its first output executes exactly one no-argument node-pty shutdown immediately; Orca never deletes ownership without accepted native shutdown or an observed exit, and a later retry can target only the same provider generation and pane while project removal fences the removed host without blocking unrelated hosts.",
|
||||
"oracle": "Load the installed patched WindowsTerminal prototype in a pre-ready state and assert kill and destroy each close the terminal and native agent once with zero deferred work, while signal arguments still throw. Build both Windows SSH relay artifacts, reproduce vanilla npm node-pty with close=0/native-kill=0/deferred=1, apply the shipped patch, and assert close=1/native-kill=1/deferred=0, AttachConsole failure falls back to the shell pid, and the artifact version derives from all patch bytes. Assert remote npm install executes patch enforcement before its runtime probe and never finalizes a failed patch. In local and relay providers, assert native throws retain exact ownership/listeners and retry succeeds, while a synchronous onExit is accepted as independent death proof; provider absence must retain the SSH lease and reject shutdown. Persist relay generation with every durable lease, expire mismatches before attach while releasing their exact retry owner, and preserve generation through target migration. Reject local daemon and remote-runtime teardown, reload the durable store, wait for the startup provider before replay, replace an in-flight provider, and prove the same caller re-drives the replacement before settling, stronger coalesced shutdown options persist, and capped retries drain exact identities with zero final retry timers; persisted close intent for an already-removed runtime must drain without an unhandled rejection or retry wake loop. A generationless SSH id must not mutate or tombstone a generation-qualified replacement lease. Paired/mobile headless close must preserve persisted UI state when controller ownership is rejected. Create and clear 65 local plus 65 remote-runtime owners, reload immediately at both boundaries, and assert 260 O(1) journal records stay below 100 KiB with a 1 KiB per-record cap; repeat the same byte and record bounds for 50 generation-qualified SSH owners. Synchronous full-store flush count must remain zero, both a partial JSON tail and a complete record missing its newline must preserve all complete records, the next mutation must first rewrite an atomic recovered checkpoint, and the next full-state save must compact the journal to one revision-fenced checkpoint; an older valid sidecar must lose to a newer full-store fallback revision. Replace SSH relay and remote-runtime generations while a retry is retained and prove the new owner is never closed. Start pre-spawn trust/team preparation plus worktree and terminal creation surfaces before whole-project and host-scoped project removal and assert removal performs no ownership/liveness snapshot until they publish; host-scoped liveness must query only its selected provider so an unrelated SSH outage cannot veto removal. Removal-first and failed/concurrent admissions must reject or release without deadlock. Start a real worktree removal, pause its preflight, and assert a late renderer terminal admission is rejected; assert every successful registered, orphaned, leftover, already-missing, and stale-registration removal path verifies shutdown before mutation or metadata purge; assert desktop repo removal, worktree create/remove, and PTY spawn handlers all enter their runtime-owned transactions before mutation or provider work.",
|
||||
"commands": [
|
||||
"pnpm exec vitest run --config config/vitest.config.ts config/scripts/build-relay-windows-node-pty-patch.test.mjs src/main/ssh/ssh-relay-native-deps-install.test.ts src/relay/node-pty-windows-pre-ready-shutdown.test.ts src/relay/pty-handler.test.ts src/renderer/src/components/terminal-pane/pty-transport.test.ts src/renderer/src/store/slices/repos-remove-project-purge-leak.test.ts src/renderer/src/store/slices/repos.test.ts src/renderer/src/store/slices/repos-runtime-vm-removal.test.ts src/renderer/src/lib/ephemeral-vm-runtime-cleanup.test.ts",
|
||||
@@ -3490,7 +3490,8 @@
|
||||
"pnpm exec vitest run --config config/vitest.config.ts src/main/ipc/repos-create.test.ts src/main/window/attach-main-window-services.test.ts src/main/ipc/worktrees.test.ts src/main/ipc/worktrees-windows.test.ts src/main/ipc/pty.test.ts src/main/runtime/orca-runtime.test.ts src/renderer/src/lib/launch-agent-background-session.test.ts src/renderer/src/store/slices/repos-host-identity-routing.test.ts",
|
||||
"pnpm exec vitest run --config config/vitest.config.ts src/shared/ssh-pty-id.test.ts src/shared/ssh-types.test.ts src/main/providers/local-pty-provider.test.ts src/main/providers/ssh-pty-provider.test.ts src/main/persistence.test.ts src/main/ssh/ssh-relay-session.test.ts src/main/ssh/ssh-relay-generation-reattach.test.ts src/main/ssh/ssh-target-id-migration.test.ts src/relay/pty-handler.test.ts src/relay/subprocess.test.ts src/renderer/src/components/terminal-pane/pty-transport.test.ts src/renderer/src/components/terminal-pane/pty-pre-handler-buffer.test.ts src/renderer/src/lib/pty-kill-retry-ownership.test.ts src/renderer/src/lib/runtime-terminal-close-retry-ownership.test.ts src/renderer/src/lib/ephemeral-vm-runtime-cleanup.test.ts src/renderer/src/lib/launch-agent-background-session.test.ts src/renderer/src/store/slices/repos.test.ts src/renderer/src/store/slices/repos-host-identity-routing.test.ts src/main/runtime/orca-runtime.test.ts src/main/ipc/pty.test.ts",
|
||||
"pnpm exec vitest run --config config/vitest.config.ts src/main/ipc/repos-create.test.ts src/main/window/attach-main-window-services.test.ts src/main/ipc/worktrees.test.ts src/main/ipc/worktrees-windows.test.ts src/main/ipc/pty.test.ts src/main/runtime/orca-runtime.test.ts src/renderer/src/lib/launch-agent-background-session.test.ts src/renderer/src/lib/runtime-terminal-close-retry-ownership.test.ts src/renderer/src/store/slices/repos-host-identity-routing.test.ts",
|
||||
"pnpm exec vitest run --config config/vitest.config.ts src/main/runtime/orca-runtime.test.ts src/main/ipc/worktrees.test.ts src/main/ipc/pty.test.ts src/main/ipc/runtime-environments.test.ts src/main/runtime/rpc/terminal-close-generation.test.ts src/main/persistence.test.ts src/main/ssh/ssh-relay-session.test.ts src/main/ipc/ssh.test.ts src/renderer/src/store/slices/worktrees.test.ts"
|
||||
"pnpm exec vitest run --config config/vitest.config.ts src/main/runtime/orca-runtime.test.ts src/main/ipc/worktrees.test.ts src/main/ipc/pty.test.ts src/main/ipc/runtime-environments.test.ts src/main/runtime/rpc/terminal-close-generation.test.ts src/main/persistence.test.ts src/main/ssh/ssh-relay-session.test.ts src/main/ipc/ssh.test.ts src/renderer/src/store/slices/worktrees.test.ts",
|
||||
"pnpm exec vitest run --config config/vitest.config.ts src/main/providers/local-pty-provider.test.ts src/main/daemon/daemon-pty-adapter.test.ts src/main/daemon/terminal-host.test.ts src/main/ipc/pty.test.ts src/main/ipc/runtime-terminal-close-retry-owner.test.ts src/main/runtime/orca-runtime.test.ts"
|
||||
],
|
||||
"testFiles": [
|
||||
"config/scripts/build-relay-windows-node-pty-patch.test.mjs",
|
||||
@@ -3506,6 +3507,8 @@
|
||||
"src/shared/ssh-pty-id.test.ts",
|
||||
"src/shared/ssh-types.test.ts",
|
||||
"src/main/providers/local-pty-provider.test.ts",
|
||||
"src/main/daemon/daemon-pty-adapter.test.ts",
|
||||
"src/main/daemon/terminal-host.test.ts",
|
||||
"src/main/providers/ssh-pty-provider.test.ts",
|
||||
"src/main/persistence.test.ts",
|
||||
"src/main/ssh/ssh-relay-session.test.ts",
|
||||
@@ -3515,6 +3518,7 @@
|
||||
"src/renderer/src/components/terminal-pane/pty-pre-handler-buffer.test.ts",
|
||||
"src/renderer/src/lib/pty-kill-retry-ownership.test.ts",
|
||||
"src/main/ipc/pty.test.ts",
|
||||
"src/main/ipc/runtime-terminal-close-retry-owner.test.ts",
|
||||
"src/main/ipc/repos-create.test.ts",
|
||||
"src/main/ipc/worktrees.test.ts",
|
||||
"src/main/ipc/worktrees-windows.test.ts",
|
||||
@@ -3600,6 +3604,7 @@
|
||||
"host-scoped project removal drains create-first work and rejects removal-first creation before mutating one duplicate-host row",
|
||||
"duplicate repo and worktree ids isolate admissions and PTY liveness by execution host while whole-project removal still drains every host scope",
|
||||
"host-scoped terminal stop succeeds when an unrelated provider is unavailable without disconnecting that host's cached PTYs",
|
||||
"workspace resolution on an unrelated host proceeds while the removed host remains admission-fenced",
|
||||
"worktree removal rejects a late renderer terminal admission while destructive preflight is still pending",
|
||||
"already-missing exceptional removal verifies terminal stop before metadata purge",
|
||||
"failed and concurrent admissions release without deadlocking project removal",
|
||||
@@ -3622,7 +3627,20 @@
|
||||
{
|
||||
"file": "src/main/providers/local-pty-provider.test.ts",
|
||||
"assertions": [
|
||||
"a synchronous Windows native kill throw retains PTY ownership and listeners so retry can succeed"
|
||||
"a synchronous Windows native kill throw retains PTY ownership and listeners so retry can succeed",
|
||||
"a stale pane identity is rejected before the local PTY can be killed"
|
||||
]
|
||||
},
|
||||
{
|
||||
"file": "src/main/daemon/daemon-pty-adapter.test.ts",
|
||||
"assertions": [
|
||||
"daemon shutdown carries pane identity across RPC and rejects a stale kill"
|
||||
]
|
||||
},
|
||||
{
|
||||
"file": "src/main/daemon/terminal-host.test.ts",
|
||||
"assertions": [
|
||||
"a reused live daemon session rejects attach and kill from a replacement pane identity"
|
||||
]
|
||||
},
|
||||
{
|
||||
@@ -3679,7 +3697,14 @@
|
||||
"generationless identity-mismatch failure cannot tombstone a replacement relay-generation lease",
|
||||
"paired/mobile controller kill fails closed when offline SSH intent cannot be made durable",
|
||||
"host-scoped liveness lists only the selected provider instead of fanning out to unrelated SSH hosts",
|
||||
"relay generation reuse removes stale retry ownership without touching the new PTY"
|
||||
"relay generation reuse removes stale retry ownership without touching the new PTY",
|
||||
"lease-less SSH shutdown survives provider unregister and re-drives the replacement transport"
|
||||
]
|
||||
},
|
||||
{
|
||||
"file": "src/main/ipc/runtime-terminal-close-retry-owner.test.ts",
|
||||
"assertions": [
|
||||
"a late runtime close completion cannot delete a replacement-generation retry owner"
|
||||
]
|
||||
},
|
||||
{
|
||||
|
||||
@@ -3,7 +3,7 @@ import { PREVIOUS_DAEMON_PROTOCOL_VERSIONS, PROTOCOL_VERSION } from './types'
|
||||
|
||||
describe('foreground-confirmation daemon protocol', () => {
|
||||
it('rejects daemons from before the fresh-confirmation RPC', () => {
|
||||
expect(PROTOCOL_VERSION).toBeGreaterThan(19)
|
||||
expect(PREVIOUS_DAEMON_PROTOCOL_VERSIONS).toContain(19)
|
||||
expect(PROTOCOL_VERSION).toBeGreaterThan(21)
|
||||
expect(PREVIOUS_DAEMON_PROTOCOL_VERSIONS).toContain(21)
|
||||
})
|
||||
})
|
||||
|
||||
@@ -377,6 +377,30 @@ describe('DaemonPtyAdapter (IPtyProvider)', () => {
|
||||
})
|
||||
|
||||
describe('shutdown', () => {
|
||||
it('carries pane identity through the daemon and rejects a stale kill', async () => {
|
||||
const { id } = await adapter.spawn({
|
||||
cols: 80,
|
||||
rows: 24,
|
||||
env: { ORCA_PANE_KEY: 'tab-b:leaf-b', ORCA_TAB_ID: 'tab-b' }
|
||||
})
|
||||
|
||||
await expect(
|
||||
adapter.shutdown(id, {
|
||||
immediate: true,
|
||||
expectedPaneKey: 'tab-a:leaf-a',
|
||||
expectedTabId: 'tab-a'
|
||||
})
|
||||
).rejects.toThrow('PTY identity mismatch')
|
||||
expect(lastSubprocess.forceKill).not.toHaveBeenCalled()
|
||||
expect(await adapter.listProcesses()).toHaveLength(1)
|
||||
|
||||
await adapter.shutdown(id, {
|
||||
immediate: true,
|
||||
expectedPaneKey: 'tab-b:leaf-b',
|
||||
expectedTabId: 'tab-b'
|
||||
})
|
||||
})
|
||||
|
||||
it('kills the session', async () => {
|
||||
const { id } = await adapter.spawn({ cols: 80, rows: 24 })
|
||||
await adapter.shutdown(id, { immediate: false })
|
||||
|
||||
@@ -24,6 +24,7 @@ import type {
|
||||
PtyBackgroundStreamEvent,
|
||||
PtyProviderBufferSnapshot,
|
||||
PtyProcessInfo,
|
||||
PtyShutdownOptions,
|
||||
PtySpawnOptions,
|
||||
PtySpawnResult
|
||||
} from '../providers/types'
|
||||
@@ -31,6 +32,11 @@ import { isShellProcess } from '../../shared/agent-detection'
|
||||
import { recognizeAgentProcessFromCommandLine } from '../../shared/agent-process-recognition'
|
||||
import { shouldUseShellReadyStartupDelivery } from '../../shared/codex-startup-delivery'
|
||||
import type { TerminalOscLinkRange } from '../../shared/terminal-osc-link-ranges'
|
||||
import {
|
||||
assertPtyPaneIdentity,
|
||||
getPtyPaneIdentityFromEnv,
|
||||
type PtyPaneIdentity
|
||||
} from '../pty/pty-shutdown-identity'
|
||||
|
||||
type ColdRestorePayload = {
|
||||
scrollback: string
|
||||
@@ -102,6 +108,7 @@ export class DaemonPtyAdapter implements IPtyProvider {
|
||||
private coldRestoreCache = new Map<string, ColdRestorePayload>()
|
||||
private sleepRestoreSessionIds = new Set<string>()
|
||||
private activeSessionIds = new Set<string>()
|
||||
private paneIdentityBySessionId = new Map<string, PtyPaneIdentity>()
|
||||
private dirtySessionVersions = new Map<string, number>()
|
||||
// Why: a cold-restored session is a fresh shell whose on-disk checkpoint and
|
||||
// log belong to the pre-crash session. Incremental appends would land on
|
||||
@@ -168,16 +175,35 @@ export class DaemonPtyAdapter implements IPtyProvider {
|
||||
return this.historyManager
|
||||
}
|
||||
|
||||
private rememberSessionIdentity(session: SessionInfo): void {
|
||||
if (session.paneKey || session.tabId) {
|
||||
this.paneIdentityBySessionId.set(session.sessionId, {
|
||||
paneKey: session.paneKey ?? null,
|
||||
tabId: session.tabId ?? null
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
async spawn(opts: PtySpawnOptions): Promise<PtySpawnResult> {
|
||||
return this.withDaemonRetry(() => this.doSpawn(opts))
|
||||
}
|
||||
|
||||
private async doSpawn(opts: PtySpawnOptions): Promise<PtySpawnResult> {
|
||||
const sessionId = opts.sessionId ?? mintPtySessionId(opts.worktreeId)
|
||||
const requestedIdentity = getPtyPaneIdentityFromEnv(opts.env)
|
||||
|
||||
if (this.killedSessionTombstones.has(sessionId)) {
|
||||
throw new TerminalKilledError(sessionId)
|
||||
}
|
||||
if (this.protocolVersion < 22 && this.activeSessionIds.has(sessionId)) {
|
||||
const knownIdentity = this.paneIdentityBySessionId.get(sessionId)
|
||||
if (knownIdentity) {
|
||||
assertPtyPaneIdentity(sessionId, knownIdentity, {
|
||||
expectedPaneKey: requestedIdentity.paneKey ?? undefined,
|
||||
expectedTabId: requestedIdentity.tabId ?? undefined
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
if (opts.isNewSession) {
|
||||
await this.replaceUnhealthyMacResolverDaemonBeforeNewPty()
|
||||
@@ -238,6 +264,8 @@ export class DaemonPtyAdapter implements IPtyProvider {
|
||||
command: opts.command,
|
||||
startupCommandDelivery: opts.startupCommandDelivery,
|
||||
launchAgent: opts.launchAgent,
|
||||
...(requestedIdentity.paneKey ? { paneKey: requestedIdentity.paneKey } : {}),
|
||||
...(requestedIdentity.tabId ? { tabId: requestedIdentity.tabId } : {}),
|
||||
// Why: without this, the daemon always spawns cmd.exe (COMSPEC) or
|
||||
// PowerShell as a fallback — regardless of which shell the renderer
|
||||
// asked for in the "+" menu or persisted as the default. Forwarding
|
||||
@@ -253,6 +281,7 @@ export class DaemonPtyAdapter implements IPtyProvider {
|
||||
|
||||
let scrollback = restoreInfo ? getRecoveredHistorySeed(restoreInfo) : null
|
||||
let result = await createOrAttach(scrollback)
|
||||
this.paneIdentityBySessionId.set(sessionId, requestedIdentity)
|
||||
const launchIdentity = (): { launchAgent?: NonNullable<typeof result.launchAgent> } =>
|
||||
result.launchAgent ? { launchAgent: result.launchAgent } : {}
|
||||
|
||||
@@ -485,7 +514,19 @@ export class DaemonPtyAdapter implements IPtyProvider {
|
||||
this.client.notify('setSessionBackground', { sessionId: id, background: safeBackground })
|
||||
}
|
||||
|
||||
async shutdown(id: string, opts: { immediate?: boolean; keepHistory?: boolean }): Promise<void> {
|
||||
async shutdown(id: string, opts: PtyShutdownOptions): Promise<void> {
|
||||
if (
|
||||
this.protocolVersion < 22 &&
|
||||
(opts.expectedPaneKey !== undefined || opts.expectedTabId !== undefined)
|
||||
) {
|
||||
// Why: preserved pre-v22 daemons ignore kill identity fields. Fail
|
||||
// closed unless this main process observed the exact attached pane.
|
||||
assertPtyPaneIdentity(
|
||||
id,
|
||||
this.paneIdentityBySessionId.get(id) ?? { paneKey: null, tabId: null },
|
||||
opts
|
||||
)
|
||||
}
|
||||
// Why: sleep/exact-stop kills the live PTY before the periodic checkpoint may run.
|
||||
// Force a final snapshot so wake can restore the pane users left.
|
||||
if (opts.keepHistory) {
|
||||
@@ -500,12 +541,18 @@ export class DaemonPtyAdapter implements IPtyProvider {
|
||||
this.sleepRestoreSessionIds.add(id)
|
||||
}
|
||||
}
|
||||
await this.client.request('kill', { sessionId: id, immediate: opts.immediate ?? false })
|
||||
await this.client.request('kill', {
|
||||
sessionId: id,
|
||||
immediate: opts.immediate ?? false,
|
||||
expectedPaneKey: opts.expectedPaneKey,
|
||||
expectedTabId: opts.expectedTabId
|
||||
})
|
||||
this.activeSessionIds.delete(id)
|
||||
this.dirtySessionVersions.delete(id)
|
||||
if (!opts.keepHistory) {
|
||||
this.coldRestoreCache.delete(id)
|
||||
this.sleepRestoreSessionIds.delete(id)
|
||||
this.paneIdentityBySessionId.delete(id)
|
||||
}
|
||||
// Why: the !keepHistory close path doesn't take a final checkpoint, so a
|
||||
// session stranded in sessionsNeedingFullCheckpoint would never be cleared.
|
||||
@@ -714,6 +761,7 @@ export class DaemonPtyAdapter implements IPtyProvider {
|
||||
const killed: string[] = []
|
||||
|
||||
for (const session of result.sessions) {
|
||||
this.rememberSessionIdentity(session)
|
||||
if (!session.isAlive) {
|
||||
continue
|
||||
}
|
||||
@@ -746,6 +794,9 @@ export class DaemonPtyAdapter implements IPtyProvider {
|
||||
async listProcesses(): Promise<PtyProcessInfo[]> {
|
||||
await this.ensureConnected()
|
||||
const result = await this.client.request<ListSessionsResult>('listSessions', undefined)
|
||||
for (const session of result.sessions) {
|
||||
this.rememberSessionIdentity(session)
|
||||
}
|
||||
return result.sessions
|
||||
.filter((s) => s.isAlive)
|
||||
.map((s) => ({
|
||||
@@ -763,6 +814,9 @@ export class DaemonPtyAdapter implements IPtyProvider {
|
||||
async listSessions(): Promise<SessionInfo[]> {
|
||||
await this.ensureConnected()
|
||||
const result = await this.client.request<ListSessionsResult>('listSessions', undefined)
|
||||
for (const session of result.sessions) {
|
||||
this.rememberSessionIdentity(session)
|
||||
}
|
||||
return result.sessions.filter((s) => s.isAlive)
|
||||
}
|
||||
|
||||
@@ -1326,6 +1380,7 @@ export class DaemonPtyAdapter implements IPtyProvider {
|
||||
this.backgroundedSessionIds.delete(event.sessionId)
|
||||
if (!this.sleepRestoreSessionIds.has(event.sessionId)) {
|
||||
this.coldRestoreCache.delete(event.sessionId)
|
||||
this.paneIdentityBySessionId.delete(event.sessionId)
|
||||
}
|
||||
// Why: an exited session can never be checkpointed again, so its pending
|
||||
// full-checkpoint flag is dead state. Without this, a cold-restored
|
||||
|
||||
@@ -1,5 +1,7 @@
|
||||
import { DaemonClient } from './client'
|
||||
import type { DaemonEvent, CreateOrAttachResult } from './types'
|
||||
import type { PtyShutdownOptions } from '../providers/types'
|
||||
import { getPtyPaneIdentityFromEnv } from '../pty/pty-shutdown-identity'
|
||||
|
||||
export type DaemonPtyProviderOptions = {
|
||||
socketPath: string
|
||||
@@ -38,6 +40,7 @@ export class DaemonPtyProvider {
|
||||
async spawn(opts: DaemonSpawnOptions): Promise<DaemonSpawnResult> {
|
||||
await this.client.ensureConnected()
|
||||
this.setupEventRouting()
|
||||
const identity = getPtyPaneIdentityFromEnv(opts.env)
|
||||
|
||||
const result = await this.client.request<CreateOrAttachResult>('createOrAttach', {
|
||||
sessionId: opts.sessionId,
|
||||
@@ -46,7 +49,9 @@ export class DaemonPtyProvider {
|
||||
cwd: opts.cwd,
|
||||
env: opts.env,
|
||||
envToDelete: opts.envToDelete,
|
||||
command: opts.command
|
||||
command: opts.command,
|
||||
...(identity.paneKey ? { paneKey: identity.paneKey } : {}),
|
||||
...(identity.tabId ? { tabId: identity.tabId } : {})
|
||||
})
|
||||
|
||||
return {
|
||||
@@ -64,8 +69,13 @@ export class DaemonPtyProvider {
|
||||
this.client.notify('resize', { sessionId: id, cols, rows })
|
||||
}
|
||||
|
||||
async shutdown(id: string, opts: { immediate?: boolean; keepHistory?: boolean }): Promise<void> {
|
||||
await this.client.request('kill', { sessionId: id, immediate: opts.immediate ?? false })
|
||||
async shutdown(id: string, opts: PtyShutdownOptions): Promise<void> {
|
||||
await this.client.request('kill', {
|
||||
sessionId: id,
|
||||
immediate: opts.immediate ?? false,
|
||||
expectedPaneKey: opts.expectedPaneKey,
|
||||
expectedTabId: opts.expectedTabId
|
||||
})
|
||||
}
|
||||
|
||||
onData(callback: (payload: { id: string; data: string }) => void): () => void {
|
||||
|
||||
@@ -4,6 +4,7 @@ import type {
|
||||
PtyBackgroundStreamEvent,
|
||||
PtyProviderBufferSnapshot,
|
||||
PtyProcessInfo,
|
||||
PtyShutdownOptions,
|
||||
PtySpawnOptions,
|
||||
PtySpawnResult
|
||||
} from '../providers/types'
|
||||
@@ -94,7 +95,7 @@ export class DaemonPtyRouter implements IPtyProvider {
|
||||
this.adapterFor(id).setPtyBackgrounded(id, background)
|
||||
}
|
||||
|
||||
async shutdown(id: string, opts: { immediate?: boolean; keepHistory?: boolean }): Promise<void> {
|
||||
async shutdown(id: string, opts: PtyShutdownOptions): Promise<void> {
|
||||
await this.adapterFor(id).shutdown(id, opts)
|
||||
// Why: sleep passes keepHistory=true and re-spawns against the same
|
||||
// sessionId on wake. If we delete the routing entry here, adapterFor()
|
||||
|
||||
@@ -345,6 +345,8 @@ export class DaemonServer {
|
||||
// Why: daemon RPC payloads are untrusted JSON. Persist only the
|
||||
// allowlisted enum used for byte routing, never arbitrary identity.
|
||||
...(isTuiAgent(p.launchAgent) ? { launchAgent: p.launchAgent } : {}),
|
||||
...(typeof p.paneKey === 'string' ? { paneKey: p.paneKey } : {}),
|
||||
...(typeof p.tabId === 'string' ? { tabId: p.tabId } : {}),
|
||||
shellOverride: p.shellOverride,
|
||||
terminalWindowsWslDistro: p.terminalWindowsWslDistro,
|
||||
terminalWindowsPowerShellImplementation: p.terminalWindowsPowerShellImplementation,
|
||||
@@ -498,7 +500,11 @@ export class DaemonServer {
|
||||
sessionId: request.payload.sessionId,
|
||||
immediate: request.payload.immediate === true
|
||||
})
|
||||
this.host.kill(request.payload.sessionId, { immediate: request.payload.immediate })
|
||||
this.host.kill(request.payload.sessionId, {
|
||||
immediate: request.payload.immediate,
|
||||
expectedPaneKey: request.payload.expectedPaneKey,
|
||||
expectedTabId: request.payload.expectedTabId
|
||||
})
|
||||
return {}
|
||||
|
||||
case 'signal':
|
||||
|
||||
@@ -76,6 +76,8 @@ export type SubprocessHandle = {
|
||||
|
||||
export type SessionOptions = {
|
||||
sessionId: string
|
||||
paneKey?: string
|
||||
tabId?: string
|
||||
cols: number
|
||||
rows: number
|
||||
terminalHandle?: string
|
||||
@@ -101,6 +103,8 @@ type AttachedClient = {
|
||||
|
||||
export class Session {
|
||||
readonly sessionId: string
|
||||
readonly paneKey: string | null
|
||||
readonly tabId: string | null
|
||||
readonly terminalHandle: string | null
|
||||
readonly launchAgent: TuiAgent | null
|
||||
private _state: SessionState = 'running'
|
||||
@@ -128,6 +132,8 @@ export class Session {
|
||||
|
||||
constructor(opts: SessionOptions) {
|
||||
this.sessionId = opts.sessionId
|
||||
this.paneKey = opts.paneKey ?? null
|
||||
this.tabId = opts.tabId ?? null
|
||||
this.terminalHandle = opts.terminalHandle ?? null
|
||||
this.launchAgent = opts.launchAgent ?? null
|
||||
this.subprocess = opts.subprocess
|
||||
|
||||
@@ -12,6 +12,8 @@ export type CreateOrAttachOptions = {
|
||||
command?: string
|
||||
startupCommandDelivery?: StartupCommandDelivery
|
||||
launchAgent?: TuiAgent
|
||||
paneKey?: string
|
||||
tabId?: string
|
||||
/** Explicit shell the renderer asked for, forwarded to the subprocess. */
|
||||
shellOverride?: string
|
||||
terminalWindowsWslDistro?: string | null
|
||||
|
||||
@@ -72,6 +72,29 @@ describe('TerminalHost', () => {
|
||||
})
|
||||
|
||||
describe('createOrAttach', () => {
|
||||
it('rejects attaching a replacement pane to a reused live session id', async () => {
|
||||
await host.createOrAttach({
|
||||
sessionId: 'session-1',
|
||||
paneKey: 'tab-a:leaf-a',
|
||||
tabId: 'tab-a',
|
||||
cols: 80,
|
||||
rows: 24,
|
||||
streamClient: { onData: vi.fn(), onExit: vi.fn() }
|
||||
})
|
||||
|
||||
await expect(
|
||||
host.createOrAttach({
|
||||
sessionId: 'session-1',
|
||||
paneKey: 'tab-b:leaf-b',
|
||||
tabId: 'tab-b',
|
||||
cols: 80,
|
||||
rows: 24,
|
||||
streamClient: { onData: vi.fn(), onExit: vi.fn() }
|
||||
})
|
||||
).rejects.toThrow('PTY identity mismatch')
|
||||
expect(spawnFn).toHaveBeenCalledOnce()
|
||||
})
|
||||
|
||||
it('creates a new session when none exists', async () => {
|
||||
const result = await host.createOrAttach({
|
||||
sessionId: 'session-1',
|
||||
@@ -361,6 +384,27 @@ describe('TerminalHost', () => {
|
||||
})
|
||||
|
||||
describe('kill', () => {
|
||||
it('rejects a stale pane kill before force-killing the current session', async () => {
|
||||
await host.createOrAttach({
|
||||
sessionId: 'session-1',
|
||||
paneKey: 'tab-b:leaf-b',
|
||||
tabId: 'tab-b',
|
||||
cols: 80,
|
||||
rows: 24,
|
||||
streamClient: { onData: vi.fn(), onExit: vi.fn() }
|
||||
})
|
||||
|
||||
expect(() =>
|
||||
host.kill('session-1', {
|
||||
immediate: true,
|
||||
expectedPaneKey: 'tab-a:leaf-a',
|
||||
expectedTabId: 'tab-a'
|
||||
})
|
||||
).toThrow('PTY identity mismatch')
|
||||
expect(lastSubprocess.forceKill).not.toHaveBeenCalled()
|
||||
expect(host.listSessions()).toHaveLength(1)
|
||||
})
|
||||
|
||||
it('kills the session and tombstones it', async () => {
|
||||
await host.createOrAttach({
|
||||
sessionId: 'session-1',
|
||||
|
||||
@@ -7,6 +7,7 @@ import { buildStartupCommandSubmission } from '../../shared/startup-command-subm
|
||||
import type { SessionInfo, TakePendingOutputResult, TerminalSnapshot } from './types'
|
||||
import { SessionNotFoundError } from './types'
|
||||
import type { CreateOrAttachOptions, CreateOrAttachResult } from './terminal-host-create-contract'
|
||||
import { assertPtyPaneIdentity } from '../pty/pty-shutdown-identity'
|
||||
|
||||
export type { CreateOrAttachOptions, CreateOrAttachResult } from './terminal-host-create-contract'
|
||||
|
||||
@@ -67,6 +68,11 @@ export class TerminalHost {
|
||||
// subsequent operation (write/kill/resize) would fail once the subprocess
|
||||
// finally exits. Treat terminating sessions the same as fully-exited ones.
|
||||
if (existing && existing.isAlive && !existing.isTerminating) {
|
||||
assertPtyPaneIdentity(
|
||||
opts.sessionId,
|
||||
{ paneKey: existing.paneKey, tabId: existing.tabId },
|
||||
{ expectedPaneKey: opts.paneKey, expectedTabId: opts.tabId }
|
||||
)
|
||||
const snapshot = existing.getSnapshot()
|
||||
existing.detachAllClients()
|
||||
const token = existing.attachClient(opts.streamClient)
|
||||
@@ -117,6 +123,8 @@ export class TerminalHost {
|
||||
|
||||
const session = new Session({
|
||||
sessionId: opts.sessionId,
|
||||
paneKey: opts.paneKey,
|
||||
tabId: opts.tabId,
|
||||
cols: size.cols,
|
||||
rows: size.rows,
|
||||
terminalHandle: opts.env?.ORCA_TERMINAL_HANDLE,
|
||||
@@ -193,8 +201,12 @@ export class TerminalHost {
|
||||
this.sessions.get(sessionId)?.resumeProducer()
|
||||
}
|
||||
|
||||
kill(sessionId: string, opts: { immediate?: boolean } = {}): void {
|
||||
kill(
|
||||
sessionId: string,
|
||||
opts: { immediate?: boolean; expectedPaneKey?: string; expectedTabId?: string } = {}
|
||||
): void {
|
||||
const session = this.getAliveSession(sessionId)
|
||||
assertPtyPaneIdentity(sessionId, { paneKey: session.paneKey, tabId: session.tabId }, opts)
|
||||
this.recordTombstone(sessionId)
|
||||
if (opts.immediate) {
|
||||
session.forceKillAndDisposeSubprocess()
|
||||
@@ -326,6 +338,8 @@ export class TerminalHost {
|
||||
const size = session.getAppliedSize()
|
||||
result.push({
|
||||
sessionId: session.sessionId,
|
||||
...(session.paneKey ? { paneKey: session.paneKey } : {}),
|
||||
...(session.tabId ? { tabId: session.tabId } : {}),
|
||||
state: session.state,
|
||||
shellState: session.shellState,
|
||||
isAlive: true,
|
||||
|
||||
@@ -12,13 +12,12 @@ export type {
|
||||
// ─── Protocol Version ────────────────────────────────────────────────
|
||||
import type { StartupCommandDelivery } from '../../shared/codex-startup-delivery'
|
||||
import type { TuiAgent } from '../../shared/types'
|
||||
// Why: daemons can survive app updates. Bump for IPC wire-shape changes, or
|
||||
// when daemon-baked behavior cannot be delivered by on-disk wrapper refresh.
|
||||
// Why: bump when adding daemon wire behavior so same-version old daemons do
|
||||
// not silently accept the handshake and then reject new RPCs.
|
||||
export const PROTOCOL_VERSION = 21
|
||||
import type * as PtyIdentity from '../pty/pty-shutdown-identity'
|
||||
// Why: daemons survive app updates; bump for wire changes so old daemons do
|
||||
// not accept handshakes for unsupported RPC behavior.
|
||||
export const PROTOCOL_VERSION = 22
|
||||
export const PREVIOUS_DAEMON_PROTOCOL_VERSIONS = [
|
||||
1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20
|
||||
1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20, 21
|
||||
] as const
|
||||
|
||||
// ─── Session State Machine ──────────────────────────────────────────
|
||||
@@ -98,7 +97,7 @@ export type HelloResponse = {
|
||||
export type CreateOrAttachRequest = {
|
||||
id: string
|
||||
type: 'createOrAttach'
|
||||
payload: {
|
||||
payload: PtyIdentity.PtyPaneIdentityFields & {
|
||||
sessionId: string
|
||||
cols: number
|
||||
rows: number
|
||||
@@ -190,7 +189,7 @@ export type SetSessionBackgroundRequest = {
|
||||
export type KillRequest = {
|
||||
id: string
|
||||
type: 'kill'
|
||||
payload: {
|
||||
payload: PtyIdentity.PtyExpectedPaneIdentityFields & {
|
||||
sessionId: string
|
||||
immediate?: boolean
|
||||
}
|
||||
@@ -380,7 +379,7 @@ export type SystemResolverHealthResult = {
|
||||
health: SystemResolverHealth
|
||||
}
|
||||
|
||||
export type SessionInfo = {
|
||||
export type SessionInfo = PtyIdentity.PtyPaneIdentityFields & {
|
||||
sessionId: string
|
||||
state: SessionState
|
||||
shellState: ShellReadyState
|
||||
|
||||
@@ -202,6 +202,7 @@ import {
|
||||
setLocalPtyProvider,
|
||||
rebindLocalProviderListeners,
|
||||
unregisterSshPtyProvider,
|
||||
releasePendingSshShutdownsForTarget,
|
||||
getLocalPtyProvider
|
||||
} from './pty'
|
||||
import {
|
||||
@@ -438,6 +439,7 @@ describe('registerPtyHandlers', () => {
|
||||
'ssh-runtime-env'
|
||||
]) {
|
||||
unregisterSshPtyProvider(leakedConnectionId)
|
||||
releasePendingSshShutdownsForTarget(leakedConnectionId)
|
||||
}
|
||||
setLocalPtyProvider(new LocalPtyProvider())
|
||||
if (savedProcessPlatform) {
|
||||
@@ -3651,6 +3653,53 @@ describe('registerPtyHandlers', () => {
|
||||
vi.useRealTimers()
|
||||
})
|
||||
|
||||
it('re-drives a lease-less SSH kill after provider unregister and replacement', async () => {
|
||||
vi.useFakeTimers()
|
||||
const oldShutdown = makeDeferred()
|
||||
const oldProviderShutdown = vi.fn(() => oldShutdown.promise)
|
||||
const newProviderShutdown = vi.fn().mockResolvedValue(undefined)
|
||||
const store = {
|
||||
getSshRemotePtyLeases: vi.fn(() => []),
|
||||
markSshRemotePtyShutdownRequested: vi.fn(() => false),
|
||||
markSshRemotePtyLease: vi.fn()
|
||||
}
|
||||
handlers.clear()
|
||||
registerPtyHandlers(
|
||||
mainWindow as never,
|
||||
undefined,
|
||||
undefined,
|
||||
undefined,
|
||||
undefined,
|
||||
store as never
|
||||
)
|
||||
registerSshPtyProvider('ssh-1', {
|
||||
shutdown: oldProviderShutdown,
|
||||
onExit: vi.fn(() => () => {})
|
||||
} as never)
|
||||
const id = 'ssh:ssh-1@@boot-1@@lease-less-pty'
|
||||
|
||||
const kill = handlers.get('pty:kill')!(null, {
|
||||
id,
|
||||
expectedPaneKey: 'tab-a:leaf-a',
|
||||
expectedTabId: 'tab-a'
|
||||
}) as Promise<void>
|
||||
expect(oldProviderShutdown).toHaveBeenCalledOnce()
|
||||
unregisterSshPtyProvider('ssh-1')
|
||||
registerSshPtyProvider('ssh-1', {
|
||||
shutdown: newProviderShutdown,
|
||||
onExit: vi.fn(() => () => {})
|
||||
} as never)
|
||||
oldShutdown.resolve()
|
||||
|
||||
await expect(kill).resolves.toBeUndefined()
|
||||
expect(newProviderShutdown).toHaveBeenCalledOnce()
|
||||
expect(store.markSshRemotePtyLease).toHaveBeenCalledWith('ssh-1', id, 'terminated')
|
||||
unregisterSshPtyProvider('ssh-1')
|
||||
await vi.advanceTimersByTimeAsync(30_000)
|
||||
expect(vi.getTimerCount()).toBe(0)
|
||||
vi.useRealTimers()
|
||||
})
|
||||
|
||||
it('rebinds a persisted local shutdown after async daemon startup replaces the provider', async () => {
|
||||
vi.useFakeTimers()
|
||||
const startup = makeDeferred()
|
||||
|
||||
+25
-3
@@ -180,6 +180,7 @@ type RetainedPtyShutdown = {
|
||||
options: PtyShutdownOptions
|
||||
provider: IPtyProvider | null
|
||||
store: Store | null
|
||||
durable: boolean
|
||||
attempts: number
|
||||
nextRetryAt: number
|
||||
inFlight: Promise<void> | null
|
||||
@@ -759,6 +760,7 @@ function retainLocalPtyShutdown(
|
||||
options,
|
||||
provider: null,
|
||||
store: activePtyStore,
|
||||
durable: true,
|
||||
attempts,
|
||||
nextRetryAt: deferUntilProviderReady
|
||||
? Number.POSITIVE_INFINITY
|
||||
@@ -804,6 +806,7 @@ function retainSshPtyShutdown(
|
||||
options,
|
||||
provider,
|
||||
store: activePtyStore,
|
||||
durable: persisted,
|
||||
attempts: 0,
|
||||
nextRetryAt: provider ? 0 : Number.POSITIVE_INFINITY,
|
||||
inFlight: null,
|
||||
@@ -811,6 +814,7 @@ function retainSshPtyShutdown(
|
||||
}
|
||||
pendingSshShutdownRetries.set(id, retained)
|
||||
} else {
|
||||
retained.durable ||= persisted
|
||||
retained.options = mergePtyShutdownOptions(retained.options, options)
|
||||
if (retained.provider !== provider) {
|
||||
retained.provider = provider
|
||||
@@ -1341,15 +1345,31 @@ function routesFreshSpawnsToLocalProvider(
|
||||
/** Register an SSH PTY provider for a connection. */
|
||||
export function registerSshPtyProvider(connectionId: string, provider: IPtyProvider): void {
|
||||
sshProviders.set(connectionId, provider)
|
||||
// Durable leases reconcile generation first; a stale boot-qualified owner
|
||||
// must be dropped before any replacement provider can observe it.
|
||||
retryPersistedSshShutdowns(connectionId, provider)
|
||||
for (const retained of pendingSshShutdownRetries.values()) {
|
||||
if (retained.connectionId !== connectionId || retained.durable) {
|
||||
continue
|
||||
}
|
||||
retained.provider = provider
|
||||
retained.nextRetryAt = 0
|
||||
if (!retained.inFlight) {
|
||||
void attemptRetainedPtyShutdown(retained)
|
||||
}
|
||||
}
|
||||
scheduleRetainedShutdownRetries()
|
||||
}
|
||||
|
||||
/** Remove an SSH PTY provider when a connection is closed. */
|
||||
export function unregisterSshPtyProvider(connectionId: string): void {
|
||||
sshProviders.delete(connectionId)
|
||||
for (const [id, retained] of pendingSshShutdownRetries) {
|
||||
for (const retained of pendingSshShutdownRetries.values()) {
|
||||
if (retained.connectionId === connectionId) {
|
||||
pendingSshShutdownRetries.delete(id)
|
||||
// Why: provider teardown is often a reconnect, not proof the remote PTY
|
||||
// exited. Keep even lease-less accepted kills for the replacement owner.
|
||||
retained.provider = null
|
||||
retained.nextRetryAt = Number.POSITIVE_INFINITY
|
||||
}
|
||||
}
|
||||
scheduleRetainedShutdownRetries()
|
||||
@@ -1386,7 +1406,7 @@ function retryPersistedSshShutdowns(connectionId: string, provider: IPtyProvider
|
||||
retainedLeases.map((lease) => toAppSshPtyId(connectionId, lease.ptyId, lease.relayInstanceId))
|
||||
)
|
||||
for (const [id, retained] of pendingSshShutdownRetries) {
|
||||
if (retained.connectionId === connectionId && !retainedIds.has(id)) {
|
||||
if (retained.connectionId === connectionId && retained.durable && !retainedIds.has(id)) {
|
||||
pendingSshShutdownRetries.delete(id)
|
||||
}
|
||||
}
|
||||
@@ -1406,6 +1426,7 @@ function retryPersistedSshShutdowns(connectionId: string, provider: IPtyProvider
|
||||
connectionId,
|
||||
provider,
|
||||
store,
|
||||
durable: true,
|
||||
attempts: 0,
|
||||
nextRetryAt: 0,
|
||||
inFlight: null,
|
||||
@@ -1414,6 +1435,7 @@ function retryPersistedSshShutdowns(connectionId: string, provider: IPtyProvider
|
||||
}
|
||||
pendingSshShutdownRetries.set(id, retained)
|
||||
} else {
|
||||
retained.durable = true
|
||||
retained.options = mergePtyShutdownOptions(retained.options, options)
|
||||
if (retained.provider !== provider) {
|
||||
// Why: reconnect replaces the transport owner for the same generation-qualified id.
|
||||
|
||||
@@ -942,6 +942,31 @@ describe('LocalPtyProvider', () => {
|
||||
})
|
||||
|
||||
describe('shutdown', () => {
|
||||
it('rejects a stale pane identity before killing the local PTY', async () => {
|
||||
const killSpy = mockProc.kill
|
||||
const { id } = await provider.spawn({
|
||||
cols: 80,
|
||||
rows: 24,
|
||||
env: { ORCA_PANE_KEY: 'tab-b:leaf-b', ORCA_TAB_ID: 'tab-b' }
|
||||
})
|
||||
|
||||
await expect(
|
||||
provider.shutdown(id, {
|
||||
immediate: true,
|
||||
expectedPaneKey: 'tab-a:leaf-a',
|
||||
expectedTabId: 'tab-a'
|
||||
})
|
||||
).rejects.toThrow('PTY identity mismatch')
|
||||
expect(killSpy).not.toHaveBeenCalled()
|
||||
expect(provider.hasPty(id)).toBe(true)
|
||||
|
||||
await provider.shutdown(id, {
|
||||
immediate: true,
|
||||
expectedPaneKey: 'tab-b:leaf-b',
|
||||
expectedTabId: 'tab-b'
|
||||
})
|
||||
})
|
||||
|
||||
it('kills the PTY process', async () => {
|
||||
// Why: capture the spy reference before shutdown triggers onExit →
|
||||
// POSIX kill neutralization. After neutralization, mockProc.kill is
|
||||
|
||||
@@ -64,6 +64,11 @@ import { readWindowsConptyProcessIds } from './windows-conpty-process-membership
|
||||
import { shouldUseShellReadyStartupDelivery } from '../../shared/codex-startup-delivery'
|
||||
import { assertSafeAgentStartupCwd, resolveSafePtyDefaultCwd } from './pty-default-cwd'
|
||||
import { ORCA_HERMES_STARTUP_QUERY_ENV } from '../../shared/hermes-startup-query'
|
||||
import {
|
||||
assertPtyPaneIdentity,
|
||||
getPtyPaneIdentityFromEnv,
|
||||
type PtyPaneIdentity
|
||||
} from '../pty/pty-shutdown-identity'
|
||||
|
||||
const PANE_IDENTITY_ENV_KEYS = [
|
||||
'ORCA_PANE_KEY',
|
||||
@@ -77,6 +82,7 @@ const ptyProcesses = new Map<string, pty.IPty>()
|
||||
const ptyShellName = new Map<string, string>()
|
||||
const ptyAgentForegroundContextPaths = new Map<string, string[]>()
|
||||
const ptyTerminalHandle = new Map<string, string>()
|
||||
const ptyPaneIdentity = new Map<string, PtyPaneIdentity>()
|
||||
// Why: node-pty's onData/onExit register native NAPI ThreadSafeFunction
|
||||
// callbacks. If the PTY is killed without disposing these listeners, the
|
||||
// stale callbacks survive into node::FreeEnvironment() where NAPI attempts
|
||||
@@ -188,6 +194,7 @@ function clearPtyState(id: string): void {
|
||||
ptyShellName.delete(id)
|
||||
ptyAgentForegroundContextPaths.delete(id)
|
||||
ptyTerminalHandle.delete(id)
|
||||
ptyPaneIdentity.delete(id)
|
||||
ptyLoadGeneration.delete(id)
|
||||
ptyShutdownInProgress.delete(id)
|
||||
ptyExitDuringShutdown.delete(id)
|
||||
@@ -696,6 +703,7 @@ export class LocalPtyProvider implements IPtyProvider {
|
||||
|
||||
const proc = spawnResult.process
|
||||
ptyProcesses.set(id, proc)
|
||||
ptyPaneIdentity.set(id, getPtyPaneIdentityFromEnv(finalEnv))
|
||||
ptyShellName.set(id, getSpawnedShellName(shellPath))
|
||||
if (finalEnv.ORCA_TERMINAL_HANDLE) {
|
||||
ptyTerminalHandle.set(id, finalEnv.ORCA_TERMINAL_HANDLE)
|
||||
@@ -897,11 +905,12 @@ export class LocalPtyProvider implements IPtyProvider {
|
||||
return { cols: proc.cols, rows: proc.rows }
|
||||
}
|
||||
|
||||
async shutdown(id: string, _opts: PtyShutdownOptions): Promise<void> {
|
||||
async shutdown(id: string, opts: PtyShutdownOptions): Promise<void> {
|
||||
const proc = ptyProcesses.get(id)
|
||||
if (!proc) {
|
||||
return
|
||||
}
|
||||
assertPtyPaneIdentity(id, ptyPaneIdentity.get(id) ?? { paneKey: null, tabId: null }, opts)
|
||||
// Why: keep listeners live until native shutdown is accepted so a thrown
|
||||
// kill retains retry ownership. The in-progress marker makes a synchronous
|
||||
// onExit an independent death proof without racing duplicate cleanup.
|
||||
|
||||
@@ -0,0 +1,34 @@
|
||||
import type { PtyShutdownOptions } from '../providers/types'
|
||||
|
||||
export type PtyPaneIdentity = {
|
||||
paneKey: string | null
|
||||
tabId: string | null
|
||||
}
|
||||
|
||||
export type PtyPaneIdentityFields = { paneKey?: string; tabId?: string }
|
||||
export type PtyExpectedPaneIdentityFields = Pick<
|
||||
PtyShutdownOptions,
|
||||
'expectedPaneKey' | 'expectedTabId'
|
||||
>
|
||||
|
||||
export function getPtyPaneIdentityFromEnv(
|
||||
env: Record<string, string> | undefined
|
||||
): PtyPaneIdentity {
|
||||
return {
|
||||
paneKey: env?.ORCA_PANE_KEY?.trim() || null,
|
||||
tabId: env?.ORCA_TAB_ID?.trim() || null
|
||||
}
|
||||
}
|
||||
|
||||
export function assertPtyPaneIdentity(
|
||||
id: string,
|
||||
actual: PtyPaneIdentity,
|
||||
expected: PtyExpectedPaneIdentityFields
|
||||
): void {
|
||||
if (
|
||||
(expected.expectedPaneKey !== undefined && actual.paneKey !== expected.expectedPaneKey) ||
|
||||
(expected.expectedTabId !== undefined && actual.tabId !== expected.expectedTabId)
|
||||
) {
|
||||
throw new Error(`PTY identity mismatch for "${id}"`)
|
||||
}
|
||||
}
|
||||
@@ -22624,6 +22624,51 @@ describe('OrcaRuntimeService', () => {
|
||||
expect(spawn).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it('allows workspace resolution on an unrelated host during host-scoped removal', async () => {
|
||||
const runtime = new OrcaRuntimeService(store)
|
||||
const runtimeWithResolver = runtime as unknown as {
|
||||
resolveTerminalWorkspaceLaunchScope: () => Promise<{
|
||||
id: string
|
||||
path: string
|
||||
hostId: 'ssh:target-1'
|
||||
connectionId: 'target-1'
|
||||
repo: ReturnType<typeof store.getRepo>
|
||||
folderWorkspace: null
|
||||
}>
|
||||
}
|
||||
vi.spyOn(runtimeWithResolver, 'resolveTerminalWorkspaceLaunchScope').mockResolvedValue({
|
||||
id: TEST_WORKTREE_ID,
|
||||
path: '/home/dev/orca',
|
||||
hostId: 'ssh:target-1',
|
||||
connectionId: 'target-1',
|
||||
repo: store.getRepo(TEST_REPO_ID),
|
||||
folderWorkspace: null
|
||||
})
|
||||
const spawn = vi.fn().mockResolvedValue({ id: 'ssh-pty' })
|
||||
runtime.setPtyController({
|
||||
spawn,
|
||||
write: () => true,
|
||||
kill: () => false,
|
||||
getForegroundProcess: async () => null,
|
||||
listProcesses: async () => []
|
||||
})
|
||||
const holdRemoval = deferred<void>()
|
||||
const removal = runtime.runWithWorktreeRemovalAdmission(
|
||||
TEST_WORKTREE_ID,
|
||||
() => holdRemoval.promise,
|
||||
'local'
|
||||
)
|
||||
await Promise.resolve()
|
||||
|
||||
await expect(runtime.createTerminal(`id:${TEST_WORKTREE_ID}`)).resolves.toMatchObject({
|
||||
ptyId: 'ssh-pty'
|
||||
})
|
||||
expect(spawn).toHaveBeenCalledWith(expect.objectContaining({ connectionId: 'target-1' }))
|
||||
|
||||
holdRemoval.resolve()
|
||||
await removal
|
||||
})
|
||||
|
||||
it('drains a create-first managed worktree before project removal snapshots ownership', async () => {
|
||||
const removeProject = vi.fn()
|
||||
const runtime = new OrcaRuntimeService({ ...store, removeProject } as never)
|
||||
|
||||
@@ -12351,14 +12351,35 @@ export class OrcaRuntimeService {
|
||||
): Promise<{ workspace: TerminalWorkspaceLaunchScope; release: () => void }> {
|
||||
// Why: workspace resolution can await SSH/runtime discovery. Keep removal
|
||||
// aware of the unresolved create, then transfer ownership without a gap.
|
||||
const releaseResolutionAdmission = this.beginTerminalAdmission(GLOBAL_TERMINAL_ADMISSION_KEY)
|
||||
let releaseResolutionAdmission: (() => void) | null = null
|
||||
let removalsActiveAtResolutionStart: ReadonlySet<string> | null = null
|
||||
try {
|
||||
releaseResolutionAdmission = this.beginTerminalAdmission(GLOBAL_TERMINAL_ADMISSION_KEY)
|
||||
} catch (error) {
|
||||
if (
|
||||
!(error instanceof Error) ||
|
||||
error.message !== 'terminal_admission_blocked_for_project_removal'
|
||||
) {
|
||||
throw error
|
||||
}
|
||||
// Why: an unresolved selector might belong to another execution host.
|
||||
// Resolve it, but remember the active removals so a fast completion
|
||||
// cannot reopen the exact stale scope before the atomic transfer.
|
||||
removalsActiveAtResolutionStart = new Set(this.terminalAdmissionBlockedWorktreeIds)
|
||||
}
|
||||
try {
|
||||
const workspace = await this.resolveTerminalWorkspaceLaunchScope(selector, hostId)
|
||||
if (
|
||||
removalsActiveAtResolutionStart &&
|
||||
this.isAdmissionBlocked(removalsActiveAtResolutionStart, workspace.id, workspace.hostId)
|
||||
) {
|
||||
throw new Error('terminal_admission_blocked_for_project_removal')
|
||||
}
|
||||
const releaseWorkspaceAdmission = this.beginTerminalAdmission(workspace.id, workspace.hostId)
|
||||
releaseResolutionAdmission()
|
||||
releaseResolutionAdmission?.()
|
||||
return { workspace, release: releaseWorkspaceAdmission }
|
||||
} catch (error) {
|
||||
releaseResolutionAdmission()
|
||||
releaseResolutionAdmission?.()
|
||||
throw error
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user