From 4edfaabc0604637a4c44b22aeea2f83212eeff97 Mon Sep 17 00:00:00 2001 From: Saket Aryan Date: Thu, 17 Sep 2026 19:28:08 +0530 Subject: [PATCH] fix(plugins): collect quarantined batches instead of leaving them on disk forever Review finding. _sweep_debris globbed only *.partial. The *.corrupt files this PR writes when a batch cannot be decoded are matched by no glob in the module, so they accumulated for the life of the install. Collected on the expiry window rather than the stale window, deliberately: a quarantined batch is the only remaining evidence of events that could not be delivered, so someone chasing a report of missing telemetry has to be able to find a recent one. Debris keeps the short window; it carries nothing. One test, asserting both halves: a recent quarantine survives and an expired one does not. Claude-Session: https://claude.ai/code/session_01C7tEmH86HAr7GoAAKCEHZb --- .../agent-plugin-core/python/telemetry.py | 18 +++++++++++++--- .../tests/test_spool_delivery.py | 21 +++++++++++++++++++ .../antigravity-plugin/core/telemetry.py | 18 +++++++++++++--- .../claude-code-plugin/core/telemetry.py | 18 +++++++++++++--- integrations/codex-plugin/core/telemetry.py | 18 +++++++++++++--- integrations/cursor-plugin/core/telemetry.py | 18 +++++++++++++--- integrations/kimi-plugin/core/telemetry.py | 18 +++++++++++++--- .../mem0-agent-plugin/core/telemetry.py | 18 +++++++++++++--- 8 files changed, 126 insertions(+), 21 deletions(-) diff --git a/integrations/agent-plugin-core/python/telemetry.py b/integrations/agent-plugin-core/python/telemetry.py index f46e1bf55..9084c8a24 100644 --- a/integrations/agent-plugin-core/python/telemetry.py +++ b/integrations/agent-plugin-core/python/telemetry.py @@ -462,10 +462,16 @@ def _claim_spool() -> Path | None: def _sweep_debris(directory: Path) -> None: - """Remove temp files orphaned by a crash between write and rename. + """Remove files nothing else will ever pick up again. - Neither glob in this module matches *.partial, so nothing else would ever - clean them up. + *.partial is a temp file orphaned by a crash between write and rename. + *.corrupt is a batch quarantined for undecodable content. No glob in this + module matches either, so without this they accumulate on disk for the life + of the install. + + Quarantined batches are kept far longer than debris: they are the only + evidence left of events that could not be delivered, and someone diagnosing + a report of missing telemetry has to be able to find one. """ now = time.time() for debris in directory.glob("telemetry-*.partial"): @@ -474,6 +480,12 @@ def _sweep_debris(directory: Path) -> None: debris.unlink() except OSError: continue + for quarantined in directory.glob("telemetry-*.corrupt"): + try: + if now - quarantined.stat().st_mtime > CLAIM_EXPIRY_SECONDS: + quarantined.unlink() + except OSError: + continue def _claim_parked(directory: Path) -> Path | None: diff --git a/integrations/agent-plugin-core/tests/test_spool_delivery.py b/integrations/agent-plugin-core/tests/test_spool_delivery.py index ac0bf6b22..03434177a 100644 --- a/integrations/agent-plugin-core/tests/test_spool_delivery.py +++ b/integrations/agent-plugin-core/tests/test_spool_delivery.py @@ -409,3 +409,24 @@ def test_partial_files_are_swept(telemetry): telemetry.flush() assert not debris.exists() + + +def test_quarantined_batches_are_eventually_collected(telemetry): + """Nothing re-globs .corrupt, so without a sweep they live on disk forever. + + Kept much longer than .partial debris on purpose: a quarantined batch is the + only remaining evidence of events that could not be delivered. + """ + directory = telemetry.memory_core.data_dir() + directory.mkdir(parents=True, exist_ok=True) + fresh = directory / "telemetry-1-aaaaaaaa-a0.corrupt" + old = directory / "telemetry-2-bbbbbbbb-a0.corrupt" + for path in (fresh, old): + path.write_text("torn", encoding="utf-8") + expired = time.time() - (telemetry.CLAIM_EXPIRY_SECONDS + 60) + os.utime(old, (expired, expired)) + + telemetry._sweep_debris(directory) + + assert fresh.exists(), "a recent quarantine was discarded before anyone could look at it" + assert not old.exists(), "an expired quarantine was left on disk forever" diff --git a/integrations/antigravity-plugin/core/telemetry.py b/integrations/antigravity-plugin/core/telemetry.py index f46e1bf55..9084c8a24 100644 --- a/integrations/antigravity-plugin/core/telemetry.py +++ b/integrations/antigravity-plugin/core/telemetry.py @@ -462,10 +462,16 @@ def _claim_spool() -> Path | None: def _sweep_debris(directory: Path) -> None: - """Remove temp files orphaned by a crash between write and rename. + """Remove files nothing else will ever pick up again. - Neither glob in this module matches *.partial, so nothing else would ever - clean them up. + *.partial is a temp file orphaned by a crash between write and rename. + *.corrupt is a batch quarantined for undecodable content. No glob in this + module matches either, so without this they accumulate on disk for the life + of the install. + + Quarantined batches are kept far longer than debris: they are the only + evidence left of events that could not be delivered, and someone diagnosing + a report of missing telemetry has to be able to find one. """ now = time.time() for debris in directory.glob("telemetry-*.partial"): @@ -474,6 +480,12 @@ def _sweep_debris(directory: Path) -> None: debris.unlink() except OSError: continue + for quarantined in directory.glob("telemetry-*.corrupt"): + try: + if now - quarantined.stat().st_mtime > CLAIM_EXPIRY_SECONDS: + quarantined.unlink() + except OSError: + continue def _claim_parked(directory: Path) -> Path | None: diff --git a/integrations/claude-code-plugin/core/telemetry.py b/integrations/claude-code-plugin/core/telemetry.py index f46e1bf55..9084c8a24 100644 --- a/integrations/claude-code-plugin/core/telemetry.py +++ b/integrations/claude-code-plugin/core/telemetry.py @@ -462,10 +462,16 @@ def _claim_spool() -> Path | None: def _sweep_debris(directory: Path) -> None: - """Remove temp files orphaned by a crash between write and rename. + """Remove files nothing else will ever pick up again. - Neither glob in this module matches *.partial, so nothing else would ever - clean them up. + *.partial is a temp file orphaned by a crash between write and rename. + *.corrupt is a batch quarantined for undecodable content. No glob in this + module matches either, so without this they accumulate on disk for the life + of the install. + + Quarantined batches are kept far longer than debris: they are the only + evidence left of events that could not be delivered, and someone diagnosing + a report of missing telemetry has to be able to find one. """ now = time.time() for debris in directory.glob("telemetry-*.partial"): @@ -474,6 +480,12 @@ def _sweep_debris(directory: Path) -> None: debris.unlink() except OSError: continue + for quarantined in directory.glob("telemetry-*.corrupt"): + try: + if now - quarantined.stat().st_mtime > CLAIM_EXPIRY_SECONDS: + quarantined.unlink() + except OSError: + continue def _claim_parked(directory: Path) -> Path | None: diff --git a/integrations/codex-plugin/core/telemetry.py b/integrations/codex-plugin/core/telemetry.py index f46e1bf55..9084c8a24 100644 --- a/integrations/codex-plugin/core/telemetry.py +++ b/integrations/codex-plugin/core/telemetry.py @@ -462,10 +462,16 @@ def _claim_spool() -> Path | None: def _sweep_debris(directory: Path) -> None: - """Remove temp files orphaned by a crash between write and rename. + """Remove files nothing else will ever pick up again. - Neither glob in this module matches *.partial, so nothing else would ever - clean them up. + *.partial is a temp file orphaned by a crash between write and rename. + *.corrupt is a batch quarantined for undecodable content. No glob in this + module matches either, so without this they accumulate on disk for the life + of the install. + + Quarantined batches are kept far longer than debris: they are the only + evidence left of events that could not be delivered, and someone diagnosing + a report of missing telemetry has to be able to find one. """ now = time.time() for debris in directory.glob("telemetry-*.partial"): @@ -474,6 +480,12 @@ def _sweep_debris(directory: Path) -> None: debris.unlink() except OSError: continue + for quarantined in directory.glob("telemetry-*.corrupt"): + try: + if now - quarantined.stat().st_mtime > CLAIM_EXPIRY_SECONDS: + quarantined.unlink() + except OSError: + continue def _claim_parked(directory: Path) -> Path | None: diff --git a/integrations/cursor-plugin/core/telemetry.py b/integrations/cursor-plugin/core/telemetry.py index f46e1bf55..9084c8a24 100644 --- a/integrations/cursor-plugin/core/telemetry.py +++ b/integrations/cursor-plugin/core/telemetry.py @@ -462,10 +462,16 @@ def _claim_spool() -> Path | None: def _sweep_debris(directory: Path) -> None: - """Remove temp files orphaned by a crash between write and rename. + """Remove files nothing else will ever pick up again. - Neither glob in this module matches *.partial, so nothing else would ever - clean them up. + *.partial is a temp file orphaned by a crash between write and rename. + *.corrupt is a batch quarantined for undecodable content. No glob in this + module matches either, so without this they accumulate on disk for the life + of the install. + + Quarantined batches are kept far longer than debris: they are the only + evidence left of events that could not be delivered, and someone diagnosing + a report of missing telemetry has to be able to find one. """ now = time.time() for debris in directory.glob("telemetry-*.partial"): @@ -474,6 +480,12 @@ def _sweep_debris(directory: Path) -> None: debris.unlink() except OSError: continue + for quarantined in directory.glob("telemetry-*.corrupt"): + try: + if now - quarantined.stat().st_mtime > CLAIM_EXPIRY_SECONDS: + quarantined.unlink() + except OSError: + continue def _claim_parked(directory: Path) -> Path | None: diff --git a/integrations/kimi-plugin/core/telemetry.py b/integrations/kimi-plugin/core/telemetry.py index f46e1bf55..9084c8a24 100644 --- a/integrations/kimi-plugin/core/telemetry.py +++ b/integrations/kimi-plugin/core/telemetry.py @@ -462,10 +462,16 @@ def _claim_spool() -> Path | None: def _sweep_debris(directory: Path) -> None: - """Remove temp files orphaned by a crash between write and rename. + """Remove files nothing else will ever pick up again. - Neither glob in this module matches *.partial, so nothing else would ever - clean them up. + *.partial is a temp file orphaned by a crash between write and rename. + *.corrupt is a batch quarantined for undecodable content. No glob in this + module matches either, so without this they accumulate on disk for the life + of the install. + + Quarantined batches are kept far longer than debris: they are the only + evidence left of events that could not be delivered, and someone diagnosing + a report of missing telemetry has to be able to find one. """ now = time.time() for debris in directory.glob("telemetry-*.partial"): @@ -474,6 +480,12 @@ def _sweep_debris(directory: Path) -> None: debris.unlink() except OSError: continue + for quarantined in directory.glob("telemetry-*.corrupt"): + try: + if now - quarantined.stat().st_mtime > CLAIM_EXPIRY_SECONDS: + quarantined.unlink() + except OSError: + continue def _claim_parked(directory: Path) -> Path | None: diff --git a/integrations/mem0-agent-plugin/core/telemetry.py b/integrations/mem0-agent-plugin/core/telemetry.py index f46e1bf55..9084c8a24 100644 --- a/integrations/mem0-agent-plugin/core/telemetry.py +++ b/integrations/mem0-agent-plugin/core/telemetry.py @@ -462,10 +462,16 @@ def _claim_spool() -> Path | None: def _sweep_debris(directory: Path) -> None: - """Remove temp files orphaned by a crash between write and rename. + """Remove files nothing else will ever pick up again. - Neither glob in this module matches *.partial, so nothing else would ever - clean them up. + *.partial is a temp file orphaned by a crash between write and rename. + *.corrupt is a batch quarantined for undecodable content. No glob in this + module matches either, so without this they accumulate on disk for the life + of the install. + + Quarantined batches are kept far longer than debris: they are the only + evidence left of events that could not be delivered, and someone diagnosing + a report of missing telemetry has to be able to find one. """ now = time.time() for debris in directory.glob("telemetry-*.partial"): @@ -474,6 +480,12 @@ def _sweep_debris(directory: Path) -> None: debris.unlink() except OSError: continue + for quarantined in directory.glob("telemetry-*.corrupt"): + try: + if now - quarantined.stat().st_mtime > CLAIM_EXPIRY_SECONDS: + quarantined.unlink() + except OSError: + continue def _claim_parked(directory: Path) -> Path | None: