chore(npm): answer a staging-refused read with the shed 503 (upload-session-concurrency-cap plan: 8/12)

Step 8 of the upload-session concurrency cap: the npm read routes answer a refused staging session as 503 upload_session_capacity_exceeded instead of letting it fall through to their 500.

Three arms, all keyed on storage.ErrUploadSessionsBusy through errors.Is, all reusing the publish route's code, message and jittered Retry-After rather than minting constants of their own:

  • writeRemoteProxyError — the packument and dist-tags routes.
  • RemoteTarballHandler.writeReadFailure — the tarball route, which reaches the shared mapping only on its received-refusal arm, so an arm added to writeRemoteProxyError alone would leave this route on its 500.
  • virtualReadAnswerFor — the kind=virtual winner's cache fill.

Both new writers go through envelope writers that already stamp Cache-Control: no-store, and all three log at WARN with the cause passed through redactedForLog.

What this does not reach

admission_source has six values and this step gives a status half to one of them, remote_fill, on npm's read routes. Two npm-adjacent gaps stay open and are named in docs/dev/storage.md's upload_session_admission_sheds_total row rather than left to inference:

  • npm_rebuild has no status half at all. openRebuildBlob opens that session and a refusal falls through to PackumentHandler.writeReadError's default 500. Step 10 (#1240 (closed), !2513 (merged)) owns it.
  • The virtual arm is written but unreachable, for two independent reasons: nothing calls uploadAdmissionGate.Acquire, and writeVirtualReadError has no caller outside export_test.go and its tests. Wiring either alone leaves the arm unreachable.

The virtual arm deliberately excludes errHostedDocumentInlineBuild. S31 splits the two producers of the sentinel across two rows: the winner's cache fill takes upload_session_capacity_exceeded, and a hosted position whose rebuild is refused takes the inline_build_* row. openRebuildBlob raises the same sentinel and HostedDocumentReader.fillCache wraps it errors.Is-transparently, so without the exclusion this arm would answer a refused rebuild with the fill's code. Step 10's fold does not fix that on its own, because it wraps errRebuildShed with a second %w and the sentinel stays matchable.

Merge order

This MR must merge before Step 11 (#1241), which wires the acquire into storage.NewSession. Step 11 arms the sentinel's only producer; until the read arms are in, a refused fill answers 500 with an ERROR record and no Retry-After. The order is recorded on #1238 (closed) and is not implied by the plan graph alone.

Two open siblings edit the same prose and conflict with this branch while merging cleanly into main, so the order between them matters and no pipeline reports it:

  • !2513 (merged) (Step 10) — conflicts in docs/dev/storage.md and internal/format/npm/metrics.md. The conflict is semantic: both rewrite the same per-format roster in storage.md, and !2513 (merged) narrows the two metrics.md passages to the publish path where this MR scopes them to the publish route plus a read route's cache fill.
  • !2514 (merged) (Step 7) — conflicts in docs/dev/storage.md. This MR writes "OCI has no arm yet"; !2514 (merged) lands that arm.

Whichever merges second should re-derive the roster against the merged text rather than resolving the markers, because either side taken whole ships a false statement.

The stack

Every step targets main; merge order is step order. Steps 9 and 8 have already merged out of order, which is why the order above is stated rather than inferred from the stack's existence.

Step Issue MR State
1 #1231 (closed) !2471 (merged) merged
2 #1232 (closed) !2408 (merged) merged
3 #1233 (closed) !2507 (merged) open
4 #1234 (closed) !2465 (merged) merged
5 #1235 (closed) !2462 (merged) merged
6 #1236 (closed) !2463 (merged) merged
7 #1237 (closed) !2514 (merged) open
8 #1238 (closed) this MR open
9 #1239 (closed) !2509 (merged) merged
10 #1240 (closed) !2513 (merged) open
11 #1241 not opened
12 #1242 not opened

Specs

  • S15-npm-remote.md — the staging-refusal row, which covers every standalone read route since packument, dist-tags and tarball all drive the fill.
  • S31-npm-virtual.md — the winner's-cache-fill row, and the adjacent hosted rebuild row this arm is kept off.
  • S11-npm-hosted.md — the code, the message rule, and the 35s ±20% window drawn per response, relayed unchanged.

Scope, against the plan's Files list

The plan scopes Step 8 to remote_read_errors.go and remote_tarball.go and argues "Shared seams: none" from that two-file set. The realized set is twelve files. The widening is deliberate and recorded on #1238 (closed): the scope note of 2026-09-10 gives this step the kind=virtual arm, which no step's Files list named. The rest follows from it — the sidecars that describe the arms, and the two comment-only sites (internal/format/npm/metrics.go, internal/metrics/cardinality.go) that carry the code's meaning. No new Code, no new label, and "code" is still 41 in both requestCodeLabels and closedSetValues.

The plan's "Shared seams: none" sentence is now stale, because internal/format/npm/metrics.md is in Step 10's set. A step MR does not edit the plan, so that sentence is left to the plan's single writer; the live consequence is the !2513 (merged) conflict recorded above.

Size

800 reviewable LOC, over the 500 the development model asks to justify, and 3.1x the plan's "~260 total" forecast for this step. The split is 54 production Go, 532 test, 214 docs. Splitting would not help: the production change is three switch arms and one writer, and what the diff is made of is the per-arm tests and the sidecar prose that pays for them.

Testing

Every arm has a positive case driven four ways (bare sentinel, the real production wrap remote: open cache-fill session: %w, a sentinel carrying a cause, and both route policies), a neighbour control that reddens if an adjacent arm moves onto the jittered window, and a budget test pinning the code against the request metric.

Falsified rather than assumed, each in a scratch copy:

  • Removing the virtual arm's errHostedDocumentInlineBuild exclusion reddens TestVirtualReadError_StagingShedLeavesTheHostedRebuildAlone.
  • Replacing uploadShedRetryAfterSeconds() with 35 in either new writer reddens its repeated-draw case. The shared bounds reader admits a flat constant by design, so the draw needed its own case on each writer.
  • Moving the shed arm ahead of writeIfClientClosed in the tarball switch reddens the departed-client case.

No e2e scenario is added. Guardrail 12 binds feat and fix; this is a plan-declared chore, the merged siblings !2465 (merged) and !2509 (merged) touched no catalog either, and the sentinel has no producer to drive a scenario with until Step 11 lands.

Closes #1238 (closed)

Merge request reports

Loading
Loading