fix(npm): stream metadata documents so they carry their JSON type

Why

Under DELIVERY_MODE_REDIRECT an npm packument read answers 307 to a signed object-storage URL, and the stored object carries Content-Type: application/octet-stream. Every storage driver writes that type at commit time (internal/storage/driver/gcs/gcs.go:491, internal/storage/driver/gcs/writer.go:275, internal/storage/driver/s3/s3.go:1439) and the blob layer has no channel for a per-object media type (storage.BlobUserMetadata carries SHA-1 and nothing else), so a redirected document can only ever be served under it.

A packument is JSON and a client is entitled to check the media type before parsing. The 307 arms set no Content-Type at all, which is why requesting the abbreviated form changed nothing: the request never reaches the code where the two media types differ.

Merge gate: the ADR-005 amendment

namespaces.delivery_mode_override and storage.delivery_mode stop reaching the four npm metadata routes. That is a deviation from ADR-005, which is handbook-owned, so per AGENTS.md it needs an amendment there.

The amendment merged on 2026-09-07: handbook!20993. It scopes the two axes to artifact content (container layers and blobs, Maven artifacts, npm tarballs) and records that format metadata documents are always streamed. The mechanism it gives is that a pre-signed storage URL serves the object's stored Content-Type, every driver writes application/octet-stream at commit time, and the blob layer has no per-object media-type channel, so only a streamed response can declare the JSON type. The reason it gives for streaming rather than storing a correct type is that content type is a property of how a format interprets a blob and not of the blob itself, which is why blob_storage_blobs carries no content_type column; deduplication is what turns that into a constraint, because a commit that finds the object already present skips the write, so the first format to write the bytes owns the object's type from then on. It also adds the negative consequence (neither axis is observable on a metadata read) and the two alternatives rejected in its place: the response-content-type override on the signed URL, and setting the correct content type on the object at write time.

The amendment covers the OCI manifest deviation from !991 (merged) as well as this one, so it closes both rather than only the npm half. That was the point of not tracking a second one separately.

The gate has cleared. handbook!20993 merged on 2026-09-07, and the adr-conformance thread that carried the gate on this MR is resolved. Nothing here waits on the ADR amendment any more; the remaining blocker is review approval.

What

Four routes had the defect, not the one reported: the hosted packument (packument_get.go) and dist-tags (disttags.go) reads, and their two kind=remote twins (remote_packument.go, remote_disttags.go). All four now open with documentBlobOpenOptions, which appends storage.WithForceStream to whatever ADR-005 resolved. The force outranks both of that ADR's axes (internal/storage/pg_blobstore.go resolves it first), so a metadata document streams on every deployment and the handler declares the media type.

The option is added per call site rather than inside blobOpenOptions, because that helper is shared with download.go and remote_tarball.go. The two tarball routes keep both axes: a tarball's bytes are opaque, so application/octet-stream is the right type for them and the redirect is worth keeping. That split is what the new rdmRoute.servesADocument field asserts on the remote side, and download_test.go's existing 307 cases on the hosted side.

The force needs a HEAD branch, on all four routes

Forcing the stream regresses HEAD. All four routes are served for HEAD through their GET pattern (net/http.ServeMux matches one for the other, as handler.go:548 records), and none of them branched on the method, so a HEAD read the whole cached document out of storage and handed it to net/http, which discards every byte of a HEAD response body. Before the force, a redirect-mode HEAD cost one URLFor and answered 307 with no object I/O.

The fix follows the pattern the hosted tarball route already uses at download.go serveHead. On the hosted side, HostedDocumentRequest.HeadOnly makes the cache arm resolve the row's blob through BlobInfo instead of OpenBlob and return a new HostedDocumentHeadersOnly delivery; each route answers 200 with the validator, directive, media type and length its GET carries, and armWarmReadBodyDeadline leaves on it, so nothing is armed. On the remote side, serveRemoteCached branches ahead of result.Blob.Reader(), writes Content-Length from result.Blob.Size(), and skips streamRemoteDocument.

Three placements are load-bearing rather than incidental. The branch sits after the If-None-Match match, so a conditional HEAD still answers 304 off the row alone and reaches storage for neither bytes nor length. It sits before the open, which is the point. And presence is settled by BlobInfo rather than by the row's digest, so a HEAD cannot report a 200 with a Content-Length for a document whose GET would fail; an absent blob takes the same two kind arms the open path takes, so the dist-tags map self-heals through the fill and a packument fails closed.

HeadOnly also rides into buildInline's own read-back, since that arm hands the same request to fromCacheRow. A HEAD that misses the cache therefore still pays for the fill -- that is what commits the row -- but reads no object back afterwards.

