Conversation
Member
Author
|
@coderabbitai review |
WalkthroughA new GitHub Actions workflow 🚥 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (2)
.github/workflows/patch-changelog.yml (2)
11-14:bump_kindis untyped — usetype: choiceto constrain valid valuesThe
bump_kindinput accepts any string; the*)guard in thecaseblock is the only safeguard. Declaring it astype: choicemakes the allowed values explicit in the UI and prevents accidental typos:♻️ Proposed refactor
bump_kind: description: "Which part to bump when bump=true (major, minor, patch, or custom)" required: false + type: choice + options: + - patch + - minor + - major + - custom default: "patch"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/patch-changelog.yml around lines 11 - 14, The bump_kind workflow input currently allows any string; change the bump_kind input definition to use type: choice and enumerate allowed options (e.g., major, minor, patch, custom) so the UI and validation enforce valid values; keep the default as "patch" and update the bump_kind input block to include type: choice and choices: [major, minor, patch, custom] to replace the current untyped declaration.
103-103:${{ steps.version.outputs.number }}inlined inrun:— prefer an env varLine 103 assigns
VERSION_FOR_COMMITvia an inline${{ }}expression; line 113 does the same directly inside thegh pr createcommand. While the version value originates from the TOML file and is unlikely to contain shell metacharacters, this is the same injection anti-pattern as lines 42-43. Bind viaenv:for consistency:♻️ Proposed fix
- name: Commit and push changes + env: + VERSION_FOR_COMMIT: ${{ steps.version.outputs.number }} run: | git config user.name "github-actions[bot]" git config user.email "github-actions[bot]@users.noreply.github.com" git checkout -b task/prepare_idea_changelog git add . - - VERSION_FOR_COMMIT="${{ steps.version.outputs.number }}" - git commit -m "chore: patch changelog for version $VERSION_FOR_COMMIT" git push origin task/prepare_idea_changelog- name: Create Pull Request env: GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} + VERSION_NUMBER: ${{ steps.version.outputs.number }} run: | gh pr create \ - --title "chore: patch changelog for version ${{ steps.version.outputs.number }}" \ + --title "chore: patch changelog for version $VERSION_NUMBER" \🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/patch-changelog.yml at line 103, The workflow inlines the expression ${{ steps.version.outputs.number }} directly into run: commands (used to set VERSION_FOR_COMMIT and inside the gh pr create invocation), which is an injection anti-pattern; instead declare VERSION_FOR_COMMIT under env: for the job or the specific step and reference that env var in the run: and gh pr create command. Update the step(s) that set/use VERSION_FOR_COMMIT to use env: VERSION_FOR_COMMIT: ${{ steps.version.outputs.number }} and replace any inlined ${{ ... }} occurrences in run: and gh pr create with the shell variable $VERSION_FOR_COMMIT so the version originates from the env binding rather than being interpolated into the shell script.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/patch-changelog.yml:
- Line 38: The condition `if: ${{ github.event.inputs.bump }}` is treating the
string "false" as truthy; replace it with the inputs context for the
workflow_dispatch boolean by changing the condition to use `if: ${{ inputs.bump
}}` (or explicitly compare `github.event.inputs.bump == 'true'`), so update the
`if` expression that references `github.event.inputs.bump` to use `inputs.bump`
(or an explicit string comparison) to ensure the bump step only runs when the
user actually provided true.
- Around line 42-43: The step in the workflow currently inlines user inputs via
the expressions `${{ github.event.inputs.bump_kind }}` and `${{
github.event.inputs.custom_version }}` directly into the shell script (KIND and
CUSTOM), which allows script injection; fix it by moving those expressions into
environment variables (e.g., define KIND and CUSTOM under the step's env: block
using the expressions) and then reference them in the shell script via normal
shell variables ($KIND, $CUSTOM) instead of embedding `${{ }}` in run:, ensuring
values are not expanded into the script text; also ensure you properly
quote/validate these env vars before use if further safety is needed.
- Line 83: Replace the sed substitution that uses '/' as the delimiter (the line
invoking sed with "s/^(idea-plugin-version\s*=\s*\").*(\")$/\1$NEW\2/") with a
variant that uses a delimiter that cannot appear in a version (e.g., '|' or '#')
so $NEW won't break the expression, and add a pre-check for the incoming custom
version (CUSTOM / NEW) to validate it does not contain '/' or other invalid
characters (reject or escape if it does) before running sed; update the workflow
step that sets/uses NEW to perform this simple validation and then run sed with
the new delimiter.
- Around line 100-106: The fixed branch name task/prepare_idea_changelog makes
the job non‑idempotent; change the workflow to compute a unique BRANCH name
(e.g. include github.run_id or github.run_number) and reuse that variable in the
git checkout, git push and gh pr create steps (export BRANCH as a step output or
set it in env), or alternatively delete/force-update the existing branch before
pushing (use git push --force-with-lease or git push --delete origin <branch>
then push). Also guard the PR creation by checking for an existing PR (gh pr
view --head "$BRANCH" or similar) and only run gh pr create when no PR exists to
avoid failure. Ensure references to the branch use the BRANCH variable
everywhere (git checkout -b, git push, gh pr create).
---
Nitpick comments:
In @.github/workflows/patch-changelog.yml:
- Around line 11-14: The bump_kind workflow input currently allows any string;
change the bump_kind input definition to use type: choice and enumerate allowed
options (e.g., major, minor, patch, custom) so the UI and validation enforce
valid values; keep the default as "patch" and update the bump_kind input block
to include type: choice and choices: [major, minor, patch, custom] to replace
the current untyped declaration.
- Line 103: The workflow inlines the expression ${{ steps.version.outputs.number
}} directly into run: commands (used to set VERSION_FOR_COMMIT and inside the gh
pr create invocation), which is an injection anti-pattern; instead declare
VERSION_FOR_COMMIT under env: for the job or the specific step and reference
that env var in the run: and gh pr create command. Update the step(s) that
set/use VERSION_FOR_COMMIT to use env: VERSION_FOR_COMMIT: ${{
steps.version.outputs.number }} and replace any inlined ${{ ... }} occurrences
in run: and gh pr create with the shell variable $VERSION_FOR_COMMIT so the
version originates from the env binding rather than being interpolated into the
shell script.
egorikftp
force-pushed
the
task/patch_changelog
branch
from
cfb6973 to
2db55a7
Compare
Member
Author
|
@coderabbitai review all |
|
✅ Actions performedFull review triggered. |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
.github/workflows/patch-changelog.yml (2)
11-14: Considertype: choiceforbump_kindto get a dropdown in the Actions UIDeclaring
bump_kindas a free-text string means the user can type any value; the*case correctly catches invalids and exits 1. Addingtype: choicewould surface the valid options (major,minor,patch,custom) as a GitHub-rendered dropdown, preventing typos without any behaviour change.♻️ Proposed change
bump_kind: description: "Which part to bump when bump=true (major, minor, patch, or custom)" required: false + type: choice + options: + - patch + - minor + - major + - custom default: "patch"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/patch-changelog.yml around lines 11 - 14, Change the bump_kind input definition to use GitHub Actions' choice type so users get a dropdown of valid values; specifically update the input named bump_kind to add type: choice and supply choices: [major, minor, patch, custom] (leave required/default the same) so the existing validation logic that handles the '*' case remains unchanged but typos are prevented in the UI.
97-122:BRANCHis re-computed in two separate steps; prefer a step outputThe expression
"task/prepare_idea_changelog_${{ github.run_id }}"is duplicated verbatim on lines 99 and 115. If the naming scheme ever changes, both places must be updated in sync. ExportBRANCHas a step output in the "Commit and push changes" step and reference it in "Create Pull Request":♻️ Proposed refactor
- name: Commit and push changes + id: commit run: | BRANCH="task/prepare_idea_changelog_${{ github.run_id }}" ... git push origin "$BRANCH" + echo "branch=$BRANCH" >> "$GITHUB_OUTPUT" - name: Create Pull Request env: GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} run: | - BRANCH="task/prepare_idea_changelog_${{ github.run_id }}" - gh pr create \ --title "chore: patch changelog for version ${{ steps.version.outputs.number }}" \ --body "" \ --label "changelog" \ --base main \ - --head $BRANCH + --head "${{ steps.commit.outputs.branch }}"The unquoted
$BRANCHon line 122 is also worth quoting for correctness even though the current value is safe.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/patch-changelog.yml around lines 97 - 122, In the "Commit and push changes" step, compute BRANCH once, set it as a step output (e.g., echo "::set-output name=branch::$BRANCH" or the newer GITHUB_OUTPUT mechanism) and use that output in the "Create Pull Request" step instead of recomputing the string; update references to BRANCH in both steps (symbols: BRANCH, "Commit and push changes", "Create Pull Request") and ensure you quote the variable when used (use "$BRANCH") to avoid word-splitting issues.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/patch-changelog.yml:
- Around line 53-78: The patch bump silently drops a -SNAPSHOT suffix because
PATCH="0-SNAPSHOT" is used directly in arithmetic; to fix, in the patch branch
of the case (where KIND and variables MAJOR, MINOR, PATCH are handled) split
PATCH into a numeric part and an optional suffix (e.g., using parameter
expansion to extract PATCH_NUM and SUFFIX), perform arithmetic on PATCH_NUM
only, then reassemble NEW using MAJOR.MINOR.$PATCH_NUM plus the original SUFFIX
(so -SNAPSHOT is preserved); update the code around the PATCH bump case and the
final NEW assignment to use the reassembled value.
- Around line 97-109: The workflow currently runs git add . then always tries
git commit which fails when there are no staged changes; modify the "Commit and
push changes" step to check for a staged diff before committing (use BRANCH and
VERSION_FOR_COMMIT as already defined), e.g. run git add . then test if there
are staged changes with git diff --cached --quiet (or git diff --staged --quiet)
and only run git commit -m "chore: patch changelog for version
$VERSION_FOR_COMMIT" and git push origin "$BRANCH" if that test indicates
changes exist; otherwise skip commit/push and exit successfully.
---
Nitpick comments:
In @.github/workflows/patch-changelog.yml:
- Around line 11-14: Change the bump_kind input definition to use GitHub
Actions' choice type so users get a dropdown of valid values; specifically
update the input named bump_kind to add type: choice and supply choices: [major,
minor, patch, custom] (leave required/default the same) so the existing
validation logic that handles the '*' case remains unchanged but typos are
prevented in the UI.
- Around line 97-122: In the "Commit and push changes" step, compute BRANCH
once, set it as a step output (e.g., echo "::set-output name=branch::$BRANCH" or
the newer GITHUB_OUTPUT mechanism) and use that output in the "Create Pull
Request" step instead of recomputing the string; update references to BRANCH in
both steps (symbols: BRANCH, "Commit and push changes", "Create Pull Request")
and ensure you quote the variable when used (use "$BRANCH") to avoid
word-splitting issues.
2db55a7 to
a025064
Compare
egorikftp
force-pushed
the
task/patch_changelog
branch
from
a025064 to
2c6c3a5
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (3)
.github/workflows/patch-changelog.yml (3)
119-126: Minor: quote$BRANCHand move version inline toenv:for consistency.Two nits:
--head $BRANCHis unquoted — whilegithub.run_idproduces no whitespace, quoting shell variables is a best-practice defensive habit.${{ steps.version.outputs.number }}is inlined directly into--title "..."(see comment on the commit step above).♻️ Suggested improvement
- name: Create Pull Request env: GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} + VERSION: ${{ steps.version.outputs.number }} run: | BRANCH="task/prepare_idea_changelog_${{ github.run_id }}" gh pr create \ - --title "chore: patch changelog for version ${{ steps.version.outputs.number }}" \ + --title "chore: patch changelog for version $VERSION" \ --body "" \ --label "changelog" \ --base main \ - --head $BRANCH + --head "$BRANCH"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/patch-changelog.yml around lines 119 - 126, The PR creation call should quote the branch variable and take the version output from steps.version into the job env for consistency: set an env key (e.g., VERSION = ${{ steps.version.outputs.number }}) and use that env var in the gh pr create --title value instead of inlining the expression, and change --head $BRANCH to --head "$BRANCH" (also ensure BRANCH assignment remains BRANCH="task/prepare_idea_changelog_${{ github.run_id }}"). Apply these changes around the BRANCH assignment and the gh pr create invocation.
11-14: Consider addingtype: choicetobump_kindfor better UX.Without a
type: choiceconstraint, thebump_kindinput is a free-text field in the GitHub Actions dispatch UI, which allows mistyped values that only fail at thecasestatement in the shell. Declaring it as a choice restricts the UI to valid options upfront.♻️ Suggested improvement
bump_kind: description: "major, minor or patch" required: false + type: choice + options: + - major + - minor + - patch default: "minor"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/patch-changelog.yml around lines 11 - 14, The bump_kind workflow input is a free-text field which allows typos; change its declaration to use type: choice and add options: ["major","minor","patch"] (keeping the default "minor") so the GitHub Actions UI only permits valid values; update the bump_kind input block (the bump_kind input name) accordingly and verify any downstream case/switch logic still accepts these exact option strings.
110-110: Minor: inline step output inrun:is inconsistent with the env-var security pattern used above.
VERSION_FOR_COMMIT="${{ steps.version.outputs.number }}"is expanded before the shell parses the script. The value is safe here (validatedX.Y.Z/X.Y.Z-SNAPSHOTformat), but it's inconsistent with the pattern used for user inputs (bound viaenv:). Consider aligning for hygiene:♻️ Suggested improvement
- name: Commit and push changes + env: + VERSION_FOR_COMMIT: ${{ steps.version.outputs.number }} run: | BRANCH="task/prepare_idea_changelog_${{ github.run_id }}" git config user.name "github-actions[bot]" git config user.email "github-actions[bot]@users.noreply.github.com" git checkout -b "$BRANCH" git add . - VERSION_FOR_COMMIT="${{ steps.version.outputs.number }}" - git commit -m "chore: patch changelog for version $VERSION_FOR_COMMIT" git push origin "$BRANCH"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/patch-changelog.yml at line 110, The inline expansion VERSION_FOR_COMMIT="${{ steps.version.outputs.number }}" should be changed to follow the env-var pattern used elsewhere: set VERSION_FOR_COMMIT under the step's env mapping using the expression steps.version.outputs.number (i.e., env: VERSION_FOR_COMMIT: ${{ steps.version.outputs.number }}) and then reference $VERSION_FOR_COMMIT inside the run script; update the step that currently declares VERSION_FOR_COMMIT to use env and the run body to read $VERSION_FOR_COMMIT instead of inlining the expression.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In @.github/workflows/patch-changelog.yml:
- Around line 119-126: The PR creation call should quote the branch variable and
take the version output from steps.version into the job env for consistency: set
an env key (e.g., VERSION = ${{ steps.version.outputs.number }}) and use that
env var in the gh pr create --title value instead of inlining the expression,
and change --head $BRANCH to --head "$BRANCH" (also ensure BRANCH assignment
remains BRANCH="task/prepare_idea_changelog_${{ github.run_id }}"). Apply these
changes around the BRANCH assignment and the gh pr create invocation.
- Around line 11-14: The bump_kind workflow input is a free-text field which
allows typos; change its declaration to use type: choice and add options:
["major","minor","patch"] (keeping the default "minor") so the GitHub Actions UI
only permits valid values; update the bump_kind input block (the bump_kind input
name) accordingly and verify any downstream case/switch logic still accepts
these exact option strings.
- Line 110: The inline expansion VERSION_FOR_COMMIT="${{
steps.version.outputs.number }}" should be changed to follow the env-var pattern
used elsewhere: set VERSION_FOR_COMMIT under the step's env mapping using the
expression steps.version.outputs.number (i.e., env: VERSION_FOR_COMMIT: ${{
steps.version.outputs.number }}) and then reference $VERSION_FOR_COMMIT inside
the run script; update the step that currently declares VERSION_FOR_COMMIT to
use env and the run body to read $VERSION_FOR_COMMIT instead of inlining the
expression.
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
1 participant
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.
📝 Changelog
If this PR introduces user-facing changes, please update the relevant Unreleased section in changelogs: