feat(npm): the per-request memo adapters 2/3 (S31 plan: 11/19)
The stack
Step 11 split from !2194 (closed), in merge order. Each part compiles, carries its own tests, and is reviewable alone.
| # | MR | What | Reviewable LOC at the split |
|---|---|---|---|
| 1 | !2196 (merged) | the two binding-missing orphan shapes, in internal/virtual and internal/remote |
1,045 |
| 2 | !2197 (merged) | the per-request memo adapters and the fall-through counter | 1,108 |
| 3 | !2198 (merged) | the loading slice that installs both | 2,104 |
!2194 (closed) is closed in favour of these. The split itself changed no production Go: virtual_assembly.go, virtual_memo.go, metrics.go and the four foundation files arrived byte-identical, and the split only moved shared test fixtures into virtual_testsupport_test.go and split the one contract sidecar into virtual_memo_contract.md and virtual_assembly_contract.md so that no part cites a file a later part introduces. Commits made since the split have changed virtual_memo.go on this part, so the LOC column above is the split-time figure rather than each part's current diff; the Size section below is current for this one.
The three are not equal in size, and cannot be without breaking something worth more. At the split part 3 was 49% of the stack, because its two remaining seams both resist a cut: shipping virtual_assembly.go without virtual_assembly_test.go would violate the tests-in-the-same-MR guardrail, and splitting the assembly itself passes a half-built slice across an MR boundary, which the plan's own Seam interrogation bullet rejects by name. Parts 1 and 2 were within 6% of each other.
Merge gate for the whole stack: !2191 (merged) corrects the plan in eight places where this implementation contradicts it, and should land first so a reviewer can tell an accepted trade-off from an oversight.
What this part does
Two adapters and a flight seam, each closing one gap between a process-lifetime foundation seam and the fixed statement count a virtual read has to hold.
-
VirtualHealthStatusReaderanswers a health read from a per-request map on the context and falls back to the durable store on a miss.virtual.NewResolverfixes its health seam for the whole process, so there is no per-request slot for a prefetched map, and an implementation carrying its own request scope is what the signature does admit. Without it a 20-upstream repository pays 20 sequential health round trips inside oneResolve, on the path annpm installwalks once per dependency.The map keys on presence, not on a non-zero value.
last_health_statusissmallint DEFAULT 0 NOT NULL, so a matched binding always carries a verdict and an unknown one arrives as an explicit0; readingHealthUnknownas "not in the map" would fall through for every unprobed position and answer correctly, at one statement each. The statement count is the only symptom, which is what the counter is for.WithVirtualHealthStatusesstores a copy of the map it is handed, so nothing a caller writes after the call can change a verdict already on the context or race a read of it. Anilmap still clones tonil, which keeps "the context carries no map at all" reaching the store. -
VirtualRemoteFetcherresolves its position's flight from the registry on firstFetchand caches it for the request, so a denied or skipped position never takes the registry's process-wide lock. It shortens no credential's lifetime, and nothing here claims it does.It also builds the fill-time rewriter, one per position. The fetch options are request-global, so a rewriter built once per request carries whichever position's base it was built on into every position's fill, and this is the innermost place a position's own identity is still in scope. The cached row is shared with the standalone remote route, so a collapse there writes one position's base into another repository's document.
Its constructor refuses four wiring defects: a nil
Flights, a zeroRepo.NpmRemoteRepositoryID, a zeroNamespaceID, and an emptyRewriteBaseURLon a metadata target. The namespace is there because it is the other half of the keyFlightRegistry.Flightbuilds on and the value that scopes the flight's cache store, so a nil one aliases cached documents across namespaces that share a binding id, reported by nothing.
Plus gitlab_artifact_registry_npm_virtual_health_memo_misses_total. A path that forgets to populate the map still answers correctly and silently returns to one statement per position, so nothing else makes that regression visible. It is registered into npmCollectors, because a collector outside that slice never reaches /-/metrics. It is a sixth npm_virtual_* metric, outside the five the spec's Metrics table names; a spec amendment rides behind it and gates nothing.
Size
274 production, 998 test, 306 Markdown added (1,578 reviewable), measured as added lines against the merge base. virtual_testsupport_test.go holds the fixtures this suite and part 3's share, so neither depends on the other's test file, and virtual_remote_upstream.md drops the M sequential health reads from its per-resolution accounting, which is the cost this memo removes.
470 of that total is review response, and it is why the number moved past the 1,108 this MR opened at. Six refusals were added where a signature could not carry the constraint -- a zero NamespaceID, an unvalidated Target, a path that disagrees with that target, an ExpectedDigest alongside the fill-time rewrite, plus the nil-store and zero-value-receiver contracts on the health reader -- each with the case that fails when the guard is removed. The rest is the shared-flight composition test AGENTS.md asks for on a remote.SingleFlight seam, the defensive copy of the health map, and the sidecar reasoning for all of it. Splitting the guards out would ship the seam without them, which is the state the review found.
e2e scenarios and surfaces
No scenario added or affected: docs/testing/ has no npm virtual catalog and the plan assigns the harness and catalog entries to Step 19. Nothing in this stack is reachable from a route, so there is no client-visible behaviour to script yet. No migration, no SQL, no config or proto change, no route, status code, header or response shape, and no api/openapi or api/bruno change.
Related to #892 (closed)