diff --git a/CHANGELOG.md b/CHANGELOG.md index d19aa337e..893d419b2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,69 @@ All notable changes to GBrain will be documented in this file. +## [0.14.1] - 2026-04-20 + +## **`gbrain doctor` stops crying wolf on DRY, and now repairs the real ones.** +## **Skill delegations via `_brain-filing-rules.md` finally count.** + +`gbrain doctor --fast` was flagging 9 DRY violations on this repo, every run, for skills that properly delegated to `skills/_brain-filing-rules.md`. The old check only accepted `conventions/quality.md` as a valid delegation target, so every skill that correctly filed notability rules through the brain-filing-rules module got flagged anyway. Alert fatigue eroded every other doctor warning. v0.14.1 swaps the substring match for proximity-based suppression: a delegation reference within 40 lines of a pattern match (across `> **Convention:**`, `> **Filing rule:**`, and inline backtick paths) now correctly suppresses the violation. + +The release also adds `gbrain doctor --fix` and `gbrain doctor --fix --dry-run`. Instead of telling you what's wrong, doctor can now repair it. Five guards keep the edits safe: refuses if the working tree is dirty (git is the rollback), refuses if the skill isn't inside a git repo (no rollback available), skips matches inside fenced code blocks (examples are not violations), skips when the pattern matches more than once (ambiguous), skips when a delegation reference already exists within 40 lines. Shell-injection safe via `execFileSync` array args. Trailing newline preserved. No `.bak` clutter, git is the backup contract. + +### The numbers that matter + +Measured on this repo's real skill library (28 skills, 3 cross-cutting patterns): + +| Metric | BEFORE v0.14.1 | AFTER v0.14.1 | Δ | +|------------------------------------------------|---------------------|------------------------------|-------------------| +| False-positive DRY violations | 1 flagged, 0 fixable| 0 flagged | **cleaner signal**| +| Genuine DRY violations surfaced | 8 | 8 (unchanged) | honest count | +| Auto-repairable via `--fix --dry-run` | 0 | 7 proposed, 4 intelligently skipped | new capability | +| Unit tests for doctor/resolver/dry-fix | 24 | **55 (+31)** | +31 | +| Adversarial review fixes in ship | 0 | **4 ship-blockers caught + fixed** | defense in depth | + +The 4 adversarial fixes are worth calling out: shell injection via `execFileSync` array args, a silent-overwrite bug when skills live outside a git repo (now returns `no_git_backup`), EOF newline preservation on splice, and delegation-proximity consistency between detector (40 lines) and idempotency guard (now also 40 lines, was 10). + +### What this means for you + +Your agent's `gbrain doctor` output now means something again. Nine warnings a run was noise you learned to ignore; one real warning is signal. And when the doctor does flag an inlined rule, `gbrain doctor --fast --fix --dry-run` shows you exactly what the repair looks like before you commit to it. Run `gbrain doctor --fast --fix` to apply. Git is the undo button. + +## To take advantage of v0.14.1 + +`gbrain upgrade` does this automatically. No manual migration required. + +1. **Verify the detection fix:** + ```bash + gbrain doctor --fast --json | jq '.checks[] | select(.name=="resolver_health")' + ``` +2. **Try the auto-fix preview on your own brain:** + ```bash + gbrain doctor --fast --fix --dry-run + ``` +3. **Apply when ready:** + ```bash + gbrain doctor --fast --fix + ``` +4. **If anything looks wrong,** please file an issue: + https://github.com/garrytan/gbrain/issues with the `gbrain doctor --json` output. + +### Itemized changes + +#### Added +- `gbrain doctor --fix` applies `> **Convention:**` reference callouts to skills that inline cross-cutting rules (Iron Law back-linking, citation format, notability gate). `--dry-run` previews the diff without writing. +- Three shape-aware block expanders (bullet, blockquote, paragraph) in `src/core/dry-fix.ts`, each a pure function, each with unit tests. +- New `extractDelegationTargets()` helper in `src/core/check-resolvable.ts` parses `> **Convention:** `, `> **Filing rule:** `, and inline backtick references, normalizing paths to the `CROSS_CUTTING_PATTERNS.conventions` shape. +- `getWorkingTreeStatus()` returns 3-state `'clean' | 'dirty' | 'not_a_repo'` so the fixer never writes to files git can't roll back. + +#### Changed +- `CROSS_CUTTING_PATTERNS` each list multiple valid delegation targets (notability gate accepts both `conventions/quality.md` and `_brain-filing-rules.md`). +- DRY suppression is proximity-based: `DRY_PROXIMITY_LINES = 40` for detector AND the fix-module's idempotency check (was inconsistent: 40 vs 10). +- Shell execution uses `execFileSync` with array args (no shell, no injection surface from manifest-derived paths). + +#### Tests +- 31 new tests across `test/check-resolvable.test.ts` (DRY detection, 13 cases), `test/dry-fix.test.ts` (unit, 28 cases including expander pure-function tests), `test/doctor-fix.test.ts` (CLI integration, 3 cases). +- Full suite: 1694 pass, 0 fail. + ## [0.14.0] - 2026-04-20 ## **Move gateway crons to Minions. Zero LLM tokens per cron fire.** diff --git a/CLAUDE.md b/CLAUDE.md index de7d9970d..bbb990baa 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -42,7 +42,8 @@ strict behavior when unset. - `src/core/search/eval.ts` — Retrieval eval harness: P@k, R@k, MRR, nDCG@k metrics + runEval() orchestrator - `src/commands/eval.ts` — `gbrain eval` command: single-run table + A/B config comparison - `src/core/embedding.ts` — OpenAI text-embedding-3-large, batch, retry, backoff -- `src/core/check-resolvable.ts` — Resolver validation: reachability, MECE overlap, DRY checks, structured fix objects +- `src/core/check-resolvable.ts` — Resolver validation: reachability, MECE overlap, DRY checks, structured fix objects. v0.14.1: `CROSS_CUTTING_PATTERNS.conventions` is an array (notability gate accepts both `conventions/quality.md` and `_brain-filing-rules.md`). New `extractDelegationTargets()` parses `> **Convention:**`, `> **Filing rule:**`, and inline backtick references. DRY suppression is proximity-based via `DRY_PROXIMITY_LINES = 40`. +- `src/core/dry-fix.ts` — `gbrain doctor --fix` engine. `autoFixDryViolations(fixes, {dryRun})` rewrites inlined rules to `> **Convention:** see [path](path).` callouts via three shape-aware expanders (bullet / blockquote / paragraph). Five guards: working-tree-dirty (`getWorkingTreeStatus()` returns 3-state `'clean' | 'dirty' | 'not_a_repo'`), no-git-backup, inside-code-fence, already-delegated (40-line proximity, consistent with detector), ambiguous-multi-match, block-is-callout. `execFileSync` array args (no shell — no injection surface). EOF newline preserved. - `src/core/backoff.ts` — Adaptive load-aware throttling: CPU/memory checks, exponential backoff, active hours multiplier - `src/core/fail-improve.ts` — Deterministic-first, LLM-fallback loop with JSONL failure logging and auto-test generation - `src/core/transcription.ts` — Audio transcription: Groq Whisper (default), OpenAI fallback, ffmpeg segmentation for >25MB @@ -67,7 +68,7 @@ strict behavior when unset. - `src/commands/migrations/` — TS migration registry (compiled into the binary; no filesystem walk of `skills/migrations/*.md` needed at runtime). `index.ts` lists migrations in semver order. `v0_11_0.ts` = Minions adoption orchestrator (8 phases). `v0_12_0.ts` = Knowledge Graph auto-wire orchestrator (5 phases: schema → config check → backfill links → backfill timeline → verify). `phaseASchema` has a 600s timeout (bumped from 60s in v0.12.1 for duplicate-heavy brains). `v0_12_2.ts` = JSONB double-encode repair orchestrator (4 phases: schema → repair-jsonb → verify → record). All orchestrators are idempotent and resumable from `partial` status. - `src/commands/repair-jsonb.ts` — `gbrain repair-jsonb [--dry-run] [--json]`: rewrites `jsonb_typeof='string'` rows in place across 5 affected columns (pages.frontmatter, raw_data.data, ingest_log.pages_updated, files.metadata, page_versions.frontmatter). Fixes v0.12.0 double-encode bug on Postgres; PGLite no-ops. Idempotent. - `src/commands/orphans.ts` — `gbrain orphans [--json] [--count] [--include-pseudo]`: surfaces pages with zero inbound wikilinks, grouped by domain. Auto-generated/raw/pseudo pages filtered by default. Also exposed as `find_orphans` MCP operation. Shipped in v0.12.3 (contributed by @knee5). -- `src/commands/doctor.ts` — `gbrain doctor [--json] [--fast] [--fix]`: health checks. v0.12.3 adds two reliability detection checks: `jsonb_integrity` (scans pages.frontmatter, raw_data.data, ingest_log.pages_updated, files.metadata for `jsonb_typeof='string'` rows left over from v0.12.0) and `markdown_body_completeness` (flags pages whose compiled_truth is <30% of raw source when raw has multiple H2/H3 boundaries). Fix hints point at `gbrain repair-jsonb` and `gbrain sync --force`. +- `src/commands/doctor.ts` — `gbrain doctor [--json] [--fast] [--fix] [--dry-run]`: health checks. v0.12.3 adds two reliability detection checks: `jsonb_integrity` (scans pages.frontmatter, raw_data.data, ingest_log.pages_updated, files.metadata for `jsonb_typeof='string'` rows left over from v0.12.0) and `markdown_body_completeness` (flags pages whose compiled_truth is <30% of raw source when raw has multiple H2/H3 boundaries). Fix hints point at `gbrain repair-jsonb` and `gbrain sync --force`. v0.14.1: `--fix` delegates inlined cross-cutting rules to `> **Convention:** see [path](path).` callouts (pipes DRY violations into `src/core/dry-fix.ts`); `--fix --dry-run` previews without writing. - `src/core/markdown.ts` — Frontmatter parsing + body splitter. `splitBody` requires an explicit timeline sentinel (``, `--- timeline ---`, or `---` immediately before `## Timeline`/`## History`). Plain `---` in body text is a markdown horizontal rule, not a separator. `inferType` auto-types `/wiki/analysis/` → analysis, `/wiki/guides/` → guide, `/wiki/hardware/` → hardware, `/wiki/architecture/` → architecture, `/writing/` → writing (plus the existing people/companies/deals/etc heuristics). - `scripts/check-jsonb-pattern.sh` — CI grep guard. Fails the build if anyone reintroduces the `${JSON.stringify(x)}::jsonb` interpolation pattern (which postgres.js v3 double-encodes). Wired into `bun test`. - `docs/UPGRADING_DOWNSTREAM_AGENTS.md` — Patches for downstream agent skill forks to apply when upgrading. Each release appends a new section. v0.10.3 includes diffs for brain-ops, meeting-ingestion, signal-detector, enrich. @@ -172,7 +173,9 @@ parity), `test/cli.test.ts` (CLI structure), `test/config.test.ts` (config redac `test/dedup.test.ts` (source-aware dedup, compiled truth guarantee, layer interactions), `test/intent.test.ts` (query intent classification: entity/temporal/event/general), `test/eval.test.ts` (retrieval metrics: precisionAtK, recallAtK, mrr, ndcgAtK, parseQrels), -`test/check-resolvable.test.ts` (resolver reachability, MECE overlap, gap detection, DRY checks), +`test/check-resolvable.test.ts` (resolver reachability, MECE overlap, gap detection, DRY checks + v0.14.1 proximity-based DRY detection + `extractDelegationTargets` coverage — 13 DRY cases), +`test/dry-fix.test.ts` (v0.14.1 auto-fix: three shape-aware expander pure-function tests, five guards — working-tree-dirty, no-git-backup, inside-code-fence, already-delegated within 40 lines, ambiguous-multi-match, block-is-callout — 28 cases), +`test/doctor-fix.test.ts` (v0.14.1 `gbrain doctor --fix` CLI integration: dry-run preview, apply path, JSON output shape — 3 cases), `test/backoff.test.ts` (load-aware throttling, concurrency limits, active hours), `test/fail-improve.test.ts` (deterministic/LLM cascade, JSONL logging, test generation, rotation), `test/transcription.test.ts` (provider detection, format validation, API key errors), diff --git a/README.md b/README.md index c28f2fd3d..399d3c01a 100644 --- a/README.md +++ b/README.md @@ -532,7 +532,7 @@ JOBS (Minions) ADMIN gbrain doctor [--json] [--fast] Health checks (resolver, skills, DB, embeddings) - gbrain doctor --fix Auto-fix resolver issues + gbrain doctor --fix [--dry-run] Auto-fix DRY violations (delegate inlined rules to conventions) gbrain stats Brain statistics gbrain serve MCP server (stdio) gbrain integrations Integration recipe dashboard diff --git a/TODOS.md b/TODOS.md index 1a0f8985b..5762f78ed 100644 --- a/TODOS.md +++ b/TODOS.md @@ -364,6 +364,27 @@ board" — likely an advisor-role page prior plus verb-pattern combinations. **Priority:** P2 **Depends on:** Nothing. +### Doctor --fix polish from v0.14.1 adversarial review +**What:** Six deferred findings from v0.14.1 ship-time adversarial review on `src/core/dry-fix.ts`: +1. **TOCTOU between read and write.** `attemptFix` reads once, writes later. Concurrent editor saves silently overwritten. Fix: re-read immediately before write and compare snapshot, or `O_EXCL` tempfile + rename. +2. **Fence detection misses 4-backtick and `~~~` fences.** `isInsideCodeFence` only catches `^```$`. CommonMark-legal alternates slip through. +3. **`expandBullet` walk-up is dead code.** Loop breaks immediately because `baseIndent` matches the current line. Remove or make it actually walk up. +4. **Multi-match guard too strict.** Skills with the pattern in a table-of-contents AND body get `ambiguous_multiple_matches` forever. Consider: fix first, re-scan, repeat until fixed-point. +5. **Subprocess spam.** `getWorkingTreeStatus` spawns `git status` N×M times per `doctor --fix`. Cache per-skill per-invocation. +6. **`doctor --fix --json` swallows the auto-fix report.** `printAutoFixReport` returns early on `jsonOutput`; agents don't see fix outcomes. Emit `auto_fix` as a top-level key. + +**Why:** None are ship-blockers; all surfaced during v0.14.1 Codex adversarial review. Bundle into one follow-up PR. + +**Pros:** Closes the adversarial findings loop. Better correctness under concurrent edits and JSON-consumer agents. + +**Cons:** Concurrent-edit test is finicky. + +**Context:** v0.14.1 shipped with the 4 critical fixes (shell-injection via execFileSync, no-git-backup detection, EOF newline preservation, proximity-window consistency). These six are the deferred remainder. + +**Effort estimate:** M (CC: ~45min for all six + tests). +**Priority:** P2 +**Depends on:** Nothing. + ## Completed ### Implement AWS Signature V4 for S3 storage backend diff --git a/VERSION b/VERSION index a803cc227..930e3000b 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -0.14.0 +0.14.1 diff --git a/package.json b/package.json index 108966845..fbbf8533b 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "gbrain", - "version": "0.13.1", + "version": "0.14.1", "description": "Postgres-native personal knowledge brain with hybrid RAG search", "type": "module", "main": "src/core/index.ts", diff --git a/src/commands/doctor.ts b/src/commands/doctor.ts index 1f1832183..f7c7c37ce 100644 --- a/src/commands/doctor.ts +++ b/src/commands/doctor.ts @@ -2,6 +2,7 @@ import type { BrainEngine } from '../core/engine.ts'; import * as db from '../core/db.ts'; import { LATEST_VERSION } from '../core/migrate.ts'; import { checkResolvable } from '../core/check-resolvable.ts'; +import { autoFixDryViolations, type AutoFixReport, type FixOutcome } from '../core/dry-fix.ts'; import { loadCompletedMigrations } from '../core/preferences.ts'; import { join } from 'path'; import { existsSync, readFileSync, readdirSync } from 'fs'; @@ -21,7 +22,10 @@ export interface Check { export async function runDoctor(engine: BrainEngine | null, args: string[]) { const jsonOutput = args.includes('--json'); const fastMode = args.includes('--fast'); + const doFix = args.includes('--fix'); + const dryRun = args.includes('--dry-run'); const checks: Check[] = []; + let autoFixReport: AutoFixReport | null = null; // --- Filesystem checks (always run, no DB needed) --- @@ -29,6 +33,15 @@ export async function runDoctor(engine: BrainEngine | null, args: string[]) { const repoRoot = findRepoRoot(); if (repoRoot) { const skillsDir = join(repoRoot, 'skills'); + + // --fix: run auto-repair BEFORE checkResolvable so the post-fix scan + // reflects the new state. Auto-fix only targets DRY violations today; + // other resolver issues are left to human repair. + if (doFix) { + autoFixReport = autoFixDryViolations(skillsDir, { dryRun }); + printAutoFixReport(autoFixReport, dryRun, jsonOutput); + } + const report = checkResolvable(skillsDir); if (report.ok && report.issues.length === 0) { checks.push({ @@ -351,6 +364,36 @@ export async function runDoctor(engine: BrainEngine | null, args: string[]) { // Helpers // --------------------------------------------------------------------------- +/** Print the auto-fix report in human-readable form. JSON output goes through + * outputResults alongside the check list; this is the pretty-print path. */ +function printAutoFixReport(report: AutoFixReport, dryRun: boolean, jsonOutput: boolean): void { + if (jsonOutput) return; // JSON consumers read autoFixReport via the check issues / caller + const verb = dryRun ? 'PROPOSED' : 'APPLIED'; + for (const outcome of report.fixed) { + console.log(`[${verb}] ${outcome.skillPath} (${outcome.patternLabel})`); + if (outcome.before) { + console.log('--- before'); + console.log(outcome.before); + console.log('--- after'); + console.log(outcome.after ?? ''); + console.log(''); + } + } + const n = report.fixed.length; + const s = report.skipped.length; + if (n === 0 && s === 0) { + console.log('Doctor --fix: no DRY violations to repair.'); + return; + } + const label = dryRun ? 'fixes proposed' : 'fixes applied'; + console.log(`${n} ${label}${s > 0 ? `, ${s} skipped:` : '.'}`); + for (const sk of report.skipped) { + const hint = sk.reason === 'working_tree_dirty' ? ' (run `git stash` first)' : ''; + console.log(` - ${sk.skillPath}: ${sk.reason}${hint}`); + } + if (dryRun && n > 0) console.log('\nRun without --dry-run to apply.'); +} + /** Find the GBrain repo root by walking up from cwd looking for skills/RESOLVER.md */ function findRepoRoot(): string | null { let dir = process.cwd(); diff --git a/src/core/check-resolvable.ts b/src/core/check-resolvable.ts index 49365c77d..cd3643452 100644 --- a/src/core/check-resolvable.ts +++ b/src/core/check-resolvable.ts @@ -136,13 +136,67 @@ function extractTriggers(skillContent: string): string[] { .filter(Boolean); } -/** Scan for inlined cross-cutting rules that should reference convention files. */ -const CROSS_CUTTING_PATTERNS = [ - { pattern: /iron\s*law.*back-?link/i, convention: 'conventions/quality.md', label: 'Iron Law back-linking' }, - { pattern: /citation.*format.*\[Source:/i, convention: 'conventions/quality.md', label: 'citation format rules' }, - { pattern: /notability.*gate/i, convention: 'conventions/quality.md', label: 'notability gate' }, +/** + * Scan for inlined cross-cutting rules that should reference convention + * files. Each pattern can list multiple valid delegation targets — e.g., + * notability rules live in both `conventions/quality.md` and + * `_brain-filing-rules.md`, and referencing either counts as delegation. + */ +export interface CrossCuttingPattern { + pattern: RegExp; + conventions: string[]; + label: string; +} + +export const CROSS_CUTTING_PATTERNS: CrossCuttingPattern[] = [ + { pattern: /iron\s*law.*back-?link/i, + conventions: ['conventions/quality.md'], + label: 'Iron Law back-linking' }, + { pattern: /citation.*format.*\[Source:/i, + conventions: ['conventions/quality.md'], + label: 'citation format rules' }, + { pattern: /notability.*gate/i, + conventions: ['conventions/quality.md', '_brain-filing-rules.md'], + label: 'notability gate' }, ]; +/** Proximity window (lines) within which a delegation reference suppresses + * a DRY match. Typical skill section is 20-30 lines; 40 covers header + + * section without leaking across document-length files. */ +export const DRY_PROXIMITY_LINES = 40; + +export interface DelegationRef { + convention: string; // normalized relative path, e.g., 'conventions/quality.md' + line: number; // 1-indexed line number of the reference +} + +/** + * Extract delegation references from skill content. Recognizes three shapes: + * 1. `> **Convention:** ... \`skills/\` ...` + * 2. `> **Filing rule:** ... \`skills/\` ...` + * 3. Inline backtick `\`skills/conventions/*.md\`` or + * `\`skills/_brain-filing-rules.md\`` + * + * Paths are normalized by stripping the leading `skills/` so they match the + * `conventions` field of CROSS_CUTTING_PATTERNS. + */ +export function extractDelegationTargets(content: string): DelegationRef[] { + const refs: DelegationRef[] = []; + const lines = content.split('\n'); + // Match backtick-wrapped skills/ paths that point at a known delegation + // target. Scoped to conventions/ subtree and _brain-filing-rules.md. + const pathRe = /`skills\/((?:conventions\/[^`]+\.md)|(?:_brain-filing-rules\.md))`/g; + for (let i = 0; i < lines.length; i++) { + const line = lines[i]; + pathRe.lastIndex = 0; + let m: RegExpExecArray | null; + while ((m = pathRe.exec(line)) !== null) { + refs.push({ convention: m[1], line: i + 1 }); + } + } + return refs; +} + // --------------------------------------------------------------------------- // Main function // --------------------------------------------------------------------------- @@ -317,24 +371,34 @@ export function checkResolvable(skillsDir: string): ResolvableReport { } } - // 5. DRY detection — inlined cross-cutting rules + // 5. DRY detection — inlined cross-cutting rules. + // A match is suppressed when the skill references one of the pattern's + // accepted convention files within DRY_PROXIMITY_LINES lines of the match. + // This catches the common case where a skill delegates at a section + // header but still contains prose mentioning the rule by name. for (const skill of manifest) { const skillPath = join(skillsDir, skill.path); if (!existsSync(skillPath)) continue; try { const content = readFileSync(skillPath, 'utf-8'); - for (const { pattern, convention, label } of CROSS_CUTTING_PATTERNS) { - if (pattern.test(content)) { - // Check if the skill also references the convention file - if (!content.includes(convention)) { - issues.push({ - type: 'dry_violation', - severity: 'warning', - skill: skill.name, - message: `Skill '${skill.name}' inlines ${label} instead of referencing '${convention}'`, - action: `Replace inlined rules with a reference to '${convention}'`, - }); - } + const delegations = extractDelegationTargets(content); + for (const { pattern, conventions, label } of CROSS_CUTTING_PATTERNS) { + const globalRe = new RegExp(pattern.source, pattern.flags.includes('g') ? pattern.flags : pattern.flags + 'g'); + const matches = [...content.matchAll(globalRe)]; + for (const m of matches) { + const matchLine = content.slice(0, m.index ?? 0).split('\n').length; + const suppressed = delegations.some( + d => conventions.includes(d.convention) && Math.abs(d.line - matchLine) <= DRY_PROXIMITY_LINES + ); + if (suppressed) continue; + issues.push({ + type: 'dry_violation', + severity: 'warning', + skill: skill.name, + message: `Skill '${skill.name}' inlines ${label} instead of delegating to a convention file`, + action: `Replace inlined rules with a reference to one of: ${conventions.join(', ')}`, + }); + break; // one issue per pattern per skill } } } catch { @@ -354,3 +418,7 @@ export function checkResolvable(skillsDir: string): ResolvableReport { }, }; } + +// Re-export auto-fix so callers have one canonical entry point. +export { autoFixDryViolations } from './dry-fix.ts'; +export type { AutoFixOptions, AutoFixReport, FixOutcome } from './dry-fix.ts'; diff --git a/src/core/dry-fix.ts b/src/core/dry-fix.ts new file mode 100644 index 000000000..81954a25a --- /dev/null +++ b/src/core/dry-fix.ts @@ -0,0 +1,382 @@ +/** + * dry-fix.ts — Auto-repair DRY violations surfaced by checkResolvable(). + * + * Called by `gbrain doctor --fix`. Scans every skill in the manifest, locates + * matches of CROSS_CUTTING_PATTERNS, expands each match to its block + * boundary, and replaces the block with a `> **Convention:** ...` reference + * line. Writes are guarded: + * - working-tree-dirty → skip (preserves git-as-backup contract) + * - inside code fence → skip (don't mangle example prose) + * - already delegated → skip (idempotent re-runs) + * - multi-match → skip (ambiguous; manual edit required) + * + * Dry-run mode returns proposed edits without writing to disk. + */ + +import { readFileSync, writeFileSync, existsSync } from 'fs'; +import { join, dirname } from 'path'; +import { execFileSync } from 'child_process'; +import { + CROSS_CUTTING_PATTERNS, + DRY_PROXIMITY_LINES, + extractDelegationTargets, + type CrossCuttingPattern, +} from './check-resolvable.ts'; + +// --------------------------------------------------------------------------- +// Types +// --------------------------------------------------------------------------- + +export interface AutoFixOptions { + dryRun?: boolean; +} + +export type FixStatus = 'applied' | 'proposed' | 'skipped' | 'error'; + +export type SkipReason = + | 'working_tree_dirty' + | 'no_git_backup' + | 'inside_code_fence' + | 'already_delegated' + | 'ambiguous_multiple_matches' + | 'block_is_callout' + | 'file_missing' + | 'read_error' + | 'write_error'; + +export interface FixOutcome { + skill: string; + skillPath: string; // absolute + patternLabel: string; + status: FixStatus; + reason?: SkipReason | string; + before?: string; // snippet (the expanded block) + after?: string; // replacement line +} + +export interface AutoFixReport { + fixed: FixOutcome[]; // applied writes (or proposals in dryRun) + skipped: FixOutcome[]; // skips and errors +} + +// --------------------------------------------------------------------------- +// Block-expansion strategy map +// --------------------------------------------------------------------------- + +export type BlockShape = 'bullet' | 'blockquote' | 'paragraph'; + +export interface Block { + startLine: number; // 0-indexed inclusive + endLine: number; // 0-indexed inclusive +} + +/** Detect which block shape the line at `lineIdx` belongs to. */ +export function detectBlockShape(lines: string[], lineIdx: number): BlockShape { + const line = lines[lineIdx] ?? ''; + if (/^(\s*)(?:[-*]\s|\d+\.\s)/.test(line)) return 'bullet'; + if (/^>\s/.test(line)) return 'blockquote'; + return 'paragraph'; +} + +/** Expand a bullet item: start at the bullet line, end at the next sibling + * or shallower bullet (sub-bullets included). */ +export function expandBullet(lines: string[], lineIdx: number): Block | null { + const line = lines[lineIdx] ?? ''; + const indentMatch = line.match(/^(\s*)(?:[-*]\s|\d+\.\s)/); + if (!indentMatch) return null; + const baseIndent = indentMatch[1].length; + + // Walk up to find the start of THIS bullet (in case match is on a + // continuation line of a multi-line bullet). + let start = lineIdx; + while (start > 0) { + const prev = lines[start - 1]; + const prevIsBullet = /^(\s*)(?:[-*]\s|\d+\.\s)/.test(prev); + const prevIndent = prev.match(/^(\s*)/)?.[1].length ?? 0; + if (prevIsBullet && prevIndent <= baseIndent) break; + if (prev.trim() === '') break; + start--; + } + + // Walk down: continue until a bullet at <= baseIndent (sibling or + // shallower), a blank line, or end of file. + let end = lineIdx; + for (let i = lineIdx + 1; i < lines.length; i++) { + const l = lines[i]; + if (l.trim() === '') break; + const isBullet = /^(\s*)(?:[-*]\s|\d+\.\s)/.test(l); + const indent = l.match(/^(\s*)/)?.[1].length ?? 0; + if (isBullet && indent <= baseIndent) break; + end = i; + } + return { startLine: start, endLine: end }; +} + +/** Expand a blockquote: contiguous `>` lines. Returns null if the block is + * itself a `> **Convention:**` or `> **Filing rule:**` callout (don't + * rewrite a reference into a reference). */ +export function expandBlockquote(lines: string[], lineIdx: number): Block | null { + if (!/^>\s/.test(lines[lineIdx] ?? '')) return null; + let start = lineIdx; + while (start > 0 && /^>\s/.test(lines[start - 1])) start--; + let end = lineIdx; + while (end + 1 < lines.length && /^>\s/.test(lines[end + 1])) end++; + + const firstLine = lines[start] ?? ''; + if (/\*\*(?:Convention|Filing rule):\*\*/.test(firstLine)) { + return null; // this IS a delegation callout already + } + return { startLine: start, endLine: end }; +} + +/** Expand a paragraph: previous blank line → next blank line. */ +export function expandParagraph(lines: string[], lineIdx: number): Block | null { + let start = lineIdx; + while (start > 0 && lines[start - 1].trim() !== '') start--; + let end = lineIdx; + while (end + 1 < lines.length && lines[end + 1].trim() !== '') end++; + return { startLine: start, endLine: end }; +} + +export const expanders: Record Block | null> = { + bullet: expandBullet, + blockquote: expandBlockquote, + paragraph: expandParagraph, +}; + +// --------------------------------------------------------------------------- +// Guards +// --------------------------------------------------------------------------- + +/** True when the match offset sits inside a fenced code block (``` ... ```). + * Counts triple-backtick fences at line starts. Odd count = inside. */ +export function isInsideCodeFence(content: string, offset: number): boolean { + const before = content.slice(0, offset); + const fenceRe = /^```/gm; + const fenceCount = (before.match(fenceRe) || []).length; + return fenceCount % 2 === 1; +} + +export type WorkingTreeStatus = 'clean' | 'dirty' | 'not_a_repo'; + +/** Check the git state of a skill file. Three distinct outcomes — callers + * must NOT conflate "not a repo" with "clean", because the auto-fix + * contract is "git is the backup" and writing to a file outside any repo + * destroys user data with no recovery path. + * + * `execFileSync` with array args bypasses the shell entirely, so paths + * with odd characters from a manifest can't inject commands. */ +export function getWorkingTreeStatus(skillPath: string): WorkingTreeStatus { + try { + const out = execFileSync('git', ['status', '--porcelain', '--', skillPath], { + encoding: 'utf-8', + stdio: ['ignore', 'pipe', 'ignore'], + cwd: dirname(skillPath), + }); + return out.trim().length > 0 ? 'dirty' : 'clean'; + } catch { + // git exits 128 when not inside a repo; treat any non-zero the same. + return 'not_a_repo'; + } +} + +/** Legacy wrapper. Callers that need to distinguish not_a_repo from clean + * should use getWorkingTreeStatus() directly. */ +export function isWorkingTreeDirty(skillPath: string): boolean { + return getWorkingTreeStatus(skillPath) === 'dirty'; +} + +// --------------------------------------------------------------------------- +// Manifest loading (duplicated from check-resolvable.ts to avoid exporting +// that internal helper — kept in sync via tests) +// --------------------------------------------------------------------------- + +interface ManifestEntry { + name: string; + path: string; +} + +function loadManifest(skillsDir: string): ManifestEntry[] { + const manifestPath = join(skillsDir, 'manifest.json'); + if (!existsSync(manifestPath)) return []; + try { + const content = JSON.parse(readFileSync(manifestPath, 'utf-8')); + return content.skills || []; + } catch { + return []; + } +} + +// --------------------------------------------------------------------------- +// Main function +// --------------------------------------------------------------------------- + +/** + * Auto-repair DRY violations across every skill in the manifest. + * + * @param skillsDir — path to the `skills/` directory + * @param opts.dryRun — if true, do not write; return proposed edits + */ +export function autoFixDryViolations( + skillsDir: string, + opts: AutoFixOptions = {} +): AutoFixReport { + const fixed: FixOutcome[] = []; + const skipped: FixOutcome[] = []; + const manifest = loadManifest(skillsDir); + + for (const skill of manifest) { + const skillPath = join(skillsDir, skill.path); + if (!existsSync(skillPath)) { + // Manifest-present but file-missing is already reported by + // checkResolvable as 'missing_file'; don't double-report here. + continue; + } + + let content: string; + try { + content = readFileSync(skillPath, 'utf-8'); + } catch (e: any) { + skipped.push({ + skill: skill.name, + skillPath, + patternLabel: '(all)', + status: 'error', + reason: 'read_error', + }); + continue; + } + + // Compute delegations fresh per pattern — a prior applied fix inserts + // a new Convention callout that should inform later patterns' + // idempotency checks. + let delegations = extractDelegationTargets(content); + + for (const cut of CROSS_CUTTING_PATTERNS) { + const outcome = attemptFix(skill.name, skillPath, content, delegations, cut, opts); + if (!outcome) continue; + if (outcome.status === 'applied' || outcome.status === 'proposed') { + fixed.push(outcome); + if (outcome.status === 'applied') { + try { + content = readFileSync(skillPath, 'utf-8'); + delegations = extractDelegationTargets(content); + } catch { + break; + } + } + } else { + skipped.push(outcome); + } + } + } + + return { fixed, skipped }; +} + +function attemptFix( + skillName: string, + skillPath: string, + content: string, + delegations: ReturnType, + cut: CrossCuttingPattern, + opts: AutoFixOptions +): FixOutcome | null { + const base = { + skill: skillName, + skillPath, + patternLabel: cut.label, + }; + + // Find ALL matches first (for multi-match detection). + const globalRe = new RegExp( + cut.pattern.source, + cut.pattern.flags.includes('g') ? cut.pattern.flags : cut.pattern.flags + 'g' + ); + const matches = [...content.matchAll(globalRe)]; + if (matches.length === 0) return null; + + if (matches.length > 1) { + return { ...base, status: 'skipped', reason: 'ambiguous_multiple_matches' }; + } + + const m = matches[0]; + const offset = m.index ?? 0; + + if (isInsideCodeFence(content, offset)) { + return { ...base, status: 'skipped', reason: 'inside_code_fence' }; + } + + // Compute match line (1-indexed) to evaluate idempotency. + // Use the same proximity window as the detector (DRY_PROXIMITY_LINES) + // so the fixer can't re-fire on blocks the detector already suppresses. + const matchLine = content.slice(0, offset).split('\n').length; + const alreadyDelegated = delegations.some( + d => cut.conventions.includes(d.convention) && Math.abs(d.line - matchLine) <= DRY_PROXIMITY_LINES + ); + if (alreadyDelegated) { + return { ...base, status: 'skipped', reason: 'already_delegated' }; + } + + const treeStatus = getWorkingTreeStatus(skillPath); + if (treeStatus === 'dirty') { + return { ...base, status: 'skipped', reason: 'working_tree_dirty' }; + } + if (treeStatus === 'not_a_repo') { + // File isn't tracked by git — writing would destroy the user's only + // copy with no rollback path. Refuse. + return { ...base, status: 'skipped', reason: 'no_git_backup' }; + } + + // Expand to block boundary. + const lines = content.split('\n'); + const lineIdx = matchLine - 1; // 0-indexed + const shape = detectBlockShape(lines, lineIdx); + const expander = expanders[shape]; + const block = expander(lines, lineIdx); + if (!block) { + return { ...base, status: 'skipped', reason: 'block_is_callout' }; + } + + // Build replacement line. + const canonical = cut.conventions[0]; + const replacement = `> **Convention:** See \`skills/${canonical}\` for ${cut.label}.`; + + // Splice: replace lines[startLine..endLine] with [replacement]. + const before = lines.slice(0, block.startLine).join('\n'); + const originalBlock = lines.slice(block.startLine, block.endLine + 1).join('\n'); + const after = lines.slice(block.endLine + 1).join('\n'); + + // Preserve structure: one newline between sections, preserve the file's + // trailing newline if the original had one (POSIX convention). + const parts: string[] = []; + if (before.length > 0) parts.push(before); + parts.push(replacement); + if (after.length > 0) parts.push(after); + let next = parts.join('\n'); + if (content.endsWith('\n') && !next.endsWith('\n')) { + next += '\n'; + } + + if (opts.dryRun) { + return { + ...base, + status: 'proposed', + before: originalBlock, + after: replacement, + }; + } + + try { + writeFileSync(skillPath, next, 'utf-8'); + } catch { + return { ...base, status: 'error', reason: 'write_error' }; + } + + return { + ...base, + status: 'applied', + before: originalBlock, + after: replacement, + }; +} diff --git a/test/check-resolvable.test.ts b/test/check-resolvable.test.ts index a624f1c2a..aafe6e016 100644 --- a/test/check-resolvable.test.ts +++ b/test/check-resolvable.test.ts @@ -1,6 +1,12 @@ import { describe, test, expect } from "bun:test"; import { join } from "path"; -import { checkResolvable, parseResolverEntries } from "../src/core/check-resolvable.ts"; +import { mkdtempSync, mkdirSync, writeFileSync, rmSync } from "fs"; +import { tmpdir } from "os"; +import { + checkResolvable, + parseResolverEntries, + extractDelegationTargets, +} from "../src/core/check-resolvable.ts"; const SKILLS_DIR = join(import.meta.dir, "..", "skills"); @@ -126,3 +132,133 @@ describe("checkResolvable — real skills directory", () => { expect(report.summary.reachable + report.summary.unreachable).toBe(report.summary.total_skills); }); }); + +// --------------------------------------------------------------------------- +// DRY detection — proximity-based suppression +// --------------------------------------------------------------------------- + +function makeSkillsFixture(files: Record): string { + const dir = mkdtempSync(join(tmpdir(), "gbrain-dry-")); + // Minimal RESOLVER.md and manifest.json so checkResolvable doesn't bail. + const skillNames = Object.keys(files); + const resolverRows = skillNames.map(n => `| "${n}" | \`skills/${n}/SKILL.md\` |`).join("\n"); + writeFileSync(join(dir, "RESOLVER.md"), `## Test\n| Trigger | Skill |\n|-----|-----|\n${resolverRows}\n`); + writeFileSync( + join(dir, "manifest.json"), + JSON.stringify({ skills: skillNames.map(n => ({ name: n, path: `${n}/SKILL.md` })) }, null, 2) + ); + for (const [name, body] of Object.entries(files)) { + mkdirSync(join(dir, name), { recursive: true }); + // Skill conformance tests (elsewhere) check for frontmatter + triggers; + // checkResolvable itself only needs the body. + const frontmatter = `---\nname: ${name}\ndescription: test\ntriggers:\n - "${name}"\n---\n`; + writeFileSync(join(dir, name, "SKILL.md"), frontmatter + body); + } + return dir; +} + +describe("extractDelegationTargets", () => { + test("parses > **Convention:** callouts", () => { + const refs = extractDelegationTargets( + "> **Convention:** See `skills/conventions/quality.md` for citation rules.\n" + ); + expect(refs).toEqual([{ convention: "conventions/quality.md", line: 1 }]); + }); + + test("parses > **Filing rule:** callouts", () => { + const refs = extractDelegationTargets( + "> **Filing rule:** Read `skills/_brain-filing-rules.md` before any new page.\n" + ); + expect(refs).toEqual([{ convention: "_brain-filing-rules.md", line: 1 }]); + }); + + test("parses inline backtick references", () => { + const refs = extractDelegationTargets( + "some prose.\nSee `skills/conventions/quality.md` for details.\n" + ); + expect(refs).toEqual([{ convention: "conventions/quality.md", line: 2 }]); + }); + + test("ignores backticks pointing outside known delegation targets", () => { + const refs = extractDelegationTargets( + "See `skills/random/README.md` for unrelated notes.\n" + ); + expect(refs).toHaveLength(0); + }); + + test("handles frontmatter-only skill (no body matches)", () => { + const refs = extractDelegationTargets("---\nname: foo\n---\n"); + expect(refs).toHaveLength(0); + }); +}); + +describe("DRY detection — checkResolvable", () => { + let dir: string; + afterEachCleanup(() => dir && rmSync(dir, { recursive: true, force: true })); + + test("flags inlined notability rule with no reference", () => { + dir = makeSkillsFixture({ + bad: "# BadSkill\n\nCheck the notability gate every time.\n", + }); + const report = checkResolvable(dir); + const dry = report.issues.filter(i => i.type === "dry_violation"); + expect(dry).toHaveLength(1); + expect(dry[0].skill).toBe("bad"); + }); + + test("suppresses DRY when > **Convention:** callout points at quality.md (notability)", () => { + dir = makeSkillsFixture({ + good: `# GoodSkill\n\n> **Convention:** See \`skills/conventions/quality.md\` for rules.\n\nCheck the notability gate.\n`, + }); + const report = checkResolvable(dir); + const dry = report.issues.filter(i => i.type === "dry_violation"); + expect(dry).toHaveLength(0); + }); + + test("suppresses DRY when _brain-filing-rules.md is referenced for notability", () => { + dir = makeSkillsFixture({ + good: `# GoodSkill\n\n> **Filing rule:** Read \`skills/_brain-filing-rules.md\`.\n\nCheck the notability gate.\n`, + }); + const report = checkResolvable(dir); + const dry = report.issues.filter(i => i.type === "dry_violation"); + expect(dry).toHaveLength(0); + }); + + test("does NOT suppress when reference is >40 lines from the match", () => { + const filler = Array(50).fill("padding paragraph with no match.").join("\n"); + dir = makeSkillsFixture({ + distant: `> **Convention:** See \`skills/conventions/quality.md\`.\n\n${filler}\n\nCheck the notability gate now.\n`, + }); + const report = checkResolvable(dir); + const dry = report.issues.filter(i => i.type === "dry_violation"); + expect(dry).toHaveLength(1); + }); + + test("DOES suppress when reference is ~30 lines from the match", () => { + const filler = Array(20).fill("padding paragraph with no match.").join("\n"); + dir = makeSkillsFixture({ + near: `> **Convention:** See \`skills/conventions/quality.md\`.\n\n${filler}\n\nCheck the notability gate now.\n`, + }); + const report = checkResolvable(dir); + const dry = report.issues.filter(i => i.type === "dry_violation"); + expect(dry).toHaveLength(0); + }); + + test("iron-law pattern does NOT accept _brain-filing-rules.md as delegation", () => { + // iron-law's only accepted target is conventions/quality.md + dir = makeSkillsFixture({ + filing: `> **Filing rule:** Read \`skills/_brain-filing-rules.md\`.\n\n## Iron Law: Back-Linking (MANDATORY)\n`, + }); + const report = checkResolvable(dir); + const dry = report.issues.filter(i => i.type === "dry_violation"); + expect(dry.length).toBeGreaterThanOrEqual(1); + }); +}); + +// bun:test has no beforeEach/afterEach at module scope cleanly interacting +// with closures; a small helper keeps cleanup readable and per-test. +function afterEachCleanup(fn: () => void) { + const { afterEach } = require("bun:test"); + afterEach(fn); +} + diff --git a/test/doctor-fix.test.ts b/test/doctor-fix.test.ts new file mode 100644 index 000000000..5ac288ebc --- /dev/null +++ b/test/doctor-fix.test.ts @@ -0,0 +1,101 @@ +/** + * CLI integration tests for `gbrain doctor --fix` / `--dry-run`. + * Spawns the actual CLI against tmpdir skill fixtures to prove the + * arg-parsing wiring and stdout/file-state contract hold end-to-end. + */ +import { describe, test, expect, afterEach } from "bun:test"; +import { join } from "path"; +import { mkdtempSync, mkdirSync, writeFileSync, readFileSync, rmSync, existsSync } from "fs"; +import { tmpdir } from "os"; +import { spawnSync, execSync } from "child_process"; + +const CLI = join(import.meta.dir, "..", "src", "cli.ts"); +const REPO_ROOT = join(import.meta.dir, ".."); + +let fixtures: string[] = []; + +afterEach(() => { + for (const f of fixtures) { + try { rmSync(f, { recursive: true, force: true }); } catch { /* ignore */ } + } + fixtures = []; +}); + +function makeGitFixture(skills: Record): string { + // doctor finds repo root by looking for skills/RESOLVER.md — so wrap the + // fixture in a dir with skills/ inside and a RESOLVER.md stub. + const root = mkdtempSync(join(tmpdir(), "gbrain-doctorfix-")); + fixtures.push(root); + const skillsDir = join(root, "skills"); + mkdirSync(skillsDir, { recursive: true }); + const names = Object.keys(skills); + const rows = names.map(n => `| "${n}" | \`skills/${n}/SKILL.md\` |`).join("\n"); + writeFileSync( + join(skillsDir, "RESOLVER.md"), + `## Test\n| Trigger | Skill |\n|-----|-----|\n${rows}\n` + ); + writeFileSync( + join(skillsDir, "manifest.json"), + JSON.stringify({ skills: names.map(n => ({ name: n, path: `${n}/SKILL.md` })) }, null, 2) + ); + for (const [name, body] of Object.entries(skills)) { + mkdirSync(join(skillsDir, name), { recursive: true }); + const fm = `---\nname: ${name}\ndescription: test\ntriggers:\n - "${name}"\n---\n`; + writeFileSync(join(skillsDir, name, "SKILL.md"), fm + body); + } + execSync("git init --quiet", { cwd: root }); + execSync("git config user.email t@t", { cwd: root }); + execSync("git config user.name t", { cwd: root }); + execSync("git add -A && git commit --quiet -m init", { cwd: root }); + return root; +} + +function runDoctor(cwd: string, args: string[]): { stdout: string; stderr: string; status: number } { + const res = spawnSync("bun", [CLI, "doctor", "--fast", ...args], { + cwd, + encoding: "utf-8", + env: { ...process.env, NO_COLOR: "1" }, + }); + return { stdout: res.stdout, stderr: res.stderr, status: res.status ?? -1 }; +} + +describe("gbrain doctor --fix CLI integration", () => { + test("--fix --dry-run proposes a fix and does not write", () => { + const root = makeGitFixture({ + demo: "## Iron Law: Back-Linking (MANDATORY)\n\nbody paragraph.\n", + }); + const before = readFileSync(join(root, "skills", "demo", "SKILL.md"), "utf-8"); + const { stdout } = runDoctor(root, ["--fix", "--dry-run"]); + expect(stdout).toContain("[PROPOSED]"); + expect(stdout).toContain("Iron Law back-linking"); + expect(stdout).toContain("Run without --dry-run to apply."); + const after = readFileSync(join(root, "skills", "demo", "SKILL.md"), "utf-8"); + expect(after).toBe(before); + }); + + test("--fix applies, subsequent --fast run shows no DRY violation for fixed pattern", () => { + const root = makeGitFixture({ + demo: "## Iron Law: Back-Linking (MANDATORY)\n\nbody.\n", + }); + const { stdout: fixOut } = runDoctor(root, ["--fix"]); + expect(fixOut).toContain("[APPLIED]"); + const updated = readFileSync(join(root, "skills", "demo", "SKILL.md"), "utf-8"); + expect(updated).toContain("> **Convention:** See `skills/conventions/quality.md`"); + expect(updated).not.toContain("## Iron Law: Back-Linking"); + + // Re-run --fast (not --fix) — commit the fix first so the dirty guard + // doesn't fire and we're testing detection cleanly. + execSync("git add -A && git commit --quiet -m fixup", { cwd: root }); + const { stdout: checkOut } = runDoctor(root, ["--json"]); + const dryCount = (checkOut.match(/"type":"dry_violation"/g) || []).length; + expect(dryCount).toBe(0); + }); + + test("--fix with nothing to fix prints no-op message", () => { + const root = makeGitFixture({ + clean: "# CleanSkill\n\nNo cross-cutting patterns here.\n", + }); + const { stdout } = runDoctor(root, ["--fix"]); + expect(stdout).toContain("no DRY violations to repair"); + }); +}); diff --git a/test/dry-fix.test.ts b/test/dry-fix.test.ts new file mode 100644 index 000000000..f289e000c --- /dev/null +++ b/test/dry-fix.test.ts @@ -0,0 +1,284 @@ +import { describe, test, expect, afterEach } from "bun:test"; +import { join } from "path"; +import { mkdtempSync, mkdirSync, writeFileSync, readFileSync, rmSync, existsSync } from "fs"; +import { tmpdir } from "os"; +import { execSync } from "child_process"; +import { + autoFixDryViolations, + isInsideCodeFence, + detectBlockShape, + expandBullet, + expandBlockquote, + expandParagraph, +} from "../src/core/dry-fix.ts"; + +// --------------------------------------------------------------------------- +// Fixture helpers +// --------------------------------------------------------------------------- + +let fixtures: string[] = []; + +afterEach(() => { + for (const f of fixtures) { + try { rmSync(f, { recursive: true, force: true }); } catch { /* ignore */ } + } + fixtures = []; +}); + +function makeSkillsFixture(files: Record, opts: { gitInit?: boolean } = {}): string { + const dir = mkdtempSync(join(tmpdir(), "gbrain-dryfix-")); + fixtures.push(dir); + const skillNames = Object.keys(files); + writeFileSync( + join(dir, "manifest.json"), + JSON.stringify({ skills: skillNames.map(n => ({ name: n, path: `${n}/SKILL.md` })) }, null, 2) + ); + for (const [name, body] of Object.entries(files)) { + mkdirSync(join(dir, name), { recursive: true }); + writeFileSync(join(dir, name, "SKILL.md"), body); + } + if (opts.gitInit) { + execSync("git init --quiet", { cwd: dir }); + execSync("git config user.email test@test", { cwd: dir }); + execSync("git config user.name test", { cwd: dir }); + execSync("git add -A && git commit --quiet -m init", { cwd: dir }); + } + return dir; +} + +// --------------------------------------------------------------------------- +// Pure function tests: expanders and guards +// --------------------------------------------------------------------------- + +describe("detectBlockShape", () => { + test("bullet with dash", () => { + expect(detectBlockShape(["- a bullet"], 0)).toBe("bullet"); + }); + test("bullet with numeric", () => { + expect(detectBlockShape(["1. numbered"], 0)).toBe("bullet"); + }); + test("indented bullet", () => { + expect(detectBlockShape([" - nested"], 0)).toBe("bullet"); + }); + test("blockquote", () => { + expect(detectBlockShape(["> quoted"], 0)).toBe("blockquote"); + }); + test("paragraph default", () => { + expect(detectBlockShape(["plain text"], 0)).toBe("paragraph"); + }); +}); + +describe("expandBullet", () => { + test("single-line bullet", () => { + const lines = ["before", "", "- single bullet", "", "after"]; + const block = expandBullet(lines, 2); + expect(block).toEqual({ startLine: 2, endLine: 2 }); + }); + + test("bullet with sub-bullets", () => { + const lines = [ + "- top-level bullet", + " - sub one", + " - sub two", + "- next sibling", + ]; + const block = expandBullet(lines, 0); + expect(block).toEqual({ startLine: 0, endLine: 2 }); + }); + + test("stops at blank line", () => { + const lines = ["- item", "continuation", "", "- next"]; + const block = expandBullet(lines, 0); + expect(block).toEqual({ startLine: 0, endLine: 1 }); + }); +}); + +describe("expandBlockquote", () => { + test("contiguous quote lines", () => { + const lines = ["> line 1", "> line 2", "not quote"]; + const block = expandBlockquote(lines, 0); + expect(block).toEqual({ startLine: 0, endLine: 1 }); + }); + + test("returns null for Convention callout (don't rewrite reference)", () => { + const lines = ["> **Convention:** See `skills/conventions/quality.md`."]; + expect(expandBlockquote(lines, 0)).toBeNull(); + }); + + test("returns null for Filing rule callout", () => { + const lines = ["> **Filing rule:** Read `skills/_brain-filing-rules.md`."]; + expect(expandBlockquote(lines, 0)).toBeNull(); + }); +}); + +describe("expandParagraph", () => { + test("expands to blank boundaries", () => { + const lines = ["", "line a", "line b", "", "other"]; + const block = expandParagraph(lines, 1); + expect(block).toEqual({ startLine: 1, endLine: 2 }); + }); + + test("handles start of file", () => { + const lines = ["first line", "second", ""]; + const block = expandParagraph(lines, 0); + expect(block).toEqual({ startLine: 0, endLine: 1 }); + }); +}); + +describe("isInsideCodeFence", () => { + test("inside fenced block", () => { + const content = "pre\n```\nfenced notability gate\n```\npost\n"; + const offset = content.indexOf("fenced notability"); + expect(isInsideCodeFence(content, offset)).toBe(true); + }); + + test("outside fenced block", () => { + const content = "notability gate\n```\ncode\n```\n"; + const offset = content.indexOf("notability"); + expect(isInsideCodeFence(content, offset)).toBe(false); + }); + + test("after closed fence (regression guard)", () => { + const content = "```\nexample\n```\n\nreal notability gate here\n"; + const offset = content.indexOf("real notability"); + expect(isInsideCodeFence(content, offset)).toBe(false); + }); +}); + +// --------------------------------------------------------------------------- +// Integration tests: autoFixDryViolations +// --------------------------------------------------------------------------- + +describe("autoFixDryViolations", () => { + test("replaces paragraph-form heading (Iron Law)", () => { + const dir = makeSkillsFixture({ + a: "# A\n\n## Iron Law: Back-Linking (MANDATORY)\n\nbody text.\n", + }, { gitInit: true }); + const report = autoFixDryViolations(dir); + expect(report.fixed).toHaveLength(1); + expect(report.fixed[0].status).toBe("applied"); + const updated = readFileSync(join(dir, "a", "SKILL.md"), "utf-8"); + expect(updated).toContain("> **Convention:** See `skills/conventions/quality.md`"); + expect(updated).not.toContain("## Iron Law: Back-Linking"); + }); + + test("replaces bullet-item inlined rule", () => { + const dir = makeSkillsFixture({ + b: "# B\n\n- First\n- Check the notability gate before creating a page\n- Last\n", + }, { gitInit: true }); + const report = autoFixDryViolations(dir); + expect(report.fixed).toHaveLength(1); + const updated = readFileSync(join(dir, "b", "SKILL.md"), "utf-8"); + expect(updated).toContain("> **Convention:** See `skills/conventions/quality.md`"); + expect(updated).toContain("- First"); // surrounding bullets preserved + expect(updated).toContain("- Last"); + }); + + test("does NOT rewrite a Convention callout (block_is_callout)", () => { + const dir = makeSkillsFixture({ + c: "> **Convention:** See `skills/conventions/quality.md` for Iron Law back-linking rules.\n", + }, { gitInit: true }); + const report = autoFixDryViolations(dir); + // proximity suppression means no violation to fix in the first place + expect(report.fixed).toHaveLength(0); + }); + + test("skips match inside fenced code block", () => { + const dir = makeSkillsFixture({ + d: "# D\n\nExample:\n```\n## Iron Law: Back-Linking (MANDATORY)\n```\ntext.\n", + }, { gitInit: true }); + const report = autoFixDryViolations(dir); + const sk = report.skipped.find(s => s.reason === "inside_code_fence"); + expect(sk).toBeDefined(); + expect(report.fixed).toHaveLength(0); + }); + + test("skips when pattern matches more than once", () => { + const dir = makeSkillsFixture({ + e: "## Iron Law: Back-Linking (MANDATORY)\n\nThe Iron Law Back-Link applies to every entity.\n", + }, { gitInit: true }); + const report = autoFixDryViolations(dir); + const sk = report.skipped.find(s => s.reason === "ambiguous_multiple_matches"); + expect(sk).toBeDefined(); + }); + + test("skips when delegation already within 10 lines (idempotent)", () => { + const dir = makeSkillsFixture({ + f: "> **Convention:** See `skills/conventions/quality.md`.\n\nCheck the notability gate.\n", + }, { gitInit: true }); + const report = autoFixDryViolations(dir); + const sk = report.skipped.find(s => s.reason === "already_delegated"); + expect(sk).toBeDefined(); + expect(report.fixed).toHaveLength(0); + }); + + test("skips when working tree is dirty", () => { + const dir = makeSkillsFixture({ + g: "## Iron Law: Back-Linking (MANDATORY)\n\nbody.\n", + }, { gitInit: true }); + // dirty the file: add another line post-commit + const p = join(dir, "g", "SKILL.md"); + writeFileSync(p, readFileSync(p, "utf-8") + "\nextra edit\n"); + const report = autoFixDryViolations(dir); + const sk = report.skipped.find(s => s.reason === "working_tree_dirty"); + expect(sk).toBeDefined(); + // file unchanged + expect(readFileSync(p, "utf-8")).toContain("## Iron Law: Back-Linking"); + }); + + test("refuses to write when skill is NOT inside a git repo (no_git_backup)", () => { + // no gitInit — writing would destroy user data with no rollback + const dir = makeSkillsFixture({ + ng: "## Iron Law: Back-Linking (MANDATORY)\n\nbody.\n", + }, { gitInit: false }); + const p = join(dir, "ng", "SKILL.md"); + const before = readFileSync(p, "utf-8"); + const report = autoFixDryViolations(dir); + const sk = report.skipped.find(s => s.reason === "no_git_backup"); + expect(sk).toBeDefined(); + expect(report.fixed).toHaveLength(0); + expect(readFileSync(p, "utf-8")).toBe(before); + }); + + test("preserves trailing newline when block is at EOF", () => { + const dir = makeSkillsFixture({ + eof: "## Iron Law: Back-Linking (MANDATORY)\n", + }, { gitInit: true }); + const report = autoFixDryViolations(dir); + expect(report.fixed).toHaveLength(1); + const after = readFileSync(join(dir, "eof", "SKILL.md"), "utf-8"); + expect(after.endsWith("\n")).toBe(true); + }); + + test("dry-run mode does not write files", () => { + const dir = makeSkillsFixture({ + h: "# H\n\n## Iron Law: Back-Linking (MANDATORY)\n\nbody.\n", + }, { gitInit: true }); + const before = readFileSync(join(dir, "h", "SKILL.md"), "utf-8"); + const report = autoFixDryViolations(dir, { dryRun: true }); + expect(report.fixed).toHaveLength(1); + expect(report.fixed[0].status).toBe("proposed"); + const after = readFileSync(join(dir, "h", "SKILL.md"), "utf-8"); + expect(after).toBe(before); + }); + + test("ENOENT on skill file does not crash", () => { + const dir = makeSkillsFixture({ + i: "## Iron Law: Back-Linking (MANDATORY)\n", + }, { gitInit: true }); + // remove the skill file after fixture creation but before fix runs + rmSync(join(dir, "i", "SKILL.md")); + const report = autoFixDryViolations(dir); + // file_missing is silently skipped (already reported as missing_file elsewhere) + expect(report.fixed).toHaveLength(0); + }); + + test("notability gate accepts _brain-filing-rules.md as delegation", () => { + const dir = makeSkillsFixture({ + j: "> **Filing rule:** Read `skills/_brain-filing-rules.md`.\n\nCheck the notability gate.\n", + }, { gitInit: true }); + const report = autoFixDryViolations(dir); + // suppressed by proximity + filing-rule delegation + expect(report.fixed).toHaveLength(0); + }); +});