Add code_review phase with approval gate, reviewer≠implementer enforcement, and structured CODE_REVIEW.md
CI / build (push) Has been cancelled
CI / build (push) Has been cancelled
- Insert code_review phase between implement and bug_find - Approval gate: code_review:awaiting_approval → code_review:approved - Read-only phase — no edits, no fixes, no returning to implement - Reviewer≠implementer: .state.implementer tracking + --claim enforcement - Structured CODE_REVIEW.md: spec compliance, design conformance, quality scorecard, items found (severity/category/location/resolution), test coverage - Updated status.py (10 data structures), dashboard (4 files), prompts (3 files), agent routing, tests (6 new test classes, 19 new tests)
This commit is contained in:
+51
-23
@@ -65,6 +65,7 @@ VALID_PHASES = [
|
||||
"design", "design:awaiting_approval", "design:approved",
|
||||
"test_design", "test_design:awaiting_approval", "test_design:approved",
|
||||
"implement",
|
||||
"code_review", "code_review:awaiting_approval", "code_review:approved",
|
||||
"bug_find",
|
||||
"adversarial_bug_find",
|
||||
"doc_review",
|
||||
@@ -75,11 +76,11 @@ VALID_PHASES = [
|
||||
|
||||
BASE_PHASES = [
|
||||
"new", "research", "decomposition", "design", "test_design",
|
||||
"implement", "bug_find", "adversarial_bug_find", "doc_review",
|
||||
"referee", "complete", "human_intervention",
|
||||
"implement", "code_review", "bug_find", "adversarial_bug_find",
|
||||
"doc_review", "referee", "complete", "human_intervention",
|
||||
]
|
||||
|
||||
APPROVAL_PHASES = {"research", "decomposition", "design", "test_design"}
|
||||
APPROVAL_PHASES = {"research", "decomposition", "design", "test_design", "code_review"}
|
||||
|
||||
LEGAL_TRANSITIONS = {
|
||||
"new": ["research"],
|
||||
@@ -95,7 +96,10 @@ LEGAL_TRANSITIONS = {
|
||||
"test_design": ["test_design:awaiting_approval", "implement"],
|
||||
"test_design:awaiting_approval": ["test_design:approved"],
|
||||
"test_design:approved": ["implement"],
|
||||
"implement": ["bug_find"],
|
||||
"implement": ["code_review"],
|
||||
"code_review": ["code_review:awaiting_approval"],
|
||||
"code_review:awaiting_approval": ["code_review:approved"],
|
||||
"code_review:approved": ["bug_find"],
|
||||
"bug_find": ["adversarial_bug_find"],
|
||||
"adversarial_bug_find": ["doc_review"],
|
||||
"doc_review": ["referee"],
|
||||
@@ -109,6 +113,7 @@ PHASE_REQUIRED_ARTIFACTS = {
|
||||
"design": "DESIGN.md",
|
||||
"test_design": "TEST_PLAN.md",
|
||||
"implement": "IMPLEMENTATION.md",
|
||||
"code_review": "CODE_REVIEW.md",
|
||||
"bug_find": "BUG_REPORT.md",
|
||||
"adversarial_bug_find": "ADVERSARIAL_BUG_REPORT.md",
|
||||
"doc_review": "DOC_REVIEW.md",
|
||||
@@ -117,22 +122,24 @@ PHASE_REQUIRED_ARTIFACTS = {
|
||||
|
||||
FORBIDDEN_ARTIFACTS = {
|
||||
"new": ["SPEC.md", "DESIGN.md", "DECOMPOSITION.md", "TEST_PLAN.md",
|
||||
"IMPLEMENTATION.md", "BUG_REPORT.md", "ADVERSARIAL_BUG_REPORT.md",
|
||||
"DOC_REVIEW.md", "VERDICT.md"],
|
||||
"IMPLEMENTATION.md", "CODE_REVIEW.md", "BUG_REPORT.md",
|
||||
"ADVERSARIAL_BUG_REPORT.md", "DOC_REVIEW.md", "VERDICT.md"],
|
||||
"research": ["DESIGN.md", "DECOMPOSITION.md", "TEST_PLAN.md",
|
||||
"IMPLEMENTATION.md", "BUG_REPORT.md", "ADVERSARIAL_BUG_REPORT.md",
|
||||
"DOC_REVIEW.md", "VERDICT.md"],
|
||||
"IMPLEMENTATION.md", "CODE_REVIEW.md", "BUG_REPORT.md",
|
||||
"ADVERSARIAL_BUG_REPORT.md", "DOC_REVIEW.md", "VERDICT.md"],
|
||||
"decomposition": ["DESIGN.md", "TEST_PLAN.md", "IMPLEMENTATION.md",
|
||||
"BUG_REPORT.md", "ADVERSARIAL_BUG_REPORT.md",
|
||||
"DOC_REVIEW.md", "VERDICT.md"],
|
||||
"CODE_REVIEW.md", "BUG_REPORT.md",
|
||||
"ADVERSARIAL_BUG_REPORT.md", "DOC_REVIEW.md", "VERDICT.md"],
|
||||
"design": ["DECOMPOSITION.md", "TEST_PLAN.md", "IMPLEMENTATION.md",
|
||||
"BUG_REPORT.md", "ADVERSARIAL_BUG_REPORT.md",
|
||||
"CODE_REVIEW.md", "BUG_REPORT.md", "ADVERSARIAL_BUG_REPORT.md",
|
||||
"DOC_REVIEW.md", "VERDICT.md"],
|
||||
"test_design": ["DECOMPOSITION.md", "IMPLEMENTATION.md",
|
||||
"BUG_REPORT.md", "ADVERSARIAL_BUG_REPORT.md",
|
||||
"DOC_REVIEW.md", "VERDICT.md"],
|
||||
"implement": ["BUG_REPORT.md", "ADVERSARIAL_BUG_REPORT.md",
|
||||
"CODE_REVIEW.md", "BUG_REPORT.md",
|
||||
"ADVERSARIAL_BUG_REPORT.md", "DOC_REVIEW.md", "VERDICT.md"],
|
||||
"implement": ["CODE_REVIEW.md", "BUG_REPORT.md", "ADVERSARIAL_BUG_REPORT.md",
|
||||
"DOC_REVIEW.md", "VERDICT.md"],
|
||||
"code_review": ["BUG_REPORT.md", "ADVERSARIAL_BUG_REPORT.md",
|
||||
"DOC_REVIEW.md", "VERDICT.md"],
|
||||
"bug_find": ["ADVERSARIAL_BUG_REPORT.md", "DOC_REVIEW.md", "VERDICT.md"],
|
||||
"adversarial_bug_find": ["DOC_REVIEW.md", "VERDICT.md"],
|
||||
"doc_review": ["VERDICT.md"],
|
||||
@@ -142,12 +149,12 @@ FORBIDDEN_ARTIFACTS = {
|
||||
}
|
||||
|
||||
NON_ARTIFACT_FILES = {".state", ".state.tmp", ".state.lock", ".state.approvals",
|
||||
"VRAM_CONFIG.md", "PARENT_SPEC.md", "REVIEW.md"}
|
||||
".state.implementer", "VRAM_CONFIG.md", "PARENT_SPEC.md", "REVIEW.md"}
|
||||
|
||||
PHASE_PRIORITY = {
|
||||
"referee": 11, "doc_review": 10, "adversarial_bug_find": 9,
|
||||
"bug_find": 8, "implement": 7, "test_design": 6,
|
||||
"design": 5, "decomposition": 4, "research": 3, "new": 2,
|
||||
"referee": 12, "doc_review": 11, "adversarial_bug_find": 10,
|
||||
"bug_find": 9, "code_review": 8, "implement": 7,
|
||||
"test_design": 6, "design": 5, "decomposition": 4, "research": 3, "new": 2,
|
||||
}
|
||||
|
||||
ALLOWED_ACTIONS_MAP = {
|
||||
@@ -156,6 +163,7 @@ ALLOWED_ACTIONS_MAP = {
|
||||
"design": ["Read SPEC.md", "Ask design questions", "Write DESIGN.md"],
|
||||
"test_design": ["Read SPEC.md and DESIGN.md", "Ask test questions", "Write TEST_PLAN.md"],
|
||||
"implement": ["Edit code", "Write tests", "Create IMPLEMENTATION.md", "Run test suite"],
|
||||
"code_review": ["Read code", "Read SPEC.md", "Read DESIGN.md", "Read IMPLEMENTATION.md", "Write CODE_REVIEW.md"],
|
||||
"bug_find": ["Read code", "Read SPEC.md", "Read IMPLEMENTATION.md", "Write BUG_REPORT.md"],
|
||||
"adversarial_bug_find": ["Read code", "Read SPEC.md", "Read BUG_REPORT.md", "Write ADVERSARIAL_BUG_REPORT.md"],
|
||||
"doc_review": ["Read DESIGN.md", "Read code", "Read docs", "Write DOC_REVIEW.md", "Update documentation"],
|
||||
@@ -174,6 +182,8 @@ FORBIDDEN_ACTIONS_MAP = {
|
||||
"Modify SPEC.md or DESIGN.md"],
|
||||
"implement": ["Create new tasks", "Modify SPEC.md or DESIGN.md",
|
||||
"Transition to bug-find phase (Orchestrator does this)"],
|
||||
"code_review": ["Edit code", "Fix bugs or issues", "Modify SPEC.md", "Modify DESIGN.md",
|
||||
"Modify IMPLEMENTATION.md", "Create any artifact other than CODE_REVIEW.md"],
|
||||
"bug_find": ["Edit code", "Fix bugs (separate implementation task)", "Modify SPEC.md"],
|
||||
"adversarial_bug_find": ["Edit code", "Fix bugs", "Modify SPEC.md or BUG_REPORT.md"],
|
||||
"doc_review": ["Edit non-documentation code", "Modify SPEC.md", "Modify DESIGN.md"],
|
||||
@@ -187,7 +197,8 @@ NEXT_PHASE_MAP = {
|
||||
"decomposition": "sub-task research",
|
||||
"design": "test_design or implement",
|
||||
"test_design": "implement",
|
||||
"implement": "bug_find",
|
||||
"implement": "code_review",
|
||||
"code_review": "bug_find",
|
||||
"bug_find": "adversarial_bug_find",
|
||||
"adversarial_bug_find": "doc_review",
|
||||
"doc_review": "referee",
|
||||
@@ -290,8 +301,8 @@ def _append_approval(task_path: Path, phase: str, approver: str) -> None:
|
||||
def _infer_state_from_artifacts(task_path: Path) -> Optional[str]:
|
||||
artifacts = {}
|
||||
for name in ["SPEC.md", "DECOMPOSITION.md", "DESIGN.md", "TEST_PLAN.md",
|
||||
"IMPLEMENTATION.md", "BUG_REPORT.md", "ADVERSARIAL_BUG_REPORT.md",
|
||||
"DOC_REVIEW.md", "VERDICT.md"]:
|
||||
"IMPLEMENTATION.md", "CODE_REVIEW.md", "BUG_REPORT.md",
|
||||
"ADVERSARIAL_BUG_REPORT.md", "DOC_REVIEW.md", "VERDICT.md"]:
|
||||
f = task_path / name
|
||||
if f.exists() and f.stat().st_size > 0:
|
||||
artifacts[name] = True
|
||||
@@ -306,8 +317,10 @@ def _infer_state_from_artifacts(task_path: Path) -> Optional[str]:
|
||||
return "doc_review"
|
||||
if "BUG_REPORT.md" in artifacts:
|
||||
return "adversarial_bug_find"
|
||||
if "IMPLEMENTATION.md" in artifacts:
|
||||
if "CODE_REVIEW.md" in artifacts:
|
||||
return "bug_find"
|
||||
if "IMPLEMENTATION.md" in artifacts:
|
||||
return "code_review"
|
||||
if "TEST_PLAN.md" in artifacts:
|
||||
return "implement"
|
||||
if "DESIGN.md" in artifacts:
|
||||
@@ -535,6 +548,13 @@ def cmd_transition(args):
|
||||
return 1
|
||||
if current == "human_intervention" and target == "complete":
|
||||
_auto_update_verdict_on_complete(task_path)
|
||||
if current == "implement" and target == "code_review":
|
||||
lock_file = task_path / ".state.lock"
|
||||
if lock_file.exists():
|
||||
content = lock_file.read_text().strip()
|
||||
lines = dict(l.split(": ", 1) for l in content.splitlines() if ": " in l)
|
||||
implementer = lines.get("agent", "unknown")
|
||||
(task_path / ".state.implementer").write_text(f"{implementer}\n")
|
||||
_write_state(task_path, target)
|
||||
print(f"Transitioned task '{args.task}' from '{current}' to '{target}'.")
|
||||
return 0
|
||||
@@ -614,6 +634,7 @@ def _check_forbidden_artifacts(task_path: Path, phase: str) -> list[tuple[str, s
|
||||
"DESIGN.md": "design",
|
||||
"TEST_PLAN.md": "test_design",
|
||||
"IMPLEMENTATION.md": "implement",
|
||||
"CODE_REVIEW.md": "code_review",
|
||||
"BUG_REPORT.md": "bug_find",
|
||||
"ADVERSARIAL_BUG_REPORT.md": "adversarial_bug_find",
|
||||
"DOC_REVIEW.md": "doc_review",
|
||||
@@ -729,7 +750,7 @@ def cmd_audit(args):
|
||||
if base == "implement" and (path / "IMPLEMENTATION.md").exists() and (path / "IMPLEMENTATION.md").stat().st_size == 0:
|
||||
inconsistency = True
|
||||
details.append(f"IMPLEMENTATION.md is empty but .state says {base}")
|
||||
if base in ("bug_find", "adversarial_bug_find", "doc_review", "referee") and not (path / "IMPLEMENTATION.md").exists():
|
||||
if base in ("bug_find", "adversarial_bug_find", "code_review", "doc_review", "referee") and not (path / "IMPLEMENTATION.md").exists():
|
||||
inconsistency = True
|
||||
details.append(f".state says {base} but IMPLEMENTATION.md is missing")
|
||||
if inconsistency:
|
||||
@@ -1078,6 +1099,13 @@ def cmd_claim(args):
|
||||
if "*" not in agent_phases and base not in agent_phases:
|
||||
print(f"ERROR: Agent '{args.agent}' is not configured for phase '{base}'. Allowed phases: {', '.join(agent_phases)}")
|
||||
return 1
|
||||
if base == "code_review":
|
||||
implementer_file = task_path / ".state.implementer"
|
||||
if implementer_file.exists():
|
||||
implementer = implementer_file.read_text().strip()
|
||||
if implementer == args.agent:
|
||||
print(f"ERROR: Agent '{args.agent}' implemented this task and cannot claim the code_review phase. Reviewer must be different from implementer.")
|
||||
return 1
|
||||
lock_file = task_path / ".state.lock"
|
||||
timeout_sec = _lock_timeout_seconds(args.project)
|
||||
if lock_file.exists():
|
||||
|
||||
Reference in New Issue
Block a user