Repository navigation
fix(db): sync native pgserve-v3 migration fix to dev (already on main via #2456) - #2457
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>
…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>
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ 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 |
Per "ship now + document backup→restore": covers the three host states new genie may encounter (v1.x, v2.x, v3) and the only manual step (snapshot+restore around the v3 cutover for data-bearing users). Notes the strict runtime.json detection (v1/v2 never retargeted), the GENIE_DB_NAME / GENIE_ROLE_CUTOVER=0 escape hatches, and roll-back. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1c4e9c7f3c
ℹ️ 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 (EMBEDDED_MIGRATIONS.length > 0) { | ||
| return EMBEDDED_MIGRATIONS.map((m) => ({ name: m.name, sql: m.sql })).sort((a, b) => | ||
| a.name < b.name ? -1 : a.name > b.name ? 1 : 0, | ||
| ); |
There was a problem hiding this comment.
Load on-disk migrations when manifest is stale
loadMigrationFiles() now always returns EMBEDDED_MIGRATIONS whenever that array is non-empty, which is true for normal source checkouts because migrations.generated.ts is committed. That means adding a new src/db/migrations/*.sql file without regenerating the manifest will silently skip the new migration in source-based runs (dev/CI/plain bun build paths), even though the SQL file exists. This introduces a schema-drift footgun that can break fresh environments until someone notices and manually regenerates the manifest.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Code Review
This pull request fixes a critical issue where compiled binaries failed to apply database migrations because SQL files were not bundled. It introduces a codegen script to embed migrations into the binary using Bun's static text imports. Additionally, it adds support for autopg/pgserve v3 by implementing native database provisioning and updating the discovery logic to include runtime.json. A review comment suggested optimizing the discovery process to avoid redundant file reads when checking for v3 runtime discovery.
| const directPostmaster = cutoverEnabled && transport.useSocket && readPostmasterDiscovery() !== null; | ||
|
|
||
| // Native-DB ownership is a v3-ONLY concern, and orthogonal to role | ||
| // cutover. v3 (runtime.json, router removed) has nothing to resolve the | ||
| // libpq default `postgres`, so genie must own a real named DB. v2 | ||
| // (admin.json + accept-hook router) MUST stay on `postgres` — the router | ||
| // maps it to the per-fingerprint DB; retargeting v2 to `genie` would | ||
| // boot it against a new empty DB (Codex P1 regression). And this must | ||
| // NOT depend on cutoverEnabled, or `GENIE_ROLE_CUTOVER=0` would skip | ||
| // provisioning and v3 would fall back to `postgres` and re-break | ||
| // (CodeRabbit). Hence: socket + v3 runtime.json, no cutover gate. | ||
| const v3NativeDb = transport.useSocket && hasV3RuntimeDiscovery(); |
There was a problem hiding this comment.
Both readPostmasterDiscovery() and hasV3RuntimeDiscovery() are called sequentially, and both perform synchronous file reads on the same runtime.json file when it exists. While the overhead is minimal, this could be optimized by having readPostmasterDiscovery return the discovery type or by caching the result of the file read.
Syncs the native genie → autopg/pgserve-v3 migration fix onto
dev.The fix merged to main via #2456 (it was authored
--base main).devwas left behind, which (a) means the dev channel ships a fix-less genie and (b) diverges dev↔main so the hourly rolling dev→main PR fails its history-sync check. This cherry-picks the two fix commits (31b241eeembed migrations + self-provision DB,caf7041creview fixes: v3-runtime.json gate, decoupled from role-cutover) onto dev verbatim.Identical, already-reviewed code (CodeRabbit + Codex addressed in #2456); proven end-to-end on a live autopg v3 host and redeployed there.
bun run check:fast+ biome green.After this merges to dev, the rolling dev→main PR is coherent again and the dev channel carries the fix.
🤖 Generated with Claude Code