From eca59cef9ec4995ad3c82a83c443ff74a35f0964 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?St=C3=A9phane=20Tailland?= Date: Sun, 30 Aug 2026 07:46:06 +0200 Subject: [PATCH] fix: a unique message id, and a peer conversation that survives MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two bugs in the peer call, both found in review. message_id was context_id + peer name, so every call in a conversation carried the same id. A2A wants one id per message; a peer treating the second call as a replay of the first would have looked like a stale answer rather than a bug. The peer's context_id was derived from hash(subject_token), which was wrong twice over. hash() on a str is salted per process, so a restart changed the key — the reported bug. And a fresh token is minted every turn, so the key changed every turn anyway, which is worse: the peer started a new thread on each call and lost anything it had parked, the opposite of what the key exists for. It is now the conversation id, which is what 'one conversation per peer' meant. Not changed: output_type=[str, DeferredToolRequests], also raised in review. OutputSpec is declared as , so a list is the API's own form and not a union written badly. Co-Authored-By: Claude Opus 5 --- k8s/deployment.yaml | 2 +- pyproject.toml | 2 +- src/catalogue_agent/agent.py | 23 +++++++++++++++-------- src/catalogue_agent/peers.py | 8 +++++++- 4 files changed, 24 insertions(+), 11 deletions(-) diff --git a/k8s/deployment.yaml b/k8s/deployment.yaml index 51bd036..0bbfc46 100644 --- a/k8s/deployment.yaml +++ b/k8s/deployment.yaml @@ -30,7 +30,7 @@ spec: # repo is public, so the pull needs no credentials — which removes an # imagePullSecret that would otherwise have to be created by hand in # this namespace and kept in step with a GitHub token. - image: gitea.sttlab.eu/stt/classic-models-agent-catalogue:v0.3.0 + image: gitea.sttlab.eu/stt/classic-models-agent-catalogue:v0.3.1 ports: - name: http containerPort: 8080 diff --git a/pyproject.toml b/pyproject.toml index 360dfec..e7529e3 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -1,6 +1,6 @@ [project] name = "classic-models-agent-catalogue" -version = "0.3.0" +version = "0.3.1" description = "Product management agent for the Classic Models catalog domain" requires-python = ">=3.12" diff --git a/src/catalogue_agent/agent.py b/src/catalogue_agent/agent.py index 9fa9bd2..fa436f4 100644 --- a/src/catalogue_agent/agent.py +++ b/src/catalogue_agent/agent.py @@ -214,7 +214,7 @@ class CatalogueAgent: the arguments the model proposed, and nothing reaches the gateway until someone approves them. """ - agent, deps = await self._build(subject_token) + agent, deps = await self._build(subject_token, conversation_id) async with agent: result = await agent.run( prompt, @@ -249,7 +249,7 @@ class CatalogueAgent: continue results.approvals[call.tool_call_id] = ToolApproved() - agent, deps = await self._build(subject_token) + agent, deps = await self._build(subject_token, conversation_id) async with agent: result = await agent.run( deps=deps, @@ -262,7 +262,9 @@ class CatalogueAgent: def awaiting(self, conversation_id: str) -> PendingApproval | None: return self._pending.get(conversation_id) - async def _build(self, subject_token: str) -> tuple[Agent, CallerDeps]: + async def _build( + self, subject_token: str, conversation_id: str + ) -> tuple[Agent, CallerDeps]: 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) @@ -271,7 +273,7 @@ class CatalogueAgent: instructions=[SYSTEM_PROMPT, caller_instructions], deps_type=CallerDeps, toolsets=toolsets, - tools=[self._availability_tool(subject_token)], + tools=[self._availability_tool(subject_token, conversation_id)], # A turn may end with an answer or with a decision to make. Without # DeferredToolRequests in the union, a held tool call is an error # rather than a state. @@ -279,7 +281,7 @@ class CatalogueAgent: ) return agent, CallerDeps(user=caller_of(subject_token)) - def _availability_tool(self, subject_token: str) -> Any: + def _availability_tool(self, subject_token: str, conversation_id: str) -> Any: """One narrow tool, not a generic `call_agent`. A generic one would let the model reach any peer with any prompt, which @@ -301,9 +303,14 @@ class CatalogueAgent: peer, f"Peut-on promettre le produit {productcode} ?", subject_token, - # One conversation per peer per turn. The peer keeps its own - # thread, which is what lets it resume anything it parks. - f"cat-{abs(hash(subject_token)) % 10**8}", + # Keyed on the conversation, not the token. + # + # The token was wrong twice over: `hash()` on a str is salted per + # process, so a restart changed the key; and a fresh token is + # minted every turn, so the key changed every turn anyway. The + # peer would have started a new thread each time and lost + # anything it had parked — the opposite of what this is for. + f"cat-{conversation_id}", ) return check_availability diff --git a/src/catalogue_agent/peers.py b/src/catalogue_agent/peers.py index bcbc72a..58c5e3b 100644 --- a/src/catalogue_agent/peers.py +++ b/src/catalogue_agent/peers.py @@ -19,6 +19,7 @@ the realm. from __future__ import annotations import logging +import uuid from dataclasses import dataclass import httpx @@ -87,7 +88,12 @@ async def ask_peer( try: request = a2a_types.SendMessageRequest( message=a2a_types.Message( - message_id=f"{context_id}-{peer.name}", + # Unique per message, not per conversation. Reusing one id + # across the turns of a conversation is a protocol error, and + # the failure it invites — a peer treating the second call as + # a replay of the first — would look like a stale answer + # rather than a bug. + message_id=f"{context_id}-{peer.name}-{uuid.uuid4().hex[:8]}", context_id=context_id, role=a2a_types.Role.ROLE_USER, parts=[a2a_types.Part(text=question)],