fix: System.run treated any stderr output as a fatal error

src/model/system/system.integration.test.ts

@@ -18,8 +18,16 @@ describe('System', () => {
           expect(result.output.trim()).toBe('test');
         });
 
-        test('throws when command writes to stderr', async () => {
-          await expect(System.run('echo fail >&2')).rejects.toThrow();
+        test('succeeds when a command writes to stderr but exits 0', async () => {
+          // Was 'throws when command writes to stderr' - codified the bug
+          // this fixes (game-ci/cli#84): docker writes informational
+          // messages to stderr on otherwise-successful runs, so stderr
+          // content alone can't mean failure. Exit code does.
+          await expect(System.run('echo fail >&2')).resolves.not.toBeNull();
+        });
+
+        test('throws when a command exits non-zero', async () => {
+          await expect(System.run('exit 1')).rejects.toThrow();
         });
       }
     });

src/model/system/system.test.ts

@@ -36,3 +36,34 @@ describe('System.run env option', () => {
     }
   });
 });
+
+describe('System.run exit-code-based failure', () => {
+  // Real bug (game-ci/unity-activate#111): this used to throw on any
+  // stderr output regardless of exit code. `docker run` writes "Unable to
+  // find image '...' locally" to stderr when auto-pulling, then succeeds
+  // with exit code 0 - which was being thrown as a fatal error anyway.
+  test('succeeds when stderr has output but the command exits 0', async () => {
+    const command = 'node -e "process.stderr.write(\'just a warning\\n\')"';
+
+    const result = await System.run(command, undefined, { silent: true });
+
+    expect(result.status?.success).toBe(true);
+    expect(result.error).toContain('just a warning');
+  });
+
+  test('throws when the command exits non-zero, even with empty stderr', async () => {
+    // Exit code propagation through the shell wrapper (sh -c vs
+    // powershell -Command) isn't consistent enough across platforms to
+    // assert a specific code here - what matters is that a non-zero exit
+    // with no stderr still throws, instead of silently resolving.
+    const command = 'node -e "process.exit(3)"';
+
+    await expect(System.run(command, undefined, { silent: true })).rejects.toThrow(/Command exited with code \d+/);
+  });
+
+  test('throws with the stderr content when the command exits non-zero', async () => {
+    const command = 'node -e "process.stderr.write(\'boom\\n\'); process.exit(1)"';
+
+    await expect(System.run(command, undefined, { silent: true })).rejects.toThrow('boom');
+  });
+});

src/model/system/system.ts

@@ -19,15 +19,17 @@ class System {
    * Run any command as if you're typing in shell.
    * Make sure it's Windows/MacOS/Ubuntu compatible or has alternative commands.
    *
-   * If any error is written to stderr, this method will throw them.
-   *   new Error(stdoutErrors)
+   * If the command exits with a non-zero code, this method throws (message
+   * built from stderr, falling back to a generic "exited with code N").
+   * A non-empty stderr alone is not treated as failure - many commands
+   * (e.g. `docker run` auto-pulling an uncached image) write informational
+   * output there on a genuinely successful run.
    *
-   * In case of no errors, this will return an object similar to these examples
+   * In case of success, this will return an object similar to these examples
    *   { status: { success: true, code: 0 }, output: 'output from the command' }
-   *   { status: { success: false, code: 1~255 }, output: 'output from the command' }
    *
-   * @returns {string} output of the command on success or failure
-   * @throws  {Error}  if anything was output to stderr or return code wasn't 0
+   * @returns {string} output of the command on success
+   * @throws  {Error}  if the command's exit code wasn't 0
    */
   static async run(command: string, windowsSpecificCommand?: string, options: RunOptions = { silent: false }): Promise<RunResult> {
     let shell: string;
@@ -81,13 +83,19 @@ class System {
         const exitCode = code ?? 1;
         runResult.status = { success: exitCode === 0, code: exitCode };
 
-        if (runResult.error !== '') {
-          // Make sure we don't swallow any output if silent and there is an error
+        if (exitCode !== 0) {
+          // Real bug (game-ci/unity-activate#111): this used to throw on
+          // *any* stderr output, regardless of exit code. `docker run` on
+          // an image not yet cached locally writes "Unable to find image
+          // '...' locally" to stderr as pure status output, then pulls it
+          // and succeeds with exit code 0 - which this treated as a fatal
+          // error anyway, discarding the successful run. Exit code is the
+          // actual signal; stderr content is still included below for
+          // debugging when the command genuinely failed.
           const errorMessage = runResult.output && options.silent
             ? `${runResult.error}\n\n---\n\nOutput before the error:\n${runResult.output}`
-            : runResult.error;
+            : runResult.error || `Command exited with code ${exitCode}`;
 
-          // Throw instead of returning when any output was written to stderr
           reject(new Error(errorMessage));
           return;
         }