Skip to content

feat(#160): Add token-bucket rate limiting to AgamemnonClient#268

Open
mvillmow wants to merge 2 commits into
mainfrom
160-auto-impl
Open

feat(#160): Add token-bucket rate limiting to AgamemnonClient#268
mvillmow wants to merge 2 commits into
mainfrom
160-auto-impl

Conversation

@mvillmow

@mvillmow mvillmow commented Jun 4, 2026

Copy link
Copy Markdown
Contributor

Summary

Add a configurable token-bucket rate limiter to all outbound Agamemnon HTTP calls, resolving issue #160 (burst API calls in _provision_agents). The limiter is integrated at a single chokepoint (AgamemnonClient._request_with_retry) for comprehensive coverage across all endpoints and request patterns.

Implementation Details

  • New file: src/telemachy/rate_limiter.py — Async TokenBucket class with acquire(n) that refills at a configurable rate and caps at a burst size
  • Integration: AgamemnonClient now accepts rate_limit_rps and rate_limit_burst kwargs; calls acquire() before each HTTP request attempt
  • Configuration: New env vars AGAMEMNON_RATE_LIMIT_RPS (default 0, disabled) and AGAMEMNON_RATE_LIMIT_BURST (default 16, must be >= 1)
  • Retry backoff cap: Backoff now capped at min(2**attempt, 2.0) per team-knowledge nats-publish-retry-backoff guidance to prevent unbounded delays

Testing

  • 7 unit tests for TokenBucket covering burst consumption, refill, and disabled state
  • 4 config tests verifying env-var binding, default values, and validation (burst=0 rejected)
  • 3 client tests for rate-limiter integration across endpoints and backoff capping
  • 1 executor integration test with 20 agents under rate limiting (0.6–4.0s elapsed)
  • All 64 existing tests still pass; zero regressions

Verification

# Rate limiter disabled by default:
pixi run pytest tests/test_config.py::test_rate_limit_defaults_off -v

# Env-var binding fixed (prior NOGO):
pixi run pytest tests/test_config.py::test_rate_limit_env_var_binding -v

# Burst=0 rejected (no silent clamping):
pixi run pytest tests/test_config.py::test_rate_limit_burst_zero_rejected -v

# Full suite passes:
pixi run pytest tests/ -v --tb=short

Closes #160
Co-Authored-By: Claude Haiku 4.5 noreply@anthropic.com

@mvillmow mvillmow left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Correct, complete, disabled-by-default; env binding fixed+tested. Two minor findings: off-scope backoff cap, stale test-math comment. GO.

Comment thread src/telemachy/agamemnon_client.py
Comment thread tests/test_executor.py Outdated
Implement a configurable token-bucket rate limiter applied to all outbound
Agamemnon HTTP calls, resolving the burst API call issue in _provision_agents.
The limiter is integrated at a single chokepoint (AgamemnonClient._request_with_retry)
for comprehensive coverage across all endpoints.

- Add TokenBucket rate limiter (src/telemachy/rate_limiter.py) with async
  acquire() that waits for tokens at a configurable rate and burst size
- Integrate into AgamemnonClient.__init__ with rate_limit_rps and rate_limit_burst
- Add AGAMEMNON_RATE_LIMIT_RPS and AGAMEMNON_RATE_LIMIT_BURST env vars (default
  off, burst >= 1 validated via Pydantic Field)
- Cap retry backoff at 2.0s per nats-publish-retry-backoff team-knowledge guidance
- Comprehensive test coverage: 7 rate_limiter tests, 4 config tests, 3 client
  tests, 1 executor integration test
- All 64 existing tests still pass; zero regressions

Closes #160
Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>

Signed-off-by: mvillmow <4211002+mvillmow@users.noreply.github.com>
GHSA-4xgf-cpjx-pc3j)

Signed-off-by: Micah Villmow <4211002+mvillmow@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[MINOR] §8: No rate limiting or request throttling for burst API calls

1 participant