diff --git a/controller/app/src/main/java/org/appdevforall/k2go/install/domain/MapsRunroleCommand.java b/controller/app/src/main/java/org/appdevforall/k2go/install/domain/MapsRunroleCommand.java index 165ef5001..ad4948852 100644 --- a/controller/app/src/main/java/org/appdevforall/k2go/install/domain/MapsRunroleCommand.java +++ b/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 SAT_OK = new HashSet<>(Arrays.asList("none", "7", "9", "11", "13")); private static final Set 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"; } } diff --git a/controller/app/src/main/java/org/appdevforall/k2go/install/presentation/InstallService.java b/controller/app/src/main/java/org/appdevforall/k2go/install/presentation/InstallService.java index b57bafe43..988292d86 100644 --- a/controller/app/src/main/java/org/appdevforall/k2go/install/presentation/InstallService.java +++ b/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. + * + *

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. + * + *

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; diff --git a/controller/app/src/main/java/org/appdevforall/k2go/redesign/ModuleHubFragment.java b/controller/app/src/main/java/org/appdevforall/k2go/redesign/ModuleHubFragment.java index 70f4044bb..98c9b9885 100644 --- a/controller/app/src/main/java/org/appdevforall/k2go/redesign/ModuleHubFragment.java +++ b/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(); diff --git a/controller/app/src/main/java/org/appdevforall/k2go/system/data/InstalledModulesReader.java b/controller/app/src/main/java/org/appdevforall/k2go/system/data/InstalledModulesReader.java index 883fb2e26..58a63c5b4 100644 --- a/controller/app/src/main/java/org/appdevforall/k2go/system/data/InstalledModulesReader.java +++ b/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. + * + *

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 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 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}. + * + *

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); } /** diff --git a/controller/app/src/main/java/org/appdevforall/k2go/system/domain/InstalledModules.java b/controller/app/src/main/java/org/appdevforall/k2go/system/domain/InstalledModules.java index ae8500ff7..70eab8f94 100644 --- a/controller/app/src/main/java/org/appdevforall/k2go/system/domain/InstalledModules.java +++ b/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. + * + *

{@code _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 _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. * diff --git a/controller/app/src/main/java/org/appdevforall/k2go/util/LocalVarsYamlParser.java b/controller/app/src/main/java/org/appdevforall/k2go/util/LocalVarsYamlParser.java index 3254ccdb7..7bccb5806 100644 --- a/controller/app/src/main/java/org/appdevforall/k2go/util/LocalVarsYamlParser.java +++ b/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 _install} / {@code _enabled} boolean flags. + * Minimal reader for the slice of IIAB's flat flag files the app cares about: + * top-level {@code _install} / {@code _enabled} flags in + * {@code local_vars.yml}, and the {@code _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. * *

Pure logic (no Android, no I/O) so it is JVM-unit-testable. It is * intentionally not 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); diff --git a/controller/app/src/test/java/org/appdevforall/k2go/install/domain/MapsRunroleCommandTest.java b/controller/app/src/test/java/org/appdevforall/k2go/install/domain/MapsRunroleCommandTest.java index 174b52c55..3f18d47d2 100644 --- a/controller/app/src/test/java/org/appdevforall/k2go/install/domain/MapsRunroleCommandTest.java +++ b/controller/app/src/test/java/org/appdevforall/k2go/install/domain/MapsRunroleCommandTest.java @@ -5,7 +5,8 @@ * Copyright : Copyright (c) 2026 AppDevForAll * Description : ADFA-4900. Unit tests for the maps runrole command builder — the per-layer * selection -> local_vars mapping, the "off" encoding, the search engine, the - * allowlist fallback, and that it forces --reinstall. + * allowlist fallback, and (K2GO-393) that it selects the runrole mode at runtime from + * the completion marker. * ============================================================================ */ package org.appdevforall.k2go.install.domain; @@ -17,14 +18,25 @@ public class MapsRunroleCommandTest { @Test - public void writesSelectedLayersAndForcesReinstall() { + public void writesSelectedLayers() { String cmd = MapsRunroleCommand.build("14", "13", "10", true); assertTrue(cmd.contains("maps_vector_zoom: 14")); assertTrue(cmd.contains("maps_satellite_zoom: 13")); assertTrue(cmd.contains("maps_terrain_zoom: 10")); assertTrue(cmd.contains("maps_search_engine: \"static\"")); assertTrue(cmd.contains("maps_region_downloader: True")); - assertTrue(cmd.contains("./runrole --reinstall maps")); + } + + /** K2GO-393: the mode is chosen at runtime by the completion marker, not hardcoded to --reinstall. + * Marker present -> --reinstall (deletes it, re-runs); marker absent -> plain runrole (recovers a + * half-done install; a bare --reinstall would error there). Mirrors runrole's own state gate. */ + @Test + public void selectsRunroleModeAtRuntimeFromTheMarker() { + String cmd = MapsRunroleCommand.build("11", "9", "7", true); + // the runtime gate on iiab_state.yml, and BOTH branches present + assertTrue(cmd.contains("grep -q '^maps_' /etc/iiab/iiab_state.yml")); + assertTrue(cmd.contains("./runrole --reinstall maps")); // marker present + assertTrue(cmd.contains("./runrole maps")); // marker absent (recovery) } @Test diff --git a/controller/app/src/test/java/org/appdevforall/k2go/system/domain/InstalledModulesTest.java b/controller/app/src/test/java/org/appdevforall/k2go/system/domain/InstalledModulesTest.java index 06248b7a2..285f5617a 100644 --- a/controller/app/src/test/java/org/appdevforall/k2go/system/domain/InstalledModulesTest.java +++ b/controller/app/src/test/java/org/appdevforall/k2go/system/domain/InstalledModulesTest.java @@ -123,6 +123,42 @@ public void resolveKeepsSilenceOnTheSafeSide() { InstalledModules.evidenceFor(parse("kiwix_install: True\n"), "kolibri"))); } + // ---- K2GO-393: the completion marker (intent vs result) ---------------- + + /** + * The whole reason the completion marker exists: the intent flag and the result marker can + * disagree. A maps install that set {@code maps_install: True} then died mid-download leaves + * the intent true and the {@code maps_installed} result absent -- and only the result is right. + */ + @Test + public void intentWithoutCompletionIsNotCompleted() { + assertFalse(InstalledModules.isCompleted(parse("maps_install: True\n"), "maps")); + } + + @Test + public void theCompletionMarkerMeansCompleted() { + assertTrue(InstalledModules.isCompleted(parse("maps_installed: True\n"), "maps")); + assertFalse(InstalledModules.isCompleted(parse("maps_installed: False\n"), "maps")); + } + + /** The parser must keep {@code _installed} as its own key, not fold it into {@code _install}. */ + @Test + public void completionMarkerSurvivesTheParser() { + assertTrue(parse("maps_installed: True\n").has("maps_installed")); + } + + /** An unreadable iiab_state cannot claim completion: null reads as "not finished", never true. */ + @Test + public void anUnreadableStateFileIsNotCompleted() { + assertFalse(InstalledModules.isCompleted(null, "maps")); + } + + /** A readable state file that names other roles but not this one is not completion for it. */ + @Test + public void readableStateWithoutTheMarkerIsNotCompleted() { + assertFalse(InstalledModules.isCompleted(parse("wifi_installed: True\n"), "maps")); + } + // ---- what the parser actually survives --------------------------------- /**