Skip to content

Conversation

@Fguedes90
Copy link

@Fguedes90 Fguedes90 commented Dec 28, 2025

Summary

This PR introduces a sophisticated Code Reviewer Agent with multiple specialized personas and refactors the internal agent factory system to be more type-safe and extensible. It enables targeted code reviews (e.g., focusing solely on error handling or type design) and adds a new workflow command.

📐 Architectural Highlights: Open/Closed Principle

This PR significantly refactors the agent configuration system to respect the Open/Closed Principle, moving away from a rigid "one-size-fits-all" approach:

  • Generic Factory Pattern: The AgentFactory is now generic (AgentFactory<TOptions>). This allows new agents to introduce unique configuration requirements (like code_reviewer_mode) without polluting the global AgentConfig type or requiring modifications to the core factory logic.
  • Polymorphic Configuration: Utilized Mapped Types in AgentOverrides to enforce strict type safety. Specific configurations are isolated to their respective agents (e.g., the compiler prevents passing code_reviewer_mode to oracle).
  • Extensibility: The system is now closed for modification (core types remain stable) but open for extension (new agents can define arbitrary config structures cleanly).

🔧 Extensibility Example

Here is how you would add a hypothetical DatabaseOptimizer agent with custom options without modifying the core logic:

  1. Define Options:
interface DatabaseOptimizerOptions extends BaseAgentOptions {
  dialect: "postgres" | "mysql" | "sqlite";
}
  1. Extend Overrides:
type AgentOverrides = {
  // ... other agents
  "database-optimizer"?: BaseAgentOverrideConfig & DatabaseOptimizerOptions;
}
  1. Implement Factory:
const createOptimizerAgent: AgentFactory<DatabaseOptimizerOptions> = (model, options) => {
  const dialect = options?.dialect ?? "postgres";
  return {
    // ... config using dialect-specific prompt
  };
}

No changes to createBuiltinAgents loop or the global AgentConfig type are required!

📊 Visual Documentation

1. Code Reviewer Personas

How the generic factory routes configuration to specialized prompt engineering:

graph TD
    In([Input: PR / Diff]) --> Router{"Agent Factory<br/>Config Resolver"}
    
    Router -->|Default| ModeA["<b>General Persona</b><br/>Check Project Guidelines<br/>Verify Style/Conventions"]
    Router -->|silent_failure_hunter| ModeB["<b>Silent Failure Hunter</b><br/>Find Empty Catch Blocks<br/>Audit Error Propagation"]
    Router -->|type_design_analyzer| ModeC["<b>Type Design Analyzer</b><br/>Check Invariants<br/>Evaluate Encapsulation"]
    Router -->|pr_test_analyzer| ModeD["<b>PR Test Analyzer</b><br/>Check Behavioral Coverage<br/>Identify Missing Edge Cases"]
    
    ModeA --> Out([Structured Review Comment])
    ModeB --> Out
    ModeC --> Out
    ModeD --> Out
Loading

2. Polymorphic Configuration Flow

Flowchart illustrating how the system handles agent-specific options without modifying the core loop:

graph TD
    Start([Load Configuration]) --> Loop{Iterate Agents}
    
    Loop -->|Standard Agent| GenFactory[Generic Factory]
    GenFactory --> DefaultPrompt[Use Default Prompt]
    
    Loop -->|Code Reviewer| CheckOpts{Check Options}
    CheckOpts -->|Has code_reviewer_mode| Extract[Extract Options]
    Extract --> SpecFactory[Specific Factory]
    SpecFactory --> Select{Select Persona}
    
    Select -->|silent_failure_hunter| PromptA[Load 'Silent Failure' Prompt]
    Select -->|type_design_analyzer| PromptB[Load 'Type Design' Prompt]
    Select -->|general| PromptC[Load 'General' Prompt]
    
    DefaultPrompt --> Merge([Merge Config & Register])
    PromptA --> Merge
    PromptB --> Merge
    PromptC --> Merge
    
    Merge --> Loop
Loading

🚀 New Workflow Command

/review-pr [url]

Why: To ensure code reviews are consistent, deep, and leverage the new specialized personas rather than just doing a generic "LGTM" check.
Flow:

  1. Context Fetching: Checks out the PR locally and fetches the diff.
  2. Multi-Perspective Analysis: Orchestrates a pass using the code-reviewer agent. It can specifically target "silent failures" or "type design" based on the nature of the PR, rather than a generic summary.
  3. Verification: Runs local builds/tests against the PR branch (something humans often skip).
  4. Reporting: Generates a structured review comment focusing on high-confidence issues.

Key Changes

