fix: Harden review-sync pipeline and resolve reviewer feedback by darshit2308 · Pull Request #2262 · hiero-ledger/hiero-sdk-python

darshit2308 · 2026-05-09T11:45:59Z

Description:
Harden the review-sync infrastructure ahead of Phase 2 with 6 atomic fixes addressing API efficiency, error visibility, CI safety, test coverage, routing correctness, and community visibility.

Add module-level permission cache in permissions.js to eliminate redundant API calls
Use core.setFailed() in index.js when error count > 0 to surface broken syncs
Add CI failure rollback via hasCIFailures() , failing checks override all approvals and demote PRs back to queue:junior-committer
Create test-review-sync.yml workflow to run npm test on review-sync changes
Refactor approval counters to use coreApprovals (write + maintain + admin) so maintainers inherently satisfy the core requirement
Add permanent open to community review label on all open PRs
Route early maintainer approvals to queue:committers instead of queue:maintainers to prevent duplicate maintainer reviews when a maintainer jumps in before triage

Related issue(s): #2229

Fixes #2260

Notes for reviewer:

Live-tested on fork (darshit2308/hiero-sdk-python) for API connectivity and permissions:

Unit tests: 33/33 passing (View Test Run)
Dry run (dry_run: true): Correctly logs intended label changes without modifying state
Write mode (dry_run: false): Successfully creates and applies labels to PRs via API (View Test Run)

Logic Edge-Cases Proven via Automated Tests:
4. Early maintainer routing: When a maintainer approves before triage, the test suite verifies the PR routes to queue:committers (not queue:maintainers) to call a committer, preventing wasted maintainer bandwidth.
5. CI rollback: Test suite verifies ciFailing=true overrides all approvals -> queue:junior-committer.

