Fix: cross-review gemini stall 既定 480s + fix 戻り値 fallback (PLAN21, v4.7.4) by takemi-ohama · Pull Request #6 · devbasex/ai-plugins

takemi-ohama · 2026-05-23T16:43:04Z

Summary

/ndf:cross-review 実走時の運用不整合 2 件を恒久対応する PATCH リリース (i18 報告 / PLAN21)。

monitor.py: per-agent stall timeout 既定の導入 (codex=180s / gemini=480s)。err.log にほぼ進捗を出さない gemini を 1 度目に毎回 STALLED で kill する誤検知を解消。解決順は CLI --stall-timeout > env MONITOR_STALL_<AGENT> > env MONITOR_STALL > builtin。
state.py merge-fix: fix 戻り値ファイル探索を (1) --file > (2) \$TMP_DIR/ > (3) /tmp/ の 3 段 fallback に。全候補不在時の die メッセージに探索 path 一覧を含める。commit_sha / fixed の別名 key も fix_commit / fixed_count として受理 (silent な fixed=0 記録を救済)。
SKILL.md / docs のパス記述統一: SKILL.md の /tmp/fix-pr<#>-result.json ハードコード 3 箇所を \$TMP_DIR/ に。docs/01 monitor 説明と docs/02 $TMP_DIR 解決順を追記。

差分は 11 files / +928 -17 (うち issues/PLAN21*.md と issues/i18-issue-gemini.md の plan ファイル分が大半。コード変更自体は ~130 行)。

Plan

issues/PLAN21_cross-review-gemini-stall-and-fix-merge.md を参照。

互換性

`--stall-timeout` を CLI 明示しているユーザ: 挙動不変
`MONITOR_STALL` env 指定ユーザ: 挙動不変 (両 agent に同じ値が適用)
何も指定していないユーザ: gemini 既定が 180s → 480s に緩和 (非破壊)
正規パス + 正規 key で書いていたユーザ: 挙動不変
`/tmp/` ハードコードや別名 key を使っていたユーザ: silent fail していたケースが拾われるようになる

Test plan

`pytest plugins/ndf/skills/cross-review/tests` → 24/24 pass (新規 11 ケース含む)
- `test_monitor_stall_default.py` (6): codex/gemini ビルトイン / per-agent env / 共通 env / 優先順位 / unknown agent
- `test_state_merge_fix.py` (5): 正規 / `/tmp/` fallback / key 別名 / 全候補不在 / `--file` 明示
`claude plugin validate plugins/ndf` → pass
(手動検証 / 受入時) gemini 引数なし起動で 480s まで stall 待ちが効くこと
(手動検証 / 受入時) `/tmp/fix-pr-result.json` に書いた状態でも `merge-fix` が拾うこと

….7.4) `/ndf:cross-review` 実走時の運用不整合 2 件を恒久対応する PATCH リリース (i18 報告 / PLAN21)。 - monitor.py: per-agent stall timeout 既定の導入 - 既定を agent 別ビルトイン化: codex=180s (不変) / gemini=480s - err.log にほぼ進捗を出さない gemini を 1 度目に毎回 STALLED kill する誤検知を解消 - 解決順: CLI --stall-timeout > env MONITOR_STALL_<AGENT> > env MONITOR_STALL > builtin - --stall-timeout argparse default を int → None に変更 - state.py merge-fix: fix 戻り値ファイル探索 fallback + key alias - 探索順を (1) --file > (2) $TMP_DIR/ > (3) /tmp/ の 3 段に - 全候補不在時の die メッセージに探索 path 一覧を含める - commit_sha / fixed の別名 key も fix_commit / fixed_count として受理 - SKILL.md / docs のパス記述統一 - SKILL.md の /tmp/fix-pr<#>-result.json ハードコード 3 箇所を $TMP_DIR/ に - 「すべて /tmp/ に置き」表現も $TMP_DIR/ (= _tmp_dir() 解決先) に - docs/01: monitor 説明表に per-agent stall 既定の解決順を追記 - docs/02: $TMP_DIR の解決順を 1 行明示 - pytest 追加 (全 24 ケース pass) - test_monitor_stall_default.py (6 ケース) - test_state_merge_fix.py (5 ケース) - plugin version 4.7.3 → 4.7.4 + CHANGELOG entry 関連: issues/i18-issue-gemini.md, issues/PLAN21_cross-review-gemini-stall-and-fix-merge.md Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

takemi-ohama

🤖 cross-review | round 1 | gemini | APPROVE

PLAN21に基づき、Geminiのstall判定緩和とfix戻り値マージの堅牢化が正しく実装されています。追加されたテストにより、fallbackロジックの正常動作も確認されています。

takemi-ohama

🤖 cross-review | round 1 | codex | REQUEST_CHANGES

