fix: strip plugin install records before config validation

This commit is contained in:
Shakker
2026-04-25 23:26:19 +01:00
parent 56f4264f1b
commit b018272fa1
6 changed files with 87 additions and 66 deletions

View File

@@ -273,7 +273,7 @@ describe("config cli", () => {
});
});
it("dry-runs nested plugin install updates without dropping sibling fields", async () => {
it("rejects plugin install record config updates", async () => {
const resolved = {
plugins: {
installs: {
@@ -294,18 +294,19 @@ describe("config cli", () => {
} as unknown as OpenClawConfig;
setSnapshot(resolved, resolved);
await runConfigCommand([
"config",
"set",
'plugins.installs["openclaw-web-search"].spec',
'"@ollama/openclaw-web-search@0.2.2"',
"--strict-json",
"--dry-run",
]);
await expect(
runConfigCommand([
"config",
"set",
'plugins.installs["openclaw-web-search"].spec',
'"@ollama/openclaw-web-search@0.2.2"',
"--strict-json",
"--dry-run",
]),
).rejects.toThrow("__exit__:1");
expect(mockWriteConfigFile).not.toHaveBeenCalled();
expect(mockError).not.toHaveBeenCalled();
expect(mockLog).toHaveBeenCalledWith(expect.stringContaining("Dry run successful"));
expect(mockError).toHaveBeenCalledWith(expect.stringContaining("Unrecognized key"));
});
it("rejects protected model map replacement unless explicitly requested", async () => {

View File

@@ -56,11 +56,11 @@ import {
collectChangedPaths,
createMergePatch,
formatConfigValidationFailure,
applyUnsetPathsForWrite,
projectSourceOntoRuntimeShape,
restoreEnvRefsFromMap,
resolvePersistCandidateForWrite,
resolveWriteEnvSnapshotForPath,
unsetPathForWrite,
} from "./io.write-prepare.js";
import { findLegacyConfigIssues } from "./legacy.js";
import {
@@ -1655,6 +1655,11 @@ export function createConfigIO(
}
}
persistCandidate = applyUnsetPathsForWrite(
persistCandidate as OpenClawConfig,
options.unsetPaths,
);
const validated = validateConfigObjectRawWithPlugins(persistCandidate, { env: deps.env });
if (!validated.ok) {
const issue = validated.issues[0];
@@ -1715,18 +1720,7 @@ export function createConfigIO(
envRefMap && changedPaths
? (restoreEnvRefsFromMap(cfgToWrite, "", envRefMap, changedPaths) as OpenClawConfig)
: cfgToWrite;
let outputConfig = outputConfigBase;
if (options.unsetPaths?.length) {
for (const unsetPath of options.unsetPaths) {
if (!Array.isArray(unsetPath) || unsetPath.length === 0) {
continue;
}
const unsetResult = unsetPathForWrite(outputConfig, unsetPath);
if (unsetResult.changed) {
outputConfig = unsetResult.next;
}
}
}
const outputConfig = applyUnsetPathsForWrite(outputConfigBase, options.unsetPaths);
// Do NOT apply runtime defaults when writing - user config should only contain
// explicitly set values. Runtime defaults are applied when loading (issue #6070).
const stampedOutputConfig = stampConfigVersion(outputConfig);

View File

@@ -2,6 +2,7 @@ import { describe, expect, it } from "vitest";
import {
collectChangedPaths,
formatConfigValidationFailure,
applyUnsetPathsForWrite,
restoreEnvRefsFromMap,
resolvePersistCandidateForWrite,
resolveWriteEnvSnapshotForPath,
@@ -38,54 +39,51 @@ describe("config io write prepare", () => {
expect(persisted).not.toHaveProperty("sessions.persistence");
});
it("preserves authored source-only nested fields during partial writes", () => {
const persisted = resolvePersistCandidateForWrite({
runtimeConfig: {
plugins: {
entries: {},
it("strips transient plugin install records from partial writes", () => {
const persisted = applyUnsetPathsForWrite(
resolvePersistCandidateForWrite({
runtimeConfig: {
plugins: {
entries: {},
},
},
},
sourceConfig: {
plugins: {
entries: {},
installs: {
"openclaw-web-search": {
source: "npm",
spec: "@ollama/openclaw-web-search",
installPath: "/tmp/openclaw-web-search",
resolvedName: "@ollama/openclaw-web-search",
resolvedVersion: "0.2.2",
sourceConfig: {
plugins: {
entries: {},
installs: {
"openclaw-web-search": {
source: "npm",
spec: "@ollama/openclaw-web-search",
installPath: "/tmp/openclaw-web-search",
resolvedName: "@ollama/openclaw-web-search",
resolvedVersion: "0.2.2",
},
},
},
},
},
nextConfig: {
plugins: {
entries: {},
installs: {
"openclaw-web-search": {
source: "npm",
spec: "@ollama/openclaw-web-search@0.2.2",
installPath: "/tmp/openclaw-web-search",
resolvedName: "@ollama/openclaw-web-search",
resolvedVersion: "0.2.2",
nextConfig: {
plugins: {
entries: {},
installs: {
"openclaw-web-search": {
source: "npm",
spec: "@ollama/openclaw-web-search@0.2.2",
installPath: "/tmp/openclaw-web-search",
resolvedName: "@ollama/openclaw-web-search",
resolvedVersion: "0.2.2",
},
},
},
},
},
}) as {
}),
[["plugins", "installs"]],
) as {
plugins?: {
installs?: Record<string, Record<string, unknown>>;
};
};
expect(persisted.plugins?.installs?.["openclaw-web-search"]).toEqual({
source: "npm",
spec: "@ollama/openclaw-web-search@0.2.2",
installPath: "/tmp/openclaw-web-search",
resolvedName: "@ollama/openclaw-web-search",
resolvedVersion: "0.2.2",
});
expect(persisted.plugins?.installs).toBeUndefined();
});
it("preserves untouched include-owned subtrees during unrelated writes", () => {

View File

@@ -320,6 +320,23 @@ export function unsetPathForWrite(
return { changed: false, next: root };
}
export function applyUnsetPathsForWrite(
root: OpenClawConfig,
unsetPaths: readonly string[][] | undefined,
): OpenClawConfig {
let next = root;
for (const unsetPath of unsetPaths ?? []) {
if (!Array.isArray(unsetPath) || unsetPath.length === 0) {
continue;
}
const unsetResult = unsetPathForWrite(next, unsetPath);
if (unsetResult.changed) {
next = unsetResult.next;
}
}
return next;
}
export function collectChangedPaths(
base: unknown,
target: unknown,

View File

@@ -166,7 +166,10 @@ describe("config mutate helpers", () => {
await replaceConfigFile({
baseHash: snapshot.hash,
snapshot,
writeOptions: { expectedConfigPath: snapshot.path },
writeOptions: {
expectedConfigPath: snapshot.path,
unsetPaths: [["plugins", "installs"]],
},
nextConfig: {
plugins: {
entries: {
@@ -194,7 +197,7 @@ describe("config mutate helpers", () => {
installs?: Record<string, unknown>;
};
expect(persistedPlugins.entries?.demo).toEqual({ enabled: true });
expect(persistedPlugins.installs?.demo).toMatchObject({ source: "npm", spec: "demo" });
expect(persistedPlugins.installs).toBeUndefined();
});
it("falls back to the root writer when a plugins include write is not isolated", async () => {

View File

@@ -13,6 +13,7 @@ import {
writeConfigFile,
type ConfigWriteOptions,
} from "./io.js";
import { applyUnsetPathsForWrite } from "./io.write-prepare.js";
import type { ConfigFileSnapshot, OpenClawConfig } from "./types.js";
import { validateConfigObjectWithPlugins } from "./validation.js";
@@ -111,20 +112,22 @@ async function writeJsonFileAtomic(filePath: string, value: unknown): Promise<vo
async function tryWriteSingleTopLevelIncludeMutation(params: {
snapshot: ConfigFileSnapshot;
nextConfig: OpenClawConfig;
writeOptions?: ConfigWriteOptions;
}): Promise<boolean> {
const changedKeys = getChangedTopLevelKeys(params.snapshot.sourceConfig, params.nextConfig);
const nextConfig = applyUnsetPathsForWrite(params.nextConfig, params.writeOptions?.unsetPaths);
const changedKeys = getChangedTopLevelKeys(params.snapshot.sourceConfig, nextConfig);
if (changedKeys.length !== 1 || changedKeys[0] === "<root>") {
return false;
}
const key = changedKeys[0];
const includePath = getSingleTopLevelIncludeTarget({ snapshot: params.snapshot, key });
if (!includePath || !isRecord(params.nextConfig) || !(key in params.nextConfig)) {
if (!includePath || !isRecord(nextConfig) || !(key in nextConfig)) {
return false;
}
const nextConfigRecord = params.nextConfig as Record<string, unknown>;
const nextConfigRecord = nextConfig as Record<string, unknown>;
const validated = validateConfigObjectWithPlugins(params.nextConfig);
const validated = validateConfigObjectWithPlugins(nextConfig);
if (!validated.ok) {
throw createInvalidConfigError(
params.snapshot.path,
@@ -151,6 +154,7 @@ export async function replaceConfigFile(params: {
const wroteInclude = await tryWriteSingleTopLevelIncludeMutation({
snapshot,
nextConfig: params.nextConfig,
writeOptions: params.writeOptions ?? writeOptions,
});
if (!wroteInclude) {
await writeConfigFile(params.nextConfig, {
@@ -184,6 +188,10 @@ export async function mutateConfigFile<T = void>(params: {
const wroteInclude = await tryWriteSingleTopLevelIncludeMutation({
snapshot,
nextConfig: draft,
writeOptions: {
...writeOptions,
...params.writeOptions,
},
});
if (!wroteInclude) {
await writeConfigFile(draft, {