feat(virtual): settle the collection verdict over its position records (S13 collection mode plan: 2/6)

What

Step 2 of the shared internal/virtual collection mode plan: CollectVerdict, the pure function from a position-record slice to one aggregate outcome or the failure sentinel those records select. No I/O, no resolver, and nothing calls it yet; Step 4 wires the fan-out that produces the records.

CollectOutcome has three values. CollectUnknown is the zero value and is what every failure arm returns beside its sentinel, so no verdict travels past an error. CollectMerged and CollectMergedPartial are both successes: whole where every rule-eligible position contributed or established an absence, partial where one of them did neither.

The ordering is the part worth reading. The fail-closed scan runs over the whole record set before either success verdict is settled, so an unfilled PositionRecord or an outcome this package does not define answers ErrUnsettledPositionOutcome however many positions beside it contributed. Checking for a contribution first and then splitting is the implementation this shape exists to rule out: it reads a record nobody filled as part of a served 200. PositionRecord has no constructor, so that set is reachable from a declarable value.

CollectOutcome.String renders unknown, merged and merged_partial. Both verdict tokens are spelled the same way in S30 and S31, unlike six of the ten per-position tokens, so the renderer belongs in this package rather than in each slice.

Merge order

#	MR	What
1	!2544 (merged)	amends the plan's Step 2 failure table and Step 4 item list
2	this MR	the implementation, which already carries the code for both amended rows

!2544 (merged) must merge first. Two arms here reverse rulings the merged plan made, so merging this MR against the unamended plan leaves the tree contradicting its own plan. !2544 (merged) adds that order to the plan's ## Dependencies, so once it merges the order survives the branches rather than resting on the pair existing. Until then the plan file carries no mention of it.

No other coordination. Re-derived on 2026-09-11, after !2544 (merged) was opened: of the 69 other open merge requests, every head ref resolved and not one touches internal/virtual/, so no file here overlaps another open branch. !2544 (merged) touches only the plan file, which this MR does not. This step widens no predicate and renames no label.

The two no-contribution arms that withhold the not-found

The no-contribution arms turn on one distinction that is easy to get backwards, so it is worth stating: a position that was never consulted and a position that answered but could not serve what it holds both withhold a not-found the other positions established. A policy denial does not, and neither does a position that simply produced nothing.

S13-virtual-remote-foundation.md:1251 states the first half directly, and :715-716 gives the rule behind it: the test is "whether the position was consulted at all, not how confident the positions that answered were". An ineligible position's own rules were unreadable, so nothing about the request settles whether it held the path.

