feat(python/robomp): added the wontfix primary classification and comment-only triage path
- Added `wontfix` to primary classification handling by updating host-tool classification metadata and issue taxonomy prompts. - Updated kickoff/follow-up/system prompt guidance to treat intentional-design reports as `wontfix`, with maintainer-intent signals stopping work and ending in a single explanatory comment. - Added tests to verify `classify_issue` persists a `wontfix` classification and returns a no-PR, comment-only next step.
This commit is contained in:
@@ -113,9 +113,7 @@ def test_bot_login_normalizes_mention_case_and_app_suffix(
|
||||
assert cfg.bot_login == "roboomp"
|
||||
|
||||
|
||||
def test_maintainer_logins_normalize_csv_entries(
|
||||
monkeypatch: pytest.MonkeyPatch, env: dict[str, str]
|
||||
) -> None:
|
||||
def test_maintainer_logins_normalize_csv_entries(monkeypatch: pytest.MonkeyPatch, env: dict[str, str]) -> None:
|
||||
monkeypatch.setenv("ROBOMP_MAINTAINER_LOGINS", " can1357, @ROBOOMP , @Alice[bot] ,, ")
|
||||
reset_settings_cache()
|
||||
cfg = Settings() # type: ignore[call-arg]
|
||||
|
||||
@@ -2,6 +2,7 @@ from __future__ import annotations
|
||||
|
||||
import hashlib
|
||||
import hmac
|
||||
|
||||
import pytest
|
||||
|
||||
from robomp.github_events import (
|
||||
|
||||
@@ -842,6 +842,25 @@ def test_classify_issue_question_skips_repro_path(db: Database, tmp_path: Path)
|
||||
assert row is not None and row.classification == "question"
|
||||
|
||||
|
||||
def test_classify_issue_wontfix_takes_comment_only_path(db: Database, tmp_path: Path) -> None:
|
||||
"""`wontfix` is a non-PR primary: labels land, classification persists, and the
|
||||
echoed next step routes to a single explanatory comment — no repro, no PR."""
|
||||
transport = httpx.MockTransport(lambda r: httpx.Response(200, json=[{"name": "wontfix"}, {"name": "triaged"}]))
|
||||
bindings, loop, t = _bindings(db, tmp_path, transport)
|
||||
try:
|
||||
tool = next(x for x in build(bindings) if x.name == "classify_issue")
|
||||
result = tool.execute(
|
||||
{"primary": "wontfix", "rationale": "intentional design tradeoff, no demonstrated impact"},
|
||||
_ctx(),
|
||||
)
|
||||
finally:
|
||||
_stop_loop(loop, t)
|
||||
assert "wontfix" in result
|
||||
assert "no PR" in result
|
||||
row = db.get_issue(bindings.issue_key)
|
||||
assert row is not None and row.classification == "wontfix"
|
||||
|
||||
|
||||
def test_classify_issue_rejects_bug_without_priority(db: Database, tmp_path: Path) -> None:
|
||||
bindings, loop, t = _bindings(db, tmp_path, httpx.MockTransport(lambda r: httpx.Response(500)))
|
||||
try:
|
||||
@@ -1302,7 +1321,9 @@ def test_impl_gate_allows_later_authorized_event_to_reach_repo_commands(
|
||||
assert calls
|
||||
|
||||
|
||||
def test_impl_gate_ignores_skipped_authorized_event(db: Database, tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None:
|
||||
def test_impl_gate_ignores_skipped_authorized_event(
|
||||
db: Database, tmp_path: Path, monkeypatch: pytest.MonkeyPatch
|
||||
) -> None:
|
||||
calls: list[list[str] | tuple[str, ...]] = []
|
||||
|
||||
def record_repo_command(_bindings: ToolBindings, cmd: list[str] | tuple[str, ...], *, timeout: float | None = None):
|
||||
|
||||
@@ -166,7 +166,9 @@ async def _async_client(app) -> httpx.AsyncClient:
|
||||
)
|
||||
|
||||
|
||||
def test_read_remote_urls_uses_safe_directory_and_slot_identity(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None:
|
||||
def test_read_remote_urls_uses_safe_directory_and_slot_identity(
|
||||
tmp_path: Path, monkeypatch: pytest.MonkeyPatch
|
||||
) -> None:
|
||||
from robomp.proxy import server as proxy_server
|
||||
|
||||
captured: dict[str, object] = {}
|
||||
@@ -1134,7 +1136,9 @@ async def test_git_fetch_rejects_option_shaped_origin(proxy_settings: Settings,
|
||||
pool_dir = _stage_pool(proxy_settings, upstream_repo)
|
||||
config_path = pool_dir / ".git" / "config"
|
||||
config_text = config_path.read_text(encoding="utf-8")
|
||||
config_path.write_text(config_text.replace(f"\turl = {upstream_repo}\n", "\turl = --upload-pack=env\n"), encoding="utf-8")
|
||||
config_path.write_text(
|
||||
config_text.replace(f"\turl = {upstream_repo}\n", "\turl = --upload-pack=env\n"), encoding="utf-8"
|
||||
)
|
||||
|
||||
app = _build_app(proxy_settings)
|
||||
body = b'{"repo":"octo/widget"}'
|
||||
@@ -1148,8 +1152,6 @@ async def test_git_fetch_rejects_option_shaped_origin(proxy_settings: Settings,
|
||||
assert resp.status_code == 400, resp.text
|
||||
|
||||
|
||||
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
"clone_url",
|
||||
[
|
||||
@@ -1205,6 +1207,7 @@ async def test_git_push_rejects_attacker_pushurl(proxy_settings: Settings, upstr
|
||||
assert resp.status_code == 400, resp.text
|
||||
assert not _bare_has_branch(upstream_repo, branch)
|
||||
|
||||
|
||||
# ============================================================================
|
||||
# fetch_ref refuses refspec / option injection in `ref`
|
||||
# ============================================================================
|
||||
|
||||
@@ -1110,7 +1110,9 @@ def test_remove_workspace(tmp_path: Path, upstream_repo: Path) -> None:
|
||||
assert not ws.root.exists()
|
||||
|
||||
|
||||
def test_remove_workspace_prunes_pool_after_failed_worktree_remove(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None:
|
||||
def test_remove_workspace_prunes_pool_after_failed_worktree_remove(
|
||||
tmp_path: Path, monkeypatch: pytest.MonkeyPatch
|
||||
) -> None:
|
||||
mgr = SandboxManager(tmp_path)
|
||||
# Create a real repo_dir on disk so `repo_dir.exists()` is True on entry.
|
||||
ws_root = mgr.workspace_root("o/r", 7)
|
||||
@@ -1185,6 +1187,7 @@ def test_remove_workspace_prunes_when_failed_remove_already_deleted_checkout(
|
||||
prune_idx = next(i for i, (c, _) in enumerate(calls) if c == ["git", "worktree", "prune"])
|
||||
assert calls[prune_idx][1] == pool, "prune did not run in the repo's pool dir"
|
||||
|
||||
|
||||
def test_redact_credentials_strips_userinfo() -> None:
|
||||
from robomp.sandbox import redact_credentials
|
||||
|
||||
@@ -1911,7 +1914,9 @@ def test_run_timeout_raises_git_command_error_124(monkeypatch: pytest.MonkeyPatc
|
||||
assert seen["timeout"] == s._DEFAULT_SANDBOX_SUBPROCESS_TIMEOUT
|
||||
|
||||
|
||||
def test_ensure_workspace_raises_when_local_branch_probe_times_out(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None:
|
||||
def test_ensure_workspace_raises_when_local_branch_probe_times_out(
|
||||
tmp_path: Path, monkeypatch: pytest.MonkeyPatch
|
||||
) -> None:
|
||||
mgr = SandboxManager(tmp_path)
|
||||
mgr.natives_cache = None
|
||||
mgr.transport = SimpleNamespace(
|
||||
@@ -1946,7 +1951,9 @@ def test_ensure_workspace_raises_when_local_branch_probe_times_out(tmp_path: Pat
|
||||
)
|
||||
|
||||
|
||||
def test_ensure_workspace_raises_when_remote_branch_probe_times_out(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None:
|
||||
def test_ensure_workspace_raises_when_remote_branch_probe_times_out(
|
||||
tmp_path: Path, monkeypatch: pytest.MonkeyPatch
|
||||
) -> None:
|
||||
mgr = SandboxManager(tmp_path)
|
||||
mgr.natives_cache = None
|
||||
mgr.transport = SimpleNamespace(
|
||||
|
||||
@@ -18,7 +18,6 @@ from robomp.config import Settings, reset_settings_cache
|
||||
from robomp.db import get_database
|
||||
from robomp.server import create_app
|
||||
|
||||
|
||||
# Runtime/timestamp fields vary every run; normalize them so the live payload
|
||||
# can be compared against (or regenerated into) a byte-stable committed fixture.
|
||||
_VOLATILE_TS_KEYS = {"received_at", "started_at", "last_tool_ts", "updated_at"}
|
||||
@@ -47,7 +46,7 @@ def test_status_contract(settings: Settings) -> None:
|
||||
with TestClient(app) as client:
|
||||
# Seed AFTER startup:
|
||||
db = get_database(settings.sqlite_path)
|
||||
|
||||
|
||||
# 1. A running issue with live detail:
|
||||
db.upsert_issue(
|
||||
key="octo/widget#1",
|
||||
@@ -169,15 +168,31 @@ def test_status_contract(settings: Settings) -> None:
|
||||
data = resp.json()
|
||||
|
||||
# Assert Python-side: top-level keys exactly:
|
||||
expected_keys = {"runtime", "event_counts", "issue_event_counts", "running_events", "inflight", "issues", "recent_events"}
|
||||
expected_keys = {
|
||||
"runtime",
|
||||
"event_counts",
|
||||
"issue_event_counts",
|
||||
"running_events",
|
||||
"inflight",
|
||||
"issues",
|
||||
"recent_events",
|
||||
}
|
||||
assert set(data.keys()) == expected_keys
|
||||
|
||||
# check running_events[0] keys:
|
||||
running_ev = data["running_events"]
|
||||
assert len(running_ev) == 1
|
||||
assert set(running_ev[0].keys()) == {
|
||||
"delivery_id", "event_type", "repo", "issue_key", "received_at",
|
||||
"started_at", "attempts", "model", "last_tool", "last_tool_ts"
|
||||
"delivery_id",
|
||||
"event_type",
|
||||
"repo",
|
||||
"issue_key",
|
||||
"received_at",
|
||||
"started_at",
|
||||
"attempts",
|
||||
"model",
|
||||
"last_tool",
|
||||
"last_tool_ts",
|
||||
}
|
||||
assert running_ev[0]["model"] == "anthropic/claude-3-5-sonnet"
|
||||
assert running_ev[0]["last_tool"] == "edit"
|
||||
@@ -185,13 +200,25 @@ def test_status_contract(settings: Settings) -> None:
|
||||
# check issues keys / latest_event keys:
|
||||
for issue_row in data["issues"]:
|
||||
assert set(issue_row.keys()) == {
|
||||
"key", "repo", "number", "branch", "pr_number",
|
||||
"state", "classification", "updated_at", "latest_event",
|
||||
"key",
|
||||
"repo",
|
||||
"number",
|
||||
"branch",
|
||||
"pr_number",
|
||||
"state",
|
||||
"classification",
|
||||
"updated_at",
|
||||
"latest_event",
|
||||
}
|
||||
latest = issue_row["latest_event"]
|
||||
if latest is not None:
|
||||
assert set(latest.keys()) == {
|
||||
"delivery_id", "event_type", "state", "attempts", "received_at", "last_error"
|
||||
"delivery_id",
|
||||
"event_type",
|
||||
"state",
|
||||
"attempts",
|
||||
"received_at",
|
||||
"last_error",
|
||||
}
|
||||
|
||||
# check runtime:
|
||||
@@ -358,4 +385,3 @@ def test_retry_state_transition(env, monkeypatch: pytest.MonkeyPatch) -> None:
|
||||
evt = db.get_event("failed-retry-1")
|
||||
assert evt is not None
|
||||
assert evt.state == "queued"
|
||||
|
||||
|
||||
@@ -5,6 +5,7 @@ from __future__ import annotations
|
||||
from types import SimpleNamespace
|
||||
|
||||
import pytest
|
||||
|
||||
from robomp import tasks
|
||||
from robomp.github_client import IssueInfo, RepoInfo
|
||||
from robomp.tasks import _attach_thread, _directive_from_payload
|
||||
@@ -91,7 +92,6 @@ async def test_attach_thread_preserves_authorizes_impl(monkeypatch: pytest.Monke
|
||||
assert hydrated.authorizes_impl is True
|
||||
|
||||
|
||||
|
||||
def _payload_with_directive(*, issue_number: int, body: str = "@robomp-bot ship it") -> dict[str, object]:
|
||||
return {
|
||||
"repository": {
|
||||
@@ -264,4 +264,4 @@ async def test_handle_pr_conversation_preserves_authorizes_impl_to_run_task(
|
||||
"task_kind": "handle_comment",
|
||||
"pr_number": 7,
|
||||
"run_task_authorizes_impl": True,
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user