Confirm before destructive actions (revoke key, remove policy, remove member)

e2e/scenarios/policy-remove-confirm.test.ts

@@ -0,0 +1,85 @@
+// Cross-target (browser): removing a tool-access policy asks for confirmation
+// first. Remove fires from a row's dropdown menu and is irreversible — the
+// rule is gone and every tool it governed silently falls back to the default
+// policy — so the menu item must open a confirm dialog rather than firing the
+// mutation directly. Cancel keeps the rule; confirming removes it for real
+// (asserted through the API, not just the rendered list).
+import { randomBytes } from "node:crypto";
+
+import { expect } from "@effect/vitest";
+import { Effect } from "effect";
+import { composePluginApi } from "@executor-js/api/server";
+
+import { scenario } from "../src/scenario";
+import { Api, Browser, Target } from "../src/services";
+import { visit } from "../src/surfaces/browser";
+
+const coreApi = composePluginApi([] as const);
+
+scenario(
+  "Policies (UI) · Remove asks for confirmation; cancel keeps, confirm removes",
+  {},
+  Effect.gen(function* () {
+    const target = yield* Target;
+    const browser = yield* Browser;
+    const { client: makeClient } = yield* Api;
+    const identity = yield* target.newIdentity();
+    const client = yield* makeClient(coreApi, identity);
+
+    // Selfhost scenarios share one workspace, so the pattern is unique to this
+    // run and the row lookup below cannot match another scenario's rule.
+    const pattern = `rmconfirm-${randomBytes(4).toString("hex")}.*`;
+
+    const created = yield* client.policies.create({
+      payload: { owner: "org", pattern, action: "block" },
+    });
+
+    yield* Effect.ensuring(
+      Effect.gen(function* () {
+        yield* browser.session(identity, async ({ page, step }) => {
+          const row = page.locator("[data-slot='card-stack-entry']").filter({ hasText: pattern });
+          const menuTrigger = row.locator('button[aria-haspopup="menu"]');
+          const confirm = page.getByRole("alertdialog");
+
+          await step("Open the policies page", async () => {
+            await visit(page, "/policies");
+            await row.waitFor();
+          });
+
+          await step("Remove asks for confirmation instead of firing", async () => {
+            await menuTrigger.click();
+            await page.getByRole("menuitem", { name: "Remove" }).click();
+            await confirm.getByText("Remove policy?").waitFor();
+            // The dialog names the exact rule being destroyed.
+            await confirm.getByText(pattern, { exact: false }).waitFor();
+          });
+
+          await step("Cancel keeps the policy", async () => {
+            await confirm.getByRole("button", { name: "Cancel" }).click();
+            await confirm.waitFor({ state: "detached" });
+            await row.waitFor();
+          });
+
+          await step("Confirming actually removes it", async () => {
+            await menuTrigger.click();
+            await page.getByRole("menuitem", { name: "Remove" }).click();
+            await confirm.getByRole("button", { name: "Remove policy" }).click();
+            await confirm.waitFor({ state: "detached" });
+            await row.waitFor({ state: "detached" });
+          });
+        });
+
+        // Cancel left the removal un-fired and confirm fired it for real: the
+        // rule is gone from the API, not merely from the rendered list.
+        const remaining = yield* client.policies.list();
+        expect(
+          remaining.map((policy) => policy.pattern),
+          "the removed policy is gone from the API",
+        ).not.toContain(pattern);
+      }),
+      client.policies
+        .remove({ params: { policyId: created.id }, payload: { owner: "org" } })
+        .pipe(Effect.ignore),
+    );
+  }),
+);

packages/react/src/pages/api-keys.tsx

@@ -233,6 +233,7 @@ export function ApiKeysPage(props: { readonly orgKeysSection?: ReactNode }) {
   // stays mounted for Radix's exit animation (see CreateKeyDialogBody).
   const [openCount, setOpenCount] = useState(0);
   const [revokingId, setRevokingId] = useState<string | null>(null);
+  const [confirmRevoke, setConfirmRevoke] = useState<ApiKeySummary | null>(null);
 
   const handleCreate = async (name: string): Promise<CreatedKey | null> => {
     const exit = await doCreate({ payload: { name }, reactivityKeys: apiKeyWriteKeys });
@@ -312,7 +313,11 @@ export function ApiKeysPage(props: { readonly orgKeysSection?: ReactNode }) {
                   </p>
                 </div>
               ) : (
-                <KeyTable keys={value.apiKeys} revokingId={revokingId} onRevoke={handleRevoke} />
+                <KeyTable
+                  keys={value.apiKeys}
+                  revokingId={revokingId}
+                  onRevoke={(key) => setConfirmRevoke(key)}
+                />
               ),
           })
         )}
