fix(#939): derive the service worker's cache prefix from one authored string - #947
Merged
Conversation
… string The prefix that names the worker's caches and the prefix its activate handler purges were two separate strings in two files, kept in agreement by hand. In a fork they fell out of agreement and shipped: rebrand.sh could not reach `.mjs`, so public/sw.js was rebranded and scripts/stamp-sw-version.mjs was not. The build then stamped caches under one brand while the purges matched another. Nothing ever matched, and every deploy left caches that could never be collected -- unbounded storage growth in visitors' browsers, observed live on a real fork. WHAT THIS IS, AND IS NOT. #922 already closed that specific path: discovery is now `git ls-files`, so both files are swept together and a freshly rebranded fork gets matching prefixes. Verified on a throwaway fixture carrying both halves. So this is NOT a live-bug fix upstream, and #939 has been corrected to say so. It is worth doing anyway, for a reason worth stating precisely: the two strings now agree BECAUSE both happen to be rewritten in the same sweep. That is incidental, not structural. One `rebrand:keep`, one hand edit, or one move into a file the sweep does not reach, and they part again -- silently, because the damage is only visible in a browser's storage. Deriving one from the other removes the second copy instead of re-synchronising it. CACHE_VERSION is now the only place the brand appears. CACHE_PREFIX derives from it, and both purge sites and IMAGE_CACHE derive from that. Two things were also wrong independently of #922. The guard could not fail: `cacheVersion.startsWith(REQUIRED_PREFIX)` where cacheVersion was BUILT from REQUIRED_PREFIX. That tautology is why the divergence shipped with a check sitting next to it; it now refuses to stamp when it cannot parse a prefix out of the worker, a condition that can actually occur. And nothing related what the build stamps to what the worker purges, so nothing could have caught it. scripts/__tests__/sw-cache-prefix-agrees.test.js closes that gap and deliberately never names a brand: the worker must derive rather than hard-code, the stamp script must read rather than declare, and -- behaviourally -- stamping a worker carrying an ARBITRARY brand must yield a version with that same brand. The third is the one that would have caught the original defect, precisely because it does not care what the brand is. tests/unit/sw-images.test.ts required IMAGE_CACHE to be a literal starting with the brand. That check WAS the coupling. It now asserts IMAGE_CACHE is built from CACHE_PREFIX, and that CACHE_PREFIX strips the -v tail so #438 cannot return through a per-build image cache name. Mutation-tested on this tree: hard-coding the purge prefix fails one test, re-declaring a prefix in the stamp script fails two, and deriving IMAGE_CACHE from the per-build version fails two. All return green on restore. Behaviour upstream is unchanged, which is the point: a real build still stamps `scripthammer-v0.0.1-<sha>` and all three cache names remain purge-eligible. Ported from TortoiseWolfe/geolarp#35, where it was built and proven first. Closes #939 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
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
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.
Closes #939.
The ticket's premise had already changed, so I checked before porting
#922 (PR #946) already fixed the live bug. Verified on a throwaway fixture carrying both halves of the coupling, rebranded with the fixed script:
.mjsis inside the sweep now, so both halves rebrand together and agree. The failure #939 described cannot reproduce on a fork rebranded with the fixed script. I have corrected #939's body and revised its severity rather than leave it claiming a live leak.Why this is still worth landing
The two strings now agree because both happen to be rewritten in the same sweep. That is incidental, not structural. One
rebrand:keep, one hand edit, or one move into a file the sweep does not reach, and they part again — silently, because the damage is only visible in a browser's storage.Deriving one from the other removes the second copy instead of re-synchronising it.
CACHE_VERSIONis now the only place the brand appears;CACHE_PREFIXderives from it, and both purge sites andIMAGE_CACHEderive from that.Two things wrong independently of #922
The guard could not fail.
cacheVersionwas built fromREQUIRED_PREFIX. That is why the divergence shipped with a check sitting right beside it. It now refuses to stamp when it cannot parse a prefix out of the worker — a condition that can actually occur.Nothing related what the build stamps to what the worker purges. So nothing could have caught it.
The new test never names a brand
That is deliberate:
sw-cache-prefix-agrees.test.jsasserts the worker derives rather than hard-codes, the stamp script reads rather than declares, and — behaviourally — that stamping a worker carrying an arbitrary brand yields a version with that same brand. The third would have caught the original defect precisely because it does not care what the brand is.tests/unit/sw-images.test.tsrequiredIMAGE_CACHEto be a literal starting with the brand. That check was the coupling — two hand-synchronised strings. It now assertsIMAGE_CACHEis built fromCACHE_PREFIX, and thatCACHE_PREFIXstrips the-vtail so #438 cannot return through a per-build image cache name.Mutation-tested on this tree
IMAGE_CACHEfrom the per-build version (#438)All return green on restore.
Behaviour upstream is unchanged, which is the point
A real production build still stamps
scripthammer-v0.0.1-<sha>, and all three cache names remain purge-eligible. This is hardening, not a behaviour change.Ported from
TortoiseWolfe/geolarp#35, where it was built and proven first.