mirror of
https://github.com/stablyai/orca.git
synced 2026-09-27 00:02:37 +00:00
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.
This commit is contained in:
@@ -101,23 +101,9 @@ export function readAgentSessionHistory(
|
||||
* Retiring on the way out instead lets a window land entirely on retired rows and return an empty
|
||||
* page that still claims older history — a cursor that never advances, which is a reader spinning
|
||||
* on one request rather than a transcript that finished loading.
|
||||
*
|
||||
* Keyed on the snapshot's own items array, which the reducer rebuilds on every change, so a
|
||||
* multi-page read over one snapshot filters once.
|
||||
*/
|
||||
const renderableTimelines = new WeakMap<
|
||||
readonly AgentJournalRenderItem[],
|
||||
AgentJournalRenderItem[]
|
||||
>()
|
||||
|
||||
function renderableTimeline(snapshot: AgentJournalSnapshot): AgentJournalRenderItem[] {
|
||||
const cached = renderableTimelines.get(snapshot.items)
|
||||
if (cached) {
|
||||
return cached
|
||||
}
|
||||
const items = withoutRetiredProviderExitStatusItems(snapshot.items, snapshot.sessionId)
|
||||
renderableTimelines.set(snapshot.items, items)
|
||||
return items
|
||||
return withoutRetiredProviderExitStatusItems(snapshot.items, snapshot.sessionId)
|
||||
}
|
||||
|
||||
/**
|
||||
|
||||
@@ -50,6 +50,13 @@ function hasProviderChild(
|
||||
return context.sessions.get(sessionId)?.hasProviderChild === true
|
||||
}
|
||||
|
||||
/** The wind-down this host owes for the session's child. A live child always owes one, whatever a
|
||||
* previous childless eviction recorded — the same session object is re-acquired in place on a
|
||||
* handoff back to native, so a remembered `false` must never outrank the child in front of it. */
|
||||
function owesProviderChildWindDown(session: StructuredAgentSessionHostSession): boolean {
|
||||
return session.hasProviderChild || session.owesProviderChildWindDown === true
|
||||
}
|
||||
|
||||
/** Runs the eviction steps under a deadline. A step that fails — or runs out of time — aborts the
|
||||
* rest, which leaves the session indexed and the child loaded so the next close is a real retry. */
|
||||
export async function evictHeldStructuredAgentSession(
|
||||
@@ -63,7 +70,7 @@ export async function evictHeldStructuredAgentSession(
|
||||
// The obligation OUTLIVES the child. `hasProviderChild` is retired the instant the adapter
|
||||
// proves the exit, so a step that aborts after that point would otherwise leave the retry
|
||||
// reading "no child here" and skipping the settlement and the lease release it still owes.
|
||||
const owesWindDown = session.owesProviderChildWindDown ?? session.hasProviderChild
|
||||
const owesWindDown = owesProviderChildWindDown(session)
|
||||
session.owesProviderChildWindDown = owesWindDown
|
||||
let settlementError: unknown
|
||||
const eviction: StructuredAgentSessionEvictionContext = {
|
||||
@@ -127,7 +134,7 @@ export async function evictOwnedStructuredAgentSessions(
|
||||
retainOnFailure: Set<string>
|
||||
): Promise<void> {
|
||||
const ownedSessionIds = [...context.sessions]
|
||||
.filter(([, session]) => session.hasProviderChild || session.owesProviderChildWindDown === true)
|
||||
.filter(([, session]) => owesProviderChildWindDown(session))
|
||||
.map(([sessionId]) => sessionId)
|
||||
// Retained up front and cleared only once an eviction settles: the quit phase is bounded, and a
|
||||
// timeout leaves these still running. Closing their journals underneath them is the one outcome
|
||||
|
||||
+186
-11
@@ -3,7 +3,7 @@
|
||||
// Two leaks meet here and each has to be tested against the real host, not a double: a chat that
|
||||
// closes without stopping its app-server, and a launch that starts one for every record on disk.
|
||||
|
||||
import { mkdtemp, rm } from 'node:fs/promises'
|
||||
import { mkdir, mkdtemp, rm, writeFile } from 'node:fs/promises'
|
||||
import { tmpdir } from 'node:os'
|
||||
import { join } from 'node:path'
|
||||
import { afterEach, beforeEach, describe, expect, it, vi, type Mock } from 'vitest'
|
||||
@@ -20,7 +20,11 @@ import { AgentSessionRecordStore } from '../../runtime/agent-session-record-stor
|
||||
import type { StructuredAgentSessionAdapter } from './structured-agent-session-adapter'
|
||||
import type { StructuredAgentSessionEventSink } from './structured-agent-session-event-sink'
|
||||
import { StructuredAgentSessionHost } from './structured-agent-session-host'
|
||||
import type { StructuredAgentSessionHandoffTransport } from './structured-agent-session-handoff-types'
|
||||
import type {
|
||||
StructuredAgentSessionHandoffTransport,
|
||||
StructuredTuiOwner
|
||||
} from './structured-agent-session-handoff-types'
|
||||
import { StructuredHandoffTestRequests } from './structured-agent-session-handoff-test-requests'
|
||||
import { unexpectedProviderExitOutcome } from './structured-agent-session-dead-generation-settlement'
|
||||
import type { StructuredAgentSessionStatusSink } from './structured-agent-session-status-feed'
|
||||
import {
|
||||
@@ -124,8 +128,107 @@ function waitOutSeveralGraceWindows(): Promise<void> {
|
||||
return new Promise((resolve) => setTimeout(resolve, GRACE_MS * 20))
|
||||
}
|
||||
|
||||
const handoffRequests = new StructuredHandoffTestRequests(
|
||||
NOW,
|
||||
SESSION,
|
||||
() => store.getRecord(SESSION)?.lease.runtimeFence ?? 0
|
||||
)
|
||||
/** Whether the terminal this host handed the session to can be reached again. */
|
||||
let tuiRecoverable: boolean
|
||||
|
||||
/** One operation-id source with the rest of the suite, so the durable ledger sees no duplicate. */
|
||||
function handoffRequest(direction: 'to-tui' | 'to-native') {
|
||||
return handoffRequests.request(direction, 'now', { operationId: hostTestOperationId() })
|
||||
}
|
||||
|
||||
function tuiOwner(fence: number, spawnToken: string, transcriptPath: string): StructuredTuiOwner {
|
||||
return {
|
||||
terminal: { handle: 'term-tui', tabId: 'tab-tui', paneKey: 'pane-tui', ptyId: 'pty-tui' },
|
||||
process: { hostId: 'local', pid: 5200, processStartTimeMs: NOW, spawnToken },
|
||||
link: {
|
||||
linkId: `tui-link-${fence}`,
|
||||
handle: { provider: 'codex', threadId: THREAD },
|
||||
origin: 'resumed',
|
||||
mintedAtFence: fence,
|
||||
observedAt: NOW
|
||||
},
|
||||
transcriptPath
|
||||
}
|
||||
}
|
||||
|
||||
/** A codex rollout the return trip can import, so a real to-native handoff has history to read. */
|
||||
async function writeTuiTranscript(): Promise<string> {
|
||||
const sessionsDir = join(root, 'codex-home', 'sessions', '2026', '08', '12')
|
||||
await mkdir(sessionsDir, { recursive: true })
|
||||
const transcriptPath = join(sessionsDir, `rollout-2026-08-12T10-00-00-${THREAD}.jsonl`)
|
||||
await writeFile(
|
||||
transcriptPath,
|
||||
`${JSON.stringify({
|
||||
type: 'session_meta',
|
||||
timestamp: '2026-08-12T10:00:00.000Z',
|
||||
payload: { id: THREAD, session_id: THREAD }
|
||||
})}\n`,
|
||||
'utf8'
|
||||
)
|
||||
return transcriptPath
|
||||
}
|
||||
|
||||
/** Replaces the current host with one that can hand the session to a terminal and take it back. */
|
||||
function openHandoffHost(transcriptPath: string): void {
|
||||
openHost(undefined, {
|
||||
hostLabel: 'Test host',
|
||||
launchTui: async ({ fence, spawnToken }) => tuiOwner(fence, spawnToken, transcriptPath),
|
||||
reproveTuiOwner: async ({ owner }) => owner,
|
||||
recoverTuiOwner: async (record) => {
|
||||
if (!tuiRecoverable) {
|
||||
throw new Error('the owning terminal could not be reached')
|
||||
}
|
||||
return tuiOwner(
|
||||
record.lease.runtimeFence,
|
||||
record.lease.reservedSpawnToken ?? 'recovered',
|
||||
transcriptPath
|
||||
)
|
||||
},
|
||||
stopRecoveredOwner: async () => undefined,
|
||||
closeTuiOwner: async (owner) => ({ transcriptPath: owner.transcriptPath }),
|
||||
waitForTuiExit: async (owner) => ({ transcriptPath: owner.transcriptPath }),
|
||||
waitForTuiIdleOrExit: async () => 'idle',
|
||||
tuiStatus: () => 'idle'
|
||||
})
|
||||
}
|
||||
|
||||
/** Fails the next eviction at `drain-published`, which leaves the session indexed for a retry. */
|
||||
function failNextDrain(): void {
|
||||
vi.spyOn(host['runtimeState'].eventSinkFor(SESSION), 'drained').mockResolvedValueOnce({
|
||||
ok: false,
|
||||
error: new Error('drain barrier lost')
|
||||
})
|
||||
}
|
||||
|
||||
/** The submissions as they stood when the session was forgotten; its journal is gone after that. */
|
||||
function captureSettledSubmissions(): { value: AgentJournalSubmission[] } {
|
||||
const captured: { value: AgentJournalSubmission[] } = { value: [] }
|
||||
const journal = host['sessions'].get(SESSION)!.journal
|
||||
const closeJournal = journal.close.bind(journal)
|
||||
vi.spyOn(journal, 'close').mockImplementation(async () => {
|
||||
captured.value = journal.snapshot().submissions
|
||||
await closeJournal()
|
||||
})
|
||||
return captured
|
||||
}
|
||||
|
||||
async function sendPending(text: string): Promise<void> {
|
||||
dispatch.mockResolvedValueOnce({ state: 'admitted' })
|
||||
const body = hostTestMessage(text)
|
||||
expect(
|
||||
await host.send(CALLER, { envelope: envelope('agentSession.send', { body }), body })
|
||||
).toMatchObject({ ok: true, value: { submission: { dispatchState: 'pending' } } })
|
||||
}
|
||||
|
||||
beforeEach(async () => {
|
||||
root = await mkdtemp(join(tmpdir(), 'orca-surface-lifetime-'))
|
||||
handoffRequests.reset()
|
||||
tuiRecoverable = true
|
||||
resetHostTestOperationIds()
|
||||
sink = null
|
||||
hostErrors = []
|
||||
@@ -286,14 +389,7 @@ describe('a chat that closes', () => {
|
||||
ok: false,
|
||||
error: new Error('drain barrier lost')
|
||||
})
|
||||
// The journal is gone once the retry forgets the session, so read it on its way out.
|
||||
let settledSubmissions: AgentJournalSubmission[] = []
|
||||
const journal = session!.journal
|
||||
const closeJournal = journal.close.bind(journal)
|
||||
vi.spyOn(journal, 'close').mockImplementation(async () => {
|
||||
settledSubmissions = journal.snapshot().submissions
|
||||
await closeJournal()
|
||||
})
|
||||
const settled = captureSettledSubmissions()
|
||||
|
||||
await expect(host.close(SESSION)).rejects.toMatchObject({ step: 'drain-published' })
|
||||
// The child is proven gone, but the wind-down it owes is not done: nothing settled, no release.
|
||||
@@ -306,7 +402,7 @@ describe('a chat that closes', () => {
|
||||
claimStatus: 'released',
|
||||
ownerProcess: null
|
||||
})
|
||||
expect(hasUnansweredStructuredAgentSessionDispatch(settledSubmissions)).toBe(false)
|
||||
expect(hasUnansweredStructuredAgentSessionDispatch(settled.value)).toBe(false)
|
||||
})
|
||||
})
|
||||
|
||||
@@ -709,3 +805,82 @@ describe('an unexpected provider exit', () => {
|
||||
expect(acquire).toHaveBeenCalledTimes(2)
|
||||
})
|
||||
})
|
||||
|
||||
describe('a chat handed to a terminal and taken back', () => {
|
||||
// The wind-down a close owes belongs to the child in front of it, not to whatever the LAST
|
||||
// eviction found. A session a terminal owns is indexed with no child of its own, so a close
|
||||
// there records "nothing owed" — and the trip back re-acquires into that SAME session object.
|
||||
it('settles and releases the child it was given back', async () => {
|
||||
const transcriptPath = await writeTuiTranscript()
|
||||
await host.flushAllStreamedEvents()
|
||||
openHandoffHost(transcriptPath)
|
||||
await attach()
|
||||
expect(await host.requestHandoff(CALLER, handoffRequest('to-tui'))).toMatchObject({ ok: true })
|
||||
await vi.waitFor(async () =>
|
||||
expect(await host.handoffStatus(SESSION)).toMatchObject({ owner: 'tui' })
|
||||
)
|
||||
|
||||
// The app restarts and cannot reach the terminal, so this generation restores the session for
|
||||
// reading and holds no handle to the owner it would otherwise stop on a close.
|
||||
await host.flushAllStreamedEvents()
|
||||
store = await AgentSessionRecordStore.open({ directory: join(root, 'store'), hostId: 'local' })
|
||||
tuiRecoverable = false
|
||||
openHandoffHost(transcriptPath)
|
||||
await host.restoreReadableSessions()
|
||||
expect(store.getRecord(SESSION)?.lease).toMatchObject({
|
||||
runtimeKind: 'tui',
|
||||
claimStatus: 'live'
|
||||
})
|
||||
|
||||
failNextDrain()
|
||||
await expect(host.close(SESSION)).rejects.toMatchObject({ step: 'drain-published' })
|
||||
expect(host.hasSession(SESSION)).toBe(true)
|
||||
|
||||
// The terminal answers again, and the status read the reopened pane makes recovers the owner.
|
||||
tuiRecoverable = true
|
||||
expect(await host.handoffStatus(SESSION)).toMatchObject({ owner: 'tui' })
|
||||
expect(await host.requestHandoff(CALLER, handoffRequest('to-native'))).toMatchObject({
|
||||
ok: true
|
||||
})
|
||||
await vi.waitFor(async () =>
|
||||
expect(await host.handoffStatus(SESSION)).toMatchObject({ owner: 'native' })
|
||||
)
|
||||
expect(host['sessions'].get(SESSION)?.hasProviderChild).toBe(true)
|
||||
await sendPending('pending when the retaken chat closes')
|
||||
const settled = captureSettledSubmissions()
|
||||
|
||||
await expect(host.close(SESSION)).resolves.toBeUndefined()
|
||||
|
||||
expect(store.getRecord(SESSION)?.lease).toMatchObject({
|
||||
claimStatus: 'released',
|
||||
ownerProcess: null
|
||||
})
|
||||
expect(hasUnansweredStructuredAgentSessionDispatch(settled.value)).toBe(false)
|
||||
})
|
||||
})
|
||||
|
||||
describe('a quit over an eviction that never got its retry', () => {
|
||||
// Nothing calls `close` a second time when the user quits instead of reopening the chat, so the
|
||||
// quit sweep is the last thing that can hand the lease back — and it only reaches the session if
|
||||
// it still counts a stopped child's unfinished wind-down as owed.
|
||||
it('finishes the wind-down the aborted close left behind', async () => {
|
||||
await attach()
|
||||
await sendPending('pending across an abandoned eviction')
|
||||
const settled = captureSettledSubmissions()
|
||||
failNextDrain()
|
||||
|
||||
await expect(host.close(SESSION)).rejects.toMatchObject({ step: 'drain-published' })
|
||||
expect(host['sessions'].get(SESSION)?.hasProviderChild).toBe(false)
|
||||
expect(store.getRecord(SESSION)?.lease.claimStatus).not.toBe('released')
|
||||
|
||||
await host.flushAllStreamedEvents()
|
||||
|
||||
expect(closeSession).toHaveBeenCalledOnce()
|
||||
expect(host.hasSession(SESSION)).toBe(false)
|
||||
expect(store.getRecord(SESSION)?.lease).toMatchObject({
|
||||
claimStatus: 'released',
|
||||
ownerProcess: null
|
||||
})
|
||||
expect(hasUnansweredStructuredAgentSessionDispatch(settled.value)).toBe(false)
|
||||
})
|
||||
})
|
||||
|
||||
Reference in New Issue
Block a user