-
Notifications
You must be signed in to change notification settings - Fork 1.5k
feat: unified thread model for web gateway #607
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
255a027
1b030b2
e5851c6
7afaa59
092ca85
b5c79df
dae234b
87e07b9
2b9eb75
431a9de
d0abde1
bda2b2b
78714ac
e16d63a
32afa42
7a6a26a
48ccfda
a282893
2bd9a39
db760a1
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 |
|---|---|---|
|
|
@@ -22,3 +22,4 @@ bench-results/ | |
| # WASM build artifacts (loaded from disk, not bundled) | ||
| *.wasm | ||
|
|
||
| trace_*.json | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,13 @@ | ||
| -- Partial unique indexes to prevent duplicate singleton conversations. | ||
| -- These guard against TOCTOU races in get_or_create_routine_conversation | ||
| -- and get_or_create_heartbeat_conversation. | ||
|
|
||
| -- One routine conversation per user per routine_id. | ||
| CREATE UNIQUE INDEX IF NOT EXISTS uq_conv_routine | ||
| ON conversations (user_id, (metadata->>'routine_id')) | ||
| WHERE metadata->>'routine_id' IS NOT NULL; | ||
|
|
||
| -- One heartbeat conversation per user. | ||
| CREATE UNIQUE INDEX IF NOT EXISTS uq_conv_heartbeat | ||
| ON conversations (user_id) | ||
| WHERE metadata->>'thread_type' = 'heartbeat'; | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -29,6 +29,7 @@ use std::time::Duration; | |
| use tokio::sync::mpsc; | ||
|
|
||
| use crate::channels::OutgoingResponse; | ||
| use crate::db::Database; | ||
| use crate::llm::{ChatMessage, CompletionRequest, LlmProvider, Reasoning}; | ||
| use crate::safety::SafetyLayer; | ||
| use crate::workspace::Workspace; | ||
|
|
@@ -103,6 +104,7 @@ pub struct HeartbeatRunner { | |
| llm: Arc<dyn LlmProvider>, | ||
| safety: Arc<SafetyLayer>, | ||
| response_tx: Option<mpsc::Sender<OutgoingResponse>>, | ||
| store: Option<Arc<dyn Database>>, | ||
| consecutive_failures: u32, | ||
| } | ||
|
|
||
|
|
@@ -122,6 +124,7 @@ impl HeartbeatRunner { | |
| llm, | ||
| safety, | ||
| response_tx: None, | ||
| store: None, | ||
| consecutive_failures: 0, | ||
| } | ||
| } | ||
|
|
@@ -132,6 +135,12 @@ impl HeartbeatRunner { | |
| self | ||
| } | ||
|
|
||
| /// Set the database store for persistent heartbeat conversations. | ||
| pub fn with_store(mut self, store: Arc<dyn Database>) -> Self { | ||
| self.store = Some(store); | ||
| self | ||
| } | ||
|
|
||
| /// Run the heartbeat loop. | ||
| /// | ||
| /// This runs forever, checking periodically based on the configured interval. | ||
|
|
@@ -292,9 +301,32 @@ impl HeartbeatRunner { | |
| return; | ||
| }; | ||
|
|
||
| let user_id = self.config.notify_user_id.as_deref().unwrap_or("default"); | ||
|
|
||
| // Persist to heartbeat conversation and get thread_id | ||
| let thread_id = if let Some(ref store) = self.store { | ||
| match store.get_or_create_heartbeat_conversation(user_id).await { | ||
| Ok(conv_id) => { | ||
| if let Err(e) = store | ||
| .add_conversation_message(conv_id, "assistant", message) | ||
| .await | ||
| { | ||
| tracing::error!("Failed to persist heartbeat message: {}", e); | ||
| } | ||
| Some(conv_id.to_string()) | ||
| } | ||
| Err(e) => { | ||
| tracing::error!("Failed to get heartbeat conversation: {}", e); | ||
| None | ||
| } | ||
| } | ||
| } else { | ||
| None | ||
| }; | ||
|
|
||
| let response = OutgoingResponse { | ||
| content: format!("🔔 *Heartbeat Alert*\n\n{}", message), | ||
| thread_id: None, | ||
| thread_id, | ||
|
Comment on lines
+304
to
+329
|
||
| attachments: Vec::new(), | ||
| metadata: serde_json::json!({ | ||
| "source": "heartbeat", | ||
|
|
@@ -356,11 +388,15 @@ pub fn spawn_heartbeat( | |
| llm: Arc<dyn LlmProvider>, | ||
| safety: Arc<SafetyLayer>, | ||
| response_tx: Option<mpsc::Sender<OutgoingResponse>>, | ||
| store: Option<Arc<dyn Database>>, | ||
| ) -> tokio::task::JoinHandle<()> { | ||
| let mut runner = HeartbeatRunner::new(config, hygiene_config, workspace, llm, safety); | ||
| if let Some(tx) = response_tx { | ||
| runner = runner.with_response_channel(tx); | ||
| } | ||
| if let Some(s) = store { | ||
| runner = runner.with_store(s); | ||
| } | ||
|
|
||
| tokio::spawn(async move { | ||
| runner.run().await; | ||
|
|
@@ -495,4 +531,22 @@ mod tests { | |
| let content = "<!-- comment -->\nActual task here"; | ||
| assert!(!is_effectively_empty(content)); | ||
| } | ||
|
|
||
| #[test] | ||
| fn test_spawn_heartbeat_accepts_store_param() { | ||
| // Regression: spawn_heartbeat must accept an optional Database store | ||
| // for persisting heartbeat notifications to a dedicated conversation. | ||
| // Compile-time check: the 7th parameter is `Option<Arc<dyn Database>>`. | ||
| #[allow(clippy::type_complexity)] | ||
| let _fn_ptr: fn( | ||
| HeartbeatConfig, | ||
| HygieneConfig, | ||
| Arc<crate::workspace::Workspace>, | ||
| Arc<dyn crate::llm::LlmProvider>, | ||
| Arc<crate::safety::SafetyLayer>, | ||
| Option<tokio::sync::mpsc::Sender<crate::channels::OutgoingResponse>>, | ||
| Option<Arc<dyn crate::db::Database>>, | ||
| ) -> tokio::task::JoinHandle<()> = spawn_heartbeat; | ||
| let _ = _fn_ptr; | ||
| } | ||
| } | ||
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.
This migration creates
uq_conv_routine/uq_conv_heartbeatas unique indexes, but the Postgres Store code usesON CONFLICT ON CONSTRAINT uq_conv_*, which requires a named unique constraint. Either update the Store SQL to use index inference, or addALTER TABLE conversations ADD CONSTRAINT uq_conv_* UNIQUE USING INDEX uq_conv_*;here so the constraint exists.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.
Fixed in 092ca85 — the migration correctly creates unique indexes; the Store SQL was the problem. Changed to use
ON CONFLICT (column_expr) WHERE conditionsyntax which works with both indexes and constraints.