From a7e00a97dca45fa53637e9bff07f44be05551cac Mon Sep 17 00:00:00 2001 From: Justin Ling Date: Thu, 30 Jul 2026 23:21:16 +0800 Subject: [PATCH] Report doctor capabilities separately --- docs/content/cli.mdx | 20 ++- scripts/cli-args.mjs | 16 +- scripts/doctor.mjs | 298 ++++++++++++++++++++++++++++++------ scripts/present.mjs | 9 +- tests/cli-args.test.mjs | 9 +- tests/doctor.test.mjs | 150 ++++++++++++------ tests/present-help.test.mjs | 6 +- 7 files changed, 399 insertions(+), 109 deletions(-) diff --git a/docs/content/cli.mdx b/docs/content/cli.mdx index ed8edab..283ac04 100644 --- a/docs/content/cli.mdx +++ b/docs/content/cli.mdx @@ -80,16 +80,26 @@ npx diffsplain \ ## Check your setup -Run the doctor command to see Node, Git, GitHub CLI, and coding agent paths and -versions: +Run the doctor command to see the separate local review, agent note, and pull +request lookup capabilities: ```sh npx diffsplain doctor ``` -The report lists every supported agent, even when it is not installed. Its -status says when no agent is available and whether pull request lookup can use -`gh`. +The report lists every supported agent, even when it is not installed. It keeps +installation, compatibility, authentication, and smoke-test results separate. +Missing agents do not stop a plain local review with `--no-agent`. + +Use JSON for setup checks in scripts or cloud jobs: + +```sh +npx diffsplain doctor --json +``` + +The normal check does not send prompts to a provider. `--deep` runs local +`--help` checks for installed providers after a warning; it does not send a +provider prompt. ## Run from source diff --git a/scripts/cli-args.mjs b/scripts/cli-args.mjs index 87b1e97..f02cb9d 100644 --- a/scripts/cli-args.mjs +++ b/scripts/cli-args.mjs @@ -40,7 +40,8 @@ Show the current checkout against its default branch: diffsplain Commands: - doctor Check Git, GitHub CLI, and coding agents + doctor [--json] [--deep] + Check review, agent, and pull request capabilities Targets: --branch NAME Show a remote branch against its default branch @@ -70,6 +71,7 @@ Agent fallback: Examples: diffsplain diffsplain doctor + diffsplain doctor --json diffsplain --repo owner/project --pr 42 diffsplain owner/project --branch feature/search diffsplain --agent claude`; @@ -123,8 +125,16 @@ export function parseCliArgs( } = {}, ) { if (rawArgs[0] === 'doctor') { - if (rawArgs.length > 1) fail('doctor does not take arguments or options'); - return { doctor: true }; + const options = new Set(rawArgs.slice(1)); + for (const option of options) { + if (!['--json', '--deep'].includes(option)) { + fail('doctor only accepts --json and --deep'); + } + } + if (options.size !== rawArgs.length - 1) { + fail('doctor options can only be passed once'); + } + return { doctor: { json: options.has('--json'), deep: options.has('--deep') } }; } const options = new Map(); diff --git a/scripts/doctor.mjs b/scripts/doctor.mjs index 599a670..c4ec7cd 100644 --- a/scripts/doctor.mjs +++ b/scripts/doctor.mjs @@ -12,6 +12,7 @@ const agentLabels = { cursor: 'Cursor', opencode: 'OpenCode', }; +const minimumNodeVersion = [22, 13, 0]; function firstLine(value) { return value @@ -21,32 +22,56 @@ function firstLine(value) { .find(Boolean); } -function commandVersion(command) { - const result = spawnSync(command, ['--version'], { +function commandResult(command, args, env) { + return spawnSync(command, args, { encoding: 'utf8', + env, timeout: 5_000, windowsHide: true, }); +} + +function commandVersion(command, env) { + const result = commandResult(command, ['--version'], env); if (result.error || result.status !== 0) return undefined; return firstLine(`${result.stdout || ''}\n${result.stderr || ''}`); } -async function inspectDependency( - label, - command, - { - env, - platform, - }, -) { +function commandSucceeds(command, args, env) { + const result = commandResult(command, args, env); + return !result.error && result.status === 0; +} + +function parseNodeVersion(version) { + const match = version.match(/^v?(\d+)\.(\d+)\.(\d+)/); + if (!match) return undefined; + return match.slice(1).map(Number); +} + +function supportsNodeVersion(version) { + const current = parseNodeVersion(version); + if (!current) return false; + for (const [index, part] of current.entries()) { + if (part !== minimumNodeVersion[index]) { + return part > minimumNodeVersion[index]; + } + } + return true; +} + +async function inspectDependency(label, command, { env, platform }) { const path = await findCommand(command, { env, platform }); - if (!path) return { label, command, installed: false }; + if (!path) { + return { label, command, installed: false, compatible: 'not-checked' }; + } + const version = commandVersion(path, env); return { label, command, installed: true, path, - version: commandVersion(path), + version, + compatible: version ? 'not-verified' : 'unknown', }; } @@ -56,64 +81,243 @@ function dependencyLine(dependency) { return ` ✗ ${label} not found (${dependency.command})`; } const mark = dependency.version ? '✓' : '!'; - const version = dependency.version || 'version unavailable'; - return ` ${mark} ${label} ${version} (${dependency.path})`; + return ` ${mark} ${label} ${dependency.version || 'version unavailable'} (${dependency.path})`; } -function joinedAgentNames(agents) { - return agents.map((agent) => agent.label).join(', '); +function stateLine(name, value) { + return ` ${name.padEnd(14)} ${value}`; } -export async function doctorReport({ - env = process.env, - platform = process.platform, - architecture = process.arch, - nodeVersion = process.version, - nodePath = process.execPath, -} = {}) { +function capabilityLines(name, capability) { + return [ + ` ${name}`, + stateLine( + 'installed', + typeof capability.installed === 'boolean' + ? yesNo(capability.installed) + : capability.installed, + ), + stateLine('compatible', capability.compatible), + stateLine('authenticated', capability.authenticated), + stateLine('smoke test', capability.smokeTest), + ]; +} + +function yesNo(value) { + return value ? 'yes' : 'no'; +} + +function localSmokeTest(dependency, deep, env) { + if (!deep) return 'not-run'; + if (!dependency.installed) return 'not-run'; + return commandSucceeds(dependency.path, ['--help'], env) + ? 'passed (local command only)' + : 'failed (local command only)'; +} + +function providerCapability(agent, deep, env) { + return { + installed: agent.installed, + compatible: agent.compatible, + authenticated: 'not-checked', + smokeTest: localSmokeTest(agent, deep, env), + }; +} + +async function inspectDependencies(env, platform) { const [git, gh, ...agents] = await Promise.all([ inspectDependency('Git', 'git', { env, platform }), inspectDependency('gh', 'gh', { env, platform }), ...codingAgents.map((agent) => - inspectDependency( - agentLabels[agent], - codingAgentBinary(agent, { env }), - { env, platform }, - ), + inspectDependency(agentLabels[agent], codingAgentBinary(agent, { env }), { + env, + platform, + }), ), ]); - const installedAgents = agents.filter((agent) => agent.installed); - const agentCount = installedAgents.length + return { git, gh, agents }; +} + +function coreInstalled(git, nodePath) { + return git.installed && Boolean(nodePath); +} + +function coreReady(git, nodeSupported) { + return git.installed && Boolean(git.version) && nodeSupported; +} + +function coreReviewCapability(git, nodeSupported, nodePath, deep, env) { + const ready = coreReady(git, nodeSupported); + return { + installed: coreInstalled(git, nodePath), + compatible: yesNo(ready), + authenticated: 'not-required', + smokeTest: localSmokeTest(git, deep, env), + ready, + }; +} + +function agentNoteCapabilities(agents, deep, env) { + return Object.fromEntries( + codingAgents.map((agent, index) => [ + agent, + providerCapability(agents[index], deep, env), + ]), + ); +} + +function authenticationState(gh, env) { + if (!gh.installed) return 'not-checked'; + return commandSucceeds(gh.path, ['auth', 'status', '--active'], env) + ? 'passed' + : 'failed'; +} + +function pullRequestCapability(gh, deep, env) { + return { + installed: gh.installed, + compatible: gh.compatible, + authenticated: authenticationState(gh, env), + smokeTest: localSmokeTest(gh, deep, env), + }; +} + +function machineReport({ + deep, + platform, + architecture, + nodeVersion, + nodePath, + nodeSupported, + git, + gh, + agents, + capabilities, +}) { + return { + schemaVersion: 1, + deep, + platform: { name: platform, architecture }, + dependencies: { + node: { + installed: true, + version: nodeVersion, + path: nodePath, + compatible: nodeSupported ? 'yes' : 'no', + }, + git, + gh, + agents: Object.fromEntries( + codingAgents.map((agent, index) => [agent, agents[index]]), + ), + }, + capabilities, + ready: capabilities.coreReview.ready, + }; +} + +function installedCountLabel(installedAgents) { + return installedAgents.length ? `${installedAgents.length} installed` : 'none installed'; +} + +function agentCapabilityLines(agent, capabilities) { + return capabilityLines( + `Agent notes: ${agentLabels[agent]}`, + capabilities.agentNotes[agent], + ); +} + +function appendDoctorNotes(lines, installedAgents, deep) { + if (!installedAgents.length) { + lines.push(' No agent is required for a plain local review; use --no-agent.'); + } + if (!deep) { + lines.push( + ' Smoke tests were not run. Use doctor --deep for local command checks.', + ); + } +} + +function reportText({ + deep, + platform, + architecture, + nodeVersion, + nodePath, + git, + gh, + agents, + capabilities, +}) { + const installedAgents = agents.filter((agent) => agent.installed); const lines = [ 'Diffsplain doctor', '', 'Dependencies', - ` ✓ ${'Node'.padEnd(9)} ${nodeVersion} (${nodePath})`, + dependencyLine({ + label: 'Node', + command: 'node', + installed: true, + path: nodePath, + version: nodeVersion, + }), dependencyLine(git), dependencyLine(gh), '', - `Coding agents (${agentCount})`, + `Coding agents (${installedCountLabel(installedAgents)})`, ...agents.map(dependencyLine), '', - 'Status', - git.installed - ? ' ✓ Git reviews are ready.' - : ' ✗ Git is not installed.', - installedAgents.length - ? ` ✓ Agent notes are ready with ${joinedAgentNames(installedAgents)}.` - : ' ✗ No supported coding agent is installed.', - gh.installed - ? ' ✓ Pull request lookup is ready with gh.' - : ' ✗ gh is not installed; pull request lookup is unavailable.', + 'Capabilities', + ...capabilityLines('Plain local review', { + ...capabilities.coreReview, + installed: yesNo(capabilities.coreReview.installed), + }), + ...codingAgents.flatMap((agent) => + agentCapabilityLines(agent, capabilities), + ), + ...capabilityLines('Pull request lookup', { + ...capabilities.pullRequestLookup, + installed: yesNo(capabilities.pullRequestLookup.installed), + }), ` Platform: ${platform} ${architecture}`, ]; - if (!installedAgents.length) { - lines.push(' Use --no-agent to run without agent notes.'); - } + appendDoctorNotes(lines, installedAgents, deep); + return lines.join('\n'); +} + +export async function doctorReport({ + env = process.env, + platform = process.platform, + architecture = process.arch, + nodeVersion = process.version, + nodePath = process.execPath, + deep = false, +} = {}) { + const { git, gh, agents } = await inspectDependencies(env, platform); + const nodeSupported = supportsNodeVersion(nodeVersion); + const capabilities = { + coreReview: coreReviewCapability(git, nodeSupported, nodePath, deep, env), + agentNotes: agentNoteCapabilities(agents, deep, env), + pullRequestLookup: pullRequestCapability(gh, deep, env), + }; + const reportOptions = { + deep, + platform, + architecture, + nodeVersion, + nodePath, + nodeSupported, + git, + gh, + agents, + capabilities, + }; + const json = machineReport(reportOptions); return { - text: lines.join('\n'), - ready: git.installed && installedAgents.length > 0, + text: reportText(reportOptions), + json, + ready: json.ready, }; } diff --git a/scripts/present.mjs b/scripts/present.mjs index a532e56..d130c3e 100755 --- a/scripts/present.mjs +++ b/scripts/present.mjs @@ -43,8 +43,13 @@ if (cli.version) { process.exit(0); } if (cli.doctor) { - const report = await doctorReport(); - console.log(report.text); + if (cli.doctor.deep) { + console.error( + 'Warning: deep checks run local provider commands. They do not send a provider prompt.', + ); + } + const report = await doctorReport({ deep: cli.doctor.deep }); + console.log(cli.doctor.json ? JSON.stringify(report.json, null, 2) : report.text); process.exit(report.ready ? 0 : 1); } diff --git a/tests/cli-args.test.mjs b/tests/cli-args.test.mjs index 68d3d65..5271523 100644 --- a/tests/cli-args.test.mjs +++ b/tests/cli-args.test.mjs @@ -132,10 +132,15 @@ test('accepts short help and version flags', () => { }); test('accepts the doctor command without review options', () => { - assert.deepEqual(parseCliArgs(['doctor']), { doctor: true }); + assert.deepEqual(parseCliArgs(['doctor']), { + doctor: { json: false, deep: false }, + }); + assert.deepEqual(parseCliArgs(['doctor', '--json', '--deep']), { + doctor: { json: true, deep: true }, + }); assert.throws( () => parseCliArgs(['doctor', '--no-agent']), - /doctor does not take arguments or options/i, + /doctor only accepts --json and --deep/i, ); }); diff --git a/tests/doctor.test.mjs b/tests/doctor.test.mjs index 5c75f34..456fe57 100644 --- a/tests/doctor.test.mjs +++ b/tests/doctor.test.mjs @@ -1,11 +1,6 @@ import assert from 'node:assert/strict'; import { spawnSync } from 'node:child_process'; -import { - chmod, - mkdtemp, - rm, - writeFile, -} from 'node:fs/promises'; +import { chmod, mkdtemp, rm, writeFile } from 'node:fs/promises'; import { tmpdir } from 'node:os'; import { join } from 'node:path'; import test from 'node:test'; @@ -13,68 +8,125 @@ import { doctorReport } from '../scripts/doctor.mjs'; const script = new URL('../scripts/present.mjs', import.meta.url).pathname; -async function fakeCommand(directory, name, version) { +async function fakeCommand( + directory, + name, + { authStatus = 1, versionStatus = 0 } = {}, +) { const path = join(directory, name); await writeFile( path, `#!/bin/sh -printf '%s\\n' ${JSON.stringify(version)} +if [ "$1" = "--version" ]; then printf '%s\\n' ${JSON.stringify(`${name} version test`)}; exit ${versionStatus}; fi +if [ "$1" = "--help" ]; then exit 0; fi +if [ "$1" = "auth" ] && [ "$2" = "status" ] && [ "$3" = "--active" ]; then exit ${authStatus}; fi +exit 9 `, ); await chmod(path, 0o755); } -test('reports versions and each supported coding agent', async () => { +async function withCommands(commands, callback) { const directory = await mkdtemp(join(tmpdir(), 'diffsplain-doctor-')); try { - await fakeCommand(directory, 'git', 'git version 2.50.0'); - await fakeCommand(directory, 'gh', 'gh version 2.80.0'); - await fakeCommand(directory, 'cursor-agent', '2026.07.29-test'); - - const report = await doctorReport({ - env: { PATH: directory }, - platform: process.platform, - architecture: 'test-arch', - nodeVersion: 'v22.13.0', - nodePath: '/test/node', - }); - - assert.equal(report.ready, true); - assert.match(report.text, /Node\s+v22\.13\.0/); - assert.match(report.text, /Git\s+git version 2\.50\.0/); - assert.match(report.text, /gh\s+gh version 2\.80\.0/); - assert.match(report.text, /Coding agents \(1 installed\)/); - assert.match(report.text, /✓ Cursor\s+2026\.07\.29-test/); - for (const agent of ['Codex', 'Claude', 'Copilot', 'OpenCode']) { - assert.match(report.text, new RegExp(`✗ ${agent}\\s+not found`)); - } - assert.match(report.text, /Agent notes are ready with Cursor/); - assert.match(report.text, /Platform: \S+ test-arch/); + await Promise.all(Object.entries(commands).map(([name, options]) => fakeCommand(directory, name, options))); + await callback(directory); } finally { await rm(directory, { recursive: true, force: true }); } +} + +function options(directory, deep = false) { + return { + env: { PATH: directory }, + platform: process.platform, + architecture: 'test-arch', + nodeVersion: 'v22.13.0', + nodePath: '/test/node', + deep, + }; +} + +test('reports core review independently from optional capabilities', async () => { + await withCommands({ git: {} }, async (directory) => { + const report = await doctorReport(options(directory)); + + assert.equal(report.ready, true); + assert.equal(report.json.capabilities.coreReview.compatible, 'yes'); + assert.equal(report.json.capabilities.coreReview.smokeTest, 'not-run'); + assert.equal(report.json.capabilities.agentNotes.codex.installed, false); + assert.equal(report.json.capabilities.pullRequestLookup.installed, false); + assert.equal(report.json.capabilities.agentNotes.codex.smokeTest, 'not-run'); + assert.match(report.text, /No agent is required for a plain local review; use --no-agent/); + }); +}); + +test('keeps installation, compatibility, authentication, and smoke tests separate', async () => { + await withCommands( + { git: {}, gh: { authStatus: 0 }, 'cursor-agent': {} }, + async (directory) => { + const report = await doctorReport(options(directory)); + const cursor = report.json.capabilities.agentNotes.cursor; + + assert.equal(cursor.installed, true); + assert.equal(cursor.compatible, 'not-verified'); + assert.equal(cursor.authenticated, 'not-checked'); + assert.equal(cursor.smokeTest, 'not-run'); + assert.equal(report.json.capabilities.pullRequestLookup.authenticated, 'passed'); + assert.match(report.text, /Agent notes: Cursor[\s\S]*authenticated\s+not-checked/); + }, + ); }); -test('clearly reports when no supported coding agent is installed', () => { +test('runs only explicit deep local command checks', async () => { + await withCommands({ git: {}, 'cursor-agent': {} }, async (directory) => { + const report = await doctorReport(options(directory, true)); + + assert.equal( + report.json.capabilities.agentNotes.cursor.smokeTest, + 'passed (local command only)', + ); + assert.equal( + report.json.capabilities.coreReview.smokeTest, + 'passed (local command only)', + ); + }); +}); + +test('requires a successful Git version probe for core readiness', async () => { + await withCommands({ git: { versionStatus: 1 } }, async (directory) => { + const report = await doctorReport(options(directory)); + + assert.equal(report.json.capabilities.coreReview.installed, true); + assert.equal(report.json.capabilities.coreReview.compatible, 'no'); + assert.equal(report.json.capabilities.coreReview.ready, false); + assert.equal(report.ready, false); + }); +}); + +test('prints a stable JSON report and warns before deep checks', async () => { + await withCommands({ git: {} }, async (directory) => { + const result = spawnSync(process.execPath, [script, 'doctor', '--json', '--deep'], { + encoding: 'utf8', + env: { ...process.env, PATH: directory, CODEX_BIN: '', CLAUDE_BIN: '', COPILOT_BIN: '', CURSOR_BIN: '', OPENCODE_BIN: '' }, + }); + + assert.equal(result.status, 0, result.stderr); + assert.match(result.stderr, /Warning: deep checks run local provider commands/); + const report = JSON.parse(result.stdout); + assert.equal(report.schemaVersion, 1); + assert.equal(report.deep, true); + assert.equal(report.capabilities.coreReview.ready, true); + assert.deepEqual(Object.keys(report.capabilities.agentNotes), ['codex', 'claude', 'copilot', 'cursor', 'opencode']); + }); +}); + +test('fails only when core local review is unavailable', () => { const result = spawnSync(process.execPath, [script, 'doctor'], { encoding: 'utf8', - env: { - ...process.env, - PATH: '', - CODEX_BIN: '', - CLAUDE_BIN: '', - COPILOT_BIN: '', - CURSOR_BIN: '', - OPENCODE_BIN: '', - }, + env: { ...process.env, PATH: '', CODEX_BIN: '', CLAUDE_BIN: '', COPILOT_BIN: '', CURSOR_BIN: '', OPENCODE_BIN: '' }, }); assert.equal(result.status, 1, result.stderr); - assert.match(result.stdout, /^Diffsplain doctor/m); - assert.match(result.stdout, /Coding agents \(none installed\)/); - assert.match( - result.stdout, - /No supported coding agent is installed/, - ); - assert.match(result.stdout, /gh\s+not found/); + assert.match(result.stdout, /Plain local review/); }); diff --git a/tests/present-help.test.mjs b/tests/present-help.test.mjs index 28167bc..b428357 100644 --- a/tests/present-help.test.mjs +++ b/tests/present-help.test.mjs @@ -13,7 +13,11 @@ test('prints help with either help flag', () => { }); assert.equal(result.status, 0, result.stderr); assert.match(result.stdout, /^Usage: diffsplain/m); - assert.match(result.stdout, /doctor\s+Check Git/); + assert.match(result.stdout, /doctor \[--json\] \[--deep\]/); + assert.match( + result.stdout, + /Check review, agent, and pull request capabilities/, + ); assert.match(result.stdout, /-v, --version/); } });