rulesets: implement allowed-merge-apps by marcoieni · Pull Request #2304 · rust-lang/team

marcoieni · 2026-03-09T22:35:20Z

marcoieni · 2026-03-09T22:36:30Z

repos/rust-lang/crates.io.toml

+    "Frontend / Lint",
+    "Frontend / Test",
+]
+required-approvals = 0


It was already present in the branch protection. See #2046

marcoieni · 2026-03-09T22:39:43Z

docs/toml-schema.md

+# GitHub App IDs that are allowed to bypass the branch protection rules.
+# The app ID can be found via the GitHub API (GET /orgs/{org}/installations).
+# (optional)
+bypass-app-ids = [2201425] # workflows-crates-io


for simplicity, I put directly the ID, to avoid having another rest API to call GitHub to query for the ID.
Also, this app is only used in crates-io, so I thought adding an enum between app name and ID was overengineering. We can refine this later if needed.

You can see from the screenshot of the issue linked in the PR body that this app is in the bypass list.

Tbh I would rather add the enum variant to document what are all the integrations that we care about (and hardcode the app ID in code, same as we do for bors and Renovatebot).

github-actions · 2026-03-09T22:50:54Z

Dry-run check results


[WARN  sync_team] sync-team is running in dry mode, no changes will be applied.
[INFO  sync_team] synchronizing crates-io
[INFO  sync_team] synchronizing github
[INFO  sync_team] 💻 Team Diffs:
    📝 Editing team 'rust-lang/all':
      Adding member 'edmilsonefs' with member role
      Adding member 'graciegregory' with member role
    📝 Editing team 'rust-lang/survey':
      Adding member 'graciegregory' with member role
    📝 Editing team 'rust-lang/triage':
      Adding member 'edmilsonefs' with member role

marcoieni · 2026-03-09T22:51:03Z

sync-team/src/github/api/mod.rs

    OrganizationAdmin,
    RepositoryRole,
    Team,
+    DeployKey,


From https://docs.github.com/en/rest/repos/rules?apiVersion=2022-11-28#get-a-repository-ruleset:

marcoieni · 2026-03-09T23:05:54Z

repos/rust-lang/crates.io.toml

 crates-io = "write"

+[[branch-protections]]
+name = "main"


adding this to minimize the sync team dry run diff

marcoieni · 2026-03-09T23:13:37Z

docs/toml-schema.md

@@ -429,6 +433,19 @@ allowed-merge-teams = ["awesome-team"]
 # When "homu" is used, "bors" has to be in the `bots` array.
 # (optional)
 merge-bots = ["homu"]


@Kobzol one doubt I have: does it makes sense to have merge-bots and bypass-app-ids separated?

No, I don't think so. I would suggest merging (:laughing:) merge-bots and bypass-app-ids into allowed-merge-apps, to mirror the already existing allowed-merge-teams.

If I delete merge-bots than retrocompatibility becomes a mess. I.e. I need to change all repos files 🤔
Should we do it in another PR?
Should I momentarely add this github app in "merge-bots"? and then rename it in a separate PR?

merge-bots is currently used only on rust-lang/rust and nowhere else, so I don't see why we would need to change all repo files 🤔

marcoieni · 2026-03-09T23:28:26Z

Here is the dry run diff:

Rulesets:
          Updating 'main'
            Include Branches: ["~DEFAULT_BRANCH"] => ["refs/heads/main"]
            Bypass Actors: None => Some([RulesetBypassActor { actor_id: 2201425, actor_type: Integration, bypass_mode: Always }])

updating the branch to a fixed one from the default is fine.
The bypass actors are changing, but I wonder if this is because the dry run read token doesn't have the read permission for the bypass actors.

EDIT: AI confirmed that's the case

Kobzol · 2026-03-10T10:45:22Z

sync-team/src/github/mod.rs

