From 408979f1e4b26791de7f0e9eec48db76ec9aa76a Mon Sep 17 00:00:00 2001 From: Kurt Overmier Date: Thu, 11 Jun 2026 17:39:58 -0500 Subject: [PATCH 1/2] feat(cli): MCP setup diagnostics + adf tidy routing improvements (#191/#198) - `charter hook print --mcp-config [--client claude|codex|cursor]` emits the mcpServers JSON snippet for wiring charter serve into any supported AI client - `charter doctor --mcp` detects MCP client config files and verifies charter is wired under mcpServers; surfaces actionable hint when not found - `charter adf tidy --dry-run` now shows per-item routing plan (target module + section) in text output so users can review before applying - `adf tidy` retains Session Start / Session Protocol / Session Setup headings alongside Environment and Module Index (not treated as bloat) - `adf tidy` preserves full code-block content with fencing instead of collapsing to a single-line description Closes #191, #198 Co-Authored-By: Claude Sonnet 4.6 --- packages/cli/src/commands/adf-tidy.ts | 37 +++++++++++---- packages/cli/src/commands/doctor.ts | 68 +++++++++++++++++++++++++++ packages/cli/src/commands/hook.ts | 48 ++++++++++++++++++- 3 files changed, 143 insertions(+), 10 deletions(-) diff --git a/packages/cli/src/commands/adf-tidy.ts b/packages/cli/src/commands/adf-tidy.ts index 5b5206e..532a955 100644 --- a/packages/cli/src/commands/adf-tidy.ts +++ b/packages/cli/src/commands/adf-tidy.ts @@ -175,9 +175,10 @@ export async function adfTidyCommand(options: CLIOptions, args: string[]): Promi ? projectModuleWarnings(aiDir, allModuleGroups) : scanModuleWarnings(aiDir, modulesModified); - // Collect per-item routing trace when --verbose (or always in JSON mode) + // Collect per-item routing trace when --verbose, in JSON mode, or in dry-run + // (dry-run always shows the plan so users can review before applying — #198) let itemRoutes: ItemRoute[] | undefined; - if (verbose || options.format === 'json') { + if (verbose || options.format === 'json' || dryRun) { itemRoutes = []; for (const [, sectionGroups] of Object.entries(allModuleGroups)) { for (const [, items] of Object.entries(sectionGroups)) { @@ -310,8 +311,13 @@ function extractBeyondPointer(content: string, fileName: string): string { // Section detection if (trimmed.startsWith('## ')) { - // Environment and Module Index are charter-managed retained sections — not bloat (#71) - if (trimmed === '## Environment' || trimmed === '## Module Index') { + // Charter-managed and operational protocol sections retained in the vendor file (#198). + // Environment: WSL/OS runtime config. + // Module Index: charter-generated on-demand listing. + // Session Start / Session Protocol: operational wiring steps that belong in the + // vendor file as environment context, not in a domain ADF module. + const retained = /^## (Environment|Module Index|Session Start|Session Protocol|Session Setup)$/.test(trimmed); + if (retained) { inEnvironmentSection = true; continue; } else { @@ -424,11 +430,12 @@ function formatItemForAdf(item: MigrationItem): string { switch (el.type) { case 'rule': return el.content; - case 'code-block': - if (el.language === 'bash' || el.language === 'sh') { - return `[Build commands] ${el.content.split('\n').filter(l => l.trim()).slice(0, 3).join('; ')}${el.content.split('\n').filter(l => l.trim()).length > 3 ? ' (...)' : ''}`; - } - return `[${el.language || 'code'}] ${el.content.split('\n')[0]}`; + case 'code-block': { + // Preserve the full code block so nothing is lost on round-trip (#198). + // Wrap in a fenced block so the ADF formatter keeps it as a text body. + const lang = el.language || ''; + return `\`\`\`${lang}\n${el.content}\n\`\`\``; + } case 'table-row': return el.content; case 'table-block': @@ -642,6 +649,18 @@ function printTextResult(result: TidyResult): void { console.log(` ${f.file}: ${f.itemsExtracted} items \u2192 ${routes}`); } + // In dry-run mode always show per-item routing (not just with --verbose) so + // users can review the plan before applying (#198). + if (result.dryRun && result.itemRoutes && result.itemRoutes.length > 0) { + console.log(''); + console.log(' Routing plan (run without --dry-run to apply, or adjust triggers first):'); + for (const r of result.itemRoutes) { + const preview = r.item.length > 70 ? r.item.slice(0, 67) + '...' : r.item; + console.log(` "${preview}"`); + console.log(` \u2192 ${r.targetModule} / ${r.targetSection}`); + } + } + if (clean.length > 0) { console.log(` ${clean.length} file(s) already clean.`); } diff --git a/packages/cli/src/commands/doctor.ts b/packages/cli/src/commands/doctor.ts index d2c4468..c6b7c0c 100644 --- a/packages/cli/src/commands/doctor.ts +++ b/packages/cli/src/commands/doctor.ts @@ -23,9 +23,59 @@ interface DoctorResult { }>; } +// Known MCP client config file paths (relative to repo root) and the key they look up under. +const MCP_CLIENT_CONFIGS: Array<{ label: string; configPath: string }> = [ + { label: 'Claude Code', configPath: '.claude/settings.json' }, + { label: 'Claude Code (local)', configPath: '.claude/settings.local.json' }, + { label: 'Generic MCP (.mcp.json)', configPath: '.mcp.json' }, + { label: 'Cursor', configPath: '.cursor/mcp.json' }, +]; + +function checkMcpWiring(): DoctorResult['checks'][number] { + const wired: string[] = []; + const missing: string[] = []; + + for (const { label, configPath } of MCP_CLIENT_CONFIGS) { + if (!fs.existsSync(configPath)) continue; + try { + const raw = fs.readFileSync(configPath, 'utf-8'); + const parsed = JSON.parse(raw); + const hasCharter = parsed?.mcpServers?.charter !== undefined; + if (hasCharter) { + wired.push(`${label} (${configPath})`); + } else { + missing.push(`${label} (${configPath}) — no mcpServers.charter entry`); + } + } catch { + missing.push(`${label} (${configPath}) — invalid JSON`); + } + } + + if (wired.length === 0 && missing.length === 0) { + return { + name: 'mcp wiring', + status: 'WARN', + details: 'No MCP client config files found. Run: charter hook print --mcp-config --client claude', + }; + } + + if (wired.length > 0) { + const details = `charter serve wired in: ${wired.join('; ')}` + + (missing.length > 0 ? `\n Not wired: ${missing.join('; ')}` : ''); + return { name: 'mcp wiring', status: 'PASS', details }; + } + + return { + name: 'mcp wiring', + status: 'WARN', + details: `MCP config file(s) found but charter not wired: ${missing.join('; ')}\n Run: charter hook print --mcp-config --client claude`, + }; +} + export async function doctorCommand(options: CLIOptions, args: string[] = []): Promise { const checks: DoctorResult['checks'] = []; const adfOnly = args.includes('--adf-only'); + const mcpMode = args.includes('--mcp'); const configFile = path.join(options.configPath, 'config.json'); const inGitRepo = isGitRepo(); const config = loadConfig(options.configPath); @@ -33,6 +83,24 @@ export async function doctorCommand(options: CLIOptions, args: string[] = []): P // set during manifest parse, used by the source LOC budget coverage check. let manifestLocMetricCount = 0; + // --mcp: focused MCP wiring check only + if (mcpMode) { + checks.push(checkMcpWiring()); + const hasWarn = checks.some(c => c.status === 'WARN'); + const result: DoctorResult = { status: hasWarn ? 'WARN' : 'PASS', checks }; + if (options.format === 'json') { + console.log(JSON.stringify(result, null, 2)); + } else { + console.log(` Doctor status: ${result.status}`); + for (const check of result.checks) { + const icon = check.status === 'PASS' ? '[ok]' : check.status === 'INFO' ? '[info]' : '[warn]'; + console.log(` ${icon} ${check.name}: ${check.details}`); + } + } + if (options.ciMode && hasWarn) return EXIT_CODE.POLICY_VIOLATION; + return EXIT_CODE.SUCCESS; + } + checks.push({ name: 'git repository', status: inGitRepo ? 'PASS' : 'WARN', diff --git a/packages/cli/src/commands/hook.ts b/packages/cli/src/commands/hook.ts index da649ab..4b5f337 100644 --- a/packages/cli/src/commands/hook.ts +++ b/packages/cli/src/commands/hook.ts @@ -9,6 +9,7 @@ import * as path from 'node:path'; import type { CLIOptions } from '../index'; import { CLIError, EXIT_CODE } from '../index'; import { runGit, isGitRepo, getGitErrorMessage } from '../git-helpers'; +import { getFlag } from '../flags'; interface HookInstallResult { status: 'INSTALLED' | 'SKIPPED'; @@ -113,6 +114,33 @@ export function printClaudeHookConfig(): void { console.log(JSON.stringify(CLAUDE_SESSION_HOOK_CONFIG, null, 2)); } +// MCP server entry shapes for each supported client (#191) +const MCP_CLIENTS = ['claude', 'codex', 'cursor'] as const; +type McpClient = typeof MCP_CLIENTS[number]; + +function buildMcpConfigSnippet(client: McpClient, aiDir: string): object { + const serverEntry = { + command: 'charter', + args: aiDir !== '.ai' ? ['serve', '--ai-dir', aiDir] : ['serve'], + }; + if (client === 'claude') { + // .claude/settings.json shape + return { mcpServers: { charter: serverEntry } }; + } + // .mcp.json shape — used by Codex and Cursor + return { mcpServers: { charter: serverEntry } }; +} + +function printMcpConfig(client: McpClient, aiDir: string): void { + const snippet = buildMcpConfigSnippet(client, aiDir); + console.log(JSON.stringify(snippet, null, 2)); + if (client === 'claude') { + console.error(' Paste the mcpServers entry into .claude/settings.json (or settings.local.json).'); + } else { + console.error(` Paste the mcpServers entry into .mcp.json at the repo root.`); + } +} + export async function hookCommand(options: CLIOptions, args: string[]): Promise { if (args.length === 0 || args.includes('--help') || args.includes('-h')) { printHelp(); @@ -121,8 +149,18 @@ export async function hookCommand(options: CLIOptions, args: string[]): Promise< if (args[0] === 'print') { const wantClaude = args.includes('--claude'); + const wantMcpConfig = args.includes('--mcp-config'); + + if (wantMcpConfig) { + const clientFlag = args[args.indexOf('--client') + 1] as McpClient | undefined; + const client: McpClient = MCP_CLIENTS.includes(clientFlag as McpClient) ? clientFlag as McpClient : 'claude'; + const aiDir = getFlag(args, '--ai-dir') || '.ai'; + printMcpConfig(client, aiDir); + return EXIT_CODE.SUCCESS; + } + if (!wantClaude) { - throw new CLIError('hook print requires --claude.'); + throw new CLIError('hook print requires --claude or --mcp-config.'); } printClaudeHookConfig(); return EXIT_CODE.SUCCESS; @@ -284,6 +322,7 @@ function printHelp(): void { console.log(' charter hook install --commit-msg [--force]'); console.log(' charter hook install --pre-commit [--force]'); console.log(' charter hook print --claude'); + console.log(' charter hook print --mcp-config [--client claude|codex|cursor] [--ai-dir ]'); console.log(''); console.log(' --commit-msg: Install a git commit-msg hook that normalizes Governed-By and'); console.log(' Resolves-Request trailers using git interpret-trailers.'); @@ -297,4 +336,11 @@ function printHelp(): void { console.log(' Paste into .claude/settings.json → hooks.UserPromptSubmit to auto-refresh'); console.log(' context at session start so charter_context returns live state.'); console.log(''); + console.log(' print --mcp-config: Print the MCP server JSON config snippet for wiring'); + console.log(' charter serve into an AI client. Defaults to --client claude.'); + console.log(' --client claude → mcpServers entry for .claude/settings.json'); + console.log(' --client codex → mcpServers entry for .mcp.json'); + console.log(' --client cursor → mcpServers entry for .mcp.json'); + console.log(' --ai-dir → pass-through for non-default .ai dir locations.'); + console.log(''); } From d55c7048c3fcb706b2cd081a2133255b4dc4fc17 Mon Sep 17 00:00:00 2001 From: Kurt Overmier Date: Thu, 11 Jun 2026 17:48:54 -0500 Subject: [PATCH 2/2] fix(cli): address review findings in MCP setup + tidy routing (#208) - Remove dead branch in buildMcpConfigSnippet (both arms were identical); inline the snippet construction directly in printMcpConfig - Fix --client flag parsing: guard indexOf result before indexing so a missing --client flag never reads args[0] as the client value - Fix dry-run routing plan truncation: split on newline before slicing so multi-line items (fenced code blocks) don't embed raw newlines - Add unit tests for all hook print --mcp-config paths (5 cases) - Add integration tests for charter doctor --mcp (5 cases: no files, missing entry, wired, invalid JSON, partial wiring) Co-Authored-By: Claude Sonnet 4.6 --- packages/cli/src/__tests__/hook.test.ts | 62 +++++++++++++ .../integration/doctor-loc-budget.test.ts | 92 +++++++++++++++++++ packages/cli/src/commands/adf-tidy.ts | 3 +- packages/cli/src/commands/hook.ts | 18 +--- 4 files changed, 161 insertions(+), 14 deletions(-) diff --git a/packages/cli/src/__tests__/hook.test.ts b/packages/cli/src/__tests__/hook.test.ts index 3312793..a15ab2c 100644 --- a/packages/cli/src/__tests__/hook.test.ts +++ b/packages/cli/src/__tests__/hook.test.ts @@ -105,4 +105,66 @@ describe('hookCommand', () => { it('hook print without --claude throws', async () => { await expect(hookCommand(baseOptions, ['print'])).rejects.toThrow('hook print requires --claude'); }); + + describe('hook print --mcp-config', () => { + it('returns 0 and emits mcpServers JSON to stdout', async () => { + const logs: string[] = []; + vi.spyOn(console, 'log').mockImplementation((...args: unknown[]) => { + logs.push(args.map(String).join(' ')); + }); + vi.spyOn(console, 'error').mockImplementation(() => {}); + + const exitCode = await hookCommand(baseOptions, ['print', '--mcp-config']); + expect(exitCode).toBe(0); + + const parsed = JSON.parse(logs.join('\n')) as { mcpServers: { charter: { command: string; args: string[] } } }; + expect(parsed.mcpServers.charter.command).toBe('charter'); + expect(parsed.mcpServers.charter.args).toEqual(['serve']); + }); + + it('defaults to --client claude when --client is absent', async () => { + const errors: string[] = []; + vi.spyOn(console, 'log').mockImplementation(() => {}); + vi.spyOn(console, 'error').mockImplementation((...args: unknown[]) => { + errors.push(args.map(String).join(' ')); + }); + + await hookCommand(baseOptions, ['print', '--mcp-config']); + expect(errors.join('\n')).toContain('.claude/settings.json'); + }); + + it('--client cursor emits .mcp.json hint', async () => { + const errors: string[] = []; + vi.spyOn(console, 'log').mockImplementation(() => {}); + vi.spyOn(console, 'error').mockImplementation((...args: unknown[]) => { + errors.push(args.map(String).join(' ')); + }); + + await hookCommand(baseOptions, ['print', '--mcp-config', '--client', 'cursor']); + expect(errors.join('\n')).toContain('.mcp.json'); + }); + + it('--ai-dir adds --ai-dir to server args', async () => { + const logs: string[] = []; + vi.spyOn(console, 'log').mockImplementation((...args: unknown[]) => { + logs.push(args.map(String).join(' ')); + }); + vi.spyOn(console, 'error').mockImplementation(() => {}); + + await hookCommand(baseOptions, ['print', '--mcp-config', '--ai-dir', 'custom/ai']); + const parsed = JSON.parse(logs.join('\n')) as { mcpServers: { charter: { args: string[] } } }; + expect(parsed.mcpServers.charter.args).toEqual(['serve', '--ai-dir', 'custom/ai']); + }); + + it('unknown --client value falls back to claude', async () => { + const errors: string[] = []; + vi.spyOn(console, 'log').mockImplementation(() => {}); + vi.spyOn(console, 'error').mockImplementation((...args: unknown[]) => { + errors.push(args.map(String).join(' ')); + }); + + await hookCommand(baseOptions, ['print', '--mcp-config', '--client', 'vscode']); + expect(errors.join('\n')).toContain('.claude/settings.json'); + }); + }); }); diff --git a/packages/cli/src/__tests__/integration/doctor-loc-budget.test.ts b/packages/cli/src/__tests__/integration/doctor-loc-budget.test.ts index 6fb3b67..4a6f60d 100644 --- a/packages/cli/src/__tests__/integration/doctor-loc-budget.test.ts +++ b/packages/cli/src/__tests__/integration/doctor-loc-budget.test.ts @@ -170,3 +170,95 @@ describe('charter doctor source LOC budget (integration)', () => { expect(exitCode).toBe(EXIT_CODE.SUCCESS); }); }); + +describe('charter doctor --mcp (MCP wiring detection)', () => { + async function runMcpDoctor(): Promise<{ exitCode: number; output: DoctorOutput }> { + const logs: string[] = []; + const spy = vi.spyOn(console, 'log').mockImplementation((...msgs: unknown[]) => { + logs.push(msgs.map(String).join(' ')); + }); + let exitCode: number; + try { + exitCode = await doctorCommand(ciOptions, ['--mcp']); + } finally { + spy.mockRestore(); + } + return { exitCode, output: JSON.parse(logs.join('\n').trim()) as DoctorOutput }; + } + + it('WARN when no MCP config files exist at all', async () => { + const tmp = makeTempDir('mcp-none'); + process.chdir(tmp); + execFileSync('git', ['init', '-q'], { cwd: tmp, stdio: 'ignore' }); + + const { exitCode, output } = await runMcpDoctor(); + const check = output.checks.find(c => c.name === 'mcp wiring'); + expect(check?.status).toBe('WARN'); + expect(check?.details).toContain('charter hook print --mcp-config'); + expect(exitCode).toBe(EXIT_CODE.POLICY_VIOLATION); + }); + + it('WARN when config file exists but has no mcpServers.charter entry', async () => { + const tmp = makeTempDir('mcp-missing-entry'); + process.chdir(tmp); + execFileSync('git', ['init', '-q'], { cwd: tmp, stdio: 'ignore' }); + fs.mkdirSync(path.join(tmp, '.claude'), { recursive: true }); + fs.writeFileSync(path.join(tmp, '.claude', 'settings.json'), JSON.stringify({ mcpServers: {} })); + + const { exitCode, output } = await runMcpDoctor(); + const check = output.checks.find(c => c.name === 'mcp wiring'); + expect(check?.status).toBe('WARN'); + expect(check?.details).toContain('no mcpServers.charter entry'); + expect(exitCode).toBe(EXIT_CODE.POLICY_VIOLATION); + }); + + it('PASS when mcpServers.charter is present in .claude/settings.json', async () => { + const tmp = makeTempDir('mcp-wired'); + process.chdir(tmp); + execFileSync('git', ['init', '-q'], { cwd: tmp, stdio: 'ignore' }); + fs.mkdirSync(path.join(tmp, '.claude'), { recursive: true }); + fs.writeFileSync( + path.join(tmp, '.claude', 'settings.json'), + JSON.stringify({ mcpServers: { charter: { command: 'charter', args: ['serve'] } } }), + ); + + const { exitCode, output } = await runMcpDoctor(); + const check = output.checks.find(c => c.name === 'mcp wiring'); + expect(check?.status).toBe('PASS'); + expect(check?.details).toContain('charter serve wired in'); + expect(exitCode).toBe(EXIT_CODE.SUCCESS); + }); + + it('WARN when config file contains invalid JSON', async () => { + const tmp = makeTempDir('mcp-bad-json'); + process.chdir(tmp); + execFileSync('git', ['init', '-q'], { cwd: tmp, stdio: 'ignore' }); + fs.mkdirSync(path.join(tmp, '.claude'), { recursive: true }); + fs.writeFileSync(path.join(tmp, '.claude', 'settings.json'), '{ bad json }'); + + const { exitCode, output } = await runMcpDoctor(); + const check = output.checks.find(c => c.name === 'mcp wiring'); + expect(check?.status).toBe('WARN'); + expect(check?.details).toContain('invalid JSON'); + expect(exitCode).toBe(EXIT_CODE.POLICY_VIOLATION); + }); + + it('PASS with partial wiring mentions unwired files in details', async () => { + const tmp = makeTempDir('mcp-partial'); + process.chdir(tmp); + execFileSync('git', ['init', '-q'], { cwd: tmp, stdio: 'ignore' }); + // Wire in settings.json but leave .mcp.json unwired + fs.mkdirSync(path.join(tmp, '.claude'), { recursive: true }); + fs.writeFileSync( + path.join(tmp, '.claude', 'settings.json'), + JSON.stringify({ mcpServers: { charter: { command: 'charter', args: ['serve'] } } }), + ); + fs.writeFileSync(path.join(tmp, '.mcp.json'), JSON.stringify({ mcpServers: {} })); + + const { exitCode, output } = await runMcpDoctor(); + const check = output.checks.find(c => c.name === 'mcp wiring'); + expect(check?.status).toBe('PASS'); + expect(check?.details).toContain('Not wired'); + expect(exitCode).toBe(EXIT_CODE.SUCCESS); + }); +}); diff --git a/packages/cli/src/commands/adf-tidy.ts b/packages/cli/src/commands/adf-tidy.ts index 532a955..e8c53f8 100644 --- a/packages/cli/src/commands/adf-tidy.ts +++ b/packages/cli/src/commands/adf-tidy.ts @@ -655,7 +655,8 @@ function printTextResult(result: TidyResult): void { console.log(''); console.log(' Routing plan (run without --dry-run to apply, or adjust triggers first):'); for (const r of result.itemRoutes) { - const preview = r.item.length > 70 ? r.item.slice(0, 67) + '...' : r.item; + const firstLine = r.item.split('\n')[0]; + const preview = firstLine.length > 70 ? firstLine.slice(0, 67) + '...' : firstLine; console.log(` "${preview}"`); console.log(` \u2192 ${r.targetModule} / ${r.targetSection}`); } diff --git a/packages/cli/src/commands/hook.ts b/packages/cli/src/commands/hook.ts index 4b5f337..dc17ce3 100644 --- a/packages/cli/src/commands/hook.ts +++ b/packages/cli/src/commands/hook.ts @@ -118,26 +118,17 @@ export function printClaudeHookConfig(): void { const MCP_CLIENTS = ['claude', 'codex', 'cursor'] as const; type McpClient = typeof MCP_CLIENTS[number]; -function buildMcpConfigSnippet(client: McpClient, aiDir: string): object { +function printMcpConfig(client: McpClient, aiDir: string): void { const serverEntry = { command: 'charter', args: aiDir !== '.ai' ? ['serve', '--ai-dir', aiDir] : ['serve'], }; - if (client === 'claude') { - // .claude/settings.json shape - return { mcpServers: { charter: serverEntry } }; - } - // .mcp.json shape — used by Codex and Cursor - return { mcpServers: { charter: serverEntry } }; -} - -function printMcpConfig(client: McpClient, aiDir: string): void { - const snippet = buildMcpConfigSnippet(client, aiDir); + const snippet = { mcpServers: { charter: serverEntry } }; console.log(JSON.stringify(snippet, null, 2)); if (client === 'claude') { console.error(' Paste the mcpServers entry into .claude/settings.json (or settings.local.json).'); } else { - console.error(` Paste the mcpServers entry into .mcp.json at the repo root.`); + console.error(' Paste the mcpServers entry into .mcp.json at the repo root.'); } } @@ -152,7 +143,8 @@ export async function hookCommand(options: CLIOptions, args: string[]): Promise< const wantMcpConfig = args.includes('--mcp-config'); if (wantMcpConfig) { - const clientFlag = args[args.indexOf('--client') + 1] as McpClient | undefined; + const clientIdx = args.indexOf('--client'); + const clientFlag = clientIdx !== -1 ? args[clientIdx + 1] : undefined; const client: McpClient = MCP_CLIENTS.includes(clientFlag as McpClient) ? clientFlag as McpClient : 'claude'; const aiDir = getFlag(args, '--ai-dir') || '.ai'; printMcpConfig(client, aiDir);