Skip to content

Commit bca4b2e

Browse files
Address review: clear a stale default host, repaint on window team change, cover the construction-time constraint
- The host-selector resync applied `Some` and swallowed `None`, so a window moving to a team that configures no default host kept the previous team's slug in both the selector and the run config. `clear_default_host` mirrors `set_default_host`, and the view model's host is now cleared too. - The Warp Agent settings page only repainted on `TeamsChanged`; `window_team_uids` is not `Tracked`, so a window team change would not repaint the attribution toggle once a team switcher lands. - Added a test that resolves a scope inside a view's own build closure, asserting the `ViewContext` shape works there and a self-handle resolves nothing. Every other test resolves post-construction, where the difference is invisible.
1 parent c288976 commit bca4b2e

5 files changed

Lines changed: 130 additions & 16 deletions

File tree

app/src/settings_view/warp_agent_page.rs

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -622,7 +622,13 @@ impl WarpAgentPageView {
622622

623623
let workspace = UserWorkspaces::handle(ctx);
624624
ctx.subscribe_to_model(&workspace, |me, _workspace, event, ctx| {
625-
if let UserWorkspacesEvent::TeamsChanged = event {
625+
// A window moving between teams changes what this page renders (the agent
626+
// attribution toggle resolves its policy from the window's team), and
627+
// `window_team_uids` is not `Tracked`, so autotracking will not repaint it.
628+
if matches!(
629+
event,
630+
UserWorkspacesEvent::TeamsChanged | UserWorkspacesEvent::WindowTeamChanged { .. }
631+
) {
626632
me.sync_custom_endpoint_buttons(ctx);
627633
ctx.notify();
628634
}

app/src/terminal/input.rs

Lines changed: 13 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -2412,17 +2412,21 @@ impl Input {
24122412
if !affects_this_window {
24132413
return;
24142414
}
2415+
// `None` has to be applied, not skipped: it means the window's team configures no
2416+
// self-hosted default, and leaving the previous value in place would keep the
2417+
// selector and the run config pointed at another team's worker.
24152418
let effective_host = effective_default_host(ctx);
2416-
if let Some(slug) = &effective_host {
2417-
view_for_ws.update(ctx, |selector, ctx| {
2418-
selector.set_default_host(slug.clone(), ctx);
2419-
});
2420-
}
2421-
if let Some(slug) = effective_host {
2422-
vm_for_ws.update(ctx, |model, _ctx| {
2423-
model.set_worker_host(Some(slug));
2424-
});
2419+
match effective_host.clone() {
2420+
Some(slug) => view_for_ws.update(ctx, |selector, ctx| {
2421+
selector.set_default_host(slug, ctx);
2422+
}),
2423+
None => view_for_ws.update(ctx, |selector, ctx| {
2424+
selector.clear_default_host(ctx);
2425+
}),
24252426
}
2427+
vm_for_ws.update(ctx, |model, _ctx| {
2428+
model.set_worker_host(effective_host);
2429+
});
24262430
});
24272431
view
24282432
}

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

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -208,6 +208,31 @@ impl HostSelector {
208208
self.refresh_menu(ctx);
209209
}
210210

211+
/// Drops the default host, e.g. because the window moved to a team that configures none.
212+
///
213+
/// The inverse of [`Self::set_default_host`] and it defers to a saved user selection the
214+
/// same way. Without it, a selection that came only from the previous team's default would
215+
/// survive the move and keep pointing at that team's self-hosted worker.
216+
pub fn clear_default_host(&mut self, ctx: &mut ViewContext<Self>) {
217+
if self.default_host.is_none() {
218+
return;
219+
}
220+
self.default_host = None;
221+
222+
let has_saved_selection = CloudAgentSettings::as_ref(ctx)
223+
.last_selected_host
224+
.value()
225+
.is_some();
226+
if !has_saved_selection {
227+
self.selected = Host::Warp;
228+
let label = self.selected.display_name().to_string();
229+
self.button.update(ctx, |button, ctx| {
230+
button.set_label(label, ctx);
231+
});
232+
}
233+
self.refresh_menu(ctx);
234+
}
235+
211236
/// Programmatically opens the host selector popover. No-op if already open.
212237
pub fn open_menu(&mut self, ctx: &mut ViewContext<Self>) {
213238
self.set_menu_visibility(true, ctx);

app/src/workspaces/user_workspaces.rs

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -216,8 +216,9 @@ impl TeamScope for TeamContextForOperation {
216216

217217
#[cfg(test)]
218218
impl TeamContextForOperation {
219-
// Nothing constructs a test context yet; remove this `#[allow(dead_code)]` once a Group 1
220-
// migration PR has a real call site.
219+
// Currently unused: tests that need an operation scope mint a real one from a test window
220+
// through `team_context_for_operation`, which exercises the production path as well. Kept
221+
// for a test that needs a scope with no window to mint it from.
221222
#[allow(dead_code)]
222223
pub(crate) fn new_for_test(team_uid: ServerId) -> Self {
223224
Self {
@@ -2030,10 +2031,8 @@ impl UserWorkspaces {
20302031
///
20312032
/// **Not a team-neutral read**, despite reading workspace settings: see
20322033
/// [`Self::teamless_workspace_settings`] for why. Sole remaining caller is
2033-
/// `ai::orchestration::resolve_default_host_slug`, which feeds the plan card, the
2034-
/// confirmation card, the TUI orchestration block and the handoff pipeline. That chain
2035-
/// still needs both a windowless accessor the TUI can reach and a pinned scope for the
2036-
/// handoff's chosen destination, so it moves as one follow-up rather than piecemeal.
2034+
/// `ai::orchestration::resolve_default_host_slug`, whose consumers have to migrate as one
2035+
/// unit and are blocked on decisions this getter cannot make.
20372036
/// Do not add callers: windowed code uses [`Self::default_host_slug_for_scope`], and a
20382037
/// windowless availability check uses [`Self::any_team_has_default_host_slug`].
20392038
pub fn unscoped_default_host_slug(&self) -> Option<&str> {

app/src/workspaces/user_workspaces_tests.rs

Lines changed: 80 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1056,6 +1056,86 @@ fn create_test_window(app: &mut App) -> (WindowId, ViewHandle<TeamContextTestVie
10561056
app.add_window(WindowStyle::NotStealFocus, |_| TeamContextTestView)
10571057
}
10581058

1059+
/// Records what a view can resolve about its own team *during its own construction*, which is
1060+
/// where the cloud-mode host selector's first read happens.
1061+
struct ConstructionTimeScopeProbe {
1062+
host_slug_from_view_context: Option<String>,
1063+
team_uid_from_own_handle: Option<ServerId>,
1064+
}
1065+
1066+
impl ConstructionTimeScopeProbe {
1067+
fn new(ctx: &mut ViewContext<Self>) -> Self {
1068+
let user_workspaces = UserWorkspaces::as_ref(ctx);
1069+
let scope = user_workspaces.team_context_for_operation(ctx);
1070+
Self {
1071+
host_slug_from_view_context: user_workspaces
1072+
.default_host_slug_for_scope(&scope)
1073+
.map(str::to_string),
1074+
team_uid_from_own_handle: user_workspaces
1075+
.team_context(&ctx.handle(), ctx)
1076+
.and_then(|context| context.team_uid()),
1077+
}
1078+
}
1079+
}
1080+
1081+
impl Entity for ConstructionTimeScopeProbe {
1082+
type Event = ();
1083+
}
1084+
1085+
impl View for ConstructionTimeScopeProbe {
1086+
fn ui_name() -> &'static str {
1087+
"ConstructionTimeScopeProbe"
1088+
}
1089+
1090+
fn render(&self, _: &AppContext) -> Box<dyn Element> {
1091+
Empty::new().finish()
1092+
}
1093+
}
1094+
1095+
impl TypedActionView for ConstructionTimeScopeProbe {
1096+
type Action = ();
1097+
}
1098+
1099+
/// A view is not in `view_to_window` until its build closure returns, so during its own
1100+
/// construction it can resolve its team through its `ViewContext` but *not* through a handle to
1101+
/// itself. Both halves are asserted: the shape the host selector uses works, and the shape it
1102+
/// would be tempting to refactor to silently resolves nothing. Every other test here resolves
1103+
/// post-construction, where the difference is invisible.
1104+
#[test]
1105+
fn test_a_view_can_only_resolve_its_own_team_through_its_view_context_while_constructing() {
1106+
let mut team = team_for_test();
1107+
team.settings.default_host_slug = Some("team-host".to_string());
1108+
let workspace = workspace_for_test(&team);
1109+
1110+
App::test((), |mut app| async move {
1111+
initialize_window_team_test_app(&mut app, vec![workspace]);
1112+
1113+
let (window_id, view) = create_test_window(&mut app);
1114+
UserWorkspaces::handle(&app).update(&mut app, |user_workspaces, ctx| {
1115+
user_workspaces.set_team_for_window(window_id, team.uid, ctx);
1116+
});
1117+
1118+
let probe = view.update(&mut app, |_, ctx| {
1119+
ctx.add_typed_action_view(ConstructionTimeScopeProbe::new)
1120+
});
1121+
1122+
app.read(|ctx| {
1123+
let probe = probe.as_ref(ctx);
1124+
assert_eq!(
1125+
probe.host_slug_from_view_context.as_deref(),
1126+
Some("team-host"),
1127+
"a view under construction still resolves its window's team through its \
1128+
ViewContext, whose window id is a plain field"
1129+
);
1130+
assert_eq!(
1131+
probe.team_uid_from_own_handle, None,
1132+
"a handle to a view still being constructed resolves no window, so reading the \
1133+
host through one would silently drop the team's configured host"
1134+
);
1135+
});
1136+
})
1137+
}
1138+
10591139
fn two_teams() -> (Team, Team) {
10601140
let team_a = team_for_test();
10611141
let mut team_b = team_for_test();

0 commit comments

Comments
 (0)