diff --git a/issues/issue-728-647-592-553-design.md b/issues/issue-728-647-592-553-design.md index b7e4edb7..709e5411 100644 --- a/issues/issue-728-647-592-553-design.md +++ b/issues/issue-728-647-592-553-design.md @@ -525,11 +525,12 @@ graph TD | # | 項目 | 内容 | 決める時点 | | --- | --- | --- | --- | | 1 | G3 の実装の形 | `LaunchOutcome` の欄の名前は PR #781 の設計のとおりとしている。実装で変われば `read_result` の包みが吸収し、取り込みは変わらない | G3 の実装 Pull Request のマージ | -| 2 | G1 の先後 | `impl_for_seq` の中身が `assignment.assign` か `impl_assign(participants, seq)` かは、実装の着手時点の `develop` で決める | 実装の計画 | +| 2 | G1 の先後 | **決まった。** 実装の着手時点の開発版の起点に参加者の決め方の変更は入っていなかった（`git grep impl_assign` は設計文書だけに当たる）。輪番から担当を引く関数は現行の割り当て（`assignment.assign(seq, host)`）を包む形で書いた。後から入る側がその中だけを差し替える | 決定済み | | 3 | 担当を替えた後の担当も結果を残さない割合 | 2 回目で救える群の数は測っていない。実行の要約の `apply_attempts` で数える | 配布後 | | 4 | トレーラーの形でない署名を末尾に足すランタイム | codex / agy / kiro のコミットで帰属行の段落を見ていない。散文の段落を足す者が現れれば決定 13 は効かない | 次の実行の `failed` の理由を読む | | 5 | 帰属行を同じ段落に続ける指示に claude が従うか | 決定 14 は補助で、従わなくても決定 13 で検証は通る | 実装後の最初の実行 | | 6 | 担当が雛形の進捗マーカーに従うか | 従わなくても決定 15 の許容で打ち切られないのはテスト 1 回分まで | 実装後の最初の実行 | +| 8 | 適用の説明の行数 | 結果なしの節を足したことで行数の上限（500 行）に達したため、改修計画の節を報告の説明へ移した。次に節を足すときは分割が要る | 次に適用の説明を書き足すとき | | 7 | 修正の担当が利用上限のとき、群の他の項目を救う手段 | 決定 10 は修正を見送りへ進める。担当を替えて修正を続ける形は、直しかけの文脈が要るため採らなかった。見送りが増えれば見直す | 配布後 | ## 申し送り（並行する設計との境界） diff --git a/issues/issue-728-647-592-553-plan.md b/issues/issue-728-647-592-553-plan.md new file mode 100644 index 00000000..eb4ffa65 --- /dev/null +++ b/issues/issue-728-647-592-553-plan.md @@ -0,0 +1,205 @@ +# cross-refactoring: 実装担当が結果を残さないと同じ群が上限なしに開き直され、未検証のコミットが残る → 結果なしを取り込みの 1 か所で受けて取り消し、群が開いた回数と結末で開き直しを決める（実装計画 / #728 #647 #592 #553） + +## 関連リンク + +| 文書 | 何を持つか | +| --- | --- | +| [issue-728-647-592-553-requirements.md](issue-728-647-592-553-requirements.md) | 受け入れ条件 50 件（AC1〜AC50） | +| [issue-728-647-592-553-design.md](issue-728-647-592-553-design.md) | 決定 15 件、入出力の契約、テスト設計 | +| [issue-647-592-553-design.md](issue-647-592-553-design.md) | 置き換えられる既存の設計 | +| 課題 | #728（親）/ #647 / #592 / #553 / #674（最終ゲートの修正。閉じるのは棚卸に任せる） | + +**用語は設計文書の「用語の対応」の表で引く。** この計画は業務用語で書き、識別子はコードブロックと表にだけ置く。 + +## モード + +`standard`。公開しているコマンドの終了コードの意味と、状態ファイルの構造が変わる。複数のモジュールにまたがる。 + +## 目的と非目的 + +達成したい状態: + +- 担当の作業結果が残っていなくても、3 つの取り込み（適用・修正・最終ゲートの修正）が同じ手順で受け、未検証のコミットを取り消し、結末を記録して終わる +- 同じ改善項目の集まりを開き直す回数が 2 回で止まり、2 回目は別の担当が試す +- 採用が 0 件だった提案ラウンドと、項目が 1 件も無い集まりでは担当を起動しない +- 実行環境が帰属行を足したコミットでも、必須の記名（トレーラー）が読める + +やらないこと: + +- 監視そのもの（`plugins/ndf/scripts/lib/monitor.py`）の変更。結末の読み取りの共通層は別の課題（#729）が入れ終えている +- 参加者の決め方の変更（#727）。輪番から担当を引く 1 つの関数に寄せるところまでを行う +- 記録の集計（実行の要約の新しい欄）。契約だけを合わせ、集計は別の課題が行う + +## 前提 + +- 前提 1: 結末の読み取りの共通層（`plugins/ndf/scripts/lib/monitor_outcome.py`）は開発版の起点に入っている。`read_launch_outcome(tmp_dir, stem, result_path=None)` が 5 つの欄を持つ値を返し、同じ担当を起動し直せない理由は利用上限（`usage_limit`）の 1 語だけである +- 前提 2: 参加者の決め方の変更（#727）は開発版の起点に入っていない。輪番から担当を引く関数の中身は、現行の割り当て（`assignment.assign(seq, host)`）を包む形で書く。後から入る側がその中だけを差し替える +- 前提 3: 同じ提案ラウンドの別の変更（#727 の後続）が、担当の割り当ての 2 行（`commands/apply.py:134` と `commands/gate.py:128`）を触る。後からマージする側が輪番から担当を引く関数の中身を揃える + +## 実測 + +この計画のために実行して確かめた値である。 + +| 見たもの | 値 | +| --- | --- | +| 変更前のテスト | `uv run --with pytest pytest plugins/ndf/skills/cross-refactoring/tests -q` が 704 件成功・終了コード 0（31.8 秒） | +| 記名の解析に題名が要るか | **要る。** 記名だけの段落を単独で `git interpret-trailers --parse` へ渡すと出力が空になる。題名の行と空行を前に付けると 4 つとも返る（git 2.53.0） | +| 散文と記名が混ざる段落 | 出力は空。題名を付けても同じ | +| 題名の位置に記名の形の行を置いた場合 | その行が記名として返る。**先頭の段落を解析に掛けないことで避ける** | +| 解析にリポジトリが要るか | 要らない。リポジトリの外の現在地でも終了コード 0 で動く | + +## 受け入れ条件 + +要求の文書の AC1〜AC50 をそのまま用いる。この計画では各タスクが満たす番号だけを示す。検証手段は要求の文書の「検証手段」と設計文書の「テスト設計」が持つ。 + +## 代替案と採否 + +| 案 | 内容 | 採否 | 理由 | +| --- | --- | --- | --- | +| 結末の読み取りを包みとして残す | 名前を残し、引数を状態と工程に変える | 採用 | 結果ファイルの名前の幹を組む場所が 1 つになる（設計の決定 3） | +| 取り込みが共通層を直に呼ぶ | 包みを置かない | 不採用 | 名前の幹の組み立てが 3 か所に分かれる | +| 記名を進行側が付け直す | 取り込みでコミットを書き換える | 不採用 | コミットの識別子が変わり、申告と実体の対応が切れる（設計の決定 13） | + +## ドメイン用語 + +設計文書の「用語の対応」の表を正本とする。この計画で追加する語は無い。 + +## 不変条件 + +- 検証を受けていないコミットを、公開したまま次の工程へ渡さない +- 取り消した後の作業ツリーの先端が、次の範囲の起点になる +- 同じ工程・同じ試行番号の結末は、記録に 1 件しか入らない + +## 互換性 + +| 対象 | 変更 | 互換性の扱い | +| --- | --- | --- | +| 適用の取り込みの終了コード | 着手前テストが成功と確認できていない場合が 2 から 4 へ。2 の意味に「担当を替えて開き直す」が加わる | 骨組みの分岐は変えない（2 で次の群へ進む、4 で止まる、のどちらも既存の扱い） | +| 修正・最終ゲートの修正の取り込みの終了コード | 結果なしで 2 を返す場合が増える | 骨組みは両者の終了コードを見ない | +| 結末の読み取りの引数 | ファイルのパスと担当 → 状態・担当・工程・提案ラウンド | 内部の関数。古い形の呼び出しは実行時に失敗する | +| 状態ファイル | 集まりの側に試行番号・結末の記録・取り消しの理由、最終ゲートの記録に結末の記録を足す | 版を上げない。鍵が無い状態ファイルは試行 0・失敗なしとして読む | +| 取り消しの本体 | 事実の読み取りの層にあったものを取り込みの共通手順へ移す | 内部の関数。呼び出し元 2 か所を同じ変更で書き換える | + +## 修正対象 + +```text +plugins/ndf/skills/cross-refactoring/ +├── SKILL.md +├── docs/02-apply-and-review.md +├── docs/04-fix-and-report.md +├── prompts/{apply,fix,final-fix}.md +├── scripts/refactor_lib/ +│ ├── intake.py # 新設 +│ ├── gitfacts.py +│ ├── rounds.py +│ ├── vocabulary.py +│ └── commands/{apply,converge,gate,setup}.py +└── tests/ + ├── test_intake.py # 新設 + ├── test_apply_attempts.py # 新設 + ├── test_commit_trailers_git.py # 新設 + └── test_{git_facts,merge_apply,apply_rounds,abandon_items,final_fix,skill_terms,init}.py +``` + +配布物（`plugins/ndf/dev.kiro/` と `plugins/ndf/dev.agy/`）は `bash scripts/build-runtime-plugins.sh` で揃える。 + +## タスク分解 + +### Task 1: 起動の結末を値で受け取る + +- **対象ファイル:** `scripts/refactor_lib/gitfacts.py`、`tests/test_intake.py`（新設）、`tests/test_git_facts.py`、`tests/test_skill_terms.py` +- **変更内容:** 結末の読み取り（`read_result`）を、状態・担当・工程・提案ラウンドから結果ファイルの名前の幹を組み、共通層の読み取りを呼んで値を返す包みにする。中断も画面への出力も行わない。名前の幹を組む関数（`paths.stem_for`）の値と、手順書が監視へ渡す名前の雛形（`--stem-template`）を担当名で埋めた値の一致を確かめる +- **満たす受け入れ条件:** AC1、AC2、AC3 +- **進め方:** 一時ディレクトリに結果ファイルを置かない状態・監視の結果ファイルだけを置いた状態でそれぞれ失敗するテストを書き、包みを差し替えて通す。既存の 3 件（中断を現状固定していたもの）は、値を返す形へ書き直す + +### Task 2: 取り込みの共通手順を新設し、取り消しを 1 つにする + +- **対象ファイル:** `scripts/refactor_lib/intake.py`（新設）、`gitfacts.py`、`commands/{apply,converge,gate}.py`、`tests/test_intake.py` +- **変更内容:** 範囲の確定・未検証のコミットの取り消し・叩き直しの判定・結果なしの一連を新しいモジュールへ置く。取り込み 1 つ分の「どこを見て、どこへ書くか」は範囲の値（`IntakeScope`）で渡す。事実の読み取りの層にあった取り消し（`gitfacts.revert_unverified_range`）を削除し、修正・最終ゲート・適用の 3 か所を新しい取り消しへ寄せる。試し打ちの対応（`--dry-run`）は取り消しの引数で受ける +- **満たす受け入れ条件:** AC4、AC5、AC6、AC7、AC8、AC50 +- **進め方:** 3 つの取り込みの形の範囲の値を作り、範囲が 1 件・0 件・確定できない場合で失敗するテストを書いてから実装する。取り消しと公開の順序は、既存のテストが見ている記録（外部コマンドの呼び出し列）で確かめる + +### Task 3: 同じ集まりの試行を 2 回で止め、2 回目は別の担当が試す + +- **対象ファイル:** `scripts/refactor_lib/rounds.py`、`vocabulary.py`、`commands/apply.py`、`tests/test_apply_attempts.py`（新設）、`tests/test_merge_apply.py` +- **変更内容:** 開き直しの判定（`rounds.group_reopening`）と、輪番から担当を引く関数（`rounds.impl_for_seq`）を新設する。集まりを開く側と適用の取り込みの両方が同じ判定を読む。結果なしのときは共通手順で取り消しと記録を行い、次の輪番の担当のうち、その集まりで失敗した担当のどれとも違う最初の担当へ替える。替える先が無いときだけ、起動し直しの可否で続けるか取り消すかを決める。着手前テストの確認を結果の読み取りより前へ移し、終了コードを 4 にする +- **満たす受け入れ条件:** AC9、AC10、AC11、AC12、AC13、AC14、AC15、AC16、AC17、AC18、AC49（3 つのうち 2 つ） +- **進め方:** 集まりが 2 つ・4 つの状態で、結果ファイルを置かずに開く → 取り込む、を繰り返す失敗するテストを先に書く。担当の交代は、輪番から担当を引く関数を差し替えて確かめる + +### Task 4: 採用が 0 件の提案ラウンドと、項目の無い集まりで担当を起動しない + +- **対象ファイル:** `scripts/refactor_lib/rounds.py`、`commands/apply.py`、`tests/test_apply_rounds.py` +- **変更内容:** 集まりの一覧を返す関数で、鍵が無いときだけ古い版として 1 つ作り、空の配列はそのまま返す。集まりが 1 つも無い状態で進行中の集まりを引くと中断する。開き直しの判定が「項目が無い」を返した集まりは、開かずに取り消し済みにして次を探す。取り込み済みで採用が 0 件だった集まりも取り消し済みに直す +- **満たす受け入れ条件:** AC19、AC20、AC21、AC22 +- **進め方:** 採用 0 件からの取り込み → 開く、古い形の状態ファイル、項目の無い集まりの 3 通りで失敗するテストを書いてから実装する + +### Task 5: 修正の結果が無いときに修正ラウンドを進める + +- **対象ファイル:** `scripts/refactor_lib/commands/converge.py`、`tests/test_abandon_items.py` +- **変更内容:** 修正の結果を、提案ラウンドの担当ではなく集まりの担当から読む。結果が無ければ共通手順で取り消しと記録を行い、修正ラウンドの数を 1 進めて終了コード 2 で終える。起動し直せない結末では、修正ラウンドの数を上限の値にして見送りの判定へ渡す +- **満たす受け入れ条件:** AC23、AC24、AC25、AC26、AC27 +- **進め方:** 集まりの担当と提案ラウンドの担当を分けた状態で失敗するテストを書く。上限まで続けた後に見送りの判定が 0 を返すところまで通す + +### Task 6: 最終ゲートの修正の結果が無いときにコミットを取り消す + +- **対象ファイル:** `scripts/refactor_lib/commands/gate.py`、`tests/test_final_fix.py` +- **変更内容:** 最終ゲートの修正の取り込みを共通手順へ通す。結果が無ければ取り消して起点を取り消し後の先端へ進め、終了コード 2 で最終ゲートへ判定を戻す。起動し直せない結末では最終ゲートの修正ラウンドの数を上限の値にする。最終ゲートの修正担当の決定を、輪番から担当を引く関数へ寄せる +- **満たす受け入れ条件:** AC28、AC29、AC30、AC31、AC49（残り 1 つ） +- **進め方:** 最終ゲートの記録に起点と担当を置き、結果ファイルを置かずに取り込む → 判定する、の順で失敗するテストを書いてから実装する + +### Task 7: 帰属行の後ろでも記名を読む + +- **対象ファイル:** `scripts/refactor_lib/gitfacts.py`、`prompts/{apply,fix}.md`、`tests/test_commit_trailers_git.py`（新設）、`tests/test_merge_apply.py` +- **変更内容:** コミットのメッセージを空行で段落に分け、末尾の段落から前へ 1 段落ずつ git の解析へ掛ける。解析が記名の段落と判定しなかったところで止める。**先頭の段落（題名）は掛けない。** 解析には題名の行を補って渡す（実測のとおり、補わないと何も返らない）。同じ鍵は末尾に近い段落の値を採る。雛形のコミットの規約に、必須の記名を最後の段落へ置くことと、実行環境が帰属行を足すときは空行を挟まず同じ段落に続けることを書く +- **満たす受け入れ条件:** AC32、AC33、AC34、AC35、AC36、AC37、AC38、AC39 +- **進め方:** 一時リポジトリに各形のコミットを作り、実際に git を実行して確かめる失敗するテストを先に書く + +### Task 8: テストの実行中の無出力で担当を打ち切らない + +- **対象ファイル:** `scripts/refactor_lib/vocabulary.py`、`commands/setup.py`、`SKILL.md`、`prompts/{apply,fix,final-fix}.md`、`tests/test_init.py`、`tests/test_skill_terms.py` +- **変更内容:** 起動の出力に無進捗の許容を足す。値はテストの制限時間に 900 秒を加えたものとする。手順書の骨組みで、適用・修正・最終ゲートの修正の 3 つの監視の呼び出しにこの値を渡す。3 つの雛形に、作業段階が進むたびに進捗の記録へ 1 行足す指示を書く +- **満たす受け入れ条件:** AC40、AC41、AC42 +- **進め方:** 起動の出力と手順書の骨組みを読む失敗するテストを先に書く + +### Task 9: 手順書と説明文書を実装に合わせる + +- **対象ファイル:** `SKILL.md`、`docs/02-apply-and-review.md`、`docs/04-fix-and-report.md`、`tests/test_skill_terms.py` +- **変更内容:** 使う語の表の適用ラウンドの行に、同じ集まりの試行の上限（2 回）を書く。上限を 1 つに保つとしていた段落を外す。適用の工程と修正・最終ゲートの工程の説明に、結果が無いときの取り消し・記録・終了コードを書く。記名の節に、git の標準の読み方が最後の段落しか読まないことと、進行側の読み方の 2 つを書く +- **満たす受け入れ条件:** AC43、AC44、AC45 +- **進め方:** 文言を読むテストを先に書く。記名の節の記載（AC45）は Pull Request のレビューで見る + +### Task 10: 退行の確認と配布物の同期 + +- **対象ファイル:** `tests/test_merge_apply.py`、配布物一式 +- **変更内容:** 結果があり検証を通る適用が、変更前と同じく 1 回目の試行で取り込まれ、結末の記録を持たないことを既存のテストへ足す。配布物を同期し、定義と frontmatter の検査を通す +- **満たす受け入れ条件:** AC46、AC47、AC48 +- **進め方:** 全体のテスト → 同期 → 3 つの検査の順で実行し、結果を Pull Request 本文へ載せる + +## 影響範囲 + +| 影響を受けるもの | 何が変わるか | +| --- | --- | +| 収束リファクタリングの骨組み | 監視の呼び出しに引数が 1 つ増える。分岐は変えない | +| 進行中の実行の状態ファイル | 新しい鍵は無くても読める。項目の無い集まりを持つ既存の状態は、開こうとした時点で取り消し済みになる | +| 実行の要約と改修計画 | 見送りの理由に、どの担当がどの理由で結果を残さなかったかが並ぶ | +| 収束レビューの共通層 | 読むだけで変えない | + +## リスクと対処 + +| リスク | 対処 | +| --- | --- | +| 適用の取り込み（893 行）が 1 つの関数で結果の読み取り・検証・取り消し・記録を抱えており、分岐を足すと読めなくなる | **先に構造を整える。** Task 2 で取り消しを共通手順へ出し、Task 3 の分岐はそこへ委ねる。新しい分岐を既存の関数へ直接足さない | +| 適用の検証のテスト（1735 行）が終了コードと状態を細かく固定しており、変更が広範囲の失敗として現れる | タスクごとにテストを通す。失敗したテストは、期待値が変わったものと退行とを 1 件ずつ切り分けて記録する | +| 記名の解析が git の版で変わる | 実測した版（2.53.0）を計画へ残し、一時リポジトリで実際に git を実行するテストで固定する | +| 担当の割り当ての 2 行を別の変更が触る | 輪番から担当を引く関数の中だけに割り当ての呼び出しを置く。後からマージする側がその中身を揃える | + +## 切り戻し手順 + +Pull Request 単位で戻せる。状態ファイルの版を上げないため、途中まで進んだ実行の状態は変更前のコードでもそのまま読める。足した鍵は読まれずに残るだけである。 + +## 完了の定義 + +- [ ] 受け入れ条件 AC1〜AC50 について、条件ごとに検証手段と結果が対応している +- [ ] `uv run --with pytest pytest scripts/tests plugins/ndf -q` が通る +- [ ] `bash scripts/build-runtime-plugins.sh --check` / `claude plugin validate .` / `python3 scripts/check-skill-frontmatter.py` が終了コード 0 で終わる +- [ ] 手順書と説明文書が、実装した振る舞いと同じことを書いている diff --git a/issues/issue-728-647-592-553-requirements.md b/issues/issue-728-647-592-553-requirements.md index 19079b0a..b2fac640 100644 --- a/issues/issue-728-647-592-553-requirements.md +++ b/issues/issue-728-647-592-553-requirements.md @@ -125,94 +125,94 @@ ## 受け入れ条件（結末の読み取り） -- [ ] AC1: 結果ファイルが無い状態で結末の読み取り（`gitfacts.read_result`）を呼ぶと、例外（`SystemExit`）を出さず、標準出力・標準エラーに書かない。結果なしの値（`payload` が `None`、`reason` が `missing`）を返す -- [ ] AC2: 監視の結果ファイル（`-apply-r-monitor.json`）に無進捗の理由（`reason: stalled`）があるとき、結末の読み取りの理由は `stalled`、起動し直しの可否は真である。利用上限の理由（`reason: usage_limit`）のとき、可否は偽である -- [ ] AC3: 結末の読み取りに渡す結果ファイルの名前の幹（stem）は、監視の名前の雛形（`--stem-template`: `{agent}-apply-r$ROUND` / `{agent}-fix-r$ROUND` / `{agent}-final-fix`）を担当名で埋めた値と一致する。幹を組む関数（`paths.stem_for`）の 3 つの工程の値を、骨組みの雛形から作った値と突き合わせる +- [x] AC1: 結果ファイルが無い状態で結末の読み取り（`gitfacts.read_result`）を呼ぶと、例外（`SystemExit`）を出さず、標準出力・標準エラーに書かない。結果なしの値（`payload` が `None`、`reason` が `missing`）を返す +- [x] AC2: 監視の結果ファイル（`-apply-r-monitor.json`）に無進捗の理由（`reason: stalled`）があるとき、結末の読み取りの理由は `stalled`、起動し直しの可否は真である。利用上限の理由（`reason: usage_limit`）のとき、可否は偽である +- [x] AC3: 結末の読み取りに渡す結果ファイルの名前の幹（stem）は、監視の名前の雛形（`--stem-template`: `{agent}-apply-r$ROUND` / `{agent}-fix-r$ROUND` / `{agent}-final-fix`）を担当名で埋めた値と一致する。幹を組む関数（`paths.stem_for`）の 3 つの工程の値を、骨組みの雛形から作った値と突き合わせる ## 受け入れ条件（共通の手順） -- [ ] AC4: 前提: 結果なしで、起点から HEAD までにコミットが 1 件以上ある +- [x] AC4: 前提: 結果なしで、起点から HEAD までにコミットが 1 件以上ある 操作: 3 つの取り込みのいずれかを呼ぶ 結果: そのコミットは取り消され、起点（`apply_base_sha` と群の `base_sha` / `fix_base_sha` / `final_gate.fix_base_sha`）は取り消し後の HEAD になる -- [ ] AC5: 結果なしのとき、3 つの取り込みのいずれでも、記録の辞書（群 / `final_gate`）の結末の記録（`failed_attempts`）に 1 件（`{phase, attempt, impl, reason, detail, at, reverted}`）が足される。理由（`reason`）は結末の読み取りの値、取り消した数（`reverted`）は取り消したコミットの数である -- [ ] AC6: 結果なしで範囲にコミットが無いとき、`git revert` も `git push` も実行されない -- [ ] AC7: 結果なしの取り込みを、同じ試行番号でもう一度呼ぶと、結果ファイルを読まずに前回と同じ終了コード 2 を返す。結末の記録の件数は増えない。その間に結果ファイルが現れても読まない -- [ ] AC8: 3 つの取り込みで範囲を確定できないとき（起点が無い、または git が範囲を返さない）の終了コードは次のとおりである。適用の取り込みは 4、修正の取り込みは修正ラウンドを 1 進めて 2、最終ゲートの修正の取り込みは 2 +- [x] AC5: 結果なしのとき、3 つの取り込みのいずれでも、記録の辞書（群 / `final_gate`）の結末の記録（`failed_attempts`）に 1 件（`{phase, attempt, impl, reason, detail, at, reverted}`）が足される。理由（`reason`）は結末の読み取りの値、取り消した数（`reverted`）は取り消したコミットの数である +- [x] AC6: 結果なしで範囲にコミットが無いとき、`git revert` も `git push` も実行されない +- [x] AC7: 結果なしの取り込みを、同じ試行番号でもう一度呼ぶと、結果ファイルを読まずに前回と同じ終了コード 2 を返す。結末の記録の件数は増えない。その間に結果ファイルが現れても読まない +- [x] AC8: 3 つの取り込みで範囲を確定できないとき（起点が無い、または git が範囲を返さない）の終了コードは次のとおりである。適用の取り込みは 4、修正の取り込みは修正ラウンドを 1 進めて 2、最終ゲートの修正の取り込みは 2 ## 受け入れ条件（#647: 適用ラウンド） -- [ ] AC9: 群が 2 つ（1 つ目の担当 agy、2 つ目の担当 codex）の状態で、1 つ目の結果ファイルを置かずに群を開く → 適用の取り込みを呼ぶ。終了コードは 2。1 つ目の群は未着手（`status: pending`）のまま担当が agy 以外に替わる。試行の番号（`attempt`）は 1、結末の記録は 1 件（`phase: apply`、`attempt: 1`、`impl: agy`）である -- [ ] AC10: AC9 の後、替わった担当の結果ファイルも置かずにもう一度、群を開く → 適用の取り込みを呼ぶ。1 つ目の群は取り消し済み（`status: dropped`・`drop_reason: no_result`）、項目は `abandoned` になる。見送り（`deferred_items`）に `実装担当が結果を残しませんでした（agy: missing → codex: missing）` の形の理由で入る -- [ ] AC11: 結果ファイルを 1 つも置かずに、群を開く操作が 1 を返すまで繰り返す。群を開く操作の呼び出しは 5 回（開く 4 回 + 尽きた 1 回）で終わり、両方の群が取り消し済み（`dropped`）になる -- [ ] AC12: 結果ファイルが JSON として読めない場合と JSON の配列の場合も AC9 と同じ状態になり、結末の記録の理由（`failed_attempts[].reason`）は `unparsable` である -- [ ] AC13: 群が 4 つ（輪番の通し番号 `apply_seq` が 4）あり、先頭の群（担当 codex）が結果を残さない。替えた後の担当は codex 以外である。輪番の通し番号は進めた分だけ進み、他の群の担当は変わらない -- [ ] AC14: 監視の結果ファイルの理由が `usage_limit` で、輪番から担当を引く関数（`rounds.impl_for_seq`）の差し替えにより交代先が無い状態では、1 回目の失敗で群が取り消し済み（`dropped`、`drop_reason: no_result`）になる。理由が `missing` で交代先が無い状態では、同じ担当で 2 回目を開く -- [ ] AC15: 群を開く操作を、適用の取り込みを挟まず 2 回呼ぶ（取り込みの前に進行が止まった再開）。群の試行の番号は 1 のまま進まない -- [ ] AC16: 着手前のテストの状態が `green` でない状態で適用の取り込みを呼ぶと、結果ファイルを読まずに終了コード 4 で終わる -- [ ] AC17: 適用の取り込みが終了コード 2 で終わった後の群は、取り消し済み（`dropped`）か、結末の記録を持つ未着手（`pending`）のどちらかである。確かめる経路は 4 つ（結果なし / 未割当のコミット / 適用の検証の失敗 / 取り込み済みで採用 0 件） -- [ ] AC18: 開き直しの判定（`rounds.group_reopening`）を差し替えると、群を開く側の開き方（開く・再開・開かない）と、適用の取り込みの結果なしの後の扱い（担当の交代・取り消し）の両方が、差し替えた関数の返す値に従う +- [x] AC9: 群が 2 つ（1 つ目の担当 agy、2 つ目の担当 codex）の状態で、1 つ目の結果ファイルを置かずに群を開く → 適用の取り込みを呼ぶ。終了コードは 2。1 つ目の群は未着手（`status: pending`）のまま担当が agy 以外に替わる。試行の番号（`attempt`）は 1、結末の記録は 1 件（`phase: apply`、`attempt: 1`、`impl: agy`）である +- [x] AC10: AC9 の後、替わった担当の結果ファイルも置かずにもう一度、群を開く → 適用の取り込みを呼ぶ。1 つ目の群は取り消し済み（`status: dropped`・`drop_reason: no_result`）、項目は `abandoned` になる。見送り（`deferred_items`）に `実装担当が結果を残しませんでした（agy: missing → codex: missing）` の形の理由で入る +- [x] AC11: 結果ファイルを 1 つも置かずに、群を開く操作が 1 を返すまで繰り返す。群を開く操作の呼び出しは 5 回（開く 4 回 + 尽きた 1 回）で終わり、両方の群が取り消し済み（`dropped`）になる +- [x] AC12: 結果ファイルが JSON として読めない場合と JSON の配列の場合も AC9 と同じ状態になり、結末の記録の理由（`failed_attempts[].reason`）は `unparsable` である +- [x] AC13: 群が 4 つ（輪番の通し番号 `apply_seq` が 4）あり、先頭の群（担当 codex）が結果を残さない。替えた後の担当は codex 以外である。輪番の通し番号は進めた分だけ進み、他の群の担当は変わらない +- [x] AC14: 監視の結果ファイルの理由が `usage_limit` で、輪番から担当を引く関数（`rounds.impl_for_seq`）の差し替えにより交代先が無い状態では、1 回目の失敗で群が取り消し済み（`dropped`、`drop_reason: no_result`）になる。理由が `missing` で交代先が無い状態では、同じ担当で 2 回目を開く +- [x] AC15: 群を開く操作を、適用の取り込みを挟まず 2 回呼ぶ（取り込みの前に進行が止まった再開）。群の試行の番号は 1 のまま進まない +- [x] AC16: 着手前のテストの状態が `green` でない状態で適用の取り込みを呼ぶと、結果ファイルを読まずに終了コード 4 で終わる +- [x] AC17: 適用の取り込みが終了コード 2 で終わった後の群は、取り消し済み（`dropped`）か、結末の記録を持つ未着手（`pending`）のどちらかである。確かめる経路は 4 つ（結果なし / 未割当のコミット / 適用の検証の失敗 / 取り込み済みで採用 0 件） +- [x] AC18: 開き直しの判定（`rounds.group_reopening`）を差し替えると、群を開く側の開き方（開く・再開・開かない）と、適用の取り込みの結果なしの後の扱い（担当の交代・取り消し）の両方が、差し替えた関数の返す値に従う ## 受け入れ条件（#592: 採用 0 件と項目の無い群） -- [ ] AC19: テスト整備ラウンドで提案が 0 件の状態で提案の取り込み（`merge-proposals`）を呼んだ後、群を開く操作を呼ぶ。1 回目で終了コード 1 を返し、そのラウンドの群の配列（`apply_rounds`）は空のままである -- [ ] AC20: 群の配列の鍵（`apply_rounds`）を持たない状態ファイル（群を導入する前の版）では、群を開く操作が従来どおりラウンド全体を 1 つの群として開く -- [ ] AC21: 前提: rf587 で残った形の群（`status: applied`・`items: []`・`apply.merged_at` あり・`applied: []`） +- [x] AC19: テスト整備ラウンドで提案が 0 件の状態で提案の取り込み（`merge-proposals`）を呼んだ後、群を開く操作を呼ぶ。1 回目で終了コード 1 を返し、そのラウンドの群の配列（`apply_rounds`）は空のままである +- [x] AC20: 群の配列の鍵（`apply_rounds`）を持たない状態ファイル（群を導入する前の版）では、群を開く操作が従来どおりラウンド全体を 1 つの群として開く +- [x] AC21: 前提: rf587 で残った形の群（`status: applied`・`items: []`・`apply.merged_at` あり・`applied: []`） 操作: 適用の取り込みを呼ぶ 結果: 終了コード 2 で終わり、群が取り消し済み（`dropped`、`drop_reason: empty`）になる。続く群を開く操作は 1 を返す -- [ ] AC22: 未着手で項目が無い群（`status: pending`・`items: []`）を持つ状態で、群を開く操作を呼ぶ。その群は開かれずに取り消し済み（`dropped`、`drop_reason: empty`）になる。次の群があればそれを開き、無ければ終了コード 1 を返す +- [x] AC22: 未着手で項目が無い群（`status: pending`・`items: []`）を持つ状態で、群を開く操作を呼ぶ。その群は開かれずに取り消し済み（`dropped`、`drop_reason: empty`）になる。次の群があればそれを開き、無ければ終了コード 1 を返す ## 受け入れ条件（修正ラウンド） -- [ ] AC23: 群の担当が agy、提案ラウンドの担当が codex の状態で、agy の結果ファイル（`agy-fix-r1-result.json`）を置いて修正の取り込みを呼ぶ。agy の結果が取り込まれ、修正ラウンドの数（`fix_rounds`）が 1 になる -- [ ] AC24: 修正の結果ファイルが無い状態で修正の取り込みを呼ぶと、終了コード 2 で終わり、修正ラウンドの数が 1 進む。群の結末の記録に `phase: fix` の 1 件が足される。上限（`--max-fix-rounds`）の回数だけ続けた後の見送りの判定（`should-abandon`）は終了コード 0 を返す -- [ ] AC25: AC24 の直後に検証（`verify-round`）を挟まず修正の取り込みをもう一度呼んでも、修正ラウンドの数は進まない（AC7 の修正ラウンドの形） -- [ ] AC26: 修正の結果なしで監視の理由が `usage_limit` のとき、修正の取り込みは修正ラウンドの数を上限の値にする。続く見送りの判定は終了コード 0 を返す -- [ ] AC27: 修正の結果なしで起点から HEAD にコミットがあるとき、取り消され、起点（`fix_base_sha`）が取り消し後の HEAD になる（AC4 の修正ラウンドの形） +- [x] AC23: 群の担当が agy、提案ラウンドの担当が codex の状態で、agy の結果ファイル（`agy-fix-r1-result.json`）を置いて修正の取り込みを呼ぶ。agy の結果が取り込まれ、修正ラウンドの数（`fix_rounds`）が 1 になる +- [x] AC24: 修正の結果ファイルが無い状態で修正の取り込みを呼ぶと、終了コード 2 で終わり、修正ラウンドの数が 1 進む。群の結末の記録に `phase: fix` の 1 件が足される。上限（`--max-fix-rounds`）の回数だけ続けた後の見送りの判定（`should-abandon`）は終了コード 0 を返す +- [x] AC25: AC24 の直後に検証（`verify-round`）を挟まず修正の取り込みをもう一度呼んでも、修正ラウンドの数は進まない（AC7 の修正ラウンドの形） +- [x] AC26: 修正の結果なしで監視の理由が `usage_limit` のとき、修正の取り込みは修正ラウンドの数を上限の値にする。続く見送りの判定は終了コード 0 を返す +- [x] AC27: 修正の結果なしで起点から HEAD にコミットがあるとき、取り消され、起点（`fix_base_sha`）が取り消し後の HEAD になる（AC4 の修正ラウンドの形） ## 受け入れ条件（#674: 最終ゲートの修正） -- [ ] AC28: 前提: 最終ゲートの修正の結果ファイルが無く、最終ゲートの起点（`final_gate.fix_base_sha`）から HEAD にコミットが 1 件ある +- [x] AC28: 前提: 最終ゲートの修正の結果ファイルが無く、最終ゲートの起点（`final_gate.fix_base_sha`）から HEAD にコミットが 1 件ある 操作: 最終ゲートの修正の取り込みを呼ぶ 結果: 終了コード 2。そのコミットは取り消され、最終ゲートの起点は取り消し後の HEAD になる。最終ゲートの結末の記録（`final_gate.failed_attempts`）は 1 件（`phase: final-fix`） -- [ ] AC29: AC28 の後に最終ゲートを呼ぶと、テストは取り消し後の HEAD で実行され、修正のコミットの一覧（`fix_commits`）に取り消したコミットは入らない -- [ ] AC30: 最終ゲートの修正の結果なしで監視の理由が `usage_limit` のとき、最終ゲートの修正ラウンドの数（`final_gate.fix_rounds`）は上限の値になる。続く最終ゲートは、テストが落ちれば終了コード 1（取り消さず報告）で終わる -- [ ] AC31: 結果ファイルがあり検証を通る最終ゲートの修正は、変更前と同じく取り込まれ、最終ゲートの結末の記録を持たない +- [x] AC29: AC28 の後に最終ゲートを呼ぶと、テストは取り消し後の HEAD で実行され、修正のコミットの一覧（`fix_commits`）に取り消したコミットは入らない +- [x] AC30: 最終ゲートの修正の結果なしで監視の理由が `usage_limit` のとき、最終ゲートの修正ラウンドの数（`final_gate.fix_rounds`）は上限の値になる。続く最終ゲートは、テストが落ちれば終了コード 1（取り消さず報告）で終わる +- [x] AC31: 結果ファイルがあり検証を通る最終ゲートの修正は、変更前と同じく取り込まれ、最終ゲートの結末の記録を持たない ## 受け入れ条件（#553: 帰属行の後ろのトレーラー） 一時リポジトリで実際にコミットを作って確かめる: -- [ ] AC32: 必須トレーラー 4 つの段落の後に、空行を挟んで `Co-Authored-By:` の段落が付いたコミットで、トレーラーの読み取りが 4 つとも値を返す -- [ ] AC33: AC32 の段落の後に `Co-Authored-By:` と `Claude-Session:` の 2 行の段落が付いても、4 つとも返す -- [ ] AC34: 必須トレーラーの段落と末尾の段落の間に散文の段落があるコミットで、散文より前にある `Round: …` の形の行を読まない -- [ ] AC35: 末尾の段落に散文とトレーラーの形の行が混ざる（git がトレーラーの段落と判定しない）コミットで、その行を読まない -- [ ] AC36: 同じ鍵が 2 つの段落にあるとき、末尾に近い段落の値を返す -- [ ] AC37: AC32 の形のコミットを申告した適用ラウンドが、トレーラーの欠落で取り消されない -- [ ] AC38: 本文がトレーラーの段落 1 つだけで、題名が `Round: 本文の題名` の形のコミットで、題名を読まない -- [ ] AC39: 適用と修正の雛形（`prompts/apply.md` / `prompts/fix.md`）のコミットの規約が、必須トレーラーをメッセージの最後の段落に置くことを書く +- [x] AC32: 必須トレーラー 4 つの段落の後に、空行を挟んで `Co-Authored-By:` の段落が付いたコミットで、トレーラーの読み取りが 4 つとも値を返す +- [x] AC33: AC32 の段落の後に `Co-Authored-By:` と `Claude-Session:` の 2 行の段落が付いても、4 つとも返す +- [x] AC34: 必須トレーラーの段落と末尾の段落の間に散文の段落があるコミットで、散文より前にある `Round: …` の形の行を読まない +- [x] AC35: 末尾の段落に散文とトレーラーの形の行が混ざる（git がトレーラーの段落と判定しない）コミットで、その行を読まない +- [x] AC36: 同じ鍵が 2 つの段落にあるとき、末尾に近い段落の値を返す +- [x] AC37: AC32 の形のコミットを申告した適用ラウンドが、トレーラーの欠落で取り消されない +- [x] AC38: 本文がトレーラーの段落 1 つだけで、題名が `Round: 本文の題名` の形のコミットで、題名を読まない +- [x] AC39: 適用と修正の雛形（`prompts/apply.md` / `prompts/fix.md`）のコミットの規約が、必須トレーラーをメッセージの最後の段落に置くことを書く ## 受け入れ条件（無進捗の打ち切り） -- [ ] AC40: 起動（`init`）の出力に無進捗の許容（`IMPL_STALL_TIMEOUT`）が入り、値がテストの制限時間（`--test-timeout`）の値 + 900 である（既定で 1800） -- [ ] AC41: `SKILL.md` の骨組みで、適用・修正・最終ゲートの修正（`--phase apply` / `fix` / `final-fix`）の 3 つの監視（`monitor.py`）の呼び出しが `--stall-timeout "$IMPL_STALL_TIMEOUT"` を持ち、`--timeout` を持たない -- [ ] AC42: 適用・修正・最終ゲートの修正の雛形（`prompts/apply.md` / `fix.md` / `final-fix.md`）が、作業段階ごとに進捗の記録（`$RF_STEM-progress.log`）へ 1 行追記する指示を持つ +- [x] AC40: 起動（`init`）の出力に無進捗の許容（`IMPL_STALL_TIMEOUT`）が入り、値がテストの制限時間（`--test-timeout`）の値 + 900 である（既定で 1800） +- [x] AC41: `SKILL.md` の骨組みで、適用・修正・最終ゲートの修正（`--phase apply` / `fix` / `final-fix`）の 3 つの監視（`monitor.py`）の呼び出しが `--stall-timeout "$IMPL_STALL_TIMEOUT"` を持ち、`--timeout` を持たない +- [x] AC42: 適用・修正・最終ゲートの修正の雛形（`prompts/apply.md` / `fix.md` / `final-fix.md`）が、作業段階ごとに進捗の記録（`$RF_STEM-progress.log`）へ 1 行追記する指示を持つ ## 受け入れ条件（文書） -- [ ] AC43: `SKILL.md` の「この Skill で使う語」の適用ラウンドの行が、同じ群の試行の上限（2 回）を書く。`grep -n "別の上限を置かない\|別に置かない" SKILL.md` が何も出力しない -- [ ] AC44: `docs/02-apply-and-review.md` の Step 4 と `docs/04-fix-and-report.md` の Step 6・Step 7 が 2 つを書く。結果なしのときの取り込みの振る舞い（取り消し・記録・終了コード）と、`SKILL.md` と同じ監視の引数である -- [ ] AC45: `docs/02-apply-and-review.md` のトレーラーの節が、git の標準の読み方（`git log --format='%(trailers:…)'`）が最後の段落しか読まないことと、進行側の読み方の 2 つを書く +- [x] AC43: `SKILL.md` の「この Skill で使う語」の適用ラウンドの行が、同じ群の試行の上限（2 回）を書く。`grep -n "別の上限を置かない\|別に置かない" SKILL.md` が何も出力しない +- [x] AC44: `docs/02-apply-and-review.md` の Step 4 と `docs/04-fix-and-report.md` の Step 6・Step 7 が 2 つを書く。結果なしのときの取り込みの振る舞い（取り消し・記録・終了コード）と、`SKILL.md` と同じ監視の引数である +- [x] AC45: `docs/02-apply-and-review.md` のトレーラーの節が、git の標準の読み方（`git log --format='%(trailers:…)'`）が最後の段落しか読まないことと、進行側の読み方の 2 つを書く ## 受け入れ条件（退行しない） -- [ ] AC46: 結果ファイルがあり検証を通る適用ラウンドは、変更前と同じく 1 回目の試行で取り込まれ、結末の記録を持たない -- [ ] AC47: `uv run --with pytest pytest scripts/tests plugins/ndf -q` が通る -- [ ] AC48: 配布物の同期・定義・frontmatter の 3 つの検査が終了コード 0 で終わる（コマンドは「検証手段」の表） +- [x] AC46: 結果ファイルがあり検証を通る適用ラウンドは、変更前と同じく 1 回目の試行で取り込まれ、結末の記録を持たない +- [x] AC47: `uv run --with pytest pytest scripts/tests plugins/ndf -q` が通る +- [x] AC48: 配布物の同期・定義・frontmatter の 3 つの検査が終了コード 0 で終わる（コマンドは「検証手段」の表） ## 受け入れ条件（他の設計との契約） -- [ ] AC49: 輪番から担当を引く関数（`rounds.impl_for_seq`）を差し替えると、群を割り当てたときの担当・結果を残さなかった群の交代先・最終ゲートの修正担当の 3 つが、差し替えた関数の返す担当になる -- [ ] AC50: 取り消しの本体は取り込みの共通手順の 1 つ（`intake.discard_unverified`）になる。`gitfacts.revert_unverified_range` は無くなり、`apply._revert_unverified_apply_round` は `discard_unverified` を呼ぶ +- [x] AC49: 輪番から担当を引く関数（`rounds.impl_for_seq`）を差し替えると、群を割り当てたときの担当・結果を残さなかった群の交代先・最終ゲートの修正担当の 3 つが、差し替えた関数の返す担当になる +- [x] AC50: 取り消しの本体は取り込みの共通手順の 1 つ（`intake.discard_unverified`）になる。`gitfacts.revert_unverified_range` は無くなり、`apply._revert_unverified_apply_round` は `discard_unverified` を呼ぶ ## 非機能の条件 diff --git a/plugins/ndf/skills/cross-refactoring/SKILL.md b/plugins/ndf/skills/cross-refactoring/SKILL.md index 80c1869a..4a5acd0f 100644 --- a/plugins/ndf/skills/cross-refactoring/SKILL.md +++ b/plugins/ndf/skills/cross-refactoring/SKILL.md @@ -46,15 +46,16 @@ allowed-tools: | --- | --- | --- | | テスト整備ラウンド | **足すべきテストを集める。** 3 者が提案し、採否を決める | `--max-test-rounds`（既定 2） | | 提案ラウンド | **構造改善の提案を集める。** 3 者が提案し、採否を決める | `--max-outer-rounds`（既定 3） | -| 適用ラウンド | **同時に適用して検証する。** 書き換えるファイルが重ならない項目だけを含む。**上の 2 つのラウンドが共有する** | 別に置かない（`--max-items-per-round` が実質の上限） | +| 適用ラウンド | **同時に適用して検証する。** 書き換えるファイルが重ならない項目だけを含む。**上の 2 つのラウンドが共有する** | 同じ群を開き直すのは 2 回まで（引数を持たない固定値）。件数は `--max-items-per-round` が実質の上限 | | 修正ラウンド | **検証の失敗を直す。上の 2 つのラウンドが共有する** | `--max-fix-rounds`（既定 3） | | 改善項目 | 構造改善の提案の 1 件。`<ファイル>#<シンボル>` と兆候で識別する | — | | テスト項目 | テスト整備の提案の 1 件。固定する入口（`target`）と経路の種類（`case`）で識別する | — | **「バッチ」「パッチ」の語は使わない。** 読み手が別の意味で知っている語である。 -**適用ラウンドに別の上限を置かない。** 採用件数の上限が既に群の数を切っている。 -上限を 2 つ置くと、どちらで止まったのかを読み解く必要が出る。 +**同じ群を開き直すのは 2 回までである。** 2 回目は別の担当が試す。2 回とも結果を +残さなければ、担当ではなく群の側を疑える。止まった理由は群の記録（取り消しの理由と +結末の記録）が持つ。 ## 設計方針 @@ -302,6 +303,7 @@ while :; do "$SCRIPTS/launch-cli.sh" "$a" "$PROPOSE_PHASE" "$ID" "$ROUND" done # 監視の上限は `--phase` の工程で上限の表（`lib/limits.py`）が決める。秒数を書かない。 + # 無進捗の許容だけは `init` が出す（テスト 1 回分の無出力で打ち切らないため）。 # **結果ファイルの名前は種類で変えない**ので、監視の雛形と工程（`propose`）は # テスト整備ラウンドでもそのまま使える。 "$LIB/monitor.py" "$ID" --agents "$RUNTIMES_CSV" --tmp-dir "$TMP_DIR" \ @@ -314,8 +316,9 @@ while :; do rf_eval next-apply-round "$ID" "$ROUND" || break # 終了コード 1 = 群が尽きた "$SCRIPTS/launch-cli.sh" "$IMPL" apply "$ID" "$ROUND" "$LIB/monitor.py" "$ID" --agents "$IMPL" --tmp-dir "$TMP_DIR" \ - --stem-template "{agent}-apply-r$ROUND" --phase apply - # 終了コード 2 = 適用が通らずこの群を取り消した。修正ラウンドは回さない + --stem-template "{agent}-apply-r$ROUND" --phase apply \ + --stall-timeout "$IMPL_STALL_TIMEOUT" + # 終了コード 2 = この群を取り消した、または担当を替えて開き直す。修正ラウンドは回さない rf merge-apply "$ID" "$ROUND" || continue while :; do # 検証と修正の繰り返し @@ -325,7 +328,8 @@ while :; do fi "$SCRIPTS/launch-cli.sh" "$IMPL" fix "$ID" "$ROUND" "$LIB/monitor.py" "$ID" --agents "$IMPL" --tmp-dir "$TMP_DIR" \ - --stem-template "{agent}-fix-r$ROUND" --phase fix + --stem-template "{agent}-fix-r$ROUND" --phase fix \ + --stall-timeout "$IMPL_STALL_TIMEOUT" rf merge-fix "$ID" "$ROUND" done # 次の群と、次のラウンドの提案に備えて読み取り用を同期する @@ -346,7 +350,8 @@ while :; do 1) echo "⚠ 最終ゲートが通らないまま修正の上限に達しました" >&2; break ;; 2) "$SCRIPTS/launch-cli.sh" "$FINAL_FIX_IMPL" final-fix "$ID" "$LIB/monitor.py" "$ID" --agents "$FINAL_FIX_IMPL" --tmp-dir "$TMP_DIR" \ - --stem-template "{agent}-final-fix" --phase final-fix + --stem-template "{agent}-final-fix" --phase final-fix \ + --stall-timeout "$IMPL_STALL_TIMEOUT" rf merge-final-fix "$ID" ;; *) exit $gate ;; esac diff --git a/plugins/ndf/skills/cross-refactoring/docs/02-apply-and-review.md b/plugins/ndf/skills/cross-refactoring/docs/02-apply-and-review.md index 928ab276..81185715 100644 --- a/plugins/ndf/skills/cross-refactoring/docs/02-apply-and-review.md +++ b/plugins/ndf/skills/cross-refactoring/docs/02-apply-and-review.md @@ -9,17 +9,36 @@ eval "$("$SCRIPTS/refactor.py" next-apply-round "$ID" "$ROUND")" # 1 = 群が尽きた "$SCRIPTS/launch-cli.sh" "$IMPL" apply "$ID" "$ROUND" "$LIB/monitor.py" "$ID" --agents "$IMPL" --tmp-dir "$TMP_DIR" \ - --stem-template "{agent}-apply-r$ROUND" --phase apply -"$SCRIPTS/refactor.py" merge-apply "$ID" "$ROUND" # 2 = この群を取り消した / 4 = 中断 + --stem-template "{agent}-apply-r$ROUND" --phase apply \ + --stall-timeout "$IMPL_STALL_TIMEOUT" +"$SCRIPTS/refactor.py" merge-apply "$ID" "$ROUND" # 2 = 取り消した / 開き直す / 4 = 中断 ``` 終了コード 2 と 4 を**必ず区別する**。同じ扱いにすると、取り消しに失敗した状態を 「この群の失敗」として次の群へ進み、検証を通っていない変更が Pull Request に 残ったまま先へ進む（実測）。 -**適用の単位は適用ラウンド（群）である。** 群の中の項目は書き換えるファイルが -重ならないので、**まとめて 1 コミット**にできる。群と群は同じファイルを直列に -書き換えるため順序に依存し、後続の群は先行の群を適用した後の作業ツリーを読む。 +**無進捗の許容は `init` が出す**（`IMPL_STALL_TIMEOUT` = テストの制限時間 + 900 秒）。 +担当はテストの実行中は何も出力しないため、制限時間そのままでは打ち切られる。 + +### 実装担当が結果を残さなかったとき + +結果ファイルが無い・読めないまま終わることがある（無進捗の打ち切り・利用上限・起動の +直後に落ちた）。取り込みは、未検証のコミットを新しい順に取り消して起点を取り消し後の先端へ +進め、群の `failed_attempts[]` へ結末を 1 件記録し、終了コード 2 で終える。 + +| 何回目か | 何が起きるか | +| --- | --- | +| 1 回目 | その群で失敗した担当のどれとも違う、次の輪番の担当へ替えて開き直す | +| 2 回目 | 群を取り消し（`drop_reason: no_result`）、項目を見送りへ入れる。理由にはどの担当がどの理由で残さなかったかが並ぶ | +| 替える先が無い | 起動し直しの可否で決める。利用上限は待ちと相手の枠を使うだけなので 1 回目で取り消す | + +**項目が 1 件も無い群は開かない。** 採用 0 件の提案ラウンドは群を作らず、残っている +項目なしの群は取り消し済み（`drop_reason: empty`）にして次へ進む。 + +**適用の単位は適用ラウンド（群）である。** 群の中の項目は書き換えるファイルが重ならない +ので、**まとめて 1 コミット**にできる。群と群は同じファイルを直列に書き換えるため順序に +依存し、後続の群は先行の群を適用した後の作業ツリーを読む。 実装担当を**1 つの群につき 1 回**起動し、その群の項目を優先度順に**直列適用**させる。 並列適用はしない（同一ブランチへの同時コミットは競合と取り消し単位の曖昧化を招く）。 @@ -208,38 +227,6 @@ fi 控えておき、最後に `git reset --soft` で 1 コミットへまとめる。控えた地点より前へ 戻すと他の群のコミットを巻き込むため、起点は群の着手前に固定する。 -#### 改修計画は Pull Request のコメントに残す - -**なぜ直すのか（理由）とどう直すのか（手順）は、提案の時点でしか残らない。** -状態ファイルには入っているが、そのディレクトリは差分から除外されるため、 -Pull Request を読む側からは見えない。 - -**改修計画は実行の記録であって、リポジトリの知識ではない**（#436 決定 6）。既定の -置き場所は**対象の Pull Request のコメント 1 件**で、ラウンドが進むたびに**同じ -コメントを編集する**。 - -| 置き場所 | URL の安定 | 差分に混ざるか | 更新の手数 | -| --- | --- | --- | --- | -| **Pull Request のコメント 1 件**（既定） | **永続** | 混ざらない | 編集 1 回 | -| ファイル（`--plan-file`） | `` に依存。ブランチが消えると切れる | **混ざる** | コミットと push | - -- 内容は**状態から決まる**。同じ状態からは同じ本文が出る -- **取り消した項目の内訳を持つのは改修計画だけである。** 他の文章は件数だけ述べる -- 本文の先頭に印（``）を置く。状態ファイルの - 控えが失われても、印で同じコメントを引き当てられる。**引き当てられないと、 - ラウンドのたびに新しいコメントが積まれる** -- **投稿に失敗しても進行は止めない。** 記録が残らないことと、変更が検証を通って - いないことは別である。失敗したことは出力に残る - -**`--plan-file` は残す。** 明示したときだけファイルにする。この経路では公開を -生成物の同期と**同じコミット**に乗せる（分けると進行側のコミットが公開のたびに -2 つずつ積まれる）。空文字を渡すと記録しない。 - -**絶対パスと親へ抜ける経路は受け取った時点で拒む**（終了コード 4）。進行側は利用者の -リポジトリを触るため、作業ディレクトリの外へ書き出す余地を残さない。あわせて -`./issues/plan.md` のような表記も正規化する。git が返すパスと形が違うと、公開の -コミットメッセージが取り違えられる。 - #### 範囲の指定は検証にも効かせる `--scope` を必須にした目的は**提案の発散と変更の肥大を防ぐ**ことなので、指定を検証へ @@ -357,15 +344,15 @@ Pull Request に残る。**都合の悪い変更を申告しないだけで検 | コマンド | 二重処理を防ぐ鍵 | | --- | --- | | `merge-proposals` | `proposal_keys` が既にあるか | -| `merge-apply` | `apply.merged_at` が既にあるか | +| `merge-apply` | `apply.merged_at` が既にあるか。結果を残さなかった試行は、群の `failed_attempts[]` に同じ工程と試行番号があるか | | `merge-fix` | 試行番号（`verify-round` が進める）と結果ファイルの内容の組 | | `abandon-items` | `abandoned` が既にあるか | #### 結果ファイルの形が崩れていても落ちない -相手は LLM なので、`commits` が配列でない・要素が辞書でない・`sha` が文字列でない -といった崩れ方をする。結果ファイルを読む箇所は**型を確かめてから使い**、取り出せた -ものだけを扱う。落ちると進行が止まるだけで、何の検証にもならない。 +相手は LLM なので、`commits` が配列でない・要素が辞書でないといった崩れ方をする。 +結果ファイルを読む箇所は**型を確かめてから使い**、取り出せたものだけを扱う。**JSON +オブジェクトとして読めない結果ファイルは結果なしと同じ扱いにする**（理由は `unparsable`）。 **1 件の失敗でラウンドを止めない。** 失敗した項目だけを見送りにして、残りは採用する。 全件失敗のときだけ終了コード 2 を返し、次の提案ラウンドへ進む。 @@ -444,6 +431,19 @@ Impl-Model: gpt-5.5 自由文で「codex が実装」と書かせると集計に使えない。プロンプトに書くだけでは守られない ので、`merge-apply` が有無を検証する。 +**読み方は 2 つあり、見る範囲が違う。** + +| 読み手 | 読み方 | 見る範囲 | +| --- | --- | --- | +| 人（集計） | `git log --format='%(trailers:key=Impl-Model,valueonly)'` | **最後の段落だけ** | +| 進行側（検証） | 末尾の段落から前へ 1 段落ずつ `git interpret-trailers --parse` に掛け、記名の段落と判定しなかったところで止める | 末尾から続く記名の段落すべて | + +実行環境が `Co-Authored-By:` などの帰属行を別の段落として足しても、進行側はその段落を +飛ばして前まで読むため検証は通る。人の集計は最後の段落しか読まないので、**雛形では +帰属行を空行を挟まず同じ段落に続けるよう求めている**（従わなくても検証は通る）。 +**1 段落目（題名）は判定に掛けない。** 掛けると `Round: 本文の題名` の形の題名を記名 +として読む（実測）。散文と記名の形が混ざる段落は、git が記名の段落と判定しない。 + `Impl-Model` には**実際に使ったモデル名**を書かせる。既定モデルで走った場合は `default` として報告時に区別する。 diff --git a/plugins/ndf/skills/cross-refactoring/docs/04-fix-and-report.md b/plugins/ndf/skills/cross-refactoring/docs/04-fix-and-report.md index 82810416..c2711ab0 100644 --- a/plugins/ndf/skills/cross-refactoring/docs/04-fix-and-report.md +++ b/plugins/ndf/skills/cross-refactoring/docs/04-fix-and-report.md @@ -10,8 +10,9 @@ if "$SCRIPTS/refactor.py" should-abandon "$ID" "$ROUND"; then else "$SCRIPTS/launch-cli.sh" "$IMPL" fix "$ID" "$ROUND" "$LIB/monitor.py" "$ID" --agents "$IMPL" --tmp-dir "$TMP_DIR" \ - --stem-template "{agent}-fix-r$ROUND" --phase fix - "$SCRIPTS/refactor.py" merge-fix "$ID" "$ROUND" + --stem-template "{agent}-fix-r$ROUND" --phase fix \ + --stall-timeout "$IMPL_STALL_TIMEOUT" + "$SCRIPTS/refactor.py" merge-fix "$ID" "$ROUND" # 2 = 結果なし / 範囲が確定しない # 修正後の状態を次の提案へ届ける "$SCRIPTS/prepare-worktrees.sh" "$ID" sync "$(git -C "$WORK" rev-parse HEAD)" fi @@ -28,6 +29,18 @@ fi **`--max-fix-rounds` は 1 つの適用ラウンドあたりの上限である。** 数え直しは `next-apply-round` が群を開くときに行う。 +**読むのは群の担当の結果である。** 骨組みが起動するのは群の担当であり、提案ラウンドの +担当とは限らない。食い違うと結果ファイルを一度も引けず、修正ラウンドが進まないまま +検証と修正を往復し続ける。 + +**修正の担当が結果を残さなかったときも、修正ラウンドは進める。** 未検証のコミットを +取り消し、群の `failed_attempts[]` へ 1 件（工程 `fix`）を記録し、終了コード 2 で +終える。進めないと見送りの判定が上限に達する条件を満たさない。**起動し直しても +解けない結末（利用上限）では、修正ラウンドの数を上限の値にする。** 次の見送りの判定が +そのまま見送りへ移すため、同じ担当を上限まで起動し直すことがなくなる。 + +**無進捗の許容は `init` が出す**（`IMPL_STALL_TIMEOUT`）。適用と同じ値である。 + ### 修正コミットも適用と同じ基準で見る `merge-fix` は修正コミットにも `Item-Id` / `Round` / `Impl-Runtime` / `Impl-Model` と @@ -228,7 +241,10 @@ Pull Request の読み手が持つため、失敗として報告に書く。 ```bash "$SCRIPTS/launch-cli.sh" "$FINAL_FIX_IMPL" final-fix "$ID" # 担当は final-gate が返す -"$SCRIPTS/refactor.py" merge-final-fix "$ID" # 取り込みも専用 +"$LIB/monitor.py" "$ID" --agents "$FINAL_FIX_IMPL" --tmp-dir "$TMP_DIR" \ + --stem-template "{agent}-final-fix" --phase final-fix \ + --stall-timeout "$IMPL_STALL_TIMEOUT" +"$SCRIPTS/refactor.py" merge-final-fix "$ID" # 2 = 結果なし / 範囲が確定しない ``` **`fix` フェーズと `merge-fix` は使い回せない。** どちらも適用ラウンド（群）の控えを @@ -263,6 +279,14 @@ Pull Request の読み手が持つため、失敗として報告に書く。 「上限に達しても取り消さない」のは*採用した改善項目*の話であって、検証を受けていない 修正コミットは別である。取り消せば HEAD は最終ゲートが見た地点へ戻る。 +**修正の担当が結果を残さなかったときも同じ手順を通る。** 取り消して起点を取り消し後の +先端へ進め、最終ゲートの記録の `failed_attempts[]` へ 1 件（工程 `final-fix`）を残し、 +終了コード 2 で最終ゲートへ判定を戻す。取り消さずに抜けると、次の最終ゲートがその +コミットを含む先端でテストし、落ちれば起点をそこへ置き直すため、未検証の差分が +Pull Request に残る。**修正ラウンドはここでは進めない。** 進めるのは次の最終ゲート +である。ただし**起動し直しても解けない結末（利用上限）では上限の値にする**。次の +最終ゲートは、テストが落ちれば終了コード 1（取り消さず報告）で終わる。 + ### テストで見つからない誤りは誰が拾うか Step 5 をテストへ置き換え、Step 7 を条件付きで省くと、実行の流れからレビューが @@ -307,6 +331,38 @@ Step 5 のテストが済ませている。同じ観点を後段へ渡すと、 | 実装担当 | 担当ラウンド数 / 適用成功した項目数 / 見送った項目数 / 平均修正ラウンド数 / 差分予算の超過率 / テスト失敗の発生率 / 所要時間 | | 提案の担当 | 提案件数 / 採用された率 / 他者と合意した率 / 所要時間 | +### 改修計画は Pull Request のコメントに残す + +**なぜ直すのか（理由）とどう直すのか（手順）は、提案の時点でしか残らない。** +状態ファイルには入っているが、そのディレクトリは差分から除外されるため、 +Pull Request を読む側からは見えない。 + +**改修計画は実行の記録であって、リポジトリの知識ではない**（#436 決定 6）。既定の +置き場所は**対象の Pull Request のコメント 1 件**で、ラウンドが進むたびに**同じ +コメントを編集する**。 + +| 置き場所 | URL の安定 | 差分に混ざるか | 更新の手数 | +| --- | --- | --- | --- | +| **Pull Request のコメント 1 件**（既定） | **永続** | 混ざらない | 編集 1 回 | +| ファイル（`--plan-file`） | `` に依存。ブランチが消えると切れる | **混ざる** | コミットと push | + +- 内容は**状態から決まる**。同じ状態からは同じ本文が出る +- **取り消した項目の内訳を持つのは改修計画だけである。** 他の文章は件数だけ述べる +- 本文の先頭に印（``）を置く。状態ファイルの + 控えが失われても、印で同じコメントを引き当てられる。**引き当てられないと、 + ラウンドのたびに新しいコメントが積まれる** +- **投稿に失敗しても進行は止めない。** 記録が残らないことと、変更が検証を通って + いないことは別である。失敗したことは出力に残る + +**`--plan-file` は残す。** 明示したときだけファイルにする。この経路では公開を +生成物の同期と**同じコミット**に乗せる（分けると進行側のコミットが公開のたびに +2 つずつ積まれる）。空文字を渡すと記録しない。 + +**絶対パスと親へ抜ける経路は受け取った時点で拒む**（終了コード 4）。進行側は利用者の +リポジトリを触るため、作業ディレクトリの外へ書き出す余地を残さない。あわせて +`./issues/plan.md` のような表記も正規化する。git が返すパスと形が違うと、公開の +コミットメッセージが取り違えられる。 + ### 報告も外へ出す文章である | 規約 | 報告での書き方 | diff --git a/plugins/ndf/skills/cross-refactoring/prompts/apply.md b/plugins/ndf/skills/cross-refactoring/prompts/apply.md index 8c257241..c3091132 100644 --- a/plugins/ndf/skills/cross-refactoring/prompts/apply.md +++ b/plugins/ndf/skills/cross-refactoring/prompts/apply.md @@ -63,6 +63,28 @@ Impl-Model: $RF_MODEL - **`Item-Id` にはこの適用ラウンドの先頭の項目 ID を書く。** どの項目の ID でも 検証は通りますが、揃えておくと履歴が読みやすくなります - `Impl-Model` には**実際に使ったモデル名**を書く。分からなければ `default` +- **4 つのトレーラーはメッセージの最後の段落に置く。** 実行環境が + `Co-Authored-By:` などの帰属行を足すときは、**空行を挟まず同じ段落に続けます**。 + 人が `git log --format='%(trailers:key=Impl-Model,valueonly)'` で集計するとき、 + git は最後の段落しか読みません + +## 作業の進み具合を残す + +**作業段階が進むたびに `$RF_STEM-progress.log` へ 1 行追記してください。** +テストの実行中は何も出力されないため、追記が無いと監視が「進んでいない」と見て +打ち切ります。書くのは段階と対象だけで、内容は要りません。 + +```bash +echo "test src/foo.py" >> "$RF_STEM-progress.log" +``` + +| 段階 | いつ書くか | +| --- | --- | +| `start` | 着手した | +| `edit` | ファイルを書き換えた | +| `test` | テストを実行する直前 | +| `commit` | コミットした | +| `done` | 結果ファイルを書き終えた | ## 守ること diff --git a/plugins/ndf/skills/cross-refactoring/prompts/final-fix.md b/plugins/ndf/skills/cross-refactoring/prompts/final-fix.md index a2dc4eb2..b3f49ad7 100644 --- a/plugins/ndf/skills/cross-refactoring/prompts/final-fix.md +++ b/plugins/ndf/skills/cross-refactoring/prompts/final-fix.md @@ -51,6 +51,24 @@ Impl-Model: $RF_MODEL **申告から漏れたコミットがあると、この修正の範囲ごと取り消されます。** 作った コミットは 1 件残らず結果ファイルへ書いてください。 +## 作業の進み具合を残す + +**作業段階が進むたびに `$RF_STEM-progress.log` へ 1 行追記してください。** +テストの実行中は何も出力されないため、追記が無いと監視が「進んでいない」と見て +打ち切ります。書くのは段階と対象だけで、内容は要りません。 + +```bash +echo "test src/foo.py" >> "$RF_STEM-progress.log" +``` + +| 段階 | いつ書くか | +| --- | --- | +| `start` | 着手した | +| `edit` | ファイルを書き換えた | +| `test` | テストを実行する直前 | +| `commit` | コミットした | +| `done` | 結果ファイルを書き終えた | + ## 守ること - **push しない。** 公開するのは進行側だけで、**検証を通した後**に行います diff --git a/plugins/ndf/skills/cross-refactoring/prompts/fix.md b/plugins/ndf/skills/cross-refactoring/prompts/fix.md index 5115b0fe..d0fdf562 100644 --- a/plugins/ndf/skills/cross-refactoring/prompts/fix.md +++ b/plugins/ndf/skills/cross-refactoring/prompts/fix.md @@ -63,6 +63,29 @@ Impl-Runtime: $RF_RUNTIME Impl-Model: $RF_MODEL ``` +- **4 つのトレーラーはメッセージの最後の段落に置く。** 実行環境が + `Co-Authored-By:` などの帰属行を足すときは、**空行を挟まず同じ段落に続けます**。 + 人が `git log --format='%(trailers:key=Impl-Model,valueonly)'` で集計するとき、 + git は最後の段落しか読みません + +## 作業の進み具合を残す + +**作業段階が進むたびに `$RF_STEM-progress.log` へ 1 行追記してください。** +テストの実行中は何も出力されないため、追記が無いと監視が「進んでいない」と見て +打ち切ります。書くのは段階と対象だけで、内容は要りません。 + +```bash +echo "test src/foo.py" >> "$RF_STEM-progress.log" +``` + +| 段階 | いつ書くか | +| --- | --- | +| `start` | 着手した | +| `edit` | ファイルを書き換えた | +| `test` | テストを実行する直前 | +| `commit` | コミットした | +| `done` | 結果ファイルを書き終えた | + ## 守ること - **push しない。** 公開するのは進行側だけで、**検証を通した後**に行います diff --git a/plugins/ndf/skills/cross-refactoring/scripts/refactor_lib/commands/apply.py b/plugins/ndf/skills/cross-refactoring/scripts/refactor_lib/commands/apply.py index 0e8d8fef..e89feec9 100644 --- a/plugins/ndf/skills/cross-refactoring/scripts/refactor_lib/commands/apply.py +++ b/plugins/ndf/skills/cross-refactoring/scripts/refactor_lib/commands/apply.py @@ -10,9 +10,8 @@ import json import pathlib import sys -from typing import Any +from typing import Any, Optional -import assignment import statefile from .. import die, info @@ -27,20 +26,28 @@ read_result, record_observed_model, reported_shas, - revert_item_commits, round_of, safe_int, collect_commit_facts, commits_in_range, ) +from ..intake import ( + IntakeScope, + already_closed, + close_without_result, + discard_unverified, +) from ..paths import git_out, load_state, result_path, stem_for from ..proposals import assign_apply_rounds, merge_proposals, merge_test_proposals from ..rounds import ( TEST, apply_groups, + attempt_of, current_group, deferred_record, entry_kind, + group_reopening, + impl_for_seq, item_key, item_kind, item_label, @@ -131,18 +138,19 @@ def _assign_apply_rounds_to_state( seq = safe_int(state.get("apply_seq")) for n, group in enumerate(assign_apply_rounds(adopted), start=1): seq += 1 - impl, _ = assignment.assign(seq, state["host"]) + impl, requested = impl_for_seq(state, seq) for item in group: item["apply_round"] = n entry["apply_rounds"].append({ "apply_round": n, "impl": impl, - "impl_model": {"requested": state["models"].get(impl), "observed": None}, + "impl_model": {"requested": requested, "observed": None}, "items": [i["item_id"] for i in group], "status": "pending", "base_sha": None, "head_sha": None, "fix_rounds": 0, + "attempt": 0, }) state["apply_seq"] = seq @@ -293,19 +301,44 @@ def cmd_next_apply_round(args: argparse.Namespace) -> None: # **`applied` の群も開き直す。** 適用は取り込んだが検証まで進めずに落ちた場合、 # 飛ばすとその群の項目が採用でも取り消しでもないまま残る。再開できることは # 収束ループの前提である。 - opened = next( - (g for g in groups if g.get("status") in {"pending", "applied"}), None - ) + # + # **未着手の群は、開き直しの判定へ掛ける**（#647）。無条件に開き直すと、結果を + # 残さない担当に当たり続けて上限なく起動する。項目が無い群と上限に達した群は、 + # ここで取り消し済みにして次を探す。 + opened: Optional[dict[str, Any]] = None + reopening = "" + for group in groups: + if group.get("status") not in {"pending", "applied"}: + continue + if group.get("status") == "applied": + opened = group + break + reopening = group_reopening(group) + if reopening in {"empty", "exhausted"}: + group["status"] = "dropped" + group.setdefault( + "drop_reason", "empty" if reopening == "empty" else "no_result" + ) + info( + f"適用ラウンド {group['apply_round']} は開きません" + f"（{'項目なし' if reopening == 'empty' else '試行の上限'}）" + ) + continue + opened = group + break + if opened is None: + statefile.save(path, state) info(f"提案ラウンド {args.round} の適用ラウンドは残っていません") sys.exit(1) entry["apply_round"] = opened["apply_round"] - if opened.get("status") == "pending": + if opened.get("status") == "pending" and reopening == "open": # 起点は**オーケストレータ側で**確定させる。実装担当の申告に委ねると、 # 欠落・不正時に範囲検査が無効になり、過去の任意のコミットが実在扱いになる。 head = git_out(state["worktrees"]["work"], ["rev-parse", "HEAD"]) opened["base_sha"] = head + opened["attempt"] = attempt_of(opened) + 1 entry["apply_base_sha"] = head entry["fix_rounds"] = 0 entry["apply"] = { @@ -313,6 +346,10 @@ def cmd_next_apply_round(args: argparse.Namespace) -> None: "applied": [], "failed": [], "base_sha": head, "head_sha": None, "merged_at": None, } + elif opened.get("status") == "pending": + # 開いたまま閉じていない試行の再開。**起点も試行の番号も動かさない。** + info(f"↻ 適用ラウンド {opened['apply_round']} の試行を再開します") + entry["apply_base_sha"] = opened.get("base_sha") else: # 取り込み済みの群を開き直した。**起点も修正の回数も動かさない。** info(f"↻ 適用ラウンド {opened['apply_round']} は取り込み済みです（検証から再開）") @@ -334,10 +371,89 @@ def cmd_next_apply_round(args: argparse.Namespace) -> None: ) +def _check_already_merged_apply(ctx: _ApplyExecutionContext) -> bool: + """取り込み済み判定を行い、再実行を制御する。処理済みなら True を返す。 + + **叩き直しても同じ判定を返す。** 取り込み済みで再実行すると、前回作った + 取り消しコミットが「未割当」と判定され、群ごと取り消してしまう。 + """ + record = ctx.entry.get("apply") or {} + if record.get("merged_at") and record.get("apply_round", ctx.group["apply_round"]) \ + == ctx.group["apply_round"]: + applied_before = record.get("applied") or [] + info( + f"↻ 適用ラウンド {ctx.group['apply_round']} の適用は取り込み済みです" + f"（採用 {len(applied_before)} 件 / 失敗 " + f"{len(record.get('failed') or [])} 件）" + ) + if not applied_before: + # **採用 0 件の群は取り消し済みに直す**（#592）。残したままだと、 + # 次に群を開く操作がこの群を選び直して担当を起動し続ける。 + ctx.group["status"] = "dropped" + ctx.group.setdefault("drop_reason", "empty") + ctx.state["phase"] = phase_after_group(ctx.entry) + if not ctx.args.dry_run: + statefile.save(ctx.path, ctx.state) + sys.exit(2) + return True + return False + + +def _verify_baseline_test_gate(ctx: _ApplyExecutionContext) -> None: + """着手前テスト結果 (baseline) を検証し、成功でなければ適用をブロックする。 + + **着手前のテストの確認は、結果を読むより先に行う。** 成功と確認できていない + 状態で採ると、壊したのか元から壊れていたのかを判別する手段が無い。`red` だけ + でなく `unknown`（確認していない）も拒否する。 + """ + baseline = ctx.state.get("baseline_test") or {} + if baseline.get("status") != "green": + _block_group_items(ctx) + die( + f"着手前のテストが成功と確認できていません（status={baseline.get('status')}）。" + "適用へ着手しません（全項目を blocked）", + code=4, + ) + + +def _finalize_apply_result( + ctx: _ApplyExecutionContext, + record: dict[str, Any], + failed: list[str], +) -> None: + """検証結果に応じて状態更新、取り消し、または公開プッシュを反映する。""" + # `--dry-run` では git も状態ファイルも触らない。片方だけ進むと、確認の + # つもりで実行した利用者の進行が壊れる。 + if ctx.args.dry_run: + if failed: + drop_items(ctx.state, ctx.entry, failed, dry_run=True) + info("（dry-run）状態ファイルは更新していません") + applied = list(record["applied"]) + elif failed: + # `merged_at` は `_apply_drop` が取り消しの完了時点で立てる。 + applied = _apply_drop(ctx.path, ctx.state, ctx.entry, ctx.group, failed) + else: + # **全項目が通ったときも進行側が公開する。** 実装担当は push しないため、 + # ここで公開しないと Pull Request 上の差分が古いままになる。 + ctx.group["status"] = "applied" + # 次は `verify-round` がテストで検証する。ここではまだ群を閉じない。 + ctx.state["phase"] = "verify" + record["merged_at"] = statefile.now() + # 保留の印・保存・push・印の解除は 1 か所が持つ（`push_with_retry_marker`）。 + push_with_retry_marker(ctx.path, ctx.state, ctx.entry) + applied = list(record["applied"]) + + if not applied: + info("この適用ラウンドは取り消しました。検証は行いません") + sys.exit(2) + + def cmd_merge_apply(args: argparse.Namespace) -> None: """Step 4 — 適用ラウンド 1 つ分の適用結果を検証して取り込む。 - 終了コード: 0 = 取り込んだ / 2 = この群を取り消した（次の群へ進む）。 + 終了コード: 0 = 取り込んだ / 2 = この群を取り消した、または担当を替えて開き直す + （次の群へ進む） / 4 = 着手前のテストが成功と確認できていない・範囲を確定 + できない・群が無い。 **適用そのものが通らないときは修正ラウンドを回さない**（競合・対象が消えて いる・手順を外れた）。修正ラウンドはテストの失敗を直す工程であり、前提その @@ -354,22 +470,21 @@ def cmd_merge_apply(args: argparse.Namespace) -> None: discard_impl_leftovers(state, state["worktrees"]["work"]) _resume_incomplete_apply(path, state, entry) - # **叩き直しても同じ判定を返す。** 取り込み済みで再実行すると、前回作った - # 取り消しコミットが「未割当」と判定され、群ごと取り消してしまう。 - record = entry.get("apply") or {} - if record.get("merged_at") and record.get("apply_round", group["apply_round"]) \ - == group["apply_round"]: - applied_before = record.get("applied") or [] - info( - f"↻ 適用ラウンド {group['apply_round']} の適用は取り込み済みです" - f"（採用 {len(applied_before)} 件 / 失敗 " - f"{len(record.get('failed') or [])} 件）" - ) - if not applied_before: - sys.exit(2) + if _check_already_merged_apply(ctx): return - payload, commit_range = _load_apply_context(ctx) + _verify_baseline_test_gate(ctx) + + scope = _apply_scope(ctx) + if already_closed(scope): + info("↻ この試行は結果なしとして記録済みです") + sys.exit(2) + + outcome = read_result(state, scope.impl, "apply", args.round) + if outcome.payload is None: + _close_failed_attempt(ctx, scope, outcome) + + payload, commit_range = _load_apply_context(ctx, outcome.payload) reported, unknown_ids = _collect_apply_reports(payload, group) @@ -382,30 +497,7 @@ def cmd_merge_apply(args: argparse.Namespace) -> None: ) record = _record_apply_result(entry, group, commit_range, applied, failed, payload) - - # `--dry-run` では git も状態ファイルも触らない。片方だけ進むと、確認の - # つもりで実行した利用者の進行が壊れる。 - if args.dry_run: - if failed: - drop_items(state, entry, failed, dry_run=True) - info("（dry-run）状態ファイルは更新していません") - applied = list(record["applied"]) - elif failed: - # `merged_at` は `_apply_drop` が取り消しの完了時点で立てる。 - applied = _apply_drop(path, state, entry, group, failed) - else: - # **全項目が通ったときも進行側が公開する。** 実装担当は push しないため、 - # ここで公開しないと Pull Request 上の差分が古いままになる。 - group["status"] = "applied" - # 次は `verify-round` がテストで検証する。ここではまだ群を閉じない。 - state["phase"] = "verify" - record["merged_at"] = statefile.now() - # 保留の印・保存・push・印の解除は 1 か所が持つ（`push_with_retry_marker`）。 - push_with_retry_marker(path, state, entry) - - if not applied: - info("この適用ラウンドは取り消しました。検証は行いません") - sys.exit(2) + _finalize_apply_result(ctx, record, failed) def _record_apply_result( @@ -444,26 +536,120 @@ def _block_group_items(ctx: _ApplyExecutionContext) -> None: statefile.save(ctx.path, ctx.state) -def _load_apply_context( - ctx: _ApplyExecutionContext, -) -> tuple[dict[str, Any], _ApplyCommitRange]: - impl = ctx.group.get("impl") or ctx.entry["impl"] - result = result_path(ctx.state, impl, stem_for(impl, "apply", ctx.state["id"], ctx.args.round)) - payload = read_result(result, impl) +def _apply_scope(ctx: _ApplyExecutionContext) -> IntakeScope: + """適用の取り込み 1 回分の範囲の値。 - record_observed_model(ctx.entry, "impl", impl, ctx.state, "apply", ctx.args.round) + 起点は提案ラウンドの控えが持ち、群の起点も同じ値へ揃える。結末の記録は群が + 持つ。**試行の番号が 0 なら 1 として扱う**（この版より前に開いた群の再開）。 + """ + group = ctx.group + return IntakeScope( + holder=ctx.entry, + base_key="apply_base_sha", + records=group, + phase="apply", + attempt=attempt_of(group) or 1, + impl=group.get("impl") or ctx.entry["impl"], + label=f"R{ctx.entry['round']}-A{group['apply_round']}", + mirror=group, + ) - # 着手前のテストが**成功と確認できていない限り**適用結果を採らない。 - # `red` だけでなく `unknown`（確認していない）も拒否する。確認していない状態を - # 通すと、「壊したのか元から壊れていたのか」を判別する手段が無いまま進む。 - baseline = ctx.state.get("baseline_test") or {} - if baseline.get("status") != "green": + +def _switch_apply_impl(state: dict[str, Any], group: dict[str, Any]) -> bool: + """結果を残さなかった群の担当を、次の輪番の別の担当へ替える。替えたら真。 + + **1 つ進めるだけにしない。** 輪番は参加者の数で 1 周するため、1 つ先が同じ担当 + になることがある。その群で失敗した担当のどれとも違う担当が出た最初の番号を採る。 + 参加者の数だけ進めても出なければ、替える先が無い（#728 の決定 8）。 + """ + tried = { + record.get("impl") for record in (group.get("failed_attempts") or []) + if record.get("phase") == "apply" + } + tried.add(group.get("impl")) + seq = safe_int(state.get("apply_seq")) + for _ in range(len(state.get("impl_capable") or []) or 4): + seq += 1 + impl, requested = impl_for_seq(state, seq) + if impl in tried: + continue + state["apply_seq"] = seq + group["impl"] = impl + group["impl_model"] = {"requested": requested, "observed": None} + info(f"↻ 適用ラウンド {group['apply_round']} の担当を {impl} へ替えます") + return True + return False + + +def _no_result_reason(group: dict[str, Any]) -> str: + """見送りの理由。どの担当がどの理由で結果を残さなかったかを並べる。""" + trail = " → ".join( + f"{record.get('impl')}: {record.get('reason')}" + for record in (group.get("failed_attempts") or []) + if record.get("phase") == "apply" + ) + return f"実装担当が結果を残しませんでした（{trail}）" + + +def _drop_group_without_result(ctx: _ApplyExecutionContext) -> None: + """結果を残せないまま上限に達した群を取り消し、項目を見送りへ入れる。""" + reason = _no_result_reason(ctx.group) + info(f"❌ {reason}") + for item_id in ctx.group["items"]: + item = find_item(ctx.state, item_id, required=False) + if item is None: + continue + item["status"] = "abandoned" + item["failure_reason"] = reason + ctx.group["status"] = "dropped" + ctx.group["drop_reason"] = "no_result" + ctx.entry["apply"] = { + "apply_round": ctx.group["apply_round"], + "applied": [], "failed": list(ctx.group["items"]), + "base_sha": ctx.entry.get("apply_base_sha"), + "head_sha": None, + "merged_at": statefile.now(), + } + ctx.state["phase"] = phase_after_group(ctx.entry) + _defer_abandoned_items(ctx.state, ctx.group) + + +def _close_failed_attempt( + ctx: _ApplyExecutionContext, scope: IntakeScope, outcome: Any +) -> None: + """適用担当が結果を残さなかった試行を閉じる。**必ず終了する。** + + 取り消しと記録を共通の手順へ通したあと、開き直しの判定で担当を替えるか群ごと + 取り消すかを決める。替える先が無いときだけ、起動し直しの可否で決める。 + """ + if ctx.args.dry_run: + info("（dry-run）適用結果がありません。状態ファイルは更新していません") + sys.exit(2) + closed = close_without_result(ctx.path, ctx.state, scope, outcome) + if closed.range_unknown: _block_group_items(ctx) die( - f"着手前のテストが成功と確認できていません（status={baseline.get('status')}）。" - "適用へ着手しません（全項目を blocked）", - code=2, + "適用の範囲を確定できませんでした" + f"（起点 {ctx.entry.get('apply_base_sha')}）。検証できない適用は採りません", + code=4, ) + ctx.group["attempt"] = scope.attempt + drop = group_reopening(ctx.group) != "open" + if not drop and not _switch_apply_impl(ctx.state, ctx.group): + # 替える先が無い。起動し直しても解けない結末（利用上限）なら、同じ担当で + # もう一度起動しても待ちと相手の枠を使うだけなので、ここで取り消す。 + drop = not closed.relaunch_same_agent + if drop: + _drop_group_without_result(ctx) + statefile.save(ctx.path, ctx.state) + sys.exit(2) + + +def _load_apply_context( + ctx: _ApplyExecutionContext, payload: dict[str, Any], +) -> tuple[dict[str, Any], _ApplyCommitRange]: + impl = ctx.group.get("impl") or ctx.entry["impl"] + record_observed_model(ctx.entry, "impl", impl, ctx.state, "apply", ctx.args.round) # 検証の材料は git から取る。結果ファイルから使うのは # 「どのコミットがこの群のものか」という対応付けだけ。 @@ -479,7 +665,7 @@ def _load_apply_context( "適用の範囲を確定できませんでした" f"（起点 {ctx.entry.get('apply_base_sha')} / HEAD {head_sha}）。" "検証できない適用は採りません", - code=2, + code=4, ) return payload, _ApplyCommitRange(work, head_sha, ordered_range, in_range) @@ -560,23 +746,12 @@ def _revert_unverified_apply_round( ) -> None: """検証を通らない適用ラウンドの範囲を取り消し、状態と公開を反映する。""" # 範囲全体を取り消す。どのコミットが安全かを決められない以上、 - # 起点まで戻すのが最も確実である。順序は `revert_item_commits` が - # git の履歴から決め直す。 - whole_round = { - "item_id": f"R{ctx.entry['round']}-A{ctx.group['apply_round']}", - "commits": list(commit_range.ordered_range), - } - if not ctx.args.dry_run: - # **取り消しへ着手する前に印を立てる。** 取り消しは済んだのに push - # できずに終わると、未検証の変更が Pull Request に残ったままになる。 - ctx.entry["pending_push"] = True - statefile.save(ctx.path, ctx.state) - revert_item_commits(ctx.state, whole_round, ctx.args.dry_run) - if not ctx.args.dry_run: - # 取り消し後の状態を新しい起点にする。叩き直しても範囲が空になり、 - # 取り消しコミット自体を「未割当」として再び戻すことがない。 - ctx.entry["apply_base_sha"] = git_out(commit_range.work, ["rev-parse", "HEAD"]) - ctx.group["base_sha"] = ctx.entry["apply_base_sha"] + # 起点まで戻すのが最も確実である。取り消しの本体は 3 つの取り込みで共有する + # （`intake.discard_unverified`）。印を立てる順序も起点の更新もそちらが持つ。 + discard_unverified( + ctx.path, ctx.state, _apply_scope(ctx), commit_range.ordered_range, + dry_run=ctx.args.dry_run, + ) ctx.entry["apply"] = { "apply_round": ctx.group["apply_round"], "applied": [], "failed": list(ctx.group["items"]), @@ -662,22 +837,17 @@ def _record_apply_progress( }) -def _verify_apply_group( +def _collect_apply_group_facts( ctx: _ApplyExecutionContext, commit_range: _ApplyCommitRange, reported: dict[str, dict[str, Any]], -) -> tuple[list[str], list[str]]: - """適用ラウンドをまとめて検証し `(採用, 失敗)` を返す。 +) -> tuple[list[str], list[str], list[dict[str, Any]]]: + """群の申告から `(欠落項目, 申告 SHA, コミット事実)` を組み立てる。 - **判定は全件同時である**（決定 3）。群の中は 1 コミットなので、失敗を項目まで - 特定しても取り消しは分離できない。 + **群の全項目が同じコミットを申告する。** 申告の無い項目は、適用されたことを + 確かめる手がかりが無い。群の中は 1 コミットなので、1 件の欠落が群の全件を + 巻き込む（「群の中の道連れ」）。 """ - scope = ctx.state.get("target_scope") or [] - items = [find_item(ctx.state, i) for i in ctx.group["items"]] - - # **群の全項目が同じコミットを申告する。** 申告の無い項目は、適用されたことを - # 確かめる手がかりが無い。群の中は 1 コミットなので、1 件の欠落が群の全件を - # 巻き込む（「群の中の道連れ」）。 missing = [ i for i in ctx.group["items"] if not reported_shas(reported.get(i) or {}) ] @@ -688,14 +858,33 @@ def _verify_apply_group( commit_range.work, shas, commit_range.in_range, "", ctx.state["head_branch"], safe_int(ctx.state.get("test_timeout"), DEFAULT_TEST_TIMEOUT), ) + return missing, shas, facts + + +def _determine_apply_problem( + ctx: _ApplyExecutionContext, + items: list[dict[str, Any]], + missing: list[str], + facts: list[dict[str, Any]], +) -> str: + """欠落と `verify_apply_round` から、この適用ラウンドの問題点を決める。""" if missing: - problem = ( + return ( f"適用結果に項目がありません: {', '.join(missing)}" "（群の全項目を 1 つのコミットへまとめ、各項目へ同じ SHA を申告します）" ) - else: - problem = verify_apply_round(items, facts, scope) + scope = ctx.state.get("target_scope") or [] + return verify_apply_round(items, facts, scope) + +def _record_apply_group_outcome( + ctx: _ApplyExecutionContext, + items: list[dict[str, Any]], + shas: list[str], + facts: list[dict[str, Any]], + problem: str, +) -> None: + """保留判断・項目状態・進捗を記録し、結果を出力して保存する。""" # **機械で決まらなかったテストの差分を記録する**（#443）。落とさないが、 # 通ったものとしても扱わない。進行側がこれを見て段 2（`judge-test-changes`）を # 起動する。**空でないまま収束させない。** @@ -718,6 +907,22 @@ def _verify_apply_group( ) if not ctx.args.dry_run: statefile.save(ctx.path, ctx.state) + + +def _verify_apply_group( + ctx: _ApplyExecutionContext, + commit_range: _ApplyCommitRange, + reported: dict[str, dict[str, Any]], +) -> tuple[list[str], list[str]]: + """適用ラウンドをまとめて検証し `(採用, 失敗)` を返す。 + + **判定は全件同時である**（決定 3）。群の中は 1 コミットなので、失敗を項目まで + 特定しても取り消しは分離できない。 + """ + items = [find_item(ctx.state, i) for i in ctx.group["items"]] + missing, shas, facts = _collect_apply_group_facts(ctx, commit_range, reported) + problem = _determine_apply_problem(ctx, items, missing, facts) + _record_apply_group_outcome(ctx, items, shas, facts, problem) if problem: return [], list(ctx.group["items"]) return list(ctx.group["items"]), [] diff --git a/plugins/ndf/skills/cross-refactoring/scripts/refactor_lib/commands/converge.py b/plugins/ndf/skills/cross-refactoring/scripts/refactor_lib/commands/converge.py index b4de4423..ecbeff79 100644 --- a/plugins/ndf/skills/cross-refactoring/scripts/refactor_lib/commands/converge.py +++ b/plugins/ndf/skills/cross-refactoring/scripts/refactor_lib/commands/converge.py @@ -11,7 +11,7 @@ import hashlib import pathlib import sys -from typing import Any +from typing import Any, Optional import statefile @@ -31,7 +31,12 @@ collect_commit_facts, commits_in_range, resolved_threads_on_github, - revert_unverified_range, +) +from ..intake import ( + IntakeScope, + already_closed, + close_without_result, + discard_unverified, ) from ..outbound import dropped_line, item_lines, plan_line from ..paths import git_out, load_state, result_path, stem_for @@ -178,6 +183,22 @@ def cmd_should_abandon(args: argparse.Namespace) -> None: sys.exit(2) +def _record_deferred_abandoned_items( + state: dict[str, Any], targets: list[str] +) -> None: + """取り消し対象項目の status を abandoned に更新し、未登録なら deferred_items に追記する。""" + already = {d.get("item_id") for d in state["deferred_items"]} + for item_id in targets: + item = find_item(state, item_id) + item["status"] = "abandoned" + item.setdefault( + "failure_reason", "修正ラウンドの上限に達してもテストが通らなかった") + if item_id in already: + continue + state["deferred_items"].append( + deferred_record(item, item_id, item["failure_reason"])) + + def cmd_abandon_items(args: argparse.Namespace) -> None: """Step 6 — テストが通らなかった適用ラウンドを取り消す。 @@ -219,16 +240,7 @@ def cmd_abandon_items(args: argparse.Namespace) -> None: run_drop(path, state, entry, targets) - already = {d.get("item_id") for d in state["deferred_items"]} - for item_id in targets: - item = find_item(state, item_id) - item["status"] = "abandoned" - item.setdefault( - "failure_reason", "修正ラウンドの上限に達してもテストが通らなかった") - if item_id in already: - continue - state["deferred_items"].append( - deferred_record(item, item_id, item["failure_reason"])) + _record_deferred_abandoned_items(state, targets) # 見送りの記録と印の解除を**同じ保存で**行う。保存してから push するので、 # push が失敗しても記録とローカルの git が食い違わない。 @@ -369,21 +381,17 @@ def _record_accepted_fix_commits( def _revert_invalid_fix_round( path: pathlib.Path, state: dict[str, Any], - entry: dict[str, Any], + scope: IntakeScope, ordered_range: list[str], ) -> set[str]: """検証を通らない修正ラウンドの範囲を取り消し、採用する解決スレッドを返す。 取り消した以上、解決の申告も採らないので**常に空集合を返す**。 """ - # 取り消しの本体は最終ゲートと共有する（`revert_unverified_range`）。控えの - # 形が同じなので、適用ラウンドと最終ゲートで別々に持たない。 + # 取り消しの本体は 3 つの取り込みで共有する（`intake.discard_unverified`）。 # **push は保存のあと。** ここで push して失敗すると、取り消しコミットは # ローカルに残るのに起点の更新が保存されず、叩き直しで二重に取り消してしまう。 - revert_unverified_range( - path, state, entry, ordered_range, - f"R{entry['round']}-fix{entry['fix_rounds'] + 1}", - ) + discard_unverified(path, state, scope, ordered_range) info("⚠ 修正を取り消したため、解決の申告は採用しません") return set() @@ -452,6 +460,7 @@ def _settle_fix_round( path: pathlib.Path, state: dict[str, Any], entry: dict[str, Any], + scope: IntakeScope, ordered_range: list[str], resolved: set[str], unassigned: list[str], @@ -460,44 +469,111 @@ def _settle_fix_round( ) -> None: """検証結果に応じて修正ラウンドを取り消すか受理し、解決の印を付ける。""" if unassigned or problems: - resolved = _revert_invalid_fix_round(path, state, entry, ordered_range) + resolved = _revert_invalid_fix_round(path, state, scope, ordered_range) else: _record_accepted_fix_commits(state, accepted) _mark_resolved_fix_findings(entry, resolved) -def cmd_merge_fix(args: argparse.Namespace) -> None: - """Step 6 — 修正結果を取り込み、修正ラウンドを 1 つ進める。""" - path, state = load_state(args.id) - entry = round_of(state, args.round) - discard_impl_leftovers(state, state["worktrees"]["work"]) - flush_pending_push(path, state, entry) - impl = entry["impl"] - result = result_path(state, impl, stem_for(impl, "fix", state["id"], args.round)) - payload = read_result(result, impl) +def _fix_scope(entry: dict[str, Any], impl: str) -> IntakeScope: + """修正の取り込み 1 回分の範囲の値。 - work = state["worktrees"]["work"] - head_now = git_out(work, ["rev-parse", "HEAD"]) or "" + 起点と公開の保留の印は提案ラウンドの控えが持ち、結末の記録は**その群**が持つ。 + 記録を群に置くのは、担当が群ごとに決まるためである。 + """ + return IntakeScope( + holder=entry, + base_key="fix_base_sha", + records=current_group(entry), + phase="fix", + attempt=safe_int(entry.get("fix_attempts")), + impl=impl, + label=f"R{entry['round']}-fix{safe_int(entry.get('fix_rounds')) + 1}", + ) + + +def _close_failed_fix( + path: pathlib.Path, + state: dict[str, Any], + entry: dict[str, Any], + scope: IntakeScope, + outcome: Any, +) -> None: + """修正の担当が結果を残さなかったときに、取り消して修正ラウンドを進める。 + + **必ず修正ラウンドを進める。** 進めないと見送りの判定が上限に達する条件を + 満たさず、検証と修正を往復し続ける（#647）。起動し直しても解けない結末 + （利用上限）では上限の値まで進め、次の判定で見送りへ移す（#728 の決定 10）。 + """ + closed = close_without_result(path, state, scope, outcome) + if closed.range_unknown: + entry["fix_rounds"] = safe_int(entry.get("fix_rounds")) + 1 + statefile.save(path, state) + die( + "修正の範囲を確定できませんでした" + f"（起点 {entry.get('fix_base_sha')}）。検証できない修正は採りません", + code=2, + ) + limit = safe_int(state.get("max_fix_rounds"), 3) + if closed.relaunch_same_agent: + entry["fix_rounds"] = safe_int(entry.get("fix_rounds")) + 1 + else: + entry["fix_rounds"] = limit + statefile.save(path, state) + info(f"修正ラウンド {entry['fix_rounds']} / {limit}") + sys.exit(2) + + +def _fetch_fix_result( + path: pathlib.Path, state: dict[str, Any], entry: dict[str, Any], + scope: IntakeScope, impl: str, round_no: int, +) -> tuple[Optional[dict[str, Any]], Optional[str]]: + """修正結果を取得し、`(payload, merge_key)` を返す。 + + 結果を残さなかった試行は `_close_failed_fix` が終了させる。取り込み済みの + 結果なら `(None, None)` を返し、呼び出し側が何もせず戻れるようにする。 + """ + outcome = read_result(state, impl, "fix", round_no) + if outcome.payload is None: + _close_failed_fix(path, state, entry, scope, outcome) + payload = outcome.payload + + result = result_path(state, impl, stem_for(impl, "fix", state["id"], round_no)) merge_key = _fix_merge_key(entry, result) if _already_merged_fix_result(entry, merge_key): - return - merged_keys = entry["fix_merged_keys"] + return None, None + return payload, merge_key - resolved = _resolved_fix_thread_ids(payload, state["repo"], state["current_pr"]) +def _confirm_and_settle_fix( + path: pathlib.Path, state: dict[str, Any], entry: dict[str, Any], + scope: IntakeScope, work: str, head_now: str, payload: dict[str, Any], +) -> set[str]: + """Git 範囲を確定し、修正コミットを検証して取り消すか受理する。 + + 採用した解決スレッドの集合を返す（取り込みの通知に使う）。 + """ + resolved = _resolved_fix_thread_ids(payload, state["repo"], state["current_pr"]) baseline = state.get("baseline_test") or {} ordered_range = _resolve_fix_range(path, state, entry, work, head_now) unassigned, problems, accepted = _inspect_fix_commits( state, work, payload, baseline, ordered_range ) _settle_fix_round( - path, state, entry, ordered_range, resolved, unassigned, problems, accepted + path, state, entry, scope, ordered_range, resolved, unassigned, problems, + accepted, ) + return resolved - merged_keys.append(merge_key) - entry["fix_rounds"] += 1 +def _record_and_publish_fix( + path: pathlib.Path, state: dict[str, Any], entry: dict[str, Any], + merge_key: str, payload: dict[str, Any], resolved: set[str], +) -> None: + """取り込み済みの鍵・修正回数・所要時間を記録し、保存して公開する。""" + entry["fix_merged_keys"].append(merge_key) + entry["fix_rounds"] += 1 entry.setdefault("durations", {})["fix"] = ( entry.get("durations", {}).get("fix", 0) + safe_int(payload.get("elapsed_seconds")) @@ -510,3 +586,36 @@ def cmd_merge_fix(args: argparse.Namespace) -> None: f"修正を取り込みました（解決 {len(resolved)} スレッド / " f"修正ラウンド {entry['fix_rounds']}）。{plan_line(state)}" ) + + +def cmd_merge_fix(args: argparse.Namespace) -> None: + """Step 6 — 修正結果を取り込み、修正ラウンドを 1 つ進める。 + + 終了コード: 0 = 取り込んだ / 2 = 範囲を確定できない、または担当が結果を + 残さなかった（どちらも修正ラウンドは進む） / 4 = 群が無い。 + """ + path, state = load_state(args.id) + entry = round_of(state, args.round) + discard_impl_leftovers(state, state["worktrees"]["work"]) + flush_pending_push(path, state, entry) + # **担当は群から読む。** 骨組みが起動するのは群の担当であり、提案ラウンドの + # 担当とは限らない。食い違うと結果ファイルを一度も引けない（#728 の決定 10）。 + group = current_group(entry) + impl = group.get("impl") or entry["impl"] + scope = _fix_scope(entry, impl) + if already_closed(scope): + info("↻ この修正の試行は結果なしとして記録済みです") + sys.exit(2) + + payload, merge_key = _fetch_fix_result( + path, state, entry, scope, impl, args.round + ) + if payload is None: + return + + work = state["worktrees"]["work"] + head_now = git_out(work, ["rev-parse", "HEAD"]) or "" + resolved = _confirm_and_settle_fix( + path, state, entry, scope, work, head_now, payload + ) + _record_and_publish_fix(path, state, entry, merge_key, payload, resolved) diff --git a/plugins/ndf/skills/cross-refactoring/scripts/refactor_lib/commands/gate.py b/plugins/ndf/skills/cross-refactoring/scripts/refactor_lib/commands/gate.py index e42eb409..08596df4 100644 --- a/plugins/ndf/skills/cross-refactoring/scripts/refactor_lib/commands/gate.py +++ b/plugins/ndf/skills/cross-refactoring/scripts/refactor_lib/commands/gate.py @@ -15,10 +15,10 @@ from __future__ import annotations import argparse +import pathlib import sys from typing import Any, Optional -import assignment import statefile from .. import die, info @@ -33,9 +33,15 @@ check_run_result, collect_commit_facts, commits_in_range, - revert_unverified_range, ) -from ..paths import git_out, load_state, result_path, stem_for +from ..intake import ( + IntakeScope, + already_closed, + close_without_result, + discard_unverified, +) +from ..paths import git_out, load_state +from ..rounds import impl_for_seq from ..verify import verify_final_fix_commit from ..vocabulary import DEFAULT_TEST_TIMEOUT from ..verify import unassigned_fix_commits @@ -125,12 +131,57 @@ def _final_fix_impl(state: dict[str, Any], gate: dict[str, Any]) -> str: if impl: return impl seq = safe_int(state.get("apply_seq")) + 1 - impl, _ = assignment.assign(seq, state["host"]) + impl, _ = impl_for_seq(state, seq) state["apply_seq"] = seq gate["impl"] = impl return impl +def _final_fix_scope(gate: dict[str, Any], impl: str) -> IntakeScope: + """最終ゲートの修正の取り込み 1 回分の範囲の値。 + + 起点も結末の記録も最終ゲートの控えが持つ。改善項目にも提案ラウンドにも + 属さないため、群は関わらない。 + """ + rounds = safe_int(gate.get("fix_rounds")) + return IntakeScope( + holder=gate, + base_key="fix_base_sha", + records=gate, + phase="final-fix", + attempt=rounds, + impl=impl, + label=f"final-gate-fix{rounds}", + ) + + +def _close_failed_final_fix( + path: pathlib.Path, + state: dict[str, Any], + gate: dict[str, Any], + scope: IntakeScope, + outcome: Any, +) -> None: + """最終ゲートの修正担当が結果を残さなかったときに、取り消して判定へ戻す。 + + **修正ラウンドは進めない。** 進めるのは次の最終ゲートで、そこが上限を見る。 + 起動し直しても解けない結末（利用上限）だけは上限の値まで進め、次の最終ゲートを + 「取り消さず報告」で終わらせる（#728 の決定 11）。 + """ + closed = close_without_result(path, state, scope, outcome) + if closed.range_unknown: + statefile.save(path, state) + die( + "最終ゲートの修正の範囲を確定できませんでした" + f"（起点 {gate.get('fix_base_sha')}）。検証できない修正は採りません", + code=2, + ) + if not closed.relaunch_same_agent: + gate["fix_rounds"] = safe_int(state.get("max_fix_rounds"), 3) + statefile.save(path, state) + sys.exit(2) + + def cmd_merge_final_fix(args: argparse.Namespace) -> None: """Step 7 — 最終ゲートの修正結果を取り込む。 @@ -144,8 +195,13 @@ def cmd_merge_final_fix(args: argparse.Namespace) -> None: | 正常なコミットまで取り消される | 古い起点が残っていると、そこから HEAD までが範囲になる | | トレーラーが揃わず全件が不正になる | `Item-Id` を要求するが、最終ゲートの修正は項目に属さない | - 終了コード: 0 = 取り込んだ / 2 = 取り込めなかった（範囲を確定できない）。 - 合否そのものは判定せず、**次の `final-gate` が採った側で 1 度だけ見る**。 + 終了コード: 0 = 取り込んだ / 2 = 取り込めなかった（範囲を確定できない、または + 担当が結果を残さなかった）。合否そのものは判定せず、**次の `final-gate` が + 採った側で 1 度だけ見る**。 + + **結果を残さなかったときも、作られたコミットは取り消す。** 取り消さずに抜けると、 + 次の最終ゲートがそのコミットを含む先端でテストし、落ちれば起点をそこへ置き直す。 + 未検証の差分が Pull Request に残る（#674）。 """ path, state = load_state(args.id) gate = state.setdefault("final_gate", {"fix_rounds": 0, "checks": []}) @@ -161,8 +217,15 @@ def cmd_merge_final_fix(args: argparse.Namespace) -> None: discard_impl_leftovers(state, work) flush_pending_push(path, state, gate) - result = result_path(state, impl, stem_for(impl, "final-fix", state["id"])) - payload = read_result(result, impl) + scope = _final_fix_scope(gate, impl) + if already_closed(scope): + info("↻ この最終ゲートの修正の試行は結果なしとして記録済みです") + sys.exit(2) + + outcome = read_result(state, impl, "final-fix") + if outcome.payload is None: + _close_failed_final_fix(path, state, gate, scope, outcome) + payload = outcome.payload head_now = git_out(work, ["rev-parse", "HEAD"]) or "" ordered_range = commits_in_range(work, gate.get("fix_base_sha"), head_now) if ordered_range is None: @@ -200,10 +263,7 @@ def cmd_merge_final_fix(args: argparse.Namespace) -> None: # **ここは取り消す。** 「上限に達しても取り消さない」のは*採用した改善項目* # の話で、検証を受けていない修正コミットは別である。取り消せば HEAD は # 最終ゲートが見た地点へ戻り、公開済みの内容と食い違わない。 - revert_unverified_range( - path, state, gate, ordered_range, - f"final-gate-fix{safe_int(gate.get('fix_rounds'))}", - ) + discard_unverified(path, state, scope, ordered_range) else: gate["fix_base_sha"] = head_now gate.setdefault("fix_commits", []).extend(ordered_range) diff --git a/plugins/ndf/skills/cross-refactoring/scripts/refactor_lib/commands/report.py b/plugins/ndf/skills/cross-refactoring/scripts/refactor_lib/commands/report.py index 06cc9626..955bfea4 100644 --- a/plugins/ndf/skills/cross-refactoring/scripts/refactor_lib/commands/report.py +++ b/plugins/ndf/skills/cross-refactoring/scripts/refactor_lib/commands/report.py @@ -116,6 +116,28 @@ def cmd_status(args: argparse.Namespace) -> None: def cmd_report(args: argparse.Namespace) -> None: """Step 8 — ラウンド表・項目表・見送り項目・指標を出す。""" path, state = load_state(args.id) + _print_header(state) + print() + print("## ラウンド") + print() + print(_round_table(state)) + print() + print("## 改善項目") + print() + print(_item_table(state)) + # **取り消した項目の内訳は書かない**（#436 決定 6-b）。件数だけ述べ、内訳は + # 改修計画へ譲る。同じ一覧を 2 か所に置くと、片方だけが古くなる。 + print() + _print_deferred(state) + if args.metrics: + _print_metrics(state) + # **最後の行に置く**（#662 の AC23）。作業ツリーを消した後に要約を探す手がかりになる。 + print() + _print_run_metrics(path, state) + + +def _print_header(state: dict[str, Any]) -> None: + """見出し行と実行メタ情報（対象範囲・終了理由・改修計画・着手前テスト等）を出す。""" print(f"# cross-refactoring 実行報告 — {state['repo']} #{state['current_pr']}") print() print(f"- ホスト: {state['host']}（{state['host_detection']}）") @@ -134,28 +156,26 @@ def cmd_report(args: argparse.Namespace) -> None: print(f"- 最終ゲート: {gate.get('mode') or '—'}" f"（{gate.get('status') or '未実行'}" f" / 修正 {gate.get('fix_rounds', 0)} 回）") - print() - print("## ラウンド") - print() - print(_round_table(state)) - print() - print("## 改善項目") - print() - print(_item_table(state)) - # **取り消した項目の内訳は書かない**（#436 決定 6-b）。件数だけ述べ、内訳は - # 改修計画へ譲る。同じ一覧を 2 か所に置くと、片方だけが古くなる。 - print() + + +def _print_deferred(state: dict[str, Any]) -> None: + """見送り節（件数と改修計画への参照）を出す。""" print("## 見送った提案") print() print(f"- 件数: {len(state['deferred_items'])} 件") print(f"- 内訳: 改修計画にある — {plan_reference(state)}") - if args.metrics: - print() - print("# 指標") - print() - print(metrics_lib.format_report(metrics_lib.aggregate(state))) - # **最後の行に置く**（#662 の AC23）。作業ツリーを消した後に要約を探す手がかりになる。 + + +def _print_metrics(state: dict[str, Any]) -> None: + """指標節を出す（`args.metrics` が真のときだけ呼ぶ）。""" + print() + print("# 指標") print() + print(metrics_lib.format_report(metrics_lib.aggregate(state))) + + +def _print_run_metrics(path: pathlib.Path, state: dict[str, Any]) -> None: + """run_metrics の要約 1 行を出す。""" print(run_metrics.report_line(path, state, "cross-refactoring", summary_extra)) diff --git a/plugins/ndf/skills/cross-refactoring/scripts/refactor_lib/commands/setup.py b/plugins/ndf/skills/cross-refactoring/scripts/refactor_lib/commands/setup.py index 3b4a671d..011ec37a 100644 --- a/plugins/ndf/skills/cross-refactoring/scripts/refactor_lib/commands/setup.py +++ b/plugins/ndf/skills/cross-refactoring/scripts/refactor_lib/commands/setup.py @@ -21,7 +21,7 @@ import statefile from .. import ABORT, die, info -from ..gitfacts import run_with_timeout +from ..gitfacts import run_with_timeout, safe_int from ..paths import ( default_worktree_base, load_state, @@ -35,6 +35,7 @@ from ..scope import require_scope_covers_tests from ..vocabulary import ( DEFAULT_TEST_TIMEOUT, + IMPL_STALL_MARGIN, REQUIRED_SKILLS, test_vocabulary, vocabulary, @@ -355,6 +356,12 @@ def _emit_init(state: dict[str, Any]) -> None: HEAD_BRANCH=state["head_branch"], BASE_BRANCH=state["base_branch"], SCOPE=" ".join(state["target_scope"]), + # 適用・修正・最終ゲートの修正の担当はテストを 1 回実行し、その間は何も + # 出力しない。テストの制限時間そのままでは、実行中に打ち切られる（#553）。 + IMPL_STALL_TIMEOUT=( + safe_int(state.get("test_timeout"), DEFAULT_TEST_TIMEOUT) + + IMPL_STALL_MARGIN + ), ) diff --git a/plugins/ndf/skills/cross-refactoring/scripts/refactor_lib/gitfacts.py b/plugins/ndf/skills/cross-refactoring/scripts/refactor_lib/gitfacts.py index d2f79144..740b49f0 100644 --- a/plugins/ndf/skills/cross-refactoring/scripts/refactor_lib/gitfacts.py +++ b/plugins/ndf/skills/cross-refactoring/scripts/refactor_lib/gitfacts.py @@ -7,6 +7,7 @@ import json import os import pathlib +import re import shutil import signal import subprocess @@ -15,6 +16,7 @@ import models as models_lib import statefile +from monitor_outcome import LaunchOutcome, read_launch_outcome from . import die, info from .paths import git_out, sh, stem_for @@ -97,14 +99,49 @@ def commit_trailers(work: str, sha: str) -> dict[str, str]: **結果ファイルの `trailers` は使わない。** JSON 上は仕様どおりでも、実際の `git commit` でトレーラーを書き忘れていれば集計に使えない。 + + **末尾の段落から前へ 1 段落ずつ読む**（#553）。実行環境が帰属の段落を後ろへ + 足すと、git の標準の読み方は最後の段落しか見ないため必須の記名が読めなくなる。 + トレーラーの段落と判定しなかった段落で止めるので、散文の中にある記名の形の行は + 拾わない。同じ鍵が 2 つの段落にあれば、末尾に近い段落の値を採る。 + + **1 段落目（題名）は掛けない。** 掛けると `Round: 本文の題名` の形の題名を + トレーラーとして読む。 """ - out = git_out(work, ["log", "-1", "--format=%(trailers:only,unfold)", sha]) + body = git_out(work, ["log", "-1", "--format=%B", sha], strip=False) + paragraphs = re.split(r"\n[ \t]*\n", (body or "").strip("\n")) trailers: dict[str, str] = {} - for line in (out or "").splitlines(): + for paragraph in reversed(paragraphs[1:]): + parsed = _parse_trailer_paragraph(paragraph) + if not parsed: + break + for key, value in parsed.items(): + trailers.setdefault(key, value) + return trailers + + +def _parse_trailer_paragraph(paragraph: str) -> dict[str, str]: + """1 つの段落を git の判定に掛け、トレーラーの段落なら鍵と値を返す。 + + **題名の行を補って渡す。** git はメッセージの 1 行目を題名として読むため、 + 段落だけを渡すと何も返らない（git 2.53.0 で実測）。判定そのものは git に委ね、 + 「何行以上なら記名の段落か」といった規則をこちら側に持たない。 + """ + if not paragraph.strip(): + return {} + result = subprocess.run( + ["git", "interpret-trailers", "--parse"], + input=f"subject\n\n{paragraph}\n", + capture_output=True, text=True, + ) + if result.returncode != 0: + return {} + parsed: dict[str, str] = {} + for line in result.stdout.splitlines(): key, sep, value = line.partition(":") if sep: - trailers[key.strip()] = value.strip() - return trailers + parsed[key.strip()] = value.strip() + return parsed def commit_diff_lines(work: str, sha: str) -> int: @@ -372,6 +409,43 @@ def resolved_threads_on_github(repo: str, pr: int) -> Optional[set[str]]: CHECK_RUNS_PER_PAGE = 100 +def _parse_check_runs(raw_json: Optional[str]) -> Optional[list[dict[str, Any]]]: + """API 出力から check_runs のリストを検証して返す。""" + if not raw_json: + return None + try: + body = json.loads(raw_json) + except json.JSONDecodeError: + return None + runs = body.get("check_runs") if isinstance(body, dict) else None + if not isinstance(runs, list): + return None + return [r for r in runs if isinstance(r, dict)] + + +def _filter_check_runs_by_name( + runs: list[dict[str, Any]], name: str +) -> list[dict[str, Any]]: + """名前が一致する run を選別する。""" + return [ + r for r in runs + if str(r.get("name") or "") == name + ] + + +def _aggregate_check_run_results(matched: list[dict[str, Any]]) -> Optional[str]: + """matched runs を pending・失敗結論・success の順で集約する。""" + if not matched: + return None + if any(str(r.get("status") or "").lower() != "completed" for r in matched): + return "pending" + for run in matched: + conclusion = str(run.get("conclusion") or "").lower() + if conclusion != "success": + return conclusion or "unknown" + return "success" + + def check_run_result(repo: str, sha: str, name: str) -> Optional[str]: """名前が一致した検査ジョブの結果を 1 つの語で返す。 @@ -390,28 +464,11 @@ def check_run_result(repo: str, sha: str, name: str) -> Optional[str]: f"?per_page={CHECK_RUNS_PER_PAGE}"], check=False, ) - if not out: - return None - try: - body = json.loads(out) - except json.JSONDecodeError: - return None - runs = body.get("check_runs") if isinstance(body, dict) else None - if not isinstance(runs, list): + runs = _parse_check_runs(out) + if runs is None: return None - matched = [ - r for r in runs - if isinstance(r, dict) and str(r.get("name") or "") == name - ] - if not matched: - return None - if any(str(r.get("status") or "").lower() != "completed" for r in matched): - return "pending" - for run in matched: - conclusion = str(run.get("conclusion") or "").lower() - if conclusion != "success": - return conclusion or "unknown" - return "success" + matched = _filter_check_runs_by_name(runs, name) + return _aggregate_check_run_results(matched) def revert_item_commits( @@ -450,40 +507,6 @@ def revert_item_commits( return len(shas) -def revert_unverified_range( - path: pathlib.Path, - state: dict[str, Any], - entry: dict[str, Any], - ordered_range: list[str], - label: str, -) -> None: - """検証を通らない範囲を取り消し、`entry` の起点を取り消し後の HEAD へ進める。 - - `entry` は**修正の控えを持つ辞書**である。適用ラウンドの控え（`rounds[]` の - 要素）と最終ゲートの控え（`final_gate`）の両方が同じ 3 つの鍵 - （`pending_push` / `fix_base_sha`）を持つため、どちらからも呼べる。 - `label` は取り消しの単位を人が読むための名前で、git の操作には効かない。 - """ - work = state["worktrees"]["work"] - # **状態へ記録する前に取り消す。** 先に記録すると、取り消し済みのコミットが - # 状態ファイルに残り、後の見送り処理が同じコミットをもう一度取り消そうとする。 - info("検証を通らない変更を残さないため、この修正ラウンドの範囲を取り消します") - # **取り消しへ着手する前に印を立てる。** 取り消しは済んだのに push できずに - # 終わると、未検証の変更が Pull Request に残ったままになる。 - entry["pending_push"] = True - statefile.save(path, state) - revert_item_commits( - state, - {"item_id": label, "commits": list(ordered_range)}, - dry_run=False, - ) - # 取り消し後の状態を新しい起点にし、**その場で保存する**。ここで保存せずに - # 落ちると、次の実行は古い起点から範囲を取り直して取り消しコミット自体を - # 「未申告」と判定し、**取り消しを取り消して**しまう。 - entry["fix_base_sha"] = git_out(work, ["rev-parse", "HEAD"]) - statefile.save(path, state) - - def _reset_hard(work: str, sha: Optional[str]) -> None: """着手前の HEAD へ戻す。半端な履歴を Pull Request に残さないための後始末。""" if sha: @@ -672,6 +695,22 @@ def _record_drop_result( "reverted": len(ordered), "replayed": len(mapping)} +def _drop_legacy_by_item( + state: dict[str, Any], pending: list[str], dry_run: bool = False, +) -> dict[str, Any]: + """起点を記録していない状態ファイル（旧版）で、項目のコミットだけを戻す。 + + 積み直しの起点（`apply_base_sha`）が無いため範囲を確定できない。従来どおり + 項目のコミットを新しい順に取り消すだけで、残す項目の積み直しは行わない。 + """ + info("⚠ 適用の範囲を確定できないため、項目のコミットだけを取り消します") + reverted = 0 + for item_id in pending: + reverted += revert_item_commits(state, find_item(state, item_id), dry_run) + return {"mode": "item", "dropped": pending, + "reverted": reverted, "replayed": 0} + + def drop_items( state: dict[str, Any], entry: dict[str, Any], drop_ids: list[str], dry_run: bool = False, @@ -704,13 +743,7 @@ def drop_items( ordered = commits_in_range(work, entry.get("apply_base_sha"), head or "HEAD") if ordered is None: # 起点を記録していない状態ファイル（旧版）では積み直せない。 - # 従来どおり項目のコミットだけを新しい順に戻す。 - info("⚠ 適用の範囲を確定できないため、項目のコミットだけを取り消します") - reverted = 0 - for item_id in pending: - reverted += revert_item_commits(state, find_item(state, item_id), dry_run) - return {"mode": "item", "dropped": pending, - "reverted": reverted, "replayed": 0} + return _drop_legacy_by_item(state, pending, dry_run) owner, keep_ids, replay = _drop_replay_plan(state, entry, pending, ordered) @@ -1079,26 +1112,22 @@ def find_item( return None -def read_result(path: pathlib.Path, runtime: str) -> dict[str, Any]: - """結果ファイルを読む。**JSON オブジェクトでなければ失敗させる。** +def read_result( + state: dict[str, Any], runtime: str, phase: str, round_no: Optional[int] = None +) -> LaunchOutcome: + """起動 1 回の結末を読む。**失敗しない。** - 配列や数値が返ってきたまま呼び出し側へ渡すと、`payload.get(...)` で - `AttributeError` になって進行が止まる。読み込みの時点で弾く。 + 結果ファイルの名前の幹をここで 1 度だけ組み、共通層（`read_launch_outcome`）へ + 渡す。3 つの取り込みが同じ組み立てを通るため、監視へ渡した名前の雛形 + （`--stem-template`）と食い違う幹で読むことがない。 + + **中断も出力もしない。** 結果を読めなかったときに何をするかは、読んだ側 + （取り込み）が終了コードとして決める。ここで `die` すると、未検証のコミットが + 取り消されないまま残る（#728）。 """ - if not path.exists(): - die(f"{runtime} の結果ファイルがありません: {path}", code=2) - try: - payload = json.loads(path.read_text(encoding="utf-8")) - except json.JSONDecodeError as e: - die(f"{runtime} の結果ファイルが JSON として読めません: {e}", code=2) - raise SystemExit(2) - if not isinstance(payload, dict): - die( - f"{runtime} の結果ファイルが JSON オブジェクトではありません" - f"（{type(payload).__name__}）: {path}", - code=2, - ) - return payload + return read_launch_outcome( + state["tmp_dir"], stem_for(runtime, phase, state["id"], round_no) + ) def record_observed_model( diff --git a/plugins/ndf/skills/cross-refactoring/scripts/refactor_lib/intake.py b/plugins/ndf/skills/cross-refactoring/scripts/refactor_lib/intake.py new file mode 100644 index 00000000..cb39b639 --- /dev/null +++ b/plugins/ndf/skills/cross-refactoring/scripts/refactor_lib/intake.py @@ -0,0 +1,175 @@ +"""取り込み 1 回分の共通手順（#728 の決定 4）。 + +適用・修正・最終ゲートの修正の 3 つの取り込みは、結果を読めたときも読めなかった +ときも同じことを行う。起点から HEAD までの範囲を確め、通らなければ取り消し、 +起点を取り消し後の HEAD へ進める。**違うのは起点の鍵と記録先だけ**なので、その差を +`IntakeScope` で受け取り、手順そのものはここ 1 か所に置く。 + +**`commands` 層に置かない。** 3 つのコマンドが読む層だからである（群の進行と同じ +理由。`commands` どうしの取り込みを作らない）。git の事実の読み取り +（`gitfacts`）にも置かない。取り消しは事実の読み取りではなく進行の手順である。 +""" +from __future__ import annotations + +from dataclasses import dataclass, field +import pathlib +from typing import Any, Optional + +import statefile + +from . import info +from .gitfacts import ( + commits_in_range, + push_with_retry_marker, + revert_item_commits, +) +from .paths import git_out + + +@dataclass +class IntakeScope: + """取り込み 1 つ分の「どこを見て、どこへ書くか」。 + + `holder` は公開の保留の印と起点を持つ辞書（提案ラウンドの控えか最終ゲートの + 控え）、`records` は結末の記録を持つ辞書（群か最終ゲートの控え）である。 + `mirror` は起点を同じ値に揃える辞書で、適用の取り込みだけが群を渡す。 + """ + + holder: dict[str, Any] + base_key: str + records: dict[str, Any] + phase: str + attempt: int + impl: str + label: str + mirror: Optional[dict[str, Any]] = None + + +@dataclass +class ClosedAttempt: + """結果なしを閉じた結果。 + + `range_unknown` が真のときは取り消しも記録も行っていない。**何で終わるかは + 呼び出し側が決める。** 適用は中断、修正と最終ゲートは既存の扱いへ戻す。 + """ + + reason: str + detail: str + reverted: int = 0 + relaunch_same_agent: bool = True + range_unknown: bool = False + tried: list[str] = field(default_factory=list) + + +def confirm_range(state: dict[str, Any], scope: IntakeScope) -> Optional[list[str]]: + """起点から HEAD までのコミットを新しい順で返す。確定できなければ `None`。 + + **空の配列と `None` を区別する。** 空は「1 件もコミットされていない」、`None` は + 「範囲を確定できなかった」である。混同すると、確定できないときに検査が素通りする。 + """ + work = state["worktrees"]["work"] + head = git_out(work, ["rev-parse", "HEAD"]) or "" + return commits_in_range(work, scope.holder.get(scope.base_key), head) + + +def discard_unverified( + path: pathlib.Path, + state: dict[str, Any], + scope: IntakeScope, + ordered_range: list[str], + dry_run: bool = False, +) -> int: + """検証を受けていない範囲を取り消し、起点を取り消し後の HEAD へ進める。 + + 順序は「印を立てて保存 → 新しい順に取り消す → 起点を書いて保存」である。 + **取り消しへ着手する前に印を立てる。** 取り消しは済んだのに公開できずに終わると、 + 未検証の変更が Pull Request に残ったままになる。**起点はその場で保存する。** + 保存せずに落ちると、次の実行が古い起点から範囲を取り直し、取り消しコミット自体を + 「未申告」と判定して取り消しを取り消してしまう。 + """ + if not ordered_range: + return 0 + info(f"検証を通らない変更を残さないため、{scope.label} の範囲を取り消します") + if not dry_run: + scope.holder["pending_push"] = True + statefile.save(path, state) + revert_item_commits( + state, + {"item_id": scope.label, "commits": list(ordered_range)}, + dry_run=dry_run, + ) + if not dry_run: + head = git_out(state["worktrees"]["work"], ["rev-parse", "HEAD"]) + scope.holder[scope.base_key] = head + if scope.mirror is not None: + scope.mirror["base_sha"] = head + statefile.save(path, state) + return len(ordered_range) + + +def already_closed(scope: IntakeScope) -> bool: + """この工程・この試行番号の結末を既に記録しているか。 + + **記録していれば結果ファイルを読まない。** 叩き直しのたびに読むと、後から + 現れた結果ファイルを、取り消し済みの範囲の申告として取り込んでしまう。 + """ + return any( + record.get("phase") == scope.phase and record.get("attempt") == scope.attempt + for record in (scope.records.get("failed_attempts") or []) + ) + + +def failed_impls(scope: IntakeScope) -> list[str]: + """この工程で結果を残さなかった担当を、記録の順に返す。""" + return [ + str(record.get("impl") or "") + for record in (scope.records.get("failed_attempts") or []) + if record.get("phase") == scope.phase + ] + + +def close_without_result( + path: pathlib.Path, + state: dict[str, Any], + scope: IntakeScope, + outcome: Any, +) -> ClosedAttempt: + """結果なしの起動を閉じる。範囲の確定 → 取り消し → 結末の記録の順で行う。 + + 記録は追記だけで、上書きしない。担当を替えると群の担当は書き換わるが、どの担当が + どの試行で失敗したかは記録から読める。 + """ + reason = str(outcome.reason or "missing") + detail = str(outcome.detail or "") + ordered_range = confirm_range(state, scope) + if ordered_range is None: + return ClosedAttempt( + reason=reason, + detail=detail, + relaunch_same_agent=bool(outcome.relaunch_same_agent), + range_unknown=True, + ) + reverted = discard_unverified(path, state, scope, ordered_range) + scope.records.setdefault("failed_attempts", []).append({ + "phase": scope.phase, + "attempt": scope.attempt, + "impl": scope.impl, + "reason": reason, + "detail": detail, + "at": statefile.now(), + "reverted": reverted, + }) + statefile.save(path, state) + if reverted: + push_with_retry_marker(path, state, scope.holder) + info( + f"⚠ {scope.impl} は結果を残しませんでした（{reason}）。" + f"取り消したコミットは {reverted} 件です" + ) + return ClosedAttempt( + reason=reason, + detail=detail, + reverted=reverted, + relaunch_same_agent=bool(outcome.relaunch_same_agent), + tried=failed_impls(scope), + ) diff --git a/plugins/ndf/skills/cross-refactoring/scripts/refactor_lib/proposals.py b/plugins/ndf/skills/cross-refactoring/scripts/refactor_lib/proposals.py index 21a71be1..69e3c5e8 100644 --- a/plugins/ndf/skills/cross-refactoring/scripts/refactor_lib/proposals.py +++ b/plugins/ndf/skills/cross-refactoring/scripts/refactor_lib/proposals.py @@ -21,6 +21,27 @@ ) +def _degrade_if_unknown( + value: str, + allowed: Iterable[str], + source: str, + label: str, + location: str, +) -> tuple[str, bool]: + """語彙集合に含まれない値を `unknown` へ降格する。 + + 降格したときは警告を出し `(unknown, True)` を返す。含まれていれば値をそのまま + `(value, False)` で返す。`smell` / `technique` / `severity` の同じ降格ルールと、 + テスト提案の `case` / `level` の降格を 1 箇所に集め、警告文や降格処理の変更が + 散らばらないようにする。`location` は警告に添える位置表記で、構造改善側は + `path#symbol`、テスト側は `target` を渡す。 + """ + if value not in allowed: + info(f"⚠ {source}: 語彙外の{label} `{value}` — unknown へ降格 ({location})") + return "unknown", True + return value, False + + def _normalize_proposal(raw: dict[str, Any], source: str) -> Optional[dict[str, Any]]: """1 件の提案を正規化する。必須項目を欠くものは捨てる。 @@ -37,19 +58,13 @@ def _normalize_proposal(raw: dict[str, Any], source: str) -> Optional[dict[str, smell = str(raw.get("smell") or "").strip() technique = str(raw.get("technique") or "").strip() severity = str(raw.get("severity") or "").strip().lower() - degraded = False - if smell not in SMELLS: - info(f"⚠ {source}: 語彙外の兆候 `{smell}` — unknown へ降格 ({path}#{symbol})") - smell = "unknown" - degraded = True - if technique not in TECHNIQUES: - info(f"⚠ {source}: 語彙外の手法 `{technique}` — unknown へ降格 ({path}#{symbol})") - technique = "unknown" - degraded = True - if severity not in SEVERITY_ORDER: - info(f"⚠ {source}: 語彙外の重要度 `{severity}` — unknown へ降格 ({path}#{symbol})") - severity = "unknown" - degraded = True + smell, smell_degraded = _degrade_if_unknown( + smell, SMELLS, source, "兆候", f"{path}#{symbol}") + technique, technique_degraded = _degrade_if_unknown( + technique, TECHNIQUES, source, "手法", f"{path}#{symbol}") + severity, severity_degraded = _degrade_if_unknown( + severity, SEVERITY_ORDER, source, "重要度", f"{path}#{symbol}") + degraded = smell_degraded or technique_degraded or severity_degraded if degraded: severity = "unknown" @@ -208,14 +223,20 @@ def _normalize_test_proposal( info(f"⚠ {source}: path / target の無いテスト項目を無視しました: {raw!r:.120}") return None - case = str(raw.get("case") or "").strip().lower() - level = str(raw.get("level") or "").strip().lower() - if case not in TEST_CASES: - info(f"⚠ {source}: 語彙外の経路 `{case}` — unknown へ降格 ({target})") - case = "unknown" - if level not in TEST_LEVELS: - info(f"⚠ {source}: 語彙外の階層 `{level}` — unknown へ降格 ({target})") - level = "unknown" + case, _ = _degrade_if_unknown( + str(raw.get("case") or "").strip().lower(), + TEST_CASES, + source, + "経路", + target, + ) + level, _ = _degrade_if_unknown( + str(raw.get("level") or "").strip().lower(), + TEST_LEVELS, + source, + "階層", + target, + ) return { "kind": TEST, diff --git a/plugins/ndf/skills/cross-refactoring/scripts/refactor_lib/rounds.py b/plugins/ndf/skills/cross-refactoring/scripts/refactor_lib/rounds.py index 857b0cfa..fa17d6ef 100644 --- a/plugins/ndf/skills/cross-refactoring/scripts/refactor_lib/rounds.py +++ b/plugins/ndf/skills/cross-refactoring/scripts/refactor_lib/rounds.py @@ -11,12 +11,14 @@ import pathlib -from typing import Any +from typing import Any, Optional +import assignment import statefile -from . import info +from . import die, info from .paths import git_out +from .vocabulary import MAX_APPLY_ATTEMPTS # ラウンドの種類。**宣言の無い状態ファイルは構造改善として読む**（この版より前で # 始めた実行を、再開の時点でテスト整備へ戻さないため）。 @@ -104,9 +106,13 @@ def apply_groups(entry: dict[str, Any]) -> list[dict[str, Any]]: 群を持たない状態ファイル（この版より前）は、**ラウンド全体を 1 つの群**として 読み、その場で記録する。中断から再開したときに、群の単位が実行のたびに 変わらないようにするためである。 + + **鍵が無いときだけ作る。空の配列はそのまま返す。** 採用が 0 件だった提案 + ラウンドは空の配列を書くため、ここで作ると項目が 1 件も無い群が生まれ、 + 担当を起動し続ける（#592）。 """ groups = entry.get("apply_rounds") - if groups: + if groups is not None: return groups entry["apply_rounds"] = [{ "apply_round": 1, @@ -124,12 +130,56 @@ def apply_groups(entry: dict[str, Any]) -> list[dict[str, Any]]: def current_group(entry: dict[str, Any]) -> dict[str, Any]: """進行中の適用ラウンド。まだ開いていなければ最初の群を返す。""" groups = apply_groups(entry) + if not groups: + die("このラウンドには適用ラウンド（群）がありません") current = entry.get("apply_round") or 1 for group in groups: if group.get("apply_round") == current: return group return groups[-1] + +def attempt_of(group: dict[str, Any]) -> int: + """群がいま開いている試行の番号。鍵が無ければ 0（まだ開いていない）。""" + value = group.get("attempt") + return value if isinstance(value, int) and not isinstance(value, bool) else 0 + + +def group_reopening(group: dict[str, Any]) -> str: + """この群をどう扱うか。**中断からの再開と失敗のやり直しを区別する**（#647）。 + + | 値 | 意味 | + | --- | --- | + | `open` | 開いて試行の番号を進める | + | `resume` | 開いたまま閉じていない試行を再開する。番号を進めない | + | `exhausted` | 上限に達した。開かない | + | `empty` | 項目が無い | + + 判定に使うのは群が持つ 2 つだけである。開いた回数（`attempt`）と、結末の記録の + うち工程が適用のものの件数である。**開いた回数だけを数えない。** 進行側が落ちて + 再開しただけで試行が進んでしまう。 + """ + if not (group.get("items") or []): + return "empty" + failed = len([ + record for record in (group.get("failed_attempts") or []) + if record.get("phase") == "apply" + ]) + if failed >= MAX_APPLY_ATTEMPTS: + return "exhausted" + return "resume" if attempt_of(group) > failed else "open" + + +def impl_for_seq(state: dict[str, Any], seq: int) -> tuple[str, Optional[str]]: + """輪番の通し番号から、作業を任せる担当と要求するモデルを引く。 + + **輪番を引く呼び出しはここだけにする**（#728 の決定 9）。参加者の決め方が + 変わったとき（#727）に、変える場所がこの中だけで済む。読むのは 3 か所 + （群の割り当て・結果なしの試行の交代先・最終ゲートの修正担当）である。 + """ + impl, _reviewers = assignment.assign(seq, state["host"]) + return impl, (state.get("models") or {}).get(impl) + def phase_after_group(entry: dict[str, Any]) -> str: """この群を終えた後のフェーズ。残りの群があれば適用を続ける。""" remaining = [ diff --git a/plugins/ndf/skills/cross-refactoring/scripts/refactor_lib/verify.py b/plugins/ndf/skills/cross-refactoring/scripts/refactor_lib/verify.py index 35bc2388..c6bd02ff 100644 --- a/plugins/ndf/skills/cross-refactoring/scripts/refactor_lib/verify.py +++ b/plugins/ndf/skills/cross-refactoring/scripts/refactor_lib/verify.py @@ -415,6 +415,15 @@ def pending_test_judgements(facts: Iterable[dict[str, Any]]) -> list[str]: return undecidable_test_changes(changes) +def _answers_by_path(verdicts: Iterable[dict[str, Any]]) -> dict[str, str]: + """段 2 の答えを、対象ファイルから引ける形にする。""" + return { + str(verdict.get("path")): str(verdict.get("verdict")) + for verdict in verdicts + if isinstance(verdict, dict) and verdict.get("path") + } + + def merge_test_judgements( pending: Iterable[str], verdicts: Iterable[dict[str, Any]], ) -> dict[str, Any]: @@ -428,10 +437,7 @@ def merge_test_judgements( **答えが欠けたものを `unchanged` に倒さない。** 倒すと、判定を返さないことが 通過の手段になる。知らない答えも同じ扱いにする。 """ - answers = { - str(v.get("path")): str(v.get("verdict")) - for v in verdicts if isinstance(v, dict) and v.get("path") - } + answers = _answers_by_path(verdicts) changed = sorted(p for p in pending if answers.get(p) == "changed") if changed: return { @@ -487,14 +493,10 @@ def apply_judgements_to_group( records = entry.get("pending_test_judgements") if not isinstance(records, dict): return [] - answers = { - str(v.get("path")): str(v.get("verdict")) - for v in verdicts if isinstance(v, dict) and v.get("path") - } + answers = _answers_by_path(verdicts) remaining = sorted( path for path in records.get(str(group), []) if answers.get(path) != "unchanged" ) record_pending_judgements(entry, group, remaining) return remaining - diff --git a/plugins/ndf/skills/cross-refactoring/scripts/refactor_lib/vocabulary.py b/plugins/ndf/skills/cross-refactoring/scripts/refactor_lib/vocabulary.py index b17d6d63..50fa0c6f 100644 --- a/plugins/ndf/skills/cross-refactoring/scripts/refactor_lib/vocabulary.py +++ b/plugins/ndf/skills/cross-refactoring/scripts/refactor_lib/vocabulary.py @@ -123,6 +123,16 @@ def vocabulary() -> dict[str, Any]: } +# 1 つの群に対して適用担当を起動し直す上限。**引数を足さない**（#647）。 +# 2 回目は別の担当が試す。2 回とも結果を残さなければ、担当ではなく群の側を疑える。 +# 3 回以上にしても、壊れた CLI に当たる確率が上がるだけである。 +MAX_APPLY_ATTEMPTS = 2 + +# 無進捗と見なすまでの余白。テストの制限時間（`--test-timeout`）へ足した値を +# 起動が `IMPL_STALL_TIMEOUT` として出す。適用と修正の担当はテストを 1 回実行し、 +# その間は何も出力しないため、制限時間そのままでは打ち切られる（#553）。 +IMPL_STALL_MARGIN = 900 + # 適用と修正のコミットに必須のトレーラー。1 つでも欠けたら当該項目を失敗にする。 # 自由文で「codex が実装」と書かせると集計に使えないため、必ずトレーラー形式にする。 REQUIRED_TRAILERS = ("Item-Id", "Round", "Impl-Runtime", "Impl-Model") diff --git a/plugins/ndf/skills/cross-refactoring/tests/conftest.py b/plugins/ndf/skills/cross-refactoring/tests/conftest.py index 3e0b6fc9..38695c64 100644 --- a/plugins/ndf/skills/cross-refactoring/tests/conftest.py +++ b/plugins/ndf/skills/cross-refactoring/tests/conftest.py @@ -46,7 +46,7 @@ def refactor() -> types.ModuleType: _MODULES = ( "commands.apply", "commands.converge", "commands.gate", "commands.report", "commands.setup", - "gitfacts", "outbound", "paths", "plan", "proposals", + "gitfacts", "intake", "outbound", "paths", "plan", "proposals", "rounds", "scope", "verify", "vocabulary", ) diff --git a/plugins/ndf/skills/cross-refactoring/tests/test_abandon_items.py b/plugins/ndf/skills/cross-refactoring/tests/test_abandon_items.py index 767986f6..4fe84471 100644 --- a/plugins/ndf/skills/cross-refactoring/tests/test_abandon_items.py +++ b/plugins/ndf/skills/cross-refactoring/tests/test_abandon_items.py @@ -987,3 +987,147 @@ def test_the_drop_is_reported_as_a_count_only( assert "取り消し 2 件" in out assert "内訳は改修計画にある" in out assert "R1-001 を見送りました" not in out + + +# ---------- 修正の担当が結果を残さないとき（#728 の決定 10） ---------- + +def _fix_state_with_group_impl(tmp_path, group_impl="agy", fix_rounds=0): + """群の担当と提案ラウンドの担当が違う状態。""" + state_path = _state(tmp_path, [_finding("R1-001")], groups=[{ + "apply_round": 1, "impl": group_impl, + "impl_model": {"requested": None, "observed": None}, + "items": ["R1-001", "R1-002"], "status": "applied", + "base_sha": "base0", "head_sha": None, "fix_rounds": fix_rounds, + "attempt": 1, + }]) + state = read_state(state_path) + state["rounds"][0]["fix_rounds"] = fix_rounds + state["rounds"][0]["fix_base_sha"] = "FIX_BASE" + state["rounds"][0]["fix_attempts"] = 1 + state_path.write_text(__import__("json").dumps(state), encoding="utf-8") + return state_path + + +def _fix_args(): + return type("A", (), {"id": 130, "round": 1})() + + +def test_merge_fix_reads_the_result_of_the_group_agent( + patch_lib, cmd_converge, tmp_path, env_tmp_dir, no_git +): + """AC23: 群の担当の結果を読む。提案ラウンドの担当の結果ではない。""" + state_path = _fix_state_with_group_impl(tmp_path) + env_tmp_dir(state_path) + fact = _fix_commit() + patch_lib("git_out", lambda work, args, **k: ( + args[-1].replace("^{commit}", "") + if args[:2] == ["rev-parse", "--verify"] else "HEAD_NOW")) + patch_lib("commits_in_range", lambda work, base, head: [fact["sha"]]) + patch_lib("collect_commit_facts", + lambda work, shas, rng, cmd, branch, timeout=None: [fact]) + patch_lib("resolved_threads_on_github", lambda repo, pr: set()) + write_result(state_path, "agy-fix-r1", { + "resolved_thread_ids": [], "elapsed_seconds": 3, + "commits": [{"sha": fact["sha"]}], + }) + + cmd_converge.cmd_merge_fix(_fix_args()) + + assert read_state(state_path)["rounds"][0]["fix_rounds"] == 1 + + +def test_a_missing_fix_result_advances_the_fix_round( + patch_lib, cmd_converge, tmp_path, env_tmp_dir, no_git +): + """AC24: 修正の結果が無ければ、記録を残して修正ラウンドを 1 進める。""" + state_path = _fix_state_with_group_impl(tmp_path) + env_tmp_dir(state_path) + patch_lib("git_out", lambda work, args, **k: "HEAD_NOW") + patch_lib("commits_in_range", lambda work, base, head: []) + + with pytest.raises(SystemExit) as e: + cmd_converge.cmd_merge_fix(_fix_args()) + + assert e.value.code == 2 + entry = read_state(state_path)["rounds"][0] + assert entry["fix_rounds"] == 1 + records = entry["apply_rounds"][0]["failed_attempts"] + assert [(r["phase"], r["impl"], r["reason"]) for r in records] == [ + ("fix", "agy", "missing")] + + +def test_the_abandon_check_passes_after_the_fix_rounds_run_out( + patch_lib, cmd_converge, tmp_path, env_tmp_dir, no_git +): + """AC24: 上限の回数だけ結果が無ければ、見送りの判定が 0 を返す。""" + state_path = _fix_state_with_group_impl(tmp_path) + env_tmp_dir(state_path) + patch_lib("git_out", lambda work, args, **k: "HEAD_NOW") + patch_lib("commits_in_range", lambda work, base, head: []) + + for attempt in range(1, 4): + state = read_state(state_path) + state["rounds"][0]["fix_attempts"] = attempt + state_path.write_text(__import__("json").dumps(state), encoding="utf-8") + with pytest.raises(SystemExit): + cmd_converge.cmd_merge_fix(_fix_args()) + + assert read_state(state_path)["rounds"][0]["fix_rounds"] == 3 + cmd_converge.cmd_should_abandon(_fix_args()) + + +def test_the_same_fix_attempt_does_not_advance_the_round_twice( + patch_lib, cmd_converge, tmp_path, env_tmp_dir, no_git +): + """AC25: 検証を挟まず叩き直しても、修正ラウンドは進まない。""" + state_path = _fix_state_with_group_impl(tmp_path) + env_tmp_dir(state_path) + patch_lib("git_out", lambda work, args, **k: "HEAD_NOW") + patch_lib("commits_in_range", lambda work, base, head: []) + + for _ in range(2): + with pytest.raises(SystemExit) as e: + cmd_converge.cmd_merge_fix(_fix_args()) + assert e.value.code == 2 + + entry = read_state(state_path)["rounds"][0] + assert entry["fix_rounds"] == 1 + assert len(entry["apply_rounds"][0]["failed_attempts"]) == 1 + + +def test_a_usage_limit_on_the_fix_jumps_to_the_cap( + patch_lib, cmd_converge, tmp_path, env_tmp_dir, no_git +): + """AC26: 起動し直しても解けない結末では、修正ラウンドを上限の値にする。""" + state_path = _fix_state_with_group_impl(tmp_path) + env_tmp_dir(state_path) + patch_lib("git_out", lambda work, args, **k: "HEAD_NOW") + patch_lib("commits_in_range", lambda work, base, head: []) + (state_path.parent / "agy-fix-r1-monitor.json").write_text( + __import__("json").dumps({"reason": "usage_limit", "detail": "上限"}), + encoding="utf-8") + + with pytest.raises(SystemExit) as e: + cmd_converge.cmd_merge_fix(_fix_args()) + + assert e.value.code == 2 + assert read_state(state_path)["rounds"][0]["fix_rounds"] == 3 + cmd_converge.cmd_should_abandon(_fix_args()) + + +def test_a_missing_fix_result_reverts_the_commits_in_range( + patch_lib, cmd_converge, tmp_path, env_tmp_dir, no_git +): + """AC27: 結果が無くても、起点から先端までのコミットは取り消す。""" + state_path = _fix_state_with_group_impl(tmp_path) + env_tmp_dir(state_path) + patch_lib("git_out", lambda work, args, **k: "AFTER_REVERT") + patch_lib("commits_in_range", lambda work, base, head: ["fix2", "fix1"]) + + with pytest.raises(SystemExit): + cmd_converge.cmd_merge_fix(_fix_args()) + + assert [c[-1] for c in no_git if c[:2] == ["git", "revert"]] == ["fix2", "fix1"] + entry = read_state(state_path)["rounds"][0] + assert entry["fix_base_sha"] == "AFTER_REVERT" + assert entry["apply_rounds"][0]["failed_attempts"][0]["reverted"] == 2 diff --git a/plugins/ndf/skills/cross-refactoring/tests/test_apply_attempts.py b/plugins/ndf/skills/cross-refactoring/tests/test_apply_attempts.py new file mode 100644 index 00000000..0afd3505 --- /dev/null +++ b/plugins/ndf/skills/cross-refactoring/tests/test_apply_attempts.py @@ -0,0 +1,409 @@ +"""適用担当が結果を残さなかったときの試行と担当の交代（#647 / #728）。 + +**結果ファイルを置かずに「群を開く → 取り込む」を繰り返す。** 変更前は同じ群が +上限なしに開き直され、担当が作ったコミットは検証を受けずに残っていた。 +""" +from __future__ import annotations + +import json + +import pytest + +from crossref_helpers import make_state, read_state, write_result + +_ARGS_OPEN = {"id": 130, "round": 1} +_ARGS_MERGE = {"id": 130, "round": 1, "dry_run": False} + + +def _group(n, impl, items, **over): + base = { + "apply_round": n, "impl": impl, + "impl_model": {"requested": None, "observed": None}, + "items": list(items), "status": "pending", + "base_sha": None, "head_sha": None, "fix_rounds": 0, "attempt": 0, + } + base.update(over) + return base + + +def _entry(groups, items): + return { + "round": 1, "impl": "codex", "reviewers": ["agy", "kiro"], + "impl_model": {"requested": None, "observed": None}, + "reviewer_models": {}, "proposed": {}, "merged": len(items), + "adopted": len(items), "deferred": 0, + "items": list(items), + "apply_rounds": groups, "apply_round": 0, + "apply": {"applied": [], "failed": [], "base_sha": None, "head_sha": None}, + "fix_rounds": 0, "durations": {}, "reviews": [], + } + + +def _item(item_id, path="src/a.py", symbol="f"): + return { + "item_id": item_id, "round": 1, "path": path, "symbol": symbol, + "smell": "long_method", "technique": "extract_method", + "severity": "major", "status": "pending", "commits": [], + "estimated_diff_lines": 10, + } + + +@pytest.fixture +def two_groups(tmp_path): + """担当の違う 2 つの群を持つ状態ファイル。""" + groups = [_group(1, "agy", ["R1-001"]), _group(2, "codex", ["R1-002"])] + return make_state( + tmp_path, + items=[_item("R1-001"), _item("R1-002", path="src/b.py")], + rounds=[_entry(groups, ["R1-001", "R1-002"])], + phase="apply", outer_round=1, apply_seq=2, + ) + + +@pytest.fixture +def run(patch_lib, cmd_apply, env_tmp_dir, no_git): + """群を開く・取り込むを、git を呼ばずに実行する。""" + def _make(state_path, head="HEAD_NOW", in_range=None): + env_tmp_dir(state_path) + patch_lib("git_out", lambda work, args, **k: head) + patch_lib("commits_in_range", lambda work, base, head_: list(in_range or [])) + + def _open(): + return _exit_code(cmd_apply.cmd_next_apply_round, + type("A", (), dict(_ARGS_OPEN))()) + + def _merge(): + return _exit_code(cmd_apply.cmd_merge_apply, + type("A", (), dict(_ARGS_MERGE))()) + + return _open, _merge + return _make + + +def _exit_code(fn, args): + try: + fn(args) + except SystemExit as e: + return e.code + return 0 + + +# ---------- 結果を残さない担当（AC9 / AC10 / AC12） ---------- + +def test_the_first_failure_swaps_the_agent_and_keeps_the_group_pending( + two_groups, run +): + """AC9: 1 回目の結果なしでは、群は未着手のまま担当が替わる。""" + open_round, merge = run(two_groups) + + assert open_round() == 0 + assert merge() == 2 + + group = read_state(two_groups)["rounds"][0]["apply_rounds"][0] + assert group["status"] == "pending" + assert group["impl"] != "agy" + assert group["attempt"] == 1 + assert [(r["phase"], r["attempt"], r["impl"], r["reason"]) + for r in group["failed_attempts"]] == [("apply", 1, "agy", "missing")] + + +def test_the_second_failure_drops_the_group_and_defers_its_items(two_groups, run): + """AC10: 2 回目も残さなければ取り消し、見送りの理由に担当と理由を並べる。""" + open_round, merge = run(two_groups) + open_round(); merge() + second = read_state(two_groups)["rounds"][0]["apply_rounds"][0]["impl"] + + assert open_round() == 0 + assert merge() == 2 + + state = read_state(two_groups) + group = state["rounds"][0]["apply_rounds"][0] + assert (group["status"], group["drop_reason"]) == ("dropped", "no_result") + assert group["attempt"] == 2 + assert state["items"][0]["status"] == "abandoned" + assert state["deferred_items"][0]["defer_reason"] == ( + f"実装担当が結果を残しませんでした（agy: missing → {second}: missing）" + ) + + +def test_a_broken_result_file_is_recorded_as_unparsable(two_groups, run): + """AC12: JSON として読めない結果と配列の結果も、結果なしとして閉じる。""" + open_round, merge = run(two_groups) + open_round() + (two_groups.parent / "agy-apply-r1-result.json").write_text( + '{"items": [', encoding="utf-8") + + assert merge() == 2 + + group = read_state(two_groups)["rounds"][0]["apply_rounds"][0] + assert group["status"] == "pending" + assert [r["reason"] for r in group["failed_attempts"]] == ["unparsable"] + + +def test_a_json_array_result_is_also_unparsable(two_groups, run): + """AC12: 配列の結果ファイルも同じ扱いになる。""" + open_round, merge = run(two_groups) + open_round() + write_result(two_groups, "agy-apply-r1", [{"item_id": "R1-001"}]) + + assert merge() == 2 + + group = read_state(two_groups)["rounds"][0]["apply_rounds"][0] + assert [r["reason"] for r in group["failed_attempts"]] == ["unparsable"] + + +# ---------- 繰り返しが有限回で終わる（AC11） ---------- + +def test_the_rounds_run_out_after_two_attempts_per_group(two_groups, run): + """AC11: 結果を 1 つも置かなければ、開く操作は 5 回目で尽きる。""" + open_round, merge = run(two_groups) + + opened = 0 + while open_round() == 0: + opened += 1 + merge() + assert opened <= 8, "群を開く操作が止まらない" + + assert opened == 4 + groups = read_state(two_groups)["rounds"][0]["apply_rounds"] + assert [g["status"] for g in groups] == ["dropped", "dropped"] + + +# ---------- 交代先の決め方（AC13 / AC14） ---------- + +def test_the_replacement_differs_from_the_agent_that_failed(tmp_path, run): + """AC13: 輪番が 1 周しても、失敗した担当とは違う担当が出る。""" + groups = [_group(n, "codex" if n == 1 else "agy", [f"R1-00{n}"]) + for n in range(1, 5)] + state_path = make_state( + tmp_path, + items=[_item(f"R1-00{n}", path=f"src/{n}.py") for n in range(1, 5)], + rounds=[_entry(groups, [f"R1-00{n}" for n in range(1, 5)])], + phase="apply", outer_round=1, apply_seq=4, + ) + open_round, merge = run(state_path) + + open_round(); merge() + + saved = read_state(state_path)["rounds"][0]["apply_rounds"] + assert saved[0]["impl"] != "codex" + assert [g["impl"] for g in saved[1:]] == ["agy", "agy", "agy"] + assert read_state(state_path)["apply_seq"] > 4 + + +def test_no_replacement_left_with_a_usage_limit_drops_the_group_at_once( + two_groups, run, patch_lib +): + """AC14: 交代先が無く利用上限なら、1 回目の失敗で群を取り消す。""" + patch_lib("impl_for_seq", lambda state, seq: ("agy", None)) + open_round, merge = run(two_groups) + open_round() + (two_groups.parent / "agy-apply-r1-monitor.json").write_text( + json.dumps({"reason": "usage_limit", "detail": "上限"}), encoding="utf-8") + + assert merge() == 2 + + group = read_state(two_groups)["rounds"][0]["apply_rounds"][0] + assert (group["status"], group["drop_reason"]) == ("dropped", "no_result") + + +def test_no_replacement_left_without_a_usage_limit_keeps_the_same_agent( + two_groups, run, patch_lib +): + """AC14: 交代先が無く、起動し直せる結末なら同じ担当で 2 回目を開く。""" + patch_lib("impl_for_seq", lambda state, seq: ("agy", None)) + open_round, merge = run(two_groups) + open_round() + + assert merge() == 2 + + group = read_state(two_groups)["rounds"][0]["apply_rounds"][0] + assert (group["status"], group["impl"]) == ("pending", "agy") + assert open_round() == 0 + assert read_state(two_groups)["rounds"][0]["apply_rounds"][0]["attempt"] == 2 + + +# ---------- 再開と試行の番号（AC15 / AC16 / AC17 / AC18） ---------- + +def test_opening_twice_without_a_merge_does_not_advance_the_attempt(two_groups, run): + """AC15: 取り込みを挟まずに 2 回開いても、試行の番号は進まない。""" + open_round, _ = run(two_groups) + + open_round() + open_round() + + assert read_state(two_groups)["rounds"][0]["apply_rounds"][0]["attempt"] == 1 + + +def test_an_unverified_baseline_stops_before_reading_the_result( + tmp_path, patch_lib, cmd_apply, env_tmp_dir, no_git +): + """AC16: 着手前のテストが成功と確認できていなければ、結果を読まずに止まる。""" + groups = [_group(1, "agy", ["R1-001"])] + state_path = make_state( + tmp_path, items=[_item("R1-001")], + rounds=[_entry(groups, ["R1-001"])], phase="apply", outer_round=1, + baseline_test={"command": "pytest -q", "status": "red", "checked_at": "x"}, + ) + env_tmp_dir(state_path) + patch_lib("git_out", lambda work, args, **k: "HEAD_NOW") + read_calls: list[str] = [] + patch_lib("read_result", + lambda *a, **k: read_calls.append("読んだ") or (_ for _ in ()).throw( + AssertionError("結果を読んではいけない"))) + + code = _exit_code(cmd_apply.cmd_merge_apply, type("A", (), dict(_ARGS_MERGE))()) + + assert code == 4 + assert read_calls == [] + assert read_state(state_path)["items"][0]["status"] == "blocked" + + +def test_missing_result_with_undetermined_range_blocks_items_and_exits_4( + two_groups, run, patch_lib, no_git +): + """現状固定: 結果がなく範囲を確定できないときは項目を blocked にして終了コード 4 で中断する。""" + open_round, merge = run(two_groups) + assert open_round() == 0 + + patch_lib("commits_in_range", lambda work, base, head_: None) + + code = merge() + + assert code == 4 + state = read_state(two_groups) + assert state["items"][0]["status"] == "blocked" + group = state["rounds"][0]["apply_rounds"][0] + assert "failed_attempts" not in group + assert [c for c in no_git if c[:2] == ["git", "revert"]] == [] + + +def test_a_closed_apply_attempt_stops_before_reading_the_result( + tmp_path, patch_lib, cmd_apply, env_tmp_dir, no_git +): + """現状固定: 記録済みの試行を叩き直しても、結果を読み直さない。""" + group = _group( + 1, + "agy", + ["R1-001"], + attempt=1, + failed_attempts=[{ + "phase": "apply", "attempt": 1, "impl": "agy", + "reason": "missing", "detail": "", "at": "x", "reverted": 0, + }], + ) + state_path = make_state( + tmp_path, items=[_item("R1-001")], + rounds=[_entry([group], ["R1-001"])], phase="apply", outer_round=1, + ) + env_tmp_dir(state_path) + patch_lib("read_result", lambda *a, **k: (_ for _ in ()).throw( + AssertionError("結果を読んではいけない"))) + before = read_state(state_path) + + code = _exit_code(cmd_apply.cmd_merge_apply, type("A", (), dict(_ARGS_MERGE))()) + + after = read_state(state_path) + assert code == 2 + assert after == before + assert len(after["rounds"][0]["apply_rounds"][0]["failed_attempts"]) == 1 + + +def test_every_exit_2_leaves_the_group_dropped_or_pending_with_a_record( + two_groups, run +): + """AC17: 終了コード 2 の後の群は、取り消し済みか、記録を持つ未着手である。""" + open_round, merge = run(two_groups) + + for _ in range(4): + if open_round() != 0: + break + assert merge() == 2 + for group in read_state(two_groups)["rounds"][0]["apply_rounds"]: + if group["status"] == "pending" and group.get("attempt"): + assert group.get("failed_attempts"), group + else: + assert group["status"] in {"pending", "dropped"}, group + + +def test_both_sides_follow_the_reopening_decision(two_groups, run, patch_lib): + """AC18: 開き直しの判定を差し替えると、開く側も取り込み側も従う。""" + patch_lib("group_reopening", lambda group: "exhausted") + open_round, _ = run(two_groups) + + assert open_round() == 1 + + groups = read_state(two_groups)["rounds"][0]["apply_rounds"] + assert [g["status"] for g in groups] == ["dropped", "dropped"] + assert [g["drop_reason"] for g in groups] == ["no_result", "no_result"] + + +# ---------- 輪番から担当を引く関数（AC49） ---------- + +def test_the_group_assignment_goes_through_the_single_rotation_function( + rounds, monkeypatch +): + """AC49: 輪番から担当を引く関数を差し替えると、群の担当がそれに従う。""" + state = {"host": "claude", "models": {"kiro": "auto"}} + + impl, requested = rounds.impl_for_seq(state, 3) + + assert impl in {"claude", "codex", "agy", "kiro"} + assert requested == state["models"].get(impl) + + +# ---------- 修正結果が無く範囲も確定できないとき（converge.cmd_merge_fix / R1-003） ---------- +# +# **共通部品（intake.close_without_result）単体の範囲不明テストでは足りない。** +# `cmd_merge_fix` はその戻り値を受けて、この呼び出し側固有の進行——修正ラウンドを +# 1 つ進めて終了コード 2 を返す——を行う。ここではその呼び出し側の振る舞いを固定する。 + + +def _fix_entry(fix_base, fix_rounds=0, fix_attempts=1): + """修正フェーズに入った、担当と進行中の群を持つラウンド。""" + group = _group(1, "agy", ["R1-001"], status="applied", + base_sha="base0", head_sha="sha1") + return { + "round": 1, "impl": "codex", "reviewers": ["agy", "kiro"], + "impl_model": {"requested": None, "observed": None}, + "reviewer_models": {}, "proposed": {}, "merged": 1, + "adopted": 1, "deferred": 0, + "items": ["R1-001"], + "apply_rounds": [group], "apply_round": 1, + "apply": {"applied": ["R1-001"], "failed": [], + "base_sha": "base0", "head_sha": "sha1"}, + "fix_rounds": fix_rounds, "fix_attempts": fix_attempts, + "fix_base_sha": fix_base, "durations": {}, "reviews": [], + } + + +def _fix_state(tmp_path, fix_base="fixbase0", fix_rounds=0): + item = _item("R1-001") + item["status"] = "applied" + return make_state( + tmp_path, + items=[item], + rounds=[_fix_entry(fix_base, fix_rounds=fix_rounds)], + phase="fix", outer_round=1, + ) + + +def test_a_missing_fix_result_with_an_unknown_range_advances_the_round_and_exits_2( + tmp_path, patch_lib, cmd_converge, env_tmp_dir, no_git +): + """現状固定: 修正結果が無く範囲も確定できないと、修正ラウンドを 1 つ進めて + 終了コード 2 で中断する。`failed_attempts` は足さず、取り消しも行わない。""" + state_path = _fix_state(tmp_path, fix_rounds=0) + env_tmp_dir(state_path) + patch_lib("git_out", lambda work, args, **k: "HEAD_NOW") + patch_lib("commits_in_range", lambda work, base, head_: None) + + code = _exit_code(cmd_converge.cmd_merge_fix, + type("A", (), {"id": 130, "round": 1})()) + + assert code == 2 + entry = read_state(state_path)["rounds"][0] + assert entry["fix_rounds"] == 1, "修正ラウンドを 1 つ進める" + group = entry["apply_rounds"][0] + assert "failed_attempts" not in group, "範囲不明では失敗の記録を残さない" + assert [c for c in no_git if c[:2] == ["git", "revert"]] == [], "取り消さない" diff --git a/plugins/ndf/skills/cross-refactoring/tests/test_apply_rounds.py b/plugins/ndf/skills/cross-refactoring/tests/test_apply_rounds.py index 9e779758..2daa0f85 100644 --- a/plugins/ndf/skills/cross-refactoring/tests/test_apply_rounds.py +++ b/plugins/ndf/skills/cross-refactoring/tests/test_apply_rounds.py @@ -323,3 +323,117 @@ def test_phase_after_group_returns_to_propose_when_there_is_no_group(rounds): entry = _round_with_groups([], items=()) assert rounds.phase_after_group(entry) == "propose" + + +# ---------- 採用 0 件と項目の無い群（#592） ---------- + +def _empty_group(n, items=(), **over): + base = { + "apply_round": n, "impl": "codex", + "impl_model": {"requested": None, "observed": None}, + "items": list(items), "status": "pending", + "base_sha": None, "head_sha": None, "fix_rounds": 0, "attempt": 0, + } + base.update(over) + return base + + +def test_no_adopted_proposal_leaves_the_group_list_empty( + patch_lib, refactor, cmd_apply, tmp_path, env_tmp_dir, monkeypatch +): + """AC19: 採用 0 件のテスト整備ラウンドでは群を作らず、開く操作が 1 回で尽きる。""" + state_path = make_state( + tmp_path, rounds=[{**round_of(), "kind": "test"}], + phase="propose", outer_round=1, round_kind="test", + ) + env_tmp_dir(state_path) + patch_lib("git_out", lambda work, args, **k: "base0") + for runtime in ("codex", "agy", "kiro"): + write_result(state_path, f"{runtime}-propose-rf130-r1", {"items": []}) + refactor.cmd_merge_proposals(type("A", (), {"id": 130})()) + + assert read_state(state_path)["rounds"][0]["apply_rounds"] == [] + + with pytest.raises(SystemExit) as e: + cmd_apply.cmd_next_apply_round(type("A", (), {"id": 130, "round": 1})()) + assert e.value.code == 1 + assert read_state(state_path)["rounds"][0]["apply_rounds"] == [] + + +def test_a_state_without_the_group_key_still_opens_the_whole_round( + patch_lib, refactor, tmp_path, env_tmp_dir, monkeypatch, capsys +): + """AC20: 群の鍵を持たない状態ファイルは、ラウンド全体を 1 群として開く。""" + entry = round_of(items=["R1-001"]) + entry["impl"] = "codex" + state_path = make_state(tmp_path, rounds=[entry], phase="apply", outer_round=1) + env_tmp_dir(state_path) + patch_lib("git_out", lambda work, args, **k: "HEAD_NOW") + + refactor.cmd_next_apply_round(type("A", (), {"id": 130, "round": 1})()) + + assert "APPLY_ROUND=1" in capsys.readouterr().out + groups = read_state(state_path)["rounds"][0]["apply_rounds"] + assert [g["items"] for g in groups] == [["R1-001"]] + + +def test_a_merged_group_with_nothing_adopted_is_dropped_as_empty( + patch_lib, cmd_apply, tmp_path, env_tmp_dir, no_git +): + """AC21: 取り込み済みで採用 0 件の群（項目なし）を取り消し済みに直す。""" + entry = round_of() + entry["apply_rounds"] = [_empty_group( + 1, status="applied", base_sha="base0", attempt=1)] + entry["apply_round"] = 1 + entry["apply"] = {"apply_round": 1, "applied": [], "failed": [], + "base_sha": "base0", "head_sha": "h", "merged_at": "x"} + state_path = make_state(tmp_path, rounds=[entry], phase="apply", outer_round=1) + env_tmp_dir(state_path) + patch_lib("git_out", lambda work, args, **k: "HEAD_NOW") + + with pytest.raises(SystemExit) as e: + cmd_apply.cmd_merge_apply( + type("A", (), {"id": 130, "round": 1, "dry_run": False})()) + assert e.value.code == 2 + + group = read_state(state_path)["rounds"][0]["apply_rounds"][0] + assert (group["status"], group["drop_reason"]) == ("dropped", "empty") + + with pytest.raises(SystemExit) as e: + cmd_apply.cmd_next_apply_round(type("A", (), {"id": 130, "round": 1})()) + assert e.value.code == 1 + + +def test_a_pending_group_without_items_is_dropped_before_it_opens( + patch_lib, cmd_apply, tmp_path, env_tmp_dir, capsys +): + """AC22: 項目の無い未着手の群は開かれず、次の群があればそちらを開く。""" + entry = round_of(items=["R1-002"]) + entry["apply_rounds"] = [_empty_group(1), _empty_group(2, items=["R1-002"])] + state_path = make_state(tmp_path, rounds=[entry], phase="apply", outer_round=1) + env_tmp_dir(state_path) + patch_lib("git_out", lambda work, args, **k: "HEAD_NOW") + + cmd_apply.cmd_next_apply_round(type("A", (), {"id": 130, "round": 1})()) + + assert "APPLY_ROUND=2" in capsys.readouterr().out + groups = read_state(state_path)["rounds"][0]["apply_rounds"] + assert (groups[0]["status"], groups[0]["drop_reason"]) == ("dropped", "empty") + + +def test_a_round_whose_only_group_has_no_item_runs_out( + patch_lib, cmd_apply, tmp_path, env_tmp_dir +): + """AC22: 項目の無い群だけなら、開く操作は 1 を返す。""" + entry = round_of() + entry["apply_rounds"] = [_empty_group(1)] + state_path = make_state(tmp_path, rounds=[entry], phase="apply", outer_round=1) + env_tmp_dir(state_path) + patch_lib("git_out", lambda work, args, **k: "HEAD_NOW") + + with pytest.raises(SystemExit) as e: + cmd_apply.cmd_next_apply_round(type("A", (), {"id": 130, "round": 1})()) + + assert e.value.code == 1 + group = read_state(state_path)["rounds"][0]["apply_rounds"][0] + assert (group["status"], group["drop_reason"]) == ("dropped", "empty") diff --git a/plugins/ndf/skills/cross-refactoring/tests/test_commit_trailers_git.py b/plugins/ndf/skills/cross-refactoring/tests/test_commit_trailers_git.py new file mode 100644 index 00000000..946d6f4f --- /dev/null +++ b/plugins/ndf/skills/cross-refactoring/tests/test_commit_trailers_git.py @@ -0,0 +1,167 @@ +"""実行環境が帰属の段落を足したコミットから記名を読む（#553）。 + +**一時リポジトリで実際に git を実行する。** 段落の切り分けは git の判定 +（`git interpret-trailers --parse`）に委ねているため、差し替えた出力で確かめても +本番の読み方を確かめたことにならない。 +""" +from __future__ import annotations + +import subprocess + +import pytest + +REQUIRED = ("Item-Id", "Round", "Impl-Runtime", "Impl-Model") + +_SIGNED = """Refactor: extract_method — src/foo.py#Bar.handle + +変更の説明。 + +Item-Id: R1-001 +Round: 1 +Impl-Runtime: claude +Impl-Model: claude-opus-5 + +Co-Authored-By: Claude Opus 5 +""" + + +def _git(*args, cwd): + return subprocess.run(["git", *args], cwd=cwd, capture_output=True, text=True, + check=True) + + +@pytest.fixture +def repo(tmp_path): + """コミットを積める一時リポジトリ。""" + path = tmp_path / "repo" + path.mkdir() + _git("init", "-q", "-b", "main", cwd=path) + _git("config", "user.email", "t@e.st", cwd=path) + _git("config", "user.name", "test", cwd=path) + (path / "src").mkdir() + (path / "src" / "foo.py").write_text("x = 1\n", encoding="utf-8") + _git("add", "-A", cwd=path) + _git("commit", "-qm", "init", cwd=path) + return path + + +@pytest.fixture +def commit(repo): + """メッセージを渡してコミットを作り、その完全な識別子を返す。""" + counter = {"n": 0} + + def _make(message: str) -> str: + counter["n"] += 1 + (repo / "src" / f"f{counter['n']}.py").write_text("y = 1\n", encoding="utf-8") + _git("add", "-A", cwd=repo) + _git("commit", "-q", "-m", message, cwd=repo) + return _git("rev-parse", "HEAD", cwd=repo).stdout.strip() + + return _make + + +def test_a_signature_paragraph_after_the_required_ones_is_skipped( + gitfacts, repo, commit +): + """AC32: 必須の記名の後ろに帰属の段落が付いても 4 つとも読める。""" + sha = commit(_SIGNED) + + trailers = gitfacts.commit_trailers(str(repo), sha) + + assert [trailers.get(k) for k in REQUIRED] == [ + "R1-001", "1", "claude", "claude-opus-5"] + + +def test_two_attribution_lines_in_one_paragraph_are_also_skipped( + gitfacts, repo, commit +): + """AC33: 帰属の段落が 2 行でも 4 つとも読める。""" + sha = commit( + _SIGNED + "Claude-Session: https://example.test/session_1\n") + + trailers = gitfacts.commit_trailers(str(repo), sha) + + assert all(trailers.get(k) for k in REQUIRED) + assert trailers["Claude-Session"] == "https://example.test/session_1" + + +def test_a_prose_paragraph_stops_the_reading(gitfacts, repo, commit): + """AC34: 散文の段落より前にある記名の形の行は読まない。""" + sha = commit( + "Refactor: 題名\n\n" + "Item-Id: R1-001\nRound: 1\n" + "Impl-Runtime: claude\nImpl-Model: claude-opus-5\n\n" + "この段落は説明の散文です。\n\n" + "Co-Authored-By: Someone \n" + ) + + trailers = gitfacts.commit_trailers(str(repo), sha) + + assert trailers == {"Co-Authored-By": "Someone "} + + +def test_a_mixed_last_paragraph_is_not_read(gitfacts, repo, commit): + """AC35: 散文と記名の形が混ざる段落は、git が記名の段落と判定しない。""" + sha = commit( + "Refactor: 題名\n\n" + "ここは説明です。\nこちらも説明です。\nさらに説明です。\nRound: 3\n" + ) + + trailers = gitfacts.commit_trailers(str(repo), sha) + + assert trailers == {} + + +def test_the_paragraph_nearest_the_end_wins(gitfacts, repo, commit): + """AC36: 同じ鍵が 2 つの段落にあれば、末尾に近い方の値を採る。""" + sha = commit( + "Refactor: 題名\n\n" + "Item-Id: R1-001\nRound: 1\n" + "Impl-Runtime: claude\nImpl-Model: claude-opus-5\n\n" + "Impl-Model: claude-opus-5-later\n" + ) + + trailers = gitfacts.commit_trailers(str(repo), sha) + + assert trailers["Impl-Model"] == "claude-opus-5-later" + assert trailers["Item-Id"] == "R1-001" + + +def test_a_subject_shaped_like_a_trailer_is_not_read(gitfacts, repo, commit): + """AC38: 題名が記名の形でも、1 段落目は判定に掛けない。""" + sha = commit("Round: 本文の題名\n\nItem-Id: R1-002\n") + + trailers = gitfacts.commit_trailers(str(repo), sha) + + assert trailers == {"Item-Id": "R1-002"} + + +def test_a_commit_with_an_attribution_paragraph_passes_the_apply_check( + gitfacts, verify, repo, commit +): + """AC37: 帰属の段落が付いたコミットは、記名の欠落で取り消されない。""" + sha = commit(_SIGNED) + facts = gitfacts.collect_commit_facts( + str(repo), [sha], {sha}, "", "main") + + problem = verify.verify_apply_round( + [{"item_id": "R1-001", "estimated_diff_lines": 100, + "technique": "extract_method", "test_gap": False}], + facts, ["src"], + ) + + assert problem is None + + +def test_the_commit_convention_asks_for_the_last_paragraph(prompts_dir): + """AC39: 適用と修正の雛形が、必須の記名を最後の段落へ置くことを書く。""" + for name in ("apply.md", "fix.md"): + text = (prompts_dir / name).read_text(encoding="utf-8") + assert "最後の段落" in text, name + assert "空行を挟まず" in text, name + + +@pytest.fixture +def prompts_dir(): + import pathlib + return pathlib.Path(__file__).resolve().parents[1] / "prompts" diff --git a/plugins/ndf/skills/cross-refactoring/tests/test_final_fix.py b/plugins/ndf/skills/cross-refactoring/tests/test_final_fix.py index ba0c22f9..15dde663 100644 --- a/plugins/ndf/skills/cross-refactoring/tests/test_final_fix.py +++ b/plugins/ndf/skills/cross-refactoring/tests/test_final_fix.py @@ -369,3 +369,147 @@ def test_the_propose_result_file_carries_the_round_number(paths): 始まった時点で 1 巡目の提案内容が失われる。 """ assert paths.stem_for("codex", "propose", 130, 2) == "codex-propose-rf130-r2" + + +# ---------- 最終ゲートの修正で結果が無いとき（#674 / #728 の決定 11） ---------- + +def test_a_missing_final_fix_result_reverts_and_moves_the_base( + cmd_gate, tmp_path, env_tmp_dir, merge_spy +): + """AC28: 結果が無ければ取り消し、起点を取り消し後の先端へ進める。""" + state_path = _failing_gate_state(tmp_path) + env_tmp_dir(state_path) + + with pytest.raises(SystemExit) as e: + cmd_gate.cmd_merge_final_fix(_args()) + + assert e.value.code == 2 + gate = read_state(state_path)["final_gate"] + assert len(merge_spy["reverted"]) == 1 + assert gate["fix_base_sha"] == "HEADSHA" + assert [(r["phase"], r["impl"], r["reason"]) for r in gate["failed_attempts"]] == [ + ("final-fix", "codex", "missing")] + assert "fix_commits" not in gate + + +def test_a_missing_final_fix_with_an_unknown_range_keeps_the_attempt_open( + patch_lib, cmd_gate, tmp_path, env_tmp_dir, merge_spy +): + """現状固定: 結果も範囲も無ければ、記録も取り消しも行わず止まる。""" + state_path = _failing_gate_state(tmp_path) + env_tmp_dir(state_path) + patch_lib("commits_in_range", lambda work, base, head: None) + before = read_state(state_path)["final_gate"] + + with pytest.raises(SystemExit) as e: + cmd_gate.cmd_merge_final_fix(_args()) + + gate = read_state(state_path)["final_gate"] + assert e.value.code == 2 + assert gate == before + assert gate["fix_rounds"] == 1 + assert gate["fix_base_sha"] == "BASE" + assert "failed_attempts" not in gate + assert merge_spy["reverted"] == [] + + +def test_the_next_gate_does_not_see_the_reverted_commits( + patch_lib, cmd_gate, tmp_path, env_tmp_dir, merge_spy, gate_spy +): + """AC29: 取り消した後の最終ゲートは、修正のコミットを数に入れない。""" + state_path = _failing_gate_state(tmp_path) + env_tmp_dir(state_path) + with pytest.raises(SystemExit): + cmd_gate.cmd_merge_final_fix(_args()) + + gate_spy["test_code"] = 0 + cmd_gate.cmd_final_gate(_args()) + + gate = read_state(state_path)["final_gate"] + assert gate.get("fix_commits", []) == [] + assert gate["status"] == "passed" + + +def test_a_usage_limit_on_the_final_fix_jumps_to_the_cap( + cmd_gate, tmp_path, env_tmp_dir, merge_spy +): + """AC30: 起動し直しても解けない結末では、修正ラウンドを上限の値にする。""" + state_path = _failing_gate_state(tmp_path) + env_tmp_dir(state_path) + (state_path.parent / "codex-final-fix-monitor.json").write_text( + __import__("json").dumps({"reason": "usage_limit", "detail": "上限"}), + encoding="utf-8") + + with pytest.raises(SystemExit) as e: + cmd_gate.cmd_merge_final_fix(_args()) + + assert e.value.code == 2 + assert read_state(state_path)["final_gate"]["fix_rounds"] == 3 + + +def test_the_gate_after_the_cap_reports_without_reverting( + cmd_gate, tmp_path, env_tmp_dir, merge_spy, gate_spy +): + """AC30: 上限に達した後の最終ゲートは、落ちても取り消さず報告で終わる。""" + state_path = _failing_gate_state(tmp_path) + env_tmp_dir(state_path) + (state_path.parent / "codex-final-fix-monitor.json").write_text( + __import__("json").dumps({"reason": "usage_limit", "detail": "上限"}), + encoding="utf-8") + with pytest.raises(SystemExit): + cmd_gate.cmd_merge_final_fix(_args()) + + gate_spy["test_code"] = 1 + with pytest.raises(SystemExit) as e: + cmd_gate.cmd_final_gate(_args()) + + assert e.value.code == 1 + assert read_state(state_path)["final_gate"]["status"] == "failed" + + +def test_a_verified_final_fix_keeps_no_failure_record( + cmd_gate, tmp_path, env_tmp_dir, merge_spy +): + """AC31: 検証を通る修正は、変更前と同じく取り込まれ、記録を持たない。""" + state_path = _failing_gate_state(tmp_path) + env_tmp_dir(state_path) + write_result(state_path, "codex-final-fix", + {"elapsed_seconds": 7, "commits": [{"sha": "C1FULL"}]}) + + cmd_gate.cmd_merge_final_fix(_args()) + + gate = read_state(state_path)["final_gate"] + assert "failed_attempts" not in gate + assert gate["fix_commits"] == ["C1FULL"] + + +def test_the_same_final_fix_attempt_is_closed_only_once( + cmd_gate, tmp_path, env_tmp_dir, merge_spy +): + """同じ修正ラウンドで叩き直しても、結果ファイルを読まずに同じ終了コードを返す。""" + state_path = _failing_gate_state(tmp_path) + env_tmp_dir(state_path) + with pytest.raises(SystemExit): + cmd_gate.cmd_merge_final_fix(_args()) + write_result(state_path, "codex-final-fix", {"commits": [{"sha": "C1FULL"}]}) + + with pytest.raises(SystemExit) as e: + cmd_gate.cmd_merge_final_fix(_args()) + + assert e.value.code == 2 + assert len(read_state(state_path)["final_gate"]["failed_attempts"]) == 1 + + +def test_the_final_fix_agent_comes_from_the_single_rotation_function( + patch_lib, cmd_gate, tmp_path, env_tmp_dir, gate_spy +): + """AC49: 輪番から担当を引く関数を差し替えると、最終ゲートの修正担当も従う。""" + state_path = _gate_state(tmp_path) + env_tmp_dir(state_path) + gate_spy["test_code"] = 1 + patch_lib("impl_for_seq", lambda state, seq: ("kiro", "auto")) + + with pytest.raises(SystemExit): + cmd_gate.cmd_final_gate(_args()) + + assert read_state(state_path)["final_gate"]["impl"] == "kiro" diff --git a/plugins/ndf/skills/cross-refactoring/tests/test_git_facts.py b/plugins/ndf/skills/cross-refactoring/tests/test_git_facts.py index 6d9d3275..6b5e65b1 100644 --- a/plugins/ndf/skills/cross-refactoring/tests/test_git_facts.py +++ b/plugins/ndf/skills/cross-refactoring/tests/test_git_facts.py @@ -5,6 +5,7 @@ """ from __future__ import annotations +import json import subprocess import pytest @@ -276,39 +277,58 @@ def test_cutting_off_kills_children_that_ignore_sigterm(gitfacts, work): assert not marker.exists(), "SIGTERM を無視する子が生き残っている" -def test_read_result_aborts_when_the_file_is_missing(gitfacts, tmp_path): - """現状固定: 結果ファイルが無ければ終了コード 2 で中断する。 +def test_read_result_returns_a_value_when_the_file_is_missing(gitfacts, tmp_path, capsys): + """結果ファイルが無くても中断せず、結果なしの値を返す。 - 起動した CLI が結果を残さなかった場合であり、進行は次のラウンドへ進む。 + 中断すると、担当が作ったコミットが取り消されないまま Pull Request に残る + （#728）。何で終わるかは読んだ側（取り込み）が決める。 """ - with pytest.raises(SystemExit) as e: - gitfacts.read_result(tmp_path / "missing.json", "claude") - assert e.value.code == 2 + state = {"id": 130, "tmp_dir": str(tmp_path)} + outcome = gitfacts.read_result(state, "claude", "apply", 1) + assert outcome.payload is None + assert outcome.reason == "missing" + assert outcome.relaunch_same_agent is True + captured = capsys.readouterr() + assert (captured.out, captured.err) == ("", "") -def test_read_result_aborts_on_broken_json(gitfacts, tmp_path): - """現状固定: JSON として読めなければ終了コード 2 で中断する。""" - path = tmp_path / "result.json" - path.write_text('{"items": [', encoding="utf-8") - with pytest.raises(SystemExit) as e: - gitfacts.read_result(path, "claude") - assert e.value.code == 2 +def test_read_result_returns_unparsable_for_broken_json(gitfacts, tmp_path): + """JSON として読めない結果ファイルは、理由 `unparsable` の結果なしになる。""" + (tmp_path / "claude-apply-r1-result.json").write_text( + '{"items": [', encoding="utf-8") + state = {"id": 130, "tmp_dir": str(tmp_path)} + + outcome = gitfacts.read_result(state, "claude", "apply", 1) + + assert (outcome.payload, outcome.reason) == (None, "unparsable") @pytest.mark.parametrize("body", ['[{"item_id": "R1-001"}]', "42"]) -def test_read_result_aborts_when_the_json_is_not_an_object(gitfacts, tmp_path, body): - """現状固定: 配列や数値も終了コード 2 で中断する。 +def test_read_result_returns_unparsable_when_the_json_is_not_an_object( + gitfacts, tmp_path, body +): + """配列や数値も結果なしとして返す。 - 呼び出し側は `payload.get(...)` を呼ぶため、読み込みの時点で弾かないと - `AttributeError` になって進行が止まる。 + 呼び出し側は `payload.get(...)` を呼ぶため、辞書でないものを渡すと + `AttributeError` になって進行が止まる。読み込みの時点で結果なしへ寄せる。 """ - path = tmp_path / "result.json" - path.write_text(body, encoding="utf-8") + (tmp_path / "claude-apply-r1-result.json").write_text(body, encoding="utf-8") + state = {"id": 130, "tmp_dir": str(tmp_path)} + + outcome = gitfacts.read_result(state, "claude", "apply", 1) + + assert (outcome.payload, outcome.reason) == (None, "unparsable") - with pytest.raises(SystemExit) as e: - gitfacts.read_result(path, "claude") - assert e.value.code == 2 + +def test_read_result_reads_the_stem_of_each_phase(gitfacts, tmp_path): + """名前の幹は工程ごとに変わる。最終ゲートの修正だけラウンド番号を持たない。""" + (tmp_path / "codex-fix-r2-result.json").write_text('{"ok": 1}', encoding="utf-8") + (tmp_path / "codex-final-fix-result.json").write_text('{"ok": 2}', encoding="utf-8") + state = {"id": 130, "tmp_dir": str(tmp_path)} + + assert gitfacts.read_result(state, "codex", "fix", 2).payload == {"ok": 1} + assert gitfacts.read_result(state, "codex", "final-fix").payload == {"ok": 2} def test_find_item_returns_none_for_a_missing_id_when_not_required(gitfacts): @@ -322,6 +342,22 @@ def test_find_item_returns_none_for_a_missing_id_when_not_required(gitfacts): assert gitfacts.find_item(state, "R9-999", required=False) is None +def test_scoped_item_ids_falls_back_to_all_items_when_apply_round_is_missing( + gitfacts, +): + """現状固定: 現在の適用ラウンドの群がなければ entry 全体の項目を返す。""" + entry = { + "apply_round": 3, + "items": ["R2-003", "R2-001", "R2-002"], + "apply_rounds": [ + {"apply_round": 1, "items": ["R2-001"]}, + {"apply_round": 2, "items": ["R2-002"]}, + ], + } + + assert gitfacts.scoped_item_ids(entry) == ["R2-003", "R2-001", "R2-002"] + + def test_revert_item_commits_failure_message_includes_item_id(gitfacts, work, capsys): """現状固定: revert_item_commits 失敗時は項目 ID 接頭辞付きのエラー文を出して中断する。""" first = _commit(work, "one", {"src/a.py": "a = 1\n"}) @@ -356,3 +392,89 @@ def test_revert_range_failure_message_has_no_item_id_prefix(gitfacts, work, caps assert f"（HEAD を {second} へ戻しました）" in err assert _git("rev-parse", "HEAD", cwd=work).stdout.strip() == second + +def test_check_run_result_characterization(gitfacts, monkeypatch): + """check_run_result の公開契約を固定する現状固定テスト。""" + # 1. 引数が空なら None + assert gitfacts.check_run_result("", "sha", "ci") is None + assert gitfacts.check_run_result("repo", "", "ci") is None + assert gitfacts.check_run_result("repo", "sha", "") is None + + # 2. gh api の実行失敗（sh が None または空）なら None + monkeypatch.setattr(gitfacts, "sh", lambda *args, **kwargs: None) + assert gitfacts.check_run_result("repo", "sha", "ci") is None + + monkeypatch.setattr(gitfacts, "sh", lambda *args, **kwargs: "") + assert gitfacts.check_run_result("repo", "sha", "ci") is None + + # 3. 不正 JSON なら None + monkeypatch.setattr(gitfacts, "sh", lambda *args, **kwargs: "not-json{") + assert gitfacts.check_run_result("repo", "sha", "ci") is None + + # 4. check_runs 欠損（非 dict、または check_runs がリストでない）なら None + monkeypatch.setattr(gitfacts, "sh", lambda *args, **kwargs: "[]") + assert gitfacts.check_run_result("repo", "sha", "ci") is None + + monkeypatch.setattr(gitfacts, "sh", lambda *args, **kwargs: json.dumps({"check_runs": "not-a-list"})) + assert gitfacts.check_run_result("repo", "sha", "ci") is None + + # 5. 対象名なし（一致する name がない）なら None + monkeypatch.setattr( + gitfacts, + "sh", + lambda *args, **kwargs: json.dumps({ + "check_runs": [{"name": "other", "status": "completed", "conclusion": "success"}] + }), + ) + assert gitfacts.check_run_result("repo", "sha", "ci") is None + + # 6. 未完了（status != completed）なら "pending" + monkeypatch.setattr( + gitfacts, + "sh", + lambda *args, **kwargs: json.dumps({ + "check_runs": [ + {"name": "ci", "status": "in_progress", "conclusion": None}, + {"name": "ci", "status": "completed", "conclusion": "success"}, + ] + }), + ) + assert gitfacts.check_run_result("repo", "sha", "ci") == "pending" + + # 7. 失敗（completed だが conclusion != success）ならその結論（または unknown） + monkeypatch.setattr( + gitfacts, + "sh", + lambda *args, **kwargs: json.dumps({ + "check_runs": [ + {"name": "ci", "status": "completed", "conclusion": "failure"}, + {"name": "ci", "status": "completed", "conclusion": "success"}, + ] + }), + ) + assert gitfacts.check_run_result("repo", "sha", "ci") == "failure" + + monkeypatch.setattr( + gitfacts, + "sh", + lambda *args, **kwargs: json.dumps({ + "check_runs": [ + {"name": "ci", "status": "completed", "conclusion": None}, + ] + }), + ) + assert gitfacts.check_run_result("repo", "sha", "ci") == "unknown" + + # 8. 全成功なら "success" + monkeypatch.setattr( + gitfacts, + "sh", + lambda *args, **kwargs: json.dumps({ + "check_runs": [ + {"name": "ci", "status": "completed", "conclusion": "success"}, + {"name": "ci", "status": "COMPLETED", "conclusion": "SUCCESS"}, + ] + }), + ) + assert gitfacts.check_run_result("repo", "sha", "ci") == "success" + diff --git a/plugins/ndf/skills/cross-refactoring/tests/test_init.py b/plugins/ndf/skills/cross-refactoring/tests/test_init.py index ced38334..adc7e054 100644 --- a/plugins/ndf/skills/cross-refactoring/tests/test_init.py +++ b/plugins/ndf/skills/cross-refactoring/tests/test_init.py @@ -326,6 +326,30 @@ def test_init_emits_shell_assignments(run_init, tmp_path, capsys): assert "TMP_DIR=" in out and "WORK=" in out +def test_init_emits_the_stall_timeout_for_the_implementer(run_init, tmp_path, capsys): + """AC40: 無進捗の許容は、テストの制限時間に 900 秒を足した値である。 + + 適用と修正の担当はテストを 1 回実行し、その間は何も出力しない。制限時間 + そのままでは実行中に打ち切られる。 + """ + args = _args(tmp_path) + args.test_timeout = 900 # `--test-timeout` の既定 + + run_init(args) + + assert "IMPL_STALL_TIMEOUT=1800" in capsys.readouterr().out + + +def test_the_stall_timeout_follows_the_test_timeout(run_init, tmp_path, capsys): + """AC40: テストの制限時間を変えると、無進捗の許容も一緒に動く。""" + args = _args(tmp_path) + args.test_timeout = 1200 + + run_init(args) + + assert "IMPL_STALL_TIMEOUT=2100" in capsys.readouterr().out + + def test_existing_worktree_is_synced_to_origin(run_init, tmp_path, origin_repo): """再開までに head が進んでいたら、追いついてから始めること。 diff --git a/plugins/ndf/skills/cross-refactoring/tests/test_intake.py b/plugins/ndf/skills/cross-refactoring/tests/test_intake.py new file mode 100644 index 00000000..18fda83f --- /dev/null +++ b/plugins/ndf/skills/cross-refactoring/tests/test_intake.py @@ -0,0 +1,220 @@ +"""取り込みの共通手順（#728）。 + +3 つの取り込み（適用・修正・最終ゲートの修正）が、結果を残さなかった起動を同じ +手順で閉じることを確かめる。**共通層の読み取りは差し替えない。** 一時ディレクトリに +結果ファイルと監視の結果ファイルを置いて本物を通す。 +""" +from __future__ import annotations + +import json +import types + +import pytest + +from crossref_helpers import make_state, read_state + + +def _outcome(reason=None, payload=None, detail="", relaunch=True): + """`LaunchOutcome` と同じ欄を持つ値。読む側は 5 つの欄しか見ない。""" + return types.SimpleNamespace( + payload=payload, reason=reason, detail=detail, monitor=None, + relaunch_same_agent=relaunch, + ) + + +def _write_monitor(state_path, stem, reason, detail="打ち切りました"): + out = state_path.parent / f"{stem}-monitor.json" + out.write_text( + json.dumps({"reason": reason, "detail": detail}, ensure_ascii=False), + encoding="utf-8", + ) + return out + + +# ---------- 結末の読み取り（AC1 / AC2 / AC3） ---------- + +def test_a_missing_result_file_is_read_as_a_value(gitfacts, tmp_path, capsys): + """AC1: 結果ファイルが無くても中断せず、何も出力しない。""" + state = {"id": 130, "tmp_dir": str(tmp_path)} + + outcome = gitfacts.read_result(state, "agy", "apply", 1) + + assert (outcome.payload, outcome.reason) == (None, "missing") + assert capsys.readouterr() == ("", "") + + +def test_the_monitor_reason_decides_whether_the_same_agent_can_be_relaunched( + gitfacts, tmp_path +): + """AC2: 無進捗は起動し直せる。利用上限は起動し直せない。""" + state = {"id": 130, "tmp_dir": str(tmp_path)} + state_path = tmp_path / "dummy" + + (tmp_path / "agy-apply-r1-monitor.json").write_text( + json.dumps({"reason": "stalled", "detail": "無進捗"}), encoding="utf-8") + stalled = gitfacts.read_result(state, "agy", "apply", 1) + + (tmp_path / "claude-apply-r1-monitor.json").write_text( + json.dumps({"reason": "usage_limit", "detail": "上限"}), encoding="utf-8") + limited = gitfacts.read_result(state, "claude", "apply", 1) + + assert (stalled.reason, stalled.relaunch_same_agent) == ("stalled", True) + assert (limited.reason, limited.relaunch_same_agent) == ("usage_limit", False) + assert state_path.exists() is False + + +def test_the_stem_matches_the_template_the_orchestrator_passes_to_the_monitor(paths): + """AC3: 名前の幹は、骨組みが監視へ渡す雛形を担当名で埋めた値と一致する。""" + skill = ( + paths.pathlib.Path(__file__).resolve().parents[1] / "SKILL.md" + ).read_text(encoding="utf-8") + + expected = { + "apply": "{agent}-apply-r$ROUND", + "fix": "{agent}-fix-r$ROUND", + "final-fix": "{agent}-final-fix", + } + for phase, template in expected.items(): + assert f'--stem-template "{template}"' in skill, phase + built = template.replace("{agent}", "codex").replace("$ROUND", "2") + assert paths.stem_for("codex", phase, 130, 2) == built + + +# ---------- 取り込みの共通手順（AC4〜AC8） ---------- + +def _scope(intake, holder, records, phase, base_key="fix_base_sha", mirror=None): + return intake.IntakeScope( + holder=holder, base_key=base_key, records=records, phase=phase, + attempt=1, impl="agy", label="R1-A1", mirror=mirror, + ) + + +@pytest.fixture +def one_round(tmp_path): + """1 提案ラウンド・1 群の状態ファイル。""" + return make_state( + tmp_path, + items=[{"item_id": "R1-001", "path": "src/a.py", "symbol": "f", + "smell": "long_method", "status": "pending", "round": 1, + "commits": []}], + rounds=[{ + "round": 1, "impl": "codex", "items": ["R1-001"], + "apply_base_sha": "base0", "fix_base_sha": "base0", + "fix_rounds": 0, "fix_attempts": 1, + "apply_rounds": [{ + "apply_round": 1, "impl": "agy", + "impl_model": {"requested": None, "observed": None}, + "items": ["R1-001"], "status": "pending", + "base_sha": "base0", "head_sha": None, "fix_rounds": 0, + "attempt": 1, + }], + "apply_round": 1, "apply": {"applied": [], "failed": []}, + "durations": {}, "reviews": [], + }], + final_gate={"fix_rounds": 1, "checks": [], "impl": "agy", + "fix_base_sha": "base0"}, + ) + + +@pytest.mark.parametrize( + "phase,base_key,records_from", + [("apply", "apply_base_sha", "group"), + ("fix", "fix_base_sha", "group"), + ("final-fix", "fix_base_sha", "gate")], +) +def test_a_commit_in_range_is_reverted_and_the_base_moves_to_the_new_head( + intake, patch_lib, one_round, no_git, phase, base_key, records_from +): + """AC4 / AC5: 範囲のコミットを取り消し、起点を取り消し後の先端へ進める。""" + state = read_state(one_round) + entry = state["rounds"][0] + group = entry["apply_rounds"][0] + gate = state["final_gate"] + holder = gate if records_from == "gate" else entry + records = gate if records_from == "gate" else group + patch_lib("commits_in_range", lambda work, base, head: ["c2", "c1"]) + patch_lib("git_out", lambda work, args, **kw: "newhead") + scope = _scope(intake, holder, records, phase, base_key=base_key, + mirror=group if phase == "apply" else None) + + closed = intake.close_without_result( + one_round, state, scope, _outcome(reason="stalled", detail="無進捗")) + + assert (closed.reverted, closed.range_unknown) == (2, False) + assert holder[base_key] == "newhead" + if phase == "apply": + assert group["base_sha"] == "newhead" + assert records["failed_attempts"] == [{ + "phase": phase, "attempt": 1, "impl": "agy", "reason": "stalled", + "detail": "無進捗", "at": records["failed_attempts"][0]["at"], "reverted": 2, + }] + assert [c[-1] for c in no_git if c[:2] == ["git", "revert"]] == ["c2", "c1"] + + +def test_an_empty_range_runs_neither_revert_nor_push( + intake, patch_lib, one_round, no_git +): + """AC6: 範囲にコミットが無ければ、取り消しも公開もしない。""" + state = read_state(one_round) + entry = state["rounds"][0] + group = entry["apply_rounds"][0] + patch_lib("commits_in_range", lambda work, base, head: []) + patch_lib("git_out", lambda work, args, **kw: "head0") + scope = _scope(intake, entry, group, "fix") + + closed = intake.close_without_result(one_round, state, scope, _outcome("missing")) + + assert closed.reverted == 0 + assert [c for c in no_git if c[:2] in (["git", "revert"], ["git", "push"])] == [] + assert len(group["failed_attempts"]) == 1 + + +def test_the_same_attempt_is_not_recorded_twice(intake, patch_lib, one_round): + """AC7: 同じ工程・同じ試行番号は 1 件しか記録しない。""" + state = read_state(one_round) + entry = state["rounds"][0] + group = entry["apply_rounds"][0] + patch_lib("commits_in_range", lambda work, base, head: []) + patch_lib("git_out", lambda work, args, **kw: "head0") + scope = _scope(intake, entry, group, "fix") + + intake.close_without_result(one_round, state, scope, _outcome("missing")) + assert intake.already_closed(scope) is True + + intake.close_without_result(one_round, state, scope, _outcome("missing")) + assert len(group["failed_attempts"]) == 2, "呼べば足すのは共通手順の責務である" + + +def test_a_range_that_cannot_be_determined_records_nothing( + intake, patch_lib, one_round, no_git +): + """AC8: 範囲を確定できないときは、取り消しも記録もしない。""" + state = read_state(one_round) + entry = state["rounds"][0] + group = entry["apply_rounds"][0] + patch_lib("commits_in_range", lambda work, base, head: None) + patch_lib("git_out", lambda work, args, **kw: "head0") + scope = _scope(intake, entry, group, "fix") + + closed = intake.close_without_result(one_round, state, scope, _outcome("missing")) + + assert closed.range_unknown is True + assert "failed_attempts" not in group + assert [c for c in no_git if c[:2] == ["git", "revert"]] == [] + + +def test_the_failed_agents_are_listed_in_the_order_they_were_recorded( + intake, patch_lib, one_round +): + """交代先を決めるために、失敗した担当を記録の順で読めること。""" + state = read_state(one_round) + entry = state["rounds"][0] + group = entry["apply_rounds"][0] + group["failed_attempts"] = [ + {"phase": "apply", "attempt": 1, "impl": "agy", "reason": "stalled"}, + {"phase": "fix", "attempt": 1, "impl": "kiro", "reason": "missing"}, + {"phase": "apply", "attempt": 2, "impl": "codex", "reason": "missing"}, + ] + scope = _scope(intake, entry, group, "apply", base_key="apply_base_sha") + + assert intake.failed_impls(scope) == ["agy", "codex"] diff --git a/plugins/ndf/skills/cross-refactoring/tests/test_merge_apply.py b/plugins/ndf/skills/cross-refactoring/tests/test_merge_apply.py index 9a4813b6..9f165b39 100644 --- a/plugins/ndf/skills/cross-refactoring/tests/test_merge_apply.py +++ b/plugins/ndf/skills/cross-refactoring/tests/test_merge_apply.py @@ -236,9 +236,13 @@ def test_the_wider_factor_is_limited_to_the_vocabulary(vocabulary): # ---------- git から事実を取る ---------- def test_commit_trailers_are_read_from_git(patch_lib, gitfacts, monkeypatch): - """結果ファイルではなく実際のコミットメッセージから読む。""" + """結果ファイルではなく実際のコミットメッセージから読む。 + + 読むのは**題名の次の段落から後ろ**である。段落の切り分けそのものは実際の git で + 確かめる（`test_commit_trailers_git.py`）。 + """ patch_lib("git_out", - lambda work, args, **_kw: "Item-Id: R1-001\nRound: 1\n" + lambda work, args, **_kw: "Refactor: 題名\n\nItem-Id: R1-001\nRound: 1\n" "Impl-Runtime: codex\nImpl-Model: gpt-5.5", ) assert gitfacts.commit_trailers("/w", "abc") == { @@ -449,6 +453,10 @@ def test_a_verified_apply_round_marks_every_item_applied( assert state["rounds"][0]["apply"]["applied"] == ["R1-001", "R1-002"] assert all(i["status"] == "applied" for i in state["items"]) assert state["phase"] == "verify", "次はテストによる検証へ進む" + # AC46: 結果があり検証を通る適用は、1 回目の試行で取り込まれ記録を残さない + group = state["rounds"][0]["apply_rounds"][0] + assert "failed_attempts" not in group + assert "drop_reason" not in group def test_all_failed_exits_2(refactor, tmp_path, env_tmp_dir, no_git, git_facts): @@ -934,7 +942,7 @@ def test_unverified_baseline_blocks_every_item(refactor, tmp_path, env_tmp_dir): refactor.cmd_merge_apply( type("A", (), {"id": 130, "round": 1, "dry_run": False})() ) - assert e.value.code == 2 + assert e.value.code == 4 assert read_state(state_path)["items"][0]["status"] == "blocked" @@ -955,7 +963,7 @@ def test_unknown_baseline_also_blocks(refactor, tmp_path, env_tmp_dir): refactor.cmd_merge_apply( type("A", (), {"id": 130, "round": 1, "dry_run": False})() ) - assert e.value.code == 2 + assert e.value.code == 4 assert read_state(state_path)["items"][0]["status"] == "blocked" @@ -1005,7 +1013,7 @@ def test_range_that_cannot_be_determined_fails_closed(patch_lib, refactor, tmp_p refactor.cmd_merge_apply( type("A", (), {"id": 130, "round": 1, "dry_run": False})() ) - assert e.value.code == 2 + assert e.value.code == 4 assert read_state(state_path)["items"][0]["status"] == "blocked" @@ -1101,10 +1109,27 @@ def test_broken_apply_result_does_not_crash( assert read_state(state_path)["items"][0]["status"] == "abandoned" -def test_non_object_result_file_fails(refactor, tmp_path, env_tmp_dir, no_git): - """結果が JSON オブジェクトでなければ、読み込みの時点で弾く。""" +def test_non_object_result_file_is_a_missing_result( + refactor, tmp_path, env_tmp_dir, no_git +): + """結果が JSON オブジェクトでなければ、結果なしとして扱う。 + + 呼び出し側は `payload.get(...)` を呼ぶため、辞書でないものを渡すと進行が + 止まる。取り消しと記録を通し、理由 `unparsable` を残して次の試行へ渡す。 + """ items = [item(item_id="R1-001")] - state_path = _state_with_items(tmp_path, items) + state_path = _state_with_items( + tmp_path, items, + rounds_override=[{ + "apply_round": 1, "impl": "codex", + "impl_model": {"requested": "gpt-5.5", "observed": None}, + "items": ["R1-001"], "status": "pending", + "base_sha": "base0", "head_sha": None, "fix_rounds": 0, "attempt": 1, + }], + ) + state = read_state(state_path) + state["rounds"][0]["apply_base_sha"] = "base0" + state_path.write_text(json.dumps(state, ensure_ascii=False), encoding="utf-8") env_tmp_dir(state_path) write_result(state_path, "codex-apply-r1", ["配列で返ってきた"]) with pytest.raises(SystemExit) as e: @@ -1113,6 +1138,9 @@ def test_non_object_result_file_fails(refactor, tmp_path, env_tmp_dir, no_git): ) assert e.value.code == 2 + group = read_state(state_path)["rounds"][0]["apply_rounds"][0] + assert [r["reason"] for r in group["failed_attempts"]] == ["unparsable"] + def test_the_group_sharing_one_commit_is_accepted(paths, patch_lib, refactor, tmp_path, env_tmp_dir, monkeypatch, git_facts): """群の全項目が同じコミットを申告するのが**正しい形**である（決定 2）。 diff --git a/plugins/ndf/skills/cross-refactoring/tests/test_monitor_phase_calls.py b/plugins/ndf/skills/cross-refactoring/tests/test_monitor_phase_calls.py index 83ad6a49..15d8599f 100644 --- a/plugins/ndf/skills/cross-refactoring/tests/test_monitor_phase_calls.py +++ b/plugins/ndf/skills/cross-refactoring/tests/test_monitor_phase_calls.py @@ -21,7 +21,7 @@ "SKILL.md": 4, "docs/01-state-and-propose.md": 1, "docs/02-apply-and-review.md": 2, - "docs/04-fix-and-report.md": 1, + "docs/04-fix-and-report.md": 2, } # 監視の雛形（stem）から、渡すべき工程を決める。 @@ -74,11 +74,14 @@ def test_the_call_does_not_pass_a_timeout(rel: str, call: str) -> None: @pytest.mark.skipif(shutil.which("grep") is None, reason="grep が無い") -def test_the_acceptance_grep_returns_8() -> None: - """要求の文書の AC38 のコマンドをそのまま実行する。""" +def test_the_acceptance_grep_returns_9() -> None: + """要求の文書の AC38 のコマンドをそのまま実行する。 + + 最終ゲートの修正の監視を修正の説明へも書いたため、9 件になる（#728）。 + """ command = ( 'grep -rn -A2 "monitor.py" plugins/ndf/skills/cross-refactoring/SKILL.md ' 'plugins/ndf/skills/cross-refactoring/docs | grep -c -- "--phase"' ) r = subprocess.run(["bash", "-c", command], cwd=REPO, capture_output=True, text=True) - assert r.stdout.strip() == "8", r.stdout + r.stderr + assert r.stdout.strip() == "9", r.stdout + r.stderr diff --git a/plugins/ndf/skills/cross-refactoring/tests/test_paths.py b/plugins/ndf/skills/cross-refactoring/tests/test_paths.py index f113e556..3d2bc91a 100644 --- a/plugins/ndf/skills/cross-refactoring/tests/test_paths.py +++ b/plugins/ndf/skills/cross-refactoring/tests/test_paths.py @@ -9,6 +9,7 @@ import json import pathlib +import tempfile STATE_ID = 130 @@ -82,3 +83,18 @@ def test_it_uses_the_cwd_when_the_env_var_is_unset(paths, tmp_path, monkeypatch) assert path == cwd_path assert state["phase"] == "from-cwd" + + +def test_the_explicit_worktree_base_is_resolved(paths, tmp_path, monkeypatch): + """現状固定: 明示した作業ディレクトリの親を絶対パスへ解決する。""" + monkeypatch.chdir(tmp_path) + monkeypatch.setenv("NDF_WORKTREE_BASE", "relative-worktrees") + + assert paths.default_worktree_base() == (tmp_path / "relative-worktrees").resolve() + + +def test_the_worktree_base_falls_back_to_the_system_tmpdir(paths, monkeypatch): + """現状固定: 明示指定が無ければシステム tmpdir 配下を使う。""" + monkeypatch.delenv("NDF_WORKTREE_BASE", raising=False) + + assert paths.default_worktree_base() == pathlib.Path(tempfile.gettempdir()) / "ndf-worktrees" diff --git a/plugins/ndf/skills/cross-refactoring/tests/test_skill_terms.py b/plugins/ndf/skills/cross-refactoring/tests/test_skill_terms.py index 4ef02075..ee7c4afd 100644 --- a/plugins/ndf/skills/cross-refactoring/tests/test_skill_terms.py +++ b/plugins/ndf/skills/cross-refactoring/tests/test_skill_terms.py @@ -108,3 +108,57 @@ def test_the_command_sequence_does_not_launch_reviewers(skill): block = _run_block(skill) assert " review " not in block assert "verify-round" in block + + +# ---------- 結果なしと無進捗の許容（#728 / #647 / #553） ---------- + +DOCS = SKILL.parent / "docs" + + +def test_the_apply_round_row_states_the_attempt_cap(skill): + """AC43: 適用ラウンドの行が、同じ群を開き直す上限（2 回）を書く。""" + row = next(r for r in _terms_table(skill) if "適用ラウンド" in r) + + assert "2 回" in row + + +def test_the_single_cap_paragraph_is_gone(skill): + """AC43: 上限を 1 つに保つとしていた段落が残っていないこと。""" + assert "別の上限を置かない" not in skill + assert "別に置かない" not in skill + + +def test_every_implementer_phase_passes_the_stall_timeout(skill): + """AC41: 適用・修正・最終ゲートの修正の監視が、無進捗の許容だけを受け取る。""" + for phase in ("apply", "fix", "final-fix"): + block = skill.split(f"--phase {phase}", 1)[1].split("\n\n", 1)[0] + assert '--stall-timeout "$IMPL_STALL_TIMEOUT"' in block, phase + assert "--timeout " not in block, phase + + +def test_the_apply_document_describes_a_missing_result(skill): + """AC44: 適用の説明が、結果なしのときの取り消し・記録・終了コードを書く。""" + text = (DOCS / "02-apply-and-review.md").read_text(encoding="utf-8") + + assert "実装担当が結果を残さなかったとき" in text + assert "failed_attempts" in text + assert "終了コード" in text + assert '--stall-timeout "$IMPL_STALL_TIMEOUT"' in text + + +def test_the_fix_document_describes_a_missing_result(skill): + """AC44: 修正と最終ゲートの説明が、同じ 2 つを書く。""" + text = (DOCS / "04-fix-and-report.md").read_text(encoding="utf-8") + + assert text.count('--stall-timeout "$IMPL_STALL_TIMEOUT"') == 2 + assert "修正の担当が結果を残さなかったときも、修正ラウンドは進める" in text + assert "修正の担当が結果を残さなかったときも同じ手順を通る" in text + + +def test_the_trailer_section_states_both_ways_of_reading(skill): + """AC45: 記名の節が、人の集計と進行側の検証の 2 つの読み方を書く。""" + text = (DOCS / "02-apply-and-review.md").read_text(encoding="utf-8") + section = text.split("### コミットトレーラーの形式", 1)[1].split("\n### ", 1)[0] + + assert "最後の段落だけ" in section + assert "git interpret-trailers --parse" in section