feat(virtual): collect every eligible position and hand each back (S13 collection mode plan: 4/6)

What this does

Step 4 of the shared internal/virtual collection mode: virtual.Collect, the Contributor seam it fans out over, the ordered hand-back and its hosted gate, the settle wait on every return path, the release split at the hand-back, and the per-position records CollectVerdict is settled over.

npm registers two of the collection's sentinels in the virtualServerError set, ErrUnsettledPositionOutcome and ErrCollectSinkRefused, so each answers 500 as a named record instead of the unnamed fall-through arm.

Stack

#	Step	MR	State
1	per-position outcome vocabulary	!2481 (merged)	merged
2	the collection verdict	!2541 (merged)	merged
3	the health gate marks instead of excluding	!2422 (merged)	merged
4	the collection entry point	this MR
5	per-position cache fallback and the veto arm	not opened
6	the per-position wait bound	not opened

Two docs MRs gate this one and are not plan steps: !2699 (merged) (docs(specs)) and !2700 (merged) (docs(plans)). Either may merge first, both merge before this MR.

internal/format/npm/virtual_read_errors.go and its test file are also edited by S31's Step 13 part 2. Merge order: this step lands first and S31 rebases onto it. S31's plan gates part 2 on this plan's foundation seam, so S31 can only land second. No S31 MR is open yet.

Merge gate: two amendments land first

S30 requires 503 upstream_unavailable where some positions answered absent and others failed on transport: "an absence established at every consulted position answers 404" (docs/specs/S30-maven-virtual.md:420, arm table :691, criterion :600). Those three line numbers are S30 as it reads at !2699 (merged)'s head 90debd45b, which is the tree this MR merges into; on main today each is one line lower.

S13 is where the conflict was. On main its collection arm reads "An absence established at any consulted position returns FileNotFoundOnUpstreams" (S13-virtual-remote-foundation.md:389), and !2699 (merged) moves that arm onto "at every consulted position" (:390 at 90debd45b), leaving the winner path's "any position" reading untouched. Merged Step 2's flags.absent arm still implements the "any" reading. This step's ask and settleAnswer make the conflict reachable: [remote_a fails on transport, remote_b answers 404] returns the 404 S30 forbids.

The decision was to amend the specs rather than change this code, so both amendments are merge gates here, not follow-ups:

Gate	MR	State	Carries
S13 amendment	!2699 (merged)	approved, merges before this MR	S13's collection arm, S31's two normative sentences, S30's bookkeeping
Plan amendment	!2700 (merged)	open, merges before this MR	the plan's reading, the owed code correction, this MR's Status row

The amendment leaves a code correction owed, which this MR does not make. CollectVerdict's flags.absent arm answers remote.ErrFileNotFoundOnUpstreams whenever any record is PositionAbsent and nothing contributed, so [PositionAbsent, PositionOmitted] answers a not-found where the amended specs require remote.ErrUpstreamUnavailable. That arm is merged Step 2 behaviour, outside this step's Files, and correcting it reverses a reading Step 2 pins with named tests. Booked in !2700 (merged)'s ## Dependencies and owed as its own MR. Nothing reaches it today: no slice wires Collect.

Reviewable LOC

2775 (added plus removed) across 11 files, over the 500 the development model asks a justification for.

Group	Files	LOC
Production Go	collect.go (575), position.go (6), collect_verdict.go (5), virtual_read_errors.go (4)	590
Tests	collect_test.go (1788), collect_internal_test.go (300), virtual_read_errors_test.go (27), virtual_fill_abandonment_test.go (2)	2117
Docs	virtual_read_errors.md (50), resolve_errors.md (16), configuration-reference.md (2)	68

Splitting would not help. The production Go is 590 LOC, 575 of it in one file and one control flow: fan-out, ordered hand-back, hosted gate, settle wait, release sweep. Any cut leaves one side unable to compile, or its tests unable to assert the property the other side owns. The rest is tests and docs. The npm edits are 33 LOC and cannot land alone either, because each new row fails assertNotTheDefaultArm until virtualServerError names its sentinel.

