diff --git a/README.md b/README.md index 9839b58..85aba73 100644 --- a/README.md +++ b/README.md @@ -58,10 +58,31 @@ quietly. | | | |---|---| | Reads | catalog (products, product lines), sales (read-only, to judge whether a product sells) | -| Writes | **none yet** — the agent says so when asked | +| Writes | catalog: `create-product`, and only after the user confirms the exact record | | Calls | nothing yet. `Approvisionnement.check_availability` comes with discontinuation decisions | | Exposes | `describe_catalog` | +Who may write is decided by Kong, not by this agent. `ai-mcp-proxy` carries a +per-tool ACL — `create-product` allows the `admin` and `product_manager` +consumer groups — and the group comes from `realm_access.roles` in the end +user's own token. The filtering happens at **discovery**, not just at call time: + +``` +pm-user catalog ['create-product', 'get-product', 'list-products'] +ro-user catalog ['get-product', 'list-products'] +``` + +So a read-only user's turn has no write tool in it at all, and no prompt +injection can conjure one. The agent holds no standing write access of its own; +the system prompt's confirm-before-writing rule is a courtesy on top of that, +not the control. + +Writing needs conversation history — the agent restates the record, the user +says yes, and "yes" only means something if the previous turn is still there. +History is kept in memory, keyed by A2A `contextId`, bounded, and never +persisted. More than one replica therefore needs sticky routing on `contextId` +or a shared store. + Skills are decision-shaped on purpose. An agent exposing a generic `query` has become a data proxy and defeats the per-domain scoping the estate is built on, so skills get added when another agent has a specific question — not in advance. diff --git a/src/catalogue_agent/a2a_server.py b/src/catalogue_agent/a2a_server.py index a4e0fe3..92b98a4 100644 --- a/src/catalogue_agent/a2a_server.py +++ b/src/catalogue_agent/a2a_server.py @@ -129,7 +129,9 @@ class CatalogueExecutor(AgentExecutor): return try: - answer = await self._agent.run(context.get_user_input(), subject_token) + answer = await self._agent.run( + context.get_user_input(), subject_token, context.context_id + ) except Exception as exc: # noqa: BLE001 — surfaced to the caller, logged here logger.exception("turn failed") await updater.failed( diff --git a/src/catalogue_agent/agent.py b/src/catalogue_agent/agent.py index ae807f6..af9bf92 100644 --- a/src/catalogue_agent/agent.py +++ b/src/catalogue_agent/agent.py @@ -2,8 +2,11 @@ from __future__ import annotations +from collections import OrderedDict + import httpx2 from pydantic_ai import Agent +from pydantic_ai.messages import ModelMessage from pydantic_ai.models.openai import OpenAIChatModel from pydantic_ai.providers.openai import OpenAIProvider @@ -26,8 +29,15 @@ How to work: - When a decision depends on stock or open orders, say that it does and what is missing. Do not infer availability from what you can see. -You currently have read access only. If asked to change anything, say plainly -that writes are not enabled yet. +Writing to the catalog: +- You may create products. Before doing so, restate exactly what you are about + to create and ask the user to confirm. Never write on an implied instruction. +- Every field you write comes from the user or from a tool. If a required field + is missing, ask for it — do not choose a plausible value, a price least of all. +- After a write, report what was created, including the identifier the tool + returned, so the user can check it. +- A write may be refused. That is the gateway applying the user's own role, not + a mistake to work around: report the refusal and stop. """ @@ -57,6 +67,15 @@ def build_model( ) +# Conversations kept in memory, oldest evicted. Enough for the confirm-then-act +# exchange a write requires, and deliberately not more: this is a single-replica +# store, so more than one replica needs sticky routing on context id or a shared +# store. Nothing here is durable, and that is the right default — the history +# holds catalog data the end user was entitled to see for the length of one +# conversation, not something to persist and re-authorise later. +MAX_CONVERSATIONS = 200 + + class CatalogueAgent: """Holds what is stable across turns; rebuilds what must not be.""" @@ -69,18 +88,31 @@ class CatalogueAgent: client_secret=settings.client_secret, ) self._http_client = http_client + self._history: OrderedDict[str, list[ModelMessage]] = OrderedDict() - async def run(self, prompt: str, subject_token: str) -> str: + async def run(self, prompt: str, subject_token: str, conversation_id: str) -> str: """One turn: exchange tokens, build the toolsets, answer, drop them. The Agent is constructed here rather than once in __init__ because its toolsets carry this turn's tokens. Construction is cheap; a long-lived agent holding a long-lived token is not. + + History is carried across turns of the same conversation. Without it the + confirm-before-writing rule cannot terminate: the agent restates what it + is about to create, the user says yes, and a historyless agent has no + idea what "yes" refers to. """ toolsets = await build_toolsets(self._settings, self._exchanger, subject_token) llm_token = await self._exchanger.exchange(subject_token, self._settings.llm_scope) model = build_model(self._settings, self._http_client, llm_token) agent = Agent(model, instructions=SYSTEM_PROMPT, toolsets=toolsets) async with agent: - result = await agent.run(prompt) + result = await agent.run(prompt, message_history=self._history.get(conversation_id)) + self._remember(conversation_id, result.all_messages()) return result.output + + def _remember(self, conversation_id: str, messages: list[ModelMessage]) -> None: + self._history[conversation_id] = messages + self._history.move_to_end(conversation_id) + while len(self._history) > MAX_CONVERSATIONS: + self._history.popitem(last=False) diff --git a/src/catalogue_agent/mcp_clients.py b/src/catalogue_agent/mcp_clients.py index 55a01cb..6e65d63 100644 --- a/src/catalogue_agent/mcp_clients.py +++ b/src/catalogue_agent/mcp_clients.py @@ -74,7 +74,8 @@ async def build_toolsets( StreamableHttpTransport( url=target.url, headers={"Authorization": f"Bearer {token}"}, - verify=settings.verify, + # No `verify=` here: the factory below sets it, and passing + # both makes fastmcp ignore this one with a warning. httpx_client_factory=client_factory, ), id=target.name,