Skip to content

Commit f369939

Browse files
jalehmansteipete
jalehman
authored and
steipete
committed
fix: avoid plugin-local config recovery rollback (#71289)
1 parent 306c0f7 commit f369939

8 files changed

Lines changed: 305 additions & 4 deletions

‎src/config/config.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@ export {
2828
export type { ConfigWriteNotification } from "./io.js";
2929
export { ConfigMutationConflictError, mutateConfigFile, replaceConfigFile } from "./mutate.js";
3030
export * from "./paths.js";
31+
export * from "./recovery-policy.js";
3132
export * from "./runtime-overrides.js";
3233
export * from "./types.js";
3334
export {

‎src/config/io.observe-recovery.test.ts‎

Lines changed: 115 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -347,6 +347,121 @@ describe("config observe recovery", () => {
347347
});
348348
});
349349

350+
it("does not restore stale last-known-good for plugin schema evolution issues", async () => {
351+
await withSuiteHome(async (home) => {
352+
const { deps, configPath, warn } = makeDeps(home);
353+
const staleSnapshot = await makeSnapshot(configPath, {
354+
gateway: { mode: "local" },
355+
agents: { defaults: { model: "sonnet-4.6" } },
356+
plugins: {
357+
entries: {
358+
"lossless-claw": {
359+
enabled: true,
360+
config: { compactionMode: "legacy" },
361+
},
362+
},
363+
},
364+
});
365+
await expect(
366+
promoteConfigSnapshotToLastKnownGood({
367+
deps,
368+
snapshot: staleSnapshot,
369+
logger: deps.logger,
370+
}),
371+
).resolves.toBe(true);
372+
373+
const activeConfig = {
374+
gateway: { mode: "local" },
375+
agents: { defaults: { model: "gpt-5.4" } },
376+
plugins: {
377+
entries: {
378+
"lossless-claw": {
379+
enabled: true,
380+
config: { compactionMode: "adaptive", cacheAwareCompaction: true },
381+
},
382+
},
383+
},
384+
};
385+
const active = await writeConfigRaw(configPath, activeConfig);
386+
const restored = await recoverConfigFromLastKnownGood({
387+
deps,
388+
snapshot: {
389+
...staleSnapshot,
390+
raw: active.raw,
391+
parsed: active.parsed,
392+
valid: false,
393+
issues: [
394+
{
395+
path: "plugins.entries.lossless-claw.config.cacheAwareCompaction",
396+
message: "invalid config: must NOT have additional properties",
397+
},
398+
],
399+
},
400+
reason: "reload-invalid-config",
401+
});
402+
403+
expect(restored).toBe(false);
404+
await expect(fsp.readFile(configPath, "utf-8")).resolves.toBe(active.raw);
405+
expect(warn).toHaveBeenCalledWith(
406+
expect.stringContaining("Config last-known-good recovery skipped"),
407+
);
408+
});
409+
});
410+
411+
it("does not restore stale last-known-good for plugin minHostVersion skew issues", async () => {
412+
await withSuiteHome(async (home) => {
413+
const { deps, configPath } = makeDeps(home);
414+
const staleSnapshot = await makeSnapshot(configPath, {
415+
gateway: { mode: "local" },
416+
plugins: {
417+
entries: {
418+
feishu: { enabled: false },
419+
},
420+
},
421+
});
422+
await expect(
423+
promoteConfigSnapshotToLastKnownGood({
424+
deps,
425+
snapshot: staleSnapshot,
426+
logger: deps.logger,
427+
}),
428+
).resolves.toBe(true);
429+
430+
const activeConfig = {
431+
gateway: { mode: "local" },
432+
agents: { defaults: { model: "gpt-5.4" } },
433+
plugins: {
434+
entries: {
435+
feishu: { enabled: true, config: { appId: "feishu-app" } },
436+
whatsapp: { enabled: true, config: { account: "primary" } },
437+
},
438+
},
439+
};
440+
const active = await writeConfigRaw(configPath, activeConfig);
441+
const restored = await recoverConfigFromLastKnownGood({
442+
deps,
443+
snapshot: {
444+
...staleSnapshot,
445+
raw: active.raw,
446+
parsed: active.parsed,
447+
valid: false,
448+
issues: [
449+
{
450+
path: "plugins.entries.feishu",
451+
message:
452+
"plugin feishu: plugin requires OpenClaw >=2026.4.23, but this host is 2026.4.22; skipping load",
453+
},
454+
],
455+
},
456+
reason: "reload-invalid-config",
457+
});
458+
459+
expect(restored).toBe(false);
460+
await expect(fsp.readFile(configPath, "utf-8")).resolves.toBe(active.raw);
461+
expect(JSON5.parse(active.raw)).toEqual(activeConfig);
462+
});
463+
});
464+
350465
it("refuses to promote redacted secret placeholders", async () => {
351466
await withSuiteHome(async (home) => {
352467
const warn = vi.fn();

‎src/config/io.observe-recovery.ts‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,10 @@ import {
77
type ConfigObserveAuditRecord,
88
} from "./io.audit.js";
99
import { resolveStateDir } from "./paths.js";
10+
import {
11+
isPluginLocalInvalidConfigSnapshot,
12+
shouldAttemptLastKnownGoodRecovery,
13+
} from "./recovery-policy.js";
1014
import type { ConfigFileSnapshot } from "./types.openclaw.js";
1115

1216
export type ObserveRecoveryDeps = {
@@ -1014,6 +1018,14 @@ export async function recoverConfigFromLastKnownGood(params: {
10141018
if (!snapshot.exists || typeof snapshot.raw !== "string") {
10151019
return false;
10161020
}
1021+
if (!shouldAttemptLastKnownGoodRecovery(snapshot)) {
1022+
if (isPluginLocalInvalidConfigSnapshot(snapshot)) {
1023+
deps.logger.warn(
1024+
`Config last-known-good recovery skipped: invalidity is scoped to plugin entries (${params.reason})`,
1025+
);
1026+
}
1027+
return false;
1028+
}
10171029
const healthState = await readConfigHealthState(deps);
10181030
const entry = getConfigHealthEntry(healthState, snapshot.path);
10191031
const promoted = entry.lastPromotedGood;

‎src/config/recovery-policy.ts‎

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
import type { ConfigFileSnapshot, ConfigValidationIssue } from "./types.openclaw.js";
2+
3+
const PLUGIN_ENTRY_PATH_PREFIX = "plugins.entries.";
4+
5+
function isPluginEntryIssue(issue: ConfigValidationIssue): boolean {
6+
const path = issue.path.trim();
7+
if (!path.startsWith(PLUGIN_ENTRY_PATH_PREFIX)) {
8+
return false;
9+
}
10+
return path.slice(PLUGIN_ENTRY_PATH_PREFIX.length).trim().length > 0;
11+
}
12+
13+
/**
14+
* Returns true when an invalid config snapshot is scoped entirely to plugin entries.
15+
*/
16+
export function isPluginLocalInvalidConfigSnapshot(
17+
snapshot: Pick<ConfigFileSnapshot, "valid" | "issues" | "legacyIssues">,
18+
): boolean {
19+
if (snapshot.valid || snapshot.legacyIssues.length > 0 || snapshot.issues.length === 0) {
20+
return false;
21+
}
22+
return snapshot.issues.every(isPluginEntryIssue);
23+
}
24+
25+
/**
26+
* Decides whether whole-file last-known-good recovery is safe for a snapshot.
27+
*/
28+
export function shouldAttemptLastKnownGoodRecovery(
29+
snapshot: Pick<ConfigFileSnapshot, "valid" | "issues" | "legacyIssues">,
30+
): boolean {
31+
if (snapshot.valid) {
32+
return false;
33+
}
34+
return !isPluginLocalInvalidConfigSnapshot(snapshot);
35+
}

‎src/gateway/config-reload.test.ts‎

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -727,6 +727,62 @@ describe("startGatewayConfigReloader", () => {
727727
await reloader.stop();
728728
});
729729

730+
it("skips last-known-good recovery for plugin-local invalid reloads", async () => {
731+
const activeConfig: OpenClawConfig = {
732+
gateway: { reload: { debounceMs: 0 } },
733+
agents: { defaults: { model: "gpt-5.4" } },
734+
plugins: {
735+
entries: {
736+
"lossless-claw": {
737+
enabled: true,
738+
config: { compactionMode: "adaptive", cacheAwareCompaction: true },
739+
},
740+
},
741+
},
742+
};
743+
const invalidSnapshot = makeSnapshot({
744+
valid: false,
745+
raw: `${JSON.stringify(activeConfig, null, 2)}\n`,
746+
parsed: activeConfig,
747+
sourceConfig: activeConfig,
748+
runtimeConfig: activeConfig,
749+
config: activeConfig,
750+
issues: [
751+
{
752+
path: "plugins.entries.lossless-claw.config.cacheAwareCompaction",
753+
message: "invalid config: must NOT have additional properties",
754+
},
755+
],
756+
hash: "plugin-skew-1",
757+
});
758+
const readSnapshot = vi
759+
.fn<() => Promise<ConfigFileSnapshot>>()
760+
.mockResolvedValueOnce(invalidSnapshot);
761+
const recoverSnapshot = vi.fn(async () => true);
762+
const promoteSnapshot = vi.fn(async () => true);
763+
const { watcher, onHotReload, onRestart, log, reloader } = createReloaderHarness(readSnapshot, {
764+
recoverSnapshot,
765+
promoteSnapshot,
766+
});
767+
768+
watcher.emit("change");
769+
await vi.runAllTimersAsync();
770+
771+
expect(recoverSnapshot).not.toHaveBeenCalled();
772+
expect(readSnapshot).toHaveBeenCalledTimes(1);
773+
expect(onHotReload).not.toHaveBeenCalled();
774+
expect(onRestart).not.toHaveBeenCalled();
775+
expect(promoteSnapshot).not.toHaveBeenCalled();
776+
expect(log.warn).toHaveBeenCalledWith(
777+
"config reload recovery skipped after invalid-config: invalidity is scoped to plugin entries",
778+
);
779+
expect(log.warn).toHaveBeenCalledWith(
780+
expect.stringContaining("config reload skipped (invalid config):"),
781+
);
782+
783+
await reloader.stop();
784+
});
785+
730786
it("promotes valid external config edits after they are accepted", async () => {
731787
const acceptedSnapshot = makeSnapshot({
732788
config: {

‎src/gateway/config-reload.ts‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import type {
77
ConfigWriteNotification,
88
GatewayReloadMode,
99
} from "../config/config.js";
10+
import { shouldAttemptLastKnownGoodRecovery } from "../config/config.js";
1011
import { formatConfigIssueLines } from "../config/issue-format.js";
1112
import { isPlainObject } from "../utils.js";
1213
import {
@@ -222,6 +223,12 @@ export function startGatewayConfigReloader(opts: {
222223
if (!opts.recoverSnapshot) {
223224
return null;
224225
}
226+
if (!shouldAttemptLastKnownGoodRecovery(snapshot)) {
227+
opts.log.warn(
228+
`config reload recovery skipped after ${reason}: invalidity is scoped to plugin entries`,
229+
);
230+
return null;
231+
}
225232
const recovered = await opts.recoverSnapshot(snapshot, reason);
226233
if (!recovered) {
227234
return null;

‎src/gateway/server-startup-config.recovery.test.ts‎

Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,16 @@ vi.mock("../config/config.js", () => ({
88
readConfigFileSnapshot: vi.fn(),
99
recoverConfigFromLastKnownGood: vi.fn(),
1010
recoverConfigFromJsonRootSuffix: vi.fn(),
11+
shouldAttemptLastKnownGoodRecovery: vi.fn((snapshot: ConfigFileSnapshot) => {
12+
if (snapshot.valid) {
13+
return false;
14+
}
15+
return !(
16+
snapshot.legacyIssues.length === 0 &&
17+
snapshot.issues.length > 0 &&
18+
snapshot.issues.every((issue) => issue.path.startsWith("plugins.entries."))
19+
);
20+
}),
1121
writeConfigFile: vi.fn(),
1222
}));
1323

@@ -110,6 +120,62 @@ describe("gateway startup config recovery", () => {
110120
expect(recoveryNotice.enqueueConfigRecoveryNotice).not.toHaveBeenCalled();
111121
});
112122

123+
it("does not restore last-known-good for plugin-local startup invalidity", async () => {
124+
const invalidSnapshot = buildTestConfigSnapshot({
125+
path: configPath,
126+
exists: true,
127+
raw: `${JSON.stringify({
128+
gateway: { mode: "local" },
129+
plugins: {
130+
entries: {
131+
feishu: { enabled: true },
132+
},
133+
},
134+
})}\n`,
135+
parsed: {
136+
gateway: { mode: "local" },
137+
plugins: {
138+
entries: {
139+
feishu: { enabled: true },
140+
},
141+
},
142+
},
143+
valid: false,
144+
config: {
145+
gateway: { mode: "local" },
146+
plugins: {
147+
entries: {
148+
feishu: { enabled: true },
149+
},
150+
},
151+
} as OpenClawConfig,
152+
issues: [
153+
{
154+
path: "plugins.entries.feishu",
155+
message:
156+
"plugin feishu: plugin requires OpenClaw >=2026.4.23, but this host is 2026.4.22; skipping load",
157+
},
158+
],
159+
legacyIssues: [],
160+
});
161+
vi.mocked(configIo.readConfigFileSnapshot).mockResolvedValueOnce(invalidSnapshot);
162+
const log = { info: vi.fn(), warn: vi.fn() };
163+
164+
await expect(
165+
loadGatewayStartupConfigSnapshot({
166+
minimalTestGateway: true,
167+
log,
168+
}),
169+
).rejects.toThrow(`Invalid config at ${configPath}.`);
170+
171+
expect(configIo.recoverConfigFromLastKnownGood).not.toHaveBeenCalled();
172+
expect(configIo.recoverConfigFromJsonRootSuffix).toHaveBeenCalledWith(invalidSnapshot);
173+
expect(log.warn).toHaveBeenCalledWith(
174+
`gateway: last-known-good recovery skipped for plugin-local config invalidity: ${configPath}`,
175+
);
176+
expect(recoveryNotice.enqueueConfigRecoveryNotice).not.toHaveBeenCalled();
177+
});
178+
113179
it("strips a valid JSON suffix when last-known-good recovery is unavailable", async () => {
114180
const invalidSnapshot = buildSnapshot({
115181
valid: false,

‎src/gateway/server-startup-config.ts‎

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import {
99
readConfigFileSnapshot,
1010
recoverConfigFromLastKnownGood,
1111
recoverConfigFromJsonRootSuffix,
12+
shouldAttemptLastKnownGoodRecovery,
1213
writeConfigFile,
1314
} from "../config/config.js";
1415
import { formatConfigIssueLines } from "../config/issue-format.js";
@@ -70,10 +71,18 @@ export async function loadGatewayStartupConfigSnapshot(params: {
7071
}
7172
if (configSnapshot.exists) {
7273
if (!configSnapshot.valid) {
73-
const recovered = await recoverConfigFromLastKnownGood({
74-
snapshot: configSnapshot,
75-
reason: "startup-invalid-config",
76-
});
74+
const canRecoverFromLastKnownGood = shouldAttemptLastKnownGoodRecovery(configSnapshot);
75+
const recovered = canRecoverFromLastKnownGood
76+
? await recoverConfigFromLastKnownGood({
77+
snapshot: configSnapshot,
78+
reason: "startup-invalid-config",
79+
})
80+
: false;
81+
if (!canRecoverFromLastKnownGood) {
82+
params.log.warn(
83+
`gateway: last-known-good recovery skipped for plugin-local config invalidity: ${configSnapshot.path}`,
84+
);
85+
}
7786
if (recovered) {
7887
wroteConfig = true;
7988
params.log.warn(

0 commit comments

Comments
 (0)