diff --git a/.changeset/fix-manual-tool-pending-state.md b/.changeset/fix-manual-tool-pending-state.md new file mode 100644 index 0000000..f5f19d7 --- /dev/null +++ b/.changeset/fix-manual-tool-pending-state.md @@ -0,0 +1,11 @@ +--- +"@openrouter/agent": minor +--- + +Persist unresolved manual tool calls (`execute: false` / no execute fn) to `ConversationState.pendingToolCalls` when the loop stops, and set status to the new value `'awaiting_client_tools'`. + +Previously, HITL pauses (`onToolCalled → null`) correctly populated `pendingToolCalls` with status `'awaiting_hitl'`, but bare manual tools only `break`'d the loop — `getPendingToolCalls()` returned `[]` and status was left `in_progress`/`complete`. Cold-start consumers could not recover the unresolved calls from serialized state. + +- New `ConversationStatus` value: `'awaiting_client_tools'` (additive; does not replace `'awaiting_hitl'`). +- Mixed auto+manual rounds still execute/persist regular tool outputs, then pause with only the unresolved manual calls in `pendingToolCalls`. +- A successful resume with new input from `'awaiting_client_tools'` clears the stale pendings and continues as a normal turn. Failed resume requests leave the paused state intact. Manual tools are not approved/rejected via call IDs (unlike HITL/`awaiting_approval`). diff --git a/packages/agent/src/lib/model-result.ts b/packages/agent/src/lib/model-result.ts index 5e64728..a572600 100644 --- a/packages/agent/src/lib/model-result.ts +++ b/packages/agent/src/lib/model-result.ts @@ -223,6 +223,7 @@ export class ModelResult< private resolvedRequest: models.ResponsesRequest | null = null; // Fresh user items to persist atomically with the assistant response private pendingFreshItems: models.BaseInputsUnion[] | undefined; + private resumingFromClientTools = false; // State management for multi-turn conversations private stateAccessor: StateAccessor | null = null; @@ -518,10 +519,23 @@ export class ModelResult< this.pendingFreshItems = undefined; } - await this.saveStateSafely({ - messages: appendToMessages(messages, outputItems as models.BaseInputsUnion[]), - previousResponseId: response.id, - }); + const stateUpdates: Partial, 'id' | 'createdAt' | 'updatedAt'>> = + { + messages: appendToMessages(messages, outputItems as models.BaseInputsUnion[]), + previousResponseId: response.id, + }; + if (this.resumingFromClientTools) { + this.currentState = { + ...this.currentState, + }; + this.clearOptionalStateProperties([ + 'pendingToolCalls', + ]); + this.resumingFromClientTools = false; + stateUpdates.status = 'in_progress'; + } + + await this.saveStateSafely(stateUpdates); } /** @@ -836,6 +850,43 @@ export class ModelResult< await this.saveStateSafely(stateUpdates); } + /** + * Persist state when the loop stops due to unresolved manual (client-executed) + * tool calls — tools with neither `execute` nor `onToolCalled`. + * + * Mirrors `persistHitlPause` so callers can read the unresolved calls via + * `getPendingToolCalls()` / `getState()` after the loop ends. Uses the + * distinct status `awaiting_client_tools` so consumers can discriminate + * manual pauses from HITL pauses (`awaiting_hitl`). + * + * Resume behavior: `awaiting_client_tools` is intentionally NOT treated as a + * resumable status for `processApprovalDecisions` — manual tools are not + * approved/rejected via call IDs; the caller executes them externally and + * typically supplies `function_call_output` items as new input on the next + * `callModel`. The paused status and calls remain durable until that request + * produces a response; they are then cleared atomically with the response. + * + * @param currentResponse - The response that produced the unresolved calls + * @param unresolvedCalls - Manual (or otherwise non-auto-resolvable) tool calls + */ + private async persistClientToolsPause( + currentResponse: models.OpenResponsesResult, + unresolvedCalls: ParsedToolCall[], + ): Promise { + this.finalResponse = currentResponse; + + if (!this.stateAccessor || unresolvedCalls.length === 0) { + return; + } + + const stateUpdates: Partial, 'id' | 'createdAt' | 'updatedAt'>> = + { + pendingToolCalls: unresolvedCalls as ParsedToolCall[], + status: 'awaiting_client_tools', + }; + await this.saveStateSafely(stateUpdates); + } + /** * Compute the `output` payload sent to the model for a successfully * settled tool execution. Routes through `toModelOutput` when the tool @@ -1503,14 +1554,20 @@ export class ModelResult< ]); await this.saveStateSafely(); } + + // Keep manual calls durable until the resumed request produces a + // response. Clearing before the API call would lose the only copy if + // that request failed. + this.resumingFromClientTools = loadedState.status === 'awaiting_client_tools'; } else { this.currentState = createInitialState(); } - // Update status to in_progress - await this.saveStateSafely({ - status: 'in_progress', - }); + if (!this.resumingFromClientTools) { + await this.saveStateSafely({ + status: 'in_progress', + }); + } } // Resolve async functions before initial request @@ -1907,9 +1964,11 @@ export class ModelResult< return; // Paused for approval } + // All tool calls are manual (no execute / no onToolCalled) or otherwise + // non-auto-resolvable — stop and surface them as pending client tools + // instead of marking the conversation complete with empty pendings. if (!this.hasExecutableToolCalls(toolCalls)) { - this.finalResponse = currentResponse; - await this.markStateComplete(); + await this.persistClientToolsPause(currentResponse, toolCalls); return; } @@ -1939,8 +1998,12 @@ export class ModelResult< return; } + // All-manual (or otherwise non-auto-resolvable) mid-loop round: stop + // and persist the unresolved calls the same way the first-round guard + // does above, so getPendingToolCalls() surfaces them after loop end. if (!this.hasExecutableToolCalls(currentToolCalls)) { - break; + await this.persistClientToolsPause(currentResponse, currentToolCalls); + return; } // Build turn context @@ -2016,9 +2079,13 @@ export class ModelResult< // `hasExecutableToolCalls` guards. Also covers calls to tool names // not present in `options.tools` at all. const resolvedCallIds = new Set(toolResults.map((r) => r.callId)); - const hasUnresolvedToolCalls = currentToolCalls.some((tc) => !resolvedCallIds.has(tc.id)); - if (hasUnresolvedToolCalls) { - break; + const unresolvedToolCalls = currentToolCalls.filter((tc) => !resolvedCallIds.has(tc.id)); + if (unresolvedToolCalls.length > 0) { + // Mixed auto + manual (or unknown-name) round — regular tool outputs + // were already persisted via saveToolResultsToState above; surface + // the unresolved calls on pendingToolCalls and stop. + await this.persistClientToolsPause(currentResponse, unresolvedToolCalls); + return; } // Apply nextTurnParams @@ -2731,17 +2798,22 @@ export class ModelResult< // ========================================================================= /** - * Check if the conversation requires human input to continue. + * Check if the conversation requires human/client input to continue. * Returns true when the conversation is paused waiting for caller-supplied - * decisions — either approval/rejection (`awaiting_approval`) or HITL tool - * resume (`awaiting_hitl`). Also returns true whenever `pendingToolCalls` - * is populated regardless of status. + * decisions — approval/rejection (`awaiting_approval`), HITL tool resume + * (`awaiting_hitl`), or client-executed manual tools (`awaiting_client_tools`). + * Also returns true whenever `pendingToolCalls` is populated regardless of + * status. */ async requiresApproval(): Promise { await this.initStream(); const status = this.currentState?.status; - if (status === 'awaiting_approval' || status === 'awaiting_hitl') { + if ( + status === 'awaiting_approval' || + status === 'awaiting_hitl' || + status === 'awaiting_client_tools' + ) { return true; } diff --git a/packages/agent/src/lib/tool-types.ts b/packages/agent/src/lib/tool-types.ts index 8e89016..5693664 100644 --- a/packages/agent/src/lib/tool-types.ts +++ b/packages/agent/src/lib/tool-types.ts @@ -1020,12 +1020,17 @@ export interface PartialResponse vi.fn()); + +vi.mock('@openrouter/sdk/funcs/betaResponsesSend', () => ({ + betaResponsesSend: mockBetaResponsesSend, +})); + +function functionCallItem( + callId: string, + name: string, + args: string, +): models.OutputFunctionCallItem { + return { + type: 'function_call', + id: `fc_${callId}`, + callId, + name, + arguments: args, + status: 'completed', + }; +} + +function makeResponse( + id: string, + output: models.OpenResponsesResult['output'], +): models.OpenResponsesResult { + return { + id, + object: 'response', + createdAt: 0, + model: 'test-model', + status: 'completed', + completedAt: 0, + output, + error: null, + incompleteDetails: null, + temperature: null, + topP: null, + presencePenalty: null, + frequencyPenalty: null, + metadata: null, + instructions: null, + tools: [], + toolChoice: 'auto', + parallelToolCalls: false, + } as models.OpenResponsesResult; +} + +const autoTool = { + type: ToolType.Function, + function: { + name: 'auto_search', + description: 'Auto-executed tool.', + inputSchema: z.object({ + query: z.string(), + }), + outputSchema: z.object({ + result: z.string(), + }), + execute: async (_params: { query: string }) => ({ + result: 'found it', + }), + }, +} as const; + +// No `execute` — the client is responsible for running this tool. +const manualTool = { + type: ToolType.Function, + function: { + name: 'exec_command', + description: 'Manual client-executed tool.', + inputSchema: z.object({ + command: z.string(), + }), + outputSchema: z.object({ + stdout: z.string(), + }), + }, +} as const; + +const client = {} as OpenRouterCore; + +function createMemoryAccessor(): { + accessor: StateAccessor; + get: () => ConversationState | null; +} { + let stored: ConversationState | null = null; + const accessor: StateAccessor = { + load: async () => stored, + save: async (state) => { + stored = state; + }, + }; + return { + accessor, + get: () => stored, + }; +} + +describe('manual tool pending state (PR-4)', () => { + beforeEach(() => { + mockBetaResponsesSend.mockReset(); + }); + + it('all-manual round: stops loop, pendingToolCalls + status awaiting_client_tools', async () => { + mockBetaResponsesSend.mockResolvedValueOnce({ + ok: true, + value: makeResponse('resp_manual', [ + functionCallItem('call_manual_1', 'exec_command', '{"command":"ls"}'), + ]), + }); + + const { accessor, get } = createMemoryAccessor(); + + const result = callModel(client, { + model: 'test-model', + input: 'run ls', + tools: [ + manualTool, + ] as const, + state: accessor, + }); + + const pending = await result.getPendingToolCalls(); + expect(pending).toHaveLength(1); + expect(pending[0]?.id).toBe('call_manual_1'); + expect(pending[0]?.name).toBe('exec_command'); + + const state = await result.getState(); + expect(state.status).toBe('awaiting_client_tools'); + expect(state.pendingToolCalls).toHaveLength(1); + expect(state.pendingToolCalls?.[0]?.id).toBe('call_manual_1'); + + // No follow-up request — loop stopped after the unresolved manual call. + expect(mockBetaResponsesSend).toHaveBeenCalledTimes(1); + + const saved = get(); + expect(saved?.status).toBe('awaiting_client_tools'); + expect(saved?.pendingToolCalls?.[0]?.id).toBe('call_manual_1'); + }); + + it('mixed regular+manual: auto output in messages, manual call in pendingToolCalls', async () => { + mockBetaResponsesSend.mockResolvedValueOnce({ + ok: true, + value: makeResponse('resp_mixed', [ + functionCallItem('call_auto_1', 'auto_search', '{"query":"docs"}'), + functionCallItem('call_manual_1', 'exec_command', '{"command":"ls"}'), + ]), + }); + + const { accessor, get } = createMemoryAccessor(); + + const result = callModel(client, { + model: 'test-model', + input: 'do both', + tools: [ + autoTool, + manualTool, + ] as const, + state: accessor, + }); + + const pending = await result.getPendingToolCalls(); + expect(pending).toHaveLength(1); + expect(pending[0]?.id).toBe('call_manual_1'); + expect(pending[0]?.name).toBe('exec_command'); + + // No follow-up with orphan function_call (same guard as mixed-manual-tool-round). + expect(mockBetaResponsesSend).toHaveBeenCalledTimes(1); + + const state = await result.getState(); + expect(state.status).toBe('awaiting_client_tools'); + + // Auto tool's output was persisted to state.messages; manual call was not + // given an output item (client must supply it later). + const messages = state.messages as Array<{ + type?: string; + callId?: string; + output?: string; + }>; + const autoOutput = messages.find( + (m) => m.type === 'function_call_output' && m.callId === 'call_auto_1', + ); + expect(autoOutput).toBeDefined(); + expect(autoOutput?.output).toContain('found it'); + + const manualOutput = messages.find( + (m) => m.type === 'function_call_output' && m.callId === 'call_manual_1', + ); + expect(manualOutput).toBeUndefined(); + + const saved = get(); + expect(saved?.status).toBe('awaiting_client_tools'); + expect(saved?.pendingToolCalls?.[0]?.id).toBe('call_manual_1'); + }); + + it('JSON round-trip of paused state preserves pendingToolCalls and status', async () => { + mockBetaResponsesSend.mockResolvedValueOnce({ + ok: true, + value: makeResponse('resp_manual', [ + functionCallItem('call_manual_1', 'exec_command', '{"command":"pwd"}'), + ]), + }); + + const { accessor } = createMemoryAccessor(); + + const result = callModel(client, { + model: 'test-model', + input: 'run pwd', + tools: [ + manualTool, + ] as const, + state: accessor, + }); + + const state = await result.getState(); + expect(state.status).toBe('awaiting_client_tools'); + expect(state.pendingToolCalls).toHaveLength(1); + + const roundTripped = JSON.parse(JSON.stringify(state)) as ConversationState; + expect(roundTripped.status).toBe('awaiting_client_tools'); + expect(roundTripped.pendingToolCalls).toHaveLength(1); + expect(roundTripped.pendingToolCalls?.[0]?.id).toBe('call_manual_1'); + expect(roundTripped.pendingToolCalls?.[0]?.name).toBe('exec_command'); + expect(roundTripped.pendingToolCalls?.[0]?.arguments).toEqual({ + command: 'pwd', + }); + }); + + it('clears pending manual calls only after a resume succeeds', async () => { + const { accessor, get } = createMemoryAccessor(); + mockBetaResponsesSend + .mockResolvedValueOnce({ + ok: true, + value: makeResponse('resp_manual', [ + functionCallItem('call_manual_1', 'exec_command', '{"command":"pwd"}'), + ]), + }) + .mockResolvedValueOnce({ + ok: true, + value: makeResponse('resp_done', [ + { + id: 'msg_done', + type: 'message', + role: 'assistant', + status: 'completed', + content: [ + { + type: 'output_text', + text: 'done', + annotations: [], + }, + ], + }, + ]), + }); + + await callModel(client, { + model: 'test-model', + input: 'run pwd', + tools: [ + manualTool, + ] as const, + state: accessor, + }).getPendingToolCalls(); + + await callModel(client, { + model: 'test-model', + input: [ + { + type: 'function_call_output', + callId: 'call_manual_1', + output: '{"stdout":"/tmp"}', + }, + ], + tools: [ + manualTool, + ] as const, + state: accessor, + }).getResponse(); + + expect(get()?.status).toBe('complete'); + expect(get()?.pendingToolCalls).toBeUndefined(); + }); + + it('keeps pending manual calls when a resume request fails', async () => { + const { accessor, get } = createMemoryAccessor(); + mockBetaResponsesSend.mockResolvedValueOnce({ + ok: true, + value: makeResponse('resp_manual', [ + functionCallItem('call_manual_1', 'exec_command', '{"command":"pwd"}'), + ]), + }); + + await callModel(client, { + model: 'test-model', + input: 'run pwd', + tools: [ + manualTool, + ] as const, + state: accessor, + }).getPendingToolCalls(); + + mockBetaResponsesSend.mockResolvedValueOnce({ + ok: false, + error: new Error('temporary failure'), + }); + + await expect( + callModel(client, { + model: 'test-model', + input: [ + { + type: 'function_call_output', + callId: 'call_manual_1', + output: '{"stdout":"/tmp"}', + }, + ], + tools: [ + manualTool, + ] as const, + state: accessor, + }).getResponse(), + ).rejects.toThrow('temporary failure'); + + expect(get()?.status).toBe('awaiting_client_tools'); + expect(get()?.pendingToolCalls?.[0]?.id).toBe('call_manual_1'); + }); + + it('does not mutate a loaded paused-state object before resume succeeds', async () => { + const pausedState: ConversationState = { + id: 'conv_manual', + messages: [ + functionCallItem('call_manual_1', 'exec_command', '{"command":"pwd"}'), + ], + pendingToolCalls: [ + { + id: 'call_manual_1', + name: 'exec_command', + arguments: { + command: 'pwd', + }, + }, + ], + status: 'awaiting_client_tools', + createdAt: 0, + updatedAt: 0, + }; + const accessor: StateAccessor = { + load: async () => pausedState, + save: async () => undefined, + }; + mockBetaResponsesSend.mockResolvedValueOnce({ + ok: true, + value: makeResponse('resp_done', []), + }); + + await callModel(client, { + model: 'test-model', + input: [ + { + type: 'function_call_output', + callId: 'call_manual_1', + output: '{"stdout":"/tmp"}', + }, + ], + tools: [ + manualTool, + ] as const, + state: accessor, + }).getResponse(); + + expect(pausedState.status).toBe('awaiting_client_tools'); + expect(pausedState.pendingToolCalls?.[0]?.id).toBe('call_manual_1'); + }); + + it('HITL pause still yields awaiting_hitl (no regression)', async () => { + mockBetaResponsesSend.mockResolvedValueOnce({ + ok: true, + value: makeResponse('resp_hitl', [ + functionCallItem('call_hitl_1', 'approve', '{"amount":5}'), + ]), + }); + + const approve = tool({ + name: 'approve', + inputSchema: z.object({ + amount: z.number(), + }), + outputSchema: z.object({ + ok: z.boolean(), + }), + onToolCalled: async () => null, + }); + + const { accessor, get } = createMemoryAccessor(); + + const result = callModel(client, { + model: 'test-model', + input: 'approve 5', + tools: [ + approve, + ] as const, + state: accessor, + }); + + await result.getResponse(); + + const pending = await result.getPendingToolCalls(); + expect(pending).toHaveLength(1); + expect(pending[0]?.id).toBe('call_hitl_1'); + + const state = await result.getState(); + expect(state.status).toBe('awaiting_hitl'); + expect(state.pendingToolCalls?.[0]?.id).toBe('call_hitl_1'); + + const saved = get(); + expect(saved?.status).toBe('awaiting_hitl'); + // Distinct from awaiting_client_tools — HITL keeps its existing status. + expect(saved?.status).not.toBe('awaiting_client_tools'); + }); +});