fix(plugins): stamp surface identity at record time, not send time
Six defects in 0.3.x plugin telemetry. Defects 1, 2 and 6 were not three bugs: they were one spool protocol getting three properties wrong. Identity was decided by the wrong process. `harness` was stamped in record(), correctly, but `source` was read from a module global in flush() — so whichever process drained the spool named every event in it. Two processes never call init(): mcp_server.py, and the detached `python3 telemetry.py` sender that spawn_flush() starts. record() now stamps source beside harness, and the build generates core/_harness_id.py per host so identity resolves with no init() call at all. That also unifies two defaults that disagreed (`<host>_plugin` vs `MEM0_<HOST>_PLUGIN`), which could yield three source values for one plugin. Ownership was inferred, not held. Path.replace is os.rename, which preserves mtime, so a claim made after a quiet minute inherited the spool's age and was stealable the instant it existed. Claims are touched on creation and the per-batch rewrite doubles as a lease heartbeat. Progress was not durable. flush() returned on the first failed batch without truncating, so the retry re-posted from index 0 — 150 events delivered 250 times. It now rewrites the claim with the unsent remainder after every batch, bounding a crash to one repeated batch, and each event carries a uuid. Parked batches starved. They were only reachable when no spool existed, and because sessions keep recording there usually was one, so a batch parked by a failed send waited until the 7-day expiry deleted it unsent — despite its own presence being what starts the sender. flush() drains them in the same run, and expiry now applies only after a genuine retry has failed. code.install counted upgrades and repeat sessions. is_first_run() read the identity file, which only a successful flush writes, so an offline user recorded an install every session forever. A dedicated install-state.json is claimed atomically at record time; a non-empty data directory reads as an upgrade. The docs called this anonymous. Every event carries the account email, and the hashes were unsalted SHA-256 over a git remote URL or an absolute path containing the username. READMEs, the module docstring and a new docs section now say what the code does, and repo/session digests are salted per install. A cached email outlived an API key change. It is now re-resolved when the key's fingerprint differs, and $identify aliases anonymous->email only — aliasing one account to another merges person profiles irreversibly. All six shipped green because the shared core's only tests lived under one host, behind a conftest that calls init() at import. Core behaviour was never exercised uninitialised. Adds agent-plugin-core/tests with no init, including subprocess tests and coverage for the portable plugin, which has no flush worker and would pass a native-only test vacuously. Also puts the three surface headers on the SDKs, CLIs and integrations, and corrects a README claiming ZAPIER/STRANDS were already in the platform allowlist. Verified: 59 core tests, 203 claude-code, 11 cursor, 5 codex, 2 kimi, 6 antigravity. ruff and compileall clean. --check clean for all six hosts. TypeScript changes are not typechecked locally (deps not installed). Claude-Session: https://claude.ai/code/session_01C7tEmH86HAr7GoAAKCEHZb
This commit is contained in:
Saket Aryan
@@ -0,0 +1,164 @@
|
||||
"""Delivery semantics of the telemetry spool: no duplicates, no starvation.
|
||||
|
||||
These run against a built host's core in-process (not a subprocess) because they
|
||||
need to inject failures into ``_post``. The identity tests next door cover the
|
||||
uninitialised-process case that needs a real interpreter.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import importlib
|
||||
import json
|
||||
import os
|
||||
import sys
|
||||
import time
|
||||
from pathlib import Path
|
||||
|
||||
import pytest
|
||||
|
||||
CORE_ROOT = Path(__file__).resolve().parents[1]
|
||||
REPOSITORY_ROOT = CORE_ROOT.parents[1]
|
||||
HOST_CORE = REPOSITORY_ROOT / "integrations" / "claude-code-plugin" / "core"
|
||||
|
||||
pytestmark = pytest.mark.skipif(not HOST_CORE.exists(), reason="claude-code-plugin is not built")
|
||||
|
||||
|
||||
@pytest.fixture()
|
||||
def telemetry(tmp_path, monkeypatch):
|
||||
monkeypatch.setenv("MEM0_CODE_DATA_DIR", str(tmp_path / "data"))
|
||||
monkeypatch.syspath_prepend(str(HOST_CORE))
|
||||
for name in ("telemetry", "memory_core", "_harness_id"):
|
||||
sys.modules.pop(name, None)
|
||||
module = importlib.import_module("telemetry")
|
||||
monkeypatch.setattr(module, "resolve_distinct_id", lambda: ("tester@example.com", ""))
|
||||
yield module
|
||||
for name in ("telemetry", "memory_core", "_harness_id"):
|
||||
sys.modules.pop(name, None)
|
||||
|
||||
|
||||
def _delivered(payloads):
|
||||
return [event for payload in payloads if "batch" in payload for event in payload["batch"]]
|
||||
|
||||
|
||||
def test_a_partial_failure_does_not_redeliver_what_already_arrived(telemetry):
|
||||
"""Defect 2a: flush kept the whole claim on failure and retried from the top.
|
||||
|
||||
150 events across two batches, the second failing, previously delivered 250.
|
||||
"""
|
||||
for index in range(150):
|
||||
telemetry.record("search", index=index)
|
||||
|
||||
sent: list[dict] = []
|
||||
calls = {"n": 0}
|
||||
|
||||
def flaky(payload, url):
|
||||
calls["n"] += 1
|
||||
if calls["n"] == 2: # second batch fails
|
||||
return False
|
||||
sent.append(payload)
|
||||
return True
|
||||
|
||||
telemetry._post = flaky
|
||||
telemetry.flush()
|
||||
|
||||
telemetry._post = lambda payload, url: sent.append(payload) or True
|
||||
telemetry.flush()
|
||||
|
||||
events = _delivered(sent)
|
||||
assert len(events) == 150
|
||||
assert len({event["uuid"] for event in events}) == 150
|
||||
|
||||
|
||||
def test_a_fresh_claim_is_not_immediately_stealable(telemetry):
|
||||
"""Defect 2b: rename preserves mtime, so a claim inherited the spool's age.
|
||||
|
||||
With the last write older than the stale threshold, a claim made now looked
|
||||
abandoned the instant it existed and a second sender took it over.
|
||||
"""
|
||||
telemetry.record("search")
|
||||
spool = telemetry._spool_path()
|
||||
old = time.time() - (telemetry.CLAIM_STALE_SECONDS + 60)
|
||||
os.utime(spool, (old, old))
|
||||
|
||||
first = telemetry._claim_spool()
|
||||
assert first is not None
|
||||
|
||||
# A second sender starting right now must find nothing to take.
|
||||
assert telemetry._claim_parked(first.parent) is None
|
||||
|
||||
|
||||
def test_a_parked_batch_is_drained_behind_the_live_spool(telemetry):
|
||||
"""Defect 6: parked claims were only reachable when no spool existed.
|
||||
|
||||
Because sessions keep recording there usually was one, so a batch parked by
|
||||
a failed send waited until the 7-day expiry deleted it unsent — even though
|
||||
its own presence is what starts the sender.
|
||||
"""
|
||||
telemetry.record("parked")
|
||||
telemetry._post = lambda payload, url: False
|
||||
telemetry.flush()
|
||||
|
||||
parked = list(telemetry.memory_core.data_dir().glob("telemetry-*.sending"))
|
||||
assert len(parked) == 1
|
||||
old = time.time() - (telemetry.CLAIM_STALE_SECONDS + 60)
|
||||
os.utime(parked[0], (old, old))
|
||||
|
||||
telemetry.record("fresh")
|
||||
sent: list[dict] = []
|
||||
telemetry._post = lambda payload, url: sent.append(payload) or True
|
||||
telemetry.flush()
|
||||
|
||||
names = {event["event"] for event in _delivered(sent)}
|
||||
assert names == {"code.parked", "code.fresh"}
|
||||
|
||||
|
||||
def test_an_untried_batch_is_not_expired_by_age_alone(telemetry):
|
||||
"""Expiry should discard what failed, not what never got a turn."""
|
||||
telemetry.record("parked")
|
||||
telemetry._post = lambda payload, url: False
|
||||
telemetry.flush()
|
||||
|
||||
parked = list(telemetry.memory_core.data_dir().glob("telemetry-*.sending"))
|
||||
assert len(parked) == 1
|
||||
ancient = time.time() - (telemetry.CLAIM_EXPIRY_SECONDS + 60)
|
||||
os.utime(parked[0], (ancient, ancient))
|
||||
|
||||
sent: list[dict] = []
|
||||
telemetry._post = lambda payload, url: sent.append(payload) or True
|
||||
telemetry.flush()
|
||||
|
||||
assert [event["event"] for event in _delivered(sent)] == ["code.parked"]
|
||||
|
||||
|
||||
def test_progress_is_recorded_after_every_batch(telemetry):
|
||||
"""A crash repeats at most one batch, not the whole file."""
|
||||
for index in range(250):
|
||||
telemetry.record("search", index=index)
|
||||
|
||||
calls = {"n": 0}
|
||||
|
||||
def die_after_two(payload, url):
|
||||
calls["n"] += 1
|
||||
if calls["n"] > 2:
|
||||
return False
|
||||
return True
|
||||
|
||||
telemetry._post = die_after_two
|
||||
telemetry.flush()
|
||||
|
||||
parked = list(telemetry.memory_core.data_dir().glob("telemetry-*.sending"))
|
||||
assert len(parked) == 1
|
||||
remaining = parked[0].read_text(encoding="utf-8").strip().splitlines()
|
||||
# Two batches of 100 landed; only the last 50 should still be pending.
|
||||
assert len(remaining) == 50
|
||||
assert json.loads(remaining[0])["properties"]["index"] == 200
|
||||
|
||||
|
||||
def test_the_heartbeat_stays_well_inside_the_lease(telemetry):
|
||||
"""The claim rewrite doubles as the lease heartbeat.
|
||||
|
||||
_post makes a single attempt with SEND_TIMEOUT and no retry, so a heartbeat
|
||||
lands at least that often. If a retry loop is ever added to _post, this is
|
||||
the assertion that catches a sender losing its claim mid-flight.
|
||||
"""
|
||||
assert telemetry.SEND_TIMEOUT * 4 < telemetry.CLAIM_STALE_SECONDS
|
||||
@@ -0,0 +1,179 @@
|
||||
"""Core telemetry behaviour with NO telemetry.init(), in a real subprocess.
|
||||
|
||||
Why this file exists
|
||||
--------------------
|
||||
``telemetry.py`` lives in ``agent-plugin-core/python/`` but its only tests lived
|
||||
under ``claude-code-plugin/tests/``, behind a ``conftest.py`` that calls
|
||||
``configure_harness()`` and ``telemetry.init()`` at import. Core behaviour was
|
||||
therefore only ever exercised inside an already-configured module.
|
||||
|
||||
Two processes in the real pipeline never call ``init()``:
|
||||
|
||||
- ``mcp_server.py``, which records every manual search;
|
||||
- the detached ``python3 telemetry.py`` sender that ``spawn_flush()`` starts at
|
||||
session start, after every skill command, and when the MCP server exits.
|
||||
|
||||
Both fell back to module defaults, so MCP searches reported ``harness=generic``
|
||||
and everything that sender delivered was labelled ``MEM0_PLUGIN`` regardless of
|
||||
which of the six plugins produced it. The suite stayed green throughout.
|
||||
|
||||
These tests run in a fresh interpreter with no conftest, against a built host
|
||||
bundle, which is the only arrangement that can catch that class of bug.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import json
|
||||
import subprocess
|
||||
import sys
|
||||
import tempfile
|
||||
from pathlib import Path
|
||||
|
||||
import pytest
|
||||
|
||||
CORE_ROOT = Path(__file__).resolve().parents[1]
|
||||
REPOSITORY_ROOT = CORE_ROOT.parents[1]
|
||||
HOSTS = {
|
||||
"claude-code": ("claude-code-plugin", "CLAUDE_CODE_PLUGIN"),
|
||||
"cursor": ("cursor-plugin", "CURSOR_PLUGIN"),
|
||||
"codex": ("codex-plugin", "CODEX_PLUGIN"),
|
||||
"kimi": ("kimi-plugin", "KIMI_PLUGIN"),
|
||||
"antigravity": ("antigravity-plugin", "ANTIGRAVITY_PLUGIN"),
|
||||
# Portable: no flush_worker and no hook_runner, so its ONLY sender is the
|
||||
# uninitialised telemetry.py. A native-only test passes here vacuously.
|
||||
"coding-agent": ("mem0-agent-plugin", "CODING_AGENT_PLUGIN"),
|
||||
}
|
||||
|
||||
|
||||
def _core_dir(directory: str) -> Path:
|
||||
return REPOSITORY_ROOT / "integrations" / directory / "core"
|
||||
|
||||
|
||||
def _run(core: Path, data_dir: Path, body: str) -> str:
|
||||
"""Execute `body` in a fresh interpreter with only the host's core on sys.path."""
|
||||
script = f"import sys; sys.path.insert(0, {str(core)!r})\n{body}"
|
||||
result = subprocess.run(
|
||||
[sys.executable, "-c", script],
|
||||
capture_output=True,
|
||||
text=True,
|
||||
env={
|
||||
"MEM0_CODE_DATA_DIR": str(data_dir),
|
||||
"PATH": "/usr/bin:/bin",
|
||||
"HOME": str(data_dir),
|
||||
},
|
||||
)
|
||||
assert result.returncode == 0, result.stderr
|
||||
return result.stdout.strip()
|
||||
|
||||
|
||||
@pytest.mark.parametrize("harness,spec", sorted(HOSTS.items()))
|
||||
def test_identity_resolves_without_init(harness, spec):
|
||||
"""Every built host knows what it is with no configuration call at all."""
|
||||
directory, source_tag = spec
|
||||
core = _core_dir(directory)
|
||||
if not core.exists():
|
||||
pytest.skip(f"{directory} is not built in this tree")
|
||||
|
||||
with tempfile.TemporaryDirectory() as tmp:
|
||||
out = _run(
|
||||
core,
|
||||
Path(tmp),
|
||||
"import telemetry; print(telemetry._harness, telemetry._source_tag)",
|
||||
)
|
||||
assert out == f"{harness} {source_tag}"
|
||||
|
||||
|
||||
def test_mcp_server_records_the_real_harness():
|
||||
"""mcp_server imports telemetry and never initialises it (server.py has no init).
|
||||
|
||||
Its recorded events used to carry harness=generic for every plugin.
|
||||
"""
|
||||
core = _core_dir("claude-code-plugin")
|
||||
if not core.exists():
|
||||
pytest.skip("claude-code-plugin is not built in this tree")
|
||||
|
||||
with tempfile.TemporaryDirectory() as tmp:
|
||||
data_dir = Path(tmp)
|
||||
_run(
|
||||
core,
|
||||
data_dir,
|
||||
"import mcp_server, telemetry; telemetry.record('search', trigger='mcp-search')",
|
||||
)
|
||||
spooled = (data_dir / "telemetry.jsonl").read_text(encoding="utf-8").strip()
|
||||
|
||||
event = json.loads(spooled)
|
||||
assert event["properties"]["harness"] == "claude-code"
|
||||
assert event["properties"]["source"] == "CLAUDE_CODE_PLUGIN"
|
||||
|
||||
|
||||
def test_the_detached_sender_does_not_relabel_events():
|
||||
"""`python3 telemetry.py` is the sender spawn_flush() starts, and never inits.
|
||||
|
||||
source is stamped at record time now, so which process sends is irrelevant.
|
||||
"""
|
||||
core = _core_dir("claude-code-plugin")
|
||||
if not core.exists():
|
||||
pytest.skip("claude-code-plugin is not built in this tree")
|
||||
|
||||
with tempfile.TemporaryDirectory() as tmp:
|
||||
data_dir = Path(tmp)
|
||||
_run(core, data_dir, "import telemetry; telemetry.record('search')")
|
||||
|
||||
captured = data_dir / "captured.json"
|
||||
# Drain with a fresh, unconfigured interpreter, capturing the payload
|
||||
# instead of posting it.
|
||||
_run(
|
||||
core,
|
||||
data_dir,
|
||||
"import json, telemetry\n"
|
||||
"sent = []\n"
|
||||
"telemetry._post = lambda payload, url: sent.append(payload) or True\n"
|
||||
"telemetry.flush()\n"
|
||||
f"open({str(captured)!r}, 'w').write(json.dumps(sent))",
|
||||
)
|
||||
payloads = json.loads(captured.read_text(encoding="utf-8"))
|
||||
|
||||
batches = [p for p in payloads if "batch" in p]
|
||||
assert batches, "nothing was sent"
|
||||
properties = batches[0]["batch"][0]["properties"]
|
||||
assert properties["source"] == "CLAUDE_CODE_PLUGIN"
|
||||
assert properties["harness"] == "claude-code"
|
||||
|
||||
|
||||
def test_every_event_carries_a_uuid_for_dedupe():
|
||||
core = _core_dir("claude-code-plugin")
|
||||
if not core.exists():
|
||||
pytest.skip("claude-code-plugin is not built in this tree")
|
||||
|
||||
with tempfile.TemporaryDirectory() as tmp:
|
||||
data_dir = Path(tmp)
|
||||
_run(core, data_dir, "import telemetry; telemetry.record('search'); telemetry.record('flush')")
|
||||
lines = (data_dir / "telemetry.jsonl").read_text(encoding="utf-8").strip().splitlines()
|
||||
|
||||
ids = [json.loads(line)["uuid"] for line in lines]
|
||||
assert len(ids) == 2
|
||||
assert len(set(ids)) == 2
|
||||
|
||||
|
||||
def test_source_tag_defaults_agree_between_the_two_modules():
|
||||
"""configure_harness and telemetry.init must derive the same tag.
|
||||
|
||||
They disagreed: `<host>_plugin` in one and `MEM0_<HOST>_PLUGIN` in the other,
|
||||
so one plugin could emit three different source values depending on which
|
||||
process sent the batch.
|
||||
"""
|
||||
core = _core_dir("claude-code-plugin")
|
||||
if not core.exists():
|
||||
pytest.skip("claude-code-plugin is not built in this tree")
|
||||
|
||||
with tempfile.TemporaryDirectory() as tmp:
|
||||
out = _run(
|
||||
core,
|
||||
Path(tmp),
|
||||
"import memory_core, telemetry\n"
|
||||
"memory_core.configure_harness('kimi')\n"
|
||||
"telemetry.init(harness='kimi')\n"
|
||||
"print(memory_core.harness_config()['source_tag'].upper(), telemetry._source_tag)",
|
||||
)
|
||||
left, right = out.split()
|
||||
assert left == right == "KIMI_PLUGIN"
|
||||
Reference in New Issue
Block a user