fix: bound the token cache, and make peer message ids unique

Two bugs from review, the second in both agents because identity.py is a copy.

The exchanged-token cache grew for the life of the process. It is keyed on
(subject token, scope) and the subject token is minted fresh every turn, so
every turn added keys nothing would ever read again — and expiry was checked on
read but never collected. A slow leak that only shows in a pod up for days,
which is the worst way to find one.

It now drops expired entries first and evicts oldest beyond a bound. Expired
first on purpose: an expired entry is dead weight wherever it sits in the
ordering, so evicting by age alone would keep it while discarding a live one.

The docstring was wrong before the code was: it claimed the cache lasted one
turn. It never did. Corrected rather than quietly made true.

message_id reused the context id, which is the conversation — and a conversation
makes several calls to the same peer, relaying an approval being exactly that.
A peer treating the second call as a replay of the first would surface as a
stale answer rather than an error.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
Stéphane Tailland
2026-08-30 08:03:23 +02:00
parent 3c44cc63ea
commit f79b58fc88
8 changed files with 122 additions and 7 deletions
+83
View File
@@ -0,0 +1,83 @@
"""The token cache must not grow for the life of the process.
The subject token changes every turn, so every turn adds keys that will never be
read again. An unbounded dict here is a slow leak that only shows up in a pod
that has been up for days — which is the worst way to find it.
"""
from __future__ import annotations
import time
import httpx
import pytest
from assistant_agent.identity import MAX_CACHED_TOKENS, ExchangedToken, TokenExchanger
def exchanger(handler) -> TokenExchanger:
transport = httpx.MockTransport(handler)
return TokenExchanger(
httpx.AsyncClient(transport=transport),
token_endpoint="https://keycloak/token",
client_id="agent",
client_secret="secret",
)
def ok(_request: httpx.Request) -> httpx.Response:
return httpx.Response(200, json={"access_token": "a-token", "expires_in": 300})
class TestExpiry:
def test_an_expired_entry_is_reported_expired(self) -> None:
assert ExchangedToken("t", time.monotonic() - 1).expired is True
def test_a_fresh_one_is_not(self) -> None:
assert ExchangedToken("t", time.monotonic() + 300).expired is False
def test_the_margin_covers_a_turn_that_stalls(self) -> None:
# A token valid for another 10s is treated as spent: a turn that stalls
# on a slow model must not send one that dies mid-flight.
assert ExchangedToken("t", time.monotonic() + 10).expired is True
class TestPruning:
async def test_a_hit_costs_no_round_trip(self) -> None:
calls = 0
def counting(request: httpx.Request) -> httpx.Response:
nonlocal calls
calls += 1
return ok(request)
ex = exchanger(counting)
await ex.exchange("subject", "scope")
await ex.exchange("subject", "scope")
assert calls == 1
async def test_expired_entries_are_dropped(self) -> None:
ex = exchanger(ok)
await ex.exchange("subject", "scope")
# Age the only entry, then exchange something else: the prune that runs
# on the way out must collect it.
for key in list(ex._cache):
ex._cache[key] = ExchangedToken("stale", time.monotonic() - 1)
await ex.exchange("other", "scope")
assert all(not v.expired for v in ex._cache.values())
assert len(ex._cache) == 1
async def test_the_cache_is_bounded(self) -> None:
# Every turn brings a new subject token, so without a bound this grows
# until the pod does.
ex = exchanger(ok)
for i in range(MAX_CACHED_TOKENS + 50):
await ex.exchange(f"subject-{i}", "scope")
assert len(ex._cache) <= MAX_CACHED_TOKENS
async def test_eviction_drops_the_oldest_first(self) -> None:
ex = exchanger(ok)
for i in range(MAX_CACHED_TOKENS + 10):
await ex.exchange(f"subject-{i}", "scope")
assert ("subject-0", "scope") not in ex._cache
assert (f"subject-{MAX_CACHED_TOKENS + 9}", "scope") in ex._cache