Conversation
|
Superseded by #98 and closed. This PR mapped to commit c3745c8. PR 96 round 1 returned a P1 requiring the catalog record index to be keyed by exact (id, digest) rather than by ID alone, which changed the CatalogV1 shape this slice indexes. Reworking PTD-07 meant rebuilding this commit on top of the amended parent, and the rebuild produced a new commit d0eb610 rather than an amend, so Sapling mapped it to a new pull request. No review had been requested here and no approval evidence existed, so nothing is lost. The slice content continues in #98 with the catalog-wide checks adapted to exact record identity. This is the cascade the plan names at Review Phasing: deferring remote review lets a finding in a low slice reach every slice above it. |
|
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: 3aede9663a
ℹ️ 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 invariants that exist only across a whole catalog, above record-local validation and above the per-manifest release graph walker. Every outgoing reference must resolve exactly: the catalog holds a record at that exact identity, its schema is one the reference permits, and it stays inside the referring record's tool and release namespace. A tool record indexes releases and is exempt from the release-namespace rule, which is the only exception the design allows. Reachability is proven from every tool record, and an unreachable record fails rather than being ignored. An orphan is not harmless: it is catalog data no request can ever select, so it can drift out of agreement with the records that are reachable without anything failing. Acquisition mappings are checked catalog-wide. Every artifact record must have exactly one source mapping, one content digest cannot be mapped from two manifests, and records sharing a content digest must agree on size across the whole catalog rather than only within the one manifest PTD-06 can see. No aggregate ceiling is introduced. The parked source bounded selected contributions at 4096; the design defines no such ceiling, so this slice declares none and bounds recursion by reference-edge depth instead.
|
Codex Review: Didn't find any major issues. Breezy! 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". |
omry
added
approved
|
PR-cycle state — maintained automatically. Do not edit. {
"approval": {
"approved_revision_fingerprint": "sha256:350f4c1c697a2c6fdcf5627fd9017ee3e098b806e8cd239e8d4e58f5cd0e2ec2",
"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:ba9ba93f4bcb29195a2c6d24e782ab2586c97f26c4aad62b6c4cd7d12fff19cd",
"id": 5337614896
},
"review_result": {
"body_digest": "sha256:60c9b8c254c225a9e864f422ccb0d1cfef9bcff7662aa17a9faf65a146bd5d9c",
"id": 5337625166
},
"revision_fingerprint": "sha256:350f4c1c697a2c6fdcf5627fd9017ee3e098b806e8cd239e8d4e58f5cd0e2ec2",
"thread_dispositions": {
"resolved": 1,
"total": 1
}
},
"evidence_fingerprint": "sha256:0ca5e832adc67d65f487fa32292b7b2ca6c586494c94fa86c3298142daa35ba7",
"rounds": 2
},
"checks": {
"conclusion": "success",
"count": 7
},
"delivery_deferral_ledger_digest": "sha256:4f53cda18c2baa0c0354bb5f9a3ecbe5ed12ab4d8e11ba873c2f11161202b945",
"delivery_deferrals": [],
"local_review": "6 passes run after the slice was first committed; found two dead helpers with no production caller, both removed",
"pull_request": {
"base": {
"ref": "pr96",
"sha": "c4b217450202"
},
"head": {
"ref": "pr97",
"sha": "7151bb91430d"
},
"number": 97,
"repository": "github.com/omry/reploy"
},
"review": {
"rounds": 2,
"summary": "one P1 on an earlier head, scope source-mapping uniqueness to each manifest, fixed; latest head clean"
},
"revision_fingerprint": "sha256:350f4c1c697a2c6fdcf5627fd9017ee3e098b806e8cd239e8d4e58f5cd0e2ec2",
"scope_authority": {
"docs/PORTABLE_TOOL_DEFINITION_DESIGN.md": "sha256:9148c9398eb162f9597bf66d61adbbf27257b6ea6edca8d1b68e088ed04916e6",
"docs/PORTABLE_TOOL_DEFINITION_IMPLEMENTATION_PLAN.md": "sha256:00fd5a399b6365c39b8c3edd2ec7886a1b2031b693fd48cc5ec7de1bd395da4c"
},
"task": "PTD-08",
"version": 1
} |
1 participant
Add the invariants that exist only across a whole catalog, above record-local
validation and above the per-manifest release graph walker.
Every outgoing reference must resolve exactly: the record exists, its digest
matches the reference, its schema is one the reference permits, and it stays
inside the referring record's tool and release namespace. A tool record indexes
releases and is exempt from the release-namespace rule, which is the only
exception the design allows.
Reachability is proven from every tool record, and an unreachable record fails
rather than being ignored. An orphan is not harmless: it is catalog data no
request can ever select, so it can drift out of agreement with the records that
are reachable without anything failing.
Acquisition mappings are checked catalog-wide. Every artifact record must have
exactly one source mapping, one content digest cannot be mapped from two
manifests, and records sharing a content digest must agree on size across the
whole catalog rather than only within one manifest, which is where PTD-06 could
see it.
No aggregate ceiling is introduced. The parked source bounded selected
contributions at 4096; the design defines no such ceiling, so this slice
declares none and bounds recursion by reference-edge depth instead.