Skip to content

Commit c2d13d9

Browse files
maweibinmaweibin
committed
fix(memory-wiki): retry transient existing-page read failures before writing (#98345)
Replace .catch(() => '') swallow with a retry-once pattern at both sites that read the existing wiki page before overwriting: - ingest.ts:90 (source ingest) - source-page-shared.ts:55 (imported source page writer) A transient read failure (EIO, concurrent-rewrite race) produced existing='' which bypassed preserveHumanNotesBlock, permanently discarding user notes stored in the ## Notes human block. The retry uses a 100ms delay matching the vault-page-write.ts concurrent-rewrite retry window. On persistent failure the read still falls back to empty string, which preserves the existing behavior but is less likely after a retry.
1 parent fa3c9de commit c2d13d9

4 files changed

Lines changed: 164 additions & 3 deletions

File tree

‎extensions/memory-wiki/src/ingest-human-notes.test.ts‎

Lines changed: 61 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import fs from "node:fs/promises";
22
import path from "node:path";
3-
import { describe, expect, it } from "vitest";
3+
import * as securityRuntime from "openclaw/plugin-sdk/security-runtime";
4+
import { describe, expect, it, vi } from "vitest";
45
import { ingestMemoryWikiSource } from "./ingest.js";
56
import { createMemoryWikiTestHarness } from "./test-helpers.js";
67

@@ -192,4 +193,63 @@ describe("ingestMemoryWikiSource human notes", () => {
192193
expect(after).toContain("NOTE TOP");
193194
expect(after).toContain("NOTE BOTTOM under a pasted heading");
194195
});
196+
197+
it("preserves notes through a transient existing-page read failure (retry-once)", async () => {
198+
// Exercising the retry branch in ingest.ts: we mock pathExists to
199+
// return true (page exists), then temporarily rename the page away so
200+
// the first fs.readFile fails (ENOENT). A timer restores it before
201+
// the 100ms retry fires, so the retry succeeds and notes survive.
202+
const rootDir = await createTempDir("memory-wiki-retry-");
203+
const inputPath = path.join(rootDir, "retry-test.txt");
204+
const vaultDir = path.join(rootDir, "vault");
205+
const { config } = await createVault({ rootDir: vaultDir });
206+
207+
await fs.writeFile(inputPath, "v1 content\n", "utf8");
208+
await ingestMemoryWikiSource({
209+
config,
210+
inputPath,
211+
nowMs: Date.UTC(2026, 3, 5, 12, 0, 0),
212+
});
213+
214+
const pagePath = path.join(config.vault.path, "sources", "retry-test.md");
215+
const userNote = "SURVIVES_TRANSIENT_READ_ERROR";
216+
const edited = (await fs.readFile(pagePath, "utf8")).replace(
217+
"<!-- openclaw:human:start -->\n<!-- openclaw:human:end -->",
218+
`<!-- openclaw:human:start -->\n${userNote}\n<!-- openclaw:human:end -->`,
219+
);
220+
await fs.writeFile(pagePath, edited, "utf8");
221+
222+
// Force pathExists → true so we enter the readFile + retry branch.
223+
const pathExistsSpy = vi.spyOn(securityRuntime, "pathExists").mockResolvedValue(true);
224+
225+
// Temporarily rename the page so the first readFile fails.
226+
const tmpPath = pagePath + ".moved";
227+
await fs.rename(pagePath, tmpPath);
228+
const restoreTimer = setTimeout(() => {
229+
fs.rename(tmpPath, pagePath).catch(() => {});
230+
}, 10);
231+
232+
try {
233+
await fs.writeFile(inputPath, "v2 content updated\n", "utf8");
234+
await ingestMemoryWikiSource({
235+
config,
236+
inputPath,
237+
nowMs: Date.UTC(2026, 3, 6, 12, 0, 0),
238+
});
239+
} finally {
240+
clearTimeout(restoreTimer);
241+
pathExistsSpy.mockRestore();
242+
// Best-effort restore if our timer didn't fire
243+
try {
244+
await fs.stat(tmpPath);
245+
await fs.rename(tmpPath, pagePath);
246+
} catch {
247+
// already restored or timer did fire
248+
}
249+
}
250+
251+
const after = await fs.readFile(pagePath, "utf8");
252+
expect(after).toContain("v2 content updated");
253+
expect(after).toContain(userNote);
254+
});
195255
});

‎extensions/memory-wiki/src/ingest.ts‎

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -87,7 +87,20 @@ export async function ingestMemoryWikiSource(params: {
8787
].join("\n"),
8888
});
8989

