fix(acp): seed provider handoff history - #8941
Conversation
Signed-off-by: Matt Toohey <contact@matttoohey.com>
![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: 4b8066f0c7
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| } | ||
|
|
||
| fn should_send_handoff_context(&self, messages: &[Message]) -> bool { | ||
| !self.handoff_context_sent.swap(true, Ordering::AcqRel) && has_handoff_context(messages) |
There was a problem hiding this comment.
should_send_handoff_context flips handoff_context_sent before stream() enqueues the ACP prompt, so if prompt() fails (for example when the ACP client channel is temporarily unavailable), the retry path will never include the handoff memo even though the provider never actually received it. This can drop the intended prior-session context on the first successful request after a transient failure. Move the state transition to after a successful prompt() send (or roll it back on error).
Useful? React with 👍 / 👎.
Signed-off-by: Matt Toohey <contact@matttoohey.com>
Signed-off-by: Matt Toohey <contact@matttoohey.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4cd2dfc3e0
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| return None; | ||
| } | ||
|
|
||
| let handoff_context = formatted_messages.join("\n"); |
There was a problem hiding this comment.
The new handoff path includes all prior agent-visible messages in a single memo, but there is no character/token cap before join("\n"). On long sessions (especially with large tool responses), the first ACP prompt can become too large and fail immediately with context/request-size errors, which blocks provider handoff for exactly the conversations this feature targets. Please cap or truncate the serialized history (e.g., by token/char budget and recency) before constructing the memo.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4cd2dfc3e0
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| if claim.first_prompt { | ||
| self.handoff_context_sent.store(false, Ordering::Release); | ||
| } |
There was a problem hiding this comment.
Roll back handoff_context_sent not only when prompt() enqueueing fails, but also when the first prompt later fails via AcpUpdate::Error. Right now the flag is set in claim_handoff_context, and only the immediate send error path resets it; if ACP accepts the request locally but returns an error (e.g., transport hiccup or prompt rejection) before producing a response, retries will skip the handoff memo even though no successful first prompt completed, dropping prior conversation context on the next attempt.
Useful? React with 👍 / 👎.
baxen
left a comment
baxen
left a comment
There was a problem hiding this comment.
This is a super clean approach, and i tested it out it works nicely
Signed-off-by: Matt Toohey <contact@matttoohey.com>
2 participants
There were two situations where the agent would not see the conversation history: