Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
140 changes: 140 additions & 0 deletions src/app/(app)/__tests__/init-session-generation.test.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,140 @@
/**
* Signing out while app initialization is still awaiting must retire that run: a stale
* invocation may not mark the app initialized, connect the chat hub, or restart location
* tracking that the sign-out cleanup just stopped.
*
* The layout itself pulls in Mapbox, Novu, push notifications and the whole store graph,
* so the guard protocol is exercised through the same generation-token shape the layout
* uses rather than by rendering it.
*/
import { act, renderHook } from '@testing-library/react-native';
import React from 'react';

interface Deferred {
promise: Promise<void>;
resolve: () => void;
}

function deferred(): Deferred {
let resolve: () => void = () => undefined;
const promise = new Promise<void>((res) => {
resolve = res;
});
return { promise, resolve };
}

/** Mirrors the layout's initializeApp guard: generation captured at start, checked after each await. */
function useInitGuard(gate: Deferred, effects: { connectHub: jest.Mock; startLocation: jest.Mock; markInitialized: jest.Mock }) {
const initGeneration = React.useRef(0);
const isInitializing = React.useRef(false);

const initialize = React.useCallback(async () => {
if (isInitializing.current) return;
isInitializing.current = true;
const generation = (initGeneration.current += 1);
const isCurrentRun = () => initGeneration.current === generation;

try {
await gate.promise;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

kody code-review Kody Rules high

Unhandled rejection from await gate.promise inside a try/finally block lacking a catch clause. Add a catch clause or wrap the await in its own try/catch before the finally cleanup to handle rejections explicitly.

Kody rule violation: Handle async operations with proper error handling

Prompt for LLM

File src/app/(app)/__tests__/init-session-generation.test.tsx:

Line 38:

Unhandled rejection from `await gate.promise` inside a try/finally block lacking a catch clause. Add a catch clause or wrap the await in its own try/catch before the finally cleanup to handle rejections explicitly.

Talk to Kody by mentioning @kody

Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.

if (!isCurrentRun()) return;

effects.connectHub();
if (!isCurrentRun()) return;

effects.markInitialized();
if (!isCurrentRun()) return;

effects.startLocation();
} finally {
if (isCurrentRun()) {
isInitializing.current = false;
}
}
}, [gate, effects]);

const signOut = React.useCallback(() => {
initGeneration.current += 1;
isInitializing.current = false;
}, []);

return { initialize, signOut, isInitializing };
}

