[harper-joseph]

The raw-document cache is enabled on a live deployment and storing nothing. This adds the one option that lets a deployment override an origin's claim about itself, on evidence, and teaches the console to read the result.

@harperfast/prerender v0.81.0, @harperfast/prerender-console v0.15.0.

What the metric said

One hour on the enabled route, read straight off prerender_ops:

outcome	count
has-cookie	13,106
not-200	94
stored	0

against 33,572 misses on that route. This is exactly the failure the metric's own documentation predicts — "an enabled route that is filling nothing looks exactly like a disabled one unless the reason is recorded" — and the reason was recorded.

Why the refusal is wrong here

storeRefusal refuses on hadSetCookie, and hadSetCookie exists only because sanitizeOriginResponseHeaders already dropped the header. The cookie can never reach a stored page. The check is protecting against a personalized body, using the cookie as the best available hint.

That hint is false where a CDN already serves one cached copy of these documents to every visitor with none of the session cookies in its cache key. Measured, two cold visitors fetching the same listing URL:

• all divergence sits in one contiguous bot-manager telemetry block — a counter, a request id, a signed per-request token
• 615 product links, 120 distinct, identical set; 15 price occurrences in identical sequence; identical <title>

Dropping the cookie check alone would have fixed half: sampling 8 responses on that route, Set-Cookie was present on 8 of 8 and Cache-Control: private on 4 of 8. Refusal order is not-200 → staging → has-cookie → content-type → no-store (which matches private), so removing only the first leaves the private half refused. Both are the same claim in two forms, so the option covers both.

What it does

render.raw.assumeShared, default false.

• Suppresses only the two sharedness heuristics: Set-Cookie and Cache-Control: private.
• A literal no-store is still refused — the origin instructing caches not to keep the response at all is a different statement from "not for shared caches".
• not-200, staging, content-type, oversize, no-body, capture-busy: untouched, with a test pinning that the override widens nothing else.
• Documents stored under it count as stored-unshared, not stored, so the assumption stays visible. unsharedHint() is the single source of truth for both the refusal and the outcome name.

What that series is, precisely: a census, not an alarm. On an origin that sets a cookie on every response it is pinned at 100% from the first minute and cannot rise, so no threshold on it detects anything. The detector is re-running the body diff. The option text, the metrics catalog and METRICS.md all say this — an earlier draft of this PR claimed the alarm "survives", which was false on the only deployment this is for.

Review — six findings, and two of them were real defects

No outside-model coverage was available on this machine (agy not installed, codex/gemini present but unauthenticated), so this was the repo's CI reviewer plus a fresh-context same-family pass. Stating that rather than implying independent coverage.

A blocker I introduced mid-review. The CI reviewer flagged .includes('private') as fragile. It is — but its suggested /\bprivate\b/i does not fix it, because - is a word boundary, so that regex matches inside X-Private-Header and x-private-hint exactly as includes does. I replaced it with a directive parser, and that swapped over-refusal (conservative for a cache) for under-refusal on any header it could not read. Measured against origin/main across 1,440 input combinations with assumeShared false — the default path this PR claims is untouched — six shapes went from refused to stored:

cache-control	main	first parser
private;max-age=60	refused	stored
no-store;private	refused	stored
no-cache="x, no-store	refused	stored
max-age=600, no-store"	refused	stored
"private"	refused	stored
"no-store"	refused	stored

The first is an origin plainly declaring the response unshared, about to be replayed to every crawler with no flag set anywhere. The parser now states the invariant it must hold — it may only ever remove a false positive — accepts ; and a quoted directive name, and on an unbalanced quote defers to the substring test. Re-run across the same 1,440 inputs: 8 differences remain, all four distinct values being the deliberate well-formed false-positive fixes (no-cache="X-Private-Header", x-private-hint=1, privately-held, no-cache="no-store").

So the honest form of "no behaviour change by default" is: no deployment's behaviour changes except that four well-formed cache-control shapes that were wrongly refused are now stored. That is the accuracy fix, and it is the only default-path change.

The console would have reported the feature as 100% broken the moment it worked. refused = total - stored counted only the literal stored outcome, so stored-unshared rendered as Stored 0 / Refused 100% with the warning lit, and appeared in the refusal table as "an outcome this console does not know about". The coverage guard cannot catch this class: it scans the third recordAnalytics slot and rawCache puts the outcome in the fourth. Fixed, with a test that fails without it. This is why the console bump ships in the same PR — setting assumeShared anywhere before console 0.15.0 makes the panel lie.

