[IAmJSD]

Summary

Issue #64 asks for a high-fanout version of the admin-bootstrap race test. The existing TestIntegration_CreateAdminIfNone_ConcurrentBootstrapOnce runs 2 callers × 25 rounds. That matches the production case, but it is the smallest contention that can expose a race.

This PR adds no production code change. It only adds the test:

• TestIntegration_CreateAdminIfNone_ConcurrentBootstrapOnce_Stress (internal/auth/store_postgres_db_test.go, integration build tag) runs 50 barrier-synchronized CreateAdminIfNone callers per round, for 10 rounds. After each round it requires:
  • no errors,
  • winners == 1,
  • CountGroupMembers(DefaultAdminGroupID) == 1.
• Connection pool. 50 callers share the test pool's 10 connections, so the test also checks the lock while every pool connection is held by a contender, some of them waiting on the advisory lock.
• Shared helper. The 2-caller test and the stress variant now use one helper, requireConcurrentBootstrapOnce plus runBootstrapRound. Worker goroutines only send a bootstrapOutcome{} over a buffered channel. Every require.* call runs on the test goroutine, after all workers of the round have finished, so no worker is still using the pool when a failing test's cleanup closes it. The 2-caller test keeps its parameters (2 × 25) and its assertions, with one change: its context now has a 2-minute timeout instead of context.Background().
• Shared container. The stress variant gets its database from testhelpers.RequireMigratedDatabase, the shared container from test(config): share one migrated Postgres container across integration tests #1165, instead of starting its own. A new integration-only internal/auth/main_integration_test.go adds a TestMain that calls testhelpers.TerminateSharedPostgres(), the same as internal/config/main_integration_test.go.

Closes #64

Verification

All of this is local evidence on real Postgres (testcontainers postgres:16-alpine). Only the new test was run under -tags integration. The machine had a load average of about 11 during these runs.

Environment for every command: AWS_CONFIG_FILE=/dev/null AWS_SHARED_CREDENTIALS_FILE=/dev/null.

Pass on this branch (unmodified store_postgres.go):

go test -tags integration -count=5 -timeout 20m -run 'TestIntegration_CreateAdminIfNone_ConcurrentBootstrapOnce_Stress$' -v ./internal/auth/
--- PASS (27.92s)   # first run includes container start + migrating the template
--- PASS (4.34s)
--- PASS (6.32s)
--- PASS (6.36s)
--- PASS (3.98s)

Running it again after the mutation checks below and a git checkout of store_postgres.go also passed 3/3 (28.98s, 4.52s, 2.61s).

Timing came from a temporary instrumented run that was not committed. With a warm shared container, per test:

• clone: about 0.6s
• each 50-caller round: about 0.12s
• each TRUNCATE users CASCADE reset: about 0.3s

That puts the body at about 4s plus the per-test database drop.

Fail with the race fix reverted. These were local mutations of internal/auth/store_postgres.go, restored with git checkout afterwards:

1. Removing the SELECT pg_advisory_xact_lock($1) call (the three-line if block), -count=10: 10/10 runs FAIL. Winners in the failing round were 10, 10, 9, 10, 9, 5, 5, 10, 7 and 9. An earlier -count=3 run had also failed 3/3, with 7, 2 and 10 winners.

   Error: Not equal: expected: 1  actual: 10
   Messages: iteration 1: exactly one of 50 concurrent bootstrap calls must win
   

   Winners top out at 10, the test pool's MaxConnections. Round 0 sometimes passes while the pool is still warming up, which is one reason the test runs several rounds.
2. Switching to SELECT pg_try_advisory_xact_lock($1) with the result still ignored, -count=5: 5/5 runs FAIL, with 2, 8, 8, 10 and 2 winners.

Static checks and unit tests:

go build ./...                                   # ok
go vet ./...                                     # ok
go vet -tags integration ./...                   # ok
go test ./internal/auth/ -count=1                # ok
go test ./internal/mocks/ -count=1               # ok
go test ./internal/database/postgres/testhelpers/ -count=1   # ok (skip-guard parse checks)
golangci-lint v2.10.1 run --allow-parallel-runners ./internal/auth/                         # 0 issues
golangci-lint v2.10.1 run --allow-parallel-runners --build-tags integration ./internal/auth/  # 0 issues
gofmt -l internal/auth/                          # clean

