fix(maestro): decide flow failure from status and success, not error_messages (1.4.1)

CHANGELOG.md

@@ -2,6 +2,12 @@
 
 All notable changes to `@testingbot/cli` are documented here. Releases are published to npm from GitHub releases.
 
+## 1.4.1 - 2026-10-01
+
+### Fixed
+
+- A Maestro flow that passed (`status: DONE`, `success: 1`) no longer counts as failed just because `error_messages` is non-empty. Maestro's log4j sometimes writes "Unable to write to stream ... maestro.log" to stderr at shutdown, and that turned passing runs into exit code 2. Pass/fail now comes from `status` and `success`, and cancelled flows and runs still fail.
+
 ## 1.4.0 - 2026-09-05
 
 ### Added

package.json

@@ -1,6 +1,6 @@
 {
   "name": "@testingbot/cli",
-  "version": "1.4.0",
+  "version": "1.4.1",
   "description": "CLI tool to run Espresso, XCUITest and Maestro tests on TestingBot's cloud infrastructure",
   "main": "dist/index.js",
   "bin": {

src/providers/maestro.ts

@@ -2647,11 +2647,16 @@ export default class Maestro extends BaseProvider<MaestroOptions> {
       : text;
   }
 
+  /**
+   * Decided by status and success only. error_messages also carries stderr
+   * noise from flows that passed (e.g. Maestro's log4j complaining at
+   * shutdown), so its presence alone does not mean the flow failed.
+   */
   private isFlowFailed(flow: MaestroFlowInfo): boolean {
     return (
       (flow.status === 'DONE' && flow.success !== 1) ||
       flow.status === 'FAILED' ||
-      (flow.error_messages != null && flow.error_messages.length > 0)
+      flow.status === 'CANCELLED'
     );
   }
 
@@ -2670,6 +2675,8 @@ export default class Maestro extends BaseProvider<MaestroOptions> {
 
   /** A run passes if every logical flow's latest attempt passed. */
   private runPassed(run: MaestroRunInfo): boolean {
+    // A flow can finish with success 1 after its run was cancelled.
+    if (run.status === 'CANCELLED') return false;
     const groups = this.groupLatest(run.flows ?? []);
     if (groups.length === 0) return run.success === 1;
     return groups.every((flow) => !this.isFlowFailed(flow));
@@ -3137,7 +3144,9 @@ export default class Maestro extends BaseProvider<MaestroOptions> {
       for (const flow of flows.slice().sort((a, b) => a.id - b.id)) {
         const display = this.getFlowStatusDisplay(flow);
         const errors =
-          flow.error_messages && flow.error_messages.length > 0
+          this.isFlowFailed(flow) &&
+          flow.error_messages &&
+          flow.error_messages.length > 0
             ? pc.red(`  ${flow.error_messages[0]}`)
             : '';
         console.log(
@@ -3247,12 +3256,7 @@ export default class Maestro extends BaseProvider<MaestroOptions> {
   }
 
   private hasAnyFlowFailed(flows: MaestroFlowInfo[]): boolean {
-    return flows.some(
-      (flow) =>
-        (flow.status === 'DONE' && flow.success !== 1) ||
-        flow.status === 'FAILED' ||
-        (flow.error_messages && flow.error_messages.length > 0),
-    );
+    return flows.some((flow) => this.isFlowFailed(flow));
   }
 
   private calculateFlowDuration(flow: MaestroFlowInfo): string {

tests/providers/maestro.test.ts

@@ -5277,7 +5277,7 @@ flows:
       errorSpy.mockRestore();
     });
 
-    it('should treat flows with error_messages as failed even if success is 1', async () => {
+    it('should not fail a flow with success 1 just because error_messages is set', async () => {
       const consoleSpy = jest.spyOn(console, 'log').mockImplementation();
       const errorSpy = jest.spyOn(logger, 'error').mockImplementation();
 
@@ -5288,27 +5288,30 @@ flows:
               id: 5678,
               status: 'DONE',
               capabilities: { deviceName: 'Pixel 9', platformName: 'Android' },
-              success: 0,
+              success: 1,
               flows: [
                 {
                   id: 1,
-                  name: 'flaky.yaml',
+                  name: 'login.yaml',
                   status: 'DONE',
                   success: 1,
-                  error_messages: ['Element not found'],
+                  error_messages: [
+                    '2026-10-01T07:37:29.468224744Z Thread-5 ERROR Unable to write to stream /home/testingbot/.maestro/tests/2026-10-01_072332/maestro.log for appender File\n',
+                  ],
                 },
               ],
             },
           ],
-          success: false,
+          success: true,
           completed: true,
         },
       };
       axios.get = jest.fn().mockResolvedValue(responseFlowWithErrors);
 
-      await maestro['waitForCompletion']();
+      const result = await maestro['waitForCompletion']();
 
-      expect(errorSpy).toHaveBeenCalledWith('1 flow(s) failed across 1 run(s)');
+      expect(result.success).toBe(true);
+      expect(errorSpy).not.toHaveBeenCalled();
 
       consoleSpy.mockRestore();
       errorSpy.mockRestore();
@@ -5394,14 +5397,15 @@ flows:
       expect(result).toBe(true);
     });
 
-    it('should return true when a flow has error_messages', () => {
+    it('should return true when a flow was cancelled', () => {
       const flows: MaestroFlowInfo[] = [
         { id: 1, name: 'flow1.yaml', status: 'DONE', success: 1 },
         {
           id: 2,
           name: 'flow2.yaml',
-          status: 'READY',
-          error_messages: ['Error occurred'],
+          status: 'CANCELLED',
+          success: 0,
+          error_messages: ['Cancelled by user'],
         },
       ];
 
@@ -5410,6 +5414,22 @@ flows:
       expect(result).toBe(true);
     });
 
+    it('should return false when a passed flow has error_messages', () => {
+      const flows: MaestroFlowInfo[] = [
+        {
+          id: 1,
+          name: 'flow1.yaml',
+          status: 'DONE',
+          success: 1,
+          error_messages: ['Thread-5 ERROR Unable to write to stream'],
+        },
+      ];
+
+      const result = maestro['hasAnyFlowFailed'](flows);
+
+      expect(result).toBe(false);
+    });
+
     it('should return false when all flows passed', () => {
       const flows: MaestroFlowInfo[] = [
         { id: 1, name: 'flow1.yaml', status: 'DONE', success: 1 },
@@ -6914,6 +6934,26 @@ onFlowStart:
       );
     });
 
+    it('status() reports a cancelled run as failed even if its flow finished with success 1', async () => {
+      const flows = [
+        {
+          id: 1,
+          name: 'login',
+          status: 'DONE',
+          success: 1,
+          error_messages: ['Cancelled by user'],
+        },
+      ];
+      maestro['getStatus'] = jest.fn().mockResolvedValue({
+        runs: [run({ status: 'CANCELLED', success: 0, flows })],
+        success: false,
+        completed: true,
+      });
+      const result = await maestro.status(1234);
+      expect(result.outcome).toBe('failed');
+      expect(result.success).toBe(false);
+    });
+
     it('status() reports failed using last-attempt-wins', async () => {
       const flows = [
         { id: 1, name: 'login', status: 'DONE', success: 1 },