Skip to content

fix: WebRTC auto-reconnect on tab resume + Go Available reconnects first #341

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
coygg merged 1 commit into from
Apr 4, 2026
Merged

fix: WebRTC auto-reconnect on tab resume + Go Available reconnects first #341

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
37 changes: 33 additions & 4 deletions src/components/layout/header.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,8 @@ export function Header() {
const autoOfflineAttemptedRef = useRef(false)
const { wrapUpSecondsRemaining } = useWrapUpCountdown()
const { connectionState, reconnectPresence } = usePresence()
const { isRegistered, hasInitialized, currentCall } = useTelnyxContext()
const { isRegistered, hasInitialized, currentCall, reconnect, connectionStatus } = useTelnyxContext()
const isRegisteredRef = useRef(isRegistered)

// Sync status when user.agent_status changes mid-session (e.g. a call is routed to this agent).
// Without this, the dropdown stays editable even while the agent is on_call.
Expand All @@ -76,10 +77,29 @@ export function Header() {
return () => window.removeEventListener('agent-status-optimistic', handleOptimistic)
}, [])

useEffect(() => {
isRegisteredRef.current = isRegistered
}, [isRegistered])

const waitForRegistration = useCallback(async (timeoutMs = 12000) => {

@chatgpt-codex-connector chatgpt-codex-connector Bot Apr 4, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Increase Go Available wait window to cover reconnect backoff

waitForRegistration caps the Go Available recovery wait at 12 seconds, but reconnect attempts in triggerManualReconnect use exponential delays (3s/6s/12s… up to 30s) before init() even runs, and init() itself can take up to the registration timeout. In those common failure-recovery paths, this function returns false before the reconnect attempt can realistically complete, so agents get a hard “cannot go available” error even though the phone may recover shortly after.

Useful? React with 👍 / 👎.

const startedAt = Date.now()
while (Date.now() - startedAt < timeoutMs) {
if (isRegisteredRef.current) return true
await new Promise((resolve) => setTimeout(resolve, 300))
}
return false
}, [])

