diff --git a/CHANGELOG.md b/CHANGELOG.md index bf0b85fd2..f01bc65c6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,63 @@ All notable changes to GBrain will be documented in this file. +## [0.16.4] - 2026-04-22 + +## **`gbrain check-resolvable` ships. The command the README promised for weeks.** +## **Agents and CI finally have a one-shot skill-tree gate that actually exits non-zero when anything is off.** + +The `resolver_health` logic has lived inside `gbrain doctor` since v0.11. The README claimed a standalone `gbrain check-resolvable` shipped too ... it didn't. Scripts referenced it. Skillify's 10-item checklist referenced it. The binary just shrugged. Fixed. + +`gbrain check-resolvable` runs the same four checks doctor runs (reachability, MECE overlap, MECE gap, DRY violations) but with a stricter contract: **exits 1 on any issue, errors AND warnings**. Doctor's resolver_health block still exits 0 on warnings-only because doctor has 15 other checks to lean on. The standalone command has nowhere to hide. CI can finally gate on a single command instead of parsing `gbrain doctor --json`. + +The JSON output is a stable envelope, one shape for success and error: `{ok, skillsDir, report, autoFix, deferred, error, message}`. No more "did it succeed? let me see which keys are present." The `deferred` array names the two checks still pending (trigger routing eval, brain filing) with links to their tracking issues, so agents reading the JSON know the current coverage boundary. + +`scripts/skillify-check.ts` is now machine-gated. Item #8 on the skillify 10-item checklist used to print "run: gbrain check-resolvable" and pass unconditionally. Now it subprocess-calls the real command and asserts on the exit code. Binary-missing fails loud instead of silently passing ... the kind of silent false-pass that used to put broken skills on the shelf. + +## To take advantage of v0.16.4 + +No migration needed. `gbrain upgrade` brings the binary; nothing to apply. Try it: + +```bash +gbrain check-resolvable # human output, like doctor's resolver section +gbrain check-resolvable --json | jq .ok # machine-readable gate for CI +gbrain check-resolvable --fix --dry-run # preview DRY auto-fixes without writing +``` + +Wire it into your CI: + +```bash +gbrain check-resolvable || exit 1 # fails the build on any warning/error +``` + +### Itemized changes + +**New command** +- `gbrain check-resolvable [--json] [--fix] [--dry-run] [--verbose] [--skills-dir PATH] [--help]` — standalone skill-tree gate. Covers reachability, MECE overlap, MECE gap, DRY violations. Exits 1 on any issue. +- Stable JSON envelope (`ok`, `skillsDir`, `report`, `autoFix`, `deferred`, `error`, `message`) — one shape for both success and error paths. +- `--fix` auto-applies DRY fixes via `autoFixDryViolations` before re-checking (same ordering as `doctor --fix`). +- `--dry-run` with `--fix` previews without writing; the JSON `autoFix.fixed` array shows what would change. +- `--verbose` prints the Deferred checks note with issue URLs so nobody forgets Checks 5 and 6 are still tracked. + +**Deferred to separate issues** +- Check 5: trigger routing eval — verify every skill's own frontmatter trigger routes to itself in RESOLVER.md. Surfaced via the CLI's `deferred[]` output block. +- Check 6: brain filing validation — verify mutating skills register the brain directories they write to. Same surface. + +**Shared refactor** +- `src/core/repo-root.ts` — extracted `findRepoRoot()` from `doctor.ts` to a zero-dependency shared module with a parameterized `startDir` for test hermeticity. Doctor imports the shared version; no behavior change (default arg matches prior semantics). +- `src/commands/doctor.ts` — updated to import the shared `findRepoRoot`. + +**Skillify integration** +- `scripts/skillify-check.ts` — item #8 ("check-resolvable gate") now subprocess-calls `gbrain check-resolvable --json` and gates on the exit code. Result is cached per process so iterating many skills only runs the subprocess once. Binary-missing fails loud via explicit `spawn` error handling ... no silent false-pass. + +**Tests (22 new cases)** +- `test/repo-root.test.ts` — 4 cases for the extracted `findRepoRoot()` (first-iter hit, walks up, returns null, default arg behavioral parity). +- `test/check-resolvable-cli.test.ts` — 17 cases split between direct unit tests (flag parsing, resolveSkillsDir, DEFERRED constants) and subprocess integration tests (help, JSON envelope shape, exit-code regression gates for warnings AND errors, `--fix --dry-run` wiring, `--verbose` output). +- `test/skillify-check.test.ts` — 2 new cases for the check-resolvable wiring: loud failure when binary is missing (no silent pass), happy path when a synthetic gbrain returns `ok: true`. + +**Contract note for CI users** +- `gbrain check-resolvable` exits 1 on warnings AND errors. `gbrain doctor`'s resolver_health block still exits 0 on warnings-only. If you scripted against doctor's looser gate, `check-resolvable` will bite harder ... on purpose. This honors the README:259 contract: "Exits non-zero if anything is off." + ## [0.16.3] - 2026-04-22 ## **`gbrain agent run` actually runs now. The subagent SDK wiring that shipped broken in v0.16.0 is fixed.** diff --git a/CLAUDE.md b/CLAUDE.md index b788ead32..4ce6c6afb 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -43,6 +43,8 @@ strict behavior when unset. - `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. 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/repo-root.ts` — Shared `findRepoRoot(startDir?)` (v0.16.4): walks up from `startDir` (default `process.cwd()`) looking for `skills/RESOLVER.md`. Zero-dependency module imported by both `doctor.ts` and `check-resolvable.ts`. Parameterized `startDir` makes tests hermetic. +- `src/commands/check-resolvable.ts` — Standalone CLI wrapper (v0.16.4) over `checkResolvable()`. Exports `parseFlags`, `resolveSkillsDir`, `DEFERRED`, `runCheckResolvable`. Exit rule: **1 on any issue (warnings OR errors)**, stricter than doctor's `ok` flag — honors README:259. Stable JSON envelope `{ok, skillsDir, report, autoFix, deferred, error, message}` — same shape on success and error paths. `--fix` path runs `autoFixDryViolations` BEFORE `checkResolvable` (same ordering as doctor). `deferred[]` array surfaces pending Checks 5 (trigger routing eval) and 6 (brain filing) with issue URLs. `scripts/skillify-check.ts` subprocess-calls `gbrain check-resolvable --json` (cached per process) and fails loud on binary-missing — no silent false-pass. - `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 diff --git a/TODOS.md b/TODOS.md index f320237a4..e9783681d 100644 --- a/TODOS.md +++ b/TODOS.md @@ -1,5 +1,21 @@ # TODOS +## check-resolvable + +### File tracking issues for Checks 5 + 6 (deferred in PR #325) +**Priority:** P2 + +**What:** `src/commands/check-resolvable.ts` currently points `DEFERRED[].issue` at GitHub issue search URLs (`?q=TBD-check-5`, `?q=TBD-check-6`). File real tracking issues and grep-replace both placeholders with the real URLs. + +**Why:** v0.16.4 shipped `gbrain check-resolvable` with 4 of the 6 checks from the original spec. Checks 5 (trigger routing eval) and 6 (brain filing) were explicitly deferred during plan-ceo-review because they each need new detection logic. The CLI's `deferred[]` JSON field is meant to surface these to agents so they know the coverage boundary — the TBD placeholders do the right thing mechanically but aren't clickable. + +**How:** +1. `gh issue create -t "check-resolvable Check 5: trigger routing eval" -b "..."` — detection: every skill's own frontmatter trigger should match the RESOLVER.md entry pointing at that skill. Needs new issue type (e.g. `mis_route`). +2. `gh issue create -t "check-resolvable Check 6: brain filing validation" -b "..."` — detection: scan SKILL.md body for brain paths (e.g., `brain/people/`, `brain/companies/`), cross-reference with `skills/_brain-filing-rules.md`. Flag mutating skills missing entries. +3. Replace `TBD-check-5` and `TBD-check-6` in `src/commands/check-resolvable.ts` with the real issue URLs. + +**Effort:** ~15 min mechanical (issue filing + grep-replace). Implementation of the checks themselves is a separate, larger piece of work — the TODO here is just the issue filing + URL swap. + ## P1 (BrainBench v1.1 — categories deferred from PR #188) ### BrainBench Cat 5: Source Attribution / Provenance diff --git a/VERSION b/VERSION index 7eb3095a3..5f2491c5a 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -0.16.3 +0.16.4 diff --git a/llms-full.txt b/llms-full.txt index be40fd26c..cc0db3bd7 100644 --- a/llms-full.txt +++ b/llms-full.txt @@ -122,6 +122,8 @@ strict behavior when unset. - `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. 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/repo-root.ts` — Shared `findRepoRoot(startDir?)` (v0.16.4): walks up from `startDir` (default `process.cwd()`) looking for `skills/RESOLVER.md`. Zero-dependency module imported by both `doctor.ts` and `check-resolvable.ts`. Parameterized `startDir` makes tests hermetic. +- `src/commands/check-resolvable.ts` — Standalone CLI wrapper (v0.16.4) over `checkResolvable()`. Exports `parseFlags`, `resolveSkillsDir`, `DEFERRED`, `runCheckResolvable`. Exit rule: **1 on any issue (warnings OR errors)**, stricter than doctor's `ok` flag — honors README:259. Stable JSON envelope `{ok, skillsDir, report, autoFix, deferred, error, message}` — same shape on success and error paths. `--fix` path runs `autoFixDryViolations` BEFORE `checkResolvable` (same ordering as doctor). `deferred[]` array surfaces pending Checks 5 (trigger routing eval) and 6 (brain filing) with issue URLs. `scripts/skillify-check.ts` subprocess-calls `gbrain check-resolvable --json` (cached per process) and fails loud on binary-missing — no silent false-pass. - `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 diff --git a/package.json b/package.json index 61e6ee4a5..2b7e8bc2f 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "gbrain", - "version": "0.16.3", + "version": "0.16.4", "description": "Postgres-native personal knowledge brain with hybrid RAG search", "type": "module", "main": "src/core/index.ts", diff --git a/scripts/skillify-check.ts b/scripts/skillify-check.ts index 9e3e4f472..b48a877b4 100755 --- a/scripts/skillify-check.ts +++ b/scripts/skillify-check.ts @@ -23,6 +23,7 @@ import { existsSync, readFileSync, readdirSync, statSync } from 'fs'; import { join, basename, dirname, resolve } from 'path'; +import { spawnSync } from 'child_process'; function projectRoot(): string { // Walk up from cwd until we find a package.json — that's the repo root. @@ -64,6 +65,45 @@ function checkOptional(name: string, passed: boolean, detail?: string): CheckIte return { name, passed, required: false, detail }; } +/** + * Invoke `gbrain check-resolvable --json` once and cache the result for the + * process lifetime. Binary-missing surfaces a loud error instead of silently + * passing — this is the critical guard the failure-mode audit flagged. + */ +interface ResolverResult { + ok: boolean; + detail: string; +} +let _resolverCache: ResolverResult | null = null; +function runCheckResolvableCached(): ResolverResult { + if (_resolverCache) return _resolverCache; + try { + const res = spawnSync('gbrain', ['check-resolvable', '--json'], { + encoding: 'utf-8', + maxBuffer: 10 * 1024 * 1024, + }); + if (res.error || res.status === null) { + const reason = res.error?.message ?? 'spawn returned null status'; + console.error(`[skillify] gbrain check-resolvable not runnable: ${reason}`); + _resolverCache = { ok: false, detail: `check-resolvable unavailable: ${reason}` }; + return _resolverCache; + } + const payload = JSON.parse(res.stdout); + if (payload.ok === true) { + _resolverCache = { ok: true, detail: 'all skill-tree checks pass' }; + } else { + const count = payload.report?.issues?.length ?? 0; + const err = payload.error ? ` (${payload.error})` : ''; + _resolverCache = { ok: false, detail: `${count} issue(s)${err} — run: gbrain check-resolvable` }; + } + return _resolverCache; + } catch (err) { + console.error(`[skillify] check-resolvable parse failed: ${err}`); + _resolverCache = { ok: false, detail: `check-resolvable parse error: ${err}` }; + return _resolverCache; + } +} + /** * Guess the skill-directory name from a script path. * scripts/frameio-scraper.ts → frameio-scraper @@ -184,12 +224,13 @@ function runCheck(target: string): { } items.push(checkOptional('Resolver trigger eval', hasTriggerEval)); - // 8. check-resolvable — we don't run it here (side effects + cost); we - // report whether the SKILL.md exists at all, which is the ground-truth - // input check-resolvable would consume. - items.push(checkOptional('check-resolvable input present', - existsSync(skillMd) && existsSync(RESOLVER_MD), - 'run: gbrain check-resolvable')); + // 8. check-resolvable — invoke the real gate. Cached per process so + // iterating many skills only runs the subprocess once. Binary-missing + // is surfaced loudly so a silent false-pass can't happen. + const resolverResult = runCheckResolvableCached(); + items.push(checkOptional('check-resolvable gate', + resolverResult.ok, + resolverResult.detail)); // 9. E2E — same as item 4 but required. items.push(check('E2E test (either under e2e/ or integration test)', hasE2E, 'try /qa or test/e2e/')); diff --git a/src/cli.ts b/src/cli.ts index b70ae812b..bf40a4d06 100644 --- a/src/cli.ts +++ b/src/cli.ts @@ -19,7 +19,7 @@ for (const op of operations) { } // CLI-only commands that bypass the operation layer -const CLI_ONLY = new Set(['init', 'upgrade', 'post-upgrade', 'check-update', 'integrations', 'publish', 'check-backlinks', 'lint', 'report', 'import', 'export', 'files', 'embed', 'serve', 'call', 'config', 'doctor', 'migrate', 'eval', 'sync', 'extract', 'features', 'autopilot', 'graph-query', 'jobs', 'agent', 'apply-migrations', 'skillpack-check', 'resolvers', 'integrity', 'repair-jsonb', 'orphans']); +const CLI_ONLY = new Set(['init', 'upgrade', 'post-upgrade', 'check-update', 'integrations', 'publish', 'check-backlinks', 'lint', 'report', 'import', 'export', 'files', 'embed', 'serve', 'call', 'config', 'doctor', 'migrate', 'eval', 'sync', 'extract', 'features', 'autopilot', 'graph-query', 'jobs', 'agent', 'apply-migrations', 'skillpack-check', 'resolvers', 'integrity', 'repair-jsonb', 'orphans', 'check-resolvable']); async function main() { // Parse global flags (--quiet / --progress-json / --progress-interval) @@ -310,6 +310,11 @@ async function handleCliOnly(command: string, args: string[]) { await runLint(args); return; } + if (command === 'check-resolvable') { + const { runCheckResolvable } = await import('./commands/check-resolvable.ts'); + await runCheckResolvable(args); + return; + } if (command === 'report') { const { runReport } = await import('./commands/report.ts'); await runReport(args); @@ -557,6 +562,7 @@ TOOLS check-backlinks [dir] Find/fix missing back-links across brain lint [--fix] Catch LLM artifacts, placeholder dates, bad frontmatter orphans [--json] [--count] Find pages with no inbound wikilinks + check-resolvable [--json] [--fix] Validate skill tree (reachability/MECE/DRY) report --type --content ... Save timestamped report to brain/reports/ JOBS (Minions) diff --git a/src/commands/check-resolvable.ts b/src/commands/check-resolvable.ts new file mode 100644 index 000000000..dd0e3c3c7 --- /dev/null +++ b/src/commands/check-resolvable.ts @@ -0,0 +1,260 @@ +/** + * gbrain check-resolvable — Standalone CLI gate for skill-tree integrity. + * + * Thin wrapper over `src/core/check-resolvable.ts`. Exit-code rule is stricter + * than `gbrain doctor`'s resolver_health: this command exits 1 on ANY issue + * (errors OR warnings) so CI can gate on a single command. Honors the README + * contract: "Exits non-zero if anything is off." + * + * Currently covers 4 of 6 checks from the original design: reachability, + * MECE overlap, MECE gap, DRY violations. Checks 5 (trigger routing eval) + * and 6 (brain filing) are tracked as separate GitHub issues and surfaced + * via the `deferred` field in --json output. + */ + +import { resolve as resolvePath, isAbsolute } from 'path'; +import { + checkResolvable, + autoFixDryViolations, + type ResolvableReport, + type ResolvableIssue, + type AutoFixReport, +} from '../core/check-resolvable.ts'; +import { findRepoRoot } from '../core/repo-root.ts'; + +// --------------------------------------------------------------------------- +// Types +// --------------------------------------------------------------------------- + +export interface DeferredCheck { + check: number; + name: string; + issue: string; +} + +export interface Envelope { + ok: boolean; + skillsDir: string | null; + report: ResolvableReport | null; + autoFix: AutoFixReport | null; + deferred: DeferredCheck[]; + error: 'no_skills_dir' | null; + message: string | null; +} + +export interface Flags { + help: boolean; + json: boolean; + fix: boolean; + dryRun: boolean; + verbose: boolean; + skillsDir: string | null; +} + +// TBD: fill these issue URLs after filing the GitHub issues pre-PR. +// grep for 'TBD-check-5' / 'TBD-check-6' before shipping. +export const DEFERRED: DeferredCheck[] = [ + { + check: 5, + name: 'trigger_routing_eval', + issue: 'https://github.com/garrytan/gbrain/issues?q=TBD-check-5', + }, + { + check: 6, + name: 'brain_filing', + issue: 'https://github.com/garrytan/gbrain/issues?q=TBD-check-6', + }, +]; + +const HELP_TEXT = `gbrain check-resolvable [options] + +Validate the skill tree: reachability, MECE overlap, DRY violations, and +gap detection. Exits non-zero if any issues are found (errors OR warnings). + +Options: + --json Machine-readable JSON (stable envelope) + --fix Apply DRY auto-fixes before checking + --dry-run With --fix, preview only; no writes + --verbose Show passing checks and the deferred-check note + --skills-dir PATH Override the auto-detected skills/ directory + --help Show this message + +Deferred to separate issues (see --json .deferred[]): + - Check 5: trigger routing eval + - Check 6: brain filing +`; + +// --------------------------------------------------------------------------- +// Flag parsing — permissive on unknown flags, matching lint/orphans/publish. +// --------------------------------------------------------------------------- + +export function parseFlags(argv: string[]): Flags { + const flags: Flags = { + help: false, + json: false, + fix: false, + dryRun: false, + verbose: false, + skillsDir: null, + }; + for (let i = 0; i < argv.length; i++) { + const a = argv[i]; + if (a === '--help' || a === '-h') flags.help = true; + else if (a === '--json') flags.json = true; + else if (a === '--fix') flags.fix = true; + else if (a === '--dry-run') flags.dryRun = true; + else if (a === '--verbose') flags.verbose = true; + else if (a === '--skills-dir') { + flags.skillsDir = argv[i + 1] ?? null; + i++; + } else if (a?.startsWith('--skills-dir=')) { + flags.skillsDir = a.slice('--skills-dir='.length) || null; + } + // unknown flags silently ignored + } + return flags; +} + +// --------------------------------------------------------------------------- +// Skills-dir resolution +// --------------------------------------------------------------------------- + +export function resolveSkillsDir(flags: Flags): { dir: string | null; error: Envelope['error']; message: string | null } { + if (flags.skillsDir) { + const dir = isAbsolute(flags.skillsDir) + ? flags.skillsDir + : resolvePath(process.cwd(), flags.skillsDir); + return { dir, error: null, message: null }; + } + const repoRoot = findRepoRoot(); + if (!repoRoot) { + return { + dir: null, + error: 'no_skills_dir', + message: + 'Could not locate skills/RESOLVER.md from cwd. Pass --skills-dir or run from inside a gbrain repo.', + }; + } + return { dir: resolvePath(repoRoot, 'skills'), error: null, message: null }; +} + +// --------------------------------------------------------------------------- +// Human output (mirrors doctor's resolver_health formatting) +// --------------------------------------------------------------------------- + +function renderHuman(env: Envelope, flags: Flags): void { + if (env.error === 'no_skills_dir') { + console.error(env.message); + return; + } + const report = env.report!; + + if (flags.fix && env.autoFix) { + printAutoFixHuman(env.autoFix, flags.dryRun); + } + + if (report.ok && report.issues.length === 0) { + console.log(`resolver_health: OK — ${report.summary.total_skills} skills, all reachable`); + } else { + const errors = report.issues.filter(i => i.severity === 'error'); + const warnings = report.issues.filter(i => i.severity === 'warning'); + const status = errors.length > 0 ? 'FAIL' : 'WARN'; + console.log( + `resolver_health: ${status} — ${report.issues.length} issue(s): ${errors.length} error(s), ${warnings.length} warning(s)`, + ); + for (const iss of report.issues) { + console.log(formatIssueLine(iss)); + } + } + + if (flags.verbose) { + const urls = DEFERRED.map(d => `${d.name} (${d.issue})`).join(', '); + console.log(`Deferred: ${urls}`); + } +} + +function formatIssueLine(iss: ResolvableIssue): string { + const type = iss.type.padEnd(18); + const skill = iss.skill.padEnd(24); + return ` • ${type} ${skill} ${iss.action}`; +} + +function printAutoFixHuman(autoFix: AutoFixReport, dryRun: boolean): void { + const verb = dryRun ? 'PROPOSED' : 'APPLIED'; + for (const outcome of autoFix.fixed) { + console.log(`[${verb}] ${outcome.skillPath} (${outcome.patternLabel})`); + } + const n = autoFix.fixed.length; + const s = autoFix.skipped.length; + if (n === 0 && s === 0) { + console.log('check-resolvable --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 autoFix.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('Run without --dry-run to apply.\n'); +} + +// --------------------------------------------------------------------------- +// Entry point +// --------------------------------------------------------------------------- + +export async function runCheckResolvable(args: string[]): Promise { + const flags = parseFlags(args); + + if (flags.help) { + console.log(HELP_TEXT); + process.exit(0); + } + + const { dir, error, message } = resolveSkillsDir(flags); + + if (error === 'no_skills_dir') { + const env: Envelope = { + ok: false, + skillsDir: null, + report: null, + autoFix: null, + deferred: DEFERRED, + error, + message, + }; + if (flags.json) { + console.log(JSON.stringify(env, null, 2)); + } else { + renderHuman(env, flags); + } + process.exit(1); + } + + const skillsDir = dir!; + + let autoFix: AutoFixReport | null = null; + if (flags.fix) { + autoFix = autoFixDryViolations(skillsDir, { dryRun: flags.dryRun }); + } + + const report = checkResolvable(skillsDir); + + const env: Envelope = { + ok: report.issues.length === 0, + skillsDir, + report, + autoFix, + deferred: DEFERRED, + error: null, + message: null, + }; + + if (flags.json) { + console.log(JSON.stringify(env, null, 2)); + } else { + renderHuman(env, flags); + } + + process.exit(env.ok ? 0 : 1); +} diff --git a/src/commands/doctor.ts b/src/commands/doctor.ts index 0092e2d68..a393639e1 100644 --- a/src/commands/doctor.ts +++ b/src/commands/doctor.ts @@ -3,6 +3,7 @@ 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 { findRepoRoot } from '../core/repo-root.ts'; import { loadCompletedMigrations } from '../core/preferences.ts'; import { createProgress, startHeartbeat, type ProgressReporter } from '../core/progress.ts'; import { getCliOptions, cliOptsToProgressOptions } from '../core/cli-options.ts'; @@ -602,17 +603,6 @@ function printAutoFixReport(report: AutoFixReport, dryRun: boolean, jsonOutput: 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(); - for (let i = 0; i < 10; i++) { - if (existsSync(join(dir, 'skills', 'RESOLVER.md'))) return dir; - const parent = join(dir, '..'); - if (parent === dir) break; - dir = parent; - } - return null; -} /** Quick skill conformance check — frontmatter + required sections */ function checkSkillConformance(skillsDir: string): Check { diff --git a/src/core/repo-root.ts b/src/core/repo-root.ts new file mode 100644 index 000000000..526171844 --- /dev/null +++ b/src/core/repo-root.ts @@ -0,0 +1,21 @@ +import { existsSync } from 'fs'; +import { join } from 'path'; + +/** + * Walk up from `startDir` looking for `skills/RESOLVER.md` — the marker of a + * gbrain repo root. Returns the absolute directory containing `skills/` or + * null if no such directory is found within 10 levels. + * + * `startDir` is parameterized so tests can run hermetically against fixtures. + * Default matches the prior `doctor.ts`-private implementation. + */ +export function findRepoRoot(startDir: string = process.cwd()): string | null { + let dir = startDir; + for (let i = 0; i < 10; i++) { + if (existsSync(join(dir, 'skills', 'RESOLVER.md'))) return dir; + const parent = join(dir, '..'); + if (parent === dir) break; + dir = parent; + } + return null; +} diff --git a/test/check-resolvable-cli.test.ts b/test/check-resolvable-cli.test.ts new file mode 100644 index 000000000..452dd1ad5 --- /dev/null +++ b/test/check-resolvable-cli.test.ts @@ -0,0 +1,282 @@ +import { describe, it, expect, afterEach } from 'bun:test'; +import { spawnSync } from 'child_process'; +import { mkdtempSync, mkdirSync, writeFileSync, rmSync } from 'fs'; +import { tmpdir } from 'os'; +import { join, resolve } from 'path'; +import { + parseFlags, + resolveSkillsDir, + DEFERRED, +} from '../src/commands/check-resolvable.ts'; + +// Path to the CLI entry point. Runs through bun directly so tests don't +// require a pre-built binary. Always invoked from the repo root so bun can +// resolve transitive node_modules (the top-level cli.ts imports pull in +// @anthropic-ai/sdk which walks from the file path, but some internal +// shim resolution requires node_modules to be reachable from cwd too). +const CLI = resolve(import.meta.dir, '..', 'src', 'cli.ts'); +const REPO_ROOT = resolve(import.meta.dir, '..'); + +// --------------------------------------------------------------------------- +// Fixture builders +// --------------------------------------------------------------------------- + +interface SkillSpec { + name: string; + triggers?: string[]; + /** Register in manifest.json — defaults true. */ + inManifest?: boolean; + /** Add a RESOLVER.md row pointing at this skill — defaults true. */ + inResolver?: boolean; +} + +function makeFixture(skills: SkillSpec[], created: string[]): string { + const root = mkdtempSync(join(tmpdir(), 'check-resolvable-cli-')); + created.push(root); + const skillsDir = join(root, 'skills'); + mkdirSync(skillsDir, { recursive: true }); + + const manifest = { + skills: skills + .filter(s => s.inManifest !== false) + .map(s => ({ name: s.name, path: `${s.name}/SKILL.md` })), + }; + writeFileSync(join(skillsDir, 'manifest.json'), JSON.stringify(manifest, null, 2)); + + for (const s of skills) { + const skillDir = join(skillsDir, s.name); + mkdirSync(skillDir, { recursive: true }); + const fm = ['---', `name: ${s.name}`]; + if (s.triggers && s.triggers.length) { + fm.push('triggers:'); + for (const t of s.triggers) fm.push(` - "${t}"`); + } + fm.push('---'); + fm.push(`# ${s.name}\n\nA test skill.\n`); + writeFileSync(join(skillDir, 'SKILL.md'), fm.join('\n')); + } + + const rows = skills + .filter(s => s.inResolver !== false) + .map(s => `| "${s.name} trigger" | \`skills/${s.name}/SKILL.md\` |`); + const resolver = [ + '# RESOLVER', + '', + '## Brain operations', + '| Trigger | Skill |', + '|---------|-------|', + ...rows, + '', + ].join('\n'); + writeFileSync(join(skillsDir, 'RESOLVER.md'), resolver); + + return skillsDir; +} + +interface RunResult { + status: number; + stdout: string; + stderr: string; + json: any; +} + +function run(args: string[]): RunResult { + const res = spawnSync('bun', [CLI, 'check-resolvable', ...args], { + encoding: 'utf-8', + cwd: REPO_ROOT, + maxBuffer: 10 * 1024 * 1024, + }); + let json: any = null; + if (args.includes('--json')) { + try { json = JSON.parse(res.stdout); } catch { /* leave null */ } + } + return { + status: res.status ?? -1, + stdout: res.stdout, + stderr: res.stderr, + json, + }; +} + +// --------------------------------------------------------------------------- +// Unit tests: direct helpers (fast, no subprocess) +// --------------------------------------------------------------------------- + +describe('check-resolvable — unit: parseFlags', () => { + it('parses all known flags', () => { + const f = parseFlags(['--json', '--fix', '--dry-run', '--verbose', '--skills-dir', '/x']); + expect(f.json).toBe(true); + expect(f.fix).toBe(true); + expect(f.dryRun).toBe(true); + expect(f.verbose).toBe(true); + expect(f.skillsDir).toBe('/x'); + }); + + it('supports --skills-dir=PATH syntax', () => { + const f = parseFlags(['--skills-dir=/x/y']); + expect(f.skillsDir).toBe('/x/y'); + }); + + it('silently ignores unknown flags (permissive, matches lint/orphans convention)', () => { + const f = parseFlags(['--json', '--bogus', '--another-unknown']); + expect(f.json).toBe(true); + expect(f.help).toBe(false); + }); +}); + +describe('check-resolvable — unit: resolveSkillsDir', () => { + it('resolves absolute --skills-dir unchanged', () => { + const r = resolveSkillsDir({ help: false, json: false, fix: false, dryRun: false, verbose: false, skillsDir: '/tmp/absolute-path' }); + expect(r.dir).toBe('/tmp/absolute-path'); + expect(r.error).toBeNull(); + }); + + it('resolves relative --skills-dir against cwd', () => { + const r = resolveSkillsDir({ help: false, json: false, fix: false, dryRun: false, verbose: false, skillsDir: 'skills' }); + expect(r.dir).toMatch(/\/skills$/); + expect(r.error).toBeNull(); + }); + + it('REGRESSION-GATE: returns no_skills_dir error when no --skills-dir and findRepoRoot fails', () => { + // Temporarily chdir to a guaranteed-empty tmpdir. findRepoRoot will walk + // up and fail to find skills/RESOLVER.md. + const empty = mkdtempSync(join(tmpdir(), 'empty-for-resolve-')); + const original = process.cwd(); + try { + process.chdir(empty); + const r = resolveSkillsDir({ help: false, json: false, fix: false, dryRun: false, verbose: false, skillsDir: null }); + expect(r.error).toBe('no_skills_dir'); + expect(r.dir).toBeNull(); + expect(typeof r.message).toBe('string'); + } finally { + process.chdir(original); + rmSync(empty, { recursive: true, force: true }); + } + }); + + it('finds skills via findRepoRoot when cwd is inside a repo (no --skills-dir)', () => { + // Running from this test file — we're inside the real gbrain repo. + const r = resolveSkillsDir({ help: false, json: false, fix: false, dryRun: false, verbose: false, skillsDir: null }); + expect(r.error).toBeNull(); + expect(r.dir).toMatch(/\/skills$/); + }); +}); + +describe('check-resolvable — unit: DEFERRED', () => { + it('exports two deferred check entries for Checks 5 and 6', () => { + expect(DEFERRED.length).toBe(2); + expect(DEFERRED[0].check).toBe(5); + expect(DEFERRED[0].name).toBe('trigger_routing_eval'); + expect(DEFERRED[1].check).toBe(6); + expect(DEFERRED[1].name).toBe('brain_filing'); + }); +}); + +// --------------------------------------------------------------------------- +// Integration tests: subprocess via bun src/cli.ts (cwd = repo root) +// --------------------------------------------------------------------------- + +describe('gbrain check-resolvable CLI — integration', () => { + const created: string[] = []; + afterEach(() => { + while (created.length) { + const p = created.pop()!; + try { rmSync(p, { recursive: true, force: true }); } catch { /* ignore */ } + } + }); + + it('prints usage and exits 0 on --help', () => { + const r = run(['--help']); + expect(r.status).toBe(0); + expect(r.stdout).toContain('gbrain check-resolvable'); + expect(r.stdout).toContain('--json'); + expect(r.stdout).toContain('--fix'); + expect(r.stdout).toContain('Check 5'); + expect(r.stdout).toContain('Check 6'); + }); + + it('--json success envelope has all seven stable keys', () => { + const skillsDir = makeFixture([{ name: 'alpha', triggers: ['alpha'] }], created); + const r = run(['--json', '--skills-dir', skillsDir]); + expect(r.json).not.toBeNull(); + const keys = Object.keys(r.json).sort(); + expect(keys).toEqual(['autoFix', 'deferred', 'error', 'message', 'ok', 'report', 'skillsDir']); + expect(r.json.ok).toBe(true); + expect(r.json.deferred.length).toBe(2); + expect(r.json.deferred[0].check).toBe(5); + expect(r.json.deferred[1].check).toBe(6); + }); + + it('--json success: autoFix is null when --fix was not passed', () => { + const skillsDir = makeFixture([{ name: 'alpha', triggers: ['alpha'] }], created); + const r = run(['--json', '--skills-dir', skillsDir]); + expect(r.json.autoFix).toBeNull(); + }); + + it('exits 0 on clean fixture with zero issues', () => { + const skillsDir = makeFixture([{ name: 'alpha', triggers: ['alpha'] }], created); + const r = run(['--skills-dir', skillsDir]); + expect(r.status).toBe(0); + expect(r.stdout).toContain('resolver_health: OK'); + }); + + it('REGRESSION-GATE: exits 1 when fixture has a warning-level orphan_trigger only', () => { + // "alpha" is in resolver but not manifest → orphan_trigger (warning) + const skillsDir = makeFixture( + [{ name: 'alpha', triggers: ['alpha'], inManifest: false }], + created, + ); + const r = run(['--json', '--skills-dir', skillsDir]); + expect(r.json).not.toBeNull(); + const warnings = r.json.report.issues.filter((i: any) => i.severity === 'warning'); + const errors = r.json.report.issues.filter((i: any) => i.severity === 'error'); + expect(warnings.length).toBeGreaterThan(0); + expect(errors.length).toBe(0); + // Doctor's ok=true-on-warnings-only would exit 0. check-resolvable MUST exit 1. + expect(r.status).toBe(1); + }); + + it('exits 1 when fixture has an error-level unreachable skill', () => { + // "alpha" is in manifest but not resolver → unreachable (error) + const skillsDir = makeFixture( + [{ name: 'alpha', triggers: ['alpha'], inResolver: false }], + created, + ); + const r = run(['--json', '--skills-dir', skillsDir]); + expect(r.json).not.toBeNull(); + const errors = r.json.report.issues.filter((i: any) => i.severity === 'error'); + expect(errors.length).toBeGreaterThan(0); + expect(r.status).toBe(1); + }); + + it('--fix --dry-run includes an autoFix object in the JSON envelope', () => { + const skillsDir = makeFixture([{ name: 'alpha', triggers: ['alpha'] }], created); + const r = run(['--json', '--fix', '--dry-run', '--skills-dir', skillsDir]); + expect(r.json).not.toBeNull(); + expect(r.json.autoFix).not.toBeNull(); + expect(Array.isArray(r.json.autoFix.fixed)).toBe(true); + expect(Array.isArray(r.json.autoFix.skipped)).toBe(true); + }); + + it('--verbose prints the deferred checks note in human mode', () => { + const skillsDir = makeFixture([{ name: 'alpha', triggers: ['alpha'] }], created); + const r = run(['--verbose', '--skills-dir', skillsDir]); + expect(r.stdout).toContain('Deferred:'); + expect(r.stdout).toContain('trigger_routing_eval'); + expect(r.stdout).toContain('brain_filing'); + }); + + it('clean fixture human output says all skills reachable', () => { + const skillsDir = makeFixture( + [ + { name: 'alpha', triggers: ['alpha'] }, + { name: 'beta', triggers: ['beta'] }, + ], + created, + ); + const r = run(['--skills-dir', skillsDir]); + expect(r.stdout).toContain('resolver_health: OK'); + expect(r.stdout).toContain('2 skills'); + expect(r.status).toBe(0); + }); +}); diff --git a/test/repo-root.test.ts b/test/repo-root.test.ts new file mode 100644 index 000000000..d97888538 --- /dev/null +++ b/test/repo-root.test.ts @@ -0,0 +1,53 @@ +import { describe, it, expect, afterEach } from 'bun:test'; +import { mkdtempSync, mkdirSync, writeFileSync, rmSync } from 'fs'; +import { tmpdir } from 'os'; +import { join } from 'path'; +import { findRepoRoot } from '../src/core/repo-root.ts'; + +describe('findRepoRoot', () => { + const created: string[] = []; + afterEach(() => { + while (created.length) { + const p = created.pop()!; + try { rmSync(p, { recursive: true, force: true }); } catch { /* ignore */ } + } + }); + + function scratch(): string { + const dir = mkdtempSync(join(tmpdir(), 'repo-root-')); + created.push(dir); + return dir; + } + + function seedRepo(dir: string): void { + mkdirSync(join(dir, 'skills'), { recursive: true }); + writeFileSync(join(dir, 'skills', 'RESOLVER.md'), '# RESOLVER\n'); + } + + it('finds skills/RESOLVER.md in the passed startDir on first iteration', () => { + const root = scratch(); + seedRepo(root); + expect(findRepoRoot(root)).toBe(root); + }); + + it('walks up N directories to find the repo root', () => { + const root = scratch(); + seedRepo(root); + const nested = join(root, 'a', 'b', 'c'); + mkdirSync(nested, { recursive: true }); + expect(findRepoRoot(nested)).toBe(root); + }); + + it('returns null when no skills/RESOLVER.md exists up to filesystem root', () => { + const empty = scratch(); + // Deliberately no seedRepo — empty dir; walk terminates at filesystem root. + expect(findRepoRoot(empty)).toBeNull(); + }); + + it('default arg uses process.cwd() (behavioral parity with prior doctor-private impl)', () => { + // The default arg must match calling with an explicit process.cwd(). + // Don't assert on the path contents — it varies between local checkouts + // and CI runners. What matters is parity: no-arg === cwd-arg. + expect(findRepoRoot()).toBe(findRepoRoot(process.cwd())); + }); +}); diff --git a/test/skillify-check.test.ts b/test/skillify-check.test.ts index 78a63bdcb..f2a228321 100644 --- a/test/skillify-check.test.ts +++ b/test/skillify-check.test.ts @@ -92,3 +92,75 @@ describe('skillify-check CLI', () => { expect(result.exitCode).toBe(1); }); }); + +// --------------------------------------------------------------------------- +// check-resolvable wiring (per plan-eng-review — no silent pass on missing binary) +// --------------------------------------------------------------------------- + +import { mkdtempSync, writeFileSync, chmodSync, rmSync } from 'fs'; +import { tmpdir } from 'os'; +import { spawnSync } from 'child_process'; +import { afterEach } from 'bun:test'; + +function runWithPath(opts: { path: string }): { stdout: string; stderr: string } { + // Use bun's absolute path so spawnSync doesn't need bun on the scoped PATH. + // PATH is what skillify-check's own `spawnSync('gbrain', ...)` will search. + const bunBin = process.execPath || 'bun'; + const res = spawnSync(bunBin, ['run', SCRIPT, '--json', 'src/commands/publish.ts'], { + encoding: 'utf-8', + cwd: REPO, + env: { ...process.env, PATH: opts.path }, + maxBuffer: 10 * 1024 * 1024, + }); + return { + stdout: res.stdout ?? '', + stderr: res.stderr ?? '', + }; +} + +describe('skillify-check ↔ gbrain check-resolvable wiring', () => { + const created: string[] = []; + afterEach(() => { + while (created.length) { + const p = created.pop()!; + try { rmSync(p, { recursive: true, force: true }); } catch { /* ignore */ } + } + }); + + test('loud failure when gbrain binary is not on PATH (no silent pass)', () => { + const emptyPath = mkdtempSync(join(tmpdir(), 'no-gbrain-')); + created.push(emptyPath); + const r = runWithPath({ path: emptyPath }); + // Loud warning must appear on stderr. + expect(r.stderr).toContain('gbrain check-resolvable not runnable'); + const parsed = JSON.parse(r.stdout); + const gate = parsed[0].items.find((i: any) => i.name === 'check-resolvable gate'); + expect(gate).toBeDefined(); + // Gate MUST NOT pass when the binary is unavailable. + expect(gate.passed).toBe(false); + expect(gate.detail).toContain('unavailable'); + }); + + test('happy path: synthetic gbrain on PATH returns ok=true, gate passes', () => { + const fakeBinDir = mkdtempSync(join(tmpdir(), 'fake-gbrain-')); + created.push(fakeBinDir); + const fakeBin = join(fakeBinDir, 'gbrain'); + writeFileSync( + fakeBin, + `#!/bin/sh +cat <<'JSON' +{"ok":true,"skillsDir":"/fake","report":{"ok":true,"issues":[],"summary":{"total_skills":0,"reachable":0,"unreachable":0,"overlaps":0,"gaps":0}},"autoFix":null,"deferred":[{"check":5,"name":"trigger_routing_eval","issue":""},{"check":6,"name":"brain_filing","issue":""}],"error":null,"message":null} +JSON +`, + ); + chmodSync(fakeBin, 0o755); + const r = runWithPath({ path: `${fakeBinDir}:${process.env.PATH}` }); + // No silent-pass warning. + expect(r.stderr).not.toContain('gbrain check-resolvable not runnable'); + const parsed = JSON.parse(r.stdout); + const gate = parsed[0].items.find((i: any) => i.name === 'check-resolvable gate'); + expect(gate).toBeDefined(); + expect(gate.passed).toBe(true); + expect(gate.detail).toContain('all skill-tree checks pass'); + }); +});