feat(config): add virtual_repositories.resolution.resolve_timeout

Lands the four configuration surfaces for virtual_repositories.resolution.resolve_timeout, the key !2249 (merged) declares on S13's shared resolution block.

What this changes

Surface Change
proto/artifactregistry/config/v1/config.proto string resolve_timeout = 5 on ResolutionConfig, plus the regenerated gen/**
internal/config/virtualrepositories.go the 90s default, the durationOrDefault arm, and ResolutionConfig.ResolveTimeout
config.example.yaml the key under virtual_repositories.resolution
docs/dev/configuration-reference.md the row

The default is the spec's, and this MR does not re-derive it

90s covers the slowest probe sweep: 20 upstreams (ADR-004) in ceil(20/10) waves, where a container probe can pay a token exchange before its HEAD and a fresh exchange again on a 401, so 2 * (10s + 5s + 10s + 5s) is 60s at the probe_concurrency, probe_timeout and container_remote.token_exchange_timeout defaults. The remaining 30s is headroom, because a value sized exactly to the worst case is cut by ordinary jitter.

TestVirtualRepositories_ResolveTimeoutClearsTheContainerProbeSweep reads those three defaults back out of a loaded Config and recomputes the sweep, so the derivation is enforced rather than described: a move in any of the three fails that test. It asserts the budget clears the sweep and does not also pin 90s, because a require on the value ahead of that assertion is what would stop the assertion ever failing. TestLoad_VirtualRepositories_ResolveTimeoutDefaults is where 90s is pinned.

Two things this deliberately does not add

No cross-field lower bound. S13's derivation moves with the three knobs above and explicitly not with request_total_timeout, "which now bounds a leg outside the budget", so an ordering assertion against it would pin a relation the sizing does not rest on.

No bespoke non-positive check. durationOrDefault reaches parseDurationField, which already rejects a non-positive duration with errDurationNotPositive, while an omitted key still defaults. Unlike single_flight_wait_timeout and health_check.scheduled_interval, a non-positive value here panics nothing downstream (it yields an already-expired context and no more), so validateKnobBounds gains no second arm for it. The rejection is covered as load-time behavior instead.

Nothing applies the value yet

The reference row and the config.example.yaml comment both say so, rather than describing a runtime effect the tree does not have. The deadline is armed inside virtual.Resolver and released at winner selection, which needs its own field on remote.ResolutionConfig. #1139 (closed) carries both, and extends the reference row with the runtime consequences when it lands.

Because that second half is a seam rather than a sentence, it is recorded in the tree and not only here: vrFieldNotCrossing in internal/config/remotemapping_test.go names ResolveTimeout against #1139 (closed), and the case reading it fails once the field gains a remote.ResolutionConfig counterpart.

Overlap with !2255 (merged)

!2255 (merged) carries six of this branch's nine files, from before !2249 (merged) settled the derivation: its default is 40s, derived through request_total_timeout, and it adds the lower bound this MR omits. Re-derived at f90da5633 against !2255 (merged)'s current diff, the six are config.example.yaml, docs/dev/configuration-reference.md, gen/artifactregistry/config/v1/config.pb.go, internal/config/virtualrepositories.go, internal/config/virtualrepositories_resolvetimeout_test.go, and proto/artifactregistry/config/v1/config.proto. The three this branch adds on its own (internal/config/remotemapping_test.go, internal/config/testdata/virtual_repositories_happy_path.yaml, internal/config/virtualrepositories_test.go) are not in !2255 (merged). Whichever of the two lands second conflicts on all six, and no pipeline reports it. The intended order is this MR first, after which !2255 (merged) rebases, drops its copies, and keeps its internal/format/npm work.

S32's Step 11 has to consume the key too, and its plan does not say so

The settlement recorded in docs/plans/2026-08-27-s32-container-virtual.md is a two-way race: "whichever of S31's Step 12 and this step lands second consumes the key rather than declaring it" (:2910), and Step 11's own Files list still books the proto, the loader, the example, and the reference for declaring it (:2877). This MR is a third actor and lands the key ahead of both, so both must now consume and the plan says so for neither. Open !2276 (merged) edits exactly that passage and expands the two-way framing rather than retiring it.

There is no git conflict: !2276 (merged) touches only the plan file. The cost is a Step 11 author who follows the plan and writes a duplicate proto field 5, and the bad version of that is a hand-resolved conflict that keeps !2255 (merged)'s 40s, which is below the 60s this default is sized against. The plan correction is owed and is not in this MR; !2276 (merged) is already in that paragraph and is the cheapest place for it.

Tests

Four in internal/config/virtualrepositories_resolvetimeout_test.go:

Test Covers
TestLoad_VirtualRepositories_ResolveTimeoutDefaults the omitted key takes 90s, with every other key of the block present
TestLoad_VirtualRepositories_ResolveTimeoutUnparseable an unparseable value reports the dotted path on DurationParseError.Field
TestLoad_VirtualRepositories_ResolveTimeoutOutOfRange "0s" and "-1s" rejected at load, each naming the field
TestVirtualRepositories_ResolveTimeoutClearsTheContainerProbeSweep the three defaults the value is derived from

Plus, in the shared suites, the coverage the four sibling resolution keys already have: the fixture carries a distinct resolve_timeout: "46s", TestLoad_VirtualRepositories_HappyPath asserts it round-trips, and both ..._PartialBlockDefaultsOmittedFields and ..._AbsentBlockDefaults assert the 90s default. A standalone round-trip case would now be that first one's twin, so it is gone rather than duplicated.

And one guard in internal/config/remotemapping_test.go: TestRemoteConfigFrom_EverySourceFieldCrossesOrIsDeclared. ResolveTimeout is the one VirtualRepositoriesConfig field RemoteConfigFrom does not map, and assertRemoteConfigFullyMapped cannot report that, because it reflects over remote.Config and the field does not exist there. The new case walks the source and requires every field either to cross or to be listed in vrFieldNotCrossing against the work item that closes it. Listing a field that has since gained a counterpart fails too, so the entry retires itself.

Review round: what the two follow-up commits change

docs(config) and test(config) on top of the original commit close a self-review of it. Nothing about the loader, the default, or the schema moved.

  • Which knobs bound a leg. The comments and the reference row said the other four knobs in the block "bound one leg each". probe_concurrency and max_concurrent_fills_per_repo are counts, and the reference row's own parenthetical gave three legs for four fields. Fixed at all four sites: the Go struct doc, the ResolveTimeout field doc, the proto message doc, and the proto field comment, plus the row.
  • Present tense for a sweep that does not happen. The row said the default "covers the slowest sweep the service performs". virtual.NewResolver has no caller outside its own package, so no request reaches a resolution sweep. The row now states how the default was sized without claiming the sweep runs, and "yet" is gone from both the row and the example comment, which the configuration reference's as-is rule asks for.
  • Coverage in the shared suites, and the source-side mapping guard. Both are in ## Tests above.
  • One unfalsifiable assertion, in ## The default is the spec's above.

Two residuals this round does not take:

  • RemoteConfigFrom's own doc comment still reads as a complete straight copy. Adding a clause to it means compressing its ten prose lines to the three-line cap scripts/ci/check-comment-caps.sh applies to an exported doc the moment a diff touches it, and that prose is not this MR's. The new test is the stronger record anyway: it fails, rather than reads wrong, when the seam moves.
  • The same "one leg each" phrasing in merged S13 (docs/specs/S13-virtual-remote-foundation.md:864). A spec correction is a spec MR; this one does not contradict the spec, it is more precise than it.

Checks

go test ./internal/config/... and go build ./... pass. The pinned golangci-lint 2.13.2 reports 0 issues on ./internal/config/... under a per-run GOLANGCI_LINT_CACHE (the 2.12.2 binary on PATH cannot load .golangci.yaml). The full pre-commit run over all nine changed files exits 0, markdownlint-cli2, vale, lychee, buf lint, buf format, buf generate freshness, go-test, go-mod-tidy and the comment-caps hook included. scripts/ci/check-comment-caps.sh --base origin/main passes too, which is the whole-branch diff CI checks rather than the staged blocks the hook sees.

Both mutations of the new mapping guard were run: dropping the vrFieldNotCrossing entry reddens it, and adding a ResolveTimeout to remote.ResolutionConfig reddens it the other way.

242 reviewable LOC across nine files, excluding the 27 regenerated lines in gen/artifactregistry/config/v1/config.pb.go.

No e2e scenario is added or affected. The MR lands a configuration surface that no read path reads, so no request behavior changes; docs/testing/ gains its scenario with #1139 (closed), which is what makes the bound observable.

Closes #1069 (closed)

Edited by Dzmitry (Dima) Meshcharakou

Merge request reports

Loading
Loading