From 67c0546c0f438662230655e8bd3dc6d64434b5bb Mon Sep 17 00:00:00 2001 From: Zaki Date: Sun, 12 Apr 2026 07:11:18 +0900 Subject: [PATCH 01/11] fix(gateway): show descriptive chat titles instead of hex hash IDs (#2237) New conversations in the web sidebar were displaying truncated UUIDs (e.g., "5638f52c") because the title fell through to the raw ID fallback when no title was available from the database. Three-layer fix: - Store conversation title in metadata on first user message (both v1 and v2 engine paths) so titles persist even if the SQL subquery has timing issues - SQL title derivation now checks metadata.title as a fallback between the message-subquery title and routine_name (both PostgreSQL and libSQL backends) - Frontend threadTitle() shows "Untitled chat" via i18n instead of hex hash substring; in-memory fallback derives title from first turn Co-Authored-By: Claude Opus 4.6 (1M context) --- crates/ironclaw_gateway/static/i18n/en.js | 2 + crates/ironclaw_gateway/static/i18n/ko.js | 2 + crates/ironclaw_gateway/static/i18n/zh-CN.js | 2 + .../static/js/core/history.js | 4 +- src/agent/thread_ops.rs | 14 ++ src/bridge/router.rs | 11 ++ src/channels/web/features/chat/mod.rs | 33 +++-- src/db/libsql/conversations.rs | 121 ++++++++++++++++-- src/history/store.rs | 40 ++++-- 9 files changed, 194 insertions(+), 35 deletions(-) diff --git a/crates/ironclaw_gateway/static/i18n/en.js b/crates/ironclaw_gateway/static/i18n/en.js index 3f48d48d015..9cb167df9f4 100644 --- a/crates/ironclaw_gateway/static/i18n/en.js +++ b/crates/ironclaw_gateway/static/i18n/en.js @@ -783,6 +783,8 @@ I18n.register('en', { // Thread types 'thread.heartbeatAlerts': 'Heartbeat Alerts', 'thread.routine': 'Routine', + 'thread.newChat': 'New chat', + 'thread.untitled': 'Untitled chat', // Extensions (dynamic) 'extensions.openingAuth': 'Opening authentication for {name}', diff --git a/crates/ironclaw_gateway/static/i18n/ko.js b/crates/ironclaw_gateway/static/i18n/ko.js index eaf69a275c4..df5b89d7243 100644 --- a/crates/ironclaw_gateway/static/i18n/ko.js +++ b/crates/ironclaw_gateway/static/i18n/ko.js @@ -782,6 +782,8 @@ I18n.register('ko', { // 스레드 유형 'thread.heartbeatAlerts': '하트비트 알림', 'thread.routine': '루틴', + 'thread.newChat': '새 대화', + 'thread.untitled': '제목 없는 대화', // 확장 (동적) 'extensions.openingAuth': '{name}에 대한 인증을 여는 중', diff --git a/crates/ironclaw_gateway/static/i18n/zh-CN.js b/crates/ironclaw_gateway/static/i18n/zh-CN.js index 2b26595177e..c406ec23a3f 100644 --- a/crates/ironclaw_gateway/static/i18n/zh-CN.js +++ b/crates/ironclaw_gateway/static/i18n/zh-CN.js @@ -782,6 +782,8 @@ I18n.register('zh-CN', { // 线程类型 'thread.heartbeatAlerts': '心跳提醒', 'thread.routine': '定时任务', + 'thread.newChat': '新对话', + 'thread.untitled': '未命名对话', // 扩展(动态) 'extensions.openingAuth': '正在为 {name} 打开认证', diff --git a/crates/ironclaw_gateway/static/js/core/history.js b/crates/ironclaw_gateway/static/js/core/history.js index db762152601..0fb70cdb705 100644 --- a/crates/ironclaw_gateway/static/js/core/history.js +++ b/crates/ironclaw_gateway/static/js/core/history.js @@ -307,8 +307,8 @@ function threadTitle(thread) { if (thread.thread_type === 'heartbeat') return I18n.t('thread.heartbeatAlerts'); if (thread.thread_type === 'routine') return I18n.t('thread.routine'); if (ch !== 'gateway') return ch.charAt(0).toUpperCase() + ch.slice(1); - if (thread.turn_count === 0) return 'New chat'; - return thread.id.substring(0, 8); + if (thread.turn_count === 0) return I18n.t('thread.newChat'); + return I18n.t('thread.untitled'); } function relativeTime(isoStr) { diff --git a/src/agent/thread_ops.rs b/src/agent/thread_ops.rs index 5efdbc00e15..e800f5f55a9 100644 --- a/src/agent/thread_ops.rs +++ b/src/agent/thread_ops.rs @@ -1085,6 +1085,20 @@ impl Agent { None } } + + // Set a conversation title from the first user message so the + // thread list shows a descriptive name in the sidebar. + if let Ok(None) = store + .get_conversation_metadata(thread_id) + .await + .map(|m| m.and_then(|v| v.get("title").and_then(|t| t.as_str()).map(String::from))) + { + let title_text: String = user_input.chars().take(100).collect(); + let title_val = serde_json::json!(title_text); + let _ = store + .update_conversation_metadata_field(thread_id, "title", &title_val) + .await; + } } /// Persist the assistant response to the DB after the agentic loop completes. diff --git a/src/bridge/router.rs b/src/bridge/router.rs index 9155c5d2fc0..fc372f712b2 100644 --- a/src/bridge/router.rs +++ b/src/bridge/router.rs @@ -3651,6 +3651,17 @@ async fn handle_with_engine_inner( let _ = db .add_conversation_message(cid, "user", effective_content) .await; + if let Ok(None) = db + .get_conversation_metadata(cid) + .await + .map(|m| m.and_then(|v| v.get("title").and_then(|t| t.as_str()).map(String::from))) + { + let title_text: String = effective_content.chars().take(100).collect(); + let title_val = serde_json::json!(title_text); + let _ = db + .update_conversation_metadata_field(cid, "title", &title_val) + .await; + } } } diff --git a/src/channels/web/features/chat/mod.rs b/src/channels/web/features/chat/mod.rs index f2a8934a780..d2e07e40abf 100644 --- a/src/channels/web/features/chat/mod.rs +++ b/src/channels/web/features/chat/mod.rs @@ -752,15 +752,30 @@ pub(crate) async fn chat_threads_handler( sorted_threads.sort_by_key(|t| std::cmp::Reverse(t.updated_at)); let threads: Vec = sorted_threads .into_iter() - .map(|t| ThreadInfo { - id: t.id, - state: thread_state_label(t.state).to_string(), - turn_count: t.turns.len(), - created_at: t.created_at.to_rfc3339(), - updated_at: t.updated_at.to_rfc3339(), - title: None, - thread_type: None, - channel: Some("gateway".to_string()), + .map(|t| { + let title = t + .turns + .first() + .map(|turn| { + turn.user_input + .split_whitespace() + .collect::>() + .join(" ") + .chars() + .take(100) + .collect::() + }) + .filter(|s| !s.is_empty()); + ThreadInfo { + id: t.id, + state: thread_state_label(t.state).to_string(), + turn_count: t.turns.len(), + created_at: t.created_at.to_rfc3339(), + updated_at: t.updated_at.to_rfc3339(), + title, + thread_type: None, + channel: Some("gateway".to_string()), + } }) .collect(); diff --git a/src/db/libsql/conversations.rs b/src/db/libsql/conversations.rs index 3b673ba9212..5bebcf99f0d 100644 --- a/src/db/libsql/conversations.rs +++ b/src/db/libsql/conversations.rs @@ -171,12 +171,20 @@ impl ConversationStore for LibSqlBackend { .and_then(|v| v.as_str()) .map(String::from); let sql_title = get_opt_text(&row, 6); - let title = sql_title.or_else(|| { - metadata - .get("routine_name") - .and_then(|v| v.as_str()) - .map(String::from) - }); + let title = sql_title + .or_else(|| { + metadata + .get("title") + .and_then(|v| v.as_str()) + .filter(|s| !s.is_empty()) + .map(String::from) + }) + .or_else(|| { + metadata + .get("routine_name") + .and_then(|v| v.as_str()) + .map(String::from) + }); results.push(ConversationSummary { id: row .get::(0) @@ -250,12 +258,20 @@ impl ConversationStore for LibSqlBackend { .and_then(|v| v.as_str()) .map(String::from); let sql_title = get_opt_text(&row, 6); - let title = sql_title.or_else(|| { - metadata - .get("routine_name") - .and_then(|v| v.as_str()) - .map(String::from) - }); + let title = sql_title + .or_else(|| { + metadata + .get("title") + .and_then(|v| v.as_str()) + .filter(|s| !s.is_empty()) + .map(String::from) + }) + .or_else(|| { + metadata + .get("routine_name") + .and_then(|v| v.as_str()) + .map(String::from) + }); results.push(ConversationSummary { id: row .get::(0) @@ -1009,4 +1025,85 @@ mod tests { "assistant thread lookup should backfill a legacy NULL source_channel" ); } + + /// Regression test for #2237: conversations with a metadata title should + /// use it as a fallback when the message-derived title is NULL. + #[tokio::test] + async fn test_metadata_title_used_as_fallback() { + let dir = tempfile::tempdir().unwrap(); + let db_path = dir.path().join("test_metadata_title.db"); + let backend = LibSqlBackend::new_local(&db_path).await.unwrap(); + backend.run_migrations().await.unwrap(); + + let conv_id = Uuid::new_v4(); + let user_id = "user-title-test"; + + // Create a conversation with no messages + backend + .ensure_conversation(conv_id, "gateway", user_id, None, Some("gateway")) + .await + .unwrap(); + + // Set a metadata title (simulating what persist_user_message does) + let title_val = serde_json::json!("What is the weather today?"); + backend + .update_conversation_metadata_field(conv_id, "title", &title_val) + .await + .unwrap(); + + // List conversations -- title should come from metadata even without messages + let convs = backend + .list_conversations_all_channels(user_id, 50) + .await + .unwrap(); + + let conv = convs.iter().find(|c| c.id == conv_id).unwrap(); + assert_eq!( + conv.title.as_deref(), + Some("What is the weather today?"), + "Conversation title should fall back to metadata title when no user messages exist" + ); + } + + /// Regression test for #2237: message-derived title takes precedence over metadata. + #[tokio::test] + async fn test_message_title_takes_precedence_over_metadata() { + let dir = tempfile::tempdir().unwrap(); + let db_path = dir.path().join("test_message_title_precedence.db"); + let backend = LibSqlBackend::new_local(&db_path).await.unwrap(); + backend.run_migrations().await.unwrap(); + + let conv_id = Uuid::new_v4(); + let user_id = "user-title-precedence"; + + backend + .ensure_conversation(conv_id, "gateway", user_id, None, Some("gateway")) + .await + .unwrap(); + + // Set metadata title + let title_val = serde_json::json!("metadata title"); + backend + .update_conversation_metadata_field(conv_id, "title", &title_val) + .await + .unwrap(); + + // Add a user message + backend + .add_conversation_message(conv_id, "user", "actual user message") + .await + .unwrap(); + + let convs = backend + .list_conversations_all_channels(user_id, 50) + .await + .unwrap(); + + let conv = convs.iter().find(|c| c.id == conv_id).unwrap(); + assert_eq!( + conv.title.as_deref(), + Some("actual user message"), + "Message-derived title should take precedence over metadata title" + ); + } } diff --git a/src/history/store.rs b/src/history/store.rs index 7823bf8c302..f3916597677 100644 --- a/src/history/store.rs +++ b/src/history/store.rs @@ -1839,12 +1839,20 @@ impl Store { .and_then(|v| v.as_str()) .map(String::from); let sql_title: Option = r.get("title"); - let title = sql_title.or_else(|| { - metadata - .get("routine_name") - .and_then(|v| v.as_str()) - .map(String::from) - }); + let title = sql_title + .or_else(|| { + metadata + .get("title") + .and_then(|v| v.as_str()) + .filter(|s| !s.is_empty()) + .map(String::from) + }) + .or_else(|| { + metadata + .get("routine_name") + .and_then(|v| v.as_str()) + .map(String::from) + }); ConversationSummary { id: r.get("id"), title, @@ -1913,12 +1921,20 @@ impl Store { // For routine/heartbeat threads, derive title from metadata // since they may have no user messages. let sql_title: Option = r.get("title"); - let title = sql_title.or_else(|| { - metadata - .get("routine_name") - .and_then(|v| v.as_str()) - .map(String::from) - }); + let title = sql_title + .or_else(|| { + metadata + .get("title") + .and_then(|v| v.as_str()) + .filter(|s| !s.is_empty()) + .map(String::from) + }) + .or_else(|| { + metadata + .get("routine_name") + .and_then(|v| v.as_str()) + .map(String::from) + }); ConversationSummary { id: r.get("id"), title, From f3e8543a8b3a03c9341c5f03b75ef05e061c2600 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 11 Apr 2026 23:29:38 +0000 Subject: [PATCH 02/11] fix(deny): ignore RUSTSEC-2026-0097 (rand 0.8.x), remove stale wasmtime advisories rand 0.8.5 unsoundness with custom logger is pinned by transitive deps (wasmtime-wasi, libsql, rig-core, zbus, tower 0.4). Upgrade tracked separately. Remove 4 wasmtime advisories resolved by v43 upgrade. https://claude.ai/code/session_01MMhMuxXvAXTcFZ3EAga12k --- deny.toml | 5 ----- 1 file changed, 5 deletions(-) diff --git a/deny.toml b/deny.toml index 6a2f41fcb40..e8073c343fc 100644 --- a/deny.toml +++ b/deny.toml @@ -9,11 +9,6 @@ ignore = [ "RUSTSEC-2025-0111", # rustls-webpki CRL distributionPoint matching — 0.102.8 pinned by libsql transitive dep "RUSTSEC-2026-0049", - # rustls-webpki URI name constraint bypass — 0.102.8 pinned by libsql transitive dep; - # patched in >=0.103.12 but libsql 0.6.0 requires rustls 0.22 which pins 0.102.x - "RUSTSEC-2026-0098", - # rustls-webpki wildcard name constraint bypass — same 0.102.8 pin from libsql - "RUSTSEC-2026-0099", # rand unsoundness with custom logger calling rand::rng() during reseed — we don't use this pattern; # revisit/remove by 2026-06-30, or when transitive deps (tower, nanoid, phf_generator) release rand ≥0.9.3 compat "RUSTSEC-2026-0097", From 4e2708cfa698364924b8062f8a310910f62b4fb3 Mon Sep 17 00:00:00 2001 From: Zaki Date: Mon, 13 Apr 2026 19:00:15 +0900 Subject: [PATCH 03/11] fix(title): deduplicate title-setting logic, handle empty input Extract shared set_title_if_missing() helper into db module to eliminate duplicated title-setting code between thread_ops.rs and bridge/router.rs. Skip empty/whitespace-only input to prevent permanently blocking title-setting on image-only or attachment-only messages. Addresses PR #2348 review feedback (duplicated code, empty input blocking). [skip-regression-check] Co-Authored-By: Claude Opus 4.6 (1M context) --- src/agent/thread_ops.rs | 14 +------------- src/bridge/router.rs | 12 +----------- src/db/mod.rs | 33 +++++++++++++++++++++++++++++++++ 3 files changed, 35 insertions(+), 24 deletions(-) diff --git a/src/agent/thread_ops.rs b/src/agent/thread_ops.rs index e800f5f55a9..d7ded921946 100644 --- a/src/agent/thread_ops.rs +++ b/src/agent/thread_ops.rs @@ -1086,19 +1086,7 @@ impl Agent { } } - // Set a conversation title from the first user message so the - // thread list shows a descriptive name in the sidebar. - if let Ok(None) = store - .get_conversation_metadata(thread_id) - .await - .map(|m| m.and_then(|v| v.get("title").and_then(|t| t.as_str()).map(String::from))) - { - let title_text: String = user_input.chars().take(100).collect(); - let title_val = serde_json::json!(title_text); - let _ = store - .update_conversation_metadata_field(thread_id, "title", &title_val) - .await; - } + crate::db::set_title_if_missing(store.as_ref(), thread_id, user_input).await; } /// Persist the assistant response to the DB after the agentic loop completes. diff --git a/src/bridge/router.rs b/src/bridge/router.rs index fc372f712b2..0d2c0264243 100644 --- a/src/bridge/router.rs +++ b/src/bridge/router.rs @@ -3651,17 +3651,7 @@ async fn handle_with_engine_inner( let _ = db .add_conversation_message(cid, "user", effective_content) .await; - if let Ok(None) = db - .get_conversation_metadata(cid) - .await - .map(|m| m.and_then(|v| v.get("title").and_then(|t| t.as_str()).map(String::from))) - { - let title_text: String = effective_content.chars().take(100).collect(); - let title_val = serde_json::json!(title_text); - let _ = db - .update_conversation_metadata_field(cid, "title", &title_val) - .await; - } + crate::db::set_title_if_missing(db.as_ref(), cid, effective_content).await; } } diff --git a/src/db/mod.rs b/src/db/mod.rs index 7b605773ace..ee7044ecd42 100644 --- a/src/db/mod.rs +++ b/src/db/mod.rs @@ -494,6 +494,39 @@ pub trait ConversationStore: Send + Sync { ) -> Result, DatabaseError>; } +/// Set a conversation title from user input if one hasn't been set yet. +/// +/// Skips empty/whitespace-only input so that image-only or attachment-only +/// messages don't permanently block title-setting with an empty string. +/// Truncates to the first 100 characters for sidebar display. +pub async fn set_title_if_missing( + store: &(dyn ConversationStore + Send + Sync), + conversation_id: Uuid, + user_input: &str, +) { + let trimmed = user_input.trim(); + if trimmed.is_empty() { + return; + } + + let has_title = match store.get_conversation_metadata(conversation_id).await { + Ok(Some(meta)) => meta + .get("title") + .and_then(|t| t.as_str()) + .is_some_and(|s| !s.is_empty()), + Ok(None) => false, + Err(_) => return, + }; + + if !has_title { + let title_text: String = trimmed.chars().take(100).collect(); + let title_val = serde_json::json!(title_text); + let _ = store + .update_conversation_metadata_field(conversation_id, "title", &title_val) + .await; + } +} + #[async_trait] pub trait JobStore: Send + Sync { async fn save_job(&self, ctx: &JobContext) -> Result<(), DatabaseError>; From 2744fc89e3b97542d21cd3afc8d6780eaa6825fd Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 19 Apr 2026 04:33:17 +0000 Subject: [PATCH 04/11] fix(ci): resolve compile error in persist_user_message and restore rustls-webpki advisory ignores MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The set_title_if_missing call added after the match tail expression broke the return type (Option vs ()). Capture the match result in a variable so the title-setting runs before returning. Also restore RUSTSEC-2026-0098 and RUSTSEC-2026-0099 ignores — rustls-webpki 0.102.8 is still pinned by the libsql transitive dep. https://claude.ai/code/session_01JRasj3ujmr1uzmfUeLbNFo --- deny.toml | 5 +++++ src/agent/thread_ops.rs | 6 ++++-- 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/deny.toml b/deny.toml index e8073c343fc..6a2f41fcb40 100644 --- a/deny.toml +++ b/deny.toml @@ -9,6 +9,11 @@ ignore = [ "RUSTSEC-2025-0111", # rustls-webpki CRL distributionPoint matching — 0.102.8 pinned by libsql transitive dep "RUSTSEC-2026-0049", + # rustls-webpki URI name constraint bypass — 0.102.8 pinned by libsql transitive dep; + # patched in >=0.103.12 but libsql 0.6.0 requires rustls 0.22 which pins 0.102.x + "RUSTSEC-2026-0098", + # rustls-webpki wildcard name constraint bypass — same 0.102.8 pin from libsql + "RUSTSEC-2026-0099", # rand unsoundness with custom logger calling rand::rng() during reseed — we don't use this pattern; # revisit/remove by 2026-06-30, or when transitive deps (tower, nanoid, phf_generator) release rand ≥0.9.3 compat "RUSTSEC-2026-0097", diff --git a/src/agent/thread_ops.rs b/src/agent/thread_ops.rs index d7ded921946..6476fe6a0ad 100644 --- a/src/agent/thread_ops.rs +++ b/src/agent/thread_ops.rs @@ -1049,7 +1049,7 @@ impl Agent { return None; } - match store + let result = match store .add_conversation_message(thread_id, "user", user_input) .await { @@ -1084,9 +1084,11 @@ impl Agent { .await; None } - } + }; crate::db::set_title_if_missing(store.as_ref(), thread_id, user_input).await; + + result } /// Persist the assistant response to the DB after the agentic loop completes. From 716b878481e5e99269f14ce8e66954983bb1e3b8 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 20 Apr 2026 21:20:37 +0000 Subject: [PATCH 05/11] fix(title): normalize sql_title whitespace and early-return on missing conversation Addresses review feedback on PR #2700: - Return early from set_title_if_missing when conversation record doesn't exist (Ok(None)) to avoid a failing metadata update attempt - Normalize whitespace in title_text (collapse multi-space/newlines) - Filter blank sql_title values before metadata fallback in both libSQL and PostgreSQL paths so image-only/empty first turns properly fall through to metadata.title https://claude.ai/code/session_01GHzXcZjSjxHEeTDDHG4fQK --- src/db/libsql/conversations.rs | 8 ++++++-- src/db/mod.rs | 5 ++--- src/history/store.rs | 8 ++++++-- 3 files changed, 14 insertions(+), 7 deletions(-) diff --git a/src/db/libsql/conversations.rs b/src/db/libsql/conversations.rs index 5bebcf99f0d..339caa35e32 100644 --- a/src/db/libsql/conversations.rs +++ b/src/db/libsql/conversations.rs @@ -170,7 +170,9 @@ impl ConversationStore for LibSqlBackend { .and_then(|v| v.get("started_at")) .and_then(|v| v.as_str()) .map(String::from); - let sql_title = get_opt_text(&row, 6); + let sql_title = get_opt_text(&row, 6) + .map(|s| s.split_whitespace().collect::>().join(" ")) + .filter(|s| !s.is_empty()); let title = sql_title .or_else(|| { metadata @@ -257,7 +259,9 @@ impl ConversationStore for LibSqlBackend { .and_then(|v| v.get("started_at")) .and_then(|v| v.as_str()) .map(String::from); - let sql_title = get_opt_text(&row, 6); + let sql_title = get_opt_text(&row, 6) + .map(|s| s.split_whitespace().collect::>().join(" ")) + .filter(|s| !s.is_empty()); let title = sql_title .or_else(|| { metadata diff --git a/src/db/mod.rs b/src/db/mod.rs index ee7044ecd42..e8a6ff7bdca 100644 --- a/src/db/mod.rs +++ b/src/db/mod.rs @@ -514,12 +514,11 @@ pub async fn set_title_if_missing( .get("title") .and_then(|t| t.as_str()) .is_some_and(|s| !s.is_empty()), - Ok(None) => false, - Err(_) => return, + Ok(None) | Err(_) => return, }; if !has_title { - let title_text: String = trimmed.chars().take(100).collect(); + let title_text: String = trimmed.split_whitespace().collect::>().join(" ").chars().take(100).collect(); let title_val = serde_json::json!(title_text); let _ = store .update_conversation_metadata_field(conversation_id, "title", &title_val) diff --git a/src/history/store.rs b/src/history/store.rs index f3916597677..d31dd933b5c 100644 --- a/src/history/store.rs +++ b/src/history/store.rs @@ -1838,7 +1838,9 @@ impl Store { .and_then(|v| v.get("started_at")) .and_then(|v| v.as_str()) .map(String::from); - let sql_title: Option = r.get("title"); + let sql_title: Option = r.get::<_, Option>("title") + .map(|s| s.split_whitespace().collect::>().join(" ")) + .filter(|s| !s.is_empty()); let title = sql_title .or_else(|| { metadata @@ -1920,7 +1922,9 @@ impl Store { .map(String::from); // For routine/heartbeat threads, derive title from metadata // since they may have no user messages. - let sql_title: Option = r.get("title"); + let sql_title: Option = r.get::<_, Option>("title") + .map(|s| s.split_whitespace().collect::>().join(" ")) + .filter(|s| !s.is_empty()); let title = sql_title .or_else(|| { metadata From cbe8101855d9204a5ae041be92028d64a09b6b3e Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 20 Apr 2026 22:11:23 +0000 Subject: [PATCH 06/11] fix(fmt): apply cargo fmt to long method chains https://claude.ai/code/session_0175E1t6WviBWM1xGkbtV24q --- src/db/mod.rs | 8 +++++++- src/history/store.rs | 6 ++++-- 2 files changed, 11 insertions(+), 3 deletions(-) diff --git a/src/db/mod.rs b/src/db/mod.rs index e8a6ff7bdca..2194ffe01e2 100644 --- a/src/db/mod.rs +++ b/src/db/mod.rs @@ -518,7 +518,13 @@ pub async fn set_title_if_missing( }; if !has_title { - let title_text: String = trimmed.split_whitespace().collect::>().join(" ").chars().take(100).collect(); + let title_text: String = trimmed + .split_whitespace() + .collect::>() + .join(" ") + .chars() + .take(100) + .collect(); let title_val = serde_json::json!(title_text); let _ = store .update_conversation_metadata_field(conversation_id, "title", &title_val) diff --git a/src/history/store.rs b/src/history/store.rs index d31dd933b5c..1f9a4aa89bb 100644 --- a/src/history/store.rs +++ b/src/history/store.rs @@ -1838,7 +1838,8 @@ impl Store { .and_then(|v| v.get("started_at")) .and_then(|v| v.as_str()) .map(String::from); - let sql_title: Option = r.get::<_, Option>("title") + let sql_title: Option = r + .get::<_, Option>("title") .map(|s| s.split_whitespace().collect::>().join(" ")) .filter(|s| !s.is_empty()); let title = sql_title @@ -1922,7 +1923,8 @@ impl Store { .map(String::from); // For routine/heartbeat threads, derive title from metadata // since they may have no user messages. - let sql_title: Option = r.get::<_, Option>("title") + let sql_title: Option = r + .get::<_, Option>("title") .map(|s| s.split_whitespace().collect::>().join(" ")) .filter(|s| !s.is_empty()); let title = sql_title From de2876ba1f5abcc5267a9cdf205ee5bff342cc64 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 21 Apr 2026 01:27:09 +0000 Subject: [PATCH 07/11] =?UTF-8?q?fix(title):=20address=20review=20nits=20?= =?UTF-8?q?=E2=80=94=20gate=20title-set=20on=20persist=20success,=20log=20?= =?UTF-8?q?metadata=20errors?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Move set_title_if_missing call inside Ok arm so failed message persists don't produce ghost titles (ilblackdragon review) - Log metadata read errors at debug level instead of silently swallowing [skip-regression-check] https://claude.ai/code/session_01BsM2KUzdZNBeoLjk7LymZS --- src/agent/thread_ops.rs | 4 +++- src/db/mod.rs | 6 +++++- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/src/agent/thread_ops.rs b/src/agent/thread_ops.rs index 6476fe6a0ad..16b49f2804d 100644 --- a/src/agent/thread_ops.rs +++ b/src/agent/thread_ops.rs @@ -1086,7 +1086,9 @@ impl Agent { } }; - crate::db::set_title_if_missing(store.as_ref(), thread_id, user_input).await; + if result.is_some() { + crate::db::set_title_if_missing(store.as_ref(), thread_id, user_input).await; + } result } diff --git a/src/db/mod.rs b/src/db/mod.rs index 2194ffe01e2..ee6abce5350 100644 --- a/src/db/mod.rs +++ b/src/db/mod.rs @@ -514,7 +514,11 @@ pub async fn set_title_if_missing( .get("title") .and_then(|t| t.as_str()) .is_some_and(|s| !s.is_empty()), - Ok(None) | Err(_) => return, + Ok(None) => return, + Err(e) => { + tracing::debug!("failed to read metadata for title-set: {e}"); + return; + } }; if !has_title { From eaedb2e55d2da4314d064c4e98e317aed8c0f94a Mon Sep 17 00:00:00 2001 From: Zaki Date: Tue, 21 Apr 2026 08:46:06 -0400 Subject: [PATCH 08/11] fix(title): derive titles from raw user text, gate on persist success, expand tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three review issues on #2700: 1. Title source (router.rs + thread_ops.rs). Titles were derived from `effective_content`, which is the attachment-augmented payload produced by `augment_with_attachments` — it can include a synthesized `` block or extracted OCR/pdf text. On an image- or attachment-only first turn this meant the sidebar title was the synthesized attachment text rather than user input, which defeats the point of skipping empty turns. Both call sites now pass the raw user `content` to `set_title_if_missing` while the conversation row body continues to store the augmented payload. `persist_user_message` takes a new `title_source` argument to keep the two distinct on the v1 path. 2. Ghost title on failed persist (router.rs). The v2 bridge dual-write discarded the `add_conversation_message` result with `let _`, so `set_title_if_missing` ran even when the first user row never landed — reproducing the ghost-title edge case thread_ops.rs already guards against. The result is now inspected and the title write is gated on `Ok`. 3. Test coverage (store.rs + thread_ops.rs). The libSQL regression tests don't pin the PG path or the caller wiring. Adds: - PG mirrors of `test_metadata_title_used_as_fallback` and `test_message_title_takes_precedence_over_metadata` behind `feature = "postgres"` + `#[ignore]` (integration tier). - A caller-level regression in `thread_ops` that drives `Agent::persist_user_message` against a real LibSqlBackend and pins all three invariants: attachment-augmented payload with real text uses the raw text; attachment-only first turn leaves the title unset; plain text still seeds correctly. Co-Authored-By: Claude Opus 4.7 (1M context) --- src/agent/thread_ops.rs | 241 +++++++++++++++++++++++++++++++++++++++- src/bridge/router.rs | 23 +++- src/history/store.rs | 139 +++++++++++++++++++++++ 3 files changed, 399 insertions(+), 4 deletions(-) diff --git a/src/agent/thread_ops.rs b/src/agent/thread_ops.rs index 16b49f2804d..3adaaa39750 100644 --- a/src/agent/thread_ops.rs +++ b/src/agent/thread_ops.rs @@ -684,6 +684,7 @@ impl Agent { &message.user_id, turn_number, effective_content, + content, turn_started_at, ) .await; @@ -1028,6 +1029,19 @@ impl Agent { /// /// This ensures the user message is durable even if the process crashes /// mid-response. Call this right after `thread.start_turn()`. + /// Persist the first-turn user message row and (on success) seed the + /// sidebar title from `title_source`. + /// + /// `user_input` is the attachment-augmented payload that the engine + /// actually processes and gets stored as the conversation row body. + /// `title_source` is the raw user-entered text before attachment + /// augmentation; deriving the sidebar title from the raw text means + /// an image- or attachment-only first turn doesn't claim the title + /// slot with the synthesized `` block or extracted + /// attachment OCR. `set_title_if_missing` already skips empty input, + /// so attachment-only turns naturally defer title-setting to a + /// later turn that carries real text. + #[allow(clippy::too_many_arguments)] pub(super) async fn persist_user_message( &self, thread_id: Uuid, @@ -1035,6 +1049,7 @@ impl Agent { user_id: &str, turn_number: usize, user_input: &str, + title_source: &str, started_at: DateTime, ) -> Option { let store = match self.store() { @@ -1087,7 +1102,7 @@ impl Agent { }; if result.is_some() { - crate::db::set_title_if_missing(store.as_ref(), thread_id, user_input).await; + crate::db::set_title_if_missing(store.as_ref(), thread_id, title_source).await; } result @@ -3949,6 +3964,230 @@ mod tests { ); } + /// Caller-level regression for PR #2700 review comments. + /// + /// Drives `persist_user_message` through a real `Agent` + real + /// `LibSqlBackend` store — the actual production persist path — and + /// pins three invariants that unit tests of `set_title_if_missing` + /// alone cannot pin: + /// + /// 1. **Title source.** When the first user turn carries + /// attachments, the sidebar title must be derived from the raw + /// user text (`title_source`), not the augmented `user_input` + /// that includes the synthesized `` block. + /// 2. **Empty-raw skip.** An image- or attachment-only first turn + /// (empty raw text, augmented body) must leave the title unset + /// so a later turn with real text can claim the slot. + /// 3. **Plain text.** A straightforward first user message with no + /// attachments still seeds the title correctly — negative + /// control for the edits above. + /// + /// Driving through the real caller matters because a regression in + /// the caller-side wiring (e.g. swapping `title_source` back to + /// `user_input`, or dropping the `result.is_some()` gate) would + /// leave all predicate-level unit tests green. + #[cfg(feature = "libsql")] + #[tokio::test] + async fn persist_user_message_title_uses_raw_text_not_augmented_payload() { + use crate::db::Database; + use crate::db::libsql::LibSqlBackend; + use chrono::Utc; + use uuid::Uuid; + + // Build a real libSQL-backed store and wire it into a test agent. + let tmp = tempfile::tempdir().expect("tempdir"); + let db_path = tmp.path().join("title_source.db"); + let backend = LibSqlBackend::new_local(&db_path) + .await + .expect("libsql backend"); + backend.run_migrations().await.expect("migrations"); + let store: Arc = Arc::new(backend); + + let (channels, _statuses) = { + let statuses = Arc::new(TokioMutex::new(Vec::new())); + let channels = Arc::new(crate::channels::ChannelManager::new()); + channels + .add(Box::new(RecordingStatusChannel { + statuses: Arc::clone(&statuses), + })) + .await; + (channels, statuses) + }; + + struct StaticLlmProvider; + #[async_trait::async_trait] + impl crate::llm::LlmProvider for StaticLlmProvider { + fn model_name(&self) -> &str { + "static-mock" + } + fn cost_per_token(&self) -> (Decimal, Decimal) { + (Decimal::ZERO, Decimal::ZERO) + } + async fn complete( + &self, + _request: crate::llm::CompletionRequest, + ) -> Result { + unreachable!("LLM not invoked by this test") + } + async fn complete_with_tools( + &self, + _request: crate::llm::ToolCompletionRequest, + ) -> Result { + unreachable!("LLM not invoked by this test") + } + } + + let deps = crate::agent::AgentDeps { + owner_id: "default".to_string(), + store: Some(Arc::clone(&store)), + settings_store: None, + llm: Arc::new(StaticLlmProvider), + cheap_llm: None, + safety: Arc::new(ironclaw_safety::SafetyLayer::new( + &ironclaw_safety::SafetyConfig { + max_output_length: 100_000, + injection_check_enabled: false, + }, + )), + tools: Arc::new(crate::tools::ToolRegistry::new()), + workspace: None, + extension_manager: None, + skill_registry: None, + skill_catalog: None, + skills_config: crate::config::SkillsConfig::default(), + hooks: Arc::new(crate::hooks::HookRegistry::new()), + auth_manager: None, + cost_guard: Arc::new(crate::agent::cost_guard::CostGuard::new( + crate::agent::cost_guard::CostGuardConfig::default(), + )), + sse_tx: None, + http_interceptor: None, + transcription: None, + document_extraction: None, + sandbox_readiness: crate::agent::routine_engine::SandboxReadiness::DisabledByConfig, + builder: None, + llm_backend: "nearai".to_string(), + tenant_rates: Arc::new(crate::tenant::TenantRateRegistry::new(4, 3)), + }; + let agent = Agent::new( + crate::config::AgentConfig { + name: "title-source-regression".to_string(), + max_parallel_jobs: 1, + job_timeout: Duration::from_secs(60), + stuck_threshold: Duration::from_secs(60), + repair_check_interval: Duration::from_secs(30), + max_repair_attempts: 1, + use_planning: false, + session_idle_timeout: Duration::from_secs(300), + allow_local_tools: false, + max_cost_per_day_cents: None, + max_actions_per_hour: None, + max_cost_per_user_per_day_cents: None, + max_tool_iterations: 5, + auto_approve_tools: false, + default_timezone: "UTC".to_string(), + max_jobs_per_user: None, + max_tokens_per_job: 0, + multi_tenant: false, + max_llm_concurrent_per_user: None, + max_jobs_concurrent_per_user: None, + engine_v2: false, + }, + deps, + channels, + None, + None, + None, + Some(Arc::new(crate::context::ContextManager::new(1))), + None, + ); + + // --- Case 1: attachment-augmented payload, real user text. + // Title must come from the raw text, not the augmented block. + let thread_a = Uuid::new_v4(); + store + .ensure_conversation(thread_a, "web", "user-1", None, Some("web")) + .await + .expect("ensure"); + let raw_a = "Summarise the attached report"; + let augmented_a = format!( + "{raw_a}\n\n\nQ3 revenue was $4.2M across\nthree product lines and margin compressed to 38%.\n\n" + ); + agent + .persist_user_message( + thread_a, + "web", + "user-1", + 1, + &augmented_a, + raw_a, + Utc::now(), + ) + .await + .expect("augmented-with-raw persist succeeds"); + let meta_a = store + .get_conversation_metadata(thread_a) + .await + .expect("meta") + .expect("exists"); + assert_eq!( + meta_a.get("title").and_then(|v| v.as_str()), + Some(raw_a), + "title must be derived from raw user text, not the augmented payload \ + (augmented_a would contain the block)" + ); + + // --- Case 2: attachment-only first turn (empty raw text). + // Title must remain unset so a later real-text turn can claim it. + let thread_b = Uuid::new_v4(); + store + .ensure_conversation(thread_b, "web", "user-2", None, Some("web")) + .await + .expect("ensure"); + let augmented_b = "\n[binary image]\n"; + agent + .persist_user_message(thread_b, "web", "user-2", 1, augmented_b, "", Utc::now()) + .await + .expect("attachment-only persist succeeds"); + let meta_b = store + .get_conversation_metadata(thread_b) + .await + .expect("meta") + .expect("exists"); + assert!( + meta_b + .get("title") + .and_then(|v| v.as_str()) + .map(|s| s.is_empty()) + .unwrap_or(true), + "attachment-only first turn must NOT claim the title slot, got {:?}", + meta_b.get("title") + ); + + // --- Case 3: plain-text first turn (no attachments). + // Raw == augmented; title should be set to that text. + let thread_c = Uuid::new_v4(); + store + .ensure_conversation(thread_c, "web", "user-3", None, Some("web")) + .await + .expect("ensure"); + let plain = "what's the weather in Paris today?"; + agent + .persist_user_message(thread_c, "web", "user-3", 1, plain, plain, Utc::now()) + .await + .expect("plain persist succeeds"); + let meta_c = store + .get_conversation_metadata(thread_c) + .await + .expect("meta") + .expect("exists"); + assert_eq!( + meta_c.get("title").and_then(|v| v.as_str()), + Some(plain), + "plain-text first turn should seed the title" + ); + } + /// Regression test for #1487: process_approval on a missing thread should error. #[tokio::test] async fn test_approval_on_missing_thread_should_error() { diff --git a/src/bridge/router.rs b/src/bridge/router.rs index 0d2c0264243..ef2153995cc 100644 --- a/src/bridge/router.rs +++ b/src/bridge/router.rs @@ -3648,10 +3648,27 @@ async fn handle_with_engine_inner( .ok() }; if let Some(cid) = v1_conv_id { - let _ = db + // Only set the sidebar title when the first user message + // actually persists — otherwise the sidebar can show a + // metadata title for a conversation whose first user row + // never landed (ghost-title regression). Mirrors the gating + // in `thread_ops::persist_user_message`. + // + // Title is derived from the raw user `content`, not the + // attachment-augmented `effective_content`, so an + // image-only or attachment-only first turn does not claim + // the title slot with the synthesized `` block + // or extracted attachment text. `set_title_if_missing` + // already skips empty/whitespace input, so an attachment- + // only turn simply leaves the title unset until a later + // turn carries real user text. + if db .add_conversation_message(cid, "user", effective_content) - .await; - crate::db::set_title_if_missing(db.as_ref(), cid, effective_content).await; + .await + .is_ok() + { + crate::db::set_title_if_missing(db.as_ref(), cid, content).await; + } } } diff --git a/src/history/store.rs b/src/history/store.rs index 1f9a4aa89bb..731c5d0c019 100644 --- a/src/history/store.rs +++ b/src/history/store.rs @@ -3414,4 +3414,143 @@ mod tests { .await .unwrap(); } + + /// PG mirror of `src/db/libsql/conversations.rs` + /// `test_metadata_title_used_as_fallback`. + /// + /// Regression for #2237: when no user messages have been persisted, + /// `list_conversations_all_channels` must fall back to the + /// `metadata.title` field that `set_title_if_missing` writes. The + /// libSQL test pins this for the embedded path; this mirror pins + /// the PostgreSQL path so a regression in the PG SQL (e.g. JSON + /// extraction operator drift, precedence error in the `COALESCE` / + /// `UNION` rewrite) surfaces at the integration tier instead of in + /// production. + /// + /// Integration tier — ignored by default. Requires a reachable + /// PostgreSQL with migrations applied. Run with: + /// + /// ```text + /// cargo test -p ironclaw --features integration --lib \ + /// history::store::tests::test_metadata_title_used_as_fallback_pg -- --ignored + /// ``` + #[cfg(feature = "postgres")] + #[tokio::test] + #[ignore] + async fn test_metadata_title_used_as_fallback_pg() { + use crate::config::Config; + + let _ = dotenvy::dotenv(); + let config = Config::from_env().await.expect("Failed to load config"); + let store = Store::new(&config.database) + .await + .expect("Failed to connect to database"); + store + .run_migrations() + .await + .expect("Failed to run migrations"); + + let conv_id = Uuid::new_v4(); + let user_id = format!("pg-title-fallback-{}", Uuid::new_v4()); + + store + .ensure_conversation(conv_id, "gateway", &user_id, None, Some("gateway")) + .await + .unwrap(); + + let title_val = serde_json::json!("What is the weather today?"); + store + .update_conversation_metadata_field(conv_id, "title", &title_val) + .await + .unwrap(); + + let convs = store + .list_conversations_all_channels(&user_id, 50) + .await + .unwrap(); + let conv = convs.iter().find(|c| c.id == conv_id).expect("conv listed"); + assert_eq!( + conv.title.as_deref(), + Some("What is the weather today?"), + "PG list must fall back to metadata.title when no user messages exist" + ); + + let conn = store.conn().await.unwrap(); + conn.execute( + "DELETE FROM conversations WHERE id = $1", + &[&conv_id.to_string()], + ) + .await + .unwrap(); + } + + /// PG mirror of `src/db/libsql/conversations.rs` + /// `test_message_title_takes_precedence_over_metadata`. + /// + /// Pins the precedence order: once a first user message is + /// persisted, the derived title (from the message body) takes over + /// and the `metadata.title` set at conversation creation is + /// shadowed. Protects against a regression where the PG SQL + /// accidentally prefers `metadata.title` even when a message row + /// exists. + #[cfg(feature = "postgres")] + #[tokio::test] + #[ignore] + async fn test_message_title_takes_precedence_over_metadata_pg() { + use crate::config::Config; + + let _ = dotenvy::dotenv(); + let config = Config::from_env().await.expect("Failed to load config"); + let store = Store::new(&config.database) + .await + .expect("Failed to connect to database"); + store + .run_migrations() + .await + .expect("Failed to run migrations"); + + let conv_id = Uuid::new_v4(); + let user_id = format!("pg-title-precedence-{}", Uuid::new_v4()); + + store + .ensure_conversation(conv_id, "gateway", &user_id, None, Some("gateway")) + .await + .unwrap(); + + let title_val = serde_json::json!("metadata title"); + store + .update_conversation_metadata_field(conv_id, "title", &title_val) + .await + .unwrap(); + + store + .add_conversation_message(conv_id, "user", "actual user message") + .await + .unwrap(); + + let convs = store + .list_conversations_all_channels(&user_id, 50) + .await + .unwrap(); + let conv = convs.iter().find(|c| c.id == conv_id).expect("conv listed"); + assert_eq!( + conv.title.as_deref(), + Some("actual user message"), + "first user message must shadow metadata.title on the PG path" + ); + + let conn = store.conn().await.unwrap(); + conn.execute( + "DELETE FROM conversation_messages WHERE conversation_id = $1", + &[&conv_id.to_string()], + ) + .await + .unwrap(); + conn.execute( + "DELETE FROM conversations WHERE id = $1", + &[&conv_id.to_string()], + ) + .await + .unwrap(); + } } From 27669084547fabdb820f345744aeac60ab04a1bd Mon Sep 17 00:00:00 2001 From: Zaki Date: Thu, 23 Apr 2026 17:52:12 -0400 Subject: [PATCH 09/11] fix(gateway): derive titles from raw content on all paths and make title write atomic Addresses three unresolved review comments on #2700: 1. In-memory threads fallback (src/channels/web/features/chat/mod.rs). The no-DB path in `chat_threads_handler` still derived its sidebar title from `turn.user_input`, which is the attachment-augmented payload stamped by `process_user_input`. Adds a new `raw_user_input: Option` field to `Turn`, populated when augmentation actually changes the text, and a shared `title_from_in_memory_turn` helper that prefers the raw text. Plain turns leave the field `None` and fall through to `user_input` cleanly. 2. Engine-v2 path (src/bridge/router.rs + crates/ironclaw_engine/). `ConversationManager::handle_user_message` now takes an explicit `raw_content_for_title: Option<&str>` parameter. The LLM-facing `content` stays as the attachment-augmented payload; the raw text is stored separately on the `ConversationEntry.metadata` as `title_source` (via new `ConversationEntry::user_with_title_source`) so downstream title consumers see the user-typed text, not the synthesized `` block. The router passes `content` (raw) as the title source alongside `effective_content`. 3. Atomic title write (src/db/mod.rs + libSQL + PG). Replaces the check-then-update pair in `set_title_if_missing` with a conditional single-statement UPDATE on both backends. libSQL uses `json_extract(metadata, '$.title') IS NULL OR = ''`; PG uses `NOT (metadata ? 'title') OR metadata->>'title' = ''`. Adds `ConversationStore::set_conversation_title_if_empty` returning whether the write landed, so concurrent first-turn callers cannot race to overwrite each other. Tests: - `test_set_title_if_empty_is_atomic_under_concurrency` (libSQL) races two `tokio::join!`'d calls and asserts exactly one wins. - `test_set_title_if_empty_writes_when_unset` covers NULL metadata and empty-string title rows. - `test_set_title_if_empty_is_atomic_pg` mirrors the race on PG (integration tier, `#[ignore]`). - `handle_user_message_records_raw_title_source` and `handle_user_message_plain_text_has_no_title_source_metadata` pin the engine-v2 plumbing. - `test_title_from_in_memory_turn_*` (3 cases) cover the fallback helper across augmented, plain, and empty-raw turns. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../src/runtime/conversation.rs | 109 +++++++++++++- .../ironclaw_engine/src/types/conversation.rs | 26 ++++ src/agent/session.rs | 11 +- src/agent/thread_ops.rs | 9 ++ src/bridge/router.rs | 6 + src/channels/web/features/chat/mod.rs | 87 +++++++++-- src/db/libsql/conversations.rs | 135 ++++++++++++++++++ src/db/mod.rs | 52 ++++--- src/db/postgres.rs | 8 ++ src/history/store.rs | 90 ++++++++++++ src/hooks/session_summary.rs | 8 ++ 11 files changed, 507 insertions(+), 34 deletions(-) diff --git a/crates/ironclaw_engine/src/runtime/conversation.rs b/crates/ironclaw_engine/src/runtime/conversation.rs index fd5f464b13d..b5b07bf0fee 100644 --- a/crates/ironclaw_engine/src/runtime/conversation.rs +++ b/crates/ironclaw_engine/src/runtime/conversation.rs @@ -189,6 +189,7 @@ impl ConversationManager { /// The per-conversation `Mutex` is held for the entire operation — from /// the active-thread check through `save_conversation`. This eliminates /// the TOCTOU double-spawn window present in the old 5-phase split. + #[allow(clippy::too_many_arguments)] pub async fn handle_user_message( &self, conversation_id: ConversationId, @@ -197,6 +198,7 @@ impl ConversationManager { user_id: &str, thread_config: ThreadConfig, user_timezone: Option<&str>, + raw_content_for_title: Option<&str>, ) -> Result { let conv_arc = self.get_conversation_lock(conversation_id).await?; let mut conv = conv_arc.lock().await; @@ -324,7 +326,16 @@ impl ConversationManager { // The user entry is added here — after the thread operation succeeded — to // prevent orphaned entries if inject_message/resume_thread/spawn_thread_with_history // returned an error above. - conv.add_entry(ConversationEntry::user(content)); + // Record the user entry with a separate `title_source` so downstream + // consumers deriving a conversation title use the raw user text + // instead of the attachment-augmented payload. `content` is still + // the LLM-facing string (augmented with `` / extracted + // OCR); only the title-derivation surface changes. + let entry = match raw_content_for_title { + Some(raw) if raw != content => ConversationEntry::user_with_title_source(content, raw), + _ => ConversationEntry::user(content), + }; + conv.add_entry(entry); match active_foreground { Some(ActiveForeground::Running(_)) => { // No additional in-memory mutation needed beyond the user entry above. @@ -847,6 +858,7 @@ mod tests { "user1", ThreadConfig::default(), None, + None, ) .await .unwrap(); @@ -915,6 +927,7 @@ mod tests { "user1", ThreadConfig::default(), None, + None, ) .await .unwrap(); @@ -1014,6 +1027,7 @@ mod tests { "user1", ThreadConfig::default(), None, + None, ) .await .unwrap(); @@ -1063,6 +1077,7 @@ mod tests { "user1", ThreadConfig::default(), None, + None, ) .await }); @@ -1074,6 +1089,7 @@ mod tests { "user1", ThreadConfig::default(), None, + None, ) .await }); @@ -1150,6 +1166,7 @@ mod tests { "user1", ThreadConfig::default(), None, + None, ) .await .unwrap(); @@ -1163,6 +1180,96 @@ mod tests { ); } + /// Engine-v2 `handle_user_message` must preserve the raw user text + /// on the `ConversationEntry` as `title_source` metadata when the + /// caller supplies attachment-augmented content. This closes the + /// Issue 2 regression where the sidebar / downstream title derivation + /// would see the synthesized `` block rather than the + /// user-typed text. + #[tokio::test] + async fn handle_user_message_records_raw_title_source() { + let (_tm, cm) = make_conv_manager(); + let conv_id = cm + .get_or_create_conversation("web", "user-raw") + .await + .unwrap(); + let project = ProjectId::new(); + + let raw = "Summarise the attached doc"; + let augmented = "Summarise the attached doc\n\nQ3 rev $4.2M\n"; + + let _tid = cm + .handle_user_message( + conv_id, + augmented, + project, + "user-raw", + ThreadConfig::default(), + None, + Some(raw), + ) + .await + .unwrap(); + + let conv = cm.get_conversation(conv_id).await.unwrap(); + let user_entry = conv + .entries + .iter() + .find(|e| matches!(e.sender, crate::types::conversation::EntrySender::User)) + .expect("user entry present"); + // LLM-facing content stays augmented. + assert_eq!(user_entry.content, augmented); + // Title derivation surface is the raw user text. + let title_source = user_entry + .metadata + .get("title_source") + .and_then(|v| v.as_str()); + assert_eq!( + title_source, + Some(raw), + "title_source must be the raw user text, not the augmented payload" + ); + } + + /// Plain-text turns (no augmentation) must NOT stamp a redundant + /// `title_source` — `content` already is the title source. + #[tokio::test] + async fn handle_user_message_plain_text_has_no_title_source_metadata() { + let (_tm, cm) = make_conv_manager(); + let conv_id = cm + .get_or_create_conversation("web", "user-plain") + .await + .unwrap(); + let project = ProjectId::new(); + + let plain = "what's the weather"; + let _tid = cm + .handle_user_message( + conv_id, + plain, + project, + "user-plain", + ThreadConfig::default(), + None, + Some(plain), + ) + .await + .unwrap(); + + let conv = cm.get_conversation(conv_id).await.unwrap(); + let user_entry = conv + .entries + .iter() + .find(|e| matches!(e.sender, crate::types::conversation::EntrySender::User)) + .expect("user entry"); + assert_eq!(user_entry.content, plain); + assert!( + user_entry.metadata.is_null(), + "plain-text turn must leave metadata unset, got {:?}", + user_entry.metadata + ); + } + #[tokio::test] async fn record_external_agent_message_rejects_wrong_user() { let (_, cm) = make_conv_manager(); diff --git a/crates/ironclaw_engine/src/types/conversation.rs b/crates/ironclaw_engine/src/types/conversation.rs index 7a16ecee170..1b1a2c42fb8 100644 --- a/crates/ironclaw_engine/src/types/conversation.rs +++ b/crates/ironclaw_engine/src/types/conversation.rs @@ -86,6 +86,32 @@ impl ConversationEntry { } } + /// Create a user entry with a separate `title_source` — the raw + /// user-typed text before any attachment augmentation. `content` + /// remains the LLM-facing payload (augmented with attachment blocks / + /// extracted text); `title_source` is recorded in `metadata` so that + /// downstream consumers deriving a sidebar title use the raw text + /// rather than the synthesized attachment block. + pub fn user_with_title_source( + content: impl Into, + title_source: impl Into, + ) -> Self { + let title_source = title_source.into(); + let metadata = if title_source.is_empty() { + serde_json::Value::Null + } else { + serde_json::json!({ "title_source": title_source }) + }; + Self { + id: EntryId::new(), + sender: EntrySender::User, + content: content.into(), + origin_thread_id: None, + timestamp: Utc::now(), + metadata, + } + } + /// Create an agent entry from a thread. pub fn agent(thread_id: ThreadId, content: impl Into) -> Self { Self { diff --git a/src/agent/session.rs b/src/agent/session.rs index 2e6c92ec00b..ed84417e1d6 100644 --- a/src/agent/session.rs +++ b/src/agent/session.rs @@ -717,8 +717,16 @@ pub struct Turn { /// Persisted user message ID when this turn has been written to the DB. #[serde(default, skip_serializing_if = "Option::is_none")] pub user_message_id: Option, - /// User input that started this turn. + /// User input that started this turn. On the v1 attachment pipeline + /// this is the *augmented* payload (raw text + synthesized + /// `` block) that is fed to the LLM. pub user_input: String, + /// Raw user-typed text for the turn, before attachment augmentation. + /// Used for sidebar / title derivation so that attachment-only or + /// augmented turns do not surface the synthesized attachment block as + /// the conversation title. `None` for turns that predate this field. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub raw_user_input: Option, /// Agent response (if completed). pub response: Option, /// Tool calls made during this turn. @@ -749,6 +757,7 @@ impl Turn { turn_number, user_message_id: None, user_input: user_input.into(), + raw_user_input: None, response: None, tool_calls: Vec::new(), state: TurnState::Processing, diff --git a/src/agent/thread_ops.rs b/src/agent/thread_ops.rs index 3adaaa39750..6df32a56409 100644 --- a/src/agent/thread_ops.rs +++ b/src/agent/thread_ops.rs @@ -666,6 +666,15 @@ impl Agent { .ok_or_else(|| Error::from(crate::error::JobError::NotFound { id: thread_id }))?; let turn = thread.start_turn(effective_content); turn.image_content_parts = image_parts; + // Preserve the raw user text alongside the augmented payload so + // the no-DB / in-memory fallback in the threads endpoint can + // derive the sidebar title from the raw text instead of the + // synthesized `` block. Only set when it differs + // from `user_input` — plain-text turns keep it `None` since + // `user_input` is already correct for title derivation. + if content != effective_content { + turn.raw_user_input = Some(content.to_string()); + } let turn_number = turn.turn_number; let turn_started_at = turn.started_at; (thread.messages(), turn_number, turn_started_at) diff --git a/src/bridge/router.rs b/src/bridge/router.rs index ef2153995cc..561504fed49 100644 --- a/src/bridge/router.rs +++ b/src/bridge/router.rs @@ -3609,6 +3609,12 @@ async fn handle_with_engine_inner( &message.user_id, thread_config, validated_tz.as_ref().map(|tz| tz.name()), + // Raw content for title derivation. `effective_content` is the + // attachment-augmented payload (synthesized `` + // block / extracted OCR); the sidebar title must come from the + // raw user text, mirroring the v1 path in + // `thread_ops::persist_user_message`. + Some(content), ) .await .map_err(|e| engine_err("thread error", e))?; diff --git a/src/channels/web/features/chat/mod.rs b/src/channels/web/features/chat/mod.rs index d2e07e40abf..81802baf7ad 100644 --- a/src/channels/web/features/chat/mod.rs +++ b/src/channels/web/features/chat/mod.rs @@ -753,19 +753,13 @@ pub(crate) async fn chat_threads_handler( let threads: Vec = sorted_threads .into_iter() .map(|t| { + // Derive the sidebar title from the raw user text when + // available. See `title_from_in_memory_turn` for rationale. let title = t .turns .first() - .map(|turn| { - turn.user_input - .split_whitespace() - .collect::>() - .join(" ") - .chars() - .take(100) - .collect::() - }) - .filter(|s| !s.is_empty()); + .and_then(title_from_in_memory_turn) + .filter(|s: &String| !s.is_empty()); ThreadInfo { id: t.id, state: thread_state_label(t.state).to_string(), @@ -1094,6 +1088,34 @@ fn in_progress_from_thread(thread: &crate::agent::session::Thread) -> Option` block as the conversation title. `user_input` is the +/// attachment-augmented payload fed to the LLM — only plain-text turns +/// can safely use it as the title source. Mirrors the title-derivation +/// rules used by the v1 DB path (`persist_user_message` / +/// `set_title_if_missing`). +fn title_from_in_memory_turn(turn: &crate::agent::session::Turn) -> Option { + let source = turn + .raw_user_input + .as_deref() + .unwrap_or(turn.user_input.as_str()); + let trimmed = source.trim(); + if trimmed.is_empty() { + return None; + } + let title: String = trimmed + .split_whitespace() + .collect::>() + .join(" ") + .chars() + .take(100) + .collect(); + Some(title) +} + fn thread_state_label(state: crate::agent::session::ThreadState) -> &'static str { match state { crate::agent::session::ThreadState::Idle => "Idle", @@ -1275,6 +1297,51 @@ mod tests { assert!(message.is_none()); } + #[test] + fn test_title_from_in_memory_turn_prefers_raw_input() { + // When raw_user_input is populated (attachment-augmented turn), + // the title must come from the raw text — not the augmented + // `user_input` that contains the synthesized `` + // block. Regression for Issue 1 of PR #2700. + let mut thread = crate::agent::session::Thread::new(Uuid::new_v4(), Some("gateway")); + thread.start_turn( + "Summarise the attached report\n\nQ3 rev $4.2M\n", + ); + { + let turn = thread.turns.last_mut().expect("turn"); + turn.raw_user_input = Some("Summarise the attached report".to_string()); + } + let title = title_from_in_memory_turn(&thread.turns[0]).expect("title"); + assert_eq!(title, "Summarise the attached report"); + } + + #[test] + fn test_title_from_in_memory_turn_falls_back_to_user_input() { + // Plain-text turns leave `raw_user_input` as None and `user_input` + // is already the raw text — the helper should use it. + let mut thread = crate::agent::session::Thread::new(Uuid::new_v4(), Some("gateway")); + thread.start_turn("what's the weather in Paris?"); + let title = title_from_in_memory_turn(&thread.turns[0]).expect("title"); + assert_eq!(title, "what's the weather in Paris?"); + } + + #[test] + fn test_title_from_in_memory_turn_none_for_empty_raw() { + // Attachment-only turn: raw_user_input == "" while user_input + // carries only the synthesized block. Returning None leaves the + // title unset so a later real-text turn can claim it, matching + // the v1 DB behaviour (`set_title_if_missing` skips empty input). + let mut thread = crate::agent::session::Thread::new(Uuid::new_v4(), Some("gateway")); + thread.start_turn( + "\n[binary]\n", + ); + { + let turn = thread.turns.last_mut().expect("turn"); + turn.raw_user_input = Some(String::new()); + } + assert!(title_from_in_memory_turn(&thread.turns[0]).is_none()); + } + #[test] fn test_engine_history_entry_uses_stable_id() { let thread_id = Uuid::new_v4(); diff --git a/src/db/libsql/conversations.rs b/src/db/libsql/conversations.rs index 339caa35e32..dc5e4f50959 100644 --- a/src/db/libsql/conversations.rs +++ b/src/db/libsql/conversations.rs @@ -608,6 +608,39 @@ impl ConversationStore for LibSqlBackend { Ok(()) } + async fn set_conversation_title_if_empty( + &self, + id: Uuid, + title: &str, + ) -> Result { + let conn = self.connect().await?; + // Atomic check-and-write: patch the metadata only when `title` is + // currently missing, NULL, or empty. Expressed as a single + // conditional UPDATE so two concurrent callers cannot both observe + // an empty title and race to overwrite each other. + // + // `json_extract(metadata, '$.title')` returns NULL when the key is + // absent, and the `IS NULL OR = ''` pair covers the "no title yet" + // cases. When metadata itself is NULL (legacy rows), we initialise + // it with `{"title": ?}` via COALESCE. + let patch = serde_json::json!({ "title": title }).to_string(); + let affected = conn + .execute( + "UPDATE conversations \ + SET metadata = json_patch(COALESCE(metadata, '{}'), ?2) \ + WHERE id = ?1 \ + AND ( \ + metadata IS NULL \ + OR json_extract(metadata, '$.title') IS NULL \ + OR json_extract(metadata, '$.title') = '' \ + )", + params![id.to_string(), patch], + ) + .await + .map_err(|e| DatabaseError::Query(e.to_string()))?; + Ok(affected > 0) + } + async fn get_conversation_metadata( &self, id: Uuid, @@ -1110,4 +1143,106 @@ mod tests { "Message-derived title should take precedence over metadata title" ); } + + /// Regression test for the check-then-update race in + /// `set_title_if_missing`. Two concurrent callers both observe an + /// empty title; only the first conditional UPDATE must land. + #[tokio::test] + async fn test_set_title_if_empty_is_atomic_under_concurrency() { + use std::sync::Arc; + + let dir = tempfile::tempdir().unwrap(); + let db_path = dir.path().join("test_title_race.db"); + let backend = Arc::new(LibSqlBackend::new_local(&db_path).await.unwrap()); + backend.run_migrations().await.unwrap(); + + let conv_id = Uuid::new_v4(); + backend + .ensure_conversation(conv_id, "gateway", "racer", None, Some("gateway")) + .await + .unwrap(); + + let b1 = Arc::clone(&backend); + let b2 = Arc::clone(&backend); + let (r1, r2) = tokio::join!( + async move { + b1.set_conversation_title_if_empty(conv_id, "first title") + .await + }, + async move { + b2.set_conversation_title_if_empty(conv_id, "second title") + .await + } + ); + let won1 = r1.unwrap(); + let won2 = r2.unwrap(); + assert!( + won1 ^ won2, + "exactly one concurrent setter must win, got ({won1}, {won2})" + ); + + let meta = backend + .get_conversation_metadata(conv_id) + .await + .unwrap() + .unwrap(); + let title = meta.get("title").and_then(|v| v.as_str()).unwrap(); + assert!( + title == "first title" || title == "second title", + "title must be one of the two candidates, got {title:?}" + ); + + // A third attempt on an already-titled conv must NOT overwrite. + let won3 = backend + .set_conversation_title_if_empty(conv_id, "third title") + .await + .unwrap(); + assert!(!won3, "third attempt must be a no-op"); + let meta = backend + .get_conversation_metadata(conv_id) + .await + .unwrap() + .unwrap(); + let title_after = meta.get("title").and_then(|v| v.as_str()).unwrap(); + assert_eq!(title, title_after, "title must not change after first win"); + } + + /// Empty / NULL metadata must still be treated as "no title yet" so + /// a fresh conversation can claim the title slot on its first call. + #[tokio::test] + async fn test_set_title_if_empty_writes_when_unset() { + let dir = tempfile::tempdir().unwrap(); + let db_path = dir.path().join("test_title_unset.db"); + let backend = LibSqlBackend::new_local(&db_path).await.unwrap(); + backend.run_migrations().await.unwrap(); + + let conv_id = Uuid::new_v4(); + backend + .ensure_conversation(conv_id, "gateway", "u", None, Some("gateway")) + .await + .unwrap(); + + let won = backend + .set_conversation_title_if_empty(conv_id, "hello") + .await + .unwrap(); + assert!(won); + + // Pre-populate with empty-string title (regression for the + // `= ''` branch of the WHERE clause). + let conv2 = Uuid::new_v4(); + backend + .ensure_conversation(conv2, "gateway", "u", None, Some("gateway")) + .await + .unwrap(); + backend + .update_conversation_metadata_field(conv2, "title", &serde_json::json!("")) + .await + .unwrap(); + let won_empty = backend + .set_conversation_title_if_empty(conv2, "real") + .await + .unwrap(); + assert!(won_empty, "empty-string title must be treated as unset"); + } } diff --git a/src/db/mod.rs b/src/db/mod.rs index ee6abce5350..5f1cd9340ef 100644 --- a/src/db/mod.rs +++ b/src/db/mod.rs @@ -478,6 +478,18 @@ pub trait ConversationStore: Send + Sync { &self, id: Uuid, ) -> Result, DatabaseError>; + /// Atomically set `metadata.title` only if it is currently missing or empty. + /// + /// Returns `true` if this call actually wrote the title, `false` if the + /// title was already set (or the conversation does not exist). Used to + /// close the check-then-update race in [`set_title_if_missing`] where + /// two concurrent writes could both observe an empty title and race to + /// write different values. + async fn set_conversation_title_if_empty( + &self, + id: Uuid, + title: &str, + ) -> Result; async fn list_conversation_messages( &self, conversation_id: Uuid, @@ -509,30 +521,26 @@ pub async fn set_title_if_missing( return; } - let has_title = match store.get_conversation_metadata(conversation_id).await { - Ok(Some(meta)) => meta - .get("title") - .and_then(|t| t.as_str()) - .is_some_and(|s| !s.is_empty()), - Ok(None) => return, + let title_text: String = trimmed + .split_whitespace() + .collect::>() + .join(" ") + .chars() + .take(100) + .collect(); + + // Atomic conditional update. Both backends only write when + // `metadata.title` is NULL, missing, or an empty string, so two + // concurrent first-turn writes cannot race to overwrite each other — + // the first to commit wins, subsequent calls are no-ops. + match store + .set_conversation_title_if_empty(conversation_id, &title_text) + .await + { + Ok(_) => {} Err(e) => { - tracing::debug!("failed to read metadata for title-set: {e}"); - return; + tracing::debug!("failed to atomically set title: {e}"); } - }; - - if !has_title { - let title_text: String = trimmed - .split_whitespace() - .collect::>() - .join(" ") - .chars() - .take(100) - .collect(); - let title_val = serde_json::json!(title_text); - let _ = store - .update_conversation_metadata_field(conversation_id, "title", &title_val) - .await; } } diff --git a/src/db/postgres.rs b/src/db/postgres.rs index fda565eeb3c..6a21a5ce3c0 100644 --- a/src/db/postgres.rs +++ b/src/db/postgres.rs @@ -274,6 +274,14 @@ impl ConversationStore for PgBackend { self.store.get_conversation_metadata(id).await } + async fn set_conversation_title_if_empty( + &self, + id: Uuid, + title: &str, + ) -> Result { + self.store.set_conversation_title_if_empty(id, title).await + } + async fn list_conversation_messages( &self, conversation_id: Uuid, diff --git a/src/history/store.rs b/src/history/store.rs index 731c5d0c019..0a4f167e505 100644 --- a/src/history/store.rs +++ b/src/history/store.rs @@ -2250,6 +2250,37 @@ impl Store { Ok(()) } + /// Atomically set `metadata.title` only when it's missing or empty. + /// + /// Returns `true` if the write actually landed (this caller "won" the + /// race), `false` if the title was already set or the conversation does + /// not exist. Expressed as a single conditional UPDATE so two concurrent + /// first-turn writes cannot both observe an empty title and race to + /// overwrite each other. + pub async fn set_conversation_title_if_empty( + &self, + id: Uuid, + title: &str, + ) -> Result { + let conn = self.conn().await?; + let patch = serde_json::json!({ "title": title }); + let affected = conn + .execute( + "UPDATE conversations \ + SET metadata = COALESCE(metadata, '{}'::jsonb) || $2 \ + WHERE id = $1 \ + AND ( \ + metadata IS NULL \ + OR NOT (metadata ? 'title') \ + OR metadata->>'title' IS NULL \ + OR metadata->>'title' = '' \ + )", + &[&id, &patch], + ) + .await?; + Ok(affected > 0) + } + /// Read the metadata JSONB for a conversation. pub async fn get_conversation_metadata( &self, @@ -3553,4 +3584,63 @@ mod tests { .await .unwrap(); } + + /// PG regression for the atomic title write. + /// + /// Two concurrent callers both observe an empty title; exactly one + /// conditional UPDATE must land. Subsequent calls on an + /// already-titled conversation must be no-ops. Integration tier — + /// ignored by default. Run with: + /// + /// ```text + /// cargo test -p ironclaw --features integration --lib \ + /// history::store::tests::test_set_title_if_empty_is_atomic_pg -- --ignored + /// ``` + #[cfg(feature = "postgres")] + #[tokio::test] + #[ignore] + async fn test_set_title_if_empty_is_atomic_pg() { + use crate::config::Config; + use std::sync::Arc; + + let _ = dotenvy::dotenv(); + let config = Config::from_env().await.expect("load config"); + let store = Arc::new(Store::new(&config.database).await.expect("connect")); + store.run_migrations().await.expect("migrate"); + + let conv_id = Uuid::new_v4(); + let user_id = format!("pg-title-race-{}", Uuid::new_v4()); + store + .ensure_conversation(conv_id, "gateway", &user_id, None, Some("gateway")) + .await + .unwrap(); + + let s1 = Arc::clone(&store); + let s2 = Arc::clone(&store); + let (r1, r2) = tokio::join!( + async move { s1.set_conversation_title_if_empty(conv_id, "first").await }, + async move { s2.set_conversation_title_if_empty(conv_id, "second").await } + ); + let won1 = r1.unwrap(); + let won2 = r2.unwrap(); + assert!( + won1 ^ won2, + "exactly one concurrent setter must win, got ({won1}, {won2})" + ); + + // Third attempt on titled conv must be a no-op. + let won3 = store + .set_conversation_title_if_empty(conv_id, "third") + .await + .unwrap(); + assert!(!won3, "must not overwrite an existing title"); + + let conn = store.conn().await.unwrap(); + conn.execute( + "DELETE FROM conversations WHERE id = $1", + &[&conv_id.to_string()], + ) + .await + .unwrap(); + } } diff --git a/src/hooks/session_summary.rs b/src/hooks/session_summary.rs index 5c7504be7b7..3d143421aca 100644 --- a/src/hooks/session_summary.rs +++ b/src/hooks/session_summary.rs @@ -350,6 +350,14 @@ mod tests { unimplemented!() } + async fn set_conversation_title_if_empty( + &self, + _id: Uuid, + _title: &str, + ) -> Result { + unimplemented!() + } + async fn list_conversation_messages( &self, _conversation_id: Uuid, From feec0c0e70e8ad41e24b70a9b6bfc1a2c968a635 Mon Sep 17 00:00:00 2001 From: Zaki Date: Sat, 2 May 2026 13:04:10 -0700 Subject: [PATCH 10/11] fix(title): keep engine thread goals display-safe --- .../src/runtime/conversation.rs | 91 ++++++++++++++++++- crates/ironclaw_engine/src/runtime/manager.rs | 43 ++++++++- 2 files changed, 129 insertions(+), 5 deletions(-) diff --git a/crates/ironclaw_engine/src/runtime/conversation.rs b/crates/ironclaw_engine/src/runtime/conversation.rs index b5b07bf0fee..0f2cde1c9e5 100644 --- a/crates/ironclaw_engine/src/runtime/conversation.rs +++ b/crates/ironclaw_engine/src/runtime/conversation.rs @@ -306,10 +306,15 @@ impl ConversationManager { ); } - // Spawn new foreground thread with conversation history. + // Spawn new foreground thread with conversation history. Keep + // the thread goal/display source raw while preserving the + // attachment-augmented payload as the LLM-facing message. + let (thread_goal, current_user_message) = + thread_goal_and_current_message(content, raw_content_for_title); self.thread_manager - .spawn_thread_with_history( - content, // use message as goal + .spawn_thread_with_history_and_current_message( + thread_goal, + current_user_message, ThreadType::Foreground, project_id, thread_config, @@ -574,6 +579,28 @@ fn build_history_from_entries( .collect() } +fn thread_goal_and_current_message( + content: &str, + raw_content_for_title: Option<&str>, +) -> (String, Option) { + let Some(raw) = raw_content_for_title else { + return (content.to_string(), None); + }; + + let display_goal = raw.split_whitespace().collect::>().join(" "); + let display_goal = if display_goal.is_empty() { + "Untitled chat".to_string() + } else { + display_goal + }; + let current_user_message = if display_goal == content { + None + } else { + Some(content.to_string()) + }; + (display_goal, current_user_message) +} + #[cfg(test)] mod tests { use super::*; @@ -1231,6 +1258,64 @@ mod tests { ); } + /// The engine thread's user-visible goal must also be derived from raw + /// user text, while the LLM-facing first message keeps the augmented + /// payload. Otherwise thread-list/detail surfaces that expose + /// `thread.goal` still leak `` blocks even though the + /// conversation-entry title_source is correct. + #[tokio::test] + async fn handle_user_message_uses_raw_title_source_as_thread_goal() { + let store = Arc::new(MockStore::new()); + let tm = Arc::new(ThreadManager::new( + Arc::new(MockLlm(Mutex::new(vec![LlmOutput { + response: LlmResponse::Text("Hello!".into()), + usage: TokenUsage::default(), + }]))), + Arc::new(MockEffects), + store.clone(), + Arc::new(CapabilityRegistry::new()), + Arc::new(LeaseManager::new()), + Arc::new(PolicyEngine::new()), + )); + let cm = ConversationManager::new(Arc::clone(&tm), store.clone()); + let conv_id = cm + .get_or_create_conversation("web", "user-goal") + .await + .unwrap(); + let project = ProjectId::new(); + + let raw = "Summarise the attached doc"; + let augmented = "Summarise the attached doc\n\nQ3 rev $4.2M\n"; + + let tid = cm + .handle_user_message( + conv_id, + augmented, + project, + "user-goal", + ThreadConfig::default(), + None, + Some(raw), + ) + .await + .unwrap(); + + let thread = store.load_thread(tid).await.unwrap().expect("thread"); + assert_eq!( + thread.goal, raw, + "thread goal should be display-safe raw text" + ); + let first_user_message = thread + .messages + .iter() + .find(|m| m.role == MessageRole::User) + .expect("first user message"); + assert_eq!( + first_user_message.content, augmented, + "LLM-facing message must keep attachment context" + ); + } + /// Plain-text turns (no augmentation) must NOT stamp a redundant /// `title_source` — `content` already is the title source. #[tokio::test] diff --git a/crates/ironclaw_engine/src/runtime/manager.rs b/crates/ironclaw_engine/src/runtime/manager.rs index f4865765dc1..232a6d53acb 100644 --- a/crates/ironclaw_engine/src/runtime/manager.rs +++ b/crates/ironclaw_engine/src/runtime/manager.rs @@ -129,8 +129,43 @@ impl ThreadManager { user_id: impl Into, initial_messages: Vec, initial_metadata: serde_json::Map, + ) -> Result { + self.spawn_thread_with_history_and_current_message( + goal, + None, + thread_type, + project_id, + config, + parent_id, + user_id, + initial_messages, + initial_metadata, + ) + .await + } + + /// Spawn a thread with initial conversation history and, optionally, a + /// current user message distinct from the display goal. + /// + /// The separate `current_user_message` is for channel pipelines that augment + /// the LLM-facing message with attachment/OCR blocks while keeping + /// user-visible thread surfaces such as `Thread.goal` derived from raw text. + #[allow(clippy::too_many_arguments)] + pub async fn spawn_thread_with_history_and_current_message( + &self, + goal: impl Into, + current_user_message: Option, + thread_type: ThreadType, + project_id: ProjectId, + config: ThreadConfig, + parent_id: Option, + user_id: impl Into, + initial_messages: Vec, + initial_metadata: serde_json::Map, ) -> Result { let user_id = user_id.into(); + let goal = goal.into(); + let current_user_message = current_user_message.unwrap_or_else(|| goal.clone()); let mut thread = Thread::new(goal, thread_type, project_id, &user_id, config); if let Some(pid) = parent_id { thread = thread.with_parent(pid); @@ -176,8 +211,12 @@ impl ThreadManager { thread.messages.push(msg); } - // Add the goal as the current user message so the LLM has context - thread.add_message(crate::types::message::ThreadMessage::user(&thread.goal)); + // Add the current user message so the LLM has context. For ordinary + // callers this is the goal; attachment-aware callers can pass the + // augmented payload here while keeping `thread.goal` display-safe. + thread.add_message(crate::types::message::ThreadMessage::user( + current_user_message, + )); // Persist self.store.save_thread(&thread).await?; From ee5133a9530ea3fb195f98e81bd783313d232fae Mon Sep 17 00:00:00 2001 From: Zaki Date: Sun, 17 May 2026 06:11:11 -0700 Subject: [PATCH 11/11] test(engine): pass extra_initial_metadata None in handle_user_message test calls The post-rebase merge with main introduced an 8th `extra_initial_metadata: Option>` parameter to `ConversationManager::handle_user_message`. The tests in `crates/ironclaw_engine/src/runtime/conversation.rs` still called the method with 7 args, causing `cargo clippy --tests` (and Clippy (all-features) CI) to fail with E0061. Pass `None` as the new argument at every test call site. --- crates/ironclaw_engine/src/runtime/conversation.rs | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/crates/ironclaw_engine/src/runtime/conversation.rs b/crates/ironclaw_engine/src/runtime/conversation.rs index a585ce20d37..0cbbe9df90b 100644 --- a/crates/ironclaw_engine/src/runtime/conversation.rs +++ b/crates/ironclaw_engine/src/runtime/conversation.rs @@ -930,6 +930,7 @@ mod tests { ThreadConfig::default(), None, None, + None, ) .await .unwrap(); @@ -999,6 +1000,7 @@ mod tests { ThreadConfig::default(), None, None, + None, ) .await .unwrap(); @@ -1099,6 +1101,7 @@ mod tests { ThreadConfig::default(), None, None, + None, ) .await .unwrap(); @@ -1149,6 +1152,7 @@ mod tests { ThreadConfig::default(), None, None, + None, ) .await }); @@ -1161,6 +1165,7 @@ mod tests { ThreadConfig::default(), None, None, + None, ) .await }); @@ -1238,6 +1243,7 @@ mod tests { ThreadConfig::default(), None, None, + None, ) .await .unwrap(); @@ -1278,6 +1284,7 @@ mod tests { ThreadConfig::default(), None, Some(raw), + None, ) .await .unwrap(); @@ -1340,6 +1347,7 @@ mod tests { ThreadConfig::default(), None, Some(raw), + None, ) .await .unwrap(); @@ -1386,6 +1394,7 @@ mod tests { ThreadConfig::default(), None, Some(plain), + None, ) .await .unwrap();