feat: enable catalog writes, with the conversation history they require
The gateway already authorised writes for a product_manager — ai-mcp-proxy filters tool discovery by consumer group, so create-product is simply absent from a read-only user's turn. Only the system prompt was holding the agent back, and it now describes how to write instead: restate the record, ask for confirmation, never invent a missing field, report the identifier, and stop on a refusal rather than working around it. That flow cannot terminate without history. Each turn built a fresh Agent with no message_history, so the user's "yes" arrived at an agent that had no idea what it was agreeing to. History is now kept per A2A contextId in a bounded in-memory map — enough for confirm-then-act, deliberately not durable, and documented as needing sticky routing beyond one replica. Verified end to end: pm-user creates S99_TEST1 across two turns and the row appears in MySQL; ro-user asking for the same thing is told no write tool exists, because for that token none does. Both test rows removed afterwards. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
@@ -58,10 +58,31 @@ quietly.
|
|||||||
| | |
|
| | |
|
||||||
|---|---|
|
|---|---|
|
||||||
| Reads | catalog (products, product lines), sales (read-only, to judge whether a product sells) |
|
| 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 |
|
| Calls | nothing yet. `Approvisionnement.check_availability` comes with discontinuation decisions |
|
||||||
| Exposes | `describe_catalog` |
|
| 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
|
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,
|
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.
|
so skills get added when another agent has a specific question — not in advance.
|
||||||
|
|||||||
@@ -129,7 +129,9 @@ class CatalogueExecutor(AgentExecutor):
|
|||||||
return
|
return
|
||||||
|
|
||||||
try:
|
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
|
except Exception as exc: # noqa: BLE001 — surfaced to the caller, logged here
|
||||||
logger.exception("turn failed")
|
logger.exception("turn failed")
|
||||||
await updater.failed(
|
await updater.failed(
|
||||||
|
|||||||
@@ -2,8 +2,11 @@
|
|||||||
|
|
||||||
from __future__ import annotations
|
from __future__ import annotations
|
||||||
|
|
||||||
|
from collections import OrderedDict
|
||||||
|
|
||||||
import httpx2
|
import httpx2
|
||||||
from pydantic_ai import Agent
|
from pydantic_ai import Agent
|
||||||
|
from pydantic_ai.messages import ModelMessage
|
||||||
from pydantic_ai.models.openai import OpenAIChatModel
|
from pydantic_ai.models.openai import OpenAIChatModel
|
||||||
from pydantic_ai.providers.openai import OpenAIProvider
|
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
|
- 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.
|
missing. Do not infer availability from what you can see.
|
||||||
|
|
||||||
You currently have read access only. If asked to change anything, say plainly
|
Writing to the catalog:
|
||||||
that writes are not enabled yet.
|
- 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:
|
class CatalogueAgent:
|
||||||
"""Holds what is stable across turns; rebuilds what must not be."""
|
"""Holds what is stable across turns; rebuilds what must not be."""
|
||||||
|
|
||||||
@@ -69,18 +88,31 @@ class CatalogueAgent:
|
|||||||
client_secret=settings.client_secret,
|
client_secret=settings.client_secret,
|
||||||
)
|
)
|
||||||
self._http_client = http_client
|
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.
|
"""One turn: exchange tokens, build the toolsets, answer, drop them.
|
||||||
|
|
||||||
The Agent is constructed here rather than once in __init__ because its
|
The Agent is constructed here rather than once in __init__ because its
|
||||||
toolsets carry this turn's tokens. Construction is cheap; a long-lived
|
toolsets carry this turn's tokens. Construction is cheap; a long-lived
|
||||||
agent holding a long-lived token is not.
|
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)
|
toolsets = await build_toolsets(self._settings, self._exchanger, subject_token)
|
||||||
llm_token = await self._exchanger.exchange(subject_token, self._settings.llm_scope)
|
llm_token = await self._exchanger.exchange(subject_token, self._settings.llm_scope)
|
||||||
model = build_model(self._settings, self._http_client, llm_token)
|
model = build_model(self._settings, self._http_client, llm_token)
|
||||||
agent = Agent(model, instructions=SYSTEM_PROMPT, toolsets=toolsets)
|
agent = Agent(model, instructions=SYSTEM_PROMPT, toolsets=toolsets)
|
||||||
async with agent:
|
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
|
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)
|
||||||
|
|||||||
@@ -74,7 +74,8 @@ async def build_toolsets(
|
|||||||
StreamableHttpTransport(
|
StreamableHttpTransport(
|
||||||
url=target.url,
|
url=target.url,
|
||||||
headers={"Authorization": f"Bearer {token}"},
|
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,
|
httpx_client_factory=client_factory,
|
||||||
),
|
),
|
||||||
id=target.name,
|
id=target.name,
|
||||||
|
|||||||
Reference in New Issue
Block a user