From 7900a5d8d171a88b73ddf4f006b12be89a41bb0b Mon Sep 17 00:00:00 2001 From: Saket Aryan Date: Thu, 17 Sep 2026 19:43:51 +0530 Subject: [PATCH] docs(plugins): say what the delivery budget actually counts Review finding. The comments described MAX_DELIVERY_ATTEMPTS as a per-batch budget mirroring the Python core's. It is not: Python puts the attempt count in the claim filename so it follows one batch, while this counter lives in the closure and counts consecutive failed flushes, so events captured during an outage join the same queue and are dropped with it. Per-batch accounting would mean an attempt count on every event. The queue is already bounded, so the simpler rule stands; the comments now describe it rather than the Python one. Claude-Session: https://claude.ai/code/session_01C7tEmH86HAr7GoAAKCEHZb --- .../typescript/src/telemetry.ts | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-) diff --git a/integrations/agent-plugin-core/typescript/src/telemetry.ts b/integrations/agent-plugin-core/typescript/src/telemetry.ts index 44334cfb0..51c546403 100644 --- a/integrations/agent-plugin-core/typescript/src/telemetry.ts +++ b/integrations/agent-plugin-core/typescript/src/telemetry.ts @@ -84,10 +84,15 @@ export function errorKind(error: unknown): string { // and lease machinery a correct cross-process spool needs. What that leaves // uncovered is narrow: a session that both starts and ends with no connectivity. const RETRY_BACKOFF_CEILING_MS = 60_000; -// Attempts before a batch is given up on, mirroring the Python core's budget. -// Without one, a payload the server will never accept is retried for the whole -// session and, now that the backlog is preferred over new events, would block -// everything behind it. +// Consecutive failed flushes before the queue is dropped. Deliberately NOT the +// same thing as Python's budget, which rides in the claim filename and so +// follows one batch: this counter lives in the closure and counts the outage, +// not the payload. Events captured between attempts join the same queue and go +// with it. Per-batch accounting would need an attempt count on every event, and +// the queue is already bounded, so the simpler rule is the one in force here. +// Without any bound a payload the server will never accept is retried for the +// whole session and, now that the backlog is preferred over new events, holds +// the queue against everything behind it. const MAX_DELIVERY_ATTEMPTS = 5; export function createTelemetry(config: TelemetryConfig) { @@ -136,7 +141,9 @@ export function createTelemetry(config: TelemetryConfig) { // consecutiveFailures += 1; if (consecutiveFailures >= MAX_DELIVERY_ATTEMPTS) { - // Give up on this batch so it cannot hold the queue for the session. + // Give up on the queue so a failing outage cannot hold it for the + // session. This drops whatever is queued now, which includes events + // captured during the outage, not only the batch that kept failing. consecutiveFailures = 0; retryNotBefore = 0; return;