mirror of
https://github.com/stablyai/orca.git
synced 2026-10-31 00:57:45 +00:00
fix(native-chat): offer resume after restart for chats whose only work was in the background (#27167)
* fix(native-chat): offer restart resume for chats whose only work was background The quit started the chat teardown and then stopped the hook server, which emptied the status store holding each chat's subagents and background commands. The teardown reads that store to decide which chats were still working, so a chat whose turn had finished but whose background command or subagent was still running was never offered to resume after a restart or update. The quit now stops hook ingress without clearing the store. * test: preserve background-only chat resume across shutdown
This commit is contained in:
@@ -116,6 +116,21 @@ describe('structured child-work ingest', () => {
|
||||
expect(server.getStructuredChildWork(SUBJECT)).toEqual([first])
|
||||
})
|
||||
|
||||
// A quit stops hook ingress before the chat teardown decides which chats to offer to resume,
|
||||
// and that decision reads these records: a chat whose only work was a background command or a
|
||||
// subagent was otherwise never offered.
|
||||
it('keeps the children readable through a quit stop', () => {
|
||||
const server = new AgentHookServer()
|
||||
server.ingestStructuredStatus(summary(), SUBJECT)
|
||||
server.ingestStructuredChildWork(SUBJECT, started, 'claude')
|
||||
server.stop({ keepStatus: true })
|
||||
expect(server.getStructuredChildWorkViews(SUBJECT)).toEqual([
|
||||
expect.objectContaining({ description: 'Audit the build', state: 'working' })
|
||||
])
|
||||
server.stop()
|
||||
expect(server.getStructuredChildWorkViews(SUBJECT)).toEqual([])
|
||||
})
|
||||
|
||||
it('rejects an address that names no session', () => {
|
||||
const server = new AgentHookServer()
|
||||
expect(() =>
|
||||
|
||||
@@ -214,7 +214,9 @@ export abstract class AgentHookServerLifecycle extends AgentHookServerStatusHook
|
||||
this.endpointFileWritten = false
|
||||
}
|
||||
|
||||
stop(): void {
|
||||
/** `keepStatus`: quit only. The chat teardown after it still reads each chat's subagents and
|
||||
* background tasks from the status store to decide what to offer to resume; exit frees it. */
|
||||
stop(options?: { keepStatus?: boolean }): void {
|
||||
// Terminal status may still have a pending write while hook ingress is disabled.
|
||||
if (this.statusHooksEnabled || this.statusPersistTimer) {
|
||||
this.flushStatusPersistSync()
|
||||
@@ -262,8 +264,10 @@ export abstract class AgentHookServerLifecycle extends AgentHookServerStatusHook
|
||||
this.ownerStateInitialized = false
|
||||
// Why: don't unlink the endpoint file — a stale file matches fail-open and avoids a TOCTOU race with a concurrent Orca.
|
||||
clearAllListenerCaches(this.state)
|
||||
this.resetCanonicalStatus()
|
||||
this.notifyStatusChangeListeners()
|
||||
if (!options?.keepStatus) {
|
||||
this.resetCanonicalStatus()
|
||||
this.notifyStatusChangeListeners()
|
||||
}
|
||||
this.paneStatusClearListeners.clear()
|
||||
this.statusDropListeners.clear()
|
||||
this.statusChangeListeners.clear()
|
||||
|
||||
@@ -34,6 +34,7 @@ import {
|
||||
stopStructuredAgentSessionRuntime
|
||||
} from './structured-agent-session-runtime'
|
||||
import { createStructuredAgentSessionLogger } from '../native-chat/agent-session-wire/structured-agent-session-logger'
|
||||
import { createScriptedClaudeRuntime } from './structured-claude-scripted-runtime-test-support'
|
||||
|
||||
vi.mock('../telemetry/client', () => ({ track: vi.fn() }))
|
||||
vi.mock('../telemetry/cohort-classifier', () => ({ getCohortAtEmit: vi.fn() }))
|
||||
@@ -370,3 +371,113 @@ function parentSubject(summaries: AgentSessionStatusSummary[]) {
|
||||
summary.sessionId
|
||||
)
|
||||
}
|
||||
|
||||
describe('Claude background-only resume through hook shutdown', () => {
|
||||
let claude = createScriptedClaudeRuntime([SESSION])
|
||||
let server = new AgentHookServer()
|
||||
|
||||
beforeEach(() => {
|
||||
claude = createScriptedClaudeRuntime([SESSION])
|
||||
server = new AgentHookServer()
|
||||
})
|
||||
|
||||
afterEach(async () => {
|
||||
await claude.dispose()
|
||||
server.stop()
|
||||
})
|
||||
|
||||
it.each([
|
||||
{ trigger: 'quit', kind: 'agent', settled: false },
|
||||
{ trigger: 'update', kind: 'agent', settled: false },
|
||||
{ trigger: 'quit', kind: 'command', settled: false },
|
||||
{ trigger: 'update', kind: 'command', settled: false },
|
||||
{ trigger: 'quit', kind: 'command', settled: true }
|
||||
] as const)(
|
||||
'$trigger with a background $kind (settled: $settled)',
|
||||
async ({ trigger, kind, settled }) => {
|
||||
const host = await claude.install()
|
||||
host.deps.statusSink = {
|
||||
publish: (summary, subject) => server.ingestStructuredStatus(summary, subject),
|
||||
forget: (subject) => server.dropStructuredStatus(subject),
|
||||
publishChildWork: (subject, evidence, provider) =>
|
||||
server.ingestStructuredChildWork(subject, evidence, provider),
|
||||
readChildWork: (subject) => server.getStructuredChildWorkViews(subject)
|
||||
}
|
||||
const params = claude.attachParams(SESSION, null)
|
||||
expect(await host.attach({ callerKey: 'restart-test' }, params)).toMatchObject({ ok: true })
|
||||
if (params.providerHandle?.kind !== 'claude') {
|
||||
throw new Error('missing Claude provider handle')
|
||||
}
|
||||
const providerSession = params.providerHandle.sessionId
|
||||
const message = claude.child(SESSION).handlers.onMessage
|
||||
message?.({
|
||||
type: 'assistant',
|
||||
uuid: 'lead-assistant',
|
||||
session_id: providerSession,
|
||||
parent_tool_use_id: null,
|
||||
message: {
|
||||
role: 'assistant',
|
||||
content: [{ type: 'text', text: 'Finished the foreground reply' }]
|
||||
}
|
||||
})
|
||||
message?.({
|
||||
type: 'system',
|
||||
subtype: 'task_started',
|
||||
uuid: 'background-start',
|
||||
session_id: providerSession,
|
||||
task_id: 'background-1',
|
||||
tool_use_id: 'spawn-1',
|
||||
task_type: kind === 'agent' ? 'local_agent' : 'local_bash',
|
||||
description: 'Audit the build',
|
||||
is_backgrounded: true
|
||||
})
|
||||
if (settled) {
|
||||
message?.({
|
||||
type: 'system',
|
||||
subtype: 'task_notification',
|
||||
uuid: 'background-end',
|
||||
session_id: providerSession,
|
||||
task_id: 'background-1',
|
||||
tool_use_id: 'spawn-1',
|
||||
status: 'completed'
|
||||
})
|
||||
}
|
||||
message?.({
|
||||
type: 'result',
|
||||
subtype: 'success',
|
||||
uuid: 'lead-result',
|
||||
session_id: providerSession,
|
||||
result: 'Finished the foreground reply',
|
||||
is_error: false
|
||||
})
|
||||
await host.flushStreamedEvents(SESSION)
|
||||
await vi.waitFor(() =>
|
||||
expect(server.getStatusSnapshot().find((entry) => entry.structuredHost)).toMatchObject({
|
||||
state: settled ? 'done' : 'working',
|
||||
mainAgent: { state: 'done' }
|
||||
})
|
||||
)
|
||||
const stopping = stopStructuredAgentSessionRuntime({ trigger })
|
||||
server.stop({ keepStatus: true })
|
||||
await stopping
|
||||
expect(
|
||||
server
|
||||
.getStructuredChildWorkViews(makeStructuredAgentStatusSubject(params.location, SESSION))
|
||||
.filter((child) => child.state !== 'done')
|
||||
).toEqual([])
|
||||
const activity = { state: 'done', prompts: [], tasks: [{ kind, label: 'Audit the build' }] }
|
||||
const markers = await host.deps.recoveryCapsule?.list(Date.now())
|
||||
const restarted = await claude.install()
|
||||
const candidates = await restarted.restartResume.list()
|
||||
if (settled) {
|
||||
expect(markers).toEqual([])
|
||||
expect(candidates).toEqual([])
|
||||
} else {
|
||||
expect(markers).toMatchObject([{ sessionId: SESSION, trigger, activity }])
|
||||
expect(candidates).toMatchObject([
|
||||
{ sessionId: SESSION, agent: 'claude', trigger, activity }
|
||||
])
|
||||
}
|
||||
}
|
||||
)
|
||||
})
|
||||
|
||||
@@ -230,7 +230,7 @@ export function createScriptedClaudeRuntime(sessionIds: readonly string[]) {
|
||||
children: (sessionId: string): ScriptedClaudeChild[] =>
|
||||
children.filter((entry) => entry.sessionId === sessionId),
|
||||
install: async (): Promise<StructuredAgentSessionHost> => {
|
||||
root = await mkdtemp(join(tmpdir(), 'orca-scripted-claude-runtime-'))
|
||||
root ??= await mkdtemp(join(tmpdir(), 'orca-scripted-claude-runtime-'))
|
||||
await mkdir(join(root, 'claude-home'), { recursive: true })
|
||||
const directory = root
|
||||
host = await ensureStructuredAgentSessionHost({
|
||||
|
||||
@@ -4,7 +4,7 @@
|
||||
import { mkdtemp, rm } from 'node:fs/promises'
|
||||
import { tmpdir } from 'node:os'
|
||||
import { join } from 'node:path'
|
||||
import { afterEach, describe, expect, it } from 'vitest'
|
||||
import { afterEach, describe, expect, it, vi } from 'vitest'
|
||||
import type {
|
||||
CodexAppServerConnection,
|
||||
CodexAppServerConnectionHandlers,
|
||||
@@ -20,6 +20,14 @@ import {
|
||||
stopStructuredAgentSessionRuntime
|
||||
} from './structured-agent-session-runtime'
|
||||
import { createStructuredAgentSessionLogger } from '../native-chat/agent-session-wire/structured-agent-session-logger'
|
||||
import { AgentHookServer } from '../agent-hooks/server'
|
||||
import { makeStructuredAgentStatusSubject } from '../../shared/agent-status-subject'
|
||||
import { AgentSessionRecoveryCapsule } from './agent-session-recovery-capsule'
|
||||
import {
|
||||
openStructuredCodexRpcHarness,
|
||||
THREAD as RESTART_THREAD,
|
||||
type StructuredCodexRpcHarness
|
||||
} from './structured-codex-session-rpc-test-harness'
|
||||
|
||||
const THREAD = 'thread-runtime-child-work'
|
||||
const CHILD = 'thread-runtime-reviewer'
|
||||
@@ -134,3 +142,126 @@ describe('structured Codex child work through the production runtime', () => {
|
||||
])
|
||||
})
|
||||
})
|
||||
|
||||
describe('Codex background-only resume through hook shutdown', () => {
|
||||
let harness: StructuredCodexRpcHarness | null = null
|
||||
let server: AgentHookServer | null = null
|
||||
|
||||
afterEach(async () => {
|
||||
await harness?.dispose()
|
||||
harness = null
|
||||
server?.stop()
|
||||
server = null
|
||||
})
|
||||
|
||||
it.each([
|
||||
{ trigger: 'quit', kind: 'agent', settled: false },
|
||||
{ trigger: 'update', kind: 'agent', settled: false },
|
||||
{ trigger: 'quit', kind: 'command', settled: false },
|
||||
{ trigger: 'update', kind: 'command', settled: false },
|
||||
{ trigger: 'update', kind: 'command', settled: true }
|
||||
] as const)(
|
||||
'$trigger with a background $kind (settled: $settled)',
|
||||
async ({ trigger, kind, settled }) => {
|
||||
const runtime = await openStructuredCodexRpcHarness()
|
||||
harness = runtime
|
||||
const store = new AgentHookServer()
|
||||
server = store
|
||||
const config = {
|
||||
...runtime.hostConfig(),
|
||||
resolveWorkspacePath: async () => runtime.root,
|
||||
statusSink: {
|
||||
publish: (summary, subject) => store.ingestStructuredStatus(summary, subject),
|
||||
forget: (subject) => store.dropStructuredStatus(subject),
|
||||
publishChildWork: (subject, evidence, provider) =>
|
||||
store.ingestStructuredChildWork(subject, evidence, provider),
|
||||
readChildWork: (subject) => store.getStructuredChildWorkViews(subject)
|
||||
} satisfies StructuredAgentSessionStatusSink
|
||||
}
|
||||
const host = await ensureStructuredAgentSessionHost(config)
|
||||
const params = hostTestAttachParams(null, {
|
||||
providerHandle: undefined,
|
||||
location: {
|
||||
executionHostId: 'local',
|
||||
wslDistro: null,
|
||||
workspaceId: 'workspace-1',
|
||||
workspaceKind: 'folder'
|
||||
}
|
||||
})
|
||||
params.envelope.clientOperationId = `${Date.now()}-${'1'.padStart(32, '0')}`
|
||||
expect(await host.attach({ callerKey: 'restart-test' }, params)).toMatchObject({ ok: true })
|
||||
const notify = runtime.codex.notify
|
||||
notify('turn/started', { threadId: RESTART_THREAD, turn: { id: 'lead-turn' } })
|
||||
if (kind === 'agent') {
|
||||
notify('turn/started', { threadId: CHILD, turn: { id: 'child-turn' } })
|
||||
notify('item/started', {
|
||||
threadId: RESTART_THREAD,
|
||||
turnId: 'lead-turn',
|
||||
item: {
|
||||
type: 'subAgentActivity',
|
||||
id: 'spawn-1',
|
||||
kind: 'started',
|
||||
agentThreadId: CHILD,
|
||||
agentPath: '/root/review'
|
||||
}
|
||||
})
|
||||
} else {
|
||||
const command = {
|
||||
type: 'commandExecution',
|
||||
id: 'command-1',
|
||||
command: 'npm run build',
|
||||
source: 'unifiedExecStartup'
|
||||
}
|
||||
notify('item/started', {
|
||||
threadId: RESTART_THREAD,
|
||||
turnId: 'lead-turn',
|
||||
item: { ...command, status: 'inProgress' }
|
||||
})
|
||||
if (settled) {
|
||||
notify('item/completed', {
|
||||
threadId: RESTART_THREAD,
|
||||
turnId: 'lead-turn',
|
||||
item: { ...command, status: 'completed', exitCode: 0 }
|
||||
})
|
||||
}
|
||||
}
|
||||
notify('turn/completed', {
|
||||
threadId: RESTART_THREAD,
|
||||
turn: { id: 'lead-turn', status: 'completed' }
|
||||
})
|
||||
await host.flushStreamedEvents(SESSION)
|
||||
await vi.waitFor(() =>
|
||||
expect(store.getStatusSnapshot().find((entry) => entry.structuredHost)).toMatchObject({
|
||||
state: settled ? 'done' : 'working',
|
||||
mainAgent: { state: 'done' }
|
||||
})
|
||||
)
|
||||
// The real quit starts async chat teardown, then synchronously stops hook ingress.
|
||||
const stopping = stopStructuredAgentSessionRuntime({ trigger })
|
||||
store.stop({ keepStatus: true })
|
||||
await stopping
|
||||
expect(
|
||||
store
|
||||
.getStructuredChildWorkViews(makeStructuredAgentStatusSubject(params.location, SESSION))
|
||||
.filter((child) => child.state !== 'done')
|
||||
).toEqual([])
|
||||
const activity = {
|
||||
state: 'done',
|
||||
prompts: [],
|
||||
tasks: [{ kind, label: kind === 'agent' ? 'review' : 'npm run build' }]
|
||||
}
|
||||
const markers = await new AgentSessionRecoveryCapsule(runtime.root).list(Date.now())
|
||||
const restarted = await ensureStructuredAgentSessionHost(config)
|
||||
const candidates = await restarted.restartResume.list()
|
||||
if (settled) {
|
||||
expect(markers).toEqual([])
|
||||
expect(candidates).toEqual([])
|
||||
} else {
|
||||
expect(markers).toMatchObject([{ sessionId: SESSION, trigger, activity }])
|
||||
expect(candidates).toMatchObject([
|
||||
{ sessionId: SESSION, agent: 'codex', workspaceKind: 'folder', trigger, activity }
|
||||
])
|
||||
}
|
||||
}
|
||||
)
|
||||
})
|
||||
|
||||
@@ -4,6 +4,7 @@ import { WILL_QUIT_TEARDOWN_DEADLINE_MS } from '../quit-teardown-deadline'
|
||||
|
||||
const settled = () => vi.fn(async () => {})
|
||||
const idle = () => vi.fn()
|
||||
const stopAgentHookServer = idle()
|
||||
|
||||
// Every teardown member settles except the profile store, so only the writer can hold quit.
|
||||
const dependencies: Record<string, Record<string, unknown>> = {
|
||||
@@ -13,7 +14,7 @@ const dependencies: Record<string, Record<string, unknown>> = {
|
||||
'../ipc/pty': { killAllPty: idle() },
|
||||
'../daemon/daemon-provider-state': { disconnectDaemon: settled(), shutdownDaemon: settled() },
|
||||
'../ipc/ssh-shutdown-drain': { beginSshShutdown: settled() },
|
||||
'../agent-hooks/server': { agentHookServer: { stop: idle() } },
|
||||
'../agent-hooks/server': { agentHookServer: { stop: stopAgentHookServer } },
|
||||
'../agent-hooks/wsl-hook-relay-manager': { wslHookRelayManager: { disposeAll: idle() } },
|
||||
'../agent-hooks/managed-agent-hook-controls': {
|
||||
removeManagedAgentHooksAsync: vi.fn(async () => [])
|
||||
@@ -105,3 +106,10 @@ it('releases admission only after the final flush and freeze complete', async ()
|
||||
expect(admission.release).toHaveBeenCalledOnce()
|
||||
expect(app.quit).toHaveBeenCalledOnce()
|
||||
})
|
||||
|
||||
// The chat teardown decides which chats to offer to resume from the hook server's status store,
|
||||
// and finishes after the hook server stops: that stop must leave the store readable.
|
||||
it('stops the hook server without clearing the status store the chat teardown reads', async () => {
|
||||
await startQuit(async () => {})
|
||||
expect(stopAgentHookServer).toHaveBeenCalledWith({ keepStatus: true })
|
||||
})
|
||||
|
||||
@@ -158,7 +158,9 @@ function installWillQuitHandler(): void {
|
||||
state.uninstallRepoMaintenanceIdleGate = null
|
||||
// Why stop, not wait: a delete can run for minutes, and its record makes the next start finish it.
|
||||
stopBackgroundWorktreeRemovals()
|
||||
agentHookServer.stop()
|
||||
// Why keepStatus: the chat teardown above reads each chat's background work from that store
|
||||
// after this returns; cleared, a chat whose only work was a subagent or command is not offered.
|
||||
agentHookServer.stop({ keepStatus: true })
|
||||
// Why Windows only: POSIX hooks short-circuit on ORCA_PANE_KEY, while Windows must register a
|
||||
// bare script path that cannot express the guard and would otherwise keep spawning after quit.
|
||||
// Why bounded here: every other teardown member carries its own ceiling, and this one reaches
|
||||
|
||||
Reference in New Issue
Block a user