Skip to content

test(admin): extract AdminGate, pin safety properties via regression test#56

Merged
TortoiseWolfe merged 1 commit intomainfrom
051/admingate-regression-test
Apr 26, 2026
Merged

test(admin): extract AdminGate, pin safety properties via regression test#56
TortoiseWolfe merged 1 commit intomainfrom
051/admingate-regression-test

Conversation

@TortoiseWolfe
Copy link
Copy Markdown
Owner

Summary

Refs #51. The auth-race regression that's been reverted three times in ProtectedRoute (commits 6b4c13a, 2c97e67, 259b38d) survives via a defensive shape: a wasX.current ref that debounces transient flips, a cancelled flag in async effects, and a [user, ...] dep array that re-runs the check on identity change.

AdminGate (the admin RPC check that layers inside ProtectedRoute) implements the same defensive shape, but with zero test coverage. A future refactor could silently strip any of these defenses and CI would say "all green" — exactly how the prior regressions kept landing in the first place.

This PR pins those defenses as contracts. No behavior change; the bug shape that #51's body described as "latent stale closure" was actually already defended in code. The real gap was test coverage for that defense.

What changed

  1. Extract AdminGate from src/app/admin/layout.tsx into src/app/admin/AdminGate.tsx so it's unit-testable in isolation. layout.tsx now just composes <ProtectedRoute><AdminGate>{children}</AdminGate></ProtectedRoute>.

  2. Document the safety properties as a top-of-file comment naming each load-bearing ref/flag/dep so the next reader understands what must not be removed.

  3. Add src/app/admin/AdminGate.test.tsx with 10 regression cases pinning each property:

    # Property under test
    1 Loading spinner before authLoading resolves
    2 Loading spinner while admin check is in flight
    3 Children render on admin=true
    4 router.push('/') when admin=false on first mount (and never-was)
    5 wasAdmin.current debounce: no redirect on transient admin→non-admin flip
    6 Dep-array integrity: user.id change re-runs checkIsAdmin
    7 cancelled flag: no setState after unmount mid-flight
    8 Renders nothing (not the nav) when admin=false and never-was
    9 No admin check while authLoading is true
    10 No admin check when user is null

Out of scope (intentionally)

#51's original plan listed three more steps: pin user.id via a parallel userRef, hoist into a shared useAuthGate hook, and sweep the codebase for the same shape. After reading the code carefully, the first looks like premature optimization (the existing cancelled flag + dep array catches the failure mode the userRef would defend against). The second is a real DRY opportunity but warrants its own focused refactor PR. The third is a sweep that should run on a clean baseline — i.e., after this PR lands.

If the regression cases above ever fail in CI, that's the empirical justification for steps 2–4. Until then: YAGNI.

Test plan

  • pnpm vitest run src/app/admin/AdminGate.test.tsx — 10/10 pass in 62ms
  • pnpm vitest run src/app/admin src/components/auth tests/unit/services/admin tests/unit/auth — 36 files, 209 tests, all pass (including ProtectedRoute and AdminAuthService dependents)
  • pnpm run type-check — clean
  • pnpm exec eslint src/app/admin/* — clean

🤖 Generated with Claude Code

…test

The auth-race regressions in commits 6b4c13a, 2c97e67, 259b38d kept landing
because the safety properties they violated had no test coverage. The same
shape exists in AdminGate (wasAdmin.current debounce + cancelled flag +
[user, authLoading, router] dep array) — defended in code, but a future
refactor could silently strip any of them and the bug would return without
CI catching it.

This commit moves AdminGate out of admin/layout.tsx into its own file so
it can be unit-tested in isolation, then adds 10 regression cases that
pin each safety property as a contract:

  - Loading spinner before authLoading resolves
  - Loading spinner while admin check is in flight
  - Children render on admin=true
  - router.push('/') when admin=false on first mount (and never-was)
  - wasAdmin.current debounce: no redirect on transient admin→non-admin
    flip after the user was already admin
  - Dep-array integrity: user.id change re-runs checkIsAdmin
  - cancelled flag: no setState after unmount mid-flight
  - Renders nothing (not the nav) when admin=false and never-was
  - No admin check started while authLoading is true
  - No admin check started when user is null

Refs #51.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@TortoiseWolfe TortoiseWolfe merged commit 00edd23 into main Apr 26, 2026
6 checks passed
@TortoiseWolfe TortoiseWolfe deleted the 051/admingate-regression-test branch April 26, 2026 23:41
TortoiseWolfe added a commit that referenced this pull request Apr 27, 2026
…andoff (#61)

Captures end-of-session state after 6 PRs landed (#54, #55, #56, #58,
#59, #60). Family A is empty (both stability hotspots resolved).
Family D1 done. Recommended next pickup: B1 (#43 /payment/result page).
The handoff doc is the load-bearing artifact for the next operator —
it lists open issues by family, sharp edges, and a copy-pasteable
quick-start.

Co-authored-by: TurtleWolfe <TurtleWolfe@users.noreply.github.com>
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants