test(tools): add unit tests for memory_tool.py#123
Conversation
Adds comprehensive test coverage for the memory tool module: - MemoryStore initialization and custom limits - Persistence: load/save operations, atomic writes, deduplication - add() operation: success cases, empty content, duplicates, limit enforcement - replace() operation: success, not found, multiple matches, limit enforcement - remove() operation: success, partial match, not found - Security scanning: prompt injection, role hijacking, exfiltration patterns, invisible unicode - Frozen snapshot behavior for system prompt stability - memory_tool() dispatcher function and error handling - Edge cases: delimiter character, multiline entries, unicode, boundary conditions 55 tests covering all major code paths.
Add a --worktree (-w) flag to the hermes CLI that creates an isolated git worktree for the session. This allows running multiple hermes-agent instances concurrently on the same repo without file collisions. How it works: - On startup with -w: detects git repo, creates .worktrees/<session>/ with its own branch (hermes/<session-id>), sets TERMINAL_CWD to it - Each agent works in complete isolation — independent HEAD, index, and working tree, shared git object store - On exit: auto-removes worktree and branch if clean, warns and keeps if there are uncommitted changes - .worktreeinclude file support: list gitignored files (.env, .venv/) to auto-copy/symlink into new worktrees - .worktrees/ is auto-added to .gitignore - Agent gets a system prompt note about the worktree context - Config support: set worktree: true in config.yaml to always enable Usage: hermes -w # Interactive mode in worktree hermes -w -q "Fix issue #123" # Single query in worktree # Or in config.yaml: worktree: true Includes 17 tests covering: repo detection, worktree creation, independence verification, cleanup (clean/dirty), .worktreeinclude, .gitignore management, and 10 concurrent worktrees. Closes #652
RuckVibeCodes
left a comment
There was a problem hiding this comment.
[gus-first-pass] Overall looks good. Comprehensive tests added for memory_tool.py. No critical issues found in the PR.
| """Multiline entries are preserved.""" | ||
| content = "line 1\nline 2\nline 3" | ||
| store.add("memory", content) | ||
| assert content in store.memory_entries |
There was a problem hiding this comment.
This test doesn’t actually exercise the delimiter edge case on disk. MemoryStore._read_file() splits on every "§", so an entry like "entry with § symbol" will round-trip back as multiple entries after save_to_disk() + load_from_disk(). Right now the test only checks the in-memory list immediately after add(), which will still pass even though persistence is broken. I’d either change this to a save/reload assertion or drop the claim that delimiter handling is covered.
There was a problem hiding this comment.
I implemented the save/load round-trip coverage and the corresponding persistence fix in commit 3e3bf2b9 on hermes-jr1/hermes-agent:pr-123-review-feedback. The PR head fork has maintainerCanModify=false and denied push access for hermes-jr1, so I could not push directly to Bartok9:test-memory-tool.
Changes:
MemoryStore._read_file()now splits on the fullENTRY_DELIMITER(\n§\n) instead of every§character.test_entry_with_delimiter_charnow verifies a save/reload round trip.
Verification: python3 -m pytest tests/tools/test_memory_tool.py::TestEdgeCases::test_entry_with_delimiter_char tests/tools/test_memory_tool.py::TestMemoryStorePersistence::test_load_existing_entries tests/tools/test_memory_tool.py -q → 55 passed.
|
Reviewed locally. One substantive gap:
Everything else in the PR looked reasonable for the intended test coverage. |
Add a --worktree (-w) flag to the hermes CLI that creates an isolated git worktree for the session. This allows running multiple hermes-agent instances concurrently on the same repo without file collisions. How it works: - On startup with -w: detects git repo, creates .worktrees/<session>/ with its own branch (hermes/<session-id>), sets TERMINAL_CWD to it - Each agent works in complete isolation — independent HEAD, index, and working tree, shared git object store - On exit: auto-removes worktree and branch if clean, warns and keeps if there are uncommitted changes - .worktreeinclude file support: list gitignored files (.env, .venv/) to auto-copy/symlink into new worktrees - .worktrees/ is auto-added to .gitignore - Agent gets a system prompt note about the worktree context - Config support: set worktree: true in config.yaml to always enable Usage: hermes -w # Interactive mode in worktree hermes -w -q "Fix issue NousResearch#123" # Single query in worktree # Or in config.yaml: worktree: true Includes 17 tests covering: repo detection, worktree creation, independence verification, cleanup (clean/dirty), .worktreeinclude, .gitignore management, and 10 concurrent worktrees. Closes NousResearch#652
|
Addressed the delimiter persistence review feedback, but GitHub will not let I pushed the fix branch here instead: https://github.com/hermes-jr1/hermes-agent/tree/pr-123-review-feedback Summary:
Verification:
|
Add a --worktree (-w) flag to the hermes CLI that creates an isolated git worktree for the session. This allows running multiple hermes-agent instances concurrently on the same repo without file collisions. How it works: - On startup with -w: detects git repo, creates .worktrees/<session>/ with its own branch (hermes/<session-id>), sets TERMINAL_CWD to it - Each agent works in complete isolation — independent HEAD, index, and working tree, shared git object store - On exit: auto-removes worktree and branch if clean, warns and keeps if there are uncommitted changes - .worktreeinclude file support: list gitignored files (.env, .venv/) to auto-copy/symlink into new worktrees - .worktrees/ is auto-added to .gitignore - Agent gets a system prompt note about the worktree context - Config support: set worktree: true in config.yaml to always enable Usage: hermes -w # Interactive mode in worktree hermes -w -q "Fix issue NousResearch#123" # Single query in worktree # Or in config.yaml: worktree: true Includes 17 tests covering: repo detection, worktree creation, independence verification, cleanup (clean/dirty), .worktreeinclude, .gitignore management, and 10 concurrent worktrees. Closes NousResearch#652
Add a --worktree (-w) flag to the hermes CLI that creates an isolated git worktree for the session. This allows running multiple hermes-agent instances concurrently on the same repo without file collisions. How it works: - On startup with -w: detects git repo, creates .worktrees/<session>/ with its own branch (hermes/<session-id>), sets TERMINAL_CWD to it - Each agent works in complete isolation — independent HEAD, index, and working tree, shared git object store - On exit: auto-removes worktree and branch if clean, warns and keeps if there are uncommitted changes - .worktreeinclude file support: list gitignored files (.env, .venv/) to auto-copy/symlink into new worktrees - .worktrees/ is auto-added to .gitignore - Agent gets a system prompt note about the worktree context - Config support: set worktree: true in config.yaml to always enable Usage: hermes -w # Interactive mode in worktree hermes -w -q "Fix issue NousResearch#123" # Single query in worktree # Or in config.yaml: worktree: true Includes 17 tests covering: repo detection, worktree creation, independence verification, cleanup (clean/dirty), .worktreeinclude, .gitignore management, and 10 concurrent worktrees. Closes NousResearch#652
Darling reported that Feishu showed Excel-analysis responses as a
greyed wall of unparsed text. Root cause: ``_build_outbound_payload``
only routed through Feishu's post envelope (the one that renders
markdown) when ``_MARKDOWN_HINT_RE`` or ``_MARKDOWN_TABLE_RE`` matched,
i.e. when the reply contained explicit markdown markers like ``**``,
``-``, fenced code, or pipe tables.
The agent's typical data-analysis reply in Chinese looks like
数据形状:1000 行 × 5 列
列:销售额、日期、产品、地区
缺失率最高的列是「地区」(15%)
— no asterisks, no leading dashes, no English colons. Everything is
``label:value`` with the full-width Chinese colon. The hint regex
ignores it, the table regex ignores it, and the adapter dropped the
reply into Feishu's plain ``text`` envelope. Feishu rendered it as one
wrapped greyed line with no formatting.
Add a third route: ``_looks_structured(content)`` returns True when
the reply has 2+ non-empty lines and at least 2 of them match a
``\S[^::]{0,40}[::]\s*\S`` shape (label + value, ASCII or full-width
colon). The threshold of 2 stops a single "Re: ticket NousResearch#123" from
hijacking conversational replies into post mode, while catching every
realistic data-analysis preamble.
The existing fallback in ``send_chunks`` already retries as text if
the post path returns ``_POST_CONTENT_INVALID_RE``, so the broadened
routing has a safety net.
13 new tests in ``test_feishu_markdown_routing.py`` pin both the
heuristic (single-line, empty, multiline-without-colons, one-colon-
line all stay False; Chinese and ASCII colon pairs return True) and
the integration through ``_build_outbound_payload`` (short prose →
text, structured Chinese → post, markdown markers → post).
在main函数中添加了一个新的参数session_id,默认值为None。这个参数可以在调用main函数时传入,用于标识特定的会话。 feat(test.py): 修改测试用例以包含session_id 在test.py的主测试用例中,将query参数从“今天上海天气怎么样?另外有哪些新闻?”修改为“我生日是什么时候”。同时,添加了session_id参数,并赋值为"20260522_133847_d802cb"。这些修改是为了测试新的session_id功能。 Closes NousResearch#123
…sterisks The desktop markdown preprocessor autolinks bare URLs by wrapping them in <...>. RAW_URL_RE allowed '*' in its character classes, so a bold line with a URL and no separating space — e.g. '**PR opened: https://.../pull/123**' — greedily pulled the closing '**' into the href, producing a broken link and an unterminated bold run. Exclude '*' from both URL character classes; '_' and '~' (which can appear in real paths) are preserved.
…sterisks The desktop markdown preprocessor autolinks bare URLs by wrapping them in <...>. RAW_URL_RE allowed '*' in its character classes, so a bold line with a URL and no separating space — e.g. '**PR opened: https://.../pull/123**' — greedily pulled the closing '**' into the href, producing a broken link and an unterminated bold run. Exclude '*' from both URL character classes; '_' and '~' (which can appear in real paths) are preserved.
…sterisks (#41093) The desktop markdown preprocessor autolinks bare URLs by wrapping them in <...>. RAW_URL_RE allowed '*' in its character classes, so a bold line with a URL and no separating space — e.g. '**PR opened: https://.../pull/123**' — greedily pulled the closing '**' into the href, producing a broken link and an unterminated bold run. Exclude '*' from both URL character classes; '_' and '~' (which can appear in real paths) are preserved.
Add a --worktree (-w) flag to the hermes CLI that creates an isolated git worktree for the session. This allows running multiple hermes-agent instances concurrently on the same repo without file collisions. How it works: - On startup with -w: detects git repo, creates .worktrees/<session>/ with its own branch (hermes/<session-id>), sets TERMINAL_CWD to it - Each agent works in complete isolation — independent HEAD, index, and working tree, shared git object store - On exit: auto-removes worktree and branch if clean, warns and keeps if there are uncommitted changes - .worktreeinclude file support: list gitignored files (.env, .venv/) to auto-copy/symlink into new worktrees - .worktrees/ is auto-added to .gitignore - Agent gets a system prompt note about the worktree context - Config support: set worktree: true in config.yaml to always enable Usage: hermes -w # Interactive mode in worktree hermes -w -q "Fix issue NousResearch#123" # Single query in worktree # Or in config.yaml: worktree: true Includes 17 tests covering: repo detection, worktree creation, independence verification, cleanup (clean/dirty), .worktreeinclude, .gitignore management, and 10 concurrent worktrees. Closes NousResearch#652
…sterisks (NousResearch#41093) The desktop markdown preprocessor autolinks bare URLs by wrapping them in <...>. RAW_URL_RE allowed '*' in its character classes, so a bold line with a URL and no separating space — e.g. '**PR opened: https://...NousResearch/pull/123**' — greedily pulled the closing '**' into the href, producing a broken link and an unterminated bold run. Exclude '*' from both URL character classes; '_' and '~' (which can appear in real paths) are preserved.
Summary
Adds comprehensive test coverage for the memory tool module (55 tests).
Test Coverage
What's Tested
Running Tests
All 55 tests pass.