/tmp fallback はPR番号だけの非スコープファイルを無条件に採用するため、同一PR番号の再実行や別リポジトリの古い戻り値を state にマージし得ます。fallback候補は現在round開始後に生成されたことを検証し、古い候補を無視してください。

codex review 指摘 (PR #6): `/tmp/fix-pr<PR>-result.json` は PR 番号のみで命名された共有 namespace のため、別 round / 別リポジトリの同番号 PR の戻り値を無条件に拾ってしまう major バグを修正。 - 冒頭で state を 1 回だけ load し、round 開始時刻 (UNIX time) を取得 - fallback 候補 (TMP_DIR / /tmp) の採用前に以下を検証: 1. ファイル mtime が round.started_at 以降 2. JSON 内 `pr` フィールドがあれば対象 PR と一致 - 古い候補は警告を stderr に出して skip - 明示 `--file` 指定はユーザー意図優先で stale 検証バイパス - 重複していた後段の `_load(pr)` を削除 tests/test_state_merge_fix.py: - stale mtime (TMP_DIR / /tmp 両方) / pr 不一致 / fresh 採用 / --file バイパスの 5 ケースを追加 (29 tests 全て pass) Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

takemi-ohama

🤖 cross-review | round 2 | gemini | APPROVE

fallback 指定の fix 結果ファイルに対する stale 検証（mtime および PR 番号のチェック）が導入され、共有ディレクトリにおける競合や古い実行結果の誤マージのリスクが適切に低減されています。テストコードも網羅的で、正常系・異常系ともに期待通り動作することが確認できました。

takemi-ohama

🤖 cross-review | round 2 | codex | REQUEST_CHANGES

fallback候補の検証で、古い候補・PR不一致と、正規戻り値ファイルの読み取り失敗を分けて扱ってください。

cross-review round 2 のクロスレビュー指摘 4 件に対応 (plugins/ndf/skills/cross-review/scripts/state.py)。 - (codex major, line 151) 正規パス ($TMP_DIR/fix-pr<PR>-result.json) の JSON parse 失敗を後続 fallback に流すと別 PR の戻り値を誤マージする事故が起こるため、is_canonical=True の候補の OSError / JSONDecodeError は即時 die(code=3) で中断する。stale mtime / pr 不一致は従来通り次の候補へ。 - (gemini minor, line 150) _is_fresh_fix_result の read_text() に encoding="utf-8" を明示。 - (gemini minor, line 155) file_pr の int() を try/except で囲み、 ValueError / TypeError なら warn + invalid 扱い (fresh ではない)。 - (gemini minor, line 508) _is_fresh_fix_result を (is_fresh, parsed_payload) のタプル返却に変更し、呼び出し側 cmd_merge_fix で再パースを省略。明示 --file パスのみ別途 read_text(encoding="utf-8") で読み込む。 tests/test_state_merge_fix.py に下記 4 ケース追加: - 正規パス JSON parse 失敗 → 即時 die(code=3)、/tmp fallback に流れないこと - fallback の pr フィールドが int 化不能 → skip - _is_fresh_fix_result が (True, dict) を返すこと - stale 時に (False, None) を返すこと全 33 テスト pass。 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

takemi-ohama

🤖 cross-review | round 3 | gemini | REQUEST_CHANGES

正規パスの読み取り失敗時の die 処理や再パース回避による性能改善は適切です。一方で、JSON パース後のデータが dict であることを検証していないため、リスト形式などの不正な JSON が与えられた場合に AttributeError でプロセスがクラッシュする脆弱性が残っています。

takemi-ohama

🤖 cross-review | round 3 | codex | COMMENT

MONITOR_STALL の共通 env 解決と、その回帰テストの期待値を実際の上書き値に合わせてください。

…L 動的解決 (PLAN21 round 4) PR #6 round 3 のクロスレビューで指摘された 3 件を修正: 1. state.py:_is_fresh_fix_result (major, gemini 指摘) json.loads は dict 以外 (list 等) も返しうるため、payload.get(...) 呼び出し前に isinstance(payload, dict) を検証し、dict でなければ warn + (False, None) を返す。 2. state.py:cmd_merge_fix (major, gemini 指摘) --file 明示時の JSON も同様に isinstance(fix, dict) を検証。dict でない場合は後続の fix.get(...) クラッシュを防ぐため即時 die(code=3)。 3. monitor.py:_agent_stall_default (minor, codex 指摘) MONITOR_STALL が設定されているとき、import 時固定の DEFAULT_STALL ではなく int(os.environ["MONITOR_STALL"]) を呼び出し時に再評価する。これにより共通 env が実際に反映されるようになり、テストでも検証可能になる。テスト: - test_is_fresh_fix_result_non_dict_json_is_skipped (新規) - test_is_fresh_fix_result_non_dict_json_canonical_also_skipped (新規) - test_explicit_file_non_dict_json_dies (新規) - test_shared_env_applies_to_both: 期待値を 240 に修正 (共通 env 反映の検証) - test_per_agent_env_takes_precedence_over_shared: codex 側の期待値を 240 に修正全 36 tests pass。 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

takemi-ohama

🤖 cross-review | round 4 | gemini | REQUEST_CHANGES

PLAN21 round 3 の指摘の反映を確認しました。fix 結果マージへの防御と MONITOR_STALL の動的解決により堅牢性が向上しています。一方で、review result や round payload のパース箇所に同様の non-dict JSON によるクラッシュリスクが残っています。これらへの水平展開を推奨します。

takemi-ohama

🤖 cross-review | round 4 | codex | REQUEST_CHANGES

fix 戻り値の明示/正規パスが不正な場合に共有 fallback へ継続する経路が残っているため、誤マージ防止のガードを追加してください。

… round 5) PR #6 round 4 クロスレビュー指摘 4 件への対応。 1. state.py `_is_fresh_fix_result(is_canonical=True)` の非 dict は parse 失敗と同様に即時 die(code=3) する (codex)。skip すると後続 /tmp/ fallback で別実行の戻り値を誤マージする経路が残るため。 2. state.py `cmd_merge_fix` の `--file` 明示時は fallback 探索に進まず即時失敗させる (codex)。存在しない / 空 / JSON 不正 / 非 dict のすべてのケースで die(code=3) する。 3. state.py `cmd_read_result` / `cmd_check_oscillation` でも JSON 読み込み直後に `isinstance(..., dict)` 検証を追加し、不正な JSON によるクラッシュを防止 (gemini)。`cmd_check_oscillation` は comments のエントリも dict 検証する。 4. monitor.py `_agent_stall_default()` で env が非数値だった場合、ValueError / TypeError を warn 付きで catch して builtin 既定にフォールバック (gemini)。監視プロセスを env 設定ミスでクラッシュさせない。テスト: - test_state_merge_fix.py: 正規パス非 dict / --file 不在 / 空 / 不正 JSON の die(code=3) 確認 + 既存 round 4 テストを round 5 挙動に追従 - test_state_read_result.py: result.json 非 dict / parse 不能の die(code=3) - test_state_check_oscillation.py (新規): payload 非 dict / comments エントリ非 dict の die(code=3) + 正常 dict が誤検知されない regression guard - test_monitor_stall_default.py: MONITOR_STALL=abc 等で builtin フォールバック全 48 テスト pass。 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

