From f40cb158b1d89573d4bd1c2edc42e0702aaa1834 Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Sun, 23 Aug 2026 23:22:54 -0700 Subject: [PATCH] fix(onboarding): preserve JSON option rejection output (#128542) --- config/assertion-safety-baseline.txt | 2 +- src/cli/program/register.onboard.test.ts | 33 +++++-- src/cli/program/register.onboard.ts | 27 +++--- src/cli/program/register.setup.test.ts | 49 ++++++++-- src/cli/program/register.setup.ts | 10 +- .../local.default-agent.test.ts | 91 ++++++++++++------- src/commands/onboard-non-interactive/local.ts | 15 ++- 7 files changed, 144 insertions(+), 83 deletions(-) diff --git a/config/assertion-safety-baseline.txt b/config/assertion-safety-baseline.txt index 64974e08e493..f0d1096fce83 100644 --- a/config/assertion-safety-baseline.txt +++ b/config/assertion-safety-baseline.txt @@ -2471,7 +2471,7 @@ src/cli/program/register.audit.ts 12 src/cli/program/register.backup.ts 14 src/cli/program/register.configure.ts 1 src/cli/program/register.migrate.ts 12 -src/cli/program/register.onboard.ts 28 +src/cli/program/register.onboard.ts 27 src/cli/program/register.setup.ts 2 src/cli/program/register.status-health-sessions.ts 29 src/cli/program/route-specs.ts 1 diff --git a/src/cli/program/register.onboard.test.ts b/src/cli/program/register.onboard.test.ts index 251618f9dfbf..39cb86996bc6 100644 --- a/src/cli/program/register.onboard.test.ts +++ b/src/cli/program/register.onboard.test.ts @@ -62,7 +62,8 @@ vi.mock("../../commands/system-agent-with-inference.js", () => ({ runSystemAgentWithInference: mocks.runSystemAgentWithInference, })); -vi.mock("../../runtime.js", () => ({ +vi.mock("../../runtime.js", async (importOriginal) => ({ + ...(await importOriginal()), defaultRuntime: mocks.runtime, })); @@ -251,15 +252,29 @@ describe("registerOnboardCommand", () => { expect(setupWizardOptions().skipUi).toBe(true); }); - it("rejects conflicting custom model input capabilities", async () => { - await runCli(["onboard", "--custom-image-input", "--custom-text-input"]); + it.each([false, true])( + "rejects conflicting custom model input capabilities (json: %s)", + async (json) => { + await runCli([ + "onboard", + "--custom-image-input", + "--custom-text-input", + ...(json ? ["--json"] : []), + ]); - expect(runtime.error).toHaveBeenCalledWith( - "Use either --custom-image-input or --custom-text-input, not both.", - ); - expect(runtime.exit).toHaveBeenCalledWith(1); - expect(setupWizardCommandMock).not.toHaveBeenCalled(); - }); + const message = "Use either --custom-image-input or --custom-text-input, not both."; + expect(runtime.error).toHaveBeenCalledWith(message); + expect(runtime.exit).toHaveBeenCalledWith(1); + if (json) { + expect(runtime.log).toHaveBeenCalledWith( + JSON.stringify({ ok: false, phase: "options", message }, null, 2), + ); + } else { + expect(runtime.log).not.toHaveBeenCalled(); + } + expect(setupWizardCommandMock).not.toHaveBeenCalled(); + }, + ); it("parses --mistral-api-key and forwards mistralApiKey", async () => { await runCli(["onboard", "--mistral-api-key", "sk-mistral-test"]); diff --git a/src/cli/program/register.onboard.ts b/src/cli/program/register.onboard.ts index 785126d729b8..a25785182625 100644 --- a/src/cli/program/register.onboard.ts +++ b/src/cli/program/register.onboard.ts @@ -6,6 +6,7 @@ import { theme } from "../../../packages/terminal-core/src/theme.js"; import { formatAuthChoiceChoicesForCli } from "../../commands/auth-choice-options.js"; import type { GatewayDaemonRuntime } from "../../commands/daemon-runtime.js"; import { CORE_ONBOARD_AUTH_FLAGS } from "../../commands/onboard-core-auth-flags.js"; +import { rejectOnboardingOption } from "../../commands/onboard-options.js"; import type { AuthChoice, GatewayAuthChoice, @@ -229,7 +230,12 @@ function pickOnboardAuthOptionValues(opts: Record): Partial, command: Command, -): OnboardOptions { + runtime: RuntimeEnv, +): OnboardOptions | false { + if (opts.customImageInput === true && opts.customTextInput === true) { + const message = "Use either --custom-image-input or --custom-text-input, not both."; + return rejectOnboardingOption({ json: opts.json === true }, runtime, message); + } return { workspace: readStringValue(opts.workspace), agentName: readStringValue(opts.agentName), @@ -270,18 +276,6 @@ export function resolveOnboardCommandOptions( }; } -export function validateOnboardAuthOptionValues( - opts: Record, - runtime: RuntimeEnv, -): boolean { - if (opts.customImageInput === true && opts.customTextInput === true) { - runtime.error("Use either --custom-image-input or --custom-text-input, not both."); - runtime.exit(1); - return false; - } - return true; -} - export function registerOnboardCommand(program: Command): void { const command = program .command("onboard") @@ -426,13 +420,14 @@ export function registerOnboardCommand(program: Command): void { ); return; } - if (!validateOnboardAuthOptionValues(opts as Record, defaultRuntime)) { - return; - } const onboardingOptions = resolveOnboardCommandOptions( opts as Record, commandRuntime, + defaultRuntime, ); + if (!onboardingOptions) { + return; + } const { setupWizardCommand } = await import("../../commands/onboard.js"); await setupWizardCommand(onboardingOptions, defaultRuntime); }); diff --git a/src/cli/program/register.setup.test.ts b/src/cli/program/register.setup.test.ts index b573adeac1de..0554fcf7bffa 100644 --- a/src/cli/program/register.setup.test.ts +++ b/src/cli/program/register.setup.test.ts @@ -54,7 +54,8 @@ vi.mock("../../state/local-onboarding-state.js", () => ({ readLocalOnboardingStateForConfig: mocks.readLocalOnboardingStateMock, })); -vi.mock("../../runtime.js", () => ({ +vi.mock("../../runtime.js", async (importOriginal) => ({ + ...(await importOriginal()), defaultRuntime: mocks.runtime, })); @@ -384,6 +385,7 @@ describe("registerSetupCommand", () => { it.each([ ["onboarding mode", ["--mode", "remote"]], + ["JSON onboarding mode", ["--mode", "remote", "--json"]], ["remote Gateway", ["--remote-url", "wss://example.invalid"]], ["remote Gateway password", ["--remote-password", "fixture-password"]], ["reset", ["--reset"]], @@ -394,6 +396,19 @@ describe("registerSetupCommand", () => { expect(runtime.error).toHaveBeenCalledWith(expect.stringContaining(args[0]!)); expect(runtime.exit).toHaveBeenCalledWith(1); + if (args.includes("--json")) { + expect(runtime.log).toHaveBeenCalledWith( + JSON.stringify( + { + ok: false, + phase: "options", + message: "--baseline cannot be combined with: --mode.", + }, + null, + 2, + ), + ); + } expect(setupCommandMock).not.toHaveBeenCalled(); expect(setupWizardCommandMock).not.toHaveBeenCalled(); }); @@ -435,16 +450,30 @@ describe("registerSetupCommand", () => { expect(setupCommandMock).not.toHaveBeenCalled(); }); - it("rejects conflicting custom model input capabilities", async () => { - await runCli(["setup", "--custom-image-input", "--custom-text-input"]); + it.each([false, true])( + "rejects conflicting custom model input capabilities (json: %s)", + async (json) => { + await runCli([ + "setup", + "--custom-image-input", + "--custom-text-input", + ...(json ? ["--json"] : []), + ]); - expect(runtime.error).toHaveBeenCalledWith( - "Use either --custom-image-input or --custom-text-input, not both.", - ); - expect(runtime.exit).toHaveBeenCalledWith(1); - expect(setupWizardCommandMock).not.toHaveBeenCalled(); - expect(setupCommandMock).not.toHaveBeenCalled(); - }); + const message = "Use either --custom-image-input or --custom-text-input, not both."; + expect(runtime.error).toHaveBeenCalledWith(message); + expect(runtime.exit).toHaveBeenCalledWith(1); + if (json) { + expect(runtime.log).toHaveBeenCalledWith( + JSON.stringify({ ok: false, phase: "options", message }, null, 2), + ); + } else { + expect(runtime.log).not.toHaveBeenCalled(); + } + expect(setupWizardCommandMock).not.toHaveBeenCalled(); + expect(setupCommandMock).not.toHaveBeenCalled(); + }, + ); it.each(["not-a-port", "70000"])( "rejects invalid --gateway-port %s before onboarding dispatch", diff --git a/src/cli/program/register.setup.ts b/src/cli/program/register.setup.ts index ce8e339909c8..d67a07e04351 100644 --- a/src/cli/program/register.setup.ts +++ b/src/cli/program/register.setup.ts @@ -3,6 +3,7 @@ import { readStringValue } from "@openclaw/normalization-core/string-coerce"; import type { Command } from "commander"; import { formatDocsLink } from "../../../packages/terminal-core/src/links.js"; import { theme } from "../../../packages/terminal-core/src/theme.js"; +import { rejectOnboardingOption } from "../../commands/onboard-options.js"; import type { RuntimeEnv } from "../../runtime.js"; import { runCommandWithRuntime } from "../cli-utils.js"; import { hasExplicitOptions, listExplicitOptionFlagsExcept } from "../command-options.js"; @@ -13,7 +14,6 @@ import { registerOnboardRemoteOptions, registerOnboardRuntimeOptions, resolveOnboardCommandOptions, - validateOnboardAuthOptionValues, } from "./register.onboard.js"; const SYSTEM_AGENT_OPTION_NAMES = new Set(["message", "yes", "json"]); @@ -92,8 +92,8 @@ async function runOnboardingEntry( if (options.baseline) { const unsupportedOptions = listExplicitOptionFlagsExcept(commandRuntime, BASELINE_OPTION_NAMES); if (unsupportedOptions.length > 0) { - runtime.error(`--baseline cannot be combined with: ${unsupportedOptions.join(", ")}.`); - runtime.exit(1); + const message = `--baseline cannot be combined with: ${unsupportedOptions.join(", ")}.`; + rejectOnboardingOption({ json: options.json === true }, runtime, message); return; } const { setupCommand } = await import("../../commands/setup.js"); @@ -103,10 +103,10 @@ async function runOnboardingEntry( ); return; } - if (!validateOnboardAuthOptionValues(options, runtime)) { + const onboardingOptions = resolveOnboardCommandOptions(options, commandRuntime, runtime); + if (!onboardingOptions) { return; } - const onboardingOptions = resolveOnboardCommandOptions(options, commandRuntime); const { setupWizardCommand } = await import("../../commands/onboard.js"); await setupWizardCommand(onboardingOptions, runtime); } diff --git a/src/commands/onboard-non-interactive/local.default-agent.test.ts b/src/commands/onboard-non-interactive/local.default-agent.test.ts index 5f707c22d349..bbd892deb879 100644 --- a/src/commands/onboard-non-interactive/local.default-agent.test.ts +++ b/src/commands/onboard-non-interactive/local.default-agent.test.ts @@ -95,42 +95,65 @@ describe("runNonInteractiveLocalSetup default-agent ownership", () => { mocks.inferAuthChoice.mockReturnValue({ matches: [] }); }); - it("rejects ambiguous provider flags before creating an agent or writing setup state", async () => { - mocks.inferAuthChoice.mockReturnValue({ - matches: [ - { optionKey: "openaiApiKey", authChoice: "openai-api-key", label: "--openai-api-key" }, - { - optionKey: "anthropicApiKey", - authChoice: "anthropic-api-key", - label: "--anthropic-api-key", + it.each([false, true])( + "rejects ambiguous provider flags before creating an agent or writing setup state (json: %s)", + async (json) => { + mocks.inferAuthChoice.mockReturnValue({ + matches: [ + { optionKey: "openaiApiKey", authChoice: "openai-api-key", label: "--openai-api-key" }, + { + optionKey: "anthropicApiKey", + authChoice: "anthropic-api-key", + label: "--anthropic-api-key", + }, + ], + }); + + await runNonInteractiveLocalSetup({ + opts: { + nonInteractive: true, + mode: "local", + openaiApiKey: "openai-test-key", + anthropicApiKey: "anthropic-test-key", + skipHooks: true, + skipSkills: true, + skipHealth: true, + json, }, - ], - }); + runtime, + baseConfig: {}, + }); - await runNonInteractiveLocalSetup({ - opts: { - nonInteractive: true, - mode: "local", - openaiApiKey: "openai-test-key", - anthropicApiKey: "anthropic-test-key", - skipHooks: true, - skipSkills: true, - skipHealth: true, - }, - runtime, - baseConfig: {}, - }); - - expect(runtime.error).toHaveBeenCalledWith( - expect.stringContaining("Multiple API key flags were provided"), - ); - expect(runtime.exit).toHaveBeenCalledWith(1); - expect(mocks.applyGatewayConfig).not.toHaveBeenCalled(); - expect(mocks.applyAuthChoice).not.toHaveBeenCalled(); - expect(mocks.ensureOnboardingAgent).not.toHaveBeenCalled(); - expect(mocks.commitConfig).not.toHaveBeenCalled(); - expect(mocks.ensureWorkspaceAndSessions).not.toHaveBeenCalled(); - }); + expect(runtime.error).toHaveBeenCalledWith( + expect.stringContaining("Multiple API key flags were provided"), + ); + expect(runtime.exit).toHaveBeenCalledWith(1); + if (json) { + expect(runtime.log).toHaveBeenCalledWith( + JSON.stringify( + { + ok: false, + phase: "options", + message: [ + "Multiple API key flags were provided for non-interactive setup.", + "Use a single provider flag or pass --auth-choice explicitly.", + "Flags: --openai-api-key, --anthropic-api-key", + ].join("\n"), + }, + null, + 2, + ), + ); + } else { + expect(runtime.log).not.toHaveBeenCalled(); + } + expect(mocks.applyGatewayConfig).not.toHaveBeenCalled(); + expect(mocks.applyAuthChoice).not.toHaveBeenCalled(); + expect(mocks.ensureOnboardingAgent).not.toHaveBeenCalled(); + expect(mocks.commitConfig).not.toHaveBeenCalled(); + expect(mocks.ensureWorkspaceAndSessions).not.toHaveBeenCalled(); + }, + ); it("resolves provider auth in the requested first-agent workspace before creating state", async () => { const workspace = "/tmp/requested-provider-workspace"; diff --git a/src/commands/onboard-non-interactive/local.ts b/src/commands/onboard-non-interactive/local.ts index 3712f9125369..bf1448dac6fe 100644 --- a/src/commands/onboard-non-interactive/local.ts +++ b/src/commands/onboard-non-interactive/local.ts @@ -31,6 +31,7 @@ import { waitForGatewayReachable, } from "../onboard-helpers.js"; import { enableDefaultOnboardingInternalHooks } from "../onboard-hooks.js"; +import { rejectOnboardingOption } from "../onboard-options.js"; import type { OnboardOptions } from "../onboard-types.js"; import { commitNonInteractiveOnboardConfig } from "./config-write.js"; import { applyNonInteractiveGatewayConfig } from "./local/gateway-config.js"; @@ -223,14 +224,12 @@ export async function runNonInteractiveLocalSetup(params: { if (!opts.authChoice && inferredAuthChoice && inferredAuthChoice.matches.length > 1) { // Multiple provider flags make implicit auth selection ambiguous; require a // single explicit --auth-choice rather than choosing by flag order. - runtime.error( - [ - "Multiple API key flags were provided for non-interactive setup.", - "Use a single provider flag or pass --auth-choice explicitly.", - `Flags: ${inferredAuthChoice.matches.map((match) => match.label).join(", ")}`, - ].join("\n"), - ); - runtime.exit(1); + const message = [ + "Multiple API key flags were provided for non-interactive setup.", + "Use a single provider flag or pass --auth-choice explicitly.", + `Flags: ${inferredAuthChoice.matches.map((match) => match.label).join(", ")}`, + ].join("\n"); + rejectOnboardingOption(opts, runtime, message); return; } const authChoice = opts.authChoice ?? inferredAuthChoice?.choice ?? "skip";