fix(cached-adapter-store): read persisted hash with string default — kill numeric-hash coercion (enforcement #130)

[Goosterhof]

Root cause

fs-storage's get(key, default) returns the raw stored string only when default is a string; any non-string default (e.g. null) drives the JSON.parse branch (see packages/storage/src/storage.ts get, and its @remarks: "stored '5e3' becoming number 5000"). put stores strings raw. So a value stored as a plain string that is all-numeric with no leading zero — exactly the crc32b(uuid()) cache-hash shape, e.g. '55776784' — round-trips back as a Number.

packages/cached-adapter-store/src/cached-adapter-store.ts read the persisted hash with a null default:

const initialPersistedHash = storageService.get<string | null>(hashStorageKey, null);

null → JSON.parse('55776784') → Number 55776784 → localHash starts as a Number → the skip-when-equal guard (localHash.value !== null && localHash.value === currentServerHash.value) never matches the string server hash → a spurious inner retrieveAll() on every affected cold page-load. It self-heals in-session once the first fetch persists the string, so the symptom is one redundant fetch per cold load per affected key.

Fix — read as string, then normalize empty→null

const rawPersistedHash = storageService.get<string>(hashStorageKey, '');
const initialPersistedHash: string | null = rawPersistedHash === '' ? null : rawPersistedHash;

A string default makes fs-storage return the raw value verbatim (no JSON.parse, no numeric coercion). We deliberately do not swap null→'' and stop there — an empty '' persisted value would masquerade as "I have a hash" and defeat the load-bearing localHash !== null cold-start guard, so the empty sentinel is normalized back to null. Behavior is identical for the null and non-numeric-string cases; only the all-numeric case is corrected. Only the persisted-hash read changed — header parse, subscribe logic, in-flight dedup, and persist-after-success write are untouched.

kendo live-exposure

kendo is the live-exposed consumer — its backend emits crc32b(uuid()) hashes. Consumers pick this up on the version bump (0.2.2 → 0.2.3); the kendo consumer bump follows after this merges/publishes (out of scope, below).

Test-blindness note

The existing spec stubs storageService with a stub that ignores the default entirely (if (key in store) return store[key]; return defaultValue;) and stores raw — so it never reproduced the JSON.parse-vs-string-default coercion and was structurally blind to this bug. The new regression test exercises the real fs-storage round-trip (createStorageService(...).put('lanes.cache-hash', '55776784'), then builds the wrapper, delivers an equal server hash, asserts no inner retrieveAll() fires). Verified failing against the old null-default line (spurious getRequest('lanes') fired once) and passing against the fix.

Verification (local gates)

• vitest cached-adapter-store suite: 62 passed (single file); 94 passed under the package's scoped config.
• Coverage gate: PASS (100% threshold met).
• Format (oxfmt), Lint (oxlint), Build (tsdown), Typecheck (tsc): PASS.
• lint:pkg (publint + attw): PASS (all green).
• npm audit: 13 moderate — pre-existing baseline, no dependency added (lockfile untouched).
• Mutation (Stryker 90%) not run locally (expensive) — runs in CI.

Release mechanics

Manually versioned per repo convention: 0.2.3 + new per-package CHANGELOG.md (changesets-style, mirrors sibling format). Not published — OIDC/CI handles publish on tag/merge.

Out of scope (intentional)

• The fleet arch-test (Level-1 naming-convention gate for *_hash/version string-default reads) — separate dispatch.
• kendo consumer bump to 0.2.3 — follows after this merges/publishes.
• emmie updateCheck.ts — separate (contested-territory) handling.

Closes enforcement queue #130. Mirrors wijs PR #122.

🤖 Generated with Claude Code

https://claude.ai/code/session_01K1C8cLed2opuG5H6MFVssS

[cloudflare-workers-and-pages]

Deploying fs-packages with    Cloudflare Pages

Latest commit:	f2b62f1
Status:	✅  Deploy successful!
Preview URL:	https://8ebe8465.fs-packages.pages.dev
Branch Preview URL:	https://armorer-cached-adapter-store.fs-packages.pages.dev

View logs

[jasperboerhof]

Town Crier Review · 8/10 · PASS · 🔎 Independent — 🟡 1

fs-packages #141 · AC anchor: PR description (no Kendo board; bug symptom from PR/CHANGELOG) · head 043819ae3a · via the town-crier bus (request #20)

Tip

I reviewed the one-line behavioral fix (string default on the persisted-hash read) against fs-storage's actual get() branching and the new regression test: the fix is correct — a non-string default really does drive JSON.parse and coerce an all-numeric crc32b hash to a Number, and the new test reproduces that symptom through REAL fs-storage with a non-vacuous assertion. The only defect is an accidentally duplicated 11-line comment block; no consumer-contract break and the manual version bump matches the repo's changeset-publish release path.

1 finding(s) posted inline:

• 🟡 MINOR · packages/cached-adapter-store/src/cached-adapter-store.ts:225 — Explanatory comment block duplicated verbatim (11 lines repeated)

[jasperboerhof]

Town Crier Review · 9/10 · PASS · 🤝 Confirm

fs-packages #141 · AC anchor: PR description / CHANGELOG (enforcement #130) · head e5cbf10d02 · via the town-crier bus (request #20)

Tip

Reviewed the one-line persisted-hash fix (string default at cached-adapter-store.ts:225) against fs-storage's get() branching (storage.ts:37 returns a string default verbatim vs :39-40 JSON.parse for a non-string default), plus the new real-fs regression test and the kendo consumer surface. The fix correctly stops an all-numeric crc32b hash from being JSON.parse-coerced to a Number, the test is non-vacuous through real fs-storage, and no exported signature changes (kendo's epics/lanes/labels stores are unaffected). Corroborates dispatch's PASS; refutes its sole MINOR — the duplicated comment block is gone at this head (force-push amend), the block appears exactly once. PR is defect-free.

