Exit agent loop when tool call JSON fails to parse#7840
Conversation
When a model's response is truncated (e.g. max_tokens exceeded), the tool call JSON can't be parsed. Previously we fed the parse error back as a user message and continued the loop, but truncation would just recur, causing an infinite loop burning tokens. Now we emit a message to the user and exit immediately, matching the pattern used by other terminal errors like context-length-exceeded. Fixes #7527
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ec71b8af40
ℹ️ 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".
| if is_token_cancelled(&cancel_token) || exit_chat { | ||
| break; |
There was a problem hiding this comment.
After exit_chat is set by a malformed tool call, the loop still awaits stream.next() in the while let condition and only then breaks at this guard, so that fetched chunk is discarded without processing. In OpenAI streaming, usage can arrive as a separate usage-only event (response_to_streaming_message, crates/goose/src/providers/formats/openai.rs, choices.is_empty() branch), which means this path can skip update_session_metrics and undercount token/cost metrics on parse-failure exits.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
that seems unlikely since we ran out of tokens and the provider just bailed
| exit_chat = true; | ||
| break; |
There was a problem hiding this comment.
Breaking here exits the per-request loop on the first malformed tool JSON, but tool handling for the whole request set has already run earlier in this iteration. If one tool call is malformed and a later one is valid, the later result is never yielded or added to messages_to_add, so users can miss outputs (or side effects) from tools that already executed and the persisted conversation becomes incomplete.
Useful? React with 👍 / 👎.
* origin/main: Remove include from Cargo.toml in goose-mcp (#7838) Exit agent loop when tool call JSON fails to parse (#7840) chore: remove redundant husky prepare script (#7829) Add github actions workflow for unused deps (#7681) fix: prevent SSE connection drops from silently truncating responses (#7831) doc: Added notes in contribution guide for pnpm (#7833) add prefer-offline to pnpm config to skip unnecessary registry lookups (#7828) fix: remove dead read handler from DeveloperClient (#7821) fix: prevent SageMaker TGI from being marked configured when only Bedrock keys are set (#7284) fix: disable some computercontroller functionality when no $DISPLAY detected (#7824) test(acp): align provider and server test parity (#7822) fix(acp): register MCP extensions when resuming a session (#7806) # Conflicts: # ui/desktop/src/hooks/useChatStream.ts
* main: (191 commits) fix: add tool_choice and parallel_tool_calls to chatgpt_codex provider (#7867) fix: tool confirmation handling for multiple requests (#7856) Remove dead OllamaSetup onboarding flow (#7861) fix: resolve tokio::sync::Mutex deadlock in recipe retry path (#7832) Upgrade Electron 40.6.0 → 41.0.0 (#7851) Only show up to 50 lines of source code (#7578) fix: stop writing without error when hitting broken pipe for goose session list (#7858) feat(acp): add session/set_mode handler (#7801) Keep messages in sync (#7850) More acp tools (#7843) fix: skip upgrade-insecure-requests CSP for external HTTP backends (#7714) fix(shell): prevent hang when command backgrounds a child process (#7689) Remove include from Cargo.toml in goose-mcp (#7838) Exit agent loop when tool call JSON fails to parse (#7840) chore: remove redundant husky prepare script (#7829) Add github actions workflow for unused deps (#7681) fix: prevent SSE connection drops from silently truncating responses (#7831) doc: Added notes in contribution guide for pnpm (#7833) add prefer-offline to pnpm config to skip unnecessary registry lookups (#7828) fix: remove dead read handler from DeveloperClient (#7821) ...
* main: (65 commits) feat(otel): propagate session.id to spans and log records (#7490) fix(test): add env_lock to is_openai_reasoning_model tests (#7917) fix(acp): pass session_id when loading extensions so skills are discovered (#7868) updated canonical models (#7920) feat(autovisualiser): Migrate the autovisualiser extension to MCP Apps (#7852) fix: add tool_choice and parallel_tool_calls to chatgpt_codex provider (#7867) fix: tool confirmation handling for multiple requests (#7856) Remove dead OllamaSetup onboarding flow (#7861) fix: resolve tokio::sync::Mutex deadlock in recipe retry path (#7832) Upgrade Electron 40.6.0 → 41.0.0 (#7851) Only show up to 50 lines of source code (#7578) fix: stop writing without error when hitting broken pipe for goose session list (#7858) feat(acp): add session/set_mode handler (#7801) Keep messages in sync (#7850) More acp tools (#7843) fix: skip upgrade-insecure-requests CSP for external HTTP backends (#7714) fix(shell): prevent hang when command backgrounds a child process (#7689) Remove include from Cargo.toml in goose-mcp (#7838) Exit agent loop when tool call JSON fails to parse (#7840) chore: remove redundant husky prepare script (#7829) ...
2 participants
When a model's response is truncated (e.g.
max_tokensexceeded), the tool call JSON can't be parsed. Previously we fed the parse error back as a user message and continued the loop, but truncation would just recur, causing an infinite loop burning tokens.Now we emit a message to the user and exit immediately, matching the pattern used by other terminal errors like context-length-exceeded.
Fixes #7527