fix(plugins): stop delivering telemetry events twice, and stop losing parked ones

Two defects, one cause: the spool protocol infers ownership instead of holding
it, and never records progress.

Duplicate delivery after a partial failure. flush() posts the claim in batches of
100 and returns on the first failure, keeping the whole file. The retry then
posts every batch again, including the ones that already arrived — 150 recorded
events were delivered 250 times. Progress is now written back to the claim after
each successful batch, so a retry resumes where the send stopped and a crash
repeats at most one batch.

Duplicate delivery when two senders overlap. spool.replace(claim) is os.rename,
which preserves mtime, so a claim created after a quiet minute inherited the
spool's last-write time and looked abandoned the instant it existed. A second
sender starting while the first was still posting took it over and sent it too —
most likely at session end, when the MCP server's exit sender and the SessionEnd
flush worker both drain. Claims are now touched at claim time, and the per-batch
rewrite doubles as a lease heartbeat. _post makes one attempt with SEND_TIMEOUT
and no retry, so a heartbeat lands well inside the 120s lease; a test asserts
that margin so adding a retry loop to _post cannot silently break it.

Parked batches starved. _claim_spool only looked at parked .sending files 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 in the first place.
flush() now drains the live spool and then parked claims in the same run, oldest
first, bounded. Expiry applies only after a genuine retry has failed, with the
attempt count carried in the filename.

A sender that gives up releases its lease rather than heartbeating on the way
out, so the next run picks the batch up promptly instead of waiting a full stale
window for a batch nobody is working on. A failing send stops the run, so one
broken connection cannot burn every parked batch's attempt budget at once.

Two existing tests asserted the old lifecycle and are updated in place, each
with a comment saying what changed.

Claude-Session: https://claude.ai/code/session_01C7tEmH86HAr7GoAAKCEHZb
This commit is contained in:
Saket Aryan
2026-09-15 00:18:21 +05:30
parent f9c566aa16
commit cfdfe40e09
9 changed files with 1241 additions and 118 deletions
@@ -1,6 +1,7 @@
from __future__ import annotations
import json
import os
import sys
from pathlib import Path
from unittest.mock import patch
@@ -198,9 +199,11 @@ def test_a_stale_claim_is_reclaimed(isolated_env, monkeypatch):
telemetry.record("search")
orphan = telemetry._claim_spool()
assert orphan is not None
monkeypatch.setattr(
telemetry.time, "time", lambda: orphan.stat().st_mtime + telemetry.CLAIM_STALE_SECONDS + 1
)
# Frozen rather than re-stat'd per call: flush() drains the live spool and
# then looks for parked claims in the same run, so by the second look this
# file no longer exists.
stale_now = orphan.stat().st_mtime + telemetry.CLAIM_STALE_SECONDS + 1
monkeypatch.setattr(telemetry.time, "time", lambda: stale_now)
with patch.object(telemetry, "_post", lambda payload, url: True):
assert telemetry.flush() == 1
@@ -210,9 +213,13 @@ def test_an_expired_claim_is_dropped(isolated_env, monkeypatch):
telemetry.record("search")
orphan = telemetry._claim_spool()
assert orphan is not None
monkeypatch.setattr(
telemetry.time, "time", lambda: orphan.stat().st_mtime + telemetry.CLAIM_EXPIRY_SECONDS + 1
)
expired_now = orphan.stat().st_mtime + telemetry.CLAIM_EXPIRY_SECONDS + 1
monkeypatch.setattr(telemetry.time, "time", lambda: expired_now)
# Expiry now only discards a batch that was genuinely retried and failed,
# so age alone is not enough — age it past the attempt budget too.
retried = orphan.parent / orphan.name.replace("-a0.", f"-a{telemetry.MAX_CLAIM_ATTEMPTS}.")
orphan.replace(retried)
os.utime(retried, (expired_now, expired_now - telemetry.CLAIM_EXPIRY_SECONDS - 1))
assert telemetry._claim_spool() is None
assert not list(memory_core.data_dir().glob("telemetry-*.sending"))