Skip to content

fix cache ttl#726

Merged
whoiskatrin merged 10 commits into
mainfrom
fix-async
Dec 11, 2025
Merged

fix cache ttl#726
whoiskatrin merged 10 commits into
mainfrom
fix-async

Conversation

@whoiskatrin

@whoiskatrin whoiskatrin commented Dec 9, 2025

Copy link
Copy Markdown
Contributor

No description provided.

@changeset-bot

changeset-bot Bot commented Dec 9, 2025 •
edited
Loading

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: b4a44db

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
agents Patch

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

@claude

claude Bot commented Dec 9, 2025 •
edited
Loading

Copy link
Copy Markdown

Claude Code Review

Issues:

  1. Race condition with immediate expiration (react.tsx:49): When cacheTtl=0, entries expire at Date.now() but the check is Date.now() >= entry.expiresAt. Within the same millisecond, the entry may still be retrievable. The test waits 1ms to avoid this, but production code could see flaky behavior. Consider using > instead of >= or setting expiresAt to Date.now() - 1 for immediate expiration.

  2. Unclear refetch-on-expiry semantics (react.tsx:262-265): When cache expires, setCacheInvalidatedAt forces re-render and creates new promise, refetching already-cached data. If goal is cleanup, getCacheEntry already handles this. If goal is to refetch after TTL, this should be explicit in docs/tests as it changes behavior from ref-counted cache.

  3. Memory leak potential (react.tsx:268-286): If component unmounts before TTL timer fires, cache entry remains in map forever. The cleanup only clears the timer, not the cache entry. Consider: return () => { clearTimeout(timer); deleteCacheEntry(cacheKey); }.

  4. Redundant timers (react.tsx:268-286): If N components use same query, N timers are scheduled but only first to fire deletes entry. Consider single global timer per cache key, or let cleanup happen lazily in getCacheEntry.

Minor:

  • Test names improved, coverage expanded ✓
  • Simplified cache key from array comparison to string ✓
  • Removed ref counting complexity ✓

@pkg-pr-new

pkg-pr-new Bot commented Dec 9, 2025 •
edited
Loading

Copy link
Copy Markdown

Open in StackBlitz

npm i https://pkg.pr.new/cloudflare/agents@726

commit: b4a44db

@whoiskatrin whoiskatrin mentioned this pull request Dec 9, 2025
Closed
agents-git-bot Bot pushed a commit to cloudflare/cloudflare-docs that referenced this pull request Dec 9, 2025
@github-actions @claude
Document cacheTtl option and async query caching in useAgent hook
40aa36c 
This update adds documentation for the cacheTtl option in the useAgent hook,
which was previously undocumented. The documentation clarifies:

- Default cache behavior (5 minute TTL)
- How to customize cache duration with cacheTtl
- How to disable caching by setting cacheTtl to 0
- Cache invalidation with queryDeps

This corresponds to a bug fix in cloudflare/agents PR #726 that corrected
the behavior when cacheTtl is set to 0 (previously incorrectly cached for
5 minutes instead of disabling cache).

Related: cloudflare/agents#726

🤖 Generated with Claude Code

Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
@agents-git-bot agents-git-bot Bot mentioned this pull request Dec 9, 2025
Closed
mattzcarey
mattzcarey reviewed Dec 11, 2025
View reviewed changes
Comment thread packages/agents/src/react-tests/cache-ttl.test.ts

it("should respect cacheTtl of 0 (immediate expiration)", async () => {
const key = ["test", "default", "dep1"];
const key = "test-key-1";

@mattzcarey mattzcarey Dec 11, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe silly question but is this breaking?

@whoiskatrin
whoiskatrin merged commit 59ac254 into main Dec 11, 2025
8 checks passed
@whoiskatrin
whoiskatrin deleted the fix-async branch December 11, 2025 11:53
@github-actions github-actions Bot mentioned this pull request Dec 11, 2025
Merged
@aurewill-gavel aurewill-gavel mentioned this pull request Feb 3, 2026
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@mattzcarey mattzcarey mattzcarey approved these changes
@ghostwriternr ghostwriternr Awaiting requested review from ghostwriternr

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@whoiskatrin @mattzcarey