Skip to content

Commit ad8dd0f

Browse files
committed
address agent comments
1 parent f99ed07 commit ad8dd0f

11 files changed

Lines changed: 297 additions & 339 deletions

File tree

app/src/ai/agent_management/agent_management_model.rs

Lines changed: 39 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -165,26 +165,27 @@ impl AgentNotificationsModel {
165165
CLIAgent::Codex => "Notification from Codex",
166166
_ => "Task completed.",
167167
};
168-
let is_ambient = is_terminal_view_ambient(*terminal_view_id, ctx);
168+
let metadata = TerminalViewMetadata::lookup(*terminal_view_id, ctx);
169169
self.add_notification(
170170
title,
171171
message.to_owned(),
172172
NotificationCategory::Complete,
173173
NotificationSourceAgent::CLI {
174174
agent: *agent,
175-
is_ambient,
175+
is_ambient: metadata.is_ambient,
176176
},
177177
NotificationOrigin::CLISession(*terminal_view_id),
178178
*terminal_view_id,
179179
vec![],
180+
metadata.branch,
180181
ctx,
181182
);
182183
}
183184
CLIAgentSessionStatus::Blocked { message } => {
184185
let title = session_context
185186
.display_title()
186187
.unwrap_or_else(|| format!("{} needs attention", agent.display_name()));
187-
let is_ambient = is_terminal_view_ambient(*terminal_view_id, ctx);
188+
let metadata = TerminalViewMetadata::lookup(*terminal_view_id, ctx);
188189
self.add_notification(
189190
title,
190191
message
@@ -193,11 +194,12 @@ impl AgentNotificationsModel {
193194
NotificationCategory::Request,
194195
NotificationSourceAgent::CLI {
195196
agent: *agent,
196-
is_ambient,
197+
is_ambient: metadata.is_ambient,
197198
},
198199
NotificationOrigin::CLISession(*terminal_view_id),
199200
*terminal_view_id,
200201
vec![],
202+
metadata.branch,
201203
ctx,
202204
);
203205
}
@@ -318,8 +320,10 @@ impl AgentNotificationsModel {
318320
}
319321

320322
let title = latest_query.unwrap_or_else(|| "Agent task".to_owned());
321-
let is_ambient = is_terminal_view_ambient(terminal_view_id, ctx);
322-
let oz_agent = NotificationSourceAgent::Oz { is_ambient };
323+
let metadata = TerminalViewMetadata::lookup(terminal_view_id, ctx);
324+
let oz_agent = NotificationSourceAgent::Oz {
325+
is_ambient: metadata.is_ambient,
326+
};
323327

324328
match status {
325329
// When the agent resumes its work, clear stale notifications.
@@ -336,6 +340,7 @@ impl AgentNotificationsModel {
336340
origin,
337341
terminal_view_id,
338342
artifacts,
343+
metadata.branch,
339344
ctx,
340345
);
341346
}
@@ -349,6 +354,7 @@ impl AgentNotificationsModel {
349354
origin,
350355
terminal_view_id,
351356
artifacts,
357+
metadata.branch,
352358
ctx,
353359
);
354360
}
@@ -361,6 +367,7 @@ impl AgentNotificationsModel {
361367
origin,
362368
terminal_view_id,
363369
vec![],
370+
metadata.branch,
364371
ctx,
365372
);
366373
}
@@ -374,6 +381,7 @@ impl AgentNotificationsModel {
374381
origin,
375382
terminal_view_id,
376383
artifacts,
384+
metadata.branch,
377385
ctx,
378386
);
379387
}
@@ -411,14 +419,14 @@ impl AgentNotificationsModel {
411419
origin: NotificationOrigin,
412420
terminal_view_id: EntityId,
413421
artifacts: Vec<Artifact>,
422+
branch: Option<String>,
414423
ctx: &mut ModelContext<Self>,
415424
) {
416425
if !*AISettings::as_ref(ctx).show_agent_notifications {
417426
return;
418427
}
419428

420429
let is_visible = is_terminal_view_visible(terminal_view_id, ctx);
421-
let branch = resolve_git_branch_for_terminal_view(terminal_view_id, ctx);
422430
let item = NotificationItem::new(
423431
title,
424432
message,
@@ -512,6 +520,30 @@ fn window_and_tab_idx_id_for_conversation(
512520
})
513521
}
514522

