fix(pr-validation): keep the breaking-change label after the approval

.github/labels.yml

@@ -82,6 +82,10 @@
   color: "b60205"
   description: Callers must update their workflow configuration after this change
 
+- name: awaiting-breaking-approval
+  color: "fbca04"
+  description: A breaking change on this PR is waiting on a human decision
+
 # ── Capabilities ──────────────────────────────────────────────────────────────
 
 - name: build

.github/workflows/breaking-change-approval.yml

@@ -144,10 +144,27 @@ on:
         type: boolean
         default: true
       label:
-        description: Label applied while the pull request waits for its approval, and removed once resolved. Cosmetic — failure to apply it never fails the job. Empty disables it.
+        description: >-
+          Label stating that this pull request carries a breaking change. A fact
+          about the change, so it survives the approval — the break does not go
+          away because somebody agreed to it. Removed only when the breaking
+          commits are gone. Empty disables it.
+
+          Behaviour change: this label used to come off on approval. A caller
+          that set it as a temporary waiting marker should move that value to
+          `pending_label` and set this one to an empty string.
         required: false
         type: string
         default: breaking-change
+      pending_label:
+        description: >-
+          Label stating that the breaking change is waiting on a decision. A
+          state, so it comes off once approved or rejected. Separate from
+          `label` because the two stop being true at different moments.
+          Empty disables it.
+        required: false
+        type: string
+        default: awaiting-breaking-approval
       enable_comment:
         description: Post (and keep updated) a single comment carrying the request, the approval link and the outcome.
         required: false
@@ -512,7 +529,8 @@ jobs:
           head-sha: ${{ needs.detect.outputs.head-sha }}
           acknowledged: ${{ needs.detect.outputs.acknowledged }}
           author-acknowledgement: ${{ inputs.author_acknowledgement }}
-          label: ''
+          label: ${{ inputs.label }}
+          pending-label: ''
           enabled: ${{ inputs.enable_comment && 'true' || 'false' }}
 
       - name: Request the approval on the pull request
@@ -536,6 +554,7 @@ jobs:
           run-url: ${{ github.server_url }}/${{ github.repository }}/actions/runs/${{ github.run_id }}
           close-on-rejection: ${{ inputs.close_on_rejection && 'true' || 'false' }}
           label: ${{ inputs.label }}
+          pending-label: ${{ inputs.pending_label }}
           enabled: ${{ inputs.enable_comment && 'true' || 'false' }}
 
   # ----------------- The gate itself -----------------
@@ -625,6 +644,8 @@ jobs:
           APPROVERS: ${{ steps.decisions.outputs.approvers }}
           REJECTERS: ${{ steps.decisions.outputs.rejecters }}
           API_READABLE: ${{ steps.decisions.outputs.api-readable }}
+          LABEL: ${{ inputs.label }}
+          PENDING_LABEL: ${{ inputs.pending_label }}
           DRY_RUN: ${{ inputs.dry_run && 'true' || 'false' }}
         run: |
           set -euo pipefail
@@ -705,6 +726,18 @@ jobs:
             result=success
             state=none
             emit
+
+            # The only moment the break actually stopped existing, so the only
+            # place the fact label comes off. A reword dropping the `!` left
+            # `breaking-change` on the pull request for good — the cleanup was
+            # lost in an earlier rewrite of this job and is restored here.
+            if [ "$DRY_RUN" != "true" ] && [ -n "${PR_NUMBER:-}" ]; then
+              for stale in "$LABEL" "$PENDING_LABEL"; do
+                [ -n "$stale" ] || continue
+                gh pr edit "$PR_NUMBER" --repo "$REPO" --remove-label "$stale" >/dev/null 2>&1 || true
+              done
+            fi
+
             echo "::notice::Nothing to approve on PR #${PR_NUMBER} — ${REASON}"
             exit 0
           fi
@@ -763,6 +796,7 @@ jobs:
           rejection-comment: ${{ steps.decisions.outputs.rejection-comment }}
           close-on-rejection: ${{ inputs.close_on_rejection && 'true' || 'false' }}
           label: ${{ inputs.label }}
