iOS 27 UIScene adoption and mobile app-state hardening - #29637
Draft
chrisnojima wants to merge 96 commits into
Draft
chrisnojima wants to merge 96 commits into
chrisnojima wants to merge 96 commits into
Conversation
Apps built with the iOS 27 SDK trap at launch in _UIApplicationEvaluateRuntimeIssueForNoSceneLifecycleAdoption unless they use scenes. SceneDelegate builds on Expo's ExpoAppSceneDelegate, which forwards lifecycle, URL and user-activity events to the AppDelegate overrides, and moves React Native's root view controller into a KeyboardWindow so hardware enter/shift-enter keep working.
…s under scenes sceneDidBecomeActive is forwarded to applicationDidBecomeActive while applicationState still reads .inactive, so notifyAppState told Go INACTIVE and the local http server stayed stopped, leaving images blank.
Use the stock @objc(SceneDelegate) ExpoAppSceneDelegate with only a post-connect hook for root view setup. Drop KeyboardWindow in favor of handling hardware enter/shift-enter on the app delegate at the end of the responder chain, and drop the manual RCTLinkingManager overrides that Expo's scene forwarder already covers.
…p single-flight flush MobileAppState now bumps a generation on every accepted update, including same-value ones, and exposes StateAndGeneration and UpdateIfGeneration so owners can undo only their own transitions. Update reports whether the value changed; waking NextUpdate, cancelling RPCs and flushing local DBs happen only on a real change. iOS starts in BACKGROUND. LevelDb.Flush compacts only the sentinel's key range, which still rotates and flushes the memtable, and is single-flight per DB. The lazy open now assigns l.db under the write lock.
…flushes Flush now rotates the memtable through an opened-and-discarded transaction, which waits for the memtable to reach a table without compacting any tables. A Flush that arrives while one is running makes it run once more instead of being dropped. The lazy open goes back to running under the read lock; a write-locked open deadlocked against an in-flight read-locked operation. Only the db assignment and the readers that skip the open are guarded now.
BackgroundSync, the background task window, its expiration and live location each keep the generation of the BACKGROUNDACTIVE transition they made and return to BACKGROUND with a generation CAS, so a newer lifecycle update (including iOS willEnterForeground's same-value BACKGROUNDACTIVE) is never overwritten. The background task now returns to BACKGROUND when it finishes, fails or times out. UpdateWithCheck returns the generation it applied at. New bind entry points: AppBackgroundTaskExpired for the iOS expiration handler, and AppPushWindowBegin/AppPushWindowEnd for Android's push window.
Concurrent window openers (Android's onPause and the push service) could record their generations out of order and strand BACKGROUNDACTIVE; the recorded generation now only rises. AppPushWindowEnd checks the token before querying deliveries, and an expired background task warns about pending messages only when its window was still open. Tests force the out-of-order recording, cover the live location claim and release through the tracker, and the stress test now ends phases in a background task window and in BACKGROUND and checks for leaked goroutines.
…le controller Native lifecycle events go through libkb/lifecycle.Controller, which owns the event to state mapping, owner generations, flushes and an injected clock. Bind entry points and live location become one-line adapters. Adds lifecycletest with a state recorder and replayable iOS/Android scenarios.
…rder on change channels
… in background Serve exiting on its own now clears the server so it can start again, and the manager restarts it on any non-BACKGROUND state. The token is created once per process, handlers are registered before the listener serves, and the manager's server, endpoints and monitor state are read under one lock. The monitor seeds from the current state, so a background launch never starts the server, and it exits on shutdown. Bootstrap status reads the address and token together.
… on Android, and shorten logged tokens A server whose Serve returns without Stop now reports it, and the manager restarts it at most once per app-state generation unless it should be down. Android runs the monitor too, restarting a dead server on every transition while never stopping it. Starts after shutdown are covered by a test, and the process-lifetime token is logged only by prefix.
…nager lock Reading the app state outside the lock let a BACKGROUND applied by the monitor land between the read and the restart, leaving the server up in the background.
… INACTIVE Startup, login and reconnect connects no longer connect while BACKGROUND; the URI is remembered so the monitor connects on leaving BACKGROUND. The monitor seeds from the current state, and it and every connect decide under one lock, so a BACKGROUND racing a connect still disconnects. Only BACKGROUND or a desktop suspend disconnects. Also read the URI and each connection's shutdown channel under the connection lock, so a ping loop exits with its own connection, and guard the non-TLS transport against dial/close races.
…tion on login Logout now resets through the connection gate and forgets the URI, so no app-state transition reconnects while logged out. A login resets any existing connection, including one left unconnected by a failed auth, so connectNow dials again instead of skipping on a non-nil conn.
An OnConnect that passed its connection check before a logout or reconnect could still install a client for the dropped connection. The install now rechecks the connection under the lock Shutdown takes. Also cover how a connection whose auth failed terminally recovers: the ping loop redials at the ping interval and FOREGROUND redials at once.
…nnection After SyncAll returns, a logout or reconnect could still let the old connection push its badges, mark the chat syncer connected, run the gregor state sync, and clear first connect for the next account. Each step now applies only while its connection is current: badge pushes hold a lock Shutdown takes, first connect is checked and written under the connection lock, and a syncer mark that loses to a Shutdown is undone unless a newer connection has marked it since. Wrap the client-install error with %w so a lost install is not retried.
…arts and races - convloader: the app-state monitor runs per Start/Stop run, seeds its suspension from the current state, and keeps it apart from the Suspend/Resume refcount; runs get their own queue, channels and group - archive: resumes decide under the registry lock and only in FOREGROUND, launch each paused job once, pause jobs that register after a pause, and use per-resume contexts - search indexer: attemptSync does not start a sync outside FOREGROUND - attachment URLs are empty, with no query suffix, while the server is stopped
…d conv loader - search indexer: a sync started before the loop saw FOREGROUND is canceled by a following BACKGROUND - archive: a resume from a stopped run does not launch jobs in the next run - convloader: a replaced run retries only into its own queue and drops the retry once stopped - BgTicker: Stop ends the tick goroutine
… avatar monitors with their source The leveldb cleaner's app-state monitor now starts when its db opens, seeded from the current state, and ends when the db closes, so it comes back after Close or Nuke. A reopened cleaner also cleans again instead of failing as shut down. A clean keeps running only across a transition into BACKGROUNDACTIVE, as before. The avatar sources' flush-on-background monitors end on StopBackgroundTasks and seed from the current state, and the populate workers read their own channel so a restart does not race them. The ephemeral keygen loop seeds from the current state.
…r pause across app-state changes The quota reclamation, disk cache cleaning and search indexing loops wait for FOREGROUND without watching shutdown, so shutting down while backgrounded leaked them or hung the indexer's Shutdown. They now return on shutdown while paused. The prefetcher paused for the app state and for a cell network in separate waits that each unpaused on exit and ignored the other reason. One wait now watches both and unpauses only when neither holds.
The server restarted on every FOREGROUND, which broke in-flight requests after an INACTIVE blip, never stopped in BACKGROUND, and registered its handler after serving, so a restart could answer 404. It now starts only outside BACKGROUND (including at a background launch), stops on BACKGROUND, starts a dead server on any other transition or once after an unexpected exit, and registers handlers before accepting connections.
Android still reports BACKGROUND when an activity pauses for a picker, share sheet or permission prompt, so stopping there broke in-flight previews and GUI file context lookups. As with the kbhttp manager, the server now stops in BACKGROUND only on iOS; Android still restarts a dead server on transitions and after an unexpected exit.
…ver each push to JS exactly once Swift reports scene/app events through the Go lifecycle entry points on one serial queue and no longer reports state from didFinishLaunching. Each background entry owns its own UIKit background task, so a late end of an older task can no longer strand Go in BACKGROUNDACTIVE. The expiration handler and willTerminate bound their wait on Go. Pushes are emitted only once JS has registered its listener; before that a tap is kept for getInitialNotification and anything else is queued, so background launches without React Native lose nothing. Cold-start taps are taken from the scene connection options too, deduplicated against didReceive. The scene delegate clears the window and privacy cover on disconnect.
…nd gate queued iOS pushes WillTerminate now forces BACKGROUND and kicks the flush before the slow pending-message notification, so native's short wait can't cut the state change. SetAppStateInactive had no callers and is removed. react-native-kb gains pushListenerRegistered as the explicit JS readiness signal (getInitialNotification remains a fallback). Queued non-tap pushes older than ten minutes are dropped when flushed, navigation-only pushes are never queued without a tap, and a queue that can't be emitted yet is kept.
…on consistent with native The shell store's mobileAppState is seeded from AppState when subscribing, and while it reads inactive JS re-asks native, since under iOS scenes RN's AppState can start at (or report) inactive during didBecomeActive and never send active. Image heal is no longer disabled for a whole first session. The http server address is ordered by when each value was observed: a bootstrap read loses to a notification that arrived after the read started, and to a newer read. It is applied from the bootstrap load itself, re-read once the service subscription is in place, and kept across logout. A localhost image retry points at the current address and token. Merged messages and reaction updates keep attachment, preview and emoji URLs that the service returns empty while its http server is down. Pushes navigate only when tapped, at startup and live (chat.extension and settings.contacts included). The startup read of the tapped notification no longer races a 10ms timer that could drop it, and JS tells native once its push listener is registered. Native platform listeners unsubscribe on re-init.
… unfurl URLs react-native-kb observes the UIScene activation notifications from launch and emits the aggregate app state (onAppStateChange, with a getAppState getter), so JS no longer depends on UIApplication.applicationState, which lags under scenes, and no longer polls. Android keeps RN's AppState. Merged messages keep unfurl image, favicon and video URLs that the service returns empty while its http server is down. The startup path no longer has a branch for silent pushes, which are never shown and so never tapped.
Go's live location tracker drives a native CLLocationManager on iOS through a watcher passed to KeybaseInit, reference-counted across trackers, and fixes come back through a LocationUpdate bind entry point. Trackers restored after a location relaunch start watching with no UI. JS on iOS only requests the permission and no longer runs the expo-location task; Android is unchanged. Also reads the tracker's last coordinate under its lock.
…ationUpdate guards
The tasks map had two readers and neither needed a map: Close waits on a WaitGroup, and WaitBackgroundTask waits on the hold, which is the question native is really asking -- does Go still need background time. It takes the lock again afterwards, so the state the hold's end derives is written before the caller gives its background time up. Hold.Released goes with it. Its one caller guarded against a hold the controller had ended, but only WillTerminate ends a live location hold, and nothing outlives that. AcquireBackgroundWork loses its Reason parameter too: every caller passed ReasonLiveLocation, and it is the only reason no controller event matches, so fixing it inside keeps the other holds' matchers honest. Live location's boolean sync call becomes two named methods.
The error had one consumer -- kbfs, which treated a failed first bind as fatal, while the service already discarded it with a comment saying the next app-state change retries. Both now get the retry, ready is a plain close-only barrier, and the run loop drops its per-iteration nil check. start and reconcile lose error returns nobody read. Srv.Active goes too: it was status.Load().Address != "", which is the exact condition Addr already reports, and its one caller ran both in sequence. Srv takes the two app-state functions instead of an interface, which deletes kbfs's ten-line adapter for methods that differ only in name.
gregorConnGate.do had one caller and expressed "take c.mu around this" in a function, a doc comment and a closure. Its comment moves onto onGateIfCurrent, where the lock order and the no-reentry rule are actually needed. Also drops chat/types.MobileAppState, which nothing references and whose signature stopped matching libkb's two refactors ago.
Deleting Hold.Released took a behavior change with it: once WillTerminate ended the live location hold, bgHold stayed non-nil and no later fix opened a new one, so a location update after a termination warning ran with the app free to be suspended. Released comes back for that one check. Release cannot stand in for it -- on a hold that is still open, asking that way would end it -- and a release-and-reacquire would flip the app through BACKGROUND, cancelling live RPCs on the way. AcquireBackgroundWork's doc said no controller event ends this hold; WillTerminate does, and now it says so.
New's retry rides on app state changes, and the app state only moves on mobile: the lifecycle controller is driven from go/bind, and the one hold a caller opens for itself is gated on IsMobileAppType. Desktop runs this server too (modeDefault enables it), so dropping the error turned a loud Fatalf into a warning and a permanently dead server, with GUI file previews quietly handing out empty URLs. So New reports its first start again and kbfs treats it as fatal, as before. What the simplification keeps: ready carries that error once, before the loop, instead of being re-checked on every turn. The service still discards it, as it did before this branch.
The closed check is what keeps a task from joining the wait group while Close waits on it, which would be a WaitGroup misuse panic rather than a test failure. Covers both entry points: UIBackground and PushWindowEnd's hand-over.
The type comment still sent readers to do, which is gone. The rationale it carries -- why this is a mutex gate and not a request loop like Srv -- stays.
Subscribing is now the read. setNotifications registers the channels and
then reads and returns the session fields, the http server address and a
version, so there is no separate read to order against the subscription:
a change from here on is announced to that connection rather than falling
in a gap between the two.
The version widens from a bare counter to {epoch, counter}. The epoch
identifies the service process, so a client that reconnects to a restarted
service sees a different epoch instead of a counter that looks stale, and
needs no reconnect bookkeeping. NotifyRouter stamps it in one announce
helper rather than at each of the three call sites.
BootstrapStatus drops its version -- it is no longer an ordered carrier of
the session -- and GetBootstrapStatus drops the two-second httpSrv poll
that gave one status two read times.
applyClientState applies the reply to setNotifications: the session, the current user and the http address as of the moment this connection subscribed. Nothing orders it against the bootstrap read any more, so the read starts with the handshake instead of behind the subscription, and the three-read retry loop, maxStaleSnapshotReads, startHandshake's readAfter and acceptSessionSnapshot all go with it. One rule is left where there were two: strictly newer wins, for a notification and for the snapshot alike. The snapshot is labelled before the state it carries, so it is never newer than its label and dropping it on a tie loses nothing -- anything it holds beyond its label is a change already on its way as its own notification. The applied versions now survive an engine reconnect: a restarted service announces a different epoch, which is always newer, so the reset and its -1 sentinel are gone. A service too old to answer setNotifications sends no version anywhere. Its bootstrap status keeps owning the session and the address, applied in arrival order, for as long as nothing versioned has landed. Also deletes gregorReachable, whose one consumer was a bootstrap re-read trigger and whose value could sit at UNKNOWN forever because startReachability is hardcoded to return it. The re-read now hangs off the OS network status coming back online, which JS already knows without asking the service. The checkReachability call stays, without its result: the service re-dials gregor inside it.
Review of the previous commit found the status's identity had been left with no ordering at all: it was applied above the version gate, and the gate that used to protect it lived in daemon.tsx before the store write, so a status read across a logout could repopulate the current user after resetAllStores had cleared it. It is now applied only when the status agrees with the session we are in, and re-applied when the session catches up, since a status identical to the stored one never notifies again. The snapshot had the mirror of the same problem: it wrote the current user above the userSwitching guard, and a logged-out snapshot carries an empty identity, so an account switch blanked the user the guard had just decided to keep. The unversioned fallback is now keyed on the setNotifications reply having carried no snapshot, rather than on nothing versioned having landed yet. A status can no longer own the session by merely arriving before a snapshot that is on its way, and a downgrade to an older service under a live client hands the fallback back instead of pinning the session until the app restarts. The state epoch is masked to 32 bits: a JS client decodes an int64 into a float64, and nanoseconds since 1970 are past the exactly representable range. Only distinctness was ever needed.
… out The reply to setNotifications was only correct because of a startup ordering nobody had written down, and that ordering does not hold on mobile: go/bind/keybase.go brings the loopback listener up and runs the startup login attempt in a goroutine, so a client can subscribe while the attempt is still running. The reply then said loggedIn=false with a valid version, which barred the settled bootstrap status from ever setting the session again. The only repair was SendLogin's notification, whose send discards its error, so one lost send stranded the client for the life of the process. ClientState.session is now a union, null until the attempt has settled. A null session is not a logged-out one: the client falls back to getBootstrapStatus, which waits. The window stops existing rather than being survived. registered leaves the record with it -- no consumer. Alongside: a failed subscribe now takes the same fallback, since a connection with no reply and no channels is in exactly the position of one talking to a service too old to answer, and the fallback flag is cleared per connection. applyClientState writes the current user before the session, so a login's subscribers can still read it. acceptVersion falls back to arrival order for a bare-number version instead of throwing on it.
The fallback flag was armed by the setNotifications reply and never disarmed. On mobile the reply normally arrives before the startup login attempt finishes, so the flag stayed set for the whole connection and every later bootstrap status reached the unversioned path -- where setLoggedIn does not consult the applied versions. A status read across a logout, which does network work after a wait of up to thirty seconds and is not invalidated by any generation bump, then resurrected the session the logout notification had just retired. That is the hazard the version plumbing exists to prevent, on the platform that motivated it. The flag now lives beside the applied versions and clears the moment a real session version is accepted, which is the service settling the session after all. An account that is genuinely logged out announces nothing, so the status stays authoritative for it. Alongside: the reply is applied outside the subscribe try, so a throw from applying a good reply is not read as a failed subscribe; the reply is stamped with the handshake generation, so a rejection delivered after a reconnect cannot re-arm the fallback on the connection that replaced it; and the ordering comment in GetBootstrapStatus is corrected -- the rpc codec ignores map keys it has no field for, so an older client decodes the notification fine and the claim it could not was wrong. The Go test that claimed to pin register-before-read did not: swapping the two statements still satisfied every assertion. Renamed to what it checks, with the gap stated in the test.
resetState spreads initialStore, which zeroed handshakeGeneration while the daemon's closure counter kept climbing. Round 2 stamped the subscription reply with that generation, so a logout inside the subscribe window made the live connection's own reply look superseded and it was dropped. Both the generation and the handshake state track the connection, not the account, so both survive the reset. Also corrects the comment round 2 added about session-before-identity ordering: it claimed the order holds because onLoggedInChanged is registered first, which is not why. setLoggedIn writes the store synchronously, so the read inside applyStatusIdentity sees it whatever the subscriber order is; the requirement is local to onBootstrapStatusChanged and nothing outside it is involved. A reader would have preserved the wrong invariant.
The service already derives MobileAppState from the UI reports native makes (go/libkb/lifecycle), but had no way to tell a client about it, so clients derived their own -- on iOS from a different OS notification stream, with nothing ordering the two against each other. Adds one notification on the existing App channel and one field to W2's subscribe snapshot, so a client that started late has nothing to order against. MobileAppState.Update announces from the one place the value changes, which is also before lifecycle's Flush hook runs. The notification carries a StateVersion because the fan-out is one goroutine per connection: two of these can arrive in either order, and applying the older one last would leave a client permanently wrong.
…wice JS derived the app's lifecycle state independently of Go: on iOS from UIScene notifications observed in a load-time constructor in Kb.mm and aggregated across scenes, on Android from RN's AppState. Nothing ordered that against the UIApplication delegate callbacks Go derives from, and the aggregation rules differ, so the two could disagree steadily rather than only transiently. Now the service is the only deriver. The mobileAppState -> appFocused translation stays exactly where it was, so mark-read gating and desktop's independent appFocused writer are untouched. Deletes watch-app-state and its test, the appStateSource ternary that existed only because there were two sources, onAppStateChange/getAppState from the TurboModule and both implementations, and the scene table, its mutex, the constructor observer and the max-across-scenes aggregation in Kb.mm -- unreachable generality in any case, since Info.plist sets UIApplicationSupportsMultipleScenes=false. On Android this also stops an Activity pause (a photo picker, a permission dialog) reading as a backgrounded app: only the process lifecycle counts, which is what AppLifecycleReporter already documents.
…e it Update released the lock before announcing, and the version is stamped at announce time, so two concurrent Updates could publish out of order and a client's accept-if-newer gate would keep the older state forever. It cannot happen today -- the only caller is lifecycle's applyLocked, under its own lock -- but Update is exported and tests call it from goroutines, so make the ordering structural rather than documented. The fan-out only reads the connection table and spawns a goroutine per send, and nothing it touches reads app state, so it cannot re-enter the lock. Also corrects the comment about iOS background time: native ends the background task as soon as AppUIBackground returns 0, which is the ordinary backgrounding, so a client acting on the notification is racing the OS rather than covered by a task Go holds open.
A tapped notification's payload is currently marshalled to the client as JSON so the client can parse it, map five fields to a keybase:// URL and pick out the account the notification belongs to. The service already owns every fact in that middle: the conversation, which account a uid names, and which accounts have a stored secret. Adds the shape for the service to answer with instead. takePushTapRoute returns the route a tap resolved to and clears it, so it is the one taker and a tap is handed out exactly once however many times a client reconnects. It is deliberately its own call rather than a field in setNotifications' reply: that reply goes to every subscriber, kbfs inside this same process among them, and a destructive read there would let the wrong one consume a tap. pushTapRouteAvailable carries nothing for the same reason -- it is a nudge to take, not a second way to be told, which is what keeps the pair from delivering one tap twice.
The service now owns where a tap opens. DeliverPushTap is the one door a tap comes through -- native calls it from its notification-tap handler and nowhere else -- so it is also the only thing anywhere that can name an account to switch to. A URL another app, a web page or a universal link opens goes through Linking and cannot reach it, and a silent or background push is HandleBackgroundNotification, which never routes. The resolved route waits in PendingPushTap until a client takes it. That is the whole of the exactly-once property, and it is the same property the native slot used to provide, moved to the one party that always exists: a tap that arrives before any client is running (on iOS a background launch never starts one at all) is still there when one connects, and because the take clears, a reconnect or a reload finds nothing left to act on again. ResolvePushTap's table is the one the client carried, case for case, and its test is that table. The URL escaping is spelled out rather than taken from net/url: the result is compared against URLs built with JavaScript's encodeURIComponent, and each of Go's escapers differs from it somewhere.
A tap was native's payload, parked in a native slot, taken by JS, parsed out of stringly-typed JSON by pushTapTarget, and mapped to a URL against a JS mirror of which account is current. All of that is gone: native hands the payload straight to the service, and JS takes the route the service resolved. Deletes pushTapTarget and its five-case table, enqueuePushTap, subscribePushTaps, takePushTap/onPushTap from the TurboModule spec and both native implementations, both native slots (kbPushTapMutex/kbPushTapPayload and pushTapSlot), KbDeliverPushTap, and PushTapData -- a hand-rolled Kotlin JSON encoder, a five-field projection and its test that existed only to feed the TypeScript parser. Android now puts the push in the tap Intent's extras and a digest of it in the data. Two notifications with different payloads must still differ in the data, since extras are not part of filterEquals and PendingIntent reuses by it -- but a digest rather than the payload, so the payload leaves `dumpsys activity`, where the projection used to be printed. PushTapActivity runs initOnce first, because a tap can be what starts the process; it is the same call MainActivity makes a moment later and runs at most once. The account switch stays in JS. The service resolves where to go and for which account; account-link-switch still performs the switch, still only for an intent carrying a targetUid, and still drops the tap when the switch fails.
… read takePushTapRoute cleared the holder as it built the reply, so a reply lost on the way out took the tap with it. That is not a narrow window: the call fires from the client's engine-connect, which on a cold launch is the youngest socket in the process, and the only trace left behind was a warning in a ring buffer that never reaches a log file. The user would open on the wrong screen and nothing anywhere would say why. The read no longer clears. peekPushTapRoute reports what is armed; ackPushTapRoute(id) retires it, and the client sends it after queuing the route, so a failure anywhere before that leaves the tap armed for the next peek. Each route carries an id, so an ack crossing a newer tap retires nothing -- the user tapped again and that tap has not been acted on. The trade is now "a repeat rather than a loss", and the repeat is bounded three ways: the ack, an id the client remembers for the life of the module (so a lost ack re-sends the ack instead of navigating again), and the intent store's own duplicate window. What is left is a second navigation if the client restarts with a route still armed, which is the right failure to have. Also moves Android's setupKBRuntime + deliverPushTap off the main thread and starts MainActivity at once. Init is a slow path here (leveldb, keychain) and PushTapActivity is Theme.NoDisplay, so it must finish before onResume; ChatBroadcastReceiver already does this setup on a thread of its own. Nothing races: a delivery landing after the client connected is picked up by the nudge, one landing before it by the connect-time peek.
The id that suppresses a re-queue after a lost ack was recorded before the enqueue it guards. An enqueue that threw would then leave the route armed and marked as already queued, so the next peek would skip the queue and ack anyway -- retiring a tap that never reached the router. That is the silent loss the peek/ack split exists to remove, reintroduced by the guard meant to finish it. Both statements are synchronous and both still run before the ack's await, so two peeks in flight are ordered exactly as they were; only the order of the two lines changes. Also: PushTapActivity logs a payload it could not read instead of silently substituting an empty one; Peek hands back a copy rather than the holder's own route; the client-side function is drainPushTapRoute, since peek-queue-ack is not a take; and the invariant that ids only mean anything within one service process now sits next to the guard that relies on it.
Both implementations carried a "No current caller" comment and nothing in JS ever imported it. The bridge's own desync path still resets unconditionally; only the unused module entry point goes.
The push service kept a buildStyle nobody called (KBPushNotifier has the live one), two empty notification-display branches plus an unreachable trailing one, an empty else on the follow branch, and two seenChatNotifications.add calls for a key line 90 already added. AppLifecycleReporter's started field was written and never meaningfully read: started implies reported, so reportHeadlessStart's !reported already covers it. AppDelegate builds the tap payload with uniquingKeysWith instead of uniqueKeysWithValues, which traps on a duplicate key.
tryHandleIntentWithRetry reposted handleIntent every 500ms for up to 10s and then gave up silently, but every path it was waiting on ends in JS calling shareListenersRegistered, which already re-ran the flush itself. The retry loop was therefore pure duplication of its own success condition; a share now parks in the activity until JS says it is ready to route one, which is the same park-and-drain shape the push tap uses. captureIntent no longer caches non-share intents, so a plain launch parks nothing. The file copy takes the activity's own Context, so the flush no longer waits on a live ReactContext either. Also drops the always-null permission listener and the dead isTestDevice (KbModule has the live copy).
userLoadMySettings' reply was applied unconditionally, so an emailsChanged or phoneNumbersChanged notification arriving while the RPC was in flight was overwritten by the older list the RPC had already read. Apply each half only if the store still holds the value the RPC was read against.
Nothing carved phone-app out of normalizeHttpUrl's single-segment username rule, so our own invite install link also resolved to a profile for a user that does not exist. It now normalizes to keybase://settingsAddPhone, which the linking config opens as a modal over the settings tab, and the "only when the user has no phone number yet" condition rides along in the normalizer. That was startup.link's only consumer, so the field, its two setStartupDetails sites and the bespoke once-per-process check in load-settings go with it. The launch-URL read stays: it still decides whether the saved route may be restored.
…tion sweep The mobileAppState switch only ever asked whether the state is 'active', and a second subscriber guarded the same condition to reload contact permissions; one subscriber now does both. The two navigation-intent predicates had one call site each, and onEngineConnected's two bare scoping blocks declared no colliding names. The catch in the notification subscribe dropped a falsy throw on the floor. Also re-points two comments that described the push token and the share intent as native-readiness retries: the token is parked natively and read back, and the parked share intent waits on JS being able to route, not on JS existing.
router.tsx disables the linking config while logged out, and React Navigation reads getInitialURL exactly once at NavigationContainer mount -- it is not retried when loggedIn flips, and login does not remount the container (useUserSwitchNavKey deliberately ignores '' -> username). The invite install link targets someone with no account, so it always launches a logged-out app and was therefore read, discarded, and never seen by the router. startup.link used to carry it because setLoggedIn only resets the stores on true -> false. Hold the launch URL when it arrives logged out and replay it on the first login. The navigation intent's lifetime then starts after the signup rather than at launch, so a slow signup cannot expire it. Per the user's ruling, /phone-app now always opens Add Phone Number: the phones check is gone, along with the dependency on settings having loaded (phones is undefined at every cold launch, which is exactly when the link fires). handleKeybaseLink learns the same destination, since desktop routes every URL through it.
The identity guard added earlier cannot see a logout: Z.defaultReset restores the values captured at store creation, so on a cold start emails is the same initial Map and phones the same undefined, and the reply would repopulate the stores for a logged-out app -- the next account's settings screen could then render the previous account's emails and numbers. Re-read loggedIn after the await, the same check the function already makes before it. Also reverts the carry-a-launch-link-across-signup mechanism: per the user's ruling a launch link that arrives while logged out is ignored, which is what every link other than the one startup.link reader already did. The /phone-app -> settingsAddPhone chain therefore serves only an already logged-in user; it still earns its place because the normalizeHttpUrl carve-out is what stops the single-segment username rule opening a profile for a user that does not exist.
…ile error The reply to setNotifications must describe state read after the connection is subscribed: a change landing in the other order is announced to nobody and reported stale, and since the client keeps whichever version is newer, it keeps the stale value for good. That order was held by a comment, and the test said outright it could not observe it. SetChannels now returns the StateVersion labelling a read made from there on, and the handler builds ClientState from that return value, so the reply cannot be assembled without having registered first. The version is read inside setNotificationChannels, under the lock announce takes to test registration, so there is no register/read pair left to reorder anywhere. No behaviour or wire change; SetChannels had one caller.
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.
Draft: work in progress. Remaining work: JS fix round (native scene-activation app state, unfurl URL retention), native iOS live location, Android lifecycle, on-device lifecycle e2e tests, and a final whole-branch review.
Why
Apps built with the iOS 27 SDK (Xcode 27) crash at launch unless they use the UIScene life cycle. Moving to scenes changed when lifecycle callbacks fire:
UIApplication.applicationStatelags inside the forwarded callbacks. That exposed a set of app-state races between native, Go and JS. The first visible symptom was the local http server staying stopped, so images didn't load.What
iOS scene adoption
SceneDelegate: ExpoAppSceneDelegate, a scene manifest, and an AppDelegate that provides the React Native factory.AppDelegate.pressesBegan.Go: app state is driven by lifecycle events (
go/libkb/lifecycle)applicationStateis never read.MobileAppStatehas a generation. Owners (background task, BackgroundSync, live location, push window) undo only their own transitions.Go consumers
iOS native
JS
httpSrvInfonotification wins over a stale bootstrap, and bootstrap is re-read after subscribing.Testing so far
-race, with stress and goroutine-leak checks. Every fix was mutation-checked: reverting it fails its test.yarn lint:allis clean.