Repository navigation
Conversation
The Required Merge Gate reads the newest check run of each required name on the commit. When CI runs again on the same commit (a re-run, a reopened pull request, a label), the gate starts within seconds, before the new workflows have created their own gate checks. It then found the previous run's result and, if that run had been cancelled or failed, failed at once, although the new run was still on its way. PR #229's gate failed this way three seconds after a reopen. Treat a check that concluded other than success before this gate run started as pending: it belongs to an earlier run, and a newer one may not have reported yet. The gate keeps polling and fails, with the reason named, only if no newer result arrives before its deadline. A failure after the gate run started still fails at once, and an earlier success still counts, as a label starts no new Core Checks. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
@codex 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. |
![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: 4e1f42f3f8
ℹ️ 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".
| # re-run, a reopen, a label); its newer run may not have created its | ||
| # check yet, so it is waited on rather than trusted. The test suite | ||
| # sets it. | ||
| GATE_RUN_STARTED_AT="${GATE_RUN_STARTED_AT:-$(date -u +%Y-%m-%dT%H:%M:%SZ)}" |
There was a problem hiding this comment.
When a check from the current label/reopen run fails before this step begins, date records the later step-start time rather than when the gate workflow started. The comparison at lines 923-925 therefore misclassifies that current failure as belonging to an earlier run and keeps polling for up to 19,800 seconds before reporting a timeout instead of failing immediately. Pass an actual workflow-run start timestamp into this step, or otherwise correlate check runs with the current trigger.
Useful? React with 👍 / 👎.
The gate step starts only after the jobs it needs, so a check from the current run can fail before the step's own start time and be taken for an earlier run's result, making the gate wait out its deadline instead of failing at once. Read this run attempt's run_started_at from the API instead, and only when checks are polled, so profiles that poll nothing still make no API calls. If it can't be read, fall back to the step's start with a warning. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a4a5befe15
ℹ️ 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".
| if [[ "${status}" == "completed" ]]; then | ||
| if [[ "${conclusion}" != "success" ]]; then | ||
| # ISO 8601 UTC timestamps of the same format order as strings. | ||
| if [[ -n "${completed_at}" && "${completed_at}" < "${GATE_RUN_STARTED_AT}" ]]; then |
There was a problem hiding this comment.
On a synchronize, reopened, or labeled event while the previous Full Validation run is active, .github/workflows/ci.yml uses the same PR-ref concurrency group with cancel-in-progress: true; GitHub documents that this cancels the currently running workflow when the replacement is queued (GitHub concurrency documentation). The old run's Full Validation Gate can consequently receive its completed_at cancellation timestamp after this Required Merge Gate's run_started_at, causing this branch to treat that old cancellation as current and fail immediately even though the newly triggered Full Validation later passes. Fresh evidence beyond the earlier comment is this cancellation ordering imposed by the Full Validation workflow, which a completion-time cutoff cannot distinguish; correlate the check with its originating workflow/event instead.
Useful? React with 👍 / 👎.
A newer push, reopen or label cancels the running Full Validation (cancel-in-progress), and that cancellation can complete after this gate run started, so the run-start cutoff alone took it for a current failure. A cancelled check means superseded or never run, so treat it as pending and wait for the run that replaces it; the gate still fails at its deadline if none arrives. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2dff5bcea7
ℹ️ 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".
| # sets it. | ||
| resolve_gate_run_started_at() { | ||
| if [[ -z "${GATE_RUN_STARTED_AT:-}" ]]; then | ||
| GATE_RUN_STARTED_AT="$(gh api "/repos/${REPOSITORY}/actions/runs/${RUN_ID}/attempts/${RUN_ATTEMPT}" 2>/dev/null | jq -r '.run_started_at // empty' 2>/dev/null || true)" |
There was a problem hiding this comment.
In the checked required-merge-gate.yml, this request uses GITHUB_TOKEN, but the workflow grants only contents: read and checks: read. GitHub states that omitted scopes are set to none in its workflow permissions reference, while Get a workflow run attempt requires Actions: read; therefore this lookup receives no usable run_started_at and always falls back to the later step-start time. A current required check that finishes unsuccessfully before this step begins is consequently classified as an earlier run and polled for up to 19,800 seconds instead of failing immediately. Fresh evidence beyond the earlier start-time comment is that the new workflow-run lookup is unreachable with the workflow's actual token scopes; add actions: read (and a contract assertion for it).
Useful? React with 👍 / 👎.
Getting a workflow run attempt needs Actions: read, and the workflow grants only contents and checks, so the start-time lookup always fell back to the step's own start. Grant actions: read, and pin it in the poller contract test. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 164533a5de
ℹ️ 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".
| if [[ -n "${completed_at}" && "${completed_at}" < "${GATE_RUN_STARTED_AT}" ]]; then | ||
| echo "${check_name}: concluded ${conclusion:-unknown} at ${completed_at}, before this gate run started at ${GATE_RUN_STARTED_AT}; waiting for a newer run" | ||
| pending+=("${check_name} (last ${conclusion:-unknown} before this gate run)") | ||
| continue |
There was a problem hiding this comment.
On labeled, unlabeled, and ready_for_review events, an earlier failed Core Checks Gate is classified as pending here, but .github/workflows/core-checks.yml does not list those activity types and therefore does not start a replacement Core Checks run. For source and release-policy PRs, the required gate consequently waits the full 19,800-second deadline for a check that cannot arrive. Either trigger Core Checks for the same events or only defer a stale failure when the current event can actually create a newer run.
Useful? React with 👍 / 👎.
The gate now waits for a newer Core Checks Gate instead of trusting an earlier failure, but Core Checks ran only on opened, synchronize and reopened, so a label, unlabel or ready-for-review event left the gate waiting out its deadline for a run that could not start. Give Core Checks the same pull_request activity types as the gate and Full Validation, and pin that in the poller contract test. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7d6f83909e
ℹ️ 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".
| if [[ -n "${completed_at}" && "${completed_at}" < "${GATE_RUN_STARTED_AT}" ]]; then | ||
| echo "${check_name}: concluded ${conclusion:-unknown} at ${completed_at}, before this gate run started at ${GATE_RUN_STARTED_AT}; waiting for a newer run" | ||
| pending+=("${check_name} (last ${conclusion:-unknown} before this gate run)") | ||
| continue |
There was a problem hiding this comment.
A check finishing before this workflow's run_started_at is not necessarily stale: Core Checks, Full Validation, and Required Merge Gate are independent workflows triggered by the same PR event, so runner availability can let a newly triggered validation fail before Required Merge Gate starts. This branch then treats that current failure as an earlier result and polls for up to 19,800 seconds even though no replacement is coming; rerunning Required Merge Gate alone has the same problem because it advances this timestamp without starting either validation workflow. Fresh evidence beyond the earlier step-start finding is that the new cutoff is still local to a separately scheduled workflow rather than correlated with the observed check's triggering event or run.
Useful? React with 👍 / 👎.
The gate waited on a check that failed before its own run started,
taking it for an earlier run's result. A time cutoff cannot tell the
two apart: Core Checks, Full Validation and the gate are separate
workflows, so this event's check can fail before the gate's run starts,
an earlier event's can fail or be cancelled after it, and re-running
the gate alone moves its start but starts nothing else.
Date each failed check by its event instead. Every workflow run an
event starts is created at that event, and a re-run keeps the run, its
check suite and its created_at. The gate maps a failed check's check
suite to its workflow run (GET /actions/runs?check_suite_id=) and
compares that run's created_at with its own run's (GET
/actions/runs/{run_id}), allowing 60 seconds between one event's runs.
- An earlier event's failure is waited on until a newer run reports.
- A failure from this event or a later one fails at once, including
after a gate-only re-run.
- A success counts whichever event produced it, and a cancellation is
never a verdict; neither needs a lookup.
- If either run can't be read after the usual retries, the gate fails
closed under its own error title.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8fc28a8fa8
ℹ️ 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".
| CHECK_RUN_CREATED_AT="${check_created#*$'\t'}" | ||
| check_created="${check_created%%$'\t'*}" | ||
| gate_created="${gate_created%%$'\t'*}" | ||
| if (( check_created < gate_created - EVENT_SKEW_SECONDS )); then |
There was a problem hiding this comment.
When two pull_request activities for the same SHA occur within 60 seconds—for example, a label followed quickly by an unlabel or reopen—a failed validation run from the first event satisfies this comparison and is classified as current. The second gate then fails immediately on the stale result before its replacement check appears, recreating the behavior this change is intended to prevent. Fresh evidence beyond the prior cutoff comments is this new fixed 60-second equivalence window; use an explicit per-event correlation marker rather than treating every run in that interval as belonging to one event.
Useful? React with 👍 / 👎.
| if [[ "${CHECK_EVENT}" == "earlier" ]]; then | ||
| echo "${check_name}: concluded ${conclusion:-unknown} in a workflow run created at ${CHECK_RUN_CREATED_AT}, by an earlier event than this gate's (${GATE_RUN_CREATED#*$'\t'}); waiting for a newer run" | ||
| pending+=("${check_name} (last ${conclusion:-unknown} from an earlier event)") | ||
| continue |
There was a problem hiding this comment.
When an earlier event's workflow is manually rerun after the current event's gate check has started or completed, check_run_filter selects that old rerun because it sorts solely by the check's started_at. This branch then keeps the selected failure pending because its workflow has an earlier created_at, and every subsequent poll selects the same rerun, so the gate can time out after 19,800 seconds even while a successful check from the current event is already present in the API response. Inspect and correlate all matching check runs, selecting a result from the current-or-later event before waiting on an earlier one.
Useful? React with 👍 / 👎.
This branch has not been deployed
1 participant
Summary
When CI runs again on the same commit, the Required Merge Gate can fail on the previous run's result, before the new run has reported. #229 hit this: after a reopen, its gate failed in 3 seconds with
Core Checks Gate concluded cancelled, quoting the day before's runner-starved run, while the new Core Checks went on to pass.created_at), and a re-run keeps the workflow run, its check suite and itscreated_at. For a required check that concluded other than success or cancelled, the gate maps its check suite to its workflow run (GET /actions/runs?check_suite_id=) and compares that run'screated_atwith its own run's (GET /actions/runs/{run_id}), allowing 60 seconds between one event's runs.… (last failure from an earlier event)).Required Merge Gate could not correlate check runs.actions: readfor these lookups, and Core Checks now runs on the samepull_requestactivity types as the gate (labels, unlabels, ready for review), so a waited-on Core Checks always has a newer run coming.Bitcoin Core has no such gate, since maintainers judge CI by hand, so there is nothing to backport.
Testing
Ran focused unit or functional tests for the changed area:
ci/checks/test_merge_gate_poller.py, which runs the gate's real script against a stubbedghthat also answers the workflow-run lookups. 32 of 32 pass, covering:19 mutations, one per rule, are each caught.
Checked the API semantics against GitHub's REST description and live on bench: bound the Cadence benchmark chain without -asymptote #229: re-runs kept their run's
check_suite_idandcreated_at(the attempt endpoint does not, so the gate never uses it), and one event's workflows sharedcreated_atto the second.Ran lint or formatting checks relevant to this change: actionlint (with shellcheck),
test_classifier_ci_contract.py,test_scheduled_validation_contract.pyandtest_classify_merge_profile.py.Target Branch
feature/pq-transport(Hybrid post-quantum v2 transport (#184) — integration #233), on its way to1.x.x.Risk / Review Notes
Notes: this is the repository's only required check, so it is worth a careful look. The change only turns some immediate failures into waits, and never turns a failure into a pass: an earlier event's failure with no newer run still fails at the deadline, and a failure whose event can't be read fails at once under its own title. Lookups are made only for failed checks and cached per check suite, so a green run makes none.
Docs / Process Impact
🤖 Generated with Claude Code