This is the same fix internal/format/oci took for manifests in !991 (merged), and it was chosen there over a response-content-type override on the signed URL for reasons that still hold: that option needs the URLFor(ctx, path) interface, both drivers, three URL middleware decorators and CDN query-parameter forwarding to change.

The redirect arm is removed, not left dead

HostedDocumentRedirect and HostedDocument.RedirectURL are gone, both hosted handlers lose their case, and cachedDelivery and serveRemoteCached fail closed on a redirect instead of serving one, mirroring errRemoteCacheRedirect in internal/format/oci. Serving the 307 would be the defect; a 500 is what says the storage layer disagreed with the option it was handed. Nothing outside internal/format/npm referenced either symbol.

What this does not reach

Hosted Maven maven-metadata.xml keeps both axes and can still answer a 302. It is a metadata document with the same defect, but it is served by the ordinary Maven download path (internal/format/maven/download.go), which opens with openOptions and calls serveRedirect on a redirect blob. This MR does not touch it, and docs/dev/configuration-reference.md now says so rather than claiming no metadata document reads either setting.

The kind=virtual metadata routes are orInterimStub today, so the defect cannot reach them; the S31 virtual read path will need the same force when it wires h.virtualPackument and h.virtualDistTags. Nothing fills WithVirtualPackumentHandler or WithVirtualDistTagsHandler outside tests, so they carry no force and are not a fifth and sixth forced route.

A remote HEAD on the live-upstream arm still reads the body through io.EOF. serveRemoteUpstream is left alone deliberately: reaching EOF is what commits the cache fill, so a HEAD that stopped short would abandon the fill and leave the next reader to pay for it again. The same holds on the hosted side for the fill itself, as the HEAD section above says.

Hosted Maven maven-metadata.xml keeps its HEAD shape too, for the same reason it keeps both delivery axes: this MR does not touch that route.

What this gives up

CDN offload on warm metadata reads, which is most of the metadata reads there are: a cold read already streamed, so the redirect covered the common arm, not a residue. What makes the loss small is not the arm split but the deployment — the cloudcdn and cloudfront middleware are real but are not enabled in config.example.yaml, so on the shipped configuration the redirect target is object storage rather than an edge. The service therefore takes on the egress that went direct from GCS before, bounded per document by npm.max_package_json_size x npm.max_versions_per_package, and the warm body write deadline !2306 (merged) added is what keeps a large one from being truncated.

The ..._inline_build_document_bytes histograms lose their redirect observations; a fill that fails closed on a redirect now observes nothing, because it delivered no document.

Diff size

