From 3d4c0926b468de41808a95776ae57b5a2989f99c Mon Sep 17 00:00:00 2001 From: laptran Date: Tue, 16 Jun 2026 09:00:22 -0400 Subject: [PATCH] =?UTF-8?q?Add=20code=5Freview=20phase=20with=20approval?= =?UTF-8?q?=20gate,=20reviewer=E2=89=A0implementer=20enforcement,=20and=20?= =?UTF-8?q?structured=20CODE=5FREVIEW.md?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - 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) --- .agent.md | 4 + automaton/dashboard/core/board.py | 7 +- automaton/dashboard/core/stats.py | 3 +- automaton/dashboard/core/task.py | 39 +++-- automaton/dashboard/core/timeline.py | 6 +- prompts/code_review.md | 192 +++++++++++++++++++++++ prompts/orchestrate.md | 6 +- prompts/workflow.md | 10 +- scripts/status.py | 74 ++++++--- tests/test_framework_self_consistency.py | 4 +- tests/test_status.py | 157 +++++++++++++++++- tests/test_task.py | 6 +- 12 files changed, 458 insertions(+), 50 deletions(-) create mode 100644 prompts/code_review.md diff --git a/.agent.md b/.agent.md index 7ea0913..5244bad 100644 --- a/.agent.md +++ b/.agent.md @@ -11,6 +11,7 @@ IF task type = research → load prompts/research.md + .rules.md IF task type = design → load prompts/design.md + SPEC.md IF task type = test_design → load prompts/test_design.md + SPEC.md + DESIGN.md IF task type = implement → load prompts/implement.md + SPEC.md + DESIGN.md + TEST_PLAN.md + CONTRACT.md +IF task type = code_review → load prompts/code_review.md + SPEC.md + DESIGN.md + IMPLEMENTATION.md IF task type = bug_find → load prompts/bug_finder.md + SPEC.md + code IF task type = adversarial_bug_find → load prompts/adversarial_bug_find.md + SPEC.md + code IF task type = referee → load prompts/referee.md + SPEC.md + BUG_REPORT.md + ADVERSARIAL_BUG_REPORT.md + DOC_REVIEW.md @@ -45,6 +46,9 @@ Agents: phases: [research, decomposition, design, test_design] - id: implementer phases: [implement] + - id: code-reviewer + phases: [code_review] + role: reviewer - id: bug-hunter phases: [bug_find, adversarial_bug_find] - id: referee diff --git a/automaton/dashboard/core/board.py b/automaton/dashboard/core/board.py index d573303..880ac9d 100644 --- a/automaton/dashboard/core/board.py +++ b/automaton/dashboard/core/board.py @@ -10,7 +10,7 @@ class KanbanBoard: COLUMNS = [ TaskState.BACKLOG, TaskState.RESEARCH, TaskState.DECOMPOSITION, TaskState.DESIGN, TaskState.TEST_DESIGN, - TaskState.IMPLEMENT, + TaskState.IMPLEMENT, TaskState.CODE_REVIEW, TaskState.BUG_FIND, TaskState.ADV_BUG_FIND, TaskState.DOC_REVIEW, TaskState.REFEREE, TaskState.BLOCKED, TaskState.DONE, ] @@ -27,7 +27,7 @@ class KanbanBoard: TaskState.IMPLEMENT ], "Verification": [ - TaskState.BUG_FIND, TaskState.ADV_BUG_FIND, TaskState.DOC_REVIEW, TaskState.REFEREE + TaskState.CODE_REVIEW, TaskState.BUG_FIND, TaskState.ADV_BUG_FIND, TaskState.DOC_REVIEW, TaskState.REFEREE ], "Blocked": [ TaskState.BLOCKED @@ -103,7 +103,8 @@ class KanbanBoard: def get_wip_count(self) -> int: wip_states = [ TaskState.RESEARCH, TaskState.DECOMPOSITION, TaskState.DESIGN, - TaskState.TEST_DESIGN, TaskState.IMPLEMENT, TaskState.BUG_FIND, + TaskState.TEST_DESIGN, TaskState.IMPLEMENT, TaskState.CODE_REVIEW, + TaskState.BUG_FIND, TaskState.ADV_BUG_FIND, TaskState.DOC_REVIEW, TaskState.REFEREE, ] return sum(len(self._columns.get(s, [])) for s in wip_states) diff --git a/automaton/dashboard/core/stats.py b/automaton/dashboard/core/stats.py index d4b6d49..5f44f3e 100644 --- a/automaton/dashboard/core/stats.py +++ b/automaton/dashboard/core/stats.py @@ -31,7 +31,8 @@ class TaskStats: def in_progress_count(self) -> int: wip_states = [ TaskState.RESEARCH, TaskState.DECOMPOSITION, TaskState.DESIGN, - TaskState.TEST_DESIGN, TaskState.IMPLEMENT, TaskState.BUG_FIND, + TaskState.TEST_DESIGN, TaskState.IMPLEMENT, TaskState.CODE_REVIEW, + TaskState.BUG_FIND, TaskState.ADV_BUG_FIND, TaskState.DOC_REVIEW, TaskState.REFEREE, ] return sum(1 for t in self.tasks if t.state in wip_states) diff --git a/automaton/dashboard/core/task.py b/automaton/dashboard/core/task.py index ac77e51..14f58f8 100644 --- a/automaton/dashboard/core/task.py +++ b/automaton/dashboard/core/task.py @@ -15,6 +15,7 @@ class TaskState(Enum): DESIGN = "design" TEST_DESIGN = "test_design" IMPLEMENT = "implement" + CODE_REVIEW = "code_review" BUG_FIND = "bug_find" ADV_BUG_FIND = "adv_bug_find" DOC_REVIEW = "doc_review" @@ -30,6 +31,7 @@ COLUMN_HEADERS = { TaskState.DESIGN: "Design", TaskState.TEST_DESIGN: "Test Design", TaskState.IMPLEMENT: "Implement", + TaskState.CODE_REVIEW: "Code Review", TaskState.BUG_FIND: "Bug Find", TaskState.ADV_BUG_FIND: "Adversarial Bug Find", TaskState.DOC_REVIEW: "Doc Review", @@ -44,6 +46,7 @@ ARTIFACTS = { "DESIGN.md": TaskState.DESIGN, "TEST_PLAN.md": TaskState.TEST_DESIGN, "IMPLEMENTATION.md": TaskState.IMPLEMENT, + "CODE_REVIEW.md": TaskState.CODE_REVIEW, "BUG_REPORT.md": TaskState.BUG_FIND, "ADVERSARIAL_BUG_REPORT.md": TaskState.ADV_BUG_FIND, "DOC_REVIEW.md": TaskState.DOC_REVIEW, @@ -161,8 +164,12 @@ class Task: return "Both bug report and adversarial report filed — awaiting doc review" if self.state == TaskState.DOC_REVIEW: return "Under document review" + if self.state == TaskState.CODE_REVIEW: + if "CODE_REVIEW.md" in self.artifacts: + return "Code review complete — awaiting user approval" + return "Under code review — awaiting CODE_REVIEW.md" if self.state == TaskState.IMPLEMENT: - return "Test plan approved — ready for implementation" + return "Implementation complete — awaiting code review" if self.state == TaskState.DESIGN: return "Design document written — awaiting test plan" if self.state == TaskState.DECOMPOSITION: @@ -255,6 +262,7 @@ class Task: TaskState.DESIGN: "DESIGN.md", TaskState.TEST_DESIGN: "TEST_PLAN.md", TaskState.IMPLEMENT: "IMPLEMENTATION.md", + TaskState.CODE_REVIEW: "CODE_REVIEW.md", TaskState.BUG_FIND: "BUG_REPORT.md", TaskState.ADV_BUG_FIND: "ADVERSARIAL_BUG_REPORT.md", TaskState.DOC_REVIEW: "DOC_REVIEW.md", @@ -272,7 +280,8 @@ class Task: TaskState.DECOMPOSITION: "sub-task research", TaskState.DESIGN: "test_design or implement", TaskState.TEST_DESIGN: "implement", - TaskState.IMPLEMENT: "bug_find", + TaskState.IMPLEMENT: "code_review", + TaskState.CODE_REVIEW: "bug_find", TaskState.BUG_FIND: "adversarial_bug_find", TaskState.ADV_BUG_FIND: "doc_review", TaskState.DOC_REVIEW: "referee", @@ -288,7 +297,7 @@ class Task: @property def is_approval_gated(self) -> bool: - return self.state in (TaskState.RESEARCH, TaskState.DECOMPOSITION, TaskState.DESIGN, TaskState.TEST_DESIGN) + return self.state in (TaskState.RESEARCH, TaskState.DECOMPOSITION, TaskState.DESIGN, TaskState.TEST_DESIGN, TaskState.CODE_REVIEW) @property def blocker(self) -> str: @@ -308,6 +317,9 @@ class Task: if self.state == TaskState.ADV_BUG_FIND: if "ADVERSARIAL_BUG_REPORT.md" not in self.artifacts or not self.artifacts["ADVERSARIAL_BUG_REPORT.md"].content: return "Agent must generate ADVERSARIAL_BUG_REPORT.md" + if self.state == TaskState.CODE_REVIEW: + if "CODE_REVIEW.md" not in self.artifacts or not self.artifacts["CODE_REVIEW.md"].content: + return "Agent must generate CODE_REVIEW.md" if self.state == TaskState.DOC_REVIEW: if "DOC_REVIEW.md" not in self.artifacts or not self.artifacts["DOC_REVIEW.md"].content: return "Agent must generate DOC_REVIEW.md" @@ -365,7 +377,14 @@ class Task: return ( "EDIT ALLOWED — agent can modify code, write tests, create IMPLEMENTATION.md.\n\n" "When implementation is done:\n" - f" python ~/.automaton/scripts/status.py --transition bug_find --task {self.name}" + f" python ~/.automaton/scripts/status.py --transition code_review --task {self.name}" + ) + if self.state == TaskState.CODE_REVIEW: + return ( + "READ-ONLY — agent reviews code quality and writes CODE_REVIEW.md.\n" + "Do NOT edit code or fix issues — document findings with severity and resolution.\n\n" + "When ready for review:\n" + f" python ~/.automaton/scripts/status.py --transition code_review:awaiting_approval --task {self.name}" ) if self.state == TaskState.BUG_FIND: return ( @@ -484,8 +503,10 @@ def determine_task_state(folder_path: Path) -> tuple[TaskState, dict[str, Artifa return TaskState.BUG_FIND, artifacts if "ADVERSARIAL_BUG_REPORT.md" in artifacts: return TaskState.BUG_FIND, artifacts + if "CODE_REVIEW.md" in artifacts: + return TaskState.CODE_REVIEW, artifacts if "IMPLEMENTATION.md" in artifacts: - return TaskState.BUG_FIND, artifacts + return TaskState.IMPLEMENT, artifacts if "TEST_PLAN.md" in artifacts: return TaskState.IMPLEMENT, artifacts if "DESIGN.md" in artifacts: @@ -623,10 +644,10 @@ def discover_tasks(tasks_dir: Path) -> list[Task]: # Sort by state (most advanced first) state_order = { - TaskState.DONE: 12, TaskState.BLOCKED: 11, TaskState.REFEREE: 10, - TaskState.DOC_REVIEW: 9, TaskState.ADV_BUG_FIND: 8, TaskState.BUG_FIND: 7, - TaskState.IMPLEMENT: 6, TaskState.TEST_DESIGN: 5, TaskState.DESIGN: 4, - TaskState.DECOMPOSITION: 3, TaskState.RESEARCH: 2, TaskState.BACKLOG: 1, + TaskState.DONE: 13, TaskState.BLOCKED: 12, TaskState.REFEREE: 11, + TaskState.DOC_REVIEW: 10, TaskState.ADV_BUG_FIND: 9, TaskState.BUG_FIND: 8, + TaskState.CODE_REVIEW: 7, TaskState.IMPLEMENT: 6, TaskState.TEST_DESIGN: 5, + TaskState.DESIGN: 4, TaskState.DECOMPOSITION: 3, TaskState.RESEARCH: 2, TaskState.BACKLOG: 1, } tasks.sort(key=lambda t: state_order.get(t.state, 0), reverse=True) return tasks diff --git a/automaton/dashboard/core/timeline.py b/automaton/dashboard/core/timeline.py index bedf23c..2401448 100644 --- a/automaton/dashboard/core/timeline.py +++ b/automaton/dashboard/core/timeline.py @@ -44,7 +44,8 @@ def build_task_timelines(tasks: list[Task]) -> list[TaskTimeline]: timeline = TaskTimeline(task_name=task.name, display_name=task.display_name, is_complete=task.is_done, is_blocked=task.is_blocked) all_states = [TaskState.RESEARCH, TaskState.DECOMPOSITION, TaskState.DESIGN, - TaskState.TEST_DESIGN, TaskState.IMPLEMENT, TaskState.BUG_FIND, + TaskState.TEST_DESIGN, TaskState.IMPLEMENT, TaskState.CODE_REVIEW, + TaskState.BUG_FIND, TaskState.ADV_BUG_FIND, TaskState.DOC_REVIEW, TaskState.REFEREE] for state in all_states: is_complete = False @@ -64,6 +65,9 @@ def build_task_timelines(tasks: list[Task]) -> list[TaskTimeline]: elif state == TaskState.IMPLEMENT: is_complete = "IMPLEMENTATION.md" in task.artifacts is_current = task.state == state and not is_complete + elif state == TaskState.CODE_REVIEW: + is_complete = "CODE_REVIEW.md" in task.artifacts + is_current = task.state == state and not is_complete elif state == TaskState.BUG_FIND: is_complete = "BUG_REPORT.md" in task.artifacts is_current = task.state == state and not is_complete diff --git a/prompts/code_review.md b/prompts/code_review.md new file mode 100644 index 0000000..b858ce3 --- /dev/null +++ b/prompts/code_review.md @@ -0,0 +1,192 @@ +You are in Code Review mode. + +Your job is to review the implementation for correctness, quality, and spec compliance. You are an assessor — not a fixer. + +## Read These Files + +1. {project}/.automaton/tasks/{task-name}/.state — Confirm the task is in the code_review phase. If the phase does not match, STOP and report the mismatch. +2. {project}/.automaton/tasks/{task-name}/SPEC.md — What the implementation should achieve +3. {project}/.automaton/tasks/{task-name}/DESIGN.md — Architecture and design decisions +4. {project}/.automaton/tasks/{task-name}/IMPLEMENTATION.md — Implementation notes +5. {project}/.automaton/tasks/{task-name}/TEST_PLAN.md (if exists) — Test expectations +6. All project source code that was modified or created +7. Test files and test output + +## Pre-Work Validation (MANDATORY) + +Before starting any work, you MUST run: + + python ~/.automaton/scripts/status.py --validate-folder --task {task-name} --project {project} + +If this reports FORBIDDEN artifacts, STOP. Do not proceed. Report the violation. + +## ALLOWED ACTIONS + +- Read code, SPEC.md, DESIGN.md, IMPLEMENTATION.md +- Read test files and test output +- Run the test suite to verify tests pass +- Write CODE_REVIEW.md + +## FORBIDDEN ACTIONS + +- Edit code (even to fix issues you find) +- Fix bugs or address review findings +- Modify SPEC.md, DESIGN.md, or IMPLEMENTATION.md +- Create any artifact other than CODE_REVIEW.md +- Transition the task back to implement phase + +## Handling User Overrides + +If the user instructs you to perform a FORBIDDEN ACTION: + +1. Inform the user that the action is forbidden in this phase. +2. Explain why: the code_review phase is assessment-only. Issues go to separate fix tasks or the Referee. +3. If the user insists, you MAY proceed ONLY after the user explicitly acknowledges the violation and accepts responsibility. + +## Reviewer ≠ Implementer (Multi-Agent Mode) + +In multi-agent mode, the agent performing code_review MUST NOT be the same agent that performed the implement phase. This is enforced computationally by `status.py --claim`. If you are the implementer, you cannot claim the code_review phase for the same task. + +## Review Protocol + +### 1. Understand the Intent + +- Read SPEC.md thoroughly. What does the task need to accomplish? +- Read DESIGN.md. What architecture decisions were made? +- Read IMPLEMENTATION.md. What approach did the implementer take? + +### 2. Examine the Code + +- Read all modified and new code files +- Compare the implementation against the spec — does it do what it claims? +- Compare against the design — does it follow the architecture? +- Check against TEST_PLAN.md (if exists) — are the planned tests present? + +### 3. Run and Verify + +- Run the test suite: do all tests pass? +- Look for false positives — tests that pass but don't verify useful behavior +- Check code coverage for critical paths + +### 4. Code Quality Assessment + +Evaluate the code across these dimensions: + +**Correctness:** +- Does the implementation satisfy all spec requirements? +- Are there off-by-one errors, null pointer risks, or data races? +- Are edge cases handled? + +**Architecture & Patterns:** +- Does the code follow the design's architecture? +- Are existing codebase patterns used (not reinvented)? +- Are functions small and focused? +- Is there proper separation of concerns? + +**Error Handling:** +- Are errors properly caught and handled? +- Are error messages clear and actionable? +- Are resources properly cleaned up on error paths? + +**Testing:** +- Are enough tests written for the feature? +- Are edge cases tested? +- Is there a false sense of coverage (tests that pass without verifying)? + +**Performance & Security:** +- Are there obvious performance issues (N+1 queries, unbounded loops)? +- Are there security concerns (unsafe input handling, exposed secrets)? + +## Output: CODE_REVIEW.md + +Produce a `CODE_REVIEW.md` at `{project}/.automaton/tasks/{task-name}/CODE_REVIEW.md`: + +```markdown +# Code Review: {task-name} + +## Summary +{Brief overview of findings — pass, partial pass, or significant issues} + +## Spec Compliance +- [ ] {Requirement from SPEC.md} — {Status: Met / Partial / Not Met} +- [ ] {Requirement from SPEC.md} — {Status: Met / Partial / Not Met} + +## Design Conformance +- [ ] {Design decision from DESIGN.md} — {Status: Followed / Deviated / Not Applicable} +- [ ] {Design decision from DESIGN.md} — {Status: Followed / Deviated / Not Applicable} + +## Code Quality Scorecard +| Dimension | Score (1-5) | Notes | +|---|---|---| +| Correctness | {score} | {notes} | +| Architecture | {score} | {notes} | +| Error Handling | {score} | {notes} | +| Testing | {score} | {notes} | +| Performance | {score} | {notes} | +| Security | {score} | {notes} | + +## Items Found + +### Item 1: {Title} +- **Severity**: Critical / High / Medium / Low +- **Category**: Correctness / Architecture / Error Handling / Testing / Performance / Security / Style +- **Location**: `{file}:{line}` or `{function/class name}` +- **Description**: {What is wrong and why it matters} +- **Resolution**: {Recommended fix — for a separate fix task, not to be done here} + +### Item 2: {Title} +- **Severity**: {Critical / High / Medium / Low} +- **Category**: {Category} +- **Location**: {location} +- **Description**: {description} +- **Resolution**: {recommended fix} + +## Test Coverage Assessment +- Total tests: {count} +- Tests passing: {count} +- Missing test cases: {list or "None identified"} +- False positives (tests that pass but don't verify): {list or "None identified"} + +## Overall Verdict +{RECOMMEND_PASS / RECOMMEND_FIX / RECOMMEND_REWORK} + +## Reviewer Notes +{Any additional context, patterns noticed, or concerns for the Referee} +``` + +### Severity Definitions +- **Critical**: Security vulnerability, data loss, spec non-compliance that blocks release +- **High**: Significant bug, missing feature, or design violation likely to cause problems +- **Medium**: Code quality issue, missing edge case, or pattern deviation +- **Low**: Style nit, minor improvement opportunity, or non-critical suggestion + +## Approval Gate (MANDATORY) + +This phase requires user approval before proceeding to the next phase. + +1. After producing the CODE_REVIEW.md, transition to awaiting_approval: + + python ~/.automaton/scripts/status.py --task {task-name} --project {project} --transition code_review:awaiting_approval + +2. Present the review findings to the user for sign-off. + +3. After the user says "APPROVED" or equivalent: + + python ~/.automaton/scripts/status.py --task {task-name} --project {project} --approve + +4. Then transition to the next phase: + + python ~/.automaton/scripts/status.py --task {task-name} --project {project} --transition bug_find + +## Rules + +- Do NOT fix issues you find — document them with recommended resolutions +- Do NOT send the task back to implement phase +- Be thorough but fair — recognize good work as well as problems +- The task proceeds forward regardless of findings (issues create separate fix tasks) +- If the implementation is excellent, say so clearly + +## Stop Condition (MANDATORY) + +You are not allowed to end this session until you have produced the CODE_REVIEW.md file AND output the exact phrase "CONTRACT_MET". +Until then, continue working or ask clarifying questions. diff --git a/prompts/orchestrate.md b/prompts/orchestrate.md index 1c8097b..5969fa7 100644 --- a/prompts/orchestrate.md +++ b/prompts/orchestrate.md @@ -43,7 +43,7 @@ Never piggyback on a stale task — create a new one for new work. The full state machine is defined in ~/.automaton/prompts/workflow.md. Key points: - **`.state` file is the single source of truth** — always read `.state` first, fall back to artifact heuristic if missing -- **Approval gates**: research, decomposition, design, and test_design require `:awaiting_approval` → `:approved` before proceeding +- **Approval gates**: research, decomposition, design, test_design, and code_review require `:awaiting_approval` → `:approved` before proceeding - **Transitions**: All transitions go through `python ~/.automaton/scripts/status.py --transition {phase} --task {task-name} --project {project}` - **Approvals**: All approvals go through `python ~/.automaton/scripts/status.py --approve --task {task-name} --project {project}` - **Task creation**: Always use `python ~/.automaton/scripts/status.py --create-task {name} --project {project}` @@ -59,7 +59,7 @@ For each phase in autopilot: 3. If violations found → STOP and report (phase-skipping detected) 4. Load phase prompt → confirm ALLOWED/FORBIDDEN boundaries 5. Execute phase → produce required artifact - 6. If phase requires approval (research, decomposition, design, test_design): + 6. If phase requires approval (research, decomposition, design, test_design, code_review): a. Run: python ~/.automaton/scripts/status.py --transition {phase}:awaiting_approval --task {task-name} --project {project} b. STOP and wait for user to say "APPROVED" c. Run: python ~/.automaton/scripts/status.py --approve --task {task-name} --project {project} @@ -134,7 +134,7 @@ During long autopilot runs, call `python ~/.automaton/scripts/status.py --audit ## Rules 1. **Never skip a phase** — all transitions must go through `status.py --transition` -2. **Wait for approval** — research, decomposition, design, and test_design require `:awaiting_approval` → `:approved` +2. **Wait for approval** — research, decomposition, design, test_design, and code_review require `:awaiting_approval` → `:approved` 3. **Validate before proceeding** — run `status.py --validate-folder --project {project}` before each phase 4. **Never create tasks manually** — always use `status.py --create-task` 5. **Always pass `--project {project}`** — ensures correct scoping when working on multiple projects diff --git a/prompts/workflow.md b/prompts/workflow.md index 13c5922..146213b 100644 --- a/prompts/workflow.md +++ b/prompts/workflow.md @@ -47,7 +47,10 @@ When `Mode: multi-agent` is set in `.agent.md`, a `.state.lock` file tracks whic | **Test Design** | Has `.state` = `test_design` | test_design:awaiting_approval | Present TEST_PLAN.md draft for user sign-off | | **test_design:awaiting_approval** | Has `.state` = `test_design:awaiting_approval` | test_design:approved | User says "APPROVED", call `status.py --approve` | | **test_design:approved** | Has `.state` = `test_design:approved` | Implement | Transition via `status.py --transition` | -| **Implementation** | Has `.state` = `implement` | Bug Find | Generate `BUG_REPORT.md` | +| **Implementation** | Has `.state` = `implement` | Code Review | Generate `CODE_REVIEW.md` | +| **Code Review** | Has `.state` = `code_review` | code_review:awaiting_approval | Present CODE_REVIEW.md draft for user sign-off | +| **code_review:awaiting_approval** | Has `.state` = `code_review:awaiting_approval` | code_review:approved | User says "APPROVED", call `status.py --approve` | +| **code_review:approved** | Has `.state` = `code_review:approved` | Bug Find | Transition via `status.py --transition` | | **Bug Find** | Has `.state` = `bug_find` | Adversarial Bug Find | Generate `ADVERSARIAL_BUG_REPORT.md` | | **Adversarial Bug Find** | Has `.state` = `adversarial_bug_find` | Doc Review | Generate `DOC_REVIEW.md` | | **Doc Review** | Has `.state` = `doc_review` | Referee | Generate `VERDICT.md` | @@ -59,6 +62,9 @@ The following phases do **not** have `:awaiting_approval` sub-states because the - `implement`, `bug_find`, `adversarial_bug_find`, `doc_review`, `referee` - These transition directly to the next phase upon producing their artifact and calling `status.py --transition` +Phases with approval gates (require user sign-off): +- `research`, `decomposition`, `design`, `test_design`, `code_review` + ## Task Creation (via `status.py`) New tasks MUST be created via `status.py --create-task {name} --project {project}`. This creates the folder, `.state` = `new`, and an empty `.state.approvals` file. @@ -101,7 +107,7 @@ All transitions go through `status.py --transition {phase} --project {project}`: ## Autopilot Rules 1. **Linear Progression**: Never skip a phase. Each transition must go through `status.py --transition --project {project}`. -2. **Approval Gates**: Research, Decomposition, Design, and Test Design phases require explicit user approval before proceeding. The autopilot MUST pause at `:awaiting_approval` sub-states. +2. **Approval Gates**: Research, Decomposition, Design, Test Design, and Code Review phases require explicit user approval before proceeding. The autopilot MUST pause at `:awaiting_approval` sub-states. 3. **Artifact Check**: A phase is only considered "complete" if its corresponding artifact exists, is non-empty, AND the `.state` file reflects the completed phase. 4. **Folder Validation**: Before each phase transition, run `status.py --validate-folder --project {project}`. Do not proceed past violations. 5. **Human Intervention**: If the Referee marks a task as `FAIL`, `NEEDS_REVIEW`, or identifies "Tie-Breaks", the Autopilot pauses and waits for user input. diff --git a/scripts/status.py b/scripts/status.py index 965fa43..81325a2 100755 --- a/scripts/status.py +++ b/scripts/status.py @@ -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(): diff --git a/tests/test_framework_self_consistency.py b/tests/test_framework_self_consistency.py index 4f0fac0..7897cc3 100644 --- a/tests/test_framework_self_consistency.py +++ b/tests/test_framework_self_consistency.py @@ -198,7 +198,7 @@ class TestVerdictParsingRegression: state, _ = determine_task_state(task_dir) assert state == TaskState.BLOCKED - def test_implementation_alone_is_bug_find(self): + def test_implementation_alone_is_implement(self): from automaton.dashboard.core.task import determine_task_state, TaskState from pathlib import Path import tempfile @@ -207,7 +207,7 @@ class TestVerdictParsingRegression: task_dir.mkdir() (task_dir / "IMPLEMENTATION.md").write_text("# Implementation\nDone.\n") state, _ = determine_task_state(task_dir) - assert state == TaskState.BUG_FIND + assert state == TaskState.IMPLEMENT def test_empty_verdict_is_blocked(self): from automaton.dashboard.core.task import determine_task_state, TaskState diff --git a/tests/test_status.py b/tests/test_status.py index 4e43f4e..58da19c 100644 --- a/tests/test_status.py +++ b/tests/test_status.py @@ -375,14 +375,14 @@ class TestInferStateFromArtifacts: state = (task_dir / ".state").read_text().strip() assert state == "research" - def test_implementation_only_infers_bug_find(self, tmp_project): + def test_implementation_only_infers_code_review(self, tmp_project): task_dir = tmp_project / ".automaton" / "tasks" / "impl-only" task_dir.mkdir() (task_dir / "IMPLEMENTATION.md").write_text("# Impl") out, code = _run_status(["--upgrade", "--task", "impl-only"], tmp_project) assert code == 0 state = (task_dir / ".state").read_text().strip() - assert state == "bug_find" + assert state == "code_review" def test_bug_report_only_infers_adversarial_bug_find(self, tmp_project): task_dir = tmp_project / ".automaton" / "tasks" / "bug-only" @@ -421,4 +421,155 @@ class TestValidateFolderCorruptedState: (task_dir / "SPEC.md").write_text("# Spec") out, code = _run_status(["--validate-folder", "--task", "ws-test"], tmp_project) assert code == 1 - assert "corrupted" in out.lower() \ No newline at end of file + assert "corrupted" in out.lower() + + +class TestCodeReviewPhase: + def test_transition_implement_to_code_review(self, tmp_project): + task_dir = _create_task(tmp_project, "cr-trans", "implement") + (task_dir / "IMPLEMENTATION.md").write_text("# Impl") + out, code = _run_status(["--task", "cr-trans", "--transition", "code_review"], tmp_project) + assert code == 0 + assert "Transitioned" in out + assert (task_dir / ".state").read_text().strip() == "code_review" + + def test_transition_code_review_to_awaiting_approval(self, tmp_project): + task_dir = _create_task(tmp_project, "cr-await", "code_review") + (task_dir / "IMPLEMENTATION.md").write_text("# Impl") + (task_dir / "CODE_REVIEW.md").write_text("# Code Review") + out, code = _run_status(["--task", "cr-await", "--transition", "code_review:awaiting_approval"], tmp_project) + assert code == 0 + assert (task_dir / ".state").read_text().strip() == "code_review:awaiting_approval" + + def test_code_review_approval_gate_blocks_skip(self, tmp_project): + task_dir = _create_task(tmp_project, "cr-approval-block", "code_review:awaiting_approval") + out, code = _run_status(["--task", "cr-approval-block", "--transition", "bug_find"], tmp_project) + assert code == 1 + assert "code_review:approved" in out + + def test_code_review_approved_to_bug_find(self, tmp_project): + task_dir = _create_task(tmp_project, "cr-approved", "code_review:approved") + (task_dir / "CODE_REVIEW.md").write_text("# Code Review") + out, code = _run_status(["--task", "cr-approved", "--transition", "bug_find"], tmp_project) + assert code == 0 + assert (task_dir / ".state").read_text().strip() == "bug_find" + + def test_code_review_approvable(self, tmp_project): + task_dir = _create_task(tmp_project, "cr-approvable", "code_review:awaiting_approval") + out, code = _run_status(["--approve", "--task", "cr-approvable"], tmp_project) + assert code == 0 + assert "Approved" in out + assert (task_dir / ".state").read_text().strip() == "code_review:approved" + + def test_code_review_approve_records_log(self, tmp_project): + task_dir = _create_task(tmp_project, "cr-log", "code_review:awaiting_approval") + _run_status(["--approve", "--task", "cr-log"], tmp_project) + approvals = (task_dir / ".state.approvals").read_text() + assert "code_review:approved" in approvals + + def test_code_review_not_editable(self, tmp_project): + _create_task(tmp_project, "cr-noedit", "code_review") + out, code = _run_status(["--can-edit", "--task", "cr-noedit"], tmp_project) + assert code == 1 + assert "DENIED" in out + + def test_code_review_forbids_bug_report_artifact(self, tmp_project): + task_dir = _create_task(tmp_project, "cr-forbidden", "code_review") + (task_dir / "BUG_REPORT.md").write_text("# Bugs") + out, code = _run_status(["--validate-folder", "--task", "cr-forbidden"], tmp_project) + assert code == 1 + assert "BUG_REPORT.md" in out + + +class TestImplementerTracking: + def test_transition_records_implementer_when_lock_exists(self, tmp_project): + task_dir = _create_task(tmp_project, "impl-track", "implement") + (task_dir / "IMPLEMENTATION.md").write_text("# Impl") + lock_file = task_dir / ".state.lock" + from datetime import datetime, timedelta, timezone + now = datetime.now(timezone.utc) + expires = now + timedelta(minutes=30) + lock_file.write_text(f"agent: builder-agent\nphase: implement\nclaimed: {now.isoformat()}\nexpires: {expires.isoformat()}\n") + out, code = _run_status(["--task", "impl-track", "--transition", "code_review"], tmp_project) + assert code == 0 + impl_file = task_dir / ".state.implementer" + assert impl_file.exists() + assert impl_file.read_text().strip() == "builder-agent" + + def test_no_lock_does_not_block_transition(self, tmp_project): + task_dir = _create_task(tmp_project, "impl-nolock", "implement") + (task_dir / "IMPLEMENTATION.md").write_text("# Impl") + out, code = _run_status(["--task", "impl-nolock", "--transition", "code_review"], tmp_project) + assert code == 0 + assert not (task_dir / ".state.implementer").exists() + + +class TestReviewerEnforcement: + def test_same_implementer_cannot_claim_code_review(self, tmp_project): + task_dir = _create_task(tmp_project, "same-reviewer", "code_review") + (task_dir / ".state.implementer").write_text("builder-agent\n") + agent_file = tmp_project / ".automaton" / ".agent.md" + agent_file.write_text("""## Agent Configuration +Mode: multi-agent +Agents: + - id: builder-agent + phases: [code_review, implement] +Lock timeout: 30m +""") + out, code = _run_status(["--claim", "--task", "same-reviewer", "--agent", "builder-agent"], tmp_project) + assert code == 1 + assert "cannot claim" in out + + def test_different_reviewer_can_claim_code_review(self, tmp_project): + task_dir = _create_task(tmp_project, "diff-reviewer", "code_review") + (task_dir / ".state.implementer").write_text("builder-agent\n") + agent_file = tmp_project / ".automaton" / ".agent.md" + agent_file.write_text("""## Agent Configuration +Mode: multi-agent +Agents: + - id: reviewer-agent + phases: [code_review] +Lock timeout: 30m +""") + out, code = _run_status(["--claim", "--task", "diff-reviewer", "--agent", "reviewer-agent"], tmp_project) + assert code == 0 + assert "Claimed" in out + + +class TestCodeReviewInference: + def test_code_review_artifact_infers_bug_find(self, tmp_project): + task_dir = tmp_project / ".automaton" / "tasks" / "cr-infer" + task_dir.mkdir() + (task_dir / "CODE_REVIEW.md").write_text("# Review") + out, code = _run_status(["--upgrade", "--task", "cr-infer"], tmp_project) + assert code == 0 + state = (task_dir / ".state").read_text().strip() + assert state == "bug_find" + + def test_impl_and_code_review_infers_bug_find(self, tmp_project): + task_dir = tmp_project / ".automaton" / "tasks" / "cr-plus-impl" + task_dir.mkdir() + (task_dir / "IMPLEMENTATION.md").write_text("# Impl") + (task_dir / "CODE_REVIEW.md").write_text("# Review") + out, code = _run_status(["--upgrade", "--task", "cr-plus-impl"], tmp_project) + assert code == 0 + state = (task_dir / ".state").read_text().strip() + assert state == "bug_find" + + +class TestCodeReviewShowTask: + def test_show_task_code_review(self, tmp_project): + _create_task(tmp_project, "cr-show", "code_review") + out, code = _run_status(["--task", "cr-show"], tmp_project) + assert code == 0 + assert "Phase: code_review" in out + assert "Next artifact needed: CODE_REVIEW.md" in out + + +class TestCodeReviewListStates: + def test_list_states_includes_code_review(self, tmp_project): + out, code = _run_status(["--list-states"], tmp_project) + assert code == 0 + assert "code_review * requires approval" in out + assert "code_review:awaiting_approval" in out + assert "code_review:approved" in out \ No newline at end of file diff --git a/tests/test_task.py b/tests/test_task.py index a8ee355..a193f03 100644 --- a/tests/test_task.py +++ b/tests/test_task.py @@ -45,7 +45,7 @@ def test_implementation_state(tmp_path: Path) -> None: {"SPEC.md": "# Spec", "TEST_PLAN.md": "# Tests", "IMPLEMENTATION.md": "# Impl"}, ) state, _ = determine_task_state(task_dir) - assert state == TaskState.BUG_FIND + assert state == TaskState.IMPLEMENT def test_implementation_from_test_plan(tmp_path: Path) -> None: @@ -196,10 +196,10 @@ class TestVerdictParsing: class TestStateMachineAlignment: """Tests for state machine alignment with orchestrate.md (R4).""" - def test_implementation_alone_shows_bug_find(self, tmp_path: Path) -> None: + def test_implementation_alone_shows_implement(self, tmp_path: Path) -> None: task_dir = _make_task(tmp_path, "impl-only", {"IMPLEMENTATION.md": "# Impl"}) state, _ = determine_task_state(task_dir) - assert state == TaskState.BUG_FIND + assert state == TaskState.IMPLEMENT def test_bug_report_without_adversarial(self, tmp_path: Path) -> None: task_dir = _make_task(