[Makisuo]

Stacked on #2446. Part of the audit stack that starts at #2434.

Bugs fixed

• The memory cache backend never evicted. An entry was deleted only when the same key was read again after it expired, so keys with a rotating part (an org and an hour, say) stayed forever on any long-lived host using this backend. put now sweeps expired entries once the map has doubled. A non-finite TTL, which never expired, is no longer stored.
• Non-strict redirects replayed the request body where fetch would not. lib/safe-fetch re-sent the same method and body on a 301, 302 or 303. It now follows the fetch standard: 303 turns anything but GET or HEAD into a body-less GET, 301 and 302 do so for POST, and 307 and 308 keep method and body. Behaviour change: an alert webhook that answers a POST with 301, 302 or 303 is now followed with a GET.
• The test-slop lint plugin skipped chained test modifiers. it.effect.each(rows)(...) or it.live.only(...) was not recognised as a test, so the weak-assertion rule never ran inside it. No test file uses a chained modifier today, so nothing new is reported.

Cleanup

• escapeHtml and truncate had copies in the email, status page and chat platform packages. They live in lib/strings now.
• Three symbols with no user are deleted and three un-exported.
• Three stale comments are corrected.

Checked, no change

• Status page escaping. A reviewer flagged dayLabel going into attributes unescaped. It is already escaped at its only HTML call site and is built from dates, counts and fixed words; titles reach the page only as script-safe JSON written with textContent.

Verification

• Tests pass in each touched package: cache (25), safe-fetch (128), strings (10), grid-engine (359), status-page (52), email (27), chat-platform render (36), agent-sessions (64), and the smaller ones.
• The cache and redirect tests fail on the old code. The old lint plugin reports nothing on the two chained fixtures; the fixed one reports both.
• Typecheck is clean in all 11 touched packages. Lint and format are clean.

Left for a decision

• clampSize in lib/grid-engine can push a tile past the right edge when a stored tile is narrower than its minW. The obvious fix breaks nine golden fixtures and a property test that deliberately lets minW win, and docs/dashboard-grid.md names that parity as a rule. Not changed.
• A caller-configured secret header still follows a redirect to another origin. Only authorization, cookie and proxy-authorization are stripped. Changing that could break scrape targets that rely on it.
• The five effect-strict lint rules are implemented twice, and the committed anti-slop.mjs bundle has no drift check.

---

^{Open in [code]smith · Need help on this PR? Tag @codesmith-bot with what you need.}

[coderabbitai]

Warning

Review limit reached

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Next included review available in 56 seconds.

Check out review usage here.

View limit details

Limit details: You’ve used all 4 included reviews currently available.

Learn how review limits work.

Review configuration:

Run configuration

• Source mode: Auto
• Repository: Unavailable
• Branch: Unavailable
• File: No configuration file from this repository applied
• Commit: Unavailable
• Layers: defaults
• Review profile: CHILL
• Plan: Advanced
• Run ID: 6513e679-d583-4439-8526-0b0fe6d9a576

Commits

Reviewing files that changed from the base of the PR and between 30b73be and b394a3d.

Files selected for processing: 26

• lib/cache/src/cache-backend.test.ts
• lib/cache/src/cache-backend.ts
• lib/cache/src/edge-cache.ts
• lib/oxlint-config/plugins/test-slop.mjs
• lib/oxlint-config/scripts/smoke.mjs
• lib/prometheus-otlp/src/otlp.ts
• lib/safe-fetch/src/index.test.ts
• lib/safe-fetch/src/index.ts
• lib/strings/src/index.test.ts
• lib/strings/src/index.ts
• packages/agent-sessions/src/session-summary.ts
• packages/alerting-core/src/escalation.test.ts
• packages/alerting-core/src/escalation.ts
• packages/chat-platform/src/render/approval-outcome.ts
• packages/chat-platform/src/render/message.ts
• packages/email/src/alert-notification.ts
• packages/email/src/template.ts
• packages/email/src/web-analytics-digest.ts
• packages/email/src/weekly-digest.ts
• packages/eventing-core/src/limits.ts
• packages/security-audit/src/skill.ts
• packages/status-page/src/document.ts
• packages/status-page/src/escape.ts
• packages/status-page/src/index.ts
• packages/status-page/src/manage.ts
• packages/status-page/src/page.ts

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

Share

• X
• Mastodon
• Reddit
• LinkedIn

[Makisuo]

@maple-review-bot won't fix F1 in this PR. The one-character difference is known and was left on purpose: the email truncate keeps max characters plus the ellipsis, and switching the three email renderers to the shared helper would change text in emails already going out to customers (digest and alert notification lines would be one character shorter). That is a visible change this cleanup PR should not carry. Unifying them, or renaming the email one so the difference is explicit, is a follow-up.

[maple-review-bot]

Fair — checking it at the head commit, the two are not silently interchangeable and nothing in this PR depends on them matching:

• shared helper, lib/strings/src/index.ts:82: Str.slice(0, max - 1)(text) + … → total max.
• email helper, packages/email/src/template.ts:53: Str.slice(0, max)(text) + … → total max + 1.

And the three email renderers still import truncate from ./template (alert-notification.ts:10, web-analytics-digest.ts:13, weekly-digest.ts:11) — only escapeHtml moved to @maple/strings, so no email body changes in this PR. The two call sites that did migrate (chat-platform/src/render/approval-outcome.ts, message.ts) were character-for-character the same expression as the shared one, so nothing drifted onto the shorter variant either.

F1 can be dismissed. A one-line note on the shared truncate ("email's truncate keeps max + ellipsis on purpose, see packages/email/src/template.ts") would make the difference visible to the next person who tries to unify them, but that fits the follow-up rather than blocking this.

[maple-review-bot]

Maple review

🟢 Confidence 10/10 · safe to merge. Eviction, redirect and lint fixes are pinned by new tests; only the deliberate one-character truncate duplication (F1) is left open.
_{1 note · tests covered · risk medium}

Bounds the in-memory cache backend, makes non-strict redirects follow the fetch standard, and stops test-slop skipping chained test modifiers, plus a shared-helper cleanup across email, status-page and chat-platform. Safe to merge; only the deliberate one-character truncate duplication (F1) stays open.

• makeMemoryBackend sweeps expired entries on put once the map doubles past 256
• Non-strict redirects now turn 301/302 POST and 303 non-GET into a body-less GET
• escapeHtml and truncate move to @maple/strings; email keeps its own truncate
• test-slop walks chained modifiers such as it.effect.each(rows)

Still open from earlier reviews

• 🔵 Note · F1 · Two truncate helpers now cut one character apart · lib/strings/src/index.ts:82

_{b394a3d · What was checked, coverage and telemetry details are on the Maple / review check. Reply "won't fix" to dismiss a finding, or mention @maple-review-bot to ask about one.}