Repository navigation
chore: rolling promotion dev → main (native pgserve-v3 fix + recovery runbook) - #2459
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>
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>
…-dev fix(db): sync native pgserve-v3 migration fix to dev (already on main via #2456)
The previous doc covered the DATA layer (backup→restore, in-place v1 import). It did not cover the WORK layer — restoring DB rows does NOT auto-reattach the live tmux panes / in-flight Claude conversations the operator was working in. Codifies the recovery runbook validated on a "very crowded" real instance (31 agents, 132 teams, 2725 sessions): - 3-bucket triage via `genie status --json` (ok / auto_resume_disabled / no_session_id) with the action per bucket - `genie resume` from a throwaway tmux socket (the dir:/GENIE_TEAM gotchas), validated working end-to-end - `genie agent recover` for ephemeral workers (and why the name-based heuristic often misses when customName is null) - `claude --resume <session-uuid>` direct fallback when genie can't pair the JSONL — the conversation data is durable on the filesystem - The post-mass-resume cosmetic `stale_spawn_dead_pane` reconciler glitch (one repeat resume heals it) Also documents the `migrate-v1` v3 gotcha: the archive step writes to pgserve-v2's `pgserve_meta` (deleted in v3) → migration aborts after already renaming the source DB. Workaround: `--no-archive` on v3; recover from a half-archive by renaming `genie_archive_<date>` back to `genie`. Verified on this host. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
docs(upgrade): post-upgrade work-recovery runbook + migrate-v1 v3 gotcha
📝 WalkthroughWalkthroughRelease v4.260519.3 bumps package versions, updates metrics state, embeds SQL migrations at build time via codegen, and adds pgserve v3 direct-postmaster database provisioning with validation and bootstrap helpers. ChangesRelease Bundle: Migration Embedding and DB Bootstrap
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint skipped: no ESLint configuration detected in root package.json. To enable, add 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 |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request implements support for pgserve v3 (autopg) by introducing native database provisioning and a new mechanism to embed SQL migrations directly into the compiled binary. It includes a migration manifest generation script, logic to detect v3 runtime environments, and an upgrade guide. Feedback identifies performance optimization opportunities, specifically gating database existence checks for short-lived CLI commands and removing a redundant sort operation in the migration loader.
| if (v3NativeDb && database !== DB_NAME) { | ||
| await ensureDatabaseExists(pgModule, transport, database); | ||
| } |
There was a problem hiding this comment.
The ensureDatabaseExists call is performed on every connection build when v3NativeDb is true. This includes short-lived CLI commands like genie hook dispatch (which sets GENIE_SKIP_DB_BOOT=1). Connecting to the maintenance database and querying pg_database on every hook execution adds significant overhead and contributes to the CPU saturation issues mentioned in the PR context. Since hooks are only executed when the system is already active, the database should already exist. This check should be gated by !cliShortLived to align with the other performance optimizations for short-lived commands.
if (v3NativeDb && database !== DB_NAME && !cliShortLived) {
await ensureDatabaseExists(pgModule, transport, database);
}| 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.
The sort() call here is redundant. The scripts/gen-migrations-manifest.ts script already sorts the migration files lexicographically before generating the EMBEDDED_MIGRATIONS array. Removing this extra sort will slightly improve the performance of connection establishment.
return EMBEDDED_MIGRATIONS.map((m) => ({ name: m.name, sql: m.sql }));![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2328cd360c
ℹ️ 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.
loadMigrationFiles() now returns EMBEDDED_MIGRATIONS whenever it is non-empty, which makes the filesystem scan unreachable in normal source checkouts because src/db/migrations.generated.ts is committed and populated. That means adding a new src/db/migrations/*.sql file without regenerating the manifest will silently skip that migration in genie db migrate (schema drift until runtime failures on new columns/tables). This regression is introduced here because previously the loader always read the directory at runtime; consider gating embedded-only behavior to compiled binaries or validating manifest/file parity before returning.
Useful? React with 👍 / 👎.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 233-238: The discovered socket check should verify the postmaster
is actually responsive rather than just the pathname existing: after computing
socketDir, port and pid (the block that returns { socketDir, port, pid: pid ??
-1 }), attempt a liveness probe before returning — e.g., if pid is present use
process.kill(pid, 0) to check the process exists (catch ESRCH and other errors),
and/or attempt a short timed Unix domain socket connect to join(socketDir,
`.s.PGSQL.${port}`) using net.Socket to confirm it accepts connections; only
return the discovery object when one of these probes succeeds, otherwise treat
it as stale and return null. Apply the same probe logic to the similar discovery
path in the later block covering lines 255-296 so both discovery branches
validate liveness.
- Around line 90-97: The code returns the GENIE_DB_NAME override unconditionally
(variable explicit) before checking runtime discovery mode, which allows v1/v2
paths to incorrectly use a named DB; update the logic so you only honor
process.env.GENIE_DB_NAME when hasV3RuntimeDiscovery() is true (e.g., call
hasV3RuntimeDiscovery() and return explicit only if true and explicit is
non-empty), and apply the same gated check to the other identical override block
later in the file (the second explicit/GENIE_DB_NAME handling).
🪄 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: 952cbc4c-5d27-44b3-aea0-26477bba1f64
⛔ Files ignored due to path filters (6)
.genie/assets/commits-30d.svgis excluded by!**/*.svg.genie/assets/loc-30d.svgis excluded by!**/*.svg.genie/assets/releases-30d.svgis excluded by!**/*.svgREADME.mdis excluded by!*.mdUPGRADING-pgserve-v3.mdis excluded by!*.mdVELOCITY.mdis excluded by!*.md
📒 Files selected for processing (13)
.claude-plugin/marketplace.json.genie/agents/metrics-updater/daily-stats.jsonl.genie/agents/metrics-updater/runs.jsonl.genie/agents/metrics-updater/state.jsonpackage.jsonplugins/genie/.claude-plugin/plugin.jsonplugins/genie/package.jsonscripts/build-binary.shscripts/gen-migrations-manifest.tssrc/db/migrations.generated.tssrc/lib/db-migrations.tssrc/lib/db.tssrc/types/sql.d.ts
| // Non-test explicit override. On a direct postmaster (autopg/pgserve v3, | ||
| // no accept-hook router) there is nothing to silently re-route the libpq | ||
| // default `postgres` into a per-fingerprint DB, so genie must target a | ||
| // real named database. `GENIE_DB_NAME` lets operators pin it; the | ||
| // direct-postmaster bootstrap defaults it to NATIVE_DB_NAME and creates | ||
| // it if absent. Legacy router path keeps returning DB_NAME. | ||
| const explicit = process.env.GENIE_DB_NAME; | ||
| if (explicit && explicit.length > 0) return explicit; |
There was a problem hiding this comment.
Gate GENIE_DB_NAME to v3-only paths.
This override is resolved before you've established hasV3RuntimeDiscovery(), so v1/TCP and v2/router connections can still leave postgres and run bootstrap/migrations against a fresh named DB. That breaks the back-compat contract this PR is trying to preserve.
Suggested fix
export function resolveDatabaseName(): string {
const testDbName = process.env.GENIE_TEST_DB_NAME;
if (testDbName && testDbName.length > 0) return testDbName;
- // Non-test explicit override. On a direct postmaster (autopg/pgserve v3,
- // no accept-hook router) there is nothing to silently re-route the libpq
- // default `postgres` into a per-fingerprint DB, so genie must target a
- // real named database. `GENIE_DB_NAME` lets operators pin it; the
- // direct-postmaster bootstrap defaults it to NATIVE_DB_NAME and creates
- // it if absent. Legacy router path keeps returning DB_NAME.
- const explicit = process.env.GENIE_DB_NAME;
- if (explicit && explicit.length > 0) return explicit;
return DB_NAME;
}
@@
- if (v3NativeDb && !isTestMode && database === DB_NAME) {
- database = NATIVE_DB_NAME;
- }
+ if (v3NativeDb && !isTestMode) {
+ const explicit = process.env.GENIE_DB_NAME;
+ if (explicit && explicit.length > 0) {
+ database = explicit;
+ } else if (database === DB_NAME) {
+ database = NATIVE_DB_NAME;
+ }
+ }
if (v3NativeDb && database !== DB_NAME) {
await ensureDatabaseExists(pgModule, transport, database);
}Also applies to: 1314-1354
🤖 Prompt for 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.
In `@src/lib/db.ts` around lines 90 - 97, The code returns the GENIE_DB_NAME
override unconditionally (variable explicit) before checking runtime discovery
mode, which allows v1/v2 paths to incorrectly use a named DB; update the logic
so you only honor process.env.GENIE_DB_NAME when hasV3RuntimeDiscovery() is true
(e.g., call hasV3RuntimeDiscovery() and return explicit only if true and
explicit is non-empty), and apply the same gated check to the other identical
override block later in the file (the second explicit/GENIE_DB_NAME handling).
| if (!socketDir || !port) return null; | ||
| // Validate the postmaster socket file exists. If pgserve crashed without | ||
| // cleaning up admin.json, the file path will be stale. | ||
| // Validate the postmaster socket file exists. If the server crashed | ||
| // without cleaning the discovery file, the path will be stale. | ||
| if (!existsSync(join(socketDir, `.s.PGSQL.${port}`))) return null; | ||
| // pid is informational — we don't gate on liveness here because the daemon | ||
| // process supervises the postmaster lifecycle. Stale pid + present socket | ||
| // is impossible under normal operation. | ||
| // pid is informational — the supervisor owns postmaster lifecycle. | ||
| return { socketDir, port, pid: pid ?? -1 }; |
There was a problem hiding this comment.
Validate discovered sockets for liveness, not just path existence.
A leftover .s.PGSQL.<port> pathname still satisfies existsSync(), so runtime.json/admin.json can be treated as live after an unclean crash. In that case the client will prefer a dead direct socket and miss the healthy fallback path. Please probe the discovered socket or otherwise verify the postmaster is still alive before returning discovery.
Also applies to: 255-296
🤖 Prompt for 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.
In `@src/lib/db.ts` around lines 233 - 238, The discovered socket check should
verify the postmaster is actually responsive rather than just the pathname
existing: after computing socketDir, port and pid (the block that returns {
socketDir, port, pid: pid ?? -1 }), attempt a liveness probe before returning —
e.g., if pid is present use process.kill(pid, 0) to check the process exists
(catch ESRCH and other errors), and/or attempt a short timed Unix domain socket
connect to join(socketDir, `.s.PGSQL.${port}`) using net.Socket to confirm it
accepts connections; only return the discovery object when one of these probes
succeeds, otherwise treat it as stale and return null. Apply the same probe
logic to the similar discovery path in the later block covering lines 255-296 so
both discovery branches validate liveness.
Public release: native genie ↔ autopg/pgserve-v3 + recovery runbook
Rolls
devintomainso the stable channel (latest.json) ships the full v3 story to the public. The rolling-pr.yml has been failing on its history-sync check; opening this manually so we don't gate the public release on the cron.What this brings to stable
GENIE_ROLE_CUTOVER). Already merged tomaindirectly via fix(db): native genie → autopg/pgserve-v3 migration (embed migrations + self-provision DB) #2456 (same content; cherry-picked onto dev via fix(db): sync native pgserve-v3 migration fix to dev (already on main via #2456) #2457 so dev wasn't behind).UPGRADING-pgserve-v3.md— the upgrade guide covering the three host states (v1.x / v2.x / v3), the only manual data step (backup→restore), themigrate-v1 --no-archivev3 gotcha + half-archive recovery, and the post-upgrade work-recovery runbook (genie status --json3-bucket triage →genie resumefrom throwaway tmux →genie agent recover→claude --resumedirect fallback). Validated end-to-end on a real "very crowded" instance (31 agents, 132 teams, 2725 sessions).Backward-compat reminder (already validated)
<socketDir>/runtime.json(v3-only marker; v2 publishesadmin.json), so existingapp_genie_<fp>setups stay onpostgres/router untouched.genie db migrate-v1informational prompt appears but doesn't block.After merging,
version.ymlwill fire on main (commit message contains/dev, satisfying the auto-version gate) → tag →release.yml→release-publish→latest.jsonflips to the new stable carrying the fix + doc.🤖 Generated with Claude Code
Summary by CodeRabbit
Chores
Bug Fixes