fix: isolate trigger PR admission from sweep health

.github/workflows/knowledge-model-policy.yml

@@ -17,9 +17,23 @@ permissions:
 
 jobs:
   declarations:
-    if: github.event_name != 'issue_comment' || github.event.issue.pull_request
+    # Keep eligibility identical here and in the group: skipped comments must not
+    # replace a pending writer, regardless of GitHub's evaluation order.
+    if: >-
+      ${{ github.event_name != 'issue_comment' || !github.event.issue ||
+          !github.event.comment || !github.event.action ||
+          (github.event.issue.pull_request && (github.event.action != 'created' ||
+          !github.event.comment.body ||
+          contains(github.event.comment.body, 'knowledge-policy-approval'))) }}
     concurrency:
-      group: knowledge-policy-status-writer
+      group: >-
+        ${{ (github.event_name != 'issue_comment' || !github.event.issue ||
+            !github.event.comment || !github.event.action ||
+            (github.event.issue.pull_request && (github.event.action != 'created' ||
+            !github.event.comment.body ||
+            contains(github.event.comment.body, 'knowledge-policy-approval')))) &&
+            'knowledge-policy-status-writer' ||
+            format('knowledge-policy-skipped-{0}-{1}', github.run_id, github.run_attempt) }}
       cancel-in-progress: false
     runs-on: ubuntu-latest
     timeout-minutes: 10
@@ -35,3 +49,4 @@ jobs:
         env:
           GITHUB_TOKEN: ${{ github.token }}
         run: bash tools/run_knowledge_model_policy.sh
+

docs/knowledge-model-policy.md

@@ -310,7 +310,7 @@ rejection neither greens nor fails another PR's status.
 The workflow handles PR open/reopen/close/commits/body-or-base edits on any target
 branch (a PR against another base can share a main PR's head), approval-comment
 creation/edit/deletion, every main push and manual dispatch. Main pushes refresh
-all open main-target PRs. All triggers share one serialized status writer and each
+all open main-target PRs. All eligible triggers share one serialized status writer and each
 run refreshes every open main-target PR, so replacing a pending job cannot lose another
 PR's refresh or let an older run overwrite a newer result. It checks out `main`,
 executes only trusted code, installs no PR
@@ -321,8 +321,14 @@ approvals are reread before publication; changes fail closed and need a fresh ru
 PR edits and status publication are not atomic: integrators still check the
 current head and latest run.
 
-The shared concurrency group belongs to the declaration job. Ordinary Issue
-comments skip that job and do not displace queued PR/base refreshes.
+The shared concurrency group belongs to the declaration job. Ordinary non-PR Issue
+comments skip that job. A newly created, nonempty PR comment without the
+`knowledge-policy-approval` marker also skips the job; every PR comment edit or
+deletion remains eligible, including removal of an approval marker. Missing event
+fields or an empty body are handled conservatively. The same eligibility expression
+controls both the job condition and its concurrency group: skipped events use a
+unique no-op group, never the status writer group. They therefore cannot replace
+a pending legitimate refresh, regardless of GitHub's condition/queue evaluation order.
 
 File pagination must exactly match `changed_files`; more than the GitHub files
 API's 3,000-file limit fails visibly. Exception/policy approvals require complete
@@ -331,6 +337,56 @@ never become compliant/non-applicable results. Current rules have no merge queue
 `merge_group` is explicitly unsupported. Validate a trusted integration before
 enabling a queue with this required check.
 
+HTTP diagnostics retain only the response status, a conservative category,
+numeric rate-limit timing/remaining headers, and a bounded GitHub request ID.
+Raw response messages, error arrays, request URLs and credentials are not logged.
+A 403 alone is not proof of rate limiting; a 422 is a validation error, not proof
+that the status-history cap was reached. Read requests may retry once when an
+explicit rate-limit delay fits inside their remaining retry budget (at most 60
+seconds). The wait is deducted before passing the remaining timeout to the retry.
+The urllib timeout applies to socket operations, not a strict total wall-clock
+deadline for reading the response.
+Status POSTs are never retried by the transport. The existing outer sweep retry
+plan remains unchanged; this diagnostic improvement does not repair permissions,
+restore exhausted status capacity, or guarantee that historical red checks clear.
+
+
+### Trigger-PR conclusion versus global sweep health
+
+Every eligible workflow event still refreshes **all open policy-branch PRs** through the
+single serialized writer. This is necessary because concurrency may replace a
+pending event. The runner still makes up to three attempts when global work is
+unfinished; a green triggering PR never stops those retries early. The triggering
+PR is evaluated first without dropping any peer from the sweep.
+
+Only after the final attempt does a PR or PR-comment job use that triggering PR's
+own verified result for its job conclusion. A `pull_request_target` result must
+match the immutable event head; rerunning an old event cannot turn its historical
+head green using a newer head's compliance. Missing/malformed event heads and
+advanced heads fail closed with an explicit stale-event reason. PR comments lack
+a head snapshot and retain current-head semantics. A peer PR's history/publication error
+therefore does not turn a verified compliant trigger PR red. A noncompliant
+trigger PR, unavailable evidence, status-publication failure, capacity failure,
+advanced head, or missing target still fails its own job. A freshly checked
+closed/non-policy-branch target is explicitly **not applicable**, not an admission
+or acceptance PASS. It is added to the sweep even when absent from the open list.
+Push-to-main and manual workflow-dispatch jobs continue to report global execution
+health. Explicit CLI `--pull-request` and dry-run behavior is unchanged.
+
+The runner deletes the report before each attempt. The checker atomically writes
+a report bound to repository, event digest/type, policy SHA and attempt identifier;
+the final consumer rejects missing, malformed, duplicate, stale or mismatched
+reports. It never falls back to an earlier attempt's success. Failed collection
+reads additionally produce synthetic target `0` in reporting sweeps (included in
+result counts), which blocks projection even if an individual result is green.
+
+All target results and global health remain in structured logs. The final scope,
+target kind, policy SHA and global summary are also written to the job summary;
+an incomplete/failed sweep emits a warning even when the trigger PR job passes.
+This isolates unrelated job conclusions without clearing any peer's failing
+commit status or hiding repository-wide failures. No admission rules, publishing
+permissions, status-history limits, or acceptance requirements are weakened.
+
 The bootstrap PR cannot exercise a target-branch workflow that has not landed:
 independently review and accept it before merging, then validate report-only on
 real PRs. Local/PR CI success does not close #50 or authorize required enforcement.
@@ -345,3 +401,4 @@ python tools/check_knowledge_model_policy.py --repository <owner/repo> --pull-re
 publishing statuses. It does not show the main workflow is operational. Production
 uses runtime-injected `GITHUB_TOKEN`; never pass tokens as CLI arguments. The
 existing ready lane runs unit tests on Python 3.10 and 3.11 in its full suite.
+

tests/test_knowledge_model_policy.py

@@ -329,9 +329,22 @@ def test_workflow_never_executes_pr_code_and_refreshes_metadata_and_base():
     assert value["permissions"] == {"contents": "read", "pull-requests": "read",
                                     "issues": "read", "statuses": "write"}
     assert "concurrency" not in value
-    assert value["jobs"]["declarations"]["concurrency"] == {
-        "group": "knowledge-policy-status-writer", "cancel-in-progress": False,
-    }
+    job = value["jobs"]["declarations"]
+    eligibility = (
+        "github.event_name != 'issue_comment' || !github.event.issue || "
+        "!github.event.comment || !github.event.action || "
+        "(github.event.issue.pull_request && (github.event.action != 'created' || "
+        "!github.event.comment.body || "
+        "contains(github.event.comment.body, 'knowledge-policy-approval')))"
+    )
+    assert " ".join(job["if"].split()) == "${{ " + eligibility + " }}"
+    group = (
+        "${{ (" + eligibility + ") && 'knowledge-policy-status-writer' || "
+        "format('knowledge-policy-skipped-{0}-{1}', github.run_id, github.run_attempt) }}"
+    )
+    assert " ".join(job["concurrency"]["group"].split()) == group
+    assert job["concurrency"]["cancel-in-progress"] is False
+    assert set(job["concurrency"]) == {"group", "cancel-in-progress"}
     steps = value["jobs"]["declarations"]["steps"]
     assert steps[0]["with"] == {"ref": "main", "persist-credentials": False}
     assert all("pip install" not in step.get("run", "") for step in steps)
@@ -1535,3 +1548,4 @@ def test_api_other_contexts_do_not_consume_the_budget(api_fixture, policy):
     seed_history(state, 400, context="other-ci")
     assert check_pr(api, 123, policy, BASE, False)["state"] == "success"
     assert latest_status(state)["state"] == "success"
+

tests/test_knowledge_policy_comment_routing.py

@@ -0,0 +1,110 @@
+"""Offline routing contracts, not an execution of GitHub's expression engine.
+
+Model the documented truthiness, case-insensitive contains, and short-circuit
+operators used by this exact expression. CI additionally parses the workflow YAML.
+https://docs.github.com/en/actions/reference/workflows-and-actions/expressions
+"""
+
+import re
+import unittest
+from pathlib import Path
+
+from tools import check_knowledge_model_policy as policy
+
+ROOT = Path(__file__).resolve().parents[1]
+ELIGIBILITY = (
+    "github.event_name != 'issue_comment' || !github.event.issue || "
+    "!github.event.comment || !github.event.action || "
+    "(github.event.issue.pull_request && (github.event.action != 'created' || "
+    "!github.event.comment.body || "
+    "contains(github.event.comment.body, 'knowledge-policy-approval')))"
+)
+WRITER = "knowledge-policy-status-writer"
+
+
+def truthy(value):
+    # GitHub objects/arrays are truthy, including empty objects/arrays.
+    if isinstance(value, (dict, list)):
+        return True
+    return bool(value)
+
+
+def eligible(event_name, event):
+    """Explicit model of ELIGIBILITY, with absent properties evaluating to null."""
+    issue = event.get("issue")
+    comment = event.get("comment")
+    action = event.get("action")
+    if (event_name.casefold() != "issue_comment" or not truthy(issue)
+            or not truthy(comment) or not truthy(action)):
+        return True
+    body = comment.get("body")
+    return truthy(issue.get("pull_request")) and (
+        action.casefold() != "created" or not truthy(body)
+        or "knowledge-policy-approval" in body.casefold()
+    )
+
+
+def group(event_name, event, run_id=1, run_attempt=1):
+    return WRITER if eligible(event_name, event) else (
+        f"knowledge-policy-skipped-{run_id}-{run_attempt}"
+    )
+
+
+class CommentRoutingTests(unittest.TestCase):
+    def test_identical_eligibility_controls_if_and_group(self):
+        self.assertIn(f"contains(github.event.comment.body, '{policy.APPROVAL}')", ELIGIBILITY)
+        text = (ROOT / ".github/workflows/knowledge-model-policy.yml").read_text()
+        expressions = [" ".join(value.split()) for value in re.findall(
+            r"\$\{\{ (.*?) \}\}", text, re.S
+        )]
+        self.assertEqual(expressions[0], ELIGIBILITY)
+        self.assertEqual(expressions[1], (
+            f"({ELIGIBILITY}) && '{WRITER}' || "
+            "format('knowledge-policy-skipped-{0}-{1}', github.run_id, github.run_attempt)"
+        ))
+        self.assertIn("cancel-in-progress: false", text)
+        self.assertIn("ref: main", text)
+        self.assertIn("persist-credentials: false", text)
+
+    def test_truth_matrix(self):
+        for is_pr in (False, True):
+            issue = {"pull_request": {}} if is_pr else {"number": 1}
+            for action in ("created", "edited", "deleted"):
+                for body in (None, "", "LGTM", "```knowledge-policy-approval\n{}\n```",
+                             "KNOWLEDGE-POLICY-APPROVAL"):
+                    event = {"issue": issue, "comment": {"body": body}, "action": action}
+                    expected = is_pr and (
+                        action != "created" or body in (None, "")
+                        or "knowledge-policy-approval" in body.casefold()
+                    )
+                    with self.subTest(is_pr=is_pr, action=action, body=body):
+                        self.assertEqual(eligible("issue_comment", event), expected)
+                        self.assertEqual(group("issue_comment", event) == WRITER, expected)
+
+    def test_missing_payload_is_conservative(self):
+        complete = {"issue": {"pull_request": {}}, "comment": {"body": "LGTM"},
+                    "action": "created"}
+        for key in complete:
+            for missing in (None, ""):
+                event = {**complete, key: missing}
+                self.assertTrue(eligible("issue_comment", event))
+            event = {key_: value for key_, value in complete.items() if key_ != key}
+            self.assertTrue(eligible("issue_comment", event))
+        self.assertTrue(eligible("issue_comment", {}))
+        self.assertTrue(eligible("issue_comment", {**complete, "comment": {}}))
+
+    def test_other_triggers_keep_shared_writer(self):
+        for event_name in ("pull_request_target", "push", "workflow_dispatch"):
+            self.assertEqual(group(event_name, {}), WRITER)
+
+    def test_irrelevant_comment_never_enters_writer_group(self):
+        event = {"issue": {"pull_request": {}}, "comment": {"body": "Thanks!"},
+                 "action": "created"}
+        groups = {group("issue_comment", event, run, attempt)
+                  for run in (1, 2) for attempt in (1, 2)}
+        self.assertEqual(len(groups), 4)
+        self.assertNotIn(WRITER, groups)
+
+
+if __name__ == "__main__":
+    unittest.main()