From 929fcc63f8e758abe04589621d84e589103d2c4f Mon Sep 17 00:00:00 2001 From: Heewon Oh Date: Tue, 3 Nov 2026 23:52:28 +0900 Subject: [PATCH 1/2] fix(automation): ramp rate-limit backoff to an hour and report a sustained streak MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Round-1 redteam B-F1 (L3/ops): rate_limited without a reset time retried every 60 s forever (per issue and daemon-wide), excluded from every circuit — one issue made 804 attempts in three days with no notice to anyone. - rateLimitBackoff.ts: delay doubles 60 s -> 1 h cap; per issue by attempt number (retryAtFor), daemon-wide by a consecutive-hit streak (globalRateLimit, reset after 10 quiet minutes past a pause). A provider reset time still wins and never shortens a pause in force. - A streak past 30 min sets health `degraded` (status stays ok) and sends one notification per streak via the configured notifier. - autonomousRunner.ts and durableRunCoordinator.ts keep their line counts (pre-commit ratchet): call sites only, comments rewrapped. Assumption: 30 min marks a quota problem rather than a burst; ten quiet minutes after a pause end a streak. --- CHANGELOG.md | 1 + src/automation/autonomousRunner.ts | 10 +-- src/automation/durableRunCoordinator.ts | 8 +- src/automation/rateLimitBackoff.test.ts | 93 ++++++++++++++++++++ src/automation/rateLimitBackoff.ts | 111 ++++++++++++++++++++++++ src/support/healthEndpoint.ts | 14 +++ 6 files changed, 227 insertions(+), 10 deletions(-) create mode 100644 src/automation/rateLimitBackoff.test.ts create mode 100644 src/automation/rateLimitBackoff.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index c815b8e1..49089843 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 delay from 60 s to a one-hour cap: per issue by attempt number, and daemon-wide by a streak of consecutive rate-limited results (`globalRateLimit`, which starts over after ten quiet minutes past a pause). 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..a8fa94c8 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, attemptNo); 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..848e9ad0 --- /dev/null +++ b/src/automation/rateLimitBackoff.test.ts @@ -0,0 +1,93 @@ +// 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; + // Per issue: the attempt number drives the ramp. + const delays = [1, 2, 3, 4, 5, 6, 7, 8, 20].map((attempt) => retryAtFor(rateLimited(), now, attempt) - now); + expect(delays).toEqual([1, 2, 4, 8, 16, 32, 60, 60, 60].map((m) => m * MIN)); + 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)); + + // 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..f15d8ae2 --- /dev/null +++ b/src/automation/rateLimitBackoff.ts @@ -0,0 +1,111 @@ +// ============================================ +// 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 delay doubles from 60 s to a one-hour cap while the limiting +// continues, 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 on its `attemptNo`-th attempt. */ +export function rateLimitRetryAt(resetsAt: number | undefined, now: number, attemptNo: number): number { + return resetsAt ?? now + rateLimitDelayMs(attemptNo); +} + +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; + } + + /** 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 { From 06507c4b1f6b3d52bef3bac2da2d3583132d8f11 Mon Sep 17 00:00:00 2001 From: Heewon Oh Date: Tue, 3 Nov 2026 23:54:26 +0900 Subject: [PATCH 2/2] fix(automation): ramp the daemon-wide rate-limit streak, not the issue's attempt count The full suite caught it: AGT-4673 pins that a rate limit is not slowed by the issue's attempt count, which includes ordinary failures (12 failures would have meant an hour's wait on the first rate limit). The ramp now lives only in the daemon-wide streak; a rate-limited issue waits for that streak's pause (retryAt = max(now + 60 s, pausedUntil)) and a provider reset time still wins. --- CHANGELOG.md | 2 +- src/automation/durableRunCoordinator.ts | 2 +- src/automation/rateLimitBackoff.test.ts | 11 ++++++++--- src/automation/rateLimitBackoff.ts | 23 ++++++++++++++++++----- 4 files changed, 28 insertions(+), 10 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 49089843..4d8573cd 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -37,7 +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 delay from 60 s to a one-hour cap: per issue by attempt number, and daemon-wide by a streak of consecutive rate-limited results (`globalRateLimit`, which starts over after ten quiet minutes past a pause). 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. +- **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/durableRunCoordinator.ts b/src/automation/durableRunCoordinator.ts index a8fa94c8..ed389e0c 100644 --- a/src/automation/durableRunCoordinator.ts +++ b/src/automation/durableRunCoordinator.ts @@ -151,7 +151,7 @@ function fencedResult(result: PipelineResult): PipelineResult { export const RETRY_RAMP_FROM_ATTEMPT = 4; export function retryAtFor(result: PipelineResult, now: number, attemptNo = 1): number { - if (result.finalStatus === 'rate_limited') return rateLimitRetryAt(result.rateLimitResetsAt, now, attemptNo); + 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 index 848e9ad0..9871c5a5 100644 --- a/src/automation/rateLimitBackoff.test.ts +++ b/src/automation/rateLimitBackoff.test.ts @@ -27,9 +27,6 @@ afterEach(() => { describe('rate-limit backoff', () => { it('ramps rate_limited retries without a reset time up to one hour', () => { const now = 1_000_000; - // Per issue: the attempt number drives the ramp. - const delays = [1, 2, 3, 4, 5, 6, 7, 8, 20].map((attempt) => retryAtFor(rateLimited(), now, attempt) - now); - expect(delays).toEqual([1, 2, 4, 8, 16, 32, 60, 60, 60].map((m) => m * MIN)); expect(rateLimitDelayMs(1)).toBe(RATE_LIMIT_BASE_MS); expect(rateLimitDelayMs(1000)).toBe(RATE_LIMIT_MAX_MS); @@ -44,6 +41,14 @@ describe('rate-limit backoff', () => { } 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); }); diff --git a/src/automation/rateLimitBackoff.ts b/src/automation/rateLimitBackoff.ts index f15d8ae2..7011ae68 100644 --- a/src/automation/rateLimitBackoff.ts +++ b/src/automation/rateLimitBackoff.ts @@ -10,8 +10,9 @@ // rate_limited, at 576 a day, and nobody was told. A quota that is gone for // hours was being asked every minute. // -// Now the delay doubles from 60 s to a one-hour cap while the limiting -// continues, a provider's reset time still wins, and a streak that lasts past +// 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. */ @@ -32,9 +33,16 @@ export function rateLimitDelayMs(hit: number): number { return Math.min(RATE_LIMIT_MAX_MS, RATE_LIMIT_BASE_MS * 2 ** exponent); } -/** When to retry a rate-limited issue on its `attemptNo`-th attempt. */ -export function rateLimitRetryAt(resetsAt: number | undefined, now: number, attemptNo: number): number { - return resetsAt ?? now + rateLimitDelayMs(attemptNo); +/** + * 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 { @@ -80,6 +88,11 @@ export class RateLimitStreak { 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;