fix(ssh): close source recovery review gaps

This commit is contained in:
OrcaWin
2026-07-28 23:57:43 -07:00
parent aca05800d9
commit a3ef00323a
10 changed files with 111 additions and 58 deletions
+20 -2
View File
@@ -3099,7 +3099,7 @@
"pnpm exec vitest run --config config/vitest.config.ts src/main/providers/ssh-pty-provider.test.ts src/main/providers/ssh-pty-source-delivery-ledger.test.ts src/main/ssh/ssh-pty-retired-source-deliveries.test.ts src/main/ssh/ssh-relay-session.test.ts src/main/ssh/ssh-relay-session-data-delivery.test.ts src/main/ssh/ssh-relay-session-recovery-races.test.ts src/main/ssh/ssh-relay-session-incarnation.test.ts src/main/ssh/ssh-relay-session-reconnect-incarnation.test.ts src/main/ssh/ssh-relay-session-terminal-error.test.ts src/main/ssh/ssh-pty-recovery-retention-budget.test.ts src/main/ssh/relay-protocol-backpressure.test.ts src/relay/protocol-backpressure.test.ts src/relay/pty-source-credit-ledger.test.ts src/relay/pty-source-credit-scheduler.test.ts src/relay/relay-pty-source-publication.test.ts src/relay/ssh-pty-source-credit-adapter.test.ts --reporter=dot",
"pnpm exec vitest run --config config/vitest.config.ts src/main/ipc/ssh-pty-model-admission.test.ts src/main/ipc/ssh-pty-output-model-migration.test.ts src/main/ssh/ssh-relay-session-model-migration.test.ts --reporter=dot",
"pnpm exec vitest run --config config/vitest.config.ts src/relay/git-response-stream-ownership.test.ts src/relay/pty-handler.test.ts --reporter=dot",
"pnpm exec vitest run --config config/vitest.config.ts src/main/providers/ssh-pty-notification-routing.test.ts src/main/providers/ssh-pty-provider-agent-session-create-operation.test.ts src/main/providers/ssh-pty-provider-exit-race.test.ts --reporter=dot",
"pnpm exec vitest run --config config/vitest.config.ts src/main/providers/ssh-pty-notification-routing.test.ts src/main/providers/ssh-pty-provider-agent-session-create-operation.test.ts src/main/providers/ssh-pty-provider-exit-race.test.ts src/main/providers/ssh-pty-provider-reattach-incarnation.test.ts --reporter=dot",
"pnpm exec vitest run --config config/vitest.config.ts src/relay/relay-pty-source-recovery-interleavings.test.ts src/relay/relay-pty-source-recovery-completion.test.ts src/relay/relay-pty-source-restore-retry.test.ts --reporter=dot",
"pnpm exec vitest run --config config/vitest.config.ts src/relay/dispatcher.test.ts src/relay/pty-handler-source-publication.test.ts --reporter=dot",
"pnpm exec vitest run --config config/vitest.config.ts src/main/ssh/ssh-relay-session-recovery-races.test.ts --reporter=dot",
@@ -3110,6 +3110,7 @@
"pnpm exec vitest run --config config/vitest.config.ts src/relay/relay-pty-source-restore-retry.test.ts src/main/ssh/ssh-relay-session-recovery-races.test.ts src/main/runtime/rpc/terminal-source-range-ledger.test.ts src/main/runtime/rpc/terminal-multiplex.test.ts src/main/ssh/ssh-multiplexer-transport-writer.test.ts src/main/ssh/ssh-channel-multiplexer-backpressure.test.ts src/main/providers/ssh-pty-notification-routing.test.ts src/main/providers/ssh-pty-source-delivery-ledger.test.ts src/main/ssh/ssh-pty-retired-source-deliveries.test.ts src/main/providers/ssh-pty-provider-agent-session-create-operation.test.ts src/main/providers/ssh-pty-provider-exit-race.test.ts src/main/ipc/ssh-pty-output-exit-deadline.test.ts src/main/ipc/ssh-pty-remote-source-range-consumers.test.ts --reporter=dot",
"pnpm exec vitest run src/main/providers/ssh-pty-notification-routing.test.ts src/main/providers/ssh-pty-provider-exit-race.test.ts src/main/ssh/ssh-relay-session-data-delivery.test.ts src/main/ssh/ssh-relay-session-recovery-races.test.ts src/main/ssh/ssh-relay-session-reconnect-incarnation.test.ts src/main/ssh/ssh-relay-session-incarnation.test.ts src/main/ssh/ssh-relay-session.test.ts --reporter=dot",
"pnpm exec vitest run --config config/vitest.config.ts src/main/ssh/ssh-pty-source-credit-rollout.test.ts src/main/ssh/ssh-relay-session.test.ts src/main/ssh/ssh-relay-session-reconnect-incarnation.test.ts src/main/persistence.test.ts src/renderer/src/components/settings/ssh-target-draft.test.ts src/renderer/src/components/settings/ssh-target-save-payload.test.ts src/renderer/src/components/settings/SshHostAdvancedFields.test.tsx --reporter=dot",
"pnpm exec vitest run --config config/vitest.config.ts src/main/providers/ssh-pty-notification-routing.test.ts src/main/providers/ssh-pty-provider-reattach-incarnation.test.ts src/relay/relay-pty-source-recovery-interleavings.test.ts src/main/ssh/ssh-relay-deploy.test.ts src/main/ssh/ssh-relay-session-data-delivery.test.ts --reporter=dot",
"pnpm exec vitest run --config config/vitest.config.ts src/main/agent-hooks/wsl-hook-relay-sentinel.test.ts --reporter=dot",
"ORCA_E2E_SSH_DOCKER=1 ORCA_SSH_PTY_SOURCE_CREDIT_V1=1 SKIP_BUILD=1 pnpm exec playwright test tests/e2e/ssh-docker-relay-perf.spec.ts --config tests/playwright.config.ts --project electron-headless --workers=1",
"ORCA_E2E_SSH_DOCKER=1 ORCA_SSH_PTY_SOURCE_CREDIT_V1=1 pnpm exec playwright test tests/e2e/ssh-docker-relay-perf.spec.ts --config tests/playwright.config.ts --project electron-headless --workers=1"
@@ -3126,6 +3127,7 @@
"src/main/providers/ssh-pty-source-delivery-ledger.test.ts",
"src/main/providers/ssh-pty-provider-agent-session-create-operation.test.ts",
"src/main/providers/ssh-pty-provider-exit-race.test.ts",
"src/main/providers/ssh-pty-provider-reattach-incarnation.test.ts",
"src/main/ssh/ssh-relay-session.test.ts",
"src/main/ssh/ssh-relay-session-data-delivery.test.ts",
"src/main/ssh/ssh-relay-session-model-migration.test.ts",
@@ -3223,6 +3225,7 @@
"assertions": [
"provisional source frames remain unprojected and are discarded on rollback",
"held and later recovery frames route only to the private sink until final commit, without ordinary listeners or live PTY publication",
"exact source frames use their immutable incarnation without invoking or mutating the legacy incarnation resolver",
"exit during private recovery retires the activation ledger at final commit and rejects late same-token frames",
"private recovery retirement restores the exact predecessor without issuing a second cancellation",
"older rollback cancels only its exact token without replacing a newer activation"
@@ -3247,6 +3250,12 @@
"exit before a fresh spawn reply drops provisional source data and settles exact cancellation"
]
},
{
"file": "src/main/providers/ssh-pty-provider-reattach-incarnation.test.ts",
"assertions": [
"source-credit restore-required generic reattach fails as expired instead of returning an outputless live PTY"
]
},
{
"file": "src/main/ssh/ssh-relay-session-reconnect-incarnation.test.ts",
"assertions": [
@@ -3350,7 +3359,7 @@
"failed exit publication retains sealed delivery for late cumulative ACK and exact token recovery",
"exact owner recovery republishes exit without reopening source admission",
"a failed recovery-completion frame rolls back its fence and the next exact owner republishes it before live admission",
"request cancellation remains authoritative through writer settlement and retry returns the same replacement recovery identity"
"failed activation response retains private retry authority, and exact retry clears activation only after successful settlement"
]
},
{
@@ -3619,6 +3628,15 @@
"durationSeconds": 20.19,
"summary": "Seven deterministic rollout, session, persistence, and renderer files passed 494 tests. The focused assertions prove per-target default-off opt-in, explicit environment precedence, normalized persistence through SSH-config refresh, Advanced-setting rendering/save/clear, and immutable selection through automatic reconnect; no live SSH or paired-runtime topology was exercised."
},
{
"date": "2026-07-28",
"runner": "local",
"platform": "macos",
"command": "pnpm exec vitest run --config config/vitest.config.ts src/main/providers/ssh-pty-notification-routing.test.ts src/main/providers/ssh-pty-provider-reattach-incarnation.test.ts src/relay/relay-pty-source-recovery-interleavings.test.ts src/main/ssh/ssh-relay-deploy.test.ts src/main/ssh/ssh-relay-session-data-delivery.test.ts --reporter=dot",
"result": "passed",
"durationSeconds": 1.63,
"summary": "Five deterministic provider, relay, deploy, and session files passed 70 tests after red seams proved that source frames mutated legacy incarnation state and generic restore-required reattach returned an outputless live PTY. The candidate keeps exact source identity side-effect free, fails generic restore-required reattach as expired, preserves session-latched reconnect selection, and proves a failed recovery activation stays private and exact-retryable until successful response settlement; no live topology was exercised."
},
{
"date": "2026-07-28",
"runner": "local",
@@ -810,7 +810,9 @@ The receive cursor starts at `checkpointSourceEndSu`, not
then advance only contiguously. Frames through `recoveryEndSu` stay
quarantined as recovery; later frames stay held until the exact completion
fence commits. Unknown or mismatched source identities are rejected before
`livePtyIds`, runtime, renderer, or source obligations change.
`livePtyIds`, runtime, renderer, or source obligations change. Exact
source-bearing frames carry their immutable `ptyIncarnation` directly and do
not invoke or mutate the legacy incarnation resolver.
Every exact-token private frame advances a monotonic observed-source high-water
before quarantine retention or `restoreRequired` checks. If exact exit arrives,
@@ -861,6 +863,12 @@ capacity; a retry can then mint a fresh token. It does not activate a
subscription or stream a partial gap; main either restores an authoritative
model generation before retry or surfaces the gap.
The reconnect coordinator consumes `restoreRequired` and owns snapshot-backed
replacement. A generic existing-session provider reattach has no such recovery
owner, so it maps `restoreRequired` to `SSH_SESSION_EXPIRED` instead of
returning an outputless live PTY; the existing renderer fallback may then
replace that stale session.
Recovery bodies use the ordinary source-bearing `pty.data` schema. The new
token initializes `sentEndSu = creditedEndSu = checkpointSourceEndSu`, and
the retained gap covers exactly
@@ -191,7 +191,7 @@ describe('subscribeSshPtyNotifications', () => {
})
})
it('keeps exact source incarnation independent from prior legacy delivery state', () => {
it('keeps exact source incarnation independent without mutating legacy delivery state', () => {
const { handler, dataListeners, resolvePtyIncarnation, installReceivingActivation } =
createSubscription()
const onData = vi.fn()
@@ -216,7 +216,8 @@ describe('subscribeSshPtyNotifications', () => {
'incarnation:pty-1',
'incarnation-1'
])
expect(resolvePtyIncarnation).toHaveBeenCalledTimes(2)
expect(resolvePtyIncarnation).toHaveBeenCalledTimes(1)
expect(resolvePtyIncarnation).toHaveBeenCalledWith('pty-1', undefined)
})
it('drops stale delivery generations without touching their PTY or unrelated PTYs', () => {
@@ -444,7 +445,7 @@ describe('subscribeSshPtyNotifications', () => {
expect(onData.mock.calls.map((call) => call[0].data)).toEqual(['one', 'two'])
expect(livePtyIds).toEqual(liveBeforeInvalid)
expect(toAppPtyId).toHaveBeenCalledTimes(1)
expect(resolvePtyIncarnation).toHaveBeenCalledTimes(1)
expect(resolvePtyIncarnation).not.toHaveBeenCalled()
expect(mux.request).toHaveBeenCalledWith('pty.cancelDelivery', {
id: 'pty-1',
clientGeneration,
@@ -44,15 +44,14 @@ export function subscribeSshPtyNotifications(args: {
}): SshPtyNotificationSubscription {
const toDataPayload = (pending: PendingSshPtySourceData): Parameters<SshPtyDataCallback>[0] => {
const id = args.toAppPtyId(pending.relayPtyId)
const ptyIncarnation = args.resolvePtyIncarnation(
pending.relayPtyId,
pending.params.ptyIncarnation ?? pending.params.incarnationId
)
const ptyIncarnation = pending.source
? (pending.params.ptyIncarnation as string)
: args.resolvePtyIncarnation(pending.relayPtyId, pending.params.incarnationId)
return {
id,
data: pending.data,
providerGeneration: args.providerGeneration,
ptyIncarnation: pending.source ? (pending.params.ptyIncarnation as string) : ptyIncarnation,
ptyIncarnation,
...(typeof pending.params.rawLength === 'number'
? { sequenceChars: pending.params.rawLength }
: {}),
@@ -1,4 +1,5 @@
import { describe, expect, it, vi } from 'vitest'
import { SSH_SESSION_EXPIRED_ERROR } from './ssh-pty-errors'
import { SshPtyProvider } from './ssh-pty-provider'
describe('SSH PTY provider session reattach incarnation', () => {
@@ -28,4 +29,23 @@ describe('SSH PTY provider session reattach incarnation', () => {
})
)
})
it('fails closed when generic reattach requires source restoration', async () => {
const mux = {
request: vi.fn().mockResolvedValue({
incarnationId: 'incarnation-reattached',
sourceRecovery: {
status: 'restoreRequired',
reason: 'checkpointUnavailable'
}
}),
notify: vi.fn(),
onNotification: vi.fn().mockReturnValue(vi.fn())
}
const provider = new SshPtyProvider('conn-1', mux as never)
await expect(provider.spawn({ cols: 80, rows: 24, sessionId: 'pty-old' })).rejects.toThrow(
`${SSH_SESSION_EXPIRED_ERROR}: pty-old`
)
})
})
+11 -1
View File
@@ -22,6 +22,7 @@ import { buildSshPtySpawnRequest } from './ssh-pty-spawn-request'
import { SshPtySpawnExitRaceTracker } from './ssh-pty-spawn-exit-race'
import { SshAgentSessionCapabilities } from './ssh-agent-session-capabilities'
import type { PtyProcessInspection } from './pty-process-inspection'
import { SSH_SESSION_EXPIRED_ERROR } from './ssh-pty-errors'
// Why: sequential relay teardown calls share one absolute budget; convert to the mux-relative timeout only at dispatch.
function relayTimeoutOptions(deadlineMs: number | undefined): { timeoutMs: number } | undefined {
@@ -97,9 +98,18 @@ export class SshPtyProvider implements IPtyProvider {
rememberPtyIncarnation: (relayPtyId, incarnationId) =>
this.outputState.rememberPtyIncarnation(relayPtyId, incarnationId)
})
if (result.sourceRecovery?.status === 'restoreRequired') {
throw new Error(
`${SSH_SESSION_EXPIRED_ERROR}: ${toRelaySshPtyId(this.connectionId, result.id)}`
)
}
this.livePtyIds.add(result.id)
result.sourceActivationLease?.commit()
const { sourceActivationLease: _lease, ...spawnResult } = result
const {
sourceActivationLease: _lease,
sourceRecovery: _sourceRecovery,
...spawnResult
} = result
return spawnResult
} catch (error) {
result?.sourceActivationLease?.rollback()
@@ -28,6 +28,7 @@ export type SshPtyAttachResult = {
}
type SshPtyReattachResult = PtySpawnResult & {
sourceRecovery?: PtySourceRecoveryResult
sourceActivationLease?: SshPtyReceivingActivationLease
}
@@ -209,6 +210,7 @@ export async function reattachSshPtySession(args: {
isReattach: true,
...(attachResult.replay ? { replay: attachResult.replay } : {}),
...(attachResult.incarnationId ? { incarnationId: attachResult.incarnationId } : {}),
...(attachResult.sourceRecovery ? { sourceRecovery: attachResult.sourceRecovery } : {}),
...(attachResult.sourceActivation ? { sourceActivation: attachResult.sourceActivation } : {}),
...(attachResult.sourceActivationLease
? { sourceActivationLease: attachResult.sourceActivationLease }
+1 -1
View File
@@ -697,7 +697,7 @@ async function repairInstalledNativeDeps(
} catch (err) {
signal?.throwIfAborted()
console.warn(
`[ssh-relay] Launch namespace marker is unconfirmed at ${remoteDir}; retaining the install lock`
`[ssh-relay] Launch namespace marker is unconfirmed at ${remoteDir}; deferring lock ownership to stale recovery`
)
return { ownsInstallLock: !isUnconfirmedSshCommandTermination(err) }
}
@@ -599,7 +599,7 @@ describe('SshRelaySession data delivery', () => {
expect(notifyWithSettlementMock).toHaveBeenCalledWith('pty.ackData', batch, settled)
})
it('applies the V1 kill switch to reconnect negotiation', async () => {
it('keeps the captured V1 selection through reconnect negotiation', async () => {
let enabled = true
const { mockConn, mockStore, mockPortForward, getMainWindow } = createMockDeps()
const session = new SshRelaySession(
@@ -617,7 +617,7 @@ describe('SshRelaySession data delivery', () => {
await session.reconnect(mockConn)
expect(openConsumerSessionMock.mock.calls[0][1]).toHaveProperty('outputFlowControl')
expect(openConsumerSessionMock.mock.calls[1][1]).not.toHaveProperty('outputFlowControl')
expect(openConsumerSessionMock.mock.calls[1][1]).toHaveProperty('outputFlowControl')
expect(deployAndLaunchRelay).toHaveBeenNthCalledWith(
1,
mockConn,
@@ -632,7 +632,7 @@ describe('SshRelaySession data delivery', () => {
undefined,
undefined,
'target-1',
false
true
)
})
@@ -236,7 +236,7 @@ describe('relay PTY source recovery interleavings', () => {
).toHaveLength(1)
})
it('keeps timeout cancellation authoritative until a recovery response settles', async () => {
it('keeps failed recovery activation private and retryable until exact response settles', async () => {
const primaryWrites: Buffer[] = []
dispatcher = new RelayDispatcher(
(data, settle) => {
@@ -278,24 +278,13 @@ describe('relay PTY source recovery interleavings', () => {
dispatcher.invalidateClient()
const recoveredWrites: Buffer[] = []
let blockedResponseSettlement: ((result: SinkWriteSettlement) => void) | undefined
let drainWriter: (() => void) | undefined
const recoveredClientId = dispatcher.attachClient(
(data, settle) => {
recoveredWrites.push(Buffer.from(data))
if (message(data)?.id === 3) {
blockedResponseSettlement = settle
return false
}
settle({ ok: true })
return true
},
{
supportsWriteCallback: true,
waitWriteDrain: (callback) => {
drainWriter = callback
}
},
{ supportsWriteCallback: true },
endpointIdentity
)
dispatcher.feedClient(
@@ -312,47 +301,53 @@ describe('relay PTY source recovery interleavings', () => {
})
)
await flushRequests()
dispatcher.onRequest('pty.testRecover', async (params, context) => ({
sourceRecovery: publication.activate(
'pty-1',
'incarnation-1',
context,
params.sourceRecovery as PtySourceRecoveryRequest
)
}))
const recoveryRequest = {
const recoveryRequest: PtySourceRecoveryRequest = {
status: 'checkpoint',
deliveryToken: oldData.deliveryToken,
clientGeneration: oldData.clientGeneration,
ownerGeneration: oldData.ownerGeneration,
deliveryToken: String(oldData.deliveryToken),
clientGeneration: Number(oldData.clientGeneration),
ownerGeneration: Number(oldData.ownerGeneration),
ptyIncarnation: 'incarnation-1',
acceptedSourceEndSu: 4
}
dispatcher.feedClient(
recoveredClientId,
requestFrame(3, 'pty.testRecover', { sourceRecovery: recoveryRequest })
const firstActivationSettlements: ((result: SinkWriteSettlement) => void)[] = []
const firstRecovery = publication.activate(
'pty-1',
'incarnation-1',
{
clientId: recoveredClientId,
isStale: () => false,
sessionIdentity: endpointIdentity,
onResponseSettled: (callback) => firstActivationSettlements.push(callback)
},
recoveryRequest
)
await flushRequests()
const firstRecovery = responseResult(recoveredWrites, 3)!.sourceRecovery
expect(firstRecovery).toMatchObject({
status: 'pending',
checkpointSourceEndSu: 4,
recoveryEndSu: 8
})
dispatcher.feedClient(
recoveredClientId,
encodeJsonRpcFrame({ jsonrpc: '2.0', method: 'rpc.cancel', params: { id: 3 } }, 4, 0)
)
blockedResponseSettlement!({ ok: true })
drainWriter!()
dispatcher.feedClient(
recoveredClientId,
requestFrame(4, 'pty.testRecover', { sourceRecovery: recoveryRequest })
)
await flushRequests()
firstActivationSettlements[0]({ ok: false, error: new Error('response publication failed') })
expect(publication.getDebugSnapshot()).toMatchObject({ active: 0, activating: 1 })
const writesBeforeRetry = recoveredWrites.length
publication.onCreditAvailable('pty-1')
expect(recoveredWrites).toHaveLength(writesBeforeRetry)
expect(responseResult(recoveredWrites, 4)?.sourceRecovery).toEqual(firstRecovery)
const retryActivationSettlements: ((result: SinkWriteSettlement) => void)[] = []
const retriedRecovery = publication.activate(
'pty-1',
'incarnation-1',
{
clientId: recoveredClientId,
isStale: () => false,
sessionIdentity: endpointIdentity,
onResponseSettled: (callback) => retryActivationSettlements.push(callback)
},
recoveryRequest
)
expect(retriedRecovery).toEqual(firstRecovery)
retryActivationSettlements[0]({ ok: true })
expect(publication.getDebugSnapshot()).toMatchObject({ active: 1, activating: 0 })
})