-
Notifications
You must be signed in to change notification settings - Fork 4.6k
Exit agent loop when tool call JSON fails to parse #7840
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 |
|---|---|---|
|
|
@@ -1181,9 +1181,10 @@ impl Agent { | |
| let mut messages_to_add = Conversation::default(); | ||
| let mut tools_updated = false; | ||
| let mut did_recovery_compact_this_iteration = false; | ||
| let mut exit_chat = false; | ||
|
|
||
| while let Some(next) = stream.next().await { | ||
| if is_token_cancelled(&cancel_token) { | ||
| if is_token_cancelled(&cancel_token) || exit_chat { | ||
| break; | ||
| } | ||
|
|
||
|
|
@@ -1462,15 +1463,17 @@ impl Agent { | |
| yield AgentEvent::Message(final_response.clone()); | ||
| messages_to_add.push(final_response); | ||
| } else { | ||
| let error_msg = format!( | ||
| "[system: Tool call could not be parsed: {}. The response may have been truncated. Try breaking the task into smaller steps.]", | ||
| error!( | ||
| "Tool call could not be parsed: {}", | ||
| request.tool_call.as_ref().unwrap_err(), | ||
| ); | ||
| let error_response = Message::user() | ||
| .with_generated_id() | ||
| .with_text(&error_msg); | ||
| yield AgentEvent::Message(error_response.clone()); | ||
| messages_to_add.push(error_response); | ||
| yield AgentEvent::Message( | ||
| Message::assistant().with_text( | ||
| "A tool call could not be parsed — the response may have been truncated. Try breaking the task into smaller steps or resending your message." | ||
| ) | ||
| ); | ||
| exit_chat = true; | ||
| break; | ||
|
Comment on lines
+1475
to
+1476
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.
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 Useful? React with 👍 / 👎. |
||
| } | ||
| } | ||
|
|
||
|
|
@@ -1590,7 +1593,6 @@ impl Agent { | |
| } | ||
| } | ||
|
|
||
| let mut exit_chat = false; | ||
| if no_tools_called { | ||
| if let Some(final_output_tool) = self.final_output_tool.lock().await.as_ref() { | ||
| if final_output_tool.final_output.is_none() { | ||
|
|
||
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.
After
exit_chatis set by a malformed tool call, the loop still awaitsstream.next()in thewhile letcondition 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 skipupdate_session_metricsand undercount token/cost metrics on parse-failure exits.Useful? React with 👍 / 👎.
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.
that seems unlikely since we ran out of tokens and the provider just bailed