fix: validate agent_id at LangGraph adapter agent-runtime concat sites#495
Closed
pandas-studio wants to merge 3 commits intofix/issue-493-validate-agent-toolsfrom
Closed
Conversation
memtomem
pushed a commit
that referenced
this pull request
Apr 26, 2026
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 #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]>
memtomem
force-pushed
the
fix/issue-492-langgraph-validate-agent-id
branch
from
a2cfbdf to
9123fab
Compare
Collaborator
Author
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.
1 participant
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
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 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.mm agent register(#493) #494 (mem_agent_register / mem_agent_search: validate vs sanitize divergence #493) so thevalidate_agent_iddocstring there ("Applied at every surface that concatenatesAGENT_NAMESPACE_PREFIXwith caller input") becomes accurate the moment both PRs land.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.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 the two 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.
🤖 Generated with Claude Code