diff --git a/.changeset/dashboard-429-polish.md b/.changeset/dashboard-429-polish.md
new file mode 100644
index 0000000000..ccc111a7f2
--- /dev/null
+++ b/.changeset/dashboard-429-polish.md
@@ -0,0 +1,22 @@
+---
+---
+
+Close #2937, #2938, #2939 — the three retrospective follow-ups from PR #2933 (dashboard 429 UX).
+
+**#2937 (P1) — coherent + accessible failure state.**
+- The post-auto-retry "you've refreshed too quickly" copy is now driven by a real countdown off the second response's `Retry-After`, falling back to 60s when absent. Button stays disabled for the full window instead of re-enabling while the copy says to wait.
+- Countdown container gets `role="status"`, `aria-live="polite"`, `aria-atomic="true"`. Text updates are throttled to every 10s + the final 5s + the terminal transition so screen readers aren't shouted at every tick.
+
+**#2938 (P2) — no stale session state.**
+- `autoRetriedAgents` now clears when a retry succeeds. A 9am 429 no longer taints a 5pm retry.
+- New `retryInFlight` Set prevents a click-plus-countdown race from firing `retryAgentCard` twice.
+
+**#2939 (P3) — polish.**
+- `visibilitychange → visible` forces a countdown tick so background-throttled tabs don't show stuck values.
+- Initial page render happens BEFORE the agent fetches. Cards appear in list order immediately in their "not yet checked" skeleton state, then hydrate in place as each worker resolves. Fixes the scrambled loading order and the "dead blank page" feel on large agent lists.
+- A shared `cancellation` flag lets workers stop hammering the network when the 15s timeout fires.
+- "Retrying now…" beat before auto-retry fires so the transition reads.
+- Middleware: extracted `parseRetryAfterSeconds` helper that rejects 0, negatives, and non-finite values (replaces `|| undefined` which swallowed 0). Added comment noting the HTTP-date form is technically legal but express-rate-limit only emits delta-seconds.
+- New `rate-limit-retry-after.test.ts` — 5 tests on the parse helper + a supertest that verifies the 429 body surfaces `retryAfter` when the header is set.
+
+No production behavior changes beyond what's called out above. 1946 server + 631 root unit tests pass.
diff --git a/server/public/dashboard-agents.html b/server/public/dashboard-agents.html
index c03ba4a51e..fc474299cd 100644
--- a/server/public/dashboard-agents.html
+++ b/server/public/dashboard-agents.html
@@ -849,65 +849,68 @@
});
}
- // Fetch compliance data for org's agents with a concurrency cap
- // so a large agent list doesn't self-DOS the per-agent read
- // limiter (#2804). Each agent fans out to 2 server routes
+ // Fetch compliance data for org's agents with a concurrency
+ // cap so a large agent list doesn't self-DOS the per-agent
+ // read limiter (#2804). Each agent fans out to 2 server routes
// (agentReadRateLimiter, 240/min) + 1 auth-status call, so
// holding 6 agents in-flight keeps the burst well under the
// cap while still feeling fast for typical (1-10 agent) users.
- // A hard 15s ceiling still guards against any single fetch
- // hanging the entire page — unresolved agents fall through to
- // the per-card "not yet checked" / error state.
+ //
+ // Render-in-place pattern (#2939): cards appear in the user's
+ // list order *immediately* in their skeleton "not yet checked"
+ // state, then hydrate as each fetchAgentState resolves. That
+ // replaces the old flow where the whole page sat blank until
+ // the entire batch (or the 15s timeout) resolved — for a
+ // 60-agent list over a slow connection that's the difference
+ // between "instant" and "dead for 14 seconds."
+ //
+ // A shared `cancelled` flag lets workers stop hammering the
+ // network if the user navigates away or the 15s ceiling
+ // fires. Workers always harvest results into the shared
+ // complianceMap, even after cancellation — a late success
+ // that arrived before cancel is still useful.
const AGENT_FETCH_CONCURRENCY = 6;
+ const FETCH_TIMEOUT_MS = 15_000;
const agentCompliance = new Map();
const agents = profileData?.profile?.agents || [];
+ const currentOrgId = currentOrg?.id || null;
+
+ pageState.agents = agents;
+ pageState.complianceMap = agentCompliance;
+ pageState.orgId = currentOrgId;
+ pageState.hasApiAccess = !!profileData?.has_api_access;
+ pageState.brandHostingType = null;
+ renderPage(agents, agentCompliance, currentOrgId);
+
if (agents.length > 0) {
const queue = agents.slice();
- const settled = [];
+ const cancellation = { cancelled: false };
async function worker() {
- while (queue.length > 0) {
+ while (!cancellation.cancelled && queue.length > 0) {
const agent = queue.shift();
if (!agent) break;
try {
- settled.push({ status: 'fulfilled', value: await fetchAgentState(agent) });
+ const state = await fetchAgentState(agent);
+ if (cancellation.cancelled) break;
+ agentCompliance.set(state.url, state);
+ swapAgentCard(state.url);
} catch (err) {
- settled.push({ status: 'rejected', reason: err });
+ console.error('Agent compliance fetch rejected:', err);
}
}
}
const workerCount = Math.min(AGENT_FETCH_CONCURRENCY, agents.length);
const workers = Promise.all(Array.from({ length: workerCount }, worker));
- const timeout = new Promise(resolve => setTimeout(() => resolve(null), 15000));
+ const timeout = new Promise(resolve => setTimeout(() => {
+ cancellation.cancelled = true;
+ resolve(null);
+ }, FETCH_TIMEOUT_MS));
const completed = await Promise.race([workers, timeout]);
- if (completed !== null) {
- for (const r of settled) {
- if (r.status === 'fulfilled') {
- agentCompliance.set(r.value.url, r.value);
- } else {
- console.error('Agent compliance fetch rejected:', r.reason);
- }
- }
- } else {
- console.warn('Agent compliance fetch timed out after 15s; rendering with partial data');
- // Even on timeout, harvest whatever workers managed to
- // resolve before the deadline so the user sees partial
- // results instead of nothing.
- for (const r of settled) {
- if (r.status === 'fulfilled') {
- agentCompliance.set(r.value.url, r.value);
- }
- }
+ if (completed === null) {
+ console.warn('Agent compliance fetch timed out after 15s; in-flight workers cancelled, rendering with partial data');
}
}
- const currentOrgId = currentOrg?.id || null;
- pageState.agents = agents;
- pageState.complianceMap = agentCompliance;
- pageState.orgId = currentOrgId;
- pageState.hasApiAccess = !!profileData?.has_api_access;
- pageState.brandHostingType = null;
- renderPage(agents, agentCompliance, currentOrgId);
-
// Brand hosting type gates the "Public" visibility option. Fetched
// asynchronously — the cards re-render once the answer lands.
const primaryBrandDomain = profileData?.profile?.primary_brand_domain;
@@ -1111,41 +1114,75 @@
Agents
if (loadError) {
const isRateLimited = loadError.code === 429;
- // Rate-limit rendering has three modes (#2804):
- // - first 429 with a known retry time → live countdown +
- // auto-retry-once when the countdown hits zero.
- // - first 429 with no known retry time → generic "Retry in
- // a moment" text, manual retry only.
- // - post-auto-retry second 429 → "You've refreshed too
- // quickly" guidance, retry button disabled for the
- // remaining window.
+ // Rate-limit rendering (#2804 / #2937):
+ // - first 429 → live countdown, auto-retry once at zero.
+ // - second 429 after auto-retry → same countdown mechanics
+ // but guidance copy changes and we do NOT auto-retry
+ // again. If the second response didn't carry a
+ // Retry-After, fall back to 60s so the button disables
+ // meaningfully.
+ // - 429 with no retry hint AND no prior auto-retry → plain
+ // "retry in a moment", button stays enabled.
+ const SECOND_429_FALLBACK_SEC = 60;
const autoRetried = pageState.autoRetriedAgents?.has(agent.url);
- const retryAfterSeconds = isRateLimited && typeof loadError.retryAfterSeconds === 'number'
+ const rawRetryAfterSeconds = isRateLimited && typeof loadError.retryAfterSeconds === 'number'
? loadError.retryAfterSeconds
: null;
- const retryAtMs = retryAfterSeconds !== null ? Date.now() + retryAfterSeconds * 1000 : null;
+ const effectiveRetryAfterSeconds = rawRetryAfterSeconds !== null
+ ? rawRetryAfterSeconds
+ : isRateLimited && autoRetried ? SECOND_429_FALLBACK_SEC : null;
+ const retryAtMs = effectiveRetryAfterSeconds !== null
+ ? Date.now() + effectiveRetryAfterSeconds * 1000
+ : null;
- let errMsg;
+ // Rate-limit copy splits into a stable status LABEL (the
+ // short, never-changing "Rate-limited" / "Still rate-limited"
+ // — outside the live region) and a dynamic SUFFIX that owns
+ // everything after it ("— retry in 30s…" / "— retrying now…"
+ // / "— try again" — inside the live region, so screen readers
+ // announce the whole updated clause as a unit). Earlier this
+ // tried to put the static text "retry in " in the prefix,
+ // which then left dangling prepositions when the span
+ // rewrote to "Retrying now…" or "now" — the expert reviewers
+ // flagged both. Owning the whole clause from the preposition
+ // onward keeps the countdown updates grammatical through
+ // every transition.
+ let statusLabel;
+ let countdownSuffix;
+ let plainMsg = '';
if (!isRateLimited) {
- errMsg = 'Couldn\'t load compliance data. The service may be temporarily unavailable.';
- } else if (autoRetried) {
- errMsg = 'You\'ve refreshed too quickly — wait a minute before trying again.';
- } else if (retryAfterSeconds !== null) {
- // Replaced live by the countdown interval below. The
- // initial value is rendered server-side so the user sees
- // a number even before the first tick fires.
- errMsg = `Rate-limited — retry in ${retryAfterSeconds}s…`;
+ plainMsg = 'Couldn\'t load compliance data. The service may be temporarily unavailable.';
+ statusLabel = '';
+ countdownSuffix = '';
+ } else if (effectiveRetryAfterSeconds !== null) {
+ statusLabel = autoRetried ? 'Still rate-limited' : 'Rate-limited';
+ countdownSuffix = ` — retry in ${effectiveRetryAfterSeconds}s…`;
} else {
- errMsg = 'Rate-limited — retry in a moment.';
+ plainMsg = 'Rate-limited — retry in a moment.';
+ statusLabel = '';
+ countdownSuffix = '';
}
- // Disable the retry button until the countdown expires or
- // (if we already auto-retried) for the rest of the known
- // rate-limit window.
- const retryDisabled = isRateLimited && (autoRetried || retryAtMs !== null);
- const retryLabel = retryDisabled && retryAtMs !== null
- ? `Retry in ${retryAfterSeconds}s`
+ // Disable the retry button whenever we have a known window;
+ // the single page-level countdown interval re-enables it
+ // (and re-renders) when the countdown expires.
+ const retryDisabled = isRateLimited && retryAtMs !== null;
+ const retryLabel = retryDisabled
+ ? `Retry in ${effectiveRetryAfterSeconds}s`
: 'Retry';
+ // The countdown only auto-retries on the FIRST 429 — second
+ // 429 is purely a waiting-room; the button re-enables at
+ // zero for a manual retry.
+ const shouldAutoRetryAtZero = retryAtMs !== null && !autoRetried;
+
+ // Structure: static status LABEL outside the live region,
+ // dynamic clause (from the em-dash onward) inside it. The
+ // live span owns the transition between "— retry in 30s…",
+ // "— retrying now…", and "— try again" so every variant
+ // reads cleanly next to the static label.
+ const bodyHtml = retryAtMs !== null
+ ? `
return days + 'd ago';
}
+ // Re-entry guard for retryAgentCard: set while a retry is in
+ // flight, cleared when the re-render completes. Prevents a stale
+ // click from racing the countdown auto-trigger (#2938).
+ const retryInFlight = new Set();
+
+ // Swap a single agent's card in the DOM with freshly-rendered HTML
+ // from renderAgentsSection. Used by both retry paths and the
+ // skeleton-hydrate flow at initial load (#2939). Accepts
+ // `{ clearAutoRetried }`: callers performing a successful retry
+ // pass true so a long-lived session doesn't carry "auto-retried"
+ // state from a window that has long since expired (#2938).
+ function swapAgentCard(agentUrl, { clearAutoRetried = false } = {}) {
+ if (clearAutoRetried) pageState.autoRetriedAgents.delete(agentUrl);
+ const cardId = 'agent-' + encodeURIComponent(agentUrl).replace(/%/g, '_');
+ const card = document.getElementById(cardId);
+ if (!card) {
+ // This branch shouldn't fire in today's flow — initial render
+ // inserts cards synchronously before the worker pool awaits.
+ // Log loudly if it does, because silent returns hide the bug
+ // class if anyone later moves the render behind a microtask.
+ console.warn('swapAgentCard: card not found for', agentUrl);
+ return;
+ }
+ const agent = pageState.agents.find(a => a.url === agentUrl) || { url: agentUrl };
+ const newHtml = renderAgentsSection([agent], pageState.complianceMap, pageState.orgId);
+ const wrapper = document.createElement('div');
+ wrapper.innerHTML = newHtml.trim();
+ const newCard = wrapper.firstElementChild;
+ if (newCard) card.replaceWith(newCard);
+ }
+
// Retry a single agent's compliance fetch after a load error. Shared
// between the explicit Retry button click and the post-countdown
// auto-retry triggered by the rate-limit UI. `auto` is true when
- // called from the countdown, which marks the agent as having
- // auto-retried so a subsequent 429 freezes the card with
- // "you've refreshed too quickly" guidance instead of looping.
+ // called from the countdown (adds to autoRetriedAgents so a second
+ // 429 renders guidance copy). On successful retry, we clear the
+ // auto-retried flag so stale session state doesn't linger (#2938).
async function retryAgentCard(agentUrl, cardId, { auto = false } = {}) {
+ if (retryInFlight.has(agentUrl)) return;
+ retryInFlight.add(agentUrl);
const card = document.getElementById(cardId);
- if (!card) return;
+ if (!card) {
+ retryInFlight.delete(agentUrl);
+ return;
+ }
const retryBtn = card.querySelector('.agent-reload-btn');
if (retryBtn) {
retryBtn.disabled = true;
- retryBtn.textContent = 'Retrying...';
+ retryBtn.textContent = 'Retrying…';
}
if (auto) pageState.autoRetriedAgents.add(agentUrl);
try {
const state = await fetchAgentState({ url: agentUrl });
pageState.complianceMap.set(agentUrl, state);
- const agent = pageState.agents.find(a => a.url === agentUrl) || { url: agentUrl };
- const newHtml = renderAgentsSection([agent], pageState.complianceMap, pageState.orgId);
- const wrapper = document.createElement('div');
- wrapper.innerHTML = newHtml.trim();
- const newCard = wrapper.firstElementChild;
- if (newCard) card.replaceWith(newCard);
+ // Clear any prior auto-retry marker UNLESS we hit another 429
+ // — keeping the marker on a back-to-back 429 is what drives
+ // the second-429 "Still rate-limited" UI. Anything else (a
+ // success or a non-429 failure) means the limiter window has
+ // elapsed or was never the problem; wipe the marker so a
+ // later 429 this session starts fresh (#2938).
+ const shouldClearAutoRetried = !state.loadError || state.loadError.code !== 429;
+ swapAgentCard(agentUrl, { clearAutoRetried: shouldClearAutoRetried });
} catch (err) {
console.error('Retry failed:', err);
if (retryBtn) {
retryBtn.disabled = false;
retryBtn.textContent = 'Retry';
}
+ } finally {
+ retryInFlight.delete(agentUrl);
}
}
@@ -1324,9 +1399,25 @@
Agents
// Live countdown + auto-retry for rate-limited cards. One page-level
// interval scans for `[data-rate-limited-until]` elements every
- // second, updates their text to "Retry in Ns…", and fires a
- // single auto-retry when the countdown hits zero (#2804).
- setInterval(function() {
+ // second, updates their text, and fires an auto-retry when the
+ // countdown hits zero IF the node's `data-rate-limited-auto` flag
+ // is set (first 429 only — second 429 is a waiting-room; the
+ // button just re-enables for a manual retry). #2804 / #2937.
+ //
+ // Announcements are throttled: screen readers shouldn't hear every
+ // tick of the countdown. `aria-live="polite"` on the node lets
+ // the browser debounce, but the text itself changes every second.
+ // We also track `lastAnnouncedSec` per-node so we only change
+ // textContent at meaningful intervals (every 10s + the last 5s
+ // + the terminal state).
+ function tickRateLimitCountdowns() {
+ // The live region is the owning the dynamic clause from
+ // the em-dash onward (" — retry in 30s…" / " — retrying now…"
+ // / " — try again"). The static status label ("Rate-limited" /
+ // "Still rate-limited") sits outside the live region so screen
+ // readers don't re-announce it on every tick, but the span
+ // always carries a grammatical self-contained clause — no
+ // dangling prepositions when it rewrites to "retrying now…".
const nodes = document.querySelectorAll('[data-rate-limited-until]');
if (nodes.length === 0) return;
const now = Date.now();
@@ -1334,28 +1425,66 @@
Agents
const until = parseInt(node.dataset.rateLimitedUntil || '', 10);
if (!Number.isFinite(until)) continue;
const remainingSec = Math.max(0, Math.ceil((until - now) / 1000));
+ const card = node.closest('.agent-compliance-card');
+ const retryBtn = card?.querySelector('.agent-reload-btn');
+
if (remainingSec > 0) {
- node.textContent = `Rate-limited — retry in ${remainingSec}s…`;
- // Update the button label too so disabled button reflects
- // the live countdown instead of a stale "Retry in 30s".
- const card = node.closest('.agent-compliance-card');
- const retryBtn = card?.querySelector('.agent-reload-btn');
+ // Throttle textContent updates for a11y — screen readers
+ // announce changes on polite regions; every-second updates
+ // are noisy. Announce at 10s boundaries, the final 5s, or
+ // when the node was just rendered.
+ const lastAnnounced = parseInt(node.dataset.lastAnnouncedSec || '', 10);
+ const shouldAnnounce =
+ !Number.isFinite(lastAnnounced) ||
+ remainingSec <= 5 ||
+ remainingSec % 10 === 0 ||
+ Math.abs(lastAnnounced - remainingSec) >= 10;
+ if (shouldAnnounce) {
+ node.textContent = ` — retry in ${remainingSec}s…`;
+ node.dataset.lastAnnouncedSec = String(remainingSec);
+ }
+ // Retry button label updates every tick regardless of the
+ // a11y throttle — the button isn't in a live region, so
+ // there's no announcement cost. (Intentional asymmetry.)
if (retryBtn && retryBtn.disabled) {
retryBtn.textContent = `Retry in ${remainingSec}s`;
}
continue;
}
- // Countdown expired. Clear the attribute so we don't fire
- // multiple retries if the re-render is slow, then trigger.
+
+ // Countdown expired. Clear attributes so a stuck render
+ // doesn't trigger multiple retries.
const cardId = node.dataset.cardId;
- const card = document.getElementById(cardId);
- const agentUrl = card?.querySelector('.agent-reload-btn')?.dataset.agentUrl;
+ const agentUrl = retryBtn?.dataset.agentUrl;
+ const shouldAutoRetry = node.dataset.rateLimitedAuto === '1';
node.removeAttribute('data-rate-limited-until');
- if (cardId && agentUrl) {
+ node.removeAttribute('data-rate-limited-auto');
+
+ if (shouldAutoRetry && cardId && agentUrl) {
+ // A small "retrying now…" beat before the card swap so the
+ // transition reads as an action, not a sudden DOM change.
+ // retryAgentCard's swap will replace the whole card shortly.
+ node.textContent = ' — retrying now…';
retryAgentCard(agentUrl, cardId, { auto: true });
+ } else if (retryBtn) {
+ // Second-429 waiting-room: re-enable the button for manual
+ // retry. The span rewrites to a grammatical clause ("— try
+ // again") so the card reads "Still rate-limited — try again"
+ // until the user clicks.
+ node.textContent = ' — try again';
+ retryBtn.disabled = false;
+ retryBtn.textContent = 'Retry';
}
}
- }, 1000);
+ }
+ setInterval(tickRateLimitCountdowns, 1000);
+
+ // Hidden tabs throttle setInterval, so a user flipping back sees
+ // a stuck countdown that then jumps. Force a tick on visible so
+ // the display is current (#2939).
+ document.addEventListener('visibilitychange', function() {
+ if (document.visibilityState === 'visible') tickRateLimitCountdowns();
+ });
// Connect agent toggle
document.addEventListener('click', function(e) {
diff --git a/server/src/middleware/rate-limit.ts b/server/src/middleware/rate-limit.ts
index 3cd0edf032..40e19ca65a 100644
--- a/server/src/middleware/rate-limit.ts
+++ b/server/src/middleware/rate-limit.ts
@@ -5,6 +5,28 @@ import { CachedPostgresStore } from './pg-rate-limit-store.js';
const logger = createLogger('rate-limit');
+/**
+ * Parse a `Retry-After` header value into delta-seconds. Handles both
+ * the number form (express-rate-limit with `standardHeaders: true`
+ * emits seconds as a number) and the string form. Returns `undefined`
+ * for anything that doesn't look like a positive integer — including
+ * zero, which we treat as "no meaningful wait" rather than exposing
+ * a degenerate countdown value.
+ *
+ * Callers surface the value in the 429 body as a proxy-stripped
+ * fallback for the header (#2804).
+ */
+export function parseRetryAfterSeconds(raw: number | string | string[] | undefined): number | undefined {
+ if (typeof raw === 'number') {
+ return Number.isFinite(raw) && raw > 0 ? raw : undefined;
+ }
+ if (typeof raw === 'string') {
+ const parsed = parseInt(raw, 10);
+ return Number.isFinite(parsed) && parsed > 0 ? parsed : undefined;
+ }
+ return undefined;
+}
+
/**
* Generate a rate limit key from request, preferring user ID over IP.
* Uses proper IPv6 subnet masking when falling back to IP addresses.
@@ -233,12 +255,12 @@ export const agentReadRateLimiter = rateLimit({
// headers, and the dashboard needs SOMETHING to key its countdown
// off. `retryAfter` is seconds-to-retry, matching the header's
// delta-seconds format.
- const retryAfterHeader = res.getHeader('Retry-After');
- const retryAfter = typeof retryAfterHeader === 'number'
- ? retryAfterHeader
- : typeof retryAfterHeader === 'string'
- ? parseInt(retryAfterHeader, 10) || undefined
- : undefined;
+ //
+ // The HTTP spec (RFC 9110 §10.2.3) also allows an HTTP-date here,
+ // but express-rate-limit only emits delta-seconds — so a parseInt
+ // is sufficient. If that ever changes (e.g., we swap limiter
+ // libraries), the fallback below would need a second parse path.
+ const retryAfter = parseRetryAfterSeconds(res.getHeader('Retry-After'));
res.status(429).json({
error: 'Too many requests',
message: 'Agent dashboard read rate limit exceeded. Please try again in a moment.',
diff --git a/server/tests/unit/rate-limit-retry-after.test.ts b/server/tests/unit/rate-limit-retry-after.test.ts
new file mode 100644
index 0000000000..f8ecaeaebb
--- /dev/null
+++ b/server/tests/unit/rate-limit-retry-after.test.ts
@@ -0,0 +1,90 @@
+import { describe, it, expect, beforeEach } from 'vitest';
+import express from 'express';
+import request from 'supertest';
+import { parseRetryAfterSeconds, agentReadRateLimiter } from '../../src/middleware/rate-limit.js';
+
+/**
+ * Tests for the retryAfter fallback field we surface on the 429 body
+ * from `agentReadRateLimiter` (#2804/#2939). Reverse proxies sometimes
+ * strip non-standard response headers; the dashboard client falls back
+ * to the JSON body when the `Retry-After` header is missing.
+ */
+
+describe('parseRetryAfterSeconds', () => {
+ it('accepts positive integers', () => {
+ expect(parseRetryAfterSeconds(30)).toBe(30);
+ expect(parseRetryAfterSeconds('15')).toBe(15);
+ });
+
+ it('rejects zero — the client treats a zero countdown as "no hint" rather than a degenerate tick', () => {
+ expect(parseRetryAfterSeconds(0)).toBeUndefined();
+ expect(parseRetryAfterSeconds('0')).toBeUndefined();
+ });
+
+ it('rejects negatives and non-finite numbers', () => {
+ expect(parseRetryAfterSeconds(-5)).toBeUndefined();
+ expect(parseRetryAfterSeconds(Number.NaN)).toBeUndefined();
+ expect(parseRetryAfterSeconds(Infinity)).toBeUndefined();
+ });
+
+ it('rejects non-numeric strings', () => {
+ expect(parseRetryAfterSeconds('soon')).toBeUndefined();
+ expect(parseRetryAfterSeconds('')).toBeUndefined();
+ });
+
+ it('returns undefined for unexpected shapes (array, undefined)', () => {
+ expect(parseRetryAfterSeconds(undefined)).toBeUndefined();
+ expect(parseRetryAfterSeconds(['30', '60'])).toBeUndefined();
+ });
+});
+
+describe('agentReadRateLimiter 429 body', () => {
+ // Exercise the actual middleware through a tiny express app so the
+ // assertion lives at the same layer production depends on.
+ // `agentReadRateLimiter` is a module-singleton with a cached
+ // Postgres store — the increment hot path is in-memory, so no real
+ // DB is needed, but the counter persists across tests. `resetKey`
+ // scrubs the per-key counter in `beforeEach` so each test starts
+ // from zero regardless of what ran earlier in the suite.
+ function buildApp() {
+ const app = express();
+ app.get('/ping', agentReadRateLimiter, (_req, res) => res.status(200).json({ ok: true }));
+ return app;
+ }
+
+ // Supertest hits loopback; the limiter's keyGenerator falls back to
+ // `::ffff:127.0.0.1` (IPv4-mapped IPv6) when no req.user is present.
+ const LOOPBACK_KEY = '::ffff:127.0.0.1';
+ beforeEach(async () => {
+ await agentReadRateLimiter.resetKey(LOOPBACK_KEY);
+ // Also clear the raw IPv4 key as a belt-and-braces in case
+ // Node/Express ever flips the default representation.
+ await agentReadRateLimiter.resetKey('127.0.0.1');
+ });
+
+ it('includes `retryAfter` (seconds) in the body matching the Retry-After header', async () => {
+ const app = buildApp();
+ // Race past the 240/min cap. Same IP so the limiter keys identically.
+ // Serial rather than parallel — express-rate-limit's in-flight
+ // tracking is more deterministic, and the assertion is about the
+ // 429 body shape, not concurrent behavior.
+ let last: Awaited>;
+ for (let i = 0; i < 241; i++) {
+ last = await request(app).get('/ping');
+ }
+ expect(last!.status).toBe(429);
+ expect(last!.body.error).toBe('Too many requests');
+ expect(typeof last!.body.retryAfter).toBe('number');
+ expect(last!.body.retryAfter).toBeGreaterThan(0);
+
+ // Unconditional cross-check: the body's `retryAfter` must equal
+ // the `Retry-After` header's delta-seconds. If the header is
+ // malformed or missing, this assertion fails loudly rather than
+ // silently skipping — either outcome would be a bug the test
+ // needs to catch.
+ const headerSeconds = parseInt(last!.headers['retry-after'] ?? '', 10);
+ expect(Number.isFinite(headerSeconds)).toBe(true);
+ expect(headerSeconds).toBeGreaterThan(0);
+ expect(last!.body.retryAfter).toBe(headerSeconds);
+ }, 30_000);
+});