fix: tighten replay provenance parity

Author: PLeVasseur

.github/workflows/reviewer-bot-pr-comment-router.yml

@@ -52,7 +52,7 @@ jobs:
           comment_author = str((comment.get('user') or {}).get('login') or '').strip()
           comment_user_type = str((comment.get('user') or {}).get('type') or '').strip()
           comment_sender_type = str(sender.get('type') or '').strip()
-          comment_author_association = str(comment.get('author_association') or '').strip()
+          comment_author_association = str(comment.get('author_association') or '').strip().upper()
           def _classify_issue_comment_actor(comment_user_type, comment_author, comment_sender_type, installation_id, performed_via_github_app):
               if comment_user_type == 'Bot' or comment_author.endswith('[bot]'):
                   return 'bot_account'
@@ -62,6 +62,21 @@ jobs:
                   return 'repo_user_principal'
               return 'unknown_actor'
 
+          def _route_pr_comment_outcome(actor_class, repo, pr_head_full_name, pr_author, comment_author_association, pull_request_read_failed=False):
+              if actor_class != 'repo_user_principal':
+                  return 'safe_noop'
+              if pull_request_read_failed:
+                  return 'deferred_reconcile'
+              if (
+                  not pr_head_full_name
+                  or not pr_author
+                  or pr_head_full_name != repo
+                  or pr_author == 'dependabot[bot]'
+                  or str(comment_author_association or '').strip().upper() not in {'OWNER', 'MEMBER', 'COLLABORATOR'}
+              ):
+                  return 'deferred_reconcile'
+              return 'trusted_direct'
+
           def _performed_via_github_app_truth(value):
               if isinstance(value, bool):
                   return value
@@ -86,7 +101,7 @@ jobs:
           pr_author = ''
 
           if actor_class != 'repo_user_principal':
