-
Notifications
You must be signed in to change notification settings - Fork 91
ChatOps for fixing API generated code conflicts #2566
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
base: main
Are you sure you want to change the base?
Conversation
📝 WalkthroughWalkthroughThis pull request introduces a new GitHub Action, "Rebase API Action", built with TypeScript to automate the rebasing of pull requests when triggered by specific comment commands. It adds several configuration files, including version control ignores, Prettier settings, and TypeScript/Vitest configurations, alongside comprehensive documentation and a dedicated workflow. The changes include source code implementing Git operations, GitHub API interactions, conflict resolution, input management, and tests to validate functionality. Changes
🪧 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 (
|
/rebase-api |
1 similar comment
/rebase-api |
/rebase-api |
/rebase-api |
For the workflow to run it needs to be checked in to main. Check this example run on openmeter-internal to see it working https://github.com/openmeterio/openmeter-internal/pull/10#issuecomment-2766602162 |
Will do some further testing if approved (as testing the actual conflicts isn't really possible on openemter-internal due to it being such an old version) |
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: 4
🧹 Nitpick comments (14)
.github/actions/rebase-api/src/types.ts (1)
1-11
: Consider adding detailed JSDoc for each interface propertyThe
ActionInputs
interface looks well-defined with appropriate types, but would benefit from JSDoc comments for each property to improve developer experience./** * Input parameters for the action */ export interface ActionInputs { + /** GitHub token with repo scope to perform Git operations */ githubToken: string + /** ID of the comment that triggered the action */ commentId: number + /** Full reference to the PR branch (e.g., 'refs/heads/feature-branch') */ prBranchRef: string + /** Name of the base branch to rebase against (e.g., 'main') */ baseBranch: string + /** Relative path to the API directory containing code generation scripts */ apiDirectory: string + /** Phrase that triggers this action when found in comments */ triggerPhrase: string }.github/actions/rebase-api/package.json (1)
18-18
: Consider adding author information and license.The
author
field is currently empty. Consider adding author information and a license field to complete the package metadata..github/actions/rebase-api/src/main.ts (2)
43-43
: Branch name extraction assumption could be fragile.The branch name extraction assumes a specific format with
refs/heads/
prefix. Consider adding a more robust extraction method to handle potential variations in branch references.-const prBranchName = inputs.prBranchRef.replace('refs/heads/', '') +const prBranchName = inputs.prBranchRef.includes('/') + ? inputs.prBranchRef.split('/').slice(-1)[0] + : inputs.prBranchRef
58-64
: Consider adding warning about force push.Force pushing to a PR branch can overwrite changes. While this is mentioned in the PR summary, it would be helpful to add a log warning in the code for clarity.
// If we reach here, rebase was successful or conflicts were resolved. core.info('Rebase successful or conflicts resolved.') +core.warning('Force pushing to PR branch. This will overwrite any changes made after the rebase was triggered.') await pushChanges(prBranchName)
.github/actions/rebase-api/README.md (1)
41-43
: Add language specification to the code block.The fenced code block should have a language specification for improved syntax highlighting and better markdown practices.
-``` +```bash /rebase-api<details> <summary>🧰 Tools</summary> <details> <summary>🪛 markdownlint-cli2 (0.17.2)</summary> 41-41: Fenced code blocks should have a language specified null (MD040, fenced-code-language) </details> </details> </blockquote></details> <details> <summary>.github/actions/rebase-api/src/utils.ts (2)</summary><blockquote> `19-21`: **Consider using core.setFailed for error handling.** Instead of throwing an error directly, consider using the `core.setFailed` method from `@actions/core`. This would properly mark the GitHub Action as failed with the appropriate error message. ```diff if (isNaN(commentId)) { - throw new Error('Input "comment_id" is not a valid integer.') + core.setFailed('Input "comment_id" is not a valid integer.') + throw new Error('Input "comment_id" is not a valid integer.') }
56-62
: Consider adding more robust error handling for command execution.The
tryExec
function returns the exit code but doesn't provide any logging about command failures. Consider enhancing this utility to also log warnings when commands fail.export async function tryExec( command: string, args?: string[], options?: exec.ExecOptions, ): Promise<number> { - return await exec.exec(command, args, { ...options, ignoreReturnCode: true }) + const exitCode = await exec.exec(command, args, { ...options, ignoreReturnCode: true }) + if (exitCode !== 0) { + core.warning(`Command '${command} ${args?.join(' ') || ''}' failed with exit code ${exitCode}`) + } + return exitCode }.github/actions/rebase-api/src/github.ts (2)
22-24
: Add more specific error handling for different types of API errors.The current error handling is generic. Consider handling specific error cases differently, especially rate limiting or permission issues which could provide more actionable feedback.
} catch (error: any) { - core.warning(`Failed to add reaction '${reaction}': ${error.message}`) + if (error.status === 403) { + core.warning(`Permission denied when adding reaction '${reaction}': ${error.message}`) + } else if (error.status === 429) { + core.warning(`Rate limit exceeded when adding reaction '${reaction}': ${error.message}`) + } else { + core.warning(`Failed to add reaction '${reaction}': ${error.message}`) + } }
43-48
: Clarify error handling strategy with comments.Lines 45-47 contain comments that suggest two different error handling strategies (return null or throw error), but the implementation only follows one approach (throwing an error). Remove the confusing comments.
} catch (error: any) { core.error(`Failed to get comment body: ${error.message}`) - // Return null or re-throw, depending on desired handling - // For this use case, failing to get the body should probably stop the action throw new Error(`Could not retrieve comment body for ID ${commentId}.`) }.github/actions/rebase-api/src/main.test.ts (2)
102-107
: Minimize reliance on console.log in tests.While it's fine for quick debugging, consider replacing console.log calls with Vitest's logging or additional core.debug usage to keep the test output clean and consistent.
210-261
: Consider additional test scenarios for more granular conflict patterns.Currently, you test either conflicts outside the directory (leading to an abort) or exclusively inside the directory (allowing automated resolution). You might add a scenario where multiple conflicting files exist in different directories—some within
api/
and others already resolved or not conflicting—to ensure partial merges or partial conflicts are handled as expected.Also applies to: 316-388
.github/actions/rebase-api/action.yml (1)
4-26
: Standardize input naming convention.You have mixed naming styles (underscores vs. hyphens) in fields such as
github_token
andtrigger-phrase
. Consistency in naming helps maintain clarity and avoids confusion for users configuring these inputs..github/actions/rebase-api/src/git.ts (2)
24-28
: Optional optimization for large repositories.When fetching the base branch, you could consider using a shallow fetch (e.g.,
git fetch --depth=1 origin branch
) to minimize bandwidth in very large repos. It’s not strictly necessary but can improve performance.
118-144
: Consider verifying resolved paths to protect against symlink edge cases.Relying on
path.normalize
and a simplestartsWith
check can be subverted by symlinks or other path quirks that escape theapiDirectory
. If security is a concern, you might consider checking paths with arealpath
approach to ensure all conflicts stay truly within the intended directory.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
.github/actions/rebase-api/package-lock.json
is excluded by!**/package-lock.json
📒 Files selected for processing (15)
.github/actions/rebase-api/.gitignore
(1 hunks).github/actions/rebase-api/.prettierignore
(1 hunks).github/actions/rebase-api/.prettierrc.json
(1 hunks).github/actions/rebase-api/README.md
(1 hunks).github/actions/rebase-api/action.yml
(1 hunks).github/actions/rebase-api/package.json
(1 hunks).github/actions/rebase-api/src/git.ts
(1 hunks).github/actions/rebase-api/src/github.ts
(1 hunks).github/actions/rebase-api/src/main.test.ts
(1 hunks).github/actions/rebase-api/src/main.ts
(1 hunks).github/actions/rebase-api/src/types.ts
(1 hunks).github/actions/rebase-api/src/utils.ts
(1 hunks).github/actions/rebase-api/tsconfig.json
(1 hunks).github/actions/rebase-api/vitest.config.ts
(1 hunks).github/workflows/rebase-api.yml
(1 hunks)
🧰 Additional context used
🧬 Code Definitions (4)
.github/actions/rebase-api/src/main.ts (4)
.github/actions/rebase-api/src/types.ts (1)
ActionInputs
(4-11).github/actions/rebase-api/src/utils.ts (1)
getInputs
(8-30).github/actions/rebase-api/src/github.ts (2)
getCommentBody
(30-49)addReaction
(8-25).github/actions/rebase-api/src/git.ts (5)
configureGitUser
(10-19)fetchBranch
(24-28)attemptRebase
(33-42)handleRebaseConflicts
(118-152)pushChanges
(157-163)
.github/actions/rebase-api/src/utils.ts (1)
.github/actions/rebase-api/src/types.ts (1)
ActionInputs
(4-11)
.github/actions/rebase-api/src/git.ts (2)
.github/actions/rebase-api/src/utils.ts (2)
tryExec
(56-62)getExecOutput
(35-50).github/actions/rebase-api/src/types.ts (1)
ActionInputs
(4-11)
.github/actions/rebase-api/src/main.test.ts (1)
.github/actions/rebase-api/src/main.ts (1)
run
(20-79)
🪛 LanguageTool
.github/actions/rebase-api/README.md
[style] ~67-~67: Consider shortening or rephrasing this to strengthen your wording.
Context: ...:`) - Rebase failed ## Development To make changes to this action: 1. Edit the TypeScript fi...
(MAKE_CHANGES)
🪛 markdownlint-cli2 (0.17.2)
.github/actions/rebase-api/README.md
41-41: Fenced code blocks should have a language specified
null
(MD040, fenced-code-language)
🪛 actionlint (1.7.4)
.github/workflows/rebase-api.yml
56-56: file "dist/index.js" does not exist in "/home/jailuser/git/.github/actions/rebase-api". it is specified at "main" key in "runs" section in "Rebase API Changes" action
(action)
56-56: missing input "github_token" which is required by action "Rebase API Changes" defined at "./.github/actions/rebase-api". all required inputs are "comment_id", "github_token", "pr_branch_ref"
(action)
56-56: missing input "comment_id" which is required by action "Rebase API Changes" defined at "./.github/actions/rebase-api". all required inputs are "comment_id", "github_token", "pr_branch_ref"
(action)
56-56: missing input "pr_branch_ref" which is required by action "Rebase API Changes" defined at "./.github/actions/rebase-api". all required inputs are "comment_id", "github_token", "pr_branch_ref"
(action)
58-58: input "github-token" is not defined in action "Rebase API Changes" defined at "./.github/actions/rebase-api". available inputs are "api_directory", "base_branch", "comment_id", "github_token", "pr_branch_ref", "trigger-phrase"
(action)
59-59: input "comment-id" is not defined in action "Rebase API Changes" defined at "./.github/actions/rebase-api". available inputs are "api_directory", "base_branch", "comment_id", "github_token", "pr_branch_ref", "trigger-phrase"
(action)
60-60: input "pr-branch-ref" is not defined in action "Rebase API Changes" defined at "./.github/actions/rebase-api". available inputs are "api_directory", "base_branch", "comment_id", "github_token", "pr_branch_ref", "trigger-phrase"
(action)
61-61: input "base-branch" is not defined in action "Rebase API Changes" defined at "./.github/actions/rebase-api". available inputs are "api_directory", "base_branch", "comment_id", "github_token", "pr_branch_ref", "trigger-phrase"
(action)
62-62: input "api-directory" is not defined in action "Rebase API Changes" defined at "./.github/actions/rebase-api". available inputs are "api_directory", "base_branch", "comment_id", "github_token", "pr_branch_ref", "trigger-phrase"
(action)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: CI
🔇 Additional comments (17)
.github/actions/rebase-api/.gitignore (1)
1-20
: Standard Node.js .gitignore setup looks goodThis file includes all standard patterns needed for a Node.js GitHub Action:
- Node dependencies
- Build output
- Various log files
- OS-specific files
All the essential items are properly excluded, which will keep the repository clean.
.github/actions/rebase-api/.prettierignore (1)
1-8
: Appropriate Prettier ignore patternsThe file correctly ignores:
- Configuration files (.npmrc, pnpm-lock.yaml)
- Generated output (dist/)
- Dependencies (node_modules/)
- Test artifacts (coverage/)
Good practice to note the reference to existing patterns in line 1, promoting consistency across the project.
.github/actions/rebase-api/vitest.config.ts (1)
1-12
: Well-configured Vitest setup with clear inline documentationThe Vitest configuration is appropriate for a Node.js project with good explanatory comments for each setting. The configuration enables:
- Globals for easier test writing
- Node.js test environment
- Automatic mock clearing between tests
Good use of comments to document the purpose of each setting.
.github/actions/rebase-api/tsconfig.json (1)
1-15
: Standard TypeScript configuration is well-structured.The TypeScript configuration looks appropriate for a GitHub Action with suitable compiler options. The configuration enables strict type checking, proper module resolution, and excludes test files from compilation.
.github/actions/rebase-api/package.json (4)
1-18
: Package metadata and scripts are well-configured.The package definition and scripts for building, testing, and formatting look good. Using
ncc
for bundling is appropriate for GitHub Actions.
19-23
: Dependencies are appropriate for GitHub Actions.The action uses the standard GitHub Actions libraries with pinned versions allowing minor updates, which is a good practice.
24-30
: Development dependencies are well-chosen.The development tooling with TypeScript, Vitest for testing, and Prettier for formatting is appropriate.
31-31
: Package manager pinned to specific version.Pinning to a specific pnpm version with SHA hash ensures consistency across environments.
.github/actions/rebase-api/.prettierrc.json (1)
1-9
: Formatting configuration is consistent.The Prettier configuration defines a consistent code style with reasonable settings.
.github/actions/rebase-api/src/main.ts (7)
1-14
: Imports are appropriately organized.The imports are well-structured, separating external dependencies from local modules.
17-24
: Main function is well-documented with initial state.The main function is documented and initializes variables properly, starting with a default assumption of failure.
25-42
: Comment checking is well-implemented.The code correctly checks if the comment contains the trigger phrase and exits gracefully if not, avoiding unnecessary processing.
44-57
: Rebase handling logic is clear and well-structured.The code properly configures Git, fetches the base branch, attempts the rebase, and handles conflicts when they occur.
65-72
: Error handling is comprehensive.The error handling captures and logs the error message and stack trace for debugging, and properly marks the action as failed.
73-78
: Reaction is added regardless of outcome.The finally block ensures that appropriate reactions are added to the comment regardless of success or failure, which provides good feedback to users.
81-87
: Self-execution is properly implemented.The code correctly checks if it's being run directly and handles any unhandled errors that might occur.
.github/actions/rebase-api/src/main.test.ts (1)
45-54
: Good helper function for command/args matching.Your approach of comparing the command and arguments via JSON serialization is clear and effective for verifying calls in your test. No concerns here.
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
🧹 Nitpick comments (1)
.github/workflows/rebase-api.yml (1)
32-49
: PR Branch Information Extraction is EffectiveUsing
actions/github-script
to retrieve the pull request details (head and base refs, head SHA) is a solid approach.
Suggestion: Consider adding error handling within the script (or logging additional diagnostic details) in case the API call fails unexpectedly during runtime.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/rebase-api.yml
(1 hunks)
🧰 Additional context used
🪛 actionlint (1.7.4)
.github/workflows/rebase-api.yml
72-72: file "dist/index.js" does not exist in "/home/jailuser/git/.github/actions/rebase-api". it is specified at "main" key in "runs" section in "Rebase API Changes" action
(action)
⏰ Context from checks skipped due to timeout of 90000ms (6)
- GitHub Check: E2E
- GitHub Check: CI
- GitHub Check: Developer environment
- GitHub Check: Lint
- GitHub Check: Test
- GitHub Check: Analyze (go)
🔇 Additional comments (11)
.github/workflows/rebase-api.yml (11)
1-2
: Header Declaration Looks GoodThe workflow’s name ("Rebase API via PR Comment") is clear and descriptive.
3-6
: Trigger Configuration is CorrectThe workflow is configured to trigger on issue comment creation and edits, which aligns with the purpose defined in the PR.
7-10
: Permissions Defined AppropriatelyThe required permissions (write access for contents and pull-requests) are set correctly to allow pushing rebased code and reacting to comments.
11-18
: Job Conditions and Environment Setup are SolidThe job is gated to run only when the comment is on a pull request—and only if the comment author has an appropriate association (COLLABORATOR, MEMBER, or OWNER). This condition helps prevent unauthorized actions.
19-26
: Code Checkout Step is Well ConfiguredUsing the
actions/checkout
action with a full history fetch (i.e., fetch-depth: 0) ensures that rebasing can be performed properly. The explicit reference to the pull request head is also correctly set up.
27-31
: Base Branch Fetch is Implemented CorrectlyThe step fetching the repository’s default branch with the appropriate depth and token is straightforward and appears effective for acquiring the latest state of the base branch.
50-53
: Correct Checkout of PR Head SHAChecking out the correct commit using the retrieved head SHA helps avoid potential inconsistencies if the initial checkout was off.
54-58
: Local Branch Creation is LogicalCreating a local branch and setting its upstream to the correct remote branch is a practical step to ensure that subsequent operations target the intended branch.
59-63
: Node.js Setup is Standard and Up-to-DateThe step that sets up Node.js (version 20) is appropriate for building the custom GitHub Action.
70-80
: Custom Action Invocation with Correct Input NamesThe inputs passed to the custom action (
github_token
,comment_id
,pr_branch_ref
,base_branch
, andapi_directory
) conform to the naming expected by the action definition. The use of a local branch ("local-branch") as the PR branch reference is clear and explicit. The commented-out trigger phrase is acceptable given that a default is provided by the action’s configuration.🧰 Tools
🪛 actionlint (1.7.4)
72-72: file "dist/index.js" does not exist in "/home/jailuser/git/.github/actions/rebase-api". it is specified at "main" key in "runs" section in "Rebase API Changes" action
(action)
82-85
: Action Outcome Check is StraightforwardThe final step to check the outcome of the rebase action and to exit with an error message if it fails provides a proper safeguard to ensure workflow integrity.
Overview
Adds a custom GH Action to listen to chat commands.
You can use /rebase-api to execute the following (resolving 90% of conflicts that need manual rebasing):
The action has reasonable safety measures (fails on unknown conflicts)
📜 Check the README for details 📜
Summary by CodeRabbit
New Features
Documentation
Configuration
.gitignore
,.prettierignore
, and.prettierrc.json
, to streamline development and formatting processes.Tests