fix(memory-wiki): preserve human notes block on source re-ingest#95614
Conversation
|
Codex review: needs maintainer review before merge. Reviewed June 21, 2026, 9:24 PM ET / 01:24 UTC. Summary PR surface: Source +58, Tests +329. Total +387 across 5 files. Reproducibility: yes. Source inspection on current main shows both affected writers overwrite existing source pages, and the PR body includes before/after real on-disk output showing notes lost on base and preserved after the patch. Review metrics: 1 noteworthy metric.
Stored data model Merge readiness Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch. Rank-up moves:
Next step before merge
Security Review detailsBest possible solution: Land the shared preservation fix after normal CI and maintainer merge gates, keeping the helper as the canonical path for generated source-page rewrites. Do we have a high-confidence way to reproduce the issue? Yes. Source inspection on current main shows both affected writers overwrite existing source pages, and the PR body includes before/after real on-disk output showing notes lost on base and preserved after the patch. Is this the best way to solve the issue? Yes. A shared helper colocated with the marker/fence logic and used by both missing writer paths is narrower and less drift-prone than duplicating preservation logic in each writer. AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against a2675756b875. Label changesLabel justifications:
Evidence reviewedPR surface: Source +58, Tests +329. Total +387 across 5 files. View PR surface stats
What I checked:
Likely related people:
What the crustacean ranks mean
Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics. How this review workflow works
|
e751b5f to
baa95a4
Compare
|
@clawsweeper re-review Addressed the P1 blocker: |
|
🦞🧹 I asked ClawSweeper to review this item again. |
baa95a4 to
b36ef75
Compare
|
@clawsweeper re-review Addressed the remaining P2: |
|
🦞🧹 I asked ClawSweeper to review this item again. |
b36ef75 to
e6c048a
Compare
|
@clawsweeper re-review Addressed the remaining edge: the matcher no longer anchors on a |
|
🦞🧹 I asked ClawSweeper to review this item again. |
e6c048a to
1095fe8
Compare
|
@clawsweeper re-review Addressed the scope finding: extended the same preservation to the shared imported-source writer |
|
🦞🧹 I asked ClawSweeper to review this item again. |
|
@clawsweeper re-review Addressed the CRLF rank-up finding at head bab77ff: Local verification:
|
|
🦞🧹 I asked ClawSweeper to review this item again. |
Re-ingesting an existing source regenerated the page with an empty wrote inside the human-managed markers. This broke the documented contract that human note blocks are preserved, and diverged from the synthesis and chatgpt-import writers that already preserve the block. When a source page already exists, read it and re-inject its human Notes block before writing. The block is located by scanning past the fenced the content, then taking the first human start marker and the last end marker, so the whole Notes block is preserved verbatim even when the source content or the note text contains the markers or Markdown headings. The same preservation is applied to writeImportedSourcePage so the bridge and unsafe-local source-update writers keep notes too. New page creation is unchanged. Adds regressions for plain re-ingest, marker text in source content, marker text inside the note, a heading inside the note, and an imported source page update.
bab77ff to
66a8e3c
Compare
…nclaw#95614) * fix(memory-wiki): preserve human notes block on source re-ingest Re-ingesting an existing source regenerated the page with an empty wrote inside the human-managed markers. This broke the documented contract that human note blocks are preserved, and diverged from the synthesis and chatgpt-import writers that already preserve the block. When a source page already exists, read it and re-inject its human Notes block before writing. The block is located by scanning past the fenced the content, then taking the first human start marker and the last end marker, so the whole Notes block is preserved verbatim even when the source content or the note text contains the markers or Markdown headings. The same preservation is applied to writeImportedSourcePage so the bridge and unsafe-local source-update writers keep notes too. New page creation is unchanged. Adds regressions for plain re-ingest, marker text in source content, marker text inside the note, a heading inside the note, and an imported source page update. * fix(memory-wiki): preserve notes on CRLF source pages
Summary
Re-ingesting an already-ingested memory-wiki source destroys the user's notes. memory-wiki source pages carry a
## Notesblock delimited by<!-- openclaw:human:start -->/<!-- openclaw:human:end -->markers that the user is meant to edit. When the page is regenerated (same slug, samesources/<slug>.mdpath), it is rewritten with an empty Notes block, silently dropping anything the user wrote inside the human-managed block. This affects every source-page writer:openclaw wiki ingest <path>/ the gateway ingest tool, and the bridge and unsafe-local imported-source sync paths.This contradicts the documented contract in
docs/plugins/memory-wiki.md:129: "Managed content stays inside generated blocks. Human note blocks are preserved."Root cause
extensions/memory-wiki/src/ingest.tsrebuilds the full page body with a hardcoded empty human block and writes it unconditionally, never reading the existing page:The shared imported-source writer
source-page-shared.tswriteImportedSourcePage(used bybridge.tsandunsafe-local.ts) has the same defect: it reads the existing page only to compare, then writes the freshly rendered page over it. The synthesis (apply.ts) and ChatGPT-import (chatgpt-import.ts) writers already preserve the human block; ingest and the imported-source writer were the ones that skipped it.Fix
Add one helper next to the existing human-block markers in
markdown.ts, and call it from both writers before writing.The only untrusted region on the page is the source text, which is fenced inside
## Content.renderMarkdownFencechooses a fence delimiter strictly longer than any backtick run in the content, so the source can never close its own fence early; the rest of the page (the## Notesblock, then the generated## Relatedblock, which carries onlyopenclaw:wiki:relatedmarkers) is structurally trusted. The helper scans for the human block only after that fence closes, taking the first start marker and the last end marker, so the whole Notes block is preserved verbatim even when the source content or the user's own note text contains the human marker comments or Markdown headings. New page creation is unchanged.Why this is the right boundary
There are four memory-wiki source-page writers. Two already preserved the human block (synthesis, ChatGPT import); this change fixes the other two by routing both through one shared
preserveHumanNotesBlockhelper colocated with the markers it protects:ingestMemoryWikiSource(thewiki ingestCLI command atcli.ts:588and the gateway ingest tool) andwriteImportedSourcePage(the bridge and unsafe-local sync paths). No writer is left one-sided.Verification
node scripts/run-vitest.mjs extensions/memory-wiki/src/ingest-human-notes.test.ts(four ingest cases) andextensions/memory-wiki/src/source-page-shared.test.ts(imported-source update case) pass with the patch; each new case fails on the unpatched writer.node scripts/run-vitest.mjs extensions/memory-wiki/src/ingest.test.ts(existing create-path assertion) passes unchanged.node scripts/run-oxlint.mjsandoxfmt --checkclean on the five changed files.Real behavior proof
Behavior addressed: regenerating a memory-wiki source page (via
wiki ingestor via the bridge/unsafe-local imported-source writer) wiped user notes in the human-managed## Notesblock; with the patch the notes survive while the regenerated source content is still updated, including when the source content or the note text contains the human markers or a Markdown heading.Real environment tested: drove the real
ingestMemoryWikiSourceand the realwriteImportedSourcePageruntime functions against a real on-disk vault on pristine base4cb94cc2cfand on the patched head1095fe887c, with identical inputs across five scenarios. Everything stayed real (real vault on disk, real render/write paths); nothing was stubbed.Exact steps or command run after this patch: for each scenario, write the source page, edit its Notes block to add a note, change the source, write the page again through the real writer, then read the page back from disk.
Evidence after fix:
Observed result after fix: across all five scenarios the regenerated page keeps the user's note while still showing the updated source content; on pristine base every note is gone. This holds for both the
wiki ingestwriter and the bridge/unsafe-local imported-source writer, and even when the source content or note text contains the markers or a## Notesheading.What was not tested: no live network or provider request is involved; full repo build not run.