docs(specs): amend S31 with the cross-upstream packument merge

What this MR does

S31 specified a winner-only packument: the document a virtual repository served was the lowest-position upstream's, and cross-upstream merge sat in ## Follow-ups. The product decision in !2018 (merged) reverses that for every format, so this amendment makes the merge S31's specified behavior.

The two npm metadata routes now collect every eligible upstream and serve the union. The tarball route is unchanged and stays single-winner, because a tarball is opaque bytes with nothing to union.

S31 is not a closed-beta blocker, so there is no winner-only first: the merge is the first implementation, delivered through the existing S31 step issues.

This is the npm half of that decision. The container half is !2240, already open.

The merge rules

Field Rule
versions Union; a key held at several positions takes the whole object from the lowest position. Never merged field by field.
dist-tags Union; lowest position wins each tag name, latest included and never synthesized.
time Union; lowest position wins a key. modified is the maximum across contributors, created the minimum.
dist.tarball Re-pointed at the virtual repository on every emitted entry, whichever position it came from.
dist.integrity, dist.shasum Relayed byte-for-byte from the contributing position. Never recomputed, never reconciled.
_id / _rev The requested package name / dropped, on the full variant. A merged abbreviated packument carries neither, nor any top-level field outside name, dist-tags, and versions, matching S11's projection.

GET /-/package/{package}/dist-tags merges by the same dist-tags rule, so the two surfaces cannot report different tags for one package in one upstream state.

Collection semantics mirror S30's: no Probe, every eligible position contributes, a failed or unhealthy position contributes its cached copy or is omitted, any contributor gives 200 with merged or merged_partial, all-absent gives 404, all-failed 503, and a data error vetoes the 404. One arm is not best-effort: a hosted position whose Lookup Hit is followed by a refused fenced rebuild vetoes the whole read with S11's own code, because answering from the remaining positions would serve a public remote's versions alone for a name the hosted repository demonstrably holds. Bounded by resolution.probe_concurrency, whose name is a tarball-route artifact the spec now flags where an operator reads it.

The merge streams: position 0's entries are emitted first and a later position's already-seen version key is never re-emitted, so versions never buffers. The duplicate is still read, because the divergence counter compares its digests, so the retained state is a version-key-to-digest-pair map rather than a key set. dist-tags, time, and the top-level scalars are buffered and emitted after, because a value from the last position can still win. No Content-Length, for the reason the re-point already required one to be omitted.

