Conversation
![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: 1d3980a0ea
ℹ️ 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".
| for msg in &messages_to_add { | ||
| session_manager.add_message(&session_config.id, msg).await?; | ||
| } | ||
| conversation.extend(messages_to_add); |
There was a problem hiding this comment.
When handle_retry_logic returns should_retry = true, it resets conversation back to initial_messages, but this block still persists and re-appends messages_to_add from the failed attempt. In retry-enabled runs where the model produced non-tool assistant output before failing success checks, those failed outputs are written to session history anyway and included in the next turn, so retries no longer start from a clean state and can drift or bloat context across attempts.
Useful? React with 👍 / 👎.
|
@jh-block @codefromthecrypt @rabi -- the previous fix broke how we concat messages and in the process of fixing that, I decided to add a test and then that test showed that the fix wasn't needed, so I wonder how we reproduce the original problem. FWIW, if I start goose ask it to list the files in the current folder and examine one by one whether they contain the word goose and then hit ctrl-c after the first toolcall, I can dump the session and see that it indeed got captured in the session. |
* main: (32 commits) Revert message flush & test (block#7966) docs: add Remote Access section with Telegram Gateway documentation (block#7955) fix: update webmcp blog post metadata image URL (block#7967) fix: clean up OAuth token cache on provider deletion (block#7908) fix: hard-coded tool call id in code mode callback (block#7939) Fix SSE parsers to accept optional space after data: prefix (block#7929) docs: add GOOSE_INPUT_LIMIT to config-files.md (block#7961) Add WebMCP for Beginners blog post (block#7957) Fix download manager (block#7933) Improve the formatting of tool calls, show thinking, treat Reasoning and Thinking as the same thing (sorry Kant) (block#7626) don't imply running builds all the time in AGENTS.md (block#7865) fix: unregister goosed child process's listener (block#7956) feat: adversarial agent for preventing leaking of info and more (block#7948) Update contributing.md (block#7927) docs: add credit balance monitoring section (block#7952) docs: add Cerebras provider to supported providers list (block#7953) docs: add TUI client documentation to ACP clients guide (block#7950) fix: removed double dash in pnpm command (block#7951) docs: polish ACP docs (block#7946) claude adaptive thinking (block#7944) ...
Co-authored-by: Douwe Osinga <[email protected]>
|
oh wow, we ran around in circles, huh. thanks for the explainer test! |
* main: (22 commits) feat: add gemini-acp provider, update docs on subscription models + improvements to codex (#8000) fix(openai): use Responses API for gpt-5.4 (#7982) Remove lead/worker provider (#7989) chore(release): release version 1.28.0 (#7991) Fix empty tool results from resource content (e.g. auto visualiser) (#7866) Separate SSE streaming from POST work submission (#7834) fix: include token usage in Databricks streaming responses (#7959) Optimize tool summarization (#7938) fix: overwrite the deprecated googledrive extension config (#7974) refactor: remove unnecessary Arc<Mutex> from tool execution pipeline (#7979) Revert message flush & test (#7966) docs: add Remote Access section with Telegram Gateway documentation (#7955) fix: update webmcp blog post metadata image URL (#7967) fix: clean up OAuth token cache on provider deletion (#7908) fix: hard-coded tool call id in code mode callback (#7939) Fix SSE parsers to accept optional space after data: prefix (#7929) docs: add GOOSE_INPUT_LIMIT to config-files.md (#7961) Add WebMCP for Beginners blog post (#7957) Fix download manager (#7933) Improve the formatting of tool calls, show thinking, treat Reasoning and Thinking as the same thing (sorry Kant) (#7626) ...
Co-authored-by: Douwe Osinga <[email protected]> Signed-off-by: esnyder <[email protected]>
Co-authored-by: Douwe Osinga <[email protected]> Signed-off-by: esnyder <[email protected]>
3 participants
Summary
#7850 tried to keep messages in sync, but it now seems that was not needed
Testing
Includes a test