From 38a741d24e9998d0f7d61e81754095eb0d66622e Mon Sep 17 00:00:00 2001 From: Dev Agent Date: Wed, 3 Jun 2026 18:18:36 +0300 Subject: [PATCH] feat(webhook): verdict via Approved/Rejected statuses (variant B) Feature 2. The issue updated dispatch (shipped with the status-trigger handler) also routes Approved -> _try_advance_stage (== :approved: comment) and Rejected -> _rollback_stage (== :rejected: comment). The :rejected: comment branch was refactored into the shared _rollback_stage so both mechanisms behave identically; a status reject passes Reason: (rejected via status, see latest comment) since no inline reason arrives with a status change. Comments stay fully working. This commit adds test_verdict_status.py proving both status and comment paths funnel into the same advance/rollback logic. --- tests/test_verdict_status.py | 140 +++++++++++++++++++++++++++++++++++ 1 file changed, 140 insertions(+) create mode 100644 tests/test_verdict_status.py diff --git a/tests/test_verdict_status.py b/tests/test_verdict_status.py new file mode 100644 index 0000000..4202d33 --- /dev/null +++ b/tests/test_verdict_status.py @@ -0,0 +1,140 @@ +"""Feature 2 (variant B): verdict statuses Approved / Rejected. + + * issue updated -> Approved : calls _try_advance_stage (== :approved: comment). + * issue updated -> Rejected : calls _rollback_stage (== :rejected: comment). + * the :approved: / :rejected: COMMENT mechanisms still work (both paths live). + +We mock the shared engine entry points (_try_advance_stage / _rollback_stage) +and assert they fire for both the status and the comment trigger, so the two +mechanisms are proven to funnel into the same logic. +""" + +import os +import tempfile + +_test_db = os.path.join(tempfile.gettempdir(), "test_orchestrator_verdict.db") +os.environ["ORCH_DB_PATH"] = _test_db +os.environ.setdefault("ORCH_PLANE_WEBHOOK_SECRET", "") +os.environ.setdefault("ORCH_GITEA_TOKEN", "test-token") +os.environ.setdefault("ORCH_PLANE_API_TOKEN", "test-token") + +import pytest # noqa: E402 +from unittest.mock import patch, AsyncMock # noqa: E402 +from fastapi.testclient import TestClient # noqa: E402 + +from src.main import app # noqa: E402 +from src.db import init_db, get_db # noqa: E402 +from src import projects as P # noqa: E402 +from src.projects import reload_projects # noqa: E402 + +ENDURO_PLANE_ID = "7a79f0a9-5278-49cd-9007-9a338f238f9c" +APPROVED = "a519a341-dada-4a91-8910-7604f82b79c5" +REJECTED = "ba958f3c-5db5-461d-8f82-89425e413b97" + +client = TestClient(app) + + +@pytest.fixture(autouse=True) +def setup(monkeypatch): + monkeypatch.setattr(P.settings, "db_path", _test_db) + import src.db as _db + monkeypatch.setattr(_db.settings, "db_path", _test_db) + if os.path.exists(_test_db): + os.unlink(_test_db) + init_db() + monkeypatch.setattr("src.webhooks.plane.verify_plane_signature", lambda body, sig: True) + registry_json = ( + f'[{{"plane_project_id": "{ENDURO_PLANE_ID}", "repo": "enduro-trails",' + f' "work_item_prefix": "ET", "name": "enduro-trails"}}]' + ) + monkeypatch.setattr(P.settings, "projects_json", registry_json) + reload_projects() + # Seed a task at the 'review' stage for plane_id 'v-1'. + conn = get_db() + conn.execute( + "INSERT INTO tasks (plane_id, work_item_id, repo, branch, stage, plane_issue_id) " + "VALUES (?, ?, ?, ?, ?, ?)", + ("v-1", "ET-500", "enduro-trails", "feature/ET-500-x", "review", "v-1"), + ) + conn.commit() + conn.close() + yield + reload_projects() + if os.path.exists(_test_db): + os.unlink(_test_db) + + +def _status(state_id, plane_id="v-1", old="prev"): + return client.post("/webhook/plane", json={ + "event": "issue", "action": "updated", + "data": { + "id": plane_id, "name": "Verdict task", "project": ENDURO_PLANE_ID, + "state": {"id": state_id, "name": "X", "group": "started"}, + }, + "activity": {"field": "state", "new_value": state_id, "old_value": old}, + }) + + +def _comment(text, plane_id="v-1"): + return client.post("/webhook/plane", json={ + "event": "issue_comment", "action": "created", + "data": {"work_item_id": plane_id, "comment_stripped": text, + "project": ENDURO_PLANE_ID}, + }) + + +# --------------------------------------------------------------------------- # +# Approved status -> advance +# --------------------------------------------------------------------------- # +@patch("src.plane_sync.set_issue_in_progress") +@patch("src.webhooks.plane._try_advance_stage", new_callable=AsyncMock) +def test_approved_status_advances(mock_advance, mock_sip): + resp = _status(APPROVED) + assert resp.status_code == 200 + mock_advance.assert_awaited_once() + # advanced the right task (ET-500 at review) + args = mock_advance.call_args.args + assert "ET-500" in args # work_item_id is passed positionally + + +@patch("src.plane_sync.set_issue_in_progress") +@patch("src.webhooks.plane._try_advance_stage", new_callable=AsyncMock) +def test_approved_comment_still_advances(mock_advance, mock_sip): + resp = _comment(":approved:") + assert resp.status_code == 200 + mock_advance.assert_awaited_once() + + +# --------------------------------------------------------------------------- # +# Rejected status -> rollback +# --------------------------------------------------------------------------- # +@patch("src.webhooks.plane._rollback_stage", new_callable=AsyncMock) +def test_rejected_status_rolls_back(mock_rollback): + resp = _status(REJECTED) + assert resp.status_code == 200 + mock_rollback.assert_awaited_once() + # reason note for a status reject (no inline reason available) + kwargs_reason = mock_rollback.call_args.args[-1] + assert "rejected via status" in kwargs_reason + + +@patch("src.webhooks.plane._rollback_stage", new_callable=AsyncMock) +def test_rejected_comment_still_rolls_back(mock_rollback): + resp = _comment(":rejected: bad ADR") + assert resp.status_code == 200 + mock_rollback.assert_awaited_once() + reason = mock_rollback.call_args.args[-1] + assert "bad ADR" in reason + + +# --------------------------------------------------------------------------- # +# Unknown verdict status -> no-op +# --------------------------------------------------------------------------- # +@patch("src.webhooks.plane._rollback_stage", new_callable=AsyncMock) +@patch("src.webhooks.plane._try_advance_stage", new_callable=AsyncMock) +def test_other_status_no_verdict_action(mock_advance, mock_rollback): + # In Review status is not a verdict -> neither advance nor rollback. + resp = _status("38fb1f64-aa1e-48a3-92e0-0b109679046b") # in_review + assert resp.status_code == 200 + mock_advance.assert_not_called() + mock_rollback.assert_not_called()