docs(specs): S16 states what the credential read does and answers

What

S16's credential_read stage arrived with the metric domain and nowhere else. This MR carries it through the rest of the spec: the design narrative a reader meets first, the client-facing status in Error Cases, the matching acceptance criteria, and the leg the observability prose books.

The code side is already on main. The S04-A per-format credential columns Step 7 MR merged as 883a317ad and shipped both the read leg, assertCredentialsReadable in internal/format/oci/remote_auth.go, and its credential_read booking. So the spec is the lagging side here, and every disagreement this MR removes is live on main rather than pending a merge.

The stage domain

gitlab_artifact_registry_oci_remote_auth_discovery_failures_total declared stage as four values; the credential read books a fifth. The family's description widens with it. Four of the stages sit inside challenge discovery, and this one precedes them, because the unknown-verdict arm reads the stored credential before it sends a discovery HEAD. A refused read therefore ends the acquisition with no upstream contact at all.

Two counting arguments in the same section follow the new value: four of the five legs attempt no write, and all five would move no oci_remote_* metric without this family.

Why the value is not on the exchange families

Booking the unknown arm's refusal on oci_remote_token_exchanges_total would also put a leg that reached no exchange into oci_remote_token_exchange_duration_seconds, because one helper observes both families. That contradicts the histogram's own rule, which is that it observes the outcome an exchange reached.

The bearer arm is the other half of that, and the section now says so. A row already memoized bearer sends no discovery HEAD and reads the credential inside the exchange, so a refusal there is a local refusal that reached no endpoint and books internal_error on both exchange families. The split is by leg, not by error class, so it does not follow the client-facing status. The 404 re-discovery path reads the same way, and a still-cached token books nothing at all.

The same passage loses a false quantifier. "the one stage an operator must not read as an upstream fault" is a claim over all five stages and fails on write_failed, which is this service's own storage, and on transport, which covers a HEAD this service could not send.

The design narrative

Auth-challenge and token-exchange opened the unknown-verdict arm with the unauthenticated HEAD, in the prose and in the sequence diagram alike, so a reader following the design section rather than the metrics appendix had no way to learn the step exists. Challenge discovery. now states the read and why it runs there: S13's fail-closed rule applied ahead of the verdict write, whose own table puts bearer discovery over a credential-bearing row on the failing arm. A row holding no credential unit reads as anonymous rather than refusing, which is what keeps an anonymous remote on this path.

Token exchange. states its own read: per exchange rather than once when the authenticator was built, the only read on the bearer path, nothing read on a token-cache hit, and a refusal failing before the GET is built. The diagram gains the three reads that match, plus one note for what a refusal does wherever it lands.

The client-facing status

Error Cases said nothing about what a client receives when the read refuses, so the zero-status UpstreamAuthError arm answers cache-or-503 for a permanent refusal and a transient read error alike. Two rows now split them.

A refusal by policy answers 500 INTERNAL, with no cached copy and no Retry-After. That is the reasoning three rows of this table already give. The half-set credential row answers 500 because the fault never clears on retry and no request left this service, so S13's cache-fallback definition does not reach it, and the kind=2-with-no-row and manifest read-back rows cite it for the same status. A single cache-or-503 row would have sat in a table that argues against it.

A read that failed transiently keeps the cache-or-503 UNAVAILABLE route with Retry-After, justified on its own ground rather than borrowed. The half-set row's second reason, no request leaving this service, holds for a datastore read error too, so it cannot be what admits the cache. What admits it is that the next read can reach the upstream again.

The discriminator is remote.ErrCredentialsRefused, not datastore.ErrEncryptionNotConfigured alone. That sentinel carries the marker, and so do the permanent decrypt verdicts, which are just as permanent; keying on the one sentinel would have left them on the answer these rows reject. The Resolutions entry records that widening against the narrower reading the review started from.

Acceptance Criteria gains two criteria: the status split under Error mapping, and one asserting that a refused read on the unknown arm issues no upstream call at all and memoizes no verdict.

The obligation this creates

The 500 row is a claim about a mapping that nothing makes yet. isCredentialConfigurationFault in internal/format/oci/remote_errors.go answers ahead of the zero-status handshake arm, and at origin/main 9b2540d92 it lists remote.ErrIncompleteCredentials and ErrCredentialUsernameColon only. That sha is after 883a317ad, which left remote_errors.go untouched.

While the predicate lists those two only, a refusal by policy takes the transient answer. Once it matches remote.ErrCredentialsRefused, the 500 row is what a client receives, and the transient row keeps its own answer either side of that change. Both rows are written that way so they read correctly whichever change closes the gap.

Follow-ups names the obligation, says no open MR carries it, and gives #417 (closed) as the owner. A note on #417 (closed) records the residual. No new issue is filed, and no new sentinel is needed: the marker is already internal/remote's own discriminator for a refusal by policy rather than a transient failure.

Merge order

None is required. The code side is merged, the spec change is docs-only, and nothing downstream waits on it.

git merge-tree against origin/main at 9b2540d92 reports no conflict. Two entries added to the Retry-After enumerations were dropped again before this landed: 628e76758 rewrote both lists from exhaustive into "Among them:" forms, so an entry per new condition is no longer owed at either site, and the retry hint for a transiently failed read is stated on its own row and in its own criterion instead.

Related to #417 (closed)