fix(npm): arm the body write deadline on a warm hosted metadata read

Stack

Merge in this order. Part 2 is based on part 1's branch and will not apply cleanly before it lands.

Order	MR	Closes	What
1	this MR	#1002 (closed)	Arm the body write deadline on a warm hosted metadata read
2	!2307 (merged)	#1128 (closed)	Stream metadata documents so they carry their JSON media type

Why

A hosted npm metadata read that hits the cache never re-armed the connection write deadline. It wrote its whole body under the deadline Go armed when the request headers were read, server.timeouts.write, which is 10s in config.example.yaml. Only the cache-miss arm re-armed (armInlineBuildResponseDeadline, then armInlineBuildBodyDeadline from !2283 (merged)).

A document large enough, or a client slow enough, to need more than that got a truncated 200. The status line and headers are already committed, so the client reads a short body as a complete one. With a 168 h packument cache TTL a warm read is nearly all hosted metadata traffic, so this is the arm that carries the load, and its budget was the smaller of the two.

This is live today on any instance whose effective delivery mode is proxy.

What

armWarmReadBodyDeadline arms the stream delivery of a cache hit at

readyAt + max(server.timeouts.write, inlineBuildBodyWriteBudget(size))

reusing the fill's own derivation, which is what #1002 (closed) asks for so the two arms cannot drift on the rate, the grace or the cap. The redirect and 304 deliveries arm nothing: a Location header is the envelope the server's budget already covers, and a 304 resolved no body to size a window for.

The floor is the part worth reviewing. SetWriteDeadline replaces rather than extends, and a cache hit has no first arm to take a max against, so arming the raw budget would cut an ordinary 100 KiB read from 10 s to 5.195 s. The floor is the configured server.timeouts.write itself, threaded from the composition root through mountSlugAnchoredFormats and buildNpmDispatcher into WithPackumentServerWriteTimeout and WithDistTagsServerWriteTimeout. Because it is exactly the value the server armed from, and readyAt is never earlier than the header read, readyAt + floor is never earlier than the instant on the connection: the arm is provably never-shortening at any configured value. That is what separates it from armRemoteReadResponseDeadline, which still documents the shortening asymmetry because it has no configured value to floor against.

An earlier revision of this MR used a hardcoded 30 s, matching the highest server.timeouts.write any config in this repo sets. GitLab Duo pointed out that nothing tied the constant to the configured value, so a deployment above 30 s was shortened by the arm meant to widen it; 4a168bd10 is that finding closed.

Which reads the drain still severs depends on the term that set the window. An in-flight write gets server.shutdown.delay plus server.shutdown.timeout once shutdown starts, 25 s at the config.example.yaml figures, and LabKit closes live connections when that wait runs out. A floor-bound read now carries server.timeouts.write, 10s in that file and under the drain's wait, so one that began before the drain started reaches its own deadline first and does book a kill. A budget-bound read loses that guarantee only above 10.00 MiB, where the derived budget first passes the 25 s wait, and then only while more than 25 s of it is still unspent when the drain starts: from 2.50 MiB up to that edge it still reaches its own deadline first. Above it a large warm read is severed with its window still open and the ending takes the uncoded write Warn. The fill arm's 65 s and its derived instant outrun the same drain, so that exposure is not this arm's alone. packument_cache.md prices it, metrics.md no longer claims a flat series is evidence for every warm truncation, and closing it means sizing server.shutdown.timeout against the budget, which nothing in this package can do.

What the arm costs in held connections. A stalled client on a warm streaming read holds its connection for at least server.timeouts.write, which is what every shipped config already gave it, and up to the 5 min cap on a large document. The plan priced that worst case for the fill arm, which is reached once per package per 168 h TTL and serialized behind collapseInlineBuild; this arm is issuable per request. There is no connection limiter and no in-flight cap in internal/server, and ADR-004's rate limiting is not built, so in steady state nothing bounds the count of these holds; the cap on a single hold is the only ceiling. The drain is not a second bound: it runs only once shutdown starts, so server.shutdown.delay plus server.shutdown.timeout cut a hold at 25 s during a rollout and cut nothing between rollouts. Once ADR-004's per-IP, per-user and per-namespace limits land they are what bounds the count, and this paragraph needs rewriting against them; the widening is recorded on the ADR-004 limits audit, #279.

What threading the value cost. buildNpmDispatcher receives config.NpmConfig rather than the whole config, so the value travels as its own time.Duration argument through mountSlugAnchoredFormats and buildNpmDispatcher, plus an option on NewPackumentHandler and NewDistTagsHandler. A primitive crossing those boundaries is the accepted smell; mountSlugAnchoredFormats already threads upstreamRetryAfter the same way. hostedWarmBodyDeadlineFloor stays at 30 s but is no longer any deployment's floor: parseDurationField rejects a non-positive server.timeouts.write, so the constant is unreachable through config and guards an unwired caller's zero instead.

