fix(db): native genie → autopg/pgserve-v3 migration (embed migrations + self-provision DB) - #2456
Conversation
… + self-provision DB)
genie could not migrate onto autopg/pgserve v3. Three coupled defects,
all reproduced + fixed against a live autopg v3 host:
1. KEYSTONE — migrations absent from the compiled binary.
db-migrations.ts loaded SQL via readdirSync(import.meta.dir
/../db/migrations) + Bun.file(); `bun build --compile` does NOT bundle
those runtime FS reads, so the shipped binary saw ZERO migrations →
`genie db migrate` reported "Applied: 0" on an empty DB, schema never
created. Fix: scripts/gen-migrations-manifest.ts →
src/db/migrations.generated.ts (static `with { type: 'text' }` imports
Bun embeds); loader prefers the embedded set, FS scan kept as a dev
fallback; build-binary.sh regenerates pre-compile so it can't go
stale; src/types/sql.d.ts ambient for the text import.
2. autopg v3 not detected as a direct postmaster.
readPostmasterDiscovery() only read `<socketDir>/admin.json`; autopg
v3 writes live discovery to `<socketDir>/runtime.json` (admin.json
moved to ~/.autopg/). directPostmaster was always false → genie stayed
on the legacy accept-hook/`postgres` path pgserve v3 deleted. Fix:
read runtime.json (v3) then admin.json (v2), shared parse/validate.
3. No native DB provisioning. resolveDatabaseName() always returned
`postgres`, relying on the deleted pgserve-v2 accept-hook to
auto-create+route `app_<name>_<fp>`. Fix: on the direct-postmaster
path target NATIVE_DB_NAME ('genie'; GENIE_DB_NAME override) and
ensureDatabaseExists() creates it from the `postgres` maintenance DB
(allowlisted ident; duplicate_database race tolerated). `database` is
reassigned in-place so the bootstrap pool, role-cutover GRANTs and
migrations agree on the target DB (an earlier split granted the scoped
role in `postgres` while the pool ran in `genie` → "no schema has been
selected to create in"). Legacy router path unchanged.
Proven on a live autopg v3 host, freshly compiled binary, zero env hacks
/ zero manual DB creation: genie db migrate → provisioned database
"genie" → role-cutover database=genie → 63 migrations → pg-seed 132
teams → genie db = 63 migrations / 52 tables → `genie ls` reads PG
cleanly. biome + tsc clean on changed files.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThis PR enables migration embedding in compiled binaries by generating a TypeScript manifest at build time, switching runtime migration loading to prefer embedded assets, and updating database provisioning to work correctly with direct-postmaster and autopg/pgserve v3 discovery. ChangesEmbedded Migration Support for Compiled Binaries
🎯 4 (Complex) | ⏱️ ~45 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/lib/db.ts`:
- Around line 1299-1301: The code currently gates direct-postmaster detection
behind isRoleCutoverEnabled() (cutoverEnabled), which prevents direct-postmaster
boot when GENIE_ROLE_CUTOVER=0; change the logic so directPostmaster is computed
purely from transport.useSocket and readPostmasterDiscovery() !== null (i.e.,
set directPostmaster = transport.useSocket && readPostmasterDiscovery() !==
null) and remove the dependency on cutoverEnabled; also update the DB
retarget/provisioning checks that currently use cutoverEnabled (the block
referencing DB retarget/provisioning) to instead rely on directPostmaster where
appropriate so retarget/provisioning is not skipped when cutover is disabled.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 08eb073b-ec3a-4ac1-b35e-c32936533002
📒 Files selected for processing (6)
scripts/build-binary.shscripts/gen-migrations-manifest.tssrc/db/migrations.generated.tssrc/lib/db-migrations.tssrc/lib/db.tssrc/types/sql.d.ts
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 31b241eeb4
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (directPostmaster && !isTestMode && database === DB_NAME) { | ||
| database = NATIVE_DB_NAME; |
There was a problem hiding this comment.
Gate native DB fallback to v3 runtime discovery only
directPostmaster now becomes true for both runtime.json and admin.json because readPostmasterDiscovery() checks both files, but this branch rewrites the default DB to NATIVE_DB_NAME whenever database === DB_NAME. On pgserve v2 hosts that still publish admin.json, this changes the default target from postgres to genie, so upgraded processes can start against a new empty DB instead of their existing one. This is a behavior regression for existing v2 deployments and should be limited to the v3 (runtime.json) case.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Code Review
This pull request introduces a mechanism to embed SQL migrations into the compiled binary using a generated manifest and Bun's static text imports, ensuring migrations are available without runtime filesystem access. It also adds support for detecting and connecting to direct postmasters (autopg/pgserve v3), including automatic database provisioning. Review feedback suggests optimizing performance by skipping database existence checks for short-lived CLI processes and refining error handling by removing an unnecessary 'unique_violation' check during database creation.
| // 42P04 duplicate_database / 23505 unique_violation → another booter won the race. | ||
| if (code !== '42P04' && code !== '23505') { |
There was a problem hiding this comment.
Error code 23505 (unique_violation) is typically associated with DML operations (like INSERT or UPDATE) violating a unique constraint on a table. For the CREATE DATABASE command, the standard Postgres error code for a database that already exists is 42P04 (duplicate_database). Including 23505 here is likely unnecessary and could be confusing to future maintainers.
// 42P04 duplicate_database → another booter won the race.
if (code !== '42P04') {| if (directPostmaster && database !== DB_NAME) { | ||
| await ensureDatabaseExists(pgModule, transport, database); | ||
| } |
There was a problem hiding this comment.
The ensureDatabaseExists function performs a relatively heavy operation, including creating a temporary connection pool and executing a catalog query. For short-lived processes like hook forks (which set GENIE_SKIP_DB_BOOT=1), this adds significant overhead to every invocation. Since database provisioning is a one-time setup task that should be handled by the daemon or migration commands, we should skip this check in short-lived CLI processes to improve performance.
if (directPostmaster && database !== DB_NAME && !cliShortLived) {
await ensureDatabaseExists(pgModule, transport, database);
}…ple from role-cutover CodeRabbit (Major) + Codex (P1) both correct: - Native-DB provisioning was gated on `directPostmaster`, which is `cutoverEnabled && … && readPostmasterDiscovery()`. With GENIE_ROLE_CUTOVER=0 it was skipped → v3 fell back to `postgres` and re-broke (CodeRabbit). - readPostmasterDiscovery() now also matches v2 `admin.json`, so `directPostmaster` was true on pgserve v2 too → my branch retargeted v2 hosts from `postgres` to `genie`, booting them against a new empty DB (Codex P1 regression). Fix: new `hasV3RuntimeDiscovery()` — true ONLY when `<socketDir>/ runtime.json` (the v3, router-less marker; v2 publishes only admin.json) parses. Native-DB now keys off `v3NativeDb = transport.useSocket && hasV3RuntimeDiscovery()` — NOT cutoverEnabled, NOT v2 admin.json. `directPostmaster` (role-cutover) is left exactly as-is, so v2 + v3 role-cutover behavior is unchanged. Validated on a live v3 host, freshly compiled binary: - default (cutover ON): 63 migrations → `genie` DB ✓ - GENIE_ROLE_CUTOVER=0: 63 migrations → `genie` DB ✓ (no postgres fallback) - v2 (no runtime.json): v3NativeDb=false → stays on `postgres` (router path) check:fast (typecheck/lint/dead-code/skills/wishes/emit) + biome green. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…-dev fix(db): sync native pgserve-v3 migration fix to dev (already on main via #2456)
genie could not natively migrate onto autopg/pgserve v3
Three coupled defects, all reproduced and fixed against a live autopg v3 host (freshly compiled binary, zero env hacks, zero manual DB creation):
1. KEYSTONE — migrations absent from the compiled binary
src/lib/db-migrations.tsloaded SQL viareaddirSync(import.meta.dir/../db/migrations)+Bun.file().bun build --compiledoes not bundle runtime FS reads, so the shipped binary saw zero migrations →genie db migratereportedApplied: 0on a completely empty DB and the schema was never created.Fix:
scripts/gen-migrations-manifest.tsgeneratessrc/db/migrations.generated.ts— staticwith { type: 'text' }imports Bun embeds into the binary. Loader prefers the embedded set; the FS scan stays as a dev-only fallback.scripts/build-binary.shregenerates the manifest pre-compile so it can't go stale.src/types/sql.d.tsambient for the text import.2. autopg v3 not detected as a direct postmaster
readPostmasterDiscovery()only read<socketDir>/admin.json; autopg v3 writes live discovery to<socketDir>/runtime.json(admin.json moved to~/.autopg/). SodirectPostmasterwas always false → genie stayed on the legacy accept-hook/postgrespath that pgserve v3 deleted.Fix: read
runtime.json(v3) thenadmin.json(v2 legacy), shared parse/validate.3. No native database provisioning
resolveDatabaseName()always returnedpostgres, relying on the deleted pgserve-v2 accept-hook to auto-create + routeapp_<name>_<fp>. No production code ever created genie's DB.Fix: on the direct-postmaster path, target
NATIVE_DB_NAME(genie;GENIE_DB_NAMEoverride) andensureDatabaseExists()creates it from thepostgresmaintenance DB (allowlisted identifier;duplicate_databaserace tolerated).databaseis reassigned in-place so the bootstrap pool, role-cutover GRANTs, and migrations all agree on the target DB (an earlier split granted the scoped role inpostgreswhile the pool ran ingenie→ "no schema has been selected to create in"). Legacy pgserve-v2 router path unchanged.Evidence (live autopg v3, compiled binary, no hacks)
bun run check:fast(typecheck, lint, dead-code, skills:lint, wishes:lint, lint:emit) + biome all green.Scope / safety
GENIE_DB_NAME(optional override; defaults togenieon v3).bun buildworks without the codegen step.🤖 Generated with Claude Code
Summary by CodeRabbit