chore(remote): give the fill's admission shed its own outcome (upload-session-concurrency-cap plan: 6/12)

What this does

Step 6 of the upload session concurrency cap plan.

streamAndFill used to fold every session-open failure into remote.ErrCacheFill. That sentinel is fallback-ineligible and every format maps it to 500, so a fill refused by the upload admission gate would have read as a broken cache. This gives the shed an outcome of its own:

• internal/remote/fetch.go: the shed travels bare out of streamAndFill, so a format arm can match the sentinel.
• internal/remote/errors.go: FallbackEligible excludes it by name. Stated rather than inherited: the default arm answers false for it today, and naming it is what holds if the sentinel ever travels joined to its spent wait budget, since context.DeadlineExceeded satisfies net.Error and the tail arm would then answer true.
• internal/virtual/resolve.go: the classification is recorded at fetchWinner's default: arm, so a shed does not join the ErrUpstreamUnavailable family and blame an upstream for local memory pressure.

Where the outcome is booked

Unwrapping the shed moves what the three kind=remote upstream-response classifiers would book, because each of them keys on ErrCacheFill and the sentinel no longer carries it. No step's Files list owns these three files, so the ruling is made here rather than deferred:

Package	After the unwrap, before the arms	Now
maven.classifyMavenRemoteUpstream	transport_error	cache_error
npm.classifyRemoteUpstream	booked nothing	cache_error
oci.remoteUpstreamOutcomeFor	booked nothing	booked nothing, for a corrected reason

The middle column is not measured against main, and against main this MR is metric-neutral for all three formats. On main streamAndFill wraps the shed in ErrCacheFill, so classifyMavenRemoteUpstream already books cache_error through its errors.Is(fetchErr, remote.ErrCacheFill) disjunct, which this branch does not touch; classifyRemoteUpstream already books cache_error for the same reason; and OCI already books nothing, since ErrCacheFill is not FallbackEligible either. The column reports the state inside this branch after 35b91310c unwrapped the sentinel and before the arms landed, and that state never reaches main. No maven series ever carried transport_error for a shed.

The arms are what keep it neutral. Without maven's, the unwrap alone would move a staging refusal to transport_error, blaming the upstream for this pod's refusal on the series an on-call reads to judge upstream health, which is the misattribution resolve.go's arm exists to avoid. Without npm's, the unwrap alone would go against npm's own documented rule, that a cache-side fault on a read that did reach upstream books cache_error. OCI's answer was already right and only its reason was wrong: the comment said none of the refused values reached an endpoint, and this one does, so it now says the value books nothing because every value in the set names an upstream condition.

npm's exclusion-arm walk is now derived from the arm's source, the way fallbackExclusionSentinels already derives the virtual taxonomy in the same package. The hand-written list held five of the arm's nine, which is exactly why adding the sentinel changed what the package books while reddening nothing.

The npm kind=virtual mapping has no row

An earlier revision of this branch gave the sentinel a row on the virtual read's coalescing arm. That row is gone, and internal/format/npm/virtual_read_errors.go is now identical to main. Three things ruled it out, each from the plan's ### What this control has to expose to be operable:

• Retry-After: 1 is the synchronized retry that section refuses for this control. Everyone shed in one instant returns together and re-parks for the whole budget, and each retry also burns an upstream GET whose 200 is discarded.
• upstream_unavailable describes a position that could not be reached, and this winner answered 200.
• virtualUnavailable logs at WARN, which would make this the one request-path shed arm that logs per request, unsampled. The same section bars it: "no other shed path takes a WARN", and the five request-path arms stay silent.

The row also sat above the inline-build arms, so it shadowed them. inline_build.go:388 wraps a build's own cause with errInlineBuildTimedOut, so a hosted winner whose rebuild died on a staging refusal arrives carrying both sentinels and matched the row before inlineBuildOverBudgetErr. The virtual route then answered upstream_unavailable with Retry-After: 1 where the hosted route answers inline_build_timeout with the build's own value, breaking the parity virtualInlineBuildRefusal exists for.

