mirror of
https://github.com/garrytan/gbrain.git
synced 2026-07-27 22:15:33 +00:00
fix(cli): reuse dispatcher-owned PGLite engine for reindex-frontmatter (takeover of #2764)
Salvages #2764's engine-reuse fix: the CLI dispatcher already holds a connected engine, so reindexFrontmatterCli now accepts it instead of opening a second PGLite connection against the same dir and deadlocking on its own lock. ownsEngine gates the disconnect so standalone callers keep the original lifecycle. The unrelated, undisclosed tini-probe changes (spawn-helpers.ts + supervisor-tini tests) are dropped for a separate PR. Co-authored-by: ssentamu <ssentamu@users.noreply.github.com> Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
co-authored by
ssentamu
Claude Fable 5
parent
a24f437a6f
commit
0247c51660
+5
-2
@@ -2051,8 +2051,11 @@ async function handleCliOnly(command: string, args: string[]) {
|
||||
// v0.30.1: still works; canonical entrypoint is now `gbrain backfill
|
||||
// effective_date`. This command stays as a thin alias for back-compat.
|
||||
const { reindexFrontmatterCli } = await import('./commands/reindex-frontmatter.ts');
|
||||
await reindexFrontmatterCli(args);
|
||||
return; // reindexFrontmatterCli handles its own engine lifecycle
|
||||
// This dispatcher already owns a connected engine. Passing it through
|
||||
// avoids a second PGLite connection trying to acquire the lock held by
|
||||
// this process.
|
||||
await reindexFrontmatterCli(args, engine);
|
||||
break;
|
||||
}
|
||||
case 'backfill': {
|
||||
// v0.30.1: first-class generic backfill command. Subcommand dispatch
|
||||
|
||||
@@ -151,8 +151,14 @@ export async function runReindexFrontmatter(
|
||||
};
|
||||
}
|
||||
|
||||
/** CLI entrypoint. Argv shape matches reindex-code for consistency. */
|
||||
export async function reindexFrontmatterCli(args: string[]): Promise<void> {
|
||||
/**
|
||||
* CLI entrypoint. Argv shape matches reindex-code for consistency.
|
||||
*
|
||||
* When dispatched from cli.ts, `existingEngine` is already connected and is
|
||||
* owned by the caller. Standalone callers may omit it and retain the original
|
||||
* create/connect/init/disconnect lifecycle.
|
||||
*/
|
||||
export async function reindexFrontmatterCli(args: string[], existingEngine?: BrainEngine): Promise<void> {
|
||||
const opts: ReindexFrontmatterOpts = {};
|
||||
for (let i = 0; i < args.length; i++) {
|
||||
const a = args[i];
|
||||
@@ -173,21 +179,29 @@ export async function reindexFrontmatterCli(args: string[]): Promise<void> {
|
||||
}
|
||||
}
|
||||
|
||||
const { createEngine } = await import('../core/engine-factory.ts');
|
||||
const { loadConfig, toEngineConfig } = await import('../core/config.ts');
|
||||
const cfg = loadConfig();
|
||||
if (!cfg) {
|
||||
console.error('No gbrain config; run `gbrain init` first.');
|
||||
process.exit(1);
|
||||
let engine: BrainEngine;
|
||||
let ownsEngine = false;
|
||||
|
||||
if (existingEngine) {
|
||||
engine = existingEngine;
|
||||
} else {
|
||||
const { createEngine } = await import('../core/engine-factory.ts');
|
||||
const { loadConfig, toEngineConfig } = await import('../core/config.ts');
|
||||
const cfg = loadConfig();
|
||||
if (!cfg) {
|
||||
console.error('No gbrain config; run `gbrain init` first.');
|
||||
process.exit(1);
|
||||
}
|
||||
const engineConfig = toEngineConfig(cfg);
|
||||
engine = await createEngine(engineConfig);
|
||||
// v0.37.7.0 #1225: createEngine() only constructs; callers MUST connect
|
||||
// before any executeRaw call. Pre-fix, the first query in countAffected
|
||||
// crashed with "PGLite not connected. Call connect() first." even on
|
||||
// --dry-run. initSchema is idempotent on a current schema, costs ~1ms.
|
||||
await engine.connect(engineConfig);
|
||||
await engine.initSchema();
|
||||
ownsEngine = true;
|
||||
}
|
||||
const engineConfig = toEngineConfig(cfg);
|
||||
const engine = await createEngine(engineConfig);
|
||||
// v0.37.7.0 #1225: createEngine() only constructs; callers MUST connect
|
||||
// before any executeRaw call. Pre-fix, the first query in countAffected
|
||||
// crashed with "PGLite not connected. Call connect() first." even on
|
||||
// --dry-run. initSchema is idempotent on a current schema, costs ~1ms.
|
||||
await engine.connect(engineConfig);
|
||||
await engine.initSchema();
|
||||
|
||||
try {
|
||||
const result = await runReindexFrontmatter(engine, opts);
|
||||
@@ -202,7 +216,7 @@ export async function reindexFrontmatterCli(args: string[]): Promise<void> {
|
||||
}
|
||||
if (result.status === 'cancelled') process.exit(1);
|
||||
} finally {
|
||||
if ('disconnect' in engine && typeof engine.disconnect === 'function') {
|
||||
if (ownsEngine && 'disconnect' in engine && typeof engine.disconnect === 'function') {
|
||||
await engine.disconnect();
|
||||
}
|
||||
}
|
||||
|
||||
@@ -10,9 +10,9 @@
|
||||
* happy path without throwing.
|
||||
*/
|
||||
|
||||
import { describe, test, expect, beforeAll, afterAll, beforeEach } from 'bun:test';
|
||||
import { describe, test, expect, beforeAll, afterAll, beforeEach, mock } from 'bun:test';
|
||||
import { PGLiteEngine } from '../src/core/pglite-engine.ts';
|
||||
import { runReindexFrontmatter } from '../src/commands/reindex-frontmatter.ts';
|
||||
import { reindexFrontmatterCli, runReindexFrontmatter } from '../src/commands/reindex-frontmatter.ts';
|
||||
|
||||
let engine: PGLiteEngine;
|
||||
|
||||
@@ -61,4 +61,36 @@ describe('reindex-frontmatter connect-before-query (#1225)', () => {
|
||||
expect(result.status).toBe('dry_run');
|
||||
expect(result.examined).toBeGreaterThanOrEqual(1);
|
||||
});
|
||||
|
||||
test('reuses the CLI-owned engine instead of constructing a second PGLite connection', async () => {
|
||||
await engine.executeRaw(
|
||||
`INSERT INTO pages (slug, type, title, compiled_truth, page_kind, frontmatter, effective_date)
|
||||
VALUES ($1, 'note', $2, $3, 'markdown', $4::jsonb, NULL)`,
|
||||
[
|
||||
'cli-owned-engine',
|
||||
'CLI owned engine',
|
||||
'# CLI owned engine\n\nbody',
|
||||
JSON.stringify({ effective_date: '2026-01-01' }),
|
||||
],
|
||||
);
|
||||
|
||||
const disconnect = engine.disconnect.bind(engine);
|
||||
const disconnectSpy = mock(disconnect);
|
||||
engine.disconnect = disconnectSpy;
|
||||
const logs: string[] = [];
|
||||
const originalLog = console.log;
|
||||
console.log = (message?: unknown) => logs.push(String(message));
|
||||
|
||||
try {
|
||||
await reindexFrontmatterCli(['--dry-run', '--json'], engine);
|
||||
const result = JSON.parse(logs.at(-1) ?? '{}') as { examined?: number };
|
||||
// A second engine would see a different/empty database; this asserts that
|
||||
// the CLI command queried the connected engine supplied by its dispatcher.
|
||||
expect(result.examined).toBeGreaterThanOrEqual(1);
|
||||
expect(disconnectSpy).not.toHaveBeenCalled();
|
||||
} finally {
|
||||
console.log = originalLog;
|
||||
engine.disconnect = disconnect;
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user