feat(virtual): bound each collection position's upstream leg (S13 collection mode plan: 6/6)

🎯 What this does

Step 6 of the shared internal/virtual collection mode, and the last step of that plan: the resolution.position_wait_timeout configuration key, and Collect bounding each position's upstream leg with it.

Each asked position runs its contributor under a context.WithTimeoutCause child of the fan-out's context. When it runs out, the position stops waiting on that upstream and serves the cached copy Phase 1 found, recorded PositionContributedFromCache; where it holds no copy it contributes nothing and is recorded PositionSlow. Its slot is released to the next position in the list either way, and the collection answers from the positions that finished. Because the hand-back is in position order, cutting a slow early position is also what releases the finished documents queued behind it.

PositionSlow and PositionRecord.Elapsed already existed and already described a position dropped on a wait bound; nothing produced the value until now.

🔑 The ratified key

The step's Depends on gated on the S13 DRI settling the key name and its default. Both are recorded on the work item: position_wait_timeout, default 20s. The name is the one S31 already books; the default sits below http_client.request_total_timeout so it tightens rather than never fires, and two waves of it clear the 90s resolve budget with 50s to spare.

No ordering guard against resolve_timeout is added. A probe timeout at or above the budget is unworkable, but a position wait bound at or above it is merely inert, and S31 blesses that range explicitly. resolve_errors.md records the reasoning and TestNewResolver_Validation pins both boundary values as accepted, so a later symmetry argument reddens rather than lands.

📏 One rule: the bound covers the call that may leave this service

Read	Covered
A position's request to its upstream	✅ Yes
The cached copy served when that request is cut	❌ No, runs under resolve_timeout
A health-marked position's cache-only read	❌ No, runs under resolve_timeout
A hosted position's read (a fenced rebuild)	❌ No, excluded outright
Queued behind the probe_concurrency cap	❌ No, already recorded omitted
Waiting behind a single-flight leader	⚠️ Intended no, not enforced here

Every read of this service's own storage runs under the budget. cacheContext picks the fan-out's context while it is live and the detached wait once the budget has ended, which is how an expired budget still serves a copy the read already paid for.

The hosted exclusion came out of the first review round. A hosted position is never health-marked, because the health read is skipped where the spec carries no handle, so it reached Contribute under the position context like any other. Its read is a fenced packument rebuild on that rebuild's own 30s budget, and at the 20s default the bound cut it 10s inside that budget and booked PositionSlow — which sets degraded at collect_verdict.go:151 and answers CollectMergedPartial beside any contributing remote. That is a 200 serving a public remote's versions for a name the hosted repository demonstrably holds, the package substitution S31 vetoes.

The single-flight row is intent rather than something this MR enforces. S31's ## Configuration keeps a follower's wait on its own key, and capping it here would cap the knob operators serving large layers are told to raise — but remote.SingleFlight.Fetch takes one context and derives the follower wait from it, so which bound a coalescing wait actually follows is a property of how a contributor is built, not something Collect imposes. Raised in review and recorded in #1397; the doc sites that read as a guarantee are narrowed here. A position can therefore outrun the bound and still contribute, which its record cannot show, so a WARN line reports it.

One consequence worth stating: a position starting in a late wave with less budget remaining than the bound gets a context whose deadline collapses to the budget's, so it is recorded omitted rather than cut on this key. The budget really is what stopped it there.

♻️ Composing with step 5's cache fallback

!2754 (merged) merged step 5 while this MR was open, and the two rules collide where nobody wrote them to. Step 5 says if a position's call fails, read its cached copy instead. This step gives a position a new way to fail, so a position the bound cut landed in step 5's fallback arm by default — on the detached wait that arm uses, which carries no server-side deadline. The bound would have moved where a position waited rather than how long.

The first answer here was to return early instead: cut at the bound, contribute nothing, record PositionSlow. That is what the earlier revision of this MR did, and it is wrong in the case that matters. With 20 stale remote upstreams and every upstream silent, all 20 positions record PositionSlow, scanPositions sets only degraded, and CollectVerdict falls through to errNothingContributed — an outage answer for a package whose documents are on our own disks for all 20 positions.

