fix(rls): cleanup-stale globalSetup so killed prior runs don't wedge pnpm test:rls (#50)#60
Merged
TortoiseWolfe merged 5 commits intomainfrom Apr 27, 2026
Merged
Conversation
Pure async function cleanupStaleScripthammerUsers walks the FK chain (payment_intents → subscriptions → user_profiles → auth.deleteUser) for every *@scripthammer.test user. Best-effort: errors are logged and cleanup continues to the next step. Returns a summary count. Tests pin the contract: - Chain runs in correct order per user. - Non-matching emails are never touched (production safety). - Best-effort: a transient delete error doesn't halt the rest. - No-op when no scripthammer.test users exist. Refs #50. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Constructs a Supabase service client from env, calls cleanupStaleScripthammerUsers, logs the summary. Skips silently when SUPABASE_SERVICE_ROLE_KEY is missing — same gate the tests already use via hasRlsTestEnvironment(). Wired into vitest.rls.config.ts in the next commit. Refs #50. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
vitest.rls.config.ts now invokes cleanup-stale.ts as globalSetup, so
killed prior runs that left orphan FK rows referencing
*@scripthammer.test users get scrubbed before tests collect.
While verifying empirically against cloud, the original CleanupSummary
shape (per-table 'removed' counts) turned out to be misleading:
PostgREST DELETE succeeds regardless of row-match, so the summary
reported "1 intent removed" for a stale user that had no payment
intents. Replaced the per-table counts with two honest fields:
- usersRemoved: count from auth.admin.deleteUser successes (the
only DELETE whose success/failure carries real signal)
- errorsLogged: count of per-table errors logged (non-fatal;
cleanup continues)
Manual verification on cloud Supabase:
- Inject a stale test-cleanup-probe@scripthammer.test user via
auth admin API.
- pnpm test:rls
- Output includes: [rls cleanup-stale] removed 1 user(s); 0 error(s) logged
- Suite proceeds to 55/55 (after a brief auth admin rate-limit
pause — known constraint of running cleanup + suite back-to-back
on Supabase Cloud's free-tier auth admin quota).
Refs #50.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Captures the FK-chain order, scope-narrowing rationale (why this doesn't unify with #57 even though I proposed that yesterday), and the bite-sized task plan that produced the runtime commits. Future operators can reconstruct the design intent without re-deriving from the PR description. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The implementation settled on { usersRemoved, errorsLogged } rather
than the per-table 'removed' fields the spec originally proposed,
because PostgREST DELETE returns success regardless of row-match
(would have given misleading counts). Updated the spec's verification
example + added a note explaining the change.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
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>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #50.
Summary
Adds a Vitest
globalSetuptopnpm test:rlsthat scrubs orphan*@scripthammer.testusers and their FK-blocking dependent rows (payment_intents,subscriptions,user_profiles) before any RLS test file collects. The fixture'screateTestUserretry path remains as the second line of defense; this hook just prevents the FK-block that defeated it on 2026-04-26 (and required manual psql cleanup that day).Why this matters
pnpm test:rlsagainst cloud Supabase fails onbeforeAllif a prior run died mid-test and left orphan rows. The fixture's existing recovery path (tests/fixtures/test-users.ts:171-201) tries delete-then-recreate, butauth.admin.deleteUserhitspayment_intents_template_user_id_fkeyand gives up. The next operator has to manually grep for orphan FKs, delete dependent rows in the right order, then delete the user. We saw this concretely yesterday (2026-04-26) when residue from a 2026-04-16 run wedged the suite.After this PR: the globalSetup walks the FK chain (
payment_intents → subscriptions → user_profiles → auth.admin.deleteUser) for every*@scripthammer.testuser, before tests collect. Killed runs become recoverable on the next invocation; the operator never has to know the chain.What changed
tests/rls/__setup__/cleanup-stale-impl.ts(new) — pure asynccleanupStaleScripthammerUsers(client, logger?)tests/rls/__setup__/cleanup-stale.ts(new) — globalSetup wrapper; skips silently when env is absenttests/unit/rls-cleanup.test.ts(new) — 4 unit cases pinning FK chain order, production-safety filter, best-effort error handling, no-op on emptyvitest.rls.config.ts— wireglobalSetup: ['./tests/rls/__setup__/cleanup-stale.ts']docs/superpowers/specs/2026-04-27-rls-cleanup-stale-design.md(new)docs/superpowers/plans/2026-04-27-rls-cleanup-stale.md(new)Empirical verification
auth.admin.createUser({email: 'test-cleanup-probe@scripthammer.test', ...})pnpm test:rls[rls cleanup-stale] removed 1 user(s); 0 error(s) loggedOut of scope
TEST_USER_PRIMARY_EMAIL, env-driven), different problem. Spec includes a "Scope clarification" section explaining why I withdrew yesterday's proposal to unify these.fileParallelism: false).Test plan
pnpm vitest run tests/unit/rls-cleanup.test.ts— 4/4 passpnpm test:rlsagainst cloud, no stale users — still 55/55 (silent no-op cleanup)pnpm test:rlsagainst cloud, with injected stale user — cleanup logs the removal, suite proceeds to 55/55pnpm run type-check— clean🤖 Generated with Claude Code