Skip to content

fix(intercept/responses): record all tool call types in responses interceptor#273

Open
dannykopping wants to merge 7 commits intomainfrom
danny/aigov-96-record-all-tool-types
Open

fix(intercept/responses): record all tool call types in responses interceptor#273
dannykopping wants to merge 7 commits intomainfrom
danny/aigov-96-record-all-tool-types

Conversation

@dannykopping
Copy link
Copy Markdown
Collaborator

Disclaimer: implemented by a Coder Agent using Claude Opus 4.6

Summary

Previously, the responses interceptor only recorded function_call and custom_tool_call types. This left tool calls like web_search_call, computer_call, shell_call, etc. unrecorded — meaning interceptions that did significant work via built-in tools showed no tool usage at all.

Now all tool call types are recorded for maximum auditor visibility:

Tool type Previously recorded Now recorded
function_call
custom_tool_call
web_search_call
computer_call
local_shell_call
shell_call
apply_patch_call
code_interpreter_call
mcp_call
file_search_call
image_generation_call

Approach

For the new tool types:

  • Tool name falls back to the type field when no explicit name is set (most built-in tools don't have a name, unlike function calls).
  • Call ID falls back to id when call_id is empty (web_search_call, file_search_call, code_interpreter_call, image_generation_call use id instead of call_id).
  • Non-tool output types (message, reasoning, *_output, etc.) are still skipped.

Changes

  • intercept/responses/base.go — expanded recordNonInjectedToolUsage switch to handle all tool call types.
  • intercept/responses/base_test.go — added web_search_call_with_no_name and all_additional_tool_types test cases.
  • fixtures/openai/responses/blocking/web_search.txtar — new fixture for a web search response.
  • fixtures/fixtures.go — embedded the new fixture.

Closes #121
Linear: AIGOV-96

…erceptor

Previously, the responses interceptor only recorded function_call and
custom_tool_call types. This left tool calls like web_search_call,
computer_call, shell_call, etc. unrecorded, meaning interceptions that
did significant work via built-in tools showed no tool usage.

Now all tool call types are recorded:
- web_search_call
- computer_call
- local_shell_call
- shell_call
- apply_patch_call
- code_interpreter_call
- mcp_call
- file_search_call
- image_generation_call

For these types, the tool name falls back to the type when no explicit
name is set, and the call ID falls back to the item ID when call_id is
empty (web_search_call, file_search_call, etc. use id instead of
call_id).

Adds a web_search blocking fixture and comprehensive tests covering all
new tool types.

Closes: #121
- Agentic tools (computer_call, local_shell_call, shell_call,
  apply_patch_call) use call_id for correlation.
- Hosted tools (web_search_call, file_search_call,
  code_interpreter_call, image_generation_call, mcp_call) use id
  only — referenced in OpenAI API docs.
- Use constant.ValueOf[...] for all types with SDK constants;
  define local computerCall const for the missing one.
- Remove the nolint:goconst directive.
The web_search_preview tool type is deprecated in favor of web_search.
Add blocking_web_search case to TestResponsesOutputMatchesUpstream
that exercises the hosted-tool recording path (id, not call_id).
Also fix the fixture's missing -- request -- txtar header.
@dannykopping dannykopping requested a review from pawbana April 17, 2026 14:26
@dannykopping dannykopping marked this pull request as ready for review April 17, 2026 14:26
if toolName == "" {
toolName = item.Type
}
callID = item.ID
Copy link
Copy Markdown
Contributor

@pawbana pawbana Apr 17, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't like callID being assigned here to item.ID. Those are different types.
In theory it would be possible to decipher value type (item.callID / item.ID) based on function call type but I think in this form it would cause too much confusion.
Separate field would be clearer, eg ToolCallItemID.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All we're doing is storing these values. At the end of the day they're used to track particular instances of tool calls. Whether it's a call ID or a regular ID is kinda irrelevant; I think expanding the API (and therefore the DB) to accommodate this extra detail is unnecessary and an implementation detail of OpenAI specifically.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, for just storing some ID for identification purpose it is fine to do it this way, but If there is some other data source that correctly records fields it becomes hard / unclear why sometimes tool call ID doesn't match.
Maybe I'm too paranoid but I think we should be careful about data consistency in recordings.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Turns out tool calls actually have two IDs: id and call_id 😆
Right now we're throwing id on the floor
So maybe it is actually just better to capture both separately and avoid the correctness issues you're raising

I'll expand the scope of the ticket and get to this next week
Thanks 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Consider recording of other tool types in responses interceptor

2 participants