Two more claims corrected rather than softened. The verification recipe could not see the personalization axis this origin actually has: two cold fetches from one machine share source IP, accept-language and referer, all of which the origin fetch forwards, while the cache key is url + device — blind by construction to geo and locale variance, which is what a retail origin personalizes. It now says to vary the vantage point, and notes that nothing reads Vary. And a stored private is relayed on every raw hit; not a change, since the origin proxy already relays cache-control on a miss, but it means no edge offload on those hits.

One pre-existing doc bug fixed in passing: the metrics catalog's detail-slot description said "Other series: null" while raw_cache puts its outcome there — the slot the console reads this panel from.

Verification

plugin 1180/1180, console 277/277, browser 297/297 (untouched by this branch), lint and format:check clean. Both new regression tests verified to fail against their unfixed source. This repo has no PR CI, so that is local verification rather than a gate.

Versions: main is plugin 0.80.0 / console 0.14.0; the only other open PR reserving a number is #178 (0.75.0). This takes plugin 0.81.0 and console 0.15.0.

Deploy note

Loading this needs one worker recycle per node — the drained rolling restart_service http_workers recipe, not a container restart. The config flag live-reloads but is inert until the code is present. Consumer config: HarperFast/kohls-pr#118.

🤖 Generated with Claude Code

[harper-joseph]

/gemini review

[gemini-code-assist]

Code Review

This pull request introduces an assumeShared configuration option to the prerender plugin, allowing users to override default refusal logic for documents marked as private or containing cookies, provided the origin is verified to serve shared content. The changes include updates to the configuration schema, cache refusal logic, and metrics tracking to distinguish between standard stored documents and those stored under the new assumeShared policy. The review feedback suggests using case-insensitive regular expressions with word boundaries for more robust and efficient header parsing in rawCache.js instead of full-string lowercasing.

[gemini-code-assist]

Using .includes('private') on a lowercased string can be fragile and introduces redundant lowercasing overhead. Instead, we can use a case-insensitive regular expression with word boundaries (/\bprivate\b/i) to perform a more robust check. This avoids matching partial substrings and eliminates the need to lowercase the entire string when the header is absent.

Suggested change

	export const unsharedHint = (resource) => {
	if (resource?.hadSetCookie) return 'set-cookie';
	const cacheControl = String(resource?.headers?.['cache-control'] ?? '').toLowerCase();
	if (cacheControl.includes('private')) return 'private';
	return null;
	};
	export const unsharedHint = (resource) => {
	if (resource?.hadSetCookie) return 'set-cookie';
	const cacheControl = resource?.headers?.['cache-control'];
	if (cacheControl && /\bprivate\b/i.test(cacheControl)) return 'private';
	return null;
	};

[harper-joseph]

Taken, and the reasoning is right even though the suggested fix does not close it — so this ended up stronger than proposed.

/\bprivate\b/i matches inside X-Private-Header and inside x-private-hint exactly as includes does, because - is a word boundary. Verified against all three shapes:

cache-control	includes('private')	/\bprivate\b/i
no-cache="X-Private-Header", max-age=600	true	true
x-private-hint=1, max-age=600	true	true

Both directives that matter here may carry a quoted field-name argument — no-cache="X-Private-Header" is legal per RFC 9111 §5.2.2 — so the only thing that separates an argument from a directive is reading the directive names. Fixed in 4e70015 with cacheControlDirectives(): split the list on commas outside quotes, take the name before =, lowercase. Your efficiency point is handled too — an absent or empty header returns an empty set immediately, with no coercion and no lowercasing.

Worth being precise about the severity: over-matching here refuses to store, which is the conservative direction, so this was never a safety hole. It is an accuracy one, and it matters more than it did before this PR — no-store is now the one cache-control refusal that survives assumeShared, so a phantom one silently costs a deployment stores it cannot account for and cannot distinguish from a real instruction.

Test added and verified to fail against the substring implementation and pass against the parser, covering the quoted-argument false positives, PRIVATE/No-Store casing, an array-valued repeated header, and absent/empty values.

[harper-joseph]

Follow-up, because the fix above was superseded two commits later and this thread should not point at the intermediate version.

cacheControlDirectives() as first written failed open: on any header it could not parse it answered from whatever survived the parse, so it refused less than the substring test it replaced. Measured against origin/main across 1,440 input combinations with the new option off, six shapes went from refused to stored — private;max-age=60, no-store;private, an unbalanced quote, a stray trailing quote, "private", "no-store". The first is an origin plainly declaring the response unshared, on the default path, with no flag set anywhere.

