mirror of
https://github.com/stablyai/orca.git
synced 2026-10-17 08:03:14 +00:00
fix: preserve teardown ownership across fallback paths
This commit is contained in:
@@ -3491,7 +3491,8 @@
|
||||
"pnpm exec vitest run --config config/vitest.config.ts src/shared/ssh-pty-id.test.ts src/shared/ssh-types.test.ts src/main/providers/local-pty-provider.test.ts src/main/providers/ssh-pty-provider.test.ts src/main/persistence.test.ts src/main/ssh/ssh-relay-session.test.ts src/main/ssh/ssh-relay-generation-reattach.test.ts src/main/ssh/ssh-target-id-migration.test.ts src/relay/pty-handler.test.ts src/relay/subprocess.test.ts src/renderer/src/components/terminal-pane/pty-transport.test.ts src/renderer/src/components/terminal-pane/pty-pre-handler-buffer.test.ts src/renderer/src/lib/pty-kill-retry-ownership.test.ts src/renderer/src/lib/runtime-terminal-close-retry-ownership.test.ts src/renderer/src/lib/ephemeral-vm-runtime-cleanup.test.ts src/renderer/src/lib/launch-agent-background-session.test.ts src/renderer/src/store/slices/repos.test.ts src/renderer/src/store/slices/repos-host-identity-routing.test.ts src/main/runtime/orca-runtime.test.ts src/main/ipc/pty.test.ts",
|
||||
"pnpm exec vitest run --config config/vitest.config.ts src/main/ipc/repos-create.test.ts src/main/window/attach-main-window-services.test.ts src/main/ipc/worktrees.test.ts src/main/ipc/worktrees-windows.test.ts src/main/ipc/pty.test.ts src/main/runtime/orca-runtime.test.ts src/renderer/src/lib/launch-agent-background-session.test.ts src/renderer/src/lib/runtime-terminal-close-retry-ownership.test.ts src/renderer/src/store/slices/repos-host-identity-routing.test.ts",
|
||||
"pnpm exec vitest run --config config/vitest.config.ts src/main/runtime/orca-runtime.test.ts src/main/ipc/worktrees.test.ts src/main/ipc/pty.test.ts src/main/ipc/runtime-environments.test.ts src/main/runtime/rpc/terminal-close-generation.test.ts src/main/persistence.test.ts src/main/ssh/ssh-relay-session.test.ts src/main/ipc/ssh.test.ts src/renderer/src/store/slices/worktrees.test.ts",
|
||||
"pnpm exec vitest run --config config/vitest.config.ts src/main/providers/local-pty-provider.test.ts src/main/daemon/daemon-pty-adapter.test.ts src/main/daemon/session.test.ts src/main/daemon/pty-subprocess.test.ts src/main/daemon/terminal-host.test.ts src/main/daemon/terminal-host-session-reaping-leak.test.ts src/main/daemon/daemon-server.test.ts src/main/ipc/pty.test.ts src/main/ipc/runtime-terminal-close-retry-owner.test.ts src/main/runtime/orca-runtime.test.ts"
|
||||
"pnpm exec vitest run --config config/vitest.config.ts src/main/providers/local-pty-provider.test.ts src/main/daemon/daemon-pty-adapter.test.ts src/main/daemon/session.test.ts src/main/daemon/pty-subprocess.test.ts src/main/daemon/terminal-host.test.ts src/main/daemon/terminal-host-session-reaping-leak.test.ts src/main/daemon/daemon-server.test.ts src/main/ipc/pty.test.ts src/main/ipc/runtime-terminal-close-retry-owner.test.ts src/main/runtime/orca-runtime.test.ts",
|
||||
"pnpm exec vitest run --config config/vitest.config.ts src/main/providers/local-pty-provider.test.ts src/main/daemon/daemon-pty-adapter.test.ts src/main/daemon/daemon-pty-router.test.ts src/main/daemon/degraded-daemon-pty-provider.test.ts src/main/daemon/session.test.ts src/main/daemon/pty-subprocess.test.ts src/main/daemon/terminal-host.test.ts src/main/daemon/terminal-host-session-reaping-leak.test.ts src/main/daemon/daemon-server.test.ts src/main/ipc/pty.test.ts src/main/ipc/runtime-terminal-close-retry-owner.test.ts src/main/runtime/orca-runtime.test.ts"
|
||||
],
|
||||
"testFiles": [
|
||||
"config/scripts/build-relay-windows-node-pty-patch.test.mjs",
|
||||
@@ -3508,6 +3509,8 @@
|
||||
"src/shared/ssh-types.test.ts",
|
||||
"src/main/providers/local-pty-provider.test.ts",
|
||||
"src/main/daemon/daemon-pty-adapter.test.ts",
|
||||
"src/main/daemon/daemon-pty-router.test.ts",
|
||||
"src/main/daemon/degraded-daemon-pty-provider.test.ts",
|
||||
"src/main/daemon/session.test.ts",
|
||||
"src/main/daemon/pty-subprocess.test.ts",
|
||||
"src/main/daemon/terminal-host.test.ts",
|
||||
@@ -3715,7 +3718,10 @@
|
||||
},
|
||||
{
|
||||
"file": "src/renderer/src/components/terminal-pane/pty-pre-handler-buffer.test.ts",
|
||||
"assertions": ["ownerless pre-handler exits evict oldest state at the 64-id bound"]
|
||||
"assertions": [
|
||||
"ownerless pre-handler exits evict oldest state at the 64-id bound",
|
||||
"an evicted legitimate exit triggers one targeted liveness readback and disconnects the dead pane"
|
||||
]
|
||||
},
|
||||
{
|
||||
"file": "src/renderer/src/lib/pty-kill-retry-ownership.test.ts",
|
||||
@@ -3743,10 +3749,24 @@
|
||||
"relay generation reuse removes stale retry ownership without touching the new PTY",
|
||||
"lease-less SSH shutdown survives provider unregister and re-drives the replacement transport",
|
||||
"accepted local and SSH kills retain retry ownership until provider readback proves physical exit",
|
||||
"runtime-owned and renderer SSH spawn persistence failures retain exact retry ownership after cleanup rejection and clear only after retry success",
|
||||
"an old provider exit readback cannot settle replacement-provider retry ownership",
|
||||
"unprovable pre-v22 restart intent retires once without synthesizing exit, retrying forever, or killing by recycled id"
|
||||
]
|
||||
},
|
||||
{
|
||||
"file": "src/main/daemon/daemon-pty-router.test.ts",
|
||||
"assertions": [
|
||||
"upgrade routing advertises exit-proof shutdown and retains the legacy adapter until onExit or readback",
|
||||
"a late legacy exit cannot delete a replacement route after PTY id reuse"
|
||||
]
|
||||
},
|
||||
{
|
||||
"file": "src/main/daemon/degraded-daemon-pty-provider.test.ts",
|
||||
"assertions": [
|
||||
"degraded routing advertises exit-proof shutdown and retains the exact child provider until onExit or readback"
|
||||
]
|
||||
},
|
||||
{
|
||||
"file": "src/main/ipc/runtime-terminal-close-retry-owner.test.ts",
|
||||
"assertions": [
|
||||
@@ -3978,6 +3998,24 @@
|
||||
"result": "passed",
|
||||
"durationSeconds": 9.8,
|
||||
"summary": "1781 cross-provider ownership tests passed, including whole-relay exit-proof drain and an atomic 50-live-PTY cap under 51 concurrent spawn requests."
|
||||
},
|
||||
{
|
||||
"date": "2026-07-12",
|
||||
"runner": "local",
|
||||
"platform": "macos",
|
||||
"command": "pnpm exec vitest run --config config/vitest.config.ts src/main/providers/local-pty-provider.test.ts src/main/daemon/daemon-pty-adapter.test.ts src/main/daemon/daemon-pty-router.test.ts src/main/daemon/degraded-daemon-pty-provider.test.ts src/main/daemon/session.test.ts src/main/daemon/pty-subprocess.test.ts src/main/daemon/terminal-host.test.ts src/main/daemon/terminal-host-session-reaping-leak.test.ts src/main/daemon/daemon-server.test.ts src/main/ipc/pty.test.ts src/main/ipc/runtime-terminal-close-retry-owner.test.ts src/main/runtime/orca-runtime.test.ts",
|
||||
"result": "passed",
|
||||
"durationSeconds": 9.3,
|
||||
"summary": "1414 daemon/provider/runtime tests passed, adding composite-provider exit-proof routing, late-exit generation fencing, and retained persistence-failure cleanup retries."
|
||||
},
|
||||
{
|
||||
"date": "2026-07-12",
|
||||
"runner": "local",
|
||||
"platform": "macos",
|
||||
"command": "pnpm exec vitest run --config config/vitest.config.ts src/shared/ssh-pty-id.test.ts src/shared/ssh-types.test.ts src/main/providers/local-pty-provider.test.ts src/main/providers/ssh-pty-provider.test.ts src/main/persistence.test.ts src/main/ssh/ssh-relay-session.test.ts src/main/ssh/ssh-relay-generation-reattach.test.ts src/main/ssh/ssh-target-id-migration.test.ts src/relay/pty-handler.test.ts src/relay/subprocess.test.ts src/renderer/src/components/terminal-pane/pty-transport.test.ts src/renderer/src/components/terminal-pane/pty-pre-handler-buffer.test.ts src/renderer/src/lib/pty-kill-retry-ownership.test.ts src/renderer/src/lib/runtime-terminal-close-retry-ownership.test.ts src/renderer/src/lib/ephemeral-vm-runtime-cleanup.test.ts src/renderer/src/lib/launch-agent-background-session.test.ts src/renderer/src/store/slices/repos.test.ts src/renderer/src/store/slices/repos-host-identity-routing.test.ts src/main/runtime/orca-runtime.test.ts src/main/ipc/pty.test.ts",
|
||||
"result": "passed",
|
||||
"durationSeconds": 10,
|
||||
"summary": "1784 cross-provider ownership tests passed, including bounded pre-handler exit overflow with targeted liveness recovery and both persistence-failure cleanup entry points."
|
||||
}
|
||||
],
|
||||
"runtimeBudget": {
|
||||
|
||||
@@ -122,6 +122,41 @@ function createAdapter(
|
||||
}
|
||||
|
||||
describe('DaemonPtyRouter', () => {
|
||||
it('retains the exact adapter route until exit proof', async () => {
|
||||
const current = createAdapter('current')
|
||||
const legacy = createAdapter('legacy', ['shared-session'])
|
||||
vi.mocked(legacy.shutdown).mockResolvedValueOnce(undefined)
|
||||
const router = new DaemonPtyRouter({ current, legacy: [legacy] })
|
||||
await router.discoverLegacySessions()
|
||||
|
||||
expect(router.requiresShutdownExitProof).toBe(true)
|
||||
await router.shutdown('shared-session', { immediate: true })
|
||||
router.write('shared-session', 'before-exit')
|
||||
|
||||
expect(legacy.write).toHaveBeenCalledWith('shared-session', 'before-exit')
|
||||
legacy.emitExit('shared-session', 0)
|
||||
router.write('shared-session', 'after-exit')
|
||||
expect(current.write).toHaveBeenCalledWith('shared-session', 'after-exit')
|
||||
})
|
||||
|
||||
it('fences a late legacy exit after readback and id reuse', async () => {
|
||||
const current = createAdapter('current')
|
||||
const legacy = createAdapter('legacy', ['shared-session'])
|
||||
const router = new DaemonPtyRouter({ current, legacy: [legacy] })
|
||||
const exit = vi.fn()
|
||||
router.onExit(exit)
|
||||
await router.discoverLegacySessions()
|
||||
|
||||
await router.shutdown('shared-session', { immediate: true })
|
||||
await router.listProcesses()
|
||||
await router.spawn({ sessionId: 'shared-session', cols: 80, rows: 24 })
|
||||
legacy.emitExit('shared-session', 0)
|
||||
router.write('shared-session', 'replacement')
|
||||
|
||||
expect(current.write).toHaveBeenCalledWith('shared-session', 'replacement')
|
||||
expect(exit).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it('routes fresh foreground confirmation to the session-owning daemon', async () => {
|
||||
const current = createAdapter('current', ['current-session'])
|
||||
const legacy = createAdapter('legacy', ['legacy-session'])
|
||||
|
||||
@@ -1,4 +1,5 @@
|
||||
import type { DaemonPtyAdapter } from './daemon-pty-adapter'
|
||||
import { listProviderProcessesAndReconcileRoutes } from './pty-provider-route-reconciliation'
|
||||
import type {
|
||||
IPtyProvider,
|
||||
PtyBackgroundStreamEvent,
|
||||
@@ -10,6 +11,7 @@ import type {
|
||||
} from '../providers/types'
|
||||
|
||||
export class DaemonPtyRouter implements IPtyProvider {
|
||||
readonly requiresShutdownExitProof = true
|
||||
private current: DaemonPtyAdapter
|
||||
private legacy: DaemonPtyAdapter[]
|
||||
private sessionAdapters = new Map<string, DaemonPtyAdapter>()
|
||||
@@ -33,7 +35,15 @@ export class DaemonPtyRouter implements IPtyProvider {
|
||||
}
|
||||
}),
|
||||
adapter.onExit((payload) => {
|
||||
this.sessionAdapters.delete(payload.id)
|
||||
// Why: a late exit from a legacy daemon must not delete a route that
|
||||
// has already been rebound to a replacement provider generation.
|
||||
const routed = this.sessionAdapters.get(payload.id)
|
||||
if (routed && routed !== adapter) {
|
||||
return
|
||||
}
|
||||
if (routed === adapter) {
|
||||
this.sessionAdapters.delete(payload.id)
|
||||
}
|
||||
for (const listener of this.exitListeners) {
|
||||
listener(payload)
|
||||
}
|
||||
@@ -97,15 +107,8 @@ export class DaemonPtyRouter implements IPtyProvider {
|
||||
|
||||
async shutdown(id: string, opts: PtyShutdownOptions): Promise<void> {
|
||||
await this.adapterFor(id).shutdown(id, opts)
|
||||
// Why: sleep passes keepHistory=true and re-spawns against the same
|
||||
// sessionId on wake. If we delete the routing entry here, adapterFor()
|
||||
// falls back to `this.current` on wake — for a session that originally
|
||||
// lived on a legacy adapter (different protocolVersion), the wake-side
|
||||
// createOrAttach lands on the wrong adapter and creates a fresh session,
|
||||
// losing the cold-restore from the legacy adapter's history dir.
|
||||
if (!opts.keepHistory) {
|
||||
this.sessionAdapters.delete(id)
|
||||
}
|
||||
// Why: native shutdown acceptance is not exit proof. onExit or a later
|
||||
// successful listProcesses readback retires the exact adapter route.
|
||||
}
|
||||
|
||||
async sendSignal(id: string, signal: string): Promise<void> {
|
||||
@@ -162,8 +165,7 @@ export class DaemonPtyRouter implements IPtyProvider {
|
||||
async listProcesses(): Promise<PtyProcessInfo[]> {
|
||||
// Why: runtime exact-stop/liveness flows must fail closed if any adapter
|
||||
// cannot provide a trustworthy process list.
|
||||
const results = await Promise.all(this.allAdapters().map((adapter) => adapter.listProcesses()))
|
||||
return results.flat()
|
||||
return listProviderProcessesAndReconcileRoutes(this.allAdapters(), this.sessionAdapters)
|
||||
}
|
||||
|
||||
async getDefaultShell(): Promise<string> {
|
||||
|
||||
@@ -109,6 +109,25 @@ function createDaemonAdapter(
|
||||
}
|
||||
|
||||
describe('DegradedDaemonPtyProvider', () => {
|
||||
it('retains the exact child route until exit proof', async () => {
|
||||
const daemonSessions = ['shared-session']
|
||||
const current = createDaemonAdapter('daemon', daemonSessions)
|
||||
const fallback = createProvider('fallback')
|
||||
vi.mocked(current.shutdown).mockResolvedValueOnce(undefined)
|
||||
const provider = new DegradedDaemonPtyProvider({ current, legacy: [], fallback })
|
||||
await provider.discoverDaemonSessions()
|
||||
|
||||
expect(provider.requiresShutdownExitProof).toBe(true)
|
||||
await provider.shutdown('shared-session', { immediate: true })
|
||||
provider.write('shared-session', 'before-exit')
|
||||
|
||||
expect(current.write).toHaveBeenCalledWith('shared-session', 'before-exit')
|
||||
daemonSessions.length = 0
|
||||
current.emitExit('shared-session', 0)
|
||||
provider.write('shared-session', 'after-exit')
|
||||
expect(fallback.write).toHaveBeenCalledWith('shared-session', 'after-exit')
|
||||
})
|
||||
|
||||
it('routes fresh foreground confirmation to the session owner', async () => {
|
||||
const current = createDaemonAdapter('daemon', ['daemon-session'])
|
||||
const fallback = createProvider('fallback')
|
||||
|
||||
@@ -1,5 +1,6 @@
|
||||
import type { DaemonPtyAdapter } from './daemon-pty-adapter'
|
||||
import { shutdownDegradedFallbackSessions } from './degraded-daemon-fallback-shutdown'
|
||||
import * as PtyRoutes from './pty-provider-route-reconciliation'
|
||||
import type {
|
||||
IPtyProvider,
|
||||
PtyBackgroundStreamEvent,
|
||||
@@ -15,12 +16,12 @@ type ManagedPtyProvider = IPtyProvider & {
|
||||
}
|
||||
|
||||
export class DegradedDaemonPtyProvider implements IPtyProvider {
|
||||
readonly requiresShutdownExitProof = true
|
||||
readonly routesFreshSpawnsToLocalProvider = true
|
||||
// Why: the preserved daemon answers protocol but cannot spawn fresh PTYs.
|
||||
// Surfaced (e.g. via pty:management:listSessions) so the UI can warn that
|
||||
// new terminals are running without daemon persistence until a restart.
|
||||
readonly isDegraded = true
|
||||
|
||||
private current: DaemonPtyAdapter
|
||||
private legacy: DaemonPtyAdapter[]
|
||||
private fallback: ManagedPtyProvider
|
||||
@@ -50,7 +51,15 @@ export class DegradedDaemonPtyProvider implements IPtyProvider {
|
||||
}
|
||||
}),
|
||||
provider.onExit((payload) => {
|
||||
this.sessionProviders.delete(payload.id)
|
||||
// Why: a late exit from an old daemon must not delete a route that
|
||||
// has already been rebound to the local fallback or another daemon.
|
||||
const routed = this.sessionProviders.get(payload.id)
|
||||
if (routed && routed !== provider) {
|
||||
return
|
||||
}
|
||||
if (routed === provider) {
|
||||
this.sessionProviders.delete(payload.id)
|
||||
}
|
||||
for (const listener of this.exitListeners) {
|
||||
listener(payload)
|
||||
}
|
||||
@@ -114,9 +123,8 @@ export class DegradedDaemonPtyProvider implements IPtyProvider {
|
||||
|
||||
async shutdown(id: string, opts: { immediate?: boolean; keepHistory?: boolean }): Promise<void> {
|
||||
await this.providerFor(id).shutdown(id, opts)
|
||||
if (!opts.keepHistory) {
|
||||
this.sessionProviders.delete(id)
|
||||
}
|
||||
// Why: shutdown acceptance is not physical-exit proof. Keep routing until
|
||||
// the owning provider emits onExit or listProcesses proves the id absent.
|
||||
}
|
||||
|
||||
async sendSignal(id: string, signal: string): Promise<void> {
|
||||
@@ -173,10 +181,10 @@ export class DegradedDaemonPtyProvider implements IPtyProvider {
|
||||
}
|
||||
|
||||
async listProcesses(): Promise<PtyProcessInfo[]> {
|
||||
const results = await Promise.all(
|
||||
this.allProviders().map((provider) => provider.listProcesses())
|
||||
return PtyRoutes.listProviderProcessesAndReconcileRoutes(
|
||||
this.allProviders(),
|
||||
this.sessionProviders
|
||||
)
|
||||
return results.flat()
|
||||
}
|
||||
|
||||
async getDefaultShell(): Promise<string> {
|
||||
@@ -286,7 +294,7 @@ export class DegradedDaemonPtyProvider implements IPtyProvider {
|
||||
}
|
||||
|
||||
getCurrentDaemonSessionIds(): string[] {
|
||||
return this.sessionIdsForProvider(this.current)
|
||||
return PtyRoutes.providerSessionIds(this.sessionProviders, this.current)
|
||||
}
|
||||
|
||||
fanoutCurrentDaemonSyntheticExits(code: number): void {
|
||||
@@ -336,12 +344,6 @@ export class DegradedDaemonPtyProvider implements IPtyProvider {
|
||||
return null
|
||||
}
|
||||
|
||||
private sessionIdsForProvider(provider: ManagedPtyProvider): string[] {
|
||||
return [...this.sessionProviders]
|
||||
.filter(([, mappedProvider]) => mappedProvider === provider)
|
||||
.map(([id]) => id)
|
||||
}
|
||||
|
||||
private daemonAdapterFor(sessionId: string): DaemonPtyAdapter | null {
|
||||
const provider = this.sessionProviders.get(sessionId)
|
||||
return provider && this.allDaemonAdapters().includes(provider as DaemonPtyAdapter)
|
||||
|
||||
@@ -0,0 +1,26 @@
|
||||
import type { IPtyProvider, PtyProcessInfo } from '../providers/types'
|
||||
|
||||
export async function listProviderProcessesAndReconcileRoutes<T extends IPtyProvider>(
|
||||
providers: readonly T[],
|
||||
routes: Map<string, T>
|
||||
): Promise<PtyProcessInfo[]> {
|
||||
const routesAtStart = [...routes]
|
||||
const listings = await Promise.all(
|
||||
providers.map(async (provider) => ({ provider, sessions: await provider.listProcesses() }))
|
||||
)
|
||||
const liveIdsByProvider = new Map(
|
||||
listings.map(({ provider, sessions }) => [provider, new Set(sessions.map(({ id }) => id))])
|
||||
)
|
||||
for (const [id, provider] of routesAtStart) {
|
||||
// Why: spawn/exit can mutate routing while remote listings are in flight.
|
||||
// Never let that older snapshot delete a route rebound to another owner.
|
||||
if (routes.get(id) === provider && !liveIdsByProvider.get(provider)?.has(id)) {
|
||||
routes.delete(id)
|
||||
}
|
||||
}
|
||||
return listings.flatMap(({ sessions }) => sessions)
|
||||
}
|
||||
|
||||
export function providerSessionIds<T>(routes: Map<string, T>, provider: T): string[] {
|
||||
return [...routes].filter(([, routedProvider]) => routedProvider === provider).map(([id]) => id)
|
||||
}
|
||||
@@ -6679,6 +6679,7 @@ describe('registerPtyHandlers', () => {
|
||||
})
|
||||
|
||||
it('cleans up fresh runtime-owned SSH spawns when binding persistence fails', async () => {
|
||||
vi.useFakeTimers()
|
||||
type RuntimeSpawnController = {
|
||||
spawn(args: {
|
||||
cols: number
|
||||
@@ -6692,7 +6693,10 @@ describe('registerPtyHandlers', () => {
|
||||
}): Promise<{ id: string }>
|
||||
}
|
||||
const appPtyId = 'ssh:ssh-fresh-fail@@relay-pty'
|
||||
const remoteShutdown = vi.fn()
|
||||
const remoteShutdown = vi
|
||||
.fn()
|
||||
.mockRejectedValueOnce(new Error('relay cleanup unavailable'))
|
||||
.mockResolvedValueOnce(undefined)
|
||||
registerSshPtyProvider('ssh-fresh-fail', {
|
||||
spawn: vi.fn(async () => ({ id: appPtyId })),
|
||||
write: vi.fn(),
|
||||
@@ -6720,7 +6724,8 @@ describe('registerPtyHandlers', () => {
|
||||
throw new Error('disk full')
|
||||
}),
|
||||
removeSshRemotePtyLease: vi.fn(),
|
||||
markSshRemotePtyLease: vi.fn()
|
||||
markSshRemotePtyLease: vi.fn(),
|
||||
markSshRemotePtyShutdownRequested: vi.fn(() => true)
|
||||
}
|
||||
let controller: RuntimeSpawnController | null = null
|
||||
const runtime = {
|
||||
@@ -6761,9 +6766,24 @@ describe('registerPtyHandlers', () => {
|
||||
})
|
||||
).rejects.toThrow(/ORCA_TERMINAL_SESSION_STATE_SAVE_FAILED/)
|
||||
|
||||
expect(remoteShutdown).toHaveBeenCalledWith(appPtyId, { immediate: true })
|
||||
expect(store.upsertSshRemotePtyLease).not.toHaveBeenCalled()
|
||||
expect(remoteShutdown).toHaveBeenCalledWith(appPtyId, {
|
||||
immediate: true,
|
||||
expectedPaneKey: makePaneKey('tab-remote', leafId),
|
||||
expectedTabId: 'tab-remote'
|
||||
})
|
||||
expect(store.upsertSshRemotePtyLease).toHaveBeenCalled()
|
||||
expect(store.removeSshRemotePtyLease).not.toHaveBeenCalled()
|
||||
expect(openCodeClearPtyMock).not.toHaveBeenCalledWith(appPtyId)
|
||||
expect(piClearPtyMock).not.toHaveBeenCalledWith(appPtyId)
|
||||
|
||||
await vi.advanceTimersByTimeAsync(250)
|
||||
|
||||
expect(remoteShutdown).toHaveBeenCalledTimes(2)
|
||||
expect(store.markSshRemotePtyLease).toHaveBeenCalledWith(
|
||||
'ssh-fresh-fail',
|
||||
appPtyId,
|
||||
'terminated'
|
||||
)
|
||||
expect(openCodeClearPtyMock).toHaveBeenCalledWith(appPtyId)
|
||||
expect(piClearPtyMock).toHaveBeenCalledWith(appPtyId)
|
||||
} finally {
|
||||
@@ -6771,6 +6791,78 @@ describe('registerPtyHandlers', () => {
|
||||
}
|
||||
})
|
||||
|
||||
it('retains renderer SSH cleanup when binding persistence and first shutdown fail', async () => {
|
||||
vi.useFakeTimers()
|
||||
const appPtyId = 'ssh:ssh-1@@renderer-persist-fail'
|
||||
const remoteShutdown = vi
|
||||
.fn()
|
||||
.mockRejectedValueOnce(new Error('relay cleanup unavailable'))
|
||||
.mockResolvedValueOnce(undefined)
|
||||
registerSshPtyProvider('ssh-1', {
|
||||
spawn: vi.fn(async () => ({ id: appPtyId })),
|
||||
write: vi.fn(),
|
||||
resize: vi.fn(),
|
||||
shutdown: remoteShutdown,
|
||||
sendSignal: vi.fn(),
|
||||
getCwd: vi.fn(),
|
||||
getInitialCwd: vi.fn(),
|
||||
clearBuffer: vi.fn(),
|
||||
acknowledgeDataEvent: vi.fn(),
|
||||
onData: vi.fn(() => () => {}),
|
||||
onReplay: vi.fn(() => () => {}),
|
||||
onExit: vi.fn(() => () => {}),
|
||||
listProcesses: vi.fn(async () => []),
|
||||
hasChildProcesses: vi.fn(),
|
||||
getForegroundProcess: vi.fn(),
|
||||
serialize: vi.fn(),
|
||||
revive: vi.fn(),
|
||||
getDefaultShell: vi.fn(),
|
||||
getProfiles: vi.fn()
|
||||
} as never)
|
||||
const store = {
|
||||
upsertSshRemotePtyLease: vi.fn(),
|
||||
persistPtyBinding: vi.fn(() => {
|
||||
throw new Error('disk full')
|
||||
}),
|
||||
markSshRemotePtyShutdownRequested: vi.fn(() => true),
|
||||
markSshRemotePtyLease: vi.fn(),
|
||||
removeSshRemotePtyLease: vi.fn()
|
||||
}
|
||||
registerPtyHandlers(
|
||||
mainWindow as never,
|
||||
undefined,
|
||||
undefined,
|
||||
undefined,
|
||||
undefined,
|
||||
store as never
|
||||
)
|
||||
const leafId = '11111111-1111-4111-8111-111111111111'
|
||||
const paneKey = makePaneKey('tab-renderer', leafId)
|
||||
|
||||
await expect(
|
||||
handlers.get('pty:spawn')!(null, {
|
||||
cols: 80,
|
||||
rows: 24,
|
||||
connectionId: 'ssh-1',
|
||||
worktreeId: 'wt-remote',
|
||||
tabId: 'tab-renderer',
|
||||
leafId,
|
||||
env: { ORCA_PANE_KEY: paneKey }
|
||||
})
|
||||
).rejects.toThrow(/ORCA_TERMINAL_SESSION_STATE_SAVE_FAILED/)
|
||||
|
||||
expect(remoteShutdown).toHaveBeenCalledWith(appPtyId, {
|
||||
immediate: true,
|
||||
expectedPaneKey: paneKey,
|
||||
expectedTabId: 'tab-renderer'
|
||||
})
|
||||
expect(openCodeClearPtyMock).not.toHaveBeenCalledWith(appPtyId)
|
||||
await vi.advanceTimersByTimeAsync(250)
|
||||
expect(remoteShutdown).toHaveBeenCalledTimes(2)
|
||||
expect(store.markSshRemotePtyLease).toHaveBeenCalledWith('ssh-1', appPtyId, 'terminated')
|
||||
expect(openCodeClearPtyMock).toHaveBeenCalledWith(appPtyId)
|
||||
})
|
||||
|
||||
it('maps runtime-owned spawn paneKeys for renderer serializer settlement', async () => {
|
||||
type RuntimeSpawnController = {
|
||||
spawn(args: {
|
||||
|
||||
+37
-10
@@ -856,6 +856,21 @@ function retainSshPtyShutdown(
|
||||
return retained
|
||||
}
|
||||
|
||||
async function retainFailedSpawnCleanup(
|
||||
id: string,
|
||||
connectionId: string | null | undefined,
|
||||
provider: IPtyProvider,
|
||||
options: PtyShutdownOptions
|
||||
): Promise<void> {
|
||||
const retained = connectionId
|
||||
? retainSshPtyShutdown(id, connectionId, provider, options)
|
||||
: retainLocalPtyShutdown(id, options)
|
||||
if (!retained) {
|
||||
throw new Error('PTY cleanup could not retain shutdown ownership')
|
||||
}
|
||||
await attemptRetainedPtyShutdownForCaller(retained)
|
||||
}
|
||||
|
||||
// ─── Host PTY env assembly ──────────────────────────────────────────
|
||||
// Why: both the LocalPtyProvider.buildSpawnEnv closure and the daemon-active
|
||||
// fallback in pty:spawn need the same set of host-local env injections
|
||||
@@ -3690,14 +3705,27 @@ export function registerPtyHandlers(
|
||||
})
|
||||
} catch (err) {
|
||||
console.error('[pty] failed to persist runtime PTY binding after spawn:', err)
|
||||
deletePtyOwnership(result.id)
|
||||
if (!result.isReattach) {
|
||||
if (result.isReattach) {
|
||||
// Why: the caller rejected this attach and must not keep routing
|
||||
// writes, but the pre-existing provider session remains owned.
|
||||
deletePtyOwnership(result.id)
|
||||
} else {
|
||||
if (args.connectionId) {
|
||||
// Why: the binding failed, so the SSH lease is the only durable
|
||||
// identity that can carry cleanup across an app restart.
|
||||
try {
|
||||
persistSshLease()
|
||||
} catch {}
|
||||
}
|
||||
try {
|
||||
await provider.shutdown(result.id, { immediate: true })
|
||||
await retainFailedSpawnCleanup(result.id, args.connectionId, provider, {
|
||||
immediate: true,
|
||||
...(metadataPaneKey ? { expectedPaneKey: metadataPaneKey } : {}),
|
||||
...(typeof args.tabId === 'string' ? { expectedTabId: args.tabId } : {})
|
||||
})
|
||||
} catch (shutdownErr) {
|
||||
console.warn('[pty] failed to clean up PTY after persistence failure:', shutdownErr)
|
||||
}
|
||||
clearProviderPtyState(result.id)
|
||||
}
|
||||
throw new Error(createTerminalSessionStateSaveFailureMessage())
|
||||
}
|
||||
@@ -4697,18 +4725,17 @@ export function registerPtyHandlers(
|
||||
console.error('[pty] failed to persist PTY binding after spawn:', err)
|
||||
if (!result.isReattach) {
|
||||
try {
|
||||
await provider.shutdown(result.id, { immediate: true })
|
||||
await retainFailedSpawnCleanup(result.id, args.connectionId, provider, {
|
||||
immediate: true,
|
||||
...(validatedPaneKey ? { expectedPaneKey: validatedPaneKey } : {}),
|
||||
...(typeof args.tabId === 'string' ? { expectedTabId: args.tabId } : {})
|
||||
})
|
||||
} 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)
|
||||
}
|
||||
throw new Error(createTerminalSessionStateSaveFailureMessage())
|
||||
}
|
||||
|
||||
@@ -18,7 +18,8 @@ import {
|
||||
bufferPreHandlerPtyExit,
|
||||
clearPreHandlerPtyState,
|
||||
drainPreHandlerPtyData,
|
||||
drainPreHandlerPtyExit
|
||||
drainPreHandlerPtyExit,
|
||||
reconcilePreHandlerPtyExitAfterOverflow
|
||||
} from './pty-pre-handler-buffer'
|
||||
import {
|
||||
clearReceivedPtyCharTotal,
|
||||
@@ -392,7 +393,14 @@ export function registerEagerPtyBuffer(
|
||||
// caller receives that handle before onExit fires.
|
||||
queueMicrotask(() => {
|
||||
if (ptyExitHandlers.get(ptyId) === exitHandler) {
|
||||
drainPreHandlerPtyExit(ptyId, exitHandler)
|
||||
if (!drainPreHandlerPtyExit(ptyId, exitHandler)) {
|
||||
reconcilePreHandlerPtyExitAfterOverflow(
|
||||
ptyId,
|
||||
window.api.pty.hasPty,
|
||||
exitHandler,
|
||||
() => ptyExitHandlers.get(ptyId) === exitHandler
|
||||
)
|
||||
}
|
||||
} else {
|
||||
clearPreHandlerPtyState(ptyId)
|
||||
}
|
||||
|
||||
@@ -4,7 +4,8 @@ import {
|
||||
bufferPreHandlerPtyExit,
|
||||
clearPreHandlerPtyState,
|
||||
drainPreHandlerPtyData,
|
||||
drainPreHandlerPtyExit
|
||||
drainPreHandlerPtyExit,
|
||||
reconcilePreHandlerPtyExitAfterOverflow
|
||||
} from './pty-pre-handler-buffer'
|
||||
|
||||
const RESCAN_PTY_ID = 'pty-pre-handler-rescan'
|
||||
@@ -90,4 +91,19 @@ describe('pre-handler PTY buffer', () => {
|
||||
expect(oldest).not.toHaveBeenCalled()
|
||||
expect(newest).toHaveBeenCalledWith(64)
|
||||
})
|
||||
|
||||
it('reconciles an evicted pending exit with one targeted liveness readback', async () => {
|
||||
for (let index = 0; index <= 64; index += 1) {
|
||||
bufferPreHandlerPtyExit(`pty-exit-${index}`, index)
|
||||
}
|
||||
const handler = vi.fn()
|
||||
const hasPty = vi.fn(async () => false)
|
||||
|
||||
reconcilePreHandlerPtyExitAfterOverflow('pty-exit-0', hasPty, handler, () => true)
|
||||
await Promise.resolve()
|
||||
|
||||
expect(hasPty).toHaveBeenCalledOnce()
|
||||
expect(hasPty).toHaveBeenCalledWith('pty-exit-0')
|
||||
expect(handler).toHaveBeenCalledWith(-1)
|
||||
})
|
||||
})
|
||||
|
||||
@@ -22,6 +22,7 @@ const preHandlerPtyExit = new Map<string, number>()
|
||||
const PRE_HANDLER_PTY_DATA_MAX_BYTES = 512 * 1024
|
||||
const PRE_HANDLER_PTY_DATA_MAX_PTYS = 64
|
||||
const PRE_HANDLER_PTY_EXIT_MAX_PTYS = 64
|
||||
let preHandlerPtyExitOverflowed = false
|
||||
// Why: legit pre-attach windows drain within milliseconds and hold little
|
||||
// data. Sustained accumulation means a pane lost its data handler (the
|
||||
// frozen-pane detach/attach race) — leave a breadcrumb for trace capture.
|
||||
@@ -96,18 +97,40 @@ export function bufferPreHandlerPtyExit(ptyId: string, code: number): void {
|
||||
const oldestPtyId = preHandlerPtyExit.keys().next().value
|
||||
if (typeof oldestPtyId === 'string') {
|
||||
preHandlerPtyExit.delete(oldestPtyId)
|
||||
preHandlerPtyExitOverflowed = true
|
||||
}
|
||||
}
|
||||
preHandlerPtyExit.set(ptyId, code)
|
||||
}
|
||||
|
||||
export function drainPreHandlerPtyExit(ptyId: string, handler: (code: number) => void): void {
|
||||
export function drainPreHandlerPtyExit(ptyId: string, handler: (code: number) => void): boolean {
|
||||
const code = preHandlerPtyExit.get(ptyId)
|
||||
if (code === undefined) {
|
||||
return
|
||||
return false
|
||||
}
|
||||
preHandlerPtyExit.delete(ptyId)
|
||||
handler(code)
|
||||
return true
|
||||
}
|
||||
|
||||
export function reconcilePreHandlerPtyExitAfterOverflow(
|
||||
ptyId: string,
|
||||
hasPty: ((id: string) => Promise<boolean | null>) | undefined,
|
||||
handler: (code: number) => void,
|
||||
isCurrent: () => boolean
|
||||
): void {
|
||||
if (!preHandlerPtyExitOverflowed || !hasPty) {
|
||||
return
|
||||
}
|
||||
// Why: the bounded exit buffer may evict a legitimate pre-registration exit.
|
||||
// One targeted readback after overflow recovers truth without retaining IDs.
|
||||
void hasPty(ptyId)
|
||||
.then((alive) => {
|
||||
if (alive === false && isCurrent()) {
|
||||
handler(-1)
|
||||
}
|
||||
})
|
||||
.catch(() => {})
|
||||
}
|
||||
|
||||
export function clearPreHandlerPtyData(ptyId: string): void {
|
||||
|
||||
@@ -42,6 +42,7 @@ describe('createIpcPtyTransport', () => {
|
||||
writeAccepted: vi.fn().mockResolvedValue(true),
|
||||
resize: vi.fn(),
|
||||
kill: vi.fn().mockResolvedValue(undefined),
|
||||
hasPty: vi.fn().mockResolvedValue(true),
|
||||
onData: vi.fn((callback: (payload: { id: string; data: string }) => void) => {
|
||||
onData = callback
|
||||
return () => {}
|
||||
@@ -932,6 +933,24 @@ describe('createIpcPtyTransport', () => {
|
||||
expect(onEagerExit).toHaveBeenCalledWith('pty-fast-setup', 1)
|
||||
})
|
||||
|
||||
it('reconciles an exit evicted by the bounded pre-handler buffer', async () => {
|
||||
const { ensurePtyDispatcher, registerEagerPtyBuffer } = await import('./pty-transport')
|
||||
const hasPty = window.api.pty.hasPty as unknown as ReturnType<typeof vi.fn>
|
||||
const onEagerExit = vi.fn()
|
||||
hasPty.mockResolvedValue(false)
|
||||
ensurePtyDispatcher()
|
||||
for (let index = 0; index <= 64; index += 1) {
|
||||
onExit?.({ id: `pty-overflow-${index}`, code: index })
|
||||
}
|
||||
|
||||
registerEagerPtyBuffer('pty-overflow-0', onEagerExit)
|
||||
await Promise.resolve()
|
||||
await Promise.resolve()
|
||||
|
||||
expect(hasPty).toHaveBeenCalledWith('pty-overflow-0')
|
||||
expect(onEagerExit).toHaveBeenCalledWith('pty-overflow-0', -1)
|
||||
})
|
||||
|
||||
it('enforces the eager buffer cap in UTF-8 bytes for multi-byte output', async () => {
|
||||
const { registerEagerPtyBuffer } = await import('./pty-transport')
|
||||
const cap = 512 * 1024
|
||||
|
||||
@@ -22,7 +22,11 @@ import {
|
||||
ensurePtyDispatcher,
|
||||
getEagerPtyBufferHandle
|
||||
} from './pty-dispatcher'
|
||||
import { drainPreHandlerPtyData, drainPreHandlerPtyExit } from './pty-pre-handler-buffer'
|
||||
import {
|
||||
drainPreHandlerPtyData,
|
||||
drainPreHandlerPtyExit,
|
||||
reconcilePreHandlerPtyExitAfterOverflow
|
||||
} from './pty-pre-handler-buffer'
|
||||
import { createPtyInputWriteQueue } from './pty-input-write-queue'
|
||||
import type { PtyDataMeta } from './pty-dispatcher'
|
||||
import type { IpcPtyTransportOptions, PtyConnectResult, PtyTransport } from './pty-transport-types'
|
||||
@@ -682,7 +686,14 @@ export function createIpcPtyTransport(opts: IpcPtyTransportOptions = {}): PtyTra
|
||||
// would otherwise fire stale notifications after the data handler
|
||||
// is removed but before the exit event arrives.
|
||||
ptyTeardownHandlers.set(id, clearAccumulatedState)
|
||||
drainPreHandlerPtyExit(id, exitHandler)
|
||||
if (!drainPreHandlerPtyExit(id, exitHandler)) {
|
||||
reconcilePreHandlerPtyExitAfterOverflow(
|
||||
id,
|
||||
window.api.pty.hasPty,
|
||||
exitHandler,
|
||||
() => ownedExitHandlers.get(id) === exitHandler
|
||||
)
|
||||
}
|
||||
}
|
||||
|
||||
return {
|
||||
|
||||
Reference in New Issue
Block a user