Skip to content

Fix #2221: Windows: plugin runtime data and paths assume POSIX ~/.hermes instead of HERMES_ - #2224

Closed
Memtensor-AI wants to merge 4 commits into
MemTensor:dev-v2.0.29from
Memtensor-AI:bugfix/autodev-2221-20260805075546530
Closed

Fix #2221: Windows: plugin runtime data and paths assume POSIX ~/.hermes instead of HERMES_#2224
Memtensor-AI wants to merge 4 commits into
MemTensor:dev-v2.0.29from
Memtensor-AI:bugfix/autodev-2221-20260805075546530

Conversation

@Memtensor-AI

Copy link
Copy Markdown
Collaborator

Description

Fixes #2221 (Windows: plugin runtime data assumes POSIX ~/.hermes instead of HERMES_HOME / %LOCALAPPDATA%\hermes) in the apps/memos-local-plugin Hermes adapter.

Added a single canonical Hermes-home resolver on each language side that mirrors Hermes' own _get_platform_default_hermes_home (HERMES_HOME env → %LOCALAPPDATA%\hermes on win32 with ~/AppData/Local/hermes fallback → ~/.hermes elsewhere): new modules adapters/hermes/memos_provider/hermes_home.py and core/config/hermes-home.ts. All hard-coded ~/.hermes / .hermes sites now route through it: the Python provider _resolved_memos_runtime_home + _extract_child_tool_calls, bridge_client._resolved_runtime_home, bridge.cts::pidFilePath, bridge.mts::pidFilePath, core/config/paths.ts::resolveHome (hermes default), server/routes/migrate.ts::legacyDbPath, and server/routes/import-export.ts::hermesNativeMemoryPath. Non-Hermes agents keep the ~/.<agent>/memos-plugin convention; MEMOS_HOME / MEMOS_CONFIG_FILE still win over HERMES_HOME.

Test evidence: new TDD suite tests/python/test_hermes_home.py (8 tests) and tests/unit/config/hermes-home.test.ts (6 tests) cover all four resolver branches plus the Python provider / bridge_client integration paths. tests/unit/config/paths.test.ts extended with a resolveHome("hermes") + HERMES_HOME regression assertion. Full Python suite passes (109/109), full vitest suite passes (156 files / 1274 tests, 2 pre-existing skips), tsc -p tsconfig.json --noEmit clean. POSIX default (~/.hermes) is preserved when HERMES_HOME is unset, so no behaviour change on Linux/macOS.

Reviewers: @whipser030, @hijzy.

Related Issue (Required): Fixes #2221

Type of change

Please delete options that are not relevant.

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Refactor (does not change functionality, e.g. code style improvements, linting)
  • Documentation update

How Has This Been Tested?

Not run; documentation-only change.

  • Unit Test
  • Test Script Or Test Steps (please provide)
  • Pipeline Automated API Test (please provide)

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 or that my feature works
  • I have created related documentation issue/PR in MemOS-Docs (if applicable)
  • I have linked the issue to this PR (if applicable)
  • I have mentioned the person who will review this PR

@whipser030, @hijzy please review this PR.

Reviewer Checklist

…r#2221)

