From 78a3aa51dde30770a24511b5285a3a9cbe140df0 Mon Sep 17 00:00:00 2001 From: "Luis Guzman (AppDevForAll)" Date: Sun, 6 Sep 2026 20:46:29 -0600 Subject: [PATCH 1/5] K2GO-393 fix(maps): choose runrole mode at runtime so recovery does not error MapsRunroleCommand hardcoded 'runrole --reinstall maps'. --reinstall requires an existing maps_ line in iiab_state.yml to delete; after a failed install that line is gone, so a retry errors out (exit 1). This is the retry bug for a half-done maps install. Choose the mode at runtime by the marker, mirroring runrole's own grep '^maps_' $IIAB_STATE_FILE gate: marker present (first selection or a re-selection over a completed/base-seeded install) -> --reinstall; marker absent (a prior --reinstall run failed after deleting it) -> plain runrole, which re-enters install.yml and lets creates: re-fetch only the missing file. The app's decision now always matches runrole's. Unit test asserts both branches and the runtime gate. --- .../install/domain/MapsRunroleCommand.java | 19 ++++++++++++++++--- .../domain/MapsRunroleCommandTest.java | 18 +++++++++++++++--- 2 files changed, 31 insertions(+), 6 deletions(-) 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/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 From 6d0a631ba89057403105fdf7338d3038dc7dd1cc Mon Sep 17 00:00:00 2001 From: "Luis Guzman (AppDevForAll)" Date: Sun, 6 Sep 2026 22:33:26 -0600 Subject: [PATCH 2/5] K2GO-393 fix(maps): gate maps-installed on the completion marker, not the intent Maps is a proot install. An app death mid-install left the local_vars intent (maps_install: True) claiming maps was installed, with no download done and no recovery offered. Read maps "installed" from the iiab_state completion marker (maps_installed: True, written only when the role finishes) instead. - InstalledModules.isCompleted: pure rule for the completion marker. - InstalledModulesReader: one readRootfsYaml behind both files (local_vars intent and iiab_state completion); installedKeys reads maps from the marker; isCompletionGated names the proot-module set (today maps), one source shared with the hub. - ModuleHubFragment.confirmByProbe skips completion-gated keys, so a live probe cannot re-mask a half-done maps whose partial content still answers. - LocalVarsYamlParser also keeps _installed keys (one parser, both files). Verified on device (OnePlus): with the marker absent and the intent still true, both the module detail and the hub show maps Not installed / Install now, so the half-done build is recoverable; with the marker present, both show Installed. --- .../k2go/redesign/ModuleHubFragment.java | 6 ++ .../system/data/InstalledModulesReader.java | 60 +++++++++++++++++-- .../k2go/system/domain/InstalledModules.java | 20 +++++++ .../k2go/util/LocalVarsYamlParser.java | 20 ++++--- .../system/domain/InstalledModulesTest.java | 36 +++++++++++ 5 files changed, 131 insertions(+), 11 deletions(-) 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/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 --------------------------------- /** From 5007581eafd90a38c5dedae91e6c0c5604c23b31 Mon Sep 17 00:00:00 2001 From: "Luis Guzman (AppDevForAll)" Date: Sun, 6 Sep 2026 23:18:08 -0600 Subject: [PATCH 3/5] K2GO-393 fix(maps): kill a hung module runrole so it fails through to Retry (A4) The module stall watch was surface-only: on an unstable network the in-proot download can hang forever (the aria2c/meta4 fetch blocks with no timeout) and the install spun with no way out. Add a longer hard threshold that kills the runrole. The kill reuses the existing failure path: a non-zero exit makes installNextModule's onProcessExit revert the module and offer Retry, exactly as a self-failed module -- no new recovery plumbing. A generation token drops a kill queued for a module that has since ended. The soft "stalled" hint is unchanged. Note: Ansible does not stream a shell task's stdout, so during a long download the log goes quiet; the write-dir growth is the live movement signal. Safe for maps -- aria2c writes a 60s-summary log into library/downloads/maps. --- .../install/presentation/InstallService.java | 50 +++++++++++++++---- 1 file changed, 40 insertions(+), 10 deletions(-) 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..40a9fc86d 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,21 @@ private void stopModuleStallWatch() { ModuleQueueRepository.get().postStalled(false); } + /** + * K2GO-393 (A4): kill a runrole hung for {@link #MODULE_HARD_STALL_MS}. The kill makes it exit + * non-zero, so {@code installNextModule}'s {@code onProcessExit} takes its normal failure branch + * (revert + 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. + */ + 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 +1158,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; From bf10b36e1fcdca12fa9326cec2649c2f8c64bf86 Mon Sep 17 00:00:00 2001 From: "Luis Guzman (AppDevForAll)" Date: Sun, 6 Sep 2026 23:46:30 -0600 Subject: [PATCH 4/5] K2GO-393 docs(maps): correct the A4 hard-stall recovery callback + note the kill limit Device test showed the kill drives recovery through onError (the kill closes the runrole output stream mid-read), not onProcessExit as the comment claimed; both reach the same revert + Retry path. Also note that killProcess orphans proot's in-container child rather than reaping it (shared with doCancel) -- recovery still works; reaping the subtree is a follow-up. --- .../k2go/install/presentation/InstallService.java | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) 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 40a9fc86d..8cfb89a91 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 @@ -1133,10 +1133,15 @@ private void stopModuleStallWatch() { } /** - * K2GO-393 (A4): kill a runrole hung for {@link #MODULE_HARD_STALL_MS}. The kill makes it exit - * non-zero, so {@code installNextModule}'s {@code onProcessExit} takes its normal failure branch - * (revert + 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. + * 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: killProcess SIGKILLs proot, which orphans its in-container child (a real hung + * aria2c, here the test sleep) rather than reaping it -- shared with doCancel. Recovery still works; + * reaping the subtree is a follow-up. */ private void hardStallKill(final String moduleKey, final int gen) { if (gen != moduleStallGen || finished || cancelled || moduleStallKilled) return; From 90ff6a17d396e86eb753ded9ea73d9e8970b2a0b Mon Sep 17 00:00:00 2001 From: "Luis Guzman (AppDevForAll)" Date: Mon, 7 Sep 2026 00:11:23 -0600 Subject: [PATCH 5/5] K2GO-393 docs(maps): ground the A4 kill-leak note in the role's real flags Deep recon of roles/maps/tasks/download_large_file.yml: the orphaned in-container child is low impact -- aria2c self-exits (default max-tries=5, timeout=60) and the meta4 requests.get just idles a socket; neither firehoses disk. So the orphan does not warrant a bespoke reaper. Reap it (reusing EnvironmentProcess's /proc sweep, the established idiom) only if a retry-conflict is ever observed in the field. --- .../k2go/install/presentation/InstallService.java | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) 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 8cfb89a91..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 @@ -1139,9 +1139,10 @@ private void stopModuleStallWatch() { * 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: killProcess SIGKILLs proot, which orphans its in-container child (a real hung - * aria2c, here the test sleep) rather than reaping it -- shared with doCancel. Recovery still works; - * reaping the subtree is a follow-up. + *

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;