Skip to content

test(tools): add unit tests for memory_tool.py#123

Closed
Bartok9 wants to merge 1 commit into
NousResearch:mainfrom
Bartok9:test-memory-tool
Closed

test(tools): add unit tests for memory_tool.py#123
Bartok9 wants to merge 1 commit into
NousResearch:mainfrom
Bartok9:test-memory-tool

Conversation

@Bartok9

@Bartok9 Bartok9 commented Feb 27, 2026

Copy link
Copy Markdown
Contributor

Summary

Adds comprehensive test coverage for the memory tool module (55 tests).

Test Coverage

Category Tests
MemoryStore initialization 3
Persistence (load/save) 5
add() operation 8
replace() operation 6
remove() operation 4
Security scanning 9
System prompt snapshot 4
memory_tool() dispatcher 9
Edge cases 5

What's Tested

  • Initialization: Default and custom char limits
  • Persistence: File creation, atomic writes, deduplication on load
  • CRUD operations: Success paths, error handling, boundary conditions
  • Security: Prompt injection, role hijacking, exfiltration patterns, invisible unicode
  • Frozen snapshot: Ensures system prompt stability (mutations don't affect snapshot)
  • Edge cases: Delimiter character handling, multiline entries, unicode, exact limits

Running Tests

python -m pytest tests/tools/test_memory_tool.py -v

All 55 tests pass.

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.
@teknium1 teknium1 closed this Feb 27, 2026
teknium1 added a commit that referenced this pull request Mar 8, 2026
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 RuckVibeCodes left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[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

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 full ENTRY_DELIMITER (\n§\n) instead of every § character.
  • test_entry_with_delimiter_char now 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.

@veteranbv

Copy link
Copy Markdown

Reviewed locally. One substantive gap:

  • The new delimiter edge-case test doesn't verify the save/load round trip, so it misses a real persistence bug in MemoryStore._read_file() when content itself contains §. I left an inline comment on that test with the details.

Everything else in the PR looked reasonable for the intended test coverage.

angelburgosrosado pushed a commit to angelburgosrosado/hermes-agent that referenced this pull request Apr 27, 2026
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
@hermes-jr1

Copy link
Copy Markdown

Addressed the delimiter persistence review feedback, but GitHub will not let hermes-jr1 push directly to the PR branch because this is a fork PR with maintainerCanModify=false and Bartok9/hermes-agent denied write access.

I pushed the fix branch here instead: https://github.com/hermes-jr1/hermes-agent/tree/pr-123-review-feedback
Commit: hermes-jr1@3e3bf2b9

Summary:

  • MemoryStore._read_file() now splits on the full ENTRY_DELIMITER (\n§\n) instead of every § character, preserving literal section signs in entries.
  • The delimiter edge-case test now saves and reloads before asserting the entry round-trips intact.

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.

02356abc pushed a commit to 02356abc/hermes-agent that referenced this pull request May 14, 2026
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
olympus-terminal pushed a commit to olympus-terminal/hermes-agent that referenced this pull request May 16, 2026
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
AnxForever added a commit to AnxForever/hermes-agent that referenced this pull request May 19, 2026
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).
serendipity2654 added a commit to serendipity2654/hermes-agent that referenced this pull request Jun 2, 2026
在main函数中添加了一个新的参数session_id,默认值为None。这个参数可以在调用main函数时传入,用于标识特定的会话。

feat(test.py): 修改测试用例以包含session_id

在test.py的主测试用例中,将query参数从“今天上海天气怎么样?另外有哪些新闻?”修改为“我生日是什么时候”。同时,添加了session_id参数,并赋值为"20260522_133847_d802cb"。这些修改是为了测试新的session_id功能。

Closes NousResearch#123
teknium1 added a commit that referenced this pull request Jun 7, 2026
…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.
teknium1 added a commit that referenced this pull request Jun 7, 2026
…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.
teknium1 added a commit that referenced this pull request Jun 7, 2026
…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.
Egavasyug pushed a commit to Egavasyug/hermes-agent that referenced this pull request Jun 10, 2026
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
changman pushed a commit to changman/hermes-agent that referenced this pull request Jun 10, 2026
…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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants