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)

種別パターン動作
FATALauth / quota / sandbox / HTTP 401-403-429 / gemini YOLO 降格検知時 kill
WARNError: / 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>

@takemi-ohamatakemi-ohama left a comment

Copy link
Copy Markdown
ContributorAuthor

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 threadplugins/ndf/skills/cross-review/scripts/monitor.py
Comment threadplugins/ndf/skills/cross-review/scripts/launch-gemini.sh

@takemi-ohamatakemi-ohama left a comment

Copy link
Copy Markdown
ContributorAuthor

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 threadplugins/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
ContributorAuthor

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

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

詳細

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

補足

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

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

@takemi-ohamatakemi-ohama left a comment

Copy link
Copy Markdown
ContributorAuthor

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: 誤検知発生時のトラブルシューティング手順が明文化されました。

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

@takemi-ohamatakemi-ohama left a comment

Copy link
Copy Markdown
ContributorAuthor

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 mainMay 20, 2026
@takemi-ohama
takemi-ohama deleted the fix/ndf-cross-review-monitor-false-positive branch May 20, 2026 05:42
takemi-ohama added a commit that referenced this pull request May 26, 2026
- conftest.py.template: browser_type_launch_args を依存に追加し local モードで
--headed/slow_mo/channel 等が反映されるよう修正 (#1, #6)
- conftest.py.template: 変数名 b → browser に改名 (#2)
- conftest.py.template: CDP 接続時に非 Chromium ブラウザの fail-fast チェック追加 (#5)
- config.py: BrowserConfig.cdp_endpoint の既定値を ws:// → http:// に変更 (#4)
- CHANGELOG.md: v4.10.0 エントリを追加 (#3)
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Sign up for freeto 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

@takemi-ohama