Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 6 additions & 2 deletions .github/workflows/test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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
Expand Down
17 changes: 5 additions & 12 deletions .husky/pre-push
Original file line number Diff line number Diff line change
@@ -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 ==="
2 changes: 1 addition & 1 deletion CLAUDE.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
```
Expand Down
2 changes: 2 additions & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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 .",
Expand All @@ -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"
},
Expand Down
10 changes: 10 additions & 0 deletions pnpm-lock.yaml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

11 changes: 9 additions & 2 deletions tasks/lessons.md
Original file line number Diff line number Diff line change
Expand Up @@ -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 するため並列実行禁止」と明記する
Loading