diff --git a/scripts/check-test-isolation.sh b/scripts/check-test-isolation.sh index 1ad904a71..c9bbaa91e 100755 --- a/scripts/check-test-isolation.sh +++ b/scripts/check-test-isolation.sh @@ -18,6 +18,12 @@ # R4: any file that creates `new PGLiteEngine(` must call `.disconnect(` # inside an `afterAll(` block. Without disconnect, engines leak across # file boundaries within a shard process. +# R5: any file that calls `configureGateway(` or +# `__setEmbedTransportForTests(` must also call `resetGateway()` and +# have an `afterAll(`/`afterEach(` hook. The gateway is module-global; +# a configured remote provider + fake key left past the file boundary +# makes the next embed-triggering file in the shard fire a live HTTP +# call (issue #3066; master shard 6 broke twice this way). # # Scope: # - Recursively scans `test/**/*.test.ts`. @@ -132,6 +138,22 @@ while IFS= read -r f; do emit_violation "$f" "R4" "creates PGLiteEngine but missing afterAll(() => engine.disconnect()); engine leaks across files in the shard process" "" fi fi + + # R5: gateway configuration requires resetGateway + an afterAll/afterEach + # hook. Same loose two-grep shape as R4 — `resetGateway(` present plus at + # least one afterAll(/afterEach( — but on comment-stripped lines: a prose + # mention like "a test that calls resetGateway()" must not satisfy the + # rule (that exact false pass hid the cycle-consolidate leaker). + # No [[:space:]]* before the paren on the trigger side: prose in a test + # name ("works WITHOUT configureGateway (reads registry...)") must not + # trigger the rule; real call sites are always `configureGateway(`. + r5_code=$(grep -vE '^[[:space:]]*(//|\*)' "$f" 2>/dev/null || true) + if printf '%s\n' "$r5_code" | grep -qE 'configureGateway\(|__setEmbedTransportForTests\('; then + if ! printf '%s\n' "$r5_code" | grep -qE 'resetGateway\(' \ + || ! printf '%s\n' "$r5_code" | grep -qE 'afterAll[[:space:]]*\(|afterEach[[:space:]]*\('; then + emit_violation "$f" "R5" "configures the AI gateway but never calls resetGateway() in afterAll/afterEach; gateway state (provider, fake keys, transports) leaks across files in the shard process" "" + fi + fi done < { + resetGateway(); + __setGenerateTextTransportForTests(null); +}); + describe('gbrain#2490 — Anthropic cache breakpoint placement', () => { beforeEach(() => { resetGateway(); diff --git a/test/ai/gateway-chat.test.ts b/test/ai/gateway-chat.test.ts index 4aee06a26..a105c4d9c 100644 --- a/test/ai/gateway-chat.test.ts +++ b/test/ai/gateway-chat.test.ts @@ -16,7 +16,7 @@ * `generateText` import via Bun's module-replace pattern. */ -import { describe, test, expect, beforeEach, mock } from 'bun:test'; +import { describe, test, expect, beforeEach, mock, afterAll } from 'bun:test'; import { configureGateway, resetGateway, @@ -30,6 +30,12 @@ import { parseModelId, resolveRecipe, assertTouchpoint } from '../../src/core/ai import { AIConfigError } from '../../src/core/ai/errors.ts'; import { listRecipes, getRecipe } from '../../src/core/ai/recipes/index.ts'; +// R5 shard hygiene: leave no configured gateway past the file boundary. +afterAll(() => { + resetGateway(); + __setGenerateTextTransportForTests(null); +}); + describe('chat touchpoint — recipe registry', () => { test('all six chat-capable providers ship a chat touchpoint with supports_subagent_loop', () => { const expected = ['anthropic', 'openai', 'google', 'deepseek', 'groq', 'together']; diff --git a/test/ai/gateway-openai-prompt-cache-key.test.ts b/test/ai/gateway-openai-prompt-cache-key.test.ts index 8d49e56b2..44964e409 100644 --- a/test/ai/gateway-openai-prompt-cache-key.test.ts +++ b/test/ai/gateway-openai-prompt-cache-key.test.ts @@ -16,7 +16,7 @@ * nothing), and config `provider_chat_options` overrides the derived key */ -import { describe, test, expect, beforeEach } from 'bun:test'; +import { describe, test, expect, beforeEach, afterAll } from 'bun:test'; import { chat, configureGateway, @@ -25,6 +25,12 @@ import { __setGenerateTextTransportForTests, } from '../../src/core/ai/gateway.ts'; +// R5 shard hygiene: leave no configured gateway past the file boundary. +afterAll(() => { + resetGateway(); + __setGenerateTextTransportForTests(null); +}); + describe('openAIPromptCacheKey — derivation', () => { test('same system + same tools → identical stable key (sticky routing)', () => { const a = openAIPromptCacheKey({ system: 'SYS', toolNames: ['search', 'put_page'] }); diff --git a/test/cycle-consolidate.test.ts b/test/cycle-consolidate.test.ts index fe3462bc5..b0e7b374d 100644 --- a/test/cycle-consolidate.test.ts +++ b/test/cycle-consolidate.test.ts @@ -10,7 +10,7 @@ import { describe, test, expect, beforeAll, afterAll, beforeEach } from 'bun:test'; import { PGLiteEngine } from '../src/core/pglite-engine.ts'; -import { configureGateway } from '../src/core/ai/gateway.ts'; +import { configureGateway, resetGateway } from '../src/core/ai/gateway.ts'; import { runPhaseConsolidate } from '../src/core/cycle/phases/consolidate.ts'; let engine: PGLiteEngine; @@ -37,6 +37,7 @@ beforeAll(async () => { afterAll(async () => { await engine.disconnect(); + resetGateway(); }); beforeEach(async () => { diff --git a/test/doctor-federation-health.test.ts b/test/doctor-federation-health.test.ts index 71a808fd8..bfbb557a5 100644 --- a/test/doctor-federation-health.test.ts +++ b/test/doctor-federation-health.test.ts @@ -8,7 +8,7 @@ */ import { describe, test, expect, beforeAll, afterAll, beforeEach } from 'bun:test'; import { PGLiteEngine } from '../src/core/pglite-engine.ts'; -import { configureGateway } from '../src/core/ai/gateway.ts'; +import { configureGateway, resetGateway } from '../src/core/ai/gateway.ts'; import { checkFederationHealth } from '../src/commands/doctor.ts'; let engine: PGLiteEngine; @@ -33,6 +33,7 @@ beforeAll(async () => { afterAll(async () => { await engine.disconnect(); + resetGateway(); }); beforeEach(async () => { diff --git a/test/engine-find-trajectory.test.ts b/test/engine-find-trajectory.test.ts index 4415c4a4b..2f6fd08a8 100644 --- a/test/engine-find-trajectory.test.ts +++ b/test/engine-find-trajectory.test.ts @@ -15,7 +15,7 @@ import { describe, test, expect, beforeAll, afterAll, beforeEach } from 'bun:test'; import { PGLiteEngine } from '../src/core/pglite-engine.ts'; -import { configureGateway } from '../src/core/ai/gateway.ts'; +import { configureGateway, resetGateway } from '../src/core/ai/gateway.ts'; import { detectRegressions, computeDriftScore, @@ -45,6 +45,7 @@ beforeAll(async () => { afterAll(async () => { await engine.disconnect(); + resetGateway(); }); beforeEach(async () => { diff --git a/test/eval-retrieval-quality.test.ts b/test/eval-retrieval-quality.test.ts index 9312a2892..5922fe903 100644 --- a/test/eval-retrieval-quality.test.ts +++ b/test/eval-retrieval-quality.test.ts @@ -14,7 +14,7 @@ import { readFileSync } from 'fs'; import { join } from 'path'; import { PGLiteEngine } from '../src/core/pglite-engine.ts'; import { hybridSearch } from '../src/core/search/hybrid.ts'; -import { __setEmbedTransportForTests } from '../src/core/ai/gateway.ts'; +import { __setEmbedTransportForTests, resetGateway } from '../src/core/ai/gateway.ts'; import { parseQuestionsJsonl, runRetrievalQuality, evaluateGate, type SearchFn } from '../src/eval/retrieval-quality/harness.ts'; import type { ChunkInput } from '../src/core/types.ts'; @@ -60,7 +60,7 @@ beforeAll(async () => { }); afterAll(async () => { - __setEmbedTransportForTests(null); + resetGateway(); await engine.disconnect(); }); diff --git a/test/propose-takes.test.ts b/test/propose-takes.test.ts index 8430f2ed4..2d74429d7 100644 --- a/test/propose-takes.test.ts +++ b/test/propose-takes.test.ts @@ -14,7 +14,7 @@ * - parseExtractorOutput unit tests for the raw JSON parser */ -import { describe, test, expect } from 'bun:test'; +import { describe, test, expect, afterAll } from 'bun:test'; import { runPhaseProposeTakes, parseExtractorOutput, @@ -28,6 +28,11 @@ import { type ProposedTake, } from '../src/core/cycle/propose-takes.ts'; import { configureGateway, resetGateway } from '../src/core/ai/gateway.ts'; + +// R5 shard hygiene: leave no configured gateway past the file boundary. +afterAll(() => { + resetGateway(); +}); import { BudgetMeter } from '../src/core/cycle/budget-meter.ts'; import type { OperationContext } from '../src/core/operations.ts'; import type { BrainEngine } from '../src/core/engine.ts'; diff --git a/test/scripts/check-test-isolation.test.ts b/test/scripts/check-test-isolation.test.ts index b4fa8ba0d..dcf292867 100644 --- a/test/scripts/check-test-isolation.test.ts +++ b/test/scripts/check-test-isolation.test.ts @@ -199,6 +199,84 @@ describe('check-test-isolation.sh', () => { }); }); + describe('R5 — gateway configuration requires resetGateway teardown (#3066)', () => { + it('flags configureGateway without resetGateway in a teardown hook', () => { + const r = runLintIn([ + { + path: 'gateway-leak.test.ts', + contents: + `import { beforeAll, test, expect } from 'bun:test';\n` + + `import { configureGateway } from '../src/core/ai/gateway.ts';\n` + + `beforeAll(() => { configureGateway({ env: {} }); });\n` + + `test('x', () => expect(1).toBe(1));\n`, + }, + ]); + expect(r.status).toBe(1); + expect(r.stdout).toContain('R5'); + expect(r.stdout).toContain('gateway-leak.test.ts'); + }); + + it('flags __setEmbedTransportForTests without resetGateway', () => { + const r = runLintIn([ + { + path: 'transport-leak.test.ts', + contents: + `import { afterAll, test, expect } from 'bun:test';\n` + + `import { __setEmbedTransportForTests } from '../src/core/ai/gateway.ts';\n` + + `__setEmbedTransportForTests(async () => ({ embeddings: [] }));\n` + + `afterAll(() => { /* disconnect only */ });\n` + + `test('x', () => expect(1).toBe(1));\n`, + }, + ]); + expect(r.status).toBe(1); + expect(r.stdout).toContain('R5'); + }); + + it('passes when resetGateway is called and an afterAll hook exists', () => { + const r = runLintIn([ + { + path: 'gateway-clean.test.ts', + contents: + `import { beforeAll, afterAll, test, expect } from 'bun:test';\n` + + `import { configureGateway, resetGateway } from '../src/core/ai/gateway.ts';\n` + + `beforeAll(() => { configureGateway({ env: {} }); });\n` + + `afterAll(() => { resetGateway(); });\n` + + `test('x', () => expect(1).toBe(1));\n`, + }, + ]); + expect(r.status).toBe(0); + }); + + it('a comment mentioning resetGateway() does not satisfy the rule', () => { + const r = runLintIn([ + { + path: 'gateway-comment-leak.test.ts', + contents: + `import { beforeAll, afterAll, test, expect } from 'bun:test';\n` + + `import { configureGateway } from '../src/core/ai/gateway.ts';\n` + + `// a co-sharded test that calls resetGateway() would clear this\n` + + `beforeAll(() => { configureGateway({ env: {} }); });\n` + + `afterAll(() => { /* disconnect only */ });\n` + + `test('x', () => expect(1).toBe(1));\n`, + }, + ]); + expect(r.status).toBe(1); + expect(r.stdout).toContain('R5'); + }); + + it('a test name mentioning "configureGateway (" does not trigger the rule', () => { + const r = runLintIn([ + { + path: 'gateway-prose.test.ts', + contents: + `import { test, expect } from 'bun:test';\n` + + `test('works WITHOUT configureGateway (reads registry)', () => expect(1).toBe(1));\n`, + }, + ]); + expect(r.status).toBe(0); + }); + }); + describe('scope', () => { it('skips *.serial.test.ts files entirely', () => { const r = runLintIn([ diff --git a/test/search/search-diagnose.test.ts b/test/search/search-diagnose.test.ts index e16c0dddd..9cee1b822 100644 --- a/test/search/search-diagnose.test.ts +++ b/test/search/search-diagnose.test.ts @@ -6,7 +6,7 @@ import { describe, test, expect, beforeAll, afterAll } from 'bun:test'; import { PGLiteEngine } from '../../src/core/pglite-engine.ts'; -import { __setEmbedTransportForTests } from '../../src/core/ai/gateway.ts'; +import { __setEmbedTransportForTests, resetGateway } from '../../src/core/ai/gateway.ts'; import { runSearchDiagnose } from '../../src/commands/search-diagnose.ts'; import type { ChunkInput } from '../../src/core/types.ts'; @@ -35,7 +35,7 @@ beforeAll(async () => { await engine.setPageAliases('projects/mingtang', 'default', ['hall of light']); }); -afterAll(async () => { __setEmbedTransportForTests(null); await engine.disconnect(); }); +afterAll(async () => { resetGateway(); await engine.disconnect(); }); describe('search diagnose', () => { test('alias query: trace shows alias match + hybrid rank 1', async () => { diff --git a/test/search/searchvector-maxpool.test.ts b/test/search/searchvector-maxpool.test.ts index 29359270c..64ea9f0b1 100644 --- a/test/search/searchvector-maxpool.test.ts +++ b/test/search/searchvector-maxpool.test.ts @@ -23,7 +23,7 @@ import { describe, test, expect, beforeAll, afterAll } from 'bun:test'; import { PGLiteEngine } from '../../src/core/pglite-engine.ts'; -import { configureGateway } from '../../src/core/ai/gateway.ts'; +import { configureGateway, resetGateway } from '../../src/core/ai/gateway.ts'; import type { ChunkInput } from '../../src/core/types.ts'; let engine: PGLiteEngine; @@ -95,6 +95,7 @@ beforeAll(async () => { afterAll(async () => { await engine.disconnect(); + resetGateway(); }); describe('searchVector per-page max-pool (T1)', () => { diff --git a/test/search/title-retrieval-arm.test.ts b/test/search/title-retrieval-arm.test.ts index 45d0af622..3015db2e2 100644 --- a/test/search/title-retrieval-arm.test.ts +++ b/test/search/title-retrieval-arm.test.ts @@ -27,7 +27,7 @@ import { PGLiteEngine } from '../../src/core/pglite-engine.ts'; import { resetPgliteState } from '../helpers/reset-pglite.ts'; import { hybridSearch } from '../../src/core/search/hybrid.ts'; import { buildOrFallbackWebsearchQuery } from '../../src/core/search/sql-ranking.ts'; -import { configureGateway } from '../../src/core/ai/gateway.ts'; +import { configureGateway, resetGateway } from '../../src/core/ai/gateway.ts'; let engine: PGLiteEngine; @@ -48,6 +48,7 @@ beforeAll(async () => { afterAll(async () => { await engine.disconnect(); + resetGateway(); // Restore the preload-equivalent gateway for sibling files in this shard. configureGateway({ embedding_model: 'openai:text-embedding-3-large',