Files
gbrain/test/brain-writer.test.ts
Garry Tan 891c28b582 v0.22.4 feat: frontmatter-guard — 0 resolver warnings + validate/audit/install-hook CLI (#448)
* fix: resolve check-resolvable warnings on master

- skills/maintain/SKILL.md: drop "citation audit" trigger; the focused
  citation-fixer skill is the single owner. Silences the MECE overlap
  warning surfaced by src/core/check-resolvable.ts.
- skills/RESOLVER.md: add citation-audit disambiguation row pointing
  citation-fixer (focused fix) and chain-into maintain for broader audit.
  Broaden query triggers ("who is", "background on", "notes on") so
  the failing routing-eval fixtures resolve.
- skills/enrich/SKILL.md: replace inlined Citation Requirements block with
  backtick-wrapped `skills/conventions/quality.md` reference (the format
  extractDelegationTargets recognizes). Silences the dry_violation warning.
- skills/citation-fixer/routing-eval.jsonl: rewrite the two failing fixtures
  to embed "fix citations" verbatim so the substring matcher passes.
- skills/query/SKILL.md frontmatter: mirror the broadened RESOLVER.md
  triggers so the trigger round-trip test passes.

Result: gbrain check-resolvable reports 0 warnings, 0 errors against
the actual checked-in skills/ tree.

* feat: extend parseMarkdown + lint with frontmatter validation surface

Add an opt-in validation surface to parseMarkdown(): when called with
{ validate: true }, returns errors[] populated with seven canonical
ParseValidationError codes:

  MISSING_OPEN, MISSING_CLOSE, YAML_PARSE, SLUG_MISMATCH,
  NULL_BYTES, NESTED_QUOTES, EMPTY_FRONTMATTER

Existing callers are unaffected — validation is opt-in via the new
opts argument. The validation logic lives here as the single source of
truth for what counts as malformed brain-page frontmatter.

src/commands/lint.ts now consumes parseMarkdown(..., { validate: true })
and emits stable lint rule names (frontmatter-missing-close,
frontmatter-yaml-parse, frontmatter-null-bytes, frontmatter-nested-quotes,
frontmatter-slug-mismatch, frontmatter-empty). MISSING_OPEN is suppressed
to avoid double-reporting with the legacy no-frontmatter rule.

Tests: test/markdown-validation.test.ts (NEW, all 7 codes) +
test/lint-frontmatter.test.ts (NEW, lint integration + suppression).

* feat: add brain-writer.ts orchestrator (scan / autoFix / writeBrainPage)

Thin orchestrator (~280 lines) on top of parseMarkdown(..., {validate:true})
and isSyncable() (the canonical brain-page filter from src/core/sync.ts).
Three consumers call into this module: the gbrain frontmatter CLI, the
frontmatter_integrity doctor subcheck, and the v0.22.4 migration audit
phase. Single source of truth — no parallel validation stack.

Public API:
  - autoFixFrontmatter(content, opts?): { content, fixes }
    Mechanical auto-repair for the fixable subset (NULL_BYTES,
    MISSING_CLOSE, NESTED_QUOTES, SLUG_MISMATCH). Idempotent.
  - writeBrainPage(filePath, content, opts): path-guarded, .bak backup
    before any in-place mutation. Path guard refuses writes outside
    sourcePath. .bak is the safety contract for non-git brain repos.
  - scanBrainSources(engine, opts?): walks every registered source via
    direct SQL on sources.local_path, uses isSyncable() to filter,
    blocks symlinks (matches sync's no-symlink policy), respects
    AbortSignal.

The dirty-tree guard from src/core/dry-fix.ts:getWorkingTreeStatus() is
NOT used here — it rejects non-git repos as unsafe, but brain repos
aren't always git repos. .bak backups are the contract that works
universally.

Tests: test/brain-writer.test.ts (NEW, 16 cases) — autoFix idempotency,
path-guard reject, .bak backup, per-source rollup, AbortSignal mid-scan,
single-source filter, missing-source-path graceful skip, symlink no-loop.

* feat: gbrain frontmatter CLI (validate / audit / install-hook)

New top-level command surface for the frontmatter-guard feature:

  gbrain frontmatter validate <path> [--json] [--fix] [--dry-run]
    Validate one .md file or recursively scan a directory. --fix writes
    .bak then rewrites in place. No git-tree-clean guard — .bak is the
    safety contract (works for both git and non-git brain repos).

  gbrain frontmatter audit [--source <id>] [--json]
    Read-only scan via scanBrainSources(). Per-source rollup grouped by
    error code. --fix is intentionally NOT available here; use validate
    --fix on the source path to repair.

  gbrain frontmatter install-hook [--source <id>] [--force] [--uninstall]
    Drops a pre-commit hook in each source that's a git repo (skips
    non-git sources with a one-line note). Hook script gracefully
    degrades when gbrain is missing on PATH (prints a warning, exits 0).
    Refuses to clobber existing hooks without --force; writes <hook>.bak.
    --uninstall reverses cleanly.

src/cli.ts wires frontmatter through handleCliOnly so --help works
without a DB connection. The audit subcommand instantiates an engine
internally only when needed.

Tests: test/frontmatter-cli.test.ts (NEW, 9 cases) +
test/frontmatter-install-hook.test.ts (NEW, 6 cases) — --help no-DB,
clean/broken validate, --fix dry-run, --fix non-git, --json envelope,
recursive directory scan with isSyncable filter parity, hook install
+ overwrite-protection + --force + --uninstall + silent-refresh.

* feat: doctor frontmatter_integrity subcheck

Adds a frontmatter_integrity subcheck under gbrain doctor that calls
scanBrainSources() (the same shared scanner the CLI and migration use).
Reports per-source counts grouped by error code, with a fix hint
pointing at `gbrain frontmatter validate <path> --fix`. Wrapped in
a doctor progress phase with heartbeat so 50K-page brain scans stay
visible.

Tests: test/doctor.test.ts (UPDATE) — assertion that the subcheck
calls scanBrainSources and the fix hint references the correct CLI.

* feat: frontmatter-guard skill (registered in manifest + RESOLVER)

New skill at skills/frontmatter-guard/SKILL.md that wraps the gbrain
frontmatter CLI for agent-driven workflows. Agent-agnostic — no
references to private host libraries. Registered in skills/manifest.json
and skills/RESOLVER.md (the trigger row was added in the Part A commit).

Triggers: "validate frontmatter", "check frontmatter", "fix frontmatter",
"frontmatter audit", "brain lint".

Includes routing-eval fixtures that pass the substring matcher. The
SKILL.md has the conformance-required Output Format and Anti-Patterns
sections. Anti-patterns explicitly call out: don't auto-fix MISSING_OPEN
or EMPTY_FRONTMATTER without user input, don't skip .bak backups, don't
install the pre-commit hook on non-git brain dirs.

* feat: v0.22.4 migration orchestrator (audit-only, source-aware)

Adds the v0.22.4 migration that surveys every registered source for
frontmatter issues and queues per-source repair commands without ever
mutating brain content. Three idempotent phases:

  - schema: no-op (no DB changes in v0.22.4)
  - audit: scanBrainSources() across ALL registered sources; writes
    JSON report to ~/.gbrain/migrations/v0.22.4-audit.json
  - emit-todo: appends one entry per source-with-issues to
    ~/.gbrain/migrations/pending-host-work.jsonl, each with the exact
    `gbrain frontmatter validate <source-path> --fix` command

The agent reads skills/migrations/v0.22.4.md after upgrade, surfaces
the report counts to the user, and runs the fix command only with
explicit consent. `apply-migrations --yes` never silently rewrites
brain pages.

Filename convention: TS orchestrator at v0_22_4.ts (underscores, since
TS module paths can't have dots); user-facing migration doc at
skills/migrations/v0.22.4.md (dotted, matches existing convention).
The pending-host-work.jsonl skill field references the dotted-path doc.

Skips cleanly when no sources are registered (fresh install).

Tests: test/migrations-v0_22_4.test.ts (NEW, 9 cases) + updated
test/migration-orchestrator-v0_21_0.test.ts to allow v0.22.4 after,
test/apply-migrations.test.ts skippedFuture arrays extended to include
v0.22.4, test/check-resolvable.test.ts regression guard asserting the
actual checked-in skills/ tree has 0 warnings + 0 errors.

* docs: pre-commit recipe + downstream agent upgrade notes for v0.22.4

- docs/integrations/pre-commit.md (NEW): recipe doc covering install,
  bypass (`git commit --no-verify`), uninstall, and downstream-fork
  integration notes. Includes the full pipeline diagram showing how
  the hook (write-time gate), doctor (audit gate), and CLI (fix tool)
  share parseMarkdown(..., {validate:true}) as the single source of
  truth.
- docs/UPGRADING_DOWNSTREAM_AGENTS.md: append v0.22.4 section with the
  diff pattern for forks that had inline frontmatter validators. Covers
  the five upgrade actions: replace ad-hoc validators, drop
  lib/brain-writer.mjs references (it never shipped), wire the doctor
  subcheck into custom health pipelines, optionally install the
  pre-commit hook on git-backed brain repos, and walk
  pending-host-work.jsonl after apply-migrations.
- llms.txt + llms-full.txt: regenerated from build:llms script after
  the new docs landed.

* chore: bump version and changelog (v0.22.4)

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

* fix: handle null loadConfig() return in frontmatter + migration paths

CI typecheck caught three call sites that passed loadConfig()'s
GBrainConfig | null result straight into toEngineConfig() (which
expects GBrainConfig, not null):

  - src/commands/frontmatter.ts:64 (audit subcommand connect)
  - src/commands/frontmatter-install-hook.ts:86 (install-hook connect)
  - src/commands/migrations/v0_22_4.ts:59 (audit phase connect)

The frontmatter CLI and install-hook paths follow the existing
src/commands/repair-jsonb.ts pattern: throw 'No brain configured. Run:
gbrain init' so users get an actionable message instead of a TS-shaped
runtime crash.

The v0.22.4 migration audit phase takes a different shape: a fresh
install or test environment running apply-migrations shouldn't fail
hard just because there's no brain to scan yet. Return a clean
'skipped: no_brain_configured' phase result so the orchestrator
continues normally and the ledger records a complete (skipped) run.

* test: add v0.22.4 migration E2E + injection point for testability

Closes plan item B14 (the E2E that was promised but not delivered before
the original ship). Runs the v0_22_4 orchestrator end-to-end on PGLite
against a fixture brain with two registered sources and synthetic
malformed pages on disk. Asserts:

  - audit phase writes ~/.gbrain/migrations/v0.22.4-audit.json with
    per-source counts (NESTED_QUOTES + NULL_BYTES on alpha,
    NESTED_QUOTES on beta)
  - emit-todo phase appends one entry per source-with-issues to
    pending-host-work.jsonl, each pointing at skills/migrations/v0.22.4.md
    with the exact `gbrain frontmatter validate <source> --fix` command
  - the migration is audit-only — no fixture page is mutated
    during apply-migrations (no .bak created, contents byte-identical)
  - re-running the orchestrator is idempotent — JSONL stays at 2 lines

Adds a small test-injection point to v0_22_4.ts:
  __setTestEngineOverride(engine: BrainEngine | null): void

Mirrors src/commands/repair-jsonb.ts pattern. When set, phaseBAudit
uses the injected engine instead of loadConfig + createEngine. Production
path is unchanged: the override is null by default and the existing
loadConfig logic runs end-to-end. Required because Bun's os.homedir()
does not observe mid-process process.env.HOME mutations, so we can't
redirect loadConfig's config-file lookup via env-var overrides; the
injection point is the only hermetic way to E2E-test the orchestrator
without writing to the user's real ~/.gbrain/config.json.

Test runs unconditionally in CI's Tier 1 (no DATABASE_URL needed,
PGLite in-memory).

---------

Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>
2026-04-26 20:45:05 -07:00

220 lines
8.6 KiB
TypeScript

import { describe, expect, test, beforeEach, afterEach } from 'bun:test';
import { mkdtempSync, rmSync, writeFileSync, readFileSync, existsSync, mkdirSync, symlinkSync } from 'fs';
import { join } from 'path';
import { tmpdir } from 'os';
import {
autoFixFrontmatter,
writeBrainPage,
scanBrainSources,
BrainWriterError,
} from '../src/core/brain-writer.ts';
import { PGLiteEngine } from '../src/core/pglite-engine.ts';
const fence = '---';
describe('autoFixFrontmatter', () => {
test('strips null bytes', () => {
const input = `${fence}\ntitle: ok\n${fence}\n\nbody\x00drop\x00here`;
const { content, fixes } = autoFixFrontmatter(input);
expect(content.includes('\x00')).toBe(false);
expect(fixes.some(f => f.code === 'NULL_BYTES')).toBe(true);
});
test('inserts closing --- before heading when MISSING_CLOSE', () => {
const input = `${fence}\ntype: concept\ntitle: ok\n# A heading\n\nbody`;
const { content, fixes } = autoFixFrontmatter(input);
expect(fixes.some(f => f.code === 'MISSING_CLOSE')).toBe(true);
// After fix, parsing should find a closing --- before the heading.
const idxClose = content.indexOf('---', 3);
const idxHeading = content.indexOf('# A heading');
expect(idxClose).toBeGreaterThan(0);
expect(idxClose).toBeLessThan(idxHeading);
});
test('rewrites nested-quote title to single-quoted', () => {
const input = `${fence}\ntype: concept\ntitle: "Phil "Nick" Last"\n${fence}\n\nbody`;
const { content, fixes } = autoFixFrontmatter(input);
expect(fixes.some(f => f.code === 'NESTED_QUOTES')).toBe(true);
// Outer wrapper is now single quotes.
expect(content).toMatch(/^title: '.*'\s*$/m);
});
test('removes mismatched slug field', () => {
const input = `${fence}\ntype: concept\ntitle: hi\nslug: wrong-slug\n${fence}\n\nbody`;
const { content, fixes } = autoFixFrontmatter(input, { filePath: 'people/jane-doe.md' });
expect(fixes.some(f => f.code === 'SLUG_MISMATCH')).toBe(true);
expect(content).not.toMatch(/^slug:/m);
});
test('idempotent: running twice produces no diff and no fixes on second pass', () => {
const input = `${fence}\ntype: concept\ntitle: "Phil "Nick" Last"\n${fence}\n\nbody\x00`;
const first = autoFixFrontmatter(input);
const second = autoFixFrontmatter(first.content);
expect(second.content).toBe(first.content);
expect(second.fixes).toEqual([]);
});
test('clean input: no fixes, content unchanged', () => {
const input = `${fence}\ntype: concept\ntitle: ok\n${fence}\n\nbody`;
const { content, fixes } = autoFixFrontmatter(input);
expect(content).toBe(input);
expect(fixes).toEqual([]);
});
});
describe('writeBrainPage', () => {
let tmp: string;
beforeEach(() => {
tmp = mkdtempSync(join(tmpdir(), 'brain-writer-'));
});
afterEach(() => {
rmSync(tmp, { recursive: true, force: true });
});
test('happy path: writes file inside source', () => {
const file = join(tmp, 'people', 'jane.md');
const content = `${fence}\ntype: person\ntitle: Jane\n${fence}\n\nhello`;
writeBrainPage(file, content, { sourcePath: tmp });
expect(readFileSync(file, 'utf8')).toBe(content);
});
test('throws BrainWriterError when path is outside sourcePath', () => {
const elsewhere = mkdtempSync(join(tmpdir(), 'brain-writer-other-'));
try {
const offending = join(elsewhere, 'evil.md');
expect(() =>
writeBrainPage(offending, 'content', { sourcePath: tmp }),
).toThrow(BrainWriterError);
} finally {
rmSync(elsewhere, { recursive: true, force: true });
}
});
test('writes .bak before mutating an existing file', () => {
const file = join(tmp, 'people', 'jane.md');
mkdirSync(join(tmp, 'people'), { recursive: true });
const original = `${fence}\ntype: person\ntitle: Old\n${fence}\n\nold`;
writeFileSync(file, original);
writeBrainPage(file, `${fence}\ntype: person\ntitle: New\n${fence}\n\nnew`, { sourcePath: tmp });
expect(existsSync(file + '.bak')).toBe(true);
expect(readFileSync(file + '.bak', 'utf8')).toBe(original);
});
test('autoFix: true repairs nested quotes before writing', () => {
const file = join(tmp, 'people', 'jane.md');
const broken = `${fence}\ntype: person\ntitle: "Phil "Nick" Last"\n${fence}\n\nbody`;
const { fixes } = writeBrainPage(file, broken, { sourcePath: tmp, autoFix: true });
expect(fixes.some(f => f.code === 'NESTED_QUOTES')).toBe(true);
expect(readFileSync(file, 'utf8')).toMatch(/^title: '.*'\s*$/m);
});
});
describe('scanBrainSources (PGLite)', () => {
let tmp: string;
let engine: PGLiteEngine;
beforeEach(async () => {
tmp = mkdtempSync(join(tmpdir(), 'brain-writer-scan-'));
engine = new PGLiteEngine();
await engine.connect({});
await engine.initSchema();
});
afterEach(async () => {
await engine.disconnect();
rmSync(tmp, { recursive: true, force: true });
});
async function registerSource(id: string, path: string) {
await engine.executeRaw(
`INSERT INTO sources (id, name, local_path) VALUES ($1, $1, $2)
ON CONFLICT (id) DO UPDATE SET local_path = EXCLUDED.local_path`,
[id, path],
);
}
test('returns ok=true for empty source', async () => {
await registerSource('empty', tmp);
const report = await scanBrainSources(engine);
expect(report.ok).toBe(true);
expect(report.total).toBe(0);
const empty = report.per_source.find(s => s.source_id === 'empty');
expect(empty).toBeDefined();
expect(empty!.total).toBe(0);
});
test('detects errors across multiple sources', async () => {
const srcA = join(tmp, 'a');
const srcB = join(tmp, 'b');
mkdirSync(srcA, { recursive: true });
mkdirSync(srcB, { recursive: true });
writeFileSync(join(srcA, 'p1.md'), `${fence}\ntype: x\ntitle: ok\n${fence}\n\nbody\x00`);
writeFileSync(join(srcB, 'p2.md'), `${fence}\ntype: x\ntitle: "P "I" L"\n${fence}\n\nbody`);
await registerSource('alpha', srcA);
await registerSource('beta', srcB);
const report = await scanBrainSources(engine);
expect(report.ok).toBe(false);
expect(report.total).toBeGreaterThan(0);
const alpha = report.per_source.find(s => s.source_id === 'alpha')!;
const beta = report.per_source.find(s => s.source_id === 'beta')!;
expect(alpha.errors_by_code.NULL_BYTES).toBeGreaterThanOrEqual(1);
expect(beta.errors_by_code.NESTED_QUOTES).toBeGreaterThanOrEqual(1);
});
test('respects sourceId filter', async () => {
const srcA = join(tmp, 'a');
const srcB = join(tmp, 'b');
mkdirSync(srcA, { recursive: true });
mkdirSync(srcB, { recursive: true });
writeFileSync(join(srcA, 'bad.md'), `${fence}\ntype: x\ntitle: ok\n${fence}\n\nbody\x00`);
writeFileSync(join(srcB, 'bad.md'), `${fence}\ntype: x\ntitle: ok\n${fence}\n\nbody\x00`);
await registerSource('alpha', srcA);
await registerSource('beta', srcB);
const onlyA = await scanBrainSources(engine, { sourceId: 'alpha' });
expect(onlyA.per_source.length).toBe(1);
expect(onlyA.per_source[0]!.source_id).toBe('alpha');
});
test('skips registered source with missing path', async () => {
await registerSource('ghost', join(tmp, 'does-not-exist'));
const report = await scanBrainSources(engine);
const ghost = report.per_source.find(s => s.source_id === 'ghost')!;
expect(ghost.total).toBe(0);
});
test('skips symlinks (matches sync no-symlink policy)', async () => {
mkdirSync(join(tmp, 'real'), { recursive: true });
writeFileSync(join(tmp, 'real', 'good.md'), `${fence}\ntype: x\ntitle: ok\n${fence}\n\nbody`);
// Create a symlink loop: tmp/real/loop -> tmp/real
try {
symlinkSync(join(tmp, 'real'), join(tmp, 'real', 'loop'));
} catch {
// Some CI environments forbid symlink creation; skip the assertion.
return;
}
await registerSource('with-symlink', tmp);
const report = await scanBrainSources(engine);
// The walk should complete without infinite-looping; at most one .md
// entry visited (via the real path, not the symlink).
expect(report.per_source[0]!.total).toBe(0);
});
test('AbortSignal mid-scan stops walking', async () => {
const src = join(tmp, 'big');
mkdirSync(src, { recursive: true });
for (let i = 0; i < 50; i++) {
writeFileSync(join(src, `p${i}.md`), `${fence}\ntype: x\ntitle: t${i}\n${fence}\n\nbody`);
}
await registerSource('big', src);
const ctrl = new AbortController();
ctrl.abort();
const report = await scanBrainSources(engine, { signal: ctrl.signal });
// Aborted before any source ran; per_source array stays empty (or has zero reports).
expect(report.per_source.length).toBe(0);
});
});