feat(oci): the virtual resolution facade and its error mapping (S32 plan: 11/19)
Summary
Step 11 of the S32 container-virtual plan: the resolution facade, and the mapping from S13's resolution taxonomy onto the OCI envelope.
Three new files:
internal/format/oci/virtual_resolve.go— one call that takes a namespace, a virtual repository, and a target, and answers a resolved read or a mapped error, plus theremote.FetchOptionsa virtual read runs its winner's Fetch under (the size-cap class per reference shape, and the expected digest read off the assembled path rather than derived a second time).internal/format/oci/virtual_errors.go— the mapping. One arm per resolution verdict, wiring sentinel, propagated sentinel, and addressing fault, and the fourRetry-Afterclasses.internal/format/oci/virtual_errors.md— the sidecar arguing the three points where this mapping departs from npm's and fromremote_errors.md, and the two that look like departures and are not.
The resolution deadline the step originally carried is not here: #1139 (closed) landed
resolve_timeout, the remote.ResolutionConfig field, the sentinel, the
configuration mapping, and the budget mechanism in internal/virtual/resolve.go
before this step opened. This MR consumes all six and writes none of them.
Three files sit outside the plan's Files entry:
internal/format/oci/remote_errors.go(+8/-2), in two hunks.remoteFillShedRetryAfterSeconds() intis extracted from the existingremoteFillShedRetryAfter() string, which now calls it, so the virtual arm can stamp the header off an int. The second hunk replaces the literal"1"inmapRemoteFetchFailure's coalescing arm with the new named constant. That arm is a live standalone-remote serving path; the change is value-preserving (strconv.Itoa(1) == "1"), andremote_errors_test.goalready pinsUpstreamRetryAfteras the literal"1"from the other side.internal/format/oci/remote_relay.go(+3): theremoteCoalescingRetryAfterSecondsconstant that hunk names.internal/format/oci/virtual_assembly_test.go(+6/-1): one test function, and the plan books both the file and the acceptance criterion it covers to Step 10b.TestVirtualUpstreamAssembly_DeniedAndHealthSkippedPositionsAcquireNoFlightasserted nothing about the health gate: the fixture's default doer answers404, so an ungated unhealthy remote at position 1 would be probed, miss, and leave the hosted hit at position 2 winning regardless, and a probe builds no flight either way. Verified by mutation rather than by reading: writingremote.HealthUnknownfor every row invirtualHealthStatusesleft that test green, and withcvaProbeHitthe same mutation fails it atwinner fetch at position 1.virtualHealthStatusesandVirtualHealthStatusReaderare untouched. The criterion was not unguarded in the meantime, since this MR's ownTestVirtualResolution_AnAllUnhealthyListIsNeverProbedalready fails under the same mutation.
Governing ADRs
- ADR-004 (data and application limits):
container.manifest_max_payloadis applied to every manifest-class fetch, and a non-positive value is refused at construction rather than at first read. A relayed429always carriesRetry-After. Conforms. - ADR-008 (content-addressable storage):
ExpectedDigestis taken from the assembled path's last segment, already canonicalsha256:<hex>. A tag-kind read carries none, which is correct: nothing is content-addressed at that point. Conforms. - ADR-007 / ADR-001: every resolution carries
NamespaceIDbesideContainerVirtualRepositoryID, so the read stays inside the namespace the composite foreign key enforces. Conforms. - ADR-009 (API design): adds no route, method, or client-facing URL, and introduces no OCI error code. It maps onto codes this package already declares. Conforms.
- ADR-021 (authorization): no authorization decision here; authorization runs
before
kindis read. The404onremote.ErrAllUpstreamsDeniedis an upstream allow/deny rule verdict, not an authorization one. The500arm composes no envelope field from the error, so no upstream host, URL, or response header reaches a client.
Mirror freshness re-measured on 2026-09-23 with scripts/adr-freshness.sh: STALE
on one unsynced commit, which adds pipeline_id to ADR-020's gitlab.job claim
and touches 020_authentication_flow.md alone. This change makes no
authentication decision and reads no token claim, so it is unaffected. One
internal ADR never mirrors locally and is not checked here.
Reviewable size
3070 reviewable LOC, past the 500 development-model.md asks a justification for.
The split by file group:
| Group | LOC |
|---|---|
| Non-test Go | 563 (virtual_errors.go 362, virtual_resolve.go 188, remote_errors.go 10, remote_relay.go 3) |
| Tests | 2433 (virtual_errors_test.go 1524, virtual_resolve_test.go 653, virtual_resolve_internal_test.go 249, virtual_assembly_test.go 7) |
| Prose | 74 (virtual_errors.md) |
Splitting would not help, because the seam a split would cut is the one thing this
step exists to hold. The taxonomy and its mapping are one artifact:
virtual_errors.go answers one arm per value internal/virtual and
internal/remote can return, and the acceptance criterion is that every such
value has a named arm. Landing half the arms means landing a mapping whose
fall-through silently answers 500 for the other half, which is the exact failure
the per-value discipline exists to prevent. The suite is four fifths of the diff
and is derived from that taxonomy rather than written alongside it:
virtualFallbackExclusionSentinels walks the AST of remote.FallbackEligible and
diffs its return false arms against the table, so the rows are not a list a
reviewer has to audit by eye. virtual_resolve.go is the one call the mapping
hangs off, at 188 lines.
Testing
Test-first: test(oci) lands the suite and two panic skeletons, feat(oci) the
implementation.
- The propagated sentinel set is derived, not transcribed.
virtualFallbackExclusionSentinelswalks the AST ofremote.FallbackEligibleininternal/remote/errors.goand diffs itsreturn falsearms against the table, so a sentinel added to that predicate fails this test instead of shipping a500. The two arms the predicate cannot see,remote.ErrFillAbandonedandvirtual.ErrZeroValueCoordinate, are named against the sentinels themselves. - Every no-winner branch is asserted by name, not counted:
NoUpstreamsandAllUpstreamsDenied→404,AllUpstreamsUnhealthyandUpstreamUnavailable→503,AllUpstreamsIneligible→500 INTERNAL. - A malformed tag is driven through
NewVirtualCoordinate, not injected.ErrVirtualTargetanswers the route's own404 MANIFEST_UNKNOWNper S32's Error Cases row, and the test builds the coordinate rather than handing the mapping the sentinel, so it pins the chain and not just the arm.ErrVirtualHostedReferencestays on500, and the comment says why. ErrResolverCancelledand the budget expiry are two arms, not one, with a test per cause: a departed caller gets499and no envelope, a budget expiry gets503withRetry-After: 1and a parseable body. The expiry keeps its own log record, sinceresolve_timeoutis the one condition here an operator sets.- The four
Retry-Afterclasses are asserted separately: the health sweep (300), the coalescing class (1), the deterministic floor (5), and the jittered fill-shed window. - Every content-refusal sentinel runs bare and wrapped in
remote.ErrCacheFill, the shape production produces. Without the wrapped row, a reorder letting theErrCacheFill500 claim those refusals first turns their503s into500s with the suite still green. - Every 500 the mapping owns is recorded as named.
virtualServerErrorcovers this package's own 500-class values alongside the ones it receives, so a half-set credential, an auth status outside the taxonomy, a failed discovery write, a manifest past the read-back ceiling, and a facade that bypassed its constructor no longer record as reasons the taxonomy does not name. The same split applies at the disconnect guard: the499is unchanged and only the record moves. - The digest precondition is asserted here rather than left to
remote's panics. Every digest-addressed read'sExpectedDigestvalidates, names the canonical algorithm, and equals its path's last segment, which isSingleFlight.Fetch's own rule, so a future change to the assembled path's format fails in this package. Every other shape names no digest, with the reason an empty value is correct recorded against a later attempt to salvage a partial one. - The winner's non-2xx is a second table, one row per status
mapUpstreamStatusnames by value, plus an unnamed 3xx and an undefined status. A relay decision no response can carry fails closed to this arm's own500, which is the obligationRemoteFetchOutcome's doc assigns to the renderer. - Three positional fixtures: a failing higher position with a
404at a lower one answers404; a pull whose only cached copy sits at a denied position does not serve it; and a data error at a higher position withholds the404a lower one established, with a control proving the same list answers404once the defective row is repaired. - The redaction suites cannot pass vacuously. Both assert an absence, so a fixture that stopped embedding the credential would pass them whether or not any arm redacts. One case fails if that fixture ever goes inert.
The five resolution-deadline cases and the configuration round-trip are #1139 (closed)'s
and already on main, in internal/virtual/resolve_budget_test.go and
internal/config/virtualrepositories_resolvetimeout_test.go.
Local: go build ./... and go test -race ./internal/format/oci/... ./internal/virtual/... pass, golangci-lint run ./internal/format/oci/...
reports 0 issues, and scripts/ci/check-comment-caps.sh is clean.
golangci-lint run --build-tags=integration --max-same-issues=0 --max-issues-per-linter=0 --uniq-by-line=false ./internal/format/oci/... reports
8 issues, all in virtual_assembly_integration_test.go and all pre-existing on
main, which this branch does not otherwise touch.
No e2e scenario is added: nothing routes to this facade until Step 12's serve
surface and Step 14's composition root. No docs/user/** change either, for the
same reason — no customer-visible surface is reachable yet.
Deferred
- Widening
RemoteOperationsDeps.ManifestMaxPayload's "both constructors" sentence to coverNewVirtualResolution, which takes the same pair. The sentence sits inside a 22-line comment block that predates the comment caps, and touching it charges the whole block, so it goes to the wiring step that makes the claim wrong rather than being rewritten here. Booked on Step 14 by !2904 (merged). - The virtual repository's identity on the mapping's own records.
logVirtualMappedFailureemitserror_message, plusupstream_statuson the winner refusal, andwriteVirtualReadErrortakes no attrs, so an operator asking which virtual repository is503-ing joins to the access log oncorrelation_id. Step 17'soci_virtual_resolveis the natural home, but its acceptance nameswinner_repository_idrather than the virtual repository being read, so this is not closed as written and Step 17 should pick it up. Adding the attrs parameter here would be a seam with no caller until Step 12 mounts the route. Booked on Step 17, asnamespace_idandrepository_idon the error mapping's records, by !2904 (merged). - The cache-fallback half of a failed token exchange. The new
*UpstreamAuthErrorarm answers the statuses S16 fixes, but it maps onhasCachefalse, so a status that produced no token never serves a cached copy the way the standalone arm can. Whether it should is a question aboutremote.FallbackEligiblein the foundation rather than about this mapping, so it is not closed here. Tracked in #1431. - The relayed
Retry-Afterclause of S32's WinnerFetchFailed row.remote.WinnerFetchFailedErrorcarries onlyStatus, so an upstream's own hint cannot reach this relay and the deterministic floor goes out instead. Tracked in #687.
Related to #291 (closed)