diff --git a/.github/workflows/e2e-bridge.yml b/.github/workflows/e2e-bridge.yml index c67c77a11..4cb9d7018 100644 --- a/.github/workflows/e2e-bridge.yml +++ b/.github/workflows/e2e-bridge.yml @@ -28,9 +28,18 @@ env: UNITY_IMAGE: unityci/editor:ubuntu-2021.3.45f2-linux-il2cpp-3 jobs: - e2e-bridge: - runs-on: ubuntu-24.04 - timeout-minutes: 40 + license: + # Hoisted out of the smoke job so the gate is a JOB-level condition. A step-level `if:` + # produces step-conclusion `skipped`, which contributes nothing to the job conclusion, + # so this workflow reported a green check having booted no Editor and exercised no tool + # call. A job skipped by a job-level `if:` reports "Skipped" in the merge box instead. + # Branch protection treats a skipped required check as satisfied, so merges are not blocked. + name: Detect Unity license secrets + runs-on: ubuntu-latest + permissions: + contents: read + outputs: + unity_ok: ${{ steps.detect.outputs.unity_ok }} steps: - name: Detect Unity license secrets id: detect @@ -46,30 +55,34 @@ jobs: else echo "unity_ok=false" >> "$GITHUB_OUTPUT" echo "::warning::E2E bridge smoke SKIPPED - no license secrets in scope (normal for fork PRs). This check is NOT a pass: nothing was booted or exercised." - # Every step below is gated on unity_ok, so the job reports a green check - # having run nothing at all. Say so plainly on the run page. + # The dependent job is gated on this output, so it reports Skipped rather + # than a green check. Still say plainly on the run page what did not happen. { echo "## :warning: E2E bridge smoke was SKIPPED" echo echo "No Unity license secrets were in scope, so **no Editor was booted and no tool call was exercised**." - echo "The green check means the job exited cleanly - **not** that the bridge works." + echo "The dependent job reports **Skipped** - it did not pass, it did not run." echo echo "GitHub withholds repository secrets from workflow runs triggered by a fork's pull request." } >> "$GITHUB_STEP_SUMMARY" fi + + e2e-bridge: + runs-on: ubuntu-24.04 + needs: license + if: needs.license.outputs.unity_ok == 'true' + timeout-minutes: 40 + steps: - uses: actions/checkout@v4 - if: steps.detect.outputs.unity_ok == 'true' with: fetch-depth: 0 - uses: astral-sh/setup-uv@v4 - if: steps.detect.outputs.unity_ok == 'true' with: python-version: "3.11" - name: Install MCP server - if: steps.detect.outputs.unity_ok == 'true' run: | set -eux uv venv @@ -79,7 +92,6 @@ jobs: # --- License staging (mirrors claude-nl-suite.yml) --- - name: Decide license sources - if: steps.detect.outputs.unity_ok == 'true' id: lic shell: bash env: @@ -96,7 +108,7 @@ jobs: echo "use_ebl=$use_ebl" >> "$GITHUB_OUTPUT" - name: Stage Unity .ulf license (from secret) - if: steps.detect.outputs.unity_ok == 'true' && steps.lic.outputs.use_ulf == 'true' + if: steps.lic.outputs.use_ulf == 'true' id: ulf env: UNITY_LICENSE: ${{ secrets.UNITY_LICENSE }} @@ -119,7 +131,7 @@ jobs: fi - name: Activate Unity (EBL via container) - if: steps.detect.outputs.unity_ok == 'true' && steps.lic.outputs.use_ebl == 'true' + if: steps.lic.outputs.use_ebl == 'true' shell: bash env: UNITY_IMAGE: ${{ env.UNITY_IMAGE }} @@ -140,7 +152,6 @@ jobs: ' - name: Warm up project (import Library once) - if: steps.detect.outputs.unity_ok == 'true' shell: bash env: UNITY_IMAGE: ${{ env.UNITY_IMAGE }} @@ -162,14 +173,12 @@ jobs: "${manual_args[@]}" -quit - name: Clean old MCP status - if: steps.detect.outputs.unity_ok == 'true' run: | set -eux mkdir -p "$GITHUB_WORKSPACE/.unity-mcp" rm -f "$GITHUB_WORKSPACE/.unity-mcp"/unity-mcp-status-*.json || true - name: Run headless bridge harness (boot + wait + smoke/editmode/playmode) - if: steps.detect.outputs.unity_ok == 'true' shell: bash env: UNITY_IMAGE: ${{ env.UNITY_IMAGE }} @@ -192,11 +201,11 @@ jobs: "${license_args[@]}" - name: Unity logs on failure - if: failure() && steps.detect.outputs.unity_ok == 'true' + if: failure() run: docker logs unity-mcp --tail 200 | sed -E 's/((email|serial|license|password|token)[^[:space:]]*)/[REDACTED]/Ig' || true - name: Upload E2E report - if: always() && steps.detect.outputs.unity_ok == 'true' + if: always() uses: actions/upload-artifact@v4 with: name: e2e-bridge-report diff --git a/.github/workflows/unity-tests.yml b/.github/workflows/unity-tests.yml index 9a5462c15..68a296271 100644 --- a/.github/workflows/unity-tests.yml +++ b/.github/workflows/unity-tests.yml @@ -24,9 +24,9 @@ on: - .github/workflows/unity-tests.yml # Same-repo PRs get a unity-tests status check on every open / push via this trigger # (mirrors python-tests.yml). Fork PRs ALSO fire this trigger but run in the fork's - # context without secrets — the detect step downstream writes unity_ok=false and the - # job reports a green check having compiled and tested nothing. That skip is stated - # loudly in the job's step summary so it is never mistaken for a pass. + # context without secrets — the `license` gate job writes unity_ok=false and the test + # job is skipped by its job-level `if:`, so the merge box shows "Skipped" rather than a + # green check. The reason is also written to the gate job's step summary. # # There is deliberately no pull_request_target trigger here. Running fork-authored # C# through game-ci/unity-test-runner with UNITY_* secrets in scope is the classic @@ -88,28 +88,20 @@ jobs: fi echo "versions=$versions" >> "$GITHUB_OUTPUT" - testAllModes: - name: Test in ${{ matrix.testMode }} on Unity ${{ matrix.unityVersion }} - needs: matrix + license: + # Hoisted out of testAllModes so the gate is a JOB-level condition. A step-level `if:` + # produces step-conclusion `skipped`, which contributes nothing to the job conclusion, + # so the whole suite reported a green check having compiled and tested nothing. A job + # skipped by a job-level `if:` reports "Skipped" in the merge box instead, which is + # what a reviewer actually reads. Branch protection still treats a skipped required + # check as satisfied, so this does not block merges. + name: Detect Unity license secrets runs-on: ubuntu-latest permissions: contents: read - strategy: - fail-fast: false - matrix: - projectPath: - - TestProjects/UnityMCPTests - testMode: - - editmode - unityVersion: ${{ fromJson(needs.matrix.outputs.versions) }} + outputs: + unity_ok: ${{ steps.detect.outputs.unity_ok }} steps: - - name: Checkout repository - uses: actions/checkout@v4 - with: - lfs: true - ref: ${{ inputs.ref || github.event.pull_request.head.sha || github.ref }} - persist-credentials: false - - name: Detect Unity license secrets id: detect env: @@ -125,24 +117,46 @@ jobs: echo "unity_ok=false" >> "$GITHUB_OUTPUT" fi - # A skipped run and a real pass both report a green check, because step-level - # `if:` conditions produce step-conclusion `skipped`, which contributes nothing - # to the job conclusion. Make the difference unmissable on the run page so a - # reviewer never reads this green check as "the code compiled". - - name: Skip Unity tests (missing license secrets) + - name: Explain why the Unity suite will not run if: steps.detect.outputs.unity_ok != 'true' run: | - echo "::warning::Unity tests SKIPPED - no license secrets in scope (normal for fork PRs). This check is NOT a pass: nothing was compiled or tested." + echo "::warning::Unity tests will be SKIPPED - no license secrets in scope (normal for fork PRs). Nothing will be compiled or tested." { - echo "## :warning: Unity tests were SKIPPED" + echo "## :warning: Unity tests will be SKIPPED" echo - echo "No Unity license secrets were in scope for this run, so **no C# was compiled and no test was executed**." - echo "The green check means the job exited cleanly - **not** that this code works." + echo "No Unity license secrets were in scope for this run, so **no C# will be compiled and no test will be executed**." + echo "The dependent job reports **Skipped**, not a pass." echo echo "GitHub withholds repository secrets from workflow runs triggered by a fork's pull request." - echo "To get real signal, a maintainer must run the suite against this code from a trusted context." + echo "To get real signal, a maintainer must run the suite against this code from a trusted context -" + echo "push the reviewed fork branch into this repo and the \`push\` trigger runs the full suite." + echo + echo "\`Compile MCPForUnity (win/osx/linux)\` and \`Run Python Tests\` DO run on fork PRs and are real signal." } >> "$GITHUB_STEP_SUMMARY" + testAllModes: + name: Test in ${{ matrix.testMode }} on Unity ${{ matrix.unityVersion }} + needs: [matrix, license] + if: needs.license.outputs.unity_ok == 'true' + runs-on: ubuntu-latest + permissions: + contents: read + strategy: + fail-fast: false + matrix: + projectPath: + - TestProjects/UnityMCPTests + testMode: + - editmode + unityVersion: ${{ fromJson(needs.matrix.outputs.versions) }} + steps: + - name: Checkout repository + uses: actions/checkout@v4 + with: + lfs: true + ref: ${{ inputs.ref || github.event.pull_request.head.sha || github.ref }} + persist-credentials: false + - uses: actions/cache@v4 with: path: ${{ matrix.projectPath }}/Library @@ -153,7 +167,6 @@ jobs: # Run domain reload tests first (they're [Explicit] so need explicit category) - name: Run domain reload tests - if: steps.detect.outputs.unity_ok == 'true' uses: game-ci/unity-test-runner@v4 id: domain-tests env: @@ -168,7 +181,6 @@ jobs: customParameters: -testCategory domain_reload - name: Run tests - if: steps.detect.outputs.unity_ok == 'true' uses: game-ci/unity-test-runner@v4 id: tests continue-on-error: true @@ -183,7 +195,6 @@ jobs: testMode: ${{ matrix.testMode }} - name: Check test results - if: steps.detect.outputs.unity_ok == 'true' env: ARTIFACTS_PATH: ${{ steps.tests.outputs.artifactsPath }} run: | @@ -238,7 +249,7 @@ jobs: PY - uses: actions/upload-artifact@v4 - if: always() && steps.detect.outputs.unity_ok == 'true' && steps.tests.outcome != 'skipped' + if: always() && steps.tests.outcome != 'skipped' with: name: Test results for ${{ matrix.testMode }} on Unity ${{ matrix.unityVersion }} path: ${{ steps.tests.outputs.artifactsPath }} diff --git a/website/docs/contributing/dev-setup.md b/website/docs/contributing/dev-setup.md index 850ad6aaa..fa6d11665 100644 --- a/website/docs/contributing/dev-setup.md +++ b/website/docs/contributing/dev-setup.md @@ -246,16 +246,28 @@ For compatibility PRs, note the exact editor versions you tested in the PR body. CI exercises the package across multiple Unity versions to catch breaks in `#if UNITY_*_OR_NEWER` branches. The matrix is configured in `tools/unity-versions.json` and consumed by `.github/workflows/unity-tests.yml`. -**Every PR gets a unity-tests status check on open** (mirrors `python-tests.yml`). For same-repo PRs the default Unity 6 leg actually runs; for fork PRs the workflow appears but skips with a "missing license secrets" notice until a maintainer applies `safe-to-test` (existing secret-safety gate). The full 4-version matrix is opt-in via the `full-matrix` label. +**Every Unity-scoped PR gets a unity-tests status check on open** (mirrors `python-tests.yml`). The `pull_request` trigger is path-filtered to `MCPForUnity/Editor/**`, `MCPForUnity/Runtime/**`, `TestProjects/UnityMCPTests/**` and the workflow itself, so a PR touching only `Server/**` or docs never creates the check. For same-repo PRs the default Unity 6 leg actually runs. The full 4-version matrix is opt-in via the `full-matrix` label. + +### Which checks are real signal on a fork PR + +GitHub withholds repository secrets from workflow runs triggered by a fork's pull request, and both Unity workflows need a license. They report **Skipped**, not a pass: + +| Check | On a fork PR | +|---|---| +| `Compile MCPForUnity (win/osx/linux)` | **Runs.** License-free compile across win/osx/linux — real signal. | +| `Run Python Tests` | **Runs.** Real signal. | +| `Check docs reference is fresh` | **Runs.** Real signal. | +| `Test in editmode on Unity ` | **Skipped** — no Editor booted, no C# compiled by this job. | +| `e2e-bridge` | **Skipped** — no Editor booted, no tool call exercised. | + +If you need a real Unity run against fork code, ask a maintainer: once the diff is reviewed, pushing the branch into this repo makes the `push` trigger run the full suite in a trusted context. There is deliberately no `pull_request_target` trigger for the Unity workflows — running fork-authored C# with `UNITY_*` secrets in scope is the classic "pwn request" shape, since an `[InitializeOnLoad]` script in the PR would be enough to read them. **When the full matrix runs (all 4 versions in parallel):** - Push to `beta` (the release gate). - `workflow_call` from `beta-release.yml` / `release.yml`. - Manual `workflow_dispatch` from the Actions tab. -- Any PR (in-repo or fork) labeled with **`full-matrix`** — apply when your change touches compat shims, conditional compilation, or anything else version-sensitive. Triggers a full-matrix run on the next `pull_request` or `pull_request_target` event. Cost is ~6-8 min wall clock vs ~3 min for the default leg. - -Fork PRs still need `safe-to-test` as the base gate (so secrets are exposed against reviewed-only fork code); `full-matrix` is layered on top for fork-PR full-matrix runs. +- Any PR (in-repo or fork) labeled with **`full-matrix`** — apply when your change touches compat shims, conditional compilation, or anything else version-sensitive. Triggers a full-matrix run on the next `pull_request` event (the label is read when the workflow fires, so applying it to an open PR takes effect on the next push). Cost is ~6-8 min wall clock vs ~3 min for the default leg. **Default (single leg)** — every other path runs only against `defaultVersion` from `tools/unity-versions.json` (currently Unity 6.0 LTS, `6000.0.75f1`). The `floor` role (`2021.3.45f2`) still identifies the package minimum and runs as part of the full matrix; it's no longer the default-leg version.