mirror of
https://github.com/garrytan/gbrain.git
synced 2026-07-27 22:15:33 +00:00
fix(cli,config,doctor): CLI/config UX wave — config-get file plane, idempotent archive, honest help + doctor text (#2120 #2792 #1175 #1123 #2451) (#2918)
* fix(cli,config,doctor): CLI/config UX wave — config-get file plane, idempotent archive, honest help + doctor text, prefixed model defaults (#2120 #2792 #1175 #1123 #2451) - config get resolves the file/env plane before the DB plane (runtime precedence) and reports provenance on stderr; stdout stays a bare value. - sources archive distinguishes already-archived (friendly no-op, exit 0) from not-found (clear exit-4 error). - gbrain --help SOURCES block now lists archive/restore/archived/purge/status plus a pointer at `sources --help` for the long tail. - multi_source_drift doctor advice references only real CLI surfaces (drops the never-built 'sources rehome'; pins delete to GBRAIN_SOURCE=default). - #2451 (bare model ids in calibration defaults) verified already fixed + tested on master by #2892 — no change needed. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * test: satisfy test-isolation gate for wave-B tests check-test-isolation R1 forbids direct process.env mutation in non-serial unit tests (env leaks across files sharing a shard process). Route the GBRAIN_HOME / GBRAIN_CHAT_MODEL / GBRAIN_PGLITE_SNAPSHOT overrides through the canonical withEnv() helper instead. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Sinabina <sinabina@Sinabinas-MacBook-Pro-4.local> Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
Time Attakc
co-authored by
Sinabina
Claude Fable 5
Sinabina
Claude Fable 5
parent
9aaa3be05f
commit
78bc2fef09
+8
-1
@@ -2315,7 +2315,14 @@ BRAIN (capture / ideate / explore — v0.37/v0.38)
|
||||
SOURCES (multi-repo / multi-brain)
|
||||
sources list Show registered sources
|
||||
sources add <id> --path <p> Register a source (id = short name, e.g. 'wiki')
|
||||
sources remove <id> Remove a source + its pages
|
||||
sources remove <id> Remove a source + its pages (--confirm-destructive)
|
||||
sources archive <id> Soft-delete: hide from search, recoverable for 72h
|
||||
sources restore <id> Un-archive a soft-deleted source
|
||||
sources archived List soft-deleted sources and their purge expiry
|
||||
sources purge [<id>] Permanently delete archived sources
|
||||
sources status Per-source dashboard (sync lag, embed coverage)
|
||||
sources --help Full subcommand list (rename, default, attach,
|
||||
current, federate, set-cr-mode, webhook, harden, ...)
|
||||
sync --all Sync all sources with a local_path
|
||||
sync --source <id> Sync one specific source
|
||||
repos ... DEPRECATED alias for 'sources' (v0.19.0)
|
||||
|
||||
+19
-3
@@ -98,9 +98,25 @@ export async function runConfig(engine: BrainEngine, args: string[]) {
|
||||
const value = args[2];
|
||||
|
||||
if (action === 'get' && key) {
|
||||
const val = await engine.getConfig(key);
|
||||
if (val !== null) {
|
||||
console.log(val);
|
||||
// #2120: `get` used to read only the DB plane, so a runtime-effective key
|
||||
// in ~/.gbrain/config.json (or env) reported not-found. Resolve the way
|
||||
// the runtime does — env/file plane wins over DB (loadConfig() already
|
||||
// overlays env onto the file) — and report which plane answered on
|
||||
// stderr, keeping stdout a bare value for scripts.
|
||||
const filePlane = loadConfig() as Record<string, unknown> | null;
|
||||
const fileVal = filePlane?.[key];
|
||||
const dbVal = await engine.getConfig(key);
|
||||
const val = fileVal !== undefined && fileVal !== null ? fileVal : dbVal;
|
||||
if (val !== null && val !== undefined) {
|
||||
console.log(typeof val === 'string' ? val : JSON.stringify(val));
|
||||
if (fileVal !== undefined && fileVal !== null) {
|
||||
const shadow = dbVal !== null && dbVal !== undefined
|
||||
? ' — a DB-plane value also exists and is shadowed at runtime'
|
||||
: '';
|
||||
console.error(`[config] source: file/env plane (~/.gbrain/config.json or env)${shadow}`);
|
||||
} else {
|
||||
console.error(`[config] source: db plane`);
|
||||
}
|
||||
} else {
|
||||
console.error(`Config key not found: ${key}`);
|
||||
process.exit(1);
|
||||
|
||||
+22
-7
@@ -5138,13 +5138,7 @@ export async function buildChecks(
|
||||
checks.push({
|
||||
name: 'multi_source_drift',
|
||||
status: 'warn',
|
||||
message:
|
||||
`${result.count} page slug(s) appear at 'default' but NOT at the intended source ` +
|
||||
`(e.g., ${sampleStr}). Two possible causes: (1) pre-v0.30.3 putPage misroutes; ` +
|
||||
`(2) source X never completed initial sync and the default page is unrelated. ` +
|
||||
`Verify with 'gbrain sources status', then either re-sync with ` +
|
||||
`'gbrain sync --source <id> --full' or 'gbrain delete <slug>' if the default-source ` +
|
||||
`row is the misroute. (A 'gbrain sources rehome' cleanup command is tracked for v0.32.0.)`,
|
||||
message: multiSourceDriftAdvice(result.count, sampleStr),
|
||||
});
|
||||
} else {
|
||||
checks.push({
|
||||
@@ -8074,3 +8068,24 @@ async function checkSchemaPackSourceDrift(engine: BrainEngine): Promise<Check> {
|
||||
};
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* #1123 — multi_source_drift remediation advice. Exported so the regression
|
||||
* test can pin that it only references CLI surfaces that actually exist
|
||||
* (the pre-fix text pointed at 'gbrain sources rehome', which was never
|
||||
* built, and at 'gbrain delete <slug>' without explaining that delete
|
||||
* targets the ACTIVE source — following it literally on a multi-source
|
||||
* brain deletes the correctly-routed row).
|
||||
*/
|
||||
export function multiSourceDriftAdvice(count: number, sampleStr: string): string {
|
||||
return (
|
||||
`${count} page slug(s) appear at 'default' but NOT at the intended source ` +
|
||||
`(e.g., ${sampleStr}). Two possible causes: (1) pre-v0.30.3 putPage misroutes; ` +
|
||||
`(2) the intended source never completed initial sync and the default page is unrelated. ` +
|
||||
`Verify with 'gbrain sources status', then re-sync with ` +
|
||||
`'gbrain sync --source <id> --full' (reconciles drift without deleting data). ` +
|
||||
`If a misrouted default-source row remains after re-sync, remove it with ` +
|
||||
`'GBRAIN_SOURCE=default gbrain delete <slug>' — delete targets the active source, ` +
|
||||
`so pin it to 'default' explicitly.`
|
||||
);
|
||||
}
|
||||
|
||||
@@ -502,6 +502,19 @@ async function runArchive(engine: BrainEngine, args: string[]): Promise<void> {
|
||||
|
||||
const result = await softDeleteSource(engine, id);
|
||||
if (!result) {
|
||||
// #2792: softDeleteSource returns null both for "not found" (handled by
|
||||
// the impact check above) and for "already archived" (UPDATE matched no
|
||||
// `archived = false` row). Distinguish them: already-archived is a
|
||||
// friendly idempotent no-op, not a reasonless failure.
|
||||
const rows = await engine.executeRaw<{ archived: boolean }>(
|
||||
`SELECT archived FROM sources WHERE id = $1`,
|
||||
[id],
|
||||
);
|
||||
if (rows[0]?.archived) {
|
||||
console.log(`Source "${id}" is already archived — nothing to do.`);
|
||||
console.log(` 'gbrain sources archived' shows its purge expiry; 'gbrain sources restore ${id}' un-archives it.`);
|
||||
return;
|
||||
}
|
||||
console.error(`Failed to archive source "${id}".`);
|
||||
process.exit(4);
|
||||
}
|
||||
|
||||
@@ -97,3 +97,19 @@ describe('WARN-6 — main `gbrain --help` lists capture/brainstorm/lsd', () => {
|
||||
expect(stdout).toContain('embed');
|
||||
});
|
||||
});
|
||||
|
||||
describe('#1175 — main `gbrain --help` SOURCES block matches the real subcommand set', () => {
|
||||
test('archive and its lifecycle siblings are listed', () => {
|
||||
const { stdout, status } = runCli(['--help']);
|
||||
expect(status).toBe(0);
|
||||
// Pre-fix the SOURCES block listed only list/add/remove; the soft-delete
|
||||
// alternative that `sources remove` itself recommends was undiscoverable.
|
||||
expect(stdout).toMatch(/^\s*sources archive <id>\s/m);
|
||||
expect(stdout).toMatch(/^\s*sources restore <id>\s/m);
|
||||
expect(stdout).toMatch(/^\s*sources archived\s/m);
|
||||
expect(stdout).toMatch(/^\s*sources purge/m);
|
||||
expect(stdout).toMatch(/^\s*sources status\s/m);
|
||||
// Pointer at the full per-subcommand help for the long tail.
|
||||
expect(stdout).toMatch(/^\s*sources --help\s/m);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -0,0 +1,93 @@
|
||||
/**
|
||||
* #2120 — `gbrain config get` must resolve the file/env plane, not just the
|
||||
* DB plane. Pre-fix, `get` called only `engine.getConfig(key)`, so a
|
||||
* runtime-effective key in ~/.gbrain/config.json reported not-found while
|
||||
* the runtime happily used it.
|
||||
*
|
||||
* Hermetic: GBRAIN_HOME points at a tmp dir (configDir() honors it) via
|
||||
* withEnv (check-test-isolation R1), and the engine is a getConfig-only
|
||||
* stub — the `get` path touches nothing else.
|
||||
*/
|
||||
|
||||
import { describe, test, expect, spyOn } from 'bun:test';
|
||||
import { mkdtempSync, writeFileSync, mkdirSync } from 'node:fs';
|
||||
import { join } from 'node:path';
|
||||
import { tmpdir } from 'node:os';
|
||||
import { runConfig } from '../src/commands/config.ts';
|
||||
import { withEnv } from './helpers/with-env.ts';
|
||||
import type { BrainEngine } from '../src/core/engine.ts';
|
||||
|
||||
const home = mkdtempSync(join(tmpdir(), 'gbrain-config-get-'));
|
||||
mkdirSync(join(home, '.gbrain'), { recursive: true });
|
||||
|
||||
function stubEngine(dbValues: Record<string, string>): BrainEngine {
|
||||
return {
|
||||
getConfig: async (key: string) => dbValues[key] ?? null,
|
||||
} as unknown as BrainEngine;
|
||||
}
|
||||
|
||||
function writeFileConfig(cfg: Record<string, unknown>): void {
|
||||
writeFileSync(join(home, '.gbrain', 'config.json'), JSON.stringify(cfg));
|
||||
}
|
||||
|
||||
/** Run `config get <key>` with GBRAIN_HOME pinned to the tmp brain and the
|
||||
* chat-model env overlay cleared, capturing output + exit code. */
|
||||
async function runGet(
|
||||
dbValues: Record<string, string>,
|
||||
key: string,
|
||||
): Promise<{ logs: string[]; errs: string[]; exit: number | null }> {
|
||||
const logs: string[] = [];
|
||||
const errs: string[] = [];
|
||||
let exit: number | null = null;
|
||||
const logSpy = spyOn(console, 'log').mockImplementation((...a: unknown[]) => { logs.push(a.join(' ')); });
|
||||
const errSpy = spyOn(console, 'error').mockImplementation((...a: unknown[]) => { errs.push(a.join(' ')); });
|
||||
const exitSpy = spyOn(process, 'exit').mockImplementation(((code?: number) => {
|
||||
exit = code ?? 0;
|
||||
throw new Error(`EXIT:${code}`);
|
||||
}) as never);
|
||||
try {
|
||||
await withEnv(
|
||||
{ GBRAIN_HOME: home, GBRAIN_CHAT_MODEL: undefined },
|
||||
() => runConfig(stubEngine(dbValues), ['get', key]),
|
||||
);
|
||||
} catch (e) {
|
||||
if (!(e as Error).message.startsWith('EXIT:')) throw e;
|
||||
} finally {
|
||||
logSpy.mockRestore();
|
||||
errSpy.mockRestore();
|
||||
exitSpy.mockRestore();
|
||||
}
|
||||
return { logs, errs, exit };
|
||||
}
|
||||
|
||||
describe('#2120 — config get resolves file plane with DB fallback', () => {
|
||||
test('file-plane key with no DB row is found (the pre-fix not-found bug)', async () => {
|
||||
writeFileConfig({ engine: 'pglite', chat_model: 'anthropic:claude-sonnet-4-6' });
|
||||
const { logs, errs, exit } = await runGet({}, 'chat_model');
|
||||
expect(exit).toBeNull();
|
||||
expect(logs).toContain('anthropic:claude-sonnet-4-6');
|
||||
expect(errs.join('\n')).toContain('file/env plane');
|
||||
});
|
||||
|
||||
test('file plane wins over DB plane (matches runtime precedence) and reports the shadow', async () => {
|
||||
writeFileConfig({ engine: 'pglite', chat_model: 'anthropic:claude-sonnet-4-6' });
|
||||
const { logs, errs } = await runGet({ chat_model: 'openai:gpt-5' }, 'chat_model');
|
||||
expect(logs).toContain('anthropic:claude-sonnet-4-6');
|
||||
expect(logs).not.toContain('openai:gpt-5');
|
||||
expect(errs.join('\n')).toContain('shadowed');
|
||||
});
|
||||
|
||||
test('DB-plane-only key still resolves (no regression for dotted keys)', async () => {
|
||||
writeFileConfig({ engine: 'pglite' });
|
||||
const { logs, errs } = await runGet({ 'search.mode': 'balanced' }, 'search.mode');
|
||||
expect(logs).toContain('balanced');
|
||||
expect(errs.join('\n')).toContain('db plane');
|
||||
});
|
||||
|
||||
test('key in neither plane is still not-found (exit 1)', async () => {
|
||||
writeFileConfig({ engine: 'pglite' });
|
||||
const { errs, exit } = await runGet({}, 'chat_model');
|
||||
expect(exit).toBe(1);
|
||||
expect(errs.join('\n')).toContain('Config key not found: chat_model');
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,33 @@
|
||||
/**
|
||||
* #1123 — the multi_source_drift doctor recommendation must only reference
|
||||
* CLI surfaces that actually exist. Pre-fix it pointed at
|
||||
* 'gbrain sources rehome' (never built) and at 'gbrain delete <slug>'
|
||||
* without saying that delete targets the ACTIVE source — following it
|
||||
* literally on a multi-source brain deletes the correctly-routed row.
|
||||
*/
|
||||
|
||||
import { describe, test, expect } from 'bun:test';
|
||||
import { multiSourceDriftAdvice } from '../src/commands/doctor.ts';
|
||||
|
||||
describe('#1123 — multiSourceDriftAdvice references only real surfaces', () => {
|
||||
const advice = multiSourceDriftAdvice(45, 'foo (intended=wiki)');
|
||||
|
||||
test('carries the count and sample', () => {
|
||||
expect(advice).toContain('45 page slug(s)');
|
||||
expect(advice).toContain('foo (intended=wiki)');
|
||||
});
|
||||
|
||||
test('points at the re-sync path that reconciles drift', () => {
|
||||
expect(advice).toContain("gbrain sources status");
|
||||
expect(advice).toContain("gbrain sync --source <id> --full");
|
||||
});
|
||||
|
||||
test('does not reference the never-built rehome command', () => {
|
||||
expect(advice).not.toContain('rehome');
|
||||
});
|
||||
|
||||
test('delete advice pins the source explicitly instead of implying delete targets default', () => {
|
||||
expect(advice).toContain('GBRAIN_SOURCE=default gbrain delete <slug>');
|
||||
expect(advice).not.toContain('delete --source');
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,81 @@
|
||||
/**
|
||||
* #2792 — `gbrain sources archive` idempotency.
|
||||
*
|
||||
* Pre-fix, archiving an already-archived source collapsed into the same
|
||||
* reasonless "Failed to archive" exit-4 path as a DB error, pushing operators
|
||||
* toward the destructive `sources remove`. Already-archived is now a friendly
|
||||
* no-op (exit 0); not-found stays a clear exit-4 error.
|
||||
*
|
||||
* Runs against PGLite like test/destructive-guard.test.ts (same contract on
|
||||
* Postgres; PGLite is fast + DATABASE_URL-free).
|
||||
*/
|
||||
|
||||
import { describe, test, expect, beforeAll, afterAll, spyOn } from 'bun:test';
|
||||
import { PGLiteEngine } from '../src/core/pglite-engine.ts';
|
||||
import { runSources } from '../src/commands/sources.ts';
|
||||
import { withEnv } from './helpers/with-env.ts';
|
||||
|
||||
let engine: PGLiteEngine;
|
||||
|
||||
beforeAll(async () => {
|
||||
engine = new PGLiteEngine();
|
||||
// Cold-init schema path so the archive columns exist (same intent as
|
||||
// destructive-guard tests, but env-isolated per check-test-isolation R1).
|
||||
await withEnv({ GBRAIN_PGLITE_SNAPSHOT: undefined }, async () => {
|
||||
await engine.connect({});
|
||||
await engine.initSchema();
|
||||
});
|
||||
await engine.executeRaw(
|
||||
`INSERT INTO sources (id, name) VALUES ($1, $2) ON CONFLICT (id) DO NOTHING`,
|
||||
['arch-idem', 'arch-idem'],
|
||||
);
|
||||
});
|
||||
|
||||
afterAll(async () => {
|
||||
await engine.disconnect();
|
||||
});
|
||||
|
||||
function captureRun(args: string[]): Promise<{ logs: string[]; errs: string[]; exit: number | null }> {
|
||||
const logs: string[] = [];
|
||||
const errs: string[] = [];
|
||||
const logSpy = spyOn(console, 'log').mockImplementation((...a: unknown[]) => { logs.push(a.join(' ')); });
|
||||
const errSpy = spyOn(console, 'error').mockImplementation((...a: unknown[]) => { errs.push(a.join(' ')); });
|
||||
let exit: number | null = null;
|
||||
const exitSpy = spyOn(process, 'exit').mockImplementation(((code?: number) => {
|
||||
exit = code ?? 0;
|
||||
throw new Error(`EXIT:${code}`);
|
||||
}) as never);
|
||||
return runSources(engine, args)
|
||||
.catch((e: Error) => {
|
||||
if (!e.message.startsWith('EXIT:')) throw e;
|
||||
})
|
||||
.then(() => ({ logs, errs, exit }))
|
||||
.finally(() => {
|
||||
logSpy.mockRestore();
|
||||
errSpy.mockRestore();
|
||||
exitSpy.mockRestore();
|
||||
});
|
||||
}
|
||||
|
||||
describe('#2792 — sources archive is idempotent', () => {
|
||||
test('first archive succeeds', async () => {
|
||||
const { exit, logs } = await captureRun(['archive', 'arch-idem']);
|
||||
expect(exit).toBeNull();
|
||||
expect(logs.join('\n')).toContain('arch-idem');
|
||||
});
|
||||
|
||||
test('second archive is a friendly no-op, exit 0, and points at restore/archived', async () => {
|
||||
const { exit, logs, errs } = await captureRun(['archive', 'arch-idem']);
|
||||
expect(exit).toBeNull(); // no process.exit — success path
|
||||
const out = logs.join('\n');
|
||||
expect(out).toContain('already archived');
|
||||
expect(out).toContain('gbrain sources restore arch-idem');
|
||||
expect(errs.join('\n')).not.toContain('Failed to archive');
|
||||
});
|
||||
|
||||
test('unknown source still fails loud with exit 4', async () => {
|
||||
const { exit, errs } = await captureRun(['archive', 'no-such-source-xyz']);
|
||||
expect(exit).toBe(4);
|
||||
expect(errs.join('\n')).toContain('not found');
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user