Repository navigation
Select remote Build Machines automatically - #846
Conversation
Plain `--remote` now selects a Build Machine instead of refusing. `--remote=<Machine>` still pins one and `--local` forces the local default; Compose sets the preference between them with a top-level `x-build-machine:` that any of the three flags overrides. `auto` and `local` name modes wherever they appear, so a Machine called either is pinned by its ID. Selection is a pure function of observed evidence in `build_location`: visible Machines that advertise `ployz.build.v1`, ranked by how many requested `build.platforms` their reported architecture runs natively via the existing `platform_compatible`, with Machine ID breaking ties so the choice does not depend on observation order. The selected Machine is reported before any upload, along with the platforms this client could not confirm and any Machine that never answered; the Machine still admits or refuses the Build itself. Refusals separate the three reasons a Cluster offers no Build Machine — nothing visible, nothing answered, nothing capable — so a partition is not reported as a capability problem, and each carries the corrective action that actually applies. Capture runs before selection so the platforms it fixes decide the candidates; no source leaves the client before admission. Closes #804
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 72cafcdc4b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| _ => NoBuildMachine::Incapable { | ||
| observed: excluded.into_iter().chain(unanswered).collect(), | ||
| }, |
There was a problem hiding this comment.
When no eligible Machine is found and one visible Machine refuses builds while another probe times out, this branch returns Incapable, producing “no visible Machine can run this Build” and upgrade advice. The unanswered Machine may still be capable, so this mixed case turns unknown evidence into a capability conclusion; classify it as reachability/unknown or add a distinct mixed outcome with retry guidance. This conflicts with DESIGN.md’s evidence-over-claims invariant.
AGENTS.md reference: AGENTS.md:L3-L3
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Agreed, and fixed in 8737565.
The discriminant is now "did every visible Machine answer for itself?" rather than "which buckets are non-empty":
enum NoBuildMachine {
Invisible, // nothing observed at all
Inconclusive { refused, unresolved }, // silence → no verdict, wait and retry
Incapable { refused }, // all answered, none capable
}So the mixed case you describe — one Machine refuses, another times out — is Inconclusive, reads "no Machine that answered can run this Build, and 1 did not answer", and advises waiting rather than upgrading. Still three variants: the old Silent is just the refused.is_empty() case.
Your comment also surfaced a second instance I had wrong. Evidence::Ineligible (membership doesn't invite an RPC) was in the refused bucket on the grounds that it's a known ineligibility. But CONTEXT.md:50 defines a Membership Observation as "one Machine's potentially incomplete or stale judgment that another Machine is... down. It is not the observed Machine's lifecycle or an authoritative liveness fact" — so a stale down is hearsay, not the Machine speaking, and can't settle capability either. It now counts as silence, keeping its own evidence wording so the user still sees why each Machine is unresolved.
Rung-1 coverage asserts both: a lone silent Machine and a refused+silent pair are each Inconclusive, and only an all-answered set reaches Incapable.
| /// Preferred Build execution location from the Compose `x-build-machine` | ||
| /// extension: a Machine Target, `auto`, or `local`. | ||
| pub build_machine: Option<String>, |
There was a problem hiding this comment.
Deserializing x-build-machine as an arbitrary String means invalid values such as x-build-machine: "*" survive load_project; only the build and deploy handlers later call Location::requested, while ployz changes accepts and ignores the same invalid configuration that ployz deploy rejects. This also violates the CODING_STANDARDS.md requirement that string modes and Machine Targets be represented by types that cannot hold illegal states. Deserialize into a typed local/automatic/pinned preference so every Compose consumer gets identical validation.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Resolved by removing the field entirely in 8737565, rather than by typing it.
You were right that Option<String> couldn't hold the invariant and that ployz changes silently accepted what deploy rejected. Grilling the fix surfaced a bigger problem with the feature itself: CONTEXT.md:30 scopes Machine ID uniqueness to "within one Cluster", and CONTEXT.md:33 defines Machine Name as "a human-facing selector... that may be ambiguous. It is not an identity." A Compose file is portable and usually committed, so x-build-machine: tower names a different Machine in each checkout and none in CI.
Typing it would have made an unportable value well-formed. Build location is a per-invocation fact, so it now lives only on --remote=<Machine>; the extension, the ComposeProject field, and the Go validation are all gone, which removes the fourth-consumer problem by construction. Rationale recorded on #804.
Recipe (build.x-recipe) and platforms (build.platforms) stay in Compose — both are portable properties of the project rather than of the Cluster.
…fusals Machine selectors are Cluster-local and may be ambiguous (CONTEXT.md:30, 33), so a committed Compose file cannot name one honestly: the same `x-build-machine: tower` means a different Machine in every checkout, and none at all in CI. Remove the extension rather than validate it. Build location is per invocation: `--remote=<Machine>`, plain `--remote`, `--local`, or the local default. `auto` and `local` stop being reserved words, since no Compose key spells modes any more. Refusals no longer turn silence into a capability conclusion. A Machine that never answered may still have been able to build, so the "none can build" verdict is only available when every visible Machine answered for itself; otherwise the refusal is Inconclusive and advises waiting and retrying. Membership ineligibility counts as silence, not as a refusal: a Membership Observation is one Machine's potentially stale judgment of another and is not the observed Machine speaking (CONTEXT.md:50), so it cannot settle capability either. Both follow DESIGN.md bet 8, which permits a diagnosis to remain Unknown and flags inferring certainty the observer lacks.
nick-potts
changed the title
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8737565f55
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| let location = Location::requested( | ||
| leaf.get_one::<String>("remote").map(String::as_str), | ||
| leaf.get_flag("local"), | ||
| ) |
There was a problem hiding this comment.
When no CLI location flag is supplied, this call considers only --remote and --local, so a Compose x-build-machine preference is ignored and the build silently runs locally; deploy has the same omission. This contradicts the commit’s explicit flag > Compose > local contract, and a repo-wide search at this commit finds no remaining x-build-machine handling. Fresh evidence relative to the earlier validation comment is that the field has now been removed entirely rather than given typed validation.
Useful? React with 👍 / 👎.
Closes #804.
What changed
Plain
--remotenow selects a Build Machine instead of refusing.--remote=<Machine>still pins one,--localstates the local default explicitly, and the default is unchanged. Bothbuildanddeployshare the interface.Selection lives in a new
build_locationmodule as a pure function of observed evidence: visible Machines that advertiseployz.build.v1, ranked by how many requestedbuild.platformstheir reported architecture runs natively (reusing the existingplatform_compatible), with Machine ID breaking ties so the choice does not depend on observation order. The selected Machine is reported before any upload, along with the platforms this client could not confirm and any Machine that did not answer for itself. The Machine still admits or refuses the Build itself.Capture runs before selection so the platforms it fixes decide the candidates. No source leaves the client before admission.
Descoped: the Compose build location
AC2 asked for the preferred execution location in Compose, and an earlier revision of this PR added a top-level
x-build-machine:. That has been removed deliberately — see the discussion on #804.A Machine ID is unique only "within one Cluster" (CONTEXT.md:30) and a Machine Name is "a human-facing selector that may be ambiguous... not an identity" (CONTEXT.md:33). A committed Compose file is portable, so
x-build-machine: towernames a different Machine in every checkout and nothing at all in CI — the same class of mistake as a hardcoded hostname. Which Machine builds is a per-invocation fact, and--remote=<Machine>is where it belongs.This drops AC1's "Compose overrides the local default" tier and AC2's location clause. Recipe (
build.x-recipe) and platforms (build.platforms) were already in Compose and are untouched.Refusals do not turn silence into a verdict
A Machine that never answered may still have been able to build, so "no visible Machine can run this Build" is only available when every visible Machine answered for itself:
Membership ineligibility counts as silence, not as a refusal: a Membership Observation is "one Machine's potentially incomplete or stale judgment that another Machine is... down. It is not the observed Machine's lifecycle or an authoritative liveness fact" (CONTEXT.md:50), so a stale
downcannot settle capability either.Both follow DESIGN.md bet 8 — a diagnosis "may remain Unknown", and "inferring certainty the observer lacks" is a listed red flag.
Testing rungs
build_location.rs: flag precedence and pin rejection, permuted observation order, ID tie-break, multi-platform ranking, unresolved evidence on a successful pick, and the three refusal kinds including the mixed refused+silent case.cli_shape.rs(flag/positional parsing, conflicts,--localon both commands) andremote_build_cli.rs(automatic selection, explicit pin, ambiguity refusal, capability refusal text,--localoverride).build_layer3/remote.rs: a two-candidate build-and-deploy exercise covering automatic selection, an explicit pin, and the local override.build_layer3is already registered inscripts/run-layer3-tests.sh.cargo fmt,cargo clippy --workspace --all-targets, the full workspace suite, and the Go helper tests all pass, rebased onto currentmain(which includes #844).One deliberate interpretation
Platform compatibility ranks candidates; it does not filter them. AC4 reads as though an incompatible request should be refused, but emulation support is not observable from the client and the README documents emulated cross-platform builds — filtering would refuse builds that work today. The best candidate is chosen, unverified platforms are printed, and the Machine rechecks admission.
Separately, only
ployz.build.v1is verified rather than image-store capabilities:local.rsadvertises the wholeCapabilityAdvertisement::Containerclass as one block, andBUILD_CAPABILITYsits in it alongsideLIST_IMAGES_CAPABILITYandENSURE_IMAGE_INGEST_CAPABILITY, so a separate check would be dead code rather than verification.