mirror of
https://github.com/openclaw/openclaw.git
synced 2026-08-23 19:08:22 -06:00
fix(cli): render task validation JSON failures (#127750)
This commit is contained in:
committed by
GitHub
parent
8d2024542e
commit
2e04f762ea
@@ -2,6 +2,7 @@
|
||||
import { Command } from "commander";
|
||||
import { createRequireRecord } from "openclaw/plugin-sdk/test-fixtures";
|
||||
import { beforeEach, describe, expect, it, vi } from "vitest";
|
||||
import { ExpectedCliError } from "../failure-output.js";
|
||||
import { registerTasksCommand } from "./register.tasks.js";
|
||||
|
||||
const mocks = vi.hoisted(() => ({
|
||||
@@ -82,10 +83,14 @@ describe("registerTasksCommand", () => {
|
||||
});
|
||||
|
||||
it("rejects inherited mutation options before loading task or flow owners", async () => {
|
||||
await runCli(["tasks", "--json", "cancel", "task-123"]);
|
||||
await expect(runCli(["tasks", "--json", "cancel", "task-123"])).rejects.toBeInstanceOf(
|
||||
ExpectedCliError,
|
||||
);
|
||||
expect(mocks.tasksModuleLoaded).not.toHaveBeenCalled();
|
||||
|
||||
await runCli(["tasks", "--json", "flow", "cancel", "flow-123"]);
|
||||
await expect(runCli(["tasks", "--json", "flow", "cancel", "flow-123"])).rejects.toBeInstanceOf(
|
||||
ExpectedCliError,
|
||||
);
|
||||
expect(mocks.flowsModuleLoaded).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
@@ -214,12 +219,14 @@ describe("registerTasksCommand", () => {
|
||||
});
|
||||
|
||||
it("rejects partially numeric task audit limits before owner action", async () => {
|
||||
await runCli(["tasks", "audit", "--limit", "5abc"]);
|
||||
const execution = runCli(["tasks", "audit", "--limit", "5abc"]);
|
||||
|
||||
expect(mocks.runtime.error).toHaveBeenCalledWith(
|
||||
await expect(execution).rejects.toBeInstanceOf(ExpectedCliError);
|
||||
await expect(execution).rejects.toThrow(
|
||||
"--limit must be a positive integer, for example --limit 25.",
|
||||
);
|
||||
expect(mocks.runtime.exit).toHaveBeenCalledWith(1);
|
||||
expect(mocks.runtime.error).not.toHaveBeenCalled();
|
||||
expect(mocks.runtime.exit).not.toHaveBeenCalled();
|
||||
expect(mocks.tasksAuditCommand).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
@@ -280,10 +287,12 @@ describe("registerTasksCommand", () => {
|
||||
flag: "--json",
|
||||
},
|
||||
])("rejects $label before owner action", async ({ args, flag }) => {
|
||||
await runCli(args);
|
||||
const execution = runCli(args);
|
||||
|
||||
expect(mocks.runtime.error).toHaveBeenCalledWith(expect.stringContaining(flag));
|
||||
expect(mocks.runtime.exit).toHaveBeenCalledWith(1);
|
||||
await expect(execution).rejects.toBeInstanceOf(ExpectedCliError);
|
||||
await expect(execution).rejects.toMatchObject({ message: expect.stringContaining(flag) });
|
||||
expect(mocks.runtime.error).not.toHaveBeenCalled();
|
||||
expect(mocks.runtime.exit).not.toHaveBeenCalled();
|
||||
for (const handler of ownerHandlers) {
|
||||
expect(handler).not.toHaveBeenCalled();
|
||||
}
|
||||
@@ -299,9 +308,10 @@ describe("registerTasksCommand", () => {
|
||||
error: "`tasks cancel` does not support inherited options --json, --runtime.",
|
||||
},
|
||||
])("lists only explicitly supplied unsupported flags", async ({ args, error }) => {
|
||||
await runCli(args);
|
||||
await expect(runCli(args)).rejects.toThrow(error);
|
||||
|
||||
expect(mocks.runtime.error).toHaveBeenCalledWith(error);
|
||||
expect(mocks.runtime.error).not.toHaveBeenCalled();
|
||||
expect(mocks.runtime.exit).not.toHaveBeenCalled();
|
||||
for (const handler of ownerHandlers) {
|
||||
expect(handler).not.toHaveBeenCalled();
|
||||
}
|
||||
@@ -320,11 +330,14 @@ describe("registerTasksCommand", () => {
|
||||
});
|
||||
|
||||
it("rejects an invalid notify policy before owner action", async () => {
|
||||
await runCli(["tasks", "notify", "run-123", "sometimes"]);
|
||||
const execution = runCli(["tasks", "notify", "run-123", "sometimes"]);
|
||||
|
||||
expect(mocks.runtime.error).toHaveBeenCalledWith(
|
||||
await expect(execution).rejects.toBeInstanceOf(ExpectedCliError);
|
||||
await expect(execution).rejects.toThrow(
|
||||
"Notify policy must be done_only, state_changes, or silent.",
|
||||
);
|
||||
expect(mocks.runtime.error).not.toHaveBeenCalled();
|
||||
expect(mocks.runtime.exit).not.toHaveBeenCalled();
|
||||
expect(mocks.tasksNotifyCommand).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
|
||||
@@ -15,6 +15,7 @@ import {
|
||||
import { runCommandWithRuntime } from "../cli-utils.js";
|
||||
import { inheritOptionFromParent } from "../command-options.js";
|
||||
import { parseCliEnumFilter } from "../enum-filter.js";
|
||||
import { ExpectedCliError } from "../failure-output.js";
|
||||
|
||||
type TasksParentOption = "json" | "runtime" | "status";
|
||||
const TASKS_PARENT_OPTIONS = ["json", "runtime", "status"] as const;
|
||||
@@ -56,21 +57,23 @@ function isTaskNotifyPolicy(value: unknown): value is TaskNotifyPolicy {
|
||||
return value === "done_only" || value === "state_changes" || value === "silent";
|
||||
}
|
||||
|
||||
function throwTasksCliError(message: string): never {
|
||||
throw new ExpectedCliError({ message, humanOutput: message, machineOutput: message });
|
||||
}
|
||||
|
||||
function resolveTasksLeafOptions(
|
||||
command: Command,
|
||||
leaf: TasksLeaf,
|
||||
): { json?: boolean; runtime?: string; status?: string } | undefined {
|
||||
): { json?: boolean; runtime?: string; status?: string } {
|
||||
const supported: readonly TasksParentOption[] = TASKS_LEAF_OPTION_SUPPORT[leaf];
|
||||
const flags = TASKS_PARENT_OPTIONS.filter(
|
||||
(name) =>
|
||||
!supported.includes(name) && inheritOptionFromParent(command, name, "cli") !== undefined,
|
||||
).map((name) => `--${name}`);
|
||||
if (flags.length > 0) {
|
||||
defaultRuntime.error(
|
||||
throwTasksCliError(
|
||||
`\`tasks ${leaf}\` does not support inherited ${flags.length === 1 ? "option" : "options"} ${flags.join(", ")}.`,
|
||||
);
|
||||
defaultRuntime.exit(1);
|
||||
return undefined;
|
||||
}
|
||||
|
||||
const resolveLocal = (name: TasksParentOption): unknown => {
|
||||
@@ -89,12 +92,10 @@ function resolveTasksLeafOptions(
|
||||
};
|
||||
}
|
||||
|
||||
function parseTasksAuditLimit(limit: unknown): number | null | undefined {
|
||||
function parseTasksAuditLimit(limit: unknown): number | undefined {
|
||||
const parsed = parseStrictPositiveInteger(limit);
|
||||
if (limit !== undefined && parsed === undefined) {
|
||||
defaultRuntime.error("--limit must be a positive integer, for example --limit 25.");
|
||||
defaultRuntime.exit(1);
|
||||
return null;
|
||||
throwTasksCliError("--limit must be a positive integer, for example --limit 25.");
|
||||
}
|
||||
return parsed;
|
||||
}
|
||||
@@ -119,9 +120,6 @@ export function registerTasksCommand(program: Command): void {
|
||||
addTasksListOptions(tasksCmd.command("list").description("List tracked background tasks")).action(
|
||||
async (_opts, command) => {
|
||||
const resolved = resolveTasksLeafOptions(command, "list");
|
||||
if (!resolved) {
|
||||
return;
|
||||
}
|
||||
await runOwner(loadTasksCommands, ({ tasksListCommand }) =>
|
||||
tasksListCommand(
|
||||
{
|
||||
@@ -144,13 +142,7 @@ export function registerTasksCommand(program: Command): void {
|
||||
.option("--limit <n>", "Limit displayed findings")
|
||||
.action(async (opts, command) => {
|
||||
const resolved = resolveTasksLeafOptions(command, "audit");
|
||||
if (!resolved) {
|
||||
return;
|
||||
}
|
||||
const limit = parseTasksAuditLimit(opts.limit);
|
||||
if (limit === null) {
|
||||
return;
|
||||
}
|
||||
await runOwner(loadTasksCommands, ({ tasksAuditCommand }) =>
|
||||
tasksAuditCommand(
|
||||
{
|
||||
@@ -171,9 +163,6 @@ export function registerTasksCommand(program: Command): void {
|
||||
.option("--apply", "Apply reconciliation, cleanup stamping, and pruning", false)
|
||||
.action(async (opts, command) => {
|
||||
const resolved = resolveTasksLeafOptions(command, "maintenance");
|
||||
if (!resolved) {
|
||||
return;
|
||||
}
|
||||
await runOwner(loadTasksCommands, ({ tasksMaintenanceCommand }) =>
|
||||
tasksMaintenanceCommand(
|
||||
{ json: Boolean(resolved.json), apply: Boolean(opts.apply) },
|
||||
@@ -189,9 +178,6 @@ export function registerTasksCommand(program: Command): void {
|
||||
.option("--json", "Output as JSON", false)
|
||||
.action(async (lookup, _opts, command) => {
|
||||
const resolved = resolveTasksLeafOptions(command, "show");
|
||||
if (!resolved) {
|
||||
return;
|
||||
}
|
||||
await runOwner(loadTasksCommands, ({ tasksShowCommand }) =>
|
||||
tasksShowCommand({ lookup, json: Boolean(resolved.json) }, defaultRuntime),
|
||||
);
|
||||
@@ -203,13 +189,9 @@ export function registerTasksCommand(program: Command): void {
|
||||
.argument("<lookup>", "Task id, run id, or session key")
|
||||
.argument("<notify>", "Notify policy (done_only, state_changes, silent)")
|
||||
.action(async (lookup, notify, _opts, command) => {
|
||||
if (!resolveTasksLeafOptions(command, "notify")) {
|
||||
return;
|
||||
}
|
||||
resolveTasksLeafOptions(command, "notify");
|
||||
if (!isTaskNotifyPolicy(notify)) {
|
||||
defaultRuntime.error("Notify policy must be done_only, state_changes, or silent.");
|
||||
defaultRuntime.exit(1);
|
||||
return;
|
||||
throwTasksCliError("Notify policy must be done_only, state_changes, or silent.");
|
||||
}
|
||||
await runOwner(loadTasksCommands, ({ tasksNotifyCommand }) =>
|
||||
tasksNotifyCommand({ lookup, notify }, defaultRuntime),
|
||||
@@ -221,9 +203,7 @@ export function registerTasksCommand(program: Command): void {
|
||||
.description("Cancel a running background task")
|
||||
.argument("<lookup>", "Task id, run id, or session key")
|
||||
.action(async (lookup, _opts, command) => {
|
||||
if (!resolveTasksLeafOptions(command, "cancel")) {
|
||||
return;
|
||||
}
|
||||
resolveTasksLeafOptions(command, "cancel");
|
||||
await runOwner(loadTasksCommands, ({ tasksCancelCommand }) =>
|
||||
tasksCancelCommand({ lookup }, defaultRuntime),
|
||||
);
|
||||
@@ -233,9 +213,7 @@ export function registerTasksCommand(program: Command): void {
|
||||
.command("retry <lookups...>")
|
||||
.description("Retry delivery for up to 10 blocked subagent completions")
|
||||
.action(async (lookups: string[], _opts, command) => {
|
||||
if (!resolveTasksLeafOptions(command, "retry")) {
|
||||
return;
|
||||
}
|
||||
resolveTasksLeafOptions(command, "retry");
|
||||
await runOwner(loadTasksCommands, ({ tasksRetryCommand }) =>
|
||||
tasksRetryCommand({ lookups }, defaultRuntime),
|
||||
);
|
||||
@@ -245,9 +223,7 @@ export function registerTasksCommand(program: Command): void {
|
||||
.command("dismiss <lookups...>")
|
||||
.description("Dismiss delivery for up to 10 blocked subagent completions")
|
||||
.action(async (lookups: string[], _opts, command) => {
|
||||
if (!resolveTasksLeafOptions(command, "dismiss")) {
|
||||
return;
|
||||
}
|
||||
resolveTasksLeafOptions(command, "dismiss");
|
||||
await runOwner(loadTasksCommands, ({ tasksDismissCommand }) =>
|
||||
tasksDismissCommand({ lookups }, defaultRuntime),
|
||||
);
|
||||
@@ -266,9 +242,6 @@ export function registerTasksCommand(program: Command): void {
|
||||
.option("--status <name>", `Filter by status (${TASK_FLOW_STATUSES.join(", ")})`)
|
||||
.action(async (_opts, command) => {
|
||||
const resolved = resolveTasksLeafOptions(command, "flow list");
|
||||
if (!resolved) {
|
||||
return;
|
||||
}
|
||||
await runOwner(loadFlowsCommands, ({ flowsListCommand }) =>
|
||||
flowsListCommand({ json: Boolean(resolved.json), status: resolved.status }, defaultRuntime),
|
||||
);
|
||||
@@ -281,9 +254,6 @@ export function registerTasksCommand(program: Command): void {
|
||||
.option("--json", "Output as JSON", false)
|
||||
.action(async (lookup, _opts, command) => {
|
||||
const resolved = resolveTasksLeafOptions(command, "flow show");
|
||||
if (!resolved) {
|
||||
return;
|
||||
}
|
||||
await runOwner(loadFlowsCommands, ({ flowsShowCommand }) =>
|
||||
flowsShowCommand({ lookup, json: Boolean(resolved.json) }, defaultRuntime),
|
||||
);
|
||||
@@ -294,9 +264,7 @@ export function registerTasksCommand(program: Command): void {
|
||||
.description("Cancel a running TaskFlow")
|
||||
.argument("<lookup>", "Flow id or owner key")
|
||||
.action(async (lookup, _opts, command) => {
|
||||
if (!resolveTasksLeafOptions(command, "flow cancel")) {
|
||||
return;
|
||||
}
|
||||
resolveTasksLeafOptions(command, "flow cancel");
|
||||
await runOwner(loadFlowsCommands, ({ flowsCancelCommand }) =>
|
||||
flowsCancelCommand({ lookup }, defaultRuntime),
|
||||
);
|
||||
|
||||
@@ -371,6 +371,107 @@ describe("cli json stdout contract", () => {
|
||||
);
|
||||
});
|
||||
|
||||
it.each([
|
||||
{
|
||||
name: "audit limit in human mode",
|
||||
args: ["tasks", "audit", "--limit", "5abc"],
|
||||
message: "--limit must be a positive integer, for example --limit 25.",
|
||||
human: true,
|
||||
},
|
||||
{
|
||||
name: "notify policy in human mode",
|
||||
args: ["tasks", "notify", "task-123", "sometimes"],
|
||||
message: "Notify policy must be done_only, state_changes, or silent.",
|
||||
human: true,
|
||||
},
|
||||
{
|
||||
name: "routed audit limit with leaf JSON",
|
||||
args: ["tasks", "audit", "--json", "--limit", "5abc"],
|
||||
message: "--limit must be a positive integer, for example --limit 25.",
|
||||
},
|
||||
{
|
||||
name: "routed audit limit with parent JSON",
|
||||
args: ["tasks", "--json", "audit", "--limit", "5abc"],
|
||||
message: "--limit must be a positive integer, for example --limit 25.",
|
||||
},
|
||||
{
|
||||
name: "Commander audit limit with leaf JSON",
|
||||
args: ["tasks", "audit", "--limit", "5abc", "--json"],
|
||||
message: "--limit must be a positive integer, for example --limit 25.",
|
||||
commander: true,
|
||||
},
|
||||
{
|
||||
name: "Commander audit limit with parent JSON",
|
||||
args: ["tasks", "--json", "audit", "--limit", "5abc"],
|
||||
message: "--limit must be a positive integer, for example --limit 25.",
|
||||
commander: true,
|
||||
},
|
||||
{
|
||||
name: "routed audit with an inherited runtime",
|
||||
args: ["tasks", "--json", "--runtime", "cli", "audit"],
|
||||
message: "`tasks audit` does not support inherited option --runtime.",
|
||||
},
|
||||
{
|
||||
name: "Commander audit with an inherited status",
|
||||
args: ["tasks", "--json", "--status", "running", "audit"],
|
||||
message: "`tasks audit` does not support inherited option --status.",
|
||||
commander: true,
|
||||
},
|
||||
{
|
||||
name: "routed maintenance with an inherited runtime",
|
||||
args: ["tasks", "--runtime", "cli", "maintenance", "--json"],
|
||||
message: "`tasks maintenance` does not support inherited option --runtime.",
|
||||
},
|
||||
{
|
||||
name: "routed TaskFlow list with an inherited task status",
|
||||
args: ["tasks", "--json", "--status", "running", "flow", "list"],
|
||||
message: "`tasks flow list` does not support inherited option --status.",
|
||||
},
|
||||
{
|
||||
name: "Commander TaskFlow show with an inherited runtime",
|
||||
args: ["tasks", "--runtime", "cli", "flow", "--json", "show", "flow-123"],
|
||||
message: "`tasks flow show` does not support inherited option --runtime.",
|
||||
commander: true,
|
||||
},
|
||||
{
|
||||
name: "routed audit limit through dual-TTY finalization",
|
||||
args: ["tasks", "audit", "--json", "--limit", "5abc"],
|
||||
message: "--limit must be a positive integer, for example --limit 25.",
|
||||
tty: true,
|
||||
},
|
||||
])("renders task registration validation failures for $name", async (testCase) => {
|
||||
await withTempHome(
|
||||
async (tempHome) => {
|
||||
const preload = `data:text/javascript,${encodeURIComponent(
|
||||
'Object.defineProperty(process.stdout, "isTTY", { value: true, configurable: true }); Object.defineProperty(process.stderr, "isTTY", { value: true, configurable: true });',
|
||||
)}`;
|
||||
const result = runBuiltCli(tempHome, testCase.args, {
|
||||
OPENCLAW_STATE_DIR: path.join(tempHome, "isolated-state"),
|
||||
OPENCLAW_CONFIG_PATH: path.join(tempHome, "missing-openclaw.json"),
|
||||
...("commander" in testCase ? { OPENCLAW_DISABLE_ROUTE_FIRST: "1" } : {}),
|
||||
...("tty" in testCase ? { NODE_OPTIONS: `--import=${preload}`, FORCE_COLOR: "1" } : {}),
|
||||
});
|
||||
|
||||
expect(result.status, result.stderr).toBe(1);
|
||||
expect(result.stdout, result.stderr).not.toMatch(/[\u001B\u0007]/u);
|
||||
if ("human" in testCase) {
|
||||
expect(result.stdout).toBe("");
|
||||
} else {
|
||||
expect(JSON.parse(result.stdout)).toEqual({
|
||||
ok: false,
|
||||
error: { type: "cli_error", message: testCase.message },
|
||||
});
|
||||
}
|
||||
expect(result.stderr).toContain(testCase.message);
|
||||
expect(result.stderr.split(testCase.message)).toHaveLength(2);
|
||||
if ("tty" in testCase) {
|
||||
expect(result.stderr).toContain("\u001B[?25h");
|
||||
}
|
||||
},
|
||||
{ prefix: "openclaw-task-registration-json-failure-e2e-" },
|
||||
);
|
||||
});
|
||||
|
||||
it.each([
|
||||
{ name: "qr", command: ["qr"] },
|
||||
{ name: "clawbot qr", command: ["clawbot", "qr"] },
|
||||
|
||||
Reference in New Issue
Block a user