@@ -333,6 +338,42 @@ export function ApiKeysPage(props: { readonly orgKeysSection?: ReactNode }) {
           />
         </DialogContent>
       </Dialog>
+
+      {/* A personal key revoke breaks any script or tool using it, so it asks
+          first — same as the org-key revoke. */}
+      <Dialog
+        open={confirmRevoke !== null}
+        onOpenChange={(open) => {
+          if (!open) setConfirmRevoke(null);
+        }}
+      >
+        <DialogContent className="sm:max-w-[480px]">
+          <DialogHeader>
+            <DialogTitle className="font-display text-xl">Revoke API key</DialogTitle>
+            <DialogDescription className="text-sm leading-relaxed">
+              {confirmRevoke
+                ? `Revoke ${confirmRevoke.name}? Any script or tool authenticating with it loses access immediately. This cannot be undone.`
+                : ""}
+            </DialogDescription>
+          </DialogHeader>
+          <DialogFooter>
+            <DialogClose asChild>
+              <Button variant="ghost">Cancel</Button>
+            </DialogClose>
+            <Button
+              variant="destructive"
+              onClick={() => {
+                if (confirmRevoke) {
+                  void handleRevoke(confirmRevoke);
+                  setConfirmRevoke(null);
+                }
+              }}
+            >
+              Revoke key
+            </Button>
+          </DialogFooter>
+        </DialogContent>
+      </Dialog>
     </PageContainer>
   );
 }
