fix overlong function names in provider requests - #10659
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9277cea24c
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| } | ||
| } | ||
|
|
||
| const MAX_FUNCTION_NAME_LENGTH: usize = 128; |
There was a problem hiding this comment.
Cap function names at OpenAI's 64-char limit
The Responses create schema still marks function/tool names as maxLength64, so a replayed tool call whose sanitized name is 65–128 characters will still be serialized over the provider limit and keep failing the session with a 400. The new regression test also locks in the wrong boundary by expecting length 128; please cap to the actual endpoint limit instead.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
🤖 Thanks for flagging this. I’m keeping 128 here because this PR targets Responses input[].name function-call replay, and the captured production 400 for that exact field says: Expected a string with maximum length 128, but got ... 160. The current OpenAI schema also defines FunctionToolParam.name with maxLength: 128; the 64-character limit on the linked page applies to other name fields such as structured response-format names, not this replayed function-call field. Since the shared helper already encodes the [a-zA-Z0-9_-] function-name contract, aligning it to the observed and schema-defined 128 limit is intentional.
There was a problem hiding this comment.
🤖 Follow-up from the broader code review: I removed the 128-character limit from the shared inbound is_valid_function_name validator. That helper is also used by Google, Databricks, Ollama, and local inference, so applying an OpenAI-specific limit there could reject names those providers had advertised. The 128-character cap now remains only in outbound sanitization, and a regression assertion protects 129-character regex-valid inbound names. Full provider tests and Clippy pass on 45ec30c46.
* origin/main: (24 commits) upgrade to rmcp 2.0 (#10584) chore: opus5 support (including adaptive thinking) (#10686) fix(desktop): use shell working directory on initial launch (#10655) fix(providers): enable inventory refresh for OpenRouter model picker (#10641) Add configurable GOOSE_DOCS_ROOT for air-gapped docs access (#10294) fix(i18n): fix Korean translation errors, untranslated entries, and terminology consistency (#10667) fix(acp): preserve read lines and reload tool output (#10662) fix(acp): surface tool call output in CLI and Desktop (#10654) fix overlong function names in provider requests (#10659) fix (desktop): preserve ACP tool call update fields (#10653) feat: support latest Gemini models (#10630) Merge commit from fork feat (acp): Gate tool-call label enrichment on ACP client capability (#10644) fix(session): use configured session manager for tool summaries (#10628) refactor (acp): simplify ACP tool-call handling and fix chain summaries (#10599) fix: omit disabled OpenRouter reasoning (#10517) enhance the uniffi API layer (#10427) remove sampling from smoke test (#10639) rebuild canonical model db (#10633) fix(ui): clear stale pending ACP connection after terminal recovery failure (#10552) ... # Conflicts: # ui/desktop/src/i18n/messages/ko.json
Category: fix
User Impact: Sessions no longer become permanently stuck when OpenAI replays an overlong function name from conversation history.
Problem: ACP harness activity can leave human-readable tool titles in conversation history. When one of those persisted names exceeds OpenAI's 128-character limit, every Responses API request that replays the history fails with a 400, so retrying or switching back to an OpenAI-backed provider cannot recover the session.
Solution: Enforce OpenAI's 128-character limit in the outbound function-name sanitizer used when serializing conversation history. This repairs affected history at request time while preserving the provider-neutral inbound validator used by other providers.
Scope: This PR fixes the confirmed permanent OpenAI 400 loop. It does not address whether prior conversation context is successfully handed to a newly selected ACP harness; that is a separate provider-switch path that still needs an end-to-end reproduction before changing it.
File changes
crates/goose-provider-types/src/formats/openai.rs
Caps sanitized outbound function names at 128 characters while keeping inbound character validation provider-neutral. Adds boundary coverage for both behaviors.
crates/goose-provider-types/src/formats/openai_responses.rs
Adds a request-level regression test proving a persisted 160-character tool-call name is serialized within OpenAI's limit.
Tests:
cargo fmt --all -- --checkcargo test -p goose-provider-typescargo clippy -p goose-provider-types --tests -- -D warnings