fix: a unique message id, and a peer conversation that survives
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 as a list, also raised in review. OutputSpec is declared as an item OR a Sequence of them, so a list is the API's own form and not a union written badly. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
+1
-1
@@ -30,7 +30,7 @@ spec:
|
|||||||
# repo is public, so the pull needs no credentials — which removes an
|
# repo is public, so the pull needs no credentials — which removes an
|
||||||
# imagePullSecret that would otherwise have to be created by hand in
|
# imagePullSecret that would otherwise have to be created by hand in
|
||||||
# this namespace and kept in step with a GitHub token.
|
# 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:
|
ports:
|
||||||
- name: http
|
- name: http
|
||||||
containerPort: 8080
|
containerPort: 8080
|
||||||
|
|||||||
+1
-1
@@ -1,6 +1,6 @@
|
|||||||
[project]
|
[project]
|
||||||
name = "classic-models-agent-catalogue"
|
name = "classic-models-agent-catalogue"
|
||||||
version = "0.3.0"
|
version = "0.3.1"
|
||||||
description = "Product management agent for the Classic Models catalog domain"
|
description = "Product management agent for the Classic Models catalog domain"
|
||||||
requires-python = ">=3.12"
|
requires-python = ">=3.12"
|
||||||
|
|
||||||
|
|||||||
@@ -214,7 +214,7 @@ class CatalogueAgent:
|
|||||||
the arguments the model proposed, and nothing reaches the gateway until
|
the arguments the model proposed, and nothing reaches the gateway until
|
||||||
someone approves them.
|
someone approves them.
|
||||||
"""
|
"""
|
||||||
agent, deps = await self._build(subject_token)
|
agent, deps = await self._build(subject_token, conversation_id)
|
||||||
async with agent:
|
async with agent:
|
||||||
result = await agent.run(
|
result = await agent.run(
|
||||||
prompt,
|
prompt,
|
||||||
@@ -249,7 +249,7 @@ class CatalogueAgent:
|
|||||||
continue
|
continue
|
||||||
results.approvals[call.tool_call_id] = ToolApproved()
|
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:
|
async with agent:
|
||||||
result = await agent.run(
|
result = await agent.run(
|
||||||
deps=deps,
|
deps=deps,
|
||||||
@@ -262,7 +262,9 @@ class CatalogueAgent:
|
|||||||
def awaiting(self, conversation_id: str) -> PendingApproval | None:
|
def awaiting(self, conversation_id: str) -> PendingApproval | None:
|
||||||
return self._pending.get(conversation_id)
|
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)
|
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)
|
||||||
@@ -271,7 +273,7 @@ class CatalogueAgent:
|
|||||||
instructions=[SYSTEM_PROMPT, caller_instructions],
|
instructions=[SYSTEM_PROMPT, caller_instructions],
|
||||||
deps_type=CallerDeps,
|
deps_type=CallerDeps,
|
||||||
toolsets=toolsets,
|
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
|
# 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
|
# DeferredToolRequests in the union, a held tool call is an error
|
||||||
# rather than a state.
|
# rather than a state.
|
||||||
@@ -279,7 +281,7 @@ class CatalogueAgent:
|
|||||||
)
|
)
|
||||||
return agent, CallerDeps(user=caller_of(subject_token))
|
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`.
|
"""One narrow tool, not a generic `call_agent`.
|
||||||
|
|
||||||
A generic one would let the model reach any peer with any prompt, which
|
A generic one would let the model reach any peer with any prompt, which
|
||||||
@@ -301,9 +303,14 @@ class CatalogueAgent:
|
|||||||
peer,
|
peer,
|
||||||
f"Peut-on promettre le produit {productcode} ?",
|
f"Peut-on promettre le produit {productcode} ?",
|
||||||
subject_token,
|
subject_token,
|
||||||
# One conversation per peer per turn. The peer keeps its own
|
# Keyed on the conversation, not the token.
|
||||||
# thread, which is what lets it resume anything it parks.
|
#
|
||||||
f"cat-{abs(hash(subject_token)) % 10**8}",
|
# 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
|
return check_availability
|
||||||
|
|||||||
@@ -19,6 +19,7 @@ the realm.
|
|||||||
from __future__ import annotations
|
from __future__ import annotations
|
||||||
|
|
||||||
import logging
|
import logging
|
||||||
|
import uuid
|
||||||
from dataclasses import dataclass
|
from dataclasses import dataclass
|
||||||
|
|
||||||
import httpx
|
import httpx
|
||||||
@@ -87,7 +88,12 @@ async def ask_peer(
|
|||||||
try:
|
try:
|
||||||
request = a2a_types.SendMessageRequest(
|
request = a2a_types.SendMessageRequest(
|
||||||
message=a2a_types.Message(
|
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,
|
context_id=context_id,
|
||||||
role=a2a_types.Role.ROLE_USER,
|
role=a2a_types.Role.ROLE_USER,
|
||||||
parts=[a2a_types.Part(text=question)],
|
parts=[a2a_types.Part(text=question)],
|
||||||
|
|||||||
Reference in New Issue
Block a user