fix(mcp): expire idempotency records by UTC, not local time
The stamp is written as UTC but was read back with mktime, which interprets it as local time — with TZ=Europe/Vienna that shifted the 24 h replay window by an hour or two. Adds pitfalls 24 and 25 to PLAN.md §14.
This commit is contained in:
1 parent
c79e55f3e7
commit
9f976c54ea
5 files changed
+36
-1
No files matched your search
Binary file not shown.
@@ -1058,6 +1058,17 @@ Cluster-specific, added in this revision:
|
||||
23. An ingress NetworkPolicy denies everything it does not list. Verify kubelet probes and the
|
||||
Prometheus scrape after applying it — both fail in ways that do not look like a policy problem.
|
||||
|
||||
Found while implementing:
|
||||
|
||||
24. The MCP SDK's Streamable HTTP transport enforces DNS-rebinding protection on the `Host`
|
||||
header. Bifrost dials `mdmem.apps.svc.cluster.local:8080`, so that name must be in
|
||||
`MCP_ALLOWED_HOSTS` or every proxied call returns **421 Misdirected Request** — which
|
||||
looks like an mdmem bug, not a transport setting. Covered by a test.
|
||||
25. A note created by hand in the vault *while the service runs* reaches the index through
|
||||
the watcher, not through the startup scan. Whatever repairs a missing/invalid ULID must
|
||||
therefore live on the watcher path too, or hand-written notes stay invisible until the
|
||||
next restart. Found by running the service, not by the unit tests.
|
||||
|
||||
---
|
||||
|
||||
## 15. References
|
||||
|
||||
@@ -108,6 +108,10 @@ against a live cluster — verify each step.
|
||||
| jq '.data[0].embedding|length' # expect 2560
|
||||
```
|
||||
A 404/400 means the alias or the provider prefix is wrong, not that mdmem is broken.
|
||||
While you are there, confirm Bifrost's `semantic_cache` plugin does **not** intercept
|
||||
`/v1/embeddings` — send two slightly different inputs and check the vectors differ. A
|
||||
cache hit there would return a stale vector for changed text and silently corrupt the
|
||||
index (§14.13). `uv run pytest -m live` checks this and the pooling/prefix behaviour.
|
||||
7. Only then add `deploy/bifrost-mcp-client.json` to Bifrost's `config.json` and commit,
|
||||
so a failure is attributable to one change at a time. Create a VK scoped to the six
|
||||
`memory_*` tools.
|
||||
|
||||
@@ -3,6 +3,7 @@
|
||||
from __future__ import annotations
|
||||
|
||||
import asyncio
|
||||
import calendar
|
||||
import json
|
||||
import logging
|
||||
import time
|
||||
@@ -680,7 +681,8 @@ class MemoryService:
|
||||
if row is None:
|
||||
return None
|
||||
try:
|
||||
age = time.time() - time.mktime(time.strptime(row["created"], "%Y-%m-%dT%H:%M:%SZ"))
|
||||
# the stamp is UTC; mktime would read it as local time (TZ=Europe/Vienna)
|
||||
age = time.time() - calendar.timegm(time.strptime(row["created"], "%Y-%m-%dT%H:%M:%SZ"))
|
||||
except ValueError:
|
||||
age = 0.0
|
||||
if age > IDEMPOTENCY_TTL_S:
|
||||
|
||||
@@ -349,3 +349,21 @@ async def test_refuses_to_build_without_a_token(svc, monkeypatch):
|
||||
svc.settings = replace(svc.settings, mcp_token="")
|
||||
with pytest.raises(RuntimeError, match="MCP_TOKEN"):
|
||||
build_mcp_app(svc)
|
||||
|
||||
|
||||
async def test_idempotency_expires_by_utc_not_local_time(svc):
|
||||
"""The stored stamp is UTC; reading it as local time would shift the TTL by the
|
||||
offset (TZ=Europe/Vienna) and replay a request that should have expired."""
|
||||
import calendar
|
||||
import time
|
||||
|
||||
from mdmem.notes import utcnow
|
||||
|
||||
svc.idempotent_put("req-x", "memory_write", {"id": "abc"})
|
||||
assert svc.idempotent_get("req-x", "memory_write") == {"id": "abc"}
|
||||
|
||||
old = time.strftime("%Y-%m-%dT%H:%M:%SZ", time.gmtime(calendar.timegm(time.gmtime()) - 25 * 3600))
|
||||
with svc.db.write() as w:
|
||||
w.execute("UPDATE idempotency SET created=? WHERE request_id='req-x'", (old,))
|
||||
assert svc.idempotent_get("req-x", "memory_write") is None
|
||||
assert utcnow().endswith("Z")
|
||||
Reference in new issue
Block a user