From 3cac2eb1193eb009f7db78ced5856fd678a18fdf Mon Sep 17 00:00:00 2001 From: Akio Jinsenji Date: Tue, 12 May 2026 14:28:09 +0900 Subject: [PATCH 1/4] =?UTF-8?q?docs(lessons):=20gwt-spec=20=E3=81=AE?= =?UTF-8?q?=E9=9B=86=E7=B4=84=20close=20=E4=BE=8B=E5=A4=96=E6=9D=A1?= =?UTF-8?q?=E9=A0=85=E3=82=92=E8=BF=BD=E5=8A=A0?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 別の canonical な gwt-spec に統合されて canonical 性が新 SPEC に 移行した旧 SPEC は close 可、という例外を「正式な gwt-spec を close しない」rule に追記。今日 Phase 分割の #185 / #187 / #188 / #191 / #192 をドメイン SPEC #204 に集約した運用に対応する。 Refs #204 Co-Authored-By: Claude Opus 4.7 (1M context) --- tasks/lessons.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tasks/lessons.md b/tasks/lessons.md index d968bf9..471efc7 100644 --- a/tasks/lessons.md +++ b/tasks/lessons.md @@ -56,5 +56,5 @@ - 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 番号を必ず明記する From 4fcf94402cebb70665dfa2bcb3f79b646eb934ef Mon Sep 17 00:00:00 2001 From: Akio Jinsenji Date: Wed, 13 May 2026 09:42:34 +0900 Subject: [PATCH 2/4] =?UTF-8?q?chore(hooks):=20husky=20=E3=82=92=20auto-in?= =?UTF-8?q?stall=20=E3=81=97=E3=81=A6=20commit-msg=20=E3=82=92=20CI=20?= =?UTF-8?q?=E3=81=A8=E4=B8=80=E8=87=B4=E3=81=95=E3=81=9B=E3=82=8B?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit pnpm install 時に husky で core.hookspath=.husky/_ を自動セット アップし、ローカル commit でも .husky/commit-msg 経由で commitlint が走るようにする。これまでは .husky/ ディレクトリは存在するのに husky が install されておらず hookspath が未設定で、ローカルでは footer-max-line-length 等の違反を CI でしか検知できなかった (PR #205 で実害発生)。 Refs #204 --- package.json | 2 ++ pnpm-lock.yaml | 10 ++++++++++ 2 files changed, 12 insertions(+) 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: {} From 0ad097608e9c006d86b1e917d290f97d41400263 Mon Sep 17 00:00:00 2001 From: Akio Jinsenji Date: Wed, 13 May 2026 09:51:05 +0900 Subject: [PATCH 3/4] =?UTF-8?q?chore(hooks):=20pre-push=20=E3=82=92=20fmt?= =?UTF-8?q?=20check=20=E3=81=AE=E3=81=BF=E3=81=AB=E7=B5=9E=E3=82=8B?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit cargo test / dotnet test を pre-push から削除し、format check だけ 残す。重いテストは CI に任せて push loop を高速化し、既存の並列実行 race condition (flaky integration tests) で push がブロックされる 事象を避ける。 Refs #204 --- .husky/pre-push | 17 +++++------------ 1 file changed, 5 insertions(+), 12 deletions(-) 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 ===" From b7296d8cf51efcb9bd987e111feaa4f66283ef2f Mon Sep 17 00:00:00 2001 From: Akio Jinsenji Date: Wed, 13 May 2026 10:01:43 +0900 Subject: [PATCH 4/4] =?UTF-8?q?fix(ci):=20cargo=20test=20=E3=82=92=20--tes?= =?UTF-8?q?t-threads=3D1=20=E3=81=A7=E5=AE=9F=E8=A1=8C=E3=81=97=E3=81=A6?= =?UTF-8?q?=20env=20race=20=E3=82=92=E8=A7=A3=E6=B6=88?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Rust の std::env::set_var は process-global で thread-unsafe、 複数 test が UNITY_CLI_REGISTRY_PATH / HOME を mutate すると library 内部の getenv 読み出しと衝突して flaky に失敗していた (PR #205 で `write_pid_and_cleanup_use_temp_home` / `execution_ context_uses_defaults` / `run_with_cli_set_active_text_output_ when_reachable` が intermittent に fail)。 CI workflow と CLAUDE.md の品質ゲートを cargo llvm-cov 側に既に 存在した --test-threads=1 と同じ運用に揃え、test 実行を serialize する。tasks/lessons.md に再発防止 rule を追加。 Refs #204 --- .github/workflows/test.yml | 8 ++++++-- CLAUDE.md | 2 +- tasks/lessons.md | 7 +++++++ 3 files changed, 14 insertions(+), 3 deletions(-) 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/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/tasks/lessons.md b/tasks/lessons.md index 471efc7..806ae84 100644 --- a/tasks/lessons.md +++ b/tasks/lessons.md @@ -58,3 +58,10 @@ - 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 判断を仰ぐ。ただし、別の 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 するため並列実行禁止」と明記する