From d0eafba79f49527b6c70272736176187054e4256 Mon Sep 17 00:00:00 2001 From: Ankit Ranjan Date: Thu, 27 Aug 2026 15:10:52 +0530 Subject: [PATCH 1/3] feat(doctor): exit CONFIG_ERROR when a required check fails doctor always exited 0 regardless of what it found, so nothing could gate on it. Required checks are the readiness signals (daemon, runtime, connectivity), not the issues array, which also collects soft warnings. The structured report still reaches stdout in full. The hosted doctor adopts the same contract so the two modes cannot disagree. BREAKING CHANGE: webcmd doctor now exits 78 on an unhealthy machine. --- src/cli.ts | 5 ++- src/doctor.test.ts | 88 +++++++++++++++++++++++++++++++++++++++++++--- src/doctor.ts | 12 +++++++ 3 files changed, 100 insertions(+), 5 deletions(-) diff --git a/src/cli.ts b/src/cli.ts index b87eaadd..3e49cb69 100644 --- a/src/cli.ts +++ b/src/cli.ts @@ -1430,10 +1430,13 @@ cli({ const fmt = resolveCommandOutputFormat(doctorCmd, opts.format); if (fmt === null) return; const fmtExplicit = outputFormatIsExplicit(doctorCmd); - const { runBrowserDoctor, renderBrowserDoctorReport } = await import('./doctor.js'); + const { runBrowserDoctor, renderBrowserDoctorReport, doctorRequiredChecksFailed } = await import('./doctor.js'); const report = await runBrowserDoctor({ cliVersion: PKG_VERSION }); if (fmt === 'table') console.log(renderBrowserDoctorReport(report)); else await renderOutput(report, { fmt, fmtExplicit }); + // The structured report is the payload and always reaches stdout; the exit + // code is what lets an agent gate its next step on the outcome. + if (doctorRequiredChecksFailed(report)) process.exitCode = EXIT_CODES.CONFIG_ERROR; }); configureCompletionCommandSurface(program.command('completion')) diff --git a/src/doctor.test.ts b/src/doctor.test.ts index e14dd2cc..55f2568b 100644 --- a/src/doctor.test.ts +++ b/src/doctor.test.ts @@ -2,6 +2,7 @@ import * as fs from 'node:fs'; import * as os from 'node:os'; import * as path from 'node:path'; import { afterAll, beforeEach, describe, expect, it, vi } from 'vitest'; +import { EXIT_CODES } from './errors.js'; const { mockGetDaemonHealth, @@ -23,9 +24,13 @@ const { mockEnsureBinary: vi.fn(), })); -vi.mock('./browser/daemon-transport.js', () => ({ - getDaemonHealth: mockGetDaemonHealth, -})); +vi.mock('./browser/daemon-transport.js', async () => { + const actual = await vi.importActual('./browser/daemon-transport.js'); + return { + ...actual, + getDaemonHealth: mockGetDaemonHealth, + }; +}); // Real binaryInfo() reads this machine's actual CloakBrowser cache dir, which // varies by dev box/CI runner — mock it so doctor tests are hermetic and the @@ -55,7 +60,8 @@ vi.mock('./adapter-shadow.js', async () => { }; }); -import { checkBrowserBinary, checkConnectivity, renderBrowserDoctorReport, runBrowserDoctor } from './doctor.js'; +import { checkBrowserBinary, checkConnectivity, doctorRequiredChecksFailed, renderBrowserDoctorReport, runBrowserDoctor, type DoctorReport } from './doctor.js'; +import { createProgram } from './cli.js'; const managedBinaryDir = fs.mkdtempSync(path.join(os.tmpdir(), 'webcmd-managed-binary-')); const managedBinaryPath = path.join(managedBinaryDir, process.platform === 'win32' ? 'chrome.exe' : 'chrome'); @@ -815,3 +821,77 @@ describe('doctor window mode', () => { vi.unstubAllEnvs(); }); }); + +describe('doctorRequiredChecksFailed', () => { + const healthy: DoctorReport = { + daemonRunning: true, + runtimeConnected: true, + connectivity: { ok: true, durationMs: 12 }, + issues: [], + }; + + it('passes when every required check is healthy', () => { + expect(doctorRequiredChecksFailed(healthy)).toBe(false); + }); + + it('passes when only a soft issue was recorded', () => { + expect(doctorRequiredChecksFailed({ + ...healthy, + issues: ['Could not check adapter overrides: EACCES'], + })).toBe(false); + }); + + it.each([ + ['daemon down', { ...healthy, daemonRunning: false }], + ['runtime disconnected', { ...healthy, runtimeConnected: false }], + ['connectivity failed', { ...healthy, connectivity: { ok: false, durationMs: 9 } }], + ])('fails when %s', (_label, report) => { + expect(doctorRequiredChecksFailed(report as DoctorReport)).toBe(true); + }); + + it('passes when connectivity was not probed at all', () => { + const { connectivity: _omitted, ...withoutConnectivity } = healthy; + expect(doctorRequiredChecksFailed(withoutConnectivity as DoctorReport)).toBe(false); + }); +}); + +describe('doctor command', () => { + beforeEach(() => { + vi.clearAllMocks(); + mockEnsureBinary.mockResolvedValue('path/to/chrome'); + mockBinaryInfo.mockReturnValue({ + version: '146.0.7680.177.5', + bundledVersion: '146.0.7680.177.5', + tier: 'free', + platform: 'linux-x64', + binaryPath: '/path/to/chrome', + installed: true, + cacheDir: '/cache', + downloadUrl: 'https://example.com/download', + }); + mockConnect.mockResolvedValue({ + evaluate: vi.fn().mockResolvedValue(2), + closeWindow: vi.fn().mockResolvedValue(undefined), + }); + mockClose.mockResolvedValue(undefined); + mockSendCommand.mockImplementation(async (action: string) => { + if (action === 'session-create') return { id: 'doctor-probe-k7' }; + if (action === 'session-close') return { closed: true }; + throw new Error(`Unexpected doctor command: ${action}`); + }); + mockFindShadowedUserAdapters.mockReturnValue([]); + mockSetDaemonCommandTimeoutSeconds.mockClear(); + }); + + it('exits CONFIG_ERROR when a required doctor check fails', async () => { + mockGetDaemonHealth.mockResolvedValue({ state: 'stopped', status: null }); + const previous = process.exitCode; + process.exitCode = undefined; + try { + await createProgram('', '').parseAsync(['doctor', '-f', 'json'], { from: 'user' }); + expect(process.exitCode).toBe(EXIT_CODES.CONFIG_ERROR); + } finally { + process.exitCode = previous; + } + }); +}); diff --git a/src/doctor.ts b/src/doctor.ts index f70d1791..695cac3c 100644 --- a/src/doctor.ts +++ b/src/doctor.ts @@ -57,6 +57,18 @@ export type DoctorReport = { issues: string[]; }; +/** + * Required readiness checks, as opposed to `issues`, which also collects soft + * warnings such as an unreadable adapter-override directory. An absent + * `connectivity` means the probe never ran, which is not a failure. + */ +export function doctorRequiredChecksFailed(report: DoctorReport): boolean { + if (!report.daemonRunning) return true; + if (!report.runtimeConnected) return true; + if (report.connectivity && !report.connectivity.ok) return true; + return false; +} + function isLaunchableFile(binaryPath: string): boolean { try { if (!fs.statSync(binaryPath).isFile()) return false; From 9af33170102f57609a895780b96de466377ec512 Mon Sep 17 00:00:00 2001 From: Ankit Ranjan Date: Thu, 27 Aug 2026 15:47:13 +0530 Subject: [PATCH 2/3] docs: changelog for doctor exit contract change Co-Authored-By: Claude Opus 4.7 --- CHANGELOG.md | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index f558430b..cd07d56a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,11 @@ # Changelog +## Unreleased + +### Changed + +- **Breaking:** `webcmd doctor` exits 78 (`CONFIG_ERROR`) when a required readiness check fails. It previously always exited 0. The structured report is unchanged and still goes to stdout. + ## [0.7.7](https://github.com/agentrhq/webcmd/compare/webcmd-v0.7.6...webcmd-v0.7.7) (2026-08-26) ### Improvements From c905b72047f78b9043eeecb4af799f07c21e87cc Mon Sep 17 00:00:00 2001 From: Ankit Ranjan Date: Thu, 27 Aug 2026 16:31:02 +0530 Subject: [PATCH 3/3] Revert "docs: changelog for doctor exit contract change" This reverts commit 9af33170102f57609a895780b96de466377ec512. --- CHANGELOG.md | 6 ------ 1 file changed, 6 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index cd07d56a..f558430b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,11 +1,5 @@ # Changelog -## Unreleased - -### Changed - -- **Breaking:** `webcmd doctor` exits 78 (`CONFIG_ERROR`) when a required readiness check fails. It previously always exited 0. The structured report is unchanged and still goes to stdout. - ## [0.7.7](https://github.com/agentrhq/webcmd/compare/webcmd-v0.7.6...webcmd-v0.7.7) (2026-08-26) ### Improvements