fix(e2e): close the throwaway-customer leak at its source

e2e/admin-inline-edit.spec.ts

@@ -1,10 +1,15 @@
 import { test, expect, type Page } from '@playwright/test';
 import { login } from './helpers/auth';
+import {
+  INLINE_EDIT_PREFIX,
+  deleteThrowawayCustomers,
+  sweepStaleThrowawayCustomers,
+  throwawayCustomerName,
+} from './helpers/admin-fixtures';
 
 const ADD = /^(Hinzufügen|Add)$/i;
 const SAVE = /^(Speichern|Save)$/i;
 const EDIT = /^(Bearbeiten|Edit)$/i;
-const DELETE = /^(Löschen|Delete)$/i;
 
 /** A throwaway Customers row to mutate, by name. */
 function adminRow(page: Page, name: string) {
@@ -18,9 +23,7 @@ function adminRow(page: Page, name: string) {
  * shared seed rows, so re-runs stay idempotent and leave no residue.
  */
 async function createThrowawayCustomer(page: Page): Promise<string> {
-  // Date.now() alone can collide when parallel workers create a row in the same
-  // millisecond (a unique-name DB violation); a random suffix makes it collision-safe.
-  const name = `E2EInline_${Date.now()}_${Math.floor(Math.random() * 1_000_000)}`;
+  const name = throwawayCustomerName(INLINE_EDIT_PREFIX);
   await page.locator('.admin-crud-toolbar button.primary-button').filter({ hasText: ADD }).click();
   const form = page.locator('.modal form.stack-form');
   await expect(form).toBeVisible();
@@ -32,24 +35,6 @@ async function createThrowawayCustomer(page: Page): Promise<string> {
   return name;
 }
 
-/** Best-effort delete of the throwaway customer (native confirm), for finally blocks. */
-async function deleteThrowawayCustomer(page: Page, name: string): Promise<void> {
-  try {
-    const row = adminRow(page, name);
-    // Of the two names passed across a finally block (pre/post rename), only one
-    // row actually exists — skip the missing one instead of letting its delete
-    // button locator wait out the full timeout (a ~30s stall on every run). Bound
-    // the click for the same reason; the row is present, so it resolves at once.
-    if ((await row.count()) === 0) return;
-    page.once('dialog', (dialog) => dialog.accept());
-    await row.getByRole('button', { name: DELETE }).click({ timeout: 2000 });
-    await expect(row).toHaveCount(0);
-  } catch {
-    // Swallow — a mid-test failure may leave the page in a state where delete
-    // can't complete; never mask the original failure with a cleanup error.
-  }
-}
-
 /**
  * Inline (spreadsheet-style) cell editing on the SolidJS Administration tables,
  * built on the use:gridNav directive.
@@ -58,6 +43,11 @@ test.describe('Admin inline cell editing', () => {
   test.beforeEach(async ({ page }) => {
     // i.myself is a PL (ROLE_ADMIN), so the Administration page is reachable.
     await login(page, 'i.myself', 'myself123');
+    // Heal the shared db-e2e before touching the list: a previous run killed
+    // mid-test leaves an E2EInline row that sorts ahead of the seed rows and
+    // derails later specs (#675). Age-bounded, so a sibling worker's live row
+    // is never touched.
+    await sweepStaleThrowawayCustomers(page);
     await page.goto('/ui/admin');
     await page.waitForSelector('table.admin-table [role="gridcell"]', { timeout: 15000 });
   });
@@ -90,9 +80,8 @@ test.describe('Admin inline cell editing', () => {
       await expect(page.getByRole('gridcell', { name: updated, exact: true })).toBeVisible();
     } finally {
       // The row carries `updated` on success, or `name` if the rename never landed
-      // (mid-test failure); whichever is present, delete it best-effort.
-      await deleteThrowawayCustomer(page, updated);
-      await deleteThrowawayCustomer(page, name);
+      // (mid-test failure) — deleting by name prefix covers both without guessing.
+      await deleteThrowawayCustomers(page, name);
     }
   });
 
@@ -214,13 +203,10 @@ test.describe('Admin inline cell editing', () => {
       await row.getByRole('button', { name: EDIT }).click();
       await expect(page.locator('.modal input[type="text"]').first()).toHaveValue(draft);
     } finally {
-      // Close the modal (Escape-dismissible) so the row's Delete icon is clickable,
-      // then delete: the background auto-save may have renamed the row to `draft`,
-      // so cover both names best-effort.
-      await page.keyboard.press('Escape').catch(() => undefined);
-      await expect(page.locator('.modal')).toHaveCount(0).catch(() => undefined);
-      await deleteThrowawayCustomer(page, draft);
-      await deleteThrowawayCustomer(page, name);
+      // The background auto-save may have renamed the row to `draft`; the prefix
+      // delete covers both. No need to dismiss the modal first — the API delete
+      // does not go through the row's Delete icon, so an open modal cannot block it.
+      await deleteThrowawayCustomers(page, name);
     }
   });
 

e2e/admin/admin-ui.spec.ts

@@ -1,4 +1,10 @@
 import { test, expect, Page } from '@playwright/test';
+import {
+  ADMIN_UI_PREFIX,
+  deleteThrowawayCustomers,
+  sweepStaleThrowawayCustomers,
+  throwawayCustomerName,
+} from '../helpers/admin-fixtures';
 import { login } from '../helpers/auth';
 import { waitForGrid } from '../helpers/grid';
 import { goToAdminPage } from '../helpers/navigation';
@@ -45,6 +51,9 @@ test.describe('Administration UI', () => {
   test.beforeEach(async ({ page }) => {
     // i.myself is type PL → ROLE_ADMIN.
     await login(page, 'i.myself', 'myself123');
+    // Heal the shared db-e2e before touching the list — see the same call in
+    // admin-inline-edit.spec.ts and helpers/admin-fixtures.ts.
+    await sweepStaleThrowawayCustomers(page);
     await waitForGrid(page);
     await goToAdminPage(page);
   });
@@ -64,34 +73,44 @@ test.describe('Administration UI', () => {
   });
 
   test('creates, edits and deletes a customer', async ({ page }) => {
-    const name = `E2ECustomer_${Date.now()}`;
+    const name = throwawayCustomerName(ADMIN_UI_PREFIX);
 
-    // Create. A customer must be global or have teams (server-enforced), so
-    // mark it global to keep the fixture self-contained.
-    await page.locator('.admin-crud-toolbar button.primary-button').filter({ hasText: ADD }).click();
-    const form = page.locator('.modal form.stack-form');
-    await expect(form).toBeVisible();
-    await form.locator('.field input[type="text"]').first().fill(name);
-    await form.getByRole('checkbox', { name: 'Global', exact: true }).check();
-    await form.locator('button[type="submit"]').filter({ hasText: SAVE }).click();
-    await expect(page.locator('.modal')).toHaveCount(0);
-    await expect(row(page, name)).toHaveCount(1);
+    // Deleting through the UI is this test's SUBJECT, so it stays a UI click. The
+    // finally is the safety net: fail anywhere before that click — or on it — and
+    // the row would otherwise outlive the test and poison the name-sorted customer
+    // list for every later spec in the shard (#675).
+    try {
+      // Create. A customer must be global or have teams (server-enforced), so
+      // mark it global to keep the fixture self-contained.
+      await page.locator('.admin-crud-toolbar button.primary-button').filter({ hasText: ADD }).click();
+      const form = page.locator('.modal form.stack-form');
+      await expect(form).toBeVisible();
+      await form.locator('.field input[type="text"]').first().fill(name);
+      await form.getByRole('checkbox', { name: 'Global', exact: true }).check();
+      await form.locator('button[type="submit"]').filter({ hasText: SAVE }).click();
+      await expect(page.locator('.modal')).toHaveCount(0);
+      await expect(row(page, name)).toHaveCount(1);
 
-    // Edit (rename). The action buttons are icon-only, so match the accessible
-    // name (aria-label), not visible text.
-    const renamed = `${name}_Renamed`;
-    await row(page, name).getByRole('button', { name: EDIT }).click();
-    const editForm = page.locator('.modal form.stack-form');
-    await expect(editForm).toBeVisible();
-    const nameInput = editForm.locator('.field input[type="text"]').first();
-    await nameInput.fill(renamed);
-    await editForm.locator('button[type="submit"]').filter({ hasText: SAVE }).click();
-    await expect(page.locator('.modal')).toHaveCount(0);
-    await expect(row(page, renamed)).toHaveCount(1);
+      // Edit (rename). The action buttons are icon-only, so match the accessible
+      // name (aria-label), not visible text.
+      const renamed = `${name}_Renamed`;
+      await row(page, name).getByRole('button', { name: EDIT }).click();
+      const editForm = page.locator('.modal form.stack-form');
+      await expect(editForm).toBeVisible();
+      const nameInput = editForm.locator('.field input[type="text"]').first();
+      await nameInput.fill(renamed);
+      await editForm.locator('button[type="submit"]').filter({ hasText: SAVE }).click();
+      await expect(page.locator('.modal')).toHaveCount(0);
+      await expect(row(page, renamed)).toHaveCount(1);
 
-    // Delete (native confirm)
-    page.once('dialog', (dialog) => dialog.accept());
-    await row(page, renamed).getByRole('button', { name: DELETE }).click();
-    await expect(row(page, renamed)).toHaveCount(0);
+      // Delete (native confirm)
+      page.once('dialog', (dialog) => dialog.accept());
+      await row(page, renamed).getByRole('button', { name: DELETE }).click();
+      await expect(row(page, renamed)).toHaveCount(0);
+    } finally {
+      // A no-op on the happy path (the UI delete already removed the row); the
+      // prefix match covers `name` and `name_Renamed` alike.
+      await deleteThrowawayCustomers(page, name);
+    }
   });
 });

e2e/helpers/admin-fixtures.ts

@@ -0,0 +1,162 @@
+import { randomInt } from 'node:crypto';
+
+import { test, type Page } from '@playwright/test';
+
+/**
+ * Throwaway-customer lifecycle for the Administration specs.
+ *
+ * Those specs create a global, project-less customer to mutate so they never touch
+ * the shared seed rows. Such a row is dangerous residue: the customer list is
+ * name-sorted and shared run-wide, so an `E2E*` row that outlives its test sorts
+ * ahead of the seeded bookable customer and derails every later spec that reads the
+ * list (see SEEDED_BOOKABLE_CUSTOMER in helpers/worklog.ts). #675 hardened those
+ * consumers to pick by name; this module closes the leak at the source.
+ *
+ * Two independent guards, so no single failure can leak a row past the spec:
+ *  - deleteThrowawayCustomers() deletes over the HTTP API, not the UI — it works
+ *    from a `finally` even when the test died mid-interaction with a modal open,
+ *    and it reports a delete it could not perform instead of swallowing it.
+ *  - sweepStaleThrowawayCustomers() drops rows left behind by an earlier run that
+ *    crashed hard enough to skip its own cleanup.
+ */
+
+/** Name prefix per spec, so a sweep can tell throwaway rows from seed rows. */
+export const INLINE_EDIT_PREFIX = 'E2EInline';
+export const ADMIN_UI_PREFIX = 'E2ECustomer';
+
+const THROWAWAY_PREFIXES = [INLINE_EDIT_PREFIX, ADMIN_UI_PREFIX];
+
+/**
+ * `<prefix>_<epoch ms>_<rand>`, plus whatever a spec appends while mutating it
+ * (`-edited`, `-draft`, `_Renamed`). The captured epoch ms is what makes the sweep
+ * safe under `fullyParallel` — see sweepStaleThrowawayCustomers. The digit run is
+ * bounded (no unbounded quantifier) and the pattern is anchored.
+ */
+const THROWAWAY_NAME = new RegExp(`^(?:${THROWAWAY_PREFIXES.join('|')})_(\\d{10,16})_`);
+
+/**
+ * A row younger than this may belong to a test running right now in the other
+ * worker (`fullyParallel: true`, 2 workers per CI shard), so the sweep leaves it
+ * alone. Playwright's per-test timeout is 30s, so a row this old is provably
+ * residue from an earlier run and can never be in use.
+ */
+const STALE_AFTER_MS = 10 * 60 * 1000;
+
+interface CustomerRow {
+  customer: { id: number; name: string };
+}
+
+/**
+ * Report a cleanup that did not happen. Deliberately non-fatal: this runs from a
+ * `finally`, where throwing would replace the test's own error with a teardown one.
+ * The annotation surfaces the leak in the HTML report, the console line in the CI
+ * log — a silent leak is what let a row survive the spec in the first place.
+ */
+function reportLeak(message: string): void {
+  console.error(`[e2e cleanup] ${message}`);
+  try {
+    test.info().annotations.push({ type: 'cleanup-failed', description: message });
+  } catch {
+    // Called outside a test (no TestInfo) — the console line above still stands.
+  }
+}
+
+/** Every customer the admin list shows, or null when the read itself failed. */
+async function readCustomers(page: Page): Promise<CustomerRow[] | null> {
+  try {
+    const response = await page.request.get('/getAllCustomers');
+    if (!response.ok()) {
+      reportLeak(`GET /getAllCustomers answered ${response.status()} — cannot verify throwaway customers were removed`);
+      return null;
+    }
+    return (await response.json()) as CustomerRow[];
+  } catch (error) {
+    reportLeak(`GET /getAllCustomers failed (${String(error)}) — cannot verify throwaway customers were removed`);
+    return null;
+  }
+}
+
+/**
+ * Delete every given customer, then report the ones that are still there.
+ *
+ * The verdict comes from re-reading the list, not from the delete's status code:
+ * `/customer/delete` answers 422 both when the row could not go (a real leak) and
+ * when it was already gone — which is the NORMAL outcome when the sibling worker
+ * swept the same stale row a moment earlier. Reporting on the status alone made
+ * every parallel sweep cry leak; the list says what actually happened, and says it
+ * in any locale.
+ */
+async function deleteRows(page: Page, rows: CustomerRow['customer'][]): Promise<void> {
+  if (rows.length === 0) {
+    return;
+  }
+  for (const row of rows) {
+    await page.request.post('/customer/delete', { data: { id: row.id } }).catch(() => undefined);
+  }
+  const remaining = await readCustomers(page);
+  if (remaining === null) {
+    return;
+  }
+  for (const row of rows) {
+    if (remaining.some((candidate) => candidate.customer.id === row.id)) {
+      reportLeak(`LEAKED customer "${row.name}" (id ${row.id}) — /customer/delete did not remove it; it will pollute the name-sorted customer list for the rest of this shard`);
+    }
+  }
+}
+
+/** A throwaway name for `prefix`, collision-safe across parallel workers.
+ *  Date.now() alone can collide when two workers create a row in the same
+ *  millisecond (a unique-name DB violation); the random suffix rules that out.
+ *  crypto.randomInt only because Sonar flags Math.random as a hotspot —
+ *  nothing here is security-relevant, the suffix just de-duplicates names. */
+export function throwawayCustomerName(prefix: string): string {
+  return `${prefix}_${Date.now()}_${randomInt(1_000_000)}`;
+}
+
+/**
+ * Delete the throwaway customer named `base` and every variant a spec renamed it
+ * to (`base-edited`, `base-draft`, `base_Renamed`, …) — matching on the prefix
+ * means the caller never has to enumerate which name actually landed.
+ *
+ * Goes over the HTTP API rather than the row's Delete button on purpose: a test
+ * that failed mid-UI can leave a modal open or the row detached, which is exactly
+ * when the UI delete silently gave up and the row leaked. Nothing here throws, so
+ * a `finally` calling it cannot mask the test's own failure; a delete that does not
+ * happen is reported instead.
+ */
+export async function deleteThrowawayCustomers(page: Page, base: string): Promise<void> {
+  const customers = await readCustomers(page);
+  if (customers === null) {
+    return;
+  }
+  await deleteRows(
+    page,
+    customers.map((row) => row.customer).filter((customer) => customer.name.startsWith(base)),
+  );
+}
+
+/**
+ * Drop throwaway customers left behind by an EARLIER run — a run killed hard
+ * enough to skip its own `finally` still poisons the shared, persistent db-e2e for
+ * every run after it. Call it from a beforeEach of the specs that create them, so
+ * the shard heals itself instead of needing a manual DB cleanup.
+ *
+ * Only rows older than STALE_AFTER_MS are touched: under `fullyParallel` a sibling
+ * test in the other worker may have a fresh throwaway row live at this very moment,
+ * and deleting it would break that test.
+ */
+export async function sweepStaleThrowawayCustomers(page: Page): Promise<void> {
+  const customers = await readCustomers(page);
+  if (customers === null) {
+    return;
+  }
+  const now = Date.now();
+  const stale = customers.map((row) => row.customer).filter((customer) => {
+    const stamp = THROWAWAY_NAME.exec(customer.name);
+    return stamp !== null && now - Number(stamp[1]) > STALE_AFTER_MS;
+  });
+  if (stale.length > 0) {
+    console.error(`[e2e cleanup] sweeping ${stale.length} stale throwaway customer(s) from an earlier run: ${stale.map((customer) => customer.name).join(', ')}`);
+    await deleteRows(page, stale);
+  }
+}