fix(npm): attribute a metadata copy's ending to the side that failed

What this fixes

recordRemoteDocumentCopy decided how a finished npm remote metadata copy is booked from one input: whether the source read returned an error. With no byte count and no look at the error's own shape, four endings were attributed to the wrong side. It now takes the count and the declared length and orders its arms the way recordCopyOutcome (download.go) and recordRemoteTarballCopy already do, so no narrower ending is absorbed by a looser test below it.

No response behaviour changes. Every one of these runs after the status is already on the wire.

A cache-fill failure is no longer charged to the upstream

remote's tee attaches remote.ErrCacheFill only on the read that also reports end-of-body, so every origin able to reach this call site (a staging write or flush on the final chunk, the terminal Commit, the post-Commit row upsert) failed with the source having handed over the whole document under a committed 200. It booked upstream_unavailable at Warn; it now books nothing and logs at Error.

This is the row that matters operationally. Every cold metadata read commits a fill, so an object-store or datastore brownout flipped essentially all cold packument reads to "the upstream registry is unavailable" while this service's own storage was what failed, and npm_remote_upstream_responses_total booked ok for the same reads because observeFetch had already run.

remote.ErrCacheFill's doc asks for exactly this split: a client-facing SLI "has to split on which stage failed" rather than treat the sentinel as one thing.

A truncated cached read is no longer silent

serveRemoteCached declared Content-Length from result.Blob.Size() and then discarded the count. io.Copy reports an early clean end-of-body as a nil error, so a short blob read booked nothing and logged nothing while the client stored a truncated packument under a header that promised more. It now books the branch's fault code, as both siblings do.

A fault after the final byte is no longer a server fault

The mirror image. With the declared count met and only a trailing probe faulting, the client has every byte. It booked internal_server_error; it now books nothing and logs at Error, which is what both siblings do and for the same reason.

A stream past its declared length is no longer client churn

net/http refuses a write that would cross the declared Content-Length whole, returning http.ErrContentLength with the read error nil. That ending fell past every read-side arm and landed on the generic write-failure arm: Warn, no outcome code, and a comment calling it a disconnect. The client held a prefix of an object that outgrew the size its blob row records, and npm_request_total counted the read a success. recordCopyOutcome (download.go) and recordRemoteTarballCopy each give this ending its own arm above the count arm, and this one now does too. It is the one arm whose outcome ignores the count, because even a full count sent a prefix of something longer.

Each ending writes its own log line

The message used to be fixed per call site, so all the arms shared it. Five of the eight book nothing and four of those still write a line, so for those four the line is the whole signal. Two of the four were both Error, both carried an error field, and both carried the same message, so a failed cache fill and a source fault after the final byte could only be told apart by substring-matching remote: cache fill unsuccessful inside error_message. Which branch served the copy now travels as body_source, the field recordRemoteTarballCopy already emits, carried by a new remoteDocumentSource that fixes the fault code with it. Both siblings give each arm its own message for the same reason.

A committed fill is no longer dropped from npm_remote_cache_fill_bytes

serveRemoteUpstream gated observeRemoteCacheFill on the joined copy and close verdict. The fill has already committed by the time Close runs, so the gate is now the copy error alone.

What an operator sees during the deploy

No metric name, label name, or label cardinality changes, and the set of Code values is unchanged. What moves is which ending books which value, so during the rolling deploy both mappings are live at once and any panel over npm_request_total{handler="remote_packument"} or the npm_remote_proxy outcome shows a step rather than a clean cutover.

Log volume moves with it: the pre-image logged every failed ending at a single Warn, and five of the eight arms now log Error, unrationed, one line per failed request. That matches docs/dev/logging.md and both siblings, and recordRemoteTarballCopy likewise leaves its own mid-stream source fault unrationed. Worth knowing before an Error-rate alert reads it as new.

One ending is deliberately left without a metric. A failed cache fill books nothing on npm_request_total, observeFetch has already booked the upstream response ok, and the fill gate skips npm_remote_cache_fill_bytes because the copy errored, so a brownout is read off this arm's own message against a fill-bytes count gone to zero. The Maven artifact relay (maven/remote_artifact.go) and the OCI blob relay (oci/remote_blob.go) answer the same ending the same way, so giving the stage a counter is worth doing across the three surfaces at once rather than only here. remote_document_serve.md records that.

Testing

Test-first: the failing tests land in the first commit and the fix turns them green.

• TestServeRemoteUpstream_ACacheFillFailureIsNotAnUpstreamFault covers the code, the level, and the arm's own message.
• TestServeRemoteCached_ClassifiesTheCopyByItsDeclaredLength drives serveRemoteCached itself over all seven reachable endings, asserting the outcome code, the level, and the message on each. The test it replaces re-implemented the function's last two statements, so deleting the recordRemoteDocumentCopy call from the function left it green.
• Three of those rows exist because the declared count made them load-bearing. An ordinary disconnect and a refused write are both short under a declared length, so only the arms above the count arm keep them off the error rate, and a stream past the declared length has to outrank both. httptest.ResponseRecorder enforces no length and never fails a write, so the last two drive the recorder through rdsShortWriter and a new rdsOverlongWriter.
• TestServeRemoteUpstream_ObservesAFillItsCloseThenFailed covers the fill gate. I confirmed it pins the gate rather than passing incidentally: restoring the joined-verdict form fails it.

go test ./internal/format/npm/ ./internal/format/oci/ ./internal/remote/ passes. golangci-lint (pinned 2.13) reports 0 issues on ./internal/format/npm/... ./internal/format/oci/... under default tags, and nothing in the touched files under -tags=integration. scripts/ci/check-comment-caps.sh --base origin/main passes over the whole branch diff.

E2E scenarios

No scenario in docs/testing/ is added or affected. Every change here is to a metric label, a log level, or a log message, all of them recorded after the status and the body are on the wire, so nothing an end-to-end client can observe moves. e2e.npm.remote.tarball-truncated-relay is the closest neighbour and covers the tarball route, whose behaviour is untouched.

Reviewable size

593 LOC across 6 files (515 added, 78 removed), past the 500 ceiling, so per docs/dev/development-model.md here is the split and why it is not worth cutting further:

Group	Files	+	-
Production Go	remote_document_serve.go	108	21
Tests	remote_document_serve_test.go	250	9
Sidecar prose	remote_document_serve.md, remote_read_errors.md	155	3
Comment move, no code	remote_read_errors.go	1	44
Spec	docs/specs/S15-npm-remote.md	1	1

The reviewable substance is the 129-line classifier. Tests and prose are 417 of the 593, and the prose is only that large because the caps in docs/dev/go-style.md hold an unexported doc block to one line and an inline block to two, so the arm table and the reasoning have to live in remote_document_serve.md. Splitting the tests from the arm they pin would land a red pipeline on the first half; splitting the sidecar from the code it describes would land the arm table before the arm.

The remote_read_errors.go row is the one group that could be a separate MR. It carries no code change at all: correcting a single stale word inside a 39-line unexported doc comment pulled that whole block into the comment-caps gate, whose cap for one of those is a single line, so the prose moved to remote_read_errors.md beside it. Happy to lift it out if you would rather.

Notes for the reviewer

The remote.ErrCacheFill arm is deliberately written once rather than split across the two callers: a cached blob's reader cannot produce the sentinel, so the arm is unreachable on that branch. If that ever stops being true, the arm becomes wrong for the cached caller, which is why it is called out here and in the sidecar.

remote.ErrLeaderCanceled is knowingly folded into the fault code rather than given an arm. followerLeaderError keeps context.Canceled out of the chain on purpose, so a follower truncated because the leader's client hung up does not match the cancellation arm. Its read did fail, so leaving the success series is right; only the label is arguable. The sidecar records it.

internal/format/oci/remote_relay.go cites npm's copy classifier as a precedent for logging at Warn, and this change makes that half false. It is left alone deliberately: that package keeps no sidecar, and the one-clause correction would force a 17-line exported doc comment to its three-line cap, which is a change of its own rather than a line of this one.

Found by a branch review of the step-2 follow-up to !2076 (merged), which is where this landed. The five review findings this MR then picked up are the http.ErrContentLength arm, the shared log message, the two missing short-count tests, the missing level assertions, and the two documents that still promised the Warn.

Related to #1033 (closed)