[salmanfarisvp]

Fixes the cache problems from the independent review of #46, plus the README items from that review.

Cache

The app cached the feed in one shared localStorage key (rssStore). It never recorded which feed the cache came from, and never checked its age.

Situation	Before	Now
rss_url changed and the new feed fails	The old feed's stories under the new title	Error screen
Feed down for days	Cached stories stay up indefinitely	Cached stories for up to 24 hours, then the error screen
Feed returns no items	Error screen, even with a good cache	The cached stories
cache_interval: 0 (or 1, or a huge value)	Refetches in a tight loop (huge values overflow setInterval, which then fires immediately)	Clamped to 60–86400 s, with a console warning

Caches are now stored per feed URL (rssStore:<url>), each with its URL and save time. When the app saves a cache, it removes the legacy key and other feeds' expired caches, so storage doesn't grow when rss_url changes.

One trade-off: the legacy cache doesn't say which feed it belongs to, so it's never used. If a screen's feed is down on its first load after this ships, it shows the error screen instead of the old cache, until the first successful fetch.

A correction to the review: a non-numeric cache_interval was not a loop, because getSettingWithDefault already falls back to the default for those. The loop came from 0, small values and Infinity.

README

• Releasing: any v<digit> tag, including pre-release tags like v26.11.0-rc.1, deploys to production. This comes from the shared edge-apps-actions tag filter, which every app uses.
• Overrides: use an app key with them. With the key empty, an override changes every feed app.
• Redeploying the main app: re-run the latest tag's workflow run. A manual run from a tag does nothing for it, which the README previously implied it did.
• cache_interval: the 60–86400 range and the 24-hour cache are documented in the README table and in the setting's help text in screenly.yml.

Testing

• Unit tests: 27 pass, including new ones for per-feed caches, expiry, legacy-key cleanup, pruning, malformed caches and the interval clamp.
• In a real browser (Playwright, real localStorage), each scenario in the table behaves as listed, plus the legacy-only case (error screen). With cache_interval: 0, the app fetched once in 3 s.
• Type-check, lint, format and build all pass.

[Copilot]

🟡 Changes recommended

Active moderate issues remain in cache recovery and validation, allowing a full or malformed cache to keep refreshes failing and potentially crash error rendering.

2 open issues

• Validate cached entry shapes before rendering · New
• Prune expired caches before saving to full storage · New

What changed in this PR

This pull request hardens RSS feed caching by scoping caches to feed URLs, expiring stale entries, handling refresh failures, and clamping refresh intervals. It also updates deployment and configuration documentation.

Changes:

• Added per-feed cache keys, timestamps, expiry handling, pruning, and legacy-cache cleanup.
• Added malformed-cache handling and fallback behavior for failed or empty refreshes.
• Clamped cache_interval to 60–86,400 seconds.
• Expanded unit tests and updated README and settings documentation.

This pull request requires changes.

File	Description
src/​utils.ts	Implements feed-specific caching, expiry, pruning, malformed-cache handling, and refresh interval clamping.
src/​utils.test.ts	Adds coverage for cache isolation, expiry, cleanup, malformed data, and interval limits.
src/​main.ts	Integrates feed-specific cache fallback and the clamped refresh interval.
screenly.yml	Documents cache retention, fallback behavior, and interval constraints.
README.md	Clarifies release tags, overrides, redeployment, and cache configuration.

_{Review effort: Lite}

[Copilot]

🔵 Needs a closer look

Active issues remain in src/utils.ts: future-dated cache entries can bypass the intended 24-hour limit, and valid string intervals trigger an erroneous out-of-range warning.

No open issues 🎉

2 previously missed issues

In code that hasn't changed since last review

Future-dated cache entries are incorrectly treated as fresh

src/​utils.ts:108

isFresh treats any future timestamp as fresh because a negative age still satisfies <= CACHE_MAX_AGE_MS. A clock correction or a manually preserved cache with a future timestamp can therefore keep stories on screen for longer than the promised 24 hours; require the timestamp to be no later than now before accepting the cache.

String cache intervals trigger a false out-of-range warning

src/​utils.ts:196

cache_interval is declared as a string setting (screenly.yml:18-20), and the screenshot configuration also supplies '1800'. The clamp coerces that value to the number 1800, but this strict comparison is against the original string, so every normal startup logs the out-of-range warning even though the interval is valid. Normalize the setting to a number before both clamping and comparing.

2 resolved since last review

• Prune expired caches before saving to full storage
• Validate cached entry shapes before rendering

_{Review effort: Lite}

[salmanfarisvp]

Following up on Copilot's review of 66ca21a, which listed two previously missed issues:

1. Future-dated caches treated as fresh (src/utils.ts:108): valid, fixed in the next commit. isFresh now accepts only an age from 0 to 24 hours. Players often boot with a wrong date until they sync, so a cache saved while the clock was ahead would otherwise outlive the 24-hour limit. Added a test.

2. String cache_interval triggers a false out-of-range warning (src/utils.ts:196): not an issue, no change. The value reaching getRefreshIntervalMs is already a number. getSettingWithDefault('cache_interval', 1800) has a numeric default, and in @screenly/edge-apps (dist/utils/settings.js) a string setting with a numeric default is parsed with Number(trimmed) and returned as a number. Checked at runtime with the real helper and screenly.settings.cache_interval = '1800':

value=1800 (number) ms=1800000 warnings=0

The browser tests agree: five runs with cache_interval: '1800' logged no warning, and only the run with '0' did.

[Copilot]

🟢 Approval recommended

The supplied review evidence identifies no active issues in the cache-hardening and fallback changes.

No open issues 🎉

_{Review effort: Lite}