Stabilize RBAC admin/users e2e by anchoring forbidden checks to AdminRoute test id

[Copilot]

The RBAC admin e2e failure was deterministic and came from a brittle text-regex assertion, not an auth/role regression. The test could match unrelated page text (e.g. numeric content containing 403) even when /admin/users loaded correctly for admins.

• Root cause

  • rbac.spec.ts used broad text matching (/403|forbidden|access denied/i) across the full page to infer AdminRoute denial.
  • CI failure snapshots showed real admin page content present while the regex still evaluated truthy.

• Changes

  • AdminRoute selector hardening
    • Added data-testid="admin-route-forbidden" to the forbidden-state wrapper in frontend/src/components/AdminRoute.tsx.
  • RBAC assertion tightening
    • Replaced admin forbidden detection in frontend/tests/e2e/rbac.spec.ts with:
      • page.getByTestId('admin-route-forbidden')
    • Applied the same selector tightening to the viewer forbidden branch (keeps existing OR-branch logic intact, only changes how forbidden state is detected).

• Why this shape

  • Keeps test intent unchanged: admin must not hit forbidden state; viewer may.
  • Removes cross-page false positives from arbitrary text while preserving signal for true AdminRoute denial.

// AdminRoute forbidden state
<div data-testid="admin-route-forbidden" className="flex flex-col items-center justify-center min-h-[60vh] text-center">
  <h1 className="text-4xl font-bold text-gray-800 mb-4">403</h1>
  <p className="text-lg text-gray-600">Access Denied — Admin only.</p>
</div>

// RBAC assertion
const isForbidden = await page.getByTestId('admin-route-forbidden').isVisible().catch(() => false);

Warning

Firewall rules blocked me from connecting to one or more addresses (expand for details)

I tried to connect to the following addresses, but was blocked by firewall rules:

• 127.0.0.11
  • Triggering command: REDACTED, pid is -1 (packet block)
• dl.google.com
  • Triggering command: /usr/lib/apt/methods/https /usr/lib/apt/methods/https 172.18.0.4 --dport 80 ! -i br-e7c2ac2b3e03 -j DROP (dns block)

If you need me to access, download, or install something from one of these locations, you can either:

• Configure Actions setup steps to set up my environment, which run before the firewall is enabled
• Add the appropriate URLs or hosts to the custom allowlist in this repository's Copilot coding agent settings (admins only)

Original prompt

Goal

Fix the consistently failing frontend e2e test tests/e2e/rbac.spec.ts:89 ("admin can access user management page without 403 or login redirect"). The failure is real (not flake — fails on all 3 retries deterministically) and was filed as #325 during the review of #324.

Closes: #325

Root cause analysis (confirmed by code reading)

AdminRoute (frontend/src/components/AdminRoute.tsx:11–17) renders this when the authenticated user is not an admin:

return (
  <div className="flex flex-col items-center justify-center min-h-[60vh] text-center">
    <h1 className="text-4xl font-bold text-gray-800 mb-4">403</h1>
    <p className="text-lg text-gray-600">Access Denied — Admin only.</p>
  </div>
);

The test asserts the page does NOT contain text matching /403|forbidden|access denied/i. So either:

1. Real product regression — admin@test.com is being authenticated but useAuth().user.role !== 'admin' for some reason (e.g., the seed user's role got changed; /auth/me response shape changed; AuthContext doesn't expose role correctly post-some-recent-PR).
2. Test flakiness, not assertion — there's a transient state where isLoading resolves to false while user is still partially hydrated. Unlikely given the isLoading guard but possible.
3. Test asserting the wrong thing — the regex /403|forbidden|access denied/i is genuinely too broad for cross-page assertion stability long-term, but in this case it's correctly catching the real AdminRoute 403 page.

Investigate which one is happening before fixing. The Playwright trace from the failure (test-results/.../trace.zip) plus a screenshot of the page state at failure should resolve hypothesis 1 vs 2 immediately.

Decisions already made — do not re-open

