diff --git a/docs/cli/config.md b/docs/cli/config.md index 2ead3b799aa..7b93fd6a80b 100644 --- a/docs/cli/config.md +++ b/docs/cli/config.md @@ -334,7 +334,7 @@ openclaw config set channels.discord.token \ - `checks.resolvabilityComplete`: whether resolvability checks ran to completion (false when exec refs are skipped) - `refsChecked`: number of refs actually resolved during dry-run - `skippedExecRefs`: number of exec refs skipped because `--allow-exec` was not set - - `errors`: structured schema/resolvability failures when `ok=false` + - `errors`: structured missing-path, schema, or resolvability failures when `ok=false` @@ -346,7 +346,7 @@ openclaw config set channels.discord.token \ ok: boolean, operations: number, configPath: string, - inputModes: ["value" | "json" | "builder", ...], + inputModes: ["value" | "json" | "builder" | "unset", ...], checks: { schema: boolean, resolvability: boolean, @@ -356,7 +356,7 @@ openclaw config set channels.discord.token \ skippedExecRefs: number, errors?: [ { - kind: "schema" | "resolvability", + kind: "missing-path" | "schema" | "resolvability", message: string, ref?: string, // present for resolvability errors }, diff --git a/src/cli/config-cli.test.ts b/src/cli/config-cli.test.ts index ded78b6c835..41556f10027 100644 --- a/src/cli/config-cli.test.ts +++ b/src/cli/config-cli.test.ts @@ -2694,6 +2694,181 @@ describe("config cli", () => { unsetPaths: [["channels", "discord", "guilds", "123"]], }); }); + + it("dry-runs an unset without writing the config file", async () => { + const resolved: OpenClawConfig = { + agents: { list: [{ id: "main" }] }, + gateway: { port: 18789 }, + tools: { + profile: "coding", + alsoAllow: ["agents_list"], + }, + }; + setSnapshot(resolved, resolved); + setSnapshot(resolved, resolved); + + await runConfigCommand(["config", "unset", "tools.alsoAllow", "--dry-run"]); + + expect(mockWriteConfigFile).not.toHaveBeenCalled(); + expectLogIncludes("Dry run successful: 1 update(s) validated against /tmp/openclaw.json."); + expect(mockReadConfigFileSnapshot).toHaveBeenCalledTimes(2); + }); + + it("prints JSON for config unset dry-run", async () => { + const resolved: OpenClawConfig = { + agents: { list: [{ id: "main" }] }, + gateway: { port: 18789 }, + tools: { + profile: "coding", + alsoAllow: ["agents_list"], + }, + }; + setSnapshot(resolved, resolved); + setSnapshot(resolved, resolved); + + await runConfigCommand(["config", "unset", "tools.alsoAllow", "--dry-run", "--json"]); + + expect(mockWriteConfigFile).not.toHaveBeenCalled(); + expect(parseLastLogPayload()).toMatchObject({ + ok: true, + operations: 1, + inputModes: ["unset"], + checks: { + schema: true, + resolvability: true, + resolvabilityComplete: true, + }, + }); + }); + + it("prints structured JSON when unset dry-run misses a path", async () => { + const resolved: OpenClawConfig = { + gateway: { port: 18789 }, + tools: { + profile: "coding", + }, + }; + setSnapshot(resolved, resolved); + + await expect( + runConfigCommand(["config", "unset", "tools.alsoAllow", "--dry-run", "--json"]), + ).rejects.toThrow("__exit__:1"); + + expect(mockWriteConfigFile).not.toHaveBeenCalled(); + expect(mockError).not.toHaveBeenCalled(); + const payload = parseLastLogPayload() as { + ok: boolean; + inputModes: string[]; + checks: { schema: boolean; resolvability: boolean; resolvabilityComplete: boolean }; + errors?: Array<{ kind: string; message: string }>; + }; + expect(payload.ok).toBe(false); + expect(payload.inputModes).toEqual(["unset"]); + expect(payload.checks).toEqual({ + schema: false, + resolvability: false, + resolvabilityComplete: false, + }); + expect(payload.errors).toEqual([ + { + kind: "missing-path", + message: "Config path not found: tools.alsoAllow. Nothing was changed.", + }, + ]); + }); + + it("validates existing refs when unset dry-run removes all secret providers", async () => { + const resolved: OpenClawConfig = { + gateway: { port: 18789 }, + secrets: { + providers: { + vaultfile: { source: "file", path: "/tmp/secrets.json", mode: "json" }, + }, + }, + tools: { + web: { + search: { + enabled: true, + apiKey: { + source: "file", + provider: "vaultfile", + id: "/providers/search/apiKey", + }, + }, + }, + } as never, + }; + setSnapshot(resolved, resolved); + setSnapshot(resolved, resolved); + mockResolveSecretRefValue.mockRejectedValueOnce(new Error("provider removed")); + + await expect( + runConfigCommand(["config", "unset", "secrets.providers", "--dry-run"]), + ).rejects.toThrow("__exit__:1"); + + expect(mockWriteConfigFile).not.toHaveBeenCalled(); + const [secretRef] = requireResolveSecretRefCall(0); + const secretRefRecord = requireRecord(secretRef, "existing SecretRef"); + expect(secretRefRecord.provider).toBe("vaultfile"); + expect(secretRefRecord.id).toBe("/providers/search/apiKey"); + expectErrorIncludes("Dry run failed: 1 SecretRef assignment(s) could not be resolved."); + expectErrorIncludes("provider removed"); + }); + + it("validates existing refs when unset dry-run removes secret defaults", async () => { + const resolved: OpenClawConfig = { + gateway: { port: 18789 }, + secrets: { + defaults: { + env: "vaultenv", + }, + providers: { + default: { source: "env" }, + vaultenv: { source: "env" }, + }, + }, + tools: { + web: { + search: { + enabled: true, + apiKey: "${WEB_SEARCH_API_KEY}", + }, + }, + } as never, + } as OpenClawConfig; + setSnapshot(resolved, resolved); + setSnapshot(resolved, resolved); + + await runConfigCommand(["config", "unset", "secrets.defaults", "--dry-run"]); + + expect(mockWriteConfigFile).not.toHaveBeenCalled(); + const [secretRef] = requireResolveSecretRefCall(0); + const secretRefRecord = requireRecord(secretRef, "defaulted SecretRef"); + expect(secretRefRecord).toMatchObject({ + source: "env", + provider: "default", + id: "WEB_SEARCH_API_KEY", + }); + expectLogIncludes("Dry run successful: 1 update(s) validated against /tmp/openclaw.json."); + }); + + it("rejects config unset --json without --dry-run", async () => { + await expect( + runConfigCommand(["config", "unset", "tools.alsoAllow", "--json"]), + ).rejects.toThrow("__exit__:1"); + + expect(mockWriteConfigFile).not.toHaveBeenCalled(); + expectErrorIncludes("--json can only be used with --dry-run."); + }); + + it("rejects config unset --allow-exec without --dry-run", async () => { + await expect( + runConfigCommand(["config", "unset", "tools.alsoAllow", "--allow-exec"]), + ).rejects.toThrow("__exit__:1"); + + expect(mockWriteConfigFile).not.toHaveBeenCalled(); + expectErrorIncludes("--allow-exec can only be used with --dry-run."); + }); }); describe("config file", () => { diff --git a/src/cli/config-cli.ts b/src/cli/config-cli.ts index df9983611ca..a0949c8abb6 100644 --- a/src/cli/config-cli.ts +++ b/src/cli/config-cli.ts @@ -77,6 +77,7 @@ type ConfigSetOperation = { value: unknown; mutation?: "set" | "merge" | "replace" | "delete"; schemaValidated?: boolean; + touchesAllSecretRefs?: boolean; touchedSecretTargetPath?: string; touchedProviderAlias?: string; assignedRef?: SecretRef; @@ -89,6 +90,11 @@ type ConfigPatchOptions = { json?: boolean | undefined; replacePath?: string[] | undefined; }; +type ConfigUnsetOptions = { + dryRun?: boolean | undefined; + allowExec?: boolean | undefined; + json?: boolean | undefined; +}; type ConfigMutationOptions = { dryRun?: boolean | undefined; allowExec?: boolean | undefined; @@ -1029,6 +1035,20 @@ function parseProviderAliasFromTargetPath(path: PathSegment[]): string | null { return null; } +function touchesSecretProviderCollection(path: PathSegment[]): boolean { + return ( + (path.length === 1 && path[0] === "secrets") || + (path.length === 2 && path[0] === "secrets" && path[1] === "providers") + ); +} + +function touchesSecretDefaults(path: PathSegment[]): boolean { + return ( + (path.length === 1 && path[0] === "secrets") || + (path.length === 2 && path[0] === "secrets" && path[1] === "defaults") + ); +} + function buildValueAssignmentOperation(params: { requestedPath: PathSegment[]; value: unknown; @@ -1153,6 +1173,22 @@ function buildDeleteOperation(path: PathSegment[]): ConfigSetOperation { }; } +function buildUnsetOperation(path: PathSegment[]): ConfigSetOperation { + const resolved = resolveConfigSecretTargetByPath(path); + const providerAlias = parseProviderAliasFromTargetPath(path); + const touchesAllSecretRefs = touchesSecretProviderCollection(path) || touchesSecretDefaults(path); + return { + inputMode: "unset", + requestedPath: path, + setPath: path, + value: undefined, + mutation: "delete", + ...(touchesAllSecretRefs ? { touchesAllSecretRefs: true } : {}), + ...(resolved ? { touchedSecretTargetPath: toDotPath(resolved.pathSegments) } : {}), + ...(providerAlias ? { touchedProviderAlias: providerAlias } : {}), + }; +} + function buildApplyValueOperation(params: { path: PathSegment[]; value: unknown; @@ -1357,6 +1393,7 @@ function collectDryRunRefs(params: { const refsByKey = new Map(); const targetPaths = new Set(); const providerAliases = new Set(); + let includeAllDiscoveredRefs = false; for (const operation of params.operations) { if (operation.assignedRef) { @@ -1371,9 +1408,10 @@ function collectDryRunRefs(params: { if (operation.touchedProviderAlias) { providerAliases.add(operation.touchedProviderAlias); } + includeAllDiscoveredRefs ||= operation.touchesAllSecretRefs === true; } - if (targetPaths.size === 0 && providerAliases.size === 0) { + if (!includeAllDiscoveredRefs && targetPaths.size === 0 && providerAliases.size === 0) { return [...refsByKey.values()]; } @@ -1387,7 +1425,7 @@ function collectDryRunRefs(params: { if (!ref) { continue; } - if (targetPaths.has(target.path) || providerAliases.has(ref.provider)) { + if (includeAllDiscoveredRefs || targetPaths.has(target.path) || providerAliases.has(ref.provider)) { refsByKey.set(secretRefKey(ref), ref); } } @@ -1624,9 +1662,13 @@ function formatDryRunFailureMessage(params: { skippedExecRefs: number; }): string { const { errors, skippedExecRefs } = params; + const missingPathErrors = errors.filter((error) => error.kind === "missing-path"); const schemaErrors = errors.filter((error) => error.kind === "schema"); const resolveErrors = errors.filter((error) => error.kind === "resolvability"); const lines: string[] = []; + if (missingPathErrors.length > 0) { + lines.push(...missingPathErrors.map((error) => error.message)); + } if (schemaErrors.length > 0) { lines.push("Dry run failed: config schema validation failed."); lines.push(...schemaErrors.map((error) => `- ${error.message}`)); @@ -1716,11 +1758,14 @@ async function runConfigOperations(params: { if (options.dryRun) { const hasJsonMode = operations.some((operation) => operation.inputMode === "json"); const hasBuilderMode = operations.some((operation) => operation.inputMode === "builder"); + const hasUnsetMode = operations.some((operation) => operation.inputMode === "unset"); const requiresFullSchemaValidation = operations.some( - (operation) => operation.inputMode === "json" && operation.schemaValidated !== true, + (operation) => + operation.inputMode === "unset" || + (operation.inputMode === "json" && operation.schemaValidated !== true), ); const refs = - hasJsonMode || hasBuilderMode + hasJsonMode || hasBuilderMode || hasUnsetMode ? collectDryRunRefs({ config: nextConfig, operations, @@ -1746,7 +1791,7 @@ async function runConfigOperations(params: { }), ); } - if (hasJsonMode || hasBuilderMode) { + if (hasJsonMode || hasBuilderMode || hasUnsetMode) { errors.push( ...collectDryRunStaticErrorsForSkippedExecRefs({ refs: selectedDryRunRefs.skippedExecRefs, @@ -1768,9 +1813,10 @@ async function runConfigOperations(params: { inputModes: [...new Set(operations.map((operation) => operation.inputMode))], checks: { schema: requiresFullSchemaValidation || policyIssueLines.length > 0, - resolvability: hasJsonMode || hasBuilderMode, + resolvability: hasJsonMode || hasBuilderMode || hasUnsetMode, resolvabilityComplete: - (hasJsonMode || hasBuilderMode) && selectedDryRunRefs.skippedExecRefs.length === 0, + (hasJsonMode || hasBuilderMode || hasUnsetMode) && + selectedDryRunRefs.skippedExecRefs.length === 0, }, refsChecked: selectedDryRunRefs.refsToResolve.length, skippedExecRefs: selectedDryRunRefs.skippedExecRefs.length, @@ -1986,9 +2032,20 @@ export async function runConfigGet(opts: { path: string; json?: boolean; runtime } } -export async function runConfigUnset(opts: { path: string; runtime?: RuntimeEnv }) { +export async function runConfigUnset(opts: { + path: string; + cliOptions?: ConfigUnsetOptions; + runtime?: RuntimeEnv; +}) { const runtime = opts.runtime ?? defaultRuntime; + const cliOptions = opts.cliOptions ?? {}; try { + if (cliOptions.allowExec && !cliOptions.dryRun) { + throw new Error("--allow-exec can only be used with --dry-run."); + } + if (cliOptions.json && !cliOptions.dryRun) { + throw new Error("--json can only be used with --dry-run."); + } const parsedPath = parseRequiredPath(opts.path); const autoManagedUnsetTargets = findAutoManagedMetaUnsetTargets(parsedPath); if (autoManagedUnsetTargets.length > 0) { @@ -2001,6 +2058,27 @@ export async function runConfigUnset(opts: { path: string; runtime?: RuntimeEnv const next = structuredClone(snapshot.resolved) as Record; const unsetResult = unsetAtPath(next, parsedPath); if (!unsetResult.removed) { + if (cliOptions.dryRun && cliOptions.json) { + throw new ConfigSetDryRunValidationError({ + ok: false, + operations: 1, + configPath: shortenHomePath(snapshot.path), + inputModes: ["unset"], + checks: { + schema: false, + resolvability: false, + resolvabilityComplete: false, + }, + refsChecked: 0, + skippedExecRefs: 0, + errors: [ + { + kind: "missing-path", + message: `Config path not found: ${opts.path}. Nothing was changed.`, + }, + ], + }); + } runtime.error( danger( `Config path not found: ${opts.path}. Nothing was changed. Run ${formatCliCommand("openclaw config get ")} first if you are unsure of the path.`, @@ -2009,6 +2087,15 @@ export async function runConfigUnset(opts: { path: string; runtime?: RuntimeEnv runtime.exit(1); return; } + if (cliOptions.dryRun) { + await runConfigOperations({ + runtime, + operations: [buildUnsetOperation(parsedPath)], + options: cliOptions, + successMode: "set", + }); + return; + } await replaceConfigFile({ nextConfig: next, ...(snapshot.hash !== undefined ? { baseHash: snapshot.hash } : {}), @@ -2018,8 +2105,7 @@ export async function runConfigUnset(opts: { path: string; runtime?: RuntimeEnv }); runtime.log(info(`Removed ${opts.path}. Restart the gateway to apply.`)); } catch (err) { - runtime.error(danger(String(err))); - runtime.exit(1); + handleConfigMutationError({ err, runtime, options: cliOptions }); } } @@ -2258,8 +2344,11 @@ export function registerConfigCli(program: Command) { .command("unset") .description("Remove a config value by dot path") .argument("", "Config path (dot or bracket notation)") - .action(async (path: string) => { - await runConfigUnset({ path }); + .option("--dry-run", "validate the removal without writing the config file") + .option("--allow-exec", "allow exec SecretRef providers during --dry-run") + .option("--json", "print dry-run result as JSON") + .action(async (path: string, options: ConfigUnsetOptions) => { + await runConfigUnset({ path, cliOptions: options }); }); cmd diff --git a/src/cli/config-set-dryrun.ts b/src/cli/config-set-dryrun.ts index d121f25eab1..dfc4160716d 100644 --- a/src/cli/config-set-dryrun.ts +++ b/src/cli/config-set-dryrun.ts @@ -1,7 +1,7 @@ -export type ConfigSetDryRunInputMode = "value" | "json" | "builder"; +export type ConfigSetDryRunInputMode = "value" | "json" | "builder" | "unset"; export type ConfigSetDryRunError = { - kind: "schema" | "resolvability"; + kind: "missing-path" | "schema" | "resolvability"; message: string; ref?: string; }; diff --git a/src/cli/program/route-args.test.ts b/src/cli/program/route-args.test.ts index 85e4bbc511d..ab2bb6ab248 100644 --- a/src/cli/program/route-args.test.ts +++ b/src/cli/program/route-args.test.ts @@ -149,6 +149,30 @@ describe("route-args", () => { ]), ).toEqual({ path: "update.channel", + cliOptions: { + dryRun: false, + allowExec: false, + json: false, + }, + }); + expect( + parseConfigUnsetRouteArgs([ + "node", + "openclaw", + "config", + "unset", + "--dry-run", + "--json", + "--allow-exec", + "update.channel", + ]), + ).toEqual({ + path: "update.channel", + cliOptions: { + dryRun: true, + allowExec: true, + json: true, + }, }); expect(parseConfigGetRouteArgs(["node", "openclaw", "config", "get", "--json"])).toBeNull(); }); diff --git a/src/cli/program/route-args.ts b/src/cli/program/route-args.ts index 9132896e9db..aedd2903757 100644 --- a/src/cli/program/route-args.ts +++ b/src/cli/program/route-args.ts @@ -182,11 +182,19 @@ export function parseConfigGetRouteArgs(argv: string[]) { export function parseConfigUnsetRouteArgs(argv: string[]) { const path = parseSinglePositional(argv, { commandPath: ["config", "unset"], + booleanFlags: ["--dry-run", "--allow-exec", "--json"], }); if (!path) { return null; } - return { path }; + return { + path, + cliOptions: { + dryRun: hasFlag(argv, "--dry-run"), + allowExec: hasFlag(argv, "--allow-exec"), + json: hasFlag(argv, "--json"), + }, + }; } export function parseModelsListRouteArgs(argv: string[]) { diff --git a/src/cli/program/routes.test.ts b/src/cli/program/routes.test.ts index 065f4f30a12..804dff02d17 100644 --- a/src/cli/program/routes.test.ts +++ b/src/cli/program/routes.test.ts @@ -344,7 +344,14 @@ describe("program routes", () => { await expect( route.run(["node", "openclaw", "--profile", "work", "config", "unset", "update.channel"]), ).resolves.toBe(true); - expect(runConfigUnsetMock).toHaveBeenCalledWith({ path: "update.channel" }); + expect(runConfigUnsetMock).toHaveBeenCalledWith({ + path: "update.channel", + cliOptions: { + dryRun: false, + allowExec: false, + json: false, + }, + }); }); it("passes config get path when root value options appear after subcommand", async () => { @@ -369,7 +376,38 @@ describe("program routes", () => { await expect( route.run(["node", "openclaw", "config", "unset", "--profile", "work", "update.channel"]), ).resolves.toBe(true); - expect(runConfigUnsetMock).toHaveBeenCalledWith({ path: "update.channel" }); + expect(runConfigUnsetMock).toHaveBeenCalledWith({ + path: "update.channel", + cliOptions: { + dryRun: false, + allowExec: false, + json: false, + }, + }); + }); + + it("passes config unset dry-run options", async () => { + const route = expectRoute(["config", "unset"]); + await expect( + route.run([ + "node", + "openclaw", + "config", + "unset", + "--dry-run", + "--json", + "--allow-exec", + "update.channel", + ]), + ).resolves.toBe(true); + expect(runConfigUnsetMock).toHaveBeenCalledWith({ + path: "update.channel", + cliOptions: { + dryRun: true, + allowExec: true, + json: true, + }, + }); }); it("returns false for config get route when unknown option appears", async () => {