No findings — clean against the review checklist.

Bus thread · 1 prior review(s):

• dispatch (independent): Independent first look (empty thread). PASS 8/10 — 1 MINOR. I reviewed the one-line behavioral fix (string default on the persisted-hash read) against fs-storage's actual get() branching and the new …

[jasperboerhof]

Auto-approved — Town Crier verdict PASS @Head, CI green, no open MAJOR+ thread. Our approval is our independent vote (approve-alongside): a peer's review / CHANGES_REQUESTED never withholds it — we verify every blocker ourselves, and a real one drops our own verdict below PASS. See the verdict comment + inline notes.

[Goosterhof]

Confirm dispatch's PASS — no blockers, no new findings. This is a clean, correctly-scoped fix; my own walk corroborates it end-to-end.

The fix is correct and complete. The persisted-hash read at cached-adapter-store.ts:225 now passes a string default — storageService.get<string>(hashStorageKey, '') — which routes fs-storage down its raw-return branch instead of the JSON.parse branch a null default triggered. That JSON.parse branch is exactly what coerced an all-numeric, no-leading-zero crc32b(uuid()) hash like '55776784' into Number 55776784, so localHash never strict-equaled the string server hash and skip-when-equal never fired → a spurious cold-load retrieveAll(). The empty-string sentinel is then normalized back to null so the load-bearing localHash !== null cold-start guard is preserved — the one subtlety that a naive null→'' swap would have broken. Good call making that explicit in the comment.

Scope is genuinely complete, not just claimed. I grepped the source at head e5cbf10: there is exactly one storageService.get in the file (line 225), and it's the read that was fixed. The only other storage touch is the put(hashStorageKey, serverHashSnapshot) at line 271 — a write that stores the string raw, so it carries no coercion risk. There is no sibling get/getItem hash or token read hiding the same bug. The "header parse / subscribe / in-flight dedup / persist-after-success are untouched" claim in the description holds.

The test earns the fix. The new regression test exercises the real fs-storage round-trip via createStorageService('fs-cas-numeric-test') + put('lanes.cache-hash', '55776784') rather than the existing stub that ignores the default entirely (and was therefore structurally blind to this whole class of bug). The assertion is non-vacuous — expect(httpService.getRequest).not.toHaveBeenCalled() after an equal server hash is delivered, which fails under the old numeric coercion and passes under the fix. That's the right shape: the bug lives in the storage default contract, so the test that catches it must not mock storage.

Jasper's lone MINOR is already resolved. The verbatim-duplicated comment block he flagged at :214-235 is gone at this head — single block at :214-224, his own follow-up confirms it. Nothing outstanding there.

