Skip to content

Fix: 公開の責務を進行側へ一本化し、適用失敗の項目を対象外へ記録する - #121

Merged
takemi-ohama merged 6 commits into
mainfrom
fix/cross-refactoring-sync-and-defer
Aug 16, 2026
Merged

Fix: 公開の責務を進行側へ一本化し、適用失敗の項目を対象外へ記録する#121
takemi-ohama merged 6 commits into
mainfrom
fix/cross-refactoring-sync-and-defer

Conversation

@takemi-ohama

@takemi-ohamatakemi-ohama commented Aug 16, 2026

Copy link
Copy Markdown
Contributor

概要

修正後の再検証(#120)で見つかった不具合 10・11 を直す。実施計画は
issues/issue-113-cross-refactoring-push-ownership.md

不具合 10: pre-push の同期検査と範囲ルールが両立しない

.githooks/pre-push が生成物の同期を検査する一方、この Skill は「実装担当は編集元だけを触る / 同期は進行側が収束後に行う」と決めていた。ループの途中で push が起きることを見落としており、実装担当の push も、取り消しを反映する進行側の push も落ちていた。

さらに、この検査は実装担当を範囲ルール違反へ誘導する(実測)。

ラウンド実装担当配布物を同期したか適用成功
1codexしなかった(push は失敗のまま放置)4 / 5
2kiroした(4 コミットすべてで)0 / 5

直し方: 公開の責務を進行側へ一本化する

  • 実装担当は push しない。公開するのは進行側だけで、検証を通した後に行う
  • --sync-command を新設し、push の直前に進行側が実行する。差分があれば
    どの改善項目にも属さないコミットとして積む
  • merge-apply / merge-fix は成功・失敗のどちらでも検証後に push する

これで「未検証の変更が公開される」経路自体が無くなるため、不具合 4 は緩和ではなく根絶になる。

同期を安全に行うための条件(レビューで固めた部分)

条件なぜ
同期の前に作業ツリーが綺麗であること汚れていると、同期が作った差分と元からあった差分を区別できない。状態コードの比較では、元から M のファイルを同期がさらに書き換えた場合を取りこぼす。git commit は index を丸ごと含めるため、staged 済みの変更も公開されてしまう
失敗したら git reset --hard HEAD + git clean -fd で戻す途中まで書き換えた差分を残すと、次の実行が清浄性の検査で必ず止まり、pending_push の再試行が永久に進まない
-c core.quotePath=false を付ける既定では非 ASCII を含むパスがエスケープされ、git add が失敗する
制御用ディレクトリを判定から外す状態ファイル・結果・ログは常にそこへ書かれる
git clean-x を付けない無視されたファイルまで消さない

不具合 11: 適用で失敗した項目が「対象外」に入らない

merge-apply の失敗経路だけが deferred_items へ記録していなかった。実測では、ラウンド 1 で失敗した monitor.py#monitor_agent3 ランタイム全員から再提案され、合意 3 で最優先に採用された。同じ理由で必ず失敗するため、ラウンドを 1 つ丸ごと消費する。

_defer_abandoned_items() を追加し、項目別の失敗とラウンド全体の取り消しの両方から呼ぶ。

互換性

対象変更扱い
init の引数--sync-command を追加追加のみ。省略時は同期しない
状態ファイルsync_command を追加追加のみ
実装担当の手順push を禁止に変更破る。プロンプトと手順書を同時に変更
merge-apply / merge-fix常に push するようになる破る。手順書に明記

検証結果

段階コマンド対象範囲結果
全体テストpytest(2 つの tests)収束ループ 2 Skill444 passed / exit=0
静的解析check-skill-frontmatter.pySkill 35 個エラー 0 / exit=0
ビルド・検証validate-runtime-plugins.sh配布物 3 系統 + marketplacepassed / exit=0
結合(CI)GitHub Actions7 チェック(runtime-smoke 3 種を含む)全て pass

直前は 430 件。新規テスト 14 件を追加している。

未検証の項目: 実機での再々検証は未実施。「指摘の修正と再レビューの繰り返し」「上限到達時の項目単位の見送り」は依然として未到達である

既存の失敗: なし / 範囲外と判断したもの: なし

cross-review の結果

6 ラウンドで codex / gemini の両者が APPROVE に収束。未解決スレッドは 0 件(全 8 件を返信 + Resolve)。

roundcodexgemini直したこと
1REQ (2)SKIPgit add -A の巻き込み / 全件取り消し経路の記録漏れ
2REQ (2)APP状態コード比較の取りこぼし / index の staged 変更
3REQ (1)APPプロンプトの同期タイミングが古い
4REQ (1)REQ (1)同期失敗後の再開不能 / パスの引用
5REQ (2)APP復旧が index を戻さない / Step 7 の記述矛盾
6APPAPP

9 件の指摘はすべて実在の欠陥で、うち 7 件は不具合 10 の対策として書いたコード自身に含まれていた。生成物の同期は、index・作業ツリー・無視設定・パス引用・失敗時の復旧を同時に扱う必要があり、想定より難しい題材だった。

関連

🤖 Generated with Claude Code

https://claude.ai/code/session_01GSwBvT9CH8mKfgyFn2JWfS

再検証(PR #120)で見つかった不具合 10・11 を直す。
不具合 10: pre-push で生成物の同期を検査するリポジトリでは、「実装担当は編集元
だけを触る」ルールと衝突して**あらゆる push が落ちる**。さらにこの検査は実装担当を
範囲ルール違反へ誘導し、実測ではラウンドの採用 5 件が全件範囲外で失敗した。
- 実装担当は push しない。公開するのは進行側だけで、**検証を通した後**に行う
- `--sync-command` を新設し、**push の直前**に進行側が実行する。差分があれば
どの改善項目にも属さないコミットとして積む。同期の失敗は中断(終了コード 4)
- `merge-apply` / `merge-fix` は成功・失敗のどちらでも検証後に push する
不具合 11: 適用の検証で失敗した項目が `deferred_items` に入らず、次ラウンドで
同じ提案が再び採用されていた(実測では 3 ランタイム全員から再提案され最優先で採用)。
- `_defer_abandoned_items()` を追加し、取り消しの完了時に理由付きで記録する
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01GSwBvT9CH8mKfgyFn2JWfS

@takemi-ohamatakemi-ohama left a comment

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤖 cross-review | round 1 | codex | REQUEST_CHANGES

同期コミットの対象限定と、ラウンド単位の適用失敗も対象外へ記録する経路の修正が必要です。

Comment threadplugins/ndf-shared/skills/cross-refactoring/scripts/refactor.py Outdated

@takemi-ohamatakemi-ohama left a comment

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤖 cross-review | round 1 | gemini | APPROVE

設計と実装を詳細に確認しました。公開(push)の責務を進行側へ移譲するロジック、--sync-command の push 直前での実行・失敗時の安全な中断処理、および適用失敗項目の deferred_items への記録による再提案の防止処理は、いずれも正しく堅牢に実装されています。

ドキュメントとコードの整合性も取れており、起点の SHA 管理と同期コミットのライフサイクルも期待通りに動作することを確認しました。修正は不要です。

- `git add -A` は同期前からあった未追跡・変更済みファイルまで巻き込んでいた。
同期コミットは項目の検証を通らないため、紛れ込んだ変更が検査を受けずに
Pull Request へ入る。同期の前後を比べて**同期コマンドが作った差分だけ**を
stage し、制御用ディレクトリは明示的に除外する
- `_defer_abandoned_items()` を、未割当コミットなどでラウンド全体を取り消す
経路からも呼ぶ。呼ばないと同じ提案が次のラウンドで再び採用される
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01GSwBvT9CH8mKfgyFn2JWfS

@takemi-ohamatakemi-ohama left a comment

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤖 cross-review | round 2 | codex | REQUEST_CHANGES

同期コマンドが生成した差分だけを確実に識別し、同期前から index にある変更を同期コミットへ混入させない形に修正してください。

Comment threadplugins/ndf-shared/skills/cross-refactoring/scripts/refactor.py Outdated

@takemi-ohamatakemi-ohama left a comment

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤖 cross-review | round 2 | gemini | APPROVE

指摘事項なし。進行側への責務移動、同期失敗時の中断と冪等性、対象外の記録による収束ループの保護が、いずれも要件と不変条件通りに実装・テストされていることを確認しました。

状態コードの比較では、同期が作った差分と元からあった差分を区別しきれなかった。
- 元から ` M` のファイルを同期がさらに書き換えても状態コードは変わらず、
その変更がコミットされないまま push がまた落ちる
- `git add` の対象を絞っても、`git commit` は index を丸ごと含めるため、
先に staged だった変更が検証を受けないまま公開される
比較をやめ、同期の前に作業ツリーが綺麗であることを求める。汚れていれば中断し、
コミットするか `.gitignore` へ入れるよう促す。制御用ディレクトリは判定から外す。
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01GSwBvT9CH8mKfgyFn2JWfS

@takemi-ohamatakemi-ohama left a comment

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤖 cross-review | round 3 | gemini | APPROVE

公開の責務の進行側への移行、--sync-command の追加、適用失敗項目の対象外記録について、既存のループ設計と整合し安全に実装されていることを確認しました。構造的な修正提案はありません。

同期は「収束後にまとめて」ではなく、`merge-apply` / `merge-fix` などの
**各 push の直前**に進行側が行う。プロンプト 2 つと検証の失敗理由を追従させる。
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01GSwBvT9CH8mKfgyFn2JWfS

@takemi-ohamatakemi-ohama left a comment

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤖 cross-review | round 4 | codex | REQUEST_CHANGES

同期失敗後の再開性に修正が必要です。

@takemi-ohamatakemi-ohama left a comment

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤖 cross-review | round 4 | gemini | REQUEST_CHANGES

同期前後の未コミット変更の抽出処理について、パスのエスケープに起因するバグを防ぐ修正を提案します。

Comment threadplugins/ndf-shared/skills/cross-refactoring/scripts/refactor.py Outdated
- 同期が途中まで書き換えて失敗すると差分が残り、次の実行は清浄性の検査で必ず
止まって `pending_push` の再試行が永久に進まなかった。着手前が綺麗であることを
確認済みなので、失敗時は同期が作った差分を捨ててから中断する
- `core.quotePath` の既定では非 ASCII を含むパスがエスケープされ、そのまま
`git add` へ渡すと見つからない。`-c core.quotePath=false` を付ける
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01GSwBvT9CH8mKfgyFn2JWfS

@takemi-ohamatakemi-ohama left a comment

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤖 cross-review | round 5 | codex | REQUEST_CHANGES

同期失敗時の再開保証と、同期タイミングの手順統一を修正してください。

@takemi-ohamatakemi-ohama left a comment

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤖 cross-review | round 5 | gemini | APPROVE

設計、実装、テストともに堅牢で、全ての要件(push 責務の移行、同期コマンドの実行順序とエラー処理、対象外アイテムの再提案防止)が漏れなく組み込まれています。
状態マシンのフェーズ遷移、ならびにクラッシュ時の再開(pending_push を利用した retry 機構)も破綻なく設計・実装されていることを確認しました。

- `git checkout -- .` は staged された差分を戻さないため、同期コマンドが
`git add` してから失敗すると清浄性の検査が通らないままになり、
`pending_push` を再試行できなかった。`git reset --hard HEAD` へ変える
- Step 7 に「終了後にここでまとめて同期する」が残っており、push 直前同期と
矛盾していた。追加の同期は要らない旨へ統一する
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01GSwBvT9CH8mKfgyFn2JWfS

@takemi-ohamatakemi-ohama left a comment

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤖 cross-review | round 6 | codex | APPROVE

修正を要求する新規指摘はありません。

@takemi-ohamatakemi-ohama left a comment

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤖 cross-review | round 6 | gemini | APPROVE

公開の責務を進行側に移したことによる状態管理や push の再試行機構が、想定しうるエッジケースを含め堅牢に実装されています。
--sync-command の導入と適用失敗時の対象外記録についても仕様通りであり、既存の仕組みと矛盾せず安全に統合されています。
追加の指摘事項はありません。

@takemi-ohama
takemi-ohama marked this pull request as ready for review August 16, 2026 18:49
@takemi-ohama
takemi-ohama merged commit a415242 into mainAug 16, 2026
7 checks passed
@takemi-ohama
takemi-ohama deleted the fix/cross-refactoring-sync-and-defer branch August 16, 2026 20:15
takemi-ohama added a commit that referenced this pull request Aug 16, 2026
不具合 9 件の修正(#119 / v8.2.0)を実機で確かめた記録を残す。
修正した 9 件はすべて実機で成立し、前回(#118)は到達できなかったレビュー・判定・
実装担当の輪番・集計まで通った。特に本丸だった取り消しの巻き戻しと積み直しは、
前回破綻したのと同じ条件(採用項目の多くが同一ファイルを触る)で成立している。
あわせて新しい不具合を 2 件見つけた(#121 で修正済み)。
- 10: pre-push の同期検査と範囲ルールが両立せず、あらゆる push が落ちる
- 11: 適用で失敗した項目が「対象外」に入らず、同じ提案が再採用される
構造改善の成果 4 項目は取り込まない。#121 が同じ refactor.py を大きく変えており、
手でコンフリクトを解消すると「2 者のレビューを通った内容」という性質が失われる。
指摘の修正と再レビューの繰り返し、上限到達時の項目単位の見送りは依然として未到達。
takemi-ohama added a commit that referenced this pull request Aug 17, 2026
#121 で公開インタフェースが変わったのに版数が v8.2.0 のままだったため揃える。
- `--sync-command` の新設(追加)
- 実装担当が push しない、`merge-apply` / `merge-fix` が常に push する(破壊的)
版数の基準である Claude 版 plugin.json を更新し、生成物(Kiro の VERSION)は
build-runtime-plugins.sh で同期した。README へ v8.3.0 の変更内容を追記し、
引継ぎメモの「別件で残っているタスク」からバージョン項目を削除した。
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DhXHMSW7A61J3H9LMtJbLT
takemi-ohama added a commit that referenced this pull request Aug 17, 2026
* Docs: cross-refactoring 再々検証の引継ぎメモを追加
未到達の 2 項目(指摘の修正と再レビューの繰り返し、上限到達時の項目単位の
見送り)を通すための作業メモ。実測で踏んだ落とし穴と所要時間の目安、
先に決めること(指摘が出る確率をどう上げるか)を残す。
あわせて、再々検証とは独立に残っている 2 件も記録する。
- #121 が公開インタフェースを変えたのにバージョンが v8.2.0 のまま
- cross-review が投稿の成否を突き合わせていない(#121 のラウンド 3 で実際に発生)
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01GSwBvT9CH8mKfgyFn2JWfS
* Update: NDF プラグインを v8.3.0 へ更新
#121 で公開インタフェースが変わったのに版数が v8.2.0 のままだったため揃える。
- `--sync-command` の新設(追加)
- 実装担当が push しない、`merge-apply` / `merge-fix` が常に push する(破壊的)
版数の基準である Claude 版 plugin.json を更新し、生成物(Kiro の VERSION)は
build-runtime-plugins.sh で同期した。README へ v8.3.0 の変更内容を追記し、
引継ぎメモの「別件で残っているタスク」からバージョン項目を削除した。
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DhXHMSW7A61J3H9LMtJbLT
---------
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
takemi-ohama added a commit that referenced this pull request Aug 18, 2026
未到達だった 2 経路(指摘の修正と再レビュー / 上限到達時の項目単位の見送り)を
実機で通した。あわせて PR #121 で入れた 3 経路も確認し、新しく 4 件の不具合を
見つけた。うち 2 件は生成物を持つリポジトリで進行を止める。
役目を終えた引継ぎメモは、このレポートへ置き換えた。
Claude-Session: https://claude.ai/code/session_01WxxF27RXyg3QMjayii5dp1
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant

@takemi-ohama