feat: forward agentId on remember + Hermes multi-profile project tagging#1089
feat: forward agentId on remember + Hermes multi-profile project tagging#1089kaardilugeja wants to merge 6 commits into
Conversation
api::remember allowed project but dropped agentId even though mem::remember and session start already accept it. The standalone MCP memory_save path also omitted project and agentId when proxying to POST /remember, so tool-schema fields never persisted. - Allow-list agentId on api::remember (trim, max 128) - Forward agentId from in-process MCP memory_save - Standalone validate and proxy: project + agentId - Unit test: agentId persists on mem::remember Signed-off-by: Kaardilugeja <13065992+kaardilugeja@users.noreply.github.com>
Hermes profiles often share one agentmemory daemon. The stock plugin used cwd paths as project and never stamped agentId on remember writes. - Add scoping.py for stable slugs, work-root repo detection, profile agentId - Tag session/start, remember, observe, and on_memory_write - Keep smart-search unfiltered for shared multi-profile recall - Harden dotenv preload under skewed HOME / HERMES_HOME - Document env vars; add stdlib tests for scoping helpers Signed-off-by: Kaardilugeja <13065992+kaardilugeja@users.noreply.github.com>
|
@kaardilugeja is attempting to deploy a commit to the rohitg00's projects Team on Vercel. A member of the Team first needs to authorize it. |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughHermes now resolves project and agent identity from environment, profile, and workdir data, then propagates those values through sessions and memory writes. MCP and API memory-save paths validate, forward, and persist ChangesHermes scoping and memory tagging
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant MCPClient
participant MCPServer
participant RememberAPI
participant MemoryStore
MCPClient->>MCPServer: memory_save(project, agentId, content)
MCPServer->>RememberAPI: forward validated memory payload
RememberAPI->>MemoryStore: persist project and agentId
Possibly related issues
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
integrations/hermes/__init__.py (1)
386-405: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
prefetch(),queue_prefetch(), andon_session_end()readself._basewithout_ensure_scope_defaults().Every other hook (
system_prompt_block,handle_tool_call,sync_turn,on_pre_compress,on_memory_write) callsself._ensure_scope_defaults()before touching scope attributes — exactly to cover the "initialize was skipped (tests / odd hosts)" case documented on that method. These three still accessself._basedirectly, so if any of them runs beforeinitialize(), it raisesAttributeError(synchronously, forqueue_prefetch, sinceself._baseis evaluated in the caller's thread before_api_bgspawns).🛡️ Proposed fix
def prefetch(self, query: str, **kwargs: Any) -> str: + self._ensure_scope_defaults() # Shared-scope: do not hard-filter smart-search by project (v1). result = _api(self._base, "smart-search", { def queue_prefetch(self, query: str, **kwargs: Any) -> None: + self._ensure_scope_defaults() _api_bg(self._base, "smart-search", {"query": query, "limit": 3}) def on_session_end(self, messages: list, **kwargs: Any) -> None: + self._ensure_scope_defaults() _api(self._base, "session/end", {Also applies to: 550-553
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@integrations/hermes/__init__.py` around lines 386 - 405, Call self._ensure_scope_defaults() at the start of prefetch(), queue_prefetch(), and on_session_end() before accessing self._base or any scope attributes, preserving their existing behavior after defaults are initialized.
🧹 Nitpick comments (7)
integrations/hermes/scoping.py (2)
116-121: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winBroad
except Exception: passduplicates the pwd-lookup block in__init__.py.This swallows all errors (not just a missing
pwdmodule) and is flagged by ruff (S110/BLE001). The same pattern is re-implemented independently inintegrations/hermes/__init__.py(lines 110-117) for a slightly different purpose. Since this module is explicitly the "pure helpers" file that__init__.pyalready imports from, consider extracting a shared_pwd_home() -> str | Nonehelper here (narrowing the except to(ImportError, KeyError, AttributeError)) and having__init__.pyreuse it.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@integrations/hermes/scoping.py` around lines 116 - 121, Replace the broad exception handling around the pwd lookup in the scoping initialization with a shared _pwd_home() -> str | None helper in this pure-helpers module. Catch only ImportError, KeyError, and AttributeError, return None when lookup is unavailable, and update the __init__.py pwd-home logic to reuse this helper instead of duplicating the lookup block.Source: Linters/SAST tools
1-1: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDuplicate pwd-lookup broad-except block across
scoping.pyand__init__.py. Both files independently implementtry: import pwd; ...; except Exception: pass(flagged by ruff S110/BLE001 in both locations) for platform-specific home-directory lookup.
integrations/hermes/scoping.py#L116-121: extract a shared_pwd_home() -> str | Nonehelper here (this module is already the designated "pure helpers, safe to unit-test" module), narrowing the except to(ImportError, KeyError, AttributeError).integrations/hermes/__init__.py#L110-117: import and reuse the new_pwd_home()helper fromscoping.pyinstead of re-implementing the pwd lookup.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@integrations/hermes/scoping.py` at line 1, Extract the platform-specific pwd lookup from scoping.py into a shared _pwd_home() -> str | None helper, handling only ImportError, KeyError, and AttributeError. Update the home-directory resolution in __init__.py to import and reuse _pwd_home() instead of maintaining its duplicate broad-except implementation.Source: Linters/SAST tools
src/mcp/server.ts (1)
190-202: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winSame
agentIdsanitize duplication/inconsistency asstandalone.ts/api.ts.See consolidated comment (anchored at
src/triggers/api.ts).🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/mcp/server.ts` around lines 190 - 202, Reuse the shared agentId sanitization logic already established for standalone.ts/api.ts in the mem::remember trigger flow, instead of duplicating the inline validation and truncation in the surrounding handler. Update the payload construction near sdk.trigger so it uses the same normalized agentId behavior and remains consistent across all entry points.src/triggers/api.ts (2)
994-1028: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win
agentIdsanitize/validate logic duplicated across 3 files with inconsistent error semantics.This endpoint correctly rejects a malformed
agentId(non-string/empty) with a 400, butsrc/mcp/server.tsandsrc/mcp/standalone.tssilently drop the same invalid input instead of surfacing an error — and all three re-implement the identicalagentId.trim().slice(0, 128)truncation (a near-identical instance already exists atapi::session::startlines 605-608 in this same file). Extract a sharedsanitizeAgentId(value: unknown, maxLen = 128): string | undefined(or a{value, error}variant) used by each boundary's own validation, so behavior stays consistent as this pattern gets reused. SinceagentIdfeeds multi-agent isolation filtering elsewhere in this file, keeping truncation/validation consistent across all write paths matters for correctness, not just style.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/triggers/api.ts` around lines 994 - 1028, The agentId validation and truncation logic is duplicated across API and MCP boundaries with inconsistent invalid-input behavior. Extract a shared sanitizeAgentId helper, anchored to the existing api::session::start handling and the affected endpoint handlers, that validates non-string or blank values consistently, trims and limits valid values to 128 characters, and is used by src/triggers/api.ts, src/mcp/server.ts, and src/mcp/standalone.ts while preserving each boundary’s appropriate error response.Source: Coding guidelines
1-1: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win
agentIdsanitize/validate logic duplicated across three write-path boundaries with inconsistent error behavior. All three re-implementagentId.trim().slice(0, 128)for the new write-scoping feature, but only the REST layer rejects malformed input — the two MCP paths silently drop it instead of erroring, and a near-identical instance already exists atsrc/triggers/api.tsL605-608 (api::session::start).
src/triggers/api.ts#L994-1028: extract a sharedsanitizeAgentId(value: unknown, maxLen = 128)helper and reuse it here and at the existingapi::session::starttruncation (L605-608), keeping the existing 400-on-invalid behavior.src/mcp/server.ts#L190-202: call the same shared helper formemory_save's agentId handling so an invalidagentIdbehaves the same way it does via the REST endpoint.src/mcp/standalone.ts#L127-134: call the same shared helper in thememory_savevalidation branch for consistency with the other two boundaries.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/triggers/api.ts` at line 1, Extract a shared sanitizeAgentId helper that trims, limits to 128 characters, and rejects invalid values consistently. Reuse it in the REST write path and the existing api::session::start handling while preserving REST’s 400 response for invalid input. Apply the same helper in memory_save validation in both the MCP server and standalone paths so invalid agentId values produce validation errors instead of being silently dropped.Source: Coding guidelines
src/mcp/standalone.ts (1)
102-103: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win
agentId/projectsanitize logic duplicated across 3 boundaries with inconsistent error behavior.See consolidated comment (anchored at
src/triggers/api.ts) for the full picture acrosssrc/mcp/server.ts,src/mcp/standalone.ts, andsrc/triggers/api.ts.Also applies to: 127-134, 193-194
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/mcp/standalone.ts` around lines 102 - 103, Consolidate the agentId/project sanitization used by the standalone MCP boundary instead of maintaining duplicated logic across its request paths. Reuse the shared sanitizer and standardize invalid-input handling so all affected boundaries return the same error behavior, including the declaration and handling points shown in the diff.integrations/hermes/__init__.py (1)
110-117: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDuplicate pwd-lookup broad-except block (shared root cause with
scoping.py).Same
try: import pwd; ...; except Exception: passpattern (ruff S110/BLE001) asscoping.pylines 116-121, re-implemented here for dotenv candidate paths. See consolidated comment for the combined recommendation.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@integrations/hermes/__init__.py` around lines 110 - 117, The dotenv candidate construction in the Hermes initialization flow duplicates the broad-except pwd lookup used by scoping.py. Reuse the existing shared home-directory lookup/helper for the `pwd.getpwuid(os.getuid()).pw_dir` behavior, or consolidate both call sites into one helper, and remove this local `try/import pwd/except Exception: pass` block while preserving the candidate path when a home directory is available.Source: Linters/SAST tools
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@integrations/hermes/__init__.py`:
- Around line 257-264: Update _debug_log to avoid the fixed
/tmp/agentmemory_plugin_debug.log path: resolve a per-user private location
under HERMES_HOME or use a user-scoped temporary file, ensuring the file is
created safely without following attacker-controlled symlinks and that logging
remains best-effort.
---
Outside diff comments:
In `@integrations/hermes/__init__.py`:
- Around line 386-405: Call self._ensure_scope_defaults() at the start of
prefetch(), queue_prefetch(), and on_session_end() before accessing self._base
or any scope attributes, preserving their existing behavior after defaults are
initialized.
---
Nitpick comments:
In `@integrations/hermes/__init__.py`:
- Around line 110-117: The dotenv candidate construction in the Hermes
initialization flow duplicates the broad-except pwd lookup used by scoping.py.
Reuse the existing shared home-directory lookup/helper for the
`pwd.getpwuid(os.getuid()).pw_dir` behavior, or consolidate both call sites into
one helper, and remove this local `try/import pwd/except Exception: pass` block
while preserving the candidate path when a home directory is available.
In `@integrations/hermes/scoping.py`:
- Around line 116-121: Replace the broad exception handling around the pwd
lookup in the scoping initialization with a shared _pwd_home() -> str | None
helper in this pure-helpers module. Catch only ImportError, KeyError, and
AttributeError, return None when lookup is unavailable, and update the
__init__.py pwd-home logic to reuse this helper instead of duplicating the
lookup block.
- Line 1: Extract the platform-specific pwd lookup from scoping.py into a shared
_pwd_home() -> str | None helper, handling only ImportError, KeyError, and
AttributeError. Update the home-directory resolution in __init__.py to import
and reuse _pwd_home() instead of maintaining its duplicate broad-except
implementation.
In `@src/mcp/server.ts`:
- Around line 190-202: Reuse the shared agentId sanitization logic already
established for standalone.ts/api.ts in the mem::remember trigger flow, instead
of duplicating the inline validation and truncation in the surrounding handler.
Update the payload construction near sdk.trigger so it uses the same normalized
agentId behavior and remains consistent across all entry points.
In `@src/mcp/standalone.ts`:
- Around line 102-103: Consolidate the agentId/project sanitization used by the
standalone MCP boundary instead of maintaining duplicated logic across its
request paths. Reuse the shared sanitizer and standardize invalid-input handling
so all affected boundaries return the same error behavior, including the
declaration and handling points shown in the diff.
In `@src/triggers/api.ts`:
- Around line 994-1028: The agentId validation and truncation logic is
duplicated across API and MCP boundaries with inconsistent invalid-input
behavior. Extract a shared sanitizeAgentId helper, anchored to the existing
api::session::start handling and the affected endpoint handlers, that validates
non-string or blank values consistently, trims and limits valid values to 128
characters, and is used by src/triggers/api.ts, src/mcp/server.ts, and
src/mcp/standalone.ts while preserving each boundary’s appropriate error
response.
- Line 1: Extract a shared sanitizeAgentId helper that trims, limits to 128
characters, and rejects invalid values consistently. Reuse it in the REST write
path and the existing api::session::start handling while preserving REST’s 400
response for invalid input. Apply the same helper in memory_save validation in
both the MCP server and standalone paths so invalid agentId values produce
validation errors instead of being silently dropped.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 6a55e714-fa11-4e0e-a861-b64962a3a03d
📒 Files selected for processing (9)
integrations/hermes/README.mdintegrations/hermes/__init__.pyintegrations/hermes/scoping.pyintegrations/hermes/tests/test_scoping.pysrc/mcp/server.tssrc/mcp/standalone.tssrc/mcp/tools-registry.tssrc/triggers/api.tstest/remember-project-scope.test.ts
Majors: - Move plugin debug log off fixed /tmp path to HERMES_HOME or ~/.agentmemory with O_NOFOLLOW - Call _ensure_scope_defaults in prefetch/queue_prefetch/on_session_end Nitpicks: - Shared pwd_home() helper (narrow exceptions) for Hermes dotenv/work roots - Shared sanitizeAgentId / parseOptionalAgentIdField across REST, MCP, mem::remember, session start, and events - MCP memory_save rejects blank/non-string project/agentId; local mode persists both fields Also add method docstrings on Hermes provider for CodeRabbit coverage gate, plus unit tests for sanitizer, pwd_home, and MCP validation. Signed-off-by: Kaardilugeja <13065992+kaardilugeja@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
integrations/hermes/__init__.py (1)
141-145: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winFix exact string match in
path.partsthat inadvertently filters~/.agentmemory/.env.When reading
~/.agentmemory/.env,path.partscontains the exact string".agentmemory", not"agentmemory". Because the condition checks for exact inclusion,"agentmemory" not in path.partsevaluates toTrue. This causes~/.agentmemory/.envto be unintentionally filtered to onlyAGENTMEMORY_keys, while the XDG config path (~/.config/agentmemory/.env) escapes filtering because its directory part is exactly"agentmemory".Use a substring check to correctly exempt all agentmemory configuration directories.
🛠️ Proposed fix
# Only pull agentmemory-related keys from Hermes profile .env so # we never stomp unrelated shell credentials from that file. - if path.name == ".env" and "agentmemory" not in path.parts: + if path.name == ".env" and not any("agentmemory" in p for p in path.parts): if not key.startswith("AGENTMEMORY_"): continue🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@integrations/hermes/__init__.py` around lines 141 - 145, Update the Hermes .env filtering condition in the surrounding profile-loading logic to use a substring check on the relevant path component, so directories such as ".agentmemory" and "agentmemory" are both exempted. Preserve the existing AGENTMEMORY_ key filtering for unrelated Hermes .env files.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@integrations/hermes/__init__.py`:
- Around line 266-269: Update the fallback path in _debug_log_path() to access
os.getuid() safely on platforms where it is unavailable, using an appropriate
stable fallback identifier when necessary. Ensure initialize() and
handle_tool_call() can call _debug_log_path() on Windows without an
AttributeError, while preserving the existing UID-scoped path behavior on
platforms that provide getuid().
In `@src/triggers/api.ts`:
- Around line 607-608: Replace the sanitizeAgentId/getAgentId fallback in the
REST handler with parseOptionalAgentIdField so malformed or blank body.agentId
values produce a 400 Bad Request. Preserve the optional-field behavior for
omitted agentId values and align the handling with the existing api::remember
endpoint.
---
Outside diff comments:
In `@integrations/hermes/__init__.py`:
- Around line 141-145: Update the Hermes .env filtering condition in the
surrounding profile-loading logic to use a substring check on the relevant path
component, so directories such as ".agentmemory" and "agentmemory" are both
exempted. Preserve the existing AGENTMEMORY_ key filtering for unrelated Hermes
.env files.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 2861a6b4-8a7f-47fc-8503-dc8c0614b026
📒 Files selected for processing (12)
integrations/hermes/__init__.pyintegrations/hermes/scoping.pyintegrations/hermes/tests/test_scoping.pysrc/config.tssrc/functions/remember.tssrc/mcp/server.tssrc/mcp/standalone.tssrc/triggers/api.tssrc/triggers/events.tstest/agent-id-scope.test.tstest/mcp-standalone.test.tstest/remember-project-scope.test.ts
🚧 Files skipped from review as they are similar to previous changes (4)
- test/remember-project-scope.test.ts
- src/mcp/server.ts
- integrations/hermes/tests/test_scoping.py
- integrations/hermes/scoping.py
- Safe _debug_log_path fallback without os.getuid (Windows) - session/start: reject blank/malformed body.agentId with 400 via parseOptionalAgentIdField (match api::remember) - Dotenv preload: exempt .agentmemory and agentmemory path components via substring check so daemon .env loads full keys Signed-off-by: Kaardilugeja <13065992+kaardilugeja@users.noreply.github.com>
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/triggers/api.ts (1)
606-606: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRemove the explanatory WHAT-comment.
parseOptionalAgentIdFieldand the explicit error branch already make this behavior clear. Remove this comment to comply with the guideline: “In TypeScript source code, avoid code comments explaining WHAT — use clear naming instead.”🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/triggers/api.ts` at line 606, Remove the explanatory comment above the malformed or blank body.agentId handling in the API trigger flow; leave parseOptionalAgentIdField and its explicit error branch unchanged.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@src/triggers/api.ts`:
- Line 606: Remove the explanatory comment above the malformed or blank
body.agentId handling in the API trigger flow; leave parseOptionalAgentIdField
and its explicit error branch unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: bdfeabf9-33ac-43e1-b316-e2624286e2fb
📒 Files selected for processing (2)
integrations/hermes/__init__.pysrc/triggers/api.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- integrations/hermes/init.py
parseOptionalAgentIdField + 400 branch already express the contract. Signed-off-by: Kaardilugeja <13065992+kaardilugeja@users.noreply.github.com>
CodeRabbit pre-merge gate scored integrations/hermes/__init__.py at 61.29% (19/31). Add concise docstrings on remaining helpers, the provider class/name property, and register(). Signed-off-by: Kaardilugeja <13065992+kaardilugeja@users.noreply.github.com>
What
Two related write-path fixes so multi-agent / multi-profile setups can tag memories reliably:
POST /agentmemory/rememberand MCPmemory_savenow forwardagentId. Standalone MCP also forwardsproject(it was advertised in the tool schema but dropped in the REST body).integrations/hermesstamps stableproject+agentIdon session start, remember, observe, and memory-write hooks instead of using raw cwd paths asproject.Why
Shared daemons (e.g. several Hermes profiles on one host) need:
projectagentIdWithout this, MCP/plugin writes become unscoped orphans (they always pass project filters), and
agentIdprovenance is silently discarded on REST even when clients send it.mem::rememberand session start already supportedagentId; the HTTP/MCP allow-lists did not.Search stays unfiltered by default (shared soft-read). This PR only fixes write tagging.
How to verify
Manual (with a running server on
:3111):Standalone MCP
memory_savewithproject/agentIdargs should persist the same fields viaPOST /remember.Commits
fix(remember): forward agentId on REST and MCP remember pathsfeat(hermes): multi-profile project and agentId write taggingNotes
Summary by CodeRabbit
projectandagentIdsupport to memory-saving tools and APIs (including MCP schema/validation) with proper local stamping and request forwarding.agentIdparsing/validation across session start and remember flows.