Compare commits
6 Commits
fix/taskmd
...
fix/qg-pyt
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
90c9ffe839 | ||
| b6aa107f93 | |||
|
|
0b8013cb06 | ||
| b01643fcc3 | |||
|
|
ca63bc26bb | ||
| dce9ac806b |
@@ -22,6 +22,7 @@ class Settings(BaseSettings):
|
|||||||
|
|
||||||
# Gitea
|
# Gitea
|
||||||
gitea_url: str = "http://localhost:3000"
|
gitea_url: str = "http://localhost:3000"
|
||||||
|
gitea_public_url: str = "" # external URL for clickable links in comments; falls back to gitea_url
|
||||||
gitea_token: str = ""
|
gitea_token: str = ""
|
||||||
gitea_webhook_secret: str = ""
|
gitea_webhook_secret: str = ""
|
||||||
gitea_owner: str = "admin"
|
gitea_owner: str = "admin"
|
||||||
|
|||||||
@@ -252,6 +252,11 @@ def check_tests_local(repo: str, branch: str) -> tuple[bool, str]:
|
|||||||
S-1 fix: run the project test suite locally and judge by exit code, instead of
|
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).
|
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
|
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.
|
is safe for concurrent active tasks — no shared /repos checkout race.
|
||||||
"""
|
"""
|
||||||
@@ -259,7 +264,8 @@ def check_tests_local(repo: str, branch: str) -> tuple[bool, str]:
|
|||||||
try:
|
try:
|
||||||
repo_path = ensure_worktree(repo, branch)
|
repo_path = ensure_worktree(repo, branch)
|
||||||
r = subprocess.run(
|
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,
|
capture_output=True, text=True, timeout=600,
|
||||||
)
|
)
|
||||||
if r.returncode == 0:
|
if r.returncode == 0:
|
||||||
|
|||||||
@@ -189,36 +189,48 @@ def advance_stage(
|
|||||||
|
|
||||||
# --- Quality gate ----------------------------------------------------
|
# --- Quality gate ----------------------------------------------------
|
||||||
if qg_name and qg_name in QG_CHECKS:
|
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":
|
if qg_name == "check_analysis_approved":
|
||||||
_handle_analysis_approved_flow(
|
# Launcher path (analyst just finished): set In Review + ask for
|
||||||
task_id, current_stage, repo, work_item_id, branch, agent, result
|
# the Approved status. This gate never advances on its own -- a
|
||||||
)
|
# human Approved verdict does that.
|
||||||
return result
|
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)
|
if not passed:
|
||||||
result.qg_passed = passed
|
logger.info(
|
||||||
result.qg_reason = reason
|
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:
|
_handle_qg_failure_rollbacks(
|
||||||
logger.info(
|
task_id, current_stage, repo, work_item_id, branch,
|
||||||
f"Task {task_id}: QG '{qg_name}' not passed after {agent}: {reason}"
|
agent, qg_name, reason, result,
|
||||||
)
|
)
|
||||||
# Behaviour parity:
|
return result
|
||||||
# - 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
|
|
||||||
|
|
||||||
elif qg_name:
|
elif qg_name:
|
||||||
# QG name set but not registered — do not advance (launcher behavior).
|
# QG name set but not registered — do not advance (launcher behavior).
|
||||||
@@ -296,7 +308,7 @@ def _build_analyst_ready_comment(repo: str, work_item_id: str, branch: str) -> s
|
|||||||
wt_dir = None
|
wt_dir = None
|
||||||
|
|
||||||
owner = getattr(settings, "gitea_owner", "admin")
|
owner = getattr(settings, "gitea_owner", "admin")
|
||||||
base = settings.gitea_url.rstrip("/")
|
base = (getattr(settings, "gitea_public_url", "") or settings.gitea_url).rstrip("/")
|
||||||
links = []
|
links = []
|
||||||
for label, fname in candidates:
|
for label, fname in candidates:
|
||||||
if wt_dir and not os.path.isfile(os.path.join(wt_dir, fname)):
|
if wt_dir and not os.path.isfile(os.path.join(wt_dir, fname)):
|
||||||
|
|||||||
@@ -25,7 +25,9 @@ def test_analyst_comment_asks_approved_with_links(monkeypatch, tmp_path):
|
|||||||
# 04b-ui-test-cases.md intentionally absent -> must NOT be linked
|
# 04b-ui-test-cases.md intentionally absent -> must NOT be linked
|
||||||
|
|
||||||
monkeypatch.setattr(SE, "get_worktree_path", lambda repo, branch: str(wt))
|
monkeypatch.setattr(SE, "get_worktree_path", lambda repo, branch: str(wt))
|
||||||
monkeypatch.setattr(SE.settings, "gitea_url", "https://git.mva154.duckdns.org")
|
# public URL set -> links must be built from it (not gitea_url)
|
||||||
|
monkeypatch.setattr(SE.settings, "gitea_url", "http://localhost:3000")
|
||||||
|
monkeypatch.setattr(SE.settings, "gitea_public_url", "https://git.mva154.duckdns.org")
|
||||||
monkeypatch.setattr(SE.settings, "gitea_owner", "admin")
|
monkeypatch.setattr(SE.settings, "gitea_owner", "admin")
|
||||||
|
|
||||||
html = SE._build_analyst_ready_comment(
|
html = SE._build_analyst_ready_comment(
|
||||||
@@ -45,3 +47,28 @@ def test_analyst_comment_asks_approved_with_links(monkeypatch, tmp_path):
|
|||||||
assert base + "04-test-plan.yaml" in html
|
assert base + "04-test-plan.yaml" in html
|
||||||
# the missing file is NOT invented
|
# the missing file is NOT invented
|
||||||
assert "04b-ui-test-cases.md" not in html
|
assert "04b-ui-test-cases.md" not in html
|
||||||
|
# internal git url must NOT appear in clickable links
|
||||||
|
assert "localhost:3000" not in html
|
||||||
|
|
||||||
|
|
||||||
|
def test_analyst_comment_falls_back_to_gitea_url(monkeypatch, tmp_path):
|
||||||
|
"""When gitea_public_url is empty, links fall back to gitea_url."""
|
||||||
|
from src import stage_engine as SE
|
||||||
|
|
||||||
|
wt = tmp_path / "wt"
|
||||||
|
docs = wt / "docs" / "work-items" / "ET-011"
|
||||||
|
docs.mkdir(parents=True)
|
||||||
|
(docs / "01-brd.md").write_text("x")
|
||||||
|
|
||||||
|
monkeypatch.setattr(SE, "get_worktree_path", lambda repo, branch: str(wt))
|
||||||
|
monkeypatch.setattr(SE.settings, "gitea_url", "http://localhost:3000")
|
||||||
|
monkeypatch.setattr(SE.settings, "gitea_public_url", "")
|
||||||
|
monkeypatch.setattr(SE.settings, "gitea_owner", "admin")
|
||||||
|
|
||||||
|
html = SE._build_analyst_ready_comment(
|
||||||
|
"enduro-trails", "ET-011", "feature/ET-011-gpx-upload-feature"
|
||||||
|
)
|
||||||
|
|
||||||
|
base = ("http://localhost:3000/admin/enduro-trails/src/branch/"
|
||||||
|
"feature/ET-011-gpx-upload-feature/docs/work-items/ET-011/")
|
||||||
|
assert base + "01-brd.md" in html
|
||||||
|
|||||||
@@ -17,6 +17,7 @@ from src.qg.checks import (
|
|||||||
check_ci_green,
|
check_ci_green,
|
||||||
check_review_approved,
|
check_review_approved,
|
||||||
check_tests_passed,
|
check_tests_passed,
|
||||||
|
check_tests_local,
|
||||||
)
|
)
|
||||||
|
|
||||||
|
|
||||||
@@ -186,3 +187,41 @@ class TestCheckTestsPassed:
|
|||||||
passed, reason = check_tests_passed("enduro-trails", "ET-001")
|
passed, reason = check_tests_passed("enduro-trails", "ET-001")
|
||||||
assert passed is False
|
assert passed is False
|
||||||
assert "not found" in reason.lower()
|
assert "not found" in reason.lower()
|
||||||
|
|
||||||
|
|
||||||
|
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")
|
||||||
|
|
||||||
|
|||||||
@@ -358,6 +358,63 @@ class TestAnalysisApprovedFlow:
|
|||||||
assert stage_engine.notify_approve_requested.called
|
assert stage_engine.notify_approve_requested.called
|
||||||
assert _jobs() == []
|
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
|
# launcher + plane both delegate to the engine
|
||||||
|
|||||||
Reference in New Issue
Block a user