fix(npm): stop a stale rebuild from stamping the packument cache fresh

What this fixes

enqueueRebuild let a write-triggered packument cache rebuild collapse, via singleflight, into a rebuild already in flight that had read npm_versions before the write committed. The stale rebuild then stamped expires_at = NOW() + npm.packument_cache_ttl (7 days by default), so the next read hit a fresh-but-stale cache row and served a packument missing the write for the whole TTL.

That violates S11 AC 27 ("never a stale cached packument"), and it is the race behind the intermittent conformance:npm:s3-garage failure reported in #460 (closed):

ASSERT FAIL: deprecation message via npm view:
  output does not contain 'use 2.0.0 instead'

The timeline in that job's log matches exactly:

time	event
~18.28	dist-tag rm commits, force-expires the cache, enqueues rebuild R
18.291	GET ...?write=true misses the (expired) cache, inline-builds, enqueues — collapses onto R
18.29–18.31	R reads npm_versions — pre-deprecation
18.313	PUT handler=deprecate code=success commits, enqueues — collapses onto R, contributing nothing
18.320	R commits its blobs (dedup:true — the bytes already existed, because they are the pre-deprecation packument) and stamps expires_at = NOW() + 168h
18.550	npm view hits the fresh-but-stale row, deprecation absent
18.571	assert fails

dedup:true is the tell: the rebuilt document was byte-identical to the one already in CAS, i.e. the deprecation was not in it.

Per #460 (closed)'s own classification note, the issue is relabelled type::bug / bug::functional / severity::3.

The fix

Three layers: a token the writers rotate, a fence the rebuild carries, and a write path that stops collapsing.

1. A compare-and-swap token, not a timestamp. New nullable npm_packages.packument_rebuild_token (migration 20260805170000). ForceExpireNpmMetadata mints a fresh token, stamps it on the package row, and returns it as the rebuild's fence; UpsertNpmMetadataFileForBlob refuses the stamp with ErrNpmMetadataCacheSuperseded unless the row still carries exactly that token, comparing under SELECT ... FOR UPDATE on the package row. Every write handler calls RotatePackumentRebuildTokenTx — publish, deprecate, both dist-tag mutations, and both unpublish paths — so a write landing mid-rebuild invalidates whatever fence that rebuild is holding.

An earlier revision of this MR fenced on expires_at instead. That design is gone, and the reason it had to go is worth recording: NOW() is transaction_timestamp(), so a write handler's expires_at carries the instant its transaction BEGAN and can be older than a concurrent rebuild's fence while its data becomes visible newer. No comparison of two timestamps — ordering or equality — is safe against that. A token has no clock. It also fixes the cold-package case a timestamp could not address at all: every column on npm_metadata_files is NOT NULL, so a package whose cache was never built has no row to carry a timestamp, and two rebuilds both observing no row decided the winner by INSERT order rather than by which one read post-write rows. The package row always exists, so the token is always comparable — which is why the upsert carries no fence predicate of its own (TestUpsertNpmMetadataFileStmt_CarriesNoFencePredicate).

The token lives on npm_packages, so rotation participates in the lock order: it must run while the transaction is still on npm_packages, before it touches any other table, or a handler that opens on npm_versions deadlocks against a rebuild. That rule is stated on RotatePackumentRebuildTokenTx.