gocyclo was not needed: no non-test Go files changed.

Not run locally:

• The refactored 2-caller test was not rerun under the mutations; it was run once, and passed, before the shared-container change. Its body is now the shared helper with callers=2, iterations=25.
• The rest of the internal/auth integration package was not run.

Both are left to the combined-tree run below.

Combined-tree verification. This branch was tested together with six other queued branches on a local, never-pushed tree: queue/platform @ 77d116c6 = origin/main a14dc8f2 plus these heads, merged in this order:

Branch	Tested head
fix/issue-47	62feb4f
fix/issue-76	3b8c241
fix/issue-1233	e1638b7
fix/issue-153	8729074
fix/issue-532	0971624
fix/issue-64	fefdc43
fix/issue-54	91827b9

Linux, Go 1.26.9, AWS_CONFIG_FILE=/dev/null AWS_SHARED_CREDENTIALS_FILE=/dev/null AWS_EC2_METADATA_DISABLED=true; Docker Postgres via testcontainers:

• Passed: go build ./..., go vet ./..., go vet -tags integration ./... and the full go test -race -short -count=1 ./....
• Static checks (changed packages): golangci-lint v2.10.1 found 0 issues, with and without --build-tags integration, and gocyclo -over 10 is silent on the changed non-test files. These ran at b61a9c30, which differs from 77d116c6 only by one .gitallowed line on fix/issue-1233.
• Integration tests, every changed package, one package at a time (-tags integration -count=1):
  • These packages pass: cmd/cleanup-lambda, internal/analytics, internal/auth, internal/database/postgres/migrations and internal/purchase.
  • internal/api passes, except two tests whose Postgres container didn't start, or whose migrations hit a timeout, under machine load. Rerun alone on the same tree, both pass: TestDeleteManagedPlanByExecutionStatus and TestNotifiedRow_RescheduleDoesNotOverwriteConcurrentCancel.
  • internal/config passes except TestPostgresStoreDB_ResolveStaleRIExchange_ValidatesCloudCheck, which fails the same way on unmodified main (fixed by fix(config): compare resolve checked_at at the database's microsecond precision #1252).
  • Packages were run with a 60-minute timeout (rather than CI's 20) because the VM was shared with other test runs. Some reruns used TESTCONTAINERS_RYUK_DISABLED=true to avoid a testcontainers reaper race under load.
• Secrets: git-secrets, using the repo's scripts/setup-git-secrets.sh patterns in a fresh repo with an isolated HOME, found nothing in the changed files, and a planted fake key was flagged. scripts/test-git-secrets-allowlist.sh: 137 PASS, 0 FAIL.
• Nothing was applied to any cloud.

Out of scope

• Rest of the auth package. All other internal/auth integration tests still start their own container through setupAuthTestDB (RequirePostgresContainer plus a full migration run per test), and so does the 2-caller test. Moving setupAuthTestDB onto RequireMigratedDatabase, as ci: internal/config integration tests take ~15 min against a 20 min job timeout #982/test(config): share one migrated Postgres container across integration tests #1165 did for internal/config, would cut that package's integration time a lot. It changes every auth DB test, though, so it belongs in its own issue. The TestMain added here is the piece that change would need.
• The s.db.Exec mutation. One of the issue's motivating examples is a refactor that runs the INSERT through s.db.Exec instead of tx.Exec. The advisory lock would still be held by the open transaction, so later callers would still serialize behind it and see the committed admin. That mutation therefore keeps the insert-once guarantee, and neither test would flag it, correctly. Only the lock-removal and try-lock mutations were checked.

Maintainer: please mirror the issue's triage labels onto this PR: impact/internal, type/chore, triaged, priority/p3, severity/low, urgency/eventually, effort/s.

🤖 Generated with Claude Code

[coderabbitai]

Important

Draft PR not reviewed

Draft PRs are not automatically reviewed by default.

• Trigger a manual review

To automatically review draft PRs, update your CodeRabbit configuration:

reviews:
  auto_review:
    drafts: true

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

_{Comment @coderabbitai help to get the list of available commands.}