So the npm arms own this condition. While no arm carries it the sentinel is listed beside claimedElsewhere with its owner, the mapping falls through to 500, and no client reaches that 500 because nothing calls uploadAdmissionGate.Acquire, so the sentinel has no producer. The arm is recorded in Step 8's scope on #1238 (closed), and #1241 (Step 11), which wires the acquire into storage.NewSession, must not merge before it. The value the arm will carry is the publish shed's upload_session_capacity_exceeded under a jittered window, not this file's fixed constant. docs/specs/S11-npm-hosted.md's ## Error Cases is where that answer is defined -- 503, a Retry-After of 35 seconds plus or minus 20% drawn per response over whole seconds, and a message naming no cap, no held count and no wait budget -- and it merged in df114dc4c, under this branch's own merge base. !2465 (merged) (Step 4) introduces the Go constant for it, not the policy.

Five doc claims this branch falsified

1. internal/remote/errors.md (new), and internal/remote/errors.go. ErrCacheFill's doc said Fetch wraps "every failure" of the pipeline at four stages, one of which is the session open, and that stage now has an exemption. The corrected sidecar runs five stages rather than four: the audit that text prescribes returns six wrap sites, and the final-chunk staging write (stagingWriteError, fetch.go:989) mapped onto none of the four. Three of the five carry a named exemption and two wrap every failure they can raise; operations.md's Read-surfaced group takes the same correction. FallbackEligible's "Not eligible" list enumerates its own arm and sat at eight values while the arm grew to nine. Nothing catches the second one: npm's derivation test AST-parses the arm, not the godoc.

Neither could be corrected where it stood. scripts/ci/check-comment-caps.sh caps an exported doc comment at three lines and checks any block the diff touches, so editing the 24-line and 61-line blocks fails the gate whole. Measured both ways. So they move to a sidecar beside health.md, corrected, with three-line pointers left behind. The marker arm's inline comment moves with them, because it said "see the marker bullet in the doc comment" and "the doc block above", and neither now points at anything.

2. internal/remote/errors.md and docs/dev/storage.md on the abandoned acquire. Both said an acquire its own caller abandoned answers that caller's context error instead, so it reaches neither the shed counter nor the 503. That does not hold for the fill. streamAndFill opens its session on context.WithoutCancel(ctx), so once the acquire sits inside NewSession the caller-live test at internal/storage/upload_admission.go:79 reads a context whose Err() is always nil and cannot fire. Only the bare sentinel can come back at that call site, and a client that hangs up while its fill is parked still books a shed on upload_session_admission_sheds_total, whose own comment says it counts budget expiries only and never an acquire its caller abandoned (internal/storage/metrics.go:362-363).

