diff --git a/src/commands/sources.ts b/src/commands/sources.ts index 4f8be68..0573a7a 100644 --- a/src/commands/sources.ts +++ b/src/commands/sources.ts @@ -204,9 +204,19 @@ async function runAdd(engine: BrainEngine, args: string[]): Promise { if (federated !== null) config.federated = federated; if (slugPrefixRules) config.slug_prefix_rules = slugPrefixRules; + // Double cast `($4::text)::jsonb` is intentional: postgres-js's `unsafe()` API + // detects `$N::jsonb` casts on string params and re-stringifies the value as a + // JSON-encoded literal, which Postgres then stores as a JSON STRING scalar + // (jsonb_typeof = 'string'). The text cast forces postgres-js to send the + // param verbatim as TEXT, then SQL re-parses to a jsonb object at the column + // boundary. PGLite is unaffected (its driver doesn't have postgres-js's + // auto-encoding) but the double cast is a no-op there. Verified empirically + // on D-LXC fixture 189 (2026-05-07): variant 1 (current pattern) → string, + // variant 4 (this fix) → object. See migration v26 step 0 for the matching + // healing path that recovers existing string-encoded prod data. await engine.executeRaw( `INSERT INTO sources (id, name, local_path, config) - VALUES ($1, $2, $3, $4::jsonb) + VALUES ($1, $2, $3, ($4::text)::jsonb) ON CONFLICT (id) DO NOTHING`, [id, displayName, localPath, JSON.stringify(config)], ); @@ -393,8 +403,12 @@ async function runFederate(engine: BrainEngine, args: string[], value: boolean): } const config = parseConfig(src.config); config.federated = value; + // Double cast `($1::text)::jsonb` matches the runAdd path. See comment there + // for full rationale (postgres-js auto-jsonb-encoding bug). Without this + // cast the UPDATE writes a JSON string scalar (jsonb_typeof = 'string') + // and any subsequent migration using jsonb_set throws SQLSTATE 22023. await engine.executeRaw( - `UPDATE sources SET config = $1::jsonb WHERE id = $2`, + `UPDATE sources SET config = ($1::text)::jsonb WHERE id = $2`, [JSON.stringify(config), id], ); console.log(`Source "${id}" is now ${value ? 'federated (appears in cross-source default search)' : 'isolated (only searched when explicitly named)'}.`); @@ -457,8 +471,12 @@ async function runUpdate(engine: BrainEngine, args: string[]): Promise { delete config.slug_prefix_rules; } + // Double cast `($1::text)::jsonb` matches the runAdd path. See comment there + // for full rationale (postgres-js auto-jsonb-encoding bug). Without this + // cast the UPDATE writes a JSON string scalar (jsonb_typeof = 'string') + // and any subsequent migration using jsonb_set throws SQLSTATE 22023. await engine.executeRaw( - `UPDATE sources SET config = $1::jsonb WHERE id = $2`, + `UPDATE sources SET config = ($1::text)::jsonb WHERE id = $2`, [JSON.stringify(config), id], ); diff --git a/src/commands/sync.ts b/src/commands/sync.ts index 2f747b3..3c852cd 100644 --- a/src/commands/sync.ts +++ b/src/commands/sync.ts @@ -209,6 +209,22 @@ export async function performSync(engine: BrainEngine, opts: SyncOpts): Promise< // No changes if (lastCommit === headCommit) { + // v0.18.2.fork.1: advance last_sync_at even on up-to-date sync. Pre-fix + // this branch returned without touching the source row, so quiet sources + // (no commits since last sync) kept stale last_sync_at forever and the + // drift monitor (gbrain-projects-drift.sh) would falsely flag them as + // stale. Drift's contract is "is the sync cron alive?", not "did the + // remote add commits?" — record the successful sync attempt regardless. + // Only last_sync_at advances here; last_commit is untouched (no semantic + // change to commit anchor). engine.setConfig still records the global + // sync.last_run for legacy non-sourceId callers. + if (opts.sourceId) { + await engine.executeRaw( + `UPDATE sources SET last_sync_at = now() WHERE id = $1`, + [opts.sourceId], + ); + } + await engine.setConfig('sync.last_run', new Date().toISOString()); return { status: 'up_to_date', fromCommit: lastCommit, diff --git a/test/e2e/jsonb-roundtrip.test.ts b/test/e2e/jsonb-roundtrip.test.ts index e744b4e..7c84d33 100644 --- a/test/e2e/jsonb-roundtrip.test.ts +++ b/test/e2e/jsonb-roundtrip.test.ts @@ -126,4 +126,61 @@ describeE2E('E2E: JSONB roundtrip — v0.12.1 reliability wave', () => { expect(source.match(bad)?.[0] ?? null).toBeNull(); } }); + + // v0.18.2.fork.1: sources.ts triple INSERT/UPDATE missed in v0.12.1 wave. + // Different fix variant — the unsafe()-API path uses `$N::jsonb` cast on a + // JSON.stringify'd param (not template-tag `${..}::jsonb`). postgres-js's + // unsafe() detects the cast and re-stringifies the param, landing as a + // JSON STRING scalar (jsonb_typeof = 'string'). v26 migration's jsonb_set + // then throws SQLSTATE 22023 "cannot set path in scalar". + // Fix: `($N::text)::jsonb` double cast forces postgres-js to send param + // verbatim as TEXT, then SQL re-parses to object at column boundary. + // Verified empirically on D-LXC fixture 189 (2026-05-07). + test('sources INSERT writes config as object, not double-encoded string', async () => { + const sql = getConn(); + const { runAdd } = await import('../../src/commands/sources.ts') as any; + const engine = getEngine(); + const testId = 'jsonb-sources-add-' + Math.floor(Math.random() * 1e6); + await runAdd(engine, [testId, '--federated', '--slug-prefix', 'test-prefix/']); + const [row] = await sql` + SELECT jsonb_typeof(config) AS t, + config -> 'federated' AS federated, + config -> 'slug_prefix_rules' AS rules + FROM sources WHERE id = ${testId} + `; + expect(row.t).toBe('object'); + expect(row.federated).toBe(true); + expect(row.rules).toEqual(['test-prefix/']); + await sql`DELETE FROM sources WHERE id = ${testId}`; + }); + + test('sources UPDATE (federate/unfederate) preserves config as object', async () => { + const sql = getConn(); + const { runAdd } = await import('../../src/commands/sources.ts') as any; + const { runFederate } = await import('../../src/commands/sources.ts') as any; + const engine = getEngine(); + const testId = 'jsonb-sources-update-' + Math.floor(Math.random() * 1e6); + await runAdd(engine, [testId, '--federated']); + // Toggle to isolated then back — exercises the runFederate UPDATE path. + if (runFederate) { + await runFederate(engine, [testId], false); + const [row] = await sql` + SELECT jsonb_typeof(config) AS t, config -> 'federated' AS federated + FROM sources WHERE id = ${testId} + `; + expect(row.t).toBe('object'); + expect(row.federated).toBe(false); + } + await sql`DELETE FROM sources WHERE id = ${testId}`; + }); + + test('no $N::jsonb pattern (without ::text intermediate) remains in sources.ts', async () => { + const source = await Bun.file(new URL('../../src/commands/sources.ts', import.meta.url)).text(); + // Match `$::jsonb` not preceded by `::text)` — the bad pattern. + // Allow `($N::text)::jsonb` (the fix). Strip the safe pattern first then check. + const safePattern = /\(\$\d+::text\)::jsonb/g; + const stripped = source.replace(safePattern, ''); + const bad = /\$\d+::jsonb/; + expect(stripped.match(bad)?.[0] ?? null).toBeNull(); + }); }); diff --git a/test/sync-up-to-date-stamping.test.ts b/test/sync-up-to-date-stamping.test.ts new file mode 100644 index 0000000..a0ed59d --- /dev/null +++ b/test/sync-up-to-date-stamping.test.ts @@ -0,0 +1,157 @@ +/** + * v0.18.2.fork.1 — sync.ts up_to_date path advances last_sync_at. + * + * Pre-fix: when `lastCommit === headCommit` (no new commits since last sync), + * performSync early-returned without touching the source row. Quiet sources + * (read-mostly repos like claude-config / personal-knowledge / subagent-writes) + * kept stale `last_sync_at` forever; drift monitor (gbrain-projects-drift.sh) + * false-flagged them as "stale (Nmin ago, threshold 60min)" even though sync + * cron was firing every 10 min and pulling correctly. + * + * Fix: advance last_sync_at on up_to_date path so drift's contract holds: + * "is the sync cron alive?" (real behavior), not "did the remote add commits?" + * (red-herring proxy). + * + * Surfaced 2026-05-07 PW 1 part 2 prod deploy on LXC 107 — first drift tick + * post-deploy reported stock-dashboard "stale 6197min ago" 30 seconds after + * a successful sync tick. + */ + +import { describe, test, expect, beforeAll, afterAll } from 'bun:test'; +import { mkdtempSync, mkdirSync, writeFileSync, rmSync } from 'fs'; +import { join } from 'path'; +import { tmpdir } from 'os'; +import { execFileSync } from 'child_process'; +import { PGLiteEngine } from '../src/core/pglite-engine.ts'; +import { performSync } from '../src/commands/sync.ts'; + +let engine: PGLiteEngine; +const cleanupDirs: string[] = []; + +function git(cwd: string, ...args: string[]): string { + return execFileSync('git', ['-C', cwd, ...args], { encoding: 'utf-8' }).trim(); +} + +function makeFixtureRepo(): string { + const dir = mkdtempSync(join(tmpdir(), 'gbrain-syncstamp-')); + cleanupDirs.push(dir); + git(dir, 'init', '--quiet', '--initial-branch=main'); + git(dir, 'config', 'user.email', 'test@example.com'); + git(dir, 'config', 'user.name', 'test'); + writeFileSync( + join(dir, 'note.md'), + `---\ntitle: note\ntype: note\n---\nbody\n`, + ); + git(dir, 'add', '.'); + git(dir, 'commit', '--quiet', '-m', 'init'); + return dir; +} + +beforeAll(async () => { + engine = new PGLiteEngine(); + await engine.connect({ type: 'pglite' } as never); + await engine.initSchema(); +}); + +afterAll(async () => { + await engine.disconnect(); + for (const d of cleanupDirs) { + try { rmSync(d, { recursive: true, force: true }); } catch { /* best-effort */ } + } +}); + +describe('sync up_to_date path — advances last_sync_at', () => { + test('quiet repo (no new commits) bumps last_sync_at on subsequent sync', async () => { + const repo = makeFixtureRepo(); + const headCommit = git(repo, 'rev-parse', 'HEAD'); + const sourceId = 'syncstamp-quiet'; + + // Provision a source row anchored at HEAD with a stale last_sync_at one + // hour in the past. Mirrors the prod state for quiet sources where the + // last actual write happened well before the next idle tick. + await engine.executeRaw( + `INSERT INTO sources (id, name, local_path, last_commit, last_sync_at, config) + VALUES ($1, $1, $2, $3, now() - interval '1 hour', '{"federated": true}'::jsonb) + ON CONFLICT (id) DO UPDATE SET + local_path = EXCLUDED.local_path, + last_commit = EXCLUDED.last_commit, + last_sync_at = EXCLUDED.last_sync_at`, + [sourceId, repo, headCommit], + ); + + const before = await engine.executeRaw<{ last_sync_at: Date | string }>( + `SELECT last_sync_at FROM sources WHERE id = $1`, + [sourceId], + ); + const beforeMs = new Date(before[0].last_sync_at as string).getTime(); + + // Trigger a sync where HEAD is unchanged from anchor → up_to_date branch. + const result = await performSync(engine, { + repoPath: repo, + noPull: true, + noEmbed: true, + sourceId, + }); + expect(result.status).toBe('up_to_date'); + + const after = await engine.executeRaw<{ last_sync_at: Date | string }>( + `SELECT last_sync_at FROM sources WHERE id = $1`, + [sourceId], + ); + const afterMs = new Date(after[0].last_sync_at as string).getTime(); + + // last_sync_at must advance — was 1h stale, should now be within seconds of now. + expect(afterMs).toBeGreaterThan(beforeMs); + expect(Date.now() - afterMs).toBeLessThan(10_000); // < 10s old + }); + + test('up_to_date does NOT touch last_commit (commit anchor stable)', async () => { + const repo = makeFixtureRepo(); + const headCommit = git(repo, 'rev-parse', 'HEAD'); + const sourceId = 'syncstamp-anchor'; + + await engine.executeRaw( + `INSERT INTO sources (id, name, local_path, last_commit, last_sync_at, config) + VALUES ($1, $1, $2, $3, now() - interval '1 hour', '{"federated": true}'::jsonb) + ON CONFLICT (id) DO UPDATE SET + local_path = EXCLUDED.local_path, + last_commit = EXCLUDED.last_commit, + last_sync_at = EXCLUDED.last_sync_at`, + [sourceId, repo, headCommit], + ); + + await performSync(engine, { + repoPath: repo, + noPull: true, + noEmbed: true, + sourceId, + }); + + const after = await engine.executeRaw<{ last_commit: string }>( + `SELECT last_commit FROM sources WHERE id = $1`, + [sourceId], + ); + expect(after[0].last_commit).toBe(headCommit); + }); + + test('non-sourceId (legacy global config) path: no source UPDATE attempted', async () => { + const repo = makeFixtureRepo(); + const headCommit = git(repo, 'rev-parse', 'HEAD'); + + // Seed legacy global config. + await engine.setConfig('sync.last_commit', headCommit); + await engine.setConfig('sync.repo_path', repo); + + // No sourceId → legacy path. Should not throw, should still record sync.last_run. + const result = await performSync(engine, { + repoPath: repo, + noPull: true, + noEmbed: true, + }); + expect(result.status).toBe('up_to_date'); + + const lastRun = await engine.getConfig('sync.last_run'); + expect(lastRun).not.toBeNull(); + expect(Date.now() - new Date(lastRun!).getTime()).toBeLessThan(10_000); + }); +});