diff --git a/.githooks/post-rewrite b/.githooks/post-rewrite index c2e997e12..f7f9cdfdd 100755 --- a/.githooks/post-rewrite +++ b/.githooks/post-rewrite @@ -1,18 +1,20 @@ #!/usr/bin/env sh -# ============================================================================ -# POST-REWRITE HOOK: Invalidate caches after history rewrite -# ============================================================================ -# Triggered by: git rebase, git commit --amend, git filter-branch -# Purpose: Clear the license year cache since git log dates may have changed. -# ============================================================================ +set -eu -REPO_ROOT=$(git rev-parse --show-toplevel 2>/dev/null) -if [ -z "$REPO_ROOT" ]; then - exit 0 +hook_name="post-rewrite" +hook_impl="$hook_name.ps1" +hook_dir="$(CDPATH= cd "$(dirname "$0")" && pwd -P)" +hook_script="$hook_dir/$hook_impl" + +if [ ! -f "$hook_script" ]; then + repo_root="$(git rev-parse --show-toplevel 2>/dev/null || true)" + if [ -n "$repo_root" ] && [ -f "$repo_root/.githooks/$hook_impl" ]; then + hook_script="$repo_root/.githooks/$hook_impl" + fi fi -CACHE_FILE="$REPO_ROOT/.git/license-year-cache" -if [ -f "$CACHE_FILE" ]; then - rm -f "$CACHE_FILE" - echo "License year cache invalidated (history rewritten)." +if [ ! -f "$hook_script" ] || ! command -v pwsh >/dev/null 2>&1; then + exit 0 fi + +exec pwsh -NoProfile -File "$hook_script" "$@" diff --git a/.githooks/post-rewrite.ps1 b/.githooks/post-rewrite.ps1 new file mode 100644 index 000000000..57b4ca909 --- /dev/null +++ b/.githooks/post-rewrite.ps1 @@ -0,0 +1,26 @@ +#!/usr/bin/env pwsh +Set-StrictMode -Version Latest +$ErrorActionPreference = 'Stop' + +$repoRoot = (& git rev-parse --show-toplevel 2>$null) +if ($LASTEXITCODE -ne 0 -or [string]::IsNullOrWhiteSpace($repoRoot)) { + exit 0 +} + +$repoRoot = ([string]$repoRoot).Trim() +$cachePath = (& git -C $repoRoot rev-parse --git-path license-year-cache 2>$null) +if ($LASTEXITCODE -ne 0 -or [string]::IsNullOrWhiteSpace($cachePath)) { + exit 0 +} + +$cachePath = ([string]$cachePath).Trim() +if (-not [System.IO.Path]::IsPathRooted($cachePath)) { + $cachePath = Join-Path $repoRoot $cachePath +} + +if (Test-Path -LiteralPath $cachePath -PathType Leaf) { + Remove-Item -LiteralPath $cachePath -Force + Write-Host 'License year cache invalidated (history rewritten).' +} + +exit 0 diff --git a/.githooks/pre-commit b/.githooks/pre-commit index 8e947a385..122fe4fa4 100755 --- a/.githooks/pre-commit +++ b/.githooks/pre-commit @@ -1,829 +1,102 @@ -#!/usr/bin/env bash -# Upgrade from `set -e` to include pipefail so that pipelines fail when any -# stage exits non-zero (not just the last). Without `pipefail`, a construct -# like `cspell ... | tee "$capture"` silently passes when cspell fails because -# tee's exit status is 0 — the exact regression seen when the Round 1 work -# gated the spell-check failure path on `if ! ... | tee`. `-u` (nounset) is -# intentionally omitted here: pre-commit legitimately dereferences arrays that -# may be empty (e.g. `${STAGED_FILES_ARRAY[@]}` under `bash -u` + old bash). -set -eo pipefail +#!/usr/bin/env sh +set -eu -# Source git staging helpers for safe index.lock handling -# This prevents "fatal: Unable to create '.git/index.lock': File exists" errors -# when using interactive git tools like lazygit that may hold locks -SCRIPT_DIR="$(cd "$(dirname "$0")/.." && pwd)" -HELPERS_PATH="$SCRIPT_DIR/scripts/git-staging-helpers.sh" -if [[ -f "$HELPERS_PATH" ]]; then - # shellcheck source=../scripts/git-staging-helpers.sh - source "$HELPERS_PATH" -else - echo "Error: git-staging-helpers.sh not found at $HELPERS_PATH" >&2 - echo "Pre-commit cannot safely stage files without lock-aware helpers." >&2 - echo "Restore scripts/git-staging-helpers.sh before committing." >&2 - exit 1 -fi - -# CRITICAL: Wait for any external tool (lazygit, IDE, etc.) to release the index.lock -# before starting hook operations. This is the primary fix for index.lock contention. -# Without this, the hook may start while lazygit is still holding the lock. -ensure_no_index_lock || { - echo "Error: index.lock still held after waiting." >&2 - echo "Close competing git tools (GUI, IDE git operations, lazygit) and retry commit." >&2 - exit 1 -} - -# Stage files with retry and emit actionable diagnostics on failure. -stage_with_retry_or_fail() { - local stage_context="$1" - shift - - if [ "$#" -eq 0 ]; then - return 0 - fi - - if git_add_with_retry "$@"; then - return 0 - fi - - echo "" >&2 - echo "=== Error: Failed to stage files ($stage_context) ===" >&2 - echo "Files:" >&2 - for path in "$@"; do - echo " $path" >&2 - done - echo "" >&2 - echo "Recovery:" >&2 - echo " 1. Close other git tools (GUI, IDE git operations, lazygit)." >&2 - echo " 2. Retry the commit after a few seconds." >&2 - echo " 3. Run npm run agent:preflight:fix before retrying to catch this earlier." >&2 - echo "" >&2 - exit 1 -} - -# 0) Sync version references (runs on every commit) -# a) Syncs banner version and .llm/context.md from package.json -# b) Syncs issue template package versions from package.json, CHANGELOG.md, and git tags -if command -v pwsh >/dev/null 2>&1; then - pwsh -NoProfile -File scripts/sync-banner-version.ps1 - pwsh -NoProfile -File scripts/sync-issue-template-versions.ps1 -elif command -v powershell >/dev/null 2>&1; then - powershell -NoProfile -ExecutionPolicy Bypass -File scripts/sync-banner-version.ps1 - powershell -NoProfile -ExecutionPolicy Bypass -File scripts/sync-issue-template-versions.ps1 -else - echo "PowerShell not found. Skipping version sync." >&2 -fi - -# 1) Normalize line endings (LF -> CRLF for most files, LF for .sh) -# This MUST run before any other formatting to prevent formatter diffs -if command -v pwsh >/dev/null 2>&1; then - pwsh -NoProfile -File scripts/normalize-eol.ps1 -elif command -v powershell >/dev/null 2>&1; then - powershell -NoProfile -ExecutionPolicy Bypass -File scripts/normalize-eol.ps1 -else - echo "PowerShell not found. Skipping EOL normalization." >&2 -fi - -# 2) Ensure .NET tools available (for CSharpier) -if command -v dotnet >/dev/null 2>&1; then - dotnet tool restore >/dev/null 2>&1 || true -fi - -# 3) Lint Markdown link text style (via Node wrapper -> PowerShell script) -if command -v node >/dev/null 2>&1; then - node ./scripts/run-doc-link-lint.js -else - run_pwsh() { - pwsh -NoProfile -File scripts/lint-doc-links.ps1 - } - - run_windows_ps() { - powershell -NoProfile -ExecutionPolicy Bypass -File scripts/lint-doc-links.ps1 - } - - if command -v pwsh >/dev/null 2>&1; then - run_pwsh - elif command -v powershell >/dev/null 2>&1; then - run_windows_ps - else - echo "PowerShell not found. Please install Node.js (preferred) or pwsh/powershell to run docs linter." >&2 - exit 1 - fi -fi - -run_node_tool() { - node scripts/run-node-bin.js "$@" -} - -run_prettier() { - node scripts/run-prettier.js "$@" -} - -require_node_tool() { - local tool="$1" - local purpose="$2" - if ! command -v node >/dev/null 2>&1; then - echo "Node.js is required for $purpose. Install Node.js/npm and run npm install." >&2 - exit 1 - fi - if ! run_node_tool "$tool" --version >/dev/null 2>&1; then - echo "Repo-local $tool is unavailable for $purpose. Run 'npm install' on the same host that runs git hooks." >&2 - exit 1 - fi -} - -# ============================================================================ -# SECURITY: Safe file list handling -# ============================================================================ -# File lists are read using null-delimited (-z) output from git and stored in -# bash arrays. This prevents shell injection attacks via crafted filenames -# containing spaces, newlines, semicolons, or other special characters. -# -# Pattern: git diff -z | while IFS= read -r -d '' file; do array+=("$file"); done -# This ensures each filename is treated as a single atomic argument, regardless -# of what characters it contains. -# ============================================================================ +repo_root_fast="$(git rev-parse --show-toplevel 2>/dev/null || true)" -# Read staged files into an array safely using null-delimited output -STAGED_FILES_ARRAY=() -while IFS= read -r -d '' file; do - STAGED_FILES_ARRAY+=("$file") -done < <(git diff --cached --name-only --diff-filter=ACM -z) +cleanup_ignored_hook_artifacts() { + cleanup_repo_root=$1 + cleanup_hooks_dir="$cleanup_repo_root/.githooks" + cleanup_unsafe="" + cleanup_root_extensions="txt out err" + cleanup_hook_extensions="txt log out err tmp" -# Early exit if no staged files (e.g., amend with no changes, or merge commit) -if [ ${#STAGED_FILES_ARRAY[@]} -eq 0 ]; then - echo "No staged files to check. Skipping pre-commit hooks." - exit 0 -fi - -# 5) Ensure staged text files end with a final newline -# This MUST run before Prettier formatting to prevent missing-newline diffs - -# ============================================================================ -# CRLF Detection Helper -# ============================================================================ -# Detects if a file uses CRLF (Windows) line endings by checking for carriage -# return characters (0x0d). This is used to preserve the correct line ending -# style when appending a final newline to files. -# -# Returns: -# 0 (true) - File uses CRLF line endings -# 1 (false) - File uses LF line endings (or is empty) -# ============================================================================ -file_uses_crlf() { - local file="$1" - # Check if file is non-empty and contains CR characters (part of CRLF) - if [[ -s "$file" ]] && grep -q $'\r' "$file" 2>/dev/null; then - return 0 # Uses CRLF + if [ ! -d "$cleanup_hooks_dir" ]; then + return 0 fi - return 1 # Uses LF -} -FINAL_NEWLINE_PATTERNS=('*.json' '*.jsonc' '*.asmdef' '*.asmref' '*.md' '*.markdown' - '*.yml' '*.yaml' '*.js' '*.ts' '*.cs' '*.sh' '*.ps1' '*.txt' '*.html' '*.css' '*.xml') -NEWLINE_FIXED=() -for file in "${STAGED_FILES_ARRAY[@]}"; do - # Check if file matches any of the text patterns - matched=0 - for pat in "${FINAL_NEWLINE_PATTERNS[@]}"; do - # shellcheck disable=SC2254 - case "$file" in - ${pat}) matched=1; break ;; + for cleanup_hook_path in "$cleanup_hooks_dir"/*; do + [ -f "$cleanup_hook_path" ] || continue + cleanup_hook_file=${cleanup_hook_path##*/} + case "$cleanup_hook_file" in + *.sample|*.txt|*.log|*.out|*.err|*.tmp) + continue + ;; esac - done - if [[ $matched -eq 1 ]] && [[ -s "$file" ]] && [[ "$(tail -c 1 -- "$file" | wc -l)" -eq 0 ]]; then - # Append the correct newline based on the file's line ending style - # CRLF files get \r\n, LF files get \n to avoid mixed line endings - if file_uses_crlf "$file"; then - printf '\r\n' >> "$file" - else - printf '\n' >> "$file" + cleanup_hook_name=${cleanup_hook_file%.*} + if [ -z "$cleanup_hook_name" ]; then + continue fi - NEWLINE_FIXED+=("$file") - fi -done -if [[ ${#NEWLINE_FIXED[@]} -gt 0 ]]; then - echo "Added final newline to ${#NEWLINE_FIXED[@]} file(s):" - for f in "${NEWLINE_FIXED[@]}"; do - echo " $f" - done - stage_with_retry_or_fail "final newline normalization" "${NEWLINE_FIXED[@]}" -fi - -# Filter files into type-specific arrays -MD_FILES_ARRAY=() -JSON_FILES_ARRAY=() -YAML_FILES_ARRAY=() -JS_FILES_ARRAY=() -CS_FILES_ARRAY=() -SPELL_FILES_ARRAY=() - -for file in "${STAGED_FILES_ARRAY[@]}"; do - case "$file" in - *.md|*.markdown) - MD_FILES_ARRAY+=("$file") - SPELL_FILES_ARRAY+=("$file") - ;; - *.json|*.jsonc|*.asmdef|*.asmref) - JSON_FILES_ARRAY+=("$file") - SPELL_FILES_ARRAY+=("$file") - ;; - *.yml|*.yaml) - YAML_FILES_ARRAY+=("$file") - SPELL_FILES_ARRAY+=("$file") - ;; - *.js) - JS_FILES_ARRAY+=("$file") - SPELL_FILES_ARRAY+=("$file") - ;; - *.cs) - CS_FILES_ARRAY+=("$file") - SPELL_FILES_ARRAY+=("$file") - ;; - esac -done - -# ============================================================================ -# LINE ENDING CONFIGURATION -# ============================================================================ -# Most files use CRLF (C#, JSON, Markdown) - see .gitattributes. -# -# EXCEPTIONS that MUST use LF: -# - YAML files (.yml, .yaml) - cross-platform compatibility -# - Shell scripts (.sh) - Unix requirement -# - .github/** ALL files - GitHub Actions run on Linux, Dependabot commits LF -# This includes .github/*.md files (copilot-instructions.md, etc.) -# -# The normalize-eol.ps1 script (step 1) handles this automatically. -# Prettier uses .prettierrc.json overrides for LF files. -# If you see EOL-related CI failures, run: npm run fix:eol -# ============================================================================ - -# Prettier format and re-add using retry logic to handle index.lock contention -if [ ${#MD_FILES_ARRAY[@]} -gt 0 ] || [ ${#JSON_FILES_ARRAY[@]} -gt 0 ] || [ ${#YAML_FILES_ARRAY[@]} -gt 0 ] || [ ${#JS_FILES_ARRAY[@]} -gt 0 ]; then - require_node_tool prettier "Prettier formatting" -fi - -if [ ${#MD_FILES_ARRAY[@]} -gt 0 ]; then - run_prettier --write --log-level warn -- "${MD_FILES_ARRAY[@]}" - # Verify formatting was successful before re-staging - if ! run_prettier --check -- "${MD_FILES_ARRAY[@]}" >/dev/null 2>&1; then - echo "Error: Prettier formatting verification failed for Markdown files." >&2 - exit 1 - fi - # Use git_add_with_retry to handle concurrent git operations safely - stage_with_retry_or_fail "Prettier re-stage (Markdown)" "${MD_FILES_ARRAY[@]}" -fi - -if [ ${#JSON_FILES_ARRAY[@]} -gt 0 ]; then - run_prettier --write --log-level warn -- "${JSON_FILES_ARRAY[@]}" - # Verify formatting was successful before re-staging - if ! run_prettier --check -- "${JSON_FILES_ARRAY[@]}" >/dev/null 2>&1; then - echo "Error: Prettier formatting verification failed for JSON files." >&2 - exit 1 - fi - # Use git_add_with_retry to handle concurrent git operations safely - stage_with_retry_or_fail "Prettier re-stage (JSON)" "${JSON_FILES_ARRAY[@]}" -fi - -if [ ${#YAML_FILES_ARRAY[@]} -gt 0 ]; then - run_prettier --write --log-level warn -- "${YAML_FILES_ARRAY[@]}" - # Verify formatting was successful before re-staging - if ! run_prettier --check -- "${YAML_FILES_ARRAY[@]}" >/dev/null 2>&1; then - echo "Error: Prettier formatting verification failed for YAML files." >&2 - exit 1 - fi - # Use git_add_with_retry to handle concurrent git operations safely - stage_with_retry_or_fail "Prettier re-stage (YAML)" "${YAML_FILES_ARRAY[@]}" -fi - -if [ ${#JS_FILES_ARRAY[@]} -gt 0 ]; then - run_prettier --write --log-level warn -- "${JS_FILES_ARRAY[@]}" - # Verify formatting was successful before re-staging - if ! run_prettier --check -- "${JS_FILES_ARRAY[@]}" >/dev/null 2>&1; then - echo "Error: Prettier formatting verification failed for JS files." >&2 - exit 1 - fi - # Use git_add_with_retry to handle concurrent git operations safely - stage_with_retry_or_fail "Prettier re-stage (JS)" "${JS_FILES_ARRAY[@]}" -fi - -# 6) Format staged C# files with CSharpier (if available) and re-stage -if [ ${#CS_FILES_ARRAY[@]} -gt 0 ]; then - if command -v pwsh >/dev/null 2>&1; then - pwsh -NoProfile -File scripts/format-staged-csharp.ps1 - elif command -v powershell >/dev/null 2>&1; then - powershell -NoProfile -ExecutionPolicy Bypass -File scripts/format-staged-csharp.ps1 - else - echo "PowerShell not found. Skipping CSharpier formatting." >&2 - fi -fi - -# 7) Markdown lint for staged Markdown files -if [ ${#MD_FILES_ARRAY[@]} -gt 0 ]; then - require_node_tool markdownlint "markdownlint validation" - # First, auto-fix what can be fixed - run_node_tool markdownlint --fix --config .markdownlint.json --ignore-path .markdownlintignore -- "${MD_FILES_ARRAY[@]}" || true - # Re-stage the fixed files - stage_with_retry_or_fail "markdownlint auto-fix re-stage" "${MD_FILES_ARRAY[@]}" - # Then run markdownlint again to catch any remaining unfixable issues - run_node_tool markdownlint --config .markdownlint.json --ignore-path .markdownlintignore -- "${MD_FILES_ARRAY[@]}" -fi - -# 7b) CHANGELOG lint when CHANGELOG.md is staged -CHANGELOG_STAGED=false -for file in "${STAGED_FILES_ARRAY[@]}"; do - if [[ "$file" == "CHANGELOG.md" ]]; then - CHANGELOG_STAGED=true - break - fi -done -if [ "$CHANGELOG_STAGED" = true ]; then - echo "Running CHANGELOG linter..." - if command -v pwsh >/dev/null 2>&1; then - pwsh -NoProfile -File scripts/lint-changelog.ps1 || { - echo "" >&2 - echo "=== CHANGELOG lint failed ===" >&2 - echo "Fix CHANGELOG.md structure issues before committing." >&2 - echo "" >&2 - exit 1 - } - elif command -v powershell >/dev/null 2>&1; then - powershell -NoProfile -ExecutionPolicy Bypass -File scripts/lint-changelog.ps1 || { - echo "" >&2 - echo "=== CHANGELOG lint failed ===" >&2 - echo "Fix CHANGELOG.md structure issues before committing." >&2 - echo "" >&2 - exit 1 - } - else - echo "PowerShell not found. Skipping CHANGELOG lint." >&2 - fi -fi - -# 8) YAML lint on staged YAML files -if [ ${#YAML_FILES_ARRAY[@]} -gt 0 ]; then - if command -v yamllint >/dev/null 2>&1; then - yamllint -c .yamllint.yaml -- "${YAML_FILES_ARRAY[@]}" - else - echo "yamllint not found; skipping YAML lint (run 'npm run verify:tools' to check setup)." >&2 - fi -fi - -# 8b) Dependabot config lint (if dependabot.yml is staged) -# Check for dependabot.yml specifically since yamllint doesn't validate schema -DEPENDABOT_FILES_ARRAY=() -for f in "${YAML_FILES_ARRAY[@]}"; do - if [[ "$f" == *".github/dependabot.yml" ]]; then - DEPENDABOT_FILES_ARRAY+=("$f") - fi -done -if [ ${#DEPENDABOT_FILES_ARRAY[@]} -gt 0 ]; then - if command -v pwsh >/dev/null 2>&1; then - pwsh -NoProfile -File scripts/lint-dependabot.ps1 -Paths "${DEPENDABOT_FILES_ARRAY[@]}" - elif command -v powershell >/dev/null 2>&1; then - powershell -NoProfile -ExecutionPolicy Bypass -File scripts/lint-dependabot.ps1 -Paths "${DEPENDABOT_FILES_ARRAY[@]}" - else - echo "PowerShell not found; skipping dependabot schema check." >&2 - fi -fi - -# 9) Spell check staged files with cspell -# Runs on markdown, code, and config files -if [ ${#SPELL_FILES_ARRAY[@]} -gt 0 ]; then - require_node_tool cspell "spell checking" - echo "Running spell check on staged files..." - # Use a temp file to avoid Windows command length limits with large staged sets - SPELL_FILE_LIST="$(mktemp 2>/dev/null || true)" - if [ -z "$SPELL_FILE_LIST" ]; then - echo "Error: Failed to create temporary file for spell check file list." >&2 - exit 1 - fi - - trap 'rm -f "$SPELL_FILE_LIST"' EXIT - printf '%s\n' "${SPELL_FILES_ARRAY[@]}" > "$SPELL_FILE_LIST" - # Capture cspell output so we can both display it AND scan it for - # lint-error-code-shaped tokens (the most common source of regression; - # see 2026-04-19 PWS001 incident documented in .githooks/pre-merge-commit). - # Use --no-must-find-files to avoid errors when files are filtered by cspell config. - SPELL_CAPTURE="$(mktemp 2>/dev/null || true)" - if [ -z "$SPELL_CAPTURE" ]; then - echo "Error: Failed to create temporary file for spell check output." >&2 - exit 1 - fi - # Extend the existing trap so both temp files are cleaned up. - trap 'rm -f "$SPELL_FILE_LIST" "$SPELL_CAPTURE"' EXIT - # IMPORTANT: do NOT use `if ! cspell ... | tee "$SPELL_CAPTURE"; then`. - # Even with pipefail enabled on the top of the file, keeping the - # explicit-exit-code form below is the paranoid belt-and-braces guard - # against a future revert of pipefail in this hook. The pattern is: - # 1. capture cspell output directly to the temp file, - # 2. capture cspell's exit status in SPELL_EXIT, - # 3. cat the capture so the user still sees the diagnostics, - # 4. branch on SPELL_EXIT only. - # This is the regression guard against P0-1 (Round 1 shipped the tee form - # on a `set -e`-only hook and silently passed every spelling failure). - SPELL_EXIT=0 - run_node_tool cspell lint --no-must-find-files --no-progress --show-suggestions --file-list "$SPELL_FILE_LIST" >"$SPELL_CAPTURE" 2>&1 || SPELL_EXIT=$? - cat "$SPELL_CAPTURE" - if [ "$SPELL_EXIT" -ne 0 ]; then - echo "" >&2 - echo "=== Spelling errors detected ===" >&2 - echo "Fix typos or add valid terms to the appropriate dictionary in cspell.json:" >&2 - echo " unity-terms -- Unity Engine APIs, components, lifecycle" >&2 - echo " csharp-terms -- C# language features, .NET types" >&2 - echo " package-terms -- This package's public API and type names" >&2 - echo " tech-terms -- General programming/tooling terms (CRLF, NUL, ASCII, etc.)" >&2 - echo " words (root) -- Project-specific words that don't fit a category" >&2 - echo "" >&2 - # Synthesize a copy-pasteable patch for lint-error-code-shaped unknowns. - # Extracts the "(TOKEN)" after "Unknown word" and keeps only tokens - # matching the lint-error-code prefix shape. This catches the common - # "new lint family -> new prefix -> missing cspell entry" regression. - # Width note: upper bound is unbounded (via {2,}) because cspell never - # emits monster tokens, and a narrow cap (originally 5) silently let - # longer prefixes like SHA256 or NOVELPFX slip past the patch emitter. - UNKNOWN_CODE_PREFIXES="$(grep -oE 'Unknown word \([A-Z]{2,}\)' "$SPELL_CAPTURE" 2>/dev/null | grep -oE '[A-Z]{2,}' | sort -u || true)" - if [ -n "$UNKNOWN_CODE_PREFIXES" ]; then - echo "=== Detected unregistered lint-error-code prefix(es) ===" >&2 - echo "Copy-paste this patch into the root 'words' array in cspell.json" >&2 - echo "(append each quoted prefix as a new array element):" >&2 - echo "" >&2 - while IFS= read -r prefix; do - [ -z "$prefix" ] && continue - echo " \"$prefix\"," >&2 - done <<< "$UNKNOWN_CODE_PREFIXES" - echo "" >&2 - echo "See scripts/validate-lint-error-codes.ps1 for the contract that" >&2 - echo "enforces this requirement once the prefix is registered." >&2 - echo "" >&2 - fi - echo "Re-run locally: npm run lint:spelling" >&2 - echo "" >&2 - exit 1 - fi -fi + for cleanup_extension in $cleanup_root_extensions; do + cleanup_relative="$cleanup_hook_name.$cleanup_extension" + cleanup_full="$cleanup_repo_root/$cleanup_relative" + [ -f "$cleanup_full" ] || continue + if git -C "$cleanup_repo_root" check-ignore -q -- "$cleanup_relative"; then + rm -f "$cleanup_full" + echo "[pre-commit] Removed stray hook artifact: $cleanup_relative" + else + cleanup_unsafe="$cleanup_unsafe +$cleanup_relative" + fi + done -# 10) Validate LLM instructions if .llm/ files are staged -LLM_FILES_ARRAY=() -for file in "${STAGED_FILES_ARRAY[@]}"; do - case "$file" in - .llm/*) - LLM_FILES_ARRAY+=("$file") - ;; - esac -done + for cleanup_extension in $cleanup_hook_extensions; do + cleanup_relative=".githooks/$cleanup_hook_name.$cleanup_extension" + cleanup_full="$cleanup_repo_root/$cleanup_relative" + [ -f "$cleanup_full" ] || continue + if git -C "$cleanup_repo_root" check-ignore -q -- "$cleanup_relative"; then + rm -f "$cleanup_full" + echo "[pre-commit] Removed stray hook artifact: $cleanup_relative" + else + cleanup_unsafe="$cleanup_unsafe +$cleanup_relative" + fi + done + done -if [ ${#LLM_FILES_ARRAY[@]} -gt 0 ]; then - echo "Validating LLM instructions..." - if command -v pwsh >/dev/null 2>&1; then - if ! pwsh -NoProfile -File scripts/lint-llm-instructions.ps1; then - echo "LLM lint failed. Attempting auto-fix..." >&2 - if pwsh -NoProfile -File scripts/lint-llm-instructions.ps1 -Fix; then - stage_with_retry_or_fail "LLM auto-fix re-stage" .llm/context.md - echo "LLM instructions auto-fixed and re-staged." - else - echo "LLM instructions auto-fix also failed." >&2 - exit 1 - fi - fi - elif command -v powershell >/dev/null 2>&1; then - if ! powershell -NoProfile -ExecutionPolicy Bypass -File scripts/lint-llm-instructions.ps1; then - echo "LLM lint failed. Attempting auto-fix..." >&2 - if powershell -NoProfile -ExecutionPolicy Bypass -File scripts/lint-llm-instructions.ps1 -Fix; then - stage_with_retry_or_fail "LLM auto-fix re-stage" .llm/context.md - echo "LLM instructions auto-fixed and re-staged." - else - echo "LLM instructions auto-fix also failed." >&2 - exit 1 - fi + if [ -n "$cleanup_unsafe" ]; then + echo '[pre-commit] ERROR: Stray hook-output artifact file(s) were not confirmed gitignored:' >&2 + printf '%s\n' "$cleanup_unsafe" | while IFS= read -r cleanup_relative; do + [ -n "$cleanup_relative" ] || continue + printf ' %s\n' "$cleanup_relative" >&2 + done + echo 'Add the artifact pattern to .gitignore or remove the stale file, then retry.' >&2 + return 1 fi - else - echo "PowerShell not found. Skipping LLM instructions validation." >&2 - fi -fi - -# 11) Check skill file sizes if .llm/skills/ markdown files or .llm/context.md are staged -# Note: In bash case statements, * matches any character including / -# so .llm/skills/*.md matches all depths (e.g., .llm/skills/sub/dir/file.md) -LLM_SIZE_CHECK_ARRAY=() -for file in "${STAGED_FILES_ARRAY[@]}"; do - case "$file" in - .llm/skills/*.md) - LLM_SIZE_CHECK_ARRAY+=("$file") - ;; - .llm/context.md) - LLM_SIZE_CHECK_ARRAY+=("$file") - ;; - esac -done - -if [ ${#LLM_SIZE_CHECK_ARRAY[@]} -gt 0 ]; then - echo "Checking skill and context file sizes..." - if command -v pwsh >/dev/null 2>&1; then - pwsh -NoProfile -File scripts/lint-skill-sizes.ps1 -Paths "${LLM_SIZE_CHECK_ARRAY[@]}" || { - echo "File size check failed. Some files exceed 500 lines and must be split or reduced." >&2 - exit 1 - } - elif command -v powershell >/dev/null 2>&1; then - powershell -NoProfile -ExecutionPolicy Bypass -File scripts/lint-skill-sizes.ps1 -Paths "${LLM_SIZE_CHECK_ARRAY[@]}" || { - echo "File size check failed. Some files exceed 500 lines and must be split or reduced." >&2 - exit 1 - } - else - echo "PowerShell not found. Skipping skill and context file size check." >&2 - fi -fi - -# 12) Run test linter on staged test files -# Checks for Unity object lifecycle issues (UNH001-UNH003), naming conventions (UNH004), null checks (UNH005) -TEST_FILES_ARRAY=() -for file in "${STAGED_FILES_ARRAY[@]}"; do - case "$file" in - Tests/*.cs) - TEST_FILES_ARRAY+=("$file") - ;; - esac -done -# Auto-fix Unity null assertions (Assert.IsNotNull/IsNull) in staged test files before linting -if [ ${#TEST_FILES_ARRAY[@]} -gt 0 ]; then - echo "Auto-fixing Unity null assertions in staged tests..." - if command -v pwsh >/dev/null 2>&1; then - pwsh -NoProfile -File scripts/lint-tests.ps1 -FixNullChecks -Paths "${TEST_FILES_ARRAY[@]}" - elif command -v powershell >/dev/null 2>&1; then - powershell -NoProfile -ExecutionPolicy Bypass -File scripts/lint-tests.ps1 -FixNullChecks -Paths "${TEST_FILES_ARRAY[@]}" - else - echo "PowerShell not found. Skipping test auto-fixes." >&2 - fi - - # Reformat and re-stage test files after auto-fixes - if command -v pwsh >/dev/null 2>&1; then - pwsh -NoProfile -File scripts/format-staged-csharp.ps1 "${TEST_FILES_ARRAY[@]}" - elif command -v powershell >/dev/null 2>&1; then - powershell -NoProfile -ExecutionPolicy Bypass -File scripts/format-staged-csharp.ps1 "${TEST_FILES_ARRAY[@]}" - fi -fi - -if [ ${#TEST_FILES_ARRAY[@]} -gt 0 ]; then - echo "Running test linter on staged test files..." - if command -v pwsh >/dev/null 2>&1; then - pwsh -NoProfile -File scripts/lint-tests.ps1 -Paths "${TEST_FILES_ARRAY[@]}" || { - echo "" >&2 - echo "=== Test lint failed ===" >&2 - echo "Fix the issues above or add // UNH-SUPPRESS comments for valid exceptions." >&2 - echo "For naming convention errors (UNH004):" >&2 - echo " - Use PascalCase or dot notation in TestName/SetName (e.g., 'Input.Null.ReturnsFalse')" >&2 - echo " - Use PascalCase for TestCaseSource method names (e.g., 'EdgeCaseTestData')" >&2 - echo "For null check errors (UNH005):" >&2 - echo " - Use Assert.IsTrue(x != null) instead of Assert.IsNotNull(x)" >&2 - echo " - Use Assert.IsTrue(x == null) instead of Assert.IsNull(x)" >&2 - echo " - Why: Unity's == operator performs special 'fake null' checking" >&2 - echo "Auto-fix null asserts: pwsh -NoProfile -File scripts/lint-tests.ps1 -FixNullChecks -Paths " >&2 - echo "" >&2 - exit 1 - } - elif command -v powershell >/dev/null 2>&1; then - powershell -NoProfile -ExecutionPolicy Bypass -File scripts/lint-tests.ps1 -Paths "${TEST_FILES_ARRAY[@]}" || { - echo "" >&2 - echo "=== Test lint failed ===" >&2 - echo "Fix the issues above or add // UNH-SUPPRESS comments for valid exceptions." >&2 - echo "For naming convention errors (UNH004):" >&2 - echo " - Use PascalCase or dot notation in TestName/SetName (e.g., 'Input.Null.ReturnsFalse')" >&2 - echo " - Use PascalCase for TestCaseSource method names (e.g., 'EdgeCaseTestData')" >&2 - echo "For null check errors (UNH005):" >&2 - echo " - Use Assert.IsTrue(x != null) instead of Assert.IsNotNull(x)" >&2 - echo " - Use Assert.IsTrue(x == null) instead of Assert.IsNull(x)" >&2 - echo " - Why: Unity's == operator performs special 'fake null' checking" >&2 - echo "Auto-fix null asserts: powershell -NoProfile -ExecutionPolicy Bypass -File scripts/lint-tests.ps1 -FixNullChecks -Paths " >&2 - echo "" >&2 - exit 1 - } - else - echo "PowerShell not found. Skipping test linter." >&2 - fi -fi - -# 13) Check for duplicate using directives in staged C# files -if [ ${#CS_FILES_ARRAY[@]} -gt 0 ]; then - echo "Checking for duplicate C# using directives..." - if command -v pwsh >/dev/null 2>&1; then - pwsh -NoProfile -File scripts/lint-duplicate-usings.ps1 -Paths "${CS_FILES_ARRAY[@]}" || { - echo "" >&2 - echo "=== Duplicate using directive lint failed (UNH007) ===" >&2 - echo "Remove duplicate using directives within each namespace/file scope." >&2 - echo "" >&2 - exit 1 - } - elif command -v powershell >/dev/null 2>&1; then - powershell -NoProfile -ExecutionPolicy Bypass -File scripts/lint-duplicate-usings.ps1 -Paths "${CS_FILES_ARRAY[@]}" || { - echo "" >&2 - echo "=== Duplicate using directive lint failed (UNH007) ===" >&2 - echo "Remove duplicate using directives within each namespace/file scope." >&2 - echo "" >&2 - exit 1 - } - else - echo "PowerShell not found. Skipping duplicate using directive lint." >&2 - fi -fi + return 0 +} -# 14) Check for forbidden #region/#endregion directives in C# files -# Regions are forbidden in this codebase - see .llm/skills/no-regions.md -if [ ${#CS_FILES_ARRAY[@]} -gt 0 ]; then - echo "Checking for forbidden #region directives..." - REGION_VIOLATIONS="" - for file in "${CS_FILES_ARRAY[@]}"; do - # Use grep to find #region or #endregion (case-insensitive for robustness) - # Match lines that have #region or #endregion as preprocessor directives - MATCHES=$(grep -n -iE '^[[:space:]]*#[[:space:]]*(region|endregion)' "$file" 2>/dev/null || true) - if [ -n "$MATCHES" ]; then - while IFS= read -r match; do - REGION_VIOLATIONS="${REGION_VIOLATIONS} ${file}:${match}"$'\n' - done <<< "$MATCHES" +if [ -n "$repo_root_fast" ]; then + cleanup_ignored_hook_artifacts "$repo_root_fast" + if git -C "$repo_root_fast" diff --cached --quiet --diff-filter=ACMR --; then + echo '[pre-commit] No staged files to check.' + exit 0 fi - done - - if [ -n "$REGION_VIOLATIONS" ]; then - echo "" >&2 - echo "=== Error: C# regions (#region/#endregion) are forbidden ===" >&2 - echo "The following files contain regions:" >&2 - echo "$REGION_VIOLATIONS" >&2 - echo "Remove all #region and #endregion directives before committing." >&2 - echo "See .llm/skills/no-regions.md for guidance on code organization alternatives." >&2 - echo "" >&2 - exit 1 - fi -fi - -# 15) Lint property drawers for multi-object editing issues (warnings only, non-blocking) -DRAWER_FILES_ARRAY=() -for file in "${CS_FILES_ARRAY[@]}"; do - case "$file" in - *Drawer.cs) - DRAWER_FILES_ARRAY+=("$file") - ;; - esac -done - -if [ ${#DRAWER_FILES_ARRAY[@]} -gt 0 ]; then - echo "Checking property drawers for multi-object editing issues..." - if command -v pwsh >/dev/null 2>&1; then - pwsh -NoProfile -File scripts/lint-drawer-multiobject.ps1 -Paths "${DRAWER_FILES_ARRAY[@]}" || true - elif command -v powershell >/dev/null 2>&1; then - powershell -NoProfile -ExecutionPolicy Bypass -File scripts/lint-drawer-multiobject.ps1 -Paths "${DRAWER_FILES_ARRAY[@]}" || true - else - echo "PowerShell not found. Skipping drawer multi-object lint." >&2 - fi fi -# 16) Check Odin drawer Undo safety (WeakTargets must be null-filtered before Undo.RecordObjects) -ODIN_DRAWER_FILES=() -for file in "${CS_FILES_ARRAY[@]}"; do - case "$file" in - Editor/CustomDrawers/Odin/*.cs) - ODIN_DRAWER_FILES+=("$file") - ;; - esac -done +hook_name="pre-commit" +hook_impl="$hook_name.ps1" +hook_dir="$(CDPATH= cd "$(dirname "$0")" && pwd -P)" +hook_script="$hook_dir/$hook_impl" -if [ ${#ODIN_DRAWER_FILES[@]} -gt 0 ]; then - echo "Checking Odin drawer Undo safety (UNH006)..." - if command -v pwsh >/dev/null 2>&1; then - if ! pwsh -NoProfile -File scripts/lint-odin-undo-safety.ps1 -Paths "${ODIN_DRAWER_FILES[@]}"; then - echo "" >&2 - echo "=== Odin drawer Undo safety check failed (UNH006) ===" >&2 - echo "See .llm/skills/odin-undo-safety.md for the safe pattern." >&2 - exit 1 +if [ ! -f "$hook_script" ]; then + repo_root="$(git rev-parse --show-toplevel 2>/dev/null || true)" + if [ -n "$repo_root" ] && [ -f "$repo_root/.githooks/$hook_impl" ]; then + hook_script="$repo_root/.githooks/$hook_impl" fi - elif command -v powershell >/dev/null 2>&1; then - if ! powershell -NoProfile -ExecutionPolicy Bypass -File scripts/lint-odin-undo-safety.ps1 -Paths "${ODIN_DRAWER_FILES[@]}"; then - echo "" >&2 - echo "=== Odin drawer Undo safety check failed (UNH006) ===" >&2 - echo "See .llm/skills/odin-undo-safety.md for the safe pattern." >&2 - exit 1 - fi - else - echo "PowerShell not found. Skipping Odin Undo safety lint." >&2 - fi fi -# 17) Check for missing .meta files on staged files -# Files in source roots (Runtime, Editor, Tests, docs, scripts, etc.) require .meta companions -META_REQUIRED_FILES=() -for file in "${STAGED_FILES_ARRAY[@]}"; do - case "$file" in - Runtime/*|Editor/*|Tests/*|Samples~/*|Shaders/*|Styles/*|URP/*|docs/*|scripts/*) - # Skip .meta files, package-lock.json, Gemfile.lock, temp files, dot files - case "$file" in - *.meta|*/package-lock.json|*/Gemfile.lock|*.tmp|*/.gitkeep|*/.DS_Store|*/Thumbs.db|*.pyc|*.pyo|*.swp|*.swo) - continue - ;; - esac - META_REQUIRED_FILES+=("$file") - ;; - esac -done - -if [ ${#META_REQUIRED_FILES[@]} -gt 0 ]; then - echo "Checking for missing .meta files on staged files..." - - # Use exact array membership checks instead of newline buffers so staged-path - # matching remains correct for filenames with spaces or leading dashes. - is_file_staged_exact() { - local target="$1" - local staged_file - for staged_file in "${STAGED_FILES_ARRAY[@]}"; do - if [[ "$staged_file" == "$target" ]]; then - return 0 - fi - done - return 1 - } - - META_MISSING=() - META_UNSTAGED=() - for file in "${META_REQUIRED_FILES[@]}"; do - meta="${file}.meta" - if [ ! -f "$meta" ]; then - META_MISSING+=("$file") - elif ! is_file_staged_exact "$meta"; then - # .meta exists on disk but is not staged -- auto-stage it - META_UNSTAGED+=("$meta") - fi - done - - # Also check for missing .meta files on new directories introduced by staged files - # Use associative-style dedup via sorted unique list - META_DIRS_TO_CHECK=() - for file in "${META_REQUIRED_FILES[@]}"; do - dir=$(dirname "$file") - while true; do - case "$dir" in - Runtime|Editor|Tests|Samples~|Shaders|Styles|URP|docs|scripts|.) - break - ;; - esac - META_DIRS_TO_CHECK+=("$dir") - dir=$(dirname "$dir") - done - done - - # Deduplicate directories - if [ ${#META_DIRS_TO_CHECK[@]} -gt 0 ]; then - UNIQUE_DIRS=$(printf '%s\n' "${META_DIRS_TO_CHECK[@]}" | sort -u) - while IFS= read -r dir; do - [ -z "$dir" ] && continue - meta="${dir}.meta" - if [ -d "$dir" ] && [ ! -f "$meta" ]; then - META_MISSING+=("$dir") - elif [ -d "$dir" ] && [ -f "$meta" ] && ! is_file_staged_exact "$meta"; then - META_UNSTAGED+=("$meta") - fi - done <<< "$UNIQUE_DIRS" - fi - - # Auto-stage .meta files that exist on disk but weren't staged - if [ ${#META_UNSTAGED[@]} -gt 0 ]; then - echo "Auto-staging ${#META_UNSTAGED[@]} unstaged .meta file(s):" - for meta in "${META_UNSTAGED[@]}"; do - echo " $meta" - done - if git_add_with_retry "${META_UNSTAGED[@]}"; then - echo "Successfully staged ${#META_UNSTAGED[@]} .meta file(s)." - else - echo "" >&2 - echo "=== Error: Failed to auto-stage .meta files ===" >&2 - echo "The hook could not stage one or more required .meta companions." >&2 - echo "This is usually caused by git index.lock contention from another process." >&2 - echo "" >&2 - echo "Files that could not be staged:" >&2 - for meta in "${META_UNSTAGED[@]}"; do - echo " $meta" >&2 - done - echo "" >&2 - echo "Recovery:" >&2 - echo " 1. Close other git tools (GUI, IDE git operations, lazygit)." >&2 - echo " 2. Retry the commit after a few seconds." >&2 - echo " 3. Run npm run agent:preflight:fix before retrying to catch this earlier." >&2 - echo "" >&2 - exit 1 - fi - fi +if [ ! -f "$hook_script" ]; then + echo "Error: $hook_impl not found at $hook_script" >&2 + echo "Run npm run hooks:install to restore tracked hooks." >&2 + exit 1 +fi - if [ ${#META_MISSING[@]} -gt 0 ]; then - echo "" >&2 - echo "=== Missing .meta files for staged files ===" >&2 - for file in "${META_MISSING[@]}"; do - echo " $file" >&2 - done - echo "" >&2 - echo "Generate missing meta files:" >&2 - echo " ./scripts/generate-meta.sh " >&2 - echo "" >&2 - exit 1 - fi +if ! command -v pwsh >/dev/null 2>&1; then + echo "Error: pwsh is required to run $hook_name." >&2 + echo "Install PowerShell 7+ and run npm run hooks:install." >&2 + exit 1 fi -echo "Pre-commit checks passed." -exit 0 +exec pwsh -NoProfile -File "$hook_script" "$@" diff --git a/.githooks/pre-commit.ps1 b/.githooks/pre-commit.ps1 new file mode 100644 index 000000000..d4fc40e3e --- /dev/null +++ b/.githooks/pre-commit.ps1 @@ -0,0 +1,640 @@ +#!/usr/bin/env pwsh +[CmdletBinding()] +param( + [Parameter(ValueFromRemainingArguments = $true)] + [string[]]$HookArgs +) + +Set-StrictMode -Version Latest +$ErrorActionPreference = 'Stop' + +$scriptHookDir = Split-Path -Parent $MyInvocation.MyCommand.Path +$repoRootResult = (& git rev-parse --show-toplevel 2>$null) +$repoRoot = if ($LASTEXITCODE -eq 0 -and -not [string]::IsNullOrWhiteSpace($repoRootResult)) { + ([string]$repoRootResult).Trim() +} +else { + (Resolve-Path -LiteralPath (Join-Path $scriptHookDir '..')).Path +} +$hookDir = Join-Path $repoRoot '.githooks' +$scriptsDir = Join-Path $repoRoot 'scripts' + +. (Join-Path $scriptsDir 'git-staging-helpers.ps1') + +function Write-HookInfo { + param([string]$Message) + Write-Host "[pre-commit] $Message" +} + +function Write-HookError { + param([string]$Message) + Write-Host "[pre-commit] ERROR: $Message" -ForegroundColor Red +} + +function Invoke-Native { + param( + [Parameter(Mandatory = $true)] + [string]$FileName, + [Parameter(Mandatory = $true)] + [string[]]$Arguments, + [switch]$AllowFailure + ) + + $startInfo = [System.Diagnostics.ProcessStartInfo]::new() + $startInfo.FileName = $FileName + foreach ($argument in $Arguments) { + [void]$startInfo.ArgumentList.Add($argument) + } + $startInfo.RedirectStandardOutput = $true + $startInfo.RedirectStandardError = $true + $startInfo.UseShellExecute = $false + $startInfo.WorkingDirectory = $repoRoot + + $process = [System.Diagnostics.Process]::new() + $process.StartInfo = $startInfo + [void]$process.Start() + $stdout = $process.StandardOutput.ReadToEnd() + $stderr = $process.StandardError.ReadToEnd() + $process.WaitForExit() + + if ($process.ExitCode -ne 0 -and -not $AllowFailure) { + if (-not [string]::IsNullOrWhiteSpace($stderr)) { + Write-Host $stderr.TrimEnd() -ForegroundColor Yellow + } + throw "$FileName exited with code $($process.ExitCode): $($Arguments -join ' ')" + } + + return [pscustomobject]@{ + ExitCode = $process.ExitCode + Stdout = $stdout + Stderr = $stderr + } +} + +function Invoke-Git { + param( + [Parameter(Mandatory = $true)] + [string[]]$Arguments, + [switch]$AllowFailure + ) + + return Invoke-Native -FileName 'git' -Arguments $Arguments -AllowFailure:$AllowFailure +} + +function Split-NulList { + param([AllowNull()][string]$Text) + + if ([string]::IsNullOrEmpty($Text)) { + return @() + } + + return @( + $Text -split ([string][char]0) | + Where-Object { -not [string]::IsNullOrWhiteSpace($_) } | + ForEach-Object { ([string]$_).Replace('\', '/') } + ) +} + +function ConvertTo-LiteralPathspec { + param([Parameter(Mandatory = $true)][string]$Path) + + return ":(literal)$Path" +} + +function ConvertFrom-GitGrepRegionOutput { + param([AllowNull()][string]$Text) + + $violations = [System.Collections.Generic.List[string]]::new() + if ([string]::IsNullOrEmpty($Text)) { + return $violations + } + + $cursor = 0 + while ($cursor -lt $Text.Length) { + $pathEnd = $Text.IndexOf([char]0, $cursor) + if ($pathEnd -lt 0) { + break + } + + $lineEnd = $Text.IndexOf([char]0, $pathEnd + 1) + if ($lineEnd -lt 0) { + break + } + + $textEnd = $Text.IndexOf("`n", $lineEnd + 1) + if ($textEnd -lt 0) { + $textEnd = $Text.Length + } + + $path = $Text.Substring($cursor, $pathEnd - $cursor) + $lineNumber = $Text.Substring($pathEnd + 1, $lineEnd - $pathEnd - 1) + $lineText = $Text.Substring($lineEnd + 1, $textEnd - $lineEnd - 1).TrimEnd("`r") + $violations.Add("${path}:${lineNumber}: $lineText") | Out-Null + + $cursor = $textEnd + 1 + } + + return $violations +} + +function Get-StagedRegionViolations { + param([string[]]$Paths) + + $violations = [System.Collections.Generic.List[string]]::new() + $pathspecs = @( + $Paths | + Where-Object { -not [string]::IsNullOrWhiteSpace($_) } | + Sort-Object -Unique | + ForEach-Object { ConvertTo-LiteralPathspec -Path $_ } + ) + if ($pathspecs.Count -eq 0) { + return $violations + } + + $pattern = '^[[:space:]]*#[[:space:]]*(region|endregion)' + $chunkSize = 200 + for ($offset = 0; $offset -lt $pathspecs.Count; $offset += $chunkSize) { + $end = [Math]::Min($offset + $chunkSize - 1, $pathspecs.Count - 1) + $chunk = @($pathspecs[$offset..$end]) + $grep = Invoke-Git -Arguments (@('grep', '--cached', '-n', '-I', '-E', '-z', $pattern, '--') + $chunk) -AllowFailure + if ($grep.ExitCode -eq 0) { + foreach ($violation in @(ConvertFrom-GitGrepRegionOutput -Text $grep.Stdout)) { + $violations.Add($violation) | Out-Null + } + } + elseif ($grep.ExitCode -gt 1) { + throw "git grep failed while checking staged C# regions." + } + } + + return $violations +} + +function Get-StagedPaths { + $result = Invoke-Git -Arguments @('diff', '--cached', '--name-only', '--diff-filter=ACMR', '-z') + return Split-NulList -Text $result.Stdout +} + +function Get-UnstagedPathSet { + $result = Invoke-Git -Arguments @('diff', '--name-only', '--diff-filter=ACMRTUXB', '-z') -AllowFailure + $set = [System.Collections.Generic.HashSet[string]]::new([System.StringComparer]::Ordinal) + if ($result.ExitCode -ne 0) { + return $set + } + + foreach ($path in @(Split-NulList -Text $result.Stdout)) { + [void]$set.Add($path) + } + return $set +} + +function Invoke-HookPowerShellScript { + param( + [Parameter(Mandatory = $true)] + [string]$ScriptRelativePath, + [string[]]$Arguments = @(), + [int[]]$AllowedExitCodes = @(0) + ) + + $scriptPath = Join-Path $repoRoot $ScriptRelativePath + $pwshPath = (Get-Process -Id $PID).Path + if ([string]::IsNullOrWhiteSpace($pwshPath)) { + $pwshPath = if (Get-Command pwsh -ErrorAction SilentlyContinue) { 'pwsh' } else { 'powershell' } + } + + $invokeArgs = @('-NoProfile') + if ($PSVersionTable.PSEdition -ne 'Core') { + $invokeArgs += @('-ExecutionPolicy', 'Bypass') + } + $invokeArgs += @('-File', $scriptPath) + $invokeArgs += $Arguments + + & $pwshPath @invokeArgs + $exitCode = $LASTEXITCODE + if ($AllowedExitCodes -notcontains $exitCode) { + throw "$ScriptRelativePath exited with code $exitCode." + } +} + +function Get-HookNormalizedUniquePaths { + param([AllowNull()][string[]]$Paths) + + if ($null -eq $Paths -or $Paths.Count -eq 0) { + return @() + } + + return @( + $Paths | + Where-Object { -not [string]::IsNullOrWhiteSpace($_) } | + ForEach-Object { ([string]$_).Replace('\', '/') } | + Sort-Object -Unique + ) +} + +function Assert-HookWholeFileAutoFixSafe { + param( + [string]$Context, + [string[]]$Paths, + [ValidateSet('before', 'after')] + [string]$Phase = 'before', + [switch]$AllowInitiallyUnstaged + ) + + if ($AllowInitiallyUnstaged) { + return + } + + $items = @(Get-HookNormalizedUniquePaths -Paths $Paths) + if ($items.Count -eq 0) { + return + } + + $partialPaths = @($items | Where-Object { + $null -ne $script:InitiallyUnstagedPaths -and $script:InitiallyUnstagedPaths.Contains($_) + }) + if ($partialPaths.Count -eq 0) { + return + } + + Write-HookError "Refusing to auto-stage whole file(s) with pre-existing unstaged changes ${Phase} ${Context}." + foreach ($item in $partialPaths) { + Write-Host " $item" -ForegroundColor Yellow + } + Write-Host 'Run npm run agent:preflight:fix before staging, or stage the intended hunks and retry.' -ForegroundColor Cyan + exit 1 +} + +function Add-HookPathsToIndex { + param( + [string]$Context, + [string[]]$Paths + ) + + $items = @(Get-HookNormalizedUniquePaths -Paths $Paths) + if ($items.Count -eq 0) { + return + } + + Assert-HookWholeFileAutoFixSafe ` + -Context $Context ` + -Paths $items ` + -Phase 'after' ` + -AllowInitiallyUnstaged:($Context -match '\.meta companion') + + $repositoryInfo = Get-GitRepositoryInfo + $exitCode = Invoke-GitAddWithRetry -Items $items -IndexLockPath $repositoryInfo.IndexLockPath + if ($exitCode -ne 0) { + Write-HookError "Failed to stage files ($Context)." + foreach ($item in $items) { + Write-Host " $item" -ForegroundColor Yellow + } + Write-Host 'Close other git tools, then run npm run agent:preflight:fix and retry.' -ForegroundColor Cyan + exit $exitCode + } +} + +function Test-PathDirtyOrUntracked { + param([string]$Path) + + $result = Invoke-Git -Arguments @('status', '--porcelain', '--', $Path) -AllowFailure + return ($result.ExitCode -eq 0 -and -not [string]::IsNullOrWhiteSpace($result.Stdout)) +} + +function Remove-GitIgnoredHookArtifacts { + $rootArtifactExtensions = @('txt', 'out', 'err') + $hookArtifactExtensions = @('txt', 'log', 'out', 'err', 'tmp') + $artifactExtensions = @($rootArtifactExtensions + $hookArtifactExtensions | Sort-Object -Unique) + $hookNames = [System.Collections.Generic.HashSet[string]]::new([System.StringComparer]::Ordinal) + if (-not (Test-Path -LiteralPath $hookDir -PathType Container)) { + return + } + + foreach ($hook in Get-ChildItem -LiteralPath $hookDir -File -ErrorAction SilentlyContinue) { + if ($hook.Name -like '*.sample' -or $artifactExtensions -contains $hook.Extension.TrimStart('.')) { + continue + } + + $name = if ($hook.Extension) { [System.IO.Path]::GetFileNameWithoutExtension($hook.Name) } else { $hook.Name } + if (-not [string]::IsNullOrWhiteSpace($name)) { + [void]$hookNames.Add($name) + } + } + + $candidates = [System.Collections.Generic.List[string]]::new() + foreach ($hookName in $hookNames) { + foreach ($extension in $rootArtifactExtensions) { + $relative = "$hookName.$extension" + if (Test-Path -LiteralPath (Join-Path $repoRoot $relative) -PathType Leaf) { + $candidates.Add($relative) | Out-Null + } + } + foreach ($extension in $hookArtifactExtensions) { + $relative = ".githooks/$hookName.$extension" + if (Test-Path -LiteralPath (Join-Path $repoRoot $relative) -PathType Leaf) { + $candidates.Add($relative) | Out-Null + } + } + } + + $uniqueCandidates = @($candidates | Sort-Object -Unique) + if ($uniqueCandidates.Count -eq 0) { + return + } + + $unsafe = [System.Collections.Generic.List[string]]::new() + foreach ($relative in $uniqueCandidates) { + $check = Invoke-Git -Arguments @('check-ignore', '-q', '--', $relative) -AllowFailure + if ($check.ExitCode -eq 0) { + Remove-Item -LiteralPath (Join-Path $repoRoot $relative) -Force + Write-HookInfo "Removed stray hook artifact: $relative" + } + else { + $unsafe.Add($relative) | Out-Null + } + } + + if ($unsafe.Count -gt 0) { + Write-HookError 'Stray hook-output artifact file(s) were not confirmed gitignored:' + foreach ($relative in $unsafe) { + Write-Host " $relative" -ForegroundColor Yellow + } + Write-Host 'Add the artifact pattern to .gitignore or remove the stale file, then retry.' -ForegroundColor Cyan + exit 1 + } +} + +function Add-FinalNewlines { + param([string[]]$Paths) + + $patterns = @( + '*.json', '*.jsonc', '*.asmdef', '*.asmref', '*.md', '*.markdown', + '*.yml', '*.yaml', '*.js', '*.ts', '*.cs', '*.sh', '*.ps1', '*.txt', + '*.html', '*.css', '*.xml' + ) + $fixed = [System.Collections.Generic.List[string]]::new() + + foreach ($path in $Paths) { + $matched = $false + foreach ($pattern in $patterns) { + if ($path -like $pattern) { + $matched = $true + break + } + } + if (-not $matched) { + continue + } + + $fullPath = Join-Path $repoRoot $path + if (-not (Test-Path -LiteralPath $fullPath -PathType Leaf)) { + continue + } + + $bytes = [System.IO.File]::ReadAllBytes($fullPath) + if ($bytes.Length -eq 0 -or $bytes[$bytes.Length - 1] -eq 10) { + continue + } + + $appendBytes = if ($bytes.Length -gt 0 -and $bytes -contains 13) { + [byte[]](13, 10) + } + else { + [byte[]](10) + } + Assert-HookWholeFileAutoFixSafe -Context 'final newline normalization' -Paths @($path) -Phase 'before' + [System.IO.File]::WriteAllBytes($fullPath, [byte[]]($bytes + $appendBytes)) + $fixed.Add($path) | Out-Null + } + + if ($fixed.Count -gt 0) { + Write-HookInfo "Added final newline to $($fixed.Count) file(s)." + Add-HookPathsToIndex -Context 'final newline normalization' -Paths $fixed + } +} + +function Invoke-VersionSync { + param([string[]]$StagedPaths) + + $syncTargets = [System.Collections.Generic.List[string]]::new() + if ($StagedPaths -contains 'package.json' -or $StagedPaths -contains '.llm/context.md' -or $StagedPaths -contains 'docs/images/unity-helpers-banner.svg') { + Assert-HookWholeFileAutoFixSafe ` + -Context 'banner version sync' ` + -Paths @('docs/images/unity-helpers-banner.svg', '.llm/context.md') ` + -Phase 'before' + Write-HookInfo 'Synchronizing banner version metadata.' + Invoke-HookPowerShellScript -ScriptRelativePath 'scripts/sync-banner-version.ps1' + $syncTargets.Add('docs/images/unity-helpers-banner.svg') | Out-Null + $syncTargets.Add('.llm/context.md') | Out-Null + } + + $issueInputs = @( + 'package.json', + 'CHANGELOG.md', + '.github/ISSUE_TEMPLATE/bug_report.yml', + '.github/ISSUE_TEMPLATE/feature_request.yml' + ) + if (@($issueInputs | Where-Object { $StagedPaths -contains $_ }).Count -gt 0) { + Assert-HookWholeFileAutoFixSafe ` + -Context 'issue template version sync' ` + -Paths @('.github/ISSUE_TEMPLATE/bug_report.yml', '.github/ISSUE_TEMPLATE/feature_request.yml') ` + -Phase 'before' + Write-HookInfo 'Synchronizing issue template versions.' + Invoke-HookPowerShellScript -ScriptRelativePath 'scripts/sync-issue-template-versions.ps1' + $syncTargets.Add('.github/ISSUE_TEMPLATE/bug_report.yml') | Out-Null + $syncTargets.Add('.github/ISSUE_TEMPLATE/feature_request.yml') | Out-Null + } + + $dirtyTargets = @($syncTargets | Where-Object { Test-PathDirtyOrUntracked -Path $_ }) + if ($dirtyTargets.Count -gt 0) { + Add-HookPathsToIndex -Context 'version sync' -Paths $dirtyTargets + } +} + +function Invoke-FastCSharpChecks { + param([string[]]$StagedPaths) + + $csharpFiles = @($StagedPaths | Where-Object { $_ -like '*.cs' }) + if ($csharpFiles.Count -eq 0) { + return + } + + $regionViolations = @(Get-StagedRegionViolations -Paths $csharpFiles) + + if ($regionViolations.Count -gt 0) { + Write-HookError 'Forbidden #region/#endregion directives detected:' + foreach ($violation in $regionViolations) { + Write-Host " $violation" -ForegroundColor Yellow + } + exit 1 + } +} + +function Invoke-LlmChecks { + param([string[]]$StagedPaths) + + $llmFiles = @($StagedPaths | Where-Object { $_ -like '.llm/*' }) + if ($llmFiles.Count -eq 0) { + return + } + + Write-HookInfo 'Validating LLM instruction consistency.' + try { + Invoke-HookPowerShellScript -ScriptRelativePath 'scripts/lint-llm-instructions.ps1' + } + catch { + Write-HookInfo 'LLM instructions failed validation; attempting auto-fix.' + Assert-HookWholeFileAutoFixSafe ` + -Context 'LLM instruction auto-fix' ` + -Paths @('.llm/context.md', '.llm/skills/index.md') ` + -Phase 'before' + Invoke-HookPowerShellScript -ScriptRelativePath 'scripts/lint-llm-instructions.ps1' -Arguments @('-Fix') + Add-HookPathsToIndex -Context 'LLM instruction auto-fix' -Paths @('.llm/context.md', '.llm/skills/index.md') + Invoke-HookPowerShellScript -ScriptRelativePath 'scripts/lint-llm-instructions.ps1' + } + + $sizeTargets = @($StagedPaths | Where-Object { $_ -eq '.llm/context.md' -or $_ -like '.llm/skills/*.md' }) + if ($sizeTargets.Count -gt 0) { + Write-HookInfo 'Checking changed skill/context file sizes.' + Invoke-HookPowerShellScript -ScriptRelativePath 'scripts/lint-skill-sizes.ps1' -Arguments (@('-Paths') + $sizeTargets) + } +} + +function Invoke-MetaChecks { + param([string[]]$StagedPaths) + + $sourceRootPattern = '^(Runtime|Editor|Tests|Samples~|Shaders|Styles|URP|docs|scripts)/' + $metaRequired = @($StagedPaths | Where-Object { + $_ -match $sourceRootPattern -and + $_ -notlike '*.meta' -and + $_ -notlike '*/package-lock.json' -and + $_ -notlike '*/Gemfile.lock' -and + $_ -notlike '*.tmp' -and + (Split-Path -Leaf $_) -notin @('.gitkeep', '.DS_Store', 'Thumbs.db') -and + $_ -notlike '*.pyc' -and + $_ -notlike '*.pyo' -and + $_ -notlike '*.swp' -and + $_ -notlike '*.swo' + }) + + if ($metaRequired.Count -eq 0) { + return + } + + $stagedSet = [System.Collections.Generic.HashSet[string]]::new([System.StringComparer]::Ordinal) + foreach ($path in $StagedPaths) { + [void]$stagedSet.Add($path) + } + + $missing = [System.Collections.Generic.List[string]]::new() + $unstaged = [System.Collections.Generic.HashSet[string]]::new([System.StringComparer]::Ordinal) + $directories = [System.Collections.Generic.HashSet[string]]::new([System.StringComparer]::Ordinal) + + foreach ($path in $metaRequired) { + $fullPath = Join-Path $repoRoot $path + if (-not (Test-Path -LiteralPath $fullPath)) { + continue + } + + $metaPath = "$path.meta" + $metaFullPath = Join-Path $repoRoot $metaPath + if (-not (Test-Path -LiteralPath $metaFullPath -PathType Leaf)) { + $missing.Add($path) | Out-Null + } + elseif (-not $stagedSet.Contains($metaPath) -and (Test-PathDirtyOrUntracked -Path $metaPath)) { + [void]$unstaged.Add($metaPath) + } + + $directory = if (Test-Path -LiteralPath $fullPath -PathType Container) { + $path + } + else { + (Split-Path -Path $path -Parent).Replace('\', '/') + } + + while (-not [string]::IsNullOrWhiteSpace($directory) -and $directory -ne '.') { + if ($directory -in @('Runtime', 'Editor', 'Tests', 'Samples~', 'Shaders', 'Styles', 'URP', 'docs', 'scripts')) { + break + } + [void]$directories.Add($directory) + $directory = Split-Path -Path $directory -Parent + if (-not [string]::IsNullOrWhiteSpace($directory)) { + $directory = $directory.Replace('\', '/') + } + } + } + + foreach ($directory in $directories) { + $directoryFullPath = Join-Path $repoRoot $directory + if (-not (Test-Path -LiteralPath $directoryFullPath -PathType Container)) { + continue + } + + $metaPath = "$directory.meta" + $metaFullPath = Join-Path $repoRoot $metaPath + if (-not (Test-Path -LiteralPath $metaFullPath -PathType Leaf)) { + $missing.Add($directory) | Out-Null + } + elseif (-not $stagedSet.Contains($metaPath) -and (Test-PathDirtyOrUntracked -Path $metaPath)) { + [void]$unstaged.Add($metaPath) + } + } + + $unstagedPaths = @($unstaged | Sort-Object) + if ($unstagedPaths.Count -gt 0) { + Write-HookInfo "Auto-staging $($unstagedPaths.Count) dirty .meta companion file(s)." + Add-HookPathsToIndex -Context '.meta companion auto-stage' -Paths $unstagedPaths + } + + $missingPaths = @($missing | Sort-Object -Unique) + if ($missingPaths.Count -gt 0) { + Write-HookError 'Missing .meta files for staged paths:' + foreach ($path in $missingPaths) { + Write-Host " $path" -ForegroundColor Yellow + } + Write-Host 'Run npm run agent:preflight:fix to generate recoverable .meta files.' -ForegroundColor Cyan + exit 1 + } +} + +try { + Push-Location $repoRoot + + Remove-GitIgnoredHookArtifacts + + $stagedPaths = @(Get-StagedPaths) + if ($stagedPaths.Count -eq 0) { + Write-HookInfo 'No staged files to check.' + exit 0 + } + + if (-not (Invoke-EnsureNoIndexLock)) { + Write-HookError 'index.lock still held after waiting.' + Write-Host 'Close competing git tools and retry the commit.' -ForegroundColor Cyan + exit 1 + } + + $script:InitiallyUnstagedPaths = Get-UnstagedPathSet + + # The hook is intentionally limited to fast, local, auto-repairable checks. + # Formatting, EOL normalization, spelling, Markdown lint, docs link lint, + # CSharpier, test lint, duplicate-using lint, and license audits belong in + # agent:preflight and CI so hooks remain a last-resort safety net. + Invoke-VersionSync -StagedPaths $stagedPaths + Add-FinalNewlines -Paths $stagedPaths + + $stagedPaths = @(Get-StagedPaths) + Invoke-LlmChecks -StagedPaths $stagedPaths + Invoke-FastCSharpChecks -StagedPaths $stagedPaths + Invoke-MetaChecks -StagedPaths $stagedPaths + + Write-HookInfo 'Fast pre-commit checks passed.' + exit 0 +} +catch { + Write-HookError $_.Exception.Message + Write-Host 'Run npm run agent:preflight:fix before retrying the commit.' -ForegroundColor Cyan + exit 1 +} +finally { + Pop-Location +} diff --git a/.githooks/pre-merge-commit b/.githooks/pre-merge-commit index bdfb8a677..235e36dbb 100755 --- a/.githooks/pre-merge-commit +++ b/.githooks/pre-merge-commit @@ -1,62 +1,28 @@ -#!/usr/bin/env bash -# ============================================================================ -# PRE-MERGE-COMMIT HOOK: Run pre-commit checks on auto-created merge commits -# ============================================================================ -# Triggered by: `git merge` when git AUTO-CREATES a merge commit (i.e. the -# merge succeeded without conflicts and there is nothing for the user to -# resolve manually). In that case git does NOT fire pre-commit — this hook -# is its substitute. -# -# SCOPE — what this hook DOES cover: -# - Non-fast-forward merge where the combined tree has no conflicts. -# Example: `git merge --no-ff feature` after both sides advanced. -# -# SCOPE — what this hook does NOT cover (and what does): -# - Conflict-resolution commits: when `git merge` produces conflicts, the -# user edits files and runs `git commit`. That fires pre-commit normally, -# not pre-merge-commit. So pre-commit remains the authority for conflict- -# resolved content. -# - Fast-forward merges: `git merge` that only advances HEAD fires NO -# hooks at all (no merge commit is created). The only guard is CI. -# - `git merge --squash`: creates an index state but requires a manual -# `git commit`. That fires pre-commit, not pre-merge-commit. -# - `git cherry-pick`, `git rebase`, `git rebase --continue`: bypass -# BOTH pre-commit AND pre-merge-commit. Only CI catches issues -# introduced by these paths. -# -# In every bypassed path above, CI is the final backstop. Specifically: -# - .github/workflows/lint-error-codes.yml enforces the cspell -# lint-error-code contract on every PR touching hook/lint surface. -# - scripts/validate-lint-error-codes.ps1 is the underlying validator -# that the CI workflow runs; it roundtrips every [A-Z]{2,}\d{3} prefix -# emitted by any lint/test/hook file through cspell. -# -# HISTORICAL CONTEXT: -# On 2026-04-19 a new skill file introduced 5 occurrences of the token -# `PWS` (part of lint-error-code names like `PWS001`). cspell had no -# entry for `PWS`. The change landed via a merge commit, so pre-commit -# did not run. The failure surfaced at pre-push and blocked the whole -# batch. This hook plus the CI workflow together close that gap. -# -# WHAT THIS HOOK DOES: -# Delegates (exec) to the pre-commit hook. pre-commit already derives the -# staged-file set from `git diff --cached --name-only --diff-filter=ACM`, -# which correctly reflects the pending merge commit's changed content. -# -# BYPASS: -# `git merge --no-verify` skips this hook, same as pre-commit. CI is the -# final backstop. -# ============================================================================ +#!/usr/bin/env sh +set -eu -set -e +hook_name="pre-merge-commit" +hook_impl="$hook_name.ps1" +hook_dir="$(CDPATH= cd "$(dirname "$0")" && pwd -P)" +hook_script="$hook_dir/$hook_impl" -SCRIPT_DIR="$(cd "$(dirname "$0")" && pwd)" -PRE_COMMIT="$SCRIPT_DIR/pre-commit" +if [ ! -f "$hook_script" ]; then + repo_root="$(git rev-parse --show-toplevel 2>/dev/null || true)" + if [ -n "$repo_root" ] && [ -f "$repo_root/.githooks/$hook_impl" ]; then + hook_script="$repo_root/.githooks/$hook_impl" + fi +fi + +if [ ! -f "$hook_script" ]; then + echo "Error: $hook_impl not found at $hook_script" >&2 + echo "Run npm run hooks:install to restore tracked hooks." >&2 + exit 1 +fi -if [[ ! -x "$PRE_COMMIT" ]]; then - echo "Error: pre-commit hook not found or not executable at $PRE_COMMIT" >&2 - echo "Run 'npm run hooks:install' to restore hook permissions." >&2 - exit 1 +if ! command -v pwsh >/dev/null 2>&1; then + echo "Error: pwsh is required to run $hook_name." >&2 + echo "Install PowerShell 7+ and run npm run hooks:install." >&2 + exit 1 fi -exec "$PRE_COMMIT" "$@" +exec pwsh -NoProfile -File "$hook_script" "$@" diff --git a/.githooks/pre-merge-commit.ps1 b/.githooks/pre-merge-commit.ps1 new file mode 100644 index 000000000..c55eaf1f3 --- /dev/null +++ b/.githooks/pre-merge-commit.ps1 @@ -0,0 +1,21 @@ +#!/usr/bin/env pwsh +[CmdletBinding()] +param( + [Parameter(ValueFromRemainingArguments = $true)] + [string[]]$HookArgs +) + +Set-StrictMode -Version Latest +$ErrorActionPreference = 'Stop' + +$hookDir = Split-Path -Parent $MyInvocation.MyCommand.Path +$preCommit = Join-Path $hookDir 'pre-commit.ps1' + +if (-not (Test-Path -LiteralPath $preCommit -PathType Leaf)) { + Write-Host "Error: pre-commit PowerShell implementation not found at $preCommit" -ForegroundColor Red + Write-Host 'Run npm run hooks:install to restore tracked hooks.' -ForegroundColor Cyan + exit 1 +} + +& $preCommit @HookArgs +exit $LASTEXITCODE diff --git a/.githooks/pre-push b/.githooks/pre-push index bcc52f2ee..3fd63843f 100755 --- a/.githooks/pre-push +++ b/.githooks/pre-push @@ -1,612 +1,30 @@ -#!/usr/bin/env bash -set -euo pipefail +#!/usr/bin/env sh +# This hook is a last-resort gate and must stay fast; routine formatting, +# spelling, docs, and broad validation belong in agent preflight and CI. +set -eu -# ============================================================================ -# PRE-PUSH HOOK: Fast validation of pushed changes -# ============================================================================ -# This hook reads stdin to detect which files are being pushed, then runs -# only the relevant checks on changed files. Full-repo validation happens in CI. -# -# Performance: Targets <10s for typical pushes by: -# - Parsing stdin to detect exactly which files changed -# - Running checks only on changed files (not the whole repo) -# - Executing independent checks in parallel -# - Skipping irrelevant checks (e.g. no .cs files changed → skip license audit) -# - Using cached license year data -# -# Checks performed (when relevant files change): -# 1. Prettier formatting (Markdown, JSON, asmdef/asmref, YAML, JS) -# 2. Markdownlint -# 3. Spell checking (cspell) -# 4. Doc link lint -# 5. Gitignore docs safety (no docs/ files accidentally gitignored) -# 6. Unity meta file lint (missing/orphaned .meta files) -# 7. EOL verification -# 8. CHANGELOG structure lint (when CHANGELOG.md changes) -# 9. yamllint (if installed) -# 10. actionlint (if installed and workflow files changed) -# 11. External link check with lychee (if installed) -# 12. Test lifecycle lint -# 13. Duplicate C# using directive lint -# 14. License year audit (with cache) -# 15. #region guard on changed C# files -# 16. Hook pattern tests (if hook files changed) -# 17. LLM instructions validation -# -# To skip in emergencies: git push --no-verify -# (CI will still validate all checks) -# ============================================================================ +hook_name="pre-push" +hook_impl="$hook_name.ps1" +hook_dir="$(CDPATH= cd "$(dirname "$0")" && pwd -P)" +hook_script="$hook_dir/$hook_impl" -# ---- Changed-file detection from stdin ---- -# Pre-push stdin format: -# Multiple lines possible when pushing multiple refs at once. -ZERO_SHA="0000000000000000000000000000000000000000" -HAS_REFS=false -ALL_CHANGED_FILES=() - -array_contains_exact() { - local needle="$1" - shift || true - local item - for item in "$@"; do - if [[ "$item" == "$needle" ]]; then - return 0 +if [ ! -f "$hook_script" ]; then + repo_root="$(git rev-parse --show-toplevel 2>/dev/null || true)" + if [ -n "$repo_root" ] && [ -f "$repo_root/.githooks/$hook_impl" ]; then + hook_script="$repo_root/.githooks/$hook_impl" fi - done - return 1 -} - -add_changed_file() { - local file="$1" - if [[ -z "$file" ]]; then - return 0 - fi - - if ! array_contains_exact "$file" "${ALL_CHANGED_FILES[@]}"; then - ALL_CHANGED_FILES+=("$file") - fi -} - -collect_changed_files() { - local file - while IFS= read -r -d '' file; do - add_changed_file "$file" - done -} - -# Cleanup on exit/interrupt (defined before while read so temp file is always cleaned up) -PID_NODE="" PID_PWSH="" PID_BASH="" -cleanup() { - # Kill background processes if any - for pid in $PID_NODE $PID_PWSH $PID_BASH; do - kill "$pid" 2>/dev/null || true - done -} -trap cleanup EXIT INT TERM - -while read -r _local_ref local_sha _remote_ref remote_sha; do - # Skip delete pushes (local_sha is all zeros) - if [ "$local_sha" = "$ZERO_SHA" ]; then - continue - fi - HAS_REFS=true - - if [ "$remote_sha" = "$ZERO_SHA" ]; then - # New branch: compare against default branch merge-base - merge_base=$(git merge-base main "$local_sha" 2>/dev/null || echo "") - if [ -n "$merge_base" ]; then - collect_changed_files < <(git diff --name-only -z "$merge_base".."$local_sha" 2>/dev/null || true) - else - # No merge-base (orphan branch or no main): compare against the full tree. - collect_changed_files < <(git ls-tree -r -z --name-only "$local_sha" 2>/dev/null || true) - fi - else - # Normal push: diff between remote and local - collect_changed_files < <(git diff --name-only -z "$remote_sha".."$local_sha" 2>/dev/null || true) - fi -done - -# If no refs to push (delete-only) or stdin was empty (no-op), skip all checks -if [ "$HAS_REFS" = "false" ] || [ ${#ALL_CHANGED_FILES[@]} -eq 0 ]; then - echo "No files changed in push, skipping checks." - exit 0 fi -CHANGED_COUNT=${#ALL_CHANGED_FILES[@]} -echo "Pre-push: checking $CHANGED_COUNT changed file(s)..." - -# ---- Pre-compute changed file sets ---- -CHANGED_MD=() -CHANGED_JSON=() -CHANGED_YAML=() -CHANGED_JS=() -CHANGED_CS=() -CHANGED_META=() -CHANGED_TESTS=() -CHANGED_SCRIPTS=() -CHANGED_DOCS=() -CHANGED_LLM=() -CHANGED_WORKFLOWS=() -CHANGED_WIKI=() -CHANGED_HOOK_FILES=() -CHANGED_LINT_TEST=() -CHANGED_LINT_DUPLICATE_USINGS=() -CHANGED_GITIGNORE_DOCS_LINT=() -CHANGED_SYNC_SCRIPT_CONTRACTS=() -CHANGED_LINT_ERROR_CODE_SCRIPTS=() - -for file in "${ALL_CHANGED_FILES[@]}"; do - [[ "$file" =~ \.(md|markdown)$ ]] && CHANGED_MD+=("$file") - [[ "$file" =~ \.(json|jsonc|asmdef|asmref)$ ]] && CHANGED_JSON+=("$file") - [[ "$file" =~ \.(yml|yaml)$ ]] && CHANGED_YAML+=("$file") - [[ "$file" =~ \.js$ ]] && CHANGED_JS+=("$file") - [[ "$file" =~ \.cs$ ]] && CHANGED_CS+=("$file") - [[ "$file" =~ \.meta$ ]] && CHANGED_META+=("$file") - [[ "$file" =~ ^Tests/ ]] && CHANGED_TESTS+=("$file") - [[ "$file" =~ ^scripts/ ]] && CHANGED_SCRIPTS+=("$file") - [[ "$file" =~ ^docs/ ]] && CHANGED_DOCS+=("$file") - [[ "$file" =~ ^\.llm/ ]] && CHANGED_LLM+=("$file") - [[ "$file" =~ ^\.github/workflows/ ]] && CHANGED_WORKFLOWS+=("$file") - [[ "$file" =~ ^(scripts/wiki/.*|scripts/tests/test-wiki-generation\.sh|\.github/workflows/deploy-wiki\.yml)$ ]] && CHANGED_WIKI+=("$file") - [[ "$file" =~ ^(\.githooks/.*|scripts/tests/test-hook-patterns\.sh)$ ]] && CHANGED_HOOK_FILES+=("$file") - [[ "$file" =~ ^(scripts/lint-tests\.ps1|scripts/tests/test-lint-tests\.ps1)$ ]] && CHANGED_LINT_TEST+=("$file") - [[ "$file" =~ ^(scripts/lint-duplicate-usings\.ps1|scripts/tests/test-lint-duplicate-usings\.ps1)$ ]] && CHANGED_LINT_DUPLICATE_USINGS+=("$file") - [[ "$file" =~ ^(scripts/lint-gitignore-docs\.ps1|scripts/tests/test-gitignore-docs\.ps1)$ ]] && CHANGED_GITIGNORE_DOCS_LINT+=("$file") - [[ "$file" =~ ^(scripts/sync-doc-counts\.ps1|scripts/sync-banner-version\.ps1|scripts/lint-cspell-config\.js|scripts/tests/test-sync-script-contracts\.ps1)$ ]] && CHANGED_SYNC_SCRIPT_CONTRACTS+=("$file") - # Run the lint-error-code cspell contract test whenever any file in the - # harvester's scan scope is touched. The harvester in - # scripts/validate-lint-error-codes.ps1 scans THREE roots: - # 1. scripts/lint-*.{ps1,js} -- where new PWS/UNH/DEP - # prefixes are born. - # 2. scripts/tests/test-lint-*.{ps1,js,sh} -- test assertions that cite - # codes (a new prefix may first surface in a test). - # 3. .githooks/* -- hook error messages cite - # codes (e.g. UNH004 in a pre-commit failure block). - # Plus: cspell.json itself (prefix registry changes) and the validator / - # its regression test. The `.githooks/` anchor uses `^\.githooks/` to avoid - # accidentally matching the similarly-prefixed `.github/` directory. - [[ "$file" =~ ^(scripts/lint-[^/]+\.(ps1|js)|scripts/tests/test-lint-[^/]+\.(ps1|js|sh)|\.githooks/[^/]+|scripts/validate-lint-error-codes\.ps1|scripts/tests/test-validate-lint-error-codes\.ps1|cspell\.json)$ ]] && CHANGED_LINT_ERROR_CODE_SCRIPTS+=("$file") -done - -CHANGED_PRETTIER=("${CHANGED_MD[@]}" "${CHANGED_JSON[@]}" "${CHANGED_YAML[@]}" "${CHANGED_JS[@]}") -# Spell-check scope MUST mirror pre-commit's SPELL_FILES_ARRAY (MD + JSON + -# YAML + JS + CS). Keeping the two hooks in strict parity prevents the class -# of failure where a commit slips past pre-commit (different host, wrong -# hooksPath, --no-verify) and only surfaces at pre-push with a narrower -# safety net. cspell.json's `files` glob still governs which of these are -# actually linted, so widening the pass-through list is free speed-wise. -CHANGED_SPELL=("${CHANGED_MD[@]}" "${CHANGED_JSON[@]}" "${CHANGED_YAML[@]}" "${CHANGED_JS[@]}" "${CHANGED_CS[@]}") -CHANGED_EOL=("${ALL_CHANGED_FILES[@]}") - -# ---- Track parallel job failures ---- -HOOK_FAILED=0 - -# ---- Group A: Node-based checks (run in parallel) ---- -run_node_checks() { - run_node_tool() { - node scripts/run-node-bin.js "$@" - } - - require_node() { - local purpose="$1" - if ! command -v node >/dev/null 2>&1; then - echo "Node.js is required for $purpose. Install Node.js/npm and run npm install." >&2 - return 1 - fi - } - - require_node_tool() { - local tool="$1" - local purpose="$2" - require_node "$purpose" || return 1 - if ! run_node_tool "$tool" --version >/dev/null 2>&1; then - echo "Repo-local $tool is unavailable for $purpose. Run 'npm install' on the same host that runs git hooks." >&2 - return 1 - fi - } - - # 1) Prettier (validation only — no auto-fix in pre-push) - if [ ${#CHANGED_PRETTIER[@]} -gt 0 ]; then - require_node_tool prettier "Prettier validation" || return 1 - echo "Checking formatting on changed files..." - if ! node scripts/run-prettier.js --check -- "${CHANGED_PRETTIER[@]}"; then - echo "" - echo "=== Prettier formatting issues detected ===" - echo "Run: node scripts/run-prettier.js --write -- " - echo "Then commit and push again." - echo "" - return 1 - fi - echo "✓ Prettier formatting OK" - fi - - # 2) Markdownlint - if [ ${#CHANGED_MD[@]} -gt 0 ]; then - require_node_tool markdownlint "markdownlint validation" || return 1 - run_node_tool markdownlint --config .markdownlint.json --ignore-path .markdownlintignore -- "${CHANGED_MD[@]}" || return 1 - fi - - # 3) Spell checking (only if spell-relevant files changed) - if [ ${#CHANGED_SPELL[@]} -gt 0 ]; then - require_node_tool cspell "spell checking" || return 1 - echo "Checking spelling on changed files..." - # Capture cspell output via a temp file so we can BOTH surface the raw - # diagnostics (tee to stderr) AND post-process them for lint-error-code - # prefixes that need cspell registration. Keeping the tee ensures the - # developer still sees file:line:column diagnostics. - PREPUSH_SPELL_CAPTURE="$(mktemp 2>/dev/null || true)" - if [ -z "$PREPUSH_SPELL_CAPTURE" ]; then - echo "Error: Failed to create temporary file for spell check output." >&2 - return 1 - fi - # AR-1: trap-based cleanup guarantees this temp is deleted on every exit - # path from run_node_checks — including signals. Scoped to RETURN so - # failures downstream in the function still clean up. - # shellcheck disable=SC2064 # We intend immediate expansion of the temp path. - trap "rm -f '$PREPUSH_SPELL_CAPTURE'" RETURN - # Explicit-exit-code form mirrors pre-commit's P0-1 fix: capture exit - # status separately from the pipeline so a future revert of pipefail - # cannot silently mask a failure. pre-push has `set -euo pipefail` - # today, but defense-in-depth is cheap here. - PREPUSH_SPELL_EXIT=0 - run_node_tool cspell lint --no-must-find-files --no-progress --show-suggestions -- "${CHANGED_SPELL[@]}" >"$PREPUSH_SPELL_CAPTURE" 2>&1 || PREPUSH_SPELL_EXIT=$? - cat "$PREPUSH_SPELL_CAPTURE" - if [ "$PREPUSH_SPELL_EXIT" -ne 0 ]; then - echo "" >&2 - echo "=== Spelling errors detected ===" >&2 - echo "Fix typos or add valid terms to the appropriate dictionary in cspell.json:" >&2 - echo " unity-terms -- Unity Engine APIs, components, lifecycle" >&2 - echo " csharp-terms -- C# language features, .NET types" >&2 - echo " package-terms -- This package's public API and type names" >&2 - echo " tech-terms -- General programming/tooling terms (CRLF, NUL, ASCII, etc.)" >&2 - echo " words (root) -- Project-specific words that don't fit a category" >&2 - echo "" >&2 - # Synthesize a copy-pasteable patch when the failure is specifically an - # unregistered lint-error-code-shaped token. This makes the error - # message itself the fix rather than a pointer to documentation. - # Width note: unbounded upper bound (`{2,}`) mirrors the pre-commit - # P0-3 widening so prefixes longer than 5 chars are still surfaced. - UNKNOWN_CODE_PREFIXES="$(grep -oE 'Unknown word \([A-Z]{2,}\)' "$PREPUSH_SPELL_CAPTURE" 2>/dev/null | grep -oE '[A-Z]{2,}' | sort -u || true)" - if [ -n "$UNKNOWN_CODE_PREFIXES" ]; then - echo "=== Detected unregistered lint-error-code prefix(es) ===" >&2 - echo "Copy-paste this patch into the root 'words' array in cspell.json" >&2 - echo "(append each quoted prefix as a new array element):" >&2 - echo "" >&2 - while IFS= read -r prefix; do - [ -z "$prefix" ] && continue - echo " \"$prefix\"," >&2 - done <<< "$UNKNOWN_CODE_PREFIXES" - echo "" >&2 - echo "See scripts/validate-lint-error-codes.ps1 for the contract that" >&2 - echo "enforces this requirement once the prefix is registered." >&2 - echo "" >&2 - fi - echo "Re-run locally: npm run lint:spelling" >&2 - echo "" >&2 - return 1 - fi - echo "✓ Spell check OK" - - # 3b) cspell.json config lint - if array_contains_exact "cspell.json" "${ALL_CHANGED_FILES[@]}"; then - echo "Checking cspell.json configuration..." - if ! node scripts/lint-cspell-config.js; then - echo "" - echo "=== cspell.json configuration issues ===" - echo "Run: npm run lint:spelling:config:fix" - echo "" - return 1 - fi - echo "✓ cspell.json config OK" - fi - fi - - # 4) Doc link lint (only if docs or markdown changed) - if [ ${#CHANGED_MD[@]} -gt 0 ] || [ ${#CHANGED_DOCS[@]} -gt 0 ] || [ ${#CHANGED_LLM[@]} -gt 0 ]; then - require_node "documentation link lint" || return 1 - echo "Checking documentation links..." - node ./scripts/run-doc-link-lint.js || return 1 - echo "✓ Doc links OK" - fi - - return 0 -} - -# ---- Group B: PowerShell-based checks (run in parallel) ---- -run_pwsh_checks() { - PWSH_CMD=() - if command -v pwsh >/dev/null 2>&1; then - PWSH_CMD=(pwsh -NoProfile -File) - elif command -v powershell >/dev/null 2>&1; then - PWSH_CMD=(powershell -NoProfile -ExecutionPolicy Bypass -File) - else - echo "PowerShell not found; skipping PowerShell checks. CI will validate." >&2 - return 0 - fi - - # 5) Gitignore docs safety (repo-wide, but batched & fast) - if [ ${#CHANGED_DOCS[@]} -gt 0 ] || [ ${#CHANGED_LLM[@]} -gt 0 ] || array_contains_exact ".gitignore" "${ALL_CHANGED_FILES[@]}"; then - echo "Checking gitignore docs safety..." - "${PWSH_CMD[@]}" scripts/lint-gitignore-docs.ps1 || { - echo "" - echo "=== Gitignore docs safety check failed ===" - echo "Fix .gitignore patterns to avoid excluding documentation files." - echo "" - return 1 - } - echo "✓ Gitignore docs safety OK" - fi - - # 6) Unity meta file lint (repo-wide, but optimized via git ls-files) - if [ ${#CHANGED_CS[@]} -gt 0 ] || [ ${#CHANGED_META[@]} -gt 0 ] || [ ${#CHANGED_DOCS[@]} -gt 0 ] || [ ${#CHANGED_SCRIPTS[@]} -gt 0 ]; then - echo "Checking Unity meta files..." - "${PWSH_CMD[@]}" scripts/lint-meta-files.ps1 || { - echo "" - echo "=== Unity meta file lint failed ===" - echo "Run './scripts/generate-meta.sh ' to generate missing meta files." - echo "" - return 1 - } - echo "✓ Unity meta files OK" - fi - - # 7) EOL verification (validation only — no normalize in pre-push) - if [ ${#CHANGED_EOL[@]} -gt 0 ]; then - "${PWSH_CMD[@]}" scripts/check-eol.ps1 -VerboseOutput -Paths "${CHANGED_EOL[@]}" || { - echo "" - echo "=== EOL check failed ===" - echo "Run: npm run fix:eol" - echo "" - return 1 - } - fi - - # 8) CHANGELOG lint when CHANGELOG.md is part of the push - if array_contains_exact "CHANGELOG.md" "${ALL_CHANGED_FILES[@]}"; then - echo "Checking CHANGELOG structure..." - "${PWSH_CMD[@]}" scripts/lint-changelog.ps1 || { - echo "" - echo "=== CHANGELOG lint failed ===" - echo "Fix CHANGELOG.md structure issues before pushing." - echo "" - return 1 - } - echo "✓ CHANGELOG structure OK" - fi - - # 12) Test lifecycle lint (only if test files changed) - if [ ${#CHANGED_TESTS[@]} -gt 0 ]; then - "${PWSH_CMD[@]}" scripts/lint-tests.ps1 -VerboseOutput -Paths "${CHANGED_TESTS[@]}" || { - echo "" - echo "=== Test lint failed ===" - echo "Fix the issues or add // UNH-SUPPRESS comments for valid exceptions." - echo "" - return 1 - } - fi - - # 13) Duplicate using directive lint (only if C# files changed) - if [ ${#CHANGED_CS[@]} -gt 0 ]; then - echo "Checking duplicate C# using directives..." - "${PWSH_CMD[@]}" scripts/lint-duplicate-usings.ps1 -Paths "${CHANGED_CS[@]}" || { - echo "" - echo "=== Duplicate using directive lint failed (UNH007) ===" - echo "Remove duplicate using directives within each namespace/file scope." - echo "" - return 1 - } - echo "✓ Duplicate using directives OK" - fi - - # LLM instructions validation (only if .llm or skills changed) - if [ ${#CHANGED_LLM[@]} -gt 0 ]; then - echo "Validating LLM instructions..." - "${PWSH_CMD[@]}" scripts/lint-llm-instructions.ps1 || { - echo "" - echo "=== LLM instructions validation failed ===" - echo "Run: pwsh -NoProfile -File scripts/lint-llm-instructions.ps1 -Fix" - echo "" - return 1 - } - echo "✓ LLM instructions OK" - fi - - return 0 -} - -# ---- Group C: Bash/native checks (run in parallel) ---- -run_bash_checks() { - # 14) License year audit (only if .cs files changed, uses cache) - if [ ${#CHANGED_CS[@]} -gt 0 ]; then - echo "Checking license year headers..." - if ! bash scripts/audit-license-years.sh --summary --paths "${CHANGED_CS[@]}"; then - echo "" - echo "=== License year audit failed ===" - echo "To auto-fix: bash scripts/update-license-headers.sh" - echo "" - return 1 - fi - echo "✓ License year headers OK" - fi - - # 15) Check for #region directives (only in changed .cs files) - if [ ${#CHANGED_CS[@]} -gt 0 ]; then - echo "Checking for forbidden #region directives..." - REGION_VIOLATIONS=$(grep -E -n -i '^[[:space:]]*#[[:space:]]*(region|endregion)' -- "${CHANGED_CS[@]}" 2>/dev/null || true) - if [ -n "$REGION_VIOLATIONS" ]; then - echo "" - echo "=== Error: C# regions (#region/#endregion) are forbidden ===" - echo "$REGION_VIOLATIONS" | head -50 - echo "" - echo "Remove all #region and #endregion directives before pushing." - echo "" - return 1 - fi - echo "✓ No #region directives found" - fi - - # Optional YAML lint (only if YAML files changed) - if [ ${#CHANGED_YAML[@]} -gt 0 ]; then - if command -v yamllint >/dev/null 2>&1; then - echo "Running yamllint on changed YAML files..." - yamllint -c .yamllint.yaml -- "${CHANGED_YAML[@]}" || return 1 - echo "✓ yamllint OK" - fi - fi - - # Optional external link check (only if markdown changed) - if [ ${#CHANGED_MD[@]} -gt 0 ]; then - if command -v lychee >/dev/null 2>&1; then - lychee -c .lychee.toml --no-progress --verbose -- "${CHANGED_MD[@]}" || return 1 - fi - fi - - return 0 -} - -# ---- Conditional test suites (run sequentially, only when relevant) ---- -run_conditional_tests() { - # Wiki generation tests - if [ ${#CHANGED_WIKI[@]} -gt 0 ]; then - echo "Running wiki generation tests..." - bash scripts/tests/test-wiki-generation.sh || return 1 - echo "✓ Wiki generation tests OK" - - if command -v python3 >/dev/null 2>&1 && python3 -c "import pytest" 2>/dev/null; then - python3 -m pytest scripts/wiki/test_wiki_scripts.py -v || return 1 - echo "✓ Python wiki tests OK" - fi - fi - - # Lint test suite - if [ ${#CHANGED_LINT_TEST[@]} -gt 0 ]; then - PWSH_CMD=() - if command -v pwsh >/dev/null 2>&1; then - PWSH_CMD=(pwsh -NoProfile -File) - elif command -v powershell >/dev/null 2>&1; then - PWSH_CMD=(powershell -NoProfile -ExecutionPolicy Bypass -File) - fi - if [ ${#PWSH_CMD[@]} -gt 0 ]; then - echo "Running lint-tests.ps1 test suite..." - "${PWSH_CMD[@]}" scripts/tests/test-lint-tests.ps1 || return 1 - echo "✓ Lint test suite OK" - fi - fi - - # Duplicate-using linter test suite - if [ ${#CHANGED_LINT_DUPLICATE_USINGS[@]} -gt 0 ]; then - PWSH_CMD=() - if command -v pwsh >/dev/null 2>&1; then - PWSH_CMD=(pwsh -NoProfile -File) - elif command -v powershell >/dev/null 2>&1; then - PWSH_CMD=(powershell -NoProfile -ExecutionPolicy Bypass -File) - fi - if [ ${#PWSH_CMD[@]} -gt 0 ]; then - echo "Running lint-duplicate-usings.ps1 test suite..." - "${PWSH_CMD[@]}" scripts/tests/test-lint-duplicate-usings.ps1 || return 1 - echo "✓ Duplicate-using linter test suite OK" - fi - fi - - # Gitignore docs lint test suite - if [ ${#CHANGED_GITIGNORE_DOCS_LINT[@]} -gt 0 ]; then - PWSH_CMD=() - if command -v pwsh >/dev/null 2>&1; then - PWSH_CMD=(pwsh -NoProfile -File) - elif command -v powershell >/dev/null 2>&1; then - PWSH_CMD=(powershell -NoProfile -ExecutionPolicy Bypass -File) - fi - if [ ${#PWSH_CMD[@]} -gt 0 ]; then - echo "Running lint-gitignore-docs.ps1 test suite..." - "${PWSH_CMD[@]}" scripts/tests/test-gitignore-docs.ps1 || return 1 - echo "✓ Gitignore docs lint test suite OK" - fi - fi - - # Hook pattern tests - if [ ${#CHANGED_HOOK_FILES[@]} -gt 0 ]; then - echo "Running hook pattern tests..." - bash scripts/tests/test-hook-patterns.sh || return 1 - echo "✓ Hook pattern tests OK" - fi - - # Sync script and cspell contract tests - if [ ${#CHANGED_SYNC_SCRIPT_CONTRACTS[@]} -gt 0 ]; then - PWSH_CMD=() - if command -v pwsh >/dev/null 2>&1; then - PWSH_CMD=(pwsh -NoProfile -File) - elif command -v powershell >/dev/null 2>&1; then - PWSH_CMD=(powershell -NoProfile -ExecutionPolicy Bypass -File) - fi - if [ ${#PWSH_CMD[@]} -gt 0 ]; then - echo "Running sync script contract tests..." - "${PWSH_CMD[@]}" scripts/tests/test-sync-script-contracts.ps1 || return 1 - echo "✓ Sync script contract tests OK" - fi - fi - - # Lint-error-code cspell contract: when any lint-*.ps1|.js, cspell.json, - # the validator, or its test changes, enforce that every lint-error-code - # prefix emitted by the lint family is registered with cspell. This closes - # the loop that let the PWS001/PWS002 regression ship in April 2026. - if [ ${#CHANGED_LINT_ERROR_CODE_SCRIPTS[@]} -gt 0 ]; then - PWSH_CMD=() - if command -v pwsh >/dev/null 2>&1; then - PWSH_CMD=(pwsh -NoProfile -File) - elif command -v powershell >/dev/null 2>&1; then - PWSH_CMD=(powershell -NoProfile -ExecutionPolicy Bypass -File) - fi - if [ ${#PWSH_CMD[@]} -gt 0 ]; then - echo "Validating lint-error-code cspell coverage..." - "${PWSH_CMD[@]}" scripts/validate-lint-error-codes.ps1 || return 1 - echo "✓ Lint-error-code cspell coverage OK" - - echo "Running lint-error-code validator test suite..." - "${PWSH_CMD[@]}" scripts/tests/test-validate-lint-error-codes.ps1 || return 1 - echo "✓ Lint-error-code validator test suite OK" - fi - fi - - # actionlint (only if workflow files changed) - if [ ${#CHANGED_WORKFLOWS[@]} -gt 0 ]; then - if command -v actionlint >/dev/null 2>&1; then - echo "Running actionlint..." - actionlint || return 1 - echo "✓ actionlint OK" - fi - fi - - return 0 -} - -# ---- Execute parallel groups ---- -# Run Groups A, B, C in parallel with proper error propagation -run_node_checks & -PID_NODE=$! -run_pwsh_checks & -PID_PWSH=$! -run_bash_checks & -PID_BASH=$! - -# Wait for all groups, collect exit codes -wait $PID_NODE || HOOK_FAILED=1 -wait $PID_PWSH || HOOK_FAILED=1 -wait $PID_BASH || HOOK_FAILED=1 - -# Run conditional tests sequentially (they depend on having seen parallel output) -if [ $HOOK_FAILED -eq 0 ]; then - run_conditional_tests || HOOK_FAILED=1 +if [ ! -f "$hook_script" ]; then + echo "Error: $hook_impl not found at $hook_script" >&2 + echo "Run npm run hooks:install to restore tracked hooks." >&2 + exit 1 fi -# ============================================================================ -# FINAL RESULT -# ============================================================================ -if [ $HOOK_FAILED -ne 0 ]; then - echo "" - echo "Pre-push checks FAILED. Fix the issues above and push again." - echo "To skip in emergencies: git push --no-verify (CI will still validate)" - exit 1 +if ! command -v pwsh >/dev/null 2>&1; then + echo "Error: pwsh is required to run $hook_name." >&2 + echo "Install PowerShell 7+ and run npm run hooks:install." >&2 + exit 1 fi -echo "" -echo "Pre-push checks complete. Push proceeding..." +exec pwsh -NoProfile -File "$hook_script" "$@" diff --git a/.githooks/pre-push.ps1 b/.githooks/pre-push.ps1 new file mode 100644 index 000000000..ba2f26a86 --- /dev/null +++ b/.githooks/pre-push.ps1 @@ -0,0 +1,554 @@ +#!/usr/bin/env pwsh +[CmdletBinding()] +param( + [Parameter(ValueFromRemainingArguments = $true)] + [string[]]$HookArgs +) + +Set-StrictMode -Version Latest +$ErrorActionPreference = 'Stop' + +$scriptHookDir = Split-Path -Parent $MyInvocation.MyCommand.Path +$repoRootResult = (& git rev-parse --show-toplevel 2>$null) +$repoRoot = if ($LASTEXITCODE -eq 0 -and -not [string]::IsNullOrWhiteSpace($repoRootResult)) { + ([string]$repoRootResult).Trim() +} +else { + (Resolve-Path -LiteralPath (Join-Path $scriptHookDir '..')).Path +} +$hookDir = Join-Path $repoRoot '.githooks' +$script:TreeRegionCheckShas = [System.Collections.Generic.HashSet[string]]::new([System.StringComparer]::Ordinal) +$script:RegionRecoveryPaths = [System.Collections.Generic.HashSet[string]]::new([System.StringComparer]::Ordinal) +$script:RegionPattern = '^[[:space:]]*#[[:space:]]*(region|endregion)' + +function Write-HookInfo { + param([string]$Message) + Write-Host "[pre-push] $Message" +} + +function Write-HookError { + param([string]$Message) + Write-Host "[pre-push] ERROR: $Message" -ForegroundColor Red +} + +function Invoke-Native { + param( + [Parameter(Mandatory = $true)] + [string]$FileName, + [Parameter(Mandatory = $true)] + [string[]]$Arguments, + [AllowNull()] + [string]$StandardInput = $null, + [switch]$AllowFailure + ) + + $startInfo = [System.Diagnostics.ProcessStartInfo]::new() + $startInfo.FileName = $FileName + foreach ($argument in $Arguments) { + [void]$startInfo.ArgumentList.Add($argument) + } + $startInfo.RedirectStandardOutput = $true + $startInfo.RedirectStandardError = $true + $startInfo.RedirectStandardInput = $null -ne $StandardInput + $startInfo.UseShellExecute = $false + $startInfo.WorkingDirectory = $repoRoot + + $process = [System.Diagnostics.Process]::new() + $process.StartInfo = $startInfo + [void]$process.Start() + if ($null -ne $StandardInput) { + $process.StandardInput.Write($StandardInput) + $process.StandardInput.Close() + } + $stdout = $process.StandardOutput.ReadToEnd() + $stderr = $process.StandardError.ReadToEnd() + $process.WaitForExit() + + if ($process.ExitCode -ne 0 -and -not $AllowFailure) { + if (-not [string]::IsNullOrWhiteSpace($stderr)) { + Write-Host $stderr.TrimEnd() -ForegroundColor Yellow + } + throw "$FileName exited with code $($process.ExitCode): $($Arguments -join ' ')" + } + + return [pscustomobject]@{ + ExitCode = $process.ExitCode + Stdout = $stdout + Stderr = $stderr + } +} + +function Invoke-Git { + param( + [Parameter(Mandatory = $true)] + [string[]]$Arguments, + [AllowNull()] + [string]$StandardInput = $null, + [switch]$AllowFailure + ) + + return Invoke-Native -FileName 'git' -Arguments $Arguments -StandardInput $StandardInput -AllowFailure:$AllowFailure +} + +function Test-ZeroObjectId { + param([AllowNull()][string]$ObjectId) + + return (-not [string]::IsNullOrWhiteSpace($ObjectId) -and $ObjectId -match '^0+$') +} + +function Split-NulList { + param([AllowNull()][string]$Text) + + if ([string]::IsNullOrEmpty($Text)) { + return @() + } + + return @( + $Text -split ([string][char]0) | + Where-Object { -not [string]::IsNullOrWhiteSpace($_) } | + ForEach-Object { ([string]$_).Replace('\', '/') } + ) +} + +function ConvertTo-LiteralPathspec { + param([Parameter(Mandatory = $true)][string]$Path) + + return ":(literal)$Path" +} + +function ConvertFrom-GitGrepRegionOutput { + param( + [AllowNull()] + [string]$Text, + [AllowNull()] + [string]$TreeId = $null, + [AllowNull()] + [System.Collections.Generic.HashSet[string]]$AllowedPaths = $null, + [switch]$AsObjects + ) + + $violations = [System.Collections.Generic.List[object]]::new() + if ([string]::IsNullOrEmpty($Text)) { + return $violations + } + + $cursor = 0 + while ($cursor -lt $Text.Length) { + $pathEnd = $Text.IndexOf([char]0, $cursor) + if ($pathEnd -lt 0) { + break + } + + $lineEnd = $Text.IndexOf([char]0, $pathEnd + 1) + if ($lineEnd -lt 0) { + break + } + + $textEnd = $Text.IndexOf("`n", $lineEnd + 1) + if ($textEnd -lt 0) { + $textEnd = $Text.Length + } + + $path = $Text.Substring($cursor, $pathEnd - $cursor) + if (-not [string]::IsNullOrWhiteSpace($TreeId) -and $path.StartsWith("${TreeId}:", [System.StringComparison]::Ordinal)) { + $path = $path.Substring($TreeId.Length + 1) + } + $lineNumber = $Text.Substring($pathEnd + 1, $lineEnd - $pathEnd - 1) + $lineText = $Text.Substring($lineEnd + 1, $textEnd - $lineEnd - 1).TrimEnd("`r") + if ($null -eq $AllowedPaths -or $AllowedPaths.Contains($path)) { + if ($AsObjects) { + $violations.Add([pscustomobject]@{ + Path = $path + Text = "${path}:${lineNumber}:$lineText" + }) | Out-Null + } + else { + $violations.Add("${path}:${lineNumber}:$lineText") | Out-Null + } + } + + $cursor = $textEnd + 1 + } + + return $violations +} + +function Remove-GitIgnoredHookArtifacts { + $rootArtifactExtensions = @('txt', 'out', 'err') + $hookArtifactExtensions = @('txt', 'log', 'out', 'err', 'tmp') + $artifactExtensions = @($rootArtifactExtensions + $hookArtifactExtensions | Sort-Object -Unique) + $hookNames = [System.Collections.Generic.HashSet[string]]::new([System.StringComparer]::Ordinal) + if (-not (Test-Path -LiteralPath $hookDir -PathType Container)) { + return + } + + foreach ($hook in Get-ChildItem -LiteralPath $hookDir -File -ErrorAction SilentlyContinue) { + if ($hook.Name -like '*.sample' -or $artifactExtensions -contains $hook.Extension.TrimStart('.')) { + continue + } + + $name = if ($hook.Extension) { [System.IO.Path]::GetFileNameWithoutExtension($hook.Name) } else { $hook.Name } + if (-not [string]::IsNullOrWhiteSpace($name)) { + [void]$hookNames.Add($name) + } + } + + $candidates = [System.Collections.Generic.List[string]]::new() + foreach ($hookName in $hookNames) { + foreach ($extension in $rootArtifactExtensions) { + $relative = "$hookName.$extension" + if (Test-Path -LiteralPath (Join-Path $repoRoot $relative) -PathType Leaf) { + $candidates.Add($relative) | Out-Null + } + } + foreach ($extension in $hookArtifactExtensions) { + $relative = ".githooks/$hookName.$extension" + if (Test-Path -LiteralPath (Join-Path $repoRoot $relative) -PathType Leaf) { + $candidates.Add($relative) | Out-Null + } + } + } + + $uniqueCandidates = @($candidates | Sort-Object -Unique) + if ($uniqueCandidates.Count -eq 0) { + return + } + + $unsafe = [System.Collections.Generic.List[string]]::new() + foreach ($relative in $uniqueCandidates) { + $check = Invoke-Git -Arguments @('check-ignore', '-q', '--', $relative) -AllowFailure + if ($check.ExitCode -eq 0) { + Remove-Item -LiteralPath (Join-Path $repoRoot $relative) -Force + Write-HookInfo "Removed stray hook artifact: $relative" + } + else { + $unsafe.Add($relative) | Out-Null + } + } + + if ($unsafe.Count -gt 0) { + Write-HookError 'Stray hook-output artifact file(s) were not confirmed gitignored:' + foreach ($relative in $unsafe) { + Write-Host " $relative" -ForegroundColor Yellow + } + Write-Host 'Add the artifact pattern to .gitignore or remove the stale file, then retry.' -ForegroundColor Cyan + exit 1 + } +} + +function Add-UniquePath { + param( + [Parameter(Mandatory = $true)] + [AllowEmptyCollection()] + [System.Collections.Generic.HashSet[string]]$Set, + [Parameter(Mandatory = $true)] + [string]$Path + ) + + if (-not [string]::IsNullOrWhiteSpace($Path)) { + [void]$Set.Add($Path.Replace('\', '/')) + } +} + +function Get-NewBranchBaseSha { + param( + [Parameter(Mandatory = $true)] + [string]$LocalSha + ) + + $baseCandidates = [System.Collections.Generic.List[string]]::new() + $upstream = Invoke-Git -Arguments @('rev-parse', '--abbrev-ref', '--symbolic-full-name', '@{upstream}') -AllowFailure + if ($upstream.ExitCode -eq 0 -and -not [string]::IsNullOrWhiteSpace($upstream.Stdout)) { + $baseCandidates.Add($upstream.Stdout.Trim()) | Out-Null + } + + $originHead = Invoke-Git -Arguments @('symbolic-ref', '--quiet', '--short', 'refs/remotes/origin/HEAD') -AllowFailure + if ($originHead.ExitCode -eq 0 -and -not [string]::IsNullOrWhiteSpace($originHead.Stdout)) { + $baseCandidates.Add($originHead.Stdout.Trim()) | Out-Null + } + + foreach ($candidate in @('origin/main', 'origin/master', 'main', 'master')) { + $baseCandidates.Add($candidate) | Out-Null + } + + foreach ($candidate in @($baseCandidates | Sort-Object -Unique)) { + $candidateRev = Invoke-Git -Arguments @('rev-parse', '--verify', "$candidate^{commit}") -AllowFailure + if ($candidateRev.ExitCode -ne 0 -or [string]::IsNullOrWhiteSpace($candidateRev.Stdout)) { + continue + } + + if ($candidateRev.Stdout.Trim() -eq $LocalSha) { + continue + } + + $mergeBase = Invoke-Git -Arguments @('merge-base', $candidate, $LocalSha) -AllowFailure + if ($mergeBase.ExitCode -eq 0 -and -not [string]::IsNullOrWhiteSpace($mergeBase.Stdout)) { + $baseSha = $mergeBase.Stdout.Trim() + if ($baseSha -ne $LocalSha) { + return $baseSha + } + } + } + + return $null +} + +function Get-RegionChangedPathsForRange { + param( + [Parameter(Mandatory = $true)] + [string]$BaseSha, + [Parameter(Mandatory = $true)] + [string]$LocalSha + ) + + $range = "$BaseSha..$LocalSha" + $diff = Invoke-Git -Arguments @( + 'diff', + '--name-only', + '-z', + '--diff-filter=ACMRTUXB', + '-G', + $script:RegionPattern, + $range, + '--', + '*.cs' + ) -AllowFailure + if ($diff.ExitCode -gt 1) { + throw "git diff failed while checking pushed C# region changes." + } + + return Split-NulList -Text $diff.Stdout +} + +function Get-RegionChangedPathsForRefUpdate { + param( + [Parameter(Mandatory = $true)] + [string]$LocalSha, + [Parameter(Mandatory = $true)] + [string]$RemoteSha + ) + + if (Test-ZeroObjectId -ObjectId $RemoteSha) { + $baseSha = Get-NewBranchBaseSha -LocalSha $LocalSha + if ([string]::IsNullOrWhiteSpace($baseSha)) { + [void]$script:TreeRegionCheckShas.Add($LocalSha) + return @() + } + + return Get-RegionChangedPathsForRange -BaseSha $baseSha -LocalSha $LocalSha + } + + return Get-RegionChangedPathsForRange -BaseSha $RemoteSha -LocalSha $LocalSha +} + +function Write-AgentPreflightFixHint { + param([string[]]$Paths) + + $pathListResult = Invoke-Git -Arguments @('rev-parse', '--git-path', 'pre-push-agent-preflight-paths.bin') -AllowFailure + $pathList = if ($pathListResult.ExitCode -eq 0 -and -not [string]::IsNullOrWhiteSpace($pathListResult.Stdout)) { + $pathListResult.Stdout.Trim() + } + else { + Join-Path $repoRoot '.git/pre-push-agent-preflight-paths.bin' + } + + $payload = if ($Paths.Count -gt 0) { ($Paths -join ([string][char]0)) + ([string][char]0) } else { '' } + [System.IO.File]::WriteAllBytes($pathList, [System.Text.UTF8Encoding]::new($false).GetBytes($payload)) + + Write-Host 'Automated recovery (path-scoped):' + Write-Host " npm run agent:preflight:fix -- -PathList `"$pathList`"" + Write-Host 'Then commit generated fixes and push again.' +} + +function Test-RegionChangesInPushedCSharp { + param( + [Parameter(Mandatory = $true)] + [hashtable]$ShaToPaths + ) + + $violations = [System.Collections.Generic.List[string]]::new() + $chunkSize = 200 + foreach ($sha in $ShaToPaths.Keys) { + $changedPaths = @($ShaToPaths[$sha] | Sort-Object -Unique) + $pathspecs = @( + $changedPaths | + Where-Object { -not [string]::IsNullOrWhiteSpace($_) } | + ForEach-Object { ConvertTo-LiteralPathspec -Path $_ } + ) + if ($pathspecs.Count -eq 0) { + continue + } + + for ($offset = 0; $offset -lt $pathspecs.Count; $offset += $chunkSize) { + $end = [Math]::Min($offset + $chunkSize - 1, $pathspecs.Count - 1) + $chunk = @($pathspecs[$offset..$end]) + $grep = Invoke-Git -Arguments (@('grep', '-n', '-I', '-E', '-z', $script:RegionPattern, $sha, '--') + $chunk) -AllowFailure + if ($grep.ExitCode -eq 0) { + foreach ($violation in @(ConvertFrom-GitGrepRegionOutput -Text $grep.Stdout -TreeId $sha -AsObjects)) { + $violations.Add($violation.Text) | Out-Null + Add-UniquePath -Set $script:RegionRecoveryPaths -Path $violation.Path + } + } + elseif ($grep.ExitCode -gt 1) { + throw "git grep failed while checking pushed C# region changes." + } + } + } + + return Write-RegionViolations -Violations $violations +} + +function Test-RegionsInPushedCSharpTree { + param( + [Parameter(Mandatory = $true)] + [string[]]$Shas + ) + + $violations = [System.Collections.Generic.List[string]]::new() + foreach ($sha in @($Shas | Sort-Object -Unique)) { + $grep = Invoke-Git -Arguments @('grep', '-n', '-I', '-E', '-z', $script:RegionPattern, $sha, '--', '*.cs') -AllowFailure + if ($grep.ExitCode -eq 0) { + foreach ($violation in @(ConvertFrom-GitGrepRegionOutput -Text $grep.Stdout -TreeId $sha -AsObjects)) { + $violations.Add($violation.Text) | Out-Null + Add-UniquePath -Set $script:RegionRecoveryPaths -Path $violation.Path + } + } + elseif ($grep.ExitCode -gt 1) { + throw "git grep failed while checking pushed C# tree." + } + } + + return Write-RegionViolations -Violations $violations +} + +function Write-RegionViolations { + param( + [Parameter(Mandatory = $true)] + [AllowEmptyCollection()] + [System.Collections.Generic.List[string]]$Violations + ) + + if ($Violations.Count -eq 0) { + return $true + } + + Write-Host '' + Write-HookError 'C# regions (#region/#endregion) are forbidden.' + foreach ($violation in @($Violations | Select-Object -First 50)) { + Write-Host " $violation" -ForegroundColor Yellow + } + Write-Host '' + Write-Host 'Remove all #region and #endregion directives before pushing.' -ForegroundColor Cyan + Write-Host '' + return $false +} + +try { + Push-Location $repoRoot + Remove-GitIgnoredHookArtifacts + + $stdin = [Console]::In.ReadToEnd() + $lines = @($stdin -split "`r?`n" | Where-Object { -not [string]::IsNullOrWhiteSpace($_) }) + if ($lines.Count -eq 0) { + Write-HookInfo 'No files requiring pre-push checks, skipping.' + exit 0 + } + + $hasRefs = $false + $hasRelevantChanges = $false + $allChanged = [System.Collections.Generic.HashSet[string]]::new([System.StringComparer]::Ordinal) + $shaToCSharp = @{} + + foreach ($line in $lines) { + $parts = @($line -split '\s+') + if ($parts.Count -lt 4) { + continue + } + + $localSha = $parts[1] + $remoteSha = $parts[3] + if (Test-ZeroObjectId -ObjectId $localSha) { + continue + } + + $hasRefs = $true + $changedPaths = @(Get-RegionChangedPathsForRefUpdate -LocalSha $localSha -RemoteSha $remoteSha) + if ($script:TreeRegionCheckShas.Contains($localSha)) { + $hasRelevantChanges = $true + if (-not $shaToCSharp.ContainsKey($localSha)) { + $shaToCSharp[$localSha] = [System.Collections.Generic.List[string]]::new() + } + } + foreach ($path in $changedPaths) { + Add-UniquePath -Set $allChanged -Path $path + if ($path -like '*.cs') { + $hasRelevantChanges = $true + if (-not $shaToCSharp.ContainsKey($localSha)) { + $shaToCSharp[$localSha] = [System.Collections.Generic.List[string]]::new() + } + $shaToCSharp[$localSha].Add($path) | Out-Null + } + } + } + + if (-not $hasRefs -or -not $hasRelevantChanges) { + Write-HookInfo 'No files requiring pre-push checks, skipping.' + exit 0 + } + + $changedPathList = @($allChanged | Sort-Object) + if ($changedPathList.Count -gt 0) { + Write-HookInfo "Checking $($changedPathList.Count) changed file(s)." + } + else { + Write-HookInfo 'Checking pushed C# tree.' + } + + if ($shaToCSharp.Count -gt 0) { + Write-HookInfo 'Checking pushed C# changes for forbidden #region directives.' + $regionCheckPassed = $true + if ($script:TreeRegionCheckShas.Count -gt 0) { + $regionCheckPassed = (Test-RegionsInPushedCSharpTree -Shas @($script:TreeRegionCheckShas)) -and $regionCheckPassed + } + + $changedRegionShaToPaths = @{} + foreach ($sha in $shaToCSharp.Keys) { + if ($script:TreeRegionCheckShas.Contains($sha)) { + continue + } + + if ($shaToCSharp[$sha].Count -gt 0) { + $changedRegionShaToPaths[$sha] = $shaToCSharp[$sha] + } + } + + if ($changedRegionShaToPaths.Count -gt 0) { + $regionCheckPassed = (Test-RegionChangesInPushedCSharp -ShaToPaths $changedRegionShaToPaths) -and $regionCheckPassed + } + + if (-not $regionCheckPassed) { + Write-Host 'Pre-push checks FAILED.' + $recoveryPaths = [System.Collections.Generic.HashSet[string]]::new([System.StringComparer]::Ordinal) + foreach ($path in $changedPathList) { + Add-UniquePath -Set $recoveryPaths -Path $path + } + foreach ($path in $script:RegionRecoveryPaths) { + Add-UniquePath -Set $recoveryPaths -Path $path + } + Write-AgentPreflightFixHint -Paths @($recoveryPaths | Sort-Object) + Write-Host 'To skip in emergencies: git push --no-verify (CI will still validate)' + exit 1 + } + Write-HookInfo 'No #region directives found.' + } + + Write-HookInfo 'Pre-push checks complete. Push proceeding.' + exit 0 +} +catch { + Write-HookError $_.Exception.Message + exit 1 +} +finally { + Pop-Location +} diff --git a/.github/ISSUE_TEMPLATE/documentation.yml b/.github/ISSUE_TEMPLATE/documentation.yml index bfb47bf73..8c75b616b 100644 --- a/.github/ISSUE_TEMPLATE/documentation.yml +++ b/.github/ISSUE_TEMPLATE/documentation.yml @@ -13,7 +13,7 @@ body: attributes: label: Page URL or Section description: Link to the documentation page or describe the section. - placeholder: "https://wallstop.github.io/unity-helpers/... or 'Getting Started > Installation'" + placeholder: "https://ambiguous-interactive.github.io/unity-helpers/... or 'Getting Started > Installation'" validations: required: true diff --git a/.github/actionlint.yaml b/.github/actionlint.yaml new file mode 100644 index 000000000..36a127ca9 --- /dev/null +++ b/.github/actionlint.yaml @@ -0,0 +1,20 @@ +# actionlint configuration. +# Declares custom self-hosted runner labels that actionlint cannot infer +# from the workflow files. Without this, actionlint reports +# "label \"\" is unknown" for every job referencing them. +self-hosted-runner: + labels: + # Marker label applied to self-hosted Windows runners with 64GB+ RAM, + # used by the Unity workflows (unity-tests.yml, unity-benchmarks.yml, + # runner-bootstrap.yml). Add additional custom labels here as new runner + # pools are introduced. + - RAM-64GB + # Speed marker reserved on the org's runners for future opt-in (e.g., + # hotfix dispatch); no currently-active workflow requests it. actionlint + # still allows the label so a future workflow can opt in without churn. + - fast + # Legacy marker reserved here so any future inline + # `runs-on: [self-hosted, Linux, ..., old]` does not trip an actionlint + # false-positive. + - old +config-variables: null diff --git a/.github/actions/compute-unity-assemblies/action.yml b/.github/actions/compute-unity-assemblies/action.yml new file mode 100644 index 000000000..e88407370 --- /dev/null +++ b/.github/actions/compute-unity-assemblies/action.yml @@ -0,0 +1,70 @@ +name: Compute Unity test assembly list +description: >- + Resolve the default Unity test assembly include list via the single-source + asmdef-discovery.js module. Exports it as the UH_TEST_ASSEMBLIES env var AND + as step outputs (assemblies, is-empty) so a caller can SKIP a target that has + no unity-helpers-owned test assemblies instead of failing the whole run. +inputs: + include-perf: + description: "Include the Performance/Stress perf assemblies." + required: false + default: "false" + include-integrations: + description: "Include the DI-container integration suites (VContainer / Zenject / Reflex)." + required: false + default: "false" + runtime-only: + description: "Drop editor-only asmdefs (standalone player flow cannot run EditMode tests)." + required: false + default: "false" + target: + description: "Unity test target: editmode, playmode, or standalone." + required: false + default: "" +outputs: + assemblies: + description: "Semicolon-joined unity-helpers test assembly names (empty string when none match)." + value: ${{ steps.compute.outputs.assemblies }} + is-empty: + description: >- + 'true' when no unity-helpers-owned test assembly matched this target -- the + caller should skip the Unity run/verify steps. 'false' otherwise. + value: ${{ steps.compute.outputs.is-empty }} +runs: + using: composite + steps: + - name: Compute assembly list + id: compute + shell: pwsh + run: | + $parts = @() + if ("${{ inputs.include-perf }}" -eq "true") { $parts += "includePerf: true" } + if ("${{ inputs.include-integrations }}" -eq "true") { $parts += "includeIntegrations: true" } + if ("${{ inputs.runtime-only }}" -eq "true") { $parts += "runtimeOnly: true" } + if ("${{ inputs.target }}" -ne "") { $parts += "target: '${{ inputs.target }}'" } + $opts = if ($parts.Count) { "{ " + ($parts -join ", ") + " }" } else { "{}" } + $assemblies = node -e "const m=require('./scripts/unity/lib/asmdef-discovery.js'); console.log(m.defaultIncludeAssemblies(process.cwd(), $opts).join(';'))" + $discoveryExit = $LASTEXITCODE + # A non-zero exit means the discovery SCRIPT itself failed (bad repo + # checkout, syntax error, throw) -- a real error, not an empty list. Fail + # hard so it is never silently mistaken for "no assemblies". + if ($discoveryExit -ne 0) { + Write-Host "::error::asmdef discovery script failed (exit code $discoveryExit). The Unity test assembly list could not be resolved." + exit 1 + } + $assemblies = ([string]$assemblies).Trim() + if ([string]::IsNullOrWhiteSpace($assemblies)) { + # SKIP, do not fail: the discovery succeeded but no unity-helpers-owned + # test assembly matched this target (for example a runtime-only + # standalone run when every unity-helpers test asmdef is editor-only). + # The caller gates its provision/run/verify steps on is-empty so the + # target is skipped cleanly instead of failing with "0 tests ran". + Write-Host "::notice::No unity-helpers test assemblies matched this target; the Unity run will be skipped." + Add-Content -Path $env:GITHUB_OUTPUT -Value "assemblies=" -Encoding utf8 + Add-Content -Path $env:GITHUB_OUTPUT -Value "is-empty=true" -Encoding utf8 + exit 0 + } + Add-Content -Path $env:GITHUB_ENV -Value "UH_TEST_ASSEMBLIES=$assemblies" -Encoding utf8 + Add-Content -Path $env:GITHUB_OUTPUT -Value "assemblies=$assemblies" -Encoding utf8 + Add-Content -Path $env:GITHUB_OUTPUT -Value "is-empty=false" -Encoding utf8 + Write-Host "Resolved assemblies: $assemblies" diff --git a/.github/actions/dump-unity-log-tail/action.yml b/.github/actions/dump-unity-log-tail/action.yml new file mode 100644 index 000000000..cb4ca9936 --- /dev/null +++ b/.github/actions/dump-unity-log-tail/action.yml @@ -0,0 +1,181 @@ +name: Dump Unity log tail on failure or cancellation +description: >- + Surface the tail of /unity.log and rescan it for catastrophic + compile-time failure patterns when an upstream Unity step fails OR is + cancelled. Gives operators something concrete to look at when the + test-runner step timed out or was cancelled and verify-unity-results + (which gates on !cancelled()) would otherwise skip. +inputs: + results-dir: + description: "Directory the Unity runner wrote artifacts (and unity.log) into." + required: true + label: + description: "Human label used in the group annotation." + required: false + default: "Unity tests" + tail-lines: + description: "Number of trailing log lines to print." + required: false + default: "200" +runs: + using: composite + steps: + # IMPORTANT: this step is gated by the workflow caller's `if:` clause + # (which must include both `failure()` AND `cancelled()`); the inner + # guards here only protect against the log being absent (cancellation + # before Unity launched, or a different upstream failure that never + # wrote unity.log). This step itself NEVER fails -- it is purely a + # best-effort diagnostic. A throw here would mask the upstream failure + # the operator is actually trying to investigate. + - name: Dump Unity log tail and rescan for catastrophic patterns + shell: pwsh + run: | + $dir = "${{ inputs.results-dir }}" + $label = "${{ inputs.label }}" + $tailLines = [int]"${{ inputs.tail-lines }}" + if ($tailLines -lt 1) { $tailLines = 200 } + + function Add-UnityDiagnosticLogFile { + param( + [Parameter(Mandatory = $true)]$LogFiles, + [Parameter(Mandatory = $true)]$Seen, + [string]$Path + ) + + if (-not $Path -or -not (Test-Path -LiteralPath $Path -PathType Leaf)) { + return + } + + try { + $fullPath = (Resolve-Path -LiteralPath $Path -ErrorAction Stop).Path + } catch { + return + } + + if ($Seen.Add($fullPath)) { + $LogFiles.Add($fullPath) + } + } + + function Get-UnityDiagnosticLogFiles { + param([string]$ResultsDir) + + $logFiles = New-Object System.Collections.Generic.List[string] + $seen = New-Object 'System.Collections.Generic.HashSet[string]' ([StringComparer]::OrdinalIgnoreCase) + if (-not $ResultsDir) { + return @($logFiles) + } + + Add-UnityDiagnosticLogFile ` + -LogFiles $logFiles ` + -Seen $seen ` + -Path (Join-Path $ResultsDir 'unity.log') + + if (Test-Path -LiteralPath $ResultsDir -PathType Container) { + Get-ChildItem -LiteralPath $ResultsDir -File -Recurse -Filter '*.log' -ErrorAction SilentlyContinue | + Sort-Object FullName | + ForEach-Object { + Add-UnityDiagnosticLogFile ` + -LogFiles $logFiles ` + -Seen $seen ` + -Path $_.FullName + } + } + + return @($logFiles) + } + + $logFiles = @(Get-UnityDiagnosticLogFiles -ResultsDir $dir) + $log = if ($dir) { Join-Path $dir 'unity.log' } else { $null } + if (-not $log -or -not (Test-Path -LiteralPath $log -PathType Leaf)) { + # Cancellation before Unity launched leaves no unity.log; this is + # NOT an error condition for this diagnostic helper. + Write-Host "::notice::${label}: no unity.log under '$dir' to dump (Unity may not have started, or the run was cancelled before launch)." + } else { + Write-Host "::group::Unity log tail (last $tailLines lines) -- $label" + try { + Get-Content -LiteralPath $log -Tail $tailLines | ForEach-Object { + Write-Host $_ + } + } catch { + # Best-effort: a partially-written log (Unity killed mid-flush) + # may surface an IO error. Surface a notice + continue so the + # downstream catastrophic-pattern scan still runs. + Write-Host "::notice::Unable to read tail of '$log': $($_.Exception.Message)" + } + Write-Host "::endgroup::" + } + + # STANDALONE: the test player runs as a SEPARATE process and writes its own + # log (player.log), not unity.log. On a standalone failure the player log + # carries the actual test output and quit diagnostics, so tail it too when + # present (no-op for editmode/playmode, which write only unity.log). + $playerLog = if ($dir) { Join-Path $dir 'player.log' } else { $null } + if ($playerLog -and (Test-Path -LiteralPath $playerLog -PathType Leaf)) { + Write-Host "::group::Standalone player log tail (last $tailLines lines) -- $label" + try { + Get-Content -LiteralPath $playerLog -Tail $tailLines | ForEach-Object { + Write-Host $_ + } + } catch { + Write-Host "::notice::Unable to read tail of '$playerLog': $($_.Exception.Message)" + } + Write-Host "::endgroup::" + } + + if ($logFiles.Count -lt 1) { + return + } + + # Rerun the catastrophic-pattern scan over every available Unity + # diagnostic log, including retry logs saved as *.first-attempt.log. + # The pattern list MUST stay in sync with verify-unity-results/ + # action.yml's $patterns array and run-ci-tests.ps1's + # $script:CatastrophicPatterns -- all three call sites must use the + # same labels (kept in sync by convention; update all three together). + $patterns = @( + @{ Label = 'PrecompiledAssemblyException'; Pattern = 'PrecompiledAssemblyException'; UseSimple = $true } + @{ Label = 'CompilationFailedException'; Pattern = 'CompilationFailedException'; UseSimple = $true } + @{ Label = 'Multiple precompiled assemblies with the same name'; Pattern = 'Multiple precompiled assemblies with the same name'; UseSimple = $true } + @{ Label = 'error CS\d+'; Pattern = 'error CS\d+'; UseSimple = $false } + @{ Label = 'warning CS8032'; Pattern = 'warning CS8032'; UseSimple = $false } + ) + + $maxPerPattern = 5 + foreach ($entry in $patterns) { + $hits = New-Object System.Collections.Generic.List[object] + foreach ($logFile in $logFiles) { + try { + if ($entry.UseSimple) { + Select-String -LiteralPath $logFile -SimpleMatch -Pattern $entry.Pattern -ErrorAction SilentlyContinue | + ForEach-Object { + if ($hits.Count -lt $maxPerPattern) { + $hits.Add($_) + } + } + } else { + Select-String -LiteralPath $logFile -Pattern $entry.Pattern -ErrorAction SilentlyContinue | + ForEach-Object { + if ($hits.Count -lt $maxPerPattern) { + $hits.Add($_) + } + } + } + } catch { + # Never throw from a diagnostic helper. + } + if ($hits.Count -ge $maxPerPattern) { + break + } + } + + if ($hits.Count -gt 0) { + Write-Host "::group::Catastrophic pattern: $($entry.Label) -- $label" + foreach ($hit in $hits) { + $line = $hit.Line.Trim() + Write-Host "::error::Pattern detected -- $($entry.Label):: $line" + Write-Host " $($hit.Path):$($hit.LineNumber): $line" + } + Write-Host "::endgroup::" + } + } diff --git a/.github/actions/print-self-hosted-runner-diagnostics/action.yml b/.github/actions/print-self-hosted-runner-diagnostics/action.yml new file mode 100644 index 000000000..71bc35b23 --- /dev/null +++ b/.github/actions/print-self-hosted-runner-diagnostics/action.yml @@ -0,0 +1,188 @@ +name: Print self-hosted runner diagnostics +# cspell:ignore WSL prereqs +description: >- + Emit a self-contained diagnostic block for a self-hosted Windows Unity + runner. First runs a Windows PowerShell 5.1 preflight that fails fast with + a clear, actionable error when PowerShell 7 (pwsh) is missing, so the + cryptic "pwsh: command not found" never surfaces. Then emits diagnostics + via PowerShell so we never depend on the runner-host PATH ordering between + WSL bash and Git Bash. Surfaces the resolved bash path and warns when the + WSL stub at System32\bash.exe leaks ahead of + C:\Program Files\Git\bin\bash.exe so the operator can fix the agent + PATH before queueing further Unity work. + +inputs: + concurrency-note: + description: One-line note describing the job's concurrency contract. + required: false + default: "wallstop-organization-builds (central organization Unity lock)" + matrix-note: + description: One-line note describing matrix scheduling. + required: false + default: "" + requested-labels: + description: Human-readable runs-on label set for this job. + required: false + default: "self-hosted, Windows, RAM-64GB" + +runs: + using: "composite" + steps: + # Runs in Windows PowerShell 5.1 (always built into Windows runners), so + # it executes even when pwsh is absent. The remaining steps in this action + # and the Unity jobs that consume it use `shell: pwsh`; without this + # preflight a missing pwsh surfaces only as the cryptic + # "##[error]pwsh: command not found". This turns that into a clear, + # actionable failure pointing at the runbook. PowerShell 5.1-safe: no + # `??`, no ternary. + - name: Verify PowerShell 7 (pwsh) is available + shell: powershell + run: | + $pwshCommand = Get-Command pwsh -CommandType Application -ErrorAction SilentlyContinue + if ($null -eq $pwshCommand -or + [string]::IsNullOrWhiteSpace($pwshCommand.Source) -or + -not (Test-Path $pwshCommand.Source -PathType Leaf)) { + Write-Output ("::error title=pwsh missing on self-hosted runner::PowerShell 7 (pwsh) is not installed on runner '$env:RUNNER_NAME'. Install it before queueing Unity jobs -- see " + + "docs/runbooks/unity-runners-after-transfer.md (PowerShell 7 prerequisite).") + exit 1 + } + Write-Output "pwsh found at: $($pwshCommand.Source)" + - name: Emit runner diagnostics + shell: pwsh + env: + # Surfaced as inputs so the diagnostic line per workflow stays + # accurate ( # other three do). Empty strings render as "n/a". + UH_DIAG_CONCURRENCY_NOTE: ${{ inputs.concurrency-note }} + UH_DIAG_MATRIX_NOTE: ${{ inputs.matrix-note }} + UH_DIAG_REQUESTED_LABELS: ${{ inputs.requested-labels }} + run: | + $ErrorActionPreference = 'Continue' + function Default-Value { + param([string]$value, [string]$fallback = '') + if ([string]::IsNullOrEmpty($value)) { return $fallback } + return $value + } + Write-Output "::group::Runner diagnostics" + Write-Output ("Runner name: " + (Default-Value $env:RUNNER_NAME)) + Write-Output ("Runner OS: " + (Default-Value $env:RUNNER_OS)) + Write-Output ("Runner arch: " + (Default-Value $env:RUNNER_ARCH)) + Write-Output ("Runner workspace: " + (Default-Value $env:RUNNER_WORKSPACE)) + Write-Output ("Concurrency group: " + (Default-Value $env:UH_DIAG_CONCURRENCY_NOTE 'n/a')) + $matrixNote = $env:UH_DIAG_MATRIX_NOTE + if (-not [string]::IsNullOrEmpty($matrixNote)) { + Write-Output ("Matrix scheduling: " + $matrixNote) + } + Write-Output ("Requested labels: " + $env:UH_DIAG_REQUESTED_LABELS) + Write-Output ("GitHub workflow: " + (Default-Value $env:GITHUB_WORKFLOW)) + Write-Output ("GitHub job: " + (Default-Value $env:GITHUB_JOB)) + $runId = Default-Value $env:GITHUB_RUN_ID + $runAttempt = Default-Value $env:GITHUB_RUN_ATTEMPT + Write-Output ("GitHub run id/attempt: " + $runId + " / " + $runAttempt) + $githubRef = Default-Value $env:GITHUB_REF + $refProtected = Default-Value $env:GITHUB_REF_PROTECTED + Write-Output ("GitHub ref: " + $githubRef + " (protected=" + $refProtected + ")") + Write-Output ("GitHub event: " + (Default-Value $env:GITHUB_EVENT_NAME)) + Write-Output "" + Write-Output ("PowerShell version: " + $PSVersionTable.PSVersion.ToString()) + # PATH-walk that mirrors actions/runner WhichUtil. The runner agent + # resolves shell:bash by walking $env:Path entries in order and + # returning the first directory that contains bash.exe (no PATHEXT + # search, no PowerShell-specific aliases). PowerShell's Get-Command + # consults aliases, functions, and built-ins BEFORE PATH, so it can + # report a bash binding that the runner agent will never actually + # pick. Reproduce the agent's algorithm explicitly. We probe for + # bash.exe first (the standard Windows artifact), then bash without + # an extension as a fallback for unconventional installs. + $resolvedBash = $null + $pathEntries = ($env:Path -split ';') | Where-Object { -not [string]::IsNullOrWhiteSpace($_) } + foreach ($candidateName in @('bash.exe', 'bash')) { + foreach ($pathEntry in $pathEntries) { + $candidatePath = Join-Path $pathEntry $candidateName + if (Test-Path $candidatePath -PathType Leaf) { + $resolvedBash = $candidatePath + break + } + } + if ($null -ne $resolvedBash) { break } + } + if ($null -ne $resolvedBash) { + Write-Output ("Resolved bash: " + $resolvedBash) + # The WSL stub sits at C:\Windows\System32\bash.exe and tries to + # launch a WSL distro; on Unity runners that distro is usually + # not installed and `shell: bash` steps fail with + # "Windows Subsystem for Linux has no installed distributions." + # When that stub wins the PATH resolution race, surface a + # ::warning:: so the operator fixes the runner-host PATH + # ordering (Git Bash must precede System32 in PATH). + if ($resolvedBash -match '\\System32\\bash(\.exe)?$' -or $resolvedBash -match '/System32/bash(\.exe)?$') { + Write-Output ("::warning::self-hosted runner PATH resolves bash to the WSL stub; ensure Git Bash precedes System32 in PATH") + } + } else { + Write-Output "Resolved bash: (not on PATH)" + } + $pathParts = ($env:Path -split ';') | Where-Object { -not [string]::IsNullOrWhiteSpace($_) } + $head = $pathParts | Select-Object -First 10 + Write-Output "PATH (first 10 entries):" + foreach ($entry in $head) { + Write-Output (" - " + $entry) + } + Write-Output "::endgroup::" + - name: Configure Git compression tools for Actions cache + shell: pwsh + run: | + $ErrorActionPreference = 'Continue' + $candidateDirs = New-Object System.Collections.Generic.List[string] + foreach ($root in @($env:ProgramFiles, ${env:ProgramFiles(x86)}, $env:ProgramW6432)) { + if ([string]::IsNullOrWhiteSpace($root)) { + continue + } + $dir = Join-Path $root 'Git\usr\bin' + if (-not $candidateDirs.Contains($dir)) { + [void]$candidateDirs.Add($dir) + } + } + + $gitToolDir = $null + foreach ($dir in $candidateDirs) { + $gzipPath = Join-Path $dir 'gzip.exe' + $tarPath = Join-Path $dir 'tar.exe' + if ((Test-Path $gzipPath -PathType Leaf) -and (Test-Path $tarPath -PathType Leaf)) { + $gitToolDir = $dir + break + } + } + + if ($null -ne $gitToolDir) { + $env:Path = "$gitToolDir;$env:Path" + if (-not [string]::IsNullOrWhiteSpace($env:GITHUB_PATH)) { + Add-Content -Path $env:GITHUB_PATH -Value $gitToolDir -Encoding utf8 + } + Write-Output "::notice::Prepended Git usr/bin for Actions cache compression tools" + Write-Output ("Git tool dir: " + $gitToolDir) + Write-Output ("gzip: " + (Join-Path $gitToolDir 'gzip.exe')) + Write-Output ("tar: " + (Join-Path $gitToolDir 'tar.exe')) + } else { + Write-Output ("::warning::Git usr/bin with gzip.exe and tar.exe was not found; " + + "actions/cache may fail to restore/save compressed archives.") + Write-Output "Checked candidate Git usr/bin directories:" + foreach ($dir in $candidateDirs) { + Write-Output (" - " + $dir) + } + } + # TODO(unity-helpers): DxMessaging ran an `assert-unity-host-prereqs` + # composite here to detect missing Windows host prerequisites + # (the Visual C++ runtime, long-paths, Defender exclusions, pwsh, the + # Universal C Runtime) -- the class of failure that surfaces as Unity.exe + # STATUS_DLL_NOT_FOUND (0xC0000135). That composite plus its + # scripts/unity/bootstrap-windows-runner.ps1 / maintain-windows-runner.ps1 + # backends were NOT ported in this batch (out of scope). The diagnostic + # emission above (PATH walk, bash resolution, Git compression tools) still + # runs. To restore the host prerequisite assertion, port + # .github/actions/assert-unity-host-prereqs/ and its PowerShell backends + # from DxMessaging, then re-add the step: + # - name: Assert Unity host prerequisites + # uses: ./.github/actions/assert-unity-host-prereqs + # with: + # auto-install: "false" + # Until then this composite is detect/diagnose-only, which is a safe + # superset of "do nothing extra" -- it never fails the job on its own. diff --git a/.github/actions/return-unity-license/action.yml b/.github/actions/return-unity-license/action.yml new file mode 100644 index 000000000..d67167e45 --- /dev/null +++ b/.github/actions/return-unity-license/action.yml @@ -0,0 +1,123 @@ +name: Return Unity license +description: >- + Defense-in-depth: explicitly return the per-run Unity license seat via + `Unity.exe -returnlicense` (classic serial activation). Runs as an if:always() + backstop for a killed/crashed/timed-out editor that could not return its own + seat. NEVER fails the build (a missing editor path or credentials, or any error, is + non-fatal -- the next run's return-at-start is a further backstop). +inputs: + unity-editor-path: + description: "Path to the resolved Unity editor exe (exported to GITHUB_ENV by run-ci-tests.ps1). When empty there is nothing to return." + required: false +runs: + using: composite + steps: + - name: Return Unity license + shell: pwsh + run: | + Set-StrictMode -Version Latest + # Pin $PSNativeCommandUseErrorActionPreference = $false (mirrors the + # rationale in scripts/unity/run-ci-tests.ps1). PowerShell 7.4+ + # can ENABLE the native-error behavior so that `& ` THROWS on a + # non-zero exit. If that fired here, `& $editorPath @returnArgs` would + # throw BEFORE `$exitCode = $LASTEXITCODE` runs, jumping straight to the + # catch and making the best-effort return's exit-code-classified warning + # unreliable across hosts. Pinning it $false keeps the LASTEXITCODE-based + # classification authoritative and identical on every runner. (Pre-7.4 + # builds lack the variable; assigning it is harmless and StrictMode-safe.) + $PSNativeCommandUseErrorActionPreference = $false + + # Best-effort, never-fails seat return. Top-level try/catch + a final + # `exit 0` guarantee this step can never fail the build: a leaked seat is + # also reclaimed by the NEXT run's return-at-start even if every branch + # below is skipped. + try { + $editorPath = '${{ inputs.unity-editor-path }}' + if ([string]::IsNullOrWhiteSpace($editorPath)) { + $editorPath = $env:UNITY_EDITOR_PATH + } + + # Nothing to return when the editor path was never resolved (the Unity + # run never reached the point of exporting UNITY_EDITOR_PATH). + if ([string]::IsNullOrWhiteSpace($editorPath)) { + Write-Host "::notice::No Unity editor path resolved; nothing to return." + exit 0 + } + if (-not (Test-Path -LiteralPath $editorPath -PathType Leaf)) { + Write-Host "::notice::Resolved Unity editor path does not exist; nothing to return." + exit 0 + } + + # Serial activation returns by account, so both credentials are required. If + # either is empty we cannot return -- warn (never echo the values) and + # exit 0 (the next run's return-at-start is the backstop). + if ([string]::IsNullOrWhiteSpace($env:UNITY_EMAIL) -or [string]::IsNullOrWhiteSpace($env:UNITY_PASSWORD)) { + Write-Host "::warning::UNITY_EMAIL/UNITY_PASSWORD are not both set; cannot return the Unity license here. The next run's return-at-start is the backstop." + exit 0 + } + + # SECURITY: email/password ride in the argument array, and Unity may echo + # account fragments into its log, so write the return log to a NON-uploaded + # temp dir (RUNNER_TEMP, fallback the system temp path), NOT under the + # uploaded artifacts. Never echo the args/secrets. + $licenseLogDir = if ($env:RUNNER_TEMP) { $env:RUNNER_TEMP } else { [System.IO.Path]::GetTempPath() } + $returnLog = Join-Path $licenseLogDir 'unity-return-license.log' + + # SECURITY: do NOT echo this array (it carries the credentials). + $returnArgs = @( + '-quit', + '-batchmode', + '-nographics', + '-returnlicense', + '-username', $env:UNITY_EMAIL, + '-password', $env:UNITY_PASSWORD, + '-logFile', '-' + ) + + Write-Host "::group::Return Unity license" + # Unity.exe is a Windows GUI-subsystem binary: PowerShell's `&` does NOT + # wait for it or set $LASTEXITCODE unless its stdout is consumed via the + # pipeline. `-logFile -` puts the log on stdout and `| Tee-Object` forces + # the wait and sets $LASTEXITCODE. Tee-Object DOES persist the log to + # $returnLog, but that target lives under the NON-uploaded temp dir + # ($licenseLogDir above), so it stays out of any UPLOADED ARTIFACT and the + # account fragments Unity may print cannot leak into uploads. Same proven + # idiom as run-ci-tests.ps1. + & $editorPath @returnArgs 2>&1 | Tee-Object -FilePath $returnLog + $exitCode = $LASTEXITCODE + Write-Host "::endgroup::" + + if ($exitCode -ne 0) { + $returnedEntitlement = $false + $legacyFileUnavailable = $false + try { + if (Test-Path -LiteralPath $returnLog -PathType Leaf) { + $returnedEntitlement = Select-String ` + -LiteralPath $returnLog ` + -Pattern 'Successfully returned the entitlement license' ` + -SimpleMatch ` + -Quiet + $legacyFileUnavailable = Select-String ` + -LiteralPath $returnLog ` + -Pattern 'Serial number unavailable for ULF return' ` + -SimpleMatch ` + -Quiet + } + } catch { + $returnedEntitlement = $false + $legacyFileUnavailable = $false + } + + if ($returnedEntitlement -and $legacyFileUnavailable) { + Write-Host "::notice::Unity returned the entitlement license, then exited with code $exitCode while skipping legacy ULF return; treating the seat return as successful." + } else { + Write-Host "::warning::Unity license return exited with code $exitCode; the next run's return-at-start is the backstop for the leaked seat." + } + } else { + Write-Host "::notice::Returned the Unity license seat." + } + } catch { + Write-Host "::warning::Unity license return step hit an unexpected error: $($_.Exception.Message). The next run's return-at-start is the backstop." + } + # Defense-in-depth: this step must never fail the build. + exit 0 diff --git a/.github/actions/validate-unity-license/action.yml b/.github/actions/validate-unity-license/action.yml new file mode 100644 index 000000000..6f2bed281 --- /dev/null +++ b/.github/actions/validate-unity-license/action.yml @@ -0,0 +1,62 @@ +name: Validate Unity license +description: >- + Fail fast before acquiring the org build lock when classic serial Unity + activation is misconfigured: UNITY_SERIAL, UNITY_EMAIL, and UNITY_PASSWORD must + all be set, and the retired UNITY_LICENSING_SERVER must NOT be. Prints only + presence booleans, never secret values. The directory name + (validate-unity-license) is retained to avoid churn in callers/validators. +runs: + using: composite + steps: + - name: Validate Unity serial-activation preflight + shell: pwsh + run: | + Set-StrictMode -Version Latest + # Hygiene/parity pin (same rationale as scripts/unity/run-ci-tests.ps1 + # and the return action). This block no longer invokes a native binary + # through a pipe, so $PSNativeCommandUseErrorActionPreference is purely + # belt-and-suspenders here; pinning it $false keeps every pwsh block in + # these actions behaving identically regardless of host/version. StrictMode + # is the load-bearing part: it makes any accidental $null/undefined-property + # access throw loudly instead of silently passing the preflight. + $PSNativeCommandUseErrorActionPreference = $false + + # This action validates classic SERIAL activation (the repo uses classic + # serial activation, NOT the Unity Licensing Server / floating license). + # The directory name validate-unity-license is retained so callers do not + # churn. + $hasSerial = -not [string]::IsNullOrWhiteSpace($env:UNITY_SERIAL) + $hasEmail = -not [string]::IsNullOrWhiteSpace($env:UNITY_EMAIL) + $hasPassword = -not [string]::IsNullOrWhiteSpace($env:UNITY_PASSWORD) + + Write-Host "::group::Unity serial-activation preflight" + + # This repo uses classic serial activation, not the floating-license + # server. If UNITY_LICENSING_SERVER is still wired up, fail loudly so it + # gets removed (no silent fallback). + if (-not [string]::IsNullOrWhiteSpace($env:UNITY_LICENSING_SERVER)) { + Write-Host ("::error::UNITY_LICENSING_SERVER is set but unused. This repo uses classic serial activation (UNITY_SERIAL + UNITY_EMAIL + UNITY_PASSWORD); remove the " + + "UNITY_LICENSING_SERVER secret.") + Write-Host "::endgroup::" + exit 1 + } + + # All three serial-activation credentials are required. Name WHICH are + # missing (never their values) so the fix is obvious. + $missing = @() + if (-not $hasSerial) { $missing += 'UNITY_SERIAL' } + if (-not $hasEmail) { $missing += 'UNITY_EMAIL' } + if (-not $hasPassword) { $missing += 'UNITY_PASSWORD' } + if ($missing.Count -gt 0) { + Write-Host ("::error::Serial Unity activation requires UNITY_SERIAL, UNITY_EMAIL, and UNITY_PASSWORD. Missing or empty: " + ($missing -join ', ') + ".") + Write-Host "::endgroup::" + exit 1 + } + + # Presence booleans only -- never echo the values. + Write-Host "UNITY_SERIAL present: $hasSerial" + Write-Host "UNITY_EMAIL present: $hasEmail" + Write-Host "UNITY_PASSWORD present: $hasPassword" + + Write-Host "::notice::Unity serial-activation preflight passed." + Write-Host "::endgroup::" diff --git a/.github/actions/verify-unity-results/action.yml b/.github/actions/verify-unity-results/action.yml new file mode 100644 index 000000000..26519e988 --- /dev/null +++ b/.github/actions/verify-unity-results/action.yml @@ -0,0 +1,603 @@ +name: Verify Unity tests actually ran +# cspell:ignore ieq IPC +description: >- + Fail unless an NUnit results.xml under results-dir reports total > 0 and + failed = 0. When XML is absent, list provisioning diagnostics and scan the + Unity log for catastrophic compile-time failures. +inputs: + results-dir: + description: "Directory the Unity runner wrote the NUnit results XML into." + required: true + label: + description: "Human label used in log lines and annotations." + required: false + default: "Unity tests" + expected-empty: + description: >- + Set to 'true' by the caller when the unity-helpers test-assembly list + resolved EMPTY for this target and the Unity run was intentionally + skipped. The verifier then treats the absence of results as an expected + skip (notice + success) instead of failing with "tests did not run". + required: false + default: "false" +runs: + using: composite + steps: + # Skip on cancellation so this step does not race against the user's + # cancel and emit a misleading "tests did not run" error annotation + # when in fact the run was cancelled. Note: this is a STEP-level + # condition; the workflow-level "Verify tests actually ran" entry that + # invokes this composite should use the SAME idiom so the composite is + # not invoked at all on cancel, but the inner guard belt-and-suspenders + # protects against a future workflow forgetting the outer condition. + - name: Verify tests actually ran + if: ${{ !cancelled() }} + shell: pwsh + # Pass EVERY caller-supplied value through env: (NOT inline ${{ }} expansion + # in the run script). A run: block that contains even one ${{ }} interpolation + # is compiled by GitHub as a single template expression, and THIS block's body + # is ~26 KB -- well over the hard 21,000-character per-expression limit, which + # made every Unity leg fail to start with "Exceeded max expression length 21000" + # (Line 38 = this run:). Reading the inputs from $env:* keeps the run body a + # plain literal (no template compilation, no length limit) and is also safer: + # a label/dir containing a quote can no longer break the script. scripts/lint- + # workflow-run-expression-length.ps1 enforces this for every action/workflow + # run block. + env: + UH_EXPECTED_EMPTY: ${{ inputs.expected-empty }} + UH_RESULTS_DIR: ${{ inputs.results-dir }} + UH_LABEL: ${{ inputs.label }} + run: | + $dir = $env:UH_RESULTS_DIR + $label = $env:UH_LABEL + # When the caller resolved an empty unity-helpers test-assembly list for + # this target it skips the Unity run entirely (see compute-unity-assemblies + # and unity-tests.yml). There are then no results to verify, so treat it + # as an explicit success instead of a "tests did not run" failure. Read + # the signal from the environment via an env: mapping above (NOT an inline + # workflow-expression expansion in this run script) so the spawn-based + # verifier tests, which only substitute results-dir/label, observe an + # unset var and exercise the normal verification path. + if ($env:UH_EXPECTED_EMPTY -eq 'true') { + Write-Host "::notice::No unity-helpers test assemblies were selected for $label; the Unity run was skipped, so there is nothing to verify." + exit 0 + } + function Write-ProvisioningDiagnosticsHint { + param([Parameter(Mandatory = $true)][string]$ResultsDir) + + $roots = New-Object System.Collections.Generic.List[string] + foreach ($candidate in @($ResultsDir, '.artifacts', 'artifact')) { + if ($candidate -and (Test-Path -LiteralPath $candidate)) { + $full = (Resolve-Path -LiteralPath $candidate).Path + if (-not $roots.Contains($full)) { + $roots.Add($full) + } + } + } + + $dirs = New-Object System.Collections.Generic.List[string] + foreach ($root in $roots) { + if ((Split-Path -Leaf $root) -ieq 'provisioning') { + if (-not $dirs.Contains($root)) { + $dirs.Add($root) + } + } + + $direct = Join-Path $root 'provisioning' + if ((Test-Path -LiteralPath $direct) -and -not $dirs.Contains($direct)) { + $dirs.Add((Resolve-Path -LiteralPath $direct).Path) + } + + Get-ChildItem -LiteralPath $root -Directory -Recurse -ErrorAction SilentlyContinue | + Where-Object { $_.Name -ieq 'provisioning' } | + ForEach-Object { + if (-not $dirs.Contains($_.FullName)) { + $dirs.Add($_.FullName) + } + } + } + + if ($dirs.Count -lt 1) { + Write-Host "::warning::No provisioning diagnostics directory found under $ResultsDir, .artifacts, or artifact." + return + } + + Write-Host "::group::Provisioning diagnostics files" + foreach ($diagnosticsDir in $dirs) { + Write-Host "Provisioning diagnostics directory: $diagnosticsDir" + $files = @( + Get-ChildItem -LiteralPath $diagnosticsDir -File -Recurse -ErrorAction SilentlyContinue | + Sort-Object FullName + ) + if ($files.Count -lt 1) { + Write-Host "- (empty)" + continue + } + + foreach ($file in $files) { + Write-Host "- $($file.FullName)" + } + + $summary = $files | + Where-Object { $_.Name -match '(?i)summary' } | + Select-Object -First 1 + if ($summary) { + Write-Host "::notice::Provisioning summary: $($summary.FullName)" + try { + $summaryJson = Get-Content -LiteralPath $summary.FullName -Raw | ConvertFrom-Json + $classification = if ($summaryJson.finalClassification) { + [string]$summaryJson.finalClassification + } else { + '(missing)' + } + $provisioningProfile = if ($summaryJson.provisioningProfile) { + [string]$summaryJson.provisioningProfile + } else { + '(missing)' + } + Write-Host "::notice::Provisioning summary classification=$classification profile=$provisioningProfile" + if ($env:GITHUB_STEP_SUMMARY) { + Add-Content -Path $env:GITHUB_STEP_SUMMARY -Value ("- Provisioning summary: ``" + $summary.FullName + "``") + Add-Content -Path $env:GITHUB_STEP_SUMMARY -Value ("- Provisioning classification: ``" + $classification + "`` (profile ``" + $provisioningProfile + "``)") + } + if ($classification -ne 'success' -and $classification -ne '(missing)') { + $missingModules = @() + if ($summaryJson.requiredModulePresence) { + foreach ($property in @($summaryJson.requiredModulePresence.PSObject.Properties)) { + if ($property.Value -eq $false) { + $missingModules += [string]$property.Name + } + } + } + $missingText = if ($missingModules.Count -gt 0) { + " missingModules=$($missingModules -join ',')" + } else { + "" + } + Write-Host "::error::Provisioning failed before Unity results verification: classification=$classification profile=$provisioningProfile summary=$($summary.FullName)$missingText" + } + } catch { + Write-Host "::notice::Provisioning summary metadata could not be parsed: $($_.Exception.Message)" + } + } + } + Write-Host "::endgroup::" + } + + function Add-UnityDiagnosticLogFile { + param( + [Parameter(Mandatory = $true)]$LogFiles, + [Parameter(Mandatory = $true)]$Seen, + [string]$Path + ) + + if (-not $Path -or -not (Test-Path -LiteralPath $Path -PathType Leaf)) { + return + } + + try { + $fullPath = (Resolve-Path -LiteralPath $Path -ErrorAction Stop).Path + } catch { + return + } + + if ($Seen.Add($fullPath)) { + $LogFiles.Add($fullPath) + } + } + + function Get-UnityDiagnosticLogFiles { + param([string]$ResultsDir) + + $logFiles = New-Object System.Collections.Generic.List[string] + $seen = New-Object 'System.Collections.Generic.HashSet[string]' ([StringComparer]::OrdinalIgnoreCase) + if (-not $ResultsDir) { + return @($logFiles) + } + + Add-UnityDiagnosticLogFile ` + -LogFiles $logFiles ` + -Seen $seen ` + -Path (Join-Path $ResultsDir 'unity.log') + + if (Test-Path -LiteralPath $ResultsDir -PathType Container) { + Get-ChildItem -LiteralPath $ResultsDir -File -Recurse -Filter '*.log' -ErrorAction SilentlyContinue | + Sort-Object FullName | + ForEach-Object { + Add-UnityDiagnosticLogFile ` + -LogFiles $logFiles ` + -Seen $seen ` + -Path $_.FullName + } + } + + return @($logFiles) + } + + # CLASS-OF-ISSUE DIAGNOSTIC: when results.xml is missing, the operator's + # next question is "WHY did Unity not produce results?". The most common + # silent-killer answers are catastrophic compile-time errors -- the editor + # exits before running tests at all, leaving no NUnit XML. Surface these + # patterns BEFORE the generic "no results.xml" message so the operator + # sees the actual root cause in the GitHub error summary. + # + # Patterns covered: + # - PrecompiledAssemblyException -- "Multiple precompiled assemblies + # with the same name" (the analyzer-DLL duplicate that motivated this + # diagnostic; the runtime auto-copy that caused it has been removed). + # - CompilationFailedException -- generic compile-failure path. + # - error CS\d+ -- compiler errors (CS0246, CS0103, CS0117, etc). + # - warning CS8032 -- "An instance of analyzer cannot be created" + # (analyzer failed to instantiate; same class of issue). + function Write-UnityCatastrophicPatternHits { + param([Parameter(Mandatory = $true)][string]$ResultsDir) + + $patterns = @( + @{ Label = 'PrecompiledAssemblyException'; Pattern = 'PrecompiledAssemblyException'; UseSimple = $true } + @{ Label = 'CompilationFailedException'; Pattern = 'CompilationFailedException'; UseSimple = $true } + @{ Label = 'Multiple precompiled assemblies with the same name'; Pattern = 'Multiple precompiled assemblies with the same name'; UseSimple = $true } + @{ Label = 'error CS\d+'; Pattern = 'error CS\d+'; UseSimple = $false } + @{ Label = 'warning CS8032'; Pattern = 'warning CS8032'; UseSimple = $false } + ) + + $logFiles = @(Get-UnityDiagnosticLogFiles -ResultsDir $ResultsDir) + + if ($logFiles.Count -lt 1) { + return + } + + $maxPerPattern = 5 + foreach ($entry in $patterns) { + $hits = New-Object System.Collections.Generic.List[object] + foreach ($logFile in $logFiles) { + try { + if ($entry.UseSimple) { + Select-String -LiteralPath $logFile -SimpleMatch -Pattern $entry.Pattern -ErrorAction SilentlyContinue | + ForEach-Object { + if ($hits.Count -lt $maxPerPattern) { + $hits.Add($_) + } + } + } else { + Select-String -LiteralPath $logFile -Pattern $entry.Pattern -ErrorAction SilentlyContinue | + ForEach-Object { + if ($hits.Count -lt $maxPerPattern) { + $hits.Add($_) + } + } + } + } catch { + # Best-effort; never throw from the diagnostic helper. + } + if ($hits.Count -ge $maxPerPattern) { + break + } + } + + if ($hits.Count -gt 0) { + Write-Host "::group::Catastrophic pattern: $($entry.Label)" + foreach ($hit in $hits) { + $line = $hit.Line.Trim() + Write-Host "::error::Pattern detected -- $($entry.Label):: $line" + Write-Host " $($hit.Path):$($hit.LineNumber): $line" + } + Write-Host "::endgroup::" + } + } + } + + # DIAGNOSTIC: when results.xml reports failures, name WHICH tests failed + # and print their assertion messages. The aggregate "failed=N" count is + # not actionable on its own (a real 2021.3 PlayMode run failed 1 of 697 + # tests and the logs never named it). Emits a single-line ::error:: + # annotation per failed test plus a ::group:: console block with the full + # multi-line message + stack trace. Two classes are enumerated: + # (1) failed leaf cases: //test-case[@result='Failed'], and + # (2) failed suites with their OWN direct child (the + # OneTimeSetUp/OneTimeTearDown shape, e.g. a [OneTimeTearDown] + # Assert.Fail), which manifest as a failed SUITE. Reported even when + # the suite also has a failed child case, so the teardown's own + # message is not lost; fullname de-dup keeps it distinct from the + # child cases. An aggregate-only suite (no direct ) is + # skipped. + # De-duplicated by fullname, capped at the first 50 (with a truncation + # notice), and best-effort (never throws -- a diagnostic must not mask + # the real failure below). Attribute reads use XmlElement.GetAttribute + # (returns '' when absent, never throws) so behavior stays synchronized + # with run-ci-tests.ps1 even though this script does not set StrictMode. + function ConvertTo-SingleLineDiagnostic { + param([string]$Text) + if (-not $Text) { + return '' + } + return (($Text -replace '\s+', ' ').Trim()) + } + function Write-UnityPackageManagerFailureHints { + param([Parameter(Mandatory = $true)][string]$ResultsDir) + + $patterns = @( + 'Cancelled resolving packages', + 'Failed to resolve packages:\s+operation cancelled', + 'IPCStream \(Upm-[^)]+\): IPC stream failed to read' + ) + $logFiles = @(Get-UnityDiagnosticLogFiles -ResultsDir $ResultsDir) + + foreach ($logFile in $logFiles) { + try { + $hit = Select-String -LiteralPath $logFile -Pattern $patterns -ErrorAction SilentlyContinue | + Select-Object -First 1 + if ($hit) { + $line = ConvertTo-SingleLineDiagnostic -Text $hit.Line + Write-Host "::error::Unity Package Manager canceled package resolution before tests started: $line" + Write-Host " $($hit.Path):$($hit.LineNumber): $line" + return + } + } catch { + # Best-effort diagnostic only. + } + } + } + # Token holder for the ::stop-commands:: ... :::: fence that + # wraps caller-controlled raw multi-line dumps (NUnit + # /). GitHub parses every stdout line for + # `::command::` directives; fencing the raw body disables that processing + # so an assertion message containing a line like `::error file=...::` or + # `::set-output name=x::` cannot inject a spurious workflow command. The + # token is NOT a fixed literal: a crafted message containing the exact + # `::::` close line could otherwise end the fence early and + # re-enable injection. A FRESH random GUID token is generated per dump + # via New-WorkflowCommandStopToken (mirroring GitHub's own @actions/core + # random per-invocation delimiter) and reused for the open and close. + # Kept in sync with run-ci-tests.ps1's matching fence scheme. + function New-WorkflowCommandStopToken { + return ('uh-stop-commands-{0}' -f [guid]::NewGuid().ToString('N')) + } + # Resolve a node's display name via XmlElement.GetAttribute (returns '' + # for an ABSENT attribute, never throws) -- kept synchronized with + # run-ci-tests.ps1's StrictMode-safe Get-NUnitNodeFullName. Prefers + # fullname, then name, then a '(unnamed test)' fallback. + function Get-NUnitNodeFullName { + param([Parameter(Mandatory = $true)]$Node) + $fullName = $Node.GetAttribute('fullname') + if (-not $fullName) { + $fullName = $Node.GetAttribute('name') + } + if (-not $fullName) { + $fullName = '(unnamed test)' + } + return $fullName + } + function Write-FailedTestAnnotations { + param( + [Parameter(Mandatory = $true)]$Doc, + [Parameter(Mandatory = $true)][string]$Label, + [int]$MaxFailures = 50 + ) + + try { + $failedCases = @($Doc.SelectNodes("//test-case[@result='Failed']")) + $failedSuites = @($Doc.SelectNodes("//test-suite[@result='Failed']")) + # Report a failed suite on its OWN merits whenever it carries a + # direct child (even if it also has a failed descendant + # case) so a [OneTimeTearDown] message is not lost. fullname de-dup + # keeps it distinct from its child cases; an aggregate-only suite + # (no direct ) is skipped. + $ownFailureSuites = @( + foreach ($suite in $failedSuites) { + $directFailure = $suite.SelectSingleNode('failure') + if ($directFailure) { + $suite + } + } + ) + $failedNodes = @($failedCases) + @($ownFailureSuites) + if ($failedNodes.Count -lt 1) { + return + } + + $seen = New-Object 'System.Collections.Generic.HashSet[string]' + $uniqueNodes = New-Object 'System.Collections.Generic.List[object]' + foreach ($node in $failedNodes) { + $fullName = Get-NUnitNodeFullName -Node $node + if ($seen.Add($fullName)) { + $uniqueNodes.Add($node) + } + } + + $totalFailed = $uniqueNodes.Count + $shown = @($uniqueNodes | Select-Object -First $MaxFailures) + foreach ($node in $shown) { + $fullName = Get-NUnitNodeFullName -Node $node + $failureNode = $node.SelectSingleNode('failure') + $message = '' + $stackTrace = '' + if ($failureNode) { + $messageNode = $failureNode.SelectSingleNode('message') + if ($messageNode) { + $message = $messageNode.InnerText + } + $stackNode = $failureNode.SelectSingleNode('stack-trace') + if ($stackNode) { + $stackTrace = $stackNode.InnerText + } + } + $firstLine = ConvertTo-SingleLineDiagnostic -Text $message + # The single-line ::error:: annotation stays OUTSIDE the fence so + # GitHub still processes it; it is already flattened to one line. + Write-Host "::error::${Label} failed test: $fullName -- $firstLine" + Write-Host "::group::Failed test: $fullName" + # SECURITY: the raw NUnit / are + # caller-controlled; GitHub parses every stdout line for + # `::command::` directives. Fence the raw multi-line dump with + # ::stop-commands:: ... :::: so an injected line like + # `::set-output name=x::` is neutralized. The token is a FRESH + # random GUID per dump (never a fixed literal) so a crafted message + # containing the exact `::::` close line cannot end the + # fence early. The ::group::/::endgroup:: markers stay OUTSIDE the + # fence so they are still processed. + $stopToken = New-WorkflowCommandStopToken + Write-Host "::stop-commands::$stopToken" + if ($message) { + Write-Host "Message:" + Write-Host $message + } else { + Write-Host "Message: (none recorded)" + } + if ($stackTrace) { + Write-Host "Stack trace:" + Write-Host $stackTrace + } + Write-Host "::$stopToken::" + Write-Host "::endgroup::" + } + + if ($totalFailed -gt $shown.Count) { + $omitted = $totalFailed - $shown.Count + Write-Host "::notice::${Label}: $omitted additional failed test(s) not shown (showing first $($shown.Count) of $totalFailed)." + } + } catch { + Write-Host "::warning::Could not enumerate failed tests for ${Label}: $($_.Exception.Message)" + } + } + function Get-FailedNodeCount { + param([Parameter(Mandatory = $true)]$Doc) + + try { + $failedCases = @($Doc.SelectNodes("//test-case[@result='Failed']")) + $failedSuitesWithOwnFailure = @($Doc.SelectNodes("//test-suite[@result='Failed'][failure]")) + return $failedCases.Count + $failedSuitesWithOwnFailure.Count + } catch { + return 0 + } + } + function Write-UnityExecutionSymptomDiagnostics { + param( + [Parameter(Mandatory = $true)][string]$ResultsDir, + [Parameter(Mandatory = $true)][string]$Label, + [int]$MaxLines = 80 + ) + + $patterns = @( + 'Files generated by test without cleanup\.', + 'Found \d+ new files\.', + '^\s+Assets[\\/]', + 'IgnoreFailingMessages:false', + 'Unhandled log message', + 'UnexpectedLogMessageException', + 'CleanupVerificationTask', + 'Test run completed\. Exiting with code 2', + 'One or more tests failed', + 'Unable to find (child|sibling|parent) component', + 'Can''t add ''.*'' to .* because', + 'EditorWindow\.ShowUtility', + 'd3d12: Unrecoverable GPU device error', + 'No graphic device is available to initialize the view', + 'AddCursorRect called outside an editor OnGUI' + ) + $hits = New-Object 'System.Collections.Generic.List[object]' + $sawCleanupLeak = $false + $sawUnexpectedLog = $false + $sawNativeWindowFailure = $false + + foreach ($logFile in @(Get-UnityDiagnosticLogFiles -ResultsDir $ResultsDir)) { + try { + foreach ($hit in @(Select-String -LiteralPath $logFile -Pattern $patterns -ErrorAction SilentlyContinue)) { + if ($hits.Count -lt $MaxLines) { + $hits.Add($hit) + } + + if ($hit.Line -match 'Files generated by test without cleanup\.') { + $sawCleanupLeak = $true + } + if ($hit.Line -match 'Unhandled log message' -or $hit.Line -match 'UnexpectedLogMessageException') { + $sawUnexpectedLog = $true + } + if ($hit.Line -match 'EditorWindow\.ShowUtility' -or $hit.Line -match 'd3d12: Unrecoverable GPU device error') { + $sawNativeWindowFailure = $true + } + } + } catch { + # Best-effort diagnostic only. + } + } + + if ($hits.Count -lt 1) { + return + } + + Write-Host "::group::Unity execution symptom diagnostics ($Label)" + foreach ($hit in $hits) { + $line = $hit.Line.Trim() + Write-Host (" {0}:{1}: {2}" -f $hit.Path, $hit.LineNumber, $line) + } + Write-Host "::endgroup::" + + if ($sawCleanupLeak) { + Write-Host "::error::Unity cleanup verification failed for ${Label}: one or more tests left generated files under Assets. See the generated-file lines above." + } + if ($sawUnexpectedLog) { + Write-Host "::error::Unity Test Framework rejected an unexpected log for ${Label}. Add a precise LogAssert.Expect for expected logs or fix the production log." + } + if ($sawNativeWindowFailure) { + $nativeWindowMessage = @( + "::error::Unity editor-window creation failed for ${Label}." + "CI-driven IMGUI tests should use TestIMGUIExecutor/offscreen UIElements" + "instead of creating native EditorWindow surfaces." + ) -join " " + Write-Host $nativeWindowMessage + } + } + + # ALWAYS scan the Unity log for catastrophic patterns first, regardless + # of which exit branch fires below. The patterns (PrecompiledAssemblyException, + # CompilationFailedException, error CS####, warning CS8032) are the + # most common silent-killer causes of every exit branch -- missing dir + # AND missing XML AND total==0 AND failed>0 can all coincide with a + # partial Unity crash. The helper gracefully skips when no log file + # exists, so this is cheap on the happy path. Run it at the TOP so + # the operator sees the actual root cause first in the GitHub error + # summary, not buried under a generic "tests did not run" line. + Write-UnityCatastrophicPatternHits -ResultsDir $dir + if (-not (Test-Path $dir)) { + Write-ProvisioningDiagnosticsHint -ResultsDir $dir + Write-Host "::error::No artifacts directory ($dir) -- Unity Test Runner did not produce results for $label" + exit 1 + } + $xml = Get-ChildItem -Path $dir -Recurse -Filter *.xml -ErrorAction SilentlyContinue | + Where-Object { Select-String -Path $_.FullName -Pattern ' element) under $dir -- tests did not run for $label" + exit 1 + } + [xml]$doc = Get-Content -LiteralPath $xml.FullName + $run = $doc.SelectSingleNode('//test-run') + $total = [int]($run.total) + $passed = [int]($run.passed) + $failed = [int]($run.failed) + $skipped = [int]($run.skipped) + $failedNodeCount = Get-FailedNodeCount -Doc $doc + Write-Host "Results: total=$total passed=$passed failed=$failed skipped=$skipped ($($xml.Name))" + if ($total -lt 1) { + Write-UnityExecutionSymptomDiagnostics -ResultsDir $dir -Label $label + $zeroCountMessage = @( + "::error::$label produced zero-count NUnit XML." + "This can be a failed or aborted Unity run with misleading results.xml," + "not proof of an empty assembly list; inspect Unity execution diagnostics above." + ) -join " " + Write-Host $zeroCountMessage + exit 1 + } + if ($failed -gt 0 -or $failedNodeCount -gt 0) { + Write-FailedTestAnnotations -Doc $doc -Label $label + $failureCountForMessage = [Math]::Max($failed, $failedNodeCount) + Write-Host "::error::$label reported $failureCountForMessage failed test(s); see $($xml.FullName)" + exit 1 + } + Write-Host "::notice::${label}: total=$total passed=$passed failed=$failed skipped=$skipped" diff --git a/.github/integration-packages.json b/.github/integration-packages.json new file mode 100644 index 000000000..bea3a0080 --- /dev/null +++ b/.github/integration-packages.json @@ -0,0 +1,19 @@ +{ + "_comment": "Canonical single source for the OpenUPM scoped registry and PINNED third-party dependency-injection container packages (Reflex / VContainer / Zenject-Extenject) used by the integration test legs. Consumed by scripts/unity/run-ci-tests.ps1 (ephemeral manifest, integration legs only via -IncludeIntegrations). The Runtime/Integrations and Tests/{Editor,Runtime}/Integrations asmdefs gate on REFLEX_PRESENT / VCONTAINER_PRESENT / ZENJECT_PRESENT, which Unity defines automatically (via each asmdef's versionDefines) ONLY when the matching package below is installed; installing them here is what makes those integration assemblies compile and their tests run. Bump versions/scopes HERE only.", + "_note_pins": "Pins are the OpenUPM 'latest' dist-tag as of 2026-06: Reflex 14.3.0, VContainer 1.18.0, Extenject 9.2.0-stcf3 (the same Extenject pin DxMessaging uses; the only OpenUPM-published Extenject build with proper meta files). A wrong pin only surfaces at the first real self-hosted CI run (the package fails to resolve); confirm against https://openupm.com if an integration leg reports a package-resolution failure. # TODO(unity-helpers): confirm pins on first self-hosted integration run.", + "registry": { + "name": "package.openupm.com", + "url": "https://package.openupm.com", + "scopes": ["com.gustavopsantos", "jp.hadashikick", "com.svermeulen"] + }, + "packages": { + "com.gustavopsantos.reflex": "14.3.0", + "jp.hadashikick.vcontainer": "1.18.0", + "com.svermeulen.extenject": "9.2.0-stcf3" + }, + "defines": { + "com.gustavopsantos.reflex": "REFLEX_PRESENT", + "jp.hadashikick.vcontainer": "VCONTAINER_PRESENT", + "com.svermeulen.extenject": "ZENJECT_PRESENT" + } +} diff --git a/.github/unity-test-project-modules.json b/.github/unity-test-project-modules.json new file mode 100644 index 000000000..9e8592c0e --- /dev/null +++ b/.github/unity-test-project-modules.json @@ -0,0 +1,33 @@ +{ + "_comment": "SINGLE SOURCE OF TRUTH for the UnityEngine built-in modules + editor-bundled packages (com.unity.ugui) that the ephemeral Unity TEST PROJECT must declare so com.wallstop-studios.unity-helpers' Runtime + Editor code AND its test fixtures COMPILE. If a required module is absent the editor fails compilation BEFORE any test runs (CS1069 'type forwarded to assembly UnityEngine.Module' / 'Enable the built in package '), which produces no NUnit results.xml and fails the whole matrix leg. Consumed by BOTH manifest generators: scripts/unity/run-ci-tests.ps1 (New-ManifestJson, the self-hosted CI path) and scripts/unity/create-test-project.sh (local/devcontainer path). Keeping the list HERE (not duplicated inline in each generator) is what prevents the two from drifting apart.", + "_note_packagejson": "These deliberately do NOT live in the package's package.json: that file is dual-purpose (npm devDependencies for repo tooling + the UPM manifest), and CI runs `npm ci`, which would try to resolve com.unity.modules.* / com.unity.ugui from the npm registry and fail (no matching version). The test-project manifest is therefore the correct home for them.", + "_note_adding": "When shipped (Runtime/Editor) or test code starts using a new UnityEngine module, add the package id here and BOTH generators pick it up automatically. The compile error and run-ci-tests.ps1's missing-module diagnostic name the module assembly; map UnityEngine.Module -> com.unity.modules. (lowercased), and UnityEngine.UI -> com.unity.ugui. EXCEPTIONS to that lowercase rule: some module assemblies have NO matching com.unity.modules. package id. Most notably UnityEngine.GridModule (Grid/GridLayout) has NO 'com.unity.modules.grid' package -- declare 'com.unity.modules.tilemap' (the built-in module package for Grid/Tilemap functionality, which a default Unity project ships) instead. Declaring a non-existent id makes UPM fail resolution ('Package [id] cannot be found') BEFORE compilation, producing no results.xml. Every id below MUST be a real built-in package id from https://docs.unity3d.com/Manual/pack-build.html (plus com.unity.ugui); scripts/lint-unity-test-modules.ps1 enforces this.", + "modules": { + "com.unity.modules.animation": "1.0.0", + "com.unity.modules.audio": "1.0.0", + "com.unity.modules.imageconversion": "1.0.0", + "com.unity.modules.imgui": "1.0.0", + "com.unity.modules.jsonserialize": "1.0.0", + "com.unity.modules.particlesystem": "1.0.0", + "com.unity.modules.physics": "1.0.0", + "com.unity.modules.physics2d": "1.0.0", + "com.unity.modules.tilemap": "1.0.0", + "com.unity.modules.ui": "1.0.0", + "com.unity.modules.uielements": "1.0.0", + "com.unity.ugui": "1.0.0" + }, + "_evidence": { + "com.unity.modules.animation": "Runtime/Core/Extension/AnimatorExtensions.cs, Runtime/Utils/AnimatorEnumStateMachine.cs (Animator, AnimationClip, AnimationEvent)", + "com.unity.modules.audio": "TEST-ONLY: Tests/Editor/CustomDrawers/TestTypes/*.cs fixtures (AudioSource/AudioClip [WNotNull] fields). Not used by shipped code.", + "com.unity.modules.tilemap": "Runtime/Core/Extension/Partials/GridConvexHull.cs (Grid grid parameter). Grid/GridLayout live in UnityEngine.GridModule; there is NO standalone 'com.unity.modules.grid' package, so the test project declares com.unity.modules.tilemap (the built-in Grid/Tilemap module package a default Unity project ships) to cover Grid usage.", + "com.unity.modules.imageconversion": "Editor/Sprites/SpriteCropper.cs (Texture2D.EncodeToPNG)", + "com.unity.modules.imgui": "Editor/* (GUIContent/GUILayout/GUIStyle, 64 files)", + "com.unity.modules.jsonserialize": "Editor/PrefabChecker.cs (JsonUtility)", + "com.unity.modules.particlesystem": "Runtime/Core/Serialization/JsonConverters/MinMaxCurveConverter.cs (ParticleSystem.MinMaxCurve)", + "com.unity.modules.physics": "Runtime/Core/Serialization/JsonConverters/RaycastHitConverter.cs (RaycastHit)", + "com.unity.modules.physics2d": "Runtime/Utils/CollisionProxy.cs (Collider2D, Collision2D)", + "com.unity.modules.ui": "Transitive base of com.unity.ugui (UnityEngine.UIModule: Canvas/CanvasRenderer); declared explicitly for the test project.", + "com.unity.modules.uielements": "Runtime/Visuals/UIToolkit/MultiFileSelectorElement.cs (VisualElement, ListView)", + "com.unity.ugui": "Runtime/Visuals/UGUI/EnhancedImage.cs, Runtime/Core/Serialization/JsonConverters/ColorBlockConverter.cs (UnityEngine.UI: Image, ColorBlock)" + } +} diff --git a/.github/unity-versions.json b/.github/unity-versions.json new file mode 100644 index 000000000..d5a93f256 --- /dev/null +++ b/.github/unity-versions.json @@ -0,0 +1,5 @@ +{ + "_comment": "Canonical Unity version source of truth for all CI. The newest entry of `all` (last element) is the 'latest' tracked by unity-benchmarks. `release` is the version release/publish flows pin. Bump versions HERE so every consumer stays in sync.", + "all": ["2021.3.45f1", "2022.3.45f1", "6000.3.16f1"], + "release": "2022.3.45f1" +} diff --git a/.github/workflows/asmdef-lint.yml b/.github/workflows/asmdef-lint.yml index 1df644b4e..46dea8a49 100644 --- a/.github/workflows/asmdef-lint.yml +++ b/.github/workflows/asmdef-lint.yml @@ -7,12 +7,17 @@ on: paths: - "**.asmdef" - "**.asmref" + # .cs is in scope: the cross-assembly check verifies that any `#if ` + # lives in an assembly whose asmdef declares that symbol, so a code-only change can + # introduce (or fix) a finding without touching an asmdef. + - "**.cs" - "scripts/lint-asmdef.ps1" - ".github/workflows/asmdef-lint.yml" pull_request: paths: - "**.asmdef" - "**.asmref" + - "**.cs" - "scripts/lint-asmdef.ps1" - ".github/workflows/asmdef-lint.yml" diff --git a/.github/workflows/lint-doc-links.yml b/.github/workflows/lint-doc-links.yml index 3ddaabac8..9223c121d 100644 --- a/.github/workflows/lint-doc-links.yml +++ b/.github/workflows/lint-doc-links.yml @@ -1,17 +1,37 @@ name: Lint Docs Links +# Link checking is split by reliability, NOT by file: +# * INTERNAL / relative links (do the linked files and anchors exist?) are +# deterministic and entirely within this repo's control, so they gate every +# PR via the PowerShell linters below. +# * EXTERNAL links (third-party sites, GitHub issue/PR pages) fail for reasons +# OUTSIDE a contributor's control -- rate limits (HTTP 429), org/repo +# transfers, and ordinary link rot. Gating PRs on them punishes contributors +# for the internet's flakiness and grows an endless manual exclude list, so +# the lychee external sweep runs on a SCHEDULE instead. A failing scheduled +# run notifies the maintainers about rot without ever blocking a PR. on: push: branches: - main # Run on every PR: doc-link rot can be introduced by changes to any file type. pull_request: + schedule: + # Weekly external-link rot sweep (Mondays 12:00 UTC). Offset from the Unity + # Tests cron so the two do not contend for scheduler capacity. + - cron: "0 12 * * 1" + workflow_dispatch: permissions: contents: read jobs: - lint: + # ── INTERNAL links + linter self-tests: gates every PR ────────────────────── + internal-links: + name: Internal doc links + # Skip on the scheduled trigger; the external sweep is the only thing the + # cron needs to run. + if: github.event_name != 'schedule' runs-on: ubuntu-latest steps: - name: Checkout @@ -33,7 +53,17 @@ jobs: shell: pwsh run: ./scripts/lint-doc-links.ps1 -VerboseOutput - - name: Check dead links (lychee) + # ── EXTERNAL links: scheduled rot sweep, never blocks a PR ────────────────── + external-links: + name: External link rot sweep (scheduled) + # Run only on the weekly cron or an explicit manual dispatch -- never on a PR. + if: github.event_name == 'schedule' || github.event_name == 'workflow_dispatch' + runs-on: ubuntu-latest + steps: + - name: Checkout + uses: actions/checkout@v6 + + - name: Check external links (lychee) uses: lycheeverse/lychee-action@v2.8.0 with: args: >- @@ -43,8 +73,8 @@ jobs: --verbose -- "./**/*.md" - # Fail workflow only for actual broken links, not transient server errors - # The .lychee.toml config already accepts 5xx as valid to handle outages + # A red scheduled run is the rot NOTIFICATION; .lychee.toml already + # accepts 429/5xx so only genuine, persistent 404s surface here. fail: true env: GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} diff --git a/.github/workflows/llm-instructions-lint.yml b/.github/workflows/llm-instructions-lint.yml index 1a7b8e2dc..ea5f715ac 100644 --- a/.github/workflows/llm-instructions-lint.yml +++ b/.github/workflows/llm-instructions-lint.yml @@ -27,7 +27,16 @@ permissions: jobs: lint-llm-instructions: - runs-on: ubuntu-latest + # Run on BOTH Ubuntu and Windows so the skills-index generator's cross-OS + # determinism (ordinal sort, UTF-8 no BOM, LF) is actually enforced in CI, not + # just assumed. A Windows-only drift (e.g. a BOM from Set-Content -Encoding + # UTF8 on PowerShell 5.1) would fail the Windows leg. + name: lint-llm-instructions (${{ matrix.os }}) + strategy: + fail-fast: false + matrix: + os: [ubuntu-latest, windows-latest] + runs-on: ${{ matrix.os }} steps: - name: Checkout uses: actions/checkout@v6 @@ -47,6 +56,9 @@ jobs: node-version: "22" - name: Install dependencies + # Explicit bash so the POSIX `if [ -f ... ]` works on the Windows runner + # (whose default `run:` shell is pwsh). Git for Windows provides bash. + shell: bash run: | if [ -f package-lock.json ]; then npm ci @@ -71,13 +83,29 @@ jobs: run: ./scripts/lint-skill-sizes.ps1 -VerboseOutput - name: Validate skills index is up-to-date + # Regenerate the file and fail if it differs from the committed bytes. This + # is the cross-OS guard: if Windows and Ubuntu generate different content, + # one leg's git diff is non-empty. The lint step above also asserts this, + # plus encoding (no BOM / LF) and generator determinism. shell: pwsh run: | - ./scripts/generate-skills-index.ps1 -VerboseOutput - $diff = git diff -- .llm/context.md - if ($diff) { - Write-Host "::error::Skills index in .llm/context.md is out of date. Run: pwsh -NoProfile -File scripts/generate-skills-index.ps1" - Write-Host $diff + # generate-skills-index.ps1 is pure PowerShell (it runs no native process), + # so on success it leaves $LASTEXITCODE unset ($null) in this fresh pwsh step. + # The old `if ($LASTEXITCODE -ne 0)` fired a phantom failure ($null -ne 0 is + # $true in PowerShell), logged as "failed (exit )." with an empty code. Gate on + # truthiness instead: `if ($LASTEXITCODE)` is false for BOTH $null and 0, and + # true only for a real non-zero code (the generator's `exit 1` on a missing + # skills dir), which we then surface. A thrown / ErrorActionPreference='Stop' + # error already aborts the step before this line; the git-status check below is + # the substantive assertion (drift / never-committed index.md). + ./scripts/generate-skills-index.ps1 + if ($LASTEXITCODE) { exit $LASTEXITCODE } + # --porcelain catches BOTH a drifted (modified) and a never-committed + # (untracked) index.md, unlike `git diff` which ignores untracked files. + $status = git status --porcelain -- .llm/skills/index.md + if ($status) { + Write-Host "::error::.llm/skills/index.md is out of date or uncommitted. Run: pwsh -NoProfile -File scripts/generate-skills-index.ps1 and commit it." + git diff -- .llm/skills/index.md exit 1 } Write-Host "Skills index is up-to-date." diff --git a/.github/workflows/pwsh-invocations-lint.yml b/.github/workflows/pwsh-invocations-lint.yml index c9ca598df..ea4ec4502 100644 --- a/.github/workflows/pwsh-invocations-lint.yml +++ b/.github/workflows/pwsh-invocations-lint.yml @@ -8,6 +8,7 @@ on: - ".githooks/**" - ".github/workflows/**.yml" - ".github/workflows/**.yaml" + - ".github/actions/**" - "**/*.sh" - "scripts/**.ps1" - "package.json" @@ -16,6 +17,7 @@ on: - ".githooks/**" - ".github/workflows/**.yml" - ".github/workflows/**.yaml" + - ".github/actions/**" - "**/*.sh" - "scripts/**.ps1" - "package.json" @@ -39,3 +41,11 @@ jobs: - name: Lint PowerShell invocations shell: pwsh run: ./scripts/lint-pwsh-invocations.ps1 -VerboseOutput + + - name: Test workflow run-expression-length linter + shell: pwsh + run: ./scripts/tests/test-lint-workflow-run-expression-length.ps1 -VerboseOutput + + - name: Lint workflow run-expression length + shell: pwsh + run: ./scripts/lint-workflow-run-expression-length.ps1 -VerboseOutput diff --git a/.github/workflows/runner-bootstrap.yml b/.github/workflows/runner-bootstrap.yml new file mode 100644 index 000000000..cf8d64100 --- /dev/null +++ b/.github/workflows/runner-bootstrap.yml @@ -0,0 +1,403 @@ +name: Runner Bootstrap (Windows) + +# cspell:ignore Redistributables redist redists MSVCP MSVCR VCRUNTIME UCRT prereqs + +# Operator-driven, workflow_dispatch-only bootstrap of a self-hosted Windows +# runner's host-OS prerequisites for Unity CI. Installs the Microsoft Visual +# C++ 2010 SP1 + 2015-2022 x64 Redistributables (BOTH generations -- Unity +# 2021/2022/6000 depend on each: MSVCP100.dll/MSVCR100.dll come from the +# 2010 redist, VCRUNTIME140.dll/MSVCP140.dll/VCRUNTIME140_1.dll from the +# 2015-2022 redist), Windows long-paths, Windows Defender exclusions, +# PowerShell 7, and (where applicable) the UCRT KB. +# +# Root cause this workflow addresses: Unity.exe failed at startup with +# -1073741515 / 0xC0000135 (STATUS_DLL_NOT_FOUND) on DAD-MACHINE because the +# VC++ Redistributables were missing -- Unity 2021/2022/6000 depend on BOTH +# the 2010 SP1 (MSVCP100.dll / MSVCR100.dll) AND the 2015-2022 x64 (MSVCP140 + +# VCRUNTIME140) generations. GitHub-hosted windows-2022 ships both preinstalled; +# self-hosted runners do not. The fix is OS-level (install the redists), so +# ensure-editor.ps1's Unity-reinstall retry loop is futile. This workflow +# gives operators a one-click recovery path that does not require shelling +# onto the runner host. +# +# Targeting a SPECIFIC runner (HARD-FAIL on wrong target): +# * The repo's runner topology currently labels both Windows runners +# uniformly (`self-hosted, Windows, RAM-64GB`); only ELI-MACHINE carries +# the additional `fast` label (the org runner topology). +# * To bootstrap one specific machine without affecting the other, take +# the OTHER runner offline (gh CLI or Actions UI -> Runners) BEFORE +# dispatching this workflow, then bring it back online afterwards. +# * If the dispatch lands on the wrong runner the workflow HARD-FAILS with +# `::error::` and exit 1 (NOT a silent ::warning::). This is by design: +# a silent bootstrap of the WRONG (healthy) machine is operator-fatal. +# * Long-term fix: add machine-name labels to runner agents so the +# `runs-on:` set can include the runner name itself. +# +# Idempotent: the bootstrap script is safe to re-run on a healthy host +# (every prereq is detected first and remediated only when missing). A +# repeat dispatch on the same machine is a no-op. +# +# SHELL CHOICE (chicken-and-egg): self-hosted-runner steps use +# `shell: powershell` (Windows PowerShell 5.1, ALWAYS preinstalled on +# Windows) rather than `shell: pwsh` (PowerShell 7). This workflow's +# purpose INCLUDES installing pwsh on runners that lack it -- so it cannot +# require pwsh in order to run. The bootstrap script declares +# `#Requires -Version 5.1` and is verified PS 5.1-compatible. The +# ubuntu-latest preflight job uses `shell: bash` as normal. +# +# MODE RESOLUTION PRECEDENCE (highest wins): +# 1. UH_RUNNER_DISABLE_AUTO_BOOTSTRAP=1 -> forces detect-only (composite) +# 2. inputs.detect-only == true -> detect-only via the composite +# 3. (composite default) -> auto-install +# Operators flipping the dispatch toggle should know that the composite +# input is the workflow-author surface; the env var is the operator override. + +on: + workflow_dispatch: + inputs: + runner-label: + description: >- + (HARD-FAIL on wrong target) Name of the runner you want to bootstrap + (DAD-MACHINE or ELI-MACHINE). Both runners share the same label set, + so the scheduler may land on either machine. If it lands on the + WRONG one, this workflow FAILS FAST with exit 1 -- it does NOT + silently bootstrap the unintended machine. Take the unwanted runner + offline (Settings -> Runners) BEFORE dispatching to guarantee + targeting. + required: true + type: choice + options: + - DAD-MACHINE + - ELI-MACHINE + detect-only: + description: >- + When true, run the bootstrap script with -DetectOnly (no installs); + the workflow fails with exit 2 if any prereq is missing. Useful for + a pure preflight audit without mutating the host. Accepts the + natural truthy strings (true/True/1/yes/y); normalized server-side. + required: false + type: boolean + default: false + +# Forbid concurrent bootstraps OF THIS REPO across BOTH runners. The group +# is intentionally NOT parameterized by `inputs.runner-label` so that two +# dispatches (one per machine) cannot race on installer locks or quarantine +# the same Windows feature/UCRT machine-wide state simultaneously. This is +# stricter than per-runner concurrency: any in-flight bootstrap anywhere +# blocks any other bootstrap dispatch from starting. +# Distinct from `wallstop-organization-builds` (the Unity build lock) -- this +# workflow does not enter the Unity license seat path, so it must not block +# Unity jobs and Unity jobs must not block it. +concurrency: + group: runner-bootstrap-windows + cancel-in-progress: false + +permissions: + contents: read + actions: read + +jobs: + # Preflight convention: every job whose runs-on declares `self-hosted` + # must `needs:` a runner-access preflight (see + # docs/runbooks/unity-runners-after-transfer.md). This mirrors the + # unity-tests.yml runner-preflight pattern exactly. + runner-preflight: + name: Self-hosted runner access preflight + runs-on: ubuntu-latest + timeout-minutes: 3 + permissions: + actions: read + # NOTE: `administration: read` is NOT a valid permissions key for + # workflow GITHUB_TOKEN (per actionlint and the GitHub docs schema). + # Listing self-hosted runners requires admin:org (org-scope) OR a + # fine-grained PAT with repo "Administration: read" -- neither of + # which the default GITHUB_TOKEN can carry. The soft-pass path in + # the run script below is the supported coverage when the token + # is unscoped. See docs/runbooks/unity-runners-after-transfer.md. + concurrency: + group: ${{ github.workflow }}-runner-preflight-${{ github.ref }} + cancel-in-progress: true + steps: + - name: Probe self-hosted runner availability + shell: bash + env: + GH_TOKEN: ${{ secrets.RUNNER_AUDIT_PAT || secrets.GITHUB_TOKEN }} + REQUIRED_LABELS: "self-hosted,Windows,RAM-64GB" + REQUIRED_RUNNER_NAME: ${{ inputs.runner-label }} + # gh api --paginate | jq -s pattern mirrors stuck-job-watchdog.yml and + # unity-tests.yml. CRITICAL: this preflight must NEVER make CI more broken + # than the no-preflight baseline -- if both runner-inventory endpoints + # return 403/404, soft-pass with a ::warning::. When the inventory IS + # visible AND the named runner is offline, hard-fail (operator action + # required before bootstrap can dispatch). + run: | + set -euo pipefail + echo "::group::Runner-access preflight" + echo "Required labels: ${REQUIRED_LABELS}" + echo "Required runner name: ${REQUIRED_RUNNER_NAME}" + IFS=',' read -r -a required <<< "${REQUIRED_LABELS}" + runners_json="$(mktemp)" + runners_scope="" + if gh api --paginate \ + "orgs/${GITHUB_REPOSITORY_OWNER}/actions/runners?per_page=100" \ + 2>/dev/null \ + | jq -s '[.[] | (.runners // [])[]]' > "${runners_json}" 2>/dev/null; then + runners_scope="org" + else + if gh api --paginate \ + "repos/${GITHUB_REPOSITORY}/actions/runners?per_page=100" \ + 2>/dev/null \ + | jq -s '[.[] | (.runners // [])[]]' > "${runners_json}" 2>/dev/null; then + runners_scope="repo" + fi + fi + if [ -z "${runners_scope}" ]; then + org_url="orgs/${GITHUB_REPOSITORY_OWNER}/actions/runners" + repo_url="repos/${GITHUB_REPOSITORY}/actions/runners" + { + echo "::warning::Runner inventory unavailable from both ${org_url}" + echo "::warning::and ${repo_url} (likely 403: GITHUB_TOKEN lacks admin:org" + echo "::warning::or administration:read)." + echo "::warning::See docs/runbooks/unity-runners-after-transfer.md" + echo "::warning::for how to plumb a RUNNER_AUDIT_PAT secret to upgrade" + echo "::warning::this soft-pass to a hard-pass." + } + echo "Soft pass: skipping runner inventory check." + echo "::endgroup::" + exit 0 + fi + echo "Runner inventory scope: ${runners_scope}" + total="$(jq 'length' < "${runners_json}")" + echo "Visible runners (${runners_scope} scope): ${total}" + matched="$(jq --argjson labels "$(printf '%s\n' "${required[@]}" | jq -R . | jq -s .)" ' + [ .[] + | select(.status == "online") + | select(($labels | all(. as $l | (.labels // []) | map(.name) | index($l) | type == "number"))) + ] | length + ' < "${runners_json}")" + echo "Runners satisfying ${REQUIRED_LABELS}: ${matched}" + if [ "${matched}" -lt 1 ]; then + { + echo "::error::No online self-hosted runner satisfies the required labels (${REQUIRED_LABELS})." + echo "Bootstrap cannot dispatch. Bring the target runner online first." + echo "See docs/runbooks/unity-runners-after-transfer.md for the post-transfer runner-group ACL fix." + } + jq '[ .[] | {name, status, busy, labels: [(.labels // [])[].name]} ]' \ + < "${runners_json}" + exit 1 + fi + # F12: the label match above is necessary but not sufficient when the + # operator named a SPECIFIC runner. Require that the named runner is + # also online (otherwise the dispatch would land on the other + # machine, which the bootstrap job's "Confirm runner identity" step + # then HARD-FAILS on -- catching it here gives a clearer error + # before consuming a self-hosted slot). + named_online="$(jq --arg name "${REQUIRED_RUNNER_NAME}" ' + [ .[] | select(.name == $name) | select(.status == "online") ] | length + ' < "${runners_json}")" + echo "Named runner online (name='${REQUIRED_RUNNER_NAME}'): ${named_online}" + if [ "${named_online}" -lt 1 ]; then + { + echo "::error::Requested runner '${REQUIRED_RUNNER_NAME}' is not currently online." + echo "Bring it online via Settings -> Runners (or the runner-host service) before dispatching." + echo "See docs/runbooks/unity-runners-after-transfer.md." + } + jq --arg name "${REQUIRED_RUNNER_NAME}" ' + [ .[] | select(.name == $name) | {name, status, busy, labels: [(.labels // [])[].name]} ] + ' < "${runners_json}" + exit 1 + fi + echo "::endgroup::" + + bootstrap: + # F9: surface the mode (Audit vs Maintenance) in the job display name so + # operators reading the Actions UI immediately see which mode this run is + # without expanding the inputs. + name: ${{ inputs.detect-only && 'Audit' || 'Maintain' }} ${{ inputs.runner-label }} + needs: + - runner-preflight + # Cannot dynamically pin to a specific runner without machine-name labels + # configured on the runners themselves (operator action; not yet done in + # this org). Use the documented allowlisted set; operator takes the + # unwanted runner offline before dispatching when targeting is needed. + # The "Confirm runner identity" step below HARD-FAILS if the dispatch + # lands on the wrong machine (see header comment for the chicken-and-egg + # rationale around `shell: powershell` for the rest of this job). + runs-on: [self-hosted, Windows, RAM-64GB] + # F13: inherit workflow-level permissions (contents: read, actions: read). + # No job-level override -- the previous explicit block silently dropped + # `actions: read` and shadowed the workflow-level intent. + timeout-minutes: 360 + steps: + - name: Enable Git long paths + shell: powershell + env: + GIT_CONFIG_GLOBAL: ${{ runner.temp }}/uh-gitconfig + run: git config --global core.longpaths true + + - name: Checkout + # Shallow checkout: bootstrap only needs scripts/unity/* and a + # composite action; no history depth required. + uses: actions/checkout@v6 + env: + GIT_CONFIG_GLOBAL: ${{ runner.temp }}/uh-gitconfig + with: + persist-credentials: false + + - name: Confirm runner identity (hard-fail on wrong target) + # F15: Windows PowerShell 5.1, NOT pwsh -- this workflow's purpose + # includes installing pwsh, so requiring pwsh here would be the + # chicken-and-egg failure mode. + shell: powershell + env: + UH_BOOTSTRAP_REQUESTED_RUNNER: ${{ inputs.runner-label }} + run: | + Set-StrictMode -Version Latest + $ErrorActionPreference = 'Stop' + $actual = $env:RUNNER_NAME + $requested = $env:UH_BOOTSTRAP_REQUESTED_RUNNER + Write-Host "Requested runner: $requested" + Write-Host "Actual runner: $actual" + # F2: HARD-FAIL on mismatch. The previous ::warning:: would silently + # bootstrap the wrong (healthy) machine when the scheduler picked + # the runner the operator did NOT intend. + if (-not [string]::IsNullOrWhiteSpace($actual) -and -not [string]::IsNullOrWhiteSpace($requested)) { + if (-not [string]::Equals($actual, $requested, [System.StringComparison]::OrdinalIgnoreCase)) { + Write-Host ("::error::Bootstrap dispatched to '$actual' but operator requested '$requested'. Cancel this run, take '$actual' offline (Settings -> Runners or 'gh api -X POST " + + "orgs//actions/runners//...'), then re-dispatch so the run lands on '$requested'. Alternatively, add machine-name labels to runner agents so 'runs-on:' can pin to a " + + "specific runner.") + if ($env:GITHUB_STEP_SUMMARY) { + Add-Content -Path $env:GITHUB_STEP_SUMMARY -Value ("## Runner Bootstrap (FAILED: wrong target)`n`n- Requested runner: ``$requested```n- Actual runner: ``$actual```n- Action: " + + "hard-fail; take ``$actual`` offline and re-dispatch.") + } + exit 1 + } + } + # Surface to the step summary for the Actions UI. + $summary = @( + "## Runner Bootstrap" + "" + "- Requested runner: ``$requested``" + "- Actual runner: ``$actual``" + "- Detect-only: ``${{ inputs.detect-only }}``" + ) -join "`n" + if ($env:GITHUB_STEP_SUMMARY) { + Add-Content -Path $env:GITHUB_STEP_SUMMARY -Value $summary + } + + - name: Run runner maintenance (single-step transcript) + # F5/F7: a single PowerShell 5.1 step opens the transcript, invokes + # the bootstrap script, and Stop-Transcripts in a finally{} -- the + # previous design split Start-Transcript and the bootstrap into + # separate pwsh processes, leaving the artifact empty. + # F15: Windows PowerShell 5.1 -- see header comment. + # F22: we deliberately do NOT `uses: ./.github/actions/assert-unity-host-prereqs` + # here, even though that composite would normally own this orchestration. + # The composite contains a PS 5.1 preflight (F14) that hard-fails when + # pwsh is missing -- but pwsh missing is EXACTLY the case this + # workflow exists to repair (chicken-and-egg). To avoid duplication + # without breaking the bootstrap-out-of-pwsh-missing case, this + # workflow's logic is kept thin: it owns ONLY the transcript wrapper + # and the detect-only normalization. All real orchestration + # (auto-install vs DetectOnly, redist install, defender exclusions, + # UCRT, pwsh install) lives inside scripts/unity/bootstrap-windows-runner.ps1 + # -- the SAME script the composite invokes. The composite remains the + # canonical Unity-job-side entry point. + # F4: parse inputs.detect-only via natural truthy normalization + # rather than string-equality against the unquoted 'true' literal. + # Also honor UH_RUNNER_DISABLE_AUTO_BOOTSTRAP=1 here (forces + # DetectOnly) so the operator override behaves identically across + # the workflow path and the composite path. + shell: powershell + env: + UH_BOOTSTRAP_DETECT_ONLY: ${{ inputs.detect-only }} + run: | + Set-StrictMode -Version Latest + $ErrorActionPreference = 'Stop' + $artifactDir = Join-Path $env:GITHUB_WORKSPACE '.artifacts\runner-bootstrap' + New-Item -ItemType Directory -Force -Path $artifactDir | Out-Null + $timestamp = (Get-Date -Format 'yyyyMMdd-HHmmss') + $logPath = Join-Path $artifactDir ("maintenance-{0}-{1}-{2}.log" -f $timestamp, $env:GITHUB_RUN_ID, $env:GITHUB_RUN_ATTEMPT) + Write-Host "Transcript: $logPath" + + # F4: normalize detect-only to a single bool. Accepts true/True/1/yes/y. + $rawDetect = '' + $env:UH_BOOTSTRAP_DETECT_ONLY + $normalizedDetect = $rawDetect.Trim().ToLowerInvariant() + $detectOnly = ($normalizedDetect -eq 'true' -or $normalizedDetect -eq '1' -or $normalizedDetect -eq 'yes' -or $normalizedDetect -eq 'y') + # Operator escape hatch -- UH_RUNNER_DISABLE_AUTO_BOOTSTRAP=1 + # forces DetectOnly even when the dispatch input says auto-install. + # Mirrors the composite (./.github/actions/assert-unity-host-prereqs) + # so this workflow path and the composite path observe the same + # precedence rule (per the header's MODE RESOLUTION PRECEDENCE list). + if ($env:UH_RUNNER_DISABLE_AUTO_BOOTSTRAP -eq '1') { + Write-Host "::notice::UH_RUNNER_DISABLE_AUTO_BOOTSTRAP=1 -> forcing DetectOnly (overrides inputs.detect-only)" + $detectOnly = $true + } + Write-Host "Resolved detect-only: $detectOnly (raw='$rawDetect')" + + # We are inside a workflow (not a composite), so $GITHUB_WORKSPACE is + # reliably the repo root post-checkout (actions/checkout@v6 above). + # + # TODO(unity-helpers): scripts/unity/maintain-windows-runner.ps1 (and + # the scripts/unity/bootstrap-windows-runner.ps1 it wraps, plus the + # .github/actions/assert-unity-host-prereqs composite) were NOT ported + # in this batch -- they are the host-OS remediation backend (VC++ + # redist, long-paths, Defender exclusions, pwsh, UCRT installs). Until + # they are ported from DxMessaging, this workflow HARD-FAILS below with + # a clear "script not found" error instead of silently doing nothing. + # The runner-preflight job above (runner online + label check) still + # works standalone; only this maintenance step needs the backend. + $script = Join-Path $env:GITHUB_WORKSPACE 'scripts\unity\maintain-windows-runner.ps1' + if (-not (Test-Path -LiteralPath $script)) { + Write-Host "::error::Runner maintenance script not found at $script (see TODO(unity-helpers) in this workflow: scripts/unity/maintain-windows-runner.ps1 has not been ported yet)." + exit 1 + } + + $code = 0 + try { + Start-Transcript -Path $logPath -Force | Out-Null + . $script + $unityVersions = @('2021.3.45f1', '2022.3.45f1', '6000.3.16f1') + $provisioningProfile = 'StandaloneWindowsIl2Cpp' + $installRoot = if ($env:UNITY_EDITOR_INSTALL_ROOT) { $env:UNITY_EDITOR_INSTALL_ROOT } else { 'C:\Unity\Editors' } + $maintenanceArgs = @{ + UnityVersions = $unityVersions + ProvisioningProfile = $provisioningProfile + InstallRoot = $installRoot + Force = $true + DiagnosticsRoot = $artifactDir + } + if ($detectOnly) { + $maintenanceArgs.DetectOnly = $true + } + $code = Invoke-WindowsRunnerMaintenance @maintenanceArgs + } finally { + try { Stop-Transcript | Out-Null } catch { Write-Host "::notice::Stop-Transcript: $($_.Exception.Message) (non-fatal)" } + } + + Write-Host "Runner maintenance exit code: $code" + if ($env:GITHUB_STEP_SUMMARY) { + $label = switch ($code) { + 0 { 'ok' } + 2 { 'detect-only-missing' } + 3 { 'mutex-busy' } + 4 { 'runner-busy' } + default { "fail($code)" } + } + Add-Content -Path $env:GITHUB_STEP_SUMMARY -Value ("- Runner maintenance result: ``" + $label + "`` (exit $code)") + Add-Content -Path $env:GITHUB_STEP_SUMMARY -Value ("- Transcript: ``" + $logPath + "``") + } + if ($code -ne 0) { exit $code } + + # F1: actions/upload-artifact@v7 -- matches the repo-wide pinned + # version used by unity-tests.yml/unity-benchmarks.yml. + # F19: if-no-files-found is `error` (not `warn`) so a missing transcript + # is a HARD failure rather than a silent empty artifact upload. + - name: Upload bootstrap transcript + if: always() + uses: actions/upload-artifact@v7 + with: + name: runner-bootstrap-${{ inputs.runner-label }}-${{ github.run_id }}-${{ github.run_attempt }} + path: .artifacts/runner-bootstrap/** + if-no-files-found: error + retention-days: 30 diff --git a/.github/workflows/stuck-job-watchdog.yml b/.github/workflows/stuck-job-watchdog.yml new file mode 100644 index 000000000..a232f818b --- /dev/null +++ b/.github/workflows/stuck-job-watchdog.yml @@ -0,0 +1,519 @@ +name: Stuck Job Watchdog + +# cspell:ignore pushd popd + +# Recovery automation for the known GitHub Actions self-hosted dispatcher bug +# documented at https://github.com/orgs/community/discussions/186811: +# self-hosted runners report Online/Idle but `runner_id` stays at 0 for +# 7+ minutes, so a queued job whose label set is satisfied by an idle runner +# nonetheless waits indefinitely until manually re-run. +# +# Detection requires ALL of the following to be true: +# * The workflow run is `status: queued` AND older than MIN_QUEUE_AGE_SECONDS +# (default 300s / 5 min; round-2 false-positive guards below -- zero +# in-progress jobs, self-run exclusion, excluded-workflow list -- make +# the previous conservative 600s buffer unnecessary). +# * No job in the run is `status: in_progress` - a run with even one +# in-progress job is by definition holding/using a runner, not +# dispatcher-stuck. This is what prevents false positives for matrix cells +# or jobs waiting while another job from the same run is actively running. +# * At least one job in the run is `status: queued`. +# * At least one idle runner's labels satisfy a queued job's label +# requirements (superset match). +# * The run's workflow file is NOT in the exclusion list (`release.yml` +# is hard-excluded by default; additional entries may be added via the +# `WATCHDOG_EXCLUDED_WORKFLOWS` repo variable, whitespace-separated). +# * The run id is NOT the watchdog's own run id. +# +# Recovery action (per cli/cli#9221 and the gh-run-rerun manual, +# `gh run rerun --failed` cannot be used on a `status: queued` run because +# the run never reached `failed`; the documented workaround is cancel + +# redispatch / cancel + operator-managed re-run): +# * `gh run cancel ` kills the stuck dispatch. +# * If the run was triggered by push/schedule/workflow_dispatch on a +# branch (not a tag) AND the workflow file declares `workflow_dispatch:`, +# re-dispatch via REST API +# (`POST repos/{owner}/{repo}/actions/workflows/{workflow_id}/dispatches`). +# * Otherwise (pull_request, tag, no workflow_dispatch trigger), emit a +# clear `GITHUB_STEP_SUMMARY` line instructing the operator to click +# "Re-run all jobs" in the GitHub UI. Do NOT push a commit, comment on +# the PR, or escalate any other automatic action. +# +# Cancel attempts are capped at 2 per run-id per 24h via a small state file +# on the `watchdog-state` orphan branch. State is pushed immediately after each +# successful cancel, with a single rebase+retry on push failure. +# +# Workflow-level concurrency guarantees only one watchdog instance ever runs; +# newer schedules must not cancel an in-flight audit after it has cancelled a +# stuck run but before it persists the state-branch counter. + +on: + schedule: + - cron: "*/5 * * * *" + workflow_dispatch: + +concurrency: + group: stuck-job-watchdog + cancel-in-progress: false + +permissions: + actions: write + contents: write + +jobs: + audit-queue: + name: Audit queued runs and recover dispatcher-stuck ones + runs-on: ubuntu-latest + timeout-minutes: 10 + steps: + - name: Audit + cancel-and-redispatch + shell: bash + env: + GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} + REPO: ${{ github.repository }} + OWNER: ${{ github.repository_owner }} + SELF_RUN_ID: ${{ github.run_id }} + STATE_BRANCH: watchdog-state + STATE_DIR: .watchdog-state + MAX_CANCELS_PER_DAY: "2" + MIN_QUEUE_AGE_SECONDS: "300" + DEFAULT_EXCLUDED_WORKFLOWS: "release.yml" + EXTRA_EXCLUDED_WORKFLOWS: ${{ vars.WATCHDOG_EXCLUDED_WORKFLOWS }} + run: | + set -euo pipefail + + summary_file="$(mktemp)" + : > "${summary_file}" + log_summary() { + printf '%s\n' "$1" | tee -a "${summary_file}" + } + + flush_summary_and_exit() { + local code="${1:-0}" + { + echo "## Watchdog summary" + cat "${summary_file}" + } >> "${GITHUB_STEP_SUMMARY}" + exit "${code}" + } + + # Category buckets for the final step-summary table. + healthy_runs_file="$(mktemp)" + : > "${healthy_runs_file}" + stuck_runs_file="$(mktemp)" + : > "${stuck_runs_file}" + excluded_runs_file="$(mktemp)" + : > "${excluded_runs_file}" + + log_summary "## Stuck-job watchdog audit ($(date -u +'%Y-%m-%dT%H:%M:%SZ'))" + log_summary "Repo: ${REPO}" + log_summary "Owner: ${OWNER}" + log_summary "Self run id (will skip): ${SELF_RUN_ID}" + + # Build the workflow-file exclusion list (default + repo variable). + declare -A EXCLUDED_BY_FILE=() + for wf in ${DEFAULT_EXCLUDED_WORKFLOWS} ${EXTRA_EXCLUDED_WORKFLOWS:-}; do + [[ -z "${wf}" ]] && continue + # Normalize: strip any leading .github/workflows/ if present. + base="${wf##*/}" + EXCLUDED_BY_FILE["${base}"]=1 + done + excluded_list="" + for k in "${!EXCLUDED_BY_FILE[@]}"; do + excluded_list+="${k} " + done + log_summary "Excluded workflows: ${excluded_list:-}" + + # ------------------------------------------------------------------ + # 1. Bootstrap or check out the watchdog-state orphan branch. + # ------------------------------------------------------------------ + work_dir="$(mktemp -d)" + pushd "${work_dir}" > /dev/null + # Scope credentials to the exact remote commands that need them; the + # cloned repo keeps a plain HTTPS origin with no tokenized remote. + auth_header="$(printf 'x-access-token:%s' "${GH_TOKEN}" | base64 | tr -d '\n')" + git_auth() { + git -c "http.https://github.com/.extraheader=AUTHORIZATION: basic ${auth_header}" "$@" + } + git_auth clone --depth 1 "https://github.com/${REPO}.git" repo + cd repo + GIT_AUTHOR_ID=(-c "user.email=actions@github.com" -c "user.name=stuck-job-watchdog") + + # Decoupled probe + bootstrap: ls-remote distinguishes + # "branch missing" (zero rows + exit 0) from "transient fetch + # failure" (non-zero exit). We MUST NOT bootstrap on transient + # failure - that would push-corrupt an existing branch by + # rewriting it as a fresh orphan. + state_branch_probe="$(mktemp)" + if ! git_auth ls-remote --heads origin "${STATE_BRANCH}" > "${state_branch_probe}" 2>/dev/null; then + log_summary "WARN: 'git ls-remote --heads origin ${STATE_BRANCH}' failed (transient?); refusing to bootstrap. Skipping this cycle." + popd > /dev/null + flush_summary_and_exit 0 + fi + + if [[ -s "${state_branch_probe}" ]]; then + if ! git_auth fetch origin "${STATE_BRANCH}:refs/remotes/origin/${STATE_BRANCH}"; then + log_summary "WARN: state branch '${STATE_BRANCH}' exists per ls-remote but fetch failed; skipping this cycle." + popd > /dev/null + flush_summary_and_exit 0 + fi + git checkout -B "${STATE_BRANCH}" "refs/remotes/origin/${STATE_BRANCH}" + log_summary "State branch '${STATE_BRANCH}' checked out." + else + log_summary "State branch '${STATE_BRANCH}' missing -- bootstrapping orphan branch." + git checkout --orphan "${STATE_BRANCH}" + git rm -rf . > /dev/null 2>&1 || true + mkdir -p "${STATE_DIR}" + touch "${STATE_DIR}/.gitkeep" + git add "${STATE_DIR}/.gitkeep" + git "${GIT_AUTHOR_ID[@]}" commit -m "Initialize watchdog state" || true + if ! git_auth push origin "${STATE_BRANCH}"; then + log_summary "WARN: bootstrap push failed; will retry on next run." + popd > /dev/null + flush_summary_and_exit 0 + fi + fi + mkdir -p "${STATE_DIR}" + state_dirty=0 + + persist_state_changes() { + local reason="${1:-state update}" + if (( state_dirty != 1 )); then + return 0 + fi + + git add "${STATE_DIR}" + if git diff-index --cached --quiet HEAD --; then + if git rev-parse --verify "refs/remotes/origin/${STATE_BRANCH}" > /dev/null 2>&1 \ + && [[ "$(git rev-parse HEAD)" == "$(git rev-parse "refs/remotes/origin/${STATE_BRANCH}")" ]]; then + state_dirty=0 + return 0 + fi + else + if ! git "${GIT_AUTHOR_ID[@]}" commit -m "Watchdog: ${reason} at $(date -u +'%Y-%m-%dT%H:%M:%SZ')"; then + log_summary "WARN: state commit failed; counters may double-count next run." + return 1 + fi + fi + + if git_auth push origin "${STATE_BRANCH}"; then + log_summary "State branch updated (${reason})." + state_dirty=0 + return 0 + fi + + log_summary "WARN: state push failed; attempting fetch + rebase + retry." + if git_auth fetch origin "${STATE_BRANCH}:refs/remotes/origin/${STATE_BRANCH}" \ + && git rebase "refs/remotes/origin/${STATE_BRANCH}" \ + && git_auth push origin "${STATE_BRANCH}"; then + log_summary "State branch updated on second attempt (${reason}, after rebase)." + state_dirty=0 + return 0 + fi + + log_summary "WARN: state push failed twice; counters may double-count next run." + return 1 + } + + is_nonnegative_integer() { + [[ "${1:-}" =~ ^[0-9]+$ ]] + } + + # ------------------------------------------------------------------ + # 2. Enumerate queued runs older than MIN_QUEUE_AGE_SECONDS. + # ------------------------------------------------------------------ + now_epoch="$(date -u +%s)" + queued_runs_json="$(mktemp)" + # `gh api --paginate` over the runs endpoint returns one JSON object + # per page; `jq -s '[.[] | .workflow_runs[]?]'` flattens all pages + # into a single array. Equivalent to using `--slurp` on newer gh + # versions but works on all gh versions shipped with ubuntu-latest. + if ! gh api --paginate "repos/${REPO}/actions/runs?status=queued&per_page=100" \ + | jq -s '[.[] | (.workflow_runs // [])[]]' > "${queued_runs_json}"; then + log_summary "ERROR: failed to list queued runs." + popd > /dev/null + flush_summary_and_exit 0 + fi + + mapfile -t queued_ids < <(jq -r --argjson now "${now_epoch}" --argjson min "${MIN_QUEUE_AGE_SECONDS}" ' + .[] + | select(.created_at != null) + | (.created_at | fromdateiso8601) as $created + | select(($now - $created) >= $min) + | .id + ' < "${queued_runs_json}" | sort -u) + + log_summary "Queued runs older than ${MIN_QUEUE_AGE_SECONDS}s: ${#queued_ids[@]}" + if [[ ${#queued_ids[@]} -eq 0 ]]; then + log_summary "Queue is clean. No action." + popd > /dev/null + flush_summary_and_exit 0 + fi + + # ------------------------------------------------------------------ + # 3. Fetch idle runners (org first, fall back to repo on 403). + # `gh api --paginate` over an object-shaped endpoint returns one + # object per page; without `jq -s`, downstream evaluation runs + # against only the last page (silent false negatives once the + # org grows past 100 runners - see cli/cli#1268). + # ------------------------------------------------------------------ + runners_json="$(mktemp)" + runners_scope="org" + if ! gh api --paginate "orgs/${OWNER}/actions/runners?per_page=100" \ + | jq -s '[.[] | (.runners // [])[]]' > "${runners_json}" 2>/dev/null; then + runners_scope="repo" + log_summary "WARN: org-level runner list unavailable (likely 403). Falling back to repo runners." + if ! gh api --paginate "repos/${REPO}/actions/runners?per_page=100" \ + | jq -s '[.[] | (.runners // [])[]]' > "${runners_json}"; then + log_summary "ERROR: repo-level runner list also failed; cannot evaluate idle runners. No action issued." + popd > /dev/null + flush_summary_and_exit 0 + fi + fi + log_summary "Runner inventory scope: ${runners_scope}" + + idle_runners_json="$(mktemp)" + jq ' + [ .[] + | select(.status == "online") + | select(.busy == false) + | { id: .id, name: .name, labels: ([(.labels // [])[].name]) } ] + ' < "${runners_json}" > "${idle_runners_json}" + idle_count="$(jq 'length' < "${idle_runners_json}")" + log_summary "Idle runners (online + not busy): ${idle_count}" + + # ------------------------------------------------------------------ + # 4. For each queued run, decide stuck vs healthy vs excluded. + # ------------------------------------------------------------------ + state_dirty=0 + for run_id in "${queued_ids[@]}"; do + # Skip the watchdog's own run id (defense in depth - the watchdog + # workflow file is excluded by name above, but if someone renames + # the file or runs ad-hoc the id check still protects us). + if [[ "${run_id}" == "${SELF_RUN_ID}" ]]; then + log_summary "run ${run_id}: this is the watchdog's own run; skipping." + continue + fi + + # Pull the run metadata we need from the cached list (workflow + # file path, event, head_branch). + run_meta="$( + jq -c --argjson id "${run_id}" ' + .[] + | select(.id == $id) + | { + path: (.path // ""), + event: (.event // ""), + workflow_id: (.workflow_id // 0), + head_branch: (.head_branch // ""), + html_url: (.html_url // "") + } + ' < "${queued_runs_json}" | head -n 1 + )" + if [[ -z "${run_meta}" ]]; then + log_summary "run ${run_id}: metadata unavailable; skipping." + continue + fi + run_path="$(jq -r '.path' <<< "${run_meta}")" + run_event="$(jq -r '.event' <<< "${run_meta}")" + run_workflow_id="$(jq -r '.workflow_id' <<< "${run_meta}")" + run_head_branch="$(jq -r '.head_branch' <<< "${run_meta}")" + run_html_url="$(jq -r '.html_url' <<< "${run_meta}")" + run_path_base="${run_path##*/}" + + # Workflow-file exclusion (must NOT cancel release.yml). + if [[ -n "${EXCLUDED_BY_FILE[${run_path_base}]+x}" ]]; then + log_summary "run ${run_id} (${run_path_base}, event=${run_event}): workflow is excluded; operator action needed if stuck." + printf '* run %s (%s, event=%s) -- %s\n' "${run_id}" "${run_path_base}" "${run_event}" "${run_html_url}" >> "${excluded_runs_file}" + continue + fi + + jobs_json="$(mktemp)" + if ! gh api --paginate "repos/${REPO}/actions/runs/${run_id}/jobs?per_page=100" \ + | jq -s '[.[] | (.jobs // [])[]]' > "${jobs_json}" 2>/dev/null; then + log_summary "run ${run_id}: failed to list jobs; skipping." + continue + fi + + in_progress_count="$(jq '[ .[] | select(.status == "in_progress") ] | length' < "${jobs_json}")" + queued_count="$(jq '[ .[] | select(.status == "queued") ] | length' < "${jobs_json}")" + + if (( in_progress_count > 0 )); then + # A run with even one in-progress job is by definition not + # dispatcher-stuck. This covers matrix cells or later jobs + # waiting while another job from the same run is active, and the + # general case of a run that has at least one runner. + log_summary "run ${run_id} (${run_path_base}, event=${run_event}): healthy queued (${in_progress_count} in_progress, ${queued_count} queued -- waiting on concurrency/matrix slot)." + printf '* run %s (%s, event=%s) -- %d in_progress, %d queued\n' "${run_id}" "${run_path_base}" "${run_event}" "${in_progress_count}" "${queued_count}" >> "${healthy_runs_file}" + continue + fi + + if (( queued_count == 0 )); then + # No queued jobs at all - the run is in some other transitional + # state, not the dispatcher-stuck pattern. + log_summary "run ${run_id} (${run_path_base}, event=${run_event}): no queued jobs yet (early state); skipping." + continue + fi + + mapfile -t queued_job_labels < <(jq -c ' + .[] + | select(.status == "queued") + | (.labels // []) + ' < "${jobs_json}") + + matched=0 + for labels_csv in "${queued_job_labels[@]}"; do + # labels_csv is a JSON array like ["self-hosted","Windows","RAM-64GB"] + if jq -e --argjson labels "${labels_csv}" ' + map( + . as $r + | ($labels | all(. as $l | $r.labels | index($l) | type == "number")) + ) | any + ' < "${idle_runners_json}" > /dev/null; then + matched=1 + break + fi + done + + if [[ ${matched} -eq 0 ]]; then + log_summary "run ${run_id} (${run_path_base}, event=${run_event}): no matching idle runner -- investigate label config. No action." + continue + fi + + # ------------------------------------------------------------------ + # 5. Genuinely stuck. Cap at MAX_CANCELS_PER_DAY per run-id. + # ------------------------------------------------------------------ + state_file="${STATE_DIR}/${run_id}.json" + cancels=0 + last_cancel=0 + if [[ -f "${state_file}" ]]; then + state_ok=1 + state_content="$(cat "${state_file}" 2>/dev/null)" || state_ok=0 + if [[ ${state_ok} -eq 1 ]]; then + parsed_cancels="$(jq -r '.cancels // .reruns // 0' <<< "${state_content}" 2>/dev/null)" || state_ok=0 + parsed_last="$(jq -r '.last_cancel // .last_rerun // 0' <<< "${state_content}" 2>/dev/null)" || state_ok=0 + fi + if [[ ${state_ok} -eq 1 ]]; then + if is_nonnegative_integer "${parsed_cancels}" && is_nonnegative_integer "${parsed_last}"; then + cancels="${parsed_cancels}" + last_cancel="${parsed_last}" + else + state_ok=0 + fi + fi + if [[ ${state_ok} -ne 1 ]]; then + log_summary "run ${run_id}: state file corrupt; resetting." + else + log_summary "run ${run_id}: loaded cancel state (cancels=${cancels}, last_cancel=${last_cancel})." + fi + fi + # Reset counter if last action was >24h ago. + if (( now_epoch - last_cancel > 86400 )); then + cancels=0 + fi + + if (( cancels >= MAX_CANCELS_PER_DAY )); then + log_summary "run ${run_id} (${run_path_base}, event=${run_event}): cancel cap (${MAX_CANCELS_PER_DAY}/24h) reached; skipping. Manual intervention required." + printf '* run %s (%s, event=%s) -- cap reached, operator action: %s\n' "${run_id}" "${run_path_base}" "${run_event}" "${run_html_url}" >> "${stuck_runs_file}" + continue + fi + + log_summary "run ${run_id} (${run_path_base}, event=${run_event}): dispatcher-stuck; cancelling (attempt $((cancels + 1))/${MAX_CANCELS_PER_DAY})." + if ! gh run cancel "${run_id}" --repo "${REPO}" 2>&1 | tee -a "${summary_file}"; then + log_summary "run ${run_id}: 'gh run cancel' failed; will try again next cycle." + continue + fi + + cancels=$((cancels + 1)) + printf '{"cancels": %d, "last_cancel": %d}\n' "${cancels}" "${now_epoch}" > "${state_file}" + git add "${state_file}" + state_dirty=1 + persist_state_changes "record cancel for run ${run_id}" || true + + # Decide redispatch path based on event + workflow_dispatch trigger. + workflow_def="$(mktemp)" + workflow_supports_dispatch=0 + if gh api "repos/${REPO}/actions/workflows/${run_workflow_id}" > "${workflow_def}" 2>/dev/null; then + workflow_path="$(jq -r '.path // ""' < "${workflow_def}")" + if [[ -n "${workflow_path}" ]]; then + # Look at the contents of the workflow file to see if it + # declares `workflow_dispatch:` (the trigger we need to use + # the dispatches REST API). + workflow_file_raw="$(mktemp)" + if gh api "repos/${REPO}/contents/${workflow_path}" --jq '.content' 2>/dev/null \ + | base64 -d > "${workflow_file_raw}" 2>/dev/null; then + if grep -Eq '^[[:space:]]*workflow_dispatch:' "${workflow_file_raw}"; then + workflow_supports_dispatch=1 + fi + fi + fi + fi + + case "${run_event}" in + push|schedule|workflow_dispatch) + if [[ -n "${run_head_branch}" && ${workflow_supports_dispatch} -eq 1 ]]; then + log_summary "run ${run_id}: re-dispatching workflow ${run_workflow_id} on ref '${run_head_branch}'." + if gh api -X POST "repos/${REPO}/actions/workflows/${run_workflow_id}/dispatches" \ + -f "ref=${run_head_branch}" 2>&1 | tee -a "${summary_file}"; then + printf '* run %s (%s, event=%s) -- cancelled and re-dispatched on %s\n' "${run_id}" "${run_path_base}" "${run_event}" "${run_head_branch}" >> "${stuck_runs_file}" + else + log_summary "run ${run_id}: re-dispatch failed; operator action: ${run_html_url}" + printf '* run %s (%s, event=%s) -- cancelled; re-dispatch FAILED, operator action: %s\n' "${run_id}" "${run_path_base}" "${run_event}" "${run_html_url}" >> "${stuck_runs_file}" + fi + else + log_summary "run ${run_id}: workflow does not support workflow_dispatch on ref '${run_head_branch}'; operator action: click 'Re-run all jobs' at ${run_html_url}" + printf \ + '* run %s (%s, event=%s) -- cancelled; operator action: click "Re-run all jobs" at %s\n' \ + "${run_id}" "${run_path_base}" "${run_event}" "${run_html_url}" >> "${stuck_runs_file}" + fi + ;; + pull_request|pull_request_target) + # No safe API path: the dispatches endpoint cannot re-trigger + # a pull_request run, and pushing a no-op commit to the head + # ref would tamper with the PR. Operator-visible cancel + + # explicit step-summary instruction is the supported path. + log_summary "run ${run_id}: pull_request-triggered; operator action: click 'Re-run all jobs' at ${run_html_url}" + printf '* run %s (%s, event=%s) -- cancelled; operator action: click "Re-run all jobs" at %s\n' "${run_id}" "${run_path_base}" "${run_event}" "${run_html_url}" >> "${stuck_runs_file}" + ;; + *) + log_summary "run ${run_id}: event '${run_event}' has no automatic recovery path; operator action: click 'Re-run all jobs' at ${run_html_url}" + printf '* run %s (%s, event=%s) -- cancelled; operator action: click "Re-run all jobs" at %s\n' "${run_id}" "${run_path_base}" "${run_event}" "${run_html_url}" >> "${stuck_runs_file}" + ;; + esac + done + + # ------------------------------------------------------------------ + # 6. Commit + push state changes with a single rebase+retry. + # ------------------------------------------------------------------ + persist_state_changes "final state sync" || true + + popd > /dev/null + + # ------------------------------------------------------------------ + # Final categorized step-summary table. + # ------------------------------------------------------------------ + { + echo "## Watchdog summary" + cat "${summary_file}" + echo "" + echo "### Healthy queued (waiting on concurrency / matrix slot)" + if [[ -s "${healthy_runs_file}" ]]; then + cat "${healthy_runs_file}" + else + echo "_(none)_" + fi + echo "" + echo "### Stuck (auto-cancelled)" + if [[ -s "${stuck_runs_file}" ]]; then + cat "${stuck_runs_file}" + else + echo "_(none)_" + fi + echo "" + echo "### Stuck but excluded (operator action needed)" + if [[ -s "${excluded_runs_file}" ]]; then + cat "${excluded_runs_file}" + else + echo "_(none)_" + fi + } >> "${GITHUB_STEP_SUMMARY}" diff --git a/.github/workflows/test-lint.yml b/.github/workflows/test-lint.yml index c3fa068c8..5b803cb25 100644 --- a/.github/workflows/test-lint.yml +++ b/.github/workflows/test-lint.yml @@ -10,6 +10,8 @@ on: - "scripts/lint-tests.ps1" - "scripts/tests/test-comment-stripping.ps1" - "scripts/tests/test-lint-tests.ps1" + - "scripts/unity/report-slow-tests.ps1" + - "scripts/tests/test-report-slow-tests.ps1" - ".github/workflows/test-lint.yml" pull_request: paths: @@ -18,6 +20,8 @@ on: - "scripts/lint-tests.ps1" - "scripts/tests/test-comment-stripping.ps1" - "scripts/tests/test-lint-tests.ps1" + - "scripts/unity/report-slow-tests.ps1" + - "scripts/tests/test-report-slow-tests.ps1" - ".github/workflows/test-lint.yml" permissions: @@ -38,6 +42,10 @@ jobs: shell: pwsh run: ./scripts/tests/test-lint-tests.ps1 -VerboseOutput + - name: Test slow-test reporter + shell: pwsh + run: ./scripts/tests/test-report-slow-tests.ps1 -VerboseOutput + - name: Lint tests shell: pwsh run: ./scripts/lint-tests.ps1 -VerboseOutput diff --git a/.github/workflows/unity-benchmarks.yml b/.github/workflows/unity-benchmarks.yml new file mode 100644 index 000000000..a7ca1534d --- /dev/null +++ b/.github/workflows/unity-benchmarks.yml @@ -0,0 +1,535 @@ +name: Unity Benchmarks + +# cspell:ignore lfs WSL + +on: + schedule: + - cron: "29 10 * * 3" + workflow_dispatch: + inputs: + unity-version: + description: "Pin a single Unity version. Empty = full matrix." + required: false + default: "" + type: string + +# Top-level permissions are least-privilege and mirror unity-tests.yml. The +# licensed matrix jobs (matrix-config, runner-preflight, benchmarks) only need +# read access plus checks:write for NUnit annotations and issues:write for +# benchmark-failure notifications; they must NOT inherit write scope. The +# dedicated commit-perf-results job below opts into contents:write at the JOB +# level so it (and only it) can push the refreshed perf artifacts. +permissions: + contents: read + checks: write + issues: write + +jobs: + matrix-config: + name: Resolve benchmark matrix + runs-on: ubuntu-latest + timeout-minutes: 2 + outputs: + unity-versions: ${{ steps.resolve.outputs.unity-versions }} + latest-version: ${{ steps.resolve.outputs.latest-version }} + has-required-secrets: ${{ steps.check-secrets.outputs.has-secrets }} + steps: + - name: Checkout + uses: actions/checkout@v6 + with: + persist-credentials: false + - name: Resolve dispatch overrides + id: resolve + env: + INPUT_UNITY_VERSION: ${{ inputs.unity-version }} + # The benchmark matrix list and the latest entry both come from the + # canonical source (.github/unity-versions.json), so the scheduled perf + # coverage tracks every supported runtime and the latest-vs-list pair + # cannot drift. The LAST entry is the latest Unity version. A dispatch + # override pins a single version. + run: | + set -euo pipefail + if [ -n "${INPUT_UNITY_VERSION:-}" ]; then + versions="[\"${INPUT_UNITY_VERSION}\"]" + latest="${INPUT_UNITY_VERSION}" + else + versions="$(jq -c '.all' .github/unity-versions.json)" + latest="$(jq -r '.all[-1]' .github/unity-versions.json)" + fi + { + echo "unity-versions=${versions}" + echo "latest-version=${latest}" + } >> "${GITHUB_OUTPUT}" + + - name: Check for required Unity license secrets + id: check-secrets + env: + UNITY_SERIAL: ${{ secrets.UNITY_SERIAL }} + UNITY_EMAIL: ${{ secrets.UNITY_EMAIL }} + UNITY_PASSWORD: ${{ secrets.UNITY_PASSWORD }} + ORG_BUILD_LOCK_TOKEN: ${{ secrets.ORG_BUILD_LOCK_TOKEN }} + run: | + set -euo pipefail + if [ -n "${UNITY_SERIAL:-}" ] && \ + [ -n "${UNITY_EMAIL:-}" ] && \ + [ -n "${UNITY_PASSWORD:-}" ] && \ + [ -n "${ORG_BUILD_LOCK_TOKEN:-}" ]; then + echo "has-secrets=true" >> "${GITHUB_OUTPUT}" + else + echo "has-secrets=false" >> "${GITHUB_OUTPUT}" + fi + + runner-preflight: + name: Self-hosted runner access preflight + # See docs/runbooks/unity-runners-after-transfer.md for the design + # contract. CRITICAL: this preflight must NEVER make Unity CI more + # broken than the no-preflight baseline -- if both runner-inventory + # endpoints return 403/404, soft-pass with a ::warning::. + runs-on: ubuntu-latest + timeout-minutes: 3 + permissions: + actions: read + # NOTE: `administration: read` is NOT a valid permissions key for + # workflow GITHUB_TOKEN (per actionlint and the GitHub docs schema). + # Listing self-hosted runners requires admin:org (org-scope) OR a + # fine-grained PAT with repo "Administration: read" -- neither of + # which the default GITHUB_TOKEN can carry. The soft-pass path in + # the run script below is the supported coverage when the token + # is unscoped. See docs/runbooks/unity-runners-after-transfer.md. + concurrency: + group: ${{ github.workflow }}-runner-preflight-${{ github.ref }} + cancel-in-progress: true + steps: + - name: Probe self-hosted runner availability + env: + GH_TOKEN: ${{ secrets.RUNNER_AUDIT_PAT || secrets.GITHUB_TOKEN }} + REQUIRED_LABELS: "self-hosted,Windows,RAM-64GB" + # gh api --paginate | jq -s pattern mirrors stuck-job-watchdog.yml. + run: | + set -euo pipefail + echo "::group::Runner-access preflight" + echo "Required labels: ${REQUIRED_LABELS}" + IFS=',' read -r -a required <<< "${REQUIRED_LABELS}" + runners_json="$(mktemp)" + runners_scope="" + if gh api --paginate \ + "orgs/${GITHUB_REPOSITORY_OWNER}/actions/runners?per_page=100" \ + 2>/dev/null \ + | jq -s '[.[] | (.runners // [])[]]' > "${runners_json}" 2>/dev/null; then + runners_scope="org" + else + if gh api --paginate \ + "repos/${GITHUB_REPOSITORY}/actions/runners?per_page=100" \ + 2>/dev/null \ + | jq -s '[.[] | (.runners // [])[]]' > "${runners_json}" 2>/dev/null; then + runners_scope="repo" + fi + fi + if [ -z "${runners_scope}" ]; then + org_url="orgs/${GITHUB_REPOSITORY_OWNER}/actions/runners" + repo_url="repos/${GITHUB_REPOSITORY}/actions/runners" + { + echo "::warning::Runner inventory unavailable from both ${org_url}" + echo "::warning::and ${repo_url} (likely 403: GITHUB_TOKEN lacks admin:org" + echo "::warning::or administration:read)." + echo "::warning::See docs/runbooks/unity-runners-after-transfer.md" + echo "::warning::for how to plumb a RUNNER_AUDIT_PAT secret to upgrade" + echo "::warning::this soft-pass to a hard-pass." + } + echo "Soft pass: skipping runner inventory check." + echo "::endgroup::" + exit 0 + fi + echo "Runner inventory scope: ${runners_scope}" + total="$(jq 'length' < "${runners_json}")" + echo "Visible runners (${runners_scope} scope): ${total}" + matched="$(jq --argjson labels "$(printf '%s\n' "${required[@]}" | jq -R . | jq -s .)" ' + [ .[] + | select(.status == "online") + | select(($labels | all(. as $l | (.labels // []) | map(.name) | index($l) | type == "number"))) + ] | length + ' < "${runners_json}")" + echo "Runners satisfying ${REQUIRED_LABELS}: ${matched}" + if [ "${matched}" -lt 1 ]; then + { + echo "::error::No online self-hosted runner satisfies the required labels (${REQUIRED_LABELS})." + echo "The Unity matrix would queue forever." + echo "See docs/runbooks/unity-runners-after-transfer.md for the post-transfer runner-group ACL fix." + } + jq '[ .[] | {name, status, busy, labels: [(.labels // [])[].name]} ]' \ + < "${runners_json}" + exit 1 + fi + echo "::endgroup::" + + benchmarks: + name: Benchmarks ${{ matrix.unity-version }} ${{ matrix.test-mode }} + needs: + - matrix-config + - runner-preflight + # Skip the self-hosted matrix entirely when license/lock secrets are absent, + # so a secret-less repo does not queue forever on a nonexistent runner. + if: ${{ needs.matrix-config.outputs.has-required-secrets == 'true' }} + runs-on: [self-hosted, Windows, RAM-64GB] + timeout-minutes: 660 + strategy: + fail-fast: false + max-parallel: 1 + matrix: + unity-version: ${{ fromJSON(needs.matrix-config.outputs.unity-versions) }} + test-mode: + - editmode + - playmode + steps: + - name: Enable Git long paths + shell: pwsh + env: + GIT_CONFIG_GLOBAL: ${{ runner.temp }}/uh-gitconfig + run: git config --global core.longpaths true + + - name: Checkout + uses: actions/checkout@v6 + env: + GIT_CONFIG_GLOBAL: ${{ runner.temp }}/uh-gitconfig + with: + lfs: true + persist-credentials: false + + - name: Print runner diagnostics + # Never use plain `shell: bash` on self-hosted Windows runners (the WSL + # stub at C:\Windows\System32\bash.exe can win PATH resolution and fail). + uses: ./.github/actions/print-self-hosted-runner-diagnostics + with: + matrix-note: "default (organization lock wraps the Unity section)" + + - name: Cache Unity Library and package caches + uses: actions/cache@v5 + env: + PACKAGE_HASH: >- + ${{ hashFiles( + 'package.json', + 'Runtime/**', + 'Editor/**', + 'Tests/**', + 'scripts/unity/run-ci-tests.ps1', + 'scripts/unity/lib/asmdef-discovery.js', + '.github/actions/compute-unity-assemblies/action.yml' + ) }} + with: + path: | + .artifacts/unity/projects/${{ matrix.unity-version }}-${{ matrix.test-mode }}/Library + .artifacts/unity/cache/${{ matrix.unity-version }} + key: Library-bench-${{ runner.os }}-${{ runner.arch }}-${{ matrix.unity-version }}-${{ matrix.test-mode }}-${{ env.PACKAGE_HASH }} + + - name: Setup Node.js + uses: actions/setup-node@v6 + with: + node-version: "22.18.0" + + # Single source of truth for the assembly include list: the same + # asmdef-discovery.js module. For benchmarks we OPT IN to the perf + # assemblies (*.Tests.Runtime.Performance) via include-perf. + - name: Compute benchmark assembly list + id: compute + uses: ./.github/actions/compute-unity-assemblies + with: + include-perf: "true" + target: "${{ matrix.test-mode }}" + + - name: Validate Unity license secrets + uses: ./.github/actions/validate-unity-license + env: + UNITY_SERIAL: ${{ secrets.UNITY_SERIAL }} + UNITY_EMAIL: ${{ secrets.UNITY_EMAIL }} + UNITY_PASSWORD: ${{ secrets.UNITY_PASSWORD }} + + # Skip provisioning, the org lock, and the run when the compute step + # resolved an empty assembly list. Mirrors unity-tests.yml. The benchmark + # list is structurally non-empty, so this is defense-in-depth. + - name: Provision Unity Editor + if: ${{ steps.compute.outputs.is-empty != 'true' }} + timeout-minutes: 180 + shell: pwsh + run: | + $artifactsPath = '.artifacts/unity/benchmarks/${{ matrix.unity-version }}-${{ matrix.test-mode }}' + $diagnosticsPath = Join-Path $artifactsPath 'provisioning' + $diagnosticsFile = Join-Path $diagnosticsPath 'ensure-editor-summary.json' + New-Item -ItemType Directory -Force -Path $diagnosticsPath | Out-Null + $editor = ./scripts/unity/ensure-editor.ps1 ` + -UnityVersion '${{ matrix.unity-version }}' ` + -CiManagedOnly ` + -RequireHealthyExisting ` + -ProvisioningProfile EditorOnly ` + -DiagnosticsPath $diagnosticsFile + "UNITY_EDITOR_PATH=$editor" | Out-File -FilePath $env:GITHUB_ENV -Append + + - name: Upload Unity provisioning diagnostics + if: always() + uses: actions/upload-artifact@v7 + with: + name: unity-benchmark-provisioning-${{ matrix.unity-version }}-${{ matrix.test-mode }} + path: .artifacts/unity/benchmarks/${{ matrix.unity-version }}-${{ matrix.test-mode }}/provisioning + if-no-files-found: warn + retention-days: 90 + + - name: Acquire organization Unity lock + if: ${{ steps.compute.outputs.is-empty != 'true' }} + uses: Ambiguous-Interactive/ambiguous-organization-build-lock/.github/actions/acquire-build-lock@v1 + with: + lock-name: wallstop-organization-builds + holder-id-suffix: ${{ matrix.unity-version }}-${{ matrix.test-mode }} + timeout-minutes: "300" + env: + BUILD_LOCK_TOKEN: ${{ secrets.ORG_BUILD_LOCK_TOKEN }} + + - name: Run Unity Test Runner + id: run_tests + if: ${{ steps.compute.outputs.is-empty != 'true' }} + timeout-minutes: 120 + shell: pwsh + env: + UNITY_SERIAL: ${{ secrets.UNITY_SERIAL }} + UNITY_EMAIL: ${{ secrets.UNITY_EMAIL }} + UNITY_PASSWORD: ${{ secrets.UNITY_PASSWORD }} + UNITY_ACCELERATOR_ENDPOINT: ${{ secrets.UNITY_ACCELERATOR_ENDPOINT }} + # Run ONLY the perf/stress categories in this dedicated perf job (the + # inverse of unity-tests.yml, which excludes them). + UH_UNITY_TEST_CATEGORY: "Performance;Stress" + UH_PERF_COMMIT: ${{ github.sha }} + run: | + # Release (not Debug) code optimization for the editmode/playmode perf + # legs, mirroring the repo-wide Unity Release-mode contract so the + # weekly coverage run measures the same optimized hot path the docs do. + ./scripts/unity/run-ci-tests.ps1 ` + -UnityVersion '${{ matrix.unity-version }}' ` + -TestMode '${{ matrix.test-mode }}' ` + -AssemblyNames $env:UH_TEST_ASSEMBLIES ` + -ArtifactsPath '.artifacts/unity/benchmarks/${{ matrix.unity-version }}-${{ matrix.test-mode }}' ` + -ReleaseCodeOptimization ` + -ReleasePlayerBuild + + # The Random distribution tests are [Category("Fast")] and run in the main + # PR suite at the REDUCED default sample count (Tests/Runtime/Random/ + # RandomTestBase.cs). This dedicated leg re-runs the Random assembly at the + # FULL statistical sample count so the thorough bias-detection coverage the + # fast default trades away is recovered weekly in CI. editmode-only (the + # Random tests are pure C# EditMode); runs inside the org-lock section. + # This is the ONLY place UH_RANDOM_SAMPLE_COUNT is consumed. + - name: Run Random suite at full sample count + id: run_random_thorough + if: ${{ matrix.test-mode == 'editmode' && steps.compute.outputs.is-empty != 'true' }} + timeout-minutes: 120 + shell: pwsh + env: + UNITY_SERIAL: ${{ secrets.UNITY_SERIAL }} + UNITY_EMAIL: ${{ secrets.UNITY_EMAIL }} + UNITY_PASSWORD: ${{ secrets.UNITY_PASSWORD }} + UNITY_ACCELERATOR_ENDPOINT: ${{ secrets.UNITY_ACCELERATOR_ENDPOINT }} + # No perf-category filter: run the Fast-tagged Random tests by assembly. + UH_UNITY_TEST_CATEGORY: "" + UH_RANDOM_SAMPLE_COUNT: "12750000" + run: | + ./scripts/unity/run-ci-tests.ps1 ` + -UnityVersion '${{ matrix.unity-version }}' ` + -TestMode 'editmode' ` + -AssemblyNames 'WallstopStudios.UnityHelpers.Tests.Runtime.Random' ` + -ArtifactsPath '.artifacts/unity/benchmarks-random/${{ matrix.unity-version }}' + + - name: Return Unity license + if: always() + uses: ./.github/actions/return-unity-license + env: + UNITY_EMAIL: ${{ secrets.UNITY_EMAIL }} + UNITY_PASSWORD: ${{ secrets.UNITY_PASSWORD }} + + - name: Release organization Unity lock + if: always() + uses: Ambiguous-Interactive/ambiguous-organization-build-lock/.github/actions/release-build-lock@v1 + with: + lock-name: wallstop-organization-builds + holder-id-suffix: ${{ matrix.unity-version }}-${{ matrix.test-mode }} + env: + BUILD_LOCK_TOKEN: ${{ secrets.ORG_BUILD_LOCK_TOKEN }} + + - name: Dump Unity log tail on failure or cancellation + if: ${{ failure() || cancelled() }} + uses: ./.github/actions/dump-unity-log-tail + with: + results-dir: .artifacts/unity/benchmarks/${{ matrix.unity-version }}-${{ matrix.test-mode }} + label: Benchmarks ${{ matrix.unity-version }} ${{ matrix.test-mode }} + + - name: Verify tests actually ran + if: >- + ${{ + !cancelled() && + steps.compute.outcome == 'success' && + (steps.compute.outputs.is-empty == 'true' || steps.run_tests.outcome != 'skipped') + }} + uses: ./.github/actions/verify-unity-results + with: + results-dir: .artifacts/unity/benchmarks/${{ matrix.unity-version }}-${{ matrix.test-mode }} + label: Benchmarks ${{ matrix.unity-version }} ${{ matrix.test-mode }} + expected-empty: ${{ steps.compute.outputs.is-empty }} + + # Stage the raw NUnit results for this leg into a deterministic path so + # the downstream commit job can collect every leg's results via + # download-artifact and commit them under perf-results/. We commit + # the raw results.xml (the source of truth) rather than a hand-formatted + # doc; unity-helpers has no NUnit->markdown perf-doc generator yet (the + # human-authored docs under docs/performance/*.md stay hand-maintained). + # TODO(unity-helpers): if a perf-doc generator is added, render the + # polished table here instead of copying raw XML. + - name: Stage perf results for commit + if: ${{ !cancelled() && steps.run_tests.outcome == 'success' }} + shell: pwsh + run: | + $src = '.artifacts/unity/benchmarks/${{ matrix.unity-version }}-${{ matrix.test-mode }}/results.xml' + $stageDir = 'perf-staging' + New-Item -ItemType Directory -Force -Path $stageDir | Out-Null + if (Test-Path -LiteralPath $src -PathType Leaf) { + Copy-Item -LiteralPath $src -Destination (Join-Path $stageDir 'results-${{ matrix.unity-version }}-${{ matrix.test-mode }}.xml') -Force + Write-Host "::notice::Staged perf results for ${{ matrix.unity-version }} ${{ matrix.test-mode }}." + } else { + Write-Host "::warning::No results.xml to stage for ${{ matrix.unity-version }} ${{ matrix.test-mode }}." + } + + - name: Upload staged perf results + if: ${{ !cancelled() && steps.run_tests.outcome == 'success' }} + uses: actions/upload-artifact@v7 + with: + name: perf-results-${{ matrix.unity-version }}-${{ matrix.test-mode }} + path: perf-staging + if-no-files-found: warn + retention-days: 90 + + - name: Upload benchmark artifacts + if: always() + uses: actions/upload-artifact@v7 + with: + name: unity-benchmarks-${{ matrix.unity-version }}-${{ matrix.test-mode }} + path: .artifacts/unity/benchmarks/${{ matrix.unity-version }}-${{ matrix.test-mode }} + if-no-files-found: warn + retention-days: 90 + + commit-perf-results: + name: Commit refreshed perf results + needs: benchmarks + # schedule + workflow_dispatch only ever run on the canonical repo (scheduled + # runs fire only from the default branch; a dispatch requires write access), + # so no repo-slug guard is needed (avoids hardcoding a slug that breaks after + # an org transfer). Run even if some benchmark legs failed (!cancelled, not + # success()) so a partial result set is still captured; if there are no + # artifacts at all, git-auto-commit-action is a no-op. + if: ${{ !cancelled() }} + runs-on: ubuntu-latest + timeout-minutes: 10 + permissions: + contents: write + steps: + - name: Checkout + uses: actions/checkout@v6 + with: + # Persist credentials so git-auto-commit-action can push. + persist-credentials: true + + - name: Download staged perf results + uses: actions/download-artifact@v8 + with: + pattern: perf-results-* + path: perf-download + merge-multiple: true + + - name: Assemble perf-results + shell: bash + run: | + set -euo pipefail + # Commit raw NUnit results to a TOP-LEVEL perf-results/ dir (NOT under + # docs/ or any other Unity-asset source root), so they need no Unity + # .meta companions and fall outside the prettier/markdown/cspell scan + # globs (none scan .xml). Committing under docs/** would break + # meta-file-lint (every tracked docs file/dir requires a .meta) on the + # next push; perf-results/ sidesteps that entirely. + dest="perf-results" + mkdir -p "${dest}" + if [ -d perf-download ] && [ -n "$(ls -A perf-download 2>/dev/null || true)" ]; then + find perf-download -type f -name '*.xml' -exec cp -f {} "${dest}/" \; + echo "Assembled $(find "${dest}" -name '*.xml' | wc -l) result file(s) under ${dest}." + else + echo "::notice::No staged perf results were downloaded; nothing to commit." + fi + + # Node 22 (matches the benchmarks job's setup-node) to run the pure-Node, + # zero-dependency perf-delta tooling under scripts/unity/lib/. + - name: Setup Node.js + uses: actions/setup-node@v6 + with: + node-version: "22.18.0" + + # Extract normalized perf metrics from the assembled NUnit results, diff + # them against the committed rolling baseline (perf-results/baseline.json), + # write the human-readable report (perf-results/perf-deltas.md) plus a + # machine-readable payload (perf-results/perf-deltas.json), and roll the + # baseline forward to the latest numbers. These are committed alongside the + # raw XML. REPORT-ONLY by default: the weekly scheduled run must never fail + # just because perf moved, so we do NOT pass --fail-on-regression here; the + # `regressed=` signal is surfaced as a step output + workflow annotation for + # visibility. To make a regression hard-fail later, set + # PERF_FAIL_ON_REGRESSION=1 on the render step (and gate on its outcome). + - name: Compute perf deltas vs baseline + id: perf_deltas + shell: bash + run: | + set -euo pipefail + shopt -s nullglob + results=(perf-results/results-*.xml) + if [ ${#results[@]} -eq 0 ]; then + echo "::notice::No perf results XML to analyze; leaving baseline untouched." + echo "regressed=false" >> "${GITHUB_OUTPUT}" + exit 0 + fi + node scripts/unity/lib/extract-perf-metrics.js "${results[@]}" \ + --output perf-results/perf-current.json + metric_count="$(node -e 'const m=require("./perf-results/perf-current.json");process.stdout.write(String(m.length))')" + echo "Extracted ${metric_count} perf metric(s) from ${#results[@]} result file(s)." + # render-perf-deltas.js always exits 0 in report-only mode and prints + # changed=/regressed= on stdout; capture regressed for the annotation. + render_output="$(node scripts/unity/lib/render-perf-deltas.js \ + --current perf-results/perf-current.json \ + --baseline perf-results/baseline.json \ + --out-md perf-results/perf-deltas.md \ + --out-json perf-results/perf-deltas.json \ + --update-baseline perf-results/baseline.json)" + echo "${render_output}" + regressed="$(printf '%s\n' "${render_output}" | sed -n 's/^regressed=//p' | tail -n1)" + regressed="${regressed:-false}" + echo "regressed=${regressed}" >> "${GITHUB_OUTPUT}" + if [ "${regressed}" = "true" ]; then + echo "::warning::Perf delta computed a significant regression; see perf-results/perf-deltas.md." + fi + # perf-current.json is a transient intermediate; do not commit it. + rm -f perf-results/perf-current.json + + # git-auto-commit-action is a no-op when there is no diff, so a run that + # produced no new results does not create an empty commit. + # + # TODO(unity-helpers): if the default branch is protected such that the + # built-in GITHUB_TOKEN cannot push (branch protection / required reviews), + # this direct commit will be REJECTED. DxMessaging's perf-numbers.yml hit + # exactly this and switched to a GitHub App installation token + # (actions/create-github-app-token@v3) that opens an auto-merge PR instead. + # If that happens here, provision an App token and mirror that rationale; + # the default below is the simplest path that works on an UNprotected + # default branch. + - name: Commit perf results + uses: stefanzweifel/git-auto-commit-action@v7.1.0 + with: + commit_message: "chore(perf): refresh CI perf results [skip ci]" + # Broadened from perf-results/*.xml to the whole tree so the rolling + # baseline (baseline.json) and the generated delta report + # (perf-deltas.md / perf-deltas.json) are committed next to the raw XML. + # perf-results/** is in .prettierignore and cspell's ignorePaths, so + # these generated files never trip the prettier/cspell CI checks. + file_pattern: perf-results/** + commit_user_name: "github-actions[bot]" + commit_user_email: "41898282+github-actions[bot]@users.noreply.github.com" + commit_author: "github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>" diff --git a/.github/workflows/unity-tests.yml b/.github/workflows/unity-tests.yml new file mode 100644 index 000000000..1c68f5f8e --- /dev/null +++ b/.github/workflows/unity-tests.yml @@ -0,0 +1,697 @@ +name: Unity Tests + +# cspell:ignore lfs WSL + +on: + pull_request: + branches: + - master + - main + push: + branches: + - master + - main + schedule: + - cron: "17 8 * * 1" + workflow_dispatch: + inputs: + unity-version: + description: "Pin a single Unity version. Empty = full matrix." + required: false + default: "" + type: string + test-mode: + description: "Pin a single test mode." + required: false + default: "all" + type: choice + options: + - all + - editmode + - playmode + - standalone + +permissions: + contents: read + checks: write + +jobs: + matrix-config: + name: Resolve Unity test matrix + runs-on: ubuntu-latest + timeout-minutes: 2 + outputs: + unity-versions: ${{ steps.resolve.outputs.unity-versions }} + test-modes: ${{ steps.resolve.outputs.test-modes }} + has-required-secrets: ${{ steps.check-secrets.outputs.has-secrets }} + steps: + - name: Checkout + uses: actions/checkout@v6 + with: + persist-credentials: false + + # Fail FAST on a bad test-project module manifest before any (expensive, + # self-hosted) Unity leg starts. A non-existent UPM id here -- e.g. the + # bogus 'com.unity.modules.grid' (declare com.unity.modules.tilemap for + # Grid) -- makes every matrix leg fail ~30 min later with no NUnit + # results.xml and no clearly-named cause. This + # hosted-runner check turns that whole class into an instant, self- + # describing red on the PR. run-ci-tests.ps1's catastrophic-pattern scan is + # the in-Unity backstop for the same failure. + - name: Validate Unity test-project module manifest + shell: pwsh + run: ./scripts/lint-unity-test-modules.ps1 -VerboseOutput + + # NOTE: DxMessaging had a "ci-owned docs-only PR" detection here that + # skipped the licensed matrix when a PR touched ONLY its perf-numbers doc + # + baseline. unity-helpers has no such CI-owned perf-doc auto-commit on + # the PR path (the unity-benchmarks workflow commits perf results from a + # SCHEDULED run, not a PR), so that detection is intentionally DROPPED for + # simplicity. If a perf-doc auto-update PR flow is added later, repoint a + # docs-only gate at docs/performance/ rather than DxMessaging's + # docs/architecture/performance.md path. + - name: Resolve dispatch overrides + id: resolve + env: + INPUT_UNITY_VERSION: ${{ inputs.unity-version }} + INPUT_TEST_MODE: ${{ inputs.test-mode }} + run: | + set -euo pipefail + if [ -n "${INPUT_UNITY_VERSION:-}" ]; then + versions="[\"${INPUT_UNITY_VERSION}\"]" + else + versions="$(jq -c '.all' .github/unity-versions.json)" + fi + if [ -n "${INPUT_TEST_MODE:-}" ] && [ "${INPUT_TEST_MODE}" != "all" ]; then + modes="[\"${INPUT_TEST_MODE}\"]" + else + modes='["editmode","playmode","standalone"]' + fi + { + echo "unity-versions=${versions}" + echo "test-modes=${modes}" + } >> "${GITHUB_OUTPUT}" + + - name: Check for required Unity license secrets + id: check-secrets + env: + UNITY_SERIAL: ${{ secrets.UNITY_SERIAL }} + UNITY_EMAIL: ${{ secrets.UNITY_EMAIL }} + UNITY_PASSWORD: ${{ secrets.UNITY_PASSWORD }} + ORG_BUILD_LOCK_TOKEN: ${{ secrets.ORG_BUILD_LOCK_TOKEN }} + run: | + set -euo pipefail + if [ -n "${UNITY_SERIAL:-}" ] && \ + [ -n "${UNITY_EMAIL:-}" ] && \ + [ -n "${UNITY_PASSWORD:-}" ] && \ + [ -n "${ORG_BUILD_LOCK_TOKEN:-}" ]; then + echo "has-secrets=true" >> "${GITHUB_OUTPUT}" + else + echo "has-secrets=false" >> "${GITHUB_OUTPUT}" + fi + + runner-preflight: + name: Self-hosted runner access preflight + # Runs on ubuntu-latest BEFORE any self-hosted matrix entry tries to + # queue. This converts the "stuck forever" failure mode that motivated + # docs/runbooks/unity-runners-after-transfer.md into a fast, + # clearly-explained failure. The runbook explains the post-transfer + # runner-group ACL pitfall and the resolution paths. + # + # CRITICAL CONTRACT: this preflight must NEVER make Unity CI more + # broken than the no-preflight baseline. If the token cannot list + # either the org-level OR repo-level runner inventory (403/404 from + # both), the preflight emits a ::warning:: and exits 0 (soft pass). + # Only when we can prove the inventory is wrong do we fail hard. + if: >- + ${{ + (github.event_name != 'pull_request' || + github.event.pull_request.head.repo.full_name == github.repository) && + (github.event_name != 'push' || github.ref_protected) + }} + runs-on: ubuntu-latest + timeout-minutes: 3 + permissions: + actions: read + # NOTE: `administration: read` is NOT a valid permissions key for + # workflow GITHUB_TOKEN (per actionlint and the GitHub docs schema). + # Listing self-hosted runners requires admin:org (org-scope) OR a + # fine-grained PAT with repo "Administration: read" -- neither of + # which the default GITHUB_TOKEN can carry. The soft-pass path in + # the run script below is the supported coverage when the token + # is unscoped. See docs/runbooks/unity-runners-after-transfer.md. + concurrency: + group: ${{ github.workflow }}-runner-preflight-${{ github.ref }} + cancel-in-progress: true + steps: + - name: Probe self-hosted runner availability + env: + GH_TOKEN: ${{ secrets.RUNNER_AUDIT_PAT || secrets.GITHUB_TOKEN }} + REQUIRED_LABELS: "self-hosted,Windows,RAM-64GB" + # Repeated literal of the labels list. Keep in sync with unity-tests + # `runs-on:` declaration below; this preflight must run before any + # self-hosted matrix in this workflow. + # gh api --paginate | jq -s pattern mirrors stuck-job-watchdog.yml; + # see that file's comments for why `jq -s` is required. + run: | + set -euo pipefail + echo "::group::Runner-access preflight" + echo "Required labels: ${REQUIRED_LABELS}" + IFS=',' read -r -a required <<< "${REQUIRED_LABELS}" + runners_json="$(mktemp)" + runners_scope="" + # 1) Try the ORG-scoped endpoint first. Self-hosted runners live + # at org scope in this repo (group "Default" in Ambiguous-Interactive). + # The default GITHUB_TOKEN often lacks admin:org and will 403 here. + if gh api --paginate \ + "orgs/${GITHUB_REPOSITORY_OWNER}/actions/runners?per_page=100" \ + 2>/dev/null \ + | jq -s '[.[] | (.runners // [])[]]' > "${runners_json}" 2>/dev/null; then + runners_scope="org" + else + # 2) Fall back to the REPO-scoped endpoint. Requires + # administration:read; less common to be granted but possible. + if gh api --paginate \ + "repos/${GITHUB_REPOSITORY}/actions/runners?per_page=100" \ + 2>/dev/null \ + | jq -s '[.[] | (.runners // [])[]]' > "${runners_json}" 2>/dev/null; then + runners_scope="repo" + fi + fi + if [ -z "${runners_scope}" ]; then + # 3) Soft pass: both endpoints failed (almost certainly the + # default GITHUB_TOKEN lacks scope). DO NOT fail the build -- + # that would make Unity CI strictly more broken than baseline. + # The watchdog + manual unstick workflows still cover the + # actually-stuck case. + org_url="orgs/${GITHUB_REPOSITORY_OWNER}/actions/runners" + repo_url="repos/${GITHUB_REPOSITORY}/actions/runners" + { + echo "::warning::Runner inventory unavailable from both ${org_url}" + echo "::warning::and ${repo_url} (likely 403: GITHUB_TOKEN lacks admin:org" + echo "::warning::or administration:read)." + echo "::warning::See docs/runbooks/unity-runners-after-transfer.md" + echo "::warning::for how to plumb a RUNNER_AUDIT_PAT secret to upgrade" + echo "::warning::this soft-pass to a hard-pass." + } + echo "Soft pass: skipping runner inventory check." + echo "::endgroup::" + exit 0 + fi + echo "Runner inventory scope: ${runners_scope}" + total="$(jq 'length' < "${runners_json}")" + echo "Visible runners (${runners_scope} scope): ${total}" + # Find at least one ONLINE runner with all required labels. + matched="$(jq --argjson labels "$(printf '%s\n' "${required[@]}" | jq -R . | jq -s .)" ' + [ .[] + | select(.status == "online") + | select(($labels | all(. as $l | (.labels // []) | map(.name) | index($l) | type == "number"))) + ] | length + ' < "${runners_json}")" + echo "Runners satisfying ${REQUIRED_LABELS}: ${matched}" + if [ "${matched}" -lt 1 ]; then + { + echo "::error::No online self-hosted runner satisfies the required labels (${REQUIRED_LABELS})." + echo "The Unity matrix would queue forever." + echo "See docs/runbooks/unity-runners-after-transfer.md for the post-transfer runner-group ACL fix." + } + jq '[ .[] | {name, status, busy, labels: [(.labels // [])[].name]} ]' \ + < "${runners_json}" + exit 1 + fi + echo "::endgroup::" + + unity-tests: + name: Unity ${{ matrix.unity-version }} ${{ matrix.test-mode }} + needs: + - matrix-config + - runner-preflight + if: >- + ${{ + (github.event_name != 'pull_request' || + github.event.pull_request.head.repo.full_name == github.repository) && + (github.event_name != 'push' || github.ref_protected) && + needs.matrix-config.outputs.has-required-secrets == 'true' + }} + runs-on: [self-hosted, Windows, RAM-64GB] + timeout-minutes: 660 + strategy: + fail-fast: false + max-parallel: 1 + matrix: + unity-version: ${{ fromJSON(needs.matrix-config.outputs.unity-versions) }} + test-mode: ${{ fromJSON(needs.matrix-config.outputs.test-modes) }} + steps: + - name: Enable Git long paths + shell: pwsh + env: + GIT_CONFIG_GLOBAL: ${{ runner.temp }}/uh-gitconfig + run: git config --global core.longpaths true + + - name: Checkout + uses: actions/checkout@v6 + env: + GIT_CONFIG_GLOBAL: ${{ runner.temp }}/uh-gitconfig + with: + lfs: true + persist-credentials: false + + - name: Print runner diagnostics + # Composite action runs PowerShell internally; `shell: bash` on a + # self-hosted Windows runner can resolve to the WSL stub at + # C:\Windows\System32\bash.exe and fail with "Windows Subsystem + # for Linux has no installed distributions." Never use plain + # `shell: bash` on self-hosted Windows runners. + uses: ./.github/actions/print-self-hosted-runner-diagnostics + with: + matrix-note: "default (organization lock wraps the Unity section)" + + - name: Cache Unity Library and package caches + uses: actions/cache@v5 + env: + PACKAGE_HASH: >- + ${{ hashFiles( + 'package.json', + 'Runtime/**', + 'Editor/**', + 'Tests/**', + 'scripts/unity/run-ci-tests.ps1', + 'scripts/unity/lib/asmdef-discovery.js', + '.github/actions/compute-unity-assemblies/action.yml', + '.github/integration-packages.json' + ) }} + with: + path: | + .artifacts/unity/projects/${{ matrix.unity-version }}-${{ matrix.test-mode }}/Library + .artifacts/unity/cache/${{ matrix.unity-version }} + key: Library-${{ runner.os }}-${{ runner.arch }}-${{ matrix.unity-version }}-${{ matrix.test-mode }}-${{ env.PACKAGE_HASH }} + + - name: Setup Node.js + uses: actions/setup-node@v6 + with: + node-version: "22.18.0" + + # Single source of truth for the assembly include list: the same + # asmdef-discovery.js module the local discovery uses. Extracted into a + # composite action so all Unity workflows share one implementation. + # shell: pwsh is enforced inside the composite. + - name: Compute test assembly list + id: compute + uses: ./.github/actions/compute-unity-assemblies + with: + target: "${{ matrix.test-mode }}" + runtime-only: "${{ matrix.test-mode == 'standalone' && 'true' || 'false' }}" + # Run the DI-container integration suites (Reflex / VContainer / Zenject) + # alongside core on EVERY version x mode leg. The matching run step passes + # -IncludeIntegrations so the 3 packages are installed into the ephemeral + # manifest; without that the integration assemblies would not compile. + include-integrations: "true" + + - name: Validate Unity license secrets + uses: ./.github/actions/validate-unity-license + env: + UNITY_SERIAL: ${{ secrets.UNITY_SERIAL }} + UNITY_EMAIL: ${{ secrets.UNITY_EMAIL }} + UNITY_PASSWORD: ${{ secrets.UNITY_PASSWORD }} + + # Skip provisioning, the org lock, and the run when no unity-helpers test + # assembly matched this target (compute step resolved an empty list). The + # verify step runs only after compute succeeds and is told the run was an + # expected skip via the expected-empty input. This never triggers for the + # current asmdef set; it is the robustness path for a target with no + # matching assemblies. If checkout/cache/setup fails before compute, let + # that setup failure stand on its own instead of adding a misleading + # "tests did not run" annotation for a Unity run that never started. + - name: Provision Unity Editor + if: ${{ steps.compute.outputs.is-empty != 'true' }} + timeout-minutes: 180 + shell: pwsh + run: | + $artifactsPath = '.artifacts/unity/${{ matrix.unity-version }}-${{ matrix.test-mode }}' + $diagnosticsPath = Join-Path $artifactsPath 'provisioning' + $diagnosticsFile = Join-Path $diagnosticsPath 'ensure-editor-summary.json' + New-Item -ItemType Directory -Force -Path $diagnosticsPath | Out-Null + $provisioningProfile = if ('${{ matrix.test-mode }}' -eq 'standalone') { + 'StandaloneWindowsIl2Cpp' + } else { + 'EditorOnly' + } + $editor = ./scripts/unity/ensure-editor.ps1 ` + -UnityVersion '${{ matrix.unity-version }}' ` + -CiManagedOnly ` + -RequireHealthyExisting ` + -ProvisioningProfile $provisioningProfile ` + -DiagnosticsPath $diagnosticsFile + "UNITY_EDITOR_PATH=$editor" | Out-File -FilePath $env:GITHUB_ENV -Append + + - name: Upload Unity provisioning diagnostics + if: always() + uses: actions/upload-artifact@v7 + with: + name: unity-provisioning-${{ matrix.unity-version }}-${{ matrix.test-mode }} + path: .artifacts/unity/${{ matrix.unity-version }}-${{ matrix.test-mode }}/provisioning + if-no-files-found: warn + retention-days: 14 + + - name: Acquire organization Unity lock + if: ${{ steps.compute.outputs.is-empty != 'true' }} + uses: Ambiguous-Interactive/ambiguous-organization-build-lock/.github/actions/acquire-build-lock@v1 + with: + lock-name: wallstop-organization-builds + holder-id-suffix: ${{ matrix.unity-version }}-${{ matrix.test-mode }} + timeout-minutes: "300" + env: + BUILD_LOCK_TOKEN: ${{ secrets.ORG_BUILD_LOCK_TOKEN }} + + - name: Run Unity Test Runner + id: run_tests + if: ${{ steps.compute.outputs.is-empty != 'true' }} + # Per-mode timeout. 150: ONLY the standalone leg needs it -- it builds a + # NON-development IL2CPP player with the Release C++ configuration, and a + # run-ci-tests.ps1 change rotates the Library cache key, so first runs + # build cold. editmode/playmode are in-editor runs whose healthy worst case + # is the ~40-min SINGLE_THREADED EditMode baseline, so 90 gives >2x headroom + # yet fails a hang ~60 min sooner than the old blanket 150 (a dead Unity + # Accelerator once stalled EditMode for the full 150 and produced no + # results.xml). All values stay under the job timeout (660) so the step + # clock fires first and releases the shared Unity seat. + timeout-minutes: ${{ (matrix.test-mode == 'standalone' && 150) || 90 }} + shell: pwsh + env: + UNITY_SERIAL: ${{ secrets.UNITY_SERIAL }} + UNITY_EMAIL: ${{ secrets.UNITY_EMAIL }} + UNITY_PASSWORD: ${{ secrets.UNITY_PASSWORD }} + UNITY_ACCELERATOR_ENDPOINT: ${{ secrets.UNITY_ACCELERATOR_ENDPOINT }} + UH_UNITY_TEST_CATEGORY: "!Performance;!Stress" + run: | + ./scripts/unity/run-ci-tests.ps1 ` + -UnityVersion '${{ matrix.unity-version }}' ` + -TestMode '${{ matrix.test-mode }}' ` + -AssemblyNames $env:UH_TEST_ASSEMBLIES ` + -ArtifactsPath '.artifacts/unity/${{ matrix.unity-version }}-${{ matrix.test-mode }}' ` + -IncludeIntegrations ` + -ReleaseCodeOptimization ` + -ReleasePlayerBuild + + # Surface the slowest fixtures/cases every run (the measurement backbone for + # keeping the suite fast). Report-only for now; pass -FixtureBudgetSeconds N + # -FailOverBudget here to turn it into a hard wall-clock gate once baselined. + - name: Report slowest tests + if: ${{ always() && steps.compute.outputs.is-empty != 'true' }} + shell: pwsh + run: | + $results = '.artifacts/unity/${{ matrix.unity-version }}-${{ matrix.test-mode }}/results.xml' + if (Test-Path -LiteralPath $results) { + ./scripts/unity/report-slow-tests.ps1 -ResultsPath $results -Top 30 -StepSummary + } else { + Write-Host "::notice::No results.xml at $results to report slow tests." + } + + - name: Return Unity license + if: always() + uses: ./.github/actions/return-unity-license + env: + UNITY_EMAIL: ${{ secrets.UNITY_EMAIL }} + UNITY_PASSWORD: ${{ secrets.UNITY_PASSWORD }} + + - name: Release organization Unity lock + if: always() + uses: Ambiguous-Interactive/ambiguous-organization-build-lock/.github/actions/release-build-lock@v1 + with: + lock-name: wallstop-organization-builds + holder-id-suffix: ${{ matrix.unity-version }}-${{ matrix.test-mode }} + env: + BUILD_LOCK_TOKEN: ${{ secrets.ORG_BUILD_LOCK_TOKEN }} + + # CANCELLATION + TIMEOUT DIAGNOSTIC: when Unity hangs in csc the runner + # step times out (or the operator cancels), and the verify step below + # skips -- so its catastrophic-pattern scan never fires either. This step + # covers both failure() AND cancelled() so the operator has SOMETHING to + # look at in the GitHub summary (the tail of unity.log + any catastrophic + # patterns the partial log happened to surface). Best-effort: never fails + # the job, no-ops when unity.log doesn't exist (cancellation before Unity + # launched). + - name: Dump Unity log tail on failure or cancellation + if: ${{ failure() || cancelled() }} + uses: ./.github/actions/dump-unity-log-tail + with: + results-dir: .artifacts/unity/${{ matrix.unity-version }}-${{ matrix.test-mode }} + label: Unity ${{ matrix.unity-version }} ${{ matrix.test-mode }} + + # CLASS GUARD: Unity can fail before NUnit writes results. The shared + # composite fails unless a results XML exists AND reports total > 0. + # Skip on cancellation so a user-cancelled run does not emit the + # generic "tests did not run" annotation -- the cancellation itself + # is the explanation. Also skip if setup/provisioning/lock work failed + # before the Unity run step attempted; that earlier failure is then the + # actionable signal. The guard still runs for intentional empty targets, + # after a Unity-run failure (to surface "tests did not run" / + # catastrophic-compile diagnostics), and on success (to confirm a real + # result XML exists). + - name: Verify tests actually ran + if: >- + ${{ + !cancelled() && + steps.compute.outcome == 'success' && + (steps.compute.outputs.is-empty == 'true' || steps.run_tests.outcome != 'skipped') + }} + uses: ./.github/actions/verify-unity-results + with: + results-dir: .artifacts/unity/${{ matrix.unity-version }}-${{ matrix.test-mode }} + label: Unity ${{ matrix.unity-version }} ${{ matrix.test-mode }} + expected-empty: ${{ steps.compute.outputs.is-empty }} + + - name: Upload Unity test artifacts + if: always() + uses: actions/upload-artifact@v7 + with: + name: unity-${{ matrix.unity-version }}-${{ matrix.test-mode }} + path: .artifacts/unity/${{ matrix.unity-version }}-${{ matrix.test-mode }} + if-no-files-found: warn + retention-days: 14 + + # SINGLE_THREADED leg. The default matrix never exercises the SINGLE_THREADED + # code paths (Runtime/Core/DataStructure/Cache.cs et al. gate thread-safe vs + # single-threaded behavior on `#if SINGLE_THREADED`, OFF by default). This job + # builds+tests with SINGLE_THREADED (and WALLSTOP_CONCAVE_HULL_STATS) defined as + # GLOBAL scripting symbols so EVERY asmdef test assembly compiles the + # single-threaded path. Restricted to Unity 6 (6000.3.16f1) editmode+playmode to + # keep time small (no IL2CPP standalone build). Core-only (no integrations): the + # single-threaded toggle is a core-runtime concern, so installing the DI packages + # would only add cost. It is a SEPARATE job from unity-tests but shares the SAME + # self-hosted Unity seat; the org lock `wallstop-organization-builds` (acquired + # below, released if:always()) is what actually serializes it against the main + # matrix on that single seat -- max-parallel:1 only serializes within this job's + # own (version x mode) legs. Reuses the same secrets gate, runner-preflight, + # provisioning, perf-category exclusion, verify-results, and artifact-upload + # scaffolding as unity-tests, with DISTINCT project/Library/artifact paths + # (suffix `-single-threaded`) so its differently-compiled Library never collides + # with or poisons the default matrix's `6000.3.16f1-{mode}` cache. + unity-tests-single-threaded: + name: Unity ${{ matrix.unity-version }} ${{ matrix.test-mode }} (SINGLE_THREADED) + needs: + - matrix-config + - runner-preflight + if: >- + ${{ + (github.event_name != 'pull_request' || + github.event.pull_request.head.repo.full_name == github.repository) && + (github.event_name != 'push' || github.ref_protected) && + needs.matrix-config.outputs.has-required-secrets == 'true' + }} + runs-on: [self-hosted, Windows, RAM-64GB] + timeout-minutes: 660 + strategy: + fail-fast: false + max-parallel: 1 + matrix: + # Unity 6 ONLY (6000.3.16f1) to keep the extra leg cheap. editmode + playmode + # only -- both are fast in-editor runs; the SINGLE_THREADED path does not need + # a standalone IL2CPP player build to be exercised. + unity-version: ["6000.3.16f1"] + test-mode: ["editmode", "playmode"] + steps: + - name: Enable Git long paths + shell: pwsh + env: + GIT_CONFIG_GLOBAL: ${{ runner.temp }}/uh-gitconfig + run: git config --global core.longpaths true + + - name: Checkout + uses: actions/checkout@v6 + env: + GIT_CONFIG_GLOBAL: ${{ runner.temp }}/uh-gitconfig + with: + lfs: true + persist-credentials: false + + - name: Print runner diagnostics + # Composite action runs PowerShell internally; `shell: bash` on a + # self-hosted Windows runner can resolve to the WSL stub and fail. Never + # use plain `shell: bash` on self-hosted Windows runners. + uses: ./.github/actions/print-self-hosted-runner-diagnostics + with: + matrix-note: "single-threaded (organization lock wraps the Unity section)" + + - name: Cache Unity Library and package caches + uses: actions/cache@v5 + env: + PACKAGE_HASH: >- + ${{ hashFiles( + 'package.json', + 'Runtime/**', + 'Editor/**', + 'Tests/**', + 'scripts/unity/run-ci-tests.ps1', + 'scripts/unity/lib/asmdef-discovery.js', + '.github/actions/compute-unity-assemblies/action.yml', + '.github/integration-packages.json' + ) }} + with: + path: | + .artifacts/unity/projects/${{ matrix.unity-version }}-${{ matrix.test-mode }}-single-threaded/Library + .artifacts/unity/cache/${{ matrix.unity-version }} + # `single-threaded` in BOTH the project path above and this key keeps this + # leg's Library (compiled WITH SINGLE_THREADED) separate from the default + # matrix's `6000.3.16f1-{mode}` Library (compiled WITHOUT it). + key: Library-${{ runner.os }}-${{ runner.arch }}-${{ matrix.unity-version }}-${{ matrix.test-mode }}-single-threaded-${{ env.PACKAGE_HASH }} + + - name: Setup Node.js + uses: actions/setup-node@v6 + with: + node-version: "22.18.0" + + - name: Compute test assembly list + id: compute + uses: ./.github/actions/compute-unity-assemblies + with: + target: "${{ matrix.test-mode }}" + runtime-only: "false" + + - name: Validate Unity license secrets + uses: ./.github/actions/validate-unity-license + env: + UNITY_SERIAL: ${{ secrets.UNITY_SERIAL }} + UNITY_EMAIL: ${{ secrets.UNITY_EMAIL }} + UNITY_PASSWORD: ${{ secrets.UNITY_PASSWORD }} + + - name: Provision Unity Editor + if: ${{ steps.compute.outputs.is-empty != 'true' }} + timeout-minutes: 180 + shell: pwsh + run: | + $artifactsPath = '.artifacts/unity/${{ matrix.unity-version }}-${{ matrix.test-mode }}-single-threaded' + $diagnosticsPath = Join-Path $artifactsPath 'provisioning' + $diagnosticsFile = Join-Path $diagnosticsPath 'ensure-editor-summary.json' + New-Item -ItemType Directory -Force -Path $diagnosticsPath | Out-Null + $editor = ./scripts/unity/ensure-editor.ps1 ` + -UnityVersion '${{ matrix.unity-version }}' ` + -CiManagedOnly ` + -RequireHealthyExisting ` + -ProvisioningProfile 'EditorOnly' ` + -DiagnosticsPath $diagnosticsFile + "UNITY_EDITOR_PATH=$editor" | Out-File -FilePath $env:GITHUB_ENV -Append + + - name: Upload Unity provisioning diagnostics + if: always() + uses: actions/upload-artifact@v7 + with: + name: unity-provisioning-${{ matrix.unity-version }}-${{ matrix.test-mode }}-single-threaded + path: .artifacts/unity/${{ matrix.unity-version }}-${{ matrix.test-mode }}-single-threaded/provisioning + if-no-files-found: warn + retention-days: 14 + + - name: Acquire organization Unity lock + if: ${{ steps.compute.outputs.is-empty != 'true' }} + uses: Ambiguous-Interactive/ambiguous-organization-build-lock/.github/actions/acquire-build-lock@v1 + with: + lock-name: wallstop-organization-builds + holder-id-suffix: ${{ matrix.unity-version }}-${{ matrix.test-mode }}-single-threaded + timeout-minutes: "300" + env: + BUILD_LOCK_TOKEN: ${{ secrets.ORG_BUILD_LOCK_TOKEN }} + + - name: Run Unity Test Runner + id: run_tests + if: ${{ steps.compute.outputs.is-empty != 'true' }} + # This job is editmode + playmode ONLY (no standalone IL2CPP build), so 90 + # min -- >2x the ~40-min healthy SINGLE_THREADED EditMode baseline -- is the + # fail-fast cap. The old 150 let a dead-Accelerator EditMode stall burn the + # full window and emit no results.xml; 90 surfaces that ~60 min sooner and + # releases the shared Unity seat. Stays under the job timeout (660). + timeout-minutes: 90 + shell: pwsh + env: + UNITY_SERIAL: ${{ secrets.UNITY_SERIAL }} + UNITY_EMAIL: ${{ secrets.UNITY_EMAIL }} + UNITY_PASSWORD: ${{ secrets.UNITY_PASSWORD }} + UNITY_ACCELERATOR_ENDPOINT: ${{ secrets.UNITY_ACCELERATOR_ENDPOINT }} + UH_UNITY_TEST_CATEGORY: "!Performance;!Stress" + run: | + ./scripts/unity/run-ci-tests.ps1 ` + -UnityVersion '${{ matrix.unity-version }}' ` + -TestMode '${{ matrix.test-mode }}' ` + -AssemblyNames $env:UH_TEST_ASSEMBLIES ` + -ArtifactsPath '.artifacts/unity/${{ matrix.unity-version }}-${{ matrix.test-mode }}-single-threaded' ` + -ProjectPath '.artifacts/unity/projects/${{ matrix.unity-version }}-${{ matrix.test-mode }}-single-threaded' ` + -AdditionalScriptingDefines SINGLE_THREADED,WALLSTOP_CONCAVE_HULL_STATS ` + -ReleaseCodeOptimization ` + -ReleasePlayerBuild + + # Surface the slowest fixtures/cases for the SINGLE_THREADED leg (the 40-min + # leg the optimization targets). Report-only; see the main job's note. + - name: Report slowest tests + if: ${{ always() && steps.compute.outputs.is-empty != 'true' }} + shell: pwsh + run: | + $results = '.artifacts/unity/${{ matrix.unity-version }}-${{ matrix.test-mode }}-single-threaded/results.xml' + if (Test-Path -LiteralPath $results) { + ./scripts/unity/report-slow-tests.ps1 -ResultsPath $results -Top 30 -StepSummary + } else { + Write-Host "::notice::No results.xml at $results to report slow tests." + } + + - name: Return Unity license + if: always() + uses: ./.github/actions/return-unity-license + env: + UNITY_EMAIL: ${{ secrets.UNITY_EMAIL }} + UNITY_PASSWORD: ${{ secrets.UNITY_PASSWORD }} + + - name: Release organization Unity lock + if: always() + uses: Ambiguous-Interactive/ambiguous-organization-build-lock/.github/actions/release-build-lock@v1 + with: + lock-name: wallstop-organization-builds + holder-id-suffix: ${{ matrix.unity-version }}-${{ matrix.test-mode }}-single-threaded + env: + BUILD_LOCK_TOKEN: ${{ secrets.ORG_BUILD_LOCK_TOKEN }} + + - name: Dump Unity log tail on failure or cancellation + if: ${{ failure() || cancelled() }} + uses: ./.github/actions/dump-unity-log-tail + with: + results-dir: .artifacts/unity/${{ matrix.unity-version }}-${{ matrix.test-mode }}-single-threaded + label: Unity ${{ matrix.unity-version }} ${{ matrix.test-mode }} (SINGLE_THREADED) + + - name: Verify tests actually ran + if: >- + ${{ + !cancelled() && + steps.compute.outcome == 'success' && + (steps.compute.outputs.is-empty == 'true' || steps.run_tests.outcome != 'skipped') + }} + uses: ./.github/actions/verify-unity-results + with: + results-dir: .artifacts/unity/${{ matrix.unity-version }}-${{ matrix.test-mode }}-single-threaded + label: Unity ${{ matrix.unity-version }} ${{ matrix.test-mode }} (SINGLE_THREADED) + expected-empty: ${{ steps.compute.outputs.is-empty }} + + - name: Upload Unity test artifacts + if: always() + uses: actions/upload-artifact@v7 + with: + name: unity-${{ matrix.unity-version }}-${{ matrix.test-mode }}-single-threaded + path: .artifacts/unity/${{ matrix.unity-version }}-${{ matrix.test-mode }}-single-threaded + if-no-files-found: warn + retention-days: 14 diff --git a/.github/workflows/unstick-run.yml b/.github/workflows/unstick-run.yml new file mode 100644 index 000000000..8c55cae85 --- /dev/null +++ b/.github/workflows/unstick-run.yml @@ -0,0 +1,330 @@ +name: Unstick Run + +# Manual one-click recovery for a single GitHub Actions run stuck on the +# known self-hosted dispatcher bug +# (https://github.com/orgs/community/discussions/186811). +# +# This workflow is the operator-driven sibling of stuck-job-watchdog.yml: +# the watchdog auto-scans every 5 minutes from the default branch, while +# this workflow targets ONE explicit run id on demand. Use this when: +# * The watchdog is not yet on the default branch (GitHub `schedule:` +# cron triggers fire only from the repo default branch, so a watchdog +# committed to a feature branch is inactive until merged to master). +# * You want immediate recovery and do not want to wait for the next +# cron tick or the queue-age threshold. +# +# Behavior mirrors the watchdog's per-run logic: +# * Confirm the run exists, is `status: queued`, and is older than +# MIN_AGE_SECONDS=30 (guards against accidental cancellation of fresh +# runs). +# * Honor the same workflow-file exclusion list (release.yml + +# `vars.WATCHDOG_EXCLUDED_WORKFLOWS`) unless `bypass_exclusion=true`. +# * `gh run cancel` the run. +# * If `force_redispatch=true` AND the run was push/schedule/ +# workflow_dispatch on a branch AND the workflow file declares +# `workflow_dispatch:`, REST-redispatch via +# `POST repos/{owner}/{repo}/actions/workflows/{workflow_id}/dispatches`. +# * Otherwise, surface a step-summary line instructing the operator to +# click "Re-run all jobs" in the UI (the only safe recovery path for +# pull_request runs). +# +# This workflow does NOT touch the watchdog state branch and does NOT +# count against the watchdog's per-run cancel cap. + +on: + workflow_dispatch: + inputs: + run_id: + description: "GitHub Actions run id to recover (digits only)." + required: true + type: string + force_redispatch: + description: "Attempt REST re-dispatch after cancel (push/schedule/workflow_dispatch on a branch only)." + required: false + type: boolean + default: false + bypass_exclusion: + description: "Operate on a run whose workflow file is in the exclusion list (e.g., release.yml). Use deliberately." + required: false + type: boolean + default: false + +concurrency: + group: unstick-run-${{ inputs.run_id }} + cancel-in-progress: false + +permissions: + actions: write + contents: read + +jobs: + unstick: + name: Unstick a single dispatcher-stuck run + runs-on: ubuntu-latest + timeout-minutes: 5 + steps: + - name: Validate, cancel, and optionally re-dispatch + shell: bash + env: + GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} + REPO: ${{ github.repository }} + INPUT_RUN_ID: ${{ inputs.run_id }} + INPUT_FORCE_REDISPATCH: ${{ inputs.force_redispatch }} + INPUT_BYPASS_EXCLUSION: ${{ inputs.bypass_exclusion }} + MIN_AGE_SECONDS: "30" + DEFAULT_EXCLUDED_WORKFLOWS: "release.yml" + EXTRA_EXCLUDED_WORKFLOWS: ${{ vars.WATCHDOG_EXCLUDED_WORKFLOWS }} + run: | + set -euo pipefail + + summary_file="$(mktemp)" + : > "${summary_file}" + log_summary() { + printf '%s\n' "$1" | tee -a "${summary_file}" + } + + flush_summary_and_exit() { + local code="${1:-0}" + { + echo "## Unstick-run summary" + cat "${summary_file}" + } >> "${GITHUB_STEP_SUMMARY}" + exit "${code}" + } + + run_id="${INPUT_RUN_ID}" + force_redispatch="${INPUT_FORCE_REDISPATCH:-false}" + bypass_exclusion="${INPUT_BYPASS_EXCLUSION:-false}" + + log_summary "## Unstick-run audit ($(date -u +'%Y-%m-%dT%H:%M:%SZ'))" + log_summary "Repo: ${REPO}" + log_summary "Requested run id: ${run_id}" + log_summary "force_redispatch: ${force_redispatch}" + log_summary "bypass_exclusion: ${bypass_exclusion}" + + # -------------------------------------------------------------- + # 1. Validate run_id is a positive integer (string input from + # workflow_dispatch is free-form; reject anything else). + # -------------------------------------------------------------- + if ! [[ "${run_id}" =~ ^[0-9]+$ ]]; then + log_summary "ERROR: run_id '${run_id}' is not a positive integer." + flush_summary_and_exit 1 + fi + + # -------------------------------------------------------------- + # 1b. Defense-in-depth: refuse to act on this workflow's own + # run id. The watchdog has the same SELF_RUN_ID guard for + # the same reason -- a self-cancel mid-run produces a + # confusing partial step-summary and leaves the operator + # uncertain whether the target run was actually touched. + # -------------------------------------------------------------- + if [[ "${run_id}" == "${GITHUB_RUN_ID}" ]]; then + log_summary "ERROR: cannot unstick this workflow's own run (run_id == GITHUB_RUN_ID == ${GITHUB_RUN_ID})." + flush_summary_and_exit 1 + fi + + # -------------------------------------------------------------- + # 2. Fetch the run. 404 => either the id is bogus or it belongs + # to a different repository (gh api scopes the call to REPO). + # -------------------------------------------------------------- + run_json="$(mktemp)" + if ! gh api "repos/${REPO}/actions/runs/${run_id}" > "${run_json}" 2>/dev/null; then + log_summary "ERROR: run ${run_id} not found in ${REPO} (404). Confirm the run id and that it belongs to this repository." + flush_summary_and_exit 1 + fi + + run_status="$(jq -r '.status // ""' < "${run_json}")" + run_event="$(jq -r '.event // ""' < "${run_json}")" + run_head_branch="$(jq -r '.head_branch // ""' < "${run_json}")" + run_workflow_id="$(jq -r '.workflow_id // 0' < "${run_json}")" + run_path="$(jq -r '.path // ""' < "${run_json}")" + run_name="$(jq -r '.name // ""' < "${run_json}")" + run_html_url="$(jq -r '.html_url // ""' < "${run_json}")" + run_created_at="$(jq -r '.created_at // ""' < "${run_json}")" + run_path_base="${run_path##*/}" + + log_summary "Run name: ${run_name}" + log_summary "Workflow file: ${run_path_base}" + log_summary "Status (pre-cancel): ${run_status}" + log_summary "Event: ${run_event}" + log_summary "Head branch: ${run_head_branch:-}" + log_summary "URL: ${run_html_url}" + + # -------------------------------------------------------------- + # 3. Cheap safety guard: only operate on queued runs. + # -------------------------------------------------------------- + if [[ "${run_status}" != "queued" ]]; then + log_summary "Run is not currently queued (status: ${run_status}). Skipping cancel; nothing to recover." + flush_summary_and_exit 0 + fi + + # -------------------------------------------------------------- + # 4. Cheap safety guard: ensure the run is at least + # MIN_AGE_SECONDS old so a manual misclick does not nuke a + # fresh, legitimately-queued run. + # -------------------------------------------------------------- + if [[ -z "${run_created_at}" ]]; then + log_summary "ERROR: run ${run_id} has no created_at timestamp; refusing to act." + flush_summary_and_exit 1 + fi + created_epoch="$(date -u -d "${run_created_at}" +%s 2>/dev/null || echo 0)" + if [[ "${created_epoch}" -eq 0 ]]; then + log_summary "ERROR: could not parse created_at '${run_created_at}'." + flush_summary_and_exit 1 + fi + now_epoch="$(date -u +%s)" + age=$(( now_epoch - created_epoch )) + log_summary "Run age: ${age}s (min: ${MIN_AGE_SECONDS}s)" + if (( age < MIN_AGE_SECONDS )); then + log_summary "Run is younger than MIN_AGE_SECONDS=${MIN_AGE_SECONDS}; skipping. Re-invoke after the run has had time to dispatch normally." + flush_summary_and_exit 0 + fi + + # -------------------------------------------------------------- + # 5. Workflow-file exclusion check (release.yml + repo variable). + # NOTE: This exclusion-list construction is duplicated from + # stuck-job-watchdog.yml. Kept intentionally duplicated so each + # workflow file is self-contained; if you change the rules here, + # update the watchdog too. + # -------------------------------------------------------------- + declare -A EXCLUDED_BY_FILE=() + for wf in ${DEFAULT_EXCLUDED_WORKFLOWS} ${EXTRA_EXCLUDED_WORKFLOWS:-}; do + [[ -z "${wf}" ]] && continue + base="${wf##*/}" + EXCLUDED_BY_FILE["${base}"]=1 + done + excluded_list="" + for k in "${!EXCLUDED_BY_FILE[@]}"; do + excluded_list+="${k} " + done + log_summary "Excluded workflows: ${excluded_list:-}" + + if [[ -n "${EXCLUDED_BY_FILE[${run_path_base}]+x}" ]]; then + if [[ "${bypass_exclusion}" != "true" ]]; then + log_summary "Workflow '${run_path_base}' is in the exclusion list; pass bypass_exclusion=true to operate on it. Skipping." + flush_summary_and_exit 0 + fi + log_summary "Workflow '${run_path_base}' is excluded but bypass_exclusion=true; proceeding." + fi + + # -------------------------------------------------------------- + # 6. Cancel the run. From here on, this is the recovery action. + # -------------------------------------------------------------- + log_summary "Cancelling run ${run_id}..." + # TOCTOU note: if the run transitions out of `queued` between status check and + # cancel, gh run cancel returns non-zero. We let the step fail loudly so the + # operator notices; watchdog has the same intentional pattern. + if ! gh run cancel "${run_id}" --repo "${REPO}" 2>&1 | tee -a "${summary_file}"; then + log_summary "ERROR: 'gh run cancel ${run_id}' failed." + flush_summary_and_exit 1 + fi + log_summary "Cancel issued." + + # Re-fetch status post-cancel for the summary. If the re-fetch + # fails (transient API hiccup, run vanished), surface that + # explicitly rather than logging the stale pre-cancel status as + # though it were the post-cancel value. + post_status="unknown (status re-fetch failed)" + if gh api "repos/${REPO}/actions/runs/${run_id}" > "${run_json}" 2>/dev/null; then + post_status="$(jq -r '.status // ""' < "${run_json}")" + fi + log_summary "Status (post-cancel): ${post_status}" + + # -------------------------------------------------------------- + # 7. Optional REST re-dispatch. + # -------------------------------------------------------------- + redispatched="no" + redispatch_reason="" + if [[ "${force_redispatch}" == "true" ]]; then + case "${run_event}" in + push|schedule|workflow_dispatch) + if [[ -z "${run_head_branch}" ]]; then + redispatch_reason="event=${run_event} but head_branch is empty (likely a tag); cannot REST-dispatch." + else + # Parse the workflow file to confirm it declares + # `workflow_dispatch:` (required by the dispatches API). + workflow_file_raw="$(mktemp)" + has_dispatch=0 + if [[ -n "${run_path}" ]]; then + set +e + gh api "repos/${REPO}/contents/${run_path}" --jq '.content' 2>/dev/null \ + | base64 -d > "${workflow_file_raw}" 2>/dev/null + contents_rc=$? + set -e + if [[ "${contents_rc}" -eq 0 && -s "${workflow_file_raw}" ]]; then + set +e + grep -Eq '^[[:space:]]*workflow_dispatch:' "${workflow_file_raw}" + grep_rc=$? + set -e + if [[ "${grep_rc}" -eq 0 ]]; then + has_dispatch=1 + fi + fi + fi + + if [[ "${has_dispatch}" -eq 1 ]]; then + log_summary "Re-dispatching workflow ${run_workflow_id} on ref '${run_head_branch}'..." + if gh api -X POST \ + "repos/${REPO}/actions/workflows/${run_workflow_id}/dispatches" \ + -f "ref=${run_head_branch}" 2>&1 | tee -a "${summary_file}"; then + redispatched="yes" + else + redispatch_reason="REST dispatches call failed (see log above)." + fi + else + redispatch_reason="workflow file '${run_path_base}' does not declare 'workflow_dispatch:' (or could not be read)." + fi + fi + ;; + *) + redispatch_reason="event='${run_event}' has no safe REST re-dispatch path (only push/schedule/workflow_dispatch on a branch are supported). Operator must use the UI." + ;; + esac + else + redispatch_reason="force_redispatch=false (default)." + fi + + if [[ "${redispatched}" == "yes" ]]; then + log_summary "Re-dispatch: succeeded." + else + log_summary "Re-dispatch: skipped (${redispatch_reason})" + fi + + # -------------------------------------------------------------- + # 8. Operator next-step guidance. + # -------------------------------------------------------------- + if [[ "${redispatched}" == "yes" ]]; then + log_summary "Next step: the workflow has been re-dispatched. Watch ${run_html_url%/runs/*} for the new run." + else + case "${run_event}" in + pull_request|pull_request_target) + log_summary "Next step: open ${run_html_url} and click 'Re-run all jobs' (PR-triggered runs cannot be REST-dispatched)." + ;; + *) + # When force_redispatch=true was requested but the re-dispatch + # was skipped (event/branch/workflow_dispatch trigger guard), + # re-suggesting force_redispatch=true would just repeat the + # path the operator already tried. Branch the guidance. + if [[ "${force_redispatch}" == "true" ]]; then + log_summary "Next step: force_redispatch was attempted but skipped (see reason above). Click 'Re-run all jobs' on the run's UI page (${run_html_url}) to recover." + else + log_summary "Next step: open ${run_html_url} and click 'Re-run all jobs', OR re-invoke this workflow with force_redispatch=true if the event supports it." + fi + ;; + esac + fi + + # -------------------------------------------------------------- + # 9. Rate-limit guidance (this workflow has NO automatic cap). + # -------------------------------------------------------------- + # Unlike the watchdog (MAX_CANCELS_PER_DAY=2 per run-id via + # state branch), unstick-run.yml is operator-driven and relies on + # the implicit gate of a human clicking "Run workflow". That + # means rapid repeat invocations on the same run-id are not + # blocked here -- the operator must self-pace. GitHub's + # dispatcher state can take time to settle after a cancel, so a + # second attempt within seconds may race the previous one. + log_summary "Rate-limit note: unstick-run.yml has no automatic per-run cap (operator-driven)." + log_summary "If a single run id needs another recovery attempt, wait at least 5 minutes between invocations to let GitHub's dispatcher state settle." + + flush_summary_and_exit 0 diff --git a/.github/workflows/validate-mcp-config.yml b/.github/workflows/validate-mcp-config.yml new file mode 100644 index 000000000..092c2979b --- /dev/null +++ b/.github/workflows/validate-mcp-config.yml @@ -0,0 +1,47 @@ +name: Validate MCP Config + +on: + push: + branches: + - main + paths: + - ".mcp.json" + - ".cursor/**" + - ".vscode/**" + - ".codex/**" + - "scripts/mcp/**" + - "scripts/validate-mcp-config.ps1" + - "scripts/tests/test-validate-mcp-config.ps1" + - "docs/guides/mcp-local-setup.md" + - ".gitignore" + - ".github/workflows/validate-mcp-config.yml" + pull_request: + paths: + - ".mcp.json" + - ".cursor/**" + - ".vscode/**" + - ".codex/**" + - "scripts/mcp/**" + - "scripts/validate-mcp-config.ps1" + - "scripts/tests/test-validate-mcp-config.ps1" + - "docs/guides/mcp-local-setup.md" + - ".gitignore" + - ".github/workflows/validate-mcp-config.yml" + +permissions: + contents: read + +jobs: + validate-mcp-config: + runs-on: ubuntu-latest + steps: + - name: Checkout + uses: actions/checkout@v6 + + - name: Test MCP config validator + shell: pwsh + run: ./scripts/tests/test-validate-mcp-config.ps1 -VerboseOutput + + - name: Validate MCP config + shell: pwsh + run: ./scripts/validate-mcp-config.ps1 -VerboseOutput diff --git a/.gitignore b/.gitignore index 0724d1ef7..5b2076571 100644 --- a/.gitignore +++ b/.gitignore @@ -46,6 +46,14 @@ logs_extracted* !.claude/settings.json !.claude/.gitkeep .codex/* + +# Machine-local Unity MCP client configuration. The bridge endpoint (host:port) +# is per-developer, written by scripts/mcp/configure-unity-mcp-endpoint.sh — never +# commit these. See docs/guides/mcp-local-setup.md. (.vscode/** and .codex/* above +# already cover the VS Code and Codex client configs.) +.mcp.json +.cursor/mcp.json + _llm_* _llm_*.meta /.*/**/*.meta @@ -333,41 +341,6 @@ __pycache__/ .pytest_cache/ *.pyc -/Unity/DxMessagingUnity/[Ll]ibrary/ -/Unity/DxMessagingUnity/[Tt]emp/ -/Unity/DxMessagingUnity/[Oo]bj/ -/Unity/DxMessagingUnity/[Bb]uild/ -/Unity/DxMessagingUnity/[Bb]uilds/ -/Unity/DxMessagingUnity/Assets/AssetStoreTools* - -# Visual Studio 2015 cache directory -/Unity/DxMessagingUnity/.vs/ - -# Autogenerated VS/MD/Consulo solution and project files -/Unity/DxMessagingUnityExportedObj/ -/Unity/DxMessagingUnity.consulo/ -/Unity/DxMessagingUnity*.csproj -/Unity/DxMessagingUnity*.unityproj -/Unity/DxMessagingUnity*.sln -/Unity/DxMessagingUnity*.suo -/Unity/DxMessagingUnity*.tmp -/Unity/DxMessagingUnity*.user -/Unity/DxMessagingUnity*.userprefs -/Unity/DxMessagingUnity*.pidb -/Unity/DxMessagingUnity*.booproj -/Unity/DxMessagingUnity*.svd -/Unity/DxMessagingUnity*.pdb - -# Unity3D generated meta files -/Unity/DxMessagingUnity*.pidb.meta - -# Unity3D Generated File On Crash Reports -/Unity/DxMessagingUnitysysinfo.txt - -# Builds -/Unity/DxMessagingUnity*.apk -/Unity/DxMessagingUnity*.unitypackage - package-lock.json package-lock.json.meta @@ -406,8 +379,43 @@ artifacts.meta # MkDocs build output site/ +# Git hook output artifacts created by accidental redirection, e.g. +# `git push 2> pre-push.txt`. These are auto-removed by pre-push and +# `npm run agent:preflight:fix` only after git confirms they are ignored. pre-commit.txt* +pre-commit.out +pre-commit.err +pre-merge-commit.txt* +pre-merge-commit.out +pre-merge-commit.err pre-push.txt* +pre-push.out +pre-push.err +post-rewrite.txt* +post-rewrite.out +post-rewrite.err +.githooks/pre-commit.txt +.githooks/pre-commit.log +.githooks/pre-commit.out +.githooks/pre-commit.err +.githooks/pre-commit.tmp +.githooks/pre-merge-commit.txt +.githooks/pre-merge-commit.log +.githooks/pre-merge-commit.out +.githooks/pre-merge-commit.err +.githooks/pre-merge-commit.tmp +.githooks/pre-push.txt +.githooks/pre-push.log +.githooks/pre-push.out +.githooks/pre-push.err +.githooks/pre-push.tmp +.githooks/post-rewrite.txt +.githooks/post-rewrite.log +.githooks/post-rewrite.out +.githooks/post-rewrite.err +.githooks/post-rewrite.tmp benchmarks/ pr-description.md* -test-results* \ No newline at end of file +test-results* +.artifacts/ +.artifacts.meta diff --git a/.llm/code-samples/patterns/ValidationFixPatterns.sh b/.llm/code-samples/patterns/ValidationFixPatterns.sh index d0f9f8b57..1d71ec329 100644 --- a/.llm/code-samples/patterns/ValidationFixPatterns.sh +++ b/.llm/code-samples/patterns/ValidationFixPatterns.sh @@ -32,10 +32,10 @@ fix_line_endings() { } # --- Fix Git Hook Permissions --- -# Hook files must be executable or git silently skips them. +# Extensionless hook entrypoints must be executable or git silently skips them. fix_hook_permissions() { - chmod +x .githooks/* - git update-index --chmod=+x .githooks/pre-commit .githooks/pre-push + chmod +x .githooks/pre-commit .githooks/pre-merge-commit .githooks/pre-push .githooks/post-rewrite + git update-index --chmod=+x .githooks/pre-commit .githooks/pre-merge-commit .githooks/pre-push .githooks/post-rewrite npm run validate:hook-perms } diff --git a/.llm/context.md b/.llm/context.md index 376f7b663..dff8c6e36 100644 --- a/.llm/context.md +++ b/.llm/context.md @@ -50,128 +50,10 @@ Samples~/ # Sample projects (imported via Package Manager) ## Skills Reference -Invoke these skills for specific tasks. - -**Regenerate with**: `pwsh -NoProfile -File scripts/generate-skills-index.ps1` - - - - - -### Core Skills (Always Consider) - -| Skill | When to Use | -| -------------------------------------------------------------------------------------------- | -------------------------------------------------------------------------------------- | -| [apply-completeness](./skills/apply-completeness.md) | Always do the complete thing when cost is near-zero | -| [ask-structured-questions](./skills/ask-structured-questions.md) | Present questions with context, options, and recommendations | -| [asset-postprocessor-safety](./skills/asset-postprocessor-safety.md) | AssetPostprocessor callbacks - avoid SendMessage warnings | -| [avoid-magic-strings](./skills/avoid-magic-strings.md) | ALL code - use nameof() not strings | -| [avoid-reflection](./skills/avoid-reflection.md) | ALL code - never reflect on our own types | -| [bash-pwsh-invocation](./skills/bash-pwsh-invocation.md) | Calling .ps1 scripts from bash/hooks/workflows | -| [create-csharp-file](./skills/create-csharp-file.md) | Creating any new .cs file | -| [create-editor-tool](./skills/create-editor-tool.md) | Creating Editor windows and inspectors | -| [create-enum](./skills/create-enum.md) | Creating a new enum type | -| [create-property-drawer](./skills/create-property-drawer.md) | Creating PropertyDrawers for custom attributes | -| [create-scriptable-object](./skills/create-scriptable-object.md) | Creating ScriptableObject data assets | -| [create-test](./skills/create-test.md) | Writing or modifying test files | -| [create-unity-meta](./skills/create-unity-meta.md) | After creating ANY new file or folder | -| [defensive-editor-programming](./skills/defensive-editor-programming.md) | Editor code - handle Unity Editor edge cases | -| [defensive-programming](./skills/defensive-programming.md) | ALL code - never throw, handle gracefully | -| [documentation-consistency](./skills/documentation-consistency.md) | When writing or reviewing documentation | -| [editor-api-rules](./skills/editor-api-rules.md) | Forbidden Editor APIs and value handling rules | -| [editor-caching-patterns](./skills/editor-caching-patterns.md) | Caching strategies for Editor code | -| [editor-multi-object-editing](./skills/editor-multi-object-editing.md) | Multi-object editing patterns and undo support for editor code | -| [editor-singleton-patterns](./skills/editor-singleton-patterns.md) | Singleton asset management patterns for Editor code | -| [editor-undo-complete](./skills/editor-undo-complete.md) | Complete undo policy for editor tooling with enforceable scope boundaries | -| [formatting](./skills/formatting.md) | After ANY file change (CSharpier/Prettier) | -| [formatting-and-linting](./skills/formatting-and-linting.md) | Before committing, after editing files | -| [git-hook-lifecycle-debugging](./skills/git-hook-lifecycle-debugging.md) | Hook validation philosophy, framework config, PowerShell exit codes, debugging | -| [git-hook-patterns](./skills/git-hook-patterns.md) | Git hook safety, syntax, and debugging patterns (hub) | -| [git-hook-safety](./skills/git-hook-safety.md) | Hook index safety, permissions, and execution templates | -| [git-hook-syntax-portability](./skills/git-hook-syntax-portability.md) | Hook regex, CLI safety, CRLF handling, portable grep patterns | -| [git-safe-operations](./skills/git-safe-operations.md) | Scripts or hooks that interact with git index | -| [git-staging-helpers](./skills/git-staging-helpers.md) | PowerShell/Bash helpers for safe git staging | -| [github-actions-shell-foundations](./skills/github-actions-shell-foundations.md) | Core shell scripting safety for GitHub Actions | -| [github-actions-shell-scripting](./skills/github-actions-shell-scripting.md) | Shell scripting best practices for GitHub Actions | -| [github-actions-shell-workflow-patterns](./skills/github-actions-shell-workflow-patterns.md) | Workflow integration patterns for GitHub Actions shell steps | -| [high-performance-csharp](./skills/high-performance-csharp.md) | ALL code - zero allocation patterns | -| [investigate-test-failures](./skills/investigate-test-failures.md) | ANY test failure - investigate before fixing | -| [license-headers](./skills/license-headers.md) | Maintaining MIT license headers in C# files | -| [linter-reference](./skills/linter-reference.md) | Detailed linter commands, configurations | -| [manage-skills](./skills/manage-skills.md) | Creating, updating, splitting, consolidating, or removing skills | -| [markdown-reference](./skills/markdown-reference.md) | Link formatting, escaping, linting rules | -| [no-regions](./skills/no-regions.md) | ALL C# code - never use #region/#endregion | -| [odin-undo-safety](./skills/odin-undo-safety.md) | Safe undo recording patterns for Odin Inspector drawers | -| [optimize-git-hooks](./skills/optimize-git-hooks.md) | How to keep git hooks fast | -| [prefer-logging-extensions](./skills/prefer-logging-extensions.md) | Unity logging in UnityEngine.Object classes | -| [property-drawer-examples](./skills/property-drawer-examples.md) | Property drawer code examples | -| [property-drawer-rules](./skills/property-drawer-rules.md) | PropertyDrawer critical rules and requirements | -| [review-code-changes](./skills/review-code-changes.md) | Pre-landing code review with two-pass analysis | -| [review-plan](./skills/review-plan.md) | Engineering review of implementation plans | -| [run-retrospective](./skills/run-retrospective.md) | Structured retrospective analyzing what happened, what worked, and what to improve | -| [search-codebase](./skills/search-codebase.md) | Finding code, files, or patterns | -| [self-regulate-changes](./skills/self-regulate-changes.md) | Know when to stop: risk scoring and hard caps for cascading changes | -| [ship-changes](./skills/ship-changes.md) | End-to-end workflow for shipping changes: validate, review, version, changelog, commit | -| [test-data-driven](./skills/test-data-driven.md) | Data-driven testing with TestCase and TestCaseSource | -| [test-naming-conventions](./skills/test-naming-conventions.md) | Test method and TestName naming rules | -| [test-odin-drawers](./skills/test-odin-drawers.md) | Odin Inspector drawer testing patterns | -| [test-parallelization-rules](./skills/test-parallelization-rules.md) | Unity Editor test threading constraints | -| [test-unity-lifecycle](./skills/test-unity-lifecycle.md) | Track(), DestroyImmediate, object cleanup | -| [update-documentation](./skills/update-documentation.md) | After ANY feature/bug fix/API change | -| [validate-before-commit](./skills/validate-before-commit.md) | Before completing any task (run linters!) | -| [validation-troubleshooting](./skills/validation-troubleshooting.md) | Common validation errors, CI failures, fixes | - -### Performance Skills - -| Skill | When to Use | -| -------------------------------------------------------------------- | ------------------------------------------------- | -| [avoid-allocations](./skills/avoid-allocations.md) | Avoiding heap allocations and boxing | -| [gc-architecture-unity](./skills/gc-architecture-unity.md) | Understanding Unity GC, incremental GC, manual GC | -| [linq-elimination-patterns](./skills/linq-elimination-patterns.md) | Converting LINQ to zero-allocation loops | -| [memory-allocation-traps](./skills/memory-allocation-traps.md) | Finding hidden allocation sources | -| [mobile-xr-optimization](./skills/mobile-xr-optimization.md) | Mobile, VR/AR, 90+ FPS targets | -| [optimize-unity-physics](./skills/optimize-unity-physics.md) | Physics colliders, raycasts, non-alloc | -| [optimize-unity-rendering](./skills/optimize-unity-rendering.md) | Materials, shaders, batching | -| [performance-audit](./skills/performance-audit.md) | Reviewing performance-sensitive code | -| [profile-debug-performance](./skills/profile-debug-performance.md) | Profiling, debugging, measuring performance | -| [refactor-to-zero-alloc](./skills/refactor-to-zero-alloc.md) | Converting allocating code to zero-allocation | -| [unity-performance-patterns](./skills/unity-performance-patterns.md) | Unity-specific optimizations (APIs, pooling) | -| [use-array-pool](./skills/use-array-pool.md) | Working with temporary arrays | -| [use-pooling](./skills/use-pooling.md) | Working with temporary collections | - -### Feature Skills - -| Skill | When to Use | -| ------------------------------------------------------------------------------ | ----------------------------------------------------------------- | -| [add-inspector-attribute](./skills/add-inspector-attribute.md) | Improving editor UX with attributes | -| [debug-il2cpp](./skills/debug-il2cpp.md) | IL2CPP build issues or AOT errors | -| [devcontainer-volume-permissions](./skills/devcontainer-volume-permissions.md) | Docker volume permission fixes for non-root devcontainer users | -| [github-actions-script-pattern](./skills/github-actions-script-pattern.md) | Extract GHA logic to testable scripts | -| [github-pages](./skills/github-pages.md) | GitHub Pages, Jekyll, markdown link format | -| [github-pages-theming](./skills/github-pages-theming.md) | GitHub Pages CSS theming, Jekyll theme customization | -| [github-workflow-permissions](./skills/github-workflow-permissions.md) | Workflow permissions, automated PRs, debugging | -| [integrate-odin-inspector](./skills/integrate-odin-inspector.md) | Odin Inspector integration patterns | -| [integrate-optional-dependency](./skills/integrate-optional-dependency.md) | Odin, VContainer, Zenject integration patterns | -| [manage-assembly-definitions](./skills/manage-assembly-definitions.md) | Assembly definition creation, splitting, and reference management | -| [unity-devcontainer-testing](./skills/unity-devcontainer-testing.md) | Compile and test Unity C# code in devcontainer | -| [use-algorithmic-structures](./skills/use-algorithmic-structures.md) | Connectivity, prefix search, bit manipulation, caching | -| [use-data-structures](./skills/use-data-structures.md) | Selecting appropriate data structures | -| [use-discriminated-union](./skills/use-discriminated-union.md) | OneOf/Result types, type-safe unions | -| [use-effects-system](./skills/use-effects-system.md) | Buffs, debuffs, stat modifications | -| [use-extension-methods](./skills/use-extension-methods.md) | Collection, string, color utilities | -| [use-priority-structures](./skills/use-priority-structures.md) | Priority ordering or task scheduling | -| [use-prng](./skills/use-prng.md) | Implementing randomization | -| [use-queue-structures](./skills/use-queue-structures.md) | Rolling history, double-ended queues | -| [use-relational-attributes](./skills/use-relational-attributes.md) | Auto-wiring components via hierarchy | -| [use-serializable-types](./skills/use-serializable-types.md) | Dictionaries, HashSets, Nullable, Type, Guid | -| [use-serializable-types-patterns](./skills/use-serializable-types-patterns.md) | Common patterns for serializable collections | -| [use-serialization](./skills/use-serialization.md) | Save files, network, persistence | -| [use-singleton](./skills/use-singleton.md) | Global managers, service locators, configuration | -| [use-spatial-structure](./skills/use-spatial-structure.md) | Spatial queries or proximity logic | -| [use-threading](./skills/use-threading.md) | Main thread dispatch, thread safety | -| [wiki-generation](./skills/wiki-generation.md) | GitHub Wiki deployment, sidebar links | - - +The full skill catalog, grouped by category, is in the generated +**[Skills Index](./skills/index.md)**. Regenerate it after adding or editing any +skill's trigger comment with `pwsh -NoProfile -File scripts/generate-skills-index.ps1` +(validated by `scripts/lint-llm-instructions.ps1`). ## Critical Rules Summary @@ -187,7 +69,7 @@ See [create-csharp-file](./skills/create-csharp-file.md) for detailed C# rules. 6. One file per MonoBehaviour/ScriptableObject (production AND tests) 7. NEVER use `?.`, `??`, `??=` on UnityEngine.Object types 8. Minimal comments -- only explain **why**, never **what** -9. Generate `.meta` files after creating ANY file/folder (see [create-unity-meta](./skills/create-unity-meta.md)); exception: no `.meta` for dot folders (`.llm/`, `.github/`, `.git/`, `.vscode/`). Use `./scripts/generate-meta.sh `, then run `npm run agent:preflight:fix` immediately. +9. Generate `.meta` files after creating ANY file/folder (see [create-unity-meta](./skills/create-unity-meta.md)); exception: no `.meta` for dot folders (`.llm/`, `.github/`, `.git/`, `.vscode/`). Use `./scripts/generate-meta.sh ` for new or empty folders, then run `npm run agent:preflight:fix` for changed-file `.meta` recovery. 10. Enums: explicit values, `None`/`Unknown` = 0 with `[Obsolete]` (see [create-enum](./skills/create-enum.md)) 11. Never reflect on our own code; use `internal` + `[InternalsVisibleTo]` (see [avoid-reflection](./skills/avoid-reflection.md)) 12. Never use magic strings; use `nameof()` (see [avoid-magic-strings](./skills/avoid-magic-strings.md)) @@ -219,9 +101,9 @@ Run formatters/linters **immediately after each file change**, not batched at ta - **Markdown**: `npm run lint:docs` + `npm run lint:markdown` - **YAML**: `npm run lint:yaml` (then `actionlint` for workflows) - **Spelling**: `npm run lint:spelling` (add valid terms to `cspell.json`). A Claude Code PostToolUse hook (`scripts/hooks/cspell-post-edit.js`, registered in the tracked [`.claude/settings.json`](../.claude/settings.json) which ships with the repo) auto-runs cspell after every Edit/Write/MultiEdit/NotebookEdit, so typos surface immediately; manual invocation before completion remains the expectation (the hook is a safety net, not a substitute -- it does not fire in CI or when editing outside Claude Code) -- **Tests**: `pwsh -NoProfile -File scripts/lint-tests.ps1 -FixNullChecks -Paths ` +- **Tests**: `pwsh -NoProfile -File scripts/lint-tests.ps1 -FixNullChecks -Paths `, then `pwsh -NoProfile -File scripts/lint-tests.ps1 -Paths ` - **Skill files and [context](./context.md)**: `pwsh -NoProfile -File scripts/lint-skill-sizes.ps1` (500-line limit) -- **Commit prep**: stage files, then run `npm run agent:preflight:fix` (includes changed markdown spelling checks) before any commit attempt +- **Commit prep**: stage files, then run `npm run agent:preflight:fix` (includes changed spell-checkable file checks) before any commit attempt - **Pre-push parity**: run `npm run validate:prepush` (includes full `lint:spelling`) before push; treat git hooks as last-resort only. For the push step itself (setup, redirection, rejection handling) follow [ship-changes Step 9](./skills/ship-changes.md#step-9-push-to-remote) See [formatting](./skills/formatting.md) and [validate-before-commit](./skills/validate-before-commit.md) for details. @@ -242,7 +124,8 @@ See [formatting](./skills/formatting.md) and [validate-before-commit](./skills/v - If a script derives `REPO_ROOT` / `$repoRoot` from its own location, every `git ls-files` / `git diff --relative` / similar repo-relative git call must also be anchored there (`git -C "$REPO_ROOT" ...` or `cd "$REPO_ROOT"` first). Never combine repo-root-derived filesystem paths with caller-cwd-derived git output. - When adding formatter support for a new language, add explicit `[language]` entry in `devcontainer.json` - When adding new script calls to git hooks, update the hook's step comments AND the "What the Hook Does" list in [formatting-and-linting](./skills/formatting-and-linting.md) -- Never redirect git command output to files in the working tree (e.g. `git push 2> pre-push.txt`) — creates gitignored pollution. Let errors stream to stderr; `npm run agent:preflight` detects and `npm run agent:preflight:fix` removes these artifacts +- Never run `pwsh -File .githooks/` for extensionless hook launchers. Run the hook directly through Git/shell, or invoke `.githooks/.ps1` when debugging the PowerShell implementation. +- Never redirect git command output to files in the working tree (e.g. `git push 2> pre-push.txt`) — creates gitignored pollution. Let errors stream to stderr; pre-push and `npm run agent:preflight:fix` auto-remove gitignored hook artifacts before validation --- diff --git a/.llm/references/forbidden-patterns.md b/.llm/references/forbidden-patterns.md index ea497d63d..23aa0f129 100644 --- a/.llm/references/forbidden-patterns.md +++ b/.llm/references/forbidden-patterns.md @@ -333,6 +333,22 @@ When passing file arguments to CLI tools, a `--` (end-of-options) separator MUST --- +## Serialization Patterns + +`Serializer` is the single documented carve-out from the "never throw" rule (see [Serialization Safety](../skills/serialization-safety.md)). Inside `Runtime/Core/Serialization/Serializer.cs` and any future format added there: + +| Forbidden | Use Instead | Reason | +| ----------------------------------------------------------------------------------- | ----------------------------------------------------------------------------------------------------------- | ---------------------------------------------------------------------------------------- | +| `new MemoryStream(byte[])` without a prior null/empty guard | `SerializationFailureException.ThrowNullInput` / `ThrowEmptyInput` then `new MemoryStream(data)` | Legacy crash: `ArgumentNullException: Buffer cannot be null` leaked to a ZLinq pipeline. | +| `throw new ProtoBuf.ProtoException(...)` | `SerializationFailureException.ThrowCorrupt(..., inner)` (wrap the framework exception) | Callers can only catch one type — the documented hierarchy. | +| `throw new System.Text.Json.JsonException(...)` | `SerializationFailureException.ThrowCorrupt(..., inner)` | Same. | +| `throw new ArgumentNullException(nameof(data))` from `*Deserialize*` | `SerializationFailureException.ThrowNullInput(format, op)` | Same hierarchy. | +| A new `*Deserialize*` method without a matching `Try*` sibling | Add `TryXxx` overload that catches `SerializationInputException` + `SerializationCorruptDataException` only | `SerializerApiContractTests` will fail the build otherwise. | +| A `Try*` sibling that catches `Exception` | Catch only `SerializationInputException` + `SerializationCorruptDataException` | Programmer errors (`Type` / `Configuration`) must propagate. | +| Silent `catch { return default; }` around `Serializer.*Deserialize*` in caller code | `Try*` sibling, OR catch `SerializationFailureException` and log+fallback | Silent corruption looks identical to a missing field hours later. | + +--- + ## Related Documentation - [high-performance-csharp](../skills/high-performance-csharp.md) - Core performance patterns @@ -340,3 +356,4 @@ When passing file arguments to CLI tools, a `--` (end-of-options) separator MUST - [memory-allocation-traps](../skills/memory-allocation-traps.md) - Hidden allocation sources - [avoid-reflection](../skills/avoid-reflection.md) - Reflection avoidance - [avoid-magic-strings](../skills/avoid-magic-strings.md) - Magic string avoidance +- [serialization-safety](../skills/serialization-safety.md) - Serializer exception contract diff --git a/.llm/skills/asset-postprocessor-safety.md b/.llm/skills/asset-postprocessor-safety.md index 87ba13861..b9a5dff28 100644 --- a/.llm/skills/asset-postprocessor-safety.md +++ b/.llm/skills/asset-postprocessor-safety.md @@ -32,7 +32,7 @@ Use this skill when: Any call we make that forces deserialization, component inspection, or user-callback invocation while we are still inside the callback can trigger this warning. The fix is to **defer the work one editor tick** via `EditorApplication.delayCall`, which lands after the import phase completes. -See issue [#234](https://github.com/wallstop/unity-helpers/issues/234) for the motivating bug. +See [#234](https://github.com/wallstop/unity-helpers/pull/234) for the motivating bug. --- diff --git a/.llm/skills/bash-pwsh-invocation.md b/.llm/skills/bash-pwsh-invocation.md index 532303a90..a05bc7617 100644 --- a/.llm/skills/bash-pwsh-invocation.md +++ b/.llm/skills/bash-pwsh-invocation.md @@ -10,6 +10,10 @@ When calling a `.ps1` script from bash using `pwsh -File` or `powershell -File`, **always use explicit named parameters** (e.g. `-Paths "${ARR[@]}"`). +PowerShell `-File` targets must be `.ps1` scripts. Extensionless git hook +launchers such as `.githooks/pre-commit` run directly through Git/shell; for +PowerShell debugging, invoke `.githooks/pre-commit.ps1`. + **NEVER** use the POSIX `--` end-of-options separator: ```bash @@ -21,9 +25,9 @@ pwsh -NoProfile -File scripts/lint-foo.ps1 -- "${FILES[@]}" pwsh -NoProfile -File scripts/lint-foo.ps1 -Paths "${FILES[@]}" ``` -This rule is enforced by: +These rules are enforced by: -1. `scripts/lint-pwsh-invocations.ps1` — scans `*.sh`, `.githooks/*`, `.github/workflows/*.yml`, `scripts/tests/*.ps1`, and `package.json` for `-File