Skip to content

Fix: cross-review monitor.py の EARLY_ERROR 誤検知を修正 (ndf v4.7.2)#3

Merged
takemi-ohama merged 2 commits into
mainfrom
fix/ndf-cross-review-monitor-false-positive
May 20, 2026
Merged

Fix: cross-review monitor.py の EARLY_ERROR 誤検知を修正 (ndf v4.7.2)#3
takemi-ohama merged 2 commits into
mainfrom
fix/ndf-cross-review-monitor-false-positive

Conversation

@takemi-ohama
Copy link
Copy Markdown
Contributor

Summary

issues/REPORT01.md の不具合報告に対応 — 中〜大規模 PR で /ndf:cross-reviewmonitor.py が codex/gemini を 誤って kill し、ループが回らなくなる High バグの修正。

背景

monitor.pyEARLY_ERROR_PATTERNS が以下の正常実行ログを「致命的エラー」と誤判定して SIGTERM/SIGKILL していた:

  • gemini-cli の Error in: mcpServers.<name> 警告 (config 検証エラーだが起動継続可能)
  • codex がレビュー対象 diff の test コード片を echo した Traceback (most recent call last):

結果として result.json が生成されず、ラウンド全体が EARLY_ERROR で中断。

修正内容 (REPORT01 提案 1-4)

1. monitor.py: EARLY_ERROR を FATAL/WARN に分離 (提案1)

種別 パターン 動作
FATAL auth / quota / sandbox / HTTP 401-403-429 / gemini YOLO 降格 検知時 kill
WARN Error: / Traceback / FATAL / panic 等の曖昧パターン 警告ログのみ、kill せず通常判定継続

_scan_early_errors_scan_patterns / _scan_early_fatal / _scan_early_warn に分割。

2. monitor.py: BENIGN フィルタ強化 (提案2)

gemini-cli の Error in: mcpServers.<name> / Error in: <name> を benign に追加。

3. monitor.py: --no-early-error フラグ追加 (提案3)

--no-early-error / MONITOR_NO_EARLY_ERROR=1 で EARLY_ERROR 検知自体を無効化する escape hatch。

4. launch-gemini.sh: settings.json sanitize ロジック追加 (提案4)

.gemini/settings.jsondisabled キーを jq で再帰削除した sanitize 版に一時差し替え → gemini 起動 (settings 読み込み) → 2 秒後に元のファイルを復元。worktree を dirty にしない。

5. SKILL.md: アンチパターン節を更新 (提案4)

  • EARLY_ERROR の曖昧パターンで kill するのを禁止事項として明記
  • monitor.py が誤検知した場合の切り分け手順を追加

6. plugin.json: 4.7.1 → 4.7.2 (パッチ)

Test plan

  • python3 monitor.py --help--no-early-error フラグが表示される
  • bash -n launch-gemini.sh syntax OK
  • claude plugin validate . warning のみ (description 警告は既存)
  • 実 PR で /ndf:cross-review を回し、Traceback / Error in: mcpServers.* 検知時に WARN ログのみ出て kill されないこと
  • MONITOR_NO_EARLY_ERROR=1 で起動した場合、EARLY_ERROR 検知が完全に発火しないこと
  • gemini 起動後 .gemini/settings.json が元の状態に戻っていること

🤖 Generated with Claude Code

