v1.7: Postgres migration + SuperTokens foundation (design) - #6
Draft
NeverEndingCode wants to merge 9 commits into
Draft
v1.7: Postgres migration + SuperTokens foundation (design)#6NeverEndingCode wants to merge 9 commits into
NeverEndingCode wants to merge 9 commits into
Conversation
Spec for moving persistence from SQLite to Postgres (v1.7) and adopting SuperTokens without breaking existing Discord/GitHub logins (v1.8). Decisions settled during brainstorming: dual backend with Postgres as the default, tests against real Postgres in Docker, auto-migrate on boot behind guards, and the identities auth split done up front so v1.8 stays small. Two findings drove the design. SuperTokens supports external user ID mapping, so users.id can stay 'provider:providerId' and no foreign key or SUPER_ADMIN_IDS handling has to change. And SuperTokens' /auth/callback/github is not a subdirectory of the currently registered /auth/github/callback, which would fail GitHub's redirect_uri rule - the registered callback gets widened to /auth so both paths work at once and passport keeps running throughout. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Eight tasks, each ending in an independently testable deliverable: 1. Make the db interface async (SQLite unchanged underneath) - 292 call sites 2. Split db.js into facade + SQLite driver + schema module 3. Postgres test harness: Testcontainers, per-file databases, CI matrix 4. Postgres schema and driver, gated by a cross-dialect parity test 5. The identities auth split, with in-place upgrades for both backends 6. The SQLite to Postgres migrator, verified before COMMIT 7. Auto-migration on boot behind guards 8. Deployment config, Unraid runbook, smoke suites against Postgres Task 1 is deliberately alone: mixing the async refactor with the driver split would make the diff unreviewable. Two landmines found while writing this that the spec had not caught, both silent-corruption class rather than crash class: - pg returns BIGINT as a string by default. Every epoch-ms column here is BIGINT and every consumer does arithmetic on it, so the int8 type parser has to be registered before the first query runs. - Postgres folds unquoted identifiers to lowercase, so listLeaderboard's 'AS userId' would return 'userid' and every rungsClaimed would read undefined on a leaderboard that still rendered. Both are pinned by tests/db.parity.test.js. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Every server/db.js export returns a Promise; all call sites across server/ and tests/ now await. better-sqlite3 remains the implementation, so behaviour is unchanged - this is purely the shape change that lets a pg driver slot in behind the same interface. shared/ is deliberately untouched and stays synchronous: the client bundles it, and an async reducer would be a far larger change than this migration needs. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Pure refactor - no SQL changes. server/db.js is now a re-export shim so every existing import path keeps working, while server/db/ holds the seam the Postgres driver plugs into next. Adds a schema_migrations table: the schema has outgrown what CREATE TABLE IF NOT EXISTS plus guarded ALTERs can honestly express, and the identities split needs a real version marker. Also extracts server/db/shared.js: findAvailableUsername, parseEventRow, and the putEvent/upsertParticipation row normalizers are dialect-free logic the Postgres driver will need identically, so they live once instead of being duplicated (and drifting) per driver. findAvailableUsername is now async and awaits its isTaken predicate - harmless for SQLite's synchronous predicate, required for Postgres's async one - so one implementation serves both.
Static `import { driver } from '../server/db/index.js'` is hoisted by the
ESM spec above the `process.env.DB_PATH = ':memory:'` assignment earlier in
the same file, so it stood up the driver (and its data/rackstack.db file)
against the real default path instead of an in-memory DB - polluting a
real on-disk file across test runs and producing state leakage between
tests. Pull `driver` off the existing dynamic `dbMod` import instead, same
as every other export these files already consume that way.
Caught by rerunning tests/db.test.js in isolation during self-review: it
failed deterministically (config/roles state carried over from a prior
run) even though the full `npm test` run had passed, because npm test's
earlier passing run happened to be the one that created the stray file.
Code review finding: the function has been async (it awaits the shared, async findAvailableUsername) since this refactor split it out of db.js, so the inherited "Sync" suffix was false - misleading for Task 4's Postgres driver author, who needs this exact seam. Renamed in schema.sqlite.js and updated its one call site in applySchema. driver.sqlite.js imports it aliased (dedupeUsernamesSchema) so it doesn't shadow the driver's own dedupeUsernames interface method, which stays a thin, now explicitly awaited, delegation to it (was a bare `return dedupeUsernamesSync(db)`, inconsistent with the explicit awaits elsewhere in the same file).
Testcontainers locally, a service container in CI, and a per-test-file database so the 19 db-touching suites cannot see each other's rows. The matrix exists because we ship two backends: a SQLite driver that CI never exercises would drift from the Postgres one, and dialect drift is the bug class that loses data. Locally, the only container runtime available is rootless Podman (no docker binary), so pg-global.js points DOCKER_HOST at the Podman socket and disables Ryuk when the developer hasn't already set either - Testcontainers then works unmodified on Docker or Podman. Ryuk's absence makes teardown() the only thing that stops/removes the container; StartedTestContainer#stop() removes volumes too, so no reaper is required as long as teardown runs. No Postgres driver exists yet (that's Task 4), so tests/harness.test.js is the only suite that actually exercises TEST_DATABASE_URL right now - the other 452 tests hardcode DB_PATH=':memory:' and keep running against SQLite regardless of TEST_BACKEND.
Both backends now satisfy the same interface and the full suite runs against each. tests/db.parity.test.js pins the behaviours that differ between the dialects and would otherwise fail silently in production: - pg returns BIGINT as a string by default; every epoch-ms column in this schema is BIGINT, so the int8 type parser is registered before any query. - Postgres folds unquoted identifiers to lowercase, so listLeaderboard's camelCase aliases are double-quoted - unquoted, the client receives 'userid' and every rungsClaimed reads undefined. - config_history ordered by rowid on SQLite; Postgres has none, so it gains a BIGSERIAL key and the admin rollback UI keeps its newest-first order. - COLLATE NOCASE becomes a unique functional index on lower(username). - SQLITE_CONSTRAINT_UNIQUE becomes SQLSTATE 23505 - that retry path is what keeps a display-name collision from locking a player out on login. Also points all 13 db-touching test files at provisionDatabase() instead of hardcoding DB_PATH=':memory:', so npm test genuinely exercises Postgres instead of silently falling back to SQLite regardless of TEST_BACKEND. tests/db.test.js and tests/db.events.test.js gain Postgres-branch schema introspection (pg_tables/information_schema vs. sqlite_master/PRAGMA); the two guarded-ALTER tests are SQLite-only (schema.pg.js has no ALTER history to replay) and skip on pg via it.runIf. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Critical: an ambient DATABASE_URL was never cleared on the sqlite test path, and server/db/index.js checks it before DB_PATH - so a developer with DATABASE_URL exported (plausible mid-migration, and the var Task 8 documents for production) would have every "sqlite" test file silently connect to that real database instead. Centralized all env-var writing for both backends inside provisionDatabase() itself (the pg branch is now the only writer of DATABASE_URL in the codebase; the sqlite branch clears it), removing the duplicated per-file if/else from all 14 test files. Covered by a new regression test in tests/harness.test.js that sets DATABASE_URL to an RFC-2606-reserved unreachable host and proves the facade still resolves to sqlite end-to-end, plus a deliberate full `npm run test:sqlite` run with that bogus DATABASE_URL exported (464/464 green in under 2s - no connection ever attempted). Important: extracted dedupeUsernames' suffixing walk (identical in both drivers apart from the SQL) into shared.js's dedupeUsernameRows, so the -2/-3 convention can't drift between drivers again. Replaced the false "exercised elsewhere in this suite" claim on db.test.js's guarded-ALTER skip with a real cross-backend test that calls applySchema() a second time against an already-populated database - the actual production restart path - on both backends. Minor: fixed db.parity.test.js's beforeAll/afterAll split (a RED-path import failure left `db` undefined, so afterAll's TypeError masked the real error and skipped cleanup) by switching to a top-level import like every other rewired file; added ORDER BY tie-breakers to driver.pg.js's listEvents/getAllUsersWithSaves so ties sort deterministically instead of arbitrarily on Postgres. Co-Authored-By: Claude Opus 5 <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
1 participant
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.
What this is
The approved design spec for moving RackStack off SQLite onto Postgres, and for adopting SuperTokens afterwards without breaking existing Discord/GitHub logins.
Spec:
docs/superpowers/specs/2026-08-01-postgres-supertokens-design.mdImplementation follows in this PR once the plan is written. Design only so far — no code changes.
Shape
Two releases from one design:
identitiesauth split, and a verified one-shot migration. No auth behaviour change at all.AUTH_MODEstrangler rollout, external user ID mapping, shadow-mode verification gate.Splitting them means that if a save ever goes missing, it is attributable to exactly one change.
Two findings that shaped it
SuperTokens supports external user ID mapping.
createUserIdMapping({ supertokensUserId, externalUserId })is documented for exactly this kind of gradual migration. Sousers.idstaysprovider:providerId, and none of the 3 foreign keys orSUPER_ADMIN_IDShave to change.SuperTokens' callback path would have broken GitHub login. It uses
/auth/callback/github; we use/auth/github/callback. GitHub requires the redirect URL to be a subdirectory of the registered callback, and it is not — every SuperTokens GitHub login would fail with a redirect_uri mismatch. Fix is to widen the registered callback to/auth, after which both paths qualify and work simultaneously, so passport keeps running throughout the rollout.Data-safety properties
COMMIT; any mismatch rolls back.TEXT, notjsonb—jsonbreorders keys and rejects the\u0000escape, so saves would not round-trip.rackstack.db*files; copying only the.dbsilently loses recent commits.Landmines catalogued
ORDER BY rowidingetConfigHistory(no rowid in PG — history order silently lost),COLLATE NOCASEusername uniqueness,SQLITE_CONSTRAINT_UNIQUE→ SQLSTATE23505(its retry path is what stops users being locked out on login), and epoch-msINTEGER→BIGINT.Scale
The async refactor is the bulk of the work: 111 call sites across 8 server files and 181 across 19 test files.
🤖 Generated with Claude Code