Key files changed:
.github/scripts/review-sync/helpers/permissions.js --> permission cache + coreApprovals
.github/scripts/review-sync/helpers/labels.js --> determineLabel routing + CI rollback
.github/scripts/review-sync/helpers/constants.js --> community review label definition
.github/scripts/review-sync/helpers/ci.js --> hasCIFailures() helper
.github/scripts/review-sync/index.js --> core.setFailed() error handling
.github/scripts/review-sync/tests/* --> 33 unit tests
.github/workflows/review-sync.yml --> checks:read permission for CI API
.github/workflows/test-review-sync.yml --> [NEW] CI test workflow

Checklist

Documented (Code comments, etc.)
Tested (unit, integration, etc.)

codacy-production · 2026-05-09T11:46:57Z

Up to standards ✅

🟢 Issues 0 issues

Results:
0 new issues

View in Codacy

🟢 Metrics 9 complexity · 0 duplication

Metric Results

Complexity 9

Duplication 0

View in Codacy

_{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.}

coderabbitai · 2026-05-09T11:50:59Z

Walkthrough

Adds COMMUNITY_REVIEW label, caches collaborator permissions, refactors approval counting, detects CI failures to demote PRs, updates sync to require community label for human authors, improves error reporting via core.setFailed, and adds tests plus a test workflow.

Changes

Review Queue Hardening

Layer / File(s)	Summary
Data Contracts `.github/scripts/review-sync/helpers/constants.js`	Introduces `COMMUNITY_REVIEW` label constant with name, color, and description; updates `QUEUE_LABELS.MERGE.description`; exports `COMMUNITY_REVIEW`.
Permission Caching & Approval Counting `.github/scripts/review-sync/helpers/permissions.js`	Adds in-memory `permissionCache` keyed by `owner/repo/username`; refactors `countApprovals` to return `{ maintainerApprovals, coreApprovals, softApprovals, anyApproval }`; exports `clearPermissionCache()`.
CI Failure Detection `.github/scripts/review-sync/helpers/labels.js`	Adds `hasCIFailures(github, owner, repo, sha)` to page `checks.listForRef`; updates `determineLabel(approvals, ciFailing = false)` to demote CI-failing PRs and to use revised promotion logic; exports `hasCIFailures`.
Label Synchronization `.github/scripts/review-sync/helpers/labels.js`	Reworks `syncLabel()` to compute approvals and CI state, determine correct queue label, ensure `COMMUNITY_REVIEW` on human PRs, add correct queue + community labels together, then remove stale queue labels.
Job Orchestration & Error Handling `.github/scripts/review-sync/index.js`, `.github/workflows/review-sync.yml`	Ensures `COMMUNITY_REVIEW` exists during startup; replaces `process.exitCode = 1` with `core.setFailed(...)` when label-sync errors occur; workflow adds `checks: read` permission.
Test Infrastructure `.github/scripts/review-sync/package.json`, `.github/scripts/review-sync/tests/*`, `.github/scripts/review-sync/tests/test-utils.js`, `.github/workflows/test-review-sync.yml`	Adds package.json with test script; extends mock GitHub client to support `rest.checks.listForRef` and updated paginate behavior; updates unit tests for new approval shape and CI behavior; adds workflow to run tests on review-sync changes.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title 'fix: Harden review-sync pipeline and resolve reviewer feedback' accurately reflects the main change: hardening the review-sync infrastructure with multiple fixes.
Description check	✅ Passed	The description is thorough and directly related to the changeset, detailing six atomic fixes with specific file changes, testing evidence, and addressing issue `#2260`.
Linked Issues check	✅ Passed	All six core objectives from issue `#2260` are met: permission caching added, error handling via core.setFailed() implemented, CI failure rollback via hasCIFailures() implemented, test workflow created, approval logic refactored to coreApprovals, and community review label added.
Out of Scope Changes check	✅ Passed	All changes are directly scoped to hardening the review-sync infrastructure as specified in issue `#2260`; no extraneous modifications detected outside the stated objectives.
Docstring Coverage	✅ Passed	Docstring coverage is 80.00% which is sufficient. The required threshold is 80.00%.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

📋 Issue Planner

Built with CodeRabbit's Coding Plans for faster development and fewer bugs.

View plan used: #2260

✨ Finishing Touches

🧪 Generate unit tests (beta)

Create PR with unit tests

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.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

coderabbitai

Actionable comments posted: 5

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 43981541-3f67-4ac6-9071-89febccb1036

📥 Commits

Reviewing files that changed from the base of the PR and between fd669c5 and 3148f54.

📒 Files selected for processing (10)

.github/scripts/review-sync/helpers/constants.js
.github/scripts/review-sync/helpers/labels.js
.github/scripts/review-sync/helpers/permissions.js
.github/scripts/review-sync/index.js
.github/scripts/review-sync/package.json
.github/scripts/review-sync/tests/test-labels.js
.github/scripts/review-sync/tests/test-permissions.js
.github/scripts/review-sync/tests/test-utils.js
.github/workflows/review-sync.yml
.github/workflows/test-review-sync.yml

Signed-off-by: darshit2308 <darshit2308@gmail.com>

Copilot

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

Signed-off-by: darshit2308 <darshit2308@gmail.com>

coderabbitai

Actionable comments posted: 4

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 2cfb1a69-1317-4a0b-b870-54f3949637c7

📥 Commits

Reviewing files that changed from the base of the PR and between 3148f54 and dba3d66.

📒 Files selected for processing (10)

.github/scripts/review-sync/helpers/constants.js
.github/scripts/review-sync/helpers/labels.js
.github/scripts/review-sync/helpers/permissions.js
.github/scripts/review-sync/index.js
.github/scripts/review-sync/package.json
.github/scripts/review-sync/tests/test-labels.js
.github/scripts/review-sync/tests/test-permissions.js
.github/scripts/review-sync/tests/test-utils.js
.github/workflows/review-sync.yml
.github/workflows/test-review-sync.yml

exploreriii · 2026-05-09T16:58:30Z

you've not set up the pre commit hook

Signed-off-by: darshit2308 <darshit2308@gmail.com>

coderabbitai

Actionable comments posted: 1

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: ea71164e-029e-400c-8f72-6959fea9e6df

📥 Commits

Reviewing files that changed from the base of the PR and between dba3d66 and 876c95b.

📒 Files selected for processing (1)

.github/scripts/review-sync/helpers/labels.js

AntonioCeppellini

🚀

…-ledger#2262) Signed-off-by: darshit2308 <darshit2308@gmail.com>

Copilot AI review requested due to automatic review settings May 9, 2026 11:46

darshit2308 requested review from a team as code owners May 9, 2026 11:46

darshit2308 requested review from jeromy-cannon and nadineloepfe May 9, 2026 11:46

Copilot started reviewing on behalf of darshit2308 May 9, 2026 11:46 View session

darshit2308 force-pushed the phase1-followup branch from 3148f54 to 76aba36 Compare May 9, 2026 11:47

darshit2308 requested a review from a team as a code owner May 9, 2026 11:47

darshit2308 force-pushed the phase1-followup branch from 76aba36 to 638c92e Compare May 9, 2026 11:49

darshit2308 marked this pull request as draft May 9, 2026 11:50

coderabbitai Bot reviewed May 9, 2026

View reviewed changes

fix: Harden review-sync pipeline and resolve reviewer feedback

34eb7b5

Signed-off-by: darshit2308 <darshit2308@gmail.com>

darshit2308 force-pushed the phase1-followup branch from 638c92e to 34eb7b5 Compare May 9, 2026 11:51

fix: Address some more issues

ecdf571

Signed-off-by: darshit2308 <darshit2308@gmail.com>

darshit2308 changed the title ~~Phase1 followup~~ fix: Harden review-sync pipeline and resolve reviewer feedback May 9, 2026

Copilot AI reviewed May 9, 2026

darshit2308 marked this pull request as ready for review May 9, 2026 12:14

github-actions Bot added the queue:junior-committer PR awaiting initial quality review label May 9, 2026

fix: Address CodeRabbit review feedback

dba3d66

Signed-off-by: darshit2308 <darshit2308@gmail.com>

coderabbitai Bot reviewed May 9, 2026

View reviewed changes

Comment thread .github/scripts/review-sync/helpers/labels.js

Comment thread .github/scripts/review-sync/tests/test-labels.js

Comment thread .github/scripts/review-sync/tests/test-permissions.js

Comment thread .github/workflows/test-review-sync.yml

exploreriii previously approved these changes May 9, 2026

View reviewed changes

Comment thread .github/scripts/review-sync/helpers/labels.js

Comment thread .github/scripts/review-sync/helpers/labels.js

Comment thread .github/scripts/review-sync/helpers/labels.js

Comment thread .github/scripts/review-sync/helpers/labels.js

github-actions Bot added the skill: beginner Achievable by a fairly new comer that has already completed a couple of good first issues label May 9, 2026

fix: remove trailing whitespace in labels.js

876c95b

Signed-off-by: darshit2308 <darshit2308@gmail.com>

darshit2308 dismissed exploreriii’s stale review via 876c95b May 9, 2026 17:08

coderabbitai Bot reviewed May 9, 2026

View reviewed changes

Comment thread .github/scripts/review-sync/helpers/labels.js

exploreriii approved these changes May 9, 2026

View reviewed changes

github-actions Bot added the queue:maintainers PR awaiting maintainer final review label May 9, 2026

github-actions Bot removed the queue:junior-committer PR awaiting initial quality review label May 9, 2026

AntonioCeppellini approved these changes May 9, 2026

View reviewed changes

manishdait merged commit e19f3cc into hiero-ledger:main May 9, 2026
14 checks passed

manishdait added this to the v0.2.7 milestone May 9, 2026

NssGourav pushed a commit to NssGourav/hiero-sdk-python that referenced this pull request May 14, 2026

fix: Harden review-sync pipeline and resolve reviewer feedback (hiero…

b6f45e7

…-ledger#2262) Signed-off-by: darshit2308 <darshit2308@gmail.com>

Uh oh!

Conversation

darshit2308 commented May 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codacy-production Bot commented May 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Up to standards ✅

Uh oh!

coderabbitai Bot commented May 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Estimated code review effort

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

exploreriii commented May 9, 2026

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

AntonioCeppellini left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

darshit2308 commented May 9, 2026 •

edited

Loading

codacy-production Bot commented May 9, 2026 •

edited

Loading

coderabbitai Bot commented May 9, 2026 •

edited

Loading