fix(maven): re-arm the artifact PUT deadline on each read (upload-inactivity-deadline plan: 3/4)

What

Installs server.DeadlineReader on the Maven artifact PUT route and adds maven.upload_inactivity_timeout to size the gap it bounds. maven.upload_read_timeout stays the absolute ceiling every arm clamps to, and it is also the instant the write half is pinned at for the whole request.

The wrap is built below the blob.NewSession call, and only when the request declares a body, so the window bounds gaps between reads of that body and nothing else: not the session open ahead of the first read, and not a Content-Length: 0 deploy whose connection already carries net/http's background read. Both came out of review; the section below works them through.

Sibling of !2346 (merged) (OCI). Neither depends on the other. !2345 (merged), which both needed, has merged.

Expected on rollout. maven.upload_inactivity_timeout defaults to 5m, so every deployment that sets neither key now cuts an idle artifact PUT at five minutes where it previously ran to the 1h ceiling. That is what the step is for, but each cut answers 500 and writes one unsampled ERROR, and no counter ships with it, so the Maven PUT route's 5xx rate is the first place an operator meets the change.

Why

maven.upload_read_timeout bounds the whole transfer, so a client that stops sending mid-body holds a connection for the full hour. That is #1043 (closed).

This step also closes #776 (closed): the route now arms a write deadline of its own. extendReadDeadline arms both halves at now + maven.upload_read_timeout, and server.DeadlineReader pins the write half at that instant again at construction, so a body read that outlives server.timeouts.write can still write its status line. Maven previously moved the read deadline alone.

What the window covers, and what it must not

Two install-site properties came out of review, both about work charged to a window that was not measuring the client.

The wrap sits below the session open. server.DeadlineReader arms the read half at now + maven.upload_inactivity_timeout at construction, so anything between that arm and the first body read is charged to the client's window. The open is the one step that can outlast it: an upload_sessions INSERT plus a driver.Writer, which on S3 is ListMultipartUploads and CreateMultipartUpload under SDK retry. Built above it, a slow-but-successful open left the first read on an already-expired deadline, and carriedBytes is false before any read so no re-arm ran, which meant the error came back marked server.ErrInactivityTimeout: a healthy client booked as idle, the exact mis-attribution the marker exists to prevent. Two claims assumed the fixed ordering without stating an excursion, and are now true as written: internal/server/body_size.md's "every read opens with more than half a window that no caller work has spent", and the upload_inactivity_timeout row of docs/dev/configuration-reference.md ("between half this value and this value").

A bodyless PUT is not wrapped at all. internal/server/body_size.md works that precondition through and ends "So do not wrap a request that carries no body". On Content-Length: 0, net/http hands the handler NoBody and has already started the connection's background read, so the construction arm reaches handleReadErrorLocked and cancels the request context at now + maven.upload_inactivity_timeout rather than at the ceiling: under the 5m default and the 1h ceiling, twelve times sooner, and mid-commit. The deferred session.Cancel then runs on that cancelled context, so the abort can fail too and leave a staged object nothing reclaims. extendReadDeadline's ceiling arm is untouched, so an empty-artifact deploy keeps maven.upload_read_timeout as its transfer bound and loses only a gap bound over a body with no gaps. Chunked is not exempt: ContentLength is -1 there and body remains, so no background read is live and the wrap is built as before.

One cost of the first fix is bounded rather than accepted. With the wrap below the open, the post-handler drain on a refused open would read under the ceiling instead of under the fresh window the hand-back used to leave, so a client still holding the socket would hold it to maven.upload_read_timeout. Review asked what makes an hour acceptable there, and the answer turns on a correlation: an open is refused exactly when storage is degraded, so those holds accumulate through the outage rather than arriving independently, on a route with no connection limiter in front of it. armDrainWindow therefore arms the read half at min(now + maven.upload_inactivity_timeout, ceilingAt) on that one return, which is the bound the hand-back used to leave there, and skips a bodyless PUT for the reason the wrap skips it. net/http bounds the drain itself only where it can measure the remainder, giving up on an *io.LimitedReader with more than 256 KiB still declared and not on a smaller declared body or a chunked one, which are the two the arm covers. upload_deadlines.md carries the correlation as the reason the bound is in code rather than argued in prose.

