fix: stop quoting docker's pull noise as the cause of a Unity abort

.github/workflows/tests.yml

@@ -190,6 +190,18 @@ jobs:
         # lands in that window - e.g. this repo's own version-bump commit,
         # created right after cutting the release it's for - hit exactly
         # this 404 in practice, failing Tests on main itself, not just a PR.
+        #
+        # GH_TOKEN because install.sh resolves "latest" through the GitHub
+        # API and only authenticates when handed one (see its own comment).
+        # Unauthenticated is 60 requests/hour *per IP*, and Actions runners
+        # share IPs across unrelated repos and orgs - so the macOS job below,
+        # running the same install.sh against the same endpoint seconds
+        # later, was 403ing on that limit while this job passed beside it.
+        # install.sh reads GITHUB_TOKEN or GH_TOKEN and treats an empty one
+        # as no token, so a fork PR (no secrets, read-only github.token)
+        # degrades to today's unauthenticated behaviour rather than failing.
+        env:
+          GH_TOKEN: ${{ secrets.GIT_PRIVATE_TOKEN || github.token }}
         run: |
           for attempt in 1 2 3 4 5; do
             # install.sh writes progress to stderr and only the final binary
@@ -203,7 +215,7 @@ jobs:
               echo "FAIL: scripts/install.sh still failing after 5 attempts"
               exit 1
             fi
-            echo "install.sh failed (attempt $attempt/5) - likely the latest release's binaries are still uploading - retrying in 30s..."
+            echo "install.sh failed (attempt $attempt/5) - see the error above; retrying in 30s..."
             sleep 30
           done
           [ -x "$binary_path" ] || { echo "FAIL: $binary_path is not executable"; exit 1; }
@@ -221,6 +233,10 @@ jobs:
       # above has already established that the latest release is fully
       # uploaded.
       - name: Smoke-test the root install.sh wrapper
+        # Same GH_TOKEN reason as the step above: the wrapper defaults to
+        # "latest" and resolves it through the same API call.
+        env:
+          GH_TOKEN: ${{ secrets.GIT_PRIVATE_TOKEN || github.token }}
         run: |
           export GAME_CI_INSTALL=/tmp/root-install-sh-smoke-test
           sh ./install.sh
@@ -257,8 +273,10 @@ jobs:
             *) echo "FAIL: expected macOS's system bash to be 3.x, got $bash_version - this job no longer covers the bash-3.2 compatibility class it exists for"; exit 1 ;;
           esac
       - name: Smoke-test scripts/install.sh against the latest release
-        # Same retry rationale as the ubuntu job: "latest" can resolve to a
-        # release whose binaries are still uploading.
+        # Same retry rationale, and the same GH_TOKEN reason, as the ubuntu
+        # job above - this is the job the token exists to stop 403ing.
+        env:
+          GH_TOKEN: ${{ secrets.GIT_PRIVATE_TOKEN || github.token }}
         run: |
           for attempt in 1 2 3 4 5; do
             if binary_path="$(/bin/bash scripts/install.sh latest /tmp/install-sh-smoke-test)"; then
@@ -268,13 +286,17 @@ jobs:
               echo "FAIL: scripts/install.sh still failing after 5 attempts"
               exit 1
             fi
-            echo "install.sh failed (attempt $attempt/5) - likely the latest release's binaries are still uploading - retrying in 30s..."
+            echo "install.sh failed (attempt $attempt/5) - see the error above; retrying in 30s..."
             sleep 30
           done
           [ -x "$binary_path" ] || { echo "FAIL: $binary_path is not executable"; exit 1; }
           [ -d "$(dirname "$binary_path")/dist" ] || { echo "FAIL: dist/ was not extracted next to $binary_path"; exit 1; }
           "$binary_path" --help
       - name: Smoke-test the root install.sh wrapper
+        # Same GH_TOKEN reason as the step above: the wrapper defaults to
+        # "latest" and resolves it through the same API call.
+        env:
+          GH_TOKEN: ${{ secrets.GIT_PRIVATE_TOKEN || github.token }}
         run: |
           export GAME_CI_INSTALL=/tmp/root-install-sh-smoke-test
           /bin/sh ./install.sh

src/model/docker.test.ts

@@ -105,8 +105,9 @@ describe("Docker", () => {
     });
     System.run = mock(() => Promise.reject(dockerError));
 
-    await expect(
-      Docker.run("game-ci/unity-editor-stub:latest", {
+    let rejection: Error | undefined;
+    try {
+      await Docker.run("game-ci/unity-editor-stub:latest", {
         hostOS: "linux",
         hostPlatform: "linux",
         currentWorkDir: "/home/runner/work/cli/cli",
@@ -117,8 +118,19 @@ describe("Docker", () => {
         dockerWorkspacePath: "/github/workspace",
         engine: "unity",
         runTests: true,
-      } as any),
-    ).rejects.toThrow(/Scripts have compiler errors\./);
+      } as any);
+    } catch (error: any) {
+      rejection = error;
+    }
+
+    expect(rejection?.message).toContain('Scripts have compiler errors.');
+
+    // And it must not be quoted back at the reader as the cause: System.run
+    // already streamed this stderr live, and leading with "Unable to find
+    // image ... locally" under an "Original error:" heading is exactly what
+    // made MirrorNetworking/Mirror#4128 read as an editor failing to load
+    // 6000.3.23f1 rather than scripts failing to compile.
+    expect(rejection?.message).not.toContain('Unable to find image');
   });
 
   it("still throws the original error when there is no Unity abort reason to extract", async () => {

src/model/docker.ts

@@ -157,7 +157,14 @@ class Docker {
       // failure this command hits, and error.message alone never contains
       // it - see UnityBatchmodeFailure's own comment for why. Checked first
       // since it's the highest-value case to get right.
-      const batchmodeFailure = UnityBatchmodeFailure.describe(error.stdout, error.message);
+      // No originalMessage: this command's error text is `docker run`'s own
+      // stderr - pull progress, led by its benign "Unable to find image ...
+      // locally" status line - which System.run has already streamed live
+      // above. Repeating it under an "Original error:" heading asserted it
+      // was the cause: MirrorNetworking/Mirror#4128 read as an editor failing
+      // to load 6000.3.23f1 when the real cause was scripts failing to
+      // compile, two lines up in the same message.
+      const batchmodeFailure = UnityBatchmodeFailure.describe(error.stdout);
       if (batchmodeFailure) {
         throw new Error(batchmodeFailure);
       }

src/model/unity/unity-batchmode-failure.test.ts

@@ -53,6 +53,18 @@ describe('UnityBatchmodeFailure', () => {
       expect(described).toContain('not a\ndocker/game-ci infrastructure problem');
     });
 
+    // Regression: Docker.run passes no originalMessage, because its error
+    // text is `docker run`'s stderr - pull progress it has already streamed
+    // live. Appending that under "Original error:" made
+    // MirrorNetworking/Mirror#4128 read as an editor failing to load a
+    // version when Unity had in fact aborted on script compiler errors.
+    it('omits the original-error section when the caller supplies nothing to add', () => {
+      const described = UnityBatchmodeFailure.describe(realWorldStdout);
+
+      expect(described).toContain('Scripts have compiler errors.');
+      expect(described).not.toContain('Original error:');
+    });
+
     it('returns undefined when there is nothing to extract, leaving the caller to fall back to the original error', () => {
       expect(UnityBatchmodeFailure.describe('some unrelated stdout', 'Command exited with code 1')).toBeUndefined();
       expect(UnityBatchmodeFailure.describe(undefined, 'Command exited with code 1')).toBeUndefined();

src/model/unity/unity-batchmode-failure.ts

@@ -13,6 +13,16 @@
  * run failed with exit code 1" - the real reason, "Scripts have compiler
  * errors.", was sitting ~1400 log lines earlier and never reached the final
  * error message at all.
+ *
+ * Reported live once more, from the opposite direction: the same CI then
+ * appended that stderr under the "Original error:" heading below, burying
+ * the reason it was supposed to support. Docker leads its stderr with the
+ * benign pre-pull "Unable to find image '...' locally" line, so the message
+ * read as an editor failing to load 6000.3.23f1 - two maintainers went
+ * looking for a version problem while "Scripts have compiler errors." sat
+ * two lines above it. Hence `originalMessage` is optional, and the one
+ * caller whose error text is already in the log - Docker, via System.run's
+ * live stderr passthrough - omits it.
  */
 class UnityBatchmodeFailure {
   static extractReason(stdout: string | undefined): string | undefined {
@@ -25,21 +35,30 @@ class UnityBatchmodeFailure {
     return match?.[1]?.trim() || undefined;
   }
 
-  /** Returns a clearer error message when `stdout` contains Unity's own abort reason, or undefined otherwise. */
-  static describe(stdout: string | undefined, originalMessage: string): string | undefined {
+  /**
+   * Returns a clearer error message when `stdout` contains Unity's own abort
+   * reason, or undefined otherwise.
+   *
+   * `originalMessage` is the caller's own error text, appended under an
+   * "Original error:" heading. Omit it when that text is already in the log
+   * above - the reason is the useful part, and repeating a stream the reader
+   * has already scrolled past only buries it.
+   */
+  static describe(stdout: string | undefined, originalMessage?: string): string | undefined {
     const reason = UnityBatchmodeFailure.extractReason(stdout);
     if (!reason) return undefined;
 
-    return String.dedent`
+    const message = String.dedent`
       Unity aborted before completing: ${reason}
 
       This is a failure inside the Unity Editor itself (commonly script
       compiler errors, a missing package, or a crash during startup) - not a
       docker/game-ci infrastructure problem. The full Unity log is above.
-
-      Original error:
-      ${originalMessage}
     `;
+
+    if (!originalMessage) return message;
+
+    return `${message}\n\nOriginal error:\n${originalMessage}`;
   }
 }