fix: validate agent_id at mem_agent_register / search and `mm agent register` (#493) by pandas-studio · Pull Request #494 · memtomem/memtomem

pandas-studio · 2026-04-26T04:56:34Z

Summary

Closes #493.

PR #491 wired validate_agent_id into the three session-start surfaces
(mem_session_start / mm session start / mm session wrap) so a
hostile-shaped agent_id like "foo:bar" or "../x" was rejected
loudly. The matching multi-agent registration / search surfaces
(mem_agent_register, mem_agent_search, mm agent register) still
ran sanitize_namespace_segment, so the same input produced two UX
outcomes — registering an agent silently rewrote the id to
agent-runtime:foo_bar, while mem_session_start for that same id
loudly rejected. This PR closes that asymmetry.

The read / write contract is now symmetric across the MCP + CLI
surfaces — an agent_id either works on every surface or fails on
every surface, with the same invalid agent-id 'X' error fragment
regardless of entry point. sanitize_namespace_segment itself stays
as the permissive helper for the ingest pipeline (where filesystem /
provider segments still need rewriting); only the multi-agent
surfaces stop calling it.

Breaking change

Callers that relied on silent sanitisation will now see a hard error.
Those callers were already storing memories under unexpected
namespaces (e.g. agent-runtime:foo_bar from agent_id="foo:bar")
so the change is the forcing function to pick a canonical id matching
[A-Za-z0-9._-]+. Flagged in CHANGELOG.md under
[Unreleased] > Changed (BREAKING).

What changed

packages/memtomem/src/memtomem/server/tools/multi_agent.py —
mem_agent_register and mem_agent_search call validate_agent_id
(raises InvalidNameError, surfaced as Error: ... by
tool_handler); drop the now-redundant non-empty / sanitised-empty
branches; drop the sanitize_namespace_segment import.
packages/memtomem/src/memtomem/cli/agent_cmd.py — mm agent register validates with the session-start try/except InvalidNameError → ClickException pattern.
packages/memtomem/src/memtomem/constants.py — validate_agent_id
docstring lists every MCP + CLI surface that now validates, points
at CHANGELOG.md Unreleased + mem_agent_register / mem_agent_search: validate vs sanitize divergence #493 for the breaking-change note,
and keeps the LangGraph adapter under "Not yet applied at" (tracked
in Apply validate_agent_id at LangGraph adapter agent-runtime concat sites #492 — out of scope here).
CHANGELOG.md — Unreleased > Changed (BREAKING) entry.

Tests

packages/memtomem/tests/test_validate_agent_id.py adds:
- TestMcpMultiAgentBoundary — parametrised over the existing
  HOSTILE_AGENT_IDS corpus (13 shapes); pins error string + that
  agent-runtime:foo:bar never reaches list_namespace_meta; valid
  ids still succeed; agent_id=None for mem_agent_search is
  unaffected.
- TestCliAgentRegisterBoundary — mm agent register foo:bar /
  ../etc reject; storage set_namespace_meta never called on the
  rejection path; valid id still succeeds.
- test_session_and_multi_agent_surfaces_share_core_error_text —
  cross-surface parity pin: all three boundaries emit
  invalid agent-id 'foo:bar'.
packages/memtomem/tests/test_agent_cmd.py::test_register_rejects_empty_agent_id
retargeted to the new shared error vocabulary (invalid agent-id)
- storage-not-called assertion.
packages/memtomem/tests/test_multi_agent.py docstring corrected —
the file now exercises the helper itself, not its (former) use by
the multi-agent tools.

Test plan

uv run ruff check packages/memtomem/src packages/memtomem/tests — clean
uv run ruff format --check packages/memtomem/src packages/memtomem/tests — clean
uv run pytest packages/memtomem/tests -m "not ollama" — 2589 passed, 46 deselected
Targeted: uv run pytest packages/memtomem/tests/test_validate_agent_id.py — 70 passed

Out of scope

LangGraph adapter parity for MemtomemStore.start_agent_session and
the remaining agent-runtime concat sites — tracked in Apply validate_agent_id at LangGraph adapter agent-runtime concat sites #492 as a
separate follow-up (review feedback: this PR shouldn't claim
LangGraph validation it doesn't ship).
mem_agent_share(target=...) validation — target is a full
namespace string (not a bare agent_id), so validate_agent_id
doesn't drop in directly. Currently a caller passing
target="agent-runtime:foo:bar" plumbs straight into
_mem_add_core(namespace=target) and stores under exactly the
hostile namespace this PR closes for register / search. Worth a
follow-up issue under the same mem_agent_register / mem_agent_search: validate vs sanitize divergence #493 banner — see review thread.
New "loose vs strict" config knob — adds an axis we don't actually
need until users ask for it (per mem_agent_register / mem_agent_search: validate vs sanitize divergence #493 acceptance).

Review fixups (post-#494 review)

94e9422 — drop LangGraph claim from validate_agent_id docstring.
The original PR docstring listed
integrations.langgraph.MemtomemStore.start_agent_session under
"Applied at", but the LangGraph wiring is not in this PR (it lives
in the stacked-PR work for Apply validate_agent_id at LangGraph adapter agent-runtime concat sites #492). Until Apply validate_agent_id at LangGraph adapter agent-runtime concat sites #492 lands this PR shouldn't
claim coverage it doesn't ship — the LangGraph mention is now under
"Not yet applied at" with a concrete pointer to Apply validate_agent_id at LangGraph adapter agent-runtime concat sites #492. The vague
"see issue tracker history" history pointer is also replaced with
a concrete CHANGELOG.md Unreleased + mem_agent_register / mem_agent_search: validate vs sanitize divergence #493 anchor.

🤖 Generated with Claude Code

…egister` (#493) PR #491 wired ``validate_agent_id`` into the three session-start surfaces (``mem_session_start``, ``mm session start``, ``mm session wrap``) so a hostile-shaped ``agent_id`` like ``"foo:bar"`` or ``"../x"`` is rejected loudly. The matching multi-agent registration / search surfaces still ran ``sanitize_namespace_segment``, so the same input produced two UX outcomes: registering an agent silently rewrote the id to ``agent-runtime:foo_bar``, while ``mem_session_start`` for that same id loudly rejected. The asymmetry was small while the multi-agent surface review was in flight, but it would have only gotten harder to reason about as more tools fan out. Switch ``mem_agent_register``, ``mem_agent_search``, and ``mm agent register`` to ``validate_agent_id``. The read / write contract is now symmetric — an id either works on every surface or fails on every surface, with the same ``invalid agent-id 'X'`` error fragment regardless of entry point. ``sanitize_namespace_segment`` itself stays as the permissive helper for the ingest pipeline (where filesystem / provider segments still need rewriting). CHANGELOG flags the breaking shape: callers that relied on silent sanitisation will now see a hard error. Those callers were already storing memories under unexpected namespaces so the fix is the forcing function to pick a canonical id. Tests: extends ``test_validate_agent_id.py`` with a ``TestMcpMultiAgentBoundary`` class (parametrised over the same hostile-id corpus as the session-start surfaces) and a ``TestCliAgentRegisterBoundary`` class for the CLI shape, plus a cross-surface parity assertion that all three multi-agent boundaries emit the same ``invalid agent-id 'foo:bar'`` core fragment. ``test_agent_cmd.py``'s empty-id assertion is updated for the new shared error vocabulary. Closes #493. Co-Authored-By: Claude <[email protected]>

PR #494 reviewer flagged: the docstring listed ``integrations.langgraph.MemtomemStore.start_agent_session`` as a site that validates, but the LangGraph wiring is not in this PR — it lives in the stacked-PR work for #492 (separate branch). Until #492 lands, this PR should not claim LangGraph validates. Move the LangGraph mention into a "Not yet applied at" paragraph, mirroring the structure on ``main`` pre-#493. Also replace the vague "see issue tracker history for the breaking-change note" with the concrete pointer (``CHANGELOG.md`` Unreleased entry + issue #493). No code change — pure docstring honesty fix. The stacked #492 PR will re-promote LangGraph to "Applied at" when it ships. Co-Authored-By: Claude <[email protected]>

pandas-studio · 2026-04-26T05:07:26Z

Thanks — addressed the blocking review item:

#1 (LangGraph overclaim) — fixed in 94e9422

You were right, the docstring as written claimed coverage this PR doesn't ship. Moved the LangGraph mention back under Not yet applied at (mirroring main pre-#493) and pointed at #492 explicitly. Also replaced the vague "see issue tracker history" pointer with a concrete CHANGELOG.md Unreleased + #493 anchor. The stacked-PR work for #492 will re-promote LangGraph to Applied at when it lands. Updated the PR description to drop the matching "LangGraph adapter unaffected (already validates as of #491 land)" sentence — same overclaim.

#2 (mem_agent_share target validation) — filed as #497

target= is a full namespace string (not a bare agent_id), so validate_agent_id doesn't drop in directly — needs a "split prefix, validate the suffix when prefix == AGENT_NAMESPACE_PREFIX" gate. Sibling gap to #493 but worth its own scope. Acceptance criteria mirror this PR (cross-surface error parity, storage never sees malformed namespace, docstring update). Linked from this PR's Out of scope list.

#3 (vague history pointer) — fixed in 94e9422 alongside #1.

#4 (validate return value discarded) and #5 (test error string change polish) — leaving as-is per your "acceptable / optional" verdict; the cross-surface error vocabulary unification was the intentional outcome and the empty-id case is covered transitively by the BREAKING entry's "contains... whitespace" line.

Ready for re-review when you have a minute.

This was referenced Apr 26, 2026

fix: validate agent_id at LangGraph adapter agent-runtime concat sites #495

Closed

Validate user-supplied namespace= overrides on agent session entry points #496

Closed

pandas-studio mentioned this pull request Apr 26, 2026

mem_agent_share: validate agent-runtime: target's agent_id segment #497

Closed

memtomem approved these changes Apr 26, 2026

View reviewed changes

pandas-studio merged commit 7ea138d into main Apr 26, 2026
7 checks passed

pandas-studio deleted the fix/issue-493-validate-agent-tools branch April 26, 2026 05:14

github-actions Bot locked and limited conversation to collaborators Apr 26, 2026

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: validate agent_id at mem_agent_register / search and `mm agent register` (#493)#494

fix: validate agent_id at mem_agent_register / search and `mm agent register` (#493)#494
pandas-studio merged 2 commits intomainfrom
fix/issue-493-validate-agent-tools

pandas-studio commented Apr 26, 2026 •

edited

Loading

Uh oh!

pandas-studio commented Apr 26, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

pandas-studio commented Apr 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Breaking change

What changed

Tests

Test plan

Out of scope

Review fixups (post-#494 review)

Uh oh!

pandas-studio commented Apr 26, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

pandas-studio commented Apr 26, 2026 •

edited

Loading