Skip to content

Commit 9c5c425

Browse files
authored
Fix AI blocks not appearing due to conversation existing in more than one pane (warpdotdev#11494)
## Description Fix queries and output not showing up due to a conversation existing in more than one pane. A conversation and its blocks should only be in one pane at a time. This PR enforces this by notifying when a conversation becomes active in a new terminal view, so that old terminal views can delete the AI and command blocks for that conversation. https://github.com/warpdotdev/warp/pull/10241/changes#diff-fe3dd831b48d66196f87f13cd7a57f0e3660444b8f4fd9f6a5718be5fa617e40 introduced `mark_active_conversation_id` to not transfer ownership. warpdotdev#10327 uses it in the restoration path. The first PR mentions it's short term, and @advait-m mentioned these original fixes are probably no longer needed, but would like review from you and @szgupta to verify. My understanding is that a conversation must never be in more than one terminal view, even with orchestration, so it should be correct to enforce this when setting active conversation id. I'm also unsure what should be the source of truth for where the conversation lives. I'm using the ai history model for this, but it seems like the agent view controller also has this info. ## Testing <!-- How did you test this change? What automated tests did you add? If you didn't add any new tests, what's your justification for not adding any? Manual testing is required for changes that can be manually tested, and almost all changes can be manually tested. If your change can be manually tested, please include screenshots or a screen recording that show it working end to end. You can run the app locally using `./script/run` - see WARP.md for more details on how to get set up. --> - [x] I have manually tested my changes locally with `./script/run` ### Screenshots / Videos <!-- Attach screenshots or a short video demonstrating the change, where appropriate. Remove this section if it is not relevant to your PR. --> https://www.loom.com/share/e0af242e4f2f42dcbe77b1753dca5177 ## Changelog Entries for Stable CHANGELOG-BUG-FIX: Fixed an issue that could prevent an AI query and output from showing up
1 parent 46a59cb commit 9c5c425

12 files changed

Lines changed: 514 additions & 112 deletions

File tree

app/src/ai/blocklist/agent_view/agent_view_block.rs

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -88,7 +88,6 @@ impl AgentViewEntryBlock {
8888
_ => (),
8989
});
9090
ctx.subscribe_to_model(&agent_view_controller, |_, _, _, ctx| ctx.notify());
91-
9291
let active_agent_views_model = ActiveAgentViewsModel::handle(ctx);
9392
ctx.subscribe_to_model(&active_agent_views_model, |_, _, _, ctx| ctx.notify());
9493

app/src/ai/blocklist/agent_view/controller.rs

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -783,11 +783,8 @@ impl AgentViewController {
783783
});
784784
(id, 0)
785785
};
786-
// Non-transferring: don't rip the conversation out of another terminal
787-
// view's live list (e.g. a child agent's hidden pane). Explicit
788-
// cross-view ownership transfer is handled by callers elsewhere.
789786
history_model.update(ctx, |history_model, ctx| {
790-
history_model.mark_active_conversation_id(conversation_id, self.terminal_view_id, ctx)
787+
history_model.set_active_conversation_id(conversation_id, self.terminal_view_id, ctx)
791788
});
792789

