K2GO-393 fix(maps): recover a half-done install (runrole mode, completion gate, stall kill)

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

@@ -10,8 +10,10 @@
  *               off; search maps to maps_search_engine + maps_search_static_db. Every var the role's
  *               iiab.ini step references is written so the play never hits an undefined var. Values
  *               are validated against a fixed allowlist (D2 shell-injection guard); anything
- *               unexpected falls back to a safe default. --reinstall forces install.yml to re-fetch
- *               the chosen tiles (a plain runrole skips it because maps ships in the base image).
+ *               unexpected falls back to a safe default. K2GO-393: the runrole mode is chosen at
+ *               RUNTIME from the completion marker in iiab_state.yml -- --reinstall over a completed or
+ *               base-seeded install, plain runrole to recover a half-done one (a bare --reinstall errors
+ *               when the marker was already deleted by a prior failed --reinstall run).
  * ============================================================================
  */
 package org.appdevforall.k2go.install.domain;
@@ -31,6 +33,9 @@ private MapsRunroleCommand() {}
     private static final Set<String> SAT_OK = new HashSet<>(Arrays.asList("none", "7", "9", "11", "13"));
     private static final Set<String> TERRAIN_OK = new HashSet<>(Arrays.asList("0-none", "7", "8", "9", "10"));
     private static final String LV = "/etc/iiab/local_vars.yml";
+    // K2GO-393: the completion marker (maps_installed: True) lives here, written only at the END of the
+    // maps role's install.yml. runrole gates on this file (and the vars files), not on iiab.ini.
+    private static final String IIAB_STATE = "/etc/iiab/iiab_state.yml";
 
     /** Build the sed-delete + echo (append-if-missing) + runrole command for the given selection. */
     public static String build(String vector, String sat, String terrain, boolean searchOn) {
@@ -54,6 +59,14 @@ public static String build(String vector, String sat, String terrain, boolean se
                 " && echo 'maps_search_nominatim_db: basic' >> " + LV +
                 " && echo 'maps_ne6_zoom: 6' >> " + LV +
                 " && echo 'maps_preset_full_quality_regions: []' >> " + LV +
-                " && cd /opt/iiab/iiab && ./runrole --reinstall maps";
+                " && cd /opt/iiab/iiab" +
+                // K2GO-393: pick the mode at runtime by the marker, mirroring runrole's own
+                // `grep -q "^maps_" $IIAB_STATE_FILE` gate. Marker present (a first selection, or a
+                // re-selection over a completed install -- the base image pre-seeds maps_installed) ->
+                // --reinstall (deletes the marker, re-runs install.yml). Marker absent (a prior run
+                // failed after --reinstall already deleted it) -> plain runrole, which re-enters
+                // install.yml and lets `creates:` re-fetch only the missing file. A bare --reinstall
+                // here would ERROR ("no maps_ line") -- the retry bug this fixes.
+                " && if grep -q '^maps_' " + IIAB_STATE + " 2>/dev/null; then ./runrole --reinstall maps; else ./runrole maps; fi";
     }
 }

controller/app/src/main/java/org/appdevforall/k2go/install/presentation/InstallService.java

