mirror of
https://github.com/garrytan/gbrain.git
synced 2026-07-27 22:15:33 +00:00
fix(init): explicit --embedding-model overrides persisted --no-embedding sentinel (#3138)
* fix(init): explicit --embedding-model overrides the persisted --no-embedding sentinel (#2301) Pre-fix, once ~/.gbrain/config.json carried embedding_disabled: true (the --no-embedding deferred-setup sentinel), every re-init silently re-deferred embedding: resolveAIOptions honored the sentinel BEFORE the explicit --embedding-model flag and never cleared noEmbedding, and the persistence merge carried the sentinel forward via ...existingFile. Both recovery paths were dead ends — `gbrain config set embedding_model` is hard-refused (schema-sizing field), and re-init hit the sentinel. Fix: - resolveAIOptions: an explicit --embedding-model / --model flag clears the sentinel-derived noEmbedding (explicit --no-embedding on the same invocation still wins — that branch runs after). - initPGLite + initPostgres persistence: a resolved (model, dims) tuple drops the stale embedding_disabled key instead of inheriting it. - assertEmbeddingEnabled message no longer recommends the refused `gbrain config set embedding_model` command; the working re-init recipe leads. Test: test/e2e/init-reinit-after-deferred.test.ts — deferred init then re-init with an explicit model recovers (sentinel gone, model persisted); bare re-init still honors the sentinel. Fixes #2301 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * review: scrub remaining hard-refused `config set embedding_model` advice from init deferred-setup messages The PR fixed the recovery recipe in assertEmbeddingEnabled but the deferred-setup lines in initPGLite/initPostgres and the fail-loud defer hint still pointed users at the Lane C.2 hard-refused command. Point all three at the working re-init recipe instead. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Garry Tan <garrytan@gmail.com> Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Garry Tan
Claude Fable 5
parent
d2fd1f297c
commit
df22c81996
+20
-3
@@ -249,6 +249,13 @@ async function resolveAIOptions(opts: ResolveAIOptionsArgs): Promise<ResolvedAIO
|
||||
|
||||
// --- Tier 1+2: explicit flags ---------------------------------------------
|
||||
|
||||
// #2301: an explicit embedding flag on THIS invocation overrides the
|
||||
// persisted deferred-setup sentinel above. Without this, a stale
|
||||
// `embedding_disabled: true` in config.json made every re-init defer
|
||||
// embedding — including `gbrain init --embedding-model ...`, the exact
|
||||
// recovery path the deferred-setup message tells users to take.
|
||||
if (verbose || shorthand) delete out.noEmbedding;
|
||||
|
||||
if (verbose) {
|
||||
out.embedding_model = verbose;
|
||||
} else if (shorthand) {
|
||||
@@ -435,7 +442,7 @@ function printNoEmbeddingProviderHint(typos: Array<{ userSet: string; suggested:
|
||||
console.error(' gbrain init --pglite --embedding-model openai:text-embedding-3-large');
|
||||
console.error('');
|
||||
console.error('Or defer setup: gbrain init --pglite --no-embedding');
|
||||
console.error(' (you can configure later with `gbrain config set embedding_model <id>`)');
|
||||
console.error(' (you can configure later with `gbrain init --force --embedding-model <provider>:<model>`)');
|
||||
// D13: surface near-miss env vars (e.g. OPENAPI_API_KEY → OPENAI_API_KEY).
|
||||
if (typos.length > 0) {
|
||||
console.error('');
|
||||
@@ -833,7 +840,7 @@ async function initPGLite(opts: {
|
||||
let resolvedModel: string | undefined;
|
||||
if (opts.aiOpts?.noEmbedding) {
|
||||
// D9 deferred-setup mode: skip preflight, no model/dim resolved.
|
||||
console.log(` --no-embedding: deferred setup — configure with \`gbrain config set embedding_model <id>\` before import`);
|
||||
console.log(` --no-embedding: deferred setup — run \`gbrain init --force --embedding-model <provider>:<model>\` before import`);
|
||||
} else if (opts.aiOpts?.embedding_model) {
|
||||
const { resolveSchemaEmbeddingDim } = await import('../core/embedding-dim-check.ts');
|
||||
const pre = resolveSchemaEmbeddingDim({
|
||||
@@ -972,6 +979,12 @@ async function initPGLite(opts: {
|
||||
// unless explicitly overridden by --schema-pack on re-init.
|
||||
...(opts.schemaPack ? { schema_pack: opts.schemaPack } : {}),
|
||||
};
|
||||
// #2301: a resolved embedding model supersedes any stale deferred-setup
|
||||
// sentinel carried over via ...existingFile — otherwise the sentinel
|
||||
// re-defers embedding on every future init/embed forever.
|
||||
if (!opts.aiOpts?.noEmbedding && resolvedModel && resolvedDim) {
|
||||
delete config.embedding_disabled;
|
||||
}
|
||||
// PR1: new installs publish their skill catalog over MCP by default
|
||||
// (existing config wins on re-init, so a prior opt-out is preserved).
|
||||
config.mcp = { publish_skills: true, ...(config.mcp ?? {}) };
|
||||
@@ -1056,7 +1069,7 @@ async function initPostgres(opts: {
|
||||
let resolvedDim: number | undefined;
|
||||
let resolvedModel: string | undefined;
|
||||
if (opts.aiOpts?.noEmbedding) {
|
||||
console.log(` --no-embedding: deferred setup — configure with \`gbrain config set embedding_model <id>\` before import`);
|
||||
console.log(` --no-embedding: deferred setup — run \`gbrain init --force --embedding-model <provider>:<model>\` before import`);
|
||||
} else if (opts.aiOpts?.embedding_model) {
|
||||
const { resolveSchemaEmbeddingDim } = await import('../core/embedding-dim-check.ts');
|
||||
const pre = resolveSchemaEmbeddingDim({
|
||||
@@ -1220,6 +1233,10 @@ async function initPostgres(opts: {
|
||||
// v0.42 (T17): same schema_pack default as PGLite path.
|
||||
...(opts.schemaPack ? { schema_pack: opts.schemaPack } : {}),
|
||||
};
|
||||
// #2301: same stale-sentinel drop as the PGLite path above.
|
||||
if (!opts.aiOpts?.noEmbedding && resolvedModel && resolvedDim) {
|
||||
delete config.embedding_disabled;
|
||||
}
|
||||
// PR1: new installs publish their skill catalog over MCP by default
|
||||
// (existing config wins on re-init, so a prior opt-out is preserved).
|
||||
config.mcp = { publish_skills: true, ...(config.mcp ?? {}) };
|
||||
|
||||
@@ -71,9 +71,8 @@ export function assertEmbeddingEnabled(cfg: { embedding_disabled?: boolean } | n
|
||||
throw new EmbeddingDisabledError(
|
||||
'This brain was initialized with `--no-embedding` (deferred setup).\n' +
|
||||
'Configure an embedding provider before running embed / import:\n' +
|
||||
' gbrain config set embedding_model <provider>:<model>\n' +
|
||||
' gbrain config set embedding_dimensions <N>\n' +
|
||||
' gbrain init --force --embedding-model <provider>:<model> # re-init to size schema\n',
|
||||
' gbrain init --force --embedding-model <provider>:<model> # re-init to size schema\n' +
|
||||
'(`gbrain config set embedding_model` is refused — schema-sizing fields are set at init.)\n',
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -0,0 +1,111 @@
|
||||
/**
|
||||
* #2301 — re-init with an explicit --embedding-model must recover a brain
|
||||
* that was initialized with --no-embedding (deferred setup).
|
||||
*
|
||||
* Pre-fix: resolveAIOptions honored the persisted `embedding_disabled: true`
|
||||
* sentinel BEFORE the explicit flag and never cleared noEmbedding, and the
|
||||
* persistence merge carried the sentinel forward via ...existingFile. Result:
|
||||
* every re-init (including the recovery command the deferred-setup error
|
||||
* itself recommends) silently re-deferred embedding, forever.
|
||||
*
|
||||
* Hermetic: in-process runInit, GBRAIN_HOME pinned to a tmpdir (same pattern
|
||||
* as test/e2e/fresh-install-pglite.test.ts).
|
||||
*/
|
||||
|
||||
import { afterEach, beforeEach, describe, expect, test } from 'bun:test';
|
||||
import { mkdtempSync, rmSync, readFileSync } from 'fs';
|
||||
import { tmpdir } from 'os';
|
||||
import { join } from 'path';
|
||||
import { configureGateway, resetGateway } from '../../src/core/ai/gateway.ts';
|
||||
|
||||
describe('E2E: re-init with --embedding-model after --no-embedding init (#2301)', () => {
|
||||
let tmpHome: string;
|
||||
let origHome: string | undefined;
|
||||
let origZeKey: string | undefined;
|
||||
let origOpenaiKey: string | undefined;
|
||||
let origVoyageKey: string | undefined;
|
||||
|
||||
beforeEach(() => {
|
||||
tmpHome = mkdtempSync(join(tmpdir(), 'gbrain-e2e-reinit-'));
|
||||
origHome = process.env.GBRAIN_HOME;
|
||||
origZeKey = process.env.ZEROENTROPY_API_KEY;
|
||||
origOpenaiKey = process.env.OPENAI_API_KEY;
|
||||
origVoyageKey = process.env.VOYAGE_API_KEY;
|
||||
delete process.env.OPENAI_API_KEY;
|
||||
delete process.env.VOYAGE_API_KEY;
|
||||
process.env.GBRAIN_HOME = tmpHome;
|
||||
process.env.ZEROENTROPY_API_KEY = 'sk-test-ze';
|
||||
resetGateway();
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
rmSync(tmpHome, { recursive: true, force: true });
|
||||
if (origHome === undefined) delete process.env.GBRAIN_HOME;
|
||||
else process.env.GBRAIN_HOME = origHome;
|
||||
if (origZeKey === undefined) delete process.env.ZEROENTROPY_API_KEY;
|
||||
else process.env.ZEROENTROPY_API_KEY = origZeKey;
|
||||
if (origOpenaiKey !== undefined) process.env.OPENAI_API_KEY = origOpenaiKey;
|
||||
if (origVoyageKey !== undefined) process.env.VOYAGE_API_KEY = origVoyageKey;
|
||||
// Restore legacy-preload gateway state (mirrors fresh-install-pglite.test.ts).
|
||||
configureGateway({
|
||||
embedding_model: 'openai:text-embedding-3-large',
|
||||
embedding_dimensions: 1536,
|
||||
env: { ...process.env },
|
||||
});
|
||||
});
|
||||
|
||||
async function runInitCapturing(args: string[]): Promise<string> {
|
||||
const { runInit } = await import('../../src/commands/init.ts');
|
||||
const origLog = console.log;
|
||||
const origWarn = console.warn;
|
||||
const stdoutBuf: string[] = [];
|
||||
console.log = (...a: unknown[]) => {
|
||||
stdoutBuf.push(a.map(x => (typeof x === 'string' ? x : JSON.stringify(x))).join(' '));
|
||||
};
|
||||
console.warn = () => {};
|
||||
try {
|
||||
await runInit(args);
|
||||
} finally {
|
||||
console.log = origLog;
|
||||
console.warn = origWarn;
|
||||
}
|
||||
return stdoutBuf.join('\n');
|
||||
}
|
||||
|
||||
const cfgPath = () => join(tmpHome, '.gbrain', 'config.json');
|
||||
const readCfg = () => JSON.parse(readFileSync(cfgPath(), 'utf-8'));
|
||||
|
||||
test('explicit --embedding-model clears the persisted embedding_disabled sentinel', async () => {
|
||||
// Step 1: deferred-setup init writes the sentinel.
|
||||
const out1 = await runInitCapturing(['--pglite', '--non-interactive', '--no-embedding']);
|
||||
expect(out1).toContain('deferred setup');
|
||||
const cfg1 = readCfg();
|
||||
expect(cfg1.embedding_disabled).toBe(true);
|
||||
expect(cfg1.embedding_model).toBeUndefined();
|
||||
|
||||
// Step 2: re-init with an explicit embedding model — the recovery path.
|
||||
// Pre-fix this printed the deferred-setup line again and re-persisted
|
||||
// embedding_disabled: true.
|
||||
const out2 = await runInitCapturing([
|
||||
'--pglite', '--non-interactive', '--skip-embed-check',
|
||||
'--embedding-model', 'zeroentropyai:zembed-1',
|
||||
'--embedding-dimensions', '1280',
|
||||
]);
|
||||
expect(out2).not.toContain('deferred setup');
|
||||
expect(out2).toContain('zeroentropyai:zembed-1');
|
||||
|
||||
const cfg2 = readCfg();
|
||||
expect(cfg2.embedding_model).toBe('zeroentropyai:zembed-1');
|
||||
expect(cfg2.embedding_dimensions).toBe(1280);
|
||||
expect(cfg2.embedding_disabled).toBeUndefined();
|
||||
}, 60000);
|
||||
|
||||
test('re-init WITHOUT flags still honors the deferred-setup sentinel (no regression)', async () => {
|
||||
await runInitCapturing(['--pglite', '--non-interactive', '--no-embedding']);
|
||||
const out = await runInitCapturing(['--pglite', '--non-interactive']);
|
||||
expect(out).toContain('deferred setup');
|
||||
const cfg = readCfg();
|
||||
expect(cfg.embedding_disabled).toBe(true);
|
||||
expect(cfg.embedding_model).toBeUndefined();
|
||||
}, 60000);
|
||||
});
|
||||
Reference in New Issue
Block a user