Repository navigation
fix: Read Codex custom_tool_call_output items as tool results #1771
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -59,8 +59,11 @@ type responsesRequest struct { | |
| // Messages, for the same reason the Anthropic one does: tool output | ||
| // must not reach the plugins that read Messages. The output value is | ||
| // read with flattenResponsesContent, which accepts the string form this | ||
| // API documents (and Codex sends) and a text-part array should one | ||
| // arrive. | ||
| // API documents and a text-part array should one arrive. | ||
| // - "custom_tool_call_output": the same thing for a custom (freeform) | ||
| // tool, with the same call_id and output fields. Codex declares its | ||
| // tools this way (one "exec" tool taking a script), so every tool result | ||
| // it sends arrives as this type, with the output as input_text parts. | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: The paragraph this list belongs to (line 41, outside the diff) still says "Three shapes are read, the first two confirmed on live Codex traffic". There are now four, and this one is also confirmed on live Codex traffic (your 0.167 capture). Worth updating that line so the header matches the list. |
||
| // | ||
| // Every other item type (reasoning echoes, function_call round-trips) is | ||
| // skipped — this parser surfaces conversation text, the tool manifest, and | ||
|
|
@@ -106,7 +109,7 @@ func (it *responsesInputItem) UnmarshalJSON(data []byte) error { | |
| }) | ||
| } | ||
| } | ||
| if raw.Type == "function_call_output" { | ||
| if raw.Type == "function_call_output" || raw.Type == "custom_tool_call_output" { | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. suggestion: The script SDK has the same match at |
||
| it.ToolResults = append(it.ToolResults, model.InferenceToolResult{ | ||
| ToolUseID: raw.CallID, | ||
| Content: flattenResponsesContent(raw.Output), | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
suggestion (follow-up, as noted in the PR body):
core/plugins/cpex/cmf_parts.go:186(theinferenceToCMFPartsdoc comment) andcore/plugins/cpex/README.md:226still name onlyfunction_call_output. The CPEX path now handlescustom_tool_call_outputautomatically throughinf.ToolResults, so those two are doc-only.core/plugins/script/sdk/cortex.py(_walk_body/_messages) is a real behaviour gap, though. A script plugin's redaction walk and tool-output detection will still skip Codex tool results. A follow-up issue or PR for that would be good, if #1770 doesn't already track it.