PositionDataError withholds for the opposite reason, and this is the arm the plan amendment corrects. Such a position answered with a document (over the format's cap, unparseable, or directive-carrying), so it does hold the path, and remote.ErrFileNotFoundOnUpstreams is then false of the list whatever the other positions established. internal/remote/errors.go states that as the sentinel's own contract: it "asserts an absence, so resolution returns it only when every position that could establish one did". Neither consumer spec decides the shape, and S13:342 names the category ("a position that established a presence it could not serve") only on the veto path, so the amendment's ## Dependencies entry records the reasoning rather than leaving it in a commit.

A position that produced nothing is an omission, and the absence still stands. That covers a health-marked position too: :330 rules that in collection mode "a health-marked position counts as consulted here", unlike the winner path where it is skipped without a request. The clause is scoped there to cache eligibility, so the reason the arm operates on is the simpler one, that a position producing nothing established nothing. A denial establishes no absence of its own and withholds nothing.

What that costs, named because the arm is the one a client caches. PositionSlow is an omission too, so [absent, timed_out] answers the 404 npm caches as a hard E404, and the shed position may well hold the package. Shedding rises with load, so the false-404 rate rises with it. This follows :715-716's consulted/answered test and neither consumer spec settles the set, so it is a consequence of the rule rather than a defect here; the wait bound that produces PositionSlow is Step 6's, and the trade-off belongs with it.

All four readings are pinned by tests on the case they disagree on:

• TestCollectVerdict_FailureArms/an_established_absence_beside_a_policy_denial_is_still_the_not-found against .../an_ineligible_position_withholds_the_not-found_an_absence_established: the same set with the denial swapped for an ineligibility flips 404 to 503.
• TestCollectVerdict_AnIneligibilityOutranksAnUnservedPresence pins which of the two withholding arms fires when a set carries both, which no other case reaches. TestCollectVerdict_AnUnservedPresenceWithholdsTheNotFound holds both directions of the data-error pair, and TestCollectVerdict_AnAbsenceStandsBesideAContributorSideFailure holds the three values that do leave the absence standing.

The fail-closed arms answer a wiring sentinel

An unfilled record and an outcome outside the ten defined both name a defect in this service, not an upstream that was unavailable. They answer ErrUnsettledPositionOutcome, a new sentinel beside the three internal/virtual already exports for that class (ErrZeroValueUpstreamSpec, ErrResolverNotConfigured, ErrZeroValueCoordinate), which internal/format/npm's virtualServerError answers with a 500.

The plan's earlier ruling named remote.ErrUpstreamUnavailable here, which files a wiring bug in the upstream-outage bucket at WARN and sends a Retry-After for a failure no retry clears. !2544 (merged) amends that row too.

ErrZeroValueCoordinate called itself "the third of this package's wiring sentinels" beside two named ones. There are four now, so it names three.

Two aggregates this function never mints

An all-denied read answers 404 and an all-ineligible one 500, and EligibleUpstreams returns remote.ErrAllUpstreamsDenied or remote.ErrAllUpstreamsIneligible before any record exists, so neither reaches this function. Only the partial ineligible list arrives here. That is the caller's guarantee to keep, and CollectVerdict says so on the ineligible flag rather than claiming to enforce it: reaching it with an all-denied set answers 503 where S30-maven-virtual.md:639 requires 404, and with an all-ineligible set 503 where :650 requires 500 and says never 404.

Telling the three outage arms apart

Three arms answer remote.ErrUpstreamUnavailable: an ineligibility withholding the not-found, an unserved presence withholding it, and a genuine "nothing contributed and no absence". Each wraps its own message, as the winner path's resolve.go:638 does, so a 503's log line names the arm that fired instead of collapsing three conditions onto one bare value. TestCollectVerdict_TheOutageArmsAreDistinguishable asserts the three stay distinct; unifying any two passes every other assertion in the file.

What the review round changed

Nothing in the verdict table moved; every finding was about what the code says or hands the next step.

• The fail-closed scan returned one bare sentinel for two different defects. Each arm now names the position it found, and the undefined one reports the integer rather than the invalid token String renders.
• The three outage arms are package-level values. They still wrap remote.ErrUpstreamUnavailable, so every slice answers them exactly as before; hoisting drops three per-read allocations and gives each arm a stable identity.
• The caller precondition below was lost when scanPositions was extracted, while this description still cited it. It is back, on the flag it governs, and now names the 404 and 500 the two aggregates earn.
• The absence arm justified itself with a clause that contradicts PositionOmitted's own doc and misreads :330. It now gives the reason that operates.
• Two tests added, one pinning which withholding arm fires when a set carries both, one pinning the two fail-closed messages apart. Both were mutation-checked.

Spec coverage

Spec: docs/specs/S13-virtual-remote-foundation.md, which owns internal/virtual and the format-blind collection mode. The behavior is also specified in the two merged consumer specs, S30 Package-level metadata merge and S31 Cross-upstream packument merge.

Every test and subtest name below was re-derived from the branch head with go test -v -run TestCollect, which reports 16 test functions and 34 subtests. Line numbers are measured at 019c8e76cb, the branch point; all three spec files are byte-identical at that commit, at main and at this branch's head, so the numbers hold for a reader of the merged MR too. This MR edits no specification file.

Acceptance criteria

S13's own ## Acceptance Criteria section states no collection-mode criterion: its two mentions of the mode both scope themselves out of it, saying the read under test is a winner-path one. So the rows below come from S13's #### Collection mode design contract and from the two consumer specs.

Row ids are this table's own; #### Collection mode numbers nothing. CM-1 to CM-10 keep the ids Step 1's table minted, so the two aggregate. CM-11 to CM-14 are new here, over the no-contribution taxonomy paragraph (:346-349): CM-11 takes :347, CM-12 :348, and CM-13 and CM-14 the two clauses of :349.

#	Criterion (S13 #### Collection mode)	Tests
CM-1	Ask every eligible position for its document	Not in this MR: CollectVerdict is pure and does no I/O. Owned by Step 4 (Collect).
CM-2	Record each position's own outcome	Not in this MR. Landed in Step 1 (!2481 (merged)), whose PositionRecord and PositionOutcome every set here is built from.
CM-3	Settle one aggregate verdict over those records	All 16 test functions. TestCollectVerdict_SuccessSplit and TestCollectVerdict_FailureArms are the two halves; the rest pin cases a neighbouring arm passes on its own.
CM-4	No Probe runs on a collection path	Not reachable here: no PositionOutcome value names a probe. Owned by Step 4.
CM-5	The health gate marks the position instead of excluding it, and a failed fetch is marked the same way	TestCollectVerdict_AListOfCachedContributionsIsMerged is the verdict half: a whole list contributing only cached copies is CollectMerged, not an outage. The gate itself is Step 3 (!2422 (merged)); the fetch-failure fallback is Step 5.
CM-6	Each position's outcome is recorded rather than collapsed	TestCollectVerdict_SuccessSplit (10 subtests), TestCollectVerdict_AnAbsenceStandsBesideAContributorSideFailure (3) and TestCollectVerdict_AnUnservedPresenceWithholdsTheNotFound are what one success flag per position cannot answer.
CM-7	Fan-out is bounded by resolution.probe_concurrency	Not in this MR. Owned by Step 4.
CM-8	A collection is best-effort, and a partial one is a success	TestCollectVerdict_SuccessSplit/{data_error,omitted,slow,panicked} each answer CollectMergedPartial with a nil error, which is what makes a degraded collection a success.
CM-9	A slice may arm a per-position veto	Not reachable here: the veto stops the collection before a verdict is settled. Owned by Step 5.
CM-10	A denied position contributes nothing, its cached copy included	TestCollectVerdict_SuccessSplit/denied and TestCollectVerdict_FailureArms/a_denial_beside_an_omission_establishes_no_absence: a denial neither contributes nor degrades the merge, and establishes no absence of its own. Whether its bytes are withheld is Step 4's.
CM-11	An absence established at any consulted position returns FileNotFoundOnUpstreams (:347)	TestCollectVerdict_FailureArms/an_established_absence_beside_an_omission_is_the_not-found, .../an_established_absence_beside_a_policy_denial_is_still_the_not-found, TestCollectVerdict_AnAbsenceStandsBesideAContributorSideFailure (3 subtests)
CM-12	No absence and no contribution returns UpstreamUnavailable (:348)	TestCollectVerdict_FailureArms/no_absence_and_nothing_contributed_is_the_outage, TestCollectVerdict_AnAllOmittedSetIsUnavailable (asserted not to be the not-found)
CM-13	A data-error position anywhere in the list withholds the not-found (:349)	TestCollectVerdict_AnIneligiblePositionWithholdsTheNotFound (both directions), TestCollectVerdict_FailureArms/an_ineligible_position_withholds_the_not-found_an_absence_established and .../an_ineligible_position_with_no_absence_to_withhold. PositionDataError withholds on its own reasoning rather than under this clause: see The two no-contribution arms.
CM-14	A list left with nothing eligible answers on the two-ineligibility-aggregates arm (:349)	Not reachable here: EligibleUpstreams returns ErrAllUpstreamsDenied or ErrAllUpstreamsIneligible before any record exists, so neither aggregate reaches CollectVerdict. Stated on the ineligible flag, naming the 404 and the 500 each aggregate earns.

#	Criterion (S30 and S31)	Tests
S30-1	merged when every rule-eligible position contributed or established an absence, merged_partial when any could not answer at all (S30-maven-virtual.md:701)	TestCollectVerdict_SuccessSplit, 10 subtests, one per PositionOutcome value
S30-2	All-absent is a 404 (:551)	TestCollectVerdict_FailureArms/an_established_absence_beside_an_omission_is_the_not-found
S30-3	All-failed is a 503 (:552)	TestCollectVerdict_AnAllOmittedSetIsUnavailable, TestCollectVerdict_FailureArms/no_absence_and_nothing_contributed_is_the_outage
S30-4	A data error vetoes the merge 404 (:553), and withholds the not-found even where an absence was established (:533)	TestCollectVerdict_AnIneligiblePositionWithholdsTheNotFound: the set with the ineligible record answers unavailable and is asserted not to be the not-found, and the same set without it answers the not-found. :533's own pair, a well-formed deny rule in place of the corrupt one, is TestCollectVerdict_FailureArms/an_established_absence_beside_a_policy_denial_is_still_the_not-found.
S30-5	A malformed input cannot fail the read (:554): omitted, and the read answers 200	TestCollectVerdict_SuccessSplit/data_error (CollectMergedPartial, nil error). This criterion governs a set that still has a contributor; with none left, the read is the 503 of TestCollectVerdict_AnUnservedPresenceWithholdsTheNotFound rather than a 404, which is the amendment !2544 (merged) carries.
S30-6	Best-effort under outage (:548): omitted where no cached copy exists, and merged_partial	TestCollectVerdict_SuccessSplit/omitted, TestCollectVerdict_AnAbsenceStandsBesideAContributorSideFailure/omitted
S30-7	An unhealthy remote contributes from cache only (:549)	TestCollectVerdict_AListOfCachedContributionsIsMerged, TestCollectVerdict_SuccessSplit/contributed_from_cache
S31-1	merged / merged_partial, with the partial set enumerated: a failed leg, an unhealthy remote with no copy, a shed position, an over-cap or unparseable input (S31-npm-virtual.md:1806-1810)	TestCollectVerdict_SuccessSplit/{omitted,slow,data_error}, and /panicked, which neither spec enumerates (see the resolved readings below)
S31-2	All-absent is a 404 (:1414-1415) and all-failed a 503 (:1416-1417)	Same two as S30-2 and S30-3; both specs state the arms identically.
S31-3	A data error vetoes the merge 404 (:1418-1420)	TestCollectVerdict_AnIneligiblePositionWithholdsTheNotFound
S31-4	A malformed input cannot fail the read (:1421-1423)	TestCollectVerdict_SuccessSplit/data_error, under the same scope note as S30-5
S31-5	A denied position contributes nothing, not even from cache (:1408-1410)	TestCollectVerdict_SuccessSplit/denied covers the verdict half. Withholding the bytes is Step 4's.
S31-6	omitted and timed_out are omissions and make a merge merged_partial; negative_cached is a replayed absence and does not (:1876-1878)	TestCollectVerdict_SuccessSplit/{omitted,slow}. No negative_cached counterpart: Step 1's reconciliation mints no PositionOutcome value for it, so no set expresses it.
S31-7	A shed hosted rebuild fails the merge, it does not degrade it (:1399-1407)	Not reachable here: the veto stops the collection before a verdict is settled. Owned by Step 5.

Error cases

S13's ## Error Cases table has 24 rows (:1103-1126). Six are in this function's territory and get a row below, and two of those six are settled before it is reached; E-9 accounts for the other eighteen.

#	Condition (S13 ## Error Cases)	Tests
E-1	No upstreams configured: NoUpstreams (:1103)	TestCollectVerdict_FailureArms/an_empty_slice_has_no_position_to_ask, .../a_nil_slice_takes_the_same_arm
E-2	Every upstream filtered out by well-formed rules: AllUpstreamsDenied (:1104)	Not reachable here: EligibleUpstreams answers it before any record exists. TestCollectVerdict_SuccessSplit/denied and TestCollectVerdict_FailureArms/a_denial_beside_an_omission_establishes_no_absence cover the partial-denial sets that do reach this function.
E-3	Nothing eligible, at least one position ineligible for a data error: AllUpstreamsIneligible (:1105)	Not reachable here, the same pass-through as E-2. Only the partial ineligible list arrives, which E-5 covers.
E-4	No winner, an absence established, no data-error exclusion: FileNotFoundOnUpstreams (:1106)	TestCollectVerdict_FailureArms/an_established_absence_beside_an_omission_is_the_not-found, TestCollectVerdict_AnAbsenceStandsBesideAContributorSideFailure
E-5	No winner, an absence established, another position excluded for a data error: never FileNotFoundOnUpstreams (:1107)	TestCollectVerdict_AnIneligiblePositionWithholdsTheNotFound, TestCollectVerdict_FailureArms/an_ineligible_position_withholds_the_not-found_an_absence_established, and the same pair for an unserved presence: TestCollectVerdict_AnUnservedPresenceWithholdsTheNotFound, .../an_unserved_presence_withholds_the_not-found_an_absence_established
E-6	No winner, no absence established, no cache entry: UpstreamUnavailable (:1109)	TestCollectVerdict_AnAllOmittedSetIsUnavailable, TestCollectVerdict_FailureArms/no_absence_and_nothing_contributed_is_the_outage, and TestCollectVerdict_TheOutageArmsAreDistinguishable for which arm reports it
E-7	A record nobody filled must not read as a contribution or as an absence	TestCollectVerdict_AnAllUnknownSetFailsClosed, TestCollectVerdict_SuccessSplit/unknown (a contribution beside a zero-value record), TestCollectVerdict_FailureArms/an_unfilled_record_fails_closed_beside_an_absence, and TestCollectVerdict_TheFailClosedArmsNameTheirPosition for the position the message names
E-8	An outcome outside the ten defined values	TestCollectVerdict_AnOutOfRangeOutcomeFailsClosed (4 subtests), TestCollectVerdict_FailureArms/an_outcome_past_the_last_defined_value_fails_closed_beside_an_absence, TestCollectVerdict_NoSuccessCarriesTheUnknownVerdict, and TestCollectVerdict_TheFailClosedArmsNameTheirPosition, which pins the integer this arm reports and keeps it distinct from the unfilled one
E-9	The remaining 18 rows: the two cache-fallback serves (:1108, :1112), AllUpstreamsUnhealthy (:1117), the standalone status rows, WinnerFetchFailed, cancellation, the resolve budget, and the transport, SSRF, header, timeout and cap rows	Not reached: CollectVerdict is pure over records and issues no request, reads no cache, and elects no winner. Owned by the merged resolver work and by internal/remote.

Security considerations

S13's ## Security Considerations (:1130-1144) has ten concerns. None is reached: CollectVerdict performs no I/O, constructs no query, builds no path, logs nothing, emits no audit event, and reads no document bytes. PositionRecord declares no credential-bearing field, and this function reads only Outcome. The one security-relevant property is the fail-open direction, and it is right in both places: an out-of-range value takes the default arm and a declarable zero-value record takes case PositionUnknown, so neither reads as a contribution and neither reaches the not-found arm. That matters because ErrFileNotFoundOnUpstreams becomes a 404 npm and Maven clients cache as a hard failure, which is the same hazard the data-error arm now closes. Error-payload hygiene (:1141-1143) holds: CollectOutcome is an integer enum with no payload, and every error answered is a sentinel matched by identity, wrapped only with a fixed message naming its arm.

Seam conventions

The three internal/AGENTS.md conventions this step's design rests on, listed because the fail-closed arm exists for them rather than for a spec row.

#	Convention	Tests
SC-1	An aggregate "nothing succeeded" is not a not-found	TestCollectVerdict_AnAllOmittedSetIsUnavailable and TestCollectVerdict_AnAllUnknownSetFailsClosed, both asserted not to be ErrFileNotFoundOnUpstreams
SC-2	An exported type's zero value is part of its contract	TestCollectVerdict_SuccessSplit/unknown (a declarable virtual.PositionRecord{} beside a contribution), TestCollectVerdict_AnAllUnknownSetFailsClosed, TestCollectOutcome_UnknownIsTheZeroValue
SC-3	Validate an impossible enum value where it enters, not where it is used	TestCollectVerdict_AnOutOfRangeOutcomeFailsClosed, and the rapid property, whose generator draws three out-of-range values beside the ten defined ones. The entry-side rejection is Step 4's, and !2544 (merged) records it there as its own item; here the scan is the only line of defense, which is why it runs over the whole set.

Resolved spec ambiguities

Nothing was stop-tier and nothing needed the spec author. Three readings were derived rather than read off:

• #### Collection mode:346-349 gives PositionPanicked the merged_partial side. Neither consumer spec names a recovered panic. Step 1's committed reconciliation maps the value onto S30's failed and S31's omitted, and both sit in each spec's enumerated partial set, so the value inherits the verdict its token already carries.

• S30:701 and S31:1806-1810 leave a PositionIneligible or PositionDenied position beside a contribution as CollectMerged, not partial. S30 quantifies merged over the rule-eligible positions and S31 over the eligible ones, and neither lists a denial or an ineligibility in its partial set. The no-contribution arms are where an ineligibility does act, which is why the same value appears on both sides of the file.

  Worth naming where the residual signal lives, because the read-level outcome will not carry it: a corrupt rule row costs the merge a contributor and still records merged, so the per-repository merged_partial rate never moves for it. What does move is the per-read ERROR log and the upstream-exclusion audit event excludePosition (internal/virtual/resolve.go) emits for every data-error reason, plus the ineligible entry in the wide event's positions array and a contributor count one short. That is loud enough, but it is not the signal an operator reaches for first.

• An all-PositionDenied record set is left unasserted for a verdict, because EligibleUpstreams answers ErrAllUpstreamsDenied before any record exists. The rapid property is the only thing that reaches the set, and it asserts of it only that a nil error never comes back with CollectUnknown.

A fourth reading, the verdict for a set whose only non-absent position answered with an unusable document, was settled the other way after review and is carried by !2544 (merged) rather than derived here.

Testing

16 test functions and 34 subtests, all in internal/virtual/collect_verdict_test.go. No build tag, so they run in go_unittests; nothing here touches a database.

Every arm and flag was broken one at a time in a go test -overlay copy and the suite re-run. Re-derived at 22c128961: 37 behavioural mutations, all 37 killed, none surviving and none lost to a compile error. They cover both flips of the denial arm (setting absent on it, and setting ineligible), all four ways of getting the unserved-presence arm wrong (never setting the flag, deleting the arm, ordering it after absent, and dropping its merge-degrading half), both fail-closed arms reverted to the outage sentinel and both stripped of the position they name, all three pairwise unifications of the outage messages, the contribution-first implementation the design exists to rule out, and all four String tokens. An earlier revision of this paragraph said "both ways of unifying two of the three"; three arms give three pairs, not two, and the third is now run with the rest.

case PositionUnknown returns what default returns and no test can separate them, by design, so it is not in that count: .golangci.yaml sets default-signifies-exhaustive: false, so exhaustive is what enforces the explicit arm, verified by deleting it and re-linting.

Diff size

676 reviewable LOC, over the 500 development model asks about, so the split by file group:

File	+	-
internal/virtual/collect_verdict_test.go	495	0
internal/virtual/collect_verdict.go	156	0
internal/virtual/resolve.go	3	9
internal/virtual/resolve_errors.md	4	2
internal/virtual/result.go	4	1
internal/virtual/position.go	1	1

Splitting would not help. 495 of the 676 are the test file, so the production change is 181 reviewable LOC across five files, and the only obvious seam separates the tests from the implementation they constrain, which the test-first authorship flow does not allow.

result.go and position.go are outside the plan's file list for this step and are here for one mechanical reason: CollectOutcome.String makes "unknown" the package's third occurrence of that literal, which trips goconst and flags the two existing String methods as well. The five lines hoist the zero-value token into one unknownOutcomeToken that all three read. resolve.go is here because the new ErrUnsettledPositionOutcome makes ErrZeroValueCoordinate's "the third of this package's wiring sentinels" a count of four. No open merge request touches any of the three.

e2e scenarios

No scenario in docs/testing/ changes or is affected. CollectVerdict adds no route, no handler and no caller outside its own test, so no user-level scenario reaches it; the seam is unwired until the Step 4 fan-out.

Follow-ups

Two, both owned by the step that wires the fan-out, and both recorded as items on !2544 (merged) rather than left in a code comment here.

1. The entry-side rejection of a record whose Outcome is PositionUnknown or out of range. That item also states what closing it does not buy: CollectVerdict is exported over a PositionRecord with no constructor, so any caller can declare one and this scan stays whatever Step 4 does.
2. Naming ErrUnsettledPositionOutcome in the npm error taxonomy. internal/format/npm's virtualServerError lists the three internal/virtual wiring sentinels and not this one, and virtualReadAnswerFor has no arm for it, so it reaches its 500 through the fall-through and is logged as "a reason the taxonomy does not name" rather than the named-infrastructure record its siblings get. The status is already right; only the classification is not. Nothing can reach it until Collect is wired, so the registration belongs with the wiring rather than here, and the sentinel's doc comment says so in the future tense instead of claiming a slice already answers it that way.

Related to #1245 (closed)