fix(cc-task): fix hold/resume not working after recording pause/resume#667
fix(cc-task): fix hold/resume not working after recording pause/resume#667bhabalan wants to merge 3 commits intowebex:nextfrom
Conversation
… name mismatch in cleanup Recording pause/resume callbacks called onRecordingToggle without a null check, throwing TypeError when the optional prop wasn't provided. This crashed the SDK event emitter, preventing subsequent hold/resume events from firing. Additionally, cleanup used CONTACT_RECORDING_PAUSED/RESUMED (different string values) instead of TASK_RECORDING_PAUSED/RESUMED, so old callbacks were never removed on re-renders.
|
This pull request is automatically being deployed by Amplify Hosting (learn more). |
The SDK fires TASK_HOLD events before updating internal task data, so getAllTasks() returns stale isHold values. This caused the hold/resume UI to not update after recording pause/resume. Track isHeld locally in useCallControl (same pattern as isRecording), driven by holdCallback/resumeCallback events, synced from task data on currentTask changes, and override controlVisibility.isHeld with this event-driven state.
bhabalan
changed the title
![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: ae62a5ca97
ℹ️ 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".
| } catch { | ||
| // findHoldStatus may fail if task data is incomplete | ||
| } |
There was a problem hiding this comment.
If findHoldStatus(currentTask, 'mainCall', agentId) throws (which this block explicitly anticipates for incomplete task payloads), the catch path leaves isHeld unchanged from the previous task. Because controlVisibility is later overridden with this local isHeld, a task switch from a held call can keep the next call stuck in a held UI state (e.g., Hold button sends resume). Please set a safe fallback (setIsHeld(false)) in the catch/else path so stale state does not leak across task updates.
Useful? React with 👍 / 👎.
…ncerns - Add 3 tests verifying controlVisibility.isHeld is driven by hold/resume callbacks rather than stale task data from getAllTasks() - Fix sync useEffect dependency to use interactionId (not currentTask ref) to prevent overwriting event-driven state on refreshTaskList - Replace empty catch block with logger.warn for better observability
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 28233f59af
ℹ️ 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".
| }); | ||
| } | ||
| } | ||
| }, [currentTask?.data?.interactionId, agentId]); |
There was a problem hiding this comment.
The new hold sync effect only depends on currentTask?.data?.interactionId, so it runs once per interaction and never retries when the same interaction is refreshed with newer data. In the exact incomplete-payload case this block already anticipates (the findHoldStatus try/catch), a later refreshTaskList update for the same interaction ID will not re-sync isHeld, and the stale local value will keep overriding getControlsVisibility. Please include hold-relevant task data (or currentTask) in the dependencies so local state can recover when the task object is hydrated.
Useful? React with 👍 / 👎.
| getConferenceParticipants, | ||
| Participant, | ||
| findMediaResourceId, | ||
| findHoldStatus, |
There was a problem hiding this comment.
dont we have this already in getControlsVisibility ?
rsarika
left a comment
rsarika
left a comment
There was a problem hiding this comment.
check if can use the value in getControlsVisibility
2 participants
COMPLETES https://jira-eng-sjc12.cisco.com/jira/browse/CAI-7507
This pull request addresses
Hold followed by recording Pause or Resume stops working — the hold/resume button stays stuck as "Hold the call" even though the SDK successfully processes the hold request.
by making the following changes
Fix 1: Track hold state locally via TASK_HOLD/TASK_RESUME events
Root cause: The SDK fires TASK_HOLD events before updating its internal task data. When
refreshTaskListruns in response and callsgetAllTasks(), the returned data still hasisHold: false. SincecontrolVisibility.isHeldreads from this stale task data, the UI never reflects the held state.Fix: Track
isHeldlocally inuseCallControlusinguseState, driven byholdCallback/resumeCallbackevents (same pattern already used forisRecording). The local state is synced from task data oncurrentTaskchanges and overridescontrolVisibility.isHeld.Fix 2: Guard optional onRecordingToggle callback
onRecordingToggleis an optional prop but was called unconditionally inpauseRecordingCallbackandresumeRecordingCallback, unlike other callbacks (onHoldResume,onEnd,onWrapUp) which all have null checks.Fix 3: Fix event name mismatch in useEffect cleanup
Registration used
TASK_EVENTS.TASK_RECORDING_PAUSED('task:recordingPaused') but cleanup usedTASK_EVENTS.CONTACT_RECORDING_PAUSED('ContactRecordingPaused'). These are different string values, soremoveTaskCallbacknever matched the registered listeners.Vidcast: https://app.vidcast.io/share/355ef012-b9ce-4b84-b35a-cc0f3cc5e303
Change Type
The following scenarios were tested
The GAI Coding Policy And Copyright Annotation Best Practices
Checklist before merging