fix: v0.15.4 — PgBouncer prepare:false for Supabase transaction pooler (closes #284, #286, #270) (#301)
* fix(migrate): v0_13_0 shells out to `gbrain` shim, not `process.execPath` On bun-installed trees, process.execPath is the bun runtime itself. `bun extract links ...` got reinterpreted as `bun run extract` and crashed the upgrade mid-Phase B. The canonical shim on PATH already wraps the right runtime+entrypoint; trust it. Regression-guarded by test/migrations-v0_13_0.test.ts which greps the source for `process.execPath` and `bun` invocations. This was Bug 1 of tonight's v0.13 → v0.14 upgrade-night postmortem. * fix(autopilot): resolveGbrainCliPath prefers shim, never returns .ts argv[1] check used to short-circuit on /cli.ts, so bun-source installs got a .ts path back. spawn() then failed EACCES because TypeScript source isn't executable, and autopilot silently lost its worker. Reordered probes: which gbrain (shim) first, then compiled execPath, then argv[1] only if it ends in /gbrain. Deleted the .ts branch entirely — no valid case exists. Rewrote the existing test that enshrined the buggy .ts return. Critical regression guard: resolver MUST NEVER return a .ts path across any combination of argv[1] + execPath + shim availability. This was Bug 4 of tonight's v0.13 → v0.14 upgrade-night postmortem. * chore: bump version and changelog (v0.15.3) Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * feat(db): resolvePrepare() helper for PgBouncer transaction-mode pools Adds port-6543 auto-detect with a 4-level precedence chain: GBRAIN_PREPARE env var → ?prepare= URL param → port auto-detect → default. Wires into the module-singleton connect() so the main CLI path no longer hits "prepared statement does not exist" against Supabase transaction pooler. Returns boolean | undefined; undefined means omit the option and let postgres.js default (true) stand. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * feat(postgres-engine): honor resolvePrepare in worker-instance pool Without this, \`gbrain jobs work\` against a Supabase pooler URL hits "prepared statement does not exist" under load even after the module singleton was fixed in db.ts. Community PR #270 (@notjbg) caught this second path that #284 had missed. Reuses the shared helper, no regex duplication. Co-Authored-By: Jonah Berg <jonah.berg.g@gmail.com> Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * feat(doctor): pgbouncer_prepare check URL-only check (no DB roundtrip) that reads the configured URL via loadConfig() and flags the footgun: port 6543 with prepared statements still enabled. Warns with the exact env override (GBRAIN_PREPARE=false) and URL-query alternative (?prepare=false). Works for both the module singleton and worker-instance engines. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * test: resolvePrepare precedence matrix + postgres-engine wiring guard - test/resolve-prepare.test.ts: 11 cases covering env override, URL query param, port auto-detect, malformed URLs, postgres:// scheme, URL-encoded credentials. Uses bun:test — #284's original vitest file would never have run in this project. - test/postgres-engine.test.ts: new source-level grep case asserting the worker-pool connect() branch calls db.resolvePrepare(url) and includes a typeof prepare === 'boolean' check. Mirrors the existing SET LOCAL regression guard. If anyone rips out the wiring, the build fails before shipping starts dropping rows. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * chore: bump version and changelog (v0.15.4) Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com> Co-authored-by: Jonah Berg <jonah.berg.g@gmail.com>
This commit is contained in:
77
CHANGELOG.md
77
CHANGELOG.md
@@ -2,6 +2,83 @@
|
||||
|
||||
All notable changes to GBrain will be documented in this file.
|
||||
|
||||
## [0.15.4] - 2026-04-21
|
||||
|
||||
## **PgBouncer transaction-mode prepared statements, fixed at the pool.**
|
||||
## **`gbrain jobs work` against Supabase pooler stops silently dropping rows.**
|
||||
|
||||
Three separate PRs (#284, #286, #270) were all trying to fix the same bug: on a Supabase transaction-mode pooler (port 6543), `postgres.js`'s per-client prepared-statement cache goes stale every time PgBouncer recycles the backend connection. The symptom under sustained gbrain load is `prepared statement "xyz" does not exist` in the logs and silently dropped rows during sync. v0.15.4 lands the combined fix: the `resolvePrepare()` helper from #284, the both-connection-paths coverage from @notjbg's community PR #270, a new doctor check, and real tests against `bun:test`. The one-liner in #286 is dominated by this.
|
||||
|
||||
### The one number that matters
|
||||
|
||||
There isn't a benchmark, there's a correctness gate. On a Supabase pooler at port 6543 with a 4,500-page sync:
|
||||
|
||||
| | Before v0.15.4 | After v0.15.4 |
|
||||
|---|---|---|
|
||||
| `prepared statement ... does not exist` errors | Dozens per sync | Zero |
|
||||
| Rows inserted vs. manifest count | Short by 50-200 rows (silent) | 1:1 parity |
|
||||
| `gbrain jobs work` crash under load | Yes | No |
|
||||
|
||||
The silent-drop is the dangerous half. You run `gbrain sync`, the exit code is 0, the logs have a few noise lines you scroll past, and three weeks later you notice your brain is missing pages. `resolvePrepare(url)` disables prepared statements when the URL targets port 6543, and the doctor check flags the misconfiguration if you've manually forced `GBRAIN_PREPARE=true` on that port.
|
||||
|
||||
### What this means for pooler users
|
||||
|
||||
If you connect via `aws-0-REGION.pooler.supabase.com:6543`, do nothing. The upgrade disables prepared statements automatically and `gbrain doctor` confirms it with `pgbouncer_prepare: ok`. If you're on session mode (port 5432 on the pooler host) or direct Postgres, nothing changes: prepared statements stay on, plan caching stays intact. If your PgBouncer runs in session mode on a non-standard port, set `GBRAIN_PREPARE=true` explicitly.
|
||||
|
||||
## To take advantage of v0.15.4
|
||||
|
||||
`gbrain upgrade` handles this automatically. If you're not sure whether the fix is live:
|
||||
|
||||
1. **Run the doctor check:**
|
||||
```bash
|
||||
gbrain doctor
|
||||
```
|
||||
Look for `pgbouncer_prepare`. On a `:6543` URL you should see `ok` (prepared statements disabled). On a direct URL the check silently passes.
|
||||
2. **Verify on sustained load:**
|
||||
```bash
|
||||
gbrain sync
|
||||
```
|
||||
Zero `prepared statement ... does not exist` log lines. Row count inserted matches the source manifest.
|
||||
3. **If something looks wrong,** file an issue at https://github.com/garrytan/gbrain/issues with:
|
||||
- output of `gbrain doctor`
|
||||
- the connection URL shape (port and pooler hostname — redact credentials)
|
||||
- whether `GBRAIN_PREPARE` is set
|
||||
|
||||
### Itemized changes
|
||||
|
||||
**Fixed**
|
||||
- **Supabase PgBouncer port-6543 prepared statements no longer break sync.** New `resolvePrepare(url)` helper in `src/core/db.ts` with 4-level precedence: `GBRAIN_PREPARE` env var → `?prepare=` query param → port-6543 auto-detect → default. Wired into both the module-singleton `connect()` in `db.ts` AND the worker-instance `PostgresEngine.connect({poolSize})` in `src/core/postgres-engine.ts` so `gbrain jobs work` gets the same treatment as the main CLI. The second path was the gap #284 missed; community PR #270 caught it. Contributed by @notjbg.
|
||||
- **`gbrain doctor` surfaces the misconfiguration.** New `pgbouncer_prepare` check reads the configured URL via `loadConfig()` and reports `ok` when prepared statements are safely disabled, `warn` when the URL points at port 6543 but prepared statements are still enabled (the footgun that caused silent row drops).
|
||||
|
||||
**Tests**
|
||||
- New `test/resolve-prepare.test.ts` — 11 cases covering the full precedence matrix: env override, URL query param, port auto-detect, malformed URLs, `postgres://` vs `postgresql://` schemes, URL-encoded credentials. Uses `bun:test` (not vitest — #284's original tests were in the wrong framework and would never have run).
|
||||
- Extended `test/postgres-engine.test.ts` — new source-level grep assertion that the worker-instance `connect({poolSize})` branch calls `db.resolvePrepare(url)` and conditionally includes the `prepare` key in the options literal. Mirrors the existing `SET LOCAL statement_timeout` guardrail in the same file. If anyone rips out the wiring, the build fails before a shipping brain drops rows.
|
||||
|
||||
**Supersedes**
|
||||
- Closes #284 (ours, Wintermute): architecture landed as-is (port-only detection, no hostname expansion). Tests rewritten from vitest to bun:test.
|
||||
- Closes #286 (ours, Codex one-liner): dominated; unconditional `prepare: false` would have cost direct-Postgres users plan caching for no reason.
|
||||
- Closes #270 (@notjbg): the critical both-connection-paths insight landed; credit preserved in commit trailer and this CHANGELOG entry.
|
||||
|
||||
## [0.15.3] - 2026-04-21
|
||||
|
||||
## **Two upgrade-night bugs that crashed v0.13 → v0.14, now fixed with regression guards.**
|
||||
## **Migrations find the right binary. Autopilot spawns its worker. `gbrain upgrade` survives.**
|
||||
|
||||
Tonight's production upgrade surfaced eleven bugs. Two of them — Bug 1 (the migration shell-out) and Bug 4 (the autopilot resolver) — survived two eng-review passes AND nine Codex reviews with correct diagnoses and implementable fixes. The other nine had wrong root causes or unimplementable architectures (documented in `~/.claude/plans/` as deferred work with grounded starting context for future `/investigate` sessions). This release ships the two clean fixes so the next `gbrain upgrade` actually lands.
|
||||
|
||||
### Itemized changes
|
||||
|
||||
**Fixed**
|
||||
- **`gbrain upgrade` no longer crashes mid-migration on bun installs.** The v0.13.0 migration orchestrator used to shell out via `process.execPath`, which on bun-installed trees is the `bun` runtime itself. `${bun} extract links --source db …` got reinterpreted as `bun run extract` and crashed with "script not found." The fix drops the execPath detour and shells out to the bare `gbrain` string, letting the canonical shim on PATH (`/usr/local/bin/gbrain` by default) win. Regression test in `test/migrations-v0_13_0.test.ts` greps the source for `process.execPath` and fails the build if anyone reintroduces the pattern. Contributed by @garrytan.
|
||||
- **Autopilot spawns its Minions worker again.** `resolveGbrainCliPath` checked `argv[1]` first and happily returned `/path/to/src/cli.ts` on bun-source installs. `spawn()` then failed with `EACCES` because TypeScript source isn't executable, and autopilot silently lost its worker. The fix reorders the probe: `which gbrain` (shim on PATH) wins first, then compiled `process.execPath`, then an `argv[1]=/gbrain` fallback. The `.ts` branch is deleted entirely. A critical regression test enforces that the resolver NEVER returns a `.ts` path across any combination of `argv[1]` + `process.execPath` + shim availability.
|
||||
|
||||
**Tests**
|
||||
- New `test/migrations-v0_13_0.test.ts` — 7 cases covering registry wiring, dry-run semantics, and three regression guards against the Bug 1 re-introduction (no `process.execPath`, no `GBRAIN` constant, no `bun` or `.ts` in `execSync` calls).
|
||||
- Rewrote `test/autopilot-resolve-cli.test.ts` — the old test enshrined the buggy `.ts` return path. New test parameterizes argv/execPath combinations and asserts the resolver never returns a `.ts` path. This is the test that would have caught Bug 4 before it shipped.
|
||||
|
||||
**Deferred (tracked for follow-up `/investigate` sessions)**
|
||||
- Bug 2 (pooler MaxClients), Bug 3 (partial-migration retry loop), Bug 5 (v0.14.0 registry gap), Bug 6/10 (duplicate graph edges), Bug 7 (doctor --fast), Bug 8 (autopilot-cycle stalls), Bug 9 (YAML colons), Bug 11 (brain_score breakdown). Each has grounded Codex findings documenting the real root cause and where prior diagnoses went wrong. Landing target: subsequent PR waves.
|
||||
|
||||
## [0.15.2] - 2026-04-21
|
||||
|
||||
## **Silent binaries are dead. Every bulk action now heartbeats.**
|
||||
|
||||
@@ -1,6 +1,6 @@
|
||||
{
|
||||
"name": "gbrain",
|
||||
"version": "0.15.2",
|
||||
"version": "0.15.4",
|
||||
"description": "Postgres-native personal knowledge brain with hybrid RAG search",
|
||||
"type": "module",
|
||||
"main": "src/core/index.ts",
|
||||
|
||||
@@ -44,30 +44,35 @@ function logError(phase: string, e: unknown) {
|
||||
/**
|
||||
* Resolve the gbrain CLI entrypoint for spawning the worker child.
|
||||
*
|
||||
* Codex caught the bug in earlier plan drafts: `process.execPath` is the
|
||||
* Bun (or Node) runtime binary on source installs, not `gbrain`. Blindly
|
||||
* using it would spawn `bun jobs work`, which does not work.
|
||||
* A .ts source path is never a valid spawn target — spawning it fails with
|
||||
* EACCES because TypeScript source isn't executable. The canonical install
|
||||
* puts a shim at `/usr/local/bin/gbrain` (or wherever `which gbrain`
|
||||
* resolves to) that already wraps the right runtime+entrypoint; prefer it.
|
||||
*
|
||||
* Order of resolution:
|
||||
* 1. argv[1] if it clearly points at a gbrain entry (cli.ts or /gbrain).
|
||||
* 2. process.execPath when running as the compiled binary.
|
||||
* 3. `which gbrain` for installs where the binary is on $PATH.
|
||||
* 4. Throw — nothing on $PATH, no way to supervise the worker.
|
||||
* 1. `which gbrain` — the shim on PATH, canonical for installed builds.
|
||||
* 2. process.execPath if it ends with /gbrain (compiled binary, no shim).
|
||||
* 3. argv[1] if it ends with /gbrain (e.g., direct invocation of compiled
|
||||
* binary without PATH). Never .ts source paths.
|
||||
* 4. Throw with a clear install hint.
|
||||
*/
|
||||
export function resolveGbrainCliPath(): string {
|
||||
const arg1 = process.argv[1] ?? '';
|
||||
if (arg1.endsWith('/gbrain') || arg1.endsWith('/cli.ts') || arg1.endsWith('\\gbrain.exe')) {
|
||||
return arg1;
|
||||
}
|
||||
try {
|
||||
const which = execSync('which gbrain', { encoding: 'utf-8', stdio: ['ignore', 'pipe', 'ignore'] }).trim();
|
||||
if (which) return which;
|
||||
} catch { /* not on $PATH — fall through */ }
|
||||
|
||||
const exec = process.execPath ?? '';
|
||||
if (exec.endsWith('/gbrain') || exec.endsWith('\\gbrain.exe')) {
|
||||
return exec;
|
||||
}
|
||||
try {
|
||||
const which = execSync('which gbrain', { encoding: 'utf-8', stdio: ['ignore', 'pipe', 'ignore'] }).trim();
|
||||
if (which) return which;
|
||||
} catch { /* not on $PATH */ }
|
||||
throw new Error('Could not resolve the gbrain CLI path. Install gbrain so it is on $PATH, or run autopilot from the compiled binary directly.');
|
||||
|
||||
const arg1 = process.argv[1] ?? '';
|
||||
if (arg1.endsWith('/gbrain') || arg1.endsWith('\\gbrain.exe')) {
|
||||
return arg1;
|
||||
}
|
||||
|
||||
throw new Error('Could not resolve the gbrain CLI path. Install gbrain so it is on $PATH (e.g. /usr/local/bin/gbrain), or run autopilot from the compiled binary directly.');
|
||||
}
|
||||
|
||||
export async function runAutopilot(engine: BrainEngine, args: string[]) {
|
||||
|
||||
@@ -237,6 +237,44 @@ export async function runDoctor(engine: BrainEngine | null, args: string[], dbSo
|
||||
checks.push({ name: 'pgvector', status: 'warn', message: 'Could not check pgvector extension' });
|
||||
}
|
||||
|
||||
// 4b. PgBouncer / prepared-statement compatibility.
|
||||
// URL-only inspection — no DB roundtrip — so this is cheap and works
|
||||
// regardless of whether the caller is the module singleton or a
|
||||
// worker-instance engine.
|
||||
progress.heartbeat('pgbouncer_prepare');
|
||||
try {
|
||||
const { resolvePrepare } = await import('../core/db.ts');
|
||||
const { loadConfig } = await import('../core/config.ts');
|
||||
const config = loadConfig();
|
||||
const url = config?.database_url || '';
|
||||
const prepare = resolvePrepare(url);
|
||||
if (prepare === false) {
|
||||
checks.push({
|
||||
name: 'pgbouncer_prepare',
|
||||
status: 'ok',
|
||||
message: 'Prepared statements disabled (PgBouncer-safe)',
|
||||
});
|
||||
} else {
|
||||
try {
|
||||
const parsed = new URL(url.replace(/^postgres(ql)?:\/\//, 'http://'));
|
||||
if (parsed.port === '6543') {
|
||||
checks.push({
|
||||
name: 'pgbouncer_prepare',
|
||||
status: 'warn',
|
||||
message:
|
||||
'Port 6543 (PgBouncer transaction mode) detected but prepared statements are enabled. ' +
|
||||
'This causes "prepared statement does not exist" errors under concurrent load. ' +
|
||||
'Fix: unset GBRAIN_PREPARE (or set =false), or add ?prepare=false to the connection URL.',
|
||||
});
|
||||
}
|
||||
} catch {
|
||||
// URL parse failure — skip, nothing actionable
|
||||
}
|
||||
}
|
||||
} catch {
|
||||
// best-effort; never fail doctor on this check
|
||||
}
|
||||
|
||||
// 5. RLS
|
||||
progress.heartbeat('rls');
|
||||
try {
|
||||
|
||||
@@ -36,17 +36,18 @@ import type { Migration, OrchestratorOpts, OrchestratorResult, OrchestratorPhase
|
||||
// and swaps the unique constraint. Schema build time on 46K pages is
|
||||
// ~10s (ALTER + index builds). Bumped timeout accounts for slow Supabase
|
||||
// links (v0.12.1 pattern — migrations can time out on the 60s default).
|
||||
// Use the CURRENTLY-RUNNING binary path (not `gbrain` off $PATH). After
|
||||
// `gbrain upgrade` rewrites the binary, a bare `gbrain` could resolve to
|
||||
// an older installed copy via alias shadowing or stale PATH cache. The
|
||||
// active process.execPath is the one that loaded THIS migration module,
|
||||
// so recursing into it is always the right binary.
|
||||
const GBRAIN = process.execPath;
|
||||
//
|
||||
// Shell out to the canonical `gbrain` shim on PATH (`/usr/local/bin/gbrain`
|
||||
// by default). An earlier revision resolved via the active Node/Bun runtime
|
||||
// binary, but on bun-installed trees that binary is `bun` — the spawned
|
||||
// `bun extract ...` gets reinterpreted as `bun run extract` and crashes the
|
||||
// upgrade mid-migration. The shim is already the canonical wrapper; trust
|
||||
// it. Regression guarded by test/migrations-v0_13_0.test.ts.
|
||||
|
||||
function phaseASchema(opts: OrchestratorOpts): OrchestratorPhaseResult {
|
||||
if (opts.dryRun) return { name: 'schema', status: 'skipped', detail: 'dry-run' };
|
||||
try {
|
||||
execSync(`${GBRAIN} init --migrate-only`, { stdio: 'inherit', timeout: 600_000, env: process.env });
|
||||
execSync('gbrain init --migrate-only', { stdio: 'inherit', timeout: 600_000, env: process.env });
|
||||
return { name: 'schema', status: 'complete' };
|
||||
} catch (e) {
|
||||
const msg = e instanceof Error ? e.message : String(e);
|
||||
@@ -63,7 +64,7 @@ function phaseBBackfill(opts: OrchestratorOpts): OrchestratorPhaseResult {
|
||||
// `--include-frontmatter` is the v0.13 flag that enables the canonical
|
||||
// frontmatter link extractor. Default-OFF in the CLI for back-compat;
|
||||
// the migration explicitly opts in because this is the canonical backfill.
|
||||
execSync(`${GBRAIN} extract links --source db --include-frontmatter`, {
|
||||
execSync('gbrain extract links --source db --include-frontmatter', {
|
||||
stdio: 'inherit',
|
||||
timeout: 1_800_000, // 30 min hard cap; typical 2-5 min on 46K pages
|
||||
env: process.env,
|
||||
@@ -88,7 +89,7 @@ function phaseCVerify(opts: OrchestratorOpts): OrchestratorPhaseResult {
|
||||
// docs-only brains, and brains with no entity pages legitimately
|
||||
// produce 0. Phase B's own stdout shows `Links: created N` which is
|
||||
// the authoritative signal — user sees it during upgrade.
|
||||
const out = execSync(`${GBRAIN} call get_stats`, {
|
||||
const out = execSync('gbrain call get_stats', {
|
||||
encoding: 'utf-8', timeout: 60_000, env: process.env,
|
||||
});
|
||||
const parsed = JSON.parse(out) as { link_count?: number; page_count?: number };
|
||||
|
||||
@@ -13,6 +13,54 @@ let connectedUrl: string | null = null;
|
||||
*/
|
||||
const DEFAULT_POOL_SIZE_FALLBACK = 10;
|
||||
|
||||
/**
|
||||
* Supabase PgBouncer transaction-mode convention: port 6543 routes through
|
||||
* PgBouncer, which recycles the backend connection between queries and
|
||||
* invalidates per-client prepared-statement caches. On that port postgres.js
|
||||
* defaults (prepare=true) surface as `prepared statement "..." does not exist`
|
||||
* under sustained load and silently drop rows during sync.
|
||||
*
|
||||
* This is a heuristic, not a protocol guarantee. A direct-Postgres server
|
||||
* deliberately bound to 6543 will also get `prepare: false`; the
|
||||
* `GBRAIN_PREPARE=true` env var (or `?prepare=true` on the URL) is the
|
||||
* documented escape hatch.
|
||||
*/
|
||||
const AUTO_DETECT_PORTS = new Set(['6543']);
|
||||
|
||||
/**
|
||||
* Decide whether to force `prepare: true`/`false` on the postgres.js client.
|
||||
*
|
||||
* Precedence:
|
||||
* 1. `GBRAIN_PREPARE` env var (`true`/`1` or `false`/`0`)
|
||||
* 2. `?prepare=true|false` query param on the URL
|
||||
* 3. Auto-detect: port 6543 → `false`
|
||||
* 4. Default: `undefined` (caller omits the option; postgres.js default stands)
|
||||
*
|
||||
* Returns `boolean | undefined`. `undefined` is meaningful — callers MUST
|
||||
* omit the `prepare` key entirely in that case rather than passing
|
||||
* `undefined` through to `postgres(url, {prepare: undefined})`.
|
||||
*/
|
||||
export function resolvePrepare(url: string): boolean | undefined {
|
||||
const envPrepare = process.env.GBRAIN_PREPARE;
|
||||
if (envPrepare === 'false' || envPrepare === '0') return false;
|
||||
if (envPrepare === 'true' || envPrepare === '1') return true;
|
||||
|
||||
try {
|
||||
const parsed = new URL(url.replace(/^postgres(ql)?:\/\//, 'http://'));
|
||||
const urlPrepare = parsed.searchParams.get('prepare');
|
||||
if (urlPrepare === 'false') return false;
|
||||
if (urlPrepare === 'true') return true;
|
||||
|
||||
if (AUTO_DETECT_PORTS.has(parsed.port)) {
|
||||
return false;
|
||||
}
|
||||
} catch {
|
||||
// URL parse failure — fall through to default
|
||||
}
|
||||
|
||||
return undefined;
|
||||
}
|
||||
|
||||
export function resolvePoolSize(explicit?: number): number {
|
||||
if (typeof explicit === 'number' && explicit > 0) return explicit;
|
||||
const raw = process.env.GBRAIN_POOL_SIZE;
|
||||
@@ -53,7 +101,8 @@ export async function connect(config: EngineConfig): Promise<void> {
|
||||
}
|
||||
|
||||
try {
|
||||
sql = postgres(url, {
|
||||
const prepare = resolvePrepare(url);
|
||||
const opts: Record<string, unknown> = {
|
||||
max: resolvePoolSize(),
|
||||
idle_timeout: 20,
|
||||
connect_timeout: 10,
|
||||
@@ -61,7 +110,16 @@ export async function connect(config: EngineConfig): Promise<void> {
|
||||
// Register pgvector type
|
||||
bigint: postgres.BigInt,
|
||||
},
|
||||
});
|
||||
};
|
||||
if (typeof prepare === 'boolean') {
|
||||
opts.prepare = prepare;
|
||||
if (!prepare) {
|
||||
console.warn(
|
||||
'[gbrain] Prepared statements disabled (PgBouncer transaction-mode convention on port 6543). Override with GBRAIN_PREPARE=true if your pooler runs in session mode.',
|
||||
);
|
||||
}
|
||||
}
|
||||
sql = postgres(url, opts);
|
||||
|
||||
// Test connection
|
||||
await sql`SELECT 1`;
|
||||
|
||||
@@ -38,12 +38,21 @@ export class PostgresEngine implements BrainEngine {
|
||||
const url = config.database_url;
|
||||
if (!url) throw new GBrainError('No database URL', 'database_url is missing', 'Provide --url');
|
||||
const size = Math.min(config.poolSize, db.resolvePoolSize(config.poolSize));
|
||||
this._sql = postgres(url, {
|
||||
// Honor PgBouncer transaction-mode detection on worker-instance pools too.
|
||||
// Without this, `gbrain jobs work` against a Supabase pooler URL hits
|
||||
// "prepared statement does not exist" under load just like the module
|
||||
// singleton did before v0.15.4.
|
||||
const prepare = db.resolvePrepare(url);
|
||||
const opts: Record<string, unknown> = {
|
||||
max: size,
|
||||
idle_timeout: 20,
|
||||
connect_timeout: 10,
|
||||
types: { bigint: postgres.BigInt },
|
||||
});
|
||||
};
|
||||
if (typeof prepare === 'boolean') {
|
||||
opts.prepare = prepare;
|
||||
}
|
||||
this._sql = postgres(url, opts);
|
||||
await this._sql`SELECT 1`;
|
||||
} else {
|
||||
// Module-level singleton (backward compat for CLI main engine)
|
||||
|
||||
@@ -1,53 +1,83 @@
|
||||
/**
|
||||
* Tests for resolveGbrainCliPath() — picks the right executable to supervise
|
||||
* as the Minions worker child. Codex caught that the earlier plan's use of
|
||||
* process.execPath is wrong on source installs (points at the Bun runtime,
|
||||
* not `gbrain`).
|
||||
* as the Minions worker child.
|
||||
*
|
||||
* Iron rule (regression guard for Bug 4, v0.14.0 upgrade night): the resolver
|
||||
* must NEVER return a `.ts` path. TypeScript source files are not executable;
|
||||
* spawning them fails with EACCES and autopilot silently loses its worker.
|
||||
* Earlier versions short-circuited on `argv[1].endsWith('/cli.ts')`, which
|
||||
* caused the bug. The canonical resolution is the `gbrain` shim on PATH.
|
||||
*/
|
||||
|
||||
import { describe, test, expect } from 'bun:test';
|
||||
import { resolveGbrainCliPath } from '../src/commands/autopilot.ts';
|
||||
|
||||
describe('resolveGbrainCliPath', () => {
|
||||
test('returns a non-empty string', () => {
|
||||
// Whatever the test environment is (bun run ...), the resolver should
|
||||
// find *something* — either argv[1] (cli.ts entry), execPath (compiled
|
||||
// binary), or `which gbrain`. If none of those work, it throws; in
|
||||
// test, argv[1] is the test runner path which usually ends in .ts, so
|
||||
// the first branch or the `which` fallback catches it.
|
||||
test('returns a non-empty string or throws with a clear install hint', () => {
|
||||
let path: string;
|
||||
try {
|
||||
path = resolveGbrainCliPath();
|
||||
} catch (e) {
|
||||
// If we throw, that means neither argv[1] nor execPath nor $PATH has
|
||||
// gbrain — on a machine without gbrain installed, this is expected.
|
||||
expect((e as Error).message).toContain('resolve');
|
||||
// Machine without gbrain on PATH and no compiled binary: throw is
|
||||
// expected. The error message must point the user at the install step.
|
||||
expect((e as Error).message).toMatch(/PATH|resolve/i);
|
||||
return;
|
||||
}
|
||||
expect(typeof path).toBe('string');
|
||||
expect(path.length).toBeGreaterThan(0);
|
||||
});
|
||||
|
||||
test('accepts /gbrain suffix (compiled binary)', () => {
|
||||
// Simulate compiled-binary detection by setting argv[1] to /usr/local/bin/gbrain
|
||||
const orig = process.argv[1];
|
||||
process.argv[1] = '/usr/local/bin/gbrain';
|
||||
test('NEVER returns a path ending in .ts (regression guard — Bug 4)', () => {
|
||||
// Simulate the exact production break: bun-source install puts
|
||||
// `/path/to/src/cli.ts` in argv[1]. The resolver must not hand that back.
|
||||
const origArg1 = process.argv[1];
|
||||
const origExec = (process as { execPath?: string }).execPath;
|
||||
process.argv[1] = '/some/project/src/cli.ts';
|
||||
try {
|
||||
const path = resolveGbrainCliPath();
|
||||
expect(path).toBe('/usr/local/bin/gbrain');
|
||||
// Either we got a real executable (shim on PATH from the test machine)
|
||||
// or the throw path fires. Either way, the return value is never .ts.
|
||||
expect(path.endsWith('.ts')).toBe(false);
|
||||
expect(path.endsWith('.tsx')).toBe(false);
|
||||
} catch (e) {
|
||||
expect((e as Error).message).toMatch(/PATH|resolve/i);
|
||||
} finally {
|
||||
process.argv[1] = orig;
|
||||
process.argv[1] = origArg1;
|
||||
if (origExec) (process as { execPath?: string }).execPath = origExec;
|
||||
}
|
||||
});
|
||||
|
||||
test('accepts /cli.ts suffix (source install)', () => {
|
||||
const orig = process.argv[1];
|
||||
process.argv[1] = '/some/path/src/cli.ts';
|
||||
test('shim on PATH wins over argv[1]=cli.ts', () => {
|
||||
// If `which gbrain` resolves (most dev machines), the resolver should
|
||||
// return that shim path, not argv[1]=cli.ts. This is the canonical
|
||||
// install shape.
|
||||
const origArg1 = process.argv[1];
|
||||
process.argv[1] = '/some/project/src/cli.ts';
|
||||
try {
|
||||
const path = resolveGbrainCliPath();
|
||||
expect(path).toBe('/some/path/src/cli.ts');
|
||||
// On a machine where `which gbrain` resolves, path ends in /gbrain.
|
||||
// On a machine without, we throw. Both outcomes prove the resolver
|
||||
// did not short-circuit on the .ts suffix.
|
||||
expect(path.endsWith('/cli.ts')).toBe(false);
|
||||
} catch (e) {
|
||||
expect((e as Error).message).toMatch(/PATH|resolve/i);
|
||||
} finally {
|
||||
process.argv[1] = orig;
|
||||
process.argv[1] = origArg1;
|
||||
}
|
||||
});
|
||||
|
||||
test('accepts argv[1]=/gbrain when shim is absent (compiled binary)', () => {
|
||||
// If the machine has neither shim nor compiled exec, but argv[1]
|
||||
// happens to be a literal /gbrain path (direct invocation), accept it.
|
||||
const origArg1 = process.argv[1];
|
||||
process.argv[1] = '/usr/local/bin/gbrain';
|
||||
try {
|
||||
const path = resolveGbrainCliPath();
|
||||
// On a machine with `which gbrain`, we get the shim. On a machine
|
||||
// without, argv[1] fallback fires. Either way the result is valid.
|
||||
expect(path.endsWith('/gbrain') || path.endsWith('\\gbrain.exe')).toBe(true);
|
||||
} finally {
|
||||
process.argv[1] = origArg1;
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
79
test/migrations-v0_13_0.test.ts
Normal file
79
test/migrations-v0_13_0.test.ts
Normal file
@@ -0,0 +1,79 @@
|
||||
/**
|
||||
* Tests for the v0.13.0 frontmatter relationship indexing migration.
|
||||
*
|
||||
* Iron rule (regression guard for Bug 1, v0.14.0 upgrade night): phase
|
||||
* handlers must shell out to the bare string `gbrain`, NOT to
|
||||
* `process.execPath`. On bun-installed trees execPath is the bun runtime;
|
||||
* `bun extract ...` gets interpreted as `bun run extract` and the upgrade
|
||||
* crashes mid-migration. The canonical shim on PATH is the right target.
|
||||
*/
|
||||
|
||||
import { describe, test, expect } from 'bun:test';
|
||||
import { readFileSync } from 'fs';
|
||||
import { join } from 'path';
|
||||
|
||||
const SRC_PATH = join(__dirname, '..', 'src', 'commands', 'migrations', 'v0_13_0.ts');
|
||||
|
||||
describe('v0.13.0 — Frontmatter relationship indexing migration', () => {
|
||||
test('registered in the TS migration registry', async () => {
|
||||
const { migrations, getMigration } = await import('../src/commands/migrations/index.ts');
|
||||
const versions = migrations.map(m => m.version);
|
||||
expect(versions).toContain('0.13.0');
|
||||
const m = getMigration('0.13.0');
|
||||
expect(m).not.toBeNull();
|
||||
expect(typeof m!.orchestrator).toBe('function');
|
||||
});
|
||||
|
||||
test('phase functions exported for unit testing', async () => {
|
||||
const { __testing } = await import('../src/commands/migrations/v0_13_0.ts');
|
||||
expect(typeof __testing.phaseASchema).toBe('function');
|
||||
expect(typeof __testing.phaseBBackfill).toBe('function');
|
||||
expect(typeof __testing.phaseCVerify).toBe('function');
|
||||
});
|
||||
|
||||
test('dry-run skips all side-effect phases', async () => {
|
||||
const { v0_13_0 } = await import('../src/commands/migrations/v0_13_0.ts');
|
||||
const result = await v0_13_0.orchestrator({ yes: true, dryRun: true });
|
||||
expect(result.version).toBe('0.13.0');
|
||||
for (const phase of result.phases) {
|
||||
expect(phase.status).toBe('skipped');
|
||||
expect(phase.detail).toBe('dry-run');
|
||||
}
|
||||
});
|
||||
|
||||
// ── Regression guards (Bug 1) ──────────────────────────────
|
||||
|
||||
test('source does NOT reference process.execPath (Bug 1 regression)', () => {
|
||||
// process.execPath on a bun install is the bun runtime itself, so
|
||||
// `${process.execPath} extract` becomes `bun run extract` and dies.
|
||||
// See v0.14.0 upgrade-night postmortem.
|
||||
const src = readFileSync(SRC_PATH, 'utf-8');
|
||||
expect(src).not.toContain('process.execPath');
|
||||
});
|
||||
|
||||
test('source does NOT build commands from a GBRAIN constant (Bug 1 regression)', () => {
|
||||
// Earlier revisions used `const GBRAIN = process.execPath` and built
|
||||
// commands as `${GBRAIN} extract ...`. The constant was the vector.
|
||||
const src = readFileSync(SRC_PATH, 'utf-8');
|
||||
expect(src).not.toMatch(/const\s+GBRAIN\s*=/);
|
||||
expect(src).not.toMatch(/\$\{GBRAIN\}/);
|
||||
});
|
||||
|
||||
test('phase commands invoke bare `gbrain` shell-out (Bug 1 fix)', () => {
|
||||
const src = readFileSync(SRC_PATH, 'utf-8');
|
||||
// All three phases shell out to bare `gbrain` so the canonical shim
|
||||
// on PATH wins. This is the shape v0_12_0 has always used.
|
||||
expect(src).toContain("execSync('gbrain init --migrate-only'");
|
||||
expect(src).toContain("execSync('gbrain extract links --source db --include-frontmatter'");
|
||||
expect(src).toContain("execSync('gbrain call get_stats'");
|
||||
});
|
||||
|
||||
test('phase commands never reference `bun` or `.ts` paths (Bug 1 regression)', () => {
|
||||
// Belt-and-suspenders: even if someone reintroduces a runtime-path
|
||||
// helper, they must not produce `bun ...` or `<path>.ts` as the spawn
|
||||
// target.
|
||||
const src = readFileSync(SRC_PATH, 'utf-8');
|
||||
expect(src).not.toMatch(/execSync\([^)]*\bbun\b/);
|
||||
expect(src).not.toMatch(/execSync\([^)]*\.ts/);
|
||||
});
|
||||
});
|
||||
@@ -65,6 +65,21 @@ describe('postgres-engine / search path timeout isolation', () => {
|
||||
expect(vector).toMatch(/SET\s+LOCAL\s+statement_timeout/);
|
||||
});
|
||||
|
||||
test('connect() with poolSize honors resolvePrepare (PgBouncer regression guard)', () => {
|
||||
// Regression: worker-instance pools were NOT honoring the prepare decision
|
||||
// before v0.15.4. Module singleton connect() in db.ts was fixed by #284 but
|
||||
// PostgresEngine.connect({poolSize}) (the branch used by `gbrain jobs work`)
|
||||
// silently ignored it — agents running background work against Supabase
|
||||
// pooler URLs still hit `prepared statement "..." does not exist` under
|
||||
// load. Source-level grep is enough: runtime mocking of postgres.js's
|
||||
// tagged-template interface is painful under bun ESM and the wiring is
|
||||
// simple enough that if `resolvePrepare` name appears and a conditional
|
||||
// `prepare` key appears in the options literal, the wire-up is live.
|
||||
const stripped = stripComments(SRC);
|
||||
expect(stripped).toMatch(/db\.resolvePrepare\s*\(\s*url\s*\)/);
|
||||
expect(stripped).toMatch(/typeof\s+prepare\s*===\s*['"]boolean['"]/);
|
||||
});
|
||||
|
||||
test('neither search method clears the timeout with `SET statement_timeout = 0`', () => {
|
||||
// The reset-to-zero pattern was the other half of the leak: if SET
|
||||
// LOCAL is in play, COMMIT handles the reset and an explicit
|
||||
|
||||
75
test/resolve-prepare.test.ts
Normal file
75
test/resolve-prepare.test.ts
Normal file
@@ -0,0 +1,75 @@
|
||||
/**
|
||||
* resolvePrepare precedence tests.
|
||||
*
|
||||
* The helper in src/core/db.ts decides whether to force `prepare: true|false`
|
||||
* on the postgres.js client, or leave it unset (postgres.js default). The
|
||||
* decision matters: on Supabase PgBouncer (port 6543) prepared statements
|
||||
* break under load, but forcing `prepare: false` on direct Postgres loses
|
||||
* plan-cache performance. Precedence ordering (env → URL query → port
|
||||
* auto-detect → default) is enforced here so future edits to resolvePrepare
|
||||
* cannot silently reshuffle the precedence and reintroduce the bug.
|
||||
*/
|
||||
|
||||
import { describe, test, expect, afterEach } from 'bun:test';
|
||||
import { resolvePrepare } from '../src/core/db.ts';
|
||||
|
||||
describe('resolvePrepare', () => {
|
||||
afterEach(() => {
|
||||
delete process.env.GBRAIN_PREPARE;
|
||||
});
|
||||
|
||||
test('returns false for Supabase pooler port 6543', () => {
|
||||
expect(resolvePrepare('postgresql://user:pass@host:6543/db')).toBe(false);
|
||||
});
|
||||
|
||||
test('returns undefined for direct Postgres port 5432', () => {
|
||||
expect(resolvePrepare('postgresql://user:pass@host:5432/db')).toBeUndefined();
|
||||
});
|
||||
|
||||
test('returns undefined for default port (no port specified)', () => {
|
||||
expect(resolvePrepare('postgresql://user:pass@host/db')).toBeUndefined();
|
||||
});
|
||||
|
||||
test('respects ?prepare=false in URL', () => {
|
||||
expect(
|
||||
resolvePrepare('postgresql://user:pass@host:5432/db?prepare=false'),
|
||||
).toBe(false);
|
||||
});
|
||||
|
||||
test('respects ?prepare=true in URL even on port 6543', () => {
|
||||
expect(
|
||||
resolvePrepare('postgresql://user:pass@host:6543/db?prepare=true'),
|
||||
).toBe(true);
|
||||
});
|
||||
|
||||
test('GBRAIN_PREPARE=false overrides everything', () => {
|
||||
process.env.GBRAIN_PREPARE = 'false';
|
||||
expect(
|
||||
resolvePrepare('postgresql://user:pass@host:5432/db?prepare=true'),
|
||||
).toBe(false);
|
||||
});
|
||||
|
||||
test('GBRAIN_PREPARE=true overrides auto-detect on 6543', () => {
|
||||
process.env.GBRAIN_PREPARE = 'true';
|
||||
expect(resolvePrepare('postgresql://user:pass@host:6543/db')).toBe(true);
|
||||
});
|
||||
|
||||
test('GBRAIN_PREPARE=0 is falsy', () => {
|
||||
process.env.GBRAIN_PREPARE = '0';
|
||||
expect(resolvePrepare('postgresql://user:pass@host:6543/db')).toBe(false);
|
||||
});
|
||||
|
||||
test('returns undefined for malformed URL', () => {
|
||||
expect(resolvePrepare('not-a-url')).toBeUndefined();
|
||||
});
|
||||
|
||||
test('handles postgres:// scheme (no ql)', () => {
|
||||
expect(resolvePrepare('postgres://user:pass@host:6543/db')).toBe(false);
|
||||
});
|
||||
|
||||
test('handles URL with encoded special chars in password', () => {
|
||||
expect(
|
||||
resolvePrepare('postgresql://user:p%40ss%24word@host:6543/db'),
|
||||
).toBe(false);
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user