diff --git a/CHANGELOG.md b/CHANGELOG.md index c815b8e1..4d8573cd 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -37,6 +37,7 @@ ### Fixed +- **A rate limit without a reset time backs off up to an hour, and a sustained one is reported (round-1 redteam B-F1).** `rate_limited` results without a provider reset time came back after a fixed 60 seconds — per issue (`retryAtFor`) and daemon-wide (`rateLimitUntil`) — and rate-limited attempts are excluded from every circuit, so on the live ledger one issue made 804 attempts in three days (792 rate limited, ~576 a day) while nobody was told. `src/automation/rateLimitBackoff.ts` now doubles the daemon-wide pause from 60 s to a one-hour cap over a streak of consecutive rate-limited results (`globalRateLimit`, which starts over after ten quiet minutes past a pause), and a rate-limited issue waits for that pause instead of a flat minute. The issue's own attempt count still plays no part — it includes ordinary failures, so an issue that failed twelve times would otherwise wait an hour on its first rate limit (AGT-4673). A provider reset time is still used as given, and never shortens a pause already in force. A streak longer than 30 minutes adds `degraded: { reason: 'rate_limited', since, until, hits }` to `/api/health` (whose `status` stays `ok`: the process is alive, it is just not doing work) and sends **one** notification through the configured notifier per streak. - **The worker can no longer edit the tests and test-runner config that grade it (round-1 redteam A-F3).** `protectedFiles` has been enforced by the `edit_file`/`write_file` tools and the agentic loop since INT-1914, but no production caller ever set it. `src/agents/protectedFiles.ts` (`computeProtectedFiles`) now runs once at the start of every pair-pipeline run and protects every tracked test file (the kinds the deleted-test guard counts: JS/TS `*.test.*`/`*.spec.*`, pytest modules, Go `_test.go`) and every tracked test-runner config verify pins to base (`vitest.config.*`, `jest.config.*`, `pytest.ini`, `conftest.py`, `tox.ini`, `.coveragerc`); the list reaches the worker as `WorkerOptions.protectedFiles`. New test files stay writable. Protection lifts for files the issue names (by path, or by a basename only one file carries) and for globs in the DoD contract's new `"tests": { "allowEdits": [...] }`. **Behaviour change:** a task that has to rewrite an existing test now needs to name the file or allow it in the DoD; otherwise the worker is told the file is part of the verification harness. Delegated CLI adapters (`claude`/`codex`/`cursor`) still run their own tool loop and do not receive the list — the active-test gate and the trusted verify config are their backstop. A shell command (`sed -i` on a test) is not covered by the tool-level check either; the same two gates catch its effect. - **Deterministic verification runs the change with the base revision's test-runner config (round-1 redteam A-F2).** `runTrustedCommand` pinned package.json `scripts` to base, but everything the runner then reads to decide which tests run and what counts as passing came from the change under test — a `test.exclude` in `vitest.config.ts`, a lowered coverage threshold, a `collect_ignore` in `conftest.py` — so the head run could pass a change it never measured, and with head passing the base comparison never ran. `src/verify/trustedConfig.ts` (`pinTrustedTestConfig`) now runs in the head sandbox after the symlink validation: every changed `vitest.config.*`, `vitest.workspace.*`, `jest.config.*`, `pytest.ini`, `conftest.py` (any directory), `tox.ini` and `.coveragerc` gets its base version. Where pinning would also throw away a legitimate change, verification fails closed with a `[security]` reason instead: a test-runner config the base never had, changed `[tool.pytest*]`/`[tool.coverage*]` sections of `pyproject.toml`, changed `[tool:pytest]`/`[coverage:*]` sections of `setup.cfg`, changed `jest`/`vitest`/`c8`/`nyc` keys in a `package.json`, and a changed `vite.config.*` that carries a `test:` block. Dependency-only edits to those manifests are unaffected, and the live checkout is never modified. **Behaviour change:** a change that legitimately edits test config is now verified with the old config, or held for a person when that cannot be done. - **An unattended run that lowers the active test count is held for a person instead of published (round-1 redteam A-F1).** The deleted-test guard (AGT-4277) only commented on the PR afterwards, counted `it.skip(` the same as `it(`, and read JS/TS files only — so turning a failing acceptance test into a skip, or deleting a pytest case, produced green CI and a ready PR with nothing in the way. `countTestCases` now counts **active** cases: `.skip`, `.todo`, `.skipIf`, `xit`/`xtest` and cases inside `describe.skip`/`xdescribe` are excluded, pytest `def test_*` (minus `@pytest.mark.skip`/`skipif`, `@unittest.skip*`, a skipped class, or a module-level `pytestmark` skip) and Go `func Test*` (minus a leading `t.Skip`) are counted. `src/automation/testRemovalGate.ts` runs at the top of `publishApprovedWork`, before the lease fence moves the run to PUBLISHING: it measures the worktree as it will be committed (tracked edits, untracked files, renames — through a throwaway index, never the worker's own) against the merge base, and a per-file loss parks the run with `active_tests_removed` and the file list; the existing parked path then publishes it as a draft. An issue opts out with `"tests": { "allowRemoval": true }` in its `openswarm:dod` block (`docs/DOD_CONTRACT.md`). A measurement that breaks fails **closed**; a branch with no merge base is the one exception, said in the log, as the PR-time check already does. diff --git a/src/automation/autonomousRunner.ts b/src/automation/autonomousRunner.ts index 07d227a2..ca924880 100644 --- a/src/automation/autonomousRunner.ts +++ b/src/automation/autonomousRunner.ts @@ -152,6 +152,7 @@ import { import { buildInstructionCapsule } from '../agents/instructionCapsule.js'; import type { IntegrationConflictEvidence } from './integrationCoordinator.js'; import { coordinatorResolutionComment, planCoordinatorResolution } from './coordinatorResolution.js'; +import { globalRateLimit } from './rateLimitBackoff.js'; // Re-export types and integration setters (used by service.ts) export { setNotifier, setTaskSource } from './runnerExecution.js'; @@ -922,11 +923,10 @@ export class AutonomousRunner { const taskCtx = this.formatTaskContext(task); this.recordPipelineHistory(task, result); - // Rate-limited: pause execution until the quota resets. Do NOT count it as a - // task failure, run the rejection/block path, or post a Linear comment — - // that is exactly the retry-spam this issue fixes. (INT-1906) + // Rate-limited: pause (ramping, rateLimitBackoff.ts) — not a task failure, no rejection + // path, no Linear comment; that is the retry-spam INT-1906 fixed. if (result.finalStatus === 'rate_limited') { - const resetsAt = result.rateLimitResetsAt ?? Date.now() + 60_000; + const resetsAt = globalRateLimit.hit(result.rateLimitResetsAt); this.rateLimitUntil = resetsAt; const waitSec = Math.max(0, Math.ceil((resetsAt - Date.now()) / 1000)); const resetsLabel = new Date(resetsAt).toISOString(); @@ -3646,7 +3646,7 @@ export class AutonomousRunner { // reporting or state change — no failure count, no card spam. Same as the // scheduler 'failed' handler's rate_limited branch. (INT-1906) if (result.finalStatus === 'rate_limited') { - const resetsAt = result.rateLimitResetsAt ?? Date.now() + 60_000; + const resetsAt = globalRateLimit.hit(result.rateLimitResetsAt); this.rateLimitUntil = resetsAt; const waitSec = Math.max(0, Math.ceil((resetsAt - Date.now()) / 1000)); const resetsLabel = new Date(resetsAt).toISOString(); diff --git a/src/automation/durableRunCoordinator.ts b/src/automation/durableRunCoordinator.ts index 0a7eaf4e..ed389e0c 100644 --- a/src/automation/durableRunCoordinator.ts +++ b/src/automation/durableRunCoordinator.ts @@ -13,6 +13,7 @@ import type { TaskItem } from '../orchestration/decisionEngine.js'; import { normalizeProjectPath } from '../orchestration/taskScheduler.js'; import type { WorktreeInfo } from '../support/worktreeManager.js'; import { ACTIVE_LEASE_STATES } from './runLedgerTypes.js'; +import { rateLimitRetryAt } from './rateLimitBackoff.js'; import { RunLedger, type EffectClaim, @@ -146,14 +147,11 @@ function fencedResult(result: PipelineResult): PipelineResult { }; } -/** - * Attempt number from which a failing run's retry delay doubles per attempt. The - * runner's idle fill reads the same value to leave such a run alone (AGT-4675). - */ +/** Attempt from which a failing run's retry delay doubles; idle fill reads it too (AGT-4675). */ export const RETRY_RAMP_FROM_ATTEMPT = 4; export function retryAtFor(result: PipelineResult, now: number, attemptNo = 1): number { - if (result.finalStatus === 'rate_limited') return result.rateLimitResetsAt ?? now + 60_000; + if (result.finalStatus === 'rate_limited') return rateLimitRetryAt(result.rateLimitResetsAt, now); if (result.finalStatus === 'deferred') return Math.max(now + 1_000, result.retryAt ?? 0); if (result.finalStatus === 'superseded') { // A first overlap can disappear quickly, but a still-open PR or persistent diff --git a/src/automation/rateLimitBackoff.test.ts b/src/automation/rateLimitBackoff.test.ts new file mode 100644 index 00000000..9871c5a5 --- /dev/null +++ b/src/automation/rateLimitBackoff.test.ts @@ -0,0 +1,98 @@ +// Round-1 redteam B-F1 (2026-11-03): a rate_limited result without a reset time +// came back every 60 s forever — 804 attempts on one issue in three days — and +// nothing told the operator. The delay now ramps to an hour and a sustained +// streak shows up in health and in one notification. + +import { afterEach, describe, expect, it, vi } from 'vitest'; + +const reportToDiscord = vi.hoisted(() => vi.fn(async () => {})); +vi.mock('./runnerExecution.js', () => ({ reportToDiscord })); + +import { retryAtFor } from './durableRunCoordinator.js'; +import { + DEGRADED_AFTER_MS, globalRateLimit, RATE_LIMIT_BASE_MS, RATE_LIMIT_MAX_MS, RateLimitStreak, rateLimitDelayMs, +} from './rateLimitBackoff.js'; +import { buildHealthPayload } from '../support/healthEndpoint.js'; +import type { PipelineResult } from '../agents/pairPipeline.js'; + +const rateLimited = (rateLimitResetsAt?: number) => ({ finalStatus: 'rate_limited', rateLimitResetsAt }) as unknown as PipelineResult; +const MIN = 60_000; + +afterEach(() => { + vi.useRealTimers(); + globalRateLimit.reset(); + reportToDiscord.mockClear(); +}); + +describe('rate-limit backoff', () => { + it('ramps rate_limited retries without a reset time up to one hour', () => { + const now = 1_000_000; + expect(rateLimitDelayMs(1)).toBe(RATE_LIMIT_BASE_MS); + expect(rateLimitDelayMs(1000)).toBe(RATE_LIMIT_MAX_MS); + + // Daemon-wide: consecutive hits inside one streak ramp the global pause. + const streak = new RateLimitStreak(() => {}); + let t = now; + const pauses: number[] = []; + for (let i = 0; i < 8; i++) { + const until = streak.hit(undefined, t); + pauses.push(until - t); + t = until; // the next attempt runs when the pause ends, and is limited again + } + expect(pauses).toEqual([1, 2, 4, 8, 16, 32, 60, 60].map((m) => m * MIN)); + + // Per issue: a rate-limited issue waits for the daemon-wide pause, not a flat + // minute — and its own attempt count (ordinary failures included) plays no part. + for (let i = 0; i < 6; i++) globalRateLimit.hit(undefined, now); + const pausedUntil = globalRateLimit.pausedUntil(now); + expect(pausedUntil - now).toBe(32 * MIN); + expect(retryAtFor(rateLimited(), now, 1)).toBe(pausedUntil); + expect(retryAtFor(rateLimited(), now, 40)).toBe(pausedUntil); + + // A quiet stretch after the pause expires starts a new streak at 60 s. + expect(streak.hit(undefined, t + 30 * MIN) - (t + 30 * MIN)).toBe(RATE_LIMIT_BASE_MS); + }); + + it('uses the provider reset time when given', () => { + const now = 5_000_000; + expect(retryAtFor(rateLimited(now + 7 * MIN), now, 9)).toBe(now + 7 * MIN); + const streak = new RateLimitStreak(() => {}); + expect(streak.hit(now + 3 * MIN, now)).toBe(now + 3 * MIN); + // An earlier reset time never shortens a pause already in force. + expect(streak.hit(now + MIN, now + 10_000)).toBe(now + 3 * MIN); + }); + + it('reports degraded health after sustained rate limiting', async () => { + const start = Date.UTC(2026, 10, 3, 12, 0, 0); + vi.useFakeTimers(); + vi.setSystemTime(start); + + // Hits at 0, 1, 3, 7 and 15 min: a streak, but not yet a sustained one. + let t = start; + for (let i = 0; i < 5; i++) { + vi.setSystemTime(t); + t = globalRateLimit.hit(undefined, t); + expect(buildHealthPayload({ credentials: {} }).degraded).toBeUndefined(); + } + expect(reportToDiscord).not.toHaveBeenCalled(); + // The retry at 31 min is limited again: health says why and until when, and one notice goes out. + vi.setSystemTime(t); + const until = globalRateLimit.hit(undefined, t); + const health = buildHealthPayload({ credentials: {} }); + expect(health.status).toBe('ok'); + expect(health.degraded).toMatchObject({ reason: 'rate_limited', since: new Date(start).toISOString(), until: new Date(until).toISOString(), hits: 6 }); + await vi.waitFor(() => expect(reportToDiscord).toHaveBeenCalledTimes(1)); + expect(String(reportToDiscord.mock.calls[0]?.[0])).toContain('rate limited for 31 min'); + t = until; + + // Further hits in the same streak do not notify again. + globalRateLimit.hit(undefined, t); + globalRateLimit.hit(undefined, t + MIN); + await Promise.resolve(); + expect(reportToDiscord).toHaveBeenCalledTimes(1); + + // Long after the last pause, the streak is over and health is clean again. + vi.setSystemTime(t + 3 * 60 * MIN); + expect(buildHealthPayload({ credentials: {} }).degraded).toBeUndefined(); + }); +}); diff --git a/src/automation/rateLimitBackoff.ts b/src/automation/rateLimitBackoff.ts new file mode 100644 index 00000000..7011ae68 --- /dev/null +++ b/src/automation/rateLimitBackoff.ts @@ -0,0 +1,124 @@ +// ============================================ +// OpenSwarm — rate-limit backoff that ramps and gets noticed +// ============================================ +// +// A `rate_limited` result without a provider reset time used to come back +// after a fixed 60 seconds, both for the issue (`retryAtFor`) and for the +// daemon-wide pause (`rateLimitUntil`), and rate-limited attempts are excluded +// from every circuit. Measured on the live ledger (round-1 redteam B-F1, +// 2026-11-03): one issue made 804 attempts in three days, 792 of them +// rate_limited, at 576 a day, and nobody was told. A quota that is gone for +// hours was being asked every minute. +// +// Now the daemon-wide pause doubles from 60 s to a one-hour cap while the +// limiting continues, a rate-limited issue waits for that pause instead of a +// flat minute, a provider's reset time still wins, and a streak that lasts past +// DEGRADED_AFTER_MS turns health to `degraded` and sends ONE notification. + +/** First delay when the provider gave no reset time. */ +export const RATE_LIMIT_BASE_MS = 60_000; +/** Ceiling for the ramp. */ +export const RATE_LIMIT_MAX_MS = 60 * 60_000; +/** A streak this long is a quota problem, not a burst: report it. */ +export const DEGRADED_AFTER_MS = 30 * 60_000; +/** + * Quiet time after a pause expires before the next hit counts as a NEW streak. + * Long enough that the retry right after a pause is part of the same streak. + */ +const STREAK_GAP_MS = 10 * 60_000; + +/** Delay before the `hit`-th consecutive rate-limited retry (1-based). */ +export function rateLimitDelayMs(hit: number): number { + const exponent = Math.max(0, Math.min(16, Math.floor(hit) - 1)); + return Math.min(RATE_LIMIT_MAX_MS, RATE_LIMIT_BASE_MS * 2 ** exponent); +} + +/** + * When to retry a rate-limited issue. + * + * Not scaled by the issue's own attempt count: that count includes ordinary + * failures, and an issue that failed twelve times would wait an hour on its + * first rate limit (AGT-4673 pins this). The quota is the daemon's, so the + * ramp lives in the daemon-wide streak and the issue waits for its pause. + */ +export function rateLimitRetryAt(resetsAt: number | undefined, now: number, streak: RateLimitStreak = globalRateLimit): number { + return resetsAt ?? Math.max(now + RATE_LIMIT_BASE_MS, streak.pausedUntil(now)); +} + +export interface RateLimitDegradation { + reason: 'rate_limited'; + /** ISO time the current streak began. */ + since: string; + /** ISO time dispatch resumes. */ + until: string; + /** Consecutive rate-limited results in this streak. */ + hits: number; +} + +/** + * Daemon-wide rate-limit streak. One per process (`globalRateLimit`); the + * runner asks it for the pause on every rate-limited result. + */ +export class RateLimitStreak { + private hits = 0; + private startedAt = 0; + private until = 0; + private notified = false; + + constructor( + private readonly notify: (message: string) => Promise | void = defaultNotify, + ) {} + + /** Record a rate-limited result; returns when dispatch may resume. */ + hit(resetsAt: number | undefined, now = Date.now()): number { + if (this.hits > 0 && now > this.until + STREAK_GAP_MS) this.reset(); + if (this.hits === 0) this.startedAt = now; + this.hits++; + this.until = Math.max(this.until, resetsAt ?? now + rateLimitDelayMs(this.hits)); + const degraded = this.degradation(now); + if (degraded && !this.notified) { + this.notified = true; + const minutes = Math.round((now - this.startedAt) / 60_000); + void Promise.resolve(this.notify( + `⏸ OpenSwarm has been rate limited for ${minutes} min (${this.hits} consecutive results). ` + + `Dispatch resumes at ${degraded.until}; retries back off up to ${RATE_LIMIT_MAX_MS / 60_000} min. ` + + 'Check the provider quota or credentials.', + )).catch((error) => console.warn('[RateLimit] Could not send the degraded notice:', error)); + } + return this.until; + } + + /** When the current streak's pause ends, or 0 when no streak is running. */ + pausedUntil(now = Date.now()): number { + return this.hits > 0 && now <= this.until + STREAK_GAP_MS ? this.until : 0; + } + + /** The current degradation, or null when no streak has lasted long enough. */ + degradation(now = Date.now()): RateLimitDegradation | null { + if (this.hits === 0 || now > this.until + STREAK_GAP_MS) return null; + if (now - this.startedAt < DEGRADED_AFTER_MS) return null; + return { + reason: 'rate_limited', + since: new Date(this.startedAt).toISOString(), + until: new Date(this.until).toISOString(), + hits: this.hits, + }; + } + + /** The provider answered: the streak, and its one notification, start over. */ + reset(): void { + this.hits = 0; + this.startedAt = 0; + this.until = 0; + this.notified = false; + } +} + +async function defaultNotify(message: string): Promise { + // Lazy: runnerExecution pulls in the whole runner, and this module is also + // read by the health endpoint. + const { reportToDiscord } = await import('./runnerExecution.js'); + await reportToDiscord(message); +} + +export const globalRateLimit = new RateLimitStreak(); diff --git a/src/support/healthEndpoint.ts b/src/support/healthEndpoint.ts index 42c07d7e..ad07151d 100644 --- a/src/support/healthEndpoint.ts +++ b/src/support/healthEndpoint.ts @@ -18,6 +18,7 @@ import { fileURLToPath } from 'node:url'; import { randomUUID } from 'node:crypto'; import { getHeapStatistics } from 'node:v8'; import { credentialProbeSnapshot } from '../core/credentialProbes.js'; +import { globalRateLimit, type RateLimitDegradation } from '../automation/rateLimitBackoff.js'; export interface HealthPayload { status: 'ok'; @@ -48,6 +49,12 @@ export interface HealthPayload { * contract above is unchanged when nothing was probed. */ credentials?: Record; + /** + * Present only while the daemon is degraded: today, a rate-limit streak that + * has lasted past DEGRADED_AFTER_MS (rateLimitBackoff.ts). `status` stays + * 'ok' — the process is alive and serving; this says it is not doing work. + */ + degraded?: RateLimitDegradation; } const MB = 1024 * 1024; @@ -78,6 +85,7 @@ export function buildHealthPayload( instanceId?: string; memory?: { heapUsedBytes: number; heapLimitBytes: number; rssBytes: number }; credentials?: HealthPayload['credentials']; + degraded?: RateLimitDegradation | null; } = {}, ): HealthPayload { const env = deps.env ?? process.env; @@ -100,9 +108,15 @@ export function buildHealthPayload( uptime_s: Math.floor(uptimeS), ...memoryFields(deps.memory), ...credentialField(deps.credentials), + ...degradedField(deps.degraded), }; } +function degradedField(provided: RateLimitDegradation | null | undefined): Pick { + const degraded = provided === undefined ? globalRateLimit.degradation() : provided; + return degraded ? { degraded } : {}; +} + function credentialField( provided: HealthPayload['credentials'] | undefined, ): Pick {