Fix CI approval flows and stale fixtures (#1478)

* Fix CI approval flows and stale fixtures

* Backfill approval thread mapping across channels
This commit is contained in:
Henry Park
2026-03-20 12:21:46 -07:00
committed by GitHub
parent ee6f5cd62a
commit d3b69e7be3
6 changed files with 169 additions and 141 deletions
+54 -9
View File
@@ -10,6 +10,7 @@
use std::sync::Arc;
use futures::StreamExt;
use uuid::Uuid;
use crate::agent::context_monitor::ContextMonitor;
use crate::agent::heartbeat::spawn_heartbeat;
@@ -1014,15 +1015,59 @@ impl Agent {
}
}
// Resolve session and thread
let (session, thread_id) = self
.session_manager
.resolve_thread(
&message.user_id,
&message.channel,
message.conversation_scope(),
)
.await;
// Resolve session and thread. Approval submissions are allowed to
// target an already-loaded owned thread by UUID across channels so the
// web approval UI can approve work that originated from HTTP/other
// owner-scoped channels.
let approval_thread_uuid = if matches!(
submission,
Submission::ExecApproval { .. } | Submission::ApprovalResponse { .. }
) {
message
.conversation_scope()
.and_then(|thread_id| Uuid::parse_str(thread_id).ok())
} else {
None
};
let (session, thread_id) = if let Some(target_thread_id) = approval_thread_uuid {
let session = self
.session_manager
.get_or_create_session(&message.user_id)
.await;
let mut sess = session.lock().await;
if sess.threads.contains_key(&target_thread_id) {
sess.active_thread = Some(target_thread_id);
sess.last_active_at = chrono::Utc::now();
drop(sess);
self.session_manager
.register_thread(
&message.user_id,
&message.channel,
target_thread_id,
Arc::clone(&session),
)
.await;
(session, target_thread_id)
} else {
drop(sess);
self.session_manager
.resolve_thread(
&message.user_id,
&message.channel,
message.conversation_scope(),
)
.await
}
} else {
self.session_manager
.resolve_thread(
&message.user_id,
&message.channel,
message.conversation_scope(),
)
.await
};
tracing::debug!(
message_id = %message.id,
thread_id = %thread_id,
+27
View File
@@ -772,6 +772,33 @@ mod tests {
assert_ne!(resolved, tid);
}
#[tokio::test]
async fn test_register_then_resolve_same_uuid_on_second_channel_reuses_thread() {
use crate::agent::session::{Session, Thread};
let manager = SessionManager::new();
let tid = Uuid::new_v4();
let session = Arc::new(Mutex::new(Session::new("user-cross")));
{
let mut sess = session.lock().await;
let thread = Thread::with_id(tid, sess.id);
sess.threads.insert(tid, thread);
}
manager
.register_thread("user-cross", "http", tid, Arc::clone(&session))
.await;
manager
.register_thread("user-cross", "gateway", tid, Arc::clone(&session))
.await;
let (_, resolved) = manager
.resolve_thread("user-cross", "gateway", Some(&tid.to_string()))
.await;
assert_eq!(resolved, tid);
}
// === QA Plan P3 - 4.2: Concurrent session stress tests ===
#[tokio::test]
+2 -5
View File
@@ -299,15 +299,12 @@ mod tests {
// SAFETY: Under ENV_MUTEX, no concurrent env access.
unsafe {
std::env::set_var("EMBEDDING_BASE_URL", "https://custom.example.com");
std::env::set_var("EMBEDDING_BASE_URL", "https://8.8.8.8");
}
let settings = Settings::default();
let config = EmbeddingsConfig::resolve(&settings).expect("resolve should succeed");
assert_eq!(
config.openai_base_url.as_deref(),
Some("https://custom.example.com")
);
assert_eq!(config.openai_base_url.as_deref(), Some("https://8.8.8.8"));
// SAFETY: Under ENV_MUTEX.
unsafe {
std::env::remove_var("EMBEDDING_BASE_URL");
+4 -4
View File
@@ -855,19 +855,19 @@ mod tests {
// SAFETY: Under ENV_MUTEX.
unsafe {
std::env::set_var("LLM_BACKEND", "openai_compatible");
std::env::set_var("LLM_BASE_URL", "http://env-url/v1");
std::env::set_var("LLM_BASE_URL", "http://localhost:8000/v1");
}
let settings = Settings {
llm_backend: Some("openai_compatible".to_string()),
openai_compatible_base_url: Some("http://settings-url/v1".to_string()),
openai_compatible_base_url: Some("http://localhost:9000/v1".to_string()),
..Default::default()
};
let cfg = LlmConfig::resolve(&settings).expect("resolve should succeed");
let provider = cfg.provider.expect("should have provider config");
assert_eq!(
provider.base_url, "http://env-url/v1",
provider.base_url, "http://localhost:8000/v1",
"env var should take priority over settings"
);
@@ -879,7 +879,7 @@ mod tests {
let cfg = LlmConfig::resolve(&settings).expect("resolve should succeed");
let provider = cfg.provider.expect("should have provider config");
assert_eq!(
provider.base_url, "http://settings-url/v1",
provider.base_url, "http://localhost:9000/v1",
"settings should take priority over registry default"
);