feat(virtual): fall a failed position back to cache, and honor a veto (S13 collection mode plan: 5/6)

📦 What this does

Step 5 of the shared internal/virtual collection mode: the per-position cache fallback, and the veto that ends a read.

A consulted remote position whose contributor failed now contributes its cached copy where it holds one, through a second call to the same contributor at the cache-only entry point. A contributor may also veto the whole read from its own position, where a copy served in its place would make the answer wrong rather than smaller.

The plan revision this implements is the one !2700 (merged) landed, now on main: it rewrote one paragraph inside ### Step 5, ruling that the PositionDataError-versus-PositionOmitted distinction is asserted on the record unconditionally rather than moved to the verdict.

🔍 The fallback arm, and what it excludes

The record comes from the contributor's failure kind and the cache-only call's own result, because nothing outside the record separates the two omissions once a contribution is in the set:

Contributor's failed report	Cache-only call	Record
data error (over-cap or unparseable)	produced a document	PositionContributedFromCache
data error	produced nothing	PositionDataError
transport or 5xx	produced a document	PositionContributedFromCache
transport or 5xx	produced nothing	PositionOmitted

An ended read outranks both columns. A position cut by a veto or by a caller that stopped reading records PositionOmitted whatever it failed with, because it was consulted and could not answer; the merged plan rules that for the veto, and routing such a position to the data-error arm would misreport what the end of the read cost. Both arms that reach the pairing carry a case for it.

An expired budget is not such a signal, and the arm treats it as one no longer. The collection still answers from what settled, so a position that already paid for its Lookup row still gets the cache-only call, and books the from-cache contribution where the copy answers. That call runs on the read's caller-derived context, so the caller's own context is the only bound on it; no second budget caps the post-expiry work.

internal/virtual/resolve_budget.md carries that rule, and its sentence is phrased for the winner path — "an expiry with no winner tries the consulted positions' cached copies on the caller's context". Collection mode reads it by intent rather than by that clause: an expiry should not discard a cached copy the read already paid for. A post-expiry cache read that then fails keeps its log line, since that is a store fault rather than the collection's own doing.

Four exclusions are scoped by name rather than by what happens to work:

• A reported absence is not a failure. The upstream answered 404, so a stale copy would contradict it and remove the not-found the collection owes.
• A health-marked position already spent its one cache-only call at the gate; covering it here would ask one contributor twice in a read.
• A hosted spec carries no remote handle to dereference.
• A Phase 1 miss holds nothing, which is where the arm parts from a cache that answered and could not serve. Those are separate records and separate log lines, the distinction serveFallback already keeps.

A fifth exclusion the plan asks of this step is deliberately absent: a position shed on resolution.position_wait_timeout. Naming it here would forward-reference what Step 6 introduces, so this MR merges first and !2745 (merged) will add that exclusion to the predicate, together with the plan's row for it. That MR owns the residual, and a merge-request dependency enforces the order rather than leaving it to the stack's existence.

No second cache query. The arm reads the remote.LookupResult Phase 1 stored, because both consumer specs make the per-read statement count an acceptance criterion and this arm fires during an outage, at up to twenty positions per read. There is no remote.CacheFallback call; the test fixture's sealed cache store fails the suite on any touch.

⛔ The veto

ErrReadVetoed is returned by a contributor and returned by Collect to its caller, wrapped to name the position. Collect stops handing contributions back, cancels every position still running, waits for each to settle, releases what it still holds but not what the sink already took, and keeps every record.

Which positions may veto is the arming slice's to state, per S13's Collection mode, so a veto is honored from any position and through either contributor entry point. The hosted gate on the first hand-back still covers hosted positions only, so a slice arming a veto elsewhere accepts that a lower position may already be the caller's. resolve_errors.md says so rather than leaving it to be found.

The vetoing position keeps the record its own Lookup answered with, chosen by where its copy came from rather than by the row's freshness: PositionContributed where it reached its own upstream, fresh row or stale, PositionContributedFromCache where a health mark or the fallback arm means the copy came from its cache, and PositionOmitted on a miss, which holds no copy to name. Reading the row's freshness here gave one unmarked position two records on the same row, a contribution for succeeding and a cached copy for vetoing, and a hosted position is never health-read. Which positions token an npm slice then emits for the armed case is open with the S31-npm-virtual.md author: that spec asks for hit_fresh in its acceptance criterion while also saying the token carries Phase 1's freshness, and the armed veto always follows a rebuild that ran because no fresh row existed. This step follows the token definition and settles nothing for S31.

One sentinel, three S13 return values, and who maps them. S13's #### Collection mode assigns a vetoed read three different return values by what the vetoing position established: FileNotFoundOnUpstreams for an established absence, UpstreamUnavailable for an established nothing (transport failure or 5xx), and no verdict of S13's own for a presence it could not serve, which the arming slice maps in its own error surface. Collect supplies one sentinel for all three, and its records separate none of them — vetoingOutcome derives the vetoing position's record from its Phase 1 cache row, so the first two both land on PositionOmitted. The mapping is therefore the arming slice's, and the veto now travels wrapping the contributor's own error rather than the bare sentinel, so a slice recovers its case from the error chain. errors.Is(err, ErrReadVetoed) still matches, because the contributor's error already carries the sentinel — that is how the veto was recognised.

