fix(oci): re-arm the blob upload deadline on each read (upload-inactivity-deadline plan: 2/4)

What

Installs server.DeadlineReader (step 1) on the three OCI blob upload routes (chunked PATCH, monolithic PUT, single-shot POST) and adds container.upload_inactivity_timeout to size the gap it bounds.

container.upload_read_timeout stays the absolute ceiling. Every arm clamps to it, so an upload that keeps sending is held to the ceiling and an idle one is cut at the window. A cut answers 499 exactly as a client hangup already did, and records oci.upload.inactivity_cut on the upload wide event so the two can be told apart. It records oci.upload.bytes_read beside the marker, because the byte fields already there are the client's promise: oci.bytes_received holds the declared length and oci.bytes_persisted is never set on a failure exit, so without it a stall after 40 bytes and one after 4 GB render identically.

The gap bound moves the read half only. The write half is pinned at ceilingAt when the wrap is constructed and no later arm moves it, so container.upload_inactivity_timeout never sizes a response write. That is step 1's shape, not this MR's choice, and it is what the next section turns on.

Why

container.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), and ADR-004's upload-inactivity paragraph is what it implements.

Where the finish goes, and why it is a bare defer at handler scope

Worth a reviewer's attention, because it is the one place this could have cost an upload rather than saved one, and because two requirements on the same call pull in opposite directions.

net/http drains the unread body from body.Close inside finishRequest, after the handler has returned, and that drain reads under whatever the last call left. So the finish has to run late: called at the end of the body read it arms min(now + inactivity, ceilingAt) before the persist, and a persist outlasting that window leaves the drain's first socket read failing on an already-expired deadline, which closes the connection instead of keeping it. internal/server/body_size.md states the rule and TestDeadlineReader_FinishReadBelongsInADefer measures the reuse loss over real TCP.

It also has to be registered before the read, so a panicking staging write or a client that disconnects mid-copy still books it.

Both hold only if the handler owns it, so handlePatch, handlePut and handleSinglePost each build the gap bound and register defer finishUploadBodyRead(...) themselves. newUploadGapBound arms both halves on the way out, so the two finalize routes build one only where appendFinalChunk goes on to read, keeping a bodyless PUT and a zero-byte single POST on the ceiling alone.

What the read half's hand-back leaves is the exit's own business, and FinishRead branches on it. A body read to EOF leaves the half cleared rather than back on ceilingAt, because net/http cleared it on EOF and left a background read on the socket, so arming over it would cancel the request context. That is what keeps the persist safe: a read deadline left at the last gap-bound arm would cancel pgSession.Commit a window after the last byte arrived, and the commit's Move is a GCS rewrite whose cost scales with the blob, so the upload would fail after the client had sent every byte of it. Monolithic PUT and single-shot POST carry that; a chunked PATCH finished by a bodyless PUT does not, because the route builds no wrap.

The arm sequences cannot see the placement: all three are identical whether the finish is deferred at handler scope or moved into the read, because the recorder observes the order of the arms and not the persist boundary they straddle. What does separate the two is the read deadline in force at the persist, so TestUpload_PacedBodyReArmsTheReadHalf snapshots it from a stub.WithBlobRowPersister callback, which the stub runs inside session Commit. On PUT and single POST the last non-failed arm there must still be a window arm; moved into the read it is FinishRead's cleared zero instant, and lastReadArm skips failed arms rather than cleared ones. Measured: moving both defers reddens exactly those two cases.

What this does not reach. PATCH reaches no Commit, so its own defer is still unpinned in this package, and internal/server's TestDeadlineReader_FinishReadBelongsInADefer is what covers it, at the wrapper's contract rather than at this route's call site. upload_deadlines.md says so where a reader looking at these defers will find it.

The default yields to a lower ceiling

upload_inactivity_timeout defaults to 5m. Comparing that default against a configured upload_read_timeout would fail the next startup of any deployment that had lowered the ceiling, with an error naming a key absent from its file. So ociFromProto clamps the default to the ceiling, and ContainerConfig.validate refuses only a value the operator explicitly wrote.

No container config in this repo sits below 5m, but scripts/conformance/maven_setup.sh sets maven.upload_read_timeout to 60s, so the shape is one we use, and it is the shape !2351 (merged) meets.

Tests

Seven inactivity suites across all three routes, six of them driven under synctest. They include the negative control that the sentinel alone books no cut without a fired deadline, and the case that a failed arm completes the upload anyway. An eighth suite, TestUpload_BodylessBodyRoutesBuildNoWrap, runs outside the bubble and pins handlePut's and handleSinglePost's r.ContentLength != 0 guard by arm count: with both guards deleted a bodyless finalize takes three read arms instead of one, two of them at now+5m, while the 201 is unaffected. Config coverage pins parse failure, explicit zero, explicit empty, the default, both sides of the cross-field bound including the equal-pair acceptance, the defaulted-collision case above, and config.example.yaml's block against the compiled defaults.

Three properties of step 1's shape are asserted rather than assumed, and each was measured before it was written:

