mirror of
https://github.com/openclaw/openclaw.git
synced 2026-05-18 16:54:46 +00:00
fix(config): rotate clobber snapshots at cap
This commit is contained in:
committed by
Peter Steinberger
parent
de18f77737
commit
57e699a09a
@@ -370,6 +370,7 @@ Look for:
|
||||
- `Config write rejected: ...`
|
||||
- A timestamped `openclaw.json.rejected.*` file beside the active config
|
||||
- A timestamped `openclaw.json.clobbered.*` file if `doctor --fix` repaired a broken direct edit
|
||||
- OpenClaw keeps the latest 32 `.clobbered.*` files for each config path and rotates older ones
|
||||
|
||||
<AccordionGroup>
|
||||
<Accordion title="What happened">
|
||||
@@ -378,6 +379,7 @@ Look for:
|
||||
- Hot reload skips invalid external edits and keeps the current runtime config active.
|
||||
- OpenClaw-owned writes reject invalid/destructive payloads before commit and save `.rejected.*`.
|
||||
- `openclaw doctor --fix` owns repair. It can remove non-JSON prefixes or restore the last-known-good copy while preserving the rejected payload as `.clobbered.*`.
|
||||
- When many repairs happen for one config path, OpenClaw rotates older `.clobbered.*` files so the newest repaired payload is still available.
|
||||
|
||||
</Accordion>
|
||||
<Accordion title="Inspect and repair">
|
||||
|
||||
@@ -35,7 +35,15 @@ describe("config clobber snapshots", () => {
|
||||
return entries.filter((entry) => entry.startsWith(prefix));
|
||||
}
|
||||
|
||||
it("keeps concurrent async snapshots under the per-path cap", async () => {
|
||||
async function readClobberFileContents(configPath: string): Promise<string[]> {
|
||||
const dir = path.dirname(configPath);
|
||||
const clobberFiles = await listClobberFiles(configPath);
|
||||
return await Promise.all(
|
||||
clobberFiles.map((entry) => fsp.readFile(path.join(dir, entry), "utf-8")),
|
||||
);
|
||||
}
|
||||
|
||||
it("keeps concurrent async snapshots under the per-path cap by rotating oldest files", async () => {
|
||||
await withCase(async (configPath) => {
|
||||
const warn = vi.fn();
|
||||
const observedAt = "2026-05-03T00:00:00.000Z";
|
||||
@@ -61,7 +69,35 @@ describe("config clobber snapshots", () => {
|
||||
});
|
||||
});
|
||||
|
||||
it("keeps sync snapshots under the per-path cap and warns once", async () => {
|
||||
it("rotates async snapshots so the latest clobbered config is preserved", async () => {
|
||||
await withCase(async (configPath) => {
|
||||
const warn = vi.fn();
|
||||
|
||||
for (let index = 0; index < CONFIG_CLOBBER_SNAPSHOT_LIMIT + 3; index++) {
|
||||
await persistBoundedClobberedConfigSnapshot({
|
||||
deps: { fs, logger: { warn } },
|
||||
configPath,
|
||||
raw: `polluted-${index}\n`,
|
||||
observedAt: `2026-05-03T00:00:${String(index).padStart(2, "0")}.000Z`,
|
||||
});
|
||||
}
|
||||
|
||||
const clobberFiles = await listClobberFiles(configPath);
|
||||
expect(clobberFiles).toHaveLength(CONFIG_CLOBBER_SNAPSHOT_LIMIT);
|
||||
const contents = await readClobberFileContents(configPath);
|
||||
expect(contents).not.toContain("polluted-0\n");
|
||||
expect(contents).not.toContain("polluted-1\n");
|
||||
expect(contents).not.toContain("polluted-2\n");
|
||||
expect(contents).toContain(`polluted-${CONFIG_CLOBBER_SNAPSHOT_LIMIT + 2}\n`);
|
||||
const capWarnings = warn.mock.calls.filter(
|
||||
([message]) =>
|
||||
typeof message === "string" && message.includes("Config clobber snapshot cap reached"),
|
||||
);
|
||||
expect(capWarnings).toHaveLength(1);
|
||||
});
|
||||
});
|
||||
|
||||
it("rotates sync snapshots so the latest clobbered config is preserved", async () => {
|
||||
await withCase(async (configPath) => {
|
||||
const warn = vi.fn();
|
||||
|
||||
@@ -76,6 +112,11 @@ describe("config clobber snapshots", () => {
|
||||
|
||||
const clobberFiles = await listClobberFiles(configPath);
|
||||
expect(clobberFiles).toHaveLength(CONFIG_CLOBBER_SNAPSHOT_LIMIT);
|
||||
const contents = await readClobberFileContents(configPath);
|
||||
expect(contents).not.toContain("polluted-0\n");
|
||||
expect(contents).not.toContain("polluted-1\n");
|
||||
expect(contents).not.toContain("polluted-2\n");
|
||||
expect(contents).toContain(`polluted-${CONFIG_CLOBBER_SNAPSHOT_LIMIT + 2}\n`);
|
||||
const capWarnings = warn.mock.calls.filter(
|
||||
([message]) =>
|
||||
typeof message === "string" && message.includes("Config clobber snapshot cap reached"),
|
||||
|
||||
@@ -13,6 +13,7 @@ type ConfigClobberSnapshotFs = {
|
||||
readdir(path: string): Promise<string[]>;
|
||||
rmdir(path: string): Promise<unknown>;
|
||||
stat(path: string): Promise<{ mtimeMs?: number } | null>;
|
||||
unlink(path: string): Promise<unknown>;
|
||||
writeFile(
|
||||
path: string,
|
||||
data: string,
|
||||
@@ -23,6 +24,7 @@ type ConfigClobberSnapshotFs = {
|
||||
readdirSync(path: string): string[];
|
||||
rmdirSync(path: string): unknown;
|
||||
statSync(path: string, options?: { throwIfNoEntry?: boolean }): { mtimeMs?: number } | null;
|
||||
unlinkSync(path: string): unknown;
|
||||
writeFileSync(
|
||||
path: string,
|
||||
data: string,
|
||||
@@ -117,28 +119,59 @@ function acquireClobberLockSync(deps: ConfigClobberSnapshotDeps, lockPath: strin
|
||||
return false;
|
||||
}
|
||||
|
||||
async function countClobberedSiblings(
|
||||
type ClobberedSiblingSnapshot = {
|
||||
name: string;
|
||||
path: string;
|
||||
mtimeMs: number;
|
||||
};
|
||||
|
||||
function compareClobberedSiblings(
|
||||
left: ClobberedSiblingSnapshot,
|
||||
right: ClobberedSiblingSnapshot,
|
||||
): number {
|
||||
return left.mtimeMs - right.mtimeMs || left.name.localeCompare(right.name);
|
||||
}
|
||||
|
||||
async function listClobberedSiblings(
|
||||
deps: ConfigClobberSnapshotDeps,
|
||||
dir: string,
|
||||
prefix: string,
|
||||
): Promise<number> {
|
||||
): Promise<ClobberedSiblingSnapshot[]> {
|
||||
try {
|
||||
const entries = await deps.fs.promises.readdir(dir);
|
||||
return entries.filter((entry) => entry.startsWith(prefix)).length;
|
||||
const snapshots: ClobberedSiblingSnapshot[] = [];
|
||||
for (const entry of entries) {
|
||||
if (!entry.startsWith(prefix)) {
|
||||
continue;
|
||||
}
|
||||
const snapshotPath = path.join(dir, entry);
|
||||
const stat = await deps.fs.promises.stat(snapshotPath).catch(() => null);
|
||||
snapshots.push({ name: entry, path: snapshotPath, mtimeMs: stat?.mtimeMs ?? 0 });
|
||||
}
|
||||
return snapshots.toSorted(compareClobberedSiblings);
|
||||
} catch {
|
||||
return 0;
|
||||
return [];
|
||||
}
|
||||
}
|
||||
|
||||
function countClobberedSiblingsSync(
|
||||
function listClobberedSiblingsSync(
|
||||
deps: ConfigClobberSnapshotDeps,
|
||||
dir: string,
|
||||
prefix: string,
|
||||
): number {
|
||||
): ClobberedSiblingSnapshot[] {
|
||||
try {
|
||||
return deps.fs.readdirSync(dir).filter((entry) => entry.startsWith(prefix)).length;
|
||||
const snapshots: ClobberedSiblingSnapshot[] = [];
|
||||
for (const entry of deps.fs.readdirSync(dir)) {
|
||||
if (!entry.startsWith(prefix)) {
|
||||
continue;
|
||||
}
|
||||
const snapshotPath = path.join(dir, entry);
|
||||
const stat = deps.fs.statSync(snapshotPath, { throwIfNoEntry: false });
|
||||
snapshots.push({ name: entry, path: snapshotPath, mtimeMs: stat?.mtimeMs ?? 0 });
|
||||
}
|
||||
return snapshots.toSorted(compareClobberedSiblings);
|
||||
} catch {
|
||||
return 0;
|
||||
return [];
|
||||
}
|
||||
}
|
||||
|
||||
@@ -152,10 +185,44 @@ function warnClobberCapReached(
|
||||
}
|
||||
clobberCapWarnedPaths.add(configPath);
|
||||
deps.logger.warn(
|
||||
`Config clobber snapshot cap reached for ${configPath}: ${existing} existing .clobbered.* files; skipping additional forensic snapshots.`,
|
||||
`Config clobber snapshot cap reached for ${configPath}: ${existing} existing .clobbered.* files; rotating oldest snapshots to preserve the latest forensic copy.`,
|
||||
);
|
||||
}
|
||||
|
||||
async function rotateOldestClobberedSiblings(
|
||||
deps: ConfigClobberSnapshotDeps,
|
||||
snapshots: ClobberedSiblingSnapshot[],
|
||||
): Promise<boolean> {
|
||||
const deleteCount = Math.max(0, snapshots.length - CONFIG_CLOBBER_SNAPSHOT_LIMIT + 1);
|
||||
for (const snapshot of snapshots.slice(0, deleteCount)) {
|
||||
try {
|
||||
await deps.fs.promises.unlink(snapshot.path);
|
||||
} catch (error) {
|
||||
if (!isFsErrorCode(error, "ENOENT")) {
|
||||
return false;
|
||||
}
|
||||
}
|
||||
}
|
||||
return true;
|
||||
}
|
||||
|
||||
function rotateOldestClobberedSiblingsSync(
|
||||
deps: ConfigClobberSnapshotDeps,
|
||||
snapshots: ClobberedSiblingSnapshot[],
|
||||
): boolean {
|
||||
const deleteCount = Math.max(0, snapshots.length - CONFIG_CLOBBER_SNAPSHOT_LIMIT + 1);
|
||||
for (const snapshot of snapshots.slice(0, deleteCount)) {
|
||||
try {
|
||||
deps.fs.unlinkSync(snapshot.path);
|
||||
} catch (error) {
|
||||
if (!isFsErrorCode(error, "ENOENT")) {
|
||||
return false;
|
||||
}
|
||||
}
|
||||
}
|
||||
return true;
|
||||
}
|
||||
|
||||
function buildClobberedTargetPath(configPath: string, observedAt: string, attempt: number): string {
|
||||
const basePath = `${configPath}.clobbered.${formatConfigArtifactTimestamp(observedAt)}`;
|
||||
return attempt === 0 ? basePath : `${basePath}-${String(attempt).padStart(2, "0")}`;
|
||||
@@ -173,10 +240,13 @@ export async function persistBoundedClobberedConfigSnapshot(params: {
|
||||
return null;
|
||||
}
|
||||
try {
|
||||
const existing = await countClobberedSiblings(params.deps, paths.dir, paths.prefix);
|
||||
if (existing >= CONFIG_CLOBBER_SNAPSHOT_LIMIT) {
|
||||
warnClobberCapReached(params.deps, params.configPath, existing);
|
||||
return null;
|
||||
const existing = await listClobberedSiblings(params.deps, paths.dir, paths.prefix);
|
||||
if (existing.length >= CONFIG_CLOBBER_SNAPSHOT_LIMIT) {
|
||||
warnClobberCapReached(params.deps, params.configPath, existing.length);
|
||||
const rotated = await rotateOldestClobberedSiblings(params.deps, existing);
|
||||
if (!rotated) {
|
||||
return null;
|
||||
}
|
||||
}
|
||||
for (let attempt = 0; attempt < CONFIG_CLOBBER_SNAPSHOT_LIMIT; attempt++) {
|
||||
const targetPath = buildClobberedTargetPath(params.configPath, params.observedAt, attempt);
|
||||
@@ -210,10 +280,12 @@ export function persistBoundedClobberedConfigSnapshotSync(params: {
|
||||
return null;
|
||||
}
|
||||
try {
|
||||
const existing = countClobberedSiblingsSync(params.deps, paths.dir, paths.prefix);
|
||||
if (existing >= CONFIG_CLOBBER_SNAPSHOT_LIMIT) {
|
||||
warnClobberCapReached(params.deps, params.configPath, existing);
|
||||
return null;
|
||||
const existing = listClobberedSiblingsSync(params.deps, paths.dir, paths.prefix);
|
||||
if (existing.length >= CONFIG_CLOBBER_SNAPSHOT_LIMIT) {
|
||||
warnClobberCapReached(params.deps, params.configPath, existing.length);
|
||||
if (!rotateOldestClobberedSiblingsSync(params.deps, existing)) {
|
||||
return null;
|
||||
}
|
||||
}
|
||||
for (let attempt = 0; attempt < CONFIG_CLOBBER_SNAPSHOT_LIMIT; attempt++) {
|
||||
const targetPath = buildClobberedTargetPath(params.configPath, params.observedAt, attempt);
|
||||
|
||||
@@ -43,6 +43,7 @@ export type ObserveRecoveryDeps = {
|
||||
mkdir(path: string, options?: { recursive?: boolean; mode?: number }): Promise<unknown>;
|
||||
readdir(path: string): Promise<string[]>;
|
||||
rmdir(path: string): Promise<unknown>;
|
||||
unlink(path: string): Promise<unknown>;
|
||||
appendFile(
|
||||
path: string,
|
||||
data: string,
|
||||
@@ -73,6 +74,7 @@ export type ObserveRecoveryDeps = {
|
||||
mkdirSync(path: string, options?: { recursive?: boolean; mode?: number }): unknown;
|
||||
readdirSync(path: string): string[];
|
||||
rmdirSync(path: string): unknown;
|
||||
unlinkSync(path: string): unknown;
|
||||
appendFileSync(
|
||||
path: string,
|
||||
data: string,
|
||||
|
||||
@@ -780,7 +780,7 @@ describe("config io write", () => {
|
||||
});
|
||||
});
|
||||
|
||||
it("caps repeated prefix-recovery clobber snapshots for doctor-style repair loops", async () => {
|
||||
it("rotates repeated prefix-recovery clobber snapshots for doctor-style repair loops", async () => {
|
||||
await withSuiteHome(async (home) => {
|
||||
const configPath = path.join(home, ".openclaw", "openclaw.json");
|
||||
const cleanConfig = {
|
||||
@@ -806,6 +806,13 @@ describe("config io write", () => {
|
||||
const entries = await fs.readdir(path.dirname(configPath));
|
||||
const clobbered = entries.filter((entry) => entry.includes(".clobbered."));
|
||||
expect(clobbered).toHaveLength(CONFIG_CLOBBER_SNAPSHOT_LIMIT);
|
||||
const clobberedContents = await Promise.all(
|
||||
clobbered.map((entry) => fs.readFile(path.join(path.dirname(configPath), entry), "utf-8")),
|
||||
);
|
||||
expect(clobberedContents).not.toContain(`Found and updated: False 0\n${cleanRaw}`);
|
||||
expect(clobberedContents).toContain(
|
||||
`Found and updated: False ${CONFIG_CLOBBER_SNAPSHOT_LIMIT + 3}\n${cleanRaw}`,
|
||||
);
|
||||
const capWarnings = warn.mock.calls.filter(
|
||||
([message]) =>
|
||||
typeof message === "string" && message.includes("Config clobber snapshot cap reached"),
|
||||
|
||||
Reference in New Issue
Block a user