Two behaviour changes a reviewer should weigh. First, a small document on a 10s deployment is now held for 10s where the constant held it for 30s. The constant existed to avoid shortening rather than to add headroom, so this is the over-approximation going away rather than a guarantee being dropped. Second, kill_outcome="budget_exhausted_before_body" becomes reachable on the warm arm, which metrics.md and observability.md called out of reach outright. It needs both terms of the max: a server.timeouts.write at or under inlineBuildResponseGrace and a document whose length storage did not report, which leaves the window equal to the grace. No config here reaches it, and a positive size adds a rate term that clears the grace whatever the timeout. Both documents now carry the condition.

TestWarmArmKillOutcome_IsAlwaysTheOverrunValue still pins the overrun value across six sizes at the package floor, and TestWarmArmKillOutcome_BudgetValueNeedsALowFloorAndNoLength pins each term of the reachable case.

The kill counters gain document_source. Both carried kill_outcome alone, and every warm kill books body_write_overran at the write timeouts this repo's configs set, where most fill kills also land. Once this arm began arming cache hits the series therefore held two populations with no dimension between them, and at a 168 h TTL the warm one is the larger. document_source is read off HostedDocument.Source at the increment, which is the same value the Error line beside a kill already carried, from the same document at the same site, so the series and the line cannot disagree. Both labels are pinned in internal/metrics/cardinality.go; the budget is 6 series per counter.

### The kill_outcome label also sent an operator to ..._document_bytes to judge whether size was in play. documentBytes.Observe runs in buildInline alone, so that histogram has no warm observations at all and a flat top bucket there is the absence of samples rather than of large documents. The section now says to split on the arm first, what the histogram does and does not cover, and that a warm kill's size is only on the Error line's size_bytes, which is rationed to one line per repository per minute per replica where the counter is not, so it reads as an existence check rather than a distribution.

Accepted smell: the series keep their ..._inline_build_deadline_kills_total names, which are still narrower than the population they count. A label is not a rename: adding the dimension leaves the external Grafana dashboards that already read these names working, which is why the label was taken over the rename, and the Help text, metrics.md and observability.md continue to say the names are narrow.

A failed warm arm is logged and not metered. It opens the same blind spot the fill's first-arm counter names, but on a read that ran no fill, so a tick there would report a fill arm that never ran.

Diff size

1169 insertions over 34 files, past the 500 reviewable-LOC line docs/dev/development-model.md draws. Re-derived at 2ca449673 against merge base 414c074de. Split by group:

Group	Added	Files
Tests	689	19
Docs (.md)	303	6
Production (internal/)	164	6
Wiring (cmd/)	13	3

Splitting would not help here. The production change is 164 lines and cannot be staged: threading server.timeouts.write to the arm means the argument, both options and the arm's own signature land together or the tree does not compile. The 13 wiring lines are the composition-root call sites for that one argument. The document_source label is in that group too, across internal/format/npm/metrics.go, internal/format/npm/hosted_stream_record.go and internal/metrics/cardinality.go. What makes the total large is the other two groups, and neither is separable either.

The 686 test lines are the mechanical testServerWriteTimeout argument across 12 wire test files, six new tests in internal/format/npm/hosted_document_read_internal_test.go (five for the floor and one for the kill-outcome term the change opens), and the shared assertBooksBothDocumentSources both routes call for the new arm label.

The 303 doc lines are four facts corrected everywhere they were written down: the 30 s constant, what the kill counters can and cannot separate, the drain wait as server.shutdown.delay plus server.shutdown.timeout rather than the timeout alone, and the size conditions on which warm reads the drain severs and on what the warm arm adds to the hold count. The per-file added counts are internal/format/npm/metrics.md (145), internal/format/npm/packument_cache.md (134), docs/dev/observability.md (3), docs/plans/2026-08-31-npm-hosted-fill-body-write-deadline.md (19), docs/testing/e2e/npm.md (1) and docs/dev/configuration-reference.md (1). Leaving any of them stating the old fact is the stale-claim failure this repo's own docs rules name. The sixth .md in the diff, the plan file, carries the earlier commit's note and not this one's.

Test plan

Every assertion added here was checked by breaking the implementation one property at a time and confirming the right subtest reddens:

Mutation	Reddens
Drop the floor, arm the raw budget	the cache arm floors a small document at the server's own budget
Never arm on the warm path	both cache-arm subtests
Arm on every warm delivery, 304 included	the cache arm's 304 arms nothing
Record the instant without arming the connection	both cache-arm subtests
Guard on redirect-or-304 instead of on stream, so the error return arms	the cache arm's error return arms nothing
Arm the package constant instead of the floor argument	TestWarmArm_FloorsAtTheFloorItIsGiven, ..._ReadArmsTheConfiguredFloor
Return the constant from warmBodyFloorFrom at every input	TestWarmBodyFloorFrom_TakesTheConfiguredWriteTimeout
Drop the reader's option loop	TestHostedDocumentReader_ReadArmsTheConfiguredFloor
Drop the reader's floor seed	TestHostedDocumentReader_WarmBodyFloorDefaultsToTheConstant
Drop either GET constructor's option loop	that route's ..._ThreadTheWriteTimeoutToTheWarmArm subtest

