Merged
Conversation
- Extract logic to config.py, pr_label.py, coderabbit.py, ci_check.py, claude_runner.py, prompt_builder.py, git_ops.py, and report.py - Reduce auto_fixer.py from 4174 to 1521 lines - Move --list-commands to Makefile - Update tests to support new module structure and patch targets
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughauto_fixer.py を小型のオーケストレータへ縮小し、CI チェック、Claude 実行、CodeRabbit 統合、設定、Git 操作、PR ラベル、プロンプト生成、レポートをそれぞれの新規モジュールへ分割。Makefile の help ターゲット表示を簡素化。 Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Possibly related PRs
Suggested labels
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
📝 Coding Plan
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/auto_fixer.py (2)
975-1018:⚠️ Potential issue | 🟠 Majordirty worktree を捨てた後に state を更新しないでください。
ここは Claude が未コミット変更を残したときに
reset --hard/clean -fdで差分を破棄したうえで、そのまま後続の state comment 更新と thread 解決へ進みます。AI が修正を一部だけ commit して残りを未コミットで残したケースだと、必要な差分を落としたまま review を処理済み扱いにしてしまいます。修正例
- elif dirty_check.stdout.strip(): - print( - "Cleaning worktree (uncommitted work files; per assumption: correct work is committed)." - ) - git_path = shutil.which("git") - if git_path is None: - print( - "Warning: git not found in PATH; skipping cleanup and state update.", - file=sys.stderr, - ) - should_update_state = False - else: - try: - subprocess.run( - [git_path, "reset", "--hard", "HEAD"], - cwd=str(works_dir), - check=True, - capture_output=True, - ) - subprocess.run( - [git_path, "clean", "-fd"], - cwd=str(works_dir), - check=True, - capture_output=True, - ) - except subprocess.CalledProcessError as e: - print( - f"Warning: git clean failed; skipping state update to allow retry: {e}", - file=sys.stderr, - ) - should_update_state = False + elif dirty_check.stdout.strip(): + print( + "Warning: worktree still dirty after Claude run; skipping state update to allow retry.", + file=sys.stderr, + ) + should_update_state = False🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/auto_fixer.py` around lines 975 - 1018, When an uncommitted dirty worktree is detected (dirty_check.stdout non-empty) do not proceed to update state even if you successfully run the git cleanup commands; specifically, inside the branch that handles dirty_check.stdout.strip() set should_update_state = False regardless of whether the git_path is found or the subprocess.run reset/clean commands succeed (and still log warnings on failures). Update the logic around dirty_check, works_dir, git_path and the subprocess.run(reset/clean) calls so that cleanup is only attempted to restore the worktree but the variable should_update_state is always cleared after detecting uncommitted changes.
700-710:⚠️ Potential issue | 🟠 Majorコンフリクト解消コミットが上限管理から漏れ、merge 完了も確認できていません。
conflict_commitsが出てもcommitted_prs.add((repo, pr_number))を呼んでいないので、max_committed_prs_per_runをこの経路だけ素通りできます。加えて、成功判定がnot _has_merge_conflicts(works_dir)だけなので、Claude が競合マーカーを消しただけで merge commit を作らずMERGE_HEADを残した場合も「resolved」と扱われます。修正例
if conflict_commits: commits_by_phase.append(conflict_commits) + committed_prs.add((repo, pr_number)) claude_prs.add((repo, pr_number)) - conflict_resolved = not _has_merge_conflicts(works_dir) + merge_head_result = subprocess.run( + ["git", "rev-parse", "-q", "--verify", "MERGE_HEAD"], + cwd=str(works_dir), + capture_output=True, + text=True, + check=False, + ) + conflict_resolved = ( + not _has_merge_conflicts(works_dir) + and merge_head_result.returncode != 0 + )🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/auto_fixer.py` around lines 700 - 710, When conflict_commits is non-empty, add the PR to committed_prs (e.g., committed_prs.add((repo, pr_number))) so this path respects max_committed_prs_per_run, and tighten the conflict_resolved check to require both !_has_merge_conflicts(works_dir) and verification that a merge commit was actually created (e.g., ensure MERGE_HEAD is cleared or confirm a merge commit exists in git history for works_dir) before treating the PR as resolved; update the block that currently only adds to claude_prs and checks _has_merge_conflicts to also add to committed_prs and perform the merge-commit existence check using repo, pr_number, works_dir, and _has_merge_conflicts.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/ci_check.py`:
- Around line 201-280: The <instructions> block currently includes
user-controlled PR title (escaped_title) and other CI-derived text, which can be
interpreted as directives by an LLM; modify the function that builds the return
string (the code that composes checks_block, digest_block, logs_block and then
returns the f-string with <instructions>) to keep <instructions> as a fixed,
static string only, and move all dynamic/pr-controlled values (escaped_title,
pr_number and extra_data) into separate data-only blocks (e.g., a new <pr_meta>
or reuse existing <ci_failure_logs>/<ci_error_digest>) appended after
<instructions>; update the return assembly so <instructions> contains no
interpolated variables and all user-controlled content is included only in the
dedicated data blocks (refer to variables escaped_title, pr_number, extra_data,
checks_block, digest_block, logs_block).
In `@src/claude_runner.py`:
- Around line 168-176: The call to subprocess.Popen followed by
process.communicate() in claude_runner.py has no timeout and can hang; update
the code that creates and communicates with the subprocess (the block using
subprocess.Popen and process.communicate) to pass a reasonable timeout (matching
summarizer.py), catch subprocess.TimeoutExpired, terminate/kill the child
process, read any remaining output, and raise a ClaudeCommandFailedError with
context; ensure stderr/stdout handling and cleanup mirror the pattern used in
src/summarizer.py so the job is aborted instead of blocking indefinitely.
In `@src/git_ops.py`:
- Around line 31-43: The current update path runs git reset --hard and git fetch
but leaves untracked files behind; after the existing subprocess.run calls for
["git", "reset", "--hard"] (and before or after fetch) run a git clean to remove
untracked/ignored files and directories for the reused worktree: invoke a
subprocess.run that executes git clean with flags to force removal of files and
directories (e.g., -f -d and -x as needed) using the same cwd=works_dir and
check=True, referencing the existing repo and works_dir variables so leftover
artifacts from prior PRs are fully removed.
- Around line 21-23: The code constructs works_dir from Path("../works") which
depends on the process CWD; change it to resolve relative to this module (so it
matches the same base used by main()/config). Replace Path("../works") with a
path computed from Path(__file__).resolve().parent (e.g.
Path(__file__).resolve().parent.parent / "works" or whatever ancestor matches
your project root), then use that base to build works_dir (where owner and
repo_name are used) and still call mkdir(parents=True, exist_ok=True); update
references to works_dir, owner, repo_name accordingly.
In `@src/prompt_builder.py`:
- Around line 193-202: The prompt currently injects the raw PR title directly
into the top-level <instructions> block (see the f-string return in
generate_prompt / prompt_builder.py), which allows user-controlled text to be
interpreted as instructions; remove {title} from inside the <instructions> block
and instead pass the title in a separate, clearly delimited data block (e.g.
<pr_title> or <metadata> placed outside <instructions>) alongside the existing
PR number and base_branch, so the instruction block remains invariant and the
title is treated as data.
---
Outside diff comments:
In `@src/auto_fixer.py`:
- Around line 975-1018: When an uncommitted dirty worktree is detected
(dirty_check.stdout non-empty) do not proceed to update state even if you
successfully run the git cleanup commands; specifically, inside the branch that
handles dirty_check.stdout.strip() set should_update_state = False regardless of
whether the git_path is found or the subprocess.run reset/clean commands succeed
(and still log warnings on failures). Update the logic around dirty_check,
works_dir, git_path and the subprocess.run(reset/clean) calls so that cleanup is
only attempted to restore the worktree but the variable should_update_state is
always cleared after detecting uncommitted changes.
- Around line 700-710: When conflict_commits is non-empty, add the PR to
committed_prs (e.g., committed_prs.add((repo, pr_number))) so this path respects
max_committed_prs_per_run, and tighten the conflict_resolved check to require
both !_has_merge_conflicts(works_dir) and verification that a merge commit was
actually created (e.g., ensure MERGE_HEAD is cleared or confirm a merge commit
exists in git history for works_dir) before treating the PR as resolved; update
the block that currently only adds to claude_prs and checks _has_merge_conflicts
to also add to committed_prs and perform the merge-commit existence check using
repo, pr_number, works_dir, and _has_merge_conflicts.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: f1d7a542-7082-4302-8581-5e218d37b1a5
📒 Files selected for processing (11)
Makefilesrc/auto_fixer.pysrc/ci_check.pysrc/claude_runner.pysrc/coderabbit.pysrc/config.pysrc/git_ops.pysrc/pr_label.pysrc/prompt_builder.pysrc/report.pytests/test_auto_fixer.py
- ci_check/prompt_builder: <instructions>からユーザー制御値を<pr_meta>に分離 - claude_runner: process.communicate()にタイムアウトを追加 - git_ops: CWD依存のパスをモジュール相対パスに変更、git clean追加 - auto_fixer: dirty worktree時のstate update防止、コンフリクト解消時のcommitted_prs追加とMERGE_HEAD検証
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/ci_check.py`:
- Around line 303-329: The subprocess.run calls that fetch PR head SHA (assigned
to head_result) and check runs (assigned to result) inside
_collect_ci_failure_materials lack a timeout; add timeout=60 to both
subprocess.run invocations and wrap the calls (or the surrounding logic) to
catch subprocess.TimeoutExpired, logging a clear message and returning None
(same behavior as other failure paths) so the function doesn't hang if gh api
blocks.
- Around line 359-371: この subprocess.run 呼び出し (変数 commit_result に代入している箇所) に
timeout パラメータを追加し、タイムアウト発生時に subprocess.TimeoutExpired
を捕捉して適切にログ出力またはフォールバックするように修正してください；具体的には commit_result を作る subprocess.run(...,
timeout=<適切な秒数>, ...) に timeout 引数を追加し、同じ関数内で try/except
subprocess.TimeoutExpired: を追加してエラーメッセージを出力（またはエラー処理/リトライ/デフォルト値設定）するようにしてください。
In `@src/git_ops.py`:
- Around line 165-168: The current check inspects merge_result.stdout/stderr for
the English phrase "already up to date", which is locale-dependent; instead
capture the commit id (or tree) before the merge and compare it to the commit id
after the merge to detect whether anything changed. Concretely: before invoking
the merge save pre_merge_head = run_git("rev-parse", "HEAD"), perform the merge
(merge_result), then get post_merge_head = run_git("rev-parse", "HEAD") (or use
reflog HEAD@{1} if needed); if pre_merge_head == post_merge_head treat as no
changes (return (False, False) or equivalent), otherwise treat as merged
changes. Replace the current merge_output / "already up to date" string check
with this HEAD comparison using the existing merge_result, pre_merge_head and
post_merge_head variables.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 373f186a-bc26-4f88-9255-99f4025edade
📒 Files selected for processing (5)
src/auto_fixer.pysrc/ci_check.pysrc/claude_runner.pysrc/git_ops.pysrc/prompt_builder.py
🚧 Files skipped from review as they are similar to previous changes (1)
- src/prompt_builder.py
Owner
Author
🤖 Refix Status対応済みレビュー一覧
|
HappyOnigiri
added
refix:done
HappyOnigiri
added
refix:auto-merge-requested
This was referenced Mar 13, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
This PR refactors the monolithic (4,174 lines) into smaller, specialized modules to reduce complexity and improve maintainability.
Module Breakdown
config.py: Configuration loading and validationpr_label.py: PR label managementcoderabbit.py: CodeRabbit rate limit and auto-resume logicci_check.py: CI status checks and CI-fix prompt generationclaude_runner.py: Claude CLI executionprompt_builder.py: Prompt generation logicgit_ops.py: Git operationsreport.py: Runtime report managementOther Changes
--list-commandslogic toMakefile.auto_fixer.pyto 1,521 lines.CI checks and tests (173/173) are passing.
Summary by CodeRabbit
Refactor
New Features
Tests
Chores