@@ -135,13 +135,18 @@ public final class InstallService extends Service {
     private volatile boolean finished = false;
     private volatile boolean started = false;
 
-    // ADFA-4898 P4: movement-based stall detection for the running module (surface only, never kills).
+    // ADFA-4898 P4 / K2GO-393 (A4): movement-based stall detection. MODULE_STALL_MS surfaces a soft
+    // "stalled" hint; MODULE_HARD_STALL_MS kills a hung runrole so it fails through to Retry.
     /** Generous, minute-scale: well above a quiet git-clone/apt phase, so a slow-but-alive install is
      *  never flagged. Above Freshness.STALE_MS (30s), which is tuned for ~1s-cadence REST polls. */
     private static final long MODULE_STALL_MS = 120_000L;
+    /** K2GO-393 (A4): kill backstop, well above the soft hint so only a true forever-hang trips it. */
+    private static final long MODULE_HARD_STALL_MS = 360_000L;
     private static final long MODULE_STALL_POLL_MS = 15_000L;
     private volatile long lastModuleMovementMs = 0L;   // stamped on a runrole output line OR write-dir growth
     private volatile long lastModuleDirSize = -1L;
+    private volatile boolean moduleStallKilled = false; // K2GO-393 (A4): one-shot hard-stall kill per module
+    private volatile int moduleStallGen = 0;           // K2GO-393 (A4): supersedes a kill queued for a prior run
     private Runnable moduleStallCheck;                 // main-thread poller; null when not watching
 
     /**
@@ -1076,18 +1081,20 @@ private void revertModuleInLocalVars(String module, Runnable then) {
         });
     }
 
-    // ---- ADFA-4898 P4: movement-based stall watch (surface only, never kills) -------------------
+    // ---- ADFA-4898 P4 / K2GO-393 A4: movement-based stall watch (surface, then kill a forever-hang) --
 
     /**
      * Watch the running module for movement — a runrole output line (stamped in onOutputLine) OR growth
-     * of its on-disk write directory. When neither moves for {@link #MODULE_STALL_MS}, publish a
-     * "stalled" hint the live card shows; the install is never touched. Re-armed per module; the
-     * dir-growth backstop covers quiet network phases (e.g. calibre-web's git clone) that emit no log.
+     * of its on-disk write directory. No movement for {@link #MODULE_STALL_MS} surfaces a "stalled" hint;
+     * for the much longer {@link #MODULE_HARD_STALL_MS} the install has hung, so {@link #hardStallKill}
+     * ends it. Re-armed per module; the dir-growth backstop covers quiet phases that emit no log.
      */
     private void startModuleStallWatch(final String moduleKey) {
         stopModuleStallWatch();
         lastModuleMovementMs = android.os.SystemClock.elapsedRealtime();
         lastModuleDirSize = -1L;
+        moduleStallKilled = false;
+        final int gen = ++moduleStallGen;
         ModuleQueueRepository.get().postStalled(false);
         // ADFA-4898 P4: make write-dir drift visible instead of silent. If a module has no mapped dir
         // (a new module added without updating moduleWriteDirRel), the watch still works off the log
@@ -1105,9 +1112,14 @@ private void startModuleStallWatch(final String moduleKey) {
                         lastModuleDirSize = size;
                         lastModuleMovementMs = android.os.SystemClock.elapsedRealtime();
                     }
+                    long now = android.os.SystemClock.elapsedRealtime();
                     boolean fresh = org.appdevforall.k2go.env.Freshness.fresh(
-                            lastModuleMovementMs, android.os.SystemClock.elapsedRealtime(), MODULE_STALL_MS);
+                            lastModuleMovementMs, now, MODULE_STALL_MS);
                     ModuleQueueRepository.get().postStalled(!fresh);
+                    // K2GO-393 (A4): a much longer dead window is a real hang; kill on the main thread.
+                    boolean hardStalled = !org.appdevforall.k2go.env.Freshness.fresh(
+                            lastModuleMovementMs, now, MODULE_HARD_STALL_MS);
+                    if (hardStalled) heldHandler.post(() -> hardStallKill(moduleKey, gen));
                 });
                 heldHandler.postDelayed(this, MODULE_STALL_POLL_MS);
             }
@@ -1120,6 +1132,27 @@ private void stopModuleStallWatch() {
         ModuleQueueRepository.get().postStalled(false);
     }
 
