fix(npm): one cache-event derivation for both remote read paths

Merge order

This MR was hard-gated behind two MRs that touch the same files. Both have merged, and this MR has been rebased onto main past them; the table records the order and what each one settled.

# MR Why it goes first State
1 !2188 (merged) Rewrites the head of internal/format/npm/remote_document_serve.md, whose intro this MR also edits when it moves the cache-event section out. It also edits docs/specs/S15-npm-remote.md, at line 908, which none of this MR's three hunks there reaches merged
2 !2187 (merged) Edits docs/specs/S15-npm-remote.md and internal/format/npm/metrics.md, both of which this MR edits. Its spec hunk appending at line 1275 lands inside this MR's ## Resolutions hunk, which is the one overlap in the table that collides on its own. It also edits docs/dev/observability.md, at line 299, away from the counter row this MR rewrites merged
3 this MR this MR

Neither was a stack: all three target main and carried their own diff base. The order was a conflict-avoidance order, not a compile-order one. The rebase hit the two collisions the table predicted, both in Markdown and both resolved by keeping each side's own addition: internal/format/npm/remote_document_serve.md keeps !2188 (merged)'s copy-classifier section alongside this MR's move of the cache-event section out to remote_cache_event.md, and the S15 spec's ## Resolutions keeps !2187 (merged)'s version-field entry alongside this MR's remote.ReadObserver one. No Go file conflicted.

What this fixes

remote.ReadObserver gains a third method, ObserveServe, which reports each settled remote.ReadDelivery.ServeCached: the verdict the flow ran it under, and the error the delivery reported. remote.StandaloneFlow calls it at all three of its serve sites, ahead of the switch that classifies the error, so an observer reads the outcome the delivery reported rather than what the flow made of it.

Before it existed, the two npm remote read paths answered the same question two different ways:

  • the metadata routes inferred a failed cache serve from the remote.ServeSource the read resolved plus a fetch that happened to follow it, which cannot separate a row whose bytes had vanished from one served intact;
  • the tarball route was its own ServeCached and booked from inside it.

Both now feed one derivation, remoteReadCacheState (internal/format/npm/remote_cache_event.go), and book its answer once the read resolves. The metadata routes' inference and the tarball route's two in-ServeCached bookings are both gone.

The shapes that change

Five shapes booked a different npm_remote_cache_events_total{event} on the two paths. Each now books one value, the one the S15 spec's ## Resolutions entry and remote_cache_event.md call defensible.

Shape Booked before Books now
a cache-fallback serve of a row the first lookup missed miss (tarball) / hit_stale (metadata) hit_stale
a fresh row whose vanished blob the unhealthy-remote gate refused to refill hit_fresh (metadata) / miss (tarball) miss
a cache-fallback row that had also lost its blob (remote.ErrFallbackBlobVanished) hit_stale (metadata) / miss (tarball) miss
a 304 whose re-read row had gone, recovery served the upstream hit_stale (tarball) / miss (metadata) miss
a 304 whose re-read Lookup failed hit_stale (tarball) / revalidated (metadata) revalidated

The metadata routes were wrong on two of the five, the tarball route on three.

Tests

  • internal/remote: TestStandaloneFlow_Deliver_ObservesEveryCacheServe pins one observation per ServeCached, under the verdict the flow passed, carrying the sentinel the delivery reported. The vanished-blob recovery test now also asserts both serves are observed, and the nil-observer tests in both standaloneflow_test.go and standalone_test.go drive all three sites in one read.
  • internal/format/npm: the derivation table gains a serves column and three rows. Two are the metadata shapes above, the ones whose booked value changes. The third, stale row whose revalidated row had gone is a miss, pins the half of the fourth shape that reaches no serve at all, where the metadata answer is unchanged but the arm that reaches it is new; the sibling row stale row whose revalidated blob vanished is a miss covers the other half. The tarball footprint table gains three rows for the tarball shapes.

Both halves were falsified, not just asserted. Disabling the derived booking in the tarball route's resolve fails exactly six rows of the footprint table, the three new ones included. Disabling the vanished-blob arm of remoteReadCacheState.event fails exactly four rows of the derivation table, the two new ones included. Two of those four used to be answered by the fetch-behind-a-fresh-row proxy, so they now depend on the real signal rather than on its stand-in.

One subtest was dropped, deliberately: a source outside the domain is treated as unresolved covered cacheEvent's default arm against a remote.ServeSource outside its domain. The method no longer takes a source. The equivalent guard for the new input has a row of its own, a serve delivered under an undecided verdict answers from the row, which covers the remote.CacheServeInvalid arm the same way.

//nolint:maintidx was added to the tarball footprint test, measured: the three added rows push its Halstead volume past the threshold, and the linter reads a data table as complexity. Splitting the table would break the one-read-one-footprint property the test exists to assert. The paralleltest token already on that line is left alone because it is load-bearing, measured the same way: paralleltest is enabled (.golangci.yaml sets linters.default: all and does not disable it, and no exclusion rule covers it), and dropping the token reports the missing t.Parallel() on the function and on the t.Run range. An earlier revision of this description called it inert repo-wide; that was wrong and unmeasured.

Docs

The derivation's prose moved out of remote_document_serve.md into a sidecar of its own, internal/format/npm/remote_cache_event.md, because the derivation is now shared and a tarball reader would never look in the metadata-serve file. Four passages that asserted the defect in the present tense were closed together, per the plan-fix rule:

  • docs/specs/S15-npm-remote.md ## Dependencies: the S13 row, and the ratified-interface list, which now carries the read observer;
  • the same file's ## Resolutions entry, which said "until that lands" and "books the wrong value today";
  • internal/format/npm/metrics.md, "The four cache-event values", which described the fold being resolved from the serve source;
  • docs/dev/observability.md, whose counter row said hit_fresh covers a refused recovery and that the withdrawal "keys on a fetch having followed the row".

docs/plans/2026-08-28-npm-remote-observability.md gains two sentences: its declined alternative "extend ReadObserver with a verdict method" was adopted later, outside that plan, and its "all eight shapes" claim about the derivation table is now "every shape". No Status-table row is touched.

S13 conciliation

remote.ReadObserver is internal/remote surface, so the spec asked for conciliation with the S13 DRI before this MR opened. I am co-DRI of S13, so the ratification is recorded where the spec keeps it: the serve outcome is now on ## Dependencies' ratified-interface list rather than in the prose of seams outside it. docs/specs/S13-virtual-remote-foundation.md describes no observer at all, so no S13 spec amendment is owed. Per the development model's "change in place, cc the DRI", the other S13 co-DRI is the right reviewer for that half.

Diff size

1,022 reviewable LOC (additions plus deletions), past the 500 the development model asks about. Splitting was weighed and rejected: remote.ReadObserver has two implementers in one package, so the interface change and both consumers cannot compile apart.

Group LOC
Production Go: the interface, its three call sites, the shared derivation, the two routes 317
Tests 403
Docs and sidecars 302

e2e scenarios

No scenario is added or affected. docs/testing/e2e/npm.md catalogs client-visible protocol behaviour, and this change moves no bytes and no status: every shape above answers the client exactly as it did before, and only the event label a read books changes. The shapes are covered by the unit tables instead, each row naming the remote.StandaloneFlow arm that produces it.

Related to #986 (closed)

Edited by Dzmitry (Dima) Meshcharakou

Merge request reports

Loading
Loading