From 98222a08cb767c613a0a6c7b65d6f4e20be6e208 Mon Sep 17 00:00:00 2001 From: Garry Tan Date: Sun, 24 May 2026 22:20:16 -0700 Subject: [PATCH] fix(cycle/synthesize): refuse empty brainDir + resolve relative paths MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Pre-fix, runPhaseSynthesize accepted any brainDir string and passed it to writeReversePages which does join(brainDir, '.md'). When brainDir is '' or relative ('.' / './brain' / etc), join() produces a relative path that writeFileSync resolves against cwd. Result: every synthesize reverse-write spills into /companies/.md, /people/.md, etc. instead of the intended brainDir tempdir. Surfaced by the warm-narwhal wave when E2E test cleanup found orphan synthesize pages (companies/novamind.md, people/sarah-chen.md, meetings/2025-04-01-novamind-board-update.md) at the gbrain repo root from a runCycle({brainDir: '.'}) chain that ran during morning E2E execution. Fix at the function entry, single location, all callers protected: 1. Empty/whitespace brainDir → return failed(BRAINDIR_EMPTY) loud instead of silently resolving against cwd 2. Relative brainDir → resolve(opts.brainDir) before any read/write can use it. opts.brainDir mutated so writeReversePages, writeSummaryPage, and every join() downstream see the absolute path Regression test pins all 4 contracts: - empty string → fail(BRAINDIR_EMPTY) - whitespace-only → fail(BRAINDIR_EMPTY) - '.' → mutated to absolute on entry - already-absolute → unchanged Co-Authored-By: Claude Opus 4.7 --- src/core/cycle/synthesize.ts | 19 ++++- .../cycle-synthesize-braindir-resolve.test.ts | 79 +++++++++++++++++++ 2 files changed, 97 insertions(+), 1 deletion(-) create mode 100644 test/cycle-synthesize-braindir-resolve.test.ts diff --git a/src/core/cycle/synthesize.ts b/src/core/cycle/synthesize.ts index e701bddc4..ea86f3959 100644 --- a/src/core/cycle/synthesize.ts +++ b/src/core/cycle/synthesize.ts @@ -28,7 +28,7 @@ import Anthropic from '@anthropic-ai/sdk'; import { readFileSync, existsSync, writeFileSync, mkdirSync } from 'node:fs'; -import { join, dirname } from 'node:path'; +import { join, dirname, isAbsolute, resolve } from 'node:path'; import type { BrainEngine } from '../engine.ts'; import type { PhaseResult, PhaseError } from '../cycle.ts'; import { MinionQueue } from '../minions/queue.ts'; @@ -239,6 +239,23 @@ export async function runPhaseSynthesize( opts: SynthesizePhaseOpts, ): Promise { const start = Date.now(); + // Normalize brainDir to an absolute path BEFORE any reverse-write. Without + // this, a relative or empty brainDir flows down to writeReversePages → + // `join(brainDir, '${slug}.md')` → relative path → resolves against cwd at + // writeFileSync time, spilling synthesize output into whatever directory + // the cycle ran from (e.g., `companies/novamind.md` at the repo root). + // Surfaced by the warm-narwhal wave when E2E test cleanup found orphan + // synthesize pages at repo root from a `runCycle({brainDir: '.'})` call + // chain. Throw on empty (silent cwd-resolution is worse than a loud + // failure); resolve if relative (`.` / `./brain` / `../sibling` all valid + // inputs but must canonicalize before the write). + if (!opts.brainDir || opts.brainDir.trim() === '') { + return failed(makeError('InternalError', 'BRAINDIR_EMPTY', + 'opts.brainDir is empty; refusing to run synthesize. Pass an absolute path.')); + } + if (!isAbsolute(opts.brainDir)) { + opts.brainDir = resolve(opts.brainDir); + } try { const config = await loadSynthConfig(engine); diff --git a/test/cycle-synthesize-braindir-resolve.test.ts b/test/cycle-synthesize-braindir-resolve.test.ts new file mode 100644 index 000000000..ac6ddf3d4 --- /dev/null +++ b/test/cycle-synthesize-braindir-resolve.test.ts @@ -0,0 +1,79 @@ +/** + * Regression: synthesize phase MUST refuse to write reverse-pages to a + * relative brainDir. Pre-fix, `runCycle({brainDir: '.'})` or any caller + * passing a relative path (or empty string) would silently let + * writeFileSync resolve against cwd, spilling synthesize output into + * `/companies/novamind.md` etc. Surfaced by the warm-narwhal wave + * when E2E test cleanup found orphan synthesize pages at repo root. + * + * Two contracts pinned here: + * 1. Empty/whitespace-only brainDir → returns failed() with code + * `BRAINDIR_EMPTY` (loud, not silent cwd resolution). + * 2. Relative brainDir → resolved to absolute via path.resolve() before + * any reverse-write can use it. Verified by checking opts.brainDir + * after the call returns. + * + * Doesn't drive Anthropic — synthesize hits the "not_configured" skip + * branch first (no corpus dir set), which is sufficient to exercise the + * brainDir gate at function entry. + */ + +import { describe, test, expect, beforeAll, afterAll } from 'bun:test'; +import { mkdtempSync, rmSync } from 'node:fs'; +import { tmpdir } from 'node:os'; +import { join, isAbsolute } from 'node:path'; +import { PGLiteEngine } from '../src/core/pglite-engine.ts'; +import { runPhaseSynthesize } from '../src/core/cycle/synthesize.ts'; + +let engine: PGLiteEngine; +let tmpDir: string; + +beforeAll(async () => { + engine = new PGLiteEngine(); + await engine.connect({}); + await engine.initSchema(); + tmpDir = mkdtempSync(join(tmpdir(), 'synth-braindir-')); +}); + +afterAll(async () => { + await engine.disconnect(); + try { rmSync(tmpDir, { recursive: true, force: true }); } catch { /* */ } +}); + +describe('runPhaseSynthesize brainDir resolution (regression)', () => { + test('empty brainDir returns failed(BRAINDIR_EMPTY) instead of silently resolving against cwd', async () => { + const result = await runPhaseSynthesize(engine, { + brainDir: '', + dryRun: true, + }); + expect(result.status).toBe('fail'); + expect((result as { error?: { code?: string } }).error?.code).toBe('BRAINDIR_EMPTY'); + }); + + test('whitespace-only brainDir also fails BRAINDIR_EMPTY', async () => { + const result = await runPhaseSynthesize(engine, { + brainDir: ' ', + dryRun: true, + }); + expect(result.status).toBe('fail'); + expect((result as { error?: { code?: string } }).error?.code).toBe('BRAINDIR_EMPTY'); + }); + + test('relative brainDir gets resolved to absolute before any reverse-write', async () => { + const opts = { brainDir: '.', dryRun: true }; + // The phase will return early ('not_configured' — no corpus dir set on + // this fresh engine) but the normalization runs unconditionally at entry. + await runPhaseSynthesize(engine, opts); + // After the call, opts.brainDir should be the resolved absolute path, + // proving the normalization fired. + expect(isAbsolute(opts.brainDir)).toBe(true); + expect(opts.brainDir).not.toBe('.'); + }); + + test('absolute brainDir is preserved unchanged', async () => { + const opts = { brainDir: tmpDir, dryRun: true }; + await runPhaseSynthesize(engine, opts); + // Already absolute → no mutation. + expect(opts.brainDir).toBe(tmpDir); + }); +});