fix(migrate): reject duplicate item ids, fingerprint the full plan, route Hermes memory copies by item (#109323)

extensions/migrate-hermes/apply.ts

@@ -28,7 +28,6 @@ import {
   HERMES_REASON_MODEL_PROVIDER_CONFLICT,
   readHermesModelDetails,
 } from "./items.js";
-import { isMemoryOnlyMigration } from "./memory.js";
 import { applyModelItem } from "./model.js";
 import { buildHermesPlan } from "./plan.js";
 import { applySecretItem } from "./secrets.js";
@@ -36,6 +35,25 @@ import { resolveTargets } from "./targets.js";
 
 const HERMES_SQLITE_SNAPSHOT_PREFIX = "openclaw-migrate-hermes-sqlite-";
 
+function isHermesMemoryOnlyCopyItem(item: MigrationItem): boolean {
+  return (
+    item.kind === "memory" &&
+    item.action === "copy" &&
+    item.details?.sourceType === "hermes-memory" &&
+    item.details?.collectionId === "hermes"
+  );
+}
+
+function assertConsistentMemoryPlan(plan: MigrationPlan): void {
+  const hasMemoryOnlyCopy = plan.items.some(isHermesMemoryOnlyCopyItem);
+  const hasMemoryAppend = plan.items.some(
+    (item) => item.kind === "memory" && item.action === "append",
+  );
+  if (hasMemoryOnlyCopy && hasMemoryAppend) {
+    throw new Error("Hermes migration plan mixes memory-only copy and append items");
+  }
+}
+
 async function archiveHermesItem(item: MigrationItem, reportDir: string): Promise<MigrationItem> {
   if (!item.source || path.extname(item.source) !== ".db") {
     return await archiveMigrationItem(item, reportDir);
@@ -126,6 +144,7 @@ export async function applyHermesPlan(params: {
   runtime?: MigrationProviderContext["runtime"];
 }): Promise<MigrationApplyResult> {
   const plan = params.plan ?? (await buildHermesPlan(params.ctx));
+  assertConsistentMemoryPlan(plan);
   const reportDir = params.ctx.reportDir ?? path.join(params.ctx.stateDir, "migration", "hermes");
   const targets = resolveTargets(params.ctx);
   // Item ids are report labels, not unique execution keys. Preserve object identity so
@@ -171,13 +190,14 @@ export async function applyHermesPlan(params: {
       appliedItem = await applyAuthItem(applyCtx, item, targets);
     } else if (item.kind === "secret") {
       appliedItem = await applySecretItem(applyCtx, item, targets);
-    } else if (item.action === "append") {
-      appliedItem = await appendItem(item);
-    } else if (isMemoryOnlyMigration(params.ctx) && item.kind === "memory") {
+    } else if (isHermesMemoryOnlyCopyItem(item)) {
+      // Route from the reviewed item shape; ctx.itemKinds is caller metadata and may be absent.
       appliedItem = await copyMemoryMigrationFileItem(item, reportDir, {
         workspaceDir: targets.workspaceDir,
         overwrite: params.ctx.overwrite,
       });
+    } else if (item.action === "append") {
+      appliedItem = await appendItem(item);
     } else {
       appliedItem = await copyMigrationFileItem(item, reportDir, {
         overwrite: params.ctx.overwrite,

extensions/migrate-hermes/provider.test.ts

@@ -256,6 +256,60 @@ describe("Hermes migration provider", () => {
     expect(result.summary.migrated).toBe(1);
   });
 
+  it.runIf(process.platform !== "win32")(
+    "uses the fs-safe copier for memory-only plans applied without itemKinds",
+    async () => {
+      const root = await makeTempRoot();
+      const source = path.join(root, "hermes");
+      const workspaceDir = path.join(root, "workspace");
+      const stateDir = path.join(root, "state");
+      const outsideDir = path.join(root, "outside");
+      await writeFile(path.join(source, "memories", "MEMORY.md"), "remember this\n");
+      const provider = buildHermesMigrationProvider();
+      const plan = await provider.plan(
+        makeContext({ source, stateDir, workspaceDir, itemKinds: ["memory"] }),
+      );
+      await fs.mkdir(workspaceDir, { recursive: true });
+      await fs.mkdir(outsideDir, { recursive: true });
+      await fs.symlink(outsideDir, path.join(workspaceDir, "memory"));
+
+      const result = await provider.apply(makeContext({ source, stateDir, workspaceDir }), plan);
+
+      expect(itemById(result.items, "memory:MEMORY.md")).toMatchObject({
+        status: "error",
+        reason: expect.stringContaining("path alias escape blocked"),
+      });
+      await expect(
+        fs.access(path.join(outsideDir, "imports", "hermes", "MEMORY.md")),
+      ).rejects.toThrow();
+    },
+  );
+
+  it("rejects append items mixed into a memory-only copy plan", async () => {
+    const root = await makeTempRoot();
+    const source = path.join(root, "hermes");
+    const workspaceDir = path.join(root, "workspace");
+    const stateDir = path.join(root, "state");
+    await writeFile(path.join(source, "memories", "MEMORY.md"), "remember this\n");
+    const provider = buildHermesMigrationProvider();
+    const plan = await provider.plan(
+      makeContext({ source, stateDir, workspaceDir, itemKinds: ["memory"] }),
+    );
+    plan.items.push({
+      id: "memory:mixed-append",
+      kind: "memory",
+      action: "append",
+      status: "planned",
+      source: path.join(source, "memories", "MEMORY.md"),
+      target: path.join(workspaceDir, "MEMORY.md"),
+    });
+
+    await expect(
+      provider.apply(makeContext({ source, stateDir, workspaceDir }), plan),
+    ).rejects.toThrow("mixes memory-only copy and append items");
+    await expect(fs.access(path.join(workspaceDir, "MEMORY.md"))).rejects.toThrow();
+  });
+
   it("rejects missing Hermes sources before planning", async () => {
     const root = await makeTempRoot();
     const source = path.join(root, "missing-hermes");

src/commands/migrate/memory-import.test.ts

@@ -0,0 +1,88 @@
+import fs from "node:fs/promises";
+import os from "node:os";
+import path from "node:path";
+import { afterEach, describe, expect, it } from "vitest";
+import type { OpenClawConfig } from "../../config/types.openclaw.js";
+import type { MigrationPlan, MigrationProviderPlugin } from "../../plugins/types.js";
+import { planProviderMemoryImport } from "./memory-import.js";
+
+const tempRoots: string[] = [];
+
+async function makeSourceDir(): Promise<string> {
+  const dir = await fs.realpath(await fs.mkdtemp(path.join(os.tmpdir(), "memory-import-test-")));
+  tempRoots.push(dir);
+  return dir;
+}
+
+afterEach(async () => {
+  await Promise.all(tempRoots.splice(0).map((dir) => fs.rm(dir, { force: true, recursive: true })));
+});
+
+async function memoryPlan(sourceDir: string, ids: string[]): Promise<MigrationPlan> {
+  const items = await Promise.all(
+    ids.map(async (id, index) => {
+      // Duplicate ids still resolve to distinct real files so the dedupe check,
+      // not a missing-file error, is what rejects them.
+      const source = path.join(sourceDir, `${id}-${index}.md`);
+      await fs.writeFile(source, `# ${id}\n`);
+      return {
+        id,
+        kind: "memory" as const,
+        action: "copy" as const,
+        status: "planned" as const,
+        source,
+        target: `/target/${id}-${index}.md`,
+      };
+    }),
+  );
+  return {
+    providerId: "test",
+    source: sourceDir,
+    summary: {
+      total: items.length,
+      planned: items.length,
+      migrated: 0,
+      skipped: 0,
+      conflicts: 0,
+      errors: 0,
+      sensitive: 0,
+    },
+    items,
+  };
+}
+
+function stubProvider(plan: MigrationPlan): MigrationProviderPlugin {
+  return {
+    id: "test",
+    label: "Test",
+    supportedItemKinds: ["memory"],
+    detect: () => ({ found: true, source: plan.source }),
+    plan: () => plan,
+    apply: () => ({ ...plan, summary: plan.summary }),
+  };
+}
+
+const config = {} as OpenClawConfig;
+
+describe("planProviderMemoryImport memory-only shaping", () => {
+  it("rejects a plan with duplicate memory item ids", async () => {
+    const dir = await makeSourceDir();
+    await expect(
+      planProviderMemoryImport({
+        provider: stubProvider(await memoryPlan(dir, ["same-id", "same-id"])),
+        config,
+        agentId: "main",
+      }),
+    ).rejects.toThrow('duplicate memory migration item id "same-id"');
+  });
+
+  it("accepts a plan with unique memory item ids", async () => {
+    const dir = await makeSourceDir();
+    const { plan } = await planProviderMemoryImport({
+      provider: stubProvider(await memoryPlan(dir, ["memory-a", "memory-b"])),
+      config,
+      agentId: "main",
+    });
+    expect(plan.items.map((item) => item.id)).toEqual(["memory-a", "memory-b"]);
+  });
+});

src/commands/migrate/memory-import.ts

@@ -40,6 +40,14 @@ function shapeMemoryOnlyPlan(plan: MigrationPlan): MigrationPlan {
       `memory import found ${items.length} items; the maximum is ${MAX_MEMORY_MIGRATION_ITEMS}. Narrow or split the source memory before importing.`,
     );
   }
+  // Selection is id-based; duplicates would make one reviewed id execute multiple items.
+  const itemIds = new Set<string>();
+  for (const item of items) {
+    if (itemIds.has(item.id)) {
+      throw new Error(`duplicate memory migration item id "${item.id}"`);
+    }
+    itemIds.add(item.id);
+  }
   const unsupported = items.find(
     (item) => (item.status === "planned" || item.status === "conflict") && item.action !== "copy",
   );

src/gateway/server-methods/migrations.test.ts

@@ -302,17 +302,15 @@ describe("memory migration gateway handlers", () => {
     expect(mocks.runMigrationApply).not.toHaveBeenCalled();
   });
 
-  it("allows volatile provider diagnostics to change after preview", async () => {
+  it.each([
+    ["metadata", (plan: MigrationPlan) => (plan.metadata = { revision: "new" })],
+    ["warnings", (plan: MigrationPlan) => (plan.warnings = ["updated warning"])],
+    ["item message", (plan: MigrationPlan) => (plan.items[0]!.message = "updated message")],
+  ])("rejects apply when plan %s changes after preview", async (_field, mutatePlan) => {
     const plan = memoryPlan();
     mocks.providers = [provider(plan)];
-    const applied = memoryPlan();
-    applied.items = [applied.items[0]!];
-    applied.summary.total = 1;
-    mocks.runMigrationApply.mockResolvedValue(applied);
     const planFingerprint = await loadPlanFingerprint();
-    plan.warnings = ["updated diagnostic"];
-    plan.nextSteps = ["updated next step"];
-    plan.metadata = { plannedAt: Date.now() };
+    mutatePlan(plan);
     const request = invoke("migrations.memory.apply", {
       agentId: "research",
       providerId: "codex",
@@ -322,8 +320,10 @@ describe("memory migration gateway handlers", () => {
 
     await request.run();
 
-    expect(firstCall(request.respond)[0]).toBe(true);
-    expect(mocks.runMigrationApply).toHaveBeenCalledOnce();
+    const [ok, , error] = firstCall(request.respond);
+    expect(ok).toBe(false);
+    expect(error?.message).toContain("plan changed");
+    expect(mocks.runMigrationApply).not.toHaveBeenCalled();
   });
 
   it("rejects stale item ids from a freshly rebuilt apply plan", async () => {

src/gateway/server-methods/migrations.ts

@@ -107,27 +107,13 @@ function fingerprintMemoryPlan(params: {
     .createHash("sha256")
     .update(
       stableStringify({
-        version: 2,
+        version: 3,
         agentId: params.agentId,
         workspace: params.workspace,
         providerId: params.providerId,
         overwrite: params.overwrite === true,
-        plan: {
-          source: params.plan.source,
-          target: params.plan.target ?? null,
-          items: params.plan.items.map((item) => ({
-            id: item.id,
-            kind: item.kind,
-            action: item.action,
-            status: item.status,
-            source: item.source ?? null,
-            target: item.target ?? null,
-            reason: item.reason ?? null,
-            sensitive: item.sensitive === true,
-            sourceRevision: item.sourceRevision ?? null,
-            details: item.details ?? null,
-          })),
-        },
+        // Apply receives the full plan, so every provider-visible field must bind to the review.
+        plan: params.plan,
       }),
     )
     .digest("hex");