Skip to content

Commit adedca5

Browse files
committed
fix(reviewer-bot): recover freshness from visible review activity
1 parent aa0c4e5 commit adedca5

File tree

5 files changed

+413
-14
lines changed

5 files changed

+413
-14
lines changed

.github/reviewer-bot-tests/test_main.py

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -274,6 +274,41 @@ def test_main_workflow_run_fails_closed_on_invalid_context(monkeypatch):
274274
assert excinfo.value.code == 1
275275

276276

277+
def test_main_schedule_backfills_existing_transition_notice_without_duplicate_comment(monkeypatch):
278+
monkeypatch.setenv("EVENT_NAME", "schedule")
279+
monkeypatch.setenv("EVENT_ACTION", "")
280+
state = make_state()
281+
review = reviewer_bot.ensure_review_entry(state, 42, create=True)
282+
assert review is not None
283+
review["current_reviewer"] = "alice"
284+
review["assigned_at"] = "2026-03-01T00:00:00Z"
285+
review["last_reviewer_activity"] = "2026-03-01T00:00:00Z"
286+
review["transition_warning_sent"] = "2026-03-10T00:00:00Z"
287+
288+
monkeypatch.setattr(reviewer_bot, "acquire_state_issue_lease_lock", lambda: None)
289+
monkeypatch.setattr(reviewer_bot, "release_state_issue_lease_lock", lambda: True)
290+
monkeypatch.setattr(reviewer_bot, "load_state", lambda *args, **kwargs: state)
291+
monkeypatch.setattr(reviewer_bot, "process_pass_until_expirations", lambda current: (current, []))
292+
monkeypatch.setattr(reviewer_bot, "sync_members_with_queue", lambda current: (current, []))
293+
monkeypatch.setattr(reviewer_bot.maintenance_module, "sweep_deferred_gaps", lambda bot, current: False)
294+
monkeypatch.setattr(reviewer_bot.maintenance_module, "maybe_record_head_observation_repair", lambda bot, issue_number, review_data: False)
295+
monkeypatch.setattr(reviewer_bot, "get_issue_or_pr_snapshot", lambda issue_number: {"number": issue_number, "state": "open", "pull_request": {}, "labels": []})
296+
monkeypatch.setattr(reviewer_bot, "save_state", lambda current: True)
297+
monkeypatch.setattr(reviewer_bot, "sync_status_labels_for_items", lambda current, issue_numbers: True)
298+
posted = []
299+
monkeypatch.setattr(reviewer_bot, "post_comment", lambda issue_number, body: posted.append(body) or True)
300+
301+
def fake_api(method, endpoint, data=None):
302+
if endpoint == "issues/42/comments?per_page=100":
303+
return [{"id": 99, "created_at": "2026-03-25T15:22:42Z", "body": "🔔 **Transition Period Ended**\n\nExisting notice", "user": {"login": "github-actions[bot]"}}]
304+
raise AssertionError(endpoint)
305+
306+
monkeypatch.setattr(reviewer_bot, "github_api", fake_api)
307+
reviewer_bot.main()
308+
assert review["transition_notice_sent_at"] == "2026-03-25T15:22:42Z"
309+
assert posted == []
310+
311+
277312
def test_main_mutating_event_fails_closed_when_state_unavailable(monkeypatch):
278313
monkeypatch.setenv("EVENT_NAME", "issue_comment")
279314
monkeypatch.setenv("EVENT_ACTION", "created")

.github/reviewer-bot-tests/test_reviewer_bot.py

Lines changed: 253 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -319,6 +319,105 @@ def test_pr_comment_direct_path_is_epoch_gated(monkeypatch):
319319
assert reviewer_bot.handle_comment_event(state) is False
320320

321321

