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

Closes #118.

PR #115's parseMaestroFailure ran each pattern against the whole buffer
and returned the first lexical match anywhere in the output. On
maestro-runner runs with retry hooks active, this captured a transient
`[INFO] Element with id "X" not found in current screen — retrying` line
earlier in the buffer — even when the actual terminal failure was on a
different selector. The transient (already-resolved) selector then got
sent to cdp_repair_action, burning a 24h-budget slot on a non-existent
problem and missing the real failure.

Fix: nested-loop selection that preserves BOTH invariants:

  1. Outer loop walks PATTERNS in order (most-specific first). The first
     pattern that hits any line wins, regardless of line position. Keeps
     the existing pattern-specificity invariant (e.g. 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 over earlier
     transient retries.

Falls back to a whole-buffer scan if no line matches a known pattern,
preserving prior behavior for single-line inputs and any future pattern
that spans a `\n`.

Tests:
- Inverted `returns first match` → `returns LAST match` (the existing
  test encoded the broken behavior)
- Added GH #118 transient-retry-then-real-failure shape using the issue's
  exact example
- Added single-line input test confirming the fallback whole-buffer scan
  path still works
- Updated the section comment that documented the old "first-match"
  semantic
- Existing 1.0.9 `id=` priority test still passes (proves pattern
  specificity is preserved over line position)

Verified: 1467/1467 cdp-bridge unit tests passing (+2 net new).

Note: codex-pair flagged a hypothetical edge case (multiple known
failure snippets on a SINGLE line — `line.match()` returns leftmost).
Not addressing in this PR — no real-world maestro-runner output emits
multiple failure snippets per line, and the recommended fix (collect
all match indexes and return rightmost) materially complicates the
code for an unobserved case. Can revisit with real captures if it
ever surfaces.

Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>

Author: Lykhoyda

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');
 });
 
 // ─────────────────────────────────────────────────────────────────────────────