fix(gateway): keep engine threads out of chat sidebar (#2751)

* fix(gateway): keep engine threads out of chat sidebar

* fix: address review findings (iteration 1)

* fix: address review findings (iteration 2)

crates/ironclaw_gateway/static/js/core/bootstrap.js

@@ -79,6 +79,7 @@ let logEventSource = null;
 let currentTab = 'chat';
 let currentThreadId = null;
 let currentThreadIsReadOnly = false;
+const threadChannelHints = new Map();
 let assistantThreadId = null;
 let hasMore = false;
 let oldestTimestamp = null;

crates/ironclaw_gateway/static/js/core/history.js

@@ -40,6 +40,10 @@ function loadHistory(before) {
   apiFetch(historyUrl).then((data) => {
     const container = document.getElementById('chat-messages');
 
+    if (!isPaginating && currentThreadId && data.channel) {
+      threadChannelHints.set(currentThreadId, data.channel);
+    }
+
     if (!isPaginating) {
       // Fresh load: clear and render
       container.innerHTML = '';
@@ -117,6 +121,16 @@ function loadHistory(before) {
       } else if (lastTurn && !lastTurn.response && lastTurn.state === 'Processing') {
         showActivityThinking(ActivityEntry.t('activity.processing', 'Processing...'));
       }
+      const hintedChannel = currentThreadId
+        ? (data.channel || threadChannelHints.get(currentThreadId) || 'gateway')
+        : 'gateway';
+      currentThreadIsReadOnly = isReadOnlyChannel(hintedChannel);
+      if (currentThreadIsReadOnly) {
+        disableChatInputReadOnly();
+      } else {
+        enableChatInput();
+      }
+
       if (data.pending_gate) {
         handleGateRequired({
           ...data.pending_gate,
@@ -467,7 +481,10 @@ function loadThreads() {
       const currentThread = currentThreadId === assistantThreadId
         ? data.assistant_thread
         : threads.find(t => t.id === currentThreadId);
-      const ch = currentThread ? currentThread.channel : 'gateway';
+      const hintedChannel = currentThread
+        ? currentThread.channel
+        : threadChannelHints.get(currentThreadId);
+      const ch = hintedChannel || 'gateway';
       currentThreadIsReadOnly = isReadOnlyChannel(ch);
       if (currentThreadIsReadOnly) {
         disableChatInputReadOnly();

src/channels/web/features/chat/mod.rs

@@ -523,6 +523,7 @@ pub(crate) async fn chat_history_handler(
             turns,
             has_more,
             oldest_timestamp,
+            channel: None,
             pending_gate: history_pending_gate_info(&state, &user.user_id, thread_scope).await,
             in_progress: None,
         }));
@@ -559,6 +560,7 @@ pub(crate) async fn chat_history_handler(
             turns,
             has_more: false,
             oldest_timestamp: None,
+            channel: None,
             pending_gate,
             in_progress: in_progress_from_thread(thread),
         }));
@@ -588,6 +590,7 @@ pub(crate) async fn chat_history_handler(
                 turns,
                 has_more,
                 oldest_timestamp,
+                channel: None,
                 pending_gate: history_pending_gate_info(&state, &user.user_id, thread_scope).await,
                 in_progress,
             }));
@@ -608,19 +611,18 @@ pub(crate) async fn chat_history_handler(
             .enumerate()
             .filter_map(|(index, entry)| engine_history_entry_to_message(thread_id, index, entry))
             .collect();
-        if !synthetic.is_empty() {
-            let oldest_timestamp = synthetic.first().map(|m| m.created_at.to_rfc3339());
-            let mut turns = build_turns_from_db_messages(&synthetic);
-            enforce_generated_image_history_budget(&mut turns);
-            return Ok(Json(HistoryResponse {
-                thread_id,
-                turns,
-                has_more: false,
-                oldest_timestamp,
-                pending_gate: history_pending_gate_info(&state, &user.user_id, thread_scope).await,
-                in_progress: None,
-            }));
-        }
+        let oldest_timestamp = synthetic.first().map(|m| m.created_at.to_rfc3339());
+        let mut turns = build_turns_from_db_messages(&synthetic);
+        enforce_generated_image_history_budget(&mut turns);
+        return Ok(Json(HistoryResponse {
+            thread_id,
+            turns,
+            has_more: false,
+            oldest_timestamp,
+            channel: Some("engine".to_string()),
+            pending_gate: history_pending_gate_info(&state, &user.user_id, thread_scope).await,
+            in_progress: None,
+        }));
     }
 
     // Empty thread (just created, no messages yet)
@@ -639,6 +641,7 @@ pub(crate) async fn chat_history_handler(
         turns: Vec::new(),
         has_more: false,
         oldest_timestamp: None,
+        channel: None,
         pending_gate: history_pending_gate_info(&state, &user.user_id, thread_scope).await,
         in_progress,
     }))
@@ -723,46 +726,12 @@ pub(crate) async fn chat_threads_handler(
                     });
                 }
 
