857bad314c
handle_verdict(rejected): the reason is now pulled from the issue latest Plane comment (_latest_comment_reason: GET comments, newest by created_at, HTML stripped) instead of a fixed stub. Slava writes the reason in a comment before flipping the status to Rejected. Falls back to a fixed note when there is no comment / the API call fails. tests: add test_status_only_verdict.py (test_inreview_comment_does_not_revert [bug 3 root], test_any_comment_no_pipeline_action, test_approved_status_advances_without_inprogress_reset, test_rejected_status_pulls_reason_from_comment) and test_inprogress_from_needs_input_relaunches_analyst in test_status_trigger.py. Rewrote the comment-based tests (test_verdict_status, test_plane_approved/ rejected in test_webhooks) under the status-only model: comments are no-ops, verdicts come from status changes.
172 lines
6.6 KiB
Python
172 lines
6.6 KiB
Python
"""Status-only verdict model: verdict statuses Approved / Rejected.
|
|
|
|
* issue updated -> Approved : calls _try_advance_stage, with NO intermediate
|
|
set_issue_in_progress reset (bug 3 fix).
|
|
* issue updated -> Rejected : calls _rollback_stage, with the reason pulled
|
|
from the issue's latest comment.
|
|
* COMMENTS NEVER trigger the pipeline: a :approved: / :rejected: comment is a
|
|
pure no-op (the comment-based control mechanism was removed).
|
|
|
|
We mock the shared engine entry points (_try_advance_stage / _rollback_stage)
|
|
and assert they fire ONLY for the status trigger, never for a comment.
|
|
"""
|
|
|
|
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},
|
|
})
|
|
|
|
|
|
class _FakeResp:
|
|
def __init__(self, status_code, payload):
|
|
self.status_code = status_code
|
|
self._payload = payload
|
|
|
|
def json(self):
|
|
return self._payload
|
|
|
|
|
|
def _comments_response(comments):
|
|
return _FakeResp(200, {"results": comments})
|
|
|
|
|
|
# --------------------------------------------------------------------------- #
|
|
# Approved status -> advance (no in_progress reset)
|
|
# --------------------------------------------------------------------------- #
|
|
@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
|
|
# bug 3 fix: handle_verdict no longer resets the status to In Progress.
|
|
mock_sip.assert_not_called()
|
|
|
|
|
|
@patch("src.plane_sync.set_issue_in_progress")
|
|
@patch("src.webhooks.plane._rollback_stage", new_callable=AsyncMock)
|
|
@patch("src.webhooks.plane._try_advance_stage", new_callable=AsyncMock)
|
|
def test_approved_comment_is_noop(mock_advance, mock_rollback, mock_sip):
|
|
"""Status-only model: a :approved: comment NEVER advances the pipeline."""
|
|
resp = _comment(":approved:")
|
|
assert resp.status_code == 200
|
|
mock_advance.assert_not_called()
|
|
mock_rollback.assert_not_called()
|
|
mock_sip.assert_not_called()
|
|
|
|
|
|
# --------------------------------------------------------------------------- #
|
|
# Rejected status -> rollback (reason from latest comment)
|
|
# --------------------------------------------------------------------------- #
|
|
@patch("src.webhooks.plane.httpx.get")
|
|
@patch("src.webhooks.plane._rollback_stage", new_callable=AsyncMock)
|
|
def test_rejected_status_rolls_back(mock_rollback, mock_get):
|
|
mock_get.return_value = _comments_response(
|
|
[{"comment_stripped": "ADR missing tradeoffs",
|
|
"created_at": "2026-06-03T10:00:00Z"}]
|
|
)
|
|
resp = _status(REJECTED)
|
|
assert resp.status_code == 200
|
|
mock_rollback.assert_awaited_once()
|
|
# reason pulled from the latest comment
|
|
reason = mock_rollback.call_args.args[-1]
|
|
assert "ADR missing tradeoffs" in reason
|
|
|
|
|
|
@patch("src.webhooks.plane.httpx.get")
|
|
@patch("src.plane_sync.set_issue_in_progress")
|
|
@patch("src.webhooks.plane._rollback_stage", new_callable=AsyncMock)
|
|
@patch("src.webhooks.plane._try_advance_stage", new_callable=AsyncMock)
|
|
def test_rejected_comment_is_noop(mock_advance, mock_rollback, mock_sip, mock_get):
|
|
"""Status-only model: a :rejected: comment NEVER rolls back the pipeline."""
|
|
resp = _comment(":rejected: bad ADR")
|
|
assert resp.status_code == 200
|
|
mock_advance.assert_not_called()
|
|
mock_rollback.assert_not_called()
|
|
mock_sip.assert_not_called()
|
|
mock_get.assert_not_called()
|
|
|
|
|
|
# --------------------------------------------------------------------------- #
|
|
# 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()
|