2. A pre-check before the blob is committed. A refused rebuild that has already written its CAS blob leaks a stored object permanently — nothing in this service reclaims an unreferenced committed blob, because the ADR-011 reconciler is unimplemented (#498). PackumentRebuildFenceHolds is an advisory read taken after rendering and before the blob write, which turns the common case (a writer committed while this rebuild rendered) from a leak into a cheap early return. It does not close the window: a writer can rotate between the pre-check and the upsert, and that residual is metered as ..._npm_packument_rebuild_orphaned_blobs_total, an upper bound on the leak rather than an exact count.

2b. The fence refuses a tombstoned package. npmPackageRowPredicate matches only active rows, so a rebuild that takes its fence after a whole-package unpublish commits gets uuid.Nil and its upsert is refused, instead of rotating a token onto the tombstoned row and re-inserting three npm_metadata_files rows plus three attachments for a package that no longer exists. Republish allocates a new package id, so nothing stale was ever served, but those rows are permanent and their attachments keep the blobs referenced — the leak ADR-007's in-transaction delete exists to prevent. The filter is only safe while every writer rotates against a live row, and RotatePackumentRebuildTokenTx ignores the row count, so NpmPackageUnpublishDeleter.cascade rotates before SoftDeleteNpmPackage; both statements are on npm_packages and take the same row lock, so the lock order is unchanged.

3. Write handlers stop collapsing. Publish, deprecate, unpublish, and the two dist-tag mutations call the new enqueueRebuildAfterWrite, which retires the singleflight key so the write actually gets a rebuild whose input reads happen after its commit, instead of one silently swallowed by the collapse. Layer 1 alone is correct but would leave the cache cold until the next read; this is what makes the write's own rebuild land.

Read paths (packument_get.go, the dist-tags inline build) keep calling enqueueRebuild and keep collapsing. AC 29's guarantee is scoped to concurrent reads against an expired row, so it is unchanged.

A superseded rebuild is metered ..._npm_packument_rebuild_total{result="superseded"} and logged at Info, not counted as an error: it is the guard working as designed, and it must stay distinguishable from a real failure for the metric to be alertable.

Known remaining costs

Retiring the singleflight key means a burst of N writes to one package dispatches N rebuilds instead of one, and all but the last are refused by the fence. 7c65814d bounds that: rebuildMaxPerPackage = 4 caps how many of the process-wide 64 rebuildSem slots a single package can hold on the write path, so a hot package sheds its own dispatches (metered dropped) instead of crowding out other packages. The bound is a heuristic, not a coalescing fix — real per-package coalescing is #499 — and it is the reason the S27 River swap must not simply restore per-key dedup. That constraint is recorded on enqueueRebuildAfterWrite and at all five write-path TODO(s27-cache-rebuild) markers.

The read path stays uncapped, and that bound is weaker than it looks. A cap there would shed followers a rendering leader already covers, so it is the wrong lever — but read dispatches collapse only within one generation of the singleflight key. enqueueRebuildAfterWrite retires the key before its own dispatch faces either cap, so every write — including one whose dispatch is then shed — starts a generation whose first read becomes an uncapped rendering leader. Under sustained writes to one package those leaders accumulate at roughly one per write per rebuild duration, bounded only by rebuildMaxInFlight, and the failure mode is the result=dropped starvation of other packages the sub-quota exists to prevent. 87cc4505 records this on rebuildMaxPerPackage and rebuildUncapped rather than presenting the collapse as unconditional. Whether it warrants more than a comment — a per-generation leader cap, or pulling #499 forward — is open.

Orphaned blobs. The pre-check narrows but does not close the refused-blob window (layer 2 above). The residual is observable rather than silent, and its cost is zero whenever the refused document byte-matched a blob already in CAS.

Rolling deploys. During a rollout, an old pod's rebuild carries no token and stamps unconditionally. If it lands between a new pod's force-expiry and that pod's upsert, the new pod yields where it would previously have overwritten. Bounded by the rollout window, self-heals on the next write or at TTL, and never produces a worse stale serve than main does today.

Un-backfilled rows. The migration does not backfill, so a pre-existing package row starts with a NULL token. Nothing strands on it: the rebuild mints and writes a token rather than reading the stored one, so the first rebuild after deploy warms the cache normally.

Spec follow-up. docs/specs/S11-npm-hosted.md §Async work says River's per-key uniqueness "collapses concurrent enqueues into one pending execution per package". That stays true for pending jobs but must not extend to running ones, and §Packument cache does not yet describe the refusal outcome or the token. Both want a spec MR; not amended here because spec changes land on their own MRs.

Spec coverage

Spec: docs/specs/S11-npm-hosted.md

Acceptance criteria

#	Criterion	Tests
AC 27	After any packument-mutating write, the next GET returns the post-write state — never a stale cached packument	TestNpmMetadataFileStore_UpsertNpmMetadataFileForBlob/refuses to stamp a document rendered before a writer rotated the token (kind=0..2), .../a cold-package publish-then-deprecate race stamps the post-write document, TestEnqueueRebuildAfterWrite_DoesNotCollapseIntoAnInFlightRebuild
AC 27	Every write handler arms the fence by rotating the token in its own transaction	TestNpmWriteHandlers_RotatePackumentRebuildToken (publish, deprecate, both dist-tag mutations, both unpublish paths)
AC 29	Concurrent reads against an expired row still produce one rebuild per package	TestEnqueueRebuildAfterWrite_DoesNotCollapseIntoAnInFlightRebuild (asserts exactly 2 bodies: the write escapes, the read collapses), TestEnqueueRebuild_ReadPathIsNotPerPackageCapped
—	A rebuild whose own force-expiry rotated the token still stamps (positive control)	TestNpmMetadataFileStore_UpsertNpmMetadataFileForBlob/stamps when only the rebuild's own force-expiry rotated the token, .../stamps on a first rebuild for a package with no cache rows
—	A cold package (no cache row) can still be fenced and warmed	TestNpmMetadataFileStore_ForceExpireNpmMetadata/still fences a package with no metadata rows, so a cold cache can warm
—	The fence reaches every kind and is threaded unchanged	TestRebuildPackumentCache_ThreadsFenceIntoEveryUpsert, TestRebuildStoreKind_ThreadsTheTargetAndFenceToBothFenceChecks
—	Schema: nullable, uuid-typed, no default, present on the routed partition; Down drops it	TestNpmPackagesPackumentRebuildToken_ColumnExistsNullableAndUUIDTyped, _AcceptsNullAndValue, _DownDropsTheColumn

Error cases

#	Condition	Tests
E-1	Rebuild holding a rotated-away token: refused, no repoint, no leaked attachment, row stays a cache miss	TestNpmMetadataFileStore_UpsertNpmMetadataFileForBlob/refuses to stamp a document rendered before a writer rotated the token (kind=0..2)
E-2	Two rebuilds racing on a cold package: the one whose token was superseded loses, regardless of INSERT order	.../refuses a first rebuild superseded by a concurrent rebuild
E-3	Rebuild abandons the remaining kinds after the first refusal and reports errRebuildSuperseded	TestRebuildPackumentCache_AbandonsRemainingKindsWhenSuperseded
E-4	A refusal is metered result="superseded", never result="error"	TestEnqueueRebuild_MetersSupersededNotError
E-5	Refused after the blob was committed: the orphan is counted, not silent	TestRebuildStoreKind_CountsTheOrphanItCannotAvoid
E-6	Pre-check refuses before the blob is committed, so the common refusal leaks nothing	TestRebuildStoreKind_PreCheckRefusesBeforeCommittingABlob
E-7	Saturated global cap sheds the write-path dispatch and meters dropped	TestEnqueueRebuildAfterWrite_ShedsWhenSaturated
E-9	Rebuild fencing a package unpublished since its dispatch: no fence, refused upsert, no cache row and no attachment left behind	TestForceExpireNpmMetadata_RefusesAFenceOnATombstonedPackage
E-10	A fence that no longer matches is refused at the pre-check, before the upsert	TestRebuildStoreKind_ThreadsTheTargetAndFenceToBothFenceChecks/a fence that no longer matches is refused
E-8	One package beyond its sub-quota sheds its own dispatches instead of crowding out others	TestEnqueueRebuild_PerPackageCapShedsBeyondTheSubQuota

Security considerations

#	Concern	Tests
S-1	The token read, rotation, and cache upsert are all namespace-scoped, so no fence can be taken or matched across namespaces	TestNpmMetadataFileStore_ForceExpireNpmMetadata/does not expire rows in a different namespace; every new fixture seeds its own namespace
S-2	The row lock the fence rests on is invisible to behavioural tests — dropping FOR UPDATE leaves every sequential test green	TestPackumentRebuildTokenStmt_LocksThePackageRow pins the generated SQL
S-3	A transposed argument could silently disable the fence: the identity is grouped into datastore.NpmPackageRef / rebuildTarget so a miscall is a compile error, and one double now checks the arguments it receives rather than accepting anything. RotatePackumentRebuildTokenTx carries NpmPackageRef too (9c23b6ac): it was the one path where a transposition failed open rather than safe, since its zero-guards both pass, the UPDATE matches no rows, and rotating zero rows is documented as not an error	TestRebuildStoreKind_ThreadsTheTargetAndFenceToBothFenceChecks
S-4	No change to authentication, authorization, or any request-facing surface: the token is internal cache state and is never serialized into a response	n/a

Ran locally: go test and go test -tags=integration for internal/datastore and internal/format/npm/..., go test -race ./internal/format/npm/..., golangci-lint clean, and the full scripts/conformance/npm-e2e.sh harness against a local AR + MinIO on the CI-pinned Node 24.18.1 — every scenario passes, including ==> npm deprecate marks only the targeted version.

e2e scenario catalogs

No change. docs/testing/e2e/ currently covers only docker.md and oci.md; there is no npm catalog to update. The behaviour is already exercised by the existing conformance:npm:s3-garage deprecate scenario — this MR makes that scenario deterministic rather than adding one.

MR size

3394 added and 427 removed lines excluding generated jet types and structure.sql, well over the 500 ceiling in docs/dev/development-model.md. A large share of the added Go lines are comments.

An earlier version of this section claimed the change could not be split because "the datastore contract change and its callers cannot land separately". That was only half true and this file's own comments said so: the retire and the fence are independent, and neither subsumes the other. The retire half — enqueueRebuildAfterWrite, five one-line call-site swaps, and its tests — is roughly 150 LOC with no datastore contract change, and on its own it fixes the flaky npm deprecate assertion that motivated the MR. It was separable when the MR opened.

It is not being split now, and the reason is sequencing rather than indivisibility. The fence half has since been rebuilt on a schema migration (npm_packages.packument_rebuild_token), and the per-package sub-quota that bounds what the retire costs is part of the fence work's review response. Landing the retire alone at this point would mean shipping the concurrency increase without the bound that makes it safe, then reverting and re-landing the rest.

What is genuinely novel, for reviewers deciding where to spend attention: the rebuild-token fence and its lock ordering (RotatePackumentRebuildTokenTx), the fence pre-check that keeps a refused rebuild from leaking a blob, the per-package sub-quota, and enqueueRebuildAfterWrite. Much of the remainder is mechanical churn from threading one changed parameter through every call site and test double.

Related to #460 (closed)