small chat 3

.agents/skills/react-native-best-practices/SKILL.md

@@ -1,6 +1,6 @@
 ---
 name: react-native-best-practices
-description: "Software Mansion's best practices for production React Native and Expo apps on the New Architecture. MUST USE before writing, reviewing, or debugging ANY code in a React Native or Expo project. If the working directory contains a package.json with react-native, expo, or expo-router as a dependency, this skill applies. Trigger on: any code task in a React Native/Expo project, 'React Native', 'Expo', 'New Architecture', 'Reanimated', 'Gesture Handler', 'react-native-svg', 'ExecuTorch', 'react-native-audio-api', 'react-native-enriched', 'Worklet', 'Fabric', 'TurboModule', 'WebGPU', 'react-native-wgpu', 'TypeGPU', 'GPU shader', 'WGSL', 'svg', 'animation', 'gesture', 'audio', 'rich text', 'AI model', 'multithreading', 'chart', 'vector', 'image filter', 'shared value', 'useSharedValue', 'runOnJS', 'scheduleOnRN', 'thread', 'worklet', or any question involving UI, graphics, native modules, or React Native threading and animation behavior. Also use when a more specific sub-skill matches."
+description: "Software Mansion's best practices for production React Native and Expo apps on the New Architecture. Trigger on: any code task in a React Native/Expo project, 'React Native', 'Expo', 'New Architecture', 'Reanimated', 'Gesture Handler', 'react-native-svg', 'ExecuTorch', 'react-native-audio-api', 'react-native-enriched', 'Worklet', 'Fabric', 'TurboModule', 'WebGPU', 'react-native-wgpu', 'TypeGPU', 'GPU shader', 'WGSL', 'svg', 'animation', 'gesture', 'audio', 'rich text', 'AI model', 'multithreading', 'chart', 'vector', 'image filter', 'shared value', 'useSharedValue', 'runOnJS', 'scheduleOnRN', 'thread', 'worklet', or any question involving UI, graphics, native modules, or React Native threading and animation behavior. Also use when a more specific sub-skill matches."
 license: MIT
 ---
 

AGENTS.md

@@ -8,7 +8,6 @@
 - Keep types accurate. Do not use casts or misleading annotations to mask a real type mismatch just to get around an issue; fix the type or fix the implementation.
 - Do not add new exported functions, types, or constants unless they are required outside the file. Prefer file-local helpers for one-off implementation details and tests.
 - Do not edit lockfiles by hand. They are generated artifacts. If you cannot regenerate one locally, leave it unchanged.
-- Do not use `navigation.setOptions` for header state in this repo. Pass header-driving state through route params so `getOptions` can read it synchronously, or use [`shared/stores/modal-header.tsx`](/Users/ChrisNojima/SourceCode/go/src/github.com/keybase/client/shared/stores/modal-header.tsx) when the flow already uses the shared modal header mechanism.
 - Components must not mutate Zustand stores directly with `useXState.setState`, `getState()`-based writes, or similar ad hoc store mutation. If a component needs to affect store state, route it through a store dispatch action or move the state out of the store.
 - During refactors, do not delete existing guards, conditionals, or platform/test-specific behavior unless you have proven they are dead and the user asked for that behavior change. Port checks like `androidIsTestDevice` forward into the new code path instead of silently dropping them.
 - When addressing PR or review feedback, including bot or lint-style suggestions, do not apply it mechanically. Verify that the reported issue is real in this codebase and that the proposed fix is consistent with repo rules and improves correctness, behavior, or maintainability before making changes.

go/chat/search/deadlock_test.go