+          pending-label: ${{ inputs.pending_label }}
           enabled: ${{ inputs.enable_comment && 'true' || 'false' }}
 
       # Only on a recorded rejection, never on a cancellation. Closing a pull

docs/breaking-change-approval.md

@@ -408,8 +408,24 @@ ends as the record of who decided what:
 > 👉 **[Review the pending deployment](#)** — *Review deployments* →
 > `breaking-approval`.
 
-The `breaking-change` label is applied while it waits and removed once resolved —
-cosmetic, and a repository that never declared it still gets its gate.
+Two labels, and they expire at different times. `breaking-change` states a fact
+about the commits and **survives the approval** — the break does not stop being a
+break because somebody agreed to it, and a search for that label months later
+should still find this pull request. It comes off only when the breaking commits
+are gone. `awaiting-breaking-approval` states that a decision is pending, and
+comes off the moment one is made.
+
+Both are cosmetic: a repository that never declared them still gets its gate.
+Declare them with [`labels-sync`](labels-sync.md), or pass `label: ''` /
+`pending_label: ''` to turn them off.
+
+> **Behaviour change.** `label` used to be removed once the change was approved
+> or rejected. If you set it as a temporary waiting marker, move that value to
+> `pending_label` and set `label: ''` — otherwise the marker now stays on the
+> pull request after the decision.
+>
+> Callers on the default need nothing: `breaking-change` staying put is the
+> point, and `awaiting-breaking-approval` takes over the transient role.
 
 ## Inputs
 

src/notify/breaking-change-approval-reporter/README.md

@@ -82,6 +82,21 @@ may be gated by different people than the caller configured, and naming the wron
 ones is worse than naming none — with neither available, the line is omitted
 rather than guessed.
 
+## Two labels, because they expire at different times
+
+`breaking-change` is a **fact about the commits**. Approving does not make the
+break go away, and somebody filtering for that label three months from now wants
+this pull request in the results. It is applied in every state this composite
+handles, and only the caller removes it — at the one moment the fact stops being
+true, when the breaking commits are gone.
+
+`awaiting-breaking-approval` is a **state**. It goes on while a decision is
+pending and comes off the moment one is made, either way.
+
+Collapsing the two is tempting and wrong: removing `breaking-change` on approval
+erases the record exactly when it starts to matter, because now the break is
+both real and agreed.
+
 ## What it does not do
 
 It never closes, merges or approves anything. `close-on-rejection` only changes
@@ -113,7 +128,8 @@ side effect in the caller is what makes this safe to run with
 | `rejecters` | Comma-separated logins that rejected. | No | `''` |
 | `rejection-comment` | Comment left with the rejection. | No | `''` |
 | `close-on-rejection` | Whether the caller closes on rejection. Wording only. | No | `true` |
-| `label` | Applied while waiting, removed once resolved. Cosmetic. | No | `''` |
+| `label` | States that the PR carries a breaking change. A fact — stays once applied. Cosmetic. | No | `''` |
+| `pending-label` | States that the breaking change is waiting on a decision. Removed once approved or rejected. | No | `''` |
 | `enabled` | `false` skips commenting while still managing the label. | No | `true` |
 
 ## Outputs

src/notify/breaking-change-approval-reporter/action.yml

@@ -80,7 +80,11 @@ inputs:
     required: false
     default: 'true'
   label:
-    description: Label applied while waiting and removed once resolved. Cosmetic — failing to apply it never fails the step. Empty disables it.
+    description: 'Label stating that this pull request carries a breaking change. A fact about the change, so it stays once applied — approving does not make the break go away. The caller removes it when the breaking commits are gone. Cosmetic; failing to apply it never fails the step. Empty disables it. Behaviour change: it used to be removed on a decision — a caller using it as a temporary waiting marker should move that value to `pending-label` and clear this one.'
+    required: false
+    default: ''
+  pending-label:
+    description: 'Label stating that the breaking change is waiting on a decision. A state, so it is removed once approved or rejected. Kept separate from `label` because "this breaks things" and "nobody has decided yet" stop being true at different moments. Empty disables it.'
     required: false
     default: ''
   enabled:
@@ -120,6 +124,7 @@ runs:
         REJECTION_COMMENT: ${{ inputs.rejection-comment }}
         CLOSE_ON_REJECTION: ${{ inputs.close-on-rejection }}
         LABEL: ${{ inputs.label }}
+        PENDING_LABEL: ${{ inputs.pending-label }}
         ENABLED: ${{ inputs.enabled }}
       run: |
         set -euo pipefail
@@ -356,14 +361,30 @@ runs:
         fi
 
         # Cosmetic and deliberately last — a repository that never declared the
-        # label still gets its gate.
+        # labels still gets its gate.
+        #
+        # Two of them, because they stop being true at different moments. The
+        # breaking change is a property of the commits: approving it does not
+        # make the break go away, and somebody filtering for `breaking-change`
+        # three months from now wants this pull request in the results. Waiting
+        # on a decision is a state, and it ends.
+        #
+        # Every state reaching this composite means a breaking change is
+        # present — the caller skips it entirely when there is none — so the
+        # fact label is applied throughout and only the caller removes it.
         if [ -n "$LABEL" ]; then
+          if ! gh pr edit "$PR_NUMBER" --repo "$REPO" --add-label "$LABEL" >/dev/null 2>&1; then
+            echo "::notice::Label '${LABEL}' not applied — it is not declared in ${REPO}. Cosmetic only."
+          fi
+        fi
+
+        if [ -n "$PENDING_LABEL" ]; then
           if [ "$STATE" = "waiting" ]; then
-            if ! gh pr edit "$PR_NUMBER" --repo "$REPO" --add-label "$LABEL" >/dev/null 2>&1; then
-              echo "::notice::Label '${LABEL}' not applied — it is not declared in ${REPO}. Cosmetic only."
+            if ! gh pr edit "$PR_NUMBER" --repo "$REPO" --add-label "$PENDING_LABEL" >/dev/null 2>&1; then
+              echo "::notice::Label '${PENDING_LABEL}' not applied — it is not declared in ${REPO}. Cosmetic only."
             fi
           else
-            gh pr edit "$PR_NUMBER" --repo "$REPO" --remove-label "$LABEL" >/dev/null 2>&1 || true
+            gh pr edit "$PR_NUMBER" --repo "$REPO" --remove-label "$PENDING_LABEL" >/dev/null 2>&1 || true
           fi
         fi
 

src/validate/breaking-change-guard/test-workflow.py

@@ -347,6 +347,79 @@ def run_verdict(**overrides):
         return result, parse_github_output(output_path.read_text(encoding="utf-8"))
 
 
+def run_reporter_labels(state, **overrides):
+    """Run the reporter end to end and return the `gh pr edit` calls it made.
+
+    `render_reporter` stops before the label block, so asserting on the
+    composite's text was the only coverage the lifecycle had — and text cannot
+    tell which state made which call. Moving `--add-label "$LABEL"` into the
+    `waiting` branch would keep every string in place and break the behaviour.
+    """
+    action = yaml.safe_load(
+        (
+            REPO_ROOT / "src" / "notify" / "breaking-change-approval-reporter"
+            / "action.yml"
+        ).read_text(encoding="utf-8")
+    )
+    body = action["runs"]["steps"][0]["run"]
+
+    env = {
+        "REPO": "LerianStudio/example",
+        "PR_NUMBER": "42",
+        "STATE": state,
+        "REASON": "rule develop <- '*'",
+        "HEAD_SHA": "abc1234567",
+        "ACKNOWLEDGED": "false",
+        "ACK_LINE": "Breaking change acknowledged: …",
+        "RUN_URL": "https://github.com/LerianStudio/example/actions/runs/1",
+        "REVIEWERS": "team:devops",
+        "SERVER_URL": "https://github.com",
+        "BADGE_URL": "https://img.shields.io/badge/x-1f6feb",
+        "DOCS_URL": "https://example.invalid/docs",
+        "APPROVERS": "carol",
+        "REJECTERS": "dave",
+        "REJECTION_COMMENT": "",
+        "CLOSE_ON_REJECTION": "true",
+        "LABEL": "breaking-change",
+        "PENDING_LABEL": "awaiting-breaking-approval",
+        "ENABLED": "false",
+        "FINGERPRINT": "",
+        "RUN_ID": "1",
+        "GH_TOKEN": "stub",
+    }
+    env.update(overrides)
+
+    with tempfile.TemporaryDirectory() as temp_dir:
+        calls = Path(temp_dir) / "calls"
+        calls.touch()
+        stub = Path(temp_dir) / "gh"
+        stub.write_text(
+            "#!/usr/bin/env bash\n"
+            'case "$*" in\n'
+            "  *\"pr edit\"*)"
+            " printf '%s\\n' \"$*\" | grep -oE '(add|remove)-label [a-z-]+'"
+            f' >> "{calls}" ;;\n'
+            "  *\"/commits\"*) printf '%s\\n' 'feat(x)!: breaks' ;;\n"
+            "  *) exit 1 ;;\n"
+            "esac\n",
+            encoding="utf-8",
+        )
+        stub.chmod(0o755)
+        env["PATH"] = f"{temp_dir}:{os.environ.get('PATH', '/usr/bin:/bin')}"
+        env["GITHUB_OUTPUT"] = str(Path(temp_dir) / "out")
+        result = subprocess.run(
+            ["bash", "-c", body],
+            cwd=temp_dir,
+            env=env,
+            text=True,
+            capture_output=True,
+            check=False,
+        )
+        if result.returncode != 0:
+            raise AssertionError(f"reporter failed for {state!r}: {result.stderr}")
+        return calls.read_text(encoding="utf-8").split()
+
+
 def parse_github_output(output):
     values = {}
     for line in output.splitlines():
@@ -1219,6 +1292,51 @@ def test_an_unevaluated_acknowledgement_is_not_reported_as_missing(self):
         self.assertIn("Author acknowledgement", rendered)
         self.assertIn("missing", rendered)
 
+    def test_the_fact_label_outlives_the_decision(self):
+        # Run it, per state, and read the `gh pr edit` calls. Asserting on the
+        # composite's text cannot tell which state made which call: moving
+        # `--add-label "$LABEL"` into the `waiting` branch would leave every
+        # string in place and stop the fact label from surviving a decision.
+        for state in ("waiting", "approved", "rejected", "acknowledgement"):
+            with self.subTest(state=state):
+                calls = run_reporter_labels(state)
+                pairs = list(zip(calls[::2], calls[1::2]))
+
+                self.assertIn(
+                    ("add-label", "breaking-change"),
+                    pairs,
+                    "the fact label belongs on every state the gate reports",
+                )
+                self.assertNotIn(
+                    ("remove-label", "breaking-change"),
+                    pairs,
+                    "approving does not make the break go away",
+                )
+
+                expected = "add-label" if state == "waiting" else "remove-label"
+                self.assertIn((expected, "awaiting-breaking-approval"), pairs)
+
+    def test_the_two_labels_can_be_turned_off_independently(self):
+        # Cosmetic throughout: a repository that declared neither still gets
+        # its gate, and one that wants only the fact can say so.
+        self.assertEqual([], run_reporter_labels("waiting", LABEL="", PENDING_LABEL=""))
+        self.assertEqual(
+            ["add-label", "breaking-change"],
+            run_reporter_labels("waiting", PENDING_LABEL=""),
+        )
+
+    def test_both_labels_go_when_the_break_does(self):
+        body = extract_run_body(
+            extract_job(APPROVAL_WORKFLOW, "outcome"), "Resolve the verdict"
+        )
+        # The one moment the fact stops being true. This cleanup was lost in an
+        # earlier rewrite, leaving `breaking-change` on a pull request whose
+        # breaking commit had been reworded away.
+        self.assertIn('for stale in "$LABEL" "$PENDING_LABEL"', body)
+        none_at = body.index("state=none")
+        cleanup_at = body.index('for stale in "$LABEL" "$PENDING_LABEL"')
+        self.assertLess(none_at, cleanup_at)
+
     def test_the_fingerprint_comes_from_the_breaking_commits(self):
         detect = extract_job(APPROVAL_WORKFLOW, "detect")
         self.assertIn("src/validate/breaking-change-commits@", detect)