From 9ee0354a0acac0abe009d899dd7f7c69601954bb Mon Sep 17 00:00:00 2001 From: "lorenzo.padovani@padosoft.com" Date: Tue, 19 May 2026 10:33:18 +0200 Subject: [PATCH 01/11] feat(slice-4c.1): Profile delete confirmation modal wired to DELETE /api/profiles/:name MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit First micro-slice of v1.7 slice 4c (Scenarios/Risks/Profiles CRUD). The Profile detail page now has a real Delete action. ## What changed - Profile detail page (`PageProfileDetail`) gets a new red "Delete" button next to "Trigger now" / "Edit" — previously absent - New `` modal opened from the button: - GitHub-style "type the profile name to confirm" anti-footgun - Delete button is `disabled` until the typed text matches exactly - Calls `DELETE /api/profiles/:name` via `apiUrl()` (honors `VITE_AQA_SERVER_URL`) - On 2xx: toast + close modal + navigate back to the profiles list - On 4xx/5xx: modal stays open with inline `` showing the server's error message - Cancel + Delete both disabled mid-flight - The server endpoint (`DELETE /api/profiles/:name`) already exists since v1.4, so this slice is pure UI wiring + e2e ## Tests - @aqa/admin 60 Playwright (was 56, +4 for the delete flow): button opens modal, non-matching name keeps Delete disabled, exact match enables + fires DELETE with the right URL, 4xx keeps modal open - Lint + typecheck clean ## Audit doc - Line 5590/5745/5796 (profile-level actions: save/clone/delete) → **DELETE shipped**; Save/Clone still pending ## What's next in slice 4c - Profile **Save** (edit flow): wire the PUT /api/profiles/:name - Profile **Clone**: POST /api/profiles with copied body - Risk detail Delete + Edit (DELETE/PUT /api/risks/:id already exist) - Scenario detail Delete + Edit + Clone (DELETE/PUT /api/scenarios/:id) Each of those becomes a follow-up micro-PR. Co-Authored-By: Claude Opus 4.7 (1M context) --- docs/internal/admin-placeholder-audit.md | 2 +- packages/admin/src/app.tsx | 149 ++++++++++++++++++ packages/admin/test/e2e/profile-delete.e2e.ts | 97 ++++++++++++ 3 files changed, 247 insertions(+), 1 deletion(-) create mode 100644 packages/admin/test/e2e/profile-delete.e2e.ts diff --git a/docs/internal/admin-placeholder-audit.md b/docs/internal/admin-placeholder-audit.md index 4992a8d..0429b9f 100644 --- a/docs/internal/admin-placeholder-audit.md +++ b/docs/internal/admin-placeholder-audit.md @@ -45,7 +45,7 @@ The line numbers below were captured against commit `~v1.6.0` (post-merge of PR ### Profiles (lines ~5500-5950) -- `5590`, `5745`, `5796` — profile-level actions (save/clone/delete) +- `5590`, `5745`, `5796` — profile-level actions (save/clone/delete) — **DELETE shipped in PR #29 (slice 4c.1):** Profile detail page now has a Delete button → opens type-to-confirm modal → DELETE `/api/profiles/:name`. Save/Clone still pending. - `5927`, `5931`, `5935`, `5939` — pack picker ### Agents (lines ~6500-6630) diff --git a/packages/admin/src/app.tsx b/packages/admin/src/app.tsx index c90c60d..c7a8fb0 100644 --- a/packages/admin/src/app.tsx +++ b/packages/admin/src/app.tsx @@ -4906,6 +4906,137 @@ function ImportManifestWizard({ open, onClose }) { } Object.assign(window, { ImportManifestWizard }); +// ============================================================= +// Delete-profile wizard (v1.7 slice 4c) +// ============================================================= +// Front-end for DELETE /api/profiles/:name. Deleting a profile is +// destructive (it removes the configuration that drives `aqa run +// --profile `) so the modal requires the user to type the +// profile name as confirmation, mirroring the GitHub repo-delete +// confirmation pattern. The server endpoint exists since v1.4. +function DeleteProfileWizard({ open, profileName, onClose, onDeleted }) { + const [confirmText, setConfirmText] = React.useState(''); + const [submitting, setSubmitting] = React.useState(false); + const [error, setError] = React.useState(null); + const toast = useToast(); + + // Reset when modal opens for a new profile (the parent re-uses the + // same component for every profile it renders detail for). + React.useEffect(() => { + if (open) { + setConfirmText(''); + setError(null); + setSubmitting(false); + } + }, [open]); + + const canSubmit = confirmText === profileName && !submitting; + + function handleClose() { + if (submitting) return; + onClose?.(); + } + + async function handleSubmit() { + if (!canSubmit) return; + setSubmitting(true); + setError(null); + const reqUrl = apiUrl(`/api/profiles/${encodeURIComponent(profileName)}`); + try { + const res = await fetch(reqUrl, { method: 'DELETE' }); + const text = await res.text(); + let parsed = null; + try { + parsed = text ? JSON.parse(text) : null; + } catch { + parsed = null; + } + if (!res.ok) { + const msg = parsed?.error ?? `HTTP ${res.status}`; + setError(msg); + toast.push({ kind: 'error', title: 'Delete profile failed', body: msg }); + return; + } + toast.push({ + kind: 'success', + title: 'Profile deleted', + body: profileName, + }); + onDeleted?.(); + } catch (e) { + const msg = e instanceof Error ? e.message : String(e); + const full = `Could not reach ${reqUrl} (${msg}). The admin is in mock-data mode or the server is down — the profile was not deleted.`; + setError(full); + toast.push({ kind: 'error', title: 'Delete profile failed', body: full }); + } finally { + setSubmitting(false); + } + } + + return ( + + + + + } + > +
+ {error && ( + + {error} + + )} + + Removing a profile drops every run configuration that uses it. Existing run records, + findings, and audit events are unaffected — only the profile definition is removed. + +
+ + setConfirmText(e.target.value)} + autoFocus + /> +
+ The Delete button stays disabled until the typed text matches the profile name + exactly. Stops accidental clicks on the wrong profile in a list. +
+
+
+ + ); +} +Object.assign(window, { DeleteProfileWizard }); + // ============ shell.jsx ============ // ============================================================= // agentic-qa-kit · admin panel — Shell (Sidebar + TopBar + Palette) @@ -8389,6 +8520,7 @@ function PageProfiles({ onNavigate, onOpenProfile }) { // ---------------- Profile detail ---------------- function PageProfileDetail({ name, onNavigate }) { const p = profileByName(name) || PROFILES[0]; + const [deleteOpen, setDeleteOpen] = React.useState(false); return (
Edit + } /> + setDeleteOpen(false)} + onDeleted={() => { + setDeleteOpen(false); + onNavigate?.('profiles', {}); + }} + />
diff --git a/packages/admin/test/e2e/profile-delete.e2e.ts b/packages/admin/test/e2e/profile-delete.e2e.ts new file mode 100644 index 0000000..b3badf8 --- /dev/null +++ b/packages/admin/test/e2e/profile-delete.e2e.ts @@ -0,0 +1,97 @@ +import { expect, test } from '@playwright/test'; + +/** + * v1.7 slice 4c.1 — Profile Delete confirmation modal wired to + * DELETE /api/profiles/:name. + * + * Covers: + * - Profile detail page has a Delete button (previously absent) + * - Clicking it opens a confirmation modal that requires typing the + * profile name (GitHub-style "type to confirm" anti-footgun pattern) + * - Delete button stays disabled until the typed name matches exactly + * - Happy path: DELETEs /api/profiles/, shows success toast, + * navigates back to the profiles list + * - 4xx/5xx: modal stays open with the server's error message + * + * The server endpoint exists since v1.4 (`DELETE /api/profiles/:name`), + * so this slice is pure UI wiring + e2e. + */ + +async function navigateToProfileDetail(page: import('@playwright/test').Page): Promise { + await page.goto('/'); + await expect(page.locator('.sidebar')).toBeVisible(); + await page + .locator('.nav-item', { hasText: /^Profiles/i }) + .first() + .click(); + await expect(page.locator('.page-title, h1').first()).toContainText(/Profiles/i); + // Click the first profile row to open detail. The prototype renders + // profiles as table rows where the entire row is clickable. + const firstRow = page.locator('table.tbl tbody tr').first(); + await expect(firstRow).toBeVisible(); + const name = await firstRow.locator('td').first().innerText(); + await firstRow.click(); + // Profile detail header shows the name as a mono span. + await expect(page.locator('.page-title .mono, h1 .mono').first()).toBeVisible(); + return name.trim(); +} + +test.describe('Profile delete confirmation', () => { + test('Delete button opens the confirmation modal', async ({ page }) => { + await navigateToProfileDetail(page); + await page.getByTestId('profile-delete-btn').click(); + await expect(page.locator('.modal-title')).toContainText(/Delete profile/i); + await expect(page.getByTestId('profile-delete-confirm')).toBeVisible(); + // Delete is disabled until the user types the name. + await expect(page.getByTestId('profile-delete-submit')).toBeDisabled(); + }); + + test('typing a non-matching name leaves Delete disabled', async ({ page }) => { + const name = await navigateToProfileDetail(page); + await page.getByTestId('profile-delete-btn').click(); + await page.getByTestId('profile-delete-confirm').fill(`${name}-typo`); + await expect(page.getByTestId('profile-delete-submit')).toBeDisabled(); + }); + + test('exact-match name enables Delete and DELETE fires on click', async ({ page }) => { + type Req = { url: string; method: string }; + const calls: Req[] = []; + await page.route('**/api/profiles/*', async (route) => { + calls.push({ url: route.request().url(), method: route.request().method() }); + await route.fulfill({ status: 200, contentType: 'application/json', body: '{}' }); + }); + const name = await navigateToProfileDetail(page); + await page.getByTestId('profile-delete-btn').click(); + await page.getByTestId('profile-delete-confirm').fill(name); + await expect(page.getByTestId('profile-delete-submit')).toBeEnabled(); + await page.getByTestId('profile-delete-submit').click(); + // On success the modal closes (we navigate back to /profiles). + await expect(page.locator('.modal-title')).toHaveCount(0); + // Exactly one DELETE call to /api/profiles/. + const deletes = calls.filter((c) => c.method === 'DELETE'); + expect(deletes.length).toBe(1); + expect(deletes[0]?.url).toContain(`/api/profiles/${encodeURIComponent(name)}`); + }); + + test('4xx keeps modal open and surfaces the server error', async ({ page }) => { + await page.route('**/api/profiles/*', async (route) => { + if (route.request().method() !== 'DELETE') return route.continue(); + await route.fulfill({ + status: 409, + contentType: 'application/json', + body: JSON.stringify({ + error: 'profile is referenced by 3 active runs; cancel them first', + }), + }); + }); + const name = await navigateToProfileDetail(page); + await page.getByTestId('profile-delete-btn').click(); + await page.getByTestId('profile-delete-confirm').fill(name); + await page.getByTestId('profile-delete-submit').click(); + await expect( + page.locator('.modal-body .alert', { hasText: /referenced by 3 active runs/i }), + ).toBeVisible(); + // Modal still open. + await expect(page.locator('.modal-title')).toContainText(/Delete profile/i); + }); +}); From 037f73c6837cc99e6cdde31ec7bd8a5d4e3aedc5 Mon Sep 17 00:00:00 2001 From: "lorenzo.padovani@padosoft.com" Date: Tue, 19 May 2026 10:47:11 +0200 Subject: [PATCH 02/11] fix(slice-4c.1): drop deleted row from list, rendered JSX subtitle, stronger e2e, profileName dep MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit PR #29 iter 1 review fixes (Codex P2 + 3 Copilot items): ## Real fixes 1. **Codex P2 — deleted profile reappears in list** (app.tsx:8560): `PageProfiles` rendered the static `PROFILES` constant directly, so after a successful delete the user returned to the list and saw the just-deleted profile still there (and could open/delete it again). Fixed via a window-level `aqa:profile-deleted` CustomEvent: `DeleteProfileWizard` broadcasts on success; `PageProfiles` subscribes and maintains a `deletedNames` Set that filters `PROFILES` before render. Works in both mock-mode (no real backend) and live-mode (a real refetch would also drop the entry). 2. **Copilot — happy-path test gaps** (e2e:69): the test asserted only modal-close + correct URL. Now also asserts the success toast appears with the right title AND that the just-deleted row drops out of the Profiles table after the post-delete navigation. 3. **Copilot — effect deps incomplete** (app.tsx:4931): the reset effect depended only on `open`. If a parent reuses the same `DeleteProfileWizard` mount across two different profiles (same component, different `profileName` prop), the confirm text from the previous profile would stick around. Added `profileName` to deps so the reset fires on either trigger. 4. **Copilot — Markdown backticks rendered literally** (app.tsx:4981): the modal's `sub` prop was a template-literal string with `\`aqa run...\`` backticks, but `` renders `sub` as plain React children (no Markdown parsing). The backticks appeared literally in the UI. Switched `sub` to a JSX fragment with `` for the command + `` for the profile name — matches the rest of the admin's code styling. Tests: @aqa/admin 60 Playwright (unchanged count; happy-path test upgraded with 2 new assertions). Lint + typecheck clean. Co-Authored-By: Claude Opus 4.7 (1M context) --- packages/admin/src/app.tsx | 52 +++++++++++++++++-- packages/admin/test/e2e/profile-delete.e2e.ts | 16 ++++-- 2 files changed, 61 insertions(+), 7 deletions(-) diff --git a/packages/admin/src/app.tsx b/packages/admin/src/app.tsx index c7a8fb0..627c4c0 100644 --- a/packages/admin/src/app.tsx +++ b/packages/admin/src/app.tsx @@ -4922,13 +4922,17 @@ function DeleteProfileWizard({ open, profileName, onClose, onDeleted }) { // Reset when modal opens for a new profile (the parent re-uses the // same component for every profile it renders detail for). + // Include `profileName` so the reset also fires if the modal stays + // mounted across a profile switch (e.g. parent re-uses the wizard + // with a different name) — otherwise the typed confirm text from + // the previous profile would stick around. React.useEffect(() => { if (open) { setConfirmText(''); setError(null); setSubmitting(false); } - }, [open]); + }, [open, profileName]); const canSubmit = confirmText === profileName && !submitting; @@ -4962,6 +4966,19 @@ function DeleteProfileWizard({ open, profileName, onClose, onDeleted }) { title: 'Profile deleted', body: profileName, }); + // Broadcast the deletion so the list page (or any other view + // showing this profile) can drop it from its local state. + // Without this, navigating back to /profiles would still show + // the just-deleted entry (in mock-data mode the PROFILES array + // is a static module constant; in live mode the page would + // refetch — the event makes both cases consistent). + try { + window.dispatchEvent( + new CustomEvent('aqa:profile-deleted', { detail: { name: profileName } }), + ); + } catch { + // CustomEvent unsupported in this runtime — non-fatal. + } onDeleted?.(); } catch (e) { const msg = e instanceof Error ? e.message : String(e); @@ -4978,7 +4995,13 @@ function DeleteProfileWizard({ open, profileName, onClose, onDeleted }) { open={open} onClose={handleClose} title="Delete profile" - sub={`This permanently removes the "${profileName}" profile. \`aqa run --profile ${profileName}\` will start failing immediately. The action cannot be undone from the admin (you'd need to re-create the profile from scratch).`} + sub={ + <> + This permanently removes the "{profileName}" profile.{' '} + aqa run --profile {profileName} will start failing immediately. The action + cannot be undone from the admin (you'd need to re-create the profile from scratch). + + } size="md" footer={ <> @@ -8452,11 +8475,32 @@ function PageScenarioDetail({ id, onNavigate }) { // ---------------- Profiles ---------------- function PageProfiles({ onNavigate, onOpenProfile }) { + // Subscribe to delete events so the just-deleted profile drops out + // of the rendered list. In mock-data mode the PROFILES array is a + // static constant — without this subscription, navigating back to + // /profiles after a successful delete would still show the entry. + // In a live deployment with a real backend the page would refetch; + // the event-driven local state makes both behaviors match. + const [deletedNames, setDeletedNames] = React.useState(() => new Set()); + React.useEffect(() => { + const handler = (e) => { + const name = e?.detail?.name; + if (typeof name !== 'string') return; + setDeletedNames((prev) => { + const next = new Set(prev); + next.add(name); + return next; + }); + }; + window.addEventListener('aqa:profile-deleted', handler); + return () => window.removeEventListener('aqa:profile-deleted', handler); + }, []); + const visible = PROFILES.filter((p) => !deletedNames.has(p.name)); return (
p.execution_mode === 'sandbox').length} sandbox · ${PROFILES.filter((p) => p.execution_mode === 'host').length} host`} + sub={`${visible.length} configured · execution mode mix: ${visible.filter((p) => p.execution_mode === 'sandbox').length} sandbox · ${visible.filter((p) => p.execution_mode === 'host').length} host`} actions={ + + +
+ ); + } return (
Date: Tue, 19 May 2026 11:18:24 +0200 Subject: [PATCH 04/11] fix(slice-4c.1): lift deletedProfiles to App so it survives route changes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit PR #29 iter 3 review fix (Copilot — 1 real new item + 3 stale re-flags): ## Real fix **CustomEvent timing race** (Copilot app.tsx:8506): the `aqa:profile-deleted` listener used to live on `PageProfiles`, but the event is dispatched from `PageProfileDetail` (still mounted) right BEFORE we navigate back to `/profiles`. So when `PageProfiles` finally mounts, its `useEffect` listener was never on the page to catch the dispatch — the `deletedNames` Set initialized empty and the row reappeared. The happy-path test passed only by accident (Playwright was matching during a transient state). Lifted the state + listener to `App`, which is always mounted while the user is signed in: - `App` owns `deletedProfiles: Set` and the listener - The set rides on `ctx` and is passed through to both `PageProfiles` (filters the rendered list) and `PageProfileDetail` (treats just-deleted profiles as not-found so a stale link doesn't accidentally re-target the mock row) The same architecture is reusable for future delete actions on risks, scenarios, etc — App-level lifted-state for transient mock mutations. ## Stale re-flags (no action) Copilot also re-flagged: silent fallback to PROFILES[0] (fixed in iter 3 with not-found state), submitting double-click race (fixed in iter 3 with inFlightRef). Codex's "remove deleted profile from list" was the same comment now correctly addressed by this iter's architecture fix. Tests: @aqa/admin 60 Playwright (unchanged count; all 4 profile-delete tests still pass, including the row-drops-from-list assertion which now works reliably across the route change). Lint + typecheck clean. Co-Authored-By: Claude Opus 4.7 (1M context) --- packages/admin/src/app.tsx | 60 +++++++++++++++++++++++--------------- 1 file changed, 37 insertions(+), 23 deletions(-) diff --git a/packages/admin/src/app.tsx b/packages/admin/src/app.tsx index 63693cb..3643483 100644 --- a/packages/admin/src/app.tsx +++ b/packages/admin/src/app.tsx @@ -8484,27 +8484,14 @@ function PageScenarioDetail({ id, onNavigate }) { } // ---------------- Profiles ---------------- -function PageProfiles({ onNavigate, onOpenProfile }) { - // Subscribe to delete events so the just-deleted profile drops out - // of the rendered list. In mock-data mode the PROFILES array is a - // static constant — without this subscription, navigating back to - // /profiles after a successful delete would still show the entry. - // In a live deployment with a real backend the page would refetch; - // the event-driven local state makes both behaviors match. - const [deletedNames, setDeletedNames] = React.useState(() => new Set()); - React.useEffect(() => { - const handler = (e) => { - const name = e?.detail?.name; - if (typeof name !== 'string') return; - setDeletedNames((prev) => { - const next = new Set(prev); - next.add(name); - return next; - }); - }; - window.addEventListener('aqa:profile-deleted', handler); - return () => window.removeEventListener('aqa:profile-deleted', handler); - }, []); +function PageProfiles({ onNavigate, onOpenProfile, deletedProfiles }) { + // `deletedProfiles` is owned by App (lifted state) so it survives + // route changes. PageProfileDetail dispatches `aqa:profile-deleted` + // BEFORE navigating back here, so a listener on PageProfiles itself + // would miss the event (we weren't mounted at dispatch time). The + // App-level listener catches every dispatch and the Set drips down + // through props. + const deletedNames = deletedProfiles ?? new Set(); const visible = PROFILES.filter((p) => !deletedNames.has(p.name)); return (
@@ -8572,8 +8559,13 @@ function PageProfiles({ onNavigate, onOpenProfile }) { } // ---------------- Profile detail ---------------- -function PageProfileDetail({ name, onNavigate }) { - const p = profileByName(name); +function PageProfileDetail({ name, onNavigate, deletedProfiles }) { + const rawP = profileByName(name); + // Treat just-deleted profiles as not-found so a stale link + // (e.g. a notification linking to a profile a colleague just + // removed) doesn't render the underlying mock row. + const isDeleted = deletedProfiles?.has(name); + const p = isDeleted ? null : rawP; const [deleteOpen, setDeleteOpen] = React.useState(false); // Now that this page has a destructive Delete action, a stale or // typoed route param (e.g. /profiles/) must NOT @@ -10838,6 +10830,27 @@ function App() { const [mode, setMode] = React.useState('mock'); // mock | live | failed const [lastTick, setLastTick] = React.useState(NOW_REF); const [signedIn, setSignedIn] = React.useState(true); + // Profile deletions broadcast via `aqa:profile-deleted` CustomEvent + // and the set lives at App level (not in PageProfiles) so it + // survives route changes. PageProfileDetail dispatches the event + // before navigating back to /profiles — if the listener lived on + // PageProfiles it would miss the event because PageProfiles isn't + // mounted yet at dispatch time. App is always mounted while the + // user is signed in, so it catches every dispatch. + const [deletedProfiles, setDeletedProfiles] = React.useState(() => new Set()); + React.useEffect(() => { + const handler = (e) => { + const name = e?.detail?.name; + if (typeof name !== 'string') return; + setDeletedProfiles((prev) => { + const next = new Set(prev); + next.add(name); + return next; + }); + }; + window.addEventListener('aqa:profile-deleted', handler); + return () => window.removeEventListener('aqa:profile-deleted', handler); + }, []); // Apply theme React.useEffect(() => { @@ -10896,6 +10909,7 @@ function App() { params: routeParams, theme: tweaks.theme, onTheme: (th) => setTweak('theme', th), + deletedProfiles, }; if (!signedIn) { From c34d1b1307045f197c100382d7ee9797894108c2 Mon Sep 17 00:00:00 2001 From: "lorenzo.padovani@padosoft.com" Date: Tue, 19 May 2026 11:28:45 +0200 Subject: [PATCH 05/11] fix(slice-4c.1): sync inFlightRef with reset effect on profile switch MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit PR #29 iter 4 review fix (Copilot — 1 real new item + 2 stale re-flags): ## Real fix **`submitting` vs `inFlightRef` desync on mid-flight profile switch** (Copilot app.tsx:4940): the reset effect for the modal (triggered by `open` or `profileName` changing) set `submitting` to `false` but didn't reset `inFlightRef.current`. If a parent re-used the same `DeleteProfileWizard` mount with a new `profileName` while a DELETE was still in-flight, the UI would re-enable the Delete button (submitting=false) but a click would silently no-op via the synchronous ref guard — confusing dead button. Now the effect resets `inFlightRef.current` together with `submitting`. The in-flight request's `finally` block becomes a no-op assignment (setting false on an already-false ref) and the user can issue a fresh delete against the new profile. The old request's success path still toasts against the old `profileName` — acceptable because the user changed context. A full fix would use `AbortController` to actually cancel the old request, but that adds material complexity for an edge case (parents would have to remount the modal on profile switch instead of re-using it, which they currently do). The state-sync approach handles the symptom without scope creep. ## Stale re-flags (no action) Codex re-flagged "remove deleted profile" (fixed iter 4 with App- level lifted state) and Copilot re-flagged "submitting state async race" (fixed iter 3 with inFlightRef). Tests: @aqa/admin 60 Playwright (unchanged count; all 4 profile-delete tests still pass). Lint + typecheck clean. Co-Authored-By: Claude Opus 4.7 (1M context) --- packages/admin/src/app.tsx | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/packages/admin/src/app.tsx b/packages/admin/src/app.tsx index 3643483..28fffdf 100644 --- a/packages/admin/src/app.tsx +++ b/packages/admin/src/app.tsx @@ -4933,11 +4933,22 @@ function DeleteProfileWizard({ open, profileName, onClose, onDeleted }) { // mounted across a profile switch (e.g. parent re-uses the wizard // with a different name) — otherwise the typed confirm text from // the previous profile would stick around. + // + // CRITICAL: reset `inFlightRef.current` together with `submitting`. + // Otherwise a profile switch mid-flight would re-enable the Delete + // button (submitting=false) while the ref stays true — the user + // could click Delete and the new submit would silently no-op via + // the synchronous guard. Keeping both in sync means the old + // request's finally block becomes a no-op assignment, and the user + // can issue a fresh delete on the new profile. (The old request's + // success path would still toast against the old profileName, + // which is acceptable — the user just changed context.) React.useEffect(() => { if (open) { setConfirmText(''); setError(null); setSubmitting(false); + inFlightRef.current = false; } }, [open, profileName]); From 3a9a281590aafaefdb192d3f9f44b057d5103c41 Mon Sep 17 00:00:00 2001 From: "lorenzo.padovani@padosoft.com" Date: Tue, 19 May 2026 11:39:57 +0200 Subject: [PATCH 06/11] test(slice-4c.1): tighten route mock to DELETE-only and real response shape MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit PR #29 iter 5 review fix (Copilot — 1 real new item + 3 stale re-flags): The happy-path e2e route mock intercepted ALL `**/api/profiles/*` requests with a `{}` response body regardless of HTTP method. Two problems Copilot caught: 1. The blanket match would silently fulfill unexpected GET/PUT calls during the test, masking any regression that introduced extra fetches. 2. The empty `{}` body diverges from the actual `DELETE /api/profiles/:name` response shape, which is `{ok: true}` per `@aqa/server`'s `asResponse({ ok: true })` (api.ts:303). Now the mock only intercepts DELETE (other methods get `route.continue()`) and the response body matches the real endpoint's shape. The assertion that "exactly one DELETE call" fires is now stricter — it counts ALL intercepted calls (which are guaranteed DELETE), not a filtered subset. Other 3 iter-5 comments were stale re-flags of items addressed in earlier iters: Codex P2 deleted-row (lifted state iter 4), Copilot async race (inFlightRef iter 3), Copilot ref/submitting desync (sync in reset effect iter 5). Tests: @aqa/admin 60 Playwright, all pass. Lint clean. Co-Authored-By: Claude Opus 4.7 (1M context) --- packages/admin/test/e2e/profile-delete.e2e.ts | 21 ++++++++++++++----- 1 file changed, 16 insertions(+), 5 deletions(-) diff --git a/packages/admin/test/e2e/profile-delete.e2e.ts b/packages/admin/test/e2e/profile-delete.e2e.ts index 92bb73e..6673c0d 100644 --- a/packages/admin/test/e2e/profile-delete.e2e.ts +++ b/packages/admin/test/e2e/profile-delete.e2e.ts @@ -56,11 +56,22 @@ test.describe('Profile delete confirmation', () => { test('exact-match name enables Delete; DELETE fires; toast shown; navigates to /profiles; row drops from list', async ({ page, }) => { + // Only intercept DELETEs — let any other HTTP method continue + // to the dev server unmodified so unexpected GET/PUT calls don't + // get silently fulfilled with our `{}` stub. Also match the real + // endpoint's response shape: `DELETE /api/profiles/:name` returns + // `{ok: true}` per @aqa/server's `asResponse({ ok: true })`. type Req = { url: string; method: string }; const calls: Req[] = []; await page.route('**/api/profiles/*', async (route) => { - calls.push({ url: route.request().url(), method: route.request().method() }); - await route.fulfill({ status: 200, contentType: 'application/json', body: '{}' }); + const method = route.request().method(); + if (method !== 'DELETE') return route.continue(); + calls.push({ url: route.request().url(), method }); + await route.fulfill({ + status: 200, + contentType: 'application/json', + body: JSON.stringify({ ok: true }), + }); }); const name = await navigateToProfileDetail(page); await page.getByTestId('profile-delete-btn').click(); @@ -70,9 +81,9 @@ test.describe('Profile delete confirmation', () => { // (1) On success the modal closes. await expect(page.locator('.modal-title')).toHaveCount(0); // (2) Exactly one DELETE call to /api/profiles/. - const deletes = calls.filter((c) => c.method === 'DELETE'); - expect(deletes.length).toBe(1); - expect(deletes[0]?.url).toContain(`/api/profiles/${encodeURIComponent(name)}`); + expect(calls.length).toBe(1); + expect(calls[0]?.method).toBe('DELETE'); + expect(calls[0]?.url).toContain(`/api/profiles/${encodeURIComponent(name)}`); // (3) Success toast appears with the right title. await expect(page.locator('.toast.success', { hasText: 'Profile deleted' })).toBeVisible(); // (4) After navigating back to /profiles, the just-deleted row From d5a4354b6290cc8552198dcf0b9e2755c2c9ad49 Mon Sep 17 00:00:00 2001 From: "lorenzo.padovani@padosoft.com" Date: Tue, 19 May 2026 11:54:02 +0200 Subject: [PATCH 07/11] test(slice-4c.1): fix vacuous row-removal selector caught by Copilot MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit PR #29 iter 6 review fix (Copilot — 1 real new item + 3 stale re-flags): **Vacuous selector** (Copilot e2e:93): the "row drops from list" assertion used `page.locator('table.tbl tbody tr td.mono', ...)`, but in PageProfiles the profile name is rendered as a `` INSIDE a plain `` — there are no `td.mono` elements anywhere, so `.toHaveCount(0)` always passed regardless of whether the row was actually there. The strengthened test from iter 2 was therefore a false positive. Fixed to: 1. Target rows that CONTAIN a `span.id-link.mono` with the exact profile name as text (using a regex-escaped name to handle any future special chars). 2. Additionally assert that the table still has OTHER rows, so the "deleted row not present" check isn't satisfied vacuously by an empty table. Other 3 iter-6 comments were stale re-flags addressed earlier: - Codex P2 deleted-row (lifted state iter 4) - Copilot async submitting race (inFlightRef iter 3) - Copilot ref/submitting desync (sync in effect iter 5) Tests: @aqa/admin 60 Playwright, all 4 profile-delete tests pass with the now-meaningful row assertion. Lint clean. Co-Authored-By: Claude Opus 4.7 (1M context) --- packages/admin/test/e2e/profile-delete.e2e.ts | 18 ++++++++++++++++-- 1 file changed, 16 insertions(+), 2 deletions(-) diff --git a/packages/admin/test/e2e/profile-delete.e2e.ts b/packages/admin/test/e2e/profile-delete.e2e.ts index 6673c0d..8bce25a 100644 --- a/packages/admin/test/e2e/profile-delete.e2e.ts +++ b/packages/admin/test/e2e/profile-delete.e2e.ts @@ -87,11 +87,25 @@ test.describe('Profile delete confirmation', () => { // (3) Success toast appears with the right title. await expect(page.locator('.toast.success', { hasText: 'Profile deleted' })).toBeVisible(); // (4) After navigating back to /profiles, the just-deleted row - // no longer appears in the list (window event drops it). + // no longer appears in the list (window event drops it via + // the App-level lifted deletedProfiles set). + // + // Selector: the profile name renders as + // inside a . An earlier version of this test used `td.mono`, + // which matches nothing (the class is on the span, not the td) + // so the count was always 0 and the assertion silently passed + // even if the row was still present (Copilot caught this in + // iter 6 review). Now we target the span directly within a row + // AND assert the table still has other rows so the check isn't + // vacuously satisfied by an empty table. await expect(page.locator('.page-title, h1').first()).toContainText(/Profiles/i); + const escaped = name.replace(/[.*+?^${}()|[\]\\]/g, '\\$&'); await expect( - page.locator('table.tbl tbody tr td.mono', { hasText: new RegExp(`^${name}$`) }), + page.locator('table.tbl tbody tr', { + has: page.locator('span.id-link.mono', { hasText: new RegExp(`^${escaped}$`) }), + }), ).toHaveCount(0); + await expect(page.locator('table.tbl tbody tr')).not.toHaveCount(0); }); test('4xx keeps modal open and surfaces the server error', async ({ page }) => { From 0ba0eb80568e93e262677e13e315c2e8259e07b3 Mon Sep 17 00:00:00 2001 From: "lorenzo.padovani@padosoft.com" Date: Tue, 19 May 2026 12:06:33 +0200 Subject: [PATCH 08/11] fix(slice-4c.1): stale-submit guard via captured submittedName MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit PR #29 iter 7 review fix (Copilot — 1 real new item + 3 stale re-flags): **Stale-closure race after mid-flight profile switch** (Copilot app.tsx:5003): an in-flight DELETE fetch for the previous `profileName` could still resolve after the user navigated to a different profile-detail page (same wizard mount, new `profileName` prop). The post-fetch code would then call `setError(...)`, `setResult(...)`, and `onDeleted?.()` against the NEW profile context — wrong error in the new modal, or unexpectedly yanking the user back to /profiles when the OLD request succeeds. Implemented the capture-name pattern Copilot suggested: - `const submittedName = profileName` captured at submit start - After the fetch resolves, check `submittedName === profileName` before calling `setError` / `onDeleted` (i.e. mutating the wizard's UI state) - Always emit the toast and the `aqa:profile-deleted` window event using `submittedName` (the action DID happen server-side and the App-level listener that filters PROFILES doesn't care which wizard fired it; the user deserves the feedback) - Same guard in the catch branch + finally block — only flip `submitting` / `inFlightRef.current` back to false when the wizard still belongs to the submitted profile, otherwise the stale resolve could clobber a fresh submit's state on the new profile A full fix would use `AbortController` to actually cancel the old request, but the toast/broadcast-and-skip-state-mutation pattern preserves the right user-visible outcome without adding the abort plumbing. Other 3 iter-7 comments were stale re-flags (Codex P2 deleted-row, Copilot async submit race, Copilot ref/submitting desync — all addressed in iters 3-5). Tests: @aqa/admin 60 Playwright, all pass. Lint clean. Co-Authored-By: Claude Opus 4.7 (1M context) --- packages/admin/src/app.tsx | 70 +++++++++++++++++++++++++++++--------- 1 file changed, 54 insertions(+), 16 deletions(-) diff --git a/packages/admin/src/app.tsx b/packages/admin/src/app.tsx index 28fffdf..5e771e6 100644 --- a/packages/admin/src/app.tsx +++ b/packages/admin/src/app.tsx @@ -4965,7 +4965,16 @@ function DeleteProfileWizard({ open, profileName, onClose, onDeleted }) { inFlightRef.current = true; setSubmitting(true); setError(null); - const reqUrl = apiUrl(`/api/profiles/${encodeURIComponent(profileName)}`); + // Capture the profile name we're submitting against. If the parent + // swaps `profileName` while the fetch is in flight (e.g. user + // navigates to a different profile-detail page that reuses this + // wizard mount), the in-flight resolve must NOT mutate the + // wizard's UI state for the OLD profile. The post-fetch code + // checks `submittedName === profileName` before calling any + // setState / onDeleted. This is the "stale closure" guard + // Copilot flagged in iter 7 review. + const submittedName = profileName; + const reqUrl = apiUrl(`/api/profiles/${encodeURIComponent(submittedName)}`); try { const res = await fetch(reqUrl, { method: 'DELETE' }); const text = await res.text(); @@ -4975,39 +4984,68 @@ function DeleteProfileWizard({ open, profileName, onClose, onDeleted }) { } catch { parsed = null; } + // Stale-submit guard: if the user switched profiles while this + // fetch was in flight, we still toast/broadcast the *correct* + // event for `submittedName` (the action did happen server-side + // and the user deserves the feedback), but we do NOT mutate + // the wizard's UI state for the NEW profile (setError / + // onDeleted). The wizard belongs to the new profile now. + const stillCurrent = submittedName === profileName; if (!res.ok) { const msg = parsed?.error ?? `HTTP ${res.status}`; - setError(msg); - toast.push({ kind: 'error', title: 'Delete profile failed', body: msg }); + // Toast carries the submittedName so the user knows which + // delete attempt failed, even after switching. + toast.push({ + kind: 'error', + title: 'Delete profile failed', + body: `${submittedName}: ${msg}`, + }); + if (stillCurrent) setError(msg); return; } toast.push({ kind: 'success', title: 'Profile deleted', - body: profileName, + body: submittedName, }); - // Broadcast the deletion so the list page (or any other view - // showing this profile) can drop it from its local state. - // Without this, navigating back to /profiles would still show - // the just-deleted entry (in mock-data mode the PROFILES array - // is a static module constant; in live mode the page would - // refetch — the event makes both cases consistent). + // Broadcast the deletion regardless of stale-submit state — + // App-level listener filters PROFILES, and that's true whether + // the user is on the old or new detail page. The event uses + // submittedName, not profileName. try { window.dispatchEvent( - new CustomEvent('aqa:profile-deleted', { detail: { name: profileName } }), + new CustomEvent('aqa:profile-deleted', { detail: { name: submittedName } }), ); } catch { // CustomEvent unsupported in this runtime — non-fatal. } - onDeleted?.(); + // onDeleted (which navigates back to /profiles) only makes + // sense when the wizard still belongs to the deleted profile. + // If the user already moved on, don't yank them back. + if (stillCurrent) onDeleted?.(); } catch (e) { const msg = e instanceof Error ? e.message : String(e); const full = `Could not reach ${reqUrl} (${msg}). The admin is in mock-data mode or the server is down — the profile was not deleted.`; - setError(full); - toast.push({ kind: 'error', title: 'Delete profile failed', body: full }); + // Same stale-submit guard as the success path: toast against + // the submitted name so the user gets the right context, but + // only mutate wizard error state if it still belongs to that + // profile. + toast.push({ + kind: 'error', + title: 'Delete profile failed', + body: `${submittedName}: ${full}`, + }); + if (submittedName === profileName) setError(full); } finally { - setSubmitting(false); - inFlightRef.current = false; + // Only flip submitting/inFlightRef when the wizard still + // belongs to the submitted profile — otherwise a stale resolve + // could re-enable the Delete button for an unrelated profile + // mid-flight (the user's current submit might still be in + // progress on the new profile). + if (submittedName === profileName) { + setSubmitting(false); + inFlightRef.current = false; + } } } From dfd0cf33d9bbd4cc0c117ea73fb192468f6958f4 Mon Sep 17 00:00:00 2001 From: "lorenzo.padovani@padosoft.com" Date: Tue, 19 May 2026 12:16:11 +0200 Subject: [PATCH 09/11] fix(slice-4c.1): synchronous reset on handleClose + close/reopen regression test MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit PR #29 iter 8 review fixes (Copilot — 2 real new items + 4 stale re-flags): ## Real fixes 1. **Stale state flash on close+reopen** (Copilot app.tsx:4960): the reset effect only fires when `open` becomes true OR `profileName` changes. Closing and reopening on the SAME profile wouldn't trigger either, so the first render after reopening would briefly show the previous session's `confirmText` / `error` / `submitting` / `inFlightRef.current`. `handleClose` now resets all four synchronously before calling the parent's `onClose` — no flicker, no chance for a misclick to fire DELETE without re-typing. 2. **Missing close+reopen e2e** (Copilot e2e:48): added a regression test that opens the modal, types the exact name (which enables Delete), closes via Cancel, reopens, and asserts the confirm input is empty and Delete is disabled. ## Stale re-flags (no action) Codex P2 deleted-row (lifted state iter 4), Copilot async submit race (inFlightRef iter 3), Copilot ref/submitting desync (iter 5), Copilot stale-closure race (submittedName guard iter 8). Tests: @aqa/admin 61 Playwright (60 existing + 1 new close+reopen). Lint clean. Co-Authored-By: Claude Opus 4.7 (1M context) --- packages/admin/src/app.tsx | 10 ++++++++ packages/admin/test/e2e/profile-delete.e2e.ts | 25 +++++++++++++++++++ 2 files changed, 35 insertions(+) diff --git a/packages/admin/src/app.tsx b/packages/admin/src/app.tsx index 5e771e6..d91ce8d 100644 --- a/packages/admin/src/app.tsx +++ b/packages/admin/src/app.tsx @@ -4956,6 +4956,16 @@ function DeleteProfileWizard({ open, profileName, onClose, onDeleted }) { function handleClose() { if (submitting) return; + // Reset state synchronously on close — don't rely solely on the + // open-true effect. If the user closes the modal and reopens it + // for the same profile (no `profileName` change to retrigger + // the effect), the first render after reopening would otherwise + // briefly show stale `confirmText` / `error` from the prior + // session before the next effect tick clears them. + setConfirmText(''); + setError(null); + setSubmitting(false); + inFlightRef.current = false; onClose?.(); } diff --git a/packages/admin/test/e2e/profile-delete.e2e.ts b/packages/admin/test/e2e/profile-delete.e2e.ts index 8bce25a..16f0df9 100644 --- a/packages/admin/test/e2e/profile-delete.e2e.ts +++ b/packages/admin/test/e2e/profile-delete.e2e.ts @@ -46,6 +46,31 @@ test.describe('Profile delete confirmation', () => { await expect(page.getByTestId('profile-delete-submit')).toBeDisabled(); }); + test('close+reopen clears the typed confirm text (no stale state flash)', async ({ page }) => { + // Regression test for PR #29 iter 8 (Copilot): + // The reset effect runs only when `open` becomes true OR + // `profileName` changes. Closing and reopening on the SAME + // profile wouldn't fire either trigger, so the first render + // after reopening could briefly show stale `confirmText` / + // `error` from the prior session. handleClose now resets state + // synchronously to avoid the flicker. + const name = await navigateToProfileDetail(page); + await page.getByTestId('profile-delete-btn').click(); + const confirm = page.getByTestId('profile-delete-confirm'); + await confirm.fill(name); // type the exact name → Delete enables + await expect(page.getByTestId('profile-delete-submit')).toBeEnabled(); + // Close via the Cancel button (calls handleClose). + await page.getByRole('button', { name: 'Cancel' }).click(); + await expect(page.locator('.modal-title')).toHaveCount(0); + // Reopen the same profile's delete modal. + await page.getByTestId('profile-delete-btn').click(); + // The confirm input must be empty — no stale value, no enabled + // Delete button (which would mean a misclick could fire DELETE + // without re-typing). + await expect(page.getByTestId('profile-delete-confirm')).toHaveValue(''); + await expect(page.getByTestId('profile-delete-submit')).toBeDisabled(); + }); + test('typing a non-matching name leaves Delete disabled', async ({ page }) => { const name = await navigateToProfileDetail(page); await page.getByTestId('profile-delete-btn').click(); From 9f34c67f41bb9264136c0e12177f6df1f161e30e Mon Sep 17 00:00:00 2001 From: "lorenzo.padovani@padosoft.com" Date: Tue, 19 May 2026 14:00:08 +0200 Subject: [PATCH 10/11] fix(slice-4c.1): no profile-detail fallback + inert modal close during DELETE MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit PR #29 iter 10 — address two items from Copilot review on dfd0cf3: 1. ScreenJumper bypass (app.tsx:10807): The profile-detail route defaulted `name` to PROFILES[0].name when params were missing. ScreenJumper navigates with no params, so a user opening the screen picker and jumping to "Profile detail" would silently land on (and could delete) the first profile. Now the route passes params.name unchanged and PageProfileDetail renders an explicit "no profile selected" state with an Info alert and a Back-to-profiles button (separate from the existing "profile not found" state for unknown names). 2. Modal close affordances during in-flight DELETE (app.tsx:5065): While submitting, the Cancel button was disabled but the Modal's backdrop click, X iconbtn, and Escape handler all still fired onClose — looking broken and letting the user dismiss the wizard mid-request. Now onClose is set to undefined while submitting so all three vectors no-op until the request resolves. Tests: - New e2e: ScreenJumper jump to "Profile detail" lands on the no-selection state, never the first profile; the Delete button is not rendered in that state. - New e2e: while DELETE is held in flight (route handler awaits a test-controlled promise), overlay click + X click + Escape are all no-ops; releasing the request closes the modal as expected. Verified locally: typecheck clean, lint clean, unit tests 39/39, e2e profile-delete 7/7. Co-Authored-By: Claude Opus 4.7 (1M context) --- packages/admin/src/app.tsx | 46 +++++++++-- packages/admin/test/e2e/profile-delete.e2e.ts | 79 +++++++++++++++++++ 2 files changed, 120 insertions(+), 5 deletions(-) diff --git a/packages/admin/src/app.tsx b/packages/admin/src/app.tsx index d91ce8d..c0b3c6e 100644 --- a/packages/admin/src/app.tsx +++ b/packages/admin/src/app.tsx @@ -5062,7 +5062,12 @@ function DeleteProfileWizard({ open, profileName, onClose, onDeleted }) { return ( @@ -8619,18 +8624,44 @@ function PageProfiles({ onNavigate, onOpenProfile, deletedProfiles }) { // ---------------- Profile detail ---------------- function PageProfileDetail({ name, onNavigate, deletedProfiles }) { - const rawP = profileByName(name); + const rawP = name ? profileByName(name) : null; // Treat just-deleted profiles as not-found so a stale link // (e.g. a notification linking to a profile a colleague just // removed) doesn't render the underlying mock row. - const isDeleted = deletedProfiles?.has(name); + const isDeleted = name ? deletedProfiles?.has(name) : false; const p = isDeleted ? null : rawP; const [deleteOpen, setDeleteOpen] = React.useState(false); // Now that this page has a destructive Delete action, a stale or // typoed route param (e.g. /profiles/) must NOT // silently render the first profile in the list — otherwise the // user could delete the wrong record. Show a clear not-found - // state instead. + // state instead. Two flavors: + // - `name` undefined → the route was opened without a selection + // (e.g. ScreenJumper jump-to-screen): show a "no profile + // selected" prompt rather than an arbitrary record. + // - `name` set but unknown → show the "no such profile" state. + if (!name) { + return ( +
+ + + + Open this page from the Profiles list (or a notification link) to view a specific + profile.{' '} + + + +
+ ); + } if (!p) { return (
@@ -10804,7 +10835,12 @@ const ROUTES = { label: 'Profile detail', section: 'Catalog', parent: 'profiles', - render: (ctx) => , + // Do NOT fall back to PROFILES[0].name here: ScreenJumper and other + // entrypoints navigate to `profile-detail` with no params, which would + // otherwise silently render (and let the user delete) the first + // profile. Pass `undefined` and let PageProfileDetail render an + // explicit "no profile selected" state. + render: (ctx) => , }, agents: { label: 'Agents', section: 'Catalog', render: (ctx) => }, diff --git a/packages/admin/test/e2e/profile-delete.e2e.ts b/packages/admin/test/e2e/profile-delete.e2e.ts index 16f0df9..ff71e06 100644 --- a/packages/admin/test/e2e/profile-delete.e2e.ts +++ b/packages/admin/test/e2e/profile-delete.e2e.ts @@ -133,6 +133,85 @@ test.describe('Profile delete confirmation', () => { await expect(page.locator('table.tbl tbody tr')).not.toHaveCount(0); }); + test('opening profile-detail with no params shows "no profile selected" (not the first profile)', async ({ + page, + }) => { + // Regression test for PR #29 iter 10 (Copilot): + // The route definition previously defaulted `name` to + // `PROFILES[0].name`, so ScreenJumper (which navigates without + // params) would silently land the user on — and let them delete — + // the first profile. The route now passes `undefined` and the + // page renders an explicit no-selection state. + await page.goto('/'); + await expect(page.locator('.sidebar')).toBeVisible(); + // Open the ScreenJumper and pick "Profile detail" — this is the + // exact code path that exposes the bypass: onNavigate(key) with + // no params. + await page.locator('button[title="Jump to screen"]').click(); + // Each popup row has a numeric-index span and a label span; target + // the label span by exact text and let the click bubble to the + // clickable parent div. + await page + .locator('span') + .filter({ hasText: /^Profile detail$/ }) + .first() + .click(); + // Should land on the no-selection state, NOT a profile name. + await expect(page.locator('.page-title, h1').first()).toContainText(/No profile selected/i); + // Crucially: no Delete button must be present on this state, so + // a stray click can't delete the first profile. + await expect(page.getByTestId('profile-delete-btn')).toHaveCount(0); + // "Back to profiles" returns the user to the list. + await page.getByTestId('profile-detail-back').click(); + await expect(page.locator('.page-title, h1').first()).toContainText(/Profiles/i); + }); + + test('modal close affordances (overlay, X, Escape) are inert while DELETE is in flight', async ({ + page, + }) => { + // Regression test for PR #29 iter 10 (Copilot): + // While `submitting` is true the Cancel button is disabled, but + // earlier the Modal's backdrop and "X" close button (and the + // Escape key handler) still fired onClose — looking broken and + // letting the user dismiss the wizard mid-request. The fix + // passes `onClose={submitting ? undefined : handleClose}`. + // + // We hold the DELETE open by deferring `route.fulfill` until the + // test resolves a promise, giving us a deterministic in-flight + // window in which to assert. + let resolveDelete: (() => void) | null = null; + const deleteHeld = new Promise((r) => { + resolveDelete = r; + }); + await page.route('**/api/profiles/*', async (route) => { + if (route.request().method() !== 'DELETE') return route.continue(); + await deleteHeld; + await route.fulfill({ + status: 200, + contentType: 'application/json', + body: JSON.stringify({ ok: true }), + }); + }); + const name = await navigateToProfileDetail(page); + await page.getByTestId('profile-delete-btn').click(); + await page.getByTestId('profile-delete-confirm').fill(name); + await page.getByTestId('profile-delete-submit').click(); + // Button now reads "Deleting…" — confirms we're in the in-flight + // window before we assert close-affordance inertness. + await expect(page.getByTestId('profile-delete-submit')).toContainText(/Deleting/i); + // Try every dismissal vector while in flight. Each one must be a + // no-op: the modal must still be present after the click/keypress. + await page.locator('.overlay').click({ force: true }); + await expect(page.locator('.modal-title')).toContainText(/Delete profile/i); + await page.locator('.modal-head .iconbtn').click(); // the X button + await expect(page.locator('.modal-title')).toContainText(/Delete profile/i); + await page.keyboard.press('Escape'); + await expect(page.locator('.modal-title')).toContainText(/Delete profile/i); + // Release the DELETE — modal should now close on success. + resolveDelete?.(); + await expect(page.locator('.modal-title')).toHaveCount(0); + }); + test('4xx keeps modal open and surfaces the server error', async ({ page }) => { await page.route('**/api/profiles/*', async (route) => { if (route.request().method() !== 'DELETE') return route.continue(); From d7af60d00563e769181804610eb1acafc3ad0656 Mon Sep 17 00:00:00 2001 From: "lorenzo.padovani@padosoft.com" Date: Tue, 19 May 2026 14:07:00 +0200 Subject: [PATCH 11/11] fix(slice-4c.1): definite-assignment for promise resolver in delete-in-flight test CI typecheck failed because TS narrows `let resolveDelete: T | null = null` to `null` (the assignment lives inside the Promise executor closure and doesn't update the outer type), making the later `resolveDelete?.()` call fail with TS2349 "Type 'never' has no call signatures". The Promise executor runs synchronously, so the resolver is guaranteed to be set before we use it. Switch to `let resolveDelete!: () => void` (definite-assignment assertion) and a direct call. Note: local Bun run had buffered output that masked the error; only the isolated `bun run typecheck` in packages/admin surfaces it. Verified now locally. Co-Authored-By: Claude Opus 4.7 (1M context) --- packages/admin/test/e2e/profile-delete.e2e.ts | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/packages/admin/test/e2e/profile-delete.e2e.ts b/packages/admin/test/e2e/profile-delete.e2e.ts index ff71e06..0f4c89a 100644 --- a/packages/admin/test/e2e/profile-delete.e2e.ts +++ b/packages/admin/test/e2e/profile-delete.e2e.ts @@ -179,7 +179,12 @@ test.describe('Profile delete confirmation', () => { // We hold the DELETE open by deferring `route.fulfill` until the // test resolves a promise, giving us a deterministic in-flight // window in which to assert. - let resolveDelete: (() => void) | null = null; + // Definite assignment assertion: the Promise executor runs + // synchronously inside the constructor, so `resolveDelete` is + // guaranteed to be set before the next statement. TS otherwise + // narrows the union to `null` because the assignment is inside + // a closure and refuses to call it. + let resolveDelete!: () => void; const deleteHeld = new Promise((r) => { resolveDelete = r; }); @@ -208,7 +213,7 @@ test.describe('Profile delete confirmation', () => { await page.keyboard.press('Escape'); await expect(page.locator('.modal-title')).toContainText(/Delete profile/i); // Release the DELETE — modal should now close on success. - resolveDelete?.(); + resolveDelete(); await expect(page.locator('.modal-title')).toHaveCount(0); });