Rebased onto the merged step 1, and what that changed

!2345 (merged) merged as a squash, so its commits are not ancestors of main and this branch carried its own copies of them. That is what the reported conflict was, and it cleared by rebasing off the step-1 tip onto main, not by merging main in, which would have left both copies.

!2345 (merged) also merged in a reviewed shape this branch was written before, and two of those changes are semantic rather than cosmetic. Neither raised a git conflict; the branch simply stopped compiling against the merged type.

• The write half is pinned at ceilingAt at construction and no later arm moves it, so the gap bound moves the read half alone. maven.upload_inactivity_timeout is therefore not a key that arms both halves, and the server.timeouts.write row of the configuration reference goes back to four keys, naming maven.upload_read_timeout in its place.
• RestoreCeiling is gone, replaced by FinishRead, which internal/server/body_size.md says belongs in a bare defer at handler scope: net/http drains the unread body from body.Close after the handler returns, so calling it at the end of the read arms a bound that can already have expired by then, costing a connection reuse. streamAndCommitPrimary drops both inline restores for one defer deadlineBody.FinishRead(), registered after the arm report so it runs first and the report covers its own arm. The NewSession early return is owed nothing on the write half, which has been at the ceiling since extendReadDeadline; the read half it leaves to the drain is armed by armDrainWindow, added in review and worked through above.

One behaviour improves as a result. An inactivity cut now writes its own response, because the write half is at the ceiling when the read half fires: session.ReadFrom returns the deadline error, writeStreamError maps it, and the 500 lands. TestUploadDeadline_StallPastTheWindowIsCut asserts that status alongside the marker on the log line, and the e2e scenario no longer asks for a log-only assertion. Before the rebase both halves fired at one instant and the route could not answer its own cut at all.

The conformance suite could not boot before this MR

upload_inactivity_timeout defaults to 5m, and validate compared that default against a configured upload_read_timeout. scripts/conformance/maven_setup.sh:221 sets upload_read_timeout: "60s" and names no window, so config.Load refused it and the Maven conformance suite could not start on this branch.

mavenFromProto now clamps the default to the ceiling; validate refuses only a value the operator explicitly wrote. The new test case uses 60s deliberately, so the shape that broke is the shape pinned.

Tests

The Maven deadline suite drives the window, the ceiling, the cut, and the #776 write-deadline case under synctest. Three cases moved with the rebase, because each asserted a property of the write half that the merged reader no longer has:

• TestUploadDeadline_WriteDeadlineOutlivesTheOriginal pins two ceiling arms that the read sequence outgrows, where it asserted the two arm sequences agree.
• TestUploadDeadline_ArmFailureNeitherFailsTheUploadNorRetries expects the deferred hand-back to clear the read half past EOF, where it expected a restore to the ceiling.
• TestUploadDeadline_SessionOpenFailureRestoresTheCeiling became TestUploadDeadline_SessionOpenFailureHandsBackTheReadHalf with the rebase, because nothing restores anything on that path. Review moved it again, to TestUploadDeadline_SessionOpenFailureNeverReachesTheWrapper: with the wrap below the open, nothing is handed back there either, and what it pins is the two read arms that return does make, the ceiling and then armDrainWindow's gap bound, alongside the 500 that still lands.

TestUploadDeadline_ExtendWarningNamesBothHalves, added in review, is still the case the extend warning's wording rests on, and review re-sized it. At two chunks three seconds apart the transfer ended inside the first ten-second window, so the arm a failed write half leaves standing was never met. It now drives mavenDeadlineLongChunks at the same gap and asserts the cut, the instant it lands on, the chunks that fit and the inactivity marker it carries. Those four assert today's behavior rather than the contract, and What this MR does not reach says why. Each reddens on its own inversion, on the old two-chunk size, and with writeArmErr dropped.

Two cases are new, one per install-site fix:

• TestUploadDeadline_SlowSessionOpenDoesNotSpendTheWindow is the mirror the suite lacked. It drives a 30s open that succeeds against a 10s window and asserts the first read is not cut, every chunk lands, and no line carries the inactivity marker. The open-failure case cannot cover it: that one drives the same 30s open refusing, so the first read is never reached.
• TestUploadDeadline_BodylessPutIsNotWrapped drives a Content-Length: 0 PUT and asserts extendReadDeadline's ceiling is the only read arm made.

Two more came with the drain bound, beside the refused-open case reworded above:

• TestUploadDeadline_RefusedOpenDrainBoundIsClamped drives the same refused open with the window set equal to the ceiling, the opt-out MavenConfig.validate accepts, and asserts the bound resolves to the ceiling rather than extending past it by however long the open took.
• TestUploadDeadline_RefusedOpenLeavesABodylessPutAlone drives a refused open on a Content-Length: 0 PUT and asserts the ceiling stays the only read arm, which is the wrap's own precondition applied to the new call.

Two more came with the deadline_arm field, which is what tells the one arm-failure message's two writers apart:

• TestUploadDeadline_ConstructionArmFailureNamesItsArm fails the wrapper's arm over an open that succeeds, so armDrainWindow never runs, and asserts the single line books construction.
• TestUploadDeadline_DrainArmFailureNamesItsArm fails the drain bound on a refused open, where the wrap is never built, and asserts the same message books drain.

Every fix was mutation-tested, before the rebase, again after it, again for the two install-site fixes, again for the drain bound, and again for the arm field. The three reconciliation mutations: dropping the deferred FinishRead reddens the arm-failure case; dropping extendReadDeadline's own write arm reddens the extend-warning and #776 cases; pinning the wrapper's write half on the window instead of the ceiling reddens the cut, open-failure and #776 cases. The two install-site ones: moving the construction back above the open reddens both open cases, and forcing the bodyless guard true adds the window construction arm and FinishRead's cleared arm, which reddens the bodyless case on both. The three drain-bound ones, each run on its own: deleting the armDrainWindow call reddens the refused-open and clamp cases, deleting its ceiling clamp reddens the clamp case, and deleting its ContentLength == 0 guard reddens the bodyless refused-open case. The two arm-field ones: booking construction from both call sites reddens the drain case, and dropping the deadline_arm attribute from the log call reddens both.

What this MR does not reach

internal/server/body_size.md names two internal/metrics counters that would make the wrapper's fail-open arm and its cut rate visible, and this step adds neither: an arm failure is visible only on the maven upload: arming the upload deadlines failed line, and a cut only as server.ErrInactivityTimeout inside the stream-failure line's error field. Two arms write that one message, the wrapper's construction arm and armDrainWindow's drain arm, so review asked for a field naming which one failed rather than a second message: deadline_arm carries construction or drain. Both records are unsampled, because internal/logging's sampler acts only on records carrying the access message, so a failure the whole ResponseWriter chain shares arrives as one record per upload request rather than as something to alert on.

The counters were written into body_size.md by !2345 (merged)'s review, after this branch was authored, and put on "the step that installs the wrapper". That pointed the obligation at an MR that was never going to discharge it, so this branch amends the sentence to name #1256 instead. #1256 is open and describes exactly this work: the same two counters, declared in internal/server for the same reason, with the plan scheduling no counter work at any of its four steps. internal/format/maven/upload_deadlines.md now refers to that one pointer rather than repeating the fact, per AGENTS.md's rule that a work-item pointer appears once per obligation. The amended paragraph is byte-identical to !2346 (merged)'s rewrite of the same paragraph, deliberately; the Stack section says why.

internal/server/upload_deadline.go also keeps a defect this branch's review found, deliberately unfixed here. The wrapper's write arm fires once at construction, ahead of the first read arm, and has no fall back of its own: when it fails the failure is recorded, the read arm after it still succeeds, and every later read then returns from the re-arm gate that reads that record. Whatever that one arm placed stands for the whole transfer. With a window inside the ceiling, which both defaults, config.example.yaml and internal/config/testdata/maven_happy_path.yaml set, it is a window arm: a client sending steadily is cut at one window and the cut carries server.ErrInactivityTimeout, booking an idle verdict against a client that never went idle. With the two equal, the opt-out MavenConfig.validate accepts, that arm resolved to the ceiling instead and the failure costs nothing. scripts/conformance/maven_setup.sh is that second shape, because it sets only upload_read_timeout: "60s" and mavenFromProto clamps the absent window down to it, so the conformance suite runs where this is inert.

