Implement auth secret deletion from selector menu by abhishekp106 · Pull Request #11241 · warpdotdev/warp

abhishekp106 · 2026-05-18T22:34:33Z

Summary

Adds the ability to delete secrets from the auth selector menu. I copied the modal design from the web app.

Testing

https://www.loom.com/share/40647dedc738436e9106ce763c7c98c6

This PR was created by Oz (running Claude Code).

Co-Authored-By: Oz <oz-agent@warp.dev>

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 <oz-agent@warp.dev>

abhishekp106 · 2026-05-20T20:47:00Z

Implement auth secret deletion from selector menu #11241 👈 (View in Graphite)
master

This stack of pull requests is managed by Graphite. Learn more about stacking.

oz-for-oss · 2026-05-20T21:00:56Z

@abhishekp106

I'm starting a first review of this pull request.

You can view the conversation on Warp.

I completed the review and no human review was requested for this pull request.

Comment /oz-review on this pull request to retrigger a review (up to 3 times on the same pull request).

Powered by Oz

oz-for-oss

Overview

This PR adds owner metadata for harness auth secrets, a selector-menu delete affordance, a destructive confirmation dialog, server-backed deletion handling, and refresh/toast behavior for related auth-secret surfaces.

Concerns

Clicking the right-side delete icon can still dispatch the parent row selection because the menu row does not defer child-handled events, and the disabled icon state does not consume clicks.
Successful deletion updates the cached auth-secret list by name only even though the server delete is scoped by owner, so same-named user/team secrets can be hidden incorrectly.
Delete completion only clears selected/persisted state when the deleted harness is still the active harness, leaving stale settings if the user switches harnesses while deletion is in flight.

Verdict

Found: 0 critical, 3 important, 0 suggestions

Request changes

Comment /oz-review on this pull request to retrigger a review (up to 3 times on the same pull request).

Powered by Oz

oz-for-oss · 2026-05-20T21:06:26Z

+                });
+                // 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(|_, _, _| {});


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

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

oz-for-oss · 2026-05-20T21:06:26Z

+                if let Some(AuthSecretFetchState::Loaded(entries)) =
+                    me.auth_secrets.get_mut(&harness)
+                {
+                    entries.retain(|entry| entry.name != name);


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

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

oz-for-oss · 2026-05-20T21:06:26Z

+        let active_harness = self.ambient_agent_model.as_ref(ctx).selected_harness();
+        let removed_pending = self.pending_deletes.remove(&name);
+
+        if harness == active_harness {


⚠️ [IMPORTANT] This gates selected/persisted cleanup on whichever harness is active when the async delete completes. If the user confirms deletion and switches harnesses before the server responds, last_selected_auth_secret for the deleted harness remains and can restore a non-existent secret later; clear persisted state for the event harness regardless of current visibility, and only limit UI refreshes to the active harness.

This seems legit

liliwilson

Woohoo, this is awesome! I can finally make test keys with horrible names and get rid of them at lightning speed

liliwilson · 2026-05-21T05:36:32Z

+
+   Add a small destructive dialog view under `app/src/terminal/view/ambient_agent/` that mirrors existing confirmation dialogs:
+
+   - Title: `Delete secret`


nit: "Delete credentials" maybe better, if we're trying not to use the secret language here?

liliwilson · 2026-05-21T06:07:16Z

+                if let Some(AuthSecretFetchState::Loaded(entries)) =
+                    me.auth_secrets.get_mut(&harness)
+                {
+                    entries.retain(|entry| entry.name != name);


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

liliwilson · 2026-05-21T06:08:47Z

+            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) {


Accidental extra copy paste?

liliwilson · 2026-05-21T06:12:28Z

+                .terminal_view_from_pane_id(pane_id, app)
+                .and_then(|tv| {
+                    tv.as_ref(app)
+                        .auth_secret_delete_confirmation_dialog_element(app)


OOC why pass the element back directly here instead of the handle? (Noticing the surrounding fns use handle)

liliwilson · 2026-05-21T06:14:05Z

+        let active_harness = self.ambient_agent_model.as_ref(ctx).selected_harness();
+        let removed_pending = self.pending_deletes.remove(&name);
+
+        if harness == active_harness {


This seems legit

liliwilson · 2026-05-21T06:19:32Z

+        } = pending_deletion;
+
+        // Drop duplicate dispatches while a delete for this same
+        // secret is still in flight (PRODUCT behavior 9).


nit: can we remove the PRODUCT reference from this comment

liliwilson · 2026-05-21T06:23:55Z

+    visible: bool,
+    pending_deletion: Option<PendingAuthSecretDeletion>,


Do we need these to be two separate fields, or can they be consolidated into the pending_deletion Option type

liliwilson · 2026-05-21T06:26:53Z

    right_side_icon: Option<(icons::Icon, Option<Fill>)>,
+    /// Optional action dispatched when the right-side icon is clicked. When
+    /// set, the right-side icon becomes its own hit target: clicking it
+    /// dispatches this action without firing the row's own `on_select_action`,
+    /// and prevents the row click from propagating.
+    right_side_icon_action: Option<A>,
+    /// Optional accessibility label for the right-side icon hit target.
+    right_side_icon_a11y_label: Option<String>,
+    /// When true, the right-side icon is rendered as disabled (no hover,
+    /// no click action). Used to lock the affordance while a pending
+    /// request is in flight.
+    right_side_icon_disabled: bool,
+    /// Optional mouse state for the right-side icon so its hover state is
+    /// tracked independently from the row.
+    right_side_icon_mouse_state: MouseStateHandle,


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

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

liliwilson · 2026-05-21T06:30:01Z

+                });
+                // 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(|_, _, _| {});


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

liliwilson · 2026-05-21T06:33:18Z

            .with_override_hover_background_color(hover_background)
            .with_icon(Icon::Plus)
-            .with_right_side_label("›", Properties::default())
+            .with_right_side_icon(Icon::ChevronRight)


Not sure if this is where this changed but the ">" seems a bit close now

Add auth secret deletion specs

09a2310

Co-Authored-By: Oz <oz-agent@warp.dev>

cla-bot Bot added the cla-signed label May 18, 2026

abhishekp106 changed the title ~~Add auth secret deletion specs~~ Implement auth secret deletion from selector menu May 18, 2026

abhishekp106 added 2 commits May 20, 2026 14:27

delete button working

e832e87

Working

fdcd967

abhishekp106 requested a review from liliwilson May 20, 2026 20:48

abhishekp106 marked this pull request as ready for review May 20, 2026 21:00

oz-for-oss Bot reviewed May 20, 2026

View reviewed changes

liliwilson approved these changes May 21, 2026

View reviewed changes


		Add a small destructive dialog view under `app/src/terminal/view/ambient_agent/` that mirrors existing confirmation dialogs:

		- Title: `Delete secret`

		visible: bool,
		pending_deletion: Option<PendingAuthSecretDeletion>,

Conversation

abhishekp106 commented May 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Testing

Uh oh!

abhishekp106 commented May 20, 2026

Uh oh!

oz-for-oss Bot commented May 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

oz-for-oss Bot left a comment

Choose a reason for hiding this comment

Overview

Concerns

Verdict

Uh oh!

oz-for-oss Bot May 20, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

oz-for-oss Bot May 20, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

oz-for-oss Bot May 20, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

liliwilson left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

abhishekp106 commented May 18, 2026 •

edited

Loading

oz-for-oss Bot commented May 20, 2026 •

edited

Loading