523+
/// Per-notification metadata derived from a single [`TerminalView`] lookup. Both fields
524+
/// are read on the same emit path, so we resolve the view once and pass the projection
525+
/// down rather than walking the workspace tree for each.
526+
struct TerminalViewMetadata {
527+
is_ambient: bool,
528+
branch: Option<String>,
529+
}
530+
531+
impl TerminalViewMetadata {
532+
fn lookup(terminal_view_id: EntityId, app: &AppContext) -> Self {
533+
let Some(terminal_view) = find_terminal_view_by_id(terminal_view_id, app) else {
534+
return Self {
535+
is_ambient: false,
536+
branch: None,
537+
};
538+
};
539+
let view = terminal_view.as_ref(app);
540+
Self {
541+
is_ambient: view.is_ambient_agent_session(app),
542+
branch: view.current_git_branch(app),
543+
}
544+
}
545+
}
546+
515547
fn find_terminal_view_by_id(
516548
terminal_view_id: EntityId,
517549
app: &AppContext,
@@ -531,26 +563,6 @@ fn find_terminal_view_by_id(
531563
None
532564
}
533565

534-
fn resolve_git_branch_for_terminal_view(
535-
terminal_view_id: EntityId,
536-
app: &AppContext,
537-
) -> Option<String> {
538-
find_terminal_view_by_id(terminal_view_id, app)
539-
.and_then(|terminal_view| terminal_view.as_ref(app).current_git_branch(app))
540-
}
541-
542-
fn is_terminal_view_ambient(terminal_view_id: EntityId, app: &AppContext) -> bool {
543-
find_terminal_view_by_id(terminal_view_id, app)
544-
.map(|terminal_view| {
545-
terminal_view
546-
.as_ref(app)
547-
.ambient_agent_view_model()
548-
.as_ref(app)
549-
.is_ambient_agent()
550-
})
551-
.unwrap_or(false)
552-
}
553-
554566
fn active_focused_terminal_id(app: &AppContext) -> Option<EntityId> {
555567
let active_window = app.windows().active_window()?;
556568
let workspace = app

app/src/ai/ambient_agents/task.rs

Lines changed: 6 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -83,30 +83,17 @@ impl HarnessConfig {
8383
}
8484
}
8585

86-
/// Parses a harness type name (e.g. `"claude"`) into a [`Harness`] variant.
87-
/// Unknown values fall back to [`Harness::Unknown`] so we don't
88-
/// misrepresent a future-server harness as Oz; UI surfaces should treat
89-
/// `Unknown` as a non-Oz, non-runnable harness.
90-
pub(crate) fn harness_from_name(name: &str) -> Harness {
91-
match name {
92-
"claude" => Harness::Claude,
93-
"opencode" => Harness::OpenCode,
94-
"gemini" => Harness::Gemini,
95-
"oz" => Harness::Oz,
96-
other => {
97-
log::warn!("Unknown harness config name: {other:?}; treating as Unknown");
98-
Harness::Unknown
99-
}
100-
}
101-
}
102-
10386
fn serialize_harness<S: Serializer>(harness: &Harness, serializer: S) -> Result<S::Ok, S::Error> {
104-
serializer.serialize_str(&harness.to_string())
87+
serializer.serialize_str(harness.config_name())
10588
}
10689

10790
fn deserialize_harness<'de, D: Deserializer<'de>>(deserializer: D) -> Result<Harness, D::Error> {
10891
let name = String::deserialize(deserializer)?;
109-
Ok(harness_from_name(&name))
92+
let harness = Harness::from_config_name(&name);
93+
if matches!(harness, Harness::Unknown) && name != Harness::Unknown.config_name() {
94+
log::warn!("Unknown harness config name: {name:?}; treating as Unknown");
95+
}
96+
Ok(harness)
11097
}
11198

11299
/// Authentication secrets for third-party harnesses.

app/src/terminal/view/ambient_agent/model.rs

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -261,14 +261,10 @@ impl AmbientAgentViewModel {
261261
}
262262

263263
/// Returns the [`CLIAgent`] corresponding to the currently selected harness when it is a
264-
/// third-party harness (e.g. Claude, Gemini).
265-
/// Returns `None` for [`Harness::Oz`] and when [`FeatureFlag::AgentHarness`] is disabled.
264+
/// third-party harness (e.g. Claude, Gemini). Returns `None` for [`Harness::Oz`].
266265
/// Used to drive the correct tab icon for a cloud run as soon as a non-oz harness is
267266
/// selected, even before the CLI session is registered with [`CLIAgentSessionsModel`].
268267
pub fn selected_third_party_cli_agent(&self) -> Option<CLIAgent> {
269-
if !self.is_third_party_harness() {
270-
return None;
271-
}
272268
CLIAgent::from_harness(self.harness)
273269
}
274270

app/src/terminal/view/pane_impl.rs

Lines changed: 8 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -937,21 +937,19 @@ impl TerminalView {
937937
}
938938

939939
/// Returns the conversation status for display purposes, suppressing the status when the
940-
/// conversation is empty (no exchanges yet). This avoids showing a misleading "In progress"
941-
/// indicator when a new conversation hasn't started streaming, except when a shell command
942-
/// is actively long-running or the cloud environment is still spinning up — those
943-
/// InProgress states are real and should always surface.
940+
/// conversation is empty (no exchanges yet) AND nothing else makes the run "busy". This
941+
/// avoids showing a misleading "In progress" indicator on a brand-new conversation; real
942+
/// InProgress states (long-running shell commands, cloud-environment setup) come through
943+
/// because [`Self::selected_conversation_status`] surfaces them as `InProgress`.
944944
pub fn selected_conversation_status_for_display(
945945
&self,
946946
ctx: &AppContext,
947947
) -> Option<ConversationStatus> {
948-
if self.selected_conversation_is_empty(ctx)
949-
&& !self.is_long_running()
950-
&& !self.is_in_cloud_agent_setup_phase(ctx)
951-
{
952-
None
948+
let status = self.selected_conversation_status(ctx)?;
949+
if matches!(status, ConversationStatus::InProgress) || !self.selected_conversation_is_empty(ctx) {
950+
Some(status)
953951
} else {
954-
self.selected_conversation_status(ctx)
952+
None
955953
}
956954
}
957955

0 commit comments

Comments
 (0)