docs(plans): S32 fix the Step 8 and Step 9 designs and what rests on them

What this does

Corrects two step designs in the merged S32 container-virtual plan, gives two unowned pieces of wiring a home, and repairs the entries that rest on them. Documentation only; no code ships here.

Both corrections are cases where the plan's reasoning was done in the standalone remote frame — one remote repository per request — and the virtual frame puts several behind one request.

Decisions a reviewer should check first

Step 8: two of the four values are per position, so one request-scoped value cannot carry them. The plan moves four values off oci.UpstreamRequestBuilder onto a per-call channel, and described that channel as a request-scoped context value. That is right for accept and imageName, which are uniform across a resolution. It is wrong for urls, rooted at one row's url, and state, a whole UpstreamAuthState — namespace, repository, and remote repository ids, URL, AuthURL, AuthStatus — as the request observed them. A resolution consults up to ADR-004's 20 rows and runPhase1 runs one goroutine per position against one derived context, so one value means every position after the first probes row 1's registry under row 1's token-cache key, and row 1's discovery write applies on every position's behalf because all three guarded fields come from that same struct.

Nothing already written fails on it: the standalone path holds one row per request, so a single value is correct there and every S16 read suite passes. The shape only breaks once a fan-out reaches it, at Step 15, nine steps later. The case that pins the split now has a test — two builders held for two different remote repositories, reached concurrently on one request context.

The per-position source is scoped to one resolution. A source held for the repository's life behind the flight registry would pin a url change for the process's life; a per-call row read would cost a statement per position and break Step 10b's 2 + K. Building it once per resolution from the row Step 4's read returned satisfies both. Each registry-held builder selects its own with the key it was constructed under — its position's namespace and remote repository — because after this step it holds none of the three ids itself, and a key is an identity rather than a row value.

One question is escalated, not answered: whether that satisfies S13's "on every call". !2013 requires a RequestBuilder to read the url inside BuildRequest on every call and not capture it at construction. The spec's own conformance note exempts container on the fact that its builder is constructed per fetch — the fact Step 8 deletes. A resolution-scoped source sits between the two things the clause names, since one resolution's Probe and Fetch read the same snapshot. The !2013 entry under ## Dependencies records the question and names the S13 DRI as its owner. No step waits on the answer: both readings are served by the same seam and only the source's refresh point moves. Routing Step 9's Probe back through BuildRequest is ruled out on the 401 rule's own grounds.

Step 4 projects the four columns those premises rest on. The Acceptance and the projected-column-set assertion named none of container_remote_repositories.id, url, auth_status, or auth_url, so an author following the entry shipped a row type the per-position source and the cache store have nothing to build from. The id has three consumers and repositories.id cannot stand in — transposing them compiles, which lands as a wrong token-cache key rather than a build failure.

Projecting url is a divergence from S31, not parity with it, and the entry now says so: S31 excludes the column by name and pins the exclusion with an assertion, S32 projects it for all 20 positions, and the projection is forced by the refusal of the per-call read rather than preferred.

Step 9: the 401 rule cannot be expressed over remote.Prober. The spec asks for opposite answers to a 401 on a cached token and on a freshly minted one. The deciding signal is where the token came from, and Prober.Probe builds through BuildRequest, which discards the Authorization by contract.

So Step 9 issues its own HEAD and maps the status itself. What that duplicates is the status table and the doer-error classification: a Do error becomes ProbeError with a nil Go error except a context.Canceled one, and a Probe that maps a cancelled error to ProbeError writes failed for every sibling the winner cancelled. "Cannot drift" is the whole mitigation, so the step owns a shared Probe conformance assertion in internal/remote/remotetest covering the table plus both arms, with 401 excluded from it by name — the two disagree on it by design — and a 401 case added to the Prober suite, which has none today.

The HEAD budget binds every implementation of the prober seam, and the step now assigns it. The application and the zero-budget guard sat on Prober alone, which this step's Probe does not route through, so a container Probe that ignored the field would ship green bounded at 30s by the client timeout rather than 5s. Both obligations are on virtual_probe.go too, and the criterion names the vehicle that makes "over the seam" mean something: internal/virtual's prober is unexported and satisfied structurally, so only the shared remotetest/probe_outcome.go assertion binds a third implementation. The zero-budget sentinel lands on internal/remote/operations.go beside the field it rejects a zero of, because errors.go is Step 11's.

