Skip to content

feat(cache): add tag-based caching and revalidation helpers - #1964

Open
dinwwwh wants to merge 44 commits into
mainfrom
claude/orpc-cache-implementation-09e313
Open

feat(cache): add tag-based caching and revalidation helpers#1964
dinwwwh wants to merge 44 commits into
mainfrom
claude/orpc-cache-implementation-09e313

Conversation

@dinwwwh

@dinwwwh dinwwwh commented Aug 28, 2026

Copy link
Copy Markdown
Member

Adds @orpc/experimental-cache: tag-based caching and revalidation for procedure output, with stale-while-revalidate, a CacheStore contract with memory, Redis, Upstash, Bun, and Vercel adapters plus a purge-only Cloudflare Workers Caching store, and a handler plugin that reflects cache tags into response headers for client-side revalidation or response caches in front. Concurrent callers of one key run the procedure once.

Resolves #1262

Features

  • cache() middleware: a hit returns the cached output without running the handler; a miss runs it once, even across concurrent callers, and stores the result. key defaults to the procedure path and input, canonically encoded so structurally equal keys share an entry; key, tags, ttl, swr, and enabled accept static values or functions of the middleware options and input, and enabled: false skips the store and the other resolvers.
  • Stale-while-revalidate: past ttl but within swr, stale output is served at once while one caller refreshes in the background. cache/waitUntil hands the refresh to runtimes that stop pending work after the response; without it, refresh failures surface as unhandled rejections rather than being swallowed.
  • revalidate({ tags }) middleware invalidates tags after a successful mutation; tags are non-empty at compile time, and resolving to null or undefined skips it.
  • CacheHandlerPlugin sets only the headers listed: orpc-cache-tag and orpc-cache-tag-invalidation for clients, cache-control and cache-tag for caches in front. cache-control uses max-age because s-maxage carries proxy-revalidate semantics that forbid stale reuse. Only the root procedure's activity is reflected, only on successful responses, and tags are percent-encoded so case-insensitive caches cannot collide distinct tags.
  • Stores implement fetch(key, fill, options) and revalidate({ tags }). The Redis, Upstash, and Bun stores keep entries as Redis hashes driven by Lua scripts: one round trip per hit, two per miss, with the lock taken in the same script that reads the entry and lockTtl freeing waiters if a holder dies. They share BaseRedisCacheStore and one entry format, so any two can serve the same database. The memory and Vercel stores share BaseKeyValueCacheStore and coalesce within the process. Every store carries one RPCJsonSerializer for keys and outputs.

Shared

  • @orpc/shared gains nowInSeconds, deepSortKeys, and the cache tag header codec; @orpc/bun and @orpc/cloudflare gain their stores.

Testing

  • 100% coverage on @orpc/experimental-cache, with a store contract shared by every adapter, env-gated Redis and Upstash integration suites, a Bun suite, cross-adapter compatibility suites proving Redis, Upstash, and Bun share entries, counters, retention, and locks, workerd tests for the Workers store, and race and lock-takeover tests.
  • Full type-check, lint, and docs validation pass.

Docs

  • New docs/helpers/cache page with per-adapter sections; API reference and package lists updated.

dinwwwh added 18 commits August 27, 2026 14:53
…implementation-09e313

# Conflicts:
#	README.md
#	apps/content/docs/procedure.mdx
#	packages/ai-sdk/README.md
#	packages/arktype/README.md
#	packages/bun/README.md
#	packages/client/README.md
#	packages/cloudflare/README.md
#	packages/contract/README.md
#	packages/effect/README.md
#	packages/evlog/README.md
#	packages/hibernation/README.md
#	packages/json-schema/README.md
#	packages/nest/README.md
#	packages/next/README.md
#	packages/node/README.md
#	packages/openapi/README.md
#	packages/opentelemetry/README.md
#	packages/pinia-colada/README.md
#	packages/pino/README.md
#	packages/publisher/README.md
#	packages/ratelimit/README.md
#	packages/server/README.md
#	packages/server/src/procedure-client.test.ts
#	packages/shared/README.md
#	packages/swr/README.md
#	packages/tanstack-query/README.md
#	packages/trpc/README.md
#	packages/valibot/README.md
#	packages/zod/README.md
#	pnpm-lock.yaml
@pkg-pr-new

pkg-pr-new Bot commented Aug 28, 2026

Copy link
Copy Markdown
More templates

@orpc/ai-sdk

npm i https://pkg.pr.new/middleapi/orpc/@orpc/ai-sdk@1964

@orpc/arktype

npm i https://pkg.pr.new/middleapi/orpc/@orpc/arktype@1964

@orpc/bun

npm i https://pkg.pr.new/middleapi/orpc/@orpc/bun@1964

@orpc/experimental-cache

npm i https://pkg.pr.new/middleapi/orpc/@orpc/experimental-cache@1964

@orpc/client

npm i https://pkg.pr.new/middleapi/orpc/@orpc/client@1964

@orpc/cloudflare

npm i https://pkg.pr.new/middleapi/orpc/@orpc/cloudflare@1964

@orpc/contract

npm i https://pkg.pr.new/middleapi/orpc/@orpc/contract@1964

@orpc/experimental-effect

npm i https://pkg.pr.new/middleapi/orpc/@orpc/experimental-effect@1964

@orpc/evlog

npm i https://pkg.pr.new/middleapi/orpc/@orpc/evlog@1964

@orpc/hibernation

npm i https://pkg.pr.new/middleapi/orpc/@orpc/hibernation@1964

@orpc/json-schema

npm i https://pkg.pr.new/middleapi/orpc/@orpc/json-schema@1964

@orpc/experimental-msw

npm i https://pkg.pr.new/middleapi/orpc/@orpc/experimental-msw@1964

@orpc/nest

npm i https://pkg.pr.new/middleapi/orpc/@orpc/nest@1964

@orpc/next

npm i https://pkg.pr.new/middleapi/orpc/@orpc/next@1964

@orpc/node

npm i https://pkg.pr.new/middleapi/orpc/@orpc/node@1964

@orpc/openapi

npm i https://pkg.pr.new/middleapi/orpc/@orpc/openapi@1964

@orpc/opentelemetry

npm i https://pkg.pr.new/middleapi/orpc/@orpc/opentelemetry@1964

@orpc/pinia-colada

npm i https://pkg.pr.new/middleapi/orpc/@orpc/pinia-colada@1964

@orpc/pino

npm i https://pkg.pr.new/middleapi/orpc/@orpc/pino@1964

@orpc/publisher

npm i https://pkg.pr.new/middleapi/orpc/@orpc/publisher@1964

@orpc/ratelimit

npm i https://pkg.pr.new/middleapi/orpc/@orpc/ratelimit@1964

@orpc/server

npm i https://pkg.pr.new/middleapi/orpc/@orpc/server@1964

@orpc/shared

npm i https://pkg.pr.new/middleapi/orpc/@orpc/shared@1964

@orpc/swr

npm i https://pkg.pr.new/middleapi/orpc/@orpc/swr@1964

@orpc/tanstack-query

npm i https://pkg.pr.new/middleapi/orpc/@orpc/tanstack-query@1964

@orpc/trpc

npm i https://pkg.pr.new/middleapi/orpc/@orpc/trpc@1964

@orpc/valibot

npm i https://pkg.pr.new/middleapi/orpc/@orpc/valibot@1964

@orpc/zod

npm i https://pkg.pr.new/middleapi/orpc/@orpc/zod@1964

commit: 468cd88

@cloudflare-workers-and-pages

cloudflare-workers-and-pages Bot commented Aug 28, 2026

Copy link
Copy Markdown

Deploying with  Cloudflare Workers  Cloudflare Workers

The latest updates on your project. Learn more about integrating Git with Workers.

Status Name Latest Commit Preview URL Updated (UTC)
✅ Deployment successful!
View logs
orpc 468cd88 Commit Preview URL

Branch Preview URL
Sep 09 2026, 01:30 PM

@codecov

codecov Bot commented Aug 28, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@codspeed-hq

codspeed-hq Bot commented Aug 28, 2026

Copy link
Copy Markdown
Contributor

Merging this PR will degrade performance by 70.7%

❌ 1 regressed benchmark
✅ 29 untouched benchmarks

Warning

Please fix the performance issues or acknowledge them on CodSpeed.

Performance Changes

Benchmark BASE HEAD Efficiency
flat object from query params 106 µs 361.8 µs -70.7%

Tip

Investigate this regression by commenting @codspeedbot fix this regression on this PR, or directly use the CodSpeed MCP with your agent.


Comparing claude/orpc-cache-implementation-09e313 (468cd88) with main (523a9d7)

Open in CodSpeed

@pullfrog pullfrog Bot left a comment

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.

Important

One behavioral issue to resolve: a revalidation failure after a committed mutation surfaces as an error on a request whose write already succeeded. See the inline comment on revalidate.

Reviewed changes

  • @orpc/cache (new package)cache()/revalidate() middlewares, CacheStore contract, tag-version invalidation, stale-while-revalidate, CacheHandlerPlugin header reflection, and MemoryCacheStore/RedisCacheStore/VercelCacheStore adapters.
  • @orpc/cloudflareKVCacheStore (real KV bindings) and purge-only WorkersCacheStore, plus workerd coverage.
  • @orpc/shared — new deepSortKeys util and tests.
  • Docs/config — new docs/helpers/cache page, README/package-list updates, api-reference row, new package package.json with subpath exports, workspace wiring.

Overall this is a careful, well-tested addition. I verified the highest-risk semantics rather than taking them on faith: the tag-version technique errs on the safe side (a lost concurrency race produces a spurious miss and recompute, never a stale hit), the tag header encoding round-trips correctly under case-folding and stays consistent between the reflected cache-tag and WorkersCacheStore purge, blob/streaming outputs are guarded where they cannot be stored, and the docs call out the CDN/purge-store and per-request-shared-key caveats. Two non-blocking nits are inline.

Pullfrog  | Fix all ➔Fix 👍s ➔View workflow run | Using DeepSeek Flash (free via Pullfrog for OSS) | 𝕏

Comment thread packages/cache/src/middleware.ts Outdated
Comment thread packages/cache/src/middleware.ts Outdated
Comment thread packages/cache/src/middleware.ts Outdated

@pullfrog pullfrog Bot left a comment

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.

Important

One behavior to confirm from the delta's cleanup: dropping isUncacheableOutput from the middleware also dropped the "don't record a cache check for streaming outputs" guarantee, so streaming responses are now advertised as cache activity even though every adapter ignores them. Full context inline.

Reviewed changes

This run reviews the single-commit delta e93c93e5 since the last-reviewed head 17c31c21:

  • CacheStore.revalidateTags — replaced revalidateTag(string | list) with a non-empty tag list across the interface, all four adapters, and their mocked/integration tests; stores dropped their empty-list branches and the middleware now guards with tags?.length.
  • revalidate({ tags }) API — options-object form whose tags is a Value of a non-empty tuple or undefined; resolving to undefined (or []) skips both the store call and the plugin-context recording.
  • Context keysCacheContext moved to namespaced 'cache/store' / 'cache/waitUntil'; tests, type tests, docs, and e2e all updated, with no context.cache / context.waitUntil stragglers.
  • cache() key semantics — any provided key (string or object) is now used as given; only the omitted-key default derives [path, input]; documented as "procedures sharing a key share an entry".
  • Streaming-output drops moved into storesisUncacheableOutput deleted; MemoryCacheStore.set now ignores ReadableStream/async-iterator outputs, matching Redis/KV/Vercel.

