diff --git a/src/qg/checks.py b/src/qg/checks.py index e2a6b0a..fdca16e 100644 --- a/src/qg/checks.py +++ b/src/qg/checks.py @@ -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/ 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, } diff --git a/src/stages.py b/src/stages.py index a27cd5e..070f9f4 100644 --- a/src/stages.py +++ b/src/stages.py @@ -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},