mirror of
https://github.com/garrytan/gbrain.git
synced 2026-07-27 22:15:33 +00:00
* merge master: rebump v0.41.25.0 → v0.41.27.0 (queue collision) Master shipped v0.41.25.0 (#1538 batched sync deletes) and v0.41.26.0 (#1571 dream --source fix) while this branch was in flight. Conflict resolution rebumps to the next available slot. - VERSION: 0.41.25.0 → 0.41.27.0 - package.json: synced - CHANGELOG.md: my v0.41.27.0 entry placed above master's v0.41.26.0 and v0.41.25.0; in-entry version references updated 0.41.25.0 → 0.41.27.0 and forward-references bumped to v0.41.28+. - TODOS.md: kept master's v0.41.20.x section + my v0.41.27.0+ follow-ups No source-file conflicts during the merge. * feat(diagnostics): db-disconnect audit + doctor surface (v0.41.27.0) Instruments every db.disconnect() and PostgresEngine.disconnect() call with a JSONL audit record so the next user-reported #1570 cycle gives us the offender's caller stack instead of the symptomatic "No database connection" error. Audit shape (~/.gbrain/audit/db-disconnect-YYYY-Www.jsonl): {ts, engine_kind, connection_style, caller_stack[], command, pid} - src/core/audit/db-disconnect-audit.ts (NEW): the audit writer, built on the v0.40.4.0 createAuditWriter cathedral. Captures a 6-frame stack via new Error().stack so the offender is readable without spending stderr noise. - src/core/db.ts: logDbDisconnect call at the top of disconnect() (best-effort; never blocks the real teardown). - src/core/postgres-engine.ts: same instrumentation in PostgresEngine.disconnect() — distinguishes 'module' vs 'instance' connection_style so we can tell legitimate worker-pool teardowns apart from the load-bearing module-singleton class. - src/commands/doctor.ts: extends batch_retry_health to surface 24h disconnect count + most-recent caller stack. Warns when the caller frame isn't a known CLI-exit frame (e.g. cli.ts's finally block at the end of an op-dispatch). This is the diagnostic that tells v0.41.28+ where to apply the real ownership fix. - test/db-disconnect-audit.test.ts: unit coverage for the audit writer + caller-stack capture + JSONL shape. - test/e2e/db-singleton-shared-recovery.test.ts: real-Postgres regression that exercises the singleton-null path end-to-end. Refs #1570 * feat(retry): self-heal on null singleton — closes #1570 symptom (v0.41.27.0) withRetry gains an opt-in reconnect callback that fires between the isRetryableConnError classification and the inter-attempt sleep. PostgresEngine.batchRetry injects this.reconnect() — race-safe via the existing _reconnecting guard, handles module and instance pools. Closes the production loss reported in #1570: dream cycles on Supabase no longer drop ~150 link rows per cycle when the singleton goes null mid-batch. The retry now rebuilds the connection between attempts so the second try has somewhere to write to. - src/core/retry.ts: WithRetryOpts gains `reconnect?: () => Promise<void>`. Awaited in the catch branch. onRetry is also now awaited (back-compat- safe: every existing in-tree caller is a sync arrow). Reconnect failures propagate as the real cause — replaces the symptomatic "No database connection" error with whatever the connect() throw was, so operators see the truth. - src/core/postgres-engine.ts:batchRetry — injects `reconnect: () => this.reconnect()`. Covers all 9 batch-retry call sites (addLinksBatch, addTimelineEntriesBatch, upsertChunks, plus the 6 caller-supplied auditSite labels in extract / sync / reindex). - test/core/retry-reconnect.test.ts: 8 hermetic cases pinning the contract — reconnect fires before sleep, only on retryable errors, back-compat when omitted, signal-aborted bypasses reconnect, onRetry is awaited, full success path end-to-end. The deeper bug (who's calling disconnect mid-cycle) is left unaddressed in this commit by design — the diagnostic instrumentation in the prior commit will tell us in the next production run. Refs #1570 * feat(facts): drainPending() + CLI await before disconnect (v0.41.27.0) Closes the silent 'No database connection' tail-end errors after gbrain capture / put_page: the facts:absorb fire-and-forget queue sometimes outlived the CLI process's connection lifetime, so absorb attempts after engine.disconnect() landed in stderr as the GBrainError shape. - src/core/facts/queue.ts: new drainPending({timeout: 1000}) method distinct from shutdown(). Stops accepting new enqueues, awaits in-flight settle, bounded by timeout, returns count of unfinished. Semantically different from shutdown() (which aborts in-flight) so the symptom — drop work that hasn't started yet but let in-flight work finish — matches what CLI exit actually needs. - src/cli.ts: op-dispatch finally block awaits the drain BEFORE engine.disconnect(). Bounded 1s. Opt-out env GBRAIN_NO_FACTS_DRAIN for callers that don't enqueue (keeps fast-exit paths fast). Mirrors the v0.41.8.0 awaitPendingLastRetrievedWrites pattern. - test/facts-queue-drain-pending.test.ts: 6 hermetic cases — empty drain returns immediately, single in-flight settles, timeout bounds wait, shutdown-after-drain is idempotent, post-drain enqueues are dropped, signal-aborted skips waiting. Refs #1570 * docs: update project documentation for v0.41.27.0 README.md: added troubleshooting entry for the v0.41.27.0 retry-reconnect + facts:absorb drain fix (closes #1570), pointing operators at `gbrain doctor --json` to find the offending disconnect caller. CLAUDE.md: extended `src/core/retry.ts` entry with the new optional `reconnect` callback (v0.41.27.0); added two new Key Files entries for `src/core/audit/db-disconnect-audit.ts` (the diagnostic half of the "instrument first, fix later" pivot) and `FactsQueue.drainPending`; extended `doctor.ts:checkBatchRetryHealth` entry with the in-place extension that surfaces 24h disconnect-call count. llms-full.txt: regenerated to absorb CLAUDE.md edits. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * chore: rebump v0.41.27.0 → v0.41.28.0 (queue collision with #1573) Master shipped v0.41.27.0 (#1573 git-aware sync_freshness) claiming the same slot. Rebump to the next available version. - VERSION + package.json → 0.41.28.0 - CHANGELOG.md: my entry header + in-entry refs 0.41.27.0 → 0.41.28.0 - TODOS.md: my #1570 follow-up section header + body refs bumped * test: pin gateway in put-page-provenance + embedding-dim-check (CI shard fix) Both files failed on CI shards 1 and 8 under the cross-file gateway-state leak class (CLAUDE.md "Test-isolation lint and helpers"). The v0.41.28.0 merge reshuffled the weight-based shard bin-packing, landing a gateway-mutating sibling ahead of these two victims in the same `bun test` process. Mechanism: - put-page-provenance: put_page embeds via the gateway. A sibling left the gateway configured with OpenAI + the CI placeholder `sk-test` (captured at configureGateway time, survives the withEnv restore as cached gateway state). put_page's embed then fired against live OpenAI and 401'd. The bunfig legacy-embedding preload's beforeEach only re-applies legacy when the gateway was RESET — it does NOT correct a sibling that configured a different LIVE config. - embedding-dim-check: initSchema builds the content_chunks vector column at the gateway's configured dim. A sibling leaking ZE/1280 made the column 1280-d, so `expect(dims).toBe(1536)` failed. Fix (victim-side pinning, the escape hatch the preload documents): - Both: configure the gateway explicitly in beforeAll BEFORE initSchema (OpenAI/1536), resetGateway() in afterAll so neither leaks onward. - put-page-provenance also stubs the embed transport via __setEmbedTransportForTests so embed is deterministic and offline; a dummy OPENAI_API_KEY is supplied in the gateway env because instantiateEmbedding builds the OpenAI client (key check) BEFORE the stubbed transport is reached — the stub then intercepts the actual call so the key never leaves the process. Verified: CI shards 1 (1337 pass) + 8 (905 pass) green with OPENAI_API_KEY unset, plus adversarial sibling orderings (gateway.test / doctor-ze-checks preceding). Typecheck + check-test-isolation clean. --------- Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>
141 lines
5.6 KiB
TypeScript
141 lines
5.6 KiB
TypeScript
/**
|
|
* v0.41.25.0 (#1570) — focused regression test for the dream-cycle
|
|
* row-loss bug class. Each case pins a real production failure mode
|
|
* codex recommended pinning (codex finding 4: instrument + targeted
|
|
* regression test, not architectural refactor).
|
|
*
|
|
* Skipped when DATABASE_URL is unset — mirrors every other test/e2e/
|
|
* file's posture. Caller is expected to bring up gbrain-test-pg via
|
|
* the canonical lifecycle described in CLAUDE.md.
|
|
*/
|
|
|
|
import { describe, test, expect, beforeAll, afterAll, beforeEach } from 'bun:test';
|
|
import * as fs from 'node:fs';
|
|
import * as path from 'node:path';
|
|
import * as os from 'node:os';
|
|
import { PostgresEngine } from '../../src/core/postgres-engine.ts';
|
|
import * as db from '../../src/core/db.ts';
|
|
import { withEnv } from '../helpers/with-env.ts';
|
|
import {
|
|
readRecentDbDisconnects,
|
|
logDbDisconnect,
|
|
} from '../../src/core/audit/db-disconnect-audit.ts';
|
|
|
|
const DATABASE_URL = process.env.DATABASE_URL;
|
|
const skip = !DATABASE_URL;
|
|
|
|
if (skip) {
|
|
// eslint-disable-next-line no-console
|
|
console.log('Skipping db-singleton-shared-recovery E2E (DATABASE_URL not set)');
|
|
}
|
|
|
|
describe.skipIf(skip)('v0.41.25.0 db-singleton shared-recovery regressions (#1570)', () => {
|
|
let tmpAuditDir: string;
|
|
|
|
beforeAll(async () => {
|
|
// Fresh module-level connection so each test starts from a known state.
|
|
await db.disconnect();
|
|
await db.connect({ database_url: DATABASE_URL! });
|
|
}, 30_000);
|
|
|
|
afterAll(async () => {
|
|
await db.disconnect();
|
|
if (tmpAuditDir) {
|
|
try { fs.rmSync(tmpAuditDir, { recursive: true, force: true }); } catch { /* ignore */ }
|
|
}
|
|
});
|
|
|
|
beforeEach(() => {
|
|
tmpAuditDir = fs.mkdtempSync(path.join(os.tmpdir(), 'gbrain-1570-e2e-'));
|
|
});
|
|
|
|
test('CASE 1: shared singleton survives mid-operation disconnect via retry reconnect', async () => {
|
|
// Reproduce the dream-cycle scenario: caller A is mid-batch, caller B
|
|
// disconnects the module singleton, caller A's NEXT attempt enters
|
|
// retry and the reconnect callback rebuilds the singleton before the
|
|
// retry's fn fires. This is the symptom-fix contract we ship.
|
|
await db.connect({ database_url: DATABASE_URL! });
|
|
|
|
const engineA = new PostgresEngine();
|
|
await engineA.connect({ database_url: DATABASE_URL! });
|
|
const engineB = new PostgresEngine();
|
|
await engineB.connect({ database_url: DATABASE_URL! });
|
|
|
|
// Sanity: both engines share the live singleton.
|
|
expect((await engineA.sql`SELECT 1 as ok`)[0].ok).toBe(1);
|
|
expect((await engineB.sql`SELECT 1 as ok`)[0].ok).toBe(1);
|
|
|
|
// Engine B disconnects mid-operation (the "offending caller" scenario).
|
|
// This nulls the module singleton for engine A too.
|
|
await engineB.disconnect();
|
|
|
|
// Engine A's direct unsafe call will throw — proving the bug class
|
|
// exists at the engine.sql layer.
|
|
let directThrew = false;
|
|
try {
|
|
await engineA.sql`SELECT 1`;
|
|
} catch {
|
|
directThrew = true;
|
|
}
|
|
expect(directThrew).toBe(true);
|
|
|
|
// The retry layer's reconnect callback recovers. We exercise it via
|
|
// engine.reconnect() directly (which is what batchRetry's injected
|
|
// reconnect callback calls). After reconnect, engine A's next call
|
|
// succeeds.
|
|
await engineA.reconnect();
|
|
const afterRecovery = await engineA.sql`SELECT 1 as ok`;
|
|
expect(afterRecovery[0].ok).toBe(1);
|
|
|
|
// Cleanup
|
|
await engineA.disconnect();
|
|
});
|
|
|
|
test('CASE 2: diagnostic audit records every mid-process disconnect call', async () => {
|
|
// Per codex finding 4: instrument first. Production data tells us
|
|
// which caller is firing the mid-process disconnect. This case pins
|
|
// that the instrumentation is wired correctly: a disconnect call
|
|
// emits an audit JSONL line containing connection_style + caller_stack.
|
|
await withEnv({ GBRAIN_AUDIT_DIR: tmpAuditDir }, async () => {
|
|
await db.connect({ database_url: DATABASE_URL! });
|
|
const engine = new PostgresEngine();
|
|
await engine.connect({ database_url: DATABASE_URL! });
|
|
// module-style engine.disconnect() should log an audit line.
|
|
await engine.disconnect();
|
|
|
|
// Read it back. doctor uses the same readRecentDbDisconnects path.
|
|
const result = readRecentDbDisconnects(24);
|
|
expect(result.count).toBeGreaterThanOrEqual(1);
|
|
const last = result.events[0];
|
|
expect(last.engine_kind).toBe('postgres');
|
|
expect(['module', 'unknown']).toContain(last.connection_style);
|
|
expect(last.caller_stack.length).toBeGreaterThan(0);
|
|
expect(last.pid).toBe(process.pid);
|
|
});
|
|
});
|
|
|
|
test('CASE 3: instance-pool disconnect leaves shared singleton ALIVE for other callers', async () => {
|
|
// Codex finding 5/6: BrainEngine contract is asymmetric across engines.
|
|
// Instance-pool engines (workerPoolSize set) should NEVER touch the
|
|
// module singleton on disconnect. This case pins that contract —
|
|
// existing v0.28.1 idempotency test covers the same shape but here
|
|
// we explicitly verify the "two callers, one in instance mode" case
|
|
// matters for #1570.
|
|
await db.connect({ database_url: DATABASE_URL! });
|
|
const moduleEngine = new PostgresEngine();
|
|
await moduleEngine.connect({ database_url: DATABASE_URL! }); // module mode
|
|
|
|
const workerEngine = new PostgresEngine();
|
|
await workerEngine.connect({ database_url: DATABASE_URL!, poolSize: 2 }); // instance mode
|
|
|
|
// Worker disconnect: should ONLY tear down its own _sql, not touch module.
|
|
await workerEngine.disconnect();
|
|
|
|
// Module engine still works.
|
|
const result = await moduleEngine.sql`SELECT 1 as ok`;
|
|
expect(result[0].ok).toBe(1);
|
|
|
|
await moduleEngine.disconnect();
|
|
});
|
|
});
|