Skip to content

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

Open
keeponlight wants to merge 1 commit into
MemTensor:mainfrom
keeponlight:fix/hermes-native-user-import
Open

keeponlight wants to merge 1 commit into
MemTensor:mainfrom
keeponlight:fix/hermes-native-user-import

Conversation

@keeponlight

Copy link
Copy Markdown

Supersedes #2360 (closed automatically due to base branch dev-v2.0.34 deletion).

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 main (rebased following the merge and deletion of dev-v2.0.34).

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 16, 2026
@Memtensor-AI

Copy link
Copy Markdown
Collaborator

🤖 Open Code Review

Target: PR #2378
Task: f2484549948011d9
Base: main
Head: fix/hermes-native-user-import

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

⚠️ 1 warning(s) occurred during review.


1. apps/memos-local-plugin/server/routes/import-export.ts (L415-L418)

Re-throwing any non-ENOENT error (e.g. EACCES, EPERM, EIO) means a permission-denied or transient I/O failure on the optional USER.md aborts the entire import — including the already-stat'd MEMORY.md — and surfaces as a generic 404 not_found with no indication of which file failed or why. The comment says USER.md is optional, but the behaviour makes it a hard dependency whenever it exists and is unreadable.

Consider logging the error and continuing rather than re-throwing, or at minimum wrapping the re-throw to add context about the failing file path. Example:

} catch (err) {
  const code = (err as NodeJS.ErrnoException).code;
  if (code !== "ENOENT") {
    // Degraded mode: skip USER.md but surface a warning so the caller can report it.
    console.warn(`[hermes-import] Could not read optional ${userPath}: ${(err as Error).message}`);
  }
}

2. apps/memos-local-plugin/server/routes/import-export.ts (L465-L473)

Two related issues here:

  1. Non-monotonic timestamps across files. baseTs is now per-entry and derived from the file's mtimeMs, but ts is computed using the global index (opts.offset + i) and opts.total. When a batch mixes entries from MEMORY.md (newer mtime) and USER.md (older mtime), the timestamp formula can produce values that interleave non-monotonically or collide, since a lower baseTs combined with a lower opts.total - index offset can land on the same millisecond as an entry from the other file.

  2. Position-dependent hash (memory.index) causes re-import on insertion. identity uses memory.index — the entry's position within its own file. Inserting or deleting any entry before a given one shifts all subsequent index values, regenerates their hashes, and causes them to be re-imported as new episodes on the next sync. The original scheme used the global batch offset, which had the same fragility, but the intent here seems to be stability — in that case the hash should be keyed on content alone (e.g. just memory.text namespaced by file), not on position.


3. apps/memos-local-plugin/server/routes/import-export.ts (L428-L434)

The two readFile calls are independent but awaited sequentially inside a for...of loop. With only two files the impact is small, but they can be parallelised with Promise.all for consistency:

const rawContents = await Promise.all(files.map(({ path: p }) => readFile(p, "utf8")));
for (const [i, { file, info }] of files.entries()) {
  for (const [index, text] of splitHermesNativeMemories(rawContents[i]!).entries()) {
    source.memories.push({ text, file, index, mtimeMs: info.mtimeMs });
  }
  source.bytes += info.size;
}

4. apps/memos-local-plugin/server/routes/import-export.ts (L467-L472)

The episode ID hash now uses memory.index — the entry's position within its own file. This means inserting or deleting any entry before a given one shifts all subsequent index values, regenerating their hashes and causing those entries to be re-imported as new episodes on the next sync (silent duplicates). If the goal is content-stable IDs, the hash should depend on content alone (e.g. just memory.text namespaced by file), not on its file-local position.


🧹 Filtered 3 low-confidence OCR finding(s) before posting/fix-loop (duplicate: 2, existing_code_mismatch: 1).

Generated by cloud-assistant via Open Code Review.

@keeponlight

Copy link
Copy Markdown
Author

Hi @hijzy @syzsunshine219,

Friendly ping for review!

This PR is a clean resubmission of #2360 (which was automatically closed when dev-v2.0.34 was merged into main and deleted).

Could you please take a look when you have a moment? Thank you!

@Memtensor-AI

Copy link
Copy Markdown
Collaborator

✅ Automated Test Results: PASSED

All tests passed (85/85 executed). memos_local_plugin/unit: 85/85. Duration: 4s

Branch: fix/hermes-native-user-import

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

Labels

area:plugin OpenClaw & Hermes status:in-progress Someone or AI is working on it | 人工或 AI 正在处理

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Native memory import ignores USER.md (Hermes adapter reads only MEMORY.md)

3 participants