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.
!2249merged, so the plan's last external gate is closed and four sites still called it owed.- Merged S13 rules the budget expiry's
Retry-Afterinto the existing coalescing class at1, against this plan's fifth-value entry. - The cross-plan settlement assumes a merge order it does not control:
!2255is open and declares the shared key at40s, not90s. - 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.UpstreamExclusionReasonvalues the configuration-source gate above records under. They land there rather than here because!2276is 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.