E2E scenarios

None added or affected. virtual.Collect has no route wired to it, so no request can reach a merge path, and the catalogs are keyed on client-visible behaviour.

Spec coverage

Spec: docs/specs/S13-virtual-remote-foundation.md (#### Collection mode), with the two consumer specs that book the behaviour: docs/specs/S30-maven-virtual.md (### Package-level metadata merge) and docs/specs/S31-npm-virtual.md (### Cross-upstream packument merge). Plan: docs/plans/2026-09-04-virtual-collection-mode.md, ### Step 4: the collection entry point.

Acceptance criteria

#	Criterion	Tests
AC-1	Three positions, one hosted and two remote, are asked in parallel, return three records, and are handed back in ascending position order whatever order they completed in	TestCollect_AsksEveryPositionInParallelAndHandsBackInPositionOrder, TestCollect_HandsBackInAscendingOrderForAnyCompletionOrder (rapid, over every completion permutation)
AC-2	The sink is called for the earliest positions before the last contributor has returned	TestCollect_HandsBackTheEarliestPositionsBeforeTheLastContributorReturns
AC-3	A position that contributes nothing is skipped rather than handed a zero value, leaving a gap its record explains	TestCollect_APositionThatContributesNothingIsSkipped
AC-4	A denied position is recorded PositionDenied and its contributor is never called	TestCollect_ADeniedPositionIsRecordedDeniedAndNeverAsked
AC-5	Fan-out never exceeds resolution.probe_concurrency, asserted over upstream reads still open rather than over contributor invocations	TestCollect_ConcurrencyNeverExceedsProbeConcurrency, over a contributor held open until the case releases each slot
AC-6	No Probe runs on the collection path	newSealedRemote's prober fails the test if invoked, in every case built on it
AC-7	A caller who abandons gets remote.ErrResolverCancelled and a collection that outruns resolve_timeout gets remote.ErrResolveBudgetExpired, as two sentinels and not one	TestCollect_AbortArmsAnswerTwoSentinelsWithANonEmptyRecordSet
AC-8	Each position's single-flight wait is detached from the budget: a leader commit past resolve_timeout but inside single_flight_wait_timeout still contributes, and the read is not ResolveBudgetExpired	TestCollect_TheSingleFlightWaitIsDetachedFromTheBudget (both rows)
AC-9	An expiry answers from the decision the phases did reach, whole merge or partial (S13 :1145)	TestCollect_AnExpiryAnswersFromThePartialMergeItReached, TestCollect_TheSingleFlightWaitIsDetachedFromTheBudget (both rows assert the sink, not only the record)
AC-10	Five zero-value guards on values a caller can declare: ErrResolverNotConfigured, ErrZeroValueCoordinate, ErrZeroValueUpstreamSpec, and ErrCollectRequestNotWired for a nil Contributor and a nil Sink	TestCollect_ZeroValueGuards (five rows)
AC-11	The hosted position in both Phase 1 states: a miss is PositionAbsent, a stale row contributes	TestCollect_HostedPositionInBothPhase1States, TestRecordsForState_HostedLookupStates
AC-12	The unusable-for-Lookup row is not a third hosted case	Unreachable by construction: loadAndFilter fails such a spec first. Pinned by the pre-existing TestRunPhase1_UnusablePositionFailsClosed; no subtest here
AC-13	A remote position whose contributor reports an absence is PositionAbsent, with a cache handle that fails the test if read	TestCollect_ARemoteAbsenceIsRecordedFromTheContributorsReport (over failingCacheStore)
AC-14	An all-remote list of reported absences answers remote.ErrFileNotFoundOnUpstreams, not remote.ErrUpstreamUnavailable	TestCollect_AnAllRemoteListOfAbsencesIsANotFound
AC-15	One contribution beside that record answers CollectMerged, not CollectMergedPartial	TestCollect_ARemoteAbsenceIsRecordedFromTheContributorsReport
AC-16	Every returned record set is non-empty on both abort arms	TestCollect_AbortArmsAnswerTwoSentinelsWithANonEmptyRecordSet
AC-17	The hosted gate on both sides of its condition: hosted above index 0 withholds the first hand-back; hosted at index 0 and an all-remote list emit immediately	TestCollect_TheHostedGateIsAssertedOnBothSidesOfItsCondition (three rows)
AC-18	The settle wait on both abort arms: every position's record is written before Collect returns, and the ownership of a contribution that settled past the cut follows what the arm answers	TestCollect_WaitsForEveryPositionToSettleOnBothAbortArms (per-arm hand-back and release expectations)
AC-19	A panicking contributor does not kill the process, siblings still contribute, the position is PositionPanicked, and the ERROR line carries the recovered value plus position, path, namespace_id and upstream_repository_id, field by field	TestCollect_APanickingContributorIsRecoveredRecordedAndLogged
AC-20	Two independently failing positions beside one success answer from the success, with both failures recorded, so no sibling was cancelled	TestCollect_TwoFailingPositionsBesideOneSuccessAnswerFromTheSuccess
AC-21	An unhealthy position's contributor is handed no fetcher and no prober	TestCollect_AnUnhealthyPositionIsAskedThroughTheCacheOnlyEntryPoint, over newSealedRemote's prober and fetcher
AC-22	Every contribution Collect still holds is released, and one already handed back is not, a refusal included	TestCollect_ReleasesWhatItStillHoldsAndNotWhatItHandedBack (both subtests), TestCollect_ASinkErrorStopsTheCollectionAndIsNotThePositionsFault
AC-23	Collect runs the health gate in gateMark, so a wholly unhealthy eligible set reaches the fan-out instead of the all-unhealthy aggregate	TestCollect_RunsTheHealthGateInMarkMode
AC-24	The record-validity guard refuses PositionUnknown and an outcome outside the ten defined, both answering ErrUnsettledPositionOutcome, and admits the nine a collection settles on	TestCheckPositionRecord_RefusesAnOutcomeNoCollectionSettlesOn, TestCheckPositionRecord_AdmitsEveryOutcomeACollectionSettlesOn
AC-25	ErrUnsettledPositionOutcome is a value the npm taxonomy names rather than one that falls through to its unnamed 500 arm	TestVirtualReadErrors_MapsEveryTaxonomyValue/ErrUnsettledPositionOutcome
AC-26	A sink error is the caller's own failure: the collection stops handing back and returns it under ErrCollectSinkRefused with the caller's own value still matching underneath, and the position keeps its contributing record	TestCollect_ASinkErrorStopsTheCollectionAndIsNotThePositionsFault, TestCollect_ASinkErrorOnTheResumedHandBackIsTheAnswer (both assert the sentinel and the caller's value)
AC-27	The eligibility Reason a position was excluded for maps onto its own outcome, so a data error is not recorded as a denial	TestExcludedOutcome_MapsEveryReasonOntoItsOutcome (seven rows, every arm), TestRecordsForState_ADataErrorPositionIsNotADenial

Error cases

#	Condition	Tests
E-1	Virtual: no upstreams configured, NoUpstreams	Shared loadAndFilter/EligibleUpstreams path, unchanged here: pre-existing TestResolve_NoUpstreams and Step 2's CollectVerdict empty-set arm. No subtest here
E-2	Every upstream filtered out by well-formed rules, AllUpstreamsDenied	Same shared path: pre-existing TestResolve_AllUpstreamsDenied. No subtest here
E-3	Nothing eligible with one data error, AllUpstreamsIneligible	Same shared path: pre-existing TestResolve_TwoAggregatesDistinguishable. No subtest here
E-4	No contribution, every consulted position established an absence, no data error: FileNotFoundOnUpstreams. This is S13's collection row as !2699 (merged) amends it; "some position established an absence" is the winner path's row and not this one	TestCollect_AnAllRemoteListOfAbsencesIsANotFound
E-5	An absence beside a data-error position never answers FileNotFoundOnUpstreams	Step 2's merged CollectVerdict table over the records Collect now mints, with the Reason-to-outcome half pinned by TestExcludedOutcome_MapsEveryReasonOntoItsOutcome and TestRecordsForState_ADataErrorPositionIsNotADenial. PositionDataError's own producer is Step 5's
E-6	No contribution and anything short of an absence at every consulted position: UpstreamUnavailable. Tested here for no absence at all; the mixed case, some positions absent and others failed on transport, is the CollectVerdict correction owed above and answers the not-found until it lands	TestCollect_AnOutageWithNothingEstablishedIsNotANotFound
E-7	resolve_timeout runs out with nothing to answer from: ResolveBudgetExpired	TestCollect_AbortArmsAnswerTwoSentinelsWithANonEmptyRecordSet, TestCollect_TheSingleFlightWaitIsDetachedFromTheBudget/the position's own work is still cut at resolve_timeout
E-8	resolve_timeout runs out and something was settled: answer from it	TestCollect_AnExpiryAnswersFromThePartialMergeItReached, TestCollect_TheSingleFlightWaitIsDetachedFromTheBudget/a leader commit past resolve_timeout still contributes
E-9	The caller's own context ends: ResolverCancelled, no response	TestCollect_AbortArmsAnswerTwoSentinelsWithANonEmptyRecordSet/a caller who abandons the request, TestCollect_ReleasesWhatItStillHoldsAndNotWhatItHandedBack
E-10	All eligible upstreams unhealthy: AllUpstreamsUnhealthy	Deliberately not answered on this path, which is what gateMark buys: TestCollect_RunsTheHealthGateInMarkMode
E-11	Probe cancelled mid-Phase-2: ResolverCancelled	Not applicable: no Probe runs on the collection path (AC-6)
E-12	Winner's Fetch returns non-2xx: WinnerFetchFailed{status}	Not applicable: a collection elects no winner, which S13's collection mode states outright
E-13	A veto on a position that established a presence it could not serve	Step 5's arm. No S13 error surface names it; it travels to the arming slice
E-14	A record carrying no outcome a collection settles on: ErrUnsettledPositionOutcome	TestCheckPositionRecord_RefusesAnOutcomeNoCollectionSettlesOn, TestVirtualReadErrors_MapsEveryTaxonomyValue/ErrUnsettledPositionOutcome
E-15	A collection request left with no contributor or no sink: ErrCollectRequestNotWired	TestCollect_ZeroValueGuards (two rows). Refused before any position is asked, so no upstream is blamed for a composition the caller never finished
E-16	Transport failure, blocked URL, header injection, slow-header and idle-transfer aborts, body cap, decompression bomb	Owned by internal/remote's upstream client and reached through the slice's own contributor, not by Collect, which issues no upstream request. Not tested here

Security considerations

#	Concern	Tests
S-1	A merge read against a health-marked remote issues no request to it	TestCollect_AnUnhealthyPositionIsAskedThroughTheCacheOnlyEntryPoint: the seam half. The residual, a slice's own contributor honouring it, is the consuming slice's, per the plan's ## Dependencies
S-2	A rule-denied position's cached bytes are never served, so an exclusion cannot be undone	TestCollect_ADeniedPositionIsRecordedDeniedAndNeverAsked: the contributor is never reached at all, so no arm can read that position
S-3	Package substitution: a hosted veto must not fire after a public remote's document is already on the wire	TestCollect_TheHostedGateIsAssertedOnBothSidesOfItsCondition. The veto itself is Step 5's
S-4	A not-found must not be answered for a package an upstream holds, because npm caches a 404 as a hard E404	TestCollect_AnOutageWithNothingEstablishedIsNotANotFound, TestCollect_AnAllRemoteListOfAbsencesIsANotFound
S-5	Error-payload hygiene: no upstream host, URL, header or path reaches a client-visible field	Pre-existing TestVirtualReadErrors_ServerErrorLeaksNoWrappedDetail covers the 500 envelope both new sentinels take
S-6	Logs carry metadata only, never credentials or response bodies	TestCollect_APanickingContributorIsRecoveredRecordedAndLogged asserts the panic line's field set exactly. The recovered value is slice-supplied, so a slice panicking with a secret is the slice's residual
S-7	SSRF and outbound request safety, cross-origin redirect credential stripping, TLS, outbound proxy, upstream content trust, credentials at rest, audit events	Owned by internal/remote and by the eligibility pass, neither of which this step changes. Not tested here
S-8	Deny rules are scoped to the fields a request addresses	Owned by rules.go, unchanged here: pre-existing TestResolve_RuleOnUnaddressedField_DoesNotDecide and rules_test.go

Design decisions, for reviewer confirmation

The plan requires each of these but names no shape, so the shape was chosen here and the tests are its record.

• The detached single-flight wait. Contribute takes ctx under resolution.resolve_timeout and waitCtx off the budget, bounded by resolution.single_flight_wait_timeout alone, which keeps Resolver unwidened. Two adjacent same-typed parameters are a swap hazard: reversing them runs the position's own work on the 5m wait and its leader wait on the 90s budget. Wrapper types were considered and left to the three slices that will implement the seam. Accepted knowingly.
• The cache-only entry point is a second method, ContributeFromCache, with no wait context, rather than a flag on one method. Collect books PositionContributedFromCache from which entry point it used.
• The state-to-record mapping. recordsForState emits a record only for a position the fan-out will not ask, so a hosted stale row has none and a hosted miss has PositionAbsent. The record-validity guard is its own function over one record, which is what makes its two rejections drivable.
• A plain contributor failure is PositionOmitted. No cache-only call is made for a failed position, so Step 5's row "transport or 5xx failure, produced nothing" applies. PositionDataError has no producer until Step 5.

Files outside the step's planned list

Four paths carry a fact this step falsifies, which guardrail 17 requires fixing in the same change:

• internal/virtual/collect_verdict.go: drops "No slice names it yet, so until the step wiring the fan-out adds it to its taxonomy it takes that taxonomy's unnamed 500 arm". This step is that wiring.
• internal/virtual/position.go: PositionPanicked's "until a log line is emitted at the recovery the three read alike" fired here.
• internal/virtual/resolve_errors.md: said only CollectVerdict answers ErrUnsettledPositionOutcome, which checkPositionRecord falsifies, and now names ErrCollectRequestNotWired too.
• internal/format/npm/virtual_fill_abandonment_test.go: the "nineteenth taxonomy value" ordinal moves to twentieth. The plan's npm-taxonomy item names five sites in one test file and eight in the md, and no third file, so its list undercounts by a file.

Residuals, each with an owner

• ErrCollectRequestNotWired is not in npm's taxonomy, so it takes the unnamed 500 fall-through, the way ErrUnsettledPositionOutcome did before this step named it. Owner: the consuming slice's own taxonomy step. The count-site cost this bullet used to give as the reason is not it: naming ErrCollectSinkRefused here moved nine count sites across three files, and this sentinel would move the same set. The reason is whose defect the value reports. A request left with no contributor or no sink is the consuming slice's own composition defect, raised where that slice wires Collect, so the arm for it belongs with that wiring. A sink refusal is Collect's own documented return surface whatever the caller does, which is why it is named here instead. Neither value is reachable from an npm read today.
• Three of settleAnswer's four return arms emit no collection-level log line, so a failure from the sink-refusal arm, from the cut arm, or from the resumed hand-back's own failure names no arm. Only the arm that outran the budget and still answered logs, at logCollectionBudgetExpired. The cut arm no longer discards its verdict error, which is the diagnostic, but the three lines still need their own messages and a docs/dev/observability.md row. Owner: the open review thread on settleAnswer, left unresolved for it.
• docs/plans/2026-09-10-s30-maven-virtual.md:1262 still scopes its taxonomy step to "the three wiring sentinels"; internal/virtual now exports seven error values, five of them wiring defects (all but ErrPositionAbsent and ErrCollectSinkRefused). A step MR must not edit a plan file. Owner: the S30 DRI or a docs(plans) MR.
• Plan text this MR cannot touch (single-writer rule), carried by !2700 (merged): the :36-39 claim that S30 and S31 are byte-identical at the basis and at main, which is false, and the npm-taxonomy item's eight md count sites, where main has since added a ninth (the "one of the twenty" phrase at internal/format/npm/virtual_read_errors.md:89, now "one of the twenty-one"). This diff updates that ninth site; only the plan text was stale. The plan's spec citations are correct as they stand, checked with git show 07f4e8521:<path> against the basis it pins.
• Two recorded S13 deviations ship with this step, owed to the S13 DRI alongside #1139 (closed): the budget's timer running over a contributor's transfer, and over a caller still reading past the budget. The plan books them at S13-virtual-remote-foundation.md:1040 (plan :1662 and :1672) and permits this step to open against them.
• No slice may wire Collect until Steps 5 and 6 land. Between 4 and 5 a position that could not answer is omitted instead of falling back to its cache, which both consumer specs make a success. Between 5 and 6 there is no per-position bound, so one slow upstream is paid in full on every read. The plan states this at :2778, and Collect's doc comment now carries it too. Both consumer plans already gate on it.

Verification

• golangci-lint run (2.13.2 via mise, fresh cache, --max-same-issues=0 --max-issues-per-linter=0 --uniq-by-line=false) over ./internal/virtual/... and ./internal/format/npm/...: 0 issues. No changed file is //go:build integration, and the integration-tagged run over both packages reports no issue from this diff.
• go test -race -count=1 over both packages: pass.
• scripts/ci/check-comment-caps.sh --base origin/main: OK.
• Fourteen arms were inverted or deleted in a scratch copy and confirmed to redden the suite: the nil-Contributor and nil-Sink guards, excludedOutcome's data-error, ReasonEligible and default arms, the expiry-resume hand-back, the pre-sink handedBack mark, the expiry arm's verdict key, release()'s outcome-blind keying, the hosted gate's cut arm, the cache-only fail-closed arm, the queued-position skip, and the two log lines. release()'s keying is the one the first pass missed: re-keying it on contributes(record.Outcome) left the suite green, and the review-fix commit added the fixture that now catches it.
• Each of this round's five review fixes was mutation-checked. Reverting errors.Join on the cut arm reddens both abort arms; removing the two cancels from the deferred drain reddens the new panicking-sink case at exactly 5m0s, the single_flight_wait_timeout default; guarding the release sweep on the sink-error path reddens the two new settled-sibling assertions and nothing else in the file; and reverting b42284f5f's arm split while keeping the new default guard reddens the cache-only absence row, on a captured WARN naming an upstream that was never asked. For the sink sentinel, reverting the %w wrap in handBack reddens both collect_test.go sink-refusal cases, and dropping the virtualServerError line reddens the new TestVirtualReadErrors_MapsEveryTaxonomyValue/ErrCollectSinkRefused row.
• The cut guard on the failure log was half-defended: deleting the log reddened the suite, but logging unconditionally left it green, so nothing pinned the claim that a cut is one line at the collection rather than one per position it caught in flight. TestCollect_APositionCutMidFlightIsNotLoggedAsItsOwnFailure closes that, and both mutations redden now.

Related to #1247 (closed)