mirror of
https://github.com/stablyai/orca.git
synced 2026-10-19 16:03:26 +00:00
fix(daemon): require authoritative recovery liveness
This commit is contained in:
@@ -75,3 +75,31 @@ it('rejects a degraded existing spawn after its selected daemon is replaced', as
|
||||
|
||||
await expect(spawning).rejects.toThrow('daemon_session_routing_unavailable')
|
||||
})
|
||||
|
||||
it('rejects an existing fallback spawn when daemon ownership appears before its reply', async () => {
|
||||
const sessionId = 'fallback-owner-race'
|
||||
const current = createDaemonAdapter('daemon')
|
||||
const fallback = createProvider('fallback')
|
||||
const provider = new DegradedDaemonPtyProvider({ current, legacy: [], fallback })
|
||||
await provider.spawn({ sessionId, isNewSession: true, cols: 80, rows: 24 })
|
||||
let resolveSpawn!: (result: PtySpawnResult) => void
|
||||
let markSpawnStarted!: () => void
|
||||
const spawnStarted = new Promise<void>((resolve) => {
|
||||
markSpawnStarted = resolve
|
||||
})
|
||||
vi.mocked(fallback.spawn).mockImplementationOnce(
|
||||
() =>
|
||||
new Promise((resolve) => {
|
||||
markSpawnStarted()
|
||||
resolveSpawn = resolve
|
||||
})
|
||||
)
|
||||
|
||||
const spawning = provider.spawn({ sessionId, cols: 80, rows: 24 })
|
||||
await spawnStarted
|
||||
current.emitData(sessionId, 'daemon ownership')
|
||||
resolveSpawn({ id: sessionId })
|
||||
|
||||
await expect(spawning).rejects.toThrow('daemon_session_routing_unavailable')
|
||||
expect(() => provider.write(sessionId, 'blocked')).toThrow('daemon_session_routing_unavailable')
|
||||
})
|
||||
|
||||
@@ -75,6 +75,7 @@ export class DegradedDaemonSessionRouting {
|
||||
if (!this.fallbackSpawns.hasExited(result.id)) {
|
||||
this.fallbackRoutes.recordSpawnedSession(result.id, target)
|
||||
}
|
||||
this.fallbackRoutes.assertExistingSpawnAvailable(opts, result.id)
|
||||
} else {
|
||||
this.fallbackRoutes.recordDaemonSpawn(result.id, opts.sessionId, target as DaemonPtyAdapter)
|
||||
}
|
||||
|
||||
@@ -1,6 +1,6 @@
|
||||
import type { IPtyProvider } from '../providers/types'
|
||||
import type { IPtyProvider, PtySpawnOptions } from '../providers/types'
|
||||
import type { DaemonPtyAdapter } from './daemon-pty-adapter'
|
||||
import { sameDaemonIncarnation } from './daemon-session-route'
|
||||
import { DaemonSessionOwnerUnknownError, sameDaemonIncarnation } from './daemon-session-route'
|
||||
import type { DaemonSessionRouteTable } from './daemon-session-route-table'
|
||||
import { recordValidatedDaemonSpawn } from './daemon-spawn-route-validation'
|
||||
import type { DegradedFallbackSpawnRoutes } from './degraded-fallback-spawn-routes'
|
||||
@@ -44,6 +44,19 @@ export class DegradedFallbackSessionRoutes {
|
||||
}
|
||||
}
|
||||
|
||||
assertExistingSpawnAvailable(opts: PtySpawnOptions, resultSessionId: string): void {
|
||||
if (!opts.sessionId || opts.isNewSession) {
|
||||
return
|
||||
}
|
||||
const hasDaemonOwner = [opts.sessionId, resultSessionId].some((sessionId) => {
|
||||
const route = this.daemonRoutes.get(sessionId)
|
||||
return route !== undefined && route.state !== 'unavailable'
|
||||
})
|
||||
if (hasDaemonOwner) {
|
||||
throw new DaemonSessionOwnerUnknownError(opts.sessionId)
|
||||
}
|
||||
}
|
||||
|
||||
recordDaemonSpawn(
|
||||
resultSessionId: string,
|
||||
requestedSessionId: string | undefined,
|
||||
|
||||
@@ -6299,11 +6299,14 @@ describe('registerPtyHandlers', () => {
|
||||
})
|
||||
|
||||
it('checks single-PTY liveness without listing every session', async () => {
|
||||
const hasPty = vi.fn((id: string) => id === 'live-pty')
|
||||
const probePtyLiveness = vi.fn(async (id: string) => (id === 'live-pty' ? true : null))
|
||||
const hasPty = vi.fn((_id: string): boolean => {
|
||||
throw new Error('cached ownership is not authoritative')
|
||||
})
|
||||
const listProcesses = vi.fn(async () => {
|
||||
throw new Error('listProcesses should not be called')
|
||||
})
|
||||
setLocalPtyProvider({
|
||||
const provider = {
|
||||
spawn: vi.fn(),
|
||||
write: vi.fn(),
|
||||
resize: vi.fn(),
|
||||
@@ -6323,16 +6326,27 @@ describe('registerPtyHandlers', () => {
|
||||
listProcesses,
|
||||
attach: vi.fn(),
|
||||
hasPty,
|
||||
probePtyLiveness,
|
||||
getDefaultShell: vi.fn(),
|
||||
getProfiles: vi.fn()
|
||||
} as never)
|
||||
}
|
||||
setLocalPtyProvider(provider as never)
|
||||
registerPtyHandlers(mainWindow as never)
|
||||
|
||||
await expect(handlers.get('pty:hasPty')!(null, { id: 'live-pty' })).resolves.toBe(true)
|
||||
await expect(handlers.get('pty:hasPty')!(null, { id: 'dead-pty' })).resolves.toBe(false)
|
||||
await expect(handlers.get('pty:hasPty')!(null, { id: 'uncertain-pty' })).resolves.toBe(null)
|
||||
|
||||
expect(hasPty).toHaveBeenCalledWith('live-pty')
|
||||
expect(hasPty).toHaveBeenCalledWith('dead-pty')
|
||||
expect(probePtyLiveness).toHaveBeenCalledTimes(2)
|
||||
expect(probePtyLiveness).toHaveBeenNthCalledWith(1, 'live-pty')
|
||||
expect(probePtyLiveness).toHaveBeenNthCalledWith(2, 'uncertain-pty')
|
||||
expect(hasPty).not.toHaveBeenCalled()
|
||||
expect(listProcesses).not.toHaveBeenCalled()
|
||||
|
||||
provider.probePtyLiveness = undefined as never
|
||||
hasPty.mockImplementation((id: string) => id === 'cached-live-pty')
|
||||
await expect(handlers.get('pty:hasPty')!(null, { id: 'cached-pty' })).resolves.toBe(false)
|
||||
|
||||
expect(hasPty).toHaveBeenCalledExactlyOnceWith('cached-pty')
|
||||
expect(listProcesses).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
|
||||
+5
-2
@@ -6163,11 +6163,14 @@ export function registerPtyHandlers(
|
||||
const provider = parsedSshId
|
||||
? sshProviders.get(parsedSshId.connectionId)
|
||||
: tryGetProviderForPty(args.id)
|
||||
if (!provider?.hasPty) {
|
||||
if (!provider) {
|
||||
return null
|
||||
}
|
||||
try {
|
||||
return provider.hasPty(args.id)
|
||||
const live = provider.probePtyLiveness
|
||||
? await provider.probePtyLiveness(args.id)
|
||||
: provider.hasPty?.(args.id)
|
||||
return typeof live === 'boolean' ? live : null
|
||||
} catch {
|
||||
// Why: liveness is only allowed to close panes on an authoritative false.
|
||||
return null
|
||||
|
||||
Reference in New Issue
Block a user