From 3aa064bcc67cd8be4b3e04aa6e946bd9f637aabf Mon Sep 17 00:00:00 2001 From: Time Attakc <89218912+time-attack@users.noreply.github.com> Date: Tue, 28 Jul 2026 17:08:27 -0700 Subject: [PATCH] fix(test): make resetGateway restore the test baseline instead of unconfiguring (#3554) (#3557) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * 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 * 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 --------- Co-authored-by: Garry Tan Co-authored-by: Claude Opus 5 --- src/core/ai/gateway.ts | 65 +++++++++++++++++- test/ai/gateway-reset-baseline.test.ts | 68 +++++++++++++++++++ test/ai/gateway.test.ts | 4 ++ test/doctor-ze-checks.test.ts | 7 +- test/e2e/skill-brain-first.test.ts | 8 ++- test/embed-preflight.test.ts | 24 +++---- ...oreground-chat-gateway-init.serial.test.ts | 11 ++- test/helpers/legacy-embedding-preload.ts | 26 +++++-- test/llm-intent-escalation.test.ts | 5 +- test/minions-shell.test.ts | 9 ++- test/v0_37_fix_wave.serial.test.ts | 18 ++--- 11 files changed, 208 insertions(+), 37 deletions(-) create mode 100644 test/ai/gateway-reset-baseline.test.ts diff --git a/src/core/ai/gateway.ts b/src/core/ai/gateway.ts index 10bdb642e..53d25b34f 100644 --- a/src/core/ai/gateway.ts +++ b/src/core/ai/gateway.ts @@ -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. diff --git a/test/ai/gateway-reset-baseline.test.ts b/test/ai/gateway-reset-baseline.test.ts new file mode 100644 index 000000000..db521f659 --- /dev/null +++ b/test/ai/gateway-reset-baseline.test.ts @@ -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); + }); +}); diff --git a/test/ai/gateway.test.ts b/test/ai/gateway.test.ts index 730cc8b71..7445afb92 100644 --- a/test/ai/gateway.test.ts +++ b/test/ai/gateway.test.ts @@ -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); }); diff --git a/test/doctor-ze-checks.test.ts b/test/doctor-ze-checks.test.ts index aebf5d33c..0bbbfa7e6 100644 --- a/test/doctor-ze-checks.test.ts +++ b/test/doctor-ze-checks.test.ts @@ -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'); diff --git a/test/e2e/skill-brain-first.test.ts b/test/e2e/skill-brain-first.test.ts index 59cabeb9c..f733ff884 100644 --- a/test/e2e/skill-brain-first.test.ts +++ b/test/e2e/skill-brain-first.test.ts @@ -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(); }); diff --git a/test/embed-preflight.test.ts b/test/embed-preflight.test.ts index 5a0a0ca84..4f9fecd87 100644 --- a/test/embed-preflight.test.ts +++ b/test/embed-preflight.test.ts @@ -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 { @@ -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); diff --git a/test/foreground-chat-gateway-init.serial.test.ts b/test/foreground-chat-gateway-init.serial.test.ts index 5bdc7879d..81b747cf4 100644 --- a/test/foreground-chat-gateway-init.serial.test.ts +++ b/test/foreground-chat-gateway-init.serial.test.ts @@ -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(() => { diff --git a/test/helpers/legacy-embedding-preload.ts b/test/helpers/legacy-embedding-preload.ts index 257c21d35..ca76b5303 100644 --- a/test/helpers/legacy-embedding-preload.ts +++ b/test/helpers/legacy-embedding-preload.ts @@ -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. diff --git a/test/llm-intent-escalation.test.ts b/test/llm-intent-escalation.test.ts index 35bb636ca..33ad461fc 100644 --- a/test/llm-intent-escalation.test.ts +++ b/test/llm-intent-escalation.test.ts @@ -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'); }); diff --git a/test/minions-shell.test.ts b/test/minions-shell.test.ts index ff1f311f9..42f801cc1 100644 --- a/test/minions-shell.test.ts +++ b/test/minions-shell.test.ts @@ -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', () => { diff --git a/test/v0_37_fix_wave.serial.test.ts b/test/v0_37_fix_wave.serial.test.ts index 500309570..801254856 100644 --- a/test/v0_37_fix_wave.serial.test.ts +++ b/test/v0_37_fix_wave.serial.test.ts @@ -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(); } });