fix(app-router): propagate "use cache" tags to route-handler ISR entries (#1453)

[james-elicx]

Summary

Addresses #1453 (the route-handler revalidate sub-part); the other sub-parts
are deferred (see below).

A route handler whose body is produced by a "use cache" function tagged with
cacheTag() could not be invalidated via revalidateTag() / revalidatePath().
cacheTag() tags were attached only to the inner data cache entry and never
propagated to the surrounding route-handler ISR entry, so on-demand
revalidation evicted the data cache entry but left the cached route-handler
response in place — the handler kept serving stale (HIT) output.

Fix

In the "use cache" runtime (packages/vinext/src/shims/cache-runtime.ts),
propagate a cache scope's tags up to the surrounding request's collected tags
(which feed the page / route-handler ISR entry), mirroring Next.js's
propagateCacheLifeAndTagsToRevalidateStore:

• On a data cache HIT, surface the cached entry's stored tags.
• On a MISS, surface the resolved scope's ctx.tags.
• Nested "use cache" scopes bubble their tags into the parent scope, so only
  the outermost scope writes onto the request tag list. "use cache: private"
  tags stay isolated (private caches only nest inside private caches).

New helper addCollectedRequestTags() in fetch-cache.ts appends (deduped)
into the same currentRequestTags list that tagged fetch() already uses.

Test

tests/app-router-production-server.test.ts — new case:
"route handler ISR: revalidateTag evicts a 'use cache' cacheTag()-tagged route
handler". Fixtures: app/api/use-cache-tagged (a "use cache"-bodied GET tagged
use-cache-rh-tag, long revalidate so only tag invalidation evicts it) and
app/api/revalidate-use-cache-rh-tag. Fails before the fix (post-revalidate
request still HIT with the old timestamp), passes after.

