Skip to content

Fix contribution policy exemptions - #184

Merged
dcartertwo merged 2 commits into
mainfrom
fix/maintainer-contribution-policy
Aug 13, 2026
Merged

Fix contribution policy exemptions#184
dcartertwo merged 2 commits into
mainfrom
fix/maintainer-contribution-policy

Conversation

@dcartertwo

@dcartertwo dcartertwo commented Aug 13, 2026

Copy link
Copy Markdown
Collaborator

GitHub’s author_association depends on the viewer: the Actions token classified a maintainer as CONTRIBUTOR and auto-closed #167. This uses the author’s effective repository permission instead, which is the authority the exemption is meant to represent.

Drafts are no longer a blanket exemption. External draft PRs are subject to the same checks as ready PRs; maintainers, Dependabot, and policy/override remain exempt.

@dcartertwo
dcartertwo marked this pull request as ready for review August 13, 2026 19:39
@ask-bonk

ask-bonk Bot commented Aug 13, 2026

Copy link
Copy Markdown

The change is small, focused, well-tested, and correct. Let me do a final assessment of potential issues:

  1. Fail-open on API error — if getCollaboratorPermissionLevel throws, the function throws and no close happens. This is acceptable (safe direction) but undocumented. Not a blocking issue.

  2. permission values — GitHub's API returns admin/maintain/write/triage/read/none. The TRUSTED_PERMISSIONS set correctly captures the push-capable levels (admin, maintain, write) and excludes triage/read/none. Correct.

  3. Extra API call cost — only incurred when violations exist, which is the uncommon path. Efficient.

I found no discrete, actionable problems introduced by this change. The logic correctly addresses the visibility-dependent author_association bug by checking effective repository permission, the placement of the check is efficient, the trusted-permission set matches GitHub's push-capable levels, and the tests cover both the new exempt and still-enforced cases. Tests pass locally (22/22).

LGTM!

github run

@github-actions github-actions Bot added the delivery Changes to CI or release delivery label Aug 13, 2026
@dcartertwo dcartertwo changed the title Fix maintainer contribution policy exemption Fix contribution policy exemptions Aug 13, 2026
@ask-bonk

ask-bonk Bot commented Aug 13, 2026

Copy link
Copy Markdown

All 23 tests pass. Let me assess the core logic for correctness and any introduced issues.

The change is sound. Let me evaluate the key design points:

  1. The bug fix is correct. author_association is visibility-dependent — the Actions token sees maintainers as CONTRIBUTOR. Using getCollaboratorPermissionLevel returns the effective repository permission, which is not visibility-dependent, so this correctly fixes the Add OpenAI-compatible model provider #167 auto-close.

  2. Permission setadmin/maintain/write are exactly the push-capable levels; triage/read/none are correctly excluded.

  3. Placement — the extra API call only runs when violations exist (uncommon path), which is efficient.

  4. Draft change — removing the draft bypass from both the workflow if: and the function is consistent, and the workflow test guards against reintroducing the draft condition.

One observation worth noting (non-blocking): the permission check at line 78-86 uses the first-fetch pullRequest.user?.login, and the re-fetch at line 94 recomputes violations but does not re-run the permission check. This is fine — the PR author cannot change during evaluation, so a single permission check is correct. There's no issue here.

I found no discrete, actionable problems introduced by this change. The fix correctly addresses the visibility-dependent author_association bug, the trusted-permission set matches GitHub's push-capable levels, the draft-enforcement changes are consistent across workflow and script, and tests cover both the new exempt path and the still-enforced path (23/23 pass locally).

LGTM!

github run

@dcartertwo
dcartertwo merged commit f9fc374 into main Aug 13, 2026
12 checks passed
@dcartertwo
dcartertwo deleted the fix/maintainer-contribution-policy branch August 13, 2026 19:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

delivery Changes to CI or release delivery policy/override

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants