ci: Windows/Linux の 2 OS matrix を新設し hooks smoke test を追加 (ADR-065) - #342
Merged
Conversation
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughUbuntu と Windows の CI matrix を追加し、jj の固定版検証、Rust テスト、hooks smoke test を実行します。ADR-065 と関連文書を更新し、hooks の実行結果を一時環境で検証します。 ChangesCI matrix と hooks 検証
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant GitHubActions
participant jj_0_42_0
participant Cargo
participant HooksSmokeTest
GitHubActions->>jj_0_42_0: OS 別アーカイブを取得してバージョンを検証
GitHubActions->>Cargo: clippy と workspace test を実行
GitHubActions->>HooksSmokeTest: hooks smoke test と ignored test を実行
HooksSmokeTest-->>GitHubActions: exit code とテスト結果を返す
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Contributor
🤖 PR Monitor 分析 (GitHub Actions バックストップ)
Applicable Findings (Critical / High / Major)該当なし (レビュー未着のため指摘自体が存在しない) Applicable Findings (Medium 以下)該当なし Filtered (not applicable)該当なし 次のアクション
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/hooks-stop-quality/src/main.rs`:
- Line 623: Remove the Windows-only cfg attribute from the test around the
run_quality_steps parallel aggregation check so it runs on all supported
platforms. Preserve the existing run_cmd_shell_capped commands using only
POSIX-compatible exit 0 and exit 1 behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 59802111-1837-4f44-b0c0-8802dfe1925e
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (15)
.github/workflows/ci.yml.github/workflows/release-binaries.ymlCLAUDE.mddocs/adr/adr-049-incident-eval-regression-suite.mddocs/adr/adr-063-linux-portability-release-binaries.mddocs/adr/adr-065-ci-matrix-cross-os-regression.mddocs/dev-conventions.mddocs/harness-improvement-plan.mddocs/todo-summary2.mddocs/todo14.mdsrc/cli-push-runner/src/stages/lint_screen/classifier.rssrc/hooks-pre-tool-validate/Cargo.tomlsrc/hooks-pre-tool-validate/tests/smoke.rssrc/hooks-stop-quality/src/main.rssrc/hooks-stop-quality/tests/t7_cwd_independence.rs
| /// WP-05 並列化: 複数ステップを並列実行しても、失敗が step 定義順で集約され、 | ||
| /// 成功ステップは failure に含まれないこと。`run_cmd_shell_capped` は `cmd /c` 依存 | ||
| /// のため Windows でのみ実行する (WP-16 CI matrix の非 Windows leg では skip)。 | ||
| /// のため Windows でのみ実行する (ADR-065 CI matrix の非 Windows leg では skip)。 |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# run_quality_steps と run_cmd_shell_capped の呼び出し経路を確認する。
rg -n -C 8 'run_quality_steps|run_cmd_shell_capped|cfg\(windows\)' \
src/hooks-stop-quality/srcRepository: aloekun/claude-code-hook-test
Length of output: 8554
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "## locate lib_subprocess"
rg -n 'mod lib_subprocess|lib_subprocess|run_cmd_shell_capped|run_cmd_shell' . --glob '!target/**' --glob '!node_modules/**'
echo
echo "## main.rs relevant definitions"
sed -n '36,42p;248,252p;380,408p;615,678p' src/hooks-stop-quality/src/main.rs
echo
echo "## ADR-063 relevant text"
rg -n -C 6 'ADR-063|run_cmd_shell|cmd /c|sh -c' . --glob '*.md' --glob '!target/**' || trueRepository: aloekun/claude-code-hook-test
Length of output: 50386
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "## src/lib-subprocess/src/lib.rs relevant implementation"
sed -n '260,380p' src/lib-subprocess/src/lib.rs
echo
echo "## ADR-063 full content"
sed -n '1,80p' docs/adr/adr-063-linux-portability-release-binaries.md
echo
echo "## ADR-065 full content"
sed -n '1,120p' docs/adr/adr-065-ci-matrix-cross-os-regression.mdRepository: aloekun/claude-code-hook-test
Length of output: 13597
Windows 限定の cfg を外してください。
run_quality_steps は run_cmd_shell_capped を経由して sh -c / cmd /c を OS 分岐で実行します。このテストは POSIX でも通る exit 0 / exit 1 のみを使うため、cfg(windows) のままだと Linux leg で並列集約判定が常に skip されます。cfg(windows) を外してください。
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/hooks-stop-quality/src/main.rs` at line 623, Remove the Windows-only cfg
attribute from the test around the run_quality_steps parallel aggregation check
so it runs on all supported platforms. Preserve the existing
run_cmd_shell_capped commands using only POSIX-compatible exit 0 and exit 1
behavior.
Contributor
🤖 PR Monitor 分析 (GitHub Actions バックストップ)
Applicable Findings (Critical / High / Major)該当なし Applicable Findings (Medium 以下)
Filtered (not applicable)該当なし 次のアクション
|
aloekun
added a commit
that referenced
this pull request
Aug 1, 2026
…abbit 指摘対応) run_quality_steps_parallel_collects_failures_in_step_order の #[cfg(windows)] は、 ADR-063 で run_cmd_shell_capped が shell_command (cmd /c ↔ sh -c) へ抽象化される 以前の名残だった。step は exit 0 / exit 1 のみで両シェルで同義のため、ガードを外して 両 OS で実行する。実 Linux (WSL Ubuntu 24.04) で 1 passed、Windows でも 1 passed を実測。 t7_cwd_independence の Windows 限定理由も同じ stale な記述 (「cmd /c 依存」) だったため、 実態 (再現対象の incident が .\.claude\probe.cmd という cmd.exe 固有のルート相対パス 解決そのもの) に修正。こちらはガード自体が正当なので維持する。 ADR-065 に副次発見として記録: ローカル post-PR レビュー層は本指摘を、まさにこの stale な コメントの主張を根拠に false positive と誤判定していた。コメントが実装から乖離すると レビュー層はその乖離を増幅する。 併せて ubuntu leg 相当を WSL で先取り実行し、clippy -D warnings clean / cargo test --workspace 1,707 pass (Windows 1,881 との差 174 = cfg(windows) 群) を ADR-065 の検証記録に追記した。
This was referenced Aug 1, 2026
aloekun
added a commit
that referenced
this pull request
Aug 1, 2026
…修正 (#344) PR #342 CI (windows-latest, 2 コア runner) で concurrent_takeover_with_orphaned_sentinel_single_winner が「得た数: 2」で失敗した 根本原因の修正。出遅れスレッドが除去前に読んだ stale sentinel content を握ったまま reclaim に入ると、勝者が finish_reclaim 完了時に reclaim marker を除去した後で content 由来の同一 gate path を再作成でき、勝者の fresh sentinel を無条件除去して 2 本目の takeover 実行権を得ていた (勝者の rename 前に lock を読めば双方 Acquired)。 2 層で塞ぐ: - finish_reclaim: sentinel が gate を正当化した stale content のままかを検証してから 除去する。不一致 / 消失 / 読取失敗は一切触れず Busy (検証済み: 修正前に決定論 テストが red、修正後 green)。 - reclaim marker を takeover 完了まで保持する (SentinelGate::Acquired が marker path を 運び、takeover_stale_lock が sentinel と共に除去)。takeover 実行中の同一 gate 再作成を構造的に排除。 検証: 決定論再現テスト (red→green) + marker 残留 leak guard + 高競合 stress (32 threads x 400 rounds、opt-in ignored) + 既存 concurrency guard 全 pass + workspace 全 test pass + clippy -D warnings clean。 verify-concurrency-by-observation (計測で確認、推論に頼らない) に従い、 部分書き込み仮説は 91.8 万回読取の実測 (partial=0) で棄却してから本経路を特定した。
ハーネス改善計画 WP-16。従来の CI は release-binaries.yml のみで Linux only / master push only / --ignored 非対象だったため、片 OS でしか出ない欠陥 (Linux の lock 同時取得レース、Windows の cmd.exe クォート除去) が PR 段階で 止まらなかった。 - .github/workflows/ci.yml: windows-latest + ubuntu-latest の 2 leg (pull_request / master push / workflow_dispatch、fail-fast: false)。各 leg で jj 0.42.0 導入 (版一致を fail-closed 検証) + clippy + cargo test + hooks smoke test + --ignored --test-threads=1。ローカル push-runner と同一コマンド。 - src/hooks-pre-tool-validate/tests/smoke.rs: 唯一 block する hook に exe-spawn テストが無かったため新設 (block/pass 対 + 不正 stdin が block に倒れないこと)。 exe と deploy 済 config を temp dir へ staging し、target/debug の残置 config に verdict が左右される環境依存を排除した。 - ADR-065 起票。各永続成果物の WP-16 参照を ADR-065 へ付け替え。 required check 化と Linux 側 cfg(windows) テストの POSIX 版追加は ADR-065 の 残課題として追跡する。
…abbit 指摘対応) run_quality_steps_parallel_collects_failures_in_step_order の #[cfg(windows)] は、 ADR-063 で run_cmd_shell_capped が shell_command (cmd /c ↔ sh -c) へ抽象化される 以前の名残だった。step は exit 0 / exit 1 のみで両シェルで同義のため、ガードを外して 両 OS で実行する。実 Linux (WSL Ubuntu 24.04) で 1 passed、Windows でも 1 passed を実測。 t7_cwd_independence の Windows 限定理由も同じ stale な記述 (「cmd /c 依存」) だったため、 実態 (再現対象の incident が .\.claude\probe.cmd という cmd.exe 固有のルート相対パス 解決そのもの) に修正。こちらはガード自体が正当なので維持する。 ADR-065 に副次発見として記録: ローカル post-PR レビュー層は本指摘を、まさにこの stale な コメントの主張を根拠に false positive と誤判定していた。コメントが実装から乖離すると レビュー層はその乖離を増幅する。 併せて ubuntu leg 相当を WSL で先取り実行し、clippy -D warnings clean / cargo test --workspace 1,707 pass (Windows 1,881 との差 174 = cfg(windows) 群) を ADR-065 の検証記録に追記した。
pre-push review が「release-binaries.yml は paths: でフィルタしているのに ci.yml は していない非対称を ADR-065 が説明していない」と指摘 (非ブロッキング)。 指摘のとおり説明が欠落していたが、対応は「フィルタを追加」ではなく「非対称の理由を 明記」が正しい。paths: で skip された check を Required status checks に指定すると GitHub はそれを success ではなく pending として扱い、PR が永久にマージ不能になる。 ci.yml は required check にする予定 (ADR-065 § 決定 5) なので paths: を使えず、 publish 用で required にならない release-binaries.yml とは正しい選択が逆になる。 将来 run 量を削る場合も paths: ではなく early-success (job は必ず起動し中身を条件分岐) を使う旨を併記した。ADR-065 に § 決定 6 として追加し、既存の § 決定 6 は § 決定 7 へ繰り下げ。 ci.yml 側の古い節番号参照 (§ 決定 4 → § 決定 7) も修正。
aloekun
force-pushed
the
ci/two-os-matrix
branch
from
August 1, 2026 18:23
616cf4a to
d60ee39
Compare
This was referenced Aug 2, 2026
aloekun
added a commit
that referenced
this pull request
Aug 2, 2026
…3) (#346) PR #342/#343/#344 の post-merge feedback 採用候補 14 件のうち、2026-08-02 の 方針決定 (文書系 10 件 = 1 docs バッチ PR、実装系 4 件 = A/B 系統別 PR) に従い 5 エントリとして登録: - 順位 359: 文書系 10 件の docs バッチ (dev-conventions 集中、Severity High 2 件含む) - 順位 360: cargo test コマンド等価性検証テスト (#342/#343 で連続指摘 = Frequency High) - 順位 361: JJ_VERSION の 2 ファイル一致検証テスト (順位 360 と同一 PR 可) - 順位 362: git subprocess ブランチ名依存引数の custom lint rule (#343 incident 由来) - 順位 363: check-ci-coderabbit の detached HEAD 回帰統合テスト (順位 362 と同一 PR 可)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
.github/workflows/ci.ymlを新設し、windows-latest+ubuntu-latestの 2 leg matrix を PR ゲートとして追加 (pull_request/ master push /workflow_dispatch、fail-fast: false)-D warnings/cargo test --workspace/--ignored --test-threads=1) を実行。--ignoredのため jj 0.42.0 を導入し、版一致を fail-closed で検証するsrc/hooks-pre-tool-validate/tests/smoke.rsを新設。リポジトリで唯一 block する hook の exe-spawn テストが無かったため、block/pass を経路ごとに対で固定するtarget/debugの残置 config で verdict が変わる環境依存 (ローカルと fresh clone の CI で挙動が食い違う) を排除したContext
Why: 従来の CI は
release-binaries.ymlの 1 job のみで、(1) Linux only、(2) master push only (PR では何も走らない)、(3)--ignored統合テストが対象外、という 3 つの穴があった。一方で「片 OS でしか検出できない欠陥」は両方向で実在が確認されている — Linux でしか出ないcli-pr-monitorの lock 同時取得レース (ADR-063 の副次発見) と、Windows でしか出ない jj revset のシェル引数クォート除去。Trigger: ハーネス改善計画 WP-16。前提となる Linux 可搬性レイヤは ADR-063 (WP-15) で完了済み。
Scope decision:
pump_child_ioの deadlock 保護 /run_cmd_captureの stdout/stderr 分離が無検証」は本 PR に含めない。matrix はこれらを Windows leg で CI 実行対象にするが (従来の Linux only CI では一度も走っていなかった)、Linux 側の同等検証には POSIX 版テストの追加が必要。ci.yml 自体が GitHub Actions 上で未実走のため、同じ PR で 2 つの未知を混ぜると赤が出たときに「移植で発掘した実バグ / 移植テストの書き損じ / CI 設定の不備」が切り分けられない。ADR-065 の残課題として追跡し別 PR で実施する。未対応の非ブロッキング指摘: pre-push review が
smoke.rsのmalformed_stdin_does_not_blockについて「code != 2しか assert せず、doc コメントが述べる exit 1 を固定していない」と指摘 (非ブロッキング、両レビュアー APPROVE)。契約としては「block しない」が本質のため本 PR では据え置き。Validation
pnpm pushpre-push review: verdict=APPROVE (simplicity-review / security-review 双方)pr_size_check: 607 行 (warning 800 内)quality_gate全 4 グループ PASS (lint / test / build / rust-lint-test)cargo test --workspace1,881 pass /--ignored --test-threads=120 pass / 新規smoke2 pass /incident_eval2 pass / clippy-D warningscleanjj 0.42.0-b8f7c455...を取得・実行 (Linux は WSL Ubuntu 24.04、Windows はローカル PowerShell)。版照合 gate が通る形式であることも確認target/debug/hooks-config.toml(CI の fresh clone には存在しない) を退避して両スイートを再実行し passReferences
--ignoredの直列実行 / jj 版照合の fail-closed / jj バージョンの 3 箇所結合docs/harness-improvement-plan.mdWP-16 (状態を「観測中」に更新、残作業を § 5 に記載)Summary by CodeRabbit
新機能
改善
ドキュメント