mirror of
https://github.com/stablyai/orca.git
synced 2026-10-22 00:03:47 +00:00
fix(pty): retain rejected SSH split cleanup owner
This commit is contained in:
@@ -6609,6 +6609,7 @@
|
||||
"direct SSH reconnect finalization",
|
||||
"direct SSH folder workspace reattach",
|
||||
"direct SSH split-pane retry ownership",
|
||||
"rejected SSH split cleanup ownership",
|
||||
"same-authority terminal correction",
|
||||
"remote-runtime host surface materialization",
|
||||
"remote-runtime mirror polling",
|
||||
@@ -6630,11 +6631,13 @@
|
||||
"https://github.com/stablyai/orca/pull/6979",
|
||||
"https://github.com/stablyai/orca/pull/7009",
|
||||
"https://github.com/stablyai/orca/pull/8597",
|
||||
"https://linear.app/stably/issue/STA-4023",
|
||||
"https://github.com/stablyai/orca/issues/11541"
|
||||
],
|
||||
"invariant": "SSH, WSL, and remote-runtime restore paths must treat provider listing failures and unknown liveness as unknown, not dead, while still avoiding duplicate spawn and clearing expired relay leases exactly once. Direct SSH reconnect must atomically clear only exact-target live PTY bindings, preserve relay identity, retry Git and folder panes without paired close or provider shutdown, and allow at most two automatic attempts in one authority chain even when each settlement exceeds the rolling window. A rejected acknowledgement mutates no store map. A successful exact split-pane spawn or reattach must retain that attempt as shared live authority until sibling leaves settle; the first success cannot consume sibling authority, a sibling failure can start at most one second tab-wide attempt, and prior-attempt callbacks become inert after rotation. Once the retry budget is exhausted, a failure cannot start attempt three or revoke attempt-two authority from siblings that may still settle. Primary PTY exit must promote a bound survivor or preserve exact authority through an empty activation gap, and split detach must project that authority to both resulting tabs. Hydrated PTY hints cannot supersede a current exact-attempt owner, and target snapshot hydration/reconnect cannot reset sibling SSH, local, WSL, or runtime-owned state. Every restored remote terminal must preserve its provider PTY identity, including the authoritative incarnation returned by a successful session-ID reattach. After a recoverable partition the same authenticated runtime must reattach the same PTY, reject detached input, apply the latest viewport, and report healthy only after authoritative replay. A successful one-shot reachability probe may replace a pre-ready shared-control socket, but waiting RPCs must continue onto the replacement under their original deadline without duplicate host delivery or retained request bytes. Automatic PTY recovery stops after one bounded minute without a fatal terminal error; a manual reconnect starts a newly fenced epoch against the same PTY, and closed panes retain no recovery UI state. One capability-gated terminal-create mutation must produce at most one host PTY across an unknown response outcome, remain manually retryable after cutoff, and never let a stale completion replace a newer pane lifecycle. Reconnect must alternate exact activation with authoritative inventory so neither a stale activation response nor an activation failure can strand or retire a pane, and activating a parked surface whose persisted binding was already retired must respawn it rather than report a changed owner after signalling its exit.",
|
||||
"invariant": "SSH, WSL, and remote-runtime restore paths must treat provider listing failures and unknown liveness as unknown, not dead, while still avoiding duplicate spawn and clearing expired relay leases exactly once. A source-rejected split retains exact provider ownership or a durable cleanup-only lease until shutdown is confirmed; shutdown failure stays retryable without restoring the rejected pane. Direct SSH reconnect must atomically clear only exact-target live PTY bindings, preserve relay identity, retry Git and folder panes without paired close or provider shutdown, and allow at most two automatic attempts in one authority chain even when each settlement exceeds the rolling window. A rejected acknowledgement mutates no store map. A successful exact split-pane spawn or reattach must retain that attempt as shared live authority until sibling leaves settle; the first success cannot consume sibling authority, a sibling failure can start at most one second tab-wide attempt, and prior-attempt callbacks become inert after rotation. Once the retry budget is exhausted, a failure cannot start attempt three or revoke attempt-two authority from siblings that may still settle. Primary PTY exit must promote a bound survivor or preserve exact authority through an empty activation gap, and split detach must project that authority to both resulting tabs. Hydrated PTY hints cannot supersede a current exact-attempt owner, and target snapshot hydration/reconnect cannot reset sibling SSH, local, WSL, or runtime-owned state. Every restored remote terminal must preserve its provider PTY identity, including the authoritative incarnation returned by a successful session-ID reattach. After a recoverable partition the same authenticated runtime must reattach the same PTY, reject detached input, apply the latest viewport, and report healthy only after authoritative replay. A successful one-shot reachability probe may replace a pre-ready shared-control socket, but waiting RPCs must continue onto the replacement under their original deadline without duplicate host delivery or retained request bytes. Automatic PTY recovery stops after one bounded minute without a fatal terminal error; a manual reconnect starts a newly fenced epoch against the same PTY, and closed panes retain no recovery UI state. One capability-gated terminal-create mutation must produce at most one host PTY across an unknown response outcome, remain manually retryable after cutoff, and never let a stale completion replace a newer pane lifecycle. Reconnect must alternate exact activation with authoritative inventory so neither a stale activation response nor an activation failure can strand or retire a pane, and activating a parked surface whose persisted binding was already retired must respawn it rather than report a changed owner after signalling its exit.",
|
||||
"oracle": "Deterministic tests cover bounded stale-handle replacement, suspended heartbeat clocks, cold and established subscription failure, ten partition/recovery cycles, automatic-recovery cutoff, manual reconnect, and exact direct SSH binding recovery. They assert one atomic store publication clears only exact-target PTY indexes, null-PTY activation remains unchanged, relay identity survives, Git and folder panes retry symmetrically, another target/local/WSL/runtime panes remain byte-identical through target snapshot hydration and reconnect, only an accepted exact failure or timeout starts the second attempt, two 31-second timeouts cannot start a third settlement-triggered attempt, rejected stale/mismatched acknowledgements preserve every store map, and concurrent split-pane spawn and reattach callbacks both commit under the same attempt ID after the first success replaces pending state with live shared authority. A sibling failure revokes that shared authority and starts exactly one second attempt; duplicate failures and late first-attempt PTY callbacks preserve the second attempt and every state map. Attempt-two failure retains continuation authority for later siblings, primary exit promotes a bound survivor or preserves the lease until a late sibling binds, and primary plus non-primary detach retain exact authority and history on both resulting tabs. Both remount callbacks consume split-count activity suppression, intentional dispose emits no failure/timeout, and a same-attempt StrictMode remount still owns one timeout. Hydration clears an untrusted PTY hint without clearing its current pending owner, healthy current-authority bindings suppress correction, hydration finalizes once, and reconnect emits no paired close lifecycle. A provider-level session-ID reattach returns an incarnation, then a legacy exit without an incarnation must resolve to that returned identity rather than minting a fallback identity. The shared-control oracle withholds the first encrypted ready frame, starts one RPC, triggers the successful-probe refresh, then requires exactly two client connections, one host request, a successful response, zero pending calls, and zero retained request bytes. Tests also assert one unsubscribe per remote-runtime epoch, observable recovery phases, stable PTY identity, resumed snapshot/output/input, no healthy state before replay, no retry or input after cutoff, a new manual epoch against the same PTY, quiet recovery UI with an explicit Reconnect action, pane-close state cleanup, one stable create mutation id, old-runtime no-retry behavior, cross-process PTY adoption, and bounded in-flight coordination.",
|
||||
"commands": [
|
||||
"pnpm exec vitest run --config config/vitest.config.ts src/main/ipc/pty.test.ts src/main/ssh/ssh-relay-session.test.ts",
|
||||
"pnpm exec vitest run --config config/vitest.config.ts src/main/providers/ssh-pty-provider-reattach-incarnation.test.ts --reporter=dot",
|
||||
"pnpm exec vitest run --config config/vitest.config.ts src/renderer/src/startup/ssh-startup-reconnect.test.ts src/renderer/src/lib/resolved-worktree-execution-host.test.ts src/renderer/src/components/terminal/background-terminal-worktree-mount.test.ts src/renderer/src/runtime/sync-runtime-graph-scheduling.test.ts src/renderer/src/components/terminal-pane/use-terminal-pane-lifecycle.test.ts src/renderer/src/components/terminal-pane/pty-connection.test.ts src/renderer/src/components/terminal-pane/remote-runtime-pty-transport.test.ts src/renderer/src/runtime/remote-runtime-session-tabs-inflight.test.ts src/renderer/src/runtime/web-session-terminal-handle-events.test.ts src/renderer/src/store/slices/terminal-pty-identity-replacement.test.ts",
|
||||
"pnpm exec vitest run --config config/vitest.config.ts src/renderer/src/components/terminal-pane/pty-transport.test.ts",
|
||||
@@ -6649,6 +6652,8 @@
|
||||
"ORCA_E2E_SSH_DOCKER=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"
|
||||
],
|
||||
"testFiles": [
|
||||
"src/main/ipc/pty.test.ts",
|
||||
"src/main/ssh/ssh-relay-session.test.ts",
|
||||
"src/main/providers/ssh-pty-provider-reattach-incarnation.test.ts",
|
||||
"src/renderer/src/startup/ssh-startup-reconnect.test.ts",
|
||||
"src/renderer/src/lib/resolved-worktree-execution-host.test.ts",
|
||||
@@ -6700,6 +6705,18 @@
|
||||
"tests/e2e/ssh-docker-relay-perf.spec.ts"
|
||||
],
|
||||
"assertionRefs": [
|
||||
{
|
||||
"file": "src/main/ipc/pty.test.ts",
|
||||
"assertions": [
|
||||
"a source-binding race plus remote shutdown failure leaves no UI pane but retains exact routing and a cleanup-only SSH lease until explicit kill retry confirms process death"
|
||||
]
|
||||
},
|
||||
{
|
||||
"file": "src/main/ssh/ssh-relay-session.test.ts",
|
||||
"assertions": [
|
||||
"reconnect may reclaim a cleanup-only PTY lease but never registers or persists it as a rejected terminal surface"
|
||||
]
|
||||
},
|
||||
{
|
||||
"file": "src/renderer/src/startup/ssh-startup-reconnect.test.ts",
|
||||
"assertions": [
|
||||
|
||||
@@ -8789,9 +8789,12 @@ describe('registerPtyHandlers', () => {
|
||||
expect.objectContaining({
|
||||
targetId: connectionId,
|
||||
ptyId: 'rejected-split-pty',
|
||||
worktreeId: 'repo-1::/remote/repo',
|
||||
state: 'attached'
|
||||
})
|
||||
)
|
||||
expect(store.upsertSshRemotePtyLease.mock.calls[0]?.[0]).not.toHaveProperty('tabId')
|
||||
expect(store.upsertSshRemotePtyLease.mock.calls[0]?.[0]).not.toHaveProperty('leafId')
|
||||
expect(warn).toHaveBeenCalledWith(
|
||||
'[pty] failed to clean up PTY after persistence failure:',
|
||||
expect.objectContaining({ message: 'injected remote shutdown failure' })
|
||||
@@ -11582,7 +11585,21 @@ describe('registerPtyHandlers', () => {
|
||||
).rejects.toThrow(/ORCA_TERMINAL_SESSION_STATE_SAVE_FAILED/)
|
||||
|
||||
expect(remoteShutdown).toHaveBeenCalledWith(appPtyId, { immediate: true })
|
||||
expect(store.upsertSshRemotePtyLease).not.toHaveBeenCalled()
|
||||
expect(store.upsertSshRemotePtyLease).toHaveBeenCalledWith(
|
||||
expect.objectContaining({
|
||||
targetId: 'ssh-fresh-fail',
|
||||
ptyId: 'relay-pty',
|
||||
worktreeId: 'wt-remote',
|
||||
state: 'attached'
|
||||
})
|
||||
)
|
||||
expect(store.upsertSshRemotePtyLease.mock.calls[0]?.[0]).not.toHaveProperty('tabId')
|
||||
expect(store.upsertSshRemotePtyLease.mock.calls[0]?.[0]).not.toHaveProperty('leafId')
|
||||
expect(store.markSshRemotePtyLease).toHaveBeenCalledWith(
|
||||
'ssh-fresh-fail',
|
||||
'relay-pty',
|
||||
'terminated'
|
||||
)
|
||||
expect(store.removeSshRemotePtyLease).not.toHaveBeenCalled()
|
||||
expect(openCodeClearPtyMock).toHaveBeenCalledWith(appPtyId)
|
||||
expect(piClearPtyMock).toHaveBeenCalledWith(appPtyId)
|
||||
|
||||
+38
-20
@@ -1110,6 +1110,23 @@ function finishPtyShutdown(
|
||||
return incarnationId
|
||||
}
|
||||
|
||||
async function shutdownFreshPtyAfterBindingFailure(
|
||||
provider: IPtyProvider,
|
||||
id: string,
|
||||
connectionId: string | null | undefined,
|
||||
store: Store | undefined
|
||||
): Promise<void> {
|
||||
try {
|
||||
await provider.shutdown(id, { immediate: true })
|
||||
} catch (error) {
|
||||
if (!isPtyAlreadyGoneError(error)) {
|
||||
console.warn('[pty] failed to clean up PTY after persistence failure:', error)
|
||||
return
|
||||
}
|
||||
}
|
||||
finishPtyShutdown(id, connectionId, store)
|
||||
}
|
||||
|
||||
// ─── Host PTY env assembly ──────────────────────────────────────────
|
||||
// Why: centralize host-local env injections so both spawn paths (local + daemon) get them; implemented twice they drifted, silently breaking daemon PTYs.
|
||||
|
||||
@@ -5168,7 +5185,7 @@ export function registerPtyHandlers(
|
||||
markNativeWindowsConptyPty(result.id)
|
||||
}
|
||||
const relayResultId = getRelayPtyId(args.connectionId, result.id)
|
||||
const persistSshLease = (): void => {
|
||||
const persistSshLease = (includeSurfaceIdentity = true): void => {
|
||||
if (!store || !args.connectionId) {
|
||||
return
|
||||
}
|
||||
@@ -5177,8 +5194,12 @@ export function registerPtyHandlers(
|
||||
targetId: args.connectionId,
|
||||
ptyId: relayResultId,
|
||||
...(typeof args.worktreeId === 'string' ? { worktreeId: args.worktreeId } : {}),
|
||||
...(typeof args.tabId === 'string' ? { tabId: args.tabId } : {}),
|
||||
...(typeof args.leafId === 'string' && isTerminalLeafId(args.leafId)
|
||||
...(includeSurfaceIdentity && typeof args.tabId === 'string'
|
||||
? { tabId: args.tabId }
|
||||
: {}),
|
||||
...(includeSurfaceIdentity &&
|
||||
typeof args.leafId === 'string' &&
|
||||
isTerminalLeafId(args.leafId)
|
||||
? { leafId: args.leafId }
|
||||
: {}),
|
||||
state: 'attached',
|
||||
@@ -5227,13 +5248,14 @@ export function registerPtyHandlers(
|
||||
} catch (err) {
|
||||
console.error('[pty] failed to persist runtime PTY binding after spawn:', err)
|
||||
if (!result.isReattach) {
|
||||
deletePtyOwnership(result.id)
|
||||
try {
|
||||
await provider.shutdown(result.id, { immediate: true })
|
||||
} catch (shutdownErr) {
|
||||
console.warn('[pty] failed to clean up PTY after persistence failure:', shutdownErr)
|
||||
}
|
||||
clearProviderPtyState(result.id)
|
||||
// Why: retain durable cleanup authority without reconnect resurrecting the rejected pane.
|
||||
persistSshLease(false)
|
||||
await shutdownFreshPtyAfterBindingFailure(
|
||||
provider,
|
||||
result.id,
|
||||
args.connectionId,
|
||||
store
|
||||
)
|
||||
}
|
||||
if (err instanceof Error && err.message === 'terminal_split_source_not_found') {
|
||||
throw err
|
||||
@@ -6737,16 +6759,12 @@ export function registerPtyHandlers(
|
||||
} catch (err) {
|
||||
console.error('[pty] failed to persist PTY binding after spawn:', err)
|
||||
if (!result.isReattach) {
|
||||
try {
|
||||
await provider.shutdown(result.id, { immediate: true })
|
||||
} catch (shutdownErr) {
|
||||
console.warn('[pty] failed to clean up PTY after persistence failure:', shutdownErr)
|
||||
}
|
||||
clearProviderPtyState(result.id)
|
||||
deletePtyOwnership(result.id)
|
||||
}
|
||||
if (!result.isReattach && args.connectionId && store) {
|
||||
store.removeSshRemotePtyLease(args.connectionId, relayResultId)
|
||||
await shutdownFreshPtyAfterBindingFailure(
|
||||
provider,
|
||||
result.id,
|
||||
args.connectionId,
|
||||
store
|
||||
)
|
||||
}
|
||||
throw Object.assign(new Error(createTerminalSessionStateSaveFailureMessage()), {
|
||||
agentSessionOperationOutcome: 'unknown' as const
|
||||
|
||||
@@ -531,6 +531,46 @@ describe('SshRelaySession', () => {
|
||||
)
|
||||
})
|
||||
|
||||
it('does not restore a cleanup-only lease as a terminal surface', async () => {
|
||||
const { mockConn, mockStore, mockPortForward, getMainWindow } = createMockDeps()
|
||||
const { getSshPtyProvider } = await import('../ipc/pty')
|
||||
const mockAttach = vi.fn().mockResolvedValue({ incarnationId: 'cleanup-incarnation' })
|
||||
vi.mocked(getSshPtyProvider).mockReturnValue({
|
||||
attachForReconnect: mockAttach,
|
||||
dispose: vi.fn()
|
||||
} as unknown as ReturnType<typeof getSshPtyProvider>)
|
||||
vi.mocked(mockStore.getSshRemotePtyLeases).mockReturnValue([
|
||||
{
|
||||
targetId: 'target-1',
|
||||
ptyId: 'cleanup-pty',
|
||||
worktreeId: 'repo-1::/remote/repo',
|
||||
state: 'detached'
|
||||
}
|
||||
] as ReturnType<typeof mockStore.getSshRemotePtyLeases>)
|
||||
const runtime = {
|
||||
onPtySpawned: vi.fn(),
|
||||
registerPty: vi.fn()
|
||||
}
|
||||
const session = new SshRelaySession(
|
||||
'target-1',
|
||||
getMainWindow,
|
||||
mockStore,
|
||||
mockPortForward,
|
||||
runtime as never
|
||||
)
|
||||
|
||||
await session.establish(mockConn)
|
||||
|
||||
expect(mockAttach).toHaveBeenCalledWith('cleanup-pty')
|
||||
expect(runtime.onPtySpawned).toHaveBeenCalledWith(
|
||||
'ssh:target-1@@cleanup-pty',
|
||||
'cleanup-incarnation',
|
||||
{ awaitsRegistration: false }
|
||||
)
|
||||
expect(runtime.registerPty).not.toHaveBeenCalled()
|
||||
expect(mockStore.persistPtyBinding).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it('forwards a lease tab identity to reattach so a reset relay cannot cross-wire it', async () => {
|
||||
const { mockConn, mockStore, mockPortForward, getMainWindow } = createMockDeps()
|
||||
const { getSshPtyProvider } = await import('../ipc/pty')
|
||||
|
||||
Reference in New Issue
Block a user