From 7d9512117c03609294b96f2e8ecce23a3b82d2c9 Mon Sep 17 00:00:00 2001 From: seemeroland Date: Mon, 13 Jul 2026 19:46:20 +0000 Subject: [PATCH] Fix APP-4853: restore subagent chips after session reopen MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The TUI startup speedup (commit 0ac6f594) changed startup to deliver conversations with empty task lists, then load full task payloads lazily from the local DB on demand. The eager orchestration-child hydration path in `initialize_historical_conversations` was updated to call `load_conversation_from_db` when `agent_conversation.tasks.is_empty()`, but without a fallback: if the RO DB connection cannot be established (or any other read error occurs), the function returns `None` and the child is never inserted into `conversations_by_id`. That missing insertion caused the regression: - `OrchestrationPillBar::pill_specs` filters descendant IDs through `history.conversation(&id)`, so unhydrated children produce an empty pill list → no chips at the top of the conversation. - `participant_for_agent_id` resolves an agent ID to a conversation, then looks it up in `conversations_by_id`. When the lookup fails it falls back to `OrchestrationParticipant::unknown_child()` → "Unknown agent" in send_message / messages_received transcript rows. The fix: when `agent_conversation.tasks.is_empty()`, try `load_conversation_from_db` first (to get full task data for rich display) and fall back to `convert_persisted_conversation_to_ai_conversation_with_metadata` if that fails. The fallback synthesizes a minimal `AIConversation` from the `conversation_data` column already in memory — same as the pre-TUI- speedup code path in QUALITY-768. The synthesized conversation carries the correct `task_id` (run_id), `agent_name`, and parent linkage, which is all the pill bar and name resolution need. A new test, `…with_empty_tasks`, exercises the production code path (tasks == []) to pin the fallback behaviour. The existing test `test_initialize_historical_conversations_eagerly_hydrates_orchestration_children` used non-empty tasks, so it took the direct-conversion branch and missed the regression entirely. CHANGELOG-BUG-FIX: Fixed subagent chips being missing and 'Unknown agent' displayed when reopening a session that used orchestrated subagents. Co-Authored-By: Oz --- .../history_model/conversation_loader.rs | 14 +- app/src/ai/blocklist/history_model_tests.rs | 152 ++++++++++++++++++ 2 files changed, 165 insertions(+), 1 deletion(-) diff --git a/app/src/ai/blocklist/history_model/conversation_loader.rs b/app/src/ai/blocklist/history_model/conversation_loader.rs index e78ffa5675e..be8cd997796 100644 --- a/app/src/ai/blocklist/history_model/conversation_loader.rs +++ b/app/src/ai/blocklist/history_model/conversation_loader.rs @@ -582,9 +582,21 @@ impl BlocklistAIHistoryModel { // // Startup rows carry no tasks, so the child's task // payload is loaded from the local DB; fully-hydrated - // inputs convert directly. + // inputs convert directly. If the DB load fails (e.g. + // the RO connection could not be established), fall back + // to synthesizing a minimal conversation from the + // metadata already in memory. This is the same shape as + // the pre-TUI-speedup code path and is sufficient for + // the pill bar and orchestration transcript name + // resolution, which only need `agent_name`, `task_id`, + // and parent linkage — not the full task transcript. let child_conversation = if agent_conversation.tasks.is_empty() { self.load_conversation_from_db(&conversation_id) + .or_else(|| { + convert_persisted_conversation_to_ai_conversation_with_metadata( + agent_conversation.clone(), + ) + }) } else { convert_persisted_conversation_to_ai_conversation_with_metadata( agent_conversation.clone(), diff --git a/app/src/ai/blocklist/history_model_tests.rs b/app/src/ai/blocklist/history_model_tests.rs index 8b2b8313cf7..a7a5ea0f932 100644 --- a/app/src/ai/blocklist/history_model_tests.rs +++ b/app/src/ai/blocklist/history_model_tests.rs @@ -947,6 +947,158 @@ fn test_initialize_historical_conversations_eagerly_hydrates_orchestration_child }); } +#[test] +fn test_initialize_historical_conversations_eagerly_hydrates_orchestration_children_with_empty_tasks( +) { + // Regression test for APP-4853: after the TUI startup speedup + // (commit 0ac6f594), conversations arrive from startup with empty + // task lists. The eager child hydration must still work via the + // `convert_persisted_conversation_to_ai_conversation_with_metadata` + // fallback when `load_conversation_from_db` cannot find a DB (which is + // always the case in tests). This covers the production code path where + // `agent_conversation.tasks.is_empty() == true`. + App::test((), |app| async move { + let parent_id = AIConversationId::new(); + let child_id = AIConversationId::new(); + let parent_run_id = Uuid::new_v4().to_string(); + let child_run_id = Uuid::new_v4().to_string(); + let now = Utc::now().naive_utc(); + + // Simulate the production startup shape: task lists are EMPTY, and + // each record carries a pre-computed `summary` as written by + // `upsert_agent_conversation`. + let child_summary = serde_json::to_string(&AgentConversationSummary { + initial_query: "Child query".to_string(), + title: "Child query".to_string(), + initial_working_directory: None, + is_restorable: true, + is_unlisted_auto_code_diff: false, + }) + .expect("summary should serialize"); + let parent_summary = serde_json::to_string(&AgentConversationSummary { + initial_query: "Parent query".to_string(), + title: "Parent query".to_string(), + initial_working_directory: None, + is_restorable: true, + is_unlisted_auto_code_diff: false, + }) + .expect("summary should serialize"); + + let conversations = vec![ + AgentConversation { + conversation: AgentConversationRecord { + id: 0, + conversation_id: child_id.to_string(), + conversation_data: serde_json::to_string(&AgentConversationData { + server_conversation_token: Some("child-token".to_string()), + conversation_usage_metadata: None, + reverted_action_ids: None, + forked_from_server_conversation_token: None, + artifacts_json: None, + parent_agent_id: Some(parent_run_id.clone()), + agent_name: Some("Agent 1".to_string()), + orchestration_harness_type: None, + parent_conversation_id: Some(parent_id.to_string()), + is_remote_child: false, + root_task_is_optimistic: None, + run_id: Some(child_run_id.clone()), + autoexecute_override: None, + last_event_sequence: None, + pinned: false, + }) + .expect("conversation data should serialize"), + last_modified_at: now, + summary: Some(child_summary), + }, + tasks: vec![], // Production startup shape: no tasks loaded + }, + AgentConversation { + conversation: AgentConversationRecord { + id: 1, + conversation_id: parent_id.to_string(), + conversation_data: serde_json::to_string(&AgentConversationData { + server_conversation_token: Some("parent-token".to_string()), + conversation_usage_metadata: None, + reverted_action_ids: None, + forked_from_server_conversation_token: None, + artifacts_json: None, + parent_agent_id: None, + agent_name: None, + orchestration_harness_type: None, + parent_conversation_id: None, + is_remote_child: false, + root_task_is_optimistic: None, + run_id: Some(parent_run_id.clone()), + autoexecute_override: None, + last_event_sequence: None, + pinned: false, + }) + .expect("conversation data should serialize"), + last_modified_at: now - chrono::Duration::seconds(1), + summary: Some(parent_summary), + }, + tasks: vec![], // Production startup shape: no tasks loaded + }, + ]; + + let history_model = app + .add_singleton_model(|_| BlocklistAIHistoryModel::new(vec![], vec![], &conversations)); + + history_model.read(&app, |model, _| { + // APP-4853 regression: child must still be eagerly hydrated even + // when its task list is empty (the production startup shape). + // Without the fallback to + // `convert_persisted_conversation_to_ai_conversation_with_metadata`, + // `load_conversation_from_db` returns `None` in tests (no backing + // DB), leaving the child absent from `conversations_by_id` and + // causing the pill bar and name resolution to show nothing / + // "Unknown agent". + assert!( + model.conversation(&child_id).is_some(), + "APP-4853: orchestration child with empty tasks should still be eagerly hydrated \ + into conversations_by_id via the metadata-synthesis fallback", + ); + // The hydrated child must have the correct agent name and run_id + // so the pill bar label and name resolution both work. + let child = model.conversation(&child_id).unwrap(); + assert_eq!( + child.agent_name(), + Some("Agent 1"), + "child agent_name should be restored from conversation_data", + ); + assert_eq!( + child.orchestration_agent_id().as_deref(), + Some(child_run_id.as_str()), + "child run_id should be restorable from conversation_data", + ); + // children_by_parent index is populated so the pill bar tree + // traversal finds the child. + assert_eq!( + model.child_conversation_ids_of(&parent_id), + &[child_id], + "orchestration children should be indexed in children_by_parent", + ); + // run_id → conversation_id index is populated so name resolution + // in send_message / messages_received rows resolves correctly. + assert_eq!( + model.conversation_id_for_agent_id(&child_run_id), + Some(child_id), + "child run_id should be indexed in agent_id_to_conversation_id", + ); + assert_eq!( + model.conversation_id_for_agent_id(&parent_run_id), + Some(parent_id), + "parent run_id should be indexed in agent_id_to_conversation_id", + ); + // Parent must NOT be in conversations_by_id (stays on lazy path). + assert!( + model.conversation(&parent_id).is_none(), + "parent conversation should NOT be eagerly loaded", + ); + }); + }); +} + #[test] fn prompt_history_candidates_seeds_from_snapshot_then_appends_session_prompts() { App::test((), |mut app| async move {