fix: hotfix release flow ships only hotfix commits and uses patch bump#12022
Merged
0xApotheosis merged 12 commits intodevelopfrom Mar 3, 2026
Merged
fix: hotfix release flow ships only hotfix commits and uses patch bump#120220xApotheosis merged 12 commits intodevelopfrom
0xApotheosis merged 12 commits intodevelopfrom
Conversation
…sion bump
The hotfix option in `yarn release` was shipping all commits since the last tag
instead of just the hotfix commits. This happened because `getCommits()` used
`${latestTag}..origin/${branch}` for hotfix branches, which includes develop
history.
- Use `origin/main..origin/${branch}` range for hotfix branches so only commits
not on main are included
- Add programmatic verification that the hotfix branch is actually based on
origin/main (not just the user's confirmation)
- Use patch version bump for hotfixes instead of minor
- Detect hotfix vs regular release at merge time via PR title prefix
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThe release script replaces the branch-clean prompt with an unreleased-commits-driven hotfix flow: discover unreleased commits between Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant User as User
participant Script as Release Script
participant Origin as Git Remote
participant Repo as Local Repo
User->>Script: start hotfix flow
Script->>Origin: fetch origin/main and origin/develop
Origin-->>Script: return commit lists
Script->>User: present unreleased commits (inquireSelectCommits)
User-->>Script: selected commits
Script->>Repo: checkout main and pull origin/main
Script->>Repo: cherry-pick selected commits (reverse order)
alt cherry-pick success
Repo->>Repo: create tag on main
Repo->>Origin: push main and tags
Repo->>Repo: reset/push private branch to main
Repo->>Repo: checkout develop and merge main
Repo->>Origin: push develop
Script->>User: complete
else cherry-pick failure
Repo->>Repo: reset main to pre-cherry-pick commit
Script->>User: show error and abort
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
1 task
…equiring manual branch Hotfix flow now fetches unreleased commits (origin/main..origin/develop), presents them as checkboxes, cherry-picks selected commits onto main, tags with patch version, and syncs private + develop - eliminating the need to manually create a branch off main. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…tput correctly simple-git's .log() collapses custom --pretty=format output into a single entry. Switching to pify(exec) gives us raw stdout we can split by newline. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@scripts/release.ts`:
- Around line 330-346: The getCommits function is misusing simple-git by passing
a raw --pretty flag which breaks simple-git's parsing and makes all[].hash
unreliable; change getCommits to call git().log with the object-style options
(e.g., include '--oneline' and '--first-parent' as keys and use a format option
with subject: '%s') and then map messages from all.map(({ subject }) => subject)
instead of hash, or for consistency switch getCommits to the same raw exec
approach used by getUnreleasedCommits; ensure you still build the same range
(latestTag..origin/branch) and return { messages, total } from getCommits.
- Around line 482-485: The loop that cherry-picks commits applies them
newest-first and doesn't handle failures; change the logic that iterates over
selected so it processes commits oldest-first (e.g., iterate over selected in
reverse order or use selected.slice().reverse()), and wrap the await
pify(exec)(`git cherry-pick ${c.hash}`) call in try/catch; on catch run a
cleanup command like pify(exec)(`git cherry-pick --abort`), log the error
(including c.hash and error.message) and exit the process with a non-zero code
to avoid leaving the repo in a conflicted state. Ensure you keep the existing
console.log message (updated to match the new order) so each attempted
cherry-pick is recorded.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
scripts/release.ts
Use simple-git's object-style format option instead of raw --pretty flag which broke internal parsing. Reverse cherry-pick order to oldest-first and add error handling with abort on failure. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
scripts/release.ts (1)
558-558:⚠️ Potential issue | 🟡 MinorMissing
awaitondoHotfixRelease()call.The ternary awaits
doRegularRelease()but notdoHotfixRelease(). While Node.js won't exit with pending I/O, this inconsistency causescreateReleaseto return before the hotfix completes, and the implicit return type becomes a union ofvoidandPromise<never>.Proposed fix
const createRelease = async () => { - ;(await inquireReleaseType()) === 'Regular' ? await doRegularRelease() : doHotfixRelease() + ;(await inquireReleaseType()) === 'Regular' ? await doRegularRelease() : await doHotfixRelease() }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/release.ts` at line 558, The ternary expression that calls inquireReleaseType() awaits the Regular branch but not the hotfix branch, causing createRelease's return to be inconsistent; update the expression so both branches are awaited by adding await before doHotfixRelease() (i.e., change the ternary to await doRegularRelease() : await doHotfixRelease()), ensuring the call sites/functions inquireReleaseType, doRegularRelease, doHotfixRelease (and the createRelease flow) all return a consistent awaited Promise.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@scripts/release.ts`:
- Line 558: The ternary expression that calls inquireReleaseType() awaits the
Regular branch but not the hotfix branch, causing createRelease's return to be
inconsistent; update the expression so both branches are awaited by adding await
before doHotfixRelease() (i.e., change the ternary to await doRegularRelease() :
await doHotfixRelease()), ensuring the call sites/functions inquireReleaseType,
doRegularRelease, doHotfixRelease (and the createRelease flow) all return a
consistent awaited Promise.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
scripts/release.ts
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
scripts/release.ts (1)
558-558:⚠️ Potential issue | 🔴 CriticalMissing
awaitbeforedoHotfixRelease().The hotfix branch of the ternary doesn't await the async function. If
doHotfixReleasethrows before callingexit(), it results in an unhandled promise rejection that won't be caught by any error handling.🐛 Proposed fix
const createRelease = async () => { - ;(await inquireReleaseType()) === 'Regular' ? await doRegularRelease() : doHotfixRelease() + ;(await inquireReleaseType()) === 'Regular' ? await doRegularRelease() : await doHotfixRelease() }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/release.ts` at line 558, The ternary call using inquireReleaseType() currently awaits only doRegularRelease() but not doHotfixRelease(), causing unhandled promise rejections; update the expression so both branches are awaited—ensure the call using inquireReleaseType(), doRegularRelease(), and doHotfixRelease() awaits the hotfix branch as well (i.e., add await before doHotfixRelease()) so errors from doHotfixRelease() are properly propagated and can be handled.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@scripts/release.ts`:
- Around line 519-522: Wrap the merge and push around a try/catch and add
rollback: before calling git().merge(['main']) capture the current commit SHA
(e.g. const preMergeSha = await git().revparse(['HEAD'])), then try the merge
and push; on any error call git().merge(['--abort']) to stop a conflicted merge,
run git().reset(['--hard', preMergeSha]) to restore develop to the pre-merge
state, log the error via console.error, and exit/throw so the script does not
continue; ensure the push(['origin','develop']) only runs after a successful
merge. This mirrors the cherry-pick rollback pattern and uses the existing git()
helper and the merge(['main']) / push(['origin','develop']) calls as anchors.
---
Outside diff comments:
In `@scripts/release.ts`:
- Line 558: The ternary call using inquireReleaseType() currently awaits only
doRegularRelease() but not doHotfixRelease(), causing unhandled promise
rejections; update the expression so both branches are awaited—ensure the call
using inquireReleaseType(), doRegularRelease(), and doHotfixRelease() awaits the
hotfix branch as well (i.e., add await before doHotfixRelease()) so errors from
doHotfixRelease() are properly propagated and can be handled.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
scripts/release.ts
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
scripts/release.ts (1)
516-519:⚠️ Potential issue | 🟠 MajorHandle
developmerge failures with rollback to avoid conflicted local state.
git().merge(['main'])and the following push are not protected. A conflict here can leave the repo stuck mid-merge ondevelop. This concern was already raised earlier and still appears unresolved in this revision.Proposed fix
console.log(chalk.green('Pulling develop...')) await git().pull() + const developSha = (await git().revparse(['HEAD'])).trim() console.log(chalk.green('Merging main back into develop...')) - await git().merge(['main']) + try { + await git().merge(['main']) + } catch (err) { + await git().merge(['--abort']).catch(() => undefined) + await git().reset(['--hard', developSha]) + const message = err instanceof Error ? err.message : String(err) + exit(chalk.red(`Merge into develop failed: ${message}`)) + } console.log(chalk.green('Pushing develop...')) await git().push(['origin', 'develop'])🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/release.ts` around lines 516 - 519, Wrap the merge/push sequence in a try/catch and guard the push so we only push when merge succeeds: call git().merge(['main']) inside try, and on error run an explicit rollback sequence (invoke git().raw(['merge', '--abort']) or git().merge(['--abort']) if supported, then git().checkout(['develop']) and git().reset(['--hard', 'origin/develop']) to restore the local branch), rethrow or log the error and exit without calling git().push(['origin', 'develop']); ensure git().merge and git().push are the referenced symbols updated so the merge is protected and the repo cannot remain in a conflicted mid-merge state.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@scripts/release.ts`:
- Around line 351-354: getUnreleasedCommits() currently hardcodes the git range
to "origin/main..origin/develop" which leaks unrelated develop commits in hotfix
mode and lacks an ancestry check; modify getUnreleasedCommits to determine the
correct git range dynamically by reading the current branch (e.g., via git
rev-parse --abbrev-ref HEAD), choose the range based on whether we're in hotfix
mode (use origin/main..HEAD or origin/main..<hotfix-branch>) or normal develop
mode, and before using origin/main as the base run a git merge-base check (git
merge-base --is-ancestor origin/main HEAD) to assert the current branch is
descended from origin/main and fail with a clear error if not; apply the same
dynamic-range + ancestry-guard fix to the other similar block referenced around
lines 447-453 so both occurrences use the computed branch name, git log range,
and ancestry validation instead of the hardcoded "origin/main..origin/develop".
- Around line 359-362: The current map over stdout lines assumes a space
separator and uses spaceIdx which can be -1, causing slice(0, -1) to drop the
last char of the hash; update the mapping callback (the arrow function that uses
spaceIdx, line, hash, message) to handle the no-space case by checking if
spaceIdx === -1 and in that branch set hash = line.trim() and message = '' (or
use line.split(' ', 2) and default message to ''), otherwise keep the existing
slice logic so empty commit subjects don't corrupt the hash.
---
Duplicate comments:
In `@scripts/release.ts`:
- Around line 516-519: Wrap the merge/push sequence in a try/catch and guard the
push so we only push when merge succeeds: call git().merge(['main']) inside try,
and on error run an explicit rollback sequence (invoke git().raw(['merge',
'--abort']) or git().merge(['--abort']) if supported, then
git().checkout(['develop']) and git().reset(['--hard', 'origin/develop']) to
restore the local branch), rethrow or log the error and exit without calling
git().push(['origin', 'develop']); ensure git().merge and git().push are the
referenced symbols updated so the merge is protected and the repo cannot remain
in a conflicted mid-merge state.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
scripts/release.ts
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
♻️ Duplicate comments (2)
scripts/release.ts (2)
348-351:⚠️ Potential issue | 🔴 CriticalHotfix commit discovery is still hardcoded to
origin/developand lacks ancestry validation.At Line 350, the range is fixed to
origin/main..origin/develop, and at Line 447 hotfix mode consumes that directly. This can still surface unrelated develop commits and ship the wrong changes tomain.Proposed fix
-type UnreleasedCommit = { hash: string; message: string } +type UnreleasedCommit = { hash: string; message: string } -const getUnreleasedCommits = async (): Promise<UnreleasedCommit[]> => { +const getUnreleasedCommits = async (branch: string): Promise<UnreleasedCommit[]> => { const result = await pify(exec)( - 'git log --first-parent --pretty=format:"%H %s" origin/main..origin/develop', + `git log --first-parent --pretty=format:"%H %s" origin/main..origin/${branch}`, ) const stdout = typeof result === 'string' ? result : (result as { stdout: string }).stdout if (!stdout.trim()) return [] @@ const doHotfixRelease = async () => { - const unreleased = await getUnreleasedCommits() + const currentBranch = (await git().revparse(['--abbrev-ref', 'HEAD'])).trim() + const originMainSha = (await git().revparse(['origin/main'])).trim() + const mergeBaseSha = (await git().raw(['merge-base', 'origin/main', `origin/${currentBranch}`])).trim() + if (mergeBaseSha !== originMainSha) { + exit(chalk.red(`Hotfix branch must be based on origin/main. Current branch: ${currentBranch}`)) + } + + const unreleased = await getUnreleasedCommits(currentBranch) if (unreleased.length === 0) { - exit(chalk.red('No unreleased commits found between origin/main and origin/develop.')) + exit(chalk.red(`No unreleased commits found between origin/main and origin/${currentBranch}.`)) }Also applies to: 447-450
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/release.ts` around lines 348 - 351, getUnreleasedCommits currently hardcodes the range "origin/main..origin/develop" which can include unrelated develop commits; update getUnreleasedCommits (and any hotfix branch handling) to compute the correct range dynamically and validate ancestry: determine the merge base between origin/main and the target branch (e.g., with git merge-base origin/main <branch>) and use "<merge-base>..<branch>" for git log, and additionally verify the target branch is a descendant of origin/main (git merge-base --is-ancestor or equivalent) before proceeding so hotfix mode only picks commits actually based on main.
518-521:⚠️ Potential issue | 🟠 MajorHandle
main -> developmerge failures with cleanup and explicit exit.At Line 519, a merge conflict will currently throw and leave a conflicted
developworking tree. This path should mirror the cherry-pick rollback pattern to restoredevelopbefore exiting.Proposed fix
console.log(chalk.green('Checking out develop...')) await git().checkout(['develop']) console.log(chalk.green('Pulling develop...')) await git().pull() + const developSha = (await git().revparse(['HEAD'])).trim() console.log(chalk.green('Merging main back into develop...')) - await git().merge(['main']) + try { + await git().merge(['main']) + } catch (err) { + await git().merge(['--abort']).catch(() => undefined) + await git().reset(['--hard', developSha]) + const message = err instanceof Error ? err.message : String(err) + exit( + chalk.red( + `Merge into develop failed: ${message}\n` + + `Hotfix ${nextVersion} is on main, but develop was reset to ${developSha.slice(0, 8)}. ` + + 'Please merge main into develop manually.', + ), + ) + } console.log(chalk.green('Pushing develop...')) await git().push(['origin', 'develop'])🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/release.ts` around lines 518 - 521, Wrap the git().merge(['main']) call in a try/catch and on error perform the same rollback steps used for cherry-pick failures: abort the merge (git().merge(['--abort']) or equivalent), reset/checkout develop to the previous clean state (e.g., git().reset(['--hard', 'origin/develop']) or the existing rollback routine), log the failure with details, and then exit explicitly with process.exit(1); ensure this uses the same helper functions/patterns as the cherry-pick rollback so that git().merge(['main']), the cleanup, and the subsequent git().push(['origin', 'develop']) behave safely.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@scripts/release.ts`:
- Around line 348-351: getUnreleasedCommits currently hardcodes the range
"origin/main..origin/develop" which can include unrelated develop commits;
update getUnreleasedCommits (and any hotfix branch handling) to compute the
correct range dynamically and validate ancestry: determine the merge base
between origin/main and the target branch (e.g., with git merge-base origin/main
<branch>) and use "<merge-base>..<branch>" for git log, and additionally verify
the target branch is a descendant of origin/main (git merge-base --is-ancestor
or equivalent) before proceeding so hotfix mode only picks commits actually
based on main.
- Around line 518-521: Wrap the git().merge(['main']) call in a try/catch and on
error perform the same rollback steps used for cherry-pick failures: abort the
merge (git().merge(['--abort']) or equivalent), reset/checkout develop to the
previous clean state (e.g., git().reset(['--hard', 'origin/develop']) or the
existing rollback routine), log the failure with details, and then exit
explicitly with process.exit(1); ensure this uses the same helper
functions/patterns as the cherry-pick rollback so that git().merge(['main']),
the cleanup, and the subsequent git().push(['origin', 'develop']) behave safely.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
scripts/release.ts
Guard against empty commit subjects in getUnreleasedCommits where spaceIdx === -1 would corrupt the hash via slice(0, -1). Add try/catch around the merge-main-into-develop step in both hotfix and regular release flows so a conflict doesn't leave develop in a broken state with no recovery guidance. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
NeOMakinG
reviewed
Mar 2, 2026
Collaborator
NeOMakinG
left a comment
NeOMakinG
left a comment
There was a problem hiding this comment.
Code Review — PR #12022
Verdict: ✅ APPROVE (with one required fix)
Summary: Major improvement to the hotfix release flow. Replaces the fragile "branch off main → force push → draft PR" approach with a clean cherry-pick workflow: show unreleased commits → user selects → cherry-pick onto main → tag with patch version → sync private + develop. Proper rollback on failure. gomes-bot already did a thorough deep review — I concur with the approval.
Architecture Assessment
The new flow is fundamentally better:
- Old: User manually branches off main, self-reports ancestry, force-pushes, creates draft PR. Wrong commit range (
latestTag..origin/branch) pulled develop history. - New: Script fetches unreleased commits (
origin/main..origin/develop), presents interactive checkbox selection, cherry-picks oldest-first onto main, tags with patch version, syncs private + develop. User never leaves the script.
What's Good
- Cherry-pick with rollback: On failure →
cherry-pick --abort→reset --hard mainSha→ clean exit with message. Well-implemented. - Develop merge with rollback: Same pattern —
merge --abort→reset --hard developSha→ clean exit. Applied in bothdoHotfixReleaseandmergeRelease. - Defensive parsing:
spaceIdx === -1guard prevents hash corruption from empty commit subjects. - Patch bump instead of minor — correct per semver for hotfixes.
getUnreleasedCommits()usesorigin/main..origin/develop— this is correct for the new flow (cherry-pick model, not branch model). CodeRabbit kept flagging it but it's by design.
⚠️ Required Fix: Missing await on doHotfixRelease()
Line 524:
;(await inquireReleaseType()) === 'Regular' ? await doRegularRelease() : doHotfixRelease()doHotfixRelease() is NOT awaited. With the old flow this was mostly harmless (just created a draft PR), but now doHotfixRelease does cherry-picks, tags, pushes, and develop merges — all critical async operations. Without await, the script could exit mid-operation or swallow errors as unhandled rejections.
Fix: Add await before doHotfixRelease().
Risk Assessment
Low risk — Developer-only CLI tool, no runtime code changes. The missing await is the only real concern and is a one-word fix.
Browser Test
N/A — CLI tooling change, no UI surface.
Without await, the script could exit mid-operation or swallow errors as unhandled rejections during cherry-picks, tags, pushes, and develop merges. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
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
2 participants
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.
Description
The hotfix option in
yarn releasewas shipping all commits since the last tag instead of just the hotfix commits. When a hotfix branch had any develop ancestry,getCommits()used${latestTag}..origin/${branch}which included everything since the last release tag — not just the hotfix.This PR fixes three issues:
origin/main..origin/${branch}so only commits not on main are includedorigin/main, a programmatic check validates the merge-base matchesorigin/main's SHA — catches mistakes where the branch is actually off developpatchbump instead ofminor(both at PR creation and at merge time via PR title prefix detection)The flow:
Issue (if applicable)
Risk
Low risk. Only affects the
scripts/release.tsCLI tool — no runtime application code changes. The release script is only run manually by developers.None. This is a developer tooling fix only.
Testing
Engineering
origin/mainwith a single commityarn release→ select Hotfixchore: hotfix release v1.1010.1)yarn releaseagain → verifymergeRelease()tags with patch versionyarn releaseon develop → verify the hotfix commit is NOT in the regular release diffOperations
This is a developer-only CLI tool change with no user-facing impact.
Screenshots (if applicable)
N/A
Summary by CodeRabbit
New Features
Chores