Conversation
omry
marked this pull request as ready for review
August 18, 2026 19:57
Owner
Author
|
You have reached your Codex usage limits for security reviews. Please try again later. |
![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: a0084bb047
ℹ️ 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".
Owner
Author
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ede20166d4
ℹ️ 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".
Add the graph layer above per-target composition: resolve a whole release manifest and validate what only becomes visible once every referenced record is present, plus the tool release index and the external validation evidence schema. Five functions move from the parked extraction source b39985d247e5 without semantic change. resolvedRecordV1 already landed with PTD-05, so nothing else is new. Two truth fixes, each with negative coverage. The parked walker wires only four of the five composition validators PTD-05 delivered. It calls the per-artifact binding contract check inside its artifact loop and never reaches validateTargetBindingsAgainstContractsV1, so the set-level rule that a binding's artifacts must cover every interpreter its contract advertises would never run outside tests. That rule is itself a carried finding from retired PR 83, so extraction alone would have retired a finding without enforcing it. The walker now calls the set-level function once per target, and the per-artifact call it duplicated was removed. Artifacts sharing a mapped content digest could disagree on size. The source mapping compares sizes only for the record it names, so a second reachable record was free to declare the same SHA-256 at a different size. A digest fixes the bytes, so every record claiming it must agree on how many there are. This is the carried finding from retired PR 83 that this task owns. The entry points have no production caller yet. PTD-07 owns injected-filesystem loading and must invoke the walker once every referenced record is loaded; PTD-06 non-goals exclude filesystem catalog discovery. Recorded as an open discovery against PTD-07 rather than resolved here.
Owner
Author
|
Codex Review: Didn't find any major issues. You're on a roll. Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
Owner
Author
|
PR-cycle state — maintained automatically. Do not edit. {
"approval": {
"approved_revision_fingerprint": "sha256:ffd317497cb4de3314016aa235b6ec6fb33952b0dee2e6e594badeae95af8953",
"evidence": {
"delivery_deferral_ledger_digest": "sha256:4f53cda18c2baa0c0354bb5f9a3ecbe5ed12ab4d8e11ba873c2f11161202b945",
"required_checks": [
[
"Linux CI checks",
"success"
],
[
"Target smoke (darwin-amd64)",
"success"
],
[
"Target smoke (darwin-arm64)",
"success"
],
[
"Target smoke (linux-amd64)",
"success"
],
[
"Target smoke (linux-arm64)",
"success"
],
[
"Windows host smoke (windows-amd64)",
"success"
],
[
"Windows host smoke (windows-arm64)",
"success"
]
],
"review_request": {
"body_digest": "sha256:04761ac450a844632fdfdecfc638ad713851c4a99cc5c8715f636a3fcedfc35d",
"id": 5333447614
},
"review_result": {
"body_digest": "sha256:386f799d5ee6e4ab2dab084152eea6993c599b507196b288ae6cf3fbff606cc3",
"id": 5333540010
},
"revision_fingerprint": "sha256:ffd317497cb4de3314016aa235b6ec6fb33952b0dee2e6e594badeae95af8953",
"thread_dispositions": {
"resolved": 2,
"total": 2
}
},
"evidence_fingerprint": "sha256:2390b9342db7b60d78ad85e85492370849d9ae916aa52e2aec8009a1abb26673",
"rounds": 3
},
"checks": {
"conclusion": "success",
"count": 7
},
"delivery": "executed through swe:deliver-design-stack; checkpoint-commit created the commit and submit-native-stack created and linked PR 95 into native stack #87",
"delivery_deferral_ledger_digest": "sha256:4f53cda18c2baa0c0354bb5f9a3ecbe5ed12ab4d8e11ba873c2f11161202b945",
"delivery_deferrals": [],
"findings": [
{
"disposition": "fixed in ede20166d42c; addArtifact checked binding name and platform only, so an artifact could name its contract ID at a different digest and leave a dangling exact reference",
"id": "resolve-each-binding-artifacts-contract-digest",
"round": 1,
"severity": "P2"
},
{
"disposition": "fixed in 22cca582369a; a recurrence of the fixture-selection gap fixed in PTD-04. That sweep covered landed code only, so the same defect rode in from the parked extraction source.",
"id": "validate-evidence-selection-identifiers",
"round": 2,
"severity": "P2"
}
],
"local_review": "6/6 passes, returned clean; both remote findings were in code it had read and passed",
"open_discovery": "the graph entry points still have no production caller; PTD-07 owns injected-filesystem loading and must invoke the walker once every referenced record is loaded",
"preflight_audit": "coverage measured before requesting review, 85.3 percent of statements with no unreached function. The set-level wiring gap was found because the delivery workflow routed PR 94's P1 into this slice's acceptance criteria, not from memory.",
"pull_request": {
"base": {
"ref": "pr94",
"sha": "89c8d6048e6a"
},
"head": {
"ref": "pr95",
"sha": "22cca582369a"
},
"number": 95,
"repository": "github.com/omry/reploy"
},
"review": {
"request": "@codex, regular review 22cca582369a2adc1e2d9c42960cfc38d88390fc",
"result": "Codex Review - no issues on 22cca58236"
},
"revision_fingerprint": "sha256:ffd317497cb4de3314016aa235b6ec6fb33952b0dee2e6e594badeae95af8953",
"rounds": [
{
"attempt": 1,
"findings": 1,
"resulting_head": "ede20166d42c",
"reviewed_head": "a0084bb047f7"
},
{
"attempt": 2,
"findings": 1,
"resulting_head": "22cca582369a",
"reviewed_head": "ede20166d42c"
},
{
"attempt": 3,
"findings": 0,
"note": "clean",
"resulting_head": "22cca582369a",
"reviewed_head": "22cca582369a"
}
],
"scope_authority": {
"docs/PORTABLE_TOOL_DEFINITION_DESIGN.md": "sha256:9148c9398eb162f9597bf66d61adbbf27257b6ea6edca8d1b68e088ed04916e6",
"docs/PORTABLE_TOOL_DEFINITION_IMPLEMENTATION_PLAN.md": "sha256:ae49b856a644489e7a49d87601b12469b45cb6060e80abf49eb9c866dcd48f5a"
},
"task": "PTD-06: Validate Release Graphs and External Evidence",
"truth_fixes": [
"the parked walker wired four of five PTD-05 composition validators, never reaching validateTargetBindingsAgainstContractsV1, so the set-level interpreter coverage rule carried from retired PR 83 would not have run in production",
"artifacts sharing a mapped content digest could disagree on size; the source mapping size-checked only the record it names"
],
"version": 2
} |
|
Codex Review: Didn't find any major issues. 🚀 Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
This was referenced Aug 18, 2026
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.
Add the graph layer above per-target composition: resolve a whole release
manifest and validate what only becomes visible once every referenced record is
present, plus the tool release index and the external validation evidence
schema.
Five functions move from the parked extraction source b39985d247e5 without
semantic change. resolvedRecordV1 already landed with PTD-05, so nothing else
is new.
Two truth fixes, each with negative coverage.
The parked walker wires only four of the five composition validators PTD-05
delivered. It calls the per-artifact binding contract check inside its artifact
loop and never reaches validateTargetBindingsAgainstContractsV1, so the
set-level rule that a binding's artifacts must cover every interpreter its
contract advertises would never run outside tests. That rule is itself a
carried finding from retired PR 83, so extraction alone would have retired a
finding without enforcing it. The walker now calls the set-level function once
per target, and the per-artifact call it duplicated was removed.
Artifacts sharing a mapped content digest could disagree on size. The source
mapping compares sizes only for the record it names, so a second reachable
record was free to declare the same SHA-256 at a different size. A digest fixes
the bytes, so every record claiming it must agree on how many there are. This
is the carried finding from retired PR 83 that this task owns.
The entry points have no production caller yet. PTD-07 owns injected-filesystem
loading and must invoke the walker once every referenced record is loaded;
PTD-06 non-goals exclude filesystem catalog discovery. Recorded as an open
discovery against PTD-07 rather than resolved here.