Skip to content

Implement auth secret deletion from selector menu #11241

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
abhishekp106 wants to merge 4 commits into
base: master
Choose a base branch
from
Open

Implement auth secret deletion from selector menu #11241

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
09a2310
Add auth secret deletion specs
abhishekp106 May 18, 2026
d837120
Implement auth secret deletion from selector menu
abhishekp106 May 18, 2026
e832e87
delete button working
abhishekp106 May 20, 2026
fdcd967
Working
abhishekp106 May 20, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 6 additions & 2 deletions app/src/ai/blocklist/inline_action/run_agents_card_view.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -483,14 +483,18 @@ impl RunAgentsCardView {
}
HarnessAvailabilityEvent::Changed
| HarnessAvailabilityEvent::AuthSecretsLoaded
| HarnessAvailabilityEvent::AuthSecretsFetchFailed => {
| HarnessAvailabilityEvent::AuthSecretsFetchFailed
| HarnessAvailabilityEvent::AuthSecretDeleted { .. } => {
// Repopulate even on fetch failure to replace "Loading…".
// Deleted events also force a repopulate so this card
// stops surfacing the deleted secret as an option.
oc::repopulate_all_pickers(&mut me.state.orch, &me.handles.pickers, ctx);
me.refresh_accept_button_state(ctx);
me.maybe_auto_open_create_modal(ctx);
ctx.notify();
}
HarnessAvailabilityEvent::AuthSecretCreationFailed { .. } => {}
HarnessAvailabilityEvent::AuthSecretCreationFailed { .. }
| HarnessAvailabilityEvent::AuthSecretDeletionFailed { .. } => {}
},
);

Expand Down
9 changes: 7 additions & 2 deletions app/src/ai/document/orchestration_config_block.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -266,15 +266,20 @@ impl OrchestrationConfigBlockView {
}
HarnessAvailabilityEvent::Changed
| HarnessAvailabilityEvent::AuthSecretsLoaded
| HarnessAvailabilityEvent::AuthSecretsFetchFailed => {
| HarnessAvailabilityEvent::AuthSecretsFetchFailed
| HarnessAvailabilityEvent::AuthSecretDeleted { .. } => {
// Repopulate even on fetch failure to replace "Loading…".
// The Deleted event also triggers a refresh so any
// already-mounted picker drops the deleted entry from
// its menu.
if me.pickers_initialized {
oc::repopulate_all_pickers(&mut me.edit_state, &me.pickers, ctx);
}
me.maybe_auto_open_create_modal(ctx);
ctx.notify();
}
HarnessAvailabilityEvent::AuthSecretCreationFailed { .. } => {}
HarnessAvailabilityEvent::AuthSecretCreationFailed { .. }
| HarnessAvailabilityEvent::AuthSecretDeletionFailed { .. } => {}
},
);

