diff --git a/controller/app/src/main/java/org/iiab/controller/MainActivity.java b/controller/app/src/main/java/org/iiab/controller/MainActivity.java index 53ee682f..a521e007 100644 --- a/controller/app/src/main/java/org/iiab/controller/MainActivity.java +++ b/controller/app/src/main/java/org/iiab/controller/MainActivity.java @@ -264,16 +264,20 @@ public void onReceive(Context context, Intent intent) { public void onCreate(Bundle savedInstanceState) { super.onCreate(savedInstanceState); - // Intercept launch and redirect to Setup Wizard if first time - SharedPreferences internalPrefs = getSharedPreferences(getString(R.string.pref_file_internal), Context.MODE_PRIVATE); - if (!internalPrefs.getBoolean(getString(R.string.pref_key_setup_complete), false)) { + // Intercept launch and redirect to the legacy Setup Wizard when there is nothing to run. + // ADFA-5137: asks the device rather than setup_complete, and says which mode it wants. The + // old catch wrote the flag true so this branch would stop firing; with the flag gone there is + // nothing to write, and nothing needs writing — the condition is re-derived every launch, so + // a missing Activity stops being permanent state and goes back to being a log line. + if (!terminalOnlyLaunch(getIntent()) + && !org.iiab.controller.system.data.SystemFactsReader.hereOrOnTheWay(this)) { try { - startActivity(new Intent(this, SetupActivity.class)); + startActivity(new Intent(this, SetupActivity.class) + .putExtra(SetupActivity.EXTRA_WIZARD_MODE, true)); finish(); return; // We stop the execution of MainActivity right here } catch (android.content.ActivityNotFoundException e) { android.util.Log.w(TAG, "SetupActivity not found. Skipping initial setup."); - internalPrefs.edit().putBoolean(getString(R.string.pref_key_setup_complete), true).apply(); } } @@ -558,6 +562,26 @@ protected void onNewIntent(Intent intent) { maybeOpenTerminalFromIntent(intent); } + /** + * ADFA-5137 (review): was this Activity launched only to show the terminal? + * + *
Asked before the first-run redirect above, because the order matters and it did not use to. + * That redirect fired on {@code setup_complete}, which was true on any device that had ever + * started an install, so a terminal launch never met it. Now it asks the disk — and with no + * system, Settings → Terminal and the terminal's own keep-alive notification would land in the + * legacy setup shell in wizard mode with Back blocked, having dropped the extras that said what + * they came for. + * + *
Read here rather than deferring to {@code maybeOpenTerminalFromIntent}, which runs much later + * in {@code onCreate}: the redirect happens first, so the question has to be answerable first. + */ + private static boolean terminalOnlyLaunch(Intent intent) { + // EXTRA_OPEN_TERMINAL alone, not paired with EXTRA_TERMINAL_ONLY: the redesign's Settings entry + // sets both, but TerminalSessionService's keep-alive notification sets only the first, and both + // came here to open a terminal. What decides the redirect is what the caller came for. + return intent != null && intent.getBooleanExtra(EXTRA_OPEN_TERMINAL, false); + } + /** Open the full terminal when launched from its keep-alive notification (ADFA-4696). */ private void maybeOpenTerminalFromIntent(Intent intent) { if (intent == null || terminalController == null) return; diff --git a/controller/app/src/main/java/org/iiab/controller/SetupActivity.java b/controller/app/src/main/java/org/iiab/controller/SetupActivity.java index f9e69a50..6e7e20d5 100644 --- a/controller/app/src/main/java/org/iiab/controller/SetupActivity.java +++ b/controller/app/src/main/java/org/iiab/controller/SetupActivity.java @@ -8,8 +8,6 @@ */ package org.iiab.controller; -import android.content.Context; -import android.content.SharedPreferences; import android.os.Bundle; import android.view.View; @@ -31,6 +29,14 @@ */ public class SetupActivity extends AppCompatActivity { + /** + * ADFA-5137: true opens this screen as the first-run wizard, false as Settings. Set by whoever + * opens it, because the mode is a property of the reason for opening it and of nothing else. + * Absent means Settings, which is the safe default: a Settings screen is navigable and a wizard + * blocks Back. + */ + public static final String EXTRA_WIZARD_MODE = "org.iiab.controller.SETUP_WIZARD_MODE"; + private boolean wizardMode; @Override @@ -39,9 +45,14 @@ protected void onCreate(Bundle savedInstanceState) { setContentView(R.layout.activity_setup); org.iiab.controller.help.TooltipWiring.wireAll(getWindow().getDecorView()); - SharedPreferences prefs = getSharedPreferences( - getString(R.string.pref_file_internal), Context.MODE_PRIVATE); - wizardMode = !prefs.getBoolean(getString(R.string.pref_key_setup_complete), false); + // ADFA-5137: the caller says which mode this is, because only the caller knows. + // + // It used to read setup_complete, and that was the one reader asking a genuinely different + // question: not "is there a system" but "am I the first-run wizard or am I Settings". Two + // callers open this screen for those two reasons — MainActivity's first-run redirect and its + // Settings button — so the answer belongs in the Intent. Migrating this one to the presence + // rule would have produced a Settings screen that believes it is a wizard. + wizardMode = getIntent() != null && getIntent().getBooleanExtra(EXTRA_WIZARD_MODE, false); View rail = findViewById(R.id.setup_rail); if (wizardMode) { diff --git a/controller/app/src/main/java/org/iiab/controller/SetupSectionFragment.java b/controller/app/src/main/java/org/iiab/controller/SetupSectionFragment.java index 4e473a10..96d5393d 100644 --- a/controller/app/src/main/java/org/iiab/controller/SetupSectionFragment.java +++ b/controller/app/src/main/java/org/iiab/controller/SetupSectionFragment.java @@ -240,9 +240,10 @@ private void finishEnrollment(SharedPreferences delivery) { private void completeSetup() { // ADFA-4466 Phase 2: setup funnel completion (no-op unless opted in). org.iiab.controller.analytics.AnalyticsClient.with(requireContext()).logOnboardingCompleted(); - SharedPreferences prefs = requireContext().getSharedPreferences( - getString(R.string.pref_file_internal), Context.MODE_PRIVATE); - prefs.edit().putBoolean(getString(R.string.pref_key_setup_complete), true).apply(); + // ADFA-5137: nothing to mark. This screen belongs to the legacy setup shell, and it wrote + // setup_complete on its way out — a fourth writer of a flag nobody cleared. LibraryActivity + // now asks the device instead, so if this path really did install a system it will be found, + // and if it did not, the wizard is where the user should land. startActivity(new Intent(requireContext(), org.iiab.controller.redesign.LibraryActivity.class)); requireActivity().finish(); } diff --git a/controller/app/src/main/java/org/iiab/controller/install/presentation/InstallService.java b/controller/app/src/main/java/org/iiab/controller/install/presentation/InstallService.java index ea00f7b5..758ac164 100644 --- a/controller/app/src/main/java/org/iiab/controller/install/presentation/InstallService.java +++ b/controller/app/src/main/java/org/iiab/controller/install/presentation/InstallService.java @@ -1509,7 +1509,7 @@ private void doCancel() { /** * ADFA-5119: erase every trace of a system the user decided not to build. * - *
Five things claimed that system existed or was about to. Leaving any one of them behind is + *
Four things claimed that system existed or was about to. Leaving any one of them behind is * a specific bug, not untidiness: * *
There used to be a fifth: {@code setup_complete} → false, which was what kept the promise that + * no path ends in the app with no system. ADFA-5137 deleted the flag, so the promise is now kept by + * not having anything to unset — the launch asks whether a rootfs, an install or a deep operation + * is there, and after this cleanup none of the three is. + * *
The install marker is a sixth, and it is already handled — {@link #teardown()} clears it on * every clean terminal. Left set, the next launch would open in damaged-system recovery over a * system nobody asked for. @@ -1564,22 +1566,22 @@ private void forgetTheAbandonedSystem() { org.iiab.controller.system.data.ContentStateInvalidator.replacementSucceeded(this, org.iiab.controller.system.domain.SystemReplacement.Cause.ABANDONED_INSTALL); - // 4 + 5. The decision itself, and last on purpose. If the process is killed part-way + // 4. The recorded tier, and last on purpose. If the process is killed part-way // through this method, everything above it is disposable wreckage and the install marker // is still set, so the next launch enters damaged-system recovery — which offers a // reinstall. Clearing setup_complete first and dying here would instead send the user to // the wizard while the marker still says an install is running. // - // commit(), not apply(): the state posted below sends the UI - // to the tier selection immediately, and a later cold launch reads setup_complete to - // decide between the library and the wizard. An asynchronous write is a race with both. + // commit(), not apply(): the state posted below sends the UI to the tier selection + // immediately, and an asynchronous write would race it. // - // We are the first writer to set setup_complete false — the other four only ever set it - // true, which is why an abandoned install used to strand the user on an empty library. + // ADFA-5119 added a setup_complete → false here and called itself the first writer of + // false among five. ADFA-5137 removed the flag altogether, so there is nothing to unset: + // the marker cleared by teardown() and the absent rootfs now say the same thing between + // them, and they cannot disagree with each other the way the flag could disagree with both. getSharedPreferences(getString(R.string.pref_file_internal), Context.MODE_PRIVATE) .edit() .remove("installed_tier") - .putBoolean(getString(R.string.pref_key_setup_complete), false) .commit(); } catch (Exception e) { // Never leave the UI waiting because a cleanup step failed: the state below is what @@ -1602,7 +1604,7 @@ private void teardown() { * no system, so forgetting that an install happened is exactly what let the * app open an empty library. Success, cancellation and every module or reset * path still pass true — a cancellation has already removed the residue and - * cleared setup_complete, so it needs no marker to be recovered from. + * removed the residue and left no rootfs, so it needs no marker to say so. */ private void teardown(boolean clearMarker) { // ADFA-5119: nothing to wait for once this is over — neither the window nor a queued attempt. diff --git a/controller/app/src/main/java/org/iiab/controller/redesign/LibraryActivity.java b/controller/app/src/main/java/org/iiab/controller/redesign/LibraryActivity.java index f0734e66..9e7b3f3d 100644 --- a/controller/app/src/main/java/org/iiab/controller/redesign/LibraryActivity.java +++ b/controller/app/src/main/java/org/iiab/controller/redesign/LibraryActivity.java @@ -3,7 +3,6 @@ import android.animation.Animator; import android.animation.AnimatorListenerAdapter; import android.content.Intent; -import android.content.SharedPreferences; import android.content.res.Configuration; import android.os.Bundle; import android.os.Handler; @@ -40,6 +39,23 @@ public class LibraryActivity extends AppCompatActivity implements ServerControll public static final String EXTRA_INSTALLING = "installing"; /** ADFA-4777: preselect a bottom-nav tab on launch (e.g. from the wizard's "Copy from a phone"). */ public static final String EXTRA_TAB = "tab"; + /** + * ADFA-5137: the caller knows there is no system and is bringing the user here to get one. + * + *
Only the wizard's "Copy from a phone" sets it. That choice has to land on the Clone tab with + * nothing installed and nothing yet in flight, which is precisely the state that otherwise sends + * the user back to the wizard — so before this ticket the wizard wrote {@code setup_complete} to + * get past the check, and that lie is the entrance to findings 3 and 5. + * + *
An Intent extra rather than a stored fact — but its lifetime is the task record, not + * this navigation, and the difference is worth stating because a first draft of this comment got it + * wrong in both directions. Android replays the launching Intent when the process is killed and the + * task is restored, so the extra survives that; swiping the task away is what ends it. And + * {@code onNewIntent} calls {@code setIntent}, so a later arrival carrying no {@code settingUp} + * replaces it. Both outcomes are truthful — the user lands on Home, which since ADFA-5137 has a + * labelled way to install a system — but nobody should read this as "it dies when you navigate". + */ + public static final String EXTRA_SETTING_UP = "settingUp"; private boolean installing = false; /** ADFA-4799: bottom bar (compact) and rail (medium/expanded) share the NavigationBarView @@ -97,10 +113,18 @@ public class LibraryActivity extends AppCompatActivity implements ServerControll protected void onCreate(Bundle savedInstanceState) { super.onCreate(savedInstanceState); - // Not set up yet? Run the first-run wizard, then it routes back here. - SharedPreferences prefs0 = getSharedPreferences( - getString(R.string.pref_file_internal), MODE_PRIVATE); - if (!prefs0.getBoolean(getString(R.string.pref_key_setup_complete), false)) { + // ADFA-5137: nothing here and nothing coming? Run the first-run wizard, then it routes back. + // + // This used to read setup_complete, a flag written when an install STARTED and cleared by + // nobody — so it could say "set up" while the device had no system, and then this branch + // routed past the wizard forever. That pair is findings 3 and 5 of state-spine.svg. The + // question was never "did setup happen": it is "is there a system, or one on the way", and + // that is answerable from the disk and the two markers, none of which can drift from what + // they describe. + boolean broughtHereToSetUp = getIntent() != null + && getIntent().getBooleanExtra(EXTRA_SETTING_UP, false); + if (!broughtHereToSetUp + && !org.iiab.controller.system.data.SystemFactsReader.hereOrOnTheWay(this)) { startActivity(new Intent(this, WizardActivity.class)); finish(); return; diff --git a/controller/app/src/main/java/org/iiab/controller/redesign/LibraryHomeFragment.java b/controller/app/src/main/java/org/iiab/controller/redesign/LibraryHomeFragment.java index a8e54333..99159a4d 100644 --- a/controller/app/src/main/java/org/iiab/controller/redesign/LibraryHomeFragment.java +++ b/controller/app/src/main/java/org/iiab/controller/redesign/LibraryHomeFragment.java @@ -105,6 +105,13 @@ public View onCreateView(@NonNull LayoutInflater inflater, @Nullable ViewGroup c requireContext(), SetupProgressActivity.class)); return; } + // ADFA-5137: with no system, the way forward is choosing one — the tier step, the same + // place the wizard sends people. Named, on the header, instead of a sentence pointing + // at a control at the bottom of the screen called "Get more". + if (headerState == H_NO_LIBRARY) { + openGetMore(); + return; + } // ADFA-4837: retry only when it is genuinely safe — canStartServer guards // against stacking a second proot over a live one. if (headerState != H_FAILED) return; @@ -242,6 +249,26 @@ private View makeGetMoreCell(int cardH) { } private void openGetMore() { + // ADFA-5137 (review): refuse while a deep operation owns the environment, and refuse HERE so + // both entrances are covered — the footer control and the header button this ticket added. + // + // The hole is not theoretical and the header made it one tap wide. isSystemInstalled() is false + // for the whole time an install marker is set, and a clone-receive holds both the marker and + // the lock — so during a live receive the header reads "no library" and this method would take + // the Step-1 branch. That branch starts an install with reinstall=false, InstallService's + // non-destructive guard sees the half-received rootfs directory, skips the extract and reports + // success, and its teardown clears the marker that a killed receive needs for recovery. That is + // exactly the "boot the wreck" failure InstallService's own cleanup comment warns about, + // reached from a button labelled as a way out. + // + // ownerHeld, not isHeld: a live content download holds no owner marker and must not block this + // (ADFA-4957 draws the same line for the server toggle). + if (org.iiab.controller.env.EnvironmentLock.ownerHeld(requireContext())) { + if (getView() != null) { + org.iiab.controller.util.Snackbars.make(getView(), R.string.k2go_install_busy).show(); + } + return; + } // If a system is already installed, skip the destructive system step and go straight // to content (Step 2). Otherwise run the full setup from Step 1. Intent i = new Intent(requireContext(), SetupLibraryActivity.class); @@ -572,8 +599,19 @@ private void setHeader(int h) { // app working and gets a spinner; the rest are statements and get nothing. The status // colour lives on the dot only — the button wears the brand colour, because it is a // control rather than a severity. + // ADFA-5137: H_NO_LIBRARY gets one too. It was the only state here that offered nothing, and + // that is finding 5 of state-spine.svg: the header said "tap Get more to install" while being + // plain text, pointing at a control at the far bottom of the screen whose name says content + // rather than system. Meanwhile both cards on the way there offer Install and Schedule, and + // both refuse. + // + // ADFA-5137 also closes the way INTO this state, so in principle nobody arrives here any more. + // The button stays anyway, because "in principle" is what the last four dead ends had in + // common: a state with no exit is a bug whoever reaches it, including by a route that does not + // exist yet. One line in a switch that already hands out two other buttons. int action = h == H_FAILED ? R.string.k2go_home_retry - : h == H_INSTALLING ? R.string.k2go_home_see_progress : 0; + : h == H_INSTALLING ? R.string.k2go_home_see_progress + : h == H_NO_LIBRARY ? R.string.k2go_home_install_system : 0; if (homeStatusAction != null) { homeStatusAction.setVisibility(action != 0 ? View.VISIBLE : View.GONE); if (action != 0) homeStatusAction.setText(action); diff --git a/controller/app/src/main/java/org/iiab/controller/redesign/ModuleHubFragment.java b/controller/app/src/main/java/org/iiab/controller/redesign/ModuleHubFragment.java index 64ce9e50..983076e0 100644 --- a/controller/app/src/main/java/org/iiab/controller/redesign/ModuleHubFragment.java +++ b/controller/app/src/main/java/org/iiab/controller/redesign/ModuleHubFragment.java @@ -65,9 +65,12 @@ public class ModuleHubFragment extends Fragment { * with. The screen still listed all six as installable, because it decided that from a probe * — and with no system nothing answers, so everything looked missing. * - *
Reachable, not theoretical: {@code pref_key_setup_complete} is only ever written true — - * five writers, none of them clears it — so after a reset or a failed restore the app still - * routes to the Library, and Settings still opens this screen over an empty rootfs. + *
Reachable, not theoretical — and it stays reachable after ADFA-5137. That ticket removed the + * flag this note used to blame ({@code setup_complete}, written true by four sites and cleared by + * none), so a device that simply has no system now opens the wizard instead of the Library. What + * still lands here is the case where a marker is held: a failed restore keeps it, so the launch + * treats the device as having something on the way, reaches the Library, and this screen can be + * opened over a rootfs that is empty or half-written. The precondition still has to be asked here. * *
Seeded true so the first frame looks like the ordinary case; the background pass * corrects it before anything is offered. diff --git a/controller/app/src/main/java/org/iiab/controller/redesign/SetupLibraryActivity.java b/controller/app/src/main/java/org/iiab/controller/redesign/SetupLibraryActivity.java index 5c7c1caf..88ed5155 100644 --- a/controller/app/src/main/java/org/iiab/controller/redesign/SetupLibraryActivity.java +++ b/controller/app/src/main/java/org/iiab/controller/redesign/SetupLibraryActivity.java @@ -277,11 +277,25 @@ public void startWizardInstall() { // Fire exactly once. (The install service also dedupes the actual install via its `started` guard.) if (installStarting) return; installStarting = true; - // ADFA-4982: the real install is starting — mark setup complete NOW (it is no longer set at the - // wizard's "download" choice, so bailing before this resumes the wizard). This also lets the - // install LibraryActivity below show progress instead of redirecting back to the wizard. - getSharedPreferences(getString(R.string.pref_file_internal), MODE_PRIVATE) - .edit().putBoolean(getString(R.string.pref_key_setup_complete), true).apply(); + // ADFA-5137: nothing is written here any more. ADFA-4982 set setup_complete at this line so the + // LibraryActivity below would show progress instead of bouncing back to the wizard — which + // worked, and was also the bug: it claimed setup was done at the moment an install BEGAN, and + // then nobody unclaimed it if the install never finished. LibraryActivity now asks whether a + // system is here or on the way, and the install marker this service is about to plant answers + // yes for exactly as long as the install runs. + // + // Which is why the marker is planted HERE, before the service is asked to start. Both the + // service start and the Activity start below are asynchronous dispatches with no ordering + // between them, so LibraryActivity.onCreate can run before InstallService.onStartCommand — and + // it would then find no rootfs, no marker and no lock, and bounce the user back to the wizard + // on the one path that matters most. The marker is a file, so writing it here makes the fact + // true at the moment the user commits rather than whenever the scheduler gets to the service. + // InstallGuard.begin is idempotent, so the service planting it again costs nothing. + // + // The trade-off, stated: if the service never starts at all, the marker is left set with no + // install behind it, and the next launch enters recovery. That is a state with a dialog and a + // way out (ADFA-5119) rather than a silent dead end, which is the right side to fail on. + org.iiab.controller.InstallGuard.begin(this); Intent i = new Intent(this, InstallService.class); i.setAction(InstallService.ACTION_START); i.putExtra(InstallService.EXTRA_TIER, getSelectedTier().name()); diff --git a/controller/app/src/main/java/org/iiab/controller/redesign/WizardActivity.java b/controller/app/src/main/java/org/iiab/controller/redesign/WizardActivity.java index 128cc29b..2a1c408f 100644 --- a/controller/app/src/main/java/org/iiab/controller/redesign/WizardActivity.java +++ b/controller/app/src/main/java/org/iiab/controller/redesign/WizardActivity.java @@ -61,10 +61,14 @@ protected void onCreate(Bundle b) { // applied language, so we don't flash back to the welcome step. langTag = AppLocaleController.currentTag(); if (b != null) step = b.getInt("step", 0); - // ADFA-4982: a fresh launch that isn't complete yet but already has permissions means the user - // passed language + permissions and only bailed from the setup choice / edition selection — - // resume at the setup choice (step 3), not welcome/language/permissions all over again. - else if (!prefs().getBoolean(getString(R.string.pref_key_setup_complete), false) && allPermsGranted()) step = 3; + // ADFA-4982: a fresh launch that already has permissions means the user passed language + + // permissions and only bailed from the setup choice / edition selection — resume at the setup + // choice (step 3), not welcome/language/permissions all over again. + // + // ADFA-5137 dropped the setup_complete half of this condition, which was always true here: + // LibraryActivity is the only thing that opens this screen, and it only opens it when there + // is no system and none on the way. The permissions check was doing all the work. + else if (allPermsGranted()) step = 3; title = findViewById(R.id.wiz_title); subtitle = findViewById(R.id.wiz_subtitle); primary = findViewById(R.id.wiz_primary); @@ -99,10 +103,14 @@ protected void onCreate(Bundle b) { finish(); }); findViewById(R.id.setup_copy).setOnClickListener(v -> { - markComplete(); // ADFA-4777: "Copy from a phone" lands directly on the (now functional) Clone tab. + // ADFA-5137: it used to write setup_complete to get past LibraryActivity's check, because + // this is the one navigation that must land with no system and nothing yet in flight. It + // now says that, instead of claiming setup is finished. The difference is that the extra + // dies with this Intent: leave before scanning and the next launch is back here, correctly. startActivity(new Intent(this, LibraryActivity.class) - .putExtra(LibraryActivity.EXTRA_TAB, R.id.nav_clone)); + .putExtra(LibraryActivity.EXTRA_TAB, R.id.nav_clone) + .putExtra(LibraryActivity.EXTRA_SETTING_UP, true)); finish(); }); primary.setOnClickListener(v -> onPrimary()); @@ -304,7 +312,8 @@ private void requestBattery() { private SharedPreferences prefs() { return getSharedPreferences(getString(R.string.pref_file_internal), MODE_PRIVATE); } - private void markComplete() { - prefs().edit().putBoolean(getString(R.string.pref_key_setup_complete), true).apply(); - } + // ADFA-5137: markComplete() is gone. It wrote setup_complete on the "Copy from a phone" choice — + // before a transfer existed, let alone succeeded — which is the entrance to finding 3 that survived + // ADFA-4982 and ADFA-5116. Nothing replaces it: the clone takes the environment lock while it runs, + // and that is what SystemPresence reads. } diff --git a/controller/app/src/main/java/org/iiab/controller/system/data/SystemFactsReader.java b/controller/app/src/main/java/org/iiab/controller/system/data/SystemFactsReader.java index 9ab78081..d4d50e6f 100644 --- a/controller/app/src/main/java/org/iiab/controller/system/data/SystemFactsReader.java +++ b/controller/app/src/main/java/org/iiab/controller/system/data/SystemFactsReader.java @@ -19,6 +19,7 @@ import org.iiab.controller.install.presentation.InstallProgressRepository; import org.iiab.controller.install.presentation.ModuleQueueRepository; import org.iiab.controller.system.domain.SystemFacts; +import org.iiab.controller.system.domain.SystemPresence; import java.io.File; @@ -87,6 +88,46 @@ public static SystemFacts read(Context ctx) { : SystemFacts.serverUnknown(installed, healthy); } + /** + * ADFA-5137: does this device have a system, or is one on its way? + * + *
The question the launch path asks before deciding between the library and the first-run + * wizard. It replaces {@code setup_complete}, a stored claim about the past that four sites wrote, + * none cleared, and all of them wrote when an install started — so it could answer yes on + * a device with nothing, and the launch then routed past the wizard forever. Findings 3 and 5 of + * {@code state-spine.svg} were both that. + * + *
Why it lives here and not next door. A first pass put it in a class of its own, which + * made this file's own warning come true: the ADFA-5061 survey found nine answers to "is a system + * installed" and said a tenth would be the joke telling itself. There is one reader for what is + * true about the box, and this is it. + * + *
Why it is a separate method rather than a field on {@link SystemFacts}. Because it is + * not derivable from what {@link #read} returns, and that is worth stating rather than discovering + * later. {@code isInstalled()} is {@code rootfs && !marker}, so an install in flight and a device + * with nothing at all produce the identical tuple — {@code installed=false, healthy=true} — and + * telling them apart is the whole point of this question. It needs the three raw facts, which + * {@code read} deliberately does not carry. + * + *
The rule itself is {@link SystemPresence}, pure and with its eight-row truth table in tests. + * This method is the three reads, and each one is taken from the thing it describes: the rootfs + * from the disk, the install from its durable marker, a clone or restore from the lock's owner + * marker. None of the three can drift from what it is about, which is the property the flag lacked. + * + * @return false only when the device has nothing and nothing is coming — the one case where the + * wizard is the right place to be. A null context answers true: the wizard is a decision, + * not something to fall into on a missing argument. + */ + public static boolean hereOrOnTheWay(Context ctx) { + if (ctx == null) { + return true; + } + return SystemPresence.hereOrOnTheWay( + SystemStateEvaluator.rootfsPresent(ctx), + InstallGuard.inProgress(ctx), + EnvironmentLock.ownerHeld(ctx)); + } + /** * ADFA-5061: whether the box is observed to be answering — one fact, no disk. * diff --git a/controller/app/src/main/java/org/iiab/controller/system/domain/SystemPresence.java b/controller/app/src/main/java/org/iiab/controller/system/domain/SystemPresence.java new file mode 100644 index 00000000..95d1f81c --- /dev/null +++ b/controller/app/src/main/java/org/iiab/controller/system/domain/SystemPresence.java @@ -0,0 +1,54 @@ +/* + * ============================================================================ + * Name : SystemPresence.java + * Author : AppDevForAll + * Copyright : Copyright (c) 2026 AppDevForAll + * Description : ADFA-5137. The rule that replaces setup_complete: does this + * device have a system, or is one on its way? Pure JVM. + * ============================================================================ + */ +package org.iiab.controller.system.domain; + +/** + * Whether this device has a system, or one is being put there right now. + * + *
What this replaces, and why. {@code setup_complete} was a claim about the past — "setup + * happened" — used to answer a question about the present: "should this person see the wizard?". + * Four sites wrote it, none cleared it, and it was written on intent rather than on a finished + * install, so it could say yes while the device had nothing. That combination is findings 3 and 5 of + * {@code state-spine.svg}: an app that routes past the wizard forever, onto a screen whose only way + * forward has no button. + * + *
The fix is not to give that flag a lifecycle but to stop storing it. The answer is already on + * the device, in three facts that each die with the thing they describe — the rootfs with the rootfs, + * the install marker with the install, the lock with the operation. A fact that can be derived should + * not be kept, because a kept copy has to be maintained in agreement with the original forever, and + * that agreement is exactly what broke. + * + *
Why a disjunction and not just "is there a rootfs". Two of the three are about work in + * flight rather than about a system that exists. Without them, a user who is halfway through a first + * install — or halfway through receiving a clone, which has no rootfs yet by definition — would be + * sent back to the wizard on every relaunch, which is the same dead end in the other direction. The + * question is not "is it finished" but "is anything there or coming". + * + *
Pure: booleans in, boolean out, no Android. {@code SystemFactsReader.hereOrOnTheWay} gathers the
+ * three facts — the one reader for what is true about the box, so this question does not stand up a
+ * second one beside it.
+ */
+public final class SystemPresence {
+
+ private SystemPresence() {
+ }
+
+ /**
+ * @param rootfsOnDisk a rootfs exists — asked of the disk, not of a flag
+ * @param installInProgress the durable install marker is set
+ * @param deepOpInFlight a clone, backup or restore owns the environment lock
+ * @return false only when the device has nothing and nothing is on its way, which is the one
+ * case where the wizard is the right place to be
+ */
+ public static boolean hereOrOnTheWay(boolean rootfsOnDisk, boolean installInProgress,
+ boolean deepOpInFlight) {
+ return rootfsOnDisk || installInProgress || deepOpInFlight;
+ }
+}
diff --git a/controller/app/src/main/res/values-ar/strings.xml b/controller/app/src/main/res/values-ar/strings.xml
index 3b97ee2b..32cb2184 100644
--- a/controller/app/src/main/res/values-ar/strings.xml
+++ b/controller/app/src/main/res/values-ar/strings.xml
@@ -133,7 +133,6 @@
Written as eight explicit rows rather than as three assertions about "or", because the point of
+ * the change is that every reachable combination now describes something real. The table is the
+ * verification: if a row ever has to be argued about, the model is wrong again.
+ */
+public class SystemPresenceTest {
+
+ private static boolean q(boolean rootfs, boolean installing, boolean deepOp) {
+ return SystemPresence.hereOrOnTheWay(rootfs, installing, deepOp);
+ }
+
+ /** The one row that sends a user to the wizard — nothing there, nothing coming. */
+ @Test
+ public void nothingHereAndNothingComing() {
+ assertFalse(q(false, false, false));
+ }
+
+ /** A finished system. The ordinary case, and the reason the flag existed at all. */
+ @Test
+ public void aRootfsOnDiskIsEnoughOnItsOwn() {
+ assertTrue(q(true, false, false));
+ }
+
+ /**
+ * Halfway through a first install: no usable rootfs yet, and sending this user back to the wizard
+ * on every relaunch is the same dead end from the other side.
+ */
+ @Test
+ public void anInstallInFlightCounts() {
+ assertTrue(q(false, true, false));
+ }
+
+ /**
+ * Receiving a clone, or restoring a backup. By definition there is no rootfs yet on a fresh
+ * clone-receive, which is exactly why "is there a rootfs" alone would have been wrong.
+ */
+ @Test
+ public void aDeepOperationInFlightCounts() {
+ assertTrue(q(false, false, true));
+ }
+
+ /** The four remaining rows, all overlaps of the three above. */
+ @Test
+ public void theOverlapsAreAllTrue() {
+ assertTrue(q(true, true, false)); // reinstall over an existing system
+ assertTrue(q(true, false, true)); // backup or restore on a live system
+ assertTrue(q(false, true, true)); // an install and a deep op both marked
+ assertTrue(q(true, true, true));
+ }
+
+ /**
+ * The property that makes the dead end unrepresentable: false has exactly one row. Under the old
+ * flag, "no system" and "the wizard is done" were independent, so they could disagree; here they
+ * are the same statement, and there is nowhere for a disagreement to live.
+ */
+ @Test
+ public void exactlyOneOfTheEightRowsIsFalse() {
+ int falses = 0;
+ for (int i = 0; i < 8; i++) {
+ if (!q((i & 1) != 0, (i & 2) != 0, (i & 4) != 0)) falses++;
+ }
+ assertEquals(1, falses);
+ }
+}
diff --git a/controller/docs/operation-model-roadmap.svg b/controller/docs/operation-model-roadmap.svg
index c092d4c4..1cc3bc8c 100644
--- a/controller/docs/operation-model-roadmap.svg
+++ b/controller/docs/operation-model-roadmap.svg
@@ -24,7 +24,7 @@