diff --git a/issues/issue-729-619-584-design.md b/issues/issue-729-619-584-design.md index 1963eb64..137efdb8 100644 --- a/issues/issue-729-619-584-design.md +++ b/issues/issue-729-619-584-design.md @@ -534,16 +534,16 @@ sequenceDiagram ## 未確認のまま残ること -6 件。実装で決めるものが 4 件、確かめないまま進めるものが 2 件（どちらでも設計が塞ぐ）である。 +6 件。実装で決めたものが 4 件（1・3・5・6。決めた結果を「決める時点」の列に残す）、確かめないまま進めるものが 2 件（2・4。どちらでも設計が塞ぐ）である。 | # | 項目 | 内容 | 決める時点 | | --- | --- | --- | --- | -| 1 | claude の 429 の出る先 | `"api_error_status":429` が err.log と stdout.log のどちらに出るか。実物のログが手元に無い。両方を見るためどちらでも拾える | 実装で偽の claude を両方の形で試す。実物は次に上限に当たったときの記録で確かめる | +| 1 | claude の 429 の出る先 | `"api_error_status":429` が err.log と stdout.log のどちらに出るか。実物のログが手元に無い。両方を見るためどちらでも拾える | 実装で偽の claude を err.log と stdout.log の両方の形で試し、どちらでも `EARLY_ERROR` / `usage_limit` になることをテストで固定した（`test_monitor_usage_limit.py` の `test_usage_limit_stops_the_agent_as_early_error_with_reason_usage_limit`）。実物の出る先は次に上限に当たったときの記録で確かめる | | 2 | kiro の利用上限のときの終わり方 | プロセスが直ちに終わるのか、待ち続けるのか。#619 の実物では `err.log` に 1 行出て `NO_RESULT` になった（直ちに終わったと読める） | どちらでも監視は巡回で文言を拾い `usage_limit` にする。実装で確かめない | -| 3 | macOS の bash 3.2 の `set -m` | Linux の bash 5.3 だけで確かめた | 実装で確かめる。成り立たなければ pid だけの停止に落ちる（構造は同じ） | +| 3 | macOS の bash 3.2 の `set -m` | Linux の bash 5.3 だけで確かめた | 一次資料で確かめた。GNU の配布物 bash-3.2 の `doc/bash.1`（2006-09-28）の `set` の `-m` の項に「Background processes run in a separate process group」とあり、`CHANGES` の bash-2.01 の節に `set -m` の修正の記載がある（2.01 の時点で存在する）。手元の bash 5.3.9 では非対話・tty 無しで pid = pgid になり、標準エラーへジョブ制御の通知は出ない。macOS の実機では未確認のまま。成り立たなければ `_leads_own_group` が偽になり pid だけの停止に落ちる（構造は同じ） | | 4 | agy が子プロセスで結果を書くか | #584 の事例が止めた後の書き出しだったかは確かめられていない | 確かめないまま進める（グループで止めればどちらでも塞がる） | -| 5 | 利用上限と他の致命が同じ err.log に並ぶ順序 | 上限の後に別の致命が続く形を想定して利用上限を先に見る。逆の順で並ぶ実物は未確認 | 実装で順序を固定し、逆の実物が出たら見直す | -| 6 | `read_launch_outcome` に渡す stem の組み立て | cross-review は `-review-pr`、cross-refactoring は `stem_for` の値。監視の `--stem-template` と食い違うと監視の結果ファイルを引けない | 実装で、`launch-*.sh` と `monitor.py` の呼び出しの stem を突き合わせるテストを置く | +| 5 | 利用上限と他の致命が同じ err.log に並ぶ順序 | 上限の後に別の致命が続く形を想定して利用上限を先に見る。逆の順で並ぶ実物は未確認 | 照合の順序を利用上限 → 致命 → 警告の見た目の致命に固定し、err.log の並びがどちらの順でも理由が `usage_limit` になることをテストで固定した（`test_usage_limit_wins_over_other_fatal_lines_in_either_order`）。逆の順で並ぶ実物が出ても結果は変わらない | +| 6 | `read_launch_outcome` に渡す stem の組み立て | cross-review は `-review-pr`、cross-refactoring は `stem_for` の値。監視の `--stem-template` と食い違うと監視の結果ファイルを引けない | cross-review の 3 か所（`launch-reviewer.sh` の `STEM=` の行・監視の `DEFAULT_STEM_TEMPLATE`・取り込みが渡す stem）が同じ形であることを 1 つのテストで固定した（`test_read_result_reason.py` の `test_the_three_stems_have_the_same_shape`）。cross-refactoring の `stem_for` との突き合わせは G4 が `read_result` を置き換えるときに同じ形のテストを置く | ## 申し送り（並行する設計との境界） diff --git a/issues/issue-729-619-584-implementation-plan.md b/issues/issue-729-619-584-implementation-plan.md new file mode 100644 index 00000000..c1fbb7e3 --- /dev/null +++ b/issues/issue-729-619-584-implementation-plan.md @@ -0,0 +1,173 @@ +# cross-review: 利用上限で止まった担当が「結果ファイル無し」と報告されて空振りの起動し直しで待たされ、止めた担当が後から結果を書く → 上限を理由に報告して同じラウンドで起動し直さず、止めた後は書かせない（実装計画 / #729 #619 #584） + +## 関連リンク + +- 親 issue #729、子 issue #619 #584 +- 要求と受け入れ条件: [issue-729-619-584-requirements.md](issue-729-619-584-requirements.md)（AC1〜AC24） +- 設計: [issue-729-619-584-design.md](issue-729-619-584-design.md)（決定 12 件。識別子は冒頭の「用語の対応表」で引く） +- 設計 Pull Request: #781（`develop` へマージ済み） + +## モード + +`standard`。本番の振る舞い（結果なしの理由と起動し直しの判断）を変え、共通層と cross-review の複数モジュールにまたがる。 + +## 目的と非目的 + +達成したい状態: + +- 担当の CLI が利用上限で落ちたとき、進行側と利用者に理由「利用上限」が届き、同じラウンドで同じ担当を起動し直さない +- 結果なしの判断と起動し直しの可否を、結末の共通層の 1 つの関数が持つ。cross-review はその値を読むだけになる +- 監視が止めた担当の子プロセスが、止めた後に結果ファイルを書かない + +やらないこと: + +- cross-refactoring の結果の読み取りと 3 つの取り込みの実装（G4 #728 が、この計画が作る結末を読む関数を使って行う） +- 投稿の後に打ち切られた担当の記録と重ねての投稿（G5 #730） +- 利用上限の担当を外して残りの担当で回す判断（#478） +- 監視の終了コードと標準出力の変更、cross-review の骨組み（`SKILL.md`）の変更 +- テストが共通層のモジュールを読み込む流儀を揃えること（実装中に見つけ、#789 として起票した） + +## 前提 + +- 前提 1: 設計文書の決定 12 件を変えない。実装で決めるのは「未確認のまま残ること」の 4 件（claude の 429 の出る先・bash 3.2 の `set -m`・文言の並ぶ順序・stem の突き合わせ）で、決めた結果は設計文書の同じ節へ書く +- 前提 2: 監視の結果ファイルと監視の記録（P1）、上限の表と工程（P2）は `develop` にある。この変更はその上に載せる +- 前提 3: 手元の bash は 5.3.9 である。macOS の bash 3.2 は実機が無いため、`set -m` の互換は bash の変更履歴の確認までとし、成り立たない場合の逃げ道（グループの先頭でなければ pid だけを止める）を実装で保証する + +## 受け入れ条件 + +要求文書の AC1〜AC24 をそのまま使う。検証手段は設計文書の「テスト設計」の表にある。この計画では、各タスクが満たす番号を「タスク分解」で示す。 + +## ドメイン用語 + +設計文書の「用語の対応表」を使う。この計画で新たに使う語は次の 2 つである。 + +| 用語 | 意味 | +| --- | --- | +| 共通層のタスク | 結末の語彙・監視・起動の手順を変えるタスク（Task 1〜4）。cross-review を触らない | +| 読む側のタスク | cross-review の状態の操作を変えるタスク（Task 5〜6）。共通層のタスクの後に行う | + +## 不変条件 + +- 監視の終了コード 0〜6 と標準出力の 13 個のキーは変わらない +- 結果の取り込みの終了コード（使える結果 0 / 読めない結果 3 / それ以外 1）と、判定の 0 / 2 / 7 / 8 の意味は変わらない +- 結果ファイルが JSON オブジェクトとして読めるなら、監視の結末が何であっても使える結果として扱う +- 理由の語彙と起動し直しの可否の表を持つのは結末の共通層だけである + +## 互換性 + +| 対象 | 変更 | 互換性の扱い | +| --- | --- | --- | +| 監視の標準出力・終了コード | 変えない | 変えない | +| 監視の結果ファイル・監視の記録の `reason` | `usage_limit` / `cli_timeout` の 2 値が増える | 追加のみ。読む側は値を集計するだけで一覧を持たない | +| 状態ファイルの `rounds[-1].<担当>` | `monitor_detail` の鍵が増える（結果なしのときだけ）。`no_result_reason` の値が 10 種類になる | 追加のみ。既存の状態ファイルはそのまま読める | +| 結末の共通層の関数 | `read_launch_outcome` / `relaunch_same_agent` / `LaunchOutcome` / `NO_RELAUNCH_REASONS` を新設 | 追加のみ。既存の `read_outcome` / `reason_for` / `REASONS` の呼び出し側は変わらない | +| 起動の手順の引数・pid ファイル | 変えない | 変えない | + +## 修正対象 + +```text +plugins/ndf/scripts/lib/monitor_outcome.py +plugins/ndf/scripts/lib/monitor.py +plugins/ndf/scripts/lib/launch-cli.sh +plugins/ndf/scripts/lib/README.md +plugins/ndf/scripts/tests/test_monitor_outcome_unit.py +plugins/ndf/skills/cross-review/scripts/state.py +plugins/ndf/skills/cross-review/docs/01-state-and-review.md +plugins/ndf/skills/cross-review/docs/03-review-output.md +plugins/ndf/skills/cross-review/docs/04-contracts.md +plugins/ndf/skills/cross-review/tests/test_monitor_usage_limit.py 新設 +plugins/ndf/skills/cross-review/tests/test_launch_cli_process_group.py 新設 +plugins/ndf/skills/cross-review/tests/test_read_result_reason.py 新設 +plugins/ndf/skills/cross-review/tests/test_judge_no_result_reason.py 新設 +issues/issue-729-619-584-design.md 「未確認のまま残ること」の更新 +plugins/ndf/dev.kiro/ / plugins/ndf/dev.agy/ 配布物の同期（生成物） +``` + +## タスク分解 + +共通層のタスク（1〜4）→ 読む側のタスク（5〜6）→ 文書と退行の確認（7〜8）の順に進める。各タスクは失敗するテスト → 通す最小実装 → 整理の順で行う。 + +### Task 1: 結末を 1 つの値として読む関数を共通層に置く + +- **対象ファイル:** `plugins/ndf/scripts/lib/monitor_outcome.py`、`plugins/ndf/scripts/tests/test_monitor_outcome_unit.py` +- **変更内容:** 理由の語彙を 9 語にする（`REASONS` に `usage_limit` / `cli_timeout` / `unparsable`）。起動し直せない理由の集合 `NO_RELAUNCH_REASONS = frozenset({"usage_limit"})` と `relaunch_same_agent(reason)` を置く。`LaunchOutcome`（`payload` / `reason` / `detail` / `monitor` / `relaunch_same_agent`）と `read_launch_outcome(tmp_dir, stem, result_path=None)` を新設する。理由の決め方は設計文書の「結末を読む関数」の表のとおり。例外・`SystemExit`・標準出力/標準エラーへの出力を出さない。モジュールの冒頭の説明に読み取りの責務を足す +- **満たす受け入れ条件:** AC1、AC8〜AC11 +- **進め方:** 監視の結果ファイル（各理由・壊れた JSON・無し）× 結果ファイル（オブジェクト・配列・壊れた JSON・空・無し）の組み合わせを一時ディレクトリに置く単体テストを先に書く。`capsys` で出力が空であることも見る + +### Task 2: 監視が利用上限の文言を検知し、理由「利用上限」を結末に添える + +- **対象ファイル:** `plugins/ndf/scripts/lib/monitor.py`、`plugins/ndf/skills/cross-review/tests/test_monitor_usage_limit.py`（新設） +- **変更内容:** `MonitorOutcome` に `reason: Optional[str] = None` を足し、`create(status, detail, reason=None)` にする。`USAGE_LIMIT_FATAL`（kiro の文言・claude の 429 の JSON・既存の `quota exceeded` / `rate limit exceeded`・`HTTP/x 429`）と `CLAUDE_STDOUT_USAGE_LIMIT` を新設し、`EARLY_ERROR_FATAL` から 429 と quota / rate limit の一致を外す（401 / 403 は残す）。致命の照合の前に利用上限の照合を置く（err.log は全担当、stdout.log は claude だけ JSON 向けの照合）。`_early_error_outcome` は利用上限に一致したとき `reason="usage_limit"` を添える。`_record_outcome` は `outcome.reason or reason_for(status)` で理由を書く。結末を作る既存の呼び出し 8 か所は変えない +- **満たす受け入れ条件:** AC2〜AC4、AC6、AC7 +- **進め方:** 設計文書の「実測」の 10 行を入力にした照合の単体テストと、文言を 1 行書いた err.log / stdout.log と終わったプロセスの pid ファイルで `monitor_agent` を呼ぶテストを先に書く。既存の `test_monitor_outcome_file.py` と `test_monitor_early_error.py` は変えずに通す。**照合の順序（利用上限 → 致命 → 警告の見た目の致命）をテストで固定する**（設計文書の未確認 5） + +### Task 3: CLI 自身の上限で結果を書かずに終わった担当を、理由「CLI の上限」にする + +- **対象ファイル:** `plugins/ndf/scripts/lib/monitor.py`、`plugins/ndf/skills/cross-review/tests/test_monitor_usage_limit.py` +- **変更内容:** `CLI_TIMEOUT_AFTER_EXIT`（`print timeout after \S+ with turn in progress`）を新設する。`_process_exit_outcome` で、終了して結果ファイルが無いときだけ err.log を照合し、一致すれば `reason="cli_timeout"` を添える。結果ファイルがあれば従来どおり `OK` +- **満たす受け入れ条件:** AC5 +- **進め方:** 結果ファイルあり・なしの 2 通りのテストを先に書く + +### Task 4: CLI を独立したプロセスグループで起動し、止めるときはグループへ送る + +- **対象ファイル:** `plugins/ndf/scripts/lib/launch-cli.sh`、`plugins/ndf/scripts/lib/monitor.py`、`plugins/ndf/skills/cross-review/tests/test_launch_cli_process_group.py`（新設） +- **変更内容:** 起動の手順の `launch_runtime` を呼ぶ前に `set -m` を有効にし、背景起動した CLI の pid がプロセスグループの番号になるようにする（起動の直後に `set +m` で戻す）。`_kill_pid` は `os.getpgid(pid) == pid` かつ `!= os.getpgrp()` のとき `os.killpg` で SIGTERM → 3 秒 → SIGKILL を送り、それ以外は従来どおり pid だけへ送る。生存の確認はグループの先頭の pid で見る +- **満たす受け入れ条件:** AC19〜AC21 +- **進め方:** 3 秒後に子プロセスが結果ファイルを書く偽の CLI（`bash -c`）を PATH に置いて起動の手順から起動し、`os.getpgid(pid) == pid` を確かめるテスト、監視の上限 2 秒で止めた後 4 秒待っても結果ファイルが無いテスト、先頭でない pid で `os.killpg` が呼ばれないことを `mock` で見るテストを先に書く。**bash 3.2 の互換は、`set -m` が bash 2 系から存在する組み込みであることを `man bash` / 変更履歴で確かめ、設計文書の未確認 3 へ書く**（実機での確認は未確認のまま残す） + +### Task 5: cross-review の結果の取り込みが共通の値を読み、理由と監視の詳細を残す + +- **対象ファイル:** `plugins/ndf/skills/cross-review/scripts/state.py`、`plugins/ndf/skills/cross-review/tests/test_read_result_reason.py`（新設） +- **変更内容:** 共通層の `monitor_outcome` を読み込む。`_read_review_result_file` は結果ファイルを自前で開かず `read_launch_outcome(tmp_dir, f"{agent}-review-pr{pr}", rfile)` を呼び、`payload` が無ければ `reason` を `no_result_reason` として記録して従来の終了コード（`unparsable` は 3、それ以外は 1）で止める。`_record_no_result` に `monitor_detail`（監視の結果ファイルがあるときだけ鍵を書く。空文字は書かない）を足す。`no_verdict` / `not_posted` の上書きは従来どおり +- **満たす受け入れ条件:** AC13、AC12（`plugins/ndf/skills/` に `usage_limit` の条件分岐を書かない） +- **進め方:** 結果ファイル無し + 監視の結果ファイル（各理由）/ 無しで `read-result` を呼び、状態ファイルの `no_result_reason` と `monitor_detail` と終了コードを見るテストを先に書く。既存の `test_state_read_result.py` / `test_state_no_result.py` は変えずに通す。**stem の突き合わせ**（設計文書の未確認 6）は、`launch-reviewer.sh` の `STEM` と監視の `DEFAULT_STEM_TEMPLATE` と取り込みの stem が同じ形であることをテストで固定する + +### Task 6: 判定が理由を出し、起動し直せない理由があれば止める。報告の表に理由を出す + +- **対象ファイル:** `plugins/ndf/skills/cross-review/scripts/state.py`、`plugins/ndf/skills/cross-review/tests/test_judge_no_result_reason.py`（新設） +- **変更内容:** `_handle_no_result_round` は結果なしの担当の理由を集めて標準出力に `NO_RESULT_REASONS='<担当>=<理由> ...'` を出す。理由に `relaunch_same_agent` が偽のものがあれば、起動し直さずに `final=error` として終了コード 1 で止め、標準エラーに担当・理由・`monitor_detail` を出す。すべて起動し直してよい理由なら従来どおり 7 / 2 度目は 1。`_print_round_summary` は結果なしの担当を `<担当>=NO_RESULT(<理由>)` の形で出す +- **満たす受け入れ条件:** AC14〜AC17、AC24 +- **進め方:** 状態ファイルを作って `judge` と `report` を呼ぶ。`usage_limit` を含む / 含まない / 2 度目の 3 通りを先に書く。`SKILL.md` の骨組みの行（`bg-wait.sh run` から `case $JUDGE_RC` まで）は `git diff` で差が無いことを確かめる + +### Task 7: 文書を更新する + +- **対象ファイル:** `plugins/ndf/skills/cross-review/docs/01-state-and-review.md`、`docs/03-review-output.md`、`docs/04-contracts.md`、`plugins/ndf/scripts/lib/README.md`、`issues/issue-729-619-584-design.md` +- **変更内容:** 理由の表を 10 語にする（`missing` / `unparsable` / `no_verdict` / `not_posted` / `timeout` / `stalled` / `early_error` / `usage_limit` / `cli_timeout` / `pidfile_bad`）。「monitor.py が誤って kill する場合の手順」に、上限に当たった場合の見分け方（`reason` と `monitor-outcomes.jsonl` の読み方）を足す。契約の文書に `monitor_detail` の鍵と `reason` の 2 値を足す。共通層の一覧の `monitor_outcome.py` の行に読み取りの責務を足す。設計文書の「未確認のまま残ること」の 1・3・5・6 を、実装で決めた結果へ更新する +- **満たす受け入れ条件:** AC18 +- **進め方:** テスト駆動を適用しない（文書）。`grep` で 10 語と「見分け方」の節を確かめる。`python3 scripts/check-doc-line-limit.py` を通す + +### Task 8: 退行の確認と配布物の同期 + +- **対象ファイル:** `plugins/ndf/dev.kiro/` `plugins/ndf/dev.agy/` の生成物 +- **変更内容:** `bash scripts/build-runtime-plugins.sh` で生成物を揃える。全テスト・定義の検査を通す。#619 と #584 の再現を手動で確かめる（要求文書の「検証手段」） +- **満たす受け入れ条件:** AC22、AC23 +- **進め方:** テスト駆動を適用しない（検証）。コマンドの終了コードを記録し、Pull Request 本文へ載せる + +## 影響範囲 + +- 監視を使う 2 つの Skill（cross-review / cross-refactoring）。cross-refactoring は監視の結果ファイルの `reason` に 2 値が増えるだけで、終了コードの分岐は変わらない +- 実行の要約（`run_metrics.py`）は `reason` の値を集計するだけで、語彙の一覧を持たないため変更なし +- 担当の CLI のプロセスが独立したプロセスグループで動く。起動の手順を呼ぶ側（`launch-reviewer.sh`、cross-refactoring の `launch-*.sh`）の引数は変わらない + +## リスクと対処 + +| リスク | 対処 | +| --- | --- | +| `state.py` は 4470 行の 1 ファイルで、G1（#727）も同じファイルの別の節を触る | タスクごとにテストを通す。触る関数を `_read_review_result_file` / `_record_no_result` / `_handle_no_result_round` / `_print_round_summary` の 4 つに限る。競合は後からマージする側が解く | +| `docs/01-state-and-review.md` は 497 行で、行数の上限（500）に近い。理由の表に 7 行足すと超える | 同じ文書の既存の記述を詰め、501 行以上にしない。詰められなければ理由の表を契約の文書（`04-contracts.md`）へ移し、元の場所からリンクする | +| 利用上限の文言の照合を致命の照合の前に置くため、既存の 429 / quota の一致の `reason` が変わる | 既存テストは `_scan_early_fatal` の返り値だけを見ており、利用上限の照合を先に置いても `_scan_early_fatal` 単体の挙動は変えない。理由の変化は新しいテストで固定する | +| `set -m` を有効にすると、bash がジョブの終了を標準エラーへ出す・起動元がジョブ制御の影響を受ける | `launch_runtime` の直前で有効にし、直後に `set +m` で戻す。既存の `test_lib_launch_cli_runtimes.py` / `test_launch_cli_guards.py` で標準エラーの形が変わらないことを見る | +| 監視の環境変数（`MONITOR_*`）を export したシェルでは既存テストが落ちる（#678、G6） | export していないシェルでテストを実行する | + +## 切り戻し手順 + +- すべてコードと文書の変更で、データ移行は無い。Pull Request の revert で戻せる +- 状態ファイルの `monitor_detail` は追加の鍵で、戻した後の読み手は無視する。監視の結果ファイルの `usage_limit` / `cli_timeout` は戻した後の `reason_for` の一覧に無いが、読む側は値を集計するだけで一覧を照合しない + +## 完了の定義 + +- [ ] AC1〜AC24 をすべて満たし、条件ごとに検証手段と結果が対応している（Pull Request 本文の表） +- [ ] `uv run --with pytest pytest scripts/tests plugins/ndf -q` が通る（export していないシェル） +- [ ] `bash scripts/build-runtime-plugins.sh --check`、`python3 scripts/check-skill-frontmatter.py`、`claude plugin validate .` が終了コード 0 +- [ ] `git grep -n 'usage_limit' -- plugins/ndf/skills` の一致行が文書・テスト・テンプレートの文言だけである（AC12） +- [ ] 設計文書の「未確認のまま残ること」の 1・3・5・6 が、実装で決めた結果へ更新されている diff --git a/issues/refactoring-plan-rf791.md b/issues/refactoring-plan-rf791.md new file mode 100644 index 00000000..a5a60c35 --- /dev/null +++ b/issues/refactoring-plan-rf791.md @@ -0,0 +1,370 @@ +# 改修計画 — devbasex/ai-plugins #791 + +`/ndf:cross-refactoring` が提案し、適用した改善項目の記録である。 +理由と手順は提案の時点でしか残らないため、公開の直前に書き出している。 + +- 対象範囲: plugins/ndf/scripts/lib, plugins/ndf/skills/cross-review/scripts, plugins/ndf/scripts/tests, plugins/ndf/skills/cross-review/tests +- 着手前のテスト: uv run --with pytest pytest scripts/tests plugins/ndf -q + +## ラウンド 1（実装 codex / レビュー agy / kiro） + +### R1-001 — `plugins/ndf/scripts/lib/auth.py#check_auth` + +| 兆候・経路 | 手法・階層 | 重要度 | 提案元 | 状態 | コミット | +| --- | --- | --- | --- | --- | ---: | +| branch | unit | — | kiro | 採用 | 1 | + +**なぜ**: 終了コード 0 でも UNAUTHENTICATED_MARKERS を含む出力を未認証と判定する分岐が固定されていない。これはこのモジュールの存在理由そのものだが未固定である + +**手順**: 1. subprocess.run を差し替え、returncode 0 かつ stdout に 'not logged in' を含む結果を返す +2. check_auth(['codex'], ...) を呼ぶ +3. results['codex']['ok'] が False であることを確認する +4. failed があるため die が一度呼ばれることを確認する + +### R1-002 — `plugins/ndf/scripts/lib/auth.py#check_auth` + +| 兆候・経路 | 手法・階層 | 重要度 | 提案元 | 状態 | コミット | +| --- | --- | --- | --- | --- | ---: | +| error | unit | — | kiro | 未着手 | 0 | + +**なぜ**: 確認コマンドが見つからないとき（FileNotFoundError）に ok=False とし detail を 'コマンドが見つかりません' にする分岐が固定されていない + +**手順**: 1. subprocess.run を差し替え、FileNotFoundError を送出させる +2. check_auth(['codex'], ...) を呼ぶ +3. results['codex']['ok'] が False であることを確認する +4. results['codex']['detail'] に見つからない旨が入り、die が一度呼ばれることを確認する + +### R1-003 — `plugins/ndf/scripts/lib/monitor_outcome.py#append_journal` + +| 兆候・経路 | 手法・階層 | 重要度 | 提案元 | 状態 | コミット | +| --- | --- | --- | --- | --- | ---: | +| normal | unit | — | agy | 採用 | 1 | + +**なぜ**: monitor_outcome.py には read_journal の単体テストはあるが、ペアとなる append_journal の単体テストが存在しない。親ディレクトリの自動生成や非ASCII文字（UTF-8）の保持を含め、複数回の追記によって順序通りに記録・復元できる正常系が単体レベルで未固定である。 + +**手順**: 1. 一時ディレクトリと複数の outcome 辞書を準備する +2. append_journal を複数回呼び出して追記する +3. read_journal で読み出し、追記された順序と内容が期待通り一致することを検証する + +### R1-004 — `plugins/ndf/scripts/lib/monitor_outcome.py#default_result_path` + +| 兆候・経路 | 手法・階層 | 重要度 | 提案元 | 状態 | コミット | +| --- | --- | --- | --- | --- | ---: | +| normal | unit | — | agy | 未着手 | 0 | + +**なぜ**: read_launch_outcome の既定フォールバック先として使われる公開関数 default_result_path について、パス命名規則（/-result.json）を直接検証する単体テストが存在しない。 + +**手順**: 1. tmp_dir と stem を準備する +2. default_result_path(tmp_dir, stem) を呼び出す +3. 返された Path が期待される /-result.json と等しいことを検証する + +### R1-005 — `plugins/ndf/scripts/lib/monitor_outcome.py#relaunch_same_agent` + +| 兆候・経路 | 手法・階層 | 重要度 | 提案元 | 状態 | コミット | +| --- | --- | --- | --- | --- | ---: | +| boundary | unit | — | agy | 未着手 | 0 | + +**なぜ**: relaunch_same_agent は再試行可否の判定に使われる。既存テストは定義済み語彙（REASONSの9語）のみをテストしており、reason=None や空文字列 ""、未知の理由文字列が渡された場合に True を返す境界値の挙動が固定されていない。 + +**手順**: 1. None、空文字列 ""、未知の理由文字列を引数として準備する +2. relaunch_same_agent を呼び出す +3. いずれも戻り値が True であることを検証する + +## ラウンド 2（実装 agy / レビュー codex / kiro） + +### R2-001 — `plugins/ndf/scripts/lib/refresh.py#compare` + +| 兆候・経路 | 手法・階層 | 重要度 | 提案元 | 状態 | コミット | +| --- | --- | --- | --- | --- | ---: | +| branch | unit | — | kiro | 採用 | 1 | + +**なぜ**: compare は取得失敗・前回記録なし・一致・不一致の 4 分岐を返す純関数だが、対象範囲のどこでも固定されていない。instructions-check の既存テストは refresh.fetch をスタブへ差し替えるため、compare 本体は一度も通らない + +**手順**: 1. ok=False の FetchResult を作り compare(result, 'sha256:aa') を呼ぶ +2. ok=True で fingerprint を持つ FetchResult を作り、previous を None・空文字・同じ指紋・異なる指紋の 4 通りで呼ぶ +3. 実行して得た戻り値（取得失敗・前回記録なし・一致・不一致を表す文字列）を期待値として固定する + +### R2-002 — `plugins/ndf/scripts/lib/refresh.py#fetch` + +| 兆候・経路 | 手法・階層 | 重要度 | 提案元 | 状態 | コミット | +| --- | --- | --- | --- | --- | ---: | +| error | unit | — | kiro | 取り消し | 1 | + +**なぜ**: fetch は取得の失敗を例外にせず FetchResult(ok=False, error=...) へ畳む分岐を持ち、error の文言は _reason が例外の種類ごとに書き分ける。この経路は未固定で、instructions-check のテストは fetch 自体を差し替えるため通らない。opener は差し替え用に設計された引数である + +**手順**: 1. opener に OSError を送出する呼び出し可能を渡し fetch(url, timeout, opener) を呼ぶ +2. urllib.error.HTTPError と urllib.error.URLError を送出する opener でも同様に呼ぶ +3. 3 通りとも ok が False で、実行して得た error の文言（種類ごとに異なる）を期待値として固定する + +### R2-003 — `plugins/ndf/scripts/lib/refresh.py#refresh` + +| 兆候・経路 | 手法・階層 | 重要度 | 提案元 | 状態 | コミット | +| --- | --- | --- | --- | --- | ---: | +| normal | integration | — | kiro | 採用 | 1 | + +**なぜ**: refresh は fetch・row・compare をつないで全件の 1 行と失敗件数を返す公開入口だが、この配線を通す固定が対象範囲に無い。取れなかった URL を黙って落とさず件数へ数える振る舞いが未固定である + +**手順**: 1. 成功と失敗を混ぜて返す opener を差し替えで用意する +2. name/checked_at/claim を持つ複数の source を渡し refresh(sources, timeout, opener) を呼ぶ +3. 返る行数が source 数と一致し、失敗件数が失敗した source 数と一致することを固定する +4. 各行に source の name と取得の成否が含まれることを、実行して得た値で固定する + +## ラウンド 3（実装 kiro / レビュー codex / agy） + +### R3-001 — `plugins/ndf/scripts/lib/monitor_outcome.py#OUTCOME_KEYS` + +| 兆候・経路 | 手法・階層 | 重要度 | 提案元 | 状態 | コミット | +| --- | --- | --- | --- | --- | ---: | +| scattered_config | centralize_configuration | major | kiro | 採用 | 1 | + +**なぜ**: 監視の結果ファイルのキーの一覧が 3 か所に散っている。monitor_outcome.OUTCOME_KEYS（14 個・runtime では未使用）と、実際に辞書を組み立てる monitor.py の _record_outcome（`phase` を含む 15 個）と、test_monitor_outcome_file.py の独自コピー（15 個）である。正本のはずの OUTCOME_KEYS が `phase` を欠いており既に食い違っている。キーを足すたびにどこかが古くなる。 + +**手順**: 1. OUTCOME_KEYS へ `phase` を加え、契約文書の並びと揃える +2. monitor.py の _record_outcome が組み立てた辞書のキー集合を OUTCOME_KEYS と突き合わせる assert を置き、食い違いをその場で落とす（値の生成は現状のまま） +3. read_journal / read_outcome の既存テストと test_monitor_outcome_file.py を実行し、キー集合の期待が変わっていないことを確かめる + +### R3-002 — `plugins/ndf/skills/cross-review/scripts/state.py#_handle_no_result_round` + +| 兆候・経路 | 手法・階層 | 重要度 | 提案元 | 状態 | コミット | +| --- | --- | --- | --- | --- | ---: | +| long_method | extract_method | major | codex | 採用 | 1 | + +**なぜ**: 1 関数に理由の集約と出力、再起動不能時の終了処理、再起動済み時の終了処理、再起動対象の記録とシェル向け出力という別々の段階が同居し、状態保存と終了条件が複数箇所に散っている。 + +**手順**: 1. 担当別理由の収集と NO_RESULT_REASONS 出力を小さな関数へ抽出する +2. final・ended_at・保存・die を行う共通の異常終了処理を抽出する +3. 再起動対象の記録と互換出力を行う処理を抽出する +4. usage_limit、再起動済み、再起動要求の既存テストで終了コード・state・出力順を固定する + +### R3-003 — `plugins/ndf/skills/cross-review/scripts/state.py#_print_init_result` + +| 兆候・経路 | 手法・階層 | 重要度 | 提案元 | 状態 | コミット | +| --- | --- | --- | --- | --- | ---: | +| long_parameter_list | introduce_parameter_object | major | codex | 採用 | 1 | + +**なぜ**: 初期化結果という同じ概念を表す 11 引数を位置で受け取り、特に連続する 3 個の bool と末尾の件数・再開フラグは呼び出し側で順序を取り違えても検出しにくい。新規初期化と再開の 2 経路が同じ組を渡している。 + +**手順**: 1. 出力対象を表す _InitResult の値オブジェクトを定義する +2. _print_init_result の引数を _InitResult 1 個へ置き換える +3. 新規初期化経路と再開経路で名前付きフィールドから _InitResult を構築する +4. 両経路の既存テストで標準出力が不変であることを確認する + +### R3-004 — `plugins/ndf/scripts/lib/monitor.py#monitor_agent` + +| 兆候・経路 | 手法・階層 | 重要度 | 提案元 | 状態 | コミット | +| --- | --- | --- | --- | --- | ---: | +| duplication | consolidate_duplication | minor | codex | 採用 | 1 | + +**なぜ**: 監視ループの 5 つの終了分岐が、同じ _finish_monitor(status, outcome, (config.log_prefix, agent)) 呼び出しを繰り返している。終了時に渡すログ文脈を変更すると各分岐を同時に直す必要がある。 + +**手順**: 1. status とログ文脈を閉じ込めて outcome を受け取る局所的な finish 処理を定義する +2. PID 不正・完了・timeout・early error・process exit・stall の各終了分岐を同じ入口へ寄せる +3. monitor_agent を通る既存テストで status、ログ、終了結果が不変であることを確認する + +### R3-005 — `plugins/ndf/skills/cross-review/scripts/state.py#_verify_findings` + +| 兆候・経路 | 手法・階層 | 重要度 | 提案元 | 状態 | コミット | +| --- | --- | --- | --- | --- | ---: | +| long_method | extract_method | minor | kiro | 採用 | 1 | + +**なぜ**: 1 つの関数が 2 つの独立した段を通しで行う。前段は各指摘の suggested_check を（重複を除いて）実行し verification を記録する反復、後段は束ねた組の代表へ最良の結果を選び直す反復である。段ごとに名前が付き、共有するのは targets と by_id だけである。 + +**手順**: 1. 前段を _run_finding_checks(targets, allowed, work, codes, run) として抽出し、実行済みコマンドの対応表を関数内へ閉じる +2. 後段を _propagate_best_verification(targets, by_id) として抽出する +3. _verify_findings は codes / run / targets / by_id を用意し、2 つを順に呼ぶだけにする +4. test_verify_findings.py を実行して verification の記録が変わらないことを確かめる + +## ラウンド 4（実装 claude / レビュー codex / kiro） + +### R4-001 — `plugins/ndf/skills/cross-review/scripts/state.py#_resume_from_state` + +| 兆候・経路 | 手法・階層 | 重要度 | 提案元 | 状態 | コミット | +| --- | --- | --- | --- | --- | ---: | +| long_method | extract_method | major | codex | 採用 | 1 | + +**なぜ**: 既存stateの探索、旧形式の補完、追加レビュー観点の再計算、引き継ぎ記録、保存、待ち行列flush、worktree同期、機械可読出力までが1関数に直列で置かれ、副作用の順序を長い本体とコメントから追う必要がある。各段階には独立した終了条件と入出力があり、名前を付けて分離できる。 + +**手順**: 1. cmd_initを通る既存の再開テストで、stateなし、final済み、旧形式補完、追加観点更新、引き継ぎ、flush後の同期と出力を現状固定する +2. stateファイルの探索と再開可否判定を、stateとpathを返す関数へ抽出する +3. 旧形式の補完、manual指示の反映、review_instructions再計算、carried_over記録を、変更有無も返す関数へ抽出する +4. 保存後のauto_flush、tmp_dir解決、登録済みworktree同期を副作用順序が見える関数へ抽出する +5. _resume_from_stateを各段階の呼び出しと_print_init_resultだけにし、再開関連テストと全体テストで出力と保存順序が不変であることを確認する + +### R4-002 — `plugins/ndf/skills/cross-review/scripts/state.py#_init_new_state` + +| 兆候・経路 | 手法・階層 | 重要度 | 提案元 | 状態 | コミット | +| --- | --- | --- | --- | --- | ---: | +| long_method | extract_method | major | codex | 取り消し | 1 | + +**なぜ**: 新規初期化の1関数に、PR所有権の解決、レビュー観点の構築、worktreeと既存コメントの準備、認証確認、state構築、永続化と出力が同居し、さらに5個のローカル関数が本体を約200行へ広げている。各段階は既に名前と入出力を持つため、モジュールレベルへ抽出すれば段階単位で読めて個別にテストできる。 + +**手順**: 1. 既存の入口テストで、新規worktree、既存worktree、変更ファイル取得fallback、認証失敗、初期state出力の経路を現状固定する +2. _resolve_pr_and_ownership と _prepare_review_instructions をモジュールレベル関数へ抽出し、既存のcontext型を入出力に使う +3. _prepare_worktree_and_comments をモジュールレベル関数へ抽出し、worktree作成より後に_tmp_dirを呼ぶ順序とコメント取得失敗時の停止を保つ +4. _prepare_initial_assignment、_build_initial_review_state、_finalize_initial_state をモジュールレベルへ抽出し、_init_new_stateを段階を順に呼ぶオーケストレーションだけにする +5. 初期化関連テストと全体テストを実行し、標準出力、stateの内容、副作用の順序が不変であることを確認する + +### R4-003 — `plugins/ndf/skills/cross-review/scripts/state.py#cmd_check_oscillation` + +| 兆候・経路 | 手法・階層 | 重要度 | 提案元 | 状態 | コミット | +| --- | --- | --- | --- | --- | ---: | +| conditional_chain | replace_with_lookup_table | minor | kiro | 取り消し | 1 | + +**なぜ**: 現ラウンドの各指摘について _finding_match_kind の戻り値 ("exact"/"near"/"body") を if/elif で数えている。種別ごとの集計は種別を増やすたびに分岐を足すことになる。あわせて、collect_keys クロージャは _finding_keys(st, pr, round_no) をそのまま呼ぶだけの指標なしの間接参照で、読み手が本体を追う負荷を増やしている。test_state_check_oscillation.py と test_state_oscillation_matching.py が cmd_check_oscillation / _finding_match_kind を通す。 + +**手順**: 1. exact/near/same_body の 3 変数と if/elif/elif の加算を、collections.Counter に対する `Counter(_finding_match_kind(k, prev) for k in curr)` へ置き換える +2. overlap_count は None 以外の合計として counts の値の総和から出す +3. info の表示は counts.get("exact", 0) 等から読む +4. collect_keys クロージャを消し、呼び出し 2 箇所を _finding_keys(st, pr, prev_round_no) / (curr_round_no) の直接呼び出しに戻す +5. テストを実行して振る舞い不変を確認する + +### R4-004 — `plugins/ndf/skills/cross-review/scripts/state.py#_normalize_fix_result` + +| 兆候・経路 | 手法・階層 | 重要度 | 提案元 | 状態 | コミット | +| --- | --- | --- | --- | --- | ---: | +| long_method | extract_method | minor | kiro | 採用 | 1 | + +**なぜ**: 1 関数に (a) 別名 fallback（fix_commit/commit_sha、fixed_count/fixed）、(b) deferred の list/dict/int による件数の場合分けと dict 要素への正規化、(c) rejected の同じ正規化、(d) 記録用辞書の組み立て、が同居する。deferred と rejected はどちらも _normalize_dict_items + 件数決定という同型の処理で、片方だけ直すと食い違いうる。test_state_merge_fix.py と test_state_ci_classification.py が cmd_merge_fix 経由で通す。 + +**手順**: 1. 「_normalize_dict_items した項目」と「保存する件数」を組で返す小関数 _normalize_deferred_like(raw) を抽出する（list/dict は展開件数、劣化表現の int/str は _count の値、という現在の規則をそのまま移す） +2. deferred と rejected の両方をこの関数で得る（現状 rejected の件数は常に _count なので、raw の型で分岐する現在の deferred 規則へ揃える形にはせず、抽出関数は deferred の規則を表し、rejected は従来どおり _count を使うなら別に保つ。振る舞いを変えないため、まず deferred 経路だけを抽出する） +3. 別名 fallback（fix_commit/fixed_count）を _resolve_fix_aliases として抽出する +4. 末尾の辞書組み立てを、抽出した値を差し込む形へ整える +5. テストを実行して出力の辞書が不変であることを確認する + +## ラウンド 5（実装 codex / レビュー agy / kiro） + +### R5-001 — `plugins/ndf/scripts/lib/post_queue.py#Queue.flush` + +| 兆候・経路 | 手法・階層 | 重要度 | 提案元 | 状態 | コミット | +| --- | --- | --- | --- | --- | ---: | +| long_method | extract_method | major | codex | 取り消し | 1 | + +**なぜ**: 1 件の処理の中に、壊れた JSON の停止判定、既投稿の照合と削除、送信成功時の応答保存と削除、送信失敗時の再試行情報保存と rate limit 判定が直列に並び、flush 自体が順序制御と各項目の状態遷移の両方を担っている。 + +**手順**: 1. 読み取り済み項目について既投稿・送信成功・送信失敗を処理する部分を Queue の補助メソッドへ抽出する +2. 補助メソッドの戻り値で継続または停止と rate_limited を表し、flush は連番走査と集計だけを担うようにする +3. 壊れた項目で停止する既存経路は flush 側に残し、項目順序と停止位置を変えない +4. test_post_queue.py と cross-review/tests/test_queue_idempotency.py で skipped・sent・failed・remaining とファイル削除順を確認する + +### R5-002 — `plugins/ndf/skills/cross-review/scripts/state.py#_sync_worktree` + +| 兆候・経路 | 手法・階層 | 重要度 | 提案元 | 状態 | コミット | +| --- | --- | --- | --- | --- | ---: | +| long_method | extract_method | major | codex | 取り消し | 1 | + +**なぜ**: PR head の取得方法の決定、strict 時の同期済み判定と失敗処理、worktree の reset・未追跡ファイル掃除、同期結果の表示という独立した段階が 1 関数に同居している。HeadRef と旧来の文字列 head の分岐も取得段階に閉じず、後続の制御へ have_base・target・label の組で持ち越されている。 + +**手順**: 1. HeadRef と文字列 head から have_base・target・label を解決する取得段階を補助関数へ抽出する +2. strict 時の同期済み早期終了と基準取得失敗の判定を補助関数へ抽出する +3. _sync_worktree は取得、判定、reset、clean、結果表示の順序だけを示す構成にする +4. test_state_sync_worktree.py と test_state_offline_fetch.py で既存の strict／fallback／失敗時終了コードを確認する + +### R5-003 — `plugins/ndf/skills/cross-review/scripts/state.py#_load_payload` + +| 兆候・経路 | 手法・階層 | 重要度 | 提案元 | 状態 | コミット | +| --- | --- | --- | --- | --- | ---: | +| duplication | consolidate_duplication | minor | kiro | 取り消し | 0 | + +**なぜ**: payload が dict でない場合と comments が list でない場合の 2 経路が、`info(f"⚠ {agent}: ...形式不正で、判定は中断します")` を出して `return None` する同じ形で並ぶ。返す条件（dict でない / list でない）と型名の埋め込みが繰り返され、警告文の末尾の定型句も重複する。片方の文言だけ直すと 2 経路のメッセージが食い違う。test_review_findings.py が不正 payload での 0 件記録を固定している。 + +**手順**: 1. `_reject_payload(agent: str, path: pathlib.Path, detail: str) -> None` を追加し、`info(f"⚠ {agent}: {detail}（{path}...）。指摘の記録は 0 件です。review launcher の出力形式不正で、判定は中断します")` を出して None を返す +2. dict でない場合と comments が list でない場合の 2 経路を、type 名を含む detail 文字列を渡す呼び出しへ置き換える +3. 部分不正（items != raw）は継続する経路のため対象にせず、そのまま残す +4. `uv run --with pytest pytest plugins/ndf/skills/cross-review/tests/test_review_findings.py -q` で 0 件記録と警告が不変なことを確認する + +### R5-004 — `plugins/ndf/scripts/lib/metrics.py#format_report` + +| 兆候・経路 | 手法・階層 | 重要度 | 提案元 | 状態 | コミット | +| --- | --- | --- | --- | --- | ---: | +| duplication | consolidate_duplication | minor | kiro | 取り消し | 1 | + +**なぜ**: unmeasured と assumed の 2 節が同じ形（見出し + 空行を lines へ足し、dict.fromkeys で重複を除いた項目を `- {w}` で並べる）で並んでいる。片方だけ書式を変えると 2 節の見た目が食い違う。同じ業務ルール（分離・代用の一覧の出し方）に由来し、変わるときは一緒に変わる。既存テスト test_models_and_metrics.py が format_report の出力を固定している。 + +**手順**: 1. `_emit_bullet_section(lines: list[str], title: str, items: list[str]) -> None` を追加し、items が空でなければ `['', f'## {title}', '']` と `[f'- {w}' for w in dict.fromkeys(items)]` を lines へ足す +2. unmeasured の if ブロックを `_emit_bullet_section(lines, "集計から分離したラウンド", metrics["unmeasured"])` へ置き換える +3. assumed の if ブロックを `_emit_bullet_section(lines, "指定値で代用したラウンド", metrics.get("assumed") or [])` へ置き換える +4. 比較の限界（COMPARISON_CAVEATS）節は常に出るため対象外のまま残す +5. `uv run --with pytest pytest scripts/tests plugins/ndf -q` で出力が不変なことを確認する + +### R5-005 — `plugins/ndf/skills/cross-review/scripts/state.py#build_parser` + +| 兆候・経路 | 手法・階層 | 重要度 | 提案元 | 状態 | コミット | +| --- | --- | --- | --- | --- | ---: | +| long_method | extract_method | minor | codex | 採用 | 1 | + +**なぜ**: init の多数のオプション定義と、ラウンド進行・結果取込・検証・報告に属する 11 個の副コマンド登録が 1 関数に連続しており、個別コマンドの引数変更でも 125 行の構築処理全体を読む必要がある。副コマンドごとに独立した名前を付けられる段階になっている。 + +**手順**: 1. init のパーサ設定を専用の補助関数へ抽出する +2. 各副コマンドの parser 作成・引数追加・set_defaults を用途別の小さな登録関数へ抽出する +3. build_parser はトップレベル parser と subparsers を作り、登録関数を順に呼んで返すだけにする +4. test_state_subcommand_help.py、test_state_review_pool.py、test_findings_pipeline_wiring.py で選択肢・help・func の対応が不変であることを確認する + +## ラウンド 6（実装 agy / レビュー codex / kiro） + +### R6-001 — `plugins/ndf/scripts/lib/auth.py#check_auth` + +| 兆候・経路 | 手法・階層 | 重要度 | 提案元 | 状態 | コミット | +| --- | --- | --- | --- | --- | ---: | +| long_method | extract_method | major | codex | 採用 | 1 | + +**なぜ**: スキップ判定、CLI ごとの subprocess 実行、例外の認証結果への変換、結果の集約、全体の失敗通知が 1 関数に同居しており、個別 CLI のプローブ規則と複数 CLI の制御を別々に読めない。既存の test_auth_probe.py が未知 runtime、成功、未認証マーカー、コマンド不在を公開入口から固定している。 + +**手順**: 1. 1 runtime の probe 実行と FileNotFoundError・TimeoutExpired の認証結果への変換を、runtime と probe を受け取る補助関数へ抽出する +2. ok・detail・command の結果を check_auth が受け取り、既存どおり info 出力、failed 集約、die 判定を行う形へ置き換える +3. test_auth_probe.py を実行し、戻り値、通知文、die 呼び出しが不変であることを確認する + +### R6-002 — `plugins/ndf/skills/cross-review/scripts/state.py#_confirm_flushed` + +| 兆候・経路 | 手法・階層 | 重要度 | 提案元 | 状態 | コミット | +| --- | --- | --- | --- | --- | ---: | +| long_method | extract_method | major | codex | 採用 | 1 | + +**なぜ**: 待ち行列項目の適用可否判定、response からの URL 復元、対象ラウンド探索、GitHub 到達確認、結果なしまたは成功状態への更新、永続化が 1 関数に直列で同居している。投稿確認は収束可否に関わるため、対象特定と状態遷移を独立した名前で読める構造にする価値が高く、test_state_queue_judge.py が送信済み・冪等スキップ・未到達の経路を固定している。 + +**手順**: 1. item の kind・extra・response を解釈して agent、round、review URL を返す処理を補助関数へ抽出する +2. state の rounds から書き戻し対象を探す処理を補助関数へ抽出する +3. _confirm_flushed は早期 return、到達確認、既存と同じ target 更新、_save の順序だけを担うよう置き換える +4. test_state_queue_judge.py を実行し、queued、review_url、not_posted、保存回数を含む観測結果が不変であることを確認する + +### R6-003 — `plugins/ndf/skills/cross-review/scripts/state.py#_guard_previous_round` + +| 兆候・経路 | 手法・階層 | 重要度 | 提案元 | 状態 | コミット | +| --- | --- | --- | --- | --- | ---: | +| long_method | extract_method | major | codex | 未着手 | 0 | + +**なぜ**: 旧形式 state の verdict 復元、修正記録の必須判定、申告済み Resolve の GitHub 照会、未解決 ID の検出という独立した 2 つのガードが 1 関数に同居している。各ガードは異なる理由で変更され、test_state_round_guard.py が修正記録なし、照会不能、未解決残存、正常通過を固定している。 + +**手順**: 1. 保存済み verdict が無い場合の no_result・pass からの復元を補助関数へ抽出する +2. fix の resolved_thread_ids と対象 PR を受け、照会不能時の通知または未解決 ID を判定する補助関数へ抽出する +3. _guard_previous_round は修正記録ガードと Resolve 状態ガードを順に呼ぶ構成へ置き換える +4. test_state_round_guard.py を実行し、終了コード、GitHub 照会先、警告、正常通過が不変であることを確認する + +### R6-004 — `plugins/ndf/skills/cross-review/scripts/state.py#_is_generated_path` + +| 兆候・経路 | 手法・階層 | 重要度 | 提案元 | 状態 | コミット | +| --- | --- | --- | --- | --- | ---: | +| scattered_config | centralize_configuration | minor | kiro | 未着手 | 0 | + +**なぜ**: 兄弟の判定述語（_is_dependency_path は DEPENDENCY_FILENAMES、_is_infra_path は INFRA_FILENAMES、_is_generated_path 自身も GENERATED_MARKERS）はいずれもモジュール定数を引くのに、_is_generated_path だけ lockfile 名の集合（package-lock.json / go.sum / cargo.lock など 9 件）を関数本体へじか書きしている。うち package-lock.json 等は DEPENDENCY_FILENAMES にも重複して載っており、lockfile を足すとき 2 か所を直す必要があるうえ、どこに定義があるか読み手が探す。定義を 1 か所へ寄せて兄弟と同じ形にする。 + +**手順**: 1. モジュール定数群（DEPENDENCY_FILENAMES / GENERATED_MARKERS / INFRA_FILENAMES の並び）へ GENERATED_LOCKFILES を新設し、現在インラインにある 9 件の集合をそのまま移す +2. _is_generated_path の本体を `name in GENERATED_LOCKFILES` へ置き換え、インラインの集合リテラルを消す +3. test_gap を埋めるため、先に go.sum / cargo.lock（DEPENDENCY_FILENAMES に無く、インライン集合にだけある名前）で generated カテゴリが立つ現状固定テストを test_state_auto_review_templates.py に追加し、移動の前後で同じ結果になることを確認する + +## 見送った項目 + +| ラウンド | 対象 | 兆候・経路 | 理由 | +| --- | --- | --- | --- | +| 1 | `plugins/ndf/scripts/lib/statefile.py#save` | error | 1 ラウンドの採用上限 5 件を超えた | +| 1 | `plugins/ndf/scripts/lib/statefile.py#save` | normal | 1 ラウンドの採用上限 5 件を超えた | +| 1 | `plugins/ndf/skills/cross-review/scripts/state.py#cmd_read_result` | boundary | 1 ラウンドの採用上限 5 件を超えた | +| 1 | `plugins/ndf/skills/cross-review/scripts/state.py#cmd_read_result` | error | 1 ラウンドの採用上限 5 件を超えた | +| 2 | `plugins/ndf/scripts/lib/refresh.py#fetch` | error | テストの期待する振る舞いが変わっています（plugins/ndf/scripts/tests/test_refresh.py）。構造改善では期待出力を変えません。振る舞いの変更は別の変更に分けてください | +| 3 | `plugins/ndf/scripts/lib/monitor.py#_record_outcome` | long_method | 1 ラウンドの採用上限 5 件を超えた | +| 4 | `plugins/ndf/skills/cross-review/scripts/state.py#_init_new_state` | long_method | テストの期待する振る舞いが変わっています（plugins/ndf/skills/cross-review/tests/test_init_body_not_duplicated.py）。構造改善では期待出力を変えません。振る舞いの変更は別の変更に分けてください | +| 4 | `plugins/ndf/skills/cross-review/scripts/state.py#cmd_check_oscillation` | conditional_chain | コミット 95387271f1b307d6c0a88f1cde8f6401fe3a6111 にトレーラーが欠けています: Item-Id, Round, Impl-Runtime, Impl-Model | +| 5 | `plugins/ndf/scripts/lib/post_queue.py#Queue.flush` | long_method | どの改善項目にも割り当てられていないコミットが 1 件（311bf7f）。検証を回避した変更や、状態と実差分の食い違いを Pull Request に残さないため、この適用ラウンドを取り消します | +| 5 | `plugins/ndf/skills/cross-review/scripts/state.py#_sync_worktree` | long_method | どの改善項目にも割り当てられていないコミットが 1 件（311bf7f）。検証を回避した変更や、状態と実差分の食い違いを Pull Request に残さないため、この適用ラウンドを取り消します | +| 5 | `plugins/ndf/scripts/lib/metrics.py#format_report` | duplication | どの改善項目にも割り当てられていないコミットが 1 件（311bf7f）。検証を回避した変更や、状態と実差分の食い違いを Pull Request に残さないため、この適用ラウンドを取り消します | +| 5 | `plugins/ndf/skills/cross-review/scripts/state.py#_load_payload` | duplication | どの改善項目にも割り当てられていないコミットが 1 件（1e8216f）。検証を回避した変更や、状態と実差分の食い違いを Pull Request に残さないため、この適用ラウンドを取り消します | diff --git a/plugins/ndf/scripts/lib/README.md b/plugins/ndf/scripts/lib/README.md index 3226c021..21debcf2 100644 --- a/plugins/ndf/scripts/lib/README.md +++ b/plugins/ndf/scripts/lib/README.md @@ -20,7 +20,7 @@ | [lock-common.sh](lock-common.sh) | 排他の取得と解放（#293） | 上の 2 つと `development-workflow` | | [monitor.py](monitor.py) | 別プロセスの多軸監視。対象と命名規則を引数で受ける | 収束ループの 2 つ | | [limits.py](limits.py) | 監視の上限（工程ごと）・無進捗の許容（担当ごと）・CLI の上限（監視の上限 + 120 秒）の表。既定値はここだけが持つ（#598 / #537） | 同上 | -| [monitor_outcome.py](monitor_outcome.py) | 監視の結果の理由の語彙と、結果ファイル・監視の記録の読み書き（#662） | 同上 | +| [monitor_outcome.py](monitor_outcome.py) | 監視の結果の理由の語彙（9 語）と起動し直しの可否、結果ファイル・監視の記録の読み書き（#662）、起動 1 回の結末を 1 つの値として読む `read_launch_outcome`（#729） | 同上 | | [launch-cli.sh](launch-cli.sh) | claude / codex / agy / kiro をランタイム名で分岐して背景起動する | 同上 | | [_tmpdir.sh](_tmpdir.sh) | 一時ディレクトリの解決。環境変数名とディレクトリ名を引数で受ける | 同上 | | [statefile.py](statefile.py) | 状態ファイルの読み書きと KEY=VALUE 出力、保存の後の差し込み口 | 同上 | diff --git a/plugins/ndf/scripts/lib/auth.py b/plugins/ndf/scripts/lib/auth.py index 1fc7d63b..d68c0d93 100644 --- a/plugins/ndf/scripts/lib/auth.py +++ b/plugins/ndf/scripts/lib/auth.py @@ -33,6 +33,26 @@ SKIP_ENV = "NDF_SKIP_AUTH_CHECK" +def _probe_auth(runtime: str, probe: tuple[str, ...]) -> tuple[bool, str, str]: + """1 つの CLI を確認し、集約に必要な結果へ変換する。""" + command = " ".join(probe) + try: + result = subprocess.run( + list(probe), capture_output=True, text=True, + timeout=AUTH_PROBE_TIMEOUT, + ) + merged = f"{result.stdout}\n{result.stderr}".lower() + ok = result.returncode == 0 and not any( + marker in merged for marker in UNAUTHENTICATED_MARKERS + ) + detail = (result.stderr.strip() or result.stdout.strip())[:200] + except FileNotFoundError: + ok, detail = False, "コマンドが見つかりません" + except subprocess.TimeoutExpired: + ok, detail = False, f"{AUTH_PROBE_TIMEOUT} 秒で応答しませんでした" + return ok, detail, command + + def check_auth( runtimes: Iterable[str], *, @@ -59,20 +79,9 @@ def check_auth( probe = AUTH_PROBES.get(runtime) if probe is None: continue - try: - r = subprocess.run(list(probe), capture_output=True, text=True, - timeout=AUTH_PROBE_TIMEOUT) - merged = f"{r.stdout}\n{r.stderr}".lower() - ok = r.returncode == 0 and not any( - m in merged for m in UNAUTHENTICATED_MARKERS - ) - detail = (r.stderr.strip() or r.stdout.strip())[:200] - except FileNotFoundError: - ok, detail = False, "コマンドが見つかりません" - except subprocess.TimeoutExpired: - ok, detail = False, f"{AUTH_PROBE_TIMEOUT} 秒で応答しませんでした" - results[runtime] = {"command": " ".join(probe), "ok": ok, "detail": detail} - info(f"{'✅' if ok else '❌'} {runtime}: {' '.join(probe)}") + ok, detail, command = _probe_auth(runtime, probe) + results[runtime] = {"command": command, "ok": ok, "detail": detail} + info(f"{'✅' if ok else '❌'} {runtime}: {command}") if not ok: failed.append(f"{runtime}（{detail}）") diff --git a/plugins/ndf/scripts/lib/launch-cli.sh b/plugins/ndf/scripts/lib/launch-cli.sh index 2c1416c2..d874ff2b 100755 --- a/plugins/ndf/scripts/lib/launch-cli.sh +++ b/plugins/ndf/scripts/lib/launch-cli.sh @@ -141,7 +141,15 @@ CLAUDE_ALLOWED_TOOLS=${NDF_CLAUDE_ALLOWED_TOOLS:-Bash,Read,Write,Edit,Glob,Grep} cd "$WORKDIR" +# **CLI を独立したプロセスグループで起動する（#584 / #729 の決定 10）。** ジョブ制御を +# 有効にして背景起動すると、CLI の pid がそのままプロセスグループの番号になる。監視は +# 止めるときにグループへシグナルを送るので、CLI の子プロセスが止めた後に結果ファイルを +# 書かない。`setsid` は macOS に標準で無いため使わない。起動の直後に戻し、この script の +# 残りはジョブ制御の影響を受けない。`set -m` は bash 3.2（macOS）にもある（bash.1 の +# 「Monitor mode. Job control is enabled.」）。 +set -m launch_runtime +set +m echo "$PID" > "$PID_FILE" disown 2>/dev/null || true diff --git a/plugins/ndf/scripts/lib/monitor.py b/plugins/ndf/scripts/lib/monitor.py index 5e7df1d3..2427ba38 100755 --- a/plugins/ndf/scripts/lib/monitor.py +++ b/plugins/ndf/scripts/lib/monitor.py @@ -19,11 +19,16 @@ - 可能なら `/proc//cmdline` で codex/agy であることを再確認 (PID 再利用対策) 2. **sentinel** (codex のみ): err.log に `^tokens used$` 出現 3. **early-error pattern**: err.log に既知の致命的キーワードが出たら即中断 - - **FATAL** (auth/quota/sandbox 等の明確な致命): 検知時に kill + - **USAGE LIMIT** (利用上限。kiro の `Monthly request limit reached` / claude の + `"api_error_status":429` / HTTP 429 / quota・rate limit): 検知時に kill し、 + 状態は EARLY_ERROR のまま理由 `usage_limit` を結末に添える（#729 / #619）。 + claude だけは stdout.log の JSON も見る + - **FATAL** (auth/sandbox 等の明確な致命): 検知時に kill - **WARN** (生の `Error:` / `Traceback` 等の曖昧パターン): 警告ログのみ、kill せず通常判定を継続 - `--no-early-error` / `MONITOR_NO_EARLY_ERROR=1` で検知自体を無効化可 4. **result.json**: プロセス終了後に `/.cross_review/-review-pr-result.json` が - 生成されていなければ失敗扱い + 生成されていなければ失敗扱い。err.log に CLI 自身の上限の文言（agy の + `print timeout after <時間> with turn in progress`）があれば理由 `cli_timeout`（#729） 5. **hard timeout**: 既定は `--phase` の工程で上限の表（`limits.py`）から引く （省略時は `review`）。`--timeout` → `MONITOR_TIMEOUT_` → `MONITOR_TIMEOUT` の順で上書き可 6. **stall timeout**: err.log + stdout.log の合計サイズが一定時間変化しなければ @@ -35,7 +40,9 @@ 8. **result.json + age fallback**: sentinel を持たない agent (agy) 向け。 result.json の mtime が 30 秒以上前なら完了とみなし kill → OK 9. **失敗時 kill**: TIMEOUT / STALLED / EARLY_ERROR (FATAL のみ) / PIDFILE_BAD で - 返るとき、対象プロセスを SIGTERM (3 秒後に SIGKILL) で停止する + 返るとき、対象プロセスを SIGTERM (3 秒後に SIGKILL) で停止する。対象がプロセス + グループの先頭（`launch-cli.sh` は `set -m` で起動する）なら、グループへ送って + 子プロセスも止める（#584）。監視自身のグループへは送らない Usage: monitor.py target ∈ {codex, agy, both} @@ -108,23 +115,43 @@ def _lib_dir() -> pathlib.Path: "1", "true", "yes", "on", } -# err.log の行頭に近い形で出る **明確な致命** パターン (kill 対象)。 -# auth / quota / sandbox / HTTP 401-403-429 など、 -# プロセスが続行しても result を生成できないと判明しているケースだけを入れる。 +# **利用上限** の文言 (kill 対象。理由は `usage_limit`)。起動し直しても解けないため、 +# 他の致命と区別して結末に理由を添える（#729 の決定 4）。err.log は全担当で見る。 +# 照合は致命の表より **先** に行う。上限で落ちた後に別の致命が続く形が普通で、上限のほうが原因。 +USAGE_LIMIT_FATAL = [ + # kiro の実物（#619） + re.compile(r"Monthly request limit reached"), + # claude の `--output-format json` の結果行（#647）。`:` の前後の空白は問わない + re.compile(r'"api_error_status"\s*:\s*429'), + # quota / rate limit （`m.start()` をキーワード位置に合わせるため `^.*` を付けない。 + # `_match_is_quoted()` が backtick / 「」 引用を判定するために match 開始位置を使うため） + re.compile(r"\b(?:quota exceeded|rate limit exceeded)\b", re.IGNORECASE), + # HTTP 429 の状態行 + re.compile(r"^HTTP/\d\S* 429 ", re.MULTILINE), +] + +# err.log の行頭に近い形で出る **明確な致命** パターン (kill 対象。理由は `early_error`)。 +# auth / sandbox / HTTP 401-403 など、プロセスが続行しても result を生成できないと +# 判明しているケースだけを入れる。利用上限は `USAGE_LIMIT_FATAL` の側。 EARLY_ERROR_FATAL = [ # HTTP エラーステータス行 (`HTTP/1.1 401 Unauthorized` 等) - re.compile(r"^HTTP/\d\S* (?:401|403|429) ", re.MULTILINE), + re.compile(r"^HTTP/\d\S* (?:401|403) ", re.MULTILINE), # 認証 / 権限系（行頭限定） re.compile(r"^(?:Authentication failed|Permission denied)", re.MULTILINE), - # 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)\b", re.IGNORECASE), # codex 固有: sandbox エラー re.compile(r"\bsandbox error\b", re.IGNORECASE), ] +# **CLI 自身の上限** で結果を書かずに終わったことを示す文言（理由は `cli_timeout`）。 +# **終了した後、結果ファイルが無いときだけ** 照合する。生きている間に見ると途中の警告を +# 致命と読み、結果ファイルがあれば上限に当たっても書き終えているので使える（#729 の決定 5）。 +CLI_TIMEOUT_AFTER_EXIT = [ + # agy の `--print-timeout` の打ち切り（#598 / #537 の実物） + re.compile(r"print timeout after \S+ with turn in progress"), +] + # **警告の見た目で出る致命** パターン。`EARLY_ERROR_FATAL` と違い、行頭の # `warning:` を benign とする規則を適用しない（適用すると自分自身が消える）。 # 引用符・バックティック・markdown 引用による誤検知の除外だけを効かせる。 @@ -245,6 +272,12 @@ def _strip_ansi(text: str) -> str: re.compile(r'"is_error"\s*:\s*true'), ] +# claude の stdout.log に出る利用上限（理由は `usage_limit`）。err.log と stdout.log の +# どちらに出るか未確認のため両方を見る（#729 の決定 6）。JSON 向けの照合で除外を掛けない。 +CLAUDE_STDOUT_USAGE_LIMIT = [ + re.compile(r'"api_error_status"\s*:\s*429'), +] + # env を safe に int parse する。非数値時は warn を stderr に出して fallback 値を返す。 # 上限の表と同じ規則で読むため、表の側の実装を使う。 @@ -358,9 +391,12 @@ class MonitorOutcome: exit_code: int icon: str detail: str + # 状態からは決まらない理由（`usage_limit` / `cli_timeout`）。`None` なら結末を書くときに + # `monitor_outcome.reason_for(status)` へ落ちる（#729 の決定 8）。 + reason: Optional[str] = None @classmethod - def create(cls, status: str, detail: str) -> "MonitorOutcome": + def create(cls, status: str, detail: str, reason: Optional[str] = None) -> "MonitorOutcome": exit_code, icon = { "OK": (0, "✅"), "TIMEOUT": (2, "⏰"), @@ -369,7 +405,15 @@ def create(cls, status: str, detail: str) -> "MonitorOutcome": "STALLED": (5, "🛑"), "PIDFILE_BAD": (6, "❓"), }[status] - return cls(status, exit_code, icon, detail) + return cls(status, exit_code, icon, detail, reason) + + +@dataclass(frozen=True) +class EarlyFatal: + """早期の致命の一致。どのファイルで・何が・理由は何か（`None` なら `early_error`）。""" + source: str + message: str + reason: Optional[str] = None @dataclass @@ -441,6 +485,20 @@ def _is_zombie(pid: int) -> bool: return state is not None and "Z" in state +def _leads_own_group(pid: int) -> bool: + """pid がプロセスグループの先頭で、かつ監視自身のグループではないか。 + + `launch-cli.sh` は `set -m` で起動するため CLI の pid = pgid になる。そうでない pid + （古い起動の手順・別の経路）は先頭でないか、監視と同じグループに居る。**監視自身の + グループへ送ると、進行側のシェルまで止まる**（#584 の候補で退けた形）。 + """ + try: + pgid = os.getpgid(pid) + except OSError: + return False + return pgid == pid and pgid != os.getpgrp() + + def _kill_pid(pid: int, sigterm_grace: float = 3.0) -> None: """対象プロセスに SIGTERM、`sigterm_grace` 秒後も生きていたら SIGKILL。 @@ -448,13 +506,18 @@ def _kill_pid(pid: int, sigterm_grace: float = 3.0) -> None: だと後から `gh api` 投稿や result.json 書き込みを実行してメインフローと 競合する。失敗扱いで返るときは必ず停止させる。 ゾンビプロセスにはシグナルを送れないためスキップする。 + + 対象がプロセスグループの先頭なら **グループへ送る**（#584 / #729 の決定 10）。pid だけへ + 送ると、CLI の子プロセスが残って止めた後に結果ファイルを書く。生存の確認は先頭の pid で見る。 """ if pid <= 0: return if _is_zombie(pid): return + send = (lambda sig: os.killpg(pid, sig)) if _leads_own_group(pid) else ( + lambda sig: os.kill(pid, sig)) try: - os.kill(pid, signal.SIGTERM) + send(signal.SIGTERM) except OSError: return deadline = time.monotonic() + sigterm_grace @@ -463,7 +526,7 @@ def _kill_pid(pid: int, sigterm_grace: float = 3.0) -> None: return time.sleep(0.5) try: - os.kill(pid, signal.SIGKILL) + send(signal.SIGKILL) except OSError: pass @@ -538,7 +601,12 @@ def _scan_patterns( def _scan_early_fatal(path: pathlib.Path) -> Optional[str]: - hit = _scan_patterns(path, EARLY_ERROR_FATAL) + """err.log の致命の一致（kill 対象）。**利用上限も含む。** + + 理由（`usage_limit` か `early_error` か）の区別はここでは行わず、`_early_error` が + `USAGE_LIMIT_FATAL` を先に照合して決める。この関数は「止めるべき文言があるか」だけを返す。 + """ + hit = _scan_patterns(path, USAGE_LIMIT_FATAL) or _scan_patterns(path, EARLY_ERROR_FATAL) if hit: return hit return _scan_patterns( @@ -552,12 +620,8 @@ def _scan_early_warn(path: pathlib.Path) -> Optional[str]: return _scan_patterns(path, EARLY_ERROR_WARN) -def _scan_claude_stdout_fatal(path: pathlib.Path) -> Optional[str]: - """claude の JSON 出力から承認失敗・実行失敗を検出する。 - - `--output-format json` は完了時に 1 個の JSON を吐くため、 - `permission_denials` が非空、または `is_error` が真であれば失敗が確定する。 - err.log 側の行単位パターンでは拾えないので専用に見る。 +def _scan_claude_stdout(path: pathlib.Path, patterns: list[re.Pattern[str]]) -> Optional[str]: + """claude の JSON 出力を `patterns` で照合し、一致の前後 80 文字を返す。 `_scan_patterns()` は使わない。あちらは行単位の benign 判定と引用符パリティ判定を 行うが、JSON は 1 行に多数の引用符を含むため、パリティ判定が「引用の内側」を @@ -567,13 +631,28 @@ def _scan_claude_stdout_fatal(path: pathlib.Path) -> Optional[str]: if data is None: return None data = _strip_ansi(data) - for pat in CLAUDE_STDOUT_FATAL: + for pat in patterns: m = pat.search(data) if m: return data[max(0, m.start() - 80):m.end() + 80].strip() return None +def _scan_claude_stdout_fatal(path: pathlib.Path) -> Optional[str]: + """claude の JSON 出力から承認失敗・実行失敗を検出する。 + + `--output-format json` は完了時に 1 個の JSON を吐くため、 + `permission_denials` が非空、または `is_error` が真であれば失敗が確定する。 + err.log 側の行単位パターンでは拾えないので専用に見る。 + """ + return _scan_claude_stdout(path, CLAUDE_STDOUT_FATAL) + + +def _scan_claude_stdout_usage_limit(path: pathlib.Path) -> Optional[str]: + """claude の JSON 出力から利用上限（`"api_error_status":429`）を検出する。""" + return _scan_claude_stdout(path, CLAUDE_STDOUT_USAGE_LIMIT) + + def _scan_codex_sentinel(path: pathlib.Path) -> bool: tail = _read_tail(path, 64 * 1024) if tail is None: @@ -646,19 +725,43 @@ def _lingering_completion( ) +def _scan_usage_limit(paths: AgentPaths, agent: str) -> EarlyFatal | None: + """利用上限の文言。err.log は全担当、stdout.log は claude だけ JSON 向けの照合で見る。""" + hit = _scan_patterns(paths.err_log, USAGE_LIMIT_FATAL) + if hit: + return EarlyFatal("err.log", hit, "usage_limit") + if agent == "claude": + hit = _scan_claude_stdout_usage_limit(paths.stdout_log) + if hit: + return EarlyFatal("stdout.log", hit, "usage_limit") + return None + + +def _scan_fatal(paths: AgentPaths, agent: str) -> EarlyFatal | None: + """利用上限以外の致命（理由は `early_error`）。致命 → 警告の見た目の致命の順。""" + hit = _scan_early_fatal(paths.err_log) + if hit: + return EarlyFatal("err.log", hit) + if agent == "claude": + hit = _scan_claude_stdout_fatal(paths.stdout_log) + if hit: + return EarlyFatal("stdout.log", hit) + return None + + def _early_error( paths: AgentPaths, agent: str, disabled: bool, -) -> tuple[tuple[str, str] | None, str | None]: +) -> tuple[EarlyFatal | None, str | None]: + """早期の致命と警告。**照合の順序は利用上限 → 致命 → 警告の見た目の致命。** + + 同じ err.log に利用上限と他の致命が並んでいれば理由は `usage_limit` になる（#729 の + 決定 6）。`disabled`（`--no-early-error`）は利用上限の検知も一緒に無効にする。 + """ if disabled: return None, None - fatal_err = _scan_early_fatal(paths.err_log) - fatal_source = "err.log" - if not fatal_err and agent == "claude": - fatal_err = _scan_claude_stdout_fatal(paths.stdout_log) - fatal_source = "stdout.log" - fatal = (fatal_source, fatal_err) if fatal_err else None + fatal = _scan_usage_limit(paths, agent) or _scan_fatal(paths, agent) return fatal, _scan_early_warn(paths.err_log) @@ -729,9 +832,9 @@ def _early_error_outcome( return None, warning if alive: _kill_pid(status.pid) - source, message = fatal return MonitorOutcome.create( - "EARLY_ERROR", f"early error (fatal) in {source}: {message[:200]}" + "EARLY_ERROR", f"early error (fatal) in {fatal.source}: {fatal.message[:200]}", + reason=fatal.reason, ), warning @@ -747,6 +850,15 @@ def _process_exit_outcome( f"process exited; sentinel={status.sentinel_seen}; " f"result_exists={status.result_exists}", ) + # 結果なしの理由を err.log から引く。CLI の上限の文言があれば `cli_timeout`、無ければ + # 状態からの既定（`missing`）に落ちる。 + cli_timeout = _scan_patterns(paths.err_log, CLI_TIMEOUT_AFTER_EXIT) + if cli_timeout: + return MonitorOutcome.create( + "NO_RESULT", + f"process exited but result.json missing (CLI timeout): {cli_timeout[:200]}", + reason="cli_timeout", + ) return MonitorOutcome.create( "NO_RESULT", f"process exited but result.json missing: {paths.result}" ) @@ -776,11 +888,15 @@ def monitor_agent( hard timeout / stall / sentinel / result.json のみで判定する。 """ paths, status, started, pid = _initialize_monitor(agent, pr, config.stem_template) + + def finish(outcome: MonitorOutcome) -> AgentStatus: + """status とログ文脈を閉じ込めて結末を確定する。終了時のログ文脈を変えるときは + ここ 1 か所を直せばよい（各終了分岐が同じ呼び出しを繰り返さない）。""" + return _finish_monitor(status, outcome, (config.log_prefix, agent)) + if pid is None: - return _finish_monitor( - status, + return finish( MonitorOutcome.create("PIDFILE_BAD", f"pidfile not found: {paths.pidfile}"), - (config.log_prefix, agent), ) status.pid = pid @@ -816,9 +932,7 @@ def monitor_agent( if alive and (status.sentinel_seen or cmdline_validated): completion_detail = _lingering_completion(paths, status, pid, started_wall) if completion_detail is not None: - return _finish_monitor( - status, MonitorOutcome.create("OK", completion_detail), (config.log_prefix, agent) - ) + return finish(MonitorOutcome.create("OK", completion_detail)) # result.json が書かれた後もプロセスがハングするケース (実測: # MCP サーバー切断待ち等で exit しない)。sentinel 機構を持たない agent 向け @@ -831,12 +945,12 @@ def monitor_agent( pid, agent, alive, cmdline_validated ) if outcome: - return _finish_monitor(status, outcome, (config.log_prefix, agent)) + return finish(outcome) # 2. hard timeout outcome = _timeout_outcome(elapsed, config.timeout, alive, pid) if outcome: - return _finish_monitor(status, outcome, (config.log_prefix, agent)) + return finish(outcome) # 3. early error # 明確な致命 (FATAL) のみ kill する。曖昧パターン (生 Error: / Traceback) は @@ -844,7 +958,7 @@ def monitor_agent( # echo するケースで誤 kill されるのを防ぐ。 outcome, warn_err = _early_error_outcome(paths, status, alive, config.no_early_error) if outcome: - return _finish_monitor(status, outcome, (config.log_prefix, agent)) + return finish(outcome) if not warned_early_error and warn_err: print( f"{config.log_prefix}⚠️ {agent} early-error WARN " @@ -855,7 +969,7 @@ def monitor_agent( outcome = _process_exit_outcome(paths, status, alive, config.require_result) if outcome: - return _finish_monitor(status, outcome, (config.log_prefix, agent)) + return finish(outcome) # 4. stall detection (err.log / stdout.log / progress.log をモニタ。 # agy は stdout 側だけ進捗が出るケースがあり、progress.log には @@ -866,7 +980,7 @@ def monitor_agent( ) outcome = _stall_outcome(status, config.stall_timeout, pid, last_progress_size) if outcome: - return _finish_monitor(status, outcome, (config.log_prefix, agent)) + return finish(outcome) # poll 中の進捗ログ _emit_progress(config.log_prefix, agent, status) @@ -905,7 +1019,9 @@ def _record_outcome( stem = stem_template.format(agent=agent, id=pr) try: paths = AgentPaths.for_(agent, pr, stem_template) - st.reason = monitor_outcome.reason_for(st.status) + # 結末が理由を持てばそれを、無ければ状態からの既定を書く（#729 の決定 8） + st.reason = (st.outcome.reason if st.outcome and st.outcome.reason + else monitor_outcome.reason_for(st.status)) st.started_at = started_at st.ended_at = monitor_outcome.now_iso() try: @@ -931,6 +1047,10 @@ def _record_outcome( # `--phase` の値。省いたときは null（#598 / #537） "phase": phase, } + # 組み立てたキー集合を正本（`monitor_outcome.OUTCOME_KEYS`）と突き合わせる。 + # キーを片方だけへ足すと、ここで食い違いがその場で落ちる（#662）。 + assert set(outcome) == set(monitor_outcome.OUTCOME_KEYS), ( + set(outcome).symmetric_difference(monitor_outcome.OUTCOME_KEYS)) tmp_dir = paths.pidfile.parent monitor_outcome.write_outcome(tmp_dir, stem, outcome) monitor_outcome.append_journal(tmp_dir, outcome) diff --git a/plugins/ndf/scripts/lib/monitor_outcome.py b/plugins/ndf/scripts/lib/monitor_outcome.py index 30ad5422..f2018c70 100644 --- a/plugins/ndf/scripts/lib/monitor_outcome.py +++ b/plugins/ndf/scripts/lib/monitor_outcome.py @@ -13,14 +13,21 @@ キーと値の形は `issues/issue-662-598-537-619-584-583-design-contracts.md` の 「監視の結果ファイル」にある。 -""" -from __future__ import annotations +**起動 1 回の結末を 1 つの値として読むのもここである（#729）。** `read_launch_outcome` が +結果ファイルの有無・読めるかと監視の結果を突き合わせ、使える結果（`payload`）か理由 +（`reason`）と起動し直しの可否（`relaunch_same_agent`）を返す。結果なしの判断と可否の表を +cross-review / cross-refactoring がそれぞれ持つと、語彙を足すたびに片方が古くなる。 +""" +# `from __future__ import annotations` を置かない。注釈が文字列になると `dataclass` が +# `sys.modules[<モジュール名>]` を引くが、読む側の多くはこのファイルを `importlib` で +# `sys.modules` に登録せずに読み込むため落ちる。実行時に評価できる形（3.10 以上）で書く。 import datetime as _dt import json import os import pathlib import threading +from dataclasses import dataclass from typing import Any, Optional try: # Windows には無い。無ければスレッドの排他だけで書く。 @@ -28,9 +35,23 @@ except ImportError: # pragma: no cover - POSIX では通らない fcntl = None # type: ignore[assignment] -# 監視が書く理由。**状態（`status`）からの対応だけで決まる。** `usage_limit` と -# `cli_timeout` は P3 で足す（それまでは `early_error` と `missing` に落ちる）。 -REASONS = ("ok", "timeout", "stalled", "early_error", "missing", "pidfile_bad") +# 理由の語彙。先頭の 6 語は監視の状態（`status`）から決まる。`usage_limit` / `cli_timeout` は +# 監視が文言の照合で結末に添えたときだけ現れ、`unparsable` は読む側（`read_launch_outcome`） +# だけが書く（#729 の決定 7）。 +REASONS = ( + "ok", "timeout", "stalled", "early_error", "missing", "pidfile_bad", + "usage_limit", "cli_timeout", "unparsable", +) + +# 同じ担当を同じ条件で起動し直しても解けない理由。利用上限は起動のたびに待ちと相手の +# 枠を使うだけで直らない（#619）。それ以外は対象や負荷で変わりうるので 1 度は起動し直せる。 +# **理由を足すときはこの集合だけを見直す。** 偽のときに何をするかは Skill が決める。 +NO_RELAUNCH_REASONS = frozenset({"usage_limit"}) + +# 監視がこの理由を書いていれば、監視が止めたか、結果を書けない終わり方をしたと分かっている。 +# 結果ファイルの状態を見ずにその値を採る（`ok` / `missing` は結果ファイルの側で決め直す）。 +_MONITOR_DECIDED_REASONS = frozenset( + {"timeout", "stalled", "early_error", "usage_limit", "cli_timeout", "pidfile_bad"}) _STATUS_REASON = { "OK": "ok", @@ -42,10 +63,12 @@ } # 結果ファイルのキー。**並びも契約の文書の表と揃える**（読む人が突き合わせやすい）。 +# `phase` は P2（#598 / #537）で足した `--phase` の値。省いたときは null で、契約の文書の +# とおり末尾に置く。 OUTCOME_KEYS = ( "agent", "stem", "status", "exit_code", "reason", "detail", "launched_at", "started_at", "ended_at", "elapsed", "idle_seconds", - "progress_tail", "result_exists", "pid", + "progress_tail", "result_exists", "pid", "phase", ) JOURNAL_NAME = "monitor-outcomes.jsonl" @@ -75,10 +98,20 @@ def reason_for(status: str) -> str: raise ValueError(f"監視の状態として知らない値です: {status!r}") from None +def relaunch_same_agent(reason: Optional[str]) -> bool: + """同じ担当を同じ条件で起動し直せば解けるか。`NO_RELAUNCH_REASONS` に無ければ可。""" + return reason not in NO_RELAUNCH_REASONS + + def outcome_path(tmp_dir: os.PathLike[str] | str, stem: str) -> pathlib.Path: return pathlib.Path(tmp_dir) / f"{stem}-monitor.json" +def default_result_path(tmp_dir: os.PathLike[str] | str, stem: str) -> pathlib.Path: + """結果ファイルの既定の置き場所。`launch-cli.sh` の `-result.json` と同じ形。""" + return pathlib.Path(tmp_dir) / f"{stem}-result.json" + + def journal_path(tmp_dir: os.PathLike[str] | str) -> pathlib.Path: return pathlib.Path(tmp_dir) / JOURNAL_NAME @@ -105,6 +138,65 @@ def read_outcome(tmp_dir: os.PathLike[str] | str, stem: str) -> Optional[dict[st return data if isinstance(data, dict) else None +@dataclass(frozen=True) +class LaunchOutcome: + """起動 1 回の結末。`payload` があれば使える結果、無ければ `reason` が理由。""" + payload: Optional[dict[str, Any]] + reason: Optional[str] + detail: str + monitor: Optional[dict[str, Any]] + relaunch_same_agent: bool + + +def _read_result_file(path: pathlib.Path) -> tuple[Optional[dict[str, Any]], str]: + """結果ファイルを読む。使える辞書か、無ければ結果なしの理由（`missing` / `unparsable`）。""" + try: + text = path.read_text(encoding="utf-8") + except (OSError, ValueError): + return None, "missing" + if not text.strip(): + return None, "missing" + try: + data = json.loads(text) + except ValueError: + return None, "unparsable" + return (data, "") if isinstance(data, dict) else (None, "unparsable") + + +def read_launch_outcome(tmp_dir: os.PathLike[str] | str, stem: str, + result_path: Optional[os.PathLike[str] | str] = None) -> LaunchOutcome: + """起動 1 回の結末を 1 つの値として読む（#729 の決定 2）。 + + **結果ファイルが JSON オブジェクトとして読めれば使える結果が勝つ。** 監視が止めた後にも + 結果ファイルが残っていれば、止める前に書き終えていた結果である。無いときの理由は、監視が + 理由を知っていればその値、知らなければ結果ファイルの状態（無い・空 → `missing`、あるが + 読めない → `unparsable`）で決める。 + + **失敗しない。** 例外・`SystemExit`・標準出力/標準エラーへの出力を出さない。壊れた監視の + 結果ファイルは無いものとして扱う。読む側（両 Skill の取り込み）が終了コードを決める。 + """ + path = (pathlib.Path(result_path) if result_path is not None + else default_result_path(tmp_dir, stem)) + payload, result_reason = _read_result_file(path) + monitor = read_outcome(tmp_dir, stem) + monitor_detail = str(monitor.get("detail") or "") if monitor else "" + if payload is not None: + return LaunchOutcome(payload=payload, reason=None, detail=monitor_detail, + monitor=monitor, relaunch_same_agent=True) + monitor_reason = monitor.get("reason") if monitor else None + reason = monitor_reason if monitor_reason in _MONITOR_DECIDED_REASONS else result_reason + detail = monitor_detail or _unusable_detail(path, result_reason) + return LaunchOutcome(payload=None, reason=reason, detail=detail, monitor=monitor, + relaunch_same_agent=relaunch_same_agent(reason)) + + +def _unusable_detail(path: pathlib.Path, result_reason: str) -> str: + """監視の詳細が無いときに `detail` へ置く、結果を読めなかった理由の 1 文。""" + if result_reason == "missing": + return f"結果ファイルが無い、または空です: {path}" + return f"結果ファイルが JSON オブジェクトとして読めません: {path}" + + def append_journal(tmp_dir: os.PathLike[str] | str, outcome: dict[str, Any]) -> None: """記録へ 1 行を追記する。 diff --git a/plugins/ndf/scripts/tests/test_auth_probe.py b/plugins/ndf/scripts/tests/test_auth_probe.py index 30eaa4d0..d2470df3 100644 --- a/plugins/ndf/scripts/tests/test_auth_probe.py +++ b/plugins/ndf/scripts/tests/test_auth_probe.py @@ -4,6 +4,7 @@ import pathlib import subprocess import sys +from types import SimpleNamespace LIB = pathlib.Path(__file__).resolve().parents[1] / "lib" @@ -43,3 +44,22 @@ def time_out(*args, **kwargs): assert results["codex"]["ok"] is False assert str(auth.AUTH_PROBE_TIMEOUT) in results["codex"]["detail"] assert len(failures) == 1 + + +def test_unauthenticated_marker_fails_even_when_probe_exits_zero(monkeypatch): + auth = _load_auth() + messages: list[str] = [] + failures: list[str] = [] + + monkeypatch.setattr( + auth.subprocess, + "run", + lambda *args, **kwargs: SimpleNamespace( + returncode=0, stdout="Not logged in", stderr="" + ), + ) + + results = auth.check_auth(["codex"], info=messages.append, die=failures.append, env={}) + + assert results["codex"]["ok"] is False + assert len(failures) == 1 diff --git a/plugins/ndf/scripts/tests/test_monitor_outcome_unit.py b/plugins/ndf/scripts/tests/test_monitor_outcome_unit.py index 333173c4..91af3943 100644 --- a/plugins/ndf/scripts/tests/test_monitor_outcome_unit.py +++ b/plugins/ndf/scripts/tests/test_monitor_outcome_unit.py @@ -29,3 +29,157 @@ def test_read_journal_ignores_malformed_and_non_object_rows(tmp_path): ) assert _load_monitor_outcome().read_journal(tmp_path) == [valid] + + +def test_append_journal_preserves_order_and_utf8_content(tmp_path): + mod = _load_monitor_outcome() + outcomes = [ + {"agent": "codex", "status": "OK", "detail": "完了"}, + {"agent": "kiro", "status": "EARLY_ERROR", "detail": "認証が必要"}, + ] + + for outcome in outcomes: + mod.append_journal(tmp_path, outcome) + + assert mod.read_journal(tmp_path) == outcomes + + +# ---------- 理由の語彙と起動し直しの可否（#729 の AC1 / AC10） ---------- + +import pytest # noqa: E402 + +MONITOR_REASONS = ("timeout", "stalled", "early_error", "usage_limit", "cli_timeout", "pidfile_bad") + + +def test_reasons_are_the_nine_words(): + assert _load_monitor_outcome().REASONS == ( + "ok", "timeout", "stalled", "early_error", "missing", "pidfile_bad", + "usage_limit", "cli_timeout", "unparsable", + ) + + +@pytest.mark.parametrize(("status", "reason"), [ + ("OK", "ok"), ("TIMEOUT", "timeout"), ("STALLED", "stalled"), + ("EARLY_ERROR", "early_error"), ("NO_RESULT", "missing"), ("PIDFILE_BAD", "pidfile_bad"), +]) +def test_reason_for_keeps_the_six_status_mappings(status, reason): + assert _load_monitor_outcome().reason_for(status) == reason + + +def test_only_usage_limit_forbids_relaunching_the_same_agent(): + mod = _load_monitor_outcome() + assert mod.NO_RELAUNCH_REASONS == frozenset({"usage_limit"}) + for reason in mod.REASONS: + assert mod.relaunch_same_agent(reason) is (reason != "usage_limit"), reason + + +# ---------- 結末を 1 つの値として読む（#729 の AC8 / AC9 / AC11） ---------- + +STEM = "kiro-review-pr7" + + +def _place(tmp_path, *, monitor=None, result=None): + """監視の結果ファイルと結果ファイルを置く。`monitor` / `result` は書く中身（None は置かない）。""" + if monitor is not None: + (tmp_path / f"{STEM}-monitor.json").write_text(monitor, encoding="utf-8") + if result is not None: + (tmp_path / f"{STEM}-result.json").write_text(result, encoding="utf-8") + + +def _monitor_json(reason, detail="early error (fatal) in err.log: Monthly request limit reached"): + return json.dumps({"agent": "kiro", "stem": STEM, "status": "EARLY_ERROR", + "reason": reason, "detail": detail}) + + +@pytest.mark.parametrize("monitor", [None, "{broken", *[_monitor_json(r) for r in ("ok", "usage_limit")]]) +def test_readable_result_object_wins_whatever_the_monitor_says(tmp_path, monitor, capsys): + mod = _load_monitor_outcome() + _place(tmp_path, monitor=monitor, result='{"event": "APPROVE"}') + + got = mod.read_launch_outcome(tmp_path, STEM) + + assert got.payload == {"event": "APPROVE"} + assert got.reason is None + assert got.relaunch_same_agent is True + if monitor and monitor != "{broken": + assert got.monitor == json.loads(monitor) + assert got.detail == json.loads(monitor)["detail"] + else: + assert got.monitor is None + assert got.detail == "" + assert capsys.readouterr() == ("", "") + + +@pytest.mark.parametrize("reason", MONITOR_REASONS) +@pytest.mark.parametrize("result", [None, "", "[1, 2]", "{broken"]) +def test_monitor_reason_is_taken_when_the_monitor_knows_why(tmp_path, reason, result): + mod = _load_monitor_outcome() + _place(tmp_path, monitor=_monitor_json(reason), result=result) + + got = mod.read_launch_outcome(tmp_path, STEM) + + assert got.payload is None + assert got.reason == reason + assert got.relaunch_same_agent is (reason != "usage_limit") + assert got.monitor["reason"] == reason + assert got.detail == "early error (fatal) in err.log: Monthly request limit reached" + + +@pytest.mark.parametrize("monitor", [None, "{broken", '"scalar"', _monitor_json("ok"), _monitor_json("missing")]) +@pytest.mark.parametrize(("result", "reason"), [ + (None, "missing"), ("", "missing"), (" \n", "missing"), + ("[1, 2]", "unparsable"), ("{broken", "unparsable"), ('"text"', "unparsable"), +]) +def test_missing_or_unparsable_is_decided_by_the_result_file(tmp_path, monitor, result, reason, capsys): + mod = _load_monitor_outcome() + _place(tmp_path, monitor=monitor, result=result) + + got = mod.read_launch_outcome(tmp_path, STEM) + + assert got.payload is None + assert got.reason == reason + assert got.relaunch_same_agent is True + if monitor in (None, "{broken", '"scalar"'): + assert got.monitor is None + # 監視の詳細が無いときは、読めなかった理由を 1 文で持つ + assert got.detail and got.detail.strip() == got.detail + else: + assert got.monitor == json.loads(monitor) + assert got.detail == json.loads(monitor)["detail"] + assert capsys.readouterr() == ("", "") + + +def test_detail_without_monitor_explains_why_the_result_is_unusable(tmp_path): + mod = _load_monitor_outcome() + (tmp_path / f"{STEM}-result.json").write_text("[1]", encoding="utf-8") + assert mod.read_launch_outcome(tmp_path, STEM).detail != mod.read_launch_outcome( + tmp_path, "other-stem").detail + + +def test_result_path_overrides_the_default_location(tmp_path): + mod = _load_monitor_outcome() + other = tmp_path / "elsewhere.json" + other.write_text('{"verdict": "ok"}', encoding="utf-8") + _place(tmp_path, result="[1]") + + got = mod.read_launch_outcome(tmp_path, STEM, result_path=other) + + assert got.payload == {"verdict": "ok"} + + +def test_result_path_that_is_a_directory_does_not_raise(tmp_path, capsys): + mod = _load_monitor_outcome() + (tmp_path / f"{STEM}-result.json").mkdir() + + got = mod.read_launch_outcome(tmp_path, STEM) + + assert got.payload is None + assert got.reason == "missing" + assert capsys.readouterr() == ("", "") + + +def test_launch_outcome_is_frozen(tmp_path): + mod = _load_monitor_outcome() + got = mod.read_launch_outcome(tmp_path, STEM) + with pytest.raises(Exception): + got.reason = "ok" diff --git a/plugins/ndf/scripts/tests/test_refresh.py b/plugins/ndf/scripts/tests/test_refresh.py new file mode 100644 index 00000000..72363c43 --- /dev/null +++ b/plugins/ndf/scripts/tests/test_refresh.py @@ -0,0 +1,121 @@ +"""`lib/refresh.py` の `compare` の現状固定テスト（cross-refactoring R2-001）。 + +`compare` は取得失敗・前回記録なし・一致・不一致の 4 分岐を持つ純関数だが、 +instructions-check のテストは `refresh.fetch` をスタブへ差し替えるため本体を通らない。 +ここでは **現状の戻り値をそのまま正解として記録する**。正しさの主張ではない。 +""" +from __future__ import annotations + +import importlib.util +import pathlib +import sys + + +LIB = pathlib.Path(__file__).resolve().parents[1] / "lib" + + +def _load_refresh(): + # `from __future__ import annotations` 下の dataclass は `sys.modules` から + # 自モジュールを引くため、登録してから実行する。 + spec = importlib.util.spec_from_file_location("ndf_lib_refresh_unit", LIB / "refresh.py") + mod = importlib.util.module_from_spec(spec) + sys.modules[spec.name] = mod + spec.loader.exec_module(mod) + return mod + + +def test_compare_returns_undecidable_when_fetch_failed(): + refresh = _load_refresh() + result = refresh.FetchResult(url="https://example.test/x", ok=False, error="HTTP 404") + assert refresh.compare(result, "sha256:aa") == "判定できない" + + +def test_compare_reports_no_previous_record_when_previous_is_none(): + refresh = _load_refresh() + result = refresh.FetchResult(url="https://example.test/x", ok=True, fingerprint="sha256:aa") + assert refresh.compare(result, None) == "前回の記録が無い" + + +def test_compare_reports_no_previous_record_when_previous_is_empty(): + refresh = _load_refresh() + result = refresh.FetchResult(url="https://example.test/x", ok=True, fingerprint="sha256:aa") + assert refresh.compare(result, "") == "前回の記録が無い" + + +def test_compare_reports_unchanged_when_fingerprint_matches(): + refresh = _load_refresh() + result = refresh.FetchResult(url="https://example.test/x", ok=True, fingerprint="sha256:aa") + assert refresh.compare(result, "sha256:aa") == "変わっていない" + + +def test_compare_reports_changed_when_fingerprint_differs(): + refresh = _load_refresh() + result = refresh.FetchResult(url="https://example.test/x", ok=True, fingerprint="sha256:aa") + assert refresh.compare(result, "sha256:bb") == "変わった" + + +class _FakeResponse: + """`opener` が返す応答の疑似実装。1 度本文を返し、次は空で終わる。""" + + def __init__(self, body: bytes): + self._body = body + self._done = False + + def read(self, _size: int) -> bytes: + if self._done: + return b"" + self._done = True + return self._body + + def close(self) -> None: + pass + + +def _opener_success_and_failure(failing_urls: set[str]): + """URL ごとに成功／失敗を出し分ける opener を作る。 + + 失敗させたい URL では例外を投げ、それ以外は本文を返す。`fetch` は失敗を + 例外にせず理由へ畳むため、この差し替えで成功と失敗を混ぜられる。 + """ + + def opener(url, timeout=None): # noqa: ARG001 - timeout は使わない + if url in failing_urls: + raise OSError("接続できない") + return _FakeResponse(url.encode("utf-8")) + + return opener + + +def test_refresh_returns_one_row_per_source_and_counts_failures(): + refresh = _load_refresh() + sources = [ + {"name": "alpha", "url": "https://example.test/a", "checked_at": "2026-01-01", "claim": "A の主張"}, + {"name": "beta", "url": "https://example.test/b", "checked_at": "2026-01-02", "claim": "B の主張"}, + {"name": "gamma", "url": "https://example.test/c", "checked_at": "2026-01-03", "claim": "C の主張"}, + ] + opener = _opener_success_and_failure({"https://example.test/b"}) + + lines, failed = refresh.refresh(sources, timeout=1.0, opener=opener) + + # 返る行数は source 数と一致する（取れなかった URL も黙って落とさない）。 + assert len(lines) == len(sources) + # 失敗件数は失敗した source の数と一致する。 + assert failed == 1 + + +def test_refresh_row_carries_name_and_fetch_state(): + refresh = _load_refresh() + sources = [ + {"name": "alpha", "url": "https://example.test/a", "checked_at": "2026-01-01", "claim": "A の主張"}, + {"name": "beta", "url": "https://example.test/b", "checked_at": "2026-01-02", "claim": "B の主張"}, + ] + opener = _opener_success_and_failure({"https://example.test/b"}) + + lines, _failed = refresh.refresh(sources, timeout=1.0, opener=opener) + + # 各行に source の name が含まれる。 + assert "alpha" in lines[0] + assert "beta" in lines[1] + # 取得の成否が行へ現れる（成功／失敗をそのまま固定する）。 + assert "取得できた" in lines[0] + assert "取得できなかった" in lines[1] 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 b8d74728..3b392c35 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 @@ -191,28 +191,22 @@ fi |---|---| | pidfile + `kill -0` | プロセス生存確認。alive 確認後に `/proc//cmdline` で agent 名一致も検証 (PID 再利用対策)。**プロセスが既に死んでいる場合は result.json の有無のみで OK 判定**する (死亡直後 cmdline 不一致で誤検知しないため) | | codex sentinel | err.log に `^tokens used$` 出現で正常完了マーク | -| early-error | **行頭限定** で `^Error:` / `^FATAL:` / `^panic:` / `^Traceback ` / `^HTTP/1.1 401\|403\|429` / `^Approval mode overridden to "default"` / `^Authentication failed` / 「quota exceeded」「rate limit exceeded」「API key not found/missing/invalid」「sandbox error」を含む行を検出 (diff/doc 引用文中の同語句は誤検知しないよう anchor + benign フィルタ併用) | +| early-error | 致命の文言を検出して止める。**利用上限**（`Monthly request limit reached` / `"api_error_status":429` / 「quota exceeded」「rate limit exceeded」/ `^HTTP/1.1 429`）は理由 `usage_limit`、それ以外の致命（`^HTTP/1.1 401\|403` / `^Authentication failed` / 「API key not found/missing/invalid」「sandbox error」など）は理由 `early_error`。どちらも状態は `EARLY_ERROR`（終了コード 4）。err.log は全担当、stdout.log は claude だけ JSON 向けの照合で見る。diff / doc 引用文中の同語句は anchor + benign フィルタで除外する。`^Error:` / `^Traceback ` などは警告に留めて止めない | | stall timeout | err.log + stdout.log + progress.log の合計サイズが一定時間変化しなければ STALLED で中断。既定は **agent 別** (codex=**180s** / agy=**480s** / kiro=**480s** / claude=**900s**、上限の表)。agy は err.log がほぼ無音のため大きめに取る。上書き方法: CLI `--stall-timeout` (明示優先) > env `MONITOR_STALL_` (per-agent) > env `MONITOR_STALL` (両 agent 共通) > agent 別ビルトイン | | hard timeout | 既定は `--phase` の工程の値で、上限の表（`scripts/lib/limits.py`）が持つ（review / critique は **1200 秒**）。上書きは `--timeout` > env `MONITOR_TIMEOUT_` > env `MONITOR_TIMEOUT`。agy の CLI の上限（`--print-timeout`）は同じ解決の値 + 120 秒で、監視より先に打ち切らない。解決した stall が hard timeout 以上なら担当ごとに警告する | | progress.log heartbeat | launcher が任意で `-review-pr-progress.log` への短いフェーズマーカー出力を要求し、monitor が最終行を stderr の heartbeat に表示する。内部推論ではなく `scan` / `analyze` / `post` / `done` などの監視用ステータスだけを出す | -| result.json 存在 | プロセス終了後、result.json が無ければ NO_RESULT (exit 3) | +| result.json 存在 | プロセス終了後、result.json が無ければ NO_RESULT (exit 3)。err.log に CLI 自身の上限の文言（agy の `print timeout after … with turn in progress`）があれば理由は `cli_timeout`、無ければ `missing` | | **result.json + age fallback** | sentinel を持たない agent (agy) 向け。プロセスが alive のまま result.json の mtime が **30 秒以上前**なら完了とみなし kill → OK。結果を書いた後にプロセスが終わらないケースに対応 (codex は sentinel チェックが先に発火するため影響なし) | -| **失敗時 kill** | TIMEOUT / STALLED / EARLY_ERROR / PIDFILE_BAD で返るときは対象プロセスに SIGTERM → 3 秒後 SIGKILL。残存プロセスが後から `gh api` 投稿や result.json 書き込みを行うのを防ぐ | - -> ⚠ **罠**: `nohup ... &` でラッパーシェルは即終了し、ハーネスから -> 「タスク完了」通知が飛んでくる。これに惑わされず、`monitor.py` で -> 実プロセスの完了を pidfile / sentinel で確認すること。 -> -> ⚠ **`pgrep -fa ` で完了判定しない**: agy は long `-p` プロンプトを -> 引数に持つため、`grep` のキーワード選定で誤検知する。**pidfile 必須**。 -> -> ⚠ **sentinel 単独で完了判定しない**: codex がクラッシュすると `tokens used` が -> 永遠に出ない。`monitor.py` は sentinel と pidfile/result.json/err.log を併用する。 -> -> ⚠ **Docker 環境ではゾンビプロセスに注意**: `nohup ... & disown` で起動した -> プロセスは、終了後にゾンビ化する (PID 1 が proper init でない場合)。 -> `monitor.py` は `/proc//status` でゾンビを検出して dead 扱いする。 -> **推奨: Docker 実行時に `--init` フラグを付ける** (tini が PID 1 になりゾンビを reap する)。 +| **失敗時 kill** | TIMEOUT / STALLED / EARLY_ERROR / PIDFILE_BAD で返るときは対象に SIGTERM → 3 秒後 SIGKILL。`launch-cli.sh` は CLI を独立したプロセスグループで起動する（`set -m`）ため、対象がグループの先頭なら**グループごと**止める。止めた後に子プロセスが `gh api` 投稿や result.json 書き込みを行うのを防ぐ（#584） | + +完了判定の罠を 4 つ挙げる。 + +| 罠 | 守ること | +|---|---| +| `nohup ... &` でラッパーシェルは即終了し、ハーネスから「タスク完了」通知が飛んでくる | 惑わされず、`monitor.py` で実プロセスの完了を pidfile / sentinel で確認する | +| agy は長い `-p` プロンプトを引数に持ち、`pgrep -fa ` はキーワード選定で誤検知する | `pgrep` で完了判定しない。**pidfile 必須** | +| codex がクラッシュすると sentinel `tokens used` が永遠に出ない | sentinel 単独で完了判定しない。`monitor.py` は pidfile / result.json / err.log を併用する | +| Docker では `nohup ... & disown` のプロセスが終了後にゾンビ化する（PID 1 が init でない場合） | `monitor.py` は `/proc//status` でゾンビを検出して dead 扱いする。**Docker 実行時は `--init` を付ける**（tini がゾンビを回収する） | AI への入出力の契約（2.2）と、AI が書き出すファイルの契約（2.3）は [04-contracts.md](04-contracts.md) にある。 @@ -230,9 +224,9 @@ done #### 申告されたコメント数を GitHub 側と突き合わせる -投稿は **AI 自身が `gh api` で行う**ため、失敗しても結果ファイルの申告だけは残る。 -申告のまま進むと、修正担当が読むべき指摘が GitHub 上に存在しないまま収束判定まで走る。 -実測では、2 件の申告に対しスレッドが 1 つも作られていなかった。 +投稿は **AI 自身が `gh api` で行う**ため、失敗しても結果ファイルの申告だけは残る。申告のまま進むと、 +修正担当が読むべき指摘が GitHub 上に存在しないまま収束判定まで走る。実測では、2 件の申告に対し +スレッドが 1 つも作られていなかった。 `read-result` は申告が 1 件以上のとき、`review_url` の識別子から `repos//pulls//reviews//comments` を数えて突き合わせる。 @@ -244,8 +238,7 @@ done | n 件 | n 件未満 | **中断する。** 投稿が届いていない | | n 件 | 取得できない | 申告を採用し、確認できなかったことを出力へ残す | -**「取得できなかった」と「0 件」を区別する。** 取得の失敗で止めると、GitHub 側の -一時的な不調でループが進まなくなる。 +**「取得できなかった」と「0 件」を区別する。** 取得の失敗で止めると、GitHub 側の一時的な不調でループが進まなくなる。 **誰がレビューし、いつ止めるかは [05-pool-and-convergence.md](05-pool-and-convergence.md) にある。** 母集合・担当の輪番・認証の確認と、終了基準の 3 つの層をそこで定める。 @@ -289,35 +282,45 @@ eval "$JUDGE_VARS" | `SKIP` | `--only` の指定で起動しなかった | 判定へ届かない（短絡する） | | `NO_RESULT` | 起動したが、使える結果が残らなかった | 通らない | -`NO_RESULT` は `read-result` が書き込む。理由は `no_result_reason` に残る。 +`NO_RESULT` は `read-result` が書き込む。理由は `no_result_reason` に、監視が残した詳細（err.log の +抜粋、最大 200 文字）は `monitor_detail` に残る（監視の結果ファイルがあったときだけ）。理由の語彙と +起動し直しの可否を持つのは共通層の `monitor_outcome.py` だけで、`read-result` はその値を写す（#729）。 -| 理由 | 何が起きたか | `read-result` の終了コード | -| --- | --- | --- | -| `missing` | 結果ファイルが無い、または空 | 1 | -| `unparsable` | JSON として読めない、または dict ではない | 3 | -| `no_verdict` | `event` も `intent` も無い | 1 | - -**記録が無いラウンドも結果なしとして読む。** 骨組みが取り込みを呼び忘れても、判定は -収束しない。 - -終了コード 7 のとき、判定は次の 2 行を追加で出力し、対象を `rounds[-1].relaunched` へ -記録する。**2 度目の判定は記録を見て中断へ回る。** +| 理由 | 何が起きたか | 起動し直し | `read-result` の終了コード | +| --- | --- | --- | --- | +| `missing` | 結果ファイルが無い・空で、監視の理由の文言も無い | 可 | 1 | +| `unparsable` | 結果ファイルがあるが JSON オブジェクトとして読めない | 可 | 3 | +| `no_verdict` | `event` も `intent` も無い | 可 | 1 | +| `not_posted` | 投稿が Pull Request に届いていない | 可 | 1 | +| `timeout` | 監視の上限で打ち切った | 可 | 1 | +| `stalled` | 無進捗の許容を超えて打ち切った | 可 | 1 | +| `early_error` | 利用上限以外の致命の文言で止めた | 可 | 1 | +| `usage_limit` | 担当の CLI の利用上限（月間・週間の枠）に達した | **否** | 1 | +| `cli_timeout` | CLI 自身の上限（agy の `--print-timeout`）で結果を書かずに終わった | 可 | 1 | +| `pidfile_bad` | pid ファイルが無い・別のプロセスを指す | 可 | 1 | + +**記録が無いラウンドも結果なしとして読む。** 骨組みが取り込みを呼び忘れても、判定は収束しない。 + +結果なしの担当があると、判定は理由を `NO_RESULT_REASONS='<担当>=<理由> ...'` の 1 行で出す。 +**起動し直しても解けない理由（`usage_limit`）を含むときは起動し直さず**、`final=error` として終了 +コード 1 で終える（標準エラーに担当・理由・`monitor_detail`）。利用上限は同じ担当を何度起動しても +解けず、待ちと相手の枠を消費するだけである（#619）。それ以外の理由なら終了コード 7 で次の 2 行を +追加で出力し、対象を `rounds[-1].relaunched` へ記録する。**2 度目の判定は記録を見て中断へ回る。** ```text +NO_RESULT_REASONS='agy=missing' RELAUNCH_AGENTS='agy' RELAUNCH_TARGET=agy ``` -`RELAUNCH_TARGET` は `codex` / `agy` / `both` のいずれかで、`monitor.py` へそのまま -渡せる値である。起動し直しはラウンドの中で完結するため、**ラウンドの数え方と上限の -意味は変わらない。** 回数を 1 度に限るのは、待ち時間の上限をラウンドあたり 2 回分に -収めるためである。 +`RELAUNCH_TARGET` は `codex` / `agy` / `both` のいずれかで、`monitor.py` へそのまま渡せる値である。 +起動し直しはラウンドの中で完結するため、**ラウンドの数え方と上限の意味は変わらない。** 回数を +1 度に限るのは、待ち時間の上限をラウンドあたり 2 回分に収めるためである。 ### 判定へ入れる対象 -判定は 2 つの対象を別々に見る。**投稿数と未解決の指摘の数は一致しない。** -投稿数はそのラウンドで外部の AI が新しく投稿した件数で、未解決の指摘は前のラウンドの -分も含む Pull Request 上の総数である。 +判定は 2 つの対象を別々に見る。**投稿数と未解決の指摘の数は一致しない。** 投稿数はそのラウンドで +外部の AI が新しく投稿した件数で、未解決の指摘は前のラウンドの分も含む Pull Request 上の総数である。 | 対象 | 判定への入れ方 | 数え方 | |---|---|---| diff --git a/plugins/ndf/skills/cross-review/docs/03-review-output.md b/plugins/ndf/skills/cross-review/docs/03-review-output.md index a74f062e..b070b56b 100644 --- a/plugins/ndf/skills/cross-review/docs/03-review-output.md +++ b/plugins/ndf/skills/cross-review/docs/03-review-output.md @@ -120,7 +120,23 @@ pint / larastan / test / build などは **中断** を原則とする。 環境変数) で EARLY_ERROR 検知自体を無効化し、hard timeout / stall / sentinel / result.json のみで判定するモードに切り替える 3. **新しい致命パターンを観測した場合**: `EARLY_ERROR_FATAL` に追記する (PR で plugin に反映)。 - 曖昧パターンは `EARLY_ERROR_WARN` 側に置き、kill 対象にはしない + 曖昧パターンは `EARLY_ERROR_WARN` 側に置き、kill 対象にはしない。利用上限の文言は + `USAGE_LIMIT_FATAL`（claude の JSON は `CLAUDE_STDOUT_USAGE_LIMIT`）に置く + +### 上限に当たった場合の見分け方 + +止めたのが監視の上限か、担当の CLI の利用上限か、CLI 自身の上限かは、監視が書いた理由で +見分ける。`err.log` の目視より先にこれを読む（#619 #729）。 + +| 読む場所 | 何が分かるか | +| --- | --- | +| `$TMP_DIR/-review-pr-monitor.json` の `reason` | その担当の**最後の**起動の理由。`usage_limit` なら利用上限、`cli_timeout` なら CLI 自身の上限、`timeout` なら監視の上限、`missing` なら文言の無い結果なし | +| 同じファイルの `detail` | 一致した文言を含む err.log / stdout.log の抜粋（最大 200 文字） | +| `$TMP_DIR/monitor-outcomes.jsonl` | 起動ごとに 1 行が追記だけで積まれる。起動し直した担当の **1 回目の理由**はここに残る（`-monitor.json` は 2 回目で上書きされる）。`reason` と `ended_at` で並べて読む | +| 状態ファイルの `rounds[-1]..no_result_reason` / `monitor_detail` | `read-result` が写した値。`state.py report` のラウンド表には `=NO_RESULT(<理由>)` の形で出る | + +`usage_limit` は起動し直しても解けないため、判定は同じラウンドで起動し直さず終了コード 1 +で終える。枠が戻るまで待つか、その担当を外して回すかは進行側が決める。 - ❌ **fix サブエージェントが Resolve をスキップ** — reply だけでは未対応扱い。Resolve まで実行 - ❌ **review body に identifier prefix を付け忘れる** — GitHub UI 上で誰のレビューか不明になる diff --git a/plugins/ndf/skills/cross-review/docs/04-contracts.md b/plugins/ndf/skills/cross-review/docs/04-contracts.md index b73c2bf3..91b8e8b4 100644 --- a/plugins/ndf/skills/cross-review/docs/04-contracts.md +++ b/plugins/ndf/skills/cross-review/docs/04-contracts.md @@ -239,10 +239,20 @@ launcher が生成するプロンプトに以下を強制している: | `monitor-outcomes.jsonl` | `$TMP_DIR` | 監視の結果を 1 行 1 つで**追記だけ**で積む | 同上。消さない | | `cross-review-pr-<開始時刻の UTC>.json` | 要約の置き場所の `--/` | 実行の要約（所要・結末・ラウンド・起動と `measure`）。**本文・`detail` を含まない** | 状態を保存するたび（同じ実行は上書き） | -`reason` は `status` から決まる（`OK`→`ok` / `TIMEOUT`→`timeout` / `STALLED`→`stalled` / -`EARLY_ERROR`→`early_error` / `NO_RESULT`→`missing` / `PIDFILE_BAD`→`pidfile_bad`）。 +`reason` は既定では `status` から決まる（`OK`→`ok` / `TIMEOUT`→`timeout` / `STALLED`→`stalled` / +`EARLY_ERROR`→`early_error` / `NO_RESULT`→`missing` / `PIDFILE_BAD`→`pidfile_bad`）。監視が +文言で区別した 2 つだけが状態から決まらない: 利用上限は `EARLY_ERROR` のまま `usage_limit`、 +CLI 自身の上限で結果を書かずに終わったときは `NO_RESULT` のまま `cli_timeout`（#729）。 監視の標準出力と終了コードは変わらない。 +結果の取り込みは結果ファイルを自前で開かず、共通層の `monitor_outcome.read_launch_outcome(tmp_dir, +"-review-pr", result_path)` が返す値（使える結果 `payload` / 理由 `reason` / 監視の詳細 +`detail` / 起動し直しの可否 `relaunch_same_agent`）を読む。結果なしのときは +`rounds[-1].` に `intent: "NO_RESULT"`、`no_result_reason: `、監視の結果ファイルが +あれば `monitor_detail: ` を書く（**鍵が無い** = 監視の結果ファイルが無かった。空文字は +書かない）。理由の一覧は [01-state-and-review.md](01-state-and-review.md) の「結果を残さなかった +レビュアーの扱い」にある。 + **要約の置き場所は作業ツリーの外である。** `NDF_METRICS_DIR` → `$XDG_STATE_HOME/ndf/metrics` → `$HOME/.local/state/ndf/metrics` の順に決まり、`NDF_METRICS=0` のときは書かない。`state.py report` の最後の行が、書いた要約のパスか書かなかった理由を出す。束ねるのは共通層の `run_metrics.py aggregate` diff --git a/plugins/ndf/skills/cross-review/scripts/state.py b/plugins/ndf/skills/cross-review/scripts/state.py index f0a02e60..9f58ad0a 100755 --- a/plugins/ndf/skills/cross-review/scripts/state.py +++ b/plugins/ndf/skills/cross-review/scripts/state.py @@ -35,6 +35,7 @@ import auth # noqa: E402 import post_queue # noqa: E402 import run_metrics # noqa: E402 実行の要約（#662） +import monitor_outcome # noqa: E402 起動 1 回の結末（#729） # ---------------- helpers ---------------- @@ -1402,6 +1403,33 @@ def _pending_posts(pr: int) -> int: return _queue(pr).count() +def _flushed_review(item: dict[str, Any]) -> tuple[Any, Any, str] | None: + """待ち行列の項目から、確認するレビューの担当・ラウンド・URL を得る。""" + if item.get("kind") != "review-post": + return None + extra = item.get("extra") or {} + agent, round_no = extra.get("agent"), extra.get("round") + if not (agent and round_no): + return None + response = item.get("response") + response = response if isinstance(response, dict) else {} + url = str(response.get("html_url") or "") + if not url and response.get("id"): + url = f"#pullrequestreview-{response['id']}" + return agent, round_no, url + + +def _flushed_review_target( + state: dict[str, Any], agent: Any, round_no: Any) -> dict[str, Any] | None: + """レビューを積んだラウンドから、担当の書き戻し先を探す。""" + return next( + (entry for entry in state.get("rounds", []) + if entry.get("round") == round_no + and isinstance(entry.get(agent), dict)), + None, + ) + + def _confirm_flushed(pr: int, item: dict[str, Any]) -> None: """流した直後に、投稿が届いたことを 1 度だけ確かめる。 @@ -1415,24 +1443,14 @@ def _confirm_flushed(pr: int, item: dict[str, Any]) -> None: 確認を促す側にも働かない**（判定は `queued` を見て照会を飛ばす）。共通層が 見つけた投稿を `response` として渡すため、どちらも同じ経路で確かめられる。 """ - if item.get("kind") != "review-post": - return - extra = item.get("extra") or {} - agent, round_no = extra.get("agent"), extra.get("round") - if not (agent and round_no): + review = _flushed_review(item) + if review is None: return - resp = item.get("response") if isinstance(item.get("response"), dict) else {} - url = str(resp.get("html_url") or "") - if not url and resp.get("id"): - url = f"#pullrequestreview-{resp['id']}" + agent, round_no, url = review st = _load(pr) # **書き戻す先は、その項目が属するラウンドである。** 積んだラウンドと流した # ラウンドが同じとは限らないため、最後のラウンドへ書かない。 - target = next( - (e for e in st.get("rounds", []) - if e.get("round") == round_no and isinstance(e.get(agent), dict)), - None, - ) + target = _flushed_review_target(st, agent, round_no) if target is None: return exists = _review_exists(str(st.get("repo") or ""), @@ -1506,51 +1524,54 @@ def cmd_flush(args: argparse.Namespace) -> None: info(f"✅ 待ち行列は空です（送った {len(result.sent)} 件）") -def _print_init_result( - pr: object, - worktree: object, - tmp_dir: object, - repo: object, - head_branch: object, - base_branch: object, - is_own: bool, - event_downgrade: bool, - has_extra: bool, - carried_count: int, - resumed: bool, -) -> None: - """cmd_init の 2 経路（再開・新規）が共有する末尾の出力ブロック。 +class _InitResult(NamedTuple): + """cmd_init が標準出力の機械可読ブロックへ書く初期化結果。 - 出力形式は再開側・新規側で同一のため 1 箇所へ寄せる。PR 番号だけは - 元の両分岐に合わせて quote しない（数値のため）。 + 再開経路と新規経路が同じ組を渡すため、位置引数の並びではなく名前付きの + フィールドで受け渡す。 """ - print(f"PR={pr}") - print(f'WORKTREE={shlex.quote(str(worktree))}') - print(f'TMP_DIR={shlex.quote(str(tmp_dir))}') - print(f'REPO={shlex.quote(str(repo))}') - print(f'HEAD_BRANCH={shlex.quote(str(head_branch))}') - print(f'BASE_BRANCH={shlex.quote(str(base_branch))}') - print(f"IS_OWN_PR={'1' if is_own else '0'}") - print(f"EVENT_DOWNGRADE={'1' if event_downgrade else '0'}") - print(f"HAS_EXTRA_REVIEW_INSTRUCTIONS={'1' if has_extra else '0'}") - print(f"CARRIED_OVER_THREADS={carried_count}") - print(f"RESUMED={'1' if resumed else '0'}") + pr: object + worktree: object + tmp_dir: object + repo: object + head_branch: object + base_branch: object + is_own: bool + event_downgrade: bool + has_extra: bool + carried_count: int + resumed: bool -def _resume_from_state( - pr: object, - repo: str, - worktree: str, - manual_extra_review: str, -) -> bool: - """既存 state からの再開経路。 - 再開に該当し出力まで済ませたら True、該当する state が無ければ False を返す。 - False のとき cmd_init は新規 init へ進む。 +def _print_init_result(result: _InitResult) -> None: + """cmd_init の 2 経路（再開・新規）が共有する末尾の出力ブロック。 + + 出力形式は再開側・新規側で同一のため 1 箇所へ寄せる。PR 番号だけは + 元の両分岐に合わせて quote しない（数値のため）。 + """ + print(f"PR={result.pr}") + print(f'WORKTREE={shlex.quote(str(result.worktree))}') + print(f'TMP_DIR={shlex.quote(str(result.tmp_dir))}') + print(f'REPO={shlex.quote(str(result.repo))}') + print(f'HEAD_BRANCH={shlex.quote(str(result.head_branch))}') + print(f'BASE_BRANCH={shlex.quote(str(result.base_branch))}') + print(f"IS_OWN_PR={'1' if result.is_own else '0'}") + print(f"EVENT_DOWNGRADE={'1' if result.event_downgrade else '0'}") + print(f"HAS_EXTRA_REVIEW_INSTRUCTIONS={'1' if result.has_extra else '0'}") + print(f"CARRIED_OVER_THREADS={result.carried_count}") + print(f"RESUMED={'1' if result.resumed else '0'}") + + +def _find_resumable_state( + pr: object, worktree: str, +) -> tuple[dict[str, Any], pathlib.Path] | None: + """既存 state を探し、再開できるものだけを (state, path) で返す。 + + 再開に該当しなければ `None`。**探索の入口は `_tmp_dir()` を使わない**（mkdir の + 副作用でパスが作られてしまう）。`CROSS_REVIEW_TMP_DIR` があればそれを、無ければ + `/.cross_review/` を直接組む。`final` が確定した state は再開しない。 """ - # 再開チェック: CROSS_REVIEW_TMP_DIR が設定されている場合はそちらを優先し、 - # 未設定なら /.cross_review/ を直接パスとして組む。 - # _tmp_dir() は mkdir 副作用があるため使用せず、パス解決のみ行う。 env_tmp = os.environ.get("CROSS_REVIEW_TMP_DIR") if env_tmp: resume_dir = pathlib.Path(env_tmp).resolve() @@ -1558,10 +1579,21 @@ def _resume_from_state( resume_dir = pathlib.Path(worktree) / ".cross_review" resume_state_file = resume_dir / f"cross-review-pr{pr}-state.json" if not resume_state_file.exists(): - return False + return None st = json.loads(resume_state_file.read_text(encoding="utf-8")) if st.get("final") is not None: - return False + return None + return st, resume_state_file + + +def _refresh_resume_state( + st: dict[str, Any], pr: object, repo: str, manual_extra_review: str, +) -> bool: + """再開する state を最新化し、書き換えたかどうかを返す。 + + 旧形式の補完・manual 指示の反映・`review_instructions` の再計算・引き継ぎの記録を + 行う。**保存はしない**（呼び出し側が変更有無を見て 1 度だけ書く）。 + """ state_changed = False if "auto_review_instructions" not in st: changed_files = _fetch_changed_files(pr, st.get("repo") or repo) @@ -1586,37 +1618,66 @@ def _resume_from_state( # 再開した時点で残っている未解決の指摘を引き継ぎとして記録する。 if _record_carried_over(st, st.get("repo") or repo, st.get("current_pr") or pr): state_changed = True - if state_changed: - _write_state(resume_state_file, st) - info("↻ 追加レビュー観点を state に反映して再開") - # 待ち行列を流すのは、手元の `st` を書き戻した**後**である。流した結果 - # （`queued` の解除と、届かなかった投稿の結果なし）は `_confirm_flushed` が - # 状態ファイルへ直接書く。先に流すと、この関数がその後に書き戻す古い `st` が - # それらを消す。再開の入口で流すこと自体は変えないため、回復した後の - # 1 本目のコマンドで届く。 - # 渡すのは状態ファイルの鍵（`args.pr`）で、`current_pr` ではない。待ち行列も - # 状態ファイルも鍵で引くため、巻き直しの後に `current_pr` を渡すと引けない。 + return state_changed + + +def _sync_resume_worktree(st: dict[str, Any], pr: object, worktree: str) -> pathlib.Path: + """保存後の副作用（待ち行列の flush → tmp_dir 解決 → 作業ツリー同期）を順に行う。 + + 順序に意味がある: + 1. **待ち行列を流すのは、手元の `st` を書き戻した後である。** 流した結果 + （`queued` の解除と、届かなかった投稿の結果なし）は `_confirm_flushed` が + 状態ファイルへ直接書く。先に流すと、この後の書き戻しが古い `st` でそれらを + 消す。渡すのは状態ファイルの鍵（`args.pr`）で、`current_pr` ではない。 + 2. `tmp_dir` を解決して返す（`_print_init_result` が使う）。 + 3. **再開でも同期する。** 中断から再開までの間に head が進んでいることがあり、 + そのまま次のラウンドを回すと古い差分をレビューさせる。 + """ _auto_flush(pr) tmp_dir = _tmp_dir(worktree) wt = st.get("worktree_path") or "" - # 再開でも同期する。中断から再開までの間に head が進んでいることがあり、 - # そのまま次のラウンドを回すと古い差分をレビューさせる。 resume_head = str(st.get("head_branch") or "") if wt and resume_head and _is_registered_worktree(str(wt)): _sync_worktree(str(wt), int(st.get("current_pr") or pr), resume_head) + return tmp_dir + + +def _resume_from_state( + pr: object, + repo: str, + worktree: str, + manual_extra_review: str, +) -> bool: + """既存 state からの再開経路。 + + 再開に該当し出力まで済ませたら True、該当する state が無ければ False を返す。 + False のとき cmd_init は新規 init へ進む。 + """ + found = _find_resumable_state(pr, worktree) + if found is None: + return False + st, resume_state_file = found + + if _refresh_resume_state(st, pr, repo, manual_extra_review): + _write_state(resume_state_file, st) + info("↻ 追加レビュー観点を state に反映して再開") + + tmp_dir = _sync_resume_worktree(st, pr, worktree) info(f"↻ 前回中断 state から再開（round={len(st.get('rounds', []))}）") _print_init_result( - st["current_pr"], - wt, - tmp_dir, - st.get("repo") or "", - st.get("head_branch") or "", - st.get("base_branch") or "", - bool(st.get("is_own_pr")), - bool(st.get("event_downgrade")), - bool(st.get("review_instructions")), - (st.get("carried_over") or {}).get("count", 0), - True, + _InitResult( + pr=st["current_pr"], + worktree=st.get("worktree_path") or "", + tmp_dir=tmp_dir, + repo=st.get("repo") or "", + head_branch=st.get("head_branch") or "", + base_branch=st.get("base_branch") or "", + is_own=bool(st.get("is_own_pr")), + event_downgrade=bool(st.get("event_downgrade")), + has_extra=bool(st.get("review_instructions")), + carried_count=(st.get("carried_over") or {}).get("count", 0), + resumed=True, + ) ) return True @@ -1854,17 +1915,19 @@ def _finalize_initial_state( _write_state(ws_ctx.state_file, state) info(f"✅ state 初期化: {ws_ctx.state_file}") _print_init_result( - pr, - pr_ctx.worktree, - ws_ctx.tmp_dir, - pr_ctx.repo, - pr_ctx.meta.head_branch, - pr_ctx.meta.base_branch, - pr_ctx.is_own, - pr_ctx.event_downgrade, - bool(review_ctx.review_instructions), - 0, - False, + _InitResult( + pr=pr, + worktree=pr_ctx.worktree, + tmp_dir=ws_ctx.tmp_dir, + repo=pr_ctx.repo, + head_branch=pr_ctx.meta.head_branch, + base_branch=pr_ctx.meta.base_branch, + is_own=pr_ctx.is_own, + event_downgrade=pr_ctx.event_downgrade, + has_extra=bool(review_ctx.review_instructions), + carried_count=0, + resumed=False, + ) ) pr_ctx = _resolve_pr_and_ownership(pr, repo, worktree, args.worktree) @@ -2404,12 +2467,17 @@ def cmd_unresolved_threads(args: argparse.Namespace) -> None: info(f"- {t['id']} {t.get('path', '')}:{t.get('line', '')}") -def _record_no_result(pr: int, agent: str, reason: str) -> None: +def _record_no_result( + pr: int, agent: str, reason: str, monitor_detail: str | None = None, +) -> None: """使える結果が残らなかったことを、そのラウンドへ残す。 判定（`cmd_judge`）はこの記録を読んで、起動し直しか中断かを決める。記録が無い ラウンドも結果なしとして読むため、ここで書けなかった場合も収束はしない。 + `monitor_detail` は監視の `detail`（#729）。**鍵が無い = 監視の結果ファイルが + 無かった**を保つため、`None` と空文字では鍵を書かない。 + 状態ファイルを読めないときとラウンドがまだ無いときは、何も書かずに戻る。呼び出し 元はこの直後に die するため、ここで新たに止める理由が無い。 """ @@ -2422,7 +2490,7 @@ def _record_no_result(pr: int, agent: str, reason: str) -> None: return if not isinstance(st, dict) or not st.get("rounds"): return - st["rounds"][-1][agent] = { + entry: dict[str, Any] = { "intent": NO_RESULT, "no_result_reason": reason, "posted_as": None, @@ -2430,6 +2498,9 @@ def _record_no_result(pr: int, agent: str, reason: str) -> None: "review_url": None, "by_severity": {}, } + if monitor_detail: + entry["monitor_detail"] = monitor_detail + st["rounds"][-1][agent] = entry _save(pr, st) @@ -2440,33 +2511,30 @@ def _die_no_result(pr: int, agent: str, reason: str, msg: str, code: int = 1) -> def _read_review_result_file(pr: int, agent: str, rfile: pathlib.Path) -> dict[str, Any]: - if not rfile.exists() or rfile.stat().st_size == 0: - _die_no_result(pr, agent, "missing", f"{agent}: result 未生成 ({rfile})") + """結果ファイルを、結末の共通層の値として読む（#729 の決定 2）。 - try: - r = json.loads(rfile.read_text(encoding="utf-8")) - except json.JSONDecodeError as exc: - _die_no_result( - pr, - agent, - "unparsable", - f"{agent}: result.json の parse に失敗 ({rfile}): {exc}", - code=3, - ) - - # gemini round 4 指摘: result.json は本来 dict だが、launcher の出力バグや - # 別実行の残骸で list / str が入り込むと `r.get(...)` で AttributeError になる。 - # 不正な review result はバグなので即時 die(code=3) で停止させる。 - if not isinstance(r, dict): - _die_no_result( - pr, - agent, - "unparsable", - f"{agent}: result.json が dict ではない " - f"({rfile}, type={type(r).__name__})。review launcher の出力形式不正。", + 結果ファイルを自前で開かない。監視の結果ファイル（`-monitor.json`）と突き合わせて + 使える結果か理由かを決めるのは `read_launch_outcome` で、ここが決めるのは終了コードだけ + である（読めない結果は 3、それ以外は 1。変更前と同じ）。理由の語彙も起動し直しの可否も + ここには置かない。 + """ + outcome = monitor_outcome.read_launch_outcome( + _resolve_tmp_dir(pr), f"{agent}-review-pr{pr}", rfile) + if outcome.payload is not None: + return outcome.payload + reason = outcome.reason or "missing" + # 監視の結果ファイルがあるときだけ、その `detail` を残す（無いときの `outcome.detail` は + # 読めなかった理由の 1 文で、監視の詳細ではない） + monitor_detail = str(outcome.monitor.get("detail") or "") if outcome.monitor else None + _record_no_result(pr, agent, reason, monitor_detail) + if reason == "unparsable": + # 結果ファイルはあるが JSON の dict として parse できない。launcher の出力形式不正 + die( + f"{agent}: result.json の parse に失敗、または dict ではない ({rfile}):" + f" {outcome.detail}", code=3, ) - return r + die(f"{agent}: 使える結果が無い (reason={reason}, {rfile}): {outcome.detail}") def _verify_review_arrival( @@ -2761,34 +2829,83 @@ def _round_ci(st: dict[str, Any], last: dict[str, Any], pr: int) -> dict[str, An return {"verdict": "success", "sha": sha} +def _no_result_reasons(last: dict[str, Any], no_result: list[str]) -> dict[str, str]: + """結果なしの担当ごとの理由を集め、`NO_RESULT_REASONS` を出す。 + + どの出口でも進行側が理由を読めるように、先頭で 1 度だけ出す（#729 の AC14）。 + """ + reasons = { + a: (last.get(a) or {}).get("no_result_reason") or "missing" for a in no_result + } + print(f"NO_RESULT_REASONS='{' '.join(f'{a}={r}' for a, r in reasons.items())}'") + return reasons + + +def _abort_no_result_round(pr: int, st: dict[str, Any], msg: str) -> None: + """結果なしのラウンドを異常終了させる共通処理。 + + `final=error` にして保存し、終了コード 1 で止める。最終スイープを通してから + 完了報告へ進むよう促す文言は呼び出し側が渡す。 + """ + st["final"] = "error" + st["ended_at"] = _now() + _save(pr, st) + die(msg, code=1) + + +def _record_relaunch( + pr: int, st: dict[str, Any], last: dict[str, Any], pending: list[str] +) -> None: + """同じラウンドで起動し直す担当を記録し、シェル向けの出力を出す。""" + last["relaunched"] = (last.get("relaunched") or []) + pending + _save(pr, st) + print(f"RELAUNCH_AGENTS='{' '.join(pending)}'") + print(f"RELAUNCH_AGENTS_CSV={','.join(pending)}") + # 互換のために残す。**`both` は codex / agy の 2 者だけを指す語**であるため、 + # 担当がそれ以外を含むラウンドでは CSV の側を使う。 + print(f"RELAUNCH_TARGET={'both' if len(pending) == 2 else pending[0]}") + info( + f"→ 結果を残さなかったレビュアーがいる: {' '.join(pending)}。" + "同じラウンドで 1 度だけ起動し直す。" + ) + + def _handle_no_result_round( pr: int, st: dict[str, Any], last: dict[str, Any], no_result: list[str] ) -> None: + """結果なしの担当があるラウンドの出口を決める。 + + 先に理由の行（`NO_RESULT_REASONS`）を出す。どの出口でも進行側が理由を読めるようにする + ためである（#729 の AC14）。**起動し直しの可否は結末の共通層だけが決める** + （`monitor_outcome.relaunch_same_agent`）。可否が偽の理由が 1 つでもあれば、誰も起動し直さず + 誤りの終わりへ進む。起動し直しても解けない理由で待つのは、相手の CLI の枠と時間を使うだけ + である（#619）。骨組みは既存の 1 の枝で受けるため、終了コードは増えない（決定 12）。 + """ last["verdict"] = "no_result" + reasons = _no_result_reasons(last, no_result) + blocked = [a for a, r in reasons.items() + if not monitor_outcome.relaunch_same_agent(r)] + if blocked: + for a in blocked: + detail = (last.get(a) or {}).get("monitor_detail") + info(f" {a}: reason={reasons[a]}" + (f" detail={detail}" if detail else "")) + _abort_no_result_round( + pr, st, + f"起動し直しても解けない理由で結果が残りませんでした: {' '.join(blocked)}。" + " 同じラウンドで起動し直さずに中断します。最終スイープを通してから" + "完了報告へ進んでください", + ) relaunched = last.get("relaunched") or [] pending = [a for a in no_result if a not in relaunched] if not pending: # 2 度続けて結果が残らないのは、対象や負荷ではなく実行環境の側の事象である。 - st["final"] = "error" - st["ended_at"] = _now() - _save(pr, st) - die( + _abort_no_result_round( + pr, st, f"起動し直した後も結果が残りませんでした: {' '.join(no_result)}。" " 実行環境の側の問題として中断します。最終スイープを通してから" "完了報告へ進んでください", - code=1, ) - last["relaunched"] = relaunched + pending - _save(pr, st) - print(f"RELAUNCH_AGENTS='{' '.join(pending)}'") - print(f"RELAUNCH_AGENTS_CSV={','.join(pending)}") - # 互換のために残す。**`both` は codex / agy の 2 者だけを指す語**であるため、 - # 担当がそれ以外を含むラウンドでは CSV の側を使う。 - print(f"RELAUNCH_TARGET={'both' if len(pending) == 2 else pending[0]}") - info( - f"→ 結果を残さなかったレビュアーがいる: {' '.join(pending)}。" - "同じラウンドで 1 度だけ起動し直す。" - ) + _record_relaunch(pr, st, last, pending) sys.exit(7) @@ -3065,41 +3182,14 @@ def _merged_root( return current -def _verify_findings( - st: dict[str, Any], - round_no: int, +def _run_finding_checks( + targets: list[dict[str, Any]], allowed: list[str], work: str, - reproduced_codes: Optional[list[int]] = None, - runner: Optional[Any] = None, + codes: set[int], + run: Any, ) -> None: - """`suggested_check` を実行し、結果を `verification` へ残す（#156）。 - - **担当の再評価より先に走らせる。** 機械が再現した事実は、担当の支持より確かである。 - - **束ねた組の全員を対象にする**（`docs/specifications/cross-review-evidence-based.md` - の「重複の統合」）。 - 代表の `suggested_check` だけを読むと、代表が手順を書いていない組は、束ねられた側が - 実行できる手順を書いていても実行回数 0・`not_run` のまま `insufficient_evidence` へ - 落ちる。**どちらが先に取り込まれたかで採否が変わる。** 1 段目の統合は実行検証より - **前**にあるため、束ねられた側を読み飛ばすと、その手順は一度も実行されない。 - - 代表が持つのは組の集約である。`reproduced` > `not_reproduced` > `not_run` の順で - 最初に当たった 1 件を採り、**出所を `verification.finding_id` へ残す**。 - - **同じコマンドは 1 度しか実行しない。** 組の全員が同じ `suggested_check` を書くのは - 普通に起こり（統合の条件は本文の一致である）、そのたびに走らせると実行が増える。 - - **`ran_at` は結果を受け取った記録にだけ入れる。** 初期化で入れると、実行していない - `not_run` の記録にも時刻が残り、実行済みに見える。記録の `result` / `exit_code` / - `ran_at` が同じことを指すようにする。 - """ - codes = set(reproduced_codes or VERIFY_REPRODUCED_CODES) - run = runner or _run_verify - targets = [ - f for f in st.get("review_findings") or [] if f.get("round") == round_no - ] - by_id = {f.get("finding_id"): f for f in targets} + """各指摘の検証コマンドを重複なく実行し、結果を記録する。""" ran: dict[tuple[str, ...], Optional[int]] = {} for finding in targets: @@ -3124,7 +3214,11 @@ def _verify_findings( record["result"] = "not_reproduced" finding["verification"] = record - # 代表は組から選び直す。**実行し直さない**（記録済みの結果を選ぶだけである）。 + +def _propagate_best_verification( + targets: list[dict[str, Any]], by_id: dict[Any, dict[str, Any]] +) -> None: + """束ねた組から最良の検証結果を代表へ反映する。""" for rep in targets: if rep.get("merged_into"): continue @@ -3141,6 +3235,45 @@ def _verify_findings( rep["verification"] = dict(best) +def _verify_findings( + st: dict[str, Any], + round_no: int, + allowed: list[str], + work: str, + reproduced_codes: Optional[list[int]] = None, + runner: Optional[Any] = None, +) -> None: + """`suggested_check` を実行し、結果を `verification` へ残す（#156）。 + + **担当の再評価より先に走らせる。** 機械が再現した事実は、担当の支持より確かである。 + + **束ねた組の全員を対象にする**（`docs/specifications/cross-review-evidence-based.md` + の「重複の統合」）。 + 代表の `suggested_check` だけを読むと、代表が手順を書いていない組は、束ねられた側が + 実行できる手順を書いていても実行回数 0・`not_run` のまま `insufficient_evidence` へ + 落ちる。**どちらが先に取り込まれたかで採否が変わる。** 1 段目の統合は実行検証より + **前**にあるため、束ねられた側を読み飛ばすと、その手順は一度も実行されない。 + + 代表が持つのは組の集約である。`reproduced` > `not_reproduced` > `not_run` の順で + 最初に当たった 1 件を採り、**出所を `verification.finding_id` へ残す**。 + + **同じコマンドは 1 度しか実行しない。** 組の全員が同じ `suggested_check` を書くのは + 普通に起こり（統合の条件は本文の一致である）、そのたびに走らせると実行が増える。 + + **`ran_at` は結果を受け取った記録にだけ入れる。** 初期化で入れると、実行していない + `not_run` の記録にも時刻が残り、実行済みに見える。記録の `result` / `exit_code` / + `ran_at` が同じことを指すようにする。 + """ + codes = set(reproduced_codes or VERIFY_REPRODUCED_CODES) + run = runner or _run_verify + targets = [ + f for f in st.get("review_findings") or [] if f.get("round") == round_no + ] + by_id = {f.get("finding_id"): f for f in targets} + _run_finding_checks(targets, allowed, work, codes, run) + _propagate_best_verification(targets, by_id) + + def cmd_verify_findings(args: argparse.Namespace) -> None: """Step 2.5 前段 — 重複を束ね（1 段目）、`suggested_check` を実行する（#156）。 @@ -3951,31 +4084,39 @@ def _merge_fix_records(st: dict, fix: dict, pr: int) -> dict: return st["rounds"][-1]["fix"] -def _normalize_fix_result(fix: dict) -> dict[str, Any]: - """fix の戻り値から別名と劣化表現を吸収し、記録へ写す値にそろえる。""" - # key 名 fallback (サブエージェントが別名で書いた場合の救済)。 - # 正規は fix_commit / fixed_count、別名は commit_sha / fixed のみ受理する。 +def _resolve_fix_aliases(fix: dict) -> tuple[object, object]: + """fix の commit と fixed 件数を正規 key と別名から解決する。""" fix_commit = fix.get("fix_commit") or fix.get("commit_sha") fixed_count = fix.get("fixed_count") if fixed_count is None: fixed_count = fix.get("fixed", 0) + return fix_commit, fixed_count + + +def _normalize_deferred_like(raw: object) -> tuple[list[dict], int]: + """deferred の項目と、劣化表現を考慮した保存件数を返す。""" + items = _normalize_dict_items(raw) + if isinstance(raw, (list, dict)): + return items, len(items) + return items, _count(raw) + + +def _normalize_fix_result(fix: dict) -> dict[str, Any]: + """fix の戻り値から別名と劣化表現を吸収し、記録へ写す値にそろえる。""" + # key 名 fallback (サブエージェントが別名で書いた場合の救済)。 + # 正規は fix_commit / fixed_count、別名は commit_sha / fixed のみ受理する。 + fix_commit, fixed_count = _resolve_fix_aliases(fix) # deferred は list が正だが、LLM がスキーマを無視して文字列リスト # (例: ["nit: ..."]) や単一 dict、int(件数) を返すケースがある。後段の # deferred_nits 展開ループは dict 以外をスキップするため、まず dict 要素のみへ # 正規化する (単一 dict は 1 件として包む)。 - _deferred_raw = fix.get("deferred") - _deferred_nits = _normalize_dict_items(_deferred_raw) - # 保存件数の単一整合ルール: # - 構造化データ (list / dict) は per-item を保持できるので、展開件数 # (len(_deferred_nits)) を保存し deferred_nits の件数と一致させる。 # - int / 数値文字列は per-item データを失った「劣化表現」なので、件数を # 失わないよう _count() の値を保存する (展開はできないので nits は空)。 - if isinstance(_deferred_raw, (list, dict)): - _deferred_count = len(_deferred_nits) - else: - _deferred_count = _count(_deferred_raw) + _deferred_nits, _deferred_count = _normalize_deferred_like(fix.get("deferred")) # 却下も同じ正規化を通す。**件数だけが返る劣化表現（int）では per-item を作れない** # ため、そのときは記録を空にし、件数は `_count()` の値で残す。 @@ -4244,7 +4385,10 @@ def _print_round_summary(rounds: list) -> None: parts = [] for name in reviewers: entry = r.get(name) or {} - if entry: + if entry.get("intent") == NO_RESULT: + # 結果なしは投稿の数を持たない。括弧には理由を出す（#729 の AC17） + parts.append(f"{name}=NO_RESULT({entry.get('no_result_reason', '-')})") + elif entry: parts.append(f"{name}={entry.get('intent', '-')} ({entry.get('comments', '-')})") else: parts.append(f"{name}=-") @@ -4334,19 +4478,10 @@ def cmd_report(args: argparse.Namespace) -> None: # ---------------- main ---------------- -def build_parser() -> argparse.ArgumentParser: - """副コマンドの引数を組み立てる。**テストが選択肢を検査できるように分ける。** +_Subparsers = argparse._SubParsersAction - 実機で `kiro` の結果が `invalid choice` で弾かれた。担当が 4 つの名前を取りうる - 以上、副コマンドの引数も同じ母集合を持たなければ、結果を残した担当が「結果なし」 - として扱われる。 - """ - # 副コマンドの説明はここ（`help`）だけが持つ。**モジュールの docstring へ写さない。** - # 2 か所へ書くと片方だけが実装から離れる。振動の検知の基準は実装が 3 つの一致へ - # 変わった後も、docstring 側が古い基準を出し続けていた（#329）。 - p = argparse.ArgumentParser(description=__doc__) - sub = p.add_subparsers(dest="cmd", required=True) +def _add_init_parser(sub: _Subparsers) -> None: sp = sub.add_parser("init", help="Step 0 — state 初期化 or 再開") sp.add_argument("pr", type=int) sp.add_argument("--max-rounds", type=int, default=12) @@ -4377,6 +4512,8 @@ def build_parser() -> argparse.ArgumentParser: ) sp.set_defaults(func=cmd_init) + +def _add_start_round_parser(sub: _Subparsers) -> None: sp = sub.add_parser( "start-round", help="Step 1 — round 開始判定 (1=上限到達/5=後始末の未了/8=同期できない)", @@ -4384,12 +4521,16 @@ def build_parser() -> argparse.ArgumentParser: sp.add_argument("pr", type=int) sp.set_defaults(func=cmd_start_round) + +def _add_read_result_parser(sub: _Subparsers) -> None: sp = sub.add_parser("read-result", help="Step 2.4 — review result を state にマージ") sp.add_argument("pr", type=int) sp.add_argument("agent", choices=list(assignment.ALL_RUNTIMES)) sp.add_argument("--file", default=None) sp.set_defaults(func=cmd_read_result) + +def _add_unresolved_threads_parser(sub: _Subparsers) -> None: sp = sub.add_parser( "unresolved-threads", help="PR 上の未解決の指摘を数える (0=数えられた/1=取得できなかった)", @@ -4397,10 +4538,14 @@ def build_parser() -> argparse.ArgumentParser: sp.add_argument("pr", type=int) sp.set_defaults(func=cmd_unresolved_threads) + +def _add_flush_parser(sub: _Subparsers) -> None: sp = sub.add_parser("flush", help="待ち行列に積んだ投稿を流す (常に 0)") sp.add_argument("pr", type=int) sp.set_defaults(func=cmd_flush) + +def _add_judge_parser(sub: _Subparsers) -> None: sp = sub.add_parser( "judge", help="Step 3 — intent ベース pass 判定 " @@ -4409,6 +4554,8 @@ def build_parser() -> argparse.ArgumentParser: sp.add_argument("pr", type=int) sp.set_defaults(func=cmd_judge) + +def _add_check_oscillation_parser(sub: _Subparsers) -> None: sp = sub.add_parser( "check-oscillation", help="Step 4 — 同じ箇所を指す指摘の割合を計算 (2=続行/4=振動で中断)", @@ -4416,27 +4563,37 @@ def build_parser() -> argparse.ArgumentParser: sp.add_argument("pr", type=int) sp.set_defaults(func=cmd_check_oscillation) + +def _add_verify_findings_parser(sub: _Subparsers) -> None: sp = sub.add_parser( "verify-findings", help="Step 2.5 前段 — 重複の統合（1 段目）と実行検証（#156）") sp.add_argument("pr", type=int) sp.set_defaults(func=cmd_verify_findings) + +def _add_collect_critiques_parser(sub: _Subparsers) -> None: sp = sub.add_parser( "collect-critiques", help="Step 2.5 後段 — 反証の結果を指摘へ結び、申告の重複を束ねる（#156）") sp.add_argument("pr", type=int) sp.set_defaults(func=cmd_collect_critiques) + +def _add_merge_fix_parser(sub: _Subparsers) -> None: sp = sub.add_parser("merge-fix", help="Step 5 post — fix 戻り値マージ + CI 分類") sp.add_argument("pr", type=int) sp.add_argument("--file", default=None) sp.set_defaults(func=cmd_merge_fix) + +def _add_should_rotate_parser(sub: _Subparsers) -> None: sp = sub.add_parser("should-rotate", help="Step 6 — rotate 要否 (0=rotate/2=keep)") sp.add_argument("pr", type=int) sp.set_defaults(func=cmd_should_rotate) + +def _add_set_current_pr_parser(sub: _Subparsers) -> None: sp = sub.add_parser("set-current-pr", help="rotation 後の current_pr 更新") sp.add_argument( "--head-branch", @@ -4447,6 +4604,8 @@ def build_parser() -> argparse.ArgumentParser: sp.add_argument("new_pr", type=int) sp.set_defaults(func=cmd_set_current_pr) + +def _add_verify_sweep_parser(sub: _Subparsers) -> None: sp = sub.add_parser( "verify-sweep", help="Step 7.5 後段 — 最終スイープ後の未解決の指摘を検証 (0=残なし/6=残あり)", @@ -4455,9 +4614,46 @@ def build_parser() -> argparse.ArgumentParser: sp.add_argument("--file", default=None) sp.set_defaults(func=cmd_verify_sweep) + +def _add_report_parser(sub: _Subparsers) -> None: sp = sub.add_parser("report", help="Step 8 — deferred nit + サマリ表示") sp.add_argument("pr", type=int) sp.set_defaults(func=cmd_report) + + +# 副コマンドの登録関数。**`--help` の一覧はこの順で出る**ため、並びを変えない。 +_SUBCOMMAND_REGISTRARS = ( + _add_init_parser, + _add_start_round_parser, + _add_read_result_parser, + _add_unresolved_threads_parser, + _add_flush_parser, + _add_judge_parser, + _add_check_oscillation_parser, + _add_verify_findings_parser, + _add_collect_critiques_parser, + _add_merge_fix_parser, + _add_should_rotate_parser, + _add_set_current_pr_parser, + _add_verify_sweep_parser, + _add_report_parser, +) + + +def build_parser() -> argparse.ArgumentParser: + """副コマンドの引数を組み立てる。**テストが選択肢を検査できるように分ける。** + + 実機で `kiro` の結果が `invalid choice` で弾かれた。担当が 4 つの名前を取りうる + 以上、副コマンドの引数も同じ母集合を持たなければ、結果を残した担当が「結果なし」 + として扱われる。 + """ + # 副コマンドの説明は各登録関数の `help` だけが持つ。**モジュールの docstring へ写さない。** + # 2 か所へ書くと片方だけが実装から離れる。振動の検知の基準は実装が 3 つの一致へ + # 変わった後も、docstring 側が古い基準を出し続けていた（#329）。 + p = argparse.ArgumentParser(description=__doc__) + sub = p.add_subparsers(dest="cmd", required=True) + for register in _SUBCOMMAND_REGISTRARS: + register(sub) return p diff --git a/plugins/ndf/skills/cross-review/tests/test_judge_no_result_reason.py b/plugins/ndf/skills/cross-review/tests/test_judge_no_result_reason.py new file mode 100644 index 00000000..771a92e3 --- /dev/null +++ b/plugins/ndf/skills/cross-review/tests/test_judge_no_result_reason.py @@ -0,0 +1,278 @@ +"""判定が結果なしの理由を出し、起動し直せない理由があれば止めることのテスト（#729 AC14〜AC17）。 + +| ラウンドの結果なし | 出口 | 終了コード | +| --- | --- | --- | +| 理由がすべて起動し直してよいもの、まだ起動し直していない | `RELAUNCH_AGENTS` を出して起動し直す | 7 | +| 理由がすべて起動し直してよいもの、既に起動し直している | 中断（`final = error`） | 1 | +| 起動し直せない理由（利用上限）を 1 つでも含む | **起動し直さず**中断（`final = error`） | 1 | + +どの出口でも、判定は先に標準出力へ `NO_RESULT_REASONS='<担当>=<理由> ...'` の 1 行を出す。 +起動し直しの可否は結末の共通層（`monitor_outcome.relaunch_same_agent`）だけが決め、 +判定は `usage_limit` という値を知らない（AC12）。報告の表は結果なしの担当を +`<担当>=NO_RESULT(<理由>)` の形で出す（AC17）。 +""" +from __future__ import annotations + +import argparse +import json +import pathlib + +import pytest + +PR = 8729 +REPO = "o/r" +DETAIL = "Monthly request limit reached" + + +# ---------------- 足場 ---------------- + + +@pytest.fixture() +def 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 _state(rounds: list[dict], **over) -> dict: + state = { + "current_pr": PR, + "repo": REPO, + "max_rounds": 12, + "rotate_after": 8, + "only": None, + "rounds": rounds, + "pr_history": [{"pr": PR, "rounds": len(rounds)}], + "deferred_nits": [], + "final": None, + } + state.update(over) + return state + + +def _round(no: int = 1, **over) -> dict: + entry: dict = {"round": no, "pr": PR, "started_at": "2026-09-19T00:00:00+00:00"} + entry.update(over) + return entry + + +def _approve() -> dict: + return { + "intent": "APPROVE", + "posted_as": "APPROVE", + "comments": 0, + "by_severity": {"critical": 0, "major": 0}, + } + + +def _no_result(reason: str, detail: str | None = None) -> dict: + """`read-result` が残す結果なしの形（`test_read_result_reason.py` が固定する）。""" + entry: dict = { + "intent": "NO_RESULT", + "no_result_reason": reason, + "posted_as": None, + "comments": None, + "review_url": None, + "by_severity": {}, + } + if detail: + entry["monitor_detail"] = detail + return entry + + +def _write(tmp_dir: pathlib.Path, state: dict) -> None: + (tmp_dir / f"cross-review-pr{PR}-state.json").write_text(json.dumps(state)) + + +def _read(tmp_dir: pathlib.Path) -> dict: + return json.loads((tmp_dir / f"cross-review-pr{PR}-state.json").read_text()) + + +def _judge(state_mod) -> int: + with pytest.raises(SystemExit) as e: + state_mod.cmd_judge(argparse.Namespace(pr=PR)) + return int(e.value.code or 0) + + +# ---------------- AC14: 理由の行 ---------------- + + +def test_judge_prints_the_reason_of_each_agent_without_a_result(tmp_dir, state_mod, capsys): + """結果なしの担当があると、標準出力に `NO_RESULT_REASONS='<担当>=<理由> ...'` が出る。""" + _write(tmp_dir, _state([_round(codex=_no_result("timeout"), agy=_no_result("stalled"))])) + + assert _judge(state_mod) == 7 + + out = capsys.readouterr().out + assert "NO_RESULT_REASONS='codex=timeout agy=stalled'" in out + + +def test_an_agent_without_an_entry_is_reported_as_missing(tmp_dir, state_mod, capsys): + """`read-result` すら記録を残していない担当は、理由 `missing` として出る。""" + _write(tmp_dir, _state([_round(codex=_approve())])) + + assert _judge(state_mod) == 7 + + assert "NO_RESULT_REASONS='agy=missing'" in capsys.readouterr().out + + +def test_the_reason_line_comes_before_the_relaunch_line(tmp_dir, state_mod, capsys): + """理由の行は起動し直しの指示より先に出る。骨組みが読む順序を固定する。""" + _write(tmp_dir, _state([_round(codex=_approve(), agy=_no_result("cli_timeout"))])) + + assert _judge(state_mod) == 7 + + out = capsys.readouterr().out + assert out.index("NO_RESULT_REASONS=") < out.index("RELAUNCH_AGENTS=") + + +# ---------------- AC15: 起動し直せない理由があれば止める ---------------- + + +def test_a_usage_limit_stops_the_review_without_a_relaunch(tmp_dir, state_mod, capsys): + """利用上限の担当があれば、起動し直さず `final = error` で 1 で止まる。""" + _write(tmp_dir, _state([ + _round(codex=_approve(), agy=_no_result("usage_limit", DETAIL)), + ])) + + assert _judge(state_mod) == 1 + + st = _read(tmp_dir) + assert st["final"] == "error" + assert st["ended_at"] + assert st["rounds"][-1]["verdict"] == "no_result" + assert "relaunched" not in st["rounds"][-1] + captured = capsys.readouterr() + assert "NO_RESULT_REASONS='agy=usage_limit'" in captured.out + assert "RELAUNCH_AGENTS" not in captured.out + assert "agy" in captured.err + assert "usage_limit" in captured.err + assert DETAIL in captured.err + + +def test_a_usage_limit_stops_even_when_another_agent_could_be_relaunched( + tmp_dir, state_mod, capsys +): + """起動し直してよい担当が混ざっていても、利用上限が 1 つあれば誰も起動し直さない。""" + _write(tmp_dir, _state([ + _round(codex=_no_result("timeout"), agy=_no_result("usage_limit")), + ])) + + assert _judge(state_mod) == 1 + + st = _read(tmp_dir) + assert st["final"] == "error" + assert "relaunched" not in st["rounds"][-1] + captured = capsys.readouterr() + assert "NO_RESULT_REASONS='codex=timeout agy=usage_limit'" in captured.out + assert "RELAUNCH_AGENTS" not in captured.out + + +def test_a_usage_limit_without_monitor_detail_still_stops(tmp_dir, state_mod, capsys): + """`monitor_detail` の鍵が無くても、担当と理由が標準エラーに出て止まる。""" + _write(tmp_dir, _state([_round(codex=_approve(), agy=_no_result("usage_limit"))])) + + assert _judge(state_mod) == 1 + + err = capsys.readouterr().err + assert "agy" in err + assert "usage_limit" in err + + +def test_the_verdict_reads_relaunchability_from_the_common_layer( + tmp_dir, state_mod, monkeypatch, capsys +): + """可否を決めるのは共通層の集合であり、判定は理由の値を見ない（AC12）。 + + 共通層の「起動し直せない理由」に `timeout` を足すと、判定は `timeout` でも止まる。 + 判定が `usage_limit` を直に比べていれば、この変更は届かず 7 になる。 + """ + monkeypatch.setattr( + state_mod.monitor_outcome, "NO_RELAUNCH_REASONS", frozenset({"timeout"})) + _write(tmp_dir, _state([_round(codex=_approve(), agy=_no_result("timeout"))])) + + assert _judge(state_mod) == 1 + assert _read(tmp_dir)["final"] == "error" + + # 逆に `usage_limit` を外せば、判定は起動し直す + monkeypatch.setattr(state_mod.monitor_outcome, "NO_RELAUNCH_REASONS", frozenset()) + _write(tmp_dir, _state([_round(codex=_approve(), agy=_no_result("usage_limit"))])) + + assert _judge(state_mod) == 7 + assert "RELAUNCH_AGENTS='agy'" in capsys.readouterr().out + + +# ---------------- AC16: 起動し直してよい理由なら従来どおり ---------------- + + +def test_relaunchable_reasons_ask_for_one_relaunch(tmp_dir, state_mod, capsys): + """理由がすべて起動し直してよいものなら、1 度目は 7 で `RELAUNCH_AGENTS` を返す。""" + _write(tmp_dir, _state([ + _round(codex=_no_result("cli_timeout", "print timeout after 60m"), + agy=_no_result("early_error", "fatal: something")), + ])) + + assert _judge(state_mod) == 7 + + out = capsys.readouterr().out + assert "RELAUNCH_AGENTS='codex agy'" in out + assert "RELAUNCH_TARGET=both" in out + st = _read(tmp_dir) + assert st["rounds"][-1]["relaunched"] == ["codex", "agy"] + assert st["final"] is None + + +def test_a_second_no_result_with_relaunchable_reasons_stops(tmp_dir, state_mod, capsys): + """起動し直した後も結果が残らなければ、従来どおり `final = error` で 1 で止まる。""" + _write(tmp_dir, _state([ + _round(codex=_approve(), agy=_no_result("timeout"), relaunched=["agy"]), + ])) + + assert _judge(state_mod) == 1 + + st = _read(tmp_dir) + assert st["final"] == "error" + assert st["rounds"][-1]["verdict"] == "no_result" + assert "NO_RESULT_REASONS='agy=timeout'" in capsys.readouterr().out + + +def test_rounds_without_a_no_result_do_not_print_the_reason_line(tmp_dir, state_mod, capsys): + """結果なしが無いラウンドでは理由の行が出ず、終了コード 0 の枝は変わらない。""" + _write(tmp_dir, _state([_round(codex=_approve(), agy=_approve())])) + + assert _judge(state_mod) == 0 + + assert "NO_RESULT_REASONS" not in capsys.readouterr().out + assert _read(tmp_dir)["final"] == "approved" + + +# ---------------- AC17: 報告の表 ---------------- + + +def test_the_round_summary_shows_the_reason_of_a_no_result(tmp_dir, state_mod, capsys): + """報告の表で、結果なしの担当は `<担当>=NO_RESULT(<理由>)` の形で出る。""" + _write(tmp_dir, _state([ + _round(codex=_approve(), agy=_no_result("usage_limit", DETAIL), + verdict="no_result"), + ], final="error")) + + state_mod.cmd_report(argparse.Namespace(pr=PR)) + + out = capsys.readouterr().out + assert "agy=NO_RESULT(usage_limit)" in out + assert "NO_RESULT (" not in out + # 使える結果を残した担当の形は変わらない + assert "codex=APPROVE (0)" in out + + +def test_the_round_summary_shows_a_dash_when_the_reason_is_unknown( + tmp_dir, state_mod, capsys +): + """理由の鍵を持たない古い記録は `NO_RESULT(-)` として出る。""" + _write(tmp_dir, _state([ + _round(codex=_approve(), agy={"intent": "NO_RESULT"}, verdict="no_result"), + ])) + + state_mod.cmd_report(argparse.Namespace(pr=PR)) + + assert "agy=NO_RESULT(-)" in capsys.readouterr().out diff --git a/plugins/ndf/skills/cross-review/tests/test_launch_cli_process_group.py b/plugins/ndf/skills/cross-review/tests/test_launch_cli_process_group.py new file mode 100644 index 00000000..abf1e706 --- /dev/null +++ b/plugins/ndf/skills/cross-review/tests/test_launch_cli_process_group.py @@ -0,0 +1,164 @@ +"""起動の手順が CLI を独立したプロセスグループで起動し、監視がグループへ止める（#729 の AC19〜AC21、#584）。 + +偽の CLI（3 秒後に **子プロセス** が結果ファイルを書く bash）を PATH に `codex` の名前で置き、 +`launch-cli.sh` から実際に起動する。監視が pid だけを止めると子が残って結果を書く（#584 の +再現）。グループへ止めれば書かれない。 + +- AC19: 起動した CLI の pid がプロセスグループの番号（pgid = pid） +- AC20: `_kill_pid` で止めて 4 秒待っても結果ファイルが無い +- AC21: 先頭でない pid では `os.killpg` を呼ばず、pid だけへ送る +""" +from __future__ import annotations + +import os +import pathlib +import signal +import subprocess +import time +from unittest import mock + +import pytest + +_HERE = pathlib.Path(__file__).resolve().parent +_LAUNCH_CLI = _HERE.parents[2] / "scripts" / "lib" / "launch-cli.sh" + +# 標準入力（プロンプト）を読み切ってから、子プロセスが 3 秒後に結果ファイルを書く。 +# 親（この script）は `wait` で子を待つので、監視から見て生きている。子を起こした後に +# 印（`.ready`）を書き、テストは印を待ってから止める（子が居ない時点で止めると、 +# グループで止めなくても結果が書かれず、テストが判別しない）。 +FAKE_CLI = """#!/usr/bin/env bash +cat >/dev/null +( sleep 3; echo '{"event": "APPROVE"}' > "$NDF_TEST_RESULT_FILE" ) & +: > "$NDF_TEST_RESULT_FILE.ready" +wait +""" + + +@pytest.fixture() +def launched(tmp_path): + """偽の CLI を起動し、(pid, 結果ファイルのパス) を返す。終わりに残ったプロセスを片付ける。""" + bin_dir = tmp_path / "bin" + bin_dir.mkdir() + fake = bin_dir / "codex" + fake.write_text(FAKE_CLI, encoding="utf-8") + fake.chmod(0o755) + work = tmp_path / "work" + work.mkdir() + tmp_dir = tmp_path / "tmp" + tmp_dir.mkdir() + prompt = tmp_path / "prompt.md" + prompt.write_text("レビューしてください\n", encoding="utf-8") + stem = tmp_dir / "codex-review-pr7" + result = tmp_dir / "codex-review-pr7-result.json" + env = dict(os.environ, PATH=f"{bin_dir}:{os.environ['PATH']}", + NDF_TEST_RESULT_FILE=str(result)) + + proc = subprocess.run( + ["bash", str(_LAUNCH_CLI), "codex", str(work), str(prompt), str(stem)], + capture_output=True, text=True, env=env, timeout=30, + ) + assert proc.returncode == 0, proc.stderr + # 起動の案内の 1 行だけが出る（ジョブ制御の通知が混ざらない） + assert [l for l in proc.stderr.splitlines() if l.strip()] == [ + l for l in proc.stderr.splitlines() if "launched" in l], proc.stderr + pid = int((tmp_dir / "codex-review-pr7.pid").read_text().strip()) + for _ in range(500): + if (tmp_dir / "codex-review-pr7-result.json.ready").exists(): + break + time.sleep(0.01) + assert (tmp_dir / "codex-review-pr7-result.json.ready").exists(), "偽の CLI が子を起こしていない" + try: + yield pid, result + finally: + # **自分のグループへは送らない。** 実装前は CLI がこのテストと同じグループに居るため、 + # 無条件の killpg は pytest 自身を落とす（AC21 が防ぐ事故そのもの）。 + try: + if os.getpgid(pid) == pid and os.getpgid(pid) != os.getpgrp(): + os.killpg(pid, signal.SIGKILL) + else: + os.kill(pid, signal.SIGKILL) + except OSError: + pass + + +def _alive(pid: int) -> bool: + try: + os.kill(pid, 0) + except OSError: + return False + return True + + +def test_launched_cli_leads_its_own_process_group(launched): + pid, _ = launched + assert _alive(pid) + assert os.getpgid(pid) == pid # AC19 + assert os.getpgid(pid) != os.getpgrp() # 起動元（このテスト）のグループではない + + +def test_killing_the_group_prevents_the_child_from_writing_the_result(launched, monitor_mod): + pid, result = launched + assert not result.exists() + + monitor_mod._kill_pid(pid) + time.sleep(4) + + assert not result.exists() # AC20 + assert not _alive(pid) or monitor_mod._is_zombie(pid) + + +def test_non_leader_pid_is_signalled_alone(monitor_mod): + """先頭でない pid（pgid ≠ pid）は pid だけへ送り、`os.killpg` を呼ばない（AC21）。""" + with ( + mock.patch.object(monitor_mod, "_is_zombie", return_value=False), + mock.patch.object(monitor_mod, "_pid_alive", return_value=False), + mock.patch("os.getpgid", return_value=4242), + mock.patch("os.killpg") as killpg, + mock.patch("os.kill") as kill, + ): + monitor_mod._kill_pid(4243) + killpg.assert_not_called() + kill.assert_called_once_with(4243, signal.SIGTERM) + + +def test_pid_in_the_monitors_own_group_is_signalled_alone(monitor_mod): + """pgid = pid でも監視自身のグループなら `os.killpg` を呼ばない（監視まで止まる）。""" + with ( + mock.patch.object(monitor_mod, "_is_zombie", return_value=False), + mock.patch.object(monitor_mod, "_pid_alive", return_value=False), + mock.patch("os.getpgid", return_value=4242), + mock.patch("os.getpgrp", return_value=4242), + mock.patch("os.killpg") as killpg, + mock.patch("os.kill") as kill, + ): + monitor_mod._kill_pid(4242) + killpg.assert_not_called() + kill.assert_called_once_with(4242, signal.SIGTERM) + + +def test_group_leader_gets_sigterm_then_sigkill_via_killpg(monitor_mod): + with ( + mock.patch.object(monitor_mod, "_is_zombie", return_value=False), + mock.patch.object(monitor_mod, "_pid_alive", return_value=True), + mock.patch("os.getpgid", return_value=4242), + mock.patch("os.getpgrp", return_value=1), + mock.patch("os.killpg") as killpg, + mock.patch("os.kill") as kill, + ): + monitor_mod._kill_pid(4242, sigterm_grace=0.6) + assert killpg.call_args_list == [mock.call(4242, signal.SIGTERM), mock.call(4242, signal.SIGKILL)] + kill.assert_not_called() + + +def test_getpgid_failure_falls_back_to_the_pid(monitor_mod): + """`os.getpgid` が失敗する（もう居ない・権限が無い）ときは従来どおり pid だけへ送る。""" + with ( + mock.patch.object(monitor_mod, "_is_zombie", return_value=False), + mock.patch.object(monitor_mod, "_pid_alive", return_value=False), + mock.patch("os.getpgid", side_effect=ProcessLookupError), + mock.patch("os.killpg") as killpg, + mock.patch("os.kill") as kill, + ): + monitor_mod._kill_pid(4242) + killpg.assert_not_called() + kill.assert_called_once_with(4242, signal.SIGTERM) diff --git a/plugins/ndf/skills/cross-review/tests/test_monitor_usage_limit.py b/plugins/ndf/skills/cross-review/tests/test_monitor_usage_limit.py new file mode 100644 index 00000000..490bda45 --- /dev/null +++ b/plugins/ndf/skills/cross-review/tests/test_monitor_usage_limit.py @@ -0,0 +1,283 @@ +"""利用上限と CLI の上限の文言の検知（#729 の AC2〜AC7、#619）。 + +監視は PATH の偽物ではなく **実プロセス** を相手にする（`test_monitor_outcome_file.py` と +同じ形）。終わったプロセスの pid ファイルと、文言を 1 行書いた err.log / stdout.log を +置いて監視を呼び、状態・終了コード・監視の結果ファイルの `reason` を見る。 + +- AC2: `Monthly request limit reached`（kiro の実物）→ `EARLY_ERROR` / `usage_limit` +- AC3: `"api_error_status":429`（claude）は err.log（全担当）と stdout.log（claude だけ）で拾う +- AC4: 既存の `quota exceeded` / `rate limit exceeded` / `HTTP/x 429` は `usage_limit`、 + `HTTP/x 401` / `403` と他の致命は `early_error` のまま +- AC5: 結果ファイル無しで終わり err.log に `print timeout after <時間> with turn in progress` + → `NO_RESULT` / `cli_timeout`。結果ファイルがあれば `OK` / `ok` +- AC6: 表・引用・バッククォート・grep 形式の中の文言は一致しない。stdout.log の JSON は除外を掛けない +- AC7: `usage_limit` は監視の結果ファイルと記録の `reason` に入り、標準出力のキーは変わらない +- 照合の順序は利用上限 → 致命 → 警告の見た目の致命（設計文書の未確認 5 を固定する） +""" +from __future__ import annotations + +import json +import os +import pathlib +import subprocess +import sys + +import pytest + +_HERE = pathlib.Path(__file__).resolve().parent +_MONITOR_LIB = _HERE.parents[2] / "scripts" / "lib" / "monitor.py" + +STDOUT_KEYS = { + "agent", "status", "exit_code", "pid", "elapsed", "detail", "err_log_size", + "stdout_log_size", "progress_log_size", "progress_tail", "idle_seconds", + "result_exists", "sentinel_seen", +} + +# 設計文書「実測」の 10 行。見出しはそのまま、行は実物の形に合わせて組み立てた。 +KIRO_LIMIT = "Monthly request limit reached" +CLAUDE_429 = ('{"type":"result","subtype":"success","is_error":false,' + '"api_error_status":429,"result":"rate limited"}') +CLAUDE_429_SPACED = '{"type": "result", "api_error_status" : 429, "is_error": false}' +HTTP_429 = "HTTP/1.1 429 Too Many Requests" +HTTP_401 = "HTTP/1.1 401 Unauthorized" +IN_TABLE = "| usage_limit | Monthly request limit reached | 利用上限 |" +IN_BACKTICKS = "see `Monthly request limit reached` in err.log" +IN_QUOTE = "> Monthly request limit reached" +AGY_PRINT_TIMEOUT = "[agy] print timeout after 10m0s with turn in progress; returning partial output" +IN_GREP = ('plugins/ndf/skills/cross-review/tests/test_monitor_usage_limit.py:12:' + ' KIRO_LIMIT = "Monthly request limit reached"') + +# (見出し, 行, 利用上限の表の一致, CLI の上限の表の一致, 利用上限を除いた致命の表の一致) +# 設計文書の「現行fatal」は変更前の表で測った値。HTTP 429 はこの変更で利用上限の表へ移る（AC4）。 +TEN_LINES = [ + ("kiro 実物", KIRO_LIMIT, True, False, False), + ("claude 429 JSON 1 行", CLAUDE_429, True, False, False), + ("claude 429 空白あり", CLAUDE_429_SPACED, True, False, False), + ("HTTP 429 行", HTTP_429, True, False, False), + ("HTTP 401 行", HTTP_401, False, False, True), + ("表の中", IN_TABLE, False, False, False), + ("バッククォート", IN_BACKTICKS, False, False, False), + ("引用行", IN_QUOTE, False, False, False), + ("agy print timeout", AGY_PRINT_TIMEOUT, False, True, False), + ("grep 形式", IN_GREP, False, False, False), +] + + +def _write(path: pathlib.Path, text: str) -> pathlib.Path: + path.write_text(text + "\n", encoding="utf-8") + return path + + +# ---------- 照合の単体（AC4 / AC6） ---------- + +@pytest.mark.parametrize(("label", "line", "usage_hit", "cli_timeout_hit", "fatal_hit"), TEN_LINES, + ids=[t[0] for t in TEN_LINES]) +def test_ten_measured_lines_match_as_the_design_records(tmp_path, monitor_mod, label, line, + usage_hit, cli_timeout_hit, fatal_hit): + log = _write(tmp_path / "err.log", line) + assert (monitor_mod._scan_patterns(log, monitor_mod.USAGE_LIMIT_FATAL) is not None) is usage_hit + assert (monitor_mod._scan_patterns(log, monitor_mod.CLI_TIMEOUT_AFTER_EXIT) is not None) is cli_timeout_hit + assert (monitor_mod._scan_patterns(log, monitor_mod.EARLY_ERROR_FATAL) is not None) is fatal_hit + # 止めるべき文言があるかを返す `_scan_early_fatal` は、どちらの表の一致も拾う（既存テストの契約） + assert (monitor_mod._scan_early_fatal(log) is not None) is (usage_hit or fatal_hit) + + +def test_claude_stdout_json_is_matched_without_the_quote_exclusion(tmp_path, monitor_mod): + """stdout.log の JSON は 1 行に引用符が多く、行単位の除外を掛けると取りこぼす。""" + out = _write(tmp_path / "stdout.log", CLAUDE_429) + assert monitor_mod._scan_claude_stdout_usage_limit(out) is not None + assert monitor_mod._scan_claude_stdout_usage_limit(_write(tmp_path / "ok.log", '{"is_error":false}')) is None + + +@pytest.mark.parametrize("line", ["quota exceeded: please upgrade", "Rate limit exceeded for model", HTTP_429]) +def test_existing_limit_matches_moved_to_usage_limit(tmp_path, monitor_mod, line): + log = _write(tmp_path / "err.log", line) + assert monitor_mod._scan_patterns(log, monitor_mod.USAGE_LIMIT_FATAL) is not None + assert monitor_mod._scan_patterns(log, monitor_mod.EARLY_ERROR_FATAL) is None + + +@pytest.mark.parametrize("line", [HTTP_401, "HTTP/2 403 Forbidden", "Authentication failed: token", + "Internal sandbox error: cannot start"]) +def test_other_fatal_lines_stay_early_error(tmp_path, monitor_mod, line): + log = _write(tmp_path / "err.log", line) + assert monitor_mod._scan_patterns(log, monitor_mod.USAGE_LIMIT_FATAL) is None + assert monitor_mod._scan_early_fatal(log) is not None + + +# ---------- 照合の順序（利用上限 → 致命 → 警告の見た目の致命） ---------- + +def _paths(monitor_mod, tmp_path, agent="kiro"): + return monitor_mod.AgentPaths( + agent=agent, pr=7, + pidfile=tmp_path / "x.pid", err_log=tmp_path / "err.log", + stdout_log=tmp_path / "stdout.log", progress_log=tmp_path / "progress.log", + result=tmp_path / "result.json", + ) + + +@pytest.mark.parametrize("order", ["limit_first", "fatal_first"]) +def test_usage_limit_wins_over_other_fatal_lines_in_either_order(tmp_path, monitor_mod, order): + lines = [KIRO_LIMIT, "Authentication failed: retry"] + if order == "fatal_first": + lines.reverse() + _write(tmp_path / "err.log", "\n".join(lines)) + fatal, _warn = monitor_mod._early_error(_paths(monitor_mod, tmp_path), "kiro", False) + assert fatal is not None + assert fatal.reason == "usage_limit" + assert fatal.source == "err.log" + + +def test_usage_limit_wins_over_warning_shaped_fatal(tmp_path, monitor_mod): + _write(tmp_path / "err.log", + "WARNING: --trust-tools arg for custom tool foo\n" + KIRO_LIMIT) + fatal, _warn = monitor_mod._early_error(_paths(monitor_mod, tmp_path), "kiro", False) + assert fatal.reason == "usage_limit" + + +def test_fatal_without_usage_limit_has_no_reason(tmp_path, monitor_mod): + _write(tmp_path / "err.log", "Authentication failed: retry") + fatal, _warn = monitor_mod._early_error(_paths(monitor_mod, tmp_path), "kiro", False) + assert fatal.reason is None + assert fatal.message == "Authentication failed: retry" + + +def test_claude_stdout_usage_limit_is_seen_only_for_claude(tmp_path, monitor_mod): + _write(tmp_path / "stdout.log", CLAUDE_429) + fatal, _ = monitor_mod._early_error(_paths(monitor_mod, tmp_path, "claude"), "claude", False) + assert (fatal.source, fatal.reason) == ("stdout.log", "usage_limit") + fatal, _ = monitor_mod._early_error(_paths(monitor_mod, tmp_path, "kiro"), "kiro", False) + assert fatal is None + + +def test_disabled_early_error_disables_usage_limit_too(tmp_path, monitor_mod): + _write(tmp_path / "err.log", KIRO_LIMIT) + assert monitor_mod._early_error(_paths(monitor_mod, tmp_path), "kiro", True) == (None, None) + + +def test_monitor_outcome_create_keeps_the_two_argument_form(monitor_mod): + plain = monitor_mod.MonitorOutcome.create("EARLY_ERROR", "x") + assert (plain.reason, plain.exit_code) == (None, 4) + limited = monitor_mod.MonitorOutcome.create("EARLY_ERROR", "x", reason="usage_limit") + assert limited.reason == "usage_limit" + + +# ---------- 監視を実プロセスで呼ぶ（AC2 / AC3 / AC4 / AC7） ---------- + +def _dead_pid() -> int: + proc = subprocess.Popen(["true"]) + proc.wait() + return proc.pid + + +def _run_monitor(tmp_dir: pathlib.Path, agent: str, *extra: str) -> subprocess.CompletedProcess: + env = {k: v for k, v in os.environ.items() if not k.startswith("MONITOR_")} + return subprocess.run( + [sys.executable, str(_MONITOR_LIB), "7", "--agents", agent, + "--tmp-dir", str(tmp_dir), "--poll", "1", *extra], + capture_output=True, text=True, env=env, timeout=60, + ) + + +def _finished(tmp_dir: pathlib.Path, agent: str, *, err: str = "", stdout: str = "", + result: bool = False) -> str: + stem = f"{agent}-review-pr7" + (tmp_dir / f"{stem}.pid").write_text(str(_dead_pid())) + if err: + _write(tmp_dir / f"{stem}-err.log", err) + if stdout: + _write(tmp_dir / f"{stem}-stdout.log", stdout) + if result: + (tmp_dir / f"{stem}-result.json").write_text('{"event": "APPROVE"}') + return stem + + +def _outcome(tmp_dir: pathlib.Path, stem: str) -> dict: + return json.loads((tmp_dir / f"{stem}-monitor.json").read_text(encoding="utf-8")) + + +@pytest.mark.parametrize(("agent", "err", "stdout"), [ + ("kiro", KIRO_LIMIT, ""), # AC2 + ("codex", CLAUDE_429, ""), # AC3: err.log は全担当 + ("claude", "", CLAUDE_429), # AC3: stdout.log は claude だけ + ("claude", "", CLAUDE_429_SPACED), # AC3: 空白の有無を問わない + ("agy", "quota exceeded: upgrade", ""), # AC4 + ("agy", HTTP_429, ""), # AC4 +]) +def test_usage_limit_stops_the_agent_as_early_error_with_reason_usage_limit(tmp_path, agent, err, stdout): + stem = _finished(tmp_path, agent, err=err, stdout=stdout) + + proc = _run_monitor(tmp_path, agent) + + assert proc.returncode == 4, proc.stderr + outcome = _outcome(tmp_path, stem) + assert (outcome["status"], outcome["reason"]) == ("EARLY_ERROR", "usage_limit") + assert outcome["detail"].startswith("early error (fatal) in ") + rows = [json.loads(l) for l in (tmp_path / "monitor-outcomes.jsonl").read_text().splitlines()] + assert rows[-1]["reason"] == "usage_limit" + # AC7: 標準出力のキーは変わらない + out = [json.loads(l) for l in proc.stdout.splitlines() if l.strip()] + assert len(out) == 1 and set(out[0]) == STDOUT_KEYS and out[0]["exit_code"] == 4 + + +@pytest.mark.parametrize("err", [HTTP_401, "HTTP/1.1 403 Forbidden", "Permission denied"]) +def test_other_fatal_lines_keep_reason_early_error(tmp_path, err): + stem = _finished(tmp_path, "kiro", err=err) + + proc = _run_monitor(tmp_path, "kiro") + + assert proc.returncode == 4, proc.stderr + assert (_outcome(tmp_path, stem)["status"], _outcome(tmp_path, stem)["reason"]) == ( + "EARLY_ERROR", "early_error") + + +def test_claude_json_in_kiro_stdout_is_not_seen(tmp_path): + """stdout.log を見るのは claude だけ。他の担当の stdout は結果なしのまま。""" + stem = _finished(tmp_path, "kiro", stdout=CLAUDE_429) + + proc = _run_monitor(tmp_path, "kiro") + + assert proc.returncode == 3 + assert _outcome(tmp_path, stem)["reason"] == "missing" + + +def test_quoted_usage_limit_in_err_log_is_not_a_hit(tmp_path): + stem = _finished(tmp_path, "kiro", err="\n".join([IN_TABLE, IN_BACKTICKS, IN_QUOTE, IN_GREP]), + result=True) + + proc = _run_monitor(tmp_path, "kiro") + + assert proc.returncode == 0, proc.stderr + assert _outcome(tmp_path, stem)["reason"] == "ok" + + +# ---------- CLI の上限（AC5） ---------- + +def test_cli_timeout_without_result_is_no_result_with_reason_cli_timeout(tmp_path): + stem = _finished(tmp_path, "agy", err=AGY_PRINT_TIMEOUT) + + proc = _run_monitor(tmp_path, "agy") + + assert proc.returncode == 3, proc.stderr + outcome = _outcome(tmp_path, stem) + assert (outcome["status"], outcome["reason"]) == ("NO_RESULT", "cli_timeout") + rows = [json.loads(l) for l in (tmp_path / "monitor-outcomes.jsonl").read_text().splitlines()] + assert rows[-1]["reason"] == "cli_timeout" + + +def test_cli_timeout_with_result_is_still_ok(tmp_path): + """上限に当たっても結果を書き終えていれば使える。""" + stem = _finished(tmp_path, "agy", err=AGY_PRINT_TIMEOUT, result=True) + + proc = _run_monitor(tmp_path, "agy") + + assert proc.returncode == 0, proc.stderr + assert (_outcome(tmp_path, stem)["status"], _outcome(tmp_path, stem)["reason"]) == ("OK", "ok") + + +def test_cli_timeout_in_a_table_row_is_not_a_hit(tmp_path): + stem = _finished(tmp_path, "agy", err=f"| cli_timeout | {AGY_PRINT_TIMEOUT} |") + + proc = _run_monitor(tmp_path, "agy") + + assert proc.returncode == 3 + assert _outcome(tmp_path, stem)["reason"] == "missing" diff --git a/plugins/ndf/skills/cross-review/tests/test_read_result_reason.py b/plugins/ndf/skills/cross-review/tests/test_read_result_reason.py new file mode 100644 index 00000000..2e1ee0b6 --- /dev/null +++ b/plugins/ndf/skills/cross-review/tests/test_read_result_reason.py @@ -0,0 +1,236 @@ +"""結果の取り込みが共通の結末を読み、理由と監視の詳細を残すことのテスト（#729 AC13）。 + +`read-result` は結果ファイルを自前で開かず、結末の共通層（`monitor_outcome`）の +`read_launch_outcome` を呼ぶ。使える結果が無いとき、`no_result_reason` に共通の `reason` +を残し、監視の結果ファイルがあれば `monitor_detail` に監視の `detail` を残す。 +終了コードは変更前と同じ（`unparsable` は 3、それ以外は 1）である。 + +| 監視の結果ファイル | 結果ファイル | `no_result_reason` | `monitor_detail` | 終了コード | +| --- | --- | --- | --- | --- | +| 監視が決めた理由（6 語） | 無い | その値 | 監視の `detail` | 1 | +| 監視が決めた理由（6 語） | 読めない | その値 | 監視の `detail` | 1 | +| 無い | 無い | `missing` | 鍵なし | 1 | +| 無い | 読めない | `unparsable` | 鍵なし | 3 | +| `ok` / `missing` | 無い | `missing` | 監視の `detail`（空なら鍵なし） | 1 | +| 問わない | 読める | （使える結果として取り込む） | — | 0 | + +stem の突き合わせ（設計文書の未確認 6）もここで固定する。起動の手順・監視・取り込みの +3 つが同じ形の stem を組み立てなければ、監視の結果ファイルを引けない。 +""" +from __future__ import annotations + +import argparse +import json +import pathlib +import re + +import pytest + +PR = 7729 +AGENT = "kiro" +STEM = f"{AGENT}-review-pr{PR}" +DETAIL = "Monthly request limit reached" + +_LAUNCHER = pathlib.Path(__file__).resolve().parent.parent / "scripts" / "launch-reviewer.sh" + +# 監視が結末に書く理由のうち、結果ファイルの状態を見ずにその値を採るもの（設計文書の +# 「理由の語彙」の表で「監視」が書き、`ok` / `missing` でない 6 語）。 +MONITOR_DECIDED = ("timeout", "stalled", "early_error", "usage_limit", "cli_timeout", "pidfile_bad") + + +# ---------------- 足場 ---------------- + + +@pytest.fixture() +def 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 + + +@pytest.fixture(autouse=True) +def review_posted(monkeypatch, state_mod): + """投稿の実在確認は届いた前提にする。ここで見るのは結末の読み方である。""" + monkeypatch.setattr(state_mod, "_review_exists", lambda repo, pr, url: True) + + +def _seed_state(tmp_dir: pathlib.Path) -> None: + state = { + "current_pr": PR, + "repo": "o/r", + "rounds": [{"round": 1, "pr": PR, "started_at": "2026-09-19T00:00:00+00:00"}], + "final": None, + } + (tmp_dir / f"cross-review-pr{PR}-state.json").write_text(json.dumps(state)) + + +def _entry(tmp_dir: pathlib.Path) -> dict: + st = json.loads((tmp_dir / f"cross-review-pr{PR}-state.json").read_text()) + return st["rounds"][-1][AGENT] + + +def _write_monitor(tmp_dir: pathlib.Path, reason: str, detail: str = DETAIL) -> None: + """監視の結果ファイル `-monitor.json` を置く。キーは監視が書く形に倣う。""" + outcome = { + "agent": AGENT, + "stem": STEM, + "status": "EARLY_ERROR", + "exit_code": 4, + "reason": reason, + "detail": detail, + } + (tmp_dir / f"{STEM}-monitor.json").write_text(json.dumps(outcome, ensure_ascii=False)) + + +def _read_result(state_mod, rfile: pathlib.Path | None) -> int: + """`read-result` を呼び、終了コードを返す。`rfile` が None なら既定のパスを使う。""" + args = argparse.Namespace(pr=PR, agent=AGENT, file=str(rfile) if rfile else None) + with pytest.raises(SystemExit) as e: + state_mod.cmd_read_result(args) + return int(e.value.code or 0) + + +# ---------------- AC13: 理由と監視の詳細を残す ---------------- + + +@pytest.mark.parametrize("reason", MONITOR_DECIDED) +def test_the_monitor_reason_is_recorded_when_no_result_file_exists( + tmp_dir, state_mod, reason, capsys +): + """監視が理由を決めていれば、結果ファイルが無いときその理由と詳細が残る。""" + _seed_state(tmp_dir) + _write_monitor(tmp_dir, reason) + + assert _read_result(state_mod, tmp_dir / "absent-result.json") == 1 + + entry = _entry(tmp_dir) + assert entry["intent"] == "NO_RESULT" + assert entry["no_result_reason"] == reason + assert entry["monitor_detail"] == DETAIL + err = capsys.readouterr().err + assert reason in err + assert DETAIL in err + + +def test_the_monitor_reason_wins_over_an_unreadable_result_file(tmp_dir, state_mod): + """監視が止めたと分かっているなら、読めない結果ファイルがあってもその理由で 1 で止まる。""" + _seed_state(tmp_dir) + _write_monitor(tmp_dir, "usage_limit") + rfile = tmp_dir / "result.json" + rfile.write_text("{ not json") + + assert _read_result(state_mod, rfile) == 1 + + entry = _entry(tmp_dir) + assert entry["no_result_reason"] == "usage_limit" + assert entry["monitor_detail"] == DETAIL + + +def test_without_a_monitor_file_the_reason_is_missing_and_no_detail_key_is_written( + tmp_dir, state_mod +): + """監視の結果ファイルが無ければ `missing` で、`monitor_detail` の鍵そのものが無い。""" + _seed_state(tmp_dir) + + assert _read_result(state_mod, tmp_dir / "absent-result.json") == 1 + + entry = _entry(tmp_dir) + assert entry["no_result_reason"] == "missing" + assert "monitor_detail" not in entry + + +def test_an_unparsable_result_without_a_monitor_file_keeps_exit_code_3(tmp_dir, state_mod): + """監視の結果ファイルが無く結果が読めなければ、`unparsable` で従来どおり 3 で止まる。""" + _seed_state(tmp_dir) + rfile = tmp_dir / "result.json" + rfile.write_text(json.dumps([{"event": "APPROVE"}])) + + assert _read_result(state_mod, rfile) == 3 + + entry = _entry(tmp_dir) + assert entry["no_result_reason"] == "unparsable" + assert "monitor_detail" not in entry + + +@pytest.mark.parametrize("reason", ("ok", "missing")) +def test_a_monitor_that_did_not_decide_falls_back_to_the_result_file( + tmp_dir, state_mod, reason +): + """監視が `ok` / `missing` なら理由は結果ファイルの側で決まり、詳細だけ残る。""" + _seed_state(tmp_dir) + _write_monitor(tmp_dir, reason, detail="exit 0") + + assert _read_result(state_mod, tmp_dir / "absent-result.json") == 1 + + entry = _entry(tmp_dir) + assert entry["no_result_reason"] == "missing" + assert entry["monitor_detail"] == "exit 0" + + +def test_an_empty_monitor_detail_does_not_write_the_key(tmp_dir, state_mod): + """監視の結果ファイルがあっても `detail` が空なら、`monitor_detail` を書かない。""" + _seed_state(tmp_dir) + _write_monitor(tmp_dir, "timeout", detail="") + + assert _read_result(state_mod, tmp_dir / "absent-result.json") == 1 + + entry = _entry(tmp_dir) + assert entry["no_result_reason"] == "timeout" + assert "monitor_detail" not in entry + + +def test_a_readable_result_file_wins_over_the_monitor_reason(tmp_dir, state_mod): + """監視が利用上限で止めた後でも、結果ファイルが読めればそれは使える結果である。""" + _seed_state(tmp_dir) + _write_monitor(tmp_dir, "usage_limit") + rfile = tmp_dir / "result.json" + rfile.write_text(json.dumps({ + "event": "APPROVE", "posted_as": "APPROVE", "comments_count": 0, + "review_url": "https://example/pr/1#1", "by_severity": {}, + })) + + state_mod.cmd_read_result(argparse.Namespace(pr=PR, agent=AGENT, file=str(rfile))) + + entry = _entry(tmp_dir) + assert entry["intent"] == "APPROVE" + assert "no_result_reason" not in entry + + +def test_no_verdict_still_overrides_after_the_common_read(tmp_dir, state_mod): + """判定の値が無い結果は、共通の読み取りの後で cross-review が `no_verdict` に上書きする。""" + _seed_state(tmp_dir) + _write_monitor(tmp_dir, "ok", detail="exit 0") + rfile = tmp_dir / "result.json" + rfile.write_text(json.dumps({"comments_count": 0})) + + assert _read_result(state_mod, rfile) == 1 + assert _entry(tmp_dir)["no_result_reason"] == "no_verdict" + + +# ---------------- 未確認 6: stem の突き合わせ ---------------- + + +def _launcher_stem_pattern() -> str: + """`launch-reviewer.sh` の `STEM=` の行から、ディレクトリを除いた stem の形を取る。""" + lines = [l for l in _LAUNCHER.read_text(encoding="utf-8").splitlines() + if re.match(r"^\s*STEM=", l)] + assert len(lines) == 1, lines + value = lines[0].split("=", 1)[1].strip() + assert value.startswith("$TMP_DIR/"), value + return value[len("$TMP_DIR/"):] + + +def test_the_three_stems_have_the_same_shape(tmp_dir, state_mod, monitor_mod): + """起動の手順・監視・取り込みの stem が同じ形で、監視の結果ファイルを引ける。""" + # 起動の手順（bash）: 変数名を置き換えて形を比べる + launcher = (_launcher_stem_pattern() + .replace("$RUNTIME", AGENT).replace("$STATE_PR", str(PR))) + # 監視（Python）: 既定の stem テンプレート + monitor = monitor_mod.DEFAULT_STEM_TEMPLATE.format(agent=AGENT, id=PR) + assert launcher == monitor == STEM + + # 取り込み: 既定のパス（`--file` 無し）で、監視の結果ファイルの理由を引ける + _seed_state(tmp_dir) + _write_monitor(tmp_dir, "usage_limit") + assert _read_result(state_mod, None) == 1 + assert _entry(tmp_dir)["no_result_reason"] == "usage_limit"