The residual is unreachable today. S31 arms the veto on exactly one condition, a hosted position whose Lookup Hit and whose fenced rebuild commits no row, which is S13's third case; it states that a hosted position whose Lookup misses establishes an absence and vetoes nothing, and that a shed remote position on a merge read is omitted rather than vetoing. S30 arms no per-position veto at all. So no wired slice can reach S13's first two cases, and whether the foundation should answer them itself is left open rather than decided here. That question is owned by #1407.

📏 Size

1384 insertions and 43 deletions across six files, 384 over the 1000-line ceiling, shipped whole rather than split. By file group:

Group	Added	Share
production (collect.go 209, position.go 10)	219	16%
tests (collect_test.go 1028, collect_internal_test.go 95)	1123	81%
docs (resolve_errors.md 41, the plan's Status row 1)	42	3%

The production surface barely moved. It was 213 lines when this MR opened and is 219 now, so the behavior-carrying code a reviewer reads is essentially what it was at the first round. The growth is tests, 815 to 1123, and docs, 19 to 42.

That growth is review rounds, not scope creep. Every case added since the MR opened came from a finding: the veto record's overwrite, the failure line's placement, two faults collapsed onto one line, the cut-versus-data-error pairing, the record that turned on the row's freshness, and the expiry that kept skipping the cache arm. Several exist because the fixtures could not distinguish the shapes at all — both failure arms returned the same error, and no case had two positions vetoing together, or a parked position below a vetoing one.

The step's own Files: list is a single production file and there is no second concern to cut along. Splitting code from tests would break the test-first authorship the branch was built on, and splitting the tests among themselves would separate each arm from the negatives that bound it — the three exclusions only mean something beside the arm they exclude.

➕ Two files beyond the step's list

internal/virtual/resolve_errors.md is not in the step's Files: list. It is here because the step makes it wrong: it called ErrPositionAbsent "the package's sixth exported sentinel" where there are now nine, and it is the document collect.go points at for every surface Collect returns, which ErrReadVetoed joins. Same reasoning step 4 used when it swept eight count sites in the npm error sidecar.

docs/plans/2026-09-04-virtual-collection-mode.md carries this step's Status row, which a step MR does not normally write: rows are batch-recorded from a dedicated docs(plans) branch so that adjacent-row edits cannot conflict on a merge train. It rides this MR because the author asked for it directly. Only row 5 is touched; row 4 is left to step 4's author, who has since recorded it on main. That single-writer rule earned its keep in the meantime — recording row 5 here is exactly what conflicted when this branch was rebased onto main after step 4 merged, and the resolution kept both rows.

⚠️ One deliberate departure from the plan

The plan argues against a per-position log line on this arm: "twenty WARNs per read would bury the very signal the winner path's single line carries." A position the read serves nothing from gets one anyway.

The rule the code lands on is one line per position the read serves nothing from, and never two. A position whose cached copy answers logs nothing at all: the failure line sits below the cache-fallback gate, so recovering from cache costs the read no line. A position that ends up contributing nothing logs exactly one — the upstream's cause when it had no cached row to try, or the store's cause when the fallback call itself failed. Those two are separate lines because a dead object store and a busy upstream are otherwise the same omitted token, which sends an operator after the wrong fault; the winner path already refuses to collapse them.

That is the departure: the plan's figure assumes a line per position on the arm, and the arm keeps one for the positions that lose their document. What it does not keep is a line on reads that succeed.

Log volume, and sampling. The accepted worst case is up to 20 WARNs per read — one per position, under ADR-004's cap of 20 upstreams per virtual repository (docs/adr/004_data_and_application_limits.md) — reached only when every position loses its document. These records are unsampled: internal/logging's sampler acts only on records carrying the access message and passes every other record through untouched, so the figure above is what reaches the pipeline.

A gap this arm creates. An upstream serving documents the slice cannot use — over-cap, unparseable, directive-carrying — while its cache stays warm now logs nothing, and its record reads PositionContributedFromCache rather than PositionDataError. Nothing else surfaces it either: the health sweep probes reachability and credentials, not document validity. The reads stay correct, served from cache, so this is a blind spot rather than a defect, and it is recorded here rather than left to be found.

🧪 Testing

Unit tests only, in internal/virtual, against the package's own doubles — no format package, no database, no HTTP, matching the plan's Testing Strategy.

Every arm this step adds reddens the suite when deleted; that was checked by mutation, not assumed. Three of them were green-when-deleted at first review and earned their case from it: the veto check inside the fallback's own call, awaitGate's veto arm, and the detached wait's cancel.

Two races no fixture over the exported surface can schedule are pinned white-box in collect_internal_test.go: the two-signal read of an ended read, and the select-shuffle that lets a hand-back or the gate take a settled arm on a vetoed read.

go test ./... green (67 packages), -race -count=2 green, golangci-lint 0 issues, comment caps and vale clean.

No e2e scenario is added. The seam has no client-facing route — nothing imports Collect yet — so no scenario in docs/testing/e2e/ exercises it. The rows belong to the consuming slices, and the plan books S31's Step 19 for the npm merge.

No user documentation is affected. There is no customer-visible surface: Collect has no caller, so nothing here is reachable from a request.

🛡️ Review

/security-review found no vulnerabilities. It confirmed the exclusions hold: a rule-denied position contributes no cached bytes past its denial, gateMark opens no HTTP path to a health-marked upstream, and no position reads another's lookup, handle or contribution.

Related to #1248 (closed)