mirror of
https://github.com/openclaw/openclaw.git
synced 2026-05-06 10:40:43 +00:00
fix(doctor): commit legacy migrations even when unrelated validation fails (#76800)
* fix(doctor): commit legacy migrations even when unrelated validation fails (#76798) migrateLegacyConfig previously returned config: null when post-migration validation found any issue (e.g. a missing plugin). The caller then kept the unmigrated config as the candidate, so doctor --fix never wrote the legacy migration to disk. Now when validation fails after a successful migration, the migrated config is returned with partiallyValid: true. applyLegacyCompatibilityStep always commits the migrated config to state.candidate, ensuring agents.defaults.llm and other known-legacy keys are cleaned up on doctor --fix even when an unrelated provider or plugin issue blocks the full validator. Adds regression test asserting that candidate is updated to the migrated shape when partiallyValid is set. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fixup: extend skipPluginValidation to write path with E2E coverage Thread skipPluginValidationOnWrite through loadAndMaybeMigrateDoctorConfig return value into runWriteConfigHealth so replaceConfigFile bypasses plugin validation when migration is only partially valid. Add E2E test verifying the flag propagates end-to-end. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fixup(doctor): wire skipPluginValidation through full write path (#76800) Clawsweeper P2 x2: 1. io.ts exported writeConfigFile wrapper now passes skipPluginValidation to createConfigIO so both write-phase validation and post-write loadConfig re-read honor the flag. 2. mutate.ts tryWriteSingleTopLevelIncludeMutation now skips plugin validation when writeOptions.skipPluginValidation is set, so include-write fast path no longer blocks safe legacy migrations with unrelated plugin errors. Adds regression test: skipPluginValidation bypasses plugin schema rejection on writeConfigFile and falls back to throwing when flag is not set. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fixup(doctor): bail out of include fast path when skipPluginValidation is set (#76800) Clawsweeper P2: after the include write, readConfigFileSnapshotForWrite() calls loadConfig() which validates with plugins; refreshedSnapshot.valid is false when an unrelated plugin issue exists, causing the include path to throw even though skipPluginValidation was requested. Simplest fix: return false from tryWriteSingleTopLevelIncludeMutation when skipPluginValidation is set, letting the root writer handle the write with plugin validation disabled end-to-end (including post-write readback via createConfigIO({ pluginValidation: "skip" })). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * test(doctor): cover partial legacy migration writes --------- Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com> Co-authored-by: Peter Steinberger <steipete@gmail.com>
This commit is contained in:
@@ -13,6 +13,7 @@ Docs: https://docs.openclaw.ai
|
||||
- Channels/streaming: add unified `streaming.mode: "progress"` drafts with auto single-word status labels and shared progress configuration across Discord, Telegram, Matrix, Slack, and Microsoft Teams.
|
||||
- Agents/commands: add `/steer <message>` for queue-independent steering of the active current-session run without starting a new turn when the session is idle. (#76934)
|
||||
- Tools/BTW: add `/side` as a text and native slash-command alias for `/btw` side questions.
|
||||
- Doctor/config: `doctor --fix` now commits safe legacy migrations even when unrelated validation issues (e.g. a missing plugin) prevent full validation from passing, so `agents.defaults.llm` and other known-legacy keys are always cleaned up by `doctor --fix` regardless of other config problems. (#76798)
|
||||
- Agents/tools: skip optional media and PDF tool factories when the effective tool denylist already blocks them, avoiding unnecessary hot-path setup for tools that will be filtered out before model use. (#76773) Thanks @dorukardahan.
|
||||
- Discord/status: let explicit reaction tool calls opt into tracking subsequent tool progress on the reacted message with `trackToolCalls: true`, and use the shared tool display emoji table for status reactions.
|
||||
- Gateway/config: stop Gateway startup and hot reload from auto-restoring invalid config; invalid config now fails closed and `openclaw doctor --fix` owns last-known-good repair.
|
||||
|
||||
@@ -196,11 +196,17 @@ const legacyConfigMigrationForTest = vi.hoisted(() => {
|
||||
return changes.length > 0 ? { next, changes } : { next: null, changes: [] };
|
||||
}
|
||||
|
||||
let partiallyValidOverride: boolean | undefined;
|
||||
|
||||
return {
|
||||
migrate,
|
||||
migrateLegacyConfig: (raw: unknown) => {
|
||||
const { next, changes } = migrate(raw);
|
||||
return { config: next, changes };
|
||||
const partiallyValid = partiallyValidOverride;
|
||||
return { config: next, changes, ...(partiallyValid ? { partiallyValid } : {}) };
|
||||
},
|
||||
setPartiallyValidOverride(value: boolean | undefined) {
|
||||
partiallyValidOverride = value;
|
||||
},
|
||||
};
|
||||
});
|
||||
@@ -595,7 +601,7 @@ vi.mock("./doctor/shared/channel-legacy-config-migrate.js", () => ({
|
||||
}));
|
||||
|
||||
vi.mock("./doctor/shared/legacy-config-migrate.js", () => ({
|
||||
migrateLegacyConfig: legacyConfigMigrationForTest.migrateLegacyConfig,
|
||||
migrateLegacyConfig: (raw: unknown) => legacyConfigMigrationForTest.migrateLegacyConfig(raw),
|
||||
}));
|
||||
|
||||
vi.mock("./doctor/shared/bundled-plugin-load-paths.js", () => ({
|
||||
@@ -2643,4 +2649,23 @@ describe("doctor config flow", () => {
|
||||
{ skipSessionCleanup: true },
|
||||
);
|
||||
});
|
||||
|
||||
it("sets skipPluginValidationOnWrite when legacy migration is only partially valid (#76800)", async () => {
|
||||
legacyConfigMigrationForTest.setPartiallyValidOverride(true);
|
||||
try {
|
||||
const result = await runDoctorConfigWithInput({
|
||||
config: {
|
||||
heartbeat: { model: "openai/gpt-4o", every: 60 },
|
||||
tools: { web: { search: { provider: "brave" } } },
|
||||
},
|
||||
repair: true,
|
||||
preflightMode: "compat",
|
||||
run: ({ options, confirm }) =>
|
||||
loadAndMaybeMigrateDoctorConfig({ options, confirm: async () => confirm() }),
|
||||
});
|
||||
expect(result.skipPluginValidationOnWrite).toBe(true);
|
||||
} finally {
|
||||
legacyConfigMigrationForTest.setPartiallyValidOverride(undefined);
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
@@ -87,6 +87,7 @@ export async function loadAndMaybeMigrateDoctorConfig(params: {
|
||||
doctorFixCommand,
|
||||
});
|
||||
({ cfg, candidate, pendingChanges, fixHints } = legacyStep.state);
|
||||
const legacyMigrationPartiallyValid = legacyStep.partiallyValid === true;
|
||||
const pluginLegacyIssues = await (async () => {
|
||||
if (snapshot.parsed === snapshot.sourceConfig) {
|
||||
return [];
|
||||
@@ -280,5 +281,6 @@ export async function loadAndMaybeMigrateDoctorConfig(params: {
|
||||
shouldWriteConfig: finalized.shouldWriteConfig,
|
||||
sourceConfigValid: snapshot.valid,
|
||||
...(sourceLastTouchedVersion ? { sourceLastTouchedVersion } : {}),
|
||||
...(legacyMigrationPartiallyValid ? { skipPluginValidationOnWrite: true } : {}),
|
||||
};
|
||||
}
|
||||
|
||||
@@ -101,6 +101,44 @@ describe("doctor config flow steps", () => {
|
||||
);
|
||||
});
|
||||
|
||||
it("commits migration even when post-migration validation has unrelated issues (#76798)", () => {
|
||||
const migratedConfig = { agents: { defaults: { model: { primary: "openai/gpt-5.4" } } } };
|
||||
migrateLegacyConfigMock.mockReturnValueOnce({
|
||||
config: migratedConfig,
|
||||
changes: ["Removed agents.defaults.llm; model idle timeout now follows models.providers."],
|
||||
partiallyValid: true,
|
||||
});
|
||||
|
||||
const result = createLegacyStepResult({
|
||||
exists: true,
|
||||
parsed: {
|
||||
agents: {
|
||||
defaults: { llm: { idleTimeoutSeconds: 120 }, model: { primary: "openai/gpt-5.4" } },
|
||||
},
|
||||
tools: { web: { search: { provider: "brave" } } },
|
||||
},
|
||||
legacyIssues: [{ path: "agents.defaults.llm", message: "deprecated key" }],
|
||||
path: "/tmp/config.json",
|
||||
valid: false,
|
||||
issues: [
|
||||
{
|
||||
path: "tools.web.search.provider",
|
||||
message: "web_search provider is not available: brave",
|
||||
},
|
||||
],
|
||||
raw: "{}",
|
||||
resolved: {},
|
||||
sourceConfig: {},
|
||||
config: {},
|
||||
runtimeConfig: {},
|
||||
warnings: [],
|
||||
} satisfies DoctorConfigPreflightResult["snapshot"]);
|
||||
|
||||
expect(result.state.candidate).toEqual(migratedConfig);
|
||||
expect(result.state.cfg).toEqual(migratedConfig);
|
||||
expect(result.state.pendingChanges).toBe(true);
|
||||
});
|
||||
|
||||
it("removes unknown keys and adds preview hint", () => {
|
||||
stripUnknownConfigKeysMock.mockReturnValueOnce({
|
||||
config: {},
|
||||
|
||||
@@ -13,6 +13,7 @@ export function applyLegacyCompatibilityStep(params: {
|
||||
state: DoctorConfigMutationState;
|
||||
issueLines: string[];
|
||||
changeLines: string[];
|
||||
partiallyValid?: boolean;
|
||||
} {
|
||||
if (params.snapshot.legacyIssues.length === 0) {
|
||||
return {
|
||||
@@ -23,7 +24,7 @@ export function applyLegacyCompatibilityStep(params: {
|
||||
}
|
||||
|
||||
const issueLines = formatConfigIssueLines(params.snapshot.legacyIssues, "-");
|
||||
const { config: migrated, changes } = migrateLegacyConfig(params.snapshot.parsed);
|
||||
const { config: migrated, changes, partiallyValid } = migrateLegacyConfig(params.snapshot.parsed);
|
||||
if (!migrated) {
|
||||
return {
|
||||
state: {
|
||||
@@ -45,6 +46,9 @@ export function applyLegacyCompatibilityStep(params: {
|
||||
state: {
|
||||
// Doctor should keep using the best-effort migrated shape in memory even
|
||||
// during preview mode; confirmation only controls whether we write it.
|
||||
// When partiallyValid, the migration succeeded but unrelated validation issues
|
||||
// remain — still commit the migration so doctor --fix always applies safe migrations
|
||||
// even when other problems prevent full validation from passing.
|
||||
cfg: migrated,
|
||||
candidate: migrated,
|
||||
// The read path can normalize legacy config into the snapshot before
|
||||
@@ -55,11 +59,12 @@ export function applyLegacyCompatibilityStep(params: {
|
||||
? params.state.fixHints
|
||||
: [
|
||||
...params.state.fixHints,
|
||||
`Run "${params.doctorFixCommand}" to migrate legacy config keys.`,
|
||||
`Run "${params.doctorFixCommand}" to ${partiallyValid ? "finish fixing" : "migrate"} legacy config keys.`,
|
||||
],
|
||||
},
|
||||
issueLines,
|
||||
changeLines: changes,
|
||||
partiallyValid: partiallyValid === true ? true : undefined,
|
||||
};
|
||||
}
|
||||
|
||||
|
||||
@@ -1,5 +1,6 @@
|
||||
import { describe, expect, it } from "vitest";
|
||||
import type { OpenClawConfig } from "../../../config/types.js";
|
||||
import { migrateLegacyConfig } from "./legacy-config-migrate.js";
|
||||
import { LEGACY_CONFIG_MIGRATIONS } from "./legacy-config-migrations.js";
|
||||
|
||||
function migrateLegacyConfigForTest(raw: unknown): {
|
||||
@@ -210,6 +211,38 @@ describe("legacy migrate mention routing", () => {
|
||||
});
|
||||
|
||||
describe("legacy migrate sandbox scope aliases", () => {
|
||||
it("returns migrated config when unrelated plugin validation issues remain (#76798)", () => {
|
||||
const res = migrateLegacyConfig({
|
||||
agents: {
|
||||
defaults: {
|
||||
model: { primary: "openai/gpt-5.5" },
|
||||
llm: { idleTimeoutSeconds: 120 },
|
||||
},
|
||||
},
|
||||
plugins: {
|
||||
entries: {
|
||||
brave: {
|
||||
enabled: true,
|
||||
config: { webSearch: { mode: "definitely-invalid" } },
|
||||
},
|
||||
},
|
||||
},
|
||||
tools: { web: { search: { provider: "brave" } } },
|
||||
});
|
||||
|
||||
expect(res.partiallyValid).toBe(true);
|
||||
expect(res.changes).toContain(
|
||||
"Removed agents.defaults.llm; model idle timeout now follows models.providers.<id>.timeoutSeconds.",
|
||||
);
|
||||
expect(res.changes).toContain(
|
||||
"Migration applied; other validation issues remain — run doctor to review.",
|
||||
);
|
||||
expect(res.config?.agents?.defaults).toEqual({
|
||||
model: { primary: "openai/gpt-5.5" },
|
||||
});
|
||||
expect(res.config?.tools?.web?.search?.provider).toBe("brave");
|
||||
});
|
||||
|
||||
it("removes legacy agents.defaults.llm timeout config", () => {
|
||||
const res = migrateLegacyConfigForTest({
|
||||
agents: {
|
||||
|
||||
@@ -5,6 +5,7 @@ import { applyLegacyDoctorMigrations } from "./legacy-config-compat.js";
|
||||
export function migrateLegacyConfig(raw: unknown): {
|
||||
config: OpenClawConfig | null;
|
||||
changes: string[];
|
||||
partiallyValid?: boolean;
|
||||
} {
|
||||
const { next, changes } = applyLegacyDoctorMigrations(raw);
|
||||
if (!next) {
|
||||
@@ -12,8 +13,8 @@ export function migrateLegacyConfig(raw: unknown): {
|
||||
}
|
||||
const validated = validateConfigObjectWithPlugins(next);
|
||||
if (!validated.ok) {
|
||||
changes.push("Migration applied, but config still invalid; fix remaining issues manually.");
|
||||
return { config: null, changes };
|
||||
changes.push("Migration applied; other validation issues remain — run doctor to review.");
|
||||
return { config: next as OpenClawConfig, changes, partiallyValid: true };
|
||||
}
|
||||
return { config: validated.config, changes };
|
||||
}
|
||||
|
||||
@@ -224,6 +224,12 @@ export type ConfigWriteOptions = {
|
||||
* Omitted means the observer should use its normal reload plan.
|
||||
*/
|
||||
afterWrite?: ConfigWriteAfterWrite;
|
||||
/**
|
||||
* Skip plugin-aware validation before writing. Use only for safe partial
|
||||
* migrations (e.g. legacy key removal) where the base schema is valid but
|
||||
* an unrelated plugin rule prevents the full write from succeeding.
|
||||
*/
|
||||
skipPluginValidation?: boolean;
|
||||
};
|
||||
|
||||
export type ReadConfigFileSnapshotForWriteResult = {
|
||||
@@ -2016,7 +2022,10 @@ export function createConfigIO(
|
||||
|
||||
persistCandidate = applyUnsetPathsForWrite(persistCandidate as OpenClawConfig, unsetPaths);
|
||||
|
||||
const validated = validateConfigObjectRawWithPlugins(persistCandidate, { env: deps.env });
|
||||
const validated = validateConfigObjectRawWithPlugins(persistCandidate, {
|
||||
env: deps.env,
|
||||
pluginValidation: options.skipPluginValidation ? "skip" : "full",
|
||||
});
|
||||
if (!validated.ok) {
|
||||
const issue = validated.issues[0];
|
||||
const pathLabel = issue?.path ? issue.path : "<root>";
|
||||
@@ -2421,7 +2430,7 @@ export async function writeConfigFile(
|
||||
cfg: OpenClawConfig,
|
||||
options: ConfigWriteOptions = {},
|
||||
): Promise<void> {
|
||||
const io = createConfigIO();
|
||||
const io = createConfigIO(options.skipPluginValidation ? { pluginValidation: "skip" } : {});
|
||||
let nextCfg = cfg;
|
||||
const runtimeConfigSnapshot = getRuntimeConfigSnapshotState();
|
||||
const runtimeConfigSourceSnapshot = getRuntimeConfigSourceSnapshotState();
|
||||
@@ -2442,6 +2451,7 @@ export async function writeConfigFile(
|
||||
allowConfigSizeDrop: options.allowConfigSizeDrop,
|
||||
skipRuntimeSnapshotRefresh: options.skipRuntimeSnapshotRefresh,
|
||||
skipOutputLogs: options.skipOutputLogs,
|
||||
skipPluginValidation: options.skipPluginValidation,
|
||||
});
|
||||
if (
|
||||
options.skipRuntimeSnapshotRefresh &&
|
||||
|
||||
@@ -1219,6 +1219,69 @@ describe("config io write", () => {
|
||||
});
|
||||
});
|
||||
|
||||
it("skipPluginValidation bypasses plugin schema rejection on writeConfigFile (#76800)", async () => {
|
||||
await withSuiteHome(async (home) => {
|
||||
const configPath = path.join(home, ".openclaw", "openclaw.json");
|
||||
const previousConfigPath = process.env.OPENCLAW_CONFIG_PATH;
|
||||
process.env.OPENCLAW_CONFIG_PATH = configPath;
|
||||
await fs.mkdir(path.dirname(configPath), { recursive: true });
|
||||
await fs.writeFile(configPath, "{}\n", "utf-8");
|
||||
mockLoadPluginManifestRegistry.mockReturnValue({
|
||||
diagnostics: [],
|
||||
plugins: [
|
||||
{
|
||||
id: "strict-plugin",
|
||||
origin: "bundled",
|
||||
channels: [],
|
||||
providers: [],
|
||||
cliBackends: [],
|
||||
skills: [],
|
||||
hooks: [],
|
||||
rootDir: "/tmp/openclaw-test-strict-plugin",
|
||||
source: "/tmp/openclaw-test-strict-plugin/index.ts",
|
||||
manifestPath: "/tmp/openclaw-test-strict-plugin/openclaw.plugin.json",
|
||||
configSchema: {
|
||||
type: "object",
|
||||
properties: { token: { type: "string" } },
|
||||
required: ["token"],
|
||||
additionalProperties: false,
|
||||
},
|
||||
},
|
||||
],
|
||||
} satisfies PluginManifestRegistry);
|
||||
|
||||
try {
|
||||
// Plugin is enabled but missing required "token" — validation fails without skip.
|
||||
const cfg: OpenClawConfig = {
|
||||
agents: { list: [{ id: "main", default: true }] },
|
||||
plugins: { entries: { "strict-plugin": { enabled: true } } },
|
||||
};
|
||||
|
||||
await expect(writeConfigFile(cfg, { skipPluginValidation: true })).resolves.not.toThrow();
|
||||
await expect(fs.readFile(configPath, "utf-8")).resolves.toContain('"strict-plugin"');
|
||||
|
||||
await expect(writeConfigFile(cfg, { skipPluginValidation: false })).rejects.toThrow(
|
||||
/Config validation failed/,
|
||||
);
|
||||
await expect(
|
||||
writeConfigFile({ agents: { list: "not-array" } } as unknown as OpenClawConfig, {
|
||||
skipPluginValidation: true,
|
||||
}),
|
||||
).rejects.toThrow(/Config validation failed/);
|
||||
} finally {
|
||||
mockLoadPluginManifestRegistry.mockReturnValue({
|
||||
diagnostics: [],
|
||||
plugins: [],
|
||||
} satisfies PluginManifestRegistry);
|
||||
if (previousConfigPath === undefined) {
|
||||
delete process.env.OPENCLAW_CONFIG_PATH;
|
||||
} else {
|
||||
process.env.OPENCLAW_CONFIG_PATH = previousConfigPath;
|
||||
}
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
it("preserves authored tilde paths when runtime-shaped writes hand back absolute paths", async () => {
|
||||
await withSuiteHome(async (home) => {
|
||||
const configPath = path.join(home, ".openclaw", "openclaw.json");
|
||||
|
||||
@@ -152,6 +152,12 @@ async function tryWriteSingleTopLevelIncludeMutation(params: {
|
||||
}
|
||||
const nextConfigRecord = nextConfig as Record<string, unknown>;
|
||||
|
||||
if (params.writeOptions?.skipPluginValidation) {
|
||||
// Skip the include fast path so the root writer handles the write with
|
||||
// plugin validation disabled end-to-end (including the post-write readback).
|
||||
return false;
|
||||
}
|
||||
|
||||
const validated = validateConfigObjectWithPlugins(nextConfig);
|
||||
if (!validated.ok) {
|
||||
throw createInvalidConfigError(
|
||||
|
||||
@@ -14,6 +14,7 @@ type DoctorConfigResult = {
|
||||
shouldWriteConfig?: boolean;
|
||||
sourceConfigValid?: boolean;
|
||||
sourceLastTouchedVersion?: string;
|
||||
skipPluginValidationOnWrite?: boolean;
|
||||
};
|
||||
|
||||
type DoctorHealthFlowContext = {
|
||||
@@ -566,6 +567,7 @@ async function runWriteConfigHealth(ctx: DoctorHealthFlowContext): Promise<void>
|
||||
afterWrite: { mode: "auto" },
|
||||
writeOptions: {
|
||||
allowConfigSizeDrop: ctx.configResult.shouldWriteConfig === true,
|
||||
skipPluginValidation: ctx.configResult.skipPluginValidationOnWrite === true,
|
||||
},
|
||||
});
|
||||
logConfigUpdated(ctx.runtime);
|
||||
|
||||
Reference in New Issue
Block a user