fix(agent): validate approval decisions and keep decision-less resumes paused

.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
+```

packages/agent/src/lib/async-params.ts

@@ -248,9 +248,27 @@ type BaseCallModelInput<
 type ApprovalParamsWithState<TTools extends readonly Tool[] = readonly Tool[]> = {
   /** State accessor for multi-turn persistence and approval gates */
   state: StateAccessor<TTools>;
-  /** 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[];
 };
 

packages/agent/src/lib/model-result.ts

@@ -264,6 +264,13 @@ function extractServerToolIdentity(item: ServerToolResultItem): Record<string, u
 // buggy handler fails fast with a visible warning.
 const MAX_FORCE_RESUME_OVERRIDES = 3;
 
+/**
+ * Most pending call IDs listed in a "conversation is paused" error. A state
+ * can hold any number of pending calls; the message stays readable and the
+ * full list is available from `getPendingToolCalls()`.
+ */
+const MAX_PENDING_IDS_IN_ERROR = 10;
+
 /**
  * Sentinel marking a tool-call id that appeared MORE THAN ONCE in one batch.
  * Ids are model-emitted and nothing upstream enforces uniqueness; a colliding
@@ -639,6 +646,7 @@ export class ModelResult<
   private approvedToolCalls: string[] = [];
   private rejectedToolCalls: string[] = [];
   private isResumingFromApproval = false;
+  private initialRequestPromise: Promise<ResolvedCallModelInput> | null = null;
 
   // Unified turn broadcaster for multi-turn streaming
   private turnBroadcaster: ToolEventBroadcaster<CorrelatedResponseStreamEvent<TTools>> | 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<ResolvedCallModelInput> {
+    this.initialRequestPromise ??= this.resolveInitialRequestUncached(initialContext);
+    return this.initialRequestPromise;
+  }
+
+  private async resolveInitialRequestUncached(
+    initialContext: TurnContext,
+  ): Promise<ResolvedCallModelInput> {
+    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<TTools>,
+  ): Promise<boolean> {
+    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();