🤖 New Code Reviewer Agent

  • Inspiration: The specialized personas and prompt engineering were heavily inspired by the official Anthropic PR Review Toolkit (source). We adapted their rigorous prompt structure (Guidelines Compliance, Bug Detection, Confidence Scoring) into our agent orchestration layer.
  • 4 Specialized Personas:
    • general: Comprehensive review against project guidelines (default).
    • silent_failure_hunter: ruthlessly finds swallowed errors and empty catch blocks.
    • type_design_analyzer: Evaluates type invariants, encapsulation, and domain modeling.
    • pr_test_analyzer: Assesses behavioral test coverage and gaps.
  • Configuration: Fully configurable via oh-my-opencode.json using the new code_reviewer_mode option.
  • Safety: Default permissions (edit/bash) are disabled for safety.

🏗️ Agent System Refactoring

  • Robustness: Added error handling around agent initialization to prevent one bad config from crashing the plugin.
  • Context Injection: Improved logic for injecting environment/directory context into agents.

📜 New Commands & Config

  • New Command: review-pr template added.
  • Schema: Updated Zod and JSON schemas to support the new agent and its options.

Testing

  • Added comprehensive tests for code-reviewer agent creation and mode selection.
  • Added tests for createBuiltinAgents refactoring and override logic.
  • Verified schema generation.

- Rename 'review' command to 'review-pr' to avoid conflicts with OpenCode builtins
- Implement consolidated PR review toolkit with 4 specialized analysis modes:
  * Safety Review (silent failure hunter)
  * Type Design Analysis (encapsulation & invariants)
  * Test Coverage Analysis (critical gaps & quality)
  * General Review (logic bugs & conventions)
- Orchestrate multiple code-reviewer personas for comprehensive code analysis
- Maintain feature-dev and init-deep builtin commands
- Update type definitions and command registry

This restores the advanced review capabilities from feat/integrate-claude-plugins branch while maintaining clean architecture with separate templates and proper TypeScript types.
- Fix environment context ordering: inject after mergeAgentConfig to prevent
  context loss when prompt is overridden
- Add explore agent to envContext recipients for better codebase awareness
- Remove legacy 'mode' option, keeping only 'persona' to avoid confusion
  with AgentConfig.mode property
- Add barrel exports for code-reviewer types in agents/index.ts
- Update AGENTS.md documentation to reflect grok-code as default model
- Add tests for environment context injection and prompt override behavior
…erride types

- Add BaseAgentOptions interface for extensible agent factory options
- Create generic AgentFactory<TOptions> type for type-safe factories
- Split AgentOverrideConfig into BaseAgentOverrideConfig and CodeReviewerOverrideConfig
- Update schema.ts to use agent-specific schemas (eliminates ghost fields)
- CodeReviewerOptions now extends BaseAgentOptions
- Add isCodeReviewerOverride type guard in utils.ts

This change ensures code_reviewer_mode is only valid for the code-reviewer
agent (previously it appeared on all agents as a 'ghost field').

Schema reduced by ~99 lines - code_reviewer_mode now appears only once.
@github-actions
Copy link
Contributor

github-actions bot commented Dec 28, 2025

All contributors have signed the CLA. Thank you! ✅
Posted by the CLA Assistant Lite bot.

@Fguedes90
Copy link
Author

I have read the CLA Document and I hereby sign the CLA

github-actions bot added a commit that referenced this pull request Dec 28, 2025
@Fguedes90 Fguedes90 changed the title feat: advanced code review workflows feat: advanced code review workflows & robust agent factory Dec 28, 2025
@Fguedes90 Fguedes90 marked this pull request as ready for review December 28, 2025 23:58
@greptile-apps
Copy link

greptile-apps bot commented Dec 29, 2025

Greptile Summary

This PR introduces a sophisticated code review system with a new code-reviewer agent and refactors the agent factory architecture to follow the Open/Closed Principle.

Key Changes:

  • Generic Agent Factory Pattern: The AgentFactory type is now generic (AgentFactory<TOptions>), enabling agents to define custom configuration without polluting the global config type. The code-reviewer agent uses this to introduce code_reviewer_mode option.
  • Code Reviewer Agent: New agent with 4 specialized personas inspired by Anthropic's PR Review Toolkit:
    • general: Comprehensive guideline compliance and bug detection (confidence scoring 0-100)
    • silent_failure_hunter: Ruthlessly audits error handling and catch blocks
    • type_design_analyzer: Evaluates type invariants and encapsulation (rates 1-10)
    • pr_test_analyzer: Assesses behavioral test coverage and identifies gaps
  • Type-Safe Configuration: AgentOverrides uses mapped types to enforce that code_reviewer_mode is only valid for code-reviewer, preventing configuration errors at compile-time
  • Enhanced Factory Logic: createBuiltinAgents now:
    • Passes agent-specific options to factories via isCodeReviewerOverride type guard
    • Wraps each agent initialization in try-catch to prevent one bad config from crashing the system
    • Injects environment context for code-reviewer and explore agents (previously only Sisyphus/librarian)
  • New /review-pr Command: Orchestrates multi-perspective PR analysis using all 4 code-reviewer personas
  • Comprehensive Testing: 129 lines in code-reviewer.test.ts + 166 lines in utils.test.ts covering mode selection, validation, overrides, environment context injection, and edge cases
  • Schema Updates: Zod schemas properly import CODE_REVIEWER_MODES from code-reviewer.ts for DRY (single source of truth)

