diff --git a/.claude/skills/self-contained-modals/SKILL.md b/.claude/skills/self-contained-modals/SKILL.md
new file mode 100644
index 000000000..6ff660805
--- /dev/null
+++ b/.claude/skills/self-contained-modals/SKILL.md
@@ -0,0 +1,77 @@
+---
+name: self-contained-modals
+description: "Build modals/dialogs self-contained: form and in-flight state lives inside, closing unmounts it. Use when writing or reviewing a modal/dialog, especially one that owns async work (OAuth popups, timers, subscriptions, AbortControllers). Catches the stuck-on-Connecting class of lifecycle-leak bugs."
+---
+
+# Self-contained modals
+
+A modal's **form state and in-flight work live INSIDE the modal**, and **closing
+the modal UNMOUNTS that state** so it is destroyed, not hand-reset. Only the
+**open/route intent** belongs to the parent (deep links, programmatic open,
+reconnect handoffs need it).
+
+## Why
+
+A hand-written `reset()` has to enumerate every field, and it silently drifts out
+of sync with state owned by **child hooks the parent can't see**. Unmounting
+resets everything for free, including child-hook cleanup effects (cancelling a
+dangling server session, clearing a timer, aborting a fetch).
+
+Concrete bug this prevents (executor, add-account-modal): the modal was mounted
+unconditionally, so `useOAuthPopupFlow`'s `busy` survived close. `reset()` zeroed
+its own booleans but never called `oauthPopup.cancel()`. Abandon the OAuth popup,
+close, reopen, and `oauthBusy = false || busy(true) = true`, so the footer is
+wedged on "Connecting…" with Close disabled. Unmounting would have cleared `busy`
+AND run the hook's cleanup that cancels the server OAuth session.
+
+## How to apply
+
+Default to **genuine conditional unmount**, state inside. When closed the
+component returns `null`, so React destroys all of it and runs every child
+hook's cleanup. This is the cleanest fix and the one to reach for first:
+
+```tsx
+function Parent() {
+ const [open, setOpen] = useState(false);
+ return open ? setOpen(false)} /> : null;
+}
+```
+
+A key bump (``) is **still a manual reset**, just
+spelled as a remount. Prefer real unmount; only reach for keyed remount in the
+one case below.
+
+That case: **Radix Dialog** (this repo's `components/dialog.tsx`) Content/Overlay
+use `data-[state=closed]:animate-out` exit animations, so unmounting the whole
+`Dialog` drops the close animation. If you must keep that animation, keep the
+`Dialog` + `DialogContent` shell mounted and remount only the state-bearing
+**body** per open:
+
+```tsx
+function Parent() {
+ const [open, setOpen] = useState(false);
+ const [openCount, setOpenCount] = useState(0);
+ return (
+
+ );
+}
+```
+
+This only works when `DialogContent` is **independent of body state**. If the
+shell depends on the body (e.g. a width className driven by the body's current
+sub-view), the body must own `DialogContent`, so there is no stable shell to
+keep, and genuine unmount (losing the exit animation) is the right call. That is
+exactly the executor add-account-modal: it genuinely unmounts and accepts the
+lost animation rather than plumbing body state up to a shell.
+
+## Reviewing: flag these smells
+
+1. A hand-written `reset()` exists. Its presence means state outlives the modal;
+ ask why the modal isn't just unmounted.
+2. An always-mounted dialog (rendered unconditionally with an `open` prop) that
+ owns async/in-flight state: popups, timers, subscriptions, AbortControllers,
+ server sessions.
+3. A busy/loading flag composed from a child hook (e.g. `ccBusy || someHook.busy`)
+ where `reset()` clears only part of it.
diff --git a/e2e/cloud/connection-modal-oauth-abandon.test.ts b/e2e/cloud/connection-modal-oauth-abandon.test.ts
new file mode 100644
index 000000000..2b33bb82a
--- /dev/null
+++ b/e2e/cloud/connection-modal-oauth-abandon.test.ts
@@ -0,0 +1,157 @@
+// Cloud (browser): abandoning an OAuth connection must not wedge the
+// add-connection modal. A user picks a registered OAuth app, clicks "Connect
+// with OAuth" (the modal opens the provider popup and flips to a busy
+// "Connecting…" state), then bails by closing the popup without granting consent.
+// The popup-closed signal is intentionally not polled (providers' COOP headers
+// make `popup.closed` unreliable), so the modal can't detect the abandonment on
+// its own. The guarantee under test: closing the modal afterwards RESETS it, so
+// reopening offers a fresh attempt instead of staying stuck on "Connecting…".
+//
+// Repro for the user report: "I bailed on finishing the OAuth connection … the
+// Executor app can't detect [it]. But closing the modal should reset the state
+// so I can try again."
+import { randomBytes } from "node:crypto";
+
+import { expect } from "@effect/vitest";
+import { Effect } from "effect";
+import { composePluginApi } from "@executor-js/api/server";
+import { openApiHttpPlugin } from "@executor-js/plugin-openapi/api";
+import { IntegrationSlug, OAuthClientSlug } from "@executor-js/sdk/shared";
+import { serveOAuthTestServer } from "@executor-js/sdk/testing";
+
+import { scenario } from "../src/scenario";
+import { Api, Browser, Target } from "../src/services";
+
+const api = composePluginApi([openApiHttpPlugin()] as const);
+
+const unique = (prefix: string) => `${prefix}_${randomBytes(4).toString("hex")}`;
+
+scenario(
+ "Connections · closing the add-connection modal after abandoning OAuth lets you try again (not stuck on Connecting)",
+ { timeout: 120_000 },
+ Effect.scoped(
+ Effect.gen(function* () {
+ const target = yield* Target;
+ const browser = yield* Browser;
+ const { client: makeClient } = yield* Api;
+ // A real authorization server on 127.0.0.1: the modal's popup navigates to
+ // its authorize page, which we abandon by closing the window.
+ const oauth = yield* serveOAuthTestServer();
+ const identity = yield* target.newIdentity();
+ const client = yield* makeClient(api, identity);
+
+ // An integration that declares an OAuth auth method (no DCR: it carries
+ // explicit endpoints, no registration/discovery URL), so the modal shows
+ // the bring-your-own app picker.
+ const integration = IntegrationSlug.make(unique("oauthint"));
+ yield* client.openapi.addSpec({
+ payload: {
+ spec: {
+ kind: "blob",
+ value: JSON.stringify({
+ openapi: "3.0.3",
+ info: { title: "OAuth-protected API", version: "1.0.0" },
+ paths: {
+ "/me": {
+ get: {
+ operationId: "getMe",
+ tags: ["default"],
+ responses: { "200": { description: "the caller" } },
+ },
+ },
+ },
+ }),
+ },
+ slug: integration,
+ baseUrl: "http://127.0.0.1:59999",
+ authenticationTemplate: [
+ {
+ slug: "oauth",
+ kind: "oauth2",
+ authorizationUrl: oauth.authorizationEndpoint,
+ tokenUrl: oauth.tokenEndpoint,
+ scopes: ["read"],
+ },
+ ],
+ },
+ });
+
+ // A registered OAuth app whose endpoints match the integration's, so the
+ // picker auto-selects it and the footer offers "Connect with OAuth".
+ const clientSlug = OAuthClientSlug.make(unique("oauthc"));
+ yield* client.oauth.createClient({
+ payload: {
+ owner: "org",
+ slug: clientSlug,
+ authorizationUrl: oauth.authorizationEndpoint,
+ tokenUrl: oauth.tokenEndpoint,
+ grant: "authorization_code",
+ clientId: "test-client",
+ clientSecret: "test-secret",
+ },
+ });
+
+ yield* browser.session(identity, async ({ page, step }) => {
+ const dialog = page.getByRole("dialog");
+ const addConnection = page.getByRole("button", { name: "Add connection", exact: true });
+ const connectWithOAuth = dialog.getByRole("button", { name: "Connect with OAuth" });
+ const connecting = dialog.getByRole("button", { name: "Connecting…" });
+
+ await step("Open the integration and start a new connection", async () => {
+ await page.goto(`/integrations/${integration}`, { waitUntil: "networkidle" });
+ await addConnection.click();
+ // The registered app is auto-selected, so the OAuth connect button is
+ // present and enabled.
+ await connectWithOAuth.waitFor({ state: "visible", timeout: 15_000 });
+ expect(
+ await connectWithOAuth.isDisabled(),
+ "the auto-selected app makes Connect with OAuth actionable",
+ ).toBe(false);
+ });
+
+ await step("Begin OAuth, then bail by closing the provider popup", async () => {
+ const [popup] = await Promise.all([page.waitForEvent("popup"), connectWithOAuth.click()]);
+ // The footer flips to the busy "Connecting…" state while the popup is
+ // open; the flow is genuinely in flight.
+ await connecting.waitFor({ state: "visible", timeout: 15_000 });
+ // Let the popup actually reach the authorize page so the OAuth session
+ // is live, then abandon it: the user closes the window without
+ // granting consent.
+ await popup.waitForURL((url) => !url.href.startsWith("about:"), { timeout: 15_000 });
+ await popup.close();
+ });
+
+ await step("Close the modal", async () => {
+ // The Close button is disabled while busy, so the user backs out with
+ // Escape, exactly the "bail" path from the report.
+ await page.keyboard.press("Escape");
+ await dialog.waitFor({ state: "hidden", timeout: 15_000 });
+ });
+
+ await step(
+ "Reopen the modal: it offers a fresh attempt, not a stuck Connecting",
+ async () => {
+ await addConnection.click();
+ await dialog.waitFor({ state: "visible", timeout: 15_000 });
+ await page.waitForLoadState("networkidle");
+
+ // The guarantee: the reopened modal is reset. Before the fix it stays
+ // wedged on "Connecting…" (the abandoned flow's busy state survived the
+ // close), so this count is 1 and the test fails, reproducing the bug.
+ expect(
+ await connecting.count(),
+ "the reopened modal must not be stuck in the Connecting state",
+ ).toBe(0);
+
+ // And a fresh OAuth attempt is actually offered and actionable again.
+ await connectWithOAuth.waitFor({ state: "visible", timeout: 15_000 });
+ expect(
+ await connectWithOAuth.isDisabled(),
+ "the reopened modal lets the user start OAuth again",
+ ).toBe(false);
+ },
+ );
+ });
+ }),
+ ),
+);
diff --git a/packages/react/src/components/add-account-modal.tsx b/packages/react/src/components/add-account-modal.tsx
index f7927a230..bb314fd6b 100644
--- a/packages/react/src/components/add-account-modal.tsx
+++ b/packages/react/src/components/add-account-modal.tsx
@@ -588,7 +588,7 @@ function OAuthAppRadioRow(props: {
);
}
-export function AddAccountModal(props: {
+interface AddAccountModalProps {
readonly integration: IntegrationSlug;
readonly integrationName: string;
readonly methods: readonly AuthMethod[];
@@ -601,7 +601,20 @@ export function AddAccountModal(props: {
* plugin whose auth is fixed (MCP) omits this, hiding the row. */
readonly createCustomMethod?: CreateCustomMethod;
readonly removeCustomMethod?: (method: AuthMethod) => Promise;
-}) {
+}
+
+/** The add-connection modal is self-contained: every transient bit of state
+ * (form fields, the in-flight OAuth popup flow) lives in `AddAccountModalView`,
+ * so closing the modal genuinely unmounts that view and React destroys all of
+ * it, never hand-reset. Unmounting also runs `useOAuthPopupFlow`'s cleanup,
+ * which cancels a dangling server OAuth session. That is why abandoning an
+ * OAuth popup can't wedge a later open: the stuck flow died with its instance.
+ * The parent owns only open/route intent (deep links, the reconnect handoff). */
+export function AddAccountModal(props: AddAccountModalProps) {
+ return props.open ? : null;
+}
+
+function AddAccountModalView(props: AddAccountModalProps) {
const {
integration,
integrationName,
@@ -855,27 +868,6 @@ export function AddAccountModal(props: {
const showSavedToPicker = !oauthRegistering && savedToOptions.length > 1;
const callableName = connectionNameFrom(label, savedToOwner, integrationName, organizationId);
- const reset = () => {
- setMethodId(methods[0]?.id ?? "");
- setValues({});
- setCredentialOrigin("paste");
- setOnePasswordItemId("");
- setLabel("");
- setOwner(defaultOwner);
- setSubmitting(false);
- setPickedApp(null);
- setRegisteringOAuthClient(false);
- setCcBusy(false);
- setDcrBusy(false);
- setDcrFailed(false);
- setShowOtherApps(false);
- setEditingClient(null);
- setRemovingClient(null);
- setCreatedMethods([]);
- setRemovedMethodIds(new Set());
- setAddingMethod(false);
- };
-
// Build the picker row's Edit/Remove menu for an app, but only once its full
// summary has loaded (the picker option lacks endpoints/resource). Until then
// the row shows no actions menu rather than a broken one.
@@ -953,10 +945,10 @@ export function AddAccountModal(props: {
}
};
- const close = () => {
- onOpenChange(false);
- reset();
- };
+ // Just ask the parent to close. Reopening remounts this whole component (see
+ // AddAccountModal), so there is nothing to hand-reset: the form fields and the
+ // OAuth popup flow's busy state die with this instance.
+ const close = () => onOpenChange(false);
const credentialPayloadOrigin = createCredentialPayloadOrigin({
origin: credentialOrigin,
@@ -1169,13 +1161,7 @@ export function AddAccountModal(props: {
};
return (
-