feat(remote): derive the Retry-After value from the probe-sweep interval
What
remote.RetryAfterSeconds converts health_check.scheduled_interval into the value the Retry-After header carries on a 503 answered for an unavailable upstream: the interval rounded up to the next whole second, never below 1. That is 300 under the default 5m configuration, and 1 for any legal interval below one second, per the rule the amended S13 fixes. It lands the derivation that item 3 of #609 (closed)'s scope depends on; asserting the value where the 503 responses are built stays with the slice and fill-handler MRs, and #609 (closed) now says so.
No handler wiring ships here because no 503-writing site for the covered verdicts exists on main yet: Maven's MapFetchOutcome decision surface has no non-test caller and the fill paths answer 501 (internal/format/maven/remote_stub.go), npm's remote dispatcher slots are unset and serve the interim 501, OCI remote reads are 501 stubs (internal/format/oci/remote_stub.go), and no virtual HTTP slice exists. The slice and fill-handler MRs that render these responses set the header from this helper. Its doc comment scopes the rule by verdict rather than by Go sentinel, so a decision surface that reaches the verdict without holding one of the three sentinels (Maven's fallback-or-unavailable mapping is the standing example) still reads itself as covered.
One case stays open, and the doc comment names it rather than settling it: a 503 that relays a status the upstream itself produced. S31 forbids adding this value on its WinnerFetchFailed row, while S15 requires the header on every 503 upstream_unavailable, which its propagated-upstream-5xx mapping produces, and S16 requires the handler to supply its own value where the upstream sent none. Relaying the upstream's own value is not possible either: forwardableHeaders in internal/remote/operations.go carries no Retry-After. Reconciling S13, S15, S16 and S31 is a spec amendment, not a decision this helper makes; #687 tracks it, with the six spec locations and the two shapes the amendment could take.
Why
S13, S15, and docs/dev/api-style.md each require the header, and until the S13 amendment (!1502 (merged)) no spec stated a number, so no slice could write an assertion for one. Deriving it in one exported helper means every renderer answers the same value, and the round-up plus the floor of 1 for a legal sub-second interval are decided once instead of per handler.
Test plan
TestRetryAfterSeconds pins the derivation table: 5m answers 300, 500ms answers 1, an exact whole second does not bump, a fractional second rounds up, 1h-1ms answers 3600, and the zero and negative rows pin the exported function's non-positive contract. Full untagged go test ./... passes, and golangci-lint reports no issues on the touched package.
The config side of the plumbing is already covered on main: the 5m default in internal/config/virtualrepositories_test.go, and a non-default interval through the mapping into remote.HealthCheckConfig.ScheduledInterval in internal/config/remotemapping_test.go.
No e2e scenario is added or affected: no response on main renders these 503s yet, so there is no client-visible behavior to catalog. The scenario belongs to the slice MRs that set the header.
Spec coverage
Spec: docs/specs/S13-virtual-remote-foundation.md
Scoped to work item #609 (closed) item 3, the fixed Retry-After value. Rows cover the criterion and error cases that name the header; the spec's other rows belong to the merged S13 MRs and the pending slice MRs.
Acceptance criteria
| # | Criterion | Tests |
|---|---|---|
| AC-1 | Retry-After value: a 503 mapped from UpstreamUnavailable, AllUpstreamsUnhealthy, or the blocked Fetch path of the direct-request-to-unhealthy-remote criterion carries Retry-After equal to health_check.scheduled_interval rounded up to the next whole second; 300 under the default configuration; 1 for any interval below one second | Derivation: TestRetryAfterSeconds (new). Header-on-response: no 503-writing site exists on main for these verdicts (Maven fill paths answer 501 via internal/format/maven/remote_stub.go, npm remote slots are unset 501s, OCI remote reads answer 501 via internal/format/oci/remote_stub.go, and no virtual HTTP slice exists); the header assertion is owned by those slice MRs. Decision-level flag already pinned by TestMapFetchOutcome_Every503CarriesRetryAfter in internal/format/maven/remote_errors_test.go |
| AC-2 | Direct request to unhealthy remote: Lookup still runs, the Fetch path is blocked with a structured error | Existing, unchanged: internal/remote/standalone_test.go covers the ErrRemoteUnhealthy return. Its 503 + Retry-After rendering has no handler site on main (see AC-1) |
Error cases
| # | Condition | Tests |
|---|---|---|
| E-1 | Virtual: no winner, no absence established, no cache entry -> UpstreamUnavailable, "upstream unavailable" with Retry-After | Value derivation: TestRetryAfterSeconds. Sentinel production: existing internal/virtual/resolve_test.go. Header: no virtual HTTP slice on main; owned by the virtual slice MRs |
| E-2 | Standalone or virtual: transport failure, no cache entry -> UpstreamUnavailable; format handler maps to 503 with Retry-After | Value derivation: TestRetryAfterSeconds. Maven decision flag: existing internal/format/maven/remote_errors_test.go. Header: Maven fill handlers are 501 stubs on main; owned by the fill-handler MRs |
| E-3 | All eligible upstreams unhealthy at Phase 2 entry -> AllUpstreamsUnhealthy, "upstream unavailable" with Retry-After | Value derivation: TestRetryAfterSeconds. Sentinel production: existing internal/virtual/resolve_test.go. Header: no virtual HTTP slice on main; owned by the virtual slice MRs |
Security considerations
| # | Concern | Tests |
|---|---|---|
| S-1 | None of the spec's security considerations name the Retry-After value | Not applicable to this commit |
Related to #609 (closed)