TestHostedDocumentReader_Read_ArmsTheWriteDeadlineForAFillOnly is renamed and its the cache arm does not subtest rewritten rather than extended, which #1002 (closed) calls for by name. The rewritten cache-arm cases then moved out to TestHostedDocumentReader_Read_ArmsTheWarmBodyDeadline in 43a9c6ae7, because both arms in one function put it past the maintidx cap; the rename #1002 (closed) asks for stands, and the fill arm keeps the name. ..._RecordsTheArmedInstantOnEveryFillArmDelivery is renamed too, and its the warm arm leaves it zero subtest inverted.

hostedWarmDocument in packument_get_test.go carried a zero WriteDeadline, which is no longer the shape a warm read produces; it now carries a real instant, so those read-side ending tests stay off the kill arm on their own evidence rather than because nothing was armed. The blob-fault cases have that evidence in the tracker's readErr; assertACleanWriteFailureTakesTheWriteWarn is deliberately a clean read, so there the open window is the only thing holding it off the arm, and the two helpers' assertion messages now say which term operates in each rather than sharing one.

armWarmReadBodyDeadline's doc.Delivery != HostedDocumentStream guard is also what keeps a fromCacheRow error from arming, since Read hands the arm the zero document on every cache error but the dist-tags self-heal. The new the cache arm's error return arms nothing subtest is what makes that half of the guard falsifiable.

• go test ./internal/format/npm/... green.
• golangci-lint run ./internal/format/npm/ ./internal/metrics/ ./cmd/artifact-registry/ (pinned through mise, isolated cache, --max-same-issues=0 --max-issues-per-linter=0 --uniq-by-line=false): 0 issues, at 2ca449673. An earlier revision of this branch was red on maintidx for TestHostedDocumentReader_Read_ArmsTheWriteDeadline, and that is what the golangci_lint job was failing on. The finding was this branch's, not main's: the function is at the merge base as TestHostedDocumentReader_Read_ArmsTheWriteDeadlineForAFillOnly, and this branch renamed it and grew it to nine subtests across two arms. 43a9c6ae7 splits the cache arm's four into TestHostedDocumentReader_Read_ArmsTheWarmBodyDeadline, preserving every subtest and every assertion, with no //nolint.
• scripts/ci/check-comment-caps.sh over the whole branch diff: OK.
• markdownlint-cli2, lychee, vale over every changed Markdown file: pass.
• The same command with --build-tags=integration reports 76 issues across internal/format/npm and cmd/artifact-registry, none on a line this branch touches: the eight integration test files here changed only by gaining the testServerWriteTimeout argument, at lines the report does not name.
• gofmt -l names internal/format/npm/disttags_test.go, which this branch does not touch and whose merge-base copy is named too.

Docs

packument_cache.md gains ### Why the warm arm takes a floor and the fill arm does not, and its Two arm failures section becomes Three arm failures. metrics.md and docs/dev/observability.md move the warm arm from "cannot raise the kill counters" to "raises them, as body_write_overran at every write timeout this repo's configs set", and both now name the two terms that reach the budget value instead. metrics.md gains ### The document_source label, and its series table carries the second label and the 6-series budget. docs/testing/e2e/npm.md gains e2e.npm.consume.warm-read-slow-client, the twin of the cold-read row.

docs/dev/configuration-reference.md's server.timeouts.write row said the hosted npm metadata reads "re-arm only on the cache-miss build path" and that "a read the cache answers arms nothing and is still bounded by the value set here". Both are false once this arm lands, and that row is the only place an operator sizes the key from, so it now carries what the cache arm arms, that the arm floors at the value set there and so never shortens it at any setting, and the drain bound above. The guardrail that would have caught the staleness is scoped to internal/config/**, proto/artifactregistry/config/** and config.example.yaml, none of which this MR touches.

docs/plans/2026-08-31-npm-hosted-fill-body-write-deadline.md gets a note, not a rewrite: its ### The warm arm carries the same defect, and is out of scope section is the record of why step 2 stopped where it did, so the note says the tree has moved past it and names the renamed test rather than editing the scoping decision away. Two passages that note does not reach ("a warm read arms no deadline", "the warm arm still arms nothing") move to past tense and name #1002 (closed).

HostedDocument.ReadyAt said it was "set wherever WriteDeadline is", which a failed warm arm falsifies: the instant is stamped and the deadline is not. packument_cache.md now carries which arms set both, and the field points there.

Closes #1002 (closed)