feat: Support posting of dive results to PR for succesful runs
#69
Conversation
To enable such posibility, add `always-comment: true` to GHA invocation
📝 WalkthroughWalkthroughThis update introduces a new optional Boolean parameter, Changes
Sequence Diagram(s)sequenceDiagram
participant Runner as Action Runner
participant Main as run() in src/main.ts
participant Comment as GitHub PR Comment Service
Runner->>Main: Trigger execution with inputs (including always-comment)
Main-->>Main: Retrieve and convert always-comment input
Main->>Main: Execute Docker analysis and obtain exitCode
alt always-comment is true
Main->>Comment: Post analysis comment
Main-->>Runner: Return after comment post
else always-comment is false
alt exitCode is 0 (no issues)
Main-->>Runner: Early exit (no comment posted)
else
Main->>Comment: Post analysis comment for detected issues
Main-->>Runner: Return result after comment post
end
end
Possibly related PRs
✨ Finishing Touches
🪧 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
Documentation and Community
|
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (3)
dist/index.jsis excluded by!**/dist/**dist/index.js.mapis excluded by!**/dist/**,!**/*.mappackage-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (4)
README.md(2 hunks)action.yml(1 hunks)package.json(1 hunks)src/main.ts(3 hunks)
✅ Files skipped from review due to trivial changes (1)
- package.json
🔇 Additional comments (3)
action.yml (1)
11-16: LGTM! Well-structured configuration for the new feature.The new
always-commentparameter is well-defined with a clear description and appropriate default value that maintains backward compatibility.src/main.ts (1)
71-71: LGTM! Clean input parameter handling.The Boolean conversion of the input parameter is implemented correctly.
README.md (1)
20-27: LGTM! Clear and comprehensive documentation.The changes are well-documented with:
- Clear parameter description
- Improved table formatting for better readability
- Up-to-date example usage
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
.github/workflows/test.yml (1)
158-177: Consider adding a descriptive comment for the error condition.While the test case is well-implemented, it would be helpful to add a comment explaining why we expect this error (similar to the comment on line 130).
Apply this diff to add the explanatory comment:
- name: '[Verification] Negative test: always-comment w/o github-token' + # Verifies that the action properly handles missing GitHub token when always-comment is true + # Expected to fail as posting comments requires GitHub token authentication # It will not be able to create comment outside PR. result in main branch: # Not Found - https://docs.github.com/rest if: always() && steps.pre-setup.outcome == 'success' && github.ref !=🧰 Tools
🪛 actionlint (1.7.4)
164-164: property "error" is not defined in object type {}
(expression)
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/test.yml(1 hunks)
🧰 Additional context used
🪛 actionlint (1.7.4)
.github/workflows/test.yml
164-164: property "error" is not defined in object type {}
(expression)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: Analyze (typescript)
- GitHub Check: Analyze (javascript)
🔇 Additional comments (1)
.github/workflows/test.yml (1)
150-157: LGTM! The test case configuration is well-structured.The test case follows the established pattern and correctly tests the
always-commentparameter without a GitHub token.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
.github/workflows/test.yml (1)
150-177: LGTM! Comprehensive test coverage for the new parameter.The test cases thoroughly verify the behavior when
always-commentis used without the requiredgithub-token. The test structure follows the established pattern and includes proper error message verification.Consider adding a positive test case to verify the successful scenario when both
always-commentandgithub-tokenare provided.🧰 Tools
🪛 actionlint (1.7.4)
164-164: property "error" is not defined in object type {}
(expression)
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
.github/workflows/test.yml(1 hunks)README.md(2 hunks)action.yml(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- README.md
🧰 Additional context used
🪛 actionlint (1.7.4)
.github/workflows/test.yml
164-164: property "error" is not defined in object type {}
(expression)
🔇 Additional comments (2)
action.yml (2)
11-16: LGTM! Well-defined input parameter.The new
always-commentparameter is well-defined with a clear description, appropriate default value for backward compatibility, and correctly mentions the dependency ongithub-token.
22-22: LGTM! Improved description clarity.The updated description better clarifies the purpose of the GitHub token by explicitly stating it's used for posting dive analysis as PR comments.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
.github/workflows/test.yml (1)
184-194: Enhance test coverage for successful comment posting.While the test case verifies the basic functionality, consider adding verification steps to ensure:
- The comment is actually created on the PR.
- The posted comment contains the expected dive analysis results.
Add a verification step similar to other test cases:
- name: '[Verification] Params: always-comment with github-token' if: github.ref != 'refs/heads/main' shell: bash run: | # Verify comment creation using GitHub API comment_id=$(gh api graphql -f query=' query($owner:String!, $repo:String!, $pr:Int!) { repository(owner:$owner, name:$repo) { pullRequest(number:$pr) { comments(last:1) { nodes { id body } } } } }' -f owner="${{ github.repository_owner }}" \ -f repo="${{ github.event.repository.name }}" \ -f pr="${{ github.event.pull_request.number }}" \ --jq '.data.repository.pullRequest.comments.nodes[0].id') if [[ -z "$comment_id" ]]; then echo "Expected comment not found on PR" exit 1 fi # Verify comment content comment_body=$(gh api graphql -f query=' query($owner:String!, $repo:String!, $pr:Int!) { repository(owner:$owner, name:$repo) { pullRequest(number:$pr) { comments(last:1) { nodes { body } } } } }' -f owner="${{ github.repository_owner }}" \ -f repo="${{ github.event.repository.name }}" \ -f pr="${{ github.event.pull_request.number }}" \ --jq '.data.repository.pullRequest.comments.nodes[0].body') if [[ ! "$comment_body" =~ "Dive Analysis Results" ]]; then echo "Comment content does not match expected format" exit 1 fi env: GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/test.yml(1 hunks)
🧰 Additional context used
🪛 actionlint (1.7.4)
.github/workflows/test.yml
168-168: property "error" is not defined in object type {}
(expression)
🔇 Additional comments (1)
.github/workflows/test.yml (1)
150-182: LGTM! Well-structured negative test case.The test case thoroughly validates the behavior when
always-commentis enabled without providing the requiredgithub-token. The verification step includes comprehensive checks for both the error message and step outcome.🧰 Tools
🪛 actionlint (1.7.4)
168-168: property "error" is not defined in object type {}
(expression)
This comment was marked as resolved.
This comment was marked as resolved.
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
dist/index.jsis excluded by!**/dist/**dist/index.js.mapis excluded by!**/dist/**,!**/*.map
📒 Files selected for processing (2)
.github/workflows/test.yml(2 hunks)src/main.ts(3 hunks)
🧰 Additional context used
🪛 actionlint (1.7.4)
.github/workflows/test.yml
171-171: property "error" is not defined in object type {}
(expression)
🔇 Additional comments (6)
src/main.ts (3)
47-51: LGTM! Improved error handling.The modification ensures consistent error propagation by throwing an Error after setting outputs.
53-61: LGTM! Well-structured comment posting logic.The new
postCommentfunction follows SOLID principles by encapsulating the comment posting logic in a reusable way.
85-93: LGTM! Robust parameter validation.The code properly validates the required
github-tokenwhenalways-commentis true, with clear comments explaining the boolean conversion logic..github/workflows/test.yml (3)
24-27: LGTM! Proper permissions configuration.The pull-requests write permission is correctly set and documented for the comment posting functionality.
153-185: LGTM! Comprehensive negative test case.The test thoroughly validates the error handling when
always-commentis used without a GitHub token, including proper error message verification and branch handling.🧰 Tools
🪛 actionlint (1.7.4)
171-171: property "error" is not defined in object type {}
(expression)
187-197: LGTM! Well-structured positive test case.The test properly validates the success path for
always-commentwith GitHub token, including appropriate branch handling.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
.github/workflows/test.yml (1)
187-195: Add verification step for successful comment posting.While the test case correctly sets up the parameters, it lacks verification that the comment was successfully posted to the PR.
Consider adding a verification step that uses the GitHub API to check if the comment was posted:
+ - name: '[Verification] Params: always-comment with github-token' + if: github.ref != 'refs/heads/main' + shell: bash + env: + GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} + run: | + # Wait briefly for the comment to be posted + sleep 5 + + # Get the PR number + pr_number=$(jq --raw-output .pull_request.number "$GITHUB_EVENT_PATH") + + # Check if the comment exists + gh api "repos/${{ github.repository }}/issues/${pr_number}/comments" \ + --jq 'any(.body | contains("Dive Analysis Results"))' || { + echo "Expected comment not found in PR" + exit 1 + }
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/test.yml(2 hunks)
🧰 Additional context used
🪛 actionlint (1.7.4)
.github/workflows/test.yml
171-171: property "error" is not defined in object type {}
(expression)
🔇 Additional comments (2)
.github/workflows/test.yml (2)
24-27: LGTM! Well-scoped permissions.The pull-requests write permission is correctly scoped and well-documented.
156-183: LGTM! Comprehensive negative test case.The test case thoroughly verifies the behavior when
always-commentis set without a GitHub token:
- Clear error message verification
- Proper outcome validation
- Appropriate use of
continue-on-error🧰 Tools
🪛 actionlint (1.7.4)
171-171: property "error" is not defined in object type {}
(expression)
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
dist/index.jsis excluded by!**/dist/**dist/index.js.mapis excluded by!**/dist/**,!**/*.map
📒 Files selected for processing (2)
.github/workflows/test.yml(2 hunks)src/main.ts(3 hunks)
🧰 Additional context used
🪛 actionlint (1.7.4)
.github/workflows/test.yml
171-171: property "error" is not defined in object type {}
(expression)
🔇 Additional comments (6)
src/main.ts (3)
53-61: LGTM!The function is well-implemented with proper async/await handling and correct usage of GitHub context.
85-93: LGTM!The parameter validation is well-implemented with:
- Clear boolean conversion logic
- Proper validation of required github-token when always-comment is true
148-169: LGTM!The error handling implementation properly addresses the past review comments by:
- Awaiting the postComment call
- Correctly handling the exit code when always-comment is true
.github/workflows/test.yml (3)
24-27: LGTM!The permissions are properly scoped to only what's needed for posting comments to PRs.
156-183: LGTM!The negative test case is well-implemented with:
- Proper error handling using continue-on-error
- Comprehensive verification of both error message and step outcome
🧰 Tools
🪛 actionlint (1.7.4)
171-171: property "error" is not defined in object type {}
(expression)
187-195: LGTM!The positive test case is well-implemented with proper parameter configuration and PR-only execution.
e59980a to
7c0394a
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
src/main.ts (3)
8-11: Consider using readonly arrays for immutability.The
customLeadingCommentparameter is an array that should not be modified. Consider usingreadonly string[]to enforce immutability.- customLeadingComment: string[] + customLeadingComment: readonly string[]
36-44: Extract table formatting logic into a helper function.The markdown table construction logic could be more readable and maintainable if extracted into a dedicated helper function.
+function formatTableRow(line: string): string { + const count = line.slice(0, 5).trim() + const wastedSpace = line.slice(7, 19).trim() + const filePath = line.slice(21).trim() + return `| ${count} | ${wastedSpace} | ${filePath} |` +} if (line.startsWith('Count')) { ret.push('| Count | Wasted Space | File Path |') ret.push('|---|---|---|') } else { - ret.push( - `| ${line.slice(0, 5)} | ${line.slice(7, 19)} | ${line.slice(21)} |` - ) + ret.push(formatTableRow(line)) }
155-163: Ensure consistent error message format.The error message format should be consistent with other error messages in the codebase. Consider adding a period at the end of the message.
-const scanFailedErrorMsg = `Scan failed (exit code: ${exitCode})` +const scanFailedErrorMsg = `Scan failed (exit code: ${exitCode}).`
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
dist/index.jsis excluded by!**/dist/**dist/index.js.mapis excluded by!**/dist/**,!**/*.map
📒 Files selected for processing (2)
.github/workflows/test.yml(2 hunks)src/main.ts(4 hunks)
🧰 Additional context used
🪛 actionlint (1.7.4)
.github/workflows/test.yml
171-171: property "error" is not defined in object type {}
(expression)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: test
- GitHub Check: Analyze (typescript)
🔇 Additional comments (5)
src/main.ts (2)
50-54: Remove redundant process.exit(1) call.The
process.exit(1)call is redundant ascore.setFailedalready sets the exit code and handles the failure appropriately.
157-163: Add await to prevent potential race condition.The
postCommentcall should be awaited to ensure the comment is posted before the error is thrown..github/workflows/test.yml (3)
24-27: LGTM! Clear and well-documented permissions.The permissions section is well-documented with a clear explanation of why the
pull-requests: writepermission is needed.
153-183: LGTM! Comprehensive test coverage.The negative test case for
always-commentwithout a GitHub token is well-implemented and includes proper verification of error messages.🧰 Tools
🪛 actionlint (1.7.4)
171-171: property "error" is not defined in object type {}
(expression)
185-220: LGTM! Thorough verification of comment posting.The verification step for
always-commentwith GitHub token is comprehensive, checking both the existence and content of the posted comment.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
src/main.ts (1)
176-182: 🛠️ Refactor suggestionAdd error handling for GitHub API calls.
The
postCommentcall should be wrapped in a try-catch block to handle potential API failures gracefully.Apply this diff:
- await postComment(ghToken, diveOutput) + try { + await postComment(ghToken, diveOutput) + } catch (error) { + core.error(`Failed to post comment: ${error instanceof Error ? error.message : String(error)}`) + }
🧹 Nitpick comments (1)
.github/workflows/test.yml (1)
187-220: Consider adding more assertions to the verification step.The verification step could be enhanced to validate the comment content.
run: | # Get the PR number pr_number=$(jq --raw-output .pull_request.number "$GITHUB_EVENT_PATH") echo "PR number: $pr_number" # Check if the comment exists and get the latest comment ID comment_id=$(gh api "repos/${{ github.repository }}/issues/${pr_number}/comments" --jq \ 'sort_by(.created_at) | reverse | .[0] | select(.body | contains("Dive Summary")) | .id') echo "Comment ID: $comment_id" if [ -z "$comment_id" ]; then echo "Expected comment not found in PR" exit 1 fi + # Get comment content + comment_body=$(gh api "repos/${{ github.repository }}/issues/comments/${comment_id}" --jq '.body') + + # Verify required sections + for section in "Dive Summary" "Results"; do + if [[ ! "$comment_body" =~ $section ]]; then + echo "Missing required section: $section" + exit 1 + fi + done + # Delete the comment after verification gh api -X DELETE "repos/${{ github.repository }}/issues/comments/${comment_id}" echo "Comment deleted"
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
dist/index.jsis excluded by!**/dist/**dist/index.js.mapis excluded by!**/dist/**,!**/*.map
📒 Files selected for processing (2)
.github/workflows/test.yml(2 hunks)src/main.ts(2 hunks)
🧰 Additional context used
🪛 actionlint (1.7.4)
.github/workflows/test.yml
171-171: property "error" is not defined in object type {}
(expression)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: Analyze (typescript)
- GitHub Check: Analyze (javascript)
🔇 Additional comments (5)
src/main.ts (3)
111-119: LGTM! Clear parameter validation.The implementation correctly validates the
always-commentparameter and its dependency on the GitHub token.
174-174: LGTM! Improved error message.The error message now includes the exit code, which helps with debugging.
184-198:✅ Verification successful
Verify error handling when GitHub token is missing.
The error message for missing GitHub token is clear, but let's verify the behavior in different scenarios.
🏁 Script executed:
#!/bin/bash # Description: Verify error handling for missing GitHub token # Test cases: # 1. Missing token with always-comment=true # 2. Missing token with always-comment=false and failed scan # Search for error handling patterns rg -A 5 "github-token" || echo "No token handling found" # Search for error messages rg -A 5 "Scan failed" || echo "No error messages found"Length of output: 69639
Error Handling for Missing GitHub Token Verified
The search results confirm that the error handling paths are implemented as expected. The code correctly:
- Checks for missing GitHub token when the "always-comment" flag is enabled, issuing the clear error:
"always-comment" parameter requires "github-token" to be set.- Uses a distinct error message when the scan fails and the token is missing:
Scan failed (exit code: …). To post scan results as a PR comment, please provide the github-token in the action inputs.No additional changes are needed.
.github/workflows/test.yml (2)
24-27: LGTM! Correct permissions setup.The
pull-requests: writepermission is correctly set for posting comments.
156-183: LGTM! Comprehensive error testing.The test case properly verifies the error handling when
always-commentis true but GitHub token is missing.🧰 Tools
🪛 actionlint (1.7.4)
171-171: property "error" is not defined in object type {}
(expression)
To enable such possibility, add
always-comment: trueto GHA invocationExample usage:
Summary by CodeRabbit
always-commentparameter and its behavior, including modifications to the existinggithub-tokendescription.always-commentparameter is set to true without a GitHub token.always-commentfunctionality.