fix(config): add --dry-run support to unset (#81895)

* Add config unset dry-run

Add --dry-run support to config unset, including JSON output and allow-exec validation parity with config set/patch dry-run handling.

* Refresh checks after proof update

* fix(config): address unset dry-run review

Return structured JSON when config unset dry-run misses a path and validate broad secret provider/default unsets against affected SecretRefs.
This commit is contained in:
Gio Della-Libera
2026-05-16 11:09:42 -07:00
committed by GitHub
parent e06782d5e7
commit 489cab2738
7 changed files with 354 additions and 20 deletions
+3 -3
View File
@@ -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`
</Accordion>
</AccordionGroup>
@@ -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
},
+175
View File
@@ -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", () => {
+101 -12
View File
@@ -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<string, SecretRef>();
const targetPaths = new Set<string>();
const providerAliases = new Set<string>();
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<string, unknown>;
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 <path>")} 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("<path>", "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
+2 -2
View File
@@ -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;
};
+24
View File
@@ -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();
});
+9 -1
View File
@@ -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[]) {
+40 -2
View File
@@ -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 () => {