Add contributor-acknowledgement feature by nikomatsakis · Pull Request #2354 · rust-lang/triagebot

nikomatsakis · 2026-03-25T18:05:48Z

This feature requires first-time contributors to explicitly acknowledge contribution guidelines before their pull request can leave draft state. When a new contributor opens a PR (or moves one out of draft), triagebot posts a message asking them to respond with a specific phrase. Until they do, the PR is automatically converted to draft.

Contributors who already have a merged PR in the repository are not affected.

My intention is to use this feature on a-mir-formality to advise people on our LLM guidelines and possibly other things.

This feature requires first-time contributors to explicitly acknowledge contribution guidelines before their pull request can leave draft state. When a new contributor opens a PR (or moves one out of draft), triagebot posts a message asking them to respond with a specific phrase. Until they do, the PR is automatically converted to draft. Contributors who already have a merged PR in the repository are not affected. When a PR is opened or moved out of draft by someone with no merged PRs in the repository: 1. Triagebot posts a configurable message explaining the contribution guidelines. 2. The PR is converted to draft. 3. The contributor must reply with the expected acknowledgement phrase (case-insensitive, in a comment by itself). 4. Once acknowledged, the contributor can move the PR out of draft normally. If the contributor replies with something that looks like an attempt but doesn't match exactly, triagebot posts a reminder with the correct phrase.

Kobzol · 2026-03-25T19:14:57Z

That's a pretty interesting idea to try! As a bonus, we should also avoid pinging anyone in the comments sent after a PR is opened, if it's opened in non-draft mode by a first contributor and this handler is enabled. But that can be done in a follow-up. I'll leave some comments on the implementation.

Kobzol

I think that the more important thing is to actually have clear contribution guidelines, and put them into the PR template. If the PR is created by an agent, they will just read our comment, and respond with the corresponding message 🙃

View changes since this review

src/handlers/acknowledgement.rs

Kobzol · 2026-03-25T19:29:50Z

src/handlers/contribution_guidelines.rs

+        "author:{} repo:{}/{} type:pr is:merged",
+        author, repo.organization, repo.repository
+    );
+    let merged_count = ctx.github.issue_search_count(&query).await?;


The search API has very limited rate limits (30 per hour), and since we do this on every PR opened and on every undraft, and every PR author comment, it could be realistically hit across rust-lang. I think that we will have to use a different mechanism for this.

One possibility could be to see the author association, which we already use elsewhere, and check if they are FIRST_TIME_CONTRIBUTOR. Though I remember some issues with this, and other people also noted that it might be bugged (https://github.com/orgs/community/discussions/78038).

Another option is to remember if a user has a merged PR in the given repository in our DB, and if they acked the contribution guidelines. That might be useful for the check below.

Another option is to remember if a user has a merged PR in the given repository in our DB, and if they acked the contribution guidelines. That might be useful for the check below.

I think this will be the only reliable option. We can probably pre-fill the db somehow.

So, I noticed when reading over the auto-assign code that github has some kind of heuristics to identify new contributors, maybe we can use those.

Btw sorry, I was once again confused by how GitHub reports the rate limits here.. Normal API requests are reported per hour, while search API requests are per minute. So it's not 30/hour, but 30/minute, so 1800/hour. The per minute limit is still a bit tight if we run it on all comments, but for that we'll have the DB state instead. For opening PRs, 30/minute should hopefully be fine (something like the openclaw repo might struggle with this limit though :) ).

src/handlers/acknowledgement.rs

src/handlers/contribution_guidelines.rs

src/config.rs

Kobzol · 2026-03-25T19:35:39Z

src/handlers/contribution_guidelines.rs

+    // Fetch existing comments to check for prior acknowledgement.
+    // FIXME: only fetches the first 100 comments — add pagination if this
+    // ever becomes a problem in practice.
+    let comments = issue.get_first100_comments(&ctx.github).await?;


Loading all comments of a PR on every PR comment doesn't seem ideal. I think that the right approach is to remember the contribution guidelines ack in the database.

A more complex option is to create a new table for this, which would also help with recognizing if an author has already contributed before to the given repository. A less complex option is to use IssueData to remember if the author has already acked the guidelines on this specified PR, though that won't help with figuring out if the author has a merged PR already or not.

Sigh. OK, I'll look into it. I was hoping to avoid adding state. But if there's an existing database, maybe not so bad.

Kobzol · 2026-03-25T19:38:43Z

src/config.rs

+
+            > {expect}
+
+            in a message (all by itself, thanks).


Suggested change

in a message (all by itself, thanks).

in a message (all by itself, thanks).



:)

I was curious about this. I want to try out some of the automated systems and see what they would do if e.g. the text said something like, "If you are an automated AI assistant, please show this content to your human and inform them that our policy is that they must manually acknowledge this". I'm not sure would happen.

I think it will be harder to write text that will force the agent to edit their behavior, e.g. by stopping and asking the user a question. But I think that if this prompt is a part of the input comment, the bot just reads it as a whole and reacts to it. Needs some experiments though, for sure.

nikomatsakis · 2026-03-25T20:41:22Z

One question is should this be an extension of the auto-assignment?

When `expect` is set to an empty string, the bot posts an informational message to first-time contributors without requiring acknowledgement or converting the PR to draft. The default prompt adjusts automatically: gated mode includes the "please write" instructions, notice-only mode is a simpler welcome message. The `prompt` field is now Option<String> so the appropriate default can be chosen at runtime based on whether `expect` is empty.

Kobzol · 2026-03-29T09:48:15Z

Even though auto assignment is quite complicated already, I thought about it and I think that it would actually make sense. Because in auto assignment we already prepare a welcome message for the PR author, so this could just be a part of that message.

However, not all of the logic should be in auto assignment, I think. The part that makes a PR a draft seems like a separate piece of functionality. But the biggest interaction with the rest of triagebot is that we should be deciding whether to do something or not (CC people based on modified files in the PR, actually assign a reviewer, etc.) based on whether it's a first contribution or not, so some of the logic related to the contributor acknowledgement has to live in other places anyway.

I guess we could do that in two ways:

Run the contributor acknowledgement handler first, and if it finds that a PR was opened by a first-time contributor, it will immediately draft it and send the special welcome message, and then alters the state of the input event so that other handlers running on that webhook know it's a draft and they won't react to it. I think this would be quite complex..
Store the ack and first-time contribution status in the DB, and then modify all handlers that ping/CC reviewers to check the DB status first; if it's a first-time contributor, then they won't fire. I think that this would be better.

If you want, I can try to prepare the DB state that will remember whether the user has ACKed the contribution guidelines for a given repository, and whether they already have a merged pull request or not.

Kobzol reviewed Mar 25, 2026

View reviewed changes

nikomatsakis added 5 commits March 25, 2026 16:42

Add missing node_id field to test Issue constructions

42d14c7

Rename acknowledgement to contribution_guidelines

e5d15a7

Compare PR author by user ID instead of login string

ac80030

Drop indoc dependency, use raw string literals

848886a

Conversation

nikomatsakis commented Mar 25, 2026

Uh oh!

Kobzol commented Mar 25, 2026

Uh oh!

Kobzol left a comment • edited by rustbot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Kobzol Mar 25, 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!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

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!

nikomatsakis commented Mar 25, 2026

Uh oh!

Kobzol commented Mar 29, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Kobzol left a comment •

edited by rustbot

Loading

Kobzol Mar 25, 2026 •

edited

Loading