Skip to content

fix(cc-task): fix hold/resume not working after recording pause/resume #667

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

Closed
bhabalan wants to merge 6 commits into from
Closed

fix(cc-task): fix hold/resume not working after recording pause/resume #667

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
c8c0a28
fix(cc-task): guard optional onRecordingToggle callback and fix event…
bhabalan Apr 6, 2026
ae62a5c
fix(cc-task): track hold state locally via TASK_HOLD/TASK_RESUME events
bhabalan Apr 7, 2026
28233f5
fix(cc-task): add tests for event-driven hold state and fix eslint co…
bhabalan Apr 7, 2026
87e64aa
Merge branch 'next' into CAI-7507-recording-hold-fix
bhabalan Apr 8, 2026
3bcf6af
fix(cc-task): address PR review - use controlVisibilityBase instead o…
bhabalan Apr 8, 2026
8a0cc4b
Merge branch 'next' into CAI-7507-recording-hold-fix
bhabalan Apr 8, 2026
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
48 changes: 37 additions & 11 deletions packages/contact-center/task/src/helper.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import store, {
getConferenceParticipants,
Participant,
findMediaResourceId,
findHoldStatus,
Copy link
Copy Markdown
Contributor

@rsarika rsarika Apr 8, 2026

Choose a reason for hiding this comment

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

dont we have this already in getControlsVisibility ?

Copy link
Copy Markdown
Contributor Author

@bhabalan bhabalan Apr 8, 2026

Choose a reason for hiding this comment

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

Good catch — removed the separate findHoldStatus import. The sync useEffect now reads from controlVisibilityBase.isHeld (which already computes this via getControlsVisibility), eliminating the duplication. See 3bcf6af.

MEDIA_TYPE_TELEPHONY_LOWER,
} from '@webex/cc-store';
import {getControlsVisibility} from './Utils/task-util';
Expand Down Expand Up @@ -297,6 +298,7 @@ export const useCallControl = (props: useCallControlProps) => {
agentId,
} = props;
const [isRecording, setIsRecording] = useState(true);
const [isHeld, setIsHeld] = useState(false);
const [buddyAgents, setBuddyAgents] = useState<BuddyDetails[]>([]);
const [loadingBuddyAgents, setLoadingBuddyAgents] = useState(false);
const [consultAgentName, setConsultAgentName] = useState<string>('Consult Agent');
Expand Down Expand Up @@ -445,6 +447,21 @@ export const useCallControl = (props: useCallControlProps) => {
}
}, [currentTask, logger, lastTargetType, consultAgentName, setConsultAgentName]);

// Sync local hold state from task data only when a different task is selected
// (not on every currentTask reference change, which would overwrite event-driven state)
useEffect(() => {
if (currentTask?.data?.interaction) {
try {
setIsHeld(findHoldStatus(currentTask, 'mainCall', agentId));
} catch (error) {
logger?.warn(`CC-Widgets: Task: Error syncing hold state - ${error?.message || error}`, {
module: 'useCallControl',
method: 'syncHoldState',
});
}
}
}, [currentTask?.data?.interactionId, agentId]);
Copy link
Copy Markdown

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

Choose a reason for hiding this comment

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

P2 Badge Re-run hold-state sync when task payload changes

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 👍 / 👎.

Copy link
Copy Markdown
Contributor Author

@bhabalan bhabalan Apr 8, 2026

Choose a reason for hiding this comment

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

Intentionally keeping interactionId as the dependency rather than currentTask. The root cause of this bug is that the SDK fires TASK_HOLD/TASK_RESUME events before updating its internal task data, so refreshTaskList → getAllTasks() returns stale isHold: false. Adding currentTask back to the deps would re-run the sync on every refreshTaskList and overwrite the event-driven state — reintroducing the original bug. The event callbacks (holdCallback/resumeCallback) are the source of truth during a call; this effect only seeds the initial value on task switch.

Copy link
Copy Markdown

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

Choose a reason for hiding this comment

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

P2 Badge Re-sync held state when task payload is refreshed

This effect only depends on currentTask?.data?.interactionId and agentId, so it never runs when the same interaction is refreshed with newer task data (for example, after refreshTaskList hydration), leaving local isHeld stale while it continues to override getControlsVisibility. I re-checked the current head and confirmed controlVisibilityBase is recomputed from currentTask each render, but this sync path ignores those updates because exhaustive deps are explicitly suppressed.

Useful? React with 👍 / 👎.


// Extract main call timestamp whenever currentTask changes
useEffect(() => {
extractConsultingAgent();
Expand Down Expand Up @@ -530,6 +547,7 @@ export const useCallControl = (props: useCallControlProps) => {

const holdCallback = () => {
try {
setIsHeld(true);
if (onHoldResume) {
onHoldResume({
isHeld: true,
Expand All @@ -546,6 +564,7 @@ export const useCallControl = (props: useCallControlProps) => {

const resumeCallback = () => {
try {
setIsHeld(false);
if (onHoldResume) {
onHoldResume({
isHeld: false,
Expand Down Expand Up @@ -595,10 +614,12 @@ export const useCallControl = (props: useCallControlProps) => {
const pauseRecordingCallback = () => {
try {
setIsRecording(false);
onRecordingToggle({
isRecording: false,
task: currentTask,
});
if (onRecordingToggle) {
onRecordingToggle({
isRecording: false,
task: currentTask,
});
}
} catch (error) {
logger?.error(`CC-Widgets: Task: Error in pauseRecordingCallback - ${error.message}`, {
module: 'useCallControl',
Expand All @@ -610,10 +631,12 @@ export const useCallControl = (props: useCallControlProps) => {
const resumeRecordingCallback = () => {
try {
setIsRecording(true);
onRecordingToggle({
isRecording: true,
task: currentTask,
});
if (onRecordingToggle) {
onRecordingToggle({
isRecording: true,
task: currentTask,
});
}
} catch (error) {
logger?.error(`CC-Widgets: Task: Error in resumeRecordingCallback - ${error.message}`, {
module: 'useCallControl',
Expand Down Expand Up @@ -648,8 +671,8 @@ export const useCallControl = (props: useCallControlProps) => {
store.removeTaskCallback(TASK_EVENTS.TASK_RESUME, resumeCallback, interactionId);
store.removeTaskCallback(TASK_EVENTS.TASK_END, endCallCallback, interactionId);
store.removeTaskCallback(TASK_EVENTS.AGENT_WRAPPEDUP, wrapupCallCallback, interactionId);
store.removeTaskCallback(TASK_EVENTS.CONTACT_RECORDING_PAUSED, pauseRecordingCallback, interactionId);
store.removeTaskCallback(TASK_EVENTS.CONTACT_RECORDING_RESUMED, resumeRecordingCallback, interactionId);
store.removeTaskCallback(TASK_EVENTS.TASK_RECORDING_PAUSED, pauseRecordingCallback, interactionId);
store.removeTaskCallback(TASK_EVENTS.TASK_RECORDING_RESUMED, resumeRecordingCallback, interactionId);
};
}, [currentTask]);

Expand Down Expand Up @@ -927,11 +950,14 @@ export const useCallControl = (props: useCallControlProps) => {
currentTask.cancelAutoWrapupTimer();
};

const controlVisibility = useMemo(
const controlVisibilityBase = useMemo(
() => getControlsVisibility(deviceType, featureFlags, currentTask, agentId, conferenceEnabled, logger),
[deviceType, featureFlags, currentTask, agentId, conferenceEnabled, logger]
);

// Override isHeld with event-driven local state to avoid stale task data from getAllTasks()
const controlVisibility = useMemo(() => ({...controlVisibilityBase, isHeld}), [controlVisibilityBase, isHeld]);
Copy link
Copy Markdown

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

Choose a reason for hiding this comment

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

P2 Badge Recompute dependent controls using local hold state

Overriding only controlVisibility.isHeld creates inconsistent UI state whenever local isHeld diverges from task-derived hold status (the stale-payload race this patch is handling). getControlsVisibility(...) already computed holdResume.isEnabled/end.isEnabled from the old hold value, while the button icon/action uses the overridden isHeld in cc-components (call-control.utils.ts uses controlVisibility.isHeld for icon/tooltip but controlVisibility.holdResume.isEnabled for disabled). In conference+consult-completed flows this can render a “Resume” hold button that stays disabled, blocking recovery.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown

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

Choose a reason for hiding this comment

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

P2 Badge Recompute hold-dependent controls after overriding isHeld

Overriding only controlVisibility.isHeld leaves all other hold-derived flags (holdResume.isEnabled, end.isEnabled, etc.) based on stale task data, so the UI can become internally inconsistent when local hold state diverges from task-derived state. Fresh evidence on current head: in cc-components/src/components/task/CallControl/call-control.utils.ts, the hold icon/tooltip use controlVisibility.isHeld while disabled state still uses controlVisibility.holdResume.isEnabled, which can render a “Resume” button that remains disabled in consult/conference paths.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown

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

Choose a reason for hiding this comment

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

P2 Badge Recompute hold-dependent controls after overriding isHeld

The new controlVisibility override only replaces isHeld ({...controlVisibilityBase, isHeld}), but holdResume.isEnabled, end.isEnabled, and other flags in controlVisibilityBase were already computed from the old task-derived hold value. In consult/conference states where enablement depends on isHeld, this creates internally inconsistent UI (e.g., the button renders "Resume" from isHeld but stays disabled from stale holdResume.isEnabled), which can block recovery actions after hold/resume events.

Useful? React with 👍 / 👎.


// Add useEffect for auto wrap-up timer
useEffect(() => {
let timerId: NodeJS.Timeout;
Expand Down
171 changes: 171 additions & 0 deletions packages/contact-center/task/tests/helper.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -957,6 +957,119 @@ describe('useCallControl', () => {
expect(mockOnHoldResume).toHaveBeenCalledWith({isHeld: false, task: mockCurrentTask});
});

it('should set controlVisibility.isHeld to true when holdCallback fires', async () => {
const setTaskCallbackSpy = jest.spyOn(store, 'setTaskCallback');

const {result} = renderHook(() =>
useCallControl({
currentTask: mockCurrentTask,
onHoldResume: mockOnHoldResume,
onEnd: mockOnEnd,
onWrapUp: mockOnWrapUp,
logger: mockLogger,
featureFlags: store.featureFlags,
deviceType: store.deviceType,
isMuted: false,
conferenceEnabled: true,
agentId: 'test-agent-id',
})
);

// Initially isHeld should be false
expect(result.current.controlVisibility.isHeld).toBe(false);

// Find the hold callback registered via setTaskCallback
const holdCallback = setTaskCallbackSpy.mock.calls.find((call) => call[0] === TASK_EVENTS.TASK_HOLD)?.[1];

act(() => {
holdCallback();
});

// After holdCallback fires, isHeld should be true
expect(result.current.controlVisibility.isHeld).toBe(true);
});

it('should set controlVisibility.isHeld to false when resumeCallback fires', async () => {
const setTaskCallbackSpy = jest.spyOn(store, 'setTaskCallback');

const {result} = renderHook(() =>
useCallControl({
currentTask: mockCurrentTask,
onHoldResume: mockOnHoldResume,
onEnd: mockOnEnd,
onWrapUp: mockOnWrapUp,
logger: mockLogger,
featureFlags: store.featureFlags,
deviceType: store.deviceType,
isMuted: false,
conferenceEnabled: true,
agentId: 'test-agent-id',
})
);

// First hold the call
const holdCallback = setTaskCallbackSpy.mock.calls.find((call) => call[0] === TASK_EVENTS.TASK_HOLD)?.[1];
act(() => {
holdCallback();
});
expect(result.current.controlVisibility.isHeld).toBe(true);

// Then resume — isHeld should go back to false
const resumeCallback = setTaskCallbackSpy.mock.calls.find((call) => call[0] === TASK_EVENTS.TASK_RESUME)?.[1];
act(() => {
resumeCallback();
});

expect(result.current.controlVisibility.isHeld).toBe(false);
});

it('should override getControlsVisibility isHeld with local event-driven state', async () => {
// Mock getControlsVisibility to return isHeld: true (simulating stale task data)
mockGetControlsVisibility.mockReturnValue({
...mockGetControlsVisibility.mock.results[0]?.value,
muteUnmute: {isVisible: true, isEnabled: true},
holdResume: {isVisible: true, isEnabled: true},
end: {isVisible: true, isEnabled: true},
isHeld: true,
wrapup: {isVisible: false, isEnabled: false},
});

const setTaskCallbackSpy = jest.spyOn(store, 'setTaskCallback');

const {result} = renderHook(() =>
useCallControl({
currentTask: mockCurrentTask,
onHoldResume: mockOnHoldResume,
onEnd: mockOnEnd,
onWrapUp: mockOnWrapUp,
logger: mockLogger,
featureFlags: store.featureFlags,
deviceType: store.deviceType,
isMuted: false,
conferenceEnabled: true,
agentId: 'test-agent-id',
})
);

// Even though getControlsVisibility returns isHeld: true,
// the local state starts as false, so controlVisibility.isHeld should be false
expect(result.current.controlVisibility.isHeld).toBe(false);

// After holdCallback, it should be true (event-driven, not from getControlsVisibility)
const holdCallback = setTaskCallbackSpy.mock.calls.find((call) => call[0] === TASK_EVENTS.TASK_HOLD)?.[1];
act(() => {
holdCallback();
});
expect(result.current.controlVisibility.isHeld).toBe(true);

// After resumeCallback, it should be false again
const resumeCallback = setTaskCallbackSpy.mock.calls.find((call) => call[0] === TASK_EVENTS.TASK_RESUME)?.[1];
act(() => {
resumeCallback();
});
expect(result.current.controlVisibility.isHeld).toBe(false);
});

it('should log an error if hold fails', async () => {
mockCurrentTask.hold.mockRejectedValueOnce(new Error('Hold error'));

Expand Down Expand Up @@ -5730,6 +5843,64 @@ describe('Task Hook Error Handling and Logging', () => {
);
});

it('should not throw when onRecordingToggle is not provided and pauseRecordingCallback fires', () => {
const setTaskCallbackSpy = jest.spyOn(store, 'setTaskCallback');

renderHook(() =>
useCallControl({
currentTask: mockTaskWithInteraction,
logger,
deviceType: 'BROWSER',
featureFlags: {},
isMuted: false,
conferenceEnabled: false,
agentId: 'agent1',
})
);

const pauseCallback = setTaskCallbackSpy.mock.calls.find(
(call) => call[0] === TASK_EVENTS.TASK_RECORDING_PAUSED
)?.[1];

act(() => {
pauseCallback();
});

expect(logger.error).not.toHaveBeenCalledWith(
expect.stringContaining('pauseRecordingCallback'),
expect.any(Object)
);
});

it('should not throw when onRecordingToggle is not provided and resumeRecordingCallback fires', () => {
const setTaskCallbackSpy = jest.spyOn(store, 'setTaskCallback');

renderHook(() =>
useCallControl({
currentTask: mockTaskWithInteraction,
logger,
deviceType: 'BROWSER',
featureFlags: {},
isMuted: false,
conferenceEnabled: false,
agentId: 'agent1',
})
);

const resumeCallback = setTaskCallbackSpy.mock.calls.find(
(call) => call[0] === TASK_EVENTS.TASK_RECORDING_RESUMED
)?.[1];

act(() => {
resumeCallback();
});

expect(logger.error).not.toHaveBeenCalledWith(
expect.stringContaining('resumeRecordingCallback'),
expect.any(Object)
);
});

it('should handle synchronous errors in toggleRecording', () => {
const errorTask = {
...mockTaskWithInteraction,
Expand Down
Loading