-              route_outcome = 'safe_noop'
+              route_outcome = _route_pr_comment_outcome(actor_class, repo, pr_head_full_name, pr_author, comment_author_association)
           else:
               pr_number = issue['number']
               req = urllib.request.Request(
@@ -100,19 +115,12 @@ jobs:
                   with urllib.request.urlopen(req) as response:
                       pull_request = json.load(response)
               except urllib.error.URLError:
-                  route_outcome = 'deferred_reconcile'
+                  route_outcome = _route_pr_comment_outcome(actor_class, repo, pr_head_full_name, pr_author, comment_author_association, pull_request_read_failed=True)
               else:
                   head_repo = pull_request.get('head', {}).get('repo') or {}
                   pr_head_full_name = str(head_repo.get('full_name') or '').strip()
                   pr_author = str((pull_request.get('user') or {}).get('login') or '').strip()
-                  if (
-                      not pr_head_full_name
-                      or not pr_author
-                      or pr_head_full_name != repo
-                      or pr_author == 'dependabot[bot]'
-                      or comment_author_association not in {'OWNER', 'MEMBER', 'COLLABORATOR'}
-                  ):
-                      route_outcome = 'deferred_reconcile'
+                  route_outcome = _route_pr_comment_outcome(actor_class, repo, pr_head_full_name, pr_author, comment_author_association)
 
           with open(os.environ['GITHUB_OUTPUT'], 'a', encoding='utf-8') as handle:
               print(f'route_outcome={route_outcome}', file=handle)

scripts/reviewer_bot_lib/reconcile_reads.py

@@ -109,11 +109,16 @@ def read_live_comment_replay_context(live_comment: dict, payload: dict) -> LiveC
             comment_sender_type_available = True
     installation = live_comment.get("installation")
     comment_installation_id = None
-    comment_installation_id_available = "installation" in live_comment
+    comment_installation_id_available = False
     if isinstance(installation, dict):
         installation_id = installation.get("id")
         if installation_id is not None and str(installation_id).strip():
-            comment_installation_id = str(installation_id).strip()
+            try:
+                if int(installation_id) > 0:
+                    comment_installation_id = str(installation_id).strip()
+                    comment_installation_id_available = True
+            except (TypeError, ValueError):
+                pass
     performed_via_app_available = False
     performed_via_app = live_comment.get("performed_via_github_app")
     comment_performed_via_github_app = None
@@ -122,11 +127,13 @@ def read_live_comment_replay_context(live_comment: dict, payload: dict) -> LiveC
             comment_performed_via_github_app = performed_via_app
             performed_via_app_available = True
         elif isinstance(performed_via_app, dict):
-            try:
-                comment_performed_via_github_app = int(performed_via_app.get("id") or 0) > 0
-            except (TypeError, ValueError):
-                comment_performed_via_github_app = False
-            performed_via_app_available = True
+            app_id = performed_via_app.get("id")
+            if app_id is not None and str(app_id).strip():
+                try:
+                    comment_performed_via_github_app = int(app_id) > 0
+                    performed_via_app_available = True
+                except (TypeError, ValueError):
+                    pass
         elif performed_via_app is None:
             comment_performed_via_github_app = False
             performed_via_app_available = True

tests/contract/reviewer_bot/test_workflow_files.py

@@ -10,6 +10,7 @@
 import yaml
 
 from scripts.reviewer_bot_core import comment_routing_policy
+from scripts.reviewer_bot_lib.context import PrCommentAdmission
 
 
 def _load_observer_contract_matrix() -> dict:
@@ -129,6 +130,16 @@ def test_pr_comment_router_normalizes_performed_via_app_without_raw_truthiness()
     assert "bool(comment.get('performed_via_github_app'))" not in workflow_text
 
 
+def _load_pr_comment_router_inline_namespace():
+    workflow_text = Path(".github/workflows/reviewer-bot-pr-comment-router.yml").read_text(encoding="utf-8")
+    first_def = workflow_text.index("def _classify_issue_comment_actor(")
+    start = workflow_text.rfind("\n", 0, first_def) + 1
+    end = workflow_text.index("\n\n          performed_via_github_app =", start)
+    namespace = {}
+    exec(textwrap.dedent(workflow_text[start:end]), namespace)
+    return namespace
+
+
 @pytest.mark.parametrize(
     ("value", "expected"),
     [
@@ -172,11 +183,7 @@ def test_pr_comment_router_actor_classifier_matches_core_policy(
     installation_id,
     performed_via_app,
 ):
-    workflow_text = Path(".github/workflows/reviewer-bot-pr-comment-router.yml").read_text(encoding="utf-8")
-    start = workflow_text.index("def _classify_issue_comment_actor(")
-    end = workflow_text.index("\n          def _performed_via_github_app_truth", start)
-    namespace = {}
-    exec(textwrap.dedent(workflow_text[start:end]), namespace)
+    namespace = _load_pr_comment_router_inline_namespace()
 
     request = SimpleNamespace(
         comment_user_type=comment_user_type,
@@ -195,6 +202,142 @@ def test_pr_comment_router_actor_classifier_matches_core_policy(
     ) == comment_routing_policy.classify_issue_comment_actor(request)
 
 
+@pytest.mark.parametrize(
+    "scenario",
+    [
+        {
+            "name": "same_repo_trusted_member",
+            "comment_user_type": "User",
+            "comment_author": "alice",
+            "comment_sender_type": "User",
+            "installation_id": None,
+            "performed_via_app": False,
+            "comment_author_association": "MEMBER",
+            "pr_head_full_name": "rustfoundation/safety-critical-rust-coding-guidelines",
+            "pr_author": "carol",
+            "route_outcome": comment_routing_policy.PrCommentRouterOutcome.TRUSTED_DIRECT,
+        },
+        {
+            "name": "same_repo_untrusted_author_association",
+            "comment_user_type": "User",
+            "comment_author": "alice",
+            "comment_sender_type": "User",
+            "installation_id": None,
+            "performed_via_app": False,
+            "comment_author_association": "contributor",
+            "pr_head_full_name": "rustfoundation/safety-critical-rust-coding-guidelines",
+            "pr_author": "carol",
+            "route_outcome": comment_routing_policy.PrCommentRouterOutcome.TRUSTED_DIRECT,
+        },
+        {
+            "name": "cross_repo_deferred",
+            "comment_user_type": "User",
+            "comment_author": "alice",
+            "comment_sender_type": "User",
+            "installation_id": None,
+            "performed_via_app": False,
+            "comment_author_association": "MEMBER",
+            "pr_head_full_name": "fork/example",
+            "pr_author": "carol",
+            "route_outcome": comment_routing_policy.PrCommentRouterOutcome.TRUSTED_DIRECT,
+        },
+        {
+            "name": "dependabot_pr_author_deferred",
+            "comment_user_type": "User",
+            "comment_author": "alice",
+            "comment_sender_type": "User",
+            "installation_id": None,
+            "performed_via_app": False,
+            "comment_author_association": "OWNER",
+            "pr_head_full_name": "rustfoundation/safety-critical-rust-coding-guidelines",
+            "pr_author": "dependabot[bot]",
+            "route_outcome": comment_routing_policy.PrCommentRouterOutcome.TRUSTED_DIRECT,
+        },
+        {
+            "name": "automation_actor_noop",
+            "comment_user_type": "User",
+            "comment_author": "alice",
+            "comment_sender_type": "Organization",
+            "installation_id": None,
+            "performed_via_app": False,
+            "comment_author_association": "MEMBER",
+            "pr_head_full_name": "rustfoundation/safety-critical-rust-coding-guidelines",
+            "pr_author": "carol",
+            "route_outcome": comment_routing_policy.PrCommentRouterOutcome.TRUSTED_DIRECT,
+        },
+        {
+            "name": "pull_request_read_failed",
+            "comment_user_type": "User",
+            "comment_author": "alice",
+            "comment_sender_type": "User",
+            "installation_id": None,
+            "performed_via_app": False,
+            "comment_author_association": "MEMBER",
+            "pr_head_full_name": "",
+            "pr_author": "",
+            "pull_request_read_failed": True,
+            "route_outcome": comment_routing_policy.PrCommentRouterOutcome.DEFERRED_RECONCILE,
+        },
+    ],
+    ids=lambda scenario: scenario["name"],
+)
+def test_pr_comment_router_route_outcome_matches_core_policy(scenario):
+    repo = "rustfoundation/safety-critical-rust-coding-guidelines"
+    namespace = _load_pr_comment_router_inline_namespace()
+    request = SimpleNamespace(
+        is_pull_request=True,
+        comment_user_type=scenario["comment_user_type"],
+        comment_author=scenario["comment_author"],
+        comment_sender_type=scenario["comment_sender_type"],
+        comment_installation_id=scenario["installation_id"],
+        comment_performed_via_github_app=scenario["performed_via_app"],
+        comment_author_association=scenario["comment_author_association"],
+    )
+    pr_admission = PrCommentAdmission(
+        route_outcome=scenario["route_outcome"],
+        declared_trust_class="pr_trusted_direct",
+        github_repository=repo,
+        pr_head_full_name=scenario["pr_head_full_name"],
+        pr_author=scenario["pr_author"],
+        issue_state="open",
+        issue_labels=(),
+        comment_author_id=123,
+        github_run_id=1,
+        github_run_attempt=1,
+    )
+    actor_class = comment_routing_policy.classify_issue_comment_actor(request)
+    processing_target = comment_routing_policy.classify_pr_comment_processing_target(
+        request,
+        pr_admission,
+        actor_class=actor_class,
+        is_self_comment=False,
+    )
+    expected = comment_routing_policy.route_issue_comment_trust(
+        request,
+        pr_admission,
+        processing_target=processing_target,
+    )
+    workflow_actor_class = namespace["_classify_issue_comment_actor"](
+        scenario["comment_user_type"],
+        scenario["comment_author"],
+        scenario["comment_sender_type"],
+        scenario["installation_id"],
+        scenario["performed_via_app"],
+    )
+
+    workflow_route = namespace["_route_pr_comment_outcome"](
+        workflow_actor_class,
+        repo,
+        scenario["pr_head_full_name"],
+        scenario["pr_author"],
+        scenario["comment_author_association"],
+        pull_request_read_failed=scenario.get("pull_request_read_failed", False),
+    )
+
+    assert workflow_actor_class == actor_class
+    assert workflow_route == expected.value
+
+
 def test_pr_comment_router_workflow_builds_payload_inline_without_bot_src_root():
     workflow = Path(".github/workflows/reviewer-bot-pr-comment-router.yml").read_text(encoding="utf-8")
     assert "build_pr_comment_observer_payload" not in workflow

tests/unit/reviewer_bot/test_reconcile_artifacts.py

@@ -248,6 +248,46 @@ def test_replay_comment_request_preserves_payload_app_truth_for_non_exact_live_s
     assert request.comment_performed_via_github_app is True
 
 
+@pytest.mark.parametrize("installation", [None, {}, {"id": ""}, {"id": "bad"}, {"id": 0}])
+def test_replay_comment_request_preserves_payload_installation_for_non_exact_live_shape(installation):
+    payload = _load_fixture_payload("tests/fixtures/observer_payloads/workflow_pr_comment_deferred.json")
+    payload["comment_installation_id"] = "12345"
+    parsed_payload = reconcile_payloads.parse_deferred_context_payload(payload)
+    live_context = reconcile_reads.read_live_comment_replay_context(
+        {
+            "user": {"login": "alice", "type": "User"},
+            "installation": installation,
+        },
+        {"actor_login": "alice"},
+    )
+
+    request = event_inputs.build_replay_comment_event_request(parsed_payload, live_comment=live_context)
+
+    assert live_context.comment_installation_id is None
+    assert live_context.comment_installation_id_available is False
+    assert request.comment_installation_id == "12345"
+
+
+@pytest.mark.parametrize("performed_via_app", [{}, {"id": ""}, {"id": "bad"}])
+def test_replay_comment_request_preserves_payload_app_truth_for_malformed_live_app_dict(performed_via_app):
+    payload = _load_fixture_payload("tests/fixtures/observer_payloads/workflow_pr_comment_deferred.json")
+    payload["comment_performed_via_github_app"] = True
+    parsed_payload = reconcile_payloads.parse_deferred_context_payload(payload)
+    live_context = reconcile_reads.read_live_comment_replay_context(
+        {
+            "user": {"login": "alice", "type": "User"},
+            "performed_via_github_app": performed_via_app,
+        },
+        {"actor_login": "alice"},
+    )
+
+    request = event_inputs.build_replay_comment_event_request(parsed_payload, live_comment=live_context)
+
+    assert live_context.comment_performed_via_github_app is None
+    assert live_context.comment_performed_via_github_app_available is False
+    assert request.comment_performed_via_github_app is True
+
+
 def test_replay_comment_request_preserves_payload_provenance_without_exact_live_fields():
     payload = _load_fixture_payload("tests/fixtures/observer_payloads/workflow_pr_comment_deferred.json")
     payload.update(