Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
10 changes: 5 additions & 5 deletions src/automation/autonomousRunner.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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();
Expand Down
8 changes: 3 additions & 5 deletions src/automation/durableRunCoordinator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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
Expand Down
98 changes: 98 additions & 0 deletions src/automation/rateLimitBackoff.test.ts
Original file line number Diff line number Diff line change
@@ -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();
});
});
124 changes: 124 additions & 0 deletions src/automation/rateLimitBackoff.ts
Original file line number Diff line number Diff line change
@@ -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> | 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<void> {
// 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();
Loading
Loading