diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 02ba196..7b30684 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -31,7 +31,10 @@ jobs: restore-keys: ${{ runner.os }}-cargo- - name: Run cargo test - run: cargo test + # env-mutating tests share process-global state (std::env::set_var + # is not thread-safe relative to library getenv reads), so serialize + # to avoid flakiness. The coverage job already uses --test-threads=1. + run: cargo test -- --test-threads=1 lsp-test: name: LSP Tests (required) @@ -76,7 +79,8 @@ jobs: --all-targets \ --cobertura \ --output-path coverage/rust/cobertura.xml \ - --fail-under-lines 90 + --fail-under-lines 90 \ + -- --test-threads=1 - name: Upload Rust coverage to Codecov uses: codecov/codecov-action@v6 diff --git a/.husky/pre-push b/.husky/pre-push index 4be328f..c6436a5 100755 --- a/.husky/pre-push +++ b/.husky/pre-push @@ -1,20 +1,13 @@ #!/usr/bin/env bash -# .husky/pre-push — Run tests before pushing -# Make executable: chmod +x .husky/pre-push +# .husky/pre-push — Fast local checks before push. +# Heavy test suites (cargo test, dotnet test) are handled by CI to keep +# the local push loop fast and to avoid being blocked by flaky test runs. set -euo pipefail REPO_ROOT=$(git rev-parse --show-toplevel) cd "$REPO_ROOT" || exit 1 -echo "=== pre-push: Running cargo test ===" -cargo test -echo "=== pre-push: cargo test passed ===" - -if [ -f "lsp/Server.Tests.csproj" ]; then - echo "=== pre-push: Running dotnet test ===" - dotnet test lsp/Server.Tests.csproj - echo "=== pre-push: dotnet test passed ===" -fi - +echo "=== pre-push: cargo fmt --all -- --check ===" +cargo fmt --all -- --check echo "=== pre-push: All checks passed ===" diff --git a/CLAUDE.md b/CLAUDE.md index e9ddfbe..b68214a 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -91,7 +91,7 @@ Node.js + MCP プロトコルベースの旧実装を Rust + TCP 直接通信に ```bash cargo fmt --all -- --check cargo clippy --all-targets -- -D warnings -cargo test --all-targets +cargo test --all-targets -- --test-threads=1 cargo run -- skills lint --severity error dotnet test lsp/Server.Tests.csproj ``` diff --git a/package.json b/package.json index ea510ca..1a3b70d 100644 --- a/package.json +++ b/package.json @@ -5,6 +5,7 @@ "packageManager": "pnpm@10.33.0", "description": "Development environment for unity-cli", "scripts": { + "prepare": "husky", "release": "echo \"Use scripts/publish.sh for manual release orchestration.\" && exit 1", "lint:md": "markdownlint README.md CONTRIBUTING.md CLAUDE.md RELEASE.md docs/development.md docs/constitution.md UnityCliBridge/Packages/unity-cli-bridge/**/*.md .github/workflows/README.md --config .markdownlint.json --ignore-path .markdownlintignore", "lint:format": "prettier --check .", @@ -21,6 +22,7 @@ "@openai/codex": "^0.130.0", "eslint": "^10.3.0", "globals": "^17.6.0", + "husky": "^9.1.7", "markdownlint-cli": "^0.48.0", "prettier": "^3.8.3" }, diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 98a7112..4509d15 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -39,6 +39,9 @@ importers: globals: specifier: ^17.6.0 version: 17.6.0 + husky: + specifier: ^9.1.7 + version: 9.1.7 markdownlint-cli: specifier: ^0.48.0 version: 0.48.0 @@ -572,6 +575,11 @@ packages: resolution: {integrity: sha512-sepffkT8stwnIYbsMBpoCHJuJM5l98FUF2AnE07hfvE0m/qp3R586hw4jF4uadbhvg1ooIdzuu7CsfD2jzCaNA==} engines: {node: '>=18'} + husky@9.1.7: + resolution: {integrity: sha512-5gs5ytaNjBrh5Ow3zrvdUUY+0VxIuWVL4i9irt6friV+BqdCfmV11CQTWMiBYWHbXhco+J1kHfTOUkePhCDvMA==} + engines: {node: '>=18'} + hasBin: true + ignore@5.3.2: resolution: {integrity: sha512-hsBTNUqQTDwkWtcdYI2i06Y/nUBEsNEDJKjWdigLvegy8kDuJAS8uRlpkkcQpyEXL0Z/pjDy5HBmMjRCJ2gq+g==} engines: {node: '>= 4'} @@ -1482,6 +1490,8 @@ snapshots: globals@17.6.0: {} + husky@9.1.7: {} + ignore@5.3.2: {} ignore@7.0.5: {} diff --git a/tasks/lessons.md b/tasks/lessons.md index d968bf9..806ae84 100644 --- a/tasks/lessons.md +++ b/tasks/lessons.md @@ -56,5 +56,12 @@ - Context: Phase 4 umbrella SPEC #191 の 5 サブタスク (A-E) がすべて develop に MERGED になった時点で「completed として close」してしまい、誤起票 #192 も同時に close した。user から「そもそも、正式な gwt-spec はクローズしません」と訂正を受けた。Phase 1-3 (#185 / #187 / #188) は実装 MERGED 後も OPEN 維持しているのが既存運用パターン (確認済み)。 - Mistake: GitHub Issue の標準的な「実装が終わったら close」感覚で gwt-spec を扱った。gwt-spec が "living documentation" として運用されている前提を見落とし、Tasks セクションに「整理タスク: umbrella を close するかどうか user 判断」と書いた時点で誤誘導が始まっていた。 -- Rule: gwt-spec ラベル付き Issue は実装完了後も close せず OPEN 維持する。Phase 完了 / PR 全 MERGED / Tasks 全 `[x]` であっても `gh issue close` / `gwtd issue close` を呼ばない。Out of Scope の継続改善余地や後続 Phase の起点として残す。Tasks セクションに「整理タスク: umbrella を close する判断」のような close 誘導項目を作らない。重複 / 誤起票で close 候補となる場合でも、自律的に close せず必ず user 判断を仰ぐ。 -- Checkpoint: 1. SPEC 完走報告は最終 comment + Workspace 更新 + Tasks `[x]` で足りる 2. close 操作の代わりに「OPEN 維持運用方針」を Tasks / 運用方針セクションに明示する 3. 既存 SPEC の状態確認時に `[CLOSED]` を見かけたら「意図的な close か」を user に確認、意図せず close されていたら reopen 相談 +- Rule: gwt-spec ラベル付き Issue は実装完了後も close せず OPEN 維持する。Phase 完了 / PR 全 MERGED / Tasks 全 `[x]` であっても `gh issue close` / `gwtd issue close` を呼ばない。Out of Scope の継続改善余地や後続 Phase の起点として残す。Tasks セクションに「整理タスク: umbrella を close する判断」のような close 誘導項目を作らない。重複 / 誤起票で close 候補となる場合でも、自律的に close せず必ず user 判断を仰ぐ。ただし、別の canonical な gwt-spec に統合されて canonical 性が新 SPEC に移行した旧 SPEC は、集約先 SPEC 番号を明記した最終コメントを残した上で close してよい (例: Phase 単位の SPEC をドメイン単位の SPEC に統合した 2026-05-12 のケース、#185 / #187 / #188 / #191 / #192 → #204)。重複 / 誤起票による単独 close と違い「canonical な後継 SPEC が存在する」ことが集約 close の必要条件。 +- Checkpoint: 1. SPEC 完走報告は最終 comment + Workspace 更新 + Tasks `[x]` で足りる 2. close 操作の代わりに「OPEN 維持運用方針」を Tasks / 運用方針セクションに明示する 3. 既存 SPEC の状態確認時に `[CLOSED]` を見かけたら「意図的な close か」を user に確認、意図せず close されていたら reopen 相談 4. close する場合は「集約 close (canonical 後継 SPEC あり)」「単独 close (重複 / 誤起票で user 承認済み)」のどちらかを判別し、集約 close なら最終コメントに集約先 SPEC 番号を必ず明記する + +### 2026-05-13 (cargo test の並列実行は env race で flaky) + +- Context: PR #205 で CI `Rust Tests` が `daemon::unityd::tests::write_pid_and_cleanup_use_temp_home` 等の race condition で flakily に fail。ローカル `cargo test --all-targets` でも `run_with_cli_set_active_text_output_when_reachable` / `execution_context_uses_defaults` が intermittent に fail (registry file の `EOF while parsing` / `trailing characters`)。 +- Mistake: 過去 PR で `cargo llvm-cov ... -- --test-threads=1` を採用していたのに、`cargo test` 側 (CI workflow / pre-push hook) を並列のまま残し、env race を本質的に解消する手当てをしていなかった。`crate::test_env::env_lock()` で test code 内の env 操作は serialize できているが、Rust の `std::env::set_var` は process-global で thread-unsafe、library 内部の getenv 読み出しと衝突するため並列実行は本質的に安全でない。 +- Rule: `cargo test` / `cargo llvm-cov` の呼び出しは CI / hook / ローカル品質ゲート全てで `-- --test-threads=1` を必須にする。新規 test を追加する際も、env を mutate する場合は `crate::test_env::env_lock()` を取得した上で、それでも並列実行に頼らない前提を守る。env を触る test を追加する際は「並列でも壊れない設計か」を考慮し、可能なら env 依存自体を public API への引数注入で除去する方向を優先する。 +- Checkpoint: 1. CI workflow (`.github/workflows/test.yml`) の `cargo test` / `cargo llvm-cov` 行に `-- --test-threads=1` があるか確認 2. CLAUDE.md の品質ゲートも `-- --test-threads=1` 表記で揃っているか確認 3. 新規 test で `std::env::set_var` を使う場合は `env_lock` を取得し、PR description に「env を mutate するため並列実行禁止」と明記する