diff --git a/.changeset/approval-decision-validation.md b/.changeset/approval-decision-validation.md new file mode 100644 index 0000000..03ef48a --- /dev/null +++ b/.changeset/approval-decision-validation.md @@ -0,0 +1,20 @@ +--- +'@openrouter/agent': patch +--- + +Harden approval resumes. + +`approveToolCalls` / `rejectToolCalls` are now de-duplicated, so a call ID listed twice no longer executes an approval-gated tool twice (or skips the re-approval a `PreToolUse` argument mutation requires), and passing the same ID in both lists throws before anything runs. + +Resuming an `awaiting_approval` conversation without any decision now leaves it paused instead of sending a new model request. Any `input` passed with that call is not sent to the model and not saved to state, including a `function_call_output` for the gated call: only an approve/reject decision releases an approval gate. + +An `awaiting_hitl` conversation still resumes the documented way, by passing a `function_call_output` for a paused call in `input` (which runs `onResponseReceived`). Without one, and without decisions, it now stays paused too. A paused status with no pending tool calls continues with a normal request, as before. + +While a resume leaves the conversation paused, `getResponse()`, `getText()`, `getToolCalls()` and the stream getters reject with an explicit error that names the pause, lists up to 10 pending call IDs, and says when supplied `input` was dropped (previously the generic "Response not available" / "Stream not initialized"). `requiresApproval()`, `getPendingToolCalls()` and `getState()` keep working. + +```ts +const result = callModel(client, { model, input: [], tools, state }); +// state is awaiting_approval and no decisions were passed: +await result.requiresApproval(); // true, and no request was sent +await result.getResponse(); // throws: Conversation is awaiting approval ... Pending tool call IDs: call_1 +``` diff --git a/packages/agent/src/lib/async-params.ts b/packages/agent/src/lib/async-params.ts index f2f72e7..573e30a 100644 --- a/packages/agent/src/lib/async-params.ts +++ b/packages/agent/src/lib/async-params.ts @@ -248,9 +248,27 @@ type BaseCallModelInput< type ApprovalParamsWithState = { /** State accessor for multi-turn persistence and approval gates */ state: StateAccessor; - /** Tool call IDs to approve (for resuming from awaiting_approval status) */ + /** + * Tool call IDs to approve when resuming a paused conversation + * (`awaiting_approval`, or `awaiting_hitl` to re-run `onToolCalled`). + * + * Duplicate IDs are ignored: each approved call executes once. An ID that + * also appears in `rejectToolCalls` makes `callModel` throw before any + * state is loaded or any tool runs. + * + * Resuming an `awaiting_approval` conversation without any decision leaves + * it paused: no model request is sent, and `input` passed with that call is + * neither sent nor saved. + */ approveToolCalls?: string[]; - /** Tool call IDs to reject (for resuming from awaiting_approval status) */ + /** + * Tool call IDs to reject when resuming a paused conversation. The model + * receives a rejection output for each. + * + * Duplicate IDs are ignored: each rejection is recorded once. An ID that + * also appears in `approveToolCalls` makes `callModel` throw before any + * state is loaded or any tool runs. + */ rejectToolCalls?: string[]; }; diff --git a/packages/agent/src/lib/model-result.ts b/packages/agent/src/lib/model-result.ts index ceebd86..166fb96 100644 --- a/packages/agent/src/lib/model-result.ts +++ b/packages/agent/src/lib/model-result.ts @@ -264,6 +264,13 @@ function extractServerToolIdentity(item: ServerToolResultItem): Record | null = null; // Unified turn broadcaster for multi-turn streaming private turnBroadcaster: ToolEventBroadcaster> | null = @@ -805,8 +813,23 @@ export class ModelResult< // Initialize state management this.stateAccessor = options.state ?? null; this.requireApprovalFn = options.requireApproval ?? null; - this.approvedToolCalls = options.approveToolCalls ?? []; - this.rejectedToolCalls = options.rejectToolCalls ?? []; + // De-duplicate decisions so a call ID listed twice cannot execute an + // approval-gated tool twice, and refuse contradictory input outright: + // an ID that is both approved and rejected has no safe interpretation. + this.approvedToolCalls = [ + ...new Set(options.approveToolCalls ?? []), + ]; + this.rejectedToolCalls = [ + ...new Set(options.rejectToolCalls ?? []), + ]; + const rejectedIds = new Set(this.rejectedToolCalls); + const conflictingIds = this.approvedToolCalls.filter((id) => rejectedIds.has(id)); + if (conflictingIds.length > 0) { + throw new Error( + `Tool call IDs cannot be both approved and rejected: ${conflictingIds.join(', ')}. ` + + 'Pass each pending call ID in exactly one of approveToolCalls or rejectToolCalls.', + ); + } // Concurrency gates. Round default stays unbounded (existing behavior); // the background pool always has a cap (default 16 — detached work has @@ -2596,7 +2619,7 @@ export class ModelResult< try { await this.initStream(); if (requireStream && !this.reusableStream && !this.finalResponse) { - throw new Error('Stream not initialized'); + throw this.responseUnavailableError('Stream not initialized'); } } catch (error) { await this.finishHooksSessionForStream('error'); @@ -5653,6 +5676,61 @@ export class ModelResult< // Core Methods // ========================================================================= + /** + * Initialize the context store and resolve the turn-0 request exactly once. + * Caller-supplied async parameter functions (including a function-valued + * `input`) must not run twice when the paused-resume check needs the + * resolved input before the normal initialization path does. + */ + private resolveInitialRequest(initialContext: TurnContext): Promise { + this.initialRequestPromise ??= this.resolveInitialRequestUncached(initialContext); + return this.initialRequestPromise; + } + + private async resolveInitialRequestUncached( + initialContext: TurnContext, + ): Promise { + if (this.options.context !== undefined) { + const resolvedCtx = await resolveContext(this.options.context, initialContext); + this.contextStore = new ToolContextStore(resolvedCtx); + } + return this.resolveRequestForContext(initialContext); + } + + /** + * Whether a state loaded as `awaiting_approval` / `awaiting_hitl` stays + * paused when the caller supplied no approve/reject decisions. + * + * - No pending calls: nothing is owed a decision, so there is nothing to + * wait for. Continue with a normal request instead of stranding the + * conversation behind a pause nobody can resolve. + * - `awaiting_approval`: always paused. Only a recorded approve/reject + * decision releases an approval gate; a caller-supplied + * `function_call_output` for the gated call does not. + * - `awaiting_hitl`: paused unless `input` carries a `function_call_output` + * for a pending call. That is the documented way to resume a HITL tool + * (see `HITLToolFunction`): the output goes through `onResponseReceived` + * and on to the model via the normal request path. + */ + private async staysPausedWithoutDecisions( + loadedState: ConversationState, + ): Promise { + const pendingIds = new Set((loadedState.pendingToolCalls ?? []).map((tc) => tc.id)); + if (pendingIds.size === 0) { + return false; + } + if (loadedState.status !== 'awaiting_hitl') { + return true; + } + const { input } = await this.resolveInitialRequest({ + numberOfTurns: 0, + }); + if (!Array.isArray(input)) { + return true; + } + return !input.some((item) => isFunctionCallOutputItem(item) && pendingIds.has(item.callId)); + } + /** * Initialize the stream if not already started * This is idempotent - multiple calls will return the same promise @@ -5732,6 +5810,16 @@ export class ModelResult< return; // Skip normal initialization, we're resuming } + // Paused for a human decision but none was supplied: stay paused. + // Falling through would flip the status to 'in_progress' and send a + // fresh model request, advancing the conversation past calls that + // still owe a decision. Every response/stream getter surfaces this + // as an explicit "awaiting ..." error (see pausedError()). + if (isResumableStatus && (await this.staysPausedWithoutDecisions(loadedState))) { + this.isResumingFromApproval = true; + return; + } + // Check for interruption flag and handle if (loadedState.interruptedBy) { // Clear interruption flag and continue from saved state @@ -5770,14 +5858,10 @@ export class ModelResult< numberOfTurns: 0, }; - // Initialize context store from the context option - if (this.options.context !== undefined) { - const resolvedCtx = await resolveContext(this.options.context, initialContext); - this.contextStore = new ToolContextStore(resolvedCtx); - } - - // Resolve any async functions first - let baseRequest = await this.resolveRequestForContext(initialContext); + // Initialize the context store and resolve any async functions first. + // Memoized: an `awaiting_hitl` resume already resolved the request to + // look for caller-supplied outputs (staysPausedWithoutDecisions). + let baseRequest = await this.resolveInitialRequest(initialContext); // Emit SessionStart hook. The `config` payload carries a stable, small // summary of session-level options so handlers can make routing/auditing @@ -6761,12 +6845,58 @@ export class ModelResult< await this.executeToolsIfNeeded(); if (!this.finalResponse) { - throw new Error('Response not available'); + throw this.responseUnavailableError(); } return extractTextFromResponse(this.finalResponse); } + /** + * Error for a getter that has nothing to return. A resume that leaves the + * conversation paused (no decisions supplied, or decisions for only some of + * the pending calls) produces neither a response nor a stream; say so + * explicitly instead of the generic message. + */ + private responseUnavailableError(fallbackMessage = 'Response not available'): Error { + return this.pausedError() ?? new Error(fallbackMessage); + } + + /** + * Describes the pause when the conversation is waiting on the caller + * (`awaiting_approval` / `awaiting_hitl`), or null when it is not paused. + */ + private pausedError(): Error | null { + const status = this.currentState?.status; + if (status !== 'awaiting_approval' && status !== 'awaiting_hitl') { + return null; + } + const pendingIds = (this.currentState?.pendingToolCalls ?? []).map((tc) => tc.id); + const shownIds = pendingIds.slice(0, MAX_PENDING_IDS_IN_ERROR).join(', '); + const hiddenCount = pendingIds.length - MAX_PENDING_IDS_IN_ERROR; + const idList = hiddenCount > 0 ? `${shownIds}, ... (${hiddenCount} more)` : shownIds; + const [what, howToResume] = + status === 'awaiting_approval' + ? [ + 'awaiting approval', + 'Resume with approveToolCalls and/or rejectToolCalls for the pending calls.', + ] + : [ + 'awaiting HITL input', + 'Resume by passing a function_call_output for a pending call ID in input, ' + + 'or with approveToolCalls / rejectToolCalls (approving re-runs onToolCalled).', + ]; + const input = this.options.request.input; + const inputSupplied = typeof input === 'function' || (input?.length ?? 0) > 0; + return new Error( + `Conversation is ${what} (status '${status}'); no response is available. ` + + `Pending tool call IDs: ${idList}. ${howToResume} ` + + 'Use requiresApproval() or getPendingToolCalls() to inspect a paused conversation.' + + (inputSupplied + ? ' The input supplied with this call was not sent to the model and was not saved to state.' + : ''), + ); + } + /** * Get just the text content from the response. * This will consume the stream until completion, execute any tools, and extract the text. @@ -6793,7 +6923,7 @@ export class ModelResult< await this.executeToolsIfNeeded(); if (!this.finalResponse) { - throw new Error('Response not available'); + throw this.responseUnavailableError(); } return this.finalResponse; @@ -7276,7 +7406,7 @@ export class ModelResult< } if (!this.reusableStream) { - throw new Error('Stream not initialized'); + throw this.responseUnavailableError('Stream not initialized'); } const completedResponse = await this.getInitialResponse(); diff --git a/packages/agent/tests/unit/approval-gate-regressions.test.ts b/packages/agent/tests/unit/approval-gate-regressions.test.ts index 76ae9ca..46a2ab8 100644 --- a/packages/agent/tests/unit/approval-gate-regressions.test.ts +++ b/packages/agent/tests/unit/approval-gate-regressions.test.ts @@ -1993,3 +1993,683 @@ describe('allowFinalResponse path enforces the approval gate (#54)', () => { expect(text).toBe('Done.'); }); }); + +// --------------------------------------------------------------------------- +// Approval decisions are validated before anything runs (typescript-sdk#518), +// and a paused conversation resumed without decisions stays paused +// (typescript-sdk#517). +// --------------------------------------------------------------------------- +describe('approval decision validation and paused resumes (#517, #518)', () => { + const doneResponse = (id: string) => + makeResponse(id, [ + { + type: 'message', + id: `msg_${id}`, + role: 'assistant', + status: 'completed', + content: [ + { + type: 'output_text', + text: 'Done.', + annotations: [], + }, + ], + }, + ]); + + const request = { + model: 'test-model', + input: 'run the gated action', + tools: [ + { + type: 'function' as const, + name: 'gated', + description: null, + strict: null, + parameters: {}, + }, + ], + }; + + function functionCallOutputs(callIndex: number): Array<{ + type?: string; + callId?: string; + output?: unknown; + }> { + const sent = mockBetaResponsesSend.mock.calls[callIndex]?.[1]?.responsesRequest; + const input = (sent?.input ?? []) as Array<{ + type?: string; + callId?: string; + output?: unknown; + }>; + return input.filter((item) => item?.type === 'function_call_output'); + } + + function callerOutput(callId: string, output: unknown): models.FunctionCallOutputItem { + return { + type: 'function_call_output', + id: `output_${callId}`, + callId, + output: JSON.stringify(output), + }; + } + + /** Runs one turn in which a HITL tool pauses on `call_hitl`; returns the tool + state. */ + async function pauseOnHitlCall() { + const onToolCalled = vi.fn(async (_input: { v: string }) => null); + const onResponseReceived = vi.fn(async (raw: unknown) => ({ + answer: String( + ( + raw as { + answer?: unknown; + } + ).answer, + ).toUpperCase(), + })); + const review = tool({ + name: 'gated', + inputSchema: z.object({ + v: z.string(), + }), + outputSchema: z.object({ + answer: z.string(), + }), + onToolCalled, + onResponseReceived, + }); + const tools = [ + review, + ] as const; + mockBetaResponsesSend.mockResolvedValueOnce({ + ok: true, + value: makeResponse('resp_hitl', [ + makeFunctionCallItem('call_hitl', 'gated', '{"v":"x"}'), + ]), + }); + const memory = createMemoryAccessor(); + await new ModelResult({ + request, + client: {} as OpenRouterCore, + tools, + state: memory.accessor, + }).getResponse(); + expect(memory.get()?.status).toBe('awaiting_hitl'); + expect(onToolCalled).toHaveBeenCalledTimes(1); + mockBetaResponsesSend.mockClear(); + return { + onToolCalled, + onResponseReceived, + tools, + ...memory, + }; + } + + /** Runs one turn that pauses on gated calls with `callIds`; returns the tool + state. */ + async function pauseOnGatedCalls(callIds: string[]) { + const execute = vi.fn(async (_input: { v: string }) => ({ + ok: true, + })); + const gated = tool({ + name: 'gated', + inputSchema: z.object({ + v: z.string(), + }), + outputSchema: z.object({ + ok: z.boolean(), + }), + requireApproval: true, + execute, + }); + const tools = [ + gated, + ] as const; + mockBetaResponsesSend.mockResolvedValueOnce({ + ok: true, + value: makeResponse( + 'resp_pause', + callIds.map((id) => makeFunctionCallItem(id, 'gated', '{"v":"x"}')), + ), + }); + const memory = createMemoryAccessor(); + await new ModelResult({ + request, + client: {} as OpenRouterCore, + tools, + state: memory.accessor, + }).getResponse(); + expect(memory.get()?.status).toBe('awaiting_approval'); + expect(execute).not.toHaveBeenCalled(); + mockBetaResponsesSend.mockClear(); + return { + execute, + tools, + ...memory, + }; + } + + beforeEach(() => { + mockBetaResponsesSend.mockReset(); + }); + + it('executes an approved call once when its ID is listed twice', async () => { + const paused = await pauseOnGatedCalls([ + 'call_1', + ]); + mockBetaResponsesSend.mockResolvedValueOnce({ + ok: true, + value: doneResponse('resp_done'), + }); + + const text = await new ModelResult({ + request, + client: {} as OpenRouterCore, + tools: paused.tools, + state: paused.accessor, + approveToolCalls: [ + 'call_1', + 'call_1', + ], + }).getText(); + + expect(text).toBe('Done.'); + expect(paused.execute).toHaveBeenCalledTimes(1); + expect(functionCallOutputs(0).map((item) => item.callId)).toEqual([ + 'call_1', + ]); + }); + + it('records a rejection once when its ID is listed twice', async () => { + const paused = await pauseOnGatedCalls([ + 'call_1', + ]); + mockBetaResponsesSend.mockResolvedValueOnce({ + ok: true, + value: doneResponse('resp_done'), + }); + + await new ModelResult({ + request, + client: {} as OpenRouterCore, + tools: paused.tools, + state: paused.accessor, + rejectToolCalls: [ + 'call_1', + 'call_1', + ], + }).getResponse(); + + expect(paused.execute).not.toHaveBeenCalled(); + expect(functionCallOutputs(0).map((item) => item.callId)).toEqual([ + 'call_1', + ]); + }); + + it('refuses an ID that is both approved and rejected before anything runs', async () => { + const paused = await pauseOnGatedCalls([ + 'call_1', + 'call_2', + ]); + const before = structuredClone(paused.get()); + + expect( + () => + new ModelResult({ + request, + client: {} as OpenRouterCore, + tools: paused.tools, + state: paused.accessor, + approveToolCalls: [ + 'call_1', + 'call_2', + ], + rejectToolCalls: [ + 'call_2', + ], + }), + ).toThrow(/both approved and rejected: call_2/); + + expect(paused.execute).not.toHaveBeenCalled(); + expect(mockBetaResponsesSend).not.toHaveBeenCalled(); + expect(paused.get()).toEqual(before); + }); + + it('stays awaiting_approval and sends no request when resumed without decisions', async () => { + const paused = await pauseOnGatedCalls([ + 'call_1', + ]); + const before = structuredClone(paused.get()); + mockBetaResponsesSend.mockResolvedValueOnce({ + ok: true, + value: doneResponse('resp_should_not_be_requested'), + }); + + const result = new ModelResult({ + request, + client: {} as OpenRouterCore, + tools: paused.tools, + state: paused.accessor, + }); + + await expect(result.getResponse()).rejects.toThrow(/awaiting approval.*call_1/); + await expect(result.getText()).rejects.toThrow(/awaiting approval/); + expect(await result.requiresApproval()).toBe(true); + expect((await result.getPendingToolCalls()).map((tc) => tc.id)).toEqual([ + 'call_1', + ]); + + expect(mockBetaResponsesSend).not.toHaveBeenCalled(); + expect(paused.execute).not.toHaveBeenCalled(); + expect(paused.get()?.status).toBe('awaiting_approval'); + expect(paused.get()).toEqual(before); + + // The pause still resolves normally once a decision is supplied. + await new ModelResult({ + request, + client: {} as OpenRouterCore, + tools: paused.tools, + state: paused.accessor, + approveToolCalls: [ + 'call_1', + ], + }).getResponse(); + expect(paused.execute).toHaveBeenCalledTimes(1); + expect(paused.get()?.status).toBe('complete'); + }); + + it('stays awaiting_hitl and sends no request when resumed without decisions', async () => { + const onToolCalled = vi.fn(async () => null); + const review = tool({ + name: 'gated', + inputSchema: z.object({ + v: z.string(), + }), + outputSchema: z.object({ + ok: z.boolean(), + }), + onToolCalled, + }); + const tools = [ + review, + ] as const; + mockBetaResponsesSend.mockResolvedValueOnce({ + ok: true, + value: makeResponse('resp_hitl', [ + makeFunctionCallItem('call_hitl', 'gated', '{"v":"x"}'), + ]), + }); + const { accessor, get } = createMemoryAccessor(); + await new ModelResult({ + request, + client: {} as OpenRouterCore, + tools, + state: accessor, + }).getResponse(); + expect(get()?.status).toBe('awaiting_hitl'); + mockBetaResponsesSend.mockClear(); + mockBetaResponsesSend.mockResolvedValueOnce({ + ok: true, + value: doneResponse('resp_should_not_be_requested'), + }); + + const result = new ModelResult({ + request, + client: {} as OpenRouterCore, + tools, + state: accessor, + }); + + await expect(result.getResponse()).rejects.toThrow(/awaiting HITL input.*call_hitl/); + expect(mockBetaResponsesSend).not.toHaveBeenCalled(); + expect(onToolCalled).toHaveBeenCalledTimes(1); + expect(get()?.status).toBe('awaiting_hitl'); + }); + + it('reports the pause, not a generic error, when decisions cover only some pending calls', async () => { + const paused = await pauseOnGatedCalls([ + 'call_1', + 'call_2', + ]); + + const result = new ModelResult({ + request, + client: {} as OpenRouterCore, + tools: paused.tools, + state: paused.accessor, + approveToolCalls: [ + 'call_1', + ], + }); + + await expect(result.getResponse()).rejects.toThrow(/awaiting approval.*call_2/); + expect(paused.execute).toHaveBeenCalledTimes(1); + expect(mockBetaResponsesSend).not.toHaveBeenCalled(); + expect(paused.get()?.status).toBe('awaiting_approval'); + }); + + // The documented way to resume a HITL tool: the caller supplies a + // function_call_output for the paused call through `input`, with no + // approve/reject decision. That must keep working on persisted state. + it('resumes awaiting_hitl when input carries a function_call_output for the paused call', async () => { + const paused = await pauseOnHitlCall(); + mockBetaResponsesSend.mockResolvedValueOnce({ + ok: true, + value: doneResponse('resp_done'), + }); + + const text = await new ModelResult({ + request: { + ...request, + input: [ + callerOutput('call_hitl', { + answer: 'yes', + }), + ], + }, + client: {} as OpenRouterCore, + tools: paused.tools, + state: paused.accessor, + }).getText(); + + expect(text).toBe('Done.'); + expect(paused.onResponseReceived).toHaveBeenCalledTimes(1); + expect(paused.onResponseReceived.mock.calls[0]?.[0]).toEqual({ + answer: 'yes', + }); + // The paused call was not re-run; the model got the hook-transformed output. + expect(paused.onToolCalled).toHaveBeenCalledTimes(1); + expect(mockBetaResponsesSend).toHaveBeenCalledTimes(1); + expect(functionCallOutputs(0)).toEqual([ + expect.objectContaining({ + callId: 'call_hitl', + output: JSON.stringify({ + answer: 'YES', + }), + }), + ]); + expect(paused.get()?.status).toBe('complete'); + }); + + it('resolves a function-valued input once when it resumes awaiting_hitl', async () => { + const paused = await pauseOnHitlCall(); + mockBetaResponsesSend.mockResolvedValueOnce({ + ok: true, + value: doneResponse('resp_done'), + }); + const input = vi.fn(async () => [ + callerOutput('call_hitl', { + answer: 'yes', + }), + ]); + + const text = await new ModelResult({ + request: { + ...request, + input, + }, + client: {} as OpenRouterCore, + tools: paused.tools, + state: paused.accessor, + }).getText(); + + expect(text).toBe('Done.'); + expect(input).toHaveBeenCalledTimes(1); + expect(paused.onResponseReceived).toHaveBeenCalledTimes(1); + }); + + it('stays awaiting_hitl when input has no output for a pending call, and says the input was dropped', async () => { + const paused = await pauseOnHitlCall(); + const before = structuredClone(paused.get()); + + for (const input of [ + 'a new user message', + [ + callerOutput('call_other', { + answer: 'yes', + }), + ], + ]) { + const result = new ModelResult({ + request: { + ...request, + input, + }, + client: {} as OpenRouterCore, + tools: paused.tools, + state: paused.accessor, + }); + await expect(result.getResponse()).rejects.toThrow( + /awaiting HITL input.*call_hitl.*function_call_output.*input supplied with this call was not sent/, + ); + } + + expect(mockBetaResponsesSend).not.toHaveBeenCalled(); + expect(paused.onResponseReceived).not.toHaveBeenCalled(); + expect(paused.get()).toEqual(before); + }); + + // An approval gate is released only by a recorded decision. A caller-made + // output for the gated call is not one, so it must not advance the run. + it('stays awaiting_approval when input carries a function_call_output for the gated call', async () => { + const paused = await pauseOnGatedCalls([ + 'call_1', + ]); + const before = structuredClone(paused.get()); + + const result = new ModelResult({ + request: { + ...request, + input: [ + callerOutput('call_1', { + ok: true, + }), + ], + }, + client: {} as OpenRouterCore, + tools: paused.tools, + state: paused.accessor, + }); + + await expect(result.getResponse()).rejects.toThrow( + /awaiting approval.*call_1.*input supplied with this call was not sent/, + ); + expect(mockBetaResponsesSend).not.toHaveBeenCalled(); + expect(paused.execute).not.toHaveBeenCalled(); + expect(paused.get()).toEqual(before); + }); + + it('rejects every response and stream getter with the awaiting error on a paused resume', async () => { + const paused = await pauseOnGatedCalls([ + 'call_1', + ]); + const before = structuredClone(paused.get()); + const result = new ModelResult({ + request, + client: {} as OpenRouterCore, + tools: paused.tools, + state: paused.accessor, + }); + + const consumers: Record Promise> = { + getResponse: () => result.getResponse(), + getText: () => result.getText(), + getToolCalls: () => result.getToolCalls(), + getTextStream: () => result.getTextStream().next(), + getReasoningStream: () => result.getReasoningStream().next(), + getFullResponsesStream: () => result.getFullResponsesStream().next(), + getItemsStream: () => result.getItemsStream().next(), + getNewMessagesStream: () => result.getNewMessagesStream().next(), + getToolStream: () => result.getToolStream().next(), + getToolCallsStream: () => result.getToolCallsStream().next(), + }; + for (const [name, consume] of Object.entries(consumers)) { + await expect(consume(), name).rejects.toThrow(/awaiting approval.*call_1/); + } + + // Inspection APIs still work on the paused conversation. + expect(await result.requiresApproval()).toBe(true); + expect((await result.getPendingToolCalls()).map((tc) => tc.id)).toEqual([ + 'call_1', + ]); + expect((await result.getState()).status).toBe('awaiting_approval'); + + expect(mockBetaResponsesSend).not.toHaveBeenCalled(); + expect(paused.get()).toEqual(before); + }); + + // A paused status with nothing pending has no decision to wait for. Pausing + // there would strand the conversation; it continues with a normal request. + it.each([ + [ + 'awaiting_approval', + [], + ], + [ + 'awaiting_approval', + undefined, + ], + [ + 'awaiting_hitl', + [], + ], + ] as const)('continues a %s state whose pendingToolCalls is %j', async (status, pendingToolCalls) => { + const paused = await pauseOnGatedCalls([ + 'call_1', + ]); + const stranded = structuredClone(paused.get()); + if (!stranded) { + throw new Error('expected a paused state'); + } + stranded.status = status; + if (pendingToolCalls === undefined) { + delete stranded.pendingToolCalls; + } else { + stranded.pendingToolCalls = [ + ...pendingToolCalls, + ]; + } + const { accessor, get } = createMemoryAccessor(stranded); + mockBetaResponsesSend.mockResolvedValueOnce({ + ok: true, + value: doneResponse('resp_done'), + }); + + const text = await new ModelResult({ + request, + client: {} as OpenRouterCore, + tools: paused.tools, + state: accessor, + }).getText(); + + expect(text).toBe('Done.'); + expect(mockBetaResponsesSend).toHaveBeenCalledTimes(1); + expect(paused.execute).not.toHaveBeenCalled(); + expect(get()?.status).toBe('complete'); + }); + + it('caps the pending IDs listed in the awaiting error and omits the input note when no input was supplied', async () => { + const callIds = Array.from( + { + length: 25, + }, + (_, index) => `call_${index + 1}`, + ); + const paused = await pauseOnGatedCalls(callIds); + + const error = await new ModelResult({ + request: { + ...request, + input: [], + }, + client: {} as OpenRouterCore, + tools: paused.tools, + state: paused.accessor, + }) + .getResponse() + .then( + () => undefined, + (caught: unknown) => caught, + ); + + expect(error).toBeInstanceOf(Error); + const message = (error as Error).message; + expect(message).toContain('call_1, call_2,'); + expect(message).toContain('call_10, ... (15 more)'); + expect(message).not.toContain('call_11'); + expect(message).not.toContain('was not sent'); + }); + + // Before de-duplication, the second copy of a duplicated ID skipped the + // re-approval that a PreToolUse argument mutation requires: the first copy + // re-gated the call (marking it preToolUseApplied), the second executed it. + it('does not let a duplicated ID bypass re-approval after a PreToolUse mutation', async () => { + const execute = vi.fn(async (_input: { v: string }) => ({ + ok: true, + })); + const gated = tool({ + name: 'gated', + inputSchema: z.object({ + v: z.string(), + }), + outputSchema: z.object({ + ok: z.boolean(), + }), + requireApproval: () => true, + execute, + }); + const tools = [ + gated, + ] as const; + mockBetaResponsesSend.mockResolvedValueOnce({ + ok: true, + value: makeResponse('resp_pause', [ + makeFunctionCallItem('call_1', 'gated', '{"v":"reviewed"}'), + ]), + }); + const { accessor, get } = createMemoryAccessor(); + await new ModelResult({ + request, + client: {} as OpenRouterCore, + tools, + state: accessor, + }).getResponse(); + expect(get()?.status).toBe('awaiting_approval'); + mockBetaResponsesSend.mockClear(); + + const hooks = new HooksManager(); + hooks.on('PreToolUse', { + handler: () => ({ + mutatedInput: { + v: 'mutated', + }, + }), + }); + const result = new ModelResult({ + request, + client: {} as OpenRouterCore, + tools, + hooks, + state: accessor, + approveToolCalls: [ + 'call_1', + 'call_1', + ], + }); + + // The human approved {"v":"reviewed"}; the mutated arguments need their + // own approval, so nothing executes and the call is pending again. + await expect(result.getResponse()).rejects.toThrow(/awaiting approval.*call_1/); + expect(execute).not.toHaveBeenCalled(); + expect(mockBetaResponsesSend).not.toHaveBeenCalled(); + expect(get()?.status).toBe('awaiting_approval'); + expect(get()?.pendingToolCalls).toEqual([ + { + id: 'call_1', + name: 'gated', + arguments: { + v: 'mutated', + }, + preToolUseApplied: true, + }, + ]); + }); +});