From f6700ccd2491cc1cdc35d5dc377517ef84bcd099 Mon Sep 17 00:00:00 2001 From: Elior Erez Date: Mon, 2 Nov 2026 09:00:51 -0500 Subject: [PATCH 1/6] NO-ISSUE: Do not pick a maintainer for each pull request Turn review request delegation off for the maintainers team: any member may approve, and GitHub must not pick one of them. The resource keeps no review_request_delegation block, so every apply keeps it off. Notifications are the team's notification setting, which the provider cannot manage; the README says how it is set. Signed-off-by: Elior Erez --- README.md | 5 +++++ teams.tf | 12 +++++------- 2 files changed, 10 insertions(+), 7 deletions(-) diff --git a/README.md b/README.md index cd860bf..0661158 100644 --- a/README.md +++ b/README.md @@ -33,6 +33,11 @@ The team then becomes a child of the `maintainers` team, whose members are every The point of the team is `osac`'s CODEOWNERS: a follow-up change to that file will make `@osac-project/maintainers` the owner of every path that no other rule names, so that a person in one of these teams has to approve it and a bot (CodeRabbit included) never counts. Until that change is merged, the team is only created, and paths with no owner can still be approved by a bot. +**Notifications and review assignment.** A pull request requests `maintainers` as a team, and every member may approve. Two settings decide who is told, and both are set so that nobody is picked and nobody is broadcast to: + +- *Review request delegation* (GitHub: Code review, "Enable auto assignment") is **off**. It would pick one member of the team for each pull request. `teams.tf` keeps it off: the `github_team_settings` resource has no `review_request_delegation` block (the provider enables delegation when the block is present), so every apply turns it off again. +- The team's *notification setting* is `notifications_disabled`, so the members are not notified when the team is requested. The provider has no argument for it, so Terraform neither sets nor reverts it: set it with `gh api -X PATCH orgs/osac-project/teams/maintainers -f notification_setting=notifications_disabled` (or Team settings, Notifications) and check that it stays. + ### Add an organization member to a team? 1. Open `team-members/.csv` diff --git a/teams.tf b/teams.tf index 0aa444e..54f8447 100644 --- a/teams.tf +++ b/teams.tf @@ -21,15 +21,13 @@ resource "github_team" "maintainers" { privacy = "closed" } +# The code review settings of the maintainers team: review request delegation is OFF. CODEOWNERS requests this team for +# every path no working group owns, and any member may approve; GitHub must not pick one member of it for the pull request +# (the provider enables delegation when a review_request_delegation block is present, and this resource has none, so every +# apply keeps it off). Whether members are notified is the team's notification setting (notification_setting of the team), +# which the provider cannot manage: it is set to "notifications_disabled" through the API, see README.md. resource "github_team_settings" "maintainers" { team_id = github_team.maintainers.id - # Team-only requests still email members; assign one reviewer so notify = false suppresses whole-team notifications. - notify = false - - review_request_delegation { - algorithm = "LOAD_BALANCE" - member_count = 1 - } } # Populate team members based on the csv files in the team-members directory. From 86ea87525e5de2f2b96cf80067fd5b6e8ab4461d Mon Sep 17 00:00:00 2001 From: Elior Erez Date: Mon, 2 Nov 2026 09:18:42 -0500 Subject: [PATCH 2/6] NO-ISSUE: Manage the notification setting of the maintainers team The provider sets notification_setting on every apply and defaults it to notifications_enabled, so a setting changed by hand is undone at the next apply. State it as notifications_disabled in teams.tf, and correct the README, which said the provider cannot manage it. Signed-off-by: Elior Erez --- README.md | 2 +- teams.tf | 12 +++++++----- 2 files changed, 8 insertions(+), 6 deletions(-) diff --git a/README.md b/README.md index 0661158..3ad667a 100644 --- a/README.md +++ b/README.md @@ -36,7 +36,7 @@ The point of the team is `osac`'s CODEOWNERS: a follow-up change to that file wi **Notifications and review assignment.** A pull request requests `maintainers` as a team, and every member may approve. Two settings decide who is told, and both are set so that nobody is picked and nobody is broadcast to: - *Review request delegation* (GitHub: Code review, "Enable auto assignment") is **off**. It would pick one member of the team for each pull request. `teams.tf` keeps it off: the `github_team_settings` resource has no `review_request_delegation` block (the provider enables delegation when the block is present), so every apply turns it off again. -- The team's *notification setting* is `notifications_disabled`, so the members are not notified when the team is requested. The provider has no argument for it, so Terraform neither sets nor reverts it: set it with `gh api -X PATCH orgs/osac-project/teams/maintainers -f notification_setting=notifications_disabled` (or Team settings, Notifications) and check that it stays. +- The team's *notification setting* is `notifications_disabled` (`notification_setting` on the team in `teams.tf`), so the members are not notified when the team is requested. The provider sets this field on every apply and defaults it to `notifications_enabled`, so it must be in the configuration: a change made by hand in the team's settings is undone at the next apply. ### Add an organization member to a team? diff --git a/teams.tf b/teams.tf index 54f8447..a42d444 100644 --- a/teams.tf +++ b/teams.tf @@ -16,16 +16,18 @@ resource "github_team" "all" { # working group", and a bot, which is in none of them, is not one. Child teams inherit the access of their parent; the # parent only has push on osac (see repositories.tf), which they all have already. resource "github_team" "maintainers" { - name = "maintainers" - description = "Everyone who may approve changes: the members of the working-group teams (their parent team)" - privacy = "closed" + name = "maintainers" + description = "Everyone who may approve changes: the members of the working-group teams (their parent team)" + privacy = "closed" + notification_setting = "notifications_disabled" # see the comment below: nobody is told when the team is requested } # The code review settings of the maintainers team: review request delegation is OFF. CODEOWNERS requests this team for # every path no working group owns, and any member may approve; GitHub must not pick one member of it for the pull request # (the provider enables delegation when a review_request_delegation block is present, and this resource has none, so every -# apply keeps it off). Whether members are notified is the team's notification setting (notification_setting of the team), -# which the provider cannot manage: it is set to "notifications_disabled" through the API, see README.md. +# apply keeps it off). Notifications are the notification_setting of the team above: CODEOWNERS requests this team on every +# pull request no working group owns, and nobody is to be told. The provider defaults that field to "notifications_enabled" +# and sets it on every apply, so it has to be stated. resource "github_team_settings" "maintainers" { team_id = github_team.maintainers.id } From 03fa52ea8e86b1f291eeba3d96c7b9e3555dbbf4 Mon Sep 17 00:00:00 2001 From: Elior Erez Date: Mon, 2 Nov 2026 09:44:56 -0500 Subject: [PATCH 3/6] NO-ISSUE: Assign osac-ci-bot, not a maintainer, to maintainers reviews The notification setting of a team does not cover review requests, so drop it. Make osac-ci-bot the only direct member of maintainers and set review assignment after every apply (the provider cannot set includeChildTeamMembers): it picks the bot, notifies only the bot, and leaves the members of the working groups out. Any of them may still approve. github_team_settings is forgotten, not destroyed. Signed-off-by: Elior Erez --- .github/workflows/apply.yaml | 26 ++++++++++++++++++++++++++ README.md | 8 +++++--- teams.tf | 36 ++++++++++++++++++++++++------------ 3 files changed, 55 insertions(+), 15 deletions(-) diff --git a/.github/workflows/apply.yaml b/.github/workflows/apply.yaml index 703f7cb..ac9ca20 100644 --- a/.github/workflows/apply.yaml +++ b/.github/workflows/apply.yaml @@ -166,6 +166,32 @@ jobs: env: GITHUB_TOKEN: ${{ steps.generate-token.outputs.token }} EXCLUDE_ADDRESSES: ${{ inputs.exclude_addresses }} + # Review assignment of the maintainers team: GitHub assigns one DIRECT member of the team (osac-ci-bot, see teams.tf) to + # a pull request that CODEOWNERS sends to the team, and notifies only that member, not the whole team. The provider + # (github_team_settings) cannot set includeChildTeamMembers, which is what leaves the members of the working-group + # teams out of the assignment, so it is set here, on every apply: the mutation is idempotent, and it puts the setting + # back if someone changed it by hand. Not readable back (the team has no field for includeChildTeamMembers). + - name: Maintainers team review assignment + if: ${{ steps.tofu_apply.outcome == 'success' }} + env: + GH_TOKEN: ${{ steps.generate-token.outputs.token }} + ORG: ${{ github.repository_owner }} + run: | + set -euo pipefail + team_id=$(gh api graphql -f org="${ORG}" -f query=' + query($org: String!) { organization(login: $org) { team(slug: "maintainers") { id } } }' \ + --jq '.data.organization.team.id // empty') + if [[ -z "${team_id}" ]]; then + echo "::error::The maintainers team was not found." >&2 + exit 1 + fi + gh api graphql -f id="${team_id}" -f query=' + mutation($id: ID!) { + updateTeamReviewAssignment(input: { + id: $id, enabled: true, algorithm: LOAD_BALANCE, teamMemberCount: 1, + notifyTeam: false, includeChildTeamMembers: false, removeTeamRequest: false + }) { team { slug reviewRequestDelegationEnabled reviewRequestDelegationMemberCount reviewRequestDelegationNotifyTeam } } + }' --jq '.data.updateTeamReviewAssignment.team' # tofu apply isn't atomic and doesn't roll back changes already # applied earlier in the same run if a later resource fails -- but a # hard error on one resource does still stop the run before anything diff --git a/README.md b/README.md index 3ad667a..daaa6e2 100644 --- a/README.md +++ b/README.md @@ -33,10 +33,12 @@ The team then becomes a child of the `maintainers` team, whose members are every The point of the team is `osac`'s CODEOWNERS: a follow-up change to that file will make `@osac-project/maintainers` the owner of every path that no other rule names, so that a person in one of these teams has to approve it and a bot (CodeRabbit included) never counts. Until that change is merged, the team is only created, and paths with no owner can still be approved by a bot. -**Notifications and review assignment.** A pull request requests `maintainers` as a team, and every member may approve. Two settings decide who is told, and both are set so that nobody is picked and nobody is broadcast to: +**Notifications and review assignment.** A pull request that CODEOWNERS sends to `maintainers` requests the team, and every member of a working group may approve. GitHub notifies every member of a team that is requested for review, and its team notification setting does not change that (it only covers @mentions). The only switch is *review assignment* (Code review in the team's settings): GitHub assigns members and notifies only them. So that nobody is picked and nobody is broadcast to: -- *Review request delegation* (GitHub: Code review, "Enable auto assignment") is **off**. It would pick one member of the team for each pull request. `teams.tf` keeps it off: the `github_team_settings` resource has no `review_request_delegation` block (the provider enables delegation when the block is present), so every apply turns it off again. -- The team's *notification setting* is `notifications_disabled` (`notification_setting` on the team in `teams.tf`), so the members are not notified when the team is requested. The provider sets this field on every apply and defaults it to `notifications_enabled`, so it must be in the configuration: a change made by hand in the team's settings is undone at the next apply. +- `osac-ci-bot` is the only **direct** member of `maintainers` (`github_team_membership.maintainers_review_sink` in `teams.tf`). It approves nothing. +- Review assignment is on for `maintainers`: `LOAD_BALANCE`, one member, the team is not notified, and the members of the child teams are **left out** (`includeChildTeamMembers: false`), so the assignment always picks the bot. The pull request lists `maintainers` and `osac-ci-bot` as reviewers, and the bot's request stays pending (nothing waits for it). Approval is not affected: members of a child team count as members of `maintainers` for CODEOWNERS. +- The provider cannot set `includeChildTeamMembers` (and the team has no field to read it back), so `.github/workflows/apply.yaml` sets all of it with the GraphQL mutation `updateTeamReviewAssignment` after every apply, scheduled ones included. `github_team_settings.maintainers` is gone from the configuration (`removed` block, nothing destroyed). +- The teams that own a folder in CODEOWNERS (`wg-infra`, `wg-vmaas`, ...) are requested and notified as usual. ### Add an organization member to a team? diff --git a/teams.tf b/teams.tf index a42d444..4c4fe5c 100644 --- a/teams.tf +++ b/teams.tf @@ -16,20 +16,32 @@ resource "github_team" "all" { # working group", and a bot, which is in none of them, is not one. Child teams inherit the access of their parent; the # parent only has push on osac (see repositories.tf), which they all have already. resource "github_team" "maintainers" { - name = "maintainers" - description = "Everyone who may approve changes: the members of the working-group teams (their parent team)" - privacy = "closed" - notification_setting = "notifications_disabled" # see the comment below: nobody is told when the team is requested + name = "maintainers" + description = "Everyone who may approve changes: the members of the working-group teams (their parent team)" + privacy = "closed" +} + +# The only DIRECT member of maintainers. A pull request that CODEOWNERS sends to maintainers has GitHub assign one member of +# the team to review it (review assignment, set after every apply, see .github/workflows/apply.yaml), and only that member is +# notified, not the 80-odd people of the working groups. The assignment is limited to direct members (the members of the +# child teams are left out), so it always picks this account, which approves nothing: nobody is picked, nobody is +# broadcast to, and any member of a working group may still approve (members of a child team count as members of +# maintainers for CODEOWNERS). osac-ci-bot is already a member of the organization and of wg-infra. +resource "github_team_membership" "maintainers_review_sink" { + team_id = github_team.maintainers.id + username = "osac-ci-bot" + role = "member" } -# The code review settings of the maintainers team: review request delegation is OFF. CODEOWNERS requests this team for -# every path no working group owns, and any member may approve; GitHub must not pick one member of it for the pull request -# (the provider enables delegation when a review_request_delegation block is present, and this resource has none, so every -# apply keeps it off). Notifications are the notification_setting of the team above: CODEOWNERS requests this team on every -# pull request no working group owns, and nobody is to be told. The provider defaults that field to "notifications_enabled" -# and sets it on every apply, so it has to be stated. -resource "github_team_settings" "maintainers" { - team_id = github_team.maintainers.id +# Review assignment used to be managed here (github_team_settings), but the provider cannot set the option that leaves the +# child teams out of the assignment, and an apply would undo it. It is set by apply.yaml instead; forget the old resource +# without destroying it. +removed { + from = github_team_settings.maintainers + + lifecycle { + destroy = false + } } # Populate team members based on the csv files in the team-members directory. From d7d5e732d5d924e88e87205ab2be821c21faef99 Mon Sep 17 00:00:00 2001 From: Elior Erez Date: Mon, 2 Nov 2026 10:04:27 -0500 Subject: [PATCH 4/6] NO-ISSUE: Set review assignment even after a failed apply A broken resource elsewhere must not hold the maintainers review assignment back, so the step runs after a failed apply too. It first asks for the direct members of the team and does nothing unless osac-ci-bot is one of them: with nobody eligible the assignment would have no one to pick. Signed-off-by: Elior Erez --- .github/workflows/apply.yaml | 21 ++++++++++++++++----- 1 file changed, 16 insertions(+), 5 deletions(-) diff --git a/.github/workflows/apply.yaml b/.github/workflows/apply.yaml index ac9ca20..08d85e8 100644 --- a/.github/workflows/apply.yaml +++ b/.github/workflows/apply.yaml @@ -170,21 +170,32 @@ jobs: # a pull request that CODEOWNERS sends to the team, and notifies only that member, not the whole team. The provider # (github_team_settings) cannot set includeChildTeamMembers, which is what leaves the members of the working-group # teams out of the assignment, so it is set here, on every apply: the mutation is idempotent, and it puts the setting - # back if someone changed it by hand. Not readable back (the team has no field for includeChildTeamMembers). + # back if someone changed it by hand. It is not readable back (the team has no field for includeChildTeamMembers). + # It also runs after an apply that failed on something else (a broken resource must not hold the setting back), but + # only when osac-ci-bot is a direct member of the team: with nobody eligible, the assignment has no one to pick. - name: Maintainers team review assignment - if: ${{ steps.tofu_apply.outcome == 'success' }} + if: ${{ !cancelled() && steps.tofu_apply.outcome != 'skipped' }} env: GH_TOKEN: ${{ steps.generate-token.outputs.token }} ORG: ${{ github.repository_owner }} + SINK: osac-ci-bot run: | set -euo pipefail - team_id=$(gh api graphql -f org="${ORG}" -f query=' - query($org: String!) { organization(login: $org) { team(slug: "maintainers") { id } } }' \ - --jq '.data.organization.team.id // empty') + answer=$(gh api graphql -f org="${ORG}" -f query=' + query($org: String!) { + organization(login: $org) { + team(slug: "maintainers") { id members(membership: IMMEDIATE, first: 100) { nodes { login } } } + } + }') + team_id=$(jq -r '.data.organization.team.id // empty' <<<"${answer}") if [[ -z "${team_id}" ]]; then echo "::error::The maintainers team was not found." >&2 exit 1 fi + if ! jq -e --arg sink "${SINK}" '.data.organization.team.members.nodes | map(.login) | index($sink)' <<<"${answer}" >/dev/null; then + echo "::warning::${SINK} is not a direct member of maintainers (yet): the review assignment is left as it is." + exit 0 + fi gh api graphql -f id="${team_id}" -f query=' mutation($id: ID!) { updateTeamReviewAssignment(input: { From 8f1414bb1fc266e104bd526fbc92a59b77e0affd Mon Sep 17 00:00:00 2001 From: Elior Erez Date: Mon, 2 Nov 2026 10:06:19 -0500 Subject: [PATCH 5/6] NO-ISSUE: Fail the apply if GitHub does not keep the assignment Read back what GitHub says it stored after the mutation (enabled, one member, team not notified) and fail the run when it is not what was asked, so a setting that did not take is not silent. Signed-off-by: Elior Erez --- .github/workflows/apply.yaml | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/.github/workflows/apply.yaml b/.github/workflows/apply.yaml index 08d85e8..22ef429 100644 --- a/.github/workflows/apply.yaml +++ b/.github/workflows/apply.yaml @@ -202,7 +202,10 @@ jobs: id: $id, enabled: true, algorithm: LOAD_BALANCE, teamMemberCount: 1, notifyTeam: false, includeChildTeamMembers: false, removeTeamRequest: false }) { team { slug reviewRequestDelegationEnabled reviewRequestDelegationMemberCount reviewRequestDelegationNotifyTeam } } - }' --jq '.data.updateTeamReviewAssignment.team' + }' --jq '.data.updateTeamReviewAssignment.team' | tee "${RUNNER_TEMP}/review-assignment.json" + # What GitHub says it stored (includeChildTeamMembers is not part of the answer): fail the run if it is not what was asked. + jq -e '.reviewRequestDelegationEnabled == true and .reviewRequestDelegationMemberCount == 1 + and .reviewRequestDelegationNotifyTeam == false' "${RUNNER_TEMP}/review-assignment.json" >/dev/null # tofu apply isn't atomic and doesn't roll back changes already # applied earlier in the same run if a later resource fails -- but a # hard error on one resource does still stop the run before anything From 45577cde7306057555fe29defd2b655eba016661 Mon Sep 17 00:00:00 2001 From: Elior Erez Date: Mon, 2 Nov 2026 10:28:35 -0500 Subject: [PATCH 6/6] NO-ISSUE: Assign only while the bot is the sole direct member Only direct members of maintainers can be assigned, so any other direct member would be picked for pull requests. The step now fails when it finds one besides osac-ci-bot, and still leaves the assignment alone when the bot is not a member (yet). The README says so. Signed-off-by: Elior Erez --- .github/workflows/apply.yaml | 7 +++++++ README.md | 2 +- 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/.github/workflows/apply.yaml b/.github/workflows/apply.yaml index 22ef429..5a04a2d 100644 --- a/.github/workflows/apply.yaml +++ b/.github/workflows/apply.yaml @@ -192,10 +192,17 @@ jobs: echo "::error::The maintainers team was not found." >&2 exit 1 fi + direct=$(jq -r '.data.organization.team.members.nodes | map(.login) | join(" ")' <<<"${answer}") if ! jq -e --arg sink "${SINK}" '.data.organization.team.members.nodes | map(.login) | index($sink)' <<<"${answer}" >/dev/null; then echo "::warning::${SINK} is not a direct member of maintainers (yet): the review assignment is left as it is." exit 0 fi + # Only direct members can be assigned, so the assignment stays harmless only while the bot is the ONLY one: anyone + # else added to the team directly would be picked for pull requests. + if [[ "${direct}" != "${SINK}" ]]; then + echo "::error::maintainers has direct members besides ${SINK} (${direct}): they would be assigned to pull requests. Remove them from the team (its members are the working-group teams), then run this again." >&2 + exit 1 + fi gh api graphql -f id="${team_id}" -f query=' mutation($id: ID!) { updateTeamReviewAssignment(input: { diff --git a/README.md b/README.md index daaa6e2..8bc0d76 100644 --- a/README.md +++ b/README.md @@ -35,7 +35,7 @@ The point of the team is `osac`'s CODEOWNERS: a follow-up change to that file wi **Notifications and review assignment.** A pull request that CODEOWNERS sends to `maintainers` requests the team, and every member of a working group may approve. GitHub notifies every member of a team that is requested for review, and its team notification setting does not change that (it only covers @mentions). The only switch is *review assignment* (Code review in the team's settings): GitHub assigns members and notifies only them. So that nobody is picked and nobody is broadcast to: -- `osac-ci-bot` is the only **direct** member of `maintainers` (`github_team_membership.maintainers_review_sink` in `teams.tf`). It approves nothing. +- `osac-ci-bot` is the only **direct** member of `maintainers` (`github_team_membership.maintainers_review_sink` in `teams.tf`). It approves nothing. Nobody else may be added to `maintainers` directly (its members are the working-group teams): a direct member would be assigned to pull requests, and the apply step fails when it finds one. - Review assignment is on for `maintainers`: `LOAD_BALANCE`, one member, the team is not notified, and the members of the child teams are **left out** (`includeChildTeamMembers: false`), so the assignment always picks the bot. The pull request lists `maintainers` and `osac-ci-bot` as reviewers, and the bot's request stays pending (nothing waits for it). Approval is not affected: members of a child team count as members of `maintainers` for CODEOWNERS. - The provider cannot set `includeChildTeamMembers` (and the team has no field to read it back), so `.github/workflows/apply.yaml` sets all of it with the GraphQL mutation `updateTeamReviewAssignment` after every apply, scheduled ones included. `github_team_settings.maintainers` is gone from the configuration (`removed` block, nothing destroyed). - The teams that own a folder in CODEOWNERS (`wg-infra`, `wg-vmaas`, ...) are requested and notified as usual.