Repository navigation
fix(config): adopt the landed stage_parallelism vocabulary in the project manifest (#938) - #1084
Conversation
…ject manifest (#938) The manifest spelled the inner build tunables `podman_jobs` / `podman_jobs_cap`. Those keys are dead: the cutover that landed the knob as `--stage-parallelism` / `CHARLY_STAGE_PARALLELISM` retired the podman-era spellings, and a retired key under `defaults:` is dropped by the decode — silently, with no `KnownFields` check — so this manifest asked for the sdk's fallback of 4 and nothing else. Spelling them the landed way makes the request explicit. The cap stays at 4, which is what the org's builds already ran at: the retired key was dropped, so every project resolved to the sdk fallback (4), and the BuildKit daemon is host-global per uid — carrying the podman-era 8 would request a max-parallelism the running daemon was not started with, and would fail the build rather than raise it. The comment is corrected with it. The resolved value is the DAEMON's [worker.oci] max-parallelism (candy/plugin-build/buildkit.go), set at daemon start and shared by every project on that uid — not a per-build stage count, which is what the podman-era wording claimed. Filed issues (each RCA'd on its own thread; none of them is fixed by this change): #919 (the daemon's value is host-global per uid and no charly verb stops it), opencharly/spec#241 (a retired key under `defaults:` validates clean and is then dropped), opencharly/plugin-build#35 (the build registry mirror never reaches an already-running daemon), opencharly/pod-nested-podman-socket#7 (a stale consumer pin in the closure), #1083 (a bare-named deploy-level member gets no substrate provider connected, so the dispatch refuses it after the side effect). Assisted-by: Claude Code minimax/minimax-m3.1-flash:cloud (fully tested and validated)
Review — PASSWhat the diff actually doesTwo files, exactly as the body states (
Consistency checks against the body
Disposition of commentsThere are 0 comments. No maintainer sign-off exists, so the change must stand on its own evidence — and the diff is self-consistent and matches every concrete claim in the body (file counts, changed lines, the nature of the Observations (non-blocking)
The change does what the body claims, is coherent and scoped, and carries an honest justification for the one judgment call it makes. Verdict: PASS ollama-cloud/deepseek-v4.1-flash — action-review. |
|
STATUS STATUS — the review round's one non-blocking observation, disposed with evidence. The observation. "The PR ships no automated regression test that would fail without it — the Agreed on the fact, and here is why the discriminator cannot be a unit test in this repo. The schema's vocabulary is independent of this manifest's spelling: The witness that DOES discriminate this rename is a live two-arm probe, and it is in the PR body's evidence. Same manifest, same declared intent (a cap of 8), two spellings, one binary, one box, one host, one running daemon:
The two arms differ only in the spelling, and only the renamed spelling produces a word from the daemon. That difference is observable only against a live BuildKit daemon whose configured Why the automated control for this class belongs to the schema, and is not built here. The general defect — a retired key under What this repo did instead, in the same cutover. The retired spelling is deleted from this manifest (the R5 sweep is in the body's evidence), and the comment above the keys is corrected with them. No test was added, because none added HERE could fail without this change, and the one that could — the validator refusing the retired key — is the schema's to make and is already filed with its evidence. Nothing is filed away on the strength of this comment: the finding's disposition is opencharly/spec#241, and the evidence on that thread is the instance's own measurement. Agent: |
Closes #938
Summary
Head
a4096c21ed51b27864c4b9f7680146572be67a12— the commit this PR pushes: 2 files changed, 8 insertions(+), 3 deletions(-).The defect. This repo's own
charly.ymlspelled the inner build tunablespodman_jobs/podman_jobs_cap. Those keys are dead: the cutover that landed the knob as--stage-parallelism/CHARLY_STAGE_PARALLELISMretired the podman-era spellings, and a retired key underdefaults:is dropped by the decode — silently, because thedefaults:block is validated against an OPEN schema struct. So the manifest's declared inner-concurrency cap of 8 was never applied: every build resolved to the sdk's fallback of 4, andcharly box validatereported 0 warnings, 0 errors the whole time. That is the exact shape opencharly/spec#241 names.The RCA.
spec.Config.Defaultsis aspec.BoxConfig, decoded with no strict-field check. The retired key is dropped on decode, so the value the operator declared never reaches the resolver (candy/plugin-build/resolve.go:246-255,resolveDriveStageParallelism) and the build silently asks forbuildkit.ResolveStageParallelism(0, 0)— the sdk's fallback (sdk/buildkit/build_helpers.go:37-74). The schema half of the mechanism is an open/closed asymmetry:spec/schema/box.cuemarks#BoxCLOSED ("an unknown key is a typo"), but thedefaults:block is validated against#BoxConfig, which is not closed — so a retired key underdefaults:is accepted by validation and dropped by the decode, and no tool in the chain can see it.spec/schema/box.cue:120-121), the MIGRATION TABLE (plugin-migrate'srename_keyops) and the FLAG. Nothing connected the contract's rename to the CONSUMER's own manifest, and this repo'scharly box validatecannot see it either: the retired key is not an unknown key to the validator, it is a dropped one, so the validator has nothing to reject.podman build"; the builder is BuildKit and the resolved value is the DAEMON's[worker.oci] max-parallelism, set at daemon start and host-global per uid).What lands. Two keys and their comment:
charly.yml:56podman_jobs: 0stage_parallelism: 0charly.yml:57podman_jobs_cap: 8stage_parallelism_cap: 4Why the cap is 4, and not the 8 the manifest declared. This is the one decision in this PR, so it is measured rather than asserted:
$XDG_CACHE_HOME/charly/buildkitd, socket/run/user/1000/charly/buildkitd.sock), not per project. The resolved value is the daemon'smax-parallelism, and a differing value is a hard error (sdk/buildkitdrive/daemon.go:96-99) — measured, with the renamed key at 8: "the running daemon was configured with max-parallelism=4 but 8 was requested". Carrying 8 would therefore have failed every build on this host, and would have forced the value onto every other project on the uid.charlyverb stops that daemon (there is no verb for it —charly cleanis an out-of-process plugin), so the remedy is not expressible with the binary. That is The BuildKit daemon has no supervisor: a changed stage_parallelism cannot be applied while it runs (a D-2 revision with a packaging leg) #919, and it is where the operator's next step already lives.The cap is therefore 4: it is the value this repo already runs at, it is the sdk's conservative fallback, and it is the value the running daemon was started with. Raising it is a host-level decision, not a manifest edit — it is filed under Filed issues.
The comment is corrected with the keys. It now says what the value IS: the resolved number is the BuildKit daemon's
[worker.oci] max-parallelism(candy/plugin-build/buildkit.go), set at daemon START and shared by every project on the uid — so it applies only when it matches the value the running daemon was started with.This is consumer leg 6b of the
stage_parallelismfan-out tracked in opencharly/opencharly#785. The producer (opencharly/plugin-box#66, tagv2026.301.1542) and the shipped-binary leg (#1082) are landed; this leg is the repo's own manifest.How tested
Everything below ran on the committed tree this PR pushes.
1. This repo's own core gate —
go test ./...andgo vet ./...fromcharly/, exactly asAGENTS.mdstates.2. The pre-flight the change class requires — the project's own manifest validates, on the tree that carries the rename.
This is the operator's stated criterion on #938: "Re-run
charly box validate— it will stay green, which is the point: it is green now with the retired keys." It is green both before and after; the rename changes the KEYS that are honoured, and the validator cannot see that either way. Which is precisely why the R7 instruments below exist.3. R9 — the binary is this worktree's own CalVer-stamped build.
2026.301.1632is HEAD's own stamp (CHANGELOG/2026.301.1632.md, the entry PR #1082's merge wrote), so the binary equals the source. No Go production code changed — the only.gofile in the diff is a comment incharly/cue_tighten_test.go— so the built binary is unchanged by this commit, and the tree is clean after the build:4. R7 (a) — the SCHEMA discriminator: the new spelling is the one the schema knows, the old one is the one it drops.
The claim under test:
stage_parallelism/stage_parallelism_capare the keys this repo's schema ACTUALLY reads, andpodman_jobs/podman_jobs_capare the keys it accepts and throws away. The discriminator is the schema's own constraint —stage_parallelism_cap?: int & >=1(spec/schema/box.cue:121) — against the open#BoxConfigstruct that validates thedefaults:block.Arm X is the discriminator: the validator names the new key and constrains it (
defaults: stage_parallelism_cap must be >= 1, got 0), sostage_parallelism_capis a key this repo's schema knows and reads. Arm Y is the same block with the retired spelling: the validator is green — 0 warnings, 0 errors — because thedefaults:block is validated against#BoxConfig, which is not closed, so an unknown key is accepted and then dropped by the decode. The two arms differ only in the spelling of the key, and only the renamed spelling produces a word from any tool in the chain.5. R7 (b) — the CAP discriminator: the renamed key is LIVE and reaches the builder; the retired one never does.
The claim under test: with the RENAMED key the declared cap REACHES the builder; with the RETIRED key it is DROPPED and the sdk's fallback (4) is used instead. Both arms declare the SAME intent (a cap of 8) and differ ONLY in the spelling of the two keys. Same binary, same box, same command, same host, same running daemon.
The two arms differ in ONE thing — the spelling of the two keys — and they read the two ways the manifest can mean "8":
ARM_B_EXIT=0).That is the whole defect in two arms: the retired spelling declares 8 and gets 4 in silence; the renamed spelling declares 8 and gets 8 (or a refusal that names 8). The key is honoured only in its new spelling, and this PR is the change that makes this repo's manifest use it.
6. R10 — the disposable bed that composes the changed entity.
check-pod-overlayis the disposable pod bed that BUILDS this project's image —check-k8s-deploy-appplus alayer-ripgrepoverlay, so the build path reads the renameddefaults:keys twice and asks the daemon for the resolved value — and then deploys the result and runs its check steps against it.The bed's own witness, written by the bed runner (
.check/check-pod-overlay/2026.301.1858/summary.yml) — the header, the diagnostics and the verdict:The three skips are the bed's own declared box-mode skips ("context [runtime] not active in box mode") — the same three assertions run and PASS in the LIVE phases (
check-live,check-live-rebuild,feature-run,feature-run-rebuild), where the runtime context IS active. The bed'supdatestep is a fresh rebuild (charly update, 4m27s) followed bycheck-live-rebuildagainst the container it produced, so the R10 "fresh rebuild and live run" requirement is met by the bed itself, not asserted.What this bed proves about THIS change.
check-pod-overlaybuilds an image through this repo's owndefaults:block — the build resolver readsstage_parallelism/stage_parallelism_capout of the project'scharly.yml(candy/plugin-build/resolve.go:246-255) — and it does so twice: once for the initialimage-buildand once for theupdaterebuild. With the retired spelling the resolver would have read nothing and fallen back to the sdk's 4; with the renamed spelling it reads the declared values. The two discriminators above are what turn that from "the file was edited" into "the renamed key is the one that is read".7. R1 — the three failures met on the way, each RCA'd, none of them this change. The R10 gate was not a straight line, and the honest record is what the rulebook asks for: every anomaly gets a fresh RCA, and every finding is either fixed in this change or filed.
(a)
check-boxload-pod— the first R10 witness — could not build at all. The bed's image step failed on a per-run image tag that does not exist upstream:Mechanism —
CHARLY_BUILD_REGISTRY_MIRRORis read by the build drive at build time, but the mirror is rendered into thebuildkitd.tomlthe daemon reads at start; the daemon on this host was already running with no[registry.…]section, so the build resolved the internal base against the upstream registry. Missed control — nothing asserts that a running daemon carries the mirror the drive is using. Blast radius — every bed whose internal base image is served by a mirror the running daemon does not carry. Root fix — filed as opencharly/plugin-build#35 (the daemon must be able to pick the mirror up without a manual restart). Not this change: the failure is in the daemon's registry configuration, and the two keys this PR renames are read by the build resolver, not by the registry path — the samedefaults:block, a different field.(b) The same bed, re-run with the mirror working, failed one step later, inside the build's own shell step:
Mechanism — the nested
podman pullinside the build hits the sandbox boundary of the pinnedlayer-container-nestingcandy. Missed control — the consumer pin in this repo's closure predates that candy's sandbox-boundary fix, and nothing in the build path flags a stale consumer pin before the step fails. Blast radius — every box composing that candy in this repo. Root fix — filed as opencharly/pod-nested-podman-socket#7 (the pin is the stale part; the candy owns the fix). Not this change: the failing step is a nestedpodman pullof a base image; the renamed keys set the daemon'smax-parallelismand neither add nor remove a step from the build.(c)
check-sidecar-pod— the second R10 witness — failed atbring-up-members, with a substrate refusal:Mechanism — a deploy-level member addressed by its bare name is invisible to the host-side deploy-name resolver (
charly/plugin_loader.go:resolveDeployNodeByPathresolves a top-level key or a DOTTED path, but not a bare member name), so the member's substrate word is never collected, its provider is never connected in that invocation, and the dispatch refuses a substrate the project does compose. The refusal's own text says thevmdeploy "compiles no primary image plan" — so the build path, the only consumer of the two keys this PR renames, is never on that code path. Missed control — two resolvers implement the deploy-name addressing law and they disagree; nothing asserts that every name the deploy path accepts resolves to a node whose substrate word is collected. Blast radius — every deploy-level member addressed by its bare name whose substrate is served by an external provider. Root fix — filed as #1083 (with the full RCA, the two invocations' plugin-set diff, and the evidence paths). Not this change: the failure is in the deploy dispatch's substrate recognition, and the manifest'sdefaults:keys are read by the build path only.Because all three findings are independent of this change and are genuinely separable from it, they are filed, not fixed here — and each is listed under Filed issues below, with its own RCA on its own thread.
8. R5 — the retired vocabulary is gone from this repo's live files. The predicate is calibrated against a known positive first, and the claim is the same pattern over the live files.
Rulebook compliance
/charly-internals:git-workflow,/charly-internals:strict-policy,/charly-internals:root-cause-analyzer,/charly-internals:cutover-policy,/charly-internals:go,/charly-build:buildand/charly-check:checkwere loaded before the first source read and before the first git action, and this repo's ownAGENTS.mdwas read before acting in it.go vetrc=0,go testrc=0 across both packages,box validate0 warnings 0 errors, and the R10 bed with 0 errors and 0 warnings.defaults:block — and there is no alias, no shim and no second spelling anywhere in the tree (proof: evidence block 8).git statuswas stated before every action; this PR is one commit onfeat/stage-parallelism-938off freshorigin/main(d10f09f); no force-push, no amend after the push, no direct push tomain, no hook bypass, and no file outside this repo touched.stage_parallelism_capand acceptspodman_jobs_capwithout a word) and the cap discriminator (evidence block 5 — the renamed key reaches the builder and the retired one is dropped before it does). The changed path executes live: the build path reads these exact keys, and the R10 bed builds through it twice.scripts/bootstrap-charly.sh→./bin/charly), never a shared install;charly versionis2026.301.1632= HEAD's stamp. No Go production code changed and no new runtime OS dependency is introduced — this is a YAML config edit — sopackaging/is untouched.charly box validate, green, 0 warnings 0 errors) plus the disposable bed that composes the changed entity (check-pod-overlay, disposable, PASS) — and, because the diff also touches a.gofile — this repo's own Go class gate (go test ./...,go vet ./...), all pasted above on the committed tree.Cutover. One cutover, one PR: this repo's own manifest keys, on the landed vocabulary, with the retired names deleted in the same commit. The contract's rename, its migration entry and the flag's rename are the producer legs, already landed (opencharly/spec, opencharly/plugin-migrate, opencharly/plugin-box#66) — this PR is the consumer's own copy of the vocabulary, not a second cutover of the same surface. The GENERAL question (should a retired key FAIL validation rather than be dropped?) is the schema's and is deliberately not folded in: the operator separated it on #938 ("that decision is the schema's, and I am not folding it in here"), and it is filed under Filed issues.
Landing. PR-only; the org's
charly/pr-validatoris the merge gate and arms native auto-merge on PASS. No agent merges this PR.Change classification
defaults:block) plus a Go source comment — the union of the two classes' gates, as the matrix requires. NOT documentation: the keys are honoured configuration, and the probes in evidence blocks 4 and 5 are what prove the renamed spelling reaches the builder.fully tested and validated— the change-class gate passed on the final tree (this repo'sgo vet/go test, thecharly box validatepre-flight, and the disposable R10 bed above), and the changed path was executed live, with the output retained above.stage_parallelismfan-out tracked in stage_parallelism consumer legs: the retired --podman-jobs flag name remains in consumer repos after the BuildKit cutover opencharly#785. It depends on the landed producer (refactor(cli)!: the build concurrency flag is --stage-parallelism, and a control holds every skill body to the grammar plugin-box#66) and on the landed shipped-binary leg (chore(pins): compile in the plugin-box that spells the knob--stage-parallelism#1082); it gates the umbrella's own leg, whose comment defers to this file.Filed issues
[worker.oci] max-parallelism, and the daemon is host-global per uid, so the manifest's cap is a REQUEST to a host-scoped service, not a per-project setting. With the cap at 8 the build refuses by name ("the running daemon was configured with max-parallelism=4 but 8 was requested"), and nocharlyverb stops that daemon, so the remedy is not expressible with the binary. This PR keeps the cap at 4 — the value the org already runs at — and the question "may this host run above 4?" is the operator's, on that thread, where it is already framed as the next step.defaults:validates clean and is silently ignored — a renamed key's old spelling disappears without a word spec#241 — "an undeclared key underdefaults:validates clean and is silently ignored — a renamed key's old spelling disappears without a word". The GENERAL form of this PR's defect: the retired key is dropped by the decode instead of being rejected, so a declared cap of 8 was inert while every check passed. The evidence from this instance is posted on that thread, and this PR adds the schema half of the mechanism:spec/schema/box.cuemarks#BoxCLOSED ("an unknown key is a typo"), but thedefaults:block is validated against#BoxConfig, which is not closed — so a retired key underdefaults:is accepted by validation and dropped by the decode, and no tool in the chain can see it. The decision (should a retired key FAIL validation, with a remediation hint pointing atcharly migrate?) is the schema's, and the operator separated it from this fix on charly's own charly.yml still ships the retired podman_jobs / podman_jobs_cap keys, and validate accepts them silently #938.check-boxload-pod) unrunnable: the drive pushed to the mirror while the already-running daemon resolved the internal base against the upstream registry, so the build died with "ghcr.io/opencharly/fedora-layer-supervisord:check-boxload-pod-2026.301.1829: not found". Independent of this change (a daemon registry configuration, not the build resolver'sdefaults:keys); the evidence and the measurement are on that thread.defaults:keys); the evidence is on that thread.check-sidecar-pod) fail atbring-up-members: the host-side deploy-name resolver does not know the bare-name addressing law for a deploy-level member, so the member's substrate word is never collected, its provider is never connected, and the dispatch refuses a substrate the project does compose — after the VM has already been created. Independent of this change (the refusal's own text says avmdeploy compiles no primary image plan, so the build path is never on that code path); the full RCA, the two invocations' plugin-set diff and the evidence paths are on that thread, filed from this PR's R10 gate.Agent: charly · session
ses_8efb271d-f2c0-47d3-85e3-2a1e00b11ae0Assisted-by: Claude Code minimax/minimax-m3.1-flash:cloud (fully tested and validated)