The bug is on main, in step 1's file, so this branch did not introduce it. What it introduced is the upload_deadlines.md sentence asserting it cannot happen, and this is the first Maven caller that puts the state within reach. That sentence and body_size.md's "Only the read half is arming by then" are both corrected here, and #1258 owns the code fix, which is internal/server's to make rather than this route's; editing internal/server/upload_deadline.go here would pull the MR into step 1's territory.

No production wiring reaches the state. net/http resolves SetReadDeadline and SetWriteDeadline to the same net.Conn, so a chain that fails one fails both and leaves the ceiling in force with no window arm to strand. Separating the halves takes an http.ResponseWriter wrapper that resolves one and not the other, which only the test suite builds. That is also why no docs/testing/ scenario is added or affected by this correction, per guardrail 12.

Size

1714 reviewable LOC at 771b12597, added plus deleted, measured against this branch's merge base with main (7bd582891), which is the diff the MR shows today. gen/artifactregistry/config/v1/config.pb.go (32) is regenerated and is not counted; with it the diff is 1746. Eight earlier figures in this section are superseded rather than wrong, each re-derived at its own head rather than carried forward: 2418 was measured before the rebase off !2345 (merged), when the branch still carried !2345 (merged)'s own commits and its internal/server files; 1155 at 32ea7fe80, before the review-fix test work; 1242 at 1fc4798dc, before the two install-site fixes and their two cases; 1461 at 1adc4bb6e, before the write-arm doc correction and its test re-size; 1525 at 5ffdcf455, before the rebase onto 7bd582891 that resolved the two conflicts below; and 1512 at f5557bddc, before the documentation corrections of bac0e9062 and the drain bound of 1896885e0; 1625 at 1896885e0, before the review-fix documentation of fbe7e7e69; and 1638 at fbe7e7e69, before the deadline_arm field of 771b12597. Over the 500 ceiling docs/dev/development-model.md sets, so per guardrail 18, by file group:

Group	LOC	Files
Tests	1097	internal/format/maven/upload_deadline_test.go (963), internal/config/maven_test.go (110), internal/format/maven/upload_test.go (15), internal/format/maven/upload_stream_test.go (9)
Sidecar prose	423	internal/format/maven/upload_deadlines.md (344), internal/server/body_size.md (73), internal/format/npm/unpublish_deadlines.md (3), internal/format/oci/manifest_deadlines.md (3)
Handler and config	161	internal/format/maven/upload.go (103), internal/config/maven.go (58)
Schema and docs	33	config.proto (20), configuration-reference.md (5), e2e/maven.md (4), config.example.yaml (3), testdata (1)

1520 of the 1714 are tests and sidecar prose; the handler and config together are 161. Splitting the key from the route that reads it would leave a key that does nothing, which is the shape guardrail 20 warns about.

That 1520 is not separable work either: guardrail 6 does not accept an MR that defers its tests, and upload_deadlines.md is a .md file only because the comment caps push that prose out of the Go files, its own header naming the upload.go symbols and the upload_deadline_test.go header it belongs to. The 73 lines in internal/server/body_size.md are this MR's own edits to another package's sidecar, in five passages: the SetReadDeadline call-site paragraph, the write-deadline count, the install sentence, the failed-arm fall back with its #1258 pointer, and the 499 attribution. They describe this change rather than travelling without it. The six lines in internal/format/npm/unpublish_deadlines.md and internal/format/oci/manifest_deadlines.md are the one stale #776 sentence each carried. The counters paragraph is not among them, because the rebase took main's wording for it, leaving it byte-identical to main and outside this diff.

Stack

Order	MR	Branch	Target	State
1	!2345 (merged)	dmeshcharakou/upload-inactivity-deadline-step-1	main	merged
2	!2346 (merged) (OCI)	dmeshcharakou/upload-inactivity-deadline-step-2	main	merged 2026-09-08
3	this MR (Maven)	dmeshcharakou/upload-inactivity-deadline-step-3	main	open
4	not yet opened (npm)	n/a	main	not opened

!2346 (merged) and this MR were siblings off !2345 (merged). !2346 (merged) merged on 2026-09-08, so there is no merge order left to state: this MR is the only one of the pair still open, and every shared passage is now a question of what it owes against main rather than of which sibling lands second.

