fix: presence badge shows ONLINE when agent is offline#339
Conversation
… stripe-retry, rate-limit - archiveRecordingToStorage: 21 tests covering happy path, extension detection, upload failure, signedUrl failure, call_update_failures insert path, failure insert error logging, and missing callRecord skip - health-checks.ts: 23 tests (0% -> 100%) — checkTelnyxCCA, checkTelnyxCredentialConnection, checkSupabaseReachability, EXPECTED_WEBHOOK_URL derivation, all error/timeout paths - stripe-retry.ts: 11 tests (41% -> 100%) — isStripeConnectionError, withStripeRetry happy path, connection error passthrough, 429 retry with retry-after cap, status fallback - rate-limit.ts: 12 tests added (64% -> 93%) — isRateLimited, peekRateLimit, getRateLimitRetryAfterSecs, refundRateLimit, clearRateLimit Total: 975 -> 1030 tests (+55), all passing
…x/presence-badge-status
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Caution Review failedPull request was closed or merged during review 📝 WalkthroughWalkthroughRefactored presence UI/state mapping and tightened session-restore heartbeat logic; centralized voicemail prompting into a shared helper, added unbridged-call TTL enforcement across call flows, and introduced voicemail recording watchdogs and failure dispositions. Changes
Sequence DiagramsequenceDiagram
participant Client
participant Handler as Webhook Handler
participant DB as Database
participant CallCtrl as Call Control Service
rect rgba(100, 150, 200, 0.5)
Note over Client,CallCtrl: Voicemail flow via startVoicemailFlow
end
Client->>Handler: Incoming call / hangup triggers
activate Handler
Handler->>DB: updateCallRecord(voicemail: true, disposition: VOICEMAIL_PENDING)
activate DB
DB-->>Handler: Updated
deactivate DB
Handler->>CallCtrl: speakMessage(with voicemail_mode + voicemail_started_at)
activate CallCtrl
rect rgba(150, 100, 100, 0.5)
Note over Handler: Start 30s watchdog to verify recording started
end
alt Prompt delivered
CallCtrl-->>Handler: Prompt completed
Handler->>DB: check recording state / disposition
DB-->>Handler: recording_started/ended/bridged
alt Recording OK
Handler->>CallCtrl: continue (or bridge)
else Recording never started / stuck
Handler->>CallCtrl: forceHangup
end
else Prompt failed or TTL exceeded
CallCtrl-->>Handler: error
Handler->>DB: set disposition VOICEMAIL_RECORD_START_FAILED / VOICEMAIL_PROMPT_FAILED
Handler->>CallCtrl: hangup
end
deactivate CallCtrl
deactivate Handler
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 86122613eb
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const showOfflineBanner = status === 'offline' | ||
| const showDegradedBanner = !showOfflineBanner && connectionState === 'degraded' |
There was a problem hiding this comment.
showOfflineBanner is now tied only to status === 'offline', so when heartbeats fail and connectionState transitions to offline (which usePresence does after repeated failures) while status is still available/busy/on_call, the reconnect banner is suppressed. In that state the user is actually disconnected but loses the explicit warning and recovery CTA, which can leave them stranded until they notice another signal.
Useful? React with 👍 / 👎.
| : status === 'available' && connectionState !== 'offline' | ||
| ? connectionState === 'degraded' | ||
| ? 'degraded' | ||
| : 'online' | ||
| : 'offline' |
There was a problem hiding this comment.
The new badge mapping only treats available as online/degraded, so busy and away now always render as 🔴 OFFLINE even when the connection is healthy. This misreports presence state and conflicts with the status selector (which still shows Busy/Away), making operators look disconnected when they are intentionally in a non-available state.
Useful? React with 👍 / 👎.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/components/layout/header.tsx`:
- Around line 187-188: Update the banner visibility logic in the Header
component so an offline connection always shows an offline/connection-issue
banner: set showOfflineBanner to true when connectionState === 'offline' OR
status === 'offline' (instead of only status === 'offline'), and set
showDegradedBanner to true only when connectionState === 'degraded' and
connectionState !== 'offline' (or use !showOfflineBanner && connectionState ===
'degraded') to avoid conflicting banners; adjust the banner rendering message to
distinguish connectionState 'offline' (connection issue) from status 'offline'
(manual status) and use the existing symbols showOfflineBanner,
showDegradedBanner, connectionState, and status in header.tsx.
In `@src/hooks/usePresence.ts`:
- Around line 605-618: Hoist the HEARTBEAT_RECENCY_MS constant out of the block
and define it at module scope alongside the other presence-related constants for
consistency and discoverability; update the code that currently defines
HEARTBEAT_RECENCY_MS inside usePresence.ts (the block that computes
heartbeatAgeMs and checks parsed?.status, ageMs, and heartbeatAgeMs before
calling updateStatus) to reference the newly exported/top-level
HEARTBEAT_RECENCY_MS constant instead of the inline value, keeping the same
numeric value (60_000) and leaving the restore logic and calls to updateStatus
unchanged.
In `@src/lib/webhooks/handlers.ts`:
- Around line 3008-3026: The watchdog currently aborts active voicemails because
it checks !recording_url (which is only set after upload); update the timeout
logic in the setTimeout block so it only force-hangs up when recording never
started or truly timed out: fetch and check a reliable flag such as
recording_started or recording_started_at (or recording_status) from the calls
row instead of recording_url, or compute the timeout using the voicemail max
length (max_length) plus a small grace period from the call row and only run the
watchdog after that interval; change references in this block
(createAdminClient(), admin.from('calls').select(...),
hangupCall(callControlId)) to use the new check so in-progress recordings aren’t
aborted, and ensure handleRecordingSaved() still writes the
recording_started/recording_started_at field so the watchdog can detect started
recordings.
- Around line 1488-1496: The TTL handler currently returns true even if
hangupCall(callControlId) fails, causing callers to treat the webhook as
handled; update the handler so that on hangupCall failure it re-throws the
caught error (or otherwise propagates it) instead of logging and returning true.
Locate the catch block around hangupCall(callControlId) and replace the current
pattern that logs and returns true with logic that logs the error and then throw
the error (or re-throw the original err) so the webhook caller will retry
instead of acknowledging a failed forced hangup.
- Around line 1512-1541: startVoicemailFlow currently continues even if
updateCallRecordWithRetry failed and leaves the call in VOICEMAIL_PENDING when
speakMessage fails; make the transition atomic by aborting if the pending-state
write fails: if updateCallRecordWithRetry returns falsy or throws, immediately
hang up the call via hangupCall(callControlId) and return false (do not proceed
to encodeClientState/speakMessage). Additionally, if speakMessage fails, persist
a terminal disposition (e.g., updateCallRecordWithRetry with disposition
'VOICEMAIL_FAILED' or similar) before calling hangupCall and returning false so
the DB reflects final state (this ensures handleRecordingSaved can reliably
resolve callRecord.id/tenant_id).
🪄 Autofix (Beta)
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: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: c02cbf3c-b0ce-4bad-8a3d-3ff189caf660
📒 Files selected for processing (3)
src/components/layout/header.tsxsrc/hooks/usePresence.tssrc/lib/webhooks/handlers.ts
coygg
dismissed
coderabbitai[bot]’s stale review
Findings addressed or minor - merging.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a4679a14f2
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if ( | ||
| parsed?.status && | ||
| parsed.status !== 'offline' && | ||
| parsed?.status === 'available' && |
There was a problem hiding this comment.
Restricting session restore to parsed?.status === 'available' causes agents who refresh while busy or away to be forced to offline: beforeunload/pagehide still sends the offline beacon, init() then reads currentStatus === 'offline', and this guard now blocks restoring the prior manual status. This is a regression from the previous parsed.status !== 'offline' behavior and will unexpectedly drop busy/away agents offline on every reload.
Useful? React with 👍 / 👎.
* test: full audit coverage — archiveRecordingToStorage, health-checks, stripe-retry, rate-limit - archiveRecordingToStorage: 21 tests covering happy path, extension detection, upload failure, signedUrl failure, call_update_failures insert path, failure insert error logging, and missing callRecord skip - health-checks.ts: 23 tests (0% -> 100%) — checkTelnyxCCA, checkTelnyxCredentialConnection, checkSupabaseReachability, EXPECTED_WEBHOOK_URL derivation, all error/timeout paths - stripe-retry.ts: 11 tests (41% -> 100%) — isStripeConnectionError, withStripeRetry happy path, connection error passthrough, 429 retry with retry-after cap, status fallback - rate-limit.ts: 12 tests added (64% -> 93%) — isRateLimited, peekRateLimit, getRateLimitRetryAfterSecs, refundRateLimit, clearRateLimit Total: 975 -> 1030 tests (+55), all passing * fix: address CodeRabbit test quality findings on PR #336 * fix: strengthen test assertions per CodeRabbit review on PR #336 * fix: add missing assertions in archive-recording tests * Harden agent presence heartbeat and surface offline state * fix: address CodeRabbit findings on PR #337 * fix presence stale-check error handling and codrabbit test findings * fix: final test assertion hardening per CodeRabbit * Fix ghost voicemail loops and add hangup safeguards * fix: tighten presence restore and badge offline guard * fix: address CodeRabbit presence and voicemail reliability findings
1 participant
Session restore was overriding DB offline status with stale sessionStorage. Now only restores if last heartbeat was within 60s (actual page refresh). Badge also guards against connectionState mismatch.
Summary by CodeRabbit
New Features
Bug Fixes
Improvements