From 73237d175bfc255245ef30d2b0a50880b026149f Mon Sep 17 00:00:00 2001 From: Garry Tan Date: Sat, 18 Apr 2026 13:26:23 +0800 Subject: [PATCH] refactor(commands): extract library-level Core functions that throw not exit MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Codex architecture finding #5: reusing CLI entry-point functions as Minions handler bodies is wrong. If a Minion invokes runExtract / runEmbed / runBacklinks / runLint and the handler hits a process.exit(1), the ENTIRE WORKER process dies — killing every other in-flight job. Handlers need library-level APIs that throw, and the CLI stays a thin wrapper that catches + exits. Per-command shape: - runXxxCore(opts): throws on validation errors, returns structured result. Handler-safe. - runXxx(args): arg parser; calls Core; catches; process.exit(1) on thrown errors. CLI-safe. Shipped: - runExtractCore({ mode, dir, dryRun?, jsonMode? }) → ExtractResult - runEmbedCore({ slug? | slugs? | all? | stale? }) → void - runBacklinksCore({ action, dir, dryRun? }) → BacklinksResult - runLintCore({ target, fix?, dryRun? }) → LintResult sync.ts is already correct — performSync throws; runSync wraps. No change. import.ts deferred to v0.12.0 (its one process.exit fires only on a missing dir arg; handlers always pass a dir, so worker-kill risk is zero in practice). Noted in the plan's Out-of-scope. Smoke verified: all four Core functions throw on invalid mode / missing dir / not-found target instead of exiting the process. Co-Authored-By: Claude Opus 4.7 (1M context) --- src/commands/backlinks.ts | 64 +++++++++++++++++++++++----- src/commands/embed.ts | 59 +++++++++++++++++++++----- src/commands/extract.ts | 68 +++++++++++++++++++++++------- src/commands/lint.ts | 87 ++++++++++++++++++++++++++++++++------- 4 files changed, 227 insertions(+), 51 deletions(-) diff --git a/src/commands/backlinks.ts b/src/commands/backlinks.ts index ec9fcb395..1fe2ae02b 100644 --- a/src/commands/backlinks.ts +++ b/src/commands/backlinks.ts @@ -168,6 +168,43 @@ export function fixBacklinkGaps(brainDir: string, gaps: BacklinkGap[], dryRun: b return fixed; } +export interface BacklinksOpts { + action: 'check' | 'fix'; + dir: string; + dryRun?: boolean; +} + +export interface BacklinksResult { + action: 'check' | 'fix'; + gaps_found: number; + fixed: number; + pages_affected: number; + dryRun: boolean; +} + +/** + * Library-level backlinks check/fix. Throws on validation errors; returns a + * structured result so Minions handlers + autopilot-cycle can surface counts. + * Safe to call from the worker — no process.exit. + */ +export async function runBacklinksCore(opts: BacklinksOpts): Promise { + if (!['check', 'fix'].includes(opts.action)) { + throw new Error(`Invalid backlinks action "${opts.action}". Allowed: check, fix.`); + } + if (!existsSync(opts.dir)) { + throw new Error(`Directory not found: ${opts.dir}`); + } + + const gaps = findBacklinkGaps(opts.dir); + const pagesAffected = new Set(gaps.map(g => g.targetPage)).size; + + if (opts.action === 'fix' && gaps.length > 0) { + const fixed = fixBacklinkGaps(opts.dir, gaps, !!opts.dryRun); + return { action: 'fix', gaps_found: gaps.length, fixed, pages_affected: pagesAffected, dryRun: !!opts.dryRun }; + } + return { action: opts.action, gaps_found: gaps.length, fixed: 0, pages_affected: pagesAffected, dryRun: !!opts.dryRun }; +} + export async function runBacklinks(args: string[]) { const subcommand = args[0]; const dirIdx = args.indexOf('--dir'); @@ -183,19 +220,25 @@ export async function runBacklinks(args: string[]) { process.exit(1); } - if (!existsSync(brainDir)) { - console.error(`Directory not found: ${brainDir}`); + let result: BacklinksResult; + try { + result = await runBacklinksCore({ + action: subcommand as 'check' | 'fix', + dir: brainDir, + dryRun, + }); + } catch (e) { + console.error(e instanceof Error ? e.message : String(e)); process.exit(1); } - const gaps = findBacklinkGaps(brainDir); - - if (gaps.length === 0) { + if (result.gaps_found === 0) { console.log('No missing back-links found.'); return; } - - if (subcommand === 'check') { + if (result.action === 'check') { + // Re-walk for user-facing output (core returns counts, CLI shows detail). + const gaps = findBacklinkGaps(brainDir); console.log(`Found ${gaps.length} missing back-link(s):\n`); for (const gap of gaps) { console.log(` ${gap.targetPage} <- ${gap.sourcePage}`); @@ -203,10 +246,9 @@ export async function runBacklinks(args: string[]) { } console.log(`\nRun 'gbrain check-backlinks fix --dir ${brainDir}' to create them.`); } else { - const label = dryRun ? '(dry run) ' : ''; - const fixed = fixBacklinkGaps(brainDir, gaps, dryRun); - console.log(`${label}Fixed ${fixed} missing back-link(s) across ${new Set(gaps.map(g => g.targetPage)).size} page(s).`); - if (dryRun) { + const label = result.dryRun ? '(dry run) ' : ''; + console.log(`${label}Fixed ${result.fixed} missing back-link(s) across ${result.pages_affected} page(s).`); + if (result.dryRun) { console.log('\nRe-run without --dry-run to apply.'); } } diff --git a/src/commands/embed.ts b/src/commands/embed.ts index 2a28a5630..cf6de5839 100644 --- a/src/commands/embed.ts +++ b/src/commands/embed.ts @@ -3,29 +3,66 @@ import { embedBatch } from '../core/embedding.ts'; import type { ChunkInput } from '../core/types.ts'; import { chunkText } from '../core/chunkers/recursive.ts'; +export interface EmbedOpts { + /** Embed ALL pages (every chunk). */ + all?: boolean; + /** Embed only stale chunks (missing embedding). */ + stale?: boolean; + /** Embed specific pages by slug. */ + slugs?: string[]; + /** Embed a single page. */ + slug?: string; +} + +/** + * Library-level embed. Throws on validation errors; per-page embed failures + * are logged to stderr but do not throw (matches the existing CLI semantics + * for batch runs). Safe to call from Minions handlers — no process.exit. + */ +export async function runEmbedCore(engine: BrainEngine, opts: EmbedOpts): Promise { + if (opts.slugs && opts.slugs.length > 0) { + for (const s of opts.slugs) { + try { await embedPage(engine, s); } catch (e: unknown) { + console.error(` Error embedding ${s}: ${e instanceof Error ? e.message : e}`); + } + } + return; + } + if (opts.all || opts.stale) { + await embedAll(engine, !!opts.stale); + return; + } + if (opts.slug) { + await embedPage(engine, opts.slug); + return; + } + throw new Error('No embed target specified. Pass { slug }, { slugs }, { all }, or { stale }.'); +} + export async function runEmbed(engine: BrainEngine, args: string[]) { const slugsIdx = args.indexOf('--slugs'); const all = args.includes('--all'); const stale = args.includes('--stale'); + let opts: EmbedOpts; if (slugsIdx >= 0) { - // --slugs slug1 slug2 ... (embed specific pages) - const slugs = args.slice(slugsIdx + 1).filter(a => !a.startsWith('--')); - for (const s of slugs) { - try { await embedPage(engine, s); } catch (e: unknown) { - console.error(` Error embedding ${s}: ${e instanceof Error ? e.message : e}`); - } - } + opts = { slugs: args.slice(slugsIdx + 1).filter(a => !a.startsWith('--')) }; } else if (all || stale) { - await embedAll(engine, stale); + opts = { all, stale }; } else { const slug = args.find(a => !a.startsWith('--')); - if (slug) { - await embedPage(engine, slug); - } else { + if (!slug) { console.error('Usage: gbrain embed [|--all|--stale|--slugs s1 s2 ...]'); process.exit(1); } + opts = { slug }; + } + + try { + await runEmbedCore(engine, opts); + } catch (e) { + console.error(e instanceof Error ? e.message : String(e)); + process.exit(1); } } diff --git a/src/commands/extract.ts b/src/commands/extract.ts index 442a0de7f..e7db8a55c 100644 --- a/src/commands/extract.ts +++ b/src/commands/extract.ts @@ -174,6 +174,49 @@ export function extractTimelineFromContent(content: string, slug: string): Extra // --- Main command --- +export interface ExtractOpts { + /** What to extract: 'links' (wiki-style refs), 'timeline' (date entries), or 'all'. */ + mode: 'links' | 'timeline' | 'all'; + /** Brain directory to walk. */ + dir: string; + /** Report what would change without writing. */ + dryRun?: boolean; + /** Emit JSON (progress to stderr, result to stdout) instead of human text. */ + jsonMode?: boolean; +} + +/** + * Library-level extract. Throws on error; prints nothing unless jsonMode or + * explicit output is warranted. Safe to call from Minions handlers because it + * never calls process.exit — a bad mode or missing dir throws through, which + * the handler wrapper turns into a failed job (NOT a killed worker). + */ +export async function runExtractCore(engine: BrainEngine, opts: ExtractOpts): Promise { + if (!['links', 'timeline', 'all'].includes(opts.mode)) { + throw new Error(`Invalid extract mode "${opts.mode}". Allowed: links, timeline, all.`); + } + if (!existsSync(opts.dir)) { + throw new Error(`Directory not found: ${opts.dir}`); + } + + const dryRun = !!opts.dryRun; + const jsonMode = !!opts.jsonMode; + const result: ExtractResult = { links_created: 0, timeline_entries_created: 0, pages_processed: 0 }; + + if (opts.mode === 'links' || opts.mode === 'all') { + const r = await extractLinksFromDir(engine, opts.dir, dryRun, jsonMode); + result.links_created = r.created; + result.pages_processed = r.pages; + } + if (opts.mode === 'timeline' || opts.mode === 'all') { + const r = await extractTimelineFromDir(engine, opts.dir, dryRun, jsonMode); + result.timeline_entries_created = r.created; + result.pages_processed = Math.max(result.pages_processed, r.pages); + } + + return result; +} + export async function runExtract(engine: BrainEngine, args: string[]) { const subcommand = args[0]; const dirIdx = args.indexOf('--dir'); @@ -186,24 +229,19 @@ export async function runExtract(engine: BrainEngine, args: string[]) { process.exit(1); } - if (!existsSync(brainDir)) { - console.error(`Directory not found: ${brainDir}`); + let result: ExtractResult; + try { + result = await runExtractCore(engine, { + mode: subcommand as 'links' | 'timeline' | 'all', + dir: brainDir, + dryRun, + jsonMode, + }); + } catch (e) { + console.error(e instanceof Error ? e.message : String(e)); process.exit(1); } - const result: ExtractResult = { links_created: 0, timeline_entries_created: 0, pages_processed: 0 }; - - if (subcommand === 'links' || subcommand === 'all') { - const r = await extractLinksFromDir(engine, brainDir, dryRun, jsonMode); - result.links_created = r.created; - result.pages_processed = r.pages; - } - if (subcommand === 'timeline' || subcommand === 'all') { - const r = await extractTimelineFromDir(engine, brainDir, dryRun, jsonMode); - result.timeline_entries_created = r.created; - result.pages_processed = Math.max(result.pages_processed, r.pages); - } - if (jsonMode) { console.log(JSON.stringify(result, null, 2)); } else if (!dryRun) { diff --git a/src/commands/lint.ts b/src/commands/lint.ts index 3a509066f..1c80a420f 100644 --- a/src/commands/lint.ts +++ b/src/commands/lint.ts @@ -181,6 +181,71 @@ function collectPages(dir: string): string[] { return pages.sort(); } +export interface LintOpts { + target: string; + fix?: boolean; + dryRun?: boolean; +} + +export interface LintResult { + pages_scanned: number; + pages_with_issues: number; + total_issues: number; + total_fixed: number; + dryRun: boolean; + applied_fix: boolean; +} + +/** + * Library-level lint. Throws on validation errors (missing target, target + * not found); lints otherwise. Does NOT print human-readable details (the + * CLI wrapper handles that) — returns counts so Minions handlers can + * report structured results. Safe from the worker — no process.exit. + */ +export async function runLintCore(opts: LintOpts): Promise { + if (!opts.target) { + throw new Error('lint: target (dir|file.md) required'); + } + if (!existsSync(opts.target)) { + throw new Error(`Not found: ${opts.target}`); + } + + const isSingleFile = statSync(opts.target).isFile(); + const pages = isSingleFile ? [opts.target] : collectPages(opts.target); + + let totalIssues = 0; + let totalFixed = 0; + let pagesWithIssues = 0; + + for (const page of pages) { + const content = readFileSync(page, 'utf-8'); + const issues = lintContent(content, isSingleFile ? page : relative(opts.target, page)); + if (issues.length === 0) continue; + pagesWithIssues++; + totalIssues += issues.length; + + if (opts.fix && issues.some(i => i.fixable)) { + const fixed = fixContent(content); + if (fixed !== content) { + const fixCount = issues.filter(i => i.fixable).length; + totalFixed += fixCount; + if (!opts.dryRun) { + writeFileSync(page, fixed); + } + } + } + } + + return { + pages_scanned: pages.length, + pages_with_issues: pagesWithIssues, + total_issues: totalIssues, + total_fixed: totalFixed, + dryRun: !!opts.dryRun, + applied_fix: !!opts.fix, + }; +} + export async function runLint(args: string[]) { const target = args.find(a => !a.startsWith('--')); const doFix = args.includes('--fix'); @@ -198,22 +263,16 @@ export async function runLint(args: string[]) { process.exit(1); } - // Single file or directory + // Single file or directory — print human detail as we go, then rely on + // Core for the aggregate numbers at the end. const isSingleFile = statSync(target).isFile(); const pages = isSingleFile ? [target] : collectPages(target); - let totalIssues = 0; - let totalFixed = 0; - let pagesWithIssues = 0; - for (const page of pages) { const content = readFileSync(page, 'utf-8'); const relPath = isSingleFile ? page : relative(target, page); const issues = lintContent(content, relPath); - if (issues.length === 0) continue; - pagesWithIssues++; - totalIssues += issues.length; console.log(`\n${relPath}:`); for (const issue of issues) { @@ -221,12 +280,10 @@ export async function runLint(args: string[]) { console.log(` L${issue.line} ${issue.rule}: ${issue.message}${fixLabel}`); } - // Auto-fix if requested if (doFix && issues.some(i => i.fixable)) { const fixed = fixContent(content); if (fixed !== content) { const fixCount = issues.filter(i => i.fixable).length; - totalFixed += fixCount; if (!dryRun) { writeFileSync(page, fixed); } @@ -235,11 +292,13 @@ export async function runLint(args: string[]) { } } - console.log(`\n${pages.length} pages scanned. ${totalIssues} issue(s) in ${pagesWithIssues} page(s).`); + // Re-run core for the aggregate counts (cheap; re-parses contents but + // produces canonical numbers for the summary line). + const result = await runLintCore({ target, fix: doFix, dryRun }); + console.log(`\n${result.pages_scanned} pages scanned. ${result.total_issues} issue(s) in ${result.pages_with_issues} page(s).`); if (doFix) { - console.log(`${dryRun ? '(dry run) ' : ''}${totalFixed} auto-fixed.`); - } else if (totalIssues > 0) { - const fixable = totalIssues; // rough estimate + console.log(`${dryRun ? '(dry run) ' : ''}${result.total_fixed} auto-fixed.`); + } else if (result.total_issues > 0) { console.log(`Run with --fix to auto-fix fixable issues.`); } }