diff --git a/python/robomp/src/github_events.py b/python/robomp/src/github_events.py index 8af7a0e10..cf2b7029a 100644 --- a/python/robomp/src/github_events.py +++ b/python/robomp/src/github_events.py @@ -300,10 +300,13 @@ def route( if not isinstance(number, int): return RouteDecision("skip", None, repo, None, "issue missing number") key = issue_key(repo, number) - if action == "opened": + if action in ("opened", "reopened"): + # A reopen is submitter-attributable exactly like an open, and + # `finalized_issue_comment.md` promises re-triage on reopen, so it + # re-triages from scratch and spends the same per-user rate budget. login, assoc = _submitter_info(issue) return RouteDecision( - "queue", "triage_issue", repo, key, "issues.opened", submitter=login, association=assoc + "queue", "triage_issue", repo, key, f"issues.{action}", submitter=login, association=assoc ) if action == "closed": # Cleanup is a lifecycle event, not a user submission; no rate-limit subject. diff --git a/python/robomp/src/queue.py b/python/robomp/src/queue.py index d64e07d7f..9d1c711f9 100644 --- a/python/robomp/src/queue.py +++ b/python/robomp/src/queue.py @@ -382,7 +382,7 @@ class WorkerPool: "recovered": row.attempts >= 2, }, ) - if event == "issues" and action == "opened": + if event == "issues" and action in ("opened", "reopened"): await tasks.triage_issue( settings=self.settings, db=self.db, diff --git a/python/robomp/src/tasks.py b/python/robomp/src/tasks.py index ae77df876..c8f6dcee3 100644 --- a/python/robomp/src/tasks.py +++ b/python/robomp/src/tasks.py @@ -282,7 +282,8 @@ async def triage_issue( log.info("skip: triage on PR-like issue", extra={"repo": repo.full_name, "n": issue.number}) return key = issue_key(repo.full_name, issue.number) - if db.get_issue(key) is None: + existing = db.get_issue(key) + if existing is None: # First-time triage: bail if a PR (human or another bot) already # claims to close this issue via Closes/Fixes/Resolves syntax or # the Development panel. We never replay closing-PR detection on @@ -304,6 +305,12 @@ async def triage_issue( extra={"key": key, "prs": list(closing_prs)}, ) return + elif existing.state in ("merged", "closed", "abandoned"): + # Reopen of a finalized issue (issues.reopened): the prior branch is + # stale (merged/deleted), so tear the workspace down and branch afresh + # from default — the same teardown the maintainer directive-reopen uses. + log.info("reopen re-triage", extra={"key": key, "from_state": existing.state}) + await _run_workspace_op(sandbox.remove_workspace, repo=repo.full_name, number=issue.number) db.upsert_issue(key=key, repo=repo.full_name, number=issue.number, state="reproducing") clone_url = repo.clone_url workspace = await _run_workspace_op( diff --git a/python/robomp/tests/test_github_events.py b/python/robomp/tests/test_github_events.py index cbefdc034..35aa71dbf 100644 --- a/python/robomp/tests/test_github_events.py +++ b/python/robomp/tests/test_github_events.py @@ -53,6 +53,27 @@ def test_route_issue_opened_queues_triage() -> None: assert decision.issue_key == "octo/widget#4" +def test_route_issue_reopened_queues_triage() -> None: + # `finalized_issue_comment.md` promises re-triage on reopen; the router must + # queue it as a submitter-attributable triage (not drop it to the skip branch). + decision = route( + "issues", + { + "action": "reopened", + "issue": {"number": 4, "user": {"login": "alice"}, "author_association": "CONTRIBUTOR"}, + "repository": {"full_name": "octo/widget"}, + }, + allowlist=ALLOWLIST, + bot_login=BOT, + ) + assert decision.should_queue + assert decision.task == "triage_issue" + assert decision.issue_key == "octo/widget#4" + assert decision.reason == "issues.reopened" + assert decision.submitter == "alice" + assert decision.association == "CONTRIBUTOR" + + def test_route_skips_disallowed_repo() -> None: decision = route( "issues", diff --git a/python/robomp/tests/test_tasks.py b/python/robomp/tests/test_tasks.py index 98304823b..3a0bf6a6a 100644 --- a/python/robomp/tests/test_tasks.py +++ b/python/robomp/tests/test_tasks.py @@ -167,3 +167,71 @@ async def test_run_workspace_op_logs_worker_exception_on_concurrent_cancel(caplo assert any(r.exc_info and r.exc_info[1] is boom for r in warnings), ( "the worker's exception was not attached to the warning" ) + + +async def test_triage_issue_reopen_tears_down_finalized_workspace(db, settings, monkeypatch, tmp_path): + """Re-triage of a finalized (reopened) issue must clear the stale workspace first. + + The prior branch was merged/deleted when the issue finalized, so a reopen has + to branch afresh — mirroring the maintainer directive-reopen teardown. + """ + + async def _resolve_repo_and_issue(_github, _payload): + repo = RepoInfo( + full_name="octo/widget", + default_branch="main", + clone_url="https://x/octo/widget.git", + private=False, + ) + issue = IssueInfo( + repo="octo/widget", + number=1, + title="bug", + body="b", + state="open", + author="alice", + labels=(), + is_pull_request=False, + ) + return repo, issue + + monkeypatch.setattr(tasks, "_resolve_repo_and_issue", _resolve_repo_and_issue) + + # The bot previously finalized this issue: a stale row + workspace exist. + db.upsert_issue(key="octo/widget#1", repo="octo/widget", number=1, state="closed") + + calls: list[str] = [] + + def _remove(**_kwargs): + calls.append("remove") + + def _ensure(**_kwargs): + calls.append("ensure") + return SimpleNamespace(branch="farm/x/y", session_dir=str(tmp_path / "sess")) + + async def _fail_closing(*_a, **_k): + raise AssertionError("closing-PR guard must not run when a DB row already exists") + + github = SimpleNamespace(list_closing_pull_requests=_fail_closing) + sandbox = SimpleNamespace(natives_cache=None, ensure_workspace=_ensure, remove_workspace=_remove) + + async def _noop_run_task(**_kwargs): + return None + + monkeypatch.setattr(tasks, "run_task", _noop_run_task) + + await tasks.triage_issue( + settings=settings, + db=db, + github=github, + sandbox=sandbox, + git_transport=SimpleNamespace(), + payload={}, + delivery_id="d1", + ) + + # Teardown must precede re-provisioning, and the row resets to a live state. + assert calls == ["remove", "ensure"] + row = db.get_issue("octo/widget#1") + assert row is not None + assert row.state == "reproducing"