diff --git a/src/core/cycle.ts b/src/core/cycle.ts index 169f71fb2..0b73596ed 100644 --- a/src/core/cycle.ts +++ b/src/core/cycle.ts @@ -2063,6 +2063,13 @@ export async function runCycle( yieldDuringPhase: opts.yieldDuringPhase, once: opts.onceForPhase === 'patterns', deadlineAtMs: opts.deadlineAtMs ?? null, + // #1586: scope pattern writes to the cycle's resolved source, same as + // synthesize above. Without it the child's put_page rows land in + // 'default' while the reverse-write drops the file into the named + // source's checkout — the row and the file disagree about which + // source owns the page, which is what doctor reports as + // multi_source_drift. + sourceId: cycleSourceId, })); result.duration_ms = duration_ms; phaseResults.push(result); diff --git a/src/core/cycle/patterns.ts b/src/core/cycle/patterns.ts index 0673f718e..2da0d18fe 100644 --- a/src/core/cycle/patterns.ts +++ b/src/core/cycle/patterns.ts @@ -60,6 +60,13 @@ export interface PatternsPhaseOpts { * mid-phase and starves every tail phase (#2781). */ deadlineAtMs?: number | null; + /** + * #1586: the cycle's resolved source. Stamped onto every subagent child as + * `source_id` so put_page writes land in this source's rows, and passed to + * reverseWriteRefs so getPage/getTags read the correct (source_id, slug) + * row. Unset → legacy 'default'. Mirrors synthesize.ts's `sourceId`. + */ + sourceId?: string; } /** @@ -197,6 +204,9 @@ export async function runPhasePatterns( model: config.model, max_turns: 30, allowed_slug_prefixes: allowedSlugPrefixes, + // #1586: scope every child tool call to the cycle's resolved source so + // put_page writes land there instead of the hardcoded 'default'. + ...(opts.sourceId ? { source_id: opts.sourceId } : {}), }; const submitOpts: Partial = { max_stalled: 3, @@ -243,10 +253,14 @@ export async function runPhasePatterns( // Collect refs the subagent wrote (codex finding #2 — query tool exec rows). // v0.32.8: refs carry source_id so reverseWriteRefs targets the right // (source, slug) row instead of the first DB match. - const writtenRefs = await collectChildPutPageSlugs(engine, [job.id]); + // #1586: refs carry the cycle's resolved source (children wrote there via + // SubagentHandlerData.source_id), so getPage/getTags read the same row the + // child wrote, and the reverse-write treats it as the native source. + const cycleSourceId = opts.sourceId ?? 'default'; + const writtenRefs = await collectChildPutPageSlugs(engine, [job.id], cycleSourceId); // Reverse-write to fs. - const reverseWriteCount = await reverseWriteRefs(engine, opts.brainDir, writtenRefs); + const reverseWriteCount = await reverseWriteRefs(engine, opts.brainDir, writtenRefs, cycleSourceId); const details = { reflections_considered: reflections.length, @@ -454,13 +468,14 @@ When done, briefly list the pattern slugs you wrote/updated in your final messag async function collectChildPutPageSlugs( engine: BrainEngine, childIds: number[], + sourceId = 'default', ): Promise> { if (childIds.length === 0) return []; // v0.32.8: subagent put_page tool schema doesn't expose source_id (subagents - // are scoped to a single source). Default to 'default' here; multi-source - // dream cycles are a v0.33 follow-up. The point of threading source_id is - // so reverseWriteRefs can pass it through getPage and pick the correct - // (source_id, slug) row instead of whatever the DB happens to return. + // are scoped to a single source). #1586: stamp the cycle's resolved source — + // children write there via SubagentHandlerData.source_id — so reverseWriteRefs + // can pass it through getPage and pick the correct (source_id, slug) row + // instead of whatever the DB happens to return. Unset → legacy 'default'. const rows = await engine.executeRaw<{ slug: string }>( `SELECT DISTINCT COALESCE(input->>'slug', (input #>> '{}')::jsonb->>'slug') AS slug @@ -474,7 +489,7 @@ async function collectChildPutPageSlugs( return rows .map(r => r.slug) .filter((s): s is string => typeof s === 'string' && s.length > 0) - .map(slug => ({ slug, source_id: 'default' })); + .map(slug => ({ slug, source_id: sourceId })); } // ── Reverse-write ──────────────────────────────────────────────────── @@ -485,6 +500,7 @@ async function reverseWriteRefs( engine: BrainEngine, brainDir: string, refs: Array<{ slug: string; source_id: string }>, + nativeSourceId = 'default', ): Promise { let count = 0; for (const { slug, source_id } of refs) { @@ -496,11 +512,12 @@ async function reverseWriteRefs( const tags = await engine.getTags(slug, { sourceId: source_id }); try { const md = renderPageToMarkdown(page, tags); - // v0.32.8 F6: non-default sources land under brainDir/.sources//.md - // so same-slug-different-source pages don't collide on disk. Default-source - // pages stay at brainDir/.md so single-source brains see no change. - // `.sources/` is a reserved prefix; walkBrainRepo skips dot-dirs. - const filePath = source_id === 'default' + // v0.32.8 F6: foreign-source pages land under brainDir/.sources//.md + // so same-slug-different-source pages don't collide on disk. Pages belonging + // to the cycle's own source (#1586: brainDir IS that source's checkout — + // legacy 'default' when unscoped) stay at brainDir/.md so single-source + // brains see no change. `.sources/` is a reserved prefix; walkBrainRepo skips dot-dirs. + const filePath = source_id === nativeSourceId ? join(brainDir, `${slug}.md`) : join(brainDir, '.sources', source_id, `${slug}.md`); mkdirSync(dirname(filePath), { recursive: true }); @@ -558,3 +575,11 @@ function failed(error: PhaseError): PhaseResult { function makeError(cls: string, code: string, message: string, hint?: string): PhaseError { return hint ? { class: cls, code, message, hint } : { class: cls, code, message }; } + +// `__testing` re-exports otherwise-private helpers so unit tests can pin the +// source-scoping contract (#1586) without driving a whole dream cycle. +// Mirrors synthesize.ts's `__testing` block. +export const __testing = { + collectChildPutPageSlugs, + reverseWriteRefs, +}; diff --git a/test/cycle-patterns-source-scope.test.ts b/test/cycle-patterns-source-scope.test.ts new file mode 100644 index 000000000..2b7cfb5d3 --- /dev/null +++ b/test/cycle-patterns-source-scope.test.ts @@ -0,0 +1,114 @@ +/** + * Regression for #1586's unfixed half: the patterns phase. + * + * #1586 threaded the cycle's resolved source through `synthesize.ts` so dream + * output lands in the named source's `(source_id, slug)` rows. `patterns.ts` + * was left on the pre-#1586 shape: `collectChildPutPageSlugs` stamped a literal + * `'default'` and `reverseWriteRefs` compared against a literal `'default'`. + * + * The result on a per-source cycle was a page filed against the wrong source: + * the ROW is created under `default` (the child had no `source_id` to scope + * its put_page calls), while the reverse-write drops the FILE into the named + * source's checkout — `source_id === 'default'` selected the + * `brainDir/.md` branch, and on a per-source cycle `brainDir` IS the + * named source's checkout. Row and file then disagree about which source owns + * the page, which is what `doctor` reports as `multi_source_drift`. + * + * These tests pin both halves of the fix, plus the unscoped legacy behavior. + */ + +import { describe, test, expect, beforeAll, afterAll } from 'bun:test'; +import { mkdtempSync, rmSync, existsSync } from 'node:fs'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; +import { PGLiteEngine } from '../src/core/pglite-engine.ts'; +import { __testing } from '../src/core/cycle/patterns.ts'; + +const { collectChildPutPageSlugs, reverseWriteRefs } = __testing; + +const SOURCE_ID = 'coast'; +const SLUG = 'wiki/personal/patterns/a-pattern'; + +let engine: PGLiteEngine; +let brainDir: string; + +beforeAll(async () => { + engine = new PGLiteEngine(); + await engine.connect({}); + await engine.initSchema(); + + brainDir = mkdtempSync(join(tmpdir(), 'gbrain-patterns-scope-')); + + const db = (engine as any).db; + await db.exec(` + INSERT INTO sources (id, name, local_path) + VALUES ('${SOURCE_ID}', 'coast', '/tmp/coast-brain') + ON CONFLICT (id) DO NOTHING; + `); + await db.exec(` + INSERT INTO minion_jobs (id, queue, name, data, status) + VALUES (2001, 'default', 'subagent', '{}'::jsonb, 'completed') + ON CONFLICT (id) DO NOTHING; + `); + await db.query( + `INSERT INTO subagent_tool_executions (job_id, message_idx, tool_use_id, tool_name, status, input) + VALUES (2001, 0, 'tool_p1', 'brain_put_page', 'complete', $1::jsonb)`, + [JSON.stringify({ slug: SLUG, body: 'a pattern' })], + ); + + // The page the child wrote lives in the cycle's source, not in 'default'. + await engine.putPage(SLUG, { + type: 'note', + title: 'A pattern', + compiled_truth: 'a pattern', + } as any, { sourceId: SOURCE_ID }); +}); + +afterAll(async () => { + await engine.disconnect(); + rmSync(brainDir, { recursive: true, force: true }); +}); + +describe('#1586: the patterns phase scopes its writes to the cycle source', () => { + test('collected refs carry the cycle source, not a hardcoded default', async () => { + const refs = await collectChildPutPageSlugs(engine as any, [2001], SOURCE_ID); + expect(refs).toHaveLength(1); + expect(refs[0]!.slug).toBe(SLUG); + // Pre-fix this was the literal 'default' no matter which source the cycle + // resolved, which is what filed the page against the wrong source. + expect(refs[0]!.source_id).toBe(SOURCE_ID); + }); + + test('unscoped callers keep the legacy default source', async () => { + const refs = await collectChildPutPageSlugs(engine as any, [2001]); + expect(refs[0]!.source_id).toBe('default'); + }); + + test('reverse-write resolves the row the child actually wrote', async () => { + const refs = await collectChildPutPageSlugs(engine as any, [2001], SOURCE_ID); + const count = await reverseWriteRefs(engine as any, brainDir, refs, SOURCE_ID); + // The lookup is keyed on the ref's source_id, so a ref carrying the cycle + // source resolves the row the child wrote there. + expect(count).toBe(1); + }); + + test('the cycle source is native — its pages stay at brainDir/.md', async () => { + const refs = await collectChildPutPageSlugs(engine as any, [2001], SOURCE_ID); + await reverseWriteRefs(engine as any, brainDir, refs, SOURCE_ID); + expect(existsSync(join(brainDir, `${SLUG}.md`))).toBe(true); + expect(existsSync(join(brainDir, '.sources', SOURCE_ID, `${SLUG}.md`))).toBe(false); + }); + + test('a foreign source still lands under .sources//', async () => { + const foreignDir = mkdtempSync(join(tmpdir(), 'gbrain-patterns-foreign-')); + try { + const refs = await collectChildPutPageSlugs(engine as any, [2001], SOURCE_ID); + // brainDir belongs to 'default' here, so 'coast' is foreign to it. + await reverseWriteRefs(engine as any, foreignDir, refs, 'default'); + expect(existsSync(join(foreignDir, '.sources', SOURCE_ID, `${SLUG}.md`))).toBe(true); + expect(existsSync(join(foreignDir, `${SLUG}.md`))).toBe(false); + } finally { + rmSync(foreignDir, { recursive: true, force: true }); + } + }); +});