322+
def test_check_overdue_reviews_skips_transition_after_transition_notice_sent():
323+
state = make_state()
324+
review = reviewer_bot.ensure_review_entry(state, 42, create=True)
325+
assert review is not None
326+
review["current_reviewer"] = "alice"
327+
review["assigned_at"] = "2026-03-01T00:00:00Z"
328+
review["last_reviewer_activity"] = "2026-03-01T00:00:00Z"
329+
review["transition_warning_sent"] = "2026-03-10T00:00:00Z"
330+
review["transition_notice_sent_at"] = "2026-03-25T00:00:00Z"
331+
assert reviewer_bot.maintenance_module.check_overdue_reviews(reviewer_bot, state) == []
332+
333+
334+
def test_handle_transition_notice_records_transition_notice_sent_at_once(monkeypatch):
335+
state = make_state()
336+
review = reviewer_bot.ensure_review_entry(state, 42, create=True)
337+
assert review is not None
338+
review["current_reviewer"] = "alice"
339+
posted = []
340+
monkeypatch.setattr(reviewer_bot, "post_comment", lambda issue_number, body: posted.append((issue_number, body)) or True)
341+
assert reviewer_bot.handle_transition_notice(state, 42, "alice") is True
342+
assert review["transition_notice_sent_at"] is not None
343+
assert reviewer_bot.handle_transition_notice(state, 42, "alice") is False
344+
assert len(posted) == 1
345+
346+
347+
def test_handle_transition_notice_message_does_not_claim_reassignment(monkeypatch):
348+
state = make_state()
349+
reviewer_bot.ensure_review_entry(state, 42, create=True)
350+
posted = []
351+
monkeypatch.setattr(reviewer_bot, "post_comment", lambda issue_number, body: posted.append(body) or True)
352+
assert reviewer_bot.handle_transition_notice(state, 42, "alice") is True
353+
assert "reassigned to the next person in the queue" not in posted[0]
354+
assert "/pass" in posted[0]
355+
356+
357+
def test_reviewer_comment_clears_warning_and_transition_notice_markers(monkeypatch):
358+
state = make_state()
359+
review = reviewer_bot.ensure_review_entry(state, 42, create=True)
360+
assert review is not None
361+
review["current_reviewer"] = "alice"
362+
review["transition_warning_sent"] = "2026-03-10T00:00:00Z"
363+
review["transition_notice_sent_at"] = "2026-03-25T00:00:00Z"
364+
monkeypatch.setenv("IS_PULL_REQUEST", "true")
365+
monkeypatch.setenv("ISSUE_NUMBER", "42")
366+
monkeypatch.setenv("ISSUE_AUTHOR", "dana")
367+
monkeypatch.setenv("COMMENT_USER_TYPE", "User")
368+
monkeypatch.setenv("COMMENT_AUTHOR", "alice")
369+
monkeypatch.setenv("COMMENT_AUTHOR_ASSOCIATION", "MEMBER")
370+
monkeypatch.setenv("COMMENT_ID", "100")
371+
monkeypatch.setenv("COMMENT_CREATED_AT", "2026-03-17T10:00:00Z")
372+
monkeypatch.setenv("COMMENT_BODY", "hello")
373+
monkeypatch.setenv("CURRENT_WORKFLOW_FILE", ".github/workflows/reviewer-bot-pr-comment-trusted.yml")
374+
monkeypatch.setenv("GITHUB_REPOSITORY", "rustfoundation/safety-critical-rust-coding-guidelines")
375+
monkeypatch.setenv("GITHUB_REF", "refs/heads/main")
376+
monkeypatch.setattr(
377+
reviewer_bot,
378+
"github_api",
379+
lambda method, endpoint, data=None: {
380+
"head": {"repo": {"full_name": "rustfoundation/safety-critical-rust-coding-guidelines"}},
381+
"user": {"login": "dana"},
382+
},
383+
)
384+
assert reviewer_bot.handle_comment_event(state) is True
385+
assert review["transition_warning_sent"] is None
386+
assert review["transition_notice_sent_at"] is None
387+
388+
389+
def test_scheduled_check_backfills_transition_notice_without_reposting(monkeypatch):
390+
state = make_state()
391+
review = reviewer_bot.ensure_review_entry(state, 42, create=True)
392+
assert review is not None
393+
review["current_reviewer"] = "alice"
394+
review["assigned_at"] = "2026-03-01T00:00:00Z"
395+
review["last_reviewer_activity"] = "2026-03-01T00:00:00Z"
396+
review["transition_warning_sent"] = "2026-03-10T00:00:00Z"
397+
monkeypatch.setattr(reviewer_bot.maintenance_module, "sweep_deferred_gaps", lambda bot, state: False)
398+
monkeypatch.setattr(reviewer_bot.maintenance_module, "maybe_record_head_observation_repair", lambda bot, issue_number, review_data: False)
399+
monkeypatch.setattr(reviewer_bot, "get_issue_or_pr_snapshot", lambda issue_number: {"pull_request": {}})
400+
posted = []
401+
monkeypatch.setattr(reviewer_bot, "post_comment", lambda issue_number, body: posted.append(body) or True)
402+
403+
def fake_api(method, endpoint, data=None):
404+
if endpoint == "issues/42/comments?per_page=100":
405+
return [
406+
{
407+
"id": 99,
408+
"created_at": "2026-03-25T15:22:42Z",
409+
"body": "🔔 **Transition Period Ended**\n\nExisting notice",
410+
"user": {"login": "github-actions[bot]"},
411+
}
412+
]
413+
raise AssertionError(endpoint)
414+
415+
monkeypatch.setattr(reviewer_bot, "github_api", fake_api)
416+
assert reviewer_bot.handle_scheduled_check(state) is True
417+
assert review["transition_notice_sent_at"] == "2026-03-25T15:22:42Z"
418+
assert posted == []
419+
420+
322421
def test_issue_edit_by_author_records_contributor_freshness(monkeypatch):
323422
state = make_state()
324423
review = reviewer_bot.ensure_review_entry(state, 42, create=True)
@@ -495,6 +594,73 @@ def test_handle_workflow_run_event_rebuilds_completion_from_live_review_commit_i
495594
assert state["active_reviews"]["42"]["current_cycle_completion"]["completed"] is False
496595

