From eef9aaa2ead4587c6ad70155b0fb5b3be00ac43e Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Sun, 23 Aug 2026 21:16:19 -0700 Subject: [PATCH] fix(cli): render agent binding JSON failures (#128484) --- src/commands/agents.bind.commands.test.ts | 157 ++++++++++++++++- src/commands/agents.commands.bind.ts | 101 ++++------- test/cli-json-stdout.e2e.test.ts | 203 ++++++++++++++++++++++ 3 files changed, 382 insertions(+), 79 deletions(-) diff --git a/src/commands/agents.bind.commands.test.ts b/src/commands/agents.bind.commands.test.ts index 49ad506cba26..a377e1fa6684 100644 --- a/src/commands/agents.bind.commands.test.ts +++ b/src/commands/agents.bind.commands.test.ts @@ -203,21 +203,130 @@ describe("agents bind/unbind commands", () => { expect(runtime.exit).not.toHaveBeenCalled(); }); - it.each(["агент✨", " "])( - "rejects an explicit unrepresentable agent %j instead of binding the default", - async (agent) => { + it.each([ + { + name: "bindings with an unrepresentable agent", + command: "bindings", + options: { agent: "агент✨", json: true }, + message: 'Agent "агент✨" not found. Run openclaw agents list to see configured agents.', + }, + { + name: "bindings with an unknown agent", + command: "bindings", + options: { agent: "ghost", json: true }, + message: 'Agent "ghost" not found. Run openclaw agents list to see configured agents.', + }, + { + name: "bind with an unrepresentable agent", + command: "bind", + options: { agent: "агент✨", bind: ["telegram"], json: true }, + message: 'Agent "агент✨" not found. Run openclaw agents list to see configured agents.', + }, + { + name: "bind with a blank agent", + command: "bind", + options: { agent: " ", bind: ["telegram"], json: true }, + message: 'Agent " " not found. Run openclaw agents list to see configured agents.', + }, + { + name: "bind with an unknown agent before missing bindings", + command: "bind", + options: { agent: "ghost", json: true }, + message: 'Agent "ghost" not found. Run openclaw agents list to see configured agents.', + }, + { + name: "bind without bindings", + command: "bind", + options: { json: true }, + message: "Provide at least one --bind .", + }, + { + name: "bind with only blank bindings", + command: "bind", + options: { bind: [" "], json: true }, + message: "Provide at least one --bind .", + }, + { + name: "bind with multiple malformed bindings in input order", + command: "bind", + options: { bind: ["telegram:", "telegram:work:extra"], json: true }, + message: [ + 'Invalid binding "telegram:". Account id is empty. Use :, for example telegram:default.', + 'Invalid binding "telegram:work:extra". Account id cannot contain ":". Use :, for example telegram:default.', + ].join("\n"), + }, + { + name: "bind with an unknown channel", + command: "bind", + options: { bind: ["definitely-not-a-channel"], json: true }, + message: + 'Unknown channel "definitely-not-a-channel". Run `openclaw channels list --all` to see configured and installable channels.', + loadsPluginRegistry: true, + }, + { + name: "unbind with an unrepresentable agent", + command: "unbind", + options: { agent: "агент✨", all: true, json: true }, + message: 'Agent "агент✨" not found. Run openclaw agents list to see configured agents.', + }, + { + name: "unbind with an unknown agent before incompatible options", + command: "unbind", + options: { agent: "ghost", all: true, bind: ["telegram"], json: true }, + message: 'Agent "ghost" not found. Run openclaw agents list to see configured agents.', + }, + { + name: "unbind without bindings", + command: "unbind", + options: { json: true }, + message: "Provide at least one --bind or use --all.", + }, + { + name: "unbind with only blank bindings", + command: "unbind", + options: { bind: [" "], json: true }, + message: "Provide at least one --bind or use --all.", + }, + { + name: "unbind with incompatible all and binding options", + command: "unbind", + options: { all: true, bind: ["telegram"], json: true }, + message: "Use either --all or --bind, not both.", + }, + { + name: "unbind with a malformed binding", + command: "unbind", + options: { bind: ["telegram:work:extra"], json: true }, + message: + 'Invalid binding "telegram:work:extra". Account id cannot contain ":". Use :, for example telegram:default.', + }, + ])( + "rejects $name through the root failure owner before mutation", + async ({ command, options, message, loadsPluginRegistry }) => { readConfigFileSnapshotMock.mockResolvedValue({ ...baseConfigSnapshot, config: {}, }); - await agentsBindCommand({ agent, bind: ["telegram"] }, runtime); + const execution = + command === "bindings" + ? agentsBindingsCommand(options, runtime) + : command === "bind" + ? agentsBindCommand(options, runtime) + : agentsUnbindCommand(options, runtime); - expect(runtime.error).toHaveBeenCalledWith( - `Agent "${agent}" not found. Run openclaw agents list to see configured agents.`, - ); - expect(runtime.exit).toHaveBeenCalledWith(1); + await expect(execution).rejects.toMatchObject({ + name: "ExpectedCliError", + message, + humanOutput: message, + machineOutput: message, + }); + expect(runtime.error).not.toHaveBeenCalled(); + expect(runtime.exit).not.toHaveBeenCalled(); expect(writeConfigFileMock).not.toHaveBeenCalled(); + if (!loadsPluginRegistry) { + expect(pluginRegistryMocks.loadPluginRegistrySnapshot).not.toHaveBeenCalled(); + } }, ); @@ -329,4 +438,36 @@ describe("agents bind/unbind commands", () => { expect(runtime.error).toHaveBeenCalledWith("Bindings are owned by another agent:"); expect(runtime.exit).toHaveBeenCalledWith(1); }); + + it.each(["bind", "unbind"])( + "preserves the post-decision %s conflict JSON result and exit status", + async (command) => { + readConfigFileSnapshotMock.mockResolvedValue({ + ...baseConfigSnapshot, + config: { + agents: { list: [{ id: "ops", workspace: "/tmp/ops" }] }, + bindings: [{ agentId: "main", match: { channel: "telegram", accountId: "ops" } }], + }, + }); + const jsonRuntime = createJsonTestRuntime(); + const options = { agent: "ops", bind: ["telegram:ops"], json: true }; + + if (command === "bind") { + await agentsBindCommand(options, jsonRuntime); + } else { + await agentsUnbindCommand(options, jsonRuntime); + } + + expect(writeConfigFileMock).not.toHaveBeenCalled(); + expect(jsonRuntime.writeJson.mock.calls[0]?.[0]).toStrictEqual({ + agentId: "ops", + ...(command === "bind" + ? { added: [], updated: [], skipped: [] } + : { removed: [], missing: [] }), + conflicts: ["telegram accountId=ops (agent=main)"], + }); + expect(jsonRuntime.error).not.toHaveBeenCalled(); + expect(jsonRuntime.exit).toHaveBeenCalledWith(1); + }, + ); }); diff --git a/src/commands/agents.commands.bind.ts b/src/commands/agents.commands.bind.ts index 04ee8e6acf10..34415cbdb617 100644 --- a/src/commands/agents.commands.bind.ts +++ b/src/commands/agents.commands.bind.ts @@ -2,6 +2,7 @@ import { normalizeStringEntries } from "@openclaw/normalization-core/string-normalization"; import { listAgentEntries, resolveDefaultAgentId } from "../agents/agent-scope.js"; import { formatCliCommand } from "../cli/command-format.js"; +import { ExpectedCliError } from "../cli/failure-output.js"; import { isRouteBinding, listRouteBindings } from "../config/bindings.js"; import { replaceConfigFile } from "../config/config.js"; import { logConfigUpdated } from "../config/logging.js"; @@ -13,6 +14,7 @@ import { describeBinding } from "./agents.binding-format.js"; import { requireValidConfig, requireValidConfigFileSnapshot } from "./config-validation.js"; type AgentBindingsModule = typeof import("./agents.bindings.js"); +type AgentConfig = NonNullable>>; type AgentsBindingsListOptions = { agent?: string; @@ -40,10 +42,7 @@ function loadAgentBindingsModule(): Promise { return agentBindingsModuleLoader.load(); } -function hasAgent(cfg: Awaited>, agentId: string): boolean { - if (!cfg) { - return false; - } +function hasAgent(cfg: AgentConfig, agentId: string): boolean { const targetAgentId = normalizeAgentId(agentId); const agents = listAgentEntries(cfg); if (agents.length === 0) { @@ -56,27 +55,26 @@ function formatBindingOwnerLine(binding: AgentRouteBinding): string { return `${normalizeAgentId(binding.agentId)} <- ${describeBinding(binding)}`; } -function resolveTargetAgentIdOrExit(params: { - cfg: NonNullable>>; - runtime: RuntimeEnv; +function failAgentBinding(message: string): never { + throw new ExpectedCliError({ message, humanOutput: message, machineOutput: message }); +} + +function resolveTargetAgentId(params: { + cfg: AgentConfig; agentInput: string | undefined; -}): string | null { +}): string { const normalized = params.agentInput === undefined ? null : normalizeAgentIdStrict(params.agentInput); if (normalized && !normalized.ok) { - params.runtime.error( + failAgentBinding( `Agent "${params.agentInput}" not found. Run ${formatCliCommand("openclaw agents list")} to see configured agents.`, ); - params.runtime.exit(1); - return null; } const agentId = normalized?.value ?? resolveDefaultAgentId(params.cfg); if (!hasAgent(params.cfg, agentId)) { - params.runtime.error( + failAgentBinding( `Agent "${agentId}" not found. Run ${formatCliCommand("openclaw agents list")} to see configured agents.`, ); - params.runtime.exit(1); - return null; } return agentId; } @@ -89,31 +87,23 @@ function formatBindingConflicts( ); } -async function resolveParsedBindingsOrExit(params: { - runtime: RuntimeEnv; - cfg: NonNullable>>; +async function resolveParsedBindings(params: { + cfg: AgentConfig; agentId: string; bindValues: string[] | undefined; emptyMessage: string; -}): Promise<{ - bindings: AgentRouteBinding[]; - errors: string[]; -} | null> { +}): Promise { const specs = normalizeStringEntries(params.bindValues); if (specs.length === 0) { - params.runtime.error(params.emptyMessage); - params.runtime.exit(1); - return null; + failAgentBinding(params.emptyMessage); } const { parseBindingSpecs } = await loadAgentBindingsModule(); const parsed = parseBindingSpecs({ agentId: params.agentId, specs, config: params.cfg }); if (parsed.errors.length > 0) { - params.runtime.error(parsed.errors.join("\n")); - params.runtime.exit(1); - return null; + failAgentBinding(parsed.errors.join("\n")); } - return parsed; + return parsed.bindings; } function emitJsonPayload(params: { @@ -132,11 +122,11 @@ function emitJsonPayload(params: { return true; } -async function resolveConfigAndTargetAgentIdOrExit(params: { +async function resolveConfigAndTargetAgentId(params: { runtime: RuntimeEnv; agentInput: string | undefined; }): Promise<{ - cfg: NonNullable>>; + cfg: AgentConfig; agentId: string; baseHash?: string; } | null> { @@ -145,14 +135,7 @@ async function resolveConfigAndTargetAgentIdOrExit(params: { return null; } const cfg = configSnapshot.sourceConfig ?? configSnapshot.config; - const agentId = resolveTargetAgentIdOrExit({ - cfg, - runtime: params.runtime, - agentInput: params.agentInput, - }); - if (!agentId) { - return null; - } + const agentId = resolveTargetAgentId({ cfg, agentInput: params.agentInput }); return { cfg, agentId, baseHash: configSnapshot.hash }; } @@ -166,22 +149,8 @@ export async function agentsBindingsCommand( return; } - const normalizedFilter = opts.agent === undefined ? null : normalizeAgentIdStrict(opts.agent); - if (normalizedFilter && !normalizedFilter.ok) { - runtime.error( - `Agent "${opts.agent}" not found. Run ${formatCliCommand("openclaw agents list")} to see configured agents.`, - ); - runtime.exit(1); - return; - } - const filterAgentId = normalizedFilter?.value; - if (filterAgentId && !hasAgent(cfg, filterAgentId)) { - runtime.error( - `Agent "${filterAgentId}" not found. Run ${formatCliCommand("openclaw agents list")} to see configured agents.`, - ); - runtime.exit(1); - return; - } + const filterAgentId = + opts.agent === undefined ? undefined : resolveTargetAgentId({ cfg, agentInput: opts.agent }); const filtered = listRouteBindings(cfg).filter( (binding) => !filterAgentId || normalizeAgentId(binding.agentId) === filterAgentId, @@ -218,7 +187,7 @@ export async function agentsBindCommand( opts: AgentsBindOptions, runtime: RuntimeEnv = defaultRuntime, ) { - const resolved = await resolveConfigAndTargetAgentIdOrExit({ + const resolved = await resolveConfigAndTargetAgentId({ runtime, agentInput: opts.agent, }); @@ -227,19 +196,15 @@ export async function agentsBindCommand( } const { cfg, agentId, baseHash } = resolved; - const parsed = await resolveParsedBindingsOrExit({ - runtime, + const bindings = await resolveParsedBindings({ cfg, agentId, bindValues: opts.bind, emptyMessage: "Provide at least one --bind .", }); - if (!parsed) { - return; - } const { applyAgentBindings } = await loadAgentBindingsModule(); - const result = applyAgentBindings(cfg, parsed.bindings); + const result = applyAgentBindings(cfg, bindings); if (result.added.length > 0 || result.updated.length > 0) { await replaceConfigFile({ nextConfig: result.config, @@ -300,7 +265,7 @@ export async function agentsUnbindCommand( opts: AgentsUnbindOptions, runtime: RuntimeEnv = defaultRuntime, ) { - const resolved = await resolveConfigAndTargetAgentIdOrExit({ + const resolved = await resolveConfigAndTargetAgentId({ runtime, agentInput: opts.agent, }); @@ -309,9 +274,7 @@ export async function agentsUnbindCommand( } const { cfg, agentId, baseHash } = resolved; if (opts.all && (opts.bind?.length ?? 0) > 0) { - runtime.error("Use either --all or --bind, not both."); - runtime.exit(1); - return; + failAgentBinding("Use either --all or --bind, not both."); } if (opts.all) { @@ -362,19 +325,15 @@ export async function agentsUnbindCommand( return; } - const parsed = await resolveParsedBindingsOrExit({ - runtime, + const bindings = await resolveParsedBindings({ cfg, agentId, bindValues: opts.bind, emptyMessage: "Provide at least one --bind or use --all.", }); - if (!parsed) { - return; - } const { removeAgentBindings } = await loadAgentBindingsModule(); - const result = removeAgentBindings(cfg, parsed.bindings); + const result = removeAgentBindings(cfg, bindings); if (result.removed.length > 0) { await replaceConfigFile({ nextConfig: result.config, diff --git a/test/cli-json-stdout.e2e.test.ts b/test/cli-json-stdout.e2e.test.ts index a7895c250a59..9389c1f2e5cc 100644 --- a/test/cli-json-stdout.e2e.test.ts +++ b/test/cli-json-stdout.e2e.test.ts @@ -52,6 +52,209 @@ async function seedTrajectorySession(tempHome: string, sessionKey: string) { } describe("cli json stdout contract", () => { + it.each([ + { + name: "bindings with an invalid agent", + args: ["agents", "bindings", "--agent", "агент✨", "--json"], + message: 'Agent "агент✨" not found. Run openclaw agents list to see configured agents.', + }, + { + name: "bindings with an unknown agent", + args: ["agents", "bindings", "--json", "--agent", "ghost"], + message: 'Agent "ghost" not found. Run openclaw agents list to see configured agents.', + }, + { + name: "bind with an invalid agent", + args: ["agents", "bind", "--agent", "агент✨", "--bind", "telegram", "--json"], + message: 'Agent "агент✨" not found. Run openclaw agents list to see configured agents.', + }, + { + name: "bind with an unknown agent before missing bindings", + args: ["agents", "bind", "--json", "--agent", "ghost"], + message: 'Agent "ghost" not found. Run openclaw agents list to see configured agents.', + }, + { + name: "bind without bindings", + args: ["agents", "bind", "--json"], + message: "Provide at least one --bind .", + }, + { + name: "bind with only a blank binding", + args: ["agents", "bind", "--bind", " ", "--json"], + message: "Provide at least one --bind .", + }, + { + name: "bind with multiple malformed bindings in input order", + args: ["agents", "bind", "--bind", "telegram:", "--bind", "telegram:work:extra", "--json"], + message: [ + 'Invalid binding "telegram:". Account id is empty. Use :, for example telegram:default.', + 'Invalid binding "telegram:work:extra". Account id cannot contain ":". Use :, for example telegram:default.', + ].join("\n"), + }, + { + name: "bind with an unknown channel", + args: ["agents", "bind", "--json", "--bind", "definitely-not-a-channel"], + message: + 'Unknown channel "definitely-not-a-channel". Run `openclaw channels list --all` to see configured and installable channels.', + }, + { + name: "unbind with an invalid agent", + args: ["agents", "unbind", "--agent", "агент✨", "--all", "--json"], + message: 'Agent "агент✨" not found. Run openclaw agents list to see configured agents.', + }, + { + name: "unbind with an unknown agent before incompatible options", + args: ["agents", "unbind", "--agent", "ghost", "--all", "--bind", "telegram", "--json"], + message: 'Agent "ghost" not found. Run openclaw agents list to see configured agents.', + }, + { + name: "unbind without bindings", + args: ["agents", "unbind", "--json"], + message: "Provide at least one --bind or use --all.", + }, + { + name: "unbind with a malformed binding", + args: ["agents", "unbind", "--bind", "telegram:work:extra", "--json"], + message: + 'Invalid binding "telegram:work:extra". Account id cannot contain ":". Use :, for example telegram:default.', + }, + { + name: "unbind with incompatible options in human mode", + args: ["agents", "unbind", "--all", "--bind", "telegram"], + message: "Use either --all or --bind, not both.", + human: true, + }, + { + name: "unbind with incompatible options in JSON mode", + args: ["agents", "unbind", "--all", "--bind", "telegram", "--json"], + message: "Use either --all or --bind, not both.", + }, + { + name: "bind without bindings through dual-TTY finalization", + args: ["agents", "bind", "--json"], + message: "Provide at least one --bind .", + tty: true, + }, + ])("renders agent binding $name through the canonical failure owner", async (testCase) => { + await withTempHome( + async (tempHome) => { + const configPath = path.join(tempHome, "missing-openclaw.json"); + 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: configPath, + ...("tty" in testCase ? { NODE_OPTIONS: `--import=${preload}`, FORCE_COLOR: "1" } : {}), + }); + + expect(result.status, result.stderr).toBe(1); + if ("human" in testCase) { + expect(result.stdout).toBe(""); + } else { + expect(result.stdout, result.stderr).not.toMatch(/[\u001B\u0007]/u); + 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"); + } + await expect(fs.access(configPath)).rejects.toMatchObject({ code: "ENOENT" }); + }, + { prefix: "openclaw-agent-bindings-json-failure-e2e-" }, + ); + }); + + it.each([ + { + name: "bindings list success", + args: ["agents", "bindings", "--json"], + payload: [], + }, + { + name: "bind success", + args: ["agents", "bind", "--bind", "telegram:work", "--json"], + payload: { + agentId: "main", + added: ["telegram accountId=work"], + updated: [], + skipped: [], + conflicts: [], + }, + writesConfig: true, + }, + { + name: "unbind-all success", + args: ["agents", "unbind", "--all", "--json"], + payload: { agentId: "main", removed: [], missing: [], conflicts: [] }, + }, + { + name: "bind ownership conflict", + args: ["agents", "bind", "--agent", "main", "--bind", "telegram:work", "--json"], + payload: { + agentId: "main", + added: [], + updated: [], + skipped: [], + conflicts: ["telegram accountId=work (agent=ops)"], + }, + conflict: true, + }, + { + name: "unbind ownership conflict", + args: ["agents", "unbind", "--agent", "main", "--bind", "telegram:work", "--json"], + payload: { + agentId: "main", + removed: [], + missing: [], + conflicts: ["telegram accountId=work (agent=ops)"], + }, + conflict: true, + }, + ])("preserves agent binding $name as its existing domain payload", async (testCase) => { + await withTempHome( + async (tempHome) => { + const configPath = path.join(tempHome, "openclaw.json"); + const existingConfig = `${JSON.stringify({ + agents: { + ownership: "explicit", + list: [ + { id: "main", workspace: path.join(tempHome, "main") }, + { id: "ops", workspace: path.join(tempHome, "ops") }, + ], + }, + bindings: [ + { type: "route", agentId: "ops", match: { channel: "telegram", accountId: "work" } }, + ], + })}\n`; + if ("conflict" in testCase) { + await fs.writeFile(configPath, existingConfig, "utf8"); + } + + const result = runBuiltCli(tempHome, testCase.args, { + OPENCLAW_STATE_DIR: path.join(tempHome, "isolated-state"), + OPENCLAW_CONFIG_PATH: configPath, + }); + + expect(result.status, result.stderr).toBe("conflict" in testCase ? 1 : 0); + expect(result.stdout, result.stderr).not.toBe(""); + expect(JSON.parse(result.stdout)).toEqual(testCase.payload); + if ("writesConfig" in testCase) { + await expect(fs.access(configPath)).resolves.toBeUndefined(); + } else if ("conflict" in testCase) { + await expect(fs.readFile(configPath, "utf8")).resolves.toBe(existingConfig); + } else { + await expect(fs.access(configPath)).rejects.toMatchObject({ code: "ENOENT" }); + } + }, + { prefix: "openclaw-agent-bindings-domain-payload-e2e-" }, + ); + }); + it.each([ { name: "routed config get",