fix(release): harden release agent, guard PR body, and add missing test coverage#1018
Conversation
|
Warning Review limit reached
More reviews will be available in 58 minutes and 31 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more credits in the billing tab to continue. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based credits. 🚦 How do rate limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan refill rate. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, the refill rate gradually slows as usage increases. The highest same-day bursts are limited more strictly. 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 selected for processing (7)
✨ 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 |
|
✅ Template check passed after update. Thanks for fixing the PR description. |
🎨 Mermaid Diagram Validation✅ All Mermaid diagram checks passed.
|
🔍 Reviewer Summary for PR #1018CI Status: ✅ Recommendations
|
Metadata governance
|
There was a problem hiding this comment.
Code Review
This pull request updates the release management workflow by enhancing the release issue template, introducing comprehensive Jest test suites for changelog utilities, main branch PR validation, and the release agent, and updating the release agent script to use lint-staged instead of Husky. It also adds a structured release PR body generator to satisfy branch guard policies. Feedback on these changes highlights a potential shell injection and syntax error risk in createReleasePR due to shell command interpolation of backticks, recommending the use of a temporary body file with the gh CLI instead. Additionally, a minor documentation update is suggested to keep a code comment in sync with the transition to lint-staged.
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.
| const title = `chore(release): v${version}`; | ||
| const body = | ||
| "Automated release PR generated by release.agent.js. Includes version bump, changelog update, and tag creation."; | ||
| const body = buildReleasePRBody(version); |
There was a problem hiding this comment.
Shell Injection & Syntax Error Risk
The body string returned by buildReleasePRBody(version) contains literal backticks (e.g., \\CHANGELOG.md\``) and newlines. When this is interpolated directly into the shell command inside double quotes:
exec(
`gh pr create --base main --head ${branch} --title "${title}" --body "${body}"`,
dryRun,
);the shell (used by execSync under the hood) will interpret the backticks inside the double quotes as command substitution. This will cause the shell to attempt to execute CHANGELOG.md (and other backticked terms) as commands, leading to errors like CHANGELOG.md: command not found. Additionally, literal newlines and any double quotes within the body will break the shell command syntax.
Recommended Solution
Instead of passing the multi-line markdown body via the --body argument, write the PR body to a temporary file and use the --body-file (or -F) option of the gh CLI, similar to how release notes are handled in createRelease.
Here is how the entire createReleasePR function should be refactored:
function createReleasePR(version, branch, options = {}) {
const { dryRun = false } = options;
const title = `chore(release): v${version}`;
const body = buildReleasePRBody(version);
if (dryRun) {
console.log(
`[DRY-RUN] Would create PR from ${branch} to main with title "${title}"`,
);
return;
}
const bodyFile = path.join(os.tmpdir(), `release-pr-body-${version}.md`);
fs.writeFileSync(bodyFile, body, "utf8");
try {
exec(
`gh pr create --base main --head ${branch} --title "${title}" --body-file "${bodyFile}"`,
dryRun,
);
console.log("✓ Release PR created");
} finally {
if (fs.existsSync(bodyFile)) {
fs.unlinkSync(bodyFile);
}
}
}| @@ -1158,7 +1183,7 @@ async function run() { | |||
| // Step 5: Stage all changes and run Husky pre-commit hooks, then commit | |||
There was a problem hiding this comment.
The comment still mentions running Husky pre-commit hooks, but the code has been updated to run lint-staged directly. Let's update the comment to keep it accurate and in sync with the implementation.
| // Step 5: Stage all changes and run Husky pre-commit hooks, then commit | |
| // Step 5: Stage all changes and run lint-staged, then commit |
…st coverage Bugs fixed in release.agent.js: - Regex escape (getMergedPRs): \d+ in regex literal matched literal backslash; PR numbers were silently empty on every run. - createReleasePR body: both shell and MCP providers generated a plain prose body. main-branch-guard requires three specific sections, so every automated release PR was blocked. Added buildReleasePRBody() helper used by both providers. - createReleasePR shell injection: backticks in the PR body caused command substitution when interpolated into the shell command. Now writes body to a temp file and uses --body-file instead. - Husky v9 compatibility: npx husky run pre-commit is v8 syntax. Correct call for v9 is npx lint-staged directly. - Hardcoded /tmp/ path: release-notes temp file now uses os.tmpdir(). - Stale comment: inline comment still referred to Husky after fix. Test coverage added / rewritten: - release.agent.test.js: rewrites to use subprocess ESM pattern - changelogUtils.test.js: new file, full coverage of parsing/validation - validate-changelog.test.js: replaced stub with real CLI and integration tests - validate-main-branch-pr.test.js: new file, full coverage of guard logic Issue template clarification (18-release.md): - Adds explicit develop → release/vX.Y.Z → main flow comment - Checklist now specifies that release branch is created FROM develop and the PR targets main Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LNVr8xNwtAwfWVpaxW75Aq
996d90e to
3ac2074
Compare
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LNVr8xNwtAwfWVpaxW75Aq
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
@Mergifyio queue |
Merge Queue Status
Waiting for
All conditions
|
Linked issues
Closes #968 (addresses release checklist confusion and governance gaps)
Summary
Deep audit of the release agent, workflows, and related documentation found four bugs and significant gaps in test coverage. This PR fixes all of them.
Bugs fixed
release.agent.jsgetMergedPRs):\\d+in a regex literal matched a literal backslash, so PR numbers were never extracted fromgit logoutput — contributor lists and PR-linked release notes were silently empty on every run.createReleasePRbody: both the shell and MCP providers generated a plain prose body.validate-main-branch-pr.cjs(the main-branch-guard) requires three specific sections —## Linked issues & merged PRs,## Changelog, and### Checklist (Global DoD / PR)— so every automated release PR was blocked by the guard. AddedbuildReleasePRBody()helper used by both providers.createReleasePRshell injection: backticks in the PR body were interpreted as command substitution when interpolated into the shell command. Now writes the body to a temp file and uses--body-fileinstead.npx husky run pre-commitis Husky v8 syntax. v9 is installed; the correct call isnpx lint-stageddirectly./tmp/path: release-notes temp file now usesos.tmpdir()for portability.Test coverage added / rewritten
release.agent.test.jsdetermineNextVersion,compareVersions,isValidGitRef,buildReleasePRBody,detectBreakingChanges,generateHighlights,updateChangelog,validatePostReleaseChangelog,getReleaseProviderchangelogUtils.test.jsparseChangelog,validateChangelog,getLatestRelease,getUnreleasedChanges,hasUnreleasedChangesvalidate-changelog.test.jsvalidate-main-branch-pr.test.jsnormaliseBranchName,extractReleaseVersion,isReleaseBranch,isHotfixBranch,isAllowedBranch,validatePullRequestMetadata(release and hotfix); verifies PR body shape satisfies guardIssue template clarification (
18-release.md)The release issue checklist now explicitly states:
This directly resolves the confusion in issue #968 where
release/v0.6.0 PR opened against mainlooked wrong — the branch is created from develop, but the PR correctly targets main (onlyrelease/*andhotfix/*branches may merge to main, enforced bymain-branch-guard).Changelog
Fixed
release.agent.js: regex\\d+→\d+ingetMergedPRs— PR numbers now correctly extractedrelease.agent.js: automated release PR body now includes all three sections required bymain-branch-guardrelease.agent.js: automated release PR body now uses--body-fileto avoid shell injection from backticksrelease.agent.js: Husky v9 command corrected fromnpx husky run pre-committonpx lint-stagedrelease.agent.js: temp file path usesos.tmpdir()instead of hardcoded/tmp/18-release.md: removeddescriptionfield (issue templates must useabout)Added
changelogUtils.test.js: full test suite for core changelog parsing and validation utilityvalidate-main-branch-pr.test.js: full test suite for the main-branch-guard validation logicrelease.agent.test.js: rewritten with real module imports via ESM subprocess patternvalidate-changelog.test.js: replaced stub with real CLI and integration testsChecklist (Global DoD / PR)
createReleasePRfixed (uses--body-file)lint:jsexits 0 (no new errors)develop → release/vX.Y.Z → mainflow explicit