diff --git a/gateway/agent_restrictions.py b/gateway/agent_restrictions.py index 97ec630898..ba584f41e4 100644 --- a/gateway/agent_restrictions.py +++ b/gateway/agent_restrictions.py @@ -42,11 +42,15 @@ class AgentRole: ARCHITECT = "architect" TASK_PLANNER = "task_planner" RISK_ANALYST = "risk_analyst" + # Refine-phase roles + REFINER = "refiner" # Reviewer roles REVIEWER_UNIFIED = "reviewer_unified" REVIEWER_CODE = "reviewer_code" REVIEWER_CONTRACT = "reviewer_contract" REVIEWER_AGENT_DESIGN = "reviewer_agent_design" + REVIEWER_REFINE = "reviewer_refine" + REVIEWER_PLAN = "reviewer_plan" @dataclass @@ -415,6 +419,43 @@ def _matches_pattern(file_path: str, pattern: str) -> bool: blocked_patterns=_REVIEWER_BLOCKED, ) +# Refine-phase agent patterns + +REFINER_PATTERNS = AgentFilePattern( + role=AgentRole.REFINER, + description="Refiner agent: drafts and agent-outputs only", + allowed_patterns=[ + ".egg-state/drafts/", + ".egg-state/agent-outputs/", + ], + blocked_patterns=[ + # Source code (refiner must not modify code) + "**/*.py", + "**/*.ts", + "**/*.tsx", + "**/*.js", + "**/*.jsx", + "**/*.go", + "**/*.java", + # Contracts + ".egg-state/contracts/", + ], +) + +REVIEWER_REFINE_PATTERNS = AgentFilePattern( + role=AgentRole.REVIEWER_REFINE, + description="Refine reviewer agent: reviews and agent-outputs only", + allowed_patterns=_REVIEWER_ALLOWED, + blocked_patterns=_REVIEWER_BLOCKED, +) + +REVIEWER_PLAN_PATTERNS = AgentFilePattern( + role=AgentRole.REVIEWER_PLAN, + description="Plan reviewer agent: reviews and agent-outputs only", + allowed_patterns=_REVIEWER_ALLOWED, + blocked_patterns=_REVIEWER_BLOCKED, +) + # Registry of all agent patterns AGENT_PATTERNS: dict[str, AgentFilePattern] = { AgentRole.CODER: CODER_PATTERNS, @@ -428,6 +469,9 @@ def _matches_pattern(file_path: str, pattern: str) -> bool: AgentRole.REVIEWER_CODE: REVIEWER_CODE_PATTERNS, AgentRole.REVIEWER_CONTRACT: REVIEWER_CONTRACT_PATTERNS, AgentRole.REVIEWER_AGENT_DESIGN: REVIEWER_AGENT_DESIGN_PATTERNS, + AgentRole.REFINER: REFINER_PATTERNS, + AgentRole.REVIEWER_REFINE: REVIEWER_REFINE_PATTERNS, + AgentRole.REVIEWER_PLAN: REVIEWER_PLAN_PATTERNS, } diff --git a/integration_tests/sdlc/test_multi_agent_orchestration.py b/integration_tests/sdlc/test_multi_agent_orchestration.py index f027651c07..adde3b63b8 100644 --- a/integration_tests/sdlc/test_multi_agent_orchestration.py +++ b/integration_tests/sdlc/test_multi_agent_orchestration.py @@ -35,7 +35,7 @@ class TestDependencyGraph: def test_build_graph_all_roles(self): """Build graph includes all agent roles.""" graph = build_dependency_graph() - assert len(graph.nodes) == 11 + assert len(graph.nodes) == 14 assert AgentRole.CODER in graph.nodes assert AgentRole.TESTER in graph.nodes assert AgentRole.DOCUMENTER in graph.nodes @@ -43,6 +43,7 @@ def test_build_graph_all_roles(self): assert AgentRole.ARCHITECT in graph.nodes assert AgentRole.TASK_PLANNER in graph.nodes assert AgentRole.RISK_ANALYST in graph.nodes + assert AgentRole.REFINER in graph.nodes def test_build_graph_implement_roles(self): """Build graph with implement-phase roles only.""" @@ -543,6 +544,88 @@ def test_get_roles_with_reviewers(self): assert AgentRole.REVIEWER_UNIFIED in roles assert AgentRole.REVIEWER_CODE in roles + def test_get_plan_roles_with_reviewers(self): + """Get plan roles with reviewers included.""" + from egg_contracts.agent_roles import get_roles_for_phase + + roles = get_roles_for_phase("plan", include_reviewers=True) + assert len(roles) == 6 # 3 plan + 3 reviewers + assert AgentRole.ARCHITECT in roles + assert AgentRole.TASK_PLANNER in roles + assert AgentRole.RISK_ANALYST in roles + assert AgentRole.REVIEWER_UNIFIED in roles + assert AgentRole.REVIEWER_AGENT_DESIGN in roles + assert AgentRole.REVIEWER_PLAN in roles + + def test_reviewer_plan_role_definition(self): + """Reviewer plan role has correct properties.""" + from egg_contracts.agent_roles import get_role_definition + + role_def = get_role_definition(AgentRole.REVIEWER_PLAN) + assert role_def.role == AgentRole.REVIEWER_PLAN + assert AgentRole.TASK_PLANNER in role_def.dependencies + assert AgentRole.RISK_ANALYST in role_def.dependencies + assert ".egg-state/reviews/" in role_def.file_access.allowed_write + assert role_def.file_access.can_write(".egg-state/reviews/verdict.json") + assert not role_def.file_access.can_write("src/main.py") + + +class TestRefinePhaseRoles: + """Tests for refine-phase agent roles.""" + + def test_get_roles_for_refine_phase(self): + """Get roles for refine phase returns refiner.""" + from egg_contracts.agent_roles import get_roles_for_phase + + roles = get_roles_for_phase("refine") + assert len(roles) == 1 + assert AgentRole.REFINER in roles + + def test_get_refine_roles_with_reviewers(self): + """Get refine roles with reviewers included.""" + from egg_contracts.agent_roles import get_roles_for_phase + + roles = get_roles_for_phase("refine", include_reviewers=True) + assert len(roles) == 3 # 1 refiner + 2 reviewers + assert AgentRole.REFINER in roles + assert AgentRole.REVIEWER_REFINE in roles + assert AgentRole.REVIEWER_AGENT_DESIGN in roles + + def test_refine_phase_dependency_graph(self): + """Refine-phase has simple single-agent dependency structure.""" + from egg_contracts.agent_roles import get_roles_for_phase + + roles = get_roles_for_phase("refine") + graph = build_dependency_graph(roles) + + assert len(graph.nodes) == 1 + waves = graph.compute_waves() + + # Wave 1: refiner (no dependencies) + assert AgentRole.REFINER in waves[0] + + def test_refiner_role_definition(self): + """Refiner role has correct properties.""" + from egg_contracts.agent_roles import get_role_definition + + role_def = get_role_definition(AgentRole.REFINER) + assert role_def.role == AgentRole.REFINER + assert role_def.dependencies == [] + assert ".egg-state/drafts/" in role_def.file_access.allowed_write + assert role_def.file_access.can_write(".egg-state/drafts/123-analysis.md") + assert not role_def.file_access.can_write("src/main.py") + + def test_reviewer_refine_role_definition(self): + """Reviewer refine role has correct properties.""" + from egg_contracts.agent_roles import get_role_definition + + role_def = get_role_definition(AgentRole.REVIEWER_REFINE) + assert role_def.role == AgentRole.REVIEWER_REFINE + assert AgentRole.REFINER in role_def.dependencies + assert ".egg-state/reviews/" in role_def.file_access.allowed_write + assert role_def.file_access.can_write(".egg-state/reviews/verdict.json") + assert not role_def.file_access.can_write("src/main.py") + class TestReviewerRoles: """Tests for reviewer agent roles.""" @@ -573,6 +656,8 @@ def test_reviewer_roles_read_only(self): AgentRole.REVIEWER_CODE, AgentRole.REVIEWER_CONTRACT, AgentRole.REVIEWER_AGENT_DESIGN, + AgentRole.REVIEWER_REFINE, + AgentRole.REVIEWER_PLAN, ]: role_def = get_role_definition(role) # Reviewers can write to reviews and agent-outputs only @@ -625,7 +710,7 @@ def test_default_multi_agent_config(self): config = MultiAgentConfig() assert config.enabled is True assert config.parallel_execution is True - assert len(config.roles_enabled) == 11 # All roles + assert len(config.roles_enabled) == 14 # All roles assert len(config.phase_overrides) == 0 def test_phase_override(self): diff --git a/orchestrator/models.py b/orchestrator/models.py index cb9ea9fcbc..5a9b2a6a23 100644 --- a/orchestrator/models.py +++ b/orchestrator/models.py @@ -74,11 +74,15 @@ class AgentRole(StrEnum): ARCHITECT = "architect" TASK_PLANNER = "task_planner" RISK_ANALYST = "risk_analyst" + # Refine-phase roles + REFINER = "refiner" # Reviewer roles (specific subtypes) REVIEWER_UNIFIED = "reviewer_unified" REVIEWER_CODE = "reviewer_code" REVIEWER_CONTRACT = "reviewer_contract" REVIEWER_AGENT_DESIGN = "reviewer_agent_design" + REVIEWER_REFINE = "reviewer_refine" + REVIEWER_PLAN = "reviewer_plan" class ReviewerType(StrEnum): diff --git a/orchestrator/routes/pipelines.py b/orchestrator/routes/pipelines.py index 9fe3d8a276..16374f7240 100644 --- a/orchestrator/routes/pipelines.py +++ b/orchestrator/routes/pipelines.py @@ -834,6 +834,66 @@ def _get_contract_review_criteria() -> str: +def _get_refine_review_criteria() -> str: + """Return review criteria for the dedicated refine reviewer.""" + return ( + "### 1. Problem Understanding\n" + "- Does the analysis correctly identify the core problem or feature request?\n" + "- Is the current behavior (if applicable) accurately described?\n" + "- Are the goals and desired outcomes clear?\n\n" + "### 2. Research Quality\n" + "- Has the agent explored the relevant parts of the codebase?\n" + "- Are existing patterns and conventions identified?\n" + "- Is the technical context accurate and thorough?\n\n" + "### 3. Options Analysis\n" + "- Are the proposed options meaningfully different?\n" + "- Are trade-offs clearly articulated for each option?\n" + "- Is the reasoning logical and well-founded?\n\n" + "### 4. Constraints and Dependencies\n" + "- Are technical constraints identified (performance, compatibility, etc.)?\n" + "- Are dependencies on other code or systems noted?\n" + "- Are potential risks or complications surfaced?\n\n" + "### 5. Open Questions\n" + "- Are open questions specific enough for a human to answer?\n" + "- Do questions address genuine ambiguities?\n" + "- Are questions actionable?\n\n" + "### 6. Recommendation Quality\n" + "- Is there a clear recommended approach?\n" + "- Is the recommendation justified with specific reasons?\n" + "- Does the recommendation align with the analysis findings?\n" + ) + + +def _get_plan_review_criteria() -> str: + """Return review criteria for the dedicated plan reviewer.""" + return ( + "### 1. Task Breakdown\n" + "- Are tasks discrete, actionable, and properly scoped?\n" + "- Is each task small enough to implement in a single pass?\n" + "- Are task boundaries clear (no overlapping responsibilities)?\n\n" + "### 2. Acceptance Criteria\n" + "- Does each task have clear, testable acceptance criteria?\n" + "- Are criteria specific enough to verify completion?\n" + "- Do criteria cover both happy path and edge cases?\n\n" + "### 3. Dependency Ordering\n" + "- Are task dependencies correctly identified?\n" + "- Is the ordering logical (foundations before features)?\n" + "- Are there opportunities for parallelism that are missed?\n\n" + "### 4. Risk Assessment\n" + "- Are technical risks identified (security, performance, compatibility)?\n" + "- Are mitigation strategies concrete and actionable?\n" + "- Is the rollback plan realistic?\n\n" + "### 5. Test Strategy\n" + "- Is the test strategy appropriate for the scope of changes?\n" + "- Are both unit and integration tests considered?\n" + "- Are test scenarios aligned with acceptance criteria?\n\n" + "### 6. Completeness\n" + "- Does the plan cover all aspects of the original request?\n" + "- Are documentation updates included where needed?\n" + "- Are there any obvious gaps or missing tasks?\n" + ) + + def _get_review_criteria_for_type(reviewer_type: str, phase: str) -> str: """Dispatch to the correct criteria function based on reviewer type.""" if reviewer_type == "unified": @@ -844,6 +904,10 @@ def _get_review_criteria_for_type(reviewer_type: str, phase: str) -> str: return _get_code_review_criteria() elif reviewer_type == "contract": return _get_contract_review_criteria() + elif reviewer_type == "refine": + return _get_refine_review_criteria() + elif reviewer_type == "plan": + return _get_plan_review_criteria() else: return _get_unified_criteria(phase) @@ -871,6 +935,20 @@ def _get_reviewer_scope_preamble(reviewer_type: str, phase: str) -> str: "matches the contract and all acceptance criteria are met. Do NOT review " "general code quality or security — other reviewers handle those." ) + elif reviewer_type == "refine": + return ( + "This is a **refine phase review**. Focus on the quality and completeness " + "of the analysis produced during the refine phase. Evaluate problem " + "understanding, codebase research, options analysis, and the recommended " + "approach. Agent-mode design alignment is handled by another reviewer." + ) + elif reviewer_type == "plan": + return ( + "This is a **plan phase review**. Focus on the quality and completeness " + "of the implementation plan. Evaluate task breakdown, acceptance criteria, " + "dependency ordering, risk assessment, and test strategy. Agent-mode " + "design alignment is handled by another reviewer." + ) return "" @@ -1211,32 +1289,92 @@ def _build_phase_prompt( if phase == "refine": lines.extend( [ - "Analyze the task and produce a structured analysis:", - "", + "Analyze this issue and produce a structured analysis document. " + "Your goal is to:\n", "1. Understand the problem or feature request", "2. Research the current codebase to understand existing patterns", "3. Identify constraints and dependencies", "4. Consider multiple implementation approaches", "5. Recommend an approach with justification", + "6. Surface any questions that need human input", + "", + "**IMPORTANT**: Do NOT create an implementation plan, task breakdown, " + "or phased rollout. That is the **plan** phase's job. Stay focused on " + "**analysis**: understanding the problem, researching the codebase, " + "evaluating options, and surfacing decisions for the human.", + "", + "## Output Format\n", + "Create an analysis document following this template:\n", + "```markdown", + "# Analysis: [Issue Title]\n", + "> Issue: #[number] | Phase: refine\n", + "## Problem Statement\n", + "[Describe the problem or feature request. " + "What is the current state? What is the desired outcome?]\n", + "## Current Behavior\n", + "[Describe how the system currently works in the relevant area. " + "Include code references where helpful.]\n", + "## Constraints\n", + "- [Technical constraints (compatibility, performance, security)]", + "- [Business constraints (timeline, scope)]", + "- [Dependencies on other systems or features]\n", + "## Options Considered\n", + "### Option A: [Name]\n", + "**Approach**: [Brief description]\n", + "**Pros**:", + "- [Advantage 1]\n", + "**Cons**:", + "- [Disadvantage 1]\n", + "### Option B: [Name]\n", + "**Approach**: [Brief description]\n", + "**Pros**:", + "- [Advantage 1]\n", + "**Cons**:", + "- [Disadvantage 1]\n", + "## Recommended Approach\n", + "[Which option is recommended and why. " + "Reference the option above.]\n", + "## Open Questions\n", + "[Questions that require human input before proceeding.]\n", + "---\n", + "*Authored-by: egg*", + "```\n", + "## HITL Decisions\n", + "For questions that require human input before proceeding:\n", + "**Multiple-choice questions** (use formal HITL decisions):", + "```bash", + 'egg-contract add-decision --question "Which approach should we use?" \\', + ' --options "Option A" "Option B" "Option C" --format markdown', + "```", + "Copy the markdown output into your analysis. The human can check " + "a checkbox to select an option. An \"Other (explain in reply)\" " + "option is auto-appended.\n", + "**Open-ended questions** (use dedicated feedback comment):", + "```bash", + "egg-contract add-feedback \\", + ' --question "What is the expected request volume?" \\', + ' --question "Are there any constraints on third-party dependencies?" \\', + " --format markdown", + "```", + "This creates a dedicated comment for the human to fill in answers. " + "They edit the comment to add their responses and check \"Submit " + "feedback\" when done. The pipeline will resume with the feedback " + "available in the contract.", + "", + ] + ) + lines.extend( + [ + f"Write your analysis to `{analysis_path}`.", + "Commit and push the draft when done.\n", + "**IMPORTANT**: Do NOT post your analysis directly to the issue. " + "The pipeline will have an internal reviewer check your analysis. " + "If revisions are needed, you'll be re-invoked with feedback. " + "Only after internal review passes will the analysis be posted " + "for human approval.", "", ] ) - if is_local: - lines.extend( - [ - f"Write your analysis to `{analysis_path}`.", - "Commit and push the draft when done.", - "", - ] - ) - else: - lines.extend( - [ - f"Write your analysis to `{analysis_path}`.", - "Commit and push the draft when done.", - "", - ] - ) elif phase == "plan": lines.extend( @@ -1315,6 +1453,8 @@ def _build_phase_prompt( [ "In this phase:", "- You CAN push state files to git (contracts, drafts, checkpoints)", + "- You CAN create HITL decisions (egg-contract add-decision)", + "- You CAN create feedback requests (egg-contract add-feedback)", "- You CANNOT push code changes", "- You CANNOT create PRs (gh pr create)", "- You CANNOT post issue comments", @@ -1354,6 +1494,10 @@ def _build_phase_prompt( [ "- You CAN write drafts to `.egg-state/drafts/`", "- You CAN push draft files (git push)", + "- You CAN create HITL decisions (egg-contract add-decision)", + "- You CAN create feedback requests (egg-contract add-feedback)", + "- You CANNOT post analysis/plan directly to the issue " + "(internal review must pass first)", "- You CANNOT create PRs (gh pr create)", "", ] @@ -1379,10 +1523,19 @@ def _build_phase_prompt( # --- Completion --- lines.append("## Phase Completion\n") - lines.append( - "When you have completed your work for this phase, " - "ensure everything is committed and exit successfully." - ) + if phase in ("refine", "plan"): + lines.append( + "When your draft is complete, commit and push it. " + "The pipeline will have an internal reviewer evaluate your work. " + "If revisions are needed, you'll be re-invoked with feedback. " + "Only after internal review passes will the output be posted " + "for human approval." + ) + else: + lines.append( + "When you have completed your work for this phase, " + "ensure everything is committed and exit successfully." + ) return "\n".join(lines) @@ -1429,8 +1582,9 @@ def _build_agent_prompt( Returns: Complete prompt string for the agent """ - # CODER uses the existing phase prompt - if role_value == "coder": + # CODER and REFINER use the existing phase prompt (phase-specific + # instructions are already tailored for refine vs implement etc.) + if role_value in ("coder", "refiner"): return _build_phase_prompt( phase=phase, pipeline_id=pipeline_id, @@ -2583,11 +2737,19 @@ def _run_pipeline(pipeline_id: str, repo_path: Path) -> None: phase_prompt, ] + # Use the REFINER role for the refine phase, + # CODER for all other single-agent phases. + single_agent_role = ( + AgentRole.REFINER + if current_phase.value == "refine" + else AgentRole.CODER + ) + try: exit_code, container_logs = _spawn_and_wait( spawner=spawner, pipeline_id=pipeline_id, - agent_role=AgentRole.CODER, + agent_role=single_agent_role, issue_number=pipeline.issue_number, repo_volumes=repo_volumes, gateway_mode=phase_gateway_mode, diff --git a/orchestrator/tests/test_models.py b/orchestrator/tests/test_models.py index f8f82b2a43..6d16c3d4d6 100644 --- a/orchestrator/tests/test_models.py +++ b/orchestrator/tests/test_models.py @@ -359,11 +359,14 @@ def test_all_roles(self): assert AgentRole.ARCHITECT in roles assert AgentRole.TASK_PLANNER in roles assert AgentRole.RISK_ANALYST in roles + assert AgentRole.REFINER in roles assert AgentRole.REVIEWER_UNIFIED in roles assert AgentRole.REVIEWER_CODE in roles assert AgentRole.REVIEWER_CONTRACT in roles assert AgentRole.REVIEWER_AGENT_DESIGN in roles - assert len(roles) == 13 + assert AgentRole.REVIEWER_REFINE in roles + assert AgentRole.REVIEWER_PLAN in roles + assert len(roles) == 16 class TestPipelinePhase: diff --git a/shared/egg_contracts/agent_roles.py b/shared/egg_contracts/agent_roles.py index 542723d0b5..e65dbb0022 100644 --- a/shared/egg_contracts/agent_roles.py +++ b/shared/egg_contracts/agent_roles.py @@ -10,6 +10,7 @@ - TESTER: Writes tests for the implemented changes - DOCUMENTER: Updates documentation for the changes - INTEGRATOR: Runs full test suite and validates integration +- REFINER: Analyzes tasks and produces structured analysis in the refine phase The orchestrator uses these definitions to: 1. Determine execution order based on dependencies @@ -30,7 +31,9 @@ class AgentRole(StrEnum): Implement-phase roles: CODER, TESTER, DOCUMENTER, INTEGRATOR Plan-phase roles: ARCHITECT, TASK_PLANNER, RISK_ANALYST - Reviewer roles: REVIEWER_UNIFIED, REVIEWER_CODE, REVIEWER_CONTRACT, REVIEWER_AGENT_DESIGN + Refine-phase roles: REFINER + Reviewer roles: REVIEWER_UNIFIED, REVIEWER_CODE, REVIEWER_CONTRACT, + REVIEWER_AGENT_DESIGN, REVIEWER_REFINE, REVIEWER_PLAN """ CODER = "coder" @@ -41,11 +44,15 @@ class AgentRole(StrEnum): ARCHITECT = "architect" TASK_PLANNER = "task_planner" RISK_ANALYST = "risk_analyst" + # Refine-phase roles + REFINER = "refiner" # Reviewer roles REVIEWER_UNIFIED = "reviewer_unified" REVIEWER_CODE = "reviewer_code" REVIEWER_CONTRACT = "reviewer_contract" REVIEWER_AGENT_DESIGN = "reviewer_agent_design" + REVIEWER_REFINE = "reviewer_refine" + REVIEWER_PLAN = "reviewer_plan" class AgentStatus(StrEnum): @@ -411,6 +418,43 @@ def depends_on(self, other: AgentRole) -> bool: requires_inputs=["architecture_analysis"], ) + +# Refine-phase agent role definitions + +REFINER_ROLE = AgentRoleDefinition( + role=AgentRole.REFINER, + description="Analyzes the task and produces a structured analysis in the refine phase", + responsibilities=[ + "Understand the problem or feature request", + "Research the current codebase to understand existing patterns", + "Identify constraints and dependencies", + "Consider multiple implementation approaches with pros/cons", + "Recommend an approach with justification", + "Surface open questions as HITL decisions or feedback requests", + "Write analysis to the draft file (NOT an implementation plan)", + ], + dependencies=[], # Refiner runs first, no dependencies + file_access=FileAccessPattern( + allowed_read=[], # Can read all files + allowed_write=[ + ".egg-state/drafts/", + ".egg-state/agent-outputs/", + ], + blocked_write=[ + "**/*.py", + "**/*.ts", + "**/*.tsx", + "**/*.js", + "**/*.jsx", + "**/*.go", + "**/*.java", + ".egg-state/contracts/", + ], + ), + produces_outputs=["analysis_draft"], + requires_inputs=[], +) + # Reviewer agent role definitions # Reviewers can only write to reviews/ and agent-outputs/ directories. # Use directory-based blocks instead of "**/*" which breaks can_write() @@ -525,6 +569,52 @@ def depends_on(self, other: AgentRole) -> bool: requires_inputs=["integration_report"], ) +REVIEWER_REFINE_ROLE = AgentRoleDefinition( + role=AgentRole.REVIEWER_REFINE, + description="Reviews refine phase analysis quality and completeness", + responsibilities=[ + "Verify the analysis correctly identifies the core problem", + "Assess research quality and codebase exploration", + "Evaluate options analysis and trade-off reasoning", + "Check that constraints and dependencies are identified", + "Validate the recommendation is justified and actionable", + ], + dependencies=[AgentRole.REFINER], + file_access=FileAccessPattern( + allowed_read=[], + allowed_write=[ + ".egg-state/reviews/", + ".egg-state/agent-outputs/", + ], + blocked_write=_REVIEWER_BLOCKED_WRITE, + ), + produces_outputs=["review_verdict"], + requires_inputs=["analysis_draft"], +) + +REVIEWER_PLAN_ROLE = AgentRoleDefinition( + role=AgentRole.REVIEWER_PLAN, + description="Reviews plan phase output quality and completeness", + responsibilities=[ + "Verify task breakdown is discrete, actionable, and properly scoped", + "Assess acceptance criteria clarity and testability", + "Evaluate dependency ordering between tasks", + "Check that risks and mitigations are identified", + "Validate test strategy coverage", + ], + dependencies=[AgentRole.TASK_PLANNER, AgentRole.RISK_ANALYST], + file_access=FileAccessPattern( + allowed_read=[], + allowed_write=[ + ".egg-state/reviews/", + ".egg-state/agent-outputs/", + ], + blocked_write=_REVIEWER_BLOCKED_WRITE, + ), + produces_outputs=["review_verdict"], + requires_inputs=["task_breakdown", "risk_assessment"], +) + # Registry of all agent roles AGENT_ROLES: dict[AgentRole, AgentRoleDefinition] = { @@ -537,11 +627,15 @@ def depends_on(self, other: AgentRole) -> bool: AgentRole.ARCHITECT: ARCHITECT_ROLE, AgentRole.TASK_PLANNER: TASK_PLANNER_ROLE, AgentRole.RISK_ANALYST: RISK_ANALYST_ROLE, + # Refine-phase roles + AgentRole.REFINER: REFINER_ROLE, # Reviewer roles AgentRole.REVIEWER_UNIFIED: REVIEWER_UNIFIED_ROLE, AgentRole.REVIEWER_CODE: REVIEWER_CODE_ROLE, AgentRole.REVIEWER_CONTRACT: REVIEWER_CONTRACT_ROLE, AgentRole.REVIEWER_AGENT_DESIGN: REVIEWER_AGENT_DESIGN_ROLE, + AgentRole.REVIEWER_REFINE: REVIEWER_REFINE_ROLE, + AgentRole.REVIEWER_PLAN: REVIEWER_PLAN_ROLE, } @@ -644,6 +738,7 @@ def can_retry(self, max_retries: int = 2) -> bool: _PHASE_ROLES: dict[str, list[AgentRole]] = { "implement": [AgentRole.CODER, AgentRole.TESTER, AgentRole.DOCUMENTER, AgentRole.INTEGRATOR], "plan": [AgentRole.ARCHITECT, AgentRole.TASK_PLANNER, AgentRole.RISK_ANALYST], + "refine": [AgentRole.REFINER], } _PHASE_REVIEWERS: dict[str, list[AgentRole]] = { @@ -656,9 +751,10 @@ def can_retry(self, max_retries: int = 2) -> bool: "plan": [ AgentRole.REVIEWER_UNIFIED, AgentRole.REVIEWER_AGENT_DESIGN, + AgentRole.REVIEWER_PLAN, ], "refine": [ - AgentRole.REVIEWER_UNIFIED, + AgentRole.REVIEWER_REFINE, AgentRole.REVIEWER_AGENT_DESIGN, ], } diff --git a/shared/egg_contracts/models.py b/shared/egg_contracts/models.py index 59be26d235..a859fa322e 100644 --- a/shared/egg_contracts/models.py +++ b/shared/egg_contracts/models.py @@ -321,11 +321,15 @@ class AgentRoleType(StrEnum): ARCHITECT = "architect" TASK_PLANNER = "task_planner" RISK_ANALYST = "risk_analyst" + # Refine-phase roles + REFINER = "refiner" # Reviewer roles REVIEWER_UNIFIED = "reviewer_unified" REVIEWER_CODE = "reviewer_code" REVIEWER_CONTRACT = "reviewer_contract" REVIEWER_AGENT_DESIGN = "reviewer_agent_design" + REVIEWER_REFINE = "reviewer_refine" + REVIEWER_PLAN = "reviewer_plan" class AgentExecutionModel(BaseModel): diff --git a/shared/egg_orchestrator/types.py b/shared/egg_orchestrator/types.py index 3535c23e06..0767a3fd6a 100644 --- a/shared/egg_orchestrator/types.py +++ b/shared/egg_orchestrator/types.py @@ -60,6 +60,19 @@ class AgentRole(StrEnum): TESTER = "tester" DOCUMENTER = "documenter" INTEGRATOR = "integrator" + # Plan-phase roles + ARCHITECT = "architect" + TASK_PLANNER = "task_planner" + RISK_ANALYST = "risk_analyst" + # Refine-phase roles + REFINER = "refiner" + # Reviewer roles + REVIEWER_UNIFIED = "reviewer_unified" + REVIEWER_CODE = "reviewer_code" + REVIEWER_CONTRACT = "reviewer_contract" + REVIEWER_AGENT_DESIGN = "reviewer_agent_design" + REVIEWER_REFINE = "reviewer_refine" + REVIEWER_PLAN = "reviewer_plan" @dataclass diff --git a/tests/gateway/test_agent_restrictions.py b/tests/gateway/test_agent_restrictions.py index 2a0f517d3c..639d34e474 100644 --- a/tests/gateway/test_agent_restrictions.py +++ b/tests/gateway/test_agent_restrictions.py @@ -198,3 +198,127 @@ def test_mixed_files_reports_blocked(self): assert not result.allowed assert len(result.blocked_files) == 1 assert ".egg-state/contracts/123.json" in result.blocked_files + + +class TestRefinerPatterns: + """Test REFINER_PATTERNS can_write enforcement at the gateway level. + + The refiner uses extension-based blocks (e.g. **/*.py) rather than + directory-based blocks, so these tests verify pattern matching works + for various source code extensions and edge cases. + """ + + def test_refiner_can_write_drafts(self): + """Refiner should be able to write to drafts directory.""" + pattern = get_agent_pattern("refiner") + assert pattern is not None + assert pattern.can_write(".egg-state/drafts/analysis.md") + assert pattern.can_write(".egg-state/drafts/refine-output.json") + + def test_refiner_can_write_agent_outputs(self): + """Refiner should be able to write to agent-outputs directory.""" + pattern = get_agent_pattern("refiner") + assert pattern is not None + assert pattern.can_write(".egg-state/agent-outputs/refiner-output.json") + + def test_refiner_blocked_from_source_code_extensions(self): + """Refiner must not write files with source code extensions.""" + pattern = get_agent_pattern("refiner") + assert pattern is not None + assert not pattern.can_write("src/module.py") + assert not pattern.can_write("lib/component.ts") + assert not pattern.can_write("lib/component.tsx") + assert not pattern.can_write("src/app.js") + assert not pattern.can_write("src/app.jsx") + assert not pattern.can_write("cmd/main.go") + assert not pattern.can_write("src/Main.java") + + def test_refiner_blocked_from_nested_source_files(self): + """Extension-based blocks should match at any directory depth.""" + pattern = get_agent_pattern("refiner") + assert pattern is not None + assert not pattern.can_write("deeply/nested/dir/module.py") + assert not pattern.can_write("a/b/c/d/file.ts") + + def test_refiner_blocked_from_contracts(self): + """Refiner should not be able to write to contracts directory.""" + pattern = get_agent_pattern("refiner") + assert pattern is not None + assert not pattern.can_write(".egg-state/contracts/123.json") + + def test_refiner_blocked_outside_allowed_directories(self): + """Refiner should not write to arbitrary directories.""" + pattern = get_agent_pattern("refiner") + assert pattern is not None + assert not pattern.can_write("README.md") + assert not pattern.can_write("docs/guide.md") + + +class TestReviewerRefinePatterns: + """Test REVIEWER_REFINE_PATTERNS can_write enforcement at the gateway level.""" + + def test_reviewer_refine_can_write_reviews(self): + """Reviewer refine should be able to write to reviews directory.""" + pattern = get_agent_pattern("reviewer_refine") + assert pattern is not None + assert pattern.can_write(".egg-state/reviews/refine-review.md") + + def test_reviewer_refine_can_write_agent_outputs(self): + """Reviewer refine should be able to write to agent-outputs.""" + pattern = get_agent_pattern("reviewer_refine") + assert pattern is not None + assert pattern.can_write(".egg-state/agent-outputs/review-output.json") + + def test_reviewer_refine_blocked_from_source(self): + """Reviewer refine should not write to source directories.""" + pattern = get_agent_pattern("reviewer_refine") + assert pattern is not None + assert not pattern.can_write("src/module.py") + assert not pattern.can_write("lib/utils.ts") + + def test_reviewer_refine_blocked_from_contracts(self): + """Reviewer refine should not write to contracts.""" + pattern = get_agent_pattern("reviewer_refine") + assert pattern is not None + assert not pattern.can_write(".egg-state/contracts/123.json") + + def test_reviewer_refine_blocked_from_drafts(self): + """Reviewer refine should not write to drafts (only refiner can).""" + pattern = get_agent_pattern("reviewer_refine") + assert pattern is not None + assert not pattern.can_write(".egg-state/drafts/analysis.md") + + +class TestReviewerPlanPatterns: + """Test REVIEWER_PLAN_PATTERNS can_write enforcement at the gateway level.""" + + def test_reviewer_plan_can_write_reviews(self): + """Reviewer plan should be able to write to reviews directory.""" + pattern = get_agent_pattern("reviewer_plan") + assert pattern is not None + assert pattern.can_write(".egg-state/reviews/plan-review.md") + + def test_reviewer_plan_can_write_agent_outputs(self): + """Reviewer plan should be able to write to agent-outputs.""" + pattern = get_agent_pattern("reviewer_plan") + assert pattern is not None + assert pattern.can_write(".egg-state/agent-outputs/review-output.json") + + def test_reviewer_plan_blocked_from_source(self): + """Reviewer plan should not write to source directories.""" + pattern = get_agent_pattern("reviewer_plan") + assert pattern is not None + assert not pattern.can_write("src/module.py") + assert not pattern.can_write("lib/utils.ts") + + def test_reviewer_plan_blocked_from_contracts(self): + """Reviewer plan should not write to contracts.""" + pattern = get_agent_pattern("reviewer_plan") + assert pattern is not None + assert not pattern.can_write(".egg-state/contracts/123.json") + + def test_reviewer_plan_blocked_from_drafts(self): + """Reviewer plan should not write to drafts.""" + pattern = get_agent_pattern("reviewer_plan") + assert pattern is not None + assert not pattern.can_write(".egg-state/drafts/plan.md")