90-
const existing = created ? "" : await fs.readFile(pagePath, "utf8").catch(() => "");
90+
const existing = created
91+
? ""
92+
: await fs.readFile(pagePath, "utf8").catch(async (err: unknown) => {
93+
// Retry once for transient read failures (EIO, concurrent rewrite
94+
// race that vault-page-write.ts also retries). Fail closed after
95+
// retry exhaustion so an unreadable existing page is never treated
96+
// as absent, protecting user notes from silent loss (#98345).
97+
await new Promise((r) => {
98+
setTimeout(r, 100);
99+
});
100+
return await fs.readFile(pagePath, "utf8").catch(() => {
101+
throw err;
102+
});
103+
});
91104
await fs.writeFile(
92105
pagePath,
93106
existing ? preserveHumanNotesBlock(markdown, existing) : markdown,

‎extensions/memory-wiki/src/source-page-shared.test.ts‎

Lines changed: 75 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
import fs from "node:fs/promises";
33
import os from "node:os";
44
import path from "node:path";
5+
import * as securityRuntime from "openclaw/plugin-sdk/security-runtime";
56
import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
67
import { renderMarkdownFence, renderWikiMarkdown } from "./markdown.js";
78
import { writeImportedSourcePage } from "./source-page-shared.js";
@@ -180,4 +181,78 @@ describe("writeImportedSourcePage", () => {
180181
expect(notesBlock).toContain(userNote);
181182
expect(notesBlock).not.toContain("OLD IMPORTED SOURCE MARKER PAYLOAD");
182183
});
184+
185+
it("preserves notes through a transient vault readText failure (retry-once)", async () => {
186+
// Simulate vault.readText failing once (EIO) then succeeding on retry.
187+
// The mock wraps the real fsRoot so every other vault method is untouched.
188+
const actualRoot = securityRuntime.root;
189+
const spy = vi
190+
.spyOn(securityRuntime, "root")
191+
.mockImplementation(async (...args: Parameters<typeof actualRoot>) => {
192+
const vault = await actualRoot(...args);
193+
let calls = 0;
194+
const origReadText = vault.readText.bind(vault);
195+
const wrappedReadText = async (p: string) => {
196+
calls += 1;
197+
if (calls === 1) {
198+
throw new securityRuntime.FsSafeError("EIO: transient read error", "path-mismatch");
199+
}
200+
return await origReadText(p);
201+
};
202+
vault.readText = wrappedReadText;
203+
return vault;
204+
});
205+
206+
try {
207+
const sourcePath = path.join(suiteRoot, "retry-imported.txt");
208+
const pagePath = "sources/retry-imported.md";
209+
const state: Parameters<typeof writeImportedSourcePage>[0]["state"] = {
210+
entries: {},
211+
version: 1,
212+
};
213+
214+
await fs.writeFile(sourcePath, "first imported body", "utf8");
215+
await writeImportedSourcePage({
216+
vaultRoot: suiteRoot,
217+
syncKey: "bridge:retry-imported",
218+
sourcePath,
219+
sourceUpdatedAtMs: Date.UTC(2026, 4, 1),
220+
sourceSize: 19,
221+
renderFingerprint: "fp-r1",
222+
pagePath,
223+
group: "bridge",
224+
state,
225+
buildRendered: buildSourcePage,
226+
});
227+
228+
const absPage = path.join(suiteRoot, pagePath);
229+
const userNote = "SURVIVES_TRANSIENT_VAULT_READ_ERROR";
230+
const edited = (await fs.readFile(absPage, "utf8")).replace(
231+
"<!-- openclaw:human:start -->\n<!-- openclaw:human:end -->",
232+
`<!-- openclaw:human:start -->\n${userNote}\n<!-- openclaw:human:end -->`,
233+
);
234+
await fs.writeFile(absPage, edited, "utf8");
235+
236+
await fs.writeFile(sourcePath, "second imported body", "utf8");
237+
const result = await writeImportedSourcePage({
238+
vaultRoot: suiteRoot,
239+
syncKey: "bridge:retry-imported",
240+
sourcePath,
241+
sourceUpdatedAtMs: Date.UTC(2026, 4, 2),
242+
sourceSize: 19,
243+
renderFingerprint: "fp-r2",
244+
pagePath,
245+
group: "bridge",
246+
state,
247+
buildRendered: buildSourcePage,
248+
});
249+
250+
const after = await fs.readFile(absPage, "utf8");
251+
expect(result.changed).toBe(true);
252+
expect(after).toContain("second imported body");
253+
expect(after).toContain(userNote);
254+
} finally {
255+
spy.mockRestore();
256+
}
257+
});
183258
});

‎extensions/memory-wiki/src/source-page-shared.ts‎

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,20 @@ export async function writeImportedSourcePage(params: {
5252

5353
const raw = await fs.readFile(params.sourcePath, "utf8");
5454
const rendered = params.buildRendered(raw, updatedAt);
55-
const existing = pageStat ? await vault.readText(params.pagePath).catch(() => "") : "";
55+
const existing = pageStat
56+
? await vault.readText(params.pagePath).catch(async (err: unknown) => {
57+
// Retry once for transient read failures (concurrent rewrite race
58+
// that vault-page-write.ts also retries). Fail closed after retry
59+
// exhaustion so an unreadable existing page is never treated as
60+
// absent, protecting user notes from silent loss (#98345).
61+
await new Promise((r) => {
62+
setTimeout(r, 100);
63+
});
64+
return await vault.readText(params.pagePath).catch(() => {
65+
throw err;
66+
});
67+
})
68+
: "";
5669
const nextRendered = existing ? preserveHumanNotesBlock(rendered, existing) : rendered;
5770
if (existing !== nextRendered) {
5871
await writeGuardedVaultPage({

0 commit comments

Comments
 (0)