Skip to content

Commit c298216

Browse files
dsarnoclaude
andauthored
fix: unblock beta compile and gate releases on test success (#1103)
* fix: align CaptureComposited with renamed Project*-folder API PR #1040 added CaptureComposited referencing the old Assets-folder names (CaptureFromCameraToAssetsFolder, AssetsRelativePath) and called PrepareCaptureResult without the now-required folderOverride argument. Renames into the Project* equivalents; same fix at the call sites in ManageScene.cs. Closes #1100 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * ci: gate releases on test success and trigger tests on PRs Beta-release was publishing to PyPI in parallel with Unity Tests, so broken commits could ship if Unity tests failed (as happened with #1100 on 9.6.9-beta.5). Make publish/version-bump jobs depend on the test jobs in both beta-release.yml and release.yml. The whole release halts before any irreversible commit/tag/push if either test job fails. Also extends the test workflows to fire on PRs so failures are caught before merge: - python-tests: pull_request trigger; runs on every PR (no secrets). - unity-tests: pull_request_target [labeled] trigger gated on the safe-to-test label and on the PR being from a fork. Maintainers apply the label after reviewing the diff; the workflow then runs with UNITY_LICENSE in scope against the PR head SHA. Re-pushed commits do NOT auto-trigger; maintainer must remove and re-apply the label to re-run after additional review. In-repo PRs continue to be tested via the existing push trigger, so no labeling friction for collaborator branches. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(screenshot): propagate folderOverride to composited and specific-camera paths Two pre-existing inconsistencies surfaced by CodeRabbit on #1103: 1. CaptureComposited dropped the caller's output_folder by hardcoding folderOverride: null in PrepareCaptureResult and the camera fallbacks. Adds the parameter to CaptureComposited's signature and plumbs it through both fallback paths. 2. The targetCamera and includeImage-in-play paths in ManageScene's game_view screenshot did not resolve cmd.outputFolder, so a request that selected a specific camera would always write to the default folder. Resolve via ScreenshotPreferences.Resolve as the other paths already do, and gate AssetDatabase.ImportAsset on IsUnderAssets so non-Assets folders don't trigger a futile import. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * ci(unity-tests): harden pull_request_target and gate artifact upload Address CodeRabbit security review on #1103: - persist-credentials: false on the checkout step, so GITHUB_TOKEN is not written to disk and cannot be read by subsequent steps running PR-controlled code. - Explicit permissions: contents: read on the testAllModes job, scoping the workflow's token down from the default read/write set. - Skip upload-artifact when the main test step was skipped (e.g., because the preceding domain-reload step failed without continue-on-error). Avoids the noisy "No files were found" error on top of an already-failed run. The label-gated trigger plus these mitigations narrow the blast radius of the pull_request_target + checkout-PR-head pattern. The remaining trust boundary is the maintainer review before applying safe-to-test; documenting the review checklist (especially TestProjects/UnityMCPTests diffs) is a follow-up. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent 37ef016 commit c298216

6 files changed

Lines changed: 90 additions & 14 deletions

File tree

.github/workflows/beta-release.yml

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,9 +13,21 @@ on:
1313
- "MCPForUnity/**"
1414

1515
jobs:
16+
unity_tests:
17+
name: Unity tests gate
18+
if: github.actor != 'github-actions[bot]'
19+
uses: ./.github/workflows/unity-tests.yml
20+
secrets: inherit
21+
22+
python_tests:
23+
name: Python tests gate
24+
if: github.actor != 'github-actions[bot]'
25+
uses: ./.github/workflows/python-tests.yml
26+
1627
update_unity_beta_version:
1728
name: Update Unity package to beta version
1829
runs-on: ubuntu-latest
30+
needs: [unity_tests, python_tests]
1931
# Avoid running when the workflow's own automation merges the PR
2032
# created by this workflow (prevents a version-bump loop).
2133
if: github.actor != 'github-actions[bot]'
@@ -143,6 +155,7 @@ jobs:
143155
publish_pypi_prerelease:
144156
name: Publish beta to PyPI (pre-release)
145157
runs-on: ubuntu-latest
158+
needs: [unity_tests, python_tests]
146159
# Avoid double-publish when the bot merges the version bump PR
147160
if: github.actor != 'github-actions[bot]'
148161
environment:

.github/workflows/python-tests.yml

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,19 @@ on:
66
paths:
77
- Server/**
88
- .github/workflows/python-tests.yml
9+
pull_request:
10+
branches: [main, beta]
11+
paths:
12+
- Server/**
13+
- .github/workflows/python-tests.yml
914
workflow_dispatch: {}
15+
workflow_call:
16+
inputs:
17+
ref:
18+
description: "Git ref to test (defaults to the triggering ref)."
19+
type: string
20+
required: false
21+
default: ""
1022

1123
jobs:
1224
test:
@@ -15,6 +27,8 @@ jobs:
1527
steps:
1628
- name: Checkout repository
1729
uses: actions/checkout@v4
30+
with:
31+
ref: ${{ inputs.ref || github.ref }}
1832

1933
- name: Install uv
2034
uses: astral-sh/setup-uv@v4

.github/workflows/release.yml

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,9 +19,23 @@ on:
1919
required: true
2020

2121
jobs:
22+
unity_tests:
23+
name: Unity tests gate
24+
uses: ./.github/workflows/unity-tests.yml
25+
with:
26+
ref: beta
27+
secrets: inherit
28+
29+
python_tests:
30+
name: Python tests gate
31+
uses: ./.github/workflows/python-tests.yml
32+
with:
33+
ref: beta
34+
2235
bump:
2336
name: Bump version, tag, and create release
2437
runs-on: ubuntu-latest
38+
needs: [unity_tests, python_tests]
2539
permissions:
2640
contents: write
2741
pull-requests: write

.github/workflows/unity-tests.yml

Lines changed: 27 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,17 +2,41 @@ name: Unity Tests
22

33
on:
44
workflow_dispatch: {}
5+
workflow_call:
6+
inputs:
7+
ref:
8+
description: "Git ref to test (defaults to the triggering ref)."
9+
type: string
10+
required: false
11+
default: ""
512
push:
613
branches: ["**"]
714
paths:
815
- TestProjects/UnityMCPTests/**
916
- MCPForUnity/Editor/**
1017
- .github/workflows/unity-tests.yml
18+
# Fork PRs: maintainer applies the 'safe-to-test' label after reviewing
19+
# the diff. The workflow runs with UNITY_LICENSE in scope against the
20+
# PR's head SHA. Re-pushed commits do NOT auto-trigger — maintainer must
21+
# remove and re-apply the label to re-run after additional review.
22+
pull_request_target:
23+
types: [labeled]
24+
branches: [main, beta]
25+
paths:
26+
- TestProjects/UnityMCPTests/**
27+
- MCPForUnity/Editor/**
28+
- .github/workflows/unity-tests.yml
1129

1230
jobs:
1331
testAllModes:
1432
name: Test in ${{ matrix.testMode }}
1533
runs-on: ubuntu-latest
34+
permissions:
35+
contents: read
36+
if: >
37+
github.event_name != 'pull_request_target' ||
38+
(github.event.pull_request.head.repo.full_name != github.repository &&
39+
github.event.label.name == 'safe-to-test')
1640
strategy:
1741
fail-fast: false
1842
matrix:
@@ -27,6 +51,8 @@ jobs:
2751
uses: actions/checkout@v4
2852
with:
2953
lfs: true
54+
ref: ${{ inputs.ref || github.event.pull_request.head.sha || github.ref }}
55+
persist-credentials: false
3056

3157
- name: Detect Unity license secrets
3258
id: detect
@@ -107,7 +133,7 @@ jobs:
107133
fi
108134
109135
- uses: actions/upload-artifact@v4
110-
if: always() && steps.detect.outputs.unity_ok == 'true'
136+
if: always() && steps.detect.outputs.unity_ok == 'true' && steps.tests.outcome != 'skipped'
111137
with:
112138
name: Test results for ${{ matrix.testMode }}
113139
path: ${{ steps.tests.outputs.artifactsPath }}

MCPForUnity/Editor/Tools/ManageScene.cs

Lines changed: 14 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -598,26 +598,32 @@ private static object CaptureScreenshot(SceneCommand cmd)
598598
{
599599
if (!Application.isBatchMode) EnsureGameView();
600600

601-
ScreenshotCaptureResult result = ScreenshotUtility.CaptureFromCameraToAssetsFolder(
601+
string folderOverride = ScreenshotPreferences.Resolve(cmd.outputFolder);
602+
ScreenshotCaptureResult result = ScreenshotUtility.CaptureFromCameraToProjectFolder(
602603
targetCamera, fileName, resolvedSuperSize, ensureUniqueFileName: true,
603-
includeImage: includeImage, maxResolution: maxResolution);
604+
includeImage: includeImage, maxResolution: maxResolution,
605+
folderOverride: folderOverride);
604606

605-
AssetDatabase.ImportAsset(result.AssetsRelativePath, ImportAssetOptions.ForceSynchronousImport);
606-
string message = $"Screenshot captured to '{result.AssetsRelativePath}' (camera: {targetCamera.name}).";
607+
if (ScreenshotUtility.IsUnderAssets(result.ProjectRelativePath))
608+
AssetDatabase.ImportAsset(result.ProjectRelativePath, ImportAssetOptions.ForceSynchronousImport);
609+
string message = $"Screenshot captured to '{result.ProjectRelativePath}' (camera: {targetCamera.name}).";
607610
return new SuccessResponse(message, BuildScreenshotResponseData(result, targetCamera.name, includeImage));
608611
}
609612

610613
if (includeImage && Application.isPlaying)
611614
{
612615
if (!Application.isBatchMode) EnsureGameView();
613616

617+
string folderOverride = ScreenshotPreferences.Resolve(cmd.outputFolder);
614618
ScreenshotCaptureResult result = ScreenshotUtility.CaptureComposited(
615619
fileName, resolvedSuperSize, ensureUniqueFileName: true,
616-
includeImage: true, maxResolution: maxResolution);
620+
includeImage: true, maxResolution: maxResolution,
621+
folderOverride: folderOverride);
617622

618-
AssetDatabase.ImportAsset(result.AssetsRelativePath, ImportAssetOptions.ForceSynchronousImport);
623+
if (ScreenshotUtility.IsUnderAssets(result.ProjectRelativePath))
624+
AssetDatabase.ImportAsset(result.ProjectRelativePath, ImportAssetOptions.ForceSynchronousImport);
619625
string cameraName = Camera.main != null ? Camera.main.name : "composited";
620-
string message = $"Screenshot captured to '{result.AssetsRelativePath}' (camera: {cameraName}).";
626+
string message = $"Screenshot captured to '{result.ProjectRelativePath}' (camera: {cameraName}).";
621627
return new SuccessResponse(message, BuildScreenshotResponseData(result, cameraName, includeImage: true));
622628
}
623629

@@ -747,7 +753,7 @@ private static Dictionary<string, object> BuildScreenshotResponseData(
747753
{
748754
var data = new Dictionary<string, object>
749755
{
750-
{ "path", result.AssetsRelativePath },
756+
{ "path", result.ProjectRelativePath },
751757
{ "fullPath", result.FullPath },
752758
{ "superSize", result.SuperSize },
753759
{ "isAsync", false },

MCPForUnity/Runtime/Helpers/ScreenshotUtility.cs

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -246,18 +246,20 @@ public static ScreenshotCaptureResult CaptureComposited(
246246
int superSize = 1,
247247
bool ensureUniqueFileName = true,
248248
bool includeImage = false,
249-
int maxResolution = 0)
249+
int maxResolution = 0,
250+
string folderOverride = null)
250251
{
251252
if (!IsScreenCaptureModuleAvailable)
252253
{
253254
var fallbackCamera = FindAvailableCamera();
254255
if (fallbackCamera != null)
255-
return CaptureFromCameraToAssetsFolder(fallbackCamera, fileName, superSize, ensureUniqueFileName, includeImage, maxResolution);
256+
return CaptureFromCameraToProjectFolder(fallbackCamera, fileName, superSize, ensureUniqueFileName,
257+
includeImage, maxResolution, folderOverride: folderOverride);
256258

257259
throw new InvalidOperationException("ScreenCapture module is unavailable and no fallback camera found.");
258260
}
259261

260-
ScreenshotCaptureResult result = PrepareCaptureResult(fileName, superSize, ensureUniqueFileName, isAsync: false);
262+
ScreenshotCaptureResult result = PrepareCaptureResult(fileName, superSize, ensureUniqueFileName, folderOverride: folderOverride, isAsync: false);
261263
Texture2D tex = null;
262264
Texture2D downscaled = null;
263265
string imageBase64 = null;
@@ -270,7 +272,8 @@ public static ScreenshotCaptureResult CaptureComposited(
270272
// Fallback to camera-based if ScreenCapture fails
271273
var cam = FindAvailableCamera();
272274
if (cam != null)
273-
return CaptureFromCameraToAssetsFolder(cam, fileName, superSize, ensureUniqueFileName, includeImage, maxResolution);
275+
return CaptureFromCameraToProjectFolder(cam, fileName, superSize, ensureUniqueFileName,
276+
includeImage, maxResolution, folderOverride: folderOverride);
274277
throw new InvalidOperationException("ScreenCapture.CaptureScreenshotAsTexture returned null and no fallback camera available.");
275278
}
276279

@@ -308,7 +311,7 @@ public static ScreenshotCaptureResult CaptureComposited(
308311
if (includeImage && imageBase64 != null)
309312
{
310313
return new ScreenshotCaptureResult(
311-
result.FullPath, result.AssetsRelativePath, result.SuperSize, false,
314+
result.FullPath, result.ProjectRelativePath, result.SuperSize, false,
312315
imageBase64, imgW, imgH);
313316
}
314317
return result;

0 commit comments

Comments
 (0)