diff --git a/CHANGELOG.md b/CHANGELOG.md index 1d43c8c..e91fe75 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -227,6 +227,18 @@ This project follows [Semantic Versioning](https://semver.org/spec/v2.0.0.html). `app_state` / `app_client` fixtures, and CI runs the server suite file by file so the whole class fails loudly instead of being averaged away by test ordering. +- **The MCP binding can act as a real agent** (`AMP_MCP_AGENT_ID`). Every access + rule is decided from the caller's identity, and this binding hard-coded + `mcp_client` - so every MCP client pointed at one store was the same agent: a + cell one of them created was readable by the others, `readable_by` patterns + naming real agent ids matched nothing, and `identity.created_by` recorded a name + no agent uses, which is the attribution RFC-AMP-001 §5 relies on to make a + poisoned memory traceable. A caller passing `readable_by` to `amp_remember` was + in the worst spot: it stored a cell its own server could not recall. The identity + is now read from the environment, one value per MCP server, defaulting to the old + shared name so nothing breaks - and the getting-started config and the example + `mcp_config.json` both set it. + ### Changed - **Every endpoint returns one error shape.** `PATCH /memories/{id}` answered a conflict with `{"detail": ...}` while `DELETE` answered with diff --git a/docs/getting-started.md b/docs/getting-started.md index 2037259..6f8a053 100644 --- a/docs/getting-started.md +++ b/docs/getting-started.md @@ -152,13 +152,21 @@ Add the following JSON snippet to your `claude_desktop_config.json` (typically l "command": "python", "args": ["-m", "amp_server.mcp_server"], "env": { - "AMP_PERSIST_DIR": "C:\\path\\to\\your\\persistent\\dir" + "AMP_PERSIST_DIR": "C:\\path\\to\\your\\persistent\\dir", + "AMP_MCP_AGENT_ID": "claude-desktop" } } } } ``` +`AMP_MCP_AGENT_ID` is the agent this MCP server acts as. Set it to something that +names the client, and set a different value for each one you run: every access rule +is decided from this identity, so a shared value means every MCP client pointed at +the same store is the same agent - cells one of them created are readable by the +others, and `readable_by` patterns naming real agent ids never match. Unset, it +falls back to `mcp_client`, which is a shared namespace rather than an identity. + > [!IMPORTANT] > The command must be run in an environment where the `amp-server` package (containing the `amp_server` module) is installed. Ensure your python environment path or active virtual environment is correctly accessible to the command. diff --git a/docs/release-notes-v0.1.0.md b/docs/release-notes-v0.1.0.md index c5836f5..eb44094 100644 --- a/docs/release-notes-v0.1.0.md +++ b/docs/release-notes-v0.1.0.md @@ -97,6 +97,10 @@ Stated plainly rather than left for you to discover: was developed on has no PostgreSQL, so the adapter's proof is the CI service container. Run `pytest tests/test_adapter_contract.py` with `AMP_TEST_POSTGRES_DSN` set to check it against yours. +- **The MCP binding's identity is shared by default.** One MCP server acts as one + agent (`AMP_MCP_AGENT_ID`); unset, every MCP client pointed at the same store is + the same agent, so per-agent `readable_by` is only meaningful once you set it - + and run one server per agent. - **Some state is per process.** The scoring-edit budget lives in the server process, so two processes over one database keep two budgets. The retention purge is likewise an in-process scheduled pass. diff --git a/examples/mcp-claude-desktop/mcp_config.json b/examples/mcp-claude-desktop/mcp_config.json index 7f3b8b7..461e524 100644 --- a/examples/mcp-claude-desktop/mcp_config.json +++ b/examples/mcp-claude-desktop/mcp_config.json @@ -4,7 +4,8 @@ "command": "python", "args": ["-m", "amp_server.mcp_server"], "env": { - "AMP_PERSIST_DIR": "/absolute/path/to/a/persistent/data/directory" + "AMP_PERSIST_DIR": "/absolute/path/to/a/persistent/data/directory", + "AMP_MCP_AGENT_ID": "claude-desktop" } } } diff --git a/server/amp_server/mcp_server.py b/server/amp_server/mcp_server.py index 42ccbf4..9e6cdc9 100644 --- a/server/amp_server/mcp_server.py +++ b/server/amp_server/mcp_server.py @@ -26,6 +26,29 @@ # Create the FastMCP server instance mcp = FastMCP("AMP") +#: The identity this MCP server acts as when `AMP_MCP_AGENT_ID` is unset. +DEFAULT_AGENT_ID = "mcp_client" + + +def agent_id() -> str: + """Which agent this MCP server acts as. + + Every access rule is decided from this identity, so one shared default means + every MCP client pointed at the same store is the same agent: a cell one of + them created is readable by all of them, `readable_by` patterns naming real + agent ids never match anything a caller wrote, and `identity.created_by` - + the attribution RFC-AMP-001 §5 leans on to make a poisoned memory traceable - + records a name no agent uses. A caller who passes `readable_by` to + `amp_remember` is in the worst spot: the cell it just stored is one its own + server may not be able to recall. + + Run one MCP server per agent and set `AMP_MCP_AGENT_ID` in its `mcp_config.json` + `env` block. Read per call rather than cached at import, because the client + sets the environment when it spawns this process. + """ + return os.environ.get("AMP_MCP_AGENT_ID") or DEFAULT_AGENT_ID + + # Storage injection holder for testing _storage: ChromaAdapter | None = None @@ -61,7 +84,7 @@ async def amp_remember( identity=MemoryIdentity( owner_id=owner_id, owner_type=OwnerType.USER, - created_by="mcp_client", + created_by=agent_id(), ), lifecycle=MemoryLifecycle( created_at=datetime.now(UTC), @@ -89,7 +112,7 @@ async def amp_recall( limit=limit, include_stale=include_stale, ) - results = await storage.search(request, agent_id="mcp_client") + results = await storage.search(request, agent_id=agent_id()) if not results: return "No memories found." @@ -114,7 +137,7 @@ async def amp_forget(memory_id: str, owner_id: str) -> str: if cell.identity.owner_id != owner_id: return "Memory not found." - if not check_write_access(cell, "mcp_client"): + if not check_write_access(cell, agent_id()): return "Memory not found." try: @@ -146,7 +169,7 @@ async def amp_list_memories( limit=limit, ) - allowed_cells = [c for c in cells if check_read_access(c, "mcp_client")] + allowed_cells = [c for c in cells if check_read_access(c, agent_id())] if not allowed_cells: return "No memories found." diff --git a/server/tests/test_mcp_tools.py b/server/tests/test_mcp_tools.py index 30e4e31..509b552 100644 --- a/server/tests/test_mcp_tools.py +++ b/server/tests/test_mcp_tools.py @@ -6,6 +6,7 @@ from conftest import make_cell from amp_server.mcp_server import ( + DEFAULT_AGENT_ID, amp_forget, amp_list_memories, amp_recall, @@ -168,3 +169,75 @@ async def test_amp_list_memories_success(storage): # List with no matches res_empty = await amp_list_memories(owner_id="user-456") assert res_empty == "No memories found." + + +# --------------------------------------------------------------------------- +# The identity the binding acts as +# --------------------------------------------------------------------------- + + +@pytest.mark.asyncio +async def test_the_configured_identity_is_recorded_as_created_by(storage, monkeypatch): + """`created_by` is what makes a memory attributable; a shared name is not that.""" + monkeypatch.setenv("AMP_MCP_AGENT_ID", "agent-research") + + result = await amp_remember(content="a fact", owner_id="user-123") + cell = await storage._get_raw(result.split(": ")[1]) + + assert cell.identity.created_by == "agent-research" + + +@pytest.mark.asyncio +async def test_the_default_identity_is_used_when_none_is_configured( + storage, monkeypatch +): + monkeypatch.delenv("AMP_MCP_AGENT_ID", raising=False) + + result = await amp_remember(content="a fact", owner_id="user-123") + cell = await storage._get_raw(result.split(": ")[1]) + + assert cell.identity.created_by == DEFAULT_AGENT_ID + + +@pytest.mark.asyncio +async def test_a_cell_restricted_to_an_agent_is_reachable_when_it_is_that_agent( + storage, monkeypatch +): + """The point of the variable: per-agent policy has to work through this binding. + + `amp_remember` takes `readable_by`. With one shared identity, a caller that + restricts a cell to a specific agent stores something its own server cannot + recall - the restriction names an agent that, from this binding, never calls. + """ + monkeypatch.setenv("AMP_MCP_AGENT_ID", "agent-research") + stored = await amp_remember( + content="restricted fact", owner_id="user-123", readable_by=["agent-research"] + ) + + recalled = await amp_recall(query="restricted fact", owner_id="user-123") + + assert "restricted fact" in recalled + + # And another identity cannot read it. + monkeypatch.setenv("AMP_MCP_AGENT_ID", "agent-somebody-else") + assert await amp_recall(query="restricted fact", owner_id="user-123") == ( + "No memories found." + ) + assert stored.startswith("Memory stored: mem_") + + +@pytest.mark.asyncio +async def test_forget_is_gated_on_the_configured_identity(storage, monkeypatch): + monkeypatch.setenv("AMP_MCP_AGENT_ID", "agent-research") + cell = make_cell( + owner_id="user-123", + created_by="agent-research", + writable_by=["agent-research"], + ) + await storage.save(cell) + + monkeypatch.setenv("AMP_MCP_AGENT_ID", "agent-intruder") + assert await amp_forget(cell.id, owner_id="user-123") == "Memory not found." + + monkeypatch.setenv("AMP_MCP_AGENT_ID", "agent-research") + assert (await amp_forget(cell.id, owner_id="user-123")).startswith("Memory ")