diff --git a/packages/host/apiproxy/src/api-proxy.ts b/packages/host/apiproxy/src/api-proxy.ts index 0315349fff..f178bfefd0 100644 --- a/packages/host/apiproxy/src/api-proxy.ts +++ b/packages/host/apiproxy/src/api-proxy.ts @@ -617,6 +617,11 @@ export function createApiProxy(ctx: Context, defaults: ApiProxyDefaults): ApiPro for (const pending of [...pendingApprovals.values()]) pending.resolve('cancelled') }, 'api-proxy: approval registry teardown') ctx.on('approval/request', (req, next) => { + // Dispatch rides a microtask behind the service's own signal check: an + // abort landing in that window would register the abort listener AFTER + // the signal fired — never invoked, entry pending forever, zombie frame + // on every mux replay. Settle synchronously instead of publishing. + if (req.signal?.aborted === true) return Promise.resolve('cancelled') // The audit pair `approval/asked` is already appended by the service // before dispatch, but dispatch rides a microtask: parallel tool calls // can append several asked events before any answerer runs. THIS @@ -634,7 +639,12 @@ export function createApiProxy(ctx: Context, defaults: ApiProxyDefaults): ApiPro decided.add(event.data.id) } else if (event.type === 'approval/asked') { if (decided.has(event.data.id) || claimed.has(event.data.id)) continue - if (req.callId !== undefined && event.data.callId !== req.callId) continue + // Symmetric pairing: a callId-bearing ask only takes its own call's + // record, and a callId-less ask only takes a callId-less record — + // so neither shape can steal the other's audit id under parallel + // asks. (Today every producer — the tool executor — passes callId; + // the callId-less arm guards any future non-tool asker.) + if ((req.callId ?? null) !== (event.data.callId ?? null)) continue approvalId = event.data.id break } diff --git a/packages/host/apiproxy/tests/api-proxy-approval.spec.ts b/packages/host/apiproxy/tests/api-proxy-approval.spec.ts index 632fa898de..f9a00cf9ef 100644 --- a/packages/host/apiproxy/tests/api-proxy-approval.spec.ts +++ b/packages/host/apiproxy/tests/api-proxy-approval.spec.ts @@ -176,6 +176,36 @@ describe('approval pending registry', () => { abort.abort() }) + it('an ask whose signal aborted before dispatch settles cancelled without publishing', async () => { + // The service checks the signal, then dispatch rides a microtask: an + // abort in that window must not register a dead listener and strand the + // entry (zombie frame on every replay). Drive the waterfall directly + // with a pre-aborted signal to hit the answerer's register-path guard. + const { ctx, api } = await harness() + const abort = new AbortController() + const mux = openMux(api, abort) + const session = ctx.sessions.create() + session.append('turn/start', { turn: 1, trigger: { kind: 'message', source: { kind: 'user' } } }) + session.append('approval/asked', { id: 'pre-aborted' as ApprovalRequestId, toolName: 'bash' }) + const agent = { session } as unknown as Agent + const cancelled = new AbortController() + cancelled.abort() + const outcome = await ctx.waterfall( + 'approval/request', + { agent, toolName: 'bash', signal: cancelled.signal }, + () => Promise.resolve('unavailable' as const), + ) + expect(outcome).toBe('cancelled') + // Nothing was published: a fresh mux open replays no approval frame. + const abort2 = new AbortController() + const mux2 = openMux(api, abort2) + await new Promise(resolve => setTimeout(resolve, 10)) + expect(mux2.envelopes.some(e => e.payload.type === 'approval/requested')).toBe(false) + abort2.abort() + abort.abort() + void mux + }) + it('gateway teardown settles pending approvals as cancelled (question-provider parity)', async () => { // Mount the proxy on its own fiber so disposal exercises the teardown // effect while an ask is still pending.