Compare commits
6 Commits
fix/gitea-
...
fix/drop-l
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
e15d339b14 | ||
| 994f73a78e | |||
|
|
90c9ffe839 | ||
| b6aa107f93 | |||
|
|
0b8013cb06 | ||
| b01643fcc3 |
@@ -249,9 +249,17 @@ def check_reviewer_verdict(repo: str, work_item_id: str, branch: str | None = No
|
||||
|
||||
def check_tests_local(repo: str, branch: str) -> tuple[bool, str]:
|
||||
"""
|
||||
DEPRECATED: replaced by check_ci_green on the development stage (CI is now
|
||||
configured). Kept for backward-compat; not wired to any stage.
|
||||
|
||||
S-1 fix: run the project test suite locally and judge by exit code, instead of
|
||||
depending on Gitea CI (which is not configured -> always false).
|
||||
|
||||
БАГ 5 fix: invoke pytest directly instead of make test. make is not installed
|
||||
in the orchestrator container, so the previous ["make", "test"] call raised
|
||||
FileNotFoundError. This reproduces the Makefile test target 1:1
|
||||
(cd src/api && python -m pytest ../../tests/ -v).
|
||||
|
||||
ORCH-2 / S-4: tests run inside the per-branch worktree (ensure_worktree), so this
|
||||
is safe for concurrent active tasks — no shared /repos checkout race.
|
||||
"""
|
||||
@@ -259,7 +267,8 @@ def check_tests_local(repo: str, branch: str) -> tuple[bool, str]:
|
||||
try:
|
||||
repo_path = ensure_worktree(repo, branch)
|
||||
r = subprocess.run(
|
||||
["make", "test"], cwd=repo_path,
|
||||
["python", "-m", "pytest", "../../tests/", "-v"],
|
||||
cwd=os.path.join(repo_path, "src", "api"),
|
||||
capture_output=True, text=True, timeout=600,
|
||||
)
|
||||
if r.returncode == 0:
|
||||
|
||||
@@ -189,36 +189,48 @@ def advance_stage(
|
||||
|
||||
# --- Quality gate ----------------------------------------------------
|
||||
if qg_name and qg_name in QG_CHECKS:
|
||||
# Human-approval gate: special analyst approved-flow (launcher only).
|
||||
# Human-approval gate: split by path.
|
||||
if qg_name == "check_analysis_approved":
|
||||
_handle_analysis_approved_flow(
|
||||
task_id, current_stage, repo, work_item_id, branch, agent, result
|
||||
)
|
||||
return result
|
||||
# Launcher path (analyst just finished): set In Review + ask for
|
||||
# the Approved status. This gate never advances on its own -- a
|
||||
# human Approved verdict does that.
|
||||
if agent == "analyst":
|
||||
_handle_analysis_approved_flow(
|
||||
task_id, current_stage, repo, work_item_id, branch, agent, result
|
||||
)
|
||||
return result
|
||||
# Webhook Approved-verdict path (agent is None): the human flipped
|
||||
# the Plane status to Approved, which IS the approval. The gate is
|
||||
# satisfied -- do NOT re-run check_analysis_approved (it looks for
|
||||
# an :approved: *comment* and would block on a status-only
|
||||
# approval). Mark it passed and fall through to the Advance block.
|
||||
result.qg_name = qg_name
|
||||
result.qg_passed = True
|
||||
result.qg_reason = "approved-via-status"
|
||||
else:
|
||||
passed, reason = _run_qg(qg_name, repo, work_item_id, branch)
|
||||
result.qg_passed = passed
|
||||
result.qg_reason = reason
|
||||
|
||||
passed, reason = _run_qg(qg_name, repo, work_item_id, branch)
|
||||
result.qg_passed = passed
|
||||
result.qg_reason = reason
|
||||
if not passed:
|
||||
logger.info(
|
||||
f"Task {task_id}: QG '{qg_name}' not passed after {agent}: {reason}"
|
||||
)
|
||||
# Behaviour parity:
|
||||
# - webhook path (finished_agent is None): emit the generic
|
||||
# QG-failure notification, exactly like the old plane handler.
|
||||
# - launcher path (finished_agent set): NO generic notification;
|
||||
# the rollback branches below own their own messaging, exactly
|
||||
# like the old launcher handler.
|
||||
if agent is None:
|
||||
notify_qg_failure(task_id, current_stage, qg_name, reason)
|
||||
plane_notify_qg(work_item_id, current_stage, qg_name, reason)
|
||||
|
||||
if not passed:
|
||||
logger.info(
|
||||
f"Task {task_id}: QG '{qg_name}' not passed after {agent}: {reason}"
|
||||
)
|
||||
# Behaviour parity:
|
||||
# - webhook path (finished_agent is None): emit the generic
|
||||
# QG-failure notification, exactly like the old plane handler.
|
||||
# - launcher path (finished_agent set): NO generic notification;
|
||||
# the rollback branches below own their own messaging, exactly
|
||||
# like the old launcher handler.
|
||||
if agent is None:
|
||||
notify_qg_failure(task_id, current_stage, qg_name, reason)
|
||||
plane_notify_qg(work_item_id, current_stage, qg_name, reason)
|
||||
|
||||
_handle_qg_failure_rollbacks(
|
||||
task_id, current_stage, repo, work_item_id, branch,
|
||||
agent, qg_name, reason, result,
|
||||
)
|
||||
return result
|
||||
_handle_qg_failure_rollbacks(
|
||||
task_id, current_stage, repo, work_item_id, branch,
|
||||
agent, qg_name, reason, result,
|
||||
)
|
||||
return result
|
||||
|
||||
elif qg_name:
|
||||
# QG name set but not registered — do not advance (launcher behavior).
|
||||
|
||||
@@ -13,7 +13,7 @@ STAGE_TRANSITIONS = {
|
||||
"created": {"next": "analysis", "agent": "analyst", "qg": None},
|
||||
"analysis": {"next": "architecture", "agent": "architect", "qg": "check_analysis_approved"},
|
||||
"architecture": {"next": "development", "agent": "developer", "qg": "check_architecture_done"},
|
||||
"development": {"next": "review", "agent": "reviewer", "qg": "check_tests_local"},
|
||||
"development": {"next": "review", "agent": "reviewer", "qg": "check_ci_green"},
|
||||
"review": {"next": "testing", "agent": "tester", "qg": "check_reviewer_verdict"},
|
||||
"testing": {"next": "deploy", "agent": "deployer", "qg": "check_tests_passed"},
|
||||
"deploy": {"next": "done", "agent": None, "qg": None},
|
||||
|
||||
@@ -216,12 +216,10 @@ async def handle_ci_status(payload: dict):
|
||||
else:
|
||||
notify_qg_failure(task_id, current_stage, "check_ci_green", reason)
|
||||
|
||||
elif state == "failure":
|
||||
# S-1: Gitea CI is NOT the authoritative gate anymore (the orchestrator runs
|
||||
# tests locally via check_tests_local). Gitea CI is often unconfigured, so a
|
||||
# "failure"/empty status here is not actionable. Log only, do not alert.
|
||||
logger.debug(f"Task {task_id}: Gitea CI state='failure' on branch '{branch}' "
|
||||
f"(non-authoritative, suppressed — local tests are the gate)")
|
||||
elif state == "failure" and current_stage == "development":
|
||||
# CI is now the authoritative gate for development -> review.
|
||||
# A failing CI means the QG did not pass; notify (do not silently advance).
|
||||
notify_qg_failure(task_id, current_stage, "check_ci_green", f"Gitea CI failed on branch '{branch}'")
|
||||
|
||||
|
||||
async def handle_pr(payload: dict):
|
||||
|
||||
@@ -17,7 +17,9 @@ from src.qg.checks import (
|
||||
check_ci_green,
|
||||
check_review_approved,
|
||||
check_tests_passed,
|
||||
check_tests_local,
|
||||
)
|
||||
from src.stages import get_qg_for_stage
|
||||
|
||||
|
||||
@pytest.fixture(autouse=True)
|
||||
@@ -186,3 +188,57 @@ class TestCheckTestsPassed:
|
||||
passed, reason = check_tests_passed("enduro-trails", "ET-001")
|
||||
assert passed is False
|
||||
assert "not found" in reason.lower()
|
||||
|
||||
|
||||
class TestDevelopmentStageQG:
|
||||
"""BUG 6: development stage QG is now check_ci_green (CI is the authoritative
|
||||
gate), not the deprecated check_tests_local."""
|
||||
|
||||
def test_development_qg_is_check_ci_green(self):
|
||||
assert get_qg_for_stage("development") == "check_ci_green"
|
||||
|
||||
def test_check_tests_local_is_deprecated_and_unwired(self):
|
||||
# Kept in the registry for backward-compat, but not wired to any stage.
|
||||
from src.qg.checks import QG_CHECKS
|
||||
from src.stages import STAGE_TRANSITIONS
|
||||
assert "check_tests_local" in QG_CHECKS
|
||||
wired = {t.get("qg") for t in STAGE_TRANSITIONS.values()}
|
||||
assert "check_tests_local" not in wired
|
||||
|
||||
|
||||
class TestCheckTestsLocal:
|
||||
"""BUG 5: check_tests_local must run pytest directly (not make, which is
|
||||
not installed in the orchestrator container)."""
|
||||
|
||||
@patch("src.qg.checks.ensure_worktree")
|
||||
@patch("subprocess.run")
|
||||
def test_passes_on_returncode_zero(self, mock_run, mock_wt, tmp_path):
|
||||
mock_wt.return_value = str(tmp_path)
|
||||
mock_run.return_value = MagicMock(returncode=0, stdout="ok", stderr="")
|
||||
passed, reason = check_tests_local("enduro-trails", "feature/ET-001-x")
|
||||
assert passed is True
|
||||
assert reason == "Local tests passed"
|
||||
|
||||
@patch("src.qg.checks.ensure_worktree")
|
||||
@patch("subprocess.run")
|
||||
def test_fails_on_nonzero_returncode(self, mock_run, mock_wt, tmp_path):
|
||||
mock_wt.return_value = str(tmp_path)
|
||||
mock_run.return_value = MagicMock(returncode=1, stdout="boom", stderr="trace")
|
||||
passed, reason = check_tests_local("enduro-trails", "feature/ET-001-x")
|
||||
assert passed is False
|
||||
assert "Local tests failed" in reason
|
||||
|
||||
@patch("src.qg.checks.ensure_worktree")
|
||||
@patch("subprocess.run")
|
||||
def test_invokes_pytest_not_make(self, mock_run, mock_wt, tmp_path):
|
||||
"""The subprocess call must be pytest, from src/api, against ../../tests/."""
|
||||
mock_wt.return_value = str(tmp_path)
|
||||
mock_run.return_value = MagicMock(returncode=0, stdout="", stderr="")
|
||||
check_tests_local("enduro-trails", "feature/ET-001-x")
|
||||
args, kwargs = mock_run.call_args
|
||||
cmd = args[0]
|
||||
assert "make" not in cmd
|
||||
assert cmd[:3] == ["python", "-m", "pytest"]
|
||||
assert "../../tests/" in cmd
|
||||
assert kwargs["cwd"] == os.path.join(str(tmp_path), "src", "api")
|
||||
|
||||
|
||||
@@ -203,10 +203,13 @@ class TestQgFailureDoesNotAdvance:
|
||||
assert _jobs() == []
|
||||
|
||||
def test_webhook_path_emits_qg_failure_notification(self, monkeypatch):
|
||||
"""finished_agent=None -> generic QG-failure notification fires (plane parity)."""
|
||||
"""finished_agent=None -> generic QG-failure notification fires (plane parity).
|
||||
|
||||
development stage QG is now check_ci_green (was check_tests_local).
|
||||
"""
|
||||
monkeypatch.setattr(
|
||||
stage_engine, "QG_CHECKS",
|
||||
{**stage_engine.QG_CHECKS, "check_tests_local": _fail("ci red")},
|
||||
{**stage_engine.QG_CHECKS, "check_ci_green": _fail("ci red")},
|
||||
)
|
||||
task_id = _make_task("development")
|
||||
advance_stage(task_id, "development", "enduro-trails", "ET-001",
|
||||
@@ -358,6 +361,63 @@ class TestAnalysisApprovedFlow:
|
||||
assert stage_engine.notify_approve_requested.called
|
||||
assert _jobs() == []
|
||||
|
||||
def test_approved_verdict_advances_analysis_to_architecture(self, monkeypatch):
|
||||
"""BUG 4: a human Approved STATUS (webhook path, finished_agent=None)
|
||||
must satisfy the analysis gate and advance analysis -> architecture,
|
||||
enqueuing the architect. The status-only approval must NOT re-run
|
||||
check_analysis_approved (which looks for an :approved: COMMENT and would
|
||||
otherwise wrongly block the advance).
|
||||
"""
|
||||
# Make check_analysis_approved FAIL if it is ever called: the webhook
|
||||
# path must bypass it entirely (status == approval). If the engine were
|
||||
# to re-run the gate, this would block the advance and fail the test.
|
||||
monkeypatch.setattr(
|
||||
stage_engine, "QG_CHECKS",
|
||||
{
|
||||
**stage_engine.QG_CHECKS,
|
||||
"check_analysis_approved": _fail("no :approved: comment"),
|
||||
},
|
||||
)
|
||||
# Guard: the approval-flow (launcher-only) must NOT be invoked here.
|
||||
flow = MagicMock()
|
||||
monkeypatch.setattr(stage_engine, "_handle_analysis_approved_flow", flow)
|
||||
|
||||
task_id = _make_task("analysis")
|
||||
res = advance_stage(
|
||||
task_id, "analysis", "enduro-trails", "ET-001",
|
||||
"feature/ET-001-x", finished_agent=None,
|
||||
)
|
||||
|
||||
assert res.advanced is True
|
||||
assert res.to_stage == "architecture"
|
||||
assert _stage(task_id) == "architecture"
|
||||
assert res.enqueued_agent == "architect"
|
||||
# Sanity: agent for analysis is architect, never analyst (no re-run loop).
|
||||
assert get_agent_for_stage("analysis") == "architect"
|
||||
jobs = _jobs()
|
||||
assert len(jobs) == 1
|
||||
assert jobs[0]["agent"] == "architect"
|
||||
# The launcher-only approval-flow was NOT called on the webhook path.
|
||||
flow.assert_not_called()
|
||||
|
||||
def test_launcher_path_does_not_advance_and_calls_flow(self, monkeypatch):
|
||||
"""Regression: the launcher path (finished_agent='analyst') still routes
|
||||
into _handle_analysis_approved_flow and does NOT advance.
|
||||
"""
|
||||
flow = MagicMock()
|
||||
monkeypatch.setattr(stage_engine, "_handle_analysis_approved_flow", flow)
|
||||
|
||||
task_id = _make_task("analysis")
|
||||
res = advance_stage(
|
||||
task_id, "analysis", "enduro-trails", "ET-001",
|
||||
"feature/ET-001-x", finished_agent="analyst",
|
||||
)
|
||||
|
||||
assert res.advanced is not True
|
||||
assert _stage(task_id) == "analysis"
|
||||
assert _jobs() == []
|
||||
flow.assert_called_once()
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# launcher + plane both delegate to the engine
|
||||
|
||||
@@ -272,6 +272,46 @@ def test_gitea_ci_success_advances_to_review(mock_launcher, mock_ci):
|
||||
assert task["stage"] == "review"
|
||||
|
||||
|
||||
@patch("src.webhooks.gitea.notify_qg_failure")
|
||||
@patch("src.webhooks.gitea.launcher")
|
||||
def test_gitea_ci_failure_on_development_notifies_qg_failure(mock_launcher, mock_notify):
|
||||
"""BUG 6: CI failure at development is now the authoritative QG gate failing.
|
||||
|
||||
It must notify QG failure (not silently suppress) and must NOT advance the stage.
|
||||
"""
|
||||
conn = get_db()
|
||||
conn.execute(
|
||||
"INSERT INTO tasks (plane_id, work_item_id, repo, branch, stage) VALUES (?, ?, ?, ?, ?)",
|
||||
("ci-fail-001", "ET-011", "enduro-trails", "feature/ET-011-test", "development"),
|
||||
)
|
||||
conn.commit()
|
||||
conn.close()
|
||||
|
||||
resp = client.post(
|
||||
"/webhook/gitea",
|
||||
json={
|
||||
"state": "failure",
|
||||
"branches": [{"name": "feature/ET-011-test"}],
|
||||
"repository": {"name": "enduro-trails"},
|
||||
},
|
||||
headers={"X-Gitea-Event": "status"},
|
||||
)
|
||||
assert resp.status_code == 200
|
||||
|
||||
# QG failure was reported for the development stage with check_ci_green.
|
||||
assert mock_notify.called
|
||||
args, kwargs = mock_notify.call_args
|
||||
call = list(args) + list(kwargs.values())
|
||||
assert "development" in call
|
||||
assert "check_ci_green" in call
|
||||
|
||||
# Stage did NOT advance.
|
||||
conn = get_db()
|
||||
task = conn.execute("SELECT * FROM tasks WHERE plane_id = 'ci-fail-001'").fetchone()
|
||||
conn.close()
|
||||
assert task["stage"] == "development"
|
||||
|
||||
|
||||
def test_gitea_webhook_pr():
|
||||
"""PR event is accepted."""
|
||||
resp = client.post(
|
||||
|
||||
Reference in New Issue
Block a user