mirror of
https://github.com/garrytan/gbrain.git
synced 2026-08-14 08:53:22 +00:00
v0.45.9.0 fix(bootstrap): preview source_id + create brain/ eagerly in hooks phase (#4064)
* fix(bootstrap): preview source_id + create brain/ eagerly in hooks phase
`bootstrap render`/`hooks` never told a human what source_id the
workspace expects until `verify` (the only engine-holding phase) ran.
A human who hand-registered a source before that point would guess an
"intuitive" name, hit an FK error on the first `verify` roundtrip (the
guessed id has no `sources` row), then hit `overlapping_path` on the
retry (their first guess still claims the same brain/ dir) — three
round trips to land the right id.
`hooks` is the last ENGINE-FREE phase before `verify`, and already
knows both the manifest's current source_id and the workspace path, so
it now:
- creates `<ws>/brain` eagerly (idempotent mkdir), removing the
manual-mkdir step before `git init && sources add`
- prints the exact `gbrain sources add <source_id> --path <brain>`
command
- previews the collision-fallback id verify would derive
(`workspace-<hash>`) — a pure function of the workspace's real
path, so it needs no DB lookup and is safe to preview engine-free
The collision-fallback derivation itself is unchanged; it is now
factored into an exported `deriveWorkspaceSourceId()` in verify.ts so
both call sites (the new hooks preview and the existing
`resolveSourceIdCollision`) share one formula instead of two copies
drifting apart.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016JNV9ABwb32DZEwvo7P4ay
* fix(bootstrap): --force the preview command; quote paths; fix runbook
Codex review round 2 caught three issues with the source_id preview
added in the prior commit:
- The printed `gbrain sources add <id> --path <brain>` command failed
immediately on a pristine bootstrap: the brain/ dir this phase just
created is empty (no git history), so `sources add --path` fail-fasts
as `not_a_git_repo` (#2707). Fixed by appending `--force` — the same
sanctioned opt-in `test/bootstrap-verify.serial.test.ts` already uses
to register a brand-new brain/ before any content exists
(`addSource(engine, { id: 'workspace', localPath: ..., force: true })`).
Safe here specifically because brainDir is the fixed
`<workspace>/brain` path this phase just created, not an arbitrary
user path.
- brainDir was interpolated unquoted; a workspace path containing a
space broke the printed command. Added a local
`shellQuoteForDisplay()` (mirroring the existing private `shellQuote`
already duplicated in hooks.ts / sources-ops.ts / connect.ts).
- The dispatcher test only pattern-matched the collision-fallback id's
shape (`workspace-[0-9a-f]{8}`) instead of pinning exact equality
with `deriveWorkspaceSourceId()`, so preview/verify drift could pass
silently. Now asserts exact equality, plus a new test for the space-
quoting fix.
Also corrects BOOTSTRAP_FOR_AGENTS.md's runbook step 5, which claimed
skill scaffolding "registers `brain/` as the workspace source" — no
code path does this automatically (confirmed by grep); the step now
points at the `hooks` phase's actual preview + --force command instead
of telling the installing agent there is "nothing to judge" on a step
that silently never ran.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016JNV9ABwb32DZEwvo7P4ay
---------
Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
This commit is contained in:
@@ -22,6 +22,7 @@ import { runBootstrap, workspaceBrainStats } from '../src/commands/bootstrap.ts'
|
||||
import type { ExecRunner } from '../src/core/bootstrap/repo.ts';
|
||||
import { attachWorkspace } from '../src/core/bootstrap/attach.ts';
|
||||
import { readReceipt, receiptPath, writeManifest, type InstallReceipt } from '../src/core/bootstrap/format.ts';
|
||||
import { deriveWorkspaceSourceId } from '../src/core/bootstrap/verify.ts';
|
||||
import { initState, setAnswer, skipAnswer, confirm, readBackHash } from '../src/core/bootstrap/interview.ts';
|
||||
|
||||
let tmpParent: string; // GBRAIN_HOME parent (configDir appends .gbrain)
|
||||
@@ -121,6 +122,147 @@ describe('consent-skip is a decline (HOOKS_CONSENT)', () => {
|
||||
}, 30_000);
|
||||
});
|
||||
|
||||
describe('hooks previews the source_id + creates brain/ eagerly [defect: multi-round-trip source registration]', () => {
|
||||
// Self-contained fixtures (same pattern as the flip block below): the
|
||||
// file-level `ws` is shared by other describes, so this needs its own.
|
||||
const scratch: string[] = [];
|
||||
function freshWorkspace(): { fws: string; fhome: string; fparent: string } {
|
||||
const fparent = mkdtempSync(join(tmpdir(), 'gb-srcid-'));
|
||||
const fhome = join(fparent, '.gbrain');
|
||||
mkdirSync(fhome, { recursive: true });
|
||||
const fws = mkdtempSync(join(tmpdir(), 'gb-srcid-ws-'));
|
||||
scratch.push(fparent, fws);
|
||||
const prev = process.env.GBRAIN_HOME;
|
||||
process.env.GBRAIN_HOME = fparent;
|
||||
try {
|
||||
expect(initState(fws).ok).toBe(true);
|
||||
for (const [key, value] of Object.entries(REQUIRED_ANSWERS)) {
|
||||
const r = setAnswer(fws, key, value);
|
||||
if (!r.ok) throw new Error(r.message);
|
||||
}
|
||||
expect(setAnswer(fws, 'MCP_SCOPE', 'project').ok).toBe(true);
|
||||
const h = readBackHash(fws);
|
||||
if (!h.ok) throw new Error(h.message);
|
||||
expect(confirm(fws, h.hash).ok).toBe(true);
|
||||
} finally {
|
||||
if (prev === undefined) delete process.env.GBRAIN_HOME;
|
||||
else process.env.GBRAIN_HOME = prev;
|
||||
}
|
||||
return { fws, fhome, fparent };
|
||||
}
|
||||
afterAll(() => {
|
||||
for (const d of scratch) rmSync(d, { recursive: true, force: true });
|
||||
});
|
||||
|
||||
async function withHome<T>(parent: string, fn: () => Promise<T>): Promise<T> {
|
||||
const prev = process.env.GBRAIN_HOME;
|
||||
process.env.GBRAIN_HOME = parent;
|
||||
try {
|
||||
return await fn();
|
||||
} finally {
|
||||
if (prev === undefined) delete process.env.GBRAIN_HOME;
|
||||
else process.env.GBRAIN_HOME = prev;
|
||||
}
|
||||
}
|
||||
|
||||
test('hooks prints the exact `sources add` command up front and creates brain/ before verify ever runs', async () => {
|
||||
const { fws, fparent } = freshWorkspace();
|
||||
const brainDir = join(fws, 'brain');
|
||||
const out = await withHome(fparent, async () => {
|
||||
expect((await capture(() => runBootstrap(['render', '--workspace', fws]))).result).toBe(0);
|
||||
// render is engine-free and never touches brain/ — the gap this fix closes.
|
||||
expect(existsSync(brainDir)).toBe(false);
|
||||
const { runner } = makeRunner();
|
||||
return capture(() =>
|
||||
runBootstrap(['hooks', '--workspace', fws, '--harness', 'claude-code', '--gbrain-bin', process.execPath], {
|
||||
runner,
|
||||
}),
|
||||
);
|
||||
});
|
||||
expect(out.result).toBe(0);
|
||||
// brain/ now exists — ready for `sources add` without a manual mkdir
|
||||
// round trip.
|
||||
expect(existsSync(brainDir)).toBe(true);
|
||||
// The manifest's actual source_id (default: 'workspace' for a first
|
||||
// render) is spelled out verbatim — no guessing an "intuitive" name that
|
||||
// would only surface as an FK error three steps later at verify time.
|
||||
// --force is required and printed: brain/ is freshly created and has no
|
||||
// git history yet, so `sources add` without --force would fail-fast on
|
||||
// `not_a_git_repo` (#2707) the instant the printed command is pasted —
|
||||
// the exact same `{ force: true }` the engine-backed verify fixtures use
|
||||
// to register a brand new brain/ (test/bootstrap-verify.serial.test.ts).
|
||||
expect(out.out).toContain(`gbrain sources add workspace --path ${brainDir} --force`);
|
||||
// The collision-fallback id is previewed too, pinned to the SAME formula
|
||||
// verify.ts would derive (not just the id's shape) — so preview/verify
|
||||
// drift, or a change to the derivation formula, fails this test.
|
||||
expect(out.out).toContain(`'${deriveWorkspaceSourceId(fws)}'`);
|
||||
expect(deriveWorkspaceSourceId(fws)).toMatch(/^workspace-[0-9a-f]{8}$/);
|
||||
}, 30_000);
|
||||
|
||||
test('a --repair re-run is idempotent: brain/ survives, the same preview reprints', async () => {
|
||||
const { fws, fparent } = freshWorkspace();
|
||||
const brainDir = join(fws, 'brain');
|
||||
await withHome(fparent, async () => {
|
||||
expect((await capture(() => runBootstrap(['render', '--workspace', fws]))).result).toBe(0);
|
||||
expect((await capture(() => runBootstrap(['hooks', '--workspace', fws, '--harness', 'claude-code', '--gbrain-bin', process.execPath], { runner: makeRunner().runner }))).result).toBe(0);
|
||||
// Simulate the human having already registered + committed into brain/.
|
||||
writeFileSync(join(brainDir, 'probe.md'), '# probe\n');
|
||||
const repaired = await capture(() =>
|
||||
runBootstrap(
|
||||
['hooks', '--workspace', fws, '--harness', 'claude-code', '--repair', '--gbrain-bin', process.execPath],
|
||||
{ runner: makeRunner().runner },
|
||||
),
|
||||
);
|
||||
expect(repaired.result).toBe(0);
|
||||
expect(repaired.out).toContain(`gbrain sources add workspace --path ${brainDir} --force`);
|
||||
// Idempotent mkdir never clobbers what the human already committed.
|
||||
expect(existsSync(join(brainDir, 'probe.md'))).toBe(true);
|
||||
});
|
||||
}, 30_000);
|
||||
|
||||
test('a workspace path containing a space is shell-quoted in the printed command', async () => {
|
||||
const fparent = mkdtempSync(join(tmpdir(), 'gb-srcid-space-'));
|
||||
scratch.push(fparent);
|
||||
const fhome = join(fparent, '.gbrain');
|
||||
mkdirSync(fhome, { recursive: true });
|
||||
const fwsRoot = mkdtempSync(join(tmpdir(), 'gb-srcid-space-root-'));
|
||||
scratch.push(fwsRoot);
|
||||
const fws = join(fwsRoot, 'has space');
|
||||
mkdirSync(fws, { recursive: true });
|
||||
const prev = process.env.GBRAIN_HOME;
|
||||
process.env.GBRAIN_HOME = fparent;
|
||||
try {
|
||||
expect(initState(fws).ok).toBe(true);
|
||||
for (const [key, value] of Object.entries(REQUIRED_ANSWERS)) {
|
||||
const r = setAnswer(fws, key, value);
|
||||
if (!r.ok) throw new Error(r.message);
|
||||
}
|
||||
expect(setAnswer(fws, 'MCP_SCOPE', 'project').ok).toBe(true);
|
||||
const h = readBackHash(fws);
|
||||
if (!h.ok) throw new Error(h.message);
|
||||
expect(confirm(fws, h.hash).ok).toBe(true);
|
||||
} finally {
|
||||
if (prev === undefined) delete process.env.GBRAIN_HOME;
|
||||
else process.env.GBRAIN_HOME = prev;
|
||||
}
|
||||
const brainDir = join(fws, 'brain');
|
||||
const out = await withHome(fparent, async () => {
|
||||
expect((await capture(() => runBootstrap(['render', '--workspace', fws]))).result).toBe(0);
|
||||
const { runner } = makeRunner();
|
||||
return capture(() =>
|
||||
runBootstrap(['hooks', '--workspace', fws, '--harness', 'claude-code', '--gbrain-bin', process.execPath], {
|
||||
runner,
|
||||
}),
|
||||
);
|
||||
});
|
||||
expect(out.result).toBe(0);
|
||||
// A bare, unquoted path with a space would split into two shell words —
|
||||
// the printed command must single-quote it so copy/paste actually works.
|
||||
expect(out.out).toContain(`--path '${brainDir}' --force`);
|
||||
expect(out.out).not.toContain(`--path ${brainDir} --force`);
|
||||
}, 30_000);
|
||||
});
|
||||
|
||||
describe('per-turn hooks are ON by default (v0.45 flip); --no-hooks opts out', () => {
|
||||
// Self-contained fixtures: the file-level `ws` deliberately SKIPS
|
||||
// HOOKS_CONSENT (for the decline test), so the default-on path needs a
|
||||
|
||||
Reference in New Issue
Block a user