Files
gbrain/src/core/brain-writer.ts
T
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

395 lines
13 KiB
TypeScript

/**
* brain-writer — frontmatter validation/audit/auto-fix orchestrator.
*
* Thin layer on top of `parseMarkdown(..., {validate:true})` (the canonical
* source of frontmatter validation rules) and `isSyncable()` (the canonical
* brain-page filter). 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.
*
* Path-guard contract: writeBrainPage refuses to write outside the source
* path. .bak backups are the safety contract (works for both git and non-git
* brain repos; the existing src/core/dry-fix.ts:getWorkingTreeStatus rejects
* non-git repos as unsafe, which is the wrong shape for brain rewrites).
*/
import { existsSync, readFileSync, readdirSync, statSync, copyFileSync, writeFileSync, mkdirSync, lstatSync } from 'fs';
import { join, relative, resolve, dirname } from 'path';
import type { BrainEngine } from './engine.ts';
import type { ProgressReporter } from './progress.ts';
import {
parseMarkdown,
type ParseValidationCode,
type ParseValidationError,
} from './markdown.ts';
import { isSyncable, slugifyPath } from './sync.ts';
export type { ParseValidationCode };
export interface AuditFix {
code: ParseValidationCode;
description: string;
}
export interface PerSourceReport {
source_id: string;
source_path: string;
total: number;
errors_by_code: Partial<Record<ParseValidationCode, number>>;
sample: { path: string; codes: ParseValidationCode[] }[];
}
export interface AuditReport {
ok: boolean;
total: number;
errors_by_code: Partial<Record<ParseValidationCode, number>>;
per_source: PerSourceReport[];
scanned_at: string;
}
const SAMPLE_PER_SOURCE = 20;
// ---------------------------------------------------------------------------
// autoFixFrontmatter
// ---------------------------------------------------------------------------
/**
* Mechanical auto-repair for the fixable subset of validation codes:
* - NULL_BYTES — strip \x00 characters
* - NESTED_QUOTES — rewrite `"... "inner" ..."` to single-quoted outer
* - MISSING_CLOSE — insert `---` before the first heading found inside
* the YAML zone
* - SLUG_MISMATCH — remove `slug:` line (gbrain derives slug from path)
*
* Idempotent: running twice is a no-op on already-clean input. Any error class
* not in the list above is left untouched (e.g. EMPTY_FRONTMATTER, YAML_PARSE,
* MISSING_OPEN — those need human review).
*/
export function autoFixFrontmatter(
content: string,
opts?: { filePath?: string },
): { content: string; fixes: AuditFix[] } {
const fixes: AuditFix[] = [];
let working = content;
// 1. NULL_BYTES — strip them. Cheap, byte-level. Run first so subsequent
// line-based passes don't trip on stray nulls.
if (working.indexOf('\x00') >= 0) {
working = working.replace(/\x00/g, '');
fixes.push({ code: 'NULL_BYTES', description: 'Stripped null bytes' });
}
// 2. MISSING_CLOSE — if there's an opener but no closer before a heading,
// insert `---` immediately before the heading. Walk lines once.
{
const lines = working.split('\n');
let firstNonEmpty = -1;
for (let i = 0; i < lines.length; i++) {
if (lines[i].trim().length > 0) { firstNonEmpty = i; break; }
}
if (firstNonEmpty >= 0 && lines[firstNonEmpty].trim() === '---') {
let closeIdx = -1;
let headingIdx = -1;
for (let i = firstNonEmpty + 1; i < lines.length; i++) {
const t = lines[i].trim();
if (t === '---') { closeIdx = i; break; }
if (/^#{1,6}\s/.test(t)) { headingIdx = i; break; }
}
if (closeIdx === -1 && headingIdx >= 0) {
const fixed = [
...lines.slice(0, headingIdx),
'---',
'',
...lines.slice(headingIdx),
];
working = fixed.join('\n');
fixes.push({
code: 'MISSING_CLOSE',
description: `Inserted closing --- before heading at line ${headingIdx + 1}`,
});
}
}
}
// 3. NESTED_QUOTES — rewrite `key: "...inner..."` lines that have 3+ unescaped
// double-quotes by switching the outer wrapper to single quotes and
// leaving inner quotes alone.
{
const lines = working.split('\n');
let firstNonEmpty = -1;
for (let i = 0; i < lines.length; i++) {
if (lines[i].trim().length > 0) { firstNonEmpty = i; break; }
}
if (firstNonEmpty >= 0 && lines[firstNonEmpty].trim() === '---') {
let closeIdx = lines.length;
for (let i = firstNonEmpty + 1; i < lines.length; i++) {
if (lines[i].trim() === '---') { closeIdx = i; break; }
}
let fixedAny = false;
for (let i = firstNonEmpty + 1; i < closeIdx; i++) {
const m = lines[i].match(/^(\s*[A-Za-z_][\w-]*\s*:\s*)"(.*)"\s*(.*)$/);
if (!m) continue;
const [, prefix, inner, trailing] = m;
let count = 0;
for (let j = 0; j < inner.length; j++) {
if (inner[j] === '"' && (j === 0 || inner[j - 1] !== '\\')) count++;
}
// Total " on the line includes the two outer quotes the regex
// captured, plus whatever's in inner. We need 3+ to trigger.
if (count >= 1) {
// Inner already has unescaped " — outer wrap is causing the YAML
// parse failure. Rewrite to 'single-quoted'. YAML escapes `'` inside
// a single-quoted string by doubling it.
const escapedInner = inner.replace(/'/g, "''");
lines[i] = `${prefix}'${escapedInner}'${trailing ? ' ' + trailing : ''}`.replace(/\s+$/, '');
fixedAny = true;
}
}
if (fixedAny) {
working = lines.join('\n');
fixes.push({
code: 'NESTED_QUOTES',
description: 'Rewrote nested double-quoted YAML values to single-quoted',
});
}
}
}
// 4. SLUG_MISMATCH — remove `slug:` line if filePath is provided and the
// declared slug doesn't match the path-derived one. Per PR #392 spec,
// gbrain derives slug from path; the field shouldn't be in frontmatter.
if (opts?.filePath) {
const expectedSlug = slugifyPath(opts.filePath);
// Use the (possibly partially-fixed) working content to detect whether
// the slug field is present and mismatched.
const re = /^slug:\s*(.+?)\s*$/m;
const m = working.match(re);
if (m && m[1].replace(/^["']|["']$/g, '') !== expectedSlug) {
working = working.replace(re, '').replace(/\n{3,}/g, '\n\n');
fixes.push({
code: 'SLUG_MISMATCH',
description: `Removed mismatched slug field (was "${m[1]}", expected "${expectedSlug}")`,
});
}
}
return { content: working, fixes };
}
// ---------------------------------------------------------------------------
// writeBrainPage — path-guarded write with .bak backup
// ---------------------------------------------------------------------------
export class BrainWriterError extends Error {
code: string;
hint?: string;
constructor(code: string, message: string, hint?: string) {
super(message);
this.name = 'BrainWriterError';
this.code = code;
this.hint = hint;
}
}
/**
* Path-guarded brain page writer. Always writes `<filePath>.bak` before any
* in-place mutation (the contract that replaces git-tree-clean for non-git
* brain repos). Throws BrainWriterError if filePath is not under sourcePath.
*/
export function writeBrainPage(
filePath: string,
content: string,
opts: { sourcePath: string; autoFix?: boolean },
): { fixes: AuditFix[] } {
const resolvedSource = resolve(opts.sourcePath);
const resolvedTarget = resolve(filePath);
if (resolvedTarget !== resolvedSource && !resolvedTarget.startsWith(resolvedSource + '/')) {
throw new BrainWriterError(
'PATH_OUTSIDE_SOURCE',
`writeBrainPage: ${filePath} is not under ${opts.sourcePath}`,
'Pass --source <id> matching the source the file lives in.',
);
}
let toWrite = content;
let fixes: AuditFix[] = [];
if (opts.autoFix) {
const result = autoFixFrontmatter(content, { filePath });
toWrite = result.content;
fixes = result.fixes;
}
if (existsSync(filePath)) {
copyFileSync(filePath, filePath + '.bak');
} else {
mkdirSync(dirname(filePath), { recursive: true });
}
writeFileSync(filePath, toWrite, 'utf8');
return { fixes };
}
// ---------------------------------------------------------------------------
// scanBrainSources
// ---------------------------------------------------------------------------
interface SourceRow {
id: string;
local_path: string | null;
}
export interface ScanOpts {
/** Limit scan to one source. When omitted, all registered sources with a
* local_path are scanned. */
sourceId?: string;
onProgress?: ProgressReporter;
signal?: AbortSignal;
}
export async function scanBrainSources(
engine: BrainEngine,
opts: ScanOpts = {},
): Promise<AuditReport> {
const sources = await listSources(engine, opts.sourceId);
const totals: Partial<Record<ParseValidationCode, number>> = {};
const perSource: PerSourceReport[] = [];
let grandTotal = 0;
for (const src of sources) {
if (opts.signal?.aborted) break;
if (!src.local_path) continue;
if (!existsSync(src.local_path)) {
// Source registered but path is missing on disk; surface as a zero-row
// entry with a synthetic SCAN_PATH_MISSING note via warn-and-skip.
perSource.push({
source_id: src.id,
source_path: src.local_path,
total: 0,
errors_by_code: {},
sample: [],
});
continue;
}
const report = scanOneSource(src.id, src.local_path, opts);
perSource.push(report);
grandTotal += report.total;
for (const [code, n] of Object.entries(report.errors_by_code)) {
const k = code as ParseValidationCode;
totals[k] = (totals[k] ?? 0) + (n as number);
}
}
return {
ok: grandTotal === 0,
total: grandTotal,
errors_by_code: totals,
per_source: perSource,
scanned_at: new Date().toISOString(),
};
}
function scanOneSource(
sourceId: string,
sourcePath: string,
opts: ScanOpts,
): PerSourceReport {
const errorsByCode: Partial<Record<ParseValidationCode, number>> = {};
const sample: PerSourceReport['sample'] = [];
const rootResolved = resolve(sourcePath);
let scanned = 0;
let total = 0;
walkDir(rootResolved, (absPath) => {
if (opts.signal?.aborted) return false;
const relPath = relative(rootResolved, absPath);
if (!isSyncable(relPath, { strategy: 'markdown' })) return true;
scanned++;
let content: string;
try {
content = readFileSync(absPath, 'utf8');
} catch {
return true; // skip unreadable
}
const expectedSlug = slugifyPath(relPath);
const parsed = parseMarkdown(content, relPath, { validate: true, expectedSlug });
const errs = parsed.errors ?? [];
if (errs.length > 0) {
total += errs.length;
const codes: ParseValidationCode[] = [];
for (const e of errs) {
errorsByCode[e.code] = (errorsByCode[e.code] ?? 0) + 1;
codes.push(e.code);
}
if (sample.length < SAMPLE_PER_SOURCE) {
sample.push({ path: relPath, codes });
}
}
if (opts.onProgress && scanned % 50 === 0) {
opts.onProgress.tick(50);
}
return true;
});
if (opts.onProgress) {
opts.onProgress.heartbeat(`scanned ${scanned} pages in ${sourceId}`);
}
return {
source_id: sourceId,
source_path: sourcePath,
total,
errors_by_code: errorsByCode,
sample,
};
}
/** Recursive directory walker with symlink-loop protection (via lstat).
* Calls `visit` for each regular file. Returning false from `visit` stops
* the walk. Skips entries lstat reports as symlinks (sync's no-symlink
* policy). */
function walkDir(root: string, visit: (absPath: string) => boolean | void): void {
const stack: string[] = [root];
const visited = new Set<string>();
while (stack.length > 0) {
const dir = stack.pop()!;
let entries: string[];
try {
entries = readdirSync(dir);
} catch {
continue;
}
for (const name of entries) {
const full = join(dir, name);
let st: ReturnType<typeof lstatSync>;
try {
st = lstatSync(full);
} catch {
continue;
}
if (st.isSymbolicLink()) continue; // matches sync's no-symlink policy
if (st.isDirectory()) {
const real = resolve(full);
if (visited.has(real)) continue;
visited.add(real);
stack.push(full);
} else if (st.isFile()) {
const result = visit(full);
if (result === false) return;
}
}
}
}
async function listSources(engine: BrainEngine, sourceId?: string): Promise<SourceRow[]> {
if (sourceId) {
const rows = await engine.executeRaw<SourceRow>(
`SELECT id, local_path FROM sources WHERE id = $1`,
[sourceId],
);
return rows;
}
return engine.executeRaw<SourceRow>(
`SELECT id, local_path FROM sources WHERE local_path IS NOT NULL ORDER BY id`,
);
}