fix(wave): v0.15.1 - 4 hot issues + scope expansion (#248)

* fix(wave): 4 hot issues + 3 scope expansions (v0.13.1)

Addresses four user-filed regressions after v0.13.0 plus three adjacent
footgun closures.

* #170 — CREATE INDEX [CONCURRENTLY] IF NOT EXISTS idx_pages_updated_at_desc
  on pages (updated_at DESC). Engine-aware migration v12 with invalid-index
  cleanup on Postgres, plain CREATE on PGLite. ~700x on 30k+ row brains.
  Contributed by @fuleinist (#215).

* #219 — Minions schema default max_stalled 1 -> 5. v13 migration ALTERs
  the default and UPDATEs existing non-terminal rows (waiting/active/
  delayed/waiting-children/paused) so live queues get rescued on upgrade.
  Adds MinionJobInput.max_stalled with [1,100] clamp. New --max-stalled
  CLI flag on `jobs submit`. Reported by @macbotmini-eng.

* #218 — package.json postinstall surfaces errors instead of silencing.
  trustedDependencies whitelists @electric-sql/pglite. doctor
  schema_version check fails loudly when migrations never ran and links
  to #218. README + INSTALL_FOR_AGENTS warn against `bun install -g`.
  Reported by @gopalpatel.

* #223 — @electric-sql/pglite pinned to exactly 0.4.3 (was ^0.4.4).
  PGLiteEngine.connect() wraps PGlite.create() errors with a message
  pointing at the issue + gbrain doctor. Does NOT suggest 'missing
  migrations' as a cause (create-time abort happens before migrations
  run). Pin is unverified against macOS 26.3; error-wrap is the safety
  net. Reported by @AndreLYL.

* Scope: `gbrain jobs submit` gains --backoff-type/--backoff-delay/
  --backoff-jitter/--timeout-ms/--idempotency-key (MinionJobInput audit).
* Scope: `gbrain jobs smoke --sigkill-rescue` regression case (opt-in,
  CI-only) that simulates a killed worker and asserts the new default
  rescues.
* Scope: `gbrain doctor --index-audit` reports zero-scan Postgres indexes
  as drop candidates (informational; no auto-drop).

Infrastructure:
* Migration interface extended with sqlFor: { postgres?, pglite? } and
  transaction: boolean. Runner picks the engine-specific branch and
  bypasses engine.transaction() when transaction:false (required for
  CONCURRENTLY). BrainEngine.kind readonly discriminator added.
* scripts/check-jsonb-pattern.sh CI guard extended to block
  `max_stalled DEFAULT 1` from regressing.

Tests:
* 15 new unit tests: v12/v13 structural + behavioral assertions,
  max_stalled default/clamp/backfill, PGLite error-wrap source guard,
  engine kind discriminator.
* 3 regression tests pinned by IRON RULE.
* Full unit suite: 1416 pass.
* Full E2E suite against Postgres 16 + pgvector: 126 pass.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* chore: bump version and changelog (v0.13.1)

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* docs: sync documentation for v0.13.1

CLAUDE.md "Key files" and "Commands" sections refreshed to match the
v0.13.1 fix wave:

- Note `BrainEngine.kind` discriminator on engine.ts
- Document v0.13.1 connect() error-wrap on pglite-engine.ts
- Refresh src/core/minions/ layout (no shell handler, no protected-names,
  no quiet-hours/stagger — that was v0.13-development scaffolding that
  did not ship)
- Add src/core/migrate.ts entry with `Migration` interface extensions
  (`sqlFor`, `transaction: false`)
- Document new `gbrain jobs submit` flags (--max-stalled, --backoff-type,
  --backoff-delay, --backoff-jitter, --timeout-ms, --idempotency-key)
- Document `gbrain jobs smoke --sigkill-rescue` regression guard
- Document `gbrain doctor --index-audit` and the schema_version=0
  surface that catches #218 postinstall failures
- Extend check-jsonb-pattern.sh note with the max_stalled DEFAULT 1
  regression guard
- Touch up test file blurbs for migrate.test.ts, pglite-engine.test.ts,
  minions.test.ts with v0.13.1 coverage

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* test(e2e): run files sequentially to eliminate shared-DB race

The E2E suite was flaky. ~3 of every 5 runs had 4-10 failures clustered
in Links, Timeline, Versions, Minions resilience, Parallel Import, and
Page CRUD tests. Symptoms included "expected 16 pages, got 8" (half),
"expected 1 link inserted, got 0", timeline entries missing after
round-trip, and similar data-shape mismatches.

Root cause: bun test runs test FILES in parallel (each in a worker
process). 13 E2E files share one DATABASE_URL, and `setupDB()` in
`test/e2e/helpers.ts` does `TRUNCATE ... CASCADE` on all tables before
each file's `importFixtures()`. File A's TRUNCATE would race with file
B's in-flight INSERT stream, producing the observed half-populated or
wrong-count states.

An earlier attempt used a Postgres advisory lock held on a dedicated
single-connection client for the lifetime of each file's run. It broke
because bun's default 5000 ms hook timeout fires on queued beforeAll()
calls: with 13 files serializing through the lock, files 2-13 would
time out waiting for file 1 to finish.

This commit switches to sequential file execution at the harness level
via scripts/run-e2e.sh, which loops through test/e2e/*.test.ts one at
a time, tracks aggregate pass/fail counts, and exits non-zero on the
first failing file. No lock, no timeout issues, no changes to any test
file. package.json test:e2e points at the new script.

Verified: 5 back-to-back runs against the same Postgres container,
each completing in ~5 min. Every run: 13 files, 138 tests, 0 fails.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* chore: bump version to 0.15.1 (fix wave locked to MINOR line)

Master v0.14.2 was the last /investigate root-cause wave on the
v0.14.x line. This fix wave opens v0.15.x: four hot issues (#170,
#218, #219, #223) close v0.13.x regressions that v0.14.x didn't
cover, so the MINOR bump reflects the semantic shift — new schema
migrations (v14, v15), a new CLI surface (`--max-stalled`,
`--sigkill-rescue`, `--index-audit`), a new BrainEngine contract
(`kind` discriminator + extended `Migration` interface), and a new
install-time contract (PGLite 0.4.3 pin + `trustedDependencies`).

Locked to 0.15.1 in advance: other work may land before/after this
PR, but the version is fixed so reviewers can cite a stable number.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
Garry Tan
2026-04-21 13:19:23 -07:00
committed by GitHub
co-authored by Claude Opus 4.7
parent 7f156c8873
commit ff10796a00
25 changed files with 797 additions and 94 deletions
+3
View File
@@ -50,6 +50,9 @@ export function clampSearchLimit(limit: number | undefined, defaultLimit = 20, c
}
export interface BrainEngine {
/** Discriminator: lets migrations and other consumers branch on engine kind without instanceof + dynamic imports. */
readonly kind: 'postgres' | 'pglite';
// Lifecycle
connect(config: EngineConfig): Promise<void>;
disconnect(): Promise<void>;
+92 -19
View File
@@ -17,7 +17,20 @@ import { slugifyPath } from './sync.ts';
interface Migration {
version: number;
name: string;
/** Engine-agnostic SQL. Used when `sqlFor` is absent. Set to '' for handler-only or sqlFor-only migrations. */
sql: string;
/**
* Engine-specific SQL. If present, overrides `sql` for the matching engine.
* Needed when Postgres wants CONCURRENTLY but PGLite can't honor it.
*/
sqlFor?: { postgres?: string; pglite?: string };
/**
* When false, the runner does NOT wrap the SQL in `engine.transaction()`.
* Required for `CREATE INDEX CONCURRENTLY` (which Postgres refuses inside a transaction).
* Enforced Postgres-only; ignored on PGLite (PGLite has no concurrent writers anyway).
* Defaults to true.
*/
transaction?: boolean;
handler?: (engine: BrainEngine) => Promise<void>;
}
@@ -102,7 +115,7 @@ export const MIGRATIONS: Migration[] = [
backoff_delay INTEGER NOT NULL DEFAULT 1000,
backoff_jitter REAL NOT NULL DEFAULT 0.2,
stalled_counter INTEGER NOT NULL DEFAULT 0,
max_stalled INTEGER NOT NULL DEFAULT 1,
max_stalled INTEGER NOT NULL DEFAULT 5,
lock_token TEXT,
lock_until TIMESTAMPTZ,
delay_until TIMESTAMPTZ,
@@ -355,9 +368,8 @@ export const MIGRATIONS: Migration[] = [
// midnight rollover in the user's TZ naturally creates a new row instead of
// mutating yesterday's. reserved_usd and committed_usd track reservations
// vs actuals so process death between reserve() and commit()/rollback()
// can be cleaned up by TTL scan. status and reserved_at exist for that
// reclaim path. Rollback: DROP TABLE (budget is regenerable from resolver
// call logs; no durable product data lives here).
// can be cleaned up by TTL scan. Rollback: DROP TABLE (regenerable from
// resolver call logs; no durable product data lives here).
sql: `
CREATE TABLE IF NOT EXISTS budget_ledger (
scope TEXT NOT NULL,
@@ -388,16 +400,6 @@ export const MIGRATIONS: Migration[] = [
version: 13,
name: 'minion_quiet_hours_stagger',
// Adds quiet-hours gating + deterministic stagger to Minions.
//
// quiet_hours (JSONB): {start, end, tz, policy} — checked at claim
// time by the worker, not at dispatch. A queued job inside its quiet
// window is released back to 'waiting' and claimed again outside the
// window. 'skip' policy drops the event, 'defer' re-queues.
// stagger_key (TEXT): hashed to a minute-slot offset so jobs with the
// same key don't collide when a cron boundary fires. Optional; NULL
// = no stagger. The hash lives in application code (deterministic,
// ensures same key always lands on same slot) so the column is
// just the key.
sql: `
ALTER TABLE minion_jobs ADD COLUMN IF NOT EXISTS quiet_hours JSONB;
ALTER TABLE minion_jobs ADD COLUMN IF NOT EXISTS stagger_key TEXT;
@@ -405,6 +407,65 @@ export const MIGRATIONS: Migration[] = [
ON minion_jobs(stagger_key) WHERE stagger_key IS NOT NULL;
`,
},
{
version: 14,
name: 'pages_updated_at_index',
// v0.14.1 (fix wave): fixes the 14.6s "list pages newest-first" seqscan on 31k+ row brains.
// Original report: https://github.com/garrytan/gbrain/issues/170 (PR #215).
//
// Engine-aware via handler (not SQL): Postgres uses CREATE INDEX CONCURRENTLY
// to avoid the write-blocking SHARE lock on `pages`. CONCURRENTLY refuses to
// run inside a transaction AND postgres.js's multi-statement `.unsafe()` wraps
// in an implicit transaction, so the handler runs each statement as a separate
// call. A failed CONCURRENTLY leaves an invalid index with the target name;
// the handler pre-drops any invalid remnant via pg_index.indisvalid. PGLite
// has no concurrent writers, so plain CREATE is safe.
sql: '',
handler: async (engine) => {
if (engine.kind === 'postgres') {
await engine.runMigration(
14,
`DO $$ BEGIN
IF EXISTS (
SELECT 1 FROM pg_index i
JOIN pg_class c ON c.oid = i.indexrelid
WHERE c.relname = 'idx_pages_updated_at_desc' AND NOT i.indisvalid
) THEN
EXECUTE 'DROP INDEX CONCURRENTLY IF EXISTS idx_pages_updated_at_desc';
END IF;
END $$;`
);
await engine.runMigration(
14,
`CREATE INDEX CONCURRENTLY IF NOT EXISTS idx_pages_updated_at_desc
ON pages (updated_at DESC);`
);
} else {
await engine.runMigration(
14,
`CREATE INDEX IF NOT EXISTS idx_pages_updated_at_desc
ON pages (updated_at DESC);`
);
}
},
},
{
version: 15,
name: 'minion_jobs_max_stalled_default_5',
// v0.14.1 (fix wave): fixes https://github.com/garrytan/gbrain/issues/219
// Shipped default was 1 — first stall = dead-letter, contradicting the
// "SIGKILL rescued" claim. New default 5. UPDATE backfills existing non-
// terminal rows so upgrading brains don't keep dead-lettering queued work.
// Statuses come from MinionJobStatus in types.ts. Row locks serialize
// against claim()'s FOR UPDATE SKIP LOCKED — race-safe. Idempotent.
sql: `
ALTER TABLE minion_jobs ALTER COLUMN max_stalled SET DEFAULT 5;
UPDATE minion_jobs
SET max_stalled = 5
WHERE status IN ('waiting','active','delayed','waiting-children','paused')
AND max_stalled < 5;
`,
},
];
export const LATEST_VERSION = MIGRATIONS.length > 0
@@ -418,11 +479,23 @@ export async function runMigrations(engine: BrainEngine): Promise<{ applied: num
let applied = 0;
for (const m of MIGRATIONS) {
if (m.version > current) {
// SQL migration (transactional)
if (m.sql) {
await engine.transaction(async (tx) => {
await tx.runMigration(m.version, m.sql);
});
// Pick SQL: engine-specific `sqlFor` wins over engine-agnostic `sql`.
const sql = m.sqlFor?.[engine.kind] ?? m.sql;
if (sql) {
const useTransaction = m.transaction !== false;
// Non-transactional path is Postgres-only: `CREATE INDEX CONCURRENTLY`
// refuses to run inside a transaction. PGLite has no concurrent
// writers, so even if a migration sets transaction:false we wrap it
// anyway (harmless; keeps behavior consistent).
if (useTransaction || engine.kind === 'pglite') {
await engine.transaction(async (tx) => {
await tx.runMigration(m.version, sql);
});
} else {
// Postgres + transaction:false → direct execution, no BEGIN/COMMIT.
await engine.runMigration(m.version, sql);
}
}
// Application-level handler (runs outside transaction for flexibility)
+23 -11
View File
@@ -134,23 +134,34 @@ export class MinionQueue {
// 3. Insert child. Use ON CONFLICT for idempotency; if a concurrent submit
// raced past the fast-path SELECT, the unique index catches it here.
// v12 adds quiet_hours + stagger_key passed through from opts.
const insertSql = opts?.idempotency_key
? `INSERT INTO minion_jobs (name, queue, status, priority, data, max_attempts, backoff_type,
// v13 quiet_hours + stagger_key always present (null fallback; schema
// stores NULL). v15 max_stalled is conditional: provided values get
// clamped to [1, 100] and included in the INSERT; omitted values
// skip the column so the schema DEFAULT (5 as of v0.14.1) kicks in.
// Keeps the app layer from hardcoding the schema default constant.
const hasMaxStalled = opts?.max_stalled !== undefined && opts.max_stalled !== null;
const clampedMaxStalled = hasMaxStalled
? Math.max(1, Math.min(100, Math.floor(opts!.max_stalled as number)))
: null;
const baseCols = `name, queue, status, priority, data, max_attempts, backoff_type,
backoff_delay, backoff_jitter, delay_until, parent_job_id, on_child_fail,
depth, max_children, timeout_ms, remove_on_complete, remove_on_fail, idempotency_key,
quiet_hours, stagger_key)
VALUES ($1, $2, $3, $4, $5::jsonb, $6, $7, $8, $9, $10, $11, $12, $13, $14, $15, $16, $17, $18, $19::jsonb, $20)
quiet_hours, stagger_key`;
const baseVals = `$1, $2, $3, $4, $5::jsonb, $6, $7, $8, $9, $10, $11, $12, $13, $14, $15, $16, $17, $18, $19::jsonb, $20`;
const cols = hasMaxStalled ? `${baseCols}, max_stalled` : baseCols;
const vals = hasMaxStalled ? `${baseVals}, $21` : baseVals;
const insertSql = opts?.idempotency_key
? `INSERT INTO minion_jobs (${cols})
VALUES (${vals})
ON CONFLICT (idempotency_key) WHERE idempotency_key IS NOT NULL DO NOTHING
RETURNING *`
: `INSERT INTO minion_jobs (name, queue, status, priority, data, max_attempts, backoff_type,
backoff_delay, backoff_jitter, delay_until, parent_job_id, on_child_fail,
depth, max_children, timeout_ms, remove_on_complete, remove_on_fail, idempotency_key,
quiet_hours, stagger_key)
VALUES ($1, $2, $3, $4, $5::jsonb, $6, $7, $8, $9, $10, $11, $12, $13, $14, $15, $16, $17, $18, $19::jsonb, $20)
: `INSERT INTO minion_jobs (${cols})
VALUES (${vals})
RETURNING *`;
const params = [
const params: unknown[] = [
jobName,
opts?.queue ?? 'default',
childStatus,
@@ -172,6 +183,7 @@ export class MinionQueue {
opts?.quiet_hours ?? null,
opts?.stagger_key ?? null,
];
if (hasMaxStalled) params.push(clampedMaxStalled);
const inserted = await tx.executeRaw<Record<string, unknown>>(insertSql, params);
+6
View File
@@ -103,6 +103,12 @@ export interface MinionJobInput {
backoff_type?: BackoffType;
backoff_delay?: number;
backoff_jitter?: number;
/**
* Max number of stall windows before dead-letter. Default is the schema
* default (5 as of v0.13.1). Clamped to [1, 100] on insert — values
* outside that range are silently coerced. See migration v13.
*/
max_stalled?: number;
delay?: number; // ms delay before eligible
parent_job_id?: number;
on_child_fail?: ChildFailPolicy;
+27 -4
View File
@@ -24,6 +24,7 @@ import { validateSlug, contentHash, rowToPage, rowToChunk, rowToSearchResult } f
type PGLiteDB = PGlite;
export class PGLiteEngine implements BrainEngine {
readonly kind = 'pglite' as const;
private _db: PGLiteDB | null = null;
private _lock: LockHandle | null = null;
@@ -43,10 +44,32 @@ export class PGLiteEngine implements BrainEngine {
throw new Error('Could not acquire PGLite lock. Another gbrain process is using the database.');
}
this._db = await PGlite.create({
dataDir,
extensions: { vector, pg_trgm },
});
try {
this._db = await PGlite.create({
dataDir,
extensions: { vector, pg_trgm },
});
} catch (err) {
// v0.13.1: any PGLite.create() failure becomes actionable. Most commonly
// this is the macOS 26.3 WASM bug (#223). We deliberately do NOT suggest
// "missing migrations" as a cause — migrations run AFTER create(), so a
// create-time abort has nothing to do with them. Nest the original error
// message so debugging isn't erased.
const original = err instanceof Error ? err.message : String(err);
const wrapped = new Error(
`PGLite failed to initialize its WASM runtime.\n` +
` This is most commonly the macOS 26.3 WASM bug: https://github.com/garrytan/gbrain/issues/223\n` +
` Run \`gbrain doctor\` for a full diagnosis.\n` +
` Original error: ${original}`
);
// Release the lock so a fresh process can try again; leaking the lock
// here turns a recoverable init error into a stuck-brain state.
if (this._lock?.acquired) {
try { await releaseLock(this._lock); } catch { /* ignore cleanup error */ }
this._lock = null;
}
throw wrapped;
}
}
async disconnect(): Promise<void> {
+1 -1
View File
@@ -185,7 +185,7 @@ CREATE TABLE IF NOT EXISTS minion_jobs (
backoff_delay INTEGER NOT NULL DEFAULT 1000,
backoff_jitter REAL NOT NULL DEFAULT 0.2,
stalled_counter INTEGER NOT NULL DEFAULT 0,
max_stalled INTEGER NOT NULL DEFAULT 3,
max_stalled INTEGER NOT NULL DEFAULT 5,
lock_token TEXT,
lock_until TIMESTAMPTZ,
delay_until TIMESTAMPTZ,
+1
View File
@@ -20,6 +20,7 @@ import * as db from './db.ts';
import { validateSlug, contentHash, rowToPage, rowToChunk, rowToSearchResult, parseEmbedding, tryParseEmbedding } from './utils.ts';
export class PostgresEngine implements BrainEngine {
readonly kind = 'postgres' as const;
private _sql: ReturnType<typeof postgres> | null = null;
// Instance connection (for workers) or fall back to module global (backward compat)
+3 -1
View File
@@ -28,6 +28,8 @@ CREATE TABLE IF NOT EXISTS pages (
CREATE INDEX IF NOT EXISTS idx_pages_type ON pages(type);
CREATE INDEX IF NOT EXISTS idx_pages_frontmatter ON pages USING GIN(frontmatter);
CREATE INDEX IF NOT EXISTS idx_pages_trgm ON pages USING GIN(title gin_trgm_ops);
-- v0.13.1 #170: avoids 14.6s seqscan on large brains when listing pages newest-first.
CREATE INDEX IF NOT EXISTS idx_pages_updated_at_desc ON pages (updated_at DESC);
-- ============================================================
-- content_chunks: chunked content with embeddings
@@ -280,7 +282,7 @@ CREATE TABLE IF NOT EXISTS minion_jobs (
backoff_delay INTEGER NOT NULL DEFAULT 1000,
backoff_jitter REAL NOT NULL DEFAULT 0.2,
stalled_counter INTEGER NOT NULL DEFAULT 0,
max_stalled INTEGER NOT NULL DEFAULT 3,
max_stalled INTEGER NOT NULL DEFAULT 5,
lock_token TEXT,
lock_until TIMESTAMPTZ,
delay_until TIMESTAMPTZ,