Honor role_name in runtime role authorization for on.roles (maintain/triage)

[Copilot]

on.roles allowed maintain and triage in schema, but runtime authorization matched only permission, making those values effectively unreachable in real API responses. This change aligns runtime behavior with schema by resolving roles using the collaborator API’s role_name when present.

• Runtime authorization alignment

  • Updated checkRepositoryPermission to derive an effectiveRole from:
    • role_name (preferred, normalized maintainer -> maintain)
    • fallback to permission when role_name is absent
  • Authorization now compares required roles against effectiveRole, removing the schema-valid-but-unreachable gap.

• API-shape-consistent fixtures

  • Updated permission-check fixtures to model real collaborator responses containing both permission and role_name (e.g., permission: "write", role_name: "maintain").
  • Kept existing alias compatibility (maintainer accepted as maintain).

• Focused coverage for precedence semantics

  • Added coverage proving role_name is authoritative when it differs from permission (e.g., deny roles: [write] when role_name is maintain even if permission is write).

const rawRoleName = repoPermission.data.role_name;
const roleName = rawRoleName == null ? "" : typeof rawRoleName === "string" ? rawRoleName : "";
const normalizedRoleName = roleName === "maintainer" ? "maintain" : roleName;
const normalizedPermission = permission === "maintainer" ? "maintain" : permission;
const effectiveRole = normalizedRoleName || normalizedPermission;

const hasPermission = requiredPermissions.some(requiredPerm => {
  const normalizedRequired = requiredPerm === "maintainer" ? "maintain" : requiredPerm;
  return normalizedRequired === effectiveRole;
});

[Copilot]

Pull request overview

This pull request fixes a schema/runtime mismatch in the JavaScript pre-activation authorization logic by honoring the GitHub collaborator API’s role_name (when present) instead of relying solely on permission, making maintain and triage roles reachable in real runtime authorization decisions.

Changes:

• Updated checkRepositoryPermission to compute an effectiveRole from role_name (preferred, including maintainer → maintain alias) with fallback to permission.
• Adjusted permission-check test fixtures to reflect realistic collaborator API responses that include both permission and role_name.
• Added coverage to verify role_name takes precedence over permission when they differ.

Show a summary per file

File	Description
actions/setup/js/check_permissions_utils.cjs	Derives and authorizes against an effectiveRole using role_name when available; logs both when they differ.
actions/setup/js/check_permissions_utils.test.cjs	Updates fixtures and adds a precedence test to ensure role_name is authoritative.
actions/setup/js/check_permissions.test.cjs	Updates fixtures to model real collaborator responses for maintain/triage scenarios.
actions/setup/js/check_membership.test.cjs	Updates maintainer/maintain alias fixture to use realistic permission + role_name shape.

Copilot's findings

Tip

Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

• Files reviewed: 4/4 changed files
• Comments generated: 0

[github-actions]

✅ Design Decision Gate 🏗️ completed the design decision gate check.

No ADR enforcement needed: PR #37959 does not have the 'implementation' label and has 0 new lines of code in business logic directories (threshold is 100).

[github-actions]

🧠 Matt Pocock Skills Reviewer failed during the skills-based review.

[github-actions]

⚠️ PR Code Quality Reviewer failed during code quality review.

[github-actions]

🧪 Test Quality Sentinel completed test quality analysis.

[github-actions]

🧪 Test Quality Sentinel Report

✅ Test Quality Score: 96/100 — Excellent

Analyzed 7 test(s) across 3 JavaScript test files: 7 design tests, 0 implementation tests, 0 guideline violations.

📊 Metrics & Test Classification (7 tests analyzed)

Metric	Value
New/modified tests analyzed	7
✅ Design tests (behavioral contracts)	7 (100%)
⚠️ Implementation tests (low value)	0 (0%)
Tests with error/edge cases	6 (86%)
Duplicate test clusters	0
Test inflation detected	No (1:1 ratio for utils files)
🚨 Coding-guideline violations	0

Test Classification Details

Test	File	Classification	Issues Detected
should handle maintainer/maintain alias	check_membership.test.cjs:307	✅ Design	None — verifies end-to-end script behavior with realistic API response (permission: write, role_name: maintain)
should pass validation for maintain permission when maintainer is required	check_permissions.test.cjs:89	✅ Design	None — verifies authorization passes when role_name: maintain satisfies the maintainer requirement
should handle triage permission correctly	check_permissions.test.cjs:138	✅ Design	None — covers triage edge case (permission: read, role_name: triage)
should handle repo names with hyphens and underscores	check_permissions.test.cjs:193	✅ Design	None — covers special-character repo names combined with maintain role alias
should return authorized for maintain when maintainer is required	check_permissions_utils.test.cjs:227	✅ Design	None — adds assertion for new log format distinguishing raw permission from role
should handle triage permission	check_permissions_utils.test.cjs:300	✅ Design	None — verifies triage authorization with realistic permission: read API shape
should use role_name over permission for authorization decisions	check_permissions_utils.test.cjs:311	✅ Design	None — key invariant: when role_name ≠ permission, role_name wins; rejects write requirement when effective role is maintain

Language Support

Tests analyzed:

• 🐹 Go (*_test.go): 0 tests
• 🟨 JavaScript (*.test.cjs, *.test.js): 7 tests (vitest)

Verdict

✅ Check passed. 0% of new tests are implementation tests (threshold: 30%). All 7 modified/new tests verify observable behavioral contracts. 6/7 tests exercise the key edge case this PR introduces: the GitHub API returning permission: write alongside role_name: maintain (or permission: read alongside role_name: triage), where role_name must take precedence for correct authorization decisions.

📖 Understanding Test Classifications

Design Tests (High Value) verify what the system does:

• Assert on observable outputs, return values, or state changes
• Cover error paths and boundary conditions
• Would catch a behavioral regression if deleted
• Remain valid even after internal refactoring

Implementation Tests (Low Value) verify how the system does it:

• Assert on internal function calls (mocking internals)
• Only test the happy path with typical inputs
• Break during legitimate refactoring even when behavior is correct
• Give false assurance: they pass even when the system is wrong

Goal: Shift toward tests that describe the system's behavioral contract — the promises it makes to its users and collaborators.

References: §27172683290

🧪 Test quality analysis by Test Quality Sentinel · 232.9 AIC · ⌖ 51.1 AIC · ◷

[github-actions]

✅ Test Quality Sentinel: 96/100. Test quality is excellent — 0% of new tests are implementation tests (threshold: 30%). All 7 modified/new tests verify observable behavioral contracts, with 6/7 directly exercising the key edge case this PR addresses.

[pelikhan]

@copilot run pr-finisher skill (ignore impacted tests failure)

[Copilot]

@copilot run pr-finisher skill (ignore impacted tests failure)

Local validation is green (make fmt, make lint, make test-unit, make test all pass). No open review threads.

The only CI failure is impacted-js-tests — this is a pre-existing infra issue where the version of vitest installed doesn't support the --related flag (CACError: Unknown option '--related'). It is unrelated to this PR's changes, and all four JS test shards, lint-js, and js-typecheck passed cleanly.

Ready for merge pending a human CI re-trigger (close/reopen PR or workflow_dispatch).