Skip to content

fix: import Hermes USER.md native memories - #2360

Open
keeponlight wants to merge 1 commit into
MemTensor:dev-v2.0.34from
keeponlight:fix/hermes-native-user-import
Open

fix: import Hermes USER.md native memories#2360
keeponlight wants to merge 1 commit into
MemTensor:dev-v2.0.34from
keeponlight:fix/hermes-native-user-import

Conversation

@keeponlight

Copy link
Copy Markdown

Description

Hermes native scans and paged imports only read MEMORY.md, silently omitting the user profiles stored in the sibling USER.md. Both endpoints now include the optional profile file and report the combined entry count and byte size.

Imported traces retain their source filename in the existing tags field and use that file's modification time. The cache fingerprints each file separately, so same-size edits to the older file and creation/removal of USER.md invalidate cached pages. Existing MEMORY.md IDs are preserved; profile IDs are distinct and remain stable when MEMORY.md grows. Missing USER.md remains valid, while other read errors are reported.

No new dependencies or public request/response schema changes.

Related Issue (Required): Fixes #2306

Reviewer: @syzsunshine219

This PR targets the current development branch, dev-v2.0.34; the dev branch named in CONTRIBUTING.md does not currently exist upstream.

Type of change

  • Bug fix (non-breaking change which fixes an issue)

How Has This Been Tested?

Tested on Windows with Node.js 24.13.1 and Vitest 2.1.9.

  • Unit Test — nine new regression cases cover both files, paging, source tags/timestamps, optional empty/missing profiles, stable IDs, cache invalidation, and read errors. The regression suite failed before the fix.
  • Test Script Or Test Steps — run from apps/memos-local-plugin:
npm test -- tests/unit/server/hermes-native-import.test.ts tests/unit/server/import-export-path.test.ts
npm run lint
npm run build

Results:

Test Files  2 passed (2)
     Tests  12 passed (12)

TypeScript lint and build passed. Repository-root make format passed (All checks passed!; 629 files left unchanged). The normal pre-commit hook passed for the committed files.

The broader npm test -- tests/unit/server run reports 105 passed / 10 failed. Re-running with the original importer from base commit 0f747744 reports 96 passed / the same 10 failed: six assertions assume POSIX path separators, two Windows lifecycle tests lack a response mock, and two SSE shutdown tests fail their unsubscribe assertions. These existing failures are outside this fix. No FastAPI pipeline/API contract changes are involved.

Checklist

  • I have performed a self-review of my own code.
  • I have commented my code in hard-to-understand areas.
  • I have added tests that prove my fix is effective.
  • Related MemOS-Docs issue/PR considered — not applicable to this bug fix; existing import endpoints and public schemas are unchanged.
  • I have linked the issue to this PR.
  • I have mentioned the person who will review this PR.

Reviewer Checklist

@Memtensor-AI Memtensor-AI added area:plugin OpenClaw & Hermes status:in-progress Someone or AI is working on it | 人工或 AI 正在处理 labels Sep 11, 2026
@Memtensor-AI

Copy link
Copy Markdown
Collaborator

🤖 Open Code Review

Target: PR #2360
Task: 687d961a629c4ba0
Base: dev-v2.0.34
Head: fix/hermes-native-user-import

🔍 OpenCodeReview found 2 issue(s) in this PR.


1. apps/memos-local-plugin/server/routes/import-export.ts (L468-L470)

The hash input for MEMORY.md entries has changed from the old ${globalIndex}\0${memory} to ${memory.index}\0${memory.text}, where memory.index is the per-file position. When opts.offset > 0 (i.e. any batch after the first), memory.index and opts.offset + i diverge, so the same memory entry will produce a different episodeId than it did before this change. The comment says "Preserve existing MEMORY.md IDs" but this only holds true when offset is 0. On subsequent batch imports, every MEMORY.md entry will get a new hash, causing duplicate imports for users who have previously run the importer.

Suggestion: use opts.offset + i (the global index, matching the previous behavior) as the index component for MEMORY.md entries, or document clearly that re-import is expected and the storage layer deduplicates by content rather than ID.

💡 Suggested Change

Before:

    const identity = `${memory.index}\0${memory.text}`;
    const hash = createHash("sha256")
      .update(memory.file === "MEMORY.md" ? identity : `${memory.file}\0${identity}`)

After:

    // For MEMORY.md, reproduce the pre-change hash input exactly:
    // the global index (offset + position-in-batch) paired with the text.
    const globalIndex = opts.offset + i;
    const identity = memory.file === "MEMORY.md"
      ? `${globalIndex}\0${memory.text}`
      : `${memory.index}\0${memory.text}`;
    const hash = createHash("sha256")
      .update(memory.file === "MEMORY.md" ? identity : `${memory.file}\0${identity}`)

2. apps/memos-local-plugin/server/routes/import-export.ts (L427-L435)

If readFile for USER.md throws a non-ENOENT error (e.g. a permission error or I/O failure), the loop exits mid-way: source.memories contains only MEMORY.md entries while source.bytes and the fingerprint already reflect both files. This partial result is then written into the cache at line 435. On the next call, the fingerprint will still match (stat succeeded for both files), so the incomplete cache entry will be returned without re-reading USER.md, silently dropping all profile memories until the server restarts.

Suggestion: only call hermesNativeCache.set after the loop completes successfully, or build the source into a local variable and assign atomically after the loop.

💡 Suggested Change

Before:

  const source: HermesNativeSource = { memories: [], bytes: 0 };
  for (const { path: filePath, file, info } of files) {
    const raw = await readFile(filePath, "utf8");
    for (const [index, text] of splitHermesNativeMemories(raw).entries()) {
      source.memories.push({ text, file, index, mtimeMs: info.mtimeMs });
    }
    source.bytes += info.size;
  }
  hermesNativeCache.set(path, { source, fingerprint });

After:

  const source: HermesNativeSource = { memories: [], bytes: 0 };
  for (const { path: filePath, file, info } of files) {
    const raw = await readFile(filePath, "utf8"); // throws → cache is not written
    for (const [index, text] of splitHermesNativeMemories(raw).entries()) {
      source.memories.push({ text, file, index, mtimeMs: info.mtimeMs });
    }
    source.bytes += info.size;
  }
  // Only reached if all files were read successfully.
  hermesNativeCache.set(path, { source, fingerprint });

Generated by cloud-assistant via Open Code Review.

@Memtensor-AI

Copy link
Copy Markdown
Collaborator

✅ Automated Test Results: PASSED

All tests passed (85/85 executed). memos_local_plugin/unit: 85/85. Duration: 5s [advisory, non-gating] AI-generated tests on branch test/auto-gen-687d961a629c4ba0-20260912001821: 0/60 passed, 60 failed — these do NOT affect the PR verdict; review the branch manually.

Branch: fix/hermes-native-user-import

@Memtensor-AI Memtensor-AI added status:ready Ready for implementation; waiting for assignee or AI dispatch | 可进入实现,等待认领或派发 and removed status:in-progress Someone or AI is working on it | 人工或 AI 正在处理 labels Sep 11, 2026
@keeponlight

Copy link
Copy Markdown
Author

Thanks for the review. I checked both findings against the current head, 9675e99c:

  1. Legacy IDs on later pages: memory.index is assigned while parsing the entire file (L430), before pagination at L174. MEMORY.md entries always come first, so their stored index equals offset + i, including on subsequent pages. Their trace and episode hash inputs remain identical to the pre-PR implementation.

  2. Caching after a read failure: source is a fresh local object, and hermesNativeCache.set at L435 already runs after the entire read loop. A rejected readFile exits the function before that call, so a partial source cannot be cached or replace a previous complete cache entry.

Verification against the unchanged PR head:

  • Existing Hermes import/path tests: 12 passed.
  • Additional temporary local checks (not committed): 7 passed, covering legacy IDs with batch sizes 1, 2, and 25, a cold-cache import starting at offset 25, and USER.md read failures followed by nonzero-offset retries with unchanged file size/mtime. They also verify that a failed forced refresh preserves a previous complete cache entry.
  • npm run lint and npm run build: passed.

These two findings appear to be false positives. For the separate advisory report of 60 AI-generated test failures, the named branch is not currently visible in the upstream repository or my fork. Please share the branch URL or failure logs so those failures can be investigated; the reported regular test suite passed 85/85.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:plugin OpenClaw & Hermes status:ready Ready for implementation; waiting for assignee or AI dispatch | 可进入实现,等待认领或派发

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants