[Andrushika]

Why

#111 lets .asf.yaml turn on GitHub's pull request creation cap. GitHub also has a bypass list for that cap, but .asf.yaml cannot set it. Letting each project manage that list in .asf.yaml gives maintainers a way to keep the cap from blocking active contributors who do not have write access.

What

Add an optional bypass_users list under creation_cap.

• When present, asfyaml syncs GitHub's bypass list to match the yaml on every run.
• bypass_users: ~ clears the list. When the key is absent, the list is not touched.

creation_cap:
  enabled: true
  max_open_pull_requests: 5
  bypass_users:
    - octocat

Testing

Tested on a sandbox repo, checking the list through the API after each run.
Full suite passes (176 tests), ruff and mypy clean on the new code.

Generated-by: Claude Fable 5.1, reviewed and tested by me

[raulcd]

On Arrow we ended up implementing the creation_cap manually instead of relying on GitHub / waiting for this. We also added the ability of bypassing the limit for collaborators. Thanks for the PR, having a bypass_users list or a toggle like bypass_collaborators: True would be something we need in order to use the functionality.

[Andrushika]

Thanks! Good to know Arrow has been doing this by hand.
Out of curiosity, how does your implementation treat collaborators? Always bypassed, or opt-in?
I'm wondering whether a project would ever want the cap to apply to collaborators, which decides if this should be a toggle or just the default.

[potiuk]

Sandbox-tested at a45c4778599dc8d9c516bf753cf4bff4e30de6df against potiuk/infrastructure-asfyaml-sandbox, config on the sandbox's main. Branch needed no rebase; pytest 178 passed and pre-commit --all-files clean on the PR head.

Every "after" below is a read back from GET /repos/.../interaction-limits/pulls/bypass-list, not a log line.

Row	.asf.yaml	Bypass list before → after	Calls	Run
Set (noop)	[octocat, hubot], cap on/5	[] → []	none	35700315360
Set (live)	same	[] → hubot, octocat	PUT both	35700377540
Update	[defunkt, hubot]	defunkt, hubot, octocat → defunkt, hubot	PUT defunkt, DELETE octocat	35700446866
Idempotent	unchanged	no change	none — "already up to date"	35700519089
Key absent	creation_cap without bypass_users	defunkt, hubot, octocat → unchanged	none	35700617331
Case + duplicates	[DeFunkt, defunkt, HUBOT, OctoCat]	unchanged	none	35700699561
enabled: false + list	[octocat], cap off	defunkt, hubot, octocat → octocat	DELETE 2	35700784788
Clear	bypass_users: ~	octocat → []	DELETE octocat	35700901772
Unknown login	[octocat, asfyaml-no-such-user-9999]	[] → []	PUT → HTTP 422, run fails	35701068977

The hand-added octocat in the "key absent" row was put on the list through the API, never in the yaml — it survived the run, which is the behaviour the README promises.

Two non-blocking observations:

1. One unknown login discards the whole batch. GitHub's add is all-or-nothing: PUT with [octocat, asfyaml-no-such-user-9999] returned 422 Could not find all specified users. and added neither. The parser surfaces it correctly and the run fails, but by then the cap PATCH has already applied (max_open_pull_requests was 9 afterwards), and the message doesn't say which login was wrong. For a project with a 40-name list, a single typo means reading the yaml by eye. Checking each login (GET /users/{login}) before the PUT, or naming the offenders on a 422, would turn that into a usable error. I verified the 422 is GitHub's, not the parser's — a hand-written gh api -X PUT ... -f 'users[]=asfyaml-no-such-user-9999' fails identically.

2. enabled: false still reconciles the bypass list. Row 7 above: the cap was turned off and the list was still synced to [octocat], which GitHub accepts and keeps. That seems like the right call — the list survives the cap being toggled — but README doesn't mention it, and someone reading "Set enabled: false to turn the cap off" wouldn't expect bypass_users to still be applied. One sentence would cover it.

Not covered: the section-removal path (elif was_previously_configured). previous_yaml comes from the /x1/asfyaml cache, which does not exist outside production — the runs log CACHE Path '/x1/asfyaml' does not exist, skip caching. The unit tests cover that branch; the sandbox structurally cannot.

Sandbox reset afterwards and confirmed through the API: baseline .asf.yaml, cap {"enabled": false, "max_open_pull_requests": 7}, bypass list empty.

The matrix passes and README is updated, so this is an approve from me, with the two points above as optional follow-ups.

[potiuk]

cc: @Humbedooh -> that one is almost ready - there is one nit to fix @Andrushika will fix it shortly I guess :)

[raulcd]

Always bypassed, or opt-in? I'm wondering whether a project would ever want the cap to apply to collaborators, which decides if this should be a toggle or just the default.

In our specific case if someone has been given triage (collaborator) allowing them to not have the cap by default seems reasonable. I don't think I can foresee a case where someone is collaborator but want a cap for opened PRs to apply to them. This feature is basically to prevent abuse and those users have already demonstrated they understand how to work with the project.

[Andrushika]

@potiuk I fixed those two nit -- please take a look when you have a chance :)

In our specific case if someone has been given triage (collaborator) allowing them to not have the cap by default seems reasonable.

Understood, thanks for clarifying!
Let's keep this PR to the explicit list, and I'll do "collaborators bypass by default" in a follow-up if we really need it. In the meantime, listing collaborators in bypass_users gives the same result.

[potiuk]

Let's keep this PR to the explicit list, and I'll do "collaborators bypass by default" in a follow-up if we really need it. In the meantime, listing collaborators in bypass_users gives the same result.

yeah. I prefer to keep things explicit as well (Python Zen FTW). We can always add prek-hook to sync the two lists.

[potiuk]

Re-tested at b835821064986c252a75fe8be59247242dc33e00 against potiuk/infrastructure-asfyaml-sandbox. pytest 180 passed, pre-commit --all-files clean.

The pre-flight check does what the README claims. With bypass_users: [octocat, defunkt, asfyaml-no-such-user-9999, asfyaml-also-missing-8888] and max_open_pull_requests: 9 against a repo sitting at max 5 with hubot, octocat on the list (run):

ASFYAMLException: github.pull_requests.creation_cap.bypass_users lists GitHub users
that do not exist: asfyaml-no-such-user-9999, asfyaml-also-missing-8888

and reading the API back afterwards: {"enabled": true, "max_open_pull_requests": 5}, list still hubot, octocat. The cap PATCH never fired and both bad logins are named. That is the partial-application problem gone.

The rest of the matrix re-run clean on the new head:

Row	Bypass list before → after	Run
Set (noop)	[] → []	35708596667
Set (live)	[] → hubot, octocat	35708687597
Update	hubot, octocat → defunkt, hubot	35709438170
Idempotent	unchanged, "already up to date"	35709522245
Key absent	defunkt, hubot, octocat → unchanged	35709780361
Case + duplicates	unchanged, no writes	35709865161
enabled: false + list	defunkt, hubot, octocat → octocat	35709954702
Clear ~	octocat → []	35710039800

One small thing, non-blocking: _plan_bypass_list_changes is called inside the if not self.noop(...) block, so a dry run never does the lookups. The noop run above printed Setting pull request creation cap bypass list to: octocat, hubot and stopped — a --noop against a yaml with a typo'd login reports success. Since the planning calls are all reads (the list GET plus one GET /users/{login} each), hoisting the plan above the noop guard would let a dry run name the bad login, which is where you'd most want to hear about it given the sandbox workflow tells people to noop first. Happy for that to be a separate PR or nothing at all.

Also checked, since the new check treats "GitHub knows this login" as "valid": an organization passes GET /users/{login} (apache → 200, type=Organization), and GitHub's PUT .../bypass-list accepts an org too (204, by hand). So the check is neither stricter nor looser than the endpoint it guards. No action needed — recording it so nobody has to re-derive it.

Sandbox reset and confirmed through the API: baseline .asf.yaml, cap {"enabled": false, "max_open_pull_requests": 7}, empty bypass list.

LGTM — this addresses both points from my earlier round.

[potiuk]

Sandbox-tested at b835821 — full set/update/idempotent/unset/unmanaged matrix verified by reading the GitHub API back, plus the unknown-login path confirming nothing is changed before the check fails. Details in the comment above. The one remaining note (noop skipping the lookups) is non-blocking.

[Andrushika]

One small thing, non-blocking: _plan_bypass_list_changes is called inside the if not self.noop(...) block, so a dry run never does the lookups. ...

Thanks! I would prefer to keep that one in a separate PR.

[potiuk]

OK. @Humbedooh -> Tested ... Useful :)

[henry3260]

Thanks @Andrushika and @potiuk make it possible!!

[potiuk]

@Humbedooh — following up on my note in #119 with a concrete list. These are sandbox-tested, with the results verified by reading the GitHub API back, and they only need a committer to decide and merge:

• Add GitHub pull request creation cap bypass list support #135: PR creation cap bypass list. Approved and fully sandbox-tested (set/update/idempotent/unset/unmanaged).
• Support changing or removing Jira autolinks #116: change or remove Jira autolinks. Sandbox-tested, approved by @ppkarwasz.
• Add restrict_update convenience key to prevent tags from being moved #120: restrict_update for tag rulesets. Sandbox-tested. Note for the changelog: existing type: tag rulesets gain the update rule on their next apply, which matches the old protected_tags behaviour.
• Group Dependabot updates by ecosystem #129: group Dependabot updates by ecosystem. Small and self-contained.

