fix(cli): render hooks report JSON failures (#128641)

This commit is contained in:
Peter Steinberger
2026-08-24 02:21:41 -07:00
committed by GitHub
parent 7a7bb1f670
commit 5e253f4f42
6 changed files with 356 additions and 42 deletions
+1
View File
@@ -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
+3 -1
View File
@@ -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.`;
}
+8
View File
@@ -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");
+147 -19
View File
@@ -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();
});
+20 -22
View File
@@ -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<T>(action: () => Promise<T> | T): Promise<T> {
try {
return await action();
} catch (err) {
return exitHooksCliWithError(err);
}
}
async function runOneShotHooksCliAction(action: () => Promise<number | void>): Promise<void> {
const result = await runHooksCliAction(action);
async function runOneShotHooksCliAction(
action: () => Promise<number | void>,
failureOwner: "command" | "root" = "command",
): Promise<void> {
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"),
);
}
+177
View File
@@ -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",