So the bound is scoped to the upstream leg, and the composition is:

• Bound runs out — context.Cause is errPositionWaitExpired — stop waiting on the upstream, read the cached copy on the budget, PositionContributedFromCache. No copy: PositionSlow.
• Budget expires — cause is ErrResolveBudgetExpired — step 5's arm, unchanged: the copy is served on the detached wait.
• Caller hangs up — readEnded is true — nothing is read, PositionOmitted, because nobody is waiting for it.

positionOutcomeForError now reports whether a report settles the outcome at all. Only a success and an absence do; the bound, an ancestor's cut and a plain failure each name an outcome and leave the position to the cache arm below the call. That is what keeps step 5's rule intact under the new arm.

The accepted cost, stated rather than bounded: upstreams silent and the store slow, where a wave's cached-copy reads can spend what remains of the budget and the wave behind it is never asked. A store answering a keyed read that slowly is failing every route in the service. The alternative was a second position_wait_timeout for the cache leg, which doubles a position's worst case to 40s and the fan-out figure to 80s against a 90s budget — buying a guarantee for a compound failure at the price of the margin S30 already runs on.

⚠️ Two deviations from convention, both deliberate

This MR carries its own plan Status-table row, against the single-writer rule in AGENTS.md guardrail 4. Done at the maintainer's explicit direction for this MR rather than by oversight.

This MR edits docs/specs/ directly, against the Documentation pairing convention in docs/dev/conventions.md, which pairs a spec change with its own Spec MR.

This is an explicit maintainer override, decided in review, not scope that crept in. Review established that the spec corrections this key forces had no holder: their only tracker was work item 1249, which is this MR's own implementation task and closes when this merges, and no S13 Configuration amendment item existed to inherit them. The choice was to open a tracking item, land a separate spec MR first, or land the text here. The maintainer chose here, so the corrections are discharged rather than owed.

Four specs are touched, all of them for the same reason — a figure or a scoping sentence this key makes false:

• S13 gains the position_wait_timeout row its Configuration section never had (proto sketch, YAML example and prose entry), and its collection derivation is re-derived over the new per-leg cost.
• S31 has its "above 30s it never fires" sentence scoped to the upstream-reaching leg, gains the exclusions, and has two clauses that called the restatement outstanding recorded as discharged.
• S30 has its cold-merge worst case re-derived from roughly 60s to roughly 40s. The accepted-risk conclusion is untouched and was not reopened: 40s is still above the ~30s client read timeout, so the client still disconnects first. The passage now also states that the margin narrowed from roughly 30s to roughly 10s.
• No S32 edit. S32:102-108 is a Depends on gate tracked in #1389, and this key removes its premise rather than dating a figure inside it: container tag listing goes from ceil(40/10) * 30s = 120s, past the 90s default, to ceil(40/10) * 20s = 80s, inside it. Whether that gate still has work left is the S13 DRI's call, so it is named here rather than closed unilaterally.

📒 Residual owed, not blocking

The fetch seam cannot express the split between a position's own bound and a single-flight leader wait. remote.SingleFlight.Fetch takes one context and FetchOptions carries no second, so keeping a leader wait outside the position bound rests on the contributor rather than on anything Collect enforces. The doc sites that read as a guarantee are narrowed in this MR; the seam change is tracked in #1397, with FetchOptions.WaitBudget as the leading candidate. Owner: @10io.

🔄 Merge order, all gates discharged

Step 4. This MR was written on step 4's branch and opened Draft because step 4 (!2698 (merged)) had to merge first. It has, and the branch is rebased onto main.

The mixed-absence correction. Review raised a second gate this MR had missed: the plan gates that correction on this step, and this MR is what makes the gated case reachable, because nothing on main assigned PositionSlow before it. It merged as !2768 (merged). Its arm reads flags.absent && !flags.degraded, and PositionSlow sets degraded, so [PositionAbsent, PositionSlow] answers the outage rather than a 404.

Step 5. !2754 (merged) merged after this MR was approved, and the branch is rebased onto it. The composition it forced is the section above.