793790
self.agent_view_state = AgentViewState::Active {

app/src/ai/blocklist/controller.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1449,7 +1449,7 @@ impl BlocklistAIController {
14491449
}
14501450

14511451
BlocklistAIHistoryModel::handle(ctx).update(ctx, |history, ctx| {
1452-
history.mark_active_conversation_id(conversation_id, self.terminal_view_id, ctx);
1452+
history.set_active_conversation_id(conversation_id, self.terminal_view_id, ctx);
14531453
});
14541454

14551455
if !FeatureFlag::AgentView.is_enabled() && trigger == FollowUpTrigger::Auto {
@@ -2402,7 +2402,7 @@ impl BlocklistAIController {
24022402
});
24032403
if !is_passive_request {
24042404
history_model.update(ctx, |history_model, ctx| {
2405-
history_model.mark_active_conversation_id(
2405+
history_model.set_active_conversation_id(
24062406
conversation_data.id,
24072407
self.terminal_view_id,
24082408
ctx,

app/src/ai/blocklist/history_model.rs

Lines changed: 11 additions & 49 deletions
Original file line numberDiff line numberDiff line change
@@ -813,10 +813,13 @@ impl BlocklistAIHistoryModel {
813813
for conversation in conversations.into_iter() {
814814
let conversation_id = conversation.id();
815815
conversation_ids.push(conversation_id);
816-
self.live_conversation_ids_for_terminal_view
816+
let live_conversation_ids = self
817+
.live_conversation_ids_for_terminal_view
817818
.entry(terminal_view_id)
818-
.or_default()
819-
.push(conversation_id);
819+
.or_default();
820+
if !live_conversation_ids.contains(&conversation_id) {
821+
live_conversation_ids.push(conversation_id);
822+
}
820823

821824
if let Some(key) = agent_id_key(&conversation) {
822825
self.agent_id_to_conversation_id
@@ -856,13 +859,8 @@ impl BlocklistAIHistoryModel {
856859
});
857860
}
858861

859-
/// Sets the active conversation ID, transferring ownership from any other
860-
/// terminal view that currently holds it.
861-
///
862-
/// Use this when the user **explicitly navigates** to a conversation in a
863-
/// different view (e.g. from the conversation history or command palette).
864-
/// For automatic follow-ups during tool-call cycles, use [`Self::mark_active_conversation_id`]
865-
/// instead — it updates the active pointer without touching other views.
862+
/// Sets the active conversation ID for a terminal view and transfers ownership
863+
/// from any other terminal view that currently holds it.
866864
pub fn set_active_conversation_id(
867865
&mut self,
868866
conversation_id: AIConversationId,
@@ -892,11 +890,9 @@ impl BlocklistAIHistoryModel {
892890
.iter_mut()
893891
.filter(|(other_terminal_view_id, _)| **other_terminal_view_id != terminal_view_id)
894892
{
895-
if let Some(pos) = other_terminal_view_live_conversation_ids
896-
.iter()
897-
.position(|id| *id == conversation_id)
898-
{
899-
other_terminal_view_live_conversation_ids.remove(pos);
893+
let previous_len = other_terminal_view_live_conversation_ids.len();
894+
other_terminal_view_live_conversation_ids.retain(|id| *id != conversation_id);
895+
if other_terminal_view_live_conversation_ids.len() != previous_len {
900896
previous_owners.push(*other_terminal_view);
901897
}
902898

@@ -930,40 +926,6 @@ impl BlocklistAIHistoryModel {
930926
});
931927
}
932928

933-
/// Marks a conversation as the active conversation for a terminal view
934-
/// **without** removing it from other views.
935-
///
936-
/// This is the non-transferring counterpart to [`Self::set_active_conversation_id`].
937-
/// Use this during automatic follow-ups and request sending where the
938-
/// conversation already belongs to this view and we only need to update
939-
/// the "most recently streamed" pointer.
940-
pub fn mark_active_conversation_id(
941-
&mut self,
942-
conversation_id: AIConversationId,
943-
terminal_view_id: EntityId,
944-
ctx: &mut ModelContext<Self>,
945-
) {
946-
if !self
947-
.live_conversation_ids_for_terminal_view
948-
.get(&terminal_view_id)
949-
.is_some_and(|conversation_ids| conversation_ids.contains(&conversation_id))
950-
{
951-
log::warn!(
952-
"mark_active_conversation_id: conversation {conversation_id:?} is not in \
953-
terminal view {terminal_view_id:?} live list, skipping"
954-
);
955-
return;
956-
}
957-
958-
self.active_conversation_for_terminal_view
959-
.insert(terminal_view_id, conversation_id);
960-
961-
ctx.emit(BlocklistAIHistoryEvent::SetActiveConversation {
962-
conversation_id,
963-
terminal_view_id,
964-
});
965-
}
966-
967929
/// Starts a new conversation in the given terminal view's history, effectively marking the
968930
/// existing conversation (if any) as completed.
969931
///

app/src/pane_group/mod.rs

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -138,7 +138,9 @@ use crate::terminal::shared_session::{
138138
use crate::terminal::view::inline_banner::{
139139
ZeroStatePromptSuggestionTriggeredFrom, ZeroStatePromptSuggestionType,
140140
};
141-
use crate::terminal::view::load_ai_conversation::RestoredAIConversation;
141+
use crate::terminal::view::load_ai_conversation::{
142+
RestoreConversationEntryBehavior, RestoredAIConversation,
143+
};
142144
use crate::terminal::view::ssh_file_upload::FileUploadId;
143145
use crate::terminal::view::{
144146
BlockNotification, ConversationRestorationInNewPaneType, ExecuteCommandEvent,
@@ -3348,6 +3350,7 @@ impl PaneGroup {
33483350
terminal_view.restore_conversation_after_view_creation(
33493351
RestoredAIConversation::new(child_conversation),
33503352
true,
3353+
RestoreConversationEntryBehavior::PreserveAgentViewState,
33513354
ctx,
33523355
);
33533356
terminal_view.enter_agent_view(
@@ -3379,6 +3382,7 @@ impl PaneGroup {
33793382
terminal_view.restore_conversation_after_view_creation(
33803383
RestoredAIConversation::new(child_conversation),
33813384
true,
3385+
RestoreConversationEntryBehavior::PreserveAgentViewState,
33823386
ctx,
33833387
);
33843388
terminal_view.enter_agent_view(
@@ -3442,6 +3446,7 @@ impl PaneGroup {
34423446
terminal_view.restore_conversation_after_view_creation(
34433447
RestoredAIConversation::new(child_conversation),
34443448
true,
3449+
RestoreConversationEntryBehavior::PreserveAgentViewState,
34453450
ctx,
34463451
);
34473452
terminal_view.enter_agent_view(
@@ -3540,6 +3545,7 @@ impl PaneGroup {
35403545
terminal_view.restore_conversation_after_view_creation(
35413546
RestoredAIConversation::new(child_conversation),
35423547
true,
3548+
RestoreConversationEntryBehavior::PreserveAgentViewState,
35433549
ctx,
35443550
);
35453551
terminal_view.enter_agent_view(
@@ -4260,6 +4266,7 @@ impl PaneGroup {
42604266
view.restore_conversation_after_view_creation(
42614267
RestoredAIConversation::new(*conversation),
42624268
true,
4269+
RestoreConversationEntryBehavior::EnterRestoredConversation,
42634270
ctx,
42644271
);
42654272
});
@@ -4281,6 +4288,7 @@ impl PaneGroup {
42814288
view.restore_conversation_and_directory_context(
42824289
CloudConversationData::CLIAgent(cli_conversation),
42834290
true,
4291+
RestoreConversationEntryBehavior::PreserveAgentViewState,
42844292
|_, _| {},
42854293
ctx,
42864294
);
@@ -5861,6 +5869,7 @@ impl PaneGroup {
58615869
view.restore_conversation_after_view_creation(
58625870
RestoredAIConversation::new(*conversation),
58635871
true,
5872+
RestoreConversationEntryBehavior::PreserveAgentViewState,
58645873
ctx,
58655874
);
58665875
view.enter_agent_view(None, Some(id), AgentViewEntryOrigin::CloudAgent, ctx);
@@ -5884,6 +5893,7 @@ impl PaneGroup {
58845893
view.restore_conversation_and_directory_context(
58855894
CloudConversationData::CLIAgent(cli_conversation),
58865895
true,
5896+
RestoreConversationEntryBehavior::PreserveAgentViewState,
58875897
|_, _| {},
58885898
ctx,
58895899
);

app/src/terminal/model/blocks.rs

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1510,6 +1510,44 @@ impl BlockList {
15101510
self.event_proxy.send_wakeup_event();
15111511
}
15121512

1513+
pub fn remove_command_blocks_for_conversation(&mut self, conversation_id: AIConversationId) {
1514+
let active_block_index = self.active_block_index();
1515+
1516+
let mut indices_to_remove = Vec::new();
1517+
for (i, block) in self.blocks.iter().enumerate() {
1518+
let index: BlockIndex = i.into();
1519+
if index == active_block_index {
1520+
continue;
1521+
}
1522+
1523+
if matches!(
1524+
block.agent_view_visibility(),
1525+
AgentViewVisibility::Agent {
1526+
origin_conversation_id,
1527+
..
1528+
} if *origin_conversation_id == conversation_id
1529+
) {
1530+
indices_to_remove.push(index);
1531+
}
1532+
}
1533+
1534+
if indices_to_remove.is_empty() {
1535+
return;
1536+
}
1537+
1538+
self.clear_selection();
1539+
self.clear_smart_select_override();
1540+
self.clear_scroll_position_before_filter();
1541+
1542+
// Remove in reverse order so indices remain valid.
1543+
for index in indices_to_remove.into_iter().rev() {
1544+
self.remove_block_at_index(index);
1545+
}
1546+
1547+
// Force a re-draw since the blocklist has changed.
1548+
self.event_proxy.send_wakeup_event();
1549+
}
1550+
15131551
/// Gets the active background block, if one exists.
15141552
pub(super) fn background_block_mut(&mut self) -> Option<&mut Block> {
15151553
// The active background block will be the one immediately before

app/src/terminal/view.rs

Lines changed: 16 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -5361,16 +5361,16 @@ impl TerminalView {
53615361
| BlocklistAIHistoryEvent::UpdatedConversationStatus {
53625362
conversation_id, ..
53635363
}
5364-
| BlocklistAIHistoryEvent::UpdatedConversationMetadata {
5365-
conversation_id, ..
5366-
}
53675364
| BlocklistAIHistoryEvent::UpdatedConversationArtifacts {
53685365
conversation_id, ..
53695366
} => history_model.terminal_view_id_for_conversation(conversation_id),
53705367
BlocklistAIHistoryEvent::ReassignedExchange {
53715368
new_conversation_id,
53725369
..
53735370
} => history_model.terminal_view_id_for_conversation(new_conversation_id),
5371+
BlocklistAIHistoryEvent::UpdatedConversationMetadata {
5372+
conversation_id, ..
5373+
} => history_model.terminal_view_id_for_conversation(conversation_id),
53745374
BlocklistAIHistoryEvent::StartedNewConversation { .. }
53755375
| BlocklistAIHistoryEvent::CreatedSubtask { .. }
53765376
| BlocklistAIHistoryEvent::UpgradedTask { .. }
@@ -5823,27 +5823,22 @@ impl TerminalView {
58235823
// The conversation has moved to another terminal view. We are
58245824
// the previous owner (the per-view filter at the top of this
58255825
// function uses `previous_terminal_view_id`), so drop any
5826-
// rendered AI blocks and agent-view entry blocks tagged to
5827-
// this conversation. Otherwise the user sees a transcript
5828-
// split across two panes (old exchanges here, new exchanges
5829-
// in the new owner).
5826+
// rendered blocks tagged to this conversation. Leave the
5827+
// agent-view entry in place so the user can click it to
5828+
// navigate to the current owner pane later.
58305829
if *previous_terminal_view_id != self.view_id {
58315830
return;
58325831
}
58335832
let view_ids_to_remove = self
58345833
.rich_content_views
58355834
.iter()
58365835
.filter_map(|view| {
5837-
let belongs_to_conversation = match view.metadata() {
5838-
Some(RichContentMetadata::AIBlock(metadata)) => {
5839-
metadata.conversation_id == *conversation_id
5840-
}
5841-
Some(RichContentMetadata::AgentViewEntry(metadata)) => {
5842-
metadata.conversation_id == *conversation_id
5843-
}
5844-
_ => false,
5845-
};
5846-
belongs_to_conversation.then_some(view.view_id())
5836+
let is_ai_block_for_conversation = matches!(
5837+
view.metadata(),
5838+
Some(RichContentMetadata::AIBlock(metadata))
5839+
if metadata.conversation_id == *conversation_id
5840+
);
5841+
is_ai_block_for_conversation.then_some(view.view_id())
58475842
})
58485843
.collect_vec();
58495844
for view_id_to_remove in view_ids_to_remove.into_iter() {
@@ -5854,6 +5849,10 @@ impl TerminalView {
58545849
self.rich_content_views
58555850
.retain(|view| view.view_id() != view_id_to_remove);
58565851
}
5852+
self.model
5853+
.lock()
5854+
.block_list_mut()
5855+
.remove_command_blocks_for_conversation(*conversation_id);
58575856
}
58585857
BlocklistAIHistoryEvent::CreatedSubtask { .. }
58595858
| BlocklistAIHistoryEvent::UpdatedAutoexecuteOverride { .. }

app/src/terminal/view/agent_view.rs

Lines changed: 39 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,9 @@ use crate::persistence::ModelEvent;
1717
use crate::server::telemetry::TelemetryAgentViewEntryOrigin;
1818
use crate::terminal::input::message_bar::{Message, MessageItem};
1919
use crate::terminal::model::rich_content::RichContentType;
20+
use crate::terminal::view::load_ai_conversation::{
21+
RestoreConversationEntryBehavior, RestoredAIConversation,
22+
};
2023
use crate::terminal::view::{
2124
AgentViewEntryMetadata, RichContentInsertionPosition, RichContentMetadata,
2225
};
@@ -124,14 +127,18 @@ impl TerminalView {
124127
conversation_id: AIConversationId,
125128
ctx: &mut ViewContext<Self>,
126129
) {
127-
let history_model = BlocklistAIHistoryModel::handle(ctx).as_ref(ctx);
128-
129-
let is_conversation_in_memory = history_model.conversation(&conversation_id).is_some();
130-
let is_live = history_model
131-
.all_live_conversations_for_terminal_view(self.view_id)
132-
.any(|conversation| conversation.id() == conversation_id);
130+
let history_model = BlocklistAIHistoryModel::handle(ctx);
131+
let (in_memory_conversation, is_live) = {
132+
let history_model_ref = history_model.as_ref(ctx);
133+
let in_memory_conversation = history_model_ref.conversation(&conversation_id).cloned();
134+
let is_live = in_memory_conversation.is_some()
135+
&& history_model_ref
136+
.all_live_conversations_for_terminal_view(self.view_id)
137+
.any(|conversation| conversation.id() == conversation_id);
138+
(in_memory_conversation, is_live)
139+
};
133140

134-
if is_conversation_in_memory && is_live {
141+
if is_live {
135142
if let Err(e) = self.try_enter_agent_view(
136143
initial_prompt.clone(),
137144
origin,
@@ -146,9 +153,32 @@ impl TerminalView {
146153
);
147154
self.show_error_toast(e.to_string(), ctx);
148155
}
156+
} else if let Some(conversation) = in_memory_conversation {
157+
self.restore_conversation_after_view_creation(
158+
RestoredAIConversation::new(conversation),
159+
false,
160+
RestoreConversationEntryBehavior::PreserveAgentViewState,
161+
ctx,
162+
);
163+
if let Err(e) = self.try_enter_agent_view(
164+
initial_prompt.clone(),
165+
origin,
166+
Some(conversation_id),
167+
ctx,
168+
) {
169+
log::error!(
170+
"Failed to enter agent view for restored in-memory conversation ({:?}) from origin {:?}: {:?}",
171+
conversation_id,
172+
origin,
173+
e
174+
);
175+
self.show_error_toast(e.to_string(), ctx);
176+
}
149177
} else {
150178
let conversation_id_copy = conversation_id;
151-
let future = history_model.load_conversation_data(conversation_id_copy, ctx);
179+
let future = history_model
180+
.as_ref(ctx)
181+
.load_conversation_data(conversation_id_copy, ctx);
152182
ctx.spawn(future, move |me, conversation, ctx| {
153183
let Some(conversation) = conversation else {
154184
me.show_error_toast(
@@ -185,6 +215,7 @@ impl TerminalView {
185215
me.restore_conversation_and_directory_context(
186216
conversation,
187217
false,
218+
RestoreConversationEntryBehavior::PreserveAgentViewState,
188219
on_restored,
189220
ctx,
190221
);

0 commit comments

Comments
 (0)