Bound the workflow-artifact rate limiter's per-run map

packages/artifacts-hub/package.json

@@ -15,6 +15,7 @@
   "dependencies": {
     "@corbits/artifact-ui": "workspace:*",
     "@corbits/artifacts": "github:corbitsdev/corbits-artifacts#81049ed24a64e927498c7238bda6ffa66b63d2ab",
+    "@corbits/collections": "workspace:*",
     "@corbits/folded-runs": "workspace:*",
     "@intx/crypto": "0.3.0",
     "@intx/db": "workspace:*",

packages/artifacts-hub/src/workflow-routes.rate-limiter.test.ts

@@ -0,0 +1,67 @@
+import { describe, expect, test } from "bun:test";
+
+import { createRunCreateRateLimiter, RATE_WINDOW_MS } from "./workflow-routes";
+
+describe("createRunCreateRateLimiter", () => {
+  test("evicts an idle run's entry instead of holding it for the process lifetime", () => {
+    let clock = 0;
+    const limiter = createRunCreateRateLimiter(3, () => clock);
+
+    for (let i = 0; i < 500; i++) {
+      limiter.allow(`run-${i}`);
+    }
+    expect(limiter.trackedRunCount).toBe(500);
+
+    // Every one of those runs has gone idle for a full window: their
+    // entries should be reclaimed, not carried forever.
+    clock += RATE_WINDOW_MS;
+    limiter.allow("run-fresh");
+    expect(limiter.trackedRunCount).toBe(1);
+  });
+
+  test("a caller cannot exceed the rate by exploiting eviction timing", () => {
+    let clock = 0;
+    const maxPerWindow = 3;
+    const limiter = createRunCreateRateLimiter(maxPerWindow, () => clock);
+    const runId = "run-under-test";
+
+    for (let i = 0; i < maxPerWindow; i++) {
+      expect(limiter.allow(runId)).toBe(true);
+    }
+    expect(limiter.allow(runId)).toBe(false);
+
+    // Advance right up to (but not past) the window boundary: the
+    // entry's TTL must not have lapsed yet, so the earlier timestamps
+    // are still counted and the limit still holds.
+    clock += RATE_WINDOW_MS - 1;
+    expect(limiter.allow(runId)).toBe(false);
+
+    // Advance past the window: the original timestamps are now stale
+    // and a fresh budget opens up, which is the intended sliding-window
+    // behavior rather than a leak of the earlier eviction.
+    clock += 1;
+    for (let i = 0; i < maxPerWindow; i++) {
+      expect(limiter.allow(runId)).toBe(true);
+    }
+    expect(limiter.allow(runId)).toBe(false);
+  });
+
+  test("does not let concurrently active runs go unbounded by other idle ones", () => {
+    let clock = 0;
+    const limiter = createRunCreateRateLimiter(3, () => clock);
+
+    // A burst of one-shot runs, each idle immediately after.
+    for (let i = 0; i < 200; i++) {
+      limiter.allow(`idle-run-${i}`);
+      clock += 1;
+    }
+
+    // One run stays continuously active, well past when the idle runs'
+    // entries should have expired.
+    clock += RATE_WINDOW_MS;
+    const activeRunId = "active-run";
+    limiter.allow(activeRunId);
+
+    expect(limiter.trackedRunCount).toBe(1);
+  });
+});

packages/artifacts-hub/src/workflow-routes.ts

@@ -22,6 +22,7 @@
  * itself never holds a database handle.
  */
 import { type } from "arktype";
+import { createExpiringMap } from "@corbits/collections";
 import {
   ARTIFACT_UPLOAD_POLICY,
   anonymousIdentity,
@@ -58,8 +59,8 @@ const MAX_ARTIFACT_CONTENT_CHARS = 64_000;
 // A finalized turn can legitimately persist a handful of artifacts in
 // one burst; 30/minute per run comfortably covers that while still
 // catching a runaway loop before it floods Library storage.
-const MAX_CREATES_PER_RUN_PER_MINUTE = 30;
-const RATE_WINDOW_MS = 60_000;
+export const MAX_CREATES_PER_RUN_PER_MINUTE = 30;
+export const RATE_WINDOW_MS = 60_000;
 
 // Same per-file ceiling the tenant Library's own `POST /upload` enforces
 // (`MAX_UPLOAD_BYTES`) — one number for "how big a file artifact may be"
@@ -80,24 +81,45 @@ export const MAX_WORKFLOW_BINARY_BYTES = MAX_UPLOAD_BYTES;
  * only what it personally handled — a known fail-open gap, not a
  * fail-closed one, so it under-limits rather than wrongly rejecting a
  * caller a sibling replica hasn't seen yet.
+ *
+ * The per-run entry lives in a `createExpiringMap` (CL-7243) rather than
+ * a plain `Map`, so a run's entry is reclaimed once it goes idle instead
+ * of staying resident for the rest of the hub process's uptime. The TTL
+ * is exactly `RATE_WINDOW_MS`: every `allow()` call re-`set`s the entry,
+ * refreshing its expiry, so an entry can only lapse after a full window
+ * with no calls for that run — by which point every timestamp it held
+ * has already aged out of the sliding window's own `cutoff` filter below.
+ * A shorter TTL could evict an entry (and thus its still-in-window
+ * timestamps) before the window's filter would have dropped them,
+ * silently resetting a caller's quota early; this TTL can't.
  */
-function createRunCreateRateLimiter(maxPerWindow: number) {
-  const timestampsByRunId = new Map<string, number[]>();
+export function createRunCreateRateLimiter(
+  maxPerWindow: number,
+  now: () => number = Date.now,
+) {
+  const timestampsByRunId = createExpiringMap<string, number[]>({
+    ttlMs: RATE_WINDOW_MS,
+    now,
+  });
   return {
     allow(runId: string): boolean {
-      const now = Date.now();
-      const cutoff = now - RATE_WINDOW_MS;
+      const at = now();
+      const cutoff = at - RATE_WINDOW_MS;
       const recent = (timestampsByRunId.get(runId) ?? []).filter(
         (timestamp) => timestamp > cutoff,
       );
       if (recent.length >= maxPerWindow) {
         timestampsByRunId.set(runId, recent);
         return false;
       }
-      recent.push(now);
+      recent.push(at);
       timestampsByRunId.set(runId, recent);
       return true;
     },
+    /** Live entry count, for tests asserting the map stays bounded. */
+    get trackedRunCount(): number {
+      return timestampsByRunId.size;
+    },
   };
 }