feat: add notification workflow for Good First Issue candidates [(#1296)] #1342
Conversation
📝 WalkthroughWalkthroughAdds a GitHub Actions workflow and a Node.js script that post a team notification comment when an issue is labeled "good first issue candidate", skipping already-notified issues by checking for a unique marker. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant GitHub as GitHub<br/>(Issue System)
participant Workflow as GitHub Actions<br/>Workflow
participant Script as Notification<br/>Script
participant API as GitHub API
User->>GitHub: Add label "good first issue candidate" to issue
GitHub->>Workflow: Emit issue_label event
Workflow->>Workflow: Guard verifies label match
Workflow->>Script: Execute with { github, context }
Script->>API: List comments for the issue
API-->>Script: Return comments
Script->>Script: Search for unique marker
alt Marker present
Script-->>Workflow: Skip posting (duplicate)
else Marker absent
Script->>API: Post notification comment (includes marker)
API-->>Script: Confirm comment created
Script-->>Workflow: Log success
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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: 5
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (3)
.github/scripts/bot-gfi-candidate-notification.js.github/workflows/bot-gfi-candidate-notification.yamlCHANGELOG.md
🧰 Additional context used
📓 Path-based instructions (2)
.github/workflows/**/*
⚙️ CodeRabbit configuration file
.github/workflows/**/*: Review workflows as security-sensitive infrastructure.A good workflow is small, focused, and boring.
If a workflow is clever, generic, or overly flexible, it is a risk.
PRIORITY 0 — ABSOLUTE REQUIREMENTS
- All third-party actions MUST be pinned to full commit SHAs, similar to other workflows.
permissions:MUST be explicitly declared and minimally scoped.- Workflows MUST behave safely when executed from forks.
- YAML MUST orchestrate steps, not implement business logic.
- Any workflow that mutates GitHub state MUST support dry-run mode.
- Dry-run behavior must be explicit and visible in logs.
- Workflows MUST NOT modify repository source code outside
.github/.
PRIORITY 1 — SCOPE, FOCUS & RESTRAINT
- The title of each workflow must be relevant, match similar naming schemes, and match its script filename.
- Each workflow MUST have a single, clearly defined objective and SHOULD document this in a top-level comment.
- Flag workflows that:
- Attempt to be generic “frameworks”
- Include speculative or future-facing logic
- Perform actions unrelated to the stated goal
- Over-abstraction and excess flexibility are maintenance risks.
PRIORITY 2 — INPUT HARDENING
- Treat ALL GitHub event data as potentially hostile input, including:
- issue titles, bodies, and comments
- labels, usernames, branch names
- Free-form user input MUST NOT be passed directly into:
- shell commands
- gh CLI arguments
- Node.js exec / spawn calls
- Require strict allowlists or exact string matches.
- Flag any use of:
- eval or bash -c
- backticks or $(...) with user-controlled input
------------------...
Files:
.github/workflows/bot-gfi-candidate-notification.yaml
.github/scripts/**/*.js
⚙️ CodeRabbit configuration file
.github/scripts/**/*.js: Review JavaScript scripts as long-lived automation code.Scripts must remain:
Focused
Readable
Purpose-built
All
context.payloadfields MUST be validatedFree-form text MUST NOT be trusted
Dynamic code execution is prohibited
Avoid
child_process.exec; preferexecFileif neededAll async operations MUST be wrapped in try/catch
Errors MUST include contextual metadata
Duplicate API calls MUST be avoided
Marker-based deduplication is required
Scripts MUST NOT assume write access
Permission failures MUST be handled gracefully
Files:
.github/scripts/bot-gfi-candidate-notification.js
🧬 Code graph analysis (1)
.github/scripts/bot-gfi-candidate-notification.js (1)
.github/scripts/pr_inactivity_reminder.js (2)
owner(77-77)repo(78-78)
🔇 Additional comments (5)
.github/scripts/bot-gfi-candidate-notification.js (5)
18-34: LGTM: Error handling follows best practices.The error handling correctly wraps the API call in try/catch, logs contextual information (issue number, error message), and returns a boolean status. This makes debugging straightforward and prevents unhandled promise rejections.
51-58: LGTM: Pagination and deduplication implemented correctly.The marker-based deduplication follows the required pattern from coding guidelines. Using
github.paginateis appropriate here since issue comments rarely reach problematic volumes, and thesome()check short-circuits efficiently on finding an existing marker.
41-48: LGTM: Payload validation is thorough and safe.The validation correctly uses optional chaining to safely access payload fields and returns early for invalid data. The label check is case-sensitive and exact-match, which prevents unintended triggers.
As per coding guidelines, all
context.payloadfields are properly validated before use.
36-74: Implement dry-run support in the script.To support the workflow's dry-run mode, the script must check
process.env.DRY_RUNand skip comment posting when enabled, while still logging the intended action.🔎 Proposed implementation
module.exports = async ({ github, context }) => { try { + const dryRun = process.env.DRY_RUN === 'true'; + if (dryRun) { + console.log('🔍 DRY RUN MODE: No comments will be posted'); + } + const { owner, repo } = context.repo; const { issue, label } = context.payload; if (!issue?.number || !label?.name) { return console.log('Missing issue or label in payload'); } // Only handle Good First Issue Candidate if (label.name !== 'good first issue candidate') { return; } // Prevent duplicate notifications const comments = await github.paginate( github.rest.issues.listComments, { owner, repo, issue_number: issue.number, per_page: 100 } ); if (comments.some(c => c.body?.includes(marker))) { return console.log(`Notification already exists for #${issue.number}`); } const message = 'A new Good First Issue Candidate has been created. Please review it and confirm whether it should be labeled as a Good First Issue.'; - const success = await notifyTeam(github, owner, repo, issue, message); + if (dryRun) { + console.log(`[DRY RUN] Would notify team about issue #${issue.number}`); + console.log(`[DRY RUN] Repository: ${owner}/${repo}`); + console.log(`[DRY RUN] Label: ${label.name}`); + return; + } + + const success = await notifyTeam(github, owner, repo, issue, message); if (success) { console.log('=== Summary ==='); console.log(`Repository: ${owner}/${repo}`); console.log(`Issue Number: ${issue.number}`); console.log(`Label: ${label.name}`); } } catch (err) { console.log('❌ Error:', err.message); } };As per coding guidelines, dry-run behavior must be explicit and visible in logs.
Likely an incorrect or invalid review comment.
4-4: The GitHub teamhiero-ledger/hiero-sdk-good-first-issue-supportdoes not exist. The @ mention in the notification will be posted without error, but GitHub will not notify the non-existent team. Create this team in the organization before deploying this script, or update theTEAM_ALIASconstant to reference an existing team.
|
Hi, this is MergeConflictBot. Please resolve these conflicts locally and push the changes. To assist you, please read: Thank you for contributing! |
parvninama
force-pushed
the
feat/gfi-candidate
branch
from
bc24dd4 to
1bb5e58
Compare
|
All checks are now passing — happy to address any feedback . |
exploreriii
left a comment
exploreriii
left a comment
There was a problem hiding this comment.
Hi @parvninama
On your fork, please do aim to generate a comment too 👍
Just pointing out an error picked up in one of the logs
https://github.com/parvninama/hiero-sdk-python/actions/runs/20693552576/job/59405228725
https://github.com/parvninama/hiero-sdk-python/actions/runs/20693509974/job/59405118571
Thank you
aceppaluni
left a comment
aceppaluni
left a comment
There was a problem hiding this comment.
@parvninama This is looking super! 👍
Could you take a look at your changelog? There seem to be a few things left over from a rebase.
If you need assistance please let us know!
Thank you!!
aceppaluni
left a comment
aceppaluni
left a comment
There was a problem hiding this comment.
@parvninama
This is really great work! 👍
Please take a look at https://github.com/hiero-ledger/hiero-sdk-python/pull/1342/checks?check_run_id=59482726440
A few commits were not signed properly and need to be corrected. I have attached our SIGNING.md file for assistance.
Please reach out if you need assistance with signing, we are happy to help assist.
|
Hi please let us know if you need help with the signing check 👍 |
typo: Corrected the typo correcting bugs typo:Changed some console logs feat: add notification workflow for Good First Issue candidates - Added workflow to notify the support team when a Good First Issue Candidate is created - Fixed typos and minor console log issues - Corrected small bugs in the notification script Signed-off-by: Parv <[email protected]>
Signed-off-by: Parv <[email protected]>
Signed-off-by: Parv <[email protected]>
Signed-off-by: Parv <[email protected]>
parvninama
force-pushed
the
feat/gfi-candidate
branch
3 times, most recently
from
870e902 to
296739b
Compare
Signed-off-by: Parv Ninama <[email protected]>
parvninama
force-pushed
the
feat/gfi-candidate
branch
from
296739b to
b35f9c1
Compare
|
Hi @exploreriii @aceppaluni |
|
Also , I noticed there are two Discord links mentioned in the README. Thanks! |
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (3)
.github/scripts/bot-gfi-candidate-notification.js.github/workflows/bot-gfi-candidate-notification.yamlCHANGELOG.md
🧰 Additional context used
📓 Path-based instructions (2)
.github/scripts/**/*.js
⚙️ CodeRabbit configuration file
.github/scripts/**/*.js: Review JavaScript scripts as long-lived automation code.Scripts must remain:
Focused
Readable
Purpose-built
All
context.payloadfields MUST be validatedFree-form text MUST NOT be trusted
Dynamic code execution is prohibited
Avoid
child_process.exec; preferexecFileif neededAll async operations MUST be wrapped in try/catch
Errors MUST include contextual metadata
Duplicate API calls MUST be avoided
Marker-based deduplication is required
Scripts MUST NOT assume write access
Permission failures MUST be handled gracefully
Files:
.github/scripts/bot-gfi-candidate-notification.js
.github/workflows/**/*
⚙️ CodeRabbit configuration file
.github/workflows/**/*: Review workflows as security-sensitive infrastructure.A good workflow is small, focused, and boring.
If a workflow is clever, generic, or overly flexible, it is a risk.
PRIORITY 0 — ABSOLUTE REQUIREMENTS
- All third-party actions MUST be pinned to full commit SHAs, similar to other workflows.
permissions:MUST be explicitly declared and minimally scoped.- Workflows MUST behave safely when executed from forks.
- YAML MUST orchestrate steps, not implement business logic.
- Any workflow that mutates GitHub state MUST support dry-run mode.
- Dry-run behavior must be explicit and visible in logs.
- Workflows MUST NOT modify repository source code outside
.github/.
PRIORITY 1 — SCOPE, FOCUS & RESTRAINT
- The title of each workflow must be relevant, match similar naming schemes, and match its script filename.
- Each workflow MUST have a single, clearly defined objective and SHOULD document this in a top-level comment.
- Flag workflows that:
- Attempt to be generic “frameworks”
- Include speculative or future-facing logic
- Perform actions unrelated to the stated goal
- Over-abstraction and excess flexibility are maintenance risks.
PRIORITY 2 — INPUT HARDENING
- Treat ALL GitHub event data as potentially hostile input, including:
- issue titles, bodies, and comments
- labels, usernames, branch names
- Free-form user input MUST NOT be passed directly into:
- shell commands
- gh CLI arguments
- Node.js exec / spawn calls
- Require strict allowlists or exact string matches.
- Flag any use of:
- eval or bash -c
- backticks or $(...) with user-controlled input
------------------...
Files:
.github/workflows/bot-gfi-candidate-notification.yaml
🧬 Code graph analysis (1)
.github/scripts/bot-gfi-candidate-notification.js (1)
.github/scripts/pr_inactivity_reminder.js (2)
owner(77-77)repo(78-78)
🪛 LanguageTool
CHANGELOG.md
[grammar] ~13-~13: Ensure spelling is correct
Context: ... and updated GFI and GFIC templates and documentation - Enable auto assignment to good first iss...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
[grammar] ~14-~14: Use a hyphen to join words.
Context: ...emplates and documentation - Enable auto assignment to good first issues (#1312),...
(QB_NEW_EN_HYPHEN)
[grammar] ~15-~15: Ensure spelling is correct
Context: ...ssign instruction. - Intermediate issue documentation - Added unit test for 'endpoint.py' to inc...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Codacy Static Code Analysis
🔇 Additional comments (5)
CHANGELOG.md (2)
11-11: Add space before the issue reference.The changelog entry text was improved based on previous feedback—good work! However, there's a minor formatting inconsistency: add a space before the issue reference to match the style used elsewhere in the changelog (e.g., line 21).
Proposed fix
-- Added a notification workflow that alerts the support team when an issue is labeled as a Good First Issue Candidate.[(#1296)] +- Added a notification workflow that alerts the support team when an issue is labeled as a Good First Issue Candidate. [(#1296)]
18-21: LGTM!The Hbar-related changelog entries are well-structured with clear sub-bullets documenting each aspect of the changes (parameter acceptance, unit tests, and module-level docstring).
.github/workflows/bot-gfi-candidate-notification.yaml (1)
29-29: Nice addition of version comments.The version annotations
#v6.0.1and#v8.0.0directly address the feedback from exploreriii, making it easier for future maintainers to understand which versions these SHAs represent.Also applies to: 35-35
.github/scripts/bot-gfi-candidate-notification.js (2)
45-71: Excellent payload validation and duplicate prevention.The implementation properly:
- Validates
context.payloadfields before use (lines 51-53)- Performs case-insensitive label matching (line 56)
- Uses marker-based deduplication with paginated comment fetch (lines 64-70)
- Logs dry-run mode activation (line 61)
All free-form text (issue title, label name) is safely used only in logs and comment bodies, never in dangerous contexts like shell execution.
As per coding guidelines, the script correctly implements all PRIORITY 6 (concurrency & idempotency) and input validation requirements.
64-67: Pagination lacks explicit upper bound.While
github.paginateis appropriate for ensuring complete duplicate detection, the coding guidelines (PRIORITY 5) recommend explicit upper bounds on pagination. For issue comments, this is generally acceptable since:
- Most issues have fewer than 1,000 comments
- GitHub's rate limits provide implicit protection
- Complete comment history is necessary for accurate marker detection
However, consider adding a safeguard for pathological cases:
♻️ Optional: Add pagination limit
// Prevent duplicate notifications + const MAX_COMMENTS_TO_CHECK = 1000; const comments = await github.paginate( github.rest.issues.listComments, - { owner, repo, issue_number: issue.number, per_page: 100 } + { owner, repo, issue_number: issue.number, per_page: 100 }, + (response, done) => { + if (response.data.length >= MAX_COMMENTS_TO_CHECK) done(); + return response.data; + } );As per coding guidelines PRIORITY 5, this adds defense against unexpectedly large comment threads.
Likely an incorrect or invalid review comment.
|
Hi @parvninama the discord is explained here: docs/discord.md |
Codecov Report✅ All modified and coverable lines are covered by tests. @@ Coverage Diff @@
## main #1342 +/- ##
=======================================
Coverage 92.44% 92.44%
=======================================
Files 139 139
Lines 8528 8528
=======================================
Hits 7884 7884
Misses 644 644 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
CHANGELOG.md
🧰 Additional context used
🪛 LanguageTool
CHANGELOG.md
[grammar] ~13-~13: Ensure spelling is correct
Context: ... and updated GFI and GFIC templates and documentation - Enable auto assignment to good first iss...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
[grammar] ~14-~14: Use a hyphen to join words.
Context: ...emplates and documentation - Enable auto assignment to good first issues (#1312),...
(QB_NEW_EN_HYPHEN)
[grammar] ~15-~15: Ensure spelling is correct
Context: ...ssign instruction. - Intermediate issue documentation - Added unit test for 'endpoint.py' to inc...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Codacy Static Code Analysis
|
Thank you for the clarification on the Discord links—I'll check out the documentation now and join the server. |
3 participants
PR Summary :
Closes[(#1296)]