fix(gateway): filter out tool-approval-response from prompt #11491
Conversation
…t invalid request error
There was a problem hiding this comment.
Pull request overview
This PR fixes issue #10663 by filtering out tool-approval-response messages from prompts before sending them to the Gateway API, preventing backend crashes when these internal message types are encountered.
Key Changes:
- Added filtering logic to remove
tool-approval-responsecontent from tool messages in prompts - Created test suite to verify the filtering behavior
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
packages/gateway/src/gateway-tool-approval.test.ts |
New test file that verifies tool-approval-response messages are filtered from prompts before API calls |
packages/gateway/src/gateway-language-model.ts |
Implements filterToolApprovalResponses method to remove tool-approval-response content from prompts; integrates filtering into the request pipeline |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| 'https://api.test.com/language-model': {}, | ||
| }); | ||
|
|
||
| it('should currently pass tool-approval-response through to the API', async () => { |
There was a problem hiding this comment.
The test description says "should currently pass tool-approval-response through to the API" but the test actually verifies that the tool-approval-response is filtered out (removed). The description should be updated to reflect the actual behavior being tested, such as "should filter out tool-approval-response messages from the prompt".
| it('should currently pass tool-approval-response through to the API', async () => { | |
| it('should filter out tool-approval-response messages from the prompt', async () => { |
| }); | ||
|
|
||
| const requestBody = await server.calls[0].requestBodyJson; | ||
| const toolMessage = requestBody.prompt[0]; |
There was a problem hiding this comment.
The variable 'toolMessage' is declared but never used. Since the test expects the prompt array to be empty after filtering, this variable assignment is unnecessary and should be removed.
| const toolMessage = requestBody.prompt[0]; |
| readonly modelId: GatewayModelId, | ||
| private readonly config: GatewayChatConfig, | ||
| ) {} | ||
| ) { } |
There was a problem hiding this comment.
There's an unnecessary space added between the closing parenthesis and the opening brace. This change should be reverted to maintain consistency with the existing code style.
| ) { } | |
| ) {} |
| it('should currently pass tool-approval-response through to the API', async () => { | ||
| server.urls['https://api.test.com/language-model'].response = { | ||
| type: 'json-value', | ||
| body: { | ||
| content: [{ type: 'text', text: 'Response' }], | ||
| finish_reason: 'stop', | ||
| usage: { prompt_tokens: 10, completion_tokens: 5 }, | ||
| }, | ||
| }; | ||
|
|
||
| const prompt: LanguageModelV3Prompt = [ | ||
| { | ||
| role: 'tool', | ||
| content: [ | ||
| { | ||
| type: 'tool-approval-response', | ||
| approvalId: 'approval-1', | ||
| approved: false, | ||
| reason: 'User denied', | ||
| }, | ||
| ], | ||
| }, | ||
| ]; | ||
|
|
||
| await createTestModel().doGenerate({ | ||
| prompt, | ||
| }); | ||
|
|
||
| const requestBody = await server.calls[0].requestBodyJson; | ||
| const toolMessage = requestBody.prompt[0]; | ||
|
|
||
| // This assertion confirms that the fix REMOVES the tool-approval-response | ||
| // which prevents the crash on the backend. | ||
| expect(requestBody.prompt).toHaveLength(0); // The tool message should be dropped completely as it becomes empty | ||
| }); |
There was a problem hiding this comment.
The test only covers the case where a tool message contains only tool-approval-response content. Consider adding test cases for:
- A tool message with both tool-approval-response and other content types (to verify partial filtering)
- Multiple tool messages with mixed content
- Prompts with non-tool messages mixed with tool messages
This would ensure the filtering logic works correctly in all scenarios.
- Updated test description to reflect actual behavior\n- Removed unused variable in test\n- Reverted unnecessary code style change\n- Added comprehensive test cases for various scenarios
1 participant
Fixes #10663. Filters out tool-approval-response messages before sending to Gateway API.