mirror of
https://github.com/openclaw/openclaw.git
synced 2026-08-02 07:41:33 +00:00
fix(memory-wiki): protect notes with damaged human markers (#117104)
This commit is contained in:
committed by
GitHub
parent
d0b6a92f85
commit
6d8f0a1365
@@ -39,6 +39,51 @@ describe("ingestMemoryWikiSource human notes", () => {
|
||||
expect(after).toContain(userNote);
|
||||
});
|
||||
|
||||
it.each([
|
||||
{
|
||||
name: "closing",
|
||||
malformedNotes: "<!-- openclaw:human:start -->\nHANDWRITTEN NOTE MUST SURVIVE",
|
||||
missingMarker: /openclaw:human:end/i,
|
||||
},
|
||||
{
|
||||
name: "opening",
|
||||
malformedNotes: "HANDWRITTEN NOTE MUST SURVIVE\n<!-- openclaw:human:end -->",
|
||||
missingMarker: /openclaw:human:start/i,
|
||||
},
|
||||
])(
|
||||
"preserves the source page when handwritten Notes are missing the $name marker",
|
||||
async ({ malformedNotes, missingMarker }) => {
|
||||
const rootDir = await createTempDir("memory-wiki-malformed-notes-");
|
||||
const inputPath = path.join(rootDir, "roadmap.txt");
|
||||
const { config } = await createVault({ rootDir: path.join(rootDir, "vault") });
|
||||
|
||||
await fs.writeFile(inputPath, "original source content\n", "utf8");
|
||||
await ingestMemoryWikiSource({
|
||||
config,
|
||||
inputPath,
|
||||
nowMs: Date.UTC(2026, 3, 5, 12, 0, 0),
|
||||
});
|
||||
|
||||
const pagePath = path.join(config.vault.path, "sources", "roadmap.md");
|
||||
const existingPage = (await fs.readFile(pagePath, "utf8")).replace(
|
||||
"<!-- openclaw:human:start -->\n<!-- openclaw:human:end -->",
|
||||
malformedNotes,
|
||||
);
|
||||
await fs.writeFile(pagePath, existingPage, "utf8");
|
||||
await fs.writeFile(inputPath, "updated source content\n", "utf8");
|
||||
|
||||
await expect(
|
||||
ingestMemoryWikiSource({
|
||||
config,
|
||||
inputPath,
|
||||
nowMs: Date.UTC(2026, 3, 6, 12, 0, 0),
|
||||
}),
|
||||
).rejects.toThrow(missingMarker);
|
||||
|
||||
await expect(fs.readFile(pagePath, "utf8")).resolves.toBe(existingPage);
|
||||
},
|
||||
);
|
||||
|
||||
it("preserves notes without corrupting source content that contains human markers", async () => {
|
||||
const rootDir = await createTempDir("memory-wiki-markers-");
|
||||
const inputPath = path.join(rootDir, "notes.txt");
|
||||
|
||||
@@ -4,7 +4,9 @@ import path from "node:path";
|
||||
import { describe, expect, it } from "vitest";
|
||||
import {
|
||||
createWikiPageFilename,
|
||||
extractHumanNotesBlock,
|
||||
parseWikiMarkdown,
|
||||
preserveHumanNotesBlock,
|
||||
renderWikiMarkdown,
|
||||
scanWikiPageSummary,
|
||||
slugifyWikiSegment,
|
||||
@@ -65,6 +67,112 @@ describe("slugifyWikiSegment", () => {
|
||||
});
|
||||
});
|
||||
|
||||
describe("human Notes blocks", () => {
|
||||
const startMarker = "<!-- openclaw:human:start -->";
|
||||
const endMarker = "<!-- openclaw:human:end -->";
|
||||
const rendered = ["# Source", "", "## Notes", startMarker, endMarker, ""].join("\n");
|
||||
|
||||
it("extracts and preserves complete human Notes blocks", () => {
|
||||
const existing = rendered.replace(
|
||||
`${startMarker}\n${endMarker}`,
|
||||
`${startMarker}\nDurable human annotation\n${endMarker}`,
|
||||
);
|
||||
|
||||
expect(extractHumanNotesBlock(existing)).toBe(
|
||||
`${startMarker}\nDurable human annotation\n${endMarker}`,
|
||||
);
|
||||
expect(preserveHumanNotesBlock(rendered, existing)).toBe(existing);
|
||||
expect(extractHumanNotesBlock(rendered)).toBeNull();
|
||||
});
|
||||
|
||||
it("leaves pages without human Notes markers unchanged", () => {
|
||||
const existing = "# Source\n\nGenerated content only.\n";
|
||||
|
||||
expect(extractHumanNotesBlock(existing)).toBeNull();
|
||||
expect(preserveHumanNotesBlock(rendered, existing)).toBe(rendered);
|
||||
expect(preserveHumanNotesBlock(existing, rendered)).toBe(existing);
|
||||
});
|
||||
|
||||
it.each([
|
||||
{
|
||||
name: "closing",
|
||||
lines: [startMarker, "Durable human annotation"],
|
||||
missingMarker: endMarker,
|
||||
},
|
||||
{
|
||||
name: "opening",
|
||||
lines: ["Durable human annotation", endMarker],
|
||||
missingMarker: startMarker,
|
||||
},
|
||||
])(
|
||||
"rejects a missing $name marker before extracting or replacing Notes",
|
||||
({ lines, missingMarker }) => {
|
||||
const malformed = ["# Source", "", "## Notes", ...lines, ""].join("\n");
|
||||
const expectedError = `Memory Wiki human Notes are missing ${missingMarker}; restore the missing marker before updating or removing this page`;
|
||||
|
||||
expect(() => extractHumanNotesBlock(malformed)).toThrow(expectedError);
|
||||
expect(() => preserveHumanNotesBlock(rendered, malformed)).toThrow(expectedError);
|
||||
expect(() => preserveHumanNotesBlock(malformed, rendered)).toThrow(expectedError);
|
||||
},
|
||||
);
|
||||
|
||||
it.each([startMarker, endMarker])(
|
||||
"ignores a standalone marker inside fenced source content",
|
||||
(marker) => {
|
||||
const existing = ["# Source", "", "## Content", "```text", marker, "```", ""].join("\n");
|
||||
|
||||
expect(extractHumanNotesBlock(existing)).toBeNull();
|
||||
expect(preserveHumanNotesBlock(rendered, existing)).toBe(rendered);
|
||||
},
|
||||
);
|
||||
|
||||
it("preserves marker comments embedded in complete human Notes", () => {
|
||||
const notes = [
|
||||
"Before copied markers",
|
||||
startMarker,
|
||||
"Between copied markers",
|
||||
endMarker,
|
||||
"After copied markers",
|
||||
].join("\n");
|
||||
const existing = rendered.replace(
|
||||
`${startMarker}\n${endMarker}`,
|
||||
`${startMarker}\n${notes}\n${endMarker}`,
|
||||
);
|
||||
|
||||
expect(extractHumanNotesBlock(existing)).toBe(`${startMarker}\n${notes}\n${endMarker}`);
|
||||
expect(preserveHumanNotesBlock(rendered, existing)).toBe(existing);
|
||||
});
|
||||
|
||||
it("ignores source-body marker pairs when extracting and preserving actual Notes", () => {
|
||||
const sourceWithMarkers = [
|
||||
"# Source",
|
||||
"",
|
||||
"## Content",
|
||||
"```text",
|
||||
startMarker,
|
||||
"Generated source annotation",
|
||||
endMarker,
|
||||
"```",
|
||||
"",
|
||||
"## Notes",
|
||||
startMarker,
|
||||
"Durable human annotation",
|
||||
endMarker,
|
||||
"",
|
||||
].join("\n");
|
||||
|
||||
expect(extractHumanNotesBlock(sourceWithMarkers)).toBe(
|
||||
`${startMarker}\nDurable human annotation\n${endMarker}`,
|
||||
);
|
||||
expect(preserveHumanNotesBlock(rendered, sourceWithMarkers)).toBe(
|
||||
rendered.replace(
|
||||
`${startMarker}\n${endMarker}`,
|
||||
`${startMarker}\nDurable human annotation\n${endMarker}`,
|
||||
),
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
describe("toWikiPageSummary", () => {
|
||||
it("marks raw and generated source body metadata", () => {
|
||||
const rawSource = toWikiPageSummary({
|
||||
|
||||
@@ -530,12 +530,15 @@ function afterSourceContentFence(page: string): number {
|
||||
function findNotesHumanBlock(page: string): { start: number; end: number } | null {
|
||||
const searchFrom = afterSourceContentFence(page);
|
||||
const start = page.indexOf(HUMAN_START_MARKER, searchFrom);
|
||||
if (start === -1) {
|
||||
const endMarker = page.lastIndexOf(HUMAN_END_MARKER);
|
||||
if (start === -1 && endMarker < searchFrom) {
|
||||
return null;
|
||||
}
|
||||
const endMarker = page.lastIndexOf(HUMAN_END_MARKER);
|
||||
if (endMarker < start) {
|
||||
return null;
|
||||
if (start === -1 || endMarker < start) {
|
||||
const missingMarker = start === -1 ? HUMAN_START_MARKER : HUMAN_END_MARKER;
|
||||
throw new Error(
|
||||
`Memory Wiki human Notes are missing ${missingMarker}; restore the missing marker before updating or removing this page`,
|
||||
);
|
||||
}
|
||||
return { start, end: endMarker + HUMAN_END_MARKER.length };
|
||||
}
|
||||
|
||||
@@ -220,6 +220,72 @@ describe("writeImportedSourcePage", () => {
|
||||
expect(after).toContain(userNote);
|
||||
});
|
||||
|
||||
it.each([
|
||||
{ group: "bridge" as const, missingMarker: "opening" as const },
|
||||
{ group: "bridge" as const, missingMarker: "closing" as const },
|
||||
{ group: "unsafe-local" as const, missingMarker: "opening" as const },
|
||||
{ group: "unsafe-local" as const, missingMarker: "closing" as const },
|
||||
])(
|
||||
"preserves a $group page and tracked state when its human Notes $missingMarker marker is missing",
|
||||
async ({ group, missingMarker }) => {
|
||||
const sourcePath = path.join(suiteRoot, "imported-malformed.txt");
|
||||
const pagePath = "sources/imported-malformed.md";
|
||||
const syncKey = `${group}:imported-malformed`;
|
||||
const state: Parameters<typeof writeImportedSourcePage>[0]["state"] = {
|
||||
entries: {},
|
||||
version: 1,
|
||||
};
|
||||
const firstSource = "first imported body";
|
||||
|
||||
await fs.writeFile(sourcePath, firstSource, "utf8");
|
||||
await writeImportedSourcePage({
|
||||
vaultRoot: suiteRoot,
|
||||
syncKey,
|
||||
sourcePath,
|
||||
sourceUpdatedAtMs: Date.UTC(2026, 4, 1),
|
||||
sourceSize: Buffer.byteLength(firstSource),
|
||||
renderFingerprint: "fp-1",
|
||||
pagePath,
|
||||
group,
|
||||
state,
|
||||
buildRendered: buildSourcePage,
|
||||
});
|
||||
|
||||
const absPage = path.join(suiteRoot, pagePath);
|
||||
const userNote = `DURABLE ${group.toUpperCase()} ANNOTATION`;
|
||||
const malformedNotes =
|
||||
missingMarker === "opening"
|
||||
? `${userNote}\n<!-- openclaw:human:end -->`
|
||||
: `<!-- openclaw:human:start -->\n${userNote}`;
|
||||
const existing = (await fs.readFile(absPage, "utf8")).replace(
|
||||
"<!-- openclaw:human:start -->\n<!-- openclaw:human:end -->",
|
||||
malformedNotes,
|
||||
);
|
||||
await fs.writeFile(absPage, existing, "utf8");
|
||||
const previousState = structuredClone(state);
|
||||
const secondSource = "second imported body must not overwrite human Notes";
|
||||
await fs.writeFile(sourcePath, secondSource, "utf8");
|
||||
|
||||
await expect(
|
||||
writeImportedSourcePage({
|
||||
vaultRoot: suiteRoot,
|
||||
syncKey,
|
||||
sourcePath,
|
||||
sourceUpdatedAtMs: Date.UTC(2026, 4, 2),
|
||||
sourceSize: Buffer.byteLength(secondSource),
|
||||
renderFingerprint: "fp-2",
|
||||
pagePath,
|
||||
group,
|
||||
state,
|
||||
buildRendered: buildSourcePage,
|
||||
}),
|
||||
).rejects.toThrow(/human Notes.*missing|missing.*human Notes|openclaw:human:(start|end)/i);
|
||||
|
||||
await expect(fs.readFile(absPage, "utf8")).resolves.toBe(existing);
|
||||
expect(state).toEqual(previousState);
|
||||
},
|
||||
);
|
||||
|
||||
it("preserves CRLF human notes without copying marker comments from existing imported content", async () => {
|
||||
const sourcePath = path.join(suiteRoot, "imported-crlf.txt");
|
||||
const pagePath = "sources/imported-crlf.md";
|
||||
|
||||
157
extensions/memory-wiki/src/source-sync-malformed-notes.test.ts
Normal file
157
extensions/memory-wiki/src/source-sync-malformed-notes.test.ts
Normal file
@@ -0,0 +1,157 @@
|
||||
import fs from "node:fs/promises";
|
||||
import os from "node:os";
|
||||
import path from "node:path";
|
||||
import type { OpenKeyedStoreOptions } from "openclaw/plugin-sdk/plugin-state-runtime";
|
||||
import {
|
||||
createPluginStateKeyedStoreForTests,
|
||||
resetPluginStateStoreForTests,
|
||||
} from "openclaw/plugin-sdk/plugin-state-test-runtime";
|
||||
import { afterEach, beforeEach, describe, expect, it } from "vitest";
|
||||
import {
|
||||
createMemoryWikiSourceSyncStateStore,
|
||||
pruneImportedSourceEntries,
|
||||
readMemoryWikiSourceSyncState,
|
||||
setImportedSourceEntry,
|
||||
writeMemoryWikiSourceSyncState,
|
||||
type MemoryWikiImportedSourceGroup,
|
||||
} from "./source-sync-state.js";
|
||||
|
||||
const tempDirs: string[] = [];
|
||||
|
||||
async function makeTempDir(): Promise<string> {
|
||||
const dir = await fs.mkdtemp(path.join(os.tmpdir(), "memory-wiki-malformed-notes-"));
|
||||
tempDirs.push(dir);
|
||||
return dir;
|
||||
}
|
||||
|
||||
function createImportedSourceState(pagePath: string, group: MemoryWikiImportedSourceGroup) {
|
||||
return {
|
||||
version: 1 as const,
|
||||
entries: {
|
||||
"sync-key": {
|
||||
group,
|
||||
pagePath,
|
||||
sourcePath: "/tmp/source.md",
|
||||
sourceUpdatedAtMs: 0,
|
||||
sourceSize: 0,
|
||||
renderFingerprint: "fp",
|
||||
},
|
||||
},
|
||||
};
|
||||
}
|
||||
|
||||
describe("memory wiki source sync malformed human Notes", () => {
|
||||
beforeEach(() => {
|
||||
resetPluginStateStoreForTests();
|
||||
});
|
||||
|
||||
afterEach(async () => {
|
||||
resetPluginStateStoreForTests();
|
||||
await Promise.all(
|
||||
tempDirs.splice(0).map((dir) => fs.rm(dir, { recursive: true, force: true })),
|
||||
);
|
||||
});
|
||||
|
||||
it.each([
|
||||
{ group: "bridge" as const, missingMarker: "opening" as const },
|
||||
{ group: "bridge" as const, missingMarker: "closing" as const },
|
||||
{ group: "unsafe-local" as const, missingMarker: "opening" as const },
|
||||
{ group: "unsafe-local" as const, missingMarker: "closing" as const },
|
||||
])(
|
||||
"preserves $group pages and persisted state when the $missingMarker marker is missing",
|
||||
async ({ group, missingMarker }) => {
|
||||
const stateDir = await makeTempDir();
|
||||
const vaultRoot = path.join(stateDir, "vault");
|
||||
const pagePath = `sources/${group}-missing-${missingMarker}.md`;
|
||||
const pageAbsPath = path.join(vaultRoot, pagePath);
|
||||
await fs.mkdir(path.dirname(pageAbsPath), { recursive: true });
|
||||
const pageContent = [
|
||||
group === "bridge" ? "# Memory Bridge (test)" : "# Unsafe Local Import: test",
|
||||
"## Content",
|
||||
"```",
|
||||
"generated content",
|
||||
"```",
|
||||
"## Notes",
|
||||
...(missingMarker === "opening" ? [] : ["<!-- openclaw:human:start -->"]),
|
||||
"human annotations must survive malformed markers",
|
||||
...(missingMarker === "closing" ? [] : ["<!-- openclaw:human:end -->"]),
|
||||
"",
|
||||
].join("\n");
|
||||
await fs.writeFile(pageAbsPath, pageContent, "utf8");
|
||||
|
||||
const env = { ...process.env, OPENCLAW_STATE_DIR: stateDir };
|
||||
const store = createMemoryWikiSourceSyncStateStore(<T>(options: OpenKeyedStoreOptions) =>
|
||||
createPluginStateKeyedStoreForTests<T>("memory-wiki", { ...options, env }),
|
||||
);
|
||||
const initialState = createImportedSourceState(pagePath, group);
|
||||
await writeMemoryWikiSourceSyncState(vaultRoot, initialState, store);
|
||||
const state = await readMemoryWikiSourceSyncState(vaultRoot, store);
|
||||
const entry = state.entries["sync-key"]!;
|
||||
const updatedEntry = { ...entry, sourceSize: entry.sourceSize + 1 };
|
||||
setImportedSourceEntry({ state, syncKey: "sync-key", entry: updatedEntry });
|
||||
const expectedMissingMarker =
|
||||
missingMarker === "opening"
|
||||
? "<!-- openclaw:human:start -->"
|
||||
: "<!-- openclaw:human:end -->";
|
||||
|
||||
await expect(
|
||||
pruneImportedSourceEntries({
|
||||
vaultRoot,
|
||||
group,
|
||||
activeKeys: new Set(),
|
||||
state,
|
||||
}),
|
||||
).rejects.toThrow(expectedMissingMarker);
|
||||
|
||||
expect(state.entries["sync-key"]).toEqual(updatedEntry);
|
||||
await expect(readMemoryWikiSourceSyncState(vaultRoot, store)).resolves.toEqual(initialState);
|
||||
await expect(fs.readFile(pageAbsPath, "utf8")).resolves.toBe(pageContent);
|
||||
await expect(fs.access(path.join(vaultRoot, ".salvage"))).rejects.toMatchObject({
|
||||
code: "ENOENT",
|
||||
});
|
||||
|
||||
await writeMemoryWikiSourceSyncState(vaultRoot, state, store);
|
||||
await expect(readMemoryWikiSourceSyncState(vaultRoot, store)).resolves.toMatchObject({
|
||||
entries: { "sync-key": updatedEntry },
|
||||
});
|
||||
},
|
||||
);
|
||||
|
||||
it("rejects pruning an oversized source page with an unclosed human Notes marker", async () => {
|
||||
const vaultRoot = await makeTempDir();
|
||||
const pagePath = "sources/oversized-missing-closing-marker.md";
|
||||
const pageAbsPath = path.join(vaultRoot, pagePath);
|
||||
await fs.mkdir(path.dirname(pageAbsPath), { recursive: true });
|
||||
await fs.writeFile(
|
||||
pageAbsPath,
|
||||
[
|
||||
"# Memory Bridge (test)",
|
||||
"## Content",
|
||||
"```",
|
||||
"x".repeat(16 * 1024 * 1024),
|
||||
"```",
|
||||
"## Notes",
|
||||
"<!-- openclaw:human:start -->",
|
||||
"oversized annotations must survive malformed markers",
|
||||
"",
|
||||
].join("\n"),
|
||||
"utf8",
|
||||
);
|
||||
const state = createImportedSourceState(pagePath, "bridge");
|
||||
|
||||
await expect(
|
||||
pruneImportedSourceEntries({
|
||||
vaultRoot,
|
||||
group: "bridge",
|
||||
activeKeys: new Set(),
|
||||
state,
|
||||
}),
|
||||
).rejects.toThrow("<!-- openclaw:human:end -->");
|
||||
|
||||
expect(state.entries["sync-key"]).toBeDefined();
|
||||
await expect(fs.stat(pageAbsPath)).resolves.toSatisfy((stat) => stat.isFile());
|
||||
await expect(fs.access(path.join(vaultRoot, ".salvage"))).rejects.toMatchObject({
|
||||
code: "ENOENT",
|
||||
});
|
||||
});
|
||||
});
|
||||
@@ -454,17 +454,15 @@ export async function pruneImportedSourceEntries(params: {
|
||||
}
|
||||
// Recover durable Notes before removing an imported source page. The root
|
||||
// handle applies containment and no-follow checks to each operation.
|
||||
let notesBlock: string | null = null;
|
||||
let pageAlreadyRemoved = false;
|
||||
let pageContent: string | undefined;
|
||||
try {
|
||||
const pageContent = await readImportedSourcePageForNotes(vault, entry.pagePath);
|
||||
notesBlock = extractHumanNotesBlock(pageContent);
|
||||
pageContent = await readImportedSourcePageForNotes(vault, entry.pagePath);
|
||||
} catch (error) {
|
||||
if (!(error instanceof FsSafeError && error.code === "not-found")) {
|
||||
continue;
|
||||
}
|
||||
pageAlreadyRemoved = true;
|
||||
}
|
||||
const notesBlock = pageContent === undefined ? null : extractHumanNotesBlock(pageContent);
|
||||
if (notesBlock) {
|
||||
const salvageStem = entry.pagePath.replace(/\//g, "_");
|
||||
const contentHash = createHash("sha256").update(notesBlock).digest("hex").slice(0, 16);
|
||||
@@ -501,7 +499,7 @@ export async function pruneImportedSourceEntries(params: {
|
||||
continue;
|
||||
}
|
||||
}
|
||||
if (!pageAlreadyRemoved) {
|
||||
if (pageContent !== undefined) {
|
||||
try {
|
||||
await vault.remove(entry.pagePath);
|
||||
} catch (error) {
|
||||
|
||||
Reference in New Issue
Block a user