mirror of
https://github.com/stablyai/orca.git
synced 2026-10-19 16:03:26 +00:00
fix(daemon): close exact-head routing gaps
This commit is contained in:
@@ -90,6 +90,7 @@ describe('DaemonPtyRouter routing safety', () => {
|
||||
await router.discoverLegacySessions()
|
||||
await router.shutdown(sessionId, { immediate: true, keepHistory: true })
|
||||
expect(router.getSessionRouteState(sessionId)).toBe('unavailable')
|
||||
expect(router.supportsGitCredentialGuardHost(sessionId)).toBe(false)
|
||||
|
||||
const spawning = router.spawn({ sessionId, cols: 80, rows: 24 })
|
||||
current.emitData(sessionId, 'restored frame')
|
||||
|
||||
@@ -382,6 +382,22 @@ describe('DaemonPtyRouter', () => {
|
||||
expect(router.supportsGitCredentialGuardHost('legacy-session')).toBe(false)
|
||||
})
|
||||
|
||||
it('keeps an unresolved guard-host hint conservative until async owner resolution', async () => {
|
||||
const sessionId = 'preserved-current-session'
|
||||
const current = createAdapter('current')
|
||||
const legacy = createAdapter('legacy')
|
||||
vi.mocked(current.probePtyLiveness).mockResolvedValue(true)
|
||||
const router = new DaemonPtyRouter({ current, legacy: [legacy] })
|
||||
|
||||
expect(router.supportsGitCredentialGuardHost(sessionId)).toBe(false)
|
||||
await expect(router.spawn({ sessionId, cols: 80, rows: 24 })).resolves.toEqual({
|
||||
id: sessionId
|
||||
})
|
||||
|
||||
expect(current.spawn).toHaveBeenCalledExactlyOnceWith({ sessionId, cols: 80, rows: 24 })
|
||||
expect(legacy.spawn).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'])
|
||||
|
||||
@@ -74,7 +74,8 @@ export class DaemonPtyRouter implements IPtyProvider {
|
||||
}
|
||||
|
||||
supportsGitCredentialGuardHost(sessionId?: string): boolean {
|
||||
return this.routing.ownerOrCurrent(sessionId).supportsGitCredentialGuardHost()
|
||||
const owner = sessionId ? this.routing.ownerForHint(sessionId) : this.current
|
||||
return owner?.supportsGitCredentialGuardHost() ?? false
|
||||
}
|
||||
|
||||
supportsAgentSessionClaims(): boolean {
|
||||
|
||||
@@ -622,9 +622,54 @@ describe('DegradedDaemonPtyProvider', () => {
|
||||
expect(current.write).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it('releases an in-flight fallback when the colliding daemon changes incarnation', async () => {
|
||||
const sessionId = 'in-flight-incarnation-collision'
|
||||
const previous = { pid: 1, startedAtMs: 1, launchNonce: 'previous' }
|
||||
const replacement = { pid: 2, startedAtMs: 2, launchNonce: 'replacement' }
|
||||
const current = createDaemonAdapter('daemon')
|
||||
const fallback = createProvider('fallback')
|
||||
let identity = previous
|
||||
let notifyIdentityChange!: (event: DaemonIdentityChangeEvent) => void
|
||||
vi.mocked(current.getLastAuthenticatedDaemonIdentity).mockImplementation(() => identity)
|
||||
vi.mocked(current.onDaemonIdentityChanged).mockImplementation((listener) => {
|
||||
notifyIdentityChange = listener
|
||||
return () => {}
|
||||
})
|
||||
let resolveSpawn!: (result: PtySpawnResult) => void
|
||||
vi.mocked(fallback.spawn).mockReturnValueOnce(
|
||||
new Promise((resolve) => {
|
||||
resolveSpawn = resolve
|
||||
})
|
||||
)
|
||||
const provider = new DegradedDaemonPtyProvider({ current, legacy: [], fallback })
|
||||
const data = vi.fn()
|
||||
provider.onData(data)
|
||||
|
||||
const spawning = provider.spawn({ sessionId, isNewSession: true, cols: 80, rows: 24 })
|
||||
current.emitData(sessionId, 'foreign daemon output')
|
||||
identity = replacement
|
||||
notifyIdentityChange({ previous, current: replacement })
|
||||
fallback.emitData(sessionId, 'fallback first')
|
||||
resolveSpawn({ id: sessionId })
|
||||
await spawning
|
||||
provider.write(sessionId, 'fallback input')
|
||||
|
||||
expect(data).toHaveBeenCalledExactlyOnceWith({ id: sessionId, data: 'fallback first' })
|
||||
expect(fallback.write).toHaveBeenCalledExactlyOnceWith(sessionId, 'fallback input')
|
||||
})
|
||||
|
||||
it('rejects unknown fallback output after unavailable tombstones are evicted', async () => {
|
||||
const current = createDaemonAdapter('daemon')
|
||||
const fallback = createProvider('fallback')
|
||||
const previous = { pid: 1, startedAtMs: 1, launchNonce: 'previous' }
|
||||
const replacement = { pid: 2, startedAtMs: 2, launchNonce: 'replacement' }
|
||||
let identity = previous
|
||||
let notifyIdentityChange!: (event: DaemonIdentityChangeEvent) => void
|
||||
vi.mocked(current.getLastAuthenticatedDaemonIdentity).mockImplementation(() => identity)
|
||||
vi.mocked(current.onDaemonIdentityChanged).mockImplementation((listener) => {
|
||||
notifyIdentityChange = listener
|
||||
return () => {}
|
||||
})
|
||||
const provider = new DegradedDaemonPtyProvider({ current, legacy: [], fallback })
|
||||
|
||||
for (let index = 0; index <= 1_000; index += 1) {
|
||||
@@ -657,6 +702,18 @@ describe('DegradedDaemonPtyProvider', () => {
|
||||
id: 'explicit-fallback',
|
||||
data: 'safe frame'
|
||||
})
|
||||
|
||||
current.emitData('explicit-fallback', 'foreign frame')
|
||||
identity = replacement
|
||||
notifyIdentityChange({ previous, current: replacement })
|
||||
fallback.emitData('explicit-fallback', 'surviving frame')
|
||||
provider.write('explicit-fallback', 'safe input')
|
||||
|
||||
expect(data).toHaveBeenNthCalledWith(2, {
|
||||
id: 'explicit-fallback',
|
||||
data: 'surviving frame'
|
||||
})
|
||||
expect(fallback.write).toHaveBeenCalledExactlyOnceWith('explicit-fallback', 'safe input')
|
||||
})
|
||||
|
||||
it('preserves explicit sequence accounting on daemon data events', () => {
|
||||
|
||||
@@ -4,15 +4,15 @@ import { shutdownDegradedFallbackSessions } from './degraded-daemon-fallback-shu
|
||||
import { DaemonSessionRouteTable } from './daemon-session-route-table'
|
||||
import { DaemonSessionOwnerUnknownError, sameDaemonIncarnation } from './daemon-session-route'
|
||||
import type { DaemonSnapshotAcknowledgementRoutes } from './daemon-snapshot-acknowledgement-routes'
|
||||
import { DegradedFallbackSessionRoutes } from './degraded-fallback-session-routes'
|
||||
import { DegradedFallbackSessionRoutes as FallbackRoutes } from './degraded-fallback-session-routes'
|
||||
import { DegradedFallbackSpawnRoutes } from './degraded-fallback-spawn-routes'
|
||||
|
||||
export class DegradedDaemonSessionRouting {
|
||||
private readonly adapters: DaemonPtyAdapter[]
|
||||
private readonly adapterSet: ReadonlySet<DaemonPtyAdapter>
|
||||
private readonly routes: DaemonSessionRouteTable
|
||||
private readonly fallbackSessions = new Map<string, IPtyProvider>()
|
||||
private readonly fallbackRoutes: DegradedFallbackSessionRoutes
|
||||
private readonly fallbackOwners = new Map<string, IPtyProvider>()
|
||||
private readonly fallbackRoutes: FallbackRoutes
|
||||
private readonly fallbackSpawns = new DegradedFallbackSpawnRoutes()
|
||||
private readonly identityUnsubscribes: (() => void)[] = []
|
||||
|
||||
@@ -25,7 +25,7 @@ export class DegradedDaemonSessionRouting {
|
||||
this.adapters = [current, ...legacy]
|
||||
this.adapterSet = new Set(this.adapters)
|
||||
this.routes = new DaemonSessionRouteTable(this.adapters)
|
||||
this.fallbackRoutes = new DegradedFallbackSessionRoutes(this.fallbackSessions, this.routes)
|
||||
this.fallbackRoutes = new FallbackRoutes(this.fallbackOwners, this.routes, this.fallbackSpawns)
|
||||
for (const adapter of this.adapters) {
|
||||
this.identityUnsubscribes.push(
|
||||
adapter.onDaemonIdentityChanged(({ previous }) => {
|
||||
@@ -51,7 +51,7 @@ export class DegradedDaemonSessionRouting {
|
||||
incarnation
|
||||
)
|
||||
for (const session of sessions) {
|
||||
if (this.fallbackSessions.has(session.id)) {
|
||||
if (this.fallbackOwners.has(session.id)) {
|
||||
this.fallbackRoutes.recordCollision(session.id)
|
||||
}
|
||||
}
|
||||
@@ -95,7 +95,7 @@ export class DegradedDaemonSessionRouting {
|
||||
if (this.fallbackRoutes.hasCollision(sessionId)) {
|
||||
throw new DaemonSessionOwnerUnknownError(sessionId)
|
||||
}
|
||||
return this.fallbackSessions.get(sessionId) ?? this.routes.resolveOwnerSync(sessionId)
|
||||
return this.fallbackOwners.get(sessionId) ?? this.routes.resolveOwnerSync(sessionId)
|
||||
}
|
||||
|
||||
ownerForHint(sessionId: string): IPtyProvider | null {
|
||||
@@ -110,7 +110,7 @@ export class DegradedDaemonSessionRouting {
|
||||
if (this.fallbackRoutes.hasCollision(sessionId)) {
|
||||
return true
|
||||
}
|
||||
const fallback = this.fallbackSessions.get(sessionId)
|
||||
const fallback = this.fallbackOwners.get(sessionId)
|
||||
return fallback ? (fallback.hasPty?.(sessionId) ?? true) : this.routes.hasPty(sessionId)
|
||||
}
|
||||
|
||||
@@ -131,23 +131,28 @@ export class DegradedDaemonSessionRouting {
|
||||
shouldForwardStreamEvent(provider: IPtyProvider, sessionId: string): boolean {
|
||||
const adapter = this.daemonAdapter(provider)
|
||||
if (adapter) {
|
||||
const shouldForward = this.routes.recordDataOwner(sessionId, adapter)
|
||||
if (this.fallbackSessions.has(sessionId) || this.fallbackSpawns.hasLiveCandidate(sessionId)) {
|
||||
if (this.fallbackOwners.has(sessionId)) {
|
||||
this.routes.recordFreshOwned(sessionId, adapter)
|
||||
this.fallbackRoutes.recordCollision(sessionId)
|
||||
return false
|
||||
}
|
||||
return shouldForward
|
||||
const shouldForward = this.routes.recordDataOwner(sessionId, adapter)
|
||||
if (!this.fallbackSpawns.hasLiveCandidate(sessionId)) {
|
||||
return shouldForward
|
||||
}
|
||||
this.fallbackRoutes.recordCollision(sessionId)
|
||||
return false
|
||||
}
|
||||
const route = this.routes.get(sessionId)
|
||||
if (!route) {
|
||||
return (
|
||||
!this.fallbackSpawns.hasExited(sessionId) &&
|
||||
(this.fallbackSessions.get(sessionId) === provider ||
|
||||
(this.fallbackOwners.get(sessionId) === provider ||
|
||||
this.fallbackSpawns.isInFlight(sessionId))
|
||||
)
|
||||
}
|
||||
if (
|
||||
(this.fallbackSessions.get(sessionId) === provider ||
|
||||
(this.fallbackOwners.get(sessionId) === provider ||
|
||||
this.fallbackSpawns.hasLiveCandidate(sessionId)) &&
|
||||
route.state === 'unavailable' &&
|
||||
!this.fallbackRoutes.hasCollision(sessionId)
|
||||
@@ -166,7 +171,7 @@ export class DegradedDaemonSessionRouting {
|
||||
const adapter = this.daemonAdapter(provider)
|
||||
if (!adapter) {
|
||||
const isOwned =
|
||||
this.fallbackSessions.get(sessionId) === provider ||
|
||||
this.fallbackOwners.get(sessionId) === provider ||
|
||||
this.fallbackSpawns.hasLiveCandidate(sessionId)
|
||||
if (!isOwned) {
|
||||
return false
|
||||
@@ -177,7 +182,7 @@ export class DegradedDaemonSessionRouting {
|
||||
return !route || route.state === 'unavailable'
|
||||
}
|
||||
this.snapshotAcks.dropForProducer(sessionId, adapter)
|
||||
if (this.fallbackSessions.has(sessionId) || this.fallbackSpawns.hasLiveCandidate(sessionId)) {
|
||||
if (this.fallbackOwners.has(sessionId) || this.fallbackSpawns.hasLiveCandidate(sessionId)) {
|
||||
this.routes.markUnavailable(sessionId, adapter)
|
||||
if (this.routes.get(sessionId)?.state === 'unavailable') {
|
||||
this.fallbackRoutes.clearCollision(sessionId)
|
||||
@@ -236,7 +241,7 @@ export class DegradedDaemonSessionRouting {
|
||||
for (const { adapter, incarnation, alive: sessionIds } of reconciled) {
|
||||
for (const id of sessionIds) {
|
||||
this.routes.recordOwnedIncarnation(id, adapter, incarnation)
|
||||
if (this.fallbackSessions.has(id)) {
|
||||
if (this.fallbackOwners.has(id)) {
|
||||
this.fallbackRoutes.recordCollision(id)
|
||||
}
|
||||
}
|
||||
@@ -277,7 +282,7 @@ export class DegradedDaemonSessionRouting {
|
||||
}
|
||||
|
||||
async shutdownFallbackSessions(): Promise<number> {
|
||||
const count = await shutdownDegradedFallbackSessions(this.fallbackSessions, this.fallback)
|
||||
const count = await shutdownDegradedFallbackSessions(this.fallbackOwners, this.fallback)
|
||||
this.fallbackRoutes.pruneSessionEpochs()
|
||||
return count
|
||||
}
|
||||
@@ -307,7 +312,7 @@ export class DegradedDaemonSessionRouting {
|
||||
this.routes.assertFreshSpawnAvailable(opts.sessionId)
|
||||
return this.fallback
|
||||
}
|
||||
return this.fallbackSessions.get(opts.sessionId) ?? this.routes.resolveOwner(opts.sessionId)
|
||||
return this.fallbackOwners.get(opts.sessionId) ?? this.routes.resolveOwner(opts.sessionId)
|
||||
}
|
||||
|
||||
private daemonAdapter(provider: IPtyProvider): DaemonPtyAdapter | null {
|
||||
|
||||
@@ -2,6 +2,7 @@ import type { IPtyProvider } from '../providers/types'
|
||||
import type { DaemonPtyAdapter } from './daemon-pty-adapter'
|
||||
import { sameDaemonIncarnation } from './daemon-session-route'
|
||||
import type { DaemonSessionRouteTable } from './daemon-session-route-table'
|
||||
import type { DegradedFallbackSpawnRoutes } from './degraded-fallback-spawn-routes'
|
||||
|
||||
type FallbackLivenessProbe = {
|
||||
provider: IPtyProvider
|
||||
@@ -15,7 +16,8 @@ export class DegradedFallbackSessionRoutes {
|
||||
|
||||
constructor(
|
||||
private readonly sessions: Map<string, IPtyProvider>,
|
||||
private readonly daemonRoutes: DaemonSessionRouteTable
|
||||
private readonly daemonRoutes: DaemonSessionRouteTable,
|
||||
private readonly spawns: DegradedFallbackSpawnRoutes
|
||||
) {}
|
||||
|
||||
hasCollision(sessionId: string): boolean {
|
||||
@@ -94,7 +96,7 @@ export class DegradedFallbackSessionRoutes {
|
||||
owner: DaemonPtyAdapter,
|
||||
incarnation: ReturnType<DaemonPtyAdapter['getLastAuthenticatedDaemonIdentity']>
|
||||
): boolean {
|
||||
if (!this.sessions.has(sessionId)) {
|
||||
if (!this.sessions.has(sessionId) && !this.spawns.hasLiveCandidate(sessionId)) {
|
||||
return false
|
||||
}
|
||||
const route = this.daemonRoutes.get(sessionId)
|
||||
|
||||
@@ -2883,6 +2883,7 @@ describe('registerPtyHandlers', () => {
|
||||
env: Record<string, string>
|
||||
envToDelete?: string[]
|
||||
isNewSession?: boolean
|
||||
sessionId?: string
|
||||
shellOverride?: string
|
||||
terminalWindowsWslDistro?: string | null
|
||||
terminalWindowsPowerShellImplementation?: string
|
||||
@@ -2943,6 +2944,7 @@ describe('registerPtyHandlers', () => {
|
||||
spawnArgs?: {
|
||||
cwd?: string
|
||||
worktreeId?: string
|
||||
sessionId?: string
|
||||
shellOverride?: string
|
||||
command?: string
|
||||
launchAgent?: TuiAgent
|
||||
@@ -4057,6 +4059,28 @@ describe('registerPtyHandlers', () => {
|
||||
expect(env.GIT_CONFIG_KEY_2).toBe('credential.guiPrompt')
|
||||
})
|
||||
|
||||
it('uses a conservative guard for an unresolved caller-provided daemon session', async () => {
|
||||
const supportsGuard = vi.fn(() => false)
|
||||
const options = await daemonSpawnAndGetOptions(
|
||||
{
|
||||
GIT_CONFIG_COUNT: '1',
|
||||
GIT_CONFIG_KEY_0: 'http.proxy',
|
||||
GIT_CONFIG_VALUE_0: 'http://proxy.invalid'
|
||||
},
|
||||
undefined,
|
||||
undefined,
|
||||
undefined,
|
||||
{ sessionId: 'preserved-session', command: 'claude' },
|
||||
supportsGuard
|
||||
)
|
||||
|
||||
expect(supportsGuard).toHaveBeenCalledExactlyOnceWith('preserved-session')
|
||||
expect(options.sessionId).toBe('preserved-session')
|
||||
expect(options.env.GIT_CONFIG_COUNT).toBe('3')
|
||||
expect(options.env.GIT_CONFIG_KEY_1).toBe('credential.interactive')
|
||||
expect(options.env.GIT_CONFIG_KEY_2).toBe('credential.guiPrompt')
|
||||
})
|
||||
|
||||
it('passes the minted sessionId through to provider.spawn and host env setup', async () => {
|
||||
const daemonSpawn = setupDaemonAdapter()
|
||||
handlers.clear()
|
||||
|
||||
@@ -0,0 +1,65 @@
|
||||
// @vitest-environment happy-dom
|
||||
|
||||
import '@testing-library/jest-dom/vitest'
|
||||
import { cleanup, render, screen } from '@testing-library/react'
|
||||
import { afterEach, describe, expect, it, vi } from 'vitest'
|
||||
import type { PtyManagementSession } from '../../../../preload/api-types'
|
||||
import { TooltipProvider } from '../ui/tooltip'
|
||||
import { ManageSessionsTable } from './ManageSessionsTable'
|
||||
|
||||
vi.mock('@/i18n/i18n', () => ({
|
||||
translate: (_key: string, fallback: string) => fallback
|
||||
}))
|
||||
|
||||
function session(incarnationId: string): PtyManagementSession {
|
||||
return {
|
||||
sessionId: 'colliding-session',
|
||||
incarnationId,
|
||||
state: 'running',
|
||||
shellState: 'ready',
|
||||
isAlive: true,
|
||||
pid: 123,
|
||||
cwd: '/workspace',
|
||||
cols: 80,
|
||||
rows: 24,
|
||||
createdAt: 1,
|
||||
protocolVersion: 30
|
||||
}
|
||||
}
|
||||
|
||||
afterEach(() => {
|
||||
cleanup()
|
||||
vi.restoreAllMocks()
|
||||
})
|
||||
|
||||
describe('ManageSessionsTable', () => {
|
||||
it('renders same-id daemon incarnations without duplicate React keys', () => {
|
||||
const consoleError = vi.spyOn(console, 'error').mockImplementation(() => {})
|
||||
|
||||
render(
|
||||
<TooltipProvider>
|
||||
<ManageSessionsTable
|
||||
sessions={[session('old-incarnation'), session('new-incarnation')]}
|
||||
hasLoadedOnce
|
||||
sessionCount={2}
|
||||
isBusy={false}
|
||||
isRefreshing={false}
|
||||
daemonBusyKind={null}
|
||||
ptyIdToTabId={new Map()}
|
||||
onRefresh={vi.fn()}
|
||||
onKillAll={vi.fn()}
|
||||
onRestartDaemon={vi.fn()}
|
||||
onNavigate={vi.fn()}
|
||||
onRequestKill={vi.fn()}
|
||||
/>
|
||||
</TooltipProvider>
|
||||
)
|
||||
|
||||
expect(screen.getAllByText('colliding-session')).toHaveLength(2)
|
||||
expect(
|
||||
consoleError.mock.calls.filter((args) =>
|
||||
args.some((arg) => /same key|unique "key"/i.test(String(arg)))
|
||||
)
|
||||
).toEqual([])
|
||||
})
|
||||
})
|
||||
@@ -132,7 +132,7 @@ export function ManageSessionsTable({
|
||||
const rowClickable = tabId !== null
|
||||
return (
|
||||
<tr
|
||||
key={session.sessionId}
|
||||
key={`${session.protocolVersion}:${session.incarnationId ?? ''}:${session.sessionId}`}
|
||||
className={`border-t border-border/50 first:border-t-0 ${
|
||||
rowClickable ? 'cursor-pointer hover:bg-accent/60' : ''
|
||||
}`}
|
||||
|
||||
Reference in New Issue
Block a user