• The write half takes exactly two arms, both at the ceiling, which assertWriteHalfPinned pins on every route. A stray third arm reddens it.
• The read half's arm count discriminates the exit FinishRead branched on: a window cut records two arms, a completed read three with the last cleared, and an error no deadline raised three with the last a fresh window arm.
• A failed arm leaves the handler's ceiling standing, read back through lastReadArm() rather than off the last slot, since that slot now holds a cleared arm.
• The finish is still deferred at handler scope when the persist runs, read at the persist itself rather than off the arm order, which cannot see it.

TestUpload_CutRecordsWhatTheReadAccepted runs the cut record at two delivered sizes, 0 chunks and 4, because one size would not show oci.upload.bytes_read moving while the fields beside it stand still.

Each behaviour was mutation-tested rather than reasoned about: broken one property at a time, the suite re-run, the reddening test recorded. Dropping FinishRead, and arming the write half a third time, both redden.

Size

1817 reviewable LOC at beafb5df4, over the 500 ceiling docs/dev/development-model.md sets, so per guardrail 18, by file group:

Group	LOC	Files
Tests	940	internal/format/oci/upload_deadline_test.go (816), internal/config/container_test.go (124)
Sidecar prose	317	internal/format/oci/upload_deadlines.md (303), manifest_deadlines.md (14)
Handler	261	internal/format/oci/upload.go
Config and schema	162	internal/config/container.go (73), generated config.pb.go (38), config.proto (26), config.example.yaml (24), testdata (1)
Conventions, wiring and docs	137	AGENTS.md (82), body_size.md (41), wire_oci.go (9), docker.md (2), S16 (2), configuration-reference.md (1)

1257 of the 1817 are tests and sidecar prose, and a further 82 are the five AGENTS.md conventions the review round asked to persist, each in its own commit beside the change it came from. The handler, config, schema and the rest of the wiring together are 478. Splitting the config key from the routes that read it would leave a key that does nothing, which is the shape guardrail 20 warns about; splitting the three routes apart would triple the sidecar and the recorder fixture.

Stack

!2345 (merged) (step 1) has merged, so this MR and !2351 (merged) both target main directly.

Order	MR	Branch	Target	State
1	!2345 (merged)	dmeshcharakou/upload-inactivity-deadline-step-1	main	merged
2	this MR	dmeshcharakou/upload-inactivity-deadline-step-2	main	open
3	!2351 (merged) (Maven)	dmeshcharakou/upload-inactivity-deadline-step-3	main	open
4	not yet opened (npm)

Steps 2, 3 and 4 are siblings: none depends on another, and each is reviewable and mergeable on its own now that !2345 (merged) has landed.

!2345 (merged) merged with two changes this branch was written before, so the rebase was not mechanical: the write half became pinned, and RestoreCeiling was replaced by FinishRead. The commit fix(oci): hand the upload body deadline back after the read is where the branch takes both on, and the reworked assertions in ## Tests are that commit's.

Merge order against other open MRs

No pipeline reports these. Re-derived by test-merging each MR's head against beafb5df4.

• !2351 (merged) (Maven, step 3) conflicts on gen/artifactregistry/config/v1/config.pb.go and internal/server/body_size.md. Both branches add an inactivity key next to each other in the generated stub, and both rewrite body_size.md's sentence about which route installs server.DeadlineReader. Either order works, and whichever lands second resolves that sentence by naming both routes, both keys and both sidecars in one clause. Each branch currently names its own format and defers the other, so the resolution is a merge of two structurally identical sentences rather than a rewrite, and neither side carries a count to re-derive. The deferring clause stays true until step 4 (npm) lands, since that is a later format step that installs it; it is the npm MR that should drop it. This branch now also rewrites body_size.md's arm-failure-counter paragraph, to name #1256 instead of "the step that installs the wrapper". If !2351 (merged) leaves that paragraph alone the hunks do not overlap; if it edits it, take this branch's text, since the sentence it replaces pointed at no owner.
• !2307 (merged) (fix(npm): stream metadata documents so they carry their JSON type) merges clean. It rewrites configuration-reference.md's server.timeouts.write row, which this MR no longer touches at all: with the write half pinned, container.upload_inactivity_timeout does not belong in that row's list of keys that move the write deadline, so the row reverts to main's text and this MR's only change to the file is the new container row. The earlier conflict on that row is gone. !2306 (merged), which also rewrote it, has merged.
• !2332 (fix(oci): fence the blob link against a repository tombstone) merges clean despite sharing internal/format/oci/upload.go and upload_deadline_test.go, and it no longer wants a follow-up here: it adds a fourth in-code write-deadline arming site (setMountWriteDeadline in handleMount), which mattered only while this MR listed the blob upload routes in that row, and it no longer does.
• !1011 (closed) (Draft, PyPI) conflicts across 29 files including the proto, the generated stub and the configuration reference. It is on a stale base and conflicts with main independently, so it is not this MR's ordering problem.

e2e scenarios

Adds e2e.docker.publish.stalled-upload-cut to docs/testing/e2e/docker.md, in the Publish table and in the usage-data events table.

Conformance

No protocol surface moves: no route, header, status code or envelope changes, and an inactivity cut answers the same 499 a fired ceiling already did. S12's #### Upload Read Deadlines prescribes arming before the body read and 500 on a failed arm, and says nothing about whether the deadline is absolute or re-armed, so there is no drift to report.

Related to #1043 (closed)