docs(plans): correct Step 4's file set, counts, and borrowed precedent
What this does
Corrects Step 4 of the upload-session concurrency cap plan, and the passages elsewhere in the plan that describe Step 4. Step MRs cannot edit the plan, so the corrections travel here.
Plan text only. One file, no Go, no configuration, no generated file.
The borrowed precedent argued the reverse
Step 4's publish.go bullet said inlineBuildTimeoutRetryAfter's doc "prices
it against a rebuild worker's lifetime", and used that to argue against citing
it as the precedent for the shed's Retry-After. packument_cache.md says
the opposite in the sentence that prices it:
It is
rebuildTimeoutplus five seconds, so it is sized against the render budget and not against a worker's whole lifetime.
So the borrowed reason argued the reverse of what it was cited for, and it
read as verified. The bullet now separates the two claims that were tangled:
the magnitude is copied from that constant, which
### What this control has to expose to be operable already says, and what is
not shared is the constant itself, because the two headers wait on different
work and a change to rebuildTimeout must not move this shed's retry.
internal/format/npm/inline_build.go joins Step 4's Files list for the one
line carrying the same inverted summary, which is where the step's copy came
from. Step 4 owns that edit because it is the step whose comment borrowed the
claim. Grepped for the phrase:
rebuild_counter_helpers_integration_test.go is the third site and is
correct, being about a worker's lifetime rather than about this constant.
Two files the Files list did not name
internal/format/npm/observe.go, for the code's membership inisServerErrorCode. The omission read as a decision rather than a gap: the two inline-build503s that are already members log at Error, so a third that did not would have stated a deliberate difference with its reason nowhere.internal/format/npm/metrics.mdalready appeared for thecodecount, and now also carries the account the comment caps push out ofobserve.go.
observe_test.go and metrics_test.go join Tests for the same reason.
Merge order, stated rather than implied
Step 4 gains a Depends on (merge order) line naming both predecessors:
!2464 (merged), which gave upload_session_capacity_exceeded its S11 Error Cases row,
and !2460 (merged), the npm-publish half of #31, which pins the publish response write
deadline. Without the second, a shed answered after the gate's wait budget
cannot be written at all, because net/http arms the write deadline once at
the header read from server.timeouts.write. The line also records that Step
4 adds no write-deadline arm of its own, since !2460 (merged) asserts exactly one such
arm. Both have since merged, so the line reads as a record rather than an
instruction; see the third review round below.
Conflict note
The conflict note named !2259 (merged) and !2268 (merged), which are no longer open. It now says to re-derive the set before opening, and describes the two real overlaps by what they are rather than by number, so it does not go stale the same way again.
Fixes from the review of the first revision
A review of the first revision found two wrong numbers and five stale passages. All seven are fixed here, and each is listed below.
Two numbers were wrong.
- The realised size is 444 reviewable LOC, not 439: 410 added and 34 removed across 12 files. That is also the figure !2465 (merged)'s own description carries, so the two documents disagreed. The plan now cites !2465 (merged)'s head SHA beside the number, since !2465 (merged) postdates the basis "the commit above" names.
- The
observe.gobullet said the account moved intometrics.mdcovers "the one503the predicate still excludes". It covers two:inline_build_superseded, which holds arequestCodeLabelsslot, andservice_unavailable, which holds none.metrics.mdnames both, so the bullet undercounted the artifact it points at. The same bullet said "the two inline-build503s" where the package has three, and now says which two are members and why the third is not.
The Status row is gone. !2472 (merged) is the standing table MR that the
Status-table guardrail prescribes, and it fills five rows including Step 4's. Two writers on that table is what the guardrail exists to
prevent, and git merge-tree confirmed the collision: this branch and !2472 (merged)
conflicted on the plan file while each merged cleanly into main. With the
row reverted the two merge clean against each other, re-checked at
a91aa9269.
A sentence stopped being true. "No merge-order constraint is used anywhere
in this plan" was still there while this branch added Step 4's
Depends on (merge order) line. It now says what it meant, that no step is
ordered against another step by anything but a Depends on: edge, and names
Step 4's line as an order against two merge requests outside the plan.
Three passages had not caught up with Step 4's new file set.
- Step 8's Shared seams now checks files rather than the package, and lists both sibling sets. The conclusion was already right; the premise a reader would re-check it against had gone stale.
- Step 10's Shared seams records the
metrics.mdcollision with Step 4 from its own side, so a Step 10 author reading only their own section learns about it. Its conflict note stops naming !2268 (merged) as live, which has merged. - The
## Dependenciesoverlap census recordsmetrics.mdagainst Step 4 as well as Step 10.
Fixes from the review of the second revision
A second review found six more stale or unchecked passages. All six are fixed here, one commit each.
docs/dev/observability.md is a second sibling collision (512253d5).
Step 4 and Step 6 both edit it and they are parallel off Step 2 with no edge
between them, so the census's "No other sibling pair shares a path" was false.
Step 4's Shared seams now carries the pair in the words the metrics.md
pair uses, the census names both pairs, Step 6's Files list gains the path
so a file check from its side finds it, and the Conflict note stops reading
the file's traffic as all outside the plan. merge-tree --write-tree between
the two steps' open MRs is clean at heads 57cf1f989 and adc3b2803, so it is
an overlap rather than a textual conflict. !2463 (merged) has since moved to
85ecdcb7a, and the third round below re-measures and re-pins it.
Three contention counts were wrong (855070a8, with 153db723).
Counted at a91aa9269: internal/metrics/cardinality.go is named by four
steps (2, 4, 11 and 12), which makes it the plan's most-contended file;
internal/format/oci/upload.go by three (3, 11 and 12); and
internal/storage/blobstore.go by two (11 and 12), not the three the census
gave it. The safety conclusion carries over unchanged, because each set is a
chain rather than a sibling group. Step 4's Shared seams was the second
site the file was missing from and now names it too. 153db723 corrects that
new sentence's own quantifier: it claimed all three sets are chains and then
showed only the four-step one, so it now names the three Depends on: edges
the dependency table carries.
Step 11's Shared seams was missing a path (cb23abad).
npm/publish_stream.go is in Step 11's Files list and among the census's
six pairs, but not in that step's own Shared seams. "Every one is settled
by a Depends on: edge" is unchanged and still holds, since Step 11 names
Step 4.
Step 4 does not extend an existing 503 vocabulary (0e108762). The
Files list already recorded that the step adds its own
CodeUploadSessionCapacityExceeded and MsgUploadSessionCapacityExceeded in
transport.go and its own jittered constants in publish.go, inheriting only
the magnitude. Three passages still described the older reading: the Scope
line, which a reader meets first; the Forecast reason, inside the same
Forecast that then lists what it missed; and "So Steps 4 and 5 extend an
existing vocabulary rather than building one". Step 5 does reuse
CodeServiceUnavailable; Step 4 no longer does.
Two of Step 4's files had no acceptance criterion (4a610fc1).
observe.go and the code pin reached Tests but not Acceptance, so
checking the Files list against the criteria turned up two files with
nothing to falsify. The criteria are now the exact membership of
isServerErrorCode over requestCodeLabels, because a criterion that only
says the new code logs at Error is satisfied by a predicate admitting every
503, and requestCodeLabels asserted at 41 entries against the code
budget. service_unavailable is credited to the direct assertion rather than
the membership one: it holds no requestCodeLabels slot, so iterating that
enumeration cannot see it.
Step 8's re-check sentence named two of three files (3ee8c04b). A new npm
code also needs internal/metrics/cardinality.go, which carries both code
sites, and expectedDistinctValues["code"]'s comment reads exact. Step 4
takes the label to 41 of 41, so there is no headroom, and a Step 8 arm that
stops at transport.go and metrics.go gets an audit failure in a file it
never opened. The none verdict above that sentence still holds.
Fixes from the review of the third revision
Two findings, one commit each.
internal/format/npm/metrics.md is a three-step group, not a pair
(6e73bc9b). Step 6 edits it as well as Steps 4 and 10, so the four passages
recording a 4-and-10 pair were short by one step, and the sibling-pair count
in ## Implementation Steps read two where three distinct pairs exist:
(4, 6), (4, 10) and (6, 10). Step 6's Files list is what hid it. The path
was not declared there, so re-deriving the file-to-steps map from the twelve
Files lists returned the pair the passages already named; intersecting the
two open step MRs is the instrument that finds it, and it returns exactly
docs/dev/observability.md and internal/format/npm/metrics.md at !2465 (merged)'s
head 57cf1f989 and !2463 (merged)'s head 85ecdcb7a. Step 6's edit is its own work
rather than a merge-in artifact: !2463 (merged) adds a section on what
classifyRemoteUpstream books and what it drops, and rewrites the
cache_error paragraph below it, which is where the fill's admission shed
gets its outcome. Fixed at five sites, the four passages plus Step 6's
Files list, which gains the path with the pointer shape its
observability.md bullet has. merge-tree --write-tree between those two
heads is clean over both shared files, so Step 6's overlap stays a rebase for
whoever lands second; Step 10 has no MR open, so its two pairs rest on the
distinct passages instead. The three contention counts are unchanged and
re-derived at this head: internal/metrics/cardinality.go at four steps
(2, 4, 11 and 12) is still uniquely the most, and metrics.md at three is a
sibling group rather than a chain.
Step 4's merge order is now a record, not an instruction (df91121b). Both
predecessors landed on 2026-09-10, !2464 (merged) at 10:54Z and !2460 (merged) at 09:46Z, while
the previous commit here is 10:37Z, so the future tense was accurate when
written and is not now. docs/dev/writing-discipline.md asks for the past
tense with the merge request named rather than a deletion, so a reader
arriving after Step 4 lands can still tell the order was taken; both reason
clauses stay, because they are what say why the order mattered. Three sites
carried the fact: the field itself, the metrics.md bullet in Files,
which read "the S11 amendment this step depends on moves", and the Conflict
note, which read "merges first and therefore rebases nothing here". It
merged, so it is a rebase to take: !2460 (merged)'s squash commit 323eeaddd is an
ancestor of main and not of !2465 (merged)'s head 57cf1f989. The
condition-and-event instruction for the code decomposition is left as it
stands, since it reads correctly on either side of the merge.
Merge order for this set
| Order | MR | What it carries |
|---|---|---|
| 1 | !2464 (merged) | the S11 Error Cases row for upload_session_capacity_exceeded. Merged 2026-09-10. |
| 2 | !2460 (merged) | the publish response write deadline, the npm half of #31. Merged 2026-09-10. |
| 3 | !2465 (merged) | Step 4: the 503 rendering, its code, and the code budget slot |
| 4 | this MR | Step 4's corrected Files list, counts, and the passages describing it |
This MR is last because its Files list describes what !2465 (merged) actually shipped, and because the realised LOC figure it records is measured against !2465 (merged)'s head. It carries no code, so nothing depends on it at runtime.
!2472 (merged), which also edits this plan file, has since merged (8e63bd37c), so the
plan file has a single writer again and the order against it no longer needs
stating. Re-derived at this branch's head df91121bf against the other 68
open merge requests: none of them touches this plan file, and git merge-tree
against origin/main returns clean.
Related to #1234 (closed)