Conversation
This comment has been minimized.
This comment has been minimized.
📝 WalkthroughWalkthroughThe change centralizes token refresh, adds SAML state validation, introduces incident chat channels and direct messaging, strengthens session cleanup, updates Zustand subscriptions, adjusts map and call rendering, aligns tests, and expands localized content. ChangesApplication runtime
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 15
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/stores/app/audio-stream-store.web.ts (1)
211-216: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winDetach listeners before checking
audioElement.When
audio.play()rejects,audioElementremainsnull, sostopStream()skipsdetachStreamListeners()and leaves the failed stream's handlers attached. CalldetachStreamListeners()unconditionally at the start ofstopStream().🤖 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/stores/app/audio-stream-store.web.ts` around lines 211 - 216, Update stopStream to call detachStreamListeners unconditionally at the beginning of the method, before checking audioElement or accessing stream state. Preserve the existing cleanup and stopping behavior for cases where audioElement is present.src/app/(app)/map.tsx (1)
183-246: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick winUse the declared Mapbox types and update the interaction check.
Type
innerContainerStyleasStyleProp<ViewStyle>, camera configurations asCameraStop, and the callback asMapState.MapStatehas noproperties.isUserInteraction; usestate.gestures.isGestureActive. Otherwise, user panning is not recorded and GPS updates can keep recentering an unlocked map.🤖 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)/map.tsx around lines 183 - 246, Update the map component to use declared Mapbox types: type innerContainerStyle as StyleProp<ViewStyle>, camera configuration objects as CameraStop, and the relevant callback state as MapState. In the interaction handling, replace state.properties.isUserInteraction with state.gestures.isGestureActive so user panning correctly updates the map-moved state and prevents recentering while unlocked.Source: Coding guidelines
🧹 Nitpick comments (9)
src/hooks/__tests__/use-signalr-lifecycle.test.tsx (1)
48-59: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winType the SignalR selector mock.
Replace both
anyusages with a typed mock state and selector. SinceSignalRStateis private, export it for aPickor define a local type containing the six selected hub actions and connection fields. Remove theas anycast.🤖 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/hooks/__tests__/use-signalr-lifecycle.test.tsx` around lines 48 - 59, Type the signalRStoreState fixture and mockUseSignalRStore selector using a local type or exported SignalRState Pick containing the six hub actions and connection fields. Replace both any usages, type the selector parameter and return value consistently, and remove the as any cast while preserving the existing mock behavior.Source: Coding guidelines
src/__tests__/app/calls.test.tsx (1)
70-70: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRemove the
anycasts from the mocked store setup.The casts at Line 70 and Line 79 disable type checking while attaching
getState. UseObject.assignor an explicit callable intersection type instead.Proposed type-safe replacement
- (useCallsStore as any).getState = jest.fn(() => mockCallsStore); + Object.assign(useCallsStore, { getState: jest.fn(() => mockCallsStore) }); - (securityStore as any).getState = jest.fn(() => mockSecurityStore); + Object.assign(securityStore, { getState: jest.fn(() => mockSecurityStore) });Verify the inferred mock type with the repository's TypeScript check. As per coding guidelines,
**/*.{ts,tsx}requires type-safe TypeScript and says to avoidany.Also applies to: 79-79
🤖 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/__tests__/app/calls.test.tsx` at line 70, Remove the any casts from the mocked useCallsStore setup at both getState assignments, using Object.assign or an explicit callable intersection type so getState remains type-checked while assigning mockCallsStore. Run the repository’s TypeScript check to verify the inferred mock type.Source: Coding guidelines
src/app/(app)/calls.tsx (1)
78-85: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winExtract the row press handler into a memoized item component.
Line 80 creates a new
onPressfunction for every rendered row. Use a memoized call-row component with a stable callback.As per coding guidelines, “Avoid anonymous functions in
renderItemor event handlers to prevent re-renders.”🤖 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)/calls.tsx around lines 78 - 85, Extract the row rendering logic from renderCallItem into a memoized call-row component with a stable press callback that navigates using the item’s CallId. Update renderCallItem to render this component and pass the required call and priority data, avoiding anonymous functions in the item renderer and event handler.Source: Coding guidelines
src/app/(app)/map.tsx (1)
53-70: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winFlatten the combined view styles.
Lines 53, 61, and 65 pass style arrays directly. Use
StyleSheet.flatten()for each combined style before passing it to the React Native view.As per coding guidelines, “When combining styles, always use
StyleSheet.flatten()to merge them into a single object for web platform compatibility.”🤖 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)/map.tsx around lines 53 - 70, Use StyleSheet.flatten() for every combined style array in the marker rendering JSX: the outer marker container transform style, the markerInnerContainer style combining markerInnerContainer and innerContainerStyle, and the directionIndicator transform style. Pass each flattened object to its corresponding View while preserving the existing style values.Source: Coding guidelines
src/lib/auth/token-refresh.ts (1)
85-92: 🩺 Stability & Availability | 🔵 Trivial | 💤 Low valueClear the in-flight marker inside the async operation.
operation.finally(...)creates a new promise that nobody observes. Ifhandlers.onRefreshFailed()throws,operationrejects, and that detached promise becomes an unhandled rejection. Moving the cleanup into afinallyblock inside the IIFE removes the detached promise and keeps the reset guaranteed.♻️ Proposed refactor
- const operation = (async (): Promise<boolean> => { + let operation!: Promise<boolean>; + operation = (async (): Promise<boolean> => { if (!handlers) { logger.error({ message: 'Token refresh attempted before initTokenRefresh was called' }); return false; } @@ - } catch (error) { + } catch (error) { logger.error({ message: 'Token refresh failed', context: { error: error instanceof Error ? error.message : String(error) }, }); handlers.onRefreshFailed(); return false; + } finally { + if (inFlightRefresh === operation) { + inFlightRefresh = null; + } } })(); inFlightRefresh = operation; - operation.finally(() => { - if (inFlightRefresh === operation) { - inFlightRefresh = null; - } - }); - return operation;The early
return falsepaths also need the same reset, so wrap the whole body intry { ... } finally { ... }rather than only the request block.🤖 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/lib/auth/token-refresh.ts` around lines 85 - 92, Move the in-flight marker cleanup into the async IIFE that creates operation, wrapping its entire body—including early return false paths—in try/finally. In the finally block, clear inFlightRefresh only when it still equals operation, and remove the detached operation.finally call while preserving the existing return behavior.src/hooks/use-saml-login.ts (1)
29-45: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRemove the redundant manual JSON round-trip.
setIteminsrc/lib/storage/index.tsxalready serializes the value, andgetItemalready parses it. The current code stringifies twice and parses twice. The values round-trip correctly today, so this is not a defect. Typing the helpers directly makes the contract clearer and removes the dependency on the double encoding.♻️ Proposed refactor
async function savePendingState(state: SamlPendingState | null): Promise<void> { if (state) { - await setItem(SAML_PENDING_STATE_KEY, JSON.stringify(state)); + await setItem<SamlPendingState>(SAML_PENDING_STATE_KEY, state); } else { await removeItem(SAML_PENDING_STATE_KEY); } } function readPendingState(): SamlPendingState | null { - const raw = getItem<string>(SAML_PENDING_STATE_KEY); - if (!raw) return null; - try { - return JSON.parse(raw) as SamlPendingState; - } catch { - return null; - } + const state = getItem<SamlPendingState>(SAML_PENDING_STATE_KEY); + return state && typeof state.nonce === 'string' && typeof state.startedAt === 'number' ? state : null; }Note: if you apply this refactor, an already persisted double-encoded value parses to a string and is rejected by the type check. That only forces one extra login start.
🤖 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/hooks/use-saml-login.ts` around lines 29 - 45, Update savePendingState and readPendingState to rely directly on setItem and getItem serialization, removing the explicit JSON.stringify and JSON.parse calls. Type the storage helper usage for SamlPendingState, and validate the retrieved value before returning it so legacy double-encoded strings are treated as null.src/services/push-notification.ts (1)
35-37: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRemove the unchecked
Hrefassertion.
as unknown as Hrefsuppresses route validation. Regenerate or correct the typed-route declaration, then pass a typed chat route object. Otherwise, a route refactor can break deep links without a compile-time error.As per coding guidelines, “Write concise, type-safe TypeScript code.”
🤖 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/services/push-notification.ts` around lines 35 - 37, Remove the double cast from the routerPushWithRetry call and correct or regenerate the typed-route union so the '/chat/[channelId]' route with its channelId parameter is recognized as a valid Href. Pass the chat route object directly while preserving the existing retry options and error handling.Source: Coding guidelines
src/components/incident-command/command-board-view.tsx (1)
73-82: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winStabilize the new press handlers.
The added inline
onPressfunctions create new handler identities on every command-board render. Extract memoized row components or callbacks for channel and direct-message actions.As per coding guidelines, “Avoid anonymous functions in renderItem or event handlers to prevent re-renders.”
Also applies to: 538-558
🤖 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/components/incident-command/command-board-view.tsx` around lines 73 - 82, Stabilize the inline onPress handlers in the command-board row component and the corresponding direct-message actions around the referenced section by extracting memoized callbacks or memoized row components. Preserve the existing arguments, disabled behavior, and onOpen action while ensuring handler identities remain stable across renders.Source: Coding guidelines
src/stores/chat/store.ts (1)
265-267: 🚀 Performance & Scalability | 🔵 Trivial | 🏗️ Heavy liftAvoid downloading the full channel archive for one incident.
chatApi.getChannels(undefined, true)retrieves all active and archived chat channels, then filters them locally byCallId. Add a call-scoped or paginated chat API, or use a bounded cache for incident chat channels. The existinggetChannelsForCallendpoint returns voice-channel records and cannot replace this request.🤖 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/stores/chat/store.ts` around lines 265 - 267, Update the channel-loading flow containing getChannels(undefined, true) so it does not download the full active and archived channel archive before filtering by CallId. Use a call-scoped or paginated chat-channel API, or a bounded incident-channel cache, while preserving the incidentChannelsByCallId update; do not substitute the existing getChannelsForCall endpoint because it returns voice-channel records.
🤖 Prompt for all review comments with 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.
Inline comments:
In `@src/app/`(app)/_layout.tsx:
- Around line 435-437: Reset hasInitialized.current in the session-ended branch
immediately before or after calling teardownSignedInSession(), so subsequent
sign-ins in the same process satisfy shouldInitialize and rerun initializeApp.
Apply this to the shared sign-out/unauthorized-user path without changing the
teardown behavior.
In `@src/app/`(app)/map.tsx:
- Line 411: Update showRecenterButton to also require a location-presence
boolean derived with null checks for latitude and longitude, preserving zero
coordinates as valid; keep the existing isMapLocked and hasUserMovedMap
conditions.
In `@src/components/incident-command/command-board-view.tsx`:
- Around line 82-83: Remove the isDisabled={!channelId} restriction from the
Button in the channel row so it remains pressable without a channelId. Preserve
the existing onPress handler calling onOpen(channelId, unavailableMessage),
allowing the unavailable feedback path to execute while keeping the current
label behavior unchanged.
- Line 511: Update the icon render sites in
src/components/incident-command/command-board-view.tsx at lines 511, 523, 539,
and 555 and src/components/incident-command/incident-command-tab.tsx at line 57;
also apply the same change to incident-command-tab.tsx line 92 as requested.
Render Lucide components directly inside styled View wrappers, moving icon
classes from the icons or ButtonIcon as props onto the wrappers and replacing
ButtonIcon as usage. Use the existing component patterns and preserve each
icon’s appearance and behavior.
In `@src/hooks/use-direct-message.ts`:
- Line 35: Update the navigation call in the direct-message hook to use the
typed router.push object form with the /chat/[channelId] pathname and channelId
params, replacing the dynamic template-string route while preserving the
existing destination.
In `@src/hooks/use-saml-login.ts`:
- Around line 56-60: Update the URL construction in the SAML login flow around
savePendingState so any existing RelayState query parameter is replaced with the
generated nonce rather than duplicated. Ensure callback parsing and validation
in the corresponding login handler accepts Linking.parse values that may be
string arrays, while still validating the generated nonce correctly.
In `@src/lib/auth/token-refresh.ts`:
- Around line 37-45: Update scheduleTokenRefresh so short-lived tokens do not
trigger a perpetual minimum-delay refresh loop. Confirm the identity server’s
token lifetime contract; if lifetimes can approach REFRESH_BUFFER_MS, derive the
refresh delay from expiresInSeconds (for example, refreshing no earlier than
half the token lifetime) while retaining MIN_REFRESH_DELAY_MS as the lower
bound.
In `@src/stores/app/livekit-store.ts`:
- Around line 220-225: Extract the web audio element cleanup logic from
disconnectFromRoom into a shared helper, then invoke that helper for currentRoom
before removeAllListeners() and disconnect() in the reconnect path. Ensure the
helper removes the mapped audio elements from the document and clears
webAudioElements so resources from the old room are released.
- Around line 402-420: In src/stores/app/livekit-store.ts lines 402-420, move
the cleanup operations and disconnected-state set call into a finally block so
currentRoom, connection flags, and isTalking are reset even if listener removal,
disconnect, or audio playback rejects. In src/stores/app/livekit-store.ts line
376, update the failed-connect reset to also set isTalking to false.
In `@src/stores/auth/store.tsx`:
- Around line 249-269: The applyAuthResponse handler in initTokenRefresh must
not restore authentication after logout has signed the store out. Before
applying response tokens and setting status to signedIn, verify the current
useAuthStore state is not signedOut; reject or otherwise ignore the refresh
response when it is signed out, while preserving normal token updates for active
sessions.
In `@src/translations/ar.json`:
- Line 791: Update the form.invalid_url translation in
src/translations/ar.json:791-791, src/translations/de.json:791-791,
src/translations/es.json:791-791, src/translations/fr.json:791-791,
src/translations/it.json:791-791, and src/translations/pl.json:791-791 so each
locale states that https:// is required and http:// is allowed only for
localhost.
- Line 815: Translate the shared incident-command and authorization strings
currently retaining English values in all six catalogs:
src/translations/ar.json, src/translations/de.json, src/translations/es.json,
src/translations/fr.json, src/translations/it.json, and
src/translations/pl.json. Update the entries at lines 815, 819-821, 828-832,
844-846, 872-873, 877, 896, and 956, covering incident chat states, channel
messages, direct-message errors, authorization messages, open_chat,
send_message, and login.dispatch_not_authorized, while preserving the existing
keys and placeholders.
In `@src/translations/sv.json`:
- Around line 815-846: Translate every specified English user-facing value in
the Swedish and Ukrainian locale catalogs, covering incident-command channels,
chat and direct-message labels, authorization and open_chat entries,
send_message, and login.dispatch_not_authorized. Update src/translations/sv.json
ranges 815-846, 872-877, 896, and 956 with Swedish text, and the corresponding
ranges in src/translations/uk.json with Ukrainian text; preserve all translation
keys and JSON structure.
- Around line 486-489: Update the Swedish translation entries warning_count and
summary to use the locale’s count-aware pluralization convention, preserving the
existing interpolation keys and singular wording for a count of one while using
the correct plural form for counts greater than one.
- Line 837: Correct the Swedish wording of the establish_description translation
so it is grammatically valid, while preserving its intended meaning: creating an
optional leaderboard from a template.
---
Outside diff comments:
In `@src/app/`(app)/map.tsx:
- Around line 183-246: Update the map component to use declared Mapbox types:
type innerContainerStyle as StyleProp<ViewStyle>, camera configuration objects
as CameraStop, and the relevant callback state as MapState. In the interaction
handling, replace state.properties.isUserInteraction with
state.gestures.isGestureActive so user panning correctly updates the map-moved
state and prevents recentering while unlocked.
In `@src/stores/app/audio-stream-store.web.ts`:
- Around line 211-216: Update stopStream to call detachStreamListeners
unconditionally at the beginning of the method, before checking audioElement or
accessing stream state. Preserve the existing cleanup and stopping behavior for
cases where audioElement is present.
---
Nitpick comments:
In `@src/__tests__/app/calls.test.tsx`:
- Line 70: Remove the any casts from the mocked useCallsStore setup at both
getState assignments, using Object.assign or an explicit callable intersection
type so getState remains type-checked while assigning mockCallsStore. Run the
repository’s TypeScript check to verify the inferred mock type.
In `@src/app/`(app)/calls.tsx:
- Around line 78-85: Extract the row rendering logic from renderCallItem into a
memoized call-row component with a stable press callback that navigates using
the item’s CallId. Update renderCallItem to render this component and pass the
required call and priority data, avoiding anonymous functions in the item
renderer and event handler.
In `@src/app/`(app)/map.tsx:
- Around line 53-70: Use StyleSheet.flatten() for every combined style array in
the marker rendering JSX: the outer marker container transform style, the
markerInnerContainer style combining markerInnerContainer and
innerContainerStyle, and the directionIndicator transform style. Pass each
flattened object to its corresponding View while preserving the existing style
values.
In `@src/components/incident-command/command-board-view.tsx`:
- Around line 73-82: Stabilize the inline onPress handlers in the command-board
row component and the corresponding direct-message actions around the referenced
section by extracting memoized callbacks or memoized row components. Preserve
the existing arguments, disabled behavior, and onOpen action while ensuring
handler identities remain stable across renders.
In `@src/hooks/__tests__/use-signalr-lifecycle.test.tsx`:
- Around line 48-59: Type the signalRStoreState fixture and mockUseSignalRStore
selector using a local type or exported SignalRState Pick containing the six hub
actions and connection fields. Replace both any usages, type the selector
parameter and return value consistently, and remove the as any cast while
preserving the existing mock behavior.
In `@src/hooks/use-saml-login.ts`:
- Around line 29-45: Update savePendingState and readPendingState to rely
directly on setItem and getItem serialization, removing the explicit
JSON.stringify and JSON.parse calls. Type the storage helper usage for
SamlPendingState, and validate the retrieved value before returning it so legacy
double-encoded strings are treated as null.
In `@src/lib/auth/token-refresh.ts`:
- Around line 85-92: Move the in-flight marker cleanup into the async IIFE that
creates operation, wrapping its entire body—including early return false
paths—in try/finally. In the finally block, clear inFlightRefresh only when it
still equals operation, and remove the detached operation.finally call while
preserving the existing return behavior.
In `@src/services/push-notification.ts`:
- Around line 35-37: Remove the double cast from the routerPushWithRetry call
and correct or regenerate the typed-route union so the '/chat/[channelId]' route
with its channelId parameter is recognized as a valid Href. Pass the chat route
object directly while preserving the existing retry options and error handling.
In `@src/stores/chat/store.ts`:
- Around line 265-267: Update the channel-loading flow containing
getChannels(undefined, true) so it does not download the full active and
archived channel archive before filtering by CallId. Use a call-scoped or
paginated chat-channel API, or a bounded incident-channel cache, while
preserving the incidentChannelsByCallId update; do not substitute the existing
getChannelsForCall endpoint because it returns voice-channel records.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 2c1f8898-5fac-465e-86d3-b7fcb3002d3c
📒 Files selected for processing (40)
src/__tests__/app/call/[id].test.tsxsrc/__tests__/app/calls.test.tsxsrc/__tests__/security-integration.test.tssrc/api/chat/chat.tssrc/api/common/client.tsxsrc/app/(app)/_layout.tsxsrc/app/(app)/calls.tsxsrc/app/(app)/chatbot.tsxsrc/app/(app)/home.tsxsrc/app/(app)/map.tsxsrc/components/checkIn/check-in-bottom-sheet.tsxsrc/components/incident-command/command-board-view.tsxsrc/components/incident-command/incident-command-tab.tsxsrc/components/pois/poi-detail-screen.tsxsrc/components/sidebar/call-sidebar.tsxsrc/hooks/__tests__/use-signalr-lifecycle.test.tsxsrc/hooks/use-direct-message.tssrc/hooks/use-saml-login.tssrc/hooks/use-signalr-lifecycle.tssrc/lib/__tests__/navigation.test.tssrc/lib/auth/token-refresh.tssrc/models/v4/chat/chatEnums.tssrc/models/v4/security/departmentRightsResultData.tssrc/services/push-notification.tssrc/services/signalr.service.tssrc/stores/app/audio-stream-store.web.tssrc/stores/app/livekit-store.tssrc/stores/auth/store.tsxsrc/stores/chat/store.tssrc/stores/security/__tests__/store.test.tssrc/stores/security/store.tssrc/translations/ar.jsonsrc/translations/de.jsonsrc/translations/en.jsonsrc/translations/es.jsonsrc/translations/fr.jsonsrc/translations/it.jsonsrc/translations/pl.jsonsrc/translations/sv.jsonsrc/translations/uk.json
| <Button variant="link" size="xs" onPress={() => onOpen(channelId, unavailableMessage)} isDisabled={!channelId}> | ||
| <ButtonText className="text-xs">{channelId ? openLabel : '—'}</ButtonText> |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Keep unavailable channel feedback reachable.
isDisabled={!channelId} prevents Line 82 from calling openChannel. The unavailable toast at Lines 148-151 is therefore unreachable. Keep the row pressable when no channel exists so it can explain the unavailable state.
Proposed fix
- <Button variant="link" size="xs" onPress={() => onOpen(channelId, unavailableMessage)} isDisabled={!channelId}>
+ <Button variant="link" size="xs" onPress={() => onOpen(channelId, unavailableMessage)}>📝 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.
| <Button variant="link" size="xs" onPress={() => onOpen(channelId, unavailableMessage)} isDisabled={!channelId}> | |
| <ButtonText className="text-xs">{channelId ? openLabel : '—'}</ButtonText> | |
| <Button variant="link" size="xs" onPress={() => onOpen(channelId, unavailableMessage)}> | |
| <ButtonText className="text-xs">{channelId ? openLabel : '—'}</ButtonText> |
🤖 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/components/incident-command/command-board-view.tsx` around lines 82 - 83,
Remove the isDisabled={!channelId} restriction from the Button in the channel
row so it remains pressable without a channelId. Preserve the existing onPress
handler calling onOpen(channelId, unavailableMessage), allowing the unavailable
feedback path to execute while keeping the current label behavior unchanged.
| <ChatChannelRow | ||
| label={t('incident_command.incident_channel')} | ||
| hint={t('incident_command.incident_channel_hint')} | ||
| icon={<MessagesSquareIcon size={16} className="text-blue-500" />} |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -e
printf '%s\n' '--- file map ---'
ast-grep outline src/components/incident-command/command-board-view.tsx
ast-grep outline src/components/incident-command/incident-command-tab.tsx
printf '%s\n' '--- focused source ---'
sed -n '1,90p' src/components/incident-command/incident-command-tab.tsx
sed -n '470,575p' src/components/incident-command/command-board-view.tsx
printf '%s\n' '--- relevant symbols and wrapper patterns ---'
rg -n -C 3 'MessagesSquareIcon|RadioIcon|MessageCircleIcon|NetworkIcon|ButtonIcon|lucide-react-native|StyleSheet\.flatten|<View' \
src/components/incident-command/command-board-view.tsx \
src/components/incident-command/incident-command-tab.tsx
printf '%s\n' '--- package/config references ---'
rg -n -C 2 'lucide-react-native|ButtonIcon' package.json pnpm-lock.yaml yarn.lock package-lock.json 2>/dev/null || trueRepository: Resgrid/Dispatch
Length of output: 29751
🏁 Script executed:
#!/bin/bash
set -e
printf '%s\n' '--- changed files and focused diff ---'
git status --short
git diff --stat
git diff -- src/components/incident-command/command-board-view.tsx src/components/incident-command/incident-command-tab.tsx
printf '%s\n' '--- UI component locations ---'
fd -i 'button|view|hstack|vstack' src/components/ui -t f
printf '%s\n' '--- Button implementation ---'
BUTTON_FILE="$(fd -i -t f 'button' src/components/ui | head -n 1)"
if [ -n "$BUTTON_FILE" ]; then
ast-grep outline "$BUTTON_FILE"
cat -n "$BUTTON_FILE"
fi
printf '%s\n' '--- View and layout imports/usages ---'
rg -n -C 3 "from 'react-native'|from '`@/components/ui/view`'|<View|className=.*mr-1|className=.*text-blue-500" \
src/components/incident-command src/components/ui | head -n 300
printf '%s\n' '--- all affected icon occurrences ---'
rg -n -C 2 'NetworkIcon|MessagesSquareIcon|RadioIcon|MessageCircleIcon' src/components/incident-commandRepository: Resgrid/Dispatch
Length of output: 32909
🏁 Script executed:
#!/bin/bash
set -e
printf '%s\n' '--- tracked UI files ---'
git ls-files src/components/ui | rg '(^|/)(button|box|hstack|vstack)(/|\.|$)' || true
printf '%s\n' '--- ButtonIcon declarations and exports ---'
rg -n -C 8 'ButtonIcon|createButton|buttonIcon|Icon' src/components/ui src | head -n 500
printf '%s\n' '--- relevant component files ---'
for f in $(git ls-files src/components/ui | rg 'button'); do
echo "--- $f"
wc -l "$f"
sed -n '1,240p' "$f"
done
printf '%s\n' '--- direct lucide styling and wrapper conventions ---'
rg -n -C 2 '<(NetworkIcon|MessagesSquareIcon|RadioIcon|MessageCircleIcon|[A-Za-z]+Icon)[^>]*(className|style)=|<(Box|View)[^>]*className="[^"]*(text-|mr-|ml-)' src | head -n 400Repository: Resgrid/Dispatch
Length of output: 50372
🏁 Script executed:
#!/bin/bash
set -e
printf '%s\n' '--- Button implementation tail ---'
sed -n '220,340p' src/components/ui/button/index.tsx
printf '%s\n' '--- focused JSX and imports ---'
sed -n '1,75p' src/components/incident-command/command-board-view.tsx
sed -n '35,110p' src/components/incident-command/incident-command-tab.tsx
printf '%s\n' '--- compact wrapper examples around icons ---'
rg -n -C 4 '<(Box|View)[^>]*>[^<]*<(MessagesSquareIcon|RadioIcon|MessageCircleIcon|NetworkIcon)|<(Box|View)[^>]*className="[^"]*(mr-1|text-blue-500|text-gray-400)' src/components src/app -g '*.tsx' | head -n 160Repository: Resgrid/Dispatch
Length of output: 9934
🏁 Script executed:
#!/bin/bash
set -e
python3 - <<'PY'
from pathlib import Path
import re
files = [
Path("src/components/incident-command/command-board-view.tsx"),
Path("src/components/incident-command/incident-command-tab.tsx"),
]
direct_styled = re.compile(r"<([A-Za-z]+Icon)\b[^>]*\bclassName=")
button_icon = re.compile(r"<ButtonIcon\b[^>]*\bas=\{([A-Za-z]+Icon)\}")
for path in files:
print(path)
for number, line in enumerate(path.read_text().splitlines(), 1):
direct = direct_styled.search(line)
button = button_icon.search(line)
if direct:
print(f" direct styled Lucide: line {number}: {direct.group(1)}")
if button:
print(f" ButtonIcon-wrapped Lucide: line {number}: {button.group(1)}")
PYRepository: Resgrid/Dispatch
Length of output: 1418
Render Lucide icons through styled View wrappers.
Move icon classes to wrappers and render Lucide components directly. Replace ButtonIcon as={...} at command-board-view.tsx lines 539 and 555, and incident-command-tab.tsx lines 79 and 98. Apply the same wrapper to direct styled icons at lines 511, 523, 57, and 92.
📍 Affects 2 files
src/components/incident-command/command-board-view.tsx#L511-L511(this comment)src/components/incident-command/command-board-view.tsx#L523-L523src/components/incident-command/command-board-view.tsx#L539-L539src/components/incident-command/command-board-view.tsx#L555-L555src/components/incident-command/incident-command-tab.tsx#L57-L57
🤖 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/components/incident-command/command-board-view.tsx` at line 511, Update
the icon render sites in src/components/incident-command/command-board-view.tsx
at lines 511, 523, 539, and 555 and
src/components/incident-command/incident-command-tab.tsx at line 57; also apply
the same change to incident-command-tab.tsx line 92 as requested. Render Lucide
components directly inside styled View wrappers, moving icon classes from the
icons or ButtonIcon as props onto the wrappers and replacing ButtonIcon as
usage. Use the existing component patterns and preserve each icon’s appearance
and behavior.
Source: Coding guidelines
| "table_address": "العنوان", | ||
| "table_scheduled": "مجدول في" | ||
| "form": { | ||
| "invalid_url": "يرجى إدخال عنوان URL صالح يبدأ بـ http:// أو https://", |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Keep form.invalid_url consistent across all locales.
The English catalog now states that https:// is required and http:// is allowed only for localhost. The six non-English catalogs still state that both schemes are valid.
src/translations/ar.json#L791-L791: update the Arabic validation message.src/translations/de.json#L791-L791: update the German validation message.src/translations/es.json#L791-L791: update the Spanish validation message.src/translations/fr.json#L791-L791: update the French validation message.src/translations/it.json#L791-L791: update the Italian validation message.src/translations/pl.json#L791-L791: update the Polish validation message.
📍 Affects 6 files
src/translations/ar.json#L791-L791(this comment)src/translations/de.json#L791-L791src/translations/es.json#L791-L791src/translations/fr.json#L791-L791src/translations/it.json#L791-L791src/translations/pl.json#L791-L791
🤖 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/translations/ar.json` at line 791, Update the form.invalid_url
translation in src/translations/ar.json:791-791,
src/translations/de.json:791-791, src/translations/es.json:791-791,
src/translations/fr.json:791-791, src/translations/it.json:791-791, and
src/translations/pl.json:791-791 so each locale states that https:// is required
and http:// is allowed only for localhost.
| "call": "البلاغ", | ||
| "channel_name": "اسم القناة", | ||
| "chat": "Chat", | ||
| "chat_frozen": "This incident is closed. Conversations are kept as a point-in-time record — no new messages.", |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Localize the shared incident-command and authorization strings.
All six non-English catalogs repeat English values in the incident-command chat states, channel messages, direct-message errors, authorization messages, open_chat, send_message, and login.dispatch_not_authorized.
src/translations/ar.json#L815-L815: translate the English values at Lines 815, 819-821, 828-832, 844-846, 872-873, 877, 896, and 956.src/translations/de.json#L815-L815: translate the English values at Lines 815, 819-821, 828-832, 844-846, 872-873, 877, 896, and 956.src/translations/es.json#L815-L815: translate the English values at Lines 815, 819-821, 828-832, 844-846, 872-873, 877, 896, and 956.src/translations/fr.json#L815-L815: translate the English values at Lines 815, 819-821, 828-832, 844-846, 872-873, 877, 896, and 956.src/translations/it.json#L815-L815: translate the English values at Lines 815, 819-821, 828-832, 844-846, 872-873, 877, 896, and 956.src/translations/pl.json#L815-L815: translate the English values at Lines 815, 819-821, 828-832, 844-846, 872-873, 877, 896, and 956.
📍 Affects 6 files
src/translations/ar.json#L815-L815(this comment)src/translations/de.json#L815-L815src/translations/es.json#L815-L815src/translations/fr.json#L815-L815src/translations/it.json#L815-L815src/translations/pl.json#L815-L815
🤖 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/translations/ar.json` at line 815, Translate the shared incident-command
and authorization strings currently retaining English values in all six
catalogs: src/translations/ar.json, src/translations/de.json,
src/translations/es.json, src/translations/fr.json, src/translations/it.json,
and src/translations/pl.json. Update the entries at lines 815, 819-821, 828-832,
844-846, 872-873, 877, 896, and 956, covering incident chat states, channel
messages, direct-message errors, authorization messages, open_chat,
send_message, and login.dispatch_not_authorized, while preserving the existing
keys and placeholders.
| "warning_count": "{{count}} varning", | ||
| "enable_timers": "Aktivera timrar", | ||
| "disable_timers": "Inaktivera timrar", | ||
| "summary": "{{overdue}} försenade, {{warning}} varning, {{ok}} ok", |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Add count-aware Swedish pluralization.
warning_count and summary always use singular varning after interpolated counts. The UI displays incorrect Swedish when more than one warning exists. Use the locale's pluralization convention for these messages.
🤖 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/translations/sv.json` around lines 486 - 489, Update the Swedish
translation entries warning_count and summary to use the locale’s count-aware
pluralization convention, preserving the existing interpolation keys and
singular wording for a count of one while using the correct plural form for
counts greater than one.
| "chat_frozen": "This incident is closed. Conversations are kept as a point-in-time record — no new messages.", | ||
| "close_all_channels": "Stäng alla kanaler", | ||
| "close_command": "Avsluta ledning", | ||
| "closed": "Avslutad", | ||
| "command_channel": "Command chat", | ||
| "command_channel_hint": "Command staff and dispatch", | ||
| "command_channel_unavailable": "No command channel has been created for this incident yet.", | ||
| "commander": "Insatsledare", | ||
| "complete": "Slutför", | ||
| "completed": "Slutförd", | ||
| "confirm_close": "Avsluta insatsledningen för det här larmet?", | ||
| "critical": "Kritisk", | ||
| "delete_annotation_confirm": "Ta bort den här anteckningen?", | ||
| "dispatch_channel": "Dispatch", | ||
| "dispatch_channel_hint": "The incident's line to the desk", | ||
| "dispatch_channel_unavailable": "No dispatch channel has been created for this incident yet.", | ||
| "dm_failed": "Couldn't open that conversation.", | ||
| "dm_unavailable": "That contact has no Resgrid account to message.", | ||
| "due": "Förfaller", | ||
| "edit": "Redigera", | ||
| "edit_action_plan": "Redigera insatsplan", | ||
| "establish": "Upprätta ledning", | ||
| "establish_description": "Skapa valfritt ledningstavlan från en mall.", | ||
| "establish_error": "Det gick inte att upprätta ledningen", | ||
| "establish_success": "Insatsledningen har upprättats", | ||
| "establish_title": "Upprätta insatsledning", | ||
| "established_on": "Upprättad", | ||
| "green": "Grön", | ||
| "hold_to_talk": "Håll in för att tala", | ||
| "incident_channel": "Incident chat", | ||
| "incident_channel_hint": "Everyone working the incident", | ||
| "incident_channel_unavailable": "No incident channel has been created for this call yet.", |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Complete the translations for incident-command and login flows.
Both locale catalogs contain English user-facing values in these ranges. This creates mixed-language UI during chat, authorization, direct-message, and login flows.
src/translations/sv.json#L815-L846: Translate the English incident-command channel, chat, and direct-message values into Swedish.src/translations/sv.json#L872-L877: Translate the authorization andopen_chatvalues into Swedish.src/translations/sv.json#L896-L896: Translatesend_messageinto Swedish.src/translations/sv.json#L956-L956: Translatelogin.dispatch_not_authorizedinto Swedish.src/translations/uk.json#L815-L846: Translate the English incident-command channel, chat, and direct-message values into Ukrainian.src/translations/uk.json#L872-L877: Translate the authorization andopen_chatvalues into Ukrainian.src/translations/uk.json#L896-L896: Translatesend_messageinto Ukrainian.src/translations/uk.json#L956-L956: Translatelogin.dispatch_not_authorizedinto Ukrainian.
📍 Affects 2 files
src/translations/sv.json#L815-L846(this comment)src/translations/sv.json#L872-L877src/translations/sv.json#L896-L896src/translations/sv.json#L956-L956src/translations/uk.json#L815-L846src/translations/uk.json#L872-L877src/translations/uk.json#L896-L896src/translations/uk.json#L956-L956
🤖 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/translations/sv.json` around lines 815 - 846, Translate every specified
English user-facing value in the Swedish and Ukrainian locale catalogs, covering
incident-command channels, chat and direct-message labels, authorization and
open_chat entries, send_message, and login.dispatch_not_authorized. Update
src/translations/sv.json ranges 815-846, 872-877, 896, and 956 with Swedish
text, and the corresponding ranges in src/translations/uk.json with Ukrainian
text; preserve all translation keys and JSON structure.
| "edit": "Redigera", | ||
| "edit_action_plan": "Redigera insatsplan", | ||
| "establish": "Upprätta ledning", | ||
| "establish_description": "Skapa valfritt ledningstavlan från en mall.", |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Correct the Swedish grammar in establish_description.
Skapa valfritt ledningstavlan från en mall. is not grammatical Swedish.
Proposed fix
- "establish_description": "Skapa valfritt ledningstavlan från en mall.",
+ "establish_description": "Skapa vid behov en ledningstavla från en mall.",📝 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.
| "establish_description": "Skapa valfritt ledningstavlan från en mall.", | |
| "establish_description": "Skapa vid behov en ledningstavla från en mall.", |
🤖 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/translations/sv.json` at line 837, Correct the Swedish wording of the
establish_description translation so it is grammatically valid, while preserving
its intended meaning: creating an optional leaderboard from a template.
| const response = await api.get<ChatV4Response<ChatChannelResultData[]>>(`${CHAT}/GetChannels`, { | ||
| params: activeUnitId != null ? { activeUnitId } : undefined, | ||
| params: Object.keys(params).length > 0 ? params : undefined, | ||
| signal, | ||
| }); |
There was a problem hiding this comment.
Uncaught network exception: The external HTTP call api.get lacks a try/catch with context mapping, violating rule [27]. Wrap the call to map exceptions to application-level errors using AppError.
Kody rule violation: Add try-catch blocks for external calls
Prompt for LLM
File src/api/chat/chat.ts:
Line 47 to 50:
Uncaught network exception: The external HTTP call `api.get` lacks a try/catch with context mapping, violating rule [27]. Wrap the call to map exceptions to application-level errors using `AppError`.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| isInitializing.current = false; | ||
|
|
||
| // Stop hubs, voice, audio and timers that belong to the ended session | ||
| void teardownSignedInSession(); |
There was a problem hiding this comment.
Unhandled Promise rejection: Calling teardownSignedInSession() with void discards its Promise, meaning errors thrown before the internal loop (e.g., useSignalRStore.getState() on line 53) violate rule [1]. Attach a .catch() handler or wrap the entire function body in try/catch.
Kody rule violation: Handle async operations with proper error handling
Prompt for LLM
File src/app/(app)/_layout.tsx:
Line 437:
Unhandled Promise rejection: Calling `teardownSignedInSession()` with `void` discards its Promise, meaning errors thrown before the internal loop (e.g., `useSignalRStore.getState()` on line 53) violate rule [1]. Attach a `.catch()` handler or wrap the entire function body in try/catch.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
|
|
||
| const renderCallItem = useCallback( | ||
| ({ item }: { item: CallResultData }) => ( | ||
| <Pressable onPress={() => router.push(`/call/${item.CallId}` as Href)}> |
There was a problem hiding this comment.
Performance degradation: Inline arrow functions in JSX props create new functions on every render. Move these function definitions outside the render method.
Kody rule violation: Avoid using .bind() or arrow functions in JSX props
Prompt for LLM
File src/app/(app)/calls.tsx:
Line 80:
Performance degradation: Inline arrow functions in JSX props create new functions on every render. Move these function definitions outside the render method.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| .filter((call) => call.CallId.toLowerCase().includes(searchQuery.toLowerCase()) || (call.Nature?.toLowerCase() || '').includes(searchQuery.toLowerCase())); | ||
| const filteredCalls = useMemo(() => { | ||
| const query = searchQuery.toLowerCase(); | ||
| return calls.filter((call) => call.State !== CallState.SCHEDULED).filter((call) => call.CallId.toLowerCase().includes(query) || (call.Nature?.toLowerCase() || '').includes(query)); |
There was a problem hiding this comment.
Redundant array iteration: Two chained .filter() calls iterate the calls array twice per render, violating rule 97. Combine both predicates into a single .filter() call to optimize performance.
Kody rule violation: Optimize chained array operations
Prompt for LLM
File src/app/(app)/calls.tsx:
Line 68:
Redundant array iteration: Two chained `.filter()` calls iterate the `calls` array twice per render, violating rule 97. Combine both predicates into a single `.filter()` call to optimize performance.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| centerCoordinate: [location.longitude, location.latitude], | ||
| zoomLevel: location.isMapLocked ? 16 : 12, | ||
| centerCoordinate: [longitude, latitude], | ||
| zoomLevel: locked ? 16 : 12, |
There was a problem hiding this comment.
Magic numbers: Inline zoom levels 16 and 12 are repeated across multiple effects and handlers in map.tsx. Define named constants like LOCKED_ZOOM_LEVEL and UNLOCKED_ZOOM_LEVEL at module scope.
Kody rule violation: Replace magic numbers with named constants
Prompt for LLM
File src/app/(app)/map.tsx:
Line 185:
Magic numbers: Inline zoom levels 16 and 12 are repeated across multiple effects and handlers in `map.tsx`. Define named constants like `LOCKED_ZOOM_LEVEL` and `UNLOCKED_ZOOM_LEVEL` at module scope.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| useToastStore.getState().showToast('error', t('incident_command.dm_failed')); | ||
| return; | ||
| } | ||
| router.push(`/chat/${channelId}`); |
There was a problem hiding this comment.
Hardcoded route literal: The route path /chat/${channelId} is inlined as a raw string, violating rule 6. Define a centralized routes constant or route-builder function to prevent drift.
Kody rule violation: Centralize string constants
Prompt for LLM
File src/hooks/use-direct-message.ts:
Line 35:
Hardcoded route literal: The route path `/chat/${channelId}` is inlined as a raw string, violating rule 6. Define a centralized routes constant or route-builder function to prevent drift.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| router.push(`/chat/${channelId}`); | ||
| } catch (error) { | ||
| logger.error({ message: 'chat: failed to open direct message', context: { error, targetUserId } }); | ||
| useToastStore.getState().showToast('error', t('incident_command.dm_failed')); |
There was a problem hiding this comment.
Magic string: The toast severity 'error' is repeated multiple times, violating rule 8. Replace these string literals with a ToastType enum to prevent typos and drift.
Kody rule violation: Use enums instead of magic strings
Prompt for LLM
File src/hooks/use-direct-message.ts:
Line 38:
Magic string: The toast severity `'error'` is repeated multiple times, violating rule 8. Replace these string literals with a `ToastType` enum to prevent typos and drift.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| if (!raw) return null; | ||
| try { | ||
| return JSON.parse(raw) as SamlPendingState; | ||
| } catch { |
There was a problem hiding this comment.
Silent error swallowing: The catch block in use-saml-login.ts hides potential JSON.parse security issues like injection attempts by returning null. Log the error with context before returning null to expose corrupted storage states.
Kody rule violation: Avoid empty catch blocks
Prompt for LLM
File src/hooks/use-saml-login.ts:
Line 42:
Silent error swallowing: The catch block in `use-saml-login.ts` hides potential `JSON.parse` security issues like injection attempts by returning null. Log the error with context before returning null to expose corrupted storage states.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| const raw = getItem<string>(SAML_PENDING_STATE_KEY); | ||
| if (!raw) return null; | ||
| try { | ||
| return JSON.parse(raw) as SamlPendingState; |
There was a problem hiding this comment.
Security bypass risk: Type-asserting JSON.parse output as SamlPendingState allows tampered storage to bypass the SAML_FLOW_MAX_AGE_MS expiry check and weaken CSRF nonce comparisons. Validate the shape of the parsed object to ensure nonce and startedAt exist before returning.
Kody rule violation: Always validate JSON parsing
Prompt for LLM
File src/hooks/use-saml-login.ts:
Line 41:
Security bypass risk: Type-asserting `JSON.parse` output as `SamlPendingState` allows tampered storage to bypass the `SAML_FLOW_MAX_AGE_MS` expiry check and weaken CSRF nonce comparisons. Validate the shape of the parsed object to ensure `nonce` and `startedAt` exist before returning.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| try { | ||
| const response = await chatApi.getChannels(undefined, true); | ||
| const forCall = (response.Data ?? []).filter((channel) => channel.CallId === numericCallId); | ||
| set((state) => ({ incidentChannelsByCallId: { ...state.incidentChannelsByCallId, [callId]: forCall } })); |
There was a problem hiding this comment.
Performance bottleneck: loadIncidentChannels fetches all archived department channels via getChannels(undefined, true) and filters client-side, loading hundreds of unnecessary objects per tab open. Pass a callId query param to the API for server-side filtering, or cache results in incidentChannelsByCallId.
// Skip if already loaded for this call
if (get().incidentChannelsByCallId[callId]) return;
// Prefer a server-side filter when available:
// const response = await chatApi.getIncidentChannels(numericCallId);
const response = await chatApi.getChannels(undefined, true);
const forCall = (response.Data ?? []).filter((channel) => channel.CallId === numericCallId);Prompt for LLM
File src/stores/chat/store.ts:
Line 264 to 267:
Performance bottleneck: `loadIncidentChannels` fetches all archived department channels via `getChannels(undefined, true)` and filters client-side, loading hundreds of unnecessary objects per tab open. Pass a `callId` query param to the API for server-side filtering, or cache results in `incidentChannelsByCallId`.
Suggested Code:
// Skip if already loaded for this call
if (get().incidentChannelsByCallId[callId]) return;
// Prefer a server-side filter when available:
// const response = await chatApi.getIncidentChannels(numericCallId);
const response = await chatApi.getChannels(undefined, true);
const forCall = (response.Data ?? []).filter((channel) => channel.CallId === numericCallId);
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| try { | ||
| const response = await chatApi.getChannels(undefined, true); | ||
| const forCall = (response.Data ?? []).filter((channel) => channel.CallId === numericCallId); | ||
| set((state) => ({ incidentChannelsByCallId: { ...state.incidentChannelsByCallId, [callId]: forCall } })); |
There was a problem hiding this comment.
Network bandwidth waste: loadIncidentChannels fetches every archived department channel via getChannels(undefined, true) and filters client-side, transferring thousands of objects on every tab open. Add an optional callId filter parameter to getChannels or call a dedicated server-side endpoint to reduce network and memory costs.
// Filter server-side by CallId to avoid loading the entire department's channel history
const response = await chatApi.getChannels(undefined, true, undefined, numericCallId);
const forCall = response.Data ?? [];Prompt for LLM
File src/stores/chat/store.ts:
Line 264 to 267:
Network bandwidth waste: `loadIncidentChannels` fetches every archived department channel via `getChannels(undefined, true)` and filters client-side, transferring thousands of objects on every tab open. Add an optional `callId` filter parameter to `getChannels` or call a dedicated server-side endpoint to reduce network and memory costs.
Suggested Code:
// Filter server-side by CallId to avoid loading the entire department's channel history
const response = await chatApi.getChannels(undefined, true, undefined, numericCallId);
const forCall = response.Data ?? [];
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| const forCall = (response.Data ?? []).filter((channel) => channel.CallId === numericCallId); | ||
| set((state) => ({ incidentChannelsByCallId: { ...state.incidentChannelsByCallId, [callId]: forCall } })); | ||
| } catch (error) { | ||
| logger.error({ message: 'chat: failed to load incident channels', context: { error, callId } }); |
There was a problem hiding this comment.
Unstructured logging: The operation name is embedded only in the message string, violating rule [3]. Add an explicit operation field like op: 'loadIncidentChannels' in the logger context for searchable, queryable logs.
Kody rule violation: Include error context in structured logs
Prompt for LLM
File src/stores/chat/store.ts:
Line 269:
Unstructured logging: The operation name is embedded only in the message string, violating rule [3]. Add an explicit operation field like `op: 'loadIncidentChannels'` in the logger context for searchable, queryable logs.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| // ------------------------------------------------------------------ | ||
| incidentChannelsByCallId: {}, | ||
|
|
||
| loadIncidentChannels: async (callId: string) => { |
There was a problem hiding this comment.
Missing documentation: The async function loadIncidentChannels lacks JSDoc for its Promise return value and rejection conditions, violating rule [22]. Add a @returns {Promise<Type>} block to document resolve and rejection behavior.
Kody rule violation: Document async/Promise behavior and errors
Prompt for LLM
File src/stores/chat/store.ts:
Line 258:
Missing documentation: The async function `loadIncidentChannels` lacks JSDoc for its Promise return value and rejection conditions, violating rule [22]. Add a `@returns {Promise<Type>}` block to document resolve and rejection behavior.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| canUserCreateMessages: store.rights?.CanCreateMessage, | ||
| canUserViewPII: store.rights?.CanViewPII, | ||
| // Undefined (rights not loaded yet) is treated as allowed by callers; only an explicit false blocks. | ||
| canUserWorkCommand: store.rights?.CanLoginToCommandApp, |
There was a problem hiding this comment.
Authorization bypass vulnerability: The canUserWorkCommand permission flag returns undefined when store.rights hasn't loaded, causing callers to fail open and grant unauthorized Command App access. Default the derived flag to false using ?? false, and apply this deny-by-default fix to sibling flags like canUserCreateNotes and canUserViewPII.
Kody rule violation: Implement RBAC with least privilege and deny-by-default
Prompt for LLM
File src/stores/security/store.ts:
Line 84:
Authorization bypass vulnerability: The `canUserWorkCommand` permission flag returns `undefined` when `store.rights` hasn't loaded, causing callers to fail open and grant unauthorized Command App access. Default the derived flag to `false` using `?? false`, and apply this deny-by-default fix to sibling flags like `canUserCreateNotes` and `canUserViewPII`.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
Code Review Completed! 🔥The code review was successfully completed based on your current configurations. Kody Guide: Usage and ConfigurationInteracting with Kody
Current Kody ConfigurationReview OptionsThe following review options are enabled or disabled:
|
|
Approve |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with 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.
Inline comments:
In `@src/stores/auth/__tests__/token-refresh-race.test.ts`:
- Around line 18-22: Update the authResponse fixture in
token-refresh-race.test.ts to declare it directly as AuthResponse instead of
using a type assertion, and provide the required id_token, token_type, and
expiration_date fields alongside the existing values.
- Around line 51-55: Update logout and the token-refresh coordination so signing
out invalidates any in-flight refresh before awaiting
clearPasswordVerificationHash, preventing applyAuthResponse from restoring the
session. Revise the race test around logout and clearPasswordVerificationHash to
use a deferred cleanup promise, invoke the real logout flow, resolve the refresh
during cleanup, and assert the signed-out state and false refresh result.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 8c98143f-f55a-4e33-997f-97a98765b2b1
📒 Files selected for processing (12)
src/api/chat/chat.tssrc/app/(app)/_layout.tsxsrc/app/(app)/chat.tsxsrc/app/(app)/map.tsxsrc/components/incident-command/command-board-view.tsxsrc/hooks/use-direct-message.tssrc/lib/auth/__tests__/token-refresh.test.tssrc/lib/auth/token-refresh.tssrc/stores/app/livekit-store.tssrc/stores/auth/__tests__/token-refresh-race.test.tssrc/stores/auth/store.tsxsrc/stores/chat/store.ts
🚧 Files skipped from review as they are similar to previous changes (8)
- src/stores/auth/store.tsx
- src/hooks/use-direct-message.ts
- src/app/(app)/_layout.tsx
- src/stores/app/livekit-store.ts
- src/stores/chat/store.ts
- src/app/(app)/map.tsx
- src/lib/auth/token-refresh.ts
- src/components/incident-command/command-board-view.tsx
| const authResponse = { | ||
| access_token: 'new-access', | ||
| refresh_token: 'new-refresh', | ||
| expires_in: 3600, | ||
| } as AuthResponse; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n -A12 -B2 'interface AuthResponse' src/lib/auth/types.tsx
rg -n -A8 -B2 'const authResponse|as AuthResponse' src/stores/auth/__tests__/token-refresh-race.test.tsRepository: Resgrid/Dispatch
Length of output: 861
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- test file ---'
cat -n src/stores/auth/__tests__/token-refresh-race.test.ts | sed -n '1,180p'
printf '%s\n' '--- AuthResponse usages ---'
rg -n -S 'AuthResponse|id_token|expiration_date|token_type' src/stores src/lib | head -200
printf '%s\n' '--- TypeScript configuration ---'
fd -a -i 'tsconfig*.json' . -x sh -c 'echo "--- $1"; cat "$1"' sh {} \;
printf '%s\n' '--- Static fixture check ---'
python3 - <<'PY'
from pathlib import Path
import re
types = Path("src/lib/auth/types.tsx").read_text()
test = Path("src/stores/auth/__tests__/token-refresh-race.test.ts").read_text()
fields = re.findall(r"^\s+([A-Za-z_]\w*)\s*:", types[types.index("export interface AuthResponse {"):types.index("export interface LoginResponse {")], re.M)
fixture = re.search(r"const authResponse\s*=\s*\{(.*?)\}\s*as AuthResponse", test, re.S)
fixture_fields = re.findall(r"^\s+([A-Za-z_]\w*)\s*:", fixture.group(1), re.M) if fixture else []
print("AuthResponse fields:", fields)
print("Fixture fields:", fixture_fields)
print("Missing fields:", [field for field in fields if field not in fixture_fields])
PYRepository: Resgrid/Dispatch
Length of output: 7248
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- token refresh implementation ---'
cat -n src/lib/auth/token-refresh.ts | sed -n '1,120p'
printf '%s\n' '--- auth response application ---'
cat -n src/stores/auth/store.tsx | sed -n '235,285p'
printf '%s\n' '--- complete AuthResponse type ---'
cat -n src/lib/auth/types.tsx | sed -n '1,25p'
printf '%s\n' '--- Test type-safety probe ---'
python3 - <<'PY'
from pathlib import Path
import re
types = Path("src/lib/auth/types.tsx").read_text()
test = Path("src/stores/auth/__tests__/token-refresh-race.test.ts").read_text()
interface = re.search(r"export interface AuthResponse\s*\{(.*?)\}", types, re.S)
fixture = re.search(r"const authResponse\s*=\s*\{(.*?)\}\s*as AuthResponse", test, re.S)
required = re.findall(r"^\s+([A-Za-z_]\w*)\s*:\s*([^;]+);", interface.group(1), re.M)
provided = re.findall(r"^\s+([A-Za-z_]\w*)\s*:", fixture.group(1), re.M)
print("Required AuthResponse fields:", [name for name, _ in required])
print("Provided fixture fields:", provided)
print("Assertion bypasses missing-field diagnostics:", set(name for name, _ in required) - set(provided))
PYRepository: Resgrid/Dispatch
Length of output: 6959
Use a complete AuthResponse fixture.
The fixture omits id_token, token_type, and expiration_date. The as AuthResponse assertion suppresses this type error. Declare the fixture as AuthResponse and provide all required fields.
🤖 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/stores/auth/__tests__/token-refresh-race.test.ts` around lines 18 - 22,
Update the authResponse fixture in token-refresh-race.test.ts to declare it
directly as AuthResponse instead of using a type assertion, and provide the
required id_token, token_type, and expiration_date fields alongside the existing
values.
Source: Coding guidelines
| // Logout wins the race: store cleared and signed out | ||
| useAuthStore.setState({ status: 'signedOut', accessToken: null, refreshToken: null, profile: null, userId: null }); | ||
|
|
||
| resolveRefresh(authResponse); | ||
| await expect(refreshPromise).resolves.toBe(false); |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
ast-grep outline src/stores/auth/store.tsx --items all
rg -n -C4 --glob '*.{ts,tsx}' 'performTokenRefresh|initTokenRefresh|applyAuthResponse|signOut|logout' src/stores/auth src/lib/authRepository: Resgrid/Dispatch
Length of output: 12144
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- auth store logout implementation ---'
sed -n '110,170p' src/stores/auth/store.tsx
printf '%s\n' '--- token refresh implementation ---'
sed -n '1,130p' src/lib/auth/token-refresh.ts
printf '%s\n' '--- race test ---'
cat -n src/stores/auth/__tests__/token-refresh-race.test.tsRepository: Resgrid/Dispatch
Length of output: 7980
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- cleanup implementation and response type ---'
rg -n -C6 'clearPasswordVerificationHash|interface AuthResponse|type AuthResponse' src/lib src/stores
printf '%s\n' '--- deterministic source-order check ---'
python3 - <<'PY'
from pathlib import Path
store = Path("src/stores/auth/store.tsx").read_text()
logout_start = store.index("logout: async () =>")
logout_end = store.index("refreshAccessToken: async () =>", logout_start)
logout = store[logout_start:logout_end]
cleanup = logout.index("await clearPasswordVerificationHash();")
signed_out = logout.index("status: 'signedOut'")
print({
"cleanup_is_awaited": "await clearPasswordVerificationHash();" in logout,
"signed_out_assignment_after_cleanup": signed_out > cleanup,
"timer_cancelled_before_cleanup": logout.index("cancelScheduledTokenRefresh();") < cleanup,
})
PYRepository: Resgrid/Dispatch
Length of output: 5879
Make sign-out invalidate the in-flight refresh.
logout() awaits clearPasswordVerificationHash() before setting status to signedOut. If the refresh resolves during cleanup, applyAuthResponse can restore the session. Set the signed-out state before cleanup or add explicit refresh invalidation. Test this path with a deferred cleanup promise instead of calling setState directly.
🤖 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/stores/auth/__tests__/token-refresh-race.test.ts` around lines 51 - 55,
Update logout and the token-refresh coordination so signing out invalidates any
in-flight refresh before awaiting clearPasswordVerificationHash, preventing
applyAuthResponse from restoring the session. Revise the race test around logout
and clearPasswordVerificationHash to use a deferred cleanup promise, invoke the
real logout flow, resolve the refresh during cleanup, and assert the signed-out
state and false refresh result.
| await currentRoom.disconnect(); | ||
| await audioService.playDisconnectedFromAudioRoomSound(); | ||
| currentRoom.removeAllListeners(); | ||
| await currentRoom.disconnect(); |
There was a problem hiding this comment.
Unhandled rejection occurs in src/stores/app/livekit-store.ts:413 and 417 because the currentRoom.disconnect() external network call is wrapped only in try/finally, violating Rule 27 by propagating teardown failures uncaught and unlogged. Add a catch block to log structured context (e.g., room SID, currentRoomInfo) before rethrowing a mapped application error.
Kody rule violation: Add try-catch blocks for external calls
Prompt for LLM
File src/stores/app/livekit-store.ts:
Line 412:
Unhandled rejection occurs in `src/stores/app/livekit-store.ts:413` and `417` because the `currentRoom.disconnect()` external network call is wrapped only in `try/finally`, violating Rule 27 by propagating teardown failures uncaught and unlogged. Add a `catch` block to log structured context (e.g., room SID, `currentRoomInfo`) before rethrowing a mapped application error.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
2 participants
PR Description: RC-T39 Dispatch Fixes
This PR delivers authorization controls, incident command chat integration, authentication reliability fixes, and significant performance improvements across the Dispatch app.
Authorization & Access Control
CanLoginToDispatchApppermission. Users not authorized for Dispatch are now signed out during initialization with a clear toast message rather than landing in an empty app.CanLoginToCommandApppermission. The Incident Command tab now shows a "not authorized" state instead of attempting to load boards that will fail with 403s.truefor departments that haven't configured them.Incident Command Chat
useDirectMessagehook handles opening or reusing 1:1 conversations (server deduplicates).Authentication Reliability
token-refreshmodule so the automatic refresh timer and the axios 401 interceptor never rotate the refresh token in parallel (which would invalidate one request).Session Lifecycle
teardownSignedInSessionthat disconnects SignalR hubs, LiveKit rooms, audio streams, check-in polling, chat timers, and push notification listeners — preventing resources from running against a signed-out session.Performance Optimizations
Bug Fixes
AbortControlleron stop, preventing listener accumulation across play/stop cycles.Summary by CodeRabbit
New Features
Bug Fixes