Nothing observes that yet, because no caller reaches Acquire. Closing it means acquiring on the live request context ahead of NewSession and passing the reservation down, the shape the npm rebuild already uses. The claim now names the exception everywhere it lives: internal/remote/errors.md, both docs/dev/storage.md rows (the ErrUploadSessionsBusy row in ## Supporting types and errors and the counter's row in the metrics table), and the counter's own comment at internal/storage/metrics.go:362-364. Each hands it to #1035, which owns the admission control. Only the counter half is claimed, because no remote-read arm maps the sentinel to a status yet; maven.dispatchHandler.writeSessionOpenError is the one arm that does, on the hosted upload path, and it is on main at b158b7653 rather than added here. The plan's own statement of the same fact is left alone, because a step MR does not edit the plan file; it needs the amendment #1035 carries.

3. internal/remote/operations.md (new), and internal/remote/operations.go. Operations.Fetch's interface contract said the session open on a 200 and the 304 bump "both wrap the exported [ErrCacheFill] sentinel". The session open no longer does. That block is the seam the maven, npm and OCI RemoteOperations compose against, so an arm author reading it to learn what Fetch can return matches ErrCacheFill and misses the shed, which is the defect this MR exists to prevent. The block is 53 lines inside an interface body, which the caps gate classifies as other with a cap of 2, so it moves to a sidecar beside errors.md and health.md, corrected, with a two-line pointer left behind. The sidecar states the wrap as it is: each of the two stages routes one value around it, the session open exempting storage.ErrUploadSessionsBusy and the 304 bump exempting ErrCacheEntryNotFound.

4. internal/virtual/resolve_errors.md (new), and internal/virtual/resolve.go. fetchWinner's doc partitioned on "a transport-level failure (everything else, the remaining security-cover rejections included) takes the cache fallback", and after the unwrap the shed is "everything else". The sentence had the two arms the wrong way round: the switch reaches default: only after remote.FallbackEligible has declined, so the fallback is the enumerated arm and default: is the catch-all. The arm is right and the sentence was wrong, so the fallback clause is not widened. Resolve's returned-surface list had a second problem: it enumerated "the remote package's error surfaces" and Resolve can now return a bare storage.ErrUploadSessionsBusy, which is not a remote sentinel at all. The list gains a fourth group for a sentinel propagated unwrapped from a package below remote. Resolve's block is 27 lines against the exported cap of 3 and fetchWinner's is 21 against the unexported cap of 1, so both move to a sidecar beside orphan_upstream_shapes.md, with pointers at the caps. Membership of the fallback-eligible set stays in internal/remote/errors.md rather than being copied a fourth time.

5. internal/format/npm/metrics.md, and internal/format/npm/remote_metrics.go. classifyRemoteUpstream's doc said "Two of those three faults are booked as cache_error rather than dropped, since a fill that could not commit is a fact about a read that did reach upstream". Three of four are booked now, and a fill the admission gate refused before it opened is not one that could not commit. The corrected passage gives the reason that operates for the whole set instead: the three share the read, which reached upstream and got a usable answer, and what failed is this pod's handling of it. The count stays at the four values this arm names, so ErrUpstreamDigestMismatch, which is unnamed there and predates this branch, is not pulled in. The doc is 36 counted lines against the unexported cap of 1, so it moves into the package's existing metrics.md with a one-line pointer left behind; the arm's own five-line head comment carried the same clause and took the same treatment. metrics.md:575 was the second copy of the characterization and is corrected in the same commit, its conclusion (a probe cannot reach cache_error) unchanged.

cache_error's member list, and one spec line left alone

docs/dev/observability.md's maven, npm and OCI rows each enumerate what cache_error covers, and none named the shed. All three now do. The OCI leg also takes the reason internal/format/oci/remote_metrics.go corrected on this branch: an upstream did answer the staging refusal, so that leg books nothing because every value in the set names an upstream condition, not because nothing arrived. The booked value is unchanged against main in all three cases, so no series moves and no dashboard breaks.

docs/specs/S14-maven-remote.md:533 carries the maven parenthetical verbatim and is not touched. That line describes what the counter covered when S14 was written rather than fixing its membership, so it is descriptive rather than normative and no spec amendment is owed for it. That reading is a carve-out, so it is stated where the rule lives rather than only here: docs/dev/observability.md's ### A metric value's member list is enumerated in more than one row now carries it with this row as its worked example, and names the one-line spec amendment as settling the same case equally well.

Where the four conventions landed

main's ac3e84dc4 routed AGENTS.md's detail into lazily-loaded modules and deleted the region these four sections appended to, so the resolution takes main's AGENTS.md whole and puts each section in the module the Context Routing table names for its topic, converted from AGENTS.md title case to that module's sentence case:

Convention	Module
A metric value's member list is enumerated in more than one row	docs/dev/observability.md, under ## Authoring metrics
A doc saying a test reddens names every way to clear it	docs/dev/writing-discipline.md
A before-and-after table measures against the target branch	docs/dev/writing-discipline.md
An obligation recorded only in a commit body does not survive a squash	docs/dev/conventions.md

No destination already carried the rule, so each is a new section rather than a merge into an existing one, and each points at its nearest neighbour instead of restating it. The observability section additionally carries the carve-out for a spec row that records a value's membership at declaration time rather than defining it, so the rule and this MR's decision about S14-maven-remote.md:533 sit in one document. No routing row is added: all three modules already have one, and AGENTS.md grants a row only to a convention that opens a new module. ### Why MR titles matter gains one line saying squashing is per MR, pointing at the fourth section for the setting rather than repeating it.

Merge order

Nothing returns storage.ErrUploadSessionsBusy yet. Step 11 is the only producer and it depends on Steps 1 through 10, so no window exists in which a shed reaches a route with no arm. This MR is inert on its own.

• Depends on Step 2 (!2408 (merged), merged), which defines the sentinel.
• Blocks Steps 7, 8 and 9, the OCI, npm and Maven kind=remote arms. Each renders the outcome this MR creates, so each needs this merged first. Step 8 additionally owns the npm kind=virtual answer this MR declines to give.
• Independent of Step 5 (!2462 (merged)). Both target main, both depend only on Step 2, and their file sets do not intersect. Either may merge first.
• An S13 amendment is owed before Step 11. docs/specs/S13-virtual-remote-foundation.md:375 lists the CacheFill class as the 304 bump and the revalidated re-read, and its ## Error Cases table has no row for a local staging refusal. This MR squashes, so a commit body cannot carry that obligation past the merge; it is recorded on #1241 (Step 11), whose author is the one who needs it, with the npm kind=virtual arm it makes reachable on #1238 (closed) (Step 8).

Diff size

1,222 reviewable LOC at ace7fae01, past the 500 ceiling docs/dev/development-model.md sets. Derived as added plus removed lines from git diff --numstat $(git merge-base origin/main HEAD)..HEAD, split into prose (a Markdown line, or a Go line whose first token is //) and code off the same diff:

Group	Prose	Code	LOC
Sidecars: internal/remote/errors.md, internal/remote/operations.md, internal/virtual/resolve_errors.md, and the added half of internal/format/npm/metrics.md	468	0	468
The Go blocks those sidecars replace: internal/remote/errors.go, internal/remote/operations.go, internal/virtual/resolve.go, internal/format/npm/remote_metrics.go	270	7	277
Step 6 proper: internal/remote/fetch.go, fetch_test.go, errors_test.go, internal/virtual/resolve_test.go	37	147	184
Upstream classifiers: the maven, npm and OCI arms and their tests	44	94	138
The four conventions the review rounds asked to persist, in the modules that own them: docs/dev/writing-discipline.md, docs/dev/observability.md, docs/dev/conventions.md	82	0	82
npm virtual mapping (virtual_read_errors.md and the derivation test)	58	2	60
docs/dev/observability.md's three cache_error rows	6	0	6
docs/dev/storage.md	4	0	4
internal/storage/metrics.go	3	0	3

docs/dev/observability.md sits in two rows: 38 lines of relocated convention and 6 of cache_error row edits, which is the 44 its own --numstat reports.

The doc relocation is the first two rows: 745 of the 1,222, and 738 of those 745 are prose. The 7 code lines in the second row are the whole of the Go this MR adds to those four files, and they are listable: an import, a blank line and the storage.ErrUploadSessionsBusy exclusion in errors.go, and an import plus the two lines that put the same sentinel into npm's cache_error case list. Everything else in those two rows is comment text leaving a .go file and Markdown arriving in a .md beside it. Across the whole diff the split is 972 prose against 250 code.

Reviewing the four relocations as moves against the blocks they replace is the cheaper read. Their net new prose is 240 lines, on the same Markdown-line and //-first-token rule the table uses: 466 Markdown lines added and 2 removed across the four sidecars, against 247 comment lines removed from the four .go files and 23 added back there as sidecar pointers and new arm comments.

Six comment blocks moved rather than being corrected in place, across four files, each because scripts/ci/check-comment-caps.sh refuses the edit at the site and checks any block the diff touches:

Block	Counted	In file	Cap
ErrCacheFill (errors.go)	24	25	3
FallbackEligible (errors.go)	61	65	3
Operations.Fetch (operations.go, inside an interface body)	49	53	2
Resolve (resolve.go)	25	27	3
fetchWinner (resolve.go)	21	21	1
classifyRemoteUpstream (npm/remote_metrics.go)	36	41	1

Counted is what the gate compares against the cap: a bare // separator does not count and does not split a block, so it is the smaller of the two wherever a block has one. In file is the raw line span, which is what the threads above quote. Both measured at the merge base, b158b7653; the four files are byte-identical there and at a951ebf20, so merging main moved neither column. Measured each way. None of the six is split into its own MR, because each corrects a claim this branch falsifies, and a separate MR's reason would then live in this one.

The classifier group is not split either. Each of those three arms answers a value the unwrap would otherwise move, so the ruling and the change that forces it belong in one commit; splitting them out would put the unwrap on main with maven's arm still keyed on ErrCacheFill, and the misattribution would sit there waiting for the follow-up, unreachable only because nothing produces the sentinel yet.

Testing

New cases:

• TestFetcher_Fetch_UploadSessionShed_TravelsAsItsOwnOutcome: the sentinel survives, ErrCacheFill and ErrUpstreamTransport do not, the result is zero-valued, one admission attempt, no cache row, and the upstream body is still closed.
• TestFetcher_Fetch_Revalidation_UnderARefusingStore_StillServesTheCachedCopy: the 304 arm asks for no session, so a refusing store cannot fail it and the cached copy is still the answer.
• TestResolve_MultiPositionWinnerFillShed_AnswersTheShedNotAnOutage: the aggregate answers the shed, never an outage, a broken cache or a not-found, at exactly one fill attempt, and the losing position is never fetched.
• TestFallbackEligible: the shed, and the shed joined to a spent wait budget.
• TestMavenRemoteUpstreamOutcome_Classifies: the sentinel bare and wrapped, both cache_error.
• TestClassifyRemoteUpstream_BooksNothingForEveryFallbackIneligibleError: rewritten to walk fallbackExclusionSentinels, asserting cache_error for the three fill-side members and "books nothing" for the other six, and guarded by a require.Subset so a name that leaves the exclusion arm is reported rather than skipped.
• TestClassifyRemoteUpstream_MapsEveryAnswerToItsOutcome: two direct rows for the staging refusal, bare and wrapped the way the session open returns it, both cache_error and both counted. That derived walk reads its cache_error list only for names the derivation still produces, so npm needed the direct rows maven and OCI already carry.
• remoteUpstreamOutcomeFor's table: the staging refusal after a 200 books nothing, which is the member OCI's corrected comment rests on.

Each guard was broken in a scratch copy to confirm it reddens:

Mutation	Result
the bare-travel branch in streamAndFill	reddens the fetch case and the virtual case
the FallbackEligible exclusion	reddens TestFallbackEligible only
the maven arm points at another storage sentinel	reddens both maven rows
the npm arm points at another storage sentinel	reddens the derived subtest for the sentinel, and both direct npm rows
the sentinel swapped out of FallbackEligible's exclusion arm	reddens the require.Subset in the npm derived walk
the name dropped from claimedElsewhere	reddens the virtual derivation test

The second row is worth stating plainly: the exclusion is inert against every shape uploadAdmissionGate.Acquire produces today, because a single call returns either the bare sentinel or the caller's context error and never both. Its falsifying case builds the joined shape by hand. The arm is kept for the harm it guards against, and errors.md says so rather than implying a live producer.

Counts in virtual_read_errors.md were re-derived from the table by counting its source: fields, not decremented: 9 verdicts, 3 wiring sentinels, 6 derived, 1 named arm, 5 hosted refusals, 4 target faults. Nineteen resolution-side, twenty-eight in all.

golangci-lint (pinned 2.13, uncapped --max-same-issues, --max-issues-per-linter and --uniq-by-line) reports 0 issues on internal/remote/..., internal/virtual/..., internal/storage/..., and the three internal/format/... trees. No test here carries a build tag, so go_unittests is the job that runs them.

Scenario catalogs and conformance

No docs/testing/ scenario is added or affected. Nothing produces the sentinel until Step 11, so no client-visible behaviour changes on merge. The npm kind=virtual shed answer becomes reachable with Step 8, and the scenario belongs to it rather than here.

No conformance run: this changes error classification inside the remote fill, not npm, Maven or OCI protocol behaviour. This MR now decides no client-visible status at all, since the virtual row is gone.

Related to #1236 (closed)