-
Notifications
You must be signed in to change notification settings - Fork 7
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: Automerge #1247
base: main
Are you sure you want to change the base?
feat: Automerge #1247
Conversation
Adds helper scripts Adds hopefully good action to manage automerges
WalkthroughThis pull request introduces several new scripts and a GitHub Actions workflow to automate Git operations related to rebasing and cherry-picking branches. The Changes
Possibly related PRs
Suggested reviewers
Warning There were issues while running some tools. Please review the errors and either fix the tool’s configuration or disable the tool if it’s a critical failure. 🔧 eslint (1.23.1)
scripts/git/rebase-branch.jsOops! Something went wrong! :( ESLint: 8.57.1 ESLint couldn't find the plugin "eslint-plugin-prettier". (The package "eslint-plugin-prettier" was not found when loaded as a Node module from the directory "".) It's likely that the plugin isn't installed correctly. Try reinstalling by running the following:
The plugin "eslint-plugin-prettier" was referenced from the config file in ".eslintrc.json » eslint-config-universe/native » ./shared/prettier.js". If you still can't figure out the problem, please stop by https://eslint.org/chat/help to chat with the team. Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 12
🧹 Outside diff range and nitpick comments (9)
scripts/git/rebase-branch.js (1)
1-90
: Consider adding tests and documentationAs this script handles sensitive Git operations and is part of an automated system:
- Add unit tests mocking Git operations
- Add integration tests with a test repository
- Document usage, prerequisites, and failure scenarios
- Consider adding logging for audit trails
Would you like help setting up a testing framework and documentation structure?
scripts/git/auto_rebase.js (2)
69-80
: Use a semantic versioning library for accurate comparisonThe
compareReleaseBranches
function manually parses and compares version numbers, which may not handle all edge cases of semantic versioning (e.g., pre-release identifiers). Consider using thesemver
library for robust version comparison.Apply this diff to utilize
semver
:+const semver = require('semver'); function compareReleaseBranches(a, b) { - // Extract version numbers and compare - const versionA = a.replace("release/", "").split(".").map(Number); - const versionB = b.replace("release/", "").split(".").map(Number); - - for (let i = 0; i < Math.max(versionA.length, versionB.length); i++) { - const numA = versionA[i] || 0; - const numB = versionB[i] || 0; - if (numA !== numB) { - return numA - numB; - } - } - return 0; + const versionA = a.replace("release/", ""); + const versionB = b.replace("release/", ""); + return semver.compare(versionA, versionB); }Don't forget to install
semver
by adding it to your dependencies:npm install semver
113-131
: Include actual rebase output in the issue bodyThe issue body currently contains a placeholder for rebase output. Capturing and including the actual rebase output will help maintainers resolve conflicts more efficiently.
Modify
rebaseBranch
to return the error message:async function rebaseBranch(baseBranch, targetBranch) { try { // Existing code... return true; } catch (error) { const errorMessage = error.message; await git.rebase(["--abort"]); - return false; + return { success: false, errorMessage }; } }Update the call in
main()
:- if (!rebaseResult) { + if (!rebaseResult.success) { await createIssue(octokit, owner, repo, updatedBranch, nextBranch, rebaseResult.errorMessage); } else { console.log(`Successfully rebased ${nextBranch} onto ${updatedBranch}`); }And update
createIssue
function to accept the error message:async function createIssue(octokit, owner, repo, baseBranch, targetBranch, errorMessage) { // ... <summary>Rebase attempt output</summary> \`\`\` - [Rebase output and conflict details can be included here] + ${errorMessage} \`\`\` // ... }scripts/git/cherry-pick-branch.js (6)
140-157
: Handle empty commit list more gracefullyIn
getCommitsNotInTarget
, if there are no commits, the function returns an empty array. Ensure that this case is appropriately handled in the calling function.Consider adding a check after fetching
commitsOutput
to handle situations where the output may be empty or when thegit log
command returns nothing due to no new commits.
26-115
: Avoid using 'process.exit' for better error handlingUsing
process.exit()
within the function can prevent proper cleanup or further error handling by parent processes or calling functions.Replace
process.exit(0);
andprocess.exit(1);
with return statements or throw errors as appropriate.Apply this diff:
- process.exit(0); + return;- process.exit(1); + throw new Error("Operation aborted by the user.");
73-104
: Improve user experience during conflict resolutionWhen a conflict occurs during cherry-picking, the script exits after prompting the user to resolve conflicts manually. This can be streamlined to enhance usability.
Consider implementing a loop that waits for the user to resolve conflicts and then continues the cherry-pick operation. Alternatively, automate the conflict resolution if possible or provide clearer instructions.
190-190
: Export functions for reusability and testingCurrently,
cherryPickCommits()
is invoked directly. Exporting the function can facilitate testing and reuse in other scripts or modules.Apply this diff to export the function:
+module.exports = { cherryPickCommits };
5-22
: Validate user inputs for source and target branchesThe script assumes valid branch names are provided. Adding validation can prevent errors later in the execution.
Consider adding validation prompts or checks to ensure the branch names are valid and not empty.
47-65
: Improve commit selection interfaceFor repositories with many commits, the commit selection prompt may become unwieldy.
Consider implementing a search or filter functionality within the prompt to help users find specific commits more easily.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (4)
.github/workflows/auto_rebase.yml
(1 hunks)scripts/git/auto_rebase.js
(1 hunks)scripts/git/cherry-pick-branch.js
(1 hunks)scripts/git/rebase-branch.js
(1 hunks)
🔇 Additional comments (5)
.github/workflows/auto_rebase.yml (1)
26-29
: 🛠️ Refactor suggestion
Verify script existence and add error handling
The workflow assumes the existence of scripts/git/auto_rebase.js
and lacks error handling.
Let's verify the script exists and check its implementation:
Add error handling and script verification:
- name: Run Auto Rebase Script
env:
GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
- run: node scripts/git/auto_rebase.js
+ run: |
+ if [ ! -f "scripts/git/auto_rebase.js" ]; then
+ echo "Error: auto_rebase.js script not found!"
+ exit 1
+ fi
+ node scripts/git/auto_rebase.js || {
+ echo "Auto rebase failed!"
+ exit 1
+ }
scripts/git/rebase-branch.js (1)
1-2
:
Security: Potential command injection vulnerability
The combination of user input and shell: true
in spawn could lead to command injection. Consider using shell: false
and properly escaping arguments.
scripts/git/auto_rebase.js (1)
96-96
: Ensure safe force-pushing with updated references
Using --force-with-lease
helps prevent overwriting changes made by others. However, if the remote branch has new commits, the push will be rejected. Ensure that this behavior aligns with your workflow requirements.
Consider verifying the push behavior:
Ensure that the script handles rejected pushes appropriately.
✅ Verification successful
Force-push with lease is properly implemented with error handling
The implementation in rebaseBranch
function is safe and well-structured:
- Fetches latest changes before rebase
- Uses
--ff-only
for pulling to ensure clean state - Has proper error handling for conflicts
- Aborts rebase on conflicts and creates issues for manual intervention
- Propagates other errors for handling in the main function
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify that force-with-lease behaves as expected
# Fetch the latest changes
git fetch origin
# Simulate concurrent update by adding a new commit to the remote branch
# (This would be done by another user in a real scenario)
# Attempt to push with --force-with-lease
git push origin targetBranch --force-with-lease
Length of output: 203
Script:
#!/bin/bash
# Let's examine the error handling in the auto_rebase.js file
rg -A 10 "force-with-lease" scripts/git/auto_rebase.js
Length of output: 393
Script:
#!/bin/bash
# Let's check the entire function implementation to understand the error handling context
ast-grep --pattern 'async function $FUNC($$$) {
$$$
force-with-lease
$$$
}'
Length of output: 83
Script:
#!/bin/bash
# Let's examine the entire auto_rebase.js file to understand the complete context
cat scripts/git/auto_rebase.js
Length of output: 4315
scripts/git/cherry-pick-branch.js (2)
1-2
: Ensure 'prompts' library is included in dependencies
The script uses the prompts
library, but it may not be listed in your project's package.json
dependencies.
Run the following script to check if prompts
is listed:
✅ Verification successful
prompts
dependency is correctly declared in package.json
The package is already listed in the project's dependencies with version ^2.4.2
.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify 'prompts' is declared in package.json dependencies.
grep '"prompts"' package.json || echo "'prompts' is not listed in dependencies."
Length of output: 56
26-36
: Ensure branches are properly checked out and up-to-date
When fetching and checking out branches, it's crucial to handle potential issues like non-existent branches or fetch failures.
Run the following script to confirm that the branches exist and are fetched correctly:
on: | ||
push: | ||
branches: | ||
- "release/*" | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add workflow permissions and trigger restrictions
The current trigger configuration could pose security risks:
- Missing
permissions
configuration to limit the workflow's access scope - No restrictions on who can trigger the workflow
Apply these changes to improve security:
name: Auto Rebase Branches
on:
push:
branches:
- "release/*"
+ paths-ignore:
+ - "**.md"
+
+permissions:
+ contents: write
+ issues: write
+
+# Ensure workflow is not triggered by forks
+if: github.repository == 'ephemeraHQ/converse-app'
Committable suggestion skipped: line range outside the PR's diff.
} | ||
|
||
// Attempt to rebase nextBranch onto updatedBranch | ||
const rebaseResult = await rebaseBranch(updatedBranch, nextBranch); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Handle the case when nextBranch
is null
to prevent errors
Currently, if nextBranch
is null
, calling rebaseBranch(updatedBranch, nextBranch)
will result in an error. Add a check to ensure nextBranch
is not null
before attempting to rebase.
Apply this diff to fix the issue:
if (nextBranch) {
// Attempt to rebase nextBranch onto updatedBranch
const rebaseResult = await rebaseBranch(updatedBranch, nextBranch);
if (!rebaseResult) {
// Rebase failed due to conflicts, create an issue
await createIssue(octokit, owner, repo, updatedBranch, nextBranch);
} else {
console.log(`Successfully rebased ${nextBranch} onto ${updatedBranch}`);
}
+ } else {
+ console.log('No subsequent release branches to rebase.');
}
Committable suggestion skipped: line range outside the PR's diff.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (4)
scripts/git/cherry-pick-branch.js (4)
81-83
: Enhance error message for cherry-pick conflictsThe current error message could be more informative about the nature of the conflict.
- console.error( - `Conflict occurred while cherry-picking commit ${commit.hash}.` - ); + console.error( + `Conflict occurred while cherry-picking commit ${commit.hash}.\n` + + `Conflicted files:\n${await executeCommand("git", ["diff", "--name-only", "--diff-filter=U"], { captureOutput: true })}` + );
111-113
: Add push confirmation promptConsider adding a confirmation prompt before pushing to the remote branch.
+ const { confirmPush } = await prompts({ + type: "confirm", + name: "confirmPush", + message: `Are you sure you want to push ${commitsToCherryPick.length} commits to ${targetBranch}?`, + initial: true, + }); + + if (!confirmPush) { + console.log("Push cancelled. Your changes are still local."); + process.exit(0); + } + await executeCommand("git", ["push", "origin", targetBranch]);
159-176
: Enhance commit information retrievalConsider including more commit details and handling edge cases better.
- ["log", `${targetBranch}..${sourceBranch}`, "--pretty=format:%H %s"], + ["log", `${targetBranch}..${sourceBranch}`, "--pretty=format:%H|%an|%ae|%s"], { captureOutput: true } ); const commits = commitsOutput .split("\n") .filter((line) => line.trim() !== "") .map((line) => { - const [hash, ...messageParts] = line.split(" "); + const [hash, author, email, ...messageParts] = line.split("|"); return { hash, + author, + email, - message: messageParts.join(" ") + message: messageParts.join("|").trim() || "(no commit message)" }; });
1-178
: Consider implementing a dry-run modeThe script handles Git operations well, but adding a dry-run mode would allow users to preview the changes before applying them.
This could be implemented by:
- Adding a dry-run prompt at the beginning
- When enabled, show what commits would be cherry-picked
- Display which files would be modified
- Simulate the cherry-pick process without actually performing it
This would help users validate their selections before making any changes.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (2)
scripts/git/auto_rebase.js
(1 hunks)scripts/git/cherry-pick-branch.js
(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- scripts/git/auto_rebase.js
🔇 Additional comments (1)
scripts/git/cherry-pick-branch.js (1)
1-2
: Verify the 'prompts' package dependency
The script uses the 'prompts' package but we need to ensure it's properly declared in package.json.
✅ Verification successful
'prompts' package dependency is properly configured and secure
The package is correctly declared in package.json with version ^2.4.2
as a production dependency, and there are no known security vulnerabilities reported for this package.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check if prompts package is declared in package.json
# and verify its version for security concerns
# Check if prompts is in package.json
if [ -f "package.json" ]; then
echo "Checking package.json for prompts dependency..."
cat package.json | jq '.dependencies.prompts, .devDependencies.prompts'
fi
# Check latest version and security advisories
gh api graphql -f query='
{
securityVulnerabilities(first: 5, ecosystem: NPM, package: "prompts") {
nodes {
advisory {
summary
severity
publishedAt
}
vulnerableVersionRange
firstPatchedVersion {
identifier
}
}
}
}'
Length of output: 559
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (2)
scripts/git/rebase-branch.js (2)
42-65
: Enhance error handling with specific error typesThe error handling could be improved by categorizing different types of Git errors and providing more specific user guidance.
} catch (error) { + const errorMessage = error.message.toLowerCase(); console.error( "An error occurred during the rebase process:", error.message ); - // Check if the error is due to conflicts if ( - error.message.includes("CONFLICT") || - error.message.includes("Merge conflict") + errorMessage.includes("conflict") || + errorMessage.includes("merge conflict") ) { console.error("Conflicts were detected during the rebase."); + } else if (errorMessage.includes("could not find remote branch")) { + console.error("The specified branch does not exist on the remote."); + } else if (errorMessage.includes("permission denied")) { + console.error("You don't have permission to perform this operation."); + } else if (errorMessage.includes("not a git repository")) { + console.error("Not in a git repository. Please run this script from a git repository root."); + } // Abort the rebase to return to a clean state await executeCommand("git", ["rebase", "--abort"]);
1-93
: Consider adding automated testsGiven the critical nature of Git operations, consider adding automated tests using a Git testing framework like
jest-git-utils
or similar. This would help ensure the reliability of the rebase operations.Key areas to test:
- Branch validation
- Conflict scenarios
- Permission issues
- Network failures
Would you like me to help create a testing strategy or open an issue to track this task?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (2)
.github/workflows/auto_rebase.yml
(1 hunks)scripts/git/rebase-branch.js
(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- .github/workflows/auto_rebase.yml
🔇 Additional comments (2)
scripts/git/rebase-branch.js (2)
90-93
: LGTM!
The main execution properly handles errors and sets appropriate exit codes.
1-2
: Specify version for the prompts package
To ensure consistent behavior across different environments, consider specifying the version of the prompts
package in your package.json.
// If dry-run is successful, perform the actual rebase | ||
console.log("Proceeding with the rebase..."); | ||
await executeCommand("git", ["rebase", `origin/${baseBranch}`]); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove or implement the dry-run comment
The comment suggests a dry-run check, but no dry-run is actually performed. Either implement the dry-run functionality or remove the misleading comment.
- // If dry-run is successful, perform the actual rebase
console.log("Proceeding with the rebase...");
await executeCommand("git", ["rebase", `origin/${baseBranch}`]);
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
// If dry-run is successful, perform the actual rebase | |
console.log("Proceeding with the rebase..."); | |
await executeCommand("git", ["rebase", `origin/${baseBranch}`]); | |
console.log("Proceeding with the rebase..."); | |
await executeCommand("git", ["rebase", `origin/${baseBranch}`]); |
Adds helper scripts
Adds hopefully good action to manage automerges
Summary by CodeRabbit
New Features
Bug Fixes