#139 (count drafts toward the creation cap) touches the same module as #135, so merging #135 first and letting me rebase #139 on top would be the easiest order.

[potiuk]

@Andrushika — #139 (include_drafts for the creation cap) was merged first, so this PR now conflicts with main. I've rebased it for you: potiuk:sandbox-test/pr-135 (5766f09). Both of your commits are kept with you as author.

The conflicts were purely additive (schema, pr_creation_cap.py, README), so I kept both sides: include_drafts and bypass_users now sit together. pytest (185 passed) and pre-commit run --all-files are clean.

I sandbox-tested the rebased branch against my own sandbox, checking the API after each run (cap = {enabled, max_open_pull_requests, include_drafts}):

Row	Config	Cap after	Bypass list after	Run
Set (noop)	max 3, drafts on, [octocat, monalisa]	unchanged {false,7,false}	unchanged []	run
Set	same	{true,3,true}	[monalisa, octocat]	run
Update	drafts off, [octocat]	{true,3,false}	[octocat]	run
Both keys omitted, drafts set on by hand	max 4	{true,4,true} (kept)	[octocat] (kept)	run
Unknown login	max 5, drafts off, [octocat, no-such-user…]	fails before any write, unchanged {true,4,true}	unchanged	run
Disable	enabled: false, bypass_users: ~	{false,4,true}	[]	run

I can't push to your branch (no write access on this repo), so could you pull it in and force-push?

git fetch https://github.com/potiuk/infrastructure-asfyaml.git sandbox-test/pr-135
git checkout add-creation-cap-bypass-list
git reset --hard FETCH_HEAD
git push --force-with-lease

Once that's up it should be mergeable again. @Humbedooh, the order in my comment above flipped (#139 went first), but the rest of that list still stands.

[Andrushika]

Okay I force-pushed.

One small thing, non-blocking: _plan_bypass_list_changes is called inside the if not self.noop(...) block, so a dry run never does the lookups.

BTW I ended up fixing this nit here in fd4e831 after all, since a separate follow-up PR could be a burden for maintainers.

[potiuk]

Cool. Should be ready to merge. I will test it now.

[potiuk]

Sandbox-tested the current head fd4e831 against my own sandbox, checking the API after each run (cap = {enabled, max_open_pull_requests, include_drafts}). The bypass list was seeded with [octocat] first. pytest (187 passed) and pre-commit run --all-files are clean.

Row	Config	Log	Cap after	Bypass after	Run
Noop plan	max 3, drafts on, [octocat, monalisa]	Adding … monalisa	unchanged {false,7,false}	unchanged [octocat]	run
Noop, unknown login	[octocat, no-such-user…]	fails: users that do not exist	unchanged	unchanged	run
Set	max 3, drafts on, [octocat, monalisa]	Adding … monalisa	{true,3,true}	[monalisa, octocat]	run
Noop re-run	same	already up to date	unchanged	unchanged	run
Disable	enabled: false, bypass_users: ~	Removing … monalisa, octocat	{false,3,true}	[]	run

fd4e831 does what it says: a dry run now shows the real add/remove plan and catches an unknown login before a live run would. The "no GH_TOKEN in noop" path can't be reached from the sandbox workflow, which always has a token, so only the unit tests cover it.

Thanks @Andrushika! @Humbedooh — this is ready to merge.

[potiuk]

I hope after returning from Community Over Code and Infra meetings - this one could be looked at very positively soon. We have members of our community who we would love to help with being able to open more PRs.

[Dev-iL]

Thank you for this effort - this will be very useful to me personally as an Airflow collaborator. Some committers recently started using the stacked pr feature. This helps a lot with the organization and review of complex changes, but the main downside is this cannot be done across fork boundaries. Thus, non-committers working on complex changes are stuck between a rock and a hard place: either submit one giant PR that won't realistically be reviewed due to its size, or create multiple PRs, most of which are initially drafts (not because "unready", but purely because of the cap), where reviewers might not be able to see the big picture since the commit in isolation has little/no effect.
Another example is when the cap is used up, I cannot preapre solutions for emerging issues found on scheduled CI jobs or broken main. So instead of "address issue -> create PR -> CI is green -> maintainer comes later and merges", I can at most analyze the root cause and suggest a fix e.g. on slack, then wait for a maintainer to come to create a regular PR then wait for CI to run - potentially wasting many CI- as well as wall-hours.