fix(robomp): route issues.reopened to admission-gated re-triage
The finalized-issue boilerplate promises "reopen and I'll triage again from scratch," but github_events.route() dropped issues.reopened to the ignored skip branch (only opened/closed handled), while pull_request.reopened was already routed. A user following the instruction got silence. Route issues.reopened like issues.opened (submitter-attributable triage_issue, same per-user rate budget) and dispatch it in queue._dispatch(). triage_issue now tears down a stale finalized workspace (merged/closed/abandoned) before re-provisioning, mirroring the maintainer directive-reopen teardown so the re-triage branches afresh from default instead of a merged/deleted branch. Fixes #5891
This commit is contained in:
@@ -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.
|
||||
|
||||
@@ -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,
|
||||
|
||||
@@ -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(
|
||||
|
||||
@@ -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",
|
||||
|
||||
@@ -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"
|
||||
|
||||
Reference in New Issue
Block a user