bad title test#216
Conversation
Signed-off-by: Ashhar Ahmad Khan <145142826+AshharAhmadKhan@users.noreply.github.com>
|
Hi @exploreriii, this is **LinkBot** 👋
Linking pull requests to issues helps us significantly with reviewing pull requests and keeping the repository healthy. 🚨 This pull request does not have an issue linked. Please link an issue using the following format: 📖 Guide: If no issue exists yet, please create one: Thanks! |
WalkthroughIntroduces a new GitHub bot script to automate PR title validation against conventional-commit patterns and provide guidance via PR comments. Updates the corresponding workflow to enable dry-run mode, add new permissions, and trigger bot comment logic on validation failure. Changes
Sequence DiagramsequenceDiagram
participant WF as Workflow
participant Bot as Bot Script
participant GH as GitHub API
participant PR as PR
WF->>WF: Trigger on pull_request event
WF->>Bot: run({github, context, prNumber, prTitle})
activate Bot
Bot->>Bot: suggestConventionalType(prTitle)
Note over Bot: Parse title against<br/>conventional patterns
Bot->>Bot: Generate suggested type
Bot->>GH: Fetch PR comments<br/>(paginated, up to 500)
activate GH
GH-->>Bot: Return comment list
deactivate GH
Bot->>Bot: Search for existing<br/>bot comment by marker
alt Bot comment exists
Bot->>GH: Update existing comment
else Bot comment not found
Bot->>GH: Create new comment
end
activate GH
GH->>PR: Update/Create comment
GH-->>Bot: Success or error
deactivate GH
Bot->>Bot: formatMessage(title, type, prNumber)
Note over Bot: Build Markdown<br/>guidance message
deactivate Bot
Bot-->>WF: Return status
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~35 minutes 🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (2 inconclusive)
✅ Passed checks (1 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}
| function escapeForMarkdown(text) { | ||
| return text | ||
| .replace(/```/g, "'''") | ||
| .replace(/\r?\n|\r/g, ' ') | ||
| .trim(); | ||
| } |
There was a problem hiding this comment.
escapedTitle doesn't strip newlines, which can break the suggested CLI command.
escapeForMarkdown strips newlines from safeTitle, but the shell-escaped escapedTitle (line 164) is derived from the raw currentTitle and retains embedded newlines. A PR title containing \n would produce a broken multi-line gh pr edit command in the instructions.
Proposed fix — strip newlines before shell-escaping
function formatMessage(currentTitle, suggestedType, prNumber) {
- // Escape shell-sensitive characters for the CLI example
- const escapedTitle = currentTitle.replace(/["$`\\]/g, '\\$&');
const safeTitle = escapeForMarkdown(currentTitle);
+ // Escape shell-sensitive characters for the CLI example (use safeTitle which already has newlines stripped)
+ const escapedTitle = safeTitle.replace(/["$`\\]/g, '\\$&');
return composeBotMessage({Also applies to: 162-164
| // Fetch comments with pagination and early exit | ||
| let commentCount = 0; | ||
| let page = 1; | ||
| let botComment = null; | ||
|
|
||
| while (commentCount < MAX_COMMENTS_TO_FETCH && !botComment) { | ||
| const response = await github.rest.issues.listComments({ | ||
| owner: context.repo.owner, | ||
| repo: context.repo.repo, | ||
| issue_number: prNumber, | ||
| per_page: 100, | ||
| page: page | ||
| }); | ||
|
|
||
| commentCount += response.data.length; | ||
|
|
||
| // Check if we found the bot comment | ||
| botComment = response.data.find(comment => | ||
| comment.body && comment.body.includes(COMMENT_IDENTIFIER) | ||
| ); | ||
|
|
||
| // Exit if no more pages or found the comment | ||
| if (response.data.length < 100 || botComment) { | ||
| break; | ||
| } | ||
|
|
||
| page++; | ||
| } | ||
|
|
||
| console.log(`[Bot] Fetched ${commentCount} comments across ${page} page(s)`); |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Pagination is well-bounded with early exit — consider filtering by bot author.
The pagination logic is correct with a 500-comment upper bound and early termination. One minor improvement: filter comments by the bot's user login (or at minimum check comment.user.type === 'Bot') to avoid matching the marker in a non-bot comment.
| on: | ||
| workflow_dispatch: | ||
| inputs: | ||
| dry_run: | ||
| description: 'Run without posting comments' | ||
| type: boolean | ||
| default: true | ||
| pull_request_target: | ||
| types: | ||
| - opened | ||
| - reopened | ||
| - edited | ||
| - synchronize |
There was a problem hiding this comment.
synchronize trigger type is unnecessary for a title-check workflow.
The synchronize event fires when new commits are pushed to the PR branch, but that doesn't change the PR title. This causes redundant workflow runs on every push. Only opened, reopened, and edited are relevant for title validation.
Proposed fix — remove synchronize
pull_request_target:
types:
- opened
- reopened
- edited
- - synchronize📝 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.
| on: | |
| workflow_dispatch: | |
| inputs: | |
| dry_run: | |
| description: 'Run without posting comments' | |
| type: boolean | |
| default: true | |
| pull_request_target: | |
| types: | |
| - opened | |
| - reopened | |
| - edited | |
| - synchronize | |
| on: | |
| workflow_dispatch: | |
| inputs: | |
| dry_run: | |
| description: 'Run without posting comments' | |
| type: boolean | |
| default: true | |
| pull_request_target: | |
| types: | |
| - opened | |
| - reopened | |
| - edited |
🧰 Tools
🪛 YAMLlint (1.38.0)
[warning] 6-6: truthy value should be one of [false, true]
(truthy)
| script: | | ||
| const script = require('./.github/scripts/bot-conventional-pr-title.js'); | ||
|
|
||
| if (!context.payload.pull_request) { | ||
| console.log('[Bot] No pull_request in context, skipping'); | ||
| return; | ||
| } | ||
|
|
||
| const isDryRun = context.eventName === 'workflow_dispatch' | ||
| ? (context.payload.inputs?.dry_run === 'true') | ||
| : false; | ||
|
|
||
| await script.run({ | ||
| github: github, | ||
| context: context, | ||
| prNumber: context.payload.pull_request.number, | ||
| prTitle: context.payload.pull_request.title, | ||
| dryRun: isDryRun | ||
| }); |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Inline script is appropriately minimal — orchestration only.
The YAML delegates all logic to the external script, with the inline block handling only payload validation and parameter passing. This aligns well with the guideline to keep business logic in .github/scripts/.
One note: when triggered via workflow_dispatch, context.payload.pull_request will be undefined, so the guard at line 67 will always exit early — making the dry_run input effectively untestable through workflow_dispatch alone. Consider documenting this or adding a pr_number input for manual testing.
2 participants
Description:
Related issue(s):
Fixes #
Notes for reviewer:
Checklist