Pullfrog  | Fix all ➔Fix 👍s ➔View workflow run | Using DeepSeek Flash (free via Pullfrog for OSS) | 𝕏

`CacheStore.revalidateTag` becomes `revalidate({ tags })`, taking a non-empty
tag list, and every duration is now in seconds rather than milliseconds,
matching what Redis, Workers KV, the Vercel Runtime Cache, and `Cache-Control`
all accept. Entries without tags carry `undefined` instead of an empty array,
and stores no longer inspect output, passing it straight to their serializer.

The store and its background-work hook move to the namespaced `cache/store`
and `cache/waitUntil` context keys. A background refresh is handed over
uncaught so `cache/waitUntil` can report its failures. `revalidate` takes an
options object with a required `tags`, and a provided `key` is used as given.

The handler plugin takes header names as plain literals rather than exported
constants, sets them whatever the request method, and emits `max-age` instead
of `s-maxage`, which carries the `proxy-revalidate` semantics that would
forbid the stale reuse `stale-while-revalidate` grants.

Tag header encoding and `nowInSeconds` move to `@orpc/shared`, and each store
builds its key serializer once instead of per call.
…mplementation-09e313

# Conflicts:
#	README.md
#	packages/ai-sdk/README.md
#	packages/arktype/README.md
#	packages/bun/README.md
#	packages/client/README.md
#	packages/cloudflare/README.md
#	packages/cloudflare/package.json
#	packages/contract/README.md
#	packages/effect/README.md
#	packages/evlog/README.md
#	packages/hibernation/README.md
#	packages/json-schema/README.md
#	packages/nest/README.md
#	packages/next/README.md
#	packages/node/README.md
#	packages/openapi/README.md
#	packages/opentelemetry/README.md
#	packages/pinia-colada/README.md
#	packages/pino/README.md
#	packages/publisher/README.md
#	packages/ratelimit/README.md
#	packages/server/README.md
#	packages/shared/README.md
#	packages/swr/README.md
#	packages/tanstack-query/README.md
#	packages/trpc/README.md
#	packages/valibot/README.md
#	packages/zod/README.md
#	pnpm-lock.yaml
- RedisCacheStore and experimental_KVCacheStore take their client as the first argument
- experimental_WorkersCacheStore defaults to the cache exported by cloudflare:workers
- Redis, Upstash, and Bun stores share the shorter e:/t: key families and one envelope format
- race-condition tests for every store and the cache middleware
@dinwwwh
dinwwwh force-pushed the claude/orpc-cache-implementation-09e313 branch from e93c93e to 8a12845 Compare September 4, 2026 03:25

@pullfrog pullfrog Bot left a comment

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.

Important

This delta removes the fallback .catch from the stale-while-revalidate refresh path, so on the common Node setup (no cache/waitUntil wired) a failed background refresh now becomes an unhandled rejection — which terminates the process by default. Prior reviews' IMPORTANT threads on revalidate fail-loud and un-storable outputs stored as {} also remain open.

Reviewed changes

This run reviews the PR-owned delta since the prior pullfrog review at 8a12845b:

  • Removed experimental_KVCacheStore from @orpc/cloudflare — breaking; only the purge-only WorkersCacheStore remains. Clean: no dangling references in code, exports, tsconfig.json, README, wrangler KV binding, or the docs page.
  • Reworked SWR refresh ownership — the middleware now hands the refresh to cache/waitUntil unconditionally with no fallback catch, so without a waiter a refresh failure surfaces as an unhandled rejection.
  • Updated the handler-plugin headers JSDoc to match current behavior (any method, overriding), and dropped the now-orphaned "overrides existing headers" test, leaving the presetHeadersPlugin/preset test scaffolding unused.
  • Hardened the shared holdResult race-test helper with a read promise so the racing-revalidation tests sequence deterministically across Memory/Redis/Upstash/Bun.
  • Expanded cross-adapter compatibility suites (tag counters, retention staging) and gated the Upstash integration suite off Node 26 with a TODO.
  • Synced the packages table across every package README (markdown-table restyle).

Inline feedback anchors on the SWR refresh change. The two prior IMPORTANT threads (revalidate fail-loud; un-storable outputs persisted and served as {}) are unchanged by this delta and remain to address. Two nits are inline.

Pullfrog  | Fix all ➔Fix 👍s ➔View workflow run | Using DeepSeek Flash (free via Pullfrog for OSS) | 𝕏

Comment thread packages/cache/src/middleware.ts Outdated
Comment thread packages/cache/src/handler-plugin.test.ts
CacheStore gains an optional lock(key, fn) that runs fn for one caller at a
time per key, reporting whether the caller waited. The middleware fills a miss
and refreshes a stale entry under it, re-reading only after waiting so hits
never read twice. Redis, Upstash, and Bun hold the lock in Redis with SET NX
and a token-checked release; memory and Vercel hold it in process.
@pullfrog

pullfrog Bot commented Sep 5, 2026

Copy link
Copy Markdown
Contributor

Pullfrog stalled

The agent stopped emitting events for 125s and was killed by the activity-timeout watchdog. The model produced no output at all before the stall — the request was sent but nothing came back. This is usually transient; re-running often succeeds.

Pullfrog  | Rerun failed job ➔View workflow run | via Pullfrog | Using DeepSeek Flash (free via Pullfrog for OSS) | 𝕏

@pullfrog pullfrog Bot left a comment

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.

ℹ️ No new issues in this delta — the per-key locking rework and the MemoryLock relocation to @orpc/shared review clean.

Reviewed changes