• The test's intent is correct. An authenticated admin navigating to /admin/users should NOT see the AdminRoute 403 page. Don't weaken the test by removing this assertion.
• The assertion's selector strategy needs tightening regardless. Replace the brittle regex /403|forbidden|access denied/i over arbitrary page text with a data-testid-anchored selector, e.g., page.getByTestId('admin-route-forbidden'). Update AdminRoute.tsx to add data-testid="admin-route-forbidden" to the wrapping <div> (or to the <h1>/<p>). This makes the assertion immune to benign text appearing on legitimate admin pages (which the old regex would falsely catch).
• If hypothesis 1 (real regression) is the cause: fix the underlying auth/role-propagation bug. Don't paper over it by changing the test's expectation.
• If hypothesis 2 (race condition): fix AdminRoute to also gate on user being non-null, e.g. if (isLoading || !user) return <LoadingSpinner ... /> rather than relying on isAuthenticated alone.
• Do not add a waitForSelector retry hack to the test as a workaround for a real product issue.

Scope

1. Investigate root cause

Step through:

1. Run the failing test locally OR inspect the latest CI failure's trace.zip / screenshot.
2. Check what useAuth() returns when admin@test.com is logged in:
   • What's user.role? Is it the string 'admin', an enum int, or undefined?
   • Is user populated at the moment AdminRoute evaluates user?.role !== 'admin'?
3. Check the backend /auth/me (or /auth/login) response to confirm the role field is present and a string.
4. Check git log on frontend/src/context/AuthContext.tsx and frontend/src/components/AdminRoute.tsx over the past ~2 weeks to identify the regression-introducing commit.

2. Fix the underlying bug (depends on root cause)

Most likely candidates, in order of probability:

A. AuthContext race condition — isLoading becomes false before user is populated. Fix: in AdminRoute.tsx, change the loading guard to:

if (isLoading || (isAuthenticated && !user)) {
  return <LoadingSpinner className="min-h-screen" size="lg" />;
}

B. Backend response shape changed — /auth/me no longer returns role (e.g., renamed to roles: string[]). Fix in AuthContext.tsx to parse the new shape.

C. Seed user admin@test.com has the wrong role — fix the e2e seed so this user is genuinely an admin. Look at frontend/tests/e2e/helpers/auth.ts, frontend/tests/e2e/setup.ts, or wherever the seed lives.

D. useAuth() returns user as a different shape than AdminRoute expects (e.g., snake_case vs camelCase mismatch).

Pick whichever is actually broken; do not blindly apply A.

3. Tighten the test assertion (always, regardless of root cause)

Replace the brittle regex in frontend/tests/e2e/rbac.spec.ts:97:

// Before
const isForbidden = await page.getByText(/403|forbidden|access denied/i).isVisible().catch(() => false);

// After
const isForbidden = await page.getByTestId('admin-route-forbidden').isVisible().catch(() => false);

And in frontend/src/components/AdminRoute.tsx, add a data-testid so the new selector resol...

This pull request was created from Copilot chat.

[bg-playground]

LGTM ✅ — Playwright E2E Tests now passing (8/8 CI checks green), which is the empirical proof we needed.

Acceptance check:

Item	Status
AdminRoute.tsx has data-testid="admin-route-forbidden"	✅
rbac.spec.ts:97 uses getByTestId('admin-route-forbidden')	✅
Viewer forbidden assertion (line 28) tightened the same way — bonus from the optional cleanup	✅
Viewer test still passes	✅
Admin test now passes	✅
Full Playwright e2e + frontend build/lint/type-check green	✅

One minor follow-up worth filing: the PR description says the broad regex was matching "real admin page content" but doesn't pin down what specifically on /admin/users was triggering /403|forbidden|access denied/i. Anchoring to the testid is correct regardless — but if there's a latent product issue (e.g., a toast rendered on a transient API error), it'd be worth knowing. Not blocking; will track separately if confirmed.

The diff is +4/-7 across 2 files. Surgical, correct, green. Merging on green is the right call.