From fa7c5bc3e0ed006cedfd5434ab4b00ed9d903322 Mon Sep 17 00:00:00 2001 From: Luis-ADFA Date: Mon, 31 Aug 2026 16:10:24 -0600 Subject: [PATCH 1/2] =?UTF-8?q?ADFA-5343=20fix(server-lifecycle):=20=C2=A7?= =?UTF-8?q?3.3=20follow-up=20-=20route=20an=20interrupted=20install=20off?= =?UTF-8?q?=20the=20module=20index=20to=20recovery?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A killed module install can have its SetupProgressActivity task restored on top by the OS. That index resumed and polled for a server that never comes up on a broken base, and the DAMAGED loop-cut (LibraryActivity.evaluateRecovery -> userWantsOn=false) never ran, so the reconciler retried pdsm start uncut until the user manually reached Home. Root: the give-up decision is owned by a UI Activity, so any path that bypasses LibraryActivity.onCreate bypasses the cut. - SetupProgressActivity.onCreate: if InstallGuard.isInterrupted (a dead-process marker = a killed install; a live run reads isLive and clears on a clean finish), route to LibraryActivity via FLAG_ACTIVITY_CLEAR_TOP WITHOUT SINGLE_TOP so the standard- launchMode Home is re-created and its onCreate re-runs the recovery evaluation (onNewIntent is a monitor and would not). The single recovery owner then boots a healthy base and clears the marker, or declares DAMAGED and cuts. No new state, no duplicated verdict - the cut stays owned in one place; this only funnels the stray path into it. The reconciler-owned end-state (a retry budget that removes the cut-hack entirely) is recorded in ADR-5343b as a future redesign. Gates: :app:testDebugUnitTest + :app:lintDebug green. Runtime is device-only. --- .../redesign/SetupProgressActivity.java | 28 +++++++++++++++++++ ...installguard-token-and-recovery-residue.md | 22 +++++++++++++++ 2 files changed, 50 insertions(+) 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 de39ba89..89a7cb9c 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 @@ -169,6 +169,19 @@ protected void onCreate(@Nullable Bundle s) { serverController = new org.iiab.controller.ServerController(this, this); serverController.start(); + // ADFA-5343 (§3.3 follow-up): an install marker left by a DEAD process launch + // (InstallGuard.isInterrupted) is a killed install, not a resumable session. If the OS + // restores this index on top after such a kill, resuming here strands it polling for a server + // that may never come up — and the reconciler keeps retrying pdsm start uncut, because the + // DAMAGED loop-cut lives only in LibraryActivity.evaluateRecovery. Hand it to that single + // recovery owner instead: it boots a healthy base and clears the marker, or declares DAMAGED + // and cuts the retry. A live install in THIS process reads isLive (the marker is cleared on a + // clean finish), so a normal run never takes this branch. + if (org.iiab.controller.InstallGuard.isInterrupted(this)) { + routeToRecovery(); + return; + } + // ADFA-4919: observe the maps (proot) queue so its RUNNING -> DONE transition always // re-renders the index. The REST streams have service listeners; the proot stage had none, // so a proot-only install could finish without the index ever updating to Finish/redirect. @@ -1239,6 +1252,21 @@ private void onModuleBatchTerminal() { } } + /** + * ADFA-5343 (§3.3 follow-up): hand an interrupted (dead-process) install to LibraryActivity's single + * recovery path. {@code CLEAR_TOP} without {@code SINGLE_TOP} so the standard-launchMode + * LibraryActivity is finished and re-created — its {@code onCreate} re-computes {@code recovering} + * and schedules {@code evaluateRecovery} (which boots a healthy base and clears the marker, or + * declares DAMAGED and sets {@code userWantsOn=false} to cut the reconciler's retry). Reusing the + * instance via {@code onNewIntent} would not, since Home is a monitor there. + */ + private void routeToRecovery() { + startActivity(new android.content.Intent(this, LibraryActivity.class) + .addFlags(android.content.Intent.FLAG_ACTIVITY_CLEAR_TOP) + .putExtra(LibraryActivity.EXTRA_TAB, R.id.nav_library)); + finish(); + } + private void goHome(boolean clearSessions) { cancelRedirect(); if (clearSessions) { ZimDownloadService.finishSession(); BooksDownloadService.finishSession(); KolibriSeedService.finishSession(); } diff --git a/controller/docs/ADR-5343b-installguard-token-and-recovery-residue.md b/controller/docs/ADR-5343b-installguard-token-and-recovery-residue.md index d5a8ef18..c4e42ac2 100644 --- a/controller/docs/ADR-5343b-installguard-token-and-recovery-residue.md +++ b/controller/docs/ADR-5343b-installguard-token-and-recovery-residue.md @@ -198,6 +198,28 @@ not re-open it, and the window is bounded by the first foreground recovery (rebo boot), so the cut is momentary. This is the one net-new line of behavior beyond deletions, and it is subtraction-shaped (it *stops* work). +**Device-verified (2026-08-31) and closed by a follow-up.** On device the cut works on the `LibraryActivity` +path (S2), but the residual has a second, real face: after a killed **module** install, the OS can restore the +`SetupProgressActivity` task on top; that index resumes and polls for a server that never comes on a broken base, +and `evaluateRecovery` (the only owner of the cut) never runs — so the reconciler retries `pdsm start` **uncut** +until the user manually reaches Home. Root: the *give-up* decision is owned by a **UI Activity**, not by the +lifecycle owner, so any path that bypasses `LibraryActivity.onCreate` bypasses the cut. + +- **Fix (landed — small seam, one recovery owner).** `SetupProgressActivity.onCreate`: if + `InstallGuard.isInterrupted(this)` (a dead-process marker = a killed install, never a live run — the marker is + cleared on a clean finish), it does **not** resume the index — it routes to the single recovery owner via + `startActivity(LibraryActivity, FLAG_ACTIVITY_CLEAR_TOP)` **without** `SINGLE_TOP`, so the standard-launchMode + `LibraryActivity` is re-created and its `onCreate` re-computes `recovering` and schedules `evaluateRecovery` + (reusing it via `onNewIntent` would not — Home is a monitor there). `LibraryActivity` then boots a healthy base + and clears the marker, or declares DAMAGED and cuts. No new state, no duplicated verdict — the cut stays owned in + one place; this only funnels the stray path into it. (The `batchServerSlow` 45 s timeout is *not* the hook: after + a fresh-process restore the reset queue is not "terminal", so its wait anchor never arms.) +- **End-state (recorded, not pulled in) — move the give-up to the owner.** The clean fix is for the *reconciler* + itself to stop driving `desired=UP` after the box repeatedly fails to boot on an **installed** system (a bounded + retry / present-but-unbootable signal), publishing a DAMAGED phase any screen observes — which lets us delete + both the `userWantsOn=false` cut-hack and the Activity-coupling entirely. That is a larger redesign of the core + owner (a retry budget + a new phase) with its own note, deliberately out of scope for this follow-up. + --- ## 4. Decision (piece 2) — route recovery through `desired`, delete the residue From 3b3f88d915d922e0cd6dfc5a93dfbb9fc0bac324 Mon Sep 17 00:00:00 2001 From: Luis-ADFA Date: Mon, 31 Aug 2026 16:17:29 -0600 Subject: [PATCH 2/2] =?UTF-8?q?ADFA-5343=20fix(server-lifecycle):=20=C2=A7?= =?UTF-8?q?3.3=20follow-up=20-=20guard=20the=20interrupted-install=20rerou?= =?UTF-8?q?te=20against=20live=20sessions?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Code-review hardening of fa7c5bc3: SetupProgressActivity.onCreate is entered from ~14 sites, so the isInterrupted reroute must not hijack a screen that has live work. Route to recovery only when the marker is interrupted AND nothing live owns the screen: !rebuildInSession() (a dashboard rebuild does not plant InstallGuard) && !EnvironmentLock.isBusyNow() (a live content download). After a real kill the queue/downloads are idle (device-observed: holder=NONE), so the guard never blocks the case it exists for. Gates: :app:testDebugUnitTest + :app:lintDebug green. Runtime is device-only. --- .../controller/redesign/SetupProgressActivity.java | 10 +++++++++- ...DR-5343b-installguard-token-and-recovery-residue.md | 5 ++++- 2 files changed, 13 insertions(+), 2 deletions(-) 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 89a7cb9c..442cc2b7 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 @@ -177,7 +177,15 @@ protected void onCreate(@Nullable Bundle s) { // recovery owner instead: it boots a healthy base and clears the marker, or declares DAMAGED // and cuts the retry. A live install in THIS process reads isLive (the marker is cleared on a // clean finish), so a normal run never takes this branch. - if (org.iiab.controller.InstallGuard.isInterrupted(this)) { + // + // Guarded so it only fires for a genuinely stranded install with nothing live: this screen is + // also the home of the dashboard rebuild (which does NOT plant InstallGuard) and of live content + // downloads (LIVE), so a stale marker coinciding with one of those must not hijack it into + // recovery. After a real kill the queue/downloads are idle (device-observed: holder=NONE), so the + // guard never blocks the case it exists for. + if (org.iiab.controller.InstallGuard.isInterrupted(this) + && !rebuildInSession() + && !org.iiab.controller.env.EnvironmentLock.isBusyNow()) { routeToRecovery(); return; } diff --git a/controller/docs/ADR-5343b-installguard-token-and-recovery-residue.md b/controller/docs/ADR-5343b-installguard-token-and-recovery-residue.md index c4e42ac2..1a72239c 100644 --- a/controller/docs/ADR-5343b-installguard-token-and-recovery-residue.md +++ b/controller/docs/ADR-5343b-installguard-token-and-recovery-residue.md @@ -207,7 +207,10 @@ lifecycle owner, so any path that bypasses `LibraryActivity.onCreate` bypasses t - **Fix (landed — small seam, one recovery owner).** `SetupProgressActivity.onCreate`: if `InstallGuard.isInterrupted(this)` (a dead-process marker = a killed install, never a live run — the marker is - cleared on a clean finish), it does **not** resume the index — it routes to the single recovery owner via + cleared on a clean finish) **and nothing live owns the screen** (`!rebuildInSession() && !EnvironmentLock.isBusyNow()`, + so a stale marker coinciding with a dashboard rebuild or a live content download — both of which also live on this + screen and neither plants InstallGuard — cannot hijack it), it does **not** resume the index — it routes to the + single recovery owner via `startActivity(LibraryActivity, FLAG_ACTIVITY_CLEAR_TOP)` **without** `SINGLE_TOP`, so the standard-launchMode `LibraryActivity` is re-created and its `onCreate` re-computes `recovering` and schedules `evaluateRecovery` (reusing it via `onNewIntent` would not — Home is a monitor there). `LibraryActivity` then boots a healthy base