takemi-ohama

🤖 cross-review | round 5 | gemini | APPROVE

環境変数のパース失敗時や JSON ファイルが不正な型（dict 以外）である場合の防御策が網羅的に追加されており、システムの堅牢性が大幅に向上しています。

特に cmd_merge_fix において --file 指定時のバリデーションを厳格化した点、および正規パスでの不正データ検出時にフォールバックせず即時停止（die）するようにした点は、意図しないデータのマージを防ぐ上で非常に重要な改善です。テストコードも追加された各エラーパスを的確に検証しており、品質面で問題ありません。

takemi-ohama

🤖 cross-review | round 5 | codex | REQUEST_CHANGES

MONITOR_STALL の非数値フォールバックが import 前に無効化される経路が残っています。

… / 最終) codex round 5 指摘: `DEFAULT_TIMEOUT` / `DEFAULT_STALL` / `DEFAULT_POLL` を `int(os.environ.get(name, ...))` で import 時に評価すると、`MONITOR_STALL=abc` のような非数値 env 設定で `_agent_stall_default()` に到達する前に `int()` の `ValueError` で monitor.py の import そのものが落ち、監視プロセスが起動できなくなる。修正: - `DEFAULT_TIMEOUT=420` / `DEFAULT_STALL=180` / `DEFAULT_POLL=15` を import 時に固定数値で保持 - env の解釈は `_agent_stall_default()` / 新規 `_safe_int_env()` 内で try/except 付きで行う (非数値時は warn を出して fallback) - argparse 既定値も `_safe_int_env()` 経由で env を呼び出し時に読むテスト追加 (`tests/test_monitor_import_safety.py`): - `MONITOR_STALL=abc` / `MONITOR_TIMEOUT=xxx` / `MONITOR_POLL=foo` を個別 / 同時に設定しても monitor.py の fresh import が成功すること - `_safe_int_env()` ヘルパの正常系 / fallback / warn 出力 cross-review test suite: 55 passed (新規 7 件含む、既存テスト全 pass)。 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

takemi-ohama

🤖 cross-review | round 6 | gemini | APPROVE

環境変数の非数値設定によるクラッシュを修正し、網羅的なテストが追加されています。修正内容は適切で、堅牢性が向上しています。

takemi-ohama