S32 step 9. bfaba02d merged after that, and split probe_timeout so it bounds one HEAD leg rather than a whole probe. It re-derived defaultVRResolveTimeout and the resolve_timeout reference entry, which this MR was re-deriving from the collection side at the same time. Both derivations now stand in one place: a container probe sweep costs 2 * (2 * probe_timeout + 2 * token_exchange_timeout) = 60s, a collection sweep 2 * position_wait_timeout = 40s, and the 30s remaining is headroom measured against the larger of the two. The one claim of ours their split made false — that probeOne bounds a whole probe with its exchange — is gone. No figure in the four amended specs moved: their change is a Phase 2 probe cost, and S13's, S30's and S31's collection arithmetic is position_wait_timeout against the same budget.

🧪 Testing

Unit tests only, which is what the plan's ## Testing Strategy asks for: no route, no schema, no migration, no new SQL.

• internal/virtual/collect_test.go — the three acceptance assertions: a position whose read outruns the bound is cut; two slow positions in one request are cut independently; a slow position 0 releases the finished positions above it. The delay sits inside the contributor call rather than around its return, so a bound armed where a contributor merely hands back a reader fails the first. Plus a cut position serving its cached copy, a health-marked position asserted not bounded, a hosted position run past the bound and asserted not cut, a release of the fan-out slot at a cap of one, and the overrun WARN asserted both ways.
• internal/virtual/collect_internal_test.go — a table over positionOutcomeForError covering every arm and whether it settles, including the rows that pair an absence with a dead context. Ordering the absence above either context arm reddens all three.
• internal/config — a dedicated file mirroring virtualrepositories_resolvetimeout_test.go subtest for subtest, plus the round-trip, defaulting and mapping assertions in the shared suites.

Two step-5 cases whose subject is the budget now arm collectInertPositionWait, a bound above the budget that never fires. At the default bound a position parked on its context wakes at 20s, so the budget could no longer be the thing that cut it and the case was measuring the wrong clock. The same helper already served three other budget-subject cases in this MR.

e2e scenario impact: none, per the plan's own reasoning: the seam has no client-facing route, so no docs/testing/ scenario exercises it. The rows that will cover the behavior belong to the consuming slices.

docs/user/** impact: none. The key is operator-facing configuration, documented in docs/dev/configuration-reference.md, which this MR updates alongside config.example.yaml per guardrail 14. No customer-visible surface changes.

📚 The prose sweep that travels with the key

probe_concurrency now bounds body-transferring document reads, not only bodyless HEAD probes, so every surface that described it otherwise is reworded: the proto message and field docs and their regenerated twins, internal/remote/config.go, internal/config/virtualrepositories.go, internal/virtual/resolve.go's sentinel doc, config.example.yaml, and two configuration-reference rows.

Three sites the plan did not enumerate are included, because this diff makes each false rather than merely narrow: the proto resolve_timeout field doc claiming probe_timeout is the one per-leg duration inside the budget, its generated twin, and defaultVRResolveTimeout's comment deriving the 90s default purely from probe-cost arithmetic, where a collection sweep now costs 2 * 20s = 40s and leaves 50s rather than 80s.

The hosted exclusion's reasoning now lives in internal/virtual/resolve_budget.md and the Go comments cite that page. main's Go-comment rule, merged as 10ee7bea after this branch was cut, admits an ADR, a docs/dev/ page and a package's own .md, and no spec at any granularity; a pre-existing spec citation stands until an edit touches its block, and these blocks were being rewritten anyway.

📊 Size

27 files, +1,226/-173, measured three-dot against main. Excluding the regenerated gen/**: 26 files, +1,189/-158, of which 224 insertions are Go and proto production code and 221 are prose and configuration, the four spec files above included. Just under two thirds of the non-generated insertions are tests.

Past the 500-line guidance in docs/dev/development-model.md, and not usefully splittable: the key and the bound are one concern, and each half is untestable alone — a key nothing reads asserts only its own default, and a bound with no key cannot be configured. Insertions by group: tests 744, production Go and proto 224, prose and configuration 221.

Related to #1249 (closed)