diff --git a/docs/bugfix-batch-plan.md b/docs/bugfix-batch-plan.md index 24cd9458..fd616412 100644 --- a/docs/bugfix-batch-plan.md +++ b/docs/bugfix-batch-plan.md @@ -13,10 +13,10 @@ | # | PR | 対象順位 | 状態 | |---|---|---|---| | A | fix(merge-pipeline): feedback ループの誤 bail・誤ブロック解消 | 444 + 328 + 347 | **完了。** 444 は [PR #417](https://github.com/aloekun/claude-code-hook-test/pull/417) でマージ済み。328 は順位 398 の guard 変更で既に解消済みと判明し、再発防止テストのみ追加。347 は実装済み | -| B-1 | fix(merge-pipeline): transcript の連結順序を時系列にする | 446 (再定義) | 実装済み | -| B-2 | fix(merge-pipeline): 分析ソース選定を陽性照合ベースに統一 | 336 + 288(a) | 実装済み | -| B-3 | fix(merge-pipeline): transcript 抽出を workspace 横断にする | 469 (446 から分離) | 実装済み | -| C | fix(hooks): smoke suite の ETXTBSY 解消 | 396 | 未着手 | +| B-1 | fix(merge-pipeline): transcript の連結順序を時系列にする | 446 (再定義) | **完了** ([PR #419](https://github.com/aloekun/claude-code-hook-test/pull/419)) | +| B-2 | fix(merge-pipeline): 分析ソース選定を陽性照合ベースに統一 | 336 + 288(a) | **完了** ([PR #420](https://github.com/aloekun/claude-code-hook-test/pull/420))。実 run で照合成功を確認済み | +| B-3 | fix(merge-pipeline): transcript 抽出を workspace 横断にする | 469 (446 から分離) | **完了** ([PR #421](https://github.com/aloekun/claude-code-hook-test/pull/421)) | +| C | fix(hooks): smoke suite の ETXTBSY 解消 | 396 | 実装済み | | D | fix(check-ci-coderabbit): rate-limit 第 3 format + 実レビュー有無分離 | 318 + 320 | 未着手 | | E | fix(ci): 監視系 workflow の誤動作修正 | 319 + 431 | 未着手 | | F | fix(pr-monitor): cli-pr-monitor 小修正束 | 246 + 292 + 385 | 未着手 | @@ -38,6 +38,35 @@ 5. **jj squash は `-u` を付ける** (source/dest 両方に description があると editor 起動で headless hang)。 6. 各 PR の DoD: `cargo test --workspace` green (+ 該当 crate の clippy)。workflow を触る PR は `pnpm lint:workflows` も。 7. **夜間ループとの競合**: 着手前に該当ファイルを触る `claude/nightly-*` ブランチが無いか確認する。既知の衝突は PR D の節に記載。 +8. **マージ後は `pnpm build:all` を実行する**。`.claude/*.exe` は gitignore 対象で、**マージしただけでは挙動が変わらない**。PATH に Git の coreutils が要る (`export PATH="$PATH:/c/Program Files/Git/usr/bin"`、`cp.exe` のため)。 + - **ビルドの成否は exe の mtime で判断しない。** 完了通知とファイル書き込みの間にずれがあり、2026-08-19 に「更新されていない」と誤判定した。中身で確かめる: `grep -c "<その PR で追加した文字列>" .claude/` + +## 着手前に必ずやること + +本計画の PR A〜B-3 (2026-08-18〜19) で実際に踏んだ穴。以降の PR C〜L でも同じ形で再発する。 + +**台帳の記述をそのまま信じない。** 本計画で着手した 6 件のうち **5 件で台帳と実態がずれていた**。 + +| 順位 | 台帳の記述 | 実際 | +|---|---|---| +| 444 | 未着手 | 本計画とは独立に PR #417 として起票済みだった | +| 328 | leftover context.json で誤 bail する | 順位 398 の guard 変更で**前提が消滅**していた | +| 347 | `cli-merge-pipeline` の欠陥 | 実装先は `cli-pr-monitor` | +| 446 | 並列 workspace のセッションが不可視 | 真因は**連結順序が時系列でないこと** | +| 336 | 時刻範囲のみで照合しない | 時刻範囲すら使わず**辞書順で最新 1 件** | + +台帳は起票時点のスナップショットで、実装が動くほどずれる。328 と 446 は、台帳どおりに実装すれば**存在しない不具合を直すか誤った箇所を直す**ところだった。**自分が前日に書いたエントリでも同じ** — 順位 469 の Frequency 評価は実測で覆った。 + +**自分の修正が下流の分岐を変えていないか追う。** CodeRabbit の Major 指摘 2 件が同じ構造だった。 + +- [PR #418](https://github.com/aloekun/claude-code-hook-test/pull/418): `FixCommitState::None` を返す変更が、下流 `decide_repush_action` の `(HasChange, _, true) => AutoPush` 経路を開いた (分離コミットなしで push される) +- [PR #421](https://github.com/aloekun/claude-code-hook-test/pull/421): 走査 source を単数→複数にしたのに `?` を残し、1 ディレクトリの失敗で全 workspace 分が失われる構造になった + +いずれも**局所的には正しい変更が、周囲の構造が変わったことで別の意味を持った**。シグネチャ・戻り値・引数の数を変えたら、呼び出し元と下流の `match` を必ず追う。 + +**識別子を照合キーにするなら、一意性の根拠をリポジトリ全体で確認する。** [PR #420](https://github.com/aloekun/claude-code-hook-test/pull/420) で bookmark 名を一意キーと仮定したが、`claude/nightly-<順位>` は夜間ループが再利用する。CodeRabbit はリポジトリ全体を走査して気づき、私は変更箇所の周辺しか見ていなかった。 + +**doc に「機構がある」と書いたら、その機構が実在するか確かめる。** 3 度やった — `RUN_REPORT_FILE_NAME` の pin テスト (存在しなかった / [#418](https://github.com/aloekun/claude-code-hook-test/pull/418) で追加)、fix step が書いた「3 crate を共通化」(1 crate だけだった)、`sort_key` の mtime tie-break (`source_path` へ変えた後も doc が残っていた)。 --- @@ -117,10 +146,27 @@ - **不具合**: `src/hooks-pre-tool-validate/tests/smoke.rs` の 2 テストが並列で exe を tempdir へ `fs::copy` → spawn するため、Linux で片方の copy 中の書き込み fd を fork した子が継承し、exec が `Text file busy` (os error 26) で落ちる。PR #376 の CI (ubuntu のみ) で実観測。flaky の放置は「また flake だろう」で実バグを見落とす経路になる (2026-08-10 ユーザー判断で Tier 1 格上げ)。 - **対処** (実装時に選択): (a) ci.yml の hooks smoke step を `--test-threads=1` (最小・即効)、(b) spawn を ETXTBSY でリトライ、(c) staging をやめて `built_exe()` 直接起動 (config staging 設計との整合要確認)。 -- **手順**: WSL Ubuntu で並列実行を再現させてから直し、同じ手順で消えたことを確認。他の smoke/E2E suite に「copy してから spawn」同型パターンが無いか棚卸しする。 +- **手順**: **まず WSL Ubuntu-24.04 で並列実行を再現させる** — 3 案のどれを採るかは再現の観察 (どの段で落ちるか) に依存するため、再現前に実装へ入らない。環境は導入済み (`wsl -u root` はパスワード不要 / PowerShell から複数行 bash を渡すと壊れるのでスクリプトファイル経由)。再現したら直し、同じ手順で消えたことを確認する。他の smoke/E2E suite に「copy してから spawn」同型パターンが無いか棚卸しする。 + - **再現しなかった場合は実装に入らずユーザーに相談する。** ETXTBSY は fd 継承のタイミング依存で、ローカル WSL では CI の並列度・I/O 特性が再現しないことがある。測定できないまま 3 案から選ぶと「効いたかどうか確認できない修正」を入れることになり、flaky を Tier 1 に上げた趣旨 (「また flake だろう」で実バグを見落とす経路を塞ぐ) に反する。 - **完了基準**: Linux で ETXTBSY が出ないこと (再現手順付き)。同型パターンの棚卸し完了。 - **後始末**: todo21.md「hooks smoke suite の並列実行が…」節 + todo-summary2.md 396 行を削除。 +> **実装済み (2026-08-19)。採ったのは 3 案のどれでもなく「copy と spawn の相互排除」だった** — 再現の観察から選び直した。以下は記録。 +> +> **再現**: WSL Ubuntu-24.04 の **ext4 上** (`~/etxtbsy`) にリポジトリを複製してテストバイナリを 200 回回すと **43 回 ETXTBSY** (別測定で 30/200)。`--test-threads=1` と各テスト単独では 0/100 で、**2 テストの並列実行に固有**と確定した。`/mnt/c` (drvfs) 上では再現しない。 +> +> **3 案を実測比較した** (各 200 run): (a) `--test-threads=1`、(b) spawn リトライ、(c) 共有 staging (`LazyLock`) — **どれも 0 件**に落ちた。決め手は副作用のほう: +> +> - **(a) は穴が残る**。smoke テストは専用 step (ci.yml:175) だけでなく `cargo test --workspace` (ci.yml:168) でも走るので、専用 step だけ直列化しても同じ race が残る。workspace 全体の直列化はコストが大きい。 +> - **(c) は `LazyLock` の `TempDir` が drop されず、実測で 1 run あたり 37MB を `/tmp` に残した** (200 run で 7.2GB)。固定パス staging へ変えればリークは消えるが、「`target/debug` を汚さない」という smoke.rs の設計意図と衝突する。 +> - **(b) は原因 (fd 継承) に触れず症状を待つ形**で、テストコードに retry ループが入る。 +> +> **採った対処**: `static EXEC_STAGING_LOCK: Mutex<()>` で **copy と spawn (fork〜exec) を相互排除**する。`Command::spawn` は子の exec 完了まで親へ返らないため、spawn 呼び出しを囲めば fd 継承の窓が閉じる。テストごとの tempdir 分離と後始末はそのままで、リークも無い。 +> +> **回帰 seal**: `concurrent_staging_and_spawn_survives_etxtbsy` (`#[ignore]`、8 スレッド × 16 ラウンド) を追加。**ロックを外すと Linux で 10/10 落ち、戻すと 0/10** (ADR-049 の「修正前に落ちることを確認」)。CI は `--ignored --test-threads=1` の leg で回す。所要 Linux 2.1s / Windows 9.7s。 +> +> **同型パターンの棚卸し**: リポジトリ全体で「exe を copy してから spawn」は 2 ファイルのみ。`hooks-stop-quality/tests/t7_cwd_independence.rs` は **5 テスト全部が copy→spawn** で、現状 `#![cfg(windows)]` のため POSIX 経路は踏まないが、cfg を外した瞬間に smoke.rs 以上の危険度になるため**同じガードを入れた**。他 (`hooks-post-tool-linter/tests/incident_eval.rs`、`hooks-stop-tool-call-leak/tests/e2e.rs`) は exe をコピーせず直接 spawn するため対象外。 + --- ## PR D: fix(check-ci-coderabbit): rate-limit 第 3 format 対応 + 実レビュー有無の分離 (順位 318 + 320) @@ -295,6 +341,26 @@ --- +## 保留事項 (PR D 完了時に扱う) + +ユーザー判断で PR D まで先送りしたもの。**本計画の外に記録が無いため、ここが唯一の記録である。** + +### フィードバック採否 (5 PR 分が滞留) + +[PR #417](https://github.com/aloekun/claude-code-hook-test/pull/417) / [#418](https://github.com/aloekun/claude-code-hook-test/pull/418) / [#419](https://github.com/aloekun/claude-code-hook-test/pull/419) / [#420](https://github.com/aloekun/claude-code-hook-test/pull/420) / [#421](https://github.com/aloekun/claude-code-hook-test/pull/421) の post-merge-feedback で挙がった採用候補を**未処理で溜めている** (2026-08-18 ユーザー判断: 「PR D まで完了したタイミングで実施」)。レポートは `.claude/feedback-reports/.md` にある (gitignore 対象なのでローカルのみ)。 + +PR D 完了時に 5 件分をまとめて採否判定する。件数が多いので、系統ごとに統合してから台帳へ登録する運用が要る (先例: `docs/todo24.md` の「#409-#414 の 5 PR 分」を 3 タスクへ統合した節)。 + +### `cwd_to_project_id` の Linux での case 不一致 + +**順位 469 のエントリを削除した際にこの記録も消えたため、ここに移設する** (2026-08-18 ユーザー判断: 「case 問題は D の作業完了後に対応を検討」)。 + +`src/cli-merge-pipeline/src/feedback/transcript.rs` の `cwd_to_project_id` は path を `to_lowercase()` するが、`~/.claude/projects/` の実フォルダ名は**大文字小文字が保存されている** (`c--Users-owner-...` と `C--Users-owner-...-improve` が併存)。Windows は case-insensitive なので現状は偶然動いているだけで、**case-sensitive filesystem では一致しない**。 + +- **未検証**: Linux の典型的なパスは全小文字なので `to_lowercase()` が実質 no-op になり、発現しない可能性が高い。WSL Ubuntu で確認できる (→ memory `wsl-linux-verification-setup`) +- **影響範囲**: [ADR-063](adr/adr-063-linux-portability-release-binaries.md) のクラウドセッションと [ADR-065](adr/adr-065-ci-matrix-cross-os-regression.md) の Linux CI matrix +- **B-3 の範囲外とした理由**: 発現条件が限定的で、workspace 横断の本体とは独立に直せるため + ## 残観測トラッキング 完了基準に実走観測を含むタスク。マージ後に観測し、確認できたらエントリ後始末 (todoN.md 節 + summary 行の削除) を docs バッチで行う。 @@ -311,7 +377,9 @@ 1. 進行表の 12 PR がすべてマージ済みであること 2. [§ 残観測トラッキング](#残観測トラッキング) の 4 項目がすべて消化され、対応するエントリ後始末が完了していること 3. 順位 288 のエントリ (todo15.md) が PR I 完了時に削除されていること -4. `grep -rn "bugfix-batch-plan" .` で本ファイルへの参照が残っていないことを確認する (検索対象パス `.` を省くと標準入力待ちになるため必ず付ける) -5. 本ファイルを物理削除する (削除自体は残観測の最後のエントリ後始末と同じ docs バッチ PR に同乗してよい) +4. **[§ 保留事項](#保留事項-pr-d-完了時に扱う) が空であること** — 未処理のまま残っていれば、行き先を作ってから削除する (フィードバック採否は消化、`cwd_to_project_id` の case 問題は台帳エントリへ起票)。**本ファイルが唯一の記録である項目を、本ファイルの削除と一緒に消してはならない。** 順位 469 のエントリ削除で実際にこれをやり、記録を一度失った +5. **[§ 着手前に必ずやること](#着手前に必ずやること) の各項が、本ファイル外へ移送済みであること** — 台帳前提の実測・シグネチャ変更時の下流追跡・照合キーの一意性確認・doc 記述の実在確認はいずれも本計画に固有でない再発防止知見なので、[dev-conventions.md](dev-conventions.md) へ移す +6. `grep -rn "bugfix-batch-plan" .` で本ファイルへの参照が残っていないことを確認する (検索対象パス `.` を省くと標準入力待ちになるため必ず付ける) +7. 本ファイルを物理削除する (削除自体は残観測の最後のエントリ後始末と同じ docs バッチ PR に同乗してよい) -永続化すべき知見 (再発防止策・設計判断) は各 PR で ADR / module doc / dev-conventions に書き込む方針のため、本ファイルに永続価値は残らない。 +永続化すべき知見 (再発防止策・設計判断) は各 PR で ADR / module doc / dev-conventions に書き込む方針のため、**上記 4・5 を終えた後の**本ファイルに永続価値は残らない。 diff --git a/docs/harness-improvement-plan.md b/docs/harness-improvement-plan.md index fc07dbaa..ad1e38b5 100644 --- a/docs/harness-improvement-plan.md +++ b/docs/harness-improvement-plan.md @@ -181,7 +181,7 @@ WP-18 が生んだ / WP-18 の運用で踏む問題の 5 件（順位 397 / 398- | 内容 | 管理先 | 期限 / 条件 | |---|---|---| -| **順位 396: hooks smoke suite の Linux `ETXTBSY` flake** — WP-18 の CI で見つかったが別クレートの既存テスト競合で、WP-18 の経路とは無関係。**flaky テストは「また flake だろう」で実バグを見落とす経路を作る**ため、両 OS matrix（[ADR-065](adr/adr-065-ci-matrix-cross-os-regression.md)）の信号品質を守る意味で**早期に潰す** | [todo21.md](todo21.md) | **高優先度**（WP-18 とは独立に着手） | +| **順位 396: hooks smoke suite の Linux `ETXTBSY` flake** — WP-18 の CI で見つかったが別クレートの既存テスト競合で、WP-18 の経路とは無関係。**flaky テストは「また flake だろう」で実バグを見落とす経路を作る**ため、両 OS matrix（[ADR-065](adr/adr-065-ci-matrix-cross-os-regression.md)）の信号品質を守る意味で**早期に潰す** | [smoke.rs](../src/hooks-pre-tool-validate/tests/smoke.rs) の `EXEC_STAGING_LOCK` | **完了**（2026-08-19。staging と spawn を相互排除。WSL Ubuntu-24.04 で 30/200 → 0/200 を実測し、`concurrent_staging_and_spawn_survives_etxtbsy` で seal） | | **順位 411: `cargo fmt` を PreToolUse でブロック** — WP-18 作業中の誤実行が発端だが、対象は開発環境全般。**規約ではなく機構で弾く**判断（[ADR-042](adr/adr-042-rule-vs-mechanism-boundary.md)）。反射的に実行されやすく無関係な差分を生むため**早期に塞ぐ** | [todo21.md](todo21.md) | **高優先度**（WP-18 とは独立に着手） | | 順位 402-409（post-merge feedback 採用分のうち一般則。観測の完全性 / 重複実装の予防 / shell·config パースの安全性）。**順位 410 のみ (2) へ分類**した（WP-18 成果物自身の堅牢化のため） | [todo-summary2.md](todo-summary2.md) | リポジトリ全体に適用する一般則 | | 順位 382（injection payload regression test。依存先の順位 380 完了で unblock）/ 順位 383（`is_separator_row` のパイプ検証欠落） | [todo-summary2.md](todo-summary2.md) | 🔧 Tier 2、任意 | diff --git a/docs/todo-summary2.md b/docs/todo-summary2.md index 65c0ab01..e9efa87d 100644 --- a/docs/todo-summary2.md +++ b/docs/todo-summary2.md @@ -134,7 +134,6 @@ | 390 | 🔧 Tier 2 | **台帳 framing 区切りの定数と workflow リテラルの cross-file 一致を CI 検証 (#369 T2 採用)** | todo21.md | M | なし (LEDGER_DATA_FRAME_MARKER と ===BEGIN/END_LEDGER_DATA=== が対。片側変更で ADR-072 決定 13 の framing が破れる) | | 391 | 🔧 Tier 3 | **jj の落とし穴 (squash 方向・空コミットでの bookmark ずれ) を dev-conventions へ (#369 T3 採用)** | todo21.md | S | なし (本セッションで複数回踏んだ。コミット確定は describe+bookmark set、new は新作業時のみ、を明文化) | | 392 | 🔧 Tier 3 | **push パイプラインの terminal outcome を telemetry へ記録し失敗回数・原因を機械集計可能にする** | todo21.md | M | なし (2026-08-09 WP-18 失敗頻度分析で構造化記録の欠落が判明。stage + reason code を ADR-055 系へ fail-open で追記し ADR-062 月次で集計。順位 386/387/376 の効果測定ベースラインにもなる) | -| 396 | 🚀 Tier 1 | **hooks smoke suite の並列実行が Linux で `ETXTBSY` を起こす (flaky テスト、早期修正)** | todo21.md | S | なし (#376 CI で ubuntu のみ失敗、windows は成功、当該クレートは無変更。2 テストが並列に exe を copy→spawn し、fork した子が copy 側の書き込み fd を継承するため exec が Text file busy。直近 15 run で初出だが ADR-065 の両 OS matrix の信号品質を下げる。**flaky を放置すると「また flake だろう」で実バグを見落とす**ため WP-18 とは独立に早期着手する = 2026-08-10 ユーザー判断で Tier 1 へ格上げ) | | 402 | 🚀 Tier 1 | **「対処後は効果を観測するまで完了と見なさない」を明文化 (系統 A-1)** | todo21.md | S | なし (2026-08-10 採用。決定 11 は投稿の成否だけ見て 10 時間気づけず、決定 15 は同じ症状が続くか確かめる前に解決済みと記録した。fail-open は効果の観測を別に用意して初めて成立する) | | 403 | 🚀 Tier 1 | **AI レビューの数値・外部仕様の主張は仮説として扱い実測で二重検証 (系統 A-2)** | todo21.md | S | なし (2026-08-10 採用。組合せ数の指摘は観察は正しいが提示値も誤り (実測 384)、gh のオプション併用提案は実行時エラー、jq 正規表現案はパースエラー。観察と修正手段の確信度は別) | | 404 | 🔧 Tier 2 | **外部依存の非同期応答待ちに timeout / retry を明記する convention (系統 A-3)** | todo21.md | S | なし (2026-08-10 採用。cli-stale-branch-scan の初版が timeout 無しで、同期実行経路の無診断ハング要因だった) | diff --git a/docs/todo21.md b/docs/todo21.md index 6c5fa903..281f9be0 100644 --- a/docs/todo21.md +++ b/docs/todo21.md @@ -207,48 +207,6 @@ > > **順位 395 (週次レビューでの浮きブランチ検出) も実装済み・削除済み** (2026-08-09)。`cli-stale-branch-scan` として実装し、`pnpm stale-branch-scan` で実行する。設計は [ADR-031](adr/adr-031-weekly-review-pipeline.md) § 残存ブランチ検出 が正 — **takt workflow はネットワークを持たない** (`network_access: false`) ため決定論 scan を skill 側 (L3) に置いた経緯もそちらに記録した。 -## CI 安定性 (2026-08-09 登録) - -### hooks smoke suite の並列実行が Linux で `ETXTBSY` を起こす - -> **由来**: PR [#376](https://github.com/aloekun/claude-code-hook-test/pull/376) の CI で `rust (ubuntu-latest)` が失敗した。`windows-latest` は成功、変更は当該クレートを 1 ファイルも触っていない。 -> -> ```text -> test malformed_stdin_does_not_block ... FAILED -> panicked at src/hooks-pre-tool-validate/tests/smoke.rs:137: -> spawn hooks-pre-tool-validate: Os { code: 26, kind: ExecutableFileBusy, -> message: "Text file busy" } -> ``` -> -> **機構**: [smoke.rs](../src/hooks-pre-tool-validate/tests/smoke.rs) の 2 テストは cargo 既定で並列実行される。各テストは `stage_hook()` で exe を tempdir へ `fs::copy` してから spawn する。片方の `fs::copy` が**書き込み用 fd を開いている最中に**、もう片方の `Command::spawn()` が fork すると、子プロセスがその fd を継承する。copy 側が fd を閉じても fork された子が exec するまで複製が残るため、copy 側の exec が `ETXTBSY` で落ちる。Linux 固有 (Windows では再現しない)。 -> -> **頻度**: 直近 15 回の `ci.yml` run で初出。恒常的ではないが、**両 OS matrix (ADR-065) が意味を持つのは CI が信頼できるときだけ**で、原因不明の赤が続くと「また flake だろう」と実バグを見落とす経路になる。 -> -> **対処案** (実装時に判断): -> -> - (a) `ci.yml` の hooks smoke step を `--test-threads=1` にする — 最小・即効だが、他 suite の並列性は保たれるので損失は小さい -> - (b) `run_hook` の spawn を `ETXTBSY` でリトライする — 根本に近いが、テストコードに retry を持ち込む -> - (c) staging をやめて `built_exe()` を直接起動する — copy 自体が消えるが、config を tempdir に staging する設計 (テストの副作用隔離) と噛み合うか要確認 -> -> **参照**: [ADR-065](adr/adr-065-ci-matrix-cross-os-regression.md) (両 OS matrix の意義)、PR [#376](https://github.com/aloekun/claude-code-hook-test/pull/376)。 -> -> **実行優先度**: 🚀 Tier 1 — Severity Medium (実バグではないが CI の信号品質を下げる) / Frequency Low (初観測) / Effort S / Adoption Risk Low (テスト実行方法の変更のみ)。 -> -> **2026-08-10 に Tier 1 へ格上げ (ユーザー判断)**。単発の Severity では Tier 2 相当だが、**flaky テストは「また flake だろう」という読み替えを生み、実バグの見落とし経路になる**。両 OS matrix ([ADR-065](adr/adr-065-ci-matrix-cross-os-regression.md)) の信号品質そのものを守る意味で早期に潰す。**WP-18 の完了条件には含めない** (別クレートの既存競合で WP-18 の経路と無関係、計画書 § WP-18 残作業 (3) 参照) が、着手は WP-18 と独立に早める。 - -#### 作業計画 - -- [ ] 対処案 (a)-(c) から選び、ローカル (WSL Ubuntu) で並列実行を再現させてから直す -- [ ] 修正後に同じ再現手順で `ETXTBSY` が出ないことを確認する -- [ ] 他の smoke/E2E suite に同型の「copy してから spawn」パターンが無いか棚卸しする - -#### 完了基準 - -- hooks smoke suite が Linux で `ETXTBSY` を起こさないこと (再現手順付きで確認)。 -- 同型パターンが他 suite に無いこと、またはあれば同じ対処が入っていること。 - ---- - ## post-merge feedback 採用分 (#376/#377/#380/#381/#382、2026-08-10 採否確定) > **由来**: WP-18 の一連 PR の post-merge feedback で挙がった採用候補を、2026-08-10 に系統別へ分類してユーザーが採否を決定した。**系統 A (観測の完全性) / B (重複実装の予防) / C (shell・config パースの安全性) を採用**、系統 D (workflow セキュリティ標準化) / E (PAT 失効監視) は却下 (様子見)。 diff --git a/src/hooks-pre-tool-validate/tests/smoke.rs b/src/hooks-pre-tool-validate/tests/smoke.rs index cd47f410..39416bbf 100644 --- a/src/hooks-pre-tool-validate/tests/smoke.rs +++ b/src/hooks-pre-tool-validate/tests/smoke.rs @@ -25,6 +25,7 @@ use lib_subprocess::{drain_pipe_unlimited, wait_with_timeout_safe}; use std::io::Write; use std::path::{Path, PathBuf}; use std::process::{Command, Stdio}; +use std::sync::{Mutex, MutexGuard, PoisonError}; /// spawn した hook exe の bounded wait (dev-conventions.md § bounded wait)。 /// ハングした子プロセスは kill してテストを失敗させ、CI を無期限に止めない。 @@ -99,9 +100,40 @@ fn repo_root() -> PathBuf { PathBuf::from(env!("CARGO_MANIFEST_DIR")).join("..").join("..") } +/// **exe の staging (copy) と spawn (fork〜exec) を相互排除する** ロック。 +/// +/// これが無いと Linux で `ETXTBSY` (`Text file busy`) が出る。機構は次のとおり: +/// テスト A の `fs::copy` が staging 先 exe への **書き込み用 fd を開いている最中に**、 +/// 並列のテスト B が `Command::spawn` で fork すると、その fd が子プロセスへ複製される。 +/// `O_CLOEXEC` は **execve が完了した時点**で閉じるため、B の子が exec するまでの間は +/// A の exe が「書き込み用に開かれたファイル」のままで、A がそれを exec しようとすると +/// カーネルが `ETXTBSY` で拒否する。テストごとに temp dir が別でも起きる +/// (fd はパスではなくプロセスに紐づくため)。 +/// +/// **ロックが copy と spawn の両方を囲む必要がある**。`Command::spawn` は +/// posix_spawn 経路でも fork+exec 経路でも「子の exec が完了 (または失敗) するまで」 +/// 親へ返らないので、spawn 呼び出しを囲めば fd 継承の窓が閉じる。片側だけでは意味が無い。 +/// +/// 実測 (WSL Ubuntu-24.04 / ext4、テストバイナリを 200 回実行): +/// ロック無し = 30 回 ETXTBSY、ロック有り = 0 回。再現手順は +/// `concurrent_staging_and_spawn_survives_etxtbsy` を参照。 +static EXEC_STAGING_LOCK: Mutex<()> = Mutex::new(()); + +/// poisoning を無視して `EXEC_STAGING_LOCK` を取る。 +/// +/// このロックが守るのは **fd 継承のタイミング窓**であって共有データの不変条件ではない。 +/// 先に panic したテストがあっても後続を道連れにしないよう、中身をそのまま取り出す +/// (poisoning で二次的な失敗を増やすと、本当の failure が読みにくくなる)。 +fn exec_staging_guard() -> MutexGuard<'static, ()> { + EXEC_STAGING_LOCK + .lock() + .unwrap_or_else(PoisonError::into_inner) +} + /// exe と deploy 済 `hooks-config.toml` を temp dir へ配置し、staging 先の exe パスを返す。 /// 返り値の `TempDir` は生存させ続けること (drop で削除される)。 fn stage_hook() -> (tempfile::TempDir, PathBuf) { + let _guard = exec_staging_guard(); let tmp = tempfile::tempdir().expect("create temp dir"); let exe_name = built_exe() @@ -127,7 +159,11 @@ fn stage_hook() -> (tempfile::TempDir, PathBuf) { /// /// telemetry の kill-switch を立てるのは、staging した config が `[telemetry]` を /// enable していても書き込みを起こさないため (テストの副作用を config に依存させない)。 +/// +/// spawn だけを `EXEC_STAGING_LOCK` で囲む (→ ロックの doc)。子の待ち受けまで +/// ロックを持つと、並列テストが hook の実行時間ぶん直列化されて意味なく遅くなる。 fn run_hook(exe: &Path, payload: &str) -> (i32, String) { + let guard = exec_staging_guard(); let mut child = Command::new(exe) .env("CLAUDE_TELEMETRY_DISABLE", "1") .stdin(Stdio::piped()) @@ -135,6 +171,7 @@ fn run_hook(exe: &Path, payload: &str) -> (i32, String) { .stderr(Stdio::piped()) .spawn() .expect("spawn hooks-pre-tool-validate"); + drop(guard); let stdout_drain = drain_pipe_unlimited(child.stdout.take().expect("child stdout")); let stderr_drain = drain_pipe_unlimited(child.stderr.take().expect("child stderr")); child @@ -208,3 +245,62 @@ fn malformed_stdin_does_not_block() { "不正な stdin が block (exit 2) になった — 全ツール呼び出しを止めうる" ); } + +/// 並列スレッド数。**この値で検出率が決まる** — ロックを外した状態での実測 (WSL +/// Ubuntu-24.04 / ext4、10 回試行) は 4 スレッドで 6/10、8 スレッドで 10/10 だった。 +/// ラウンド数を増やしても検出率は上がらず (4 スレッドは 12 → 24 ラウンドで 4/5 → 6/10)、 +/// 効くのは copy と spawn が重なる同時実行数のほうだと判った。 +const STRESS_THREADS: usize = 8; +/// 1 スレッドあたりの staging + spawn 回数。8 スレッドなら 16 で検出率 10/10 に達し、 +/// 24 に増やしても検出率は変わらず所要時間だけ伸びた (Windows で 9.7s → 14.5s)。 +const STRESS_ROUNDS: usize = 16; + +/// **incident 回帰**: 並列な staging と spawn が `ETXTBSY` を起こさないこと。 +/// +/// 由来は PR #376 の CI (ubuntu-latest) で `malformed_stdin_does_not_block` が +/// `Os { code: 26, kind: ExecutableFileBusy }` で落ちた flake。機構と対処は +/// `EXEC_STAGING_LOCK` の doc を参照。 +/// +/// **`#[ignore]` にする理由**: 由来する失敗が確率的で、通常テストとして置くと +/// 「落ちたら実バグ」の信号が濁る。`--ignored` (CI の直列 leg) で回すことで、 +/// ガードを外す変更が入れば検出される。所要時間は Linux 2.1s / Windows 9.7s。 +/// +/// **この seal が効くことの実測** (ADR-049 — 修正前に落ちることを確認する): +/// `EXEC_STAGING_LOCK` の取得 2 箇所を外して Linux (ext4) で +/// `cargo test -p hooks-pre-tool-validate --test smoke -- --ignored` を回すと +/// **10 回中 10 回 ETXTBSY で落ちる**。ロックを戻すと 10 回中 0 回。 +/// `/mnt/c` などの drvfs 上では再現しないので、必ず ext4 上で回すこと。 +#[test] +#[ignore = "確率的な負荷テスト。--ignored (CI の直列 leg) と手動再現でのみ回す"] +fn concurrent_staging_and_spawn_survives_etxtbsy() { + let results: Vec<_> = std::thread::scope(|scope| { + let handles: Vec<_> = (0..STRESS_THREADS) + .map(|_| { + scope.spawn(|| { + for _ in 0..STRESS_ROUNDS { + let (_keep, exe) = stage_hook(); + let payload = serde_json::json!({ + "tool_name": "Bash", + "tool_input": { "command": "ls -la" }, + }) + .to_string(); + let (code, stderr) = run_hook(&exe, &payload); + assert_eq!( + code, EXIT_PASS, + "負荷下で verdict が変わった (exit {code}, stderr: {stderr})" + ); + } + }) + }) + .collect(); + handles.into_iter().map(|h| h.join()).collect() + }); + + assert!( + results.iter().all(Result::is_ok), + "並列 staging/spawn でスレッドが panic した ({} / {} 本)。\ + ETXTBSY なら EXEC_STAGING_LOCK が効いていない", + results.iter().filter(|r| r.is_err()).count(), + STRESS_THREADS + ); +} diff --git a/src/hooks-stop-quality/tests/t7_cwd_independence.rs b/src/hooks-stop-quality/tests/t7_cwd_independence.rs index 51a6f6e0..a88462ed 100644 --- a/src/hooks-stop-quality/tests/t7_cwd_independence.rs +++ b/src/hooks-stop-quality/tests/t7_cwd_independence.rs @@ -29,12 +29,41 @@ use std::io::Write; use std::path::{Path, PathBuf}; use std::process::{Command, Stdio}; use std::sync::atomic::{AtomicU32, Ordering}; +use std::sync::{Mutex, MutexGuard, PoisonError}; /// spawn した hook exe の bounded wait (dev-conventions.md § bounded wait)。 const HOOK_TIMEOUT_SECS: u64 = 60; static UNIQUE_COUNTER: AtomicU32 = AtomicU32::new(0); +/// exe の staging (copy) と spawn (fork〜exec) を相互排除するロック。 +/// +/// このファイルの `#[test]` は **5 本すべてが copy→spawn** で、cargo 既定では並列に走る。 +/// POSIX でこの形は `ETXTBSY` を起こす — copy 側の書き込み fd を、並列 spawn が fork した +/// 子が継承し、exec 完了まで exe が「書き込み用に開かれたまま」になるため +/// (機構と実測は `hooks-pre-tool-validate/tests/smoke.rs` の `EXEC_STAGING_LOCK` の doc)。 +/// +/// **本ファイルは現状 `#![cfg(windows)]` なので POSIX のその経路は踏まない**が、同型の +/// パターンであることに変わりはなく、Windows でも「実行中/オープン中の exe への書き込み」は +/// sharing violation になりうる。`cfg(windows)` を外して ubuntu leg に載せる変更が入った +/// 瞬間に smoke.rs と同じ flake が出る箇所なので、先にガードを入れて構造を揃えておく。 +/// +/// **smoke.rs と同型のまま複製しているのは意図的** — [ADR-044](../../../docs/adr/adr-044-subprocess-utility-extraction-boundary.md) +/// 層 1 で「2 crate 重複」は extract 必須ではなく要 dogfood の区分にあたる。加えて本体は +/// `Mutex<()>` 1 個で、共有すると **test 専用の同期プリミティブを `lib-subprocess` の +/// production surface に載せる**ことになる (ロックはプロセス内でしか意味を持たず、 +/// テストバイナリはそれぞれ別プロセスなので共有しても得られる保証は増えない)。 +/// **3 つ目の copy→spawn テストが現れた時点で extract を再評価する。** +static EXEC_STAGING_LOCK: Mutex<()> = Mutex::new(()); + +/// poisoning を無視して `EXEC_STAGING_LOCK` を取る (守るのは fd 継承の窓であって +/// 共有データの不変条件ではないため、先に panic したテストで後続を道連れにしない)。 +fn exec_staging_guard() -> MutexGuard<'static, ()> { + EXEC_STAGING_LOCK + .lock() + .unwrap_or_else(PoisonError::into_inner) +} + /// incident と同じ形の「ルート相対パスを含む step cmd」。 /// /// 由来 incident の file-length step (`.\.claude\hooks-post-tool-comment-lint-rust.exe ...`) と @@ -55,6 +84,7 @@ fn exe_path() -> PathBuf { /// - `/.claude/probe.cmd` — ルート相対で呼ばれる成功 probe /// - `/.takt/runs/` — incident の cwd (存在する非ルートディレクトリ) fn stage_project(prefix: &str, steps_toml: &str) -> PathBuf { + let _guard = exec_staging_guard(); let n = UNIQUE_COUNTER.fetch_add(1, Ordering::Relaxed); let root = std::env::temp_dir().join(format!("t7_{}_{}_{}", prefix, std::process::id(), n)); let claude_dir = root.join(".claude"); @@ -72,7 +102,11 @@ fn stage_project(prefix: &str, steps_toml: &str) -> PathBuf { } /// staging 済み exe を `cwd` から起動し、stdout を返す。 +/// +/// spawn だけを `EXEC_STAGING_LOCK` で囲む (子の待ち受けまで持つと、並列テストが +/// hook の実行時間ぶん直列化されて意味なく遅くなる)。 fn run_hook(root: &Path, cwd: &Path) -> String { + let guard = exec_staging_guard(); let mut child = Command::new(root.join(".claude").join("hooks-stop-quality.exe")) .current_dir(cwd) .stdin(Stdio::piped()) @@ -80,6 +114,7 @@ fn run_hook(root: &Path, cwd: &Path) -> String { .stderr(Stdio::piped()) .spawn() .expect("spawn hooks-stop-quality"); + drop(guard); let stdout = drain_pipe_unlimited(Box::new(child.stdout.take().expect("stdout piped"))); let stderr = drain_pipe_unlimited(Box::new(child.stderr.take().expect("stderr piped")));