feat(lifecycle): derive the app state from a lifecycle controller - #29651
Draft
chrisnojima wants to merge 16 commits into
Draft
chrisnojima wants to merge 16 commits into
chrisnojima wants to merge 16 commits into
Conversation
chrisnojima
added this pull request to stack #29658
September 21, 2026 18:56
chrisnojima
changed the title
This was referenced Sep 21, 2026
Native reports only what the UI is doing -- active, inactive, off screen -- and the controller derives MobileAppState from those reports plus the holds open on background work: FOREGROUND and INACTIVE follow the UI, and a backgrounded UI is BACKGROUNDACTIVE while any hold is open and BACKGROUND otherwise. One writer, one place the state is decided, instead of each native callback picking a state for itself. Holds cover the background task that drains the outbox, the background sync window, the push window and live location, and each ends on its own terms: leaving the background ends the first two, expiration ends every background task, and termination ends them all. The package must not import libkb, since libkb holds a Controller and libkb's own tests drive it. lifecycletest replays native event sequences against a controller with a fake clock and records what an app-state consumer observes, so consumer packages can assert against the same scenarios. Additive: nothing wires the controller up yet.
The service derives the app's lifecycle state; this is how a client learns it. Deriving it a second time from the OS would mean two answers -- on iOS from two different notification streams -- with nothing ordering them against each other. Additive: nothing sends or handles it yet.
A location fix arriving while the app was backgrounded used to push MobileAppState to BACKGROUNDACTIVE itself, so the update could get out. The app state is about to become something the service derives -- from the UI reports native makes and the holds background work opens -- and a writer that reaches around that derivation would just be overwritten by the next thing the controller applies. KNOWN TEMPORARY REGRESSION, accepted deliberately: tracking does not open a hold of its own yet, so a background location fix no longer keeps the app out of BACKGROUND. A long background track can go quiet once the background task that started when the UI left the screen has ended -- roughly fifteen seconds with an empty outbox. The hold that restores this is part of the live-location rework later in this stack; nothing else in this change series puts it back, so it must not be dropped.
GlobalContext grows a lifecycle.Controller over MobileAppState, and the bind
layer reports UI state to it instead of picking an app state per callback:
SetAppStateForeground/Inactive -> AppUIActive / AppUIInactive
AppDidEnterBackground
+ AppBeginBackgroundTask -> AppUIBackground / AppWaitBackgroundTask
(new) -> AppBackgroundTaskExpired
SetAppStateBackgroundActive -> gone; the controller decides
AppUIBackground returns a token and returns at once; native waits on it with
AppWaitBackgroundTask and ends its UIKit background task when that returns.
BackgroundSync and AppWillExit now go through the controller too, so the
window and the terminate path no longer juggle the state by hand. The
delivery-polling loop that AppBeginBackgroundTask ran moves into the
controller, where it is testable against a fake clock.
MobileAppState.Update returns whether the value changed and is the only
writer; UpdateWithCheck is gone, since the check it existed for is now the
controller's single critical section. A change tells connected clients from
that one place, so no writer can move the state without announcing it. That
announce is not yet ordered per connection -- the router still sends on a
goroutine per notification -- which the ordered client-state stream later in
this stack fixes.
Both mobile platforms now start in BACKGROUND. The OS starts the process
without UI for pushes, notification actions and background refresh, and the
first UI report moves it out; Android's old BACKGROUNDACTIVE default claimed
work was running when none was.
flushLocalDbs moves to (*GlobalContext) and becomes the controller's Flush
hook, so it runs on every entry into the background rather than at the two
call sites that remembered to ask.
Also drops two things that were already dead: the appState.updateAppState
handler, which no longer appears in the protocol, and chat's MobileAppState
interface, which nothing implemented.
AppPushWindowBegin/AppPushWindowEnd are transitional. They exist only because
Android still opens the push window from Kotlin; they go away once the bind
layer wraps push handling in the window itself.
AppLifecycleForwarder hands each UIKit callback to Go as what it is -- the UI became active, went inactive, went off screen, is terminating -- and Go derives the app state. notifyAppState is gone with it: it read UIApplication.applicationState, which lags inside the scene-forwarded callbacks, so every caller had already grown a comment about why it could not be trusted there. The forwarder also owns the UIKit background task. Each background entry starts its own task and ends it once Go's background task has ended, and the expiration handler tells Go its background time is up rather than pretending the app is terminating. Background time is per app, so every task still open expires together, which is why Go ends all of its background tasks at once. The launch-time app-state report is gone: Go starts in BACKGROUND, and a foreground launch gets willEnterForeground and didBecomeActive from the scene. sceneDidDisconnect drops the window and the cover view, which didStartReactNative rebuilds if a new scene connects.
…ates onStart and onResume report an inactive and then an active UI; the background report moves from onPause to onStop, where the UI has actually left the screen, so a dialog, permission prompt or picker no longer takes the service to the background and back. onDestroy still reports the exit. WithBackgroundActive opens a push window around its task instead of setting BACKGROUNDACTIVE and then guessing its way back out. Ending it in a finally also fixes a task that threw leaving the app stuck BACKGROUNDACTIVE. The push window is opened from here, and from Kotlin, only for now: it belongs in the bind layer, which is the only place that knows iOS needs no window at all. Keybase.appPushWindowBegin/End go away with it.
mobileAppStateChanged is now the store's only source for it. JS stops reading React Native's AppState: with the service deriving the state from what native reports, a second derivation in JS would be a second answer, arriving on its own schedule, with nothing ordering the two against each other. Go's two background states collapse to one here -- nothing in the UI distinguishes "backgrounded with work still running" from "backgrounded" -- and an unmapped state says so rather than leaving the store stuck on the one before it. Desktop ignores the value: its constant FOREGROUND describes nothing, and window focus is a separate fact. The two subscribers watching mobileAppState become one, since they now run off the same source. Known gap while the rest of this stack lands: the service only sends this on a change, so a client that connects after the launch transitions keeps mobileAppState 'unknown' until the next one. appFocused defaults to true, so a foreground launch still reads correctly; what a client can miss is the first contact-permission reload and one system-theme re-read. The client-state snapshot sent on subscribing closes this.
The app now reports INACTIVE where it used to report BACKGROUNDACTIVE: Control Center, system alerts and the app switcher, and -- since Expo forwards sceneWillEnterForeground into applicationWillEnterForeground -- every cold launch and every return from the background. In all of those the UI is on screen or about to be, and React Native is painting. Stopping the server there strands image loads against a stopped server, which is exactly what the willEnterForeground report used to exist to avoid. Only BACKGROUND takes it down now.
iOS used to report BACKGROUNDACTIVE from willEnterForeground so HTTP and gregor came up before React Native resumed painting. It now reports a UI inactive event, which the lifecycle controller turns into INACTIVE, and gregor disconnected on INACTIVE: it waited for didBecomeActive to connect, and when a background hold had it connected, coming back to the foreground tore the connection down only to reconnect moments later. Connect on INACTIVE as on BACKGROUNDACTIVE and disconnect only on BACKGROUND. Connect is a no-op on a live connection.
A background task asked Stay once and then polled only the outbox, so three empty polls ended the hold even when Stay had kept it for an active coin flip or live location. Android used to stay BACKGROUNDACTIVE after the task, so a flip survived being backgrounded; now it lost the app state 15 seconds in. Poll Stay instead, and count the polls where nothing must keep running. The maximum duration and iOS expiration still bound the hold; the outbox is read only for the failure notice when time runs out.
The controller flushed on every change into BACKGROUNDACTIVE or BACKGROUND from anything but BACKGROUND, so a plain backgrounding flushed twice (into BACKGROUNDACTIVE while the background task asks whether to stay, then into BACKGROUND milliseconds later) and every hold's end flushed again. Each flush is a full CompactRange. Flush when the state leaves FOREGROUND or INACTIVE for the background, and not on hold changes after that. Keep the two flushes master had beyond the backgrounding itself: the end of a push window in the background, since handling the push wrote to the DBs, and termination, whatever the state. Master flushed neither when a background sync nor when a background task ended.
The flush rule was "the state leaves FOREGROUND/INACTIVE for the background", plus a forced flush after a push window and on termination. That missed the writes a hold makes: a background task, background sync or live location hold ends BACKGROUNDACTIVE -> BACKGROUND without a flush, so the OS suspends the app with a non-empty journal. It also flushed on every push window end in the background, even one that overlapped another window or handed over to a background task, so a burst of pushes started a compaction per push. The termination flush only starts goroutines that the dying process rarely lets finish. Flush on every change into BACKGROUND and nowhere else. BACKGROUND is where the OS suspends or kills the process, and reaching it means every hold that could have written has ended, so one flush covers them all: backgrounding with a task flushes once, when the task ends; overlapping holds flush once, when the last ends; entering BACKGROUNDACTIVE never flushes. Termination flushes only if it moves the state into BACKGROUND.
The controller's flush rule was the only caller that read it; it now compares the state before and after the write itself.
…hold's end Flushing only on a change into BACKGROUND had three problems. Leaving the UI for BACKGROUNDACTIVE (a hold open, such as live location for hours) no longer flushed, so the whole foreground session's writes sat in the journal until the hold ended, and a jetsam kill or swipe-away meanwhile paid a journal replay on the next cold start. The flush started exactly when the last hold ended, which is when iOS ends the background task and suspends the app, so the fire-and-forget flush raced suspension. And sequential holds, such as pushes a few seconds apart, each ended in BACKGROUND and each started a full compaction. Flush on every change from FOREGROUND/INACTIVE into BACKGROUNDACTIVE or BACKGROUND, which runs within the background task's lifetime and covers the foreground session. Also flush on BACKGROUNDACTIVE -> BACKGROUND, to cover what the holds wrote, unless a flush started less than FlushCoalesceInterval (default 10s) ago: a backgrounding whose task ends at once flushes once, a long task or hold flushes again at its end, and a burst of short holds coalesces.
Coalescing the end of a hold's flush against the last one dropped writes: a hold that ended less than the interval after the last flush never flushed what it wrote, a backward wall-clock jump suppressed flushes, and an exit event inside the window skipped the final flush. With LevelDb.Flush reduced to a memtable flush, flushes are cheap enough not to skip any. Flush on every change into BACKGROUNDACTIVE or BACKGROUND from anything but BACKGROUND -- the UI leaving the screen and every hold's end -- and always on WillTerminate and BackgroundTaskExpired, even when the state doesn't change. The interval, the last-flush time and the clock reads go.
A hold ending while another stayed open left the state BACKGROUNDACTIVE, so nothing flushed: under a live location hold, hours of push windows, background tasks, syncs and location posts went unflushed. dropLocked, the one place holds end, now marks the end and the next apply flushes for it whenever the resulting state is in the background, once per event even when the end also moves BACKGROUNDACTIVE to BACKGROUND. Exit events (WillTerminate, BackgroundTaskExpired) now force a flush only when the resulting state is in the background; in FOREGROUND or INACTIVE the UI's own departure will flush. applyExitLocked is folded into applyLocked, which takes the event name and an exit flag and logs the event and transition whenever it flushes.
chrisnojima
force-pushed
the
nojima/HOTPOT-as-03-appstate
branch
from
September 22, 2026 23:08
8f1784e to
6a2507d
Compare
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.
Stack 3/9, splitting #29637. Parent: #29650. This is the core of the work.
Why
On master the mobile app state is a value that native writes into Go from several places, and that JS also derives independently by reading the OS. Nothing owns it. That shape is the root cause of a family of bugs: two sources can disagree, a transition can be missed between a read and the work it guards, and "should this background work run?" is answered differently in each subsystem.
This PR makes Go the single owner: native reports lifecycle events, Go derives the app state from those events plus outstanding background-work holds, and everyone else — including JS — reads that one value.
Master problems this addresses
go/bindwith an errgroup poll loop, untestable and duplicated per entry point.UpdateWithCheckis a compare-and-set used to promote the app out of BACKGROUND. Callers race each other and there is no record of why the app is being kept up, so one owner's release can undo another's.What this changes
go/libkb/lifecycle(new) — a controller that takes UI events (UIActive/UIInactive/UIBackground) plus background-work holds and derives the state. It deliberately does not import libkb, so it is testable standalone; most of its lines are tests and a scenario harness that pins whole transition sequences.appstate.go—UpdateWithCheckis deleted in favour of holds; both platforms now start in BACKGROUND, because a process can be started by a push long before any UI exists.go/bind—SetAppState*gives way toAppUIActive/AppUIInactive/AppUIBackground, and the background-task poll loop moves into the controller where it can be tested.AppLifecycleForwarder; Android reports them fromMainActivity. These are gomobile-bound renames, so the Swift and Kotlin callers must move in the same commit or both app builds break. That is why this PR spans four languages and cannot be split further.NotifyApp.mobileAppStateChanged, additive only.Fixed during review
gregor.godisconnected on INACTIVE. It now connects on INACTIVE too and disconnects only on BACKGROUND. That restores the early connect set background active state on will enter foreground #29632 added, and a resume during background work no longer drops and re-dials gregor. Side effect: gregor now stays up during Control Center or an alert, as the local http server already does.Stay()once, then ended after three empty outbox polls (about 15s) even whenStay()was true because of a coin flip or live location. On Android master stayed BACKGROUNDACTIVE indefinitely, so this would have cut off coin flips. It now asksStay()on every poll; the 10-minute cap and iOS expiry still bound it. An outbox read error counts as nothing to deliver, as the initialStay()check already did.Decision for review: when the local DBs are flushed
The controller flushes LocalDb and LocalChatDb:
Each flush logs the event that caused it.
Alternatives tried and dropped:
The chosen rule covers every write, at the price of flushing often: a plain backgrounding flushes twice.
Cost and merge order. This assumes a flush is cheap. On this branch
LevelDb.Flushis still a fullCompactRangeof both DBs. #29661 switches it to the fork's memtable-onlyFlushMemdb, so #29661 should land before or with this PR. Master flushed once per backgrounding.Known limits. Flushes are fire-and-forget goroutines, as on master, so one started just before a suspension can be cut short.
Flushalso leaves its sentinel tombstone in the new journal; #29661 rewritesFlush.Tested by
TestFlushRuleand the scenario harness's per-step flush counts. Each branch of the rule was mutation-checked.Two deliberate, temporary states
Live location. The
UpdateWithCheckCAS is removed here but its replacement hold arrives in #29655, so background live location goes dark once the UIBackground hold releases. Knowingly accepted; there is a comment at the site. Do not merge this without #29655 following.Android push window.
whileActiveused four verbs this PR deletes, and the real replacement (the push window in Go) lands in #29654. A transitionalAppPushWindowBegin/AppPushWindowEndpair bridges the gap; both sides are commented as transitional and #29654 deletes them.Ordering — important
Android now boots in BACKGROUND. Three consumers on master hardcode
state := MobileAppState_FOREGROUNDas their loop seed (gregor.go:275,convloader.go:178,leveldb_cleaner.go:134), so once this lands they wake immediately at boot, read BACKGROUND and take the background branch — gregor disconnected, conv loader suspended. #29659, #29660 and #29661 fix exactly those three and should land before or with this PR, not after.One token in
kbhttp/manager.gois included here so the local http server is not stopped on INACTIVE, which the app now reports where it used to report BACKGROUNDACTIVE — including every cold launch and resume, since Expo forwardssceneWillEnterForegroundintoapplicationWillEnterForeground.Verification
go build,go vet,gofmt,golangci-lint --new-from-rev master0 issues; lifecycle and app-state tests pass.yarn lint:allclean, unit suite green. iOS and Android both build.Two of five
appstate_test.gotests are held back — they need the notify recorder, which arrives in #29653.Protocol artifacts here were hand-edited rather than regenerated; #29653 re-ran the real generators and confirmed the cumulative output is byte-identical, but this commit in isolation is worth a
makecheck.