0844fe7 fixes that. The parser now returns null when it cannot parse, and hasCacheControlDirective falls back to the conservative substring test there, so the stated invariant holds: the parser may only ever remove a false positive. ; is accepted as a separator and a quote is stripped from a directive name (neither is legal, both are real, and the charitable reading is the one that keeps refusing). Re-run across the same 1,440 inputs leaves 8 differences, all four distinct values being the intended well-formed fixes your finding was about.

So your finding was right, the mechanism took two attempts, and there is now a test table pinning all six malformed shapes.

[gemini-code-assist]

Similarly, we can avoid redundant string coercion and lowercasing of the cache-control header here by reusing the header value and checking it with a case-insensitive regular expression with word boundaries (/\bno-store\b/i). This is more robust and efficient.

Suggested change

	const cacheControl = String(headers['cache-control'] ?? '').toLowerCase();
	// `no-store` is the origin instructing caches not to keep this response AT ALL, and
	// `assumeShared` does NOT override it: that setting answers "is this response the same for every
	// crawler", which is a different question from "may it be kept". `private` is the explicit form
	// of the sharedness claim, so that one yields.
	//
	// `no-cache` is deliberately NOT refused: it means revalidate-before-use, not do-not-store, and
	// refusing it would exclude most correctly-configured HTML.
	const cacheControl = String(headers['cache-control'] ?? '').toLowerCase();
	if (cacheControl.includes('private') || cacheControl.includes('no-store')) return 'no-store';
	if (cacheControl.includes('no-store')) return 'no-store';
	if (unshared === 'private' && !policy.assumeShared) return 'no-store';
	const cacheControl = headers['cache-control'];
	// no-store is the origin instructing caches not to keep this response AT ALL, and
	// assumeShared does NOT override it: that setting answers "is this response the same for every
	// crawler", which is a different question from "may it be kept". private is the explicit form
	// of the sharedness claim, so that one yields.
	//
	// no-cache is deliberately NOT refused: it means revalidate-before-use, not do-not-store, and
	// refusing it would exclude most correctly-configured HTML.
	if (cacheControl && /\bno-store\b/i.test(cacheControl)) return 'no-store';
	if (unshared === 'private' && !policy.assumeShared) return 'no-store';

[harper-joseph]

Same fix covers this one, and this is the line where it actually earns its keep.

no-cache="no-store" is legal and includes('no-store') reads it as a no-store directive. That matters more after this PR than before it: no-store is now the only cache-control refusal that survives assumeShared, so a false positive here is a store the deployment loses with a reason it cannot distinguish from the origin genuinely saying "do not keep this".

4e70015 replaces both checks with cacheControlDirectives(headers['cache-control']).has('no-store'), which parses the directive names once and answers exactly. The early return on an absent header covers the redundant-coercion point you raised.

[gemini-code-assist]

Code Review

This pull request introduces a new configuration option assumeShared to the prerender plugin, allowing documents with Set-Cookie or Cache-Control: private to be cached under specific deployment conditions. It also updates metrics to track these as stored-unshared and adds comprehensive unit tests to validate the new behavior. No review comments were provided, and the changes are well-implemented.

[harper-joseph]

Pushed 4e70015 and 0844fe7. The second one matters: a pre-merge review found that the directive parser I added for the first round of feedback failed open on any cache-control it could not parse, so six malformed shapes — including private;max-age=60, an origin plainly declaring the response unshared — went from refused to stored on the default path with no flag set. Measured against origin/main across 1,440 input combinations. The parser now defers to the substring test wherever the parse is not trustworthy, so it can only ever remove a false positive; re-run leaves 8 differences, all of them the intended well-formed fixes.

Same commit teaches the console stored-unshared (it was counting it as a refusal, which would have shown Stored 0 / Refused 100% at the moment the option made the cache work) and corrects three claims the option text made to operators. PR body rewritten to current state.

/gemini review

[gemini-code-assist]

Code Review

This pull request introduces the render.raw.assumeShared configuration option, which allows the prerender plugin to store documents even when the origin marks them as personal via Set-Cookie or Cache-Control: private. To support this, a robust Cache-Control directive parser has been implemented to replace unsafe substring matching, and a new stored-unshared metric outcome has been added. The management console UI and tests have been updated to correctly classify these as stored documents rather than refusals, preventing false alarms. There are no review comments provided, so I have no feedback to address.