From cc40ca287bd193bb59cc8821d7551cf44af3471d Mon Sep 17 00:00:00 2001 From: Neil Date: Sat, 12 Sep 2026 00:50:56 -0700 Subject: [PATCH] refactor(orcad): simplify lifecycle and transfer validation --- config/reliability-gates.jsonc | 2 + src/main/orcad/orcad-decommission.test.ts | 11 +- src/main/orcad/orcad-decommission.ts | 39 ++---- src/main/orcad/orcad-delegated-connection.ts | 22 ++- .../orcad-delegated-provider-adapters.ts | 14 +- .../orcad-delegated-recovery-lifecycle.ts | 4 +- ...nsfer-destination-output-admission.test.ts | 23 ++++ ...p-transfer-destination-output-admission.ts | 28 +--- ...ership-transfer-destination-output-sink.ts | 17 +-- ...rship-transfer-destination-runtime.test.ts | 10 +- ...nership-transfer-recovered-source-route.ts | 21 +-- .../pty-ownership-transfer-recovery.ts | 30 +---- ...nership-transfer-response-identity.test.ts | 29 ++++ ...ty-ownership-transfer-response-identity.ts | 10 +- ...me-pty-ownership-transfer-output-credit.ts | 4 - ...me-pty-ownership-transfer-source-grants.ts | 5 - src/main/ssh/orcad-incumbent-recovery.ts | 15 +-- src/main/ssh/orcad-installed-activation.ts | 16 +-- src/main/ssh/orcad-remote-rollback.test.ts | 126 +++++++++++++----- src/main/ssh/orcad-remote-runtime-control.ts | 55 ++++---- src/main/ssh/orcad-rollback-recovery.ts | 17 +-- src/main/ssh/orcad-rollback-transition.ts | 20 ++- ...hip-transfer-destination-adapter-replay.ts | 4 +- ...transfer-destination-adapter-validation.ts | 39 ++---- ...rship-transfer-destination-adapter.test.ts | 36 ++++- .../pty-ownership-transfer-journal.test.ts | 28 +++- src/shared/pty-ownership-transfer-journal.ts | 10 +- 27 files changed, 318 insertions(+), 317 deletions(-) create mode 100644 src/main/persistence/pty-ownership-transfer/pty-ownership-transfer-response-identity.test.ts diff --git a/config/reliability-gates.jsonc b/config/reliability-gates.jsonc index 888b013f510..a6285d9ac81 100644 --- a/config/reliability-gates.jsonc +++ b/config/reliability-gates.jsonc @@ -20344,6 +20344,7 @@ "oracle": "Populate one SSH target with unrelated and target-owned repositories, a folder workspace, inline and stored scrollback, eligible and cross-target sleeping-agent records, disabled and enabled automations, final and in-flight runs, mobile selections, UI routing, detached and terminated PTY leases, and a saved forward. Require preflight to retain exact identities and source fencing to reject every non-final lease, main-owned recovery, unsafe sleeping record, enabled automation, in-flight or cross-host run, and unsupported client-state projection. Export only the target catalog plus referenced group ancestry, bounded content-addressed scrollback descriptors, eligible destination-local sleeping-agent resume state, eligible settled automation history, representable client state, and the exact saved-forward intent, then hash it deterministically. Require source fence plus journal to survive reload before any remote mutation; stage to publish nothing; resume chunk upload from durable exact byte length; reconcile a lost chunk response; reject source drift, noncanonical bytes, bad offsets, incomplete digests, duplicate refs, duplicate forward ports, and destination conflicts; commit bytes, references, dormant state, client state, and exact receipt; preserve the saved forward on the fenced source target without opening a destination listener; and require malformed, stripped-extension, tampered, conflicting, or unpaired requests to fail closed. Census every session, PTY-recovery, worktree, lineage, preset, retired-name, automation, run, mobile-selection, UI-routing, lease, and forward owner for the exact host while excluding another target. Lose stage, commit, and abort responses and reconcile exact absent, staged, or committed status without deleting source rows or bytes. Require old-host method-not-found to release only an untouched source-fenced transaction, committed abort to refuse, catalog, snapshot, sleeping-agent, automation, client-state, or dependency drift to retain the fence, and every failed profile or destination flush to remain retryable without early acknowledgement. After exact commit evidence, retire only manifest rows and unshared snapshot files owned by the source host, preserve same-ID rows on another host, shared snapshot files, saved forwards, and shared group ancestry, fail closed if source rows reappear, and persist retirement idempotently across restart.", "commands": [ "node config/scripts/run-orcad-managed-environment-cross-version.mjs config/scripts/orcad-managed-environment-cross-version.test.ts", + "pnpm exec vitest run --config config/vitest.config.ts src/main/persistence/pty-ownership-transfer/pty-ownership-transfer-response-identity.test.ts", "ORCA_BACKGROUND_LAUNCH=1 ORCA_TEST_MOCK_KEYCHAIN=1 SKIP_BUILD=1 ORCA_E2E_SSH_DOCKER=1 ORCA_E2E_SSH_DOCKER_IMAGE=orca-orcad-desktop-migration:e2e ORCA_REVIEW_ORCAD_DESKTOP_MIGRATION=1 ORCA_REVIEW_ORCAD_TARGET=linux-arm64-glibc pnpm exec playwright test tests/e2e/orcad-live-catalog-desktop-migration.spec.ts --config tests/playwright.config.ts --project electron-headless --workers=1 --repeat-each=3 --trace=on", "pnpm exec vitest run --config config/vitest.config.ts src/main/ssh/orcad-live-migration-renderer-plan.test.ts src/main/ssh/orcad-live-migration-renderer-plan-evidence.test.ts src/renderer/src/runtime/orcad-migration-renderer-refresh.test.ts src/renderer/src/runtime/orcad-migration-mirror-cohort.test.ts src/main/runtime/runtime-mobile-session-incarnation-projection.test.ts src/main/runtime/session-tabs-inventory-publication.test.ts", "pnpm run smoke:orcad-ssh-ui -- tests/e2e/orcad-managed-ssh-lifecycle.spec.ts", @@ -20395,6 +20396,7 @@ "src/main/persistence-orcad-migration-sleeping-agent.test.ts", "src/main/persistence/pty-ownership-transfer/pty-ownership-transfer-destination-output-admission.test.ts", "src/main/persistence/pty-ownership-transfer/pty-ownership-transfer-recovery.test.ts", + "src/main/persistence/pty-ownership-transfer/pty-ownership-transfer-response-identity.test.ts", "src/main/persistence/pty-ownership-transfer/pty-ownership-transfer-coordinator.test.ts", "src/main/persistence/pty-ownership-transfer/pty-ownership-transfer-destination-runtime.test.ts", "src/main/runtime/paired-runtime-pty-ownership-transfer-client.test.ts" diff --git a/src/main/orcad/orcad-decommission.test.ts b/src/main/orcad/orcad-decommission.test.ts index da1ce33be5b..33eb76b5b01 100644 --- a/src/main/orcad/orcad-decommission.test.ts +++ b/src/main/orcad/orcad-decommission.test.ts @@ -62,9 +62,16 @@ describe('orcad decommission adapter', () => { transactionId, '0.2.0+new', undefined, - 'validated-transaction' + 'validated-transaction', + undefined + ) + expect(validateTransaction).toHaveBeenCalledWith( + transactionId, + '0.2.0+new', + undefined, + undefined, + undefined ) - expect(validateTransaction).toHaveBeenCalledWith(transactionId, '0.2.0+new') expect(validateTransaction.mock.invocationCallOrder[0]).toBeLessThan( adapter.mock.invocationCallOrder[0] ) diff --git a/src/main/orcad/orcad-decommission.ts b/src/main/orcad/orcad-decommission.ts index b8ade33f772..886b88a3156 100644 --- a/src/main/orcad/orcad-decommission.ts +++ b/src/main/orcad/orcad-decommission.ts @@ -215,15 +215,13 @@ async function executeExclusiveOrcadDecommission( let transactionSnapshot: string | undefined if (transactionId) { try { - const validated = authority - ? validateOrcadDecommissionTransaction( - transactionId, - expectedVersion, - undefined, - undefined, - authority - ) - : validateOrcadDecommissionTransaction(transactionId, expectedVersion) + const validated = validateOrcadDecommissionTransaction( + transactionId, + expectedVersion, + undefined, + undefined, + authority + ) if ( authority && (!runningInstance || @@ -259,22 +257,13 @@ async function executeExclusiveOrcadDecommission( return result } try { - if (authority) { - persistOrcadDecommissionAcceptance( - transactionId, - expectedVersion, - undefined, - transactionSnapshot, - authority - ) - } else { - persistOrcadDecommissionAcceptance( - transactionId, - expectedVersion, - undefined, - transactionSnapshot - ) - } + persistOrcadDecommissionAcceptance( + transactionId, + expectedVersion, + undefined, + transactionSnapshot, + authority + ) return { outcome: 'accepted', transactionId } } catch (error) { return { diff --git a/src/main/orcad/orcad-delegated-connection.ts b/src/main/orcad/orcad-delegated-connection.ts index f75f92cb046..da4175757ee 100644 --- a/src/main/orcad/orcad-delegated-connection.ts +++ b/src/main/orcad/orcad-delegated-connection.ts @@ -12,10 +12,7 @@ import { OrcadDelegatedTransferClient } from './orcad-delegated-transfer-client' import { reconcileOrcadInitialModelBaseline } from './orcad-delegated-initial-model-ack' import { installOrcadDelegatedOutputReceiver } from './orcad-delegated-output-receiver' import { createOrcadDelegatedPtyOperations } from './orcad-delegated-pty-operations' -import { - createOrcadDelegatedProviderAdapters, - createOrcadDelegatedProviderInput -} from './orcad-delegated-provider-adapters' +import { createOrcadDelegatedProviderAdapters } from './orcad-delegated-provider-adapters' import { createOrcadDelegatedExecutionRefresh } from './orcad-delegated-execution-refresh' import { createOrcadDelegatedExitDelivery } from './orcad-delegated-exit-delivery' import { createOrcadDelegatedExecutionState } from './orcad-delegated-execution-state' @@ -40,7 +37,7 @@ export async function connectOrcadDelegatedTransfer(options: OrcadDelegatedConne let disposed = false let multiplexer: SshChannelMultiplexer | undefined let receiver: ReturnType | undefined - let providerInput: ReturnType | undefined + let providerAdapters: ReturnType | undefined let executionRefresh: ReturnType | undefined let executionState: ReturnType | undefined let finalOutputSeq: number | undefined @@ -59,7 +56,7 @@ export async function connectOrcadDelegatedTransfer(options: OrcadDelegatedConne removeDispose() stopping = Promise.all([ receiver?.dispose(), - providerInput?.whenIdle(), + providerAdapters?.providerInput.whenIdle(), executionRefresh?.dispose() ]).then(() => undefined) executionState?.disconnect() @@ -276,16 +273,15 @@ export async function connectOrcadDelegatedTransfer(options: OrcadDelegatedConne isActive: active, isCommitReconciled: reconciliation.isReconciled }) - providerInput = createOrcadDelegatedProviderInput({ ...options, identity }, operations, active) + providerAdapters = createOrcadDelegatedProviderAdapters( + { ...options, identity }, + operations, + active + ) return { onExit: providerExits.onExit, operations, - ...createOrcadDelegatedProviderAdapters( - { ...options, identity }, - operations, - providerInput, - active - ), + ...providerAdapters, client, multiplexer: transport, proof, diff --git a/src/main/orcad/orcad-delegated-provider-adapters.ts b/src/main/orcad/orcad-delegated-provider-adapters.ts index f196f8d9695..8be60f1948d 100644 --- a/src/main/orcad/orcad-delegated-provider-adapters.ts +++ b/src/main/orcad/orcad-delegated-provider-adapters.ts @@ -5,26 +5,18 @@ import { OrcadDelegatedProviderAttachment } from './orcad-delegated-provider-att import { OrcadDelegatedProviderInspection } from './orcad-delegated-provider-inspection' import { OrcadDelegatedProviderControls } from './orcad-delegated-provider-controls' -export function createOrcadDelegatedProviderInput( +export function createOrcadDelegatedProviderAdapters( options: OrcadDelegatedConnectionOptions, operations: ReturnType, isActive: () => boolean ) { - return new OrcadDelegatedProviderInput({ + const providerInput = new OrcadDelegatedProviderInput({ ptyId: options.identity.terminalId, operations, readInputs: () => options.store.input.loadDelegated(options.identity), isActive, onError: options.onError }) -} - -export function createOrcadDelegatedProviderAdapters( - options: OrcadDelegatedConnectionOptions, - operations: ReturnType, - providerInput: OrcadDelegatedProviderInput, - active: () => boolean -) { return { providerInput, providerAttachment: options.providerModel @@ -32,7 +24,7 @@ export function createOrcadDelegatedProviderAdapters( ptyId: options.identity.terminalId, input: providerInput, operations, - isActive: active, + isActive, getSnapshot: options.providerModel.snapshot, getModelSequence: options.providerModel.sequence }) diff --git a/src/main/orcad/orcad-delegated-recovery-lifecycle.ts b/src/main/orcad/orcad-delegated-recovery-lifecycle.ts index 70c4b77b06d..749ec8f54a1 100644 --- a/src/main/orcad/orcad-delegated-recovery-lifecycle.ts +++ b/src/main/orcad/orcad-delegated-recovery-lifecycle.ts @@ -96,7 +96,6 @@ export function installOrcadDelegatedRecovery(options: { return } const identity = Object.freeze({ ...destination.identity }) - let initialized = false let initializing: Promise | undefined const trackedDestination: TrackedDestination = { ...destination, @@ -124,7 +123,7 @@ export function installOrcadDelegatedRecovery(options: { initializeModel: async (signal) => { controller.signal.throwIfAborted() signal.throwIfAborted() - if (initialized) { + if (initializedModels.has(identity.bridgeId)) { return } initializing ??= Promise.resolve() @@ -134,7 +133,6 @@ export function installOrcadDelegatedRecovery(options: { await options.initializeModel(identity, signal) controller.signal.throwIfAborted() signal.throwIfAborted() - initialized = true initializedModels.add(identity.bridgeId) initializationErrors.delete(identity.bridgeId) }) diff --git a/src/main/persistence/pty-ownership-transfer/pty-ownership-transfer-destination-output-admission.test.ts b/src/main/persistence/pty-ownership-transfer/pty-ownership-transfer-destination-output-admission.test.ts index 67ba72c38dd..58500a557ae 100644 --- a/src/main/persistence/pty-ownership-transfer/pty-ownership-transfer-destination-output-admission.test.ts +++ b/src/main/persistence/pty-ownership-transfer/pty-ownership-transfer-destination-output-admission.test.ts @@ -86,4 +86,27 @@ describe('PtyOwnershipTransferDestinationOutputAdmission', () => { 'pty_ownership_transfer_output_admission_backpressure' ) }) + + it.each([1, 2, 3])('rejects only the failed suffix at frame %s', async (failedSeq) => { + const admission = new PtyOwnershipTransferDestinationOutputAdmission() + const pending = [1, 2, 3].map((seq) => admission.defer(identity, { seq, data: String(seq) })) + const failure = new Error('destination-failed') + const accept = vi.fn((frame: { seq: number }) => { + if (frame.seq === failedSeq) { + throw failure + } + }) + + admission.settle(identity, accept) + + expect(accept.mock.calls.map(([frame]) => frame.seq)).toEqual([1, 2, 3].slice(0, failedSeq)) + const settled = await Promise.allSettled(pending) + expect(settled).toEqual( + [1, 2, 3].map((seq) => + seq < failedSeq + ? { status: 'fulfilled', value: undefined } + : { status: 'rejected', reason: failure } + ) + ) + }) }) diff --git a/src/main/persistence/pty-ownership-transfer/pty-ownership-transfer-destination-output-admission.ts b/src/main/persistence/pty-ownership-transfer/pty-ownership-transfer-destination-output-admission.ts index c783cafee5e..6f41a04d098 100644 --- a/src/main/persistence/pty-ownership-transfer/pty-ownership-transfer-destination-output-admission.ts +++ b/src/main/persistence/pty-ownership-transfer/pty-ownership-transfer-destination-output-admission.ts @@ -2,13 +2,13 @@ import type { PtyOwnershipTransferOutputFrame, PtyOwnershipTransferWireIdentity } from '../../../shared/pty-ownership-transfer-wire' +import { samePtyOwnershipTransferIdentity } from '../../../shared/pty-ownership-transfer-identity' import { PTY_OWNERSHIP_TRANSFER_OUTPUT_OUTBOX_MAX_BYTES, PTY_OWNERSHIP_TRANSFER_OUTPUT_OUTBOX_MAX_FRAMES } from './pty-ownership-transfer-destination-output-outbox' type PendingOutput = Readonly<{ - identity: PtyOwnershipTransferWireIdentity frame: PtyOwnershipTransferOutputFrame promise: Promise resolve: () => void @@ -50,7 +50,7 @@ export class PtyOwnershipTransferDestinationOutputAdmission { let queue = this.pending.get(identity.bridgeId) let created = false if (queue) { - if (!sameIdentity(queue.identity, identity)) { + if (!samePtyOwnershipTransferIdentity(queue.identity, identity)) { throw new Error('pty_ownership_transfer_output_admission_identity_conflict') } } else { @@ -85,7 +85,6 @@ export class PtyOwnershipTransferDestinationOutputAdmission { reject = onReject }) const entry: PendingOutput = { - identity: queue.identity, frame: Object.freeze({ ...frame }), promise, resolve, @@ -108,12 +107,12 @@ export class PtyOwnershipTransferDestinationOutputAdmission { if (!queue) { return } - if (!sameIdentity(queue.identity, identity)) { + if (!samePtyOwnershipTransferIdentity(queue.identity, identity)) { throw new Error('pty_ownership_transfer_output_admission_identity_conflict') } this.pending.delete(identity.bridgeId) let failed: Error | undefined - for (const [index, entry] of queue.entries.entries()) { + for (const entry of queue.entries) { if (failed) { entry.reject(failed) continue @@ -124,9 +123,6 @@ export class PtyOwnershipTransferDestinationOutputAdmission { } catch (error) { failed = asError(error) entry.reject(failed) - for (const remaining of queue.entries.slice(index + 1)) { - remaining.reject(failed) - } } } } @@ -136,7 +132,7 @@ export class PtyOwnershipTransferDestinationOutputAdmission { if (!queue) { return } - if (!sameIdentity(queue.identity, identity)) { + if (!samePtyOwnershipTransferIdentity(queue.identity, identity)) { throw new Error('pty_ownership_transfer_output_admission_identity_conflict') } this.pending.delete(identity.bridgeId) @@ -154,20 +150,6 @@ function sameFrame( return left.seq === right.seq && left.data === right.data && left.truncated === right.truncated } -function sameIdentity( - left: PtyOwnershipTransferWireIdentity, - right: PtyOwnershipTransferWireIdentity -): boolean { - return ( - left.bridgeId === right.bridgeId && - left.terminalId === right.terminalId && - left.incarnationId === right.incarnationId && - left.ownerLease === right.ownerLease && - left.sourceOwnerGeneration === right.sourceOwnerGeneration && - left.destinationRuntimeId === right.destinationRuntimeId - ) -} - function asError(error: unknown): Error { return error instanceof Error ? error : new Error(String(error)) } diff --git a/src/main/persistence/pty-ownership-transfer/pty-ownership-transfer-destination-output-sink.ts b/src/main/persistence/pty-ownership-transfer/pty-ownership-transfer-destination-output-sink.ts index 826f37d9840..9c4e650b479 100644 --- a/src/main/persistence/pty-ownership-transfer/pty-ownership-transfer-destination-output-sink.ts +++ b/src/main/persistence/pty-ownership-transfer/pty-ownership-transfer-destination-output-sink.ts @@ -1,6 +1,7 @@ import type { PtyOwnershipTransferOutputFrame } from '../../../shared/pty-ownership-transfer-wire' import type { PtyOwnershipTransferWireIdentity } from '../../../shared/pty-ownership-transfer-wire' import type { PtyOwnershipTransferSurfaceBinding } from '../../../shared/pty-ownership-transfer-surface-binding' +import { samePtyOwnershipTransferIdentity } from '../../../shared/pty-ownership-transfer-identity' import type { PtyOwnershipTransferDestinationOutputOutbox, PtyOwnershipTransferDestinationOutputSnapshot @@ -100,7 +101,7 @@ function assertAcknowledgement( if ( !acknowledgement || !isTransferIdentity(acknowledgement.identity) || - !sameIdentity(acknowledgement.identity, expectedIdentity) || + !samePtyOwnershipTransferIdentity(acknowledgement.identity, expectedIdentity) || !Number.isSafeInteger(acknowledgement.throughSeq) || acknowledgement.throughSeq < minimumSeq ) { @@ -122,17 +123,3 @@ function isTransferIdentity(value: unknown): value is PtyOwnershipTransferWireId typeof record.destinationRuntimeId === 'string' ) } - -function sameIdentity( - left: PtyOwnershipTransferWireIdentity, - right: PtyOwnershipTransferWireIdentity -): boolean { - return ( - left.bridgeId === right.bridgeId && - left.terminalId === right.terminalId && - left.incarnationId === right.incarnationId && - left.ownerLease === right.ownerLease && - left.sourceOwnerGeneration === right.sourceOwnerGeneration && - left.destinationRuntimeId === right.destinationRuntimeId - ) -} diff --git a/src/main/persistence/pty-ownership-transfer/pty-ownership-transfer-destination-runtime.test.ts b/src/main/persistence/pty-ownership-transfer/pty-ownership-transfer-destination-runtime.test.ts index 35f0a19f13b..a353f94e3fa 100644 --- a/src/main/persistence/pty-ownership-transfer/pty-ownership-transfer-destination-runtime.test.ts +++ b/src/main/persistence/pty-ownership-transfer/pty-ownership-transfer-destination-runtime.test.ts @@ -298,7 +298,15 @@ describe('PtyOwnershipTransferDestinationRuntimeRegistry', () => { ...checkpoint, identity: { ...identity, incarnationId: 'stale-incarnation' } }) - ).toThrow('pty_ownership_transfer_model_checkpoint_conflict') + ).toThrow('orcad_terminal_layout_reservation_identity_conflict') + expect(ref && store.readTerminalScrollbackSnapshot(ref)).toBe(checkpoint.modelData) + + store.getWorkspaceSession().terminalLayoutsByTabId[surfaceBinding.tabId].buffersByLeafId = { + [leafId]: 'unrelated local output' + } + expect(() => store.checkpointPtyOwnershipTransferTerminalModel(checkpoint)).toThrow( + 'pty_ownership_transfer_model_checkpoint_conflict' + ) expect(ref && store.readTerminalScrollbackSnapshot(ref)).toBe(checkpoint.modelData) }) diff --git a/src/main/persistence/pty-ownership-transfer/pty-ownership-transfer-recovered-source-route.ts b/src/main/persistence/pty-ownership-transfer/pty-ownership-transfer-recovered-source-route.ts index 2278f815ef9..44fec5024e5 100644 --- a/src/main/persistence/pty-ownership-transfer/pty-ownership-transfer-recovered-source-route.ts +++ b/src/main/persistence/pty-ownership-transfer/pty-ownership-transfer-recovered-source-route.ts @@ -1,6 +1,7 @@ import type { PtyOwnershipTransferAttachmentResult } from '../../../shared/pty-ownership-transfer-control-wire' import { PTY_OWNERSHIP_TRANSFER_WIRE_VERSION } from '../../../shared/pty-ownership-transfer-wire' import type { PtyOwnershipTransferCoordinatorOptions } from './pty-ownership-transfer-coordinator-contract' +import { assertTransferIdentity } from './pty-ownership-transfer-response-identity' export async function attachRecoveredPtyOwnershipTransferSourceRoute( options: PtyOwnershipTransferCoordinatorOptions, @@ -32,7 +33,7 @@ export async function attachRecoveredPtyOwnershipTransferSourceRoute( capabilities, requestOptions ) - assertIdentity(result, options.identity) + assertTransferIdentity(result, options.identity) return result } @@ -69,7 +70,7 @@ async function rekeyRecoveredRoute( capabilities, requestOptions ) - assertIdentity(rekeyed, options.identity) + assertTransferIdentity(rekeyed, options.identity) if ( rekeyed.previousReconnectGeneration !== durableReconnectGeneration || rekeyed.reconnectGeneration !== reconnectGeneration || @@ -87,19 +88,3 @@ async function rekeyRecoveredRoute( ...(rekeyed.exit ? { exit: rekeyed.exit } : {}) }) } - -function assertIdentity( - value: PtyOwnershipTransferCoordinatorOptions['identity'], - expected: PtyOwnershipTransferCoordinatorOptions['identity'] -): void { - if ( - value.bridgeId !== expected.bridgeId || - value.terminalId !== expected.terminalId || - value.incarnationId !== expected.incarnationId || - value.ownerLease !== expected.ownerLease || - value.sourceOwnerGeneration !== expected.sourceOwnerGeneration || - value.destinationRuntimeId !== expected.destinationRuntimeId - ) { - throw new Error('pty_ownership_transfer_response_identity_mismatch') - } -} diff --git a/src/main/persistence/pty-ownership-transfer/pty-ownership-transfer-recovery.ts b/src/main/persistence/pty-ownership-transfer/pty-ownership-transfer-recovery.ts index 474c86bc9dd..a2264b82d0e 100644 --- a/src/main/persistence/pty-ownership-transfer/pty-ownership-transfer-recovery.ts +++ b/src/main/persistence/pty-ownership-transfer/pty-ownership-transfer-recovery.ts @@ -16,6 +16,7 @@ import type { } from './pty-ownership-transfer-coordinator' import type { PtyOwnershipTransferDestinationAdapter } from '../../../shared/pty-ownership-transfer-destination-adapter' import { attachRecoveredPtyOwnershipTransferSourceRoute } from './pty-ownership-transfer-recovered-source-route' +import { assertTransferIdentity } from './pty-ownership-transfer-response-identity' /** Reconcile a transfer after an ambiguous network response without replaying prepared output. */ export async function recoverPtyOwnershipTransfer( @@ -31,9 +32,6 @@ export async function recoverPtyOwnershipTransfer( const status = await options.source.status(request, requestOptions(options)) assertTransferIdentity(status, options.identity) - if (status.phase === 'aborted') { - return Object.freeze({ source: status, destination: null, published: false }) - } if (status.phase !== 'committed' && status.phase !== 'published') { return Object.freeze({ source: status, destination: null, published: false }) } @@ -119,9 +117,6 @@ async function reattachRecoveredDestination( const capabilities = options.getDestinationCapabilities ? await options.getDestinationCapabilities(requestOptions) : options.destinationCapabilities - if (capabilities === undefined) { - throw new Error('pty_ownership_transfer_recovery_destination_capabilities_unavailable') - } if (!capabilities) { throw new Error('pty_ownership_transfer_recovery_destination_capabilities_unavailable') } @@ -276,26 +271,3 @@ function requestOptions(options: PtyOwnershipTransferCoordinatorOptions): { ...(options.timeoutMs === undefined ? {} : { timeoutMs: options.timeoutMs }) } } - -function assertTransferIdentity( - value: { - bridgeId: string - terminalId: string - incarnationId: string - ownerLease: string - sourceOwnerGeneration: number - destinationRuntimeId: string - }, - expected: PtyOwnershipTransferCoordinatorOptions['identity'] -): void { - if ( - value.bridgeId !== expected.bridgeId || - value.terminalId !== expected.terminalId || - value.incarnationId !== expected.incarnationId || - value.ownerLease !== expected.ownerLease || - value.sourceOwnerGeneration !== expected.sourceOwnerGeneration || - value.destinationRuntimeId !== expected.destinationRuntimeId - ) { - throw new Error('pty_ownership_transfer_response_identity_mismatch') - } -} diff --git a/src/main/persistence/pty-ownership-transfer/pty-ownership-transfer-response-identity.test.ts b/src/main/persistence/pty-ownership-transfer/pty-ownership-transfer-response-identity.test.ts new file mode 100644 index 00000000000..fec4a00bff9 --- /dev/null +++ b/src/main/persistence/pty-ownership-transfer/pty-ownership-transfer-response-identity.test.ts @@ -0,0 +1,29 @@ +import { expect, it } from 'vitest' +import { assertTransferIdentity } from './pty-ownership-transfer-response-identity' + +const identity = { + bridgeId: 'bridge-1', + terminalId: 'terminal-1', + incarnationId: 'incarnation-1', + ownerLease: 'lease-1', + sourceOwnerGeneration: 1, + destinationRuntimeId: 'runtime-1' +} as const + +it('accepts an exact transfer identity with additive response fields', () => { + const response = { ...identity, phase: 'published' } + expect(() => assertTransferIdentity(response, identity)).not.toThrow() +}) + +it.each([ + { bridgeId: 'bridge-2' }, + { terminalId: 'terminal-2' }, + { incarnationId: 'incarnation-2' }, + { ownerLease: 'lease-2' }, + { sourceOwnerGeneration: 2 }, + { destinationRuntimeId: 'runtime-2' } +])('rejects a changed transfer response identity %j', (changed) => { + expect(() => assertTransferIdentity({ ...identity, ...changed }, identity)).toThrow( + 'pty_ownership_transfer_response_identity_mismatch' + ) +}) diff --git a/src/main/persistence/pty-ownership-transfer/pty-ownership-transfer-response-identity.ts b/src/main/persistence/pty-ownership-transfer/pty-ownership-transfer-response-identity.ts index 93d03e15348..2b6539d27ef 100644 --- a/src/main/persistence/pty-ownership-transfer/pty-ownership-transfer-response-identity.ts +++ b/src/main/persistence/pty-ownership-transfer/pty-ownership-transfer-response-identity.ts @@ -1,17 +1,11 @@ import type { PtyOwnershipTransferIdentity } from '../../../shared/pty-ownership-transfer-journal-contract' +import { samePtyOwnershipTransferIdentity } from '../../../shared/pty-ownership-transfer-identity' export function assertTransferIdentity( value: PtyOwnershipTransferIdentity, expected: PtyOwnershipTransferIdentity ): void { - if ( - value.bridgeId !== expected.bridgeId || - value.terminalId !== expected.terminalId || - value.incarnationId !== expected.incarnationId || - value.ownerLease !== expected.ownerLease || - value.sourceOwnerGeneration !== expected.sourceOwnerGeneration || - value.destinationRuntimeId !== expected.destinationRuntimeId - ) { + if (!samePtyOwnershipTransferIdentity(value, expected)) { throw new Error('pty_ownership_transfer_response_identity_mismatch') } } diff --git a/src/main/providers/runtime-pty-ownership-transfer-output-credit.ts b/src/main/providers/runtime-pty-ownership-transfer-output-credit.ts index afdec714e70..cb0f45446a3 100644 --- a/src/main/providers/runtime-pty-ownership-transfer-output-credit.ts +++ b/src/main/providers/runtime-pty-ownership-transfer-output-credit.ts @@ -14,8 +14,6 @@ export class RuntimePtyOwnershipTransferOutputCredit { private readonly outputAcknowledgements = new Map< string, { - identity: PtyOwnershipTransferWireIdentity - attachmentId: string binding: RuntimePtyOwnershipTransferAttachmentBinding acknowledge: (throughSeq: number) => void } @@ -58,8 +56,6 @@ export class RuntimePtyOwnershipTransferOutputCredit { } } const registration = { - identity: Object.freeze({ ...identity }), - attachmentId, binding, acknowledge } diff --git a/src/main/providers/runtime-pty-ownership-transfer-source-grants.ts b/src/main/providers/runtime-pty-ownership-transfer-source-grants.ts index 9d194bae16e..5bf38febea9 100644 --- a/src/main/providers/runtime-pty-ownership-transfer-source-grants.ts +++ b/src/main/providers/runtime-pty-ownership-transfer-source-grants.ts @@ -41,7 +41,6 @@ export class RuntimePtyOwnershipTransferSourceGrants { clientId: number transportGeneration: number pairedDeviceId: string - issuedAt: number expiresAt: number }> >() @@ -165,9 +164,6 @@ export class RuntimePtyOwnershipTransferSourceGrants { if (!source) { throw new Error('pty_ownership_transfer_source_authority_unavailable') } - if (this.runtimeId !== undefined && request.destinationRuntimeId === this.runtimeId) { - throw new Error('pty_ownership_transfer_source_self_target') - } const identity = Object.freeze({ bridgeId: derivePairedGrantBridgeId( source, @@ -204,7 +200,6 @@ export class RuntimePtyOwnershipTransferSourceGrants { clientId: binding.clientId, transportGeneration: binding.transportGeneration!, pairedDeviceId: binding.pairedDeviceId, - issuedAt, expiresAt: issuedAt + this.grantTtlMs }) ) diff --git a/src/main/ssh/orcad-incumbent-recovery.ts b/src/main/ssh/orcad-incumbent-recovery.ts index 88209fd6e35..00865db2891 100644 --- a/src/main/ssh/orcad-incumbent-recovery.ts +++ b/src/main/ssh/orcad-incumbent-recovery.ts @@ -14,17 +14,13 @@ import { parseOrcadSnapshotRestore, restoreOrcadStateSnapshotCommand } from './orcad-state-snapshot' -import { - orcadStopFreedTheHost, - parseOrcadStopOutcome, - stopOrcadCommand -} from './orcad-remote-process-control' +import { orcadStopFreedTheHost } from './orcad-remote-process-control' import { joinRemotePath } from './ssh-remote-platform' import type { OrcadActivationLockControl } from './orcad-activation-lock' import { exec, launchAndAwaitReadiness, - STOP_WAIT_SECONDS, + stopOrcadSlot, withoutAbortSignal } from './orcad-remote-runtime-control' @@ -157,12 +153,7 @@ export async function restoreIncumbent( candidateDir: string, snapshot: PreActivationSnapshot | null ): Promise<{ message: string; code?: string; recovered: boolean }> { - const stopped = parseOrcadStopOutcome( - await exec( - options, - stopOrcadCommand(options.host, candidateDir, { waitSeconds: STOP_WAIT_SECONDS }) - ) - ) + const stopped = await stopOrcadSlot(options, candidateDir) if (!orcadStopFreedTheHost(stopped)) { return { code: 'orcad_rejected_candidate_stop_incomplete', diff --git a/src/main/ssh/orcad-installed-activation.ts b/src/main/ssh/orcad-installed-activation.ts index 12680b3b585..299b4ba72af 100644 --- a/src/main/ssh/orcad-installed-activation.ts +++ b/src/main/ssh/orcad-installed-activation.ts @@ -12,11 +12,7 @@ import { evaluateOrcadActivation } from './orcad-activation-gate' import { planOrcadUpdate } from './orcad-update-plan' import { ORCAD_LOG_FILENAME } from './orcad-remote-launch' import { orcadSnapshotDirName } from './orcad-state-snapshot' -import { - orcadStopFreedTheHost, - parseOrcadStopOutcome, - stopOrcadCommand -} from './orcad-remote-process-control' +import { orcadStopFreedTheHost } from './orcad-remote-process-control' import { joinRemotePath } from './ssh-remote-platform' import { computeLocalOrcadBuildHash } from './orcad-local-build-hash' import type { OrcadActivationLockControl } from './orcad-activation-lock' @@ -36,6 +32,7 @@ import { exec, launchAndAwaitReadiness, STOP_WAIT_SECONDS, + stopOrcadSlot, withoutAbortSignal } from './orcad-remote-runtime-control' import { @@ -141,14 +138,7 @@ export async function activateInstalledOrcad( 'launched.' } } - const stopped = parseOrcadStopOutcome( - await exec( - options, - stopOrcadCommand(options.host, outgoingDir, { - waitSeconds: STOP_WAIT_SECONDS - }) - ) - ) + const stopped = await stopOrcadSlot(options, outgoingDir) if (!orcadStopFreedTheHost(stopped)) { return { outcome: 'installed-not-activated', diff --git a/src/main/ssh/orcad-remote-rollback.test.ts b/src/main/ssh/orcad-remote-rollback.test.ts index 149acbcf3ce..d4d1128fd85 100644 --- a/src/main/ssh/orcad-remote-rollback.test.ts +++ b/src/main/ssh/orcad-remote-rollback.test.ts @@ -191,6 +191,48 @@ describe('rollbackOrcad', () => { ) }) + it('polls pending readiness and stops sleeping once the target answers', async () => { + scriptHost([]) + const baseImplementation = mockExec.getMockImplementation() + let pending = true + const sleep = vi.fn(async () => { + pending = false + }) + mockExec.mockImplementation(async (conn, command, execOptions) => { + if (pending && command.startsWith('head -c ') && command.includes('.orcad-readiness')) { + return '' + } + return baseImplementation?.(conn, command, execOptions) ?? '' + }) + + await expect(rollbackOrcad(options({ sleep }))).resolves.toMatchObject({ + outcome: 'rolled-back' + }) + + expect(sleep).toHaveBeenCalledExactlyOnceWith(500) + }) + + it.each(['STILL_RUNNING', 'NO_PID', 'UNKNOWN'])( + 'does not restore or launch when the active stop answers %s', + async (stopOutput) => { + const log: string[] = [] + scriptHost(log) + const baseImplementation = mockExec.getMockImplementation() + mockExec.mockImplementation(async (conn, command, execOptions) => { + const output = await baseImplementation?.(conn, command, execOptions) + return output === 'STOPPED' ? stopOutput : (output ?? '') + }) + + await expect(rollbackOrcad(options())).resolves.toMatchObject({ + outcome: 'failed', + code: 'orcad_rollback_stop_incomplete' + }) + + expect(log).toEqual([`stop:${ACTIVE}`]) + expect(writeOrcadActivationRecord).not.toHaveBeenCalled() + } + ) + it('refuses before touching anything when terminals started after activation', async () => { const log: string[] = [] scriptHost(log) @@ -313,42 +355,58 @@ describe('rollbackOrcad', () => { ]) }) - it('restores the rescue state when a confirmed target readiness probe fails', async () => { - const log: string[] = [] - scriptHost(log) - const baseImplementation = mockExec.getMockImplementation() - mockExec.mockImplementation(async (conn, command: string, execOptions) => { - const text = String(command) - if ( - text.startsWith('head -c ') && - text.includes('.orcad-readiness') && - text.includes(TARGET) - ) { - throw new Error('confirmed readiness read failed') - } - return baseImplementation?.(conn, command, execOptions) ?? '' - }) + it.each(['probe failure', 'cancellation'])( + 'restores the rescue state after target readiness %s', + async (failure) => { + const log: string[] = [] + scriptHost(log) + const controller = new AbortController() + const baseImplementation = mockExec.getMockImplementation() + mockExec.mockImplementation(async (conn, command: string, execOptions) => { + const text = String(command) + if ( + text.startsWith('head -c ') && + text.includes('.orcad-readiness') && + text.includes(TARGET) + ) { + if (failure === 'cancellation') { + return '' + } + throw new Error('confirmed readiness read failed') + } + if (controller.signal.aborted) { + expect(execOptions?.signal).toBeUndefined() + } + return baseImplementation?.(conn, command, execOptions) ?? '' + }) - const result = await rollbackOrcad(options()) + const result = await rollbackOrcad( + options({ + signal: controller.signal, + readinessTimeoutMs: 5_000, + sleep: async () => controller.abort(new Error('cancelled during readiness')) + }) + ) - expect(result).toMatchObject({ - outcome: 'failed', - code: 'orcad_rollback_target_launch_failed' - }) - expect(writeOrcadActivationRecord).not.toHaveBeenCalled() - expect(log).toEqual([ - `stop:${ACTIVE}`, - 'rescue', - 'restore', - `launch:${TARGET}`, - `stop:${TARGET}`, - 'restore-rescue', - `launch:${ACTIVE}` - ]) - expect(result.outcome === 'failed' && result.reason).toContain( - `orcad ${ACTIVE} was restored and is serving again` - ) - }) + expect(result).toMatchObject({ + outcome: 'failed', + code: 'orcad_rollback_target_launch_failed' + }) + expect(writeOrcadActivationRecord).not.toHaveBeenCalled() + expect(log).toEqual([ + `stop:${ACTIVE}`, + 'rescue', + 'restore', + `launch:${TARGET}`, + `stop:${TARGET}`, + 'restore-rescue', + `launch:${ACTIVE}` + ]) + expect(result.outcome === 'failed' && result.reason).toContain( + `orcad ${ACTIVE} was restored and is serving again` + ) + } + ) it('records the rollback only after the target answers healthy', async () => { const log: string[] = [] diff --git a/src/main/ssh/orcad-remote-runtime-control.ts b/src/main/ssh/orcad-remote-runtime-control.ts index 2e65d8b732b..ca7a28ed91a 100644 --- a/src/main/ssh/orcad-remote-runtime-control.ts +++ b/src/main/ssh/orcad-remote-runtime-control.ts @@ -10,6 +10,7 @@ import { } from './orcad-remote-launch' import { resolveOrcadActivationReadinessTimeout } from './orcad-activation-lock' import { evaluateOrcadActivation, type OrcadActivationVerdict } from './orcad-activation-gate' +import { parseOrcadStopOutcome, stopOrcadCommand } from './orcad-remote-process-control' import type { ServeReadiness } from '../server/serve-readiness' const DEFAULT_READINESS_TIMEOUT_MS = 90_000 @@ -35,7 +36,7 @@ export function withoutAbortSignal( } export async function launchAndAwaitReadiness( - options: OrcadDeployOptions, + options: Pick, spec: OrcadLaunchSpec ): Promise> { await exec(options, orcadLaunchCommand(options.host, spec)) @@ -64,39 +65,35 @@ export async function launchAndGate( verdict: OrcadActivationVerdict readiness: ServeReadiness | null }> { - await exec( - options, - orcadLaunchCommand(options.host, { - remoteInstallDir: identity.remoteDir, - nodePath: identity.nodePath, - fullVersion: identity.version, - userDataDir: options.userDataDir, - bindHost: options.bindHost, - port: options.port, - allowHostNodeFallback: true - }) - ) - const deadline = - Date.now() + - resolveOrcadActivationReadinessTimeout(options.readinessTimeoutMs, DEFAULT_READINESS_TIMEOUT_MS) - const sleep = options.sleep ?? ((ms: number) => new Promise((resolve) => setTimeout(resolve, ms))) - let parsed = parseOrcadReadinessOutput('') - while (Date.now() < deadline && parsed.state === 'pending') { - options.signal?.throwIfAborted() - parsed = parseOrcadReadinessOutput( - await exec(options, readOrcadReadinessCommand(options.host, identity.remoteDir)) - ) - if (parsed.state === 'pending') { - await sleep(READINESS_POLL_MS) - } - } + const parsed = await launchAndAwaitReadiness(options, { + remoteInstallDir: identity.remoteDir, + nodePath: identity.nodePath, + fullVersion: identity.version, + userDataDir: options.userDataDir, + bindHost: options.bindHost, + port: options.port, + allowHostNodeFallback: true + }) + const readiness = parsed.state === 'ready' ? parsed.readiness : null return { - verdict: evaluateOrcadActivation(parsed.state === 'ready' ? parsed.readiness : null, { + verdict: evaluateOrcadActivation(readiness, { buildHash: identity.buildHash, fullVersion: identity.version, runtimeKind: identity.nodePath ? 'node' : 'bun', port: options.port }), - readiness: parsed.state === 'ready' ? parsed.readiness : null + readiness } } + +export async function stopOrcadSlot( + options: Pick, + remoteDir: string +): Promise> { + return parseOrcadStopOutcome( + await exec( + options, + stopOrcadCommand(options.host, remoteDir, { waitSeconds: STOP_WAIT_SECONDS }) + ) + ) +} diff --git a/src/main/ssh/orcad-rollback-recovery.ts b/src/main/ssh/orcad-rollback-recovery.ts index 360457e7d0b..e7e94123ed6 100644 --- a/src/main/ssh/orcad-rollback-recovery.ts +++ b/src/main/ssh/orcad-rollback-recovery.ts @@ -13,11 +13,7 @@ import { parseOrcadSnapshotRestore, restoreOrcadStateSnapshotCommand } from './orcad-state-snapshot' -import { - orcadStopFreedTheHost, - parseOrcadStopOutcome, - stopOrcadCommand -} from './orcad-remote-process-control' +import { orcadStopFreedTheHost } from './orcad-remote-process-control' import { joinRemotePath } from './ssh-remote-platform' import type { OrcadActivationLockControl } from './orcad-activation-lock' import { readRemoteOrcadBuildHash } from './orcad-remote-build-hash' @@ -25,7 +21,7 @@ import { resolveOrcadSlotNodeFallback } from './orcad-slot-runtime-eligibility' import { exec, launchAndGate, - STOP_WAIT_SECONDS, + stopOrcadSlot, withoutAbortSignal } from './orcad-remote-runtime-control' @@ -168,14 +164,9 @@ export async function stopFailedTargetAndRecoverActive( active: ActiveRuntimeIdentity, rescue: RollbackRescueSnapshot ): Promise<{ code?: string; reason: string }> { - let stoppedTarget: ReturnType + let stoppedTarget: Awaited> try { - stoppedTarget = parseOrcadStopOutcome( - await exec( - withoutAbortSignal(options), - stopOrcadCommand(options.host, target.remoteDir, { waitSeconds: STOP_WAIT_SECONDS }) - ) - ) + stoppedTarget = await stopOrcadSlot(withoutAbortSignal(options), target.remoteDir) } catch (error) { lock.retain() return { diff --git a/src/main/ssh/orcad-rollback-transition.ts b/src/main/ssh/orcad-rollback-transition.ts index 8bc338d6acf..b2ec467325f 100644 --- a/src/main/ssh/orcad-rollback-transition.ts +++ b/src/main/ssh/orcad-rollback-transition.ts @@ -13,11 +13,7 @@ import { probeOrcadStateSnapshotCommand, restoreOrcadStateSnapshotCommand } from './orcad-state-snapshot' -import { - orcadStopFreedTheHost, - parseOrcadStopOutcome, - stopOrcadCommand -} from './orcad-remote-process-control' +import { orcadStopFreedTheHost } from './orcad-remote-process-control' import { writeOrcadActivationRecord } from './orcad-activation-record-store' import { joinRemotePath } from './ssh-remote-platform' import type { OrcadActivationLockControl } from './orcad-activation-lock' @@ -28,7 +24,12 @@ import { type OrcadRollbackTransaction } from './orcad-activation-transaction' import { writeOrcadActivationTransaction } from './orcad-activation-transaction-store' -import { exec, launchAndGate, STOP_WAIT_SECONDS } from './orcad-remote-runtime-control' +import { + exec, + launchAndGate, + STOP_WAIT_SECONDS, + stopOrcadSlot +} from './orcad-remote-runtime-control' import { snapshotDirPath, readStateWritesSinceActivation, @@ -116,12 +117,7 @@ export async function rollbackLocked( await writeOrcadActivationTransaction(options, transaction) lock.retainOnError() - const stopped = parseOrcadStopOutcome( - await exec( - options, - stopOrcadCommand(options.host, activeIdentity.remoteDir, { waitSeconds: STOP_WAIT_SECONDS }) - ) - ) + const stopped = await stopOrcadSlot(options, activeIdentity.remoteDir) if (!orcadStopFreedTheHost(stopped)) { return { outcome: 'failed', diff --git a/src/shared/pty-ownership-transfer-destination-adapter-replay.ts b/src/shared/pty-ownership-transfer-destination-adapter-replay.ts index fe0ed693779..01e331d35fc 100644 --- a/src/shared/pty-ownership-transfer-destination-adapter-replay.ts +++ b/src/shared/pty-ownership-transfer-destination-adapter-replay.ts @@ -11,7 +11,7 @@ import { type PtyOwnershipTransferDestinationSnapshot } from './pty-ownership-transfer-destination-adapter-contract' import { - assertJournalIdentity, + assertIdentity, bytesOf, identityFrom, validateFrame, @@ -37,7 +37,7 @@ export function prepareDestinationTransfer( ) } if (existing) { - assertJournalIdentity(existing, identity) + assertIdentity(existing, identity) } const journal = existing ?? state.options.store.prepare(identity, result.replayStartSeq - 1) if (journal.acceptedSourceEndSeq > result.sourceOutputEndSeq) { diff --git a/src/shared/pty-ownership-transfer-destination-adapter-validation.ts b/src/shared/pty-ownership-transfer-destination-adapter-validation.ts index 45498ee6cb8..dee3bea7f9a 100644 --- a/src/shared/pty-ownership-transfer-destination-adapter-validation.ts +++ b/src/shared/pty-ownership-transfer-destination-adapter-validation.ts @@ -1,7 +1,6 @@ import { PTY_OWNERSHIP_TRANSFER_PUBLICATION_RECEIPT_VERSION, type PtyOwnershipTransferCommitReceipt, - type PtyOwnershipTransferDestinationJournal, type PtyOwnershipTransferIdentity, type PtyOwnershipTransferPublicationReceipt } from './pty-ownership-transfer-journal-contract' @@ -22,7 +21,11 @@ import { samePtyOwnershipTransferSurfaceBinding, type PtyOwnershipTransferSurfaceBinding } from './pty-ownership-transfer-surface-binding' -import { samePtyOwnershipTransferPublicationReceipt } from './pty-ownership-transfer-receipt-validation' +import { + samePtyOwnershipTransferCommitReceipt, + samePtyOwnershipTransferPublicationReceipt +} from './pty-ownership-transfer-receipt-validation' +import { samePtyOwnershipTransferIdentity } from './pty-ownership-transfer-identity' export function identityFrom( value: PtyOwnershipTransferPrepareResult @@ -125,7 +128,7 @@ export function validatePublicationReceipt( receipt.bridgeId !== record.identity.bridgeId || receipt.destinationRuntimeId !== record.identity.destinationRuntimeId || !record.commitReceipt || - !sameCommitReceipt(receipt.commitReceipt, record.commitReceipt) || + !samePtyOwnershipTransferCommitReceipt(receipt.commitReceipt, record.commitReceipt) || !Number.isFinite(Date.parse(receipt.publishedAt)) || !record.surfaceBinding || !record.surfacePublication || @@ -186,7 +189,7 @@ export function assertSameCommitReceipt( expected: PtyOwnershipTransferCommitReceipt | undefined, actual: PtyOwnershipTransferCommitReceipt ): void { - if (!expected || !sameCommitReceipt(expected, actual)) { + if (!expected || !samePtyOwnershipTransferCommitReceipt(expected, actual)) { throw new PtyOwnershipTransferDestinationError( 'receipt-invalid', 'commit receipt changed while recovering a destination transfer' @@ -194,25 +197,11 @@ export function assertSameCommitReceipt( } } -export function assertJournalIdentity( - journal: PtyOwnershipTransferDestinationJournal, - identity: PtyOwnershipTransferIdentity -): void { - assertIdentity(journal, identity) -} - export function assertIdentity( expected: PtyOwnershipTransferIdentity, actual: PtyOwnershipTransferIdentity ): void { - if ( - expected.bridgeId !== actual.bridgeId || - expected.terminalId !== actual.terminalId || - expected.incarnationId !== actual.incarnationId || - expected.ownerLease !== actual.ownerLease || - expected.sourceOwnerGeneration !== actual.sourceOwnerGeneration || - expected.destinationRuntimeId !== actual.destinationRuntimeId - ) { + if (!samePtyOwnershipTransferIdentity(expected, actual)) { throw new PtyOwnershipTransferDestinationError( 'identity-mismatch', 'destination transfer identity does not match its durable journal' @@ -241,15 +230,3 @@ export function boundedPositive(value: number, maximum: number): number { } return Math.min(value, maximum) } - -function sameCommitReceipt( - left: PtyOwnershipTransferCommitReceipt, - right: PtyOwnershipTransferCommitReceipt -): boolean { - return ( - left.receiptId === right.receiptId && - left.bridgeId === right.bridgeId && - left.acceptedSourceEndSeq === right.acceptedSourceEndSeq && - left.committedAt === right.committedAt - ) -} diff --git a/src/shared/pty-ownership-transfer-destination-adapter.test.ts b/src/shared/pty-ownership-transfer-destination-adapter.test.ts index 1200f9c7a24..d082e983eba 100644 --- a/src/shared/pty-ownership-transfer-destination-adapter.test.ts +++ b/src/shared/pty-ownership-transfer-destination-adapter.test.ts @@ -285,18 +285,46 @@ describe('PtyOwnershipTransferDestinationAdapter', () => { ) }) - it('rejects an identity change during recovery', () => { + it.each([ + { bridgeId: 'other' }, + { terminalId: 'other' }, + { incarnationId: 'other' }, + { ownerLease: 'other' }, + { sourceOwnerGeneration: 4 }, + { destinationRuntimeId: 'other' } + ])('rejects an identity change during recovery: %j', (change) => { const { store } = createStore() const adapter = createAdapter(store) adapter.prepare(prepareResult) - expect(() => + const accept = () => adapter.acceptReplay({ ...prepareResult, - ownerLease: 'other-lease', + ...change, frames, sourceOutputEndSeq: 2 }) - ).toThrow(PtyOwnershipTransferDestinationError) + expect(accept).toThrow(PtyOwnershipTransferDestinationError) + expect(accept).toThrow(expect.objectContaining({ reason: 'identity-mismatch' })) + }) + + it.each([ + { receiptId: 'other' }, + { bridgeId: 'other' }, + { acceptedSourceEndSeq: 3 }, + { committedAt: '2026-08-30T12:01:01.000Z' } + ])('rejects every changed durable commit receipt field: %j', (change) => { + const { store, calls } = createStore() + const adapter = createAdapter(store) + adapter.prepare(prepareResult) + for (const frame of frames) { + adapter.acceptReplayFrame(frame) + } + adapter.commit(commitReceipt) + expect(adapter.commit({ ...commitReceipt }).phase).toBe('committed') + expect(() => adapter.commit({ ...commitReceipt, ...change })).toThrow( + expect.objectContaining({ reason: 'receipt-invalid' }) + ) + expect(calls.filter((call) => call === 'commit')).toHaveLength(1) }) it('accepts only the newest in-process attachment generation', () => { diff --git a/src/shared/pty-ownership-transfer-journal.test.ts b/src/shared/pty-ownership-transfer-journal.test.ts index 0537c79dab5..924b3045ecc 100644 --- a/src/shared/pty-ownership-transfer-journal.test.ts +++ b/src/shared/pty-ownership-transfer-journal.test.ts @@ -1,5 +1,6 @@ import { describe, expect, it } from 'vitest' import { + assertPtyOwnershipTransferIdentity, normalizePtyOwnershipTransferJournals, parsePtyOwnershipTransferJournal } from './pty-ownership-transfer-journal' @@ -236,8 +237,10 @@ describe('PTY ownership transfer journal contract', () => { value: undefined, writable: true }) - expect(normalizePtyOwnershipTransferJournals([valid])).toEqual([ - expect.objectContaining({ bridgeId: 'bridge-1' }) + const input = Object.freeze([valid, { ...valid, bridgeId: 'bridge-2' }]) + expect(normalizePtyOwnershipTransferJournals(input)).toEqual([ + expect.objectContaining({ bridgeId: 'bridge-1' }), + expect.objectContaining({ bridgeId: 'bridge-2' }) ]) } finally { if (descriptor) { @@ -247,4 +250,25 @@ describe('PTY ownership transfer journal contract', () => { } } }) + + it.each([ + { bridgeId: 'other' }, + { terminalId: 'other' }, + { incarnationId: 'other' }, + { ownerLease: 'other' }, + { sourceOwnerGeneration: 4 }, + { destinationRuntimeId: 'other' } + ])('rejects every changed journal identity field: %j', (change) => { + const journal = parsePtyOwnershipTransferJournal({ + ...base, + side: 'source', + phase: 'prepared', + sourceOutputEndSeq: 0, + destinationOutputEndSeq: 0 + }) + expect(() => assertPtyOwnershipTransferIdentity(journal, base)).not.toThrow() + expect(() => assertPtyOwnershipTransferIdentity(journal, { ...base, ...change })).toThrow( + 'pty_ownership_transfer_identity_conflict' + ) + }) }) diff --git a/src/shared/pty-ownership-transfer-journal.ts b/src/shared/pty-ownership-transfer-journal.ts index 18ddd08956c..2fcf6a67a92 100644 --- a/src/shared/pty-ownership-transfer-journal.ts +++ b/src/shared/pty-ownership-transfer-journal.ts @@ -25,6 +25,7 @@ import { } from './pty-ownership-transfer-value-validation' import { publicationReceiptMatchesPtyOwnershipTransfer } from './pty-ownership-transfer-receipt-validation' import { parsePtyOwnershipTransferSurfaceBinding } from './pty-ownership-transfer-surface-binding' +import { samePtyOwnershipTransferIdentity } from './pty-ownership-transfer-identity' export { MAX_PTY_OWNERSHIP_TRANSFER_JOURNALS, @@ -171,14 +172,7 @@ export function assertPtyOwnershipTransferIdentity( journal: PtyOwnershipTransferJournal, identity: PtyOwnershipTransferIdentity ): void { - if ( - journal.bridgeId !== identity.bridgeId || - journal.terminalId !== identity.terminalId || - journal.incarnationId !== identity.incarnationId || - journal.ownerLease !== identity.ownerLease || - journal.sourceOwnerGeneration !== identity.sourceOwnerGeneration || - journal.destinationRuntimeId !== identity.destinationRuntimeId - ) { + if (!samePtyOwnershipTransferIdentity(journal, identity)) { throw new Error('pty_ownership_transfer_identity_conflict') } }