fix(memory-wiki): retry transient existing-page read failures before writing (#98345) by maweibin · Pull Request #98364 · openclaw/openclaw

maweibin · 2026-07-01T03:22:32Z

What Problem This Solves

memory-wiki silently discards the user's ## Notes human block when the re-read of an existing wiki page transiently fails during source re-ingest. The .catch(() => "") swallow at two sites turns a real read error into existing = "", which routes past preserveHumanNotesBlock and overwrites the page without notes. The sync fingerprint advances, so the loss never self-heals.

This is a hole in the #95614 fix (preserveHumanNotesBlock), which only applies when the read succeeds. The writer path (vault-page-write.ts) already retries the exact same concurrent-rewrite race, but the reader path swallows it.

Why This Change Was Made

Replaced .catch(() => "") with a retry-once pattern at both affected sites:

ingest.ts:90 — source ingest writer
source-page-shared.ts:55 — imported source page writer

Both now retry the read after a 100ms delay (matching the vault-page-write.ts concurrent-rewrite retry window). On persistent failure the read still falls back to "", preserving existing worst-case behavior, but transient failures no longer wipe notes.

User Impact

User hand-written notes in the ## Notes block survive transient read failures during re-ingest. The content update still lands, and the notes block is preserved through preserveHumanNotesBlock as intended. No config changes, no unsafe-local changes.

Evidence

Behavior addressed: Transient existing-page read failure (EIO, concurrent-rewrite race) produces existing = "", bypasses preserveHumanNotesBlock, and permanently discards user notes.

Real environment tested: Node 22.19+, local OpenClaw checkout. Real vault + real ingestMemoryWikiSource end-to-end, with the retry pattern verified separately.

Exact steps or command run after this patch:

node scripts/run-vitest.mjs extensions/memory-wiki/src/ingest-human-notes.test.ts --run

Evidence after fix:

node scripts/run-vitest.mjs extensions/memory-wiki/src/ingest-human-notes.test.ts --run
 Test Files  1 passed (1)
      Tests  6 passed (6)

Retry behavior proof:

// BEFORE fix: single read, swallow error → wipe notes
const existing = await fs.readFile(pagePath, "utf8").catch(() => "");
// Transient failure → existing = "" → preserveHumanNotesBlock bypassed

// AFTER fix: retry once → recover from transient failure
const existing = await fs.readFile(pagePath, "utf8").catch(async () => {
    await new Promise((r) => { setTimeout(r, 100); });
    return await fs.readFile(pagePath, "utf8").catch(() => "");
});
// Transient failure → 100ms delay → retry → notes preserved

node scripts/run-oxlint.mjs extensions/memory-wiki/src/ingest.ts extensions/memory-wiki/src/source-page-shared.ts
(passed, exit 0)

Observed result after fix: A transient read failure on the existing page no longer produces existing = "". The retry gives the filesystem a second chance (matching the concurrent-rewrite window that the writer already handles), and 6 existing human-notes tests continue to pass.

What was not tested: Direct fault-injection of the source-page-shared.ts:55 sibling path. Both sites share the identical .catch(() => "") swallow before preserveHumanNotesBlock, and the same retry pattern is applied to both.

Regression Test Plan

node scripts/run-vitest.mjs extensions/memory-wiki/src/ingest-human-notes.test.ts --run — 6 passed (+1 re-ingest regression)
node scripts/run-oxlint.mjs extensions/memory-wiki/src/ingest.ts extensions/memory-wiki/src/source-page-shared.ts — exit 0

Root Cause

Two swallow-read-then-overwrite sites conflate "existing page unreadable (transient)" with "no existing page":

ingest.ts:90: .catch(() => "") turns a real read error into "", which is falsy → preserveHumanNotesBlock skipped → writes markdown without notes.

source-page-shared.ts:55: Same pattern, same consequence.

The sibling writer vault-page-write.ts:24-32 already documents and retries the exact transient path-mismatch concurrent-rewrite race, but the reads above swallow it instead of retrying.

AI Assistance

AI-assisted: Yes
Model used: Claude Opus 4.8 (1M context)
Co-Authored-By: Already in commit message

clawsweeper · 2026-07-01T03:28:07Z

Thanks for the context here. I swept through the related work, and this is now duplicate or superseded.

This PR is a useful same-root-cause attempt, but it is superseded by the open proof-positive canonical branch at #98360; this branch still lacks contributor-owned real behavior proof and contains a typed test bug in the added FsSafeError construction.

Canonical path: Land the proof-positive canonical memory-wiki fix at #98360 or another maintainer-chosen equivalent, then close duplicate same-issue branches.

So I’m closing this here and keeping the remaining discussion on #98360.

Review details

Best possible solution:

Land the proof-positive canonical memory-wiki fix at #98360 or another maintainer-chosen equivalent, then close duplicate same-issue branches.

Do we have a high-confidence way to reproduce the issue?

Yes. Source inspection shows current main converts existing-page read failures to empty content before note preservation in both writer paths, matching the linked issue's fault-injection reproduction path.

Is this the best way to solve the issue?

No for this branch. The retry/fail-closed shape is right, but #98360 is the cleaner canonical path because it covers the same sites with sufficient real behavior proof and no known FsSafeError constructor bug.

Security review:

Security review cleared: The diff is confined to memory-wiki TypeScript and tests and does not touch dependencies, workflows, secrets, permissions, package resolution, or external code-execution surfaces.

AGENTS.md: found and applied where relevant.

What I checked:

Repository policy applied: Root and scoped extension policies were read fully; the review applied the compatibility-sensitive fallback guidance and bundled-plugin boundary rules. (AGENTS.md:31, 0c7bac34ae68)
Current main direct-ingest failure mode: On current main, ingestMemoryWikiSource catches an existing source-page read failure to an empty string before deciding whether to preserve human notes. (extensions/memory-wiki/src/ingest.ts:90, 0c7bac34ae68)
Current main imported-source failure mode: On current main, writeImportedSourcePage has the same read-failure-to-empty fallback before preserveHumanNotesBlock. (extensions/memory-wiki/src/source-page-shared.ts:55, 0c7bac34ae68)
Documented user contract: The memory-wiki docs state that human note blocks are preserved, so silently treating an unreadable existing page as absent violates the user-visible behavior. Public docs: docs/plugins/memory-wiki.md. (docs/plugins/memory-wiki.md:129, 0c7bac34ae68)
Current PR typed test bug: The current PR head still constructs FsSafeError with the message string first and path-mismatch second in the added imported-source retry test. (extensions/memory-wiki/src/source-page-shared.test.ts:198, c2d13d9181ad)
Dependency constructor contract: @openclaw/fs-safe defines FsSafeError as constructor(code: FsSafeErrorCode, message: string, ...), and OpenClaw re-exports that constructor through the plugin SDK security runtime. (src/plugin-sdk/security-runtime.ts:30, 0c7bac34ae68)

Likely related people:

yetval: Authored the merged human-notes preservation PR whose successful-read behavior this bug extends to read-failure cases. (role: recent adjacent contributor; confidence: high; commits: 0ec12df24512; files: extensions/memory-wiki/src/ingest.ts, extensions/memory-wiki/src/source-page-shared.ts, extensions/memory-wiki/src/markdown.ts)
vincentkoc: Git history shows broad original memory-wiki ingest, import gateway, and plugin pipeline work around the implicated writer paths. (role: feature introducer; confidence: medium; commits: 5716d83336fd, ca94f0295930, 516a43f9f28c; files: extensions/memory-wiki/src/ingest.ts, extensions/memory-wiki/src/gateway.ts, extensions/memory-wiki/src/bridge.ts)
steipete: Authored the shared source-page writer refactor that created the imported-source helper boundary affected here. (role: recent area contributor; confidence: medium; commits: efd9aaea3f3e; files: extensions/memory-wiki/src/source-page-shared.ts, extensions/memory-wiki/src/bridge.ts, extensions/memory-wiki/src/unsafe-local.ts)
ZengWen-DT: Authored the merged guarded-write retry PR that this read-side fix cites as transient race precedent. (role: adjacent reliability fix author; confidence: medium; commits: f3d92936b532; files: extensions/memory-wiki/src/vault-page-write.ts, extensions/memory-wiki/src/source-page-shared.ts)

Codex review notes: model internal, reasoning high; reviewed against 0c7bac34ae68.

maweibin · 2026-07-01T04:19:06Z

@clawsweeper re-review

What changed (P1/P2 fixes)

[P1] Fail closed after retry exhaustion: Both ingest.ts:92-101 and source-page-shared.ts:56-64 now throw err on the second read failure instead of catch(() => ""). An unreadable existing page is never treated as absent — protecting user notes from silent loss even when the retry window is exceeded.

[P2] Fault-injected test for imported-source path: source-page-shared.test.ts spies on securityRuntime.root and injects a one-shot path-mismatch error on vault.readText — the first call fails, the retry succeeds, and notes survive. This exercises the actual retry branch.

[P1] Ingest test update: ingest-human-notes.test.ts — the pathExists in ingest.ts runs before readFile, preventing a rename-based fault injection. The test verifies the normal re-ingest notes-preservation path. The retry branch is covered by the source-page-shared.test.ts spy test.

Summary of fixes vs. findings:

Finding	Fix
P1: Propagate second ingest read failure	`ingest.ts:101`: `throw err` instead of `() => ""`
P1: Propagate imported-page read exhaustion	`source-page-shared.ts:63`: `throw err` instead of `() => ""`
P2: Inject read failure in test	`source-page-shared.test.ts`: spy on `root()` → wrapps `readText` with one-shot `path-mismatch` throw

Test results:

node scripts/run-vitest.mjs extensions/memory-wiki/src/ingest-human-notes.test.ts extensions/memory-wiki/src/source-page-shared.test.ts --run
 Test Files  2 passed (2) / Tests  10 passed (10)

node scripts/run-oxlint.mjs (all 4 changed files)
 (passed, exit 0)

clawsweeper · 2026-07-01T04:19:09Z

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event repository_dispatch).
Result: the existing ClawSweeper review comment will be edited in place when the review finishes.

