Modernize security manager team handling (incorporate PR #1010)

[decyjphr]

Summary

Incorporates PR #1010 (bug/issue-903) into yadhav/fix-recent-issues and fixes a gap in its approach.

The comprehensive teams.js on this branch still used the deprecated GET /orgs/{org}/security-managers endpoint (the roles API was only present as commented-out code), so PR #1010's security-manager modernization was not yet incorporated. This PR brings it in, adapted to this branch's Octokit client convention, and closes a follow-up correctness gap.

Changes

lib/plugins/teams.js

• Identify security manager teams via the organization roles API (GET /orgs/{org}/organization-roles + .../{role_id}/teams) instead of the deprecated security-managers endpoint.
• Adaptation: PR Bug/issue-903 #1010 uses this.github.rest.*; this branch uses the non-rest client (this.github.repos/teams.*), so the port matches that convention.
• Added team-name/slug and role-name normalization helpers so configured names match existing slugs without add/remove churn.
• Added a skipTeamDeletion guard: if security manager discovery fails, repository teams are kept unchanged rather than deleted.

Follow-up fix: never manage security manager teams from config

checkSecurityManager() only filtered security manager teams out of the existing team list. A config entry naming a security manager team then looked "missing" to Diffable.sync(), so add() / addOrUpdateRepoPermissionsInOrg would still fire — contradicting the "should not be handled here" intent. Now the discovered identifiers are persisted on the instance, and add(), update(), and remove() no-op for security manager teams (emitting an INFO command in nop mode so reviewers see the skip).

Docs

• docs/deploy.md: document the "Custom organization roles: Read-only" org permission. (app.yml already grants organization_custom_roles, so no permission change was needed.)

Tests

• Ported PR Bug/issue-903 #1010's unit + integration coverage, adapted to this branch's mocks.
• Added tests: a configured security manager team is neither added/updated nor deleted, and nop mode emits the INFO skip.

Verification

• Unit tests: full suite passes — 327 passed (24 teams tests). Lint (eslint + standard) clean.
• Integration tests: not run in CI on this branch (a pre-existing probot-version harness incompatibility affects all integration specs equally; CI runs only test:unit). The added nock mocks mirror PR Bug/issue-903 #1010.
• Smoke test (live org, phases 1–2): Phase 1 passed all 16 checks including "Team added" + "push permission", exercising find → checkSecurityManager (org-roles API) → add. Phase 2 (drift removal) is blocked only by the drift-actor token lacking team-repo admin permission on the EMU org — an environment limitation, not a code issue.

[Copilot]

Pull request overview

Updates the teams plugin to identify and protect Security Manager teams using the organization roles API (instead of the deprecated security-managers endpoint), adds normalization to prevent add/remove churn, and expands unit/integration coverage to validate the new behavior and safety guardrails.

Changes:

• Modernize Security Manager discovery via GET /orgs/{org}/organization-roles and .../{role_id}/teams, persisting discovered identifiers and no-op’ing config-driven management for those teams.
• Add normalization helpers for role/team identifiers and a deletion-suppression guard when discovery fails.
• Expand unit + integration tests and document the required “Custom organization roles: Read-only” org permission.

Show a summary per file

File	Description
lib/plugins/teams.js	Switches Security Manager detection to org-roles API, adds normalization + skip logic, and prevents config from managing Security Manager teams.
test/unit/lib/plugins/teams.test.js	Adds focused unit coverage for Security Manager behavior and adjusts mocks for the new API usage.
test/integration/plugins/teams.test.js	Extends integration test nocks to cover organization-roles API calls and Security Manager team presence.
docs/deploy.md	Documents the needed org permission for custom organization roles read access.

Review details

Tip

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

• Files reviewed: 4/4 changed files
• Comments generated: 2
• Review effort level: Lite

[Copilot]

Review details

Suppressed comments (1)

lib/plugins/teams.js:68

• Similar to the roles listing call above, octokit.paginate() is used for GET /orgs/{org}/organization-roles/{role_id}/teams without a map function. The REST response is an object wrapper (e.g. { total_count, teams: [...] }), so this may yield incorrect results and prevent populating securityManagerTeamIdentifiers, undermining the skip/no-op logic for security manager teams.

      this.log.debug(`Calling API to get security manager teams ${JSON.stringify(this.github.request.endpoint('GET /orgs/{org}/organization-roles/{role_id}/teams', params))} `)
      const resp = await this.github.paginate('GET /orgs/{org}/organization-roles/{role_id}/teams', params)

      this.log.debug(`Response from the call is ${JSON.stringify(resp)}`)
      const securityManagerTeams = this.toArray(resp, 'teams')
      const securityManagerTeamIdentifiers = new Set(securityManagerTeams.flatMap(team => [team.slug, team.name].map(name => this.normalizeTeamIdentifier(name))).filter(Boolean))

• Files reviewed: 4/4 changed files
• Comments generated: 1
• Review effort level: Lite