The ETag is derived from the merge inputs (algorithm version byte, then per position position, upstream_repository_id, and that input's blob SHA-256), not from the served bytes, which are never buffered. If-None-Match therefore answers 304 after the database reads alone. A HEAD is not that cheap: it runs the collection and the merge and discards the body, because an over-cap or unparseable input is only discovered in the parse, and that is what separates a merged_partial from a merged and, where it takes the last contributor, a 503 from a 200. That reverses two clauses of the ## Resolutions entry recording the old rule, which now carries an *Amended:* note: a merged document has no single blob to digest, and a metadata read carries a validator even when its only contributor was filled by a cold fetch. Both original clauses stand unchanged on the tarball route.

The correction this carries

S31 claimed twice that the merge removes the packument/tarball integrity mismatch, "because a merged document names one tarball per version". It does not. A merged document does name one version object per key, but the tarball read is still a separate single-winner resolution, and the re-pointed URL is uniform per ADR-009 and carries no position. So bytes can still arrive from a position other than the one whose dist.integrity the client holds.

The uniform URL is not on its own what leaves the mismatch open, and the spec says so rather than stopping there: two independent resolutions over one predicate would agree. The passage names the two mechanisms that actually make them disagree. They read different sources, since a position contributes the packument its metadata row holds while its tarball answer is a Lookup against npm_remote_files or npm_files and, on a remote position, a live Probe. And they run at different times, which is the arm the winner-change acceptance criterion already pins with a position health-marked unhealthy between the two reads.

What the merge does add is detection: gitlab_artifact_registry_npm_virtual_integrity_divergence_total counts a version key whose contributing candidates disagree on dist.integrity or dist.shasum, which is the dependency-confusion signal. The counter is not free, and the spec says so: it widens the streamed pass's retained state from a version-key set to a version-key-to-digest-pair map, roughly 135 bytes per key (95 for a sha512- integrity value, 40 for a hex shasum). Both fields are retained because either can diverge alone: dist.integrity is optional in a packument and the abbreviated variant an npm install requests is where most virtual metadata reads land, so a map holding integrity alone would sit at zero on the busiest routes and read as "no divergence" rather than "not measured".

Two configuration keys, owed to the S13 DRI

The fan-out cover needs resolution.negative_cache_ttl and resolution.position_wait_timeout. Both land on S13's shared virtual_repositories block rather than on the npm slice, for the reason resolve_timeout already sits there: each bounds the resolution rather than an npm leg. S32's merged collections need both; S30 asks for neither, and covers the same fan-out with a rate limit over the virtual read path instead, so this is S31 proposing an addition to the foundation rather than restating a need three slices already share. Their values are recorded as owed to the S13 DRI in ## Dependencies, so the step MR that implements collection is gated on that ratification rather than guessing them.

The gate carries its ceiling, because the DRI is choosing inside one. A metadata collection runs inside resolve_timeout, so the wave count times the per-leg bound has to fit under it. At today's defaults request_total_timeout supplies that on its own: two waves of 30s inside the 90s budget, whatever position_wait_timeout is set to, so the new key can only tighten the sweep and never break it. What does move is S13's own sizing text, which derives npm's worst case as 2 * 5s of bodyless probes and says in the same passage that the figure holds only while Phase 2 is how a resolution narrows its candidates. These routes run no Phase 2, so npm's worst case becomes 2 * 30s of full-body fetches. 90s still clears it, so nothing has to change today, and the stale npm line goes to the S13 DRI alongside the two values.

The negative cache itself is specified in full rather than by its TTL alone, because its key carries a client-supplied package name: it is in-process and per pod, bounded by an entry ceiling with least-recently-used eviction. The ceiling is a fixed per-pod value rather than a third key, because what it bounds is a process's heap rather than any resolution's behavior.

Reviewable LOC

1329 changed lines, 1048 of them insertions, measured at 71553bbae against the merge base e0de46676:

File + -
docs/specs/S31-npm-virtual.md 1047 280
docs/specs/README.md 1 1

Past the 500 ceiling docs/dev/development-model.md sets, and splitting would not help. The change is one reversal propagated through one document: the design section, the acceptance criteria, the error table, the observability family, the follow-ups, and seven ## Resolutions entries all state the same fact, and a split that landed any subset would leave S31 asserting winner-only in the sections it did not reach. That is the failure mode the amendment exists to close, and it would be live on main between the parts. The single README.md line is that same fact once more, outside the spec file.

What the second commit fixes

da66b5f11 closes review findings against b33399e1d, all of them sites the first commit did not reach or a mechanism it under-stated:

  • The integrity-mismatch passage was self-refuting. It concluded that an object from position 1 is fetched from position 0 "whenever position 0 also holds that name@version", which the merge rule two sentences above it forbids. It now names the two mechanisms that operate.
  • Three ## Resolutions entries still described the winner-only world, byte-identical to main: the ETag entry (which stated an implementable HTTP contract opposite to the amended body), the re-point entry, and the configuration entry, whose amendment note still counted one added foundation key where there are now three.
  • S13's cache-fallback paragraph claimed a dist.tarball re-point and a served_from_fallback outcome that this amendment made tarball-only.
  • Two acceptance criteria and two serving lines still said "the winner's" on a route that elects none, one of them contradicting the packument-merge criterion on the same fact.
  • A conditional metadata read had no resolution_outcome. This amendment barred the winner-named values from the metadata routes, and merged/merged_partial both assert a contributor; a 304 now records what the collection settled, and contributor_count is defined for it. (71553bbae narrows that bar from five values to four and names the 304's one blind spot.)
  • The divergence map retained one digest while the rule compares two fields, so the counter would have sat at zero wherever only a shasum is published. (The abbreviated-variant reason da66b5f11 gave for this is wrong, and 71553bbae replaces it.)
  • The negative cache had a TTL but no store, bound, or eviction, on a key carrying a client-supplied package name.

What the third commit fixes

71553bbae closes review findings against da66b5f11. One is a behavior reversal and the rest are claims that did not survive checking:

  • The hosted shed-rebuild arm asserted two opposite behaviors. da66b5f11 routed a refused fenced rebuild to a merged_partial, while the acceptance criterion, the ## Resolutions entry, the ## Error Cases intro, and the row's own heading kept the winner-only rule that vetoes the read. The veto wins, because its reason is untouched by the merge: a hosted Lookup Hit is authoritative, so answering from the remaining positions serves a public remote's versions alone for a name the internal repository holds, and the shed is seconds long and self-clearing. Fixed at every site, and the ## Resolutions entry keeps its ruling and gains an *Amended:* note for the route change rather than a reversal. Two collateral errors went with it: the row's heading called itself the tarball route's, when a tarball Lookup reads npm_files and runs no rebuild so that route cannot reach it at all; and hosted_rebuild_refused was barred from the metadata routes, which left the value unreachable on every route.
  • The DRI gate named no ceiling. It asked the S13 DRI to set position_wait_timeout without saying what it has to fit inside. The section above now states the constraint and re-derives S13's own npm figure, which this amendment makes stale.
  • The residual named the wrong bounds. It said the fan-out has "one read-level bound (resolution.probe_concurrency) and no per-position bound, which makes the slowest position set every metadata read's latency". probe_concurrency bounds concurrency, not time, and two time bounds do operate: request_total_timeout per leg and resolve_timeout per read. The residual is that a slow position is paid for 30s before it sheds, not that it is paid forever.
  • "S30 and S32 need the same two" was false of S30, which is approved on main with the same collection and neither key. Corrected for resolve_timeout too, which neither sibling references.
  • The negative cache claimed it "can never turn a merged into a 404". True only against an unchanged upstream state, which the same paragraph's own second sentence denies. A publish at a negatively cached position inside the TTL is now stated and pinned by a criterion.
  • HEAD skipped the merge, so it would answer 200 where its GET answers 503. It runs the merge and discards the body; the 304 keeps the blind spot and now says so.
  • The versions bullet said the retained state is a key set and a duplicate is "skipped rather than compared", which the divergence counter three paragraphs later contradicts.
  • Half the "retain both digests" rationale did not operate. It rested on the abbreviated variant dropping dist.integrity; S11's projection carries the whole dist object. The operative reason, that integrity postdates shasum, was already in the same sentence and now carries it alone.
  • A merged abbreviated packument would have emitted _id, which neither S11's projection nor npmjs's own abbreviated documents carry.

What this MR does not cover

#266 has ten checklist items. This MR is items 1 to 7, which are all of the spec text. Three remain, and none belongs in a spec MR:

  • Item 8, the shared seam. Collection mode and per-position records in internal/virtual are Go code that does not exist on main today (internal/virtual holds decision.go, resolve.go, result.go, rules.go, upstream.go and no collection mode). S30 books the same seam and has no plan on main, so this needs its own plan and MR.
  • Item 9, the step issues. #894, #896, #897, #899, #900, and #893 (closed) are tracker edits.
  • Item 10, merge order. A statement, not an artifact.

Separately, docs/plans/2026-08-21-s31-npm-virtual.md still describes Steps 13, 15, and 16 winner-shaped and is not in #266's checklist. It needs a companion docs(plans) MR, which this one does not open.

Merge order and file overlap

git diff --name-only against the merge base gives two files. Re-derived at 71553bbae against all 88 open MRs, every head ref fetched and none unchecked:

  • docs/specs/S31-npm-virtual.md: no overlap. No open MR touches it, so the spec file is conflict-free.
  • docs/specs/README.md: seven open MRs change that file in their own diff, and none touches the S31 row. Three merge cleanly with this branch: !2339 (merged) (adds a monolith S15 row), !2317 (merged) (edits the S17 row), and !728 (edits the S28 row). Three conflict with main as well as with this branch and so need a rebase whichever way this lands: !1011 (closed), !1009 (closed), and !1007 (closed), each inserting an S34 row. !1028 (closed) is stacked on !1011 (closed) and carries that row rather than adding one, so it is diffed against its parent and rebases with it; a base-relative scan counts it as an eighth and is double-counting the parent.
  • !2240 is the only one whose conflict is with this branch alone (git merge-tree exits 1 against this head and 0 against main). It edits the S32 row, which is the line immediately after the S31 row, so whichever of the pair lands second rebases one line. There is no semantic dependency in either direction and no required merge order.

The Step 12 stack (!2325 (merged), !2326 (merged), !2255 (merged)) touches only internal/format/npm/* and is deliberately built against the spec as it stands: #893 (closed)'s description records the four parts of that step this amendment re-opens. No merge order is required between this MR and that stack, in either direction. Every step issue that implements the merge sits downstream of Step 12 anyway, so the implementation cannot reach main before Step 12 does.

ADRs

Checked docs/adr/ for a governing decision. No ADR states winner-only semantics for virtual repositories, so no handbook ADR amendment is owed. ADR-009's uniform-URL rule is load-bearing for this change rather than in conflict with it: it is why the re-pointed tarball URL carries no position, which is why the merge cannot bind the tarball read to the contributing position.

Related to #266

Edited by Dzmitry (Dima) Meshcharakou

Merge request reports

Loading
Loading