Develop #124
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| 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; | ||
| 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)); | ||
|
|
||
| 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]); | ||
|
There was a problem hiding this comment.
Batch operation failure risk: Kody rule violation: Use Promise.allSettled for batch operations with partial failures Prompt for LLMTalk 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); | ||
| }); | ||
| }); | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -78,6 +78,10 @@ export default function TabLayout() { | |
| // Refs to track initialization state | ||
| const hasInitialized = useRef(false); | ||
| const isInitializing = useRef(false); | ||
| // Bumped on every initialization start and on sign-out. An in-flight run compares its | ||
| // captured value after each await, so a run belonging to a session that ended can no | ||
| // longer connect hubs or mark the app initialized. | ||
| const initGeneration = useRef(0); | ||
| const initTimedOut = useRef(false); | ||
| const hasHiddenSplash = useRef(false); | ||
| const lastSignedInStatus = useRef<string | null>(null); | ||
|
|
@@ -116,6 +120,8 @@ export default function TabLayout() { | |
|
|
||
| isInitializing.current = true; | ||
| initTimedOut.current = false; | ||
| const generation = (initGeneration.current += 1); | ||
| const isCurrentRun = () => initGeneration.current === generation; | ||
| logger.info({ | ||
| message: 'Starting app initialization', | ||
| context: { | ||
|
|
@@ -162,6 +168,8 @@ export default function TabLayout() { | |
| context: { platform: Platform.OS }, | ||
| }); | ||
|
|
||
| if (!isCurrentRun()) return; | ||
|
|
||
|
Comment on lines
+171
to
+172
Contributor
There was a problem hiding this comment. 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift Guard and test every initialization stage. The first generation check occurs after several awaited operations, and the test model has only one awaited operation. A sign-out between production stages can therefore run stale work without a failing test.
📍 Affects 2 files
🤖 Prompt for AI AgentsSource: Coding guidelines |
||
| // Connect to SignalR after core initialization is complete | ||
| try { | ||
| await useSignalRStore.getState().connectUpdateHub(); | ||
|
|
@@ -177,6 +185,8 @@ export default function TabLayout() { | |
| // Don't fail initialization if SignalR connection fails | ||
| } | ||
|
|
||
| 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)) { | ||
|
|
@@ -199,6 +209,8 @@ export default function TabLayout() { | |
| }); | ||
| } | ||
|
|
||
| if (!isCurrentRun()) return; | ||
|
|
||
| // Initialize weather alerts | ||
| try { | ||
| await useWeatherAlertsStore.getState().fetchSettings(); | ||
|
|
@@ -217,6 +229,8 @@ export default function TabLayout() { | |
| }); | ||
| } | ||
|
|
||
| if (!isCurrentRun()) return; | ||
|
|
||
| hasInitialized.current = true; | ||
|
|
||
| logger.info({ | ||
|
|
@@ -237,7 +251,11 @@ export default function TabLayout() { | |
| } | ||
| }) | ||
| .finally(() => { | ||
| // Only the current run owns the guard; a superseded run clearing it would let | ||
| // two initializations overlap. | ||
| if (isCurrentRun()) { | ||
| isInitializing.current = false; | ||
| } | ||
| }); | ||
|
|
||
| await Promise.race([initPromise, initTimeout]); | ||
|
|
@@ -246,6 +264,10 @@ export default function TabLayout() { | |
| message: 'Failed to initialize app', | ||
| context: { error, platform: Platform.OS }, | ||
| }); | ||
| // A run whose session already ended must not clobber state a newer run (or the | ||
| // sign-out cleanup) has since established. | ||
| if (!isCurrentRun()) return; | ||
|
|
||
| // Reset initialization state on error so it can be retried | ||
| hasInitialized.current = false; | ||
| // If the init promise is still hanging, clear the guard so a retry is possible | ||
|
|
@@ -349,6 +371,12 @@ export default function TabLayout() { | |
| logger.info({ | ||
| message: 'User signed out, stopping location tracking', | ||
| }); | ||
|
|
||
| // Retire any initialization still in flight so it cannot connect hubs or mark the | ||
| // app initialized for a session that ended, and free the guard it no longer owns | ||
| // so the next sign-in is not skipped as "already initializing". | ||
| initGeneration.current += 1; | ||
| isInitializing.current = false; | ||
|
Comment on lines
+374
to
+379
Contributor
There was a problem hiding this comment. 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
# Find auth-status transitions and navigation behavior around sign-out.
rg -n -C 5 "signedOut|signOut|set.*status|status: 'idle'|status: 'error'" src
rg -n -C 5 "hasInitialized\.current|initializeApp\(" "src/app/(app)/_layout.tsx"Repository: Resgrid/Dispatch Length of output: 50373 🏁 Script executed: #!/bin/bash
set -euo pipefail
echo "== files =="
git ls-files | rg 'src/app/\(app\)/_layout\.tsx|src/(app|modules|screens|components).*sign|src/.*auth|src/.*coreStore|init-session-generation\.test\.tsx' | head -200
echo
echo "== layout outline relevant =="
wc -l "src/app/(app)/_layout.tsx"
ast-grep outline "src/app/(app)/_layout.tsx" --match core --view expanded || true
echo
echo "== layout lines 280-385 =="
sed -n '280,385p' "src/app/(app)/_layout.tsx"
echo
echo "== init-session test =="
sed -n '1,180p' "src/app/(app)/__tests__/init-session-generation.test.tsx"
echo
echo "== focused signOut/status/hasInitialized references =="
rg -n -C 3 "signOut|signedOut|setSignedOut|hasInitialized\.current|initializeApp\(" "src/app/(app)/_layout.tsx" "src/app/(app)/__tests__/init-session-generation.test.tsx"Repository: Resgrid/Dispatch Length of output: 16913 🏁 Script executed: #!/bin/bash
set -euo pipefail
echo "== src/app/(app)/_layout.tsx first 450 lines =="
sed -n '1,450p' "src/app/(app)/_layout.tsx"
echo "== auth state/status references in src =="
rg -n -C 3 "setStatus|signOut|signedOut|signedIn|idle|error" src/lib/auth src/stores src/app src/modules src/screens src/components | head -500Repository: Resgrid/Dispatch Length of output: 46670 🏁 Script executed: #!/bin/bash
set -euo pipefail
echo "== auth store outline =="
wc -l src/lib/auth/index.tsx src/stores/auth/store.tsx src/app/_layout.tsx src/__tests__/app/_layout.auth-guard.test.tsx
ast-grep outline src/stores/auth/store.tsx --view expanded || true
ast-grep outline src/lib/auth/index.tsx --view expanded || true
echo
echo "== auth store =="
sed -n '1,240p' src/stores/auth/store.tsx
echo
echo "== auth index =="
sed -n '1,220p' src/lib/auth/index.tsx
echo
echo "== root layout auth guard slices =="
sed -n '1,260p' src/app/_layout.tsx
sed -n '1,180p' src/__tests__/app/_layout.auth-guard.test.tsx
echo
echo "== focused redirect/log-out usages =="
rg -n -C 4 "useAuthStore|useAuth|login\\(|logout\\(|setStatus\\(|signedOut|redirectTo:|router\\." src/app src/lib/auth src/__tests__ src/components src/hooks | head -400Repository: Resgrid/Dispatch Length of output: 48462 Reset the completed-session initialization flag on sign-out.
🤖 Prompt for AI Agents
Comment on lines
+378
to
+379
There was a problem hiding this comment.
State management bug: The sign-out handler resets // Retire any initialization still in flight so it cannot connect hubs or mark the
// app initialized for a session that ended, and free 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;Prompt for LLMTalk to Kody by mentioning @kody Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction. |
||
| } | ||
|
|
||
| // Update last known status | ||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -8,9 +8,7 @@ | |
| import { MessageActionsSheet } from '@/components/chat/message-actions-sheet'; | ||
| import { MessageBubble } from '@/components/chat/message-bubble'; | ||
| import { TypingDots } from '@/components/chat/typing-indicator'; | ||
| import { Box } from '@/components/ui/box'; | ||
| import { Center } from '@/components/ui/center'; | ||
| import { FlatList } from '@/components/ui/flat-list'; | ||
| import { FocusAwareStatusBar } from '@/components/ui/focus-aware-status-bar'; | ||
|
|
@@ -20,7 +18,6 @@ | |
| import { Pressable } from '@/components/ui/pressable'; | ||
| import { Spinner } from '@/components/ui/spinner'; | ||
| import { Text } from '@/components/ui/text'; | ||
| import { VStack } from '@/components/ui/vstack'; | ||
| import { type ChatMessageResultData } from '@/models/v4/chat'; | ||
| import useAuthStore from '@/stores/auth/store'; | ||
|
|
@@ -40,8 +37,6 @@ | |
| const isModerator = !!securityStore((s) => s.rights)?.IsAdmin; | ||
| const [text, setText] = useState(''); | ||
| const [actionsMessage, setActionsMessage] = useState<ChatMessageResultData | null>(null); | ||
|
|
||
| useFocusEffect( | ||
| useCallback(() => { | ||
|
|
@@ -73,7 +68,7 @@ | |
|
|
||
| const renderItem = useCallback( | ||
| ({ item }: { item: ChatMessageResultData }) => ( | ||
| <MessageBubble message={item} isOwn={!!item.SenderUserId && item.SenderUserId === currentUserId} showSender={false} currentUserId={currentUserId} onLongPress={setActionsMessage} onToggleReaction={() => undefined} /> | ||
|
Check warning on line 71 in src/app/(app)/chatbot.tsx
|
||
| ), | ||
| [currentUserId] | ||
| ); | ||
|
|
@@ -147,7 +142,7 @@ | |
| </HStack> | ||
| </KeyboardAvoidingView> | ||
|
|
||
| {/* Restricted actions for assistant messages: copy, pin (moderator), flag. */} | ||
| <MessageActionsSheet | ||
| message={actionsMessage} | ||
| isOpen={actionsMessage !== null} | ||
|
|
@@ -161,42 +156,12 @@ | |
| const ok = await copyToClipboard(m.Body ?? ''); | ||
| useToastStore.getState().showToast(ok ? 'success' : 'info', ok ? t('chat.copied') : t('chat.copy_unavailable')); | ||
| }} | ||
| onEdit={() => undefined} | ||
|
There was a problem hiding this comment.
Performance regression: Inline arrow functions in JSX props create new functions on every render. Move the Kody rule violation: Avoid using .bind() or arrow functions in JSX props Prompt for LLMTalk to Kody by mentioning @kody Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction. |
||
| onDelete={() => undefined} | ||
| onFlag={(m, reason) => useChatStore.getState().flagMessage(m.ChatMessageId, reason)} | ||
| onTogglePin={(m, pinned) => chatbotChannelId && useChatStore.getState().togglePin(m.ChatMessageId, chatbotChannelId, pinned)} | ||
| onModeratorDelete={() => undefined} | ||
| /> | ||
| </Box> | ||
| ); | ||
| } | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Add a stale-session test for each initialization stage.
This helper has one await before
connectHub. The layout has four awaits before its first generation check. The test therefore stays green when a stale layout run continues from core initialization into calls, rights, or feature-flag initialization.Use deferred gates for each stage. Sign out after each gate resolves. Assert that the next stage and all later effects do not run.
As per coding guidelines, “Generate tests for all components, services and logic generated. Ensure tests run without errors and fix any issues.”
🤖 Prompt for AI Agents
Source: Coding guidelines