Skip to content

Commit f9eb2cc

Browse files
committed
fix: permissions and review sync corrections
Signed-off-by: darshit2308 <darshit2308@gmail.com>
1 parent f61613a commit f9eb2cc

4 files changed

Lines changed: 48 additions & 3 deletions

File tree

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

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,8 @@
1919

2020
const { getLatestReviewStates } = require('./reviews');
2121

22+
const permissionCache = new Map();
23+
2224
/**
2325
* Check the repository role for a given user.
2426
*
@@ -31,6 +33,10 @@ const { getLatestReviewStates } = require('./reviews');
3133
* @returns {string} 'admin' | 'maintain' | 'write' | 'triage' | 'read' | 'none'
3234
*/
3335
async function getPermissionLevel(github, owner, repo, username) {
36+
if (permissionCache.has(username)) {
37+
return permissionCache.get(username);
38+
}
39+
3440
try {
3541
const { data } = await github.rest.repos.getCollaboratorPermissionLevel({
3642
owner,
@@ -40,15 +46,19 @@ async function getPermissionLevel(github, owner, repo, username) {
4046
// CRITICAL: Use role_name, NOT permission.
4147
// The legacy 'permission' field maps maintain → write, triage → read.
4248
// role_name correctly returns: admin | maintain | write | triage | read
43-
return data.role_name || data.permission || 'none';
49+
const role = data.role_name || data.permission || 'none';
50+
permissionCache.set(username, role);
51+
return role;
4452
} catch (error) {
4553
if (error.status === 404) {
4654
// External contributor — not a collaborator
55+
permissionCache.set(username, 'none');
4756
return 'none';
4857
}
4958
// Log unexpected errors but don't crash the run
5059
const message = error instanceof Error ? error.message : String(error);
5160
console.log(` ⚠ Permission check failed for ${username}: ${message}. Treating as "none".`);
61+
permissionCache.set(username, 'none');
5262
return 'none';
5363
}
5464
}

.github/scripts/review-sync/index.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -90,6 +90,6 @@ module.exports = async ({ github, context, core }) => {
9090
console.log(` Errors: ${errors}`);
9191

9292
if (errors > 0) {
93-
process.exitCode = 1;
93+
core.setFailed(`Review sync completed with ${errors} error(s). Check logs above.`);
9494
}
9595
};

.github/workflows/review-sync.yml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ on:
2323
type: string
2424

2525
permissions:
26-
pull-requests: read
26+
pull-requests: write
2727
issues: write
2828
contents: read
2929

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
name: Test Review Sync Scripts
2+
3+
on:
4+
pull_request:
5+
paths:
6+
- '.github/scripts/review-sync/**'
7+
- '.github/workflows/test-review-sync.yml'
8+
9+
jobs:
10+
test:
11+
runs-on: hl-sdk-py-lin-md
12+
steps:
13+
- name: Harden the runner
14+
uses: step-security/harden-runner@8d3c67de8e2fe68ef647c8db1e6a09f647780f40 # v2.19.0
15+
with:
16+
egress-policy: audit
17+
18+
- name: Checkout repository
19+
uses: actions/checkout@8e8c483db84b4bee98b60c0593521ed34d9990e8 # v6.0.1
20+
with:
21+
sparse-checkout: |
22+
.github/scripts
23+
sparse-checkout-cone-mode: false
24+
25+
- name: Setup Node.js
26+
uses: actions/setup-node@v4
27+
with:
28+
node-version: '20'
29+
30+
- name: Run Unit Tests
31+
run: |
32+
node .github/scripts/review-sync/tests/test-labels.js
33+
node .github/scripts/review-sync/tests/test-permissions.js
34+
node .github/scripts/review-sync/tests/test-reviews.js
35+
node .github/scripts/review-sync/tests/test-utils.js

0 commit comments

Comments
 (0)