fix(qg): frontmatter-only reviewer verdict + local test gate (S-5, S-1)
- check_reviewer_verdict reads verdict: from YAML frontmatter of 12-review.md only - add check_tests_local: orchestrator runs make test in /repos/<repo> - stages: development QG -> check_tests_local
This commit is contained in:
@@ -189,9 +189,15 @@ def check_analysis_approved(repo: str, work_item_id: str) -> tuple[bool, str]:
|
||||
|
||||
def check_reviewer_verdict(repo: str, work_item_id: str) -> tuple[bool, str]:
|
||||
"""
|
||||
Check reviewer agent verdict from 12-review.md.
|
||||
Returns (True, reason) if APPROVED, (False, reason) if REQUEST_CHANGES or missing.
|
||||
Check reviewer agent verdict from 12-review.md (S-5 fix).
|
||||
|
||||
Reads ONLY the machine-readable `verdict:` field from the YAML frontmatter,
|
||||
so tables / prose that merely mention APPROVED or REQUEST_CHANGES no longer
|
||||
cause false positives/negatives. Returns:
|
||||
(True, ...) -> verdict: APPROVED
|
||||
(False, ...) -> verdict: REQUEST_CHANGES, missing verdict, or no frontmatter
|
||||
"""
|
||||
import yaml
|
||||
repo_path = os.path.join(settings.repos_dir, repo)
|
||||
review_path = os.path.join(repo_path, f"docs/work-items/{work_item_id}/12-review.md")
|
||||
|
||||
@@ -200,20 +206,63 @@ def check_reviewer_verdict(repo: str, work_item_id: str) -> tuple[bool, str]:
|
||||
|
||||
try:
|
||||
with open(review_path, "r") as f:
|
||||
content = f.read(5000)
|
||||
content = f.read()
|
||||
|
||||
content_upper = content.upper()
|
||||
if "REQUEST_CHANGES" in content_upper:
|
||||
return False, "Reviewer verdict: REQUEST_CHANGES"
|
||||
if "APPROVED" in content_upper:
|
||||
verdict = None
|
||||
if content.startswith("---"):
|
||||
parts = content.split("---", 2)
|
||||
if len(parts) >= 3:
|
||||
try:
|
||||
fm = yaml.safe_load(parts[1]) or {}
|
||||
except yaml.YAMLError as e:
|
||||
return False, f"Invalid YAML frontmatter in review: {e}"
|
||||
verdict = str(fm.get("verdict", "")).upper().strip()
|
||||
|
||||
if verdict == "APPROVED":
|
||||
return True, "Reviewer verdict: APPROVED"
|
||||
if "LGTM" in content_upper or "SHIP IT" in content_upper:
|
||||
return True, "Reviewer verdict: LGTM"
|
||||
return False, "Review exists but no clear APPROVED/REQUEST_CHANGES verdict"
|
||||
if verdict == "REQUEST_CHANGES":
|
||||
return False, "Reviewer verdict: REQUEST_CHANGES"
|
||||
return False, f"No machine-readable verdict in frontmatter (got: {verdict!r})"
|
||||
except OSError as e:
|
||||
return False, f"Error reading review: {e}"
|
||||
|
||||
|
||||
def check_tests_local(repo: str, branch: str) -> tuple[bool, str]:
|
||||
"""
|
||||
S-1 fix: run the project test suite locally in /repos/<repo> and judge by exit
|
||||
code, instead of depending on Gitea CI (which is not configured -> always false).
|
||||
|
||||
Checks out `branch` in the shared /repos checkout and runs `make test`.
|
||||
NOTE (known limitation): the shared /repos checkout means this is not safe for
|
||||
concurrent active tasks. git-worktree-per-task is a separate task (S-4).
|
||||
"""
|
||||
import subprocess
|
||||
repo_path = os.path.join(settings.repos_dir, repo)
|
||||
try:
|
||||
subprocess.run(
|
||||
["git", "-C", repo_path, "fetch", "origin"],
|
||||
capture_output=True, timeout=30,
|
||||
)
|
||||
co = subprocess.run(
|
||||
["git", "-C", repo_path, "checkout", branch],
|
||||
capture_output=True, text=True, timeout=30,
|
||||
)
|
||||
if co.returncode != 0:
|
||||
return False, f"Cannot checkout branch '{branch}': {co.stderr.strip()[-200:]}"
|
||||
r = subprocess.run(
|
||||
["make", "test"], cwd=repo_path,
|
||||
capture_output=True, text=True, timeout=600,
|
||||
)
|
||||
if r.returncode == 0:
|
||||
return True, "Local tests passed"
|
||||
tail = (r.stdout + r.stderr)[-500:]
|
||||
return False, f"Local tests failed: ...{tail}"
|
||||
except subprocess.TimeoutExpired:
|
||||
return False, "Local tests timed out (600s)"
|
||||
except Exception as e:
|
||||
return False, f"Local test run error: {e}"
|
||||
|
||||
|
||||
# Registry for dynamic lookup by name
|
||||
QG_CHECKS = {
|
||||
"check_analysis_approved": check_analysis_approved,
|
||||
@@ -223,4 +272,5 @@ QG_CHECKS = {
|
||||
"check_review_approved": check_review_approved,
|
||||
"check_tests_passed": check_tests_passed,
|
||||
"check_reviewer_verdict": check_reviewer_verdict,
|
||||
"check_tests_local": check_tests_local,
|
||||
}
|
||||
|
||||
@@ -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_ci_green"},
|
||||
"development": {"next": "review", "agent": "reviewer", "qg": "check_tests_local"},
|
||||
"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},
|
||||
|
||||
Reference in New Issue
Block a user