feat: support n commit messages and previous commit message resolving #714
Conversation
📝 WalkthroughWalkthroughAdds multi-commit support driven by an array of messages, introduces a Changes
Sequence Diagram(s)sequenceDiagram
participant Runner as GitHub Actions Runner
participant Action as Commit Action (src/main.ts)
participant Git as Git CLI
participant FS as Workspace FS
Runner->>Action: start with inputs (`message` / `message_template` / others)
Action->>FS: inspect staged changes
alt no staged changes and no --allow-empty
Action->>Runner: log "no changes" and exit
else staged changes or allow-empty
loop for each message in messages[]
Action->>Action: resolve message (apply `message_template` / `{original}`)
Action->>Git: git commit -m "<message>" [commit args]
Git-->>Action: commit result (sha or error)
Action->>FS: re-stage files for next commit (if messages remain)
opt failure
Action->>Runner: log error and stop loop
end
end
Action->>Runner: set outputs (committed, commit_sha, commit_long_sha) from final commit
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
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: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
README.md (1)
323-349: Clarify array input wording (flow vs. block sequence).The text says arrays “have to be encoded … with a YAML flow sequence,” but the next example shows a block sequence. Consider phrasing to allow both formats.
✏️ Suggested wording
-The action supports arrays in `add`, `remove`, and now `message`, but they have to be encoded as a string with a YAML flow sequence: +The action supports arrays in `add`, `remove`, and now `message`, encoded as a string using a YAML/JSON array (flow or block sequence):
🤖 Fix all issues with AI agents
In `@action.yml`:
- Around line 37-41: The YAML for the message_template.description contains
unescaped plain text with "chore: {original}" which breaks parsing; update the
message_template description (the description value under the message_template
mapping) to use a block scalar or a quoted/multiline string so the colon+space
and braces are preserved (e.g., replace the plain-scalar description with a
block scalar indicator or properly quoted multiline text) and keep the existing
explanatory text and examples intact.
In `@src/main.ts`:
- Around line 143-171: The multi-commit loop may attempt commits with no staged
changes after the first commit; before calling git.commit inside the messages
loop (the for loop iterating messages[]), check the index/staged state (e.g.,
run a staged-check helper or use 'git diff --cached --name-only') and if there
are no staged changes and the matchGitArgs(getInput('commit') || '') does not
include '--allow-empty', call core.setFailed with a clear message and
break/return to stop further iterations; otherwise if '--allow-empty' is present
proceed with git.commit as before. Ensure this staged-check is performed each
iteration before invoking git.commit and reference the functions add(),
remove(), git.commit, getInput('commit') and matchGitArgs in your change.
| description: The message for the commit (can be a string, array of strings for multiple commits) | ||
| required: false | ||
| message_template: | ||
| description: Template for commit message. Use 'reuse' to reuse the previous commit's message exactly, or use '{original}' as a placeholder to include the previous message (e.g., '{original}\n\nFormatted code' to suffix, or 'chore: {original}' to prefix) | ||
| required: false |
There was a problem hiding this comment.
Fix YAML syntax error in message_template description.
The plain-scalar description contains chore: {original} (colon + space), which breaks YAML parsing and invalidates the action metadata. Use a block scalar to keep the text intact.
🛠️ Proposed fix
message_template:
- description: Template for commit message. Use 'reuse' to reuse the previous commit's message exactly, or use '{original}' as a placeholder to include the previous message (e.g., '{original}\n\nFormatted code' to suffix, or 'chore: {original}' to prefix)
+ description: >-
+ Template for commit message. Use 'reuse' to reuse the previous commit's
+ message exactly, or use '{original}' as a placeholder to include the
+ previous message (e.g., '{original}\n\nFormatted code' to suffix, or
+ 'chore: {original}' to prefix)📝 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.
| description: The message for the commit (can be a string, array of strings for multiple commits) | |
| required: false | |
| message_template: | |
| description: Template for commit message. Use 'reuse' to reuse the previous commit's message exactly, or use '{original}' as a placeholder to include the previous message (e.g., '{original}\n\nFormatted code' to suffix, or 'chore: {original}' to prefix) | |
| required: false | |
| description: The message for the commit (can be a string, array of strings for multiple commits) | |
| required: false | |
| message_template: | |
| description: >- | |
| Template for commit message. Use 'reuse' to reuse the previous commit's | |
| message exactly, or use '{original}' as a placeholder to include the | |
| previous message (e.g., '{original}\n\nFormatted code' to suffix, or | |
| 'chore: {original}' to prefix) | |
| required: false |
🧰 Tools
🪛 YAMLlint (1.38.0)
[error] 40-40: syntax error: mapping values are not allowed here
(syntax)
🤖 Prompt for AI Agents
In `@action.yml` around lines 37 - 41, The YAML for the
message_template.description contains unescaped plain text with "chore:
{original}" which breaks parsing; update the message_template description (the
description value under the message_template mapping) to use a block scalar or a
quoted/multiline string so the colon+space and braces are preserved (e.g.,
replace the plain-scalar description with a block scalar indicator or properly
quoted multiline text) and keep the existing explanatory text and examples
intact.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@src/main.ts`:
- Around line 167-187: The catch on git.commit currently calls
core.setFailed(err) but lets the surrounding loop continue, so modify the
control flow in the git.commit error handler (the Promise.catch that calls
core.setFailed) to stop further processing: either rethrow the error (so the
async function exits) or set/flip a local failure flag and break out of the loop
before re-staging; ensure subsequent logic that calls add/remove (and the
isLastCommit check) only runs when no commit error occurred — update references
around git.commit, core.setFailed, isLastCommit, add, and remove accordingly.
🧹 Nitpick comments (1)
src/main.ts (1)
122-141: Invalidmessage_templatevalues are silently ignored.If a user provides a
message_templatethat is neither'reuse'nor contains the{original}placeholder, the template is silently ignored and the originalmessagesarray is used instead. This could lead to confusion if users misspell the placeholder or misunderstand the API.Consider adding a warning when the template doesn't match expected patterns:
💡 Suggested improvement
if (messageTemplate === 'reuse') { // Reuse the entire previous commit message messages = [previousMessage]; core.info(`> Reusing previous commit message: "${previousMessage}"`); } else if (messageTemplate.includes('{original}')) { // Replace {original} placeholder with previous message in the template const templatedMessage = messageTemplate.replace(/\{original\}/g, previousMessage); messages = [templatedMessage]; core.info(`> Using message template: "${templatedMessage}"`); + } else { + core.warning( + `message_template "${messageTemplate}" is not recognized. ` + + `Use "reuse" or include "{original}" placeholder. Using default messages.` + ); }
| await git | ||
| .commit(message, commitArgs) | ||
| .then(async data => { | ||
| log(undefined, data); | ||
|
|
||
| // Only set outputs for the last commit | ||
| if (isLastCommit) { | ||
| setOutput('committed', 'true'); | ||
| setOutput('commit_long_sha', data.commit); | ||
| setOutput('commit_sha', data.commit.substring(0, 7)); | ||
| } | ||
| }) | ||
| .catch(err => core.setFailed(err)); | ||
|
|
||
| // Re-stage files for subsequent commits if there are more to create | ||
| if (!isLastCommit) { | ||
| core.info('> Re-staging files for next commit...'); | ||
| if (getInput('add')) await add(ignoreErrors); | ||
| if (getInput('remove')) await remove(ignoreErrors); | ||
| } | ||
| } |
There was a problem hiding this comment.
Loop continues after commit failure - missing control flow.
When git.commit() fails at line 179, core.setFailed(err) is called but the loop continues to the next iteration. This causes the action to attempt re-staging and subsequent commits even after a failure, leading to confusing error cascades. This is inconsistent with the break at line 160 for staged changes errors.
🔧 Proposed fix to break on commit failure
await git
.commit(message, commitArgs)
.then(async data => {
log(undefined, data);
// Only set outputs for the last commit
if (isLastCommit) {
setOutput('committed', 'true');
setOutput('commit_long_sha', data.commit);
setOutput('commit_sha', data.commit.substring(0, 7));
}
})
- .catch(err => core.setFailed(err));
+ .catch(err => {
+ core.setFailed(err);
+ throw err; // Re-throw to exit the loop
+ });
// Re-stage files for subsequent commits if there are more to create
if (!isLastCommit) {
core.info('> Re-staging files for next commit...');
if (getInput('add')) await add(ignoreErrors);
if (getInput('remove')) await remove(ignoreErrors);
}
}Alternatively, use a flag-based approach:
+ let commitFailed = false;
for (let i = 0; i < messages.length; i++) {
+ if (commitFailed) break;
const message = messages[i];
// ... existing code ...
await git
.commit(message, commitArgs)
// ... .then() ...
- .catch(err => core.setFailed(err));
+ .catch(err => {
+ core.setFailed(err);
+ commitFailed = true;
+ });
+
+ if (commitFailed) break;
// Re-stage files for subsequent commits📝 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.
| await git | |
| .commit(message, commitArgs) | |
| .then(async data => { | |
| log(undefined, data); | |
| // Only set outputs for the last commit | |
| if (isLastCommit) { | |
| setOutput('committed', 'true'); | |
| setOutput('commit_long_sha', data.commit); | |
| setOutput('commit_sha', data.commit.substring(0, 7)); | |
| } | |
| }) | |
| .catch(err => core.setFailed(err)); | |
| // Re-stage files for subsequent commits if there are more to create | |
| if (!isLastCommit) { | |
| core.info('> Re-staging files for next commit...'); | |
| if (getInput('add')) await add(ignoreErrors); | |
| if (getInput('remove')) await remove(ignoreErrors); | |
| } | |
| } | |
| await git | |
| .commit(message, commitArgs) | |
| .then(async data => { | |
| log(undefined, data); | |
| // Only set outputs for the last commit | |
| if (isLastCommit) { | |
| setOutput('committed', 'true'); | |
| setOutput('commit_long_sha', data.commit); | |
| setOutput('commit_sha', data.commit.substring(0, 7)); | |
| } | |
| }) | |
| .catch(err => { | |
| core.setFailed(err); | |
| throw err; // Re-throw to exit the loop | |
| }); | |
| // Re-stage files for subsequent commits if there are more to create | |
| if (!isLastCommit) { | |
| core.info('> Re-staging files for next commit...'); | |
| if (getInput('add')) await add(ignoreErrors); | |
| if (getInput('remove')) await remove(ignoreErrors); | |
| } | |
| } |
🤖 Prompt for AI Agents
In `@src/main.ts` around lines 167 - 187, The catch on git.commit currently calls
core.setFailed(err) but lets the surrounding loop continue, so modify the
control flow in the git.commit error handler (the Promise.catch that calls
core.setFailed) to stop further processing: either rethrow the error (so the
async function exits) or set/flip a local failure flag and break out of the loop
before re-staging; ensure subsequent logic that calls add/remove (and the
isLastCommit check) only runs when no commit error occurred — update references
around git.commit, core.setFailed, isLastCommit, add, and remove accordingly.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@src/main.ts`:
- Around line 132-135: The replacement of the {original} placeholder uses
String.replace with a string replacement which can misinterpret $ sequences in
previousMessage; update the logic around messageTemplate.replace(...) (the
variables messageTemplate and previousMessage and the messages = [...]
assignment) to call String.replace with a replacer function that returns
previousMessage (instead of a replacement string) so that any $ characters in
previousMessage are treated literally and not expanded by the engine.
| } else if (messageTemplate.includes('{original}')) { | ||
| // Replace {original} placeholder with previous message in the template | ||
| const templatedMessage = messageTemplate.replace(/\{original\}/g, previousMessage); | ||
| messages = [templatedMessage]; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let me check the structure and find src/main.ts
git ls-files | grep -E "src/main\.(ts|js)$"Repository: EndBug/add-and-commit
Length of output: 76
🏁 Script executed:
# Read the relevant section of src/main.ts around lines 132-135
head -n 150 src/main.ts | tail -n 50 | cat -nRepository: EndBug/add-and-commit
Length of output: 2395
🏁 Script executed:
# Get more context around the template replacement to understand previousMessage source
sed -n '100,160p' src/main.ts | cat -nRepository: EndBug/add-and-commit
Length of output: 3093
Escape replacement to avoid $-expansion in templates.
String.replace() with a string replacement interprets $&, $$, and $n sequences specially. If a previous commit message contains these sequences, they will be misinterpreted—for example, $& will be replaced with the matched text {original} instead of preserving the literal message. Use a replacer function to prevent this.
🛠️ Proposed fix
- const templatedMessage = messageTemplate.replace(/\{original\}/g, previousMessage);
+ const templatedMessage = messageTemplate.replace(
+ /\{original\}/g,
+ () => previousMessage,
+ );📝 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.
| } else if (messageTemplate.includes('{original}')) { | |
| // Replace {original} placeholder with previous message in the template | |
| const templatedMessage = messageTemplate.replace(/\{original\}/g, previousMessage); | |
| messages = [templatedMessage]; | |
| } else if (messageTemplate.includes('{original}')) { | |
| // Replace {original} placeholder with previous message in the template | |
| const templatedMessage = messageTemplate.replace( | |
| /\{original\}/g, | |
| () => previousMessage, | |
| ); | |
| messages = [templatedMessage]; |
🤖 Prompt for AI Agents
In `@src/main.ts` around lines 132 - 135, The replacement of the {original}
placeholder uses String.replace with a string replacement which can misinterpret
$ sequences in previousMessage; update the logic around
messageTemplate.replace(...) (the variables messageTemplate and previousMessage
and the messages = [...] assignment) to call String.replace with a replacer
function that returns previousMessage (instead of a replacement string) so that
any $ characters in previousMessage are treated literally and not expanded by
the engine.
|
I think a much better approach would be to remove the formatter. Second best would be to force formatting compliance before merging, i.e. add formatting as a PR check. |
|
Hi @ibrkhalil I've looked at the PR very quickly: I get the idea of reusing a commit message, but I'm not sure I understand why you would need to create more than one commit? Could you elaborate on that a bit? |
3 participants
We have formatting github actions and I need the ability to see git blame easily
Summary by CodeRabbit
New Features
Documentation
✏️ Tip: You can customize this high-level summary in your review settings.