497596

597+
def test_workflow_run_review_submission_clears_warning_and_transition_notice_markers(tmp_path, monkeypatch):
598+
state = make_state()
599+
review = reviewer_bot.ensure_review_entry(state, 42, create=True)
600+
assert review is not None
601+
review["current_reviewer"] = "alice"
602+
review["active_cycle_started_at"] = "2026-03-17T09:00:00Z"
603+
review["transition_warning_sent"] = "2026-03-18T00:00:00Z"
604+
review["transition_notice_sent_at"] = "2026-03-25T00:00:00Z"
605+
payload_path = tmp_path / "deferred-review.json"
606+
payload_path.write_text(
607+
json.dumps(
608+
{
609+
"schema_version": 2,
610+
"source_workflow_name": "Reviewer Bot PR Review Submitted Observer",
611+
"source_workflow_file": ".github/workflows/reviewer-bot-pr-review-submitted-observer.yml",
612+
"source_run_id": 500,
613+
"source_run_attempt": 2,
614+
"source_event_name": "pull_request_review",
615+
"source_event_action": "submitted",
616+
"source_event_key": "pull_request_review:11",
617+
"pr_number": 42,
618+
"review_id": 11,
619+
"source_submitted_at": "2026-03-17T10:00:00Z",
620+
"source_review_state": "COMMENTED",
621+
"source_commit_id": "head-1",
622+
"actor_login": "alice",
623+
}
624+
),
625+
encoding="utf-8",
626+
)
627+
monkeypatch.setenv("DEFERRED_CONTEXT_PATH", str(payload_path))
628+
monkeypatch.setenv("WORKFLOW_RUN_TRIGGERING_NAME", "Reviewer Bot PR Review Submitted Observer")
629+
monkeypatch.setenv("WORKFLOW_RUN_TRIGGERING_ID", "500")
630+
monkeypatch.setenv("WORKFLOW_RUN_TRIGGERING_ATTEMPT", "2")
631+
monkeypatch.setenv("WORKFLOW_RUN_TRIGGERING_CONCLUSION", "success")
632+
monkeypatch.setattr(
633+
reviewer_bot,
634+
"github_api",
635+
lambda method, endpoint, data=None: {
636+
"pulls/42": {"head": {"sha": "head-2"}, "user": {"login": "dana"}, "labels": []},
637+
"pulls/42/reviews/11": {
638+
"id": 11,
639+
"submitted_at": "2026-03-17T10:00:00Z",
640+
"state": "COMMENTED",
641+
"commit_id": "head-1",
642+
"user": {"login": "alice"},
643+
},
644+
}.get(endpoint),
645+
)
646+
monkeypatch.setattr(
647+
reviewer_bot,
648+
"get_pull_request_reviews",
649+
lambda issue_number: [
650+
{
651+
"id": 11,
652+
"submitted_at": "2026-03-17T10:00:00Z",
653+
"state": "COMMENTED",
654+
"commit_id": "head-1",
655+
"user": {"login": "alice"},
656+
}
657+
],
658+
)
659+
assert reviewer_bot.handle_workflow_run_event(state) is True
660+
assert review["transition_warning_sent"] is None
661+
assert review["transition_notice_sent_at"] is None
662+
663+
498664
def test_deferred_comment_missing_live_object_preserves_source_time_freshness(tmp_path, monkeypatch):
499665
state = make_state()
500666
review = reviewer_bot.ensure_review_entry(state, 42, create=True)
@@ -1258,6 +1424,31 @@ def test_artifact_gap_reason_requires_prior_visibility_or_documented_retention()
12581424
assert sweeper.classify_artifact_gap_reason(missing) == "artifact_missing"
12591425

