Separate integration checks from full release verification - #808
Conversation
github-actions
Bot
added
the
coderabbit:first-pass
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. |
|
Warning Review limit reachedNext included review available in 58 minutes. View limit detailsLimit details: You’ve used the included review currently available. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Review configuration: ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (15)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![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: e3cb357cd9
ℹ️ 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".
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
|
Review disposition at Review decisions and evidenceConfirmed dirty-state mutation and hidden-untracked-file defects are fixed, and reporting order is clarified. Eight replay tests/353 assertions, complete lint, the explicitly partial pre-commit validate run, and independent corrective review pass. Unchanged installer/stack evidence comes from the original clean committed full validation. SHA-256 selection was declined as an optional expansion of the narrow example; unknown formats keep full coverage. No follow-up issue was created. CodeRabbit explicitly skipped review and is not counted as a completed clean review. Automated summary comments are status artifacts; substantive inline findings are accounted for. Future full-PR scans should start after this comment unless |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: aec8735cab
ℹ️ 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".
Review summaryThe Ruby example wrappers ( Main concern is scope/proportionality, and one behavioral change that reaches beyond this PR's stated goal — see inline comments. Scope: |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 96523f1baf
ℹ️ 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".
|
Reviewed the diff (15 files, +703/-21). One inline comment posted on a concrete content bug in Scope vs. stated Why The PR's own body says: "The design could have shipped separately from its executable adoption examples." I'd take that at face value. Concretely:
Given the PR already acknowledges the split is viable, I'd suggest actually splitting it: land the contract/doc/skill changes (verify, run-ci, autoreview, close-session, verification-evidence.md) now, and defer the toy example repos + their dedicated 252-line test suite to a follow-up once a real adopting repo needs the worked example. That also shrinks the review surface for the part that actually changes agent behavior. Duplication The "selected coverage does not prove complete promotion coverage" point is restated near-verbatim in No functional bugs found in the example wrapper scripts themselves (git argv usage avoids shell injection; the state-diffing logic for detecting mid-validation mutation looks correct on inspection). I was not able to execute |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 20305c82fb
ℹ️ 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".
justin808
added
complexity:neutral
Review summaryI read through the full diff (docs, skill instructions, the two example One scope/proportion observation, per this review's focus: Diff is dominated by illustrative code that explicitly disclaims production use. Of the 846 added lines, The PR description anticipates this ("The design could have shipped separately from its executable adoption examples; combining them makes the documented contract verifiable in the same PR"), so this may be a deliberate tradeoff already made. But given the size and independence of the two halves, splitting the examples + replay test into a follow-up PR would let the policy contract land and be reviewed at its own (much smaller) size, while the illustrative wrappers get reviewed on their own terms without inflating the core change's diff. Also left an inline note on the duplicated ~40-line candidate-capture helper block that's identical between the No correctness, security, or performance issues found beyond what's already disclosed in the PR body (per-file |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a2c52cae0e
ℹ️ 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".
Review summaryI read the full diff ( Two scope/simplicity findings posted inline:
No security or performance issues stood out in the example wrappers beyond their own acknowledged scope (they're explicitly documented as teaching examples, not a production-hardened path, and the per-file |
…usted-base-provenance * origin/main: Separate integration checks from full release verification (#808)
…ical-token-budgets * origin/main: Separate integration checks from full release verification (#808) Make address-review summaries human-ready (#795) Speed up PR validation for ordinary documentation (#806) Batch installer copies to reduce validation overhead (#807) Report oversized PR diffs as blocked preflight coverage (#748)
…iet-draft-reviews * origin/main: Accept trusted-base provenance for merged changes (#523) Limit optional review nits to the initial fix pass (#811) Make merge approval receipts human-first (#820) Simplify human-facing writing guidance (#821) Make ordinary PR closeout lightweight (#818) Separate integration checks from full release verification (#808) Make address-review summaries human-ready (#795)
…data-trust-boundary * origin/main: Accept trusted-base provenance for merged changes (#523) Limit optional review nits to the initial fix pass (#811) Make merge approval receipts human-first (#820) Simplify human-facing writing guidance (#821) Make ordinary PR closeout lightweight (#818) Separate integration checks from full release verification (#808) Make address-review summaries human-ready (#795) # Conflicts: # skills/pr-batch/bin/pr-security-preflight # skills/pr-batch/bin/pr-security-preflight-test.rb
1 participant
Why
Repositories can already customize their validation commands, but verification, review, and closeout need a consistent way to distinguish selected integration checks from complete promotion evidence. This makes those choices explicit while preserving required failures, trusted policy, and separate release authority.
Closes #514. Closes #787.
What changed
This change includes the delivery contract and executable replay fixtures. The design could have shipped separately from its executable adoption examples; combining them makes the documented contract verifiable in the same PR. New YAML settings, a shared resolver, executable fast/balanced/strict presets, and additional effort counters remain deferred.
How to review and verify
Start with
docs/delivery-policy.md, then compare the two example wrappers and their replay tests. Candidate policy edits must not reduce their own checks; selected success must not qualify omitted promotion checks. Existing repositories retain their current behavior unless they explicitly customize their wrappers.Test plan
deferred_to_update_changelog.Agent details
Candidate and verification evidence
The current Codex review completed with no new findings. Claude published a substantive review and two optional consolidation suggestions; both were dispositioned without further source changes. Its launcher reported seven permission denials, so launcher success alone is not treated as review evidence. All 46 review threads are resolved.
Clean committed local full validation passed. Current-head full hosted validation passed, including installer and stack suites, on tested integration
479aa94e2061408a7697b6d4a5142ffa1e1974a7against base48ed4f81b725c63808d78e99e12d05c920e6223a. An earlier candidate failed an unchanged batch-status timing assertion in run34426073347. The cause remains unknown and is recorded in #260; this policy correction does not claim to fix that failure.Candidate
fc2c5d61c72a964394dfd63478c6d19d9a768f2cintegrates main48ed4f81b725c63808d78e99e12d05c920e6223a. Selection/reporting use the captured candidate head. Both wrappers verify captured HEAD and working validator source identity against the trusted wrapper, and reject staged validator changes before reporting coverage. The added replay covers staged and committed validator changes hidden by restoring trusted working bytes, across both examples and phases. Independent review checked the complete HEAD/index/worktree boundary; dirty promotion already failed before this correction, so no release bypass is claimed.The prior integration proof preserves the actual base. This candidate adds only the independently reviewed three-file policy-identity correction and replay; all other feature bytes and current-main
CHANGELOG.mdare preserved. The preceding full hosted run passed ona2c52cae0ed2659ac47a0c0c1bd335485c61f209, tested integration7b76b63fcd06b187cbe86b9528ba3ccd5553ad97, and the same base. It is historical evidence, not qualification of this corrected candidate.The approved
3eabd95d9cf7fc9d95ff663fb12288f746fc828ccandidate passed clean local full validation and full hosted validation. Its independent local integration review was clean. The completed hosted Codex review and resolved prior findings remain recorded; the Claude launcher reported nine permission denials and no published review artifact, so its success was not counted as a clean review. These are historical results; the current candidate's gates are listed above.The regressions reproduce hidden tracked edits, executable-mode changes, candidate mutations, dirty gitlinks, unsupported nested entries, and operational prose that must retain full coverage. The teaching wrappers compare complete candidate bytes, with a per-file process cost. They are examples; no production adoption or measured performance improvement is claimed.
QA Evidence
fc2c5d61c72a964394dfd63478c6d19d9a768f2c; clean committed local full validation passed.Decision log
Use existing repository wrappers and verification/review stopping rules. Shared configuration, a resolver, additional loop counters, broader presets, and automatic consumer adoption remain deferred. This PR does not claim the remaining outcomes of #392.