From 9aab29c99417e5ff93a0d0c2ee276cab49f6ceff Mon Sep 17 00:00:00 2001 From: Luis Guzman Date: Fri, 28 Aug 2026 21:38:00 -0600 Subject: [PATCH 01/10] ADFA-5343 docs(server-lifecycle): key desired-state predicate on holder execution class The desired-state predicate used EnvironmentLock.currentHolder == NONE, forcing the server DOWN for every holder. But LIVE holders run against the live server and need it UP: DOWNLOAD (ZIM/Books/Kolibri are ExecutionClass.LIVE; the device only POSTs+polls, ZimDownloadService.java:10) and DASHBOARD (live dash-node self-update). This also contradicted the ADR's own section 6, which already requires DASHBOARD to stay UP. Refine the predicate to key on the holder execution class: desired = installed && healthy && userWantsOn && currentHolder.executionClass != STOPPED Reuse the existing ADR-5061 type Operation.ExecutionClass { LIVE, STOPPED } (Operation.java:49-61) as one property on the Holder enum (STOPPED for CLONE/BACKUP/RESTORE/INSTALL; LIVE for DOWNLOAD/DASHBOARD/NONE) rather than adding a parallel HolderClass. No new source of truth; NONE keeps desired=UP as before. Design edit only; the Holder enum property lands with Phase 0 code. Co-Authored-By: Claude Opus 4.8 --- .../ADR-5343-server-lifecycle-reconciler.md | 42 ++++++++++++++----- 1 file changed, 31 insertions(+), 11 deletions(-) diff --git a/controller/docs/ADR-5343-server-lifecycle-reconciler.md b/controller/docs/ADR-5343-server-lifecycle-reconciler.md index 64bf988a4..330f41d7e 100644 --- a/controller/docs/ADR-5343-server-lifecycle-reconciler.md +++ b/controller/docs/ADR-5343-server-lifecycle-reconciler.md @@ -203,18 +203,37 @@ lifecycle - and make everything else an **observer** or an **intent-setter**. holder wants it down: ``` - desired = (SystemFacts.installed ? SystemFacts.healthy) - ? userWantsOn - ? EnvironmentLock.currentHolder == NONE + desired = SystemFacts.installed && SystemFacts.healthy + && userWantsOn + && EnvironmentLock.currentHolder.executionClass != STOPPED ``` Every input already exists: `installed`/`healthy` come from `system/data/SystemFactsReader.java:72-89` - (which already folds `InstallGuard` + `InterruptedInstallDetector`); "a holder wants it down" is - exactly `env/EnvironmentLock.currentHolder()` (`env/EnvironmentLock.java:172-188`, already the one - enumerator of CLONE/BACKUP/RESTORE/INSTALL/DOWNLOAD/DASHBOARD). `userWantsOn` is the persisted + (which already folds `InstallGuard` + `InterruptedInstallDetector`); `userWantsOn` is the persisted intent that `Preferences.WatchdogEnable` is *already* standing in for today - (`ServerController.java:342,461`). **No new source of truth is created - desired is a pure function - of existing facts.** + (`ServerController.java:342,461`). + + The last term keys on the **holder's execution class, not on `== NONE`.** Not every holder wants the + server down. ADR-5061 **already** owns that split in one pure-JVM type, + `system/domain/Operation.ExecutionClass { LIVE, STOPPED }` (`system/domain/Operation.java:49-61`), + whose own doc is exactly our distinction: `LIVE` = "the box stays up; the device POSTs and polls the + in-server REST core"; `STOPPED` = "the box goes down: `pdsm stop`, then Ansible in a transient proot." + - **STOPPED holders want it DOWN** — CLONE, BACKUP, RESTORE, INSTALL each `pdsm stop` the box and run + a transient `proot` runrole (`deepop/DeepOpService.java:124,127`; `redesign/CloneFragment.java:475,1322`). + - **LIVE holders run *against* the live server and want it UP** — DOWNLOAD (the device only POSTs + + polls; the work runs on the live server, `redesign/ZimDownloadService.java:10`) and DASHBOARD (a live + dash-node self-update, `EnvironmentLock.java:184-186`, ADFA-5333). Forcing `desired=DOWN` for these + two would stop the very server they depend on — and would directly contradict §6, which already says + `DASHBOARD` must stay `UP` (expect only a blip). + + **Reuse that type; do not add a parallel one.** Give the existing `Holder` enum one property that + returns `Operation.ExecutionClass` (STOPPED for CLONE/BACKUP/RESTORE/INSTALL; LIVE for + DOWNLOAD/DASHBOARD, and for `NONE` — no holder is forcing the box down). `EnvironmentLock.currentHolder()` + (`env/EnvironmentLock.java:172-188`) stays the one enumerator of holders; `desired` asks the returned + holder its class instead of comparing to a magic `NONE`. A new `HolderClass` enum would be a second + type saying what `ExecutionClass` already says — the same duplicate-truth this ADR exists to remove. + **No new source of truth is created — `desired` is a pure function of existing facts, the LIVE/STOPPED + vocabulary has one owner (ADR-5061), and the holder just names its class.** 2. **One liveness source.** A single `ServerLiveness` snapshot, freshness-windowed, replacing the four: @@ -243,8 +262,9 @@ lifecycle - and make everything else an **observer** or an **intent-setter**. published `ServerPhase` from the reconciler (the way `onNewIntent` already only monitors, `redesign/LibraryActivity.java:870`). The user button calls `reconciler.setUserWantsOn(boolean)` - it *sets desired*, it does not start-XOR-stop on a cache. Deep operations call - `EnvironmentLock.acquire(...)` (already the signal) and the reconciler observes the holder and - stops; on `release(...)` it observes `NONE` and brings the box back **wherever the app is** - no + `EnvironmentLock.acquire(...)` (already the signal); the reconciler observes a **STOPPED-class** + holder (`executionClass == STOPPED`) and stops, while a LIVE-class holder (DOWNLOAD/DASHBOARD) leaves + `desired=UP`. On `release(...)` it observes `NONE` and brings the box back **wherever the app is** - no Activity owns the reboot. 5. **Home for the owner.** The reconciler is an **app-scoped singleton** (created in @@ -282,7 +302,7 @@ stateDiagram-v2 STARTING --> UP: liveness: servicesAnswering (NOOP_HEALTHY) STARTING --> STARTING: reconcile: WAIT (progress fresh) STARTING --> DOWN: reconcile: no progress kill, relaunch next tick - UP --> STOPPING: reconcile(desired=DOWN): a holder wants it down / user off + UP --> STOPPING: reconcile(desired=DOWN): a STOPPED-class holder wants it down / user off UP --> STARTING: liveness: !servicesAnswering && desired=UP (auto-heals flap) STOPPING --> DOWN: pdsm stop exits DOWN --> UP: reconcile keeps desired=UP until achieved From b745c067179b37597bbca373eebf4d3cf9e6e403 Mon Sep 17 00:00:00 2001 From: Luis Guzman Date: Fri, 28 Aug 2026 21:45:20 -0600 Subject: [PATCH 02/10] ADFA-5343 feat(server-lifecycle): Phase 0 - one liveness source (ServerLiveness) Add env/domain/ServerLiveness, the single freshness-windowed liveness snapshot the reconciler effort collapses the four scattered sources into. Pure JVM: it holds the two observed facts (processPresent from /proc, servicesAnswering from /k2go-api) plus a monotonic observedAtMs, and derives one Phase (UNKNOWN/DOWN/STARTING/UP). Reuses Freshness.fresh for the one definition of "still trustworthy"; a never-observed or stale snapshot folds to UNKNOWN (absorbs hasObservation()). Route the 3s status poll through it (ServerController.checkServerStatus): build the snapshot from EnvironmentProcess + RestReadiness, publish alive = (phase == UP) to ServerStateRepository as before. The one behavior shift is that "up" now means the services answer (/k2go-api) rather than nginx (/home), which answers before its dash-node upstream is ready - so a flap no longer reads as spurious "up" (ADFA-5336). Downstream is otherwise unchanged: ServerState stays a 1-bit alive fact, no consumer touched. Delete the now-unused /home pingUrl and its two java.net imports. JVM tests cover the four-phase truth table and the freshness boundary (ServerLivenessTest). startEnvironment's own liveness reads are intentionally left for Phase 4; Phase 0 is additive. Verified: pure-domain javac compile + the 9 JVM tests pass offline. Full app compile + lint must run via Gradle; runtime (boot/up/down on Home; flap no longer shows spurious up) is device-only. Rollback: revert ServerController.java; the poll returns to the /home ping (the new ServerLiveness file is then unused). Co-Authored-By: Claude Opus 4.8 --- .../org/iiab/controller/ServerController.java | 31 +++-- .../controller/env/domain/ServerLiveness.java | 114 ++++++++++++++++++ .../env/domain/ServerLivenessTest.java | 82 +++++++++++++ 3 files changed, 210 insertions(+), 17 deletions(-) create mode 100644 controller/app/src/main/java/org/iiab/controller/env/domain/ServerLiveness.java create mode 100644 controller/app/src/test/java/org/iiab/controller/env/domain/ServerLivenessTest.java diff --git a/controller/app/src/main/java/org/iiab/controller/ServerController.java b/controller/app/src/main/java/org/iiab/controller/ServerController.java index 074203a2f..57f5f5340 100644 --- a/controller/app/src/main/java/org/iiab/controller/ServerController.java +++ b/controller/app/src/main/java/org/iiab/controller/ServerController.java @@ -27,8 +27,6 @@ import org.iiab.controller.util.AppExecutors; import java.io.File; -import java.net.HttpURLConnection; -import java.net.URL; public class ServerController { @@ -141,27 +139,26 @@ private void updateServerAlive(boolean nowAlive) { // The repository is updated by the poll (checkServerStatus) right after this. } - private boolean pingUrl(String urlStr) { - try { - URL url = new URL(urlStr); - HttpURLConnection conn = (HttpURLConnection) url.openConnection(); - conn.setUseCaches(false); - conn.setConnectTimeout(1500); - conn.setReadTimeout(1500); - conn.setRequestMethod("GET"); - return (conn.getResponseCode() >= 200 && conn.getResponseCode() < 400); - } catch (Exception e) { - return false; - } - } - // --- status poll ------------------------------------------------------------ private void checkServerStatus() { if (host.isNegotiating()) return; AppExecutors.get().io().execute(() -> { - boolean localAlive = pingUrl(BoxEndpoints.BASE + "/home"); + // ADFA-5343 (Phase 0): one honest liveness snapshot instead of a single /home ping. + // nginx answers /home before its dash-node upstream is ready, so a restarting engine read + // as "up" (the flap). servicesAnswering probes /k2go-api (the usable signal); processPresent + // (/proc) is recorded for the richer phase the reconciler will consume in a later phase. + // alive stays a 1-bit fact (phase == UP) so ServerStateRepository and every reader are + // unchanged here — the only shift is that "up" now means the services answer, not nginx. + long now = android.os.SystemClock.elapsedRealtime(); + org.iiab.controller.env.domain.ServerLiveness liveness = + org.iiab.controller.env.domain.ServerLiveness.of( + org.iiab.controller.env.EnvironmentProcess.isRunning(activity), + org.iiab.controller.redesign.RestReadiness.apiReady(), + now); + boolean localAlive = + liveness.phase(now) == org.iiab.controller.env.domain.ServerLiveness.Phase.UP; updateServerAlive(localAlive); diff --git a/controller/app/src/main/java/org/iiab/controller/env/domain/ServerLiveness.java b/controller/app/src/main/java/org/iiab/controller/env/domain/ServerLiveness.java new file mode 100644 index 000000000..833d75f8f --- /dev/null +++ b/controller/app/src/main/java/org/iiab/controller/env/domain/ServerLiveness.java @@ -0,0 +1,114 @@ +/* + * ============================================================================ + * Name : ServerLiveness.java + * Author : AppDevForAll + * Copyright : Copyright (c) 2026 AppDevForAll + * Description : ADFA-5343 (Phase 0). One honest snapshot of "is the server up?", + * replacing the four scattered liveness sources with a single + * freshness-windowed reading. Pure JVM (no android.*, no HTTP): it + * holds the two observed facts + when they were observed, and + * derives one phase. The impure probes (/proc, /k2go-api) are read + * by the caller and passed in, so this stays unit-testable on the + * JVM like EnvironmentEnsure and Freshness. + * ============================================================================ + */ +package org.iiab.controller.env.domain; + +import org.iiab.controller.env.Freshness; + +/** + * "Is the server up?", answered once, from two facts read one way. + * + *

The app used to answer this four different ways (a cached {@code /home} ping, a "have we polled + * yet?" bool, a fresh {@code /k2go-api} probe, and a {@code /proc} read), and reading {@code /home} + * as "up" is the root of the post-install flap: nginx answers {@code /home} before its dash-node + * upstream is ready, so a restarting engine still reads "up" (ADFA-5336). Here there is one snapshot: + * + *

    + *
  • {@code servicesAnswering} — the honest "usable" signal: dash-node {@code /k2go-api} answers, + * not merely nginx {@code /home}.
    • + *
  • {@code processPresent} — our environment proot is alive in {@code /proc}. The discriminator + * that tells "services down, environment alive" (still starting) from "environment gone" + * (down), the reason {@code EnvironmentProcess} exists (ADFA-5061).
    • + *
  • {@code observedAtMs} — a monotonic stamp ({@code SystemClock.elapsedRealtime}). A snapshot + * nobody has refreshed is not trusted as fact; it ages back to {@code UNKNOWN}.
    • + *
+ * + *

{@code UNKNOWN} absorbs the old "have we polled yet?" bool: a never-observed ({@code + * observedAtMs <= 0}) or a stale snapshot is {@code UNKNOWN}, not a false {@code DOWN}. + */ +public final class ServerLiveness { + + /** + * How long a snapshot is trusted before it ages back to {@code UNKNOWN}. The status poll refreshes + * every ~3 s (ServerController.CHECK_INTERVAL_MS), so three intervals means "the poll stopped + * feeding us" rather than a normal gap — long enough not to flap on one slow probe, short enough + * that a genuinely stale reading is not mistaken for a live one. + */ + public static final long DEFAULT_FRESH_MS = 9_000L; + + /** The derived state. One bit of "desired" is a separate concern (the reconciler); this is only + * the observed "actual". */ + public enum Phase { + /** No trustworthy observation: never polled, or the last snapshot has gone stale. */ + UNKNOWN, + /** Neither the proot nor the services are present. */ + DOWN, + /** The proot is up but the services do not answer yet (booting, within grace elsewhere). */ + STARTING, + /** The services answer {@code /k2go-api} — genuinely usable. */ + UP + } + + private final boolean processPresent; + private final boolean servicesAnswering; + private final long observedAtMs; + + private ServerLiveness(boolean processPresent, boolean servicesAnswering, long observedAtMs) { + this.processPresent = processPresent; + this.servicesAnswering = servicesAnswering; + this.observedAtMs = observedAtMs; + } + + /** + * @param processPresent our environment proot is alive (from {@code /proc}). + * @param servicesAnswering dash-node answers {@code /k2go-api}. + * @param observedAtMs when these were read, from a monotonic clock + * ({@code SystemClock.elapsedRealtime}); {@code 0} means "never". + */ + public static ServerLiveness of(boolean processPresent, boolean servicesAnswering, + long observedAtMs) { + return new ServerLiveness(processPresent, servicesAnswering, observedAtMs); + } + + public boolean processPresent() { return processPresent; } + public boolean servicesAnswering() { return servicesAnswering; } + public long observedAtMs() { return observedAtMs; } + + /** The phase using the default freshness window. */ + public Phase phase(long nowMs) { + return phase(nowMs, DEFAULT_FRESH_MS); + } + + /** + * The phase at {@code nowMs}. A never-observed ({@code observedAtMs <= 0}) or stale snapshot is + * {@code UNKNOWN} — {@link Freshness#fresh} is the one definition of "still trustworthy" (0 is + * never fresh, so both fold here). Otherwise {@code servicesAnswering} wins ({@code UP}); + * else a present proot is still {@code STARTING}; else {@code DOWN}. + * + * @param nowMs the same monotonic clock {@code observedAtMs} was stamped from. + * @param freshnessMs how long the snapshot stays trustworthy. + */ + public Phase phase(long nowMs, long freshnessMs) { + if (!Freshness.fresh(observedAtMs, nowMs, freshnessMs)) { + return Phase.UNKNOWN; + } + if (servicesAnswering) { + return Phase.UP; + } + if (processPresent) { + return Phase.STARTING; + } + return Phase.DOWN; + } +} diff --git a/controller/app/src/test/java/org/iiab/controller/env/domain/ServerLivenessTest.java b/controller/app/src/test/java/org/iiab/controller/env/domain/ServerLivenessTest.java new file mode 100644 index 000000000..ea8634388 --- /dev/null +++ b/controller/app/src/test/java/org/iiab/controller/env/domain/ServerLivenessTest.java @@ -0,0 +1,82 @@ +package org.iiab.controller.env.domain; + +import static org.junit.Assert.assertEquals; + +import org.junit.Test; + +/** + * Unit tests for {@link ServerLiveness}. + * + *

The four-way truth table plus the freshness boundary. The case that matters most is the flap + * ({@link #servicesDownWithProotUpIsStartingNotUp}): a restarting dash-node behind a live nginx must + * read {@code STARTING}, never {@code UP} — reading {@code /home} as "up" is exactly what the single + * {@code servicesAnswering} signal removes (ADFA-5336). + */ +public class ServerLivenessTest { + + private static final long NOW = 100_000L; + private static final long FRESH = ServerLiveness.DEFAULT_FRESH_MS; + + @Test + public void neverObservedIsUnknown() { + // observedAtMs == 0: the poll has not run — not a false DOWN (absorbs the old hasObservation()). + assertEquals(ServerLiveness.Phase.UNKNOWN, + ServerLiveness.of(false, false, 0L).phase(NOW, FRESH)); + assertEquals(ServerLiveness.Phase.UNKNOWN, + ServerLiveness.of(true, true, 0L).phase(NOW, FRESH)); + } + + @Test + public void servicesAnsweringIsUp() { + assertEquals(ServerLiveness.Phase.UP, + ServerLiveness.of(true, true, NOW).phase(NOW, FRESH)); + } + + @Test + public void servicesDownWithProotUpIsStartingNotUp() { + // The flap: proot alive, /k2go-api not answering yet. STARTING, never UP. + assertEquals(ServerLiveness.Phase.STARTING, + ServerLiveness.of(true, false, NOW).phase(NOW, FRESH)); + } + + @Test + public void nothingPresentIsDown() { + assertEquals(ServerLiveness.Phase.DOWN, + ServerLiveness.of(false, false, NOW).phase(NOW, FRESH)); + } + + @Test + public void servicesAnsweringWinsOverAbsentProot() { + // Defensive: /proc missed the proot but /k2go-api answers — the usable signal wins, still UP. + assertEquals(ServerLiveness.Phase.UP, + ServerLiveness.of(false, true, NOW).phase(NOW, FRESH)); + } + + @Test + public void aFreshWindowedSnapshotWithinTheWindowIsTrusted() { + // Stamped FRESH-1 ms ago: still trustworthy, so the observed facts stand (UP). + assertEquals(ServerLiveness.Phase.UP, + ServerLiveness.of(true, true, NOW - (FRESH - 1)).phase(NOW, FRESH)); + } + + @Test + public void theFreshnessBoundaryStillTrustsAtExactlyTheWindow() { + // At exactly the window it is still fresh (Freshness.fresh uses <=), so UP holds. + assertEquals(ServerLiveness.Phase.UP, + ServerLiveness.of(true, true, NOW - FRESH).phase(NOW, FRESH)); + } + + @Test + public void aStaleSnapshotAgesBackToUnknown() { + // One ms past the window: the poll stopped feeding us — UNKNOWN, not a stale true "UP". + assertEquals(ServerLiveness.Phase.UNKNOWN, + ServerLiveness.of(true, true, NOW - (FRESH + 1)).phase(NOW, FRESH)); + } + + @Test + public void defaultWindowMatchesTheExplicitOne() { + // The convenience phase(now) uses DEFAULT_FRESH_MS. + ServerLiveness live = ServerLiveness.of(true, false, NOW); + assertEquals(live.phase(NOW, ServerLiveness.DEFAULT_FRESH_MS), live.phase(NOW)); + } +} From 7dde068be39660c3a0eb4215b297106176252a38 Mon Sep 17 00:00:00 2001 From: Luis Guzman Date: Fri, 28 Aug 2026 22:24:42 -0600 Subject: [PATCH 03/10] ADFA-5343 feat(server-lifecycle): Phase 1 - log-only reconciler + desired-state Introduce the one process-scoped owner of "the server should be up", first as a LOG-ONLY observer (no actuation), plus the pure decisions it will own. - ServerReconcile (pure JVM): desired() = installed && healthy && userWantsOn && holderClass != STOPPED - a pure function of facts that already have owners, so no new source of truth. intent() = the coarse direction (START/STOP/WAIT/HOLD/NOOP) from desired vs the observed ServerLiveness.Phase; an UNKNOWN phase always HOLDs. - EnvironmentLock.Holder now carries its Operation.ExecutionClass (ADR-5061's one LIVE/STOPPED type, reused - not a parallel enum). This lands the approved Task-1 predicate fix in code: LIVE holders (download, dashboard self-update) leave desired UP; only STOPPED holders (clone/backup/restore/install) pull it down; NONE is LIVE. - ServerLifecycleReconciler (app-scoped singleton, established in IIABApplication): observe(ctx, liveness) computes desired from SystemFactsReader + WatchdogEnable + currentHolder and logs desired-vs-actual -> wouldDo each tick. No actuation. It is FED the Phase-0 snapshot by the status poll, so there is no second liveness source. JVM tests: ServerReconcileTest covers the desired truth table (the holder-class dimension in particular) and the intent table. Verified: :app:testDebugUnitTest + :app:lintDebug BUILD SUCCESSFUL locally. Device- only: logcat "K2Go-Reconciler" on all five flows (boot, module hand-off, deep ops, dashboard rebuild, turn-off) should show desired/actual/wouldDo matching reality with no surprises. Still additive - the scorecard reduction lands when Phase 4 deletes the scaffolding these parts replace. Rollback: delete ServerReconcile + ServerLifecycleReconciler and their two call sites (the ServerController poll seam and the IIABApplication warm-up); revert the Holder field. Nothing depends on the observer. Co-Authored-By: Claude Opus 4.8 --- .../org/iiab/controller/IIABApplication.java | 3 + .../org/iiab/controller/ServerController.java | 5 + .../iiab/controller/env/EnvironmentLock.java | 26 ++++- .../env/ServerLifecycleReconciler.java | 94 ++++++++++++++++ .../env/domain/ServerReconcile.java | 95 ++++++++++++++++ .../env/domain/ServerReconcileTest.java | 106 ++++++++++++++++++ 6 files changed, 327 insertions(+), 2 deletions(-) create mode 100644 controller/app/src/main/java/org/iiab/controller/env/ServerLifecycleReconciler.java create mode 100644 controller/app/src/main/java/org/iiab/controller/env/domain/ServerReconcile.java create mode 100644 controller/app/src/test/java/org/iiab/controller/env/domain/ServerReconcileTest.java diff --git a/controller/app/src/main/java/org/iiab/controller/IIABApplication.java b/controller/app/src/main/java/org/iiab/controller/IIABApplication.java index 62ff3a87c..fe3349335 100644 --- a/controller/app/src/main/java/org/iiab/controller/IIABApplication.java +++ b/controller/app/src/main/java/org/iiab/controller/IIABApplication.java @@ -29,6 +29,9 @@ public void onCreate() { super.onCreate(); // ADFA-4640: wire up persistence for the server log (survives app restarts). org.iiab.controller.LogRepository.get().init(this); + // ADFA-5343 (Phase 1): establish the app-scoped server-lifecycle reconciler. Log-only for now, + // fed by the status poll; it gains its own tick + WatchdogService promotion in a later phase. + org.iiab.controller.env.ServerLifecycleReconciler.get(); // We inject Conscrypt as the app's primary security provider try { Security.insertProviderAt(Conscrypt.newProvider(), 1); diff --git a/controller/app/src/main/java/org/iiab/controller/ServerController.java b/controller/app/src/main/java/org/iiab/controller/ServerController.java index 57f5f5340..cfff31cbc 100644 --- a/controller/app/src/main/java/org/iiab/controller/ServerController.java +++ b/controller/app/src/main/java/org/iiab/controller/ServerController.java @@ -167,6 +167,11 @@ private void checkServerStatus() { final SystemState sysState = SystemStateEvaluator.evaluate(activity, localAlive); ServerStateRepository.get().post(ServerState.of(localAlive, sysState)); + // ADFA-5343 (Phase 1): feed the same snapshot to the log-only reconciler — no second liveness + // source, no actuation. It logs desired-vs-actual each poll. Removing this line + the class is + // the full rollback. + org.iiab.controller.env.ServerLifecycleReconciler.get().observe(activity, liveness); + // STATE MACHINE: Has the target state been reached? Boolean target = host.getTargetServerState(); if (target != null && ServerStateRepository.get().current().alive == target) { diff --git a/controller/app/src/main/java/org/iiab/controller/env/EnvironmentLock.java b/controller/app/src/main/java/org/iiab/controller/env/EnvironmentLock.java index a20abdc18..f813dfd91 100644 --- a/controller/app/src/main/java/org/iiab/controller/env/EnvironmentLock.java +++ b/controller/app/src/main/java/org/iiab/controller/env/EnvironmentLock.java @@ -38,6 +38,8 @@ import android.content.Context; +import org.iiab.controller.system.domain.Operation; + import java.io.BufferedReader; import java.io.File; import java.io.FileReader; @@ -51,8 +53,28 @@ public enum Owner { INSTALL, MODULE, BACKUP, RESTORE, CLONE } /** ADFA-5146: what is actually holding the environment, for a refusal message that names the * real cause instead of always saying "an install". ADFA-5333: DASHBOARD = a live dash-node update - * is in flight; it restarts the server, so nothing that touches the server may start on top of it. */ - public enum Holder { CLONE, BACKUP, RESTORE, INSTALL, DOWNLOAD, DASHBOARD, NONE } + * is in flight; it restarts the server, so nothing that touches the server may start on top of it. + * + *

ADFA-5343: each holder carries its {@link Operation.ExecutionClass} — reusing ADR-5061's one + * LIVE/STOPPED type, not a parallel one. STOPPED holders run the box down (pdsm stop + a transient + * proot: clone/backup/restore/install); LIVE holders run against the live server (download, dashboard + * self-update). NONE means no holder is forcing the box down. The server-lifecycle desired-state + * predicate reads this: desired stays UP unless a STOPPED-class holder is in force. */ + public enum Holder { + CLONE(Operation.ExecutionClass.STOPPED), + BACKUP(Operation.ExecutionClass.STOPPED), + RESTORE(Operation.ExecutionClass.STOPPED), + INSTALL(Operation.ExecutionClass.STOPPED), + DOWNLOAD(Operation.ExecutionClass.LIVE), + DASHBOARD(Operation.ExecutionClass.LIVE), + NONE(Operation.ExecutionClass.LIVE); + + public final Operation.ExecutionClass executionClass; + + Holder(Operation.ExecutionClass executionClass) { + this.executionClass = executionClass; + } + } // Owner marker: line 1 = Owner.name(), line 2 = epoch millis, line 3 = session token. private static final String MARKER = ".env_lock"; diff --git a/controller/app/src/main/java/org/iiab/controller/env/ServerLifecycleReconciler.java b/controller/app/src/main/java/org/iiab/controller/env/ServerLifecycleReconciler.java new file mode 100644 index 000000000..a39c09884 --- /dev/null +++ b/controller/app/src/main/java/org/iiab/controller/env/ServerLifecycleReconciler.java @@ -0,0 +1,94 @@ +/* + * ============================================================================ + * Name : ServerLifecycleReconciler.java + * Author : AppDevForAll + * Copyright : Copyright (c) 2026 AppDevForAll + * Description : ADFA-5343 (Phase 1). The app-scoped owner of the server lifecycle, + * introduced first as a LOG-ONLY observer. Each tick it computes the + * desired state from facts that already have owners and logs the action + * it WOULD take against the observed liveness — it does NOT actuate. + * Actuation, its own tick, and WatchdogService promotion arrive in + * later phases; this phase only proves the desired-vs-actual reasoning + * matches reality on every flow, at zero risk. + * ============================================================================ + */ +package org.iiab.controller.env; + +import android.content.Context; +import android.os.SystemClock; +import android.util.Log; + +import org.iiab.controller.Preferences; +import org.iiab.controller.env.domain.ServerLiveness; +import org.iiab.controller.env.domain.ServerReconcile; +import org.iiab.controller.system.data.SystemFactsReader; +import org.iiab.controller.system.domain.Operation; +import org.iiab.controller.system.domain.SystemFacts; + +/** + * One process-scoped owner of "the server should be up," observing only (Phase 1). + * + *

It is fed the single {@link ServerLiveness} snapshot the status poll already builds (Phase 0), + * so there is no second liveness source; it reads the desired-state inputs itself — all facts that + * already have owners — and logs {@code desired vs actual → wouldDo} each tick. Nothing downstream + * depends on it, so the whole phase reverts by deleting this class and its two call sites (the poll + * seam and the IIABApplication warm-up). + */ +public final class ServerLifecycleReconciler { + + private static final String TAG = "K2Go-Reconciler"; + + private static final ServerLifecycleReconciler INSTANCE = new ServerLifecycleReconciler(); + + public static ServerLifecycleReconciler get() { + return INSTANCE; + } + + private volatile ServerLiveness lastLiveness; + private volatile boolean lastDesiredUp; + + private ServerLifecycleReconciler() { + } + + /** + * A tick: compute desired, compare to the observed liveness, and log the action that would follow. + * No actuation in Phase 1. Called on the poll's worker thread; {@code synchronized} keeps the + * "single-threaded tick" invariant if two polls ever overlap. + * + * @param ctx any context (the poll's Activity today). + * @param liveness the snapshot the poll just read — reused, not re-probed. + */ + public synchronized void observe(Context ctx, ServerLiveness liveness) { + if (ctx == null || liveness == null) { + return; + } + ServerLiveness.Phase actual = liveness.phase(SystemClock.elapsedRealtime()); + + SystemFacts facts = SystemFactsReader.read(ctx); + boolean userWantsOn = new Preferences(ctx).getWatchdogEnable(); + EnvironmentLock.Holder holder = EnvironmentLock.currentHolder(ctx); + Operation.ExecutionClass holderClass = holder.executionClass; + + boolean desiredUp = ServerReconcile.desired( + facts.isInstalled(), facts.isHealthy(), userWantsOn, holderClass); + ServerReconcile.Intent wouldDo = ServerReconcile.intent(desiredUp, actual); + + this.lastLiveness = liveness; + this.lastDesiredUp = desiredUp; + + Log.i(TAG, "ADFA-5343 reconcile (log-only): desired=" + (desiredUp ? "UP" : "DOWN") + + " actual=" + actual + " wouldDo=" + wouldDo + + " [installed=" + facts.isInstalled() + " healthy=" + facts.isHealthy() + + " userWantsOn=" + userWantsOn + " holder=" + holder + "/" + holderClass + "]"); + } + + /** The last snapshot observed, or null before the first tick. For later phases / diagnostics. */ + public ServerLiveness lastLiveness() { + return lastLiveness; + } + + /** The last desired verdict. For later phases / diagnostics. */ + public boolean lastDesiredUp() { + return lastDesiredUp; + } +} diff --git a/controller/app/src/main/java/org/iiab/controller/env/domain/ServerReconcile.java b/controller/app/src/main/java/org/iiab/controller/env/domain/ServerReconcile.java new file mode 100644 index 000000000..b5b85083e --- /dev/null +++ b/controller/app/src/main/java/org/iiab/controller/env/domain/ServerReconcile.java @@ -0,0 +1,95 @@ +/* + * ============================================================================ + * Name : ServerReconcile.java + * Author : AppDevForAll + * Copyright : Copyright (c) 2026 AppDevForAll + * Description : ADFA-5343. The two pure decisions the server-lifecycle reconciler + * makes: (1) desired — should the server be up? — a pure function of + * facts that already have owners, and (2) intent — given desired vs + * the observed liveness phase, which direction would the owner move. + * No android.*, so both are unit-tested on the JVM like + * EnvironmentEnsure and Freshness. + * ============================================================================ + */ +package org.iiab.controller.env.domain; + +import org.iiab.controller.system.domain.Operation; + +/** + * "Should the server be up, and what would we do about it" — decided in one place. + * + *

{@link #desired} is the process-scoped truth the app was missing: one owner of "the server + * should be up," derived from facts that already have owners (installed/healthy from the fact reader, + * the persisted user intent, and which operation holds the environment). It creates no new source of + * truth. {@link #intent} is the coarse direction the reconciler would move given that desire and the + * observed {@link ServerLiveness.Phase}. + */ +public final class ServerReconcile { + + private ServerReconcile() { + } + + /** The direction the reconciler would move on a tick. Log-only in Phase 1; the actuator acts on it + * in a later phase, refining a {@code START} into the safe how (launch / wait-grace / kill-orphan) + * via {@link EnvironmentEnsure} — so this gates the direction, that decides the mechanics. */ + public enum Intent { + /** desired up, nothing running — would launch. */ + START, + /** desired down, something running — would stop. */ + STOP, + /** desired up, still coming up — leave it to finish. */ + WAIT, + /** liveness not yet trustworthy (UNKNOWN) — never act on an unobserved/stale snapshot. */ + HOLD, + /** actual already matches desired — nothing to do. */ + NOOP + } + + /** + * Should the server be up? Up iff the system is present and whole, the user wants it on, and + * no STOPPED-class holder is forcing it down. LIVE-class holders (a live download, a dashboard + * self-update) run against the live server, so they leave desired UP; only STOPPED holders + * (clone/backup/restore/install, which pdsm-stop the box) pull it down. + * + * @param installed a rootfs is present and no install is running over it. + * @param healthy the last install was not left half-finished. + * @param userWantsOn the persisted user intent (today {@code Preferences.WatchdogEnable}). + * @param holderClass the execution class of the current environment holder + * ({@code EnvironmentLock.currentHolder().executionClass}); {@code NONE} is LIVE. + */ + public static boolean desired(boolean installed, boolean healthy, boolean userWantsOn, + Operation.ExecutionClass holderClass) { + return installed && healthy && userWantsOn + && holderClass != Operation.ExecutionClass.STOPPED; + } + + /** + * The coarse direction to move, given desired vs the observed phase. An {@code UNKNOWN} phase always + * yields {@link Intent#HOLD}: a never-observed or stale snapshot is not a fact to act on. + */ + public static Intent intent(boolean desiredUp, ServerLiveness.Phase actual) { + if (actual == ServerLiveness.Phase.UNKNOWN) { + return Intent.HOLD; + } + if (desiredUp) { + switch (actual) { + case UP: + return Intent.NOOP; + case STARTING: + return Intent.WAIT; + case DOWN: + default: + return Intent.START; + } + } + // desired down + switch (actual) { + case DOWN: + return Intent.NOOP; + case UP: + case STARTING: + default: + return Intent.STOP; + } + } +} diff --git a/controller/app/src/test/java/org/iiab/controller/env/domain/ServerReconcileTest.java b/controller/app/src/test/java/org/iiab/controller/env/domain/ServerReconcileTest.java new file mode 100644 index 000000000..c6df5613f --- /dev/null +++ b/controller/app/src/test/java/org/iiab/controller/env/domain/ServerReconcileTest.java @@ -0,0 +1,106 @@ +package org.iiab.controller.env.domain; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; + +import org.iiab.controller.system.domain.Operation; + +import org.junit.Test; + +/** + * Unit tests for {@link ServerReconcile}. + * + *

Two tables. {@code desired} is the process-scoped "should it be up?" — the case that matters is + * the holder-class dimension (ADFA-5343): a LIVE holder (download / dashboard self-update) must leave + * desired UP, only a STOPPED holder pulls it down. {@code intent} is the coarse direction, whose one + * rule worth stating is that an UNKNOWN phase never provokes an action. + */ +public class ServerReconcileTest { + + private static final Operation.ExecutionClass LIVE = Operation.ExecutionClass.LIVE; + private static final Operation.ExecutionClass STOPPED = Operation.ExecutionClass.STOPPED; + + // --- desired ----------------------------------------------------------------- + + @Test + public void desiredUpWhenUsableWantedAndNoStoppedHolder() { + assertTrue(ServerReconcile.desired(true, true, true, LIVE)); + } + + @Test + public void aStoppedHolderForcesDesiredDown() { + // clone / backup / restore / install pdsm-stop the box: desired must be DOWN even if wanted on. + assertFalse(ServerReconcile.desired(true, true, true, STOPPED)); + } + + @Test + public void aLiveHolderLeavesDesiredUp() { + // The Task-1 fix: a live download / dashboard self-update runs against the live server, so it + // must NOT force the server down (the old currentHolder == NONE predicate got this wrong). + assertTrue(ServerReconcile.desired(true, true, true, LIVE)); + } + + @Test + public void notInstalledIsNeverDesiredUp() { + assertFalse(ServerReconcile.desired(false, true, true, LIVE)); + } + + @Test + public void unhealthyIsNeverDesiredUp() { + // Installed but half-finished: the recovery dialog's world, nothing should run against it. + assertFalse(ServerReconcile.desired(true, false, true, LIVE)); + } + + @Test + public void userOffIsNeverDesiredUp() { + assertFalse(ServerReconcile.desired(true, true, false, LIVE)); + } + + // --- intent ------------------------------------------------------------------ + + @Test + public void unknownPhaseAlwaysHolds() { + assertEquals(ServerReconcile.Intent.HOLD, + ServerReconcile.intent(true, ServerLiveness.Phase.UNKNOWN)); + assertEquals(ServerReconcile.Intent.HOLD, + ServerReconcile.intent(false, ServerLiveness.Phase.UNKNOWN)); + } + + @Test + public void desiredUpStartsWhenDown() { + assertEquals(ServerReconcile.Intent.START, + ServerReconcile.intent(true, ServerLiveness.Phase.DOWN)); + } + + @Test + public void desiredUpWaitsWhileStarting() { + assertEquals(ServerReconcile.Intent.WAIT, + ServerReconcile.intent(true, ServerLiveness.Phase.STARTING)); + } + + @Test + public void desiredUpIsNoopWhenUp() { + assertEquals(ServerReconcile.Intent.NOOP, + ServerReconcile.intent(true, ServerLiveness.Phase.UP)); + } + + @Test + public void desiredDownStopsWhenUp() { + assertEquals(ServerReconcile.Intent.STOP, + ServerReconcile.intent(false, ServerLiveness.Phase.UP)); + } + + @Test + public void desiredDownStopsWhileStarting() { + // Caught mid-boot but no longer wanted (a holder just acquired): still STOP. + assertEquals(ServerReconcile.Intent.STOP, + ServerReconcile.intent(false, ServerLiveness.Phase.STARTING)); + } + + @Test + public void desiredDownIsNoopWhenDown() { + assertEquals(ServerReconcile.Intent.NOOP, + ServerReconcile.intent(false, ServerLiveness.Phase.DOWN)); + } +} From 1f5cb0f64f2f1c34da2f0f19d41cc432cdc5ff8d Mon Sep 17 00:00:00 2001 From: Luis Guzman Date: Fri, 28 Aug 2026 22:54:03 -0600 Subject: [PATCH 04/10] ADFA-5343 feat(server-lifecycle): Phase 2 - reconciler actuates the module hand-off (5336) First actuation. The module-batch hand-off stops booting + polling the server itself and instead sets the desired state; the reconciler brings the box up and keeps it up, re-driving a post-install flap so the hand-off can no longer strand a dead Home (5336). Actuation via a bridge (A1), not a new boot path: - ServerLifecycleReconciler gains an Actuator interface. The foregrounded ServerController registers on onResume and clears on onPause (compare-and-clear, idempotent to a resume/pause overlap). The reconciler owns the DECISION (desired); the boot MECHANISM stays the one existing, idempotent path (ServerController.startEnvironment via EnvironmentEnsure). No second actuator, no double-proot. Off-UI boot + own tick are Phase 4. - On a tick, when desired=UP and the box is not confirmed up, the reconciler calls ensureServerUp() through the registered actuator. START (down) and WAIT (coming up / stuck flap) both route there (ServerReconcile.ensuresUp); startEnvironment's EnvironmentEnsure then launches / leaves-in-grace / relaunches-stuck, so a healthy boot is never disturbed and a past-grace flap is re-driven. STOP stays with the toggle / deep-ops until Phase 4; desired=DOWN never yields START/WAIT, so the reconciler cannot fight a legitimate stop, and it will not boot during runroles (holder=INSTALL => desired DOWN). Gated by ServerLifecycleReconciler.ACTUATES (the rollback lever). Hand-off rewired to observe, not poll (SetupProgressActivity): - Removed the three boot latches (moduleRestartKicked / moduleServerUp / moduleServerFailed) and the copy-pasted apiReady re-boot loop (serverUpPoll), plus ensureServerUpForModules. - "up" is now read from the one observed phase (serverObservedUp() over ServerStateRepository); "didn't come back in time" is a timeout on that phase (moduleServerWaitAt). There is no FAILED phase, so a stuck start is STARTING that the reconciler keeps re-driving and the UI shows "taking longer" + Finish - and Finish lands on a Home the reconciler drives live. - onModuleBatchTerminal() sets userWantsOn once (desired=UP); InstallGuard is already cleared and the queue stopped before DONE, so the holder is NONE at that point. Rollback path (ACTUATES=false) boots once here as before. JVM tests: ServerReconcileTest adds the ensuresUp mapping (START/WAIT drive up; NOOP/STOP/ HOLD do not). Verified: :app:testDebugUnitTest + :app:lintDebug BUILD SUCCESSFUL locally. Behavior changes to verify on device: - Reconciler now boots via ANY foregrounded Activity when desired=UP and the box is down (generalizes keep-up; fixes flap on Home). Safe: single idempotent actuator, desired-gated. - A module batch now sets WatchdogEnable (the persisted "server on" intent) true. - Redirect "up" detected via the 3s poll observation (was a 2s bespoke poll) - up to 3s later. Device matrix (must include the timeout branch, not only success): - module install + hand-off -> live Home; - module install + induced dash-node flap post-DONE -> auto-recovers, no manual toggle (5336); - module install where services are slow/never answer -> "taking longer" + Finish; reconciler keeps trying; Finish -> Home stays driven; no dead Home; - runroles in flight -> reconciler does NOT boot (holder STOPPED); - ACTUATES=false -> hand-off boots itself, reconciler log-only (rollback). Rollback: set ServerLifecycleReconciler.ACTUATES=false. Co-Authored-By: Claude Opus 4.8 --- .../org/iiab/controller/ServerController.java | 15 +- .../env/ServerLifecycleReconciler.java | 54 +++++++- .../env/domain/ServerReconcile.java | 11 ++ .../redesign/SetupProgressActivity.java | 128 +++++++++--------- .../env/domain/ServerReconcileTest.java | 16 +++ 5 files changed, 157 insertions(+), 67 deletions(-) diff --git a/controller/app/src/main/java/org/iiab/controller/ServerController.java b/controller/app/src/main/java/org/iiab/controller/ServerController.java index cfff31cbc..2057880e3 100644 --- a/controller/app/src/main/java/org/iiab/controller/ServerController.java +++ b/controller/app/src/main/java/org/iiab/controller/ServerController.java @@ -28,7 +28,7 @@ import java.io.File; -public class ServerController { +public class ServerController implements org.iiab.controller.env.ServerLifecycleReconciler.Actuator { private static final String TAG = "IIAB-ServerController"; private static final int CHECK_INTERVAL_MS = 3000; @@ -114,10 +114,23 @@ public void onResume() { updateConnectivityStatus(); // instant refresh when returning to the app serverCheckHandler.removeCallbacks(serverCheckRunnable); serverCheckHandler.post(serverCheckRunnable); + // ADFA-5343 (Phase 2): register as the foreground actuator the reconciler drives. Whichever + // Activity is resumed owns this slot; the reconciler boots through it (the one existing boot). + org.iiab.controller.env.ServerLifecycleReconciler.get().setActuator(this); } public void onPause() { serverCheckHandler.removeCallbacks(serverCheckRunnable); + // ADFA-5343 (Phase 2): release the actuator slot — but clearActuator only clears if we still + // hold it, so a resume/pause overlap does not wipe the next Activity's registration. + org.iiab.controller.env.ServerLifecycleReconciler.get().clearActuator(this); + } + + /** ADFA-5343 (Phase 2): the reconciler's boot entry point. Delegates to the one existing, idempotent + * boot path so no second actuator is introduced. */ + @Override + public void ensureServerUp() { + startEnvironment(); } public String getCurrentTargetUrl() { return currentTargetUrl; } diff --git a/controller/app/src/main/java/org/iiab/controller/env/ServerLifecycleReconciler.java b/controller/app/src/main/java/org/iiab/controller/env/ServerLifecycleReconciler.java index a39c09884..b9b5e1bc7 100644 --- a/controller/app/src/main/java/org/iiab/controller/env/ServerLifecycleReconciler.java +++ b/controller/app/src/main/java/org/iiab/controller/env/ServerLifecycleReconciler.java @@ -44,12 +44,50 @@ public static ServerLifecycleReconciler get() { return INSTANCE; } + /** + * ADFA-5343 (Phase 2): master switch for actuation. {@code true} = the reconciler drives the server + * up through the foreground actuator; {@code false} = log-only (Phase 1) and the hand-off boots + * itself. The rollback lever for the first actuation — flip to {@code false} to revert behavior + * without reverting code. + */ + public static final boolean ACTUATES = true; + + /** + * ADFA-5343 (Phase 2): the single actuator the reconciler drives — the foregrounded + * {@code ServerController}, which registers on resume. The reconciler owns the DECISION (desired); + * the boot MECHANISM stays in the one existing, tested path + * ({@code ServerController.startEnvironment} via {@code EnvironmentEnsure}) until Phase 4 relocates + * it off-UI and deletes the toggle. No second boot path is introduced. + */ + public interface Actuator { + /** Ensure the server is up. Idempotent and self-gating: a no-op if already up or still inside + * its boot grace, a relaunch only for a stuck past-grace proot. */ + void ensureServerUp(); + } + private volatile ServerLiveness lastLiveness; private volatile boolean lastDesiredUp; + private volatile Actuator actuator; private ServerLifecycleReconciler() { } + /** The foregrounded {@code ServerController} registers here in {@code onResume}. */ + public synchronized void setActuator(Actuator a) { + this.actuator = a; + } + + /** + * Cleared in {@code onPause} — but only if {@code a} is still the current one, so a resume/pause + * overlap (the next Activity registered before the previous paused) does not clear the live + * registration. Idempotent. + */ + public synchronized void clearActuator(Actuator a) { + if (this.actuator == a) { + this.actuator = null; + } + } + /** * A tick: compute desired, compare to the observed liveness, and log the action that would follow. * No actuation in Phase 1. Called on the poll's worker thread; {@code synchronized} keeps the @@ -76,10 +114,24 @@ public synchronized void observe(Context ctx, ServerLiveness liveness) { this.lastLiveness = liveness; this.lastDesiredUp = desiredUp; - Log.i(TAG, "ADFA-5343 reconcile (log-only): desired=" + (desiredUp ? "UP" : "DOWN") + // ADFA-5343 (Phase 2): actuate only the "bring it up" direction. START (down) and WAIT (still + // coming up, or a stuck flap) both route to ensureServerUp(); its EnvironmentEnsure decides + // launch / leave-in-grace / relaunch-stuck, so a healthy boot is never disturbed and a + // past-grace flap is re-driven wherever the app is foregrounded (the 5336 fix). STOP stays with + // the toggle / deep-ops until Phase 4; and desired=DOWN yields neither START nor WAIT, so the + // reconciler can never fight a legitimate stop. + boolean ensureUp = ACTUATES && ServerReconcile.ensuresUp(wouldDo); + Actuator a = ensureUp ? this.actuator : null; + + Log.i(TAG, "ADFA-5343 reconcile: desired=" + (desiredUp ? "UP" : "DOWN") + " actual=" + actual + " wouldDo=" + wouldDo + + (ensureUp ? (a != null ? " -> ensureServerUp()" : " -> (no foreground actuator)") : "") + " [installed=" + facts.isInstalled() + " healthy=" + facts.isHealthy() + " userWantsOn=" + userWantsOn + " holder=" + holder + "/" + holderClass + "]"); + + if (a != null) { + a.ensureServerUp(); + } } /** The last snapshot observed, or null before the first tick. For later phases / diagnostics. */ diff --git a/controller/app/src/main/java/org/iiab/controller/env/domain/ServerReconcile.java b/controller/app/src/main/java/org/iiab/controller/env/domain/ServerReconcile.java index b5b85083e..9f8fcf07b 100644 --- a/controller/app/src/main/java/org/iiab/controller/env/domain/ServerReconcile.java +++ b/controller/app/src/main/java/org/iiab/controller/env/domain/ServerReconcile.java @@ -92,4 +92,15 @@ public static Intent intent(boolean desiredUp, ServerLiveness.Phase actual) { return Intent.STOP; } } + + /** + * Whether an {@link Intent} means "drive the server up now". {@code START} (down) and {@code WAIT} + * (still coming up, or a stuck flap) both do — the reconciler routes both to the idempotent, + * self-gating boot, which decides launch / leave-in-grace / relaunch-stuck. {@code NOOP} / {@code + * STOP} / {@code HOLD} do not (STOP is owned elsewhere until a later phase; HOLD never acts on an + * unobserved snapshot). + */ + public static boolean ensuresUp(Intent intent) { + return intent == Intent.START || intent == Intent.WAIT; + } } diff --git a/controller/app/src/main/java/org/iiab/controller/redesign/SetupProgressActivity.java b/controller/app/src/main/java/org/iiab/controller/redesign/SetupProgressActivity.java index 42b5a0c00..ac9c815f1 100644 --- a/controller/app/src/main/java/org/iiab/controller/redesign/SetupProgressActivity.java +++ b/controller/app/src/main/java/org/iiab/controller/redesign/SetupProgressActivity.java @@ -122,10 +122,12 @@ public class SetupProgressActivity extends AppCompatActivity implements org.iiab // just to issue that start; the server proot is process-scoped so it survives into LibraryActivity. private org.iiab.controller.ServerController serverController; private Boolean targetServerState = null; // ServerController.Host state - private boolean moduleRestartKicked = false; // handleServerLaunchClick issued once - private boolean moduleServerUp = false; // REST core answered after the restart - private boolean moduleServerFailed = false; // restart timed out — surface as failure, not silent success - private long moduleRestartAt = 0L; // elapsedRealtime when the restart was kicked + // ADFA-5343 (Phase 2): the post-batch server restart is owned by the reconciler now, not this screen. + // The three boot latches (moduleRestartKicked/moduleServerUp/moduleServerFailed) are gone: "up" is + // read from the one observed server phase (serverObservedUp()), and "didn't come back in time" is a + // timeout on that phase anchored below — there is no FAILED phase, so a stuck flap is STARTING the + // reconciler keeps re-driving, and Finish still lands on a Home the reconciler drives live (5336). + private long moduleServerWaitAt = 0L; // elapsedRealtime when the post-batch wait began (0 = not yet) private org.iiab.controller.util.EllipsisAnimator statusEllipsis; // ADFA-4842: animated "…" on the amber wait line private int px(int dp) { return Math.round(dp * getResources().getDisplayMetrics().density); } @@ -236,7 +238,6 @@ protected void onResume() { protected void onPause() { super.onPause(); main.removeCallbacks(readyPoll); - main.removeCallbacks(serverUpPoll); // ADFA-4842 if (statusEllipsis != null) statusEllipsis.stop(); // ADFA-4842 cancelRedirect(); if (serverController != null) serverController.onPause(); // ADFA-4842: stop the status poll (not the server) @@ -291,16 +292,24 @@ public void onBackPressed() { /** ADFA-4919/4842: is a proot stage still blocking the index? True while a runrole is queued/running * AND, for a module batch, through the post-DONE server restart — the user must not background or * Back out (there is no "Run in background" for proot) until the server is confirmed back up - * (moduleServerUp) or the wait times out. */ + * (serverObservedUp) or the wait times out. */ private boolean prootActive() { ModuleQueueState mq = ModuleQueueRepository.get().current(); boolean mapsTerminal = mapsStartFailed || (mapsInSession() && mq.phase == ModuleQueueState.Phase.DONE); boolean mapsActive = mapsInSession() && !mapsTerminal; // A module session stays active from its runroles through the server restart that follows. - boolean moduleActive = (moduleInSession() || moduleStartFailed) && !moduleServerUp; + boolean moduleActive = (moduleInSession() || moduleStartFailed) && !serverObservedUp(); return mapsActive || moduleActive; } + /** ADFA-5343 (Phase 2): the server is observed up — the 3s poll saw /k2go-api answer. Replaces the + * moduleServerUp boot latch: the fact is read from the one published observation + * ({@link org.iiab.controller.ServerStateRepository}), not tracked per screen. */ + private boolean serverObservedUp() { + return org.iiab.controller.ServerStateRepository.get().hasObservation() + && org.iiab.controller.ServerStateRepository.get().current().alive; + } + /** ADFA-4919: is the proot (maps) stage part of THIS install session? Latched from the DURABLE * module queue (app-scoped) + wishlist, so a fresh index instance — e.g. reopened from the * notification while the queue is still running — still shows the stage, hides "Run in @@ -401,12 +410,13 @@ private boolean rebuildInSession() { // ADFA-4842: a MODULE (solo-proot) install stops the server and runs its OWN proot — there is // no REST engine to wait for, and we must NEVER try to "start services" (a second proot) mid- // runrole. Skip the REST readiness gate entirely: the runrole queue drives progress, and the - // server is (re)started only AFTER the queue is DONE (ensureServerUpForModules, via render()). - // REST and REST+proot (mixed) keep their serialized apiReady path below, untouched. + // server is (re)started only AFTER the queue is DONE — now by the reconciler (desired=UP), + // observed here via render(). REST and REST+proot (mixed) keep their serialized apiReady path + // below, untouched. if (moduleInSession()) { orchestrateStep(); // drains on first entry; harmless no-op once the queue is running render(); - if (!moduleServerUp) main.postDelayed(readyPoll, READY_POLL_MS); + if (!serverObservedUp()) main.postDelayed(readyPoll, READY_POLL_MS); return; } // ADFA-5074: nothing to start means nothing to wait for. The readiness probe exists so @@ -648,15 +658,27 @@ private void render() { // ADFA-4919: a proot module is queued/running (the gate is active). boolean prootActive = prootActive(); if (contextText != null) contextText.setText(prootActive ? R.string.k2go_setup_context_proot : R.string.k2go_setup_context); - // ADFA-4842: a terminal MODULE batch stopped the server for its runroles, so the server must be - // brought back before we can finish. Kick that here for ANY terminal module session — - // independent of the noRest/REST branch below — so ensureServerUpForModules() (and its 45s - // safety timeout) always runs and moduleServerUp / prootActive() can never hang. Idempotent. + // ADFA-4842/5343: a terminal MODULE batch stopped the server for its runroles, so the server must + // come back before we can finish. Record the intent here for ANY terminal module session — + // independent of the noRest/REST branch below — so onModuleBatchTerminal() (which sets desired=UP + // and anchors the wait timeout) always runs and serverObservedUp() / prootActive() can never hang. + // Idempotent (guarded by the wait anchor). boolean queueTerminalNotRunning = prootTerminal && !ModuleQueueRepository.get().isRunning(); - if (moduleShown && queueTerminalNotRunning) ensureServerUpForModules(); + if (moduleShown && queueTerminalNotRunning) onModuleBatchTerminal(); + + // ADFA-5343 (Phase 2): the module server state read from the observed phase, not a boot latch. + // up = the poll observed /k2go-api answering; + // slow = still not up past the wait timeout — a stuck flap the reconciler keeps re-driving, + // surfaced so the user isn't left staring (Finish lands on a Home it drives live); + // settling = terminal, not up yet, still within the timeout ((re)starting). + boolean batchServerUp = moduleShown && queueTerminalNotRunning && serverObservedUp(); + boolean batchAwaitingServer = moduleShown && queueTerminalNotRunning && !serverObservedUp(); + boolean batchServerSlow = batchAwaitingServer && moduleServerWaitAt != 0L + && SystemClock.elapsedRealtime() - moduleServerWaitAt > SERVER_UP_TIMEOUT_MS; + boolean batchServerSettling = batchAwaitingServer && !batchServerSlow; boolean allComplete; - boolean moduleServerSettled = moduleServerUp || moduleServerFailed; // ADFA-4842: up, or gave up (failure) + boolean moduleServerSettled = batchServerUp || batchServerSlow; // ADFA-5343: up, or gave up waiting here if (noRest && prootShown) { // proot-only: complete when the queue is terminal — plus, for a module batch, once the server // is back (up) or the restart has failed (a dead home that wakes up seconds later is exactly @@ -702,12 +724,12 @@ private void render() { boolean moduleFailed = moduleFlow && prootFailed > 0; // Amber "working" while a module install runs, its post-DONE restart is pending, or the batch // ended with a failed module (kept on the same amber install line, never a green success). - boolean amberWaiting = !moduleServerFailed && (moduleFailed || (moduleFlow ? !moduleServerUp : !servicesReady)); - tint(dot, (amberWaiting || moduleServerFailed) ? R.color.k2go_amber : R.color.k2go_leaf); + boolean amberWaiting = !batchServerSlow && (moduleFailed || (moduleFlow ? !batchServerUp : !servicesReady)); + tint(dot, (amberWaiting || batchServerSlow) ? R.color.k2go_amber : R.color.k2go_leaf); int statusRes; - if (moduleServerFailed) statusRes = R.string.k2go_setup_slow; // couldn't bring services online - else if (moduleRestartKicked && !moduleServerUp) statusRes = R.string.k2go_setup_starting; // (re)starting the server - else if (moduleFlow && !moduleServerUp) statusRes = R.string.install_busy_modules; // runroles in flight + if (batchServerSlow) statusRes = R.string.k2go_setup_slow; // couldn't bring services online in time + else if (batchServerSettling) statusRes = R.string.k2go_setup_starting; // reconciler is (re)starting the server + else if (moduleFlow && !batchServerUp) statusRes = R.string.install_busy_modules; // runroles in flight else if (moduleFailed) statusRes = R.string.install_busy_modules; // ADFA-4898: keep the amber install header; failure + Retry are per-module below else if (moduleFlow) statusRes = R.string.k2go_setup_adding; // module done + server up else if (!servicesReady) statusRes = (readyPolls >= SLOW_AFTER_POLLS ? R.string.k2go_setup_slow : R.string.k2go_setup_starting); @@ -752,8 +774,8 @@ && getLifecycle().getCurrentState().isAtLeast(androidx.lifecycle.Lifecycle.State // Bottom controls. ADFA-4842: a failed post-module server restart counts as a failure (Finish + // note), never a silent success — so the user is told, not dropped on a dead Home. - boolean success = allComplete && failedTotal == 0 && !moduleServerFailed; - boolean failure = allComplete && (failedTotal > 0 || moduleServerFailed); + boolean success = allComplete && failedTotal == 0 && !batchServerSlow; + boolean failure = allComplete && (failedTotal > 0 || batchServerSlow); if (success && !redirectCancelled) { show(redirect, true); show(cancel, true); show(finishBtn, false); show(finishNote, false); show(runBgBtn, false); @@ -1202,46 +1224,21 @@ private void cancelRedirect() { main.removeCallbacks(goHomeRunnable); redirectScheduled = false; } - /** ADFA-4842: after a module batch (the server was pdsm-stopped for the runroles), start the server - * once and poll the REST core until it answers. render() gates completion/redirect on - * moduleServerUp, so we only leave for the Library once the system is actually live. */ - private void ensureServerUpForModules() { - if (moduleServerUp || moduleServerFailed || moduleRestartKicked) return; - moduleRestartKicked = true; - moduleRestartAt = SystemClock.elapsedRealtime(); - // ADFA-4842: MECHANISM (proot ≠ REST — do NOT "simplify" this to the toggle; read - // ServerController.startEnvironment() first). Each module runrole runs in its own proot with - // --kill-on-exit: clean start → its tasks → clean stop, so the environment is DOWN when the batch - // finishes (that is the correct per-module cycle, especially with several modules in series). After - // the LAST module the INDEX is the actuator: it brings the environment back UNCONDITIONALLY. - // We must NOT call handleServerLaunchClick here (it is a TOGGLE): the cached alive can still read - // TRUE the instant after the runrole proot exits, and the toggle would then STOP instead of start - // (that was the "Stopping IIAB environment gracefully → dead Home" bug from the device log). - // startEnvironment() always starts. The server proot is process-scoped, so it survives into - // LibraryActivity, which only MONITORS it — Home never starts the server. - serverController.startEnvironment(); - main.postDelayed(serverUpPoll, READY_POLL_MS); - } - - private final Runnable serverUpPoll = new Runnable() { - @Override public void run() { - if (isFinishing() || moduleServerUp || moduleServerFailed) return; - AppExecutors.get().io().execute(() -> { - final boolean up = RestReadiness.apiReady(); - main.post(() -> { - if (isFinishing() || moduleServerUp || moduleServerFailed) return; - if (up) { moduleServerUp = true; render(); } // REST core answered → complete → redirect - else if (SystemClock.elapsedRealtime() - moduleRestartAt > SERVER_UP_TIMEOUT_MS) { - // ADFA-4842: the environment didn't come online in time. Do NOT declare success and - // drop the user on a dead Home (the old behavior); surface it as a FAILURE so the - // index shows the Finish/error state (like an Ansible failure). Home is a monitor — - // it won't recover this, so the honest thing is to tell the user here. - moduleServerFailed = true; render(); - } else main.postDelayed(serverUpPoll, READY_POLL_MS); - }); - }); + /** ADFA-5343 (Phase 2): a module batch stopped the server for its runroles; when the batch is + * terminal the server should come back. We no longer boot + poll here. We record the intent + * (userWantsOn) once — the lock is already released and the durable guard cleared before the queue + * publishes DONE, so desired flips UP — and the reconciler drives it up and keeps it up (re-driving + * a flap), so a redirect to Home lands on a live system rather than a dead one (5336). The wait + * timestamp anchors the "taking longer" UI. When actuation is disabled (rollback), we boot once here + * as before (and Home is a monitor again, so 5336 is not fixed in that mode). */ + private void onModuleBatchTerminal() { + if (moduleServerWaitAt != 0L) return; // once — also the timeout anchor + moduleServerWaitAt = SystemClock.elapsedRealtime(); + new org.iiab.controller.Preferences(this).setWatchdogEnable(true); // persisted intent → desired = UP + if (!org.iiab.controller.env.ServerLifecycleReconciler.ACTUATES) { + serverController.startEnvironment(); // rollback path: reconciler is log-only, boot here } - }; + } private void goHome(boolean clearSessions) { cancelRedirect(); @@ -1251,9 +1248,10 @@ private void goHome(boolean clearSessions) { // ADFA-4919: the natural end of installing is the Library — go there directly and clear the // install screens above it. Both the wizard and Get More launch from LibraryActivity, so // CLEAR_TOP + SINGLE_TOP lands on the existing Library (dropping Get More + this index). Only - // success/Finish reach here; "Run in background" (REST) still finish()es in place. ADFA-4842: a - // module batch already restarted the server and waited for it here (ensureServerUpForModules), so - // the reused Library is live on arrival — no cold-boot recreate needed. + // success/Finish reach here; "Run in background" (REST) still finish()es in place. ADFA-5343: a + // module batch set desired=UP; the reconciler brings the server up and keeps re-driving it wherever + // the app is, so the reused Library is (or becomes) live on arrival — even Finish under a slow/flap + // start lands on a Home the reconciler drives up, not a dead one (5336). startActivity(new android.content.Intent(this, LibraryActivity.class) .addFlags(android.content.Intent.FLAG_ACTIVITY_CLEAR_TOP | android.content.Intent.FLAG_ACTIVITY_SINGLE_TOP) .putExtra(LibraryActivity.EXTRA_TAB, R.id.nav_library)); // ADFA-4842: land on Home, not the launching tab (Settings) diff --git a/controller/app/src/test/java/org/iiab/controller/env/domain/ServerReconcileTest.java b/controller/app/src/test/java/org/iiab/controller/env/domain/ServerReconcileTest.java index c6df5613f..af4627430 100644 --- a/controller/app/src/test/java/org/iiab/controller/env/domain/ServerReconcileTest.java +++ b/controller/app/src/test/java/org/iiab/controller/env/domain/ServerReconcileTest.java @@ -103,4 +103,20 @@ public void desiredDownIsNoopWhenDown() { assertEquals(ServerReconcile.Intent.NOOP, ServerReconcile.intent(false, ServerLiveness.Phase.DOWN)); } + + // --- ensuresUp (which intents drive a boot) ---------------------------------- + + @Test + public void startAndWaitEnsureUp() { + // START (down) and WAIT (coming up / stuck flap) both route to the idempotent ensureServerUp. + assertTrue(ServerReconcile.ensuresUp(ServerReconcile.Intent.START)); + assertTrue(ServerReconcile.ensuresUp(ServerReconcile.Intent.WAIT)); + } + + @Test + public void noopStopHoldDoNotEnsureUp() { + assertFalse(ServerReconcile.ensuresUp(ServerReconcile.Intent.NOOP)); + assertFalse(ServerReconcile.ensuresUp(ServerReconcile.Intent.STOP)); + assertFalse(ServerReconcile.ensuresUp(ServerReconcile.Intent.HOLD)); + } } From b5d51612aaa7288352691b816e3282453102b4e2 Mon Sep 17 00:00:00 2001 From: Luis Guzman Date: Sat, 29 Aug 2026 05:48:49 -0600 Subject: [PATCH 05/10] ADFA-5343 fix(server-lifecycle): ADR-5343a D1+D2 - flap auto-recovery (5336) Phase-2 device verification found the flap flow regressed: actuation turned a self-healing dash-node blip into an unrecoverable KILL_AND_RELAUNCH loop. Two independent defects, both fixed here (ADR-5343a, approved). D1 - escalate on service downtime, not proot age. EnvironmentEnsure.decide keyed KILL_AND_RELAUNCH on envAgeMs >= BOOT_GRACE_MS, so a mature proot whose dash-node blipped escalated on the first tick - before pdsm's ~3s respawn. It now escalates on how long the services have been continuously observed down while the proot stays present. The clock is one derived field (servicesDownSinceMs) in the single ServerLiveness source, threaded across ticks by the poll (ServerLiveness.next), reset on an observation gap and on boot; a stale/absent snapshot reports -1, which decide treats as "wait". This drops envAgeMs from the decision and replaces the fixed BOOT_GRACE_MS with SERVICE_DOWN_GRACE_MS - a reduction, realizing ADR-5343 2.6. D2 - killOrphan reclaims the orphaned HTTP front. The box's nginx daemonises (setsid, reparents to init) and survives a proot kill, keeping :8085 so a relaunched proot's pdsm start cannot rebind (the loop). reapEnvironmentHttpFront() now reclaims it. This is a compensator for a guest-side defect (services should die with the proot); the clean end-state and the two deviations it carries are recorded in ADR-5343a 9, to retire when the guest-side fix lands - not now. D3 (LibraryActivity:422 second boot owner) is recorded in ADR-5343a 3, deferred to Phase 4 (not pulled forward). JVM tests: EnvironmentEnsureTest re-cast onto service downtime (mature-proot flap WAITs; past-grace relaunches; unknown downtime never kills); ServerLivenessTest adds the servicesDownSinceMs reducer (drop=0, accumulation, observation-gap reset, stale reports -1, services-up / proot-gone clear it). Verified: :app:testDebugUnitTest + :app:lintDebug BUILD SUCCESSFUL locally. Device (a026a310, Luis): flow 2 auto-recovers to UP and rebinds :8085; flow 3 (timeout) verified by hand; flows 1/4/5/6 no regression. Review follow-ups (surfaced, not folded in to keep this to the approved delta): EnvironmentProcess.environmentAgeMs/readStartTicks are now dead (D1 dropped envAgeMs); the lastLiveness reset in doLaunchEnvironment has a second writer (the comment's "single writer" is inaccurate) with a low-probability lost-reset race. Rollback: ServerLifecycleReconciler.ACTUATES=false (device-proven self-healing). Co-Authored-By: Claude Opus 4.8 --- .../org/iiab/controller/ServerController.java | 61 ++++++--- .../controller/env/EnvironmentProcess.java | 70 ++++++++-- .../env/domain/EnvironmentEnsure.java | 41 ++++-- .../controller/env/domain/ServerLiveness.java | 66 +++++++++- .../env/domain/EnvironmentEnsureTest.java | 49 ++++--- .../env/domain/ServerLivenessTest.java | 54 ++++++++ .../docs/ADR-5343a-flap-recovery-delta.md | 120 ++++++++++++++++++ 7 files changed, 401 insertions(+), 60 deletions(-) create mode 100644 controller/docs/ADR-5343a-flap-recovery-delta.md diff --git a/controller/app/src/main/java/org/iiab/controller/ServerController.java b/controller/app/src/main/java/org/iiab/controller/ServerController.java index 2057880e3..733e4e61a 100644 --- a/controller/app/src/main/java/org/iiab/controller/ServerController.java +++ b/controller/app/src/main/java/org/iiab/controller/ServerController.java @@ -33,12 +33,14 @@ public class ServerController implements org.iiab.controller.env.ServerLifecycle private static final String TAG = "IIAB-ServerController"; private static final int CHECK_INTERVAL_MS = 3000; /** - * ADFA-5103: how long "environment alive, services not answering" is read as "still starting" - * rather than "stuck", before ensure-up is allowed to kill it. Must comfortably exceed the - * observed 3.5 s mid-boot window that got the earlier kill reverted; kept well under a normal - * boot-to-services time so a genuinely stuck orphan still recovers on a Retry. + * ADFA-5103 / ADFA-5343a (D1): how long the services may be continuously observed down (proot + * present) before ensure-up escalates from "still coming up / pdsm will respawn it" to "stuck → + * relaunch". Timed from the service drop, not the proot's age (ADR-5343a): a mature proot whose + * dash-node blips stays well under this and self-heals via pdsm, while a boot — services down since + * the proot started — is still protected for this long (comfortably over the 3.5 s mid-boot window + * that got the earlier kill reverted, and over a normal boot-to-services time). */ - private static final long BOOT_GRACE_MS = 20_000L; + private static final long SERVICE_DOWN_GRACE_MS = 20_000L; /** Activity-side callbacks the server lifecycle needs. */ public interface Host { @@ -83,6 +85,11 @@ default void onStartupProgress(String service) {} // and both LAUNCH — the synchronous main-thread serialisation that used to prevent that is gone. // This flag restores it: a concurrent call is a no-op until the launch (or the no-op) resolves. private volatile boolean ensuring = false; + // ADFA-5343a (D1): the last liveness snapshot from the poll, threaded so servicesDownSinceMs + // measures CONTINUOUS observed downtime (reset on an observation gap). Read by the ensure-up + // decision to key the kill on service downtime, not proot age. Volatile: written on the poll's IO + // thread, read by the (also-IO) ensure-up decision; the poll is the single writer. + private volatile org.iiab.controller.env.domain.ServerLiveness lastLiveness; private static final java.util.regex.Pattern PDSM_SVC = java.util.regex.Pattern.compile("\\[pdsm:([^\\]]+)\\]"); private final Handler timeoutHandler = new Handler(android.os.Looper.getMainLooper()); @@ -165,11 +172,17 @@ private void checkServerStatus() { // alive stays a 1-bit fact (phase == UP) so ServerStateRepository and every reader are // unchanged here — the only shift is that "up" now means the services answer, not nginx. long now = android.os.SystemClock.elapsedRealtime(); + // ADFA-5343a (D1): thread the snapshot so servicesDownSinceMs measures CONTINUOUS observed + // downtime; next() resets it on an observation gap (a stale previous snapshot), so a + // background gap can never read as long downtime and re-drive the kill loop on resume. org.iiab.controller.env.domain.ServerLiveness liveness = - org.iiab.controller.env.domain.ServerLiveness.of( + org.iiab.controller.env.domain.ServerLiveness.next( + lastLiveness, org.iiab.controller.env.EnvironmentProcess.isRunning(activity), org.iiab.controller.redesign.RestReadiness.apiReady(), - now); + now, + org.iiab.controller.env.domain.ServerLiveness.DEFAULT_FRESH_MS); + lastLiveness = liveness; boolean localAlive = liveness.phase(now) == org.iiab.controller.env.domain.ServerLiveness.Phase.UP; @@ -287,33 +300,41 @@ public void startEnvironment() { // pure and unit-tested on the JVM. `ensuring` is cleared by doLaunchEnvironment() on the // launch paths and here on the no-op paths, so it is released exactly once. AppExecutors.get().io().execute(() -> { + long now = android.os.SystemClock.elapsedRealtime(); boolean envAlive = org.iiab.controller.env.EnvironmentProcess.isRunning(activity); - long ageMs = envAlive ? org.iiab.controller.env.EnvironmentProcess.environmentAgeMs(activity) : -1L; - // ADFA-5280: decide on FRESH liveness, not the cached ServerStateRepository.alive. - // Right after a module batch's `pdsm stop`, the cache still reads TRUE until the 3s poll - // catches up, so decide() returned NOOP_HEALTHY and the box was never relaunched (Home + // ADFA-5280: decide on FRESH liveness, not the cached ServerStateRepository.alive. Right + // after a module batch's `pdsm stop`, the cache still reads TRUE until the 3s poll catches + // up, so a stale read returned NOOP_HEALTHY and the box was never relaunched (Home // "Couldn't start" until a manual Retry). A live probe reads a just-stopped server as down - // at once (connection refused); a genuinely-healthy env still answers true -> NOOP_HEALTHY, - // so this never double-boots. Safe here: this block already runs off the main thread. + // at once; a genuinely-healthy env still answers true -> NOOP_HEALTHY, so this never + // double-boots. Safe here: this block already runs off the main thread. boolean servicesAlive = org.iiab.controller.redesign.RestReadiness.apiReady(); + // ADFA-5343a (D1): escalate on SERVICE downtime, not proot age. The continuous-downtime clock + // lives in the one liveness source (threaded by the poll); a stale/absent snapshot reports + // -1, which decide() treats as "wait, do not kill". A mature proot whose dash-node just + // blipped is a small downtime -> WAIT (pdsm respawns, ~3s); only a service down past the + // grace is a stuck environment worth relaunching. + org.iiab.controller.env.domain.ServerLiveness ll = lastLiveness; + long servicesDownMs = (ll == null) ? -1L + : ll.servicesDownMs(now, org.iiab.controller.env.domain.ServerLiveness.DEFAULT_FRESH_MS); org.iiab.controller.env.domain.EnvironmentEnsure.Action action = org.iiab.controller.env.domain.EnvironmentEnsure.decide( - envAlive, ageMs, servicesAlive, BOOT_GRACE_MS); + envAlive, servicesAlive, servicesDownMs, SERVICE_DOWN_GRACE_MS); switch (action) { case LAUNCH: activity.runOnUiThread(this::doLaunchEnvironment); break; case KILL_AND_RELAUNCH: - android.util.Log.i(TAG, "ADFA-5103: environment alive but services down past boot" - + " grace (age " + ageMs + "ms) — killing the orphan and relaunching"); + android.util.Log.i(TAG, "ADFA-5343a: services down " + servicesDownMs + "ms (past the" + + " grace) on a live proot — reclaiming the orphaned environment and relaunching"); org.iiab.controller.env.EnvironmentProcess.killOrphan(activity); activity.runOnUiThread(this::doLaunchEnvironment); break; case NOOP_HEALTHY: case WAIT_BOOT_GRACE: default: - android.util.Log.i(TAG, "ADFA-5103: ensure-up is a no-op (" + action + ", age " - + ageMs + "ms) — not stacking a second proot"); + android.util.Log.i(TAG, "ADFA-5103: ensure-up is a no-op (" + action + + ", servicesDown " + servicesDownMs + "ms) — not stacking a second proot"); ensuring = false; break; } @@ -331,6 +352,10 @@ private void doLaunchEnvironment() { File rootfsDir = new File(activity.getFilesDir(), "rootfs/installed-rootfs/iiab"); host.addToLog(activity.getString(R.string.log_server_booting_native)); host.onStartupBegan(); // ADFA-4837: fill the pre-pdsm silent window + // ADFA-5343a (D1): a fresh environment is starting — restart the service-downtime clock so the + // new proot gets its full boot grace. Without this a KILL_AND_RELAUNCH keeps the accumulated + // downtime and re-kills the booting proot every tick, before its services can come up. + lastLiveness = null; createFakeSysData(rootfsDir); if (serverEngine != null) serverEngine.killProcess(); serverEngine = new PRootEngine(); diff --git a/controller/app/src/main/java/org/iiab/controller/env/EnvironmentProcess.java b/controller/app/src/main/java/org/iiab/controller/env/EnvironmentProcess.java index c82ff20f6..64b3eb2ba 100644 --- a/controller/app/src/main/java/org/iiab/controller/env/EnvironmentProcess.java +++ b/controller/app/src/main/java/org/iiab/controller/env/EnvironmentProcess.java @@ -47,6 +47,11 @@ public final class EnvironmentProcess { private static final String TAG = "K2Go-Env"; + /** ADFA-5343a (D2): the box's nginx HTTP port ({@code config/BoxEndpoints.java:21}). Its listener is + * the reparented orphan that keeps the port after a proot kill; reclaiming it is what lets a fresh + * proot's {@code pdsm start} rebind. */ + private static final int ENV_HTTP_PORT = 8085; + private EnvironmentProcess() { } @@ -184,18 +189,67 @@ public static boolean killOrphan(Context ctx) { if (ctx == null) { return false; } + boolean signalled = false; int pid = findPid(ctx); - if (pid <= 0) { - return false; + if (pid > 0) { + try { + android.os.Process.killProcess(pid); // SIGKILL, same UID as us + Log.i(TAG, "ADFA-5061: killed an orphaned environment proot, pid " + pid); + signalled = true; + } catch (Exception e) { + Log.w(TAG, "ADFA-5061: could not kill environment pid " + pid, e); + } } - try { - android.os.Process.killProcess(pid); // SIGKILL, same UID as us - Log.i(TAG, "ADFA-5061: killed an orphaned environment proot, pid " + pid); - return true; - } catch (Exception e) { - Log.w(TAG, "ADFA-5061: could not kill environment pid " + pid, e); + // ADFA-5343a (D2): killing the proot is not enough. Its services daemonise — nginx setsid()s and + // reparents to init — so they survive the proot and keep :8085, and every relaunched proot's + // `pdsm start` then cannot rebind (the ADFA-5336 unrecoverable loop). Reclaim the HTTP front too. + boolean reaped = reapEnvironmentHttpFront(); + return signalled || reaped; + } + + /** + * ADFA-5343a (D2): reclaim the environment's orphaned nginx — the box's HTTP front on {@code :8085} + * ({@code config/BoxEndpoints.java:21}) — which daemonises ({@code setsid}, reparents to init) and + * survives the proot, keeping the port so a relaunched proot's {@code pdsm start} cannot rebind (the + * ADFA-5336 loop). + * + *

Found by cmdline, not by the port. An app cannot read {@code /proc/net/tcp} on modern + * Android (blocked since API 29), so the socket→pid path is unavailable here; nginx is instead + * matched on {@code /proc//cmdline} ({@code "nginx: master process nginx"} / worker). That is + * still scoped to us: nginx runs in the app's own uid and SELinux domain (device-verified, + * ADR-5343a §2), so it is killable, and the box's only nginx is ours. Killing each matching pid reaps + * master and workers together; idempotent — a no-op when none is running. + * + * @return true when at least one nginx process was signalled. + */ + public static boolean reapEnvironmentHttpFront() { + File[] entries = new File("/proc").listFiles(); + if (entries == null) { return false; } + boolean reaped = false; + for (File dir : entries) { + int pid; + try { + pid = Integer.parseInt(dir.getName()); + } catch (NumberFormatException notAPid) { + continue; + } + String cmd = readCmdline(new File(dir, "cmdline")); + if (cmd != null && cmd.contains("nginx")) { + try { + android.os.Process.killProcess(pid); // same uid + SELinux domain as us + reaped = true; + } catch (Exception ignored) { + // vanished mid-scan, or not ours to signal + } + } + } + if (reaped) { + Log.i(TAG, "ADFA-5343a: reclaimed the orphaned nginx holding the HTTP front (:" + + ENV_HTTP_PORT + ")"); + } + return reaped; } /** {@code /proc//cmdline} as a space-joined string, or null if it cannot be read. */ diff --git a/controller/app/src/main/java/org/iiab/controller/env/domain/EnvironmentEnsure.java b/controller/app/src/main/java/org/iiab/controller/env/domain/EnvironmentEnsure.java index 234dab654..a2ba1371a 100644 --- a/controller/app/src/main/java/org/iiab/controller/env/domain/EnvironmentEnsure.java +++ b/controller/app/src/main/java/org/iiab/controller/env/domain/EnvironmentEnsure.java @@ -38,32 +38,45 @@ public enum Action { LAUNCH, /** Alive and its services answer — a redundant start is a no-op. */ NOOP_HEALTHY, - /** Alive, services not answering yet, but still inside its boot grace — leave it to finish. */ + /** Alive, services not answering yet, but still inside the grace — leave it (pdsm respawns). */ WAIT_BOOT_GRACE, - /** Alive, services not answering, past its boot grace — a stuck orphan; end it and relaunch. */ + /** Alive, services down past the grace — a genuinely stuck environment; end it and relaunch. */ KILL_AND_RELAUNCH } /** - * @param envAlive whether an environment proot of ours is running (from {@code /proc}). - * @param envAgeMs the running proot's age in ms, or a negative value when it is unknown - * (no proot, or its start time could not be read). - * @param servicesAlive whether the box's services answer (the cached HTTP-ping fact). - * @param bootGraceMs how long "alive but not answering" is read as "still starting" rather - * than "stuck". + * ADFA-5343 (delta ADR-5343a, D1): the escalation clock is service downtime, not proot age. + * Keying on age fired {@code KILL_AND_RELAUNCH} on the first tick after a mature proot's dash-node + * blipped — before pdsm's ~3 s respawn — turning a self-healing flap into an unrecoverable loop + * (ADFA-5336, device-confirmed). Timed from the service drop instead, a flap stays inside the grace + * and self-heals via {@code WAIT_BOOT_GRACE} → {@code NOOP_HEALTHY}; only a service that stays down + * past the grace (pdsm could not bring it back) is a stuck environment worth relaunching. + * + *

The single case that got the first ADFA-5103 attempt reverted — a proot killed 3.5 s into its + * own boot — is still protected: during a boot the services have been down since the proot started, + * so {@code servicesDownMs} is that same small elapsed time and stays under the grace. An unknown + * downtime ({@code < 0}: a stale/never-observed snapshot) is never killed, the same fail-safe. + * + * @param envAlive whether an environment proot of ours is running (from {@code /proc}). + * @param servicesAlive whether the box's services answer ({@code /k2go-api}, fresh). + * @param servicesDownMs how long the services have been continuously observed down while the + * proot stayed present ({@link ServerLiveness#servicesDownMs}), or a + * negative value when that is not a trustworthy fact. + * @param serviceDownGraceMs how long "alive but not answering" is read as "still coming up / pdsm + * will respawn it" rather than "stuck". */ - public static Action decide(boolean envAlive, long envAgeMs, boolean servicesAlive, - long bootGraceMs) { + public static Action decide(boolean envAlive, boolean servicesAlive, long servicesDownMs, + long serviceDownGraceMs) { if (!envAlive) { return Action.LAUNCH; } if (servicesAlive) { return Action.NOOP_HEALTHY; } - // Alive, services down. Kill only a proot we can be sure is past its boot — never on an - // unknown age, because the mistake that got the first attempt reverted was killing one - // mid-boot, and "cannot confirm it is old" must fall on the side of not killing. - if (envAgeMs < 0 || envAgeMs < bootGraceMs) { + // Alive, services down. Kill only when we can be sure they have stayed down past the grace — + // never on an unknown downtime, because "cannot confirm it is stuck" must fall on the side of + // waiting (pdsm may still be respawning the service). + if (servicesDownMs < 0 || servicesDownMs < serviceDownGraceMs) { return Action.WAIT_BOOT_GRACE; } return Action.KILL_AND_RELAUNCH; diff --git a/controller/app/src/main/java/org/iiab/controller/env/domain/ServerLiveness.java b/controller/app/src/main/java/org/iiab/controller/env/domain/ServerLiveness.java index 833d75f8f..62d1727c6 100644 --- a/controller/app/src/main/java/org/iiab/controller/env/domain/ServerLiveness.java +++ b/controller/app/src/main/java/org/iiab/controller/env/domain/ServerLiveness.java @@ -63,14 +63,22 @@ public enum Phase { private final boolean processPresent; private final boolean servicesAnswering; private final long observedAtMs; + private final long servicesDownSinceMs; - private ServerLiveness(boolean processPresent, boolean servicesAnswering, long observedAtMs) { + private ServerLiveness(boolean processPresent, boolean servicesAnswering, long observedAtMs, + long servicesDownSinceMs) { this.processPresent = processPresent; this.servicesAnswering = servicesAnswering; this.observedAtMs = observedAtMs; + this.servicesDownSinceMs = servicesDownSinceMs; } /** + * A history-less snapshot. {@code servicesDownSinceMs} is seeded from this one observation + * (down "since now" when the proot is present but its services do not answer), so a lone snapshot + * reports ~0 downtime. The continuous-downtime clock is threaded by {@link #next}; prefer it on the + * poll path so a flap is measured across ticks rather than re-started every tick. + * * @param processPresent our environment proot is alive (from {@code /proc}). * @param servicesAnswering dash-node answers {@code /k2go-api}. * @param observedAtMs when these were read, from a monotonic clock @@ -78,13 +86,67 @@ private ServerLiveness(boolean processPresent, boolean servicesAnswering, long o */ public static ServerLiveness of(boolean processPresent, boolean servicesAnswering, long observedAtMs) { - return new ServerLiveness(processPresent, servicesAnswering, observedAtMs); + long downSince = (!servicesAnswering && processPresent && observedAtMs > 0) ? observedAtMs : 0L; + return new ServerLiveness(processPresent, servicesAnswering, observedAtMs, downSince); + } + + /** + * ADFA-5343 (delta ADR-5343a, D1): the next snapshot in a continuous poll stream, carrying how long + * the services have been down while the proot stayed present — the clock that tells a flap + * (dash-node briefly gone, pdsm about to respawn it) from a genuinely stuck environment. This + * replaces keying the kill decision on proot age (which fires instantly on a mature proot; ADFA-5336 + * regression): downtime is measured from the service drop, not the proot's birth. + * + *

Guardrail — observed-continuous, not wall-calendar. The clock is carried forward only + * across a fresh previous observation. If the poll stopped feeding us (the app was + * backgrounded, {@code prev} is stale by {@link Freshness}), the streak is broken and the clock + * resets to {@code nowMs} — otherwise a long background gap would read as long downtime and + * re-drive the kill loop the moment the app returns. + * + * @param prev the previous snapshot, or {@code null} on the first tick. + * @param processPresent our environment proot is alive now. + * @param servicesAnswering dash-node answers {@code /k2go-api} now. + * @param nowMs the monotonic clock these were read at. + * @param freshnessMs how long {@code prev} stays trustworthy for carrying the streak. + */ + public static ServerLiveness next(ServerLiveness prev, boolean processPresent, + boolean servicesAnswering, long nowMs, long freshnessMs) { + long downSince; + if (servicesAnswering || !processPresent) { + downSince = 0L; // up, or the proot is gone (that is DOWN → LAUNCH, not a downtime to time) + } else { + boolean continuousStreak = prev != null + && prev.processPresent && !prev.servicesAnswering + && prev.servicesDownSinceMs > 0L + && Freshness.fresh(prev.observedAtMs, nowMs, freshnessMs); // no observation gap + downSince = continuousStreak ? prev.servicesDownSinceMs : nowMs; + } + return new ServerLiveness(processPresent, servicesAnswering, nowMs, downSince); } public boolean processPresent() { return processPresent; } public boolean servicesAnswering() { return servicesAnswering; } public long observedAtMs() { return observedAtMs; } + /** + * How long the services have been continuously observed down (proot present), or {@code -1} when + * that is not a fact to act on: the snapshot itself is stale (a background gap — never time a kill + * off a reading the poll has not refreshed), or the services are up / the proot is gone. A caller + * treats {@code -1} as "wait, do not kill", the same fail-safe as an unknown proot age was. + * + * @param nowMs the same monotonic clock the snapshot was stamped from. + * @param freshnessMs how long the snapshot stays trustworthy (a stale one reports {@code -1}). + */ + public long servicesDownMs(long nowMs, long freshnessMs) { + if (!Freshness.fresh(observedAtMs, nowMs, freshnessMs)) { + return -1L; + } + if (servicesDownSinceMs <= 0L) { + return -1L; + } + return Math.max(0L, nowMs - servicesDownSinceMs); + } + /** The phase using the default freshness window. */ public Phase phase(long nowMs) { return phase(nowMs, DEFAULT_FRESH_MS); diff --git a/controller/app/src/test/java/org/iiab/controller/env/domain/EnvironmentEnsureTest.java b/controller/app/src/test/java/org/iiab/controller/env/domain/EnvironmentEnsureTest.java index 754a125f7..9889c4fc8 100644 --- a/controller/app/src/test/java/org/iiab/controller/env/domain/EnvironmentEnsureTest.java +++ b/controller/app/src/test/java/org/iiab/controller/env/domain/EnvironmentEnsureTest.java @@ -8,9 +8,11 @@ * Unit tests for {@link EnvironmentEnsure}. * *

The stakes are the same lopsided ones as the matcher's: the wrong verdict here either stacks a - * second proot over a live one or SIGKILLs one mid-boot. The single case that got the first attempt - * reverted — a proot killed 3.5 s into its own start — is {@link #aYoungUnansweringProotIsLeftToBoot} - * and {@link #anUnknownAgeIsNeverKilled}. + * second proot over a live one or SIGKILLs one that pdsm would have healed. Two cases anchor the + * decision: a proot 3.5 s into its own boot must be left alone ({@link #aBootingProotIsLeftToFinish}, + * {@link #anUnknownDowntimeIsNeverKilled}), and — the ADFA-5336 regression this delta fixes — a mature + * proot whose dash-node just blipped must WAIT for pdsm's respawn, not be killed + * ({@link #aMatureProotWhoseServiceJustDroppedWaitsForRespawn}). */ public class EnvironmentEnsureTest { @@ -19,45 +21,56 @@ public class EnvironmentEnsureTest { @Test public void nothingRunningLaunches() { assertEquals(EnvironmentEnsure.Action.LAUNCH, - EnvironmentEnsure.decide(false, -1L, false, GRACE)); + EnvironmentEnsure.decide(false, false, -1L, GRACE)); } @Test public void aliveAndAnsweringIsANoOp() { // The redundant-call case: five of the six callers fire "ensure it is up" when it already is. + // servicesAlive short-circuits, so the downtime argument is irrelevant. assertEquals(EnvironmentEnsure.Action.NOOP_HEALTHY, - EnvironmentEnsure.decide(true, 90_000L, true, GRACE)); + EnvironmentEnsure.decide(true, true, -1L, GRACE)); } @Test - public void aYoungUnansweringProotIsLeftToBoot() { - // 3.5 s in, services not up yet — exactly the proot the earlier wiring killed. Never touch it. + public void aBootingProotIsLeftToFinish() { + // 3.5 s in, services not up yet — services have been down since the proot started, which is + // exactly the proot the earlier wiring killed. Under the grace → never touch it. assertEquals(EnvironmentEnsure.Action.WAIT_BOOT_GRACE, - EnvironmentEnsure.decide(true, 3_500L, false, GRACE)); + EnvironmentEnsure.decide(true, false, 3_500L, GRACE)); } @Test - public void anUnknownAgeIsNeverKilled() { - // If /proc//stat could not be read, we cannot prove the proot is past its boot, so we - // must not kill it — "cannot confirm it is old" falls on the side of waiting. + public void aMatureProotWhoseServiceJustDroppedWaitsForRespawn() { + // The ADFA-5336 fix: a long-lived (mature) proot whose dash-node just died. Keyed on proot age + // this was an instant KILL_AND_RELAUNCH (age >> grace); keyed on SERVICE downtime it is a 2 s + // flap, well under the grace, so pdsm gets its ~3 s to respawn it. WAIT, do not kill. assertEquals(EnvironmentEnsure.Action.WAIT_BOOT_GRACE, - EnvironmentEnsure.decide(true, -1L, false, GRACE)); + EnvironmentEnsure.decide(true, false, 2_000L, GRACE)); } @Test - public void anOldUnansweringProotIsAStuckOrphanAndIsRelaunched() { - // Alive, services down, well past the grace — pdsm stopped it or it hung. A proot cannot be + public void anUnknownDowntimeIsNeverKilled() { + // A stale or never-observed snapshot reports downtime < 0: we cannot prove the services have + // stayed down, so we must not kill — "cannot confirm it is stuck" falls on the side of waiting. + assertEquals(EnvironmentEnsure.Action.WAIT_BOOT_GRACE, + EnvironmentEnsure.decide(true, false, -1L, GRACE)); + } + + @Test + public void servicesDownPastTheGraceIsAStuckEnvironmentAndIsRelaunched() { + // Alive, services down far past the grace — pdsm could not bring them back. A proot cannot be // re-entered, so recovery is to end it and bring up a fresh one. assertEquals(EnvironmentEnsure.Action.KILL_AND_RELAUNCH, - EnvironmentEnsure.decide(true, 120_000L, false, GRACE)); + EnvironmentEnsure.decide(true, false, 120_000L, GRACE)); } @Test public void theGraceBoundaryKillsAtOrAfterIt() { - // Just under the grace waits; at the grace it is considered booted. + // Just under the grace waits; at the grace the services are considered stuck. assertEquals(EnvironmentEnsure.Action.WAIT_BOOT_GRACE, - EnvironmentEnsure.decide(true, GRACE - 1, false, GRACE)); + EnvironmentEnsure.decide(true, false, GRACE - 1, GRACE)); assertEquals(EnvironmentEnsure.Action.KILL_AND_RELAUNCH, - EnvironmentEnsure.decide(true, GRACE, false, GRACE)); + EnvironmentEnsure.decide(true, false, GRACE, GRACE)); } } diff --git a/controller/app/src/test/java/org/iiab/controller/env/domain/ServerLivenessTest.java b/controller/app/src/test/java/org/iiab/controller/env/domain/ServerLivenessTest.java index ea8634388..8eb76e846 100644 --- a/controller/app/src/test/java/org/iiab/controller/env/domain/ServerLivenessTest.java +++ b/controller/app/src/test/java/org/iiab/controller/env/domain/ServerLivenessTest.java @@ -79,4 +79,58 @@ public void defaultWindowMatchesTheExplicitOne() { ServerLiveness live = ServerLiveness.of(true, false, NOW); assertEquals(live.phase(NOW, ServerLiveness.DEFAULT_FRESH_MS), live.phase(NOW)); } + + // --- ADFA-5343a (D1): service-downtime clock --------------------------------------------------- + + @Test + public void servicesDownMsIsZeroTheTickTheServicesDrop() { + // First tick where the proot is up but /k2go-api stops answering: downtime starts at 0. + ServerLiveness up = ServerLiveness.of(true, true, 1_000L); + ServerLiveness dropped = ServerLiveness.next(up, true, false, 4_000L, FRESH); + assertEquals(0L, dropped.servicesDownMs(4_000L, FRESH)); + } + + @Test + public void servicesDownMsAccumulatesAcrossFreshTicks() { + // A continuous streak of fresh 3 s ticks accumulates real downtime from the drop, not per-tick. + ServerLiveness t0 = ServerLiveness.next(null, true, false, 1_000L, FRESH); // dropped at 1000 + ServerLiveness t1 = ServerLiveness.next(t0, true, false, 4_000L, FRESH); + ServerLiveness t2 = ServerLiveness.next(t1, true, false, 7_000L, FRESH); + assertEquals(3_000L, t1.servicesDownMs(4_000L, FRESH)); + assertEquals(6_000L, t2.servicesDownMs(7_000L, FRESH)); + } + + @Test + public void theClockResetsOnAnObservationGap() { + // GUARDRAIL: the poll went quiet longer than the freshness window (app backgrounded). The + // previous snapshot is stale, so the streak breaks and downtime restarts from now — a calendar + // gap must not read as downtime and re-drive the kill loop when the app returns. + ServerLiveness before = ServerLiveness.next(null, true, false, 1_000L, FRESH); + long afterGap = 1_000L + FRESH + 1L; // one ms past the window since `before` + ServerLiveness resumed = ServerLiveness.next(before, true, false, afterGap, FRESH); + assertEquals(0L, resumed.servicesDownMs(afterGap, FRESH)); // reset, not FRESH+1 of "downtime" + } + + @Test + public void aStaleSnapshotReportsUnknownDowntime() { + // Read side of the same guardrail: even a snapshot that WAS timing downtime reports -1 once it + // is itself stale — never time a kill off a reading the poll has not refreshed. + ServerLiveness dropped = ServerLiveness.of(true, false, 1_000L); // down since 1000 + assertEquals(-1L, dropped.servicesDownMs(1_000L + FRESH + 1L, FRESH)); + } + + @Test + public void servicesAnsweringClearsTheClock() { + ServerLiveness down = ServerLiveness.next(null, true, false, 1_000L, FRESH); + ServerLiveness up = ServerLiveness.next(down, true, true, 4_000L, FRESH); + assertEquals(-1L, up.servicesDownMs(4_000L, FRESH)); + } + + @Test + public void aGoneProotIsNotATrackedDowntime() { + // Proot absent → DOWN → LAUNCH is the caller's business; there is no service downtime to time. + ServerLiveness down = ServerLiveness.next(null, true, false, 1_000L, FRESH); + ServerLiveness gone = ServerLiveness.next(down, false, false, 4_000L, FRESH); + assertEquals(-1L, gone.servicesDownMs(4_000L, FRESH)); + } } diff --git a/controller/docs/ADR-5343a-flap-recovery-delta.md b/controller/docs/ADR-5343a-flap-recovery-delta.md new file mode 100644 index 000000000..51a82fea6 --- /dev/null +++ b/controller/docs/ADR-5343a-flap-recovery-delta.md @@ -0,0 +1,120 @@ +# ADR-5343a - Flap auto-recovery: correcting the Phase-2 actuation (delta to ADR-5343) + +**Status:** Approved (2026-08-29) - D1 + D2 approved for implementation (including the two deviations, §9); D3 deferred to Phase 4 (recorded, not pulled forward). +**Date:** 2026-08-29 +**Deciders:** Luis (sign-off required). +**Ticket:** ADFA-5343 (Task under Epic ADFA-1028). Revises **ADR-5343** (`controller/docs/ADR-5343-server-lifecycle-reconciler.md`); resolves the open bug **ADFA-5336** whose Phase-2-v1 implementation regressed. +**Scope of this delta:** it revises exactly three things in ADR-5343 - the �2.6 grace (concretizes it), the �5 collapse row for 5336, and the �7 Phase-2 migration row. Everything else in ADR-5343 stands. + +> Method note. Produced read-only against Phase-2 commit `1f5cb0f6`, with the mechanism re-confirmed on device `a026a310` (OnePlus7T). Every structural claim cites `File.java:line`. The reduction gate (ADR-5343 �8 / `CLAUDE.local.md`) still binds: this delta must not add a state, flag, source of truth, or "who may act" special-case. + +--- + +## 1. What Phase-2 device verification found + +ADR-5343's reasoning layer verified correct on every flow (one liveness source, the holder-execution-class `desired` predicate, monitoractuator split). But the **key** flow - flap auto-recovery (ADFA-5336) - **regressed**: Phase-2 actuation turns a *self-healing* service flap into an *unrecoverable* loop. + +**A/B evidence (same box, same induction `kill `, `/k2go-api`=502 while nginx still serves `/home`):** + +| Build | Result | +|-------|--------| +| `ACTUATES=true` (Phase 2 v1) | Reconciler re-drives correctly, but the box **never returns to UP** - it loops `KILL_AND_RELAUNCH` every ~24 s (proots 16255164371649616550.), `actual=STARTING` for 70 s+, endpoints degrade to `000`. | +| `ACTUATES=false` (rollback, = pre-Phase-2 behavior) | **pdsm respawns dash-node in ~3 s, same proot; box fine.** No relaunch, no loop. | + +So Phase-2 actuation is **worse than log-only** for a mid-life flap. The reconciler's *reasoning* is not at fault; the *boot mechanism it delegates to* is. + +## 2. Mechanism - device-confirmed (this delta hinges on it; it is not different from the finding) + +Re-confirmed on `a026a310` by freezing the reconciler (background the app `ServerController.onPause` stops the poll, `ServerController.java:122-127`) and issuing `kill -9 ` - exactly what `killOrphan` does (`android.os.Process.killProcess`, `env/EnvironmentProcess.java:192`): + +``` +# healthy: nginx master is already reparented to init +18572 1 nginx: master process nginx # PPID=1 +18526 18467 libproot.so # proot, child of the app +netstat: 0.0.0.0:8085 LISTEN 18572/nginx: master process nginx + +# after kill -9 18526 (the proot): +18572 1 nginx: master process nginx # SURVIVES, still PPID=1 +netstat: 0.0.0.0:8085 LISTEN 18572/nginx: master process nginx # STILL owns :8085 +home:000 api:000 # orphan holds the port but no longer serves +``` + +**Two independent defects:** + +- **D1 - the escalation clock is proot-age, not service-downtime.** `EnvironmentEnsure.decide(...)` escalates to `KILL_AND_RELAUNCH` when `envAlive && !servicesAlive && envAgeMs >= bootGraceMs` (`env/domain/EnvironmentEnsure.java:66-69`), with `bootGraceMs = BOOT_GRACE_MS = 20_000` (`ServerController.java:41`). On a **mature** proot `envAgeMs` is already � 20 s, so the *first* ensure-up tick after any transient service death escalates immediately (device: `KILL_AND_RELAUNCH . age 134229ms` at ~t+2 s) - **before** pdsm's ~3 s respawn. The grace guards the *initial* boot (the ADFA-5103 3.5 s double-boot) but gives a mid-life flap **no** window at all. +- **D2 - `killOrphan` does not reclaim the orphaned services.** It SIGKILLs only the proot pid (`env/EnvironmentProcess.java:183-199`). nginx (and node) are **reparented to PID 1** and survive, still holding `:8085` (netstat above). Every relaunched proot's `pdsm start` then cannot rebind services never answer infinite loop. This is a correctness bug **independent of D1**: even a legitimately-stuck orphan cannot be recovered by the current relaunch. +- **D3 (secondary, orthogonal) - a second un-gated boot owner.** `LibraryActivity`'s launch auto-start calls `handleServerLaunchClick` gated on `(systemInstalled && !alive)`, **not** on `desired` (`redesign/LibraryActivity.java:414-425`, boot at `:422`). Device: after a user turn-off (`userWantsOn=false`, `desired=DOWN`) a relaunch re-booted the box and flipped `userWantsOn=true`. It did **not** cause the flap loop; it is the two-owners tension ADR-5343 �4 already names. + +ADR-5343 **anticipated D1's shape** (�2.6 "progress-aware grace, not a fixed 20 s"; �6 risk "killing a healthy-but-slow boot . the exact 5336/flap regression, in reverse") but Phase-2 v1 shipped with the fixed `envAgeMs` grace still in force via unchanged `EnvironmentEnsure`. ADR-5343 **did not anticipate D2** - �5's 5336 row assumed the relaunch works. + +## 3. Decision (approved fix direction - encode this, do not redesign) + +**Fix D1 - one clock: service-downtime, held in the single `ServerLiveness` source (concretizes ADR-5343 �2.6).** +The escalation grace is measured from **how long `servicesAnswering` has been false while `processPresent` stays true**, not from proot age. `ServerLiveness` already carries `processPresent` / `servicesAnswering` / `observedAtMs` (`env/domain/ServerLiveness.java:63-65`); it gains **one derived field**, `servicesDownSinceMs`, threaded across consecutive snapshots by the single owner (which already holds `lastLiveness`, `ServerLifecycleReconciler.java:68`): + +``` +servicesDownSinceMs(prev, now, probes) = + servicesAnswering 0 + | processPresent && prev>0 prev (still down since prev) + | processPresent now (just went down) + | else 0 (proot gone DOWN LAUNCH, not KILL) +``` + +`EnvironmentEnsure.decide` then escalates on **`servicesDownMs >= serviceDownGraceMs`** instead of `envAgeMs >= bootGraceMs`. One clock subsumes both cases it must cover: +- **Initial boot:** services have been down since the proot started the grace still protects the 3.5 s double-boot (ADFA-5103). +- **Mid-life flap:** the clock resets when the service drops WAIT lets pdsm respawn (~3 s); escalate only if it stays down past `serviceDownGraceMs` (~pdsm respawn + margin, a small constant to tune on device). + +This **replaces** the fixed `BOOT_GRACE_MS` guess and **drops `envAgeMs` from the decision** - a strict reduction, and it realizes �2.6 without parsing the pdsm service-line stream. + +**Fix D2 - `killOrphan` must reclaim the orphaned services so a legitimate relaunch can rebind.** +A correctness fix to the one existing actuator path, adding no new state. The requirement: after `killOrphan`, nothing of ours holds `:8085`. Candidate mechanisms (implementation-time, device-verified, not decided here): launch the proot in its own **process group** and signal the group; set `PR_SET_PDEATHSIG` on the service tree; or have `killOrphan` additionally terminate the box's service processes it can identify as ours. Whichever lands must be proven by the re-run (flow 2 rebinds `:8085` and reaches UP). + +**Fix D3 - record now, gate in Phase 4 (do not pull forward).** +`LibraryActivity:422` is exactly the scaffolding ADR-5343 �7 Phase 4 deletes ("replace the toggle; delete the . re-boot loops"). Gating it on `desired` now would be an out-of-phase change touching a legacy god-class flow that the flap fix does not require - against `CLAUDE.local.md`'s "one phase at a time." **Decision: defer to Phase 4; record the device-observed behavior here so Phase 4 addresses it deliberately** (it is the last second-owner of "boot the box"). Tracked, not patched. + +## 4. Reduction re-check (the hard gate) + +| Fix | States / flags / sources | Verdict | +|-----|--------------------------|---------| +| D1 | Removes fixed `BOOT_GRACE_MS` (a guess flag ADR-5343 �8 already counts for removal) and drops `envAgeMs` from `decide`; adds **one derived field** to the **existing** single `ServerLiveness` source - no new source, no external flag. | **Reduces / neutral** | +| D2 | Correctness fix to the one actuator (`killOrphan`); no new state. | **Neutral** | +| D3 | Deferred; nothing added now; slated for deletion in Phase 4. | **Neutral now, reduces later** | + +No new source of truth, no new "who may act" special-case, no compensating flag. Gate satisfied. + +## 5. Revised collapse-table row for ADFA-5336 (supersedes ADR-5343 �5's 5336 row) + +> **ADFA-5336** - post-install / mid-life server **flap** stuck (v1: **unrecoverable relaunch loop**). **Subsumed by the reconciler's `UPSTARTING` re-drive (ADR-5343 �2.1), corrected by:** (D1) the re-drive WAITs on a **service-downtime** grace so a transient dash-node death lets pdsm self-heal (~3 s), escalating to relaunch only past that grace - the escalation clock is service-downtime, not proot-age; (D2) `killOrphan` reclaims the orphaned services (the reparented nginx holding `:8085`) so a legitimate relaunch can rebind. Both are required: without D1 the reconciler preempts pdsm; without D2 its own relaunch cannot recover. Device-confirmed that the rollback (`ACTUATES=false`) self-heals in ~3 s, isolating the defect to these two. + +## 6. Revised migration note for ADR-5343 �7 (Phase 2) + +Phase 2 is **re-opened** to include D1 + D2 before actuation is considered done (the ADR-5343 �7 rollback lever - `ACTUATES=false` - stays the escape hatch and is already device-proven to be safe/self-healing). No later phase is pulled forward. First gate unchanged: `:app:testDebugUnitTest` + `:app:lintDebug` green, with the pure decision (`EnvironmentEnsure` + the new `ServerLiveness.servicesDownSinceMs` reducer) JVM-tested off device. + +## 7. Post-approval re-verification (device-only, `a026a310`) + +- **Flow 2 (flap):** kill dash-node on a mature proot box **auto-recovers to `actual=UP` with no manual toggle**; log shows WAIT during the service-down grace, and if it escalates, the relaunch **rebinds `:8085`** (netstat shows the new proot's nginx, not an orphan). Must pass. +- **Flow 3 (timeout):** a real module-batch hand-off with `/k2go-api` kept down >45 s `SetupProgressActivity` "taking longer" + Finish appears, reconciler keeps re-driving, Finish lands on a Home it keeps driven - no dead Home. +- Re-run the rest of the Phase-2 matrix to confirm no regression (flows 1, 4, 5, 6). + +## 8. For approval + +1. Approve D1 (service-downtime grace in `ServerLiveness`, dropping `envAgeMs` from `decide`) and D2 (`killOrphan` reclaims orphaned services)? +2. Approve **deferring D3** (LibraryActivity:422 desired-gating) to Phase 4, recorded here? +3. Land this as **ADR-5343a** (this file), or fold it into ADR-5343 as a revision section? (No production code until this is signed off.) + +**Resolution (2026-08-29):** D1 + D2 approved for implementation, including the two deviations recorded in §9. D3 deferred to Phase 4 (recorded, not pulled forward). Landed as ADR-5343a (this file). + +--- + +## 9. Known compensator: the host-side nginx reap (D2) — retire when the guest kills its own services + +D2's `EnvironmentProcess.reapEnvironmentHttpFront()` (`env/EnvironmentProcess.java:225-253`) is a **compensator**, accepted knowingly so the flap fix can land now. It is not the clean end-state. + +**Root cause it papers over (guest-side).** The box's HTTP front daemonises inside the proot: nginx `setsid()`s and reparents to init, so it **survives the proot's death** and keeps `:8085` (device evidence, §2). The clean end-state is **guest-side — the environment's services should die with the proot** (e.g. the runrole/`pdsm` teardown or a `PR_SET_PDEATHSIG`-style parent-death signal on the service tree, so no service outlives the container). With that, a relaunched proot's `pdsm start` rebinds with nothing to reclaim, and D2's host-side reap becomes unnecessary. + +**The two deviations this compensator carries (to retire together with it):** +1. **Host-side reclamation of a guest concern.** The app reaches into `/proc` and SIGKILLs the box's own service processes — work that belongs inside the guest. It is only *safe* here because those processes share the app's uid and SELinux domain (device-verified, §2), which is a property we should not lean on long-term. +2. **Imprecise identity.** It matches `cmdline.contains("nginx")` — a broad substring, host-side, unlike the precise rootfs-tail `EnvironmentProcessMatcher` used to find the proot — and it reaps **nginx only** (the listener holding `:8085`), not the full guest service tree. `/proc/net/tcp` → pid is unavailable (blocked since API 29), so socket-scoped reaping is not an option from the app; the guest-side fix is what removes the need for any of this. + +**Disposition.** Keep D2 as-is now (required: without it a legitimate relaunch cannot rebind `:8085`, §5). **Retire it when the guest-side service-lifetime fix lands** — at which point `reapEnvironmentHttpFront()` and the `ENV_HTTP_PORT` constant come out and `killOrphan` returns to signalling the proot alone. Tracked here; not a Phase-4 blocker, but the natural companion to the rootfs/runrole work that owns guest teardown. + From 1a01363d1001080156cf0a87f1a14b212477261b Mon Sep 17 00:00:00 2001 From: Luis Guzman Date: Sat, 29 Aug 2026 06:19:42 -0600 Subject: [PATCH 06/10] ADFA-5343 refactor(server-lifecycle): drop dead proot-age readers orphaned by D1 ADR-5343a D1 replaced the proot-age escalation grace with a service-downtime clock, dropping envAgeMs from EnvironmentEnsure.decide. That left EnvironmentProcess. environmentAgeMs() (public, no remaining callers) and its only user readStartTicks() (private) as dead code. Remove both - the reduction D1 implies. No behavior change; killOrphan / reapEnvironmentHttpFront / isRunning are untouched. Verified: :app:compileDebugJavaWithJavac + :app:lintDebug BUILD SUCCESSFUL. Co-Authored-By: Claude Opus 4.8 --- .../controller/env/EnvironmentProcess.java | 68 ------------------- 1 file changed, 68 deletions(-) diff --git a/controller/app/src/main/java/org/iiab/controller/env/EnvironmentProcess.java b/controller/app/src/main/java/org/iiab/controller/env/EnvironmentProcess.java index 64b3eb2ba..859fa9ad2 100644 --- a/controller/app/src/main/java/org/iiab/controller/env/EnvironmentProcess.java +++ b/controller/app/src/main/java/org/iiab/controller/env/EnvironmentProcess.java @@ -103,74 +103,6 @@ public static boolean isRunning(Context ctx) { return ctx != null && findPid(ctx) > 0; } - /** - * The running environment proot's age in ms, or {@code -1} when there is none or its start time - * cannot be read. - * - *

ADFA-5103: the boot grace is measured from the proot's own age, not from when this process - * launched it, so a young proot is protected whether we started it or a force-closed predecessor - * did — the observed 3.5 s double-boot after Android restored the Activity stack. Read from - * {@code /proc//stat} field 22 (starttime, in clock ticks since system boot) and compared - * against {@link android.os.SystemClock#elapsedRealtime()}, which counts from the same boot. - */ - public static long environmentAgeMs(Context ctx) { - if (ctx == null) { - return -1L; - } - int pid = findPid(ctx); - if (pid <= 0) { - return -1L; - } - long startTicks = readStartTicks(pid); - if (startTicks < 0) { - return -1L; - } - long clkTck; - try { - clkTck = android.system.Os.sysconf(android.system.OsConstants._SC_CLK_TCK); - } catch (Throwable t) { - clkTck = 100L; // the near-universal default; a wrong value only shifts the grace slightly - } - if (clkTck <= 0) { - clkTck = 100L; - } - long startedAtSinceBootMs = (startTicks * 1000L) / clkTck; - long ageMs = android.os.SystemClock.elapsedRealtime() - startedAtSinceBootMs; - return ageMs < 0 ? -1L : ageMs; - } - - /** - * {@code /proc//stat} field 22 (starttime), or {@code -1} if unreadable. The comm field (2) - * is wrapped in parentheses and may itself contain spaces and parentheses, so the fields after - * it are parsed from the last {@code ')'} — starttime is the 19th token after that. - */ - private static long readStartTicks(int pid) { - File stat = new File("/proc/" + pid + "/stat"); - try (FileInputStream in = new FileInputStream(stat)) { - byte[] buf = new byte[4096]; - int total = 0, r; - while (total < buf.length && (r = in.read(buf, total, buf.length - total)) != -1) { - total += r; - } - if (total == 0) { - return -1L; - } - String content = new String(buf, 0, total); - int lastParen = content.lastIndexOf(')'); - if (lastParen < 0 || lastParen + 2 >= content.length()) { - return -1L; - } - String[] after = content.substring(lastParen + 2).trim().split("\\s+"); - // after[0] is field 3 (state); starttime is field 22 -> index 22 - 3 = 19. - if (after.length <= 19) { - return -1L; - } - return Long.parseLong(after[19]); - } catch (Exception e) { - return -1L; // vanished, not ours to read, or an unexpected shape - } - } - /** * Stop an environment proot this process has no handle on. * From f2882028fecb17d6f08c9797155218febed03b57 Mon Sep 17 00:00:00 2001 From: Luis Guzman Date: Sat, 29 Aug 2026 06:24:27 -0600 Subject: [PATCH 07/10] ADFA-5343 fix(server-lifecycle): close the two-writer race on lastLiveness (Phase 2) lastLiveness has two writers, not one (the review's finding, correcting the stale 'single writer' comment): the poll advances it, and doLaunchEnvironment resets it to null so a fresh proot gets its full grace instead of the old one's inherited service downtime. Before this, the poll read prev (outside any lock), then wrote next(prev,..) ~2.5s later after apiReady() - so a reset that landed during that window was clobbered by the poll's stale prev, and the next ensure-up tick could KILL_AND_RELAUNCH the freshly-booting proot (the 5336 loop, in miniature). Serialise the two writers with livenessLock: probe OUTSIDE the lock (apiReady may block ~2.5s and must not hold it), then read prev and write the next snapshot atomically INSIDE it; doLaunchEnvironment's reset takes the same lock. A reset can no longer be clobbered by a poll that read prev before it. No lock is ever held across the network probe, and the reset's critical section is a single field write, so the UI thread never stalls. The ensure-up decision keeps its lock-free volatile read. Verified: :app:testDebugUnitTest + :app:lintDebug BUILD SUCCESSFUL locally. Device-only (touches the reset path): re-run Flow 2 (kill dash-node on a mature proot) - auto-recovers to UP, rebinds :8085, no relaunch loop. Co-Authored-By: Claude Opus 4.8 --- .../org/iiab/controller/ServerController.java | 43 +++++++++++++------ 1 file changed, 29 insertions(+), 14 deletions(-) diff --git a/controller/app/src/main/java/org/iiab/controller/ServerController.java b/controller/app/src/main/java/org/iiab/controller/ServerController.java index 733e4e61a..9a821e36d 100644 --- a/controller/app/src/main/java/org/iiab/controller/ServerController.java +++ b/controller/app/src/main/java/org/iiab/controller/ServerController.java @@ -87,9 +87,17 @@ default void onStartupProgress(String service) {} private volatile boolean ensuring = false; // ADFA-5343a (D1): the last liveness snapshot from the poll, threaded so servicesDownSinceMs // measures CONTINUOUS observed downtime (reset on an observation gap). Read by the ensure-up - // decision to key the kill on service downtime, not proot age. Volatile: written on the poll's IO - // thread, read by the (also-IO) ensure-up decision; the poll is the single writer. + // decision to key the kill on service downtime, not proot age. + // + // ADFA-5343 (Phase 2): TWO writers, not one — the poll advances it, and doLaunchEnvironment resets + // it to null when a fresh proot starts (so the new proot gets its full grace, not the old one's + // inherited downtime). They are serialised by livenessLock: the poll reads prev and writes the next + // snapshot atomically under the lock (AFTER probing, so the ~2.5s network probe never holds it), and + // the boot reset takes the same lock — so a reset can never be clobbered by a poll that had read prev + // before it. Still volatile, for the lock-free read on the ensure-up decision path. private volatile org.iiab.controller.env.domain.ServerLiveness lastLiveness; + /** Guards the read-prev-then-write of {@link #lastLiveness} against the boot-time reset (Phase 2). */ + private final Object livenessLock = new Object(); private static final java.util.regex.Pattern PDSM_SVC = java.util.regex.Pattern.compile("\\[pdsm:([^\\]]+)\\]"); private final Handler timeoutHandler = new Handler(android.os.Looper.getMainLooper()); @@ -172,17 +180,21 @@ private void checkServerStatus() { // alive stays a 1-bit fact (phase == UP) so ServerStateRepository and every reader are // unchanged here — the only shift is that "up" now means the services answer, not nginx. long now = android.os.SystemClock.elapsedRealtime(); - // ADFA-5343a (D1): thread the snapshot so servicesDownSinceMs measures CONTINUOUS observed - // downtime; next() resets it on an observation gap (a stale previous snapshot), so a - // background gap can never read as long downtime and re-drive the kill loop on resume. - org.iiab.controller.env.domain.ServerLiveness liveness = - org.iiab.controller.env.domain.ServerLiveness.next( - lastLiveness, - org.iiab.controller.env.EnvironmentProcess.isRunning(activity), - org.iiab.controller.redesign.RestReadiness.apiReady(), - now, - org.iiab.controller.env.domain.ServerLiveness.DEFAULT_FRESH_MS); - lastLiveness = liveness; + // Probe OUTSIDE the lock — RestReadiness.apiReady() can block ~2.5s and must never hold + // livenessLock (that would stall a concurrent boot reset). + boolean processPresent = org.iiab.controller.env.EnvironmentProcess.isRunning(activity); + boolean servicesAnswering = org.iiab.controller.redesign.RestReadiness.apiReady(); + // ADFA-5343a (D1) / ADFA-5343 (Phase 2): read prev and write the next snapshot atomically + // under livenessLock, so a boot-time reset (doLaunchEnvironment) is never clobbered by this + // poll writing a prev it had read before the reset. next() still measures CONTINUOUS downtime + // and resets it on an observation gap (a stale prev — e.g. the app was backgrounded). + org.iiab.controller.env.domain.ServerLiveness liveness; + synchronized (livenessLock) { + liveness = org.iiab.controller.env.domain.ServerLiveness.next( + lastLiveness, processPresent, servicesAnswering, now, + org.iiab.controller.env.domain.ServerLiveness.DEFAULT_FRESH_MS); + lastLiveness = liveness; + } boolean localAlive = liveness.phase(now) == org.iiab.controller.env.domain.ServerLiveness.Phase.UP; @@ -355,7 +367,10 @@ private void doLaunchEnvironment() { // ADFA-5343a (D1): a fresh environment is starting — restart the service-downtime clock so the // new proot gets its full boot grace. Without this a KILL_AND_RELAUNCH keeps the accumulated // downtime and re-kills the booting proot every tick, before its services can come up. - lastLiveness = null; + // ADFA-5343 (Phase 2): under livenessLock so the poll cannot clobber this reset with a snapshot + // whose prev it read before the reset (the two-writer race). Runs on the UI thread; the critical + // section is a single field write, so it never blocks on the poll's probe. + synchronized (livenessLock) { lastLiveness = null; } createFakeSysData(rootfsDir); if (serverEngine != null) serverEngine.killProcess(); serverEngine = new PRootEngine(); From 981b214269517a55acf30d4a56bf914a08c68bd4 Mon Sep 17 00:00:00 2001 From: Luis Guzman Date: Sat, 29 Aug 2026 10:07:59 -0600 Subject: [PATCH 08/10] =?UTF-8?q?ADFA-5343=20feat(dashboard):=20in-proot?= =?UTF-8?q?=20pdsm=20restart=20for=20content-service=20recovery=20(ADR-534?= =?UTF-8?q?3a=20=C2=A710)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A content service wedged after an environment relaunch loses proot's syscall emulation (epoll_wait -> ENOSYS) and stops serving (kiwix "Unavailable"). Recover it in place, inside the one living proot, via `pdsm restart ` through the dashboard REST core -- never a host-side reap or a proot relaunch (ADR-5343a §10). - services.ts: exact-match allowlist mirroring upstream pdsm_installed_services (dash-node excluded) + setsid-detached restartService actuator. - routes.ts: POST /system/service/:svc/restart (loopback-only, like all /k2go-api). - service-heal.ts: in-proot watcher that HEAD-probes the content tree and heals a present-but-wedged service (404 = not installed -> left alone), cooldown-bounded. - Pure allowlist/cooldown/probe-classification unit-tested; the wedge -> auto-restart recovery is device-only (verified by hand). Co-Authored-By: Claude Opus 4.8 --- .../docs/ADR-5343a-flap-recovery-delta.md | 32 +++++++ static/dashboard/package.json | 2 +- static/dashboard/routes.ts | 18 ++++ static/dashboard/server.ts | 3 + static/dashboard/sockets/service-heal.ts | 87 +++++++++++++++++++ static/dashboard/sockets/services.test.ts | 69 +++++++++++++++ static/dashboard/sockets/services.ts | 44 ++++++++++ 7 files changed, 254 insertions(+), 1 deletion(-) create mode 100644 static/dashboard/sockets/service-heal.ts create mode 100644 static/dashboard/sockets/services.test.ts create mode 100644 static/dashboard/sockets/services.ts diff --git a/controller/docs/ADR-5343a-flap-recovery-delta.md b/controller/docs/ADR-5343a-flap-recovery-delta.md index 51a82fea6..b9b91616d 100644 --- a/controller/docs/ADR-5343a-flap-recovery-delta.md +++ b/controller/docs/ADR-5343a-flap-recovery-delta.md @@ -118,3 +118,35 @@ D2's `EnvironmentProcess.reapEnvironmentHttpFront()` (`env/EnvironmentProcess.ja **Disposition.** Keep D2 as-is now (required: without it a legitimate relaunch cannot rebind `:8085`, §5). **Retire it when the guest-side service-lifetime fix lands** — at which point `reapEnvironmentHttpFront()` and the `ENV_HTTP_PORT` constant come out and `killOrphan` returns to signalling the proot alone. Tracked here; not a Phase-4 blocker, but the natural companion to the rootfs/runrole work that owns guest teardown. +--- + +## 10. Corrected root cause (device testing) and the in-proot recovery direction + +Post-D1/D2 device testing surfaced that a **wedged content service does not recover after an environment relaunch** (Kiwix "Unavailable"; the network-dashboard tiles stuck on "connecting"), and traced it to a single root cause broader than §9's nginx case. + +**Root cause — an orphan off proot loses proot's syscall emulation.** proot traces the box's processes and emulates syscalls the host kernel does not serve. When a service is reparented to init by a relaunch (it `setsid`s away and outlives the proot), it runs **without proot's emulation**: its `epoll_wait` hits the bare host kernel and returns **ENOSYS (38)**, so it malfunctions. This one defect is behind three device-observed symptoms: kiwix-serve **hangs** (`:3000` accepting but never serving; curl → 000), nginx **crashes** (`epoll_wait() failed (38)`), and php-fpm **busy-loops logging the error** (~1.3 GB/min → filled `/data`). + +The `epoll_wait` failures reported "kernel 6.17.0" — that is **proot's *spoofed* kernel version** (proot presents an invented version to escape the phone kernel's restrictions), not the device's real kernel (~4.x). The kernel version is a **red herring**; the defect is the loss of proot tracing on orphaning, not any kernel release. + +**Direction (decided) — recover in place, in one proot, via the dashboard REST core; do not relaunch proots to fix a wedged service.** Recovery of a content service that is wedged **while the proot is alive** (dash-node / `k2go-api` still answering) must be an **in-proot `pdsm restart `** issued through the dashboard REST core (`static/dashboard` — dash-node, which already execs per-service in-proot and already calls `pdsm restart dash-node`, `routes.ts:283`). Because the service is never detached from the proot, the orphan → ENOSYS class **cannot arise**. This **supersedes** "extend the host-side reap to more services" (which would grow a compensator that chases the whole service tree): the app must not hunt or kill the box's services at all. + +**Layering (one owner per fact).** +- *Android app / reconciler* owns box up/down (the proot and dash-node liveness). It does **not** manage or reap individual box services. +- *Dashboard REST core (in-proot)* owns its own service tree: it already detects a down service (the served page's `fetch("/kiwix/")` HEAD → 000) and restarts it via `pdsm restart ` in the **same** proot. Recovery is **auto-heal** here (the box heals itself; the tile only reflects status), with the existing manual "Retry" as a backstop that triggers the same in-proot restart. +- *pdsm* is the per-service mechanism (already complete: `enable/start/stop/restart`). + +**Consequence for D2.** The host-side reap (§9) shrinks to the narrow **genuine proot-death** case only (nginx orphaning while holding `:8085`); the common **wedged-service-on-a-live-proot** case moves to the in-proot REST restart and never orphans. D2 is still retired with the guest-side service-lifetime fix (§9). + +**Scope split.** +- *Here (this effort):* `static/dashboard` — a per-service `pdsm restart ` for content-service recovery (kiwix first, generalizable to the other supported services), auto-healed on a detected-down service, Retry as backstop. + + **Supported services (source of truth — do not invent).** The authoritative list is + `pdsm_installed_services` in `iiab/iiab` `roles/proot_services/defaults/main.yml`. As of today: + **`nginx`, `php-fpm`, `mariadb`, `kolibri`, `kiwix`, `calibre-web`** (only `nginx` enabled by + default). This list **lives upstream in `iiab/iiab` and will grow** — keep an eye on that role + rather than hard-coding a fixed set; the restart capability should accept a service name from the + supported set, not a baked-in enum. **`dash-node` is the exception:** it is the k2go-side service + (it lives in this repo, `static/dashboard`), not in `iiab/iiab`. +- *Separate, upstream `iiab/iiab` (via `tools/upstream-patches`):* a php-fpm guard so it cannot busy-loop-log on `epoll_wait` ENOSYS and fill the disk — defense-in-depth, and far less likely once services stop orphaning. Its own ticket. +- *Non-issue:* the "kernel 6.17" — proot's spoofed version; no action, recorded so no one chases it again. + diff --git a/static/dashboard/package.json b/static/dashboard/package.json index 10e8b23e9..d6d1a04af 100644 --- a/static/dashboard/package.json +++ b/static/dashboard/package.json @@ -4,7 +4,7 @@ "description": "", "main": "index.js", "scripts": { - "test": "node --require ts-node/register --test sockets/maps.socket.test.ts sockets/rolling-log.test.ts sockets/kolibri.session.test.ts sockets/credentials.test.ts sockets/net-retry.test.ts", + "test": "node --require ts-node/register --test sockets/maps.socket.test.ts sockets/rolling-log.test.ts sockets/kolibri.session.test.ts sockets/credentials.test.ts sockets/net-retry.test.ts sockets/services.test.ts", "test:db": "node --require ts-node/register --test sockets/jobs.test.ts", "typecheck": "tsc --noEmit", "build": "tsc", diff --git a/static/dashboard/routes.ts b/static/dashboard/routes.ts index 43f9412d2..6ef6fc99c 100644 --- a/static/dashboard/routes.ts +++ b/static/dashboard/routes.ts @@ -20,6 +20,7 @@ import { checkReadiness, KolibriAuthError, KolibriApiError, login as kolibriLogi import { describeCredential, setCredential, clearCredential, isServiceName, } from './sockets/credentials'; +import { isRestartableService, restartService } from './sockets/services'; // ADFA-4879: FQR helpers reached from the app (in-app region download/delete instead of the // copy-paste-into-a-terminal flow). tile-extract.py is installed on the box by the upstream maps @@ -418,6 +419,23 @@ apiRouter.get('/system/dashboard/update-check', async (_req: Request, res: Respo } }); +// --- System: in-proot per-service restart for content-service recovery (ADFA-5343, ADR-5343a §10) --- +// A content service wedged after an environment relaunch (kiwix "Unavailable") lost proot's syscall +// emulation by being orphaned off proot. Recover it IN PLACE via `pdsm restart ` inside the one +// living proot — the app must NOT hunt or reap box services (§10 layering). Loopback-only, like the +// whole /k2go-api surface (dash-node-nginx.conf): the on-box callers reach it (the in-proot auto-heal +// watcher, and the app's future module-card Retry backstop); a client device cannot. Fire-and-forget: +// the restart takes a few seconds and the caller re-probes to reflect status, so we answer 202 at once. +apiRouter.post('/system/service/:svc/restart', (req: Request, res: Response): void => { + const svc = String(req.params.svc || ''); + if (!isRestartableService(svc)) { + res.status(400).json({ error: 'unknown service', service: svc }); + return; + } + restartService(svc); + res.status(202).json({ ok: true, service: svc, restarting: true }); +}); + // --- Kolibri: readiness, catalogue and selection (ADFA-4949) ---------------------- // Direct (non-job) queries. The download itself is a durable job // (POST /kolibri/download), which comes free from adding 'kolibri' to VALID_TYPES. diff --git a/static/dashboard/server.ts b/static/dashboard/server.ts index a872df113..feac9f5c9 100644 --- a/static/dashboard/server.ts +++ b/static/dashboard/server.ts @@ -10,6 +10,7 @@ import './sockets/maps.exec'; import './sockets/books.exec'; import './sockets/kolibri.exec'; import { apiRouter } from './routes'; +import { startServiceHeal } from './sockets/service-heal'; const app = express(); const server = http.createServer(app); @@ -37,6 +38,8 @@ server.listen(PORT, '127.0.0.1', () => { console.log(`===========================================`); // ADFA-4838: resume any content jobs that were mid-flight before a restart. try { jobs.reconcileOnBoot(); } catch (e) { console.error('[jobs] reconcile failed', e); } + // ADFA-5343 (ADR-5343a §10): the box heals its own content-service tree in-proot. + try { startServiceHeal(); } catch (e) { console.error('[service-heal] start failed', e); } }); // ========================================== diff --git a/static/dashboard/sockets/service-heal.ts b/static/dashboard/sockets/service-heal.ts new file mode 100644 index 000000000..0ee5ebd65 --- /dev/null +++ b/static/dashboard/sockets/service-heal.ts @@ -0,0 +1,87 @@ +// sockets/service-heal.ts — ADFA-5343 (ADR-5343a §10) +// +// "The box heals itself": a small in-proot watcher that probes the content services on +// loopback and, on a wedged/down one, issues an in-proot `pdsm restart ` — the same +// actuator the app's manual Retry calls (routes.ts -> services.restartService). +// +// Why server-side, on the box: the recovery endpoint is loopback-only (dash-node-nginx.conf +// `allow 127.0.0.1; deny all`). The client captive page can *detect* a down tile but cannot +// *reach* the restart, so healing cannot live there — the tile only reflects status while the +// box heals itself (§10). The app/reconciler owns box up/down and does NOT manage individual +// box services (§10 layering); this watcher is the dashboard REST core owning its own service +// tree — the single owner of "is my content service tree healthy?". +import { restartService } from './services'; + +/** A watched content service: its pdsm name and the loopback URL that reflects its health. + * kiwix first (the device-confirmed wedge, §10); the others are added here as they are + * device-verified — the endpoint already accepts the full supported set. */ +interface Watch { svc: string; probeUrl: string; } + +// The box fronts content on nginx :8085 (dash-node-nginx.conf); a HEAD on the public path is +// exactly the served page's own probe, run here on loopback. Origin/timing are env-overridable +// so a box on a non-default port needs no code change (mirrors PORT / K2GO_* elsewhere). +const PUBLIC_ORIGIN = process.env.K2GO_PUBLIC_ORIGIN || 'http://127.0.0.1:8085'; +const INTERVAL_MS = Number(process.env.K2GO_HEAL_INTERVAL_MS) || 30_000; +const COOLDOWN_MS = Number(process.env.K2GO_HEAL_COOLDOWN_MS) || 60_000; +const PROBE_TIMEOUT_MS = Number(process.env.K2GO_HEAL_PROBE_TIMEOUT_MS) || 4_000; + +const WATCHED: Watch[] = [ + { svc: 'kiwix', probeUrl: `${PUBLIC_ORIGIN}/kiwix/` }, +]; + +/** Enough time has passed since the last restart attempt to try again. A per-service + * cooldown is the only state the loop keeps: it bounds a wedged service to one restart per + * window (never a restart storm) and gives the freshly-restarted service time to come back + * before it is judged again. Pure, so the timing is unit-tested off device. */ +export function dueForRestart(lastAttemptMs: number, nowMs: number, cooldownMs: number): boolean { + return nowMs - lastAttemptMs >= cooldownMs; +} + +export type ProbeResult = 'ok' | 'down' | 'absent'; + +/** Classify a probe's HTTP status (or null for a network error / timeout) into a heal decision. + * Pure, so the not-installed-vs-wedged split is unit-tested off device — the same split the + * served page's discovery makes (404 -> the card is not installed, dropped from monitoring): + * - 2xx -> ok (serving) + * - 404 -> absent (the box does not front this service at all; not installed — + * nothing to heal, or we would restart a service that isn't there) + * - any other status-> down (5xx/502/504: installed but the upstream is wedged) + * - null (no reply) -> down (timeout / refused: not serving) */ +export function classifyProbe(status: number | null): ProbeResult { + if (status === null) return 'down'; + if (status >= 200 && status < 300) return 'ok'; + if (status === 404) return 'absent'; + return 'down'; +} + +/** HEAD the probe URL and classify the outcome. Only a 'down' verdict heals; 'absent' (404, + * not installed) and 'ok' are left alone. */ +async function probe(url: string): Promise { + try { + const res = await fetch(url, { method: 'HEAD', signal: AbortSignal.timeout(PROBE_TIMEOUT_MS) }); + return classifyProbe(res.status); + } catch { + return classifyProbe(null); + } +} + +/** Start the watcher. Returns the interval handle (unref'd so it never keeps the process + * alive on its own). Safe to call once from server.ts after listen(). */ +export function startServiceHeal(): NodeJS.Timeout { + const lastAttempt = new Map(); + + const tick = async (): Promise => { + for (const w of WATCHED) { + if (await probe(w.probeUrl) !== 'down') continue; // 'ok'/'absent' need no heal + const now = Date.now(); + if (!dueForRestart(lastAttempt.get(w.svc) ?? 0, now, COOLDOWN_MS)) continue; + lastAttempt.set(w.svc, now); + console.log(`[service-heal] ${w.svc} down (${w.probeUrl}); pdsm restart ${w.svc}`); + restartService(w.svc); + } + }; + + const handle = setInterval(() => { void tick(); }, INTERVAL_MS); + handle.unref?.(); + return handle; +} diff --git a/static/dashboard/sockets/services.test.ts b/static/dashboard/sockets/services.test.ts new file mode 100644 index 000000000..a7afa8489 --- /dev/null +++ b/static/dashboard/sockets/services.test.ts @@ -0,0 +1,69 @@ +/// +import test from 'node:test'; +import assert from 'node:assert/strict'; +import { isRestartableService, RESTARTABLE_SERVICES } from './services'; +import { dueForRestart, classifyProbe } from './service-heal'; + +// --- isRestartableService: the /system/service/:svc/restart allowlist ------------------------- + +test('isRestartableService: accepts every upstream content service', () => { + for (const svc of RESTARTABLE_SERVICES) { + assert.equal(isRestartableService(svc), true, svc); + } + // The concrete cases §10 wires first. + assert.equal(isRestartableService('kiwix'), true); + assert.equal(isRestartableService('kolibri'), true); +}); + +test('isRestartableService: rejects dash-node (k2go\'s own service, not a content service)', () => { + // Bouncing dash-node would kill the process serving the request and the heal loop; its + // restart lives in the rebuild path, never this endpoint. + assert.equal(isRestartableService('dash-node'), false); +}); + +test('isRestartableService: rejects unknown, empty and shell-metacharacter names', () => { + assert.equal(isRestartableService(''), false); + assert.equal(isRestartableService('unknown'), false); + assert.equal(isRestartableService('KIWIX'), false); // exact match only + assert.equal(isRestartableService('kiwix '), false); // no trailing space + assert.equal(isRestartableService('kiwix; rm -rf /'), false); + assert.equal(isRestartableService('kiwix && reboot'), false); + assert.equal(isRestartableService('../kiwix'), false); +}); + +// --- dueForRestart: the per-service cooldown that bounds restarts ------------------------------ + +test('dueForRestart: true on the first attempt (no prior restart)', () => { + assert.equal(dueForRestart(0, 1_000_000, 60_000), true); +}); + +test('dueForRestart: false while still inside the cooldown window', () => { + const now = 1_000_000; + assert.equal(dueForRestart(now - 59_999, now, 60_000), false); +}); + +test('dueForRestart: true once the cooldown has elapsed (boundary is inclusive)', () => { + const now = 1_000_000; + assert.equal(dueForRestart(now - 60_000, now, 60_000), true); + assert.equal(dueForRestart(now - 120_000, now, 60_000), true); +}); + +// --- classifyProbe: heal only a present-but-wedged service, never an absent one ---------------- + +test('classifyProbe: 2xx is ok (serving), never healed', () => { + assert.equal(classifyProbe(200), 'ok'); + assert.equal(classifyProbe(204), 'ok'); +}); + +test('classifyProbe: 404 is absent (not installed) — the not-installed vs not-running split', () => { + // A box without this content fronts no such path; healing would restart a service that isn't + // there, every cooldown, forever. Must be left alone. + assert.equal(classifyProbe(404), 'absent'); +}); + +test('classifyProbe: a wedged upstream (5xx) and no reply (null) both heal', () => { + assert.equal(classifyProbe(502), 'down'); + assert.equal(classifyProbe(503), 'down'); + assert.equal(classifyProbe(504), 'down'); + assert.equal(classifyProbe(null), 'down'); // timeout / connection refused +}); diff --git a/static/dashboard/sockets/services.ts b/static/dashboard/sockets/services.ts new file mode 100644 index 000000000..4f093e2df --- /dev/null +++ b/static/dashboard/sockets/services.ts @@ -0,0 +1,44 @@ +// sockets/services.ts — ADFA-5343 (ADR-5343a §10) +// +// In-proot per-service restart for content-service recovery. A content service that was +// orphaned off proot by an environment relaunch loses proot's syscall emulation +// (epoll_wait -> ENOSYS) and wedges (kiwix-serve hangs, nginx crashes, php-fpm busy-loops). +// The fix is to recover it IN PLACE, inside the one living proot, via `pdsm restart ` +// — never a host-side reap and never a proot relaunch (ADR-5343a §10). Because the service +// is never detached from the proot, the orphan -> ENOSYS class cannot arise. +// +// Supported services — the SOURCE OF TRUTH is upstream; this is only a mirror. It tracks +// `pdsm_installed_services` in iiab/iiab roles/proot_services/defaults/main.yml. That list +// lives upstream and will grow — extend this mirror when it does, rather than inventing +// names here. `dash-node` is deliberately excluded: it is k2go's own service (this repo), +// not a content service, and bouncing it would kill the process serving the request and this +// heal loop — its restart already lives in the rebuild path (routes.ts). +import { spawn } from 'child_process'; + +export const RESTARTABLE_SERVICES: readonly string[] = [ + 'nginx', 'php-fpm', 'mariadb', 'kolibri', 'kiwix', 'calibre-web', +]; + +/** A name is restartable only when it is exactly one of the known upstream content + * services. Pure and exact-match, so a service name can never carry a shell + * metacharacter into the spawn below (defence in depth — spawn already runs no shell). */ +export function isRestartableService(name: string): boolean { + return RESTARTABLE_SERVICES.includes(name); +} + +const PDSM = '/usr/local/bin/pdsm'; + +/** Fire-and-forget `pdsm restart ` in-proot. setsid => own session, mirroring the + * dash-node rebuild call (routes.ts): a restart can never kill the run that issued it. + * Detached + stdio ignored + unref so dash-node does not wait on it. The caller MUST have + * validated `svc` with isRestartableService first. Spawn failures are logged, never thrown: + * an unheard 'error' event would otherwise crash the REST core. */ +export function restartService(svc: string): void { + try { + const child = spawn('setsid', [PDSM, 'restart', svc], { detached: true, stdio: 'ignore' }); + child.on('error', (e) => console.error(`[services] restart ${svc} failed to spawn: ${e}`)); + child.unref(); + } catch (e) { + console.error(`[services] restart ${svc} spawn threw: ${e}`); + } +} From 2596cb8025f6b6d506199bcbc85d3120fc3024c5 Mon Sep 17 00:00:00 2001 From: Luis Guzman Date: Sat, 29 Aug 2026 13:09:27 -0600 Subject: [PATCH 09/10] ADFA-5343 chore(dashboard): bump dash-node 1.2.9 -> 1.2.10 for the restart endpoint MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit REST-facing change (POST /system/service/:svc/restart) — the CHANGELOG rule bumps the version so the app's update-check surfaces it. Missed in 981b2142; corrected forward. Co-Authored-By: Claude Opus 4.8 --- static/dashboard/CHANGELOG.md | 1 + static/dashboard/package.json | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/static/dashboard/CHANGELOG.md b/static/dashboard/CHANGELOG.md index fab665bc5..e91cc4f51 100644 --- a/static/dashboard/CHANGELOG.md +++ b/static/dashboard/CHANGELOG.md @@ -4,6 +4,7 @@ One line per version, newest first. Every REST-facing change bumps the version i (the app surfaces it via `/system/dashboard/update-check` and the "Update available" pill), so this file is the human record of what each bump enables. Keep entries short: `version - change (TICKET)`. +- **1.2.10** - In-proot content-service recovery (ADFA-5343, ADR-5343a §10). New `POST /system/service/:svc/restart`: runs `pdsm restart ` in the one living proot to recover a content service wedged after an environment relaunch (orphaned off proot → `epoll_wait` ENOSYS), for the supported upstream services (mirrors `pdsm_installed_services`; `dash-node` excluded). A server-side watcher auto-heals a present-but-wedged content service (404 = not installed → left alone), cooldown-bounded; the app's future module-card Retry is the manual backstop hitting the same endpoint. Loopback-only, like all of `/k2go-api`. (ADFA-5343) - **1.2.9** - Cancelable dashboard self-update (ADFA-5333). New `POST /system/dashboard/rebuild/cancel`: stops an in-flight rebuild cleanly while it is still **building** (git fetch + staging build + smoke test — none of which touch the live dashboard) by signaling the detached `setsid` session group; **refused during "promoting"** (the short dist-swap + restart window) so the swap is never interrupted mid-flight, and a no-op (409) when nothing is running. To support this, `tools/rebuild-dashboard.sh` now records its coarse phase (`building`/`promoting`) and its session-leader pid, and its cleanup trap fires on TERM/INT so a canceled run leaves no staging behind. Pairs with the app running the update in the background with a Cancel action (ADFA-5333). (ADFA-5333) - **1.2.7** - Books homologated to the ZIM contract (ADFA-4893). The books runner now surfaces its reconnect state on the poll (`retryAttempt`/`retryTotal` via `ctx.reportRetry`), so the app can show "Reconnecting… n of N" like ZIM/rootfs (books' per-item budget is 6 tries → shows n of 5, same label). And the runner is now **idempotent on resume**: it skips books already in the Calibre-Web library (matched by title) instead of re-downloading + re-uploading them — which used to duplicate entries and reset the percent to 0 — so a resume, or a re-run after process death, is safe. (ADFA-4893) - **1.2.6** - Visible reconnect + honest resume for kiwix (ADFA-4893). The outer reconnect loop now surfaces its state on the poll — `GET /kiwix/jobs/:id` gains `retryAttempt`/`retryTotal` (in-memory, no DB change) — so the app can show "Reconnecting n/5". The schedule is now an explicit **3/6/9/18/36 s** (5 visible waits) instead of exponential, and **pause breaks a backoff wait immediately** (the loop takes `ctx.signal`), so the on-screen X during a reconnect pauses at once. Resume no longer flashes 0%: `runKiwix` stopped sending `percent: 0` on its initial update, so a resumed job keeps its retained percent until aria2 `--continue` re-reports (a fresh job stays `-1` until the first progress). (ADFA-4893) diff --git a/static/dashboard/package.json b/static/dashboard/package.json index d6d1a04af..d638da191 100644 --- a/static/dashboard/package.json +++ b/static/dashboard/package.json @@ -1,6 +1,6 @@ { "name": "dashboard-console", - "version": "1.2.9", + "version": "1.2.10", "description": "", "main": "index.js", "scripts": { From 7332f67439ca6db9d1d08d30999cc10e6c83597d Mon Sep 17 00:00:00 2001 From: Luis Guzman Date: Sat, 29 Aug 2026 15:54:59 -0600 Subject: [PATCH 10/10] =?UTF-8?q?ADFA-5343=20docs(server-lifecycle):=20ADR?= =?UTF-8?q?-5343a=20=C2=A710=20accuracy=20pass=20(loopback=20boundary,=20a?= =?UTF-8?q?bsent-vs-down,=20kiwix-only=20watcher,=20service=20source-of-tr?= =?UTF-8?q?uth)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Align §10 with the implemented dashboard recovery (981b2142): - auto-heal is server-side in dash-node; the restart endpoint is loopback-only, so captive-portal clients reflect status but cannot trigger a restart; manual Retry is the on-box Android card (ADFA-4842). - probe classifies absent (404) vs down (5xx/timeout) so an uninstalled service is never restart-looped. - the watcher wires kiwix only today; others added to WATCHED per device-verify. - supported-service list source of truth = iiab/iiab proot_services (pdsm_installed_services); dash-node is the k2go-side exception. --- controller/docs/ADR-5343a-flap-recovery-delta.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/controller/docs/ADR-5343a-flap-recovery-delta.md b/controller/docs/ADR-5343a-flap-recovery-delta.md index b9b91616d..8d772771e 100644 --- a/controller/docs/ADR-5343a-flap-recovery-delta.md +++ b/controller/docs/ADR-5343a-flap-recovery-delta.md @@ -132,13 +132,13 @@ The `epoll_wait` failures reported "kernel 6.17.0" — that is **proot's *spoofe **Layering (one owner per fact).** - *Android app / reconciler* owns box up/down (the proot and dash-node liveness). It does **not** manage or reap individual box services. -- *Dashboard REST core (in-proot)* owns its own service tree: it already detects a down service (the served page's `fetch("/kiwix/")` HEAD → 000) and restarts it via `pdsm restart ` in the **same** proot. Recovery is **auto-heal** here (the box heals itself; the tile only reflects status), with the existing manual "Retry" as a backstop that triggers the same in-proot restart. +- *Dashboard REST core (in-proot)* owns its own service tree. **Auto-heal is server-side** in dash-node — an in-proot watcher HEAD-probes the content services and restarts a present-but-wedged one via `pdsm restart ` in the **same** proot (works with no browser open, which "the box heals itself" requires). The restart endpoint is **loopback-only** (behind `/k2go-api`'s `allow 127.0.0.1; deny all`), so captive-portal clients on the hotspot can *detect* a down tile but **cannot** trigger a restart — the served page only reflects status. The manual **Retry** backstop is the on-box Android module card (loopback, owned by the merged **ADFA-4842**), out of scope for the dashboard change; the endpoint is ready for it. Probe classification splits **absent** (404 → not installed, left alone) from **down** (5xx / timeout / refused → heal), so a box without a service is never restart-looped. - *pdsm* is the per-service mechanism (already complete: `enable/start/stop/restart`). **Consequence for D2.** The host-side reap (§9) shrinks to the narrow **genuine proot-death** case only (nginx orphaning while holding `:8085`); the common **wedged-service-on-a-live-proot** case moves to the in-proot REST restart and never orphans. D2 is still retired with the guest-side service-lifetime fix (§9). **Scope split.** -- *Here (this effort):* `static/dashboard` — a per-service `pdsm restart ` for content-service recovery (kiwix first, generalizable to the other supported services), auto-healed on a detected-down service, Retry as backstop. +- *Here (this effort):* `static/dashboard` — a per-service `pdsm restart ` for content-service recovery, auto-healed on a detected-down service, Retry as backstop. **The watcher wires `kiwix` only today** (the device-confirmed wedge); the other supported services are added to `WATCHED` **one at a time as each is device-verified** — the restart endpoint already accepts the full supported set. **Supported services (source of truth — do not invent).** The authoritative list is `pdsm_installed_services` in `iiab/iiab` `roles/proot_services/defaults/main.yml`. As of today: