diff --git a/.claude/hooks-config.toml b/.claude/hooks-config.toml index 2bea4ab2..747adf14 100644 --- a/.claude/hooks-config.toml +++ b/.claude/hooks-config.toml @@ -228,13 +228,18 @@ cmd = "pnpm build" # 順位 147 / PR-W5 (file-length-enforcement-plan.md): file-length Stop gate。 # PR 範囲 (base branch..@) の .rs file が 800 行超なら Stop を block する強制層。 -# cmd は cmd.exe (`cmd /c`) 経由で実行されるため forward-slash (`./`) 始まりは不可。 -# backslash 相対パスを TOML literal string (single quote、escape 不要) で指定する。 +# +# cmd は OS のシェル経由で実行される (Windows: cmd.exe / Linux: sh、WP-15)。 +# 両シェルで共通に解決できるのは **forward-slash の絶対パス**だけなので +# (cmd.exe は forward-slash 相対を、sh は backslash を解決できない = 実測)、 +# hooks-stop-quality が展開する 2 つのプレースホルダーで書く: +# {{CLAUDE_DIR}} → .claude/ の絶対パス (forward-slash 正規化) +# {{EXE_SUFFIX}} → .exe (Windows) / 空文字 (Linux) # gate の有効化は下記 [file_length_gate] enabled で制御 (この step があっても # enabled = false なら exe は即 exit 0 で no-op)。 [[stop_quality.steps]] name = "file-length" -cmd = '.\.claude\hooks-post-tool-comment-lint-rust.exe --check-modified-files' +cmd = '{{CLAUDE_DIR}}/hooks-post-tool-comment-lint-rust{{EXE_SUFFIX}} --check-modified-files' # ─── PR-W5: file-length Stop gate 設定 (ADR-039 experimental feature 標準パターン) ─── # diff --git a/.github/workflows/release-binaries.yml b/.github/workflows/release-binaries.yml new file mode 100644 index 00000000..7f31a8cc --- /dev/null +++ b/.github/workflows/release-binaries.yml @@ -0,0 +1,156 @@ +# release-binaries — Linux バイナリのビルドと rolling release への公開 (WP-15) +# +# 役割: master への push で全 Rust 成果物を x86_64-unknown-linux-gnu 向けにビルドし、 +# 固定タグ `nightly` の prerelease へ単一 tarball として公開する。使い捨ての +# クラウドセッション (claude.ai/code) が 19 crate をビルドせずにハーネスを即時 +# 有効化できるようにするのが目的。取得側は scripts/cloud-setup.sh。 +# +# 設計メモ: +# - **単一 tarball で公開する**: バイナリを個別 asset にすると、run が途中で失敗/ +# キャンセルされたとき release に新旧が混在した不整合なセットが残る。1 asset なら +# 差し替えが実質アトミックになり、取得側も 1 回のダウンロードで済む。 +# - **rolling tag (`nightly`) を上書き更新する**: 使い捨て環境向けなので「常に master +# 最新」であればよく、バージョン解決を setup script に持ち込みたくない。取得側は +# 常に同一 URL を叩けばよい。再現性が要る場合は tarball 内の BUILD_INFO に +# commit SHA が入っているので、そこから逆引きできる。 +# - **prerelease にする**: `gh release list` の latest に載せず、人間向けの正式 +# リリースと混同されないようにする。 +# - **バイナリ一覧は cargo metadata から導出する**: package.json の build:* や +# deploy-hooks.ts の allowlist に列挙をコピーすると、crate 追加時に片方だけ +# 更新されて無言で欠落する (実際 deploy-hooks.ts の allowlist は 11 個で、 +# settings.local.json.template が参照する 3 つの hook exe を既に取りこぼしている)。 +# workspace の bin target を機械的に集めることで、この drift を構造的に断つ。 +# - **公開前に cargo test を通す**: 壊れたバイナリを rolling release に載せると、 +# クラウドセッション側が原因不明の挙動不良を起こす (しかも setup は成功する) ため。 +# Windows leg と hooks smoke test を含む本格的な CI matrix は WP-16 で扱う。 +# - **paths フィルタで docs-only push を除外する**: 本リポジトリは docs / todo 更新の +# push が多く、そのたびに全 crate を再ビルドするのは無駄。バイナリの内容に影響する +# パスに限定する。 +# - checkout は persist-credentials: false (pr-monitor.yml と同じ token 漏洩対策)。 +# release の publish に使う token は該当 step の env にのみ置く。 + +name: release-binaries + +on: + push: + branches: [master] + paths: + - "src/**" + - "Cargo.toml" + - "Cargo.lock" + - ".github/workflows/release-binaries.yml" + workflow_dispatch: + +# 同時実行を許すと 2 つの run が同じ tag の asset を奪い合う。古い run を止めるのでは +# なく直列化する: cancel すると tarball の差し替えが中途半端な状態で終わりうるため。 +concurrency: + group: release-binaries + cancel-in-progress: false + +permissions: + contents: write # rolling release (tag `nightly`) の作成と asset 差し替えに必要 + +env: + RELEASE_TAG: nightly + TARGET_TRIPLE: x86_64-unknown-linux-gnu + +jobs: + build: + # glibc は前方互換 (新しい glibc 上で動く) だが後方互換ではないため、 + # ビルド側は実行環境より古い glibc に合わせる。22.04 = glibc 2.35。 + runs-on: ubuntu-22.04 + timeout-minutes: 30 + + 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-release-${{ hashFiles('Cargo.lock') }} + restore-keys: | + ${{ runner.os }}-cargo-release- + + - name: Show toolchain versions + run: | + rustc --version + cargo --version + ldd --version | head -1 + + # 壊れたバイナリを rolling release に載せないためのゲート。 + # Linux 側で cmd.exe 依存が復活していないかの検知も兼ねる (WP-15)。 + - name: Run tests + run: cargo test --workspace + + - name: Build all workspace binaries + run: cargo build --release --workspace --bins + + # bin target 名を cargo metadata から取り出す (列挙のハードコードを避ける)。 + # workspace_members に属する package だけを見て、依存 crate の bin を拾わない。 + - name: Collect binaries into a single tarball + id: package + run: | + set -euo pipefail + mkdir -p dist/bin + + cargo metadata --format-version 1 --no-deps \ + | jq -r '.packages[].targets[] | select(.kind[] == "bin") | .name' \ + | sort -u > dist/bin-names.txt + + if [ ! -s dist/bin-names.txt ]; then + echo "error: no bin targets found via cargo metadata" >&2 + exit 1 + fi + + echo "packaging $(wc -l < dist/bin-names.txt) binaries:" + while read -r name; do + src="target/release/${name}" + if [ ! -x "$src" ]; then + echo "error: expected binary not found: ${src}" >&2 + exit 1 + fi + cp "$src" "dist/bin/${name}" + echo " - ${name}" + done < dist/bin-names.txt + + # 取得側が「いつ・どの commit のバイナリか」を確認できるようにする。 + # rolling tag は履歴を持たないため、この情報が唯一の provenance になる。 + cat > dist/bin/BUILD_INFO < "${archive}.sha256" + echo "archive=${archive}" >> "$GITHUB_OUTPUT" + + # tag が無ければ作り、あれば asset を --clobber で差し替える。 + # notes に commit を書いておくと Release ページだけで世代が判る。 + - name: Publish to the rolling prerelease + env: + GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} + ARCHIVE: ${{ steps.package.outputs.archive }} + run: | + set -euo pipefail + if ! gh release view "${RELEASE_TAG}" >/dev/null 2>&1; then + gh release create "${RELEASE_TAG}" \ + --prerelease \ + --title "Linux binaries (rolling)" \ + --notes "master の最新ビルド。scripts/cloud-setup.sh が取得する。" + fi + + gh release upload "${RELEASE_TAG}" "${ARCHIVE}" "${ARCHIVE}.sha256" --clobber + + gh release edit "${RELEASE_TAG}" \ + --notes "master の最新ビルド (commit ${GITHUB_SHA})。scripts/cloud-setup.sh が取得する。" diff --git a/.gitignore b/.gitignore index 2a67f0fd..a759fa70 100644 --- a/.gitignore +++ b/.gitignore @@ -7,6 +7,15 @@ # Built executables (can be rebuilt with pnpm build:all) .claude/*.exe +# Linux / macOS のバイナリには拡張子が無く *.exe では捕まらないため、名前で無視する +# (WP-15: クラウドセッションで cloud-setup.sh が配置したバイナリを jj が snapshot して +# しまうのを防ぐ)。設定ファイルを巻き込むのは hooks-config.toml だけなので negate で戻す。 +.claude/check-ci-coderabbit +.claude/cli-* +.claude/hooks-* +!.claude/hooks-config.toml +# cloud-setup.sh が展開する provenance ファイル +.claude/BUILD_INFO # Deploy targets (contains local paths; create from deploy-targets.template.json) scripts/deploy-targets.json diff --git a/docs/harness-improvement-plan.md b/docs/harness-improvement-plan.md index d23ed8fa..e23a8ea3 100644 --- a/docs/harness-improvement-plan.md +++ b/docs/harness-improvement-plan.md @@ -74,7 +74,7 @@ Anthropic 公式のハーネスエンジニアリング指針（決定論的基 | WP-12 | 2 | 発火テレメトリ + ハーネス ROI 棚卸し | M | なし | 実装済（step1 収集層のみ: ADR-055 + lib-telemetry + 6 hook 計装。step2-3〔集計 pre-step / 卒業判定機械化〕は 28 日 warm-up 後着手のため todo 順位 307/308 へ移管） | | WP-13 | 3 | EXE_SUFFIX 抽象化 | M | なし | 実装済（build/実行 scripts を deploy-artifacts.mjs / run-artifact.mjs 経由に、settings を `/` 区切り + `{{EXE_SUFFIX}}` 化、Rust の機能的 exe 解決を EXE_SUFFIX 化。ADR-005 amendment。cargo test 全 pass・build:all/deploy:hooks/lint:docs 退行なし実測。config TOML の cmd.exe 依存は WP-15 へ。`完了` は初回 push/PR で launcher 経路の実走確認後） | | WP-14 | 3 | PowerShell 3 本の Rust 化 | S-M ×2 | なし | 実装済（3 本すべて Rust 化: fix-metrics-check→comment-lint `--fix-metrics-check` / prepare-pr-body→cli-pr-monitor サブコマンド / analyze-takt-timings→新規 cli-takt-timings crate。cargo test カバレッジ下・実データで旧 ps1 と出力一致確認。`完了` は初回 push/PR で fix step metrics-check と prepare-pr-body 経路の実走確認後） | -| WP-15 | 3 | Linux バイナリビルド + クラウド setup script | M | WP-13, 14 | 未着手 | +| WP-15 | 3 | Linux バイナリビルド + クラウド setup script | M | WP-13, 14 | 実装済（release-binaries.yml〔master push → rolling `nightly` prerelease に単一 tarball〕+ scripts/cloud-setup.sh 新設。前提として Linux 実行時に壊れる可搬性欠陥を修正: `cmd /c` 決め打ちの唯一の shell spawn 点を `shell_command`〔Windows=cmd /c / 他=sh -c〕へ集約、taskkill のみだった timeout kill に unix 分岐、cmd.exe 構文テストの OS 中立化、config の `.exe`/backslash 依存を `{{CLAUDE_DIR}}`/`{{EXE_SUFFIX}}` 展開へ。**WSL Ubuntu 24.04 で実測**: cargo test --workspace 全 pass・ignored 含め全 pass・clippy clean・hooks 実発火・push pipeline が sh -c 経路で完走。Linux 実測により lock の同時取得レース〔8 中 6 取得〕も発見・修正。`完了` は release 実生成 + 実クラウドセッションでの cloud-setup.sh 実走確認後） | | WP-16 | 3 | CI matrix（移植退行防止） | S | WP-13, 14 | 未着手 | | WP-17 | 4 | イベント駆動バックボーン完成（Phase B + routines 移行） | M | WP-09, 10, 11 | 未着手 | | WP-18 | 4 | 夜間 todo 消化ループ | M-L | WP-15, 17 | 未着手 | @@ -251,6 +251,24 @@ Anthropic 公式のハーネスエンジニアリング指針（決定論的基 ### WP-15: Linux バイナリビルド + クラウド setup script +> **実装済 (2026-07-20)**: 8 コミットで実装。**スコープ補正**: 当初ステップ (release workflow + setup script) の手前に、**Linux では実行時に壊れる可搬性欠陥**が残っていることが着手時調査で判明したため、受け入れ基準「`cargo test` と push pipeline の dry-run が通る」を満たす前提としてこれを先に修正した。 +> +> **① 可搬性修正 (Linux 実行を成立させる前提)**: (a) `lib-subprocess` の `run_cmd_shell_*` が `Command::new("cmd").args(["/c", …])` に固定されており、これが**リポジトリ唯一の shell spawn 点**だったため Linux では quality_gate / push / merge の全 step が spawn 失敗で無言に失敗扱いになる状態だった。OS 判定で `cmd /c` / `sh -c` を返す `shell_command` へ集約 (bash 固有構文を使わない前提で POSIX `sh` を選択 = bash 不在の最小コンテナでも通る)。`cli-push-runner` の `diff.rs` も同じ経路へ統合。(b) `check-ci-coderabbit` の timeout kill が `taskkill` のみで非 Windows 分岐が無く、Linux では `wait_with_output` が**無限ハング**していた (gh がハングすると CI 監視が永久停止) → `kill_process_by_id` で Windows=taskkill / Unix=kill -9 に分岐。(c) cmd.exe 構文 (`for /L` / `type nul` / `exit /b` / `A & B` / `ping -n`) を直書きしたテストが cfg 未ガードで残っており Linux で panic / assert 失敗 → OS 別 const 化 (行数・所要時間を両 OS で揃え、片側だけ主題を検証しない穴を防ぐ)。 +> +> **② デプロイ時 config の cmd.exe 依存解消 (WP-13 からの引き継ぎ)**: file-length step の `.\.claude\….exe` は backslash + `.exe` 決め打ちで sh では解決不能、一方 cmd.exe は forward-slash **相対**パスを解決できない。**実測の結果、両シェルが共通で通るのは forward-slash の絶対パスだけ**だったため (ADR-005 が settings.local.json で確認済みの性質と同型)、`hooks-stop-quality` に `{{CLAUDE_DIR}}` / `{{EXE_SUFFIX}}` の展開を追加。`push-runner-config.toml` の `[lint_screen] exe_path` は code 側が既に OS 分岐 default を持つため明示指定をやめた。 +> +> **③ ステップ 1 (release workflow)**: `.github/workflows/release-binaries.yml`。master push (paths フィルタで docs-only を除外) で `x86_64-unknown-linux-gnu` をビルドし、固定タグ `nightly` の prerelease へ**単一 tarball**で公開。単一 asset にしたのは run 失敗時に新旧混在の不整合セットが残らないようにするため。**バイナリ一覧は `cargo metadata` から導出**し、package.json / deploy-hooks.ts への列挙コピーによる drift を構造的に断つ (実際 deploy-hooks.ts の allowlist は 11 個で、settings template が参照する hook exe を 3 つ取りこぼしていた)。ubuntu-22.04 固定は glibc 後方互換が無いため。musl は tree-sitter の C コンパイルに musl-tools が必要でビルドが一段複雑になるのに対し、実行先が Ubuntu 系で glibc 2.35 なら十分と判断して不採用。 +> +> **④ ステップ 2 (setup script)**: `scripts/cloud-setup.sh`。**要確認事項への回答**: public リポジトリの Release asset は素の HTTPS で取得できるため **gh CLI 認証は不要** (トークン受け渡し構成を持ち込まない = 失敗点を増やさない)。必須バイナリ一覧は `settings.local.json.template` から導出 (「どの exe が無いと hooks が発火しないか」の正解はテンプレート自身が持つ)。バイナリ欠落・settings 生成失敗は fail-closed (setup 成功と報告してハーネス無しで進む事故を防ぐ、ADR-005 の背景と同型)。jj は 0.42.0 固定 (ADR-011/015/045 が 0.42 系挙動に依存)、takt は `pnpm install --frozen-lockfile` で ADR-017 の固定を機械的に担保。あわせて `.gitignore` が `.claude/*.exe` のみで**拡張子なし Linux バイナリを無視しない**問題も修正 (クラウドで jj が成果物を snapshot してしまう)。 +> +> **⑤ ステップ 3 (Ollama graceful skip)**: コード監査で**無条件に skip される**ことを確認。lint_screen は `enabled = false` かつ戻り値が `()` で構造的に block 不可能。classifier は exe 側が fallback JSON + exit 0 を返し、runner 側が全失敗経路を空 Vec に潰す二重の fail-open。実 Ollama を叩く eval は `#[ignore]` + env opt-in の二重ガードで `cargo test -- --ignored` でも skip される。fail-closed であるべきゲート (`[fix.gate]` / `docs_only_routing` / `post_takt_regate`) は Ollama 非依存で、ADR-043 の線引きは正しく引かれている。 +> +> **受け入れ基準の実測 (WSL Ubuntu 24.04 = 実 Linux)**: `cargo test --workspace` 全 pass / `cargo test -- --ignored --test-threads=1` 全 pass (jj 導入後) / `clippy --workspace --all-targets --all-features -- -D warnings` clean / **hooks 実発火**(SessionStart が additionalContext JSON を出力、PreToolUse が `rm -rf /` を exit 2 でブロックし `echo hello` を通す、tree-sitter の comment-lint が違反検出) / **push pipeline が `sh -c` 経路で完走**(quality_gate の rust-lint-test が clippy・cargo test とも PASS)。cloud-setup.sh の jj 取得は実 URL・実展開ロジックで実走確認。 +> +> **Linux 実測で発見した副次不具合**: `cli-pr-monitor` の lock が**同時取得**を許していた (8 スレッド中 6 つが取得)。`create_new` は atomic だが直後のファイルは空で、その窓を読んだ側が TOML parse 失敗を一律「stale」と扱って全員 takeover していた。Windows ではスケジューリング差で顕在化していなかっただけで欠陥は同じ。parse 失敗を内容で 2 分 (空 = 書き込み中 → busy / 非空の不正 = 破損 → takeover) して修正。**「Windows だけで回していると気付けない設計欠陥が実在した」= Linux 実測と WP-16 (CI matrix) の価値を裏づける実例。** +> +> **`完了` 条件**: (1) 本変更が master に入り release-binaries.yml が実走して `nightly` release が生成されること、(2) 実際の claude.ai/code セッションで `cloud-setup.sh` を走らせ hooks 発火と `cargo test` 通過を確認すること。いずれも本セッションでは実施不能 (release 未生成 / クラウド環境未使用) のため `実装済` に留める。**Linux 側の未検証領域**: `#[cfg(windows)]` ガードのテスト (pump_child_io の deadlock 保護、run_cmd_capture の stdout/stderr 分離) は Linux で skip されるため、WP-16 の CI matrix で扱う。以下は当初ステップ (記録用)。 + - **目的**: 使い捨てのクラウドセッションで 19 crate をビルドせずにハーネスを即時有効化する。 - **ステップ**: 1. `.github/workflows/release-binaries.yml`: master push で `x86_64-unknown-linux-gnu` をビルドし artifact/Release へ。`lib-ollama-client` が ureq + rustls 構成なら musl 静的リンクも検討（openssl 依存を避ける）。public リポジトリのためビルド時間は無料。 diff --git a/push-runner-config.toml b/push-runner-config.toml index 073f8d15..cf489de6 100644 --- a/push-runner-config.toml +++ b/push-runner-config.toml @@ -178,8 +178,11 @@ output_path = ".takt/review-diff.txt" # --------------------------------------------------------------------------- [lint_screen] enabled = false -# 以下は default 値、明示しなくてもよいが意図を残すなら明示推奨 -exe_path = ".claude/cli-finding-classifier.exe" +# 以下は default 値、明示しなくてもよいが意図を残すなら明示推奨。 +# exe_path は **あえて明示しない** (WP-15): code 側の default が +# `.claude/cli-finding-classifier` + OS の実行ファイル拡張子で cfg 分岐しており、 +# ここに `.exe` 付きの値を書くと Linux で解決に失敗する。OS ごとに値を変えたい +# 場合のみ明示すること。 model = "mistral:7b" endpoint = "http://localhost:11434" timeout_secs = 60 diff --git a/scripts/cloud-setup.sh b/scripts/cloud-setup.sh new file mode 100755 index 00000000..c47077b1 --- /dev/null +++ b/scripts/cloud-setup.sh @@ -0,0 +1,238 @@ +#!/usr/bin/env bash +# +# cloud-setup.sh — 使い捨てクラウドセッション用のハーネス有効化スクリプト (WP-15) +# +# 役割: claude.ai/code のような使い捨て Linux 環境で、19 crate をビルドせずに +# hooks / CLI を即時有効化する。release-binaries.yml が公開した rolling release +# (タグ `nightly`) から Linux バイナリを取得し、.claude/ へ配置して settings を生成する。 +# +# 使い方: 環境設定の setup script に登録する (環境キャッシュが効くため 2 回目以降は高速)。 +# bash scripts/cloud-setup.sh +# +# 設計メモ: +# - **認証を要求しない**: public リポジトリの Release asset は素の HTTPS で取得できる。 +# gh CLI の認証を setup に持ち込むとトークンを環境変数で渡す構成が必要になり、 +# クラウドの許可リスト型ネットワーク設定と合わせて失敗点が増える。curl だけで完結させる。 +# - **hooks が無言で無効化される経路を fail-closed にする**: バイナリ欠落や settings 生成 +# 失敗をスキップして「setup 成功」と報告すると、セッションはハーネス無しで進み、 +# その事実に誰も気付かない (ADR-005 が対処した事故と同じ形)。必須要素は即 exit 1 で落とす。 +# - **必須バイナリの一覧は settings.local.json.template から導出する**: どの exe が +# 無いと hooks が発火しないかの正解はテンプレート自身が持っている。ここに一覧を +# コピーすると hook 追加時に片方だけ更新されて無言で穴が開く。 +# - **冪等**: 再実行しても壊れない。既存の jj / node_modules は再利用する。 +# - **Ollama 依存機能は非対象**: クラウドに Ollama は無い。lint_screen 等は default OFF かつ +# fail-open のため、導入せず「skip される」ことを最後に明示するだけに留める。 + +set -euo pipefail + +# ─── 設定 (環境変数で上書き可能) ─── + +# 取得元。fork や検証用ビルドを指す場合に差し替える。 +readonly REPO_SLUG="${CLOUD_SETUP_REPO_SLUG:-aloekun/claude-code-hook-test}" +readonly RELEASE_TAG="${CLOUD_SETUP_RELEASE_TAG:-nightly}" +readonly TARGET_TRIPLE="x86_64-unknown-linux-gnu" + +# jj は ADR-011 / ADR-015 / ADR-045 が 0.42 系の挙動 (`jj git push -b` の自動 track 等) に +# 依存しているため、ローカル検証環境と同じバージョンを固定する。 +readonly JJ_VERSION="${CLOUD_SETUP_JJ_VERSION:-0.42.0}" +readonly JJ_TARGET_TRIPLE="x86_64-unknown-linux-musl" + +readonly INSTALL_BIN_DIR="${CLOUD_SETUP_BIN_DIR:-$HOME/.local/bin}" + +# ─── ログ ─── + +log() { printf '[cloud-setup] %s\n' "$*"; } +warn() { printf '[cloud-setup] Warning: %s\n' "$*" >&2; } +die() { printf '[cloud-setup] Error: %s\n' "$*" >&2; exit 1; } + +# ─── リポジトリルートの解決 ─── +# +# cwd に依存させない (setup script がどこから呼ばれるか保証が無いため)。 +SCRIPT_DIR="$(cd -- "$(dirname -- "${BASH_SOURCE[0]}")" && pwd)" +readonly REPO_ROOT="$(cd -- "${SCRIPT_DIR}/.." && pwd)" +readonly CLAUDE_DIR="${REPO_ROOT}/.claude" +readonly SETTINGS_TEMPLATE="${CLAUDE_DIR}/settings.local.json.template" + +cd "${REPO_ROOT}" + +# ─── 前提コマンドの確認 ─── + +require_command() { + command -v "$1" >/dev/null 2>&1 || die "必須コマンドが見つかりません: $1" +} + +require_command curl +require_command tar +require_command node + +[ -f "${SETTINGS_TEMPLATE}" ] || die "settings テンプレートがありません: ${SETTINGS_TEMPLATE}" + +log "リポジトリルート: ${REPO_ROOT}" + +# ─── 1. Linux バイナリの取得と配置 ─── + +install_harness_binaries() { + local archive="claude-code-hooks-${TARGET_TRIPLE}.tar.gz" + local base_url="https://github.com/${REPO_SLUG}/releases/download/${RELEASE_TAG}" + local tmp_dir + tmp_dir="$(mktemp -d)" + # 途中で失敗しても一時ディレクトリを残さない。 + trap 'rm -rf "${tmp_dir}"' RETURN + + log "バイナリを取得中: ${base_url}/${archive}" + # --fail: HTTP 404/5xx を silent な空ファイルではなく exit 非 0 にする + # (これが無いと「取得成功・中身が HTML のエラーページ」で tar が謎の失敗をする)。 + curl --fail --location --silent --show-error \ + --output "${tmp_dir}/${archive}" \ + "${base_url}/${archive}" \ + || die "バイナリの取得に失敗しました。release-binaries.yml が tag '${RELEASE_TAG}' を公開済みか確認してください。" + + # checksum は「あれば検証する」: release 側の生成に失敗していても setup 全体を + # 止めるほどではないが、取得できたのに不一致なら転送破損なので落とす。 + if curl --fail --location --silent --show-error \ + --output "${tmp_dir}/${archive}.sha256" \ + "${base_url}/${archive}.sha256" 2>/dev/null; then + if command -v sha256sum >/dev/null 2>&1; then + ( cd "${tmp_dir}" && sha256sum --check --status "${archive}.sha256" ) \ + || die "checksum 不一致: ダウンロードが破損しています" + log "checksum 検証: OK" + else + warn "sha256sum が無いため checksum 検証を skip します" + fi + else + warn "checksum ファイルを取得できませんでした (検証を skip)" + fi + + mkdir -p "${CLAUDE_DIR}" + tar -xzf "${tmp_dir}/${archive}" -C "${CLAUDE_DIR}" + + # tar が実行ビットを保持しない環境でも hooks が起動できるようにする。 + # BUILD_INFO はバイナリではないので対象外。 + find "${CLAUDE_DIR}" -maxdepth 1 -type f ! -name '*.*' ! -name 'BUILD_INFO' \ + -exec chmod +x {} + + + if [ -f "${CLAUDE_DIR}/BUILD_INFO" ]; then + log "配置したビルドの provenance:" + sed 's/^/ /' "${CLAUDE_DIR}/BUILD_INFO" + fi +} + +# settings テンプレートが参照する hook exe が実際に配置されたかを検証する。 +# 1 つでも欠けると該当 hook が無言で発火しないため fail-closed で落とす。 +verify_required_binaries() { + local missing + missing="$( + node --input-type=module -e ' + import { readFileSync } from "node:fs"; + import { existsSync } from "node:fs"; + + const [templatePath, claudeDir] = process.argv.slice(2); + const template = readFileSync(templatePath, "utf8"); + + // テンプレートの command は "{{PROJECT_DIR}}/.claude/{{EXE_SUFFIX}}" 形式。 + const names = new Set( + [...template.matchAll(/\.claude\/([A-Za-z0-9._-]+)\{\{EXE_SUFFIX\}\}/g)].map((m) => m[1]), + ); + if (names.size === 0) { + console.error("no hook executables referenced by the template"); + process.exit(2); + } + for (const name of [...names].sort()) { + if (!existsSync(`${claudeDir}/${name}`)) console.log(name); + } + ' "${SETTINGS_TEMPLATE}" "${CLAUDE_DIR}" + )" || die "必須バイナリの検証スクリプトが失敗しました" + + if [ -n "${missing}" ]; then + printf '%s\n' "${missing}" | sed 's/^/ - /' >&2 + die "settings テンプレートが参照する hook バイナリが不足しています (hooks が無言で無効化されます)" + fi + log "必須 hook バイナリ: すべて配置済み" +} + +# ─── 2. settings.local.json の生成 ─── +# +# ADR-005: CLAUDE_PROJECT_DIR が空になる環境があるため、絶対パスを埋め込んだ +# settings.local.json をビルド時に生成する。生成失敗 = hooks 全停止なので fail-closed。 +generate_settings() { + node "${REPO_ROOT}/scripts/build-hooks-settings.mjs" \ + || die "settings.local.json の生成に失敗しました (hooks が有効化されません)" +} + +# ─── 3. jj の導入 ─── +# +# 既に PATH にあり、かつ目的のバージョンなら何もしない (再実行時の無駄な取得を避ける)。 +install_jj() { + if command -v jj >/dev/null 2>&1 && jj --version 2>/dev/null | grep -q "${JJ_VERSION}"; then + log "jj ${JJ_VERSION}: 導入済み (skip)" + return 0 + fi + + local archive="jj-v${JJ_VERSION}-${JJ_TARGET_TRIPLE}.tar.gz" + local url="https://github.com/jj-vcs/jj/releases/download/v${JJ_VERSION}/${archive}" + local tmp_dir + tmp_dir="$(mktemp -d)" + trap 'rm -rf "${tmp_dir}"' RETURN + + log "jj ${JJ_VERSION} を取得中" + if ! curl --fail --location --silent --show-error --output "${tmp_dir}/${archive}" "${url}"; then + warn "jj の取得に失敗しました。jj 依存の pipeline (pnpm push 等) は使えません。" + return 0 + fi + + tar -xzf "${tmp_dir}/${archive}" -C "${tmp_dir}" + + # アーカイブのレイアウト (flat / サブディレクトリ入り) に依存しないよう探索して拾う。 + local jj_bin + jj_bin="$(find "${tmp_dir}" -type f -name jj -print -quit)" + if [ -z "${jj_bin}" ]; then + warn "アーカイブ内に jj バイナリが見つかりませんでした" + return 0 + fi + + mkdir -p "${INSTALL_BIN_DIR}" + install -m 0755 "${jj_bin}" "${INSTALL_BIN_DIR}/jj" + log "jj を配置: ${INSTALL_BIN_DIR}/jj" + + case ":${PATH}:" in + *":${INSTALL_BIN_DIR}:"*) ;; + *) warn "${INSTALL_BIN_DIR} が PATH にありません。PATH へ追加してください。" ;; + esac +} + +# ─── 4. Node 依存 (takt を含む) の導入 ─── +# +# ADR-017: takt はバージョン完全固定 (package.json に "takt": "0.35.3")。 +# --frozen-lockfile で lockfile と一致しない解決を拒否し、固定を機械的に担保する。 +install_node_dependencies() { + if ! command -v pnpm >/dev/null 2>&1; then + warn "pnpm が無いため takt / node 依存を導入できません (pnpm push 等は使えません)" + return 0 + fi + log "pnpm install --frozen-lockfile を実行中 (takt は ADR-017 で固定)" + pnpm install --frozen-lockfile \ + || warn "pnpm install に失敗しました。takt 依存の pipeline は使えません。" +} + +# ─── 5. 環境差分の明示 ─── +# +# 「動かない」ではなく「意図して skip される」ことを可視化する。 +report_optional_features() { + if command -v ollama >/dev/null 2>&1; then + log "Ollama: 検出 (lint_screen / findings classification が利用可能)" + else + log "Ollama: 未導入 — lint_screen / findings classification は skip されます (fail-open、想定内)" + fi +} + +main() { + install_harness_binaries + verify_required_binaries + generate_settings + install_jj + install_node_dependencies + report_optional_features + log "セットアップ完了" +} + +main "$@" diff --git a/src/check-ci-coderabbit/src/main.rs b/src/check-ci-coderabbit/src/main.rs index 238704b6..fbacafca 100644 --- a/src/check-ci-coderabbit/src/main.rs +++ b/src/check-ci-coderabbit/src/main.rs @@ -93,52 +93,87 @@ fn parse_args() -> Result { // ─── gh CLI 実行 ─── -/// gh コマンドを実行し stdout を返す。タイムアウト 30 秒。 -/// パイプのデッドロックを防ぐため、タイムアウトは別スレッドで kill し、 -/// メインスレッドは wait_with_output でパイプを安全に読み取る。 +/// PID を指定して子プロセスを強制終了する (Windows: `taskkill /F` / Unix: `kill -9`)。 /// -/// NOTE: タイムアウト時のプロセス kill は Windows (taskkill) のみ実装。 -/// この exe は Windows 専用として設計されている (ADR-001)。 -fn run_gh(args: &[&str]) -> Result { - let child = Command::new("gh") - .args(args) - .stdout(std::process::Stdio::piped()) - .stderr(std::process::Stdio::piped()) - .spawn() - .map_err(|e| format!("gh の起動に失敗: {}", e))?; +/// `run_gh` のタイマースレッドは `child` を move できない (メインスレッドが +/// `wait_with_output` でパイプを読むため) ので、PID 経由で外から殺す。 +/// +/// **この分岐が片肺だと timeout が機能しない**: kill されなければ `wait_with_output` +/// は子の自然終了までブロックし続け、`timeout_flag` が立っても制御が戻らない。 +/// WP-15 以前は Windows 分岐しか無く、Linux では gh がハングすると CI 監視が +/// 永久停止する状態だった。 +/// +/// 外部コマンド経由なのは libc 依存を増やさないため。kill 自体の失敗は握り潰す +/// (既に自然終了していれば失敗するのが正常で、その場合 timeout 判定も無害)。 +fn kill_process_by_id(pid: u32) { + #[cfg(target_os = "windows")] + { + let _ = Command::new("taskkill") + .args(["/F", "/PID", &pid.to_string()]) + .output(); + } + #[cfg(not(target_os = "windows"))] + { + let _ = Command::new("kill").args(["-9", &pid.to_string()]).output(); + } +} - // タイムアウト用: done_flag で早期終了、timeout_flag でタイムアウト判定 - let child_id = child.id(); +/// タイムアウト監視スレッドを起動し `(timeout_flag, done_flag)` を返す。 +/// +/// スレッドは deadline まで一括で寝るのではなく 100ms 刻みで `done_flag` を見て +/// 早期終了する (正常終了時にプロセスの exit を 30 秒引き延ばさないため)。 +/// deadline 到達時は `timeout_flag` を立ててから PID 経由で子を強制終了する。 +/// +/// 呼び出し側は `wait_with_output` から戻ったら**必ず `done_flag` を立てる**こと。 +/// 立て忘れるとスレッドが deadline まで生き残り、既に終了した子の PID を kill する +/// (PID 再利用があれば無関係のプロセスを殺しうる)。 +fn spawn_timeout_killer( + child_id: u32, +) -> ( + std::sync::Arc, + std::sync::Arc, +) { let timeout_flag = std::sync::Arc::new(std::sync::atomic::AtomicBool::new(false)); let done_flag = std::sync::Arc::new(std::sync::atomic::AtomicBool::new(false)); let flag_clone = timeout_flag.clone(); let done_clone = done_flag.clone(); - // タイマースレッドは 100ms 刻みで done_flag をチェックし、早期終了する std::thread::spawn(move || { let deadline = std::time::Instant::now() + Duration::from_secs(30); while std::time::Instant::now() < deadline { if done_clone.load(std::sync::atomic::Ordering::Relaxed) { - return; // プロセス完了 → スレッド即終了 + return; } std::thread::sleep(Duration::from_millis(100)); } - // タイムアウト到達 flag_clone.store(true, std::sync::atomic::Ordering::Relaxed); - #[cfg(target_os = "windows")] - { - let _ = Command::new("taskkill") - .args(["/F", "/PID", &child_id.to_string()]) - .output(); - } + kill_process_by_id(child_id); }); - let output = child - .wait_with_output() - .map_err(|e| format!("gh 出力の取得に失敗: {}", e))?; + (timeout_flag, done_flag) +} + +/// gh コマンドを実行し stdout を返す。タイムアウト 30 秒。 +/// パイプのデッドロックを防ぐため、タイムアウトは別スレッドで kill し、 +/// メインスレッドは wait_with_output でパイプを安全に読み取る。 +/// +/// **`wait_with_output` の結果は一旦変数に受けてから `?` する**。エラーを直接 +/// 伝播させると `done_flag` を立てる前に関数を抜け、タイマースレッドが deadline +/// まで生き残って PID 再利用時に無関係のプロセスを kill しうる +/// (`spawn_timeout_killer` の doc が要求する契約。CodeRabbit PR #307 指摘)。 +fn run_gh(args: &[&str]) -> Result { + let child = Command::new("gh") + .args(args) + .stdout(std::process::Stdio::piped()) + .stderr(std::process::Stdio::piped()) + .spawn() + .map_err(|e| format!("gh の起動に失敗: {}", e))?; + + let (timeout_flag, done_flag) = spawn_timeout_killer(child.id()); - // プロセス完了をタイマースレッドに通知 + let wait_result = child.wait_with_output(); done_flag.store(true, std::sync::atomic::Ordering::Relaxed); + let output = wait_result.map_err(|e| format!("gh 出力の取得に失敗: {}", e))?; if timeout_flag.load(std::sync::atomic::Ordering::Relaxed) { return Err("gh コマンドがタイムアウトしました".to_string()); diff --git a/src/cli-pr-monitor/src/classifier_runner.rs b/src/cli-pr-monitor/src/classifier_runner.rs index caea191d..e0a19872 100644 --- a/src/cli-pr-monitor/src/classifier_runner.rs +++ b/src/cli-pr-monitor/src/classifier_runner.rs @@ -269,34 +269,38 @@ mod tests { assert_eq!(parsed[0].finding.severity, "Critical"); } + /// A long-running command used to exercise the timeout path. cmd.exe and POSIX sh + /// disagree on syntax, so the fixture is selected per OS (WP-15). Keep the runtime + /// comparable on both sides so neither leg silently stops exercising the timeout. + #[cfg(windows)] + const LONG_RUNNING_CMD: &str = "ping 127.0.0.1 -n 3"; + #[cfg(not(windows))] + const LONG_RUNNING_CMD: &str = "sleep 3"; + /// `wait_with_timeout` success path: a fast process completes within a generous timeout. #[test] fn wait_with_timeout_returns_some_when_process_completes() { - let child = Command::new("cmd") - .arg("/c") - .arg("echo ok") + let child = lib_subprocess::shell_command("echo ok") .stdout(Stdio::piped()) .stderr(Stdio::piped()) .spawn() - .expect("failed to spawn cmd"); + .expect("failed to spawn shell command"); let result = wait_with_timeout(child, Duration::from_secs(5)); assert!(result.is_some(), "process should complete within 5-second timeout"); } /// `wait_with_timeout` timeout path: a long-running process exceeds a short timeout. /// - /// Note: the child process (ping) continues running in a background thread until it - /// terminates naturally (~2 s). This is intentional — the thread-based design cannot + /// Note: the child process continues running in a background thread until it + /// terminates naturally (~3 s). This is intentional — the thread-based design cannot /// kill the child after the timeout (child is moved into the thread). Acceptable in tests. #[test] fn wait_with_timeout_returns_none_on_timeout() { - let child = Command::new("cmd") - .arg("/c") - .arg("ping 127.0.0.1 -n 3") + let child = lib_subprocess::shell_command(LONG_RUNNING_CMD) .stdout(Stdio::piped()) .stderr(Stdio::piped()) .spawn() - .expect("failed to spawn cmd"); + .expect("failed to spawn shell command"); let result = wait_with_timeout(child, Duration::from_millis(50)); assert!(result.is_none(), "process should not complete within 50 ms timeout"); } diff --git a/src/cli-pr-monitor/src/lock.rs b/src/cli-pr-monitor/src/lock.rs index a6f0323b..2cc9c5a7 100644 --- a/src/cli-pr-monitor/src/lock.rs +++ b/src/cli-pr-monitor/src/lock.rs @@ -134,19 +134,92 @@ fn build_lock_content(mode: &str) -> Option { } } +/// `create_new` 成功から内容書き込み完了までの窓を吸収する猶予 (WP-15)。 +/// +/// `create_new` は atomic だが、その直後にファイルは**空**で存在する。この窓で +/// 別プロセスが読むと TOML parse に失敗し、素朴に「壊れている = stale」と扱うと +/// 全員が takeover して**同時取得**が起きる (Linux で 6/6 スレッドが取得する +/// 実測不具合。Windows はスケジューリングの差で顕在化していなかっただけ)。 +/// +/// 実際の書き込みはミリ秒で完了するため数秒あれば十分。短く保つことで、 +/// 「create 直後に crash して空ファイルが残った」場合の巻き添えも数秒で解ける。 +const LOCK_WRITE_WINDOW_SECS: i64 = 5; + +/// parse 不能な lock の pid は不明。表示用に「不明」を表す番兵。 +const UNKNOWN_HOLDER_PID: u32 = 0; + +/// 2 つの時刻から経過秒を求める。**mtime が未来なら 0 (作成直後) とみなす**。 +/// +/// `duration_since` は mtime が未来だと `Err` を返す。これを「齢が不明」として +/// 扱うと呼び出し側が stale 判定に倒れ、`create_new` 直後の空 lock が takeover +/// されて WP-15 で塞いだ同時取得レースがクロックスキュー経由で再発する。 +/// クラウド / コンテナでスキューは珍しくないため、未来 mtime は「たった今作られた」 +/// と解釈するのが安全側 (CodeRabbit PR #307 指摘)。 +fn age_secs_between(modified: std::time::SystemTime, now: std::time::SystemTime) -> i64 { + match now.duration_since(modified) { + Ok(elapsed) => i64::try_from(elapsed.as_secs()).unwrap_or(i64::MAX), + Err(_) => 0, + } +} + +/// ファイル自身の mtime から経過秒を求める (内容が読めない lock の齢判定用)。 +/// +/// `None` は metadata / mtime の取得自体に失敗した場合のみ。 +fn file_age_secs(path: &PathBuf) -> Option { + let modified = std::fs::metadata(path).ok()?.modified().ok()?; + Some(age_secs_between(modified, std::time::SystemTime::now())) +} + +/// parse 不能な lock を「保持者が書き込み中」とみなせるか判定する。 +/// +/// 2 つの状況を内容で区別する: +/// - **空**: `create_new` は成功したが `write_all` がまだ = 保持者が書き込み中。 +/// busy (`Some`) を返して同時取得を防ぐ。 +/// - **非空だが不正**: 本当に壊れた lock。従来どおり `None` を返して takeover させる。 +/// +/// 空の側にも `LOCK_WRITE_WINDOW_SECS` の齢制限を掛ける。create 直後に保持者が +/// crash すると空ファイルが残るが、この制限が無いと以降の取得が永久に阻まれるため。 +/// +/// pid は内容が読めない以上 unknown。 +fn holder_still_writing( + path: &PathBuf, + content: &str, + parse_error: &toml::de::Error, +) -> Option<(LockFile, i64)> { + if !content.trim().is_empty() { + log_info(&format!( + "[lock] 既存 lock の parse 失敗 (内容あり = 破損、stale 扱い): {}", + parse_error + )); + return None; + } + + let age_secs = file_age_secs(path)?; + if age_secs >= LOCK_WRITE_WINDOW_SECS { + log_info(&format!( + "[lock] 空の lock が {}s 以上残存 (create 直後の crash とみなし stale 扱い)", + LOCK_WRITE_WINDOW_SECS + )); + return None; + } + + Some(( + LockFile { + pid: UNKNOWN_HOLDER_PID, + start_time: String::new(), + mode: String::new(), + }, + age_secs, + )) +} + /// 既存 lock が fresh なら `Some((LockFile, age_secs))` を返す。 -/// stale (parse 失敗 / 超過) の場合は `None` (= 取得可)。 +/// stale (超過 / 古くて壊れている) の場合は `None` (= 取得可)。 fn read_fresh_lock(path: &PathBuf, stale_threshold_secs: i64) -> Option<(LockFile, i64)> { let content = std::fs::read_to_string(path).ok()?; let lock: LockFile = match toml::from_str(&content) { Ok(l) => l, - Err(e) => { - log_info(&format!( - "[lock] 既存 lock の parse 失敗 (stale 扱い): {}", - e - )); - return None; - } + Err(e) => return holder_still_writing(path, &content, &e), }; let past_time = PastTime::from_iso8601_now(&lock.start_time)?; let age_secs = past_time.age_secs(); @@ -428,6 +501,55 @@ mod tests { ); } + /// クロックスキュー対策 (CodeRabbit PR #307): mtime が**未来**でも齢 0 とみなすこと。 + /// + /// 旧実装は `duration_since` の `Err` を `None` に潰しており、呼び出し側が + /// 「齢不明 = stale」に倒れて空 lock を takeover していた。つまり WP-15 で + /// 塞いだ同時取得レースが、クロックスキューという別経路から再発しうる。 + #[test] + fn future_mtime_is_treated_as_just_created() { + let now = std::time::SystemTime::now(); + let future = now + std::time::Duration::from_secs(3600); + assert_eq!( + age_secs_between(future, now), + 0, + "未来 mtime は「たった今作られた」と解釈すること (stale 誤判定を防ぐ)", + ); + } + + /// 通常経路 (good): 過去の mtime は経過秒をそのまま返すこと。 + #[test] + fn past_mtime_yields_elapsed_seconds() { + let now = std::time::SystemTime::now(); + let past = now - std::time::Duration::from_secs(42); + assert_eq!(age_secs_between(past, now), 42); + } + + /// WP-15 incident 再現 (bad): `create_new` 直後の**空** lock を stale と誤判定 + /// しないこと。 + /// + /// 由来: 2026-07-20 の Linux 実測 (WSL Ubuntu 24.04)。`create_new` は atomic + /// だが直後のファイルは空で、内容書き込みまでの窓に別スレッドが読むと TOML + /// parse に失敗する。これを「壊れている = stale」と扱っていたため全員が + /// takeover し、8 スレッド中 6 つが同時に Acquired になった。Windows では + /// スケジューリングの差で顕在化していなかっただけで、設計上の欠陥は同じ。 + /// + /// 修正の核心は「parse 不能でも十分新しければ書き込み中 = busy とみなす」。 + #[test] + fn empty_lock_file_is_treated_as_busy_not_stale() { + let tmp = TempDir::new().unwrap(); + let path = tmp.path().join("pr-monitor.lock"); + std::fs::write(&path, "").unwrap(); + + let result = acquire_at(path, "probe", DEFAULT_STALE_THRESHOLD_SECS); + + assert!( + matches!(result, LockResult::Busy { .. }), + "空 lock は「保持者が書き込み中」= Busy とすること。Acquired だと\ + create_new の排他が無意味になり同時取得が起きる (WP-15 の不具合)", + ); + } + #[test] fn lock_format_matches_util_iso8601() { // util::utc_now_iso8601() の出力 format と本 module の parse_iso8601 が diff --git a/src/cli-push-runner/src/stages/diff.rs b/src/cli-push-runner/src/stages/diff.rs index 954cfae9..6ba3b726 100644 --- a/src/cli-push-runner/src/stages/diff.rs +++ b/src/cli-push-runner/src/stages/diff.rs @@ -4,9 +4,9 @@ //! **切り詰めない** (`run_diff_cmd` の doc)。実行は timeout 付き (T6)。 use std::path::Path; -use std::process::{Command, Stdio}; +use std::process::Stdio; -use lib_subprocess::{drain_pipe_unlimited, wait_with_timeout_safe}; +use lib_subprocess::{drain_pipe_unlimited, shell_command, wait_with_timeout_safe}; use crate::config::{DiffConfig, DEFAULT_DIFF_TIMEOUT_SECS}; use crate::log::log_stage; @@ -43,7 +43,8 @@ pub(crate) enum DiffResult { /// child を kill + reap する (`_basic` ではなく `_safe` を選ぶ理由 = ADR-044 層 2)。 /// /// **child を kill した 2 経路 (timeout / wait 失敗) では reader thread を join しない**。 -/// `cmd /c ` の child は cmd.exe で、その孫 (実際の `jj` 等) は kill の対象外である。 +/// `shell_command` の child はシェル (cmd.exe / sh) で、その孫 (実際の `jj` 等) は +/// kill の対象外になり得る (cmd.exe は常に、sh も複合コマンドを fork した場合)。 /// 孫は pipe の書き込み端を継承したまま生き残るため EOF が来ず、join すると孫が自然終了する /// までブロックする = timeout が意味を成さない (T6 が直そうとしているハングの再生産)。 /// 実測: 9s 走るコマンドに 1s の timeout を設定し join すると、制御が戻るまで 9.6s 掛かった。 @@ -51,8 +52,7 @@ pub(crate) enum DiffResult { /// 終了するため thread は道連れになる)。出力も不要 (診断は timeout メッセージ自身が持つ)。 /// 子が自力で終了した経路 (exit 0 / 非 0) は pipe が閉じるため join してよい。 fn run_diff_cmd(cmd: &str, timeout_secs: u64) -> Result { - let mut child = Command::new("cmd") - .args(["/c", cmd]) + let mut child = shell_command(cmd) .stdout(Stdio::piped()) .stderr(Stdio::piped()) .spawn() @@ -148,9 +148,35 @@ pub(crate) fn run_diff(config: &DiffConfig) -> DiffResult { mod tests { use super::*; + /// 100 行を吐くコマンド。cmd.exe と POSIX sh で構文が非互換なため OS 別に + /// 出し分ける (WP-15)。POSIX 側は `seq` 不在の最小環境でも動く while ループ。 + #[cfg(windows)] + const EMIT_100_LINES_CMD: &str = "for /L %i in (1,1,100) do @echo line %i"; + #[cfg(not(windows))] + const EMIT_100_LINES_CMD: &str = "i=1; while [ $i -le 100 ]; do echo line $i; i=$((i+1)); done"; + + /// 何も出力せず正常終了するコマンド (0 バイト出力の検証用)。 + #[cfg(windows)] + const ZERO_BYTE_OUTPUT_CMD: &str = "type nul"; + #[cfg(not(windows))] + const ZERO_BYTE_OUTPUT_CMD: &str = "true"; + + /// stderr へ出力してから非 0 で終わるコマンド (失敗診断の検証用)。 + #[cfg(windows)] + const STDERR_THEN_FAIL_CMD: &str = "echo boom 1>&2& exit /b 1"; + #[cfg(not(windows))] + const STDERR_THEN_FAIL_CMD: &str = "echo boom 1>&2; exit 1"; + + /// stdout と stderr の両方へ出しつつ正常終了するコマンド。 + /// stderr (jj の警告相当) が diff 本体に混ざらない契約の検証用。 + #[cfg(windows)] + const STDOUT_AND_STDERR_CMD: &str = "echo real diff& echo Concurrent modification 1>&2"; + #[cfg(not(windows))] + const STDOUT_AND_STDERR_CMD: &str = "echo real diff; echo Concurrent modification 1>&2"; + #[test] fn run_diff_cmd_captures_more_than_40_lines() { - let result = run_diff_cmd("for /L %i in (1,1,100) do @echo line %i", 30); + let result = run_diff_cmd(EMIT_100_LINES_CMD, 30); assert!(result.is_ok(), "command should succeed"); let output = result.unwrap(); let line_count = output.lines().count(); @@ -166,9 +192,8 @@ mod tests { let out_path = std::env::temp_dir().join("test-run-diff-empty.txt"); let _ = std::fs::remove_file(&out_path); - const ZERO_BYTE_OUTPUT_COMMAND: &str = "type nul"; let config = DiffConfig { - command: ZERO_BYTE_OUTPUT_COMMAND.to_string(), + command: ZERO_BYTE_OUTPUT_CMD.to_string(), output_path: out_path.to_string_lossy().into_owned(), timeout: None, }; @@ -208,7 +233,7 @@ mod tests { #[test] fn capture_diff_snapshot_returns_none_on_failure() { let config = DiffConfig { - command: "echo boom 1>&2& exit /b 1".to_string(), + command: STDERR_THEN_FAIL_CMD.to_string(), output_path: String::new(), timeout: Some(30), }; @@ -237,7 +262,12 @@ mod tests { use std::time::{Duration, Instant}; /// 実行し続けるコマンド (ハングした jj の代役)。timeout が無ければ約 9s 待たされる。 + /// cmd.exe と POSIX sh で構文が非互換なため OS 別に出し分ける (WP-15)。 + /// 所要時間を両 OS で揃えないと片側だけ timeout 経路を検証しない穴になる。 + #[cfg(windows)] const HANGING_COMMAND: &str = "ping 127.0.0.1 -n 10"; + #[cfg(not(windows))] + const HANGING_COMMAND: &str = "sleep 10"; const SHORT_TIMEOUT_SECS: u64 = 1; @@ -334,8 +364,7 @@ mod tests { /// stdout/stderr を結合する) に載せ替えるとこのテストが落ちる。 #[test] fn stderr_is_not_merged_into_the_diff_output() { - let output = run_diff_cmd("echo real diff& echo Concurrent modification 1>&2", 30) - .expect("exit 0 なら Ok"); + let output = run_diff_cmd(STDOUT_AND_STDERR_CMD, 30).expect("exit 0 なら Ok"); assert!(output.contains("real diff"), "stdout は残ること: {:?}", output); assert!( !output.contains("Concurrent modification"), @@ -347,7 +376,7 @@ mod tests { /// 失敗時は stderr を診断として返すこと (従来契約の維持)。 #[test] fn failure_returns_stderr_as_the_diagnostic() { - let err = run_diff_cmd("echo boom 1>&2& exit /b 1", 30).expect_err("exit 1 は Err"); + let err = run_diff_cmd(STDERR_THEN_FAIL_CMD, 30).expect_err("exit 1 は Err"); assert!(err.contains("boom"), "stderr を診断に返すこと: {:?}", err); } } 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 13553dd2..907de607 100644 --- a/src/cli-push-runner/src/stages/lint_screen/classifier.rs +++ b/src/cli-push-runner/src/stages/lint_screen/classifier.rs @@ -1,202 +1,202 @@ -//! cli-finding-classifier.exe を subprocess で起動し、diff を stdin に流して -//! lint-screen JSON を stdout から回収する層。 -//! -//! Pipe orchestration は **drain-first** (stdout/stderr の drain thread を spawn -//! してから stdin へ書く)。順序を逆にすると、子プロセスが stdin 読込前に大量 -//! 出力した際にパイプバッファ (~64KB) 満杯で親子相互ブロックの deadlock になる -//! (PR #231 CodeRabbit Major、`run_cmd_capture` / Safe Subprocess Stdout Pattern -//! と同型)。 - -use std::io::Write; -use std::path::Path; -use std::process::{Command, Stdio}; -use std::thread::JoinHandle; - -use lib_subprocess::wait_with_timeout_basic; - -use super::{InvokeParams, STAGE}; - -#[derive(Debug)] -pub(super) struct ClassifierOutput { - pub(super) stdout: String, - pub(super) stderr: String, -} - -/// classifier exe を lint-screen mode で spawn する (stdin/stdout/stderr は piped)。 -fn spawn_classifier(params: &InvokeParams<'_>) -> Result { - let timeout_str = params.timeout_secs.to_string(); - Command::new(params.exe) - .args([ - "--mode", - "lint-screen", - "--model", - params.model, - "--endpoint", - params.endpoint, - "--timeout-secs", - &timeout_str, - ]) - .stdin(Stdio::piped()) - .stdout(Stdio::piped()) - .stderr(Stdio::piped()) - .spawn() - .map_err(|e| format!("spawn 失敗: {}", e)) -} - -pub(super) fn invoke_classifier( - params: &InvokeParams<'_>, - diff: &str, -) -> Result { - if !Path::new(params.exe).exists() { - return Err(format!("exe 不在 ({})", params.exe)); - } - - let child = spawn_classifier(params)?; - pump_child_io(child, diff, params.timeout_secs) -} - -/// spawn 済みの子プロセスと stdin/stdout/stderr をやり取りする pipe orchestration。 -/// -/// drain thread を **stdin write より前に** spawn する (drain-first)。子が stdin -/// 読込前に stdout/stderr へ大量出力してもパイプが詰まらず、`write_all` が -/// ブロックしない。stdin はスコープ終了時の drop で閉じ、EOF を子へ通知する。 -fn pump_child_io( - mut child: std::process::Child, - stdin_payload: &str, - timeout_secs: u64, -) -> Result { - let stdout_handle = lib_subprocess::drain_pipe_capped( - child.stdout.take().expect("stdout piped"), - crate::runner::MAX_LINES, - ); - let stderr_handle = lib_subprocess::drain_pipe_capped( - child.stderr.take().expect("stderr piped"), - crate::runner::MAX_LINES, - ); - - if let Some(mut stdin) = child.stdin.take() { - if let Err(e) = stdin.write_all(stdin_payload.as_bytes()) { - abort_child(&mut child, stdout_handle, stderr_handle); - return Err(format!("stdin 書き込み失敗: {}", e)); - } - } - - let exit = match wait_with_timeout_basic(STAGE, &mut child, timeout_secs + 5) { - Ok(v) => v, - Err(e) => { - abort_child(&mut child, stdout_handle, stderr_handle); - return Err(format!("wait 失敗: {}", e)); - } - }; - let stdout = stdout_handle.join().unwrap_or_default(); - let stderr = stderr_handle.join().unwrap_or_default(); - - match exit { - None => Err(format!("timeout ({}s)", timeout_secs + 5)), - Some(status) if !status.success() => Err(format!("非 0 終了: {}", stderr)), - Some(_) if stdout.trim().is_empty() => Err("stdout 空".to_string()), - Some(_) => Ok(ClassifierOutput { stdout, stderr }), - } -} - -/// エラー経路で子プロセスと drain thread を回収する (孤児プロセスと detached -/// thread の残留防止)。kill 後は pipe が閉じるため join は速やかに返る。 -fn abort_child( - child: &mut std::process::Child, - stdout_handle: JoinHandle, - stderr_handle: JoinHandle, -) { - let _ = child.kill(); - let _ = child.wait(); - let _ = stdout_handle.join(); - let _ = stderr_handle.join(); -} - -#[cfg(test)] -mod tests { - use super::*; - - #[cfg(windows)] - fn spawn_piped(program: &str, args: &[&str]) -> std::process::Child { - Command::new(program) - .args(args) - .stdin(Stdio::piped()) - .stdout(Stdio::piped()) - .stderr(Stdio::piped()) - .spawn() - .expect("spawn 失敗") - } - - /// `cmd /C more` は stdin を EOF まで読んで stdout へ echo する。 - /// stdin write → EOF 通知 → stdout 回収の正常系を固定化する。 - #[test] - #[cfg(windows)] - fn pump_child_io_roundtrips_stdin_to_stdout() { - let child = spawn_piped("cmd", &["/C", "more"]); - let out = pump_child_io(child, "hello classifier", 30).expect("正常完走すべき"); - assert!( - out.stdout.contains("hello classifier"), - "stdout: {:?}", - out.stdout - ); - } - - /// 非 0 終了は Err(非 0 終了) に分類される。stdin payload は空にして - /// 「子が stdin を読まずに即終了 → broken pipe」の race を排除する。 - #[test] - #[cfg(windows)] - fn pump_child_io_reports_nonzero_exit() { - let child = spawn_piped("cmd", &["/C", "exit 3"]); - let err = pump_child_io(child, "", 30).expect_err("非 0 終了は Err のはず"); - assert!(err.contains("非 0 終了"), "err: {}", err); - } - - /// exit 0 かつ stdout 空は Err(stdout 空) に分類される。 - #[test] - #[cfg(windows)] - fn pump_child_io_reports_empty_stdout() { - let child = spawn_piped("cmd", &["/C", "exit 0"]); - let err = pump_child_io(child, "", 30).expect_err("stdout 空は Err のはず"); - assert!(err.contains("stdout 空"), "err: {}", err); - } - - /// タイムアウト経路 (`None` 分岐) の regression test。 - /// 子プロセスが `timeout_secs + 5` 秒以内に終了しない場合に `Err("timeout (Ns)")` - /// を返すことを確認する。`timeout_secs=1` を渡すと内部で 6s タイムアウトが発火する。 - #[test] - #[cfg(windows)] - #[ignore = "integration: tests timeout behavior (~6s actual wait); run via `cargo test -- --ignored --test-threads=1`"] - fn pump_child_io_reports_timeout_when_child_exceeds_deadline() { - let child = spawn_piped( - "powershell", - &["-NoProfile", "-Command", "Start-Sleep -Seconds 10"], - ); - let err = pump_child_io(child, "", 1).expect_err("タイムアウトは Err のはず"); - assert!(err.contains("timeout"), "err: {}", err); - assert!(err.contains("6s"), "err: {}", err); - } - - /// PR #231 CodeRabbit Major の regression test: 子プロセスが stdin を読む前に - /// パイプバッファ (~64KB) を大きく超える stdout (~256KB) を吐いても deadlock - /// しないこと (drain-first の検証)。修正前の順序 (stdin write → drain spawn) - /// ではこのテストは `write_all` でハングし (実測 78 分継続を確認)、push gate - /// の step_timeout が FAIL として検出する。 - #[test] - #[cfg(windows)] - #[ignore = "integration: spawns real process with ~256KB stdout + ~1MB stdin; run via `cargo test -- --ignored --test-threads=1`"] - fn pump_child_io_survives_child_flooding_stdout_before_reading_stdin() { - let flood_script = "$d = 'x' * 8192; \ - for ($i = 0; $i -lt 32; $i++) { Write-Output $d }; \ - [Console]::In.ReadToEnd() | Out-Null; \ - Write-Output 'DRAINED'"; - let child = spawn_piped("powershell", &["-NoProfile", "-Command", flood_script]); - - let large_diff = "y".repeat(1_000_000); - let out = pump_child_io(child, &large_diff, 60).expect("deadlock せず完走すべき"); - assert!( - out.stdout.contains("DRAINED"), - "子プロセスが stdin を消費し切って完走していること (stdout 末尾): {:?}", - out.stdout.chars().rev().take(60).collect::() - ); - } -} +//! cli-finding-classifier.exe を subprocess で起動し、diff を stdin に流して +//! lint-screen JSON を stdout から回収する層。 +//! +//! Pipe orchestration は **drain-first** (stdout/stderr の drain thread を spawn +//! してから stdin へ書く)。順序を逆にすると、子プロセスが stdin 読込前に大量 +//! 出力した際にパイプバッファ (~64KB) 満杯で親子相互ブロックの deadlock になる +//! (PR #231 CodeRabbit Major、`run_cmd_capture` / Safe Subprocess Stdout Pattern +//! と同型)。 + +use std::io::Write; +use std::path::Path; +use std::process::{Command, Stdio}; +use std::thread::JoinHandle; + +use lib_subprocess::wait_with_timeout_basic; + +use super::{InvokeParams, STAGE}; + +#[derive(Debug)] +pub(super) struct ClassifierOutput { + pub(super) stdout: String, + pub(super) stderr: String, +} + +/// classifier exe を lint-screen mode で spawn する (stdin/stdout/stderr は piped)。 +fn spawn_classifier(params: &InvokeParams<'_>) -> Result { + let timeout_str = params.timeout_secs.to_string(); + Command::new(params.exe) + .args([ + "--mode", + "lint-screen", + "--model", + params.model, + "--endpoint", + params.endpoint, + "--timeout-secs", + &timeout_str, + ]) + .stdin(Stdio::piped()) + .stdout(Stdio::piped()) + .stderr(Stdio::piped()) + .spawn() + .map_err(|e| format!("spawn 失敗: {}", e)) +} + +pub(super) fn invoke_classifier( + params: &InvokeParams<'_>, + diff: &str, +) -> Result { + if !Path::new(params.exe).exists() { + return Err(format!("exe 不在 ({})", params.exe)); + } + + let child = spawn_classifier(params)?; + pump_child_io(child, diff, params.timeout_secs) +} + +/// spawn 済みの子プロセスと stdin/stdout/stderr をやり取りする pipe orchestration。 +/// +/// drain thread を **stdin write より前に** spawn する (drain-first)。子が stdin +/// 読込前に stdout/stderr へ大量出力してもパイプが詰まらず、`write_all` が +/// ブロックしない。stdin はスコープ終了時の drop で閉じ、EOF を子へ通知する。 +fn pump_child_io( + mut child: std::process::Child, + stdin_payload: &str, + timeout_secs: u64, +) -> Result { + let stdout_handle = lib_subprocess::drain_pipe_capped( + child.stdout.take().expect("stdout piped"), + crate::runner::MAX_LINES, + ); + let stderr_handle = lib_subprocess::drain_pipe_capped( + child.stderr.take().expect("stderr piped"), + crate::runner::MAX_LINES, + ); + + if let Some(mut stdin) = child.stdin.take() { + if let Err(e) = stdin.write_all(stdin_payload.as_bytes()) { + abort_child(&mut child, stdout_handle, stderr_handle); + return Err(format!("stdin 書き込み失敗: {}", e)); + } + } + + let exit = match wait_with_timeout_basic(STAGE, &mut child, timeout_secs + 5) { + Ok(v) => v, + Err(e) => { + abort_child(&mut child, stdout_handle, stderr_handle); + return Err(format!("wait 失敗: {}", e)); + } + }; + let stdout = stdout_handle.join().unwrap_or_default(); + let stderr = stderr_handle.join().unwrap_or_default(); + + match exit { + None => Err(format!("timeout ({}s)", timeout_secs + 5)), + Some(status) if !status.success() => Err(format!("非 0 終了: {}", stderr)), + Some(_) if stdout.trim().is_empty() => Err("stdout 空".to_string()), + Some(_) => Ok(ClassifierOutput { stdout, stderr }), + } +} + +/// エラー経路で子プロセスと drain thread を回収する (孤児プロセスと detached +/// thread の残留防止)。kill 後は pipe が閉じるため join は速やかに返る。 +fn abort_child( + child: &mut std::process::Child, + stdout_handle: JoinHandle, + stderr_handle: JoinHandle, +) { + let _ = child.kill(); + let _ = child.wait(); + let _ = stdout_handle.join(); + let _ = stderr_handle.join(); +} + +/// `pump_child_io` の挙動テスト群。 +/// +/// 全ケースが 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) で扱う。 +#[cfg(all(test, windows))] +mod tests { + use super::*; + + fn spawn_piped(program: &str, args: &[&str]) -> std::process::Child { + Command::new(program) + .args(args) + .stdin(Stdio::piped()) + .stdout(Stdio::piped()) + .stderr(Stdio::piped()) + .spawn() + .expect("spawn 失敗") + } + + /// `cmd /C more` は stdin を EOF まで読んで stdout へ echo する。 + /// stdin write → EOF 通知 → stdout 回収の正常系を固定化する。 + #[test] + fn pump_child_io_roundtrips_stdin_to_stdout() { + let child = spawn_piped("cmd", &["/C", "more"]); + let out = pump_child_io(child, "hello classifier", 30).expect("正常完走すべき"); + assert!( + out.stdout.contains("hello classifier"), + "stdout: {:?}", + out.stdout + ); + } + + /// 非 0 終了は Err(非 0 終了) に分類される。stdin payload は空にして + /// 「子が stdin を読まずに即終了 → broken pipe」の race を排除する。 + #[test] + fn pump_child_io_reports_nonzero_exit() { + let child = spawn_piped("cmd", &["/C", "exit 3"]); + let err = pump_child_io(child, "", 30).expect_err("非 0 終了は Err のはず"); + assert!(err.contains("非 0 終了"), "err: {}", err); + } + + /// exit 0 かつ stdout 空は Err(stdout 空) に分類される。 + #[test] + fn pump_child_io_reports_empty_stdout() { + let child = spawn_piped("cmd", &["/C", "exit 0"]); + let err = pump_child_io(child, "", 30).expect_err("stdout 空は Err のはず"); + assert!(err.contains("stdout 空"), "err: {}", err); + } + + /// タイムアウト経路 (`None` 分岐) の regression test。 + /// 子プロセスが `timeout_secs + 5` 秒以内に終了しない場合に `Err("timeout (Ns)")` + /// を返すことを確認する。`timeout_secs=1` を渡すと内部で 6s タイムアウトが発火する。 + #[test] + #[ignore = "integration: tests timeout behavior (~6s actual wait); run via `cargo test -- --ignored --test-threads=1`"] + fn pump_child_io_reports_timeout_when_child_exceeds_deadline() { + let child = spawn_piped( + "powershell", + &["-NoProfile", "-Command", "Start-Sleep -Seconds 10"], + ); + let err = pump_child_io(child, "", 1).expect_err("タイムアウトは Err のはず"); + assert!(err.contains("timeout"), "err: {}", err); + assert!(err.contains("6s"), "err: {}", err); + } + + /// PR #231 CodeRabbit Major の regression test: 子プロセスが stdin を読む前に + /// パイプバッファ (~64KB) を大きく超える stdout (~256KB) を吐いても deadlock + /// しないこと (drain-first の検証)。修正前の順序 (stdin write → drain spawn) + /// ではこのテストは `write_all` でハングし (実測 78 分継続を確認)、push gate + /// の step_timeout が FAIL として検出する。 + #[test] + #[ignore = "integration: spawns real process with ~256KB stdout + ~1MB stdin; run via `cargo test -- --ignored --test-threads=1`"] + fn pump_child_io_survives_child_flooding_stdout_before_reading_stdin() { + let flood_script = "$d = 'x' * 8192; \ + for ($i = 0; $i -lt 32; $i++) { Write-Output $d }; \ + [Console]::In.ReadToEnd() | Out-Null; \ + Write-Output 'DRAINED'"; + let child = spawn_piped("powershell", &["-NoProfile", "-Command", flood_script]); + + let large_diff = "y".repeat(1_000_000); + let out = pump_child_io(child, &large_diff, 60).expect("deadlock せず完走すべき"); + assert!( + out.stdout.contains("DRAINED"), + "子プロセスが stdin を消費し切って完走していること (stdout 末尾): {:?}", + out.stdout.chars().rev().take(60).collect::() + ); + } +} diff --git a/src/cli-push-runner/src/stages/push.rs b/src/cli-push-runner/src/stages/push.rs index 97c5c63d..997a8138 100644 --- a/src/cli-push-runner/src/stages/push.rs +++ b/src/cli-push-runner/src/stages/push.rs @@ -271,12 +271,24 @@ mod tests { use super::*; /// 40 行の正常出力の後に拒否行が来る = 拒否行が cap の外に落ちる状況の再現。 + /// cmd.exe と POSIX sh で構文が非互換なため OS 別に出し分ける (WP-15)。 + /// **出力行数と拒否行の文面は両 OS で揃えること** — ズレると片側だけ + /// 「cap の外の拒否行を検知する」という本 mod の主題を検証しなくなる。 + #[cfg(windows)] const REFUSAL_BEYOND_CAP: &str = "(for /L %i in (1,1,40) do @echo Changes to push to origin) \ & echo Warning: Refusing to create new remote bookmark feat/x@origin"; + #[cfg(not(windows))] + const REFUSAL_BEYOND_CAP: &str = + "i=1; while [ $i -le 40 ]; do echo Changes to push to origin; i=$((i+1)); done; \ + echo Warning: Refusing to create new remote bookmark feat/x@origin"; /// 40 行を超える正常な push 出力 (拒否なし)。 + #[cfg(windows)] const SUCCESS_BEYOND_CAP: &str = "(for /L %i in (1,1,50) do @echo Add bookmark feat/x to 3000737e)"; + #[cfg(not(windows))] + const SUCCESS_BEYOND_CAP: &str = + "i=1; while [ $i -le 50 ]; do echo Add bookmark feat/x to 3000737e; i=$((i+1)); done"; /// incident 再現 (bad): cap の外にある拒否行を検知できること。 /// jj は拒否時も exit 0 を返すため、この検知が唯一の防波堤になる。 diff --git a/src/cli-push-runner/src/stages/quality_gate.rs b/src/cli-push-runner/src/stages/quality_gate.rs index 8fcb8298..448e7019 100644 --- a/src/cli-push-runner/src/stages/quality_gate.rs +++ b/src/cli-push-runner/src/stages/quality_gate.rs @@ -297,10 +297,16 @@ mod tests { use super::*; use crate::runner::MAX_LINES; - /// 40 行を超える出力を出してから失敗する step の再現。`cmd /c "A & exit 1"` は - /// 最後のコマンド (`exit 1`) の exit code を返すため失敗として報告される。 + /// 40 行を超える出力を出してから失敗する step の再現。cmd.exe の `A & exit 1` も + /// POSIX sh の `A; exit 1` も最後のコマンドの exit code を返すため失敗として + /// 報告される。構文が非互換なため OS 別に出し分ける (WP-15)。 + /// **出力行数 (60) は両 OS で揃えること** — cap (40 行) を超えることが前提の test。 + #[cfg(windows)] const FAIL_BEYOND_CAP: &str = "(for /L %i in (1,1,60) do @echo failing test line %i) & exit 1"; + #[cfg(not(windows))] + const FAIL_BEYOND_CAP: &str = + "i=1; while [ $i -le 60 ]; do echo failing test line $i; i=$((i+1)); done; exit 1"; /// incident 再現 (bad): 失敗 step の出力が cap (40 行) を超えても全量取得でき、 /// cap の外にある診断行 (60 行目) が残ること。 diff --git a/src/hooks-stop-quality/src/main.rs b/src/hooks-stop-quality/src/main.rs index 497c8985..c025670d 100644 --- a/src/hooks-stop-quality/src/main.rs +++ b/src/hooks-stop-quality/src/main.rs @@ -309,6 +309,35 @@ fn warn_no_steps_configured(config_found: bool) { } } +/// step の `cmd` 中のプレースホルダーを実行環境に合わせて展開する (WP-15)。 +/// +/// 展開対象: +/// - `{{CLAUDE_DIR}}` → `.claude/` の**絶対パス** (forward-slash 正規化) +/// - `{{EXE_SUFFIX}}` → `.exe` (Windows) / 空文字 (それ以外) +/// +/// **なぜ絶対パス + forward-slash か**: step は cmd.exe (Windows) と sh (Linux) の +/// 双方で解釈される。cmd.exe は forward-slash の**相対**パス (`.claude/foo.exe`) を +/// コマンドとして解決できず、sh は backslash (`.\.claude\foo.exe`) を解釈できない。 +/// 実測の結果、両者が共通で通るのは **forward-slash の絶対パス**だけだった +/// (ADR-005 の settings.local.json で確認済みの性質と同じ)。 +/// +/// 解決不能時はプレースホルダーを残したまま返す。展開済みの壊れたパスで走らせるより、 +/// `{{CLAUDE_DIR}}` を含むエラーメッセージで失敗させたほうが原因が自明になるため。 +fn expand_step_placeholders(cmd: &str) -> String { + let expanded = cmd.replace("{{EXE_SUFFIX}}", std::env::consts::EXE_SUFFIX); + if !expanded.contains("{{CLAUDE_DIR}}") { + return expanded; + } + let Some(claude_dir) = lib_jj_helpers::pipeline_lock::exe_claude_dir() else { + eprintln!( + "[stop-quality] Warning: .claude ディレクトリを解決できず {{{{CLAUDE_DIR}}}} を展開できません" + ); + return expanded; + }; + let normalized = claude_dir.to_string_lossy().replace('\\', "/"); + expanded.replace("{{CLAUDE_DIR}}", &normalized) +} + /// 各ステップを並列に実行し、失敗を step 定義順で集約する (WP-05)。 /// /// 逐次実行では合計時間が全ステップの和になり Stop hook が肥大化していた @@ -325,7 +354,8 @@ fn run_quality_steps(steps: &[QualityStepConfig], timeout: u64) -> Vec { .map(|step| { let step_name = step.name.clone(); let handle = std::thread::spawn(move || { - run_cmd_shell_capped(&step.name, &step.cmd, timeout, MAX_LINES) + let cmd = expand_step_placeholders(&step.cmd); + run_cmd_shell_capped(&step.name, &cmd, timeout, MAX_LINES) }); (step_name, handle) }) @@ -365,6 +395,48 @@ fn block_on_failures(failures: &[String]) { mod tests { use super::*; + /// WP-15: プレースホルダーを持たない既存 step は素通しすること (退行なし)。 + #[test] + fn expand_step_placeholders_leaves_plain_commands_untouched() { + assert_eq!(expand_step_placeholders("pnpm test"), "pnpm test"); + } + + /// `{{EXE_SUFFIX}}` が実行 OS の拡張子に展開されること。 + #[test] + fn expand_step_placeholders_substitutes_exe_suffix_for_the_host_os() { + let expanded = expand_step_placeholders("tool{{EXE_SUFFIX}} --flag"); + assert_eq!( + expanded, + format!("tool{} --flag", std::env::consts::EXE_SUFFIX), + ); + assert!( + !expanded.contains("{{EXE_SUFFIX}}"), + "プレースホルダーが残っている: {:?}", + expanded, + ); + } + + /// `{{CLAUDE_DIR}}` は **forward-slash の絶対パス**に展開されること。 + /// backslash が残ると sh 側で、相対パスになると cmd.exe 側で解決に失敗する + /// (両者が共通で通るのは forward-slash 絶対パスのみ = 実測)。 + #[test] + fn expand_step_placeholders_yields_a_forward_slash_absolute_claude_dir() { + let Some(claude_dir) = lib_jj_helpers::pipeline_lock::exe_claude_dir() else { + return; + }; + let expanded = expand_step_placeholders("{{CLAUDE_DIR}}/tool{{EXE_SUFFIX}}"); + assert!( + !expanded.contains('\\'), + "backslash が残ると sh で解決できない: {:?}", + expanded, + ); + assert!( + expanded.starts_with(&claude_dir.to_string_lossy().replace('\\', "/")), + "絶対パスに展開されること (相対だと cmd.exe が解決できない): {:?}", + expanded, + ); + } + #[test] fn default_config_has_no_steps() { let config = Config::default(); diff --git a/src/lib-subprocess/src/lib.rs b/src/lib-subprocess/src/lib.rs index 9caa831a..bf5323f5 100644 --- a/src/lib-subprocess/src/lib.rs +++ b/src/lib-subprocess/src/lib.rs @@ -238,6 +238,35 @@ fn kill_and_join_err( (false, error) } +/// コマンド文字列を OS のシェル経由で実行する `Command` を組み立てる (WP-15)。 +/// +/// Windows は `cmd /c `、それ以外 (Linux / macOS) は `sh -c `。 +/// **リポジトリ内で「シェル経由の実行」を行う唯一の spawn 起点**であり、新たに +/// `Command::new("cmd")` を書き足してはならない (Linux 移植で無言に壊れるため)。 +/// +/// `sh` を選ぶ理由: quality gate / push / merge の step は `pnpm test` や +/// `cargo clippy ... -- -D warnings` のような単純なコマンド行であり、bash 固有構文 +/// (配列 / `[[ ]]` / process substitution) を必要としない。POSIX sh に限定しておけば +/// bash 不在の最小コンテナ (クラウドの使い捨て環境) でも同じ経路が通る。 +/// +/// **cmd.exe と sh でシェル構文は互換ではない**点に注意 (`%VAR%` vs `$VAR`、 +/// パス区切り、`&` の意味)。config に書く step の `cmd` は両者で解釈が一致する +/// 書き方 (プレーンなコマンド + `/` 区切り相対パス) に揃えること。 +pub fn shell_command(cmd: &str) -> Command { + #[cfg(windows)] + { + let mut command = Command::new("cmd"); + command.args(["/c", cmd]); + command + } + #[cfg(not(windows))] + { + let mut command = Command::new("sh"); + command.args(["-c", cmd]); + command + } +} + /// `run_cmd_shell_*` 3 variant の共通骨格 (spawn → drain → wait → combine)。 /// /// variant 間の差は `drain` (= どの `drain_pipe_*` を使うか) だけで、それ以外の @@ -254,8 +283,7 @@ fn run_cmd_shell_with(label: &str, cmd: &str, timeout_secs: u64, drain: F) -> where F: Fn(Box) -> JoinHandle, { - let mut child = match Command::new("cmd") - .args(["/c", cmd]) + let mut child = match shell_command(cmd) .stdout(Stdio::piped()) .stderr(Stdio::piped()) .spawn() @@ -288,7 +316,7 @@ where } } -/// `cmd /c ` で shell コマンドを実行し timeout 付きで結果を返す **silent capped** variant。 +/// `shell_command` 経由で shell コマンドを実行し timeout 付きで結果を返す **silent capped** variant。 /// /// 戻り値: `(success, combined_output)`。 /// - 起動失敗 / try_wait 失敗 → `(false, error_message)` @@ -312,7 +340,7 @@ pub fn run_cmd_shell_capped( }) } -/// `cmd /c ` で shell コマンドを実行し timeout 付きで結果を返す **reporting capped** variant。 +/// `shell_command` 経由で shell コマンドを実行し timeout 付きで結果を返す **reporting capped** variant。 /// /// `run_cmd_shell_capped` と同 signature だが内部で `drain_pipe_capped_reporting(max_lines)` /// を使用、stdout / stderr 超過時は末尾に `"... (N lines truncated)"` が追加される。 @@ -329,7 +357,7 @@ pub fn run_cmd_shell_capped_reporting( }) } -/// `cmd /c ` で shell コマンドを実行し timeout 付きで結果を返す **unlimited** variant。 +/// `shell_command` 経由で shell コマンドを実行し timeout 付きで結果を返す **unlimited** variant。 /// /// `run_cmd_shell_capped` から `max_lines` を除いたもので、内部で `drain_pipe_unlimited` /// を使用するため出力は truncate されない。**出力を control flow 判定に使う callsite** @@ -371,24 +399,40 @@ mod tests { assert_eq!(combine_output("out\n", "err"), "out\nerr"); } - use std::process::{Command, Stdio}; + use std::process::Stdio; + + /// timeout 経路を踏ませるための「十分に長く走る」コマンド (両 OS で約 10 秒、WP-15)。 + /// + /// `exit 0` / `exit 1` / `echo hello` は cmd.exe と POSIX sh の双方で同じ意味に + /// 解釈されるためテスト内に直接書けるが、「長時間走る」「N 行吐く」は構文が非互換 + /// なので cfg で出し分ける。**振る舞い (所要時間 / 出力行数) は両 OS で揃えること** — + /// ここがズレると片方の OS だけ timeout / truncation を検証しない抜け穴になる。 + #[cfg(windows)] + const LONG_RUNNING_CMD: &str = "ping 127.0.0.1 -n 10"; + #[cfg(not(windows))] + const LONG_RUNNING_CMD: &str = "sleep 10"; + + /// capped variant の cap (40 行) を超える 60 行を吐くコマンド。 + /// POSIX 側は `seq` が無い最小環境でも動くよう while ループで書く。 + #[cfg(windows)] + const EMIT_60_LINES_CMD: &str = "(for /L %i in (1,1,60) do @echo line %i)"; + #[cfg(not(windows))] + const EMIT_60_LINES_CMD: &str = "i=1; while [ $i -le 60 ]; do echo line $i; i=$((i+1)); done"; fn spawn_quick_exit() -> std::process::Child { - Command::new("cmd") - .args(["/c", "exit 0"]) + shell_command("exit 0") .stdout(Stdio::null()) .stderr(Stdio::null()) .spawn() - .expect("failed to spawn quick-exit cmd") + .expect("failed to spawn quick-exit command") } fn spawn_long_running() -> std::process::Child { - Command::new("cmd") - .args(["/c", "ping 127.0.0.1 -n 10"]) + shell_command(LONG_RUNNING_CMD) .stdout(Stdio::null()) .stderr(Stdio::null()) .spawn() - .expect("failed to spawn long-running cmd") + .expect("failed to spawn long-running command") } #[test] @@ -557,7 +601,7 @@ mod tests { #[test] fn run_cmd_shell_capped_reports_timeout_with_message() { - let (ok, output) = run_cmd_shell_capped("test", "ping 127.0.0.1 -n 10", 1, 40); + let (ok, output) = run_cmd_shell_capped("test", LONG_RUNNING_CMD, 1, 40); assert!(!ok, "timeout should report failure"); assert!( output.starts_with("timed out after 1s"), @@ -574,8 +618,7 @@ mod tests { #[test] fn run_cmd_shell_capped_reporting_reports_timeout_with_message() { - let (ok, output) = - run_cmd_shell_capped_reporting("test", "ping 127.0.0.1 -n 10", 1, 40); + let (ok, output) = run_cmd_shell_capped_reporting("test", LONG_RUNNING_CMD, 1, 40); assert!(!ok, "timeout should report failure"); assert!( output.starts_with("timed out after 1s"), @@ -600,7 +643,7 @@ mod tests { /// 全行が戻り値に残ること (= control flow 判定に使える)。 #[test] fn run_cmd_shell_unlimited_preserves_output_beyond_the_capped_variant_cap() { - let cmd = "(for /L %i in (1,1,60) do @echo line %i)"; + let cmd = EMIT_60_LINES_CMD; let (ok, output) = run_cmd_shell_unlimited("test", cmd, 30); assert!(ok, "command should succeed: {:?}", output); assert_eq!( @@ -613,7 +656,7 @@ mod tests { #[test] fn run_cmd_shell_unlimited_reports_timeout_with_message() { - let (ok, output) = run_cmd_shell_unlimited("test", "ping 127.0.0.1 -n 10", 1); + let (ok, output) = run_cmd_shell_unlimited("test", LONG_RUNNING_CMD, 1); assert!(!ok, "timeout should report failure"); assert!( output.starts_with("timed out after 1s"),