Conversation
Resolve each local-source recipe's build-scope tool requirements against the embedded portable catalog at build preparation, compile the PTD-21 plan and provider DAG for the observed base target, and compare that plan for exact build reuse instead of a caller-supplied lock. Acquire the selected pinned payload through the verified store, lock its acquisition provenance, and materialize the archive offline through the reviewed primitive into a host tree that exposes only the selected java and javac exports. Build each Python node's disposable source-builder image from its prefix image with a COPY-only layer after proving the destinations are absent there, run the build-scope validation schedule against that exact image through the PTD-21.5 boundary, and open the resolver session on it so the exports exist before any source build runs. Source-build commands see the exports directory on PATH; wheel resolution does not. Retained sdist identity binds the prefix image and the selected closures rather than the disposable image ID, and a snapshot whose recipe drifted from the planned one fails closed. The final application image, workload provider graph, and completion path are unchanged; the source-builder lock reaches the build lock, and builder images and the host tree are removed after the build. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
CI failed: Go tests failed on Windows because the test suite expects hardcoded Unix absolute paths (/opt/reploy/tools/...) for portable Java tools.Overview1 test failure pattern found across 1 job, caused by platform-dependent absolute path expectations in the newly added portable Java tools tests on Windows. FailuresPortable Tools Test Failures on Windows (confidence: high)
Summary
Code Review ✅ ApprovedMaterializes selected Temurin Java in the isolated builder by resolving build-scope tool requirements against the embedded portable catalog, compiling the PTD-21 plan for the observed base target, and materializing the archive offline into a host tree. Source-build commands see the exports on PATH while wheel resolution does not, and the final application image and completion path remain unchanged. No issues found. Tip Comment OptionsAuto-apply is off → Gitar will not commit updates to this branch. Comment with these commands to change the behavior for this request:
Was this helpful? React with 👍 / 👎 | Powered by Gitar — free for open source |
Owner
Author
|
PR-cycle state — maintained automatically. Do not edit by hand. Mechanical PR-cycle state (JSON){
"check_observations": [
{
"checks": [
{
"conclusion": "SUCCESS",
"name": "Linux CI checks",
"run_id": 101625261877,
"run_status": "completed",
"status": "success",
"url": "https://github.com/omry/reploy/actions/runs/34084291006/job/101625261877"
},
{
"conclusion": "SUCCESS",
"name": "Target smoke (darwin-amd64)",
"run_id": 101625262061,
"run_status": "completed",
"status": "success",
"url": "https://github.com/omry/reploy/actions/runs/34084291006/job/101625262061"
},
{
"conclusion": "SUCCESS",
"name": "Target smoke (darwin-arm64)",
"run_id": 101625261991,
"run_status": "completed",
"status": "success",
"url": "https://github.com/omry/reploy/actions/runs/34084291006/job/101625261991"
},
{
"conclusion": "SUCCESS",
"name": "Target smoke (linux-amd64)",
"run_id": 101625262095,
"run_status": "completed",
"status": "success",
"url": "https://github.com/omry/reploy/actions/runs/34084291006/job/101625262095"
},
{
"conclusion": "SUCCESS",
"name": "Target smoke (linux-arm64)",
"run_id": 101625262128,
"run_status": "completed",
"status": "success",
"url": "https://github.com/omry/reploy/actions/runs/34084291006/job/101625262128"
},
{
"conclusion": "FAILURE",
"name": "Windows host smoke (windows-amd64)",
"run_id": 101625262093,
"run_status": "completed",
"status": "failure",
"url": "https://github.com/omry/reploy/actions/runs/34084291006/job/101625262093"
},
{
"conclusion": "FAILURE",
"name": "Windows host smoke (windows-arm64)",
"run_id": 101625262104,
"run_status": "completed",
"status": "failure",
"url": "https://github.com/omry/reploy/actions/runs/34084291006/job/101625262104"
}
],
"head_sha": "dd550ca7db9ec4113b6de83a9437f45ffa588004",
"id": "sha256:9cea27c5d03862ef2376a97014cd108a695ac660f215632b3ed264c7102efc7a",
"required_checks": [
"Linux CI checks",
"Target smoke (darwin-amd64)",
"Target smoke (darwin-arm64)",
"Target smoke (linux-amd64)",
"Target smoke (linux-arm64)",
"Windows host smoke (windows-amd64)",
"Windows host smoke (windows-arm64)"
],
"status": "failure"
}
],
"delivery_deferrals": [
{
"claim": "On Windows hosts the materialized host tree carries no Unix mode bits, so the executable mode of java and javac inside the COPY-only source-builder image depends on the Docker client's context encoding; the build-scope validation schedule fails closed if the exports are not executable there, but no Windows-host build has been proved either way.",
"classification": "additive-delivery-work",
"disposition_history": [
{
"disposition": "deferred-to-PTD-25",
"evidence": "Surfaced by required-check:windows-host-smoke failures at dd550ca7db9ec4113b6de83a9437f45ffa588004: the slice's export check consulted host permission bits (fixed in round 1); the design has no Windows-host statement, the plan's Explicit Deferrals do not cover host platforms, and Windows CI runs with --docker-mode skip, so image-side executability on Windows hosts is unproven rather than broken.",
"source_head": "dd550ca7db9ec4113b6de83a9437f45ffa588004"
}
],
"id": "delivery-deferral:windows-host-source-builder-executability",
"invariant": "exact java and javac are present and executable in selected source builds on every supported host platform",
"location": {
"line": 0,
"path": "internal/dockerdeploy/source_builder_environment.go"
},
"owner": "PTD-25",
"source_head": "dd550ca7db9ec4113b6de83a9437f45ffa588004",
"status": "active",
"target_task": "PTD-25",
"trigger": "first Windows-host integration case for a source-builder portable tool (PTD-25 executes fixtures through ordinary Reploy resolution and materialization)"
}
],
"findings": [
{
"claim": "sourceBuilderExposeExportsV1 rejects an export target whose host permission bits lack 0o111; Windows filesystems carry no Unix mode bits, so five source_builder_environment tests fail on both Windows runners with 'is not an executable regular file'",
"classification": "current-slice-defect",
"disposition_history": [
{
"disposition": "accepted-current-slice-defect",
"evidence": "Read the failed job log: TestMaterializeSourceBuilderPortableToolsV1AcquiresThenMaterializesOfflineAndLocks, ...FailsClosedWhenAnExportWasNotMaterialized, TestPrepareSourceBuilderEnvironmentV1BuildsInspectsValidatesThenCleansUp, ...RemovesTheImageWhenValidationFails, and ...DoesNotBuildOverAnOccupiedDestination fail at the host-bit check. The store's ArchiveMaterializationRequest.ExecutablePaths already binds declared executables (written with mode 0o555 and fail-closed when missing) and sourceBuilderInstallRootV1 binds every export to a declared executable, so the host-bit check is redundant on Unix and wrong on Windows. Minimal fix drafted: regular-file check only, plus two tests.",
"source_head": "dd550ca7db9ec4113b6de83a9437f45ffa588004"
}
],
"id": "required-check:windows-amd64:101625262093",
"invariant": "every repository-required check completes successfully at the candidate head, and source-builder export exposure proves executability from the archive contract rather than from host mode bits",
"location": {
"line": 333,
"path": "internal/dockerdeploy/source_builder_environment.go"
},
"source": {
"name": "Windows host smoke (windows-amd64)",
"run_id": 101625262093,
"type": "required-check",
"url": "https://github.com/omry/reploy/actions/runs/34084291006/job/101625262093"
},
"source_head": "dd550ca7db9ec4113b6de83a9437f45ffa588004",
"status": "open",
"target_task": "PTD-22.2"
},
{
"claim": "sourceBuilderExposeExportsV1 rejects an export target whose host permission bits lack 0o111; Windows filesystems carry no Unix mode bits, so five source_builder_environment tests fail on both Windows runners with 'is not an executable regular file'",
"classification": "current-slice-defect",
"disposition_history": [
{
"disposition": "accepted-current-slice-defect",
"evidence": "Read the failed job log: TestMaterializeSourceBuilderPortableToolsV1AcquiresThenMaterializesOfflineAndLocks, ...FailsClosedWhenAnExportWasNotMaterialized, TestPrepareSourceBuilderEnvironmentV1BuildsInspectsValidatesThenCleansUp, ...RemovesTheImageWhenValidationFails, and ...DoesNotBuildOverAnOccupiedDestination fail at the host-bit check. The store's ArchiveMaterializationRequest.ExecutablePaths already binds declared executables (written with mode 0o555 and fail-closed when missing) and sourceBuilderInstallRootV1 binds every export to a declared executable, so the host-bit check is redundant on Unix and wrong on Windows. Minimal fix drafted: regular-file check only, plus two tests.",
"source_head": "dd550ca7db9ec4113b6de83a9437f45ffa588004"
}
],
"id": "required-check:windows-arm64:101625262104",
"invariant": "every repository-required check completes successfully at the candidate head, and source-builder export exposure proves executability from the archive contract rather than from host mode bits",
"location": {
"line": 333,
"path": "internal/dockerdeploy/source_builder_environment.go"
},
"source": {
"name": "Windows host smoke (windows-arm64)",
"run_id": 101625262104,
"type": "required-check",
"url": "https://github.com/omry/reploy/actions/runs/34084291006/job/101625262104"
},
"source_head": "dd550ca7db9ec4113b6de83a9437f45ffa588004",
"status": "open",
"target_task": "PTD-22.2"
},
{
"claim": "PlanSourceBuilderPortableToolsV1 reads every configured override's .reploy.yaml from its host directory at build preparation, before dependency resolution selects overrides, so an unselected override's recipe can fail target resolution, acquire a JDK, and enter the build lock for a package that is never built",
"classification": "design-conflict",
"disposition_history": [
{
"disposition": "accepted-design-conflict-awaiting-user-disposition",
"evidence": "Confirmed by reading the planning loop (every input.LocalOverrides entry is read through readSourceBuilderRecipeV1 from override.HostDir) against PythonLocalOverridesV1 ('without inspecting any target path') and ObserveSelectedPythonLocalSources ('Unselected override paths are never statted or walked'). Conforming requires either deferring planning, materialization, and builder preparation until dependency resolution has selected overrides and snapshots exist (a second builder-bound session per node, which PTD-22.1's removal of the resolver-session restart and PTD-22.2's 'exports exist before the source-build consumer is opened' wording do not clearly permit), or amending the documented override contract. Both change design responsibility, so the disposition is presented to the user rather than chosen here.",
"source_head": "dd550ca7db9ec4113b6de83a9437f45ffa588004"
}
],
"id": "review-comment:3946664229",
"invariant": "an unused local override mapping is never inspected, copied, or built, and a project recipe is read only from Reploy's immutable source snapshot of a selected override (docs/BLUEPRINT_ENVIRONMENT_MODEL.md:847-856; docs/adr/0001-local-source-build-recipes.md)",
"location": {
"line": 100,
"path": "internal/dockerdeploy/source_builder_portable_tools.go"
},
"source": {
"author": "chatgpt-codex-connector",
"comment_id": 3946664229,
"review_id": 5128191462,
"type": "review-thread-comment"
},
"source_head": "dd550ca7db9ec4113b6de83a9437f45ffa588004",
"status": "open",
"target_task": "PTD-22.2"
},
{
"claim": "materializeLocalOverrides calls requireSourceBuilderRecipe only when the snapshot recipe still declares requirements, so a planned recipe whose requirements were removed or whose file was deleted builds with the planned exports still on PATH",
"classification": "current-slice-defect",
"disposition_history": [
{
"disposition": "accepted-current-slice-defect",
"evidence": "Confirmed by reading materializeLocalOverrides and requireSourceBuilderRecipe. The fix depends on the disposition of review-comment:3946664229: a snapshot-only recipe read removes the planned-versus-snapshot drift entirely, while keeping preparation-time planning requires the drift check to also run for every distribution the plan recorded.",
"source_head": "dd550ca7db9ec4113b6de83a9437f45ffa588004"
}
],
"id": "review-comment:3946664240",
"invariant": "a source snapshot whose recipe drifted from the recipe its source-builder environment was planned from fails closed before any source build",
"location": {
"line": 353,
"path": "internal/dockerdeploy/prepared_python_node_operations.go"
},
"source": {
"author": "chatgpt-codex-connector",
"comment_id": 3946664240,
"review_id": 5128191462,
"type": "review-thread-comment"
},
"source_head": "dd550ca7db9ec4113b6de83a9437f45ffa588004",
"status": "open",
"target_task": "PTD-22.2"
}
],
"pr": {
"base_ref": "pr136",
"base_sha": "f78044429baaa69d47f7d4174256126e46607d89",
"body_digest": "sha256:60e02ecbbb287cbea355402c64fb7d5071d7a9a504e7a86b61fe1b7b6f1d7f4b",
"diff_digest": "sha256:bfe55c682c6b64030adc5e35f4303d994104972f042aa19d91ed1820df1db45e",
"head_ref": "pr138",
"head_sha": "dd550ca7db9ec4113b6de83a9437f45ffa588004",
"pr": 138,
"repository": "omry/reploy",
"scope_authority": [],
"title_digest": "sha256:a63944f773940235fc1b0bc8b338ca49a87414b019a81bb7b8e02764134bf3ca"
},
"record_version": 2,
"review_observations": [],
"review_requests": [
{
"base_established_at": "2026-09-07T04:45:32Z",
"base_ref": "pr136",
"base_sha": "f78044429baaa69d47f7d4174256126e46607d89",
"body_digest": "sha256:c057ab7401b06583240e3ea814c67d79cfc9aa5e93c397cfec6d3c6fb8cbc556",
"created_at": "2026-09-07T04:56:13Z",
"head_sha": "dd550ca7db9ec4113b6de83a9437f45ffa588004",
"id": 5565239945,
"kind": "regular-review"
}
],
"review_results": [
{
"base_ref": "pr136",
"base_sha": "f78044429baaa69d47f7d4174256126e46607d89",
"body_digest": "sha256:c0b5d82acebcbfe6a5ef27a7c05918a10a9618d4e22391fbc53ae385df9982e9",
"commit_id": "dd550ca7db9ec4113b6de83a9437f45ffa588004",
"id": 5128191462,
"inline_message_ids": [
3946664229,
3946664240
],
"request_comment_id": 5565239945,
"result_kind": "findings",
"status": "result",
"submitted_at": "2026-09-07T04:59:46Z",
"unresolved_thread_ids": [
"PRRT_kwDOTFQCkM6fyh1Q",
"PRRT_kwDOTFQCkM6fyh1Y"
]
}
],
"revision_fingerprint": "sha256:70bb918bb523adc81ee84b8a8b45e67ebb4fe555c26d2e73504e6331f0bac578",
"rounds": [],
"schema": "awd:swe:pr-cycle-state",
"version": 2
} |
omry
marked this pull request as ready for review
September 7, 2026 04:55
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. |
Owner
Author
![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: dd550ca7db
ℹ️ 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".
Comment on lines
+100
to
+110
| for index, override := range input.LocalOverrides { | ||
| if override.Distribution == "" || pythonprovider.NormalizeDistributionName(override.Distribution) != override.Distribution { | ||
| return nil, fmt.Errorf("local Python override %d distribution is not normalized: %q", index, override.Distribution) | ||
| } | ||
| if index > 0 && input.LocalOverrides[index-1].Distribution >= override.Distribution { | ||
| return nil, fmt.Errorf("local Python overrides must be unique and sorted") | ||
| } | ||
| if override.HostDir == "" || !filepath.IsAbs(override.HostDir) || filepath.Clean(override.HostDir) != override.HostDir { | ||
| return nil, fmt.Errorf("local Python override %q path must be absolute and clean", override.Distribution) | ||
| } | ||
| recipe, err := readSourceBuilderRecipeV1(override.HostDir, override.Distribution) |
There was a problem hiding this comment.
When package overrides contain a local project that is not part of the resolved Python closure, this loop still reads its host directory and includes its recipe requirements in the build-wide portable-tool plan. This bypasses the existing selection boundary in selectUnresolvedPythonLocalOverrides and the documented contract that unused mappings are never inspected (docs/BLUEPRINT_ENVIRONMENT_MODEL.md:847-856); consequently, an unrelated override requesting Java can make an otherwise valid deployment fail target resolution, acquire a large JDK, and alter the build lock even though that package is never built. Planning needs to be deferred or filtered to overrides actually selected by dependency resolution.
Useful? React with 👍 / 👎.
Comment on lines
353
to
+354
| if len(recipe.Requirements) != 0 { | ||
| return nil, nil, fmt.Errorf( | ||
| "local source recipe for %q requires a portable source-builder environment before Python resolution", | ||
| snapshot.Distribution, | ||
| ) | ||
| if err := session.requireSourceBuilderRecipe(snapshot.Distribution, recipe); err != nil { |
There was a problem hiding this comment.
If a recipe declared a tool during planning but the local source is edited before snapshotting so that requires becomes empty or the recipe is removed, this condition skips requireSourceBuilderRecipe entirely. The source then builds with the previously planned Java exports still on PATH instead of failing the documented drift check, so the build can consume a recipe different from the one used to resolve its builder environment. Check for a previously planned recipe as well as nonempty current requirements, and compare the identities in either case.
Useful? React with 👍 / 👎.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Resolve each local-source recipe's build-scope tool requirements against the embedded portable catalog at build preparation, compile the PTD-21 plan and provider DAG for the observed base target, and compare that plan for exact build reuse instead of a caller-supplied lock. Acquire the selected pinned payload through the verified store, lock its acquisition provenance, and materialize the archive offline through the reviewed primitive into a host tree that exposes only the selected java and javac exports.
Build each Python node's disposable source-builder image from its prefix image with a COPY-only layer after proving the destinations are absent there, run the build-scope validation schedule against that exact image through the PTD-21.5 boundary, and open the resolver session on it so the exports exist before any source build runs. Source-build commands see the exports directory on PATH; wheel resolution does not. Retained sdist identity binds the prefix image and the selected closures rather than the disposable image ID, and a snapshot whose recipe drifted from the planned one fails closed.
The final application image, workload provider graph, and completion path are unchanged; the source-builder lock reaches the build lock, and builder images and the host tree are removed after the build.
Co-Authored-By: Claude Fable 5.1 noreply@anthropic.com