-
Notifications
You must be signed in to change notification settings - Fork 4.6k
fix: persist conversation state on interruption to prevent context loss #7785
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 |
|---|---|---|
|
|
@@ -336,6 +336,13 @@ pub async fn reply( | |
| tokio::select! { | ||
| _ = task_cancel.cancelled() => { | ||
| tracing::info!("Agent task cancelled"); | ||
| if let Err(e) = state | ||
| .session_manager() | ||
| .replace_conversation(&session_id, &all_messages) | ||
|
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.
This Useful? React with 👍 / 👎. |
||
| .await | ||
| { | ||
| tracing::warn!("Failed to persist conversation on cancellation: {}", e); | ||
| } | ||
| break; | ||
| } | ||
| _ = heartbeat_interval.tick() => { | ||
|
|
@@ -385,6 +392,13 @@ pub async fn reply( | |
| } | ||
| Err(_) => { | ||
| if tx.is_closed() { | ||
| if let Err(e) = state | ||
| .session_manager() | ||
| .replace_conversation(&session_id, &all_messages) | ||
| .await | ||
| { | ||
| tracing::warn!("Failed to persist conversation on client disconnect: {}", e); | ||
| } | ||
| break; | ||
| } | ||
| continue; | ||
|
|
||
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.
Persisting
self.messageshere has the same stale-snapshot problem as the server path: local conversation state is driven by streamed events and does not observe metadata-only DB updates (e.g., agent-sideupdate_message_metadataduring tool-pair summarization incrates/goose/src/agents/agent.rs). On an interrupt, this can replace the stored conversation with an older metadata view and re-expose messages that were marked invisible.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.
Hmm. we probably don't want make it complicated and the scenario of metadata being overwritten as an edge case is better than missing the full context for the interrupted turn.