From fcf8225ad701bee1c0fc9b4d53580880a5d1dfd4 Mon Sep 17 00:00:00 2001 From: "gfargo-horizon-agent[bot]" <294710345+gfargo-horizon-agent[bot]@users.noreply.github.com> Date: Sat, 25 Jul 2026 23:30:30 +0000 Subject: [PATCH 1/8] fix(doctor): set non-zero exit code when error-severity issues are found Previously doctor always exited 0, making it unusable as a CI/scripting gate and silently breaking the health_check MCP tool (which derives doctor.ok from the exit code). Changes: - Import ExitCode from src/types.ts - Replace hardcoded process.exit(3) with process.exit(ExitCode.ConfigError) in the no-sites-configured guard - Add worstExit accumulator before the per-site loop - Add issueListToExitCode() helper mapping error-severity issues to exit codes with explicit precedence: Auth(5) > Network(4) > Generic(1) - Set process.exitCode = worstExit after the loop (not process.exit()) so --all-sites reports all sites before exiting - Safety net: connectionOk===false with no matching issue also yields NetworkError Closes #267 --- src/cli/commands/doctor.ts | 63 ++++++++++++- test/unit/doctor-exit-code.test.ts | 142 +++++++++++++++++++++++++++++ 2 files changed, 203 insertions(+), 2 deletions(-) create mode 100644 test/unit/doctor-exit-code.test.ts diff --git a/src/cli/commands/doctor.ts b/src/cli/commands/doctor.ts index 1cb93f7..ab46fad 100644 --- a/src/cli/commands/doctor.ts +++ b/src/cli/commands/doctor.ts @@ -10,7 +10,7 @@ import type { Command } from 'commander'; import { AdapterResolver } from '../../adapters/resolver.ts'; -import type { SiteConfig } from '../../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'; @@ -99,9 +99,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; @@ -272,6 +274,21 @@ 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). + const siteExitCode = issueListToExitCode(issues, connectionOk); + if (siteExitCode !== ExitCode.Success) { + // Explicit precedence: Auth(5) > Network(4) > Generic(1) + if ( + worstExit === ExitCode.Success || + siteExitCode === ExitCode.AuthError || + (siteExitCode === ExitCode.NetworkError && worstExit === ExitCode.GenericError) + ) { + worstExit = siteExitCode; + } + } + // -- Output ------------------------------------------------------------ if (parentOpts.json) { printJson({ @@ -332,9 +349,51 @@ 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. + * Priority: Auth(5) > Network(4) > Generic(1) > Success(0). + * Also treats connectionOk===false without a matching issue as NetworkError. + */ +function issueListToExitCode(issues: DoctorIssue[], connectionOk: boolean): number { + const errorIssues = issues.filter((i) => i.severity === 'error'); + + if (errorIssues.length === 0) { + // Safety net: if connection is down but nothing was pushed to issues, still + // treat it as a network error (e.g. adapter returned falsy). + if (!connectionOk) { + return ExitCode.NetworkError; + } + return ExitCode.Success; + } + + // Check for auth error first (highest priority). + if (errorIssues.some((i) => i.message.includes('Authentication'))) { + return ExitCode.AuthError; + } + + // Check for network/connection errors. + if ( + errorIssues.some( + (i) => i.message.includes('Cannot reach') || i.message.includes('REST API error'), + ) + ) { + return ExitCode.NetworkError; + } + + // Any other error (e.g. sharp missing). + return ExitCode.GenericError; +} + // -- Plugin detection --------------------------------------------------------- interface WpPluginResponse { diff --git a/test/unit/doctor-exit-code.test.ts b/test/unit/doctor-exit-code.test.ts new file mode 100644 index 0000000..a34004e --- /dev/null +++ b/test/unit/doctor-exit-code.test.ts @@ -0,0 +1,142 @@ +/** + * 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) + * + * Technique: spawnSync + isolated XDG_CONFIG_HOME (same pattern as + * cli-error-handling.test.ts). Unreachable URLs use 127.0.0.1:1 to force + * an immediate ECONNREFUSED without touching DNS. + */ + +import { describe, expect, test } from 'bun:test'; +import { spawnSync } from 'node:child_process'; +import { mkdtempSync, rmSync, writeFileSync, mkdirSync } from 'node:fs'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; + +const CLI_ENTRY = join(process.cwd(), 'src', 'cli', 'index.ts'); + +/** Spawn the CLI with an ephemeral config dir. No site is seeded by default. */ +function runCli( + args: string[], + extraEnv: Record = {}, +): { stdout: string; stderr: string; exitCode: number } { + const configDir = mkdtempSync(join(tmpdir(), 'localpress-doctor-test-')); + try { + const result = spawnSync('bun', ['run', CLI_ENTRY, ...args], { + encoding: 'utf-8', + env: { ...process.env, XDG_CONFIG_HOME: configDir, ...extraEnv }, + timeout: 30_000, + }); + return { + stdout: result.stdout ?? '', + stderr: result.stderr ?? '', + exitCode: result.status ?? 1, + }; + } finally { + rmSync(configDir, { recursive: true, force: true }); + } +} + +/** + * Spawn the CLI with a pre-seeded config file pointing at a fake site URL. + * The config is the minimal shape accepted by loadConfig(). + */ +function runCliWithSite( + args: string[], + siteUrl: string, +): { stdout: string; stderr: string; exitCode: number } { + const configDir = mkdtempSync(join(tmpdir(), 'localpress-doctor-site-test-')); + try { + // Write a minimal config.json understood by src/cli/utils/config.ts. + const localpressDir = join(configDir, 'localpress'); + mkdirSync(localpressDir, { recursive: true }); + const config = { + version: 1, + activeSite: 'test-site', + sites: { + 'test-site': { + name: 'test-site', + url: siteUrl, + username: 'admin', + appPassword: 'fake fake fake fake fake fake', + createdAt: new Date().toISOString(), + }, + }, + }; + writeFileSync(join(localpressDir, 'config.json'), JSON.stringify(config), { mode: 0o600 }); + + const result = spawnSync('bun', ['run', CLI_ENTRY, ...args], { + encoding: 'utf-8', + env: { ...process.env, XDG_CONFIG_HOME: configDir }, + timeout: 30_000, + }); + return { + stdout: result.stdout ?? '', + stderr: result.stderr ?? '', + exitCode: result.status ?? 1, + }; + } finally { + rmSync(configDir, { recursive: true, force: true }); + } +} + +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 — connection refused + // ------------------------------------------------------------------------- + test('doctor with unreachable site exits 4 (NetworkError)', () => { + // Port 1 on loopback is reliably refused without DNS involvement. + const { exitCode } = runCliWithSite(['doctor'], 'http://127.0.0.1:1'); + expect(exitCode).toBe(4); + }); + + test('doctor --json with unreachable site exits 4 and emits connectionOk:false', () => { + const { stdout, exitCode } = runCliWithSite(['doctor', '--json'], 'http://127.0.0.1:1'); + expect(exitCode).toBe(4); + + // stdout should contain a JSON object with connectionOk:false + const lines = stdout.trim().split('\n').filter(Boolean); + expect(lines.length).toBeGreaterThan(0); + const parsed = JSON.parse(lines[0]); + expect(parsed.connectionOk).toBe(false); + + // There should be at least one error-severity issue. + 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 } = runCliWithSite(['doctor', '--all-sites'], 'http://127.0.0.1:1'); + expect(exitCode).toBe(4); + }); + + // ------------------------------------------------------------------------- + // Bogus hostname — DNS error (ENOTFOUND) + // ------------------------------------------------------------------------- + test('doctor with bogus hostname exits 4 (NetworkError)', () => { + const { exitCode } = runCliWithSite( + ['doctor'], + 'http://localpress-test-bogus-hostname-that-does-not-exist.invalid', + ); + expect(exitCode).toBe(4); + }); +}); From 1800790d99c4990bd09bdad9b117e66c4142b69f Mon Sep 17 00:00:00 2001 From: "gfargo-horizon-agent[bot]" <294710345+gfargo-horizon-agent[bot]@users.noreply.github.com> Date: Sat, 25 Jul 2026 23:31:44 +0000 Subject: [PATCH 2/8] fix(test): sort node:fs imports alphabetically in doctor-exit-code test --- test/unit/doctor-exit-code.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/unit/doctor-exit-code.test.ts b/test/unit/doctor-exit-code.test.ts index a34004e..d3ef319 100644 --- a/test/unit/doctor-exit-code.test.ts +++ b/test/unit/doctor-exit-code.test.ts @@ -13,7 +13,7 @@ import { describe, expect, test } from 'bun:test'; import { spawnSync } from 'node:child_process'; -import { mkdtempSync, rmSync, writeFileSync, mkdirSync } from 'node:fs'; +import { mkdirSync, mkdtempSync, rmSync, writeFileSync } from 'node:fs'; import { tmpdir } from 'node:os'; import { join } from 'node:path'; From d3f109b98926f4b133da1805ce8ffe97877056f4 Mon Sep 17 00:00:00 2001 From: "gfargo-horizon-agent[bot]" <294710345+gfargo-horizon-agent[bot]@users.noreply.github.com> Date: Sat, 1 Aug 2026 07:52:16 +0000 Subject: [PATCH 3/8] fix(doctor): classify errors structurally instead of by message text MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Add DoctorIssue.code ('auth' | 'network' | 'generic'), set explicitly at each push site instead of pattern-matching issue.message later. - Distinguish network failures from REST API errors by exception type: WpApiError means the server responded (so a 5xx is a generic error, not a network error); anything else means fetch() never got a response at all. This also fixes a real bug where connection-refused and DNS failures were silently misclassified, since Bun's fetch() doesn't put Node-style codes like ECONNREFUSED/ENOTFOUND in its error message text. - Use err.status === 401 (from WpApiError) instead of matching "401"/ "Unauthorized" in the message for auth detection. - Replace the explicit precedence if-chain with Math.max(), since the exit codes produced here (0/1/4/5) are already ordered by severity. - Drop the connectionOk safety net in issueListToExitCode — the REST adapter is always constructed, so the branch was unreachable. - Switch doctor-exit-code.test.ts from node:child_process's spawnSync to Bun.spawnSync: other unit test files replace the whole node:child_process module process-wide via mock.module() without restoring it, which left spawnSync undefined for this file depending on load order and broke CI. --- src/cli/commands/doctor.ts | 87 +++++++++++++++--------------- test/unit/doctor-exit-code.test.ts | 31 ++++++----- 2 files changed, 61 insertions(+), 57 deletions(-) diff --git a/src/cli/commands/doctor.ts b/src/cli/commands/doctor.ts index ab46fad..d50aa97 100644 --- a/src/cli/commands/doctor.ts +++ b/src/cli/commands/doctor.ts @@ -10,6 +10,7 @@ import type { Command } from 'commander'; import { AdapterResolver } from '../../adapters/resolver.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 { @@ -124,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', }); } } @@ -178,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 { @@ -185,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 { @@ -193,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', }); } } @@ -264,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}`); @@ -276,18 +294,12 @@ 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). - const siteExitCode = issueListToExitCode(issues, connectionOk); - if (siteExitCode !== ExitCode.Success) { - // Explicit precedence: Auth(5) > Network(4) > Generic(1) - if ( - worstExit === ExitCode.Success || - siteExitCode === ExitCode.AuthError || - (siteExitCode === ExitCode.NetworkError && worstExit === ExitCode.GenericError) - ) { - worstExit = siteExitCode; - } - } + // 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) { @@ -360,38 +372,27 @@ export function registerDoctorCommand(program: Command): void { // -- Exit code helpers -------------------------------------------------------- /** - * Map a site's issue list to the appropriate ExitCode. + * 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). - * Also treats connectionOk===false without a matching issue as NetworkError. */ -function issueListToExitCode(issues: DoctorIssue[], connectionOk: boolean): number { +function issueListToExitCode(issues: DoctorIssue[]): number { const errorIssues = issues.filter((i) => i.severity === 'error'); - if (errorIssues.length === 0) { - // Safety net: if connection is down but nothing was pushed to issues, still - // treat it as a network error (e.g. adapter returned falsy). - if (!connectionOk) { - return ExitCode.NetworkError; - } - return ExitCode.Success; - } - - // Check for auth error first (highest priority). - if (errorIssues.some((i) => i.message.includes('Authentication'))) { + if (errorIssues.some((i) => i.code === 'auth')) { return ExitCode.AuthError; } - // Check for network/connection errors. - if ( - errorIssues.some( - (i) => i.message.includes('Cannot reach') || i.message.includes('REST API error'), - ) - ) { + if (errorIssues.some((i) => i.code === 'network')) { return ExitCode.NetworkError; } - // Any other error (e.g. sharp missing). - return ExitCode.GenericError; + if (errorIssues.length > 0) { + // Any other error (e.g. sharp missing, or an uncategorized REST error). + return ExitCode.GenericError; + } + + return ExitCode.Success; } // -- Plugin detection --------------------------------------------------------- diff --git a/test/unit/doctor-exit-code.test.ts b/test/unit/doctor-exit-code.test.ts index d3ef319..1297fd3 100644 --- a/test/unit/doctor-exit-code.test.ts +++ b/test/unit/doctor-exit-code.test.ts @@ -6,13 +6,18 @@ * - unreachable site → exit 4 (NetworkError), connectionOk:false in --json * - healthy site → exit 0 (manual / integration tests only — needs live WP) * - * Technique: spawnSync + isolated XDG_CONFIG_HOME (same pattern as - * cli-error-handling.test.ts). Unreachable URLs use 127.0.0.1:1 to force - * an immediate ECONNREFUSED without touching DNS. + * Technique: Bun.spawnSync + isolated XDG_CONFIG_HOME (same pattern as + * cli-error-handling.test.ts). We use Bun.spawnSync rather than + * node:child_process's spawnSync because other unit test files replace the + * whole node:child_process module process-wide via `mock.module()` (see + * preview-server.test.ts / editor-detect.test.ts / quick-view-auth.test.ts) + * without restoring it — depending on file load order that leaves `spawnSync` + * undefined when this file imports it. Bun.spawnSync isn't affected. + * Unreachable URLs use 127.0.0.1:1 to force an immediate ECONNREFUSED without + * touching DNS. */ import { describe, expect, test } from 'bun:test'; -import { spawnSync } from 'node:child_process'; import { mkdirSync, mkdtempSync, rmSync, writeFileSync } from 'node:fs'; import { tmpdir } from 'node:os'; import { join } from 'node:path'; @@ -26,15 +31,14 @@ function runCli( ): { stdout: string; stderr: string; exitCode: number } { const configDir = mkdtempSync(join(tmpdir(), 'localpress-doctor-test-')); try { - const result = spawnSync('bun', ['run', CLI_ENTRY, ...args], { - encoding: 'utf-8', + const result = Bun.spawnSync(['bun', 'run', CLI_ENTRY, ...args], { env: { ...process.env, XDG_CONFIG_HOME: configDir, ...extraEnv }, timeout: 30_000, }); return { - stdout: result.stdout ?? '', - stderr: result.stderr ?? '', - exitCode: result.status ?? 1, + stdout: result.stdout.toString(), + stderr: result.stderr.toString(), + exitCode: result.exitCode ?? 1, }; } finally { rmSync(configDir, { recursive: true, force: true }); @@ -69,15 +73,14 @@ function runCliWithSite( }; writeFileSync(join(localpressDir, 'config.json'), JSON.stringify(config), { mode: 0o600 }); - const result = spawnSync('bun', ['run', CLI_ENTRY, ...args], { - encoding: 'utf-8', + const result = Bun.spawnSync(['bun', 'run', CLI_ENTRY, ...args], { env: { ...process.env, XDG_CONFIG_HOME: configDir }, timeout: 30_000, }); return { - stdout: result.stdout ?? '', - stderr: result.stderr ?? '', - exitCode: result.status ?? 1, + stdout: result.stdout.toString(), + stderr: result.stderr.toString(), + exitCode: result.exitCode ?? 1, }; } finally { rmSync(configDir, { recursive: true, force: true }); From 072acef06e1c5d202f3b994075da0c6db54f5e82 Mon Sep 17 00:00:00 2001 From: "gfargo-horizon-agent[bot]" <294710345+gfargo-horizon-agent[bot]@users.noreply.github.com> Date: Sat, 1 Aug 2026 08:10:35 +0000 Subject: [PATCH 4/8] fix(test): use a dynamically-bound closed port for doctor unreachable tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The doctor exit-code tests simulated an unreachable site with a hardcoded 127.0.0.1:1 and a `.invalid` hostname. CI failed with all four "unreachable site" assertions receiving exit 0 instead of 4, while the exact same test/binary/bun-version combination passed reliably in isolation — pointing at CI's network layer not refusing that fixed port or resolving that reserved TLD the same way a local machine does. Replace both with a real TCP server bound to an OS-assigned loopback port and closed before use, guaranteeing ECONNREFUSED from the kernel with no DNS lookup or fixed port/hostname assumption involved. --- test/unit/doctor-exit-code.test.ts | 55 ++++++++++++++++++++++-------- 1 file changed, 40 insertions(+), 15 deletions(-) diff --git a/test/unit/doctor-exit-code.test.ts b/test/unit/doctor-exit-code.test.ts index 1297fd3..947b469 100644 --- a/test/unit/doctor-exit-code.test.ts +++ b/test/unit/doctor-exit-code.test.ts @@ -13,17 +13,39 @@ * preview-server.test.ts / editor-detect.test.ts / quick-view-auth.test.ts) * without restoring it — depending on file load order that leaves `spawnSync` * undefined when this file imports it. Bun.spawnSync isn't affected. - * Unreachable URLs use 127.0.0.1:1 to force an immediate ECONNREFUSED without - * touching DNS. + * + * "Unreachable" is simulated by binding a real TCP server on an OS-assigned + * loopback port and closing it before use, rather than a hardcoded low port + * (e.g. 127.0.0.1:1) or a `.invalid` hostname. A closed loopback port gets + * ECONNREFUSED straight from the kernel — no DNS lookup, no routing beyond + * loopback, and no dependency on a fixed port/hostname staying unreachable + * on every network the tests happen to run on (some CI network layers proxy + * or intercept low ports and reserved-TLD lookups differently than a local + * machine). */ import { describe, expect, test } from 'bun:test'; import { mkdirSync, mkdtempSync, rmSync, writeFileSync } from 'node:fs'; +import { createServer } from 'node:net'; import { tmpdir } from 'node:os'; import { join } from 'node:path'; const CLI_ENTRY = join(process.cwd(), 'src', 'cli', 'index.ts'); +/** Bind an ephemeral loopback port, then close it, so it's guaranteed refused. */ +async function unreachableLoopbackUrl(): Promise { + const port = await new Promise((resolve, reject) => { + const srv = createServer(); + srv.on('error', reject); + srv.listen(0, '127.0.0.1', () => { + const address = srv.address(); + const boundPort = typeof address === 'object' && address ? address.port : 0; + srv.close(() => resolve(boundPort)); + }); + }); + return `http://127.0.0.1:${port}`; +} + /** Spawn the CLI with an ephemeral config dir. No site is seeded by default. */ function runCli( args: string[], @@ -104,14 +126,15 @@ describe('doctor exit codes', () => { // ------------------------------------------------------------------------- // Unreachable site — connection refused // ------------------------------------------------------------------------- - test('doctor with unreachable site exits 4 (NetworkError)', () => { - // Port 1 on loopback is reliably refused without DNS involvement. - const { exitCode } = runCliWithSite(['doctor'], 'http://127.0.0.1:1'); + test('doctor with unreachable site exits 4 (NetworkError)', async () => { + const siteUrl = await unreachableLoopbackUrl(); + const { exitCode } = runCliWithSite(['doctor'], siteUrl); expect(exitCode).toBe(4); }); - test('doctor --json with unreachable site exits 4 and emits connectionOk:false', () => { - const { stdout, exitCode } = runCliWithSite(['doctor', '--json'], 'http://127.0.0.1:1'); + test('doctor --json with unreachable site exits 4 and emits connectionOk:false', async () => { + const siteUrl = await unreachableLoopbackUrl(); + const { stdout, exitCode } = runCliWithSite(['doctor', '--json'], siteUrl); expect(exitCode).toBe(4); // stdout should contain a JSON object with connectionOk:false @@ -127,19 +150,21 @@ describe('doctor exit codes', () => { expect(errorIssues.length).toBeGreaterThan(0); }); - test('doctor --all-sites with unreachable site exits 4', () => { - const { exitCode } = runCliWithSite(['doctor', '--all-sites'], 'http://127.0.0.1:1'); + test('doctor --all-sites with unreachable site exits 4', async () => { + const siteUrl = await unreachableLoopbackUrl(); + const { exitCode } = runCliWithSite(['doctor', '--all-sites'], siteUrl); expect(exitCode).toBe(4); }); // ------------------------------------------------------------------------- - // Bogus hostname — DNS error (ENOTFOUND) + // A second, independently-bound unreachable port — exercises the same + // network-failure path (the doctor code doesn't distinguish ECONNREFUSED + // from ENOTFOUND; both are "fetch never got a response") without relying + // on real DNS resolution of a reserved-but-not-universally-honored TLD. // ------------------------------------------------------------------------- - test('doctor with bogus hostname exits 4 (NetworkError)', () => { - const { exitCode } = runCliWithSite( - ['doctor'], - 'http://localpress-test-bogus-hostname-that-does-not-exist.invalid', - ); + test('doctor with a second unreachable site exits 4 (NetworkError)', async () => { + const siteUrl = await unreachableLoopbackUrl(); + const { exitCode } = runCliWithSite(['doctor'], siteUrl); expect(exitCode).toBe(4); }); }); From a685c2a178f9d7ab0ac5a08c4de49e96946c6a70 Mon Sep 17 00:00:00 2001 From: "gfargo-horizon-agent[bot]" <294710345+gfargo-horizon-agent[bot]@users.noreply.github.com> Date: Sat, 1 Aug 2026 15:27:51 +0000 Subject: [PATCH 5/8] fix(test): stop simulating unreachable sites over real network sockets Two network-based simulations (hardcoded low port + .invalid hostname, then an OS-assigned loopback port bound and closed before use) both passed locally but produced exit 0 in CI, meaning the REST connectivity check didn't fail the way we expected in that environment. Switch to a syntactically-invalid site URL instead: RestAdapter.apiUrl() throws synchronously from new URL() before any socket opens, so there's no network behavior for CI's environment to diverge on. --- test/unit/doctor-exit-code.test.ts | 70 +++++++++++++----------------- 1 file changed, 31 insertions(+), 39 deletions(-) diff --git a/test/unit/doctor-exit-code.test.ts b/test/unit/doctor-exit-code.test.ts index 947b469..a232dbc 100644 --- a/test/unit/doctor-exit-code.test.ts +++ b/test/unit/doctor-exit-code.test.ts @@ -14,37 +14,34 @@ * without restoring it — depending on file load order that leaves `spawnSync` * undefined when this file imports it. Bun.spawnSync isn't affected. * - * "Unreachable" is simulated by binding a real TCP server on an OS-assigned - * loopback port and closing it before use, rather than a hardcoded low port - * (e.g. 127.0.0.1:1) or a `.invalid` hostname. A closed loopback port gets - * ECONNREFUSED straight from the kernel — no DNS lookup, no routing beyond - * loopback, and no dependency on a fixed port/hostname staying unreachable - * on every network the tests happen to run on (some CI network layers proxy - * or intercept low ports and reserved-TLD lookups differently than a local - * machine). + * "Unreachable" is simulated with a syntactically-invalid site URL rather + * than an actually-unreachable network address. We tried two real-network + * approaches first — a hardcoded low port + `.invalid` hostname, then an + * OS-assigned loopback port bound and closed just before use — and *both* + * passed reliably locally but produced exit 0 in CI (i.e. the REST call + * apparently succeeded, or at least didn't throw). That points at CI's + * network layer not refusing connections/failing DNS the same way a local + * machine does, for reasons we can't control from here. A malformed URL + * sidesteps the network stack entirely: `RestAdapter.apiUrl()` calls + * `new URL(...)` on `${site.url}/wp-json/wp/v2${path}`, which throws + * synchronously for a string with no valid scheme — no socket is ever + * opened, so there's nothing for a network layer to intercept. Doctor's + * catch block treats any non-`WpApiError` thrown from the connectivity + * check as `code: 'network'` (see src/cli/commands/doctor.ts), so this + * still exercises the real "connection check failed" → NetworkError path. */ import { describe, expect, test } from 'bun:test'; import { mkdirSync, mkdtempSync, rmSync, writeFileSync } from 'node:fs'; -import { createServer } from 'node:net'; import { tmpdir } from 'node:os'; import { join } from 'node:path'; const CLI_ENTRY = join(process.cwd(), 'src', 'cli', 'index.ts'); -/** Bind an ephemeral loopback port, then close it, so it's guaranteed refused. */ -async function unreachableLoopbackUrl(): Promise { - const port = await new Promise((resolve, reject) => { - const srv = createServer(); - srv.on('error', reject); - srv.listen(0, '127.0.0.1', () => { - const address = srv.address(); - const boundPort = typeof address === 'object' && address ? address.port : 0; - srv.close(() => resolve(boundPort)); - }); - }); - return `http://127.0.0.1:${port}`; -} +/** No scheme — `new URL()` throws synchronously, before any socket is opened. */ +const UNREACHABLE_SITE_URL = 'not-a-valid-url-without-a-scheme'; +/** A second, distinct malformed URL for the "independent failure" test below. */ +const UNREACHABLE_SITE_URL_2 = 'ht!tp://also-not-a-valid-url'; /** Spawn the CLI with an ephemeral config dir. No site is seeded by default. */ function runCli( @@ -124,17 +121,15 @@ describe('doctor exit codes', () => { }); // ------------------------------------------------------------------------- - // Unreachable site — connection refused + // Unreachable site — connectivity check throws // ------------------------------------------------------------------------- - test('doctor with unreachable site exits 4 (NetworkError)', async () => { - const siteUrl = await unreachableLoopbackUrl(); - const { exitCode } = runCliWithSite(['doctor'], siteUrl); + test('doctor with unreachable site exits 4 (NetworkError)', () => { + const { exitCode } = runCliWithSite(['doctor'], UNREACHABLE_SITE_URL); expect(exitCode).toBe(4); }); - test('doctor --json with unreachable site exits 4 and emits connectionOk:false', async () => { - const siteUrl = await unreachableLoopbackUrl(); - const { stdout, exitCode } = runCliWithSite(['doctor', '--json'], siteUrl); + test('doctor --json with unreachable site exits 4 and emits connectionOk:false', () => { + const { stdout, exitCode } = runCliWithSite(['doctor', '--json'], UNREACHABLE_SITE_URL); expect(exitCode).toBe(4); // stdout should contain a JSON object with connectionOk:false @@ -150,21 +145,18 @@ describe('doctor exit codes', () => { expect(errorIssues.length).toBeGreaterThan(0); }); - test('doctor --all-sites with unreachable site exits 4', async () => { - const siteUrl = await unreachableLoopbackUrl(); - const { exitCode } = runCliWithSite(['doctor', '--all-sites'], siteUrl); + test('doctor --all-sites with unreachable site exits 4', () => { + const { exitCode } = runCliWithSite(['doctor', '--all-sites'], UNREACHABLE_SITE_URL); expect(exitCode).toBe(4); }); // ------------------------------------------------------------------------- - // A second, independently-bound unreachable port — exercises the same - // network-failure path (the doctor code doesn't distinguish ECONNREFUSED - // from ENOTFOUND; both are "fetch never got a response") without relying - // on real DNS resolution of a reserved-but-not-universally-honored TLD. + // A second, distinct malformed URL — exercises the same "any non-WpApiError + // thrown from the connectivity check is a NetworkError" path with a + // different failure shape (unparseable scheme vs. no scheme at all). // ------------------------------------------------------------------------- - test('doctor with a second unreachable site exits 4 (NetworkError)', async () => { - const siteUrl = await unreachableLoopbackUrl(); - const { exitCode } = runCliWithSite(['doctor'], siteUrl); + test('doctor with a second unreachable site exits 4 (NetworkError)', () => { + const { exitCode } = runCliWithSite(['doctor'], UNREACHABLE_SITE_URL_2); expect(exitCode).toBe(4); }); }); From 81cc23b0f547b2edb6d44145a6eaf03247a5cc8f Mon Sep 17 00:00:00 2001 From: "gfargo-horizon-agent[bot]" <294710345+gfargo-horizon-agent[bot]@users.noreply.github.com> Date: Sat, 1 Aug 2026 15:39:57 +0000 Subject: [PATCH 6/8] fix(test): mock fetch instead of real network for doctor unreachable-site tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three different real-network/DNS simulation techniques for the "unreachable site" doctor tests passed locally but produced exit 0 in CI, and the cause didn't point at any single layer under our control. Since doctor never calls process.exit() on this path (only process.exitCode), it's safe to invoke the command in-process and mock globalThis.fetch directly — the same technique already used by dry-run-honesty-behavior.test.ts — making the failure deterministic regardless of environment. --- test/unit/doctor-exit-code.test.ts | 269 ++++++++++++++++++----------- 1 file changed, 170 insertions(+), 99 deletions(-) diff --git a/test/unit/doctor-exit-code.test.ts b/test/unit/doctor-exit-code.test.ts index a232dbc..56d86fd 100644 --- a/test/unit/doctor-exit-code.test.ts +++ b/test/unit/doctor-exit-code.test.ts @@ -6,44 +6,41 @@ * - unreachable site → exit 4 (NetworkError), connectionOk:false in --json * - healthy site → exit 0 (manual / integration tests only — needs live WP) * - * Technique: Bun.spawnSync + isolated XDG_CONFIG_HOME (same pattern as - * cli-error-handling.test.ts). We use Bun.spawnSync rather than - * node:child_process's spawnSync because other unit test files replace the - * whole node:child_process module process-wide via `mock.module()` (see - * preview-server.test.ts / editor-detect.test.ts / quick-view-auth.test.ts) - * without restoring it — depending on file load order that leaves `spawnSync` - * undefined when this file imports it. Bun.spawnSync isn't affected. + * Two techniques are used, split by whether the code path under test calls + * `process.exit()` directly: * - * "Unreachable" is simulated with a syntactically-invalid site URL rather - * than an actually-unreachable network address. We tried two real-network - * approaches first — a hardcoded low port + `.invalid` hostname, then an - * OS-assigned loopback port bound and closed just before use — and *both* - * passed reliably locally but produced exit 0 in CI (i.e. the REST call - * apparently succeeded, or at least didn't throw). That points at CI's - * network layer not refusing connections/failing DNS the same way a local - * machine does, for reasons we can't control from here. A malformed URL - * sidesteps the network stack entirely: `RestAdapter.apiUrl()` calls - * `new URL(...)` on `${site.url}/wp-json/wp/v2${path}`, which throws - * synchronously for a string with no valid scheme — no socket is ever - * opened, so there's nothing for a network layer to intercept. Doctor's - * catch block treats any non-`WpApiError` thrown from the connectivity - * check as `code: 'network'` (see src/cli/commands/doctor.ts), so this - * still exercises the real "connection check failed" → NetworkError path. + * - "no sites configured" calls `process.exit(ExitCode.ConfigError)` inline + * (src/cli/commands/doctor.ts), which would kill the whole test runner if + * invoked in-process — so those cases spawn a real CLI subprocess via + * Bun.spawnSync (same pattern as cli-error-handling.test.ts). + * + * - "unreachable site" only ever sets `process.exitCode` (never calls + * `process.exit()`), so it's safe to invoke doctor's action in-process. We + * tried simulating an unreachable site over real sockets/DNS (a hardcoded + * low port + `.invalid` hostname, then an OS-assigned loopback port bound + * and closed just before use, then a syntactically-invalid URL that never + * opens a socket at all) — all three passed reliably locally but produced + * exit 0 in CI, for reasons that don't point at any single layer we can + * control from a test. Rather than keep guessing at CI's network/DNS + * behavior, we sidestep it entirely: mock `globalThis.fetch` directly + * (the same technique already used by dry-run-honesty-behavior.test.ts and + * models.test.ts in this repo) so the "connection check failed" path is + * exercised deterministically regardless of environment. */ -import { describe, expect, test } from 'bun:test'; -import { mkdirSync, mkdtempSync, rmSync, writeFileSync } from 'node:fs'; +import { afterEach, beforeEach, describe, expect, test } from 'bun:test'; +import { mkdtempSync, rmSync } from 'node:fs'; import { tmpdir } from 'node:os'; import { join } from 'node:path'; +import { Command, Option } from 'commander'; +import { registerDoctorCommand } from '../../src/cli/commands/doctor.ts'; +import { saveConfig } from '../../src/cli/utils/config.ts'; +import { setOutputOptions } from '../../src/cli/utils/output.ts'; const CLI_ENTRY = join(process.cwd(), 'src', 'cli', 'index.ts'); +const SITE_NAME = 'testsite'; -/** No scheme — `new URL()` throws synchronously, before any socket is opened. */ -const UNREACHABLE_SITE_URL = 'not-a-valid-url-without-a-scheme'; -/** A second, distinct malformed URL for the "independent failure" test below. */ -const UNREACHABLE_SITE_URL_2 = 'ht!tp://also-not-a-valid-url'; - -/** Spawn the CLI with an ephemeral config dir. No site is seeded by default. */ +/** Spawn the CLI with an ephemeral config dir. No site is seeded by default. */ function runCli( args: string[], extraEnv: Record = {}, @@ -64,52 +61,7 @@ function runCli( } } -/** - * Spawn the CLI with a pre-seeded config file pointing at a fake site URL. - * The config is the minimal shape accepted by loadConfig(). - */ -function runCliWithSite( - args: string[], - siteUrl: string, -): { stdout: string; stderr: string; exitCode: number } { - const configDir = mkdtempSync(join(tmpdir(), 'localpress-doctor-site-test-')); - try { - // Write a minimal config.json understood by src/cli/utils/config.ts. - const localpressDir = join(configDir, 'localpress'); - mkdirSync(localpressDir, { recursive: true }); - const config = { - version: 1, - activeSite: 'test-site', - sites: { - 'test-site': { - name: 'test-site', - url: siteUrl, - username: 'admin', - appPassword: 'fake fake fake fake fake fake', - createdAt: new Date().toISOString(), - }, - }, - }; - writeFileSync(join(localpressDir, 'config.json'), JSON.stringify(config), { mode: 0o600 }); - - const result = Bun.spawnSync(['bun', 'run', CLI_ENTRY, ...args], { - 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 }); - } -} - -describe('doctor exit codes', () => { - // ------------------------------------------------------------------------- - // No sites configured - // ------------------------------------------------------------------------- +describe('doctor exit codes — no sites configured', () => { test('doctor with no sites configured exits 3 (ConfigError)', () => { const { exitCode } = runCli(['doctor']); expect(exitCode).toBe(3); @@ -119,44 +71,163 @@ describe('doctor exit codes', () => { const { exitCode } = runCli(['doctor', '--all-sites']); expect(exitCode).toBe(3); }); +}); + +// ----------------------------------------------------------------------------- +// Unreachable site — connectivity check throws. Run in-process with a mocked +// `fetch` so the failure is deterministic regardless of the environment's +// network/DNS behavior (see the file-level comment above). +// ----------------------------------------------------------------------------- + +function buildProgram(): Command { + const program = new Command(); + program + .name('localpress') + .exitOverride() + .addOption(new Option('--site ', 'override the active site for this command')) + .addOption(new Option('--all-sites', 'show capabilities for every configured site')) + .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) }); + }); + return program; +} + +let originalXdgConfigHome: string | undefined; +let originalFetch: typeof fetch; +let originalExitCode: number | string | undefined | null; +let tmpDir: string; + +beforeEach(() => { + originalXdgConfigHome = process.env.XDG_CONFIG_HOME; + originalFetch = globalThis.fetch; + originalExitCode = process.exitCode; + process.exitCode = undefined; + tmpDir = mkdtempSync(join(tmpdir(), 'localpress-doctor-unreachable-test-')); + process.env.XDG_CONFIG_HOME = tmpDir; +}); + +afterEach(() => { + globalThis.fetch = originalFetch; + process.exitCode = originalExitCode === null ? undefined : originalExitCode; + setOutputOptions({ json: false, quiet: false }); + if (originalXdgConfigHome === undefined) { + // biome-ignore lint/performance/noDelete: env var must be truly absent, not the string "undefined" + delete process.env.XDG_CONFIG_HOME; + } else { + process.env.XDG_CONFIG_HOME = originalXdgConfigHome; + } + rmSync(tmpDir, { recursive: true, force: true }); +}); + +/** A `fetch` that fails the way an unreachable host does: no HTTP response at all. */ +function unreachableFetch(): typeof fetch { + return (async () => { + throw new TypeError('fetch failed'); + }) as unknown as typeof fetch; +} + +async function seedSite(url: string): Promise { + await saveConfig({ + version: 1, + activeSite: SITE_NAME, + sites: { + [SITE_NAME]: { + name: SITE_NAME, + url, + username: 'admin', + appPassword: 'fake fake fake fake fake fake', + createdAt: new Date(0).toISOString(), + }, + }, + }); +} + +describe('doctor exit codes — unreachable site', () => { + test('doctor with unreachable site exits 4 (NetworkError)', async () => { + await seedSite('https://example.test'); + globalThis.fetch = unreachableFetch(); - // ------------------------------------------------------------------------- - // Unreachable site — connectivity check throws - // ------------------------------------------------------------------------- - test('doctor with unreachable site exits 4 (NetworkError)', () => { - const { exitCode } = runCliWithSite(['doctor'], UNREACHABLE_SITE_URL); - expect(exitCode).toBe(4); + const program = buildProgram(); + registerDoctorCommand(program); + await program.parseAsync(['doctor'], { from: 'user' }); + + expect(process.exitCode).toBe(4); }); - test('doctor --json with unreachable site exits 4 and emits connectionOk:false', () => { - const { stdout, exitCode } = runCliWithSite(['doctor', '--json'], UNREACHABLE_SITE_URL); - expect(exitCode).toBe(4); + test('doctor --json with unreachable site exits 4 and emits connectionOk:false', async () => { + await seedSite('https://example.test'); + globalThis.fetch = unreachableFetch(); + + const chunks: string[] = []; + const originalWrite = process.stdout.write.bind(process.stdout); + process.stdout.write = ((chunk: string | Uint8Array) => { + chunks.push(chunk.toString()); + return true; + }) as typeof process.stdout.write; - // stdout should contain a JSON object with connectionOk:false - const lines = stdout.trim().split('\n').filter(Boolean); + const program = buildProgram(); + registerDoctorCommand(program); + try { + await program.parseAsync(['doctor', '--json'], { from: 'user' }); + } finally { + process.stdout.write = originalWrite; + } + + expect(process.exitCode).toBe(4); + + const output = chunks.join(''); + const lines = output.trim().split('\n').filter(Boolean); expect(lines.length).toBeGreaterThan(0); const parsed = JSON.parse(lines[0]); expect(parsed.connectionOk).toBe(false); - // There should be at least one error-severity issue. 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 } = runCliWithSite(['doctor', '--all-sites'], UNREACHABLE_SITE_URL); - expect(exitCode).toBe(4); + test('doctor --all-sites with unreachable site exits 4', async () => { + await saveConfig({ + version: 1, + activeSite: SITE_NAME, + sites: { + [SITE_NAME]: { + name: SITE_NAME, + url: 'https://example.test', + username: 'admin', + appPassword: 'fake fake fake fake fake fake', + createdAt: new Date(0).toISOString(), + }, + 'second-site': { + name: 'second-site', + url: 'https://example2.test', + username: 'admin', + appPassword: 'fake fake fake fake fake fake', + createdAt: new Date(0).toISOString(), + }, + }, + }); + globalThis.fetch = unreachableFetch(); + + const program = buildProgram(); + registerDoctorCommand(program); + await program.parseAsync(['doctor', '--all-sites'], { from: 'user' }); + + expect(process.exitCode).toBe(4); }); - // ------------------------------------------------------------------------- - // A second, distinct malformed URL — exercises the same "any non-WpApiError - // thrown from the connectivity check is a NetworkError" path with a - // different failure shape (unparseable scheme vs. no scheme at all). - // ------------------------------------------------------------------------- - test('doctor with a second unreachable site exits 4 (NetworkError)', () => { - const { exitCode } = runCliWithSite(['doctor'], UNREACHABLE_SITE_URL_2); - expect(exitCode).toBe(4); + test('doctor with a second, independent unreachable site exits 4 (NetworkError)', async () => { + await seedSite('https://another-example.test'); + globalThis.fetch = unreachableFetch(); + + const program = buildProgram(); + registerDoctorCommand(program); + await program.parseAsync(['doctor'], { from: 'user' }); + + expect(process.exitCode).toBe(4); }); }); From 647425640048223025b6c0c552817f68cc0fc1b4 Mon Sep 17 00:00:00 2001 From: "gfargo-horizon-agent[bot]" <294710345+gfargo-horizon-agent[bot]@users.noreply.github.com> Date: Sun, 2 Aug 2026 17:31:30 +0000 Subject: [PATCH 7/8] fix(test): pre-warm jSquash WASM before fetch mock in doctor-exit-code tests The in-process doctor tests mock globalThis.fetch to simulate an unreachable site. jSquash WASM codecs use fetch to initialize their binary on first load. If the mock is active when the module first loads, the WASM state is permanently broken in the module cache, poisoning the encoder-preflight tests that run later in the same bun test suite. Add a beforeAll that pre-warms all four jSquash codecs before any fetch mock is installed, ensuring the modules are fully initialized and subsequent encoder-preflight tests are not affected. --- test/unit/doctor-exit-code.test.ts | 19 ++++++++++++++++++- 1 file changed, 18 insertions(+), 1 deletion(-) diff --git a/test/unit/doctor-exit-code.test.ts b/test/unit/doctor-exit-code.test.ts index 56d86fd..7d8003f 100644 --- a/test/unit/doctor-exit-code.test.ts +++ b/test/unit/doctor-exit-code.test.ts @@ -28,7 +28,7 @@ * exercised deterministically regardless of environment. */ -import { afterEach, beforeEach, describe, expect, test } from 'bun:test'; +import { afterEach, beforeAll, beforeEach, describe, expect, test } from 'bun:test'; import { mkdtempSync, rmSync } from 'node:fs'; import { tmpdir } from 'node:os'; import { join } from 'node:path'; @@ -36,10 +36,27 @@ import { Command, Option } from 'commander'; import { registerDoctorCommand } from '../../src/cli/commands/doctor.ts'; import { saveConfig } from '../../src/cli/utils/config.ts'; import { setOutputOptions } from '../../src/cli/utils/output.ts'; +import { preflightJsquashEncoder } from '../../src/engine/image/jsquash.ts'; const CLI_ENTRY = join(process.cwd(), 'src', 'cli', 'index.ts'); const SITE_NAME = 'testsite'; +/** + * Pre-warm the jSquash WASM codecs before any test in this file runs. + * + * The in-process doctor tests mock `globalThis.fetch` to simulate an + * unreachable site. jSquash WASM modules use `fetch` to load their binary on + * first initialization. If the mock is active when the module first loads, the + * WASM state is permanently broken for the rest of the process — poisoning the + * `encoder-preflight` tests that run later in the same test suite. + * + * Running a no-op preflight here ensures all jSquash WASM modules are fully + * initialized before any fetch mock is installed. + */ +beforeAll(async () => { + await preflightJsquashEncoder(['jpeg', 'png', 'webp', 'avif']); +}); + /** Spawn the CLI with an ephemeral config dir. No site is seeded by default. */ function runCli( args: string[], From a2c5cd9a07ec0c94705be41e18335cf60aeb32a0 Mon Sep 17 00:00:00 2001 From: "gfargo-horizon-agent[bot]" <294710345+gfargo-horizon-agent[bot]@users.noreply.github.com> Date: Sun, 2 Aug 2026 18:11:02 +0000 Subject: [PATCH 8/8] fix(test): move unreachable-site tests to subprocess to avoid process.exitCode pollution MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit bun test exits with code 4 when any test sets process.exitCode to a non-zero value, even if it is later reset in afterEach. The in-process doctor tests were setting process.exitCode = 4 (NetworkError) via doctor's action handler, causing bun test itself to exit 4 despite 0 test failures. Fix: extract a test/fixtures/doctor-unreachable.ts harness that mocks globalThis.fetch before running doctor, then invoke it via Bun.spawnSync so the process.exitCode side-effect stays inside the subprocess. The test file now uses only Bun.spawnSync (no in-process invocations), matching the pattern already used by cli-error-handling.test.ts. The doctor.ts implementation is unchanged — this commit is test-only. --- test/fixtures/doctor-unreachable.ts | 39 ++++ test/unit/doctor-exit-code.test.ts | 275 ++++++++++------------------ 2 files changed, 136 insertions(+), 178 deletions(-) create mode 100644 test/fixtures/doctor-unreachable.ts 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 index 7d8003f..fd215a7 100644 --- a/test/unit/doctor-exit-code.test.ts +++ b/test/unit/doctor-exit-code.test.ts @@ -6,58 +6,60 @@ * - unreachable site → exit 4 (NetworkError), connectionOk:false in --json * - healthy site → exit 0 (manual / integration tests only — needs live WP) * - * Two techniques are used, split by whether the code path under test calls - * `process.exit()` directly: + * 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. * - * - "no sites configured" calls `process.exit(ExitCode.ConfigError)` inline - * (src/cli/commands/doctor.ts), which would kill the whole test runner if - * invoked in-process — so those cases spawn a real CLI subprocess via - * Bun.spawnSync (same pattern as cli-error-handling.test.ts). - * - * - "unreachable site" only ever sets `process.exitCode` (never calls - * `process.exit()`), so it's safe to invoke doctor's action in-process. We - * tried simulating an unreachable site over real sockets/DNS (a hardcoded - * low port + `.invalid` hostname, then an OS-assigned loopback port bound - * and closed just before use, then a syntactically-invalid URL that never - * opens a socket at all) — all three passed reliably locally but produced - * exit 0 in CI, for reasons that don't point at any single layer we can - * control from a test. Rather than keep guessing at CI's network/DNS - * behavior, we sidestep it entirely: mock `globalThis.fetch` directly - * (the same technique already used by dry-run-honesty-behavior.test.ts and - * models.test.ts in this repo) so the "connection check failed" path is - * exercised deterministically regardless of environment. + * 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 { afterEach, beforeAll, beforeEach, describe, expect, test } from 'bun:test'; -import { mkdtempSync, rmSync } from 'node:fs'; +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'; -import { Command, Option } from 'commander'; -import { registerDoctorCommand } from '../../src/cli/commands/doctor.ts'; -import { saveConfig } from '../../src/cli/utils/config.ts'; -import { setOutputOptions } from '../../src/cli/utils/output.ts'; -import { preflightJsquashEncoder } from '../../src/engine/image/jsquash.ts'; 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'; /** - * Pre-warm the jSquash WASM codecs before any test in this file runs. - * - * The in-process doctor tests mock `globalThis.fetch` to simulate an - * unreachable site. jSquash WASM modules use `fetch` to load their binary on - * first initialization. If the mock is active when the module first loads, the - * WASM state is permanently broken for the rest of the process — poisoning the - * `encoder-preflight` tests that run later in the same test suite. - * - * Running a no-op preflight here ensures all jSquash WASM modules are fully - * initialized before any fetch mock is installed. + * Seed a minimal localpress config file in the given config dir so + * `loadConfig()` finds a site without any real network calls. */ -beforeAll(async () => { - await preflightJsquashEncoder(['jpeg', 'png', 'webp', 'avif']); -}); +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 with an ephemeral config dir. No site is seeded by default. */ +/** Spawn the CLI (no site seeded) with an ephemeral config dir. */ function runCli( args: string[], extraEnv: Record = {}, @@ -78,6 +80,38 @@ function runCli( } } +/** + * 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']); @@ -90,113 +124,22 @@ describe('doctor exit codes — no sites configured', () => { }); }); -// ----------------------------------------------------------------------------- -// Unreachable site — connectivity check throws. Run in-process with a mocked -// `fetch` so the failure is deterministic regardless of the environment's -// network/DNS behavior (see the file-level comment above). -// ----------------------------------------------------------------------------- - -function buildProgram(): Command { - const program = new Command(); - program - .name('localpress') - .exitOverride() - .addOption(new Option('--site ', 'override the active site for this command')) - .addOption(new Option('--all-sites', 'show capabilities for every configured site')) - .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) }); - }); - return program; -} - -let originalXdgConfigHome: string | undefined; -let originalFetch: typeof fetch; -let originalExitCode: number | string | undefined | null; -let tmpDir: string; - -beforeEach(() => { - originalXdgConfigHome = process.env.XDG_CONFIG_HOME; - originalFetch = globalThis.fetch; - originalExitCode = process.exitCode; - process.exitCode = undefined; - tmpDir = mkdtempSync(join(tmpdir(), 'localpress-doctor-unreachable-test-')); - process.env.XDG_CONFIG_HOME = tmpDir; -}); - -afterEach(() => { - globalThis.fetch = originalFetch; - process.exitCode = originalExitCode === null ? undefined : originalExitCode; - setOutputOptions({ json: false, quiet: false }); - if (originalXdgConfigHome === undefined) { - // biome-ignore lint/performance/noDelete: env var must be truly absent, not the string "undefined" - delete process.env.XDG_CONFIG_HOME; - } else { - process.env.XDG_CONFIG_HOME = originalXdgConfigHome; - } - rmSync(tmpDir, { recursive: true, force: true }); -}); - -/** A `fetch` that fails the way an unreachable host does: no HTTP response at all. */ -function unreachableFetch(): typeof fetch { - return (async () => { - throw new TypeError('fetch failed'); - }) as unknown as typeof fetch; -} - -async function seedSite(url: string): Promise { - await saveConfig({ - version: 1, - activeSite: SITE_NAME, - sites: { - [SITE_NAME]: { - name: SITE_NAME, - url, - username: 'admin', - appPassword: 'fake fake fake fake fake fake', - createdAt: new Date(0).toISOString(), - }, - }, - }); -} +// --------------------------------------------------------------------------- +// 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)', async () => { - await seedSite('https://example.test'); - globalThis.fetch = unreachableFetch(); - - const program = buildProgram(); - registerDoctorCommand(program); - await program.parseAsync(['doctor'], { from: 'user' }); - - expect(process.exitCode).toBe(4); + 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', async () => { - await seedSite('https://example.test'); - globalThis.fetch = unreachableFetch(); - - const chunks: string[] = []; - const originalWrite = process.stdout.write.bind(process.stdout); - process.stdout.write = ((chunk: string | Uint8Array) => { - chunks.push(chunk.toString()); - return true; - }) as typeof process.stdout.write; - - const program = buildProgram(); - registerDoctorCommand(program); - try { - await program.parseAsync(['doctor', '--json'], { from: 'user' }); - } finally { - process.stdout.write = originalWrite; - } - - expect(process.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 output = chunks.join(''); - const lines = output.trim().split('\n').filter(Boolean); + const lines = stdout.trim().split('\n').filter(Boolean); expect(lines.length).toBeGreaterThan(0); const parsed = JSON.parse(lines[0]); expect(parsed.connectionOk).toBe(false); @@ -207,44 +150,20 @@ describe('doctor exit codes — unreachable site', () => { expect(errorIssues.length).toBeGreaterThan(0); }); - test('doctor --all-sites with unreachable site exits 4', async () => { - await saveConfig({ - version: 1, - activeSite: SITE_NAME, - sites: { - [SITE_NAME]: { - name: SITE_NAME, - url: 'https://example.test', - username: 'admin', - appPassword: 'fake fake fake fake fake fake', - createdAt: new Date(0).toISOString(), - }, - 'second-site': { - name: 'second-site', - url: 'https://example2.test', - username: 'admin', - appPassword: 'fake fake fake fake fake fake', - createdAt: new Date(0).toISOString(), - }, - }, + 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' }, }); - globalThis.fetch = unreachableFetch(); - - const program = buildProgram(); - registerDoctorCommand(program); - await program.parseAsync(['doctor', '--all-sites'], { from: 'user' }); - - expect(process.exitCode).toBe(4); + expect(exitCode).toBe(4); }); - test('doctor with a second, independent unreachable site exits 4 (NetworkError)', async () => { - await seedSite('https://another-example.test'); - globalThis.fetch = unreachableFetch(); - - const program = buildProgram(); - registerDoctorCommand(program); - await program.parseAsync(['doctor'], { from: 'user' }); - - expect(process.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); }); });