describe('app initialization session generation', () => {
const effects = { connectHub: jest.fn(), startLocation: jest.fn(), markInitialized: jest.fn() };

beforeEach(() => {
jest.clearAllMocks();
});

it('abandons an in-flight run when the session ends mid-initialization', async () => {
const gate = deferred();
const { result } = renderHook(() => useInitGuard(gate, effects));

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

Unmount every hook instance.

Destructure unmount from each renderHook result. Call it after assertions. This prevents hook state from remaining mounted between tests.

As per coding guidelines, “Always call unmount() in tests to clean up.”

Also applies to: 96-96, 115-115

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/app/`(app)/__tests__/init-session-generation.test.tsx at line 72, Update
each renderHook invocation in the tests using useInitGuard to destructure its
unmount function, including the instances at the referenced locations. Call
every unmount after the assertions so all hook instances are cleaned up before
each test completes.

Source: Coding guidelines


let pending: Promise<void> = Promise.resolve();
act(() => {
pending = result.current.initialize();
});

// Sign-out lands while initialization is still awaiting its first step.
act(() => {
result.current.signOut();
});

await act(async () => {
gate.resolve();
await pending;
});

expect(effects.connectHub).not.toHaveBeenCalled();
expect(effects.markInitialized).not.toHaveBeenCalled();
expect(effects.startLocation).not.toHaveBeenCalled();
});

it('completes normally when the session survives', async () => {
const gate = deferred();
const { result } = renderHook(() => useInitGuard(gate, effects));

let pending: Promise<void> = Promise.resolve();
act(() => {
pending = result.current.initialize();
});

await act(async () => {
gate.resolve();
await pending;
});

expect(effects.connectHub).toHaveBeenCalledTimes(1);
expect(effects.markInitialized).toHaveBeenCalledTimes(1);
expect(effects.startLocation).toHaveBeenCalledTimes(1);
});

it('frees the in-progress guard so the next sign-in can initialize', async () => {
const first = deferred();
const { result } = renderHook(() => useInitGuard(first, effects));

let pending: Promise<void> = Promise.resolve();
act(() => {
pending = result.current.initialize();
});
act(() => {
result.current.signOut();
});

// The new session starts before the retired run has settled.
let second: Promise<void> = Promise.resolve();
act(() => {
second = result.current.initialize();
});

await act(async () => {
first.resolve();
await Promise.all([pending, second]);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

kody code-review Kody Rules low

Promise.all fails fast on the first rejection, discarding results for independent tasks. Use Promise.allSettled and handle per-item results to support partial failures in batch operations.

Kody rule violation: Use Promise.allSettled for batch operations with partial failures

Prompt for LLM

File src/app/(app)/__tests__/init-session-generation.test.tsx:

Line 133:

`Promise.all` fails fast on the first rejection, discarding results for independent tasks. Use `Promise.allSettled` and handle per-item results to support partial failures in batch operations.

Talk to Kody by mentioning @kody

Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.

});

// Exactly one run reached the effects: the current one.
expect(effects.markInitialized).toHaveBeenCalledTimes(1);
expect(effects.startLocation).toHaveBeenCalledTimes(1);
});
});
34 changes: 31 additions & 3 deletions src/app/(app)/_layout.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,10 @@ export default function TabLayout() {
// Refs to track initialization state
const hasInitialized = useRef(false);
const isInitializing = useRef(false);
// Bumped on every initialization start and whenever the session ends. An in-flight run
// compares its captured value after each await, so a run belonging to a session that is
// over can no longer connect hubs or mark the app initialized.
const initGeneration = useRef(0);
const hasHiddenSplash = useRef(false);
const parentRef = useRef(null);

Expand Down Expand Up @@ -151,6 +155,8 @@ export default function TabLayout() {
}

isInitializing.current = true;
const generation = (initGeneration.current += 1);
const isCurrentRun = () => initGeneration.current === generation;
logger.info({
message: 'Starting app initialization',
context: {
Expand All @@ -167,10 +173,14 @@ export default function TabLayout() {
// time-to-interactive (previously 8+ serial network hops).
await Promise.all([useRolesStore.getState().init(), useCallsStore.getState().init(), useWeatherAlertsStore.getState().init(), securityStore.getState().getRights(), featureFlagsStore.getState().fetchFlags()]);

if (!isCurrentRun()) return;

// SignalR needs config (EventingUrl) and rights (DepartmentId) — both
// available now. The two hub connects are independent.
await Promise.all([useSignalRStore.getState().connectUpdateHub(), useSignalRStore.getState().connectGeolocationHub()]);

if (!isCurrentRun()) return;

// Connect the realtime chat hub only when the Chat.System feature flag is on for
// this department; when it is off every chat surface stays hidden.
if (featureFlagsStore.getState().isEnabled(FeatureFlagKeys.ChatSystem)) {
Expand All @@ -189,6 +199,8 @@ export default function TabLayout() {
});
}

if (!isCurrentRun()) return;

hasInitialized.current = true;

// Evict expired/capped API cache entries once per cold start.
Expand Down Expand Up @@ -223,12 +235,20 @@ export default function TabLayout() {
context: { error },
});
}
// A run whose session already ended must not burn the retry budget or clobber
// state a newer run has since established.
if (!isCurrentRun()) return;

// Reset initialization state on error so it can be retried
hasInitialized.current = false;
setInitRetryCount((c) => c + 1);
} finally {
isInitializing.current = false;
setIsInitComplete(true);
// Only the current run owns the guard; a superseded run clearing it would let two
// initializations overlap.
if (isCurrentRun()) {
isInitializing.current = false;
setIsInitComplete(true);
}
}
}, [status]);

Expand Down Expand Up @@ -270,7 +290,15 @@ export default function TabLayout() {
// Handle app initialization - simplified logic
const MAX_INIT_RETRIES = 3;
useEffect(() => {
if (status !== 'signedIn' && initRetryCount > 0) {
if (status === 'signedIn') return;

// Leaving the signed-in state retires any initialization still in flight, and frees
// the guard it no longer owns so the next sign-in is not skipped as "already
// initializing".
initGeneration.current += 1;
isInitializing.current = false;

if (initRetryCount > 0) {
Comment on lines +293 to +301

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Reset completed initialization state on sign-out.

When TabLayout remains mounted through a sign-out and sign-in, hasInitialized.current remains true. Line 313 then skips initializeApp for the new session. isInitComplete also remains true, so the UI can render as initialized before the new session setup completes.

Proposed fix
     initGeneration.current += 1;
     isInitializing.current = false;
+    hasInitialized.current = false;
+    setIsInitComplete(false);

     if (initRetryCount > 0) {
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
if (status === 'signedIn') return;
// Leaving the signed-in state retires any initialization still in flight, and frees
// the guard it no longer owns so the next sign-in is not skipped as "already
// initializing".
initGeneration.current += 1;
isInitializing.current = false;
if (initRetryCount > 0) {
if (status === 'signedIn') return;
// Leaving the signed-in state retires any initialization still in flight, and frees
// the guard it no longer owns so the next sign-in is not skipped as "already
// initializing".
initGeneration.current += 1;
isInitializing.current = false;
hasInitialized.current = false;
setIsInitComplete(false);
if (initRetryCount > 0) {
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/app/`(app)/_layout.tsx around lines 293 - 301, Reset the
completed-initialization state when the auth status leaves the signed-in state:
set hasInitialized.current and isInitComplete to their uninitialized values
alongside the existing initGeneration and isInitializing cleanup. Keep the
existing retry reset and signed-in early return behavior unchanged so a
subsequent sign-in runs initializeApp again and does not render as initialized
prematurely.

setInitRetryCount(0);
}
}, [status, initRetryCount]);
Expand Down
52 changes: 13 additions & 39 deletions src/app/(app)/chatbot.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,7 @@ import { copyToClipboard } from '@/components/chat/chat-utils';
import { MessageActionsSheet } from '@/components/chat/message-actions-sheet';
import { MessageBubble } from '@/components/chat/message-bubble';
import { TypingDots } from '@/components/chat/typing-indicator';
import { Actionsheet, ActionsheetBackdrop, ActionsheetContent, ActionsheetDragIndicator, ActionsheetDragIndicatorWrapper } from '@/components/ui/actionsheet';
import { Box } from '@/components/ui/box';
import { Button, ButtonText } from '@/components/ui/button';
import { Center } from '@/components/ui/center';
import { FlatList } from '@/components/ui/flat-list';
import { FocusAwareStatusBar } from '@/components/ui/focus-aware-status-bar';
Expand All @@ -20,7 +18,6 @@ import { KeyboardAvoidingView } from '@/components/ui/keyboard-avoiding-view';
import { Pressable } from '@/components/ui/pressable';
import { Spinner } from '@/components/ui/spinner';
import { Text } from '@/components/ui/text';
import { Textarea, TextareaInput } from '@/components/ui/textarea';
import { VStack } from '@/components/ui/vstack';
import { type ChatMessageResultData } from '@/models/v4/chat';
import useAuthStore from '@/stores/auth/store';
Expand All @@ -40,8 +37,6 @@ export default function ChatbotScreen() {
const isModerator = !!securityStore((s) => s.rights)?.IsAdmin;
const [text, setText] = useState('');
const [actionsMessage, setActionsMessage] = useState<ChatMessageResultData | null>(null);
const [editMessage, setEditMessage] = useState<ChatMessageResultData | null>(null);
const [editText, setEditText] = useState('');

useFocusEffect(
useCallback(() => {
Expand Down Expand Up @@ -72,7 +67,14 @@ export default function ChatbotScreen() {

const renderItem = useCallback(
({ item }: { item: ChatMessageResultData }) => (
<MessageBubble message={item} isOwn={!!item.SenderUserId && item.SenderUserId === currentUserId} showSender={false} currentUserId={currentUserId} onLongPress={setActionsMessage} onToggleReaction={() => undefined} />
<MessageBubble
message={item}
isOwn={!!item.SenderUserId && item.SenderUserId === currentUserId}
showSender={false}
currentUserId={currentUserId}
onLongPress={setActionsMessage}
onToggleReaction={() => undefined}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

kody code-review Kody Rules high

Performance impact from inline arrow functions in JSX props, which create new function instances on every render. Move the onToggleReaction definition outside the render method.

Kody rule violation: Avoid using .bind() or arrow functions in JSX props

Prompt for LLM

File src/app/(app)/chatbot.tsx:

Line 76:

Performance impact from inline arrow functions in JSX props, which create new function instances on every render. Move the `onToggleReaction` definition outside the render method.

Talk to Kody by mentioning @kody

Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.

/>
),
[currentUserId]
);
Expand Down Expand Up @@ -122,7 +124,9 @@ export default function ChatbotScreen() {
) : (
<FlatList
data={ordered}
maintainVisibleContentPosition={{ startRenderingFromBottom: true }}
// autoscrollToBottomThreshold is off by default in FlashList v2; without it a
// new answer lands below the viewport, hidden behind the input row.
maintainVisibleContentPosition={{ startRenderingFromBottom: true, autoscrollToBottomThreshold: 0.2 }}
keyExtractor={(item: ChatMessageResultData) => item.ChatMessageId}
renderItem={renderItem}
contentContainerStyle={{ paddingVertical: 8 }}
Expand Down Expand Up @@ -150,7 +154,7 @@ export default function ChatbotScreen() {
</HStack>
</KeyboardAvoidingView>

{/* Restricted actions for assistant messages: copy, edit own, pin (moderator), flag. */}
{/* Restricted actions for assistant messages: copy, pin (moderator), flag. */}
<MessageActionsSheet
message={actionsMessage}
isOpen={actionsMessage !== null}
Expand All @@ -164,42 +168,12 @@ export default function ChatbotScreen() {
const ok = await copyToClipboard(m.Body ?? '');
useToastStore.getState().showToast(ok ? 'success' : 'info', ok ? t('chat.copied') : t('chat.copy_unavailable'));
}}
onEdit={(m) => {
setEditMessage(m);
setEditText(m.Body ?? '');
}}
onEdit={() => undefined}
onDelete={() => undefined}
onFlag={(m, reason) => useChatStore.getState().flagMessage(m.ChatMessageId, reason)}
onTogglePin={(m, pinned) => chatbotChannelId && useChatStore.getState().togglePin(m.ChatMessageId, chatbotChannelId, pinned)}
onModeratorDelete={() => undefined}
/>

{/* Edit own message */}
<Actionsheet isOpen={editMessage !== null} onClose={() => setEditMessage(null)}>
<ActionsheetBackdrop />
<ActionsheetContent>
<ActionsheetDragIndicatorWrapper>
<ActionsheetDragIndicator />
</ActionsheetDragIndicatorWrapper>
<VStack className="w-full p-2" space="md">
<Text className="text-base font-semibold text-typography-900">{t('chat.edit_message')}</Text>
<Textarea>
<TextareaInput value={editText} onChangeText={setEditText} multiline />
</Textarea>
<Button
className="bg-primary-600"
onPress={() => {
if (editMessage && chatbotChannelId && editText.trim()) {
void useChatStore.getState().editMessage(editMessage.ChatMessageId, chatbotChannelId, editText.trim());
}
setEditMessage(null);
}}
>
<ButtonText>{t('chat.save')}</ButtonText>
</Button>
</VStack>
</ActionsheetContent>
</Actionsheet>
</Box>
);
}
8 changes: 7 additions & 1 deletion src/app/_layout.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -50,9 +50,15 @@ const navigationIntegration = Sentry.reactNavigationIntegration({
enableTimeToInitialDisplay: false,
});

// Sentry's own logger is off by default: watchdog-termination tracking rewrites the
// native scope on every RNSentry turbo-module call, so `debug` floods the Metro
// console with hundreds of "Writing tags to disk" lines a second. Flip to `__DEV__`
// temporarily when diagnosing Sentry itself.
const SENTRY_DEBUG = false;

Sentry.init({
dsn: Env.SENTRY_DSN,
debug: __DEV__, // Only debug in development, not production
debug: SENTRY_DEBUG,
tracesSampleRate: __DEV__ ? 0.1 : 0.2, // 10% in dev (low to avoid setTimeout wrapping overhead), 20% in production
profilesSampleRate: __DEV__ ? 0.1 : 0.2, // 10% in dev, 20% in production
sendDefaultPii: false,
Expand Down
12 changes: 7 additions & 5 deletions src/app/chat/[channelId].tsx
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import { Platform } from 'react-native';

import { getPresence, uploadAttachment } from '@/api/chat/chat';
import { AckBanner } from '@/components/chat/ack-banner';
import { copyToClipboard, getChannelDisplayName, getImageMimeType } from '@/components/chat/chat-utils';
import { buildGifMetadata, buildLocationMetadata, copyToClipboard, getChannelDisplayName, getImageMimeType } from '@/components/chat/chat-utils';
import { GifPickerSheet } from '@/components/chat/gif-picker-sheet';
import { MessageActionsSheet } from '@/components/chat/message-actions-sheet';
import { MessageBubble } from '@/components/chat/message-bubble';
Expand Down Expand Up @@ -155,7 +155,7 @@ export default function ChannelConversationScreen() {
const handleSendGif = useCallback(
(gif: GifResultData) => {
if (!channelId) return;
const metadata = JSON.stringify({ GifUrl: gif.GifUrl, PreviewUrl: gif.PreviewUrl, Width: gif.Width, Height: gif.Height, Title: gif.Title });
const metadata = buildGifMetadata(gif);
void useChatStore.getState().sendMessage({ channelId, body: gif.Title ?? 'GIF', messageType: ChatMessageType.Gif, metadataJson: metadata });
},
[channelId]
Expand All @@ -164,7 +164,7 @@ export default function ChannelConversationScreen() {
const handleSendLocation = useCallback(
(latitude: number, longitude: number, urgent: boolean) => {
if (!channelId) return;
const metadata = JSON.stringify({ Latitude: latitude, Longitude: longitude });
const metadata = buildLocationMetadata(latitude, longitude);
void useChatStore.getState().sendMessage({
channelId,
body: t('chat.shared_location'),
Expand Down Expand Up @@ -312,7 +312,9 @@ export default function ChannelConversationScreen() {
) : (
<FlatList
data={ordered}
maintainVisibleContentPosition={{ startRenderingFromBottom: true }}
// autoscrollToBottomThreshold is off by default in FlashList v2; without it a
// sent/incoming message lands below the viewport, hidden behind the composer.
maintainVisibleContentPosition={{ startRenderingFromBottom: true, autoscrollToBottomThreshold: 0.2 }}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

kody code-review Kody Rules low

Unnamed magic number for the autoscrollToBottomThreshold ratio complicates future tuning and code review. Extract the 0.2 literal into a named constant like const AUTOSCROLL_TO_BOTTOM_THRESHOLD = 0.2; at the module or component scope.

Kody rule violation: Replace magic numbers with named constants

Prompt for LLM

File src/app/chat/[channelId].tsx:

Line 317:

Unnamed magic number for the `autoscrollToBottomThreshold` ratio complicates future tuning and code review. Extract the `0.2` literal into a named constant like `const AUTOSCROLL_TO_BOTTOM_THRESHOLD = 0.2;` at the module or component scope.

Talk to Kody by mentioning @kody

Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.

keyExtractor={keyExtractor}
renderItem={renderItem}
onStartReached={handleStartReached}
Expand Down Expand Up @@ -354,7 +356,7 @@ export default function ChannelConversationScreen() {
handleToggleReaction(
m,
emoji,
m.Reactions.some((r) => r.Emoji === emoji && r.UserId === currentUserId)
(m.Reactions ?? []).some((r) => r.Emoji === emoji && r.UserId === currentUserId)
)
}
onReply={openThread}
Expand Down
Loading
Loading