It merges clean now. Two files conflicted, forecast at 1adc4bb6e with a read-only git merge-tree and resolved by rebasing onto 7bd582891. The MR reports has_conflicts: false at 771b12597. What each file took:

• gen/artifactregistry/config/v1/config.pb.go. Each side adds a field to its own message, so the struct hunks are apart, but both also rewrite the serialized descriptor string and there the hunks overlap on the same run of lines. Resolved by re-running mise run proto:generate rather than merging the two by hand: the regenerated file differs from main only by upload_inactivity_timeout, and main's own ContainerConfig growth survives.
• internal/server/body_size.md, two hunks, both about which routes have installed the wrapper. The SetReadDeadline call-site paragraph and the install sentence each named one route on this side and the other on main's, because each branch was written before its sibling landed. Both took main's text with Maven added rather than this side's: this branch's copy said the OCI blob upload routes take container.upload_inactivity_timeout "when !2346 (merged) merges; until it does, those routes are still bounded by their own read timeout alone", and !2346 (merged) merging had already falsified that. The resolved sentences name both routes as installed, with npm publish still outstanding on #31. docs(server): drop the !2346 condition from the install paragraph is the one commit the rebase added, and it carries that correction.

Three more shared passages in that file did not conflict, and each is worth knowing before a later change reaches for them:

• The counters paragraph. Both sides amend it to point at #1256, and this branch carried main's wording byte for byte, so the two agreed and the rebase dropped 2f6525e29 ("take !2346 (merged)'s wording for the counters paragraph") as already upstream. The paragraph is main's and sits outside this MR's diff; re-wording it here is what would re-open the conflict.
• The N of the five arm the write deadline sentence. main reads "Three of the five" (OCI blob upload, OCI manifest PUT, npm single-version unpublish) and this branch reads "Four", adding Maven artifact PUT and dropping #776 from the outstanding set, which this MR closes. npm publish is the fifth and stays on #31. main's sentence was untouched since the merge base, so the change applied cleanly.
• docs/dev/configuration-reference.md. Its server.timeouts.write row lists four keys that arm both halves, naming maven.upload_read_timeout; maven.upload_inactivity_timeout is not one of them, because with the write half pinned a gap-bound arm does not move it. Two open MRs edit this file where this branch does, !2307 (merged) and !2332. Re-derived at 771b12597: each merges cleanly into origin/main and conflicts against this head in that one file, so the conflict belongs to whichever of the three lands second, and no pipeline reports it in advance. The order is now picked: this MR merges first, and !2307 (merged) and !2332 rebase onto main once it does. Nothing is edited on those two from here; the order is carried over to them separately. !1011 (closed) (PyPI) was on this list and closed on 2026-09-08, so it is off it. Re-derive the set at the head SHA rather than reading it from here.

config.example.yaml and proto/artifactregistry/config/v1/config.proto are shared in name only: each side adds its key under its own format block and message, so the hunks are apart.

e2e scenarios

Adds two scenarios to docs/testing/e2e/maven.md, each in the Publish table and in the usage-data events table.

e2e.maven.publish.stalled-upload-cut covers the window itself. Its assertion changed with the rebase: the cut now answers with a 500, so the scenario asserts the status as well as the log line that carries the inactivity marker.

e2e.maven.publish.empty-artifact covers the bodyless guard, added in review. A Content-Length: 0 PUT deploys and commits, answering 201, and it states the unguarded outcome too (the request context cancelled a window into the commit, and the abort then running on that cancelled context) so the scenario fails against the unguarded route rather than passing either way. Both Maven upload timeouts stay at their defaults, so a failure is the arm and not a configured bound, and it is driven with a raw PUT, because mvn will not deploy an empty file.

The drain bound adds no third scenario, per guardrail 12. Reaching it needs blob.NewSession to refuse, which the catalog has no lever for, and what it moves is the read deadline on a connection net/http drains after the response has already gone out, so no client-visible assertion separates the bounded case from the unbounded one. The three synctest cases named under Tests are what cover it.

Conformance

No protocol surface moves: no route, header, status code or problem document changes. The suite's own config is fixed by this MR rather than broken by it, per the section above.

Related to #1043 (closed)