diff --git a/.github/scripts/pull-request-dashboard/WEBHOOK_SETUP.md b/.github/scripts/pull-request-dashboard/WEBHOOK_SETUP.md index 7da7de3baf8..addfe7e0ad9 100644 --- a/.github/scripts/pull-request-dashboard/WEBHOOK_SETUP.md +++ b/.github/scripts/pull-request-dashboard/WEBHOOK_SETUP.md @@ -53,10 +53,6 @@ Repository permissions: - Metadata: read-only - Pull requests: read and write -Organization permissions: - -- Members: read-only - Permission rationale: | Permission | Access | Why it is needed | @@ -64,9 +60,8 @@ Permission rationale: | Checks | Read | Required to subscribe to check-suite events and to read check data for dashboard rows. | | Contents | Read | Reads PR commits and repository metadata needed by pull/commit APIs. | | Issues | Read and write | Finds, creates, and updates the dashboard issue. | -| Metadata | Read | Required by GitHub for GitHub App repository access. | +| Metadata | Read | Required by GitHub for GitHub App repository access and used to list collaborators with write access. | | Pull requests | Read and write | Required to subscribe to PR review/comment/thread events; read PR details, reviews, review comments, commits, and GraphQL review threads; and create the dashboard-managed PR status comment, which is a pull request conversation comment. | -| Members | Read | Reads approver-team membership configured in `repositories.json`. | The dashboard does not create inline review comments, submit reviews, or resolve review threads. It manages one PR conversation comment (create, update, and diff --git a/.github/scripts/pull-request-dashboard/dashboard.py b/.github/scripts/pull-request-dashboard/dashboard.py index 48e787ab928..e1f879a19e1 100644 --- a/.github/scripts/pull-request-dashboard/dashboard.py +++ b/.github/scripts/pull-request-dashboard/dashboard.py @@ -13,8 +13,6 @@ Usage: python .github/scripts/pull-request-dashboard/dashboard.py --state-branch BRANCH --repo REPO - --approver-team TEAM - [--approver-team TEAM] [--pr-number N] [--github-output PATH] [--model NAME] @@ -102,7 +100,7 @@ maintenance bot. is_draft bool approval_count int Current unique APPROVED reviews - from approver-team members. + from collaborators with write access. ci_failing_count int Required checks only; absent when checks could not be fetched. ci_failing_since str (iso) Earliest completion time among @@ -136,10 +134,10 @@ "changes_requested": bool, "open_thread": bool, "top_level_feedback": bool}; approved - means an approver-team member + means a collaborator with write access is in the APPROVED state, approved_non_team means someone - outside the team approved, + without write access approved, changes_requested means an reviewer's latest review is CHANGES_REQUESTED, @@ -504,7 +502,7 @@ def latest_review_states(events: list[dict[str, Any]]) -> dict[str, str]: def commenting_reviewers(events: list[dict[str, Any]]) -> set[str]: - # Approver-team members who have participated on the PR in any way: an + # Collaborators with write access who have participated on the PR in any way: an # issue comment, an inline review comment, or a submitted review. This # surfaces engaged reviewers even when they have neither approved nor own # an open discussion. @@ -1322,8 +1320,8 @@ def add_reviewers( pending_actions: dict[str, dict[str, Any]], ) -> None: # Reviewers to display in the dashboard, each flagged with their review - # stance: approved (by an approver-team member), approved_non_team (an - # approval from someone outside the team), changes_requested (their latest + # stance: approved (by a collaborator with write access), approved_non_team + # (an approval from someone without write access), changes_requested (their latest # review blocks), open_thread (they own an # unresolved discussion), and top_level_feedback (their top-level feedback # still needs author action). The renderer turns these into icons. @@ -1883,7 +1881,7 @@ def build_targeted_dashboard_update(args: argparse.Namespace) -> DashboardUpdate print("dashboard result state not found; skipping targeted refresh", file=sys.stderr) return None - reviewers = load_reviewer_set(owner, args.approver_team) + reviewers = load_reviewer_set(repo) return build_dashboard_update_for_pr( repo, owner, @@ -1949,7 +1947,7 @@ def update_dashboard_for_backfill(args: argparse.Namespace, state_dir: Path) -> open_pr_numbers = {p["number"] for p in prs} open_non_draft_pr_numbers = {p["number"] for p in prs if not p.get("isDraft")} prune_classification_cache(open_pr_numbers) - reviewers = load_reviewer_set(owner, args.approver_team) + reviewers = load_reviewer_set(repo) state_branch.configure_git() state_branch.checkout_state(state_dir, args.state_branch, require_existing=False) try: @@ -2087,12 +2085,6 @@ def main() -> int: help="git branch used for workflow state", ) parser.add_argument("--repo", help="target repository name, e.g. opentelemetry-java-instrumentation") - parser.add_argument( - "--approver-team", - action="append", - required=True, - help="approver team slug for the target repository; repeat for multiple teams", - ) parser.add_argument("--pr-number", type=int, help="only refresh dashboard state for this PR") parser.add_argument( "--required-approvals", diff --git a/.github/scripts/pull-request-dashboard/github_cli.py b/.github/scripts/pull-request-dashboard/github_cli.py index 5e9559a1eed..5b03542bc5a 100644 --- a/.github/scripts/pull-request-dashboard/github_cli.py +++ b/.github/scripts/pull-request-dashboard/github_cli.py @@ -601,20 +601,18 @@ def detect_repo() -> str: return proc.stdout.strip() -def load_reviewer_set(org: str, approver_team_slugs: list[str]) -> set[str]: +def load_reviewer_set(repo: str) -> set[str]: token = os.environ.get("PR_DASHBOARD_TOKEN") or None - reviewers: set[str] = set() - for slug in approver_team_slugs: - members = gh_api( - f"/orgs/{org}/teams/{slug}/members?per_page=100", - paginate=True, - token=token, - ) - reviewers.update(m["login"] for m in members) + collaborators = gh_api( + f"/repos/{repo}/collaborators?permission=push&per_page=100", + paginate=True, + token=token, + ) + reviewers = {collaborator["login"] for collaborator in collaborators} if not reviewers: raise RuntimeError( - f"no reviewers found in teams {approver_team_slugs}; " - f"the dashboard app token must have org members read permission" + f"no collaborators with write access found for {repo}; " + f"the dashboard app token must have repository metadata read permission" ) return {r.lower() for r in reviewers} diff --git a/.github/scripts/pull-request-dashboard/render.py b/.github/scripts/pull-request-dashboard/render.py index 40c16e41617..d1b8df8cb80 100644 --- a/.github/scripts/pull-request-dashboard/render.py +++ b/.github/scripts/pull-request-dashboard/render.py @@ -117,8 +117,8 @@ def reviewer_icon(reviewer: dict[str, Any]) -> str: if reviewer.get("approved"): discussion_icons.append("✅") elif reviewer.get("approved_non_team"): - # A black/gray check distinguishes a non-code-owner approval from a - # code-owner approval; only code-owner approvals count toward merge. + # A black/gray check distinguishes an approval without write access; + # only approvals from collaborators with write access count here. discussion_icons.append("✔️") return WORD_JOINER.join(discussion_icons) @@ -259,7 +259,7 @@ def render_pr_tables( f"partly performed by an LLM ([source]({source_url})) and could contain mistakes." ) reviewers_note = ( - "Reviewers column: ✅ approved · ✔️ approved (non-code-owner) · " + "Reviewers column: ✅ approved with write access · ✔️ approved without write access · " "💬 open review thread · 📌 top-level feedback needs author action · 🔴 changes requested." ) out: list[str] = [ diff --git a/.github/scripts/pull-request-dashboard/repositories.json b/.github/scripts/pull-request-dashboard/repositories.json index 40e3482c016..7c20a54930f 100644 --- a/.github/scripts/pull-request-dashboard/repositories.json +++ b/.github/scripts/pull-request-dashboard/repositories.json @@ -1,84 +1,61 @@ [ { "name": "opentelemetry-collector-contrib", - "approver_teams": ["collector-contrib-approvers"], "large_repo": true }, { - "name": "opentelemetry-dotnet", - "approver_teams": ["dotnet-approvers"] + "name": "opentelemetry-dotnet" }, { - "name": "opentelemetry-dotnet-contrib", - "approver_teams": ["dotnet-contrib-approvers"] + "name": "opentelemetry-dotnet-contrib" }, { "name": "opentelemetry-java", - "approver_teams": ["java-approvers"], "non_blocking_check_patterns": [ "markdown-link-check / link-check", "codecov/*" ] }, { - "name": "opentelemetry-java-instrumentation", - "approver_teams": ["java-instrumentation-approvers"] + "name": "opentelemetry-java-instrumentation" }, { - "name": "opentelemetry-js", - "approver_teams": ["javascript-approvers"] + "name": "opentelemetry-js" }, { - "name": "opentelemetry-js-contrib", - "approver_teams": ["javascript-approvers"] + "name": "opentelemetry-js-contrib" }, { "name": "opentelemetry-proto", - "approver_teams": ["proto-approvers"], "required_approvals": 2 }, { - "name": "opentelemetry-python-genai", - "approver_teams": ["python-genai-approvers"] + "name": "opentelemetry-python-genai" }, { - "name": "opentelemetry-weaver-examples", - "approver_teams": [ - "weaver-approvers" - ] + "name": "opentelemetry-weaver-examples" }, { - "name": "opentelemetry-weaver-packages", - "approver_teams": [ - "weaver-package-approvers" - ] + "name": "opentelemetry-weaver-packages" }, { - "name": "opentelemetry.io", - "approver_teams": ["docs-approvers"] + "name": "opentelemetry.io" }, { "name": "semantic-conventions", - "approver_teams": ["specs-semconv-approvers"], "required_approvals": 2 }, { "name": "semantic-conventions-genai", - "approver_teams": ["semconv-genai-approvers"], "required_approvals": 2 }, { - "name": "shared-workflows", - "approver_teams": ["shared-workflows-approvers"] + "name": "shared-workflows" }, { - "name": "weaver", - "approver_teams": [ - "weaver-approvers" - ] + "name": "weaver" }, { - "name": "otel-arrow", - "approver_teams": ["arrow-approvers"] + "name": "otel-arrow" } ] diff --git a/.github/scripts/pull-request-dashboard/test_dashboard.py b/.github/scripts/pull-request-dashboard/test_dashboard.py index f821f62a47f..6ceb0fc90f8 100644 --- a/.github/scripts/pull-request-dashboard/test_dashboard.py +++ b/.github/scripts/pull-request-dashboard/test_dashboard.py @@ -430,7 +430,6 @@ class BackfillFailureIsolationTest(unittest.TestCase): def test_failed_pr_does_not_block_later_backfill_progress(self) -> None: args = Namespace( repo="repo", - approver_team=["approvers"], state_branch="state", model="model", required_approvals=1, @@ -565,8 +564,6 @@ def test_emits_initial_backfill_status_only_for_accepted_state_outcomes(self) -> "state", "--repo", "repo", - "--approver-team", - "approvers", "--github-output", str(Path(temp_dir) / "output"), ], diff --git a/.github/scripts/pull-request-dashboard/test_github_cli.py b/.github/scripts/pull-request-dashboard/test_github_cli.py index efe20db47b8..112fbb88323 100644 --- a/.github/scripts/pull-request-dashboard/test_github_cli.py +++ b/.github/scripts/pull-request-dashboard/test_github_cli.py @@ -15,10 +15,25 @@ is_retryable_gh_error, list_all_open_pr_numbers, list_open_prs, + load_reviewer_set, ) class GithubCliTest(unittest.TestCase): + @patch("github_cli.gh_api") + def test_load_reviewer_set_uses_write_collaborators(self, api) -> None: + api.return_value = [{"login": "Writer"}, {"login": "Admin"}] + + self.assertEqual( + load_reviewer_set("open-telemetry/shared-workflows"), + {"writer", "admin"}, + ) + api.assert_called_once_with( + "/repos/open-telemetry/shared-workflows/collaborators?permission=push&per_page=100", + paginate=True, + token=None, + ) + @patch("github_cli.gh_graphql") def test_fetch_pr_issue_comments_paginates(self, graphql) -> None: graphql.side_effect = [ diff --git a/.github/workflows/pull-request-dashboard-repo.yml b/.github/workflows/pull-request-dashboard-repo.yml index a193f03b5c8..95d7f160fba 100644 --- a/.github/workflows/pull-request-dashboard-repo.yml +++ b/.github/workflows/pull-request-dashboard-repo.yml @@ -14,10 +14,6 @@ on: required: false default: 1 type: number - approver_teams_json: - required: false - default: "[]" - type: string non_blocking_check_patterns_json: required: false default: "[]" @@ -78,7 +74,6 @@ jobs: permission-checks: read permission-contents: read permission-issues: read - permission-members: read permission-pull-requests: read - name: Restore per-PR classification cache @@ -121,16 +116,12 @@ jobs: COPILOT_GITHUB_TOKEN: ${{ secrets.COPILOT_GITHUB_TOKEN }} REPO_NAME: ${{ inputs.repository }} REQUIRED_APPROVALS: ${{ inputs.required_approvals }} - APPROVER_TEAMS_JSON: ${{ inputs.approver_teams_json }} NON_BLOCKING_CHECK_PATTERNS_JSON: ${{ inputs.non_blocking_check_patterns_json }} run: | set -euo pipefail state_branch="${DASHBOARD_STATE_BRANCH_PREFIX}/${REPO_NAME}" dashboard_args=(--state-branch "$state_branch" --repo "$REPO_NAME" --github-output "$GITHUB_OUTPUT") dashboard_args+=(--required-approvals "$REQUIRED_APPROVALS") - for team in $(jq -r '.[]' <<< "$APPROVER_TEAMS_JSON"); do - dashboard_args+=(--approver-team "$team") - done mapfile -t non_blocking_check_patterns < <(jq -r '.[]' <<< "$NON_BLOCKING_CHECK_PATTERNS_JSON") for pattern in "${non_blocking_check_patterns[@]}"; do dashboard_args+=(--non-blocking-check-pattern "$pattern") diff --git a/.github/workflows/pull-request-dashboard.yml b/.github/workflows/pull-request-dashboard.yml index d84d501e5d0..efc5347c4c8 100644 --- a/.github/workflows/pull-request-dashboard.yml +++ b/.github/workflows/pull-request-dashboard.yml @@ -146,7 +146,6 @@ jobs: repository: ${{ matrix.name }} pr_number: ${{ needs.resolve-targets.outputs.pr_number }} required_approvals: ${{ matrix.required_approvals || 1 }} - approver_teams_json: ${{ toJSON(matrix.approver_teams || fromJSON('[]')) }} non_blocking_check_patterns_json: ${{ toJSON(matrix.non_blocking_check_patterns || fromJSON('[]')) }} slack_channel: ${{ matrix.slack_channel }} slack_user_mapping_json: ${{ toJSON(matrix.slack_user_mapping || fromJSON('{}')) }} diff --git a/pull-request-dashboard/README.md b/pull-request-dashboard/README.md index 8b78439c1a2..440f405528c 100644 --- a/pull-request-dashboard/README.md +++ b/pull-request-dashboard/README.md @@ -15,8 +15,8 @@ The dashboard groups open non-draft pull requests by who is expected to act next - **PR** — Pull request number and title, followed by any configured matching labels. The number autolinks to the PR on GitHub. Configured labels are rendered inline for both active and draft PRs. - **Author** — GitHub login of the PR author. - **Reviewers** — Reviewers who have engaged with the PR, each annotated with one or more icons: - - ✅ approved - - ✔️ approved (non-code-owner — does **not** count toward merge requirements) + - ✅ approved with repository write access + - ✔️ approved without repository write access (does **not** count toward the dashboard's required approvals) - 💬 has an open (unresolved) review thread on the PR - 📌 has tracked top-level feedback that still needs author action - 🔴 requested changes @@ -46,7 +46,6 @@ Open a pull request that adds your repository to [`.github/scripts/pull-request- [ { "name": "example-repo", - "approver_teams": ["example-approvers"], "required_approvals": 1, "non_blocking_check_patterns": [ "markdown-link-check / link-check", @@ -66,7 +65,6 @@ Fields: | Field | Required | Description | | ----- | -------- | ----------- | | `name` | yes | Name of the repository under `open-telemetry`. | -| `approver_teams` | yes | GitHub team slugs whose members count as approvers. | | `required_approvals` | no | Number of approvals required for an open PR to be marked ready to merge. Defaults to `1`. | | `labels_to_display` | no | Case-sensitive shell-style label name patterns to display inline after PR titles. Exact names such as `breaking change` and wildcard patterns such as `size/*` are supported. Defaults to `[]`, which displays no labels. | | `non_blocking_check_patterns` | no | Check-name globs for non-required checks whose failures should be identified in the live PR status comment. When the PR is waiting on the author, matching failures are reported only when at least one required check is failing and are noted alongside those failures. On other routes, matching failures are shown separately. Matching checks remain informational and do not affect routing or the dashboard CI column. | @@ -187,7 +185,8 @@ Targeted updates received before the first full dashboard run are ignored. ## Configuration The dashboard uses repository-scoped GitHub App access to read and update each -configured repository and to read approver team membership. +configured repository. Approvals from collaborators with write access or higher +count toward `required_approvals`. Each repository can route Slack notifications to its own `slack_channel` and map GitHub logins to Slack user IDs via `slack_user_mapping`. Repositories