Confidence Score: 5/5

  • This PR is exceptionally safe to merge with comprehensive testing, excellent architectural design, and no breaking changes
  • Score reflects: (1) Exemplary Open/Closed Principle implementation with generic factory pattern that allows extension without modification, (2) 295 lines of comprehensive test coverage covering all modes, edge cases, validation, and integration scenarios, (3) Proper error handling with try-catch around agent initialization and fallback to "general" mode for invalid configs, (4) Type-safe configuration using mapped types that prevents misuse at compile-time, (5) Well-structured code with clear separation of concerns, (6) Proper DRY principles with CODE_REVIEWER_MODES imported in schema.ts, (7) Backward-compatible changes with no modifications to existing agent behavior, (8) Documentation thoroughly updated across README, AGENTS.md, and agent-specific docs, (9) Four highly detailed, well-thought-out persona prompts inspired by Anthropic's official toolkit
  • No files require special attention - all changes are well-tested and architecturally sound

Important Files Changed

Filename Overview
src/agents/types.ts Introduces generic AgentFactory pattern with type-safe agent-specific options; excellent Open/Closed Principle implementation
src/agents/code-reviewer.ts New specialized code review agent with 4 personas and comprehensive prompts; properly validates modes and disables write/edit for safety
src/agents/utils.ts Refactored to support generic factories and agent-specific options; added error handling and environment context injection
src/agents/code-reviewer.test.ts Comprehensive tests covering all modes, validation, tool permissions, and edge cases
src/agents/utils.test.ts Thorough tests for factory system, code-reviewer integration, environment context injection, and override handling
src/config/schema.ts Updated Zod schemas to support code-reviewer modes and CodeReviewerOverrideConfigSchema; properly imports from code-reviewer for DRY
src/features/builtin-commands/templates/review-pr.ts New PR review workflow orchestrating multiple code-reviewer personas for comprehensive analysis

Sequence Diagram

sequenceDiagram
    participant User
    participant System as createBuiltinAgents
    participant Factory as AgentFactory<T>
    participant CodeReviewer as createCodeReviewerAgent
    participant Merge as mergeAgentConfig
    participant Env as createEnvContext

    User->>System: Load config with agent overrides
    
    rect rgb(240, 248, 255)
        Note over System: Loop: For each agent in agentSources
        
        System->>System: Check if agent disabled
        
        alt Agent is code-reviewer
            System->>System: Extract override config
            System->>System: Check isCodeReviewerOverride()
            System->>System: Build factoryOptions with code_reviewer_mode
            System->>CodeReviewer: createCodeReviewerAgent(model, factoryOptions)
            CodeReviewer->>CodeReviewer: Validate mode (isValidCodeReviewerMode)
            alt Invalid mode
                CodeReviewer->>CodeReviewer: Log warning, fallback to "general"
            end
            CodeReviewer->>CodeReviewer: Select prompt from CODE_REVIEWER_PROMPTS[mode]
            CodeReviewer-->>System: Return AgentConfig with selected prompt
        else Standard Agent
            System->>Factory: buildAgent(source, model, factoryOptions)
            Factory-->>System: Return base AgentConfig
        end
        
        alt Override exists
            System->>Merge: mergeAgentConfig(config, override)
            Merge->>Merge: Deep merge config
            Merge->>Merge: Append prompt_append if present
            Merge-->>System: Return merged config
        end
        
        alt Agent needs env context (Sisyphus, librarian, code-reviewer, explore)
            System->>Env: createEnvContext(directory)
            Env-->>System: Return env context string
            System->>System: Append env context to prompt
        end
        
        System->>System: Store config in result
    end
    
    System-->>User: Return Record<string, AgentConfig>
Loading

@greptile-apps
Copy link

greptile-apps bot commented Dec 29, 2025

Greptile's behavior is changing!

From now on, if a review finishes with no comments, we will not post an additional "statistics" comment to confirm that our review found nothing to comment on. However, you can confirm that we reviewed your changes in the status check section.

This feature can be toggled off in your Code Review Settings by deselecting "Create a status check for each PR".

builtbyrobben pushed a commit to builtbyrobben/oh-my-opencode that referenced this pull request Dec 29, 2025
Merged from Fguedes90/oh-my-opencode:feat/advanced-code-review-workflows

Features added:
- code-reviewer agent with 4 personas (general, silent_failure_hunter, type_design_analyzer, pr_test_analyzer)
- /review-pr builtin command
- Generic AgentFactory<TOptions> pattern for extensibility
- CodeReviewerOverrideConfig for mode-specific overrides

Integration with our fork:
- Both developer and code-reviewer agents now available
- ALLOWED_AGENTS updated to include code-reviewer
- All BuiltinAgentName types properly combined
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant