Make review approvals advisory without blocking merges

.github/tests/review-approval-policy.test.cjs

@@ -104,27 +104,31 @@ function harness(pulls, reviews = {}, options = {}) {
   return { run, writes, creates, runs, logs, errors, failed, permissionCalls };
 }
 
-test('ordinary advisory threshold is exactly one eligible approval', async () => {
-  for (const [reviews, expected] of [[[], 'action_required'], [[review(1, 'reviewer')], 'success']]) {
+test('ordinary recommendation is neutral at zero approvals and succeeds at one', async () => {
+  for (const [reviews, expected] of [[[], 'neutral'], [[review(1, 'reviewer')], 'success']]) {
     const h = harness([pull()], { 1: reviews });
     await h.run();
     assert.equal(h.writes[0].status, 'in_progress');
     assert.equal(h.writes.at(-1).status, 'completed');
     assert.equal(h.writes.at(-1).conclusion, expected);
     assert.equal(h.writes.at(-1).name, 'Review approval policy');
     assert.match(h.writes.at(-1).output.summary, /Advisory only/);
+    assert.match(h.writes.at(-1).output.summary, /maintainers may merge without waiting/);
+    assert.deepEqual(h.failed, []);
   }
 });
 
-test('major-change advisory check fails at two and passes at three approvals', async () => {
+test('major-change recommendation stays neutral below three approvals', async () => {
   for (const count of [0, 1, 2, 3]) {
     const h = harness([pull(1, ['MAJOR-CHANGE'])], {
       1: Array.from({ length: count }, (_, index) => review(index + 1, `reviewer-${index}`))
     });
     await h.run();
-    assert.equal(h.writes.at(-1).conclusion, count === 3 ? 'success' : 'action_required');
+    assert.equal(h.writes.at(-1).conclusion, count === 3 ? 'success' : 'neutral');
+    assert.deepEqual(h.failed, []);
     if (count < 3) {
       assert.match(h.writes.at(-1).output.summary, new RegExp(`${count}/3`));
+      assert.match(h.writes.at(-1).output.title, /Review reminder/);
     }
   }
 });
@@ -176,32 +180,33 @@ test('a new approval after a change request is counted', async () => {
   assert.equal(h.writes.at(-1).conclusion, 'success');
 });
 
-test('a label change raises the requirement without trusting old success', async () => {
+test('a label change raises the recommendation to a neutral reminder', async () => {
   const current = pull();
   const h = harness([current], { 1: [review(1, 'reviewer')] });
   await h.run();
   assert.equal(h.writes.at(-1).conclusion, 'success');
   current.labels.push({ name: 'major-change' });
   await h.run();
   assert.equal(h.writes.at(-2).status, 'in_progress');
-  assert.equal(h.writes.at(-1).conclusion, 'action_required');
+  assert.equal(h.writes.at(-1).conclusion, 'neutral');
   assert.match(h.writes.at(-1).output.summary, /1\/3/);
   assert.equal(h.creates.length, 1);
 });
 
-test('PRs sharing a SHA cannot overwrite a stricter PR status with ordinary success', async () => {
+test('PRs sharing a SHA preserve the higher recommendation without blocking', async () => {
   const h = harness([pull(1, [], 'shared'), pull(2, ['major-change'], 'shared')], {
     1: [review(1, 'one')],
     2: [review(2, 'one'), review(3, 'two')]
   });
   await h.run();
   assert.equal(h.writes.length, 2);
   assert.equal(h.writes.at(-1).head_sha, 'shared');
+  assert.equal(h.writes.at(-1).conclusion, 'neutral');
   assert.match(h.writes.at(-1).output.summary, /#2: 2\/3/);
   assert.deepEqual(h.permissionCalls, ['one', 'two']);
 });
 
-test('all PRs sharing a SHA must satisfy their own approval requirements', async () => {
+test('all PRs sharing a SHA meeting recommendations produce success', async () => {
   const h = harness([pull(1, [], 'shared'), pull(2, ['major-change'], 'shared')], {
     1: [review(1, 'one')],
     2: [review(2, 'one'), review(3, 'two'), review(4, 'three')]
@@ -324,6 +329,20 @@ test('advisory workflow has only base-controlled triggers and no PR notifier', (
   assert.doesNotMatch(workflow, /actions\/checkout|\$\{\{[^}]*head\.(?:sha|ref)|download-artifact|secrets\.|pull_request_review:|workflow_run:|statuses: write|createCommitStatus/);
   assert.equal(existsSync(resolve(__dirname, '..', 'workflows', 'review-changed.yml')), false);
   const contributing = readFileSync(resolve(__dirname, '..', '..', 'CONTRIBUTING.md'), 'utf8');
-  assert.match(contributing, /Every PR targeting `main` must retain the native \*\*three-approval\*\* requirement/);
-  assert.match(contributing, /Do not use this advisory check to lower the native approval minimum to one/);
+  assert.match(contributing, /approval counts are \*\*reminders only\*\*, not merge requirements/);
+  assert.match(contributing, /approval minimum for `main` is \*\*zero\*\*/);
+  assert.match(contributing, /native `minimum_approvals` set to \*\*zero\*\*/);
+  assert.match(contributing, /Without `major-change`: \*\*one approval recommended\*\*/);
+  assert.match(contributing, /With `major-change`: \*\*three approvals recommended\*\*/);
+  for (const pattern of [
+    '.github/**', 'AGENTS.md', 'CONTRIBUTING.md', 'GOVERNANCE.md',
+    'SECURITY.md', 'LICENSE', 'MAINTAINERS.md', 'CODE_OF_CONDUCT.md'
+  ]) {
+    assert.ok(contributing.split(/\r?\n/).includes(pattern), `Document important path: ${pattern}`);
+  }
+  assert.match(contributing, /The label changes only the recommendation, not merge eligibility/);
+  assert.match(contributing, /empty\s+bypass list/);
+  assert.match(workflow, /No approving reviews are required to merge/);
+  assert.match(workflow, /conclusion: waiting.length > 0 \? 'neutral' : 'success'/);
+  assert.doesNotMatch(workflow, /action_required|native three-approval protection|Native rules require one approval/);
 });

.github/workflows/review-approval-policy.yml

@@ -62,8 +62,8 @@ jobs:
                   status: 'in_progress',
                   details_url: `${context.serverUrl}/${context.repo.owner}/${context.repo.repo}/actions/runs/${context.runId}`,
                   output: {
-                    title: 'Checking advisory approval requirements',
-                    summary: 'Checking current labels and eligible approvals. Native three-approval protection remains required.'
+                    title: 'Checking review recommendations',
+                    summary: 'Review counts are reminders only. No approving reviews are required to merge.'
                   }
                 };
                 if (check) {
@@ -76,7 +76,7 @@ jobs:
 
                 const results = [];
                 for (const pull of group) {
-                  const required = pull.labels.some(label => label.name.toLowerCase() === 'major-change') ? 3 : 1;
+                  const recommended = pull.labels.some(label => label.name.toLowerCase() === 'major-change') ? 3 : 1;
                   const reviews = await github.paginate(github.rest.pulls.listReviews, {
                     ...context.repo,
                     pull_number: pull.number,
@@ -105,22 +105,22 @@ jobs:
                       approvals++;
                     }
                   }
-                  results.push({ number: pull.number, required, approvals });
+                  results.push({ number: pull.number, recommended, approvals });
                 }
 
                 // Check runs are shared by PRs with the same head SHA.
-                const waiting = results.filter(result => result.approvals < result.required);
+                const waiting = results.filter(result => result.approvals < result.recommended);
                 const summary = results
-                  .map(result => `#${result.number}: ${result.approvals}/${result.required} eligible approvals`)
+                  .map(result => `#${result.number}: ${result.approvals}/${result.recommended} recommended eligible approvals`)
                   .join('\n');
                 await github.rest.checks.update({
                   ...context.repo,
                   check_run_id: check.id,
                   status: 'completed',
-                  conclusion: waiting.length > 0 ? 'action_required' : 'success',
+                  conclusion: waiting.length > 0 ? 'neutral' : 'success',
                   output: {
-                    title: waiting.length > 0 ? 'Advisory approval requirements not met' : 'Advisory approval requirements met',
-                    summary: `${summary}\n\nAdvisory only: retain native three-approval protection.`.slice(0, 65535)
+                    title: waiting.length > 0 ? 'Review reminder: recommended approvals not yet reached' : 'Review recommendations met',
+                    summary: `${summary}\n\nAdvisory only: maintainers may merge without waiting for these recommendations. This check must not be required for merging.`.slice(0, 65535)
                   }
                 });
                 core.info(summary);
@@ -137,7 +137,7 @@ jobs:
                       conclusion: 'failure',
                       output: {
                         title: 'Advisory approval refresh failed',
-                        summary: `Unable to evaluate current approvals: ${message}\nRetain native three-approval protection.`.slice(0, 65535)
+                        summary: `Unable to evaluate current approvals: ${message}\nReview reminders are unavailable. Review approval counts do not gate merges.`.slice(0, 65535)
                       }
                     });
                   } catch (reportError) {

CONTRIBUTING.md

@@ -149,23 +149,43 @@ review requests fail the workflow explicitly.
 
 ### Approval Requirements
 
-Every PR targeting `main` must retain the native **three-approval** requirement.
-The automation reports these **advisory** thresholds:
+Review approval counts are **reminders only**, not merge requirements.
+Maintainers may merge PRs without waiting for approving reviews. The live native
+approval minimum for `main` is **zero**, including the extra-approval setting for
+unattributed changes, which is disabled.
+
+The `core-reviewers` team remains an optional reviewer for these important paths,
+with native `minimum_approvals` set to **zero**:
+
+```text
+.github/**
+AGENTS.md
+CONTRIBUTING.md
+GOVERNANCE.md
+SECURITY.md
+LICENSE
+MAINTAINERS.md
+CODE_OF_CONDUCT.md
+```
+
+Reviewer invitations and Copilot review remain enabled. The automation reports
+these **label-based recommendations**:
 
-- Ordinary changes: at least **one approval** from someone other than the author.
-- Important changes with the `major-change` label: at least **three approvals**.
+- Without `major-change`: **one approval recommended**.
+- With `major-change`: **three approvals recommended**.
 
 Approvals must come from users with Write or higher repository permissions.
 Invitations, comments, author approvals, bot reviews, and dismissed approvals do
 not count. A later approval or change request supersedes that user's earlier
-decision; a comment does not cancel an approval. Existing native protections
-still apply, including blocking unresolved change requests.
+decision; a comment does not cancel an approval. These counts inform maintainers
+but do not authorize or prevent a merge.
 
 Maintainers should add `major-change` before review for public API changes,
-architecture changes, breaking changes, governance changes, or other high-impact
-modifications. Do not remove the label merely to bypass the three-approval
-requirement. This is a project policy, not an ASF-wide requirement for every
-GitHub PR.
+architecture changes, breaking changes, or other high-impact modifications.
+The label changes only the recommendation, not merge eligibility. Important-file
+review requests are also optional; neither the label nor a matching path requires
+three approvals.
+This is a project policy, not an ASF-wide requirement for every GitHub PR.
 
 The [approval workflow](.github/workflows/review-approval-policy.yml) updates a
 **Review approval policy** check run for each head commit instead of repeatedly
@@ -176,22 +196,22 @@ refresh immediately. The workflow never checks out or executes PR code and does
 not require `CORE_REVIEWERS_TOKEN`.
 
 Maintainers can manually run **Review approval policy** after review changes.
-PRs sharing a head commit also share its check, so all such open, non-draft PRs
-must meet their individual advisory thresholds for the check to succeed.
-Failures are reported per head commit, set its check to failure when the API
-allows, and fail the workflow without preventing other commits from refreshing.
-
-Do not use this advisory check to lower the native approval minimum to one.
-GitHub's `pull_request_review` workflow runs from the PR merge ref, which a PR
-can modify to disable dismissal notifications. It can also require maintainer
-approval for first-time fork workflows. The former notifier is therefore removed;
-scheduled reconciliation avoids relying on it but cannot eliminate stale results.
-Enforcing ordinary-one/major-three would require a separately deployed, trusted
-review-event service and its own validation before changing branch rules.
-
-Retain the native three-approval requirement and the **Copilot review for default
-branch** ruleset's empty bypass list, Copilot review, deletion protection, and
-force-push protection. The advisory check does not replace GitHub's merge gate.
+PRs sharing a head commit also share its check. If any such PR has fewer than
+the recommended approvals, the check finishes with a **neutral** reminder, not
+`action_required` or failure. Meeting all recommendations produces success.
+Neither outcome is a merge gate. API and permission failures remain explicit:
+they fail the check and workflow without preventing other commits from refreshing.
+They must not be disguised as successful review evaluations.
+
+Keep the **Review approval policy** check and reviewer-invitation workflow
+**optional**, not required status checks. Missing organization credentials can
+still make reviewer invitations fail; this is a configuration problem, not an
+approval requirement. Merge conflicts and any separately configured required
+checks must still be resolved.
+
+Retain the **Copilot review for default branch** ruleset's pull-request requirement,
+empty bypass list, Copilot review, deletion protection, and force-push protection.
+Do not restore approval-count gates without an explicit maintainer decision.
 Automatic team invitations can be enabled independently after the reviewer
 workflow is merged into `main` and its dedicated secret is configured.