diff --git a/docs/audits/stale-pty-inventory/README.md b/docs/audits/stale-pty-inventory/README.md new file mode 100644 index 00000000000..a8641795c07 --- /dev/null +++ b/docs/audits/stale-pty-inventory/README.md @@ -0,0 +1,41 @@ +# Stale provider inventory overwrites newer PTY ownership + +A provider inventory started before a process exit can finish afterward and reconnect the exited runtime record. If a replacement was admitted under the same ID, the old response can also overwrite its incarnation, invalidate its terminal handle, and clear its pane binding. + +This is a separate source-level explanation for stale terminal ownership such as [#19018](https://github.com/stablyai/orca/issues/19018). The proof does not establish the cause of [#19768](https://github.com/stablyai/orca/issues/19768) or [#19831](https://github.com/stablyai/orca/issues/19831), or measure their reported memory growth. + +## Reproduce + +From a checkout with dependencies installed: + +```sh +ORCA_BACKGROUND_LAUNCH=1 node docs/audits/stale-pty-inventory/reproduce.mjs +``` + +The script runs the actual runtime inventory, registration, spawn, and exit methods with deferred provider responses. It uses temporary Vitest configuration to reverse only `fix.patch` for the baseline, leaves checkout files untouched, and removes its temporary files. It launches no app, enumerates no real processes, and installs nothing. Node subprocesses use the repository's portable `runProcess` implementation. + +`results.json` records source hashes and the before/after result: **16 failing / 6 passing tests before; all 22 passing after**. + +## Retaining and ownership path + +1. `refreshPtyWorktreeRecordsWithControllerInventory` starts a provider request and captures its generation and liveness observation sequence. +2. While the response is pending, `onPtyExit` records a physical exit. A subsequent `registerPty` or `onPtySpawned` may admit a successor, including one absent from the runtime when the request began. +3. The original inventory returns its older row. Previously, the verdict setter's observation check did not protect the rest of the row processing: handle adoption ran, and `recordPtyWorktree` wrote `connected: true` and the old incarnation. + +For an exit without replacement, the record could therefore be connected while its verdict still said `exited`. A later empty listing skips the disconnected-record sweep while a graph leaf remains. In the successor case, `adoptControllerTerminalHandle` can invalidate the new handle before the old incarnation is written. Fresh positive inventory can restore the successor handle and incarnation; it cannot reconstruct cleared pane coordinates without binding evidence. + +The proof exercises these mutations directly. It does not recreate a native process or a headless terminal model. Consequently, it demonstrates incorrect ownership and potential retained records, rather than a quantified heap or native-process leak. + +## Fix and controls + +Accepted spawn, registration, and exit events now invalidate pending inventory for their provider through the existing provider generation registry. The existing stale-response check runs before handle adoption, record mutation, or construction of an authoritative live-ID result. + +The response is rejected as a whole, so filtered rows cannot become false evidence of absence. A targeted worktree read retries at most once using its original deadline; another invalidation yields an unknown result. A targeted different-host query remains valid. Aggregate requests conservatively reject when a provider changes during the request. This can reject a response that happened to include the newly admitted process; a subsequent fresh request remains authoritative. + +The tests cover stale positive rows, stale absence, unknown-at-request-start spawns, registrations with no incarnation, ignored predecessor EXIT, exported handle preservation, fresh discovery and replacement, local/SSH routing (including SSH aliases with spaces, `@`, and `:`), concurrent provider generations, and bounded retries. Existing partial-relay and liveness tests verify that failed contact is not promoted to process death. The change adds no permanent per-PTY registry and changes no wire fields or liveness vocabulary. + +## Version and scope + +The relevant await, guarded verdict update, unconditional handle/record updates, and spawn/registration verdict deletion exist in **v1.4.198**. That version forgot the prior verdict on a positive inventory row; current code records `live`. Both versions leave subsequent mutation outside that observation fence. The executable baseline is current source with the narrow fix reversed, not a historical application binary. + +The graph-publication fence is a separate fix. Later loss-of-contact writes and independent paths that query provider inventory directly remain outside this patch. Incident frequency and the reporting machines' actual ordering are unproven. diff --git a/docs/audits/stale-pty-inventory/fix.patch b/docs/audits/stale-pty-inventory/fix.patch new file mode 100644 index 00000000000..9f49b7bf536 --- /dev/null +++ b/docs/audits/stale-pty-inventory/fix.patch @@ -0,0 +1,88 @@ +diff --git a/src/main/runtime/orca-runtime-invalidate-all-handles-for-pty.ts b/src/main/runtime/orca-runtime-invalidate-all-handles-for-pty.ts +index 448fb2a00fb..9d6e5d56ca4 100644 +--- a/src/main/runtime/orca-runtime-invalidate-all-handles-for-pty.ts ++++ b/src/main/runtime/orca-runtime-invalidate-all-handles-for-pty.ts +@@ -1,6 +1,12 @@ + // @ts-nocheck -- mechanically split from OrcaRuntimeService; behavior is covered by AST equivalence and characterization tests. + import { OrcaRuntimeWithResolveKnownWorkspaceFileTarget } from './orca-runtime-resolve-known-workspace-file-target' + import type { PtyIncarnationId } from '../../shared/pty-incarnation' ++import { getPtyExecutionHost } from '../../shared/terminal-execution-host' ++import { ++ LOCAL_EXECUTION_HOST_ID, ++ parseExecutionHostId, ++ toSshExecutionHostId ++} from '../../shared/execution-host' + + export class OrcaRuntimeWithInvalidateAllHandlesForPty extends OrcaRuntimeWithResolveKnownWorkspaceFileTarget { + protected invalidateAllHandlesForPty(ptyId: string, preserveHandle?: string): Set { +@@ -110,11 +116,30 @@ export class OrcaRuntimeWithInvalidateAllHandlesForPty extends OrcaRuntimeWithRe + return false + } + ++ protected invalidatePtyControllerInventoryForLifecycle( ++ ptyId: string, ++ connectionId?: string | null ++ ): void { ++ const generation = ++this.ptyControllerInventorySequence ++ const hostId = getPtyExecutionHost(ptyId) ++ if (hostId === 'foreign' || (hostId && parseExecutionHostId(hostId)?.kind === 'runtime')) { ++ this.ptyControllerAggregateInventoryGeneration = generation ++ return ++ } ++ const connection = ++ connectionId === undefined ? this.ptysById.get(ptyId)?.connectionId : connectionId ++ const providerKey = ++ hostId ?? (connection ? toSshExecutionHostId(connection) : LOCAL_EXECUTION_HOST_ID) ++ // A pending census predates this admission or exit, including legacy IDs without an incarnation. ++ this.ptyControllerInventoryGenerationByProvider.set(providerKey, generation) ++ } ++ + onPtySpawned( + ptyId: string, + incarnationId?: PtyIncarnationId, + options: { awaitsRegistration?: boolean } = {} + ): void { ++ this.invalidatePtyControllerInventoryForLifecycle(ptyId) + const existingPty = this.ptysById.get(ptyId) + if ( + existingPty && +diff --git a/src/main/runtime/orca-runtime-on-pty-exit.ts b/src/main/runtime/orca-runtime-on-pty-exit.ts +index 6ddf877f87c..ce24ed65d98 100644 +--- a/src/main/runtime/orca-runtime-on-pty-exit.ts ++++ b/src/main/runtime/orca-runtime-on-pty-exit.ts +@@ -30,6 +30,7 @@ export class OrcaRuntimeWithOnPtyExit extends OrcaRuntimeWithOnClientDisconnecte + if (exitIncarnationId && pty?.incarnationId && exitIncarnationId !== pty.incarnationId) { + return + } ++ this.invalidatePtyControllerInventoryForLifecycle(ptyId, pty?.connectionId) + // A bare exit code is not enough to establish why a process ended: older + // daemons and SSH relays can report 0 for crashes and wrapper exits. + const observedCause = options.cause ?? resolveUnreportedExitCause(exitCode) +diff --git a/src/main/runtime/orca-runtime-refresh-pty-worktree-records-with-controller-inventory.ts b/src/main/runtime/orca-runtime-refresh-pty-worktree-records-with-controller-inventory.ts +index aca67f67ba2..930907da120 100644 +--- a/src/main/runtime/orca-runtime-refresh-pty-worktree-records-with-controller-inventory.ts ++++ b/src/main/runtime/orca-runtime-refresh-pty-worktree-records-with-controller-inventory.ts +@@ -55,7 +55,10 @@ export class OrcaRuntimeWithRefreshPtyWorktreeRecordsWithControllerInventory ext + } + const inventoryGeneration = this.ptyControllerInventorySequence + 1 + this.ptyControllerInventorySequence = inventoryGeneration +- const providerKey = typeof connectionId === 'string' ? `ssh:${connectionId}` : 'local' ++ const providerKey = ++ typeof connectionId === 'string' ++ ? toSshExecutionHostId(connectionId) ++ : LOCAL_EXECUTION_HOST_ID + const livenessObservationAtStart = this.ptyLivenessObservationSequence + if (connectionId === undefined) { + this.ptyControllerAggregateInventoryGeneration = inventoryGeneration +diff --git a/src/main/runtime/orca-runtime-register-pty.ts b/src/main/runtime/orca-runtime-register-pty.ts +index dae2519ca9f..2d36a18a1e0 100644 +--- a/src/main/runtime/orca-runtime-register-pty.ts ++++ b/src/main/runtime/orca-runtime-register-pty.ts +@@ -26,6 +26,7 @@ export class OrcaRuntimeWithRegisterPty extends OrcaRuntimeWithInvalidateAllHand + isWsl?: boolean + ): void { + this.assertPtyDidNotExitBeforeRegistration(ptyId, binding?.incarnationId) ++ this.invalidatePtyControllerInventoryForLifecycle(ptyId, connectionId) + const existingPty = this.ptysById.get(ptyId) + const replacementHandle = binding?.terminalHandle?.trim() + const pendingReplacement = this.pendingPtyHandleReplacementFences.get(ptyId) diff --git a/docs/audits/stale-pty-inventory/reproduce.mjs b/docs/audits/stale-pty-inventory/reproduce.mjs new file mode 100644 index 00000000000..9807b764ee3 --- /dev/null +++ b/docs/audits/stale-pty-inventory/reproduce.mjs @@ -0,0 +1,152 @@ +import { createHash } from 'node:crypto' +import { mkdtemp, readFile, rm, writeFile } from 'node:fs/promises' +import { createRequire } from 'node:module' +import { tmpdir } from 'node:os' +import { join, resolve } from 'node:path' +import { fileURLToPath, pathToFileURL } from 'node:url' +import { applyPatch, parsePatch, reversePatch } from 'diff' +import { build } from 'esbuild' + +if (process.env.ORCA_BACKGROUND_LAUNCH !== '1') { + throw new Error('Run with ORCA_BACKGROUND_LAUNCH=1.') +} + +const root = fileURLToPath(new URL('../../../', import.meta.url)) +const patch = await readFile(new URL('./fix.patch', import.meta.url), 'utf8') +const beforeSources = {} +const sourceHashes = {} +for (const parsed of parsePatch(patch)) { + const path = parsed.newFileName.replace(/^b\//, '') + const absolute = resolve(root, path) + const current = await readFile(absolute, 'utf8') + const before = applyPatch(current, reversePatch(parsed)) + if (before === false) { + throw new Error(`Source changed; review the proof patch: ${path}`) + } + beforeSources[absolute.replaceAll('\\', '/')] = before + sourceHashes[path] = { + before: createHash('sha256').update(before).digest('hex'), + after: createHash('sha256').update(current).digest('hex') + } +} + +for (const path of [ + 'src/main/runtime/pty-inventory-lifecycle-admission.test.ts', + 'src/main/runtime/pty-inventory-lifecycle-scope.test.ts', + 'src/main/runtime/pty-inventory-lifecycle-fixture.ts' +]) { + sourceHashes[path] = { + current: createHash('sha256') + .update(await readFile(resolve(root, path))) + .digest('hex') + } +} + +const scratch = await mkdtemp(join(tmpdir(), 'orca-stale-pty-inventory-')) +const require = createRequire(import.meta.url) +let runnerModuleId +try { + const runnerPath = join(scratch, 'run-process.cjs') + await build({ + absWorkingDir: root, + entryPoints: [resolve(root, 'src/shared/child-process/run-process.ts')], + outfile: runnerPath, + bundle: true, + platform: 'node', + format: 'cjs', + logLevel: 'silent' + }) + runnerModuleId = require.resolve(runnerPath) + const { runProcess } = require(runnerModuleId) + const baselineConfig = join(scratch, 'before.config.mjs') + const fixedConfig = join(scratch, 'after.config.mjs') + const includes = [ + 'src/main/runtime/pty-inventory-lifecycle-admission.test.ts', + 'src/main/runtime/pty-inventory-lifecycle-scope.test.ts' + ] + const configImport = JSON.stringify(pathToFileURL(resolve(root, 'config/vitest.config.ts')).href) + await writeFile( + baselineConfig, + `import base from ${configImport}; +const beforeSources = ${JSON.stringify(beforeSources)}; +export default {...base, test: {...base.test, include: ${JSON.stringify(includes)}}, plugins: [{ + name: 'stale-pty-inventory-before-fix', enforce: 'pre', + transform(_code, id) { + const before = beforeSources[id.replaceAll('\\\\', '/').split('?')[0]]; + return before === undefined ? null : {code: before, map: null}; + } +}]};\n` + ) + + await writeFile( + fixedConfig, + `import base from ${configImport};\nexport default {...base, test: {...base.test, include: ${JSON.stringify(includes)}}};\n` + ) + + async function run(label, config) { + const report = join(scratch, `${label}.json`) + const result = await runProcess({ + program: process.execPath, + args: [ + resolve(root, 'node_modules/vitest/vitest.mjs'), + 'run', + '--config', + config, + '--reporter=json', + `--outputFile=${report}` + ], + cwd: root, + env: process.env, + timeoutMs: 90_000, + maxOutputBytes: 4 * 1024 * 1024 + }) + let parsed + try { + parsed = JSON.parse(await readFile(report, 'utf8')) + } catch (error) { + throw new Error(`${label} runner failed: ${result.stderr || result.stdout}`, { cause: error }) + } + return { + exitCode: result.code, + passed: parsed.numPassedTests, + failed: parsed.numFailedTests, + failedCases: parsed.testResults.flatMap((suite) => + suite.assertionResults + .filter((test) => test.status === 'failed') + .map((test) => test.fullName) + ) + } + } + + const before = await run('before', baselineConfig) + const after = await run('after', fixedConfig) + const passed = + before.exitCode === 1 && + before.failed === 16 && + before.passed === 6 && + after.exitCode === 0 && + after.passed === 22 && + after.failed === 0 + console.log( + JSON.stringify( + { + comparison: + 'Actual runtime inventory, spawn, registration, and exit methods with controlled provider responses; before reverses only fix.patch in a temporary Vite transform', + sourceHashes, + before, + after, + passed + }, + null, + 2 + ) + ) + if (!passed) { + process.exitCode = 1 + } +} finally { + if (runnerModuleId) { + delete require.cache[runnerModuleId] + } + await rm(scratch, { recursive: true, force: true }) +} diff --git a/docs/audits/stale-pty-inventory/results.json b/docs/audits/stale-pty-inventory/results.json new file mode 100644 index 00000000000..fa200ec20ee --- /dev/null +++ b/docs/audits/stale-pty-inventory/results.json @@ -0,0 +1,60 @@ +{ + "comparison": "Actual runtime inventory, spawn, registration, and exit methods with controlled provider responses; before reverses only fix.patch in a temporary Vite transform", + "sourceHashes": { + "src/main/runtime/orca-runtime-invalidate-all-handles-for-pty.ts": { + "before": "5ee6ba0ae3407207ea7f894a83e3325851f8cd6b4b7b8b5adf0be0a6def7d244", + "after": "0eeaf4f89324aea51c49fd4149d632635dbf327d5d91b251e65488bb3852e3ce" + }, + "src/main/runtime/orca-runtime-on-pty-exit.ts": { + "before": "a6512283ac5ee9c45db21a96d1eb228d2c859f1fd0df16300a377fbc126a2557", + "after": "1750058230590062eb0e6488faa39c4def121008d9a7fbb533b98e7d4e31f209" + }, + "src/main/runtime/orca-runtime-refresh-pty-worktree-records-with-controller-inventory.ts": { + "before": "e64d819f6a3694168eca4ab99ce5d01d82f57ea8cb226b0aebe78201f74841ab", + "after": "d0c8228127079e4a87ef5b6cc8e73982ee15eef34f19b0c59a7e224bde2f7fcd" + }, + "src/main/runtime/orca-runtime-register-pty.ts": { + "before": "1f10fb6b9ceefb192e1fde77056897d0b913e6ea21be7b30f60473c1876dc178", + "after": "8855f7ea968e9a759377d4bc3a34c184398b9274d5793e7a1a13a61025c7cbcb" + }, + "src/main/runtime/pty-inventory-lifecycle-admission.test.ts": { + "current": "b00527c138453edf1cb6281b4f6a6fc8dede4de8bcfb39d3b82826125f89721d" + }, + "src/main/runtime/pty-inventory-lifecycle-scope.test.ts": { + "current": "76a519ce504849cf341d7b8472c0d7d5f38b79e67a5302692b65d41b5b1364d9" + }, + "src/main/runtime/pty-inventory-lifecycle-fixture.ts": { + "current": "27958421bac5c9e6ef4dea643774ff1950b77165620bfd565a89c498ddc85f3f" + } + }, + "before": { + "exitCode": 1, + "passed": 6, + "failed": 16, + "failedCases": [ + "inventory admission after a PTY lifecycle change rejects a captured predecessor row after exit before it can adopt handles", + "inventory admission after a PTY lifecycle change rejects a captured predecessor row after registered-successor before it can adopt handles", + "inventory admission after a PTY lifecycle change rejects a captured predecessor row after spawned-successor before it can adopt handles", + "inventory admission after a PTY lifecycle change rejects a captured predecessor row after unknown-start-spawn before it can adopt handles", + "inventory admission after a PTY lifecycle change does not apply stale absence to unknown-at-start spawn", + "inventory admission after a PTY lifecycle change does not apply stale absence to unknown-at-start register", + "inventory admission after a PTY lifecycle change fences a same-ID legacy registration with no incarnation", + "lifecycle invalidation respects inventory scope and retry bounds conservatively rejects pending inventory for foreign lifecycle ID remote:unscoped-handle", + "lifecycle invalidation respects inventory scope and retry bounds conservatively rejects pending inventory for foreign lifecycle ID remote:paired-host@@term_guest", + "lifecycle invalidation respects inventory scope and retry bounds uses explicit host ownership for a legacy unqualified PTY ID on host-a", + "lifecycle invalidation respects inventory scope and retry bounds uses explicit host ownership for a legacy unqualified PTY ID on deploy@10.0.0.4:2222", + "lifecycle invalidation respects inventory scope and retry bounds uses explicit host ownership for a legacy unqualified PTY ID on ssh target", + "lifecycle invalidation respects inventory scope and retry bounds rejects stale inventory for an encoding-sensitive qualified PTY ID on deploy@10.0.0.4:2222", + "lifecycle invalidation respects inventory scope and retry bounds rejects stale inventory for an encoding-sensitive qualified PTY ID on ssh target", + "lifecycle invalidation respects inventory scope and retry bounds allows only one target retry and keeps a second invalidation unknown", + "lifecycle invalidation respects inventory scope and retry bounds retries with the original remaining deadline and accepts fresh truth" + ] + }, + "after": { + "exitCode": 0, + "passed": 22, + "failed": 0, + "failedCases": [] + }, + "passed": true +} diff --git a/src/main/runtime/orca-runtime-invalidate-all-handles-for-pty.ts b/src/main/runtime/orca-runtime-invalidate-all-handles-for-pty.ts index 448fb2a00fb..9d6e5d56ca4 100644 --- a/src/main/runtime/orca-runtime-invalidate-all-handles-for-pty.ts +++ b/src/main/runtime/orca-runtime-invalidate-all-handles-for-pty.ts @@ -1,6 +1,12 @@ // @ts-nocheck -- mechanically split from OrcaRuntimeService; behavior is covered by AST equivalence and characterization tests. import { OrcaRuntimeWithResolveKnownWorkspaceFileTarget } from './orca-runtime-resolve-known-workspace-file-target' import type { PtyIncarnationId } from '../../shared/pty-incarnation' +import { getPtyExecutionHost } from '../../shared/terminal-execution-host' +import { + LOCAL_EXECUTION_HOST_ID, + parseExecutionHostId, + toSshExecutionHostId +} from '../../shared/execution-host' export class OrcaRuntimeWithInvalidateAllHandlesForPty extends OrcaRuntimeWithResolveKnownWorkspaceFileTarget { protected invalidateAllHandlesForPty(ptyId: string, preserveHandle?: string): Set { @@ -110,11 +116,30 @@ export class OrcaRuntimeWithInvalidateAllHandlesForPty extends OrcaRuntimeWithRe return false } + protected invalidatePtyControllerInventoryForLifecycle( + ptyId: string, + connectionId?: string | null + ): void { + const generation = ++this.ptyControllerInventorySequence + const hostId = getPtyExecutionHost(ptyId) + if (hostId === 'foreign' || (hostId && parseExecutionHostId(hostId)?.kind === 'runtime')) { + this.ptyControllerAggregateInventoryGeneration = generation + return + } + const connection = + connectionId === undefined ? this.ptysById.get(ptyId)?.connectionId : connectionId + const providerKey = + hostId ?? (connection ? toSshExecutionHostId(connection) : LOCAL_EXECUTION_HOST_ID) + // A pending census predates this admission or exit, including legacy IDs without an incarnation. + this.ptyControllerInventoryGenerationByProvider.set(providerKey, generation) + } + onPtySpawned( ptyId: string, incarnationId?: PtyIncarnationId, options: { awaitsRegistration?: boolean } = {} ): void { + this.invalidatePtyControllerInventoryForLifecycle(ptyId) const existingPty = this.ptysById.get(ptyId) if ( existingPty && diff --git a/src/main/runtime/orca-runtime-on-pty-exit.ts b/src/main/runtime/orca-runtime-on-pty-exit.ts index 33e1dfc5078..659ee439294 100644 --- a/src/main/runtime/orca-runtime-on-pty-exit.ts +++ b/src/main/runtime/orca-runtime-on-pty-exit.ts @@ -30,6 +30,7 @@ export class OrcaRuntimeWithOnPtyExit extends OrcaRuntimeWithOnClientDisconnecte if (exitIncarnationId && pty?.incarnationId && exitIncarnationId !== pty.incarnationId) { return } + this.invalidatePtyControllerInventoryForLifecycle(ptyId, pty?.connectionId) // A bare exit code is not enough to establish why a process ended: older // daemons and SSH relays can report 0 for crashes and wrapper exits. const observedCause = options.cause ?? resolveUnreportedExitCause(exitCode) diff --git a/src/main/runtime/orca-runtime-refresh-pty-worktree-records-with-controller-inventory.ts b/src/main/runtime/orca-runtime-refresh-pty-worktree-records-with-controller-inventory.ts index aca67f67ba2..930907da120 100644 --- a/src/main/runtime/orca-runtime-refresh-pty-worktree-records-with-controller-inventory.ts +++ b/src/main/runtime/orca-runtime-refresh-pty-worktree-records-with-controller-inventory.ts @@ -55,7 +55,10 @@ export class OrcaRuntimeWithRefreshPtyWorktreeRecordsWithControllerInventory ext } const inventoryGeneration = this.ptyControllerInventorySequence + 1 this.ptyControllerInventorySequence = inventoryGeneration - const providerKey = typeof connectionId === 'string' ? `ssh:${connectionId}` : 'local' + const providerKey = + typeof connectionId === 'string' + ? toSshExecutionHostId(connectionId) + : LOCAL_EXECUTION_HOST_ID const livenessObservationAtStart = this.ptyLivenessObservationSequence if (connectionId === undefined) { this.ptyControllerAggregateInventoryGeneration = inventoryGeneration diff --git a/src/main/runtime/orca-runtime-register-pty.ts b/src/main/runtime/orca-runtime-register-pty.ts index b7afec6f2f6..37a5434e5c5 100644 --- a/src/main/runtime/orca-runtime-register-pty.ts +++ b/src/main/runtime/orca-runtime-register-pty.ts @@ -27,6 +27,7 @@ export class OrcaRuntimeWithRegisterPty extends OrcaRuntimeWithInvalidateAllHand isWsl?: boolean ): void { this.assertPtyDidNotExitBeforeRegistration(ptyId, binding?.incarnationId) + this.invalidatePtyControllerInventoryForLifecycle(ptyId, connectionId) const existingPty = this.ptysById.get(ptyId) const replacementHandle = binding?.terminalHandle?.trim() const pendingReplacement = this.pendingPtyHandleReplacementFences.get(ptyId) diff --git a/src/main/runtime/pty-inventory-lifecycle-admission.test.ts b/src/main/runtime/pty-inventory-lifecycle-admission.test.ts new file mode 100644 index 00000000000..48cfc5fa8c7 --- /dev/null +++ b/src/main/runtime/pty-inventory-lifecycle-admission.test.ts @@ -0,0 +1,100 @@ +import { afterEach, describe, expect, it, vi } from 'vitest' +import type { PtyProcessInfo } from '../providers/pty-process-info' +import { + createInventoryRuntime, + deferred, + processRow, + PREDECESSOR, + PTY, + SUCCESSOR, + WORKTREE +} from './pty-inventory-lifecycle-fixture' + +afterEach(() => vi.restoreAllMocks()) + +describe('inventory admission after a PTY lifecycle change', () => { + it.each(['exit', 'registered-successor', 'spawned-successor', 'unknown-start-spawn'] as const)( + 'rejects a captured predecessor row after %s before it can adopt handles', + async (mode) => { + const reply = deferred() + const { runtime, hasPty } = createInventoryRuntime(() => reply.promise) + if (mode !== 'unknown-start-spawn') { + runtime.register() + } + const pending = runtime.read() + if (mode !== 'unknown-start-spawn') { + runtime.onPtyExit(PTY, 0, PREDECESSOR, { providerExitObserved: true }) + } + if (mode === 'registered-successor') { + runtime.register(SUCCESSOR) + } + if (mode === 'spawned-successor' || mode === 'unknown-start-spawn') { + runtime.onPtySpawned(PTY, SUCCESSOR) + } + const current = runtime.capture() + reply.resolve([processRow()]) + expect(await pending).toBeNull() + expect(runtime.capture()).toEqual(current) + expect(hasPty).not.toHaveBeenCalled() + } + ) + + it.each(['spawn', 'register'] as const)( + 'does not apply stale absence to unknown-at-start %s', + async (kind) => { + const reply = deferred() + const { runtime, hasPty } = createInventoryRuntime(() => reply.promise) + const pending = runtime.read() + if (kind === 'spawn') { + runtime.onPtySpawned(PTY, SUCCESSOR) + } else { + runtime.register(SUCCESSOR) + } + reply.resolve([]) + expect(await pending).toBeNull() + expect(runtime.capture()).toMatchObject({ connected: true, incarnationId: SUCCESSOR }) + expect(hasPty).not.toHaveBeenCalled() + } + ) + + it('fences a same-ID legacy registration with no incarnation', async () => { + const reply = deferred() + const { runtime } = createInventoryRuntime(() => reply.promise) + runtime.registerPty(PTY, WORKTREE) + const pending = runtime.read() + runtime.onPtyExit(PTY, 0, undefined, { providerExitObserved: true }) + runtime.registerPty(PTY, WORKTREE) + reply.resolve([{ id: PTY, cwd: '', title: 'stale legacy title', worktreeId: WORKTREE }]) + expect(await pending).toBeNull() + expect(runtime.capture()).toMatchObject({ + connected: true, + incarnationId: null, + controllerTitle: null + }) + }) + + it('does not let an ignored predecessor exit invalidate a current inventory', async () => { + const reply = deferred() + const { runtime } = createInventoryRuntime(() => reply.promise) + runtime.register(SUCCESSOR) + const pending = runtime.read(null) + runtime.onPtyExit(PTY, 0, PREDECESSOR, { providerExitObserved: true }) + reply.resolve([processRow(PTY, SUCCESSOR)]) + expect((await pending)?.allLivePtyIds).toEqual(new Set([PTY])) + expect(runtime.capture().incarnationId).toBe(SUCCESSOR) + }) + + it('accepts a genuinely new row and a replacement first observed by fresh inventory', async () => { + let sessions = [processRow()] + const { runtime } = createInventoryRuntime(async () => sessions) + expect((await runtime.read())?.allLivePtyIds).toEqual(new Set([PTY])) + expect(runtime.capture().incarnationId).toBe(PREDECESSOR) + sessions = [processRow(PTY, SUCCESSOR)] + expect((await runtime.read())?.allLivePtyIds).toEqual(new Set([PTY])) + expect(runtime.capture()).toMatchObject({ + incarnationId: SUCCESSOR, + handle: `term_${SUCCESSOR}`, + connected: true + }) + }) +}) diff --git a/src/main/runtime/pty-inventory-lifecycle-fixture.ts b/src/main/runtime/pty-inventory-lifecycle-fixture.ts new file mode 100644 index 00000000000..a163de13aa8 --- /dev/null +++ b/src/main/runtime/pty-inventory-lifecycle-fixture.ts @@ -0,0 +1,75 @@ +import { vi } from 'vitest' +import { OrcaRuntimeService } from './orca-runtime' +import type { RuntimePtyController } from './runtime-pty-controller-contract' +import type { PtyProcessInfo } from '../providers/pty-process-info' + +export const WORKTREE = 'repo::/tmp/inventory-admission' +export const PTY = `${WORKTREE}@@shell` +export const PREDECESSOR = '30000000-0000-4000-8000-000000000001' +export const SUCCESSOR = '30000000-0000-4000-8000-000000000002' +const TAB = '30000000-0000-4000-8000-000000000003' +const LEAF = '30000000-0000-4000-8000-000000000004' + +export type InventoryListing = NonNullable + +export function deferred() { + let resolve!: (value: T) => void + const promise = new Promise((done) => { + resolve = done + }) + return { promise, resolve } +} + +export function processRow(id = PTY, incarnationId = PREDECESSOR): PtyProcessInfo { + return { + id, + cwd: '', + title: 'captured listing', + worktreeId: WORKTREE, + incarnationId, + terminalHandle: `term_${incarnationId}` + } +} + +export class InventoryLifecycleRuntime extends OrcaRuntimeService { + read(connectionId?: string | null, target: string | null = null, deadline?: number) { + return this.refreshPtyWorktreeRecordsWithControllerInventory([], target, deadline, connectionId) + } + + register(incarnationId = PREDECESSOR): void { + this.registerPreAllocatedHandleForPty(PTY, `term_${incarnationId}`) + this.registerPty(PTY, WORKTREE, null, { + tabId: TAB, + leafId: LEAF, + incarnationId, + terminalHandle: `term_${incarnationId}` + }) + } + + capture(id = PTY) { + const pty = this.ptysById.get(id) + return { + connected: pty?.connected, + incarnationId: pty?.incarnationId, + controllerTitle: pty?.controllerTitle, + tabId: pty?.tabId, + paneKey: pty?.paneKey, + handle: this.handleByPtyId.get(id) ?? null, + verdict: this.getPtyLivenessVerdict(id), + headless: this.headlessTerminals.has(id) + } + } +} + +export function createInventoryRuntime(listProcesses: InventoryListing) { + const runtime = new InventoryLifecycleRuntime() + const hasPty = vi.fn(() => false) + runtime.setPtyController({ + write: () => true, + kill: () => true, + getForegroundProcess: async () => null, + listProcesses, + hasPty + }) + return { runtime, hasPty } +} diff --git a/src/main/runtime/pty-inventory-lifecycle-scope.test.ts b/src/main/runtime/pty-inventory-lifecycle-scope.test.ts new file mode 100644 index 00000000000..0309ac7631d --- /dev/null +++ b/src/main/runtime/pty-inventory-lifecycle-scope.test.ts @@ -0,0 +1,152 @@ +import { afterEach, describe, expect, it, vi } from 'vitest' +import type { PtyProcessInfo } from '../providers/pty-process-info' +import { + createInventoryRuntime, + deferred, + processRow, + PREDECESSOR, + PTY, + SUCCESSOR, + WORKTREE, + type InventoryListing +} from './pty-inventory-lifecycle-fixture' + +afterEach(() => vi.restoreAllMocks()) + +describe('lifecycle invalidation respects inventory scope and retry bounds', () => { + it.each(['remote:unscoped-handle', 'remote:paired-host@@term_guest'])( + 'conservatively rejects pending inventory for foreign lifecycle ID %s', + async (id) => { + const reply = deferred() + const list = vi.fn(() => reply.promise) + const { runtime, hasPty } = createInventoryRuntime(list) + const pending = runtime.read('host-b') + runtime.registerPty(id, WORKTREE) + reply.resolve([processRow('ssh:host-b@@child')]) + expect(await pending).toBeNull() + expect(list).toHaveBeenCalledTimes(1) + expect(list.mock.calls[0][0]).toBe('host-b') + expect(hasPty).not.toHaveBeenCalled() + expect(runtime.capture(id).connected).toBe(true) + } + ) + + it('preserves a targeted other-host response', async () => { + const reply = deferred() + const { runtime } = createInventoryRuntime(() => reply.promise) + const pending = runtime.read('host-b') + runtime.onPtySpawned('ssh:host-a@@child', SUCCESSOR) + reply.resolve([processRow('ssh:host-b@@child')]) + expect((await pending)?.allLivePtyIds).toEqual(new Set(['ssh:host-b@@child'])) + expect(runtime.capture('ssh:host-b@@child')).toMatchObject({ + connected: true, + incarnationId: PREDECESSOR + }) + }) + + it.each(['host-a', 'deploy@10.0.0.4:2222', 'ssh target'])( + 'uses explicit host ownership for a legacy unqualified PTY ID on %s', + async (connectionId) => { + const reply = deferred() + const { runtime } = createInventoryRuntime(() => reply.promise) + const pending = runtime.read(connectionId) + runtime.registerPty('legacy-child', WORKTREE, connectionId) + reply.resolve([processRow('legacy-child')]) + expect(await pending).toBeNull() + expect(runtime.capture('legacy-child').connected).toBe(true) + } + ) + + it.each(['deploy@10.0.0.4:2222', 'ssh target'])( + 'rejects stale inventory for an encoding-sensitive qualified PTY ID on %s', + async (connectionId) => { + const reply = deferred() + const { runtime } = createInventoryRuntime(() => reply.promise) + const ptyId = `ssh:${connectionId}@@child` + runtime.registerPty(ptyId, WORKTREE, connectionId) + const pending = runtime.read(connectionId) + runtime.onPtySpawned(ptyId, SUCCESSOR) + reply.resolve([processRow(ptyId)]) + expect(await pending).toBeNull() + expect(runtime.capture(ptyId)).toMatchObject({ + connected: true, + incarnationId: SUCCESSOR + }) + } + ) + + it('does not route an unqualified local admission into an SSH query', async () => { + const reply = deferred() + const { runtime } = createInventoryRuntime(() => reply.promise) + const pending = runtime.read('host-a') + runtime.register() + reply.resolve([processRow('ssh:host-a@@child')]) + expect((await pending)?.allLivePtyIds).toEqual(new Set(['ssh:host-a@@child'])) + }) + + it('keeps concurrent targeted provider generations independent', async () => { + const a = deferred() + const b = deferred() + const { runtime } = createInventoryRuntime((connection) => + connection === 'a' ? a.promise : b.promise + ) + const pa = runtime.read('a') + const pb = runtime.read('b') + b.resolve([processRow('ssh:b@@child')]) + expect((await pb)?.allLivePtyIds).toEqual(new Set(['ssh:b@@child'])) + a.resolve([processRow('ssh:a@@child')]) + expect((await pa)?.allLivePtyIds).toEqual(new Set(['ssh:a@@child'])) + }) + + it('retains ordering between an aggregate and newer targeted inventory', async () => { + const aggregate = deferred() + const targeted = deferred() + const { runtime } = createInventoryRuntime((connection) => + connection === undefined ? aggregate.promise : targeted.promise + ) + const pa = runtime.read() + const pt = runtime.read('a') + targeted.resolve([processRow('ssh:a@@child', SUCCESSOR)]) + expect(await pt).not.toBeNull() + aggregate.resolve([processRow('ssh:a@@child', PREDECESSOR)]) + expect(await pa).toBeNull() + expect(runtime.capture('ssh:a@@child').incarnationId).toBe(SUCCESSOR) + }) + + it('allows only one target retry and keeps a second invalidation unknown', async () => { + const first = deferred() + const second = deferred() + const list = vi + .fn() + .mockReturnValueOnce(first.promise) + .mockReturnValueOnce(second.promise) + const { runtime } = createInventoryRuntime(list) + runtime.register() + const pending = runtime.read(null, WORKTREE, Date.now() + 4000) + runtime.onPtyExit(PTY, 0, PREDECESSOR, { providerExitObserved: true }) + first.resolve([processRow()]) + await vi.waitFor(() => expect(list).toHaveBeenCalledTimes(2)) + runtime.onPtySpawned(PTY, SUCCESSOR) + second.resolve([]) + expect(await pending).toBeNull() + expect(list).toHaveBeenCalledTimes(2) + expect(runtime.capture()).toMatchObject({ connected: true, incarnationId: SUCCESSOR }) + }) + + it('retries with the original remaining deadline and accepts fresh truth', async () => { + const first = deferred() + const list = vi.fn().mockReturnValueOnce(first.promise).mockResolvedValue([]) + const { runtime } = createInventoryRuntime(list) + let now = 1000 + vi.spyOn(Date, 'now').mockImplementation(() => now) + runtime.register() + const pending = runtime.read(null, WORKTREE, 1500) + runtime.onPtyExit(PTY, 0, PREDECESSOR, { providerExitObserved: true }) + now = 1400 + first.resolve([processRow()]) + expect((await pending)?.allLivePtyIds).toEqual(new Set()) + expect(list).toHaveBeenCalledTimes(2) + expect(list.mock.calls[1][1]?.deadlineMs).toBeLessThanOrEqual(1500) + expect(runtime.capture().connected).toBe(false) + }) +})