A request-build failure is classified by *oci.UpstreamAuthError. The criterion enumerated the terminal and positional sets and said which was which for neither. It now names the discriminator, makes the unrecognized default terminal, and carries a case per named wiring fault — every case listed before those is green under the blanket mapping the criterion forbids.

Whose context ended is decided before that partition runs, on a failed leg. A cancelled or parent-expired handshake is an *oci.UpstreamAuthError too, so the discriminator alone books an abandoned probe as a position failure: the post-winner cancel() writes failed at every sibling still exchanging, and a client hangup reaches serveNoWinner on a dead connection. On a failing leg the Probe reads context.Cause on the parent and returns the non-nil error whenever the parent ended. One rule covers cancellation and a resolve_timeout expiry, and it binds Prober too, whose own arm keys on context.Canceled alone.

The failed leg clause is load-bearing and arrived a round later. Checked at the top of the call instead, the rule discards a probe already holding its outcome: a 200 landing just before the winner's cancel becomes an error, classifyProbeError absorbs it, and the position goes back undecided. The verdict survives; Step 17's per-position record does not, on up to probe_concurrency - 1 positions per pull. A completed-probe case sits beside each abandonment case for exactly that reason.

Step 8's install sites had no owner, and the construction site was the wrong function. oci.NewUpstreamRequestBuilder's only non-test caller sits in operationsFor, not manifestFillFor; operationsFor is both read arms' factory, so naming the wrong one dropped the blob arm. The install is the heavier half: a fetch runs on the handler's context, so remote_blob.go and remote_manifest.go's buildUpstreamFill join the Files list. Without them the standalone blob read hits Step 8's own fail-closed error on every cache miss.

Two fail-closed validations kept a home. NewUpstreamRequestBuilder re-validates imageName against the grammar and ociUpstreamAuthState refuses an auth_status outside the taxonomy. Both sat at the construction site the four values leave. They move onto the two new constructors, with a rejection test each, and Step 10b states the per-position verdict for a row either one refuses.

The Go docs carrying the two-value count are four, not three. RemoteBlobFillFor states the count inline rather than deferring to a sibling, which is how it survived a pass over the deferral chain, and it carries the seam's per-request justification as well as the number.

The worst-case probe is 20s, and merged S13 says 30s. No probe can pay a cold-token exchange and a 401 retry together: Reauthorize refuses any prior that is not a cache hit. Cold token costs 15s, cached token 20s, so container's Phase 2 worst case is 40s over two waves rather than 60s. Merged S13's ## Configuration derives the 60s, so a new Research Findings subsection records the contradiction where the plan meets the spec rather than substituting the number, and ## Dependencies books the spec sentence as owed by the S13 DRI. 90s clears both, so no default and no ruling moves.

A row that cannot build its own configuration source is a data-error position, and the gate takes two reasons. Building each source from its row moves two failing constructors into the fan-out, where the plan stated no verdict. The position is a data error, so the 404 is withheld and an all-ineligible list answers 500. Terminating instead lets one corrupt row end every pull through that virtual repository.

Two reasons rather than one, because the two failures are different pages. url carries a length cap alone, so a row failing ParseUpstreamBase is data an operator fixes. auth_status carries CHECK (auth_status IN (0, 1, 2)), so a value reaching the code is a wiring or migration fault, reachable the three ways S32 enumerates for a corrupt target_field. Five causes now sit behind four gates. The reason values are !2276's, because Step 10a owns every file they touch.

Step 8's url-change arm asked for a lifetime no step can drive. Nothing resolves until Step 15, so the arm goes back to the next fetch, which the standalone path drives, and the resolution-scoped form moves to Step 10b with a two-resolution test behind it.

Step 10b: nobody owned installing the request state. Step 8 makes absent state a named error, and no step listed the installation. Under a blanket positional mapping an unwired context would leave the resolution answering from its hosted positions alone — no data error, no 500, nothing audited. Step 10b's assembly is the only place holding both the request and every position's row, so it lands there, with an acceptance criterion and a test that a two-remote-position resolution reaches each position's own row URL and the client's own image name. The criterion also pins that the context carrying the state has to be the one Resolve runs on.

