Conversation
Signing out while initializeApp was still awaiting left the stale run free to connect the SignalR hubs and mark the app initialized for a session that had already ended. Capture a generation token at the start of each run, bump it on sign-out, and bail at every checkpoint that is no longer current. The init promise's finally handler is guarded too, so a run that lost the timeout race can no longer clear the in-progress guard a newer run owns. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01X8YKbDjQeSLXJs4kU1qdXe
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:
|
📝 WalkthroughWalkthroughThe PR guards app initialization by session generation, expands SignalR argument and lifecycle handling, adds chat connection recovery, normalizes chat metadata, and limits unsupported chat actions. It also adds tests for initialization, composer behavior, and chat hub payloads. ChangesSession initialization
SignalR chat realtime
Chat UI and metadata
Sentry logging
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant SignalRService
participant signalr-store
participant ChatState
SignalRService->>signalr-store: emit reconnect or disconnect
signalr-store->>SignalRService: arm chat connection and invoke Connect
SignalRService-->>signalr-store: forward positional hub arguments
signalr-store->>ChatState: resynchronize chat state
signalr-store->>signalr-store: restart heartbeat after connection
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✨ Finishing Touches 💡 2📝 Generate docstrings 💡
🛠️ Fix failing CI checks 💡
🧪 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 |
|
Approve |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/services/signalr.service.ts (1)
418-426: 🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy liftEmit a reconnected lifecycle event after fallback reconnection.
SignalRService.HUB_RECONNECTED_EVENTis only emitted fromonreconnected.handleConnectionClosecreates and starts a new connection, but_connectToHubInternalandconnectToHubWithEventingUrlonly emitHUB_DISCONNECTED_EVENTafter the prior session closes.When the chat store receives that restart path, it skips re-arming because
isChatHubConnectedis still true andconnectChatHubexits at line 727. Track when a fresh connection replaces a disconnected connection, then emit the reconnected lifecycle event afterconnection.start()for that case so chat can callConnectfor the new session.🤖 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/signalr.service.ts` around lines 418 - 426, Track whether fallback reconnection in handleConnectionClose creates a replacement for a previously disconnected session, carrying that state through _connectToHubInternal and connectToHubWithEventingUrl. After the replacement connection successfully completes connection.start(), emit SignalRService.HUB_RECONNECTED_EVENT for the hub, while preserving the existing HUB_DISCONNECTED_EVENT and onreconnected behavior for their respective paths.
🧹 Nitpick comments (1)
src/app/chat/thread/[messageId].tsx (1)
133-133: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winUse a stable
onTypingcallback.
onTyping={() => undefined}creates a new callback on everyThreadScreenrender.MessageComposerusesonTypingin callback dependencies. Define a memoized no-op callback before rendering, then pass that callback toMessageComposer.As per coding guidelines, avoid anonymous functions in 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/chat/thread/`[messageId].tsx at line 133, Define a memoized no-op onTyping callback in ThreadScreen before the JSX render, then pass that stable callback to MessageComposer instead of the inline arrow function. Keep the callback behavior unchanged.Source: Coding guidelines
🤖 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)/__tests__/init-session-generation.test.tsx:
- Around line 70-92: Add stale-session coverage for every await boundary in the
initialization flow exercised by useInitGuard and the layout: gate each stage
with deferred promises, sign out immediately after each gate resolves, then
assert the next stage and all subsequent effects are not called. Extend the
existing “abandons an in-flight run” test structure without changing the valid
active-session path.
In `@src/app/`(app)/_layout.tsx:
- Around line 171-172: In src/app/(app)/_layout.tsx#L171-L172, update the
session initialization flow to check isCurrentRun() after every awaited
initialization stage and before each subsequent session-scoped operation,
returning immediately when the run is stale. In
src/app/(app)/__tests__/init-session-generation.test.tsx#L70-L92, add deferred
gates for each initialization stage and assert that signing out prevents all
later stages from executing.
- Around line 374-379: Update the status === 'signedOut' cleanup path in the app
layout to also reset hasInitialized.current to false, alongside
initGeneration.current and isInitializing.current. Preserve the existing
in-flight initialization invalidation so the next signed-in session can
initialize normally.
In `@src/components/chat/chat-utils.ts`:
- Around line 96-99: Update parseLocationMetadata to validate parsed latitude
and longitude ranges before returning the location: latitude must be between -90
and 90 inclusive, and longitude between -180 and 180 inclusive. Return null for
coordinates outside those bounds while preserving the existing handling of
missing values and valid metadata.
- Around line 91-115: Add Jest tests for parseLocationMetadata and
parseGifMetadata covering nested and legacy flat payloads, malformed JSON,
invalid or missing location coordinates, and GIF metadata with optional fields
present or absent. Assert normalized output values and null results for invalid
inputs, using the existing test conventions and helpers.
In `@src/stores/signalr/signalr-store.ts`:
- Around line 89-115: Add an arm-operation generation or cancellation token
shared by runChatArm, armChatSession, and disconnectChatHub, and invalidate it
when disconnecting. In runChatArm, verify the operation is still current after
Connect resolves or rejects before scheduling retries, starting the heartbeat,
or invoking resyncChat; ensure retry timer callbacks handle the returned promise
without unhandled rejections. Preserve normal retry behavior for the active
connection generation.
---
Outside diff comments:
In `@src/services/signalr.service.ts`:
- Around line 418-426: Track whether fallback reconnection in
handleConnectionClose creates a replacement for a previously disconnected
session, carrying that state through _connectToHubInternal and
connectToHubWithEventingUrl. After the replacement connection successfully
completes connection.start(), emit SignalRService.HUB_RECONNECTED_EVENT for the
hub, while preserving the existing HUB_DISCONNECTED_EVENT and onreconnected
behavior for their respective paths.
---
Nitpick comments:
In `@src/app/chat/thread/`[messageId].tsx:
- Line 133: Define a memoized no-op onTyping callback in ThreadScreen before the
JSX render, then pass that stable callback to MessageComposer instead of the
inline arrow function. Keep the callback behavior unchanged.
🪄 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: 4e1812c6-a2ef-4854-82ca-b8bc9df9c839
📒 Files selected for processing (16)
src/app/(app)/__tests__/init-session-generation.test.tsxsrc/app/(app)/_layout.tsxsrc/app/(app)/chatbot.tsxsrc/app/_layout.tsxsrc/app/chat/[channelId].tsxsrc/app/chat/thread/[messageId].tsxsrc/components/chat/__tests__/message-composer.test.tsxsrc/components/chat/chat-utils.tssrc/components/chat/message-actions-sheet.tsxsrc/components/chat/message-bubble.tsxsrc/components/chat/message-composer.tsxsrc/components/chat/new-conversation-sheet.tsxsrc/services/signalr.service.tssrc/stores/chat/__tests__/hub-invoke-args.test.tssrc/stores/chat/store.tssrc/stores/signalr/signalr-store.ts
| 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(); | ||
| }); |
There was a problem hiding this comment.
📐 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
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 around lines 70 -
92, Add stale-session coverage for every await boundary in the initialization
flow exercised by useInitGuard and the layout: gate each stage with deferred
promises, sign out immediately after each gate resolves, then assert the next
stage and all subsequent effects are not called. Extend the existing “abandons
an in-flight run” test structure without changing the valid active-session path.
Source: Coding guidelines
| if (!isCurrentRun()) return; | ||
|
|
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.
src/app/(app)/_layout.tsx#L171-L172: checkisCurrentRun()after each awaited initialization stage and before the next session-scoped operation.src/app/(app)/__tests__/init-session-generation.test.tsx#L70-L92: add deferred stage gates and assert that sign-out prevents every subsequent stage.
📍 Affects 2 files
src/app/(app)/_layout.tsx#L171-L172(this comment)src/app/(app)/__tests__/init-session-generation.test.tsx#L70-L92
🤖 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 171 - 172, In
src/app/(app)/_layout.tsx#L171-L172, update the session initialization flow to
check isCurrentRun() after every awaited initialization stage and before each
subsequent session-scoped operation, returning immediately when the run is
stale. In src/app/(app)/__tests__/init-session-generation.test.tsx#L70-L92, add
deferred gates for each initialization stage and assert that signing out
prevents all later stages from executing.
Source: Coding guidelines
|
|
||
| // 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; |
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.
layout only resets isInitializing on sign-out, while a completed run leaves hasInitialized.current === true. A kept-mounted (app) layout will skip the next signed-in session initialization. Add hasInitialized.current = false in the status === 'signedOut' cleanup path.
🤖 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 374 - 379, Update the status ===
'signedOut' cleanup path in the app layout to also reset hasInitialized.current
to false, alongside initGeneration.current and isInitializing.current. Preserve
the existing in-flight initialization invalidation so the next signed-in session
can initialize normally.
| export function parseLocationMetadata(metadataJson?: string | null): ChatLocationMetadata | null { | ||
| return parseMetadata<ChatLocationMetadata>(metadataJson); | ||
| const raw = parseMetadata<Record<string, unknown>>(metadataJson); | ||
| if (!raw) return null; | ||
| const nested = (raw.location ?? raw.Location) as Record<string, unknown> | undefined; | ||
| const source = nested ?? raw; | ||
| const latitude = readNumber(source.latitude ?? source.Latitude); | ||
| const longitude = readNumber(source.longitude ?? source.Longitude); | ||
| if (latitude === undefined || longitude === undefined) return null; | ||
| return { Latitude: latitude, Longitude: longitude, Label: readString(source.label ?? source.Label) }; | ||
| } | ||
|
|
||
| export function parseGifMetadata(metadataJson?: string | null): ChatGifMetadata | null { | ||
| return parseMetadata<ChatGifMetadata>(metadataJson); | ||
| const raw = parseMetadata<Record<string, unknown>>(metadataJson); | ||
| if (!raw) return null; | ||
| const nested = (raw.gif ?? raw.Gif) as Record<string, unknown> | undefined; | ||
| const source = nested ?? raw; | ||
| const url = readString(source.url ?? source.Url ?? source.gifUrl ?? source.GifUrl); | ||
| if (!url) return null; | ||
| return { | ||
| GifUrl: url, | ||
| PreviewUrl: readString(source.previewUrl ?? source.PreviewUrl), | ||
| Width: readNumber(source.width ?? source.Width), | ||
| Height: readNumber(source.height ?? source.Height), | ||
| Title: readString(source.title ?? source.Title), | ||
| }; |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
ast-grep outline src/components/chat/chat-utils.ts --items all
rg -n -C 4 'parseLocationMetadata|parseGifMetadata|buildLocationMetadata|buildGifMetadata' \
src/components/chat/__tests__/chat-utils.test.tsRepository: Resgrid/Dispatch
Length of output: 2266
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Test file existence and size:"
if [ -f src/components/chat/__tests__/chat-utils.test.ts ]; then
wc -l src/components/chat/__tests__/chat-utils.test.ts
echo
echo "Relevant test imports/descriptions/expectations:"
rg -n -C 3 'describe\(|it\(|test\(|expect\(|parseLocationMetadata|parseGifMetadata|buildLocationMetadata|buildGifMetadata|metadata' \
src/components/chat/__tests__/chat-utils.test.ts || true
else
echo "NotFound src/components/chat/__tests__/chat-utils.test.ts"
fi
echo
echo "Parsed metadata implementation:"
sed -n '55,120p' src/components/chat/__tests__/chat-utils.test.ts 2>/dev/null || true
sed -n '55,120p' src/components/chat/chat-utils.tsRepository: Resgrid/Dispatch
Length of output: 11561
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Tests for parseLocationMetadata/parseGifMetadata/importers:"
rg -n 'parseLocationMetadata|parseGifMetadata|chat-location|gif-metadata|LocationMetadata|GifMetadata' src || true
echo
echo "Jest coverage config files:"
for f in package.json jest.config.js jest.config.ts jest.config.json babel.config.js jest.setup.js; do
if [ -f "$f" ]; then
echo "--- $f"
sed -n '1,220p' "$f"
fi
doneRepository: Resgrid/Dispatch
Length of output: 13715
Add Jest coverage for metadata normalization.
Add tests for parseLocationMetadata and parseGifMetadata covering nested payloads, legacy flat payloads, malformed JSON, invalid coordinates, and optional GIF 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/components/chat/chat-utils.ts` around lines 91 - 115, Add Jest tests for
parseLocationMetadata and parseGifMetadata covering nested and legacy flat
payloads, malformed JSON, invalid or missing location coordinates, and GIF
metadata with optional fields present or absent. Assert normalized output values
and null results for invalid inputs, using the existing test conventions and
helpers.
Source: Coding guidelines
| const latitude = readNumber(source.latitude ?? source.Latitude); | ||
| const longitude = readNumber(source.longitude ?? source.Longitude); | ||
| if (latitude === undefined || longitude === undefined) return null; | ||
| return { Latitude: latitude, Longitude: longitude, Label: readString(source.label ?? source.Label) }; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Reject coordinates outside valid geographic ranges.
readNumber accepts values such as latitude 91 and longitude 181. parseLocationMetadata then returns them, and MessageBubble uses them in a map URL. Reject latitude outside -90..90 and longitude outside -180..180.
Proposed fix
const latitude = readNumber(source.latitude ?? source.Latitude);
const longitude = readNumber(source.longitude ?? source.Longitude);
- if (latitude === undefined || longitude === undefined) return null;
+ if (latitude === undefined || longitude === undefined || latitude < -90 || latitude > 90 || longitude < -180 || longitude > 180) return null;
return { Latitude: latitude, Longitude: longitude, Label: readString(source.label ?? source.Label) };📝 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.
| const latitude = readNumber(source.latitude ?? source.Latitude); | |
| const longitude = readNumber(source.longitude ?? source.Longitude); | |
| if (latitude === undefined || longitude === undefined) return null; | |
| return { Latitude: latitude, Longitude: longitude, Label: readString(source.label ?? source.Label) }; | |
| const latitude = readNumber(source.latitude ?? source.Latitude); | |
| const longitude = readNumber(source.longitude ?? source.Longitude); | |
| if ( | |
| latitude === undefined || | |
| longitude === undefined || | |
| latitude < -90 || | |
| latitude > 90 || | |
| longitude < -180 || | |
| longitude > 180 | |
| ) return null; | |
| return { Latitude: latitude, Longitude: longitude, Label: readString(source.label ?? source.Label) }; |
🤖 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/chat/chat-utils.ts` around lines 96 - 99, Update
parseLocationMetadata to validate parsed latitude and longitude ranges before
returning the location: latitude must be between -90 and 90 inclusive, and
longitude between -180 and 180 inclusive. Return null for coordinates outside
those bounds while preserving the existing handling of missing values and valid
metadata.
| async function runChatArm(): Promise<void> { | ||
| stopChatArmRetry(); | ||
|
|
||
| try { | ||
| await signalRService.invoke(Env.CHAT_HUB_NAME, 'Connect'); | ||
| } catch (error) { | ||
| chatArmAttempts += 1; | ||
| logger.warn({ | ||
| message: 'Failed to announce presence to chat hub', | ||
| context: { error, attempt: chatArmAttempts, maxAttempts: CHAT_ARM_MAX_ATTEMPTS }, | ||
| }); | ||
| if (chatArmAttempts < CHAT_ARM_MAX_ATTEMPTS) { | ||
| chatArmRetryTimer = setTimeout(() => { | ||
| void armChatSession(); | ||
| }, CHAT_ARM_RETRY_MS); | ||
| } | ||
| throw error; | ||
| } | ||
|
|
||
| chatArmAttempts = 0; | ||
|
|
||
| stopChatHeartbeat(); | ||
| chatHeartbeatTimer = setInterval(() => { | ||
| signalRService.invoke(Env.CHAT_HUB_NAME, 'Heartbeat').catch(() => { | ||
| // Heartbeat is best-effort; ignore transient failures. | ||
| }); | ||
| }, CHAT_HEARTBEAT_INTERVAL_MS); |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Find file:"
fd -a 'signalr-store\.ts$' . || true
echo "Outline:"
file="$(fd 'signalr-store\.ts$' . | head -n1)"
if [ -n "${file:-}" ]; then
wc -l "$file"
ast-grep outline "$file" --view compact || true
echo "Relevant excerpts:"
sed -n '1,180p' "$file" | cat -n
echo "Lines 780-845:"
sed -n '780,845p' "$file" | cat -n
echo "Search disconnect/chatArm symbols:"
rg -n "disconnectChatHub|runChatArm|armChatSession|chatHeartbeatTimer|chatArmRetryTimer|chatArmAttempts|resync|sync" "$file"
fiRepository: Resgrid/Dispatch
Length of output: 12208
🏁 Script executed:
#!/bin/bash
set -euo pipefail
file="$(fd 'signalr-store\.ts$' . | head -n1)"
if [ -n "${file:-}" ]; then
echo "Diff context around store references:"
git diff -- "$file" | sed -n '1,260p' || true
fi
echo "SignalR service invoke type/usages:"
rg -n "signalRService|invoke\(|class SignalR|interface .*SignalR|connectChatHub|disconnectChatHub" . -g '*.ts' -g '*.tsx'Repository: Resgrid/Dispatch
Length of output: 26060
Invalidate in-flight arm operations during disconnect.
disconnectChatHub stops the retry timer, but an armChatSession promise that is already awaiting signalRService.invoke(Env.CHAT_HUB_NAME, 'Connect') can still complete after disconnect. Use a cancellation token or generation check: clear the pending arm during disconnect, have runChatArm cancel or return before scheduling a retry, starting the heartbeat, or calling resyncChat, and handle the retry promise in the timer callback.
[st2021_and_safety_of_operation]
🤖 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/signalr/signalr-store.ts` around lines 89 - 115, Add an
arm-operation generation or cancellation token shared by runChatArm,
armChatSession, and disconnectChatHub, and invalidate it when disconnecting. In
runChatArm, verify the operation is still current after Connect resolves or
rejects before scheduling retries, starting the heartbeat, or invoking
resyncChat; ensure retry timer callbacks handle the returned promise without
unhandled rejections. Preserve normal retry behavior for the active connection
generation.
|
|
||
| await act(async () => { | ||
| first.resolve(); | ||
| await Promise.all([pending, second]); |
There was a problem hiding this comment.
Batch operation failure risk: await Promise.all([pending, second]) causes a single rejection to fail the entire batch. Replace it with Promise.allSettled to properly handle per-item results for independent tasks.
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:
Batch operation failure risk: `await Promise.all([pending, second])` causes a single rejection to fail the entire batch. Replace it with `Promise.allSettled` to properly handle per-item results for independent tasks.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| initGeneration.current += 1; | ||
| isInitializing.current = false; |
There was a problem hiding this comment.
State management bug: The sign-out handler resets isInitializing.current but not hasInitialized.current, latching the shouldInitialize gate (line 344) true and skipping initialization for the next session's SignalR hubs, feature flags, and weather alerts. Reset hasInitialized.current = false; alongside the isInitializing.current = false; assignment.
// 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 LLM
File src/app/(app)/_layout.tsx:
Line 378 to 379:
State management bug: The sign-out handler resets `isInitializing.current` but not `hasInitialized.current`, latching the `shouldInitialize` gate (line 344) true and skipping initialization for the next session's SignalR hubs, feature flags, and weather alerts. Reset `hasInitialized.current = false;` alongside the `isInitializing.current = false;` assignment.
Suggested Code:
// 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;
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| setEditMessage(m); | ||
| setEditText(m.Body ?? ''); | ||
| }} | ||
| 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 () => undefined callback definition outside the render method, including the instance in src/app/chat/thread/[messageId].tsx:133.
Kody rule violation: Avoid using .bind() or arrow functions in JSX props
Prompt for LLM
File src/app/(app)/chatbot.tsx:
Line 159:
Performance regression: Inline arrow functions in JSX props create new functions on every render. Move the `() => undefined` callback definition outside the render method, including the instance in `src/app/chat/thread/[messageId].tsx:133`.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| }); | ||
| if (chatArmAttempts < CHAT_ARM_MAX_ATTEMPTS) { | ||
| chatArmRetryTimer = setTimeout(() => { | ||
| void armChatSession(); |
There was a problem hiding this comment.
Unhandled Promise rejection: The retry timer fires void armChatSession() without a .catch handler, discarding the rejected Promise when runChatArm rethrows an invoke error and potentially crashing Node ≥15 processes. Attach .catch(() => { /* runChatArm already logged and rescheduled the retry */ }) to the armChatSession() call inside the setTimeout callback.
Kody rule violation: Handle async operations with proper error handling
Prompt for LLM
File src/stores/signalr/signalr-store.ts:
Line 102:
Unhandled Promise rejection: The retry timer fires `void armChatSession()` without a `.catch` handler, discarding the rejected Promise when `runChatArm` rethrows an invoke error and potentially crashing Node ≥15 processes. Attach `.catch(() => { /* runChatArm already logged and rescheduled the retry */ })` to the `armChatSession()` call inside the `setTimeout` callback.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| * every reconnect issues a fresh connection id. Without re-arming, the websocket stays | ||
| * open but the client receives nothing. | ||
| */ | ||
| async function runChatArm(): Promise<void> { |
There was a problem hiding this comment.
Missing JSDoc annotations: The runChatArm JSDoc block (lines 82–88) violates Rule 22 by omitting @returns {Promise<void>} and @throws for the async function. Add these tags to document the rejection when signalRService.invoke('Connect') fails after logging and scheduling a retry, and apply similarly at line 128.
Kody rule violation: Document async/Promise behavior and errors
Prompt for LLM
File src/stores/signalr/signalr-store.ts:
Line 89:
Missing JSDoc annotations: The `runChatArm` JSDoc block (lines 82–88) violates Rule 22 by omitting `@returns {Promise<void>}` and `@throws` for the async function. Add these tags to document the rejection when `signalRService.invoke('Connect')` fails after logging and scheduling a retry, and apply similarly at line 128.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
2 participants
Pull Request Description
This PR addresses several critical bugs in the chat system, fixes a startup race condition, and includes various robustness improvements.
Key Changes
Chat Realtime Connectivity Fixes (SignalR)
JoinChannel,Typing, andMarkReadwere sending fewer arguments than the hub methods declare, causing SignalR to reject the invocations and leaving the client permanently outside its channel groups (silent channels). All three now pass the full argument set includingasUnitId.Connectto re-enter channel groups, with retry logic (up to 3 attempts) and debounced resync.chatPresenceChangedsendsuserId, isOnlineas separate args instead of an object).Reactions/Attachmentsarrays are now normalized on the way in, and existing collections are preserved during partial updates so reactions are never dropped from the UI.App Initialization Guard
Chat Metadata Wire Format
{ location: { latitude, longitude } }) matching the web client's contract, replacing the flat PascalCase format the web client couldn't parse. Parsers accept both formats for backward compatibility with existing server history.UI Fixes
ReactionsandAttachmentson messages predating collection normalization.Tests Added