Skip to content

feat: Add advisory code review system - #37

Merged
RevCBH merged 21 commits into
mainfrom
RevCBH/codex-code-review
Jan 22, 2026
Merged

feat: Add advisory code review system#37
RevCBH merged 21 commits into
mainfrom
RevCBH/codex-code-review

Conversation

@RevCBH

@RevCBH RevCBH commented Jan 22, 2026

Copy link
Copy Markdown
Owner

Summary

This PR implements an advisory code review system that runs before merging unit branches into the feature branch. The review system:

  • Supports multiple review providers (Claude, Codex)
  • Provides configuration options for review behavior
  • Implements a fix loop for automated issue resolution
  • Integrates into the worker merge flow

Units completed:

  • reviewer-interface: Core reviewer types and interfaces
  • review-config: Configuration types and validation
  • codex-reviewer: Codex-based code reviewer implementation
  • claude-reviewer: Claude-based code reviewer implementation
  • review-worker: Worker integration with review orchestration
  • review-wiring: Final wiring to integrate all components

Test plan

  • Unit tests for all new components
  • Integration tests for review flow
  • Manual testing with daemon

🤖 Generated with Claude Code

Test User and others added 21 commits January 21, 2026 14:53
…assed:true

Parsing failures in the Claude reviewer were incorrectly marked as
Passed:true, which masked failures and caused CodeReviewPassed events
to be emitted instead of CodeReviewFailed. This could skip fix attempts
when they should have been triggered.

Now parseOutput returns errors for:
- No JSON found in review output
- JSON unmarshaling failures

The caller in review.go already handles these gracefully by logging
the error, emitting CodeReviewFailed, and proceeding to merge.

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Add guard to cleanupWorktree() to skip when worktreePath is empty
- Add Bus.Wait() and EventCollector for thread-safe event collection in tests
- Update review_test.go and worker_test.go to use new thread-safe patterns
- Add PRD for safe git operations interface refactoring

Root cause: Tests with MaxFixIterations>0 but no worktreePath would trigger
cleanupWorktree(), which ran git checkout/reset/clean in the current
working directory (the actual repo), reverting uncommitted changes.

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Resolves conflicts:
- internal/worker/worker.go: kept getTargetRef() from main, replaced
  logReviewPlaceholder with runCodeReview from HEAD
- internal/orchestrator/orchestrator_test.go: kept main's graph tests,
  added HEAD's reviewer tests (TestResolveReviewer_*)

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
…-operations

- CODE-REVIEW.md: status changed to completed
- safe-git-operations.md: added proper frontmatter and document info

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
@RevCBH
RevCBH merged commit ab4c983 into main Jan 22, 2026
2 checks passed
@RevCBH
RevCBH deleted the RevCBH/codex-code-review branch January 22, 2026 15:43
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