Bug/issue-903#1010
Open
madkoo wants to merge 8 commits into
Open
Conversation
…s in settings.json
…erty in settings.json
…to main-enterprise
…to main-enterprise
…ings into main-enterprise
Contributor
There was a problem hiding this comment.
Pull request overview
This PR updates the teams plugin to identify “Security Manager” teams via the GitHub Organization Roles API (instead of the deprecated security managers endpoint), improves team name/slug normalization to reduce churn, and updates required GitHub App permissions + docs, with expanded unit/integration test coverage.
Changes:
- Refactor security-manager team detection to use
/orgs/{org}/organization-rolesand/orgs/{org}/organization-roles/{role_id}/teams, with graceful failure behavior. - Normalize team identifiers (name/slug) for matching and for
getByNamelookups. - Add required
organization_custom_roles: readpermission and document it; expand tests to cover the new behavior.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| lib/plugins/teams.js | Uses org roles API to discover security-manager teams, adds normalization helpers, and introduces deletion-skipping behavior on discovery failures. |
| test/unit/lib/plugins/teams.test.js | Adds/updates unit tests for security-manager exclusion, failure modes, and normalization behavior. |
| test/integration/plugins/teams.test.js | Extends integration test to mock org roles API calls and ensure security-manager teams aren’t removed. |
| docs/deploy.md | Documents the new “Custom organization roles: Read-only” permission requirement. |
| app.yml | Adds organization_custom_roles: read to app default permissions. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
42
to
+46
| this.log.debug(`Response from the call is ${JSON.stringify(resp)}`) | ||
| return teams.filter(team => !resp.some(sec => sec.name === team.name)) | ||
| const securityManagerTeams = this.toArray(resp, 'teams') | ||
| const securityManagerTeamIdentifiers = new Set(securityManagerTeams.flatMap(team => [team.slug, team.name].map(name => this.normalizeTeamIdentifier(name))).filter(Boolean)) | ||
|
|
||
| return teams.filter(team => !this.isSecurityManagerTeam(team, securityManagerTeamIdentifiers)) |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This pull request enhances the handling of GitHub organization "Security Manager" teams by updating permissions, refactoring the
teamsplugin to use the new organization roles API, and improving normalization and matching of team names. It also adds robust error handling and expands test coverage to ensure correct behavior when working with security manager teams.Key changes include:
Security Manager Team Handling:
teamsplugin to use the new organization roles API (/orgs/{org}/organization-rolesand/orgs/{org}/organization-roles/{role_id}/teams) to identify and exclude security manager teams from repository team management, replacing the deprecated security managers endpoint. The plugin now normalizes team names and slugs for accurate matching and skips deletion of security manager teams if role lookups fail gracefully.Permissions and Documentation:
organization_custom_roles: readpermission inapp.ymland updated deployment documentation to reflect the new required permission for accessing organization roles.Team Name Normalization:
teamsplugin to ensure consistent matching and prevent unnecessary add/remove operations due to formatting differences.Testing Enhancements:
These changes ensure that security manager teams are properly identified and protected from accidental removal, while keeping the codebase up to date with GitHub API changes.