fix(maestro): prefer terminal failure over transient retries in parseMaestroFailure (closes #118)

scripts/cdp-bridge/dist/domain/maestro-error-parser.js

@@ -73,15 +73,48 @@ const PATTERNS = [
     },
 ];
 /**
- * Parse the full Maestro stdout+stderr text and classify the first
- * failure found. Returns `UNKNOWN` if nothing matches a known pattern.
+ * Parse the full Maestro stdout+stderr text and classify the failure.
+ *
+ * Two-axis selection — pattern-specificity dominates, line-position breaks
+ * ties within a pattern:
+ *
+ *   1. Outer loop walks PATTERNS in order (most-specific first). The first
+ *      pattern that hits ANY line wins, regardless of where that line sits.
+ *      Preserves the existing invariant that the 1.0.9 `id=` shape outranks
+ *      the catch-all `Element 'X' not found`.
+ *
+ *   2. Inner loop scans lines from END to START. Within a single pattern,
+ *      the LAST matching line (the terminal failure) wins — earlier matches
+ *      are typically transient retries that maestro-runner reports as
+ *      `[INFO]` before the auto-retry succeeds. GH #118: PR #115's
+ *      first-match-anywhere scan captured a transient retry selector and
+ *      sent it to `cdp_repair_action`, wasting a 24h-budget slot.
+ *
+ * Falls back to a whole-buffer scan when no line matches a known pattern —
+ * preserves prior-art behavior for single-line inputs and any pattern that
+ * happens to span a line boundary (none today, but defensive).
+ *
+ * Returns `UNKNOWN` if no pattern matches at all.
  */
 export function parseMaestroFailure(output) {
     if (!output || typeof output !== 'string') {
         return { kind: 'UNKNOWN', raw: '' };
     }
+    const lines = output.split('\n');
     for (const { re, build } of PATTERNS) {
-        const m = re.exec(output);
+        for (let i = lines.length - 1; i >= 0; i--) {
+            const line = lines[i];
+            if (!line)
+                continue;
+            const m = line.match(re);
+            if (m)
+                return build(m, output);
+        }
+    }
+    // Fallback: whole-buffer scan for inputs without line breaks or for
+    // any pattern that happens to straddle a `\n`.
+    for (const { re, build } of PATTERNS) {
+        const m = output.match(re);
         if (m)
             return build(m, output);
     }

scripts/cdp-bridge/src/domain/maestro-error-parser.ts

@@ -86,16 +86,47 @@ const PATTERNS: Pattern[] = [
 ];
 
 /**
- * Parse the full Maestro stdout+stderr text and classify the first
- * failure found. Returns `UNKNOWN` if nothing matches a known pattern.
+ * Parse the full Maestro stdout+stderr text and classify the failure.
+ *
+ * Two-axis selection — pattern-specificity dominates, line-position breaks
+ * ties within a pattern:
+ *
+ *   1. Outer loop walks PATTERNS in order (most-specific first). The first
+ *      pattern that hits ANY line wins, regardless of where that line sits.
+ *      Preserves the existing invariant that the 1.0.9 `id=` shape outranks
+ *      the catch-all `Element 'X' not found`.
+ *
+ *   2. Inner loop scans lines from END to START. Within a single pattern,
+ *      the LAST matching line (the terminal failure) wins — earlier matches
+ *      are typically transient retries that maestro-runner reports as
+ *      `[INFO]` before the auto-retry succeeds. GH #118: PR #115's
+ *      first-match-anywhere scan captured a transient retry selector and
+ *      sent it to `cdp_repair_action`, wasting a 24h-budget slot.
+ *
+ * Falls back to a whole-buffer scan when no line matches a known pattern —
+ * preserves prior-art behavior for single-line inputs and any pattern that
+ * happens to span a line boundary (none today, but defensive).
+ *
+ * Returns `UNKNOWN` if no pattern matches at all.
  */
 export function parseMaestroFailure(output: string): MaestroFailure {
   if (!output || typeof output !== 'string') {
     return { kind: 'UNKNOWN', raw: '' };
   }
+  const lines = output.split('\n');
   for (const { re, build } of PATTERNS) {
-    const m = re.exec(output);
-    if (m) return build(m, output);
+    for (let i = lines.length - 1; i >= 0; i--) {
+      const line = lines[i];
+      if (!line) continue;
+      const m = line.match(re);
+      if (m) return build(m as RegExpExecArray, output);
+    }
+  }
+  // Fallback: whole-buffer scan for inputs without line breaks or for
+  // any pattern that happens to straddle a `\n`.
+  for (const { re, build } of PATTERNS) {
+    const m = output.match(re);
+    if (m) return build(m as RegExpExecArray, output);
   }
   return { kind: 'UNKNOWN', raw: output };
 }

scripts/cdp-bridge/test/unit/maestro-error-parser.test.js

@@ -119,16 +119,51 @@ test('parser: case-insensitive — works on upper-case ELEMENT', () => {
 });
 
 // ─────────────────────────────────────────────────────────────────────────────
-// First-match semantics: when output contains MULTIPLE failure-shaped lines,
-// the first one wins.
+// Terminal-match semantics (GH #118): when output contains MULTIPLE failure-
+// shaped lines, the LAST one wins — within a single pattern. Pattern
+// specificity outranks line position (covered by the 1.0.9 `id=` priority
+// test further down). Earlier in-line matches are typically transient
+// retries that maestro-runner reports as [INFO] before the auto-retry
+// succeeds; only the terminal failure should drive auto-repair.
 // ─────────────────────────────────────────────────────────────────────────────
 
-test('parser: returns first match when output contains multiple errors', () => {
+// GH #118: when output contains multiple failure lines, return the LAST
+// (terminal) one — not the first. Earlier matches are typically transient
+// retries that maestro-runner reports as [INFO] before the auto-retry
+// succeeds; the real failure is the last one before the run exits.
+test('parser: returns LAST match when output contains multiple errors (GH #118)', () => {
   const out = parseMaestroFailure([
     "Element with id 'first-failure' not found",
     "Element with id 'second-failure' not found",
   ].join('\n'));
-  assert.equal(out.selector, 'first-failure');
+  assert.equal(out.selector, 'second-failure');
+});
+
+test('parser: GH #118 transient-retry-then-real-failure shape — picks the terminal ERROR not the INFO retry', () => {
+  // Exact shape from the issue: an INFO-prefixed transient retry line
+  // earlier in the buffer matches the SELECTOR_NOT_FOUND pattern, but
+  // the run continues and ultimately fails on a different selector.
+  // Pre-fix behavior would auto-repair the transient (already-resolved)
+  // selector — wasting a budget slot and missing the real failure.
+  const out = parseMaestroFailure([
+    '[INFO] Tapping on element with id "transient-foo"',
+    '[INFO] Element with id "transient-foo" not found in current screen — retrying',
+    '[INFO] Tapping on element with id "transient-foo"',
+    '[ERROR] Element with id "real-failure" not found',
+    'Test FAILED',
+  ].join('\n'));
+  assert.equal(out.kind, 'SELECTOR_NOT_FOUND');
+  assert.equal(out.selectorKind, 'id');
+  assert.equal(out.selector, 'real-failure');
+});
+
+test('parser: single-line output still parses (whole-buffer fallback works when no newlines)', () => {
+  // The line-by-line scan returns nothing for a single-line input
+  // (the line equals the whole buffer; same path), but verifying the
+  // fallback whole-buffer scan still works for malformed-newline cases.
+  const out = parseMaestroFailure("Element with id 'lonely-failure' not found");
+  assert.equal(out.kind, 'SELECTOR_NOT_FOUND');
+  assert.equal(out.selector, 'lonely-failure');
 });
 
 // ─────────────────────────────────────────────────────────────────────────────