Expand Down
55 changes: 54 additions & 1 deletion app/src/ai/harness_availability.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@ pub enum AuthSecretFetchState {
#[derive(Debug, Clone)]
pub struct AuthSecretEntry {
pub name: String,
pub owner: SecretOwner,
}

pub enum HarnessAvailabilityEvent {
Expand All @@ -80,6 +81,15 @@ pub enum HarnessAvailabilityEvent {
AuthSecretCreationFailed {
error: String,
},
AuthSecretDeleted {
harness: Harness,
name: String,
},
AuthSecretDeletionFailed {
harness: Harness,
name: String,
error: String,
},
}

pub struct HarnessAvailabilityModel {
Expand Down Expand Up @@ -209,7 +219,10 @@ impl HarnessAvailabilityModel {
RequestState::RequestSucceeded(secrets) => {
let entries = secrets
.into_iter()
.map(|s| AuthSecretEntry { name: s.name })
.map(|s| AuthSecretEntry {
owner: secret_owner_from_space(&s.owner),
name: s.name,
})
.collect();
me.auth_secrets
.insert(harness, AuthSecretFetchState::Loaded(entries));
Expand Down Expand Up @@ -261,6 +274,7 @@ impl HarnessAvailabilityModel {
Ok(secret) => {
let entry = AuthSecretEntry {
name: secret.name.clone(),
owner: secret_owner_from_space(&secret.owner),
};
match me.auth_secrets.get_mut(&harness) {
Some(AuthSecretFetchState::Loaded(entries)) => {
Expand All @@ -284,6 +298,36 @@ impl HarnessAvailabilityModel {
});
}

pub fn delete_auth_secret(
&mut self,
harness: Harness,
name: String,
owner: SecretOwner,
ctx: &mut ModelContext<Self>,
) {
let manager = ManagedSecretManager::handle(ctx);
let delete_future = manager.as_ref(ctx).delete_secret(owner, name.clone());
ctx.spawn(delete_future, move |me, result, ctx| match result {
Ok(()) => {
if let Some(AuthSecretFetchState::Loaded(entries)) =
me.auth_secrets.get_mut(&harness)
{
entries.retain(|entry| entry.name != name);
Copy link
Copy Markdown
Contributor

@oz-for-oss oz-for-oss Bot May 20, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ [IMPORTANT] Deletion is scoped by (owner, name), but this removes every cached secret with the same name for the harness. If a user and team secret share a name, deleting one hides both locally until a refetch; retain by both name and owner and include owner in the deletion event/state.

Copy link
Copy Markdown
Contributor

@liliwilson liliwilson May 21, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤨 it seems after a quick look in warp-server that this statement about uniqueness is, in fact, correct

}
ctx.emit(HarnessAvailabilityEvent::AuthSecretDeleted { harness, name });
}
Err(e) => {
let msg = e.to_string();
report_error!(e.context("Failed to delete harness auth secret"));
ctx.emit(HarnessAvailabilityEvent::AuthSecretDeletionFailed {
harness,
name,
error: msg,
});
}
});
}

pub fn refresh(&self, ctx: &mut ModelContext<Self>) {
// The endpoint queries `user`, which requires auth.
if !AuthStateProvider::as_ref(ctx).get().is_logged_in() {
Expand Down Expand Up @@ -333,6 +377,15 @@ fn get_cached(ctx: &ModelContext<HarnessAvailabilityModel>) -> Option<Vec<Harnes
serde_json::from_str::<Vec<HarnessAvailability>>(&raw).ok()
}

fn secret_owner_from_space(space: &warp_graphql::object::Space) -> SecretOwner {
match space.type_ {
warp_graphql::object::SpaceType::Team => SecretOwner::Team {
team_uid: space.uid.clone().into_inner(),
},
warp_graphql::object::SpaceType::User => SecretOwner::CurrentUser,
}
}

fn harness_to_graphql_harness(harness: Harness) -> Option<warp_graphql::ai::AgentHarness> {
match harness {
Harness::Oz => Some(warp_graphql::ai::AgentHarness::Oz),
Expand Down
127 changes: 112 additions & 15 deletions app/src/menu.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,10 @@ use std::{fmt, vec};
use crate::safe_triangle::SafeTriangle;
use crate::themes::theme::Fill;
use crate::util::time_format::format_approx_duration_from_now_sentence_case;
use crate::{appearance::Appearance, ui_components::icons};
use crate::{
appearance::Appearance,
ui_components::{buttons::icon_button_with_color, icons},
};
use chrono::{DateTime, Local};
use pathfinder_color::ColorU;
use pathfinder_geometry::rect::RectF;
Expand Down Expand Up @@ -416,6 +419,20 @@ pub struct MenuItemFields<A: Action + Clone> {
tooltip_position: MenuTooltipPosition,
right_side_label: Option<RightSideLabel>,
right_side_icon: Option<(icons::Icon, Option<Fill>)>,
/// Optional action dispatched when the right-side icon is clicked. When
/// set, the right-side icon becomes its own hit target: clicking it
/// dispatches this action without firing the row's own `on_select_action`,
/// and prevents the row click from propagating.
right_side_icon_action: Option<A>,
/// Optional accessibility label for the right-side icon hit target.
right_side_icon_a11y_label: Option<String>,
/// When true, the right-side icon is rendered as disabled (no hover,
/// no click action). Used to lock the affordance while a pending
/// request is in flight.
right_side_icon_disabled: bool,
/// Optional mouse state for the right-side icon so its hover state is
/// tracked independently from the row.
right_side_icon_mouse_state: MouseStateHandle,
Comment on lines 421 to +435
Copy link
Copy Markdown
Contributor

@liliwilson liliwilson May 21, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What do you think about bundling these 6 fields into a RightSideIconConfig?

struct RightSideIconConfig<A> {
    icon: icons::Icon,
    override_color: Option<Fill>,
    action: Option<A>,
    a11y_label: Option<String>,
    disabled: bool,
    mouse_state: MouseStateHandle,
}

/// Optional override for the background color
/// hovered or selected. When `None`, the default hover/selected background
/// from the theme is used (accent or dark overlay, depending on
Expand Down Expand Up @@ -467,6 +484,10 @@ impl<A: Action + Clone> MenuItemFields<A> {
tooltip_position: MenuTooltipPosition::default(),
right_side_label: None,
right_side_icon: None,
right_side_icon_action: None,
right_side_icon_a11y_label: None,
right_side_icon_disabled: false,
right_side_icon_mouse_state: MouseStateHandle::default(),
override_hover_background_color: None,
icon_size_override: None,
clip_config: None,
Expand Down Expand Up @@ -497,6 +518,10 @@ impl<A: Action + Clone> MenuItemFields<A> {
tooltip_position: MenuTooltipPosition::default(),
right_side_label: None,
right_side_icon: None,
right_side_icon_action: None,
right_side_icon_a11y_label: None,
right_side_icon_disabled: false,
right_side_icon_mouse_state: MouseStateHandle::default(),
override_hover_background_color: None,
icon_size_override: None,
clip_config: None,
Expand Down Expand Up @@ -530,6 +555,10 @@ impl<A: Action + Clone> MenuItemFields<A> {
tooltip_position: MenuTooltipPosition::default(),
right_side_label: None,
right_side_icon: None,
right_side_icon_action: None,
right_side_icon_a11y_label: None,
right_side_icon_disabled: false,
right_side_icon_mouse_state: MouseStateHandle::default(),
override_hover_background_color: None,
icon_size_override: None,
clip_config: None,
Expand Down Expand Up @@ -566,6 +595,10 @@ impl<A: Action + Clone> MenuItemFields<A> {
tooltip_position: MenuTooltipPosition::default(),
right_side_label: None,
right_side_icon: None,
right_side_icon_action: None,
right_side_icon_a11y_label: None,
right_side_icon_disabled: false,
right_side_icon_mouse_state: MouseStateHandle::default(),
override_hover_background_color: None,
icon_size_override: None,
clip_config: None,
Expand Down Expand Up @@ -600,6 +633,10 @@ impl<A: Action + Clone> MenuItemFields<A> {
tooltip_position: MenuTooltipPosition::default(),
right_side_label: None,
right_side_icon: None,
right_side_icon_action: None,
right_side_icon_a11y_label: None,
right_side_icon_disabled: false,
right_side_icon_mouse_state: MouseStateHandle::default(),
override_hover_background_color: None,
icon_size_override: None,
clip_config: None,
Expand Down Expand Up @@ -633,6 +670,10 @@ impl<A: Action + Clone> MenuItemFields<A> {
tooltip_position: MenuTooltipPosition::default(),
right_side_label: None,
right_side_icon: None,
right_side_icon_action: None,
right_side_icon_a11y_label: None,
right_side_icon_disabled: false,
right_side_icon_mouse_state: MouseStateHandle::default(),
override_hover_background_color: None,
icon_size_override: None,
clip_config: None,
Expand Down Expand Up @@ -663,6 +704,10 @@ impl<A: Action + Clone> MenuItemFields<A> {
tooltip_position: MenuTooltipPosition::default(),
right_side_label: None,
right_side_icon: None,
right_side_icon_action: None,
right_side_icon_a11y_label: None,
right_side_icon_disabled: false,
right_side_icon_mouse_state: MouseStateHandle::default(),
override_hover_background_color: None,
icon_size_override: None,
clip_config: None,
Expand Down Expand Up @@ -709,6 +754,14 @@ impl<A: Action + Clone> MenuItemFields<A> {
tooltip_position: self.tooltip_position,
right_side_label: self.right_side_label,
right_side_icon: self.right_side_icon,
// The right-side icon action is `Option<A>`; we can't safely
// map it to `Option<B>` here, so drop it. Callers that need
// the right-side action must set it via
// `with_right_side_icon_action` after conversion.
right_side_icon_action: None,
right_side_icon_a11y_label: self.right_side_icon_a11y_label,
right_side_icon_disabled: self.right_side_icon_disabled,
right_side_icon_mouse_state: self.right_side_icon_mouse_state,
override_hover_background_color: self.override_hover_background_color,
icon_size_override: self.icon_size_override,
clip_config: self.clip_config,
Expand Down Expand Up @@ -848,6 +901,25 @@ impl<A: Action + Clone> MenuItemFields<A> {
self
}

/// Sets a separate action that fires when the right-side icon is
/// clicked. The action is independent from the row's
/// `on_select_action`: clicking the icon dispatches this action and
/// the row click is suppressed.
pub fn with_right_side_icon_action(mut self, action: A) -> Self {
self.right_side_icon_action = Some(action);
self
}

pub fn with_right_side_icon_a11y_label(mut self, label: impl Into<String>) -> Self {
self.right_side_icon_a11y_label = Some(label.into());
self
}

pub fn with_right_side_icon_disabled(mut self, disabled: bool) -> Self {
self.right_side_icon_disabled = disabled;
self
}

pub fn into_item(self) -> MenuItem<A> {
MenuItem::Item(self)
}
Expand Down Expand Up @@ -984,26 +1056,49 @@ impl<A: Action + Clone> MenuItemFields<A> {
&self,
appearance: &Appearance,
color: Fill,
dispatch_item_actions: bool,
) -> Option<Box<dyn Element>> {
let (icon, override_color) = self.right_side_icon.as_ref()?;
let icon_size = self
.icon_size_override
.unwrap_or_else(|| appearance.ui_font_size());
let icon_color = override_color.unwrap_or(color);
Some(
Shrinkable::new(
1.,
Container::new(
ConstrainedBox::new(icon.to_warpui_icon(icon_color).finish())
.with_width(icon_size)
.with_height(icon_size)
.finish(),
)
if let Some(action) = &self.right_side_icon_action {
let mut button = icon_button_with_color(
appearance,
icon.clone(),
false,
self.right_side_icon_mouse_state.clone(),
icon_color,
);
if self.right_side_icon_disabled {
button = button.disabled();
}
let mut hoverable = button.build();
if !self.right_side_icon_disabled {
let action = action.clone();
hoverable = hoverable.on_click(move |ctx, _, _| {
if dispatch_item_actions {
ctx.dispatch_typed_action(action.clone());
}
});
// Swallow mouse-down too so the row's click handler
// doesn't latch onto the press that targets the icon.
hoverable = hoverable.on_mouse_down(|_, _, _| {});
Copy link
Copy Markdown
Contributor

@oz-for-oss oz-for-oss Bot May 20, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ [IMPORTANT] This does not stop the parent row Hoverable from also processing the click: parent hoverables only skip child-handled events when with_defer_events_to_children() is set, and the disabled icon path installs no handler at all. Clicking the X can still select/close the row instead of only opening delete confirmation; make the row defer to the icon hit target and consume disabled-icon clicks too.

Copy link
Copy Markdown
Contributor

@liliwilson liliwilson May 21, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This also seems legit - if I click the "x" and then click cancel, the key I "x"ed is now selected

}
let element = Container::new(hoverable.finish())
.with_margin_left(icon_size / 2.)
.finish(),
)
.finish(),
)
.finish();
return Some(Shrinkable::new(1., Align::new(element).right().finish()).finish());
}
let icon_element = ConstrainedBox::new(icon.to_warpui_icon(icon_color).finish())
.with_width(icon_size)
.with_height(icon_size)
.finish();
let container = Container::new(icon_element)
.with_margin_left(icon_size / 2.)
.finish();
Some(Shrinkable::new(1., container).finish())
}

fn render_right_aligned_chevron(
Expand Down Expand Up @@ -1197,7 +1292,9 @@ impl<A: Action + Clone> MenuItemFields<A> {
));
}

if let Some(right_icon) = self.render_right_side_icon(appearance, primary_color) {
if let Some(right_icon) =
self.render_right_side_icon(appearance, primary_color, dispatch_item_actions)
{
label_row.add_child(right_icon);
}
}
Expand Down
Loading
Loading