mirror of
https://github.com/garrytan/gbrain.git
synced 2026-07-29 19:01:39 +00:00
* fix(test): make resetGateway restore the test baseline instead of unconfiguring (#3554) bunfig.toml's legacy-embedding-preload pins the gateway once at process start to openai:text-embedding-3-large @ 1536, but resetGateway() wiped that pin to _config = null. The next test file's beforeAll engine-connect then reconfigured from the SHIPPED default (zembed-1 @ 1280) before the preload's per-test beforeEach could restore anything, and every 1536-d fixture in that file failed with `expected 1280 dimensions, not 1536`. Which file pairs collided depended on shard bin-packing, so adding ANY test file reshuffled the mines (this is what blocks #3545). Fix: the preload registers its config as a reset baseline via a new test-only seam (__setGatewayResetBaselineForTests); resetGateway() clears all module state as before, then re-applies the baseline. All 93 existing resetGateway() call sites get the correct behavior with zero edits. Production is untouched: nothing in src/ calls resetGateway() or the setter, so the baseline is never registered outside tests and resetGateway() still fully unconfigures there. Five tests genuinely need an unconfigured gateway (no_gateway_config diagnosis, isAvailable=false, the #2590 cold-gateway path, the registry builtin-default tier); they switch to the new __unconfigureGatewayForTests. Two files' hand-rolled "restore the legacy pin in afterAll/finally" workarounds for this exact bug are now redundant and simplified away. Guard test (test/ai/gateway-reset-baseline.test.ts) pins the contract: 1536/openai immediately after resetGateway(), transports still cleared, hard-unconfigure still available. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(test): restore preload's GBRAIN_AUDIT_DIR instead of deleting it (#3554 sibling) Same bug class as the gateway fix in this PR: state set once by a bunfig preload (audit-dir-preload's scratch GBRAIN_AUDIT_DIR), wiped by one file's cleanup, blast radius decided by shard bin-packing. In shard 6, test/minions-shell.test.ts (position 12) unconditionally deleted the var in afterAll; test/audit/audit-dir-preload.test.ts (position 93) then found it undefined and failed 3 tests — and every file in between wrote audit fixtures toward the operator's real ~/.gbrain/audit/. Fix: capture the prior value at file load and conditionally restore it, the same inline save/restore pattern 11 sibling files already use. test/e2e/skill-brain-first.test.ts had the identical unconditional delete in afterEach; fixed the same way. Sweep of every `delete process.env.GBRAIN_AUDIT_DIR` in test/ confirms all remaining sites are conditional restores. Ordered-pair proof (minions-shell.test.ts then audit/audit-dir-preload.test.ts, one process): 3 fail on master, 43/43 pass with this fix. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> --------- Co-authored-by: Garry Tan <garrytan@gmail.com> Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 5
Garry Tan
parent
6136e13997
commit
3aa064bcc6
+63
-2
@@ -642,8 +642,42 @@ function warnRecipesMissingBatchTokens(): void {
|
||||
}
|
||||
}
|
||||
|
||||
/** Reset (for tests). */
|
||||
export function resetGateway(): void {
|
||||
/**
|
||||
* Test-only reset baseline (#3554). The bunfig preload
|
||||
* (`test/helpers/legacy-embedding-preload.ts`) pins the gateway to the legacy
|
||||
* OpenAI/1536 config at process start, but `resetGateway()` used to wipe that
|
||||
* pin to `_config = null`. The next test file's engine connect then
|
||||
* reconfigured from the SHIPPED default (zembed-1 @ 1280) and every 1536-d
|
||||
* fixture in that file exploded with `expected 1280 dimensions, not 1536` —
|
||||
* a cross-file mine whose placement depended on shard bin-packing.
|
||||
*
|
||||
* When a baseline factory is registered, `resetGateway()` means "back to the
|
||||
* test baseline" instead of "unconfigured": it clears everything as before,
|
||||
* then re-applies the factory's config via `configureGateway()`. A factory
|
||||
* (not a frozen config) so each re-application captures fresh
|
||||
* `process.env`, matching the preload's original `applyLegacy()` semantics.
|
||||
*
|
||||
* Production is untouched: nothing in `src/` calls `resetGateway()` or this
|
||||
* setter, so in production the baseline is never registered and
|
||||
* `resetGateway()` still fully unconfigures. Same `__*ForTests` seam
|
||||
* convention as `__setEmbedTransportForTests` above.
|
||||
*/
|
||||
let _resetBaseline: (() => AIGatewayConfig) | null = null;
|
||||
|
||||
/**
|
||||
* Register (or clear, with `null`) the config factory that `resetGateway()`
|
||||
* re-applies. Called once by the bunfig test preload.
|
||||
*
|
||||
* @internal exported for tests; not part of the public gateway API.
|
||||
*/
|
||||
export function __setGatewayResetBaselineForTests(
|
||||
factory: (() => AIGatewayConfig) | null,
|
||||
): void {
|
||||
_resetBaseline = factory;
|
||||
}
|
||||
|
||||
/** Clear every piece of module state. Shared by both reset flavors. */
|
||||
function clearGatewayState(): void {
|
||||
_config = null;
|
||||
_modelCache.clear();
|
||||
_shrinkState.clear();
|
||||
@@ -655,6 +689,33 @@ export function resetGateway(): void {
|
||||
_extendedModels.clear();
|
||||
}
|
||||
|
||||
/**
|
||||
* Reset (for tests). Clears all module state (config, model cache, shrink
|
||||
* state, transports, warned recipes, extended models), then — if a test
|
||||
* baseline is registered — re-applies it so the gateway returns to the
|
||||
* process-wide test default instead of an unconfigured limbo (#3554).
|
||||
*/
|
||||
export function resetGateway(): void {
|
||||
clearGatewayState();
|
||||
// configureGateway re-clears _modelCache/_shrinkState/_extendedModels and
|
||||
// registers the baseline's models; transports are NOT touched by it, so a
|
||||
// stale test transport can never leak back in through this path.
|
||||
if (_resetBaseline) configureGateway(_resetBaseline());
|
||||
}
|
||||
|
||||
/**
|
||||
* Reset AND stay unconfigured, ignoring any registered baseline. For the
|
||||
* handful of tests that assert genuine no-gateway behavior
|
||||
* (`no_gateway_config` diagnosis, `isAvailable() === false`, graceful
|
||||
* degradation paths). The preload's per-test beforeEach restores the
|
||||
* baseline before the next test, so this cannot leak across tests.
|
||||
*
|
||||
* @internal exported for tests; not part of the public gateway API.
|
||||
*/
|
||||
export function __unconfigureGatewayForTests(): void {
|
||||
clearGatewayState();
|
||||
}
|
||||
|
||||
/**
|
||||
* Test-only seam. Replaces the function the gateway calls to embed a
|
||||
* sub-batch. Pass `null` to restore the real `embedMany` from the AI SDK.
|
||||
|
||||
@@ -0,0 +1,68 @@
|
||||
/**
|
||||
* #3554 — resetGateway() must restore the test baseline, not unconfigure.
|
||||
*
|
||||
* The bunfig preload (test/helpers/legacy-embedding-preload.ts) pins the
|
||||
* gateway to openai:text-embedding-3-large @ 1536 at process start and
|
||||
* registers that config as the reset baseline. Before the fix,
|
||||
* resetGateway() wiped the pin to _config = null; the next file's beforeAll
|
||||
* engine-connect then reconfigured from the SHIPPED default (zembed-1 @
|
||||
* 1280) and every 1536-d fixture in that file failed with
|
||||
* `expected 1280 dimensions, not 1536`. Which file pairs collided depended
|
||||
* on shard bin-packing, so adding ANY test file reshuffled the mines.
|
||||
*
|
||||
* These assertions pin the contract so it cannot silently rot again.
|
||||
*/
|
||||
import { describe, test, expect, afterEach } from 'bun:test';
|
||||
import {
|
||||
configureGateway,
|
||||
resetGateway,
|
||||
__unconfigureGatewayForTests,
|
||||
__setChatTransportForTests,
|
||||
getEmbeddingModel,
|
||||
getEmbeddingDimensions,
|
||||
isAvailable,
|
||||
} from '../../src/core/ai/gateway.ts';
|
||||
|
||||
afterEach(() => resetGateway());
|
||||
|
||||
describe('resetGateway baseline restore (#3554)', () => {
|
||||
test('immediately after resetGateway(), the preload baseline is live', () => {
|
||||
resetGateway();
|
||||
expect(getEmbeddingModel()).toBe('openai:text-embedding-3-large');
|
||||
expect(getEmbeddingDimensions()).toBe(1536);
|
||||
});
|
||||
|
||||
test('resetGateway() overwrites a file-local config back to the baseline', () => {
|
||||
configureGateway({
|
||||
embedding_model: 'zeroentropyai:zembed-1',
|
||||
embedding_dimensions: 1280,
|
||||
env: {},
|
||||
});
|
||||
expect(getEmbeddingDimensions()).toBe(1280);
|
||||
resetGateway();
|
||||
expect(getEmbeddingModel()).toBe('openai:text-embedding-3-large');
|
||||
expect(getEmbeddingDimensions()).toBe(1536);
|
||||
});
|
||||
|
||||
test('resetGateway() still clears test transports (no stale transport leaks back)', () => {
|
||||
__setChatTransportForTests(async () => {
|
||||
throw new Error('should have been cleared');
|
||||
});
|
||||
resetGateway();
|
||||
// Baseline config sets no chat key in a keyless env, but the transport
|
||||
// seam itself must be gone: isAvailable('chat') short-circuits to true
|
||||
// whenever a chat transport is installed, so with a hard-unconfigured
|
||||
// gateway it can only be true if the transport survived the reset.
|
||||
__unconfigureGatewayForTests();
|
||||
expect(isAvailable('chat')).toBe(false);
|
||||
});
|
||||
|
||||
test('__unconfigureGatewayForTests() gives a genuinely unconfigured gateway', () => {
|
||||
__unconfigureGatewayForTests();
|
||||
expect(() => getEmbeddingDimensions()).toThrow(/not configured/);
|
||||
expect(isAvailable('embedding')).toBe(false);
|
||||
// And a plain reset brings the baseline back.
|
||||
resetGateway();
|
||||
expect(getEmbeddingDimensions()).toBe(1536);
|
||||
});
|
||||
});
|
||||
@@ -2,6 +2,7 @@ import { describe, test, expect, beforeEach, afterAll } from 'bun:test';
|
||||
import {
|
||||
configureGateway,
|
||||
resetGateway,
|
||||
__unconfigureGatewayForTests,
|
||||
isAvailable,
|
||||
embed,
|
||||
getEmbeddingModel,
|
||||
@@ -55,6 +56,9 @@ describe('gateway.isAvailable (silent-drop regression surface)', () => {
|
||||
beforeEach(() => resetGateway());
|
||||
|
||||
test('returns false when gateway not configured', () => {
|
||||
// resetGateway() restores the preload's test baseline (#3554); go
|
||||
// genuinely unconfigured for this one assertion.
|
||||
__unconfigureGatewayForTests();
|
||||
expect(isAvailable('embedding')).toBe(false);
|
||||
});
|
||||
|
||||
|
||||
@@ -143,9 +143,10 @@ describe('checkEmbeddingWidthConsistency', () => {
|
||||
});
|
||||
|
||||
test('gateway unconfigured: skips with ok', async () => {
|
||||
// Reset gateway so requireConfig() throws.
|
||||
const { resetGateway } = await import('../src/core/ai/gateway.ts');
|
||||
resetGateway();
|
||||
// Hard-unconfigure so requireConfig() throws — resetGateway() would
|
||||
// restore the preload's test baseline (#3554).
|
||||
const { __unconfigureGatewayForTests } = await import('../src/core/ai/gateway.ts');
|
||||
__unconfigureGatewayForTests();
|
||||
const check = await checkEmbeddingWidthConsistency(engine);
|
||||
expect(check.status).toBe('ok');
|
||||
expect(check.message).toContain('gateway not configured');
|
||||
|
||||
@@ -81,6 +81,11 @@ function copyFixturesIntoTempWorkspace(): Workspace {
|
||||
|
||||
let workspace: Workspace;
|
||||
|
||||
// Restore (not delete) after each test: the audit-dir preload sets
|
||||
// GBRAIN_AUDIT_DIR once at process start, and deleting it leaks the
|
||||
// operator's real ~/.gbrain/audit/ to every later file in the shard.
|
||||
const priorAuditDir = process.env.GBRAIN_AUDIT_DIR;
|
||||
|
||||
beforeEach(() => {
|
||||
workspace = copyFixturesIntoTempWorkspace();
|
||||
// Redirect audit dir to the tempdir so the snapshot file doesn't pollute
|
||||
@@ -89,7 +94,8 @@ beforeEach(() => {
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
delete process.env.GBRAIN_AUDIT_DIR;
|
||||
if (priorAuditDir === undefined) delete process.env.GBRAIN_AUDIT_DIR;
|
||||
else process.env.GBRAIN_AUDIT_DIR = priorAuditDir;
|
||||
workspace.cleanup();
|
||||
});
|
||||
|
||||
|
||||
@@ -6,7 +6,11 @@
|
||||
* process.env.
|
||||
*/
|
||||
import { describe, test, expect, beforeEach, afterAll } from 'bun:test';
|
||||
import { configureGateway, resetGateway } from '../src/core/ai/gateway.ts';
|
||||
import {
|
||||
configureGateway,
|
||||
resetGateway,
|
||||
__unconfigureGatewayForTests,
|
||||
} from '../src/core/ai/gateway.ts';
|
||||
import {
|
||||
validateEmbeddingCreds,
|
||||
formatEmbeddingCredsError,
|
||||
@@ -22,18 +26,12 @@ import type { AIGatewayConfig } from '../src/core/ai/types.ts';
|
||||
// isAvailable('embedding') check. That's what made facts-backstop-gating
|
||||
// fail intermittently (bin-pack-dependent) on CI shard 10.
|
||||
//
|
||||
// Don't end on a bare resetGateway() either: the NEXT file's beforeAll
|
||||
// (often engine.initSchema, which sizes vector columns from ambient gateway
|
||||
// state) runs before the legacy-embedding-preload's per-test restore, so a
|
||||
// null gateway here would seed 1280-d schemas under 1536-d fixtures.
|
||||
// Restore the preload's legacy pin instead.
|
||||
// #3554: resetGateway() now restores the preload's legacy pin itself (the
|
||||
// preload registers it via __setGatewayResetBaselineForTests), so a bare
|
||||
// reset is safe here — the NEXT file's beforeAll sees the 1536-d baseline,
|
||||
// not a null gateway that would seed 1280-d schemas under 1536-d fixtures.
|
||||
afterAll(() => {
|
||||
resetGateway();
|
||||
configureGateway({
|
||||
embedding_model: 'openai:text-embedding-3-large',
|
||||
embedding_dimensions: 1536,
|
||||
env: { ...process.env },
|
||||
});
|
||||
});
|
||||
|
||||
function baseConfig(overrides: Partial<AIGatewayConfig> = {}): AIGatewayConfig {
|
||||
@@ -138,7 +136,9 @@ describe('validateEmbeddingCreds', () => {
|
||||
});
|
||||
|
||||
test('throws no_gateway_config when gateway was not configured', () => {
|
||||
// resetGateway() in beforeEach already cleared _config.
|
||||
// resetGateway() restores the preload's test baseline (#3554), so this
|
||||
// test needs the hard variant to get a genuinely unconfigured gateway.
|
||||
__unconfigureGatewayForTests();
|
||||
let caught: unknown;
|
||||
try { validateEmbeddingCreds(); } catch (e) { caught = e; }
|
||||
expect(caught).toBeInstanceOf(EmbeddingCredentialError);
|
||||
|
||||
@@ -9,7 +9,11 @@ import { afterEach, beforeEach, describe, expect, spyOn, test } from 'bun:test';
|
||||
import { mkdirSync, mkdtempSync, rmSync, writeFileSync } from 'node:fs';
|
||||
import { tmpdir } from 'node:os';
|
||||
import { join } from 'node:path';
|
||||
import { isAvailable, resetGateway } from '../src/core/ai/gateway.ts';
|
||||
import {
|
||||
__unconfigureGatewayForTests,
|
||||
isAvailable,
|
||||
resetGateway,
|
||||
} from '../src/core/ai/gateway.ts';
|
||||
import { runExtractConversationFacts } from '../src/commands/extract-conversation-facts.ts';
|
||||
import { runEnrich } from '../src/commands/enrich.ts';
|
||||
|
||||
@@ -27,7 +31,10 @@ beforeEach(() => {
|
||||
}));
|
||||
process.env.GBRAIN_HOME = home;
|
||||
process.env.OPENAI_API_KEY = 'test-key';
|
||||
resetGateway();
|
||||
// Hard-unconfigure: this suite exists to exercise the COLD-gateway path
|
||||
// (#2590), and resetGateway() now restores the preload's test baseline
|
||||
// (#3554), which would make configureGatewayIfUninitialized a no-op.
|
||||
__unconfigureGatewayForTests();
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
|
||||
@@ -18,7 +18,11 @@
|
||||
* `configureGateway()` explicitly in their own beforeAll, which
|
||||
* overwrites this preload.
|
||||
*/
|
||||
import { configureGateway, getEmbeddingDimensions } from '../../src/core/ai/gateway.ts';
|
||||
import {
|
||||
configureGateway,
|
||||
getEmbeddingDimensions,
|
||||
__setGatewayResetBaselineForTests,
|
||||
} from '../../src/core/ai/gateway.ts';
|
||||
import { beforeEach } from 'bun:test';
|
||||
|
||||
const LEGACY_CONFIG = {
|
||||
@@ -26,12 +30,16 @@ const LEGACY_CONFIG = {
|
||||
embedding_dimensions: 1536,
|
||||
} as const;
|
||||
|
||||
function applyLegacy() {
|
||||
configureGateway({
|
||||
function legacyGatewayConfig() {
|
||||
return {
|
||||
embedding_model: LEGACY_CONFIG.embedding_model,
|
||||
embedding_dimensions: LEGACY_CONFIG.embedding_dimensions,
|
||||
env: { ...process.env },
|
||||
});
|
||||
};
|
||||
}
|
||||
|
||||
function applyLegacy() {
|
||||
configureGateway(legacyGatewayConfig());
|
||||
}
|
||||
|
||||
if (process.env.GBRAIN_DEBUG_PRELOAD === '1') {
|
||||
@@ -41,6 +49,16 @@ if (process.env.GBRAIN_DEBUG_PRELOAD === '1') {
|
||||
// Initial application — covers tests that don't reset the gateway.
|
||||
applyLegacy();
|
||||
|
||||
// #3554: make resetGateway() mean "back to this baseline" instead of
|
||||
// "unconfigured". Without this, a file whose teardown calls resetGateway()
|
||||
// leaves _config = null; the NEXT file's beforeAll engine-connect then
|
||||
// reconfigures from the shipped default (zembed-1 @ 1280) BEFORE the
|
||||
// beforeEach below can fire, and the 1280-sized schema rejects the file's
|
||||
// 1536-d fixtures. Which file pairs collide depends on shard bin-packing,
|
||||
// so adding any test file reshuffles the mines. A factory (not a frozen
|
||||
// config) so each re-application captures fresh process.env.
|
||||
__setGatewayResetBaselineForTests(legacyGatewayConfig);
|
||||
|
||||
// Per-test re-application — handles tests that call `resetGateway()`
|
||||
// in their setup/teardown. Bun's preload allows registering global
|
||||
// hooks; this fires before every test in every file in the shard.
|
||||
|
||||
@@ -15,6 +15,7 @@ import {
|
||||
} from '../src/core/search/llm-intent.ts';
|
||||
import {
|
||||
__setChatTransportForTests,
|
||||
__unconfigureGatewayForTests,
|
||||
configureGateway,
|
||||
resetGateway,
|
||||
} from '../src/core/ai/gateway.ts';
|
||||
@@ -122,7 +123,9 @@ describe('classifyModalityWithLLM — fail-open', () => {
|
||||
});
|
||||
|
||||
test('Gateway not configured → returns fallback', async () => {
|
||||
resetGateway();
|
||||
// Hard-unconfigure: resetGateway() would restore the preload's test
|
||||
// baseline (#3554), whose {...process.env} could make chat available.
|
||||
__unconfigureGatewayForTests();
|
||||
// No configureGateway called → isAvailable('chat') returns false.
|
||||
expect(await classifyModalityWithLLM('q', 'text')).toBe('text');
|
||||
});
|
||||
|
||||
@@ -282,12 +282,19 @@ describe('shell-audit: computeAuditFilename', () => {
|
||||
|
||||
describe('shell-audit: write', () => {
|
||||
let tmpDir: string;
|
||||
// #3554-sibling: the audit-dir preload sets GBRAIN_AUDIT_DIR once at
|
||||
// process start; deleting it here (instead of restoring) let every file
|
||||
// AFTER this one in the shard write audit fixtures to the operator's
|
||||
// real ~/.gbrain/audit/ — and failed audit-dir-preload.test.ts whenever
|
||||
// bin-packing placed it later in the shard. Restore the prior value.
|
||||
const priorAuditDir = process.env.GBRAIN_AUDIT_DIR;
|
||||
beforeEach(() => {
|
||||
tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'shell-audit-test-'));
|
||||
process.env.GBRAIN_AUDIT_DIR = tmpDir;
|
||||
});
|
||||
afterAll(() => {
|
||||
delete process.env.GBRAIN_AUDIT_DIR;
|
||||
if (priorAuditDir === undefined) delete process.env.GBRAIN_AUDIT_DIR;
|
||||
else process.env.GBRAIN_AUDIT_DIR = priorAuditDir;
|
||||
});
|
||||
|
||||
test('GBRAIN_AUDIT_DIR env override resolves to the custom dir', () => {
|
||||
|
||||
@@ -55,24 +55,20 @@ describe('v0.37 Lane A — defaults sweep', () => {
|
||||
test('A.5: embedding-column registry builtin defaults to ZE/1280 on empty config + gateway', async () => {
|
||||
// The registry's resolution chain is cfg > gateway > DEFAULT. With
|
||||
// no cfg AND no gateway, it should fall through to the canonical
|
||||
// default (ZE/1280). Reset gateway first to exercise that path.
|
||||
const { resetGateway } = await import('../src/core/ai/gateway.ts');
|
||||
// default (ZE/1280). Hard-unconfigure first to exercise that path —
|
||||
// resetGateway() would restore the preload's 1536 baseline (#3554).
|
||||
const { __unconfigureGatewayForTests, resetGateway } = await import('../src/core/ai/gateway.ts');
|
||||
const { getEmbeddingColumnRegistry } = await import('../src/core/search/embedding-column.ts');
|
||||
resetGateway();
|
||||
__unconfigureGatewayForTests();
|
||||
try {
|
||||
const reg = getEmbeddingColumnRegistry({ engine: 'pglite' } as any);
|
||||
expect(reg['embedding']).toBeDefined();
|
||||
expect(reg['embedding'].provider).toBe('zeroentropyai:zembed-1');
|
||||
expect(reg['embedding'].dimensions).toBe(1280);
|
||||
} finally {
|
||||
// Re-apply legacy preload defaults so the rest of the file's tests
|
||||
// (and subsequent files in this shard) see a configured gateway.
|
||||
const { configureGateway } = await import('../src/core/ai/gateway.ts');
|
||||
configureGateway({
|
||||
embedding_model: 'openai:text-embedding-3-large',
|
||||
embedding_dimensions: 1536,
|
||||
env: { ...process.env },
|
||||
});
|
||||
// Restore the preload's legacy baseline so the rest of the file's
|
||||
// tests (and subsequent files in this shard) see a configured gateway.
|
||||
resetGateway();
|
||||
}
|
||||
});
|
||||
|
||||
|
||||
Reference in New Issue
Block a user