12601426

1427+
def test_discover_visible_comment_events_skips_github_actions_and_bot_comments(monkeypatch):
1428+
state = make_state()
1429+
review = reviewer_bot.ensure_review_entry(state, 42, create=True)
1430+
assert review is not None
1431+
monkeypatch.setattr(
1432+
reviewer_bot,
1433+
"github_api",
1434+
lambda method, endpoint, data=None: [
1435+
{
1436+
"id": 100,
1437+
"created_at": "2026-03-25T10:00:00Z",
1438+
"user": {"login": "github-actions[bot]", "type": "Bot"},
1439+
},
1440+
{
1441+
"id": 101,
1442+
"created_at": "2026-03-25T11:00:00Z",
1443+
"user": {"login": "alice", "type": "User"},
1444+
},
1445+
],
1446+
)
1447+
discovered, complete = sweeper._discover_visible_comment_events(reviewer_bot, 42, review)
1448+
assert complete is True
1449+
assert [item["source_event_key"] for item in discovered] == ["issue_comment:101"]
1450+
1451+
12611452
def test_sweeper_creates_keyed_deferred_gaps_for_visible_comments_reviews_and_dismissals(monkeypatch):
12621453
state = make_state()
12631454
review = reviewer_bot.ensure_review_entry(state, 42, create=True)
@@ -1268,15 +1459,15 @@ def test_sweeper_creates_keyed_deferred_gaps_for_visible_comments_reviews_and_di
12681459
"github_api",
12691460
lambda method, endpoint, data=None: {
12701461
"pulls/42": {"state": "open", "head": {"sha": "head-1"}},
1271-
"issues/42/comments?per_page=100&page=1": [{"id": 101, "created_at": "2026-03-17T10:00:00Z"}],
1462+
"issues/42/comments?per_page=100&page=1": [{"id": 101, "created_at": "2026-03-25T10:00:00Z"}],
12721463
}.get(endpoint),
12731464
)
12741465
monkeypatch.setattr(
12751466
reviewer_bot,
12761467
"get_pull_request_reviews",
12771468
lambda issue_number: [
1278-
{"id": 202, "submitted_at": "2026-03-17T11:00:00Z", "state": "APPROVED"},
1279-
{"id": 303, "submitted_at": "2026-03-17T09:00:00Z", "updated_at": "2026-03-17T12:00:00Z", "state": "DISMISSED"},
1469+
{"id": 202, "submitted_at": "2026-03-25T11:00:00Z", "state": "APPROVED"},
1470+
{"id": 303, "submitted_at": "2026-03-25T09:00:00Z", "updated_at": "2026-03-25T12:00:00Z", "state": "DISMISSED"},
12801471
],
12811472
)
12821473
assert sweeper.sweep_deferred_gaps(reviewer_bot, state) is True
@@ -1331,6 +1522,53 @@ def test_sweeper_skips_events_already_reconciled_by_source_event_key(monkeypatch
13311522
assert state["active_reviews"]["42"]["deferred_gaps"] == {}
13321523

13331524

1525+
def test_sweeper_fetches_single_candidate_run_detail_without_exact_artifact_match(monkeypatch):
1526+
run_correlation = {
1527+
"candidate_run_ids": [123],
1528+
"correlated_run": None,
1529+
"correlated_run_found": False,
1530+
}
1531+
monkeypatch.setattr(sweeper, "_fetch_run_detail", lambda bot, run_id: {"id": run_id, "status": "completed", "conclusion": "action_required"})
1532+
detail = sweeper._maybe_fetch_single_candidate_run_detail(reviewer_bot, run_correlation, {"status": "no_exact_artifact_match"})
1533+
assert detail == {"id": 123, "status": "completed", "conclusion": "action_required"}
1534+
assert run_correlation["correlated_run"] == 123
1535+
1536+
1537+
def test_sweeper_visible_review_repair_refreshes_current_reviewer_activity_without_artifact(monkeypatch):
1538+
state = make_state()
1539+
review = reviewer_bot.ensure_review_entry(state, 42, create=True)
1540+
assert review is not None
1541+
review["current_reviewer"] = "alice"
1542+
review["active_cycle_started_at"] = "2026-03-17T09:00:00Z"
1543+
review["transition_warning_sent"] = "2026-03-18T00:00:00Z"
1544+
review["transition_notice_sent_at"] = "2026-03-25T00:00:00Z"
1545+
review["deferred_gaps"]["pull_request_review:202"] = {"reason": "artifact_missing"}
1546+
monkeypatch.setattr(
1547+
reviewer_bot,
1548+
"github_api",
1549+
lambda method, endpoint, data=None: {"state": "open", "head": {"sha": "head-1"}} if endpoint == "pulls/42" else {"workflow_runs": []},
1550+
)
1551+
monkeypatch.setattr(
1552+
reviewer_bot,
1553+
"get_pull_request_reviews",
1554+
lambda issue_number: [
1555+
{
1556+
"id": 202,
1557+
"submitted_at": "2026-03-25T11:00:00Z",
1558+
"state": "COMMENTED",
1559+
"commit_id": "head-1",
1560+
"user": {"login": "alice"},
1561+
}
1562+
],
1563+
)
1564+
assert sweeper.sweep_deferred_gaps(reviewer_bot, state) is True
1565+
assert review["last_reviewer_activity"] == "2026-03-25T11:00:00Z"
1566+
assert review["transition_warning_sent"] is None
1567+
assert review["transition_notice_sent_at"] is None
1568+
assert "pull_request_review:202" not in review["deferred_gaps"]
1569+
assert "pull_request_review:202" in review["reconciled_source_events"]
1570+
1571+
13341572
def test_workflow_policy_split_and_lock_only_boundaries():
13351573
workflows_dir = Path(".github/workflows")
13361574
required = {
@@ -1365,6 +1603,13 @@ def test_workflow_policy_split_and_lock_only_boundaries():
13651603
assert "@" in value and len(value.split("@", 1)[1]) == 40
13661604

13671605

1606+
def test_pr_comment_observer_workflow_builds_payload_inline_without_bot_src_root():
1607+
workflow = Path(".github/workflows/reviewer-bot-pr-comment-observer.yml").read_text(encoding="utf-8")
1608+
assert "BOT_SRC_ROOT" not in workflow
1609+
assert "build_pr_comment_observer_payload" not in workflow
1610+
assert "Fetch trusted bot source tarball" not in workflow
1611+
1612+
13681613
def test_workflow_summaries_and_runbook_references_exist():
13691614
runbook = Path("docs/reviewer-bot-review-freshness-operator-runbook.md")
13701615
assert runbook.exists()
@@ -1387,17 +1632,15 @@ def test_trusted_pr_comment_workflow_preflights_same_repo_before_mutation():
13871632
assert "RUN_TRUSTED_PR_COMMENT" in workflow_text
13881633

13891634

1390-
def test_pr_comment_observer_routes_through_reviewer_bot_payload_builder():
1635+
def test_pr_comment_observer_workflow_uses_inline_payload_builder():
13911636
data = yaml.safe_load(Path(".github/workflows/reviewer-bot-pr-comment-observer.yml").read_text(encoding="utf-8"))
13921637
job = data["jobs"]["observer"]
13931638
steps = job["steps"]
1394-
assert steps[0]["name"] == "Install uv"
1395-
assert steps[1]["name"] == "Fetch trusted bot source tarball"
1396-
assert steps[2]["name"] == "Build deferred comment artifact"
1397-
assert steps[3]["name"] == "Upload deferred comment artifact"
1639+
assert steps[0]["name"] == "Build deferred comment artifact"
1640+
assert steps[1]["name"] == "Upload deferred comment artifact"
13981641
workflow_text = Path(".github/workflows/reviewer-bot-pr-comment-observer.yml").read_text(encoding="utf-8")
1399-
assert "build_pr_comment_observer_payload" in workflow_text
1400-
assert 'uv run --project "$BOT_SRC_ROOT" python - <<\'PY\'' in workflow_text
1642+
assert "build_pr_comment_observer_payload" not in workflow_text
1643+
assert 'uv run --project "$BOT_SRC_ROOT"' not in workflow_text
14011644

14021645

14031646
def test_build_pr_comment_observer_payload_marks_trusted_direct_same_repo_as_observer_noop(monkeypatch):

scripts/reviewer_bot_lib/comment_routing.py

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -235,8 +235,7 @@ def _record_conversation_freshness(bot, state: dict, issue_number: int, comment_
235235
timestamp=created_at,
236236
actor=comment_author,
237237
)
238-
review_data["last_reviewer_activity"] = created_at
239-
review_data["transition_warning_sent"] = None
238+
bot.reviews_module.record_reviewer_activity(review_data, created_at)
240239
return changed
241240
return False
242241

scripts/reviewer_bot_lib/reconcile.py

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,7 @@ def _record_review_rebuild(bot, state: dict, issue_number: int, review_data: dic
6767
reviewed_head_sha=commit_id,
6868
source_precedence=1,
6969
)
70+
bot.reviews_module.record_reviewer_activity(review_data, submitted_at)
7071
return bool(completion.get("completed"))
7172

7273

@@ -108,6 +109,8 @@ def reconcile_active_review_entry(
108109
reviewed_head_sha=commit_id,
109110
source_precedence=1,
110111
) or state_changed
112+
bot.reviews_module.record_reviewer_activity(review_data, submitted_at)
113+
state_changed = True
111114
messages.append(f"latest review by @{assigned_reviewer} is `{latest_state}`")
112115
if _record_review_rebuild(bot, state, issue_number, review_data):
113116
state_changed = True
@@ -520,6 +523,7 @@ def handle_workflow_run_event(bot, state: dict) -> bool:
520523
reviewed_head_sha=live_commit_id,
521524
source_precedence=1,
522525
)
526+
bot.reviews_module.record_reviewer_activity(review_data, live_submitted_at)
523527
_record_review_rebuild(bot, state, pr_number, review_data)
524528
_mark_reconciled_source_event(review_data, source_event_key)
525529
_clear_source_event_key(review_data, source_event_key)

0 commit comments

Comments
 (0)