issues/REPORT01.md の不具合報告 ("EARLY_ERROR 誤検知で codex/gemini が無条件に
kill される") に対応。中規模以上の PR で cross-review がほぼ毎ラウンド失敗扱いに
なる High バグの修正。

## 修正内容

### monitor.py: EARLY_ERROR を FATAL/WARN に分離 (報告書 提案1)

- `EARLY_ERROR_PATTERNS` を `EARLY_ERROR_FATAL` (auth/quota/sandbox/HTTP 401-403-429
  /gemini YOLO 降格) と `EARLY_ERROR_WARN` (生 `Error:` / `Traceback` の曖昧パターン)
  に分離
- FATAL 検知時のみ kill。WARN は警告ログだけ出して通常判定 (sentinel / result.json /
  timeout) を継続する
- `_scan_early_errors` → `_scan_patterns` / `_scan_early_fatal` / `_scan_early_warn`
  に分割

### monitor.py: BENIGN フィルタ強化 (報告書 提案2)

- gemini-cli の `Error in: mcpServers.<name>` config validation 警告を benign 扱い
  に追加。`Error in: <name>` 単独パターンも除外

### monitor.py: --no-early-error フラグ追加 (報告書 提案3)

- `--no-early-error` / `MONITOR_NO_EARLY_ERROR=1` で EARLY_ERROR 検知自体を無効化
  する escape hatch を追加。誤検知が継続する場合の最終回避策

### launch-gemini.sh: settings.json sanitize ロジック追加 (報告書 提案4)

- worktree の `.gemini/settings.json` に gemini-cli 最新版で Unrecognized 扱いに
  なるキー (`disabled` 等) があると毎回 `Error in: mcpServers.<name>` 警告が出る
  問題を回避
- launcher が起動時に jq で `disabled` キーを再帰削除した sanitize 版に差し替え、
  gemini が settings を読み終えた頃 (2 秒後) に元のファイルを復元する

### SKILL.md: アンチパターン節を更新 (報告書 提案4)

- EARLY_ERROR の曖昧パターンで kill するのを禁止事項として明記
- `monitor.py` が誤って kill する場合の切り分け手順を追加
  (err.log 確認 → launcher の sanitize 確認 → --no-early-error 切替 → 新パターンを
  FATAL に追記)

### plugin.json: バージョン更新

- 4.7.1 → 4.7.2 (パッチ)

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Copy link
Copy Markdown
Contributor Author

@takemi-ohama takemi-ohama left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤖 cross-review | round 1 | gemini | REQUEST_CHANGES

monitor.py における早期エラー検知ロジックに、誤検知(benign)の後に本物のエラーが続くケースを見逃すロジック上の欠陥があります。また、launch-gemini.sh の設定復元ロジックが環境に依存する時間待ち(sleep)に基づいているため、堅牢性に懸念があります。

Comment thread plugins/ndf/skills/cross-review/scripts/monitor.py
Comment thread plugins/ndf/skills/cross-review/scripts/launch-gemini.sh
Copy link
Copy Markdown
Contributor Author

@takemi-ohama takemi-ohama left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤖 cross-review | round 1 | codex | REQUEST_CHANGES

一時的に worktree の設定ファイルを書き換える実装は、異常終了時の復元保証を追加してください。

Comment thread plugins/ndf/skills/cross-review/scripts/launch-gemini.sh
PR #3 round 1 のレビュー指摘 2 件に対応する。

1. monitor.py `_scan_patterns` の見逃しバグ (gemini major)
   - `pat.search()` は最初の一致しか返さないため、最初の一致が benign で
     除外されると後続の本物エラーを見逃していた
   - `pat.finditer()` に変更し、benign で除外された場合も後続を継続走査
   - さらに benign 判定を「マッチ行そのもの」に対して行うように変更
     (以前は前後 40 文字の文脈窓を使っていたが、benign 行が直前にある
      だけで後続の本物エラーが誤って benign 扱いされる別バグがあった)

2. launch-gemini.sh の sanitize 済み settings.json 残留リスク (codex major / gemini minor)
   - sleep 2 中 / 復元前に SIGTERM/Ctrl-C/シェル終了で止まると、sanitize
     済み settings.json が worktree に残る問題
   - `trap restore_settings EXIT INT TERM HUP` を登録して、どの経路で終了
     しても必ずバックアップから復元するように修正
   - restore_settings は冪等 (SETTINGS_BACKUP が未設定 or バックアップ不在
     なら何もしない) なので多重実行されても安全

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@takemi-ohama
Copy link
Copy Markdown
Contributor Author

🔧 /ndf:fix サマリ (round 1)

対応件数: critical=0 / major=2 / minor=1 (合計 3 件)
deferred: 0 件 / rejected: 0 件
commit: 32f2308
CI: なし (このリポジトリには CI ワークフローが設定されていない)

詳細

# thread severity (再判定) 対応
1 monitor.py:248 (_scan_patterns) major pat.search()pat.finditer() + benign 判定をマッチ行のみに限定
2 launch-gemini.sh:134 (codex) major trap restore_settings EXIT INT TERM HUP を登録
3 launch-gemini.sh:134 (gemini) minor (codex の major と同件) 2 と同じ修正で完結

補足

調査の過程で、benign 判定の文脈窓が前後 40 文字と広く、直前の別 benign 行が後続の本物エラーを誤って benign 扱いする 別バグが見つかったため合わせて修正しました (回帰テスト 8 ケースで確認)。

すべての thread は Resolve 済みです。

Copy link
Copy Markdown
Contributor Author

@takemi-ohama takemi-ohama left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤖 cross-review | round 2 | gemini | APPROVE

REPORT01.md で報告された monitor.py の早期エラー誤検知および launch-gemini.sh の設定ファイル非互換に起因する問題が包括的に修正されていることを確認しました。

修正のポイント

  • monitor.py: 致命的エラー (FATAL) と曖昧なパターン (WARN) を分離し、後者ではプロセスを kill しないように改善されました。また、finditer の採用により、先行する benign マッチによる後続エラーの見逃しも解消されています。
  • launch-gemini.sh: gemini-cli の警告原因となる disabled キーの自動 sanitize と、trap による確実な設定復元ロジックが導入され、信頼性が向上しました。
  • SKILL.md: 誤検知発生時のトラブルシューティング手順が明文化されました。

全体として設計・実装ともに品質が高く、実運用上の懸念事項が解消されています。

Copy link
Copy Markdown
Contributor Author

@takemi-ohama takemi-ohama left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤖 cross-review | round 2 | codex | APPROVE

修正必須の新規指摘はありません。

@takemi-ohama takemi-ohama merged commit c5c2346 into main May 20, 2026
@takemi-ohama takemi-ohama deleted the fix/ndf-cross-review-monitor-false-positive branch May 20, 2026 05:42
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