fix: validate agent_id at LangGraph adapter agent-runtime concat sites#498
Merged
pandas-studio merged 4 commits intomainfrom Apr 26, 2026
Merged
Conversation
PR #491 / #493 wired ``validate_agent_id`` into every MCP + CLI surface that builds an ``agent-runtime:<agent_id>`` namespace, but the in-process LangGraph adapter still trusted callers. ``MemtomemStore.start_agent_session`` checked only ``if not agent_id`` and concatenated the value straight into the namespace, so a Python caller could land ``"agent-runtime:foo:bar"`` in storage even after the MCP / CLI gate shipped — exactly the parity gap the PR #491 review flagged. Apply ``validate_agent_id`` at ``start_agent_session`` (the only entry point that takes a caller-provided ``agent_id`` and concatenates it into the agent-runtime namespace; the other concat sites at ``_resolve_search_namespace`` / ``_resolve_add_namespace`` only read ``self._current_agent_id``, which now passes through the gate). Replace the local ``_AGENT_NAMESPACE_PREFIX = "agent-runtime:"`` literal with ``from memtomem.constants import AGENT_NAMESPACE_PREFIX, SHARED_NAMESPACE, validate_agent_id`` so the integration derives from the single source of truth instead of re-declaring the string (the redefinition predated the multi-agent shipping work and was kept on the assumption that integrations shouldn't import from MCP-side modules — but ``constants.py`` is intentionally above the MCP / CLI / integration split exactly so all three can derive from it; ``langgraph.py → constants.py → context._names`` stays one-way). Update ``test_empty_agent_id_raises`` to expect the new ``InvalidNameError`` ("invalid agent-id '': empty") and add a parametrised regression test covering the same hostile shapes pinned at the MCP / CLI boundaries (``foo:bar``, ``../etc``, ``a/b``, ``a b``, ``-leading-dash``). The new test also asserts ``storage.create_session`` is never awaited and ``_current_agent_id`` stays unset on the rejection path, so a regression to a warn-only gate would surface immediately. Closes #492. Co-Authored-By: Claude <[email protected]>
Two follow-ups from the PR #495 review: * Add a `Changed (BREAKING)` entry under `[Unreleased]` for the `MemtomemStore.start_agent_session` exception text change. The exception type narrows from `ValueError("agent_id must be a non-empty string")` to `InvalidNameError("invalid agent-id 'X': ...")` — `InvalidNameError` is a `ValueError` subclass so `except ValueError:` callers stay green, but any code substring-matching the old message text breaks. The CHANGELOG bullet calls out the migration ("switch to matching `invalid agent-id`") so a Python integrator sees the change without having to dig commit history. * Promote the "why downstream concat sites trust `_current_agent_id`" reasoning from the PR description into one-line docstring notes on `_resolve_search_namespace` and `_resolve_add_namespace`. Future readers shouldn't have to chase the PR to understand why these helpers don't re-run `validate_agent_id` before concatenating — `start_agent_session` is the sole writer of the field and the validator runs there. Co-Authored-By: Claude <[email protected]>
PR #493 narrowed the ``validate_agent_id`` docstring to "MCP + CLI surfaces" only, deferring the LangGraph claim until #492 actually wires the adapter. This commit lands on top of that wiring (commit 4633256), so the LangGraph adapter is now a real entry point — re-add it to the docstring's "Applied at" list and merge the pre-#492 / pre-#493 history into a single migration paragraph that covers both surfaces. The docstring stays accurate at every snapshot: pre-#492 it cited only the MCP + CLI surfaces, post-#492 it cites all three. No code change. Co-Authored-By: Claude <[email protected]>
4 tasks
PR #498 review nit: ``MemtomemStore.start_session`` is the legacy non-agent-aware sibling of ``start_agent_session`` and accepts ``agent_id`` un-validated. The reviewer correctly noted it doesn't concatenate into ``AGENT_NAMESPACE_PREFIX``, so the immediate "malformed agent-runtime: namespace in storage" threat is contained — but the ``agent_id`` field still lands in the sessions row as metadata, and a future analytics / migration path that reads it back into a namespace concat would silently reopen the gap. Document the invariant inline so the deferred decision shows up next to the code instead of only in PR review. New paths that derive a namespace from this metadata field must either route through ``start_agent_session`` or call ``validate_agent_id`` directly. Co-Authored-By: Claude <[email protected]>
memtomem
approved these changes
Apr 26, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Re-opening of PR #495 after #494 squash-merged its base branch (which auto-closes dependent PRs per
feedback_stacked_pr_base_delete_auto_close.md). Branch is rebased onto currentmain(post-#494) — review feedback already addressed inline; same 3-commit stack.MemtomemStore.start_agent_sessionwas the last entry point that builtagent-runtime:<agent_id>from caller input without runningvalidate_agent_id. Apply the gate so a Python caller can no longer land"agent-runtime:foo:bar"in storage even though the MCP / CLI surfaces (now fully gated post-fix: validate agent_id before agent-runtime namespace concat (CLI + MCP) #491 / fix: validate agent_id at mem_agent_register / search andmm agent register(#493) #494) refuse the same shape._AGENT_NAMESPACE_PREFIX = "agent-runtime:"literal with thememtomem.constantsimport (AGENT_NAMESPACE_PREFIX,SHARED_NAMESPACE,validate_agent_id) so the integration derives from the single source of truth — the redefinition predated the multi-agent shipping work and was kept on the assumption that integrations shouldn't import from MCP-side modules, butconstants.pyis intentionally above the MCP / CLI / integration split exactly so all three can derive from it.langgraph.py → constants.py → context._namesstays one-way.validate_agent_iddocstring (fix: validate agent_id at mem_agent_register / search andmm agent register(#493) #494 narrowed it pending this PR), so the "Applied at every surface" claim becomes accurate the moment this lands.CHANGELOG.md[Unreleased] > Changed (BREAKING)gets a new bullet for the exception narrowing —ValueError("agent_id must be a non-empty string")→InvalidNameError("invalid agent-id 'X': ...").InvalidNameErroris aValueErrorsubclass soexcept ValueError:callers stay green; substring-matching the old text is the only thing that breaks.The other concat sites in
langgraph.py(_resolve_search_namespace,_resolve_add_namespace) only readself._current_agent_id, which is set exclusively bystart_agent_session— gating that single entry point covers the whole agent-runtime concat path. One-line docstring notes promote that invariant from the PR description into the file so future readers don't have to dig.Issue #496 tracks the kin gap on caller-supplied
namespace=/target=overrides (Concern 3 from the prior #495 review).Test plan
uv run pytest packages/memtomem/tests/test_langgraph.py packages/memtomem/tests/test_validate_agent_id.py packages/memtomem/tests/test_multi_agent_integration.py packages/memtomem/tests/test_multi_agent.py packages/memtomem/tests/test_agent_cmd.py -m "not ollama"— 149 passeduv run ruff check+ruff format --checkon changed filestest_empty_agent_id_raisesupdated to expectInvalidNameError("invalid agent-id '': empty") rather than the old "non-empty"ValueErrortest_hostile_agent_id_blocked_before_storageparametrises the same hostile shapes pinned at the MCP / CLI boundaries (foo:bar,../etc,a/b,a b,-leading-dash) and assertsstorage.create_sessionis never awaited and_current_agent_idstays unset on the rejection pathCloses #492. Supersedes #495 (auto-closed by base-branch deletion on #494 squash-merge).
🤖 Generated with Claude Code