mirror of
https://github.com/garrytan/gbrain.git
synced 2026-07-28 14:59:47 +00:00
fix(cli): conditional schema-init on connect (closes #651)
Adds `hasPendingMigrations(engine)` next to `runMigrations` in migrate.ts:
single getConfig('version') probe, returns true when current < LATEST_VERSION,
defensively returns true on getConfig failure (treats wedged-config as pending).
`connectEngine` in cli.ts now wraps `engine.initSchema()` in a probe gate:
short-lived CLI calls (gbrain stats, query, doctor, etc.) on already-migrated
brains skip the bootstrap-probe + SCHEMA_SQL replay + ledger-check entirely.
Wedged brains still auto-heal — the probe says "yes pending" and initSchema
runs as before.
Building on oyi77's investigation in PR #652. Same correctness as #652's
unconditional initSchema-on-every-connect, but no perf regression on the
hot path. Failure non-fatal: if probe or init throws, log a hint and let
subsequent operations surface the real error in context.
Test coverage in test/migrate.test.ts: 3 cases covering fully-migrated
(false), version-rewound (true), and missing-version-config (defensive
true). Pairs with v0.28.5's X1 (post-upgrade auto-apply) — the upgrade
path runs initSchema explicitly while every other code path that goes
through connectEngine gets the cheap probe.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 4.7
parent
1a1dcf6e92
commit
039b7c51dd
+19
@@ -646,6 +646,25 @@ async function connectEngine(): Promise<BrainEngine> {
|
||||
process.env.GBRAIN_NO_RETRY_CONNECT === '1';
|
||||
const { connectWithRetry } = await import('./core/db.ts');
|
||||
await connectWithRetry(engine, toEngineConfig(config), { noRetry });
|
||||
|
||||
// Auto-apply pending schema migrations on connect (#651). Cheap probe
|
||||
// first so already-migrated brains don't pay the bootstrap-probe +
|
||||
// SCHEMA_SQL replay + ledger-check cost on every short-lived CLI call.
|
||||
// This is the conditional version of #652 (oyi77's investigation):
|
||||
// same correctness, no perf regression on the hot path.
|
||||
try {
|
||||
const { hasPendingMigrations } = await import('./core/migrate.ts');
|
||||
if (await hasPendingMigrations(engine)) {
|
||||
await engine.initSchema();
|
||||
}
|
||||
} catch (err) {
|
||||
// Non-fatal: if probe or initSchema fails, surface a hint and continue
|
||||
// with the connected engine. Subsequent operations will surface the
|
||||
// real schema error in context.
|
||||
console.warn(` Schema probe/migrate failed: ${(err as Error).message}`);
|
||||
console.warn(' Try: gbrain init --migrate-only');
|
||||
}
|
||||
|
||||
return engine;
|
||||
}
|
||||
|
||||
|
||||
@@ -1649,6 +1649,32 @@ async function runMigrationSQL(
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Cheap probe: does this engine have schema migrations pending?
|
||||
*
|
||||
* Reads the `version` config row in a single round-trip (no schema replay,
|
||||
* no migration apply). Used by `connectEngine` to gate `initSchema()` so
|
||||
* short-lived CLI invocations on already-migrated brains don't pay the
|
||||
* full bootstrap-probe + SCHEMA_SQL replay + ledger-check cost on every
|
||||
* `gbrain stats` / `gbrain query` / `gbrain doctor`.
|
||||
*
|
||||
* Defensive: treats a getConfig failure (config table missing, query error)
|
||||
* as "yes pending" so the caller falls through to the full initSchema path.
|
||||
* Worst case on a wedged brain is one extra schema replay — same as before.
|
||||
*
|
||||
* Closes #651 in cooperation with the post-upgrade auto-apply hook (X1)
|
||||
* without the perf cost #652 would have introduced on every CLI call.
|
||||
*/
|
||||
export async function hasPendingMigrations(engine: BrainEngine): Promise<boolean> {
|
||||
try {
|
||||
const currentStr = await engine.getConfig('version');
|
||||
const current = parseInt(currentStr || '1', 10);
|
||||
return current < LATEST_VERSION;
|
||||
} catch {
|
||||
return true;
|
||||
}
|
||||
}
|
||||
|
||||
export async function runMigrations(engine: BrainEngine): Promise<{ applied: number; current: number }> {
|
||||
const currentStr = await engine.getConfig('version');
|
||||
const current = parseInt(currentStr || '1', 10);
|
||||
|
||||
+44
-1
@@ -1,5 +1,5 @@
|
||||
import { describe, test, expect, beforeAll, afterAll, spyOn } from 'bun:test';
|
||||
import { LATEST_VERSION, runMigrations, MIGRATIONS, getIdleBlockers } from '../src/core/migrate.ts';
|
||||
import { LATEST_VERSION, runMigrations, MIGRATIONS, getIdleBlockers, hasPendingMigrations } from '../src/core/migrate.ts';
|
||||
import type { IdleBlocker } from '../src/core/migrate.ts';
|
||||
import type { BrainEngine } from '../src/core/engine.ts';
|
||||
import { PGLiteEngine } from '../src/core/pglite-engine.ts';
|
||||
@@ -20,6 +20,49 @@ describe('migrate', () => {
|
||||
// and are covered in the E2E suite (test/e2e/mechanical.test.ts)
|
||||
});
|
||||
|
||||
// v0.28.5 — A1: cheap probe used by `connectEngine` to gate `initSchema()`
|
||||
// so already-migrated brains don't pay the schema-replay cost on every
|
||||
// short-lived CLI invocation. Closes #651 in cooperation with X1's
|
||||
// post-upgrade auto-apply, without #652's perf regression.
|
||||
describe('hasPendingMigrations', () => {
|
||||
test('returns false on a fully-migrated brain (version === LATEST)', async () => {
|
||||
const engine = new PGLiteEngine();
|
||||
await engine.connect({});
|
||||
try {
|
||||
await engine.initSchema(); // applies all migrations through LATEST_VERSION
|
||||
expect(await hasPendingMigrations(engine)).toBe(false);
|
||||
} finally {
|
||||
await engine.disconnect();
|
||||
}
|
||||
}, 30000);
|
||||
|
||||
test('returns true when version config is behind LATEST_VERSION', async () => {
|
||||
const engine = new PGLiteEngine();
|
||||
await engine.connect({});
|
||||
try {
|
||||
await engine.initSchema();
|
||||
// Simulate an older brain by rewinding the version row.
|
||||
await engine.setConfig('version', '1');
|
||||
expect(await hasPendingMigrations(engine)).toBe(true);
|
||||
} finally {
|
||||
await engine.disconnect();
|
||||
}
|
||||
}, 30000);
|
||||
|
||||
test('returns true when version config is missing entirely (defensive default)', async () => {
|
||||
const engine = new PGLiteEngine();
|
||||
await engine.connect({});
|
||||
try {
|
||||
// Don't call initSchema. Probe against an empty PGlite — getConfig should
|
||||
// either return null (treated as version=1) or throw on missing config
|
||||
// table; either way the probe must say "yes pending."
|
||||
expect(await hasPendingMigrations(engine)).toBe(true);
|
||||
} finally {
|
||||
await engine.disconnect();
|
||||
}
|
||||
}, 30000);
|
||||
});
|
||||
|
||||
// ─────────────────────────────────────────────────────────────────
|
||||
// v0.18.0 — v16 sources_table_additive (Step 1, Lane A)
|
||||
// ─────────────────────────────────────────────────────────────────
|
||||
|
||||
Reference in New Issue
Block a user