From 34311be2b1056cdf852507843609bd018485d2fa Mon Sep 17 00:00:00 2001 From: James Ross Date: Thu, 9 Apr 2026 18:49:11 -0700 Subject: [PATCH 1/5] feat: add doctor command --- ARCHITECTURE.md | 4 +- README.md | 11 +- docs/CLI.md | 4 + docs/MCP.md | 4 + .../0038-doctor-command/doctor-command.md | 84 ++++ .../backlog/inbox/PROCESS_doctor-command.md | 9 - src/cli-args.ts | 27 +- src/cli.ts | 34 +- src/doctor.ts | 467 ++++++++++++++++++ src/domain.ts | 44 ++ src/mcp.ts | 19 + tests/cli.test.ts | 45 ++ tests/doctor.test.ts | 101 ++++ tests/mcp.test.ts | 29 ++ 14 files changed, 844 insertions(+), 38 deletions(-) create mode 100644 docs/design/0038-doctor-command/doctor-command.md delete mode 100644 docs/method/backlog/inbox/PROCESS_doctor-command.md create mode 100644 src/doctor.ts create mode 100644 tests/doctor.test.ts diff --git a/ARCHITECTURE.md b/ARCHITECTURE.md index a4d6452..e1770eb 100644 --- a/ARCHITECTURE.md +++ b/ARCHITECTURE.md @@ -23,6 +23,7 @@ src/ cli-renderer.ts cli.ts config.ts + doctor.ts domain.ts drift.ts errors.ts @@ -104,12 +105,13 @@ Two-way sync between the local backlog and GitHub Issues: ## Testing -16 test files in `tests/` using Vitest: +17 test files in `tests/` using Vitest: - `api.test.ts` - `cli.test.ts` - `config.test.ts` - `docs.test.ts` +- `doctor.test.ts` - `domain.test.ts` - `drift.test.ts` - `exec.test.ts` diff --git a/README.md b/README.md index 91b892f..e7c0c5a 100644 --- a/README.md +++ b/README.md @@ -370,6 +370,7 @@ If you can answer these questions by reading the repo, you do not need a standup: - What is actively open in this workspace? -> `method status` +- Is this workspace structurally healthy? -> `method doctor` - What is under review? -> `method review-state` - What is committed? -> each design doc names its sponsors and hill - What is next? -> `ls docs/method/backlog/asap/` @@ -480,14 +481,14 @@ Run during development: npm run method -- status ``` -Core commands: `method init`, `method inbox`, `method pull`, -`method close`, `method drift`, `method status`, `method mcp`, -`method sync github`, `method sync ship`. +Core commands: `method init`, `method doctor`, `method inbox`, +`method pull`, `method close`, `method drift`, `method status`, +`method mcp`, `method sync github`, `method sync ship`. See `docs/CLI.md` for the full command reference with flags and examples. See `docs/MCP.md` for the MCP tool reference -(`method_status`, `method_inbox`, `method_pull`, `method_drift`, -`method_close`, `method_sync_ship`, `method_sync_github`, +(`method_doctor`, `method_status`, `method_inbox`, `method_pull`, +`method_drift`, `method_close`, `method_sync_ship`, `method_sync_github`, `method_capture_witness`). Repo-local CI currently uses GitHub Actions as a host adapter through diff --git a/docs/CLI.md b/docs/CLI.md index 6f74ad5..d96ebc8 100644 --- a/docs/CLI.md +++ b/docs/CLI.md @@ -19,6 +19,10 @@ operations. Run `method help` for a quick summary or Scaffold a METHOD workspace in the given directory. +### `method doctor [--json]` + +Inspect METHOD workspace health without mutating it. + ### `method inbox [--legend CODE] [--title TITLE]` Capture a raw idea in docs/method/backlog/inbox/. diff --git a/docs/MCP.md b/docs/MCP.md index 967985d..0df047e 100644 --- a/docs/MCP.md +++ b/docs/MCP.md @@ -21,6 +21,10 @@ workspace-agnostic — a single instance can serve multiple projects. ## Tools +### `method_doctor` + +Inspect METHOD workspace health and report concrete problems with suggested fixes, even when the workspace is partially broken. + ### `method_review_state` Get PR review / merge-readiness state for the current branch or an explicit PR. `pr` and `currentBranch` are mutually exclusive; when `pr` is omitted, current-branch resolution is the default behavior. diff --git a/docs/design/0038-doctor-command/doctor-command.md b/docs/design/0038-doctor-command/doctor-command.md new file mode 100644 index 0000000..aa5e7ea --- /dev/null +++ b/docs/design/0038-doctor-command/doctor-command.md @@ -0,0 +1,84 @@ +--- +title: "Doctor Command" +legend: "PROCESS" +cycle: "0038-doctor-command" +source_backlog: "docs/method/backlog/inbox/PROCESS_doctor-command.md" +--- + +# Doctor Command + +Source backlog item: `docs/method/backlog/inbox/PROCESS_doctor-command.md` +Legend: PROCESS + +## Sponsors + +- Human: Backlog operator +- Agent: Implementation agent + +## Hill + +Land a bounded `doctor` surface that inspects METHOD workspace health +without requiring a fully healthy `Workspace` instance, reports concrete +problems with suggested fixes, and is callable from both the CLI and +MCP. + +## Playback Questions + +### Human + +- [ ] Does `method doctor --json` return a bounded workspace health + report without throwing on malformed `.method.json`? +- [ ] Does `method doctor` surface missing required paths and malformed + frontmatter with fix suggestions? + +### Agent + +- [ ] Does `method_doctor` return the same doctor report contract under + `structuredContent.result`? +- [ ] Does `runDoctor()` report orphaned backlog items and git-hook + diagnostics without requiring a healthy `Workspace` instance? + +## Accessibility and Assistive Reading + +- Linear truth / reduced-complexity posture: the text output must be a + short check list plus issue list, so operators can read it top to + bottom without reconstructing hidden state. +- Non-visual or alternate-reading expectations: each issue must carry a + direct fix suggestion instead of relying on color or layout cues. + +## Localization and Directionality + +- Locale / wording / formatting assumptions: command output remains + ASCII-first and stable for script consumption; file paths stay repo- + relative where possible. +- Logical direction / layout assumptions: no UI layout work is in scope; + this cycle is about deterministic text output and structured results. + +## Agent Inspectability and Explainability + +- What must be explicit and deterministic for agents: the check names, + issue severities, and suggested fixes must be stable enough for MCP + callers to branch on. +- What must be attributable, evidenced, or governed: each reported issue + must identify the path or subsystem being diagnosed so agents can act + without guessing. + +## Non-goals + +- [ ] Building the broader `method validate` command family in this + cycle. +- [ ] Auto-repairing broken workspaces; `doctor` diagnoses and suggests, + but does not mutate. +- [ ] Enforcing repo-level legend coverage rules beyond bounded orphaned + backlog detection. + +## Backlog Context + +Add a method doctor command that checks workspace health: required +directories and core files exist, METHOD packet frontmatter is present +and parseable, `.method.json` parses cleanly, git hooks are configured, +and backlog cards have not escaped the recognized lane structure. + +This should stay bounded. It is not the future `validate` family, and +it should not require a healthy `Workspace` object before it can tell +you why the workspace is unhealthy. diff --git a/docs/method/backlog/inbox/PROCESS_doctor-command.md b/docs/method/backlog/inbox/PROCESS_doctor-command.md deleted file mode 100644 index faa3211..0000000 --- a/docs/method/backlog/inbox/PROCESS_doctor-command.md +++ /dev/null @@ -1,9 +0,0 @@ ---- -title: "Doctor Command" -legend: PROCESS -lane: inbox ---- - -# Doctor Command - -Add a method doctor command that checks workspace health: required directories exist, frontmatter is valid, .method.json parses cleanly, git hooks are configured, no orphaned backlog items. Should report what is wrong and suggest fixes. diff --git a/src/cli-args.ts b/src/cli-args.ts index 7f54086..77fd518 100644 --- a/src/cli-args.ts +++ b/src/cli-args.ts @@ -4,6 +4,7 @@ import { MethodError } from './errors.js'; export type ParsedCommand = | { command: 'help'; topic?: string } | { command: 'init'; path: string } + | { command: 'doctor'; json?: boolean } | { command: 'inbox'; idea: string; legend?: string; title?: string } | { command: 'pull'; item: string } | { command: 'close'; cycle?: string; driftCheck?: 'yes' | 'no'; outcome: Outcome } @@ -34,6 +35,8 @@ export function parseCliArgs(argv: readonly string[]): ParsedCommand { switch (command) { case 'init': return parseInitArgs(rest); + case 'doctor': + return parseDoctorArgs(rest); case 'inbox': return parseInboxArgs(rest); case 'pull': @@ -80,12 +83,19 @@ export function parseCliArgs(argv: readonly string[]): ParsedCommand { } } -export const CLI_TOPICS = ['init', 'inbox', 'pull', 'close', 'status', 'drift', 'review-state', 'mcp', 'sync'] as const; +export const CLI_TOPICS = ['init', 'doctor', 'inbox', 'pull', 'close', 'status', 'drift', 'review-state', 'mcp', 'sync'] as const; export function usage(topic?: string): string { if (topic === 'init') { return 'Usage: method init [path]\n\nScaffold a METHOD workspace in the given directory.'; } + if (topic === 'doctor') { + return [ + 'Usage: method doctor [--json]', + '', + 'Inspect METHOD workspace health without mutating it.', + ].join('\n'); + } if (topic === 'inbox') { return 'Usage: method inbox [--legend CODE] [--title TITLE]\n\nCapture a raw idea in docs/method/backlog/inbox/.'; } @@ -137,6 +147,7 @@ export function usage(topic?: string): string { '', 'Commands:', ' init [path] Scaffold a METHOD workspace.', + ' doctor [--json] Inspect workspace health and suggest fixes.', ' inbox Capture a raw idea in inbox/.', ' pull Promote a backlog item into a cycle.', ' close [cycle] Write a retro for an active cycle.', @@ -158,6 +169,20 @@ function parseInitArgs(args: readonly string[]): ParsedCommand { return { command: 'init', path: args[0] ?? '.' }; } +function parseDoctorArgs(args: readonly string[]): ParsedCommand { + let json = false; + + for (const value of args) { + if (value === '--json') { + json = true; + continue; + } + throw new MethodError(`Unknown option: ${value}`); + } + + return { command: 'doctor', json }; +} + function parseInboxArgs(args: readonly string[]): ParsedCommand { let legend: string | undefined; let title: string | undefined; diff --git a/src/cli.ts b/src/cli.ts index 1eb1931..5680759 100644 --- a/src/cli.ts +++ b/src/cli.ts @@ -11,6 +11,7 @@ import { initWorkspace, Workspace } from './index.js'; import { loadConfig } from './config.js'; import { createMcpServer } from './mcp.js'; import { GitHubAdapter } from './adapters/github.js'; +import { renderDoctorText, runDoctor } from './doctor.js'; import { queryReviewState, renderReviewStateText, type ReviewStateQueryOptions, type ReviewStateResult } from './review-state.js'; type Writer = Pick; type ConfirmPrompt = (options: { title: string; defaultValue: boolean }) => Promise; @@ -37,7 +38,6 @@ export async function runCli( stdout.write(`${usage(parsed.topic)}\n`); return 0; } - if (parsed.command === 'init') { const target = resolve(root, parsed.path); const config = loadConfig(target); @@ -48,23 +48,28 @@ export async function runCli( } return 0; } - + if (parsed.command === 'doctor') { + const report = runDoctor(root); + if (parsed.json) { + stdout.write(`${JSON.stringify(report, null, 2)}\n`); + } else { + stdout.write(renderDoctorText(report)); + } + return report.status === 'error' ? 1 : 0; + } const workspace = new Workspace(root); workspace.ensureInitialized(); - if (parsed.command === 'inbox') { const created = workspace.captureIdea(parsed.idea, parsed.legend, parsed.title); stdout.write(`${alert(`Captured ${relative(root, created)}`, { variant: 'success', ctx })}\n`); return 0; } - if (parsed.command === 'pull') { const cycle = workspace.pullItem(parsed.item); stdout.write(`${alert(`Pulled into ${cycle.name}`, { variant: 'success', ctx })}\n`); stdout.write(`${relative(root, cycle.designDoc)}\n`); return 0; } - if (parsed.command === 'close') { const completedDriftCheck = parsed.driftCheck === undefined ? await promptConfirm({ title: 'Drift check complete?', defaultValue: false }) @@ -77,13 +82,11 @@ export async function runCli( stdout.write(`${relative(root, cycle.retroDoc)}\n`); return 0; } - if (parsed.command === 'drift') { const report = workspace.detectDrift(parsed.cycle); stdout.write(report.output); return report.exitCode; } - if (parsed.command === 'review-state') { const reviewStateQuery = options.reviewStateQuery ?? queryReviewState; const result = await reviewStateQuery({ @@ -98,7 +101,6 @@ export async function runCli( } return 0; } - if (parsed.command === 'mcp') { const server = createMcpServer({ reviewStateQuery: options.reviewStateQuery ?? queryReviewState }); const transport = new StdioServerTransport(); @@ -106,12 +108,10 @@ export async function runCli( // Let it run indefinitely return new Promise(() => {}); } - if (parsed.command === 'sync') { if (parsed.adapter === 'github') { const token = workspace.config.github_token; const repoFull = workspace.config.github_repo; - if (!token) { throw new Error('GitHub token is required for sync. Provide it via GITHUB_TOKEN or .method.json.'); } @@ -126,12 +126,10 @@ export async function runCli( owner: owner!, repo: repo!, }); - if (!parsed.push && !parsed.pull) { stderr.write(`${alert('No sync direction specified. Use --push and/or --pull.', { variant: 'error', ctx })}\n`); return 1; } - if (parsed.push) { const results = await adapter.pushBacklog(); for (const result of results) { @@ -145,7 +143,6 @@ export async function runCli( } } } - if (parsed.pull) { const results = await adapter.pullBacklog(); for (const result of results) { @@ -160,7 +157,6 @@ export async function runCli( } return 0; } - if (parsed.adapter === 'ship') { const result = await workspace.shipSync(); for (const path of result.updated) { @@ -184,11 +180,5 @@ export async function runCli( return 1; } } -export async function main(argv: readonly string[] = process.argv.slice(2)): Promise { - return runCli(argv); -} -if (process.argv[1] !== undefined && fileURLToPath(import.meta.url) === realpathSync(resolve(process.argv[1]))) { - main().then((code) => { - process.exitCode = code; - }); -} +export async function main(argv: readonly string[] = process.argv.slice(2)): Promise { return runCli(argv); } +if (process.argv[1] !== undefined && fileURLToPath(import.meta.url) === realpathSync(resolve(process.argv[1]))) { main().then((code) => { process.exitCode = code; }); } diff --git a/src/doctor.ts b/src/doctor.ts new file mode 100644 index 0000000..03f4946 --- /dev/null +++ b/src/doctor.ts @@ -0,0 +1,467 @@ +import { execFileSync } from 'node:child_process'; +import { existsSync, readdirSync, readFileSync, statSync } from 'node:fs'; +import { basename, relative, resolve } from 'node:path'; +import { headerBox, separator } from '@flyingrobots/bijou'; +import { createNodeContext } from '@flyingrobots/bijou-node'; +import { ConfigSchema, DEFAULT_PATHS, PathsSchema, type PathsConfig } from './config.js'; +import type { DoctorCheck, DoctorCheckId, DoctorIssue, DoctorReport, DoctorSeverity, DoctorStatus } from './domain.js'; +import { LANES } from './domain.js'; +import { parse as parseYaml } from 'yaml'; + +interface ConfigInspection { + paths: PathsConfig; + issues: DoctorIssue[]; +} + +const DOCTOR_CHECKS: readonly DoctorCheckId[] = [ + 'config', + 'structure', + 'frontmatter', + 'git-hooks', + 'backlog', +] as const; + +export function runDoctor(root: string): DoctorReport { + const configInspection = inspectConfig(root); + const structureIssues = inspectStructure(root, configInspection.paths); + const frontmatterIssues = inspectFrontmatter(root, configInspection.paths); + const gitHookIssues = inspectGitHooks(root); + const backlogIssues = inspectBacklog(root, configInspection.paths); + + const issues = [ + ...configInspection.issues, + ...structureIssues, + ...frontmatterIssues, + ...gitHookIssues, + ...backlogIssues, + ]; + + const checks = DOCTOR_CHECKS.map((id) => summarizeCheck(id, issues)); + const counts = { + errors: issues.filter((issue) => issue.severity === 'error').length, + warnings: issues.filter((issue) => issue.severity === 'warning').length, + }; + + return { + root, + status: summarizeStatus(counts), + checks, + issues, + counts, + }; +} + +export function renderDoctorText(report: DoctorReport): string { + const ctx = createNodeContext(); + const checkLines = report.checks.map((check) => + `${check.id.padEnd(12, ' ')} ${check.status.padEnd(5, ' ')} ${check.message}`, + ); + + const issueLines = report.issues.length === 0 + ? ['No issues found.'] + : report.issues.flatMap((issue) => { + const header = `- [${issue.severity}] ${issue.code}: ${issue.message}`; + const pathLine = issue.path === undefined ? [] : [` Path: ${issue.path}`]; + return [header, ...pathLine, ` Fix: ${issue.fix}`]; + }); + + return [ + `${headerBox('METHOD Doctor', { detail: report.root, ctx })}`, + '', + `Status: ${report.status} (${report.counts.errors} errors, ${report.counts.warnings} warnings)`, + '', + `${separator({ label: 'Checks', ctx })}`, + ...checkLines, + '', + `${separator({ label: 'Issues', ctx })}`, + ...issueLines, + '', + ].join('\n'); +} + +function inspectConfig(root: string): ConfigInspection { + const configPath = resolve(root, '.method.json'); + if (!existsSync(configPath)) { + return { paths: DEFAULT_PATHS, issues: [] }; + } + + let rawConfig: unknown; + try { + rawConfig = JSON.parse(readFileSync(configPath, 'utf8')); + } catch (error: unknown) { + const message = error instanceof Error ? error.message : String(error); + return { + paths: DEFAULT_PATHS, + issues: [ + createIssue( + 'config-parse-failed', + 'config', + 'error', + 'The `.method.json` file does not parse as valid JSON.', + relative(root, configPath), + `Fix the JSON syntax in \`${relative(root, configPath)}\`: ${message}`, + ), + ], + }; + } + + const parsedConfig = ConfigSchema.safeParse(rawConfig); + if (parsedConfig.success) { + return { paths: parsedConfig.data.paths, issues: [] }; + } + + const parsedPaths = PathsSchema.safeParse( + rawConfig !== null && typeof rawConfig === 'object' && 'paths' in rawConfig + ? (rawConfig as { paths?: unknown }).paths + : undefined, + ); + + return { + paths: parsedPaths.success ? parsedPaths.data : DEFAULT_PATHS, + issues: [ + createIssue( + 'config-invalid', + 'config', + 'error', + 'The `.method.json` file parses, but it does not satisfy METHOD configuration rules.', + relative(root, configPath), + `Fix the invalid configuration fields in \`${relative(root, configPath)}\`: ${parsedConfig.error.message}`, + ), + ], + }; +} + +function inspectStructure(root: string, paths: PathsConfig): DoctorIssue[] { + const requiredDirectories = [ + resolve(root, paths.backlog), + resolve(root, paths.method_dir, 'legends'), + resolve(root, paths.graveyard), + resolve(root, paths.method_dir, 'releases'), + resolve(root, paths.retro), + resolve(root, 'docs/releases'), + resolve(root, paths.design), + ]; + const requiredFiles = [ + resolve(root, 'CHANGELOG.md'), + resolve(root, paths.method_dir, 'process.md'), + resolve(root, paths.method_dir, 'release.md'), + resolve(root, paths.method_dir, 'release-runbook.md'), + resolve(root, paths.method_dir, 'releases/README.md'), + resolve(root, 'docs/releases/README.md'), + ]; + + const issues: DoctorIssue[] = []; + + for (const directory of requiredDirectories) { + if (existsSync(directory)) { + continue; + } + issues.push(createIssue( + 'missing-directory', + 'structure', + 'error', + 'A required METHOD directory is missing.', + relative(root, directory), + `Run \`method init\` in \`${root}\` or recreate \`${relative(root, directory)}\`.`, + )); + } + + for (const file of requiredFiles) { + if (existsSync(file)) { + continue; + } + issues.push(createIssue( + 'missing-file', + 'structure', + 'error', + 'A required METHOD file is missing.', + relative(root, file), + `Run \`method init\` in \`${root}\` or restore \`${relative(root, file)}\`.`, + )); + } + + return issues; +} + +function inspectFrontmatter(root: string, paths: PathsConfig): DoctorIssue[] { + const issues: DoctorIssue[] = []; + const packetRoots = [ + resolve(root, paths.backlog), + resolve(root, paths.design), + resolve(root, paths.retro), + resolve(root, paths.graveyard), + ]; + + for (const packetRoot of packetRoots) { + if (!existsSync(packetRoot)) { + continue; + } + + for (const file of collectMarkdownFiles(packetRoot)) { + const relativePath = relative(root, file); + const raw = readFileSync(file, 'utf8'); + if (!raw.startsWith('---\n')) { + issues.push(createIssue( + 'missing-frontmatter', + 'frontmatter', + 'error', + 'METHOD packet markdown is missing a YAML frontmatter block.', + relativePath, + `Add a top-of-file \`---\` frontmatter block to \`${relativePath}\`.`, + )); + continue; + } + + const end = raw.indexOf('\n---\n', 4); + if (end === -1) { + issues.push(createIssue( + 'unterminated-frontmatter', + 'frontmatter', + 'error', + 'YAML frontmatter is missing the closing delimiter.', + relativePath, + `Close the frontmatter block in \`${relativePath}\` with a terminating \`---\` line.`, + )); + continue; + } + + const yamlBlock = raw.slice(4, end); + try { + const parsed = parseYaml(yamlBlock); + if (parsed === null || typeof parsed !== 'object' || Array.isArray(parsed)) { + issues.push(createIssue( + 'frontmatter-not-mapping', + 'frontmatter', + 'error', + 'Frontmatter must parse to a YAML mapping/object.', + relativePath, + `Rewrite the frontmatter in \`${relativePath}\` as key/value pairs.`, + )); + } + } catch (error: unknown) { + const message = error instanceof Error ? error.message : String(error); + issues.push(createIssue( + 'frontmatter-parse-failed', + 'frontmatter', + 'error', + 'YAML frontmatter does not parse cleanly.', + relativePath, + `Fix the YAML syntax in \`${relativePath}\`: ${message}`, + )); + } + } + } + + return issues; +} + +function inspectGitHooks(root: string): DoctorIssue[] { + try { + const configuredHooksPath = runGitOptional(['config', '--get', 'core.hooksPath'], root); + if (configuredHooksPath.length > 0) { + const hookDir = resolve(root, configuredHooksPath); + if (!existsSync(hookDir)) { + return [ + createIssue( + 'git-hooks-missing', + 'git-hooks', + 'warning', + 'Git hooks are configured, but the configured hooks directory does not exist.', + relative(root, hookDir), + `Create \`${relative(root, hookDir)}\` or update \`core.hooksPath\` to a valid hooks directory.`, + ), + ]; + } + + if (hasInstalledHook(hookDir)) { + return []; + } + + return [ + createIssue( + 'git-hooks-empty', + 'git-hooks', + 'warning', + 'Git hooks are configured, but no executable hook files are installed there.', + relative(root, hookDir), + `Add an executable hook under \`${relative(root, hookDir)}\` or remove the unused hook configuration.`, + ), + ]; + } + + const defaultHooksPath = runGit(['rev-parse', '--git-path', 'hooks'], root); + const hookDir = resolve(root, defaultHooksPath); + if (hasInstalledHook(hookDir)) { + return []; + } + + return [ + createIssue( + 'git-hooks-not-configured', + 'git-hooks', + 'warning', + 'No configured git hooks were found for this repository.', + relative(root, hookDir), + 'Configure `core.hooksPath` or install at least one executable hook under `.git/hooks/`.', + ), + ]; + } catch { + return [ + createIssue( + 'git-hooks-unavailable', + 'git-hooks', + 'warning', + 'Git repository metadata is unavailable, so hook configuration could not be inspected.', + undefined, + 'Run `method doctor` from inside a git repository if hook diagnostics are required.', + ), + ]; + } +} + +function inspectBacklog(root: string, paths: PathsConfig): DoctorIssue[] { + const backlogRoot = resolve(root, paths.backlog); + if (!existsSync(backlogRoot)) { + return []; + } + + const issues: DoctorIssue[] = []; + for (const file of collectMarkdownFiles(backlogRoot)) { + const relativeToBacklog = relative(backlogRoot, file); + const [firstSegment] = relativeToBacklog.split(/[\\/]/u); + if (firstSegment !== undefined && (LANES as readonly string[]).includes(firstSegment)) { + continue; + } + + const relativePath = relative(root, file); + issues.push(createIssue( + 'orphaned-backlog-item', + 'backlog', + 'warning', + 'Backlog markdown exists outside the recognized lane directories.', + relativePath, + `Move \`${relativePath}\` into \`${paths.backlog}//\` or retire it into \`${paths.graveyard}\`.`, + )); + } + + return issues; +} + +function summarizeCheck(id: DoctorCheckId, issues: readonly DoctorIssue[]): DoctorCheck { + const matching = issues.filter((issue) => issue.check === id); + const errors = matching.filter((issue) => issue.severity === 'error').length; + const warnings = matching.filter((issue) => issue.severity === 'warning').length; + + if (errors > 0) { + return { + id, + status: 'error', + message: `${errors} error(s), ${warnings} warning(s)`, + }; + } + + if (warnings > 0) { + return { + id, + status: 'warn', + message: `${warnings} warning(s)`, + }; + } + + return { + id, + status: 'ok', + message: 'No issues found.', + }; +} + +function summarizeStatus(counts: { errors: number; warnings: number }): DoctorStatus { + if (counts.errors > 0) { + return 'error'; + } + if (counts.warnings > 0) { + return 'warn'; + } + return 'ok'; +} + +function createIssue( + code: string, + check: DoctorCheckId, + severity: DoctorSeverity, + message: string, + path: string | undefined, + fix: string, +): DoctorIssue { + return { + code, + check, + severity, + message, + path, + fix, + }; +} + +function collectMarkdownFiles(root: string, maxDepth = 10): string[] { + if (maxDepth <= 0 || !existsSync(root)) { + return []; + } + + const files: string[] = []; + for (const entry of readdirSync(root, { withFileTypes: true })) { + const path = resolve(root, entry.name); + if (entry.isDirectory() && !entry.isSymbolicLink()) { + files.push(...collectMarkdownFiles(path, maxDepth - 1)); + } else if (entry.isFile() && entry.name.endsWith('.md')) { + files.push(path); + } + } + return files.sort((left, right) => left.localeCompare(right)); +} + +function hasInstalledHook(directory: string): boolean { + if (!existsSync(directory)) { + return false; + } + + return readdirSync(directory).some((entry) => { + if (entry.endsWith('.sample')) { + return false; + } + + const path = resolve(directory, entry); + const stats = statSync(path); + if (!stats.isFile()) { + return false; + } + + return (stats.mode & 0o111) !== 0 || /\.(cmd|ps1|bat)$/iu.test(basename(entry)); + }); +} + +function runGit(args: readonly string[], root: string): string { + return execFileSync('git', args, { + cwd: root, + encoding: 'utf8', + stdio: ['ignore', 'pipe', 'pipe'], + }).trim(); +} + +function runGitOptional(args: readonly string[], root: string): string { + try { + return runGit(args, root); + } catch (error: unknown) { + if (isExpectedMissingValue(error)) { + return ''; + } + throw error; + } +} + +function isExpectedMissingValue(error: unknown): boolean { + return error instanceof Error + && 'status' in error + && typeof error.status === 'number' + && error.status === 1; +} diff --git a/src/domain.ts b/src/domain.ts index 93664b9..781fe3b 100644 --- a/src/domain.ts +++ b/src/domain.ts @@ -40,3 +40,47 @@ export const WorkspaceStatusSchema = z.object({ legendHealth: z.array(LegendHealthSchema), }); export type WorkspaceStatus = z.infer; + +export const DoctorStatusSchema = z.enum(['ok', 'warn', 'error']); +export type DoctorStatus = z.infer; + +export const DoctorSeveritySchema = z.enum(['warning', 'error']); +export type DoctorSeverity = z.infer; + +export const DoctorCheckIdSchema = z.enum([ + 'config', + 'structure', + 'frontmatter', + 'git-hooks', + 'backlog', +]); +export type DoctorCheckId = z.infer; + +export const DoctorCheckSchema = z.object({ + id: DoctorCheckIdSchema, + status: DoctorStatusSchema, + message: z.string(), +}); +export type DoctorCheck = z.infer; + +export const DoctorIssueSchema = z.object({ + code: z.string(), + check: DoctorCheckIdSchema, + severity: DoctorSeveritySchema, + message: z.string(), + path: z.string().optional(), + fix: z.string(), +}); +export type DoctorIssue = z.infer; + +export const DoctorReportSchema = z.object({ + root: z.string(), + status: DoctorStatusSchema, + checks: z.array(DoctorCheckSchema), + issues: z.array(DoctorIssueSchema), + counts: z.object({ + errors: z.number().int().nonnegative(), + warnings: z.number().int().nonnegative(), + }), +}); +export type DoctorReport = z.infer; diff --git a/src/mcp.ts b/src/mcp.ts index 42e3128..b06b12b 100644 --- a/src/mcp.ts +++ b/src/mcp.ts @@ -3,6 +3,7 @@ import { CallToolRequestSchema, ListToolsRequestSchema } from '@modelcontextprot import { basename, relative } from 'node:path'; import { Workspace } from './index.js'; import { GitHubAdapter, type GitHubSyncResult } from './adapters/github.js'; +import { renderDoctorText, runDoctor } from './doctor.js'; import type { Cycle, Outcome, WorkspaceStatus } from './domain.js'; import { queryReviewState, renderReviewStateText, type ReviewStateQueryOptions } from './review-state.js'; @@ -27,6 +28,15 @@ export interface McpToolDef { } export const MCP_TOOLS: McpToolDef[] = [ + { + name: 'method_doctor', + description: 'Inspect METHOD workspace health and report concrete problems with suggested fixes, even when the workspace is partially broken.', + inputSchema: { + type: 'object', + properties: { ...workspaceProperty }, + required: ['workspace'], + }, + }, { name: 'method_review_state', description: 'Get PR review / merge-readiness state for the current branch or an explicit PR. `pr` and `currentBranch` are mutually exclusive; when `pr` is omitted, current-branch resolution is the default behavior.', @@ -132,6 +142,15 @@ export function createMcpServer(options: CreateMcpServerOptions = {}) { throw new Error('workspace is required. Pass the absolute path to the METHOD workspace root directory.'); } + if (request.params.name === 'method_doctor') { + const report = runDoctor(workspacePath); + return successResult( + 'method_doctor', + renderDoctorText(report), + report, + ); + } + const workspace = new Workspace(workspacePath); workspace.ensureInitialized(); diff --git a/tests/cli.test.ts b/tests/cli.test.ts index 4f80be1..9f7dc61 100644 --- a/tests/cli.test.ts +++ b/tests/cli.test.ts @@ -2,6 +2,7 @@ import { existsSync, mkdtempSync, mkdirSync, readFileSync, rmSync, writeFileSync import { dirname, join } from 'node:path'; import { tmpdir } from 'node:os'; import { afterEach, describe, expect, it, vi } from 'vitest'; +import { initWorkspace } from '../src/index.js'; import { runCli } from '../src/cli.js'; class MemoryWriter { @@ -63,6 +64,50 @@ describe('method CLI', () => { expect(stdout.output).toContain('--outcome hill-met|partial|not-met'); }); + it('Does `method doctor --json` return a bounded workspace health report without throwing on malformed `.method.json`?', async () => { + const root = createTempRoot(); + initWorkspace(root); + writeFileSync(join(root, '.method.json'), '{ broken json }\n', 'utf8'); + const stdout = new MemoryWriter(); + + const exitCode = await runCli(['doctor', '--json'], { + cwd: root, + stdout, + stderr: new MemoryWriter(), + }); + + const report = JSON.parse(stdout.output); + expect(exitCode).toBe(1); + expect(report.status).toBe('error'); + expect(report.issues).toContainEqual(expect.objectContaining({ + code: 'config-parse-failed', + check: 'config', + })); + }); + + it('Does `method doctor` surface missing required paths and malformed frontmatter with fix suggestions?', async () => { + const root = createTempRoot(); + initWorkspace(root); + rmSync(join(root, 'docs/design'), { recursive: true, force: true }); + writeFileSync( + join(root, 'docs/method/backlog/inbox/PROCESS_missing-frontmatter.md'), + '# Missing Frontmatter\n\nBody\n', + 'utf8', + ); + const stdout = new MemoryWriter(); + + const exitCode = await runCli(['doctor'], { + cwd: root, + stdout, + stderr: new MemoryWriter(), + }); + + expect(exitCode).toBe(1); + expect(stdout.output).toContain('missing-directory'); + expect(stdout.output).toContain('missing-frontmatter'); + expect(stdout.output).toContain('Fix:'); + }); + it('captures backlog ideas in inbox', async () => { const root = createTempRoot(); await runCli(['init'], { cwd: root, stdout: new MemoryWriter(), stderr: new MemoryWriter() }); diff --git a/tests/doctor.test.ts b/tests/doctor.test.ts new file mode 100644 index 0000000..f48fb6d --- /dev/null +++ b/tests/doctor.test.ts @@ -0,0 +1,101 @@ +import { execFileSync } from 'node:child_process'; +import { mkdtempSync, rmSync, writeFileSync } from 'node:fs'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; +import { afterEach, describe, expect, it } from 'vitest'; +import { initWorkspace } from '../src/index.js'; +import { runDoctor } from '../src/doctor.js'; + +const tempRoots: string[] = []; + +afterEach(() => { + for (const root of tempRoots) { + rmSync(root, { recursive: true, force: true }); + } + tempRoots.length = 0; +}); + +function createTempRoot(): string { + const root = mkdtempSync(join(tmpdir(), 'method-doctor-')); + tempRoots.push(root); + return root; +} + +describe('doctor engine', () => { + it('Does `runDoctor()` report orphaned backlog items and git-hook diagnostics without requiring a healthy `Workspace` instance?', () => { + const root = createTempRoot(); + initWorkspace(root); + writeFileSync(join(root, '.method.json'), '{ not json }\n', 'utf8'); + writeFileSync( + join(root, 'docs/method/backlog/PROCESS_orphan.md'), + [ + '---', + 'title: "Orphan"', + 'legend: PROCESS', + 'lane: inbox', + '---', + '', + '# Orphan', + '', + 'Body', + ].join('\n'), + 'utf8', + ); + + const report = runDoctor(root); + const issueCodes = report.issues.map((issue) => issue.code); + + expect(report.status).toBe('error'); + expect(issueCodes).toContain('config-parse-failed'); + expect(issueCodes).toContain('orphaned-backlog-item'); + expect(issueCodes.some((code) => code.startsWith('git-hooks-'))).toBe(true); + }); + + it('reports missing required paths and malformed packet frontmatter with concrete fix suggestions.', () => { + const root = createTempRoot(); + initWorkspace(root); + rmSync(join(root, 'docs/design'), { recursive: true, force: true }); + writeFileSync( + join(root, 'docs/method/backlog/inbox/PROCESS_bad-frontmatter.md'), + '---\ntitle: [broken\n---\n\n# Bad Frontmatter\n', + 'utf8', + ); + + const report = runDoctor(root); + const missingDirectory = report.issues.find((issue) => issue.code === 'missing-directory'); + const badFrontmatter = report.issues.find((issue) => issue.code === 'frontmatter-parse-failed'); + + expect(report.status).toBe('error'); + expect(missingDirectory?.path).toBe('docs/design'); + expect(missingDirectory?.fix).toContain('method init'); + expect(badFrontmatter?.path).toBe('docs/method/backlog/inbox/PROCESS_bad-frontmatter.md'); + expect(badFrontmatter?.fix).toContain('Fix the YAML syntax'); + }); + + it('treats absent empty backlog lane directories as acceptable and falls back to default git hook inspection.', () => { + const root = createTempRoot(); + initWorkspace(root); + execFileSync('git', ['init'], { cwd: root, stdio: 'ignore' }); + rmSync(join(root, 'docs/method/backlog/asap'), { recursive: true, force: true }); + rmSync(join(root, 'docs/method/backlog/up-next'), { recursive: true, force: true }); + + const report = runDoctor(root); + + expect(report.status).toBe('warn'); + expect(report.issues).not.toContainEqual(expect.objectContaining({ + code: 'missing-directory', + path: 'docs/method/backlog/asap', + })); + expect(report.issues).not.toContainEqual(expect.objectContaining({ + code: 'missing-directory', + path: 'docs/method/backlog/up-next', + })); + expect(report.issues).toContainEqual(expect.objectContaining({ + code: 'git-hooks-not-configured', + check: 'git-hooks', + })); + expect(report.issues).not.toContainEqual(expect.objectContaining({ + code: 'git-hooks-unavailable', + })); + }); +}); diff --git a/tests/mcp.test.ts b/tests/mcp.test.ts index e3ad5eb..33f7893 100644 --- a/tests/mcp.test.ts +++ b/tests/mcp.test.ts @@ -80,6 +80,7 @@ describe('MCP Server', () => { expect(result.tools.length).toBeGreaterThan(0); const toolNames = result.tools.map((t: any) => t.name); + expect(toolNames).toContain('method_doctor'); expect(toolNames).toContain('method_status'); expect(toolNames).toContain('method_review_state'); expect(toolNames).toContain('method_inbox'); @@ -146,6 +147,34 @@ describe('MCP Server', () => { vi.restoreAllMocks(); }); + it('Does `method_doctor` return the same doctor report contract under `structuredContent.result`?', async () => { + const root = createTempRoot(); + initWorkspace(root); + writeFileSync(join(root, '.method.json'), '{ broken json }\n', 'utf8'); + const callToolHandler = createCallToolHarness(); + const stdout = new MemoryWriter(); + + const cliExitCode = await runCli(['doctor', '--json'], { + cwd: root, + stdout, + stderr: new MemoryWriter(), + }); + const mcpResult = await callToolHandler({ + params: { + name: 'method_doctor', + arguments: { + workspace: root, + }, + }, + }); + + expect(cliExitCode).toBe(1); + expect(mcpResult.isError).toBe(false); + expect(mcpResult.structuredContent.tool).toBe('method_doctor'); + expect(mcpResult.structuredContent.result).toEqual(JSON.parse(stdout.output)); + expect(mcpResult.content[0].text).toContain('Status: error'); + }); + it('Does the CLI `--json` output exactly match the MCP `structuredContent.result` contract?', async () => { const root = createTempRoot(); initWorkspace(root); From 094c0464bcd13099f902dc0e5ec6e2f475236966 Mon Sep 17 00:00:00 2001 From: James Ross Date: Thu, 9 Apr 2026 18:49:14 -0700 Subject: [PATCH 2/5] docs: close 0038 doctor-command --- .../0038-doctor-command/doctor-command.md | 52 +++++++++++++++++++ .../witness/verification.md | 41 +++++++++++++++ 2 files changed, 93 insertions(+) create mode 100644 docs/method/retro/0038-doctor-command/doctor-command.md create mode 100644 docs/method/retro/0038-doctor-command/witness/verification.md diff --git a/docs/method/retro/0038-doctor-command/doctor-command.md b/docs/method/retro/0038-doctor-command/doctor-command.md new file mode 100644 index 0000000..6f7f705 --- /dev/null +++ b/docs/method/retro/0038-doctor-command/doctor-command.md @@ -0,0 +1,52 @@ +--- +title: "Doctor Command" +cycle: "0038-doctor-command" +design_doc: "docs/design/0038-doctor-command/doctor-command.md" +outcome: hill-met +drift_check: yes +--- + +# Doctor Command Retro + +## Summary + +This cycle landed a native `doctor` surface for METHOD that works even +when the workspace cannot be loaded through the normal `Workspace` +constructor path. The new engine inspects configuration parsing, +required repo structure, packet frontmatter, git hook setup, and +backlog lane shape, then reports bounded issues with direct fix +suggestions for both humans and agents. + +The late cleanup in this cycle mattered. The first pass treated missing +empty backlog lane directories as hard structural failures and treated +an unset `core.hooksPath` as "git metadata unavailable." The final +behavior is stricter in the right places instead: missing empty lane +directories in a clean clone are acceptable, while a real repo with no +configured hooks reports a truthful warning instead of a false error. + +## Playback Witness + +- [Verification Witness](./witness/verification.md) +- Close-time verification captured the full `npm test` pass and a direct + drift check for `0038-doctor-command`. + +## Drift + +- None recorded after aligning the final agent playback question string + to its exact test description. + +## New Debt + +- None recorded. + +## Cool Ideas + +- None recorded. + +## Backlog Maintenance + +- Backlog maintenance deferred; the cycle hill was about bounded + diagnosis, not repo-wide triage or queue grooming. +- [ ] Inbox processed +- [x] Priorities reviewed +- [ ] Dead work buried or merged diff --git a/docs/method/retro/0038-doctor-command/witness/verification.md b/docs/method/retro/0038-doctor-command/witness/verification.md new file mode 100644 index 0000000..6cbadec --- /dev/null +++ b/docs/method/retro/0038-doctor-command/witness/verification.md @@ -0,0 +1,41 @@ +--- +title: "Verification Witness for Cycle 38" +--- + +# Verification Witness for Cycle 38 + +This witness proves that `Doctor Command` now carries the required +behavior and adheres to the repo invariants. + +## Test Results + +```text + +> @flyingrobots/method@1.0.0 test +> vitest run --config vitest.config.ts + + + RUN v4.1.2 . + + + Test Files 17 passed (17) + Tests 189 passed (189) + Start at 18:48:01 + Duration 1.40s (transform 1.35s, setup 0ms, import 4.66s, tests 2.79s, environment 1ms) + + +``` + +## Drift Results + +```text +No playback-question drift found. +Scanned 1 active cycle, 4 playback questions, 202 test descriptions. +Search basis: exact normalized match in tests/**/*.test.* and tests/**/*.spec.* descriptions. + +``` + +## Manual Verification + +- [x] `method doctor` reports this repo as `warn`, not `error`, with only the truthful `git-hooks-not-configured` warning. +- [x] Automated capture completed successfully. From 614352260d89da7ac3336592cbb5cf831e8e40fd Mon Sep 17 00:00:00 2001 From: James Ross Date: Thu, 9 Apr 2026 21:30:27 -0700 Subject: [PATCH 3/5] fix: harden doctor path inspection --- src/doctor.ts | 73 ++++++++++++++++++++++++++++++++++---------- tests/doctor.test.ts | 62 +++++++++++++++++++++++++++++++++++-- 2 files changed, 116 insertions(+), 19 deletions(-) diff --git a/src/doctor.ts b/src/doctor.ts index 03f4946..c81de08 100644 --- a/src/doctor.ts +++ b/src/doctor.ts @@ -9,7 +9,7 @@ import { LANES } from './domain.js'; import { parse as parseYaml } from 'yaml'; interface ConfigInspection { - paths: PathsConfig; + paths: PathsConfig | null; issues: DoctorIssue[]; } @@ -23,10 +23,10 @@ const DOCTOR_CHECKS: readonly DoctorCheckId[] = [ export function runDoctor(root: string): DoctorReport { const configInspection = inspectConfig(root); - const structureIssues = inspectStructure(root, configInspection.paths); - const frontmatterIssues = inspectFrontmatter(root, configInspection.paths); + const structureIssues = configInspection.paths === null ? [] : inspectStructure(root, configInspection.paths); + const frontmatterIssues = configInspection.paths === null ? [] : inspectFrontmatter(root, configInspection.paths); const gitHookIssues = inspectGitHooks(root); - const backlogIssues = inspectBacklog(root, configInspection.paths); + const backlogIssues = configInspection.paths === null ? [] : inspectBacklog(root, configInspection.paths); const issues = [ ...configInspection.issues, @@ -91,7 +91,7 @@ function inspectConfig(root: string): ConfigInspection { } catch (error: unknown) { const message = error instanceof Error ? error.message : String(error); return { - paths: DEFAULT_PATHS, + paths: null, issues: [ createIssue( 'config-parse-failed', @@ -111,13 +111,13 @@ function inspectConfig(root: string): ConfigInspection { } const parsedPaths = PathsSchema.safeParse( - rawConfig !== null && typeof rawConfig === 'object' && 'paths' in rawConfig - ? (rawConfig as { paths?: unknown }).paths - : undefined, + rawConfig !== null && typeof rawConfig === 'object' + ? ('paths' in rawConfig ? (rawConfig as { paths?: unknown }).paths : {}) + : {}, ); return { - paths: parsedPaths.success ? parsedPaths.data : DEFAULT_PATHS, + paths: parsedPaths.success ? parsedPaths.data : null, issues: [ createIssue( 'config-invalid', @@ -153,7 +153,18 @@ function inspectStructure(root: string, paths: PathsConfig): DoctorIssue[] { const issues: DoctorIssue[] = []; for (const directory of requiredDirectories) { + if (isDirectoryPath(directory)) { + continue; + } if (existsSync(directory)) { + issues.push(createIssue( + 'path-not-directory', + 'structure', + 'error', + 'A required METHOD directory path exists, but it is not a directory.', + relative(root, directory), + `Replace \`${relative(root, directory)}\` with a directory before running METHOD workspace commands.`, + )); continue; } issues.push(createIssue( @@ -167,7 +178,18 @@ function inspectStructure(root: string, paths: PathsConfig): DoctorIssue[] { } for (const file of requiredFiles) { + if (isFilePath(file)) { + continue; + } if (existsSync(file)) { + issues.push(createIssue( + 'path-not-file', + 'structure', + 'error', + 'A required METHOD file path exists, but it is not a file.', + relative(root, file), + `Replace \`${relative(root, file)}\` with a regular file before running METHOD workspace commands.`, + )); continue; } issues.push(createIssue( @@ -193,14 +215,15 @@ function inspectFrontmatter(root: string, paths: PathsConfig): DoctorIssue[] { ]; for (const packetRoot of packetRoots) { - if (!existsSync(packetRoot)) { + if (!isDirectoryPath(packetRoot)) { continue; } for (const file of collectMarkdownFiles(packetRoot)) { const relativePath = relative(root, file); const raw = readFileSync(file, 'utf8'); - if (!raw.startsWith('---\n')) { + const opening = /^\uFEFF?---\r?\n/u.exec(raw); + if (opening === null) { issues.push(createIssue( 'missing-frontmatter', 'frontmatter', @@ -212,8 +235,9 @@ function inspectFrontmatter(root: string, paths: PathsConfig): DoctorIssue[] { continue; } - const end = raw.indexOf('\n---\n', 4); - if (end === -1) { + const rest = raw.slice(opening[0].length); + const closing = /\r?\n---(?:\r?\n|$)/u.exec(rest); + if (closing === null) { issues.push(createIssue( 'unterminated-frontmatter', 'frontmatter', @@ -225,7 +249,8 @@ function inspectFrontmatter(root: string, paths: PathsConfig): DoctorIssue[] { continue; } - const yamlBlock = raw.slice(4, end); + const end = opening[0].length + closing.index; + const yamlBlock = raw.slice(opening[0].length, end); try { const parsed = parseYaml(yamlBlock); if (parsed === null || typeof parsed !== 'object' || Array.isArray(parsed)) { @@ -321,7 +346,7 @@ function inspectGitHooks(root: string): DoctorIssue[] { function inspectBacklog(root: string, paths: PathsConfig): DoctorIssue[] { const backlogRoot = resolve(root, paths.backlog); - if (!existsSync(backlogRoot)) { + if (!isDirectoryPath(backlogRoot)) { return []; } @@ -404,7 +429,7 @@ function createIssue( } function collectMarkdownFiles(root: string, maxDepth = 10): string[] { - if (maxDepth <= 0 || !existsSync(root)) { + if (maxDepth <= 0 || !isDirectoryPath(root)) { return []; } @@ -440,6 +465,22 @@ function hasInstalledHook(directory: string): boolean { }); } +function isDirectoryPath(path: string): boolean { + try { + return statSync(path).isDirectory(); + } catch { + return false; + } +} + +function isFilePath(path: string): boolean { + try { + return statSync(path).isFile(); + } catch { + return false; + } +} + function runGit(args: readonly string[], root: string): string { return execFileSync('git', args, { cwd: root, diff --git a/tests/doctor.test.ts b/tests/doctor.test.ts index f48fb6d..74d225e 100644 --- a/tests/doctor.test.ts +++ b/tests/doctor.test.ts @@ -1,5 +1,5 @@ import { execFileSync } from 'node:child_process'; -import { mkdtempSync, rmSync, writeFileSync } from 'node:fs'; +import { mkdirSync, mkdtempSync, rmSync, writeFileSync } from 'node:fs'; import { tmpdir } from 'node:os'; import { join } from 'node:path'; import { afterEach, describe, expect, it } from 'vitest'; @@ -25,7 +25,7 @@ describe('doctor engine', () => { it('Does `runDoctor()` report orphaned backlog items and git-hook diagnostics without requiring a healthy `Workspace` instance?', () => { const root = createTempRoot(); initWorkspace(root); - writeFileSync(join(root, '.method.json'), '{ not json }\n', 'utf8'); + writeFileSync(join(root, '.method.json'), JSON.stringify({ forge: 'invalid' }), 'utf8'); writeFileSync( join(root, 'docs/method/backlog/PROCESS_orphan.md'), [ @@ -46,7 +46,7 @@ describe('doctor engine', () => { const issueCodes = report.issues.map((issue) => issue.code); expect(report.status).toBe('error'); - expect(issueCodes).toContain('config-parse-failed'); + expect(issueCodes).toContain('config-invalid'); expect(issueCodes).toContain('orphaned-backlog-item'); expect(issueCodes.some((code) => code.startsWith('git-hooks-'))).toBe(true); }); @@ -72,6 +72,62 @@ describe('doctor engine', () => { expect(badFrontmatter?.fix).toContain('Fix the YAML syntax'); }); + it('skips path-based checks when malformed JSON makes the configured workspace paths unknowable.', () => { + const root = createTempRoot(); + writeFileSync(join(root, '.method.json'), '{ broken json }\n', 'utf8'); + + const report = runDoctor(root); + + expect(report.issues).toContainEqual(expect.objectContaining({ + code: 'config-parse-failed', + check: 'config', + })); + expect(report.issues.some((issue) => issue.check === 'structure')).toBe(false); + expect(report.issues.some((issue) => issue.check === 'frontmatter')).toBe(false); + expect(report.issues.some((issue) => issue.check === 'backlog')).toBe(false); + }); + + it('accepts CRLF frontmatter blocks whose closing delimiter sits at EOF.', () => { + const root = createTempRoot(); + initWorkspace(root); + writeFileSync( + join(root, 'docs/method/backlog/inbox/PROCESS_windows-frontmatter.md'), + '---\r\ntitle: "Windows Frontmatter"\r\nlegend: PROCESS\r\nlane: inbox\r\n---', + 'utf8', + ); + + const report = runDoctor(root); + + expect(report.issues).not.toContainEqual(expect.objectContaining({ + code: 'missing-frontmatter', + path: 'docs/method/backlog/inbox/PROCESS_windows-frontmatter.md', + })); + expect(report.issues).not.toContainEqual(expect.objectContaining({ + code: 'unterminated-frontmatter', + path: 'docs/method/backlog/inbox/PROCESS_windows-frontmatter.md', + })); + }); + + it('reports required path type mismatches instead of accepting any existing path as healthy.', () => { + const root = createTempRoot(); + initWorkspace(root); + rmSync(join(root, 'docs/design'), { recursive: true, force: true }); + writeFileSync(join(root, 'docs/design'), 'not a directory\n', 'utf8'); + rmSync(join(root, 'CHANGELOG.md'), { recursive: true, force: true }); + mkdirSync(join(root, 'CHANGELOG.md'), { recursive: true }); + + const report = runDoctor(root); + + expect(report.issues).toContainEqual(expect.objectContaining({ + code: 'path-not-directory', + path: 'docs/design', + })); + expect(report.issues).toContainEqual(expect.objectContaining({ + code: 'path-not-file', + path: 'CHANGELOG.md', + })); + }); + it('treats absent empty backlog lane directories as acceptable and falls back to default git hook inspection.', () => { const root = createTempRoot(); initWorkspace(root); From fe88ce9cf5c38793086f037c90847fb5f4073c30 Mon Sep 17 00:00:00 2001 From: James Ross Date: Thu, 9 Apr 2026 21:30:32 -0700 Subject: [PATCH 4/5] fix: show doctor usage for invalid flags --- src/cli-args.ts | 2 +- tests/cli.test.ts | 15 +++++++++++++++ 2 files changed, 16 insertions(+), 1 deletion(-) diff --git a/src/cli-args.ts b/src/cli-args.ts index 77fd518..7a2e6ec 100644 --- a/src/cli-args.ts +++ b/src/cli-args.ts @@ -177,7 +177,7 @@ function parseDoctorArgs(args: readonly string[]): ParsedCommand { json = true; continue; } - throw new MethodError(`Unknown option: ${value}`); + throw new MethodError(`Unknown option: ${value}\n\n${usage('doctor')}`); } return { command: 'doctor', json }; diff --git a/tests/cli.test.ts b/tests/cli.test.ts index 9f7dc61..37998f4 100644 --- a/tests/cli.test.ts +++ b/tests/cli.test.ts @@ -108,6 +108,21 @@ describe('method CLI', () => { expect(stdout.output).toContain('Fix:'); }); + it('includes doctor usage guidance when an unknown doctor flag is provided.', async () => { + const root = createTempRoot(); + const stderr = new MemoryWriter(); + + const exitCode = await runCli(['doctor', '--bogus'], { + cwd: root, + stdout: new MemoryWriter(), + stderr, + }); + + expect(exitCode).toBe(1); + expect(stderr.output).toContain('Unknown option: --bogus'); + expect(stderr.output).toContain('Usage: method doctor [--json]'); + }); + it('captures backlog ideas in inbox', async () => { const root = createTempRoot(); await runCli(['init'], { cwd: root, stdout: new MemoryWriter(), stderr: new MemoryWriter() }); From 91916b625e2ff968d20daffc99ceed4e345cde89 Mon Sep 17 00:00:00 2001 From: James Ross Date: Thu, 9 Apr 2026 21:57:04 -0700 Subject: [PATCH 5/5] fix: diagnose invalid git hooks paths --- src/doctor.ts | 28 +++++++++++++++++++++++++++- tests/doctor.test.ts | 18 ++++++++++++++++++ 2 files changed, 45 insertions(+), 1 deletion(-) diff --git a/src/doctor.ts b/src/doctor.ts index c81de08..a100456 100644 --- a/src/doctor.ts +++ b/src/doctor.ts @@ -298,6 +298,19 @@ function inspectGitHooks(root: string): DoctorIssue[] { ]; } + if (!isDirectoryPath(hookDir)) { + return [ + createIssue( + 'git-hooks-not-directory', + 'git-hooks', + 'warning', + 'Git hooks are configured, but the configured hooks path is not a directory.', + relative(root, hookDir), + `Replace \`${relative(root, hookDir)}\` with a directory or update \`core.hooksPath\` to a valid hooks directory.`, + ), + ]; + } + if (hasInstalledHook(hookDir)) { return []; } @@ -316,6 +329,19 @@ function inspectGitHooks(root: string): DoctorIssue[] { const defaultHooksPath = runGit(['rev-parse', '--git-path', 'hooks'], root); const hookDir = resolve(root, defaultHooksPath); + if (existsSync(hookDir) && !isDirectoryPath(hookDir)) { + return [ + createIssue( + 'git-hooks-not-directory', + 'git-hooks', + 'warning', + 'The default git hooks path exists, but it is not a directory.', + relative(root, hookDir), + `Replace \`${relative(root, hookDir)}\` with a directory before relying on default git hooks.`, + ), + ]; + } + if (hasInstalledHook(hookDir)) { return []; } @@ -446,7 +472,7 @@ function collectMarkdownFiles(root: string, maxDepth = 10): string[] { } function hasInstalledHook(directory: string): boolean { - if (!existsSync(directory)) { + if (!isDirectoryPath(directory)) { return false; } diff --git a/tests/doctor.test.ts b/tests/doctor.test.ts index 74d225e..d229b3f 100644 --- a/tests/doctor.test.ts +++ b/tests/doctor.test.ts @@ -154,4 +154,22 @@ describe('doctor engine', () => { code: 'git-hooks-unavailable', })); }); + + it('reports a configured hooks path that exists as a file without collapsing to git-hooks-unavailable.', () => { + const root = createTempRoot(); + initWorkspace(root); + execFileSync('git', ['init'], { cwd: root, stdio: 'ignore' }); + writeFileSync(join(root, 'hooks-file'), 'not a directory\n', 'utf8'); + execFileSync('git', ['config', 'core.hooksPath', 'hooks-file'], { cwd: root, stdio: 'ignore' }); + + const report = runDoctor(root); + + expect(report.issues).toContainEqual(expect.objectContaining({ + code: 'git-hooks-not-directory', + path: 'hooks-file', + })); + expect(report.issues).not.toContainEqual(expect.objectContaining({ + code: 'git-hooks-unavailable', + })); + }); });