@@ -492,8 +533,7 @@ function OrgApiKeysSectionBody() {
         </DialogContent>
       </Dialog>
 
-      {/* Revoking an org key breaks every backend using it, so it is the one
-          revoke on this page that asks first. */}
+      {/* Revoking an org key breaks every backend using it, so it asks first. */}
       <Dialog
         open={confirmRevoke !== null}
         onOpenChange={(open) => {

packages/react/src/pages/org.tsx

@@ -14,6 +14,16 @@ import {
   DialogFooter,
   DialogClose,
 } from "../components/dialog";
+import {
+  AlertDialog,
+  AlertDialogAction,
+  AlertDialogCancel,
+  AlertDialogContent,
+  AlertDialogDescription,
+  AlertDialogFooter,
+  AlertDialogHeader,
+  AlertDialogTitle,
+} from "../components/alert-dialog";
 import { Button } from "../components/button";
 import { PageContainer, PageHeader } from "../components/page";
 import { Badge } from "../components/badge";
@@ -153,6 +163,7 @@ export function OrgPage(props: {
   const refreshMembers = useAtomRefresh(orgMembersAtom);
   const rolesResult = useAtomValue(orgRolesAtom);
   const doRemove = useAtomSet(removeMember, { mode: "promiseExit" });
+  const [removingMember, setRemovingMember] = useState<{ id: string; name: string } | null>(null);
   const doUpdateRole = useAtomSet(updateMemberRole, { mode: "promiseExit" });
   const doUpdateOrgName = useAtomSet(updateOrgName, { mode: "promiseExit" });
   const [inviteOpen, setInviteOpen] = useState(false);
@@ -179,6 +190,7 @@ export function OrgPage(props: {
   const showUpgradeOnInvite = atSeatLimit && !!props.upgradeAction;
 
   const handleRemove = async (membershipId: string, name: string) => {
+    setRemovingMember(null);
     const exit = await doRemove({
       params: { membershipId },
       reactivityKeys: orgMemberWriteKeys,
@@ -406,7 +418,12 @@ export function OrgPage(props: {
                             )}
                             <DropdownMenuItem
                               className="text-destructive focus:text-destructive text-sm"
-                              onClick={() => handleRemove(member.id, member.name ?? member.email)}
+                              onClick={() =>
+                                setRemovingMember({
+                                  id: member.id,
+                                  name: member.name ?? member.email,
+                                })
+                              }
                             >
                               Remove member
                             </DropdownMenuItem>
@@ -426,6 +443,38 @@ export function OrgPage(props: {
 
       {props.dangerZoneSection}
 
+      <AlertDialog
+        open={removingMember !== null}
+        onOpenChange={(open: boolean) => {
+          if (!open) setRemovingMember(null);
+        }}
+      >
+        <AlertDialogContent size="sm">
+          <AlertDialogHeader>
+            <AlertDialogTitle>
+              {removingMember ? `Remove ${removingMember.name}?` : "Remove member?"}
+            </AlertDialogTitle>
+            <AlertDialogDescription>
+              They lose access to this organization immediately. This cannot be undone; you would
+              need to invite them again.
+            </AlertDialogDescription>
+          </AlertDialogHeader>
+          <AlertDialogFooter>
+            <AlertDialogCancel>Cancel</AlertDialogCancel>
+            <AlertDialogAction
+              variant="destructive"
+              onClick={() => {
+                if (removingMember !== null) {
+                  void handleRemove(removingMember.id, removingMember.name);
+                }
+              }}
+            >
+              Remove member
+            </AlertDialogAction>
+          </AlertDialogFooter>
+        </AlertDialogContent>
+      </AlertDialog>
+
       <InviteDialog open={inviteOpen} onOpenChange={setInviteOpen} roles={roles} />
       <UpgradeDialog
         open={upgradeOpen}

packages/react/src/pages/policies.tsx

@@ -30,6 +30,16 @@ import {
 } from "../lib/policy-display";
 import { Button } from "../components/button";
 import { PageContainer, PageHeader } from "../components/page";
+import {
+  AlertDialog,
+  AlertDialogAction,
+  AlertDialogCancel,
+  AlertDialogContent,
+  AlertDialogDescription,
+  AlertDialogFooter,
+  AlertDialogHeader,
+  AlertDialogTitle,
+} from "../components/alert-dialog";
 import {
   CardStack,
   CardStackContent,
@@ -291,6 +301,11 @@ export function PoliciesPage() {
   const doUpdate = useAtomSet(updatePolicyOptimistic, { mode: "promiseExit" });
   const doRemove = useAtomSet(removePolicyOptimistic, { mode: "promiseExit" });
   const [busy, setBusy] = useState(false);
+  const [removingPolicy, setRemovingPolicy] = useState<{
+    id: string;
+    owner: Owner;
+    pattern: string;
+  } | null>(null);
   const ownerDisplay = useOwnerDisplay();
   // Policies default to org/workspace. On local this is the hidden Local owner
   // that v1 local data migrates into.
@@ -336,6 +351,7 @@ export function PoliciesPage() {
   };
 
   const handleRemove = async (policy: { id: string; owner: Owner }) => {
+    setRemovingPolicy(null);
     const exit = await doRemove({
       params: { policyId: PolicyId.make(policy.id) },
       payload: { owner: policy.owner },
@@ -462,7 +478,9 @@ export function PoliciesPage() {
                           isFirst={!reorderable || j === 0}
                           isLast={!reorderable || j === committed.length - 1}
                           showOwnerLabel={ownerDisplay.showOwnerLabels}
-                          onRemove={() => handleRemove({ id: p.id, owner: p.owner })}
+                          onRemove={() =>
+                            setRemovingPolicy({ id: p.id, owner: p.owner, pattern: p.pattern })
+                          }
                           onChangeAction={(action) =>
                             handleUpdate({ id: p.id, owner: p.owner }, action)
                           }
@@ -490,6 +508,37 @@ export function PoliciesPage() {
           },
         })
       )}
+
+      <AlertDialog
+        open={removingPolicy !== null}
+        onOpenChange={(open: boolean) => {
+          if (!open) setRemovingPolicy(null);
+        }}
+      >
+        <AlertDialogContent size="sm">
+          <AlertDialogHeader>
+            <AlertDialogTitle>Remove policy?</AlertDialogTitle>
+            <AlertDialogDescription>
+              {removingPolicy
+                ? `The rule for "${removingPolicy.pattern}" will be deleted. Tools it governs fall back to the default policy. This cannot be undone.`
+                : ""}
+            </AlertDialogDescription>
+          </AlertDialogHeader>
+          <AlertDialogFooter>
+            <AlertDialogCancel>Cancel</AlertDialogCancel>
+            <AlertDialogAction
+              variant="destructive"
+              onClick={() => {
+                if (removingPolicy !== null) {
+                  void handleRemove({ id: removingPolicy.id, owner: removingPolicy.owner });
+                }
+              }}
+            >
+              Remove policy
+            </AlertDialogAction>
+          </AlertDialogFooter>
+        </AlertDialogContent>
+      </AlertDialog>
     </PageContainer>
   );
 }