mirror of
https://github.com/garrytan/gbrain.git
synced 2026-08-14 00:48:18 +00:00
#1586 threaded the cycle's resolved source through the synthesize phase so subagent-written pages land in that source's (source_id, slug) rows. The patterns phase was not updated and kept the pre-#1586 shape: it stamped a literal 'default' on every collected ref and compared the reverse-write path against a literal 'default'. On a per-source cycle that files the page 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, because source_id === 'default' selects the brainDir/<slug>.md branch and brainDir IS that source's checkout. Row and file then disagree about which source owns the page — what doctor reports as multi_source_drift. Applies the same threading synthesize.ts already uses: PatternsPhaseOpts gains sourceId, cycle.ts passes cycleSourceId at the patterns call site the way it already does for synthesize, the child carries SubagentHandlerData.source_id, and reverseWriteRefs takes the cycle source as its native source. Unset stays 'default', so unscoped callers are unchanged.
This commit is contained in:
@@ -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);
|
||||
|
||||
+37
-12
@@ -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<MinionJobInput> = {
|
||||
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<Array<{ slug: string; source_id: string }>> {
|
||||
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<number> {
|
||||
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/<id>/<slug>.md
|
||||
// so same-slug-different-source pages don't collide on disk. Default-source
|
||||
// pages stay at brainDir/<slug>.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/<id>/<slug>.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/<slug>.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,
|
||||
};
|
||||
|
||||
@@ -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/<slug>.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/<slug>.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/<id>/', 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 });
|
||||
}
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user