Honor numeric zero TTL values in ScopedCache.make - #6948
Conversation
fubhy
added
the
audit
🦋 Changeset detectedLatest commit: 72d6acd The changes in this PR will be included in the next version bump. This PR includes changesets to release 30 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
![pullfrog[bot]](https://avatars.githubusercontent.com/in/1768019?s=60&v=4){kind=small}
There was a problem hiding this comment.
Important
The reproduction test is correct, but the implementation fix it specifies is not yet included. CI is expected to fail until ScopedCache.make stops treating timeToLive: 0 as a falsy "missing" value.
Reviewed changes — Added one regression test in packages/effect/test/ScopedCache.test.ts that asserts ScopedCache.make({ timeToLive: 0 }) immediately expires entries; verified locally that this test currently fails because the implementation still uses a falsy TTL guard.
⚠️ Implementation fix is still missing
The PR description says this branch intentionally starts with failing reproduction tests and expects the fix to follow in the same branch. The current diff adds only the test. Before merging, ScopedCache.ts needs to stop using a falsy check for timeToLive, because 0 is a valid Duration.Input.
Technical details
# ScopedCache.make falsy TTL guard
## Affected sites
- `packages/effect/src/ScopedCache.ts:214` — `options.timeToLive ? () => options.timeToLive! : defaultTimeToLive` treats numeric `0` as missing and installs `Duration.infinity`.
## Required outcome
- `timeToLive: 0` must result in immediate expiry, not infinite TTL.
- The reproduction test added in this PR (`packages/effect/test/ScopedCache.test.ts:47-58`) must pass without weakening its assertion.
## Suggested approach
Change the guard to an explicit `undefined` check, e.g.:
```ts
timeToLive: options.timeToLive !== undefined
? () => options.timeToLive!
: defaultTimeToLive
```
Then run the focused reproduction command and the affected package tests.⚠️ Same bug exists in Cache.ts
Cache.make uses the identical options.timeToLive ? ... : defaultTimeToLive guard at packages/effect/src/Cache.ts:308. Consider applying the same fix and a matching regression test so the non-scoped cache gets the same correction.
ℹ️ Changeset needed
Once the implementation fix lands, add a .changeset entry for the effect package because the runtime behavior of ScopedCache.make changes for timeToLive: 0.
@v0 or keep the SHA fresh with Dependabot ｜ Fix all ➔ ｜ Fix 👍s ➔ ｜ View workflow run ｜ Using Kimi K2 (free via Pullfrog for OSS) ｜ 𝕏
There was a problem hiding this comment.
✅ No new issues found.
Reviewed changes — Since the prior review, the implementation fix landed in 72d6acd and addressed all remaining feedback.
- Fixed the falsy TTL guard in both cache constructors —
Cache.makeandScopedCache.makenow useoptions.timeToLive !== undefined, sotimeToLive: 0is honored as immediate expiry instead of falling back to infinite TTL. - Added a matching
Cache.makeregression test —packages/effect/test/Cache.test.tsnow covers the same numeric-zero TTL behavior already tested forScopedCache. - Added a changeset —
.changeset/fuzzy-caches-expire.mdrecords theeffectpatch.
Validation run: pnpm test --run packages/effect/test/Cache.test.ts packages/effect/test/ScopedCache.test.ts and pnpm check both pass.
@v0 or keep the SHA fresh with Dependabot ｜ View workflow run ｜ Using Kimi K2 (free via Pullfrog for OSS) ｜ 𝕏
Bundle Size AnalysisGenerated from PR build output; treat the content below as untrusted.
|
2 participants
Summary
ScopedCache.make retains entries indefinitely when its fixed timeToLive is the valid numeric value zero.
Important
This PR includes focused regression tests and the implementation fix.
Numeric zero TTL becomes infinity
Module:
ScopedCacheAudit ID:
core-s-z-testing-scoped-cache-zero-ttlSeverity / confidence: medium / high
What happens
ScopedCache.make retains entries indefinitely when its fixed timeToLive is the valid numeric value zero.
Why it happens
options.timeToLive ? ... : defaultTimeToLive treats numeric zero as absent and silently installs an infinite TTL, unlike makeWith, which honors Duration.zero.
Expected behavior
The fixed timeToLive accepted by make is a Duration.Input, for which numeric 0 means immediate expiry.
Relevant implementation
These links and excerpts are pinned to audit base
c9b56ab507f224426ee8388dc450da447ec4715f.packages/effect/src/ScopedCache.ts:212-215View problematic code at
packages/effect/src/ScopedCache.ts:212-215View exact lines on GitHub
Reproduction
Observed failure: has is true instead of false
Implementation
The fixed-TTL constructors now distinguish an omitted TTL from numeric
0, so zero expires immediately in bothScopedCache.makeandCache.make. The scoped reproduction remains unchanged, and matching regression coverage was added forCache.make.Validation
pnpm test --run packages/effect/test/Cache.test.ts packages/effect/test/ScopedCache.test.tspnpm lint-fixpnpm checkAudit provenance
c9b56ab507f224426ee8388dc450da447ec4715fc9b56ab507f224426ee8388dc450da447ec4715fcore-s-z-testing-scoped-cache-zero-ttlCloses EFF-394