Repository navigation
fix(ci): gate the identical-tree fallback on the dedup signature - #404
Merged
Merged
Conversation
Post-merge review of #403 found a P1: if ci.yml's decision job fell back to a full main run (e.g. after a transient API failure) and any main job then failed, the unconditional identical-tree fallback in release.yml verify-ci (and the equivalent in release-readiness.yml Gate ci-matrix) still accepted the earlier green PR-head checks because the trees were identical — a release could proceed despite the tagged commit's own failed CI. The fallback now runs only when every one of the five gate check runs on the evaluated commit is absent or completed/skipped (neutral counts as skipped) — the observed dedup signature (merge commit 4d23291, run 36315089713: lint and pdf-parity completed/skipped, the three per-OS test names absent). Any real own result — a failure, a cancellation, a pending or in-progress run, or a partial success — disables the fallback and restores the pre-#403 behavior: release.yml fails with its original message, and readiness keeps its existing fail/pending evidence text byte-identical. When the dedup signature holds and the fallback is attempted but the PR head is not green or the trees differ, the #403 message naming the missing identical-tree green PR head still applies. Refs #402
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
An organization admin can review the limit in your organization's usage and billing settings.
Once the limit resets or is raised, reopen this pull request to trigger a review.
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. |
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
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.
Refs #402, fixes the P1 from the unresolved post-merge Codex review thread on #403 (
.github/workflows/release.yml).The bug
#403 added an identical-tree fallback to
release.ymlverify-ciandrelease-readiness.ymlGate ci-matrix: when the target commit's own five gate check runs were not all green, the gates accepted a merged PR head's green runs whenever the trees matched. The fallback was unconditional. So if ci.yml'sdecisionjob fell back to a full main run (e.g. after a transient API failure duringdecision) and any main job then completed with failure, the tagged commit had a failed own run — yetverify-ciwould still pass it on the strength of the earlier green PR checks. A release could proceed on a tree whose own CI was red, andGate ci-matrixcould likewise turn that failure into apass.The rule change
The fallback now runs only when the commit shows the dedup signature: every one of the five gate check runs (
lint,pdf-parity,test (ubuntu-latest),test (macos-latest),test (windows-latest)) isabsentorcompleted/skipped(completed/neutralcounts as skipped). That is the observed shape of a deduplicated main commit — verified on merge commit4d23291c(main CI run 36315089713, onlydecisionexecuted):lintandpdf-parityarecompleted/skipped, the unexpanded matrix parenttest (${{ matrix.os }})iscompleted/skipped, and the three per-OS test names are entirely absent.Any real own result disables the fallback:
lintgreen with the rest absent) — a real own result exists, so the signature no longer holds.In those cases the pre-#403 behavior applies unchanged:
release.ymlfails with its original message (Tagged commit $GITHUB_SHA has no green CI. Let the CI workflow pass on main, then re-run this workflow.), andGate ci-matrixkeeps its existing fail/pending outcome and evidence text byte-identical (no PR-head reference is added). When the dedup signature does hold and the fallback is attempted but the PR head is not green or the trees differ, the #403-era message naming the missing identical-tree green PR head still applies. The logic stays inline in both files (the readiness step is a controller step under the trust policy).Verification
yaml.safe_load) and both editedrun:blocks passbash -n.ghAPI): 31/31 pass, including all 16 pre-ci: skip main re-verification of PR-verified trees; harden release workflow #403-follow-up scenarios and the new gating cases:verify-ciandGate ci-matrix;completed/skippedmix + green identical-tree head -> fallback passes;in_progress) + green identical-tree head -> no fallback (verify-cifails with the original message; readiness recordspendingwith its own-run evidence) ;4d23291cshape (lint + pdf-parity + unexpanded matrix parent skipped,decisiongreen, per-OS names absent) + green identical-tree head -> fallback passes in both files;scripts/check.shends with== check: OK (fmt/clippy/test/crate-edges/pdf-runner/structured-corpus/claims/doc-surface/release-block/readiness-selfcheck/skip-accounting/public-parity=skipped) skipped-for-missing-fixtures=4 (optional=4) ==(
public-parity=skippedis expected on a host without the pinnedpdfinfo version 24.02.0.)No gates weakened: any doubt still fails closed, and the fallback is now strictly narrower than in #403.