Repository navigation
Back the use cache directive with Harper and persist per-entry cache lives - #65
Open
jjohnson-hdb wants to merge 29 commits into
Open
jjohnson-hdb wants to merge 29 commits into
jjohnson-hdb wants to merge 29 commits into
Conversation
Next keeps cache lives in `SharedCacheControls` — a per-process static Map plus the build-time prerender manifest — and neither replicates. An entry written on one node reaches every other node, but its `revalidate`/`expire` do not, so `calculateRevalidate` falls back to its 1-second default there and the entry looks stale on every read. Persisting `cacheControl` alongside the entry is what makes cache lives coherent across the cluster, so this is a correctness fix for the distributed story rather than fidelity to a dropped field. `get` now also withholds an entry past its own `expire`, and `revalidateTag` forwards the `durations` parameter it previously ignored. On tag invalidation the handler used to return null, which Next reads as a miss and turns into a blocking render — the worst possible response to an invalidation storm. Invalidations are now mirrored into Next's own tags manifest, so `areTagsStale` classifies affected APP_PAGE/APP_ROUTE entries as stale and Next serves them while regenerating in the background. Entries Next will not tag-recheck (FETCH) are still withheld, because they would otherwise be served indefinitely, and an on-demand `revalidatedTags` hit still forces fresh content. Invalidation state moves to a shared module so the forthcoming "use cache" handler observes the same map, subscription and tombstones — a `revalidateTag` has to reach both caches or the cluster can only be half purged. The sweep that module adds turns the tombstone TTL from a correctness parameter into a safety margin: a tag invalidation is soft-applied immediately, then a throttled background pass invalidates matching entries and drops the tombstone. Entries are invalidated, never deleted, so stale-while-revalidate survives. Chunked with bounded concurrency and jittered pauses to keep transactions short. Next's implicit `_N_T_` route tags are never swept — `_N_T_/layout` is carried by every page, and `tags` cannot be indexed (Harper cannot index array elements, and `contains` cannot use an index anyway), so sweeping one would scan and rewrite the whole cache.
`cacheHandler` and `cacheHandlers` are two different Next.js interfaces. The plugin implemented only
the first, which backs ISR, the Data Cache and `unstable_cache`. The `'use cache'` directive goes
through the second, and with it unset Next falls back to `createDefaultCacheHandler` — an in-memory
LRU. Every `'use cache'` entry was therefore per-worker, lost on restart, and invisible to the rest
of the cluster, while appearing to work.
Adds a handler implementing that interface over a new `nextjs_use_cache` table, sharing the
invalidation map, subscription and tombstones with the ISR handler so one `revalidateTag` reaches
both caches.
Four details the interface demands, each covered by a test:
- A ReadableStream is single-use, so `set` drains it to bytes and every `get` builds a fresh stream.
Returning a shared stream would serve the first reader and hand every later one an empty body.
- A value stream can error with partial data, so a failed drain persists nothing rather than caching
half a render.
- A `get` arriving before a pending `set` completes must wait rather than report a miss, so in-flight
entries are registered synchronously ahead of the first await.
- `getExpiration` consistently reports the newest invalidation timestamp (0 when none), rather than
mixing that with the `Infinity` "check the soft tags in get instead" protocol.
Per-entry cache lives come free here: the interface carries `stale`, `revalidate` and `expire` on the
entry, so they round-trip without the separate plumbing the legacy handler needed. On invalidation an
entry inside its expire window is returned backdated past its revalidate window — stale, so Next
regenerates, but still served meanwhile.
`refreshTags()` is a no-op beyond ensuring the subscription is live. The interface expects handlers to
poll a tags service; Harper replicates invalidations and pushes them to every worker, so the map is
already current.
Registration is opt-in via `withHarper(config, { useCache: true, configDir })` for one minor, since
enabling it silently changes caching behaviour for existing apps, and the interface only exists in
Next.js 16.
The repo had no fixture exercising `cacheComponents` or `'use cache'`, which is why the gap went unnoticed: every existing test covers the incremental-cache path, and that path worked. The new fixture asserts a row actually lands in `nextjs_use_cache`, and repeats a read across workers — a per-worker in-memory cache shows up there as the value changing between requests, which is the assertion that would have caught the original behaviour. README now leads the caching section with the two-interface distinction, since configuring only `cacheHandler` silently leaves `'use cache'` in Next's in-memory handler. Also documents per-entry cache lives and why they matter for a distributed cache, the sweep, the two deliberate sweep limits (implicit route tags, unindexable tags), and that a table's `expiration` does not migrate when the schema changes. AGENTS.md records two things that cost time here: integration tests need the macOS loopback alias pool and fail at startup without it, and the `format:fix` script the Code Style section references does not exist.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces support for Next.js 16's 'use cache' directive by implementing a new Harper-backed UseCacheHandler, updating the database schema with a dedicated nextjs_use_cache table, and refactoring invalidation logic into a shared background sweep utility. It also updates the withHarper configuration helper to allow opting into this feature. The review feedback highlights several important improvements: ensuring we do not forward-date entries during invalidation by using Math.min, wrapping asynchronous blob reading in a try/catch block to prevent unhandled rejections, gracefully ignoring MODULE_NOT_FOUND errors to maintain compatibility with Next.js 14, and deduplicating tags in recordInvalidation to prevent redundant database writes.
The "use cache" handler never wrote anything to Harper. Next derives entry timestamps from `performance.timeOrigin + performance.now()`, which is fractional, and the `timestamp` column is a Long — so Harper refused every write with "must be an integer". The failure was caught and logged, so the cache looked like it was working while storing nothing. Timestamps and cache lives are now floored on the way in, on both handlers, since `revalidate`/`expire` are Int columns with the same exposure. The unit tests missed this because their mock table accepted any value. The mocks now enforce the integer columns, which is what makes the two new regression tests meaningful. A silent `getTable()` miss is the other half of why this hid: it returned undefined with no signal, turning every write into a no-op. It now logs once. The background sweep is disabled by default behind HARPER_NEXTJS_EXPERIMENTAL_SWEEP. It does not coexist with Next's staleness model, in two ways that are both silent: - Harper's `invalidate()` on a table with no `sourcedFrom` leaves the record awaiting a refresh that never arrives, and every later read of that key blocks — this broke the pre-existing next-16-caching revalidateTag test with a 30s timeout. - Writing an explicit marker with `patch` instead bumps `lastModified` (`@updatedTime`), so the entry looks newer than the invalidation to Next's own `areTagsStale`. Next then treats it as fresh and never regenerates, serving the entry stale indefinitely. Marking an entry stale without disturbing the timestamp Next derives staleness from needs a primitive this schema does not have. Until then the tombstone remains the invalidation — the behaviour that shipped previously — and its 7-day expiry bounds table growth on its own. The sweep, its throttling, its race guard and its scope limits stay covered by tests so the work is not lost. The `revalidateTag` integration test asserted the tombstone row still existed, which races the sweep whenever it is enabled, and demanded regeneration on the very next request, which contradicts serving stale first. It now asserts the invalidation is durably recorded in either table and polls for the content to regenerate — the effect rather than the mechanism. Invalidation hydration is extracted so the crash window has a test: a worker that dies between `revalidateTag` and regeneration must rebuild its map from the tombstone rather than come back treating the stale entry as fresh. The use-cache fixture page was fully prerendered, so its cached boundary was resolved during `next build` and the handler was never called at request time — the test could not have failed for the right reason. It now reads cookies, which keeps the route dynamic and exercises the runtime path.
Adds a purpose-built exercise rig and a cluster suite that stands up two replicating Harper Pro nodes in Docker, so the claims this plugin exists to make are measured rather than reasoned about. Eight assertions, all passing: replication is live, the handlers run at request time, a "use cache" entry written on node A is served unchanged from node B, cache lives replicate, the Blob survives replication, revalidateTag fans out A to B, entries and tombstones survive a restart, and Harper REST resources remain reachable alongside Next. The rig bakes the generating node into the cached value and renders the serving node dynamically, so a cross-node hit is visible rather than inferred: node B reporting `served-by=nodeB` alongside `cached-by=nodeA` is the cross-cluster read. It also asserts the absence of `x-nextjs-prerender` and a growing row count, because the previous fixture was fully prerendered and passed for the wrong reason. Corrects the per-entry cache lives claim, which was wrong in the direction that flattered it. The SharedCacheControls 1-second fallback is not what breaks: a FETCH entry carries its own revalidate and route cache lives ship with the prerender manifest, and the legacy path measures 0/10 regenerations without the columns. The real failure is on the "use cache" handler, where cache lives travel on the entry — stored without them the row reads back as revalidate 0, Next treats it as immediately stale, and the entry regenerates on every single read. Measured 10/10 against 0/10 with the fix, on the same cluster. Also corrects the sweep rationale. `invalidate()` does not block reads; it is a no-op. Probed directly on 5.1.23 and 5.2.0, against a plain table and a sourcedFrom one, the record reads back unchanged and the source is never re-invoked. That makes the earlier reading wrong and the actual problem worse: a sweep built on it drops the tombstone while leaving every entry untouched, losing the invalidation outright. The sweep stays off by default, now for the right reason. Documents the ordering requirement that `rest` and `jsResource` be declared before the plugin. The plugin's handler claims every path, so a resource declared after it answers with Next's 404 or its 308 trailing-slash redirect and nothing explains why.
Four findings from the gemini-code-assist review on PR #65, all verified against the code before applying: - UseCacheHandler.get: clamp the invalidation backdate with Math.min(timestamp, staleAt). An entry already older than the revalidate window was forward-dated to staleAt, extending how long Next kept serving it stale. The real-expiry guard above the block means this never served a genuinely expired entry, so the impact was narrower than reported, but the clamp is strictly correct and free. - toBuffer: wrap blob.arrayBuffer() in try/catch. get() has no surrounding try, so a rejecting read (corrupt row, IO error) surfaced as a request error rather than degrading to a cache miss. - mirrorToNextTagsManifest: ignore MODULE_NOT_FOUND. Next 14 has no tags-manifest.external.js, so the legacy handler logged an error on every invalidation there. - recordInvalidation: deduplicate tags. Duplicates cost a redundant put each and, more expensively, schedule a duplicate sweep — and sweeps scan, because tags cannot be indexed. Separately, the fractional-timestamp test pinned an epoch literal that was "now" when it was written. With the fixture's default expire of 3600s the entry expired an hour later and get() correctly returned undefined, so the test passed once and would fail in CI from then on. It now offsets from Date.now() and keeps the fractional component, which is the thing under test. Verified: 63 unit, 5 next-16-use-cache, 4 next-16-caching, 8 cluster (2-node Docker) — all passing.
Next imposes no size limit on a "use cache" key — it is composed from the cached component's serializable props, so a component handed a large prop produces a correspondingly large key. Harper caps a primary key at MAX_KEY_BYTES = 1978 (harper/resources/Table.ts:146). Observed in a real app at 87,465 bytes: an AEM component model passed as a prop, one of whose items embeds HTML with inlined CSS. Harper answered `Primary key size is too large: 87465`, the cache boundary rejected, and the streamed subtree aborted — a dead page. The failure is invisible from the server side, because it surfaces as a Next streaming rejection rather than anything Harper logs. Seven per render, and the page rendered fine on a cold cache, so a single-fetch smoke test never sees it. toStorageKey() truncates an oversized key on a UTF-8 character boundary and appends a hash of the FULL key. Short keys pass through untouched, deliberately: hashing unconditionally would change every key and silently invalidate the whole cache on upgrade, and it discards a readable id for nothing. The untruncated key is kept in a non-indexed `cacheKey` column so a hashed row is still identifiable. Applied at BOTH table access points. A key-mangling function is only correct if every access applies it — a get that skips it misses forever, silently — so a test asserts no raw key ever reaches the table. The test mock now enforces the 1978-byte limit the way Harper does. A mock more permissive than production is how an 87KB key reached a cluster in the first place; the same class of gap previously let a no-op invalidate() pass. Verified: 71 unit (4 of the new tests fail with toStorageKey neutralised), 5 next-16-use-cache, 4 next-16-caching, 8 cluster.
A missing trailing newline had merged a new line into it, leaving `playwright-report/test-results/` and dropping `playwright-report/` from the ignore list. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…tlive entries
revalidateTag(tag, 'max') was stored as a single `now + expire` timestamp and
used as the stale time, so for a year every entry with the tag — including ones
regenerated after the invalidation — regenerated on every read ('use cache') or
missed on every read (fetch cache). Reproduced against dist/ and in the TSC app.
Tags now carry Next's own `{stale, expired}`: stale now, expired later with a
profile; expired now without one. The 'use cache' handler signals stale with
`revalidate: -1` as Next's default handler does, instead of backdating, and
getExpiration reports only expirations that have passed. The ISR handler hands
tag-stale entries to Next 16, which serves them stale from its tags manifest,
and withholds them elsewhere.
Tombstones are written with a per-record Harper expiry of the longest
cache-table expiration plus an hour. Harper caps every entry at its table
expiration from its last write, so an invalidated entry now always lapses
before its tombstone and cannot come back — whatever the TTLs are set to. This
replaces the disabled marker sweep (patch reset TTLs, created ghost rows on
deleted records, and could not signal pages stale): an immediate expiry is
swept by deleting entries through a new `tags` index (exact element match;
`contains` was a substring scan that also matched `posts-archive` for `posts`).
The sweep only reclaims space; reads never depend on it.
Also:
- Next 15's tags manifest holds numbers, not objects; mirror by shape.
- Subscribe before hydrating, share one init attempt, back off on failure.
- Stream 'use cache' Blobs instead of arrayBuffer() plus two copies; decide
expiry before touching the blob.
- Store the full key only when the id was truncated; log the storage key.
- Legacy tombstones dated in the future (the old 'max' encoding) no longer
outrank newer invalidations of the same tag.
- Fall back to an exact-match scan while Harper reports the tags index as
rebuilding: on 5.2.0 worker threads other than the one that ran the backfill
kept doing so after it finished, until restart.
- One revalidateTag reaches both handlers; record and sweep it once.
Verified: 94 unit, 42 integration (incl. 2-node Docker cluster). E2E in the TSC
homepage app on Harper 5.2.0 in Docker with real AEM content: 28/28 cases on a
fresh volume and 28/28 on a volume upgraded from ff7e895 (which scores 17/28 on
the same harness).
Not cross-model reviewed: codex and agy are not installed on this machine and
Gemini returned 503 throughout; a same-family adversarial review was done on
the design, which is advisory only.
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…at the table TTL
Both handlers stored Next's `expire` only as a column and left the record's
Harper lifetime to the table default, so an entry Next considered dead (e.g. a
cacheLife('minutes') entry after an hour) stayed stored for the full 7 days.
The tombstone guarantee — no invalidated entry outlives its tombstone — also
only held because nothing happened to set a per-record expiry; it was assumed,
not enforced.
Each write now passes `expiresAt` = write time + Next's `expire`, capped at the
table's configured `expiration`, which is the same bound tombstone lifetimes
are sized from. With no usable `expire` the table default still applies.
Longer entry lifetimes come from raising the table's expiration; tombstones
follow it because their lifetime is read from the live table.
Verified: 99 unit; 42 integration, including a new check that a
cacheLife('hours') row's Harper `$expiresAt` equals its timestamp plus Next's
expire.
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
jjohnson-hdb
marked this pull request as ready for review
September 29, 2026 15:44
…uilds' "use cache" entries Entry lifetimes were capped at the cache tables' 7-day `expiration`, so the `weeks` and `max` cacheLife profiles were evicted before (or just as) they turned stale and never got their stale-while-revalidate window. The cap is now its own constant, one year (Next's `max` expire), independent of the tables' `expiration`: Harper honours a record's own `expiresAt` past the table default, so no table migration is needed. Tombstones are sized from the cap and now live a year plus the margin, or longer if a cache table's expiration is. Cache lives are stored bounded to 0..1 year. Next's `default` profile sets `expire` to INFINITE_CACHE (0xfffffffe), outside Harper's 32-bit Int range, so every such "use cache" write was refused — caught and logged, so the entry silently never landed and was re-rendered on every request. Next puts the build ID in every "use cache" key, so each deploy leaves the previous build's entries unreadable, and with a one-year cap they would stay a year. Setting HARPER_NEXTJS_SWEEP_OLD_BUILDS=true enables a sweep: five minutes after start (so a rolling restart reaches the other nodes first), worker 0 on each node deletes entries whose key names a build that is neither its own nor the latest successful build of any app in nextjs_build_info. Keys that cannot be attributed to a build, and `development`, are kept. Off by default. Verified: 117 unit; 11 integration (next-16-use-cache, next-16-caching), run before the opt-in flag and the lower bound on stored lives were added; and the TSC homepage on a local Harper 5.2.13 amd64 container with 13 threads. With the flag unset, seeded old-build rows survived past the delay. With it set, one worker (http/1) deleted exactly the three rows from old or failed builds and kept the current build's 13, another app's current build, `development`, and a FormData-shaped key. Real rows' $expiresAt equals timestamp + expire. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…r-and-per-entry-ttl Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Ethan-Arrowood
requested changes
Oct 1, 2026
Ethan-Arrowood
left a comment
Ethan-Arrowood
left a comment
Member
There was a problem hiding this comment.
looks good. agent has some recommended changes before we merge this.
🤖 Reviewed with Codex
Member
|
wait it didn't actually post its comments. 🤦♂️ |
Ethan-Arrowood
self-requested a review
October 1, 2026 19:13
Ethan-Arrowood
requested changes
Oct 5, 2026
… subscription ends
- Reads in both cache handlers treat an entry as a miss until this worker's
invalidation view is complete, instead of serving it unchecked after a
failed or backing-off hydration.
- A subscription that ends ('close', or 'error') is dropped, so the next
read subscribes again and re-hydrates what it missed.
- Tombstone writes carry their issue time as the Harper write version, so an
older invalidation that commits late is dropped rather than replacing a
newer tombstone. Versions are strictly increasing per worker so a same-ms
pair does not tie.
- A use-cache read waiting on an in-flight set no longer rejects when that
set fails; it falls through to storage.
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…ests Derived from the review findings rather than from the fix, and verified to discriminate: with the four behavioural fixes in cd4e2d8 reverted one line at a time, each of these five fails for its own reason; with them in place all pass. - both handlers report a miss while the tombstone scan has failed, instead of serving an entry another node invalidated (CacheHandler, UseCacheHandler) - a subscription reported as errored — not only Harper's own 'close' — drops the view, so the next read re-subscribes and reloads what it missed - an older concurrent invalidation whose put commits last does not replace the newer tombstone, asserted on the row a restarting worker hydrates rather than on the arguments the put was called with - a get awaiting an in-flight set falls through to storage when that set fails Each is paired with a guard that a fix cannot satisfy by withholding everything or by never recovering: an uninvalidated entry still serves, and a read that failed closed recovers once the scan succeeds. The invalidation-table fake now keeps the row a write would leave in storage and applies Harper's write-version rule — a write whose context.timestamp precedes the stored version loses to it (harper/resources/Table.ts:4770). That is what makes the concurrent-write outcome observable at all; asserting only that the put carries a version would not have caught an older write winning. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…ves open `recordInvalidation` nudges each write's Harper version past the last so two invalidations issued in one millisecond cannot tie in storage, but that version never reaches the row: the `timestamp` column stays the whole millisecond and subscription events carry no version. `noteInvalidation` orders by that integer and lets an equal `at` overwrite, so where the two orderings disagree the older view wins. The reachable path is start-up: the subscription is live before the hydration scan, so a newer hard expiry delivered during the scan is overwritten by the older profiled row the scan returns. An entry the hard expiry had to withhold goes back to being served stale, and initialization reports ready. This test fails. Carrying a comparable version through the row, the hydration and the events would fix it, as would refusing to weaken a hard expiry on an equal-time merge. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
| merged.map(({ tag, invalidation }) => | ||
| tombstones.put( | ||
| tag, | ||
| { timestamp: now, stale: invalidation.stale, expired: invalidation.expired, lapsesAt: invalidation.lapsesAt }, |
Member
There was a problem hiding this comment.
major — The new write version distinguishes same-millisecond invalidations only in Harper’s write context; the row still carries timestamp: now. Hydration and subscription events reconstruct at from that row timestamp, and noteInvalidation accepts an equal at. If an older profiled tombstone (expired: 61000) is captured by the hydration scan, then a newer hard-expiry event (expired: 1000) arrives before that row is applied, the scan overwrites the hard expiry. The worker reports ready and serves pre-invalidation content as stale instead of withholding it. I reproduced this against the current module, including the stale field preserved by the real hard-expiry merge; the newly added regression test exercises the same sequence. Carry comparable ordering information through stored rows and subscription payloads, and use it when adopting invalidations, with explicit tie handling across workers. The write-context version alone cannot protect this read-side overlap.
| - Run `npm run test:integration` to run all tests, or `npm run test:integration -- integrationTests/next-15.pw.ts` for a single file. | ||
| - Test startup is slow by design — each test file starts a real Harper instance and waits for Next.js to build (up to 2 minutes). A slow start is not a failure. | ||
| - The ISR cache tests in `integrationTests/next-16.pw.ts` are intentionally skipped; `CacheHandler.cts` is a work in progress. | ||
| - Integration tests need the macOS loopback alias pool (127.0.0.2+). Without it every fixture fails at startup with `LoopbackAddressValidationError` / `EADDRNOTAVAIL` before any assertion runs — that is a machine-setup gap, not a test failure. `ifconfig lo0 | grep 'inet '` shows what is configured. |
Member
There was a problem hiding this comment.
If we are going to detail this; lets also detail that the @harperfast/integration-testing module has a loopback alias script included in it.
…eap fix
`toStorageKey` derives both its truncated prefix and its digest through UTF-8,
which collapses every lone surrogate to U+FFFD. Two oversized keys differing
only there produce an identical storage key, and `get` never checks which key
the row it found belongs to, so one cache boundary is served another's bytes.
Verified directly: `toStorageKey('x'.repeat(2000) + '\uD800')` equals
`toStorageKey('x'.repeat(2000) + '\uD801')`.
The second test is the one that matters for choosing the fix. Comparing a
stored `cacheKey` column against the incoming key — the obvious one-liner —
cannot work here: Harper encodes records with msgpackr, which does not preserve
a lone surrogate, so the stored key never equals the key that wrote it and the
comparison turns every such read into a permanent miss. The digest has to
distinguish the keys instead, by hashing UTF-16 code units rather than their
UTF-8 encoding.
So the use-cache table fake now stores strings the way Harper does. Checked
against `harper`'s own msgpackr: for a string long enough to take its
Buffer.write path — every key this module truncates — the round trip is exactly
a UTF-8 round trip. Storing JS strings verbatim would have let the cheap fix
look correct.
The collision test fails; the round-trip guard passes and must keep passing.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
… reviewer Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
No workflow ran `npm test`. The only test invocation in CI was `npm run test:integration`, so the 138 unit tests — including every regression test on this branch — were never executed by CI, and a red unit suite showed up as a green PR. A separate workflow rather than a job in integration-tests.yml: that file is also a reusable `workflow_call` consumed by HarperFast/harper, so a job added there would run for the caller too. The unit suite needs no Harper instance, fixtures or browsers, so it is a seconds-long check on the same Node matrix. This job is RED on this branch by design. Two committed tests pin defects that are still open — the same-millisecond invalidation ordering gap and the lone-surrogate cache-key collision. Both have a fix described in their test comments. Also corrects AGENTS.md, which claimed CI was disabled via `if: false`. That flag was removed in a52c3c7; the integration workflow has been running on every PR since. The unit suite was undocumented there too. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Ethan-Arrowood
requested changes
Oct 5, 2026
…urrogate Next composes a "use cache" key with `encodeReply`, whose FormData/binary encoding can leave lone surrogates in the string. `toStorageKey` derived both the truncated prefix and the digest through UTF-8, which collapses every lone surrogate to U+FFFD, so two keys differing only there produced the same storage key — and `get` never checks which key a row belongs to, so one cache boundary's bytes were served for another's key. The prefix has to go through UTF-8 (two nodes must derive the same key, and Harper stores the id as UTF-8 either way), so the digest is what has to tell them apart: it is now taken over UTF-16 code units, which keeps every code unit verbatim. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…rrival order `recordInvalidation` nudges each tombstone write's Harper version so two invalidations of one tag issued in the same millisecond cannot tie on storage, but that version lives only in the write context: the row's own `timestamp` stays the whole millisecond and subscription events carry no version at all. So `noteInvalidation` cannot order such a pair by `at`, and letting an equal `at` overwrite made arrival order decide — which differs per worker. A hard expiry delivered on the subscription was downgraded to stale by the profiled row the start-up scan applied second, and the worker served what it had to withhold. A tie is now resolved by taking the stricter of the two: the earliest `stale` and the earliest `expired`. That withholds the most, and it is the same answer whichever arrived first. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Member
|
@jjohnson-hdb you don't have to commit to this anymore unless you want to. I'm going to make some changes, get CI running and passing, and then merge and release. I'll notify you when its done. Might not finish today so keep an eye on slack tomorrow or something |
…of pinning it a year
Next's `default` cacheLife profile sets `expire` to `INFINITE_CACHE`
(0xfffffffe), and that is what every `'use cache'` boundary with no
`cacheLife()` gets — the common case, not an edge one. `toStoredLife` bounds it
to a year so it fits the Int column, and `entryExpiresAt` was then writing that
bounded year as the record's Harper expiry, overriding the cache tables' 7-day
`expiration` for the default case.
Next puts the build ID in every `'use cache'` key, so entries from an earlier
build can never be read again. Pinning the default case for a year meant every
deploy left a full generation of unreadable blobs behind for a year, with the
sweep that reclaims them opt-in and off by default.
`entryExpiresAt` now returns undefined for a life longer than the year it caps
at, which is only reachable from an unbounded `expire`, so those rows follow the
table's expiration. A definite life is still honoured exactly: `cacheLife('max')`
asks for 31_536_000 and gets its year, and `weeks` still outlives the table TTL.
The handlers pass the raw `expire` rather than the stored one, since
`toStoredLife` has already erased the difference.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…d in CI Docker was a hard dependency: the rig spawned `docker` directly. On macOS `docker` is usually a *shell alias* for podman, which a direct spawn never sees, so the suite reported "Docker is not available" and skipped itself on machines that could have run it perfectly well. The runtime is now probed — `docker`, then `podman`, with `HARPER_CONTAINER_CLI` to force one. Default image moves from `harper-pro-openshift:5.0.26` to `harper-pro:5.3.1`, and a missing image is pulled rather than being a precondition that silently skips the suite. A skip is how this suite came to be green everywhere and exercised nowhere, so `HARPER_CLUSTER_REQUIRED=1` turns "cannot run here" into a failure. CI sets it, and gets 30 minutes instead of 15 — the rig stages by running `npm install` and `next build` inside the image before any test runs. Two startup races surfaced once the suite actually ran: - `add_node` fired as soon as the operations API answered, but the replication listener on 9933 binds later, failing with "connect ECONNREFUSED <ip>:9933 and connection was required to sign certificate". It is now retried, and only for that failure — bad credentials or an OSS image still surface at once. - `restartNode` waited on the operations API plus a fixed 5s sleep, then the test fetched a page. Harper binds 9925 well before Next.js is up on 9926, so under load the request was reset mid-flight. Both restart and start-up now wait for the app itself to answer. `operation()` also reports the response body; a failed handshake was a bare 500. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…led ifconfig `@harperfast/integration-testing` ships `harper-integration-test-setup-loopback` and a launchd plist that re-adds the aliases at boot, which the loopback note did not mention. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
… user The Pro image runs as `harperdb` (uid 1000). A Linux bind mount preserves the host's ownership, so on a CI runner (uid 1001) the staged rig arrives owned by someone else and the in-container `npm install` dies part-way through reify with "The operation was rejected by your operating system". It failed identically on Node 22, 24 and 26. macOS hid this completely: virtiofs presents the mount as the container's own user, so the suite passed locally while being unrunnable in CI. The staged tree is now opened up (`chmod -R a+rwX`) before the build container runs and again afterwards, so the host can clean it up and the node containers can write into `.next`. npm's cache and logs are pointed at /tmp rather than a home directory the container user may not own on that mount. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
… by then The post-build `chmod` ran on the host, but by that point `npm install` and `next build` had rewritten the tree as the container's uid — and only an owner may chmod. CI got past the original permission error and then failed on `Command failed: chmod -R a+rwX /tmp/harper-nextjs-cluster-rig`. Ownership changes hands halfway through staging, so each side now opens up what it owns: the host before the build, the image after it. The second pass is what lets the host delete the tree on a re-stage. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Keeping ownership uniform is what makes the staged tree manageable. Left to itself the image runs as `harperdb` (uid 1000) while a Linux bind mount keeps the host's ownership, so on a CI runner (uid 1001) `npm install` could not write. Opening the tree up first was not enough: the container's own writes then landed as uid 1000, leaving a tree neither side owned outright — and since only an owner may chmod, neither the host pass nor an in-container pass could cover all of it. CI failed on each in turn. With `--user` pinned to the host's uid everything in the tree belongs to one user, so a single host-side chmod afterwards opens it to the node containers, and the host can still delete it on a re-stage. Verified under podman on macOS, where the host sees its own uid on the container's writes. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
kriszyp
approved these changes
Oct 5, 2026
kriszyp
left a comment
kriszyp
left a comment
Member
There was a problem hiding this comment.
This sounds great to me.
🤖 Reviewed with Codex
| const merged = uniqueTags.map((tag) => { | ||
| // Like Next's own `{...existing, stale, expired}`: a later invalidation keeps the parts of an | ||
| // earlier one it does not replace. | ||
| const existing = cacheInvalidations.get(tag); |
Member
There was a problem hiding this comment.
A worker merges with only its local cacheInvalidations view. For example, A hard-expires a tag at t=1000; before that event replicates, B issues a profiled invalidation at t=2000. B writes {stale: 2000, expired: 62000} without A’s expired: 1000. That newer tombstone replaces A’s row, and noteInvalidation discards A’s older event if it arrives later. An entry written before t=1000 is then served stale until t=62000 instead of withheld. The single-worker merge test cannot reach this boundary. Please make the persisted merge safe across workers, such as with an atomic compare-and-merge or immutable invalidation events, and check this sequence with replication delayed.
| * Harper keeps the existing record on a tie, which would drop a second invalidation issued in the same | ||
| * millisecond — and that one carries the merged, newer view of the tag. | ||
| */ | ||
| function nextTombstoneVersion(now: number): number { |
Member
There was a problem hiding this comment.
lastTombstoneVersion is local to one worker. Two workers issuing different invalidations of the same tag in the same millisecond both use now as their first write version; Harper keeps the existing row on a tie, so a softer row can permanently win over a hard expiry. stricterOf protects a live worker’s map but cannot recover the missing row on restart. Please give cross-worker writes a tie-safe ordering or merge strategy. A focused check is to issue a profiled invalidation and a hard expiry from isolated workers at the same clock millisecond, then hydrate a fresh worker and assert the hard expiry remains.
| const staging = join(tmpdir(), 'harper-nextjs-cluster-rig'); | ||
| const fixture = join(pluginRoot, 'fixtures', 'next-16-cluster'); | ||
| const stamp = join(staging, '.stamp'); | ||
| const inputs = ['config.yaml', 'next.config.mjs', 'package.json', 'probe.js', 'schema.graphql'] |
Member
There was a problem hiding this comment.
The reuse stamp omits fixtures/next-16-cluster/app/** and records the plugin handler only by string length. Changing a rig page, or changing handler code without changing its emitted length, leaves the stamp equal and reuses the old .next build and vendored plugin. A rerun can therefore pass against stale code. Please fingerprint all staged fixture inputs and the relevant built plugin contents; a same-length edit followed by stageRig should force restaging.
Ethan-Arrowood
self-requested a review
October 6, 2026 22:09
…reuse a stale build The reuse stamp read five fixture files by name and recorded the plugin by nothing but the byte length of `dist/UseCacheHandler.cjs`. So a changed rig page under `app/**` — the pages the assertions actually read — left the stamp equal, as did any edit to `dist/cacheInvalidation.cjs` or `dist/CacheHandler.cjs`, which it never looked at at all. A rerun could reuse the previous `.next` and the previous vendored plugin and report a pass against code it never built. The stamp is now a sha256 over the full contents of every staged input: the fixture tree (minus generated directories) and the built plugin. Verified that a changed rig page, a same-length handler edit, and an edit to either of the other two emitted handlers each force a restage, while an unchanged tree still reuses — cold 1.4m, warm 23s. Reported-by: kriszyp Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…on cannot revive entries `expired` is one scalar carrying two different claims — "everything before T is dead now" and "everything before T will be dead at T" — and a later invalidation replaces it wholesale. So `revalidateTag(tag)` followed by `revalidateTag(tag, 'max')` moved `expired` into the future, and entries the first call had withheld came back as merely stale for the profile's whole window. Reproduced on a single worker: no race needed, contrary to how this was first reported. Invalidations now carry `hardExpiredAt`, the newest expiry of the tag that has already passed. It only moves forward, `tagState` withholds anything written before it, `passedExpiration` reports it, and it is persisted on the tombstone so a restarted worker keeps it. It is also absorbed from an invalidation that arrives *older* than the view already held, which is how a worker learns of another node's immediate expiry after adopting a newer profiled one. Three of the new tests fail without the fix; two guard the other direction — an entry written after the hard expiry is still servable, and a newer hard expiry still moves the mark forward. This does not close the cross-worker write race: each tag still has one tombstone row written from one worker's view, so two workers invalidating the same tag within the replication window can still lose the immediate expiry from storage. That needs an atomic merge or an append-only log; it is written up in the README and tracked separately. Reported-by: kriszyp Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Adds a second Next.js cache handler so the
'use cache'directive is backed by Harper instead of Next.js's per-worker in-memory default. It also persists each entry's cache lives, and reworks tag invalidation to follow Next.js's own stale/expired semantics while never letting an invalidated entry come back.Next.js has two cache-handler interfaces, configured separately.
cacheHandlerbacks ISR, the Data Cache andunstable_cache, and this plugin already implemented it.cacheHandlersbacks'use cache'. The plugin never registered it, so those entries fell through to Next's in-memory LRU: per-worker, lost on restart, and invisible to the rest of the cluster.For the human reviewer
revalidateTag(tag)now expires immediately. The next request blocks on a fresh render, which is what Next.js 16 does. Earlier drafts of this PR served stale content here instead. To get stale-while-revalidate, callrevalidateTag(tag, 'max')or pass another profile. Worth checking against callers that relied on the earlier drafts' behaviour.expire, capped at its table's configuredexpiration(entryExpiresAt). Each tombstone gets its own Harper expiry: the longer of the two cache tables' configured expirations, read at runtime, plus an hour (tombstoneLifetimeMs). An invalidated entry therefore lapses first, whatever the tables' TTLs are set to. Entries meant to live longer than 7 days need the table'sexpirationraised, and tombstones follow it. This replaces the earlier plan of patching stale markers onto records. That plan broke in four ways:patchresets a record's TTL;lastModified: -1forces a blocking render.tagsindex, which Harper builds per array element;equalsgives an exact-match lookup. The oldcontainscomparator was a full scan doing a substring match, so sweepingpostsalso matchedposts-archive. The sweep only reclaims space; reads never depend on it. There's no longer an experimental flag.Changes
src/UseCacheHandler.cts'use cache'handler. It drains the entry's stream onsetand refuses to store a partial or errored render.getstreams the stored Blob instead of reading it whole witharrayBuffer(), and decides expiry before opening it. A tag-stale entry is returned withrevalidate: -1, as Next's own handler does.getExpirationreports only expirations that have already passed. Keys over 1500 bytes are truncated and suffixed with a hash; the full key is kept only then. Asetin flight is registered synchronously, so a concurrentgetwaits for it. Each row's Harper expiry is itsexpire, capped at the table's.src/cacheInvalidation.cts{stale, expired}plus a per-record expiry, held in a per-worker map that the subscription keeps current. The copy into Next's tag list detects the manifest's shape: Next 16 stores objects, Next 15 numbers, Next 14 has none. Initialisation subscribes before reading tombstones back, shares one attempt between callers, and backs off on failure. Legacy tombstones (a baretimestamp) count as an immediate expiry and are dated no later than now. The delete-only sweep is throttled. OnerevalidateTagreaching both handlers is recorded once.src/CacheHandler.ctsrevalidate/expire. Uses the shared tag state, including soft tags: an expired entry is withheld, and a stale one is handed to Next 16 for FETCH/APP_PAGE/APP_ROUTE and withheld otherwise. A row with nodatais treated as a miss. Each row's Harper expiry iscacheControl.expire, capped at the table's. ForwardsrevalidateTagdurations.src/withHarper.ctsuseCacheoption that registerscacheHandlers, plususeCacheHandlerPath(). It's opt-in because it changes where existing apps''use cache'entries are stored.schema.graphqlnextjs_use_cache. Addsrevalidate/expiretonextjs_isr_cache. Indexestagson both cache tables, and drops the indexes only the old scan used. Addsstale/expired/lapsesAttonextjs_cache_invalidation.README.mdrevalidateTag/revalidatePathform does what, streaming and oversized keys, how invalidation and the sweep work, the schema, and upgrading.AGENTS.md.gitignoreplaywright-report/, which a missing newline had merged into another line.Tests and fixtures, all covered in Verification:
cacheInvalidation.test.ts,CacheHandler.test.ts,UseCacheHandler.test.ts,withHarper.test.ts;next-16-use-cache.pw.ts,next-16-caching.pw.ts, and the two-node cluster suite incluster.pw.tswith its rigcluster.ts;fixtures/next-16-use-cache/(for example the cached page) andfixtures/next-16-cluster/(for example the rig page);profile.Verification
All of these were run on
919d31c: Node 24.18.0, macOS arm64, loopback alias pool127.0.0.2–33.npm test)'max'bugs were reproduced against the pre-fixdist/.npm run test:integration)harper-pro-openshift:5.0.26, plus new cases for the'max'profile, sweep deletion, the tombstone's Harper$expiresAtequallinglapsesAt, and a'use cache'row's$expiresAtequalling its timestamp plus Next'sexpire.826ef5f): the homepage app (rsc-app, Next 16.3.3, real AEM content) onharper-integration:5.2.0in Docker, 4 workersff7e895.ff7e895scores 17/28 on the same harness.The E2E cases, which were real HTTP requests plus Harper state checks:
'use cache'rows are stored with their tags and cache lives. All workers share one cached value, and the guest grid had 0 regenerations in 15 reads.home:TN:1without touchinghome:TN:12.'max': stale once, regenerated once, then 0 regenerations in 10 reads.ff7e895kept regenerating.{expire: 8}: a miss once it has passed.ff7e895served the expired entry.revalidatePath: a single route,'/', 'layout', and a page with a cached component.ff7e895: existing entries are served without regenerating. Thetagsindex is built for existing rows. An old'max'tombstone doesn't block a new one. Older rows are swept from all 4 worker threads.Untested:
updateTag, which only works inside a Server Action;Complexity: complicated
🤖 Generated with Claude Code
Origin — the dispatch brief this PR was written from
Back the
use cachedirective with Harper and persist per-entry cache livesLIVE CONVERSATION about #65.
You are answering a person, in a thread, one turn at a time. Every turn:
each of your previous turns is in it. Read the PR/issue and the code as needed.
they are talking to you, and a status template is not an answer.
Each turn arrives as ASK (answer it, change nothing) or PERFORM (do it, then say what you did) —
the person chose which when they sent it, and the run's own prompt tells you which one this is.
Never infer it from the wording: an unrequested commit in the middle of a discussion and a polite
description of work that was supposed to happen are the two failures this exists to prevent.
Never mark a PR ready and never merge from this conversation.
Dispatch: task
chat-pr-nextjs-65-ethanarrowood· queued by unknown · ran by codex/gpt-6-astra/low · worker Ethan-Work-Laptop-5fc728Review-Coverage: authored=claude; ran=codex; adjudicated=domain; blocked=gemini(auth),cursor-grok(no-receipt),cursor-composer(no-receipt),cursor-muse(no-receipt); declined=cursor-kimi; rounds=11; full=1 @ c5c3365
Human-Review-Need: 4 (decisions: oversized-key-truncate-and-hash, reads-fail-closed-until-hydrated, one-year-entry-cap-sizes-tombstones, mirror-into-next-private-tags-manifest, revalidate-path-not-swept-for-use-cache, old-build-sweep-env-opt-in-fixed-delay, rest-ordering-documented-not-fixed, cluster-suite-requires-harper-pro-image) @ c5c3365