test: fix R1 env-isolation violations in retrieval-reflex tests

Pre-existing on master: both files mutated GBRAIN_RETRIEVAL_REFLEX directly, failing scripts/check-test-isolation.sh (bun run verify). Converted to the canonical withEnv() pattern; the reflex describe's beforeEach also never restored the flag, leaking it across the shard.
2026-07-28 06:23:01 +00:00 · 2026-06-11 23:15:23 -07:00
parent 4e049efeb6
commit dfadd8e722
2 changed files with 93 additions and 79 deletions
@@ -1,46 +1,50 @@
 /**
 * Doctor retrieval_reflex_health check (#1981, T8).
 */
-import { describe, test, expect, afterEach } from 'bun:test';
+import { describe, test, expect } from 'bun:test';
 import { mkdtempSync, mkdirSync, writeFileSync, rmSync } from 'node:fs';
 import { tmpdir } from 'node:os';
 import { join } from 'node:path';
 import { buildRetrievalReflexCheck } from '../src/commands/doctor.ts';
-
-const origEnv = process.env.GBRAIN_RETRIEVAL_REFLEX;
-afterEach(() => {
-  if (origEnv === undefined) delete process.env.GBRAIN_RETRIEVAL_REFLEX;
-  else process.env.GBRAIN_RETRIEVAL_REFLEX = origEnv;
-});
+import { withEnv } from './helpers/with-env.ts';

 describe('buildRetrievalReflexCheck', () => {
-  test('disabled via env → warn, names the right check', () => {
-    process.env.GBRAIN_RETRIEVAL_REFLEX = 'false';
-    const c = buildRetrievalReflexCheck(null);
-    expect(c.name).toBe('retrieval_reflex_health');
-    expect(c.status).toBe('warn');
-    expect(c.message).toContain('disabled');
-    expect((c.details as any)?.enabled).toBe(false);
+  test('disabled via env → warn, names the right check', async () => {
+    await withEnv({ GBRAIN_RETRIEVAL_REFLEX: 'false' }, async () => {
+      const c = buildRetrievalReflexCheck(null);
+      expect(c.name).toBe('retrieval_reflex_health');
+      expect(c.status).toBe('warn');
+      expect(c.message).toContain('disabled');
+      expect((c.details as any)?.enabled).toBe(false);
+    });
  });

-  test('enabled → reports policy-skill install state in details', () => {
-    process.env.GBRAIN_RETRIEVAL_REFLEX = 'true';
-    const dir = mkdtempSync(join(tmpdir(), 'rr-doctor-'));
-    mkdirSync(join(dir, 'retrieval-reflex'), { recursive: true });
-    writeFileSync(join(dir, 'retrieval-reflex', 'SKILL.md'), '# stub\n');
-    const c = buildRetrievalReflexCheck(dir);
-    expect(c.name).toBe('retrieval_reflex_health');
-    expect((c.details as any)?.enabled).toBe(true);
-    expect((c.details as any)?.policy_skill_installed).toBe(true);
-    rmSync(dir, { recursive: true, force: true });
+  test('enabled → reports policy-skill install state in details', async () => {
+    await withEnv({ GBRAIN_RETRIEVAL_REFLEX: 'true' }, async () => {
+      const dir = mkdtempSync(join(tmpdir(), 'rr-doctor-'));
+      try {
+        mkdirSync(join(dir, 'retrieval-reflex'), { recursive: true });
+        writeFileSync(join(dir, 'retrieval-reflex', 'SKILL.md'), '# stub\n');
+        const c = buildRetrievalReflexCheck(dir);
+        expect(c.name).toBe('retrieval_reflex_health');
+        expect((c.details as any)?.enabled).toBe(true);
+        expect((c.details as any)?.policy_skill_installed).toBe(true);
+      } finally {
+        rmSync(dir, { recursive: true, force: true });
+      }
+    });
  });

-  test('enabled, policy skill absent → message includes the install hint', () => {
-    process.env.GBRAIN_RETRIEVAL_REFLEX = 'true';
-    const dir = mkdtempSync(join(tmpdir(), 'rr-doctor-2-'));
-    const c = buildRetrievalReflexCheck(dir);
-    expect((c.details as any)?.policy_skill_installed).toBe(false);
-    expect(c.message).toContain('gbrain integrations install retrieval-reflex');
-    rmSync(dir, { recursive: true, force: true });
+  test('enabled, policy skill absent → message includes the install hint', async () => {
+    await withEnv({ GBRAIN_RETRIEVAL_REFLEX: 'true' }, async () => {
+      const dir = mkdtempSync(join(tmpdir(), 'rr-doctor-2-'));
+      try {
+        const c = buildRetrievalReflexCheck(dir);
+        expect((c.details as any)?.policy_skill_installed).toBe(false);
+        expect(c.message).toContain('gbrain integrations install retrieval-reflex');
+      } finally {
+        rmSync(dir, { recursive: true, force: true });
+      }
+    });
  });
 });
@@ -8,6 +8,7 @@
 * shape the serve IPC / host ctx.brainQuery supply).
 */
 import { describe, test, expect, beforeAll, afterAll, beforeEach } from 'bun:test';
+import { withEnv } from './helpers/with-env.ts';
 import { PGLiteEngine } from '../src/core/pglite-engine.ts';
 import { normalizeAlias } from '../src/core/search/alias-normalize.ts';
 import { resolveEntitiesToPointers } from '../src/core/context/retrieval-reflex.ts';
@@ -108,68 +109,77 @@ describe('resolveEntitiesToPointers', () => {
 });

 describe('context-engine assemble() — Retrieval Reflex integration', () => {
-  beforeEach(() => {
-    process.env.GBRAIN_RETRIEVAL_REFLEX = 'true';
-  });
+  // R1 isolation: scope the reflex flag to each test via withEnv (the prior
+  // beforeEach mutated process.env for the whole shard and never restored it).
+  const withReflexOn = (fn: () => Promise<void>) =>
+    withEnv({ GBRAIN_RETRIEVAL_REFLEX: 'true' }, fn);

  test('regression: a named entity with a page surfaces a pointer (host resolver path)', async () => {
-    await seed('people/alice-example', 'Alice Example', 'Alice is a founder.');
-    // Inject a resolver the way the OpenClaw host (ctx.brainQuery) or serve IPC would.
-    const ce = createGBrainContextEngine({
-      workspaceDir: '/tmp/rr-test-ws',
-      resolveEntities: (candidates, opts) =>
-        resolveEntitiesToPointers(engine, 'default', candidates, opts),
+    await withReflexOn(async () => {
+      await seed('people/alice-example', 'Alice Example', 'Alice is a founder.');
+      // Inject a resolver the way the OpenClaw host (ctx.brainQuery) or serve IPC would.
+      const ce = createGBrainContextEngine({
+        workspaceDir: '/tmp/rr-test-ws',
+        resolveEntities: (candidates, opts) =>
+          resolveEntitiesToPointers(engine, 'default', candidates, opts),
+      });
+      const res = await ce.assemble({
+        sessionId: 's1',
+        messages: [{ role: 'user', content: 'what do you think about Alice Example?' }],
+      });
+      expect(res.systemPromptAddition).toContain('Brain pages mentioned this turn');
+      expect(res.systemPromptAddition).toContain('people/alice-example');
+      expect(res.systemPromptAddition).toContain('use get_page');
    });
-    const res = await ce.assemble({
-      sessionId: 's1',
-      messages: [{ role: 'user', content: 'what do you think about Alice Example?' }],
-    });
-    expect(res.systemPromptAddition).toContain('Brain pages mentioned this turn');
-    expect(res.systemPromptAddition).toContain('people/alice-example');
-    expect(res.systemPromptAddition).toContain('use get_page');
  });

  test('no resolver available (PGLite, no serve/host) → no throw, live context still present', async () => {
-    const ce = createGBrainContextEngine({ workspaceDir: '/tmp/rr-test-ws-2' });
-    const res = await ce.assemble({
-      sessionId: 's2',
-      messages: [{ role: 'user', content: 'what about Alice Example?' }],
+    await withReflexOn(async () => {
+      const ce = createGBrainContextEngine({ workspaceDir: '/tmp/rr-test-ws-2' });
+      const res = await ce.assemble({
+        sessionId: 's2',
+        messages: [{ role: 'user', content: 'what about Alice Example?' }],
+      });
+      // Live Context block always ships; no pointer block (nothing resolved).
+      expect(res.systemPromptAddition).toContain('Live Context');
+      expect(res.systemPromptAddition).not.toContain('Brain pages mentioned this turn');
    });
-    // Live Context block always ships; no pointer block (nothing resolved).
-    expect(res.systemPromptAddition).toContain('Live Context');
-    expect(res.systemPromptAddition).not.toContain('Brain pages mentioned this turn');
  });

  test('zero salient candidates → no brain touch, no pointer block', async () => {
-    let called = false;
-    const ce = createGBrainContextEngine({
-      workspaceDir: '/tmp/rr-test-ws-3',
-      resolveEntities: async () => { called = true; return null; },
+    await withReflexOn(async () => {
+      let called = false;
+      const ce = createGBrainContextEngine({
+        workspaceDir: '/tmp/rr-test-ws-3',
+        resolveEntities: async () => { called = true; return null; },
+      });
+      const res = await ce.assemble({
+        sessionId: 's3',
+        messages: [{ role: 'user', content: 'can you help me with this?' }],
+      });
+      expect(called).toBe(false);
+      expect(res.systemPromptAddition).not.toContain('Brain pages mentioned this turn');
    });
-    const res = await ce.assemble({
-      sessionId: 's3',
-      messages: [{ role: 'user', content: 'can you help me with this?' }],
-    });
-    expect(called).toBe(false);
-    expect(res.systemPromptAddition).not.toContain('Brain pages mentioned this turn');
  });

  test('suppression uses PRIOR turns only, not the current message', async () => {
-    await seed('people/alice-example', 'Alice Example', 'A founder.');
-    const ce = createGBrainContextEngine({
-      workspaceDir: '/tmp/rr-test-ws-4',
-      resolveEntities: (candidates, opts) =>
-        resolveEntitiesToPointers(engine, 'default', candidates, opts),
+    await withReflexOn(async () => {
+      await seed('people/alice-example', 'Alice Example', 'A founder.');
+      const ce = createGBrainContextEngine({
+        workspaceDir: '/tmp/rr-test-ws-4',
+        resolveEntities: (candidates, opts) =>
+          resolveEntitiesToPointers(engine, 'default', candidates, opts),
+      });
+      // The current message names Alice Example; prior context does NOT. Must fire.
+      const res = await ce.assemble({
+        sessionId: 's4',
+        messages: [
+          { role: 'user', content: 'hello' },
+          { role: 'assistant', content: 'hi there' },
+          { role: 'user', content: 'what do you think about Alice Example?' },
+        ],
+      });
+      expect(res.systemPromptAddition).toContain('people/alice-example');
    });
-    // The current message names Alice Example; prior context does NOT. Must fire.
-    const res = await ce.assemble({
-      sessionId: 's4',
-      messages: [
-        { role: 'user', content: 'hello' },
-        { role: 'assistant', content: 'hi there' },
-        { role: 'user', content: 'what do you think about Alice Example?' },
-      ],
-    });
-    expect(res.systemPromptAddition).toContain('people/alice-example');
  });
 });