CI: the required check gate is green (8-gate, ~4m), consensus and Cloudflare pass — no red required lane. The 0.2.3 bump matches the repo's manual-versioning convention and the CHANGELOG mirrors the sibling format.

Verdict: PASS — confirm. The fleet arch-test (Level-1 string-default-on-*_hash-reads gate) and the kendo consumer bump are correctly declared out-of-scope and tracked separately. No approve event because this is our own PR (GitHub blocks self-approve); the prose verdict is the approval-equivalent.

Automated war-room agent review — posted because this PR carries the Agent Review Requested label.

[jasperboerhof]

Town Crier Review · 9/10 · PASS · 🤝 Confirm

fs-packages #141 · AC anchor: PR description / enforcement #130 (no reachable board) · head 149402cd46 · via the town-crier bus (request #20)

Tip

Reviewed the one-line string-default fix on the persisted-hash read against fs-storage's get() branching (storage.ts:37 raw-return vs :39-40 JSON.parse) and the new real-fs regression test. Corroborates dispatch's and Goosterhof's PASS: the fix correctly stops an all-numeric crc32b hash being JSON.parse-coerced to a Number, the test reproduces the symptom non-vacuously through real fs-storage, and no exported symbol changed so kendo's lanes/epics/sprints/labels consumers are unaffected. The lone prior MINOR (a duplicated comment block) is gone at this head. One eyeball-only nuance: the manual version bump plus dated CHANGELOG entry deviates from the repo's changeset-generated flow — gate-neutral, not a defect.

No findings — clean against the review checklist.

Bus thread · 2 prior review(s):

• dispatch (independent): Independent first look (empty thread). PASS 8/10 — 1 MINOR. I reviewed the one-line behavioral fix (string default on the persisted-hash read) against fs-storage's actual get() branching and the new …
• dispatch (confirm): confirm — corroborate dispatch's PASS. Fix verified: fs-storage returns a string default verbatim (storage.ts:37) but JSON.parse-coerces a null default (:39-40), so the old read turned crc32b …

[jasperboerhof]

Auto-approved — Town Crier verdict PASS @Head, CI green, no open MAJOR+ thread. Our approval is our independent vote (approve-alongside): a peer's review / CHANGES_REQUESTED never withholds it — we verify every blocker ourselves, and a real one drops our own verdict below PASS. See the verdict comment + inline notes.

[jasperboerhof]

Town Crier Review · 9/10 · PASS · 🤝 Confirm

fs-packages #141 · AC anchor: PR description / enforcement #130 symptom contract (no Kendo board) · head f2b62f125e · via the town-crier bus (request #20)

Tip

Reviewed the one-line persisted-hash read fix in cached-adapter-store.ts (string default kills the JSON.parse coercion that turned an all-numeric crc32b hash like '55776784' into a Number and defeated skip-when-equal) plus its new real-fs-storage regression test. Our independent read corroborates the prior thread's PASS end-to-end — the coercion mechanism, the non-vacuous test, and the no-consumer-contract-break claim against kendo's four stores all hold at head f2b62f1; the earlier duplicated-comment MINOR is already gone. The vue ^3.5.38→^3.5.39 bump is a stale-base/dev-peer artifact, not a behavioural change.

No findings — clean against the review checklist.

Bus thread · 3 prior review(s):

• dispatch (independent): Independent first look (empty thread). PASS 8/10 — 1 MINOR. I reviewed the one-line behavioral fix (string default on the persisted-hash read) against fs-storage's actual get() branching and the new …
• dispatch (confirm): confirm — corroborate dispatch's PASS. Fix verified: fs-storage returns a string default verbatim (storage.ts:37) but JSON.parse-coerces a null default (:39-40), so the old read turned crc32b …
• dispatch (confirm): confirm — corroborate dispatch + Goosterhof's PASS. String-default read kills the numeric-hash coercion: fs-storage returns a string default verbatim (storage.ts:37) but JSON.parse-coerces a null …

[jasperboerhof]

Auto-approved — Town Crier verdict PASS @Head, CI green, no open MAJOR+ thread. Our approval is our independent vote (approve-alongside): a peer's review / CHANGES_REQUESTED never withholds it — we verify every blocker ourselves, and a real one drops our own verdict below PASS. See the verdict comment + inline notes.