Skip to content

fix(release): include freshly created PRs - #182

Merged
tkkhq merged 7 commits into
mainfrom
fix/release-fresh-pr-discovery
Sep 1, 2026
Merged

fix(release): include freshly created PRs#182
tkkhq merged 7 commits into
mainfrom
fix/release-fresh-pr-discovery

Conversation

@tkkhq

@tkkhq tkkhq commented Sep 1, 2026

Copy link
Copy Markdown
Contributor

Tracking

Why

  • Release Please created PR release: 0.23.2 #181, but the immediate pending-label query returned no PRs while GitHub's search index caught up. The merge loop then exited without processing the new PR.
  • PR check rollups can also show the prior head for a short time after Release Please updates an existing PR.

Summary

  • combine Release Please's direct PR output with the pending-label query and deduplicate PR numbers
  • reject malformed direct PR output
  • bind required-check registration and results to the captured PR head through the commit check-runs and status APIs
  • keep the existing author, branch, title, version, check, and head-pinned merge guards
  • add regression checks for fresh PR discovery, malformed output, and exact-head check results

Verification

  • make release-workflow-check
  • actionlint .github/workflows/release-please.yml .github/workflows/check.yml
  • shellcheck scripts/ci/check-release-workflow.sh
  • make check
  • GitHub Actions required checks passed at ea161dd74b56e236fd901e542fd3f06acb201c7e

Copilot AI balanced review requested due to automatic review settings September 1, 2026 13:51
@tkkhq
tkkhq requested a review from a team as a code owner September 1, 2026 13:51

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Combines newly created and existing Release Please PRs to avoid stale-label discovery races.

Changes:

  • Merges and deduplicates PR numbers from both discovery sources.
  • Adds a release-workflow regression guard.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
.github/workflows/release-please.yml Combines fresh and labeled release PRs.
scripts/ci/check-release-workflow.sh Adds workflow assertions and discovery regression coverage.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread scripts/ci/check-release-workflow.sh Outdated
@marckong

marckong commented Sep 1, 2026

Copy link
Copy Markdown
Contributor

Review

The prior Copilot fixture feedback is resolved, but one blocking correctness issue remains.

  • [P1] Keep pass/fail validation pinned to the captured head (priority: 1) — .github/workflows/release-please.yml:117-120

    When an updated PR’s rollup still points to its previous head—the race this patch targets—the exact-head loop stops once check names register on the new head, but these gh pr checks calls can then report the old head as passing. Because --admin bypasses protection and --match-head-commit only verifies that the head is unchanged, the new head could merge while its checks are pending or failing. Validate conclusions through the commit APIs for head_oid, or explicitly verify the rollup SHA before merging.

make check, the workflow regression check, and ShellCheck pass locally; CI is green.

Overall correctness: incorrect

@tkkhq

tkkhq commented Sep 1, 2026

Copy link
Copy Markdown
Contributor Author

Fixed in a49956e and a7026b8. Check registration and pass/fail results now come from the captured head commit APIs; missing or pending checks wait, failed checks stop the run, and the merge remains pinned to that head.

@marckong

marckong commented Sep 1, 2026

Copy link
Copy Markdown
Contributor

Review

One blocking runtime issue remains:

  • [P1] Grant commit-status permission before calling the status API.github/workflows/release-please.yml:103

    commit_statuses is fetched using CHECKS_TOKEN, which is ${{ github.token }}, but the workflow only grants checks: read, contents: read, and pull-requests: read (lines 16–19). With an explicit permissions block, omitted permissions are set to none, and reading commit statuses requires the separate statuses: read permission.

    Once a release PR enters this loop, the check-runs request can succeed but the following status request will fail authorization. Because the script uses set -e, the workflow aborts before evaluating or merging the PR. This affects license/cla, which is currently the repository’s only commit status; the other eight checks are check runs.

    Please add statuses: read to the workflow permissions and assert its presence in the regression script.

The earlier exact-head correctness issue is resolved, and the fresh-PR discovery logic otherwise looks sound. git diff --check, make release-workflow-check, ShellCheck, and make check pass locally; the PR checks are green.

Overall correctness: incorrect

@tkkhq

tkkhq commented Sep 1, 2026

Copy link
Copy Markdown
Contributor Author

Fixed in ea161dd. The workflow now grants statuses: read for exact-head commit status queries, and the regression guard requires that permission.

@marckong

marckong commented Sep 1, 2026

Copy link
Copy Markdown
Contributor

Re-review

Re-reviewed the complete PR at ea161dd74b56e236fd901e542fd3f06acb201c7e. Both previously reported blockers—the stale PR-rollup race and missing statuses: read permission—are resolved.

I also reviewed the release flow holistically: fresh and labeled PR discovery, malformed-output handling, trust and version guards, exact-head check/status aggregation, pending and failure behavior, deduplication, retry behavior, and the head-pinned admin merge. I found no remaining actionable issues.

Validation:

  • git diff --check
  • make release-workflow-check
  • shellcheck scripts/ci/check-release-workflow.sh
  • make check
  • malformed Release Please output fails closed in the actual pipeline
  • live exact-head aggregation against pending release PR release: 0.23.2 #181 returns all expected checks
  • all PR checks are green

Overall correctness: correct

@tkkhq
tkkhq merged commit 2b246de into main Sep 1, 2026
9 checks passed
@tkkhq
tkkhq deleted the fix/release-fresh-pr-discovery branch September 1, 2026 18:04
@kong-volcano-app kong-volcano-app Bot mentioned this pull request Sep 1, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants