diff --git a/docs/cli/hooks.md b/docs/cli/hooks.md index e2009ca4332f..4f8bea261053 100644 --- a/docs/cli/hooks.md +++ b/docs/cli/hooks.md @@ -125,6 +125,7 @@ grep '"action":"new"' ~/.openclaw/logs/commands.log | jq . # filter by action ## Notes - `hooks list --json`, `info --json`, and `check --json` write structured JSON directly to stdout. +- Failed hook reports use the standard [CLI JSON failure envelope](/cli#json-failures); missing hook info also includes the requested `hook` name. - `hooks list`, `info`, and `check` pass `--agent` to a running Gateway and preserve it when falling back to local read-only discovery against an older or unavailable Gateway. ## Related diff --git a/src/cli/hooks-cli.format.ts b/src/cli/hooks-cli.format.ts index ffbf43616a50..757b8480a0cf 100644 --- a/src/cli/hooks-cli.format.ts +++ b/src/cli/hooks-cli.format.ts @@ -11,6 +11,7 @@ import type { HookStatusEntry, HookStatusReport } from "../hooks/hooks-status.js import { summarizeStringEntries } from "../shared/string-sample.js"; import { shortenHomePath } from "../utils.js"; import { formatCliCommand } from "./command-format.js"; +import { formatCliJsonFailure } from "./failure-output.js"; export type HooksListOptions = { agent?: string; @@ -181,7 +182,8 @@ export function formatHookInfo( if (!hook) { if (opts.json) { - return JSON.stringify({ error: "not found", hook: hookName }, null, 2); + const failure = formatCliJsonFailure(`Hook "${hookName}" not found.`); + return JSON.stringify({ ...failure, hook: hookName }, null, 2); } return `Hook "${hookName}" not found. Run \`${formatCliCommand("openclaw hooks list")}\` to see available hooks.`; } diff --git a/src/cli/hooks-cli.test.ts b/src/cli/hooks-cli.test.ts index 8bf25ce6f742..d60e25b0c68e 100644 --- a/src/cli/hooks-cli.test.ts +++ b/src/cli/hooks-cli.test.ts @@ -178,6 +178,14 @@ describe("hooks cli formatting", () => { expect(output).toContain("DEMO_HOOK_TOKEN"); }); + it("keeps the missing hook identifier beside the canonical JSON failure", () => { + expect(JSON.parse(formatHookInfo(report, "missing-hook", { json: true }))).toEqual({ + ok: false, + error: { type: "cli_error", message: 'Hook "missing-hook" not found.' }, + hook: "missing-hook", + }); + }); + it("labels hooks status output", () => { const output = formatHooksCheck(report, {}); expect(output).toContain("Hooks Status"); diff --git a/src/cli/hooks-cli.toggle.test.ts b/src/cli/hooks-cli.toggle.test.ts index 03c9cbd688bc..36df06b8a616 100644 --- a/src/cli/hooks-cli.toggle.test.ts +++ b/src/cli/hooks-cli.toggle.test.ts @@ -4,6 +4,7 @@ import { beforeEach, describe, expect, it, vi } from "vitest"; import type { OpenClawConfig } from "../config/types.openclaw.js"; import { resolveConfiguredInternalHookNames } from "../hooks/configured.js"; import type { HookStatusEntry, HookStatusReport } from "../hooks/hooks-status.js"; +import { ExpectedCliError } from "./failure-output.js"; import { createEmptyInstallChecks } from "./requirements-test-fixtures.js"; import { createCliRuntimeCapture } from "./test-runtime-capture.js"; @@ -361,13 +362,151 @@ describe("hooks CLI metadata config keys", () => { from: "user", }); - expect(capture.defaultRuntime.writeStdout).toHaveBeenCalledWith( - expect.stringContaining('"error": "not found"'), - ); + expect(JSON.parse(capture.runtimeLogs.at(-1) ?? "{}")).toEqual({ + ok: false, + error: { type: "cli_error", message: 'Hook "missing-hook" not found.' }, + hook: "missing-hook", + }); expect(mocks.requestExitAfterOneShotOutput).toHaveBeenCalledWith(capture.defaultRuntime, 1); expect(mocks.replaceConfigFile).not.toHaveBeenCalled(); }); + it.each([ + { + label: "bare report with parent JSON", + argv: ["hooks", "--agent", "retired", "--json"], + message: 'Unknown agent id "retired"', + phase: "agent", + }, + { + label: "list with leaf JSON", + argv: ["hooks", "list", "--agent", "retired", "--json"], + message: 'Unknown agent id "retired"', + phase: "agent", + }, + { + label: "list with parent JSON", + argv: ["hooks", "--json", "list", "--agent", "retired"], + message: 'Unknown agent id "retired"', + phase: "agent", + }, + { + label: "info report", + argv: ["hooks", "info", "display-name", "--agent", "retired", "--json"], + message: 'Unknown agent id "retired"', + phase: "agent", + }, + { + label: "info report with parent JSON", + argv: ["hooks", "--json", "info", "display-name", "--agent", "retired"], + message: 'Unknown agent id "retired"', + phase: "agent", + }, + { + label: "check report", + argv: ["hooks", "check", "--agent", "retired", "--json"], + message: 'Unknown agent id "retired"', + phase: "agent", + }, + { + label: "check report with parent JSON", + argv: ["hooks", "--json", "check", "--agent", "retired"], + message: 'Unknown agent id "retired"', + phase: "agent", + }, + { + label: "blank leaf agent", + argv: ["hooks", "list", "--agent", "", "--json"], + message: "--agent must not be blank", + phase: "agent", + }, + { + label: "human report", + argv: ["hooks", "list", "--agent", "retired"], + message: 'Unknown agent id "retired"', + phase: "agent", + }, + { + label: "config loading", + argv: ["hooks", "list", "--json"], + message: "injected config loading failure", + phase: "config", + }, + { + label: "authoritative Gateway report", + argv: ["hooks", "check", "--json"], + message: "injected Gateway report failure", + phase: "gateway", + }, + { + label: "local report fallback", + argv: ["hooks", "info", "display-name", "--json"], + message: "injected local hook report failure", + phase: "report", + }, + ])("propagates $label failures to the root CLI renderer", async (testCase) => { + if (testCase.phase === "config") { + mocks.getRuntimeConfig.mockImplementation(() => { + throw new Error(testCase.message); + }); + } + if (testCase.phase === "gateway") { + mocks.callGateway.mockRejectedValue( + Object.assign(new Error(testCase.message), { + name: "GatewayClientRequestError", + gatewayCode: "INVALID_REQUEST", + }), + ); + } + if (testCase.phase === "report") { + mocks.buildWorkspaceHookStatus.mockImplementation(() => { + throw new Error(testCase.message); + }); + } + + const execution = createHooksProgram().parseAsync(testCase.argv, { from: "user" }); + await expect(execution).rejects.toBeInstanceOf(ExpectedCliError); + await expect(execution).rejects.toMatchObject({ + message: testCase.message, + humanOutput: `Error: ${testCase.message}`, + machineOutput: testCase.message, + }); + + expect(capture.defaultRuntime.error).not.toHaveBeenCalled(); + expect(capture.defaultRuntime.exit).not.toHaveBeenCalled(); + expect(capture.defaultRuntime.writeStdout).not.toHaveBeenCalled(); + expect(mocks.requestExitAfterOneShotOutput).not.toHaveBeenCalled(); + expect(mocks.replaceConfigFile).not.toHaveBeenCalled(); + if (testCase.phase === "agent" || testCase.phase === "config") { + expect(mocks.callGateway).not.toHaveBeenCalled(); + expect(mocks.buildWorkspaceHookStatus).not.toHaveBeenCalled(); + } + if (testCase.phase === "agent") { + expect(mocks.resolveDefaultAgentId).not.toHaveBeenCalled(); + } + if (testCase.phase === "gateway") { + expect(mocks.buildWorkspaceHookStatus).not.toHaveBeenCalled(); + } + }); + + it("preserves an existing expected read failure for root rendering", async () => { + const failure = new ExpectedCliError({ + message: "existing root failure", + humanOutput: "already styled failure", + machineOutput: "machine failure", + }); + mocks.getRuntimeConfig.mockImplementation(() => { + throw failure; + }); + + await expect( + createHooksProgram().parseAsync(["hooks", "list", "--json"], { from: "user" }), + ).rejects.toBe(failure); + expect(capture.defaultRuntime.error).not.toHaveBeenCalled(); + expect(capture.defaultRuntime.exit).not.toHaveBeenCalled(); + expect(mocks.callGateway).not.toHaveBeenCalled(); + }); + it("emits the default hooks report as JSON", async () => { await createHooksProgram().parseAsync(["hooks", "--json"], { from: "user" }); @@ -489,20 +628,6 @@ describe("hooks CLI metadata config keys", () => { expect(explicitFleet).toEqual(initialConfig); }); - it("rejects a blank hook agent before resolving a workspace", async () => { - configureExplicitFleet(); - - await expect( - createHooksProgram().parseAsync(["hooks", "list", "--agent", "", "--json"], { - from: "user", - }), - ).rejects.toThrow("__exit__:1"); - - expect(capture.runtimeErrors.at(-1)).toContain("--agent must not be blank"); - expect(mocks.resolveDefaultAgentId).not.toHaveBeenCalled(); - expect(mocks.callGateway).not.toHaveBeenCalled(); - }); - it("rejects a blank parent hook agent before dispatching a subcommand", async () => { await expect( createHooksProgram().parseAsync(["hooks", "--agent", "", "list"], { from: "user" }), @@ -535,9 +660,12 @@ describe("hooks CLI metadata config keys", () => { await expect( createHooksProgram().parseAsync(["hooks", "list", "--json"], { from: "user" }), - ).rejects.toThrow("__exit__:1"); + ).rejects.toMatchObject({ + name: "ExpectedCliError", + message: 'unknown agent id "retired"', + }); - expect(capture.runtimeErrors.at(-1)).toContain('unknown agent id "retired"'); + expect(capture.runtimeErrors).toEqual([]); expect(mocks.buildWorkspaceHookStatus).not.toHaveBeenCalled(); }); diff --git a/src/cli/hooks-cli.ts b/src/cli/hooks-cli.ts index 355606128567..8661ae076bcc 100644 --- a/src/cli/hooks-cli.ts +++ b/src/cli/hooks-cli.ts @@ -29,7 +29,7 @@ import { defaultRuntime } from "../runtime.js"; import { summarizeStringEntries } from "../shared/string-sample.js"; import { resolveOptionFromCommand } from "./cli-utils.js"; import { formatCliCommand } from "./command-format.js"; -import { rethrowExpectedCliError } from "./failure-output.js"; +import { ExpectedCliError, rethrowExpectedCliError } from "./failure-output.js"; import { formatHookInfo, formatHookMissingSummary, @@ -209,13 +209,6 @@ function buildConfigWithHookEnabled(params: { }; } -function exitHooksCliWithError(err: unknown): never { - rethrowExpectedCliError(err); - defaultRuntime.error(`${theme.error("Error:")} ${formatErrorMessage(err)}`); - defaultRuntime.exit(1); - throw new Error("unreachable"); -} - function writeHooksOutput(value: string, json: boolean | undefined): void { if (json) { defaultRuntime.writeStdout(value); @@ -224,16 +217,21 @@ function writeHooksOutput(value: string, json: boolean | undefined): void { defaultRuntime.log(value); } -async function runHooksCliAction(action: () => Promise | T): Promise { - try { - return await action(); - } catch (err) { - return exitHooksCliWithError(err); - } -} - -async function runOneShotHooksCliAction(action: () => Promise): Promise { - const result = await runHooksCliAction(action); +async function runOneShotHooksCliAction( + action: () => Promise, + failureOwner: "command" | "root" = "command", +): Promise { + const result = await action().catch((err: unknown) => { + rethrowExpectedCliError(err); + const message = formatErrorMessage(err); + const humanOutput = `${theme.error("Error:")} ${message}`; + if (failureOwner === "root") { + throw new ExpectedCliError({ message, humanOutput, machineOutput: message }); + } + defaultRuntime.error(humanOutput); + defaultRuntime.exit(1); + throw new Error("unreachable"); + }); const exitCode = typeof result === "number" ? result : 0; // CLI setup and handlers can leave ref'd handles behind. Defer exit until // runCli finishes shared teardown and drains both output streams. @@ -326,7 +324,7 @@ export function registerHooksCli(program: Command): void { const report = await loadHooksReport(resolveHooksAgentOption(command)); const json = hasJsonOutput(opts); writeHooksOutput(formatHooksList(report, { ...opts, json }), json); - }), + }, "root"), ); hooks @@ -340,7 +338,7 @@ export function registerHooksCli(program: Command): void { const json = hasJsonOutput(opts); writeHooksOutput(formatHookInfo(report, name, { ...opts, json }), json); return report.hooks.some((hook) => hook.name === name || hook.hookKey === name) ? 0 : 1; - }), + }, "root"), ); hooks @@ -353,7 +351,7 @@ export function registerHooksCli(program: Command): void { const report = await loadHooksReport(resolveHooksAgentOption(command)); const json = hasJsonOutput(opts); writeHooksOutput(formatHooksCheck(report, { ...opts, json }), json); - }), + }, "root"), ); hooks @@ -440,6 +438,6 @@ export function registerHooksCli(program: Command): void { const report = await loadHooksReport(resolveHooksAgentOption(command)); const json = hasJsonOutput(opts); writeHooksOutput(formatHooksList(report, { ...opts, json }), json); - }), + }, "root"), ); } diff --git a/test/cli-json-stdout.e2e.test.ts b/test/cli-json-stdout.e2e.test.ts index 64f405832d02..88b0383ddaf7 100644 --- a/test/cli-json-stdout.e2e.test.ts +++ b/test/cli-json-stdout.e2e.test.ts @@ -52,6 +52,183 @@ async function seedTrajectorySession(tempHome: string, sessionKey: string) { } describe("cli json stdout contract", () => { + it.each([ + { + name: "bare report with parent JSON", + args: ["hooks", "--agent", "retired", "--json"], + }, + { + name: "list report with leaf JSON", + args: ["hooks", "list", "--agent", "retired", "--json"], + }, + { + name: "list report with parent JSON", + args: ["hooks", "--json", "list", "--agent", "retired"], + }, + { + name: "info report with leaf JSON", + args: ["hooks", "info", "demo", "--agent", "retired", "--json"], + }, + { + name: "info report with parent JSON", + args: ["hooks", "--json", "info", "demo", "--agent", "retired"], + }, + { + name: "check report with leaf JSON", + args: ["hooks", "check", "--agent", "retired", "--json"], + }, + { + name: "check report with parent JSON", + args: ["hooks", "--json", "check", "--agent", "retired"], + }, + { + name: "blank leaf agent", + args: ["hooks", "list", "--agent", "", "--json"], + message: "--agent must not be blank", + }, + { + name: "blank parent agent", + args: ["hooks", "--agent", "", "--json", "list"], + message: "--agent must not be blank", + }, + { + name: "human report", + args: ["hooks", "list", "--agent", "retired"], + human: true, + }, + { + name: "forced Commander report", + args: ["hooks", "list", "--agent", "retired", "--json"], + commander: true, + }, + { + name: "dual-TTY report", + args: ["hooks", "check", "--agent", "retired", "--json"], + tty: true, + }, + { + name: "injected local report failure", + args: ["hooks", "list", "--json"], + message: "injected hook report loading failure", + reportFailure: true, + }, + { + name: "missing hook with leaf JSON", + args: ["hooks", "info", "missing-hook", "--json"], + message: 'Hook "missing-hook" not found.', + missingHook: true, + }, + { + name: "missing hook with parent JSON", + args: ["hooks", "--json", "info", "missing-hook"], + message: 'Hook "missing-hook" not found.', + missingHook: true, + }, + { + name: "missing hook through dual-TTY finalization", + args: ["hooks", "info", "missing-hook", "--json"], + message: 'Hook "missing-hook" not found.', + missingHook: true, + tty: true, + }, + { + name: "missing hook in human mode", + args: ["hooks", "info", "missing-hook"], + message: 'Hook "missing-hook" not found. Run `openclaw hooks list` to see available hooks.', + missingHook: true, + human: true, + }, + ])("renders hooks read failures through their canonical owner for $name", async (testCase) => { + await withTempHome( + async (tempHome) => { + const stateDir = path.join(tempHome, "isolated-state"); + const configPath = path.join(tempHome, "missing-openclaw.json"); + const workspaceHooksDir = path.join(stateDir, "workspace", "hooks"); + if ("reportFailure" in testCase) { + await fs.mkdir(workspaceHooksDir, { recursive: true }); + } + const preload = Buffer.from( + [ + 'import net from "node:net";', + 'net.Socket.prototype.connect = function () { throw new Error("AUTOQA_NETWORK_FORBIDDEN"); };', + 'globalThis.fetch = async () => { throw new Error("AUTOQA_NETWORK_FORBIDDEN"); };', + ...("reportFailure" in testCase + ? [ + 'import fs from "node:fs";', + "const originalReadDir = fs.readdirSync;", + `fs.readdirSync = (target, ...args) => { if (String(target) === ${JSON.stringify(workspaceHooksDir)}) { throw new Error("injected hook report loading failure"); } return originalReadDir(target, ...args); };`, + ] + : []), + ...("tty" in testCase + ? [ + 'Object.defineProperty(process.stdout, "isTTY", { value: true, configurable: true });', + 'Object.defineProperty(process.stderr, "isTTY", { value: true, configurable: true });', + ] + : []), + ].join("\n"), + ).toString("base64"); + const result = runBuiltCli(tempHome, testCase.args, { + NODE_OPTIONS: `--import=data:text/javascript;base64,${preload}`, + OPENCLAW_CONFIG_PATH: configPath, + OPENCLAW_DISABLE_BUNDLED_PLUGINS: "1", + OPENCLAW_GATEWAY_PORT: "29791", + OPENCLAW_STATE_DIR: stateDir, + ...("commander" in testCase ? { OPENCLAW_DISABLE_ROUTE_FIRST: "1" } : {}), + ...("tty" in testCase ? { FORCE_COLOR: "1" } : {}), + }); + const message = + testCase.message ?? + 'Unknown agent id "retired". Run openclaw agents list to see configured agents.'; + + expect(result.status, result.stderr).toBe(1); + expect(result.stdout, result.stderr).not.toMatch(/[\u001B\u0007]/u); + if ("human" in testCase) { + if ("missingHook" in testCase) { + expect(result.stdout.trim()).toBe(message); + } else { + expect(result.stdout).toBe(""); + expect(result.stderr).toContain(`Error: ${message}`); + } + } else { + expect(JSON.parse(result.stdout)).toEqual({ + ok: false, + error: { type: "cli_error", message }, + ...("missingHook" in testCase ? { hook: "missing-hook" } : {}), + }); + if (!("missingHook" in testCase)) { + expect(result.stderr).toContain(message); + } + } + expect(result.stderr).not.toContain("AUTOQA_NETWORK_FORBIDDEN"); + if ("tty" in testCase) { + expect(result.stderr).toContain("\u001B[?25h"); + } + await expect(fs.stat(configPath)).rejects.toMatchObject({ code: "ENOENT" }); + }, + { prefix: "openclaw-hooks-json-failure-e2e-" }, + ); + }); + + it("preserves successful hooks report JSON and offline discovery", async () => { + await withTempHome( + async (tempHome) => { + const result = runBuiltCli(tempHome, ["hooks", "--json", "list"], { + OPENCLAW_CONFIG_PATH: path.join(tempHome, "missing-openclaw.json"), + OPENCLAW_DISABLE_BUNDLED_PLUGINS: "1", + OPENCLAW_GATEWAY_PORT: "1", + OPENCLAW_STATE_DIR: path.join(tempHome, "isolated-state"), + }); + + expect(result.status, result.stderr).toBe(0); + expect(JSON.parse(result.stdout)).toEqual( + expect.objectContaining({ hooks: expect.any(Array) }), + ); + expect(result.stderr).toBe(""); + }, + { prefix: "openclaw-hooks-json-success-e2e-" }, + ); + }); + it.each([ { name: "add without an interactive terminal in human mode",