Skip to content

Add ONEX Opus Nightly and Baseline Review System with Full Pipeline - #17

Closed
jonahgabriel wants to merge 1 commit into
mainfrom
terragon/onex-opus-nightly-baseline-reviewer-l6r96b
Closed

jonahgabriel wants to merge 1 commit into
mainfrom
terragon/onex-opus-nightly-baseline-reviewer-l6r96b

Conversation

@jonahgabriel

Copy link
Copy Markdown
Collaborator

Summary

  • Introduces the ONEX Opus Nightly Review System for deterministic, diff-driven code reviews
  • Supports two modes: Baseline (full repo review) and Nightly (incremental daily review)
  • Adds comprehensive scripts for producing diffs, running review agents, processing findings, and orchestrating workflows
  • Includes a detailed policy configuration for import restrictions, naming conventions, severity levels, and size limits
  • Provides output formats including NDJSON findings, Markdown summaries, and combined reports
  • Enables integration with CI/CD pipelines via GitHub Actions example

Changes

Documentation

  • Added ONEX_REVIEW_README.md detailing system overview, usage, components, configuration, output formats, troubleshooting, and future enhancements

Configuration

  • New config/policy.yaml defining repository-specific import rules, severity levels, and processing limits

Scripts

  • onex_baseline_producer.sh: Generates sharded baseline diffs excluding archived code
  • onex_nightly_producer.sh: Generates incremental diffs for nightly reviews with change filtering and size truncation
  • onex_agent_runner.py: Python agent runner invoking Opus review agents, parsing NDJSON findings and Markdown summaries
  • onex_findings_processor.py: Processes findings to generate reports, fix scripts, and GitHub issue templates
  • onex_review_orchestrator.sh: Main orchestrator script to run baseline, nightly, or process modes with dependency checks and stepwise execution
  • run_enhanced_tests.sh: Made executable (no content changes)

Features

  • Deterministic regex and filename-based rules for naming, boundary, SPI purity, typing hygiene, and waiver hygiene
  • Sharded baseline review to handle large diffs efficiently
  • Incremental nightly review with marker file management
  • Automated findings processing with risk scoring and next action recommendations
  • Support for waiver annotations inline in code

Test plan

  • Manual testing of baseline and nightly review workflows
  • Validation of findings processing and report generation
  • Integration testing with GitHub Actions scheduled runs
  • Verification of policy enforcement and diff size handling

This PR establishes the foundation for automated, policy-driven code review to prevent code drift and maintain code quality during migration away from archived code.

🌿 Generated by Terry


ℹ️ Tag @terragon-labs to ask questions and address PR feedback

📎 Task: https://www.terragonlabs.com/task/d825542b-3763-4bd9-9ab5-0a3fcacf818c

…e drift detection

- Introduce baseline and nightly review modes for deterministic, diff-driven code reviews
- Add scripts for baseline and nightly diff production, agent runner, findings processing, and review orchestration
- Include comprehensive policy.yaml for import restrictions, severity levels, and limits
- Provide detailed README with usage, configuration, output formats, and integration guidance
- Enable automated findings analysis, fix script generation, and GitHub issue templates
- Support CI/CD integration and operational workflows

This initial implementation establishes a robust framework to prevent code drift while migrating away from archived code, enhancing code quality and compliance.

Co-authored-by: terragon-labs[bot] <terragon-labs[bot]@users.noreply.github.com>
@github-actions

Copy link
Copy Markdown
Contributor

🔍 ONEX Infrastructure Code Review - PR #17

Executive Summary

This PR introduces the ONEX Opus Nightly and Baseline Review System with solid architectural design but requires critical fixes for ONEX compliance violations and security vulnerabilities before merging.

Risk Score: 65/100 ⚠️
Recommendation: REQUIRES CHANGES 🔴

🚨 CRITICAL Issues (Must Fix)

1. ONEX Compliance Violations

❌ Missing Strong Typing (ZERO TOLERANCE per CLAUDE.md)

  • onex_agent_runner.py (lines 17-23): No type annotations for instance variables
  • onex_findings_processor.py (lines 15-18): Missing type hints
  • Fix: Add explicit type annotations to ALL variables

❌ Missing OnexError Exception Handling

  • onex_agent_runner.py: Lines 27-28, 42-43, 50-51
  • onex_findings_processor.py: Lines 26, 32-33
  • Fix: Wrap all file operations with OnexError exception chaining

2. Security Vulnerabilities

🔴 Shell Injection Risk

  • onex_baseline_producer.sh (line 36): Unquoted command substitution
  • onex_nightly_producer.sh: Lines 70, 73, 76
  • Fix: Quote all variable expansions and command substitutions

🔴 Path Traversal Risk

  • onex_agent_runner.py (line 32): Unsanitized path construction
  • onex_findings_processor.py (line 22): Direct path manipulation
  • Fix: Validate and sanitize all path inputs

⚠️ HIGH Priority Issues

Performance Concerns

  1. Sequential Processing in baseline_producer.sh - Consider parallel shard generation
  2. Memory Usage in agent_runner.py - Implement streaming for large finding sets

Error Handling Gaps

  1. Missing resource cleanup in error paths
  2. Incomplete rollback mechanisms on failure

✅ Positive Aspects

  • Well-architected modular design
  • Comprehensive configuration system
  • Clear documentation structure
  • Good separation of concerns

📋 Specific Fixes Required

Bug in onex_agent_runner.py (Line 323)

Currently only updates marker if findings exist. Should always update after successful completion.

Missing bounds check in onex_baseline_producer.sh (Line 56)

Add check for maximum shard count to prevent unbounded growth.

Variable scoping in onex_nightly_producer.sh (Lines 82-88)

TRUNCATED variable needs export for subshell visibility.

🧪 Testing Requirements

  1. Unit tests for Python classes
  2. Integration tests for workflows
  3. Security tests for input sanitization
  4. Performance tests for large repos

📊 Risk Assessment

  • ONEX Compliance: 5 CRITICAL issues
  • Security: 4 CRITICAL vulnerabilities
  • Performance: 2 HIGH priority optimizations
  • Error Handling: 3 HIGH priority gaps

🎯 Next Steps

  1. Fix all CRITICAL ONEX compliance and security issues
  2. Add comprehensive error handling with OnexError
  3. Implement proper resource cleanup
  4. Add test coverage before deployment

Overall: Solid foundation but needs critical fixes for ONEX compliance and security before production readiness.

🤖 Review conducted following ONEX Infrastructure standards

@jonahgabriel
jonahgabriel deleted the terragon/onex-opus-nightly-baseline-reviewer-l6r96b branch November 5, 2025 19:57
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