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 rebuildTimeout plus 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 in isServerErrorCode. The omission read as a decision rather than a gap: the two inline-build 503s 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.md already appeared for the code count, and now also carries the account the comment caps push out of observe.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.go bullet said the account moved into metrics.md covers "the one 503 the predicate still excludes". It covers two: inline_build_superseded, which holds a requestCodeLabels slot, and service_unavailable, which holds none. metrics.md names both, so the bullet undercounted the artifact it points at. The same bullet said "the two inline-build 503s" 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.md collision 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 ## Dependencies overlap census records metrics.md against 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)

Edited by Dzmitry (Dima) Meshcharakou

Merge request reports

Loading
Loading