Merge pull request #631 from appdevforall/fix/K2GO-450-forgejo-seed-fail-loud

K2GO-450 fix(install): trust the Ansible PLAY RECAP, not stray [ERROR] lines

Author: luisguzman-adfa

controller/app/src/main/java/org/appdevforall/k2go/install/domain/AnsibleRunOutcome.java

@@ -2,16 +2,25 @@
  * File        : AnsibleRunOutcome.java
  * Author      : AppDevForAll
  * Copyright   : Copyright (c) 2026 AppDevForAll
- * Description : ADFA-4435 — pure decision for whether a runrole/Ansible execution failed.
- *               Ansible can print its failure to stdout yet still exit 0 (e.g. the /dev/shm
- *               multiprocessing crash), so the verdict considers the output as well as the
- *               exit code. No Android dependencies -> unit-testable on the JVM.
+ * Description : ADFA-4435 / K2GO-450 - pure decision for whether a runrole/Ansible execution
+ *               failed. The PLAY RECAP (per-host "... unreachable=N failed=M ...") is Ansible's
+ *               authoritative verdict, so when a recap is present it decides: an IGNORED error
+ *               (ignore_errors -> counted in ignored=, not failed=) prints [ERROR]/fatal lines but
+ *               is NOT a failure. Only when no recap appears (Ansible died before it could
+ *               summarize, e.g. the /dev/shm multiprocessing crash) do the [ERROR]/crash
+ *               signatures stand in. A non-zero process exit is always a failure.
+ *               No Android dependencies -> unit-testable on the JVM.
  */
 package org.appdevforall.k2go.install.domain;
 
 public final class AnsibleRunOutcome {
 
+    // Pre-recap failure hints: used ONLY when no PLAY RECAP was seen (Ansible crashed before it
+    // could summarize). When a recap IS present these are ignored, because Ansible prints [ERROR]
+    // and fatal lines for ignored errors too, which are not failures.
     private boolean sawError = false;
+    private boolean sawRecap = false;        // a PLAY RECAP host-summary line appeared
+    private boolean recapFailure = false;    // a recap host line reported failed>0 or unreachable>0
 
     /** Feed each output line as it streams from the container. */
     public void observe(String line) {
@@ -21,10 +30,36 @@ public void observe(String line) {
                 || line.contains("HEARTBEAT SESSION STOPPED")) {
             sawError = true;
         }
+        // A PLAY RECAP host summary carries BOTH "unreachable=" and "failed=" (every real recap line
+        // does); that pair is what distinguishes it from a stray task line that happens to contain one.
+        int failed = count(line, "failed=");
+        int unreachable = count(line, "unreachable=");
+        if (failed >= 0 && unreachable >= 0) {
+            sawRecap = true;
+            if (failed > 0 || unreachable > 0) recapFailure = true;
+        }
     }
 
-    /** True if the run failed: a non-zero exit OR an error seen in the output. */
+    /** True if the run failed. A non-zero exit always fails; otherwise trust the PLAY RECAP when
+     *  present, and fall back to the [ERROR]/crash signatures only when no recap was emitted. */
     public boolean failed(int exitCode) {
-        return exitCode != 0 || sawError;
+        if (exitCode != 0) return true;
+        if (sawRecap) return recapFailure;
+        return sawError;
+    }
+
+    /** The non-negative integer immediately after {@code key} in {@code line}, or -1 if absent. */
+    private static int count(String line, String key) {
+        int i = line.indexOf(key);
+        if (i < 0) return -1;
+        int start = i + key.length();
+        int j = start;
+        while (j < line.length() && Character.isDigit(line.charAt(j))) j++;
+        if (j == start) return -1;
+        try {
+            return Integer.parseInt(line.substring(start, j));
+        } catch (NumberFormatException e) {
+            return -1;
+        }
     }
 }

controller/app/src/test/java/org/appdevforall/k2go/install/domain/AnsibleRunOutcomeTest.java

@@ -48,4 +48,28 @@ public class AnsibleRunOutcomeTest {
         o.observe(null);
         assertFalse(o.failed(0));
     }
+
+    @Test public void ignoredError_withCleanRecap_isNotFailed() {
+        // K2GO-450: the forgejo role has an ignore_errors task that prints [ERROR]/fatal, but the
+        // PLAY RECAP is clean (failed=0, unreachable=0, ignored=1). An ignored error is not a
+        // failure: with a recap present it is authoritative, so the run must NOT be marked failed.
+        AnsibleRunOutcome o = new AnsibleRunOutcome();
+        o.observe("fatal: [127.0.0.1]: FAILED! => {\"msg\": \"something\"} ...ignoring");
+        o.observe("[ERROR]: an ignored task error");
+        o.observe("127.0.0.1 : ok=338 changed=20 unreachable=0 failed=0 skipped=27 rescued=0 ignored=1");
+        assertFalse(o.failed(0));
+    }
+
+    @Test public void realFailureRecap_isFailed() {
+        // A genuinely failed task increments failed= in the recap -> failure (even on a quirky exit 0).
+        AnsibleRunOutcome o = new AnsibleRunOutcome();
+        o.observe("127.0.0.1 : ok=10 changed=3 unreachable=0 failed=2 skipped=1 rescued=0 ignored=0");
+        assertTrue(o.failed(0));
+    }
+
+    @Test public void unreachableHost_isFailed() {
+        AnsibleRunOutcome o = new AnsibleRunOutcome();
+        o.observe("127.0.0.1 : ok=1 changed=0 unreachable=1 failed=0 skipped=0 rescued=0 ignored=0");
+        assertTrue(o.failed(0));
+    }
 }