From 49663b5d5f4767f181a1a33eb1b14b6ac591a600 Mon Sep 17 00:00:00 2001 From: TurtleWolfe Date: Fri, 31 Jul 2026 19:48:02 +0000 Subject: [PATCH 1/3] =?UTF-8?q?wip(#430):=20wrap=20the=20two=20admin=20tab?= =?UTF-8?q?les=20in=20a=20padded=20sh-well=20=E2=80=94=20NOT=20YET=20VERIF?= =?UTF-8?q?IED?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The deferred half of #430. Both scrollers were a bare `
` at AdminPaymentPanel.tsx:216 and AdminMessagingOverview.tsx:173 — the line numbers in the ticket were accurate, unlike #373's, which drifted four times. `sh-well` paints an inset shadow BELOW its children, so it needs a padded parent: putting it on the `overflow-x-auto` div itself would clip the shadow inside the scroller and hide it under the table. So the scroller is WRAPPED by `sh-well rounded-lg p-2` rather than annotated. WHAT IS NOT VERIFIED, AND WHY THIS IS NOT PUSHED. I could not confirm the wells actually PAINT. Both routes are auth-gated and the E2E storage state at tests/e2e/fixtures/storage-state-auth.json is stale — with it loaded, /admin/payments redirects to the home page, so the components never render and a `.sh-well` query returns 0. That matters more than usual here: these two items were DEFERRED from #430 precisely because a sh-well with the wrong parent is a visual no-op, and "a class being present proves nothing about whether it renders" has been true four times in this session. Type-check clean and 35 component tests passing do not answer the question that was actually asked. Hit my own documented trap on the way: a `{/* comment */}` placed between `{cond ? (` and its element is two root nodes and a syntax error. The comment now sits above the ternary. Refs #430 — needs an authenticated visual check before this ships. --- .../AdminMessagingOverview.tsx | 59 +++++++----- .../AdminPaymentPanel/AdminPaymentPanel.tsx | 91 ++++++++++--------- 2 files changed, 81 insertions(+), 69 deletions(-) diff --git a/src/components/organisms/AdminMessagingOverview/AdminMessagingOverview.tsx b/src/components/organisms/AdminMessagingOverview/AdminMessagingOverview.tsx index 53031e15..f7dce3e4 100644 --- a/src/components/organisms/AdminMessagingOverview/AdminMessagingOverview.tsx +++ b/src/components/organisms/AdminMessagingOverview/AdminMessagingOverview.tsx @@ -169,33 +169,42 @@ export function AdminMessagingOverview({ />

Top Senders

+ {/* See AdminPaymentPanel: the well needs a padded parent around the + scroller, not on it (#430). The comment lives ABOVE the ternary — + a `{comment}` between `? (` and its element is two root nodes and a + syntax error, which is exactly what it was a moment ago. */} {(trends.top_senders ?? []).length > 0 ? ( -
- - - - - - - - - {(trends.top_senders ?? []).map((s) => ( - - - +
+
+
UserMessages
-
- {s.display_name ?? s.username ?? 'N/A'} -
- {s.username && s.display_name && ( -
- @{s.username} -
- )} - 		{s.messages}
+ + + + - ))} - -
UserMessages
+ + + {(trends.top_senders ?? []).map((s) => ( + + +
+ {s.display_name ?? s.username ?? 'N/A'} +
+ {s.username && s.display_name && ( +
+ @{s.username} +
+ )} + + {s.messages} + + ))} + + +
) : (

-

- - - - - - - - - - - - - {(trends.provider_breakdown ?? []).map((p) => { - const flagged = failureShare(p) > FAILURE_FLAG_THRESHOLD; - return ( - - - - - - - - - ); - })} - {(trends.provider_breakdown ?? []).length === 0 && ( + {/* The scroller is WRAPPED, not replaced (#430). `sh-well` paints an + inset shadow BELOW its children, so it needs a padded parent — put it + on the `overflow-x-auto` div itself and the shadow is clipped by the + scroller and hidden under the table. */} +
+
+
ProviderSucceededFailedRefundedRevenueHealth
{p.provider}{p.succeeded}{p.failed}{p.refunded}{formatCents(p.revenue_cents)} - {flagged ? ( - - Elevated failures - - ) : ( - OK - )} -
+ - + + + + + + - )} - -
- No activity in this range - ProviderSucceededFailedRefundedRevenueHealth
+ + + {(trends.provider_breakdown ?? []).map((p) => { + const flagged = failureShare(p) > FAILURE_FLAG_THRESHOLD; + return ( + + {p.provider} + {p.succeeded} + {p.failed} + {p.refunded} + {formatCents(p.revenue_cents)} + + {flagged ? ( + + Elevated failures + + ) : ( + OK + )} + + + ); + })} + {(trends.provider_breakdown ?? []).length === 0 && ( + + + No activity in this range + + + )} + + +
)} From ade8fc05b4328dbcf76dd6292c8083d92622c2bd Mon Sep 17 00:00:00 2001 From: TurtleWolfe Date: Mon, 3 Aug 2026 21:40:11 +0000 Subject: [PATCH 2/3] feat(#454, #430): an is_admin E2E fixture, and the two AAA violations it found MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit No E2E run had ever rendered an admin page. `AdminGate.tsx:81` bounces an authenticated non-admin to `/`, so three separate things were measuring the wrong page or nothing: the AAA sweep reported six `/admin*` routes as swept while measuring the HOME page, the admin table wells could not be verified (draft PR #503 sat 17/17 green and unmergeable), and the landmark gate had to exclude those six outright. `seedIsolatedAdmin()` promotes a throwaway user through `user_profiles.is_admin` — the single authority since #240, read live by the `is_admin()` SECURITY DEFINER RPC. One service-role UPDATE; no token refresh and no auth-hook registration, because no live RLS policy reads the JWT claim. It VERIFIES ITSELF through the user's own session and throws otherwise. A silently-unpromoted user still renders — it renders the redirect — so every assertion downstream would measure the home page and pass. That is the defect #454 is about, and without the check the fixture built to fix it would reproduce it. Not the shared storage-state user, deliberately: `is_admin()` gates 25 policies across 9 tables, so promoting it would give every existing spec cross-user reads and row-count assertions would keep passing while measuring something else. Unblinding the sweep immediately found two real AAA violations on /admin/email, both invisible until now: .stat-title #484f58 on #ebe5dd 6.62:1 (3 elements) text-base-content/70 #5f656d on #f5f0eb 5.19:1 (2 elements) The first is the sharper one. globals.css ALREADY corrected `.stat-title`, raising alpha to 80% and documenting "~7.1:1 on light". Measured: 6.62. 7.1 was computed against base-100; a `.stats` block sits on base-200 — exactly the trap #462 records, where /80 measures 7.08 on base-100 and 6.4-6.5 on base-200. The alpha was right for the surface it was checked on and wrong for the one it renders on. Now solid, like #411, #425 and #495 before it. Coverage: landmark sweep 36 -> 42 paths (exclusions 9 -> 3); contrast sweep 36 -> 42 paths x 2 themes, 85 passing. Mutation-tested against the original defect: with the sh-well wrapper reverted, admin-depth reports `0/1 scrollers welled` and fails by name. The spec asserts the INVARIANT (every scroller is welled) rather than the element, because /admin/messaging's well is behind `top_senders.length > 0` — a data-dependent element is how #495 looked like a flake. A floor stops it passing vacuously. Orphan sweep in auth.setup.ts pages through listUsers (#197 truncates at 50) and deletes throwaway admins that survived a previous run. Co-Authored-By: Claude Opus 5 (1M context) --- src/app/globals.css | 23 ++- .../EmailProviderHealth.tsx | 4 +- tests/e2e/admin/admin-depth.spec.ts | 153 ++++++++++++++++++ tests/e2e/auth.setup.ts | 57 +++++++ tests/e2e/color-contrast.spec.ts | 46 +++++- tests/e2e/landmarks.spec.ts | 90 +++++++---- tests/e2e/utils/test-user-factory.ts | 140 ++++++++++++++++ 7 files changed, 473 insertions(+), 40 deletions(-) create mode 100644 tests/e2e/admin/admin-depth.spec.ts diff --git a/src/app/globals.css b/src/app/globals.css index c1e94435..087f80b9 100644 --- a/src/app/globals.css +++ b/src/app/globals.css @@ -470,14 +470,29 @@ 0 2px 4px -2px rgb(0 0 0 / 0.2); } -/* DaisyUI's .stat-title and .stat-desc ship with base-content at 60% alpha, - * which falls short of WCAG AA (3.86:1 on scripthammer-light, needs 4.5:1). - * Raising alpha to 80% gives ~7.1:1 on light and ~9.3:1 on dark — AAA on both. */ +/* DaisyUI's .stat-title and .stat-desc ship base-content at 60% alpha, which + * falls short of WCAG AA. This block already corrected that to 80% — and 80% + * is STILL SHORT, which nobody could see until #454's admin fixture let the AAA + * sweep reach an admin route. + * + * The old comment claimed "~7.1:1 on light". Measured on /admin/email: + * + * .stat-title #484f58 on #ebe5dd = 6.62:1 against a 7:1 gate + * + * 7.1 was computed against **base-100**. A `.stats` block sits on **base-200**, + * and that is exactly the surface #462 records as the trap: `/80` measures 7.08 + * on base-100 and 6.4–6.5 on base-200. The alpha was right for the surface it + * was checked on and wrong for the one it renders on. + * + * Solid, therefore — the same conclusion #411, #425 and #495 reached for + * `.label`, the 404 copy and the conversation timestamp. A stat's caption is + * the label for its number; there is no reading in which it should be dimmer + * than the text around it. */ [data-theme='scripthammer-dark'] .stat-title, [data-theme='scripthammer-dark'] .stat-desc, [data-theme='scripthammer-light'] .stat-title, [data-theme='scripthammer-light'] .stat-desc { - color: color-mix(in oklab, var(--color-base-content) 80%, transparent); + color: var(--color-base-content); } /* Avatar glow/shadow — uses base-content so it auto-adapts: diff --git a/src/components/organisms/EmailProviderHealth/EmailProviderHealth.tsx b/src/components/organisms/EmailProviderHealth/EmailProviderHealth.tsx index 03897f12..54a6aa9d 100644 --- a/src/components/organisms/EmailProviderHealth/EmailProviderHealth.tsx +++ b/src/components/organisms/EmailProviderHealth/EmailProviderHealth.tsx @@ -96,7 +96,7 @@ export default function EmailProviderHealth({ ) : ( -

Rate-limit status unavailable.

+

Rate-limit status unavailable.

)} @@ -121,7 +121,7 @@ export default function EmailProviderHealth({

{p.name}

-

+

Priority {p.priority}

diff --git a/tests/e2e/admin/admin-depth.spec.ts b/tests/e2e/admin/admin-depth.spec.ts new file mode 100644 index 00000000..f33bf30f --- /dev/null +++ b/tests/e2e/admin/admin-depth.spec.ts @@ -0,0 +1,153 @@ +/** + * The admin console rendered AS AN ADMIN (#430, #454). + * + * ## Why this file exists + * + * Until now nothing in the suite had ever rendered an admin page. `AdminGate` + * calls `router.push('/')` for an authenticated non-admin (`AdminGate.tsx:81`) + * and returns `null` while that navigation is in flight (`:98`), so the E2E + * user — authenticated but not an admin — was bounced every time. + * + * That is not a gap you can see from a test report. It is why draft PR #503 sat + * at 17/17 green and unmergeable: the suite never reached the pages the work + * changed, so green meant "measured nothing", not "verified". + * + * These tests use the isolated-admin fixture, which promotes a throwaway user + * via `user_profiles.is_admin` — the single authority since #240 — and refuses + * to hand back a fixture whose `is_admin()` does not answer true through the + * user's OWN session. + * + * ## The assertion that has to come first + * + * Every test here asserts it LANDED on the admin route before it measures + * anything. Without that, a redirect leaves the assertions measuring the home + * page, and a clean home page passes them. That failure mode is #454 exactly, + * and it is the reason this file leads with the landing check rather than + * trusting the fixture. + */ + +import { test, expect, type Page } from '@playwright/test'; +import { + seedIsolatedAdmin, + deleteIsolatedAdmin, + openAdminAs, + type IsolatedAdmin, +} from '../utils/test-user-factory'; + +/** + * Prove we are on the route we asked for, and that AdminGate let us through. + * + * `AdminGate` renders `null` for a non-admin, so "the URL is right" is not + * enough on its own — the console's own rail is the evidence the gate opened. + */ +async function assertLandedOnAdmin(page: Page, route: string): Promise { + await expect( + page, + `expected to land on ${route}; a redirect here means AdminGate bounced the ` + + `fixture and everything below would be measuring the home page` + ).toHaveURL(new RegExp(`${route.replace(/\//g, '\\/')}\\/?$`)); + + await expect( + page.getByRole('navigation', { name: /admin/i }).first(), + `${route} rendered without the admin rail — AdminGate returned null, so the ` + + `page is empty and any assertion below would pass on nothing` + ).toBeVisible({ timeout: 20000 }); +} + +test.describe('Admin console, rendered as an admin', () => { + test.describe.configure({ timeout: 90000 }); + + /** + * `/admin/payments`'s scroller well is UNCONDITIONAL. `/admin/messaging`'s is + * behind `top_senders.length > 0` (`AdminMessagingOverview.tsx:176`), so on a + * project with no recent senders it legitimately does not render — a + * data-dependent element, the same shape that made #495 look like a flake. + * + * So the assertion is the INVARIANT rather than the element: every horizontal + * scroller in the admin console is wrapped by `sh-well`. That holds whether + * or not the data exists. `minScrollers` is the floor that stops it passing + * vacuously — without it, "no scrollers rendered" and "every scroller is + * welled" are the same green. + */ + const ROUTES = [ + { route: '/admin/payments', minScrollers: 1 }, + { route: '/admin/messaging', minScrollers: 0 }, + ]; + + for (const { route, minScrollers } of ROUTES) { + test(`${route}: every table scroller sits in a padded well`, async ({ + browser, + }) => { + let fixture: IsolatedAdmin | null = null; + let opened: Awaited> | null = null; + try { + fixture = await seedIsolatedAdmin(); + test.skip(!fixture, 'Admin client unavailable to seed an admin'); + if (!fixture) return; + + opened = await openAdminAs(browser, fixture.session, route); + const { page } = opened; + + await assertLandedOnAdmin(page, route); + + // #430: the scroller is WRAPPED, not replaced — `sh-well` paints an + // inset shadow BELOW its children, so on the `overflow-x-auto` div + // itself the shadow is clipped inside the scroll area and hidden under + // the table. + const counts = await page.evaluate(() => { + const scrollers = [...document.querySelectorAll('.overflow-x-auto')]; + return { + total: scrollers.length, + welled: scrollers.filter((el) => !!el.closest('.sh-well')).length, + unwelled: scrollers + .filter((el) => !el.closest('.sh-well')) + .map((el) => (el.parentElement?.className || '').slice(0, 60)), + }; + }); + + // Printed every run: a route measuring zero scrollers must be visibly + // different from one measuring three. + console.log( + `[admin-depth] ${route}: ${counts.welled}/${counts.total} scrollers welled` + ); + + expect( + counts.total, + `${route}: expected at least ${minScrollers} horizontal scroller(s); ` + + `found ${counts.total}. A vacuous pass is not a pass.` + ).toBeGreaterThanOrEqual(minScrollers); + + expect( + counts.unwelled, + `${route}: ${counts.unwelled.length} scroller(s) are not wrapped in ` + + `.sh-well — this is the assertion #503 never had, and the reason ` + + `that PR could not be verified on a green run.` + ).toEqual([]); + } finally { + if (opened) await opened.close(); + await deleteIsolatedAdmin(fixture); + } + }); + } + + test('an admin route is reachable at all — the gate opens for a promoted user', async ({ + browser, + }) => { + // Deliberately separate from the well assertions. If the fixture ever stops + // producing a real admin, this fails on its own and names the cause, rather + // than every admin test failing with a confusing "well not found". + let fixture: IsolatedAdmin | null = null; + let opened: Awaited> | null = null; + try { + fixture = await seedIsolatedAdmin(); + test.skip(!fixture, 'Admin client unavailable to seed an admin'); + if (!fixture) return; + + opened = await openAdminAs(browser, fixture.session, '/admin'); + await assertLandedOnAdmin(opened.page, '/admin'); + } finally { + if (opened) await opened.close(); + await deleteIsolatedAdmin(fixture); + } + }); +}); diff --git a/tests/e2e/auth.setup.ts b/tests/e2e/auth.setup.ts index 0d3200ed..3e016db9 100644 --- a/tests/e2e/auth.setup.ts +++ b/tests/e2e/auth.setup.ts @@ -19,6 +19,8 @@ import { ensureEncryptionKeys, getUserByEmail, getAdminClient, + deleteTestUser, + ISO_ADMIN_PREFIX, } from './utils/test-user-factory'; const AUTH_FILE = 'tests/e2e/fixtures/storage-state-auth.json'; @@ -27,7 +29,62 @@ const AUTH_FILE_B = 'tests/e2e/fixtures/storage-state-auth-b.json'; // Allow extra time for static page hydration + encryption setup setup.setTimeout(180000); +/** + * Delete throwaway admins that survived a previous run. + * + * `seedIsolatedAdmin` grants `user_profiles.is_admin`, which gates 25 policies + * across 9 tables — an orphan can read every message, payment and profile in + * the shared project. Teardown lives in a `finally`, but a killed worker or a + * cancelled shard skips it, so this runs once per run as the backstop. + * + * PAGINATES. `admin.listUsers()` defaults to one page and silently truncates + * past 50 (#197) — a sweep that reads page one and reports "0 orphans" is + * exactly the kind of signal this repo keeps finding: it looks like a clean + * result and is actually an unread list. + */ +async function sweepOrphanedAdmins(): Promise { + const admin = getAdminClient(); + if (!admin) return; + + const orphans: { id: string; email: string }[] = []; + const PER_PAGE = 200; + for (let page = 1; page <= 50; page++) { + const { data, error } = await admin.auth.admin.listUsers({ + page, + perPage: PER_PAGE, + }); + if (error) { + console.warn( + `[admin-sweep] listUsers page ${page} failed: ${error.message}` + ); + return; + } + const users = data?.users ?? []; + for (const u of users) { + if (u.email?.includes(ISO_ADMIN_PREFIX)) { + orphans.push({ id: u.id, email: u.email }); + } + } + if (users.length < PER_PAGE) break; // last page + } + + if (orphans.length === 0) { + console.log('[admin-sweep] no orphaned throwaway admins'); + return; + } + + console.warn( + `[admin-sweep] ${orphans.length} orphaned throwaway admin(s) from a ` + + `previous run — deleting. Each could read every message and payment in ` + + `the project:\n` + + orphans.map((o) => ` - ${o.email}`).join('\n') + ); + for (const o of orphans) await deleteTestUser(o.id); +} + setup('authenticate shared test user', async ({ page, browser }) => { + await sweepOrphanedAdmins(); + // Always run fresh — never use cached auth state. Cached state from // previous runs may lack encryption keys, causing all messaging tests // to redirect to /messages/setup and fail. diff --git a/tests/e2e/color-contrast.spec.ts b/tests/e2e/color-contrast.spec.ts index e5e86af3..7c4fc185 100644 --- a/tests/e2e/color-contrast.spec.ts +++ b/tests/e2e/color-contrast.spec.ts @@ -1,5 +1,11 @@ import { test, expect } from '@playwright/test'; import { readFileSync, readdirSync } from 'node:fs'; +import { + seedIsolatedAdmin, + deleteIsolatedAdmin, + openAuthedPage, + type IsolatedAdmin, +} from './utils/test-user-factory'; import { dirname, join } from 'node:path'; // Pa11y's axe runner reports axe `incomplete` results as errors, which @@ -201,7 +207,27 @@ test.describe('WCAG AAA color-contrast-enhanced (violations only)', () => { for (const theme of THEMES) { for (const path of PAGES) { - test(`${theme} — ${path}`, async ({ page }) => { + test(`${theme} — ${path}`, async ({ page: defaultPage, browser }) => { + // The six `/admin` routes need an ADMIN session (#454). Until now this + // sweep listed them and measured THE HOME PAGE: `AdminGate.tsx:81` + // redirects an authenticated non-admin to `/`, and nothing here + // asserted the probe landed where it asked. A populated, AAA-clean home + // page passed six times under other routes' names — not an absent + // measurement, a WRONG one that read as coverage. + const needsAdmin = path.startsWith('/admin'); + let adminFixture: IsolatedAdmin | null = null; + let openedAdmin: Awaited> | null = + null; + let page = defaultPage; + if (needsAdmin) { + adminFixture = await seedIsolatedAdmin(); + test.skip(!adminFixture, 'Admin client unavailable to seed an admin'); + if (!adminFixture) return; + openedAdmin = await openAuthedPage(browser, adminFixture.session); + page = openedAdmin.page; + await page.setViewportSize({ width: 1280, height: 1024 }); + } + // ThemeScript.tsx reads localStorage.getItem('theme') before falling // back to prefers-color-scheme; seeding it in an init script runs // before that inline script. @@ -230,6 +256,17 @@ test.describe('WCAG AAA color-contrast-enhanced (violations only)', () => { // resolves it. If that ever changes, the sweep would quietly measure a // different page and report it as template coverage — which is the // failure mode this whole ticket is about. + // Same guarantee as the template probe below, for the admin routes: + // a redirect here means everything measured belongs to another page. + if (needsAdmin) { + await expect( + page, + `${path}: redirected away before measuring. The admin fixture did ` + + `not take, and axe would be reporting the home page under this ` + + `route's name — which is #454 itself.` + ).toHaveURL(new RegExp(`${path.replace(/\//g, '\\/')}\\/?$`)); + } + const probe = TEMPLATE_PROBES[path]; if (probe) { await expect( @@ -284,6 +321,13 @@ test.describe('WCAG AAA color-contrast-enhanced (violations only)', () => { 0 ); + // Tear the throwaway admin down BEFORE the assertion, so a violation + // does not leave a live admin account behind. `auth.setup.ts` sweeps + // orphans, but a failing test is exactly when you least want to rely + // on a backstop. + if (openedAdmin) await openedAdmin.close(); + await deleteIsolatedAdmin(adminFixture); + expect( details, `color-contrast-enhanced (AAA) violations on ${path} [${theme}] ` + diff --git a/tests/e2e/landmarks.spec.ts b/tests/e2e/landmarks.spec.ts index a14d77da..c2d77e62 100644 --- a/tests/e2e/landmarks.spec.ts +++ b/tests/e2e/landmarks.spec.ts @@ -1,5 +1,12 @@ import { test, expect } from '@playwright/test'; import { readdirSync } from 'node:fs'; +import { + seedIsolatedAdmin, + deleteIsolatedAdmin, + openAuthedPage, + dismissCookieBanner, + type IsolatedAdmin, +} from './utils/test-user-factory'; import { join } from 'node:path'; /** @@ -35,33 +42,6 @@ import { join } from 'node:path'; const APP_DIR = join(process.cwd(), 'src/app'); -/** - * The six `/admin` routes, excluded together and for one reason. - * - * The E2E user is authenticated but NOT an admin, so `AdminGate.tsx:81` calls - * `router.push('/')` and `:98` returns `null` while that navigation is in - * flight. Measuring them would race the redirect: sometimes the blank frame, - * sometimes `/`'s own landmark. A gate that reports different things on - * different runs is worse than one that says plainly what it cannot see. - * - * This is the same missing admin fixture that **#454** is filed for (the AAA - * contrast sweep lists all six and measures none) and that blocks **#430**. - * When that fixture exists, delete this block — `AdminGate`'s authed wrapper is - * already a `
`, so these routes should pass the moment they can be - * reached. - */ -const ADMIN_ROUTES = [ - '/admin', - '/admin/audit', - '/admin/email', - '/admin/messaging', - '/admin/payments', - '/admin/users', -]; -const ADMIN_REASON = - 'E2E user is not an admin; AdminGate redirects to / — needs the admin ' + - 'fixture tracked in #454/#430. AdminGate now renders
for a real admin.'; - /** Routes that cannot be measured, each with the reason. Never silent. */ const EXCLUDED: Record = { '/auth/callback': @@ -70,7 +50,6 @@ const EXCLUDED: Record = { 'twin payloads are privacy-gated and gitignored; absent in a fresh checkout', '/chatt': 'Cesium canvas host; the widget owns the viewport and headless Firefox has no WebGL', - ...Object.fromEntries(ADMIN_ROUTES.map((r) => [r, ADMIN_REASON])), }; /** Dynamic segments need a real instance — the template alone proves nothing. */ @@ -123,8 +102,12 @@ const PATHS = [ * not shrunk, they had stopped being looked at, and only the floor caught it * (#396). Raise it when routes are added. * - * 36 = 44 `page.tsx` − 3 EXCLUDED − 6 ADMIN_ROUTES + 1 template probe. It is - * the CURRENT measured coverage, which is what a floor should be. + * 42 = 44 `page.tsx` − 3 EXCLUDED + 1 template probe. It is the CURRENT + * measured coverage, which is what a floor should be. + * + * It was 36. The six `/admin` routes were excluded because the E2E user was not + * an admin and `AdminGate` bounced it; `seedIsolatedAdmin` (#454) removed that + * constraint, so they are measured now and the floor rose with them. * * **Not an instance of the #396 anti-pattern.** The first version said 40 — * above the achievable maximum, so it failed on a run where all 36 paths passed. @@ -133,7 +116,7 @@ const PATHS = [ * yourself reducing this because a run went red, the sweep shrank — find out * which routes stopped being measured before touching the number. */ -const MIN_PATHS = 36; +const MIN_PATHS = 42; // Loudly, so a shrinking sweep can never read as a passing one. console.log( @@ -160,13 +143,51 @@ test.describe('Landmarks and skip link', () => { }); for (const path of PATHS) { - test(`${path} has one
and a working skip link`, async ({ page }) => { - const resp = await page.goto(path, { + test(`${path} has one
and a working skip link`, async ({ + page: defaultPage, + browser, + }) => { + // The six `/admin` routes need an ADMIN session. `AdminGate.tsx:81` + // bounces an authenticated non-admin to `/`, and `/` has both a
+ // and a skip link — so measuring them on the default page would PASS + // while measuring the home page. That is #454's defect exactly, and + // deleting the old exclusion without this would have re-created it here. + const needsAdmin = path.startsWith('/admin'); + let adminFixture: IsolatedAdmin | null = null; + let openedAdmin: Awaited> | null = null; + let page = defaultPage; + let resp: Awaited> = null; + + if (needsAdmin) { + adminFixture = await seedIsolatedAdmin(); + test.skip(!adminFixture, 'Admin client unavailable to seed an admin'); + if (!adminFixture) return; + // `openAuthedPage` + our OWN goto, deliberately — not `openAdminAs`. + // That helper prepends NEXT_PUBLIC_BASE_PATH and navigates itself, so + // going again here with the bare path produced a 404: an absolute path + // REPLACES the basePath rather than appending to it. Navigating exactly + // once, with the same bare path every other route in this sweep uses, + // keeps admin and non-admin routes measured identically. + openedAdmin = await openAuthedPage(browser, adminFixture.session); + page = openedAdmin.page; + } + resp = await page.goto(path, { waitUntil: 'domcontentloaded', timeout: 45000, }); + if (needsAdmin) await dismissCookieBanner(page); const status = resp?.status() ?? 0; + // Landing check, for the admin routes specifically: a redirect leaves + // every assertion below measuring `/`. + if (needsAdmin) { + await expect( + page, + `${path}: redirected away — the admin fixture did not take, and the ` + + `assertions below would be measuring the home page` + ).toHaveURL(new RegExp(`${path.replace(/\//g, '\\/')}\\/?$`)); + } + // A route template is REACHED, not enumerated — its probe path is // expected to 404. Everything else must be a real 200: see the header, // a non-200 measured as an a11y result is how phantom defects get filed. @@ -209,6 +230,9 @@ test.describe('Landmarks and skip link', () => { `${path}: skip link points at ${href}, which does not exist. A skip ` + `link with no target is worse than none — it silently does nothing.` ).toHaveCount(1); + + if (openedAdmin) await openedAdmin.close(); + await deleteIsolatedAdmin(adminFixture); }); } diff --git a/tests/e2e/utils/test-user-factory.ts b/tests/e2e/utils/test-user-factory.ts index 78ea935c..93a350f9 100644 --- a/tests/e2e/utils/test-user-factory.ts +++ b/tests/e2e/utils/test-user-factory.ts @@ -100,6 +100,13 @@ export interface TestUser { password: string; } +/** + * Email prefix for throwaway admins. Exported so the orphan sweep in + * `auth.setup.ts` matches on the SAME string the seeder uses — a sweep with its + * own copy of the literal silently stops matching the day one of them changes. + */ +export const ISO_ADMIN_PREFIX = 'iso-admin'; + let adminClient: SupabaseClient | null = null; /** @@ -2705,6 +2712,139 @@ export async function openPaymentHubAs( return opened; } +/** + * A throwaway user with `user_profiles.is_admin = true` — for specs that must + * actually RENDER an admin route rather than measure the redirect. + * + * ## Why this exists + * + * No E2E run had ever rendered an admin page. `AdminGate.tsx:81` calls + * `router.push('/')` for an authenticated non-admin, so three separate things + * were measuring the home page or nothing at all: the AAA contrast sweep + * reported six `/admin*` routes as swept while measuring `/` (#454), the admin + * table wells could not be verified (#430), and the landmark gate had to + * exclude those six routes outright. + * + * ## Why one UPDATE is enough + * + * `user_profiles.is_admin` is the SINGLE authority (#240), read live by the + * `is_admin()` SECURITY DEFINER RPC. `custom_access_token_hook` projects it + * into an `app_metadata` JWT claim, but that hook requires an out-of-band + * project-config registration and **no live RLS policy reads the claim** — so + * the column flip takes effect on the EXISTING session immediately, with no + * token refresh. (Older docs describe the claim as the authority; they predate + * #240.) + * + * ## Scope of what this grants + * + * `is_admin()` gates 25 policies across 9 tables — payments, messages, + * conversations, profiles, connections, subscriptions, audit logs and rate + * limits all become readable ACROSS USERS. That is why this is a throwaway per + * test rather than a flag on the shared storage-state user: every existing spec + * would otherwise gain cross-user reads, and row-count assertions would keep + * passing while measuring something else entirely. + * + * ALWAYS tear down with {@link deleteIsolatedAdmin} in a `finally`. A survivor + * is a real account that can read every message in the project; `auth.setup.ts` + * sweeps for orphans, but that is a backstop, not a substitute. + */ +export interface IsolatedAdmin { + user: TestUser; + session: InjectableSession; +} + +/** + * Seed a throwaway admin. Returns null if the admin client is unavailable + * (same contract as {@link seedIsolatedPayment}), and THROWS if the promotion + * did not take — see below. + */ +export async function seedIsolatedAdmin(): Promise { + const admin = getAdminClient(); + if (!admin) return null; + + const stamp = Date.now().toString().slice(-8); + const created = await createKeyedUserWithSession( + ISO_ADMIN_PREFIX, + 'isoadm', + stamp + ); + if (!created) return null; + + const { error } = await admin + .from('user_profiles') + .update({ is_admin: true }) + .eq('id', created.user.id); + + if (error) { + console.warn('seedIsolatedAdmin: promotion failed:', error.message); + await deleteTestUser(created.user.id); + return null; + } + + // VERIFY THE PROMOTION THROUGH THE USER'S OWN SESSION, not the service role. + // + // This is the whole point. A silently-unpromoted user still renders — it just + // renders the redirect — so every downstream assertion would measure the home + // page and pass. That is precisely the defect #454 describes, and without + // this check the fixture built to fix it would reproduce it. The service-role + // client cannot answer the question: it bypasses RLS and would say "true" + // regardless of what the user's own token can see. + const supabaseUrl = + process.env.SUPABASE_ADMIN_URL || process.env.NEXT_PUBLIC_SUPABASE_URL; + const anonKey = process.env.NEXT_PUBLIC_SUPABASE_ANON_KEY; + if (supabaseUrl && anonKey) { + const asUser = createClient(supabaseUrl, anonKey, { + auth: { persistSession: false, autoRefreshToken: false }, + global: { + headers: { Authorization: `Bearer ${created.session.access_token}` }, + }, + }); + const { data: isAdmin, error: rpcErr } = await asUser.rpc('is_admin', { + check_user_id: created.user.id, + }); + if (rpcErr || isAdmin !== true) { + await deleteTestUser(created.user.id); + throw new Error( + `seedIsolatedAdmin: is_admin() returned ${JSON.stringify(isAdmin)} for ` + + `${created.user.email} through the user's own session` + + (rpcErr ? ` (${rpcErr.message})` : '') + + '. Refusing to hand back a fixture that would silently measure the ' + + 'redirect instead of the admin page.' + ); + } + } + + console.log(`✓ Isolated admin ${created.user.id}`); + return { user: created.user, session: created.session }; +} + +/** Tear down a throwaway admin. Safe with null. */ +export async function deleteIsolatedAdmin( + fixture: IsolatedAdmin | null +): Promise { + if (!fixture) return; + await deleteTestUser(fixture.user.id); + console.log('✓ Isolated admin torn down'); +} + +/** + * Open a fresh browser context as a throwaway admin, landing on `route`. + * Mirrors {@link openPaymentHubAs}. + */ +export async function openAdminAs( + browser: Browser, + session: InjectableSession, + route = '/admin' +): Promise { + const basePath = process.env.NEXT_PUBLIC_BASE_PATH || ''; + const opened = await openAuthedPage(browser, session); + await opened.page.goto(`${basePath}${route}`, { + waitUntil: 'domcontentloaded', + }); + await dismissCookieBanner(opened.page); + return opened; +} + /** * Open a fresh browser context authenticated as the isolated subscription's * user, landing on the payment hub's Subscriptions tab From 1f5f53eb1450bd7855b267c4480a188800026b7d Mon Sep 17 00:00:00 2001 From: TurtleWolfe Date: Mon, 3 Aug 2026 22:50:33 +0000 Subject: [PATCH 3/3] fix(#430): the shared admin table was never welled, and the spec could not see it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CI found both defects on the first run that could reach an admin page as an admin — which is the entire argument for #454's fixture. 1. #430's fix wrapped the two HAND-ROLLED scrollers in AdminPaymentPanel and AdminMessagingOverview and missed `AdminDataTable`, the shared component several admin surfaces render. It only appears when there is data, so a local project with no conversations never rendered it. CI's shared project has data, and the spec reported it by name: /admin/messaging: 1 scroller(s) are not wrapped in .sh-well unwelled parent: "flex flex-col gap-3 mt-8" Wrapping it in AdminDataTable fixes every consumer at once. 2. The spec itself was timing-dependent, and that is why CI called it FLAKY rather than failed. `page.evaluate` does not retry the way a web-first assertion does (#396, instance 5) — it snapshots the DOM once. The admin panels render a `loading-spinner` until their RPCs return, so counting too early sees zero scrollers and passes on nothing. Measured: a 3.5s settle read 0/0 on /admin/payments, which had read 1/1 moments earlier. It now waits for `.loading-spinner` to reach count 0 — a signal, not a sleep — and re-reads through `expect.poll`. /admin/messaging goes from 0/0 to 1/1 locally as a result: the scroller was always there, the spec just measured before it existed. The flake was the tell. A test that passes when the data is absent and fails when it is present is not flaky; it is a defect plus a vacuous pass, which is the shape #495 wore. No mutation test needed for the AdminDataTable fix: CI already ran it against the original defect and failed by name. Co-Authored-By: Claude Opus 5 (1M context) --- .../AdminDataTable/AdminDataTable.tsx | 219 ++++++++++-------- tests/e2e/admin/admin-depth.spec.ts | 47 +++- 2 files changed, 155 insertions(+), 111 deletions(-) diff --git a/src/components/molecular/AdminDataTable/AdminDataTable.tsx b/src/components/molecular/AdminDataTable/AdminDataTable.tsx index 143223c6..7f4cdbba 100644 --- a/src/components/molecular/AdminDataTable/AdminDataTable.tsx +++ b/src/components/molecular/AdminDataTable/AdminDataTable.tsx @@ -133,115 +133,132 @@ export function AdminDataTable>({ }; return ( -
- - - - {renderExpandedRow && ( - - )} - {columns.map((col) => ( - - ))} - - - - {sortedData.map((row, rowIndex) => { - const rowKey = String(row[keyField] ?? rowIndex); - const isExpanded = expandedKey === rowKey; - - if (!renderExpandedRow) { - return ( - - {columns.map((col) => ( - - ))} - - ); - } - - // Row-click is the mouse affordance. The button is the - // a11y-correct trigger — aria-expanded is only valid on - // button/link roles outside treegrid (axe aria-conditional-attr). - return ( - - toggleExpanded(rowKey)} + // WRAPPED in `sh-well`, not welled in place (#430). `sh-well` paints an + // inset shadow BELOW its children, so putting it on the `overflow-x-auto` + // div clips the shadow inside the scroll area and hides it under the table. + // + // #430 wrapped the two hand-rolled scrollers in AdminPaymentPanel and + // AdminMessagingOverview and missed THIS one — the shared component several + // admin surfaces render. It went unseen because it only appears when there + // is data: locally /admin/messaging had no conversations, so the table + // never rendered and the check passed on nothing. CI's shared project has + // data, so the first run that could reach an admin page as an admin found + // it immediately (#454's fixture). +
+
+
- Toggle details - - {col.sortable ? ( - // Native where ARIA puts it; the - // arrow glyph is visual-only (aria-sort already says it). - - ) : ( - col.label - )} -
- {col.render - ? col.render(row) - : String(row[col.key] ?? '')} -
+ + + {renderExpandedRow && ( + + )} + {columns.map((col) => ( + - ))} - - {isExpanded && ( - - + + + {sortedData.map((row, rowIndex) => { + const rowKey = String(row[keyField] ?? rowIndex); + const isExpanded = expandedKey === rowKey; + + if (!renderExpandedRow) { + return ( + + {columns.map((col) => ( + + ))} + + ); + } + + // Row-click is the mouse affordance. The button is the + // a11y-correct trigger — aria-expanded is only valid on + // button/link roles outside treegrid (axe aria-conditional-attr). + return ( + + toggleExpanded(rowKey)} + > + + {columns.map((col) => ( + + ))} - )} - - ); - })} - -
+ Toggle details + - + {col.sortable ? ( + // Native where ARIA puts it; the + // arrow glyph is visual-only (aria-sort already says it). - - {columns.map((col) => ( - - {col.render - ? col.render(row) - : String(row[col.key] ?? '')} -
- {renderExpandedRow(row)} + ) : ( + col.label + )} + + ))} +
+ {col.render + ? col.render(row) + : String(row[col.key] ?? '')} +
+ + {col.render + ? col.render(row) + : String(row[col.key] ?? '')} +
+ {isExpanded && ( + + + {renderExpandedRow(row)} + + + )} + + ); + })} + + +
); } diff --git a/tests/e2e/admin/admin-depth.spec.ts b/tests/e2e/admin/admin-depth.spec.ts index f33bf30f..fdce9770 100644 --- a/tests/e2e/admin/admin-depth.spec.ts +++ b/tests/e2e/admin/admin-depth.spec.ts @@ -90,20 +90,47 @@ test.describe('Admin console, rendered as an admin', () => { await assertLandedOnAdmin(page, route); + // WAIT FOR THE DATA, don't sleep at it. The admin panels render a + // `loading loading-spinner` until their RPCs return, and the tables do + // not exist before that. `page.evaluate` does NOT retry the way a + // web-first assertion does (#396, instance 5) — it snapshots the DOM + // once — so counting too early reports 0 scrollers and passes on + // nothing. That is exactly what happened: a 3.5s settle measured 0/0 on + // /admin/payments, which had measured 1/1 moments earlier. + await expect( + page.locator('.loading-spinner'), + `${route}: still loading after 30s — the count below would be taken ` + + `before the tables exist` + ).toHaveCount(0, { timeout: 30000 }); + // #430: the scroller is WRAPPED, not replaced — `sh-well` paints an // inset shadow BELOW its children, so on the `overflow-x-auto` div // itself the shadow is clipped inside the scroll area and hidden under // the table. - const counts = await page.evaluate(() => { - const scrollers = [...document.querySelectorAll('.overflow-x-auto')]; - return { - total: scrollers.length, - welled: scrollers.filter((el) => !!el.closest('.sh-well')).length, - unwelled: scrollers - .filter((el) => !el.closest('.sh-well')) - .map((el) => (el.parentElement?.className || '').slice(0, 60)), - }; - }); + const readScrollers = () => + page.evaluate(() => { + const scrollers = [ + ...document.querySelectorAll('.overflow-x-auto'), + ]; + return { + total: scrollers.length, + welled: scrollers.filter((el) => !!el.closest('.sh-well')).length, + unwelled: scrollers + .filter((el) => !el.closest('.sh-well')) + .map((el) => (el.parentElement?.className || '').slice(0, 60)), + }; + }); + + // Retry the READ as well, so a table that paints a frame after the + // spinner clears cannot be missed. + await expect + .poll(async () => (await readScrollers()).unwelled.length, { + timeout: 15000, + message: `${route}: waiting for every scroller to be welled`, + }) + .toBe(0); + + const counts = await readScrollers(); // Printed every run: a route measuring zero scrollers must be visibly // different from one measuring three.