diff --git a/src/cli/commands/doctor.ts b/src/cli/commands/doctor.ts index 1cb93f7..d50aa97 100644 --- a/src/cli/commands/doctor.ts +++ b/src/cli/commands/doctor.ts @@ -10,7 +10,8 @@ import type { Command } from 'commander'; import { AdapterResolver } from '../../adapters/resolver.ts'; -import type { SiteConfig } from '../../types.ts'; +import { WpApiError } from '../../adapters/types.ts'; +import { ExitCode, type SiteConfig } from '../../types.ts'; import { loadConfig, resolveActiveSite } from '../utils/config.ts'; import { error, info, printJson, warn } from '../utils/output.ts'; @@ -77,6 +78,12 @@ interface DoctorIssue { severity: 'error' | 'warning' | 'info'; message: string; fix?: string; + /** + * Structured classification for error-severity issues, used to pick an + * exit code. Set this explicitly at each push site rather than pattern- + * matching `message` later — message text is for humans and can change. + */ + code?: 'auth' | 'network' | 'generic'; } export function registerDoctorCommand(program: Command): void { @@ -99,9 +106,11 @@ export function registerDoctorCommand(program: Command): void { if (siteNames.length === 0) { error('No sites configured. Run `localpress init` to add one.'); - process.exit(3); + process.exit(ExitCode.ConfigError); } + let worstExit: number = ExitCode.Success; + for (const name of siteNames) { const site = config.sites[name]; if (!site) continue; @@ -122,23 +131,33 @@ export function registerDoctorCommand(program: Command): void { connectionOk = true; } } catch (err) { - const msg = err instanceof Error ? err.message : String(err); - if (msg.includes('401') || msg.includes('Unauthorized')) { + // WpApiError means the server responded (with a structured HTTP + // status) — so this is an application-level error, not a + // connectivity failure. Anything else means fetch() itself never + // got a response (DNS failure, connection refused, timeout, etc.). + // We key off that distinction — and the exact HTTP status for auth + // — rather than pattern-matching error message text, which varies + // by runtime (e.g. Bun's fetch() doesn't include Node-style codes + // like ECONNREFUSED/ENOTFOUND in its error messages). + if (err instanceof WpApiError && err.status === 401) { issues.push({ severity: 'error', message: 'Authentication failed — Application Password rejected', fix: 'Run `localpress sites remove ` then `localpress init` to re-enter credentials', + code: 'auth', }); - } else if (msg.includes('ENOTFOUND') || msg.includes('ECONNREFUSED')) { + } else if (err instanceof WpApiError) { issues.push({ severity: 'error', - message: `Cannot reach ${site.url} — check the URL and your network connection`, - fix: 'Verify the site URL with `localpress sites` and update if needed', + message: `REST API error: ${err.message}`, + code: 'generic', }); } else { issues.push({ severity: 'error', - message: `REST API error: ${msg}`, + message: `Cannot reach ${site.url} — check the URL and your network connection`, + fix: 'Verify the site URL with `localpress sites` and update if needed', + code: 'network', }); } } @@ -176,6 +195,7 @@ export function registerDoctorCommand(program: Command): void { severity: 'error', message: 'sharp installation completed but module still not found. Try restarting your shell.', + code: 'generic', }); } } else { @@ -183,6 +203,7 @@ export function registerDoctorCommand(program: Command): void { severity: 'error', message: 'Auto-install failed. Neither bun nor npm is available.', fix: 'Install bun or npm, then run `localpress doctor --fix` again', + code: 'generic', }); } } else { @@ -191,6 +212,7 @@ export function registerDoctorCommand(program: Command): void { message: 'sharp is not installed — optimize, convert, resize, and remove-bg will not work', fix: 'Run `localpress doctor --fix` to auto-install, or manually: `bun install -g sharp`', + code: 'generic', }); } } @@ -262,9 +284,7 @@ export function registerDoctorCommand(program: Command): void { } } // For auth errors, offer to re-test with updated credentials. - const authIssue = issues.find( - (i) => i.severity === 'error' && i.message.includes('Authentication'), - ); + const authIssue = issues.find((i) => i.severity === 'error' && i.code === 'auth'); if (authIssue && connectionOk === false) { info('\n To update credentials, run:'); info(` localpress sites remove ${name}`); @@ -272,6 +292,15 @@ export function registerDoctorCommand(program: Command): void { } } + // -- Exit code accumulation -------------------------------------------- + // Map this site's error-severity issues to an exit code and track the + // worst code seen across all sites (for --all-sites). The exit codes + // this loop can produce (Success=0, GenericError=1, NetworkError=4, + // AuthError=5) are numbered by increasing severity, so a plain max + // is correct — no need for an explicit precedence chain. + const siteExitCode = issueListToExitCode(issues); + worstExit = Math.max(worstExit, siteExitCode); + // -- Output ------------------------------------------------------------ if (parentOpts.json) { printJson({ @@ -332,9 +361,40 @@ export function registerDoctorCommand(program: Command): void { } } } + + // Set process.exitCode after the loop so --all-sites finishes all sites. + if (worstExit !== ExitCode.Success) { + process.exitCode = worstExit; + } }); } +// -- Exit code helpers -------------------------------------------------------- + +/** + * Map a site's issue list to the appropriate ExitCode, keyed off each + * issue's structured `code` (not message text — see DoctorIssue.code). + * Priority: Auth(5) > Network(4) > Generic(1) > Success(0). + */ +function issueListToExitCode(issues: DoctorIssue[]): number { + const errorIssues = issues.filter((i) => i.severity === 'error'); + + if (errorIssues.some((i) => i.code === 'auth')) { + return ExitCode.AuthError; + } + + if (errorIssues.some((i) => i.code === 'network')) { + return ExitCode.NetworkError; + } + + if (errorIssues.length > 0) { + // Any other error (e.g. sharp missing, or an uncategorized REST error). + return ExitCode.GenericError; + } + + return ExitCode.Success; +} + // -- Plugin detection --------------------------------------------------------- interface WpPluginResponse { diff --git a/test/fixtures/doctor-unreachable.ts b/test/fixtures/doctor-unreachable.ts new file mode 100644 index 0000000..c08ef38 --- /dev/null +++ b/test/fixtures/doctor-unreachable.ts @@ -0,0 +1,39 @@ +/** + * Subprocess fixture for doctor-exit-code tests. + * + * Runs `localpress doctor` with a mock fetch that always throws TypeError + * (simulating an unreachable host), so tests can exercise the NetworkError + * exit-code path without real network I/O and without setting process.exitCode + * in the test runner's own process (which would poison bun's test exit code). + * + * Usage (via Bun.spawnSync): + * bun run test/fixtures/doctor-unreachable.ts [doctor args...] + * + * The site config is read from XDG_CONFIG_HOME, which the caller seeds before + * spawning this script. + */ + +import { Command, Option } from 'commander'; +import { registerDoctorCommand } from '../../src/cli/commands/doctor.ts'; +import { setOutputOptions } from '../../src/cli/utils/output.ts'; + +// Mock fetch before any module that calls it is initialized. +globalThis.fetch = (async () => { + throw new TypeError('fetch failed'); +}) as unknown as typeof fetch; + +const program = new Command(); +program + .name('localpress') + .exitOverride() + .addOption(new Option('--site ', 'override the active site for this command')) + .addOption(new Option('--json', 'machine-readable JSON output').default(false)) + .addOption(new Option('--quiet', 'errors only; suppress info messages').default(false)) + .hook('preAction', (thisCommand) => { + const opts = thisCommand.opts(); + setOutputOptions({ json: Boolean(opts.json), quiet: Boolean(opts.quiet) }); + }); + +registerDoctorCommand(program); + +await program.parseAsync(process.argv.slice(2), { from: 'user' }); diff --git a/test/unit/doctor-exit-code.test.ts b/test/unit/doctor-exit-code.test.ts new file mode 100644 index 0000000..fd215a7 --- /dev/null +++ b/test/unit/doctor-exit-code.test.ts @@ -0,0 +1,169 @@ +/** + * doctor exit-code contract tests. + * + * Exercises the exit-code behaviour added in #267 (OSS-1354): + * - no sites configured → exit 3 (ConfigError) + * - unreachable site → exit 4 (NetworkError), connectionOk:false in --json + * - healthy site → exit 0 (manual / integration tests only — needs live WP) + * + * All cases use Bun.spawnSync to test exit codes in a subprocess, keeping + * process.exitCode in the test runner's own process always zero. In-process + * invocations that set process.exitCode would cause bun test to exit with a + * non-zero code even when all assertions pass. + * + * For unreachable-site cases we use test/fixtures/doctor-unreachable.ts, a + * small harness that mocks globalThis.fetch before running doctor. This gives + * us deterministic "connection failed" behaviour regardless of CI network + * configuration — no real sockets needed. + */ + +import { describe, expect, test } from 'bun:test'; +import { mkdirSync, mkdtempSync, rmSync, writeFileSync } from 'node:fs'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; + +const CLI_ENTRY = join(process.cwd(), 'src', 'cli', 'index.ts'); +const UNREACHABLE_FIXTURE = join(process.cwd(), 'test', 'fixtures', 'doctor-unreachable.ts'); +const SITE_NAME = 'testsite'; + +/** + * Seed a minimal localpress config file in the given config dir so + * `loadConfig()` finds a site without any real network calls. + */ +function seedConfig( + configDir: string, + sites: Record, + activeSite = SITE_NAME, +): void { + mkdirSync(join(configDir, 'localpress'), { recursive: true }); + const config = { + version: 1, + activeSite, + sites: Object.fromEntries( + Object.entries(sites).map(([name, s]) => [ + name, + { + name, + url: s.url, + username: s.username ?? 'admin', + appPassword: s.appPassword ?? 'fake fake fake fake fake fake', + createdAt: new Date(0).toISOString(), + }, + ]), + ), + }; + writeFileSync( + join(configDir, 'localpress', 'config.json'), + JSON.stringify(config, null, 2), + 'utf8', + ); +} + +/** Spawn the CLI (no site seeded) with an ephemeral config dir. */ +function runCli( + args: string[], + extraEnv: Record = {}, +): { stdout: string; stderr: string; exitCode: number } { + const configDir = mkdtempSync(join(tmpdir(), 'localpress-doctor-test-')); + try { + const result = Bun.spawnSync(['bun', 'run', CLI_ENTRY, ...args], { + env: { ...process.env, XDG_CONFIG_HOME: configDir, ...extraEnv }, + timeout: 30_000, + }); + return { + stdout: result.stdout.toString(), + stderr: result.stderr.toString(), + exitCode: result.exitCode ?? 1, + }; + } finally { + rmSync(configDir, { recursive: true, force: true }); + } +} + +/** + * Spawn the unreachable-fixture script with a pre-seeded config dir so + * globalThis.fetch is mocked before doctor runs. The fixture exits with + * process.exitCode set by the doctor action — no pollution of the test + * runner's own process.exitCode. + */ +function runUnreachable( + doctorArgs: string[], + sites: Record = { [SITE_NAME]: { url: 'https://example.test' } }, + activeSite = SITE_NAME, +): { stdout: string; stderr: string; exitCode: number } { + const configDir = mkdtempSync(join(tmpdir(), 'localpress-doctor-unreachable-')); + try { + seedConfig(configDir, sites, activeSite); + const result = Bun.spawnSync(['bun', 'run', UNREACHABLE_FIXTURE, ...doctorArgs], { + env: { ...process.env, XDG_CONFIG_HOME: configDir }, + timeout: 30_000, + }); + return { + stdout: result.stdout.toString(), + stderr: result.stderr.toString(), + exitCode: result.exitCode ?? 1, + }; + } finally { + rmSync(configDir, { recursive: true, force: true }); + } +} + +// --------------------------------------------------------------------------- +// No sites configured — uses the real CLI entry point, no site seeded. +// --------------------------------------------------------------------------- + +describe('doctor exit codes — no sites configured', () => { + test('doctor with no sites configured exits 3 (ConfigError)', () => { + const { exitCode } = runCli(['doctor']); + expect(exitCode).toBe(3); + }); + + test('doctor --all-sites with no sites configured exits 3 (ConfigError)', () => { + const { exitCode } = runCli(['doctor', '--all-sites']); + expect(exitCode).toBe(3); + }); +}); + +// --------------------------------------------------------------------------- +// Unreachable site — fetch is mocked inside the subprocess fixture so the +// connection check always fails deterministically, regardless of CI network. +// --------------------------------------------------------------------------- + +describe('doctor exit codes — unreachable site', () => { + test('doctor with unreachable site exits 4 (NetworkError)', () => { + const { exitCode } = runUnreachable(['doctor']); + expect(exitCode).toBe(4); + }); + + test('doctor --json with unreachable site exits 4 and emits connectionOk:false', () => { + const { stdout, exitCode } = runUnreachable(['doctor', '--json']); + expect(exitCode).toBe(4); + + const lines = stdout.trim().split('\n').filter(Boolean); + expect(lines.length).toBeGreaterThan(0); + const parsed = JSON.parse(lines[0]); + expect(parsed.connectionOk).toBe(false); + + const errorIssues = (parsed.issues as Array<{ severity: string }>).filter( + (i) => i.severity === 'error', + ); + expect(errorIssues.length).toBeGreaterThan(0); + }); + + test('doctor --all-sites with unreachable site exits 4', () => { + const { exitCode } = runUnreachable(['doctor', '--all-sites'], { + [SITE_NAME]: { url: 'https://example.test' }, + 'second-site': { url: 'https://example2.test' }, + }); + expect(exitCode).toBe(4); + }); + + test('doctor with a second, independent unreachable site exits 4 (NetworkError)', () => { + const { exitCode } = runUnreachable( + ['doctor'], + { 'another-site': { url: 'https://another-example.test' } }, + 'another-site', + ); + expect(exitCode).toBe(4); + }); +});