diff --git a/.github/reviewer-bot-tests/conftest.py b/.github/reviewer-bot-tests/conftest.py index 07fbe091b..fdd82486b 100644 --- a/.github/reviewer-bot-tests/conftest.py +++ b/.github/reviewer-bot-tests/conftest.py @@ -9,6 +9,7 @@ def clear_env(): "COMMENT_BODY", "COMMENT_AUTHOR", "COMMENT_ID", + "COMMENT_SOURCE_EVENT_KEY", "ALLOW_EMPTY_ACTIVE_REVIEWS_WRITE", "EVENT_ACTION", "EVENT_NAME", diff --git a/.github/reviewer-bot-tests/test_main.py b/.github/reviewer-bot-tests/test_main.py index 3e697437c..d42be02e9 100644 --- a/.github/reviewer-bot-tests/test_main.py +++ b/.github/reviewer-bot-tests/test_main.py @@ -78,6 +78,11 @@ def test_classify_event_intent_same_repo_dismissed_review_is_non_mutating_defer( assert intent == reviewer_bot.EVENT_INTENT_NON_MUTATING_DEFER +def test_classify_event_intent_review_comment_is_non_mutating_defer(monkeypatch): + intent = reviewer_bot.classify_event_intent("pull_request_review_comment", "created") + assert intent == reviewer_bot.EVENT_INTENT_NON_MUTATING_DEFER + + def test_classify_event_intent_workflow_run_dismissed_review_is_mutating(monkeypatch): monkeypatch.setenv("WORKFLOW_RUN_EVENT", "pull_request_review") monkeypatch.setenv("WORKFLOW_RUN_EVENT_ACTION", "dismissed") @@ -157,6 +162,37 @@ def fake_acquire(): assert acquire_called["value"] is True +def test_main_workflow_run_review_comment_reconcile_acquires_lock(monkeypatch): + monkeypatch.setenv("EVENT_NAME", "workflow_run") + monkeypatch.setenv("EVENT_ACTION", "completed") + monkeypatch.setenv("WORKFLOW_RUN_EVENT", "pull_request_review_comment") + + acquire_called = {"value": False} + + def fake_acquire(): + acquire_called["value"] = True + return reviewer_bot.LeaseContext( + lock_token="token", + lock_owner_run_id="run", + lock_owner_workflow="workflow", + lock_owner_job="job", + state_issue_url="https://example.com/issues/314", + lock_ref="refs/heads/reviewer-bot-state-lock", + lock_expires_at="2999-01-01T00:00:00+00:00", + ) + + monkeypatch.setattr(reviewer_bot, "acquire_state_issue_lease_lock", fake_acquire) + monkeypatch.setattr(reviewer_bot, "release_state_issue_lease_lock", lambda: True) + monkeypatch.setattr(reviewer_bot, "load_state", lambda *args, **kwargs: make_state()) + monkeypatch.setattr(reviewer_bot, "process_pass_until_expirations", lambda state: (state, [])) + monkeypatch.setattr(reviewer_bot, "sync_members_with_queue", lambda state: (state, [])) + monkeypatch.setattr(reviewer_bot, "handle_workflow_run_event", lambda state: False) + + reviewer_bot.main() + + assert acquire_called["value"] is True + + def test_main_reloads_state_before_syncing_status_labels(monkeypatch): monkeypatch.setenv("EVENT_NAME", "issue_comment") monkeypatch.setenv("EVENT_ACTION", "created") diff --git a/.github/reviewer-bot-tests/test_reviewer_bot.py b/.github/reviewer-bot-tests/test_reviewer_bot.py index bac0e5f0c..a69065d18 100644 --- a/.github/reviewer-bot-tests/test_reviewer_bot.py +++ b/.github/reviewer-bot-tests/test_reviewer_bot.py @@ -35,6 +35,7 @@ def clean_env(monkeypatch): "COMMENT_BODY", "COMMENT_AUTHOR", "COMMENT_ID", + "COMMENT_SOURCE_EVENT_KEY", "COMMENT_CREATED_AT", "COMMENT_USER_TYPE", "COMMENT_AUTHOR_ASSOCIATION", @@ -740,6 +741,95 @@ def test_project_status_labels_uses_live_review_fallback_for_stale_head(monkeypa assert metadata["reason"] == "review_head_stale" +def test_project_status_labels_prefers_current_head_review_over_newer_stale_review(monkeypatch): + state = make_state() + review = reviewer_bot.ensure_review_entry(state, 42, create=True) + assert review is not None + review["current_reviewer"] = "alice" + review["active_cycle_started_at"] = "2026-03-17T09:00:00Z" + monkeypatch.setattr( + reviewer_bot, + "get_issue_or_pr_snapshot", + lambda issue_number: {"number": issue_number, "state": "open", "pull_request": {}, "labels": []}, + ) + monkeypatch.setattr( + reviewer_bot, + "github_api", + lambda method, endpoint, data=None: {"head": {"sha": "head-1"}} if endpoint == "pulls/42" else None, + ) + monkeypatch.setattr( + reviewer_bot, + "get_pull_request_reviews", + lambda issue_number: [ + { + "id": 10, + "state": "COMMENTED", + "submitted_at": "2026-03-17T10:01:00Z", + "commit_id": "head-1", + "user": {"login": "alice"}, + }, + { + "id": 11, + "state": "COMMENTED", + "submitted_at": "2026-03-17T11:01:00Z", + "commit_id": "head-0", + "user": {"login": "alice"}, + }, + ], + ) + desired_labels, metadata = reviewer_bot.project_status_labels_for_item(42, state) + assert desired_labels == {reviewer_bot.STATUS_AWAITING_CONTRIBUTOR_RESPONSE_LABEL} + assert metadata["reason"] == "completion_missing" + + +def test_project_status_labels_pr256_shape_remains_awaiting_contributor_response(monkeypatch): + state = make_state() + review = reviewer_bot.ensure_review_entry(state, 42, create=True) + assert review is not None + review["current_reviewer"] = "vccjgust" + review["active_cycle_started_at"] = "2026-02-18T09:00:00Z" + reviewer_bot.reviews_module.accept_channel_event( + review, + "contributor_comment", + semantic_key="issue_comment:20", + timestamp="2026-02-18T09:30:00Z", + actor="dana", + ) + monkeypatch.setattr( + reviewer_bot, + "get_issue_or_pr_snapshot", + lambda issue_number: {"number": issue_number, "state": "open", "pull_request": {}, "labels": []}, + ) + monkeypatch.setattr( + reviewer_bot, + "github_api", + lambda method, endpoint, data=None: {"head": {"sha": "head-current"}} if endpoint == "pulls/42" else None, + ) + monkeypatch.setattr( + reviewer_bot, + "get_pull_request_reviews", + lambda issue_number: [ + { + "id": 30, + "state": "COMMENTED", + "submitted_at": "2026-02-18T10:00:00Z", + "commit_id": "head-older", + "user": {"login": "vccjgust"}, + }, + { + "id": 31, + "state": "COMMENTED", + "submitted_at": "2026-02-18T11:00:00Z", + "commit_id": "head-current", + "user": {"login": "vccjgust"}, + }, + ], + ) + desired_labels, metadata = reviewer_bot.project_status_labels_for_item(42, state) + assert desired_labels == {reviewer_bot.STATUS_AWAITING_CONTRIBUTOR_RESPONSE_LABEL} + assert metadata["reason"] == "completion_missing" + + def test_project_status_labels_prefers_newer_contributor_comment_over_live_review_fallback(monkeypatch): state = make_state() review = reviewer_bot.ensure_review_entry(state, 42, create=True) @@ -913,6 +1003,136 @@ def test_handle_workflow_run_event_rebuilds_completion_from_live_review_commit_i assert state["active_reviews"]["42"]["current_cycle_completion"]["completed"] is False +def test_repair_missing_reviewer_review_state_refreshes_to_preferred_current_head_review(monkeypatch): + state = make_state() + review = reviewer_bot.ensure_review_entry(state, 42, create=True) + assert review is not None + review["current_reviewer"] = "alice" + review["active_cycle_started_at"] = "2026-03-17T09:00:00Z" + reviewer_bot.reviews_module.accept_channel_event( + review, + "reviewer_review", + semantic_key="pull_request_review:99", + timestamp="2026-03-17T11:00:00Z", + actor="alice", + reviewed_head_sha="head-0", + source_precedence=1, + ) + monkeypatch.setattr( + reviewer_bot, + "github_api", + lambda method, endpoint, data=None: {"head": {"sha": "head-1"}} if endpoint == "pulls/42" else None, + ) + monkeypatch.setattr( + reviewer_bot, + "get_pull_request_reviews", + lambda issue_number: [ + { + "id": 10, + "state": "COMMENTED", + "submitted_at": "2026-03-17T10:00:00Z", + "commit_id": "head-1", + "user": {"login": "alice"}, + }, + { + "id": 99, + "state": "COMMENTED", + "submitted_at": "2026-03-17T11:00:00Z", + "commit_id": "head-0", + "user": {"login": "alice"}, + }, + ], + ) + assert reviewer_bot.reviews_module.repair_missing_reviewer_review_state(reviewer_bot, 42, review) is True + accepted = review["reviewer_review"]["accepted"] + assert accepted["semantic_key"] == "pull_request_review:10" + assert accepted["reviewed_head_sha"] == "head-1" + assert "pull_request_review:99" in review["reviewer_review"]["seen_keys"] + + +def test_handle_workflow_run_event_refreshes_stale_stored_reviewer_review_to_current_head_preferred_review(tmp_path, monkeypatch): + state = make_state() + review = reviewer_bot.ensure_review_entry(state, 42, create=True) + assert review is not None + review["current_reviewer"] = "alice" + review["active_cycle_started_at"] = "2026-03-17T09:00:00Z" + reviewer_bot.reviews_module.accept_channel_event( + review, + "reviewer_review", + semantic_key="pull_request_review:99", + timestamp="2026-03-17T11:00:00Z", + actor="alice", + reviewed_head_sha="head-0", + source_precedence=1, + ) + payload_path = tmp_path / "deferred-review.json" + payload_path.write_text( + json.dumps( + { + "schema_version": 2, + "source_workflow_name": "Reviewer Bot PR Review Submitted Observer", + "source_workflow_file": ".github/workflows/reviewer-bot-pr-review-submitted-observer.yml", + "source_run_id": 500, + "source_run_attempt": 2, + "source_event_name": "pull_request_review", + "source_event_action": "submitted", + "source_event_key": "pull_request_review:99", + "pr_number": 42, + "review_id": 99, + "source_submitted_at": "2026-03-17T11:00:00Z", + "source_review_state": "COMMENTED", + "source_commit_id": "head-0", + "actor_login": "alice", + } + ), + encoding="utf-8", + ) + monkeypatch.setenv("DEFERRED_CONTEXT_PATH", str(payload_path)) + monkeypatch.setenv("WORKFLOW_RUN_TRIGGERING_NAME", "Reviewer Bot PR Review Submitted Observer") + monkeypatch.setenv("WORKFLOW_RUN_TRIGGERING_ID", "500") + monkeypatch.setenv("WORKFLOW_RUN_TRIGGERING_ATTEMPT", "2") + monkeypatch.setenv("WORKFLOW_RUN_TRIGGERING_CONCLUSION", "success") + + def fake_github_api(method, endpoint, data=None): + if endpoint == "pulls/42": + return {"head": {"sha": "head-1"}, "user": {"login": "dana"}, "labels": []} + if endpoint == "pulls/42/reviews/99": + return { + "id": 99, + "submitted_at": "2026-03-17T11:00:00Z", + "state": "COMMENTED", + "commit_id": "head-0", + "user": {"login": "alice"}, + } + raise AssertionError(f"Unexpected endpoint: {endpoint}") + + monkeypatch.setattr(reviewer_bot, "github_api", fake_github_api) + monkeypatch.setattr( + reviewer_bot, + "get_pull_request_reviews", + lambda issue_number: [ + { + "id": 10, + "submitted_at": "2026-03-17T10:00:00Z", + "state": "COMMENTED", + "commit_id": "head-1", + "user": {"login": "alice"}, + }, + { + "id": 99, + "submitted_at": "2026-03-17T11:00:00Z", + "state": "COMMENTED", + "commit_id": "head-0", + "user": {"login": "alice"}, + }, + ], + ) + assert reviewer_bot.handle_workflow_run_event(state) is True + accepted = review["reviewer_review"]["accepted"] + assert accepted["semantic_key"] == "pull_request_review:10" + assert accepted["reviewed_head_sha"] == "head-1" + + def test_workflow_run_review_submission_clears_warning_and_transition_notice_markers(tmp_path, monkeypatch): state = make_state() review = reviewer_bot.ensure_review_entry(state, 42, create=True) @@ -1025,6 +1245,210 @@ def test_deferred_comment_missing_live_object_preserves_source_time_freshness(tm assert state["active_reviews"]["42"]["deferred_gaps"]["issue_comment:99"]["reason"] == "reconcile_failed_closed" +def test_deferred_review_comment_reconcile_records_contributor_freshness(tmp_path, monkeypatch): + state = make_state() + review = reviewer_bot.ensure_review_entry(state, 42, create=True) + assert review is not None + review["current_reviewer"] = "alice" + live_body = "author reply in review thread" + payload_path = tmp_path / "deferred-review-comment.json" + payload_path.write_text( + json.dumps( + { + "schema_version": 2, + "source_workflow_name": "Reviewer Bot PR Review Comment Observer", + "source_workflow_file": ".github/workflows/reviewer-bot-pr-review-comment-observer.yml", + "source_run_id": 701, + "source_run_attempt": 1, + "source_event_name": "pull_request_review_comment", + "source_event_action": "created", + "source_event_key": "pull_request_review_comment:301", + "pr_number": 42, + "comment_id": 301, + "comment_class": "plain_text", + "has_non_command_text": True, + "source_body_digest": comment_routing._digest_body(live_body), + "source_created_at": "2026-03-17T10:00:00Z", + "actor_login": "dana", + "actor_id": 5, + "actor_class": "repo_user_principal", + "pull_request_review_id": 10, + "in_reply_to_id": 200, + "source_artifact_name": "reviewer-bot-review-comment-context-701-attempt-1", + } + ), + encoding="utf-8", + ) + monkeypatch.setenv("DEFERRED_CONTEXT_PATH", str(payload_path)) + monkeypatch.setenv("WORKFLOW_RUN_TRIGGERING_NAME", "Reviewer Bot PR Review Comment Observer") + monkeypatch.setenv("WORKFLOW_RUN_TRIGGERING_ID", "701") + monkeypatch.setenv("WORKFLOW_RUN_TRIGGERING_ATTEMPT", "1") + monkeypatch.setenv("WORKFLOW_RUN_TRIGGERING_CONCLUSION", "success") + + def fake_github_api(method, endpoint, data=None): + if endpoint == "pulls/42": + return {"user": {"login": "dana"}, "labels": []} + if endpoint == "pulls/comments/301": + return { + "body": live_body, + "user": {"login": "dana", "type": "User"}, + "author_association": "CONTRIBUTOR", + "performed_via_github_app": None, + } + raise AssertionError(f"Unexpected endpoint: {endpoint}") + + monkeypatch.setattr(reviewer_bot, "github_api", fake_github_api) + assert reviewer_bot.handle_workflow_run_event(state) is True + assert review["contributor_comment"]["accepted"]["semantic_key"] == "pull_request_review_comment:301" + + +def test_deferred_review_comment_reconcile_records_reviewer_freshness(tmp_path, monkeypatch): + state = make_state() + review = reviewer_bot.ensure_review_entry(state, 42, create=True) + assert review is not None + review["current_reviewer"] = "alice" + live_body = "reviewer reply in thread" + payload_path = tmp_path / "deferred-review-comment.json" + payload_path.write_text( + json.dumps( + { + "schema_version": 2, + "source_workflow_name": "Reviewer Bot PR Review Comment Observer", + "source_workflow_file": ".github/workflows/reviewer-bot-pr-review-comment-observer.yml", + "source_run_id": 702, + "source_run_attempt": 1, + "source_event_name": "pull_request_review_comment", + "source_event_action": "created", + "source_event_key": "pull_request_review_comment:302", + "pr_number": 42, + "comment_id": 302, + "comment_class": "plain_text", + "has_non_command_text": True, + "source_body_digest": comment_routing._digest_body(live_body), + "source_created_at": "2026-03-17T11:00:00Z", + "actor_login": "alice", + "actor_id": 6, + "actor_class": "repo_user_principal", + "pull_request_review_id": 10, + "in_reply_to_id": 200, + "source_artifact_name": "reviewer-bot-review-comment-context-702-attempt-1", + } + ), + encoding="utf-8", + ) + monkeypatch.setenv("DEFERRED_CONTEXT_PATH", str(payload_path)) + monkeypatch.setenv("WORKFLOW_RUN_TRIGGERING_NAME", "Reviewer Bot PR Review Comment Observer") + monkeypatch.setenv("WORKFLOW_RUN_TRIGGERING_ID", "702") + monkeypatch.setenv("WORKFLOW_RUN_TRIGGERING_ATTEMPT", "1") + monkeypatch.setenv("WORKFLOW_RUN_TRIGGERING_CONCLUSION", "success") + + def fake_github_api(method, endpoint, data=None): + if endpoint == "pulls/42": + return {"user": {"login": "dana"}, "labels": []} + if endpoint == "pulls/comments/302": + return { + "body": live_body, + "user": {"login": "alice", "type": "User"}, + "author_association": "MEMBER", + "performed_via_github_app": None, + } + raise AssertionError(f"Unexpected endpoint: {endpoint}") + + monkeypatch.setattr(reviewer_bot, "github_api", fake_github_api) + assert reviewer_bot.handle_workflow_run_event(state) is True + assert review["reviewer_comment"]["accepted"]["semantic_key"] == "pull_request_review_comment:302" + + +def test_deferred_review_comment_missing_live_object_preserves_source_time_freshness(tmp_path, monkeypatch): + state = make_state() + review = reviewer_bot.ensure_review_entry(state, 42, create=True) + assert review is not None + review["current_reviewer"] = "alice" + payload_path = tmp_path / "deferred-review-comment.json" + payload_path.write_text( + json.dumps( + { + "schema_version": 2, + "source_workflow_name": "Reviewer Bot PR Review Comment Observer", + "source_workflow_file": ".github/workflows/reviewer-bot-pr-review-comment-observer.yml", + "source_run_id": 703, + "source_run_attempt": 1, + "source_event_name": "pull_request_review_comment", + "source_event_action": "created", + "source_event_key": "pull_request_review_comment:303", + "pr_number": 42, + "comment_id": 303, + "comment_class": "plain_text", + "has_non_command_text": True, + "source_body_digest": "abc", + "source_created_at": "2026-03-17T10:00:00Z", + "actor_login": "alice", + "actor_id": 6, + "actor_class": "repo_user_principal", + "pull_request_review_id": 10, + "in_reply_to_id": 200, + "source_artifact_name": "reviewer-bot-review-comment-context-703-attempt-1", + } + ), + encoding="utf-8", + ) + monkeypatch.setenv("DEFERRED_CONTEXT_PATH", str(payload_path)) + monkeypatch.setenv("WORKFLOW_RUN_TRIGGERING_NAME", "Reviewer Bot PR Review Comment Observer") + monkeypatch.setenv("WORKFLOW_RUN_TRIGGERING_ID", "703") + monkeypatch.setenv("WORKFLOW_RUN_TRIGGERING_ATTEMPT", "1") + monkeypatch.setenv("WORKFLOW_RUN_TRIGGERING_CONCLUSION", "success") + monkeypatch.setattr( + reviewer_bot, + "github_api", + lambda method, endpoint, data=None: ({"user": {"login": "dana"}, "labels": []} if endpoint == "pulls/42" else None), + ) + assert reviewer_bot.handle_workflow_run_event(state) is True + assert review["reviewer_comment"]["accepted"]["semantic_key"] == "pull_request_review_comment:303" + assert review["deferred_gaps"]["pull_request_review_comment:303"]["reason"] == "reconcile_failed_closed" + + +def test_review_comment_artifact_identity_validation(tmp_path, monkeypatch): + state = make_state() + review = reviewer_bot.ensure_review_entry(state, 42, create=True) + assert review is not None + review["current_reviewer"] = "alice" + payload_path = tmp_path / "deferred-review-comment.json" + payload_path.write_text( + json.dumps( + { + "schema_version": 2, + "source_workflow_name": "Reviewer Bot PR Review Comment Observer", + "source_workflow_file": ".github/workflows/reviewer-bot-pr-review-comment-observer.yml", + "source_run_id": 704, + "source_run_attempt": 1, + "source_event_name": "pull_request_review_comment", + "source_event_action": "created", + "source_event_key": "pull_request_review_comment:304", + "pr_number": 42, + "comment_id": 304, + "comment_class": "plain_text", + "has_non_command_text": True, + "source_body_digest": "abc", + "source_created_at": "2026-03-17T10:00:00Z", + "actor_login": "alice", + "actor_id": 6, + "actor_class": "repo_user_principal", + "pull_request_review_id": 10, + "in_reply_to_id": 200, + "source_artifact_name": "reviewer-bot-review-comment-context-704-attempt-1", + } + ), + encoding="utf-8", + ) + monkeypatch.setenv("DEFERRED_CONTEXT_PATH", str(payload_path)) + monkeypatch.setenv("WORKFLOW_RUN_TRIGGERING_NAME", "Reviewer Bot PR Review Comment Observer") + monkeypatch.setenv("WORKFLOW_RUN_TRIGGERING_ID", "704") + monkeypatch.setenv("WORKFLOW_RUN_TRIGGERING_ATTEMPT", "1") + monkeypatch.setenv("WORKFLOW_RUN_TRIGGERING_CONCLUSION", "success") + monkeypatch.setattr(reviewer_bot, "github_api", lambda method, endpoint, data=None: {"user": {"login": "dana"}, "labels": []} if endpoint == "pulls/42" else None) + assert reviewer_bot.handle_workflow_run_event(state) is True + + def test_deferred_comment_reconcile_hydrates_pr_author_context_for_contributor_freshness(tmp_path, monkeypatch): state = make_state() review = reviewer_bot.ensure_review_entry(state, 42, create=True) @@ -1797,6 +2221,31 @@ def test_sweeper_creates_keyed_deferred_gaps_for_visible_comments_reviews_and_di assert gaps["pull_request_review_dismissed:303"]["source_workflow_file"] == ".github/workflows/reviewer-bot-pr-review-dismissed-observer.yml" +def test_sweeper_creates_keyed_deferred_gap_for_visible_review_comments(monkeypatch): + state = make_state() + review = reviewer_bot.ensure_review_entry(state, 42, create=True) + assert review is not None + review["current_reviewer"] = "alice" + + def fake_github_api(method, endpoint, data=None): + if endpoint == "pulls/42": + return {"state": "open", "head": {"sha": "head-1"}} + if endpoint == "issues/42/comments?per_page=100&page=1": + return [] + if endpoint == "pulls/42/comments?per_page=100": + return [{"id": 404, "created_at": "2026-03-25T10:30:00Z", "user": {"login": "dana", "type": "User"}}] + if endpoint.startswith("actions/workflows/"): + return {"workflow_runs": []} + return None + + monkeypatch.setattr(reviewer_bot, "github_api", fake_github_api) + monkeypatch.setattr(reviewer_bot, "get_pull_request_reviews", lambda issue_number: []) + assert sweeper.sweep_deferred_gaps(reviewer_bot, state) is True + gaps = state["active_reviews"]["42"]["deferred_gaps"] + assert "pull_request_review_comment:404" in gaps + assert gaps["pull_request_review_comment:404"]["source_workflow_file"] == ".github/workflows/reviewer-bot-pr-review-comment-observer.yml" + + def test_sweeper_skips_dismissed_reviews_already_reconciled_by_source_event_key(monkeypatch): state = make_state() review = reviewer_bot.ensure_review_entry(state, 42, create=True) @@ -1899,6 +2348,7 @@ def test_workflow_policy_split_and_lock_only_boundaries(): "reviewer-bot-pr-comment-observer.yml", "reviewer-bot-pr-review-submitted-observer.yml", "reviewer-bot-pr-review-dismissed-observer.yml", + "reviewer-bot-pr-review-comment-observer.yml", "reviewer-bot-reconcile.yml", "reviewer-bot-privileged-commands.yml", } @@ -1962,6 +2412,20 @@ def test_pr_comment_observer_workflow_uses_inline_payload_builder(): assert 'uv run --project "$BOT_SRC_ROOT"' not in workflow_text +def test_review_comment_observer_workflow_exists_and_is_read_only(): + data = yaml.safe_load(Path(".github/workflows/reviewer-bot-pr-review-comment-observer.yml").read_text(encoding="utf-8")) + on_block = data.get("on", data.get(True)) + assert on_block["pull_request_review_comment"]["types"] == ["created"] + job = data["jobs"]["observer"] + assert job["permissions"]["contents"] == "read" + steps = job["steps"] + assert steps[0]["name"] == "Build deferred review comment artifact" + assert steps[1]["name"] == "Upload deferred review comment artifact" + workflow_text = Path(".github/workflows/reviewer-bot-pr-review-comment-observer.yml").read_text(encoding="utf-8") + assert "checkout" not in workflow_text + assert "pull_request_review_comment" in workflow_text + + def test_build_pr_comment_observer_payload_marks_trusted_direct_same_repo_as_observer_noop(monkeypatch): monkeypatch.setenv("GITHUB_REPOSITORY", "rustfoundation/safety-critical-rust-coding-guidelines") monkeypatch.setenv("COMMENT_USER_TYPE", "User") @@ -2014,6 +2478,8 @@ def test_mutating_reviewer_bot_workflows_do_not_share_global_github_concurrency( def test_classify_event_intent_treats_supported_workflow_run_sources_as_mutating(monkeypatch): monkeypatch.setenv("WORKFLOW_RUN_EVENT", "issue_comment") assert reviewer_bot.classify_event_intent("workflow_run", "completed") == reviewer_bot.EVENT_INTENT_MUTATING + monkeypatch.setenv("WORKFLOW_RUN_EVENT", "pull_request_review_comment") + assert reviewer_bot.classify_event_intent("workflow_run", "completed") == reviewer_bot.EVENT_INTENT_MUTATING def test_main_records_repair_needed_when_projection_fails(monkeypatch, tmp_path): diff --git a/.github/workflows/reviewer-bot-pr-review-comment-observer.yml b/.github/workflows/reviewer-bot-pr-review-comment-observer.yml new file mode 100644 index 000000000..5d547fc18 --- /dev/null +++ b/.github/workflows/reviewer-bot-pr-review-comment-observer.yml @@ -0,0 +1,92 @@ +name: Reviewer Bot PR Review Comment Observer + +on: + pull_request_review_comment: + types: [created] + +permissions: + contents: read + +jobs: + observer: + runs-on: ubuntu-latest + permissions: + contents: read + steps: + - name: Build deferred review comment artifact + env: + PAYLOAD_PATH: ${{ runner.temp }}/deferred-review-comment.json + COMMENT_BODY: ${{ github.event.comment.body }} + PR_NUMBER: ${{ github.event.pull_request.number }} + COMMENT_ID: ${{ github.event.comment.id }} + COMMENT_CREATED_AT: ${{ github.event.comment.created_at }} + COMMENT_AUTHOR: ${{ github.event.comment.user.login }} + COMMENT_AUTHOR_ID: ${{ github.event.comment.user.id }} + COMMENT_USER_TYPE: ${{ github.event.comment.user.type }} + COMMENT_AUTHOR_ASSOCIATION: ${{ github.event.comment.author_association }} + COMMENT_SENDER_TYPE: ${{ github.event.sender.type }} + COMMENT_INSTALLATION_ID: ${{ github.event.installation.id }} + COMMENT_PERFORMED_VIA_GITHUB_APP: ${{ github.event.comment.performed_via_github_app != null }} + REVIEW_ID: ${{ github.event.comment.pull_request_review_id }} + IN_REPLY_TO_ID: ${{ github.event.comment.in_reply_to_id }} + run: | + python - <<'PY' + import hashlib + import json + import os + + body = os.environ['COMMENT_BODY'] + normalized = "\n".join(line.rstrip() for line in body.replace("\r\n", "\n").split("\n")).strip() + has_non_command_text = any( + line.strip() and not line.strip().startswith('@guidelines-bot /') + for line in normalized.split("\n") + ) + command_lines = [line for line in normalized.split("\n") if line.strip().startswith('@guidelines-bot /')] + if len(command_lines) == 0: + comment_class = 'plain_text' + elif len(command_lines) == 1 and has_non_command_text: + comment_class = 'command_plus_text' + elif len(command_lines) == 1: + comment_class = 'command_only' + else: + comment_class = 'command_plus_text' + actor_class = 'repo_user_principal' + user_type = os.environ.get('COMMENT_USER_TYPE', '') + author = os.environ.get('COMMENT_AUTHOR', '') + if user_type == 'Bot' or author.endswith('[bot]'): + actor_class = 'bot_account' + elif os.environ.get('COMMENT_INSTALLATION_ID', '').strip(): + actor_class = 'github_app_or_other_automation' + elif not user_type: + actor_class = 'unknown_actor' + payload = { + 'schema_version': 2, + 'source_workflow_name': 'Reviewer Bot PR Review Comment Observer', + 'source_workflow_file': '.github/workflows/reviewer-bot-pr-review-comment-observer.yml', + 'source_run_id': int(os.environ['GITHUB_RUN_ID']), + 'source_run_attempt': int(os.environ['GITHUB_RUN_ATTEMPT']), + 'source_event_name': 'pull_request_review_comment', + 'source_event_action': 'created', + 'source_event_key': f"pull_request_review_comment:{os.environ['COMMENT_ID']}", + 'pr_number': int(os.environ['PR_NUMBER']), + 'comment_id': int(os.environ['COMMENT_ID']), + 'comment_class': comment_class, + 'has_non_command_text': has_non_command_text, + 'source_body_digest': hashlib.sha256(normalized.encode('utf-8')).hexdigest(), + 'source_created_at': os.environ['COMMENT_CREATED_AT'], + 'actor_login': author, + 'actor_id': int(os.environ['COMMENT_AUTHOR_ID']), + 'actor_class': actor_class, + 'pull_request_review_id': int(os.environ['REVIEW_ID']) if os.environ.get('REVIEW_ID', '').strip() else None, + 'in_reply_to_id': int(os.environ['IN_REPLY_TO_ID']) if os.environ.get('IN_REPLY_TO_ID', '').strip() else None, + 'source_artifact_name': f"reviewer-bot-review-comment-context-{os.environ['GITHUB_RUN_ID']}-attempt-{os.environ['GITHUB_RUN_ATTEMPT']}", + } + with open(os.environ['PAYLOAD_PATH'], 'w', encoding='utf-8') as handle: + json.dump(payload, handle) + PY + - name: Upload deferred review comment artifact + uses: actions/upload-artifact@65462800fd760344b1a7b4382951275a0abb4808 + with: + name: reviewer-bot-review-comment-context-${{ github.run_id }}-attempt-${{ github.run_attempt }} + path: ${{ runner.temp }}/deferred-review-comment.json + retention-days: 7 diff --git a/.github/workflows/reviewer-bot-reconcile.yml b/.github/workflows/reviewer-bot-reconcile.yml index 28c62572a..5f61c60c6 100644 --- a/.github/workflows/reviewer-bot-reconcile.yml +++ b/.github/workflows/reviewer-bot-reconcile.yml @@ -6,6 +6,7 @@ on: - Reviewer Bot PR Comment Observer - Reviewer Bot PR Review Submitted Observer - Reviewer Bot PR Review Dismissed Observer + - Reviewer Bot PR Review Comment Observer types: [completed] permissions: diff --git a/scripts/reviewer_bot_lib/app.py b/scripts/reviewer_bot_lib/app.py index e40321aad..5d721b888 100644 --- a/scripts/reviewer_bot_lib/app.py +++ b/scripts/reviewer_bot_lib/app.py @@ -60,11 +60,16 @@ def classify_event_intent(bot: ReviewerBotContext, event_name: str, event_action return bot.EVENT_INTENT_NON_MUTATING_DEFER return bot.EVENT_INTENT_NON_MUTATING_READONLY + if event_name == "pull_request_review_comment": + if event_action == "created": + return bot.EVENT_INTENT_NON_MUTATING_DEFER + return bot.EVENT_INTENT_NON_MUTATING_READONLY + if event_name == "workflow_run": if event_action != "completed": return bot.EVENT_INTENT_NON_MUTATING_READONLY workflow_run_event = os.environ.get("WORKFLOW_RUN_EVENT", "").strip() - if workflow_run_event in {"pull_request_review", "issue_comment"}: + if workflow_run_event in {"pull_request_review", "issue_comment", "pull_request_review_comment"}: return bot.EVENT_INTENT_MUTATING return bot.EVENT_INTENT_NON_MUTATING_READONLY @@ -168,7 +173,7 @@ def main(bot: ReviewerBotContext): elif event_name == "workflow_run": if event_action == "completed": - if os.environ.get("WORKFLOW_RUN_EVENT", "").strip() in {"pull_request_review", "issue_comment"}: + if os.environ.get("WORKFLOW_RUN_EVENT", "").strip() in {"pull_request_review", "issue_comment", "pull_request_review_comment"}: state_changed = bot.handle_workflow_run_event(state) else: print( diff --git a/scripts/reviewer_bot_lib/comment_routing.py b/scripts/reviewer_bot_lib/comment_routing.py index 576f3026b..505593581 100644 --- a/scripts/reviewer_bot_lib/comment_routing.py +++ b/scripts/reviewer_bot_lib/comment_routing.py @@ -217,7 +217,7 @@ def _record_conversation_freshness(bot, state: dict, issue_number: int, comment_ if review_data is None: return False issue_author = os.environ.get("ISSUE_AUTHOR", "") - semantic_key = f"issue_comment:{comment_id}" + semantic_key = os.environ.get("COMMENT_SOURCE_EVENT_KEY", "").strip() or f"issue_comment:{comment_id}" if issue_author and issue_author.lower() == comment_author.lower(): return bot.reviews_module.accept_channel_event( review_data, @@ -282,7 +282,7 @@ def _handle_command(bot, state: dict, issue_number: int, comment_author: str, cl review_data = bot.ensure_review_entry(state, issue_number, create=True) if review_data is None: return False - source_event_key = f"issue_comment:{os.environ.get('COMMENT_ID', '')}" + source_event_key = os.environ.get("COMMENT_SOURCE_EVENT_KEY", "").strip() or f"issue_comment:{os.environ.get('COMMENT_ID', '')}" if command == "accept-no-fls-changes": is_valid, metadata = _validate_accept_no_fls_changes_handoff(bot, issue_number, comment_author) if not is_valid: diff --git a/scripts/reviewer_bot_lib/reconcile.py b/scripts/reviewer_bot_lib/reconcile.py index 4d0a7744e..b25759b95 100644 --- a/scripts/reviewer_bot_lib/reconcile.py +++ b/scripts/reviewer_bot_lib/reconcile.py @@ -12,9 +12,8 @@ classify_comment_payload, ) from .reviews import ( - accept_reviewer_review_from_live_review, find_triage_approval_after, - get_latest_valid_current_reviewer_review_for_cycle, + refresh_reviewer_review_from_live_preferred_review, ) @@ -54,14 +53,17 @@ def _record_review_rebuild(bot, state: dict, issue_number: int, review_data: dic reviews = bot.get_pull_request_reviews(issue_number) if reviews is None: raise RuntimeError(f"Failed to fetch live reviews for PR #{issue_number}") + refresh_reviewer_review_from_live_preferred_review( + bot, + issue_number, + review_data, + pull_request=pull_request, + reviews=reviews, + actor=review_data.get("current_reviewer"), + ) completion, _ = bot.reviews_module.rebuild_pr_approval_state(bot, issue_number, review_data, pull_request=pull_request, reviews=reviews) if completion is None: raise RuntimeError(f"Unable to rebuild approval state for PR #{issue_number}") - latest = get_latest_valid_current_reviewer_review_for_cycle(bot, issue_number, review_data, reviews=reviews) - if latest is not None: - submitted_at = latest.get("submitted_at") - if accept_reviewer_review_from_live_review(review_data, latest, actor=review_data.get("current_reviewer")) and isinstance(submitted_at, str): - bot.reviews_module.record_reviewer_activity(review_data, submitted_at) return bool(completion.get("completed")) @@ -87,14 +89,18 @@ def reconcile_active_review_entry( reviews = bot.get_pull_request_reviews(issue_number) if reviews is None: return f"❌ Failed to fetch reviews for PR #{issue_number}; cannot run `/rectify`.", False, False - latest_review = get_latest_valid_current_reviewer_review_for_cycle(bot, issue_number, review_data, reviews=reviews) messages: list[str] = [] + refreshed, latest_review = refresh_reviewer_review_from_live_preferred_review( + bot, + issue_number, + review_data, + reviews=reviews, + actor=assigned_reviewer, + ) if latest_review is not None: latest_state = str(latest_review.get("state", "")).upper() - submitted_at = latest_review.get("submitted_at") - if accept_reviewer_review_from_live_review(review_data, latest_review, actor=assigned_reviewer) and isinstance(submitted_at, str): + if refreshed: state_changed = True - bot.reviews_module.record_reviewer_activity(review_data, submitted_at) messages.append(f"latest review by @{assigned_reviewer} is `{latest_state}`") if _record_review_rebuild(bot, state, issue_number, review_data): state_changed = True @@ -164,6 +170,36 @@ def _validate_deferred_review_artifact(payload: dict) -> None: raise RuntimeError("Deferred review artifact review_id and pr_number must be integers") +def _validate_deferred_review_comment_artifact(payload: dict) -> None: + required = { + "schema_version", + "source_workflow_name", + "source_workflow_file", + "source_run_id", + "source_run_attempt", + "source_event_name", + "source_event_action", + "source_event_key", + "pr_number", + "comment_id", + "comment_class", + "has_non_command_text", + "source_body_digest", + "source_created_at", + } + missing = sorted(required - set(payload)) + if missing: + raise RuntimeError("Deferred review-comment artifact missing required fields: " + ", ".join(missing)) + if payload.get("schema_version") != 2: + raise RuntimeError("Deferred review-comment artifact schema_version is not accepted by V18 reconcile") + if not isinstance(payload.get("comment_id"), int) or not isinstance(payload.get("pr_number"), int): + raise RuntimeError("Deferred review-comment artifact comment_id and pr_number must be integers") + if not isinstance(payload.get("comment_class"), str) or not isinstance(payload.get("has_non_command_text"), bool): + raise RuntimeError("Deferred review-comment artifact parse fields are malformed") + if not isinstance(payload.get("source_body_digest"), str) or not isinstance(payload.get("source_created_at"), str): + raise RuntimeError("Deferred review-comment artifact source digest or timestamp is malformed") + + def _load_deferred_context() -> dict: path = os.environ.get("DEFERRED_CONTEXT_PATH", "").strip() if not path: @@ -225,6 +261,7 @@ def _hydrate_reconcile_comment_context(live_comment: dict, payload: dict) -> Non raise RuntimeError("Live deferred comment author association is unavailable") _set_env_if_present("COMMENT_AUTHOR", comment_author) _set_env_if_present("COMMENT_ID", payload.get("comment_id")) + _set_env_if_present("COMMENT_SOURCE_EVENT_KEY", payload.get("source_event_key")) _set_env_if_present("COMMENT_CREATED_AT", payload.get("source_created_at")) _set_env_if_present("COMMENT_USER_TYPE", comment_user_type) _set_env_if_present("COMMENT_AUTHOR_ASSOCIATION", author_association) @@ -278,6 +315,11 @@ def _expected_observer_identity(payload: dict) -> tuple[str, str]: "Reviewer Bot PR Review Dismissed Observer", ".github/workflows/reviewer-bot-pr-review-dismissed-observer.yml", ) + if event_name == "pull_request_review_comment" and event_action == "created": + return ( + "Reviewer Bot PR Review Comment Observer", + ".github/workflows/reviewer-bot-pr-review-comment-observer.yml", + ) raise RuntimeError("Unsupported deferred workflow identity") @@ -311,6 +353,8 @@ def _artifact_expected_name(payload: dict) -> str: return f"reviewer-bot-review-submitted-context-{run_id}-attempt-{run_attempt}" if event_name == "pull_request_review" and event_action == "dismissed": return f"reviewer-bot-review-dismissed-context-{run_id}-attempt-{run_attempt}" + if event_name == "pull_request_review_comment" and event_action == "created": + return f"reviewer-bot-review-comment-context-{run_id}-attempt-{run_attempt}" raise RuntimeError("Unsupported deferred artifact naming") @@ -323,9 +367,63 @@ def _artifact_expected_payload_name(payload: dict) -> str: return "deferred-review-submitted.json" if event_name == "pull_request_review" and event_action == "dismissed": return "deferred-review-dismissed.json" + if event_name == "pull_request_review_comment" and event_action == "created": + return "deferred-review-comment.json" raise RuntimeError("Unsupported deferred payload path") +def _reconcile_deferred_comment( + bot, + state: dict, + pr_number: int, + review_data: dict, + payload: dict, + *, + expected_event_name: str, + live_comment_endpoint: str, +) -> bool: + comment_id_value = payload.get("comment_id") + if not isinstance(comment_id_value, int): + raise RuntimeError("Deferred comment artifact comment_id must be an integer") + comment_id = comment_id_value + if str(payload.get("source_event_key", "")) != f"{expected_event_name}:{comment_id}": + raise RuntimeError("Deferred comment artifact source_event_key mismatch") + _set_env_if_present("COMMENT_SOURCE_EVENT_KEY", payload.get("source_event_key")) + _hydrate_reconcile_pr_context(bot, pr_number) + comment_author = str(payload.get("actor_login", "")) + comment_created_at = str(payload.get("source_created_at")) + classified = payload.get("comment_class") + source_freshness_eligible = classified in {"plain_text", "command_plus_text"} and bool(payload.get("has_non_command_text")) + live_comment = bot.github_api("GET", live_comment_endpoint) + if not isinstance(live_comment, dict): + changed = False + if source_freshness_eligible: + changed = _record_conversation_freshness(bot, state, pr_number, comment_author, comment_id, comment_created_at) + _update_deferred_gap(bot, review_data, payload, "reconcile_failed_closed", f"Deferred comment {comment_id} is no longer visible; source-time freshness only may be preserved. See {bot.REVIEW_FRESHNESS_RUNBOOK_PATH}.") + return changed + _hydrate_reconcile_comment_context(live_comment, payload) + live_body = live_comment.get("body") + if not isinstance(live_body, str): + raise RuntimeError("Live deferred comment body is unavailable") + if _digest_body(live_body) != payload.get("source_body_digest"): + changed = False + if source_freshness_eligible: + changed = _record_conversation_freshness(bot, state, pr_number, comment_author, comment_id, comment_created_at) + _update_deferred_gap(bot, review_data, payload, "reconcile_failed_closed", f"Deferred comment {comment_id} body digest changed; command execution suppressed. See {bot.REVIEW_FRESHNESS_RUNBOOK_PATH}.") + return changed + changed = False + if source_freshness_eligible: + changed = _record_conversation_freshness(bot, state, pr_number, comment_author, comment_id, comment_created_at) or changed + live_classified = _validate_live_comment_replay_contract(bot, review_data, payload, live_body) + if live_classified is None: + return changed + if classified in {"command_only", "command_plus_text"}: + changed = _handle_command(bot, state, pr_number, comment_author, live_classified) or changed + _mark_reconciled_source_event(review_data, str(payload.get("source_event_key", ""))) + _clear_source_event_key(review_data, str(payload.get("source_event_key", ""))) + return changed + + def _update_deferred_gap(bot, review_data: dict, payload: dict, reason: str, diagnostic_summary: str) -> None: source_event_key = str(payload.get("source_event_key", "")) if not source_event_key: @@ -433,45 +531,28 @@ def handle_workflow_run_event(bot, state: dict) -> bool: if event_name == "issue_comment": _validate_deferred_comment_artifact(payload) _validate_workflow_run_artifact_identity(payload) - _hydrate_reconcile_pr_context(bot, pr_number) - if source_event_key != f"issue_comment:{payload['comment_id']}": - raise RuntimeError("Deferred comment artifact source_event_key mismatch") - comment_author = str(payload.get("actor_login", "")) - comment_created_at = str(payload.get("source_created_at")) - comment_id_value = payload.get("comment_id") - if not isinstance(comment_id_value, int): - raise RuntimeError("Deferred comment artifact comment_id must be an integer") - comment_id = comment_id_value - classified = payload.get("comment_class") - source_freshness_eligible = classified in {"plain_text", "command_plus_text"} and bool(payload.get("has_non_command_text")) - live_comment = bot.github_api("GET", f"issues/comments/{payload['comment_id']}") - if not isinstance(live_comment, dict): - changed = False - if source_freshness_eligible: - changed = _record_conversation_freshness(bot, state, pr_number, comment_author, comment_id, comment_created_at) - _update_deferred_gap(bot, review_data, payload, "reconcile_failed_closed", f"Deferred comment {payload['comment_id']} is no longer visible; source-time freshness only may be preserved. See {bot.REVIEW_FRESHNESS_RUNBOOK_PATH}.") - return changed - _hydrate_reconcile_comment_context(live_comment, payload) - live_body = live_comment.get("body") - if not isinstance(live_body, str): - raise RuntimeError("Live deferred comment body is unavailable") - if _digest_body(live_body) != payload.get("source_body_digest"): - changed = False - if source_freshness_eligible: - changed = _record_conversation_freshness(bot, state, pr_number, comment_author, comment_id, comment_created_at) - _update_deferred_gap(bot, review_data, payload, "reconcile_failed_closed", f"Deferred comment {payload['comment_id']} body digest changed; command execution suppressed. See {bot.REVIEW_FRESHNESS_RUNBOOK_PATH}.") - return changed - changed = False - if source_freshness_eligible: - changed = _record_conversation_freshness(bot, state, pr_number, comment_author, comment_id, comment_created_at) or changed - live_classified = _validate_live_comment_replay_contract(bot, review_data, payload, live_body) - if live_classified is None: - return changed - if classified in {"command_only", "command_plus_text"}: - changed = _handle_command(bot, state, pr_number, comment_author, live_classified) or changed - _mark_reconciled_source_event(review_data, source_event_key) - _clear_source_event_key(review_data, source_event_key) - return changed + return _reconcile_deferred_comment( + bot, + state, + pr_number, + review_data, + payload, + expected_event_name="issue_comment", + live_comment_endpoint=f"issues/comments/{payload['comment_id']}", + ) + + if event_name == "pull_request_review_comment" and event_action == "created": + _validate_deferred_review_comment_artifact(payload) + _validate_workflow_run_artifact_identity(payload) + return _reconcile_deferred_comment( + bot, + state, + pr_number, + review_data, + payload, + expected_event_name="pull_request_review_comment", + live_comment_endpoint=f"pulls/comments/{payload['comment_id']}", + ) if event_name == "pull_request_review" and event_action == "submitted": _validate_deferred_review_artifact(payload) diff --git a/scripts/reviewer_bot_lib/reviews.py b/scripts/reviewer_bot_lib/reviews.py index 6df744572..661d8718e 100644 --- a/scripts/reviewer_bot_lib/reviews.py +++ b/scripts/reviewer_bot_lib/reviews.py @@ -90,6 +90,77 @@ def get_latest_valid_current_reviewer_review_for_cycle( return latest_review +def get_valid_current_reviewer_reviews_for_cycle( + bot, + issue_number: int, + review_data: dict, + *, + reviews: list[dict] | None = None, +) -> list[dict]: + current_reviewer = review_data.get("current_reviewer") + if not isinstance(current_reviewer, str) or not current_reviewer.strip(): + return [] + boundary = get_current_cycle_boundary(bot, review_data) + if boundary is None: + return [] + if reviews is None: + reviews = bot.get_pull_request_reviews(issue_number) + if reviews is None: + return [] + valid_reviews: list[dict] = [] + for review in reviews: + if not isinstance(review, dict): + continue + author = review.get("user", {}).get("login") + if not isinstance(author, str) or author.lower() != current_reviewer.lower(): + continue + state = str(review.get("state", "")).upper() + if state not in {"APPROVED", "COMMENTED", "CHANGES_REQUESTED"}: + continue + submitted_at = parse_github_timestamp(review.get("submitted_at")) + if submitted_at is None or submitted_at < boundary: + continue + commit_id = review.get("commit_id") + if not isinstance(commit_id, str) or not commit_id.strip(): + continue + valid_reviews.append(review) + return valid_reviews + + +def _review_sort_key(review: dict) -> tuple[datetime, str]: + return ( + parse_github_timestamp(review.get("submitted_at")) or datetime.min.replace(tzinfo=timezone.utc), + str(review.get("id", "")), + ) + + +def _review_matches_head(review: dict, current_head: str | None) -> bool: + commit_id = review.get("commit_id") if isinstance(review, dict) else None + return isinstance(commit_id, str) and isinstance(current_head, str) and commit_id.strip() == current_head.strip() + + +def get_preferred_current_reviewer_review_for_cycle( + bot, + issue_number: int, + review_data: dict, + *, + pull_request: dict | None = None, + reviews: list[dict] | None = None, +) -> dict | None: + valid_reviews = get_valid_current_reviewer_reviews_for_cycle(bot, issue_number, review_data, reviews=reviews) + if not valid_reviews: + return None + if len(valid_reviews) == 1: + return valid_reviews[0] + if pull_request is None: + pull_request = bot.github_api("GET", f"pulls/{issue_number}") + head = pull_request.get("head") if isinstance(pull_request, dict) else None + current_head = head.get("sha") if isinstance(head, dict) else None + current_head_reviews = [review for review in valid_reviews if _review_matches_head(review, current_head)] + candidates = current_head_reviews or valid_reviews + return max(candidates, key=_review_sort_key, default=None) + + def build_reviewer_review_record_from_live_review(review: dict, *, actor: str | None = None) -> dict | None: if not isinstance(review, dict): return None @@ -127,17 +198,49 @@ def accept_reviewer_review_from_live_review(review_data: dict, review: dict, *, ) -def repair_missing_reviewer_review_state(bot, issue_number: int, review_data: dict, *, reviews: list[dict] | None = None) -> bool: - reviewer_review = review_data.get("reviewer_review", {}).get("accepted") - if isinstance(reviewer_review, dict): - return False - live_review = get_latest_valid_current_reviewer_review_for_cycle(bot, issue_number, review_data, reviews=reviews) - if live_review is None: - return False - submitted_at = live_review.get("submitted_at") - changed = accept_reviewer_review_from_live_review(review_data, live_review, actor=review_data.get("current_reviewer")) +def refresh_reviewer_review_from_live_preferred_review( + bot, + issue_number: int, + review_data: dict, + *, + pull_request: dict | None = None, + reviews: list[dict] | None = None, + actor: str | None = None, +) -> tuple[bool, dict | None]: + preferred_review = get_preferred_current_reviewer_review_for_cycle( + bot, + issue_number, + review_data, + pull_request=pull_request, + reviews=reviews, + ) + if preferred_review is None: + return False, None + record = build_reviewer_review_record_from_live_review(preferred_review, actor=actor or review_data.get("current_reviewer")) + if record is None: + return False, None + channel = _ensure_channel_map(review_data, "reviewer_review") + changed = False + if record["semantic_key"] not in channel["seen_keys"]: + channel["seen_keys"].append(record["semantic_key"]) + changed = True + if channel.get("accepted") != record: + channel["accepted"] = record + changed = True + submitted_at = preferred_review.get("submitted_at") if isinstance(submitted_at, str): record_reviewer_activity(review_data, submitted_at) + return changed, preferred_review + + +def repair_missing_reviewer_review_state(bot, issue_number: int, review_data: dict, *, reviews: list[dict] | None = None) -> bool: + changed, _ = refresh_reviewer_review_from_live_preferred_review( + bot, + issue_number, + review_data, + reviews=reviews, + actor=review_data.get("current_reviewer"), + ) return changed @@ -669,27 +772,20 @@ def compute_reviewer_response_state( reviewer_review = review_data.get("reviewer_review", {}).get("accepted") contributor_comment = review_data.get("contributor_comment", {}).get("accepted") - if not reviewer_comment and not reviewer_review and is_pr: - live_review = get_latest_valid_current_reviewer_review_for_cycle(bot, issue_number, review_data, reviews=reviews) - if live_review is not None: - reviewer_review = build_reviewer_review_record_from_live_review(live_review, actor=current_reviewer) - - if not reviewer_comment and not reviewer_review: - return { - "state": "awaiting_reviewer_response", - "reason": "no_reviewer_activity", - "anchor_timestamp": _initial_reviewer_anchor(review_data), - "reviewer_comment": reviewer_comment, - "reviewer_review": reviewer_review, - "contributor_comment": contributor_comment, - "contributor_handoff": None, - } - - latest_reviewer_response = reviewer_comment - if _compare_records(reviewer_review, latest_reviewer_response) > 0: - latest_reviewer_response = reviewer_review - if not is_pr: + if not reviewer_comment and not reviewer_review: + return { + "state": "awaiting_reviewer_response", + "reason": "no_reviewer_activity", + "anchor_timestamp": _initial_reviewer_anchor(review_data), + "reviewer_comment": reviewer_comment, + "reviewer_review": reviewer_review, + "contributor_comment": contributor_comment, + "contributor_handoff": None, + } + latest_reviewer_response = reviewer_comment + if _compare_records(reviewer_review, latest_reviewer_response) > 0: + latest_reviewer_response = reviewer_review completion = review_data.get("current_cycle_completion") if not isinstance(completion, dict) or not completion.get("completed"): if review_data.get("review_completed_at"): @@ -701,6 +797,27 @@ def compute_reviewer_response_state( } return {"state": "done", "reason": None} + if not reviewer_comment and not reviewer_review: + preferred_live_review = get_preferred_current_reviewer_review_for_cycle( + bot, + issue_number, + review_data, + pull_request=pull_request, + reviews=reviews, + ) + if preferred_live_review is not None: + reviewer_review = build_reviewer_review_record_from_live_review(preferred_live_review, actor=current_reviewer) + else: + return { + "state": "awaiting_reviewer_response", + "reason": "no_reviewer_activity", + "anchor_timestamp": _initial_reviewer_anchor(review_data), + "reviewer_comment": reviewer_comment, + "reviewer_review": reviewer_review, + "contributor_comment": contributor_comment, + "contributor_handoff": None, + } + if pull_request is None: pull_request = bot.github_api("GET", f"pulls/{issue_number}") if not isinstance(pull_request, dict): @@ -711,6 +828,20 @@ def compute_reviewer_response_state( return {"state": "projection_failed", "reason": "pull_request_head_unavailable"} review_data["active_head_sha"] = current_head + preferred_live_review = get_preferred_current_reviewer_review_for_cycle( + bot, + issue_number, + review_data, + pull_request=pull_request, + reviews=reviews, + ) + if preferred_live_review is not None: + reviewer_review = build_reviewer_review_record_from_live_review(preferred_live_review, actor=current_reviewer) + + latest_reviewer_response = reviewer_comment + if _compare_records(reviewer_review, latest_reviewer_response) > 0: + latest_reviewer_response = reviewer_review + contributor_handoff = contributor_comment contributor_revision = _contributor_revision_handoff_record(review_data, current_head, reviewer_review if isinstance(reviewer_review, dict) else None) if _compare_records(contributor_revision, contributor_handoff) > 0: diff --git a/scripts/reviewer_bot_lib/sweeper.py b/scripts/reviewer_bot_lib/sweeper.py index d48f250c1..a04394927 100644 --- a/scripts/reviewer_bot_lib/sweeper.py +++ b/scripts/reviewer_bot_lib/sweeper.py @@ -108,7 +108,22 @@ def correlate_candidate_observer_runs( "later_recheck_complete": False, "correlated_run": None, } - expected_event = "issue_comment" if source_event_kind == "issue_comment:created" else "pull_request_review" + expected_event_map = { + "issue_comment:created": "issue_comment", + "pull_request_review:submitted": "pull_request_review", + "pull_request_review:dismissed": "pull_request_review", + "pull_request_review_comment:created": "pull_request_review_comment", + } + expected_event = expected_event_map.get(source_event_kind) + if expected_event is None: + return { + "status": "observer_state_unknown", + "reason": "unsupported_source_event_kind", + "candidate_run_ids": [], + "full_scan_complete": False, + "later_recheck_complete": False, + "correlated_run": None, + } window_start = created_at - timedelta(minutes=2) window_end = created_at + timedelta(minutes=30) candidates: list[dict] = [] @@ -520,6 +535,12 @@ def _repair_visible_review_gap(bot, review_data: dict, issue_number: int, source return False author, submitted_at, commit_id = repair changed = bot.reviews_module.accept_reviewer_review_from_live_review(review_data, review, actor=author) + changed = bot.reviews_module.refresh_reviewer_review_from_live_preferred_review( + bot, + issue_number, + review_data, + actor=author, + )[0] or changed bot.reviews_module.record_reviewer_activity(review_data, submitted_at) completion, _ = bot.reviews_module.rebuild_pr_approval_state(bot, issue_number, review_data) _mark_reconciled_source_event(review_data, source_event_key) @@ -592,6 +613,40 @@ def _discover_visible_review_events(bot, issue_number: int, review_data: dict) - return discovered, True +def _discover_visible_review_comment_events(bot, issue_number: int, review_data: dict) -> tuple[list[dict] | None, bool]: + watermark = _load_surface_watermark(review_data, "review_comments") + watermark["last_scan_started_at"] = _now_iso() + comments = bot.github_api("GET", f"pulls/{issue_number}/comments?per_page=100") + if comments is None: + return None, False + if not isinstance(comments, list): + return None, False + floor = _surface_scan_floor(bot, watermark) + discovered: list[dict] = [] + for comment in comments: + if not isinstance(comment, dict) or _is_automation_comment(comment): + continue + comment_id = comment.get("id") + created_at = comment.get("created_at") + if not isinstance(comment_id, int) or not isinstance(created_at, str): + continue + created_dt = parse_timestamp(created_at) + if created_dt is None or created_dt < floor: + continue + discovered.append( + { + "source_event_key": f"pull_request_review_comment:{comment_id}", + "source_event_name": "pull_request_review_comment", + "source_event_action": "created", + "source_created_at": created_at, + "object_id": str(comment_id), + "surface": "review_comments", + "comment": comment, + } + ) + return discovered, True + + def _discover_visible_review_dismissal_events(bot, issue_number: int, review_data: dict) -> tuple[list[dict] | None, bool]: watermark = _load_surface_watermark(review_data, "reviews_dismissed") watermark["last_scan_started_at"] = _now_iso() @@ -863,6 +918,73 @@ def sweep_deferred_gaps(bot, state: dict) -> bool: watermark["last_scan_started_at"] = watermark.get("last_scan_started_at") or _now_iso() watermark["last_scan_completed_at"] = _now_iso() watermark["bootstrap_completed_at"] = watermark.get("bootstrap_completed_at") or _now_iso() + discovered_review_comments, review_comments_complete = _discover_visible_review_comment_events(bot, issue_number, review_data) + if review_comments_complete and isinstance(discovered_review_comments, list): + for discovered in discovered_review_comments: + source_event_key = discovered["source_event_key"] + created_at = discovered["source_created_at"] + if _should_skip_discovered_key(bot, review_data, source_event_key, ("reviewer_comment", "contributor_comment")): + continue + existing_gap = review_data.get("deferred_gaps", {}).get(source_event_key, {}) + workflow_file = ".github/workflows/reviewer-bot-pr-review-comment-observer.yml" + workflow_runs = _fetch_workflow_runs_for_file(bot, workflow_file, "pull_request_review_comment") + run_correlation = correlate_candidate_observer_runs( + source_event_key, + source_event_kind="pull_request_review_comment:created", + source_event_created_at=created_at, + pr_number=issue_number, + workflow_file=workflow_file, + workflow_runs=workflow_runs, + ) + run_correlation["later_recheck_complete"] = bool(existing_gap.get("full_scan_complete")) + artifact_correlation = None + run_detail = None + if run_correlation.get("status") == "candidate_runs_found": + artifact_correlation = inspect_run_artifact_payloads( + bot, + run_correlation.get("candidate_runs", []), + source_event_key, + pr_number=issue_number, + source_event_kind="pull_request_review_comment:created", + ) + exact_run_id = artifact_correlation.get("correlated_run") if isinstance(artifact_correlation, dict) else None + if isinstance(exact_run_id, int): + run_correlation["correlated_run"] = exact_run_id + run_correlation["correlated_run_found"] = True + run_detail = _maybe_fetch_single_candidate_run_detail(bot, run_correlation, artifact_correlation) + reason, diagnostic_reason = evaluate_deferred_gap_state( + { + **existing_gap, + "source_event_created_at": created_at, + }, + run_correlation, + run_detail, + artifact_correlation, + ) + _record_gap_diagnostics( + bot, + review_data, + source_event_key, + source_event_name="pull_request_review_comment", + source_event_action="created", + issue_number=issue_number, + source_created_at=created_at, + workflow_file=workflow_file, + run_correlation=run_correlation, + run_detail=run_detail, + artifact_correlation=artifact_correlation, + reason=reason, + diagnostic_reason=diagnostic_reason, + ) + changed = True + if discovered_review_comments: + last_comment = discovered_review_comments[-1] + _update_observer_watermark(bot, review_data, "review_comments", last_comment["source_created_at"], last_comment["object_id"]) + else: + watermark = _load_surface_watermark(review_data, "review_comments") + watermark["last_scan_started_at"] = watermark.get("last_scan_started_at") or _now_iso() + watermark["last_scan_completed_at"] = _now_iso() + watermark["bootstrap_completed_at"] = watermark.get("bootstrap_completed_at") or _now_iso() discovered_dismissals, dismissals_complete = _discover_visible_review_dismissal_events(bot, issue_number, review_data) if dismissals_complete and isinstance(discovered_dismissals, list): for discovered in discovered_dismissals: