ADFA-5137 refactor(state): setup_complete stops existing — the app as… - #403
Merged
luisguzman-adfa merged 3 commits intoAug 14, 2026
Merged
Conversation
…ks whether a system is here Closes findings 3 and 5 of state-spine.svg and the first row of the standing directive's ledger, the one that read: setup_complete | SharedPreferences | 4 sites | nobody | never | no lifecycle. THE FAULT. setup_complete was a claim about the past — "setup happened" — answering a question about the present: "should this person see the wizard?". It was written when an install STARTED and cleared by nobody, so it could say yes while the device had nothing, and the launch then routed past the wizard forever. Finding 3. From there the only way forward was a sentence pointing at a control named "Get more". Finding 5. One bug read twice. WHAT REPLACES IT — nothing stored. The answer was already on the device, in three facts that each die with the thing they describe: a rootfs on disk, the durable install marker, the environment lock's owner. SystemPresence.hereOrOnTheWay() is that disjunction, pure and with its whole eight-row truth table in tests; SystemPresenceReader gathers the three reads, and is the only place that does — four screens each assembling them would be the same duplication arriving by a different door. Deleting the flag rather than giving it a lifecycle is the point. A stored copy has to be kept in agreement with the original forever, and that agreement is what broke. Four boolean facts allow sixteen combinations; three allow eight, and every one of the eight describes something physically true. The dead end is not fixed, it becomes unrepresentable. FOLLOWING THE VARIABLE, NOT THE SCREENS. Nine sites touched the key. Each reader was asked what it was really asking, and only one of the four was asking the question the flag pretended to answer: * LibraryActivity — "is there a system, or one on the way": migrated. * WizardActivity — "where do I resume": the flag half of that condition was always true, because LibraryActivity is the only thing that opens the wizard and only when there is nothing. allPermsGranted() was doing all the work. * SetupActivity — "am I the first-run wizard or am I Settings". A different question entirely, and the reason this step matters: migrating it to the presence rule would have produced a Settings screen that believes it is a wizard. It takes its mode from the Intent now, from the two callers that know. * MainActivity — still reachable, because it runs whenever Settings opens the Terminal. Migrated, and its catch no longer writes the flag to stop itself firing: the condition is re-derived every launch, so a missing Activity is a log line again instead of permanent state. With no readers, all five writers went, and the key with them — including the setup_complete → false that ADFA-5119 added a few hours ago. TWO THINGS THE ANALYSIS DID NOT PREDICT. A race on the happy path. startWizardInstall starts the service and the Activity one after the other with no ordering between them, so LibraryActivity.onCreate can run before InstallService.onStartCommand — and with the flag gone it would find no rootfs, no marker and no lock, and bounce the user back to the wizard on the install they had just asked for. The flag was hiding that. The marker is now planted where the user commits rather than whenever the scheduler reaches the service. If the service then never starts, the marker is left set and the next launch enters recovery — a state with a dialog and a way out, which is the right side to fail on. "Copy from a phone" needed a replacement, not a deletion. It is the one navigation that must land with no system and nothing yet in flight, which is why it wrote the flag. It now says so — EXTRA_SETTING_UP, an Intent extra that dies with the navigation. Leave before scanning and the next launch is back at the wizard, correctly, because nothing was started. THE SAFETY NET. H_NO_LIBRARY gets an action, "Install a system", routing to the tier step. It was the one header state offering nothing while FAILED offers Retry and INSTALLING offers See progress. This ticket also closes the way into that state, so in principle nobody arrives there — the button stays because "in principle" is what the last four dead ends had in common. Ledger: the setup_complete row is deleted rather than turned green. 34 files lose the key; the new string is translated into 33 locales.
…ops being a way to break things Second review of the branch. Three findings acted on, and the first one is the kind only the second pass catches. I ADDED THE TENTH ANSWER. SystemPresenceReader stood four files from SystemFactsReader, in the same package, reading the same three sources — and SystemFactsReader's own javadoc says the ADFA-5061 survey found nine answers to "is a system installed" and that "adding a tenth would have been the joke telling itself". Mine claimed to be "the only place that gathers them". Both cannot be true, and the file I duplicated warns against exactly this. The two also disagreed on the null context: SystemFactsReader.read(null) is conservative, mine was permissive. hereOrOnTheWay now lives on SystemFactsReader and the rival class is gone. Why it is a method there rather than a field on SystemFacts is written down, because it was worth verifying rather than assuming: isInstalled() is rootfs && !marker, so an install in flight and a device with nothing produce the identical tuple — installed=false, healthy=true — and telling those apart is the whole question. It is not derivable from what read() returns. SystemPresence stays as the pure rule with its eight-row table. THE SAFETY NET WAS A ONE-TAP ROUTE INTO THE FAILURE IT GUARDED AGAINST. H_NO_LIBRARY is computed from isSystemInstalled(), which is false for the whole time an install marker is set — including while a clone-receive holds both the marker and the lock, which is a state this branch keeps the user in. The gate lifts for ownerHeld, so Home is reachable, the header reads "no library", and the new button offered a tier flow with reinstall=false. InstallService's non-destructive guard would then see the half-received rootfs directory, skip the extract, report success, and its teardown would clear the marker a killed receive needs for recovery. That is the "boot the wreck" failure InstallService's own cleanup comment warns about, reached from the button labelled as the way out. Fixed at the root rather than in my branch: the header now calls openGetMore(), which already re-reads the facts at click time, and the lock check went INTO openGetMore — so the footer control that had the same hole is covered by the same line. ownerHeld rather than isHeld, so a live content download does not block it (ADFA-4957 draws that line for the server toggle). ORDER MATTERS IN MainActivity, AND IT DID NOT USE TO. The first-run redirect runs long before the terminal extras are read. It used to fire on setup_complete, true on any device that had ever started an install, so a terminal launch never met it. Asking the disk instead, a device with no system sent Settings → Terminal and the terminal's keep-alive notification into the legacy setup shell in wizard mode with Back blocked, extras dropped. The redirect now asks first whether this launch came to open a terminal. EXTRA_OPEN_TERMINAL alone, because the notification does not pass EXTRA_TERMINAL_ONLY and both callers came for the same thing. COMMENTS THAT WERE WRONG, WHICH HERE COUNTS AS A DEFECT. EXTRA_SETTING_UP's javadoc said it "dies with this navigation". It does not, in either direction: its lifetime is the task record, so Android replays it when the process is killed and the task is restored, and onNewIntent's setIntent can erase it before then. Both outcomes are truthful — the user lands on Home, which now has a labelled way to install a system — but the lifecycle claim was mine to get right and I did not. ModuleHubFragment said the app "no longer routes to the Library after a failed restore". A failed restore keeps the marker, so it does route there, with the recovery dialog. Corrected to say what actually still lands on that screen. InstallService's cleanup prose still counted five items over a four-item list and credited a setup_complete clear that no longer happens. Also: three imports left unused by the previous commit. LEFT AS A DECISION, NOT CHANGED IN PLACE. The review proposes fixing the startWizardInstall race with EXTRA_INSTALLING — already on the same Intent, already read by LibraryActivity — instead of the early InstallGuard.begin. That version persists nothing and removes the "if the service never starts, the next launch enters recovery" trade-off. Against it: the early marker also suppresses a racing server auto-start, which the extra does not. Both work, the difference is design, so it is recorded rather than swapped.
…ew one named Both diagrams had fallen behind the code, and the spine had started contradicting itself: its map still listed setup_complete as live state while its findings said the flag was gone. STATE SPINE. Findings 3 and 5 turn green beside 1. Three of the seven original dead ends are closed, 4 has lost one of its three blind spots, and the ranking line says so. The map above them stopped disagreeing. S0 and S2 no longer describe a flag that does not exist, the "Copy from a phone" edge no longer claims to mark anything, and S1 is marked UNREACHABLE with the reason the box is still drawn: a LibraryActivity instance that is already alive never re-runs the launch check, because onNewIntent skips it — which is why the header keeps its Install button rather than relying on the entrance being shut. Finding 8 is new and was found on a device, by navigating, not by reading: while a clone is being shared the server is stopped on purpose, the header reads !alive and concludes "Couldn't start — tap to retry", and canStartServer() does not know about CloneSendSession, so that Retry starts the server on top of a live transfer. ADFA-5143. ROADMAP. The ledger's first row does not turn green, it disappears: setup_complete is DELETED, lives nowhere, is written by nobody and has nothing to clear. And the standing directive gains the move it was missing. It named two good patterns, both of them "the fact dies with the thing it describes"; the third is that a fact which can be derived is deleted rather than maintained. ADFA-5119's card goes to DONE with its leftover written on it — the kiwix index leg still sends exit and error onward, so a failed companion run still reports as finished. Cards added for ADFA-5137 and ADFA-5143. Two things were quietly broken and are fixed in passing: the roadmap's background rect was still 2679 high under a 2825 canvas, leaving an unpainted band, and one of finding 5's citation lines had been pushed inside finding 6 by an earlier coordinate shift.
luisguzman-adfa
deleted the
refactor/ADFA-5137-setup-complete-stops-existing
branch
August 14, 2026 11:18
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
…ks whether a system is here
Closes findings 3 and 5 of state-spine.svg and the first row of the standing directive's ledger, the one that read: setup_complete | SharedPreferences | 4 sites | nobody | never | no lifecycle.
THE FAULT.
setup_complete was a claim about the past — "setup happened" — answering a question about the present: "should this person see the wizard?". It was written when an install STARTED and cleared by nobody, so it could say yes while the device had nothing, and the launch then routed past the wizard forever. Finding
3. From there the only way forward was a sentence pointing at a control named "Get more". Finding 5. One bug read twice.
WHAT REPLACES IT — nothing stored.
The answer was already on the device, in three facts that each die with the thing they describe: a rootfs on disk, the durable install marker, the environment lock's owner. SystemPresence.hereOrOnTheWay() is that disjunction, pure and with its whole eight-row truth table in tests; SystemPresenceReader gathers the three reads, and is the only place that does — four screens each assembling them would be the same duplication arriving by a different door.
Deleting the flag rather than giving it a lifecycle is the point. A stored copy has to be kept in agreement with the original forever, and that agreement is what broke. Four boolean facts allow sixteen combinations; three allow eight, and every one of the eight describes something physically true. The dead end is not fixed, it becomes unrepresentable.
FOLLOWING THE VARIABLE, NOT THE SCREENS.
Nine sites touched the key. Each reader was asked what it was really asking, and only one of the four was asking the question the flag pretended to answer:
With no readers, all five writers went, and the key with them — including the setup_complete → false that ADFA-5119 added a few hours ago.
TWO THINGS THE ANALYSIS DID NOT PREDICT.
A race on the happy path. startWizardInstall starts the service and the Activity one after the other with no ordering between them, so LibraryActivity.onCreate can run before InstallService.onStartCommand — and with the flag gone it would find no rootfs, no marker and no lock, and bounce the user back to the wizard on the install they had just asked for. The flag was hiding that. The marker is now planted where the user commits rather than whenever the scheduler reaches the service. If the service then never starts, the marker is left set and the next launch enters recovery — a state with a dialog and a way out, which is the right side to fail on.
"Copy from a phone" needed a replacement, not a deletion. It is the one navigation that must land with no system and nothing yet in flight, which is why it wrote the flag. It now says so — EXTRA_SETTING_UP, an Intent extra that dies with the navigation. Leave before scanning and the next launch is back at the wizard, correctly, because nothing was started.
THE SAFETY NET.
H_NO_LIBRARY gets an action, "Install a system", routing to the tier step. It was the one header state offering nothing while FAILED offers Retry and INSTALLING offers See progress. This ticket also closes the way into that state, so in principle nobody arrives there — the button stays because "in principle" is what the last four dead ends had in common.
Ledger: the setup_complete row is deleted rather than turned green. 34 files lose the key; the new string is translated into 33 locales.