diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml new file mode 100644 index 00000000..da29f45d --- /dev/null +++ b/.github/workflows/ci.yml @@ -0,0 +1,180 @@ +# ci — Windows / Linux の 2 OS matrix による移植退行防止ゲート +# +# 役割: PR と master push で Rust 成果物 (clippy / cargo test / hooks smoke test) を +# **windows-latest と ubuntu-latest の両方**で実行し、片方の OS でしか通らないコードが +# land するのを止める。設計根拠は docs/adr/adr-065-ci-matrix-cross-os-regression.md。 +# +# 設計メモ: +# - **両 OS を回す理由は実例に基づく**: Linux 実測で cli-pr-monitor の lock が同時取得を +# 許すレースが見つかった (Windows ではスケジューリング差で顕在化しなかっただけで欠陥は +# 同じ)。また `jj -r "..@"` のようなシェル経由の引数は cmd.exe ではクォートが +# 除去されず Windows だけ壊れる。どちらも「片 OS だけで回していると気付けない」欠陥で、 +# 検出には両 OS で同じスイートを流すしかない (ADR-063 § 副次発見)。 +# - **fail-fast: false**: 片方が落ちても他方を最後まで走らせる。本 workflow の目的は +# 「どちらの OS で壊れたか」の切り分けなので、巻き添えキャンセルは情報を失う。 +# - **jj を固定バージョンで導入する**: `--ignored` 統合テストは実 jj を spawn する +# (ADR-011 / ADR-015 / ADR-045 が 0.42 系の挙動に依存)。導入しないとこれらが CI を +# すり抜け、jj 呼び出し経路の OS 差 (上記のクォート事故) が land 前に検出できない。 +# バージョン不一致は fail-closed で落とす — 「入っているが別バージョン」は +# 「テストが通ったのに本番挙動が違う」を生む最悪の形なので黙って進めない。 +# - **`--ignored` を直列 (`--test-threads=1`) で回す**: これらは cwd を書き換えるため +# 並列実行では相互干渉する (ADR-041)。ローカル push-runner の rust-test group と +# 同一コマンドにして、CI とローカルで結果が一致するようにしている。 +# - **hooks smoke test を独立 step にする**: `cargo test --workspace` に含まれるが、 +# 「hook が fixture stdin に対して期待どおり block/pass するか」は他のユニットテストとは +# 性質の違う契約なので、run の一覧で独立に赤/緑が読めるようにする。コンパイル済みの +# ため再実行コストはほぼゼロ。 +# - **release-binaries.yml の clippy/test と重複するのは意図的**: あちらは「壊れた +# バイナリを rolling release に載せない」ための自己完結したゲートで、workflow_dispatch や +# PR を経ない master push でも単独で成立する必要がある。本 workflow に依存させると +# その保証が消える (ADR-065 § 決定 7)。 +# - checkout は persist-credentials: false (pr-monitor.yml / release-binaries.yml と同じ +# token 漏洩対策)。本 workflow は読み取りのみなので permissions も contents: read に絞る。 +# - **`paths:` フィルタを付けない (release-binaries.yml との非対称は意図的)**: あちらは +# publish job で required check にはならないため、docs-only push を除外して無駄な再ビルドを +# 省くのが正しい。本 workflow は required check にする予定 (§ 決定 5) で、**paths で skip した +# check を required にすると GitHub はそれを success ではなく pending として扱い、PR が +# 永久にマージ不能になる**。「回さない」と「緑」を区別できないのが GitHub の仕様なので、 +# required にする側では paths を使わない。将来 run 量を削るなら、job を必ず起動したうえで +# 中身を条件分岐して success を返す形にすること (skip ではなく early-success)。 +# public リポジトリの Actions は無料・無制限のため、現状の余剰は CPU 時間のみ。 +# - **required check 化はまだしない**: 数 run 分の安定性 (実行時間・flake の有無) を観測して +# から Branch Protection の Required status checks に登録する (段階の根拠は ADR-065 § 決定 5)。 + +name: ci + +on: + pull_request: + push: + branches: [master] + workflow_dispatch: + +# PR に追い push した場合、古い run は結果が不要なのでキャンセルしてよい +# (release-binaries.yml と違い、途中終了しても壊れる成果物が無い)。 +# 一方 **master push はキャンセルしない**: master への push は `github.ref` が常に +# `refs/heads/master` で同一 group に落ちるため、`true` 固定だと連続 merge で前の run が +# 報告前に消え、「一度も検証されていない master commit」が生まれる。required check 化 +# (ADR-065 § 決定 5) 後はそれが実害になるので、キャンセルは PR イベントに限定する。 +concurrency: + group: ci-${{ github.ref }} + cancel-in-progress: ${{ github.event_name == 'pull_request' }} + +permissions: + contents: read + +env: + # ADR-011 / ADR-015 / ADR-045 が 0.42 系の挙動に依存するため、ローカル検証環境および + # scripts/cloud-setup.sh と同じバージョンに固定する (ADR-017 と同型の版固定)。 + # 3 箇所の論理結合なので、上げるときは必ず揃えて上げること (ADR-051)。 + JJ_VERSION: "0.42.0" + +jobs: + rust: + name: rust (${{ matrix.os }}) + runs-on: ${{ matrix.os }} + timeout-minutes: 60 + + strategy: + fail-fast: false + matrix: + os: [ubuntu-latest, windows-latest] + + steps: + - name: Check out the repository + uses: actions/checkout@v4 + with: + persist-credentials: false + + - name: Cache cargo registry and build artifacts + uses: actions/cache@v4 + with: + path: | + ~/.cargo/registry + ~/.cargo/git + target + key: ${{ runner.os }}-cargo-ci-${{ hashFiles('Cargo.lock') }} + restore-keys: | + ${{ runner.os }}-cargo-ci- + + - name: Show toolchain versions + shell: bash + run: | + rustc --version + cargo --version + cargo clippy --version + + - name: Install jj (Linux) + if: runner.os == 'Linux' + shell: bash + run: | + set -euo pipefail + archive="jj-v${JJ_VERSION}-x86_64-unknown-linux-musl.tar.gz" + url="https://github.com/jj-vcs/jj/releases/download/v${JJ_VERSION}/${archive}" + tmp="$(mktemp -d)" + curl --fail --location --silent --show-error --output "${tmp}/${archive}" "${url}" + tar -xzf "${tmp}/${archive}" -C "${tmp}" + # アーカイブのレイアウト (flat / サブディレクトリ入り) に依存しないよう探索して拾う + # (cloud-setup.sh の install_jj と同じ方針)。 + bin="$(find "${tmp}" -type f -name jj -print -quit)" + if [ -z "${bin}" ]; then + echo "error: jj binary not found in ${archive}" >&2 + exit 1 + fi + mkdir -p "${HOME}/.local/bin" + install -m 0755 "${bin}" "${HOME}/.local/bin/jj" + echo "${HOME}/.local/bin" >> "${GITHUB_PATH}" + + - name: Install jj (Windows) + if: runner.os == 'Windows' + shell: pwsh + run: | + $ErrorActionPreference = 'Stop' + $archive = "jj-v${env:JJ_VERSION}-x86_64-pc-windows-msvc.zip" + $url = "https://github.com/jj-vcs/jj/releases/download/v${env:JJ_VERSION}/${archive}" + $work = Join-Path $env:RUNNER_TEMP 'jj-download' + $dest = Join-Path $env:RUNNER_TEMP 'jj-bin' + New-Item -ItemType Directory -Force -Path $work, $dest | Out-Null + Invoke-WebRequest -Uri $url -OutFile (Join-Path $work $archive) + Expand-Archive -Path (Join-Path $work $archive) -DestinationPath $work -Force + $bin = Get-ChildItem -Path $work -Filter 'jj.exe' -Recurse | Select-Object -First 1 + if (-not $bin) { throw "jj.exe not found in $archive" } + Copy-Item $bin.FullName (Join-Path $dest 'jj.exe') -Force + Add-Content -Path $env:GITHUB_PATH -Value $dest + + # 「入っているが別バージョン」を通すと、テストが緑でも本番の jj 挙動と一致しなくなる。 + # identity は統合テストが commit を作るため必須 (未設定だと author が空になり + # 挙動が環境依存になる)。 + - name: Verify the pinned jj version and configure identity + shell: bash + run: | + set -euo pipefail + jj --version + if ! jj --version | grep -q "${JJ_VERSION}"; then + echo "error: jj version mismatch (expected ${JJ_VERSION})" >&2 + exit 1 + fi + jj config set --user user.name "ci" + jj config set --user user.email "ci@example.invalid" + + # ローカル push-runner (rust-lint-test group) と同一コマンド。`--all-targets` により + # test コードも対象になるため、片 OS でしかコンパイルされない `#[cfg(...)]` 配下の + # 未使用 import 等もここで露出する。 + - name: Run clippy + shell: bash + run: cargo clippy --workspace --all-targets --all-features -- -D warnings + + - name: Run tests + shell: bash + run: cargo test --workspace + + # fixture stdin -> 期待する block/pass 判定 (ADR-049 の incident fixture 資産を流用)。 + - name: Run hooks smoke tests + shell: bash + run: | + set -euo pipefail + cargo test -p hooks-pre-tool-validate --test smoke + cargo test -p hooks-post-tool-linter --test incident_eval + + - name: Run ignored (integration) tests + shell: bash + run: cargo test --workspace -- --ignored --test-threads=1 diff --git a/.github/workflows/release-binaries.yml b/.github/workflows/release-binaries.yml index 2a8f1fe6..41ec3fc0 100644 --- a/.github/workflows/release-binaries.yml +++ b/.github/workflows/release-binaries.yml @@ -22,7 +22,8 @@ # workspace の bin target を機械的に集めることで、この drift を構造的に断つ。 # - **公開前に cargo test を通す**: 壊れたバイナリを rolling release に載せると、 # クラウドセッション側が原因不明の挙動不良を起こす (しかも setup は成功する) ため。 -# Windows leg と hooks smoke test を含む本格的な CI matrix は WP-16 で扱う。 +# ci.yml (両 OS matrix、ADR-065) と重複するが、本 workflow は PR を経ない master push や +# workflow_dispatch でも単独で成立する必要があるため、このゲートは残す。 # - **paths フィルタで docs-only push を除外する**: 本リポジトリは docs / todo 更新の # push が多く、そのたびに全 crate を再ビルドするのは無駄。バイナリの内容に影響する # パスに限定する。 diff --git a/CLAUDE.md b/CLAUDE.md index cc0e4443..de158e53 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -65,6 +65,7 @@ - [ADR-062: 月次ハーネス ROI レビュー — telemetry 発火実績によるハーネス複雑度の棚卸し (WP-12 step 2/3)](docs/adr/adr-062-monthly-harness-roi-review.md) *(試験運用)* - [ADR-063: Linux 可搬性レイヤ + nightly release + cloud-setup — クラウド向けプリビルドバイナリ配布](docs/adr/adr-063-linux-portability-release-binaries.md) - [ADR-064: PR 監視 success 判定の陽性証拠要求 — レート制限 silent success の排除](docs/adr/adr-064-monitor-success-positive-evidence.md) +- [ADR-065: CI matrix による移植退行防止 — 両 OS で同一スイートを回す](docs/adr/adr-065-ci-matrix-cross-os-regression.md) *(試験運用)* ## 開発 convention / チェックリスト diff --git a/Cargo.lock b/Cargo.lock index 544e68cf..68119a24 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -442,6 +442,7 @@ dependencies = [ "regex", "serde", "serde_json", + "tempfile", "toml", ] diff --git a/docs/adr/adr-049-incident-eval-regression-suite.md b/docs/adr/adr-049-incident-eval-regression-suite.md index e96b32b1..be4c403e 100644 --- a/docs/adr/adr-049-incident-eval-regression-suite.md +++ b/docs/adr/adr-049-incident-eval-regression-suite.md @@ -91,7 +91,7 @@ prompt/test 資産の追加であり、[ADR-039](adr-039-experimental-feature-st - ルールの検出力退行と false positive 退行を cargo test で機械検出 (ハーネス自身の回帰スイート)。 - 各ルールが由来 incident と再現 fixture を機械可読に持ち、削除可否判断が追跡可能。 - 実 exe E2E で hook の全経路 (stdin/config/feedback/exit) を保証。 -- 本 repo 初の exe-spawn integration test パターンを確立 (WP-16 CI smoke test で流用可能)。 +- 本 repo 初の exe-spawn integration test パターンを確立 ([ADR-065](adr-065-ci-matrix-cross-os-regression.md) の hooks smoke test が流用)。 ### 欠点 / 留意点 diff --git a/docs/adr/adr-063-linux-portability-release-binaries.md b/docs/adr/adr-063-linux-portability-release-binaries.md index c0011be4..19ebbdba 100644 --- a/docs/adr/adr-063-linux-portability-release-binaries.md +++ b/docs/adr/adr-063-linux-portability-release-binaries.md @@ -114,7 +114,10 @@ Linux 実測で `cli-pr-monitor` の lock が**同時取得**を許すレース ### 残課題 - `#[cfg(windows)]` ガードのテスト (pump_child_io の deadlock 保護、run_cmd_capture の - stdout/stderr 分離) は Linux 実行では skip される。CI matrix 整備 (両 OS) で扱う。 + stdout/stderr 分離) は Linux 実行では skip される。CI matrix は + [ADR-065](adr-065-ci-matrix-cross-os-regression.md) で整備し、これらは Windows leg で + CI 実行対象になった (従来の Linux only CI では一度も走っていなかった)。**Linux 上での + 同等検証 (POSIX 版テストの追加) は未了**であり、ADR-065 の残課題として引き継いでいる。 - クラウドセッションのプラットフォーム制約 (セットアップスクリプトの実行タイミング・ fresh clone 挙動・hooks の snapshot 登録) への対応は [ADR-060](adr-060-cloud-harness-sessionstart-dispatcher.md) を参照。 diff --git a/docs/adr/adr-065-ci-matrix-cross-os-regression.md b/docs/adr/adr-065-ci-matrix-cross-os-regression.md new file mode 100644 index 00000000..519ab1a1 --- /dev/null +++ b/docs/adr/adr-065-ci-matrix-cross-os-regression.md @@ -0,0 +1,206 @@ +# ADR-065: CI matrix による移植退行防止 — 両 OS で同一スイートを回す + +## ステータス + +試験運用 (2026-08-01 実装。required check 化は安定観測後) + +> 本 ADR は 2026-07-04 策定のハーネス改善計画 WP-16 の決定を永続化したものである。 +> 前提となる Linux 可搬性レイヤは [ADR-063](adr-063-linux-portability-release-binaries.md)。 + +## コンテキスト + +WP-15 完了時点の CI は `release-binaries.yml` の 1 job のみで、以下の穴があった: + +- **Linux only**: Windows 側で回るのはローカル `pnpm push` (push-runner) だけ。人が push する + ときにしか動かず、PR 単位のゲートが存在しない。 +- **master push only**: `pull_request` トリガーが無く、PR の段階では何も検証されない。 +- **`--ignored` 統合テストが対象外**: jj を実際に spawn する経路が CI をすり抜ける。 + +一方、片 OS でしか検出できない欠陥は**両方向で実在が確認されている**: + +- **Linux でしか出ない**: `cli-pr-monitor` の lock が同時取得を許すレース (8 スレッド中 6 つが + 取得)。Windows ではスケジューリング差で顕在化しなかっただけで欠陥は同じ + ([ADR-063](adr-063-linux-portability-release-binaries.md) の副次発見)。 +- **Windows でしか出ない**: シェル経由で渡す jj の revset 範囲指定 (`jj -r "..@"`) は + cmd.exe でクォートが除去されず失敗する。`sh` では通るため Linux 実行では気付けない。 + +さらに [ADR-063](adr-063-linux-portability-release-binaries.md) の残課題として、 +`#[cfg(windows)]` ガードのテスト (`pump_child_io` の deadlock 保護、`run_cmd_capture` の +stdout/stderr 分離) は Linux 実行では skip される。CI が Linux のみだと、これらは +**CI で一度も実行されない**状態だった。 + +## 決定 + +### 1. `windows-latest` + `ubuntu-latest` の 2 leg matrix を PR ゲートとして新設する + +`.github/workflows/ci.yml`。トリガーは `pull_request` / `master` への push / +`workflow_dispatch`。`fail-fast: false` とし、片方が落ちても他方を完走させる — +本 workflow の目的は「どちらの OS で壊れたか」の切り分けなので、巻き添えキャンセルは +必要な情報を失う。権限は `contents: read`、checkout は `persist-credentials: false` +(`pr-monitor.yml` / `release-binaries.yml` と同じ token 漏洩対策)。 + +### 2. 各 leg はローカル push-runner と同一のコマンドを回す + +`cargo clippy --workspace --all-targets --all-features -- -D warnings` / +`cargo test --workspace` / `cargo test --workspace -- --ignored --test-threads=1`。 + +CI とローカルでコマンドが違うと、どちらかの緑が嘘になる。`--all-targets` により +test コードも clippy 対象になるため、片 OS でしかコンパイルされない `#[cfg(...)]` 配下の +未使用 import 等もここで露出する。`--ignored` を直列 (`--test-threads=1`) で回すのは、 +これらが cwd を書き換えて相互干渉するため ([ADR-041](adr-041-test-isolation-patterns.md))。 + +### 3. `--ignored` のために jj を固定バージョンで導入し、版一致を fail-closed で検証する + +統合テストは実 jj を spawn する。導入しなければこれらは CI をすり抜け、上記の +クォート事故のような jj 呼び出し経路の OS 差が land 前に検出できない。 + +版は 0.42.0 固定 ([ADR-011](adr-011-jj-push-new-bookmark-strategy.md) / +[ADR-015](adr-015-push-runner-takt-migration.md) / +[ADR-045](adr-045-jj-workspace-parallel-sessions.md) が 0.42 系の挙動に依存)。 +**「入っているが別バージョン」は黙って進めない** — テストが緑でも本番の jj 挙動と一致 +しなくなる最悪の形なので、`jj --version` の照合に失敗したら step を落とす +([ADR-043](adr-043-security-gates-fail-closed.md))。統合テストは commit を作るため +identity (`user.name` / `user.email`) も CI で明示設定する (未設定だと author が空になり +挙動が環境依存になる)。 + +なお jj のバージョンは **ローカル検証環境 / `scripts/cloud-setup.sh` / 本 workflow の +3 箇所に論理結合**している。上げるときは必ず 3 箇所を揃えること +([ADR-051](adr-051-cross-system-config-coupling.md))。 + +### 4. hooks smoke test は「既存 E2E の両 OS 実行」+「block 経路の新規テスト」で構成する + +- 既存の exe-spawn E2E ([ADR-049](adr-049-incident-eval-regression-suite.md) の + `incident_eval`、`hooks-stop-quality`、`hooks-stop-tool-call-leak`) は `cargo test` に + 乗っているため、matrix 化によって**そのまま両 OS 実行になる**。ADR-049 が確立した + exe-spawn パターンをここで流用するという WP-16 の意図は、この形で満たされる。 +- 一方、リポジトリで唯一「Claude の操作を実際に止める」hook である + `hooks-pre-tool-validate` には exe-spawn テストが無く、stdin JSON parse → config → + preset/protected → stderr + exit code の経路が無検証だった。 + `src/hooks-pre-tool-validate/tests/smoke.rs` を新設し、**block (exit 2) と pass (exit 0) を + 対で**固定する。assert は exit code と stderr の有無のみに限定し、block メッセージ本文は + 固定しない (ADR-049 と同じ方針。文言修正でテストが壊れない)。 +- 不正な stdin が block 側に倒れないことも固定する。PreToolUse は全ツール呼び出しの前段に + 居るため、ここが exit 2 に倒れると Claude の操作が全面的に止まる。 +- **exe は temp dir へ staging してから spawn する** (`t7_cwd_independence` と同方式)。 + この hook は config を `current_exe()` の隣から解決するため、`target/debug` の exe を + 直接叩くと**そこに残っている config に verdict が左右される** — 実際、開発機の + `target/debug/` には過去の作業で置かれた `hooks-config.toml` が残っており、fresh clone の + CI とは違う config で判定していた。staging により deploy 済 config での判定を固定し、 + 同時に `target/debug` を汚して他 crate の exe-spawn テストへ干渉することも防ぐ。 +- `cargo test --workspace` に含まれるが、run の一覧で独立に赤/緑が読めるよう CI では + 独立 step としても実行する (コンパイル済みのため追加コストはほぼゼロ)。 + +### 5. required check 化は段階を分ける + +数 run 分の安定性 (実行時間・flake の有無) を観測してから Branch Protection の +Required status checks に登録する。未検証の新規 workflow をいきなり必須にすると、 +プロダクトではなく CI 側の不備で全 PR が止まる。 +[ADR-043](adr-043-security-gates-fail-closed.md) の fail-closed はゲート**関数**の +振る舞いに関する原則であり、ゲート自体の導入手順を一足飛びにする根拠ではない。 + +### 6. `paths:` フィルタは付けない (`release-binaries.yml` との非対称は意図的) + +`release-binaries.yml` は docs-only push を `paths:` で除外している。本 workflow で同じ +フィルタを付けないのは、**`paths:` で skip された check を Required status checks に指定すると、 +GitHub はそれを success ではなく pending として扱い、PR が永久にマージ不能になる**ため。 +「回さない」と「緑」を GitHub が区別できない以上、required にする側 (本 workflow、§ 決定 5) と +publish 用で required にならない側 (`release-binaries.yml`) では正しい選択が逆になる。 + +将来 run 量を削る必要が出た場合も `paths:` は使わず、**job は必ず起動したうえで中身を +条件分岐して success を返す** (skip ではなく early-success) 形にすること。public リポジトリの +Actions は無料・無制限なので、現状の余剰コストは CPU 時間のみである。 + +### 7. `release-binaries.yml` の clippy / cargo test は残す (重複は意図的) + +あちらは「壊れたバイナリを rolling release に載せない」ための自己完結したゲートで、 +`workflow_dispatch` や PR を経ない master push でも単独で成立する必要がある +([ADR-063](adr-063-linux-portability-release-binaries.md) の設計)。本 workflow に依存 +させるとその保証が消える。public リポジトリの Actions は無料・無制限のため、 +重複実行のコストは受け入れる。 + +## 検証記録 (実測) + +- **jj 取得 step を両 OS で実走**: 実 URL・実 asset を使い、Linux (WSL Ubuntu 24.04、 + musl tarball → `find` → `install`) / Windows (PowerShell、msvc zip → `Expand-Archive` → + `jj.exe` 探索) の双方で `jj 0.42.0-b8f7c455...` を取得・実行できることを確認。 + 版照合 gate が通る形式であることも併せて確認した。 +- **ローカル Windows**: `cargo test --workspace` (1,881 tests) / `cargo test --workspace -- + --ignored --test-threads=1` (20 tests) / 新規 `smoke` (2 tests) / `incident_eval` (2 tests) + が pass。clippy `-D warnings` clean。 +- **fresh checkout の擬似再現**: 開発機の `target/debug/hooks-config.toml` (過去の作業で + 残っていた成果物。CI の fresh clone には存在しない) を退避した状態で通常 / `--ignored` の + 両スイートを再実行し、いずれも pass することを確認した。ローカルだけで通る隠れた + 環境依存が無いことの実測的な裏づけ。 +- **ubuntu leg の先取り実行 (WSL Ubuntu 24.04)**: CI と同一コマンドで + clippy `-D warnings` clean / `cargo test --workspace` **1,707 pass, 0 failed**。 + Windows の 1,881 との差 174 は `#[cfg(windows)]` 群であり、両 OS の実行本数差が + 想定どおりであることも併せて確認した (差が説明できない = どちらかの leg が主題を + 検証していない、の検知)。 +- **未観測**: GitHub Actions 上での実行は本 workflow を含む PR の run が初回であり、 + run 時間・cache 効率・flake の有無は未観測。§ 決定 5 の段階分けはこの事実に基づく。 + +## 帰結 + +### 利点 + +- PR の段階で両 OS の退行が止まる。従来 CI では一度も実行されていなかった + `#[cfg(windows)]` テスト群 (`pump_child_io` の deadlock 保護等) が Windows leg で + CI 実行対象になる。 +- jj を CI に入れたことで、jj 呼び出し経路 (シェル引数の扱い・revset 指定) の OS 差が + land 前に露出する。 +- ローカル push-runner と同一コマンドのため、「ローカルは緑・CI は赤」の原因が + コマンド差ではなく環境差に絞られる。 + +### 欠点 / 留意点 + +- `--ignored` を直列実行するため 1 leg あたりの所要時間が増える。cold cache の + Windows leg が最長になる見込み。 +- rust toolchain を pin していない (runner のプリインストール stable を使う)。 + runner 更新で新しい clippy lint が入ると、コード無変更で赤になり得る。 + `release-binaries.yml` が既に受け入れている性質と同じ。 +- **jj アーカイブの取得に checksum 検証を付けていない** (受容したリスク)。jj の Release は + チェックサム asset を公開しておらず、検証するにはハッシュを workflow へ直書きして + バージョン固定の 3 箇所結合 (§ 決定 3) にもう 1 つ更新箇所を足すことになる。 + `scripts/cloud-setup.sh::install_jj` も同じ前提で、本 workflow で新たに開く穴ではない。 + `permissions: contents: read` かつ secrets を持たない job のため、影響範囲は使い捨ての + 非特権 runner に閉じる。checksum asset が公開されたら追随する。 + +### 副次発見: 理由が stale 化した `#[cfg(windows)]` ガード + +本 ADR の PR (#342) で CodeRabbit が `hooks-stop-quality` の +`run_quality_steps_parallel_collects_failures_in_step_order` の `#[cfg(windows)]` 除去を +指摘し、**指摘が正しかった**。当該テストの step は `exit 0` / `exit 1` のみで、実行経路の +`run_cmd_shell_capped` は [ADR-063](adr-063-linux-portability-release-binaries.md) で +`shell_command` (cmd /c ↔ sh -c) に抽象化済み。ガードと「`cmd /c` 依存だから Windows 限定」と +いう doc コメントは、いずれも ADR-063 以前の記述が残ったものだった。実 Linux (WSL) で +ガード除去後に pass することを実測し、除去した。 + +留意すべきは、**ローカルの post-PR レビュー層はこれを「false positive」と判定していた**点 +(理由として「`run_cmd_shell_capped` は cmd /c 依存」という、まさに stale なコメントの主張を +そのまま採用していた)。コメントが実装から乖離すると、レビュー層はその乖離を増幅する。 +`#[cfg(...)]` ガードには**理由を書くだけでなく、その理由が今も成立するかを疑う**必要がある。 +同型の点検として `t7_cwd_independence` のガード理由も実態 (再現対象の incident が +`.\.claude\probe.cmd` という cmd.exe 固有のルート相対パス解決そのもの) に書き直した +— こちらはガード自体が正当である。 + +### 残課題 + +- [ADR-063](adr-063-linux-portability-release-binaries.md) の残課題のうち + **「Linux 上で `pump_child_io` の deadlock 保護と `run_cmd_capture` の stdout/stderr 分離が + 無検証」は本 ADR では閉じない**。matrix はこれらを Windows leg で CI 実行対象にするが、 + 該当テストは cmd.exe / PowerShell を子プロセスに使うため module ごと Windows 限定であり、 + Linux 側の同等検証には POSIX 版テストの追加が必要になる。 +- required check 化 (§ 決定 5) と、観測結果に基づく cache 戦略の調整。 + +## 関連 + +- [ADR-063](adr-063-linux-portability-release-binaries.md) — Linux 可搬性レイヤ / nightly + release。本 ADR が閉じる穴の由来と、両 OS 実行の価値を裏づけた実例 +- [ADR-049](adr-049-incident-eval-regression-suite.md) — exe-spawn E2E パターン + (hooks smoke test の設計元) +- [ADR-041](adr-041-test-isolation-patterns.md) — `--ignored` を直列実行する理由 +- [ADR-043](adr-043-security-gates-fail-closed.md) — jj 版照合を fail-closed にする根拠と、 + その原則を導入手順へ拡大解釈しない線引き +- [ADR-051](adr-051-cross-system-config-coupling.md) — jj バージョンの 3 箇所結合 +- `.github/workflows/ci.yml` — workflow 本体 +- `src/hooks-pre-tool-validate/tests/smoke.rs` — hooks smoke test 本体 diff --git a/docs/dev-conventions.md b/docs/dev-conventions.md index 14cbc35c..8f99682d 100644 --- a/docs/dev-conventions.md +++ b/docs/dev-conventions.md @@ -54,7 +54,7 @@ integration test で外部バイナリを spawn する場合、**無期限 wait 1. **timeout 付き wait** — `child.wait_with_output()` / `child.wait()` は子プロセスが hang すると CI を無期限ブロックする。代わりに `lib-subprocess::wait_with_timeout_safe(label, &mut child, 30)` 等の timeout 付き wait を使い、超過時は kill + test 失敗させる。 2. **出力捕捉との両立** — 出力が必要なら stdout/stderr を `lib-subprocess::drain_pipe_unlimited` で別スレッド drain してから timeout wait する (pipe バッファ充填による deadlock 回避)。 -**由来** (PR #254 / WP-08、[ADR-049](adr/adr-049-incident-eval-regression-suite.md)): codebase 初の exe-spawn E2E テスト (`incident_eval.rs`) パターンを確立したが timeout 境界が欠落し CodeRabbit nitpick。WP-16 CI smoke test 等で同パターン流用が見込まれるため convention 化する。 +**由来** (PR #254 / WP-08、[ADR-049](adr/adr-049-incident-eval-regression-suite.md)): codebase 初の exe-spawn E2E テスト (`incident_eval.rs`) パターンを確立したが timeout 境界が欠落し CodeRabbit nitpick。同パターンの流用が見込まれるため convention 化した(実際に [ADR-065](adr/adr-065-ci-matrix-cross-os-regression.md) の hooks smoke test が本 convention に従っている)。 ## 外部 fixture 参照テストは値まで assert (順位274) diff --git a/docs/harness-improvement-plan.md b/docs/harness-improvement-plan.md index 4dd9c487..3e971ba5 100644 --- a/docs/harness-improvement-plan.md +++ b/docs/harness-improvement-plan.md @@ -19,7 +19,7 @@ Anthropic 公式のハーネスエンジニアリング指針(決定論的基盤・コンテキスト効率・フィードバックループ速度)に対する本プロジェクトの評価結果: - 決定論的ゲート(hooks)・ルール vs 仕組み化(ADR-042)・フィードバックループ(ADR-030 / 031)・決定論的オーケストレーション(takt)は **高適合**。 -- ギャップは (1) **実行環境の可搬性**(Windows 依存。WP-13〜15 で解消済み)、(2) **自律実行の常時性**(監視がローカルセッション寿命に依存。実際に PR #237 の wakeup 失効を観測済み)、(3) **外部入力の信頼境界**(CodeRabbit コメントが編集権限を持つ fix エージェントに直結。WP-11 で 3 層防御を実装済み)。 +- ギャップは (1) **実行環境の可搬性**(Windows 依存。WP-13〜16 で解消済み)、(2) **自律実行の常時性**(監視がローカルセッション寿命に依存。実際に PR #237 の wakeup 失効を観測済み)、(3) **外部入力の信頼境界**(CodeRabbit コメントが編集権限を持つ fix エージェントに直結。WP-11 で 3 層防御を実装済み)。 - 残る主戦場は (2) の常時性 = セクション 4(WP-17〜19)。 ## 2. 検証済みの前提事実(再調査不要、2026-07-04 確認) @@ -75,7 +75,7 @@ Anthropic 公式のハーネスエンジニアリング指針(決定論的基 | WP-13 | 3 | EXE_SUFFIX 抽象化 | M | なし | 完了([ADR-005](adr/adr-005-hooks-path-resolution-with-template.md) amendment。launcher 経路の実走確認済) | | WP-14 | 3 | PowerShell 3 本の Rust 化 | S-M ×2 | なし | 完了(新規 ADR 不要判断 = 決定は各 crate doc + commit message に記録。実走確認済) | | WP-15 | 3 | Linux バイナリビルド + クラウド setup script | M | WP-13, 14 | 完了([ADR-063](adr/adr-063-linux-portability-release-binaries.md)。クラウド実測は [ADR-060](adr/adr-060-cloud-harness-sessionstart-dispatcher.md) dogfood で達成、以降は ADR-060 の bounded lifetime で管理。追補の陽性証拠設計は [ADR-064](adr/adr-064-monitor-success-positive-evidence.md) → park 実観測は § 残作業) | -| WP-16 | 3 | CI matrix(移植退行防止) | S | WP-13, 14 | 未着手 | +| WP-16 | 3 | CI matrix(移植退行防止) | S | WP-13, 14 | 観測中([ADR-065](adr/adr-065-ci-matrix-cross-os-regression.md)。windows-latest + ubuntu-latest の 2 leg を新設。required check 化と GitHub Actions 実走の安定性確認は → § 残作業) | | WP-17 | 4 | イベント駆動バックボーン完成(Phase B + routines 移行) | M | WP-09, 10, 11 | 未着手 | | WP-18 | 4 | 夜間 todo 消化ループ | M-L | WP-15, 17 | 未着手 | | WP-19 | 4 | 常時性ガード(kill-switch / 自主減速 / 監査ループ) | M | WP-18 | 未着手 | @@ -92,14 +92,15 @@ Anthropic 公式のハーネスエンジニアリング指針(決定論的基 - 現状: PR 監視の陽性証拠 gate は実装・incident 実データでの単体実測済み(ADR-064)。 - 残作業: 実 push/PR サイクルで CodeRabbit レート制限が自然発生した際に (a) 監視が success で終わらず park すること、(b) レポート判定文が保留を出すこと、を実観測したら完了(ADR-064 ステータス欄の検証残。この経路は自然発生時にしか実測できない)。 -## 6. 未着手 WP +### WP-16 残: CI matrix の実走観測と required check 化 -### WP-16: CI matrix(移植退行防止) +- 現状: `.github/workflows/ci.yml` に windows-latest + ubuntu-latest の 2 leg を新設し、各 leg で clippy / `cargo test` / hooks smoke test / `--ignored` 統合テスト(jj 0.42.0 を導入)を実行する([ADR-065](adr/adr-065-ci-matrix-cross-os-regression.md))。jj 取得 step は両 OS で実 URL・実 asset を用いて実走確認済、テスト自体もローカル Windows で全 pass。 +- 残作業: + 1. **GitHub Actions 上での実走は本 WP の PR が初回**。run 時間・cache 効率・flake の有無を数 run 観測する。 + 2. 安定を確認したら Branch Protection の Required status checks に登録(todo 順位 6 の Branch Protection 整備と連動。ADR-065 § 決定 5 が段階を分ける根拠)。 + 3. ADR-063 の残課題のうち「Linux 上で pump_child_io の deadlock 保護 / run_cmd_capture の stdout/stderr 分離が無検証」は matrix では閉じない(該当テストは cmd.exe / PowerShell 依存で module ごと Windows 限定)。POSIX 版テストの追加は ADR-065 の残課題として追跡。 -- **背景**: WP-15 の Linux 実測で「Windows だけで回していると気付けない設計欠陥」(lock 同時取得レース)が実在した(ADR-063)。また `#[cfg(windows)]` ガードのテスト(pump_child_io の deadlock 保護、run_cmd_capture の stdout/stderr 分離)は Linux 実行では skip される既知ギャップがある。 -- **ステップ**: `windows-latest` + `ubuntu-latest` で cargo test + hooks smoke test(fixture stdin → 期待する block/pass 判定を assert。ADR-049 の incident fixture 資産を流用)。安定後に required check 化(todo 順位 6 の Branch Protection 整備と連動)。 - -## 7. セクション 4: ループエンジニアリングへの道筋 +## 6. セクション 4: ループエンジニアリングへの道筋 ### WP-17: イベント駆動バックボーン完成 @@ -126,7 +127,7 @@ Anthropic 公式のハーネスエンジニアリング指針(決定論的基 2. **自主減速**: routine プロンプト冒頭に自己抑制判定 —「未マージの draft PR が 3 件以上ある/直近 run の失敗が続いている場合は何もせず終了」。作りかけの山を積まないための背圧制御。 3. **監査ループを閉じる**: 自律アクション一覧(routine run 履歴 + `claude/` ブランチ PR)を weekly-review の入力に追加し、「自律動作の週次棚卸し」を人間のレビューポイントとして固定する。 -## 8. 完了条件と退役手順 +## 7. 完了条件と退役手順 本ファイルは以下を全て満たした時点で削除する: diff --git a/docs/todo-summary2.md b/docs/todo-summary2.md index 0a26a2a9..ccad82a7 100644 --- a/docs/todo-summary2.md +++ b/docs/todo-summary2.md @@ -93,7 +93,7 @@ | 335 | 🔧 Tier 2 | **post-merge-feedback の transcript 分析を cli-merge-pipeline 生成 summary index に置換 (#303 post-merge feedback 採用)** | todo14.md | M | なし (session-analysis facet が約 1.5MB transcript で 25K token limit 衝突・避難措置を自己観測。既存 filter の自然な拡張、Frequency High) | | 336 | 🚀 Tier 1 | **post-merge-feedback の分析ソース選定を対象 PR の commit/bookmark 照合ベースに修正 — 時刻範囲のみ選定を廃止 (#311/#312 post-merge feedback 採用)** | todo14.md | M | なし (時刻範囲のみの pre-push run / transcript 選定が並行 push (#311/#312/#313) で他 PR 知見を誤帰属、#311/#312 feedback で実地確認。post-merge-feedback 分析範囲欠陥として過去 3 回 recurrence した先行 todo の同型・より深刻版。#311 feedback=✅ / #312 feedback=🤔 と判定割れだが両者実害確認済、ADR-042 で mechanizable=Yes) | | 337 | 🔧 Tier 2 | **並行テストで thread::spawn 結果を Vec::collect 後に判定する pattern を custom lint 強制 (#312 post-merge feedback 採用)** | todo14.md | M | なし (#312 で遅延イテレータが実行中 thread を drop し「2 Acquired」偽陽性、collect で回避した実績。thread::spawn は 8 ファイルで使用され再発余地。対象を concurrent test 近傍限定で FP 軽減。analyzer Tier1 = mechanical enforcement → memory `feedback_tier_classification` per project Tier 2 に再分類) | -| 338 | 🔧 Tier 2 | **CodeRabbit rate-limit format の fixture ライブラリ化 + 新世代検出の定期 CI 検証 (#311 post-merge feedback 採用)** | todo14.md | M | なし (CR は 2026-01→05→07 で 3 回 format 変更。ADR-049 fixture 化を CI 定期検証まで拡張し silent drift を land 前に捕捉。WP-16 CI matrix と連動。順位 343 (ADR-034 SOP) と相補) | +| 338 | 🔧 Tier 2 | **CodeRabbit rate-limit format の fixture ライブラリ化 + 新世代検出の定期 CI 検証 (#311 post-merge feedback 採用)** | todo14.md | M | なし (CR は 2026-01→05→07 で 3 回 format 変更。ADR-049 fixture 化を CI 定期検証まで拡張し silent drift を land 前に捕捉。ADR-065 の CI matrix を土台にできる。順位 343 (ADR-034 SOP) と相補) | | 339 | 🔧 Tier 2 | **3 世代 CR format × 4 parse path × CR state の複合マトリックステスト (#311 post-merge feedback 採用)** | todo14.md | S | なし (verdict_*_takes_precedence は #311 の 7 テストで解消済だが format 世代軸 (old/new/next/fallback) の網羅は未実施。新世代追加時の回帰防止、Frequency Medium) | | 340 | 🔧 Tier 2 | **decide.rs/main.rs の境界値・parameter threading テスト拡充 (#311 post-merge feedback 採用)** | todo14.md | S | なし (前回 incident の根本原因 = parameter threading 欠落と同クラスのリグレッション防止。positive evidence 複合 + main.rs の rate_limit 構成検証。インシデントドメイン直下で Severity Medium) | | 341 | 💎 Tier 3 | **Silent Fallback 排除原則を CLAUDE.md 開発 convention に明文化 (#311 post-merge feedback 採用)** | todo14.md | XS | なし (#311/#309 の rate-limit marker 検知 + wait 解析失敗で None 誤認 = fail-open の再発防止。自動 lint は意味論解析要で FP 過多につき却下、人間向けガイドラインで担保。Severity High) | diff --git a/docs/todo14.md b/docs/todo14.md index 39ab4d4e..c543a107 100644 --- a/docs/todo14.md +++ b/docs/todo14.md @@ -219,7 +219,7 @@ > > **参照**: `.claude/feedback-reports/311.md` Tier1 #3、`adr/adr-034-coderabbit-auto-monitoring.md`、`src/check-ci-coderabbit/src/{decide,rate_limit}.rs`。 > -> **実行優先度**: 🔧 Tier 2 (analyzer の `Tier 1` だが ci_step = automation のため project Tier 2) — Severity Medium / Frequency Medium (3 世代実績) / Effort M / Adoption Risk None。本リポジトリは cargo test 用 CI 自体が未整備のため WP-16 (CI matrix) と連動して検討。 +> **実行優先度**: 🔧 Tier 2 (analyzer の `Tier 1` だが ci_step = automation のため project Tier 2) — Severity Medium / Frequency Medium (3 世代実績) / Effort M / Adoption Risk None。CI matrix は `adr/adr-065-ci-matrix-cross-os-regression.md` で整備済 (PR / master push で両 OS の `cargo test` が回る) のため、定期検証の載せ先はこの workflow を土台にできる。 #### 作業計画 diff --git a/src/cli-push-runner/src/stages/lint_screen/classifier.rs b/src/cli-push-runner/src/stages/lint_screen/classifier.rs index 907de607..a177a1fc 100644 --- a/src/cli-push-runner/src/stages/lint_screen/classifier.rs +++ b/src/cli-push-runner/src/stages/lint_screen/classifier.rs @@ -117,7 +117,8 @@ fn abort_child( /// 全ケースが cmd.exe / PowerShell を子プロセスに使うため module ごと Windows 限定 /// にしている (WP-15: 個別 `#[cfg(windows)]` だけだと Linux で `use super::*` が /// unused となり、本リポジトリの `clippy -D warnings` ゲートで落ちる)。 -/// Linux 側で pump_child_io の deadlock 保護が無検証になる点は WP-16 (CI matrix) で扱う。 +/// ADR-065 の CI matrix により本 module は Windows leg で CI 実行対象になったが、 +/// Linux 側の同等検証 (POSIX 版テスト) は未了で ADR-065 の残課題として追跡している。 #[cfg(all(test, windows))] mod tests { use super::*; diff --git a/src/hooks-pre-tool-validate/Cargo.toml b/src/hooks-pre-tool-validate/Cargo.toml index 3f3c079c..1bf3a5fd 100644 --- a/src/hooks-pre-tool-validate/Cargo.toml +++ b/src/hooks-pre-tool-validate/Cargo.toml @@ -11,4 +11,10 @@ toml = "0.8" lib-subprocess = { path = "../lib-subprocess" } lib-telemetry = { path = "../lib-telemetry" } +[dev-dependencies] +# tempfile: hooks smoke test (tests/smoke.rs、ADR-065) が exe + config を temp dir へ +# staging するために使う。serde_json / lib-subprocess も同テストで使うが、integration +# test は [dependencies] も見えるため再宣言は不要。 +tempfile = "3" + # [profile.release] は workspace root (Cargo.toml) に集約 (ADR-026) diff --git a/src/hooks-pre-tool-validate/tests/smoke.rs b/src/hooks-pre-tool-validate/tests/smoke.rs new file mode 100644 index 00000000..cd47f410 --- /dev/null +++ b/src/hooks-pre-tool-validate/tests/smoke.rs @@ -0,0 +1,210 @@ +//! hooks smoke test (ADR-065) — PreToolUse の block/pass verdict を実 exe で検証する。 +//! +//! `hooks-pre-tool-validate` はリポジトリで唯一「Claude の操作を実際に止める」hook +//! (exit 2 = block) でありながら、これまで unit test しか無く **exe を通る経路 +//! (stdin JSON parse -> config -> preset/protected -> stderr + exit code)** は無検証だった。 +//! CI matrix (windows-latest / ubuntu-latest) の目的は「片方の OS でしか通らない経路」を +//! 露出させることなので、両 OS で同一の verdict が出ることを機械検証する土台としてここに置く。 +//! +//! 設計は ADR-049 の incident-eval スイートに倣う: +//! - **1 case = 1 failure mode**、かつ block (fire すべき入力) と pass (fire してはならない +//! 入力) を対で持つ。検出退行と false positive 退行の両方を止める。 +//! - assert は **exit code と stderr の有無のみ**に限定する。block メッセージ本文は +//! 固定しない (文言修正でテストが壊れないようにする)。 +//! +//! **exe は temp dir へ staging してから spawn する** (ADR-010 の実配置を再現する +//! `t7_cwd_independence` と同方式)。この hook は config を `current_exe()` の隣から +//! 解決するため、`target/debug` の exe を直接叩くと「そこに残っている config」に +//! verdict が左右され、fresh clone の CI とローカルで結果が食い違う。staging により +//! **deploy 済 config で判定する**ことを固定し、同時に `target/debug` を汚して +//! 他 crate の exe-spawn テストへ干渉することも防ぐ。 +//! +//! `.env` / `rm -rf` 等の値は synthetic な test data であり、実在のパス・実行対象ではない。 + +use lib_subprocess::{drain_pipe_unlimited, wait_with_timeout_safe}; +use std::io::Write; +use std::path::{Path, PathBuf}; +use std::process::{Command, Stdio}; + +/// spawn した hook exe の bounded wait (dev-conventions.md § bounded wait)。 +/// ハングした子プロセスは kill してテストを失敗させ、CI を無期限に止めない。 +const HOOK_TIMEOUT_SECS: u64 = 30; + +/// PreToolUse の block を表す exit code (main.rs のドキュメント参照)。 +const EXIT_BLOCK: i32 = 2; +/// PreToolUse の許可を表す exit code。 +const EXIT_PASS: i32 = 0; + +/// 1 つの failure mode に対する block/pass 期待。 +struct Case { + /// 失敗時に「どの経路が壊れたか」が判るラベル。 + name: &'static str, + /// hook JSON の `tool_name`。 + tool_name: &'static str, + /// `tool_input` に載せるキー (`command` or `file_path`)。 + field: &'static str, + /// `field` の値 (synthetic test data)。 + value: &'static str, + /// true = exit 2 + stderr にメッセージ、false = exit 0 + stderr 空。 + expect_block: bool, +} + +/// 通す経路ごとに block/pass を 1 対ずつ持つ: +/// preset 経路 (`blocked_patterns`、default preset の rm -rf ガード)、 +/// protected_files 経路 (機密ファイルの Write ブロック)、 +/// `main.rs` の match default arm (未知の tool は素通し = 過剰ブロックの退行ガード)。 +const CASES: &[Case] = &[ + Case { + name: "Bash: rm -rf を block する", + tool_name: "Bash", + field: "command", + value: "rm -rf /tmp/smoke-test-target", + expect_block: true, + }, + Case { + name: "Bash: 無害なコマンドは pass する", + tool_name: "Bash", + field: "command", + value: "ls -la", + expect_block: false, + }, + Case { + name: "Write: 保護対象ファイルを block する", + tool_name: "Write", + field: "file_path", + value: "/srv/example-project/.env", + expect_block: true, + }, + Case { + name: "Write: 通常のファイルは pass する", + tool_name: "Write", + field: "file_path", + value: "/srv/example-project/docs/notes.md", + expect_block: false, + }, + Case { + name: "未知の tool は pass する", + tool_name: "Read", + field: "file_path", + value: "/srv/example-project/docs/notes.md", + expect_block: false, + }, +]; + +fn built_exe() -> PathBuf { + PathBuf::from(env!("CARGO_BIN_EXE_hooks-pre-tool-validate")) +} + +fn repo_root() -> PathBuf { + PathBuf::from(env!("CARGO_MANIFEST_DIR")).join("..").join("..") +} + +/// exe と deploy 済 `hooks-config.toml` を temp dir へ配置し、staging 先の exe パスを返す。 +/// 返り値の `TempDir` は生存させ続けること (drop で削除される)。 +fn stage_hook() -> (tempfile::TempDir, PathBuf) { + let tmp = tempfile::tempdir().expect("create temp dir"); + + let exe_name = built_exe() + .file_name() + .expect("built exe has a file name") + .to_owned(); + let staged_exe = tmp.path().join(exe_name); + std::fs::copy(built_exe(), &staged_exe).expect("stage hook exe"); + + let config_src = repo_root().join(".claude").join("hooks-config.toml"); + assert!( + config_src.exists(), + "deployed hooks-config.toml missing at {} (false-green guard)", + config_src.display() + ); + std::fs::copy(&config_src, tmp.path().join("hooks-config.toml")) + .expect("stage hooks-config.toml"); + + (tmp, staged_exe) +} + +/// staging 済み hook exe を spawn し `(exit code, stderr)` を返す。 +/// +/// telemetry の kill-switch を立てるのは、staging した config が `[telemetry]` を +/// enable していても書き込みを起こさないため (テストの副作用を config に依存させない)。 +fn run_hook(exe: &Path, payload: &str) -> (i32, String) { + let mut child = Command::new(exe) + .env("CLAUDE_TELEMETRY_DISABLE", "1") + .stdin(Stdio::piped()) + .stdout(Stdio::piped()) + .stderr(Stdio::piped()) + .spawn() + .expect("spawn hooks-pre-tool-validate"); + 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 + .stdin + .take() + .expect("child stdin") + .write_all(payload.as_bytes()) + .expect("write stdin payload"); + let status = wait_with_timeout_safe("hooks-pre-tool-validate", &mut child, HOOK_TIMEOUT_SECS) + .expect("wait_with_timeout_safe errored"); + let _stdout = stdout_drain.join().expect("stdout drain thread panicked"); + let stderr = stderr_drain.join().expect("stderr drain thread panicked"); + let status = status.unwrap_or_else(|| { + panic!("hooks-pre-tool-validate hung > {HOOK_TIMEOUT_SECS}s (killed) — investigate") + }); + let code = status + .code() + .expect("hook exited via signal instead of an exit code"); + (code, stderr) +} + +#[test] +fn pre_tool_validate_block_pass_verdicts() { + let (_keep, exe) = stage_hook(); + for case in CASES { + let payload = serde_json::json!({ + "tool_name": case.tool_name, + "tool_input": { case.field: case.value }, + }) + .to_string(); + let (code, stderr) = run_hook(&exe, &payload); + + if case.expect_block { + assert_eq!( + code, EXIT_BLOCK, + "{}: block されるべき入力が exit {} で通過した (stderr: {})", + case.name, code, stderr + ); + assert!( + !stderr.is_empty(), + "{}: block したが stderr が空 (Claude に理由が伝わらない)", + case.name + ); + } else { + assert_eq!( + code, EXIT_PASS, + "{}: pass すべき入力が exit {} で止められた (false positive、stderr: {})", + case.name, code, stderr + ); + assert!( + stderr.is_empty(), + "{}: pass したのに stderr へ出力した: {}", + case.name, + stderr + ); + } + } +} + +/// 不正な stdin でも panic せず、かつ **block 側に倒れない**ことを確認する。 +/// +/// PreToolUse は全ツール呼び出しの前段に居るため、ここが exit 2 に倒れると Claude の +/// 操作が全面的に止まる。JSON parse 失敗は `ExitCode::FAILURE` (=1) で、Claude Code は +/// これを block として扱わない。 +#[test] +fn malformed_stdin_does_not_block() { + let (_keep, exe) = stage_hook(); + let (code, _stderr) = run_hook(&exe, "{ this is not json"); + assert_ne!( + code, EXIT_BLOCK, + "不正な stdin が block (exit 2) になった — 全ツール呼び出しを止めうる" + ); +} diff --git a/src/hooks-stop-quality/src/main.rs b/src/hooks-stop-quality/src/main.rs index 4af273cb..b358fb68 100644 --- a/src/hooks-stop-quality/src/main.rs +++ b/src/hooks-stop-quality/src/main.rs @@ -619,9 +619,13 @@ cmd = "pnpm test" } /// WP-05 並列化: 複数ステップを並列実行しても、失敗が step 定義順で集約され、 - /// 成功ステップは failure に含まれないこと。`run_cmd_shell_capped` は `cmd /c` 依存 - /// のため Windows でのみ実行する (WP-16 CI matrix の非 Windows leg では skip)。 - #[cfg(windows)] + /// 成功ステップは failure に含まれないこと。 + /// + /// 両 OS で実行する: `run_cmd_shell_capped` は ADR-063 で `shell_command` + /// (Windows=`cmd /c` / 非 Windows=`sh -c`) に抽象化済みで、step の `exit 0` / + /// `exit 1` は cmd.exe と POSIX sh の双方で同義。かつて付いていた + /// `#[cfg(windows)]` は ADR-063 以前の `cmd /c` 直書き時代の名残で、 + /// ADR-065 の CI matrix 導入時に PR #342 の CodeRabbit 指摘で発見・除去した。 #[test] fn run_quality_steps_parallel_collects_failures_in_step_order() { let steps = vec![ diff --git a/src/hooks-stop-quality/tests/t7_cwd_independence.rs b/src/hooks-stop-quality/tests/t7_cwd_independence.rs index e9a05437..51a6f6e0 100644 --- a/src/hooks-stop-quality/tests/t7_cwd_independence.rs +++ b/src/hooks-stop-quality/tests/t7_cwd_independence.rs @@ -17,8 +17,10 @@ //! bad = incident 状態 (cwd ≠ root) で誤 block しないこと。 //! good = 正規化がゲート自体を骨抜きにしていないこと (実失敗は cwd に依らず block)。 //! -//! `run_cmd_shell_capped` が `cmd /c` 依存のため Windows でのみ実行する -//! (WP-16 CI matrix の非 Windows leg では skip)。 +//! **Windows 限定の根拠**: `run_cmd_shell_capped` 自体は ADR-063 で OS 抽象化済み +//! だが、本テストが再現する incident は `.\.claude\probe.cmd` という **cmd.exe 固有の +//! ルート相対パス解決**そのものであり、POSIX sh に等価物が無い (fixture も `.cmd`)。 +//! よって主題が Windows 固有であり、ADR-065 CI matrix の非 Windows leg では skip される。 #![cfg(windows)] use lib_subprocess::{drain_pipe_unlimited, wait_with_timeout_safe};