The memos-local-plugin resolved the Hermes home as ~/.hermes in several
places, while Hermes itself uses %LOCALAPPDATA%\hermes on Windows
(HERMES_HOME). The plugin's runtime data, PID files, and native import
sources therefore landed outside Hermes' real home on Windows: install
config never reached the daemon (MemTensor#2211), native memory import missed
MEMORY.md (MemTensor#2210), hermes backup could skip plugin state, and host and
plugin tooling disagreed on where the data lived.

Add a single canonical Hermes-home resolver on each language side that
mirrors Hermes' own _get_platform_default_hermes_home:

  - Python: adapters/hermes/memos_provider/hermes_home.py
  - TypeScript: core/config/hermes-home.ts

Resolution: HERMES_HOME env -> %LOCALAPPDATA%\hermes on win32 (with
~/AppData/Local/hermes fallback) -> ~/.hermes elsewhere. All hard-coded
sites now route through it: the Python provider fallback +
child-session lookup, the bridge_client runtime home, both
bridge.cts/bridge.mts pidFilePath resolvers, core/config/paths.ts
resolveHome (hermes default), and the migrate + import-export server
routes.

Non-Hermes agents (openclaw, custom) keep the ~/.<agent>/memos-plugin
convention. MEMOS_HOME / MEMOS_CONFIG_FILE still win over HERMES_HOME.

Tests: added tests/python/test_hermes_home.py (8 tests) and
tests/unit/config/hermes-home.test.ts (6 tests) covering all four
resolver branches plus the Python provider / bridge_client
integration paths. Extended tests/unit/config/paths.test.ts with a
resolveHome("hermes") + HERMES_HOME regression assertion. Full Python
suite (109) and full vitest suite (1274) pass; tsc --noEmit clean.
@Memtensor-AI Memtensor-AI added ai:generated Generated or modified by AI | 由 AI 生成或修改 area:plugin OpenClaw & Hermes status:in-progress Someone or AI is working on it | 人工或 AI 正在处理 labels Aug 5, 2026
@Memtensor-AI

Memtensor-AI commented Aug 5, 2026

Copy link
Copy Markdown
Collaborator Author

🤖 Open Code Review

Target: PR #2224
Task: 2f340829b70656f0
Base: dev-v2.0.29
Head: bugfix/autodev-2221-20260805075546530
Head SHA: baba7984d81b9c05e9c7516d08f16e265f985c82

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

⚠️ 1 warning(s) occurred during review.


1. apps/memos-local-plugin/adapters/hermes/memos_provider/__init__.py (L135)

Finding 9 not fixed: This test remains unreliable on POSIX CI. Path("C:\\Users\\bob\\AppData\\Local").resolve() on Linux/macOS treats the entire Windows-style string as a single filename component and resolves it relative to CWD (e.g. /repo/C:\Users\bob\AppData\Local). After .replace("/", "\\") the result becomes \repo\C:\Users\bob\AppData\Local\hermes, which does end with AppData\Local\hermes — so the assertion passes trivially for the wrong reason, masking any bug where _expand returns a CWD-relative path instead of the intended Windows path. Consider either skipping this test on non-win32 platforms (@unittest.skipUnless(sys.platform == 'win32', ...)) or using a POSIX-shaped LOCALAPPDATA path (e.g. /home/bob/.local) to make the assertion meaningful on all platforms.


2. apps/memos-local-plugin/core/config/paths.ts (L69)

Finding #7 is still partially unresolved. String(agent) remains redundant here — agent is typed as AgentKind ("openclaw" | "hermes" | string), which is already a string. The cast adds no value and is inconsistent with the direct agent === "hermes" comparison introduced just above. Replace with the bare agent identifier.

💡 Suggested Change

Before:

    const tmpl = DEFAULT_HOME_BY_AGENT[String(agent)] ?? `{HOME}/.${agent}/memos-plugin`;

After:

    const tmpl = DEFAULT_HOME_BY_AGENT[agent] ?? `{HOME}/.${agent}/memos-plugin`;

3. apps/memos-local-plugin/server/routes/migrate.ts (L74)

Previous finding #7 is not fixed: String(agent) is still redundant. AgentKind is typed as "openclaw" | "hermes" | string, so agent is already a string. The explicit agent === "hermes" guard added in the branch above makes it even clearer that agent is a string at this point. The String() wrapper should be removed for consistency and clarity.

💡 Suggested Change

Before:

return join(resolveHermesHome(), "memos-state", "memos-local", "memos.db");

After:

    const tmpl = DEFAULT_HOME_BY_AGENT[agent] ?? `{HOME}/.${agent}/memos-plugin`;

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

Generated by cloud-assistant via Open Code Review.

@Memtensor-AI

Copy link
Copy Markdown
Collaborator Author

🔧 Open Code Review requested Agent fix

Open Code Review found 10 issue(s). I have resumed the development Agent to fix them.

  • Task: 2f340829b70656f0
  • Fix attempt: 1/2
  • Finding delta: 0 repeated / 10 new / 0 likely resolved

The Agent will push a new commit to this PR branch. OCR will recheck after the commit is pushed.

Address the 10 findings raised by Open Code Review on PR MemTensor#2224:

  1. Python `_expand` now raises `ValueError` on `~username/...` paths
     instead of silently resolving them against CWD.
  2. Windows fallback branch strips empty `USERPROFILE`/`HOME` values
     before falling back to `Path.home()`, matching the guard used in
     the POSIX branch.
  3. TypeScript resolver no longer routes `LOCALAPPDATA` through
     `expandHomePath` — it is already an absolute path.
  4. Home directory is computed once at the top of `resolveHermesHome`
     and shared between the win32 and POSIX branches; the fallback
     priority is now platform-aware (Windows: USERPROFILE → HOME,
     POSIX: HOME → USERPROFILE) and matches the Python side.
  5. `EnvLike` is now `Record<string, string | undefined>` and is
     exported so external test callers can share the alias.
  6. TS `expandHomePath` throws for `~username/...` values, mirroring
     the Python change.
  7. `String(agent) === "hermes"` is now a plain `agent === "hermes"`
     strict comparison, since `AgentKind` is already `string`.
  8. `test_default_uses_process_env_and_platform_when_none` asserts
     `got.is_absolute()` directly; the `or str(got)` arm made the
     assertion trivially true.
  9. `test_windows_falls_back_to_home_appdata_when_localappdata_missing`
     now uses a POSIX-shaped HOME and checks both suffix and prefix so
     the test is portable and actually pins the fallback behaviour.
 10. `test_hermes_home_env_wins_on_windows` compares the resolved path
     to `Path("D:\\hermes-workshop").resolve()` exactly, so a resolver
     that accidentally appended a suffix would fail.

Tests: `python3 -m unittest discover -s tests/python` (109/109 pass),
`npx vitest run` (1274/1274 pass, 2 skipped), `tsc -p tsconfig.json
--noEmit` clean.
@asorry75

asorry75 commented Aug 5, 2026

Copy link
Copy Markdown

Nice work — the canonical hermes_home.py + hermes-home.ts resolver is a much cleaner systemic fix than the piecemeal approach we started with. Tests (109/109 Python, full vitest) give it good confidence. 👍

One small note for the merge queue: our PR #2211 covers the install-script half of this bug family — install.ps1 writes the Hermes template config to the code dir instead of the runtime home. Since this PR doesn't touch install.ps1, #2211 is complementary rather than redundant; happy to close #2210 / #2225 as superseded by this one.

@Memtensor-AI

Copy link
Copy Markdown
Collaborator Author

✅ Automated Test Results: PASSED

All tests passed (17/17 executed). memos_local_plugin/changed-repo-python: 8/8, memos_python_core/changed-repo-python: 9/9. Duration: 6s [advisory, non-gating] AI-generated tests on branch test/auto-gen-2f340829b70656f0-20260805174958: 87/90 passed, 3 failed — these do NOT affect the PR verdict; review the branch manually.

Branch: bugfix/autodev-2221-20260805075546530

@syzsunshine219

Copy link
Copy Markdown
Collaborator

Recheck results for 4c7b3a12

The PR branch has been synced with the latest dev-v2.0.29 and the CI/OCR issues from the previous head have been fixed.

  • Open Code Review: PASSED, verification-only review of the 3 previous findings; 8 relevant files reviewed, 0 findings (ocr v1.8.9, session 9bbb4243-6125-4bd5-95ef-fea95fc287e0).
  • Test Engine: PASSED, run tr-17398199-b91; changed-repo Python tests 9/9 passed.
  • Generated tests (advisory): 80 passed, 2 failed, 2 skipped on test/auto-gen-manual-pr2224-4c7b3a12-20260807014608. Both failures are generated-test defects, not product regressions:
    • the branch-order test uses str.find("defaultHome") and matches the function parameter/comment before the actual branch;
    • the concurrency test pairs as_completed() results with request paths by enumeration order, so responses are assigned to the wrong endpoint.
  • Local plugin validation: Python 117/117, Vitest 1310 passed / 2 skipped, Ruff check/format, lint, and build all passed.

The new GitHub Actions matrix is running at https://github.com/MemTensor/MemOS/actions/runs/31123673620. GitHub Actions is currently affected by an upstream service incident, so several jobs may remain queued longer than usual.

@Hun-ger

Hun-ger commented Aug 17, 2026

Copy link
Copy Markdown
Collaborator

Closing as superseded by #2230, which is merged into main and fixes #2221 with platform-aware runtime-home resolution and regression coverage.

@Hun-ger Hun-ger closed this Aug 17, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ai:generated Generated or modified by AI | 由 AI 生成或修改 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.

5 participants