Greptile Summary

This PR fixes the cron schedule cache eviction policy from FIFO to true LRU by adding a delete/set promotion on every cache hit in resolveCachedCron(), correctly leveraging JavaScript Map's insertion-order iteration.

• src/cron/schedule.ts — The LRU promotion logic itself is correct: deleting and re-inserting on a hit moves the entry to the tail of Map iteration order, so the existing head-eviction logic now removes the least recently used entry rather than the oldest inserted entry.
• src/cron/schedule.test.ts — The new test only asserts on getCronScheduleCacheSizeForTest() (always 512 after any eviction), so it passes identically under FIFO and LRU. The test does not actually verify that the promoted entry survives eviction; a regression that reverts the delete/set promotion would not be detected. A hasCronInCacheForTest helper and assertions checking the presence/absence of specific keys after eviction are needed to make this test meaningful.

Confidence Score: 3/5

• The implementation is correct but the accompanying test does not actually validate the LRU behaviour it claims to cover.
• The two-line production change (delete + set on hit) is a well-known correct pattern for LRU with a JavaScript Map and carries negligible risk. The score is reduced because the new test provides false confidence — it passes under both the old FIFO code and the new LRU code, meaning it would not catch a future regression.
• src/cron/schedule.test.ts — the LRU promotion test needs stronger assertions to be meaningful.

_{Last reviewed commit: a2dd5c5}

This pull request has been automatically marked as stale due to inactivity.
Please add updates or it will be closed.

Codex review: keeping this open for maintainer follow-up; there is still a little grit to resolve.

Keep this PR open. Current main still uses FIFO eviction for the cron evaluator cache: cache hits return without refreshing Map insertion order, and the full-cache path deletes the first insertion-order key. The PR remains a narrow implementation candidate, and the updated branch context addresses the earlier weak-test concern with key-level cache assertions. Security review found no CI, dependency, install, release, lockfile, or secret-handling changes in the supplied diff.

Best possible solution:

Review and land this PR, or land an equivalent maintainer patch, preserving the narrow delete/re-set promotion in resolveCachedCron() and the key-level regression test proving a recently accessed cron expression survives capacity eviction.

What I checked:

• Current main cache hit behavior: resolveCachedCron() reads cronEvalCache.get(key) and immediately returns the cached Cron on hit; there is no delete/re-set or other recency promotion. (src/cron/schedule.ts:17, ef58307f843e)
• Current main eviction behavior: When the cache reaches CRON_EVAL_CACHE_MAX, current main deletes cronEvalCache.keys().next().value, which is the insertion-order head. Without hit promotion, this is FIFO rather than LRU. (src/cron/schedule.ts:23, ef58307f843e)
• Current tests lack LRU regression coverage: Current main imports only getCronScheduleCacheSizeForTest for cache assertions and only verifies that repeated expression/timezone use leaves the cache size at 2. There is no key-level helper or promoted-entry survival test. (src/cron/schedule.test.ts:2, ef58307f843e)
• Latest release also lacks the fix: v2026.4.26 points at be8c24633aaa7ef0425ae1178f096ee8dd6226c0, and tag inspection shows the same FIFO hit/eviction code in src/cron/schedule.ts lines 17-27. (src/cron/schedule.ts:17, be8c24633aaa)
• PR branch addresses the stated behavior: Provided PR context for head 910255af9d7898286e020c6e05ee7ca425706165 adds delete/re-set promotion on cache hit, exposes cache max/key-presence test helpers, and asserts that 0 0 * * * survives while 1 0 * * * is evicted after capacity pressure. (src/cron/schedule.test.ts:146, 910255af9d78)
• Related item context: The PR body uses closing syntax for [Bug] Cron schedule cache eviction is FIFO, not LRU — hot entries get evicted prematurely #39679. The provided context shows [Bug] Cron schedule cache eviction is FIFO, not LRU — hot entries get evicted prematurely #39679 and duplicate PR fix(cron): implement true LRU cache eviction #39706 are already closed, so closing this PR without a main implementation would leave no open canonical fix path for the remaining cron cache behavior.

Likely related people:

• Chris Zhang chris@ChrisdeMac-mini.local: Current checkout blame attributes the cron schedule implementation and tests to grafted commit 081e4be11e6382e4ed49b4753f9be137bece8fe7, which created the relevant files in this local history. (role: current-main file history; confidence: medium; commits: 081e4be11e63; files: src/cron/schedule.ts, src/cron/schedule.test.ts)
• Peter Steinberger steipete@gmail.com: Latest release tag v2026.4.26 points at be8c24633aaa7ef0425ae1178f096ee8dd6226c0, whose local history includes the same cron schedule files and FIFO behavior. This is routing evidence for release-current state, not fault attribution. (role: release maintainer / latest-release touch; confidence: low; commits: be8c24633aaa; files: src/cron/schedule.ts, src/cron/schedule.test.ts)
• vincentkoc / Vincent Koc: The provided PR timeline and maintainer comment say Vincent Koc force-pushed a narrow repair to this branch and added/removes Clownfish review labels, making them a likely follow-up route for this open implementation candidate. (role: recent maintainer follow-up owner; confidence: medium; commits: 910255af9d78; files: src/cron/schedule.ts, src/cron/schedule.test.ts)

Remaining risk / open question:

• Closing this PR now would leave current main and latest release with FIFO cron evaluator-cache eviction while the linked bug and duplicate implementation path are already closed.
• This read-only review did not run the PR branch tests or changed gate; maintainers should rerun current-base validation before merge.

Codex review notes: model gpt-5.5, reasoning high; reviewed against ef58307f843e.

ProjectClownfish pushed a narrow repair to this branch so the original contributor path can stay canonical.

Source PR: #39703
Validation: pnpm check:changed
Contributor credit is preserved in the branch history and PR context.