@@ -0,0 +1,234 @@
+package search
+
+import (
+	"context"
+	"sync"
+	"testing"
+	"time"
+
+	"github.com/keybase/client/go/chat/globals"
+	"github.com/keybase/client/go/externalstest"
+	"github.com/keybase/client/go/kbtest"
+	"github.com/keybase/client/go/protocol/chat1"
+	"github.com/keybase/client/go/protocol/gregor1"
+	"github.com/stretchr/testify/require"
+)
+
+type deadlockTestDiskStorage struct {
+	clearEntered chan struct{}
+	clearRelease chan struct{}
+}
+
+func (d *deadlockTestDiskStorage) GetTokenEntry(ctx context.Context, convID chat1.ConversationID,
+	token string,
+) (res *tokenEntry, err error) {
+	return nil, nil
+}
+
+func (d *deadlockTestDiskStorage) PutTokenEntry(ctx context.Context, convID chat1.ConversationID,
+	token string, te *tokenEntry,
+) error {
+	return nil
+}
+
+func (d *deadlockTestDiskStorage) RemoveTokenEntry(ctx context.Context, convID chat1.ConversationID, token string) {
+}
+
+func (d *deadlockTestDiskStorage) GetAliasEntry(ctx context.Context, alias string) (res *aliasEntry, err error) {
+	return nil, nil
+}
+
+func (d *deadlockTestDiskStorage) PutAliasEntry(ctx context.Context, alias string, ae *aliasEntry) error {
+	return nil
+}
+
+func (d *deadlockTestDiskStorage) RemoveAliasEntry(ctx context.Context, alias string) {}
+
+func (d *deadlockTestDiskStorage) GetMetadata(ctx context.Context, convID chat1.ConversationID) (res *indexMetadata, err error) {
+	return nil, nil
+}
+
+func (d *deadlockTestDiskStorage) PutMetadata(ctx context.Context, convID chat1.ConversationID, md *indexMetadata) error {
+	return nil
+}
+
+func (d *deadlockTestDiskStorage) Clear(ctx context.Context, uid gregor1.UID, convID chat1.ConversationID) error {
+	if d.clearEntered != nil {
+		select {
+		case d.clearEntered <- struct{}{}:
+		default:
+		}
+	}
+	if d.clearRelease != nil {
+		select {
+		case <-d.clearRelease:
+		case <-ctx.Done():
+			return ctx.Err()
+		}
+	}
+	return nil
+}
+
+type blockingGetMsgsChatHelper struct {
+	*kbtest.MockChatHelper
+	calledCh  chan struct{}
+	releaseCh chan struct{}
+}
+
+func (h *blockingGetMsgsChatHelper) GetMessages(ctx context.Context, uid gregor1.UID,
+	convID chat1.ConversationID, msgIDs []chat1.MessageID,
+	resolveSupersedes bool, reason *chat1.GetThreadReason,
+) ([]chat1.MessageUnboxed, error) {
+	select {
+	case h.calledCh <- struct{}{}:
+	default:
+	}
+	select {
+	case <-h.releaseCh:
+	case <-ctx.Done():
+		return nil, ctx.Err()
+	}
+	return nil, nil
+}
+
+func setupDeadlockTestStore(t *testing.T) (*globals.Context, *store) {
+	tc := externalstest.SetupTest(t, "search-deadlock", 2)
+	t.Cleanup(tc.Cleanup)
+	g := globals.NewContext(tc.G, &globals.ChatContext{})
+	uid := gregor1.UID([]byte{1, 2, 3, 4})
+	s := newStore(g, uid)
+	s.diskStorage = &deadlockTestDiskStorage{}
+	return g, s
+}
+
+func TestSearchDeadlockRegression(t *testing.T) {
+	t.Run("store add releases lock before superseded fetch", func(t *testing.T) {
+		ctx := context.TODO()
+		g, s := setupDeadlockTestStore(t)
+		calledCh := make(chan struct{}, 1)
+		releaseCh := make(chan struct{})
+		var releaseOnce sync.Once
+		releaseFetch := func() {
+			releaseOnce.Do(func() {
+				close(releaseCh)
+			})
+		}
+		t.Cleanup(releaseFetch)
+		g.ExternalG().ChatHelper = &blockingGetMsgsChatHelper{
+			MockChatHelper: kbtest.NewMockChatHelper(),
+			calledCh:       calledCh,
+			releaseCh:      releaseCh,
+		}
+
+		convID := chat1.ConversationID([]byte{
+			1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16,
+		})
+		editMsg := chat1.NewMessageUnboxedWithValid(chat1.MessageUnboxedValid{
+			ClientHeader: chat1.MessageClientHeaderVerified{
+				MessageType: chat1.MessageType_EDIT,
+				Conv: chat1.ConversationIDTriple{
+					TopicType: chat1.TopicType_CHAT,
+				},
+			},
+			MessageBody: chat1.NewMessageBodyWithEdit(chat1.MessageEdit{
+				MessageID: 1,
+				Body:      "hello world",
+			}),
+			ServerHeader: chat1.MessageServerHeader{
+				MessageID: 2,
+			},
+		})
+
+		addDone := make(chan error, 1)
+		go func() {
+			addDone <- s.Add(ctx, convID, []chat1.MessageUnboxed{editMsg})
+		}()
+
+		select {
+		case <-calledCh:
+		case <-time.After(10 * time.Second):
+			require.Fail(t, "store.Add never reached GetMessages")
+		}
+
+		clearDone := make(chan struct{})
+		go func() {
+			s.ClearMemory()
+			close(clearDone)
+		}()
+
+		select {
+		case <-clearDone:
+		case <-time.After(5 * time.Second):
+			releaseFetch()
+			require.Fail(t, "store.Add held s.Lock while blocked in GetMessages")
+		}
+
+		releaseFetch()
+		select {
+		case err := <-addDone:
+			require.NoError(t, err)
+		case <-time.After(10 * time.Second):
+			require.Fail(t, "store.Add never completed after GetMessages was released")
+		}
+	})
+
+	t.Run("indexer clear releases lock while storage clear is blocked", func(t *testing.T) {
+		ctx := context.TODO()
+		tc := externalstest.SetupTest(t, "indexer-clear-lock", 2)
+		t.Cleanup(tc.Cleanup)
+		g := globals.NewContext(tc.G, &globals.ChatContext{})
+		uid := gregor1.UID([]byte{9, 8, 7, 6})
+		convID := chat1.ConversationID([]byte{
+			16, 15, 14, 13, 12, 11, 10, 9, 8, 7, 6, 5, 4, 3, 2, 1,
+		})
+
+		idx := NewIndexer(g)
+		idx.SetUID(uid)
+		ds := &deadlockTestDiskStorage{
+			clearEntered: make(chan struct{}, 1),
+			clearRelease: make(chan struct{}),
+		}
+		var releaseOnce sync.Once
+		releaseClear := func() {
+			releaseOnce.Do(func() {
+				close(ds.clearRelease)
+			})
+		}
+		t.Cleanup(releaseClear)
+		idx.store.diskStorage = ds
+		idx.started = true
+
+		clearDone := make(chan error, 1)
+		go func() {
+			clearDone <- idx.Clear(ctx, uid, convID)
+		}()
+
+		select {
+		case <-ds.clearEntered:
+		case <-time.After(10 * time.Second):
+			require.Fail(t, "Indexer.Clear never reached diskStorage.Clear")
+		}
+
+		suspendDone := make(chan struct{})
+		go func() {
+			idx.Suspend(ctx)
+			close(suspendDone)
+		}()
+
+		select {
+		case <-suspendDone:
+		case <-time.After(5 * time.Second):
+			releaseClear()
+			require.Fail(t, "Indexer.Clear held idx.Lock while blocked in diskStorage.Clear")
+		}
+		idx.Resume(ctx)
+
+		releaseClear()
+		select {
+		case err := <-clearDone:
+			require.NoError(t, err)
+		case <-time.After(10 * time.Second):
+			require.Fail(t, "Indexer.Clear never completed after diskStorage.Clear was released")
+		}
+	})
+}

go/chat/search/indexer.go

@@ -868,8 +868,12 @@ func (idx *Indexer) PercentIndexed(ctx context.Context, convID chat1.Conversatio
 func (idx *Indexer) Clear(ctx context.Context, uid gregor1.UID, convID chat1.ConversationID) (err error) {
 	defer idx.Trace(ctx, &err, "Indexer.Clear uid: %v convID: %v", uid, convID)()
 	idx.Lock()
-	defer idx.Unlock()
-	return idx.store.Clear(ctx, uid, convID)
+	store := idx.store
+	idx.Unlock()
+	if store == nil {
+		return nil
+	}
+	return store.Clear(ctx, uid, convID)
 }
 
 func (idx *Indexer) OnDbNuke(mctx libkb.MetaContext) (err error) {