This run covers the delta since the prior pullfrog review at 1c711593: commits 082d46f7 (per-key locks) and 685f61fa (MemoryLock@orpc/shared).

  • Added CacheStore.lock(key, fn) — an optional per-key lock on the store contract; the cache() middleware fills a miss and refreshes a stale entry under it, re-reading store.get(key) after waiting so concurrent callers fill/refresh once instead of once each (middleware.ts).
  • Implemented lock in the Redis-family adapters — a SET NX lease with PX lockTtl and a token-checked EVAL release in RedisCacheStore, UpstashCacheStore, and BunRedisCacheStore, so locks span processes; a crashed holder's lease expires and a taken-over lock is released only by its own token. Covered by mocked, integration, and cross-adapter (Redis↔Upstash↔Bun) tests.
  • Implemented lock in-process via MemoryLock for MemoryCacheStore and VercelCacheStore, handing the turn on after failures.
  • Moved MemoryLock to @orpc/shared (lock.ts, exported with tests) — clean move, no dangling references, and @orpc/experimental-cache already depends on @orpc/shared.
  • Docs updated — lock semantics, lockTtl, and "one fill / one refresh under lock" claims on the cache page.

I verified the highest-risk spots rather than taking them on faith: lock re-entry through serve() inside the miss-path lock does not deadlock on either lock kind (in-process callers queue behind the running callback; the Redis release fires as soon as serve returns, since the refresh isn't awaited), the waited re-read refuses a still-stale entry to the refresh path, and each open thread's code is untouched by this delta.

Carried forward from prior reviews and left open (unchanged by this delta): the revalidate fail-loud path, un-storable outputs persisted and served as {}, the SWR refresh unhandled-rejection default without cache/waitUntil, and the Upstash empty-tags / dead-preset scaffolding nits.

Pullfrog  | Fix it ➔View workflow run | Using DeepSeek Flash (free via Pullfrog for OSS) | 𝕏

…mplementation-09e313

# Conflicts:
#	README.md
#	apps/content/docs/api-reference.mdx
#	packages/ai-sdk/README.md
#	packages/arktype/README.md
#	packages/bun/README.md
#	packages/bun/package.json
#	packages/client/README.md
#	packages/cloudflare/README.md
#	packages/cloudflare/package.json
#	packages/cloudflare/src/index.ts
#	packages/contract/README.md
#	packages/effect/README.md
#	packages/evlog/README.md
#	packages/hibernation/README.md
#	packages/json-schema/README.md
#	packages/msw/README.md
#	packages/nest/README.md
#	packages/next/README.md
#	packages/node/README.md
#	packages/openapi/README.md
#	packages/opentelemetry/README.md
#	packages/pinia-colada/README.md
#	packages/pino/README.md
#	packages/publisher/README.md
#	packages/ratelimit/README.md
#	packages/server/README.md
#	packages/shared/README.md
#	packages/shared/src/index.ts
#	packages/swr/README.md
#	packages/tanstack-query/README.md
#	packages/trpc/README.md
#	packages/valibot/README.md
#	packages/zod/README.md
CacheStore is now fetch(key, fill, options) plus revalidate. Stores own
coalescing, so concurrent callers of one key fill once and stale entries
refresh once, with the refresh handed to options.waitUntil.

The Redis, Upstash, and Bun stores keep entries as hashes and run two Lua
scripts by sha: one reads the entry, validates its tags, and takes the lock
in the same step when it is missing or stale; the other stores the entry
with its tag versions snapshotted and releases the lock. A hit is one round
trip and a miss two, down from two and five.
…Lock

Stores with a native primitive keep their built-in lock. The Runtime Cache
has none, so VercelCacheStore accepts a Lock from @orpc/shared and holds it
within the process by default.

@pullfrog pullfrog Bot left a comment

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.

Caution

The fetch-fold rewrite (e9f5f591) mis-decodes the cache-miss sentinel on RESP3 clients — the default for the pinned redis@^6.2.1 client and for Bun's built-in client — so every cache miss is treated as a hit and the first request for each key is answered with an empty output. The head's CI is red on the new cross-adapter cache suite, and the PR description's "full suite passes" claim no longer holds.

Reviewed changes

  • Folded get/set/lock into CacheStore.fetch(key, fill, options) — stores now own entry coalescing and stale-refresh; a stale entry is served while one caller refreshes it in the background; the middleware became a thin passthrough.
  • Rewrote the Redis-family stores as prefix-scoped hashes driven by two EVALSHA Lua scripts — the read script validates tag versions and takes the SET NX PX lock in one step; the store script snapshots the versions, writes the entry with PEXPIRE, and releases the lock by token, with per-store script-sha caching and NOSCRIPT reload.
  • Moved the SWR refresh-failure contract into CacheFetchOptions.waitUntil — the option documents that without a waiter, failures surface as unhandled rejections, and the middleware test now asserts that contract.
  • Simplified the handler-plugin check recording — the middleware derives the reflected ttl from entry.expiresAt instead of carrying per-path refresh logic.
  • Merged upstream main — README package tables and description renames across every package README.

🚨 The head's CI is red

Run 34232486693 on e9f5f591 fails every test_matrix job in the new cache redis adapters compatibility suite, and lint_and_typecheck cannot resolve @standardserver/core from packages/cache/src/handler-plugin.ts. The PR description's "Full monorepo suite, type-check, lint, and docs validation pass" no longer holds for this head and should be restored before merging. The test failures — TypeError: null is not an object while destructuring the EVALSHA reply, a stale entry served back as its refreshed value, 20 s timeouts, and ERR_REDIS_CONNECTION_CLOSED — all sit in the newly scripted fetch path and corroborate the inline finding at packages/cache/src/adapters/redis.ts:158.

Technical details
# Head CI is red (run 34232486693, head e9f5f591)

## Evidence
- test_matrix (node 20/22/24/26): `packages/bun``tests/cache-redis-adapters-compatibility.test.ts` — all eight cross-adapter scenarios fail in both directions (redis → bun redis and bun redis → redis). node 22 first error: `TypeError: null is not an object (evaluating '[output, tags, expiresAt, shouldFill]')` at `redis-cache.ts:152`; node 20/22: `Expected: "v" Received: "refilled"` (the stale-serve assertion in the "shares retention" scenario); node 24/26: 20 s timeouts plus `ERR_REDIS_CONNECTION_CLOSED` in the harness.
- The root-vitest `redis-adapters-compatibility.test.ts` is skipped in CI: it requires `UPSTASH_REDIS_REST_URL`/`_TOKEN` (`stores.length < 2` guard) while CI only exports `REDIS_URL`, so the node-redis `RedisCacheStore` has no CI-enforced real-client coverage.
- lint_and_typecheck: `packages/cache/src/handler-plugin.ts(3,38): error TS2307 Cannot find module '@standardserver/core'`. `packages/cache` declares `@standardserver/core@^0.8.2` (resolved 0.8.3 in the lockfile); the runner logged a "pnpm v10 installation layout" warning, so this could be a toolchain artifact rather than a packaging gap — not yet attributed.

## Required outcome
- The cross-adapter cache suite must pass on the head before the rewrite merges; re-run the full suite after fixing the RESP3 sentinel decoding (inline comment at `redis.ts:158`).
- Re-attribute the `@standardserver/core` lint failure on a clean install; if it reproduces, the new package's dependency resolution is incomplete.
- Consider dropping the Upstash-credential gate so the node-redis store gets real-client CI coverage.

Pullfrog  | Fix all ➔Fix 👍s ➔View workflow run | Using DeepSeek Flash (free via Pullfrog for OSS) | 𝕏

Comment thread packages/cache/src/adapters/redis.ts Outdated
Comment thread packages/bun/src/redis-cache.ts Outdated
The scripts, the fetch loop, and the store step live once in an abstract
base; the Redis, Upstash, and Bun stores only run a script through their
client. Revalidation is one script too.

@pullfrog pullfrog Bot left a comment

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.

Important

The BaseRedisCacheStore extraction reads clean, but it consolidates the flagged miss-decode into a shared base the three Redis-family adapters now all depend on without fixing it, and the head's CI is still red on the cross-adapter cache suite plus the @standardserver/core type error. The two prior inline threads on the output !== null guard remain open.

Reviewed changes

This run reviews the delta since the prior pullfrog review at e9f5f591: cb97a43a + 10d3af0d (Vercel-lock feature added then reverted — net zero) and 26f97f95, the head commit.

  • BaseRedisCacheStore extraction — the two Lua scripts, the fetch loop, the store step, and revalidate now live once in an abstract base in packages/cache/src/adapters/redis.ts; RedisCacheStore, UpstashCacheStore, and BunRedisCacheStore only run a script through their client (EVALSHA with per-script sha caching for node-redis/bun, createScript for Upstash).
  • parseReply normalization — replies now take the same path whether the client returns raw strings (node-redis, bun) or pre-parsed values (Upstash); the fetch entry is decoded from parseReply(output).body.
  • REVALIDATE_SCRIPT — tag revalidation is a single INCR loop script shared by all three stores, replacing the per-store incr/multi paths.
  • NOSCRIPT recursion guard — the reload path now takes a reloaded flag so a persistent server-side script eviction cannot recurse forever (an improvement over the previous retry).
  • Test/doc churn — the bun integration suite drops its afterAll client close ("breaks the next file's client on Bun 1.4") and sets per-test timeouts; the cross-adapter suite hoists client setup to module scope with a combined afterAll; the cache docs note the shared base.

⚠️ The refactor preserves the open miss-decode blocker and head CI is still red

The extraction keeps the exact if (output !== null) guard (packages/cache/src/adapters/redis.ts:164) and the return { output or false, ... } sentinel (:40) that the prior review flagged as mis-decoding every miss as a hit on RESP3 clients — it moved them into the base rather than fixing them, which widens the blast radius to all three adapters instead of narrowing it.

Head CI on 26f97f95 (run 34237043815) confirms the blocker is live: test_matrix fails on node 20/22/24/26 in packages/bun/tests/cache-redis-adapters-compatibility.test.ts (20 s timeouts on entries/tag-counters/locks, stale-serve Expected "v" Received "refilled" on retention, ERR_REDIS_CONNECTION_CLOSED), and lint_and_typecheck still fails packages/cache/src/handler-plugin.ts(3,38) TS2307 Cannot find module '@standardserver/core'. The PR description's "full suite passes" claim still does not hold.

Technical details
# Base extraction is clean but the RESP3 miss-decode is unaddressed

## Affected sites
- packages/cache/src/adapters/redis.ts:40 and :164 (base) — `false` miss sentinel + `output !== null` guard; unchanged from the flagged e9f5f591 version, now shared by Redis/Upstash/Bun
- packages/bun/src/redis-cache.ts — thin `run` override inherits the base decode
- packages/cache/src/handler-plugin.ts:3 — `@standardserver/core` (no hyphen, deprecated) vs `@standard-server/core` used elsewhere; `StandardHeaders` is re-exported by `@orpc/server` (packages/server/src/index.ts:130), so the import source could change

## Required outcome
- Fix the miss sentinel so a miss never decodes as a hit on any client/RESP version (stop using Lua booleans as miss markers, or normalize in JS, e.g. `typeof output === 'string'`), then get the cross-adapter suite green.
- Resolve the `@standardserver/core` import/dependency so `type:check` passes.

## Open questions for the human
- Is `@standardserver/core@0.8.x` intentionally pinned (legacy deprecated name), or should the import come from `@orpc/server` / `@standard-server/core`?

Pullfrog  | Fix it ➔View workflow run | Using DeepSeek Flash (free via Pullfrog for OSS) | 𝕏

Exposed as @orpc/experimental-cache/base-redis, mirroring the publisher
package, so the Redis, Upstash, and Bun stores import it from one place.
- BaseKeyValueCacheStore owns the read, coalesce, and stale-refresh flow
  for the memory and Vercel stores; MemoryLock leaves @orpc/shared
- BaseRedisCacheStore keeps the scripts and flow, and each Redis client
  owns its script runner and NOSCRIPT handling
- every store carries one RPCJsonSerializer for keys and outputs, so the
  Redis hash and Vercel envelope hold the serialization directly
- resolveCacheExpiry replaces four copies of the ttl and swr arithmetic
- the middleware resolves enabled before the other options and treats an
  undefined key as the default; the plugin leaves untouched responses alone
- the cache package moves to @standard-server/core like the rest
- docs restructured with linked per-adapter sections

Copilot AI left a comment

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.

🟡 Changes recommended

Cache-key safety, revalidation races, Redis lock ownership, and stale-expiry handling require correction before approval.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

Introduces @orpc/experimental-cache, providing tag-based procedure caching, revalidation, stale-while-revalidate, response headers, and multiple storage adapters.

Changes:

  • Adds cache/revalidation middleware, handler plugin, and shared cache contracts.
  • Adds memory, Redis, Upstash, Vercel, Bun, and Cloudflare adapters with extensive tests.
  • Adds shared serialization helpers and updates documentation/package metadata.
File summaries
File Description
README.md Lists cache support.
eslint.config.js Permits experimental constructor naming.
package.json Adds cache workspace dependency.
pnpm-lock.yaml Updates workspace lock data.
tests/plugins/all-plugins.test.ts Includes the cache handler plugin.
apps/content/package.json Adds docs dependency.
apps/content/docs/api-reference.mdx Adds cache API links.
apps/content/docs/helpers/cache.mdx Documents cache usage and adapters.
packages/cache/README.md Adds generated package documentation.
packages/cache/package.json Defines the new package and exports.
packages/cache/tsconfig.json Configures package type-checking.
packages/cache/src/index.ts Exports public APIs.
packages/cache/src/index.test.ts Tests public exports.
packages/cache/src/types.ts Defines cache contracts and context.
packages/cache/src/utils.ts Implements key and expiry helpers.
packages/cache/src/utils.test.ts Tests stable key encoding.
packages/cache/src/middleware.ts Implements cache and revalidation middleware.
packages/cache/src/middleware.test.ts Tests middleware behavior and concurrency.
packages/cache/src/middleware.test-d.ts Tests middleware type inference.
packages/cache/src/handler-plugin.ts Emits cache response headers.
packages/cache/src/handler-plugin.test.ts Tests header generation and isolation.
packages/cache/src/adapters/base-key-value.ts Implements local request coalescing.
packages/cache/src/adapters/base-redis.ts Implements Redis scripts and locking.
packages/cache/src/adapters/memory.ts Adds the memory store.
packages/cache/src/adapters/memory.test.ts Tests memory caching behavior.
packages/cache/src/adapters/redis.ts Adds the Redis client adapter.
packages/cache/src/adapters/redis.test.ts Tests Redis integration.
packages/cache/src/adapters/upstash.ts Adds the Upstash adapter.
packages/cache/src/adapters/upstash.test.ts Tests Upstash integration.
packages/cache/src/adapters/vercel.ts Adds the Vercel Runtime Cache adapter.
packages/cache/src/adapters/vercel.test.ts Tests Vercel cache behavior.
packages/cache/tests/e2e.test.ts Tests end-to-end caching and invalidation.
packages/cache/tests/redis-adapters-compatibility.test.ts Tests cross-adapter compatibility.
packages/cache/tests/__shared__/store-contract.ts Defines shared store tests.
packages/cache/tests/__shared__/redis-store-contract.ts Defines Redis store tests.
packages/shared/src/time.ts Adds second-resolution time helper.
packages/shared/src/time.test.ts Tests time conversion.
packages/shared/src/object.ts Adds recursive key sorting.
packages/shared/src/object.test.ts Tests key sorting.
packages/shared/src/http.ts Adds cache-tag header encoding.
packages/shared/src/http.test.ts Tests tag header round-tripping.
packages/shared/src/index.ts Exports time utilities.
packages/bun/package.json Adds cache integration dependency.
packages/bun/src/index.ts Exports the Bun cache adapter.
packages/bun/src/redis-cache.ts Implements Bun Redis caching.
packages/bun/src/redis-cache.test.ts Tests the Bun adapter.
packages/bun/tests/cache-redis-adapters-compatibility.test.ts Tests Bun/Redis compatibility.
packages/cloudflare/package.json Adds Workers cache integration.
packages/cloudflare/src/index.ts Exports the Workers store.
packages/cloudflare/src/workers-cache.ts Implements purge-only Workers caching.
packages/cloudflare/src/workers-cache.test.ts Tests Workers cache purging.
packages/ai-sdk/README.md Adds cache documentation links.
packages/arktype/README.md Adds cache documentation links.
packages/bun/README.md Documents Bun cache support.
packages/client/README.md Adds cache documentation links.
packages/cloudflare/README.md Documents Workers cache support.
packages/contract/README.md Adds cache documentation links.
packages/effect/README.md Adds cache documentation links.
packages/evlog/README.md Adds cache documentation links.
packages/hibernation/README.md Adds cache documentation links.
packages/json-schema/README.md Adds cache documentation links.
packages/msw/README.md Adds cache documentation links.
packages/nest/README.md Adds cache documentation links.
packages/next/README.md Adds cache documentation links.
packages/node/README.md Adds cache documentation links.
packages/openapi/README.md Adds cache documentation links.
packages/opentelemetry/README.md Adds cache documentation links.
packages/pinia-colada/README.md Adds cache documentation links.
packages/pino/README.md Adds cache documentation links.
packages/publisher/README.md Adds cache documentation links.
packages/ratelimit/README.md Adds cache documentation links.
packages/server/README.md Adds cache documentation links.
packages/shared/README.md Adds cache documentation links.
packages/swr/README.md Adds cache documentation links.
packages/tanstack-query/README.md Adds cache documentation links.
packages/trpc/README.md Adds cache documentation links.
packages/valibot/README.md Adds cache documentation links.
packages/zod/README.md Adds cache documentation links.
Review details

Files not reviewed (1)

  • pnpm-lock.yaml: Generated file

Suppressed comments (1)

packages/cache/src/adapters/base-redis.ts:75

  • The entry is overwritten before the script verifies that this caller still owns the lock. If a slow fill exceeds lockTtl and a waiter takes over, the expired holder can finish later and overwrite the newer holder's value. Check the token before writing and skip the store when ownership was lost.
redis.call('DEL', KEYS[1])
redis.call('HSET', KEYS[1], unpack(fields))
  • Files reviewed: 77/78 changed files
  • Comments generated: 5
  • Review effort level: Balanced

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread packages/cache/src/adapters/base-key-value.ts Outdated
Comment thread packages/cache/src/adapters/base-redis.ts Outdated
Comment thread packages/shared/src/object.ts Outdated
Comment thread packages/cache/src/adapters/base-redis.ts Outdated
Comment thread packages/cache/src/middleware.ts Outdated

@pullfrog pullfrog Bot left a comment

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.

ℹ️ No critical issues in this delta — one nit inline. The delta also clears the previously red head CI for the first time.

Reviewed changes

Since the prior pullfrog review at 26f97f95 (commits 7fed718f, ebf06ccf, 2a368bb4):

  • Unified the store serializers — the Redis-family stores plus Memory and Vercel now use one RPCJsonSerializer for keys and outputs with a { json, meta } envelope; BaseRedisCacheStore moved to the new ./base-redis subpath with the scripts, flow, and parseReply shared; Memory/Vercel share BaseKeyValueCacheStore, whose inlined coalesce replaces the now-removed @orpc/shared MemoryLock (clean removal, no stragglers).
  • Centralized expiry mathresolveCacheExpiry in utils.ts now drives expiresAt/evictAt/retention across the Redis, memory, Vercel, and Workers stores.
  • Tightened middleware option resolutionenabled short-circuits before the other resolvers run, and a key resolving to undefined now falls back to [path, input] (both documented; the existing middleware tests still pass).
  • Made the handler plugin inert when idle and fixed the type-check blocker — an early return skips header work when nothing was cached or revalidated, and StandardHeaders now comes from @standard-server/core@~0.9.0 instead of the unresolvable @standardserver/core (this fixes the TS2307 lint failure that was red on the prior head).
  • Head CI is now fully green — the bun cross-adapter cache suite and the node 20/22/24/26 test_matrix pass, and lint_and_typecheck passes. @redis/client is verified to negotiate RESP2 by default (it sends a HELLO without a protover), so its script replies decode Lua false to null and the output !== null miss guard behaves correctly on the default config; the sentinel/guard pair itself is unchanged and would still need hardening before a RESP3-negotiating client is pointed at it.

Pullfrog  | Fix all ➔Fix 👍s ➔View workflow run | Using DeepSeek Flash (free via Pullfrog for OSS) | 𝕏

Comment thread packages/cache/src/adapters/redis.ts Outdated
…on time

A revalidation landing while a fill runs now invalidates what that fill
stores: the memory store snapshots versions before filling and the Redis
family captures them in the script that takes the lock. Entries report
evictAt, so the handler plugin advertises only the remaining
stale-while-revalidate window, and Redis expires entries at that absolute
time. deepSortKeys keeps an own __proto__ key, the header override test is
back, and a failed revalidation is documented as failing the request.

@pullfrog pullfrog Bot left a comment

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.

ℹ️ No new issues in this delta — the fill-time snapshot fix, evictAt reporting, and __proto__ hardening read clean and the previously red head CI is green. One design consequence of the snapshot fix is flagged in the body below for confirmation before merge.

Reviewed changes

This run reviews the delta since the prior pullfrog review at 2a368bb4 — the single commit 5b6652c7:

  • Captured tag versions when a fill starts — the Redis-family FETCH_SCRIPT now snapshots the fill's tags in the same Lua step that takes the lock, and STORE_SCRIPT persists that snapshot, so a revalidation landing mid-fill invalidates what the fill stores. The memory store mirrors it through BaseKeyValueCacheStore.snapshot(), and both gained race tests that deterministically revalidate mid-fill and assert the entry is dropped on the next read.
  • Reported eviction timeCacheEntry gains evictAt; Redis now expires entries with PEXPIREAT at the absolute time instead of a relative PEXPIRE (closing the floored-second mismatch where an entry could be served stale past its intended lifetime), and the handler plugin advertises only the remaining stale-while-revalidate window (evictAt − max(now, expiresAt)) instead of the full configured swr, so reflected headers never outlive the entry.
  • Hardened deepSortKeys — rebuilt on a null-prototype object so a JSON-parsed own __proto__ key survives canonicalization as data (prototype-pollution-safe encodeCacheKey), with a regression test.
  • Restored the handler-plugin override test — "sets its headers over ones already on the response" is back with its preset scaffolding, closing the dead-scaffolding nit.
  • Documented the revalidate fail-loud contract — the Revalidate section now states that a failed revalidation fails the request and that retrying such a request repeats the mutation.
  • Head CI is greentest_matrix (20/22/24/26), lint_and_typecheck, and the Workers builds all pass; the five prior findings (fill-time snapshot, absolute evictAt/PEXPIREAT, remaining-swr headers, __proto__ preservation, restored override test) are implemented by this commit. CodSpeed's lone regression is a json-schema-coercer benchmark flagged with a "different runtime environments detected" disclaimer and no json-schema code is in this PR — it looks like runner noise worth acknowledging on CodSpeed rather than a code regression.

ℹ️ cache + revalidate sharing a tag now guarantees a permanent cache miss

A fill that revalidates one of its own tags captures the version before the bump, so the entry it stores is invalidated by its own request — any procedure combining cache({ tags: ['t'] }) with revalidate({ tags: ['t'] }) recomputes on every call. The renamed combined test asserts exactly this (the handler now runs on every request, where it used to serve the second from cache). The direction is safe and is the correct resolution of the mid-fill-invalidation race — a mutation's own cached response should not outlive the mutation — but the composition silently disables caching and the Revalidate section says nothing about it. One sentence noting that a request whose fill revalidates one of its own tags never caches its output would save a future user from mistaking the throughput regression for a bug.

Pullfrog  | Fix it ➔View workflow run | Using DeepSeek Flash (free via Pullfrog for OSS) | 𝕏

Copilot AI left a comment

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.

🟡 Changes recommended

Redis lock takeover can overwrite newer values, cache keys can collide, and the package lacks the repository-standard publishing hook.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

Files not reviewed (1)

  • pnpm-lock.yaml: Generated file

Suppressed comments (2)

Previously missed (2) — in code that hasn't changed since the last review.

packages/shared/src/http.ts:162

  • An unpaired UTF-16 surrogate matches this branch, but encodeURIComponent throws URIError for it. Such strings can arrive through JSON and are accepted by the tag APIs, so reflecting or purging one can turn an otherwise successful request into an error. Either validate tags before executing the procedure or use a reversible encoding that handles lone surrogates.
    packages/cache/README.md:71
  • This new package README omits the Cloudflare Traces guide, while every other synchronized package README includes it in the same row (for example README.md:71 and packages/cloudflare/README.md:71). Keep the generated package listing consistent.
  • Files reviewed: 77/78 changed files
  • Comments generated: 4
  • Review effort level: Balanced

Comment on lines +79 to +80
"build": "unbuild",
"type:check": "tsc -b"
Comment on lines +82 to +91
redis.call('DEL', KEYS[1])
redis.call('HSET', KEYS[1], unpack(fields))

if evictAt ~= '' then
redis.call('PEXPIREAT', KEYS[1], tonumber(evictAt) * 1000)
end

if redis.call('GET', KEYS[2]) == token then
redis.call('DEL', KEYS[2])
end
Comment on lines +15 to +23
export function encodeCacheKey(key: unknown, serializer: Public<RPCJsonSerializer>): string {
if (typeof key === 'string') {
return key
}

const { json, meta } = serializer.serialize(key)

return stringifyJSON([deepSortKeys(json), meta?.map(entry => stringifyJSON(entry)).sort()])
}
Comment on lines +30 to +34
async fetch(_key: unknown, fill: () => Promise<unknown>, options: CacheFetchOptions = {}): Promise<CacheEntry> {
const { expiresAt, evictAt } = resolveCacheExpiry(options)

return { output: await fill(), tags: options.tags, expiresAt, evictAt }
}

@pullfrog pullfrog Bot left a comment

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.

✅ No new issues in this delta — the NOSCRIPT retry is now bounded to exactly one reload, closing the last open thread.

Reviewed changes

This run reviews the delta since the prior pullfrog review at 5b6652c7 — the single commit 69f8cc89:

  • Bounded the NOSCRIPT script reload to exactly onceRedisCacheStore.run (packages/cache/src/adapters/redis.ts) and BunRedisCacheStore.run (packages/bun/src/redis-cache.ts) dropped the while (true) loop for a single retry: on NOSCRIPT the cached sha is deleted and evalSha runs again (which reloads the script), and a second NOSCRIPT — or any other error — propagates instead of hanging forever. This is precisely the reload-once-then-throw contract the open thread asked for; the JSDoc on both stores now matches.
  • Added real-server regression tests in both packages/cache/src/adapters/redis.test.ts and packages/bun/src/redis-cache.test.ts that seed an unknown sha ('0'.repeat(40)) into the store's scriptShas map and assert the fetch still succeeds after the single reload, across both the read and store scripts. Verified passing against live Redis in the node-26 CI log.

Notes:

  • The lingering RESP3 output !== null miss-guard is unchanged (correct on the default RESP2 config) — still worth hardening eventually, but not part of this delta and not blocking.
  • test_matrix (26) is red, but only on tests/batch/batch-plugin.test.ts:128 (expected 13 to be less than or equal to 10, a Date.now() - start timing assertion in a file this PR does not touch) — a timing flake unrelated to the cache feature; every cache/bun/cloudflare suite passes on node 26 in the same run.

Pullfrog  | View workflow run | Using DeepSeek Flash (free via Pullfrog for OSS) | 𝕏

@pullfrog pullfrog Bot left a comment

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.

✅ No new issues in this delta — the fill-hook refactor is a clean internal simplification with no behavioral change.

Reviewed changes

This run reviews the delta since the prior review at 69f8cc8 — the single commit 468cd88:

  • Replaced the key-value snapshot + write hooks with one fill hookBaseKeyValueCacheStore dropped its TSnapshot generic; subclasses now own the full miss/refresh path via fill(encodedKey, fill, options). MemoryCacheStore.fill captures the tag versions at the top of the method, before await fill(), preserving the fill-start snapshot timing the old split had (a revalidation landing mid-fill still invalidates what the fill stores); the mid-fill race tests in memory.test.ts still pass. VercelCacheStore.fill mirrors its old snapshot-less write exactly.
  • Docs/JSDoc updated to match — the store class doc now says "read entries by their encoded key and fill the missing ones", and the entry-field comment notes the tag versions "each had when the fill started".

Verified: memory (18) and middleware (21) suites pass locally; head CI is green on every node plus lint/typecheck/builds.

Pullfrog  | View workflow run | Using DeepSeek Flash (free via Pullfrog for OSS) | 𝕏

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Caching

2 participants