Per-path approval and per-group reviewer suggestions

[simonfaltum]

Why

Cross-domain PRs aren't handled correctly by the current maintainer-approval workflow. If a PR touches /cmd/pipelines/ and /cmd/apps/, a single approval from either team covers the whole PR. Each domain team should approve their own area independently.

The suggest-reviewers comment also doesn't surface which ownership areas need review. It just shows a flat list.

On top of that, the workflow uses core.setFailed() to block PRs missing approval. This shows a red X, which is misleading: "waiting for approval" isn't an error, it's a normal pending state.

Changes

Before: One exemption check (isExempted) that only worked when the PR author owned all changed files. Cross-domain PRs fell through to requiring a maintainer. suggest-reviewers showed a flat list. Missing approval = red X.

Now: Per-path approval where each ownership group needs at least one approval from its owners. suggest-reviewers shows per-group sections for cross-domain PRs. Missing approval = yellow pending dot via Commit Status API.

owners.js

Added getOwnershipGroups(filenames, rules) that groups files by their last-match-wins OWNERS pattern. Returns Map<pattern, { owners, files }>. This is the shared building block used by both the approval check and reviewer suggestions.

maintainer-approval.js

Replaced isExempted with checkPerPathApproval. The new flow:

1. Any maintainer approved? -> success (unchanged)
2. PR author is a maintainer and has any approval? -> success (new, any second pair of eyes suffices)
3. Group changed files by ownership. Any files matching only *? -> pending (needs maintainer)
4. For each ownership group, has at least one owner approved? All covered -> success. Some uncovered -> pending (lists which groups need approval)

Team refs (@org/team) are resolved via getMembershipForUserInOrg. The resolution distinguishes 404 (not a member) from permission errors (logs a warning, falls back to requiring maintainer).

Switched from core.setFailed() to createCommitStatus() with pending/success states, so "waiting for approval" is a yellow dot instead of a red X. The ruleset should require the maintainer-approval commit status context.

suggest-reviewers.js

When a PR crosses 2+ ownership groups, shows "Reviewers by ownership area" with per-group sections. Each section lists the matched files, team refs, and individually ranked owners (by git history score). Single-domain PRs keep the existing flat format. The wildcard section (* files) only suggests maintainers, since only they can approve those files.

OWNERS

• Added /bundle/, /acceptance/bundle/, and /acceptance/labs/ rules mirroring their source counterparts
• Added section comments for readability
• Grouped rules by domain

maintainer-approval.yml

• Added statuses: write permission (needed for createCommitStatus)

Test plan

• 36 unit tests using Node's built-in node:test runner (zero dependencies)
  • 24 tests for owners.js: ownersMatch, parseOwnersFile, findOwners, getMaintainers, getOwnershipGroups
  • 12 tests for maintainer-approval.js with mocked GitHub API: maintainer approval, maintainer-authored PR self-approval, single domain, cross-domain (both covered and partially covered), wildcard files, team member approval, non-team-member rejection, CHANGES_REQUESTED exclusion, self-approval exclusion, missing * rule error
  • Run: node --test .github/scripts/owners.test.js .github/workflows/maintainer-approval.test.js
• Verify on a single-domain PR (only /cmd/pipelines/ files): pending until pipelines owner approves
• Verify on a cross-domain PR (/cmd/pipelines/ + /cmd/apps/): pending until both groups approved
• Verify on a PR touching only * files: pending, message says "needs maintainer"
• Verify maintainer approval short-circuits: success status immediately
• Update branch ruleset to require maintainer-approval commit status context

Edge case coverage (from unit tests)

Scenario	Expected	Tested
PR touches only * files	Maintainer required, pending	yes
PR touches one domain only	One domain owner approval suffices	yes
PR touches two domains	Both groups need approval	yes
PR touches domain + * files	Maintainer required (wildcard forces it)	yes
Maintainer approves	Always success	yes
PR author is maintainer + any approval	Success (second pair of eyes)	yes
Team member approves team-owned path	Success (via Teams API)	yes
CHANGES_REQUESTED review	Does not count as approval	yes
PR author self-approval	Excluded from approver list	yes