fix(pr-validation): give the comment job permission to comment

.agents/skills/workflow/SKILL.md

@@ -211,6 +211,21 @@ The two modes have opposite goals — design them accordingly:
 
 **Real run (`false`):** no extra `echo`, no debug flags, let failures surface via exit codes, one `::notice::` summary on success at most.
 
+**Exception — `continue-on-error` steps must annotate their own failures.** The
+rule above leans on the exit code to carry a failure. A step that deliberately
+swallows its exit code has no such channel, so a failure there is invisible: the
+job reports success and nobody looks at the log. Emit `::warning::` on the
+failure path.
+
+This is not hypothetical. `pr-validation.yml`'s breaking-change comment took a
+403 on every run from 2026-08-06 and reported success each time, because the
+step was `continue-on-error` and said nothing. The comment it posts explains to
+an author what to add; for two months it never arrived, and the only symptom was
+a blocking check failing with no explanation.
+
+Gate the annotation on the failure itself (`if: steps.<id>.outcome == 'failure'`)
+so a successful run stays silent.
+
 ## Step section titles
 
 When a job has more than one logical group of steps, separate each group with a titled section comment:

.github/workflows/pr-validation.yml

@@ -490,12 +490,21 @@ jobs:
     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: read
-      issues: write
+      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:
@@ -574,6 +583,15 @@ jobs:
             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/pr-validation.md

@@ -286,7 +286,7 @@ Full detail: [`breaking-change-approval.md`](breaking-change-approval.md).
 | `blocking-checks` | 1 (fail-fast) | guard enforcement, `pr-source-branch`, `pr-title`, `pr-description`, `pr-commit-signatures` | always; drafts run guard enforcement only |
 | `advisory-checks` | 2 (informational) | `pr-metadata`, `pr-size`, `pr-labels` | non-draft, regardless of the blocking result |
 | `pr-checks-summary` | — | `pr-checks-summary` | always (writes to step summary) |
-| `breaking-change-comment` | — | `actions/github-script` | every non-draft live run, including detector failure or cancellation |
+| `breaking-change-comment` | — | `actions/github-script` | every non-draft live run, including detector failure or cancellation. Needs `pull-requests: write` — commenting on a PR is authorised as a pull request, not as an issue |
 | `pr-validation-report` | — | `pr-validation-reporter` | non-draft (upserts single PR comment) |
 | `notify` | — | `slack-notify.yml` | non-draft, `!dry_run` |
 

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

@@ -686,15 +686,20 @@ def test_draft_pull_requests_still_reach_guard_enforcement(self):
 
     def test_comment_job_is_write_minimal_and_does_not_checkout(self):
         job = extract_job(WORKFLOW, "breaking-change-comment")
-        permissions = re.search(
-            r"(?m)^    permissions:\n(?P<body>(?:      [^\n]+\n?)+)", job
-        )
-        self.assertIsNotNone(permissions)
-        self.assertEqual(
-            {"pull-requests: read", "issues: write"},
-            {line.strip() for line in permissions.group("body").splitlines()},
-        )
-        self.assertNotIn("contents:", permissions.group("body"))
+        permissions = yaml.safe_load(WORKFLOW)["jobs"]["breaking-change-comment"][
+            "permissions"
+        ]
+        # `pull-requests: write` is the minimum that works, not a widening.
+        # This set used to be {pull-requests: read, issues: write}, which looks
+        # narrower and is simply wrong: commenting on a pull request is
+        # authorised as a pull request, so the step took a 403 on every run
+        # from 2026-08-06 and reported success anyway.
+        #
+        # The lesson is in the shape of the old assertion, not just its value —
+        # it pinned a permission set nobody had watched work, and so kept the
+        # bug in place rather than catching it.
+        self.assertEqual({"pull-requests": "write"}, permissions)
+        self.assertNotIn("contents", permissions)
         self.assertNotIn("actions/checkout", job)
         self.assertIn("github-token: ${{ github.token }}", job)
 
@@ -840,6 +845,45 @@ def test_analysis_workflows_expose_scoped_test_runner_overrides(self):
                 )
 
 
+class CommentPermissionTests(unittest.TestCase):
+    def test_jobs_that_comment_on_a_pull_request_can_write_to_it(self):
+        # Commenting on a pull request needs `pull-requests: write`. The
+        # issue-comments endpoint is the one used, but GitHub authorises the
+        # subject as a pull request, so `issues: write` is not enough — it
+        # covers real issues only. The mistake fails as a 403 at runtime, and
+        # these steps are continue-on-error, so it reports success.
+        workflow = yaml.safe_load(WORKFLOW)
+        top_level = workflow.get("permissions") or {}
+
+        for job_id, job in workflow["jobs"].items():
+            if not isinstance(job, dict):
+                continue
+            body = yaml.safe_dump(job)
+            if not any(
+                marker in body
+                for marker in ("createComment", "updateComment", "issues/comments")
+            ):
+                continue
+
+            with self.subTest(job=job_id):
+                # A job-level block replaces the workflow's, it does not merge,
+                # so an explicit block has to carry the permission itself.
+                effective = job.get("permissions") or top_level
+                self.assertEqual(
+                    "write",
+                    effective.get("pull-requests"),
+                    f"{job_id} comments on a pull request and would take a 403",
+                )
+
+    def test_a_failed_comment_is_visible_rather_than_swallowed(self):
+        job = extract_job(WORKFLOW, "breaking-change-comment")
+        # continue-on-error is right for a cosmetic comment, but silence is how
+        # a 403 ran for two months with every job green.
+        self.assertIn("continue-on-error: true", job)
+        self.assertIn("::warning::", job)
+        self.assertIn("steps.comment.outcome == 'failure'", job)
+
+
 class AdvisoryChecksTests(unittest.TestCase):
     def test_advisory_checks_do_not_cascade_off_a_blocking_failure(self):
         job = extract_job(WORKFLOW, "advisory-checks")