fix: throttle Slack API calls through a shared rate limiter (#147) (#151)
## Linked issue
Closes #147
## Summary
Slack's Enterprise Grid anomaly detection raises
[`unexpected_api_call_volume`](https://docs.slack.dev/reference/audit-logs-api/anomalous-events-reference/)
when a client produces more API traffic than a browser would, and it can
sign the session out — which is what happened to the reporter on
browser-session auth. slackcli had no throttling anywhere, and several
paths fan out one call per entity with no delay or concurrency cap:
`SlackClient.getUsersInfo()` (one `users.info` per user in a tight
loop), `enrichSavedItems()` in `saved.ts`, and the unread resolver in
`unread.ts`.
**What changed**
- **New `src/lib/rate-limiter.ts`** — a zero-dependency `RateLimiter`
combining a concurrency cap with a minimum delay between call *starts*,
plus `slackRateLimiter`, the process-wide instance every `SlackClient`
shares by default. Defaults live in named constants:
`SLACK_MAX_CONCURRENT_REQUESTS = 2`, `SLACK_MIN_REQUEST_INTERVAL_MS =
200`. Hand-rolled rather than `p-limit`/`bottleneck` per constitution §8
— the CLI ships as a `bun build --compile` binary under a 150MB budget
and this is ~60 lines with no other consumer.
- **`src/lib/slack-client.ts`** — `request()` now runs its auth-type
dispatch inside `this.rateLimiter.run(...)`. Because `request()` is a
complete funnel for every Slack API call in the tree, this covers
**both** auth paths (`standardRequest` via `@slack/web-api` and
`browserRequest` via raw `fetch`) and `saved.ts` / `unread.ts` need no
changes. The constructor takes an optional `{ rateLimiter }` so tests
can inject a fast one.
Out of scope, as the issue specifies: retry/backoff on HTTP 429, and the
raw `fetch()` calls used for file upload/download (single-shot per
invocation, not a burst source).
**Tests**
- `src/lib/rate-limiter.test.ts` (11 cases): concurrency cap never
exceeded, full serialization at `maxConcurrent: 1`, minimum spacing
between starts, no extra wait after an idle period, no delay at
`minIntervalMs: 0`, rejection propagates *and* releases the slot, queued
tasks keep running after one rejects, FIFO start order, and constructor
validation of invalid options (`0`, negative, `NaN`, `Infinity`).
- `src/lib/slack-client.test.ts` (4 cases) proving `request()` is
actually gated — pacing *and* a binding concurrency cap on the browser
path (stubbed `fetch`, held open longer than the interval so the
requests really overlap), pacing on the standard path (stubbed
`WebClient.apiCall`), one limiter shared across two clients, and slot
release after a failed request. Reverting the `request()` change fails
three of them; making the limiter per-instance fails the fourth.
A `[Unreleased]` changelog entry covers the user-visible slowdown.
**Docs**
`docs/development/architecture.md` gains a "Throttling" section and an
updated `request()` snippet; `docs/user-guide/troubleshooting.md` and
`docs/user-guide/scripting.md` explain the pacing and that the resulting
slowness on `saved list` / `conversations unread` is throttling, not a
hang (per @shaharia's note on the issue); module tables in `CLAUDE.md`
and `docs/development/project-structure.md` list the new module.
## Checklist
- [x] The linked issue is open and carries the `ready-for-pr` label
(constitution §1–2)
- [x] Change is focused on the linked issue — no unrelated edits
- [x] Tests added/updated, including edge cases (constitution §4)
- [x] `bun run type-check` and `bun test` pass locally
- [x] Pre-commit hooks are installed and passing — no `--no-verify`, no
`SKIP=`
- [x] All commits are signed (constitution §5)
- [x] Documentation in `docs/` updated, added, or deleted as needed
(constitution §7)
- [x] No tokens, credentials, or user data handled insecurely V
VibeXP Agent committed
dbefb02c6ba1d81a9400c3f4bcf01fcdb81533f5
Parent: 7458e50
Committed by GitHub <noreply@github.com>
on 9/2/2026, 7:13:33 AM