From 1601bff00f9091ef55daa3718b3ae2c3b3d386d8 Mon Sep 17 00:00:00 2001 From: Luke Parke <5702154+LukasParke@users.noreply.github.com> Date: Wed, 21 Oct 2026 17:57:14 +0000 Subject: [PATCH 1/2] fix(agent): validate approval decisions and keep decision-less resumes paused De-duplicate approveToolCalls/rejectToolCalls in the ModelResult constructor so a repeated call ID cannot execute an approval-gated tool twice, and throw if one ID appears in both lists (typescript-sdk#518). Resuming an awaiting_approval/awaiting_hitl conversation with no decisions now stays paused instead of flipping to in_progress and sending a new model request (typescript-sdk#517). getResponse()/getText() reject with an explicit "awaiting approval" error listing the pending call IDs instead of the generic "Response not available". Co-Authored-By: Claude Opus 5.5 (1M context) --- .changeset/approval-decision-validation.md | 12 + packages/agent/src/lib/model-result.ts | 54 +++- .../unit/approval-gate-regressions.test.ts | 292 ++++++++++++++++++ 3 files changed, 354 insertions(+), 4 deletions(-) create mode 100644 .changeset/approval-decision-validation.md diff --git a/.changeset/approval-decision-validation.md b/.changeset/approval-decision-validation.md new file mode 100644 index 00000000..52ae0c5f --- /dev/null +++ b/.changeset/approval-decision-validation.md @@ -0,0 +1,12 @@ +--- +'@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, and passing the same ID in both lists throws before anything runs. Resuming an `awaiting_approval` or `awaiting_hitl` conversation without any decisions now leaves it paused instead of sending a new model request; `getResponse()` / `getText()` reject with an explicit "awaiting approval" error that lists the pending call IDs (previously the generic "Response not available"). + +```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/model-result.ts b/packages/agent/src/lib/model-result.ts index bbc4f047..45c389bf 100644 --- a/packages/agent/src/lib/model-result.ts +++ b/packages/agent/src/lib/model-result.ts @@ -804,8 +804,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 @@ -5736,6 +5751,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 an approve/reject decision. getResponse()/getText() + // surface this as an explicit "awaiting approval" error. + if (isResumableStatus) { + this.isResumingFromApproval = true; + return; + } + // Check for interruption flag and handle if (loadedState.interruptedBy) { // Clear interruption flag and continue from saved state @@ -6763,12 +6788,33 @@ export class ModelResult< await this.executeToolsIfNeeded(); if (!this.finalResponse) { - throw new Error('Response not available'); + throw this.responseUnavailableError(); } return extractTextFromResponse(this.finalResponse); } + /** + * Error for getResponse()/getText() when the run produced no final + * response. A resume that leaves the conversation paused (no decisions + * supplied, or decisions for only some of the pending calls) has no + * response to return; say so explicitly instead of the generic message. + */ + private responseUnavailableError(): Error { + const status = this.currentState?.status; + if (status === 'awaiting_approval' || status === 'awaiting_hitl') { + const pendingIds = (this.currentState?.pendingToolCalls ?? []).map((tc) => tc.id); + const what = status === 'awaiting_approval' ? 'awaiting approval' : 'awaiting HITL input'; + return new Error( + `Conversation is ${what} (status '${status}'); no response is available. ` + + `Pending tool call IDs: ${pendingIds.length > 0 ? pendingIds.join(', ') : '(none)'}. ` + + 'Resume with approveToolCalls and/or rejectToolCalls for the pending calls. ' + + 'Use requiresApproval() or getPendingToolCalls() to inspect a paused conversation.', + ); + } + return new Error('Response not available'); + } + /** * Get just the text content from the response. * This will consume the stream until completion, execute any tools, and extract the text. @@ -6795,7 +6841,7 @@ export class ModelResult< await this.executeToolsIfNeeded(); if (!this.finalResponse) { - throw new Error('Response not available'); + throw this.responseUnavailableError(); } return this.finalResponse; diff --git a/packages/agent/tests/unit/approval-gate-regressions.test.ts b/packages/agent/tests/unit/approval-gate-regressions.test.ts index 76ae9ca7..cbcdb168 100644 --- a/packages/agent/tests/unit/approval-gate-regressions.test.ts +++ b/packages/agent/tests/unit/approval-gate-regressions.test.ts @@ -1993,3 +1993,295 @@ 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; + }> { + const sent = mockBetaResponsesSend.mock.calls[callIndex]?.[1]?.responsesRequest; + const input = (sent?.input ?? []) as Array<{ + type?: string; + callId?: string; + }>; + return input.filter((item) => item?.type === 'function_call_output'); + } + + /** 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'); + }); +}); From 1008d7e617c35781c32489d070dd750f08c2f0ff Mon Sep 17 00:00:00 2001 From: Matt Apperson Date: Wed, 11 Nov 2026 12:40:33 -0500 Subject: [PATCH 2/2] fix(agent): keep HITL output resumes working and report pauses from every getter Review follow-ups for the decision-less pause: - awaiting_hitl resumes again when input carries a function_call_output for a pending call (the documented HITL resume). It stays paused only when no such output and no decision is supplied. awaiting_approval still pauses unconditionally. - A paused status with no pending tool calls continues with a normal request instead of stranding the conversation. - getToolCalls() and the stream getters reject with the same awaiting error as getResponse()/getText(). The message caps the listed pending IDs at 10 and says when supplied input was dropped. - Document de-duplication and the conflict throw on approveToolCalls / rejectToolCalls, and expand the changeset. --- .changeset/approval-decision-validation.md | 10 +- packages/agent/src/lib/async-params.ts | 22 +- packages/agent/src/lib/model-result.ts | 140 +++++-- .../unit/approval-gate-regressions.test.ts | 388 ++++++++++++++++++ 4 files changed, 529 insertions(+), 31 deletions(-) diff --git a/.changeset/approval-decision-validation.md b/.changeset/approval-decision-validation.md index 52ae0c5f..03ef48a4 100644 --- a/.changeset/approval-decision-validation.md +++ b/.changeset/approval-decision-validation.md @@ -2,7 +2,15 @@ '@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, and passing the same ID in both lists throws before anything runs. Resuming an `awaiting_approval` or `awaiting_hitl` conversation without any decisions now leaves it paused instead of sending a new model request; `getResponse()` / `getText()` reject with an explicit "awaiting approval" error that lists the pending call IDs (previously the generic "Response not available"). +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 }); diff --git a/packages/agent/src/lib/async-params.ts b/packages/agent/src/lib/async-params.ts index f2f72e71..573e30a3 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 8ea16e3d..166fb96f 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 = @@ -2611,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'); @@ -5668,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 @@ -5750,9 +5813,9 @@ export class ModelResult< // 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 an approve/reject decision. getResponse()/getText() - // surface this as an explicit "awaiting approval" error. - if (isResumableStatus) { + // 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; } @@ -5795,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 @@ -6793,24 +6852,49 @@ export class ModelResult< } /** - * Error for getResponse()/getText() when the run produced no final - * response. A resume that leaves the conversation paused (no decisions - * supplied, or decisions for only some of the pending calls) has no - * response to return; say so explicitly instead of the generic message. + * 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(): Error { + 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') { - const pendingIds = (this.currentState?.pendingToolCalls ?? []).map((tc) => tc.id); - const what = status === 'awaiting_approval' ? 'awaiting approval' : 'awaiting HITL input'; - return new Error( - `Conversation is ${what} (status '${status}'); no response is available. ` + - `Pending tool call IDs: ${pendingIds.length > 0 ? pendingIds.join(', ') : '(none)'}. ` + - 'Resume with approveToolCalls and/or rejectToolCalls for the pending calls. ' + - 'Use requiresApproval() or getPendingToolCalls() to inspect a paused conversation.', - ); + if (status !== 'awaiting_approval' && status !== 'awaiting_hitl') { + return null; } - return new Error('Response not available'); + 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.' + : ''), + ); } /** @@ -7322,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 cbcdb168..46a2ab83 100644 --- a/packages/agent/tests/unit/approval-gate-regressions.test.ts +++ b/packages/agent/tests/unit/approval-gate-regressions.test.ts @@ -2034,15 +2034,76 @@ describe('approval decision validation and paused resumes (#517, #518)', () => { 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 }) => ({ @@ -2284,4 +2345,331 @@ describe('approval decision validation and paused resumes (#517, #518)', () => { 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, + }, + ]); + }); });