diff --git a/issues/PLAN20_cross-review-worktree-and-result-schema-fix.md b/issues/PLAN20_cross-review-worktree-and-result-schema-fix.md new file mode 100644 index 00000000..6a9c89ae --- /dev/null +++ b/issues/PLAN20_cross-review-worktree-and-result-schema-fix.md @@ -0,0 +1,313 @@ +# PLAN20: cross-review の worktree デフォルトパス と gemini result.json スキーマ整合性修正 + +- 起票日: 2026-05-21 +- 対象 plugin: `ndf` v4.7.2 +- 対象 skill: `ndf:cross-review` +- 関連 issue: [issues/i17.md](./i17.md) +- 報告者: takemi-ohama (`devbasex/devbase#14` の `/ndf:cross-review 14` 実行中に検出) + +## 背景・課題 + +`/ndf:cross-review` を macOS ホストで実行した際に、独立した 2 件の不具合に遭遇している。 +どちらも回避策は確立されているが、毎回手動介入が必要なためプラグイン側で恒久対応する。 + +### 問題1: worktree デフォルトパスが Linux コンテナ前提でハードコードされている + +`scripts/state.py:122` の以下が原因: + +```python +worktree = args.worktree or f"/work/worktrees/pr{pr}" +``` + +macOS では `/work` が SIP で書き込み不可のため、`git worktree add` が `Read-only file system` +で失敗する。SKILL.md / docs / launcher prompt 側にも同じパスが文字列として埋め込まれており、 +ユーザは `--worktree` を毎回明示しないと init できない。 + +### 問題2: gemini の `result.json` スキーマが launcher 間で揺れて intent が欠落 + +`launch-gemini.sh:90` がスキーマの具体的フィールドを列挙せず「フォーマットは launch-codex.sh +と同じ」とだけ書いているため、gemini が独自スキーマ (`intent` / `comment_count`) で書き出して +しまう。一方 `state.py:244-265` `cmd_read_result` は `event` / `comments_count` しか見ないため、 +**intent=None で state に取り込まれ judge で空回り**する。最悪 `max_rounds` 到達まで無駄 +round + 無駄レビューコメントが積み上がる。 + +両不具合とも cross-review skill 配下に閉じており、影響範囲も小さいため **単一 PR** で対応する。 + +## ゴール + +1. macOS / WSL / 非コンテナ環境でも `state.py init ` が `--worktree` 引数なしで成功する +2. gemini が書き出した `result.json` が `state.py read-result` で正しく `intent` を含めて + state にマージされ、judge が両者の APPROVE を認識する +3. 上記いずれかが将来再発したときに気付けるよう、**`read-result` 側で intent 欠落を検知して + エラー終了**する (silent な None マージを禁止) +4. 既存のコンテナ環境 (`/work` 書込可) の挙動は変えない (後方互換) + +## 設計方針 + +### 1. worktree デフォルトパス解決ロジック (問題1) + +`state.py` に `_default_worktree_base()` を追加し、優先度順に解決する: + +```python +def _default_worktree_base() -> pathlib.Path: + """worktree の親ディレクトリを環境に応じて解決する。 + + 優先順位: + 1. 環境変数 NDF_WORKTREE_BASE (明示オーバーライド) + 2. /work/worktrees (Linux コンテナ環境互換、書き込み可能ならそれを使う) + 3. $HOME/work/worktrees (macOS / WSL 等のフォールバック) + """ + env = os.environ.get("NDF_WORKTREE_BASE") + if env: + return pathlib.Path(env) + legacy = pathlib.Path("/work/worktrees") + try: + legacy.mkdir(parents=True, exist_ok=True) + # mkdir 成功 = 書き込み可能 → 既存環境互換でこちらを使う + return legacy + except OSError: + pass + return pathlib.Path.home() / "work" / "worktrees" +``` + +`cmd_init` 内の参照を以下に変更: + +```python +worktree = args.worktree or str(_default_worktree_base() / f"pr{pr}") +``` + +**ポイント**: + +- Linux コンテナ環境 (既存ユーザ) は `/work/worktrees` が引き続き使われ挙動不変 +- macOS / WSL は `$HOME/work/worktrees/pr` にフォールバック +- `NDF_WORKTREE_BASE=/foo/bar` で明示オーバーライド可能 +- 解決した実 path は `state.json` の `worktree_path` に書かれるため、後続スクリプト・サブエージェント + prompt は既存どおり state.json から読めば追従できる + +### 2. ドキュメント・launcher prompt のハードコード除去 (問題1 派生) + +launcher prompt は `WORKTREE` を `state.json` から読む構造になっており、修正不要。 +**ドキュメントの説明文だけ** 抽象化する: + +- `SKILL.md` 「事前確認」表 #2 の `git worktree add /work/worktrees/pr` を + `git worktree add /pr` + 注記 (`worktree-base` の解決順) に変更 +- `SKILL.md` mermaid 図中の `/work/worktrees/pr を用意` を `/pr を用意` に +- `docs/01-state-and-review.md` の Step 0 解説 (`3. worktree 作成（/work/worktrees/pr）`) と + state.json サンプル (`"worktree_path": "/work/worktrees/pr123"`) を、説明文側は抽象化しつつ + JSON 例は **解決例として 1 つだけ** 残す (具体例の方が読みやすいため) +- `docs/01-state-and-review.md:166` の「作業 worktree の絶対パスを使う」記述は、抽象化した上で + 「実 path は state.json の `worktree_path` を参照」と追記 + +state.json の `worktree_path` を一次ソースとする方針を明文化することで、将来の path 体系変更にも +追従しやすくする。 + +### 3. gemini result.json スキーマの明示化 (問題2 のうち launcher 側) + +`launch-gemini.sh:90` の曖昧な指示を **codex と同一のフィールド列挙ブロック** に置き換える: + +```text +- 投稿後、サマリを **$TMP_DIR/gemini-review-pr$STATE_PR-result.json** に + **必ず以下のキーで** 書く: + ```json + { + "event": "APPROVE", + "posted_as": "COMMENT", + "comments_count": 3, + "review_url": "https://github.com/.../pull/$PR#pullrequestreview-...", + "by_severity": {"critical": 0, "major": 0, "minor": 0, "nit": 0} + } + ``` + - `intent` / `comment_count` 等の別名は使わないこと + - `event` の値は `APPROVE` / `REQUEST_CHANGES` / `COMMENT` のいずれか + - `event_downgrade=true` のとき `posted_as` は `COMMENT` にダウングレード可 +- payload は **$TMP_DIR/gemini-review-pr$STATE_PR-round$ROUND-payload.json** に保存 +``` + +`launch-codex.sh` 側は既に同等の明示があるため変更不要。ただし将来の整合性のため、 +**両 launcher で同一のスキーマブロックをコピペで持たせる** (共通ファイル切り出しは +スコープ過大なので今回はしない)。 + +### 4. `state.py read-result` の堅牢化 (問題2 のうち state 側) + +`cmd_read_result` を以下に変更し、(a) 別名フィールドへフォールバック、(b) intent 欠落時は明示的に +fail させる: + +```python +def cmd_read_result(args: argparse.Namespace) -> None: + agent = args.agent + pr = args.pr + rfile = pathlib.Path(args.file or _tmp_dir() / f"{agent}-review-pr{pr}-result.json") + if not rfile.exists() or rfile.stat().st_size == 0: + die(f"{agent}: result 未生成 ({rfile})") + + r = json.loads(rfile.read_text()) + + # 別名フィールドへのフォールバック (gemini が `intent` / `comment_count` を使う変則 JSON を + # 書き出す既知のケースに対応する。仕様としては `event` / `comments_count` が正) + intent = r.get("event") or r.get("intent") + posted_as = r.get("posted_as") or intent + comments = r.get("comments_count") + if comments is None: + comments = r.get("comment_count") + + if intent is None: + die( + f"{agent}: result.json に event / intent フィールドが無い ({rfile})。" + " launcher prompt のスキーマ違反の可能性。" + ) + + st = _load(pr) + if not st.get("rounds"): + die(f"{agent}: state.rounds が空。`state.py start-round` を先に呼んでください") + st["rounds"][-1][agent] = { + "intent": intent, + "posted_as": posted_as, + "comments": comments, + "review_url": r.get("review_url"), + "by_severity": r.get("by_severity", {}), + } + _save(pr, st) + info(f"✅ {agent}: intent={intent} posted_as={posted_as} comments={comments}") +``` + +**ポイント**: + +- 仕様 (`event` / `comments_count`) を**優先**しつつ、別名 (`intent` / `comment_count`) も拾える +- intent が取れないときは `die()` で exit 1 する → judge 段階より早く発見できる +- 既存のスキーマで書かれた result.json は挙動不変 + +### 5. テスト (軽量) + +`scripts/state.py` は現状 unit test を持たない。 +今回は `cmd_read_result` 周辺のスキーマ揺れだけを対象に、**軽量な pytest を追加**する。 +位置は `plugins/ndf/skills/cross-review/tests/test_state_read_result.py`。 + +カバレッジ: + +1. 正規スキーマ (`event` / `comments_count`) → intent / comments が state に書かれる +2. 変則スキーマ (`intent` / `comment_count`) → 同等に state に書かれる (フォールバック動作) +3. intent / event いずれも無い → `die()` で exit 1 + state 不変 +4. (worktree path 側) `NDF_WORKTREE_BASE` が指定されていれば `_default_worktree_base()` が + その path を返す / `/work` が無い環境では `$HOME/work/worktrees` を返す + +`pytest` 実行は CI には組み込まず、開発者がローカルで `uv run pytest plugins/ndf/skills/cross-review/tests` +で回せる形で OK (現状 ndf プラグインに CI 設定が無いため)。 + +### 6. バージョン更新 + +- `plugins/ndf/.claude-plugin/plugin.json` を `4.7.2` → `4.7.3` (patch bump、バグ修正のみ) +- `plugins/ndf/CHANGELOG.md` または `plugins/ndf/CLAUDE.md` の開発履歴に v4.7.3 のエントリ追加 + (存在を確認の上、所在に合わせる) + +## 実装タスク + +### Phase 1: worktree デフォルトパス解決 (問題1 本体) +- [ ] `scripts/state.py` に `_default_worktree_base()` 関数を追加 +- [ ] `cmd_init` の `worktree = args.worktree or f"/work/worktrees/pr{pr}"` を + `worktree = args.worktree or str(_default_worktree_base() / f"pr{pr}")` に変更 +- [ ] `import os` / `import pathlib` が既に取り込まれていることを確認 (state.py 冒頭) + +### Phase 2: ドキュメントの worktree パス抽象化 +- [ ] `SKILL.md`「事前確認」表 #2 を `/pr` 表現に変更 +- [ ] `SKILL.md` mermaid 図中の `/work/worktrees/pr` を `/pr` に +- [ ] `SKILL.md` 末尾もしくは関連節に `` の解決順 (env > /work > $HOME) を追記 +- [ ] `docs/01-state-and-review.md` Step 0 解説 (line 97 付近) を抽象化、worktree_path は state.json + 参照と注記 +- [ ] `docs/01-state-and-review.md` line 166 付近の絶対パス記述を「state.json の `worktree_path` + を参照」に整理 + +### Phase 3: gemini launcher のスキーマ明示 (問題2 launcher 側) +- [ ] `scripts/launch-gemini.sh:90` のサマリ書き出し指示を、codex と同一の JSON スキーマ + ブロックに置き換え +- [ ] `intent` / `comment_count` 等の別名禁止を明記 +- [ ] `event_downgrade=true` 時の `posted_as` 扱いを明記 + +### Phase 4: state.py read-result の堅牢化 (問題2 state 側) +- [ ] `cmd_read_result` を別名フォールバック + intent 欠落時 die に書き換え +- [ ] info ログを新しい変数 (intent / posted_as / comments) ベースに変更 + +### Phase 5: テスト追加 +- [ ] `plugins/ndf/skills/cross-review/tests/__init__.py` (空) +- [ ] `plugins/ndf/skills/cross-review/tests/test_state_read_result.py`: + - [ ] 正規スキーマケース + - [ ] 変則スキーマケース (`intent` / `comment_count` 名) + - [ ] intent 欠落 → die ケース (SystemExit を確認) +- [ ] `plugins/ndf/skills/cross-review/tests/test_default_worktree_base.py`: + - [ ] `NDF_WORKTREE_BASE` 明示時にそれを返す + - [ ] `/work` 書き込み不可をモックして `$HOME/work/worktrees` を返す +- [ ] ローカルで `uv run pytest plugins/ndf/skills/cross-review/tests` を回し全 pass を確認 + +### Phase 6: バージョン更新 +- [ ] `plugins/ndf/.claude-plugin/plugin.json` の `version` を `4.7.3` に更新 +- [ ] `plugins/ndf/CHANGELOG.md` か `plugins/ndf/README.md` (存在に応じて) に v4.7.3 の節を追加 +- [ ] description フィールドに今回の修正内容の要点を追記 (任意) + +### Phase 7: 検証 +- [ ] macOS 風環境 (もしくは `/work` が存在しないコンテナ) で `state.py init` が `--worktree` + なしで成功することを確認 (`NDF_WORKTREE_BASE` のセット例も併記) +- [ ] 既存 Linux コンテナ環境で `/work/worktrees/pr` が引き続き作られることを確認 + (`/work` が書ける状態を維持) +- [ ] `result.json` を変則スキーマ (`{"intent": "APPROVE", "comment_count": 3, ...}`) で + 書き出した状態で `state.py read-result` を呼び、state.json の `rounds[-1].gemini.intent` + が `"APPROVE"` で取り込まれることを確認 +- [ ] 空 result.json で `state.py read-result` を呼んだ際に exit 1 + 「event / intent が無い」 + 旨のエラーが出ることを確認 + +## PR 構成 + +**単一 PR で実装する** (合計差分は ~250 行以内の見込み、release branch 不要): + +- branch: `fix/PLAN20-cross-review-macos-and-result-schema` +- base: `main` +- 流れ: + 1. 上記 Phase 1〜6 を順次 commit (Phase 単位で分けると review しやすい) + 2. `/ndf:review-branch` でセルフレビュー + 3. `/ndf:pr` で PR 作成 (本 plan を `## Plan` セクションで参照) + 4. レビュー対応後 squash merge + +## 互換性方針 + +- **後方互換あり**: + - `/work/worktrees` が書ける環境は挙動不変 (`mkdir(exist_ok=True)` が成功するため) + - 既存の正規 result.json スキーマ (`event` / `comments_count`) は引き続き正規系 +- **新規挙動**: + - `NDF_WORKTREE_BASE` env 対応 (任意指定) + - `intent` / `comment_count` の別名 result.json も受理 (フォールバック) + - intent / event いずれも無い場合は **die** で fail (silent な None マージは廃止) +- 旧挙動でユーザが暗黙依存していた可能性のあるもの (`intent=None` でも judge が回る挙動) は、 + **本来 bug なので破壊する** 方針。CHANGELOG に明記する。 + +## リスクと対策 + +| リスク | 対策 | +|---|---| +| `/work` が書けるが実は別ユーザに所有された共有環境で `mkdir(exist_ok=True)` が成功してしまう | `NDF_WORKTREE_BASE` で明示オーバーライドできる旨を SKILL.md に書く | +| gemini が今度は別の別名 (`reviewEvent` 等) で書く | 別名フォールバックは 1 段のみとし、検知できないケースは intent 欠落 die で発見する | +| read-result の挙動変化で既存 cross-review セッションが中断する | リリース前にローカル `cross-review` を一度通しで実走させ回帰確認 | +| テスト追加で skill ディレクトリに pytest 環境が必要になる | `uv run pytest` を README に追記、CI 強制は今回はしない | + +## 完了の定義 + +- [ ] macOS 環境で `state.py init ` が `--worktree` 引数なしで成功する +- [ ] Linux コンテナ環境で従来どおり `/work/worktrees/pr` が使われる +- [ ] `result.json` が `intent` / `comment_count` の別名で書かれても `state.py read-result` で + intent が state に取り込まれる +- [ ] `result.json` から event / intent が両方欠落しているとき `state.py read-result` が + exit 1 で fail する +- [ ] 追加した pytest が全て pass する +- [ ] plugin version が `4.7.3` に上がり、CHANGELOG / 開発履歴に v4.7.3 の節がある +- [ ] SKILL.md / docs から `/work/worktrees/pr` のハードコードが取り除かれている + (state.json サンプル中の解決例 1 箇所のみ許容) +- [ ] `monitor.py` の EARLY_ERROR 検知が SKILL.md / docs 中の Markdown 表セルや + backtick / 「」 引用内のキーワードを誤検知しない (cross-review 自己レビュー時の + 誤 kill を解消) + +## 参考 + +- [issues/i17.md](./i17.md) — 元 issue (再現手順 / 回避策 / 修正提案を含む) +- 該当コード: + - `plugins/ndf/skills/cross-review/scripts/state.py:122` (worktree default) + - `plugins/ndf/skills/cross-review/scripts/state.py:244-265` (cmd_read_result) + - `plugins/ndf/skills/cross-review/scripts/launch-gemini.sh:90` (gemini result schema 指示) + - `plugins/ndf/skills/cross-review/scripts/launch-codex.sh:81-92` (codex 側 reference) +- 関連 PR: `devbasex/devbase#14` (再現発生 PR、検証参考) diff --git a/issues/i17.md b/issues/i17.md new file mode 100644 index 00000000..6b18e05a --- /dev/null +++ b/issues/i17.md @@ -0,0 +1,245 @@ +# [cross-review] worktree デフォルトパスと gemini result.json スキーマの不整合 + +- 対象 plugin: `ndf` v4.7.2 +- 対象 skill: `ndf:cross-review` +- 報告者: takemi-ohama +- 検出経緯: `devbasex/devbase#14` に対する `/ndf:cross-review 14` 実行中 + +`/ndf:cross-review` を macOS ホストで実行したところ、独立した 2 件の不具合に遭遇しました。 +どちらも回避手順は確立できたものの、毎回手動介入が必要になっており、 +プラグイン側で対応していただきたいです。 + +--- + +## 1. worktree デフォルトパスが `/work/worktrees/prN` 固定で macOS で破綻する + +### 現象 + +`scripts/state.py init ` を引数省略で実行すると、worktree 作成が以下で失敗する: + +``` +❌ command failed (git worktree add --detach /work/worktrees/pr14 origin/feature/...): +fatal: could not create leading directories of '/work/worktrees/pr14/.git': +Read-only file system +``` + +macOS では `/work` がそもそも存在せず（root 直下は SIP で書き込み不可）、 +コンテナ／Linux ホスト前提のパスが固定値として埋め込まれている。 + +### 該当箇所 + +`skills/cross-review/scripts/state.py:122` + +```python +worktree = args.worktree or f"/work/worktrees/pr{pr}" +``` + +SKILL.md / docs 側でも `/work/worktrees/pr` を前提として記述されている: + +- `skills/cross-review/SKILL.md`「事前確認」表 #2「worktree 分離」 +- `docs/01-state-and-review.md` Step 0 解説 + +### 影響 + +- ホスト OS が macOS / 非コンテナ環境の場合、毎回 `--worktree` を明示指定しなければ init できない。 +- 初回ユーザは「`/work` がない」エラーメッセージから worktree 引数の存在に気付きにくい。 +- skill 内 prompt (`Agent(...)` に渡される prompt) や docs に `/work/worktrees/pr` が + ハードコードされているため、サブエージェント側も明示置換が必要。 + +### 回避策（当方で実施） + +```bash +mkdir -p $HOME/work/worktrees +state.py init 14 --worktree $HOME/work/worktrees/pr14 ... +``` + +### 修正提案 + +優先度の高い順に 2 案: + +1. **デフォルト値を環境変数 + OS 判定で導出する** + ```python + def _default_worktree_base() -> pathlib.Path: + env = os.environ.get("NDF_WORKTREE_BASE") + if env: + return pathlib.Path(env) + # /work が書き込み可能ならそれを優先（既存 Linux コンテナ環境互換） + work = pathlib.Path("/work/worktrees") + try: + work.mkdir(parents=True, exist_ok=True) + return work + except OSError: + pass + return pathlib.Path.home() / "work" / "worktrees" + + worktree = args.worktree or str(_default_worktree_base() / f"pr{pr}") + ``` + - 既存のコンテナ環境 (`/work` 書込可) は挙動を変えない + - macOS / WSL / その他は `$HOME/work/worktrees/prN` にフォールバック + - `NDF_WORKTREE_BASE` でユーザがオーバーライド可能 + +2. **SKILL.md / docs / launcher prompt 内のハードコード `/work/worktrees/pr` を、 + state.json の `worktree_path` から読むようテンプレート化** + - サブエージェント prompt 側で `state.json` を参照させる + - もしくはメインから `WORKTREE_PATH` 変数を埋め込むテンプレートにする + +--- + +## 2. gemini の `result.json` スキーマが launcher 間で揺れて `state.py read-result` が intent を欠落させる + +### 現象 + +`launch-gemini.sh` 起動後、gemini が書き出した +`$TMP_DIR/gemini-review-pr-result.json` を `state.py read-result gemini` +にかけると、`intent=None / posted_as=None / comments=None` で state に取り込まれる: + +``` +✅ gemini: intent=None posted_as=None comments=None +``` + +その結果 `state.py judge` で +`gemini=None` 扱いになり、両者 APPROVE 状態でも収束判定されずに次ラウンドへ進んでしまう +（最悪 max_rounds 到達まで空回りする）。 + +### 実例 + +PR #14 round 2 で gemini が書き出した result.json: + +```json +{ + "repo": "devbasex/devbase", + "pr": 14, + "round": 2, + "reviewer": "gemini", + "intent": "APPROVE", + "comment_count": 3, + "summary": "全体的に堅牢で..." +} +``` + +一方 codex が書き出した（および `read-result` が期待している）スキーマ: + +```json +{ + "event": "APPROVE", + "posted_as": "COMMENT", + "comments_count": 0, + "review_url": "https://.../pull/14#pullrequestreview-...", + "by_severity": {"critical": 0, "major": 0, "minor": 0, "nit": 0} +} +``` + +主要フィールドが全てズレている (`event` vs `intent`、`comments_count` vs `comment_count`、 +`review_url` 欠落、`posted_as` 欠落、`by_severity` 欠落)。 + +### 原因 + +`launch-gemini.sh:90` が **フィールドリストを列挙していない**: + +``` +- 投稿後、サマリを **$TMP_DIR/gemini-review-pr$STATE_PR-result.json** に書く + （フォーマットは launch-codex.sh と同じ） +``` + +一方 `launch-codex.sh:81-88` は具体的なスキーマを明示している: + +``` +- 投稿後、サマリを **$TMP_DIR/codex-review-pr$STATE_PR-result.json** に書く: + { + "event": "REQUEST_CHANGES", + "posted_as": "COMMENT", + "comments_count": 5, + "review_url": "https://.../pull/$PR#pullrequestreview-...", + "by_severity": {"critical": 0, "major": 3, "minor": 2, "nit": 0} + } +``` + +gemini は launcher prompt のみを文脈にもつため、「同じフォーマット」だけでは +launch-codex.sh の中身を読みに行けず、自分流のスキーマで書き出してしまう +（特にコンテキスト窓内に launch-codex.sh の本文が無い場合）。 + +加えて `state.py:244-265` `cmd_read_result` は `event` フィールドしか見ない: + +```python +st["rounds"][-1][agent] = { + "intent": r.get("event"), + "posted_as": r.get("posted_as", r.get("event")), + "comments": r.get("comments_count"), + ... +} +``` + +そのため `intent` / `comment_count` で書かれても拾えない。 + +### 影響 + +- gemini の APPROVE が認識されず、判定が「片方 None」状態で次ラウンドへ +- 最悪、無駄な round と CI / レビューコメントを増やしながら `max_rounds` 到達で `final=max_rounds` 終了 +- ユーザが state.json を手で書き換えるまで気付きにくい（read-result は exit 0 で成功扱い） + +### 回避策（当方で実施） + +`state.json` の `rounds[-1].gemini` を `jq` で直接パッチして再度 `judge` を回した: + +```bash +jq '.rounds[-1].gemini = { + intent: "APPROVE", posted_as: "COMMENT", + comments: 3, + review_url: "https://github.com/...", + by_severity: {} +}' "$STATE_FILE" > "$STATE_FILE.tmp" && mv "$STATE_FILE.tmp" "$STATE_FILE" +``` + +### 修正提案 + +両側から手当てするのが堅実です: + +1. **`launch-gemini.sh` 側に codex と同じフィールドリストを明示** + ``` + - 投稿後、サマリを $TMP_DIR/gemini-review-pr$STATE_PR-result.json に **必ず以下のキーで** 書く: + { + "event": "APPROVE" | "REQUEST_CHANGES" | "COMMENT", + "posted_as": "APPROVE" | "REQUEST_CHANGES" | "COMMENT", + "comments_count": , + "review_url": "", + "by_severity": {"critical": 0, "major": 0, "minor": 0, "nit": 0} + } + `intent` / `comment_count` 等の別名は使わないこと。 + ``` + - 単に「codex と同じ」と書いただけでは LLM がスキーマを再現できない実例なので、 + **両 launcher に同一のフィールドブロックをコピペで持たせる**のが安全。 + - もしくは共通プロンプト断片を `scripts/_result_schema.txt` 等に切り出して両方が + ヒアドキュメントで埋め込む構成に。 + +2. **`state.py read-result` 側で別名フィールドにフォールバックする** + ```python + intent = r.get("event") or r.get("intent") + posted_as = r.get("posted_as") or r.get("event") or intent + comments = r.get("comments_count") + if comments is None: + comments = r.get("comment_count") + review_url = r.get("review_url") + ``` + - 後方互換のための保険。スキーマ違反でも壊れずに取り込めるようにする。 + - さらに「`event` も `intent` も無ければ exit 1 + 警告」にして + **無言で None を入れる挙動をやめる**のが望ましい + （現状は judge まで進んでから初めて発覚するため発見が遅れる）。 + +3. **テスト追加** + - `tests/test_state_py.py` 等で「gemini が `intent`/`comment_count` を使う変則 JSON」を + 入力にした read-result の挙動を pin する + +--- + +## 補足: 再現環境 + +| 項目 | 値 | +|---|---| +| OS | macOS 25.5.0 (Darwin) | +| plugin | `ndf` v4.7.2 (`~/.claude/plugins/cache/ai-plugins/ndf/4.7.2`) | +| codex CLI | `/opt/homebrew/bin/codex` | +| gemini CLI | `/opt/homebrew/bin/gemini` | +| 対象 PR | https://github.com/devbasex/devbase/pull/14 | +| TMP_DIR | `/Users/takemi_ohama/.gemini/tmp/pr14` | + +参考: 当該 run の state.json / result.json は上記 TMP_DIR に残っており、必要なら共有可能。 diff --git a/plugins/ndf/.claude-plugin/plugin.json b/plugins/ndf/.claude-plugin/plugin.json index 64943573..1fb2f118 100644 --- a/plugins/ndf/.claude-plugin/plugin.json +++ b/plugins/ndf/.claude-plugin/plugin.json @@ -1,6 +1,6 @@ { "name": "ndf", - "version": "4.7.2", + "version": "4.7.3", "description": "Integrated plugin with 8 specialized agents (model-tiered: opus/sonnet/haiku), 39 skills including official mcp-builder, on-demand loader for Anthropic official skills, generic workflow/principle skills, skill usage statistics, pytest-playwright based scenario E2E testing (v0.5.0: BREAKING — package renamed scenario_test → playwright_kit, fixtures/CLI ndf_*/--ndf-* → pwk_*/--pwk-*, all-in-one runtime layout enabling Skill-independent operation via init_project.sh + run.sh, accessibility/web vitals autouse, overlay (formerly HUD), report.md, Drive integration, body_check autouse enabled by default to detect server-rendered PHP/SSR errors leaked into HTML), Google Drive/Chat integration, and Codex CLI integration via /ndf:codex skill. Transcript retention is automatically kept at >= 90 days. BREAKING (v4.0.0): Codex MCP server is removed (use /ndf:codex skill); legacy CLAUDE.ndf.md detection hook and /ndf:cleanup skill are removed (obsolete since v3.0.0). Serena MCP is a separate plugin (mcp-serena).", "author": { "name": "takemi-ohama", diff --git a/plugins/ndf/CHANGELOG.md b/plugins/ndf/CHANGELOG.md index 6a6212a3..ff7737ca 100644 --- a/plugins/ndf/CHANGELOG.md +++ b/plugins/ndf/CHANGELOG.md @@ -1,5 +1,58 @@ # NDF Plugin CHANGELOG +### v4.7.3 (cross-review: macOS 対応 worktree base + result.json スキーマ堅牢化) + +`/ndf:cross-review` を非 Linux コンテナ環境 (macOS / WSL 等) でも素直に動かせるよう、 +worktree のデフォルトパス解決を環境適応型に変更。あわせて gemini が変則スキーマで +result.json を書き出すケースで intent が silent に None マージされて judge が空回り +する不具合を恒久対応する PATCH リリース。 + +- **worktree デフォルトパスの環境適応** (`skills/cross-review/scripts/state.py`): + - `state.py init` が以下の優先順で worktree 親ディレクトリを解決: + 1. `NDF_WORKTREE_BASE` 環境変数 (明示オーバーライド) + 2. `/work/worktrees` (Linux コンテナ環境互換。書込可ならこちらを使用) + 3. `$HOME/work/worktrees` (macOS / WSL 等のフォールバック) + - 既存の Linux コンテナ環境では `/work/worktrees` が引き続き使われ挙動不変。 + - SKILL.md / docs から `/work/worktrees/pr` のハードコードを除去し、 + `/pr` 表記に統一 (state.json サンプル中の解決例 1 箇所のみ残存)。 +- **gemini result.json スキーマの明示化** (`skills/cross-review/scripts/launch-gemini.sh`): + - 「フォーマットは launch-codex.sh と同じ」という曖昧指示を、codex と同一の + フィールド列挙ブロック (`event` / `posted_as` / `comments_count` / `review_url` / + `by_severity`) に置き換え。`intent` / `comment_count` 等の別名を使わないことを明記。 +- **`state.py read-result` の堅牢化**: + - 仕様 (`event` / `comments_count`) を優先しつつ、別名 (`intent` / `comment_count`) + も拾えるようフォールバックを追加。 + - `event` / `intent` いずれも欠落している場合は `die()` (exit 1) で fail する。 + 旧挙動 (silent な `intent=None` マージで judge が空回り) は **破壊的に修正**。 +- **monitor.py EARLY_ERROR 誤検知の修正** (`skills/cross-review/scripts/monitor.py`): + - SKILL.md / `docs/01-state-and-review.md` の Markdown 表セル内で FATAL キーワード + (`「quota exceeded」`「sandbox error」等) を列挙しており、codex がレビュー時に + それを echo すると err.log 上で `_scan_early_fatal()` が誤発火してプロセスを + kill していた。以下 2 段の防御で恒久対応: + 1. `EARLY_ERROR_BENIGN` に Markdown 表セル行 (`^\|`) を追加。 + 2. マッチ位置が backtick / 日本語「」で引用されている場合に benign 扱いする + `_match_is_quoted()` ヘルパを追加し、`_scan_patterns()` から呼ぶ。 + - FATAL パターンから `^.*` プレフィックスを外し、`m.start()` をキーワード位置に + 合わせて引用判定が機能するように修正。 +- **pytest 追加** (`skills/cross-review/tests/`): + - `test_state_read_result.py` — 正規/変則/欠落スキーマ 4 ケース。 + - `test_default_worktree_base.py` — env / legacy / fallback の 3 ケース。 + - `test_monitor_early_error.py` — Markdown 表 / backtick / 日本語クォート引用の + benign 判定と、本物 fatal が依然検知される回帰テスト 7 ケース。 + - ローカル実行: `uv run --with pytest pytest plugins/ndf/skills/cross-review/tests`。 +- **関連 issue / plan**: + - `issues/i17.md` (再現報告) / `issues/PLAN20_cross-review-worktree-and-result-schema-fix.md` (実装プラン)。 + +#### 既存ユーザへの影響 + +- `/work/worktrees` が書ける環境 (大半の Linux コンテナ環境): **挙動不変**。 +- macOS / WSL 等で `/work` が書けない環境: `--worktree` 引数なしでも + `$HOME/work/worktrees/pr` に自動フォールバックして init が成功する。 +- gemini が変則スキーマ (`intent` / `comment_count`) で result.json を書く現象を + 観測していたユーザ: フォールバックで自動的に取り込まれるようになる。 +- `result.json` から `event` / `intent` が両方欠落しているケースは exit 1 で + 早期 fail する (旧: judge 段階まで silent に None が伝播)。 + ### v4.7.0 (fix / cross-review: 修正ポリシー刷新 + CI 完了待ち廃止) `/ndf:fix` と `/ndf:cross-review` の修正方針を見直し、PR の最終的なコード品質を diff --git a/plugins/ndf/skills/cross-review/SKILL.md b/plugins/ndf/skills/cross-review/SKILL.md index ffa3c3cc..1c2141fa 100644 --- a/plugins/ndf/skills/cross-review/SKILL.md +++ b/plugins/ndf/skills/cross-review/SKILL.md @@ -75,10 +75,21 @@ state.json の読み書きや AI launcher 起動・完了待ちは全て委譲 | # | 対策 | スクリプト側で何をするか | |---|---|---| | 1 | 自分の PR 判定（422 回避） | `gh api user` と `gh pr view --json author` を比較し `is_own_pr` / `event_downgrade` を state.json に書く | -| 2 | worktree 分離 | `git worktree add /work/worktrees/pr ` を冪等実行 | +| 2 | worktree 分離 | `git worktree add /pr ` を冪等実行（`` は `NDF_WORKTREE_BASE` env > `/work/worktrees` > `$HOME/work/worktrees` の優先順で解決） | | 3 | gemini trusted directory | `launch-gemini.sh` が `GEMINI_CLI_TRUST_WORKSPACE=true` + `--skip-trust` を必ず併用。さらに **tmp dir は `~/.gemini/tmp//`** を採用し、gemini の workspace 制約 (workspace 外の `read_file` / `write_file` がブロックされる) を回避 | | 4 | 既存コメント差分 | `gh api .../comments --paginate` を `$TMP_DIR/cross-review-pr-existing-comments.txt` に保存し、gemini プロンプトには **内容をインライン埋め込み**、codex プロンプトには path を渡す | +### `` の解決順 + +`state.py init` は worktree の親ディレクトリを以下の優先順で解決する: + +1. `NDF_WORKTREE_BASE` 環境変数（明示オーバーライド） +2. `/work/worktrees`（Linux コンテナ環境互換。書き込み可能ならこちらを使用） +3. `$HOME/work/worktrees`（macOS / WSL 等のフォールバック） + +解決した実パスは `state.json` の `worktree_path` に書かれるため、後続スクリプトや +サブエージェント prompt は state.json から読めば追従できる。 + ### intent / posted_as の両保持（最重要） GitHub は **自分の PR には `REQUEST_CHANGES` でレビューを投稿できない** @@ -98,7 +109,7 @@ GitHub は **自分の PR には `REQUEST_CHANGES` でレビューを投稿で ```mermaid flowchart TD - Start([事前確認 / loop 開始前に 1 回だけ]):::phase --> Init["worktree 作成 + state.json 初期化
・自分の PR 判定 → event downgrade 設定
・/work/worktrees/pr<PR> を用意
・既存コメントスナップショット保存"] + Start([事前確認 / loop 開始前に 1 回だけ]):::phase --> Init["worktree 作成 + state.json 初期化
・自分の PR 判定 → event downgrade 設定
・<worktree-base>/pr<PR> を用意
・既存コメントスナップショット保存"] Init --> Round["Round N start
current_pr = PR#"]:::phase Round -.並列バックグラウンド.-> Codex["/ndf:review <PR> codex
(AI が gh api で直接投稿)
body 先頭: cross-review / round N / codex / intent
→ result.json (intent + posted_as)"] diff --git a/plugins/ndf/skills/cross-review/docs/01-state-and-review.md b/plugins/ndf/skills/cross-review/docs/01-state-and-review.md index 4424bac6..e18ae552 100644 --- a/plugins/ndf/skills/cross-review/docs/01-state-and-review.md +++ b/plugins/ndf/skills/cross-review/docs/01-state-and-review.md @@ -94,7 +94,7 @@ cd "$WORKTREE" 1. 既存 state.json があり `final == null` なら再開 2. 自分の PR 判定（`gh api user` と `gh pr view --json author` を比較） -3. worktree 作成（`/work/worktrees/pr`） +3. worktree 作成（`/pr`。`` は `NDF_WORKTREE_BASE` env > `/work/worktrees` > `$HOME/work/worktrees` の優先順で解決。実 path は state.json の `worktree_path` を参照） 4. 既存コメントスナップショット → `$TMP_DIR/cross-review-pr-existing-comments.txt` 5. state.json 書き出し @@ -163,7 +163,7 @@ fi launcher が生成するプロンプトに以下を強制している: - **headRefOid (commit_id) を明示**: AI が自前で取得すると baseRefOid を誤って入れる事故が多発 -- **作業 worktree の絶対パス**: 「ファイル読み取りは必ず `/work/worktrees/pr/` 配下の絶対パスを使う」 +- **作業 worktree の絶対パス**: 「ファイル読み取りは必ず worktree 配下の絶対パスを使う」（実 path は state.json の `worktree_path` を参照。`` は `NDF_WORKTREE_BASE` env > `/work/worktrees` > `$HOME/work/worktrees` の優先順で解決） - **event ダウングレード警告**: `event_downgrade=true` のときは payload の `event` を `COMMENT` に - **既存コメント差分**: `$TMP_DIR/cross-review-pr-existing-comments.txt` を読んで重複指摘禁止 - **review body 先頭 prefix**: diff --git a/plugins/ndf/skills/cross-review/scripts/launch-gemini.sh b/plugins/ndf/skills/cross-review/scripts/launch-gemini.sh index c688c05d..9e24c9e1 100755 --- a/plugins/ndf/skills/cross-review/scripts/launch-gemini.sh +++ b/plugins/ndf/skills/cross-review/scripts/launch-gemini.sh @@ -87,8 +87,22 @@ $EXISTING_INLINE - 設計レベル・PR 横断の **修正提案のみ** 書く - 書くことが無ければ prefix 行 + 1 行サマリだけで良い (褒め言葉や評価文は不要) -- 投稿後、サマリを **$TMP_DIR/gemini-review-pr$STATE_PR-result.json** に書く（フォーマットは launch-codex.sh と同じ） +- 投稿後、サマリを **$TMP_DIR/gemini-review-pr$STATE_PR-result.json** に + **必ず以下のキーで** 書く: + \`\`\`json + { + "event": "APPROVE", + "posted_as": "COMMENT", + "comments_count": 3, + "review_url": "https://github.com/.../pull/$PR#pullrequestreview-...", + "by_severity": {"critical": 0, "major": 0, "minor": 0, "nit": 0} + } + \`\`\` + - \`intent\` / \`comment_count\` 等の別名は使わないこと + - \`event\` の値は \`APPROVE\` / \`REQUEST_CHANGES\` / \`COMMENT\` のいずれか + - \`event_downgrade=true\` のとき \`posted_as\` は \`COMMENT\` にダウングレード可 - payload は **$TMP_DIR/gemini-review-pr$STATE_PR-round$ROUND-payload.json** に保存 + （振動検知用、\`{ "comments": [{path, line, body, severity}, ...] }\` 形式） ## 守るべきこと - **リポジトリ編集禁止**。gh api での投稿のみ許可 diff --git a/plugins/ndf/skills/cross-review/scripts/monitor.py b/plugins/ndf/skills/cross-review/scripts/monitor.py index 89cb1d8a..3c25e7ac 100755 --- a/plugins/ndf/skills/cross-review/scripts/monitor.py +++ b/plugins/ndf/skills/cross-review/scripts/monitor.py @@ -77,12 +77,13 @@ re.compile(r'^Approval mode overridden to "default"', re.MULTILINE), # 認証 / 権限系（行頭限定） re.compile(r"^(?:Authentication failed|Permission denied)", re.MULTILINE), - # quota / rate limit - re.compile(r"^.*\b(?:quota exceeded|rate limit exceeded)\b", re.MULTILINE | re.IGNORECASE), + # quota / rate limit （`m.start()` をキーワード位置に合わせるため `^.*` を付けない。 + # `_match_is_quoted()` が backtick / 「」 引用を判定するために match 開始位置を使うため） + re.compile(r"\b(?:quota exceeded|rate limit exceeded)\b", re.IGNORECASE), # API key 系 - re.compile(r"^.*\bAPI key (?:not found|missing|invalid)", re.MULTILINE | re.IGNORECASE), + re.compile(r"\bAPI key (?:not found|missing|invalid)\b", re.IGNORECASE), # codex 固有: sandbox エラー - re.compile(r"^.*\bsandbox error\b", re.MULTILINE | re.IGNORECASE), + re.compile(r"\bsandbox error\b", re.IGNORECASE), ] # 行頭の生 `Error:` / `Traceback` 系は **kill しない警告のみ** に降格。 @@ -105,10 +106,31 @@ re.compile(r"^[ +-].*[\|`]", re.MULTILINE), # markdown のリスト / 引用 re.compile(r"^\s*[-*>]\s", re.MULTILINE), + # markdown の表セル行 (`| ... | ...` 形式)。SKILL.md / docs/*.md が + # 検知パターンを表で列挙しており、それを codex が echo すると誤検知する。 + re.compile(r"^\|", re.MULTILINE), # warning は致命ではない re.compile(r"^warning: ", re.IGNORECASE | re.MULTILINE), ] + +def _match_is_quoted(line: str, match_start: int, match_end: int) -> bool: + """マッチ位置がドキュメント引用 (backtick / 日本語「」) に囲まれているか判定。 + + - backtick: マッチ開始までの `` ` `` カウントが奇数 かつ マッチ終了以降に `` ` `` がある + - 日本語クォート: マッチ開始までに直近の `「` が `」` よりも後 かつ マッチ終了以降に `」` がある + + Why: SKILL.md / docs/*.md 内で FATAL キーワードを `「quota exceeded」` のように + 引用列挙しており、codex がそれを echo する。引用形は本物のエラーではない。 + """ + before = line[:match_start] + after = line[match_end:] + if before.count("`") % 2 == 1 and "`" in after: + return True + if before.rfind("「") > before.rfind("」") and "」" in after: + return True + return False + CODEX_SENTINEL = re.compile(r"^tokens used$", re.MULTILINE) @@ -266,6 +288,9 @@ def _scan_patterns( # 評価すれば判定可能で、文脈窓を広げると誤判定の原因になる。 if any(b.search(line) for b in EARLY_ERROR_BENIGN): continue + # マッチ部位が backtick / 日本語「」 で引用されている場合も benign。 + if _match_is_quoted(line, m.start() - line_start, m.end() - line_start): + continue return line.strip() return None diff --git a/plugins/ndf/skills/cross-review/scripts/state.py b/plugins/ndf/skills/cross-review/scripts/state.py index 1784509a..45305b73 100755 --- a/plugins/ndf/skills/cross-review/scripts/state.py +++ b/plugins/ndf/skills/cross-review/scripts/state.py @@ -36,6 +36,27 @@ # ---------------- helpers ---------------- +def _default_worktree_base() -> pathlib.Path: + """worktree の親ディレクトリを環境に応じて解決する。 + + 優先順位: + 1. 環境変数 NDF_WORKTREE_BASE (明示オーバーライド) + 2. /work/worktrees (Linux コンテナ環境互換、書き込み可能ならそれを使う) + 3. $HOME/work/worktrees (macOS / WSL 等のフォールバック) + """ + env = os.environ.get("NDF_WORKTREE_BASE") + if env: + return pathlib.Path(env) + legacy = pathlib.Path("/work/worktrees") + try: + legacy.mkdir(parents=True, exist_ok=True) + # mkdir 成功 = 書き込み可能 → 既存環境互換でこちらを使う + return legacy + except OSError: + pass + return pathlib.Path.home() / "work" / "worktrees" + + def _tmp_dir(workspace: str | None = None) -> pathlib.Path: """cross-review 用 tmp ディレクトリを決定する。 @@ -119,7 +140,7 @@ def cmd_init(args: argparse.Namespace) -> None: # os.getcwd() の basename (= 親リポジトリ名) を採用していたため、 # launch-gemini.sh で `cd $WORKTREE` した後の gemini が # `~/.gemini/tmp/` への write をブロックして hard timeout していた。 - worktree = args.worktree or f"/work/worktrees/pr{pr}" + worktree = args.worktree or str(_default_worktree_base() / f"pr{pr}") tmp_dir = _tmp_dir(worktree) state_file = tmp_dir / f"cross-review-pr{pr}-state.json" @@ -250,19 +271,33 @@ def cmd_read_result(args: argparse.Namespace) -> None: die(f"{agent}: result 未生成 ({rfile})") r = json.loads(rfile.read_text()) + + # 別名フィールドへのフォールバック (gemini が `intent` / `comment_count` を使う変則 JSON を + # 書き出す既知のケースに対応する。仕様としては `event` / `comments_count` が正) + intent = r.get("event") or r.get("intent") + posted_as = r.get("posted_as") or intent + comments = r.get("comments_count") + if comments is None: + comments = r.get("comment_count") + + if intent is None: + die( + f"{agent}: result.json に event / intent フィールドが無い ({rfile})。" + " launcher prompt のスキーマ違反の可能性。" + ) + st = _load(pr) if not st.get("rounds"): die(f"{agent}: state.rounds が空。`state.py start-round` を先に呼んでください") st["rounds"][-1][agent] = { - "intent": r.get("event"), - "posted_as": r.get("posted_as", r.get("event")), - "comments": r.get("comments_count"), + "intent": intent, + "posted_as": posted_as, + "comments": comments, "review_url": r.get("review_url"), "by_severity": r.get("by_severity", {}), } _save(pr, st) - info(f"✅ {agent}: intent={r.get('event')} posted_as={r.get('posted_as', r.get('event'))} " - f"comments={r.get('comments_count')}") + info(f"✅ {agent}: intent={intent} posted_as={posted_as} comments={comments}") def cmd_judge(args: argparse.Namespace) -> None: diff --git a/plugins/ndf/skills/cross-review/tests/__init__.py b/plugins/ndf/skills/cross-review/tests/__init__.py new file mode 100644 index 00000000..e69de29b diff --git a/plugins/ndf/skills/cross-review/tests/conftest.py b/plugins/ndf/skills/cross-review/tests/conftest.py new file mode 100644 index 00000000..a5b257b5 --- /dev/null +++ b/plugins/ndf/skills/cross-review/tests/conftest.py @@ -0,0 +1,47 @@ +"""pytest 共通フィクスチャ。 + +`scripts/state.py` は uv self-contained script として `#!/usr/bin/env -S uv run --script` +で起動される運用だが、テストでは関数を直接 import したい。 +importlib.util で source loader 経由で読み込む。 +""" +from __future__ import annotations + +import importlib.util +import pathlib +import sys +import types + +_HERE = pathlib.Path(__file__).resolve().parent +_SCRIPT = _HERE.parent / "scripts" / "state.py" +_MONITOR = _HERE.parent / "scripts" / "monitor.py" + + +def _load_module(name: str, path: pathlib.Path) -> types.ModuleType: + spec = importlib.util.spec_from_file_location(name, path) + if spec is None or spec.loader is None: + raise RuntimeError(f"failed to load {path}") + mod = importlib.util.module_from_spec(spec) + sys.modules[name] = mod + spec.loader.exec_module(mod) + return mod + + +def _load_state_module() -> types.ModuleType: + return _load_module("cross_review_state", _SCRIPT) + + +def _load_monitor_module() -> types.ModuleType: + return _load_module("cross_review_monitor", _MONITOR) + + +import pytest + + +@pytest.fixture(scope="session") +def state_mod() -> types.ModuleType: + return _load_state_module() + + +@pytest.fixture(scope="session") +def monitor_mod() -> types.ModuleType: + return _load_monitor_module() diff --git a/plugins/ndf/skills/cross-review/tests/test_default_worktree_base.py b/plugins/ndf/skills/cross-review/tests/test_default_worktree_base.py new file mode 100644 index 00000000..2515f270 --- /dev/null +++ b/plugins/ndf/skills/cross-review/tests/test_default_worktree_base.py @@ -0,0 +1,44 @@ +"""state.py `_default_worktree_base()` の解決順テスト。 + +優先順位: + 1. 環境変数 NDF_WORKTREE_BASE + 2. /work/worktrees (書込可能) + 3. $HOME/work/worktrees (フォールバック) +""" +from __future__ import annotations + +import pathlib + + +def test_env_override_takes_precedence(monkeypatch, tmp_path, state_mod): + explicit = tmp_path / "custom-base" + monkeypatch.setenv("NDF_WORKTREE_BASE", str(explicit)) + assert state_mod._default_worktree_base() == explicit + + +def test_fallback_to_home_when_legacy_unwritable(monkeypatch, tmp_path, state_mod): + """`/work/worktrees` への mkdir が失敗する環境では $HOME/work/worktrees を返す。 + + `Path.mkdir` を patch して `/work/worktrees` を書込不可な状態をエミュレートする。 + """ + monkeypatch.delenv("NDF_WORKTREE_BASE", raising=False) + fake_home = tmp_path / "home" + fake_home.mkdir() + monkeypatch.setenv("HOME", str(fake_home)) + # Path.home() は HOME env を再評価しないため明示的に差し替える + monkeypatch.setattr( + state_mod.pathlib.Path, "home", + classmethod(lambda cls: cls(str(fake_home))), + ) + + orig_mkdir = state_mod.pathlib.Path.mkdir + + def fake_mkdir(self, *args, **kwargs): + if str(self) == "/work/worktrees": + raise OSError("read-only file system") + return orig_mkdir(self, *args, **kwargs) + + monkeypatch.setattr(state_mod.pathlib.Path, "mkdir", fake_mkdir) + + result = state_mod._default_worktree_base() + assert result == pathlib.Path(str(fake_home)) / "work" / "worktrees" diff --git a/plugins/ndf/skills/cross-review/tests/test_monitor_early_error.py b/plugins/ndf/skills/cross-review/tests/test_monitor_early_error.py new file mode 100644 index 00000000..3f4134df --- /dev/null +++ b/plugins/ndf/skills/cross-review/tests/test_monitor_early_error.py @@ -0,0 +1,81 @@ +"""monitor.py `_scan_early_fatal()` の誤検知防止テスト。 + +回帰: SKILL.md / docs/*.md 内の以下のような行は **fatal 検知してはいけない**。 +- Markdown 表セル (`| ... | quota exceeded ... |`) +- backtick で囲まれた quote (`` `quota exceeded` ``) +- 日本語クォートで囲まれた quote (`「quota exceeded」`) +codex がレビュー対象の doc を echo した際に上記が err.log に書かれることで、 +従来の monitor.py は無関係なドキュメント記述を fatal とみなしてプロセスを kill していた。 +""" +from __future__ import annotations + +import pathlib + + +def _write(p: pathlib.Path, content: str) -> pathlib.Path: + p.write_text(content, encoding="utf-8") + return p + + +def test_real_quota_exceeded_is_detected(tmp_path, monitor_mod): + """行頭の本物 `quota exceeded` は依然として fatal 扱い。""" + log = _write(tmp_path / "err.log", "quota exceeded: please upgrade\n") + assert monitor_mod._scan_early_fatal(log) is not None + + +def test_markdown_table_row_is_benign(tmp_path, monitor_mod): + """Markdown 表セル行内のキーワードは doc 引用として無視。""" + log = _write( + tmp_path / "err.log", + "| early-error | `^Error:` / 「quota exceeded」「sandbox error」を含む行 |\n", + ) + assert monitor_mod._scan_early_fatal(log) is None + + +def test_backtick_quoted_keyword_is_benign(tmp_path, monitor_mod): + """backtick 引用内のキーワードは doc 引用として無視。""" + log = _write( + tmp_path / "err.log", + "explanation of `quota exceeded` pattern handling here\n", + ) + assert monitor_mod._scan_early_fatal(log) is None + + +def test_japanese_quote_wrapped_keyword_is_benign(tmp_path, monitor_mod): + """日本語「」内のキーワードは doc 引用として無視。""" + log = _write( + tmp_path / "err.log", + "「quota exceeded」を含む行を検出 (diff/doc 引用文中の同語句は誤検知しない)\n", + ) + assert monitor_mod._scan_early_fatal(log) is None + + +def test_sandbox_error_in_table_is_benign(tmp_path, monitor_mod): + """表セル内の `sandbox error` も無視。""" + log = _write( + tmp_path / "err.log", + "| pattern | codex 固有: 「sandbox error」を含む行を検出 |\n", + ) + assert monitor_mod._scan_early_fatal(log) is None + + +def test_real_sandbox_error_still_detected(tmp_path, monitor_mod): + """素の `sandbox error` は依然として fatal 扱い。""" + log = _write(tmp_path / "err.log", "Internal sandbox error: cannot start\n") + assert monitor_mod._scan_early_fatal(log) is not None + + +def test_match_is_quoted_helper(monitor_mod): + """`_match_is_quoted()` 単体テスト。""" + line_backtick = "see `quota exceeded` doc" + start = line_backtick.index("quota") + end = start + len("quota exceeded") + assert monitor_mod._match_is_quoted(line_backtick, start, end) + + line_jp = "「quota exceeded」と書かれた行" + start = line_jp.index("quota") + end = start + len("quota exceeded") + assert monitor_mod._match_is_quoted(line_jp, start, end) + + line_raw = "quota exceeded happened" + assert not monitor_mod._match_is_quoted(line_raw, 0, len("quota exceeded")) diff --git a/plugins/ndf/skills/cross-review/tests/test_state_read_result.py b/plugins/ndf/skills/cross-review/tests/test_state_read_result.py new file mode 100644 index 00000000..4cea082d --- /dev/null +++ b/plugins/ndf/skills/cross-review/tests/test_state_read_result.py @@ -0,0 +1,122 @@ +"""state.py cmd_read_result の result.json スキーマ揺れに対するテスト。 + +カバー範囲: + 1. 正規スキーマ (`event` / `comments_count`) → state にマージされる + 2. 変則スキーマ (`intent` / `comment_count`) → 同等にマージされる (フォールバック) + 3. event / intent いずれも欠落 → die(exit 1) で fail + state 不変 +""" +from __future__ import annotations + +import argparse +import json +import pathlib + +import pytest + + +PR = 4242 +AGENT = "gemini" + + +def _seed_state(tmp_dir: pathlib.Path) -> dict: + state = { + "current_pr": PR, + "rounds": [ + {"round": 1, "pr": PR, "started_at": "2026-05-21T00:00:00+00:00"} + ], + "final": None, + } + (tmp_dir / f"cross-review-pr{PR}-state.json").write_text(json.dumps(state)) + return state + + +def _make_args(file_path: pathlib.Path) -> argparse.Namespace: + return argparse.Namespace(pr=PR, agent=AGENT, file=str(file_path)) + + +def _read_state(tmp_dir: pathlib.Path) -> dict: + return json.loads((tmp_dir / f"cross-review-pr{PR}-state.json").read_text()) + + +@pytest.fixture() +def patched_tmp_dir(monkeypatch, tmp_path, state_mod): + """`CROSS_REVIEW_TMP_DIR` を tmp_path に向ける。""" + monkeypatch.setenv("CROSS_REVIEW_TMP_DIR", str(tmp_path)) + return tmp_path + + +def test_canonical_schema(patched_tmp_dir, state_mod): + tmp_dir = patched_tmp_dir + _seed_state(tmp_dir) + result = { + "event": "APPROVE", + "posted_as": "APPROVE", + "comments_count": 0, + "review_url": "https://example/pr/1#1", + "by_severity": {"critical": 0, "major": 0, "minor": 0, "nit": 0}, + } + rfile = tmp_dir / "result.json" + rfile.write_text(json.dumps(result)) + + state_mod.cmd_read_result(_make_args(rfile)) + + st = _read_state(tmp_dir) + merged = st["rounds"][-1][AGENT] + assert merged["intent"] == "APPROVE" + assert merged["posted_as"] == "APPROVE" + assert merged["comments"] == 0 + assert merged["review_url"] == "https://example/pr/1#1" + assert merged["by_severity"]["critical"] == 0 + + +def test_alias_schema_intent_and_comment_count(patched_tmp_dir, state_mod): + """gemini が `intent` / `comment_count` で書き出すパターンも受理する。""" + tmp_dir = patched_tmp_dir + _seed_state(tmp_dir) + result = { + "intent": "APPROVE", + "comment_count": 3, + "review_url": "https://example/pr/2#2", + "by_severity": {"critical": 0, "major": 0, "minor": 2, "nit": 1}, + } + rfile = tmp_dir / "result.json" + rfile.write_text(json.dumps(result)) + + state_mod.cmd_read_result(_make_args(rfile)) + + st = _read_state(tmp_dir) + merged = st["rounds"][-1][AGENT] + assert merged["intent"] == "APPROVE" + # posted_as は別名 result.json には存在しないので intent と同値にフォールバック + assert merged["posted_as"] == "APPROVE" + assert merged["comments"] == 3 + + +def test_missing_event_and_intent_dies(patched_tmp_dir, state_mod): + """event / intent いずれも無ければ exit 1 で fail し state は不変であること。""" + tmp_dir = patched_tmp_dir + seeded = _seed_state(tmp_dir) + result = {"comments_count": 0} + rfile = tmp_dir / "result.json" + rfile.write_text(json.dumps(result)) + + with pytest.raises(SystemExit) as e: + state_mod.cmd_read_result(_make_args(rfile)) + assert e.value.code == 1 + + # state は更新されていない (rounds[-1] に agent エントリが追加されていない) + st = _read_state(tmp_dir) + assert AGENT not in st["rounds"][-1] + assert st["rounds"][-1]["round"] == seeded["rounds"][-1]["round"] + + +def test_empty_result_file_dies(patched_tmp_dir, state_mod): + """空 result.json → die (result 未生成扱い)。""" + tmp_dir = patched_tmp_dir + _seed_state(tmp_dir) + rfile = tmp_dir / "result.json" + rfile.write_text("") + + with pytest.raises(SystemExit) as e: + state_mod.cmd_read_result(_make_args(rfile)) + assert e.value.code == 1