+    }
+
+    // Add non-fast-forward protection if requested
+    if branch_protection.prevent_force_push {


This is not for this PR, but I think that those two (along with forbid creation) should be the default.

marcoieni · 2026-03-10T11:52:21Z

rust_team_data/src/v1.rs

 #[derive(Debug, Clone, Serialize, Deserialize, PartialEq)]
 #[serde(rename_all = "snake_case")]
 pub enum MergeBot {
-    Homu,


I removed homu because it was unused.
Also I kept the "MergeBot" name here to minimize LoC changed for this PR

marcoieni · 2026-03-10T12:01:06Z

Not sure why after my latest changes the dry run diff increased:


    📝 Editing repo 'rust-lang/crates.io':
      Rulesets:
          Updating 'main'
            Include Branches: ["~DEFAULT_BRANCH"] => ["refs/heads/main"]
            Bypass Actors: None => Some([RulesetBypassActor { actor_id: 2201425, actor_type: Integration, bypass_mode: Always }])
            Require code owner review: deleting `false`
            Required status checks: deleting `Backend / Lint (integration_id: 15368), Backend / Test (integration_id: 15368), Backend / dependencies (integration_id: 15368), Frontend / Lint (integration_id: 15368), Frontend / Test (integration_id: 15368)`
            Strict policy for status checks: deleting `false`
            Required approvals: deleting `0`
            Dismiss stale reviews on push: deleting `false`
            Require review thread resolution: deleting `false`
            Require last push approval: deleting `false`

marcoieni · 2026-03-10T12:07:15Z

I think AI is right:

I'm investigating how I can fix this

marcoieni · 2026-03-10T12:20:48Z

repos/rust-lang/rust.toml

 [[branch-protections]]
 pattern = "main"
-merge-bots = ["bors"]
+pr-required = false


I decided to setup this field explicitly instead of inferring it via code (in case the app is bors).
But let's see the dry run.

Kobzol · 2026-03-10T12:32:11Z

sync-team/src/github/mod.rs

    use api::*;

-    let uses_merge_bot = !branch_protection.merge_bots.is_empty();
+    let uses_merge_bot = !branch_protection.allowed_merge_apps.is_empty();


This should be changed to check if allowed_merge_apps contains Bors (not just that it is empty). That should fix the diff.

yes, but I thought that having the config more explicit was better instead of this custom logic. Wdyt? 🤔

I think that this logic is fine. The high-level config should be "bors managed this branch", and then team should do whatever is necessary to make that happen. Setting pr-required = false just exposes (just one) implementation detail of that. So I'd rather just say that bors can push to this branch, and interpret that is it being bors managed, and set the various protection options accordingly.

ok, I'll change this

rustbot · 2026-03-10T12:50:38Z

This PR was rebased onto a different main commit. Here's a range-diff highlighting what actually changed.

Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers.

marcoieni · 2026-03-10T13:19:49Z

The dry run looks good now:


    📝 Editing repo 'rust-lang/crates.io':
      Rulesets:
          Updating 'main'
            Include Branches: ["~DEFAULT_BRANCH"] => ["refs/heads/main"]
            Bypass Actors: None => Some([RulesetBypassActor { actor_id: 2201425, actor_type: Integration, bypass_mode: Always }])

Kobzol · 2026-03-10T20:34:49Z

rust_team_data/src/v1.rs

-    Homu,
    RustTimer,
    Bors,
+    WorkflowsCratesIo,


This will break triagebot. I think that we should split this PR into two, make the first backwards-compatible by just adding the new code, then merge it, update triagebot and only then start using the new variants.

is 5ec5088 what you had in mind?

It's not enough, because after this PR gets merged as-is, the API will return the crates-io-workflows variant of this enum, and triagebot will fail to deserialize it.

Kobzol · 2026-03-10T20:35:59Z

rust_team_data/src/v1.rs

    pub mode: BranchProtectionMode,
    pub allowed_merge_teams: Vec<String>,
-    pub merge_bots: Vec<MergeBot>,
+    pub allowed_merge_apps: Vec<MergeBot>,


Kind of a meta-point: I wonder if we should stop using the V1 API in sync-team. We often break the API by making changes that are only required for sync-team, but now that we have sync-team in-tree, we could just directly work on the TOML representation in it. Other tools don't need to know about allowed merge apps or even branch protections.

I don't know how other tools use the API. From me, +1 on anything that makes this repo simpler 👍

ubiratansoares

Awesome work here!

marcoieni commented Mar 9, 2026

View reviewed changes

marcoieni force-pushed the rulesets-implement-bypass-app-ids branch 2 times, most recently from fa95128 to c6e6481 Compare March 9, 2026 22:50

marcoieni commented Mar 9, 2026

View reviewed changes

marcoieni force-pushed the rulesets-implement-bypass-app-ids branch 3 times, most recently from 39525b0 to 5bedc6d Compare March 9, 2026 23:05

marcoieni commented Mar 9, 2026

View reviewed changes

marcoieni force-pushed the rulesets-implement-bypass-app-ids branch 2 times, most recently from 44eeeb4 to c7b6732 Compare March 9, 2026 23:10

marcoieni commented Mar 9, 2026

View reviewed changes

marcoieni marked this pull request as ready for review March 9, 2026 23:28

marcoieni requested review from Mark-Simulacrum, emilyalbini, jdno and ubiratansoares as code owners March 9, 2026 23:28

Kobzol reviewed Mar 10, 2026

View reviewed changes

marcoieni commented Mar 10, 2026

View reviewed changes

Kobzol reviewed Mar 10, 2026

View reviewed changes

marcoieni force-pushed the rulesets-implement-bypass-app-ids branch from eea0ed8 to 9475511 Compare March 10, 2026 12:50

marcoieni changed the title ~~rulesets: implement bypass-app-ids~~ rulesets: implement allowed-merge-apps Mar 10, 2026

marcoieni force-pushed the rulesets-implement-bypass-app-ids branch from 9475511 to 7b29da4 Compare March 10, 2026 12:51

marcoieni force-pushed the rulesets-implement-bypass-app-ids branch 2 times, most recently from 0307de4 to 063d41d Compare March 10, 2026 14:41

marcoieni requested a review from Kobzol March 10, 2026 14:56

Kobzol reviewed Mar 10, 2026

View reviewed changes

marcoieni requested a review from Kobzol March 10, 2026 21:04

marcoieni force-pushed the rulesets-implement-bypass-app-ids branch 3 times, most recently from a0d17e9 to 53796fa Compare March 10, 2026 22:31

Kobzol approved these changes Mar 10, 2026

View reviewed changes

marcoieni force-pushed the rulesets-implement-bypass-app-ids branch from 53796fa to ca0e2af Compare March 10, 2026 22:45

rulesets: implement allowed-merge-apps

14a6ef7

marcoieni force-pushed the rulesets-implement-bypass-app-ids branch from ca0e2af to 14a6ef7 Compare March 10, 2026 22:58

ubiratansoares approved these changes Mar 11, 2026

View reviewed changes

marcoieni added this pull request to the merge queue Mar 11, 2026

marcoieni mentioned this pull request Mar 11, 2026

feat: implement ruleset bypass actor #2199

Closed

Merged via the queue into main with commit a0aefd8 Mar 11, 2026
3 checks passed

This was referenced Mar 12, 2026

Change rulesets defaults #2313

Closed

Implement bypass actors for rulesets based on allowed_merge_teams and merge_bots #2192

Closed

Conversation

marcoieni commented Mar 9, 2026

Uh oh!

marcoieni Mar 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

marcoieni Mar 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

github-actions bot commented Mar 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

marcoieni commented Mar 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

marcoieni commented Mar 10, 2026

Uh oh!

marcoieni commented Mar 10, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rustbot commented Mar 10, 2026

Uh oh!

marcoieni commented Mar 10, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ubiratansoares left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

marcoieni Mar 9, 2026 •

edited

Loading

marcoieni Mar 9, 2026 •

edited

Loading

github-actions bot commented Mar 9, 2026 •

edited

Loading

marcoieni commented Mar 9, 2026 •

edited

Loading