fix(tools): preserve provider-compatible descriptions - #886
Conversation
|
Warning Review limit reachedNext included review available in 15 minutes. View limit detailsLimit details: You’ve used the included review currently available. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Review configuration: ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (5)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
|
Superseded by #887 after comparing both exact implementations and the current official OpenAI schemas. The API schemas define This PR's lossless system-message indirection was a careful compatibility attempt, but it changes message semantics and is unnecessary once the unsupported local cap is removed. Its review findings and tests informed the comparison; no check, review, or approval evidence transfers to #887. #887 must complete its own exact-head protected gates before merge. |
Pull request was closed
| "tool_choice requires tools on /v1/responses", | ||
| ) | ||
| if "tools" in body: | ||
| _preserve_long_tool_descriptions(body) |
There was a problem hiding this comment.
🟡 Empty Responses input passes fail-closed check
On /v1/responses, _preserve_long_tool_descriptions runs before the input non-empty check. When a tool has an overlong description and input is an empty/blank string or empty list, it wraps input into a non-empty list first. The invalid_input guard then passes and the request is forwarded instead of rejected.
Prompt for agents
In the /v1/responses handler in contextual_orchestrator/server.py, the call to _preserve_long_tool_descriptions(body) at the tools block (around line 6360) mutates body['input']: when input is an empty/blank string or empty list and a tool has an overlong description, it wraps input into a non-empty list. This happens before the input presence/non-empty validation (around lines 6399-6409, which rejects blank strings and empty arrays with invalid_input). As a result an invalid empty input is silently accepted and forwarded to the provider. Fix by validating input (the 'input is required' and 'input must be a non-empty string or non-empty array' checks) BEFORE invoking _preserve_long_tool_descriptions, or otherwise ensure preservation does not run against invalid/empty input. Chat Completions is unaffected because its message validation already runs before preservation.
Was this helpful? React with 👍 or 👎 to provide feedback.
| for tool in tools: | ||
| if not isinstance(tool, dict): | ||
| continue | ||
| function = tool.get("function") if isinstance(tool.get("function"), dict) else tool | ||
| description = function.get("description") | ||
| name = function.get("name") | ||
| if not isinstance(description, str) or len(description) <= 1024 or not isinstance(name, str): | ||
| continue | ||
| normalized_name = name.strip() | ||
| preserved.append({"name": normalized_name, "description": description}) | ||
| function["description"] = ( | ||
| f"Full documentation for {normalized_name} is preserved in the " | ||
| "contextual_orchestrator_tool_descriptions_v1 system message." | ||
| ) |
There was a problem hiding this comment.
📝 Info: Preservation mutates body before tool-name validation
_preserve_long_tool_descriptions rewrites the description and inserts the system message before _validate_chat_tools checks the tool name. When a tool has both an overlong description and an invalid name, the body is mutated and then the request is rejected by _validate_chat_tools. Harmless since the request fails, but preservation operates on unvalidated tool objects.
Was this helpful? React with 👍 or 👎 to provide feedback.
| generated_body = {"messages": [documentation_message]} | ||
| _validate_chat_message_known_fields(generated_body) | ||
| _validate_chat_tool_message_ids(generated_body) | ||
| _validate_chat_assistant_tool_calls(generated_body) | ||
| _validate_chat_message_audio_function_call(generated_body) |
There was a problem hiding this comment.
📝 Info: Synthesized preface is self-validated
The synthesized documentation message is run through the chat message validators via generated_body. Since the list holds the same dict later inserted, any mutation by those validators carries through. The message is a fixed system role with JSON string content, so it should pass; confirm none of these validators reject a plain system message.
Was this helpful? React with 👍 or 👎 to provide feedback.
1 participant
Summary
Evidence
32986962178reached OpenCode through the gateway, then returned400 invalid_toolsfor the overlong description48 passedacross tool-shape, null/omit, passthrough, and exact HTTP forwarding regressionsgit diff --checkGovernance
Normal protected merge only. No bypass, self-approval, or force push.