Deferred (other #1453 sub-parts, out of scope here)

• Build-time "use cache" evaluation for metadata route handlers / sitemap /
  robots (sentinel=buildtime).
• "use cache: private" exposing cookies / searchParams.
• A "use cache"-bodied route handler with no export const revalidate
  being implicitly ISR-cacheable from its resolved cacheLife.
• Nested cached function passed as a prop into a server action.

These need broader/separate changes and are intentionally not bundled here to
keep this PR small.

[james-elicx]

/bigbonk review for issues

[pkg-pr-new]

Open in StackBlitz

npm i https://pkg.pr.new/@vinext/cloudflare@1848

npm i https://pkg.pr.new/vinext@1848

commit: be52d85

[github-actions]

Example	Preview	Production	Original
app-router-cloudflare	preview	production
pages-router-cloudflare	preview	production
app-router-playground	preview	production	original
realworld-api-rest	preview	production
nextra-docs-template	preview	production
benchmarks	preview	production
hackernews	preview	production	original
workers-cache	preview	production
web	preview	production

[ask-bonk]

@james-elicx Bonk workflow was cancelled.

View workflow run · To retry, trigger Bonk again.

[james-elicx]

/bigbonk review for issues

[ask-bonk]

/bigbonk review — PR #1848

Reviewed the tag-propagation changes. The core fix is correct for the top-level "use cache" case (verified empirically: a top-level cached fn that HITs now surfaces its tags to the request). The addCollectedRequestTags helper in fetch-cache.ts is sound and consistent with the existing tag-collection model.

I found one real correctness gap in the nested-cache path.

Nested "use cache" HIT does not bubble tags to its parent scope

packages/vinext/src/shims/cache-runtime.ts

The parent-scope tag bubbling (the block that mirrors Next.js propagateCacheLifeAndTagsToRevalidateStore) lives only in runCachedFunctionWithContext — i.e., the MISS path:

// runCachedFunctionWithContext (line ~810-822) — only runs on MISS
if (parentCtx) {
  parentCtx.lifeConfigs.push(effectiveLife);
  for (const tag of ctx.tags) {
    if (!parentCtx.tags.includes(tag)) parentCtx.tags.push(tag);
  }
}

On a data cache HIT (line 532-555), runCachedFunctionWithContext is never called. The HIT path instead calls propagateCacheTagsToRequest(existing.value.tags) (line 539), which bails out early when a parent cache scope is present:

function propagateCacheTagsToRequest(tags) {
  if (!tags || tags.length === 0) return;
  if (cacheContextStorage.getStore()) return; // <-- parent present: returns without bubbling
  addCollectedRequestTags(tags);
}

So when an inner "use cache" HITs while nested inside an outer "use cache" that is being (re)generated, the inner entry's stored tags are dropped entirely — they neither reach the parent ctx.tags nor the request. The outer cache entry is then written with tags: ctx.tags (line 600/606) that omit the inner's tags.

This is the same bug class this PR is fixing, just for the nested-HIT path: a later revalidateTag("inner-tag") evicts the inner data cache entry but leaves the outer page/route-handler ISR entry stale.

Empirical confirmation (scratch test, now removed):

top-level cached fn HIT propagates tags to request  ✓  -> ['toplevel-tag']
nested cached fn HIT bubbles tags to outer entry     ✗  -> outer entry tags: []  (expected to include 'inner-tag')

The outer entry's stored tags came back [], missing inner-tag.

Suggested fix: on a nested HIT, bubble the cached entry's tags into parentCtx.tags (deduped) instead of bailing out. For example, have propagateCacheTagsToRequest push into the parent context when one exists rather than returning, mirroring the MISS-path bubbling — or add an explicit parent-bubble at the HIT site (line 539). Worth adding a nested-HIT regression test in tests/shims.test.ts alongside the existing revalidateTag invalidates cached entries case.

Minor

• The comment on propagateCacheTagsToRequest ("nested cache scope's tags are propagated to its parent scope instead") describes the intended MISS-path behavior, but it is not true on the HIT path — the function silently drops the tags when a parent is present. The comment should be reconciled with the fix above.

Everything else looks good. The deferred sub-parts listed in the PR body are reasonable to keep out of scope. The top-level fix and its test are correct.

[ask-bonk]

Review posted to PR #1848.

Summary

I reviewed the tag-propagation changes and found one real correctness gap, confirmed empirically.

What works (verified):

• The top-level "use cache" fix is correct — a top-level cached fn that HITs now surfaces its tags to the request, so revalidateTag() can evict the enclosing route-handler/page ISR entry.
• The new addCollectedRequestTags() helper in fetch-cache.ts is sound and consistent with the existing tag-collection model.
• The PR's new production-server test passes.

Bug found — nested "use cache" HIT drops tags:

The parent-scope tag bubbling lives only in runCachedFunctionWithContext (the MISS path). On a data cache HIT, that function is never called, and propagateCacheTagsToRequest() bails out early (line 666) when a parent cache scope is present — without bubbling the cached entry's tags into the parent ctx.tags. As a result, an outer "use cache" entry being regenerated while an inner "use cache" HITs is written with tags that omit the inner's tags, so revalidateTag("inner-tag") later evicts the inner data cache but leaves the outer ISR entry stale.

I confirmed this with a scratch test (then removed it): the outer entry's stored tags came back [], missing inner-tag.

Minor: The comment on propagateCacheTagsToRequest claims nested tags are "propagated to its parent scope instead," but on the HIT path they are silently dropped — the comment and code disagree.

The full review with the suggested fix and reproduction is posted at the PR comment link above.

github run

[james-elicx]

/bigbonk review for issues

[ask-bonk]

/bigbonk review — PR #1848 (re-review after be52d85)

Re-reviewed after the follow-up commit be52d85 that addresses the nested-HIT gap from the previous round. The fix is correct and complete. No outstanding issues.

Previously-flagged nested-HIT bug: fixed ✅

The earlier review found that on a data cache HIT, propagateCacheTagsToRequest() bailed out early when a parent cache scope was present, dropping the inner entry's tags so the outer ISR entry was written without them. Commit be52d85 resolves this: instead of returning, the function now bubbles the tags into parentCtx.tags (deduped), mirroring the MISS-path bubbling in runCachedFunctionWithContext.

packages/vinext/src/shims/cache-runtime.ts:668

function propagateCacheTagsToRequest(tags) {
  if (!tags || tags.length === 0) return;
  const parentCtx = cacheContextStorage.getStore();
  if (parentCtx) {
    for (const tag of tags) {
      if (!parentCtx.tags.includes(tag)) parentCtx.tags.push(tag);
    }
    return;
  }
  addCollectedRequestTags(tags);
}

Verified empirically — the new regression test passes:

• tests/shims.test.ts → "nested cacheTag bubbles to the outer cache entry even when the inner HITs" ✅ (outer entry carries both outer-tag and inner-tag)

Stale comment: reconciled ✅

The docstring on propagateCacheTagsToRequest (previously claiming nested tags are "propagated to its parent scope instead" while the code dropped them) now accurately documents both HIT and MISS behavior (cache-runtime.ts:653-666).

Verification

Ran the relevant suites locally — all green:

• tests/shims.test.ts ("use cache" runtime): 43 passed
• tests/app-router-production-server.test.ts (route-handler ISR revalidate case): passed
• tests/fetch-cache.test.ts + tests/app-route-handler-cache.test.ts + tests/unified-request-context.test.ts: 136 passed
• tests/isr-cache.test.ts + tests/app-route-handler-execution.test.ts: 70 passed
• vp check on both changed source files: clean (format, lint, types)

Notes (no action required)

• MISS path double-push is harmless. On a nested MISS, the inner's tags are bubbled into parentCtx.tags twice — once in runCachedFunctionWithContext (cache-runtime.ts:829) and again via propagateCacheTagsToRequest at cache-runtime.ts:569. Both are deduped, so this is correct, just slightly redundant. Not worth changing.
• Public-nested-in-private loses tags, consistently. A public "use cache" nested inside a "use cache: private" parent will push its tags into the private parent's ctx.tags, which is never persisted or surfaced. This is the same behavior on both HIT (cache-runtime.ts:671) and MISS (cache-runtime.ts:822) paths, so it is consistent and not a regression introduced by this PR. The private cache path itself never calls propagateCacheTagsToRequest, so private tags stay isolated as intended.
• The fix is correctly wired through to the route-handler/page ISR entries: addCollectedRequestTags → currentRequestTags → getCollectedFetchTags() → app-route-handler-cache.ts:133 / app-page-dispatch.ts:840.

The deferred sub-parts in the PR body are reasonable to keep out of scope. LGTM.

[ask-bonk]

Review posted to PR #1848.

Summary

This PR has two commits — the original fix (54c81dd) and a follow-up (be52d85) that addresses the nested-HIT bug flagged in the prior /bigbonk review. I re-reviewed the full state and found no outstanding issues.

Previously-flagged nested-HIT bug — now fixed: propagateCacheTagsToRequest() previously bailed out early when a parent cache scope was present, dropping an inner "use cache" entry's tags on a data cache HIT. Commit be52d85 makes it bubble the tags into parentCtx.tags instead, mirroring the MISS-path bubbling. The accompanying regression test (nested cacheTag bubbles to the outer cache entry even when the inner HITs) confirms the outer entry now carries both tags.

Stale comment — reconciled: the docstring now accurately describes both HIT and MISS behavior.

Verification (all local, green):

• "use cache" runtime suite: 43 passed
• production-server route-handler ISR revalidate case: passed
• fetch-cache + route-handler-cache + unified-request-context: 136 passed
• isr-cache + route-handler-execution: 70 passed
• vp check on both changed source files: clean

Notes (no action required): a harmless deduped double-push on the nested MISS path, and a consistent (pre-existing, non-regression) edge where a public cache nested inside a "use cache: private" parent loses its tags. Private tags stay correctly isolated. The fix is properly wired through to the route-handler/page ISR entries.

LGTM.

github run