1238 insertions, 489 deletions across 28 files, measured at 0eceb0aba. Counted as a sum it is 1727 reviewable LOC, over the 500 bar docs/dev/development-model.md sets, so it needs the justification that document asks for. (Earlier revisions of this description gave 654, then 750, then 1703. The first two were the sum before the HEAD branch landed, and 1703 the sum before this round's test and docs fixes; re-deriving at each push is what moved the figure, not a correction to the earlier arithmetic.)

The split: 153/116 across 8 non-test files under internal/format/npm/, 889/286 across 12 test files in the same package, and 196/87 across 8 Markdown files (packument_cache.md, metrics.md, remote_document_serve.md, S11, S15, configuration-reference.md, observability.md, e2e/npm.md).

Splitting would not help, and the shape says why: 1458 of the 1727 lines are tests and prose, against a production change of 153 added and 116 removed.

The diff carries two facts, and the second is the first one's own regression: a metadata document streams, so a HEAD for one must not pay for the stream. Both are asserted at every site that used to say otherwise. Landing the force without the tests would leave the precedence unpinned; landing it without the HEAD branch would ship a per-request storage read for zero delivered bytes on all four routes, which is why that branch is here rather than in a follow-up; and landing either without the docs would leave four documents describing an arm the same MR deleted.

Test plan

Acceptance criterion Test
Hosted packument forces the stream TestPackument_CacheHitForcesTheStream
Hosted dist-tags forces the stream TestDistTagsHandler_CacheHitForcesTheStream
A hosted redirect is a 500 on both kinds TestHostedDocumentReader_Read_FailsClosedOnARedirectPerKind
That 500 arms no write window of its own the same test's w.writeDeadlines assertion; inverting it reddens all three kinds
A hosted redirect never leaks the signed URL TestPackument_CacheHitRedirectFailsClosed, TestDistTagsHandler_CacheHit_RedirectFailsClosed
A fill that re-reads a redirecting blob fails closed a fill whose committed blob redirects fails closed and arms once
Neither ADR-005 axis reaches a hosted metadata shape TestHostedMetadataReads_ForceStreamOutranksEveryDeliveryMode, all four diagonals
The same on the cache-miss arm TestHostedMetadataReads_MissArmSelectsTheServedShape
Neither axis reaches a remote metadata shape, and the tarball keeps both TestRemoteReads_ForceStreamOutranksEveryDeliveryMode, three routes x five combinations
A remote cached redirect is a 500 TestServeRemoteCached_RedirectFailsClosed
The forced open still threads the override TestHostedDocumentReader_Read_PassesBlobOptionsThrough
A warm HEAD measures once and opens nothing, on all three kinds TestHostedDocumentReader_Read_HeadMeasuresAndOpensNoBlobPerKind
That HEAD arms no write window of its own the same test's w.writeDeadlines and w.armAttempts assertions
A conditional HEAD still answers 304, and measures nothing TestHostedDocumentReader_Read_HeadStillAnswers304PerKind
A HEAD on a gone packument blob fails closed on both kinds TestHostedDocumentReader_Read_HeadOnAGonePackumentBlobFailsClosed
A HEAD on a gone dist-tags blob self-heals, and still opens nothing TestHostedDocumentReader_Read_HeadOnAGoneDistTagsBlobSelfHeals
A failed measure names its own stage TestHostedDocumentReader_Read_HeadMeasureFailureNamesItsStage
The hosted packument route's HEAD carries its GET's headers and no body TestPackumentHEAD_AnswersFromTheMeasureAndOpensNoBlob, both kinds
The same route's conditional HEAD is a 304 off the row TestPackumentHEAD_ConditionalStillAnswers304
A HEAD never reports a 200 for a document its GET could not serve TestPackumentHEAD_BlobMissingFromCASIs500
The hosted dist-tags route's HEAD carries all six of its GET's headers TestDistTagsHandler_HEAD_AnswersFromTheMeasureAndOpensNoBlob
The same route's conditional HEAD is a 304 off the row TestDistTagsHandler_HEAD_ConditionalStillAnswers304
A cached remote HEAD reads no byte of the object and still closes it TestServeRemoteCached_HeadDeclaresTheLengthAndReadsNothing
Both remote twins answer a HEAD that way end to end TestRemoteDocumentReads_HeadServesTheLengthAndReadsNoObject, two routes

hdmShapeStore did not resolve ForceStream; it now does, the way PgBlobStore.effectiveMode and the remote fake already did, so the fixture answers the precedence the production path implements.

  • go test ./internal/format/npm/... green.
  • golangci-lint run ./internal/format/npm/ (pinned 2.13.2, isolated cache, --max-same-issues=0 --max-issues-per-linter=0 --uniq-by-line=false): 0 issues.
  • scripts/ci/check-comment-caps.sh --base origin/main: OK.
  • markdownlint-cli2, lychee, vale over every changed Markdown file: pass.
  • npm conformance is a merge gate on this MR, not a pre-Draft one: the MR left Draft before conformance was reported green, so keying the obligation on that transition read as satisfied when the gate is still open. It has to be green before merge. The reported failing row is npm.http.content-type.

Docs

docs/specs/S11-npm-hosted.md and docs/specs/S15-npm-remote.md lose their metadata 307 response rows and the validator prose that hung off them; remote_document_serve.md and packument_cache.md are rewritten where they described the redirect arm, and each gains a section on what a HEAD costs (## What a HEAD costs on either hosted metadata route and ## What a HEAD costs on a cached serve); docs/dev/configuration-reference.md names the three response shapes that read neither delivery_mode setting and says maven-metadata.xml is not one of them; docs/testing/e2e/npm.md gains e2e.npm.consume.metadata-content-type and e2e.npm.consume.metadata-head, and its delivery-mode-override row now asserts the split rather than the old uniform behaviour.

Neither spec changes for the HEAD branch. The observable response is the same one HEAD already promised -- same status, same headers, no body -- so no S11 or S15 statement is falsified by it; what changed is only what the service reads from storage to produce it.

setDistTagsHeaders and cacheControlForVisibility each carried a doc comment far past the one-line cap an unexported symbol gets, and both named the 307. Editing either in place would have forced the whole block to that cap, so the rationale moves to packument_cache.md under ## The response headers a hosted metadata success carries.

Closes #1128 (closed)

Closes #1125 (closed)

Edited by Dzmitry (Dima) Meshcharakou

Merge request reports

Loading
Loading