[codex] harden main branch governance checks#817
Conversation
|
Warning Review limit reached
More reviews will be available in 35 minutes and 2 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Repository YAML (base), Organization UI (inherited) Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (12)
✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
github-actions
Bot
added
area:ci
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request enhances the branch policy validation script (validate-main-branch-pr.cjs) by adding detailed metadata validation for release and hotfix pull requests targeting the main branch, along with comprehensive unit tests and minor documentation and website updates. The feedback highlights opportunities to improve the robustness of the validation script, specifically by properly checking for empty pull request payloads, handling JSON parsing errors from the GitHub event payload, and ensuring the exported metadata validation function independently rejects unsupported branch types.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| if (!pullRequest) { | ||
| findings.push("Missing pull request payload."); | ||
| return findings; | ||
| } |
There was a problem hiding this comment.
In main(), pullRequest is initialized as payload.pull_request || {}. This means pullRequest will always be a truthy object (at least {}), so the check if (!pullRequest) inside validatePullRequestMetadata will never be triggered when called from main(). To correctly detect a missing or empty pull request payload, we should check if the object is empty (e.g., using Object.keys(pullRequest).length === 0).
if (!pullRequest || Object.keys(pullRequest).length === 0) {
findings.push("Missing pull request payload.");
return findings;
}
| if (normalisedBranch.startsWith("release/") && !isReleaseBranch(normalisedBranch)) { | ||
| findings.push( | ||
| `Release branches must use the form release/vX.Y.Z. Received '${normalisedBranch}'.`, | ||
| ); | ||
| return findings; | ||
| } | ||
|
|
||
| if (normalisedBranch.startsWith("hotfix/") && !isHotfixBranch(normalisedBranch)) { | ||
| findings.push( | ||
| `Hotfix branches must use the form hotfix/<slug>. Received '${normalisedBranch}'.`, | ||
| ); | ||
| return findings; | ||
| } |
There was a problem hiding this comment.
Currently, if validatePullRequestMetadata is called with a branch name that does not start with release/ or hotfix/ (for example, feature/my-feature or main), it skips both prefix checks and returns an empty findings array (indicating the metadata is valid). While main() has a prefix check before calling this function, validatePullRequestMetadata should be self-contained and robust on its own, especially since it is exported and unit-tested. We can combine the checks to ensure any invalid branch targeting main is rejected with an appropriate message.
if (!isReleaseBranch(normalisedBranch) && !isHotfixBranch(normalisedBranch)) {
if (normalisedBranch.startsWith("release/")) {
findings.push(
"Release branches must use the form release/vX.Y.Z. Received '" + normalisedBranch + "'."
);
} else if (normalisedBranch.startsWith("hotfix/")) {
findings.push(
"Hotfix branches must use the form hotfix/<slug>. Received '" + normalisedBranch + "'."
);
} else {
findings.push(
"Only release/* or hotfix/* branches may merge into main. Received '" + normalisedBranch + "'."
);
}
return findings;
}
| const payload = getEventPayload(); | ||
| const pullRequest = payload.pull_request || {}; |
There was a problem hiding this comment.
getEventPayload() catches JSON parsing errors and returns { parseError: error.message }. However, main() does not check for payload.parseError and proceeds to access payload.pull_request, which will be undefined. This results in confusing validation errors (e.g., missing required sections or incorrect title) instead of failing immediately with a clear message about the corrupt event payload.
const payload = getEventPayload();
if (payload.parseError) {
console.error("Failed to parse GitHub event payload: " + payload.parseError);
process.exit(1);
}
const pullRequest = payload.pull_request || {};
|
✅ Template check passed after update. Thanks for fixing the PR description. |
![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: 7aebbf795d
ℹ️ 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".
| steps: | ||
| - uses: actions/checkout@v4 | ||
| - name: Validate release and hotfix PR shape |
There was a problem hiding this comment.
In the main-branch-guard workflow I checked, this new inline policy still runs under the existing pull_request trigger, which GitHub runs in the PR merge-ref context rather than the base repository's default-branch context. A release/* or hotfix/* PR can therefore change this workflow file in the same PR to remove or weaken the script and still produce a passing validate-release-branch check, so the main-branch gate remains bypassable; move this no-checkout validation to pull_request_target or another protected/default-branch workflow context.
Useful? React with 👍 / 👎.
| const missing = missingSections(pr.body, releaseSections); | ||
| if (missing.length > 0) { | ||
| fail(`Release PR body is missing required section(s): ${missing.join(', ')}.`); |
There was a problem hiding this comment.
This new release-body check blocks the repository's existing automated release path: scripts/agents/release.agent.js creates release PRs with the correct chore(release): vX.Y.Z title but only a one-line body, so those PRs will always be missing Linked issues & merged PRs, Changelog, and the checklist and cannot pass the required main-branch guard. Either update both release PR creators to emit the required template sections or relax this gate for automation-generated release PRs.
Useful? React with 👍 / 👎.
🔍 Reviewer Summary for PR #817CI Status: ❌ Recommendations
|
There was a problem hiding this comment.
Pull request overview
This PR hardens governance enforcement in the repo by strengthening the main branch merge gate for release/hotfix PRs, improving template enforcement behaviour, and expanding CI validation coverage to catch config/docs drift earlier.
Changes:
- Reworked the
mainbranch guard to validaterelease/vX.Y.Zandhotfix/<slug>branch shapes plus PR title/body requirements. - Updated issue template enforcement to comment + fail without auto-closing/misclassifying issues, and wired additional validation scripts into CI.
- Cleaned up minor drift/unused code and bumped doc versions to keep freshness validation passing.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
website/src/pages/index.astro |
Removes an unused import and derived value from the homepage. |
scripts/workflows/branch-policy/validate-main-branch-pr.cjs |
Expands branch guard logic (branch shape + PR metadata checks) and exports helpers for testing. |
scripts/workflows/__tests__/validate-main-branch-pr.test.js |
Adds unit coverage for the new branch-shape and PR-metadata validation logic. |
scripts/validation/validate-retired-doc-links.cjs |
Marks docs/CANONICAL_CONFIGS_GUIDE.md as an “active” doc for retired-link validation. |
docs/ISSUE_FIELDS.md |
Bumps frontmatter/document version and refreshes last_updated. |
docs/CANONICAL_CONFIGS_GUIDE.md |
Bumps version and refines wording around legacy doc references. |
.github/workflows/template-enforcement.yml |
Stops auto-closing invalid issues; instead comments and fails the workflow. |
.github/workflows/main-branch-guard.yml |
Moves the main-branch guard to an in-workflow github-script implementation (no checkout). |
.github/workflows/checks.yml |
Adds validate:issue-fields and validate:retired-doc-links to CI validation. |
| name: validate-release-branch | ||
| runs-on: ubuntu-latest | ||
| steps: | ||
| - uses: actions/checkout@v4 | ||
| - name: Validate release and hotfix PR shape | ||
| uses: actions/github-script@v7 |
| const releasePattern = /^release\/v(?<version>\d+\.\d+\.\d+(?:-[0-9A-Za-z.-]+)?)$/; | ||
| const hotfixPattern = /^hotfix\/[a-z0-9._-]+$/i; | ||
|
|
||
| const releaseSections = [ | ||
| { name: 'Linked issues & merged PRs', regex: /^##\s+Linked issues\s*&\s*merged PRs$/im }, |
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> Signed-off-by: Ash Shaw <ashley@lightspeedwp.agency>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> Signed-off-by: Ash Shaw <ashley@lightspeedwp.agency>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
ashleyshaw
removed
the
meta:needs-changelog
ashleyshaw
added
meta:no-changelog
Linked issues
Changelog
Added
mainbranch guard so it validates actualrelease/vX.Y.Zandhotfix/<slug>branch shapes.validate:issue-fieldsandvalidate:retired-doc-linksinto CI.Changed
not_planned.Fixed
Removed
website/src/pages/index.astro.Checklist (Global DoD / PR)
last_updatedandversion)What changed
mainbranch guard so it validates actualrelease/vX.Y.Zandhotfix/<slug>branch shapes.validate:issue-fieldsandvalidate:retired-doc-linksinto CI.not_planned.Why
The previous protection only checked the branch prefix. That left the release gate weaker than the branch policy described in the repo and allowed the main-branch protection to depend on a single lightweight check. This patch closes that gap and extends CI coverage for docs/config drift.
Validation
npm run testnpm run validate:retired-doc-linksnpm run validate:issue-fieldsnpm run validate:workflowsnpx jest --config .jest.config.cjs scripts/workflows/__tests__/validate-main-branch-pr.test.js --runInBand