From 49e1ea2c9f0878597792fe62aef4bbba472bb9d6 Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Sat, 15 Aug 2026 00:44:18 -0700 Subject: [PATCH] fix(cli): redact machine-readable CLI error payloads (#124075) --- src/cli/channels-cli.ts | 3 +- src/cli/config-cli-runner.ts | 7 ++-- src/cli/config-cli-validation.ts | 3 +- src/cli/config-cli.test.ts | 9 +++-- src/cli/config-cli.ts | 17 +++++---- src/cli/cron-cli/register.cron-edit.ts | 3 +- src/cli/cron-cli/shared.ts | 3 +- src/cli/daemon-cli/status.test.ts | 9 +++-- src/cli/daemon-cli/status.ts | 3 +- src/cli/directory-cli.test.ts | 43 +++++++++++++++++++--- src/cli/directory-cli.ts | 6 ++-- src/cli/node-cli/daemon.test.ts | 18 +++++++--- src/cli/node-cli/daemon.ts | 14 +++++--- src/cli/program/message/helpers.test.ts | 18 +++++----- src/cli/program/message/helpers.ts | 6 ++-- src/cli/system-cli.test.ts | 29 +++++++++++++-- src/cli/system-cli.ts | 6 ++-- src/cli/webhooks-cli.test.ts | 48 +++++++++++++++++++------ src/cli/webhooks-cli.ts | 8 +++-- 19 files changed, 189 insertions(+), 64 deletions(-) diff --git a/src/cli/channels-cli.ts b/src/cli/channels-cli.ts index 347745ae04d3..38e21dd4accb 100644 --- a/src/cli/channels-cli.ts +++ b/src/cli/channels-cli.ts @@ -3,6 +3,7 @@ import { Option, type Command } from "commander"; import { formatDocsLink } from "../../packages/terminal-core/src/links.js"; import { theme } from "../../packages/terminal-core/src/theme.js"; import { danger } from "../globals.js"; +import { formatErrorMessage } from "../infra/errors.js"; import { defaultRuntime } from "../runtime.js"; import { createLazyImportLoader } from "../shared/lazy-promise.js"; import { resolveCliArgvInvocation } from "./argv-invocation.js"; @@ -70,7 +71,7 @@ function runChannelsCommand(action: () => Promise) { function runChannelsCommandWithDanger(action: () => Promise, label: string) { return runCommandWithRuntime(defaultRuntime, action, (err) => { - defaultRuntime.error(danger(`${label}: ${String(err)}`)); + defaultRuntime.error(danger(`${label}: ${formatErrorMessage(err)}`)); defaultRuntime.exit(1); }); } diff --git a/src/cli/config-cli-runner.ts b/src/cli/config-cli-runner.ts index 36389a8127e7..9e647f4d7477 100644 --- a/src/cli/config-cli-runner.ts +++ b/src/cli/config-cli-runner.ts @@ -10,6 +10,7 @@ import { diffConfigPaths } from "../gateway/config-diff.js"; import { buildGatewayReloadPlan } from "../gateway/config-reload-plan.js"; import { resolveGatewayReloadSettings } from "../gateway/config-reload-settings.js"; import { danger, info } from "../globals.js"; +import { formatErrorMessage } from "../infra/errors.js"; import type { RuntimeEnv } from "../runtime.js"; import { writeRuntimeJson } from "../runtime.js"; import { toDotPath } from "../shared/dot-path.js"; @@ -495,13 +496,13 @@ export function handleConfigMutationError(params: { runtime: RuntimeEnv; options: ConfigMutationOptions; }) { + const message = formatErrorMessage(params.err); if (params.options.dryRun && params.options.json) { if (params.err instanceof ConfigSetDryRunValidationError) { writeRuntimeJson(params.runtime, params.err.result); params.runtime.exit(1); return; } - const message = params.err instanceof Error ? params.err.message : String(params.err); const result: ConfigSetDryRunResult = { ok: false, operations: 0, @@ -513,10 +514,10 @@ export function handleConfigMutationError(params: { errors: [{ kind: "schema", message }], }; writeRuntimeJson(params.runtime, result); - params.runtime.error(danger(String(params.err))); + params.runtime.error(danger(message)); params.runtime.exit(1); return; } - params.runtime.error(danger(String(params.err))); + params.runtime.error(danger(message)); params.runtime.exit(1); } diff --git a/src/cli/config-cli-validation.ts b/src/cli/config-cli-validation.ts index 63f578b94414..4f83ab9ebb81 100644 --- a/src/cli/config-cli-validation.ts +++ b/src/cli/config-cli-validation.ts @@ -12,6 +12,7 @@ import { type SecretRef, } from "../config/types.secrets.js"; import { validateConfigObjectRawWithPlugins } from "../config/validation.js"; +import { formatErrorMessage } from "../infra/errors.js"; import { loadPluginMetadataSnapshot } from "../plugins/plugin-metadata-snapshot.js"; import { type RuntimeEnv, defaultRuntime, writeRuntimeJson } from "../runtime.js"; import { @@ -155,7 +156,7 @@ export async function collectDryRunResolvabilityErrors(params: { } catch (err) { failures.push({ kind: "resolvability", - message: String(err), + message: formatErrorMessage(err), ref: `${ref.source}:${ref.provider}:${ref.id}`, }); } diff --git a/src/cli/config-cli.test.ts b/src/cli/config-cli.test.ts index 4152fc472366..389c625546a3 100644 --- a/src/cli/config-cli.test.ts +++ b/src/cli/config-cli.test.ts @@ -3361,7 +3361,7 @@ describe("config cli", () => { }, refsChecked: 0, skippedExecRefs: 0, - errors: [{ kind: "schema", message }], + errors: [{ kind: "schema", message: expect.stringContaining(message) }], }); expectErrorIncludes(message); }); @@ -3398,7 +3398,10 @@ describe("config cli", () => { it("aggregates schema and resolvability failures in --dry-run --json mode", async () => { setGatewaySnapshot({ providers: { default: { source: "env" } } }); - mockResolveSecretRefValue.mockRejectedValue(new Error("missing env var")); + const secret = "sk-abcdefghijklmnopqrstuv"; + const error = new Error(`missing env var: Authorization: Bearer ${secret}`); + error.name = "SecretResolutionError"; + mockResolveSecretRefValue.mockRejectedValue(error); await expect( runConfigCommand([ @@ -3421,6 +3424,8 @@ describe("config cli", () => { expect(errorKinds).toContain("resolvability"); const errorRefs = (payload.errors ?? []).map((entry) => entry.ref ?? ""); expect(errorRefs).toContain("env:default:DISCORD_BOT_TOKEN"); + expect(JSON.stringify(payload)).not.toContain(error.name); + expect(JSON.stringify(payload)).not.toContain(secret); }); it("fails dry-run when provider updates make existing refs unresolvable", async () => { diff --git a/src/cli/config-cli.ts b/src/cli/config-cli.ts index 875842579e37..06e970c0abf4 100644 --- a/src/cli/config-cli.ts +++ b/src/cli/config-cli.ts @@ -10,6 +10,7 @@ import { redactConfigObject } from "../config/redact-snapshot.js"; import { readBestEffortRuntimeConfigSchema } from "../config/runtime-schema.js"; import type { OpenClawConfig } from "../config/types.openclaw.js"; import { danger, info, success, warn } from "../globals.js"; +import { formatErrorMessage } from "../infra/errors.js"; import { ExitError, type RuntimeEnv, @@ -182,11 +183,11 @@ export async function runConfigGet(opts: { path: string; json?: boolean; runtime throw err; } if (opts.json) { - writeRuntimeJson(runtime, { error: String(err) }); + writeRuntimeJson(runtime, { error: formatErrorMessage(err) }); runtime.exit(1); return; } - runtime.error(danger(String(err))); + runtime.error(danger(formatErrorMessage(err))); runtime.exit(1); } } @@ -288,7 +289,7 @@ async function runConfigFile(opts: { json?: boolean; runtime?: RuntimeEnv }) { } writeRuntimeStdout(runtime, `${path}\n`); } catch (err) { - runtime.error(danger(String(err))); + runtime.error(danger(formatErrorMessage(err))); runtime.exit(1); } } @@ -302,7 +303,7 @@ async function runConfigSchema(opts: { runtime?: RuntimeEnv } = {}) { schema.properties = { $schema: { type: "string" }, ...schema.properties }; writeRuntimeJson(runtime, schema); } catch (err) { - runtime.error(danger(`Config schema error: ${String(err)}`)); + runtime.error(danger(`Config schema error: ${formatErrorMessage(err)}`)); runtime.exit(1); } } @@ -368,9 +369,13 @@ async function runConfigValidate(opts: { json?: boolean; runtime?: RuntimeEnv } } } catch (err) { if (opts.json) { - writeRuntimeJson(runtime, { valid: false, path: outputPath, error: String(err) }, 0); + writeRuntimeJson( + runtime, + { valid: false, path: outputPath, error: formatErrorMessage(err) }, + 0, + ); } else { - runtime.error(danger(`Config validation error: ${String(err)}`)); + runtime.error(danger(`Config validation error: ${formatErrorMessage(err)}`)); } runtime.exit(1); } diff --git a/src/cli/cron-cli/register.cron-edit.ts b/src/cli/cron-cli/register.cron-edit.ts index 0ce70f7779c9..330061e21f6c 100644 --- a/src/cli/cron-cli/register.cron-edit.ts +++ b/src/cli/cron-cli/register.cron-edit.ts @@ -8,6 +8,7 @@ import type { Command } from "commander"; import { THINKING_LEVELS_HELP } from "../../auto-reply/thinking.shared.js"; import type { CronJob } from "../../cron/types.js"; import { danger } from "../../globals.js"; +import { formatErrorMessage } from "../../infra/errors.js"; import { sanitizeAgentId } from "../../routing/session-key.js"; import { defaultRuntime } from "../../runtime.js"; import { @@ -496,7 +497,7 @@ export function registerCronEditCommand(cron: Command) { defaultRuntime.writeJson(res); await warnIfCronSchedulerDisabled(opts); } catch (err) { - defaultRuntime.error(danger(String(err))); + defaultRuntime.error(danger(formatErrorMessage(err))); defaultRuntime.exit(1); } }), diff --git a/src/cli/cron-cli/shared.ts b/src/cli/cron-cli/shared.ts index 122dbafe34da..7108cb0d5ab8 100644 --- a/src/cli/cron-cli/shared.ts +++ b/src/cli/cron-cli/shared.ts @@ -12,6 +12,7 @@ import { parseAbsoluteTimeMs } from "../../cron/parse.js"; import { resolveCronStaggerMs } from "../../cron/stagger.js"; import type { CronDeliveryPreview, CronJob, CronSchedule } from "../../cron/types.js"; import { danger } from "../../globals.js"; +import { formatErrorMessage } from "../../infra/errors.js"; import { formatDurationHuman } from "../../infra/format-time/format-duration.ts"; import { isOffsetlessIsoDateTime, @@ -196,7 +197,7 @@ function formatCronStatusForDisplay(job: CronJob): string { } export function handleCronCliError(err: unknown) { - defaultRuntime.error(danger(String(err))); + defaultRuntime.error(danger(formatErrorMessage(err))); defaultRuntime.exit(1); } diff --git a/src/cli/daemon-cli/status.test.ts b/src/cli/daemon-cli/status.test.ts index 5216b6d6298b..abae75d2938a 100644 --- a/src/cli/daemon-cli/status.test.ts +++ b/src/cli/daemon-cli/status.test.ts @@ -139,7 +139,10 @@ describe("runDaemonStatus", () => { }); it("renders service-inspection failures as JSON in JSON mode", async () => { - gatherDaemonStatus.mockRejectedValueOnce(new Error("service manager unavailable")); + const secret = "sk-abcdefghijklmnopqrstuv"; + const error = new Error(`service manager unavailable: Authorization: Bearer ${secret}`); + error.name = "ServiceManagerError"; + gatherDaemonStatus.mockRejectedValueOnce(error); await expect( runDaemonStatus({ @@ -153,8 +156,10 @@ describe("runDaemonStatus", () => { expect(printDaemonStatus).not.toHaveBeenCalled(); expect(defaultRuntime.writeJson).toHaveBeenCalledWith({ ok: false, - error: "Gateway status failed: Error: service manager unavailable", + error: expect.stringContaining("Gateway status failed: service manager unavailable"), }); + expect(JSON.stringify(defaultRuntime.writeJson.mock.calls)).not.toContain(error.name); + expect(JSON.stringify(defaultRuntime.writeJson.mock.calls)).not.toContain(secret); expect(defaultRuntime.error).not.toHaveBeenCalled(); expect(defaultRuntime.exit).toHaveBeenCalledTimes(1); }); diff --git a/src/cli/daemon-cli/status.ts b/src/cli/daemon-cli/status.ts index e62677bd7632..f694474bb439 100644 --- a/src/cli/daemon-cli/status.ts +++ b/src/cli/daemon-cli/status.ts @@ -1,5 +1,6 @@ // Gateway service status command entrypoint: gathers status, prints it, and handles probe failures. import { colorize, isRich, theme } from "../../../packages/terminal-core/src/theme.js"; +import { formatErrorMessage } from "../../infra/errors.js"; import { defaultRuntime } from "../../runtime.js"; import { gatherDaemonStatus } from "./status.gather.js"; import { printDaemonStatus } from "./status.print.js"; @@ -34,7 +35,7 @@ export async function runDaemonStatus(opts: DaemonStatusOptions) { }); printDaemonStatus(status, { json: opts.json, deep: opts.deep === true }); } catch (err) { - failDaemonStatus(opts, `Gateway status failed: ${String(err)}`); + failDaemonStatus(opts, `Gateway status failed: ${formatErrorMessage(err)}`); return; } diff --git a/src/cli/directory-cli.test.ts b/src/cli/directory-cli.test.ts index 12957dde7612..11ae9b444c7e 100644 --- a/src/cli/directory-cli.test.ts +++ b/src/cli/directory-cli.test.ts @@ -316,17 +316,17 @@ describe("registerDirectoryCli", () => { [ "self", ["directory", "self", "--channel", "demo-directory", "--json"], - "Error: Channel demo-directory does not support directory self", + "Channel demo-directory does not support directory self", ], [ "peers", ["directory", "peers", "list", "--channel", "demo-directory", "--json"], - "Error: Channel demo-directory does not support directory peers", + "Channel demo-directory does not support directory peers", ], [ "groups", ["directory", "groups", "list", "--channel", "demo-directory", "--json"], - "Error: Channel demo-directory does not support directory groups", + "Channel demo-directory does not support directory groups", ], [ "group members", @@ -340,7 +340,7 @@ describe("registerDirectoryCli", () => { "group-1", "--json", ], - "Error: Channel demo-directory does not support group members listing", + "Channel demo-directory does not support group members listing", ], ])("writes JSON errors for unsupported directory %s", async (_label, args, expectedError) => { mocks.resolveInstallableChannelPlugin.mockResolvedValue({ @@ -364,6 +364,41 @@ describe("registerDirectoryCli", () => { expect(runtimeState.defaultRuntime.exit).toHaveBeenCalledWith(1); }); + it.each([ + { mode: "human", args: ["directory", "self", "--channel", "demo-directory"] }, + { + mode: "JSON", + args: ["directory", "self", "--channel", "demo-directory", "--json"], + }, + ])("renders named errors without class names in $mode mode", async ({ mode, args }) => { + const error = new Error("Multiple agents are configured, but this operation has no owner."); + error.name = "AgentSelectionRequiredError"; + const self = vi.fn().mockRejectedValue(error); + mocks.resolveInstallableChannelPlugin.mockResolvedValue({ + cfg: { channels: { "demo-directory": {} } }, + channelId: "demo-directory", + plugin: { id: "demo-directory", directory: { self } }, + configChanged: false, + }); + + const program = new Command().name("openclaw"); + registerDirectoryCli(program); + + await expect(program.parseAsync(args, { from: "user" })).rejects.toThrow("exit:1"); + + if (mode === "JSON") { + const payload = JSON.parse(runtimeState.runtimeLogs.at(-1) ?? ""); + expect(payload).toEqual({ error: error.message }); + expect(Object.keys(payload)).toEqual(["error"]); + expect(runtimeState.defaultRuntime.error).not.toHaveBeenCalled(); + } else { + expect(runtimeErrors()).toEqual([error.message]); + expect(runtimeState.defaultRuntime.writeJson).not.toHaveBeenCalled(); + } + expect([...runtimeState.runtimeLogs, ...runtimeErrors()].join("\n")).not.toContain(error.name); + expect(runtimeState.defaultRuntime.exit).toHaveBeenCalledWith(1); + }); + it.each([ ["peers list", ["directory", "peers", "list", "--channel", "slack", "--limit", "5x"]], ["groups list", ["directory", "groups", "list", "--channel", "slack", "--limit", "5x"]], diff --git a/src/cli/directory-cli.ts b/src/cli/directory-cli.ts index 379e9e0e998c..f736b56c92e3 100644 --- a/src/cli/directory-cli.ts +++ b/src/cli/directory-cli.ts @@ -16,6 +16,7 @@ import { resolveInstallableChannelPlugin } from "../commands/channel-setup/chann import { getRuntimeConfig, readConfigFileSnapshot, replaceConfigFile } from "../config/config.js"; import { applyPluginAutoEnable } from "../config/plugin-auto-enable.js"; import { danger } from "../globals.js"; +import { formatErrorMessage } from "../infra/errors.js"; import { resolveMessageChannelSelection } from "../infra/outbound/channel-selection.js"; import { commitConfigWithPendingPluginInstalls } from "../plugins/install-record-commit.js"; import { defaultRuntime } from "../runtime.js"; @@ -192,10 +193,11 @@ export function registerDirectoryCli(program: Command) { try { await action(); } catch (err) { + const message = formatErrorMessage(err); if (opts.json) { - defaultRuntime.writeJson({ error: String(err) }); + defaultRuntime.writeJson({ error: message }); } else { - defaultRuntime.error(danger(String(err))); + defaultRuntime.error(danger(message)); } defaultRuntime.exit(1); } diff --git a/src/cli/node-cli/daemon.test.ts b/src/cli/node-cli/daemon.test.ts index d5caec79dd65..631c2c135cad 100644 --- a/src/cli/node-cli/daemon.test.ts +++ b/src/cli/node-cli/daemon.test.ts @@ -427,7 +427,7 @@ describe("runNodeDaemonStatus", () => { await runNodeDaemonStatus(); expect(mocks.runtime.error).toHaveBeenCalledWith( - "Node service check failed: Error: systemd unavailable", + "Node service check failed: systemd unavailable", ); expect(mocks.runtime.exit).toHaveBeenCalledWith(1); expect(stdout()).not.toContain("not loaded"); @@ -435,27 +435,35 @@ describe("runNodeDaemonStatus", () => { }); it("reports a failed service check as JSON without inventing node status", async () => { - mocks.service.isLoaded.mockRejectedValue(new Error("systemd unavailable")); + const secret = "sk-abcdefghijklmnopqrstuv"; + const error = new Error(`systemd unavailable: Authorization: Bearer ${secret}`); + error.name = "ServiceManagerError"; + mocks.service.isLoaded.mockRejectedValue(error); await runNodeDaemonStatus({ json: true }); expect(mocks.runtime.writeJson).toHaveBeenCalledWith({ - error: "Node service check failed: Error: systemd unavailable", + error: expect.stringContaining("Node service check failed: systemd unavailable"), }); + expect(JSON.stringify(mocks.runtime.writeJson.mock.calls)).not.toContain(error.name); + expect(JSON.stringify(mocks.runtime.writeJson.mock.calls)).not.toContain(secret); expect(mocks.runtime.exit).toHaveBeenCalledWith(1); expect(mocks.runtime.error).not.toHaveBeenCalled(); }); it("reports an unknown runtime when runtime inspection fails", async () => { - mocks.service.readRuntime.mockRejectedValue(new Error("permission denied")); + const error = new Error("permission denied"); + error.name = "RuntimeInspectionError"; + mocks.service.readRuntime.mockRejectedValue(error); await runNodeDaemonStatus({ json: true }); expect(mocks.runtime.writeJson).toHaveBeenCalledWith({ service: expect.objectContaining({ - runtime: { status: "unknown", detail: "Error: permission denied" }, + runtime: { status: "unknown", detail: "permission denied" }, }), }); + expect(JSON.stringify(mocks.runtime.writeJson.mock.calls)).not.toContain(error.name); }); it("keeps missing service-unit status on stderr and prints recovery hints on stdout", async () => { diff --git a/src/cli/node-cli/daemon.ts b/src/cli/node-cli/daemon.ts index 62d41470b27d..3be8450ab95c 100644 --- a/src/cli/node-cli/daemon.ts +++ b/src/cli/node-cli/daemon.ts @@ -21,6 +21,7 @@ import { readSystemdUserLingerStatus, resolveSystemdUserServiceAccount, } from "../../daemon/systemd.js"; +import { formatErrorMessage } from "../../infra/errors.js"; import { loadNodeHostConfig } from "../../node-host/config.js"; import { defaultRuntime } from "../../runtime.js"; import { formatCliCommand } from "../command-format.js"; @@ -147,7 +148,7 @@ export async function runNodeDaemonInstall(opts: NodeDaemonInstallOptions) { try { loaded = await service.isLoaded({ env: process.env }); } catch (err) { - fail(`Node service check failed: ${String(err)}`); + fail(`Node service check failed: ${formatErrorMessage(err)}`); return; } if (loaded && !opts.force) { @@ -259,7 +260,7 @@ export async function runNodeDaemonStatus(opts: NodeDaemonStatusOptions = {}) { try { loaded = await service.isLoaded({ env: process.env }); } catch (error) { - const message = `Node service check failed: ${String(error)}`; + const message = `Node service check failed: ${formatErrorMessage(error)}`; if (json) { defaultRuntime.writeJson({ error: message }); } else { @@ -270,9 +271,12 @@ export async function runNodeDaemonStatus(opts: NodeDaemonStatusOptions = {}) { } const [command, runtime] = await Promise.all([ service.readCommand(process.env).catch(() => null), - service - .readRuntime(process.env) - .catch((err: unknown): GatewayServiceRuntime => ({ status: "unknown", detail: String(err) })), + service.readRuntime(process.env).catch( + (err: unknown): GatewayServiceRuntime => ({ + status: "unknown", + detail: formatErrorMessage(err), + }), + ), ]); const payload = { diff --git a/src/cli/program/message/helpers.test.ts b/src/cli/program/message/helpers.test.ts index 586e45c50e7f..60e2bf9f3be9 100644 --- a/src/cli/program/message/helpers.test.ts +++ b/src/cli/program/message/helpers.test.ts @@ -289,7 +289,7 @@ describe("runMessageAction", () => { await runSendAction(); expect(messageCommandMock).not.toHaveBeenCalled(); - expect(errorMock).toHaveBeenCalledWith("Error: plugin load failed"); + expect(errorMock).toHaveBeenCalledWith("plugin load failed"); expect(exitMock).toHaveBeenCalledOnce(); expect(exitMock).toHaveBeenCalledWith(1); expect(exitMock).not.toHaveBeenCalledWith(0); @@ -310,7 +310,7 @@ describe("runMessageAction", () => { ).rejects.toThrow("exit"); expect(errorMock).toHaveBeenCalledWith( - "Error: --poll-anonymous and --poll-public are mutually exclusive.", + "--poll-anonymous and --poll-public are mutually exclusive.", ); expect(loadPluginRegistryHandleMock).not.toHaveBeenCalled(); expect(messageCommandMock).not.toHaveBeenCalled(); @@ -377,7 +377,7 @@ describe("runMessageAction", () => { await expect(runMessageAction(action, opts)).rejects.toThrow("exit"); const kind = NON_NEGATIVE_INTEGER_FLAGS.has(flag) ? "non-negative" : "positive"; - expect(errorMock).toHaveBeenCalledWith(`Error: ${flag} must be a ${kind} integer.`); + expect(errorMock).toHaveBeenCalledWith(`${flag} must be a ${kind} integer.`); expect(loadPluginRegistryHandleMock).not.toHaveBeenCalled(); expect(messageCommandMock).not.toHaveBeenCalled(); expect(exitMock).toHaveBeenCalledWith(1); @@ -402,7 +402,7 @@ describe("runMessageAction", () => { ).rejects.toThrow("exit"); const kind = NON_NEGATIVE_INTEGER_FLAGS.has(flag) ? "non-negative" : "positive"; - expect(errorMock).toHaveBeenCalledWith(`Error: ${flag} must be a ${kind} integer.`); + expect(errorMock).toHaveBeenCalledWith(`${flag} must be a ${kind} integer.`); expect(messageCommandMock).not.toHaveBeenCalled(); expect(exitMock).toHaveBeenCalledWith(1); }); @@ -496,7 +496,7 @@ describe("runMessageAction", () => { messageCommandMock.mockRejectedValueOnce(new Error("send failed")); await runSendAction(); - expect(errorMock).toHaveBeenCalledWith("Error: send failed"); + expect(errorMock).toHaveBeenCalledWith("send failed"); expect(exitMock).toHaveBeenCalledOnce(); expect(exitMock).toHaveBeenCalledWith(1); }); @@ -515,7 +515,7 @@ describe("runMessageAction", () => { runGatewayStopMock.mockRejectedValueOnce(new Error("hook failed")); await runSendAction(); - expect(errorMock).toHaveBeenCalledWith("gateway_stop hook failed: Error: hook failed"); + expect(errorMock).toHaveBeenCalledWith("gateway_stop hook failed: hook failed"); expect(exitMock).toHaveBeenCalledWith(0); }); @@ -525,8 +525,8 @@ describe("runMessageAction", () => { runGatewayStopMock.mockRejectedValueOnce(new Error("hook failed")); await runSendAction(); - expect(errorMock).toHaveBeenNthCalledWith(1, "Error: send failed"); - expect(errorMock).toHaveBeenNthCalledWith(2, "gateway_stop hook failed: Error: hook failed"); + expect(errorMock).toHaveBeenNthCalledWith(1, "send failed"); + expect(errorMock).toHaveBeenNthCalledWith(2, "gateway_stop hook failed: hook failed"); expect(exitMock).toHaveBeenCalledWith(1); }); @@ -545,7 +545,7 @@ describe("runMessageAction", () => { const runMessageAction = createRunMessageAction(); await expect(runMessageAction("send", baseSendOptions)).resolves.toBeUndefined(); - expect(errorMock).toHaveBeenCalledWith("Error: boom"); + expect(errorMock).toHaveBeenCalledWith("boom"); expect(exitMock).toHaveBeenCalledOnce(); expect(exitMock).toHaveBeenCalledWith(1); expect(exitMock).not.toHaveBeenCalledWith(0); diff --git a/src/cli/program/message/helpers.ts b/src/cli/program/message/helpers.ts index 0080cb1904ca..24b6253bdce9 100644 --- a/src/cli/program/message/helpers.ts +++ b/src/cli/program/message/helpers.ts @@ -13,6 +13,7 @@ import { resolveMessageSecretScope } from "../../../cli/message-secret-scope.js" import { messageCommand } from "../../../commands/message.js"; import { getRuntimeConfig } from "../../../config/config.js"; import { danger, setVerbose } from "../../../globals.js"; +import { formatErrorMessage } from "../../../infra/errors.js"; import { CHANNEL_TARGET_DESCRIPTION } from "../../../infra/outbound/channel-target.js"; import { withActivatedPluginIds } from "../../../plugins/activation-context.js"; import { @@ -84,7 +85,8 @@ async function runPluginStopHooks(): Promise { const hookRun = runGlobalGatewayStopSafely({ event: { reason: "cli message action complete" }, ctx: {}, - onError: (err) => defaultRuntime.error(danger(`gateway_stop hook failed: ${String(err)}`)), + onError: (err) => + defaultRuntime.error(danger(`gateway_stop hook failed: ${formatErrorMessage(err)}`)), }); const bounded = new Promise<"timeout">((resolve) => { timeout = setTimeout(() => resolve("timeout"), GATEWAY_STOP_TIMEOUT_MS); @@ -210,7 +212,7 @@ export function createMessageCliHelpers( }, (err) => { failed = true; - defaultRuntime.error(danger(String(err))); + defaultRuntime.error(danger(formatErrorMessage(err))); }, ); // Outbound actions may start plugin-side resources; run bounded stop hooks even after failure. diff --git a/src/cli/system-cli.test.ts b/src/cli/system-cli.test.ts index 763356504178..92f7a6b3eb9d 100644 --- a/src/cli/system-cli.test.ts +++ b/src/cli/system-cli.test.ts @@ -95,14 +95,14 @@ describe("system-cli", () => { name: "invalid wake mode", args: ["system", "event", "--text", "hello", "--mode", "later", "--json"], gatewayResult: undefined, - expectedError: "Error: --mode must be now or next-heartbeat", + expectedError: "--mode must be now or next-heartbeat", gatewayCalls: 0, }, { name: "rejected Gateway call", args: ["system", "event", "--text", "hello", "--json"], gatewayResult: { ok: false, reason: "unwakeable-session-key" }, - expectedError: "Error: unwakeable-session-key", + expectedError: "unwakeable-session-key", gatewayCalls: 1, }, ])( @@ -122,6 +122,29 @@ describe("system-cli", () => { }, ); + it.each([ + { mode: "human", args: ["system", "event", "--text", "hello"] }, + { mode: "JSON", args: ["system", "event", "--text", "hello", "--json"] }, + ])("renders named errors without class names in $mode mode", async ({ mode, args }) => { + const error = new Error("Multiple agents are configured, but this operation has no owner."); + error.name = "AgentSelectionRequiredError"; + callGatewayFromCli.mockRejectedValueOnce(error); + + await runCli(args); + + if (mode === "JSON") { + const payload = JSON.parse(runtimeLogs.at(-1) ?? ""); + expect(payload).toEqual({ error: error.message }); + expect(Object.keys(payload)).toEqual(["error"]); + expect(runtimeErrors).toEqual([]); + } else { + expect(runtimeErrors).toEqual([error.message]); + expect(defaultRuntime.writeJson).not.toHaveBeenCalled(); + } + expect([...runtimeLogs, ...runtimeErrors].join("\n")).not.toContain(error.name); + expect(defaultRuntime.exit).toHaveBeenCalledWith(1); + }); + it("forwards --session-key on system event", async () => { await runCli([ "system", @@ -171,7 +194,7 @@ describe("system-cli", () => { expect(typeof gatewayOptions).toBe("object"); expect(params).toBeUndefined(); expect(requestOptions).toEqual({ expectFinal: false }); - const expectedError = "Error: Gateway unavailable"; + const expectedError = "Gateway unavailable"; expect(runtimeLogs).toEqual([JSON.stringify({ error: expectedError }, null, 2)]); expect(runtimeErrors).toEqual([]); expect(defaultRuntime.writeJson).toHaveBeenCalledWith({ error: expectedError }); diff --git a/src/cli/system-cli.ts b/src/cli/system-cli.ts index 8b54041de5c5..a3cf4a249342 100644 --- a/src/cli/system-cli.ts +++ b/src/cli/system-cli.ts @@ -4,6 +4,7 @@ import type { Command } from "commander"; import { formatDocsLink } from "../../packages/terminal-core/src/links.js"; import { theme } from "../../packages/terminal-core/src/theme.js"; import { danger } from "../globals.js"; +import { formatErrorMessage } from "../infra/errors.js"; import { defaultRuntime } from "../runtime.js"; import { formatCliCommand } from "./command-format.js"; import type { GatewayRpcOpts } from "./gateway-rpc.js"; @@ -44,10 +45,11 @@ async function runSystemGatewayCommand( defaultRuntime.log(successText); } } catch (err) { + const message = formatErrorMessage(err); if (machineOutput) { - defaultRuntime.writeJson({ error: String(err) }); + defaultRuntime.writeJson({ error: message }); } else { - defaultRuntime.error(danger(String(err))); + defaultRuntime.error(danger(message)); } defaultRuntime.exit(1); } diff --git a/src/cli/webhooks-cli.test.ts b/src/cli/webhooks-cli.test.ts index 9dd1d7bdd52d..2196b543f380 100644 --- a/src/cli/webhooks-cli.test.ts +++ b/src/cli/webhooks-cli.test.ts @@ -34,6 +34,7 @@ function runtimeErrors(): string[] { describe("webhooks cli", () => { beforeEach(() => { + mocks.runtimeLogs.length = 0; mocks.runtimeErrors.length = 0; mocks.defaultRuntime.error.mockClear(); mocks.defaultRuntime.writeJson.mockClear(); @@ -63,7 +64,7 @@ describe("webhooks cli", () => { if (json) { expect(mocks.defaultRuntime.writeJson).toHaveBeenCalledWith({ - error: `Error: ${flag} must be a positive integer.`, + error: `${flag} must be a positive integer.`, }); expect(mocks.defaultRuntime.error).not.toHaveBeenCalled(); } else { @@ -73,19 +74,44 @@ describe("webhooks cli", () => { expect(mocks.runGmailService).not.toHaveBeenCalled(); }); - it("writes JSON when gmail setup rejects", async () => { - mocks.runGmailSetup.mockRejectedValueOnce(new Error("setup failed")); + it.each([ + { command: "setup", mode: "human", json: false }, + { command: "setup", mode: "JSON", json: true }, + { command: "run", mode: "human", json: false }, + ])("renders named gmail $command errors safely in $mode mode", async ({ command, json }) => { + const secret = "sk-abcdefghijklmnopqrstuv"; + const error = new Error(`Gmail failed: Authorization: Bearer ${secret}`); + error.name = "GmailCredentialError"; + const runner = command === "setup" ? mocks.runGmailSetup : mocks.runGmailService; + runner.mockRejectedValueOnce(error); const program = createProgram(); + const args = + command === "setup" + ? ["webhooks", "gmail", "setup", "--account", "default"] + : ["webhooks", "gmail", "run"]; + if (json) { + args.push("--json"); + } - await expect( - program.parseAsync(["webhooks", "gmail", "setup", "--account", "default", "--json"], { - from: "user", - }), - ).rejects.toThrow("__exit__:1"); + await expect(program.parseAsync(args, { from: "user" })).rejects.toThrow("__exit__:1"); - expect(mocks.runGmailSetup).toHaveBeenCalledWith(expect.objectContaining({ json: true })); - expect(mocks.defaultRuntime.writeJson).toHaveBeenCalledWith({ error: "Error: setup failed" }); - expect(mocks.defaultRuntime.error).not.toHaveBeenCalled(); + expect(runner).toHaveBeenCalledOnce(); + if (json) { + const payload = JSON.parse(mocks.runtimeLogs.at(-1) ?? ""); + expect(payload).toEqual({ + error: expect.stringContaining("Gmail failed: Authorization: Bearer"), + }); + expect(Object.keys(payload)).toEqual(["error"]); + expect(mocks.defaultRuntime.error).not.toHaveBeenCalled(); + } else { + expect(runtimeErrors()).toEqual([ + expect.stringContaining("Gmail failed: Authorization: Bearer"), + ]); + expect(mocks.defaultRuntime.writeJson).not.toHaveBeenCalled(); + } + expect([...mocks.runtimeLogs, ...runtimeErrors()].join("\n")).not.toContain(error.name); + expect([...mocks.runtimeLogs, ...runtimeErrors()].join("\n")).not.toContain(secret); + expect(mocks.defaultRuntime.exit).toHaveBeenCalledWith(1); }); it.each([ diff --git a/src/cli/webhooks-cli.ts b/src/cli/webhooks-cli.ts index c36320ca3372..8e92a6212da5 100644 --- a/src/cli/webhooks-cli.ts +++ b/src/cli/webhooks-cli.ts @@ -21,6 +21,7 @@ import { DEFAULT_GMAIL_SUBSCRIPTION, DEFAULT_GMAIL_TOPIC, } from "../hooks/gmail.js"; +import { formatErrorMessage } from "../infra/errors.js"; import { defaultRuntime } from "../runtime.js"; import { formatCliCommand } from "./command-format.js"; @@ -71,10 +72,11 @@ export function registerWebhooksCli(program: Command) { const parsed = parseGmailSetupOptions(opts); await runGmailSetup(parsed); } catch (err) { + const message = formatErrorMessage(err); if (opts.json) { - defaultRuntime.writeJson({ error: String(err) }); + defaultRuntime.writeJson({ error: message }); } else { - defaultRuntime.error(danger(String(err))); + defaultRuntime.error(danger(message)); } defaultRuntime.exit(1); } @@ -107,7 +109,7 @@ export function registerWebhooksCli(program: Command) { const parsed = parseGmailRunOptions(opts); await runGmailService(parsed); } catch (err) { - defaultRuntime.error(danger(String(err))); + defaultRuntime.error(danger(formatErrorMessage(err))); defaultRuntime.exit(1); } });