Skip to content

Commit 97dab0f

Browse files
authored
fix(reviewer-bot): repair stale labels after projection changes (#543)
1 parent 2f3b7c4 commit 97dab0f

File tree

7 files changed

+163
-0
lines changed

7 files changed

+163
-0
lines changed

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

Lines changed: 106 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ def make_state():
1313
return {
1414
"schema_version": reviewer_bot.STATE_SCHEMA_VERSION,
1515
"freshness_runtime_epoch": reviewer_bot.FRESHNESS_RUNTIME_EPOCH_V18,
16+
"status_projection_epoch": reviewer_bot.STATUS_PROJECTION_EPOCH,
1617
"last_updated": None,
1718
"current_index": 0,
1819
"queue": [
@@ -360,6 +361,111 @@ def test_main_schedule_reviewer_review_repair_marks_item_for_label_sync(monkeypa
360361
assert synced_issue_numbers == [42]
361362

362363

364+
def test_main_schedule_status_projection_epoch_mismatch_triggers_label_repair_sweep(monkeypatch):
365+
monkeypatch.setenv("EVENT_NAME", "schedule")
366+
monkeypatch.setenv("EVENT_ACTION", "")
367+
state = make_state()
368+
state["status_projection_epoch"] = "status_projection_v1"
369+
review = reviewer_bot.ensure_review_entry(state, 42, create=True)
370+
assert review is not None
371+
review["current_reviewer"] = "alice"
372+
synced_issue_numbers = []
373+
saved_epochs = []
374+
375+
monkeypatch.setattr(reviewer_bot, "acquire_state_issue_lease_lock", lambda: None)
376+
monkeypatch.setattr(reviewer_bot, "release_state_issue_lease_lock", lambda: True)
377+
monkeypatch.setattr(reviewer_bot, "load_state", lambda *args, **kwargs: state)
378+
monkeypatch.setattr(reviewer_bot, "process_pass_until_expirations", lambda current: (current, []))
379+
monkeypatch.setattr(reviewer_bot, "sync_members_with_queue", lambda current: (current, []))
380+
monkeypatch.setattr(reviewer_bot, "handle_scheduled_check", lambda current: False)
381+
monkeypatch.setattr(reviewer_bot, "list_open_items_with_status_labels", lambda: [99])
382+
monkeypatch.setattr(
383+
reviewer_bot,
384+
"sync_status_labels_for_items",
385+
lambda current, issue_numbers: synced_issue_numbers.extend(issue_numbers) or True,
386+
)
387+
monkeypatch.setattr(
388+
reviewer_bot,
389+
"save_state",
390+
lambda current: saved_epochs.append(current.get("status_projection_epoch")) or True,
391+
)
392+
393+
reviewer_bot.main()
394+
395+
assert synced_issue_numbers == [42, 99]
396+
assert saved_epochs[-1] == reviewer_bot.STATUS_PROJECTION_EPOCH
397+
398+
399+
def test_main_schedule_status_projection_epoch_not_advanced_on_label_sync_failure(monkeypatch):
400+
monkeypatch.setenv("EVENT_NAME", "schedule")
401+
monkeypatch.setenv("EVENT_ACTION", "")
402+
state = make_state()
403+
state["status_projection_epoch"] = "status_projection_v1"
404+
review = reviewer_bot.ensure_review_entry(state, 42, create=True)
405+
assert review is not None
406+
review["current_reviewer"] = "alice"
407+
saved_epochs = []
408+
409+
monkeypatch.setattr(reviewer_bot, "acquire_state_issue_lease_lock", lambda: None)
410+
monkeypatch.setattr(reviewer_bot, "release_state_issue_lease_lock", lambda: True)
411+
monkeypatch.setattr(reviewer_bot, "load_state", lambda *args, **kwargs: state)
412+
monkeypatch.setattr(reviewer_bot, "process_pass_until_expirations", lambda current: (current, []))
413+
monkeypatch.setattr(reviewer_bot, "sync_members_with_queue", lambda current: (current, []))
414+
monkeypatch.setattr(reviewer_bot, "handle_scheduled_check", lambda current: False)
415+
monkeypatch.setattr(reviewer_bot, "list_open_items_with_status_labels", lambda: [42])
416+
monkeypatch.setattr(
417+
reviewer_bot,
418+
"sync_status_labels_for_items",
419+
lambda current, issue_numbers: (_ for _ in ()).throw(RuntimeError("projection exploded")),
420+
)
421+
monkeypatch.setattr(
422+
reviewer_bot,
423+
"save_state",
424+
lambda current: saved_epochs.append(current.get("status_projection_epoch")) or True,
425+
)
426+
427+
reviewer_bot.main()
428+
assert all(epoch != reviewer_bot.STATUS_PROJECTION_EPOCH for epoch in saved_epochs)
429+
430+
431+
def test_main_schedule_projection_epoch_repair_relabels_previously_repaired_pr(monkeypatch):
432+
monkeypatch.setenv("EVENT_NAME", "schedule")
433+
monkeypatch.setenv("EVENT_ACTION", "")
434+
state = make_state()
435+
state["status_projection_epoch"] = "status_projection_v1"
436+
review = reviewer_bot.ensure_review_entry(state, 256, create=True)
437+
assert review is not None
438+
review["current_reviewer"] = "vccjgust"
439+
review["reviewer_review"]["accepted"] = {
440+
"semantic_key": "pull_request_review:3821749029",
441+
"timestamp": "2026-02-18T20:28:12Z",
442+
"actor": "vccjgust",
443+
"reviewed_head_sha": "head-1",
444+
"source_precedence": 1,
445+
"payload": {},
446+
}
447+
review["active_head_sha"] = "head-1"
448+
synced_issue_numbers = []
449+
450+
monkeypatch.setattr(reviewer_bot, "acquire_state_issue_lease_lock", lambda: None)
451+
monkeypatch.setattr(reviewer_bot, "release_state_issue_lease_lock", lambda: True)
452+
monkeypatch.setattr(reviewer_bot, "load_state", lambda *args, **kwargs: state)
453+
monkeypatch.setattr(reviewer_bot, "process_pass_until_expirations", lambda current: (current, []))
454+
monkeypatch.setattr(reviewer_bot, "sync_members_with_queue", lambda current: (current, []))
455+
monkeypatch.setattr(reviewer_bot, "handle_scheduled_check", lambda current: False)
456+
monkeypatch.setattr(reviewer_bot, "list_open_items_with_status_labels", lambda: [256])
457+
monkeypatch.setattr(
458+
reviewer_bot,
459+
"sync_status_labels_for_items",
460+
lambda current, issue_numbers: synced_issue_numbers.extend(issue_numbers) or True,
461+
)
462+
monkeypatch.setattr(reviewer_bot, "save_state", lambda current: True)
463+
464+
reviewer_bot.main()
465+
466+
assert synced_issue_numbers == [256]
467+
468+
363469
def test_main_mutating_event_fails_closed_when_state_unavailable(monkeypatch):
364470
monkeypatch.setenv("EVENT_NAME", "issue_comment")
365471
monkeypatch.setenv("EVENT_ACTION", "created")

scripts/reviewer_bot.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -128,6 +128,7 @@
128128
STATUS_AWAITING_WRITE_APPROVAL_LABEL,
129129
STATUS_LABEL_CONFIG,
130130
STATUS_LABELS,
131+
STATUS_PROJECTION_EPOCH,
131132
TRANSITION_PERIOD_DAYS,
132133
AssignmentAttempt,
133134
GitHubApiResult,

scripts/reviewer_bot_lib/app.py

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,10 @@
44
import sys
55

66
from .context import ReviewerBotContext
7+
from .maintenance import (
8+
collect_status_projection_repair_items,
9+
status_projection_repair_needed,
10+
)
711

812

913
def _revalidate_epoch(bot: ReviewerBotContext, expected_epoch: str | None, phase: str) -> None:
@@ -106,6 +110,7 @@ def main(bot: ReviewerBotContext):
106110
touched_items: list[int] = []
107111
projection_failure: RuntimeError | None = None
108112
loaded_epoch: str | None = None
113+
projection_epoch_repair = False
109114

110115
try:
111116
if lock_required:
@@ -172,6 +177,14 @@ def main(bot: ReviewerBotContext):
172177
)
173178

174179
touched_items = bot.drain_touched_items()
180+
if event_name in {"schedule", "workflow_dispatch"} and status_projection_repair_needed(bot, state):
181+
touched_items = sorted(
182+
{
183+
*touched_items,
184+
*collect_status_projection_repair_items(bot, state),
185+
}
186+
)
187+
projection_epoch_repair = True
175188

176189
if state_changed or sync_changes or restored:
177190
if not lock_acquired:
@@ -231,6 +244,14 @@ def main(bot: ReviewerBotContext):
231244
raise RuntimeError(
232245
"Projection failed and repair-needed metadata could not be persisted."
233246
)
247+
else:
248+
if projection_epoch_repair:
249+
state["status_projection_epoch"] = bot.STATUS_PROJECTION_EPOCH
250+
_revalidate_epoch(bot, loaded_epoch, "status-projection epoch save")
251+
if not bot.save_state(state):
252+
raise RuntimeError(
253+
"Status projection epoch repair succeeded but could not be persisted."
254+
)
234255

235256
with open(os.environ.get("GITHUB_OUTPUT", "/dev/null"), "a") as output_file:
236257
output_file.write(

scripts/reviewer_bot_lib/config.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,7 @@
4444
STATE_SCHEMA_VERSION = 18
4545
FRESHNESS_RUNTIME_EPOCH_LEGACY = "legacy_v14"
4646
FRESHNESS_RUNTIME_EPOCH_V18 = "freshness_v15"
47+
STATUS_PROJECTION_EPOCH = "status_projection_v2"
4748
REVIEW_FRESHNESS_RUNBOOK_PATH = "docs/reviewer-bot-review-freshness-operator-runbook.md"
4849
AUTHOR_ASSOCIATION_TRUST_ALLOWLIST = {"OWNER", "MEMBER", "COLLABORATOR"}
4950
DEFERRED_ARTIFACT_RETENTION_DAYS = 7

scripts/reviewer_bot_lib/maintenance.py

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,17 @@ def _now_iso(bot) -> str:
2020
return bot.datetime.now(bot.timezone.utc).isoformat()
2121

2222

23+
def status_projection_repair_needed(bot, state: dict) -> bool:
24+
current_epoch = state.get("status_projection_epoch")
25+
return current_epoch != bot.STATUS_PROJECTION_EPOCH
26+
27+
28+
def collect_status_projection_repair_items(bot, state: dict) -> list[int]:
29+
numbers = set(bot.list_open_items_with_status_labels())
30+
numbers.update(bot.reviews_module.list_open_tracked_review_items(state))
31+
return sorted(number for number in numbers if isinstance(number, int) and number > 0)
32+
33+
2334
def handle_manual_dispatch(bot, state: dict) -> bool:
2435
action = os.environ.get("MANUAL_ACTION", "")
2536
if action == "show-state":

scripts/reviewer_bot_lib/reviews.py

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -882,6 +882,26 @@ def list_open_items_with_status_labels(bot) -> list[int]:
882882
return sorted(numbers)
883883

884884

885+
def list_open_tracked_review_items(state: dict) -> list[int]:
886+
numbers: set[int] = set()
887+
active_reviews = state.get("active_reviews")
888+
if not isinstance(active_reviews, dict):
889+
return []
890+
for issue_key, review_data in active_reviews.items():
891+
if not isinstance(review_data, dict):
892+
continue
893+
current_reviewer = review_data.get("current_reviewer")
894+
if not isinstance(current_reviewer, str) or not current_reviewer.strip():
895+
continue
896+
try:
897+
issue_number = int(issue_key)
898+
except (TypeError, ValueError):
899+
continue
900+
if issue_number > 0:
901+
numbers.add(issue_number)
902+
return sorted(numbers)
903+
904+
885905
def handle_pr_approved_review(bot, state: dict, issue_number: int, review_author: str, completion_source: str) -> bool:
886906
review_data = ensure_review_entry(state, issue_number)
887907
if review_data is None:

scripts/reviewer_bot_lib/state_store.py

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -324,6 +324,7 @@ def load_state(bot: StateStoreContext, *, fail_on_unavailable: bool = False) ->
324324
default_state = {
325325
"schema_version": STATE_SCHEMA_VERSION,
326326
"freshness_runtime_epoch": FRESHNESS_RUNTIME_EPOCH_LEGACY,
327+
"status_projection_epoch": None,
327328
"last_updated": None,
328329
"current_index": 0,
329330
"queue": [],
@@ -347,6 +348,8 @@ def load_state(bot: StateStoreContext, *, fail_on_unavailable: bool = False) ->
347348
state["schema_version"] = STATE_SCHEMA_VERSION
348349
if not isinstance(state.get("freshness_runtime_epoch"), str) or not state.get("freshness_runtime_epoch"):
349350
state["freshness_runtime_epoch"] = FRESHNESS_RUNTIME_EPOCH_LEGACY
351+
if not isinstance(state.get("status_projection_epoch"), str) or not state.get("status_projection_epoch"):
352+
state["status_projection_epoch"] = None
350353
if state.get("last_updated") is None:
351354
state["last_updated"] = None
352355
if not isinstance(state.get("current_index"), int):

0 commit comments

Comments
 (0)