mirror of
https://github.com/stablyai/orca.git
synced 2026-10-03 00:02:19 +00:00
* fix(native-chat): retire provider ownership on restart * fix(native-chat): stop showing a restart eviction as a provider death Restarting Orca turned a resumable structured chat into a user-visible `Provider exited: recorded pid absent on host`. Quit never released the durable lease, so restart probed the recorded pid, adjudicated the session evicted, and wrote a synthetic status row against a chat that was perfectly resumable. The fix is the missing teardown phase plus the missing fence check: quit now evicts every provider child this host owns — stopping it, settling its journal and handing the lease back — and the release compare-and-swaps on the fence it expected. Restart then finds a released lease and reopens the chat silently. What the user sees is decided by the typed death evidence rather than the shape of a settlement id: only an `exit-observed` death writes copy, and that copy now carries its cause so an auth failure and an OOM kill do not read alike. The reassuring wording stays. Historical synthetic rows are filtered out of the render projection, which needs no schema change and leaves every real provider-exit row alone. Also: - Bound the new eviction phase well below the quit deadline; a quit that dies mid-eviction leaves the lease unreleased, which is the original bug. - Scope the interruption verdict to work that was mid-response. A provider that died while waiting on an approval interrupted nothing. - Keep host bookkeeping in step with the adapter: the provider-child flag clears when the child is proven stopped, not seven steps later. - Drop the router's duplicate shutdown gate and acquisition drain — both adapters already own theirs — and latch the router closed so a late acquire cannot fan a session back out to closed adapters. - Attach the real cause to the settlement failure a quit reports, and remove a recovery-ticket field that was hardcoded at its only construction site. * fix(native-chat): scope the legacy status filter to the copy it retires The read-time filter hid every status row carrying a `restart-eviction:` identity. That identity is still minted, so a genuine provider death settled under it would have been dropped from every rendered page. Match the retired `Provider exited` copy as well, so only the legacy rows are hidden. Three smaller corrections alongside it: - The settlement retry path now applies the same unfinished-work check the live exit path uses, so a provider that died waiting on an approval no longer gets told a response was in progress. - Bound the exit reason before composing the outcome copy, so a stderr dump in the reason cannot push the "you can continue" sentence past the row's byte cap. - Correct the teardown comment: tail rows are protected by eviction's own per-session ordering, and `closeAll` is a backstop for children eviction never took, including one whose eviction was refused. * fix(native-chat): retire legacy status rows at the projection source The read-time filter that hides the retired `Provider exited …` rows ran on the way OUT of the page builder, after the paging math had already measured the unfiltered timeline. A backward window landing entirely on those rows returned an empty page that still reported `hasOlder: true` with a null `window.oldest`, so the renderer's backfill loop re-asked from the same anchor forever. Its only no-progress guard compares `window.oldest?.sequence` to the anchor, and `undefined === n` never breaks. The live subscription opens behind that loop, so the transcript never finished loading either. Filter where items ENTER the page pipeline instead: the reduced snapshot gets one renderable timeline, the forward path gets one renderable batch, and the window bound, effective limit, `hasOlder`, `window.oldest` and `nextCursor` are all computed over that single array. A window with nothing left behind it now reports end-of-history. Also restore the eviction retry contract. Clearing `hasProviderChild` as soon as the adapter proves the child gone is honest, but it is a different fact from the wind-down this host still owes. A retry after a step aborted between the two was reading "no child here" and skipping both the dead-generation settlement and the lease release the aborted attempt had promised to repeat. The obligation is now tracked separately and cleared only by a release that actually landed. And rename the filter to the copy it retires: it drops only rows carrying the retired `Provider exited` text, not restart-eviction status rows in general. * fix(native-chat): read the wind-down a close owes from the live child An eviction recorded "nothing owed" whenever it ran over a session with no provider child of its own, and the retry then read that record in preference to the child in front of it. A session suspended to an agent terminal is exactly that shape, and the trip back to native re-acquires into the SAME session object rather than replacing it, so the next close skipped both the dead-generation settlement and the lease release — leaving the record claiming a live owner this host had just stopped, and a pending send unsettled. The obligation is now derived the way the quit sweep already derived it, from one shared predicate: a live child always owes a wind-down, and a remembered `false` only carries the obligation forward, never cancels it. Also drops a memoization in the history page that could never hit. Its key was the snapshot's items array, which the reducer rebuilds on every `snapshot()` call, so each backward page allocated a fresh key; the one reader that does share a snapshot across pages reads forward and never calls it. The comment claimed a multi-page read filtered once, which was not true of either path. Tests: the handoff round trip that strands the lease, and the quit sweep picking up an eviction whose close retry never came. * chore(native-chat): scope three helpers to their file and pin the teardown order retryUnexpectedExitSettlement, hasUnfinishedStructuredAgentSessionWork and isRetiredProviderExitStatusItem each have no consumer outside the file that defines them, so they no longer advertise an external contract. The quit-path phase list documents its order as load-bearing, but nothing asserted it. Pin the phase names so evict-owned-sessions cannot drift out of its slot between drain-attaches and flush-event-sinks. * fix(native-chat): stop the router reporting a stop it never observed `closeAll` cleared the route table and set one boolean, after which that boolean was the only surviving evidence about any session. Two call sites then spent it: `releaseAcquisition` and the stop path each turned a route-lookup MISS into reported success. Eviction reads a `true` from the stop path as proof the provider child is gone and releases the durable lease on it, so a session the router never routed could have its lease handed back on the strength of "I have no record, but everything is closed." Loss of contact is not evidence of process death. The fix keeps the evidence instead of the inference: adapter shutdown only resolves once every child is proven stopped, so `closeAll` now marks each routed session `stopped` rather than forgetting it. A routed session still answers `true` from its own retained proof; a session with no route answers `false`, which leaves it indexed for a real retry. `releaseAcquisition` drops its short-circuit and asks the adapters, which answer from their own session maps. The acquire-side latch is unchanged: once closed, the router stays closed and refuses new work. Behaviour that changed: a post-`closeAll` stop for a session the router never routed, or one the host already acknowledged as released, now reports unproven instead of proven. That matches what the same call already answered before `closeAll`, and no real flow reaches it — quit evicts every owned session before `closeAll` runs, and eviction only asks the adapter for sessions whose provider child this host acquired through the router. * test(native-chat): ratchet the retired provider-exit copy out of production The retirement filter hides a status row on two facts: a restart-eviction item id and copy that opens with the retired prefix. The identity half is still minted today, so the filter cannot tell a new producer's row from the legacy row it exists to hide — any future writer of that copy would be dropped from every transcript with no trace. Until now that safety property lived only in a doc comment. Scan the shipped tree for a string literal that OPENS with the retired prefix, which is exactly what the filter's `startsWith` reads. Comments are stripped first, so prose about the retirement is not a producer, and the filter's own constant is exempt. Tests are excluded: writing the copy is how the filter is exercised. * revert(native-chat): drop the read-time retired provider-exit filter Fix forward instead. The lifecycle change in this branch stops any new `Provider exited: <reason>` row from being written; rows a previous build already persisted stay in those transcripts and age out with them. A permanent read-time filter for a cosmetic, shrinking set was not worth its maintenance cost, and its paging seam was the only place a backward window could land entirely on hidden rows. Removes the filter module and its test, restores agent-session-history-page.ts to its pre-branch form, and drops the tests that only existed to prove the filter did not over-match or wedge the backfill loop. The copy ratchet stays and now carries the whole guarantee: with no filter in front of it, any production writer that resurrects the retired prefix reaches the user's transcript directly.
258 lines
10 KiB
TypeScript
258 lines
10 KiB
TypeScript
import { describe, expect, it, vi } from 'vitest'
|
|
import type { AgentSessionJournalIdentity } from '../../shared/agent-session-journal-types'
|
|
import type { AgentSessionBackgroundTaskState } from '../../shared/agent-session-wire'
|
|
import type {
|
|
CodexAppServerConnection,
|
|
CodexAppServerConnectionHandlers,
|
|
openCodexAppServerConnection
|
|
} from './codex-app-server-connection'
|
|
import { CodexStructuredSessionAdapter } from './codex-structured-session-adapter'
|
|
import { CodexBackgroundTaskTracker } from './codex-background-task-tracker'
|
|
import type { CodexStructuredSessionEvent } from './codex-structured-session-state'
|
|
import type { StructuredAgentSessionEventSink } from '../native-chat/agent-session-wire/structured-agent-session-event-sink'
|
|
|
|
// Proves the strip is actually REACHED from provider traffic: the tracker is
|
|
// unit-tested separately, and a producer that is correct but unwired publishes
|
|
// nothing while every one of its own tests stays green.
|
|
|
|
const THREAD_ID = '01a07d54-3785-71d0-b065-82c8ebbc572a'
|
|
const PARENT_TURN = '01a07d54-37be-72e1-8206-8f0c23dd2cef'
|
|
const CHILD_ID = '01a07d54-5523-78a3-91f5-e0acb1dab065'
|
|
|
|
/** A three-route stand-in, deliberately smaller than the full adapter harness:
|
|
* this suite only needs a thread and a notification pipe. */
|
|
function fakeCodex(close: () => Promise<boolean> = async () => true): {
|
|
handlers: () => CodexAppServerConnectionHandlers
|
|
openConnection: typeof openCodexAppServerConnection
|
|
} {
|
|
let live: CodexAppServerConnectionHandlers = {}
|
|
const openConnection = (async (_launch, handlers = {}) => {
|
|
live = handlers
|
|
const connection: CodexAppServerConnection = {
|
|
pid: 4321,
|
|
closed: false,
|
|
request: async (method) =>
|
|
method === 'thread/start' ? { thread: { id: THREAD_ID, path: null } } : {},
|
|
notify: () => {},
|
|
respond: () => {},
|
|
respondWithError: () => {},
|
|
close
|
|
} as unknown as CodexAppServerConnection
|
|
return connection
|
|
}) as typeof openCodexAppServerConnection
|
|
return { handlers: () => live, openConnection }
|
|
}
|
|
|
|
function identity(sessionId: string): AgentSessionJournalIdentity {
|
|
return {
|
|
sessionId,
|
|
workspaceId: 'ws-1',
|
|
hostId: 'host-1',
|
|
agent: 'codex',
|
|
providerHandle: { kind: 'codex', threadId: THREAD_ID }
|
|
}
|
|
}
|
|
|
|
function subagentNotification(kind: string): { method: string; params: unknown } {
|
|
return {
|
|
method: 'item/started',
|
|
params: {
|
|
item: {
|
|
type: 'subAgentActivity',
|
|
id: 'call_1',
|
|
kind,
|
|
agentThreadId: CHILD_ID,
|
|
agentPath: '/root/count_a'
|
|
},
|
|
threadId: THREAD_ID,
|
|
turnId: PARENT_TURN
|
|
}
|
|
}
|
|
}
|
|
|
|
const TURN_COMPLETED = {
|
|
method: 'turn/completed',
|
|
params: { threadId: THREAD_ID, turn: { id: PARENT_TURN, status: 'completed' } }
|
|
}
|
|
|
|
async function adapterWithSession(
|
|
published: { sessionId: string; state: AgentSessionBackgroundTaskState | null }[],
|
|
events?: StructuredAgentSessionEventSink,
|
|
onEvent?: (event: CodexStructuredSessionEvent) => void,
|
|
close?: () => Promise<boolean>
|
|
): Promise<{ adapter: CodexStructuredSessionAdapter; codex: ReturnType<typeof fakeCodex> }> {
|
|
const codex = fakeCodex(close)
|
|
const adapter = new CodexStructuredSessionAdapter({
|
|
resolveLaunch: async () => ({
|
|
command: 'codex',
|
|
args: ['app-server'],
|
|
cwd: '/work/repo',
|
|
codexHome: null,
|
|
resumeThreadId: null
|
|
}),
|
|
openConnection: codex.openConnection,
|
|
readProcessStartTime: async () => 1_700_000_000_000,
|
|
onEvent,
|
|
onBackgroundTasksChanged: (sessionId, state) => published.push({ sessionId, state })
|
|
})
|
|
await adapter.acquire({
|
|
identity: identity('session-1'),
|
|
fence: 7,
|
|
spawnToken: 'spawn-9',
|
|
events
|
|
})
|
|
codex.handlers().onNotification?.('turn/started', {
|
|
threadId: THREAD_ID,
|
|
turn: { id: PARENT_TURN, status: 'inProgress' }
|
|
})
|
|
codex.handlers().onNotification?.('turn/started', {
|
|
threadId: CHILD_ID,
|
|
turn: { id: 'child-turn', status: 'inProgress' }
|
|
})
|
|
return { adapter, codex }
|
|
}
|
|
|
|
describe('codex background tasks reach the strip', () => {
|
|
it('clears natural-exit state before lifecycle observers can read it', async () => {
|
|
const published: { sessionId: string; state: AgentSessionBackgroundTaskState | null }[] = []
|
|
const onEvent = vi.fn()
|
|
const { adapter, codex } = await adapterWithSession(published, undefined, onEvent)
|
|
const spawn = subagentNotification('started')
|
|
codex.handlers().onNotification?.(spawn.method, spawn.params)
|
|
codex.handlers().onNotification?.(TURN_COMPLETED.method, TURN_COMPLETED.params)
|
|
expect(adapter.backgroundTaskState('session-1')?.tasks).toHaveLength(1)
|
|
published.length = 0
|
|
onEvent.mockImplementation((event: CodexStructuredSessionEvent) => {
|
|
if (event.type === 'ended') {
|
|
expect(adapter.backgroundTaskState('session-1')).toBeNull()
|
|
}
|
|
})
|
|
codex.handlers().onExit?.(new Error('provider exited'))
|
|
expect(adapter.backgroundTaskState('session-1')).toBeNull()
|
|
expect(published).toEqual([{ sessionId: 'session-1', state: null }])
|
|
await adapter.closeSession('session-1')
|
|
})
|
|
|
|
it('keeps live tasks when close is refused', async () => {
|
|
const published: { sessionId: string; state: AgentSessionBackgroundTaskState | null }[] = []
|
|
const close = vi.fn(async () => false)
|
|
const { adapter, codex } = await adapterWithSession(published, undefined, undefined, close)
|
|
const spawn = subagentNotification('started')
|
|
codex.handlers().onNotification?.(spawn.method, spawn.params)
|
|
codex.handlers().onNotification?.(TURN_COMPLETED.method, TURN_COMPLETED.params)
|
|
const before = adapter.backgroundTaskState('session-1')
|
|
published.length = 0
|
|
expect(await adapter.closeSession('session-1')).toBe(false)
|
|
expect(adapter.backgroundTaskState('session-1')).toEqual(before)
|
|
expect(published).toEqual([])
|
|
close.mockResolvedValue(true)
|
|
await adapter.closeSession('session-1')
|
|
})
|
|
|
|
it('does not let an old exit callback clear a replacement roster', async () => {
|
|
const published: { sessionId: string; state: AgentSessionBackgroundTaskState | null }[] = []
|
|
const { adapter, codex } = await adapterWithSession(published)
|
|
const oldExit = codex.handlers().onExit
|
|
await adapter.acquire({ identity: identity('session-1'), fence: 8, spawnToken: 'spawn-10' })
|
|
codex.handlers().onNotification?.('turn/started', {
|
|
threadId: CHILD_ID,
|
|
turn: { id: 'replacement-child-turn' }
|
|
})
|
|
const spawn = subagentNotification('started')
|
|
codex.handlers().onNotification?.(spawn.method, spawn.params)
|
|
const before = adapter.backgroundTaskState('session-1')
|
|
expect(before?.tasks).toHaveLength(1)
|
|
published.length = 0
|
|
oldExit?.(new Error('old provider exited late'))
|
|
expect(adapter.backgroundTaskState('session-1')).toEqual(before)
|
|
expect(published).toEqual([])
|
|
await adapter.closeSession('session-1')
|
|
})
|
|
|
|
it('recovers the exact provider generation when command metadata cannot be admitted', async () => {
|
|
const published: { sessionId: string; state: AgentSessionBackgroundTaskState | null }[] = []
|
|
const observed: CodexStructuredSessionEvent[] = []
|
|
const appendItem = vi.fn()
|
|
const { adapter, codex } = await adapterWithSession(
|
|
published,
|
|
{ appendItem, appendTombstone: () => {}, publish: () => {} },
|
|
(event) => observed.push(event)
|
|
)
|
|
appendItem.mockClear()
|
|
observed.length = 0
|
|
const admission = vi
|
|
.spyOn(CodexBackgroundTaskTracker.prototype, 'canObserve')
|
|
.mockReturnValue(false)
|
|
try {
|
|
codex.handlers().onNotification?.('item/started', {
|
|
threadId: THREAD_ID,
|
|
turnId: PARENT_TURN,
|
|
item: {
|
|
type: 'commandExecution',
|
|
id: 'over-budget',
|
|
command: 'sleep 1',
|
|
source: 'unifiedExecStartup',
|
|
status: 'inProgress'
|
|
}
|
|
})
|
|
await vi.waitFor(() => expect(adapter.backgroundTaskState('session-1')).toBeUndefined())
|
|
// The open turn's lifecycle row is revised to interrupted, never tombstoned.
|
|
expect(appendItem.mock.calls.map((call) => call[1])).toEqual([
|
|
expect.objectContaining({ kind: 'turn', state: 'interrupted' })
|
|
])
|
|
expect(observed).toEqual([
|
|
expect.objectContaining({
|
|
type: 'ended',
|
|
cause: 'unexpected-exit',
|
|
fence: 7,
|
|
acquisitionGeneration: expect.any(String),
|
|
reason: 'notification admission failed (failed)'
|
|
})
|
|
])
|
|
expect(published).toEqual([{ sessionId: 'session-1', state: null }])
|
|
} finally {
|
|
admission.mockRestore()
|
|
await adapter.closeSession('session-1')
|
|
}
|
|
})
|
|
|
|
it('publishes the fan-out while it runs and keeps it past the spawning turn', async () => {
|
|
const published: { sessionId: string; state: AgentSessionBackgroundTaskState | null }[] = []
|
|
const { adapter, codex } = await adapterWithSession(published)
|
|
const running = {
|
|
state: 'monitoring',
|
|
supportsStopAll: false,
|
|
tasks: [{ id: `codex-agent:${CHILD_ID}`, kind: 'agent', description: 'count_a' }]
|
|
}
|
|
|
|
const spawn = subagentNotification('started')
|
|
codex.handlers().onNotification?.(spawn.method, spawn.params)
|
|
// Mid-turn: the child is running, so the strip reports it now.
|
|
expect(published).toEqual([{ sessionId: 'session-1', state: running }])
|
|
expect(adapter.backgroundTaskState('session-1')).toEqual(running)
|
|
|
|
published.length = 0
|
|
codex.handlers().onNotification?.(TURN_COMPLETED.method, TURN_COMPLETED.params)
|
|
|
|
// Turn end is not the child's outcome: no republish and no settle.
|
|
expect(published).toEqual([])
|
|
expect(adapter.backgroundTaskState('session-1')).toEqual(running)
|
|
})
|
|
|
|
it('clears the strip when the session closes', async () => {
|
|
const published: { sessionId: string; state: AgentSessionBackgroundTaskState | null }[] = []
|
|
const { adapter, codex } = await adapterWithSession(published)
|
|
const spawn = subagentNotification('started')
|
|
codex.handlers().onNotification?.(spawn.method, spawn.params)
|
|
codex.handlers().onNotification?.(TURN_COMPLETED.method, TURN_COMPLETED.params)
|
|
published.length = 0
|
|
|
|
expect(await adapter.closeSession('session-1')).toBe(true)
|
|
|
|
// Explicit null, not silence: the reader answers `undefined` once the
|
|
// session is gone, which every channel treats as "unchanged".
|
|
expect(published).toEqual([{ sessionId: 'session-1', state: null }])
|
|
expect(adapter.backgroundTaskState('session-1')).toBeUndefined()
|
|
})
|
|
})
|