From 80ece4dc3da6f3301357a5ee4cae310e7e639331 Mon Sep 17 00:00:00 2001 From: Lily Shen <115414357+lilyshen0722@users.noreply.github.com> Date: Fri, 21 Aug 2026 20:54:48 -0700 Subject: [PATCH 1/2] fix(cli): back off and escalate when the poll itself fails (TASK-025) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `agent run`'s tick set `nextPollDelayMs = intervalMs` and only ever reassigned it inside the spawn-retry branch. A fetch that threw never reached that branch, so a network failure retried at a flat 5s forever — no backoff, no ceiling, no escalation. Measured cost: 797 consecutive `fetch failed` across three seats, ~66 minutes at 5s, whose only trace was one indistinct `onError` line per attempt. The harm was not the retry rate; it was that 797 failures produced no signal distinguishable from one. Adds `cli/src/lib/poll-retry.js` — exponential from the poll interval, bounded at 15 min (the spawn path's ceiling), with clamped anti-herd jitter so seats that lost the same upstream do not retry in lockstep. `escalate` fires on five thresholds (3, 10, 30, 60, 120), so a sustained outage announces itself at widening intervals instead of disappearing. Deliberately NOT a stop. `MAX_AUTH_ERRORS` halts the loop because a rejected token cannot heal itself and retrying is pure cost. A network failure usually can, and a seat that stops on one is dead until a human notices — so this backs off and stays alive. The counter resets beside `consecutiveAuthErrors` on any successful fetch, so a recovered outage costs nothing on the next blip. 7 unit tests on the policy; full CLI suite 24/24 suites, 334 passed. NOT COVERED, stated rather than implied: the wiring in agent.js has no test. Every existing run-loop failure test mocks a SPAWN rejection, not a fetch rejection — which is why this path went four days without one. The unit tests prove the policy; they do not prove the call site. A run-loop test that makes `client.get` reject and asserts the scheduled delay grows is the right follow-up, and it needs `makeClient` to take an `eventsError`. Co-Authored-By: Claude Opus 5 --- cli/__tests__/poll-retry.test.mjs | 71 +++++++++++++++++++++++++++ cli/src/commands/agent.js | 30 ++++++++++++ cli/src/lib/poll-retry.js | 81 +++++++++++++++++++++++++++++++ 3 files changed, 182 insertions(+) create mode 100644 cli/__tests__/poll-retry.test.mjs create mode 100644 cli/src/lib/poll-retry.js diff --git a/cli/__tests__/poll-retry.test.mjs b/cli/__tests__/poll-retry.test.mjs new file mode 100644 index 0000000000..289fe6a4a4 --- /dev/null +++ b/cli/__tests__/poll-retry.test.mjs @@ -0,0 +1,71 @@ +/** + * TASK-025: the run loop had a circuit-breaker for auth errors and none for + * network errors, so a failed fetch retried at a flat interval forever with + * one indistinct log line per attempt. + * + * Each case below pins a property the flat-retry version got wrong. The + * escalation cases matter most: the measured outage was 797 consecutive + * failures whose only trace was 797 identical lines, so "does a sustained + * outage announce itself" is the actual defect, not the delay curve. + */ +import { describe, expect, it } from '@jest/globals'; +import { + POLL_ESCALATE_AT, + POLL_RETRY_MAX_MS, + pollRetryPolicy, +} from '../src/lib/poll-retry.js'; + +const at = (n, over = {}) => pollRetryPolicy({ + consecutiveFailures: n, + intervalMs: 5000, + ...over, +}); + +describe('pollRetryPolicy', () => { + it('costs a single blip nothing — the first retry is still the poll interval', () => { + expect(at(1).delayMs).toBe(5000); + }); + + it('backs off exponentially after that, which flat retry never did', () => { + expect(at(2).delayMs).toBe(10000); + expect(at(3).delayMs).toBe(20000); + expect(at(4).delayMs).toBe(40000); + }); + + it('bounds the backoff, so a long outage does not schedule a retry past the ceiling', () => { + // 2^30 * 5s is ~178 years without the clamp. + const far = at(30); + expect(far.delayMs).toBe(POLL_RETRY_MAX_MS); + expect(far.atCeiling).toBe(true); + }); + + it('escalates on the declared thresholds and stays quiet between them', () => { + // The whole point: 797 failures must not produce 797 equally loud lines. + expect(at(3).escalate).toBe(true); + expect(at(10).escalate).toBe(true); + expect(at(4).escalate).toBe(false); + expect(at(9).escalate).toBe(false); + expect(at(797).escalate).toBe(false); + }); + + it('escalates a bounded number of times across a very long outage', () => { + const loud = Array.from({ length: 800 }, (_, i) => at(i + 1)) + .filter((r) => r.escalate).length; + expect(loud).toBe(POLL_ESCALATE_AT.length); + }); + + it('applies anti-herd jitter, clamped so a bad caller cannot widen the window', () => { + expect(at(2, { jitterRatio: 0.2 }).delayMs).toBe(12000); + // Over the cap: clamped to 0.2 rather than honoured. + expect(at(2, { jitterRatio: 5 }).delayMs).toBe(12000); + expect(at(2, { jitterRatio: -1 }).delayMs).toBe(10000); + expect(at(2, { jitterRatio: NaN }).delayMs).toBe(10000); + }); + + it('falls back to a sane interval rather than NaN when the caller passes junk', () => { + expect(pollRetryPolicy({ consecutiveFailures: 1, intervalMs: 0 }).delayMs).toBe(5000); + expect(pollRetryPolicy({ consecutiveFailures: 1, intervalMs: -1 }).delayMs).toBe(5000); + expect(pollRetryPolicy({ consecutiveFailures: 0, intervalMs: 5000 }).delayMs).toBe(5000); + expect(pollRetryPolicy({ intervalMs: 5000 }).delayMs).toBe(5000); + }); +}); diff --git a/cli/src/commands/agent.js b/cli/src/commands/agent.js index 224243870c..0aca631ada 100644 --- a/cli/src/commands/agent.js +++ b/cli/src/commands/agent.js @@ -29,6 +29,7 @@ import { recordHandledEvent, } from '../lib/session-store.js'; import { readLongTerm, syncBack } from '../lib/memory-bridge.js'; +import { pollRetryPolicy } from '../lib/poll-retry.js'; import { detectMemorySources, composeImport, importMemory } from '../lib/memory-import.js'; import { detectSkills, importSkills } from '../lib/skills-import.js'; import { parseEnvironmentFile, resolveWorkspace } from '../lib/environment.js'; @@ -732,6 +733,7 @@ export const performRun = ({ // reprovision-all; 5+ wastes rate-limit budget after the real-revoke case. let consecutiveAuthErrors = 0; const MAX_AUTH_ERRORS = 3; + let consecutivePollFailures = 0; let consecutiveSpawnFailures = 0; const spawnJitterRatio = retryJitterRatio ?? spawnRetryJitter(agentName); // Per-seat cascade state — lives with the process, like the session store. @@ -1175,6 +1177,7 @@ export const performRun = ({ agentName, instanceId, limit: 10, }); consecutiveAuthErrors = 0; + consecutivePollFailures = 0; for (const event of events) { if (!running) break; let result; @@ -1265,6 +1268,33 @@ export const performRun = ({ running = false; return; } + } else { + // TASK-025: everything that is not an auth rejection used to fall + // through to `onError` with `nextPollDelayMs` still at `intervalMs`, + // because that variable is only reassigned in the spawn-retry branch + // above and a failed fetch never reaches it. So a network outage + // retried flat at the poll interval, forever, with one indistinct + // line per attempt — 797 consecutive `fetch failed` across three + // seats, ~66 minutes, and nothing that read as an outage. + // + // Deliberately NOT a stop. `MAX_AUTH_ERRORS` halts because a rejected + // token cannot heal itself; a network failure usually can, and a seat + // that stops on one is dead until a human notices. + consecutivePollFailures += 1; + const retry = pollRetryPolicy({ + consecutiveFailures: consecutivePollFailures, + intervalMs, + jitterRatio: spawnJitterRatio, + }); + nextPollDelayMs = retry.delayMs; + if (retry.escalate) { + log( + `poll failed ${consecutivePollFailures}x in a row ` + + `(${err?.message || 'unknown error'}) — backing off to ` + + `${formatRetryDelay(retry.delayMs)}` + + `${retry.atCeiling ? ', at the ceiling' : ''}`, + ); + } } onError?.(err); } diff --git a/cli/src/lib/poll-retry.js b/cli/src/lib/poll-retry.js new file mode 100644 index 0000000000..3cab9f04b6 --- /dev/null +++ b/cli/src/lib/poll-retry.js @@ -0,0 +1,81 @@ +/** + * Retry policy for the run loop's own FETCH failures (TASK-025). + * + * `spawn-retry.js` bounds failures of the subprocess a poll produces. This + * bounds failures of the poll itself — the request to + * `/api/agents/runtime/events` that has to succeed before there is anything to + * spawn. They are different failures with different remedies and, until now, + * only one of them had a policy. + * + * The gap, measured: `agent run`'s tick set `nextPollDelayMs = intervalMs` and + * only ever reassigned it inside the spawn-retry branch. A fetch that threw + * never reached that branch, so a network failure retried at a flat 5s + * forever — no backoff, no ceiling, and no escalation. 797 consecutive + * `fetch failed` ran across three seats that way: ~66 minutes at 5s, and the + * only trace was one `onError` line per attempt in a log nobody was tailing. + * + * Two deliberate differences from the auth path directly above it: + * + * 1. NO STOP. `MAX_AUTH_ERRORS` halts the loop because a rejected token does + * not heal on its own — retrying is pure cost and the operator must act. + * A network failure is usually transient, and a seat that stops on one is + * dead until someone notices. So this backs off and stays alive. + * + * 2. ESCALATION IS THE POINT. The harm in the measured outage was not the + * retry rate, it was that 797 failures produced no signal distinguishable + * from one failure. `escalate` fires on a small set of thresholds so a + * sustained outage announces itself at widening intervals instead of + * disappearing into per-attempt noise. + */ + +// Same ceiling as the spawn path: past this, more waiting buys nothing and a +// human is the only thing that resolves it. +export const POLL_RETRY_MAX_MS = 15 * 60 * 1000; + +// Backoff starts only after the first failure has been retried once at the +// normal interval, so a single blip costs nothing. +export const POLL_BACKOFF_AFTER = 1; + +// Failure counts that emit a loud line. Chosen against the measured outage: +// at intervalMs=5000 these land at roughly 15s, 1min, 5min, 20min and 1h of +// sustained failure, so an operator reading the log sees the shape of the +// outage rather than 797 identical lines. +export const POLL_ESCALATE_AT = Object.freeze([3, 10, 30, 60, 120]); + +/** + * Next poll delay after a failed fetch, and whether this attempt should be + * announced loudly. + * + * Exponential from `intervalMs`, bounded by POLL_RETRY_MAX_MS. Jitter is the + * same anti-herd offset the spawn path uses — without it, every seat that lost + * the same upstream retries in lockstep and re-creates the thundering herd on + * recovery. + */ +export const pollRetryPolicy = ({ + consecutiveFailures, + intervalMs, + jitterRatio = 0, +}) => { + const safeIntervalMs = Number.isFinite(intervalMs) && intervalMs > 0 ? intervalMs : 5000; + const failureCount = Number.isInteger(consecutiveFailures) && consecutiveFailures > 0 + ? consecutiveFailures + : 1; + + const steps = Math.max(0, failureCount - POLL_BACKOFF_AFTER); + const raw = safeIntervalMs * (2 ** steps); + const bounded = Math.min(POLL_RETRY_MAX_MS, raw); + + // Clamp rather than trust the caller: a jitterRatio above the cap would + // widen the herd window instead of narrowing it. + const safeJitter = Number.isFinite(jitterRatio) + ? Math.min(0.2, Math.max(0, jitterRatio)) + : 0; + + return { + delayMs: Math.round(bounded * (1 + safeJitter)), + escalate: POLL_ESCALATE_AT.includes(failureCount), + atCeiling: bounded >= POLL_RETRY_MAX_MS, + }; +}; + +export default pollRetryPolicy; From cc879d1ca739f4dfdd5516cabb7249f3aba85180 Mon Sep 17 00:00:00 2001 From: Lily Shen <115414357+lilyshen0722@users.noreply.github.com> Date: Fri, 21 Aug 2026 20:59:50 -0700 Subject: [PATCH 2/2] chore(cli): bump to 0.1.16 for the poll-retry fix MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `Source changed ⇒ version bumped` failed on #1092: agent.js and the new poll-retry lib both moved while cli/package.json stayed at 0.1.15. Caught by @pod-architect, and the guard is right to block this one in particular. The CLI reaches a seat by publish → staged copy → process restart, and the staged copy is resolved by version. An unbumped version is an outage fix that ships and silently never arrives — which is the same failure class the fix itself addresses, one layer out. Co-Authored-By: Claude Opus 5 --- cli/package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cli/package.json b/cli/package.json index f3fff2bb59..c0683914a8 100644 --- a/cli/package.json +++ b/cli/package.json @@ -1,6 +1,6 @@ { "name": "@commonlyai/cli", - "version": "0.1.15", + "version": "0.1.16", "license": "Apache-2.0", "description": "The Commonly CLI \u2014 connect agents, manage pods, iterate fast", "type": "module",