mirror of
https://github.com/garrytan/gbrain.git
synced 2026-08-15 17:32:37 +00:00
Compare commits
1
Commits
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
39f750e831 |
@@ -438,6 +438,13 @@ export async function runApplyMigrations(args: string[]): Promise<void> {
|
||||
const result = await m.orchestrator(orchestratorOptsFrom(cli));
|
||||
if (result.status === 'failed') {
|
||||
console.error(`Migration v${m.version} reported status=failed.`);
|
||||
// Surface each failed phase's detail — the ledger records it, but
|
||||
// the operator needs it on stderr to act (#921).
|
||||
for (const p of result.phases) {
|
||||
if (p.status === 'failed') {
|
||||
console.error(` phase ${p.name}: ${p.detail ?? '(no detail)'}`);
|
||||
}
|
||||
}
|
||||
// Record the attempt as 'partial' (not 'complete') so the cap counts
|
||||
// it. Don't let a failed orchestrator look like it never ran.
|
||||
try {
|
||||
|
||||
@@ -186,17 +186,6 @@ async function phaseBFenceFacts(
|
||||
const localPathById = new Map<string, string | null>();
|
||||
for (const s of sources) localPathById.set(s.id, s.local_path);
|
||||
|
||||
// Dirty-tree refusal: check every source's local_path before writing.
|
||||
for (const [id, localPath] of localPathById) {
|
||||
if (localPath && isLocalPathDirty(localPath)) {
|
||||
return {
|
||||
name: 'fence_facts',
|
||||
status: 'failed',
|
||||
detail: `source "${id}" has uncommitted changes in ${localPath}. Commit or stash, then re-run.`,
|
||||
};
|
||||
}
|
||||
}
|
||||
|
||||
// Walk legacy rows in (source_id, entity_slug) groups for per-page
|
||||
// atomic writes.
|
||||
const legacy = await engine.executeRaw<LegacyFactRow>(
|
||||
@@ -235,6 +224,21 @@ async function phaseBFenceFacts(
|
||||
groups.set(key, list);
|
||||
}
|
||||
|
||||
// Dirty-tree refusal: check ONLY the sources we are about to write
|
||||
// into. A dirty tree in an unrelated source (or zero fenceable rows
|
||||
// at all) must not block a no-op or a targeted backfill (#927).
|
||||
const targetSourceIds = new Set([...groups.keys()].map(k => k.split('\0')[0]));
|
||||
for (const id of targetSourceIds) {
|
||||
const localPath = localPathById.get(id);
|
||||
if (localPath && isLocalPathDirty(localPath)) {
|
||||
return {
|
||||
name: 'fence_facts',
|
||||
status: 'failed',
|
||||
detail: `source "${id}" has uncommitted changes in ${localPath}. Commit or stash, then re-run.`,
|
||||
};
|
||||
}
|
||||
}
|
||||
|
||||
for (const [key, group] of groups) {
|
||||
const [sourceId, entitySlug] = key.split('\0');
|
||||
const localPath = localPathById.get(sourceId)!;
|
||||
|
||||
@@ -197,7 +197,7 @@ export interface SyncResult {
|
||||
/** Pages re-embedded during this sync's auto-embed step. 0 if --no-embed or skipped. */
|
||||
embedded: number;
|
||||
pagesAffected: string[];
|
||||
failedFiles?: number; // count of per-file import/sync failures (Bug 9)
|
||||
failedFiles?: number; // count of parse failures (Bug 9)
|
||||
/**
|
||||
* v0.41.13.0 partial-sync fields (only set when status === 'partial').
|
||||
*
|
||||
@@ -3183,7 +3183,7 @@ async function performSyncInner(engine: BrainEngine, opts: SyncOpts): Promise<Sy
|
||||
await clearOpCheckpoint(engine, ckpt.target);
|
||||
};
|
||||
|
||||
// issue #1939 adversarial finding #1: a file that failed to import (open ledger
|
||||
// issue #1939 adversarial finding #1: a file that failed to parse (open ledger
|
||||
// row) and is then deleted/renamed-away never re-enters failedFiles and never
|
||||
// imports, so its row would never clear and would age doctor to a permanent
|
||||
// FAIL. Treat removed paths as resolved so the ledger self-heals.
|
||||
@@ -3215,9 +3215,9 @@ async function performSyncInner(engine: BrainEngine, opts: SyncOpts): Promise<Sy
|
||||
} else {
|
||||
const fileFailCount = failedFiles.filter(f => isSkippablePath(f.path)).length;
|
||||
serr(
|
||||
`\nSync blocked: ${fileFailCount} file(s) failed to import:\n` +
|
||||
`\nSync blocked: ${fileFailCount} file(s) failed to parse:\n` +
|
||||
`${codeBreakdown}\n\n` +
|
||||
`Fix the listed file errors and re-run, or use 'gbrain sync --skip-failed' to ` +
|
||||
`Fix the frontmatter and re-run, or use 'gbrain sync --skip-failed' to ` +
|
||||
`acknowledge and move on. A file that keeps failing auto-skips after ` +
|
||||
`${resolveAutoSkipThreshold()} consecutive syncs.`,
|
||||
);
|
||||
@@ -5355,7 +5355,7 @@ function printSyncResult(result: SyncResult, sink: NodeJS.WriteStream = process.
|
||||
case 'dry_run':
|
||||
break; // already printed in performSync
|
||||
case 'blocked_by_failures':
|
||||
write(`Sync BLOCKED at ${result.toCommit.slice(0, 8)}: ${result.failedFiles ?? 0} file(s) failed to import.`);
|
||||
write(`Sync BLOCKED at ${result.toCommit.slice(0, 8)}: ${result.failedFiles ?? 0} file(s) failed to parse.`);
|
||||
write(` See ~/.gbrain/sync-failures.jsonl for details, or run 'gbrain doctor'.`);
|
||||
write(` Fix the files then re-run 'gbrain sync', or 'gbrain sync --skip-failed' to move on.`);
|
||||
break;
|
||||
|
||||
@@ -1058,16 +1058,6 @@ export class PGLiteEngine implements BrainEngine {
|
||||
RETURNING id, source_id, slug, type, title, compiled_truth, timeline, frontmatter, content_hash, created_at, updated_at, effective_date, effective_date_source, import_filename, source_kind, source_uri, ingested_via, ingested_at`,
|
||||
[sourceId, slug, page.type, pageKind, page.title, page.compiled_truth, page.timeline || '', JSON.stringify(frontmatter), hash, effectiveDate, effectiveDateSource, importFilename, chunkerVersion, sourcePath, sourceKind, sourceUri, ingestedVia, ingestedAt]
|
||||
);
|
||||
// #2189: an INSERT … ON CONFLICT DO UPDATE … RETURNING that yields 0 rows
|
||||
// (e.g. a BEFORE trigger suppressing the write) previously crashed in
|
||||
// rowToPage with an opaque "undefined is not an object (row.deleted_at)".
|
||||
// Throw a diagnosable error naming the row instead. Mirrors postgres-engine.ts.
|
||||
if (!rows[0]) {
|
||||
throw new Error(
|
||||
`putPage: INSERT … RETURNING produced no row for slug='${slug}' source_id='${sourceId}'. ` +
|
||||
`A trigger or rule on the pages table may be suppressing the write.`
|
||||
);
|
||||
}
|
||||
return rowToPage(rows[0] as Record<string, unknown>);
|
||||
}
|
||||
|
||||
|
||||
@@ -1119,16 +1119,6 @@ export class PostgresEngine implements BrainEngine {
|
||||
ingested_at = COALESCE(EXCLUDED.ingested_at, pages.ingested_at)
|
||||
RETURNING id, source_id, slug, type, title, compiled_truth, timeline, frontmatter, content_hash, created_at, updated_at, effective_date, effective_date_source, import_filename, source_kind, source_uri, ingested_via, ingested_at
|
||||
`;
|
||||
// #2189: an INSERT … ON CONFLICT DO UPDATE … RETURNING that yields 0 rows
|
||||
// (e.g. a BEFORE trigger suppressing the write) previously crashed in
|
||||
// rowToPage with an opaque "undefined is not an object (row.deleted_at)".
|
||||
// Throw a diagnosable error naming the row instead. Mirrors pglite-engine.ts.
|
||||
if (!rows[0]) {
|
||||
throw new Error(
|
||||
`putPage: INSERT … RETURNING produced no row for slug='${slug}' source_id='${sourceId}'. ` +
|
||||
`A trigger or rule on the pages table may be suppressing the write.`
|
||||
);
|
||||
}
|
||||
return rowToPage(rows[0]);
|
||||
}
|
||||
|
||||
|
||||
@@ -180,3 +180,16 @@ describe('runApplyMigrations exit codes (v0.36.1.x #1062)', () => {
|
||||
expect(src).toMatch(/All migrations up to date[\s\S]{0,80}process\.exit\(0\)/);
|
||||
});
|
||||
});
|
||||
|
||||
// #921: a failed orchestrator must print each failed phase's detail to
|
||||
// stderr — not just "reported status=failed" — so the operator can act
|
||||
// without digging through the ledger.
|
||||
describe('failed migration prints phase detail (#921)', () => {
|
||||
test('runner loops result.phases and console.errors failed phase details', async () => {
|
||||
const { readFileSync } = await import('fs');
|
||||
const src = readFileSync('src/commands/apply-migrations.ts', 'utf8');
|
||||
expect(src).toMatch(
|
||||
/reported status=failed[\s\S]{0,400}for \(const p of result\.phases\)[\s\S]{0,200}p\.status === 'failed'[\s\S]{0,200}console\.error\([\s\S]{0,80}p\.name[\s\S]{0,80}p\.detail/,
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -10,10 +10,11 @@
|
||||
* __setTestEngineOverride so we don't need a configured brain.
|
||||
*/
|
||||
|
||||
import { describe, test, expect, beforeAll, afterAll, beforeEach } from 'bun:test';
|
||||
import { describe, test, expect, beforeAll, afterAll, beforeEach, afterEach } from 'bun:test';
|
||||
import { mkdtempSync, rmSync, existsSync, readFileSync, writeFileSync, mkdirSync } from 'node:fs';
|
||||
import { tmpdir } from 'node:os';
|
||||
import { join } from 'node:path';
|
||||
import { execFileSync } from 'node:child_process';
|
||||
|
||||
import { PGLiteEngine } from '../src/core/pglite-engine.ts';
|
||||
import { v0_32_2, __setTestEngineOverride, __testing } from '../src/commands/migrations/v0_32_2.ts';
|
||||
@@ -238,6 +239,52 @@ describe('phaseBFenceFacts — happy path backfill', () => {
|
||||
});
|
||||
});
|
||||
|
||||
describe('phaseBFenceFacts — dirty-tree refusal scoping (#927)', () => {
|
||||
let dirtyDir: string;
|
||||
|
||||
beforeEach(async () => {
|
||||
// A second source whose local_path is a git repo with uncommitted changes.
|
||||
dirtyDir = mkdtempSync(join(tmpdir(), 'mig-v0_32_2-dirty-'));
|
||||
execFileSync('git', ['-C', dirtyDir, 'init', '-q']);
|
||||
writeFileSync(join(dirtyDir, 'uncommitted.md'), 'dirty', 'utf-8');
|
||||
// eslint-disable-next-line @typescript-eslint/no-explicit-any
|
||||
await (engine as any).db.query(
|
||||
`INSERT INTO sources (id, name, local_path) VALUES ('other', 'other', $1)`,
|
||||
[dirtyDir],
|
||||
);
|
||||
});
|
||||
|
||||
afterEach(async () => {
|
||||
// eslint-disable-next-line @typescript-eslint/no-explicit-any
|
||||
await (engine as any).db.query(`DELETE FROM sources WHERE id = 'other'`);
|
||||
rmSync(dirtyDir, { recursive: true, force: true });
|
||||
});
|
||||
|
||||
test('no legacy facts at all → complete, dirty unrelated source ignored', async () => {
|
||||
const r = await __testing.phaseBFenceFacts(engine, OPTS);
|
||||
expect(r.status).toBe('complete');
|
||||
expect(r.detail).toContain('scanned=0');
|
||||
});
|
||||
|
||||
test('facts scoped to a clean source fence despite dirty unrelated source', async () => {
|
||||
await seedLegacyFact({ entity_slug: 'people/alice', fact: 'Founded Acme' });
|
||||
|
||||
const r = await __testing.phaseBFenceFacts(engine, OPTS);
|
||||
expect(r.status).toBe('complete');
|
||||
expect(r.detail).toContain('fenced=1');
|
||||
expect(existsSync(join(brainDir, 'people/alice.md'))).toBe(true);
|
||||
});
|
||||
|
||||
test('still refuses when the TARGETED source is dirty', async () => {
|
||||
await seedLegacyFact({ entity_slug: 'people/alice', fact: 'F1', source_id: 'other' });
|
||||
|
||||
const r = await __testing.phaseBFenceFacts(engine, OPTS);
|
||||
expect(r.status).toBe('failed');
|
||||
expect(r.detail).toContain('"other"');
|
||||
expect(r.detail).toContain('uncommitted changes');
|
||||
});
|
||||
});
|
||||
|
||||
describe('phaseCVerify', () => {
|
||||
test('returns complete when fence + DB row counts match', async () => {
|
||||
await seedLegacyFact({ entity_slug: 'people/alice', fact: 'F1' });
|
||||
|
||||
@@ -1,74 +0,0 @@
|
||||
// #2189 regression guard: putPage's INSERT … ON CONFLICT DO UPDATE … RETURNING
|
||||
// can yield 0 rows when brain-local DB state (e.g. a BEFORE INSERT trigger)
|
||||
// suppresses the write. Pre-fix, rowToPage(rows[0]) crashed with the opaque
|
||||
// "undefined is not an object (evaluating 'row.deleted_at')" that failed
|
||||
// ~all files of a code sync. Post-fix, putPage throws a descriptive error
|
||||
// naming the slug + source_id so the failure is diagnosable per-file.
|
||||
//
|
||||
// Same guard lands in postgres-engine.ts (engine-parity invariant); this test
|
||||
// exercises the PGLite side, where the issue was reported.
|
||||
|
||||
import { describe, expect, test, beforeAll, afterAll } from 'bun:test';
|
||||
import { PGLiteEngine } from '../src/core/pglite-engine.ts';
|
||||
|
||||
let engine: PGLiteEngine;
|
||||
|
||||
beforeAll(async () => {
|
||||
engine = new PGLiteEngine();
|
||||
await engine.connect({});
|
||||
await engine.initSchema();
|
||||
// Simulate the reporter's state-dependent failure: a trigger that
|
||||
// suppresses inserts for one slug, making RETURNING produce no row.
|
||||
await engine.executeRaw(`
|
||||
CREATE OR REPLACE FUNCTION suppress_pages_insert() RETURNS trigger AS $$
|
||||
BEGIN
|
||||
IF NEW.slug = 'suppressed-page' THEN RETURN NULL; END IF;
|
||||
RETURN NEW;
|
||||
END;
|
||||
$$ LANGUAGE plpgsql;
|
||||
`);
|
||||
await engine.executeRaw(`
|
||||
CREATE TRIGGER suppress_pages_insert_trg
|
||||
BEFORE INSERT ON pages
|
||||
FOR EACH ROW EXECUTE FUNCTION suppress_pages_insert();
|
||||
`);
|
||||
});
|
||||
|
||||
afterAll(async () => {
|
||||
await engine.executeRaw('DROP TRIGGER IF EXISTS suppress_pages_insert_trg ON pages');
|
||||
await engine.executeRaw('DROP FUNCTION IF EXISTS suppress_pages_insert');
|
||||
await engine.disconnect();
|
||||
});
|
||||
|
||||
describe('putPage RETURNING guard (#2189)', () => {
|
||||
test('0-row RETURNING throws a descriptive error, not row.deleted_at TypeError', async () => {
|
||||
let err: Error | undefined;
|
||||
try {
|
||||
await engine.putPage('suppressed-page', {
|
||||
type: 'code',
|
||||
title: 'Suppressed',
|
||||
compiled_truth: 'x',
|
||||
timeline: '',
|
||||
});
|
||||
} catch (e) {
|
||||
err = e as Error;
|
||||
}
|
||||
expect(err).toBeDefined();
|
||||
expect(err!.message).toContain('putPage');
|
||||
expect(err!.message).toContain("slug='suppressed-page'");
|
||||
expect(err!.message).toContain("source_id='default'");
|
||||
// The pre-fix crash signature must be gone.
|
||||
expect(err!.message).not.toContain('deleted_at');
|
||||
});
|
||||
|
||||
test('unsuppressed slugs still upsert normally with the trigger installed', async () => {
|
||||
const page = await engine.putPage('normal-page', {
|
||||
type: 'concept',
|
||||
title: 'Normal',
|
||||
compiled_truth: 'y',
|
||||
timeline: '',
|
||||
});
|
||||
expect(page.slug).toBe('normal-page');
|
||||
expect(page.source_id).toBe('default');
|
||||
});
|
||||
});
|
||||
@@ -375,31 +375,6 @@ describe('performSync dry-run never writes', () => {
|
||||
expect(messages.some(m => m.includes('git pull failed'))).toBe(false);
|
||||
});
|
||||
|
||||
test('first PGLite code sync imports code files without runtime failures', async () => {
|
||||
const { performSync } = await import('../src/commands/sync.ts');
|
||||
mkdirSync(join(repoPath, 'src'), { recursive: true });
|
||||
writeFileSync(
|
||||
join(repoPath, 'src/example.ts'),
|
||||
'export function add(left: number, right: number) { return left + right; }\n',
|
||||
);
|
||||
execSync('git add -A && git commit -m "add code file"', { cwd: repoPath, stdio: 'pipe' });
|
||||
|
||||
const result = await performSync(engine, {
|
||||
repoPath,
|
||||
noPull: true,
|
||||
noEmbed: true,
|
||||
noExtract: true,
|
||||
strategy: 'code',
|
||||
});
|
||||
|
||||
expect(result.status).toBe('first_sync');
|
||||
expect(result.added).toBe(1);
|
||||
expect(result.failedFiles ?? 0).toBe(0);
|
||||
const page = await engine.getPage('src-example-ts');
|
||||
expect(page?.type).toBe('code');
|
||||
expect(page?.frontmatter).toMatchObject({ file: 'src/example.ts', language: 'typescript' });
|
||||
});
|
||||
|
||||
test('incremental dry-run does NOT write to DB or advance the bookmark', async () => {
|
||||
const { performSync } = await import('../src/commands/sync.ts');
|
||||
// First do a real sync to seed the bookmark.
|
||||
|
||||
Reference in New Issue
Block a user