+    /**
+     * K2GO-393 (A4): kill a runrole hung for {@link #MODULE_HARD_STALL_MS}. The kill ends the run, so
+     * installNextModule's failure path (onError when the kill closes the output stream, else
+     * onProcessExit) reverts the module and offers Retry -- no new plumbing. {@code cancelled} is left
+     * unset so that callback runs (the cancel path suppresses it). {@code gen} drops a kill queued for
+     * a module that has since ended.
+     *
+     * <p>Known limit (shared with doCancel): killProcess SIGKILLs proot, orphaning its in-container
+     * child. Low impact by the role's own flags -- aria2c self-exits (max-tries=5, timeout=60) and the
+     * meta4 fetch just idles a socket; neither firehoses disk, so recovery works regardless. Reap the
+     * orphan (reuse EnvironmentProcess's /proc sweep) only if a retry-conflict is ever observed.
+     */
+    private void hardStallKill(final String moduleKey, final int gen) {
+        if (gen != moduleStallGen || finished || cancelled || moduleStallKilled) return;
+        moduleStallKilled = true;
+        log("[Stall] '" + moduleKey + "' made no progress for " + (MODULE_HARD_STALL_MS / 1000)
+                + "s; killing the runrole so it fails through to Retry");
+        stopModuleStallWatch();
+        if (prootEngine != null) prootEngine.killProcess();
+    }
+
     /** Total bytes under the module's write directory on the host rootfs, or -1 if unknown/absent. */
     private long moduleWriteDirSize(String key) {
         String rel = moduleWriteDirRel(key);
@@ -1131,15 +1164,18 @@ private long moduleWriteDirSize(String key) {
 
     /**
      * Where each module does its heavy on-disk writes (relative to the rootfs), for the stall watch's
-     * growth backstop. Heuristic and best-effort — keep it in sync with the ansible roles. If a key is
-     * unmapped or the path drifts, the watch silently falls back to the log heartbeat (never a false
-     * verdict); {@link #startModuleStallWatch} logs the unmapped case so that drift is visible, not silent.
+     * growth backstop. Keep in sync with the ansible roles.
+     *
+     * <p>K2GO-393 (A4): load-bearing now, not just a hint. Ansible does not stream a shell task's
+     * stdout, so during a long download the log goes quiet and this dir's growth is the only movement
+     * signal -- a wrong path would kill a healthy download. Safe for maps: aria2c writes a 60s-summary
+     * log into {@code library/downloads/maps}, so the dir keeps growing while the download is alive.
      */
     private static String moduleWriteDirRel(String key) {
         if (key == null) return null;
         switch (key) {
             case "calibreweb": return "usr/local/calibre-web-py3";   // git clone + venv
-            case "maps":       return "library/downloads/maps";
+            case "maps":       return "library/downloads/maps";      // aria2c logs a 60s summary here throughout the download
             case "matomo":     return "library/www/matomo";
             case "kolibri":    return "var/cache/apt/archives";       // chatty on stdout too; disk is the fallback
             default:           return null;

controller/app/src/main/java/org/appdevforall/k2go/redesign/ModuleHubFragment.java

@@ -204,6 +204,12 @@ private void confirmByProbe(final int gen) {
         for (final ModuleCards.Card c : ModuleCards.all()) {
             if (c.requires64Bit() && !is64Bit()) continue;   // hidden on this device
             if (installed.contains(c.key())) continue;       // disk already says yes
+            // K2GO-393: a completion-gated module (maps, a proot install) is answered by its
+            // iiab_state marker, which the disk floor above already read. It always writes that
+            // marker when it finishes, so a probe can only re-add a half-done build whose partial
+            // content happens to answer -- masking the recovery the absent marker asks for. Keep the
+            // marker the single source: no probe rescue for these.
+            if (org.appdevforall.k2go.system.data.InstalledModulesReader.isCompletionGated(c.key())) continue;
             probesPending++;
             final String key = c.key();
             final String endpoint = c.endpoint();

controller/app/src/main/java/org/appdevforall/k2go/system/data/InstalledModulesReader.java

@@ -23,6 +23,7 @@
 import java.io.File;
 import java.io.FileInputStream;
 import java.nio.charset.StandardCharsets;
+import java.util.HashSet;
 import java.util.Set;
 
 /**
@@ -46,6 +47,15 @@ public final class InstalledModulesReader {
 
     private static final String TAG = "K2Go-Modules";
     private static final String LOCAL_VARS = "etc/iiab/local_vars.yml";
+    // K2GO-393: the completion markers (maps_installed: True), written at the END of a role's play.
+    private static final String IIAB_STATE = "etc/iiab/iiab_state.yml";
+    // K2GO-393: maps is the one proot install. Its "finished" truth is the iiab_state completion
+    // marker (written at the end of the play), not the local_vars intent (which a process death
+    // mid-install leaves falsely true, with no download done). Every other module -- the live/REST
+    // ones -- keeps the local_vars flag. Scoped to maps on purpose: the other roles' completion
+    // semantics are not yet verified on device, so gating them all on iiab_state is a separate,
+    // evidence-backed change, not a blind widening.
+    private static final String MAPS_KEY = "maps";
 
     /** Refuse to read anything absurd for this file; a real one is a few KB. */
     private static final long MAX_BYTES = 512L * 1024L;
@@ -60,22 +70,35 @@ private InstalledModulesReader() {
      * laid down right now, or a rootfs we cannot read. None of those mean "nothing is installed".
      */
     public static JSONObject readFlags(Context ctx) {
+        return readRootfsYaml(ctx, LOCAL_VARS);
+    }
+
+    /**
+     * K2GO-393: read and parse one flat-flag YAML file from the rootfs, or {@code null} when it
+     * cannot be read.
+     *
+     * <p>The single mechanism behind both files this class reads -- {@code local_vars.yml} (the
+     * intent flags) and {@code iiab_state.yml} (the completion markers). Keeping it in one place is
+     * the point: the size cap, the API-24-safe stream and the "unreadable is null, not empty" rule
+     * cannot drift between the two the way two hand-copied readers would.
+     */
+    private static JSONObject readRootfsYaml(Context ctx, String relPath) {
         if (ctx == null) {
             return null;
         }
-        File file = new File(SystemStateEvaluator.rootfsDir(ctx), LOCAL_VARS);
+        File file = new File(SystemStateEvaluator.rootfsDir(ctx), relPath);
         try {
             if (!file.isFile() || !file.canRead()) {
                 return null;
             }
             if (file.length() > MAX_BYTES) {
-                Log.w(TAG, "local_vars.yml is " + file.length() + " bytes; refusing to parse");
+                Log.w(TAG, relPath + " is " + file.length() + " bytes; refusing to parse");
                 return null;
             }
             return LocalVarsYamlParser.parseToJson(readUtf8(file));
         } catch (Exception e) {
             // Vanished mid-read, permissions, a wipe in flight. All of them are "not established".
-            Log.w(TAG, "could not read local_vars.yml", e);
+            Log.w(TAG, "could not read " + relPath, e);
             return null;
         }
     }
@@ -114,7 +137,36 @@ public static Set<String> installedKeys(Context ctx) {
         if (flags == null) {
             return null;
         }
-        return InstalledModules.from(flags, ModuleRegistry.validYamlKeys());
+        // Most modules answer from the local_vars intent; a completion-gated one (maps) answers
+        // from its iiab_state marker instead, so a half-done build does not read installed.
+        Set<String> installed = new HashSet<>();
+        for (String key : ModuleRegistry.validYamlKeys()) {
+            if (isCompletionGated(key)
+                    ? completed(ctx, key)
+                    : InstalledModules.isInstalled(flags, key)) {
+                installed.add(key);
+            }
+        }
+        return installed;
+    }
+
+    /**
+     * K2GO-393: keys whose "installed" is answered by the {@code iiab_state} completion marker, not
+     * by the {@code local_vars} intent or a live probe. The proot installs -- today only
+     * {@link #MAPS_KEY}.
+     *
+     * <p>Public so the one place a probe could re-mask a half-done build --
+     * {@code ModuleHubFragment.confirmByProbe} -- reads the same predicate and skips these, keeping
+     * the completion marker the single source across the hub and the detail. See {@link #MAPS_KEY}
+     * for why maps and not the REST modules.
+     */
+    public static boolean isCompletionGated(String key) {
+        return MAPS_KEY.equals(key);
+    }
+
+    /** K2GO-393: whether the completion marker in {@code iiab_state.yml} says this module finished. */
+    private static boolean completed(Context ctx, String key) {
+        return InstalledModules.isCompleted(readRootfsYaml(ctx, IIAB_STATE), key);
     }
 
     /**

controller/app/src/main/java/org/appdevforall/k2go/system/domain/InstalledModules.java

@@ -43,10 +43,30 @@ public final class InstalledModules {
 
     /** The suffix the installer appends to a module's yaml base key. */
     private static final String INSTALL_SUFFIX = "_install";
+    /** K2GO-393: the suffix the runrole appends when it records COMPLETION in iiab_state.yml. */
+    private static final String INSTALLED_SUFFIX = "_installed";
 
     private InstalledModules() {
     }
 
+    /**
+     * K2GO-393: whether the completion markers claim this module FINISHED installing.
+     *
+     * <p>{@code <key>_installed} in {@code iiab_state.yml} is written only at the END of a role's
+     * install, after every download. Unlike {@link #isInstalled} -- the {@code <key>_install}
+     * intention written BEFORE the run and reverted on failure -- it is never left true by a process
+     * death mid-install. Prefer this where the question is "did it finish", not "was it asked for":
+     * it closes the process-death window the class comment above calls out, without a live probe.
+     *
+     * @param stateFlags  parsed {@code iiab_state.yml}; null is treated as "nothing known"
+     */
+    public static boolean isCompleted(JSONObject stateFlags, String yamlBaseKey) {
+        if (stateFlags == null || yamlBaseKey == null || yamlBaseKey.isEmpty()) {
+            return false;
+        }
+        return stateFlags.optBoolean(yamlBaseKey + INSTALLED_SUFFIX, false);
+    }
+
     /**
      * Whether the flags claim this module is installed.
      *

controller/app/src/main/java/org/appdevforall/k2go/util/LocalVarsYamlParser.java

@@ -5,8 +5,12 @@
 import java.util.Locale;
 
 /**
- * Minimal reader for the slice of {@code local_vars.yml} the app cares about:
- * top-level {@code <module>_install} / {@code <module>_enabled} boolean flags.
+ * Minimal reader for the slice of IIAB's flat flag files the app cares about:
+ * top-level {@code <module>_install} / {@code <module>_enabled} flags in
+ * {@code local_vars.yml}, and the {@code <role>_installed} completion markers in
+ * {@code iiab_state.yml} (K2GO-393). All three are flat {@code key: bool} lines,
+ * so one parser reads both files -- there is no second place that knows how to
+ * parse them.
  *
  * <p>Pure logic (no Android, no I/O) so it is JVM-unit-testable. It is
  * intentionally <strong>not</strong> a general YAML parser — it splits on the
@@ -21,10 +25,10 @@ private LocalVarsYamlParser() {
     }
 
     /**
-     * Parses {@code _install}/{@code _enabled} flags into a {@link JSONObject} of
-     * key → boolean. A value counts as {@code true} when it is {@code true},
-     * {@code yes} or {@code 1} (case-insensitive). Lines that are blank, comments
-     * ({@code #}) or unrelated keys are ignored. Never returns {@code null}.
+     * Parses {@code _install} / {@code _enabled} / {@code _installed} flags into a
+     * {@link JSONObject} of key → boolean. A value counts as {@code true} when it is
+     * {@code true}, {@code yes} or {@code 1} (case-insensitive). Lines that are blank,
+     * comments ({@code #}) or unrelated keys are ignored. Never returns {@code null}.
      */
     public static JSONObject parseToJson(String yaml) {
         JSONObject json = new JSONObject();
@@ -38,7 +42,9 @@ public static JSONObject parseToJson(String yaml) {
             String[] parts = line.split(":", 2);
             String key = parts[0].trim();
             String val = parts[1].trim().toLowerCase(Locale.ROOT);
-            if (key.endsWith("_install") || key.endsWith("_enabled")) {
+            // "_installed" is the iiab_state completion marker; "_install"/"_enabled" are the
+            // local_vars intent flags. endsWith is exclusive: "maps_installed" is not "_install".
+            if (key.endsWith("_install") || key.endsWith("_enabled") || key.endsWith("_installed")) {
                 boolean isTrue = val.equals("true") || val.equals("yes") || val.equals("1");
                 try {
                     json.put(key, isTrue);