feat(pr-validation): one breaking-change comment, and no checkout

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

@@ -246,49 +246,25 @@ jobs:
             echo "::notice::PR #${PR_NUMBER} (${HEAD_REF} → ${BASE_REF}) is not gated — ${REASON}"
           fi
 
-      # The composite reads Git history, so it needs `origin/<base>` and a HEAD
-      # that is the pull request. The second checkout exists because the first
-      # one leaves HEAD on the event's commit, which is only the pull request
-      # under `pull_request`. `clean: false` keeps what the first one fetched.
-      - name: Check out full history
-        id: checkout
-        if: steps.scope.outputs.in-scope == 'true' && inputs.detected_breaking_changes == ''
-        continue-on-error: true
-        uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v6
-        with:
-          fetch-depth: 0
-          persist-credentials: false
-
-      - name: Move HEAD onto the pull request head
-        id: checkout-head
-        if: steps.scope.outputs.in-scope == 'true' && steps.checkout.outcome == 'success'
-        continue-on-error: true
-        uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v6
-        with:
-          ref: refs/pull/${{ steps.scope.outputs.pr-number }}/head
-          fetch-depth: 0
-          clean: false
-          persist-credentials: false
-
-      # Nothing from the checked-out tree is executed — the composite only reads
-      # commit messages — so fork code never runs here.
-      - name: Detect breaking changes
-        id: guard
-        if: steps.scope.outputs.in-scope == 'true' && steps.checkout-head.outcome == 'success' && inputs.detected_breaking_changes == ''
-        continue-on-error: true
-        uses: LerianStudio/github-actions-shared-workflows/src/validate/breaking-change-guard@v1
-        with:
-          base-ref: ${{ steps.scope.outputs.base-ref }}
-          breaking-change-acknowledgement: ${{ inputs.author_acknowledgement }}
-          acknowledgement-match-mode: exact-visible-line
-
       # Which commits tripped the gate, and a digest of the set. The digest is
       # what lets an approval survive a push that did not touch the breaking
       # change — see the `request` job.
+      #
+      # This also detects, when the caller did not. It replaced a pair of
+      # checkouts and the Git-reading composite: CodeQL flagged them as
+      # `actions/untrusted-checkout`, and it was right to — a non-privileged
+      # workflow was putting a contributor's tree on disk beside MANAGE_TOKEN.
+      # Nothing executed it, but nothing needed it either. Reading the API
+      # removes the tree, the token exposure and the alert together.
+      #
+      # `breaking-change-guard` stays the canonical detector and still runs in
+      # pr-validation over the real commit range; the patterns here are kept
+      # identical to it and tested against the same negative cases.
       - name: List the breaking-change commits
         id: commits
         if: steps.scope.outputs.in-scope == 'true'
         continue-on-error: true
+        # No checkout: it reads the pull request through the API.
         uses: LerianStudio/github-actions-shared-workflows/src/validate/breaking-change-commits@develop
         with:
           github-token: ${{ secrets.MANAGE_TOKEN || github.token }}
@@ -301,11 +277,8 @@ jobs:
           IN_SCOPE: ${{ steps.scope.outputs.in-scope }}
           SUPPLIED_HAS_BREAKING_CHANGES: ${{ inputs.detected_breaking_changes }}
           SUPPLIED_ACKNOWLEDGED: ${{ inputs.detected_acknowledged }}
-          CHECKOUT_OUTCOME: ${{ steps.checkout.outcome }}
-          CHECKOUT_HEAD_OUTCOME: ${{ steps.checkout-head.outcome }}
-          GUARD_OUTCOME: ${{ steps.guard.outcome }}
-          RAW_HAS_BREAKING_CHANGES: ${{ steps.guard.outputs.has-breaking-changes }}
-          RAW_ACKNOWLEDGED: ${{ steps.guard.outputs.approved }}
+          COMMITS_OUTCOME: ${{ steps.commits.outcome }}
+          BREAKING_COUNT: ${{ steps.commits.outputs.count }}
         run: |
           set -euo pipefail
 
@@ -314,9 +287,18 @@ jobs:
           detection_succeeded=false
 
           if [ "$IN_SCOPE" != "true" ]; then
-            # Nothing was scanned, and nothing needs to be: an out-of-scope pull
-            # request is a successful no-op, not a failed detection.
+            # Out of scope means no approval is required — it does not mean
+            # nothing was found. When the caller already detected, keep that
+            # answer: the acknowledgement is still enforced everywhere, and the
+            # author needs to be told which line to add. Scanning here is what
+            # would be pointless, not knowing.
             detection_succeeded=true
+            case "$SUPPLIED_HAS_BREAKING_CHANGES" in
+              true | false )
+                has_breaking_changes="$SUPPLIED_HAS_BREAKING_CHANGES"
+                [ "$SUPPLIED_ACKNOWLEDGED" = "true" ] && acknowledged=true
+                ;;
+            esac
           elif [ -n "$SUPPLIED_HAS_BREAKING_CHANGES" ]; then
             # The caller already ran the same composite over the same commits.
             # Anything but a clean boolean is a wiring mistake, and a wiring
@@ -336,15 +318,17 @@ jobs:
                 echo "detected_breaking_changes must be 'true', 'false', 'unknown' or empty; got '${SUPPLIED_HAS_BREAKING_CHANGES}'." >&2
                 ;;
             esac
-          elif [ "$CHECKOUT_OUTCOME" = "success" ] && [ "$CHECKOUT_HEAD_OUTCOME" = "success" ] \
-            && [ "$GUARD_OUTCOME" = "success" ] \
-            && { [ "$RAW_HAS_BREAKING_CHANGES" = "true" ] || [ "$RAW_HAS_BREAKING_CHANGES" = "false" ]; } \
-            && { [ "$RAW_ACKNOWLEDGED" = "true" ] || [ "$RAW_ACKNOWLEDGED" = "false" ]; }; then
-            has_breaking_changes="$RAW_HAS_BREAKING_CHANGES"
-            acknowledged="$RAW_ACKNOWLEDGED"
+          elif [ "$COMMITS_OUTCOME" = "success" ] && [ -n "$BREAKING_COUNT" ]; then
+            [ "$BREAKING_COUNT" -gt 0 ] && has_breaking_changes=true
             detection_succeeded=true
+            # Left empty on purpose, not false. Without the caller's verdict
+            # the acknowledgement was never evaluated here — the detector that
+            # reads the description lives in pr-validation — and reporting
+            # "missing" for something nobody looked at would send the author
+            # after a line no check is asking for.
+            acknowledged=""
           else
-            echo "Breaking change detection failed: checkout=${CHECKOUT_OUTCOME} checkout_head=${CHECKOUT_HEAD_OUTCOME} guard=${GUARD_OUTCOME}" >&2
+            echo "Breaking change detection failed: commits=${COMMITS_OUTCOME} count=${BREAKING_COUNT:-missing}" >&2
           fi
 
           {
@@ -510,6 +494,27 @@ jobs:
           create-if-missing: ${{ inputs.create_approval_environment && 'true' || 'false' }}
           reviewers: ${{ inputs.approval_reviewers != '' && inputs.approval_reviewers || steps.codeowners.outputs.reviewers }}
 
+      # Out of scope, so no approval is coming — but the acknowledgement is
+      # enforced on every pull request, and an author who omits it otherwise
+      # sees Blocking Checks fail with nothing explaining what to add.
+      - name: Report the acknowledgement requirement
+        if: >-
+          needs.detect.result == 'success'
+          && needs.detect.outputs.in-scope != 'true'
+          && needs.detect.outputs.has-breaking-changes == 'true'
+          && !inputs.dry_run
+        continue-on-error: true
+        uses: LerianStudio/github-actions-shared-workflows/src/notify/breaking-change-approval-reporter@develop
+        with:
+          github-token: ${{ secrets.MANAGE_TOKEN || github.token }}
+          pr-number: ${{ needs.detect.outputs.pr-number }}
+          state: acknowledgement
+          head-sha: ${{ needs.detect.outputs.head-sha }}
+          acknowledged: ${{ needs.detect.outputs.acknowledged }}
+          author-acknowledgement: ${{ inputs.author_acknowledgement }}
+          label: ''
+          enabled: ${{ inputs.enable_comment && 'true' || 'false' }}
+
       - name: Request the approval on the pull request
         if: >-
           needs.detect.result == 'success'
@@ -521,6 +526,7 @@ jobs:
           github-token: ${{ secrets.MANAGE_TOKEN || github.token }}
           pr-number: ${{ needs.detect.outputs.pr-number }}
           state: waiting
+          author-acknowledgement: ${{ inputs.author_acknowledgement }}
           fingerprint: ${{ needs.detect.outputs.fingerprint }}
           reviewers: ${{ inputs.approval_reviewers != '' && inputs.approval_reviewers || steps.codeowners.outputs.reviewers }}
           actual-reviewers: ${{ steps.environment.outputs.reviewers }}

.github/workflows/pr-validation.yml

@@ -296,7 +296,17 @@ jobs:
     uses: ./.github/workflows/breaking-change-approval.yml
     with:
       detected_breaking_changes: ${{ needs.breaking-change-guard.result == 'success' && needs.breaking-change-guard.outputs.detection-succeeded == 'true' && needs.breaking-change-guard.outputs.has-breaking-changes || 'unknown' }}
+      # The acknowledgement state travels with it, so one comment can speak for
+      # both controls. It replaces `breaking-change-comment`, removed here: two
+      # bot comments about the same breaking change, one headed by a ✅ for the
+      # author's acknowledgement and one asking for a decision, read as a
+      # contradiction. The ✅ was taken for the approval.
       detected_acknowledged: ${{ needs.breaking-change-guard.outputs.approved }}
+      # Passed explicitly rather than left to the gate's default. The comment
+      # tells the author which line to paste, and the detector decides whether
+      # what they pasted counts — two literals in two files, previously kept
+      # equal by hand. A drift would have the gate teaching a line that fails.
+      author_acknowledgement: 'Breaking change acknowledged: I understand that this PR intentionally introduces a breaking change and requires the next release to be a major version.'
       scope_rules: ${{ inputs.breaking_change_scope_rules }}
       additional_scope_rules: ${{ inputs.breaking_change_additional_scope_rules }}
       force_labels: ${{ inputs.breaking_change_force_labels }}
@@ -485,113 +495,6 @@ jobs:
           dry-run: ${{ inputs.dry_run && 'true' || 'false' }}
 
   # ----------------- Breaking Change Feedback Comment (best effort) -----------------
-  breaking-change-comment:
-    name: Breaking Change Comment
-    runs-on: ${{ vars.GENERAL_RUNNERS || inputs.runner_type }}
-    needs: [breaking-change-guard]
-    if: always() && github.event_name == 'pull_request' && github.event.pull_request.draft != true && !inputs.dry_run
-    # `pull-requests: write`, not `issues: write`. The step posts through the
-    # issue-comments endpoint, but the subject is a pull request — and GitHub
-    # authorises those as pull requests. `issues: write` covers real issues
-    # only, so this job spent from 2026-08-06 taking a 403 on every run and
-    # reporting success, because the step is continue-on-error.
-    #
-    # The job-level block narrows the workflow's own `pull-requests: write`, so
-    # declaring `read` here was not least privilege — it was below what the step
-    # needs to work at all.
-    permissions:
-      pull-requests: write
-
-    steps:
-      - name: Manage breaking change guard comment
-        id: comment
-        continue-on-error: true
-        uses: actions/github-script@3a2844b7e9c422d3c10d287c895573f7108da1b3 # v9.0.0
-        env:
-          GUARD_JOB_RESULT: ${{ needs.breaking-change-guard.result }}
-          DETECTION_SUCCEEDED: ${{ needs.breaking-change-guard.outputs.detection-succeeded }}
-          HAS_BREAKING_CHANGES: ${{ needs.breaking-change-guard.outputs.has-breaking-changes }}
-          BREAKING_CHANGE_ACKNOWLEDGED: ${{ needs.breaking-change-guard.outputs.approved }}
-          EVENT_HEAD_SHA: ${{ github.event.pull_request.head.sha }}
-          ACKNOWLEDGEMENT: 'Breaking change acknowledged: I understand that this PR intentionally introduces a breaking change and requires the next release to be a major version.'
-        with:
-          github-token: ${{ github.token }}
-          script: |
-            const marker = '<!-- lerian-breaking-change-guard -->';
-            const guardJobSucceeded = process.env.GUARD_JOB_RESULT === 'success';
-            const detectionSucceeded = process.env.DETECTION_SUCCEEDED === 'true';
-            const hasBreakingChanges = process.env.HAS_BREAKING_CHANGES === 'true';
-            const acknowledged = process.env.BREAKING_CHANGE_ACKNOWLEDGED === 'true';
-            const eventHeadSha = process.env.EVENT_HEAD_SHA;
-            const acknowledgement = process.env.ACKNOWLEDGEMENT;
-
-            const current = await github.rest.pulls.get({
-              owner: context.repo.owner,
-              repo: context.repo.repo,
-              pull_number: context.issue.number,
-            });
-            if (!eventHeadSha || current.data.head.sha !== eventHeadSha) {
-              core.warning(`Skipping stale breaking change comment update: event head ${eventHeadSha || 'missing'}, current head ${current.data.head.sha}.`);
-              return;
-            }
-
-            const comments = await github.paginate(github.rest.issues.listComments, {
-              owner: context.repo.owner,
-              repo: context.repo.repo,
-              issue_number: context.issue.number,
-              per_page: 100,
-            });
-            const owned = comments.filter(comment =>
-              comment.user?.login === 'github-actions[bot]'
-              && (comment.body === marker || comment.body?.startsWith(`${marker}\n`))
-            );
-            const deleteComment = comment => github.rest.issues.deleteComment({
-              owner: context.repo.owner,
-              repo: context.repo.repo,
-              comment_id: comment.id,
-            });
-            const upsertOwned = async body => {
-              const params = { owner: context.repo.owner, repo: context.repo.repo, body };
-
-              if (owned.length > 0) {
-                await github.rest.issues.updateComment({ ...params, comment_id: owned[0].id });
-                await Promise.all(owned.slice(1).map(deleteComment));
-                return;
-              }
-
-              await github.rest.issues.createComment({
-                ...params,
-                issue_number: context.issue.number,
-              });
-            };
-
-            if (!guardJobSucceeded || !detectionSucceeded) {
-              const body = `${marker}\n## Breaking change guard\n\n**Status:** ⚠️ Detection failed\n\nThe workflow could not determine whether this pull request contains breaking changes. Blocking Checks failed closed.`;
-              await upsertOwned(body);
-              return;
-            }
-
-            if (!hasBreakingChanges) {
-              await Promise.all(owned.map(deleteComment));
-              return;
-            }
-
-            const state = acknowledged ? '✅ Author acknowledged' : '🚫 Awaiting author acknowledgement';
-            const detail = acknowledged
-              ? 'The exact visible author acknowledgement line is present in the pull request description. It records intentional awareness and does not grant maintainer permission.'
-              : 'The PR author can acknowledge intentional awareness by adding this exact visible line to the pull request description:';
-            const body = `${marker}\n## Breaking change guard\n\n**Status:** ${state}\n\n${detail}\n\n\`${acknowledgement}\``;
-            await upsertOwned(body);
-
-      # Cosmetic enough to keep continue-on-error — losing the comment should
-      # not fail validation — but not so cosmetic that it should fail in
-      # silence. Swallowing it is what let a 403 run for two months while every
-      # job reported green.
-      - name: Surface a failed comment
-        if: always() && steps.comment.outcome == 'failure'
-        run: |
-          echo "::warning::Could not post the breaking change guard comment. The validation verdict is unaffected, but the author gets no explanation of what to add. Check the step log above — a 403 here usually means the job lacks 'pull-requests: write'."
-
   # ----------------- PR Validation Summary Comment -----------------
   pr-validation-report:
     name: PR Validation Report

docs/breaking-change-approval.md

@@ -52,7 +52,7 @@ own:
 | Composite | Responsibility |
 |---|---|
 | [`pr-gate-scope`](../src/validate/pr-gate-scope/README.md) | Is this pull request in scope, and why — base → head rules, label overrides, author vetoes |
-| [`breaking-change-guard`](../src/validate/breaking-change-guard/README.md) | Does the branch carry a breaking-change commit |
+| [`breaking-change-guard`](../src/validate/breaking-change-guard/README.md) | The canonical detector, over the real commit range. Runs in `pr-validation.yml`, whose verdict this gate is handed |
 | [`breaking-change-commits`](../src/validate/breaking-change-commits/README.md) | Which commits carry the break, and a digest of the set |
 | [`codeowners-reviewers`](../src/validate/codeowners-reviewers/README.md) | Who owns this repository, per its CODEOWNERS |
 | [`environment-protection`](../src/validate/environment-protection/README.md) | Does `breaking-approval` exist with required reviewers — creating it when it does not |
@@ -177,10 +177,19 @@ jobs:
       MANAGE_TOKEN: ${{ secrets.MANAGE_TOKEN }}
 ```
 
-Called this way the workflow detects on its own, with two checkouts: a plain one
-for the `origin/<base>` remote-tracking refs, then `refs/pull/<n>/head` with
-`clean: false` to move `HEAD` onto the pull request. Nothing from the tree is
-executed — only commit messages are read — so fork code never runs.
+Called this way the workflow detects on its own, through the API — it checks
+out nothing at all.
+
+It used to use two checkouts and read Git directly. CodeQL flagged that as
+`actions/untrusted-checkout`, and the alert was fair: a non-privileged workflow
+was placing a contributor's tree on disk next to `MANAGE_TOKEN`. Nothing ever
+executed it, but nothing needed it either, and the tree, the token exposure and
+the alert all went away together.
+
+The acknowledgement is **not** evaluated on this path — the detector that reads
+the pull request description runs in `pr-validation.yml`. The comment omits that
+row rather than reporting it missing, because telling an author to add a line no
+check is asking for is worse than saying nothing.
 
 ### Testing before adoption