From bfe38a208c7cbaab588181aada0baa8a49133d83 Mon Sep 17 00:00:00 2001 From: abhishekp106 Date: Mon, 18 May 2026 22:34:13 +0000 Subject: [PATCH 1/8] Add auth secret deletion specs Co-Authored-By: Oz --- .../PRODUCT.md | 63 ++++++++ specs/cloud-mode-auth-secret-deletion/TECH.md | 147 ++++++++++++++++++ 2 files changed, 210 insertions(+) create mode 100644 specs/cloud-mode-auth-secret-deletion/PRODUCT.md create mode 100644 specs/cloud-mode-auth-secret-deletion/TECH.md diff --git a/specs/cloud-mode-auth-secret-deletion/PRODUCT.md b/specs/cloud-mode-auth-secret-deletion/PRODUCT.md new file mode 100644 index 0000000000..d2423c7e5a --- /dev/null +++ b/specs/cloud-mode-auth-secret-deletion/PRODUCT.md @@ -0,0 +1,63 @@ +# Auth Secret Deletion From Selector — Product Spec + +## Summary + +Users can delete Warp-managed harness auth secrets directly from the auth secret selector chip menu. The delete affordance appears as a right-aligned `X` button on each existing secret row, removes the secret from the server, updates the menu state, and confirms success or failure with a toast. + +## Figma + +Figma: none provided. The prompt includes a screenshot of the current auth selector menu and requests adding a right-aligned `X` button to each non-New submenu item. + +## Behavior + +1. The feature applies to the auth secret selector chip menu shown for non-Oz cloud harnesses after auth-secret FTUX has been completed. + +2. When the auth secret selector chip menu opens, each existing Warp-managed secret row shows: + - The secret name on the left. + - A right-aligned `X` button on the same row. + - Hover, selected, spacing, icon, and text styling consistent with existing Warp menus. + +3. The `X` button appears only on existing managed-secret rows. It does not appear on: + - The header row. + - The "Inherit key from environment" row. + - Loading or error placeholder rows. + - The "New" row. + - Secret-type rows in the "New" sidecar submenu. + +4. Selecting the secret row outside the `X` button preserves current behavior: the row becomes the selected auth secret for the active harness, the per-harness selection is persisted, and the menu closes. + +5. Selecting the `X` button deletes that specific secret. It must not select the secret first, must not open the "New" sidecar, and must not accidentally dispatch the row's normal select action. + +6. Deletion is server-backed. A secret is considered deleted only after the server delete request succeeds. The UI must not present deletion as successful before the server confirms it. + +7. While deletion is pending, the user should not be able to fire duplicate delete requests for the same secret from the same open menu. Acceptable treatments include disabling the `X` button for that row, closing the menu, or otherwise preventing a second click. + +8. On successful deletion: + - The deleted secret disappears from the selector menu the next time the menu is shown, and ideally immediately if the menu remains open. + - A success toast is shown. + - If the deleted secret was currently selected for the active harness, the selected auth secret is cleared and the chip falls back to the no-secret/inherit label. + - Any persisted last-selected secret for the active harness is cleared if it points to the deleted secret. + +9. On failed deletion: + - The secret remains available in the selector menu. + - The current selected secret does not change. + - A failure toast is shown. + - The user can retry deletion after the failure. + +10. If the deleted secret is selected by another visible selector instance, that selector should refresh when it observes updated harness secret state. It must not keep showing a deleted secret as a valid selectable item after the secret list is refreshed. + +11. If the secret list is stale and the server reports that the secret no longer exists, the delete attempt is treated as a failure unless the server explicitly reports the delete as successful. The failure toast should use the server/user-facing error text when available. + +12. If the user is offline or unauthenticated when pressing `X`, the delete fails and shows a failure toast. The menu must not remove the secret locally unless a later server response confirms deletion. + +13. The success and failure toasts are ephemeral, consistent with nearby app toasts, and do not require a custom action button. + +14. Keyboard behavior must remain usable: + - Existing keyboard navigation for selecting a secret row remains unchanged. + - Adding the `X` button must not make the menu trap focus or prevent Escape/close behavior. + - If the `X` button is reachable by keyboard, it must have an accessible label such as "Delete API key". + +15. The menu layout remains stable for long secret names. Long names should truncate or shrink according to existing menu behavior rather than overlapping the right-aligned `X` button. + +16. The feature does not add editing, renaming, creating, or bulk-deleting secrets. It only deletes one existing secret at a time from the auth selector menu. + diff --git a/specs/cloud-mode-auth-secret-deletion/TECH.md b/specs/cloud-mode-auth-secret-deletion/TECH.md new file mode 100644 index 0000000000..1944b7bd79 --- /dev/null +++ b/specs/cloud-mode-auth-secret-deletion/TECH.md @@ -0,0 +1,147 @@ +# Auth Secret Deletion From Selector — Tech Spec + +## Context + +This spec implements the behavior in `PRODUCT.md` for the ambient cloud-mode auth selector chip. + +Relevant current code: + +- `app/src/terminal/view/ambient_agent/auth_secret_selector.rs:61` defines `AuthSecretSelectorAction`; it currently supports toggling the menu, selecting a secret, clearing/inheriting, opening the "New" sidecar, and selecting a new-secret type. +- `app/src/terminal/view/ambient_agent/auth_secret_selector.rs:147` subscribes the selector to `HarnessAvailabilityModel` auth-secret load/create/fetch-failure events and refreshes the menu/button. +- `app/src/terminal/view/ambient_agent/auth_secret_selector.rs:389` builds the menu. Loaded secrets are rendered as `MenuItem::Item` rows with `SelectSecret(name)` actions; the "New" row is separate and opens the sidecar. +- `app/src/terminal/view/ambient_agent/auth_secret_selector.rs:498` handles selector actions and persists selected/cleared secrets in `CloudAgentSettings.last_selected_auth_secret`. +- `app/src/ai/harness_availability.rs:55` stores per-harness auth secret fetch state as `AuthSecretFetchState::Loaded(Vec)`. +- `app/src/ai/harness_availability.rs:184` fetches harness auth secrets from `ManagedSecretsClient::list_harness_auth_secrets`. +- `app/src/ai/harness_availability.rs:250` already centralizes secret creation through `ManagedSecretManager`, updates the local cache, and emits create/failure events. +- `crates/managed_secrets/src/manager.rs:87` exposes `ManagedSecretManager::delete_secret(owner, name)`. +- `app/src/server/server_api/managed_secrets.rs:132` wires `delete_managed_secret` to the server GraphQL mutation. +- `app/src/menu.rs:394` defines reusable `MenuItemFields`. It already has right-side label/icon rendering, but right-side icons are decorative and do not dispatch a separate action from the row. + +The current `AuthSecretEntry` contains only `name`, but server `ManagedSecret` results include `owner`. Deleting team-owned secrets correctly requires preserving enough owner information to call `delete_secret` with `SecretOwner::Team { team_uid }` instead of always assuming `CurrentUser`. + +## Proposed Changes + +1. Extend auth-secret cache entries with owner metadata. + + Update `AuthSecretEntry` in `app/src/ai/harness_availability.rs` to include `owner: SecretOwner`. Convert `warp_graphql::object::Space` from fetched/created `ManagedSecret` values into `SecretOwner`: + + - `SpaceType::User` -> `SecretOwner::CurrentUser` + - `SpaceType::Team` -> `SecretOwner::Team { team_uid: space.uid.into_inner() }` + + This keeps deletion ownership close to the existing fetch/create cache and avoids guessing owner at click time. + +2. Add deletion to `HarnessAvailabilityModel`. + + Add a method similar to `create_auth_secret`: + + ```rust + pub fn delete_auth_secret( + &mut self, + harness: Harness, + name: String, + owner: SecretOwner, + ctx: &mut ModelContext, + ) + ``` + + The method should call `ManagedSecretManager::delete_secret(owner, name.clone())`. On success, remove the matching entry from `AuthSecretFetchState::Loaded` for that harness and emit a new event. On failure, leave the cache unchanged, report the error, and emit a failure event with the harness/name/error. + + Add events: + + - `AuthSecretDeleted { harness, name }` + - `AuthSecretDeletionFailed { harness, name, error }` + + Existing subscribers that only care about menu freshness should refresh on `AuthSecretDeleted`; subscribers that do not care should explicitly ignore both new events so match exhaustiveness remains clear. + +3. Add a selector action for deleting a secret. + + Extend `AuthSecretSelectorAction` with enough data to delete the row, for example: + + ```rust + DeleteSecret { + name: String, + owner: SecretOwner, + } + ``` + + Track pending deletes in `AuthSecretSelector` with a small set keyed by `(harness, name, owner)` or by a stable local row id if one is introduced. This satisfies PRODUCT behavior 7 and avoids duplicate delete requests from the same menu. + +4. Render the right-aligned `X` affordance using the menu system's existing styling. + + Preferred implementation: extend `MenuItemFields` with an optional right-side action, keeping the visual rendering path for `with_right_side_icon(Icon::X)` consistent with other menus while allowing the right icon to dispatch a different action than the row. + + The right-side action should: + + - Stop mouse event propagation so clicking `X` does not also dispatch `SelectSecret`. + - Close the menu or disable the row while the delete request is pending. + - Preserve the existing row action when the user clicks anywhere outside the `X`. + + If changing `MenuItemFields` is too broad during implementation, an acceptable alternative is a custom-label row in `auth_secret_selector.rs` that composes the existing text/icon primitives and explicitly separates row-click from `X` click. Prefer the reusable menu extension only if it stays small and does not alter existing right-side icon behavior. + +5. Wire deletion action handling in `AuthSecretSelector`. + + In the `DeleteSecret` branch: + + - Read the active harness from `AmbientAgentViewModel`. + - Mark the row pending. + - Call `HarnessAvailabilityModel::delete_auth_secret(harness, name, owner, ctx)`. + - Close the menu or refresh it into a disabled pending state. + + In the selector's `HarnessAvailabilityModel` subscription: + + - On `AuthSecretDeleted` for the active harness, clear pending state for the matching secret, refresh menu/button, and show a success toast. + - If the deleted name equals `AmbientAgentViewModel::selected_harness_auth_secret_name()`, call `set_harness_auth_secret_name(None, ctx)`. + - Remove `CloudAgentSettings.last_selected_auth_secret[harness.config_name()]` if it equals the deleted name. + - On `AuthSecretDeletionFailed`, clear pending state, refresh menu, and show a failure toast containing the user-facing error when available. + + Use `DismissibleToast::success(...)` and `DismissibleToast::error(...)` via `ToastStack::add_ephemeral_toast`, matching nearby toast patterns. + +6. Keep adjacent auth-secret surfaces consistent. + + `AuthSecretFtuxDropdown`, orchestration pickers, and model selector subscribers must compile with the new deletion events. They can ignore failure events and refresh on success where they render auth secret lists. This ensures a secret deleted from the chip no longer appears in other already-mounted selectors after the model emits. + +## Testing and Validation + +1. Unit-test or view-model-test the cache update path for PRODUCT behaviors 6, 8, and 9: + - Successful deletion removes only the matching auth secret from the loaded list and emits `AuthSecretDeleted`. + - Failed deletion leaves the list unchanged and emits `AuthSecretDeletionFailed`. + - Team-owned entries call delete with `SecretOwner::Team`, user-owned entries call delete with `SecretOwner::CurrentUser`. + +2. Test `AuthSecretSelector` action behavior for PRODUCT behaviors 4, 5, 7, and 8: + - Clicking/selecting the row still selects the secret. + - Dispatching the delete action does not select the secret. + - Duplicate delete dispatches for a pending secret are ignored. + - Deleting the currently selected secret clears the selected model value and persisted setting. + +3. Add or update menu rendering tests if the reusable `MenuItemFields` right-side action API is introduced: + - Existing right-side icon rendering remains decorative when no right-side action is set. + - A right-side action dispatches independently from the row action. + +4. Manual validation against the screenshot state: + - Open the auth selector chip menu with multiple secrets. + - Confirm only existing secret rows have a right-aligned `X`. + - Confirm the "New" row and sidecar rows do not show an `X`. + - Delete an unselected secret and verify it disappears plus a success toast appears. + - Delete the selected secret and verify the chip label falls back to no-secret/inherit state. + - Force a delete failure/offline state and verify the failure toast appears and the row remains. + +5. Run focused checks: + + ```sh + cargo check -p warp_app + cargo test -p warp_app auth_secret + ``` + + Adjust package/test targets to the repository's current presubmit conventions if those commands are too broad or renamed. + +## Parallelization + +Parallel agents are not recommended. The work is small and tightly coupled across one UI view, one shared model, and a small optional menu extension; splitting would increase merge risk around action/event names without materially reducing wall-clock time. + +## Risks and Mitigations + +- **Accidental row selection when clicking `X`:** stop event propagation on the `X` hit target and test the separate action path. +- **Deleting the wrong owner scope:** store `SecretOwner` on each fetched `AuthSecretEntry`; do not infer owner from the active workspace at click time. +- **Stale selected secret after deletion:** clear both `AmbientAgentViewModel` selection and `CloudAgentSettings.last_selected_auth_secret` when the deleted name matches the active harness selection. +- **Over-broad menu API changes:** keep any `MenuItemFields` addition opt-in so existing menus with right-side icons remain visually and behaviorally unchanged. + From a251bff68069e17abfb5cd3a45354063befa899a Mon Sep 17 00:00:00 2001 From: abhishekp106 Date: Mon, 18 May 2026 22:56:49 +0000 Subject: [PATCH 2/8] Implement auth secret deletion from selector menu Adds a right-aligned X button on each Warp-managed secret row in the auth selector chip menu (non-Oz cloud harnesses). Clicking it deletes the secret via ManagedSecretManager, surfaces a success or failure toast, and clears the selection (and persisted last-selected) when the deleted secret was the active one. Duplicate clicks while a delete is in flight are dropped. - Extends AuthSecretEntry with SecretOwner so per-secret deletes use the correct ownership scope (User vs Team) rather than guessing at click time. - Adds HarnessAvailabilityModel::delete_auth_secret plus AuthSecretDeleted / AuthSecretDeletionFailed events, and updates all match-exhaustive subscribers (FTUX, model selector, orchestration pickers, run-agents card) to refresh on Deleted. - Adds an opt-in right-side action hit target to MenuItemFields so the X dispatches a separate action and stops the row's select click from firing alongside it. Existing right-side icons remain decorative. - Derives PartialEq/Eq/Hash on SecretOwner so it can ride along on the selector's PartialEq action enum. Co-Authored-By: Oz --- .../inline_action/run_agents_card_view.rs | 8 +- .../ai/document/orchestration_config_block.rs | 9 +- app/src/ai/harness_availability.rs | 55 +++++++- app/src/menu.rs | 121 ++++++++++++++--- .../auth_secret_ftux_dropdown.rs | 4 +- .../ambient_agent/auth_secret_ftux_view.rs | 4 +- .../ambient_agent/auth_secret_selector.rs | 126 +++++++++++++++++- .../view/ambient_agent/model_selector.rs | 4 +- crates/managed_secrets/src/client.rs | 2 +- 9 files changed, 308 insertions(+), 25 deletions(-) diff --git a/app/src/ai/blocklist/inline_action/run_agents_card_view.rs b/app/src/ai/blocklist/inline_action/run_agents_card_view.rs index 242aff1857..7fd9bf9b90 100644 --- a/app/src/ai/blocklist/inline_action/run_agents_card_view.rs +++ b/app/src/ai/blocklist/inline_action/run_agents_card_view.rs @@ -435,14 +435,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 { .. } => {} }, ); diff --git a/app/src/ai/document/orchestration_config_block.rs b/app/src/ai/document/orchestration_config_block.rs index 3bece61057..09663427d5 100644 --- a/app/src/ai/document/orchestration_config_block.rs +++ b/app/src/ai/document/orchestration_config_block.rs @@ -281,15 +281,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 { .. } => {} }, ); diff --git a/app/src/ai/harness_availability.rs b/app/src/ai/harness_availability.rs index 358583af2a..f4efd016cd 100644 --- a/app/src/ai/harness_availability.rs +++ b/app/src/ai/harness_availability.rs @@ -64,6 +64,7 @@ pub enum AuthSecretFetchState { #[derive(Debug, Clone)] pub struct AuthSecretEntry { pub name: String, + pub owner: SecretOwner, } pub enum HarnessAvailabilityEvent { @@ -81,6 +82,15 @@ pub enum HarnessAvailabilityEvent { AuthSecretCreationFailed { error: String, }, + AuthSecretDeleted { + harness: Harness, + name: String, + }, + AuthSecretDeletionFailed { + harness: Harness, + name: String, + error: String, + }, } pub struct HarnessAvailabilityModel { @@ -210,7 +220,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)); @@ -262,6 +275,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)) => { @@ -285,6 +299,36 @@ impl HarnessAvailabilityModel { }); } + pub fn delete_auth_secret( + &mut self, + harness: Harness, + name: String, + owner: SecretOwner, + ctx: &mut ModelContext, + ) { + 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); + } + 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) { // The endpoint queries `user`, which requires auth. if !AuthStateProvider::as_ref(ctx).get().is_logged_in() { @@ -334,6 +378,15 @@ fn get_cached(ctx: &ModelContext) -> Option>(&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 { match harness { Harness::Oz => Some(warp_graphql::ai::AgentHarness::Oz), diff --git a/app/src/menu.rs b/app/src/menu.rs index 83ab44b75f..4ae2378a98 100644 --- a/app/src/menu.rs +++ b/app/src/menu.rs @@ -418,6 +418,20 @@ pub struct MenuItemFields { tooltip_position: MenuTooltipPosition, right_side_label: Option, right_side_icon: Option<(icons::Icon, Option)>, + /// 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, + /// Optional accessibility label for the right-side icon hit target. + right_side_icon_a11y_label: Option, + /// 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, /// 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 @@ -469,6 +483,10 @@ impl MenuItemFields { 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, @@ -499,6 +517,10 @@ impl MenuItemFields { 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, @@ -532,6 +554,10 @@ impl MenuItemFields { 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, @@ -568,6 +594,10 @@ impl MenuItemFields { 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, @@ -602,6 +632,10 @@ impl MenuItemFields { 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, @@ -635,6 +669,10 @@ impl MenuItemFields { 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, @@ -665,6 +703,10 @@ impl MenuItemFields { 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, @@ -711,6 +753,14 @@ impl MenuItemFields { 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`; we can't safely + // map it to `Option` 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, @@ -850,6 +900,25 @@ impl MenuItemFields { 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) -> 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 { MenuItem::Item(self) } @@ -986,26 +1055,46 @@ impl MenuItemFields { &self, appearance: &Appearance, color: Fill, + dispatch_item_actions: bool, ) -> Option> { 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(), - ) - .with_margin_left(icon_size / 2.) - .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(); + + // If a separate right-side icon action is set, wrap the icon in + // its own `Hoverable` so it can dispatch independently of the row. + // The child `Hoverable` consumes the click event, preventing the + // row's `on_select_action` from firing alongside it. + let element: Box = if let Some(action) = &self.right_side_icon_action { + let mut hoverable = + Hoverable::new(self.right_side_icon_mouse_state.clone(), |_| container); + if !self.right_side_icon_disabled { + let action = action.clone(); + hoverable = hoverable.with_cursor(Cursor::PointingHand); + 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(|_, _, _| {}); + } + hoverable.finish() + } else { + container + }; + + Some(Shrinkable::new(1., element).finish()) } fn render_right_aligned_chevron( @@ -1199,7 +1288,9 @@ impl MenuItemFields { )); } - 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); } } diff --git a/app/src/terminal/view/ambient_agent/auth_secret_ftux_dropdown.rs b/app/src/terminal/view/ambient_agent/auth_secret_ftux_dropdown.rs index 8bbe751e6d..24b964c11d 100644 --- a/app/src/terminal/view/ambient_agent/auth_secret_ftux_dropdown.rs +++ b/app/src/terminal/view/ambient_agent/auth_secret_ftux_dropdown.rs @@ -123,12 +123,14 @@ impl AuthSecretFtuxDropdown { |me, _, event, ctx| match event { HarnessAvailabilityEvent::AuthSecretsLoaded | HarnessAvailabilityEvent::AuthSecretCreated { .. } + | HarnessAvailabilityEvent::AuthSecretDeleted { .. } | HarnessAvailabilityEvent::AuthSecretsFetchFailed => { me.refresh_menu(ctx); ctx.notify(); } HarnessAvailabilityEvent::Changed - | HarnessAvailabilityEvent::AuthSecretCreationFailed { .. } => {} + | HarnessAvailabilityEvent::AuthSecretCreationFailed { .. } + | HarnessAvailabilityEvent::AuthSecretDeletionFailed { .. } => {} }, ); diff --git a/app/src/terminal/view/ambient_agent/auth_secret_ftux_view.rs b/app/src/terminal/view/ambient_agent/auth_secret_ftux_view.rs index 51037a14c0..9a3780941f 100644 --- a/app/src/terminal/view/ambient_agent/auth_secret_ftux_view.rs +++ b/app/src/terminal/view/ambient_agent/auth_secret_ftux_view.rs @@ -224,7 +224,9 @@ impl AuthSecretFtuxView { } HarnessAvailabilityEvent::Changed | HarnessAvailabilityEvent::AuthSecretsLoaded - | HarnessAvailabilityEvent::AuthSecretsFetchFailed => {} + | HarnessAvailabilityEvent::AuthSecretsFetchFailed + | HarnessAvailabilityEvent::AuthSecretDeleted { .. } + | HarnessAvailabilityEvent::AuthSecretDeletionFailed { .. } => {} }, ); diff --git a/app/src/terminal/view/ambient_agent/auth_secret_selector.rs b/app/src/terminal/view/ambient_agent/auth_secret_selector.rs index 8b5abed2b9..03936f7c25 100644 --- a/app/src/terminal/view/ambient_agent/auth_secret_selector.rs +++ b/app/src/terminal/view/ambient_agent/auth_secret_selector.rs @@ -1,3 +1,4 @@ +use std::collections::HashSet; use std::sync::Arc; use pathfinder_geometry::vector::vec2f; @@ -6,6 +7,7 @@ use warp_cli::agent::Harness; use warp_core::ui::appearance::Appearance; use warp_core::ui::theme::color::internal_colors; use warp_core::ui::theme::Fill; +use warp_managed_secrets::client::SecretOwner; use warpui::elements::{ Border, ChildAnchor, ChildView, OffsetPositioning, ParentAnchor, ParentElement as _, ParentOffsetBounds, Stack, @@ -28,6 +30,8 @@ use crate::terminal::view::ambient_agent::host_selector::NakedHeaderButtonTheme; use crate::terminal::view::ambient_agent::{AmbientAgentViewModel, AmbientAgentViewModelEvent}; use crate::ui_components::icons::Icon; use crate::view_components::action_button::{ActionButton, ButtonSize}; +use crate::view_components::DismissibleToast; +use crate::workspace::ToastStack; const HEADER_FONT_SIZE: f32 = 12.; @@ -64,6 +68,7 @@ pub enum AuthSecretSelectorAction { ClearSecret, OpenNewTypeSidecar, SelectNewType(usize), + DeleteSecret { name: String, owner: SecretOwner }, } pub enum AuthSecretSelectorEvent { @@ -79,6 +84,10 @@ pub struct AuthSecretSelector { is_new_type_sidecar_open: bool, menu_positioning_provider: Arc, ambient_agent_model: ModelHandle, + /// Secrets with an in-flight delete request, keyed by name. Used to + /// disable the X affordance so duplicate clicks while a delete is + /// pending don't fire a second request. + pending_deletes: HashSet, } impl AuthSecretSelector { @@ -152,6 +161,21 @@ impl AuthSecretSelector { me.refresh_menu(ctx); me.refresh_button(ctx); } + HarnessAvailabilityEvent::AuthSecretDeleted { harness, name } => { + me.handle_secret_deleted(*harness, name.clone(), ctx); + } + HarnessAvailabilityEvent::AuthSecretDeletionFailed { + harness, + name, + error, + } => { + me.handle_secret_deletion_failed( + *harness, + name.clone(), + error.clone(), + ctx, + ); + } HarnessAvailabilityEvent::Changed | HarnessAvailabilityEvent::AuthSecretCreationFailed { .. } => {} }, @@ -170,6 +194,7 @@ impl AuthSecretSelector { is_new_type_sidecar_open: false, menu_positioning_provider, ambient_agent_model, + pending_deletes: HashSet::new(), }; me.maybe_restore_auth_secret_from_settings(ctx); me.refresh_button(ctx); @@ -304,6 +329,7 @@ impl AuthSecretSelector { let availability = HarnessAvailabilityModel::as_ref(ctx); let items = build_main_menu_items( availability.auth_secrets_for(harness), + &self.pending_deletes, hover_background, header_text_color, ); @@ -314,6 +340,79 @@ impl AuthSecretSelector { }); } + fn handle_secret_deleted( + &mut self, + harness: Harness, + name: String, + ctx: &mut ViewContext, + ) { + let active_harness = self.ambient_agent_model.as_ref(ctx).selected_harness(); + let removed_pending = self.pending_deletes.remove(&name); + + if harness == active_harness { + // Drop the selection if it pointed at the just-deleted secret + // so the chip falls back to the inherit label. Persistence is + // cleared as well so the next session doesn't try to restore + // a non-existent secret. + let selected = self + .ambient_agent_model + .as_ref(ctx) + .selected_harness_auth_secret_name() + .map(|s| s.to_string()); + if selected.as_deref() == Some(name.as_str()) { + self.ambient_agent_model.update(ctx, |model, ctx| { + model.set_harness_auth_secret_name(None, ctx); + }); + } + CloudAgentSettings::handle(ctx).update(ctx, |settings, ctx| { + let mut map = settings.last_selected_auth_secret.value().clone(); + if map.get(harness.config_name()) == Some(&name) { + map.remove(harness.config_name()); + report_if_error!(settings.last_selected_auth_secret.set_value(map, ctx)); + } + }); + + self.refresh_menu(ctx); + self.refresh_button(ctx); + } + + // Only surface a toast for a deletion *this* selector initiated. + // A deletion fired from a different surface (or a different + // window) shouldn't pop a duplicate confirmation here. + if removed_pending { + let window_id = ctx.window_id(); + let message = format!("API key '{name}' deleted."); + ToastStack::handle(ctx).update(ctx, |ts, ctx| { + ts.add_ephemeral_toast(DismissibleToast::success(message), window_id, ctx); + }); + } + ctx.notify(); + } + + fn handle_secret_deletion_failed( + &mut self, + harness: Harness, + name: String, + error: String, + ctx: &mut ViewContext, + ) { + let removed_pending = self.pending_deletes.remove(&name); + let active_harness = self.ambient_agent_model.as_ref(ctx).selected_harness(); + if harness == active_harness { + self.refresh_menu(ctx); + } + // Show a toast only when the failed delete was ours; avoids + // double-toasting if another surface also tried to delete. + if removed_pending { + let window_id = ctx.window_id(); + let message = format!("Failed to delete API key '{name}': {error}"); + ToastStack::handle(ctx).update(ctx, |ts, ctx| { + ts.add_ephemeral_toast(DismissibleToast::error(message), window_id, ctx); + }); + } + ctx.notify(); + } + fn refresh_sidecar(&mut self, ctx: &mut ViewContext) { let appearance = Appearance::as_ref(ctx); let theme = appearance.theme(); @@ -387,6 +486,7 @@ impl AuthSecretSelector { fn build_main_menu_items( fetch_state: &AuthSecretFetchState, + pending_deletes: &HashSet, hover_background: Fill, header_text_color: pathfinder_color::ColorU, ) -> Vec> { @@ -413,13 +513,21 @@ fn build_main_menu_items( match fetch_state { AuthSecretFetchState::Loaded(secrets) => { for secret in secrets { + let is_pending_delete = pending_deletes.contains(&secret.name); let fields = MenuItemFields::new(secret.name.clone()) .with_font_size_override(ITEM_FONT_SIZE) .with_padding_override(ITEM_VERTICAL_PADDING, MENU_HORIZONTAL_PADDING) .with_override_hover_background_color(hover_background) .with_on_select_action(AuthSecretSelectorAction::SelectSecret( secret.name.clone(), - )); + )) + .with_right_side_icon(Icon::X) + .with_right_side_icon_action(AuthSecretSelectorAction::DeleteSecret { + name: secret.name.clone(), + owner: secret.owner.clone(), + }) + .with_right_side_icon_a11y_label(format!("Delete API key {}", secret.name)) + .with_right_side_icon_disabled(is_pending_delete); items.push(MenuItem::Item(fields)); } } @@ -541,6 +649,22 @@ impl TypedActionView for AuthSecretSelector { type_index, }); } + AuthSecretSelectorAction::DeleteSecret { name, owner } => { + // Drop duplicate dispatches while a delete for this same + // secret is still in flight (PRODUCT behavior 7). + if !self.pending_deletes.insert(name.clone()) { + return; + } + let harness = self.ambient_agent_model.as_ref(ctx).selected_harness(); + let name = name.clone(); + let owner = owner.clone(); + HarnessAvailabilityModel::handle(ctx).update(ctx, |model, ctx| { + model.delete_auth_secret(harness, name, owner, ctx); + }); + // Re-render so the X for the pending row becomes + // disabled. + self.refresh_menu(ctx); + } } } } diff --git a/app/src/terminal/view/ambient_agent/model_selector.rs b/app/src/terminal/view/ambient_agent/model_selector.rs index aa21e11c36..5c75915f7e 100644 --- a/app/src/terminal/view/ambient_agent/model_selector.rs +++ b/app/src/terminal/view/ambient_agent/model_selector.rs @@ -192,7 +192,9 @@ impl ModelSelector { HarnessAvailabilityEvent::AuthSecretsLoaded | HarnessAvailabilityEvent::AuthSecretCreated { .. } | HarnessAvailabilityEvent::AuthSecretCreationFailed { .. } - | HarnessAvailabilityEvent::AuthSecretsFetchFailed => {} + | HarnessAvailabilityEvent::AuthSecretsFetchFailed + | HarnessAvailabilityEvent::AuthSecretDeleted { .. } + | HarnessAvailabilityEvent::AuthSecretDeletionFailed { .. } => {} }, ); diff --git a/crates/managed_secrets/src/client.rs b/crates/managed_secrets/src/client.rs index 67ed9ad689..2a419c0aed 100644 --- a/crates/managed_secrets/src/client.rs +++ b/crates/managed_secrets/src/client.rs @@ -39,7 +39,7 @@ pub struct ManagedSecretConfigs { pub team_secrets: HashMap, } -#[derive(Debug, Clone)] +#[derive(Debug, Clone, PartialEq, Eq, Hash)] pub enum SecretOwner { CurrentUser, Team { team_uid: String }, From 944b38f41c5902c7b4a4f49b41c5f8601f8c5ba7 Mon Sep 17 00:00:00 2001 From: abhishekp106 Date: Wed, 20 May 2026 14:27:20 -0500 Subject: [PATCH 3/8] delete button working --- app/src/menu.rs | 54 +++++++++++-------- .../ambient_agent/auth_secret_selector.rs | 7 +-- 2 files changed, 33 insertions(+), 28 deletions(-) diff --git a/app/src/menu.rs b/app/src/menu.rs index 4ae2378a98..f5db112102 100644 --- a/app/src/menu.rs +++ b/app/src/menu.rs @@ -2,6 +2,13 @@ use std::cell::OnceCell; use std::sync::Arc; 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::{buttons::icon_button_with_color, icons}, +}; use chrono::{DateTime, Local}; use pathfinder_color::ColorU; use pathfinder_geometry::rect::RectF; @@ -1062,24 +1069,20 @@ impl MenuItemFields { .icon_size_override .unwrap_or_else(|| appearance.ui_font_size()); let icon_color = override_color.unwrap_or(color); - 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(); - - // If a separate right-side icon action is set, wrap the icon in - // its own `Hoverable` so it can dispatch independently of the row. - // The child `Hoverable` consumes the click event, preventing the - // row's `on_select_action` from firing alongside it. - let element: Box = if let Some(action) = &self.right_side_icon_action { - let mut hoverable = - Hoverable::new(self.right_side_icon_mouse_state.clone(), |_| container); + 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.with_cursor(Cursor::PointingHand); hoverable = hoverable.on_click(move |ctx, _, _| { if dispatch_item_actions { ctx.dispatch_typed_action(action.clone()); @@ -1089,12 +1092,19 @@ impl MenuItemFields { // doesn't latch onto the press that targets the icon. hoverable = hoverable.on_mouse_down(|_, _, _| {}); } - hoverable.finish() - } else { - container - }; - - Some(Shrinkable::new(1., element).finish()) + let element = Container::new(hoverable.finish()) + .with_margin_left(icon_size / 2.) + .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( diff --git a/app/src/terminal/view/ambient_agent/auth_secret_selector.rs b/app/src/terminal/view/ambient_agent/auth_secret_selector.rs index 03936f7c25..e2bfb4900c 100644 --- a/app/src/terminal/view/ambient_agent/auth_secret_selector.rs +++ b/app/src/terminal/view/ambient_agent/auth_secret_selector.rs @@ -169,12 +169,7 @@ impl AuthSecretSelector { name, error, } => { - me.handle_secret_deletion_failed( - *harness, - name.clone(), - error.clone(), - ctx, - ); + me.handle_secret_deletion_failed(*harness, name.clone(), error.clone(), ctx); } HarnessAvailabilityEvent::Changed | HarnessAvailabilityEvent::AuthSecretCreationFailed { .. } => {} From f39313559591884d2af3a55bcb132e44f9717e39 Mon Sep 17 00:00:00 2001 From: abhishekp106 Date: Wed, 20 May 2026 15:39:52 -0500 Subject: [PATCH 4/8] Working --- app/src/pane_group/mod.rs | 21 +++ app/src/pane_group/pane/terminal_pane.rs | 5 + app/src/terminal/input.rs | 16 ++ app/src/terminal/view.rs | 15 ++ .../ambient_agent/auth_secret_selector.rs | 94 ++++++++-- .../delete_auth_secret_confirmation_dialog.rs | 160 ++++++++++++++++++ app/src/terminal/view/ambient_agent/mod.rs | 1 + .../PRODUCT.md | 36 ++-- specs/cloud-mode-auth-secret-deletion/TECH.md | 58 +++++-- 9 files changed, 362 insertions(+), 44 deletions(-) create mode 100644 app/src/terminal/view/ambient_agent/delete_auth_secret_confirmation_dialog.rs diff --git a/app/src/pane_group/mod.rs b/app/src/pane_group/mod.rs index faeb204520..73f9dc1777 100644 --- a/app/src/pane_group/mod.rs +++ b/app/src/pane_group/mod.rs @@ -891,6 +891,8 @@ pub struct PaneGroup { /// Pane with an open environment setup mode selector modal (rendered at tab level). pane_with_open_environment_setup_mode_selector: Option, + /// Pane with an open auth-secret delete confirmation dialog (rendered at tab level). + pane_with_open_auth_secret_delete_confirmation_dialog: Option, /// Pane with an open agent-assisted environment modal (rendered at tab level). pane_with_open_agent_assisted_environment_modal: Option, @@ -3094,6 +3096,7 @@ impl PaneGroup { active_file_model, terminal_with_open_summarization_dialog: None, pane_with_open_environment_setup_mode_selector: None, + pane_with_open_auth_secret_delete_confirmation_dialog: None, pane_with_open_agent_assisted_environment_modal: None, right_panel_open: false, left_panel_open: false, @@ -5293,6 +5296,12 @@ impl PaneGroup { if self.pane_with_open_environment_setup_mode_selector == Some(pane_id) { self.pane_with_open_environment_setup_mode_selector = None; } + if self.pane_with_open_auth_secret_delete_confirmation_dialog == Some(pane_id) { + self.pane_with_open_auth_secret_delete_confirmation_dialog = None; + } + if self.pane_with_open_auth_secret_delete_confirmation_dialog == Some(pane_id) { + self.pane_with_open_auth_secret_delete_confirmation_dialog = None; + } if self.pane_with_open_agent_assisted_environment_modal == Some(pane_id) { self.pane_with_open_agent_assisted_environment_modal = None; } @@ -8504,6 +8513,18 @@ impl View for PaneGroup { } } + // Render auth-secret delete confirmation at tab level when open. + if let Some(pane_id) = self.pane_with_open_auth_secret_delete_confirmation_dialog { + if let Some(dialog) = self + .terminal_view_from_pane_id(pane_id, app) + .and_then(|tv| { + tv.as_ref(app) + .auth_secret_delete_confirmation_dialog_element(app) + }) + { + stack.add_child(dialog); + } + } // Render agent-assisted environment modal at tab level when open. if let Some(pane_id) = self.pane_with_open_agent_assisted_environment_modal { if let Some(handle) = self diff --git a/app/src/pane_group/pane/terminal_pane.rs b/app/src/pane_group/pane/terminal_pane.rs index ee8603d6c7..fac22ff16c 100644 --- a/app/src/pane_group/pane/terminal_pane.rs +++ b/app/src/pane_group/pane/terminal_pane.rs @@ -1282,6 +1282,11 @@ fn handle_terminal_view_event( group.pane_with_open_environment_setup_mode_selector = is_open.then_some(pane_id); ctx.notify(); } + Event::AuthSecretDeleteConfirmationDialogToggled { is_open } => { + group.pane_with_open_auth_secret_delete_confirmation_dialog = + is_open.then_some(pane_id); + ctx.notify(); + } Event::AnonymousUserSignup => ctx.emit(pane_group::Event::AnonymousUserSignup), #[cfg(feature = "local_fs")] Event::OpenFileWithTarget { diff --git a/app/src/terminal/input.rs b/app/src/terminal/input.rs index 30d46b1032..726bc3b499 100644 --- a/app/src/terminal/input.rs +++ b/app/src/terminal/input.rs @@ -1027,6 +1027,9 @@ pub enum Event { OpenAutoReloadModal { purchased_credits: i32, }, + AuthSecretDeleteConfirmationDialogToggled { + is_open: bool, + }, ShowToast { message: String, flavor: ToastFlavor, @@ -2390,6 +2393,11 @@ impl Input { } ctx.notify(); } + AuthSecretSelectorEvent::DeleteConfirmationDialogToggled { is_open } => { + ctx.emit(Event::AuthSecretDeleteConfirmationDialogToggled { + is_open: *is_open, + }); + } AuthSecretSelectorEvent::MenuVisibilityChanged { open: true } => {} }); let initial_harness = state.view_model.as_ref(ctx).selected_harness(); @@ -3679,6 +3687,14 @@ impl Input { .and_then(|state| state.auth_secret_selector.as_ref()) } + pub(super) fn auth_secret_delete_confirmation_dialog_element( + &self, + ctx: &AppContext, + ) -> Option> { + self.auth_secret_selector() + .map(|selector| selector.as_ref(ctx).delete_confirmation_dialog_element()) + } + pub(super) fn auth_secret_ftux_view(&self) -> Option<&ViewHandle> { self.ambient_agent_view_state .as_ref() diff --git a/app/src/terminal/view.rs b/app/src/terminal/view.rs index c877657204..3ee37e4e47 100644 --- a/app/src/terminal/view.rs +++ b/app/src/terminal/view.rs @@ -1687,6 +1687,9 @@ pub enum Event { EnvironmentSetupModeSelectorToggled { is_open: bool, }, + AuthSecretDeleteConfirmationDialogToggled { + is_open: bool, + }, CtrlD, ShutdownPty, // TODO: break this event down into higher-level events that hide the @@ -20965,6 +20968,9 @@ impl TerminalView { purchased_credits: *purchased_credits, }); } + InputEvent::AuthSecretDeleteConfirmationDialogToggled { is_open } => { + ctx.emit(Event::AuthSecretDeleteConfirmationDialogToggled { is_open: *is_open }); + } InputEvent::ShowToast { message, flavor } => { ctx.emit(Event::ShowToast { message: message.clone(), @@ -21925,6 +21931,15 @@ impl TerminalView { .then_some(&self.environment_setup_mode_selector) } + pub fn auth_secret_delete_confirmation_dialog_element( + &self, + ctx: &AppContext, + ) -> Option> { + self.input + .as_ref(ctx) + .auth_secret_delete_confirmation_dialog_element(ctx) + } + pub fn summarization_cancel_dialog_handle( &self, ctx: &AppContext, diff --git a/app/src/terminal/view/ambient_agent/auth_secret_selector.rs b/app/src/terminal/view/ambient_agent/auth_secret_selector.rs index e2bfb4900c..9265e25e56 100644 --- a/app/src/terminal/view/ambient_agent/auth_secret_selector.rs +++ b/app/src/terminal/view/ambient_agent/auth_secret_selector.rs @@ -12,7 +12,6 @@ use warpui::elements::{ Border, ChildAnchor, ChildView, OffsetPositioning, ParentAnchor, ParentElement as _, ParentOffsetBounds, Stack, }; -use warpui::fonts::Properties; use warpui::{ AppContext, Element, Entity, ModelHandle, SingletonEntity, TypedActionView, View, ViewContext, ViewHandle, @@ -26,6 +25,10 @@ use crate::ai::harness_availability::{ use crate::menu::{Event as MenuEvent, Menu, MenuItem, MenuItemFields, MenuVariant}; use crate::report_if_error; use crate::terminal::input::{MenuPositioning, MenuPositioningProvider}; +use crate::terminal::view::ambient_agent::delete_auth_secret_confirmation_dialog::{ + DeleteAuthSecretConfirmationDialog, DeleteAuthSecretConfirmationDialogEvent, + PendingAuthSecretDeletion, +}; use crate::terminal::view::ambient_agent::host_selector::NakedHeaderButtonTheme; use crate::terminal::view::ambient_agent::{AmbientAgentViewModel, AmbientAgentViewModelEvent}; use crate::ui_components::icons::Icon; @@ -74,12 +77,14 @@ pub enum AuthSecretSelectorAction { pub enum AuthSecretSelectorEvent { MenuVisibilityChanged { open: bool }, NewTypeSelected { harness: Harness, type_index: usize }, + DeleteConfirmationDialogToggled { is_open: bool }, } pub struct AuthSecretSelector { button: ViewHandle, menu: ViewHandle>, new_type_sidecar: ViewHandle>, + delete_confirmation_dialog: ViewHandle, is_menu_open: bool, is_new_type_sidecar_open: bool, menu_positioning_provider: Arc, @@ -137,6 +142,12 @@ impl AuthSecretSelector { MenuEvent::ItemSelected | MenuEvent::ItemHovered => {} }); + let delete_confirmation_dialog = + ctx.add_typed_action_view(DeleteAuthSecretConfirmationDialog::new); + ctx.subscribe_to_view(&delete_confirmation_dialog, |me, _, event, ctx| { + me.handle_delete_confirmation_event(event, ctx); + }); + ctx.subscribe_to_model(&ambient_agent_model, |me, _, event, ctx| match event { AmbientAgentViewModelEvent::HarnessSelected => { // When the harness changes, try to restore the saved auth secret. @@ -185,6 +196,7 @@ impl AuthSecretSelector { button, menu, new_type_sidecar, + delete_confirmation_dialog, is_menu_open: false, is_new_type_sidecar_open: false, menu_positioning_provider, @@ -231,6 +243,10 @@ impl AuthSecretSelector { self.menu.update(ctx, |menu, ctx| menu.select_previous(ctx)); } + pub(crate) fn delete_confirmation_dialog_element(&self) -> Box { + ChildView::new(&self.delete_confirmation_dialog).finish() + } + fn set_menu_visibility(&mut self, is_open: bool, ctx: &mut ViewContext) { if self.is_menu_open == is_open { return; @@ -408,6 +424,55 @@ impl AuthSecretSelector { ctx.notify(); } + fn handle_delete_confirmation_event( + &mut self, + event: &DeleteAuthSecretConfirmationDialogEvent, + ctx: &mut ViewContext, + ) { + match event { + DeleteAuthSecretConfirmationDialogEvent::Cancel => { + self.delete_confirmation_dialog + .update(ctx, |dialog, ctx| dialog.hide(ctx)); + ctx.emit(AuthSecretSelectorEvent::DeleteConfirmationDialogToggled { + is_open: false, + }); + } + DeleteAuthSecretConfirmationDialogEvent::Confirm(pending_deletion) => { + let pending_deletion = pending_deletion.clone(); + self.delete_confirmation_dialog + .update(ctx, |dialog, ctx| dialog.hide(ctx)); + ctx.emit(AuthSecretSelectorEvent::DeleteConfirmationDialogToggled { + is_open: false, + }); + self.start_secret_delete(pending_deletion, ctx); + } + } + } + + fn start_secret_delete( + &mut self, + pending_deletion: PendingAuthSecretDeletion, + ctx: &mut ViewContext, + ) { + let PendingAuthSecretDeletion { + harness, + name, + owner, + } = pending_deletion; + + // Drop duplicate dispatches while a delete for this same + // secret is still in flight (PRODUCT behavior 9). + if !self.pending_deletes.insert(name.clone()) { + return; + } + + HarnessAvailabilityModel::handle(ctx).update(ctx, |model, ctx| { + model.delete_auth_secret(harness, name, owner, ctx); + }); + // Re-render so the X for the pending row becomes + // disabled. + self.refresh_menu(ctx); + } fn refresh_sidecar(&mut self, ctx: &mut ViewContext) { let appearance = Appearance::as_ref(ctx); let theme = appearance.theme(); @@ -552,7 +617,7 @@ fn build_main_menu_items( .with_padding_override(ITEM_VERTICAL_PADDING, MENU_HORIZONTAL_PADDING) .with_override_hover_background_color(hover_background) .with_icon(Icon::Plus) - .with_right_side_label("›", Properties::default()) + .with_right_side_icon(Icon::ChevronRight) .with_on_select_action(AuthSecretSelectorAction::OpenNewTypeSidecar), )); @@ -645,20 +710,19 @@ impl TypedActionView for AuthSecretSelector { }); } AuthSecretSelectorAction::DeleteSecret { name, owner } => { - // Drop duplicate dispatches while a delete for this same - // secret is still in flight (PRODUCT behavior 7). - if !self.pending_deletes.insert(name.clone()) { - return; - } - let harness = self.ambient_agent_model.as_ref(ctx).selected_harness(); - let name = name.clone(); - let owner = owner.clone(); - HarnessAvailabilityModel::handle(ctx).update(ctx, |model, ctx| { - model.delete_auth_secret(harness, name, owner, ctx); + let pending_deletion = PendingAuthSecretDeletion { + harness: self.ambient_agent_model.as_ref(ctx).selected_harness(), + name: name.clone(), + owner: owner.clone(), + }; + self.set_menu_visibility(false, ctx); + self.delete_confirmation_dialog.update(ctx, |dialog, ctx| { + dialog.show(pending_deletion, ctx); + }); + ctx.emit(AuthSecretSelectorEvent::DeleteConfirmationDialogToggled { + is_open: true, }); - // Re-render so the X for the pending row becomes - // disabled. - self.refresh_menu(ctx); + ctx.notify(); } } } diff --git a/app/src/terminal/view/ambient_agent/delete_auth_secret_confirmation_dialog.rs b/app/src/terminal/view/ambient_agent/delete_auth_secret_confirmation_dialog.rs new file mode 100644 index 0000000000..e333aefcae --- /dev/null +++ b/app/src/terminal/view/ambient_agent/delete_auth_secret_confirmation_dialog.rs @@ -0,0 +1,160 @@ +use pathfinder_color::ColorU; +use pathfinder_geometry::vector::vec2f; +use warp_cli::agent::Harness; +use warp_managed_secrets::client::SecretOwner; +use warpui::{ + elements::{Align, ChildView, Container, Dismiss, DropShadow, Empty}, + ui_components::components::UiComponent, + AppContext, Element, Entity, SingletonEntity, TypedActionView, View, ViewContext, ViewHandle, +}; + +use crate::{ + appearance::Appearance, + ui_components::dialog::{dialog_styles, Dialog}, + view_components::action_button::{ActionButton, DangerPrimaryTheme, NakedTheme}, +}; + +const DIALOG_WIDTH: f32 = 450.; + +#[derive(Clone, Debug)] +pub(super) struct PendingAuthSecretDeletion { + pub(super) harness: Harness, + pub(super) name: String, + pub(super) owner: SecretOwner, +} + +pub(super) enum DeleteAuthSecretConfirmationDialogEvent { + Cancel, + Confirm(PendingAuthSecretDeletion), +} + +#[derive(Debug)] +pub(super) enum DeleteAuthSecretConfirmationDialogAction { + Cancel, + Confirm, +} + +pub(super) struct DeleteAuthSecretConfirmationDialog { + visible: bool, + pending_deletion: Option, + cancel_button: ViewHandle, + delete_button: ViewHandle, +} + +impl DeleteAuthSecretConfirmationDialog { + pub(super) fn new(ctx: &mut ViewContext) -> Self { + let cancel_button = ctx.add_typed_action_view(|_| { + ActionButton::new("Cancel", NakedTheme).on_click(|ctx| { + ctx.dispatch_typed_action(DeleteAuthSecretConfirmationDialogAction::Cancel); + }) + }); + + let delete_button = ctx.add_typed_action_view(|_| { + ActionButton::new("Delete", DangerPrimaryTheme).on_click(|ctx| { + ctx.dispatch_typed_action(DeleteAuthSecretConfirmationDialogAction::Confirm); + }) + }); + + Self { + visible: false, + pending_deletion: None, + cancel_button, + delete_button, + } + } + + pub(super) fn show( + &mut self, + pending_deletion: PendingAuthSecretDeletion, + ctx: &mut ViewContext, + ) { + self.pending_deletion = Some(pending_deletion); + self.visible = true; + ctx.notify(); + } + + pub(super) fn hide(&mut self, ctx: &mut ViewContext) { + self.visible = false; + self.pending_deletion = None; + ctx.notify(); + } +} + +impl Entity for DeleteAuthSecretConfirmationDialog { + type Event = DeleteAuthSecretConfirmationDialogEvent; +} + +impl View for DeleteAuthSecretConfirmationDialog { + fn ui_name() -> &'static str { + "DeleteAuthSecretConfirmationDialog" + } + + fn render(&self, app: &AppContext) -> Box { + if !self.visible { + return Empty::new().finish(); + } + + let Some(pending_deletion) = self.pending_deletion.as_ref() else { + return Empty::new().finish(); + }; + + let appearance = Appearance::as_ref(app); + let description = format!( + "Are you sure you want to delete {}? This action cannot be undone. Any agents or environments referencing this secret will no longer have access to it.", + pending_deletion.name + ); + + let dialog = Dialog::new( + "Delete secret".to_string(), + Some(description), + dialog_styles(appearance), + ) + .with_bottom_row_child(ChildView::new(&self.cancel_button).finish()) + .with_bottom_row_child( + Container::new(ChildView::new(&self.delete_button).finish()) + .with_margin_left(12.) + .finish(), + ) + .with_width(DIALOG_WIDTH) + .build() + .finish(); + let dialog = Container::new(dialog) + .with_drop_shadow(DropShadow { + color: ColorU::new(0, 0, 0, 77), + offset: vec2f(0., 7.), + blur_radius: 7., + spread_radius: 0., + }) + .finish(); + + let dialog = Dismiss::new(dialog) + .prevent_interaction_with_other_elements() + .on_dismiss(|ctx, _app| { + ctx.dispatch_typed_action(DeleteAuthSecretConfirmationDialogAction::Cancel) + }) + .finish(); + + Container::new(Align::new(dialog).finish()) + .with_background(appearance.theme().dark_overlay()) + .finish() + } +} + +impl TypedActionView for DeleteAuthSecretConfirmationDialog { + type Action = DeleteAuthSecretConfirmationDialogAction; + + fn handle_action(&mut self, action: &Self::Action, ctx: &mut ViewContext) { + match action { + DeleteAuthSecretConfirmationDialogAction::Cancel => { + ctx.emit(DeleteAuthSecretConfirmationDialogEvent::Cancel) + } + DeleteAuthSecretConfirmationDialogAction::Confirm => { + if let Some(pending_deletion) = self.pending_deletion.clone() { + ctx.emit(DeleteAuthSecretConfirmationDialogEvent::Confirm( + pending_deletion, + )); + } + } + } + } +} diff --git a/app/src/terminal/view/ambient_agent/mod.rs b/app/src/terminal/view/ambient_agent/mod.rs index 9281f29457..8eb893646f 100644 --- a/app/src/terminal/view/ambient_agent/mod.rs +++ b/app/src/terminal/view/ambient_agent/mod.rs @@ -2,6 +2,7 @@ mod auth_secret_ftux_dropdown; mod auth_secret_ftux_view; pub(crate) mod auth_secret_selector; mod block; +mod delete_auth_secret_confirmation_dialog; mod first_time_setup; mod footer; mod harness_selector; diff --git a/specs/cloud-mode-auth-secret-deletion/PRODUCT.md b/specs/cloud-mode-auth-secret-deletion/PRODUCT.md index d2423c7e5a..411abc42c0 100644 --- a/specs/cloud-mode-auth-secret-deletion/PRODUCT.md +++ b/specs/cloud-mode-auth-secret-deletion/PRODUCT.md @@ -2,11 +2,11 @@ ## Summary -Users can delete Warp-managed harness auth secrets directly from the auth secret selector chip menu. The delete affordance appears as a right-aligned `X` button on each existing secret row, removes the secret from the server, updates the menu state, and confirms success or failure with a toast. +Users can delete Warp-managed harness auth secrets directly from the auth secret selector chip menu. The delete affordance appears as a right-aligned `X` button on each existing secret row, opens a confirmation modal, removes the secret from the server after confirmation, updates the menu state, and confirms success or failure with a toast. ## Figma -Figma: none provided. The prompt includes a screenshot of the current auth selector menu and requests adding a right-aligned `X` button to each non-New submenu item. +Figma: none provided. The prompt includes screenshots of the auth selector delete affordance and the requested destructive confirmation modal layout. ## Behavior @@ -26,38 +26,46 @@ Figma: none provided. The prompt includes a screenshot of the current auth selec 4. Selecting the secret row outside the `X` button preserves current behavior: the row becomes the selected auth secret for the active harness, the per-harness selection is persisted, and the menu closes. -5. Selecting the `X` button deletes that specific secret. It must not select the secret first, must not open the "New" sidecar, and must not accidentally dispatch the row's normal select action. +5. Selecting the `X` button opens a confirmation modal for that specific secret. It must not select the secret first, must not open the "New" sidecar, and must not accidentally dispatch the row's normal select action. -6. Deletion is server-backed. A secret is considered deleted only after the server delete request succeeds. The UI must not present deletion as successful before the server confirms it. +6. The confirmation modal shows: + - Title: "Delete secret". + - Description: "Are you sure you want to delete {SECRET_NAME}? This action cannot be undone. Any agents or environments referencing this secret will no longer have access to it." + - Buttons: "Cancel" and "Delete". + - Layout and destructive styling consistent with existing Warp confirmation dialogs and the provided mock. -7. While deletion is pending, the user should not be able to fire duplicate delete requests for the same secret from the same open menu. Acceptable treatments include disabling the `X` button for that row, closing the menu, or otherwise preventing a second click. +7. Choosing "Cancel", dismissing the modal, or otherwise closing it leaves the secret untouched and does not start a delete request. Choosing "Delete" starts deletion for the secret named in the modal. -8. On successful deletion: +8. Deletion is server-backed. A secret is considered deleted only after the server delete request succeeds. The UI must not present deletion as successful before the server confirms it. + +9. While deletion is pending, the user should not be able to fire duplicate delete requests for the same secret from the same open menu. Acceptable treatments include disabling the `X` button for that row, closing the menu, or otherwise preventing a second click. + +10. On successful deletion: - The deleted secret disappears from the selector menu the next time the menu is shown, and ideally immediately if the menu remains open. - A success toast is shown. - If the deleted secret was currently selected for the active harness, the selected auth secret is cleared and the chip falls back to the no-secret/inherit label. - Any persisted last-selected secret for the active harness is cleared if it points to the deleted secret. -9. On failed deletion: +11. On failed deletion: - The secret remains available in the selector menu. - The current selected secret does not change. - A failure toast is shown. - The user can retry deletion after the failure. -10. If the deleted secret is selected by another visible selector instance, that selector should refresh when it observes updated harness secret state. It must not keep showing a deleted secret as a valid selectable item after the secret list is refreshed. +12. If the deleted secret is selected by another visible selector instance, that selector should refresh when it observes updated harness secret state. It must not keep showing a deleted secret as a valid selectable item after the secret list is refreshed. -11. If the secret list is stale and the server reports that the secret no longer exists, the delete attempt is treated as a failure unless the server explicitly reports the delete as successful. The failure toast should use the server/user-facing error text when available. +13. If the secret list is stale and the server reports that the secret no longer exists, the delete attempt is treated as a failure unless the server explicitly reports the delete as successful. The failure toast should use the server/user-facing error text when available. -12. If the user is offline or unauthenticated when pressing `X`, the delete fails and shows a failure toast. The menu must not remove the secret locally unless a later server response confirms deletion. +14. If the user is offline or unauthenticated when pressing "Delete" in the confirmation modal, the delete fails and shows a failure toast. The menu must not remove the secret locally unless a later server response confirms deletion. -13. The success and failure toasts are ephemeral, consistent with nearby app toasts, and do not require a custom action button. +15. The success and failure toasts are ephemeral, consistent with nearby app toasts, and do not require a custom action button. -14. Keyboard behavior must remain usable: +16. Keyboard behavior must remain usable: - Existing keyboard navigation for selecting a secret row remains unchanged. - Adding the `X` button must not make the menu trap focus or prevent Escape/close behavior. - If the `X` button is reachable by keyboard, it must have an accessible label such as "Delete API key". -15. The menu layout remains stable for long secret names. Long names should truncate or shrink according to existing menu behavior rather than overlapping the right-aligned `X` button. +17. The menu layout remains stable for long secret names. Long names should truncate or shrink according to existing menu behavior rather than overlapping the right-aligned `X` button. -16. The feature does not add editing, renaming, creating, or bulk-deleting secrets. It only deletes one existing secret at a time from the auth selector menu. +18. The feature does not add editing, renaming, creating, or bulk-deleting secrets. It only deletes one existing secret at a time from the auth selector menu. diff --git a/specs/cloud-mode-auth-secret-deletion/TECH.md b/specs/cloud-mode-auth-secret-deletion/TECH.md index 1944b7bd79..4b6a92d79c 100644 --- a/specs/cloud-mode-auth-secret-deletion/TECH.md +++ b/specs/cloud-mode-auth-secret-deletion/TECH.md @@ -16,6 +16,7 @@ Relevant current code: - `crates/managed_secrets/src/manager.rs:87` exposes `ManagedSecretManager::delete_secret(owner, name)`. - `app/src/server/server_api/managed_secrets.rs:132` wires `delete_managed_secret` to the server GraphQL mutation. - `app/src/menu.rs:394` defines reusable `MenuItemFields`. It already has right-side label/icon rendering, but right-side icons are decorative and do not dispatch a separate action from the row. +- Existing destructive confirmation dialogs such as `app/src/settings_view/delete_environment_confirmation_dialog.rs` and `app/src/settings_view/mcp_servers/destructive_mcp_confirmation_dialog.rs` establish the preferred `Dialog` + `DangerPrimaryTheme` pattern for the new modal. The current `AuthSecretEntry` contains only `name`, but server `ManagedSecret` results include `owner`. Deleting team-owned secrets correctly requires preserving enough owner information to call `delete_secret` with `SecretOwner::Team { team_uid }` instead of always assuming `CurrentUser`. @@ -64,7 +65,7 @@ The current `AuthSecretEntry` contains only `name`, but server `ManagedSecret` r } ``` - Track pending deletes in `AuthSecretSelector` with a small set keyed by `(harness, name, owner)` or by a stable local row id if one is introduced. This satisfies PRODUCT behavior 7 and avoids duplicate delete requests from the same menu. + Track pending deletes in `AuthSecretSelector` with a small set keyed by `(harness, name, owner)` or by a stable local row id if one is introduced. This satisfies PRODUCT behavior 9 and avoids duplicate delete requests from the same menu. 4. Render the right-aligned `X` affordance using the menu system's existing styling. @@ -78,14 +79,35 @@ The current `AuthSecretEntry` contains only `name`, but server `ManagedSecret` r If changing `MenuItemFields` is too broad during implementation, an acceptable alternative is a custom-label row in `auth_secret_selector.rs` that composes the existing text/icon primitives and explicitly separates row-click from `X` click. Prefer the reusable menu extension only if it stays small and does not alter existing right-side icon behavior. -5. Wire deletion action handling in `AuthSecretSelector`. +5. Add a dedicated confirmation dialog before deleting. + + Add a small destructive dialog view under `app/src/terminal/view/ambient_agent/` that mirrors existing confirmation dialogs: + + - Title: `Delete secret` + - Body: `Are you sure you want to delete {SECRET_NAME}? This action cannot be undone. Any agents or environments referencing this secret will no longer have access to it.` + - Buttons: `Cancel` and destructive `Delete` + - Dismiss-to-cancel behavior and centered overlay placement + + The dialog should retain a captured deletion payload containing harness, secret name, and owner. This avoids deleting the wrong target if selector state changes while the modal is open. + +6. Wire deletion action handling in `AuthSecretSelector`. In the `DeleteSecret` branch: - Read the active harness from `AmbientAgentViewModel`. + - Close the selector menu/sidecar. + - Show the confirmation dialog with the captured deletion payload. + + On confirmation: + - Mark the row pending. - Call `HarnessAvailabilityModel::delete_auth_secret(harness, name, owner, ctx)`. - - Close the menu or refresh it into a disabled pending state. + - Refresh the menu into a disabled pending state. + + On cancel or dismiss: + + - Hide the dialog. + - Leave pending-delete state untouched and do not call the server-backed delete API. In the selector's `HarnessAvailabilityModel` subscription: @@ -96,43 +118,49 @@ The current `AuthSecretEntry` contains only `name`, but server `ManagedSecret` r Use `DismissibleToast::success(...)` and `DismissibleToast::error(...)` via `ToastStack::add_ephemeral_toast`, matching nearby toast patterns. -6. Keep adjacent auth-secret surfaces consistent. +7. Keep adjacent auth-secret surfaces consistent. `AuthSecretFtuxDropdown`, orchestration pickers, and model selector subscribers must compile with the new deletion events. They can ignore failure events and refresh on success where they render auth secret lists. This ensures a secret deleted from the chip no longer appears in other already-mounted selectors after the model emits. ## Testing and Validation -1. Unit-test or view-model-test the cache update path for PRODUCT behaviors 6, 8, and 9: +Per the current implementation pass, automated test additions and test execution are deferred by request. The implementation should remain structured so the following cases are straightforward to cover later: + +1. Cache update behavior for PRODUCT behaviors 8, 10, and 11: - Successful deletion removes only the matching auth secret from the loaded list and emits `AuthSecretDeleted`. - Failed deletion leaves the list unchanged and emits `AuthSecretDeletionFailed`. - Team-owned entries call delete with `SecretOwner::Team`, user-owned entries call delete with `SecretOwner::CurrentUser`. -2. Test `AuthSecretSelector` action behavior for PRODUCT behaviors 4, 5, 7, and 8: +2. Selector and confirmation routing for PRODUCT behaviors 4, 5, 7, 9, and 10: - Clicking/selecting the row still selects the secret. - - Dispatching the delete action does not select the secret. - - Duplicate delete dispatches for a pending secret are ignored. + - Dispatching the delete affordance opens the confirmation dialog without deleting immediately. + - Cancelling or dismissing the confirmation dialog does not start deletion. + - Confirming deletion transitions into the existing pending-delete flow. + - Duplicate confirmed delete dispatches for a pending secret are ignored. - Deleting the currently selected secret clears the selected model value and persisted setting. -3. Add or update menu rendering tests if the reusable `MenuItemFields` right-side action API is introduced: +3. Menu rendering coverage if the reusable `MenuItemFields` right-side action API is introduced: - Existing right-side icon rendering remains decorative when no right-side action is set. - A right-side action dispatches independently from the row action. -4. Manual validation against the screenshot state: +4. Manual validation against the screenshot state, if revisited later: - Open the auth selector chip menu with multiple secrets. - Confirm only existing secret rows have a right-aligned `X`. - Confirm the "New" row and sidecar rows do not show an `X`. - - Delete an unselected secret and verify it disappears plus a success toast appears. - - Delete the selected secret and verify the chip label falls back to no-secret/inherit state. + - Click `X` and verify the destructive confirmation modal appears with the specified copy. + - Cancel/dismiss the modal and verify the row remains untouched. + - Confirm deletion for an unselected secret and verify it disappears plus a success toast appears. + - Confirm deletion for the selected secret and verify the chip label falls back to no-secret/inherit state. - Force a delete failure/offline state and verify the failure toast appears and the row remains. 5. Run focused checks: ```sh - cargo check -p warp_app - cargo test -p warp_app auth_secret + cargo check -p warp + cargo fmt -- --check ``` - Adjust package/test targets to the repository's current presubmit conventions if those commands are too broad or renamed. + Skip test execution for this implementation pass per request. ## Parallelization From 85b4d64b7d01168672d501d29d4d48645ab8a687 Mon Sep 17 00:00:00 2001 From: abhishekp106 Date: Wed, 20 May 2026 17:33:11 -0500 Subject: [PATCH 5/8] lint --- app/src/menu.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/src/menu.rs b/app/src/menu.rs index f5db112102..940426251a 100644 --- a/app/src/menu.rs +++ b/app/src/menu.rs @@ -1072,7 +1072,7 @@ impl MenuItemFields { if let Some(action) = &self.right_side_icon_action { let mut button = icon_button_with_color( appearance, - icon.clone(), + *icon, false, self.right_side_icon_mouse_state.clone(), icon_color, From 2123f363e60155719741017da609b8aacfa83a4c Mon Sep 17 00:00:00 2001 From: abhishekp106 Date: Wed, 20 May 2026 17:43:10 -0500 Subject: [PATCH 6/8] right arrow fix --- app/src/menu.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/src/menu.rs b/app/src/menu.rs index 940426251a..e049f823e4 100644 --- a/app/src/menu.rs +++ b/app/src/menu.rs @@ -1104,7 +1104,7 @@ impl MenuItemFields { let container = Container::new(icon_element) .with_margin_left(icon_size / 2.) .finish(); - Some(Shrinkable::new(1., container).finish()) + Some(Shrinkable::new(1., Align::new(container).right().finish()).finish()) } fn render_right_aligned_chevron( From 724c6fc68fbcd38502e987101bf1d2395d6f55de Mon Sep 17 00:00:00 2001 From: abhishekp106 Date: Fri, 22 May 2026 09:27:20 -0500 Subject: [PATCH 7/8] Pr feedback --- app/src/ai/harness_availability.rs | 22 ++++- .../ambient_agent/auth_secret_selector.rs | 81 +++++++++++++------ .../PRODUCT.md | 4 +- specs/cloud-mode-auth-secret-deletion/TECH.md | 16 ++-- 4 files changed, 85 insertions(+), 38 deletions(-) diff --git a/app/src/ai/harness_availability.rs b/app/src/ai/harness_availability.rs index f4efd016cd..5d104bba6f 100644 --- a/app/src/ai/harness_availability.rs +++ b/app/src/ai/harness_availability.rs @@ -85,10 +85,12 @@ pub enum HarnessAvailabilityEvent { AuthSecretDeleted { harness: Harness, name: String, + owner: SecretOwner, }, AuthSecretDeletionFailed { harness: Harness, name: String, + owner: SecretOwner, error: String, }, } @@ -307,15 +309,21 @@ impl HarnessAvailabilityModel { ctx: &mut ModelContext, ) { let manager = ManagedSecretManager::handle(ctx); - let delete_future = manager.as_ref(ctx).delete_secret(owner, name.clone()); + let delete_future = manager + .as_ref(ctx) + .delete_secret(owner.clone(), 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); + remove_deleted_auth_secret_entry(entries, &name, &owner); } - ctx.emit(HarnessAvailabilityEvent::AuthSecretDeleted { harness, name }); + ctx.emit(HarnessAvailabilityEvent::AuthSecretDeleted { + harness, + name, + owner, + }); } Err(e) => { let msg = e.to_string(); @@ -323,6 +331,7 @@ impl HarnessAvailabilityModel { ctx.emit(HarnessAvailabilityEvent::AuthSecretDeletionFailed { harness, name, + owner, error: msg, }); } @@ -387,6 +396,13 @@ fn secret_owner_from_space(space: &warp_graphql::object::Space) -> SecretOwner { } } +fn remove_deleted_auth_secret_entry( + entries: &mut Vec, + name: &str, + owner: &SecretOwner, +) { + entries.retain(|entry| entry.name.as_str() != name || &entry.owner != owner); +} fn harness_to_graphql_harness(harness: Harness) -> Option { match harness { Harness::Oz => Some(warp_graphql::ai::AgentHarness::Oz), diff --git a/app/src/terminal/view/ambient_agent/auth_secret_selector.rs b/app/src/terminal/view/ambient_agent/auth_secret_selector.rs index 9265e25e56..1785682a86 100644 --- a/app/src/terminal/view/ambient_agent/auth_secret_selector.rs +++ b/app/src/terminal/view/ambient_agent/auth_secret_selector.rs @@ -1,4 +1,4 @@ -use std::collections::HashSet; +use std::collections::{HashMap, HashSet}; use std::sync::Arc; use pathfinder_geometry::vector::vec2f; @@ -63,6 +63,7 @@ const NO_SECRET_LABEL: &str = "Inherit key from environment"; const NEW_ITEM_LABEL: &str = "New"; const MAIN_MENU_SAVE_POSITION_ID: &str = "auth_secret_selector_main_menu"; +type PendingDeleteKey = (Harness, String, SecretOwner); #[derive(Clone, Debug, PartialEq)] pub enum AuthSecretSelectorAction { @@ -89,10 +90,10 @@ pub struct AuthSecretSelector { is_new_type_sidecar_open: bool, menu_positioning_provider: Arc, ambient_agent_model: ModelHandle, - /// Secrets with an in-flight delete request, keyed by name. Used to - /// disable the X affordance so duplicate clicks while a delete is - /// pending don't fire a second request. - pending_deletes: HashSet, + /// Secrets with an in-flight delete request, keyed by harness, name, + /// and owner. This disables only the matching X affordance while that + /// exact request is pending. + pending_deletes: HashSet, } impl AuthSecretSelector { @@ -172,15 +173,26 @@ impl AuthSecretSelector { me.refresh_menu(ctx); me.refresh_button(ctx); } - HarnessAvailabilityEvent::AuthSecretDeleted { harness, name } => { - me.handle_secret_deleted(*harness, name.clone(), ctx); + HarnessAvailabilityEvent::AuthSecretDeleted { + harness, + name, + owner, + } => { + me.handle_secret_deleted(*harness, name.clone(), owner.clone(), ctx); } HarnessAvailabilityEvent::AuthSecretDeletionFailed { harness, name, + owner, error, } => { - me.handle_secret_deletion_failed(*harness, name.clone(), error.clone(), ctx); + me.handle_secret_deletion_failed( + *harness, + name.clone(), + owner.clone(), + error.clone(), + ctx, + ); } HarnessAvailabilityEvent::Changed | HarnessAvailabilityEvent::AuthSecretCreationFailed { .. } => {} @@ -339,6 +351,7 @@ impl AuthSecretSelector { let harness = self.ambient_agent_model.as_ref(ctx).selected_harness(); let availability = HarnessAvailabilityModel::as_ref(ctx); let items = build_main_menu_items( + harness, availability.auth_secrets_for(harness), &self.pending_deletes, hover_background, @@ -355,16 +368,22 @@ impl AuthSecretSelector { &mut self, harness: Harness, name: String, + owner: SecretOwner, ctx: &mut ViewContext, ) { let active_harness = self.ambient_agent_model.as_ref(ctx).selected_harness(); - let removed_pending = self.pending_deletes.remove(&name); + let removed_pending = self.pending_deletes.remove(&(harness, name.clone(), owner)); + + CloudAgentSettings::handle(ctx).update(ctx, |settings, ctx| { + let mut map = settings.last_selected_auth_secret.value().clone(); + if remove_persisted_auth_secret_selection_if_deleted(&mut map, harness, &name) { + report_if_error!(settings.last_selected_auth_secret.set_value(map, ctx)); + } + }); if harness == active_harness { // Drop the selection if it pointed at the just-deleted secret - // so the chip falls back to the inherit label. Persistence is - // cleared as well so the next session doesn't try to restore - // a non-existent secret. + // so the chip falls back to the inherit label. let selected = self .ambient_agent_model .as_ref(ctx) @@ -375,13 +394,6 @@ impl AuthSecretSelector { model.set_harness_auth_secret_name(None, ctx); }); } - CloudAgentSettings::handle(ctx).update(ctx, |settings, ctx| { - let mut map = settings.last_selected_auth_secret.value().clone(); - if map.get(harness.config_name()) == Some(&name) { - map.remove(harness.config_name()); - report_if_error!(settings.last_selected_auth_secret.set_value(map, ctx)); - } - }); self.refresh_menu(ctx); self.refresh_button(ctx); @@ -404,10 +416,11 @@ impl AuthSecretSelector { &mut self, harness: Harness, name: String, + owner: SecretOwner, error: String, ctx: &mut ViewContext, ) { - let removed_pending = self.pending_deletes.remove(&name); + let removed_pending = self.pending_deletes.remove(&(harness, name.clone(), owner)); let active_harness = self.ambient_agent_model.as_ref(ctx).selected_harness(); if harness == active_harness { self.refresh_menu(ctx); @@ -460,9 +473,12 @@ impl AuthSecretSelector { owner, } = pending_deletion; - // Drop duplicate dispatches while a delete for this same - // secret is still in flight (PRODUCT behavior 9). - if !self.pending_deletes.insert(name.clone()) { + // Drop duplicate dispatches while a delete for this exact + // harness-owned secret is still in flight (PRODUCT behavior 9). + if !self + .pending_deletes + .insert((harness, name.clone(), owner.clone())) + { return; } @@ -545,8 +561,9 @@ impl AuthSecretSelector { } fn build_main_menu_items( + harness: Harness, fetch_state: &AuthSecretFetchState, - pending_deletes: &HashSet, + pending_deletes: &HashSet, hover_background: Fill, header_text_color: pathfinder_color::ColorU, ) -> Vec> { @@ -573,7 +590,8 @@ fn build_main_menu_items( match fetch_state { AuthSecretFetchState::Loaded(secrets) => { for secret in secrets { - let is_pending_delete = pending_deletes.contains(&secret.name); + let is_pending_delete = + pending_deletes.contains(&(harness, secret.name.clone(), secret.owner.clone())); let fields = MenuItemFields::new(secret.name.clone()) .with_font_size_override(ITEM_FONT_SIZE) .with_padding_override(ITEM_VERTICAL_PADDING, MENU_HORIZONTAL_PADDING) @@ -624,6 +642,19 @@ fn build_main_menu_items( items } +fn remove_persisted_auth_secret_selection_if_deleted( + selections: &mut HashMap, + harness: Harness, + name: &str, +) -> bool { + if selections.get(harness.config_name()).map(String::as_str) == Some(name) { + selections.remove(harness.config_name()); + return true; + } + + false +} + fn build_sidecar_items( harness: Harness, hover_background: Fill, diff --git a/specs/cloud-mode-auth-secret-deletion/PRODUCT.md b/specs/cloud-mode-auth-secret-deletion/PRODUCT.md index 411abc42c0..3386867860 100644 --- a/specs/cloud-mode-auth-secret-deletion/PRODUCT.md +++ b/specs/cloud-mode-auth-secret-deletion/PRODUCT.md @@ -38,13 +38,13 @@ Figma: none provided. The prompt includes screenshots of the auth selector delet 8. Deletion is server-backed. A secret is considered deleted only after the server delete request succeeds. The UI must not present deletion as successful before the server confirms it. -9. While deletion is pending, the user should not be able to fire duplicate delete requests for the same secret from the same open menu. Acceptable treatments include disabling the `X` button for that row, closing the menu, or otherwise preventing a second click. +9. While deletion is pending, the user should not be able to fire duplicate delete requests for the same harness-owned secret from the same open menu. Pending delete state must stay scoped to the deletion target rather than every same-named secret. Acceptable treatments include disabling the `X` button for that row, closing the menu, or otherwise preventing a second click. 10. On successful deletion: - The deleted secret disappears from the selector menu the next time the menu is shown, and ideally immediately if the menu remains open. - A success toast is shown. - If the deleted secret was currently selected for the active harness, the selected auth secret is cleared and the chip falls back to the no-secret/inherit label. - - Any persisted last-selected secret for the active harness is cleared if it points to the deleted secret. + - Any persisted last-selected secret for the deleted harness is cleared if it points to the deleted secret, even if the user changes active harnesses before the server response arrives. 11. On failed deletion: - The secret remains available in the selector menu. diff --git a/specs/cloud-mode-auth-secret-deletion/TECH.md b/specs/cloud-mode-auth-secret-deletion/TECH.md index 4b6a92d79c..f3fcdf71ed 100644 --- a/specs/cloud-mode-auth-secret-deletion/TECH.md +++ b/specs/cloud-mode-auth-secret-deletion/TECH.md @@ -45,12 +45,12 @@ The current `AuthSecretEntry` contains only `name`, but server `ManagedSecret` r ) ``` - The method should call `ManagedSecretManager::delete_secret(owner, name.clone())`. On success, remove the matching entry from `AuthSecretFetchState::Loaded` for that harness and emit a new event. On failure, leave the cache unchanged, report the error, and emit a failure event with the harness/name/error. + The method should call `ManagedSecretManager::delete_secret(owner, name.clone())`. On success, remove the matching owner/name entry from `AuthSecretFetchState::Loaded` for that harness and emit a new event. On failure, leave the cache unchanged, report the error, and emit a failure event with the harness/name/owner/error. Add events: - - `AuthSecretDeleted { harness, name }` - - `AuthSecretDeletionFailed { harness, name, error }` + - `AuthSecretDeleted { harness, name, owner }` + - `AuthSecretDeletionFailed { harness, name, owner, error }` Existing subscribers that only care about menu freshness should refresh on `AuthSecretDeleted`; subscribers that do not care should explicitly ignore both new events so match exhaustiveness remains clear. @@ -111,10 +111,10 @@ The current `AuthSecretEntry` contains only `name`, but server `ManagedSecret` r In the selector's `HarnessAvailabilityModel` subscription: - - On `AuthSecretDeleted` for the active harness, clear pending state for the matching secret, refresh menu/button, and show a success toast. - - If the deleted name equals `AmbientAgentViewModel::selected_harness_auth_secret_name()`, call `set_harness_auth_secret_name(None, ctx)`. - - Remove `CloudAgentSettings.last_selected_auth_secret[harness.config_name()]` if it equals the deleted name. - - On `AuthSecretDeletionFailed`, clear pending state, refresh menu, and show a failure toast containing the user-facing error when available. + - On `AuthSecretDeleted`, clear pending state for the matching `(harness, name, owner)` target and show a success toast when this selector initiated that exact deletion. + - Always remove `CloudAgentSettings.last_selected_auth_secret[harness.config_name()]` if it equals the deleted name, even when the user has switched to another active harness before the server responds. + - Only when the deleted harness is still active, clear `AmbientAgentViewModel::selected_harness_auth_secret_name()` if it matches the deleted name and refresh the visible menu/button state. + - On `AuthSecretDeletionFailed`, clear pending state for the matching `(harness, name, owner)` target, refresh the visible menu only when that harness is active, and show a failure toast containing the user-facing error when available. Use `DismissibleToast::success(...)` and `DismissibleToast::error(...)` via `ToastStack::add_ephemeral_toast`, matching nearby toast patterns. @@ -124,7 +124,7 @@ The current `AuthSecretEntry` contains only `name`, but server `ManagedSecret` r ## Testing and Validation -Per the current implementation pass, automated test additions and test execution are deferred by request. The implementation should remain structured so the following cases are straightforward to cover later: +No new helper-only unit tests are retained in this implementation pass. The following behaviors remain suitable for future targeted coverage if higher-value seams are introduced: 1. Cache update behavior for PRODUCT behaviors 8, 10, and 11: - Successful deletion removes only the matching auth secret from the loaded list and emits `AuthSecretDeleted`. From cb0179a45af9e9e2698ec0a40ad2de7bfee80aa6 Mon Sep 17 00:00:00 2001 From: abhishekp106 Date: Tue, 26 May 2026 17:06:24 -0500 Subject: [PATCH 8/8] PR feedback --- app/src/menu.rs | 148 +++++++++--------- app/src/pane_group/mod.rs | 3 - .../ambient_agent/auth_secret_selector.rs | 39 +++-- .../delete_auth_secret_confirmation_dialog.rs | 6 +- 4 files changed, 97 insertions(+), 99 deletions(-) diff --git a/app/src/menu.rs b/app/src/menu.rs index e049f823e4..373fcd510c 100644 --- a/app/src/menu.rs +++ b/app/src/menu.rs @@ -34,12 +34,6 @@ use warpui::{ Action, AppContext, Entity, SingletonEntity, TypedActionView, View, ViewContext, WindowId, }; -use crate::appearance::Appearance; -use crate::safe_triangle::SafeTriangle; -use crate::themes::theme::Fill; -use crate::ui_components::icons; -use crate::util::time_format::format_approx_duration_from_now_sentence_case; - pub const CHEVRON_RIGHT_ALIGN_SVG_PATH: &str = "bundled/svg/chevron-right-align.svg"; const SUBMENU_OVERLAP: f32 = 8.; @@ -398,6 +392,47 @@ struct RightSideLabel { text: String, font_properties: Properties, } +#[derive(Clone)] +struct RightSideIconConfig { + icon: icons::Icon, + override_color: Option, + /// 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. + action: Option, + /// Optional accessibility label for the right-side icon hit target. + a11y_label: Option, + /// When true, the right-side icon is rendered as disabled with no hover or + /// click action. + disabled: bool, + /// Tracks hover state independently from the row. + mouse_state: MouseStateHandle, +} + +impl RightSideIconConfig { + fn new(icon: icons::Icon) -> Self { + Self { + icon, + override_color: None, + action: None, + a11y_label: None, + disabled: false, + mouse_state: MouseStateHandle::default(), + } + } + + fn without_action(self) -> RightSideIconConfig { + RightSideIconConfig { + icon: self.icon, + override_color: self.override_color, + action: None, + a11y_label: self.a11y_label, + disabled: self.disabled, + mouse_state: self.mouse_state, + } + } +} #[derive(Clone, Default)] pub struct MenuItemFields { @@ -424,21 +459,7 @@ pub struct MenuItemFields { tooltip: Option, tooltip_position: MenuTooltipPosition, right_side_label: Option, - right_side_icon: Option<(icons::Icon, Option)>, - /// 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, - /// Optional accessibility label for the right-side icon hit target. - right_side_icon_a11y_label: Option, - /// 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, + right_side_icon: Option>, /// 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 @@ -490,10 +511,6 @@ impl MenuItemFields { 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, @@ -524,10 +541,6 @@ impl MenuItemFields { 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, @@ -561,10 +574,6 @@ impl MenuItemFields { 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, @@ -601,10 +610,6 @@ impl MenuItemFields { 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, @@ -639,10 +644,6 @@ impl MenuItemFields { 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, @@ -676,10 +677,6 @@ impl MenuItemFields { 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, @@ -710,10 +707,6 @@ impl MenuItemFields { 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, @@ -759,15 +752,13 @@ impl MenuItemFields { tooltip: self.tooltip, 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`; we can't safely - // map it to `Option` 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, + // The right-side icon action is `Option`; we can't safely map + // it to `Option` 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: self + .right_side_icon + .map(RightSideIconConfig::without_action), override_hover_background_color: self.override_hover_background_color, icon_size_override: self.icon_size_override, clip_config: self.clip_config, @@ -903,7 +894,7 @@ impl MenuItemFields { } pub fn with_right_side_icon(mut self, icon: icons::Icon) -> Self { - self.right_side_icon = Some((icon, None)); + self.right_side_icon = Some(RightSideIconConfig::new(icon)); self } @@ -912,17 +903,23 @@ impl MenuItemFields { /// `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); + if let Some(config) = &mut self.right_side_icon { + config.action = Some(action); + } self } pub fn with_right_side_icon_a11y_label(mut self, label: impl Into) -> Self { - self.right_side_icon_a11y_label = Some(label.into()); + if let Some(config) = &mut self.right_side_icon { + config.a11y_label = Some(label.into()); + } self } pub fn with_right_side_icon_disabled(mut self, disabled: bool) -> Self { - self.right_side_icon_disabled = disabled; + if let Some(config) = &mut self.right_side_icon { + config.disabled = disabled; + } self } @@ -1064,24 +1061,24 @@ impl MenuItemFields { color: Fill, dispatch_item_actions: bool, ) -> Option> { - let (icon, override_color) = self.right_side_icon.as_ref()?; + let config = 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); - if let Some(action) = &self.right_side_icon_action { + let icon_color = config.override_color.unwrap_or(color); + if let Some(action) = &config.action { let mut button = icon_button_with_color( appearance, - *icon, + config.icon, false, - self.right_side_icon_mouse_state.clone(), + config.mouse_state.clone(), icon_color, ); - if self.right_side_icon_disabled { + if config.disabled { button = button.disabled(); } let mut hoverable = button.build(); - if !self.right_side_icon_disabled { + if !config.disabled { let action = action.clone(); hoverable = hoverable.on_click(move |ctx, _, _| { if dispatch_item_actions { @@ -1092,12 +1089,20 @@ impl MenuItemFields { // doesn't latch onto the press that targets the icon. hoverable = hoverable.on_mouse_down(|_, _, _| {}); } - let element = Container::new(hoverable.finish()) + let button_element = if config.disabled { + EventHandler::new(hoverable.finish()) + .on_left_mouse_down(|_, _, _| DispatchEventResult::StopPropagation) + .on_left_mouse_up(|_, _, _| DispatchEventResult::StopPropagation) + .finish() + } else { + hoverable.finish() + }; + let element = Container::new(button_element) .with_margin_left(icon_size / 2.) .finish(); return Some(Shrinkable::new(1., Align::new(element).right().finish()).finish()); } - let icon_element = ConstrainedBox::new(icon.to_warpui_icon(icon_color).finish()) + let icon_element = ConstrainedBox::new(config.icon.to_warpui_icon(icon_color).finish()) .with_width(icon_size) .with_height(icon_size) .finish(); @@ -1399,6 +1404,7 @@ impl MenuItemFields { }); } }); + ret = ret.with_defer_events_to_children(); let on_select_action = self.on_select_action.clone(); diff --git a/app/src/pane_group/mod.rs b/app/src/pane_group/mod.rs index 73f9dc1777..382c412f29 100644 --- a/app/src/pane_group/mod.rs +++ b/app/src/pane_group/mod.rs @@ -5299,9 +5299,6 @@ impl PaneGroup { if self.pane_with_open_auth_secret_delete_confirmation_dialog == Some(pane_id) { self.pane_with_open_auth_secret_delete_confirmation_dialog = None; } - if self.pane_with_open_auth_secret_delete_confirmation_dialog == Some(pane_id) { - self.pane_with_open_auth_secret_delete_confirmation_dialog = None; - } if self.pane_with_open_agent_assisted_environment_modal == Some(pane_id) { self.pane_with_open_agent_assisted_environment_modal = None; } diff --git a/app/src/terminal/view/ambient_agent/auth_secret_selector.rs b/app/src/terminal/view/ambient_agent/auth_secret_selector.rs index 1785682a86..bc2307d54e 100644 --- a/app/src/terminal/view/ambient_agent/auth_secret_selector.rs +++ b/app/src/terminal/view/ambient_agent/auth_secret_selector.rs @@ -371,7 +371,6 @@ impl AuthSecretSelector { owner: SecretOwner, ctx: &mut ViewContext, ) { - let active_harness = self.ambient_agent_model.as_ref(ctx).selected_harness(); let removed_pending = self.pending_deletes.remove(&(harness, name.clone(), owner)); CloudAgentSettings::handle(ctx).update(ctx, |settings, ctx| { @@ -381,24 +380,22 @@ impl AuthSecretSelector { } }); - if harness == active_harness { - // Drop the selection if it pointed at the just-deleted secret - // so the chip falls back to the inherit label. - let selected = self - .ambient_agent_model - .as_ref(ctx) - .selected_harness_auth_secret_name() - .map(|s| s.to_string()); - if selected.as_deref() == Some(name.as_str()) { - self.ambient_agent_model.update(ctx, |model, ctx| { - model.set_harness_auth_secret_name(None, ctx); - }); - } - - self.refresh_menu(ctx); - self.refresh_button(ctx); + // Drop the selection if it pointed at the just-deleted secret + // so the chip falls back to the inherit label. + let selected = self + .ambient_agent_model + .as_ref(ctx) + .selected_harness_auth_secret_name() + .map(|s| s.to_string()); + if selected.as_deref() == Some(name.as_str()) { + self.ambient_agent_model.update(ctx, |model, ctx| { + model.set_harness_auth_secret_name(None, ctx); + }); } + self.refresh_menu(ctx); + self.refresh_button(ctx); + // Only surface a toast for a deletion *this* selector initiated. // A deletion fired from a different surface (or a different // window) shouldn't pop a duplicate confirmation here. @@ -409,7 +406,11 @@ impl AuthSecretSelector { ts.add_ephemeral_toast(DismissibleToast::success(message), window_id, ctx); }); } - ctx.notify(); + + let active_harness = self.ambient_agent_model.as_ref(ctx).selected_harness(); + if harness == active_harness { + ctx.notify(); + } } fn handle_secret_deletion_failed( @@ -473,8 +474,6 @@ impl AuthSecretSelector { owner, } = pending_deletion; - // Drop duplicate dispatches while a delete for this exact - // harness-owned secret is still in flight (PRODUCT behavior 9). if !self .pending_deletes .insert((harness, name.clone(), owner.clone())) diff --git a/app/src/terminal/view/ambient_agent/delete_auth_secret_confirmation_dialog.rs b/app/src/terminal/view/ambient_agent/delete_auth_secret_confirmation_dialog.rs index e333aefcae..89cda409f9 100644 --- a/app/src/terminal/view/ambient_agent/delete_auth_secret_confirmation_dialog.rs +++ b/app/src/terminal/view/ambient_agent/delete_auth_secret_confirmation_dialog.rs @@ -35,7 +35,6 @@ pub(super) enum DeleteAuthSecretConfirmationDialogAction { } pub(super) struct DeleteAuthSecretConfirmationDialog { - visible: bool, pending_deletion: Option, cancel_button: ViewHandle, delete_button: ViewHandle, @@ -56,7 +55,6 @@ impl DeleteAuthSecretConfirmationDialog { }); Self { - visible: false, pending_deletion: None, cancel_button, delete_button, @@ -69,12 +67,10 @@ impl DeleteAuthSecretConfirmationDialog { ctx: &mut ViewContext, ) { self.pending_deletion = Some(pending_deletion); - self.visible = true; ctx.notify(); } pub(super) fn hide(&mut self, ctx: &mut ViewContext) { - self.visible = false; self.pending_deletion = None; ctx.notify(); } @@ -90,7 +86,7 @@ impl View for DeleteAuthSecretConfirmationDialog { } fn render(&self, app: &AppContext) -> Box { - if !self.visible { + if self.pending_deletion.is_none() { return Empty::new().finish(); }