fix: reduce supervisor stalls and preserve task delivery
Load Supervisor graphs only when opened, preserve source details on demand, reuse indexed story digests and window selection lists without losing keyboard or focus behavior. Route heavy UI reads through the existing worker, batch scoped queries and construct each publication snapshot once. Keep remote cancellation responsive during model-route cleanup, remove late routes and retain reported usage after response-cap cancellation. Preserve Telegram pending replies through polling conflicts, show delivery errors and honor the final rate-limit cooldown. Schema 62 adds lookup indexes without rewriting user content; older clients cannot open upgraded data. Large review publication still blocks Main and remains documented follow-up work. Record the future storage-service direction without performing the architecture migration. Validation: exact staged code passed isolated full tests (6212 passed, 87 skipped), typecheck and lint. Three-round source performance comparisons, SQLite migration equivalence, native Electron interaction checks and current-source Linux x64 Host validation completed; eight external text-model requests. Release note: Supervisor opens reviews without loading the entire graph, keeps long selection lists responsive and preserves details and keyboard navigation. Remote cancellation and Telegram reply recovery are more reliable; remote fixes require an Agent build containing this change.
This commit is contained in:
@@ -18,6 +18,7 @@ export class AgentAcpConnection {
|
||||
private closed = false
|
||||
private closeInput?: () => void
|
||||
private bridgeOrigin?: string
|
||||
private readonly routeLifetime = new AbortController()
|
||||
|
||||
constructor(
|
||||
process: RuntimeAcpProcessOwner,
|
||||
@@ -43,6 +44,7 @@ export class AgentAcpConnection {
|
||||
this.unsubscribeExit = process.subscribeExit?.(() => {
|
||||
if (this.closed) return
|
||||
const error = new Error('ACP Runtime process exited')
|
||||
this.routeLifetime.abort(error)
|
||||
controller?.error(error)
|
||||
controller = undefined
|
||||
rejectExit(error)
|
||||
@@ -89,17 +91,19 @@ export class AgentAcpConnection {
|
||||
socketPath?: string,
|
||||
imageToolName?: string
|
||||
): Promise<void> {
|
||||
this.routeLifetime.signal.throwIfAborted()
|
||||
if (!this.bridgeOrigin) throw new Error('Shared model bridge is not ready')
|
||||
const response = await fetch(`${this.bridgeOrigin}/session`, {
|
||||
method: 'POST', headers: { 'content-type': 'application/json' },
|
||||
body: JSON.stringify({ sessionId, operationId, ...(socketPath ? { socketPath, imageToolName } : { release: true }) }),
|
||||
signal: AbortSignal.timeout(10_000)
|
||||
signal: AbortSignal.any([this.routeLifetime.signal, AbortSignal.timeout(10_000)])
|
||||
})
|
||||
if (!response.ok) throw new Error(`Shared model bridge route failed (${response.status})`)
|
||||
}
|
||||
dispose(): void {
|
||||
if (this.closed) return
|
||||
this.closed = true
|
||||
this.routeLifetime.abort(new Error('ACP transport is closed'))
|
||||
this.sessions.clear()
|
||||
this.unsubscribeOutput()
|
||||
this.unsubscribeExit()
|
||||
|
||||
@@ -12,6 +12,7 @@ import type {
|
||||
RuntimeAcpProcessOwner
|
||||
} from './runtime-acp-backend'
|
||||
import { AgentOwnedAcpPrompt } from './agent-owned-acp-prompt'
|
||||
import { AgentAcpConnection } from './agent-acp-connection'
|
||||
import { SemanticPromptStore } from './semantic-prompt-store'
|
||||
|
||||
const temporary: string[] = []
|
||||
@@ -23,6 +24,35 @@ afterEach(() => {
|
||||
})
|
||||
|
||||
describe('AgentOwnedAcpPrompt', () => {
|
||||
it.each(['dispose', 'exit'])('aborts pending route HTTP requests on transport %s', async action => {
|
||||
const process = new MemoryProcess()
|
||||
let client!: Client
|
||||
const transport = new AgentAcpConnection(process, undefined, factory => {
|
||||
client = factory()
|
||||
return {} as ClientSideConnection
|
||||
})
|
||||
const signals: AbortSignal[] = []
|
||||
const fetcher = vi.fn<typeof fetch>(async (_input, init) => new Promise((_resolve, reject) => {
|
||||
const signal = init!.signal!
|
||||
signals.push(signal)
|
||||
signal.addEventListener('abort', () => reject(signal.reason), { once: true })
|
||||
}))
|
||||
vi.stubGlobal('fetch', fetcher)
|
||||
try {
|
||||
await client.extNotification!('goodbuddy/modelBridgeReady', { origin: `http://127.0.0.1:1234/${'a'.repeat(43)}` })
|
||||
const install = transport.setModelRoute('session-1', 'operation-1', '/broker').catch(error => error)
|
||||
const release = transport.setModelRoute('session-2', 'operation-2').catch(error => error)
|
||||
expect(signals.every(signal => !signal.aborted)).toBe(true)
|
||||
if (action === 'dispose') transport.dispose()
|
||||
else await process.emitExit()
|
||||
expect(signals.every(signal => signal.aborted)).toBe(true)
|
||||
expect(await install).toBeInstanceOf(Error)
|
||||
expect(await release).toBeInstanceOf(Error)
|
||||
await expect(transport.setModelRoute('session-1', 'operation-1')).rejects.toThrow()
|
||||
expect(fetcher).toHaveBeenCalledTimes(2)
|
||||
} finally { transport.dispose(); vi.unstubAllGlobals() }
|
||||
})
|
||||
|
||||
it('coalesces identical starts and rejects conflicting starts while initialization is pending', async () => {
|
||||
const root = mkdtempSync(join(tmpdir(), 'goodbuddy-owned-start-'))
|
||||
temporary.push(root)
|
||||
@@ -88,7 +118,7 @@ describe('AgentOwnedAcpPrompt', () => {
|
||||
bindingId: 'binding-1', operationId, requestId: operationId,
|
||||
prompt: [{ type: 'text', text: 'Continue' }]
|
||||
})
|
||||
expect(prepareSession).toHaveBeenLastCalledWith('session-1', operationId)
|
||||
expect(prepareSession).toHaveBeenLastCalledWith('session-1', operationId, expect.any(AbortSignal))
|
||||
if (index !== 2) {
|
||||
expect(index === 0 ? newSession : resumeSession).toHaveBeenLastCalledWith(expect.objectContaining({ mcpServers: mcpServers() }))
|
||||
}
|
||||
|
||||
@@ -32,7 +32,7 @@ export type AgentOwnedAcpPromptOptions = {
|
||||
expectedModel?: string
|
||||
process: RuntimeAcpProcessOwner
|
||||
transport?: AgentAcpConnection
|
||||
prepareSession?: (sessionId: string, operationId: string) => Promise<void>
|
||||
prepareSession?: (sessionId: string, operationId: string, signal: AbortSignal) => Promise<void>
|
||||
transcript: SemanticPromptStore
|
||||
completePrompt: (
|
||||
operationId: string,
|
||||
@@ -237,7 +237,7 @@ export class AgentOwnedAcpPrompt {
|
||||
}))
|
||||
}
|
||||
this.#hasMcpServers = mcpServers.length > 0
|
||||
await whileStarting(Promise.resolve(this.#options.prepareSession?.(this.#sessionId!, request.operationId)))
|
||||
await whileStarting(Promise.resolve(this.#options.prepareSession?.(this.#sessionId!, request.operationId, signal)))
|
||||
signal.throwIfAborted()
|
||||
this.#active = {
|
||||
operationId: request.operationId
|
||||
|
||||
@@ -518,6 +518,92 @@ function harness(input: {
|
||||
}
|
||||
|
||||
describe('RuntimeAcpBackend', () => {
|
||||
it.each(['cancel', 'deadline'])('does not hold peer control while releasing a startup route after %s', async reason => {
|
||||
const fixture = await ownedHarness(true)
|
||||
const releaseRoute = deferred()
|
||||
let finishModel: (() => Promise<void>) | undefined
|
||||
let releasing = false
|
||||
try {
|
||||
await fixture.start()
|
||||
const process = fixture.processes[0]!
|
||||
const originalWrite = process.onWrite!
|
||||
process.onWrite = payload => {
|
||||
const request = JSON.parse(Buffer.from(payload).toString())
|
||||
const reply = (result: unknown) => process.emit('stdout', `${JSON.stringify({ jsonrpc: '2.0', id: request.id, result })}\n`)
|
||||
if (request.method === 'session/new') void reply({ sessionId: 'starting-session' })
|
||||
else if (request.method === 'session/set_config_option' && request.params.sessionId === 'starting-session') {
|
||||
finishModel = () => reply({ configOptions: [{ id: 'model', name: 'Model', type: 'select', currentValue: request.params.value, options: [] }] })
|
||||
} else originalWrite(payload)
|
||||
}
|
||||
const opened = await invoke(fixture, 'runtime/openAcpChannel', { ...fixture.openRequest, bindingId: 'starting' }) as { channelEpoch: string }
|
||||
vi.useFakeTimers()
|
||||
await invoke(fixture, 'runtime/preparePrompt', fixture.preparation({ bindingId: 'starting', channelEpoch: opened.channelEpoch,
|
||||
operationId: 'starting-request', requestId: 'starting-request',
|
||||
deadlineAt: reason === 'deadline' ? new Date(1100).toISOString() : UNBOUNDED_REMOTE_PROMPT_DEADLINE }))
|
||||
const starting = invoke(fixture, 'runtime/startPrompt', { bindingId: 'starting', operationId: 'starting-request', requestId: 'starting-request', prompt: [{ type: 'text', text: 'test' }] }).catch(error => error)
|
||||
await vi.advanceTimersByTimeAsync(1)
|
||||
expect(finishModel).toBeDefined()
|
||||
vi.mocked(AgentAcpConnection.prototype.setModelRoute).mockImplementation(async (_session, _operation, socket) => {
|
||||
if (!socket) { releasing = true; await releaseRoute.promise }
|
||||
})
|
||||
let cancelled = reason === 'deadline', peer = false
|
||||
const cancellation = reason === 'cancel' ? invoke(fixture, 'runtime/escalateCancellation', {
|
||||
bindingId: 'starting', operationId: 'starting-request', requestId: 'starting-request', sessionId: 'starting-session', reason: 'requested'
|
||||
}).then(() => { cancelled = true }) : undefined
|
||||
await vi.advanceTimersByTimeAsync(200)
|
||||
expect(releasing).toBe(true)
|
||||
const query = invoke(fixture, 'runtime/getAcpCursors', { bindingId: 'binding-1' }).then(() => { peer = true })
|
||||
await vi.advanceTimersByTimeAsync(200)
|
||||
const observed = { cancelled, peer }
|
||||
releaseRoute.resolve()
|
||||
await finishModel!()
|
||||
finishModel = undefined
|
||||
await Promise.all([starting, cancellation, query])
|
||||
expect(observed).toEqual({ cancelled: true, peer: true })
|
||||
expect(process.stops).toHaveLength(0)
|
||||
expect(fixture.transcript.attach('binding-1', 'request-1', fixture.context.controller.controllerId).state).toBe('running')
|
||||
expect(fixture.page().events.filter(event => event.kind === 'prompt-terminal')).toHaveLength(0)
|
||||
} finally {
|
||||
releaseRoute.resolve(); await finishModel?.(); await fixture.prompts[0]?.()
|
||||
vi.useRealTimers(); await fixture.cleanup()
|
||||
}
|
||||
})
|
||||
|
||||
it('removes a late startup route after cancellation without revoking its peer route', async () => {
|
||||
const fixture = await ownedHarness(true)
|
||||
const install = deferred()
|
||||
const entered = deferred()
|
||||
const routes = new Map<string, string>([['session-1', 'request-1']])
|
||||
try {
|
||||
await fixture.start()
|
||||
const process = fixture.processes[0]!
|
||||
const originalWrite = process.onWrite!
|
||||
process.onWrite = payload => {
|
||||
const request = JSON.parse(Buffer.from(payload).toString())
|
||||
if (request.method === 'session/new') void process.emit('stdout', `${JSON.stringify({ jsonrpc: '2.0', id: request.id, result: { sessionId: 'starting-session' } })}\n`)
|
||||
else originalWrite(payload)
|
||||
}
|
||||
vi.mocked(AgentAcpConnection.prototype.setModelRoute).mockImplementation(async (session, operation, socket) => {
|
||||
if (socket) { entered.resolve(); await install.promise; routes.set(session, operation) }
|
||||
else if (routes.get(session) === operation) routes.delete(session)
|
||||
})
|
||||
const opened = await invoke(fixture, 'runtime/openAcpChannel', { ...fixture.openRequest, bindingId: 'starting' }) as { channelEpoch: string }
|
||||
await invoke(fixture, 'runtime/preparePrompt', fixture.preparation({ bindingId: 'starting', channelEpoch: opened.channelEpoch,
|
||||
operationId: 'starting-request', requestId: 'starting-request', deadlineAt: UNBOUNDED_REMOTE_PROMPT_DEADLINE }))
|
||||
const starting = invoke(fixture, 'runtime/startPrompt', { bindingId: 'starting', operationId: 'starting-request', requestId: 'starting-request', prompt: [{ type: 'text', text: 'test' }] }).catch(error => error)
|
||||
await entered.promise
|
||||
await invoke(fixture, 'runtime/escalateCancellation', { bindingId: 'starting', operationId: 'starting-request', requestId: 'starting-request', sessionId: 'starting-session', reason: 'requested' })
|
||||
expect(await starting).toBeInstanceOf(Error)
|
||||
install.resolve()
|
||||
await new Promise(resolve => setImmediate(resolve))
|
||||
expect([...routes]).toEqual([['session-1', 'request-1']])
|
||||
expect(process.stops).toHaveLength(0)
|
||||
const page = fixture.transcript.page({ bindingId: 'starting', operationId: 'starting-request', controllerId: fixture.context.controller.controllerId, afterSequence: '0', limit: 10 })
|
||||
expect(page.state).toBe('cancelled')
|
||||
expect(page.events.filter(event => event.kind === 'prompt-terminal')).toHaveLength(1)
|
||||
} finally { install.resolve(); await fixture.prompts[0]?.(); await fixture.cleanup() }
|
||||
})
|
||||
|
||||
it.each(['watchdog', 'dispose'])('interrupts stalled native initialization through %s', async action => {
|
||||
vi.useFakeTimers()
|
||||
const fixture = harness({ agentOwned: true, shareOwnedProcesses: true })
|
||||
|
||||
@@ -1502,11 +1502,17 @@ export class RuntimeAcpBackend {
|
||||
process: binding.process,
|
||||
...(binding.sharedProcess ? {
|
||||
transport: binding.sharedProcess.transport,
|
||||
prepareSession: async (sessionId: string, operationId: string) => {
|
||||
await binding.sharedProcess!.transport.setModelRoute(
|
||||
sessionId, operationId, binding.modelBridgeBroker!.socketPath,
|
||||
binding.imageTool ? remoteImageToolMcpName(binding.request.bindingId) : undefined
|
||||
)
|
||||
prepareSession: async (sessionId: string, operationId: string, signal: AbortSignal) => {
|
||||
const transport = binding.sharedProcess!.transport
|
||||
try {
|
||||
await transport.setModelRoute(
|
||||
sessionId, operationId, binding.modelBridgeBroker!.socketPath,
|
||||
binding.imageTool ? remoteImageToolMcpName(binding.request.bindingId) : undefined
|
||||
)
|
||||
} finally {
|
||||
// Cancellation can release the route before this installation finishes.
|
||||
if (signal.aborted) await transport.setModelRoute(sessionId, operationId).catch(() => undefined)
|
||||
}
|
||||
}
|
||||
} : {}),
|
||||
transcript: this.#options.semanticPrompts,
|
||||
@@ -1897,7 +1903,9 @@ export class RuntimeAcpBackend {
|
||||
binding.modelBridgeClient = undefined
|
||||
let brokerError: unknown
|
||||
if (binding.sharedProcess && binding.ownedAcp?.sessionId && binding.activeOperationId) {
|
||||
await binding.sharedProcess.transport.setModelRoute(
|
||||
// Broker shutdown below revokes dispatch authority. Native route cleanup
|
||||
// is bounded by the transport and must not hold the global control queue.
|
||||
void binding.sharedProcess.transport.setModelRoute(
|
||||
binding.ownedAcp.sessionId, binding.activeOperationId
|
||||
).catch(() => undefined)
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user