Split out, not dropped

Five entries this MR does not touch are corrected in !2276 (merged). They are drift in Step 10a, Step 11, and ## Dependencies rather than Step 8 or Step 9 work, and three of them quote text !2249 put on main after this branch's base. Keeping them here would widen this MR past the two step designs it exists to correct.

  • !2249 merged, so the plan's last external gate is closed and four sites still called it owed.
  • Merged S13 rules the budget expiry's Retry-After into the existing coalescing class at 1, against this plan's fifth-value entry.
  • The cross-plan settlement assumes a merge order it does not control: !2255 is open and declares the shared key at 40s, not 90s.
  • The manifest arm computes two write-deadline budgets, not one, so Steps 15 and 16 derive their arming from a false sentence.
  • Step 10a is half-landed by S31's Step 11 and its entry was sized for the whole, with two counts quantified over arms that changed.
  • Step 10a declares the two remote.UpstreamExclusionReason values the configuration-source gate above records under. They land there rather than here because !2276 is already rewriting that entry's scope and counts, and splitting the change across both MRs would let the rebase decide the total.

!2069 lands first; !2276 rebases behind it.

Size

862 insertions, 123 deletions, one file, documentation only.

Past the 500-line ceiling, and not split. The diff is one plan file and no code. Splitting it would put the Step 8 channel shape in one MR and the entries that rest on it — Step 4's projection, Step 9's classification, Step 10b's installation — in another, which is the split that leaves main carrying a premise no entry satisfies. The whole diff is reviewable against one document.

Sizing figures are re-forecast, not measured. Step 8 goes to ~610 source, Step 9 to ~550, and Step 10b to ~510. Re-measure each when its step is written. Two Files additions sit outside the figures they belong to, and each step's own over-ceiling bullet records that rather than carrying an unmeasured number. Step 9's remotetest/probe_outcome.go is a Create in a non-_test.go file, so its lines count as source and are not in the ~550. Step 8's two install sites, remote_blob.go and buildUpstreamFill, were in no Files list when the ~610 was taken.

Where the over-ceiling record lives changed with them. This diff removes the paragraph in ### How the ceiling is applied that summarised which non-schema steps sit over the 500 ceiling, and removes the two per-step forecasts the assembly-split paragraph carried. Each step's own entry is now the only record. Steps 8, 9, and 11 carry an **Over the ceiling by ~N source** bullet at ~110, ~50, and ~70. Step 10b at ~510 states its overrun inside its Seam interrogation: bullet instead, because its justification is that same sentence. Step 7's Seam interrogation now says what folding it into Step 10b would cost — ~600 source, about 100 over — rather than "would push that step over the ceiling", which held at ~490 and does not at ~510.

The non-schema source sum names its basis and pins it. Three of the 16 non-schema Source bullets carry a forecast and an actual, so "sum the bullets" no longer picks one number. The paragraph reads ~6,530 forecast source, measured at 0c1a60397, with Steps 5, 6, and 13 measuring 694, 472, and 165 against forecasts of 450, 400, and 420 — ~6,590 on an actual-where-known basis. Both bases clear 40 steps at 148 source.

Checks

markdownlint-cli2, vale, and lychee each pass on the plan file, run explicitly with pre-commit run <hook> --files docs/plans/2026-08-27-s32-container-virtual.md. The full pre-commit chain passes on every commit; no --no-verify.

lychee needs LYCHEE_VERSION=0.24.2 in the environment to run under git commit on a local worktree. git commit exports GIT_DIR, so the hook's own git describe --tags --exact-match --match 'lychee-*v*' resolves against this repository instead of its cached lychee clone and finds no tag. Both .pre-commit-config.yaml copies carry a valid lychee-v0.24.2 rev, and the variable short-circuits the detection rather than skipping the link check.

Duo review

The pass on record is stale and a re-run is owed. It read the MR at its first commit, about 50 of the 862 added lines, and the branch has since been rebased onto main past the plan MR that recorded the measured Source actuals.

All three findings from that pass are addressed and their threads carry the detail. Two were the markdown-review numeral rule; the text added since carried three more instances, now corrected to 16 and 40. The third was Step 10b's ceiling classification, which was genuine.

Edited by Radamanthus Batnag

Merge request reports

Loading
Loading