From ed8f2ac9039ba5bd06d71219502e6d26aa0d8ff7 Mon Sep 17 00:00:00 2001 From: chrisnojima Date: Mon, 21 Sep 2026 12:23:20 -0400 Subject: [PATCH 01/12] feat(protocol): carry the client state on one ordered notification ClientSession and ClientState in notify_ctl, and a clientState oneway on NotifyApp that carries them. It rides the same ordered per-connection stream as the notifications that change those fields, so a client can apply everything in arrival order with no versions. The session is nullable on purpose: until the service's own startup login attempt settles there is no session to describe, and reporting a logged-out one would be a lie. --- go/protocol/keybase1/notify_app.go | 26 ++++++++++ go/protocol/keybase1/notify_ctl.go | 44 ++++++++++++++++ protocol/avdl/keybase1/notify_app.avdl | 15 ++++-- protocol/avdl/keybase1/notify_ctl.avdl | 32 ++++++++++++ protocol/avdl/keybase1/notify_session.avdl | 1 + protocol/bin/enabled-calls.json | 1 + protocol/json/keybase1/notify_app.json | 18 +++++++ protocol/json/keybase1/notify_ctl.json | 58 ++++++++++++++++++++++ protocol/json/keybase1/notify_session.json | 7 ++- 9 files changed, 198 insertions(+), 4 deletions(-) diff --git a/go/protocol/keybase1/notify_app.go b/go/protocol/keybase1/notify_app.go index e3fae3e1d110..e2fb1a2b3742 100644 --- a/go/protocol/keybase1/notify_app.go +++ b/go/protocol/keybase1/notify_app.go @@ -17,9 +17,14 @@ type MobileAppStateChangedArg struct { State MobileAppState `codec:"state" json:"state"` } +type ClientStateArg struct { + State ClientState `codec:"state" json:"state"` +} + type NotifyAppInterface interface { Exit(context.Context) error MobileAppStateChanged(context.Context, MobileAppState) error + ClientState(context.Context, ClientState) error } func NotifyAppProtocol(i NotifyAppInterface) rpc.Protocol { @@ -51,6 +56,21 @@ func NotifyAppProtocol(i NotifyAppInterface) rpc.Protocol { return }, }, + "clientState": { + MakeArg: func() any { + var ret [1]ClientStateArg + return &ret + }, + Handler: func(ctx context.Context, args any) (ret any, err error) { + typedArgs, ok := args.(*[1]ClientStateArg) + if !ok { + err = rpc.NewTypeError((*[1]ClientStateArg)(nil), args) + return + } + err = i.ClientState(ctx, typedArgs[0].State) + return + }, + }, }, } } @@ -69,3 +89,9 @@ func (c NotifyAppClient) MobileAppStateChanged(ctx context.Context, state Mobile err = c.Cli.Notify(ctx, "keybase.1.NotifyApp.mobileAppStateChanged", []any{__arg}, 0*time.Millisecond) return } + +func (c NotifyAppClient) ClientState(ctx context.Context, state ClientState) (err error) { + __arg := ClientStateArg{State: state} + err = c.Cli.Notify(ctx, "keybase.1.NotifyApp.clientState", []any{__arg}, 0*time.Millisecond) + return +} diff --git a/go/protocol/keybase1/notify_ctl.go b/go/protocol/keybase1/notify_ctl.go index 9300430778a5..414347af0234 100644 --- a/go/protocol/keybase1/notify_ctl.go +++ b/go/protocol/keybase1/notify_ctl.go @@ -88,6 +88,50 @@ func (o NotificationChannels) DeepCopy() NotificationChannels { } } +type ClientSession struct { + LoggedIn bool `codec:"loggedIn" json:"loggedIn"` + Uid UID `codec:"uid" json:"uid"` + Username string `codec:"username" json:"username"` + DeviceID DeviceID `codec:"deviceID" json:"deviceID"` + DeviceName string `codec:"deviceName" json:"deviceName"` +} + +func (o ClientSession) DeepCopy() ClientSession { + return ClientSession{ + LoggedIn: o.LoggedIn, + Uid: o.Uid.DeepCopy(), + Username: o.Username, + DeviceID: o.DeviceID.DeepCopy(), + DeviceName: o.DeviceName, + } +} + +type ClientState struct { + Session *ClientSession `codec:"session,omitempty" json:"session,omitempty"` + HttpSrvInfo *HttpSrvInfo `codec:"httpSrvInfo,omitempty" json:"httpSrvInfo,omitempty"` + AppState MobileAppState `codec:"appState" json:"appState"` +} + +func (o ClientState) DeepCopy() ClientState { + return ClientState{ + Session: (func(x *ClientSession) *ClientSession { + if x == nil { + return nil + } + tmp := x.DeepCopy() + return &tmp + })(o.Session), + HttpSrvInfo: (func(x *HttpSrvInfo) *HttpSrvInfo { + if x == nil { + return nil + } + tmp := x.DeepCopy() + return &tmp + })(o.HttpSrvInfo), + AppState: o.AppState.DeepCopy(), + } +} + type SetNotificationsArg struct { Channels NotificationChannels `codec:"channels" json:"channels"` } diff --git a/protocol/avdl/keybase1/notify_app.avdl b/protocol/avdl/keybase1/notify_app.avdl index ca9ea15b87b7..0c6ebb0c78e5 100644 --- a/protocol/avdl/keybase1/notify_app.avdl +++ b/protocol/avdl/keybase1/notify_app.avdl @@ -1,14 +1,23 @@ @namespace("keybase.1") protocol NotifyApp { + import idl "common.avdl"; import idl "appstate.avdl"; + import idl "notify_ctl.avdl"; void exit() oneway; // The app's lifecycle state changed. The service derives it from the UI - // reports native makes, so this is the only place a client learns it -- - // deriving it a second time from the OS would mean two answers with no - // ordering between them. + // reports native makes, so this and clientState's appState are the only + // places a client learns it -- deriving it a second time from the OS would + // mean two answers with no ordering between them. void mobileAppStateChanged(MobileAppState state) oneway; + // The session, the http server address and the app state, read by the + // service when this is sent. Sent first on subscribing, after every session + // change and once the startup login attempt settles. It rides the same + // ordered stream as the notifications above, so the latest one a client + // received is never older than any of them. + void clientState(ClientState state) oneway; + } diff --git a/protocol/avdl/keybase1/notify_ctl.avdl b/protocol/avdl/keybase1/notify_ctl.avdl index 77d1849a29c0..9b6bba782c9c 100644 --- a/protocol/avdl/keybase1/notify_ctl.avdl +++ b/protocol/avdl/keybase1/notify_ctl.avdl @@ -3,6 +3,8 @@ protocol notifyCtl { import idl "common.avdl"; + import idl "notify_service.avdl"; + import idl "appstate.avdl"; record NotificationChannels { boolean session; @@ -45,5 +47,35 @@ protocol notifyCtl { boolean devicehistory; } + record ClientSession { + boolean loggedIn; + UID uid; + string username; + DeviceID deviceID; + string deviceName; + } + + // ClientState is the state a client needs before it can render anything. It + // arrives as the clientState notification, on the same ordered per-connection + // stream as the notifications that change it, so a client applies everything + // in arrival order. It carries only what is available with nothing to wait + // on; the slower derived fields stay on getBootstrapStatus. + record ClientState { + // Null until the service's own startup login attempt has settled, because + // until then there is no session to describe -- not a logged-out one. A + // client keeps waiting on a null: the service sends another clientState + // once the attempt settles. + union { null, ClientSession } session; + union { null, HttpSrvInfo } httpSrvInfo; + // The app's lifecycle state, derived here from the UI reports native makes. + // A client never derives it for itself: on iOS it would have to read a + // different OS notification stream than the one this is derived from, with + // no ordering between the two. On a platform with no lifecycle to report + // this is a constant FOREGROUND and means nothing. + MobileAppState appState; + } + + // Registers the channels. A connection subscribed to app notifications then + // gets a clientState first, ahead of any change announced after it. void setNotifications(NotificationChannels channels); } diff --git a/protocol/avdl/keybase1/notify_session.avdl b/protocol/avdl/keybase1/notify_session.avdl index e9bc17cc3dfa..814dfa705db8 100644 --- a/protocol/avdl/keybase1/notify_session.avdl +++ b/protocol/avdl/keybase1/notify_session.avdl @@ -1,6 +1,7 @@ @namespace("keybase.1") protocol NotifySession { + import idl "common.avdl"; @notify("") void loggedOut(); diff --git a/protocol/bin/enabled-calls.json b/protocol/bin/enabled-calls.json index 658ceacc02e6..725adbda124a 100644 --- a/protocol/bin/enabled-calls.json +++ b/protocol/bin/enabled-calls.json @@ -152,6 +152,7 @@ "chat.1.local.updateTyping": {"promise":true}, "chat.1.local.updateUnsentText": {"promise":true}, "chat.1.local.userEmojis": {"promise":true}, + "keybase.1.NotifyApp.clientState": {"incoming":true}, "keybase.1.NotifyApp.exit": {"custom":true}, "keybase.1.NotifyApp.mobileAppStateChanged": {"incoming":true}, "keybase.1.NotifyAudit.boxAuditError": {"incoming":true}, diff --git a/protocol/json/keybase1/notify_app.json b/protocol/json/keybase1/notify_app.json index bfe65971fc58..ec56c94121ff 100644 --- a/protocol/json/keybase1/notify_app.json +++ b/protocol/json/keybase1/notify_app.json @@ -1,9 +1,17 @@ { "protocol": "NotifyApp", "imports": [ + { + "path": "common.avdl", + "type": "idl" + }, { "path": "appstate.avdl", "type": "idl" + }, + { + "path": "notify_ctl.avdl", + "type": "idl" } ], "types": [], @@ -22,6 +30,16 @@ ], "response": null, "oneway": true + }, + "clientState": { + "request": [ + { + "name": "state", + "type": "ClientState" + } + ], + "response": null, + "oneway": true } }, "namespace": "keybase.1" diff --git a/protocol/json/keybase1/notify_ctl.json b/protocol/json/keybase1/notify_ctl.json index 20bdedc83795..af0337f1c806 100644 --- a/protocol/json/keybase1/notify_ctl.json +++ b/protocol/json/keybase1/notify_ctl.json @@ -4,6 +4,14 @@ { "path": "common.avdl", "type": "idl" + }, + { + "path": "notify_service.avdl", + "type": "idl" + }, + { + "path": "appstate.avdl", + "type": "idl" } ], "types": [ @@ -152,6 +160,56 @@ "name": "devicehistory" } ] + }, + { + "type": "record", + "name": "ClientSession", + "fields": [ + { + "type": "boolean", + "name": "loggedIn" + }, + { + "type": "UID", + "name": "uid" + }, + { + "type": "string", + "name": "username" + }, + { + "type": "DeviceID", + "name": "deviceID" + }, + { + "type": "string", + "name": "deviceName" + } + ] + }, + { + "type": "record", + "name": "ClientState", + "fields": [ + { + "type": [ + null, + "ClientSession" + ], + "name": "session" + }, + { + "type": [ + null, + "HttpSrvInfo" + ], + "name": "httpSrvInfo" + }, + { + "type": "MobileAppState", + "name": "appState" + } + ] } ], "messages": { diff --git a/protocol/json/keybase1/notify_session.json b/protocol/json/keybase1/notify_session.json index afd0e01b1cfa..a571d0d2245c 100644 --- a/protocol/json/keybase1/notify_session.json +++ b/protocol/json/keybase1/notify_session.json @@ -1,6 +1,11 @@ { "protocol": "NotifySession", - "imports": [], + "imports": [ + { + "path": "common.avdl", + "type": "idl" + } + ], "types": [], "messages": { "loggedOut": { From 087a09251e1cdf3931f318a6787b3e3253a98730 Mon Sep 17 00:00:00 2001 From: chrisnojima Date: Mon, 21 Sep 2026 12:23:28 -0400 Subject: [PATCH 02/12] refactor(notify): one ordered per-connection notification sender Every notification that a client applies as state -- loggedIn, loggedOut, HTTPSrvInfoUpdate, mobileAppStateChanged and the new clientState -- went out on its own goroutine, so two changes to the same field could arrive in either order. Give each connection an unbounded FIFO drained by one goroutine instead: queueing never blocks, and the rpc library writes one goroutine's Notify calls in the order they were made. loggedIn is a call, not a notification; callInOrder gives it the same place in line without holding the queue for the reply. NotifyRouter now owns the close listener it used to hand ConnectionManager, because it has a sender to stop as well as a slot to free, so ConnectionManager.AddConnection loses its closeListener parameter. ApplyAllDetails and ApplyDetailsFn go with HandleLogout's old body, their only caller. Shutdown stops the senders instead of doing nothing. NotifyRecorder is a test connection that records what it was sent in write order, which a real rpc.Server on the far end cannot do -- it serves each notification on its own goroutine. --- go/libkb/appstate_test.go | 60 ++++++ go/libkb/connmgr.go | 40 +--- go/libkb/notify_router.go | 334 ++++++++++++++++++++++++------- go/libkb/notify_router_test.go | 334 +++++++++++++++++++++++++++++++ go/libkb/test_notify_recorder.go | 164 +++++++++++++++ go/service/notify.go | 3 + 6 files changed, 829 insertions(+), 106 deletions(-) create mode 100644 go/libkb/notify_router_test.go create mode 100644 go/libkb/test_notify_recorder.go diff --git a/go/libkb/appstate_test.go b/go/libkb/appstate_test.go index 5c33f1e1d764..018449333bc7 100644 --- a/go/libkb/appstate_test.go +++ b/go/libkb/appstate_test.go @@ -76,3 +76,63 @@ func TestMobileAppStateBackgroundCancelsRPCsOnlyOnChange(t *testing.T) { a.Update(keybase1.MobileAppState_BACKGROUND) requireOpen(t, second.Done()) } + +func appStateChanges(t *testing.T, rec *NotifyRecorder) []keybase1.MobileAppState { + t.Helper() + var ret []keybase1.MobileAppState + for _, m := range rec.Messages() { + if m.Method != "keybase.1.NotifyApp.mobileAppStateChanged" { + continue + } + var arg keybase1.MobileAppStateChangedArg + require.NoError(t, m.Decode(&arg)) + ret = append(ret, arg.State) + } + return ret +} + +// Clients are told from the one place the value changes, so no writer can add a +// path that moves the state without announcing it. +func TestMobileAppStateAnnouncesOnlyOnChange(t *testing.T) { + tc := SetupTest(t, "MobileAppStateAnnounce", 0) + defer tc.Cleanup() + tc.G.SetService() + a := NewMobileAppState(tc.G) + rec := NewNotifyRecorder(tc.G, keybase1.NotificationChannels{App: true}) + defer rec.Close() + + require.True(t, a.Update(keybase1.MobileAppState_BACKGROUND)) + require.False(t, a.Update(keybase1.MobileAppState_BACKGROUND)) + require.True(t, a.Update(keybase1.MobileAppState_FOREGROUND)) + rec.Flush() + require.Equal(t, []keybase1.MobileAppState{ + keybase1.MobileAppState_BACKGROUND, + keybase1.MobileAppState_FOREGROUND, + }, appStateChanges(t, rec), "one notification per change, none for a same-value update") +} + +// The notification is queued in the same critical section that wrote the +// state, so two concurrent Updates queue in the order they wrote rather than in +// whatever order they reached the router. Checked white-box: holding the lock +// across updateLocked is the only way to observe "has it been queued yet", and +// the answer must be yes before the lock is released. +func TestMobileAppStateQueuesUnderTheLock(t *testing.T) { + tc := SetupTest(t, "MobileAppStateQueue", 0) + defer tc.Cleanup() + tc.G.SetService() + a := NewMobileAppState(tc.G) + rec := NewNotifyRecorder(tc.G, keybase1.NotificationChannels{App: true}) + defer rec.Close() + + a.Lock() + changed := a.updateLocked(keybase1.MobileAppState_BACKGROUND) + // nothing queued reads app state (there is no clientState reader here), so + // flushing under the lock cannot deadlock + rec.Flush() + queued := appStateChanges(t, rec) + a.Unlock() + + require.True(t, changed) + require.Equal(t, []keybase1.MobileAppState{keybase1.MobileAppState_BACKGROUND}, queued, + "the change was queued before the lock that wrote it was released") +} diff --git a/go/libkb/connmgr.go b/go/libkb/connmgr.go index 01cc2854e92a..5340e61e8d9d 100644 --- a/go/libkb/connmgr.go +++ b/go/libkb/connmgr.go @@ -22,11 +22,6 @@ type ConnectionID int // true to keep going and false to stop. type ApplyFn func(i ConnectionID, xp rpc.Transporter) bool -// ApplyDetailsFn can be applied to every connection. It is called with the -// RPC transporter, and also the connectionID. It should return a bool -// true to keep going and false to stop. -type ApplyDetailsFn func(i ConnectionID, xp rpc.Transporter, details *keybase1.ClientDetails) bool - // LabelCb is a callback to be run when a client connects and labels itself. type LabelCb func(typ keybase1.ClientType) @@ -44,23 +39,14 @@ type ConnectionManager struct { labelCbs []LabelCb } -// AddConnection adds a new connection to the table of Connection object, with a -// related closeListener. We'll listen for a close on that channel, and when one occurs, -// we'll remove the connection from the pool. -func (c *ConnectionManager) AddConnection(xp rpc.Transporter, closeListener chan error) ConnectionID { +// AddConnection adds a new connection to the table of Connection objects. +// NotifyRouter.AddConnection removes it when the connection closes. +func (c *ConnectionManager) AddConnection(xp rpc.Transporter) ConnectionID { c.Lock() + defer c.Unlock() c.nxt++ // increment first, since 0 is reserved id := c.nxt c.lookup[id] = &rpcConnection{transporter: xp} - c.Unlock() - - if closeListener != nil { - go func() { - <-closeListener - c.removeConnection(id) - }() - } - return id } @@ -183,24 +169,6 @@ func (c *ConnectionManager) ApplyAll(f ApplyFn) { } } -// ApplyAllDetails applies the given function f to all connections in the table. -// If you're going to do something blocking, please do it in a GoRoutine, -// since we're holding the lock for all connections as we do this. -func (c *ConnectionManager) ApplyAllDetails(f ApplyDetailsFn) { - c.Lock() - defer c.Unlock() - for k, v := range c.lookup { - status := v.details - var details *keybase1.ClientDetails - if status != nil { - details = &status.Details - } - if !f(k, v.transporter, details) { - break - } - } -} - // NewConnectionManager makes a new ConnectionManager. func NewConnectionManager() *ConnectionManager { return &ConnectionManager{ diff --git a/go/libkb/notify_router.go b/go/libkb/notify_router.go index 8847a4f04457..019b8cb4dbd5 100644 --- a/go/libkb/notify_router.go +++ b/go/libkb/notify_router.go @@ -5,7 +5,6 @@ package libkb import ( "context" - "fmt" "sync" "time" @@ -318,9 +317,11 @@ type NotifyListenerID string type NotifyRouter struct { sync.Mutex Contextified - cm *ConnectionManager - state map[ConnectionID]keybase1.NotificationChannels - listeners map[NotifyListenerID]NotifyListener + cm *ConnectionManager + state map[ConnectionID]keybase1.NotificationChannels + senders map[ConnectionID]*connSender + listeners map[NotifyListenerID]NotifyListener + readClientState func(context.Context) keybase1.ClientState } // NewNotifyRouter makes a new notification router; we should only @@ -330,10 +331,110 @@ func NewNotifyRouter(g *GlobalContext) *NotifyRouter { Contextified: NewContextified(g), cm: g.ConnectionManager, state: make(map[ConnectionID]keybase1.NotificationChannels), + senders: make(map[ConnectionID]*connSender), listeners: make(map[NotifyListenerID]NotifyListener), } } +// connSender sends one connection's client-state stream: loggedIn, loggedOut, +// HTTPSrvInfoUpdate, mobileAppStateChanged and clientState. Every other +// notification keeps its own goroutine. +// +// One goroutine per connection drains an unbounded FIFO, so queueing never +// blocks, and the rpc library writes one goroutine's Notify calls in the order +// they are made (each hands its frame to a single writer over an unbuffered +// channel). loggedIn is a call rather than a notification, and callInOrder +// gives it the same place in line without waiting for a reply. A connection +// therefore receives its jobs in the order they were queued. +// +// A clientState job carries no state. It reads the session, the http server +// address and the app state when it is dequeued, on this goroutine and outside +// the router's lock: MobileAppState calls the router with its own lock held, +// so the router must never read app state under its lock. +// +// Why a client can apply everything in arrival order, with no versions: for +// every field, the last message that carries it to a connection subscribed to +// that field's notification carries the latest value. +// - The app state and the http address each have one writer, which queues +// its notification after the write and before it writes the next value: +// the app state under lifecycle's Controller.mu and then MobileAppState's +// lock, the address on kbhttp's run goroutine. Say the last message is a +// notification. A later write would queue its own notification behind it, +// so there was none, and it carries the latest value. Say instead it is a +// clientState. A write after that clientState read the field would have +// queued a notification behind it, so there was none either. +// - The session: every change to it -- valid or not, which user, which +// device -- queues a clientState once switchUserMu is released after the +// write (GlobalContext.lockSwitchUser), and clientState jobs read the +// session when dequeued. So for every connection the last clientState +// carries the latest session, whatever order the loggedIn/loggedOut events +// arrive in -- those carry no session state a client may apply. The only +// writes that queue nothing are the promotions a provisioning, signup or +// oneshot flow makes before it completes, so that a client does not see a +// login early; the flow completes with SendLogin, which queues one after +// it. What this leaves: a flow that fails and leaves the promoted session +// in place without clearing it is not pushed until the next clientState, +// and a clientState dequeued partway through such a flow reads its +// promoted session. SendLogin and HandleLogout queue one too, and so does +// the startup login attempt settling, which turns a null session into a +// real one. +// - Registration: SetChannels sets the filter and queues the first +// clientState under the router's lock, which announce takes to pick its +// recipients. A change announced after that is queued behind the +// clientState, which may already hold it, and repeating it is harmless +// because applying a value replaces the old one. A change announced before +// that was written before the clientState was even queued, so the +// clientState holds it or something newer. +type connSender struct { + xp rpc.Transporter + mu sync.Mutex + jobs []func(rpc.Transporter) + wake chan struct{} + stop chan struct{} +} + +func newConnSender(xp rpc.Transporter) *connSender { + s := &connSender{ + xp: xp, + wake: make(chan struct{}, 1), + stop: make(chan struct{}), + } + go s.run() + return s +} + +func (s *connSender) enqueue(job func(rpc.Transporter)) { + s.mu.Lock() + s.jobs = append(s.jobs, job) + s.mu.Unlock() + select { + case s.wake <- struct{}{}: + default: + } +} + +func (s *connSender) run() { + for { + select { + case <-s.stop: + return + case <-s.wake: + } + s.mu.Lock() + jobs := s.jobs + s.jobs = nil + s.mu.Unlock() + for _, job := range jobs { + select { + case <-s.stop: + return + default: + } + job(s.xp) + } + } +} + func (n *NotifyRouter) AddListener(listener NotifyListener) NotifyListenerID { n.Lock() defer n.Unlock() @@ -348,12 +449,32 @@ func (n *NotifyRouter) RemoveListener(id NotifyListenerID) { delete(n.listeners, id) } -func (n *NotifyRouter) Shutdown() {} +// Shutdown stops every connection's sender; whatever is still queued is dropped. +func (n *NotifyRouter) Shutdown() { + n.Lock() + defer n.Unlock() + for id, s := range n.senders { + close(s.stop) + delete(n.senders, id) + } +} -func (n *NotifyRouter) setNotificationChannels(id ConnectionID, val keybase1.NotificationChannels) { +// SetClientStateReader sets how a clientState reads the state it sends. It is +// called on a connection's sender goroutine, with no router lock held. A +// clientState dequeued before there is a reader sends nothing, so every +// connection that wants one gets one queued here. +func (n *NotifyRouter) SetClientStateReader(read func(context.Context) keybase1.ClientState) { + if n == nil { + return + } n.Lock() defer n.Unlock() - n.state[id] = val + n.readClientState = read + for id, s := range n.senders { + if wantsClientState(n.state[id]) { + s.enqueue(n.sendClientState(context.Background())) + } + } } func (n *NotifyRouter) getNotificationChannels(id ConnectionID) keybase1.NotificationChannels { @@ -387,15 +508,116 @@ func (n *NotifyRouter) AddConnection(xp rpc.Transporter, ch chan error) Connecti if n == nil { return 0 } - id := n.cm.AddConnection(xp, ch) - n.setNotificationChannels(id, keybase1.NotificationChannels{}) + id := n.cm.AddConnection(xp) + n.Lock() + n.state[id] = keybase1.NotificationChannels{} + n.senders[id] = newConnSender(xp) + n.Unlock() + if ch != nil { + go func() { + <-ch + n.cm.removeConnection(id) + n.removeConnection(id) + }() + } return id } -// SetChannels sets which notification channels are interested for the connection -// with the given connection ID. +func (n *NotifyRouter) removeConnection(id ConnectionID) { + n.Lock() + defer n.Unlock() + delete(n.state, id) + if s := n.senders[id]; s != nil { + close(s.stop) + delete(n.senders, id) + } +} + +// SetChannels sets which notification channels are interested for the +// connection with the given connection ID. A connection that wants clientState +// gets one queued here, ahead of every change announced after this returns. func (n *NotifyRouter) SetChannels(i ConnectionID, nc keybase1.NotificationChannels) { - n.setNotificationChannels(i, nc) + if n == nil { + return + } + n.Lock() + defer n.Unlock() + s := n.senders[i] + if s == nil { + // the connection is gone; registering it now would leak its entry + return + } + n.state[i] = nc + if wantsClientState(nc) { + s.enqueue(n.sendClientState(context.Background())) + } +} + +// clientState rides NotifyApp, so it goes to the connections that registered it. +func wantsClientState(ch keybase1.NotificationChannels) bool { return ch.App } + +func (n *NotifyRouter) sendClientState(ctx context.Context) func(rpc.Transporter) { + return func(xp rpc.Transporter) { + n.Lock() + read := n.readClientState + n.Unlock() + if read == nil { + return + } + _ = (keybase1.NotifyAppClient{ + Cli: rpc.NewClient(xp, NewContextifiedErrorUnwrapper(n.G()), nil), + }).ClientState(ctx, read(ctx)) + } +} + +// announce queues a notification to every connection whose channel filter wants +// it, on that connection's sender. See connSender for why the order it is +// queued in is the order it arrives in. +func (n *NotifyRouter) announce(ctx context.Context, name string, + wants func(keybase1.NotificationChannels) bool, + send func(ctx context.Context, xp rpc.Transporter), +) { + ctx = CopyTagsToBackground(ctx) + var queued []ConnectionID + n.Lock() + for id, s := range n.senders { + if wants(n.state[id]) { + s.enqueue(func(xp rpc.Transporter) { send(ctx, xp) }) + queued = append(queued, id) + } + } + n.Unlock() + n.G().Log.CDebugf(ctx, "| NotifyRouter#%s: queued for connections %v", name, queued) +} + +// callInOrder makes a call from a sender job without holding the connection's +// queue for the reply, which a client may take its time over. The job returns +// once the call's frame is next in line for the connection's single writer -- +// the send notifier fires there, just before the write -- so everything queued +// after it is still written after it. +func (n *NotifyRouter) callInOrder(xp rpc.Transporter, call func(*rpc.Client) error) { + released := make(chan struct{}) + var once sync.Once + cli := rpc.NewClientWithSendNotifier(xp, NewContextifiedErrorUnwrapper(n.G()), nil, + func(rpc.SeqNumber) { once.Do(func() { close(released) }) }) + done := make(chan struct{}) + go func() { + defer close(done) + _ = call(cli) + }() + select { + case <-released: + case <-done: + } +} + +// AnnounceClientState queues a clientState to every connection that wants one. +func (n *NotifyRouter) AnnounceClientState(ctx context.Context) { + if n == nil { + return + } + n.announce(ctx, "AnnounceClientState", wantsClientState, + func(ctx context.Context, xp rpc.Transporter) { n.sendClientState(ctx)(xp) }) } // HandleLogout is called whenever the current user logged out. It will broadcast @@ -405,28 +627,14 @@ func (n *NotifyRouter) HandleLogout(ctx context.Context) { return } defer n.G().CTrace(ctx, "NotifyRouter#HandleLogout", nil)() - ctx = CopyTagsToBackground(ctx) - // For all connections we currently have open... - n.cm.ApplyAllDetails(func(id ConnectionID, xp rpc.Transporter, d *keybase1.ClientDetails) bool { - // If the connection wants the `Session` notification type - registered := false - if n.getNotificationChannels(id).Session { - registered = true - // In the background do... - go func() { - // A send of a `LoggedOut` RPC - _ = (keybase1.NotifySessionClient{ - Cli: rpc.NewClient(xp, NewContextifiedErrorUnwrapper(n.G()), nil), - }).LoggedOut(ctx) - }() - } - desc := "" - if d != nil { - desc = fmt.Sprintf("%+v", *d) - } - n.G().Log.CDebugf(ctx, "| NotifyRouter#HandleLogout: client %s (sent=%v)", desc, registered) - return true - }) + n.announce(ctx, "HandleLogout", + func(ch keybase1.NotificationChannels) bool { return ch.Session }, + func(ctx context.Context, xp rpc.Transporter) { + _ = (keybase1.NotifySessionClient{ + Cli: rpc.NewClient(xp, NewContextifiedErrorUnwrapper(n.G()), nil), + }).LoggedOut(ctx) + }) + n.AnnounceClientState(ctx) n.runListeners(func(listener NotifyListener) { listener.Logout() @@ -459,24 +667,17 @@ func (n *NotifyRouter) SendLogin(ctx context.Context, u string, signedUp bool) { return } n.G().Log.CDebugf(ctx, "+ Sending login notification, as user %q, signedUp %t", u, signedUp) - // For all connections we currently have open... - ctx = CopyTagsToBackground(ctx) - n.cm.ApplyAll(func(id ConnectionID, xp rpc.Transporter) bool { - // If the connection wants the `Session` notification type - if n.getNotificationChannels(id).Session { - // In the background do... - go func() { - // A send of a `LoggedIn` RPC - _ = (keybase1.NotifySessionClient{ - Cli: rpc.NewClient(xp, NewContextifiedErrorUnwrapper(n.G()), nil), - }).LoggedIn(ctx, keybase1.LoggedInArg{ + n.announce(ctx, "SendLogin", + func(ch keybase1.NotificationChannels) bool { return ch.Session }, + func(ctx context.Context, xp rpc.Transporter) { + n.callInOrder(xp, func(cli *rpc.Client) error { + return (keybase1.NotifySessionClient{Cli: cli}).LoggedIn(ctx, keybase1.LoggedInArg{ Username: u, SignedUp: signedUp, }) - }() - } - return true - }) + }) + }) + n.AnnounceClientState(ctx) n.runListeners(func(listener NotifyListener) { listener.Login(u) @@ -2823,16 +3024,13 @@ func (n *NotifyRouter) HandleHTTPSrvInfoUpdate(ctx context.Context, info keybase if n == nil { return } - n.cm.ApplyAll(func(id ConnectionID, xp rpc.Transporter) bool { - if n.getNotificationChannels(id).Service { - go func() { - _ = (keybase1.NotifyServiceClient{ - Cli: rpc.NewClient(xp, NewContextifiedErrorUnwrapper(n.G()), nil), - }).HTTPSrvInfoUpdate(ctx, info) - }() - } - return true - }) + n.announce(ctx, "HandleHTTPSrvInfoUpdate", + func(ch keybase1.NotificationChannels) bool { return ch.Service }, + func(ctx context.Context, xp rpc.Transporter) { + _ = (keybase1.NotifyServiceClient{ + Cli: rpc.NewClient(xp, NewContextifiedErrorUnwrapper(n.G()), nil), + }).HTTPSrvInfoUpdate(ctx, info) + }) n.runListeners(func(listener NotifyListener) { listener.HTTPSrvInfoUpdate(info) }) @@ -2852,17 +3050,13 @@ func (n *NotifyRouter) HandleMobileAppState(ctx context.Context, state keybase1. if n == nil { return } - ctx = CopyTagsToBackground(ctx) - n.cm.ApplyAll(func(id ConnectionID, xp rpc.Transporter) bool { - if n.getNotificationChannels(id).App { - go func() { - _ = (keybase1.NotifyAppClient{ - Cli: rpc.NewClient(xp, NewContextifiedErrorUnwrapper(n.G()), nil), - }).MobileAppStateChanged(ctx, state) - }() - } - return true - }) + n.announce(ctx, "HandleMobileAppState", + func(ch keybase1.NotificationChannels) bool { return ch.App }, + func(ctx context.Context, xp rpc.Transporter) { + _ = (keybase1.NotifyAppClient{ + Cli: rpc.NewClient(xp, NewContextifiedErrorUnwrapper(n.G()), nil), + }).MobileAppStateChanged(ctx, state) + }) } func (n *NotifyRouter) HandleHandleKeybaseLink(ctx context.Context, link string, deferred bool) { diff --git a/go/libkb/notify_router_test.go b/go/libkb/notify_router_test.go new file mode 100644 index 000000000000..d53a83e1ec85 --- /dev/null +++ b/go/libkb/notify_router_test.go @@ -0,0 +1,334 @@ +// Copyright 2026 Keybase, Inc. All rights reserved. Use of +// this source code is governed by the included BSD license. + +package libkb + +import ( + "context" + "fmt" + "sync" + "sync/atomic" + "testing" + "time" + + "github.com/keybase/client/go/protocol/keybase1" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// readSessionOnly stands in for the service's reader: the session straight off +// the active device, read when the clientState is sent. +func readSessionOnly(g *GlobalContext) func(context.Context) keybase1.ClientState { + return func(context.Context) keybase1.ClientState { + session := keybase1.ClientSession{LoggedIn: g.ActiveDevice.Valid(), Uid: g.ActiveDevice.UID()} + return keybase1.ClientState{Session: &session} + } +} + +func clientStates(t *testing.T, rec *NotifyRecorder) []keybase1.ClientState { + t.Helper() + var ret []keybase1.ClientState + for _, m := range rec.Messages() { + if m.Method != "keybase.1.NotifyApp.clientState" { + continue + } + var arg keybase1.ClientStateArg + require.NoError(t, m.Decode(&arg)) + ret = append(ret, arg.State) + } + return ret +} + +// testLoginWrite is the write a provisioning flow makes partway through +// (kex2_provisionee, signup's device_wrap): it leaves a valid session that no +// login has announced yet. +func testLoginWrite(m MetaContext, uid keybase1.UID, name string) error { + sig, err := GenerateNaclSigningKeyPair() + if err != nil { + return err + } + enc, err := GenerateNaclDHKeyPair() + if err != nil { + return err + } + deviceID, err := NewDeviceID() + if err != nil { + return err + } + uv := keybase1.UserVersion{Uid: uid, EldestSeqno: 1} + return m.SwitchUserNewConfigActiveDevice(uv, NewNormalizedUsername(name), nil, deviceID, + sig, enc, "testdevice", KeychainModeNone) +} + +func testUID(i int) keybase1.UID { + return keybase1.UID(fmt.Sprintf("%030x19", i+1)) +} + +// A write that leaves a valid session is a login still in progress, and the +// client must not see it logged in until that login completes and says so. +func TestProvisionalValidWriteQueuesNoClientState(t *testing.T) { + tc := SetupTest(t, "NotifyRouter", 0) + defer tc.Cleanup() + g := tc.G + g.SetService() + g.NotifyRouter.SetClientStateReader(readSessionOnly(g)) + m := NewMetaContextForTest(tc) + + rec := NewNotifyRecorder(g, keybase1.NotificationChannels{App: true, Session: true}) + defer rec.Close() + rec.Flush() + require.Len(t, clientStates(t, rec), 1, "the one queued on subscribing") + + require.NoError(t, testLoginWrite(m, testUID(0), "testuser")) + require.True(t, g.ActiveDevice.Valid()) + rec.Flush() + require.Len(t, clientStates(t, rec), 1, "nothing for a login that has not completed") + + g.NotifyRouter.SendLogin(context.Background(), "testuser", false) + rec.Flush() + states := clientStates(t, rec) + require.Len(t, states, 2, "the completed login queues one") + require.True(t, states[1].Session.LoggedIn) +} + +// A write that makes the session valid outside any login flow -- a Device +// prereq bootstrapping the active device from the secret store, say -- has no +// SendLogin behind it, so the write itself has to reach clients. +func TestBootstrapStyleWriteQueuesClientState(t *testing.T) { + tc := SetupTest(t, "NotifyRouter", 0) + defer tc.Cleanup() + g := tc.G + g.SetService() + g.NotifyRouter.SetClientStateReader(readSessionOnly(g)) + m := NewMetaContextForTest(tc) + + uid := testUID(0) + deviceID, err := NewDeviceID() + require.NoError(t, err) + require.NoError(t, m.SwitchUserNewConfig(uid, NewNormalizedUsername("testuser"), nil, deviceID)) + + rec := NewNotifyRecorder(g, keybase1.NotificationChannels{App: true, Session: true}) + defer rec.Close() + rec.Flush() + require.Len(t, clientStates(t, rec), 1, "the one queued on subscribing") + + sig, err := GenerateNaclSigningKeyPair() + require.NoError(t, err) + enc, err := GenerateNaclDHKeyPair() + require.NoError(t, err) + require.NoError(t, m.SetActiveDevice(keybase1.UserVersion{Uid: uid, EldestSeqno: 1}, deviceID, + sig, enc, "testdevice", KeychainModeNone)) + require.True(t, g.ActiveDevice.Valid()) + rec.Flush() + states := clientStates(t, rec) + require.Len(t, states, 2, "the write that made the session valid queued one") + require.True(t, states[1].Session.LoggedIn) +} + +// A release that changes nothing about the session has nothing to tell. +func TestUnchangedSessionQueuesNoClientState(t *testing.T) { + tc := SetupTest(t, "NotifyRouter", 0) + defer tc.Cleanup() + g := tc.G + g.SetService() + g.NotifyRouter.SetClientStateReader(readSessionOnly(g)) + m := NewMetaContextForTest(tc) + + rec := NewNotifyRecorder(g, keybase1.NotificationChannels{App: true, Session: true}) + defer rec.Close() + rec.Flush() + require.Len(t, clientStates(t, rec), 1, "the one queued on subscribing") + + require.False(t, g.ActiveDevice.Valid()) + require.NoError(t, m.SwitchUserLoggedOut()) + rec.Flush() + require.Len(t, clientStates(t, rec), 1, "logged out before and after") +} + +// A clear needs no announce to reach clients: a flow that fails and clears +// what it set, without a logout, still leaves every client logged out. +func TestSessionClearQueuesClientState(t *testing.T) { + tc := SetupTest(t, "NotifyRouter", 0) + defer tc.Cleanup() + g := tc.G + g.SetService() + g.NotifyRouter.SetClientStateReader(readSessionOnly(g)) + m := NewMetaContextForTest(tc) + + require.NoError(t, testLoginWrite(m, testUID(0), "testuser")) + g.NotifyRouter.SendLogin(context.Background(), "testuser", false) + + rec := NewNotifyRecorder(g, keybase1.NotificationChannels{App: true, Session: true}) + defer rec.Close() + rec.Flush() + states := clientStates(t, rec) + require.Len(t, states, 1) + require.True(t, states[0].Session.LoggedIn) + + require.NoError(t, m.SwitchUserLoggedOut()) + rec.Flush() + states = clientStates(t, rec) + require.Len(t, states, 2, "the clear queued one without any announce") + require.False(t, states[1].Session.LoggedIn) +} + +// Logins and logouts are not serialized against each other -- a login writes +// its device under switchUserMu and announces after releasing it -- so their +// loggedIn/loggedOut events can arrive in any order. What must hold anyway is +// that the last clientState every connection gets carries the session as it +// finally is. +func TestLastClientStateCarriesFinalSession(t *testing.T) { + tc := SetupTest(t, "NotifyRouter", 0) + defer tc.Cleanup() + g := tc.G + g.SetService() + g.NotifyRouter.SetClientStateReader(readSessionOnly(g)) + m := NewMetaContextForTest(tc) + + var recs []*NotifyRecorder + for range 3 { + rec := NewNotifyRecorder(g, keybase1.NotificationChannels{App: true, Session: true}) + defer rec.Close() + recs = append(recs, rec) + } + + ctx := context.Background() + var wg sync.WaitGroup + for i := range 6 { + wg.Add(1) + go func() { + defer wg.Done() + for j := range 10 { + switch (i + j) % 3 { + case 0: + name := fmt.Sprintf("testuser%d", i) + if assert.NoError(t, testLoginWrite(m, testUID(i*10+j), name)) { + g.NotifyRouter.SendLogin(ctx, name, false) + } + case 1: + assert.NoError(t, m.LogoutKeepSecrets()) + default: + // a flow that fails and clears what it set, announcing nothing + assert.NoError(t, m.SwitchUserLoggedOut()) + } + } + }() + } + wg.Wait() + + want := keybase1.ClientSession{LoggedIn: g.ActiveDevice.Valid(), Uid: g.ActiveDevice.UID()} + for _, rec := range recs { + rec.Flush() + states := clientStates(t, rec) + require.NotEmpty(t, states) + require.Equal(t, want, *states[len(states)-1].Session, "connection %d", rec.ID) + } +} + +// A clientState reads the state when it is sent, not when it is queued. That is +// what lets the last one carry the latest session although nothing orders a +// login's write against a logout's announce: whichever clientState is sent last +// reads after every write that queued one. +func TestClientStateReadsWhenSent(t *testing.T) { + tc := SetupTest(t, "NotifyRouter", 0) + defer tc.Cleanup() + g := tc.G + g.SetService() + + var mu sync.Mutex + loggedIn := false + var reads atomic.Int32 + entered := make(chan struct{}) + gate := make(chan struct{}) + g.NotifyRouter.SetClientStateReader(func(context.Context) keybase1.ClientState { + if reads.Add(1) == 1 { + close(entered) + <-gate + } + mu.Lock() + defer mu.Unlock() + session := keybase1.ClientSession{LoggedIn: loggedIn} + return keybase1.ClientState{Session: &session} + }) + + rec := NewNotifyRecorder(g, keybase1.NotificationChannels{App: true}) + defer rec.Close() + // the sender is now busy with the first clientState, so the next one waits in the queue + <-entered + g.NotifyRouter.AnnounceClientState(context.Background()) + mu.Lock() + loggedIn = true + mu.Unlock() + close(gate) + rec.Flush() + + states := clientStates(t, rec) + require.Len(t, states, 2) + require.True(t, states[1].Session.LoggedIn, "queued before the change, read after it") +} + +// A late SetChannels for a connection that has already closed must not bring +// its entry back: nothing would ever remove it again. +func TestSetChannelsAfterCloseRegistersNothing(t *testing.T) { + tc := SetupTest(t, "NotifyRouter", 0) + defer tc.Cleanup() + g := tc.G + g.SetService() + n := g.NotifyRouter + + rec := NewNotifyRecorder(g, keybase1.NotificationChannels{App: true}) + rec.Close() + require.Eventually(t, func() bool { + n.Lock() + defer n.Unlock() + return n.senders[rec.ID] == nil + }, 5*time.Second, time.Millisecond) + + n.SetChannels(rec.ID, keybase1.NotificationChannels{App: true}) + n.Lock() + _, registered := n.state[rec.ID] + n.Unlock() + require.False(t, registered) +} + +// A oneshot device is a login still in progress, like a provisioning write: +// clients must not see it logged in until the login completes and says so. +func TestOneshotDeviceQueuesNoClientState(t *testing.T) { + tc := SetupTest(t, "NotifyRouter", 0) + defer tc.Cleanup() + g := tc.G + g.SetService() + g.NotifyRouter.SetClientStateReader(readSessionOnly(g)) + m := NewMetaContextForTest(tc) + + rec := NewNotifyRecorder(g, keybase1.NotificationChannels{App: true, Session: true}) + defer rec.Close() + rec.Flush() + require.Len(t, clientStates(t, rec), 1, "the one queued on subscribing") + + sig, err := GenerateNaclSigningKeyPair() + require.NoError(t, err) + enc, err := GenerateNaclDHKeyPair() + require.NoError(t, err) + deviceID, err := NewDeviceID() + require.NoError(t, err) + uv := keybase1.UserVersion{Uid: testUID(0), EldestSeqno: 1} + require.NoError(t, m.SwitchUserToActiveOneshotDevice(uv, NewNormalizedUsername("testuser"), + NewDeviceWithKeys(sig, enc, deviceID, "testdevice", KeychainModeNone))) + require.True(t, g.ActiveDevice.Valid()) + rec.Flush() + require.Len(t, clientStates(t, rec), 1, "nothing for a login that has not completed") + + g.NotifyRouter.SendLogin(context.Background(), "testuser", false) + rec.Flush() + states := clientStates(t, rec) + require.Len(t, states, 2, "the completed login queues one") + require.True(t, states[1].Session.LoggedIn) +} + +// A standalone client runs the service without ever setting up a router. +func TestNilRouterSettersAreNoOps(t *testing.T) { + var n *NotifyRouter + n.SetClientStateReader(func(context.Context) keybase1.ClientState { return keybase1.ClientState{} }) + n.SetChannels(ConnectionID(1), keybase1.NotificationChannels{App: true}) +} diff --git a/go/libkb/test_notify_recorder.go b/go/libkb/test_notify_recorder.go new file mode 100644 index 000000000000..4a3401653c37 --- /dev/null +++ b/go/libkb/test_notify_recorder.go @@ -0,0 +1,164 @@ +// Copyright 2026 Keybase, Inc. All rights reserved. Use of +// this source code is governed by the included BSD license. + +package libkb + +import ( + "context" + "errors" + "io" + "net" + "sync" + "time" + + "github.com/keybase/client/go/protocol/keybase1" + "github.com/keybase/go-codec/codec" + "github.com/keybase/go-framed-msgpack-rpc/rpc" +) + +// NotifyRecorder is a connection registered with a NotifyRouter, for tests. +// It records the notifications and calls sent to it in the order they were +// written: each is decoded inside the transport's Write, so it is recorded +// before the send returns. It never answers a call. A Go rpc.Server on the +// far end would serve each notification on its own goroutine and lose that +// order. +type NotifyRecorder struct { + ID ConnectionID + router *NotifyRouter + conn *recorderConn + closed chan error +} + +// RecordedNotify is one notification or call a NotifyRecorder saw. +type RecordedNotify struct { + Method string + arg []byte +} + +func newRecorderHandle() *codec.MsgpackHandle { + return &codec.MsgpackHandle{WriteExt: true, RawToString: true} +} + +// Decode decodes the message's single argument, e.g. into a +// keybase1.ClientStateArg. +func (r RecordedNotify) Decode(v any) error { + return codec.NewDecoderBytes(r.arg, newRecorderHandle()).Decode(v) +} + +// NewNotifyRecorder adds a connection to g's router and registers it for the +// given channels. +func NewNotifyRecorder(g *GlobalContext, channels keybase1.NotificationChannels) *NotifyRecorder { + conn := &recorderConn{readDone: make(chan struct{})} + xp := rpc.NewTransport(conn, NewRPCLogFactory(g), g.LocalNetworkInstrumenterStorage, + MakeWrapError(g), rpc.DefaultMaxFrameLength) + closed := make(chan error, 1) + // runs the transport's reader, as the service does, so that closing the + // connection fails the calls that were never answered + rpc.NewServer(xp, MakeWrapError(g)).Run() + id := g.NotifyRouter.AddConnection(xp, closed) + g.NotifyRouter.SetChannels(id, channels) + return &NotifyRecorder{ID: id, router: g.NotifyRouter, conn: conn, closed: closed} +} + +const recorderFlushMethod = "keybase.1.NotifyRecorder.flush" + +// Flush waits until everything queued to this connection so far has been +// written. It queues a marker notification and waits for its send, which +// returns only once the connection's single writer has written it, and so +// everything ahead of it. +func (r *NotifyRecorder) Flush() { + n := r.router + done := make(chan struct{}) + n.Lock() + s := n.senders[r.ID] + if s != nil { + s.enqueue(func(xp rpc.Transporter) { + defer close(done) + _ = rpc.NewClient(xp, nil, nil).Notify(context.Background(), recorderFlushMethod, []any{}, 0) + }) + } + n.Unlock() + if s == nil { + return + } + select { + case <-done: + case <-s.stop: + } +} + +// Messages returns what has been recorded so far, oldest first. +func (r *NotifyRecorder) Messages() []RecordedNotify { + r.conn.mu.Lock() + defer r.conn.mu.Unlock() + return append([]RecordedNotify(nil), r.conn.msgs...) +} + +// Close closes the connection, which removes it from the router. +func (r *NotifyRecorder) Close() { + _ = r.conn.Close() + r.closed <- io.EOF +} + +type recorderConn struct { + mu sync.Mutex + msgs []RecordedNotify + closeOnce sync.Once + readDone chan struct{} +} + +var _ net.Conn = (*recorderConn)(nil) + +// Write gets exactly one frame per call: the rpc encoder writes each frame, its +// length prefix included, in a single Write. +func (c *recorderConn) Write(b []byte) (int, error) { + dec := codec.NewDecoderBytes(b, newRecorderHandle()) + var length int + var frame []any + if err := dec.Decode(&length); err != nil { + return 0, err + } + if err := dec.Decode(&frame); err != nil { + return 0, err + } + // a notification is [2, method, args, tags?]; a call is [0, seqid, method, args, tags?] + if len(frame) > 1 { + if _, isMethod := frame[1].(string); !isMethod { + frame = append(frame[:1], frame[2:]...) + } + } + if len(frame) < 3 { + return 0, errors.New("NotifyRecorder: not a call or a notification") + } + method, _ := frame[1].(string) + if method == recorderFlushMethod { + return len(b), nil + } + args, _ := frame[2].([]any) + var arg []byte + if len(args) > 0 { + if err := codec.NewEncoderBytes(&arg, newRecorderHandle()).Encode(args[0]); err != nil { + return 0, err + } + } + c.mu.Lock() + c.msgs = append(c.msgs, RecordedNotify{Method: method, arg: arg}) + c.mu.Unlock() + return len(b), nil +} + +func (c *recorderConn) Read([]byte) (int, error) { + <-c.readDone + return 0, io.EOF +} + +func (c *recorderConn) Close() error { + c.closeOnce.Do(func() { close(c.readDone) }) + return nil +} + +func (c *recorderConn) LocalAddr() net.Addr { return nil } +func (c *recorderConn) RemoteAddr() net.Addr { return nil } +func (c *recorderConn) SetDeadline(time.Time) error { return nil } +func (c *recorderConn) SetReadDeadline(time.Time) error { return nil } +func (c *recorderConn) SetWriteDeadline(time.Time) error { return nil } diff --git a/go/service/notify.go b/go/service/notify.go index c145f378da9f..085f8fc1d60d 100644 --- a/go/service/notify.go +++ b/go/service/notify.go @@ -28,6 +28,9 @@ func NewNotifyCtlHandler(xp rpc.Transporter, id libkb.ConnectionID, g *libkb.Glo } } +// SetNotifications registers the channels. A connection that registers app +// notifications then gets a clientState, ahead of every change announced after +// this returns; see libkb.connSender. func (h *NotifyCtlHandler) SetNotifications(_ context.Context, n keybase1.NotificationChannels) error { h.G().NotifyRouter.SetChannels(h.id, n) return nil From b1a5d30d2e186bfb6899e25d5f9cd76949dcd3f9 Mon Sep 17 00:00:00 2001 From: chrisnojima Date: Mon, 21 Sep 2026 12:23:34 -0400 Subject: [PATCH 03/12] feat(service): announce the client state on every session change lockSwitchUser wraps switchUserMu, the lock every session write is made under, and queues a clientState once the lock is released if the write changed the session. The promotion flag marks the writes a provisioning, signup or oneshot flow makes before it completes, so a client does not see the login early; the flow completes with SendLogin, which queues one. engine.SessionState factors the wait-free part of Bootstrap.Run out so the two cannot drift, and the service reads it, the http server address and the app state into a clientState when one is sent. The session is left out until the startup login attempt settles, and settling queues another clientState that carries it. A connection whose RegisterProtocols fails now signals its close channel, which frees its slot and its sender. --- go/engine/bootstrap.go | 41 ++++-- go/libkb/context.go | 18 +-- go/libkb/globals.go | 37 +++++- go/libkb/logout.go | 2 +- go/service/main.go | 61 ++++++++- go/service/notify_test.go | 268 ++++++++++++++++++++++++++++++++++++++ 6 files changed, 394 insertions(+), 33 deletions(-) create mode 100644 go/service/notify_test.go diff --git a/go/engine/bootstrap.go b/go/engine/bootstrap.go index d6ede82d3da2..abe355d630e2 100644 --- a/go/engine/bootstrap.go +++ b/go/engine/bootstrap.go @@ -62,28 +62,41 @@ func (e *Bootstrap) lookupFullname(m libkb.MetaContext, uv keybase1.UserVersion) e.status.Fullname = pkg.FullName.FullName } +// SessionState reads the session fields that are available with nothing to wait +// on: the active device. Bootstrap fills the same fields plus the slower derived +// ones, so the two cannot drift. The returned UserVersion is the active device's, +// empty when logged out. +func SessionState(m libkb.MetaContext) (res keybase1.ClientSession, uv keybase1.UserVersion) { + // if any Login engine worked previously, then ActiveDevice will + // be valid; the only way for it to be valid is to be logged in + // (and provisioned) + res.LoggedIn = m.G().ActiveDevice.Valid() + if !res.LoggedIn { + return res, uv + } + + uv, res.DeviceID, res.DeviceName, _, _ = m.G().ActiveDevice.AllFields() + res.Uid = uv.Uid + res.Username = m.G().ActiveDevice.Username(m).String() + return res, uv +} + // Run starts the engine. func (e *Bootstrap) Run(m libkb.MetaContext) (err error) { defer m.Trace("Bootstrap.Run", &err)() - e.status.Registered = e.signedUp(m) - - // if any Login engine worked previously, then ActiveDevice will - // be valid: - validActiveDevice := m.G().ActiveDevice.Valid() + session, uv := SessionState(m) + e.status.Registered = signedUp(m) + e.status.LoggedIn = session.LoggedIn + e.status.Uid = session.Uid + e.status.Username = session.Username + e.status.DeviceID = session.DeviceID + e.status.DeviceName = session.DeviceName - // the only way for ActiveDevice to be valid is to be logged in - // (and provisioned) - e.status.LoggedIn = validActiveDevice if !e.status.LoggedIn { m.Debug("Bootstrap: not logged in") return nil } m.Debug("Bootstrap: logged in (valid active device)") - - var uv keybase1.UserVersion - uv, e.status.DeviceID, e.status.DeviceName, _, _ = e.G().ActiveDevice.AllFields() - e.status.Uid = uv.Uid - e.status.Username = e.G().ActiveDevice.Username(m).String() m.Debug("Bootstrap status: uid=%s, username=%s, deviceID=%s, deviceName=%s", e.status.Uid, e.status.Username, e.status.DeviceID, e.status.DeviceName) if chatHelper := e.G().ChatHelper; chatHelper != nil { @@ -96,7 +109,7 @@ func (e *Bootstrap) Run(m libkb.MetaContext) (err error) { } // signedUp is true if there's a uid in config.json. -func (e *Bootstrap) signedUp(m libkb.MetaContext) bool { +func signedUp(m libkb.MetaContext) bool { cr := m.G().Env.GetConfig() if cr == nil { return false diff --git a/go/libkb/context.go b/go/libkb/context.go index 2e9fb26a1a12..b119c021e1dd 100644 --- a/go/libkb/context.go +++ b/go/libkb/context.go @@ -366,7 +366,7 @@ func (m MetaContext) SwitchUserNewConfig(u keybase1.UID, n NormalizedUsername, s func (m MetaContext) switchUserNewConfig(u keybase1.UID, n NormalizedUsername, salt []byte, d keybase1.DeviceID, ad *ActiveDevice) error { g := m.G() - defer g.switchUserMu.Acquire(m, "switchUserNewConfig")() + defer g.lockSwitchUser(m, ad != nil, "switchUserNewConfig")() cw := g.Env.GetConfigWriter() if cw == nil { return NoConfigWriterError{} @@ -398,7 +398,7 @@ func (m MetaContext) SwitchUserNewConfigActiveDevice(uv keybase1.UserVersion, n // etc). It does this in a critical section, holding switchUserMu. func (m MetaContext) SwitchUserNukeConfig(n NormalizedUsername) error { g := m.G() - defer g.switchUserMu.Acquire(m, "SwitchUserNukeConfig")() + defer g.lockSwitchUser(m, false, "SwitchUserNukeConfig")() cw := g.Env.GetConfigWriter() cr := g.Env.GetConfig() if cw == nil { @@ -435,7 +435,7 @@ func (m MetaContext) SwitchUserToActiveDevice(n NormalizedUsername, ad *ActiveDe if !n.IsValid() { return NewBadUsernameError(n.String()) } - defer g.switchUserMu.Acquire(m, "SwitchUserToActiveDevice %v", n)() + defer g.lockSwitchUser(m, false, "SwitchUserToActiveDevice %v", n)() cw := g.Env.GetConfigWriter() if cw == nil { return NoConfigWriterError{} @@ -459,7 +459,7 @@ func (m MetaContext) SwitchUserToActiveDevice(n NormalizedUsername, ad *ActiveDe func (m MetaContext) SwitchUserDeprovisionNukeConfig(username NormalizedUsername) error { g := m.G() - defer g.switchUserMu.Acquire(m, "SwitchUserDeprovisionNukeConfig %v", username)() + defer g.lockSwitchUser(m, false, "SwitchUserDeprovisionNukeConfig %v", username)() cw := g.Env.GetConfigWriter() if cw == nil { @@ -481,7 +481,7 @@ func (m MetaContext) SwitchUserToActiveOneshotDevice(uv keybase1.UserVersion, nu defer m.Trace("MetaContext#SwitchUserToActiveOneshotDevice", &err)() g := m.G() - defer g.switchUserMu.Acquire(m, "SwitchUserToActiveOneshotDevice")() + defer g.lockSwitchUser(m, true, "SwitchUserToActiveOneshotDevice")() cw := g.Env.GetConfigWriter() if cw == nil { return NoConfigWriterError{} @@ -504,7 +504,7 @@ func (m MetaContext) SwitchUserToActiveOneshotDevice(uv keybase1.UserVersion, nu func (m MetaContext) SwitchUserLoggedOut() (err error) { defer m.Trace("MetaContext#SwitchUserLoggedOut", &err)() g := m.G() - defer g.switchUserMu.Acquire(m, "SwitchUserLoggedOut")() + defer g.lockSwitchUser(m, false, "SwitchUserLoggedOut")() cw := g.Env.GetConfigWriter() if cw == nil { return NoConfigWriterError{} @@ -530,7 +530,7 @@ func (m MetaContext) SetActiveDevice(uv keybase1.UserVersion, deviceID keybase1. sigKey, encKey GenericKey, deviceName string, keychainMode KeychainMode, ) error { g := m.G() - defer g.switchUserMu.Acquire(m, "SetActiveDevice")() + defer g.lockSwitchUser(m, false, "SetActiveDevice")() if !g.Env.GetUID().Equal(uv.Uid) { return NewUIDMismatchError("UID switched out from underneath provisioning process") } @@ -539,13 +539,13 @@ func (m MetaContext) SetActiveDevice(uv keybase1.UserVersion, deviceID keybase1. func (m MetaContext) SetSigningKey(uv keybase1.UserVersion, deviceID keybase1.DeviceID, sigKey GenericKey, deviceName string) error { g := m.G() - defer g.switchUserMu.Acquire(m, "SetSigningKey")() + defer g.lockSwitchUser(m, false, "SetSigningKey")() return g.ActiveDevice.setSigningKey(g, uv, deviceID, sigKey, deviceName) } func (m MetaContext) SetEncryptionKey(uv keybase1.UserVersion, deviceID keybase1.DeviceID, encKey GenericKey) error { g := m.G() - defer g.switchUserMu.Acquire(m, "SetEncryptionKey")() + defer g.lockSwitchUser(m, false, "SetEncryptionKey")() return g.ActiveDevice.setEncryptionKey(uv, deviceID, encKey) } diff --git a/go/libkb/globals.go b/go/libkb/globals.go index 48b622bbd4cc..77cbb6dd8d2b 100644 --- a/go/libkb/globals.go +++ b/go/libkb/globals.go @@ -169,7 +169,8 @@ type GlobalContext struct { // It is threadsafe to call methods on ActiveDevice which will always be non-nil. // But don't access its members directly. If you're going to be changing out the - // user (and resetting the ActiveDevice), then you should hold the switchUserMu + // user (and resetting the ActiveDevice), then you should hold the switchUserMu, + // through lockSwitchUser switchUserMu *VerboseLock ActiveDevice *ActiveDevice switchedUsers map[NormalizedUsername]bool // bookkeep users who have been switched over (and are still in secret store) @@ -358,10 +359,42 @@ func (g *GlobalContext) SetAvatarLoader(a AvatarLoaderSource) { g.avatarLoader = a } +type sessionIdentity struct { + valid bool + uid keybase1.UID + deviceID keybase1.DeviceID +} + +func (g *GlobalContext) sessionIdentity() sessionIdentity { + return sessionIdentity{valid: g.ActiveDevice.Valid(), uid: g.ActiveDevice.UID(), deviceID: g.ActiveDevice.DeviceID()} +} + +// lockSwitchUser takes switchUserMu, which every session write (the active +// device, the config's current user) is made under. A release that changed the +// session queues a clientState to connected clients, after unlocking. +// +// promotion marks the write a provisioning, signup or oneshot flow makes before +// it completes: if it leaves a valid session it queues nothing, so clients do +// not see the login early -- the flow completes with SendLogin, which queues +// one. A promotion that leaves no valid session is a clear and queues one like +// any other. See connSender for why that is enough. +func (g *GlobalContext) lockSwitchUser(mctx MetaContext, promotion bool, reasonFormat string, args ...any) (release func()) { + unlock := g.switchUserMu.Acquire(mctx, reasonFormat, args...) + before := g.sessionIdentity() + return func() { + after := g.sessionIdentity() + unlock() + earlyLogin := promotion && after.valid + if after != before && !earlyLogin { + g.NotifyRouter.AnnounceClientState(mctx.Ctx()) + } + } +} + // simulateServiceRestart simulates what happens when a service restarts for the // purposes of testing. func (g *GlobalContext) simulateServiceRestart() { - defer g.switchUserMu.Acquire(NewMetaContext(context.TODO(), g), "simulateServiceRestart")() + defer g.lockSwitchUser(NewMetaContext(context.TODO(), g), false, "simulateServiceRestart")() _ = g.ActiveDevice.Clear() } diff --git a/go/libkb/logout.go b/go/libkb/logout.go index 5c28274431a4..94b48da0c7eb 100644 --- a/go/libkb/logout.go +++ b/go/libkb/logout.go @@ -30,7 +30,7 @@ func (mctx MetaContext) LogoutUsernameWithOptions(username NormalizedUsername, o defer mctx.Trace(fmt.Sprintf("MetaContext#LogoutWithOptions(%#v)", options), &err)() g := mctx.G() - defer g.switchUserMu.Acquire(mctx, "Logout")() + defer g.lockSwitchUser(mctx, false, "Logout")() mctx.Debug("MetaContext#logoutWithSecretKill: after switchUserMu acquisition (username: %s, options: %#v)", username, options) diff --git a/go/service/main.go b/go/service/main.go index 59f7ac42c459..53d9b67fd6f8 100644 --- a/go/service/main.go +++ b/go/service/main.go @@ -232,6 +232,8 @@ func (d *Service) Handle(c net.Conn) { } if err := d.RegisterProtocols(server, xp, connID, logReg); err != nil { d.G().Log.Warning("RegisterProtocols error: %s", err) + // frees the connection's slot and its notification sender + cl <- err return } @@ -330,6 +332,12 @@ func (d *Service) Run() (err error) { d.SetupChatModules(nil) + // Before the listen loop on purpose: this runs the startup login attempt, so a + // client that connects once we are listening finds it already settled and its + // first clientState carries a session rather than "not known yet". Mobile + // cannot do this -- go/bind/keybase.go runs the attempt off the Init thread, + // after the loopback listener -- so a clientState says so explicitly, and + // another follows once the attempt settles. d.RunBackgroundOperations(uir) // At this point initialization is complete, and we're about to start the @@ -353,6 +361,7 @@ func (d *Service) SetupCriticalSubServices() error { // up, so both see a nil router, which announces nothing -- and nothing // subscribes to it anyway. d.httpSrv = manager.NewSrv(d.G()) + d.G().NotifyRouter.SetClientStateReader(d.readClientState) d.G().RuntimeStats = runtimestats.NewRunner(allG) teams.ServiceInit(d.G()) stellar.ServiceInit(d.G(), d.walletState, d.badger) @@ -1398,12 +1407,19 @@ func (d *Service) configurePath() { } } -// tryLogin runs LoginOffline which will load the local session file and unlock the -// local device keys without making any network requests. -// -// If that fails for any reason, LoginProvisionedDevice is used, which should get -// around any issue where the session.json file is out of date or missing since the -// last time the service started. +// initialLoginAttemptSettled reports whether the first startup login attempt has +// finished, without waiting for it. A caller that must not block uses this to say +// "I do not know yet" instead of reporting a logged-out session that no attempt +// has been made for. +func (d *Service) initialLoginAttemptSettled() bool { + select { + case <-d.initialLoginAttemptDone: + return true + default: + return false + } +} + // awaitInitialLoginAttempt blocks until the first startup login attempt has // finished (however it went), the context is done, or maxWait elapses. Used // by RPCs whose answer depends on login state so they don't race the login @@ -1418,10 +1434,41 @@ func (d *Service) awaitInitialLoginAttempt(m libkb.MetaContext, maxWait time.Dur } } +// settleInitialLoginAttempt marks the first startup login attempt finished and +// then sends connected clients a clientState, which now carries the session. +func (d *Service) settleInitialLoginAttempt(ctx context.Context) { + d.initialLoginAttemptOnce.Do(func() { + close(d.initialLoginAttemptDone) + d.G().NotifyRouter.AnnounceClientState(ctx) + }) +} + +// readClientState reads what a clientState notification carries. The session is +// left out until the startup login attempt has settled: before that there is no +// session to describe, and reporting a logged-out one would be a lie. The +// attempt settling queues another clientState, which carries it. +func (d *Service) readClientState(ctx context.Context) keybase1.ClientState { + res := keybase1.ClientState{AppState: d.G().MobileAppState.State()} + if d.initialLoginAttemptSettled() { + session, _ := engine.SessionState(libkb.NewMetaContext(ctx, d.G())) + res.Session = &session + } + if info, err := d.httpSrv.Info(); err == nil { + res.HttpSrvInfo = &info + } + return res +} + +// tryLogin runs LoginOffline which will load the local session file and unlock the +// local device keys without making any network requests. +// +// If that fails for any reason, LoginProvisionedDevice is used, which should get +// around any issue where the session.json file is out of date or missing since the +// last time the service started. func (d *Service) tryLogin(ctx context.Context, mode libkb.LoginAttempt) { if mode != libkb.LoginAttemptNone { // Signal on every exit path; sync.Once makes repeat calls no-ops. - defer d.initialLoginAttemptOnce.Do(func() { close(d.initialLoginAttemptDone) }) + defer d.settleInitialLoginAttempt(ctx) } d.loginAttemptMu.Lock() diff --git a/go/service/notify_test.go b/go/service/notify_test.go new file mode 100644 index 000000000000..cbaf97281553 --- /dev/null +++ b/go/service/notify_test.go @@ -0,0 +1,268 @@ +package service + +import ( + "context" + "runtime" + "sync" + "testing" + + "github.com/keybase/client/go/kbhttp" + "github.com/keybase/client/go/kbhttp/manager" + "github.com/keybase/client/go/libkb" + "github.com/keybase/client/go/libkb/lifecycle/lifecycletest" + keybase1 "github.com/keybase/client/go/protocol/keybase1" + "github.com/stretchr/testify/require" +) + +// newTestClientStateService sets up what SetupCriticalSubServices does for +// clientState: the http server and the reader. +func newTestClientStateService(t *testing.T, g *libkb.GlobalContext) *Service { + t.Helper() + svc := NewService(g, false) + svc.httpSrv = manager.NewSrv(g) + g.NotifyRouter.SetClientStateReader(svc.readClientState) + return svc +} + +var allClientStateChannels = keybase1.NotificationChannels{App: true, Session: true, Service: true} + +const ( + methodClientState = "keybase.1.NotifyApp.clientState" + methodAppState = "keybase.1.NotifyApp.mobileAppStateChanged" + methodHTTPSrvInfo = "keybase.1.NotifyService.HTTPSrvInfoUpdate" + methodLoggedIn = "keybase.1.NotifySession.loggedIn" +) + +func decodeClientState(t *testing.T, m libkb.RecordedNotify) keybase1.ClientState { + t.Helper() + require.Equal(t, methodClientState, m.Method) + var arg keybase1.ClientStateArg + require.NoError(t, m.Decode(&arg)) + return arg.State +} + +func clientStatesOf(t *testing.T, msgs []libkb.RecordedNotify) (ret []keybase1.ClientState) { + t.Helper() + for _, m := range msgs { + if m.Method == methodClientState { + ret = append(ret, decodeClientState(t, m)) + } + } + return ret +} + +// testLoginWrite makes the session valid the way a login does before it +// announces itself. +func testLoginWrite(t *testing.T, tc libkb.TestContext, name string) { + t.Helper() + sig, err := libkb.GenerateNaclSigningKeyPair() + require.NoError(t, err) + enc, err := libkb.GenerateNaclDHKeyPair() + require.NoError(t, err) + deviceID, err := libkb.NewDeviceID() + require.NoError(t, err) + uv := keybase1.UserVersion{Uid: libkb.UsernameToUID(name), EldestSeqno: 1} + require.NoError(t, libkb.NewMetaContextForTest(tc).SwitchUserNewConfigActiveDevice(uv, + libkb.NewNormalizedUsername(name), nil, deviceID, sig, enc, "testdevice", libkb.KeychainModeNone)) +} + +// A client applies what it gets in arrival order, so the state as of +// subscribing has to arrive before any change announced after it. +func TestSnapshotIsFirstOnConnection(t *testing.T) { + tc := libkb.SetupTest(t, "notify", 0) + defer tc.Cleanup() + g := tc.G + g.SetService() + svc := newTestClientStateService(t, g) + svc.settleInitialLoginAttempt(context.Background()) + + rec := libkb.NewNotifyRecorder(g, allClientStateChannels) + defer rec.Close() + g.NotifyRouter.HandleLogout(context.Background()) + g.MobileLifecycle.UIInactive() + rec.Flush() + + msgs := rec.Messages() + require.NotEmpty(t, msgs) + first := decodeClientState(t, msgs[0]) + require.NotNil(t, first.Session) + info, err := svc.httpSrv.Info() + require.NoError(t, err) + require.Equal(t, &info, first.HttpSrvInfo) + require.Greater(t, len(msgs), 1, "the changes after it arrive after it") +} + +// Each field has one writer, which queues its notification in write order, and +// a clientState reads every field when it is sent, so whatever interleaving the +// writers and the clientStates take, the last value a connection receives for a +// field is the field's current value. +func TestLastMessagePerFieldIsLatest(t *testing.T) { + // Two Ps: the writers still run in parallel, but goroutines started in a row + // no longer reliably run in the order they were started, which is what a + // fan-out of one goroutine per message gets wrong. With one P per core that + // fan-out passes this test almost every time. + defer runtime.GOMAXPROCS(runtime.GOMAXPROCS(2)) + tc := libkb.SetupTest(t, "notify", 0) + defer tc.Cleanup() + g := tc.G + g.SetService() + svc := NewService(g, false) + // A fresh port on every bind, unlike the service's pinned one, so each + // rebind is an address change the server announces. + srv, err := manager.New("Srv", g.GetLog(), g.MobileAppState.State, g.MobileAppState.NextUpdate, + func() kbhttp.ListenerSource { return kbhttp.NewAutoPortListenerSource() }, true, + g.NotifyRouter.HandleHTTPSrvInfoUpdate) + require.NoError(t, err) + svc.httpSrv = srv + g.NotifyRouter.SetClientStateReader(svc.readClientState) + svc.settleInitialLoginAttempt(context.Background()) + + rec := libkb.NewNotifyRecorder(g, allClientStateChannels) + defer rec.Close() + + ctx := context.Background() + var wg sync.WaitGroup + // 50 app state updates from 5 goroutines. Each move from BACKGROUND to the + // foreground rebinds the http server, which is the http address's writer. + for i := range 5 { + wg.Add(1) + go func() { + defer wg.Done() + for j := range 10 { + switch (i + j) % 3 { + case 0: + g.MobileLifecycle.UIActive() + case 1: + g.MobileLifecycle.UIInactive() + default: + lifecycletest.ToBackground(g.MobileLifecycle) + } + } + }() + } + // clientStates interleaved with all of it + wg.Add(1) + go func() { + defer wg.Done() + for range 20 { + g.NotifyRouter.AnnounceClientState(ctx) + } + }() + wg.Wait() + // Stops the http server's writer for good: nothing moves the address after this. + svc.httpSrv.Shutdown() + rec.Flush() + + var lastAppState *keybase1.MobileAppState + var lastHTTP *keybase1.HttpSrvInfo + var appStateChanges []keybase1.MobileAppState + var httpChanges []keybase1.HttpSrvInfo + for _, m := range rec.Messages() { + switch m.Method { + case methodClientState: + state := decodeClientState(t, m) + lastAppState = &state.AppState + lastHTTP = state.HttpSrvInfo + case methodAppState: + var arg keybase1.MobileAppStateChangedArg + require.NoError(t, m.Decode(&arg)) + lastAppState = &arg.State + appStateChanges = append(appStateChanges, arg.State) + case methodHTTPSrvInfo: + var arg keybase1.HTTPSrvInfoUpdateArg + require.NoError(t, m.Decode(&arg)) + lastHTTP = &arg.Info + httpChanges = append(httpChanges, arg.Info) + } + } + // Each writer announces only a change, so in write order no two of its + // notifications in a row carry the same value. + for i := 1; i < len(appStateChanges); i++ { + require.NotEqual(t, appStateChanges[i-1], appStateChanges[i], "app state notification %d", i) + } + for i := 1; i < len(httpChanges); i++ { + require.NotEqual(t, httpChanges[i-1], httpChanges[i], "http notification %d", i) + } + require.NotNil(t, lastAppState) + require.Equal(t, g.MobileAppState.State(), *lastAppState) + info, err := svc.httpSrv.Info() + require.NoError(t, err) + require.NotNil(t, lastHTTP) + require.Equal(t, info, *lastHTTP) + require.NotEmpty(t, httpChanges, "the http server rebound while connected") +} + +// The identity comes from clientState alone, so a completed login is followed +// by one that carries it. +func TestSessionChangeFollowedBySnapshot(t *testing.T) { + tc := libkb.SetupTest(t, "notify", 0) + defer tc.Cleanup() + g := tc.G + g.SetService() + svc := newTestClientStateService(t, g) + svc.settleInitialLoginAttempt(context.Background()) + + rec := libkb.NewNotifyRecorder(g, allClientStateChannels) + defer rec.Close() + rec.Flush() + before := len(rec.Messages()) + + testLoginWrite(t, tc, "testuser") + g.NotifyRouter.SendLogin(context.Background(), "testuser", false) + rec.Flush() + + after := rec.Messages()[before:] + require.Len(t, after, 2) + require.Equal(t, methodLoggedIn, after[0].Method) + state := decodeClientState(t, after[1]) + require.NotNil(t, state.Session) + require.True(t, state.Session.LoggedIn) + require.Equal(t, "testuser", state.Session.Username) +} + +// Before the startup login attempt settles there is no session to describe -- +// not a logged-out one -- so the clientState says nothing about it, and the +// attempt settling sends one that does. +func TestNullSessionUntilLoginSettles(t *testing.T) { + tc := libkb.SetupTest(t, "notify", 0) + defer tc.Cleanup() + g := tc.G + g.SetService() + svc := newTestClientStateService(t, g) + + rec := libkb.NewNotifyRecorder(g, allClientStateChannels) + defer rec.Close() + rec.Flush() + states := clientStatesOf(t, rec.Messages()) + require.Len(t, states, 1) + require.Nil(t, states[0].Session, "the startup login attempt has not run") + + svc.settleInitialLoginAttempt(context.Background()) + rec.Flush() + states = clientStatesOf(t, rec.Messages()) + require.Len(t, states, 2) + require.NotNil(t, states[1].Session, "the attempt settled, so there is a session to report") + require.False(t, states[1].Session.LoggedIn, "logged out in a fresh test context") +} + +// SetNotifications is what registers the channels, and a connection that +// registers app notifications gets its clientState from it. +func TestSetNotificationsQueuesClientState(t *testing.T) { + tc := libkb.SetupTest(t, "notify", 0) + defer tc.Cleanup() + g := tc.G + g.SetService() + newTestClientStateService(t, g) + + rec := libkb.NewNotifyRecorder(g, keybase1.NotificationChannels{}) + defer rec.Close() + h := NewNotifyCtlHandler(nil, rec.ID, g) + require.NoError(t, h.SetNotifications(context.Background(), keybase1.NotificationChannels{Session: true})) + require.True(t, g.NotifyRouter.GetChannels(rec.ID).Session) + rec.Flush() + require.Empty(t, rec.Messages(), "clientState rides NotifyApp, which this client did not register") + + require.NoError(t, h.SetNotifications(context.Background(), allClientStateChannels)) + rec.Flush() + require.Len(t, clientStatesOf(t, rec.Messages()), 1) +} From 405f055f8e95b0d5e24b3e3a4bffde16b56e085e Mon Sep 17 00:00:00 2001 From: chrisnojima Date: Mon, 21 Sep 2026 12:27:16 -0400 Subject: [PATCH 04/12] refactor(js): take the session and the http address from the clientState The bootstrap status, the loggedIn/loggedOut notifications and the login RPC's own reply were three sources for one fact, with no ordering between them. Take all of it from the clientState instead: it arrives on the service's ordered per-connection stream, so applyClientState applies whatever it carries in arrival order, and the bootstrap status keeps only the slower derived fields. A clientState with no session means the service's startup login attempt has not settled -- not logged out -- so the splash keeps waiting. sessionSettledStep makes that a handshake step, and subscribing moves out of onEngineConnected's fire-and-forget so the step can retry a failed subscribe. An account switch is a logout then a login, so a logged-in clientState for a different uid logs out first; that is what clears the previous account's stores. httpSrv survives the reset, being process-wide. Reachability goes with it: the service's gregor reachability was only ever a proxy for "the network came back", which shell already knows, so onNetworkOnlineChanged re-reads the bootstrap status instead and reachabilityChanged/startReachability leave the enabled calls. --- protocol/bin/enabled-calls.json | 2 - shared/constants/init/app-state.test.ts | 16 +- shared/constants/init/shared.test.ts | 281 ++++++++++++++++++++++- shared/constants/init/shared.tsx | 199 ++++++++++------ shared/constants/rpc/index.tsx | 4 +- shared/constants/rpc/rpc-gen.tsx | 21 +- shared/engine/index.platform.tsx | 3 +- shared/login/loading.tsx | 2 +- shared/stores/config.tsx | 63 +---- shared/stores/daemon.tsx | 13 +- shared/stores/shell.tsx | 14 +- shared/stores/tests/client-state.test.ts | 220 ++++++++++++++++++ shared/stores/tests/config.test.ts | 30 +++ shared/stores/tests/daemon.test.ts | 35 +++ 14 files changed, 749 insertions(+), 154 deletions(-) create mode 100644 shared/stores/tests/client-state.test.ts diff --git a/protocol/bin/enabled-calls.json b/protocol/bin/enabled-calls.json index 725adbda124a..9c97f458cee7 100644 --- a/protocol/bin/enabled-calls.json +++ b/protocol/bin/enabled-calls.json @@ -392,8 +392,6 @@ "keybase.1.provisionUi.chooseGPGMethod": {"custom":true}, "keybase.1.provisionUi.switchToGPGSignOK": {"custom":true}, "keybase.1.reachability.checkReachability": {"promise":true}, - "keybase.1.reachability.reachabilityChanged": {"incoming":true}, - "keybase.1.reachability.startReachability": {"promise":true}, "keybase.1.rekey.getRevokeWarning": {"promise":true}, "keybase.1.rekey.rekeyStatusFinish": {"promise":true}, "keybase.1.rekey.showPendingRekeyStatus": {"promise":true}, diff --git a/shared/constants/init/app-state.test.ts b/shared/constants/init/app-state.test.ts index c8b7f373e485..89127cf7cbe5 100644 --- a/shared/constants/init/app-state.test.ts +++ b/shared/constants/init/app-state.test.ts @@ -2,7 +2,7 @@ import * as T from '@/constants/types' import {resetAllStores} from '@/util/zustand' import {useShellState} from '@/stores/shell' -import {applyMobileAppState, _onEngineIncoming} from './shared' +import {applyClientState, applyMobileAppState, _onEngineIncoming} from './shared' const g = globalThis as unknown as {isMobile: boolean} @@ -43,6 +43,20 @@ describe('the app state the service derives', () => { expect(useShellState.getState().mobileAppState).toBe('active') }) + test('arrives in the clientState, which is what catches a late-started JS up', () => { + _onEngineIncoming({ + payload: {params: {state: {appState: T.RPCGen.MobileAppState.background}}}, + type: 'keybase.1.NotifyApp.clientState', + } as never) + expect(useShellState.getState().mobileAppState).toBe('background') + }) + + test('a notification after the clientState replaces it', () => { + applyClientState({appState: T.RPCGen.MobileAppState.background}) + applyMobileAppState(T.RPCGen.MobileAppState.inactive) + expect(useShellState.getState().mobileAppState).toBe('inactive') + }) + test('a state we do not map leaves the app state alone rather than guessing', () => { applyMobileAppState(T.RPCGen.MobileAppState.background) applyMobileAppState(99 as T.RPCGen.MobileAppState) diff --git a/shared/constants/init/shared.test.ts b/shared/constants/init/shared.test.ts index e09a104c6ff6..f275686779e7 100644 --- a/shared/constants/init/shared.test.ts +++ b/shared/constants/init/shared.test.ts @@ -1,9 +1,19 @@ /// import * as T from '@/constants/types' import {resetAllStores} from '@/util/zustand' +import {ignorePromise} from '@/constants/utils' import {useConfigState} from '@/stores/config' import {useDaemonState} from '@/stores/daemon' -import {loadAccountsStep} from './shared' +import {useRouterState} from '@/stores/router' +import {useShellState} from '@/stores/shell' +import { + applyClientState, + initSharedSubscriptions, + loadAccountsStep, + onEngineConnected, + onNetworkOnlineChanged, + sessionSettledStep, +} from './shared' describe('loadAccountsStep', () => { const originalDispatch = useConfigState.getState().dispatch @@ -33,7 +43,7 @@ describe('loadAccountsStep', () => { withDeferredRefreshAccounts() useConfigState.getState().dispatch.setUserSwitching(true) useDaemonState.setState(s => { - s.bootstrapStatus = {loggedIn: false} as any + s.bootstrapStatus = {loggedIn: false} as never }) await expect(loadAccountsStep()).resolves.toBeUndefined() @@ -42,7 +52,7 @@ describe('loadAccountsStep', () => { test('does not wait for accounts when already logged in', async () => { withDeferredRefreshAccounts() useDaemonState.setState(s => { - s.bootstrapStatus = {loggedIn: true} as any + s.bootstrapStatus = {loggedIn: true} as never }) await expect(loadAccountsStep()).resolves.toBeUndefined() @@ -65,3 +75,268 @@ describe('loadAccountsStep', () => { expect(useConfigState.getState().configuredAccounts.map(a => a.username)).toEqual(['testuser']) }) }) + +describe('onEngineConnected', () => { + const originalConfigDispatch = useConfigState.getState().dispatch + const originalDaemonDispatch = useDaemonState.getState().dispatch + + afterEach(() => { + jest.restoreAllMocks() + useConfigState.setState({dispatch: originalConfigDispatch}) + useDaemonState.setState({dispatch: originalDaemonDispatch}) + resetAllStores() + }) + + const stubRegistrations = () => { + for (const rpc of [ + 'delegateUiCtlRegisterChatUIRpcPromise', + 'delegateUiCtlRegisterLogUIRpcPromise', + 'delegateUiCtlRegisterHomeUIRpcPromise', + 'delegateUiCtlRegisterSecretUIRpcPromise', + 'delegateUiCtlRegisterIdentify3UIRpcPromise', + 'delegateUiCtlRegisterRekeyUIRpcPromise', + ] as const) { + jest.spyOn(T.RPCGen, rpc).mockResolvedValue(undefined) + } + useConfigState.setState(s => { + s.dispatch = {...originalConfigDispatch, onEngineConnected: () => {}} + }) + } + + const deferredSubscription = () => { + let subscribed!: () => void + jest.spyOn(T.RPCGen, 'notifyCtlSetNotificationsRpcPromise').mockReturnValue( + new Promise(resolve => { + subscribed = resolve + }) + ) + return subscribed + } + const spyOnBootstrap = () => + jest.spyOn(T.RPCGen, 'configGetBootstrapStatusRpcPromise').mockResolvedValue({ + loggedIn: true, + } as T.RPCGen.BootstrapStatus) + + test('a reconnect clears the disconnect state at once, before the subscription resolves', () => { + stubRegistrations() + useDaemonState.setState({error: new Error('Disconnected'), handshakeState: 'failed'}) + deferredSubscription() + spyOnBootstrap() + + onEngineConnected() + + expect(useDaemonState.getState().error).toBe(undefined) + expect(useDaemonState.getState().handshakeState).toBe('loading') + }) + + test('the bootstrap read does not wait for the subscription', async () => { + stubRegistrations() + deferredSubscription() + const bootstrap = spyOnBootstrap() + + onEngineConnected() + await new Promise(resolve => setImmediate(resolve)) + + expect(bootstrap).toHaveBeenCalledTimes(1) + }) + + test('the bootstrap read still runs when the subscription fails', async () => { + stubRegistrations() + jest + .spyOn(T.RPCGen, 'notifyCtlSetNotificationsRpcPromise') + .mockRejectedValue(new Error('no notifications')) + const bootstrap = spyOnBootstrap() + + onEngineConnected() + await new Promise(resolve => setImmediate(resolve)) + + expect(bootstrap).toHaveBeenCalledTimes(1) + }) + + test('the bootstrap status is not a session source', async () => { + stubRegistrations() + jest.spyOn(T.RPCGen, 'notifyCtlSetNotificationsRpcPromise').mockResolvedValue(undefined) + jest.spyOn(T.RPCGen, 'configGetBootstrapStatusRpcPromise').mockResolvedValue({ + httpSrvInfo: {address: '127.0.0.1:1', token: 'token'}, + loggedIn: true, + uid: 'u1', + username: 'testuser', + } as never) + + onEngineConnected() + await new Promise(resolve => setImmediate(resolve)) + + expect(useDaemonState.getState().bootstrapStatus?.loggedIn).toBe(true) + expect(useConfigState.getState().loggedIn).toBe(false) + expect(useConfigState.getState().httpSrv.address).toBe('') + }) +}) + +describe('sessionSettledStep', () => { + const originalConfigDispatch = useConfigState.getState().dispatch + + afterEach(() => { + jest.useRealTimers() + jest.restoreAllMocks() + useConfigState.setState({dispatch: originalConfigDispatch}) + resetAllStores() + }) + + const connect = (subscribe: () => Promise) => { + for (const rpc of [ + 'delegateUiCtlRegisterChatUIRpcPromise', + 'delegateUiCtlRegisterLogUIRpcPromise', + 'delegateUiCtlRegisterHomeUIRpcPromise', + 'delegateUiCtlRegisterSecretUIRpcPromise', + 'delegateUiCtlRegisterIdentify3UIRpcPromise', + 'delegateUiCtlRegisterRekeyUIRpcPromise', + ] as const) { + jest.spyOn(T.RPCGen, rpc).mockResolvedValue(undefined) + } + useConfigState.setState(s => { + s.dispatch = {...originalConfigDispatch, onEngineConnected: () => {}} + }) + jest.spyOn(T.RPCGen, 'configGetBootstrapStatusRpcPromise').mockReturnValue(new Promise(() => {})) + const setNotifications = jest.spyOn(T.RPCGen, 'notifyCtlSetNotificationsRpcPromise').mockImplementation(subscribe) + onEngineConnected() + return setNotifications + } + const session = {deviceID: 'd1', deviceName: 'testuser-mac', loggedIn: false, uid: '', username: ''} + const settled = async (p: Promise) => { + let done = false + const watch = async () => { + try { + await p + } catch {} + done = true + } + ignorePromise(watch()) + await new Promise(resolve => setImmediate(resolve)) + return done + } + + test('waits for a clientState that carries a session, which may say logged out', async () => { + connect(async () => Promise.resolve()) + const step = sessionSettledStep() + + applyClientState({appState: T.RPCGen.MobileAppState.foreground}) + expect(await settled(step)).toBe(false) + + applyClientState({appState: T.RPCGen.MobileAppState.foreground, session}) + expect(await settled(step)).toBe(true) + await expect(step).resolves.toBeUndefined() + }) + + test('a new connection waits afresh', async () => { + connect(async () => Promise.resolve()) + applyClientState({appState: T.RPCGen.MobileAppState.foreground, session}) + await sessionSettledStep() + + connect(async () => Promise.resolve()) + const step = sessionSettledStep() + expect(await settled(step)).toBe(false) + applyClientState({appState: T.RPCGen.MobileAppState.foreground, session}) + await expect(step).resolves.toBeUndefined() + }) + + test('re-subscribes when the subscription failed, since no clientState is coming otherwise', async () => { + let calls = 0 + const setNotifications = connect(async () => { + calls++ + return calls === 1 ? Promise.reject(new Error('no notifications')) : Promise.resolve() + }) + await new Promise(resolve => setImmediate(resolve)) + + const step = sessionSettledStep() + await new Promise(resolve => setImmediate(resolve)) + expect(setNotifications).toHaveBeenCalledTimes(2) + + applyClientState({appState: T.RPCGen.MobileAppState.foreground, session}) + await expect(step).resolves.toBeUndefined() + }) + + test('is one of the handshake steps', () => { + // Nothing here tears the subscriptions down, so none may outlive the test. + for (const store of [useConfigState, useShellState, useRouterState]) { + jest.spyOn(store, 'subscribe').mockReturnValue(() => {}) + } + const originalDaemonDispatch = useDaemonState.getState().dispatch + let steps: ReadonlyArray = [] + useDaemonState.setState({ + dispatch: { + ...originalDaemonDispatch, + initBootstrapSteps: s => { + steps = s + }, + }, + }) + try { + initSharedSubscriptions() + } finally { + useDaemonState.setState({dispatch: originalDaemonDispatch}) + } + expect(steps).toContain(sessionSettledStep) + }) + + test('fails the handshake attempt when the session never comes', async () => { + jest.useFakeTimers() + connect(async () => Promise.resolve()) + const step = sessionSettledStep() + const failed = expect(step).rejects.toThrow("The service hasn't said who is logged in") + await jest.advanceTimersByTimeAsync(30_000) + await failed + }) +}) + +describe('onNetworkOnlineChanged', () => { + // replaces the gregor-reachability trigger: re-read the bootstrap status after an offline stretch + afterEach(() => { + jest.restoreAllMocks() + useDaemonState.setState({dispatch: originalDaemonDispatch}) + resetAllStores() + }) + + const originalDaemonDispatch = useDaemonState.getState().dispatch + const spyOnReRead = () => { + // userSwitching survives resetAllStores on purpose, and an earlier test in this file sets it + useConfigState.getState().dispatch.setUserSwitching(false) + const reRead = jest.fn(async () => {}) + useDaemonState.setState({ + dispatch: {...originalDaemonDispatch, loadDaemonBootstrapStatus: reRead}, + handshakeState: 'done', + }) + return reRead + } + + test('re-reads the bootstrap status when the network comes back', () => { + const reRead = spyOnReRead() + onNetworkOnlineChanged(true, false) + expect(reRead).toHaveBeenCalledTimes(1) + }) + + test('does not re-read on the first reading of the network at startup', () => { + const reRead = spyOnReRead() + onNetworkOnlineChanged(true, undefined) + expect(reRead).not.toHaveBeenCalled() + }) + + test('does not re-read when going offline', () => { + const reRead = spyOnReRead() + onNetworkOnlineChanged(false, true) + expect(reRead).not.toHaveBeenCalled() + }) + + test('does not re-read during an account switch', () => { + const reRead = spyOnReRead() + useConfigState.getState().dispatch.setUserSwitching(true) + onNetworkOnlineChanged(true, false) + expect(reRead).not.toHaveBeenCalled() + }) + + test('does not re-read before the handshake is done', () => { + const reRead = spyOnReRead() + useDaemonState.setState({handshakeState: 'loading'}) + onNetworkOnlineChanged(true, false) + expect(reRead).not.toHaveBeenCalled() + }) +}) diff --git a/shared/constants/init/shared.tsx b/shared/constants/init/shared.tsx index f754f7421dfc..c4d98f069797 100644 --- a/shared/constants/init/shared.tsx +++ b/shared/constants/init/shared.tsx @@ -70,7 +70,6 @@ const subscribeValue = ( }) type ConfigState = ReturnType -type DaemonState = ReturnType type RouterState = ReturnType // ─── Bootstrap steps ────────────────────────────────────────────────────────── @@ -175,10 +174,14 @@ const onGregorPushStateChanged = ( ) } -const onGregorReachableChanged = (gregorReachable: ConfigState['gregorReachable']) => { - // Re-get info about our account if you log in/we're done handshaking/became reachable +// The bootstrap read the old gregor-reachability trigger did: after an offline stretch, pick up +// what the service learned while we could not reach it. `previous === undefined` is the first +// reading of the network at startup, which the handshake's own read already covers. +export const onNetworkOnlineChanged = (online?: boolean, previous?: boolean) => { + if (!online || previous !== false) { + return + } if ( - gregorReachable === T.RPCGen.Reachable.yes && useDaemonState.getState().handshakeState === 'done' && !useConfigState.getState().userSwitching ) { @@ -186,7 +189,7 @@ const onGregorReachableChanged = (gregorReachable: ConfigState['gregorReachable' } } -const onLoggedInChanged = (loggedIn: ConfigState['loggedIn']) => { +export const onLoggedInChanged = (loggedIn: ConfigState['loggedIn']) => { if (loggedIn) { // runtime login: refresh bootstrap status. During the handshake this is already in // flight, and the store dedupes it. @@ -214,28 +217,6 @@ const onConfiguredAccountsChanged = (configuredAccounts: ConfigState['configured } } -const onBootstrapStatusChanged = (bootstrap: DaemonState['bootstrapStatus']) => { - if (!bootstrap) { - return - } - - const {deviceID, deviceName, loggedIn, uid, username} = bootstrap - useCurrentUserState.getState().dispatch.setBootstrap({deviceID, deviceName, uid, username}) - - const configDispatch = useConfigState.getState().dispatch - if (username) { - configDispatch.setDefaultUsername(username) - } - if (!loggedIn && useConfigState.getState().userSwitching) { - logger.info('[Bootstrap] ignoring loggedIn=false result during account switch') - return - } - configDispatch.setLoggedIn(loggedIn) - - if (bootstrap.httpSrvInfo) { - configDispatch.setHTTPSrvInfo(bootstrap.httpSrvInfo.address, bootstrap.httpSrvInfo.token) - } -} // The service derives the app's lifecycle state from the UI reports native makes and is the only // party that derives it; this is the whole of JS's model of it. Go's two background states are one @@ -267,6 +248,103 @@ export const applyMobileAppState = (state: T.RPCGen.MobileAppState) => { } } +// The splash waits for the service to say who is logged in. A clientState with no session means +// its startup login attempt has not settled yet -- not known, rather than logged out -- and the +// attempt settling sends another that has one. Each connection waits afresh. +const sessionWaitMs = 30_000 +let settleSession = () => {} +let sessionSettled = new Promise(resolve => { + settleSession = resolve +}) +const awaitSessionAgain = () => { + sessionSettled = new Promise(resolve => { + settleSession = resolve + }) +} + +// The service's clientState: the session, the http server address and the app state, read when it +// was sent. It rides the same ordered stream as every notification that changes them, and for each +// of them the last message to arrive carries the latest value, so everything is applied in arrival +// order. It comes first on subscribing, after every session change, and once the service's startup +// login attempt settles. +export const applyClientState = (clientState: T.RPCGen.ClientState) => { + const {appState, httpSrvInfo, session} = clientState + // On iOS JS never starts on a background launch, so it can have missed every change since the + // process started: this is what catches it up. + applyMobileAppState(appState) + const configDispatch = useConfigState.getState().dispatch + if (httpSrvInfo) { + configDispatch.setHTTPSrvInfo(httpSrvInfo.address, httpSrvInfo.token) + } + if (!session) { + logger.info('[Bootstrap] the service has not settled its startup login yet') + return + } + settleSession() + const {deviceID, deviceName, loggedIn, uid, username} = session + if (!loggedIn) { + // Session first: logging out resets the stores, the current user among them. Writing the empty + // identity first would leave a moment where we are logged in with no user. + configDispatch.setLoggedIn(false) + return + } + // A logged-in clientState for another user than the one we are logged in as is a logout and then + // a login, however it reached us -- with or without a logged-out clientState before it. Logging + // out is what clears the previous account's stores. Logged in with no current user is no switch. + const currentUid = useCurrentUserState.getState().uid + if (useConfigState.getState().loggedIn && currentUid && uid !== currentUid) { + configDispatch.setLoggedIn(false) + } + // identity before the session: setLoggedIn fans out synchronously, and every subscriber of a + // login has always been able to read the current user by the time it runs + useCurrentUserState.getState().dispatch.setBootstrap({deviceID, deviceName, uid, username}) + if (username) { + configDispatch.setDefaultUsername(username) + } + configDispatch.setLoggedIn(true) +} + +const subscribe = async () => { + try { + // prettier-ignore + await T.RPCGen.notifyCtlSetNotificationsRpcPromise({ + channels: { + allowChatNotifySkips: true, app: true, audit: true, badges: true, chat: true, chatarchive: true, + chatattachments: true, chatdev: false, chatemoji: false, chatemojicross: false, chatkbfsedits: false, + deviceclone: false, ephemeral: false, favorites: false, featuredBots: false, kbfs: true, kbfsdesktop: !isMobile, + devicehistory: true, kbfslegacy: false, kbfsrequest: false, kbfssubscription: true, keyfamily: false, notifysimplefs: true, + paperkeys: false, pgp: true, reachability: false, runtimestats: true, saltpack: true, service: true, session: true, + team: true, teambot: false, tracking: true, users: true, wallet: false, + }, + }) + return true + } catch (error) { + logger.warn('error in toggling notifications: ', error) + return false + } +} +let subscription = Promise.resolve(false) + +// A handshake step: the session is what decides between the login screen and the app. A failed +// subscribe is retried here, since without it no clientState is coming. +export const sessionSettledStep = async () => { + if (!(await subscription)) { + subscription = subscribe() + if (!(await subscription)) { + throw new Error("Can't subscribe to the service's notifications") + } + } + let timer: ReturnType | undefined + const timedOut = new Promise((_resolve, reject) => { + timer = setTimeout(() => reject(new Error("The service hasn't said who is logged in")), sessionWaitMs) + }) + try { + await Promise.race([sessionSettled, timedOut]) + } finally { + clearTimeout(timer) + } +} + const onNavStateChanged =(nextNavState: RouterState['navState'], previousNavState: RouterState['navState']) => { const next = nextNavState as Util.NavState const prev = previousNavState as Util.NavState @@ -292,50 +370,30 @@ const onNavStateChanged =(nextNavState: RouterState['navState'], previousNavStat } export const onEngineConnected = () => { - { - const registerUIs = async () => { - try { - await T.RPCGen.delegateUiCtlRegisterChatUIRpcPromise() - await T.RPCGen.delegateUiCtlRegisterLogUIRpcPromise() - logger.info('Registered Chat UI') - await T.RPCGen.delegateUiCtlRegisterHomeUIRpcPromise() - logger.info('Registered home UI') - await T.RPCGen.delegateUiCtlRegisterSecretUIRpcPromise() - logger.info('Registered secret ui') - await T.RPCGen.delegateUiCtlRegisterIdentify3UIRpcPromise() - logger.info('Registered identify ui') - await T.RPCGen.delegateUiCtlRegisterRekeyUIRpcPromise() - logger.info('Registered rekey ui') - } catch (error) { - logger.error('Error in registering UIs:', error) - } + const registerUIs = async () => { + try { + await T.RPCGen.delegateUiCtlRegisterChatUIRpcPromise() + await T.RPCGen.delegateUiCtlRegisterLogUIRpcPromise() + logger.info('Registered Chat UI') + await T.RPCGen.delegateUiCtlRegisterHomeUIRpcPromise() + logger.info('Registered home UI') + await T.RPCGen.delegateUiCtlRegisterSecretUIRpcPromise() + logger.info('Registered secret ui') + await T.RPCGen.delegateUiCtlRegisterIdentify3UIRpcPromise() + logger.info('Registered identify ui') + await T.RPCGen.delegateUiCtlRegisterRekeyUIRpcPromise() + logger.info('Registered rekey ui') + } catch (error) { + logger.error('Error in registering UIs:', error) } - ignorePromise(registerUIs()) } + ignorePromise(registerUIs()) + useConfigState.getState().dispatch.onEngineConnected() + + awaitSessionAgain() + subscription = subscribe() useDaemonState.getState().dispatch.startHandshake() - { - const notifyCtl = async () => { - try { - // prettier-ignore - await T.RPCGen.notifyCtlSetNotificationsRpcPromise({ - channels: { - allowChatNotifySkips: true, app: true, audit: true, badges: true, chat: true, chatarchive: true, - chatattachments: true, chatdev: false, chatemoji: false, chatemojicross: false, chatkbfsedits: false, - deviceclone: false, ephemeral: false, favorites: false, featuredBots: false, kbfs: true, kbfsdesktop: !isMobile, - devicehistory: true, kbfslegacy: false, kbfsrequest: false, kbfssubscription: true, keyfamily: false, notifysimplefs: true, - paperkeys: false, pgp: true, reachability: true, runtimestats: true, saltpack: true, service: true, session: true, - team: true, teambot: false, tracking: true, users: true, wallet: false, - }, - }) - } catch (error) { - if (error) { - logger.warn('error in toggling notifications: ', error) - } - } - } - ignorePromise(notifyCtl()) - } } export const onEngineDisconnected = () => { @@ -353,6 +411,7 @@ export const initSharedSubscriptions = (platformBootstrapSteps: Array s.gregorReachable, onGregorReachableChanged), subscribeValue(useConfigState, s => s.gregorPushState, onGregorPushStateChanged), subscribeValue(useConfigState, s => s.loggedIn, onLoggedInChanged), subscribeValue(useConfigState, s => s.revokedTrigger, onRevokedTriggerChanged), subscribeValue(useConfigState, s => s.configuredAccounts, onConfiguredAccountsChanged) ) - _sharedUnsubs.push(subscribeValue(useDaemonState, s => s.bootstrapStatus, onBootstrapStatusChanged)) + _sharedUnsubs.push(subscribeValue(useShellState, s => s.networkStatus?.online, onNetworkOnlineChanged)) _sharedUnsubs.push( subscribeValue(useRouterState, s => s.navState, onNavStateChanged) @@ -390,6 +448,9 @@ export const _onEngineIncoming = (action: EngineGen.Actions) => { case 'keybase.1.NotifyApp.mobileAppStateChanged': applyMobileAppState(action.payload.params.state) break + case 'keybase.1.NotifyApp.clientState': + applyClientState(action.payload.params.state) + break case 'keybase.1.NotifyBadges.badgeState': { const {badgeState} = action.payload.params diff --git a/shared/constants/rpc/index.tsx b/shared/constants/rpc/index.tsx index 14806b2581c8..03b90b4c3aaa 100644 --- a/shared/constants/rpc/index.tsx +++ b/shared/constants/rpc/index.tsx @@ -71,6 +71,7 @@ type Chat1ResponseActionMap = { } type Keybase1IncomingAction = + 'keybase.1.NotifyApp.clientState' | 'keybase.1.NotifyApp.mobileAppStateChanged' | 'keybase.1.NotifyAudit.boxAuditError' | 'keybase.1.NotifyAudit.rootAuditError' | @@ -79,8 +80,7 @@ type Keybase1IncomingAction = 'keybase.1.NotifyFS.FSActivity' | 'keybase.1.NotifySession.loggedOut' | 'keybase.1.NotifyTracking.trackingChanged' | - 'keybase.1.NotifyUsers.userChanged' | - 'keybase.1.reachability.reachabilityChanged' + 'keybase.1.NotifyUsers.userChanged' type Keybase1IncomingActionMap = { [P in K]: {readonly params: keybase1Types.RpcIn

} diff --git a/shared/constants/rpc/rpc-gen.tsx b/shared/constants/rpc/rpc-gen.tsx index e49a4dc17ece..5e5add3dc6eb 100644 --- a/shared/constants/rpc/rpc-gen.tsx +++ b/shared/constants/rpc/rpc-gen.tsx @@ -11,6 +11,10 @@ export type IncomingErrorCallback = (err?: SimpleError | null) => void export type MessageTypes = { + 'keybase.1.NotifyApp.clientState': { + inParam: {readonly state: ClientState}, + outParam: void, + }, 'keybase.1.NotifyApp.exit': { inParam: undefined, outParam: void, @@ -967,14 +971,6 @@ export type MessageTypes = { inParam: undefined, outParam: Reachability, }, - 'keybase.1.reachability.reachabilityChanged': { - inParam: {readonly reachability: Reachability}, - outParam: void, - }, - 'keybase.1.reachability.startReachability': { - inParam: undefined, - outParam: Reachability, - }, 'keybase.1.rekey.getRevokeWarning': { inParam: {readonly actingDevice: DeviceID,readonly targetDevice: DeviceID}, outParam: RevokeWarning, @@ -1284,7 +1280,7 @@ export type MessageKey = keyof MessageTypes export type RpcIn = MessageTypes[M]['inParam'] export type RpcOut = MessageTypes[M]['outParam'] export type RpcResponse = {error: IncomingErrorCallback, result: (res: RpcOut) => void} -type PromiseMethod = 'keybase.1.account.cancelReset' | 'keybase.1.account.getLockdownMode' | 'keybase.1.account.guessCurrentLocation' | 'keybase.1.account.hasServerKeys' | 'keybase.1.account.passphraseChange' | 'keybase.1.account.passphraseCheck' | 'keybase.1.account.recoverUsernameWithEmail' | 'keybase.1.account.recoverUsernameWithPhone' | 'keybase.1.account.setLockdownMode' | 'keybase.1.account.userGetContactSettings' | 'keybase.1.account.userSetContactSettings' | 'keybase.1.apiserver.Delete' | 'keybase.1.apiserver.GetWithSession' | 'keybase.1.apiserver.Post' | 'keybase.1.apiserver.PostJSON' | 'keybase.1.appState.powerMonitorEvent' | 'keybase.1.appState.updateMobileNetState' | 'keybase.1.config.appendGUILogs' | 'keybase.1.config.generateWebAuthToken' | 'keybase.1.config.getBootstrapStatus' | 'keybase.1.config.getProxyData' | 'keybase.1.config.getRememberPassphrase' | 'keybase.1.config.getUpdateInfo' | 'keybase.1.config.getUpdateInfo2' | 'keybase.1.config.guiGetValue' | 'keybase.1.config.guiSetValue' | 'keybase.1.config.helloIAm' | 'keybase.1.config.logSend' | 'keybase.1.config.requestFollowingAndUnverifiedFollowers' | 'keybase.1.config.setProxyData' | 'keybase.1.config.setRememberPassphrase' | 'keybase.1.config.startUpdateIfNeeded' | 'keybase.1.config.toggleRuntimeStats' | 'keybase.1.config.updateLastLoggedInAndServerConfig' | 'keybase.1.config.waitForClient' | 'keybase.1.contacts.getContactsForUserRecommendations' | 'keybase.1.contacts.saveContactList' | 'keybase.1.cryptocurrency.registerAddress' | 'keybase.1.ctl.dbNuke' | 'keybase.1.ctl.getOnLoginStartup' | 'keybase.1.ctl.setOnLoginStartup' | 'keybase.1.ctl.stop' | 'keybase.1.delegateUiCtl.registerChatUI' | 'keybase.1.delegateUiCtl.registerGregorFirehoseFiltered' | 'keybase.1.delegateUiCtl.registerHomeUI' | 'keybase.1.delegateUiCtl.registerIdentify3UI' | 'keybase.1.delegateUiCtl.registerLogUI' | 'keybase.1.delegateUiCtl.registerRekeyUI' | 'keybase.1.delegateUiCtl.registerSecretUI' | 'keybase.1.device.checkDeviceNameFormat' | 'keybase.1.device.deviceHistoryList' | 'keybase.1.device.dismissDeviceChangeNotifications' | 'keybase.1.emails.addEmail' | 'keybase.1.emails.deleteEmail' | 'keybase.1.emails.sendVerificationEmail' | 'keybase.1.emails.setPrimaryEmail' | 'keybase.1.emails.setVisibilityEmail' | 'keybase.1.favorite.favoriteIgnore' | 'keybase.1.featuredBot.featuredBots' | 'keybase.1.featuredBot.search' | 'keybase.1.git.createPersonalRepo' | 'keybase.1.git.createTeamRepo' | 'keybase.1.git.deletePersonalRepo' | 'keybase.1.git.deleteTeamRepo' | 'keybase.1.git.getAllGitMetadata' | 'keybase.1.git.getTeamRepoSettings' | 'keybase.1.git.setTeamRepoSettings' | 'keybase.1.gregor.dismissCategory' | 'keybase.1.gregor.getState' | 'keybase.1.gregor.updateCategory' | 'keybase.1.home.homeDismissAnnouncement' | 'keybase.1.home.homeGetScreen' | 'keybase.1.home.homeMarkViewed' | 'keybase.1.home.homeSkipTodoType' | 'keybase.1.identify3.identify3FollowUser' | 'keybase.1.identify3.identify3IgnoreUser' | 'keybase.1.incomingShare.getIncomingShareItems' | 'keybase.1.incomingShare.getPreference' | 'keybase.1.incomingShare.setPreference' | 'keybase.1.install.fuseStatus' | 'keybase.1.install.installFuse' | 'keybase.1.install.installKBFS' | 'keybase.1.install.uninstallKBFS' | 'keybase.1.kbfsMount.GetCurrentMountDir' | 'keybase.1.kbfsMount.GetKBFSPathInfo' | 'keybase.1.kbfsMount.GetPreferredMountDirs' | 'keybase.1.kbfsMount.WaitForMounts' | 'keybase.1.log.perfLogPoint' | 'keybase.1.login.accountDelete' | 'keybase.1.login.deprovision' | 'keybase.1.login.getConfiguredAccounts' | 'keybase.1.login.isOnline' | 'keybase.1.login.logout' | 'keybase.1.login.paperKeySubmit' | 'keybase.1.notifyCtl.setNotifications' | 'keybase.1.pgp.pgpStorageDismiss' | 'keybase.1.phoneNumbers.addPhoneNumber' | 'keybase.1.phoneNumbers.deletePhoneNumber' | 'keybase.1.phoneNumbers.resendVerificationForPhoneNumber' | 'keybase.1.phoneNumbers.setVisibilityPhoneNumber' | 'keybase.1.phoneNumbers.verifyPhoneNumber' | 'keybase.1.pprof.logProcessorProfile' | 'keybase.1.pprof.logTrace' | 'keybase.1.prove.checkProof' | 'keybase.1.reachability.checkReachability' | 'keybase.1.reachability.startReachability' | 'keybase.1.rekey.getRevokeWarning' | 'keybase.1.rekey.rekeyStatusFinish' | 'keybase.1.rekey.showPendingRekeyStatus' | 'keybase.1.revoke.revokeDevice' | 'keybase.1.revoke.revokeKey' | 'keybase.1.revoke.revokeSigs' | 'keybase.1.saltpack.saltpackDecryptFile' | 'keybase.1.saltpack.saltpackDecryptString' | 'keybase.1.saltpack.saltpackEncryptFile' | 'keybase.1.saltpack.saltpackEncryptString' | 'keybase.1.saltpack.saltpackSaveCiphertextToFile' | 'keybase.1.saltpack.saltpackSaveSignedMsgToFile' | 'keybase.1.saltpack.saltpackSignFile' | 'keybase.1.saltpack.saltpackSignString' | 'keybase.1.saltpack.saltpackVerifyFile' | 'keybase.1.saltpack.saltpackVerifyString' | 'keybase.1.signup.checkUsernameAvailable' | 'keybase.1.signup.getInvitationCode' | 'keybase.1.SimpleFS.simpleFSArchiveAllFiles' | 'keybase.1.SimpleFS.simpleFSArchiveAllGitRepos' | 'keybase.1.SimpleFS.simpleFSArchiveCancelOrDismissJob' | 'keybase.1.SimpleFS.simpleFSArchiveStart' | 'keybase.1.SimpleFS.simpleFSCancelDownload' | 'keybase.1.SimpleFS.simpleFSCheckReachability' | 'keybase.1.SimpleFS.simpleFSClearConflictState' | 'keybase.1.SimpleFS.simpleFSConfigureDownload' | 'keybase.1.SimpleFS.simpleFSCopyRecursive' | 'keybase.1.SimpleFS.simpleFSDismissDownload' | 'keybase.1.SimpleFS.simpleFSDismissUpload' | 'keybase.1.SimpleFS.simpleFSFinishResolvingConflict' | 'keybase.1.SimpleFS.simpleFSFolderSyncConfigAndStatus' | 'keybase.1.SimpleFS.simpleFSGetArchiveJobFreshness' | 'keybase.1.SimpleFS.simpleFSGetArchiveStatus' | 'keybase.1.SimpleFS.simpleFSGetDownloadInfo' | 'keybase.1.SimpleFS.simpleFSGetDownloadStatus' | 'keybase.1.SimpleFS.simpleFSGetFilesTabBadge' | 'keybase.1.SimpleFS.simpleFSGetFolder' | 'keybase.1.SimpleFS.simpleFSGetGUIFileContext' | 'keybase.1.SimpleFS.simpleFSGetOnlineStatus' | 'keybase.1.SimpleFS.simpleFSGetUploadStatus' | 'keybase.1.SimpleFS.simpleFSList' | 'keybase.1.SimpleFS.simpleFSListFavorites' | 'keybase.1.SimpleFS.simpleFSListRecursiveToDepth' | 'keybase.1.SimpleFS.simpleFSMakeTempDirForUpload' | 'keybase.1.SimpleFS.simpleFSMove' | 'keybase.1.SimpleFS.simpleFSOpen' | 'keybase.1.SimpleFS.simpleFSReadList' | 'keybase.1.SimpleFS.simpleFSRemove' | 'keybase.1.SimpleFS.simpleFSSetDebugLevel' | 'keybase.1.SimpleFS.simpleFSSetFolderSyncConfig' | 'keybase.1.SimpleFS.simpleFSSetNotificationThreshold' | 'keybase.1.SimpleFS.simpleFSSetSfmiBannerDismissed' | 'keybase.1.SimpleFS.simpleFSSetSyncOnCellular' | 'keybase.1.SimpleFS.simpleFSSettings' | 'keybase.1.SimpleFS.simpleFSStartDownload' | 'keybase.1.SimpleFS.simpleFSStartUpload' | 'keybase.1.SimpleFS.simpleFSStat' | 'keybase.1.SimpleFS.simpleFSSubscribeNonPath' | 'keybase.1.SimpleFS.simpleFSSubscribePath' | 'keybase.1.SimpleFS.simpleFSSyncStatus' | 'keybase.1.SimpleFS.simpleFSUnsubscribe' | 'keybase.1.SimpleFS.simpleFSUserEditHistory' | 'keybase.1.SimpleFS.simpleFSUserIn' | 'keybase.1.SimpleFS.simpleFSUserOut' | 'keybase.1.SimpleFS.simpleFSWait' | 'keybase.1.teams.findAssertionsInTeamNoResolve' | 'keybase.1.teams.getAnnotatedTeam' | 'keybase.1.teams.getInviteLinkDetails' | 'keybase.1.teams.getTeamID' | 'keybase.1.teams.getTeamRoleMap' | 'keybase.1.teams.getUntrustedTeamInfo' | 'keybase.1.teams.loadTeamTreeMembershipsAsync' | 'keybase.1.teams.setTarsDisabled' | 'keybase.1.teams.setTeamMemberShowcase' | 'keybase.1.teams.setTeamShowcase' | 'keybase.1.teams.teamAddEmailsBulk' | 'keybase.1.teams.teamAddMember' | 'keybase.1.teams.teamAddMembersMultiRole' | 'keybase.1.teams.teamCreate' | 'keybase.1.teams.teamCreateFancy' | 'keybase.1.teams.teamCreateSeitanTokenV2' | 'keybase.1.teams.teamEditMembers' | 'keybase.1.teams.teamGetMembersByID' | 'keybase.1.teams.teamIgnoreRequest' | 'keybase.1.teams.teamLeave' | 'keybase.1.teams.teamListMyAccessRequests' | 'keybase.1.teams.teamListUnverified' | 'keybase.1.teams.teamProfileAddList' | 'keybase.1.teams.teamReAddMemberAfterReset' | 'keybase.1.teams.teamRemoveMember' | 'keybase.1.teams.teamRename' | 'keybase.1.teams.teamSetSettings' | 'keybase.1.teams.untrustedTeamExists' | 'keybase.1.teams.uploadTeamAvatar' | 'keybase.1.user.blockUser' | 'keybase.1.user.canLogout' | 'keybase.1.user.dismissBlockButtons' | 'keybase.1.user.getUserBlocks' | 'keybase.1.user.interestingPeople' | 'keybase.1.user.listTrackersUnverified' | 'keybase.1.user.listTracking' | 'keybase.1.user.loadMySettings' | 'keybase.1.user.loadPassphraseState' | 'keybase.1.user.profileEdit' | 'keybase.1.user.proofSuggestions' | 'keybase.1.user.reportUser' | 'keybase.1.user.setUserBlocks' | 'keybase.1.user.unblockUser' | 'keybase.1.user.uploadUserAvatar' | 'keybase.1.user.userCard' | 'keybase.1.userSearch.bulkEmailOrPhoneSearch' | 'keybase.1.userSearch.getNonUserDetails' | 'keybase.1.userSearch.userSearch' +type PromiseMethod = 'keybase.1.account.cancelReset' | 'keybase.1.account.getLockdownMode' | 'keybase.1.account.guessCurrentLocation' | 'keybase.1.account.hasServerKeys' | 'keybase.1.account.passphraseChange' | 'keybase.1.account.passphraseCheck' | 'keybase.1.account.recoverUsernameWithEmail' | 'keybase.1.account.recoverUsernameWithPhone' | 'keybase.1.account.setLockdownMode' | 'keybase.1.account.userGetContactSettings' | 'keybase.1.account.userSetContactSettings' | 'keybase.1.apiserver.Delete' | 'keybase.1.apiserver.GetWithSession' | 'keybase.1.apiserver.Post' | 'keybase.1.apiserver.PostJSON' | 'keybase.1.appState.powerMonitorEvent' | 'keybase.1.appState.updateMobileNetState' | 'keybase.1.config.appendGUILogs' | 'keybase.1.config.generateWebAuthToken' | 'keybase.1.config.getBootstrapStatus' | 'keybase.1.config.getProxyData' | 'keybase.1.config.getRememberPassphrase' | 'keybase.1.config.getUpdateInfo' | 'keybase.1.config.getUpdateInfo2' | 'keybase.1.config.guiGetValue' | 'keybase.1.config.guiSetValue' | 'keybase.1.config.helloIAm' | 'keybase.1.config.logSend' | 'keybase.1.config.requestFollowingAndUnverifiedFollowers' | 'keybase.1.config.setProxyData' | 'keybase.1.config.setRememberPassphrase' | 'keybase.1.config.startUpdateIfNeeded' | 'keybase.1.config.toggleRuntimeStats' | 'keybase.1.config.updateLastLoggedInAndServerConfig' | 'keybase.1.config.waitForClient' | 'keybase.1.contacts.getContactsForUserRecommendations' | 'keybase.1.contacts.saveContactList' | 'keybase.1.cryptocurrency.registerAddress' | 'keybase.1.ctl.dbNuke' | 'keybase.1.ctl.getOnLoginStartup' | 'keybase.1.ctl.setOnLoginStartup' | 'keybase.1.ctl.stop' | 'keybase.1.delegateUiCtl.registerChatUI' | 'keybase.1.delegateUiCtl.registerGregorFirehoseFiltered' | 'keybase.1.delegateUiCtl.registerHomeUI' | 'keybase.1.delegateUiCtl.registerIdentify3UI' | 'keybase.1.delegateUiCtl.registerLogUI' | 'keybase.1.delegateUiCtl.registerRekeyUI' | 'keybase.1.delegateUiCtl.registerSecretUI' | 'keybase.1.device.checkDeviceNameFormat' | 'keybase.1.device.deviceHistoryList' | 'keybase.1.device.dismissDeviceChangeNotifications' | 'keybase.1.emails.addEmail' | 'keybase.1.emails.deleteEmail' | 'keybase.1.emails.sendVerificationEmail' | 'keybase.1.emails.setPrimaryEmail' | 'keybase.1.emails.setVisibilityEmail' | 'keybase.1.favorite.favoriteIgnore' | 'keybase.1.featuredBot.featuredBots' | 'keybase.1.featuredBot.search' | 'keybase.1.git.createPersonalRepo' | 'keybase.1.git.createTeamRepo' | 'keybase.1.git.deletePersonalRepo' | 'keybase.1.git.deleteTeamRepo' | 'keybase.1.git.getAllGitMetadata' | 'keybase.1.git.getTeamRepoSettings' | 'keybase.1.git.setTeamRepoSettings' | 'keybase.1.gregor.dismissCategory' | 'keybase.1.gregor.getState' | 'keybase.1.gregor.updateCategory' | 'keybase.1.home.homeDismissAnnouncement' | 'keybase.1.home.homeGetScreen' | 'keybase.1.home.homeMarkViewed' | 'keybase.1.home.homeSkipTodoType' | 'keybase.1.identify3.identify3FollowUser' | 'keybase.1.identify3.identify3IgnoreUser' | 'keybase.1.incomingShare.getIncomingShareItems' | 'keybase.1.incomingShare.getPreference' | 'keybase.1.incomingShare.setPreference' | 'keybase.1.install.fuseStatus' | 'keybase.1.install.installFuse' | 'keybase.1.install.installKBFS' | 'keybase.1.install.uninstallKBFS' | 'keybase.1.kbfsMount.GetCurrentMountDir' | 'keybase.1.kbfsMount.GetKBFSPathInfo' | 'keybase.1.kbfsMount.GetPreferredMountDirs' | 'keybase.1.kbfsMount.WaitForMounts' | 'keybase.1.log.perfLogPoint' | 'keybase.1.login.accountDelete' | 'keybase.1.login.deprovision' | 'keybase.1.login.getConfiguredAccounts' | 'keybase.1.login.isOnline' | 'keybase.1.login.logout' | 'keybase.1.login.paperKeySubmit' | 'keybase.1.notifyCtl.setNotifications' | 'keybase.1.pgp.pgpStorageDismiss' | 'keybase.1.phoneNumbers.addPhoneNumber' | 'keybase.1.phoneNumbers.deletePhoneNumber' | 'keybase.1.phoneNumbers.resendVerificationForPhoneNumber' | 'keybase.1.phoneNumbers.setVisibilityPhoneNumber' | 'keybase.1.phoneNumbers.verifyPhoneNumber' | 'keybase.1.pprof.logProcessorProfile' | 'keybase.1.pprof.logTrace' | 'keybase.1.prove.checkProof' | 'keybase.1.reachability.checkReachability' | 'keybase.1.rekey.getRevokeWarning' | 'keybase.1.rekey.rekeyStatusFinish' | 'keybase.1.rekey.showPendingRekeyStatus' | 'keybase.1.revoke.revokeDevice' | 'keybase.1.revoke.revokeKey' | 'keybase.1.revoke.revokeSigs' | 'keybase.1.saltpack.saltpackDecryptFile' | 'keybase.1.saltpack.saltpackDecryptString' | 'keybase.1.saltpack.saltpackEncryptFile' | 'keybase.1.saltpack.saltpackEncryptString' | 'keybase.1.saltpack.saltpackSaveCiphertextToFile' | 'keybase.1.saltpack.saltpackSaveSignedMsgToFile' | 'keybase.1.saltpack.saltpackSignFile' | 'keybase.1.saltpack.saltpackSignString' | 'keybase.1.saltpack.saltpackVerifyFile' | 'keybase.1.saltpack.saltpackVerifyString' | 'keybase.1.signup.checkUsernameAvailable' | 'keybase.1.signup.getInvitationCode' | 'keybase.1.SimpleFS.simpleFSArchiveAllFiles' | 'keybase.1.SimpleFS.simpleFSArchiveAllGitRepos' | 'keybase.1.SimpleFS.simpleFSArchiveCancelOrDismissJob' | 'keybase.1.SimpleFS.simpleFSArchiveStart' | 'keybase.1.SimpleFS.simpleFSCancelDownload' | 'keybase.1.SimpleFS.simpleFSCheckReachability' | 'keybase.1.SimpleFS.simpleFSClearConflictState' | 'keybase.1.SimpleFS.simpleFSConfigureDownload' | 'keybase.1.SimpleFS.simpleFSCopyRecursive' | 'keybase.1.SimpleFS.simpleFSDismissDownload' | 'keybase.1.SimpleFS.simpleFSDismissUpload' | 'keybase.1.SimpleFS.simpleFSFinishResolvingConflict' | 'keybase.1.SimpleFS.simpleFSFolderSyncConfigAndStatus' | 'keybase.1.SimpleFS.simpleFSGetArchiveJobFreshness' | 'keybase.1.SimpleFS.simpleFSGetArchiveStatus' | 'keybase.1.SimpleFS.simpleFSGetDownloadInfo' | 'keybase.1.SimpleFS.simpleFSGetDownloadStatus' | 'keybase.1.SimpleFS.simpleFSGetFilesTabBadge' | 'keybase.1.SimpleFS.simpleFSGetFolder' | 'keybase.1.SimpleFS.simpleFSGetGUIFileContext' | 'keybase.1.SimpleFS.simpleFSGetOnlineStatus' | 'keybase.1.SimpleFS.simpleFSGetUploadStatus' | 'keybase.1.SimpleFS.simpleFSList' | 'keybase.1.SimpleFS.simpleFSListFavorites' | 'keybase.1.SimpleFS.simpleFSListRecursiveToDepth' | 'keybase.1.SimpleFS.simpleFSMakeTempDirForUpload' | 'keybase.1.SimpleFS.simpleFSMove' | 'keybase.1.SimpleFS.simpleFSOpen' | 'keybase.1.SimpleFS.simpleFSReadList' | 'keybase.1.SimpleFS.simpleFSRemove' | 'keybase.1.SimpleFS.simpleFSSetDebugLevel' | 'keybase.1.SimpleFS.simpleFSSetFolderSyncConfig' | 'keybase.1.SimpleFS.simpleFSSetNotificationThreshold' | 'keybase.1.SimpleFS.simpleFSSetSfmiBannerDismissed' | 'keybase.1.SimpleFS.simpleFSSetSyncOnCellular' | 'keybase.1.SimpleFS.simpleFSSettings' | 'keybase.1.SimpleFS.simpleFSStartDownload' | 'keybase.1.SimpleFS.simpleFSStartUpload' | 'keybase.1.SimpleFS.simpleFSStat' | 'keybase.1.SimpleFS.simpleFSSubscribeNonPath' | 'keybase.1.SimpleFS.simpleFSSubscribePath' | 'keybase.1.SimpleFS.simpleFSSyncStatus' | 'keybase.1.SimpleFS.simpleFSUnsubscribe' | 'keybase.1.SimpleFS.simpleFSUserEditHistory' | 'keybase.1.SimpleFS.simpleFSUserIn' | 'keybase.1.SimpleFS.simpleFSUserOut' | 'keybase.1.SimpleFS.simpleFSWait' | 'keybase.1.teams.findAssertionsInTeamNoResolve' | 'keybase.1.teams.getAnnotatedTeam' | 'keybase.1.teams.getInviteLinkDetails' | 'keybase.1.teams.getTeamID' | 'keybase.1.teams.getTeamRoleMap' | 'keybase.1.teams.getUntrustedTeamInfo' | 'keybase.1.teams.loadTeamTreeMembershipsAsync' | 'keybase.1.teams.setTarsDisabled' | 'keybase.1.teams.setTeamMemberShowcase' | 'keybase.1.teams.setTeamShowcase' | 'keybase.1.teams.teamAddEmailsBulk' | 'keybase.1.teams.teamAddMember' | 'keybase.1.teams.teamAddMembersMultiRole' | 'keybase.1.teams.teamCreate' | 'keybase.1.teams.teamCreateFancy' | 'keybase.1.teams.teamCreateSeitanTokenV2' | 'keybase.1.teams.teamEditMembers' | 'keybase.1.teams.teamGetMembersByID' | 'keybase.1.teams.teamIgnoreRequest' | 'keybase.1.teams.teamLeave' | 'keybase.1.teams.teamListMyAccessRequests' | 'keybase.1.teams.teamListUnverified' | 'keybase.1.teams.teamProfileAddList' | 'keybase.1.teams.teamReAddMemberAfterReset' | 'keybase.1.teams.teamRemoveMember' | 'keybase.1.teams.teamRename' | 'keybase.1.teams.teamSetSettings' | 'keybase.1.teams.untrustedTeamExists' | 'keybase.1.teams.uploadTeamAvatar' | 'keybase.1.user.blockUser' | 'keybase.1.user.canLogout' | 'keybase.1.user.dismissBlockButtons' | 'keybase.1.user.getUserBlocks' | 'keybase.1.user.interestingPeople' | 'keybase.1.user.listTrackersUnverified' | 'keybase.1.user.listTracking' | 'keybase.1.user.loadMySettings' | 'keybase.1.user.loadPassphraseState' | 'keybase.1.user.profileEdit' | 'keybase.1.user.proofSuggestions' | 'keybase.1.user.reportUser' | 'keybase.1.user.setUserBlocks' | 'keybase.1.user.unblockUser' | 'keybase.1.user.uploadUserAvatar' | 'keybase.1.user.userCard' | 'keybase.1.userSearch.bulkEmailOrPhoneSearch' | 'keybase.1.userSearch.getNonUserDetails' | 'keybase.1.userSearch.userSearch' export type RpcFn = [RpcIn] extends [undefined] ? (params?: undefined, waitingKey?: WaitingKey) => Promise> : (params: RpcIn, waitingKey?: WaitingKey) => Promise> @@ -2549,6 +2545,8 @@ export type CheckProofStatus = {readonly found: boolean,readonly status: ProofSt export type CheckResult = {readonly proofResult: ProofResult,readonly time: Time,readonly freshness: CheckResultFreshness,} export type CiphertextBundle = {readonly kid: KID,readonly ciphertext: EncryptedBytes32,readonly nonce: BoxNonce,readonly publicKey: BoxPublicKey,} export type ClientDetails = {readonly pid: number,readonly clientType: ClientType,readonly argv?: ReadonlyArray | null,readonly desc: string,readonly version: string,} +export type ClientSession = {readonly loggedIn: boolean,readonly uid: UID,readonly username: string,readonly deviceID: DeviceID,readonly deviceName: string,} +export type ClientState = {readonly session?: ClientSession | null,readonly httpSrvInfo?: HttpSrvInfo | null,readonly appState: MobileAppState,} export type ClientStatus = {readonly details: ClientDetails,readonly connectionID: number,readonly notificationChannels: NotificationChannels,} export type CompatibilityTeamID ={ typ: TeamType.legacy, legacy: TLFID } | { typ: TeamType.modern, modern: TeamID } | { typ: TeamType.none} export type ComponentResult = {readonly name: string,readonly status: Status,readonly exitCode: number,} @@ -3129,7 +3127,7 @@ export type WalletAccountInfo = {readonly accountID: string,readonly numUnread: export type WebProof = {readonly hostname: string,readonly protocols?: ReadonlyArray | null,} export type WriteArgs = {readonly opID: OpID,readonly path: Path,readonly offset: number,} -type IncomingMethod = 'keybase.1.NotifyApp.mobileAppStateChanged' | 'keybase.1.NotifyAudit.boxAuditError' | 'keybase.1.NotifyAudit.rootAuditError' | 'keybase.1.NotifyBadges.badgeState' | 'keybase.1.NotifyDeviceHistory.deviceHistoryChanged' | 'keybase.1.NotifyFS.FSActivity' | 'keybase.1.NotifySession.loggedOut' | 'keybase.1.NotifyTracking.trackingChanged' | 'keybase.1.NotifyUsers.userChanged' | 'keybase.1.loginUi.displayPaperKeyPhrase' | 'keybase.1.loginUi.displayPrimaryPaperKey' | 'keybase.1.loginUi.displayResetProgress' | 'keybase.1.loginUi.explainDeviceRecovery' | 'keybase.1.pgpUi.finished' | 'keybase.1.proveUi.displayRecheckWarning' | 'keybase.1.proveUi.outputPrechecks' | 'keybase.1.provisionUi.DisplaySecretExchanged' | 'keybase.1.provisionUi.ProvisioneeSuccess' | 'keybase.1.provisionUi.ProvisionerSuccess' | 'keybase.1.reachability.reachabilityChanged' | 'keybase.1.rekeyUI.refresh' | 'keybase.1.rekeyUI.rekeySendEvent' +type IncomingMethod = 'keybase.1.NotifyApp.clientState' | 'keybase.1.NotifyApp.mobileAppStateChanged' | 'keybase.1.NotifyAudit.boxAuditError' | 'keybase.1.NotifyAudit.rootAuditError' | 'keybase.1.NotifyBadges.badgeState' | 'keybase.1.NotifyDeviceHistory.deviceHistoryChanged' | 'keybase.1.NotifyFS.FSActivity' | 'keybase.1.NotifySession.loggedOut' | 'keybase.1.NotifyTracking.trackingChanged' | 'keybase.1.NotifyUsers.userChanged' | 'keybase.1.loginUi.displayPaperKeyPhrase' | 'keybase.1.loginUi.displayPrimaryPaperKey' | 'keybase.1.loginUi.displayResetProgress' | 'keybase.1.loginUi.explainDeviceRecovery' | 'keybase.1.pgpUi.finished' | 'keybase.1.proveUi.displayRecheckWarning' | 'keybase.1.proveUi.outputPrechecks' | 'keybase.1.provisionUi.DisplaySecretExchanged' | 'keybase.1.provisionUi.ProvisioneeSuccess' | 'keybase.1.provisionUi.ProvisionerSuccess' | 'keybase.1.rekeyUI.refresh' | 'keybase.1.rekeyUI.rekeySendEvent' export type IncomingCallMapType = Partial<{[M in IncomingMethod]: (params: RpcIn) => void}> type CustomIncomingMethod = 'keybase.1.NotifyApp.exit' | 'keybase.1.NotifyEmailAddress.emailAddressVerified' | 'keybase.1.NotifyEmailAddress.emailsChanged' | 'keybase.1.NotifyFS.FSOverallSyncStatusChanged' | 'keybase.1.NotifyFS.FSSubscriptionNotify' | 'keybase.1.NotifyFS.FSSubscriptionNotifyPath' | 'keybase.1.NotifyFeaturedBots.featuredBotsUpdate' | 'keybase.1.NotifyPGP.pgpKeyInSecretStoreFile' | 'keybase.1.NotifyPhoneNumber.phoneNumbersChanged' | 'keybase.1.NotifyRuntimeStats.runtimeStatsUpdate' | 'keybase.1.NotifyService.HTTPSrvInfoUpdate' | 'keybase.1.NotifyService.handleKeybaseLink' | 'keybase.1.NotifyService.shutdown' | 'keybase.1.NotifySession.clientOutOfDate' | 'keybase.1.NotifySession.loggedIn' | 'keybase.1.NotifySimpleFS.simpleFSArchiveStatusChanged' | 'keybase.1.NotifyTeam.avatarUpdated' | 'keybase.1.NotifyTeam.teamChangedByID' | 'keybase.1.NotifyTeam.teamDeleted' | 'keybase.1.NotifyTeam.teamExit' | 'keybase.1.NotifyTeam.teamMetadataUpdate' | 'keybase.1.NotifyTeam.teamRoleMapChanged' | 'keybase.1.NotifyTeam.teamTreeMembershipsDone' | 'keybase.1.NotifyTeam.teamTreeMembershipsPartial' | 'keybase.1.NotifyTracking.notifyUserBlocked' | 'keybase.1.NotifyTracking.trackingInfo' | 'keybase.1.NotifyUsers.identifyUpdate' | 'keybase.1.NotifyUsers.passwordChanged' | 'keybase.1.gpgUi.selectKey' | 'keybase.1.gpgUi.wantToAddGPGKey' | 'keybase.1.gregorUI.pushState' | 'keybase.1.homeUI.homeUIRefresh' | 'keybase.1.identify3Ui.identify3Result' | 'keybase.1.identify3Ui.identify3ShowTracker' | 'keybase.1.identify3Ui.identify3Summary' | 'keybase.1.identify3Ui.identify3UpdateRow' | 'keybase.1.identify3Ui.identify3UpdateUserCard' | 'keybase.1.identify3Ui.identify3UserReset' | 'keybase.1.logUi.log' | 'keybase.1.loginUi.chooseDeviceToRecoverWith' | 'keybase.1.loginUi.displayPaperKeyPhrase' | 'keybase.1.loginUi.displayPrimaryPaperKey' | 'keybase.1.loginUi.displayResetProgress' | 'keybase.1.loginUi.explainDeviceRecovery' | 'keybase.1.loginUi.getEmailOrUsername' | 'keybase.1.loginUi.promptPassphraseRecovery' | 'keybase.1.loginUi.promptResetAccount' | 'keybase.1.loginUi.promptRevokePaperKeys' | 'keybase.1.logsend.prepareLogsend' | 'keybase.1.pgpUi.finished' | 'keybase.1.pgpUi.keyGenerated' | 'keybase.1.pgpUi.shouldPushPrivate' | 'keybase.1.proveUi.checking' | 'keybase.1.proveUi.continueChecking' | 'keybase.1.proveUi.displayRecheckWarning' | 'keybase.1.proveUi.okToCheck' | 'keybase.1.proveUi.outputInstructions' | 'keybase.1.proveUi.outputPrechecks' | 'keybase.1.proveUi.preProofWarning' | 'keybase.1.proveUi.promptOverwrite' | 'keybase.1.proveUi.promptUsername' | 'keybase.1.provisionUi.DisplayAndPromptSecret' | 'keybase.1.provisionUi.DisplaySecretExchanged' | 'keybase.1.provisionUi.PromptNewDeviceName' | 'keybase.1.provisionUi.ProvisioneeSuccess' | 'keybase.1.provisionUi.ProvisionerSuccess' | 'keybase.1.provisionUi.chooseDevice' | 'keybase.1.provisionUi.chooseDeviceType' | 'keybase.1.provisionUi.chooseGPGMethod' | 'keybase.1.provisionUi.switchToGPGSignOK' | 'keybase.1.rekeyUI.delegateRekeyUI' | 'keybase.1.rekeyUI.refresh' | 'keybase.1.rekeyUI.rekeySendEvent' | 'keybase.1.secretUi.getPassphrase' | 'keybase.1.teamsUi.confirmInviteLinkAccept' | 'keybase.1.teamsUi.confirmRootTeamDelete' | 'keybase.1.teamsUi.confirmSubteamDelete' @@ -3294,7 +3292,6 @@ export const pprofLogTraceRpcPromise = createRpc('keybase.1.pprof.logTrace') export const proveCheckProofRpcPromise = createRpc('keybase.1.prove.checkProof') export const proveStartProofRpcListener = createListener('keybase.1.prove.startProof') export const reachabilityCheckReachabilityRpcPromise = createRpc('keybase.1.reachability.checkReachability') -export const reachabilityStartReachabilityRpcPromise = createRpc('keybase.1.reachability.startReachability') export const rekeyGetRevokeWarningRpcPromise = createRpc('keybase.1.rekey.getRevokeWarning') export const rekeyRekeyStatusFinishRpcPromise = createRpc('keybase.1.rekey.rekeyStatusFinish') export const rekeyShowPendingRekeyStatusRpcPromise = createRpc('keybase.1.rekey.showPendingRekeyStatus') @@ -3619,6 +3616,8 @@ export const userUserCardRpcPromise = createRpc('keybase.1.user.userCard') // 'keybase.1.prove.validateUsername' // 'keybase.1.provisionUi.chooseProvisioningMethod' // 'keybase.1.quota.verifySession' +// 'keybase.1.reachability.reachabilityChanged' +// 'keybase.1.reachability.startReachability' // 'keybase.1.rekey.getPendingRekeyStatus' // 'keybase.1.rekey.debugShowRekeyStatus' // 'keybase.1.rekey.rekeySync' diff --git a/shared/engine/index.platform.tsx b/shared/engine/index.platform.tsx index f0cf37f0ba68..8e64b8c50219 100644 --- a/shared/engine/index.platform.tsx +++ b/shared/engine/index.platform.tsx @@ -257,7 +257,8 @@ function createClient( // from a session cancel handler inside disconnectCallback must // not strand the UI on the disconnect banner by skipping // connectCallback (which synchronously clears the daemon error - // via startHandshake()). + // via startHandshake(), so nothing here may be moved behind an + // await). client.transport.reset() try { disconnectCallback() diff --git a/shared/login/loading.tsx b/shared/login/loading.tsx index bac0b0886a50..a95e7da4b2c2 100644 --- a/shared/login/loading.tsx +++ b/shared/login/loading.tsx @@ -26,7 +26,7 @@ const SplashContainer = () => { C.Router2.navigateAppend({name: 'feedback', params: {}}) } : undefined - const onRetry = handshakeFailed ? startHandshake : undefined + const onRetry = handshakeFailed ? () => startHandshake() : undefined return } diff --git a/shared/stores/config.tsx b/shared/stores/config.tsx index 18715f9c7129..2d8c600bb5a3 100644 --- a/shared/stores/config.tsx +++ b/shared/stores/config.tsx @@ -24,7 +24,6 @@ type Store = T.Immutable<{ configuredAccounts: Array defaultUsername: string globalError?: Error | RPCError - gregorReachable?: T.RPCGen.Reachable gregorPushState: Array<{md: T.RPCGregor.Metadata; item: T.RPCGregor.Item}> loginError?: RPCError httpSrv: { @@ -62,7 +61,6 @@ const initialStore: Store = { defaultUsername: '', globalError: undefined, gregorPushState: [], - gregorReachable: undefined, httpSrv: { address: '', token: '', @@ -112,7 +110,6 @@ export type State = Store & { setChatStaticConfig: (s: T.Chat.StaticConfig) => void setDefaultUsername: (u: string) => void setGlobalError: (e?: unknown) => void - setGregorReachable: (r: Store['gregorReachable']) => void setHTTPSrvInfo: (address: string, token: string) => void setJustDeletedSelf: (s: string) => void setLoggedIn: (l: boolean) => void @@ -151,14 +148,6 @@ export const useConfigState = Z.createZustand('config', (set, get) => { } } - const setGregorReachable = (r: Store['gregorReachable']) => { - const old = get().gregorReachable - if (old === r) return - set(s => { - s.gregorReachable = r - }) - } - const setGregorPushState = (state: T.RPCGen.Gregor1.State) => { const items = state.items || [] const goodState = items.reduce>( @@ -277,16 +266,14 @@ export const useConfigState = Z.createZustand('config', (set, get) => { }, waitingKey: waitingKeyConfigLogin, }) + // The session arrives as a clientState, which can come before or after this reply. logger.info('login call succeeded') - get().dispatch.setLoggedIn(true) } catch (error) { if (!(error instanceof RPCError)) { return } - if (error.code === T.RPCGen.StatusCode.scalreadyloggedin) { - get().dispatch.setLoggedIn(true) - } else if (error.desc !== cancelDesc) { - // If we're canceling then ignore the error + // Already logged in: a clientState has said so, or will. Canceling: nothing to report. + if (error.code !== T.RPCGen.StatusCode.scalreadyloggedin && error.desc !== cancelDesc) { error.desc = niceError(error) get().dispatch.setLoginError(error) } @@ -316,21 +303,9 @@ export const useConfigState = Z.createZustand('config', (set, get) => { ignorePromise(f()) }, onEngineConnected: () => { - // An engine reset drops in-flight RPCs without settling their promises; a refresh - // caught by that would poison the dedupe cache forever + // An engine reset fails the old connection's in-flight RPCs, but that failure reaches the + // dedupe cache a few microtasks later: a refresh started before then would join the dead one inflightRefreshAccounts = undefined - // The startReachability RPC call both starts and returns the current - // reachability state. Then we'll get updates of changes from this state via reachabilityChanged. - // This should be run on app start and service re-connect in case the service somehow crashed or was restarted manually. - const startReachability = async () => { - try { - const reachability = await T.RPCGen.reachabilityStartReachabilityRpcPromise() - get().dispatch.setGregorReachable(reachability.reachable) - } catch (err) { - logger.warn('error bootstrapping reachability: ', err) - } - } - ignorePromise(startReachability()) // If ever you want to get OOBMs for a different system, then you need to enter it here. const registerForGregorNotifications = async () => { @@ -375,29 +350,6 @@ export const useConfigState = Z.createZustand('config', (set, get) => { get().dispatch.setHTTPSrvInfo(action.payload.params.info.address, action.payload.params.info.token) break } - case 'keybase.1.NotifySession.loggedIn': { - logger.info('keybase.1.NotifySession.loggedIn') - // only send this if we think we're not logged in - const {loggedIn, dispatch} = get() - if (!loggedIn) { - dispatch.setLoggedIn(true) - } - break - } - case 'keybase.1.NotifySession.loggedOut': { - logger.info('keybase.1.NotifySession.loggedOut') - const {loggedIn, dispatch} = get() - // only send this if we think we're logged in (errors on provison can trigger this and mess things up) - if (loggedIn) { - dispatch.setLoggedIn(false) - } - break - } - case 'keybase.1.reachability.reachabilityChanged': - if (get().loggedIn) { - get().dispatch.setGregorReachable(action.payload.params.reachability.reachable) - } - break default: } }, @@ -459,6 +411,8 @@ export const useConfigState = Z.createZustand('config', (set, get) => { configuredAccounts: s.configuredAccounts, defaultUsername: s.defaultUsername, dispatch: s.dispatch, + // process-wide, not per account; nothing reloads it on logout + httpSrv: s.httpSrv, startup: {loaded: s.startup.loaded}, userSwitching: s.userSwitching, })) @@ -523,9 +477,6 @@ export const useConfigState = Z.createZustand('config', (set, get) => { }) } }, - setGregorReachable: r => { - setGregorReachable(r) - }, setHTTPSrvInfo: (address, token) => { set(s => { s.httpSrv.address = address diff --git a/shared/stores/daemon.tsx b/shared/stores/daemon.tsx index 31c4371d35ff..b9e0c87a174d 100644 --- a/shared/stores/daemon.tsx +++ b/shared/stores/daemon.tsx @@ -66,7 +66,10 @@ export const useDaemonState = Z.createZustand('daemon', (set, get) => { `[Bootstrap] loggedIn: ${bs.loggedIn ? 1 : 0} http: ${bs.httpSrvInfo ? bs.httpSrvInfo.address : 'none'}` ) // a newer handshake owns the store now; don't write a potentially older status over its load - if (gen !== generation || isEqual(bs, get().bootstrapStatus)) { + if (gen !== generation) { + return + } + if (isEqual(bs, get().bootstrapStatus)) { return } set(s => { @@ -88,6 +91,10 @@ export const useDaemonState = Z.createZustand('daemon', (set, get) => { ...s, ...initialStore, dispatch: s.dispatch, + // Both track the connection, not the account, and the closure counter behind the + // generation keeps climbing across a reset: zeroing the copy here would make the live + // connection's own in-flight work look superseded by a logout that happened under it. + handshakeGeneration: s.handshakeGeneration, handshakeState: s.handshakeState, })) }, @@ -101,8 +108,8 @@ export const useDaemonState = Z.createZustand('daemon', (set, get) => { }, startHandshake: () => { const gen = ++generation - // startHandshake follows an engine reset, which drops in-flight RPCs without settling - // their promises; reusing one here would stall the handshake forever + // startHandshake follows an engine reset, which fails the old connection's in-flight RPCs; + // reusing one here would fail this handshake's first attempt with the old connection's error inflightBootstrapStatus = undefined set(s => { s.error = undefined diff --git a/shared/stores/shell.tsx b/shared/stores/shell.tsx index 7bb9237e8ee0..47c09f9e8dc0 100644 --- a/shared/stores/shell.tsx +++ b/shared/stores/shell.tsx @@ -6,7 +6,6 @@ import isEqual from 'lodash/isEqual' import logger from '@/logger' import {RPCError} from '@/util/errors' import {defaultUseNativeFrame} from '@/constants/platform' -import {useConfigState} from '@/stores/config' export type ConnectionType = NetInfo.NetInfoStateType | 'notavailable' @@ -156,11 +155,16 @@ export const useShellState = Z.createZustand('shell', (set, get) => { s.networkStatus.type = type } }) - const updateGregor = async () => { - const reachability = await T.RPCGen.reachabilityCheckReachabilityRpcPromise() - useConfigState.getState().dispatch.setGregorReachable(reachability.reachable) + // Not for the result: the service re-dials gregor inside this call and reconnects if the + // dial fails, which is what gets it off a dead connection after the network moves. + const nudgeGregor = async () => { + try { + await T.RPCGen.reachabilityCheckReachabilityRpcPromise() + } catch (error) { + logger.warn('failed to check gregor reachability: ', error) + } } - ignorePromise(updateGregor()) + ignorePromise(nudgeGregor()) const updateFS = async () => { if (isInit) return diff --git a/shared/stores/tests/client-state.test.ts b/shared/stores/tests/client-state.test.ts new file mode 100644 index 000000000000..d46b295c0574 --- /dev/null +++ b/shared/stores/tests/client-state.test.ts @@ -0,0 +1,220 @@ +/// +import * as T from '@/constants/types' +import {resetAllStores} from '@/util/zustand' +import {useConfigState} from '../config' +import {useCurrentUserState} from '../current-user' +import {useShellState} from '../shell' +import {_onEngineIncoming, applyClientState} from '@/constants/init/shared' + +const g = globalThis as unknown as {isMobile: boolean} + +const session = (over: Partial = {}): T.RPCGen.ClientSession => ({ + deviceID: 'd1', + deviceName: 'testuser-mac', + loggedIn: true, + uid: 'u1', + username: 'testuser', + ...over, +}) + +const loggedOut = session({deviceID: '', deviceName: '', loggedIn: false, uid: '', username: ''}) + +const clientState = (over: Partial = {}): T.RPCGen.ClientState => ({ + appState: T.RPCGen.MobileAppState.foreground, + session: session(), + ...over, +}) + +const notifyClientState = (state: T.RPCGen.ClientState) => + _onEngineIncoming({payload: {params: {state}}, type: 'keybase.1.NotifyApp.clientState'} as never) + +const notifyHTTP = (address: string) => + useConfigState.getState().dispatch.onEngineIncoming({ + payload: {params: {info: {address, token: 'token'}}}, + type: 'keybase.1.NotifyService.HTTPSrvInfoUpdate', + } as never) + +afterEach(() => { + g.isMobile = false + jest.restoreAllMocks() + resetAllStores() +}) + +describe('a clientState', () => { + test('replaces the session, the current user, the http address and the app state', () => { + g.isMobile = true + notifyClientState( + clientState({ + appState: T.RPCGen.MobileAppState.background, + httpSrvInfo: {address: '127.0.0.1:1', token: 'token'}, + }) + ) + + expect(useConfigState.getState().loggedIn).toBe(true) + expect(useConfigState.getState().httpSrv.address).toBe('127.0.0.1:1') + expect(useCurrentUserState.getState().username).toBe('testuser') + expect(useCurrentUserState.getState().deviceID).toBe('d1') + expect(useShellState.getState().mobileAppState).toBe('background') + }) + + test('is applied in arrival order, with no versions: the last one wins', () => { + applyClientState(clientState({httpSrvInfo: {address: '127.0.0.1:1', token: 'token'}})) + notifyHTTP('127.0.0.1:2') + expect(useConfigState.getState().httpSrv.address).toBe('127.0.0.1:2') + + applyClientState(clientState({httpSrvInfo: {address: '127.0.0.1:3', token: 'token'}, session: loggedOut})) + expect(useConfigState.getState().httpSrv.address).toBe('127.0.0.1:3') + expect(useConfigState.getState().loggedIn).toBe(false) + }) + + test('with no session leaves the session as it was: the service does not know it yet', () => { + applyClientState(clientState({session: undefined})) + expect(useConfigState.getState().loggedIn).toBe(false) + expect(useCurrentUserState.getState().username).toBe('') + + applyClientState(clientState()) + expect(useConfigState.getState().loggedIn).toBe(true) + + applyClientState(clientState({session: null})) + expect(useConfigState.getState().loggedIn).toBe(true) + }) + + test('has the current user in place before anything reacts to the login', () => { + // setLoggedIn fans out synchronously; every subscriber of a login has always been able to + // read the current user by the time it runs + let seen = 'not called' + const unsub = useConfigState.subscribe((st, prev) => { + if (st.loggedIn && !prev.loggedIn) { + seen = useCurrentUserState.getState().username + } + }) + + applyClientState(clientState()) + unsub() + + expect(seen).toBe('testuser') + }) + + test('logging out keeps the http server address', () => { + notifyHTTP('127.0.0.1:2') + useConfigState.getState().dispatch.resetState() + expect(useConfigState.getState().httpSrv.address).toBe('127.0.0.1:2') + }) + + test('loggedIn and loggedOut change nothing about the session: clientState owns it', () => { + useConfigState.getState().dispatch.onEngineIncoming({ + payload: {params: {signedUp: false, username: 'testuser'}}, + type: 'keybase.1.NotifySession.loggedIn', + } as never) + expect(useConfigState.getState().loggedIn).toBe(false) + + applyClientState(clientState()) + useConfigState.getState().dispatch.onEngineIncoming({ + payload: {params: undefined}, + type: 'keybase.1.NotifySession.loggedOut', + } as never) + expect(useConfigState.getState().loggedIn).toBe(true) + }) +}) + +describe('an account switch', () => { + const userB = session({deviceID: 'd2', uid: 'u2', username: 'testuser2'}) + + // what resetAllStores clears, standing in for the previous account's state + const markAccountState = () => useConfigState.setState({justDeletedSelf: 'testuser'}) + const accountStateCleared = () => useConfigState.getState().justDeletedSelf === '' + + const loginChanges = () => { + const changes: Array = [] + const unsub = useConfigState.subscribe((st, prev) => { + if (st.loggedIn !== prev.loggedIn) { + changes.push(st.loggedIn) + } + }) + return {changes, unsub} + } + + test('with both clientStates logs out, clearing the old account, then logs in as the new one', () => { + applyClientState(clientState()) + markAccountState() + useConfigState.getState().dispatch.setUserSwitching(true) + const {changes, unsub} = loginChanges() + + applyClientState(clientState({session: loggedOut})) + expect(accountStateCleared()).toBe(true) + applyClientState(clientState({session: userB})) + unsub() + + expect(changes).toEqual([false, true]) + expect(useCurrentUserState.getState().username).toBe('testuser2') + expect(useConfigState.getState().loggedIn).toBe(true) + }) + + test('whose logged-out clientState never arrived still clears the old account', () => { + applyClientState(clientState()) + markAccountState() + useConfigState.getState().dispatch.setUserSwitching(true) + const {changes, unsub} = loginChanges() + + applyClientState(clientState({session: userB})) + unsub() + + expect(changes).toEqual([false, true]) + expect(accountStateCleared()).toBe(true) + expect(useCurrentUserState.getState().username).toBe('testuser2') + expect(useCurrentUserState.getState().uid).toBe('u2') + }) + + test('whose login fails after the logout ends logged out, no longer switching', () => { + applyClientState(clientState()) + useConfigState.getState().dispatch.setUserSwitching(true) + + applyClientState(clientState({session: loggedOut})) + useConfigState.getState().dispatch.setLoginError(new Error('bad password') as never) + + expect(useConfigState.getState().loggedIn).toBe(false) + expect(useConfigState.getState().userSwitching).toBe(false) + expect(useCurrentUserState.getState().username).toBe('') + }) + + test('a logout never shows a logged-in session with no user', () => { + applyClientState(clientState()) + const seen: Array<{loggedIn: boolean; uid: string}> = [] + const record = () => + seen.push({loggedIn: useConfigState.getState().loggedIn, uid: useCurrentUserState.getState().uid}) + const unsubs = [useConfigState.subscribe(record), useCurrentUserState.subscribe(record)] + + applyClientState(clientState({session: loggedOut})) + unsubs.forEach(u => u()) + + expect(seen.length).toBeGreaterThan(0) + expect(seen.filter(s => s.loggedIn && !s.uid)).toEqual([]) + expect(useConfigState.getState().loggedIn).toBe(false) + expect(useCurrentUserState.getState().uid).toBe('') + }) + + test('logged in with no current user yet is not a switch', () => { + useConfigState.getState().dispatch.setLoggedIn(true) + markAccountState() + const {changes, unsub} = loginChanges() + + applyClientState(clientState()) + unsub() + + expect(changes).toEqual([]) + expect(accountStateCleared()).toBe(false) + expect(useCurrentUserState.getState().uid).toBe('u1') + }) + + test('the same user again is not a switch', () => { + applyClientState(clientState()) + markAccountState() + const {changes, unsub} = loginChanges() + + applyClientState(clientState()) + unsub() + + expect(changes).toEqual([]) + expect(accountStateCleared()).toBe(false) + }) +}) diff --git a/shared/stores/tests/config.test.ts b/shared/stores/tests/config.test.ts index e6595a24ca60..b131149fa66c 100644 --- a/shared/stores/tests/config.test.ts +++ b/shared/stores/tests/config.test.ts @@ -1,4 +1,6 @@ /// +import * as T from '../../constants/types' +import {RPCError} from '../../util/errors' import {noConversationIDKey} from '../../constants/types/chat/common' import {useConfigState} from '../config' @@ -116,3 +118,31 @@ test('custom resetState preserves the fields config intentionally carries across expect(state.userSwitching).toBe(true) expect(state.globalError).toBeUndefined() }) + +describe('login', () => { + afterEach(() => { + jest.restoreAllMocks() + }) + + const flush = async () => new Promise(resolve => setImmediate(resolve)) + + test('leaves the session to the clientState when the login succeeds', async () => { + jest.spyOn(T.RPCGen, 'loginLoginRpcListener').mockResolvedValue(undefined) + useConfigState.getState().dispatch.login('testuser', 'password') + await flush() + + expect(useConfigState.getState().loggedIn).toBe(false) + expect(useConfigState.getState().loginError).toBeUndefined() + }) + + test('leaves the session to the clientState when already logged in', async () => { + jest + .spyOn(T.RPCGen, 'loginLoginRpcListener') + .mockRejectedValue(new RPCError('already logged in', T.RPCGen.StatusCode.scalreadyloggedin)) + useConfigState.getState().dispatch.login('testuser', 'password') + await flush() + + expect(useConfigState.getState().loggedIn).toBe(false) + expect(useConfigState.getState().loginError).toBeUndefined() + }) +}) diff --git a/shared/stores/tests/daemon.test.ts b/shared/stores/tests/daemon.test.ts index dbb1ad951b0d..ed44e1e3c656 100644 --- a/shared/stores/tests/daemon.test.ts +++ b/shared/stores/tests/daemon.test.ts @@ -147,3 +147,38 @@ describe('daemon store', () => { expect(store.getState().handshakeRetriesLeft).toBe(maxHandshakeTries) }) }) + +describe('a superseded read', () => { + beforeEach(() => { + jest.useFakeTimers() + }) + afterEach(() => { + jest.useRealTimers() + jest.restoreAllMocks() + resetAllStores() + }) + + test('does not write its status over the newer load', async () => { + // a reconnect invalidates in-flight reads: the generation orders client attempts + let resolveLosing!: (bs: T.RPCGen.BootstrapStatus) => void + jest + .spyOn(T.RPCGen, 'configGetBootstrapStatusRpcPromise') + .mockReturnValueOnce( + new Promise(resolve => { + resolveLosing = resolve + }) + ) + .mockResolvedValue(bootstrapStatus) + const {dispatch} = useDaemonState.getState() + dispatch.initBootstrapSteps([]) + + const losing = dispatch.loadDaemonBootstrapStatus() + dispatch.startHandshake() + await jest.advanceTimersByTimeAsync(0) + resolveLosing({...bootstrapStatus, username: 'stale'}) + await losing + await jest.advanceTimersByTimeAsync(0) + + expect(useDaemonState.getState().bootstrapStatus?.username).toBe('testuser') + }) +}) From a88bd766bd0ffee4680389c51ff94d80771bb7ed Mon Sep 17 00:00:00 2001 From: chrisnojima Date: Mon, 21 Sep 2026 14:45:29 -0400 Subject: [PATCH 05/12] docs(bind): say what the loopback listener being up early means for clients Init starts the loopback server before the login attempt settles, so a client can connect and subscribe while there is still no session. Its first clientState carries none, and the attempt settling sends another that does. The old comment described GetBootstrapStatus waiting for the attempt, which is no longer how a client learns the session. --- go/bind/keybase.go | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/go/bind/keybase.go b/go/bind/keybase.go index 729cf6e0077d..f8beb533fac4 100644 --- a/go/bind/keybase.go +++ b/go/bind/keybase.go @@ -432,9 +432,11 @@ func Init(homeDir, mobileSharedHome, logFile, runModeStr string, kbSvc = service.NewService(kbCtx, false) // LoginAttemptNone: the login attempt happens inside RunBackgroundOperations // below, off the Init path. It can block for seconds (leveldb - // open/recovery, keychain reads) and Init runs on the native main thread; - // GetBootstrapStatus waits for the attempt so the GUI doesn't see a stale - // logged-out state. + // open/recovery, keychain reads) and Init runs on the native main thread. + // The loopback listener is therefore up while the attempt is still running, + // so a client can connect and subscribe before there is any session to + // report: its first clientState has no session at all in that window, and + // the attempt settling sends another that does. phase := time.Now() if err = kbSvc.StartLoopbackServer(libkb.LoginAttemptNone); err != nil { log("failed to start loopback: %s", err) From 57faaaaef4218ba6d06c3c795656653aaed0762b Mon Sep 17 00:00:00 2001 From: chrisnojima Date: Tue, 22 Sep 2026 16:35:48 -0400 Subject: [PATCH 06/12] fix(js): fall back to the bootstrap status when the service sends no clientState The handshake waited only for a clientState carrying a session, and a service older than clientState never sends one. On Linux the GUI can be upgraded while such a service keeps running -- the post-install restart is skipped when autorestart is disabled, the service isn't under systemd, or the mount is busy -- and the splash then failed every retry until the user restarted it by hand. When the wait times out with no clientState at all on the connection, the session and http address now come from the bootstrap status the handshake already read. A clientState that arrived without a session still means the service's startup login hasn't settled, so that case keeps waiting. --- shared/constants/init/shared.tsx | 28 ++++++++- shared/stores/tests/client-state.test.ts | 73 ++++++++++++++++++++++++ 2 files changed, 99 insertions(+), 2 deletions(-) diff --git a/shared/constants/init/shared.tsx b/shared/constants/init/shared.tsx index c4d98f069797..80e3862bb419 100644 --- a/shared/constants/init/shared.tsx +++ b/shared/constants/init/shared.tsx @@ -174,8 +174,8 @@ const onGregorPushStateChanged = ( ) } -// The bootstrap read the old gregor-reachability trigger did: after an offline stretch, pick up -// what the service learned while we could not reach it. `previous === undefined` is the first +// After an offline stretch, reread the bootstrap status to pick up what the service learned while +// we could not reach it. `previous === undefined` is the first // reading of the network at startup, which the handshake's own read already covers. export const onNetworkOnlineChanged = (online?: boolean, previous?: boolean) => { if (!online || previous !== false) { @@ -256,7 +256,11 @@ let settleSession = () => {} let sessionSettled = new Promise(resolve => { settleSession = resolve }) +// A service older than clientState never sends one. On Linux the GUI can be upgraded while such a +// service keeps running, until the user restarts it. +let clientStateSeen = false const awaitSessionAgain = () => { + clientStateSeen = false sessionSettled = new Promise(resolve => { settleSession = resolve }) @@ -268,6 +272,7 @@ const awaitSessionAgain = () => { // order. It comes first on subscribing, after every session change, and once the service's startup // login attempt settles. export const applyClientState = (clientState: T.RPCGen.ClientState) => { + clientStateSeen = true const {appState, httpSrvInfo, session} = clientState // On iOS JS never starts on a background launch, so it can have missed every change since the // process started: this is what catches it up. @@ -280,7 +285,12 @@ export const applyClientState = (clientState: T.RPCGen.ClientState) => { logger.info('[Bootstrap] the service has not settled its startup login yet') return } + applySession(session) +} + +const applySession = (session: T.RPCGen.ClientSession) => { settleSession() + const configDispatch = useConfigState.getState().dispatch const {deviceID, deviceName, loggedIn, uid, username} = session if (!loggedIn) { // Session first: logging out resets the stores, the current user among them. Writing the empty @@ -340,6 +350,20 @@ export const sessionSettledStep = async () => { }) try { await Promise.race([sessionSettled, timedOut]) + } catch (error) { + if (clientStateSeen) { + throw error + } + const bootstrapStatus = useDaemonState.getState().bootstrapStatus + if (!bootstrapStatus) { + throw error + } + logger.warn('[Bootstrap] the service sent no clientState, using its bootstrap status') + const {deviceID, deviceName, httpSrvInfo, loggedIn, uid, username} = bootstrapStatus + if (httpSrvInfo) { + useConfigState.getState().dispatch.setHTTPSrvInfo(httpSrvInfo.address, httpSrvInfo.token) + } + applySession({deviceID, deviceName, loggedIn, uid, username}) } finally { clearTimeout(timer) } diff --git a/shared/stores/tests/client-state.test.ts b/shared/stores/tests/client-state.test.ts index d46b295c0574..e433b3c61c4c 100644 --- a/shared/stores/tests/client-state.test.ts +++ b/shared/stores/tests/client-state.test.ts @@ -5,6 +5,11 @@ import {useConfigState} from '../config' import {useCurrentUserState} from '../current-user' import {useShellState} from '../shell' import {_onEngineIncoming, applyClientState} from '@/constants/init/shared' +import type * as ConfigModule from '../config' +import type * as CurrentUserModule from '../current-user' +import type * as DaemonModule from '../daemon' +import type * as SharedModule from '@/constants/init/shared' +import type * as TypesModule from '@/constants/types' const g = globalThis as unknown as {isMobile: boolean} @@ -218,3 +223,71 @@ describe('an account switch', () => { expect(accountStateCleared()).toBe(false) }) }) + +describe('waiting for the session', () => { + type Modules = { + config: typeof ConfigModule + currentUser: typeof CurrentUserModule + daemon: typeof DaemonModule + shared: typeof SharedModule + types: typeof TypesModule + } + // a fresh module registry per test, so each starts with nothing settled on the connection + const load = () => { + let m: Modules | undefined + jest.isolateModules(() => { + m = { + config: require('../config') as Modules['config'], + currentUser: require('../current-user') as Modules['currentUser'], + daemon: require('../daemon') as Modules['daemon'], + shared: require('@/constants/init/shared') as Modules['shared'], + types: require('@/constants/types') as Modules['types'], + } + }) + const modules = m! + jest.spyOn(modules.types.RPCGen, 'notifyCtlSetNotificationsRpcPromise').mockResolvedValue(undefined) + modules.daemon.useDaemonState.setState({ + bootstrapStatus: { + deviceID: 'd1', + deviceName: 'testuser-mac', + fullname: '', + httpSrvInfo: {address: '127.0.0.1:2', token: 'token'}, + loggedIn: true, + registered: true, + uid: 'u1', + userReacjis: {skinTone: T.RPCGen.ReacjiSkinTone.none, topReacjis: []}, + username: 'testuser', + }, + }) + return modules + } + + beforeEach(() => { + jest.useFakeTimers() + }) + afterEach(() => { + jest.useRealTimers() + }) + + test('a service that never sends a clientState is read from its bootstrap status', async () => { + const {config, currentUser, shared} = load() + const settled = shared.sessionSettledStep() + await jest.advanceTimersByTimeAsync(30_000) + await settled + + expect(config.useConfigState.getState().loggedIn).toBe(true) + expect(config.useConfigState.getState().httpSrv.address).toBe('127.0.0.1:2') + expect(currentUser.useCurrentUserState.getState().username).toBe('testuser') + }) + + test('a service that sent a clientState without a session is still waited on', async () => { + const {config, shared} = load() + const settled = shared.sessionSettledStep() + const failed = expect(settled).rejects.toThrow("The service hasn't said who is logged in") + shared.applyClientState({appState: T.RPCGen.MobileAppState.foreground}) + await jest.advanceTimersByTimeAsync(30_000) + await failed + + expect(config.useConfigState.getState().loggedIn).toBe(false) + }) +}) From 70249fa9316df7d31f0562e306425407bcb2e2c5 Mon Sep 17 00:00:00 2001 From: chrisnojima Date: Tue, 22 Sep 2026 16:49:19 -0400 Subject: [PATCH 07/12] fix(js): report an out-of-date service instead of half-supporting it Falling back to the bootstrap status covered only the startup handshake: with a service that never sends clientState, a later login, logout or account switch never reached the session, so the app was stuck in whatever state it started in. It also cost 30s on every connect, and a superseded connection's timer could settle the next connection's wait. Subscribing makes a current service send a clientState right away, so none at all by the deadline means a service older than the GUI. That is now a fatal handshake error telling the user to restart the service, rather than a mode that half works. --- shared/constants/init/shared.tsx | 28 +++++++-------------- shared/stores/tests/client-state.test.ts | 31 ++++++------------------ 2 files changed, 16 insertions(+), 43 deletions(-) diff --git a/shared/constants/init/shared.tsx b/shared/constants/init/shared.tsx index 80e3862bb419..807c67486de6 100644 --- a/shared/constants/init/shared.tsx +++ b/shared/constants/init/shared.tsx @@ -19,13 +19,13 @@ import {notifyEngineActionListeners} from '@/engine/action-listener' import {serviceStaticConfigToStaticConfig} from '@/constants/chat/static-config' import {emitDeepLink} from '@/router-v2/linking' import {ignorePromise, timeoutPromise} from '../utils' -import {isPhone, serverConfigFileName} from '../platform' +import {isLinux, isPhone, serverConfigFileName} from '../platform' import {useAvatarState} from '@/common-adapters/avatar/store' import {useInboxLayoutState} from '@/chat/inbox/layout-state' import {getPinnedConvIDs} from '@/chat/inbox/pinned-convs' import {useConfigState} from '@/stores/config' import {useCurrentUserState} from '@/stores/current-user' -import {useDaemonState, type BootstrapStep} from '@/stores/daemon' +import {FatalHandshakeError, useDaemonState, type BootstrapStep} from '@/stores/daemon' import {useDarkModeState} from '@/stores/darkmode' import {useFollowerState} from '@/stores/followers' import {useShellState} from '@/stores/shell' @@ -256,8 +256,8 @@ let settleSession = () => {} let sessionSettled = new Promise(resolve => { settleSession = resolve }) -// A service older than clientState never sends one. On Linux the GUI can be upgraded while such a -// service keeps running, until the user restarts it. +// Subscribing makes the service send a clientState, so none at all means a service older than +// clientState. On Linux the GUI can be upgraded while such a service keeps running. let clientStateSeen = false const awaitSessionAgain = () => { clientStateSeen = false @@ -285,12 +285,7 @@ export const applyClientState = (clientState: T.RPCGen.ClientState) => { logger.info('[Bootstrap] the service has not settled its startup login yet') return } - applySession(session) -} - -const applySession = (session: T.RPCGen.ClientSession) => { settleSession() - const configDispatch = useConfigState.getState().dispatch const {deviceID, deviceName, loggedIn, uid, username} = session if (!loggedIn) { // Session first: logging out resets the stores, the current user among them. Writing the empty @@ -354,16 +349,11 @@ export const sessionSettledStep = async () => { if (clientStateSeen) { throw error } - const bootstrapStatus = useDaemonState.getState().bootstrapStatus - if (!bootstrapStatus) { - throw error - } - logger.warn('[Bootstrap] the service sent no clientState, using its bootstrap status') - const {deviceID, deviceName, httpSrvInfo, loggedIn, uid, username} = bootstrapStatus - if (httpSrvInfo) { - useConfigState.getState().dispatch.setHTTPSrvInfo(httpSrvInfo.address, httpSrvInfo.token) - } - applySession({deviceID, deviceName, loggedIn, uid, username}) + throw new FatalHandshakeError( + isLinux + ? 'The Keybase service is out of date. Restart it with run_keybase.' + : 'The Keybase service is out of date. Restart Keybase.' + ) } finally { clearTimeout(timer) } diff --git a/shared/stores/tests/client-state.test.ts b/shared/stores/tests/client-state.test.ts index e433b3c61c4c..9290da9cbe6b 100644 --- a/shared/stores/tests/client-state.test.ts +++ b/shared/stores/tests/client-state.test.ts @@ -6,7 +6,6 @@ import {useCurrentUserState} from '../current-user' import {useShellState} from '../shell' import {_onEngineIncoming, applyClientState} from '@/constants/init/shared' import type * as ConfigModule from '../config' -import type * as CurrentUserModule from '../current-user' import type * as DaemonModule from '../daemon' import type * as SharedModule from '@/constants/init/shared' import type * as TypesModule from '@/constants/types' @@ -227,7 +226,6 @@ describe('an account switch', () => { describe('waiting for the session', () => { type Modules = { config: typeof ConfigModule - currentUser: typeof CurrentUserModule daemon: typeof DaemonModule shared: typeof SharedModule types: typeof TypesModule @@ -238,7 +236,6 @@ describe('waiting for the session', () => { jest.isolateModules(() => { m = { config: require('../config') as Modules['config'], - currentUser: require('../current-user') as Modules['currentUser'], daemon: require('../daemon') as Modules['daemon'], shared: require('@/constants/init/shared') as Modules['shared'], types: require('@/constants/types') as Modules['types'], @@ -246,19 +243,6 @@ describe('waiting for the session', () => { }) const modules = m! jest.spyOn(modules.types.RPCGen, 'notifyCtlSetNotificationsRpcPromise').mockResolvedValue(undefined) - modules.daemon.useDaemonState.setState({ - bootstrapStatus: { - deviceID: 'd1', - deviceName: 'testuser-mac', - fullname: '', - httpSrvInfo: {address: '127.0.0.1:2', token: 'token'}, - loggedIn: true, - registered: true, - uid: 'u1', - userReacjis: {skinTone: T.RPCGen.ReacjiSkinTone.none, topReacjis: []}, - username: 'testuser', - }, - }) return modules } @@ -269,24 +253,23 @@ describe('waiting for the session', () => { jest.useRealTimers() }) - test('a service that never sends a clientState is read from its bootstrap status', async () => { - const {config, currentUser, shared} = load() + test('a service that never sends a clientState is out of date, and retrying will not help', async () => { + const {daemon, shared} = load() const settled = shared.sessionSettledStep() + const failed = expect(settled).rejects.toBeInstanceOf(daemon.FatalHandshakeError) await jest.advanceTimersByTimeAsync(30_000) - await settled - - expect(config.useConfigState.getState().loggedIn).toBe(true) - expect(config.useConfigState.getState().httpSrv.address).toBe('127.0.0.1:2') - expect(currentUser.useCurrentUserState.getState().username).toBe('testuser') + await failed + await expect(settled).rejects.toThrow('out of date') }) test('a service that sent a clientState without a session is still waited on', async () => { - const {config, shared} = load() + const {config, daemon, shared} = load() const settled = shared.sessionSettledStep() const failed = expect(settled).rejects.toThrow("The service hasn't said who is logged in") shared.applyClientState({appState: T.RPCGen.MobileAppState.foreground}) await jest.advanceTimersByTimeAsync(30_000) await failed + await expect(settled).rejects.not.toBeInstanceOf(daemon.FatalHandshakeError) expect(config.useConfigState.getState().loggedIn).toBe(false) }) From a200da389736901bb975d022383ea82efb6fbfc1 Mon Sep 17 00:00:00 2001 From: chrisnojima Date: Tue, 22 Sep 2026 16:56:27 -0400 Subject: [PATCH 08/12] test(libkb): stop reading the dropped changed result of Update --- go/libkb/appstate_test.go | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/go/libkb/appstate_test.go b/go/libkb/appstate_test.go index 018449333bc7..ad45d961d1c0 100644 --- a/go/libkb/appstate_test.go +++ b/go/libkb/appstate_test.go @@ -101,9 +101,9 @@ func TestMobileAppStateAnnouncesOnlyOnChange(t *testing.T) { rec := NewNotifyRecorder(tc.G, keybase1.NotificationChannels{App: true}) defer rec.Close() - require.True(t, a.Update(keybase1.MobileAppState_BACKGROUND)) - require.False(t, a.Update(keybase1.MobileAppState_BACKGROUND)) - require.True(t, a.Update(keybase1.MobileAppState_FOREGROUND)) + a.Update(keybase1.MobileAppState_BACKGROUND) + a.Update(keybase1.MobileAppState_BACKGROUND) + a.Update(keybase1.MobileAppState_FOREGROUND) rec.Flush() require.Equal(t, []keybase1.MobileAppState{ keybase1.MobileAppState_BACKGROUND, @@ -125,14 +125,13 @@ func TestMobileAppStateQueuesUnderTheLock(t *testing.T) { defer rec.Close() a.Lock() - changed := a.updateLocked(keybase1.MobileAppState_BACKGROUND) + a.updateLocked(keybase1.MobileAppState_BACKGROUND) // nothing queued reads app state (there is no clientState reader here), so // flushing under the lock cannot deadlock rec.Flush() queued := appStateChanges(t, rec) a.Unlock() - require.True(t, changed) require.Equal(t, []keybase1.MobileAppState{keybase1.MobileAppState_BACKGROUND}, queued, "the change was queued before the lock that wrote it was released") } From ffec07813b85c5116c4c4189f5176572d8c8262a Mon Sep 17 00:00:00 2001 From: chrisnojima Date: Tue, 22 Sep 2026 16:59:10 -0400 Subject: [PATCH 09/12] test(js): cover the out-of-date service with the other session-wait tests --- shared/constants/init/shared.test.ts | 16 ++++++- shared/stores/tests/client-state.test.ts | 56 ------------------------ 2 files changed, 14 insertions(+), 58 deletions(-) diff --git a/shared/constants/init/shared.test.ts b/shared/constants/init/shared.test.ts index f275686779e7..f39ec7bd631c 100644 --- a/shared/constants/init/shared.test.ts +++ b/shared/constants/init/shared.test.ts @@ -3,7 +3,7 @@ import * as T from '@/constants/types' import {resetAllStores} from '@/util/zustand' import {ignorePromise} from '@/constants/utils' import {useConfigState} from '@/stores/config' -import {useDaemonState} from '@/stores/daemon' +import {FatalHandshakeError, useDaemonState} from '@/stores/daemon' import {useRouterState} from '@/stores/router' import {useShellState} from '@/stores/shell' import { @@ -283,13 +283,25 @@ describe('sessionSettledStep', () => { connect(async () => Promise.resolve()) const step = sessionSettledStep() const failed = expect(step).rejects.toThrow("The service hasn't said who is logged in") + applyClientState({appState: T.RPCGen.MobileAppState.foreground}) + await jest.advanceTimersByTimeAsync(30_000) + await failed + await expect(step).rejects.not.toBeInstanceOf(FatalHandshakeError) + }) + + test('a service that never sends a clientState is out of date, and retrying will not help', async () => { + jest.useFakeTimers() + connect(async () => Promise.resolve()) + const step = sessionSettledStep() + const failed = expect(step).rejects.toBeInstanceOf(FatalHandshakeError) await jest.advanceTimersByTimeAsync(30_000) await failed + await expect(step).rejects.toThrow('out of date') }) }) describe('onNetworkOnlineChanged', () => { - // replaces the gregor-reachability trigger: re-read the bootstrap status after an offline stretch + // re-reads the bootstrap status after an offline stretch afterEach(() => { jest.restoreAllMocks() useDaemonState.setState({dispatch: originalDaemonDispatch}) diff --git a/shared/stores/tests/client-state.test.ts b/shared/stores/tests/client-state.test.ts index 9290da9cbe6b..d46b295c0574 100644 --- a/shared/stores/tests/client-state.test.ts +++ b/shared/stores/tests/client-state.test.ts @@ -5,10 +5,6 @@ import {useConfigState} from '../config' import {useCurrentUserState} from '../current-user' import {useShellState} from '../shell' import {_onEngineIncoming, applyClientState} from '@/constants/init/shared' -import type * as ConfigModule from '../config' -import type * as DaemonModule from '../daemon' -import type * as SharedModule from '@/constants/init/shared' -import type * as TypesModule from '@/constants/types' const g = globalThis as unknown as {isMobile: boolean} @@ -222,55 +218,3 @@ describe('an account switch', () => { expect(accountStateCleared()).toBe(false) }) }) - -describe('waiting for the session', () => { - type Modules = { - config: typeof ConfigModule - daemon: typeof DaemonModule - shared: typeof SharedModule - types: typeof TypesModule - } - // a fresh module registry per test, so each starts with nothing settled on the connection - const load = () => { - let m: Modules | undefined - jest.isolateModules(() => { - m = { - config: require('../config') as Modules['config'], - daemon: require('../daemon') as Modules['daemon'], - shared: require('@/constants/init/shared') as Modules['shared'], - types: require('@/constants/types') as Modules['types'], - } - }) - const modules = m! - jest.spyOn(modules.types.RPCGen, 'notifyCtlSetNotificationsRpcPromise').mockResolvedValue(undefined) - return modules - } - - beforeEach(() => { - jest.useFakeTimers() - }) - afterEach(() => { - jest.useRealTimers() - }) - - test('a service that never sends a clientState is out of date, and retrying will not help', async () => { - const {daemon, shared} = load() - const settled = shared.sessionSettledStep() - const failed = expect(settled).rejects.toBeInstanceOf(daemon.FatalHandshakeError) - await jest.advanceTimersByTimeAsync(30_000) - await failed - await expect(settled).rejects.toThrow('out of date') - }) - - test('a service that sent a clientState without a session is still waited on', async () => { - const {config, daemon, shared} = load() - const settled = shared.sessionSettledStep() - const failed = expect(settled).rejects.toThrow("The service hasn't said who is logged in") - shared.applyClientState({appState: T.RPCGen.MobileAppState.foreground}) - await jest.advanceTimersByTimeAsync(30_000) - await failed - await expect(settled).rejects.not.toBeInstanceOf(daemon.FatalHandshakeError) - - expect(config.useConfigState.getState().loggedIn).toBe(false) - }) -}) From 176325dd822271e458ad29fdd973100bd56c6d57 Mon Sep 17 00:00:00 2001 From: chrisnojima Date: Tue, 22 Sep 2026 17:06:16 -0400 Subject: [PATCH 10/12] fix(js): keep a missing clientState retryable on mobile The mobile service runs in-process and is always the same build, so a clientState that is late there -- a stalled JS thread, a backed-up sender -- is not an out-of-date service. Only desktop, where the GUI can be upgraded under a running service, now fails the handshake for good. --- shared/constants/init/shared.test.ts | 17 +++++++++++++++++ shared/constants/init/shared.tsx | 7 ++++--- 2 files changed, 21 insertions(+), 3 deletions(-) diff --git a/shared/constants/init/shared.test.ts b/shared/constants/init/shared.test.ts index f39ec7bd631c..32752ddbb0f6 100644 --- a/shared/constants/init/shared.test.ts +++ b/shared/constants/init/shared.test.ts @@ -287,6 +287,7 @@ describe('sessionSettledStep', () => { await jest.advanceTimersByTimeAsync(30_000) await failed await expect(step).rejects.not.toBeInstanceOf(FatalHandshakeError) + expect(useConfigState.getState().loggedIn).toBe(false) }) test('a service that never sends a clientState is out of date, and retrying will not help', async () => { @@ -298,6 +299,22 @@ describe('sessionSettledStep', () => { await failed await expect(step).rejects.toThrow('out of date') }) + + test('on mobile no clientState is not an out-of-date service: the in-process service is the same build', async () => { + const g = globalThis as unknown as {isMobile: boolean} + g.isMobile = true + try { + jest.useFakeTimers() + connect(async () => Promise.resolve()) + const step = sessionSettledStep() + const failed = expect(step).rejects.toThrow("The service hasn't said who is logged in") + await jest.advanceTimersByTimeAsync(30_000) + await failed + await expect(step).rejects.not.toBeInstanceOf(FatalHandshakeError) + } finally { + g.isMobile = false + } + }) }) describe('onNetworkOnlineChanged', () => { diff --git a/shared/constants/init/shared.tsx b/shared/constants/init/shared.tsx index 807c67486de6..5fbf50547239 100644 --- a/shared/constants/init/shared.tsx +++ b/shared/constants/init/shared.tsx @@ -256,8 +256,8 @@ let settleSession = () => {} let sessionSettled = new Promise(resolve => { settleSession = resolve }) -// Subscribing makes the service send a clientState, so none at all means a service older than -// clientState. On Linux the GUI can be upgraded while such a service keeps running. +// Subscribing makes the service send a clientState, so on desktop none at all means a service +// older than clientState. On Linux the GUI can be upgraded while such a service keeps running. let clientStateSeen = false const awaitSessionAgain = () => { clientStateSeen = false @@ -346,7 +346,8 @@ export const sessionSettledStep = async () => { try { await Promise.race([sessionSettled, timedOut]) } catch (error) { - if (clientStateSeen) { + // the mobile service runs in-process, so it is always the same build + if (clientStateSeen || isMobile) { throw error } throw new FatalHandshakeError( From d4d99bf3092de3ae7857796f6f11791b33776e75 Mon Sep 17 00:00:00 2001 From: chrisnojima Date: Tue, 22 Sep 2026 17:31:13 -0400 Subject: [PATCH 11/12] fix(js): subscribe again when the session wait times out A retry awaited the same subscription, so the service was never asked for another clientState: a lost one left every retry waiting its full 30s. Resetting the subscription on a timeout makes the retry subscribe, which is what sends a fresh clientState. --- shared/constants/init/shared.test.ts | 17 +++++++++++++++++ shared/constants/init/shared.tsx | 5 ++++- 2 files changed, 21 insertions(+), 1 deletion(-) diff --git a/shared/constants/init/shared.test.ts b/shared/constants/init/shared.test.ts index 32752ddbb0f6..024ebfe95700 100644 --- a/shared/constants/init/shared.test.ts +++ b/shared/constants/init/shared.test.ts @@ -300,6 +300,23 @@ describe('sessionSettledStep', () => { await expect(step).rejects.toThrow('out of date') }) + test('a retry after the wait timed out subscribes again, so a lost clientState is sent afresh', async () => { + jest.useFakeTimers() + const setNotifications = connect(async () => Promise.resolve()) + const first = sessionSettledStep() + const failed = expect(first).rejects.toThrow("The service hasn't said who is logged in") + applyClientState({appState: T.RPCGen.MobileAppState.foreground}) + await jest.advanceTimersByTimeAsync(30_000) + await failed + expect(setNotifications).toHaveBeenCalledTimes(1) + + const retry = sessionSettledStep() + await jest.advanceTimersByTimeAsync(0) + expect(setNotifications).toHaveBeenCalledTimes(2) + applyClientState({appState: T.RPCGen.MobileAppState.foreground, session}) + await expect(retry).resolves.toBeUndefined() + }) + test('on mobile no clientState is not an out-of-date service: the in-process service is the same build', async () => { const g = globalThis as unknown as {isMobile: boolean} g.isMobile = true diff --git a/shared/constants/init/shared.tsx b/shared/constants/init/shared.tsx index 5fbf50547239..8cfce59329c4 100644 --- a/shared/constants/init/shared.tsx +++ b/shared/constants/init/shared.tsx @@ -331,7 +331,7 @@ const subscribe = async () => { let subscription = Promise.resolve(false) // A handshake step: the session is what decides between the login screen and the app. A failed -// subscribe is retried here, since without it no clientState is coming. +// subscribe, or a wait that timed out, subscribes again here, since that is what sends a clientState. export const sessionSettledStep = async () => { if (!(await subscription)) { subscription = subscribe() @@ -346,6 +346,9 @@ export const sessionSettledStep = async () => { try { await Promise.race([sessionSettled, timedOut]) } catch (error) { + // Subscribing again makes the service send a fresh clientState, so the retry isn't waiting + // on one that was lost. + subscription = Promise.resolve(false) // the mobile service runs in-process, so it is always the same build if (clientStateSeen || isMobile) { throw error From 268b7ce267ee21b6ced1196711f0b53e1ee6f380 Mon Sep 17 00:00:00 2001 From: chrisnojima Date: Tue, 22 Sep 2026 17:44:07 -0400 Subject: [PATCH 12/12] fix(js): a stale session wait leaves a newer connection's subscription alone The timeout's subscription reset also ran for a step left over from a superseded connection, overwriting the subscription the new connection had just made, so its next retry subscribed a second time. The reset now applies only while the subscription is still the one the step waited on. --- shared/constants/init/shared.test.ts | 19 +++++++++++++++++++ shared/constants/init/shared.tsx | 7 +++++-- 2 files changed, 24 insertions(+), 2 deletions(-) diff --git a/shared/constants/init/shared.test.ts b/shared/constants/init/shared.test.ts index 024ebfe95700..ea181961c06c 100644 --- a/shared/constants/init/shared.test.ts +++ b/shared/constants/init/shared.test.ts @@ -317,6 +317,25 @@ describe('sessionSettledStep', () => { await expect(retry).resolves.toBeUndefined() }) + test("a superseded connection's timeout leaves the new connection's subscription alone", async () => { + jest.useFakeTimers() + connect(async () => Promise.resolve()) + const stale = sessionSettledStep() + const failed = expect(stale).rejects.toThrow() + await jest.advanceTimersByTimeAsync(20_000) + + const setNotifications = connect(async () => Promise.resolve()) + const subscribes = setNotifications.mock.calls.length + await jest.advanceTimersByTimeAsync(10_000) + await failed + + const step = sessionSettledStep() + await jest.advanceTimersByTimeAsync(0) + expect(setNotifications).toHaveBeenCalledTimes(subscribes) + applyClientState({appState: T.RPCGen.MobileAppState.foreground, session}) + await expect(step).resolves.toBeUndefined() + }) + test('on mobile no clientState is not an out-of-date service: the in-process service is the same build', async () => { const g = globalThis as unknown as {isMobile: boolean} g.isMobile = true diff --git a/shared/constants/init/shared.tsx b/shared/constants/init/shared.tsx index 8cfce59329c4..2690ea2b5fb0 100644 --- a/shared/constants/init/shared.tsx +++ b/shared/constants/init/shared.tsx @@ -339,6 +339,7 @@ export const sessionSettledStep = async () => { throw new Error("Can't subscribe to the service's notifications") } } + const subscribed = subscription let timer: ReturnType | undefined const timedOut = new Promise((_resolve, reject) => { timer = setTimeout(() => reject(new Error("The service hasn't said who is logged in")), sessionWaitMs) @@ -347,8 +348,10 @@ export const sessionSettledStep = async () => { await Promise.race([sessionSettled, timedOut]) } catch (error) { // Subscribing again makes the service send a fresh clientState, so the retry isn't waiting - // on one that was lost. - subscription = Promise.resolve(false) + // on one that was lost. A newer connection has subscribed on its own, so leave that alone. + if (subscription === subscribed) { + subscription = Promise.resolve(false) + } // the mobile service runs in-process, so it is always the same build if (clientStateSeen || isMobile) { throw error