From 7c6ea76d269fdbf40b81055713e95ac34056de55 Mon Sep 17 00:00:00 2001 From: Ankit Ranjan Date: Wed, 19 Aug 2026 17:46:18 +0530 Subject: [PATCH 1/2] fix(cli): report built-in command errors instead of crashing MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `runCli` was `createProgram(...).parse()` with no error handling, so anything a built-in command threw escaped: sync throws printed a raw Node stack trace, and async ones surfaced as unhandled rejections. Either way the `exitCode` the error carried was discarded. Five commands hit this in practice: $ webcmd adapter path hackernews/top file:///…/dist/src/cli.js:1635 throw new ArgumentError(`Adapter source is unavailable …`); ^ ArgumentError: Adapter source is unavailable for hackernews/top. at localAdapterPath (file:///…/dist/src/cli.js:1635:19) … Adapter commands never had this problem — execution.ts wraps them and renders the shared error envelope. Use the same envelope here, so built-ins and adapters report failures identically: $ webcmd adapter path hackernews/top ok: false error: code: ARGUMENT message: Adapter source is unavailable for hackernews/top. exitCode: 2 Exit codes now come from the error rather than being lost, which also settles the taxonomy in errors.ts for these paths — `site fixture get` on a missing fixture exits 66 (EMPTY_RESULT) instead of 1, and `session close ` exits 2 (USAGE_ERROR). Stacks stay off unless WEBCMD_DEBUG is set. `parse()` -> `parseAsync()` is what lets async rejections reach the handler, and main.ts now awaits runCli. That also keeps the daemon-run signal cancellation installed for the real duration of a run: `parse()` returned as soon as it kicked off an async action, so main.ts's `finally` uninstalled the SIGINT handler while the run was still in flight, and Ctrl-C never cancelled it. Co-Authored-By: Claude Opus 5 --- src/cli-error-report.test.ts | 49 ++++++++++++++++++++++++++++++++++++ src/cli.ts | 31 ++++++++++++++++++++--- src/main.ts | 2 +- 3 files changed, 77 insertions(+), 5 deletions(-) create mode 100644 src/cli-error-report.test.ts diff --git a/src/cli-error-report.test.ts b/src/cli-error-report.test.ts new file mode 100644 index 000000000..f8ef3ddcd --- /dev/null +++ b/src/cli-error-report.test.ts @@ -0,0 +1,49 @@ +import { afterEach, describe, expect, it, vi } from 'vitest'; +import yaml from 'js-yaml'; +import { reportCliError } from './cli.js'; +import { CliError, EXIT_CODES } from './errors.js'; + +describe('reportCliError', () => { + const previousExitCode = process.exitCode; + afterEach(() => { process.exitCode = previousExitCode; }); + + function capture(err: unknown): string { + let text = ''; + reportCliError(err, { write: (chunk: string) => { text += chunk; return true; } } as unknown as NodeJS.WritableStream); + return text; + } + + it('renders a CliError as the shared envelope and keeps its exit code', () => { + const text = capture(new CliError('SITE_MEMORY_NOT_FOUND', 'Verify fixture a/b was not found.', undefined, EXIT_CODES.EMPTY_RESULT)); + + expect(yaml.load(text)).toEqual({ + ok: false, + error: { code: 'SITE_MEMORY_NOT_FOUND', message: 'Verify fixture a/b was not found.', exitCode: EXIT_CODES.EMPTY_RESULT }, + }); + expect(process.exitCode).toBe(EXIT_CODES.EMPTY_RESULT); + }); + + it('carries the hint through as help', () => { + const text = capture(new CliError('X', 'broke', 'try this', EXIT_CODES.USAGE_ERROR)); + + expect(yaml.load(text)).toMatchObject({ error: { help: 'try this', exitCode: EXIT_CODES.USAGE_ERROR } }); + expect(process.exitCode).toBe(EXIT_CODES.USAGE_ERROR); + }); + + it('falls back to UNKNOWN and a generic exit code for a plain error', () => { + const text = capture(new Error('boom')); + + expect(yaml.load(text)).toMatchObject({ error: { code: 'UNKNOWN', message: 'boom', exitCode: EXIT_CODES.GENERIC_ERROR } }); + expect(process.exitCode).toBe(EXIT_CODES.GENERIC_ERROR); + }); + + it('omits the stack unless WEBCMD_DEBUG is set', () => { + expect(yaml.load(capture(new Error('boom'))) as any).not.toHaveProperty('error.stack'); + vi.stubEnv('WEBCMD_DEBUG', '1'); + try { + expect(String((yaml.load(capture(new Error('boom'))) as any).error.stack)).toContain('Error: boom'); + } finally { + vi.unstubAllEnvs(); + } + }); +}); diff --git a/src/cli.ts b/src/cli.ts index 9f645dafc..2808e7b61 100644 --- a/src/cli.ts +++ b/src/cli.ts @@ -19,14 +19,14 @@ import './fetch/command.js'; import { commandListPresentation, filterCommandsByTag, toPresentableCommand } from './command-presentation.js'; import { configureCompletionCommandSurface, configureListCommandSurface, configurePluginInstallSurface, configurePluginListSurface, configurePluginSearchSurface } from './builtin-command-surface.js'; import { OUTPUT_FORMAT_HELP, resolveOutputFormat } from './command-surface.js'; -import { render as renderOutput } from './output.js'; +import { render as renderOutput, formatErrorEnvelope } from './output.js'; import { PKG_VERSION } from './version.js'; import { printCompletionScript } from './completion.js'; import { loadExternalClis, executeExternalCli, installExternalCli, registerExternalCli, isBinaryInstalled, formatExternalCliLabel } from './external.js'; import { addWebcmdSkills, listWebcmdSkills, removeWebcmdSkills, updateWebcmdSkill, type WebcmdSkillAddResult } from './skills.js'; import { registerAllCommands } from './commanderAdapter.js'; import { buildRootHelpPresentation, classifyAdapter, installCommanderNamespaceStructuredHelp, installRootPresentationHelp, leadingPositionalFromUsage, rootHelpData, type RootAdapterGroups } from './help.js'; -import { EXIT_CODES, getErrorMessage, BrowserConnectError, CliError, ArgumentError } from './errors.js'; +import { EXIT_CODES, getErrorMessage, toEnvelope, BrowserConnectError, CliError, ArgumentError } from './errors.js'; import { TargetError, type TargetErrorCode } from './browser/target-errors.js'; import { resolveTargetJs, getTextResolvedJs, getValueResolvedJs, getAttributesResolvedJs, selectResolvedJs, isAutocompleteResolvedJs, type ResolveOptions, type TargetMatchLevel } from './browser/target-resolver.js'; import { buildFindJs, buildSemanticFindJs, isFindError, type FindResult, type FindError, type SemanticFindOptions } from './browser/find.js'; @@ -2064,8 +2064,31 @@ export async function loadAntigravityServe(pluginsDir: string = PLUGINS_DIR): Pr return import(pathToFileURL(path.join(pluginsDir, 'antigravity', 'serve.js')).href); } -export function runCli(BUILTIN_CLIS: string, USER_CLIS: string): void { - createProgram(BUILTIN_CLIS, USER_CLIS).parse(); +/** + * Run the local CLI, reporting anything a built-in command throws as the same + * error envelope adapter commands already emit. + * + * Built-in actions used to have no handler at all: `parse()` does not await + * async actions, so a throw either crashed with a raw Node stack trace or + * surfaced as an unhandled rejection, and the `exitCode` the error carried was + * lost. `parseAsync` lets the rejection reach this catch. + */ +export async function runCli(BUILTIN_CLIS: string, USER_CLIS: string): Promise { + try { + await createProgram(BUILTIN_CLIS, USER_CLIS).parseAsync(); + } catch (err) { + reportCliError(err); + } +} + +/** Render a thrown error as the shared envelope and set the exit code it carries. */ +export function reportCliError(err: unknown, stderr: NodeJS.WritableStream = process.stderr): void { + const envelope = toEnvelope(err); + if (process.env.WEBCMD_DEBUG && err instanceof Error && err.stack) { + envelope.error.stack = err.stack; + } + stderr.write(formatErrorEnvelope(envelope)); + process.exitCode = envelope.error.exitCode; } // ── Helpers ───────────────────────────────────────────────────────────────── diff --git a/src/main.ts b/src/main.ts index bf2aa7031..e24d4d1aa 100644 --- a/src/main.ts +++ b/src/main.ts @@ -211,5 +211,5 @@ try { } await emitHook('onStartup', { command: '__startup__', args: {} }); -runCli(BUILTIN_CLIS, USER_CLIS); +await runCli(BUILTIN_CLIS, USER_CLIS); } From 1b44eba6db7d3555df52439fa2aa33e9b2bc110a Mon Sep 17 00:00:00 2001 From: Ankit Ranjan Date: Wed, 19 Aug 2026 18:10:13 +0530 Subject: [PATCH 2/2] test: isolate doctor profiles and unflake plugin-pr-scope Doctor rendering was reading ~/.webcmd aliases. Windows git fixtures were hitting the 5s default timeout. --- src/doctor.test.ts | 3 +++ src/plugin-pr-scope.test.ts | 3 ++- 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/src/doctor.test.ts b/src/doctor.test.ts index bf6bf3bd6..5c797825f 100644 --- a/src/doctor.test.ts +++ b/src/doctor.test.ts @@ -65,10 +65,13 @@ afterAll(() => fs.rmSync(managedBinaryDir, { recursive: true, force: true })); describe('doctor report rendering', () => { const strip = (s: string) => s.replace(/\x1b\[[0-9;]*m/g, ''); + const isolatedConfigDir = fs.mkdtempSync(path.join(os.tmpdir(), 'webcmd-doctor-render-')); + afterAll(() => fs.rmSync(isolatedConfigDir, { recursive: true, force: true })); beforeEach(() => { vi.clearAllMocks(); vi.unstubAllEnvs(); + vi.stubEnv('WEBCMD_CONFIG_DIR', isolatedConfigDir); mockFindShadowedUserAdapters.mockReturnValue([]); mockSetDaemonCommandTimeoutSeconds.mockClear(); mockEnsureBinary.mockResolvedValue(managedBinaryPath); diff --git a/src/plugin-pr-scope.test.ts b/src/plugin-pr-scope.test.ts index 20d6ec1b6..a3c02c655 100644 --- a/src/plugin-pr-scope.test.ts +++ b/src/plugin-pr-scope.test.ts @@ -35,6 +35,7 @@ function fixture(change: (root: string) => void) { git(root, 'init', '--quiet'); git(root, 'config', 'user.email', 'test@example.com'); git(root, 'config', 'user.name', 'Test'); + git(root, 'config', 'commit.gpgsign', 'false'); write(root, 'README.md', 'before\n'); write(root, 'webcmd-plugin.json', '{}\n'); write(root, 'package.json', '{}\n'); @@ -56,7 +57,7 @@ function runGuard(root: string, ...args: string[]) { return spawnSync(process.execPath, [checkerPath, ...args], { cwd: root, encoding: 'utf8' }); } -describe('plugin-only PR scope guard', () => { +describe('plugin-only PR scope guard', { timeout: 20_000 }, () => { it('accepts changes contained in one newly added plugin', () => { const { root, base, head } = fixture((repo) => addPlugin(repo, 'foo'));