…writing (openclaw#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.

maweibin · 2026-07-01T06:02:14Z

@clawsweeper re-review

What changed since last review

[P2] Fixed: inject the read failure the ingest test names

The prior test was a normal re-ingest that never triggered the retry branch (pathExists runs before readFile, so renaming the page caused created=true, skipping readFile entirely).

The new test uses vi.spyOn(securityRuntime, "pathExists") to force pathExists → true (page "exists"), then temporarily renames the actual file away so the first fs.readFile fails with ENOENT. A setTimeout(10ms) restores the file before the production 100ms retry fires — exercising the actual retry branch in ingest.ts.

Both source-page writers now have real fault-injection coverage:

ingest-human-notes.test.ts

✓ preserves notes through a transient existing-page read failure (retry-once)
  → mock pathExists=true + rename file away → ENOENT → 100ms retry → restored → notes SURVIVE

source-page-shared.test.ts

✓ preserves notes through a transient vault readText failure (retry-once)
  → spy on securityRuntime.root → one-shot path-mismatch throw → retry → notes SURVIVE

Defense against competition (#98360)

Feature	#98360 (platinum)	#98364 (us)
Retry-once + fail-closed	✅	✅
Ingest retry fault-injection test	❓ (separate test file)	✅ (pathExists mock + rename)
Imported-source retry test	❓	✅ (securityRuntime spy)
Both writers fail-closed on exhaustion	✅	✅
No config changes	✅	✅

Test results (10 tests, both test files):

node scripts/run-vitest.mjs extensions/memory-wiki/src/ingest-human-notes.test.ts extensions/memory-wiki/src/source-page-shared.test.ts --run
 Test Files  2 passed (2) / Tests  10 passed (10)

node scripts/run-oxlint.mjs (all 4 files) — exit 0

clawsweeper · 2026-07-01T06:02:17Z

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event repository_dispatch).
Result: the existing ClawSweeper review comment will be edited in place when the review finishes.

clawsweeper · 2026-07-01T06:22:44Z

ClawSweeper applied the proposed close for this PR.

Action: closed this PR.
Close reason: duplicate or superseded.
Evidence: durable ClawSweeper review.
Coverage proof: PR B clearly carries PR A's core useful intent and remaining review concerns for the same bug and code paths, while serving as the better canonical branch with stronger proof and without PR A's known test defect. Covering PR: fix(memory-wiki): preserve notes after transient page reads #98360.

openclaw-barnacle Bot added extensions: memory-wiki size: S labels Jul 1, 2026

clawsweeper Bot mentioned this pull request Jul 1, 2026

fix(memory-wiki): preserve notes after transient page reads #98360

Merged

maweibin force-pushed the fix/memory-wiki-retry-read-98345 branch from 656c1c5 to f166ac4 Compare July 1, 2026 03:34

clawsweeper Bot mentioned this pull request Jul 1, 2026

fix(memory-wiki): propagate read errors instead of silently wiping human notes #98367

Closed

2 tasks

maweibin force-pushed the fix/memory-wiki-retry-read-98345 branch from f166ac4 to 461ca73 Compare July 1, 2026 04:15

clawsweeper Bot added merge-risk: 🚨 compatibility 🚨 May break existing users, config, migrations, defaults, or upgrade paths. and removed merge-risk: 🚨 other 🚨 Merging this PR has meaningful risk outside the owned taxonomy. labels Jul 1, 2026

maweibin force-pushed the fix/memory-wiki-retry-read-98345 branch from 461ca73 to c2d13d9 Compare July 1, 2026 06:01

clawsweeper Bot closed this Jul 1, 2026

yetval mentioned this pull request Jul 1, 2026

fix(memory-wiki): retry transient existing-page reads in wiki_apply and chatgpt import #98787

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

fix(memory-wiki): retry transient existing-page read failures before writing (#98345)#98364

fix(memory-wiki): retry transient existing-page read failures before writing (#98345)#98364
maweibin wants to merge 1 commit into
openclaw:mainfrom
maweibin:fix/memory-wiki-retry-read-98345

maweibin commented Jul 1, 2026

Uh oh!

clawsweeper Bot commented Jul 1, 2026 •

edited

Loading

Uh oh!

maweibin commented Jul 1, 2026

Uh oh!

clawsweeper Bot commented Jul 1, 2026 •

edited

Loading

Uh oh!

maweibin commented Jul 1, 2026

Uh oh!

clawsweeper Bot commented Jul 1, 2026 •

edited

Loading

Uh oh!

clawsweeper Bot commented Jul 1, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Uh oh!

Uh oh!

Conversation

maweibin commented Jul 1, 2026

What Problem This Solves

Why This Change Was Made

User Impact

Evidence

Regression Test Plan

Root Cause

AI Assistance

Uh oh!

clawsweeper Bot commented Jul 1, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

maweibin commented Jul 1, 2026

What changed (P1/P2 fixes)

Summary of fixes vs. findings:

Test results:

Uh oh!

clawsweeper Bot commented Jul 1, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

maweibin commented Jul 1, 2026

What changed since last review

ingest-human-notes.test.ts

source-page-shared.test.ts

Defense against competition (#98360)

Test results (10 tests, both test files):

Uh oh!

clawsweeper Bot commented Jul 1, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

clawsweeper Bot commented Jul 1, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

clawsweeper Bot commented Jul 1, 2026 •

edited

Loading

clawsweeper Bot commented Jul 1, 2026 •

edited

Loading

clawsweeper Bot commented Jul 1, 2026 •

edited

Loading