test(server): tolerate transient DB restart during test teardown

[rongxin-liu]

Reviewable diff: +72/-25 across 1 file (excludes generated, test, and story files).

Summary

The server test suite intermittently fails Server Checks / Test when the TimescaleDB test container briefly crashes and auto-restarts under heavy concurrent-migration load (SQLSTATE 57P03, "database system is in recovery mode"). The per-test-database setup path already retries across this blip; this change extends the same resilience to the t.Cleanup teardown, which previously dropped each test database on a single unguarded admin connection and failed any test whose DROP DATABASE happened to land in the ~1–2s recovery window — even though the test's own assertions had passed. One such restart cascades into several unrelated "error dropping test database" failures across whatever tests are tearing down at that moment.

Observed on PR #658's run: TestCollectionStore_ListCollections_SortsByIssueCount and TestServeHTTP_SelfMonitoringNoActiveOrgsFallsBackToUnscoped both failed only at teardown, in the same 2-second window, with 57P03.

How it works

Every DB-backed test gets its own database via testutil.GetTestDB, which creates the database, migrates it, and registers a t.Cleanup that drops it. The teardown now routes through dropTestDatabase, which mirrors the existing createTestDatabase retry loop:

• Attempt terminate-connections + DROP DATABASE on a fresh admin connection.
• On a transient server error (class 57 / bad connection / "starting up" / "in recovery mode", via the existing isTransientServerError), wait for the server to accept queries again (waitForServerReady) and retry with backoff.
• Non-transient errors fail immediately, as before.

The happy path keeps teardown at a single admin connection — the readiness ping only runs before a retry, so normal teardowns add no extra connections (which would only worsen the very load that triggers the crash). waitForServerReady now takes an explicit context.Context because cleanup functions must use context.Background(): t.Context() is canceled before t.Cleanup runs.

Diagram

flowchart TD
    A["t.Cleanup fires"] --> B["conn.Close()"]
    B --> C["dropTestDatabase"]
    C --> D["tryDropTestDatabase: terminate + DROP DATABASE"]
    D -->|"success"| E["done"]
    D -->|"transient error (57P03 etc.)"| F{"attempts left?"}
    F -->|"yes"| G["waitForServerReady + backoff"]
    G --> D
    F -->|"no"| H["assert.NoError fails"]
    D -->|"non-transient error"| H

Loading

Areas of the code involved

Area / file	What changed	Why it matters for review
server/internal/testutil/database_setup.go	Added dropTestDatabase / tryDropTestDatabase; t.Cleanup now retries the DROP across transient restarts; waitForServerReady gained an explicit ctx param	Shared harness used by every DB test — confirm the happy path still drops exactly once and retries only on transient errors

Key technical decisions & trade-offs

• Reuse over reinvent: teardown leans on the same isTransientServerError / waitForServerReady machinery as setup, keeping the two paths symmetric — chosen over a bespoke teardown retry.
• Readiness ping only before a retry (not up-front like createTestDatabase): avoids doubling admin connections on every teardown, which would add connection pressure to the exact scenario that provokes the crash.
• context.Background() in cleanup (with a scoped //nolint:usetesting): required because t.Context() is already canceled once cleanup runs.

Testing & validation

• go build, go vet, and golangci-lint run on ./internal/testutil/... all pass.
• This failure is a timing-dependent flake (a test's DROP coinciding with a container restart), so it is not deterministically reproducible locally; correctness rests on teardown now sharing the setup path's already-proven retry behavior. Not covered: forcing an actual mid-teardown crash in a unit test.

[github-actions]

🔐 Codex Security Review

Note: This is an automated security-focused code review generated by Codex.
It should be used as a supplementary check alongside human review.
False positives are possible - use your judgment.

Scope summary

• Reviewed pull request diff only (2e71805b189e778fa50a62ca8373eabe76795186...3e332415a5b560560c7627cee607abfce42e8a82, exact PR three-dot diff)
• Model: gpt-5.5

💡 Click "edited" above to see previous reviews for this PR.

---

Review Summary

Overall Risk: NONE

Findings

No reportable security, correctness, or reliability findings in the scoped diff.

Notes

• Reviewed only .git/codex-review.diff; it changes server/internal/testutil/database_setup.go test database setup/teardown behavior.
• No production auth, RPC, plugin, discovery, frontend, infrastructure, pool configuration, protobuf, or miner command paths changed.
• I did not run tests; this was a static diff review in the read-only environment.

---

_{Generated by Codex Security Review |
Triggered by: @rongxin-liu |
Review workflow run}

[Copilot]

Pull request overview

This PR hardens the Go server test DB harness (server/internal/testutil) to reduce CI flakes when the TimescaleDB container transiently restarts during highly concurrent migrations, by making per-test database teardown (t.Cleanup) retry DROP DATABASE across transient “server not ready” windows.

Changes:

• Route t.Cleanup teardown through a new dropTestDatabase / tryDropTestDatabase path with transient-retry + backoff.
• Generalize waitForServerReady to accept an explicit context.Context (setup uses t.Context(), teardown uses context.Background()).
• Keep the “happy path” to a single admin connection, only pinging readiness before retries.