feat: automated review queue label sync , Phase 1 (label sync only)#2242
Conversation
Introduce a cron-based workflow (every 30 min) that classifies every open non-draft PR into one of three review stages based on the number and permission level of approvals: queue:junior-committer → queue:committers → ready-to-merge Phase 1 of 4 — label sync only. No assignments, comments, or routing. Label determination: - writeApproval >= 2 → ready-to-merge - anyApproval >= 1 → queue:committers - else → queue:junior-committer Key design decisions: - Add-first-then-remove label ordering prevents zero-label state - DISMISSED reviews actively delete prior state (prevents ghost approvals) - Reviews explicitly sorted by submitted_at (correct by construction) - Rate-limit guard (floor=200) prevents partial runs - Concurrency block prevents overlapping cron runs from racing - DRY_RUN mode for safe manual testing via workflow_dispatch - 422/404 errors handled silently (race conditions, external users) - Old step:/reviewer: labels coexist intentionally until future cleanup - Bot-authored PRs (Dependabot) receive labels (acknowledged) Files added: .github/workflows/review-sync.yml .github/scripts/review-sync/index.js .github/scripts/review-sync/labels.js Signed-off-by: darshit2308 <darshit2308@gmail.com>
Up to standards ✅🟢 Issues
|
| Metric | Results |
|---|---|
| Complexity | 127 |
NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer
TIP This summary will be updated as you push new changes.
e79b9e5 to
6e67c49
Compare
WalkthroughAdds a stateless GitHub Actions cron workflow and Node.js scripts to compute and apply a single review-queue label per PR. Includes helpers to fetch/normalize reviews, resolve collaborator permissions, decide and reconcile queue labels, unit tests with mocks, and a scheduled workflow that runs the orchestration. ChangesReview Queue Label Sync System
Sequence DiagramsequenceDiagram
participant Cron as Cron / Manual Trigger
participant Workflow as GitHub Actions Workflow
participant Script as review-sync Script
participant API as GitHub API
participant Labels as Label Helpers
Cron->>Workflow: Trigger (30 min or manual)
Workflow->>Script: Execute with DRY_RUN env
Script->>API: Check rate limit (guard)
API-->>Script: Remaining quota
Script->>API: Fetch all open non-draft PRs (paginated)
API-->>Script: PR list
Script->>API: Ensure required queue labels exist
API-->>Script: Labels confirmed
loop For each PR
Script->>API: Fetch PR reviews (paginated)
API-->>Script: Latest review states per user
Script->>Labels: determineLabel(approvals)
Labels-->>Script: Target queue label
Script->>Labels: syncLabel(currentLabels -> targetLabel)
Labels->>API: Add target label first (crash-safe)
API-->>Labels: Label added
Labels->>API: Remove stale queue labels
API-->>Labels: Stale labels removed
Labels-->>Script: Change applied or skipped
end
Script-->>Workflow: Summary (PRs processed, changed, skipped, errors)
Workflow->>Cron: Complete
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 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)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. 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 |
There was a problem hiding this comment.
Pull request overview
This PR introduces a new scheduled GitHub Actions workflow plus supporting Node.js scripts to keep PR “review queue” labels synchronized on all open, non-draft pull requests based on current approval state (Phase 1: label sync only).
Changes:
- Added a 30-minute cron + manual dispatch workflow to run the review-queue label sync with a
dry_runtoggle. - Added an entry-point script that rate-limit guards, enumerates PRs, ensures labels exist, and processes PRs with per-PR error isolation + summary output.
- Added core labeling logic to compute approval counts (including DISMISSED “ghost approval” handling) and apply queue labels in an add-first-then-remove order.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
.github/workflows/review-sync.yml |
New scheduled workflow that checks out the scripts and runs the sync via actions/github-script. |
.github/scripts/review-sync/index.js |
Orchestrates rate-limit check, PR pagination, label bootstrap, per-PR processing, and run summary. |
.github/scripts/review-sync/labels.js |
Computes latest review state, counts approvals by permission level, determines the correct label, and applies labels. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Move early return condition to after stale label determination to ensure stale labels are always removed - Add 'maintain' role to privileged approval permission levels - Downgrade pull-requests permission to 'read' to enforce least privilege Signed-off-by: darshit2308 <darshit2308@gmail.com>
7e0a6b7 to
222e06b
Compare
Signed-off-by: darshit2308 <darshit2308@gmail.com>
exploreriii
left a comment
There was a problem hiding this comment.
Main issue i see is 2 write approvals might not be enoguh to merge given code owners
|
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, |
|
Yep some of my feedback was out of scope for this PR, but can be added as notes for any future one |
Addresses maintainer feedback on PR hiero-ledger#2242: - Refactors monolithic labels.js into helpers/ structure (constants, permissions, reviews, labels) matching hiero-sdk-cpp pattern - Adds queue:maintainers label for a 4-stage pipeline: queue:junior-committer → queue:committers → queue:maintainers → ready-to-merge - Gates ready-to-merge on at least 1 maintainer (admin/maintain) approval to satisfy CODEOWNERS requirements - Uses role_name (not legacy permission field) to correctly detect maintain role — fixes critical bug where maintain maps to write - Adds comprehensive unit tests (37 tests) using inline mock pattern from hiero-sdk-cpp test-utils Signed-off-by: darshit2308 <darshit2308@gmail.com>
|
Hi @exploreriii, I have pushed the updates to address your feedback! Summary of changes in this iteration:
For the CI status check and branch freshness checks, I've left those for Phase 2 and Phase 3 as discussed, so we can keep the scope of this Phase 1 PR strictly to the label sync refactor. Let me know, if there is some issue |
There was a problem hiding this comment.
Actionable comments posted: 3
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 24244e29-9480-4592-b89b-aeb6d2526cbb
📒 Files selected for processing (11)
.github/scripts/review-sync/helpers/constants.js.github/scripts/review-sync/helpers/index.js.github/scripts/review-sync/helpers/labels.js.github/scripts/review-sync/helpers/permissions.js.github/scripts/review-sync/helpers/reviews.js.github/scripts/review-sync/index.js.github/scripts/review-sync/tests/test-labels.js.github/scripts/review-sync/tests/test-permissions.js.github/scripts/review-sync/tests/test-reviews.js.github/scripts/review-sync/tests/test-utils.js.github/workflows/review-sync.yml
25df94b to
e0be030
Compare
Addresses maintainer feedback: a single maintainer approval with only 1 total review should not mark a PR as ready-to-merge, since branch protection requires 2+ reviews. Changes: - determineLabel() now requires maintainerApproval >= 1 AND anyApproval >= 2 for ready-to-merge - If a maintainer approves alone, the PR stays at queue:maintainers until a second reviewer also approves - Updated and added unit tests (38 total, all passing) Signed-off-by: darshit2308 <darshit2308@gmail.com>
625840b to
84a35d4
Compare
exploreriii
left a comment
There was a problem hiding this comment.
LGTM
Some ideas for fine tuning, either as part of a future phase or a smaller follow up supplement
Addresses maintainer feedback to match the original MVP proposal naming convention. Also updates the label description to accurately reflect the new logic requirement of '1+ maintainer and 2+ total approvals' instead of the old description. Signed-off-by: darshit2308 <darshit2308@gmail.com>
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: e28e1874-7f3f-424d-a4ac-d339d6234b8f
📒 Files selected for processing (3)
.github/scripts/review-sync/helpers/constants.js.github/scripts/review-sync/helpers/labels.js.github/scripts/review-sync/tests/test-labels.js
|
yes @exploreriii , i will try to develop this whole iteratively, and resolve all these with time. |
Updates a JSDoc comment in constants.js that still referenced the old label name without the status: prefix. Signed-off-by: darshit2308 <darshit2308@gmail.com>
Addresses CodeRabbit review feedback. Previously, if the API failed to remove a stale label (e.g. due to rate limits or network issues), the error was logged but execution continued. Since the new label was already added earlier in the function, this left the PR in an invalid state with multiple mutually-exclusive queue labels. Now, non-404 errors are explicitly re-thrown to fail the workflow and alert maintainers, preventing broken label states. Signed-off-by: darshit2308 <darshit2308@gmail.com>
There was a problem hiding this comment.
This catches per pr failures in errors but the script still exits successfully so stale queue labels could be hidden behind a green workflow
Could we fail the workflow when errors > 0 after the summary, while keeping per PR isolation during processing?
|
please mark as ready to review again when it is thanks |
1. index.js: Exit with code 1 if errors occurred during PR processing so that the GitHub Action workflow fails instead of hiding failures behind a green checkmark. 2. labels.js: Tighten ready-to-merge logic to strictly require at least 2 core reviews (maintainer + write). Soft approvals (triage/read) no longer satisfy the second review requirement for merge status. 3. test-labels.js: Update tests to reflect the new strict requirement. Signed-off-by: darshit2308 <darshit2308@gmail.com>
There was a problem hiding this comment.
Actionable comments posted: 6
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: c506a217-fa21-447a-9018-407112bb06c1
📒 Files selected for processing (4)
.github/scripts/review-sync/helpers/constants.js.github/scripts/review-sync/helpers/labels.js.github/scripts/review-sync/index.js.github/scripts/review-sync/tests/test-labels.js
|
Pre commit is failing due to trailing whitespace you can remove the extra space after |
NssGourav
left a comment
There was a problem hiding this comment.
looks good to me, only remaining item I see is the pre commit trailing whitespace failure, which should be fixed before merge.
Signed-off-by: darshit2308 <darshit2308@gmail.com>
exploreriii
left a comment
There was a problem hiding this comment.
Thanks all for your much needed help at reviewing and speeding this process up a lot
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 401d247c-f587-4e5a-9264-625275ae1417
📒 Files selected for processing (1)
.github/scripts/review-sync/helpers/labels.js
|
Thank you so much |
…iero-ledger#2242) Signed-off-by: darshit2308 <darshit2308@gmail.com>
…iero-ledger#2242) Signed-off-by: darshit2308 <darshit2308@gmail.com>
Description:
Introduce a cron-based workflow that runs every 30 minutes and keeps exactly one review-queue label accurate on every open non-draft PR based on live approval data. This is Phase 1 of 4 , label sync only. No routing, no assignment, no comments yet.
Changes:
.github/workflows/review-sync.yml, 30-min cron,workflow_dispatchwithdry_runtoggle, harden-runner, sparse-checkout, pinned SHAs, concurrency block,hl-sdk-py-lin-mdrunner.github/scripts/review-sync/index.js, entry point: rate-limit guard (floor=200), paginated PR fetch, label bootstrap (auto-creates 3 labels on first run), per-PR error isolation, run summary.github/scripts/review-sync/labels.js, core logic: approval counting split by permission level, DISMISSED ghost approval fix, explicit chronological sort on reviews, crash-safe add-first-then-remove label orderingNo existing files were modified. No collision with
.github/scripts/labels.js.Label logic:
queue:junior-committerqueue:committersready-to-mergeWhat this PR does NOT do:
step:orreviewer:labels , they coexist with the newqueue:labels until a future cleanup PRFixes #2229
Testing:

Notes for reviewer:
@vtags, matches existing repo conventionhl-sdk-py-lin-md, matches all other cron workflows (cron-reminder-pr-inactive.yml,cron-enforcer-pr-linked-issue.yml)DISMISSEDreviews actively delete the user's prior approval state , without this, GitHub's auto-dismiss (triggered when new commits are pushed after an approval) leaves a ghost approval in the map, causing a PR to be incorrectly promotedsubmitted_atbefore processing , correct by construction, not by API assumptiongroup: ${{ github.workflow }},cancel-in-progress: true) prevents two overlapping 30-min runs from racing on the same PR's labelsstep:andreviewer:labels are intentionally left untouched , both label systems coexist on existing PRs until Phase 3 cleanup PRqueue:committersbut notready-to-mergeensureLabelchecks existence only , if a label exists with the wrong colour it is not corrected in Phase 1 (documented in code)DRY_RUNenv var is wired from the job-levelenvblock , step-level repetition is intentionally avoidedqueue:junior-committer), 0 errorsChecklist:
hl-sdk-py-lin-md)@vtags)egress-policy: auditworkflow_dispatchdry-run tested end-to-end on forksubmitted_atstep:/reviewer:labels intentionally preserved (no breaking changes)