const handleStatusChange = useCallback(async (newStatus: AgentStatus): Promise<boolean> => {
if (newStatus === 'available' && !isRegistered) {
showPresenceToast('Cannot go available — phone not connected. Please refresh.', 'error')
return false
showPresenceToast('Phone reconnecting…', 'warning')
reconnect()

const recovered = await waitForRegistration()

@chatgpt-codex-connector chatgpt-codex-connector Bot Apr 4, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Guard stale Go Available requests from overwriting newer status

This handler now awaits registration before applying the status change, but there is no cancellation/version guard for in-flight requests. If an agent clicks “Go Available” while disconnected and then chooses another status before registration completes, the delayed first call continues and can overwrite the later selection by posting available after the second change has already succeeded.

Useful? React with 👍 / 👎.

if (!recovered) {
showPresenceToast('Cannot go available — phone not connected. Click Agent Diagnostic → Reconnect.', 'error')
return false
}
}
Comment on lines 93 to 103

@coderabbitai coderabbitai Bot Apr 4, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🧹 Nitpick | 🔵 Trivial

Minor: 'go-available' reason is never used.

The reconnect() function (defined at lines 1200-1209 in the hook) always passes 'manual-button' as the reason. While this still bypasses cooldown correctly, the 'go-available' reason added to the bypass list at line 670 is never actually triggered from this "Go Available" flow, making that part of the bypass check dead code.

If distinguishing "Go Available" reconnects from manual button clicks in logs is valuable, consider exposing the reason parameter:

// In UseTelnyxReturn interface
reconnect: (reason?: string) => void

// In the hook's reconnect callback
const reconnect = useCallback((reason = 'manual-button') => {
  manualReconnectFnRef.current?.(reason)
}, [])
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@src/components/layout/header.tsx` around lines 93 - 103, handleStatusChange
triggers reconnect() when moving to 'available', but the reconnect
implementation always uses the literal 'manual-button' reason so the
'go-available' bypass branch is never reached; update the reconnect signature in
the UseTelnyxReturn type to accept an optional reason (e.g., reconnect:
(reason?: string) => void), change the hook's reconnect callback (where
manualReconnectFnRef is invoked) to accept a reason parameter with a
'manual-button' default, and call reconnect('go-available') from
handleStatusChange so the 'go-available' reason is actually passed through and
honored by the bypass logic.


const previousStatus = status
Expand Down Expand Up @@ -129,14 +149,23 @@ export function Header() {
)
return false
}
}, [isRegistered, status])
}, [isRegistered, status, reconnect, waitForRegistration])

useEffect(() => {
if (isRegistered) {
autoOfflineAttemptedRef.current = false
}
}, [isRegistered])

useEffect(() => {
if (document.visibilityState !== 'visible') return
if (connectionStatus !== 'disconnected' && connectionStatus !== 'error') return

reconnect()
const retryTimer = setTimeout(() => reconnect(), 2500)
return () => clearTimeout(retryTimer)
}, [connectionStatus, reconnect])
Comment on lines +160 to +167

@coderabbitai coderabbitai Bot Apr 4, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Potential duplicate reconnect triggers with hook's visibility handler.

This effect runs when connectionStatus changes to 'disconnected'/'error' while the tab is visible. However, the hook (use-telnyx.ts lines 882-894) also has a handleVisibilityChange listener that triggers triggerManualReconnect('visibility-resume') when the tab becomes visible with those same statuses.

When the tab becomes visible after backgrounding:

  1. Hook's handleVisibilityChange fires → calls triggerManualReconnect('visibility-resume')
  2. This effect re-renders (if connectionStatus is stale as 'disconnected') → calls reconnect()

Both reconnect paths will fire nearly simultaneously. While internal guards prevent duplicate init() calls, this creates redundant code paths and confusing logs.

Additionally, the 2500ms retry at line 165 may interfere with the hook's own recovery timing (500ms runLifecycleRecovery).

Consider either:

  1. Removing this effect and relying solely on the hook's visibility handler
  2. Or removing the hook's visibility-based reconnect and handling it entirely in the header
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@src/components/layout/header.tsx` around lines 160 - 167, The useEffect in
header.tsx that watches connectionStatus and calls reconnect() duplicates the
hook's visibility-based reconnect path; remove this entire effect (the useEffect
that checks document.visibilityState, connectionStatus and schedules reconnect
with setTimeout) and rely on the hook's handleVisibilityChange /
triggerManualReconnect and its runLifecycleRecovery timing instead (symbols: the
useEffect block in header.tsx, connectionStatus, reconnect; hook symbols:
handleVisibilityChange, triggerManualReconnect, runLifecycleRecovery in
use-telnyx). Ensure no other code depends on that setTimeout retry before
removing.


useEffect(() => {
const activeCall = currentCall?.state && ['ringing_inbound', 'ringing_outbound', 'answering', 'active', 'held'].includes(currentCall.state)
if (status === 'available' && hasInitialized && !isRegistered && !activeCall) {
Expand Down
10 changes: 9 additions & 1 deletion src/hooks/use-telnyx.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -667,7 +667,8 @@ export function useTelnyx(): UseTelnyxReturn {
}

const now = Date.now()
if (now - lastManualReconnectAtRef.current < MANUAL_RECONNECT_COOLDOWN_MS) {
const bypassCooldown = reason === 'manual-button' || reason === 'visibility-resume' || reason === 'go-available'
if (!bypassCooldown && now - lastManualReconnectAtRef.current < MANUAL_RECONNECT_COOLDOWN_MS) {
console.log(`[useTelnyx] Manual reconnect skipped (${reason}) - in cooldown`)
return
}
Expand Down Expand Up @@ -881,6 +882,13 @@ export function useTelnyx(): UseTelnyxReturn {
function handleVisibilityChange() {
if (document.visibilityState === 'visible') {
console.log('[useTelnyx] Tab visible, checking registration...')

if (connectionStatusRef.current === 'disconnected' || connectionStatusRef.current === 'error') {
void triggerManualReconnect('visibility-resume')
runLifecycleRecovery('visibilitychange', 500)
return
}

runLifecycleRecovery('visibilitychange', 1500)
Comment on lines +885 to 892

@coderabbitai coderabbitai Bot Apr 4, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🧹 Nitpick | 🔵 Trivial

Consider potential double-reconnect on visibility resume.

When the tab becomes visible with disconnected/error status, both triggerManualReconnect('visibility-resume') and runLifecycleRecovery('visibilitychange', 500) are invoked. If triggerManualReconnect completes within 500ms (resetting isReconnectingRef), ensureRegistered may trigger another reconnect attempt.

This is likely benign given the existing guards (manualReconnectInFlightRef, cooldown checks), but the dual-path could be simplified:

  1. Rely solely on triggerManualReconnect for the immediate case (it handles full reconnection)
  2. Skip runLifecycleRecovery when triggerManualReconnect is invoked

If the intent is to have ensureRegistered verify post-reconnect state, the 500ms delay may be too short for the backoff delay in triggerManualReconnect (which starts at 3s).

🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@src/hooks/use-telnyx.ts` around lines 885 - 892, When visibility resumes and
connectionStatusRef.current is 'disconnected' or 'error', avoid invoking both
triggerManualReconnect('visibility-resume') and
runLifecycleRecovery('visibilitychange', 500) to prevent a possible
double-reconnect; change the branch in the visibility handler so it only calls
triggerManualReconnect('visibility-resume') and returns (remove the
runLifecycleRecovery call), leaving runLifecycleRecovery('visibilitychange',
1500) for the non-error path — this keeps triggerManualReconnect as the sole
immediate reconnection path and prevents ensureRegistered from being driven by
the short 500ms recovery timer.

}
}
Expand Down
Loading