helm-external-secrets/design/015-contribution-process-refactoring.md

Proposal: ready-for-review label workflow

Problem

The PR list is noisy. Maintainers cannot tell at a glance which PRs are actually ready for review versus draft, WIP, or still being iterated on after feedback. This wastes reviewer time and makes it hard to prioritize the queue. The rise of AI-assisted editing has also sharply increased commit volume, often producing many small follow-up commits for individual inline comments, which further amplifies the noise.

Goal

Introduce a ready-for-review label that is only present on a PR when the author has explicitly signalled it is ready and no state has changed since that signal. Maintainers can then filter the PR list to label:ready-for-review and trust that every result deserves attention.

Mechanics

1. Author signals readiness

Author comments /ready-for-review on its own line in a PR comment. A single workflow job (add-label) listens for PR comments, checks for an exact-line match (not just a substring, so passing mentions of the command elsewhere in a comment don't trigger it), verifies the commenter is the PR author or a maintainer, and either applies the ready-for-review label or comments back explaining why the command was rejected.

2. Label removal triggers

The label is removed automatically whenever the PR's state changes in a way that invalidates the "ready" signal. In every case, the only action taken is removing the ready-for-review label; the automation never changes the PR's own draft/open state:

• A new commit is pushed to the PR branch (synchronize event).
• A maintainer submits a review with REQUEST_CHANGES or a COMMENT review containing unresolved feedback (treat CHANGES_REQUESTED as the reliable signal; treat plain comment reviews as optional/configurable).
• Optionally: the PR goes stale past a configurable number of days.

Each of these is a separate trigger condition in the same workflow, all converging on "remove label if present." Running removal in the same workflow as the gate check (below) is what lets removal finish before the gate re-evaluates, rather than racing it.

3. Gate check

A job in the same workflow (check-label) runs on every relevant pull_request_target event (push, open, reopen, label change) and verifies the PR carries ready-for-review:

• Draft PRs are skipped entirely. GitHub's own draft state already signals "not ready," so the gate doesn't pile a second signal on top of it.
• If the label is present: the job passes.
• If the label is absent: the job fails via core.setFailed. This does not post a PR comment; the explanation (what to run, and a link to docs) shows up as the failure message on the check itself and in the workflow run log, which is enough of a pointer without adding another comment to the thread.

Because check-label declares needs: [remove-label] and both run within the same pull_request_target event, check-label always executes after remove-label completes on a push, and it re-fetches the PR's labels from the API rather than trusting the event payload. This removes the race that existed when the two lived in separate workflow files: previously a push could trigger the gate and the label removal in parallel, with no guarantee the gate would see the post-removal state.

Adding the label (via add-label) or removing it (via remove-label after a change-requested review) each fire their own labeled/unlabeled event, which re-triggers this same workflow and re-runs check-label on the current commit, so the check reflects reality without waiting for a new push.

This failing check is what actually reduces noise for maintainers, since it can be made a required check that keeps the PR out of default review views until the author opts in, without blocking merge mechanics for drafts.

Implementation sketch

Combined workflow (.github/workflows/ready-for-review.yml)

name: PR readiness

on:
  issue_comment:
    types: [created]
  pull_request_review:
    types: [submitted]
  pull_request_target:
    types: [synchronize, opened, reopened, labeled, unlabeled]

permissions:
  pull-requests: write
  issues: write

jobs:
  add-label:
    if: >
      github.event_name == 'issue_comment' &&
      github.event.issue.pull_request &&
      contains(github.event.comment.body, '/ready-for-review')
    runs-on: ubuntu-latest
    steps:
      - name: Handle /ready-for-review command
        uses: actions/github-script@v7
        with:
          script: |
            const body = context.payload.comment.body || '';
            const isCommand = /^\s*\/ready-for-review\s*$/m.test(body);
            if (!isCommand) {
              core.info('Comment does not contain the /ready-for-review command on its own line, skipping.');
              return;
            }

            const commenter = context.payload.comment.user.login;
            const pr = await github.rest.pulls.get({
              owner: context.repo.owner,
              repo: context.repo.repo,
              pull_number: context.issue.number,
            });
            const isAuthor = commenter === pr.data.user.login;
            const perms = await github.rest.repos.getCollaboratorPermissionLevel({
              owner: context.repo.owner,
              repo: context.repo.repo,
              username: commenter,
            });
            const isMaintainer = ['admin', 'write'].includes(perms.data.permission);

            if (!isAuthor && !isMaintainer) {
              await github.rest.issues.createComment({
                owner: context.repo.owner,
                repo: context.repo.repo,
                issue_number: context.issue.number,
                body: `@${commenter} only the PR author or a maintainer can run \`/ready-for-review\`.`,
              });
              core.setFailed(`${commenter} is not authorized to run /ready-for-review`);
              return;
            }

            await github.rest.issues.addLabels({
              owner: context.repo.owner,
              repo: context.repo.repo,
              issue_number: context.issue.number,
              labels: ['ready-for-review'],
            });

  remove-label:
    if: >
      (github.event_name == 'pull_request_target' &&
        github.event.action == 'synchronize') ||
      (github.event_name == 'pull_request_review' &&
        github.event.review.state == 'changes_requested')
    runs-on: ubuntu-latest
    steps:
      - name: Remove label
        uses: actions/github-script@v7
        with:
          script: |
            try {
              await github.rest.issues.removeLabel({
                owner: context.repo.owner,
                repo: context.repo.repo,
                issue_number: context.issue.number,
                name: 'ready-for-review',
              });
            } catch (e) {
              if (e.status !== 404) throw e; // label already absent, fine
            }

  check-label:
    name: Verify ready-for-review label
    needs: [remove-label]
    if: >
      always() &&
      github.event_name == 'pull_request_target' &&
      github.event.pull_request.draft == false
    runs-on: ubuntu-latest
    steps:
      - name: Verify ready-for-review label
        uses: actions/github-script@v7
        with:
          script: |
            // Re-fetch fresh rather than trusting the event payload, since
            // the remove-label job may have just changed the label set.
            const { data: pr } = await github.rest.pulls.get({
              owner: context.repo.owner,
              repo: context.repo.repo,
              pull_number: context.payload.pull_request.number,
            });
            const labels = pr.labels.map(l => l.name);
            if (!labels.includes('ready-for-review')) {
              core.setFailed(
                'This PR is not marked ready for review. Comment `/ready-for-review` once it is ready. See LINK_TO_DOCS for the contributor guide.'
              );
            }

Notes on the sketch:

• pull_request_target is used for the whole workflow, not just label mutation, so add-label, remove-label, and check-label all run with full base-repo permissions and share one event context, even for PRs from forks. None of the jobs check out PR code, so this remains safe.
• The remove-label job only ever calls removeLabel. It does not touch the PR's draft/ready state, mergeability, or any other field.
• check-label re-fetches labels via pulls.get instead of using context.payload.pull_request.labels, so it reflects remove-label's outcome rather than the stale state from when the event fired.
• check-label is skipped outright when pull_request.draft == true.
• check-label never comments; its failure message is visible on the check itself and in the run log. add-label does comment, but only once, on a rejected command, not repeatedly across pushes, so no deduplication logic is needed there.
• Consider making check-label a required status check in branch protection so it visibly blocks merge until addressed, or leave it non-blocking if the intent is purely list hygiene rather than merge gating.
• check-label is given an explicit name: so its check run reads as "Verify ready-for-review label" rather than the raw job id. This is what makes the required-check string in branch protection predictable and readable; see the Rollout plan for the exact context string.

Edge cases to decide before implementation

Case	Options
Maintainer wants to force-mark a PR ready on author's behalf	Allow maintainers to run /ready-for-review too (sketch above already allows this)
Author pushes a trivial fix (typo, rebase) after being marked ready	Accept as a cost of simplicity, or add a /ready-for-review alias like a --skip flag for trivial pushes (adds complexity, probably not worth it initially)
Bots/automated pushes (e.g. Dependabot rebase)	Consider excluding pushes from known bot actors from label removal
Comment reviews without changes-requested state	Decide whether to treat these as removal triggers; recommend starting with only CHANGES_REQUESTED to avoid over-triggering
Stale ready PRs	Optional stale-bot integration to remove label after N days of inactivity

Rollout plan

1. Ship the workflow behind a feature flag or on a single pilot repo first.
2. Update CONTRIBUTING.md with the /ready-for-review instructions and a short rationale, and link it from the gate check's failure message.
3. Announce the change in the repo's discussion/announcements channel with a grace period before making the gate check required.
4. Monitor false-positive label removals (e.g. bot pushes, non-substantive reviews) for the first couple of weeks and tune the removal triggers.
5. Once stable, make the gate check a required status check in branch protection rules. For a single, non-reusable workflow the context string GitHub reports is <workflow name> / <job name>, so this should appear as PR readiness / Verify ready-for-review label. GitHub only lists checks in the branch protection picker once they've actually run against the target branch, so trigger the workflow once (e.g. merge a pilot PR or push a no-op commit) before trying to select it, and pick it from the dropdown rather than typing it from memory since the match must be exact.

Open questions for the maintainer group

• Should the gate check block merge, or only serve as a visible signal?
• Should reviews from any maintainer count, or only from a designated CODEOWNERS subset?
• Do we want a stale-label auto-removal policy, and if so what timeout?
• Should inline review comments (pull_request_review_comment), separate from a formally submitted review, also trigger label removal, or is a submitted CHANGES_REQUESTED review sufficient on its own?

Related patterns in other projects

Most large CNCF projects implement this kind of thing through Prow, the Kubernetes-originated CI/chatops bot, rather than plain GitHub Actions — heavier infrastructure than what's proposed here, but it validates the underlying pattern:

• Kubernetes contributor docs — a [WIP] title prefix or a /hold comment each drive a do-not-merge/* label that blocks merge automation until removed. This is the reverse polarity of this proposal (default mergeable, explicit block, rather than default blocked, explicit ready), but it's the same idea of a bot-managed label as the accurate signal: https://www.kubernetes.dev/docs/guide/pull-requests/
• Prow's lgtm plugin — the closest direct analog to our removal-on-push trigger. A reviewer comments /lgtm, Prow adds the lgtm label, and a new commit push automatically strips it again so the PR has to be re-reviewed: https://docs.prow.k8s.io/docs/components/plugins/lgtm/
• The original design discussion for that behavior — kubernetes/test-infra#3795 walks through adding "strip the label on synchronize" to the lgtm plugin, including the same false-positive/spam tradeoffs called out in the edge cases table above: https://github.com/kubernetes/test-infra/issues/3795
• containerd (a CNCF graduated project) — runs the Prow wip plugin on its own repo, so this isn't just a kubernetes/kubernetes-specific convention: https://prow.k8s.io/plugin-config
• A plain GitHub Actions precedent (not CNCF) — a lighter-weight version of "fail the check based on label state," built without Prow, closer in spirit to the implementation sketched above: https://bgenc.com/2023.02.18.github-actions-do-not-merge-label/