Repository navigation
id(web): SaveRateLimiter's per-file map grows without bound — entries expire after the 5s cooldown but are never evicted #1713
Description
Problem
SaveRateLimiter (pkgs/id/src/web/mod.rs:94-132) tracks the last save of each file in last_save: Arc<Mutex<HashMap<String, Instant>>>. Nothing ever removes an entry: check() only reads the map, record() only inserts. The cooldown is 5 seconds (DEFAULT_SAVE_COOLDOWN, web/mod.rs:86), so an entry older than 5s can never change a decision again — check treats it as allowed and record would overwrite it anyway — yet it is kept for the life of the id serve process.
The map therefore grows by one permanent entry for every distinct filename ever saved. Filenames are client-supplied (SaveRequest.name, checked at web/routes.rs:1203, recorded at web/routes.rs:1327 after each successful save), so growth is driven by normal use (every new document name) and can be minted deliberately by saving many distinct names once each.
Evidence (verified on main f7b70714, this run)
pkgs/id/src/web/mod.rs:94-99— the limiter is a plainHashMap<String, Instant>behind a mutex; no eviction field or policy.pkgs/id/src/web/mod.rs:114-125—check()only doesmap.get(name); an expired entry is left in place.pkgs/id/src/web/mod.rs:128-131—record()only doesmap.insert(...).- Full-tree grep for
save_limiter/SaveRateLimiter/last_saveacrosspkgs/id/srcandpkgs/id/tests: the only uses are the definition,AppStateconstruction (web/mod.rs:208, 5s cooldown),check(web/routes.rs:1203) andrecord(web/routes.rs:1327). Noremove,retain,clear, or periodic sweep exists anywhere. - Dedup:
gh issue list --search "rate limiter OR SaveRateLimiter OR save cooldown"(open + closed) returns no filing for this; Firefox e2e sandbox hardening: autosave save-count race + websocket-spec server stall (#229 follow-up) #1230 is a Firefox e2e autosave race, unrelated.
Why it matters
id serve is a long-running home server. A rate limiter whose state grows monotonically with distinct filenames is a slow, unbounded memory leak in exactly the process that is never restarted, and the retained entries serve no purpose after 5 seconds. It is also a small integrity wart: the one structure whose job is bounding save behaviour is itself unbounded.
Fix direction (for triage to pin)
Evict expired entries so the map holds only names inside their cooldown window — e.g. check() removes the entry it finds expired (it no longer limits anything), and/or record() sweeps expired entries while it holds the lock. Semantics must stay identical: a save within 5s of the same name's last save is still refused with the remaining time; a save after the cooldown is allowed. No new dependencies, no API change.
Acceptance criteria
- After a name's cooldown expires, its entry no longer occupies the map (evicted on a subsequent
check/record, or by an equivalent bounded sweep). - Rate-limit behaviour is unchanged: same-name save inside the cooldown is refused with the remaining duration; after the cooldown it is allowed and re-recorded.
- A regression test shows the map does not retain expired entries (e.g. record several names under a short cooldown, let it lapse, trigger the eviction path, assert the map shrank) — the limiter's existing behaviour tests still pass.
-
cargo testfor the web module passes on laptop/CI.
Activity
developing-today commentedon Oct 28, 2026
CODE-689
dezren39 commentedon Oct 28, 2026
🤖 Agent note · filed
f7b70714thatSaveRateLimiter(pkgs/id/src/web/mod.rs:94-132) never evicts:check()only reads itsHashMap<String, Instant>,record()only inserts, and a full-tree grep finds no remove/retain/sweep anywhere. With a 5s cooldown every entry is decision-dead after 5s yet held for the life of the server, one per distinct client-supplied filename (routes.rs:1203/1327).queue-packets/packet-1713.json(repo-health-code-pilot, triage)dezren39 commentedon Oct 28, 2026
🤖 Agent note · picked up
SaveRateLimiter's map inpkgs/id/src/web/mod.rs(check/record), keeping the 5s refusal semantics identical, plus a regression test. Branchcreator-a/1713-save-limiter-evict, worktree on the laptop, tests on the laptop, PR + merge when CI is green.dezren39 commentedon Oct 28, 2026
🤖 Agent note · handed off
creator-a/1713-save-limiter-evict(laptop worktree, rebased onto current main 8c6bdc0, head 163a40b): check() drops the expired entry it finds, record() sweeps lapsed entries under the lock; three new tokio tests cover refusal semantics and eviction. Localcargo testfrom the repo tree hits a pre-existing workspace-manifest conflict on current main, so verification rides on this PR's CI (the nix flake check matrix) per the CI-first rule.