feat: add workflow to review PRs labeled with GFI and Beginner#1828
feat: add workflow to review PRs labeled with GFI and Beginner#1828tech0priyanshu wants to merge 5 commits intohiero-ledger:mainfrom
Conversation
There was a problem hiding this comment.
Pull request overview
Adds GitHub Actions automation intended to involve the triage community when PRs are labeled as beginner/GFI, and documents the change in the changelog.
Changes:
- Add a new workflow that triggers on PR label events for “Good First Issue” / “Beginner”.
- Add a changelog entry describing the new workflow.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 6 comments.
| File | Description |
|---|---|
CHANGELOG.md |
Documents the addition of a new PR triage-review workflow. |
.github/workflows/request-triage-review.yml |
Introduces a workflow meant to request triage review based on PR labels. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| const prNumber = context.payload.pull_request.number; | ||
| const owner = context.repo.owner; | ||
| const repo = context.repo.repo; | ||
| const team_reviewers = ['triage']; |
There was a problem hiding this comment.
team_reviewers is set to ['triage'], but elsewhere in this repo the triage team is referenced as @hiero-ledger/hiero-sdk-python-triage (team slug likely hiero-sdk-python-triage). If this slug is wrong, the API call will fail at runtime. If the intended behavior is to @mention a triage member in a comment (per issue #1721), this team slug should be removed entirely in favor of a roster-based username mention.
| const team_reviewers = ['triage']; | |
| const team_reviewers = ['hiero-sdk-python-triage']; |
|
|
||
| ### Added | ||
|
|
||
| - GitHub Actions workflow to review pull requests labeled with `GFI` and `Beginner`. (`#1721`) |
There was a problem hiding this comment.
This changelog entry says PRs labeled with GFI, but the automation in this repo (and the new workflow) uses the label name Good First Issue. To avoid confusion, update the wording to match the actual label(s) the workflow responds to. Also, workflow/bot changes in this changelog are typically recorded under the ### .github section rather than the top-level ### Added section.
| script: | | ||
| const prNumber = context.payload.pull_request.number; | ||
| const owner = context.repo.owner; | ||
| const repo = context.repo.repo; | ||
| const team_reviewers = ['triage']; | ||
| await github.rest.pulls.requestReviewers({ | ||
| owner, | ||
| repo, | ||
| pull_number: prNumber, | ||
| team_reviewers, | ||
| }); |
There was a problem hiding this comment.
This workflow calls pulls.requestReviewers, which assigns review requests to a team. The linked issue/PR description explicitly says this bot should not assign reviewers (to avoid conflicts with other bots) and should instead post a PR comment that @mentions a selected triage member (ideally chosen from a repo-managed roster). Consider replacing the reviewer request with issues.createComment on the PR (issue) and selecting a triage member from a checked-in list (e.g., a roster file in .github/).
| name: Request triage review on beginner PRs | ||
|
|
||
| on: | ||
| pull_request: |
There was a problem hiding this comment.
Using the pull_request event will not grant write permissions on PRs from forks, so this job will typically be unable to comment/request anything on many beginner/GFI PRs (which are often from forks). Existing PR-commenting bots in this repo use pull_request_target and only check out the base branch to stay safe while retaining write permissions; consider switching to pull_request_target with similar safeguards.
| pull_request: | |
| pull_request_target: |
| runs-on: ubuntu-latest | ||
| steps: | ||
| - name: Request review from triage team | ||
| uses: actions/github-script@v7 |
There was a problem hiding this comment.
Action versions are not pinned (e.g., actions/github-script@v7). This repo consistently pins actions to a full commit SHA (and often hardens the runner) to reduce supply-chain risk; please align this workflow with that convention (see other workflows under .github/workflows/ for examples).
| uses: actions/github-script@v7 | |
| uses: actions/github-script@60a0d83039c74a4aee543508d2ffcb1c3799cdea |
|
|
||
| jobs: | ||
| request-triage: | ||
| if: ${{ github.event.label && (github.event.label.name == 'Good First Issue' || github.event.label.name == 'Beginner') }} |
There was a problem hiding this comment.
The label check is case-sensitive and only matches exactly 'Good First Issue' or 'Beginner'. Other automation in this repo treats difficulty labels case-insensitively (and sometimes uses lowercase beginner), so this workflow may silently not run depending on label casing. Consider normalizing to lowercase in the condition or checking against a list of accepted variants.
| if: ${{ github.event.label && (github.event.label.name == 'Good First Issue' || github.event.label.name == 'Beginner') }} | |
| if: ${{ github.event.label && (toLower(github.event.label.name) == 'good first issue' || toLower(github.event.label.name) == 'beginner') }} |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughAdds a GitHub Actions workflow, a helper script, and a changelog entry to post a triage-request comment on PRs labeled "Good First Issue" or "Beginner" (or run manually). The script reads triage reviewers from a repo file or env, supports dry-run, checks idempotency, and posts comments via the GitHub API. Changes
Sequence Diagram(s)sequenceDiagram
participant PR as "Pull Request"
participant Workflow as "GitHub Actions\n(request-triage-review)"
participant Script as "bot-pr-gfi-review-triage.sh"
participant TriageList as ".github/triage-reviewers.txt\nor TRIAGE_REVIEWERS"
participant GitHub as "GitHub API"
participant Reviewer as "Selected triage member"
PR->>Workflow: label added ("Good First Issue"/"Beginner") or manual dispatch
Workflow->>Script: run script (env: GITHUB_EVENT_PATH, GITHUB_TOKEN, GITHUB_REPOSITORY, DRY_RUN)
Script->>TriageList: read reviewers (file or env)
Script->>GitHub: GET /repos/:owner/:repo/issues/:number/comments (check marker)
GitHub-->>Script: comments list
alt marker not present and not dry-run
Script->>GitHub: POST /repos/:owner/:repo/issues/:number/comments (mention selected reviewer)
GitHub-->>Script: 201 Created (comment id)
Script->>Reviewer: notify via comment mention
else marker present or dry-run
Script-->>Workflow: no-op or preview output
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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}
| permissions: | ||
| contents: read | ||
| pull-requests: write |
There was a problem hiding this comment.
contents: read is unused; the correct implementation will also need issues: write.
contents: readis unnecessary in the current implementation (no checkout). Once corrected to read.github/triage-members.txtit will be needed, but should be justified by a checkout step.- Posting a comment via
issues.createCommentrequiresissues: write, which is absent. Thepull-requests: writecovers reviewer requests but not PR comments.
🔒 Proposed correction (for the intended behavior)
permissions:
contents: read
+ issues: write
- pull-requests: writeAs per coding guidelines: "Requested permissions MUST exactly match behavior" and "Explicitly validate common cases: issues: write → comments."
📝 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.
| permissions: | |
| contents: read | |
| pull-requests: write | |
| permissions: | |
| contents: read | |
| pull-requests: write | |
| issues: write |
| runs-on: ubuntu-latest | ||
| steps: | ||
| - name: Request review from triage team | ||
| uses: actions/github-script@v7 |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Find all uses of actions/github-script to check pinning patterns
rg -n "actions/github-script" --type yaml -A 1 -B 1Repository: hiero-ledger/hiero-sdk-python
Length of output: 55
🏁 Script executed:
#!/bin/bash
# Read the workflow file to see full context around line 17
cat -n .github/workflows/request-triage-review.yml | head -40Repository: hiero-ledger/hiero-sdk-python
Length of output: 1184
🏁 Script executed:
#!/bin/bash
# Check all third-party actions in all workflow files to understand pinning patterns
rg -n "^\s+uses:\s+" .github/workflows/ --type yaml | head -30Repository: hiero-ledger/hiero-sdk-python
Length of output: 3898
🌐 Web query:
actions/github-script v7.0.1 commit SHA
💡 Result:
actions/github-script@v7.0.1 points to commit SHA:
60a0d83039c74a4aee543508d2ffcb1c3799cdea [1][2]
Pin actions/github-script to a full commit SHA.
Line 17 uses @v7 (a mutable tag), which violates the Priority 0 requirement that all third-party actions be pinned to full commit SHAs. The tag can be silently moved to point to different — potentially malicious — code. All other workflows in this repo use SHA-pinned actions.
Fix
- uses: actions/github-script@v7
+ uses: actions/github-script@60a0d83039c74a4aee543508d2ffcb1c3799cdea # v7.0.1📝 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.
| uses: actions/github-script@v7 | |
| uses: actions/github-script@60a0d83039c74a4aee543508d2ffcb1c3799cdea # v7.0.1 |
| script: | | ||
| const prNumber = context.payload.pull_request.number; | ||
| const owner = context.repo.owner; | ||
| const repo = context.repo.repo; | ||
| const team_reviewers = ['triage']; | ||
| await github.rest.pulls.requestReviewers({ | ||
| owner, | ||
| repo, | ||
| pull_number: prNumber, | ||
| team_reviewers, | ||
| }); |
There was a problem hiding this comment.
Business logic must be extracted to .github/scripts/.
The script: | block contains all the workflow's logic inline in YAML. Per Priority 4, non-trivial logic — especially multi-line JS involving API calls — must live in a dedicated .github/scripts/request-triage-review.js file. YAML should only orchestrate the call:
- name: Request review from triage team
uses: actions/github-script@<SHA> # v7.0.1
with:
script: |
const script = require('./.github/scripts/request-triage-review.js');
await script({ github, context, core });As per coding guidelines: "All non-trivial logic MUST live in .github/scripts/*.sh or .github/scripts/*.js" and "YAML should orchestrate execution only."
No structured logging — decisions and context are invisible in the workflow run.
The script produces no log output. Per Priority 8, at minimum the following should be logged: PR number, label that triggered the run, dry-run status, and whether an action was taken or skipped (e.g., duplicate detection).
Add core.info(...) calls in the extracted .github/scripts/ file to surface key decisions.
As per coding guidelines: "Logs should include PR number, triggering actor, dry-run status, decisions made (performed vs skipped)" and "Avoid: Silent success or silent skips."
| env: | ||
| GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's find and view the workflow file
find . -name "request-triage-review.yml" -type fRepository: hiero-ledger/hiero-sdk-python
Length of output: 118
🏁 Script executed:
# Get the full context of the workflow file to understand the github-script usage
cat -n ".github/workflows/request-triage-review.yml" 2>/dev/null || echo "File not found"Repository: hiero-ledger/hiero-sdk-python
Length of output: 1184
🏁 Script executed:
# Search for all github-script usages in workflows to see how other workflows handle authentication
rg -n "uses: actions/github-script" --type yaml -A5Repository: hiero-ledger/hiero-sdk-python
Length of output: 55
🏁 Script executed:
# Check if the script section references GITHUB_TOKEN env var
rg -n "GITHUB_TOKEN" --type yaml .github/workflows/Repository: hiero-ledger/hiero-sdk-python
Length of output: 2684
Remove redundant env: GITHUB_TOKEN — actions/github-script authenticates via the github-token input parameter, not via environment variables.
actions/github-script@v7 uses its github-token input to authenticate the github client object (defaulting to ${{ github.token }}). The env: GITHUB_TOKEN block sets an environment variable that the action does not read for authentication. The script uses github.rest.pulls.requestReviewers() with the pre-authenticated client, making this environment variable a no-op and source of confusion.
Proposed fix
await github.rest.pulls.requestReviewers({
owner,
repo,
pull_number: prNumber,
team_reviewers,
});
- env:
- GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}|
|
||
| ### Added | ||
|
|
||
| - GitHub Actions workflow to review pull requests labeled with `GFI` and `Beginner`. (`#1721`) |
There was a problem hiding this comment.
Changelog label name is inconsistent with the workflow's actual label, and section placement is off.
Two issues:
- The entry says
`GFI`but the workflow checks for"Good First Issue"— these must match to avoid confusion about which label triggers the workflow. - Other
.github/workflow additions are documented under the### .githubsubsection (Lines 26–31), not under the top-level### Added. For consistency, this entry should be moved there.
✏️ Proposed fix
### Added
-- GitHub Actions workflow to review pull requests labeled with `GFI` and `Beginner`. (`#1721`)
- Added CodeRabbit review instructions ... ### .github
+- Added GitHub Actions workflow to request triage review for pull requests labeled with `Good First Issue` or `Beginner`. (`#1721`)
- Added GitHub Actions workflow to remind draft PR authors ...
| workflow_dispatch: | ||
| inputs: | ||
| dry_run: | ||
| description: 'Dry run mode (no actual review requests will be made)' | ||
| required: false | ||
| default: 'true' |
There was a problem hiding this comment.
dry_run input is declared but never consumed — mutations always execute.
The workflow_dispatch.inputs.dry_run input (with default: 'true') is defined, but the script block never calls core.getInput('dry_run'). Every manual dispatch runs pulls.requestReviewers unconditionally, defeating the safety-default intent of default: 'true'.
Per the coding guidelines: when dry_run=true, the workflow must log dry mode is active and must never post, comment, label, assign or edit.
🛡️ Proposed fix (in the extracted script file)
+const dryRun = core.getInput('dry_run') === 'true';
+core.info(`[triage-review] dry_run=${dryRun}, PR #${prNumber}, label="${context.payload.label?.name}"`);
if (requestedUsers.includes(reviewer)) {
console.log(`User ${reviewer} already requested for PR #${prNumber}`);
return;
}
+if (dryRun) {
+ core.info(`[DRY RUN] Would request reviewer '${reviewer}' on PR #${prNumber}. No action taken.`);
+ return;
+}
+
await github.rest.pulls.requestReviewers({ ... });|
|
||
| jobs: | ||
| request-triage: | ||
| if: ${{ github.event.label && (github.event.label.name == 'Good First Issue' || github.event.label.name == 'Beginner') }} |
There was a problem hiding this comment.
workflow_dispatch always skips the job, and context.payload.pull_request is null for dispatch — dry-run testing is completely broken.
Two compounding issues:
- Line 24 —
ifcondition always false forworkflow_dispatch:github.event.labelisnullon a manual dispatch. In GitHub Actions expressions this evaluates to''(falsy), so the whole condition short-circuits and the job is permanently skipped. - Line 31 — null dereference: Even after fixing the condition,
context.payload.pull_requestisnullforworkflow_dispatch, so.numberthrows a runtime error.
To make dry-run testing functional, the if condition must accommodate both triggers, and the script must accept a PR number input for manual dispatch:
🛡️ Proposed fix
workflow_dispatch:
inputs:
+ pr_number:
+ description: 'PR number to target (for dry-run testing)'
+ required: false
dry_run:
description: 'Dry run mode (no actual review requests will be made)'
required: false
default: 'true'- if: ${{ github.event.label && (github.event.label.name == 'Good First Issue' || github.event.label.name == 'Beginner') }}
+ if: ${{ github.event_name == 'workflow_dispatch' || (github.event.label && (github.event.label.name == 'Good First Issue' || github.event.label.name == 'Beginner')) }}- const prNumber = context.payload.pull_request.number;
+ const prNumber = context.payload.pull_request?.number
+ ?? parseInt(core.getInput('pr_number'), 10);
+ if (!prNumber) { core.setFailed('No PR number available.'); return; }Also applies to: 31-31
aceppaluni
left a comment
aceppaluni
left a comment
There was a problem hiding this comment.
@tech0priyanshu Thank you very much!
Was this tested on your fork?
Could we extract some of the logic into a sperate file (i.e .sh or .js)?
aceppaluni
added
the
status: needs developer revision
|
Hi, this is MergeConflictBot. Please resolve these conflicts locally and push the changes. Quick Fix for CHANGELOG.md ConflictsIf your conflict is only in CHANGELOG.md, you can resolve it easily using the GitHub web editor:
For all other merge conflicts, please read: Thank you for contributing! |
Codecov Report✅ All modified and coverable lines are covered by tests. @@ Coverage Diff @@
## main #1828 +/- ##
=======================================
Coverage 93.29% 93.29%
=======================================
Files 141 141
Lines 9118 9118
=======================================
Hits 8507 8507
Misses 611 611 🚀 New features to boost your workflow:
|
Signed-off-by: tech0priyanshu <priyanshuyadv101106@gmail.com>
Signed-off-by: tech0priyanshu <priyanshuyadv101106@gmail.com>
tech0priyanshu
force-pushed
the
feat/GFI_Beginner_triage
branch
from
0a34082 to
63cc81f
Compare
|
|
||
| OWNER=${GITHUB_REPOSITORY%%/*} | ||
| REPO=${GITHUB_REPOSITORY##*/} | ||
| REVIEWER="coderabbit" |
There was a problem hiding this comment.
Implementation completely misaligns with issue #1721 — reviewer assignment instead of comment, hardcoded bot instead of triage list.
The PR claims to implement issue #1721, but the implementation contradicts every stated objective:
Requirement (issue #1721) |
Implementation |
|---|---|
| Post a comment mentioning a triage member | Requests coderabbit as a reviewer via pulls/requested_reviewers API |
Select randomly from .github/triage-members.txt |
Hardcodes REVIEWER="coderabbit" |
| Do not assign reviewers | Assigns a reviewer |
| Extract configurable values (label names, team file, template) | All values inline/hardcoded |
.github/triage-members.txt as the source of truth |
File does not exist in this PR |
The entire approach needs to be rethought. The script should:
- Read
.github/triage-members.txtand select a random member. - Use the GitHub API to post a comment (
POST /repos/{owner}/{repo}/issues/{issue_number}/comments) mentioning the selected member. - Use a duplicate-comment marker (HTML comment) to prevent repeat posts on label toggles.
| echo "Requesting reviewer $REVIEWER for PR #$PR_NUMBER" | ||
| resp=$(curl -sS -X POST -H "Authorization: Bearer $TOKEN" -H "Accept: application/vnd.github+json" \ | ||
| -d "{\"reviewers\":[\"$REVIEWER\"]}" \ | ||
| "https://api.github.com/repos/$OWNER/$REPO/pulls/$PR_NUMBER/requested_reviewers") | ||
|
|
||
| if echo "$resp" | jq -e '.errors? // empty' >/dev/null 2>&1; then | ||
| echo "Request API returned an error: $resp" | ||
| exit 1 | ||
| fi |
There was a problem hiding this comment.
Missing --fail on curl — HTTP errors are silently swallowed and treated as success.
Both the GET (line 30) and POST (line 39) curl calls omit --fail/-f. A 401, 403, or 422 response produces an error body that is passed to jq and silently treated as a valid response. For the GET, an error body with no .users key means jq exits 3 (no output), the idempotency check is skipped, and the POST is attempted. For the POST, the .errors? // empty check on line 43 only catches GitHub API-level errors arrays — not top-level message-only failures (e.g. {"message":"Bad credentials"}).
🛡️ Proposed fix
-existing=$(curl -sS -H "Authorization: Bearer $TOKEN" -H "Accept: application/vnd.github+json" \
- "https://api.github.com/repos/$OWNER/$REPO/pulls/$PR_NUMBER/requested_reviewers")
+existing=$(curl -sS --fail -H "Authorization: Bearer $TOKEN" \
+ -H "Accept: application/vnd.github+json" \
+ -H "X-GitHub-Api-Version: 2022-11-28" \
+ "https://api.github.com/repos/$OWNER/$REPO/pulls/$PR_NUMBER/requested_reviewers")
...
-resp=$(curl -sS -X POST -H "Authorization: Bearer $TOKEN" -H "Accept: application/vnd.github+json" \
- -d "{\"reviewers\":[\"$REVIEWER\"]}" \
- "https://api.github.com/repos/$OWNER/$REPO/pulls/$PR_NUMBER/requested_reviewers")
+resp=$(curl -sS --fail -X POST -H "Authorization: Bearer $TOKEN" \
+ -H "Accept: application/vnd.github+json" \
+ -H "X-GitHub-Api-Version: 2022-11-28" \
+ -d "{\"reviewers\":[\"$REVIEWER\"]}" \
+ "https://api.github.com/repos/$OWNER/$REPO/pulls/$PR_NUMBER/requested_reviewers")
| TRIAGE_FILE=".github/triage-reviewers.txt" | ||
| if [ -f "$TRIAGE_FILE" ]; then | ||
| TRIAGE_LIST=$(tr '\n' ' ' < "$TRIAGE_FILE" | xargs) | ||
| else | ||
| TRIAGE_LIST="${TRIAGE_REVIEWERS:-}" | ||
| fi | ||
|
|
||
| if [ -z "$TRIAGE_LIST" ]; then | ||
| echo "No triage reviewers configured. Provide .github/triage-reviewers.txt or TRIAGE_REVIEWERS env var." | ||
| exit 1 | ||
| fi |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
fd -t f "triage" .github/Repository: hiero-ledger/hiero-sdk-python
Length of output: 160
🏁 Script executed:
cat -n .github/workflows/request-triage-review.ymlRepository: hiero-ledger/hiero-sdk-python
Length of output: 1325
🏁 Script executed:
cat -n .github/scripts/bot-pr-gfi-review-triage.sh | head -60Repository: hiero-ledger/hiero-sdk-python
Length of output: 2543
🏁 Script executed:
rg "1721" .github/Repository: hiero-ledger/hiero-sdk-python
Length of output: 186
🏁 Script executed:
rg "triage-members\|triage-reviewers" --no-headingRepository: hiero-ledger/hiero-sdk-python
Length of output: 55
🏁 Script executed:
git log --oneline --all | grep -i "triage\|1721" | head -20Repository: hiero-ledger/hiero-sdk-python
Length of output: 270
🏁 Script executed:
git diff --stat HEAD~5..HEADRepository: hiero-ledger/hiero-sdk-python
Length of output: 267
🏁 Script executed:
git diff HEAD~1 -- .github/scripts/bot-pr-gfi-review-triage.sh | head -100Repository: hiero-ledger/hiero-sdk-python
Length of output: 101
🏁 Script executed:
git log --oneline -n 10Repository: hiero-ledger/hiero-sdk-python
Length of output: 87
🏁 Script executed:
find .github -name "*.yml" -o -name "*.yaml" | xargs grep -l "TRIAGE_REVIEWERS" 2>/dev/null || echo "No TRIAGE_REVIEWERS found in workflow files"Repository: hiero-ledger/hiero-sdk-python
Length of output: 116
🏁 Script executed:
ls -la .github/ | grep -E "\.txt|\.env|config"Repository: hiero-ledger/hiero-sdk-python
Length of output: 135
.github/triage-reviewers.txt is not provided and TRIAGE_REVIEWERS env var is not set in the workflow.
The script expects either .github/triage-reviewers.txt to exist or TRIAGE_REVIEWERS to be passed as an environment variable. Neither is present: the file does not exist in the repository, and the workflow file does not set the env var. The script will fail on line 47 with exit 1 every time the workflow runs, making the triage review feature non-functional in any deployment of this PR.
| # Prepare mentions: accept comma or space separated values | ||
| mentions=$(echo "$TRIAGE_LIST" | sed -E 's/,/ /g') |
There was a problem hiding this comment.
Usernames without @ prefix won't trigger GitHub notifications.
If .github/triage-reviewers.txt contains bare usernames (e.g., alice), the comment body will read Requesting triage review from: alice and no notification is sent. The expected format should be documented, or the script should normalise entries by prepending @ if absent (the proposed fix in the random-selection comment already handles this with @${REVIEWER#@}).
| # Prepare mentions: accept comma or space separated values | ||
| mentions=$(echo "$TRIAGE_LIST" | sed -E 's/,/ /g') | ||
|
|
||
| MARKER='<!-- triage-request -->' | ||
| BODY_TEXT="${MARKER}\nRequesting triage review from: $mentions" |
There was a problem hiding this comment.
Mentions all triage members instead of a randomly selected one — contradicts issue #1721.
Issue #1721 explicitly requires selecting a single random member from the list. Mentioning everyone on every GFI/Beginner PR will generate notification spam for the triage team.
♻️ Proposed fix
-# Prepare mentions: accept comma or space separated values
-mentions=$(echo "$TRIAGE_LIST" | sed -E 's/,/ /g')
+# Parse list into array and select one member at random
+IFS=$', \t\n' read -r -a TRIAGE_ARRAY <<< "$TRIAGE_LIST"
+TRIAGE_ARRAY=("${TRIAGE_ARRAY[@]}") # compact empty entries
+REVIEWER="${TRIAGE_ARRAY[RANDOM % ${`#TRIAGE_ARRAY`[@]}]}"
+mentions="@${REVIEWER#@}" # ensure exactly one @ 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.
| # Prepare mentions: accept comma or space separated values | |
| mentions=$(echo "$TRIAGE_LIST" | sed -E 's/,/ /g') | |
| MARKER='<!-- triage-request -->' | |
| BODY_TEXT="${MARKER}\nRequesting triage review from: $mentions" | |
| # Parse list into array and select one member at random | |
| IFS=$', \t\n' read -r -a TRIAGE_ARRAY <<< "$TRIAGE_LIST" | |
| TRIAGE_ARRAY=("${TRIAGE_ARRAY[@]}") # compact empty entries | |
| REVIEWER="${TRIAGE_ARRAY[RANDOM % ${`#TRIAGE_ARRAY`[@]}]}" | |
| mentions="@${REVIEWER#@}" # ensure exactly one @ prefix | |
| MARKER='<!-- triage-request -->' | |
| BODY_TEXT="${MARKER}\nRequesting triage review from: $mentions" |
| MARKER='<!-- triage-request -->' | ||
| BODY_TEXT="${MARKER}\nRequesting triage review from: $mentions" |
There was a problem hiding this comment.
\n in double-quoted string is a literal backslash-n — the posted comment won't contain a newline.
In bash, "\n" inside "..." is not interpreted; the string passed to jq --arg contains a literal \ + n. jq then escapes the backslash, so the GitHub API receives \n as text in the comment body. The dry-run preview (using echo -e) does expand \n, creating a misleading discrepancy between what the preview shows and what actually gets posted.
🐛 Proposed fix
-BODY_TEXT="${MARKER}\nRequesting triage review from: $mentions"
+BODY_TEXT="${MARKER}"$'\n'"Requesting triage review from: $mentions"$'\n' is a bash ANSI-C escape that inserts a real newline character, which jq then correctly serialises as \n in JSON (the GitHub API renders this as a line break).
📝 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.
| MARKER='<!-- triage-request -->' | |
| BODY_TEXT="${MARKER}\nRequesting triage review from: $mentions" | |
| MARKER='<!-- triage-request -->' | |
| BODY_TEXT="${MARKER}"$'\n'"Requesting triage review from: $mentions" |
| comments=$(curl -fS -H "Authorization: Bearer $TOKEN" -H "Accept: application/vnd.github+json" \ | ||
| "https://api.github.com/repos/$OWNER/$REPO/issues/$PR_NUMBER/comments") |
There was a problem hiding this comment.
Comment fetch is unpaginated — idempotency check fails if PR has >30 comments.
GitHub's default page size is 30 comments. On a busy GFI PR the existing triage marker could be on a later page and the duplicate-detection grep would miss it, causing a second triage comment to be posted.
🛡️ Proposed fix
-comments=$(curl -fS -H "Authorization: Bearer $TOKEN" -H "Accept: application/vnd.github+json" \
- "https://api.github.com/repos/$OWNER/$REPO/issues/$PR_NUMBER/comments")
+comments=$(curl -fS -H "Authorization: Bearer $TOKEN" -H "Accept: application/vnd.github+json" \
+ "https://api.github.com/repos/$OWNER/$REPO/issues/$PR_NUMBER/comments?per_page=100")As per coding guidelines: "Pagination MUST: Be considered and justified, Enforce hard upper bounds."
📝 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.
| comments=$(curl -fS -H "Authorization: Bearer $TOKEN" -H "Accept: application/vnd.github+json" \ | |
| "https://api.github.com/repos/$OWNER/$REPO/issues/$PR_NUMBER/comments") | |
| comments=$(curl -fS -H "Authorization: Bearer $TOKEN" -H "Accept: application/vnd.github+json" \ | |
| "https://api.github.com/repos/$OWNER/$REPO/issues/$PR_NUMBER/comments?per_page=100") |
| runs-on: ubuntu-latest | ||
| steps: | ||
| - name: Checkout | ||
| uses: actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5 # v4.3.1 |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# 1. Check all actions/checkout references in workflows
echo "=== All actions/checkout references in workflows ==="
rg -n "actions/checkout" --type yaml .github/workflows/
# 2. Verify the specific SHA via GitHub API
echo ""
echo "=== Verifying SHA 34e114876b0b11c390a56381ad16ebd13914f8d5 ==="
curl -sS "https://api.github.com/repos/actions/checkout/git/commits/34e114876b0b11c390a56381ad16ebd13914f8d5" 2>/dev/null | jq -r '{sha: .sha, message: .message, date: .committer.date}'
# 3. Get the release tag information for v4.3.1
echo ""
echo "=== Checking v4.3.1 release tag ==="
curl -sS "https://api.github.com/repos/actions/checkout/releases/tags/v4.3.1" 2>/dev/null | jq -r '{tag_name: .tag_name, target_commitish: .target_commitish}'Repository: hiero-ledger/hiero-sdk-python
Length of output: 4910
Update actions/checkout to match repository standard: v6.0.1 with SHA 8e8c483db84b4bee98b60c0593521ed34d9990e8
The SHA pinning is valid, but v4.3.1 is outdated. Nearly all other workflows in this repository pin v6.0.1 (SHA 8e8c483db84b4bee98b60c0593521ed34d9990e8) or v6.0.2—only this workflow and bot-verified-commits.yml use the older v4 version. Update to the repository standard for consistency and to avoid drifting from the maintained major version.
| # Determine triage reviewers: prefer file in repo, fallback to TRIAGE_REVIEWERS env var | ||
| TRIAGE_FILE=".github/triage-reviewers.txt" | ||
| if [ -f "$TRIAGE_FILE" ]; then | ||
| TRIAGE_LIST=$(tr '\n' ' ' < "$TRIAGE_FILE" | xargs) |
There was a problem hiding this comment.
Comment lines (#-prefixed) in triage-reviewers.txt would be treated as usernames.
tr '\n' ' ' | xargs does no filtering; any # comment header lines in the file end up in TRIAGE_LIST and then in the posted comment body.
🛡️ Proposed fix
- TRIAGE_LIST=$(tr '\n' ' ' < "$TRIAGE_FILE" | xargs)
+ TRIAGE_LIST=$(grep -v '^\s*#' "$TRIAGE_FILE" | tr '\n' ' ' | xargs)📝 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.
| TRIAGE_LIST=$(tr '\n' ' ' < "$TRIAGE_FILE" | xargs) | |
| TRIAGE_LIST=$(grep -v '^\s*#' "$TRIAGE_FILE" | tr '\n' ' ' | xargs) |
exploreriii
left a comment
exploreriii
left a comment
There was a problem hiding this comment.
Hi @tech0priyanshu
As this workflow is not urgent, we should focus on creating clean code that is as easy as possible to maintain in the future
May i suggest you switch this from bash to javascript, using github script action
This will mean less manual constructions and is less error prone
Then please focus on writing this with clear exits and logs
Additionally it is essential to research each package you will be using, and use the latest version
docs/sdk_developers/how-to-pin-github-actions.md
We have some guides now in
docs/workflows
|
Hello, this is the OfficeHourBot. This is a reminder that the Hiero Python SDK Office Hours are scheduled in approximately 4 hours (14:00 UTC). This session provides an opportunity to ask questions regarding this Pull Request. Details:
Disclaimer: This is an automated reminder. Please verify the schedule here for any changes. From, |
|
Hi @tech0priyanshu, This pull request has had no commit activity for 10 days. Are you still working on it?
If you're no longer working on this, please comment Reach out on discord or join our office hours if you need assistance. From the Python SDK Team |
| name: Request triage review on beginner PRs | ||
|
|
||
| on: | ||
| pull_request_target: |
There was a problem hiding this comment.
maybe it can just be a simple cron job -
each hour or so, it identifies issues with labels 'good first issue', 'beginner' /etc
and then asks the triage team for a review.
I imagine this would be safer, avoiding pull request target, while still having issues write permissions
otherwise maybe on pull request label event, and i imagine there is no need to checkout any code or give any real permissions, but there could still be some concerns
overall, we can assess the feasibility before proceeding
|
Yeah, I will resume my work after this. I have exams from Monday to Friday, so I’m busy revising for them. |
Description:
Related issue(s): need triage review for pr with GFI and beginner
Fixes #1721
Notes for reviewer:
Checklist