-                // Engine v2 threads for this user in the default project. These
-                // don't always get a matching v1 conversation row (the assistant
-                // flow dual-writes into the single assistant conv id, not the
-                // engine thread id), so without this merge they'd be invisible
-                // in the sidebar even though the chat history endpoint can now
-                // render them by id.
-                if let Ok(engine_threads) =
-                    crate::bridge::list_engine_threads(None, &user.user_id).await
-                {
-                    let existing_ids: std::collections::HashSet<uuid::Uuid> = threads
-                        .iter()
-                        .map(|t| t.id)
-                        .chain(assistant_thread.as_ref().map(|a| a.id))
-                        .collect();
-                    for eng in engine_threads {
-                        let Ok(uuid) = uuid::Uuid::parse_str(&eng.id) else {
-                            continue;
-                        };
-                        if existing_ids.contains(&uuid) {
-                            continue;
-                        }
-                        threads.push(ThreadInfo {
-                            id: uuid,
-                            state: eng.state,
-                            turn_count: eng.step_count,
-                            created_at: eng.created_at,
-                            updated_at: eng.updated_at.clone(),
-                            // Engine threads carry their goal as the only
-                            // human-readable label; reuse it as the sidebar
-                            // title so the user can tell threads apart.
-                            title: Some(eng.goal),
-                            thread_type: Some(eng.thread_type),
-                            channel: Some("engine".to_string()),
-                        });
-                    }
-                    // Re-sort by updated_at descending so engine threads interleave
-                    // chronologically with v1 conversations.
-                    threads.sort_by(|a, b| b.updated_at.cmp(&a.updated_at));
-                }
-
+                // Keep the chat sidebar scoped to persisted chat conversations.
+                // Engine v2 foreground threads are assistant execution internals
+                // and can rotate per message, so surfacing them here makes
+                // ordinary prompts look like standalone `engine` threads.
+                // Explicit engine-thread history still works via
+                // `chat_history_handler` when the caller already has a thread id.
                 let active_thread = session.lock().await.active_thread;
 
                 return Ok(Json(ThreadListResponse {
@@ -1900,6 +1869,63 @@ mod tests {
         assert_eq!(response.threads[0].channel.as_deref(), Some("gateway"));
     }
 
+    #[cfg(feature = "libsql")]
+    #[tokio::test]
+    async fn test_chat_threads_handler_hides_engine_threads_from_sidebar() {
+        let _lock = crate::bridge::test_support::ENGINE_STATE_TEST_LOCK
+            .lock()
+            .await;
+        crate::bridge::test_support::clear_engine_state().await;
+
+        let project_id =
+            crate::bridge::test_support::install_engine_state_with_threads(Vec::new()).await;
+        let mut thread = ironclaw_engine::Thread::new(
+            "assistant hello",
+            ironclaw_engine::ThreadType::Foreground,
+            project_id,
+            "alice",
+            ironclaw_engine::ThreadConfig::default(),
+        );
+        thread
+            .messages
+            .push(ironclaw_engine::ThreadMessage::user("hello"));
+        let engine_thread_id = thread.id.0;
+        crate::bridge::test_support::install_engine_state_with_threads(vec![thread]).await;
+
+        let (db, _tmp) = crate::testing::test_db().await;
+        let session_manager = Arc::new(SessionManager::new());
+        let state = test_gateway_state_with_store_and_session_manager(db, session_manager);
+
+        let response = chat_threads_handler(
+            axum::extract::State(state),
+            crate::channels::web::auth::AuthenticatedUser(UserIdentity {
+                user_id: "alice".to_string(),
+                role: "member".to_string(),
+                workspace_read_scopes: Vec::new(),
+            }),
+        )
+        .await
+        .expect("handler ok");
+
+        assert!(response.assistant_thread.is_some());
+        assert!(
+            response
+                .threads
+                .iter()
+                .all(|thread| thread.id != engine_thread_id),
+            "chat sidebar must not surface engine execution threads"
+        );
+        assert!(
+            response
+                .threads
+                .iter()
+                .all(|thread| thread.channel.as_deref() != Some("engine")),
+            "chat sidebar must stay scoped to chat conversations"
+        );
+
+        crate::bridge::test_support::clear_engine_state().await;
+    }
+
     #[cfg(feature = "libsql")]
     #[tokio::test]
     async fn test_chat_new_thread_handler_persists_to_db_and_session() {
@@ -2266,11 +2292,46 @@ mod tests {
         let turn = &response.turns[0];
         assert_eq!(turn.user_input, "hello engine");
         assert_eq!(turn.response.as_deref(), Some("hi back"));
+        assert_eq!(response.channel.as_deref(), Some("engine"));
         assert!(!response.has_more);
 
         crate::bridge::test_support::clear_engine_state().await;
     }
 
+    #[tokio::test]
+    async fn test_chat_history_returns_engine_channel_hint_without_renderable_messages() {
+        let _lock = crate::bridge::test_support::ENGINE_STATE_TEST_LOCK
+            .lock()
+            .await;
+        crate::bridge::test_support::clear_engine_state().await;
+
+        let project_id =
+            crate::bridge::test_support::install_engine_state_with_threads(Vec::new()).await;
+        let thread = ironclaw_engine::Thread::new(
+            "empty engine thread",
+            ironclaw_engine::ThreadType::Foreground,
+            project_id,
+            "alice",
+            ironclaw_engine::ThreadConfig::default(),
+        );
+        let thread_uuid = thread.id.0;
+        crate::bridge::test_support::install_engine_state_with_threads(vec![thread]).await;
+
+        let mut state = test_gateway_state_with_dependencies(None, None, None, None);
+        Arc::get_mut(&mut state)
+            .expect("state should be uniquely owned")
+            .session_manager = Some(Arc::new(SessionManager::new()));
+
+        let (s, u, q) = history_request(state, "alice", thread_uuid);
+        let response = chat_history_handler(s, u, q).await.expect("history");
+
+        assert_eq!(response.thread_id, thread_uuid);
+        assert!(response.turns.is_empty());
+        assert_eq!(response.channel.as_deref(), Some("engine"));
+
+        crate::bridge::test_support::clear_engine_state().await;
+    }
+
     #[tokio::test]
     async fn test_chat_history_returns_404_for_cross_user_engine_thread() {
         let _lock = crate::bridge::test_support::ENGINE_STATE_TEST_LOCK

src/channels/web/types.rs

@@ -124,6 +124,10 @@ pub struct HistoryResponse {
     /// Cursor for the next page (ISO8601 timestamp of the oldest message returned).
     #[serde(skip_serializing_if = "Option::is_none")]
     pub oldest_timestamp: Option<String>,
+    /// Channel hint for history views that are not present in the sidebar.
+    /// Used by the frontend to keep deep-linked non-gateway threads read-only.
+    #[serde(skip_serializing_if = "Option::is_none")]
+    pub channel: Option<String>,
     /// Unified pending gate state for engine v2.
     #[serde(skip_serializing_if = "Option::is_none")]
     pub pending_gate: Option<PendingGateInfo>,

tests/e2e/scenarios/test_v2_thread_visibility.py

@@ -1,18 +1,17 @@
-"""E2E regression: engine v2 threads are visible in sidebar and history.
+"""E2E regression: engine threads stay out of chat sidebar while history works.
 
-Covers the behavior PR #2532 introduced in `chat_threads_handler` and
-`chat_history_handler`:
+Covers the intended split between the chat sidebar and engine APIs:
 
-- An engine v2 thread created from a `/api/chat/send` call shows up in the
-  `/api/chat/threads` sidebar with `channel == "engine"`.
-- `/api/chat/history?thread_id=<engine-thread-id>` returns the messages
-  synthesized from engine thread transcript even when the v1 conversation
-  table has no row for that id (deep-link-by-id path).
+- A foreground engine thread spawned by `/api/chat/send` must remain
+  discoverable via `/api/engine/threads`, but it must *not* surface as an
+  `engine` entry inside `/api/chat/threads`.
+- `/api/chat/history?thread_id=<engine-thread-id>` must still synthesize the
+  transcript for callers that explicitly deep-link to that engine thread id.
 
-Prior behavior silently dropped these threads from the sidebar and
-returned an empty history on deep-link; the fixture drives the HTTP
-surface directly so the regression survives independent of frontend
-polish.
+The staging regression merged engine foreground threads into the normal chat
+sidebar, which made ordinary prompts look like separate `ENGINE`
+conversations. This fixture keeps that bug from coming back while preserving
+explicit engine-thread history access.
 """
 
 import asyncio
@@ -156,26 +155,28 @@ async def _wait_for_assistant_response(
     )
 
 
-async def _engine_only_threads(base_url: str) -> list[dict]:
-    """Return sidebar entries whose channel is engine (the v2-merge path)."""
+async def _chat_sidebar_threads(base_url: str) -> list[dict]:
     r = await api_get(base_url, "/api/chat/threads", timeout=15)
     r.raise_for_status()
-    return [t for t in r.json().get("threads", []) if t.get("channel") == "engine"]
+    return r.json().get("threads", [])
+
+
+async def _engine_threads(base_url: str) -> list[dict]:
+    r = await api_get(base_url, "/api/engine/threads", timeout=15)
+    r.raise_for_status()
+    return r.json().get("threads", [])
 
 
 class TestV2ThreadVisibility:
-    async def test_engine_only_thread_appears_in_sidebar_with_engine_channel(
+    async def test_engine_thread_stays_out_of_chat_sidebar(
         self, v2_visibility_server
     ):
-        """Send without a client-supplied thread_id: the v1 flow dual-writes
-        into the shared assistant conversation, but the engine spins up a
-        fresh thread id that has no matching v1 row. The PR's merge should
-        surface that engine thread in the sidebar with `channel=engine`.
+        """Assistant sends still spawn engine threads, but those execution
+        threads must stay out of the normal chat sidebar.
         """
         base = v2_visibility_server
 
-        baseline = await _engine_only_threads(base)
-        baseline_ids = {t["id"] for t in baseline}
+        baseline_engine_ids = {t["id"] for t in await _engine_threads(base)}
 
         send_r = await api_post(
             base,
@@ -185,22 +186,25 @@ class TestV2ThreadVisibility:
         )
         assert send_r.status_code in (200, 202), send_r.text
 
-        new_engine_entry = None
+        engine_thread = None
         for _ in range(60):
-            merged = await _engine_only_threads(base)
-            new_entries = [t for t in merged if t["id"] not in baseline_ids]
-            if new_entries:
-                new_engine_entry = new_entries[0]
+            engine_threads = await _engine_threads(base)
+            new_threads = [t for t in engine_threads if t["id"] not in baseline_engine_ids]
+            if new_threads:
+                engine_thread = new_threads[0]
                 break
             await asyncio.sleep(0.5)
 
-        assert new_engine_entry is not None, (
-            "a new engine-only thread must appear in the sidebar after an "
-            "assistant send with no thread_id; PR #2532 added this merge path"
+        assert engine_thread is not None, "engine thread never materialized"
+
+        sidebar_threads = await _chat_sidebar_threads(base)
+        assert all(t.get("channel") != "engine" for t in sidebar_threads), (
+            "chat sidebar must not show engine execution threads as normal "
+            f"conversations, got {sidebar_threads}"
         )
-        assert new_engine_entry.get("title"), (
-            f"engine sidebar entry must carry a goal as title, got "
-            f"{new_engine_entry}"
+        assert all(t.get("id") != engine_thread["id"] for t in sidebar_threads), (
+            "the newly spawned engine thread must stay discoverable via the "
+            "/api/engine/threads surface, not /api/chat/threads"
         )
 
     async def test_history_synthesizes_messages_for_deep_linked_engine_thread(
@@ -211,7 +215,7 @@ class TestV2ThreadVisibility:
         """
         base = v2_visibility_server
 
-        baseline_ids = {t["id"] for t in await _engine_only_threads(base)}
+        baseline_engine_ids = {t["id"] for t in await _engine_threads(base)}
 
         await api_post(
             base,
@@ -222,17 +226,15 @@ class TestV2ThreadVisibility:
 
         engine_thread_id = None
         for _ in range(60):
-            merged = await _engine_only_threads(base)
-            new = [t for t in merged if t["id"] not in baseline_ids]
-            if new:
-                engine_thread_id = new[0]["id"]
+            engine_threads = await _engine_threads(base)
+            new_threads = [t for t in engine_threads if t["id"] not in baseline_engine_ids]
+            if new_threads:
+                engine_thread_id = new_threads[0]["id"]
                 break
             await asyncio.sleep(0.5)
 
-        assert engine_thread_id is not None, "engine-only thread never materialized"
+        assert engine_thread_id is not None, "engine thread never materialized"
 
-        # Deep-link by engine thread id. Before PR #2532 this returned an
-        # empty turn list because the v1 conversation lookup missed.
         turns = await _wait_for_assistant_response(
             base, engine_thread_id, timeout=45
         )