mirror of
https://github.com/stablyai/orca.git
synced 2026-09-30 08:03:12 +00:00
fix: retain relay epoch owner across spawn retries
This commit is contained in:
@@ -68,7 +68,7 @@ export function rememberRetiredRelayEpochOwner(args: {
|
||||
retiredRelayOwnerByPane.set(key, args.ownerPtyId)
|
||||
}
|
||||
|
||||
export function takeRetiredRelayEpochOwner(
|
||||
export function peekRetiredRelayEpochOwner(
|
||||
connectionId: string | null | undefined,
|
||||
paneKey: string | null | undefined
|
||||
): string | undefined {
|
||||
@@ -76,8 +76,31 @@ export function takeRetiredRelayEpochOwner(
|
||||
return undefined
|
||||
}
|
||||
const key = retiredRelayOwnerKey(connectionId, paneKey)
|
||||
const ownerPtyId = retiredRelayOwnerByPane.get(key)
|
||||
retiredRelayOwnerByPane.delete(key)
|
||||
return retiredRelayOwnerByPane.get(key)
|
||||
}
|
||||
|
||||
/** Remove a retired owner only after the replacement PTY has been committed. */
|
||||
export function consumeRetiredRelayEpochOwner(
|
||||
connectionId: string | null | undefined,
|
||||
paneKey: string | null | undefined,
|
||||
ownerPtyId: string | undefined
|
||||
): void {
|
||||
if (!connectionId || !paneKey || !ownerPtyId) {
|
||||
return
|
||||
}
|
||||
const key = retiredRelayOwnerKey(connectionId, paneKey)
|
||||
if (retiredRelayOwnerByPane.get(key) === ownerPtyId) {
|
||||
retiredRelayOwnerByPane.delete(key)
|
||||
}
|
||||
}
|
||||
|
||||
/** @deprecated Use peek + consume after a successful spawn. */
|
||||
export function takeRetiredRelayEpochOwner(
|
||||
connectionId: string | null | undefined,
|
||||
paneKey: string | null | undefined
|
||||
): string | undefined {
|
||||
const ownerPtyId = peekRetiredRelayEpochOwner(connectionId, paneKey)
|
||||
consumeRetiredRelayEpochOwner(connectionId, paneKey, ownerPtyId)
|
||||
return ownerPtyId
|
||||
}
|
||||
|
||||
|
||||
@@ -1,10 +1,7 @@
|
||||
import { describe, expect, it, vi } from 'vitest'
|
||||
import type { IPtyProvider, PtySpawnOptions, PtySpawnResult } from '../../../providers/types'
|
||||
import { toAppSshPtyId } from '../../../providers/ssh-pty-id'
|
||||
import {
|
||||
rememberRetiredRelayEpochOwner,
|
||||
takeRetiredRelayEpochOwner
|
||||
} from './relay-pty-mint-epoch'
|
||||
import { rememberRetiredRelayEpochOwner, takeRetiredRelayEpochOwner } from './relay-pty-mint-epoch'
|
||||
import { spawnForStablePane, type StablePaneOwner } from './stable-owner'
|
||||
|
||||
type EpochAwareSpawnOptions = PtySpawnOptions & { resumeProviderSession?: unknown }
|
||||
@@ -241,6 +238,90 @@ describe('spawnForStablePane relay epoch gate', () => {
|
||||
expect(harness.spawns[0]).toEqual(spawnOptions)
|
||||
})
|
||||
|
||||
it('does not let a retired owner gate an unrelated later restore of the same pane', async () => {
|
||||
const paneKey = 'tab-lifetime:55555555-5555-4555-8555-555555555555'
|
||||
const retiredPtyId = toAppSshPtyId('remote', 'pty2:previous:1')
|
||||
const provider = {
|
||||
requestHostRpc: vi.fn(async () => ({ ptyIdMintEpoch: 'current' })),
|
||||
spawn: vi.fn(async (options: EpochAwareSpawnOptions) =>
|
||||
options.attachOnly
|
||||
? { id: retiredPtyId, isReattach: true }
|
||||
: { id: toAppSshPtyId('remote', 'pty2:current:2') }
|
||||
)
|
||||
} as unknown as IPtyProvider
|
||||
rememberRetiredRelayEpochOwner({
|
||||
connectionId: 'remote',
|
||||
paneKey,
|
||||
ownerPtyId: retiredPtyId
|
||||
})
|
||||
|
||||
await spawnForStablePane({
|
||||
runtime: undefined,
|
||||
provider,
|
||||
spawnOptions: { cols: 80, rows: 24 },
|
||||
owner: owner('pty2:previous:1'),
|
||||
connectionId: 'remote',
|
||||
paneKey
|
||||
})
|
||||
|
||||
const secondOptions = agentSpawnOptions()
|
||||
await spawnForStablePane({
|
||||
runtime: undefined,
|
||||
provider,
|
||||
spawnOptions: secondOptions,
|
||||
owner: null,
|
||||
connectionId: 'remote',
|
||||
paneKey
|
||||
})
|
||||
expect(provider.spawn).toHaveBeenLastCalledWith(secondOptions)
|
||||
})
|
||||
|
||||
it('retains the retired owner when the first replacement spawn fails', async () => {
|
||||
const paneKey = 'tab-retry:66666666-6666-4666-8666-666666666666'
|
||||
const requestHostRpc = vi.fn(async () => ({ ptyIdMintEpoch: 'current' }))
|
||||
let attempts = 0
|
||||
const provider = {
|
||||
requestHostRpc,
|
||||
spawn: vi.fn(async (options: EpochAwareSpawnOptions) => {
|
||||
if (options.attachOnly) {
|
||||
throw new Error('not found')
|
||||
}
|
||||
attempts += 1
|
||||
if (attempts === 1) {
|
||||
throw new Error('relay replacement in progress')
|
||||
}
|
||||
return { id: toAppSshPtyId('remote', 'pty2:current:2') }
|
||||
})
|
||||
} as unknown as IPtyProvider
|
||||
rememberRetiredRelayEpochOwner({
|
||||
connectionId: 'remote',
|
||||
paneKey,
|
||||
ownerPtyId: toAppSshPtyId('remote', 'pty2:previous:1')
|
||||
})
|
||||
|
||||
await expect(
|
||||
spawnForStablePane({
|
||||
runtime: undefined,
|
||||
provider,
|
||||
spawnOptions: agentSpawnOptions(),
|
||||
owner: null,
|
||||
connectionId: 'remote',
|
||||
paneKey
|
||||
})
|
||||
).rejects.toThrow('relay replacement in progress')
|
||||
|
||||
const retried = await spawnForStablePane({
|
||||
runtime: undefined,
|
||||
provider,
|
||||
spawnOptions: agentSpawnOptions(),
|
||||
owner: null,
|
||||
connectionId: 'remote',
|
||||
paneKey
|
||||
})
|
||||
expect(retried.result.agentResumeUnavailable).toBe(true)
|
||||
expect(provider.spawn).toHaveBeenCalledTimes(2)
|
||||
})
|
||||
|
||||
it('does not treat a new agent launch as a retired-owner resume', async () => {
|
||||
const harness = createProvider({ ptyIdMintEpoch: 'current' })
|
||||
const paneKey = 'tab-new-agent:44444444-4444-4444-8444-444444444444'
|
||||
|
||||
@@ -13,10 +13,7 @@ import {
|
||||
import { ptyIncarnationById, ptyOwnership } from '../provider/ownership-state'
|
||||
import { isPtyAlreadyGoneError } from '../provider/liveness'
|
||||
import { clearProviderPtyState } from '../provider/state-cleanup'
|
||||
import {
|
||||
deriveStablePaneFreshSpawnOptions,
|
||||
takeRetiredRelayEpochOwner
|
||||
} from './relay-pty-mint-epoch'
|
||||
import * as relayEpoch from './relay-pty-mint-epoch'
|
||||
|
||||
export type StablePaneOwner = {
|
||||
handle?: string
|
||||
@@ -292,19 +289,22 @@ export async function spawnForStablePane(
|
||||
return attached
|
||||
}
|
||||
}
|
||||
const retiredRelayOwnerPtyId = takeRetiredRelayEpochOwner(args.connectionId, args.paneKey)
|
||||
const relayEpochOwnerPtyId =
|
||||
args.owner?.ptyId ??
|
||||
(args.spawnOptions.resumeProviderSession || args.spawnOptions.agentSessionEnsure
|
||||
? retiredRelayOwnerPtyId
|
||||
: undefined)
|
||||
const freshSpawn = await deriveStablePaneFreshSpawnOptions({
|
||||
const retiredRelayOwnerPtyId = relayEpoch.peekRetiredRelayEpochOwner(
|
||||
args.connectionId,
|
||||
args.paneKey
|
||||
)
|
||||
const freshSpawn = await relayEpoch.deriveStablePaneFreshSpawnOptions({
|
||||
provider: args.provider,
|
||||
ownerPtyId: relayEpochOwnerPtyId,
|
||||
ownerPtyId:
|
||||
args.owner?.ptyId ??
|
||||
(args.spawnOptions.resumeProviderSession || args.spawnOptions.agentSessionEnsure
|
||||
? retiredRelayOwnerPtyId
|
||||
: undefined),
|
||||
connectionId: args.connectionId,
|
||||
spawnOptions: args.spawnOptions
|
||||
})
|
||||
const providerResult = await args.provider.spawn(freshSpawn.options)
|
||||
relayEpoch.consumeRetiredRelayEpochOwner(args.connectionId, args.paneKey, retiredRelayOwnerPtyId)
|
||||
const result = freshSpawn.agentResumeDeclined
|
||||
? { ...providerResult, agentResumeUnavailable: true as const }
|
||||
: providerResult
|
||||
|
||||
@@ -0,0 +1,47 @@
|
||||
# STA-5698 relay epoch gate lifetime fix
|
||||
|
||||
## Diagnosis to verify
|
||||
|
||||
The runtime failures share the retired-owner side table's identity/lifetime bug. A
|
||||
`(connectionId, paneKey)` tombstone is consumed by whichever later spawn happens to
|
||||
use that pane, so a later same-relay restore can compare against an unrelated old
|
||||
epoch. Conversely, a failed fresh spawn consumes the only tombstone before a relay
|
||||
replacement retries the same absence, making that retry `unknown` and allowing the
|
||||
resume.
|
||||
|
||||
## Design
|
||||
|
||||
The first proposal to key by `effectiveSessionAppId` is rejected: fresh resume paths
|
||||
intentionally omit `sessionId`, so that value cannot identify the absence. The fix is
|
||||
scoped to direct IPC/SSH paths that reproduce STA-5698; paired-runtime methods and
|
||||
stream opcodes remain unchanged. Relay retirement (including the synthetic
|
||||
`webContents.send('pty:exit', {id, code:-1})` in
|
||||
`ssh-relay-session.handlePtyReattachFailure`) records `{connectionId,paneKey,ownerPtyId}`
|
||||
in the bounded main-process map. No renderer wire shape changes: this map is consulted
|
||||
only by the main process when a fresh IPC/SSH spawn already carries the pane key.
|
||||
|
||||
Presentation is transactional: lookup peeks the retired owner, and the entry is removed
|
||||
only after provider spawn succeeds (including a successful rebind or shell replacement).
|
||||
A provider rejection leaves the entry for the next relay incarnation retry; a later
|
||||
restore after successful rebind cannot see it. A restore with no entry remains `unknown`;
|
||||
known-different still declines agent resume and known-same still resumes. Concurrent
|
||||
lookups are serialized by the spawn path, and the map remains bounded at 256 with
|
||||
recency refresh.
|
||||
|
||||
## Tests and verification
|
||||
|
||||
Add red-before-fix tests in stable-owner and SSH relay suites: synthetic relay exit records
|
||||
the exact owner; first provider rejection leaves that owner for a successful retry; success
|
||||
removes it and blocks replay; an attach/rebind followed by an unrelated restore of the same
|
||||
pane is unknown; same-epoch, legacy/no-entry, and concurrent presentation controls remain
|
||||
unchanged. Run focused tests, typecheck, oxlint/format, relay build, then both runtime
|
||||
scenarios in separate and back-to-back app sessions against `openclaw`.
|
||||
|
||||
## OSS precedent
|
||||
|
||||
The synced desktop terminal manager keeps killed-session tombstones separate from
|
||||
live-resource maps, bounds them, refreshes recency on re-record, and consumes them on
|
||||
matching reuse. It clears the tombstone before create and does not roll it back on
|
||||
create failure; rollback-on-provider-failure is an Orca requirement demonstrated by
|
||||
STA-5698, not OSS precedent. This change borrows only separation, bounding, recency,
|
||||
and explicit reuse.
|
||||
Reference in New Issue
Block a user