Skip to content

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

Open
bhabalan wants to merge 3 commits intowebex:nextfrom
bhabalan:CAI-7507-recording-hold-fix
Open

fix(cc-task): fix hold/resume not working after recording pause/resume#667
bhabalan wants to merge 3 commits intowebex:nextfrom
bhabalan:CAI-7507-recording-hold-fix

Conversation

@bhabalan
Copy link
Copy Markdown
Contributor

@bhabalan bhabalan commented Apr 6, 2026

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 refreshTaskList runs in response and calls getAllTasks(), the returned data still has isHold: false. Since controlVisibility.isHeld reads from this stale task data, the UI never reflects the held state.

Fix: Track isHeld locally in useCallControl using useState, driven by holdCallback/resumeCallback events (same pattern already used for isRecording). The local state is synced from task data on currentTask changes and overrides controlVisibility.isHeld.

Fix 2: Guard optional onRecordingToggle callback

onRecordingToggle is an optional prop but was called unconditionally in pauseRecordingCallback and resumeRecordingCallback, 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 used TASK_EVENTS.CONTACT_RECORDING_PAUSED ('ContactRecordingPaused'). These are different string values, so removeTaskCallback never matched the registered listeners.

Vidcast: https://app.vidcast.io/share/355ef012-b9ce-4b84-b35a-cc0f3cc5e303

Change Type

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Documentation update
  • Tooling change
  • Internal code refactor

The following scenarios were tested

  • Verified hold/resume UI correctly toggles after recording pause using Playwright (React sample app, live call)
  • Verified recording pause/resume no longer throws TypeError when onRecordingToggle is not provided
  • Verified callback cleanup correctly removes recording event listeners on re-render
  • Unit tests pass (168/168 in helper.ts test suite)

The GAI Coding Policy And Copyright Annotation Best Practices

  • GAI was not used (or, no additional notation is required)
  • Code was generated entirely by GAI
  • GAI was used to create a draft that was subsequently customized or modified
  • Coder created a draft manually that was non-substantively modified by GAI (e.g., refactoring was performed by GAI on manually written code)
  • Tool used for AI assistance (GitHub Copilot / Other - specify)
    • Github Copilot
    • Other - Claude Code
  • This PR is related to
    • Feature
    • Defect fix
    • Tech Debt
    • Automation

Checklist before merging

  • I have not skipped any automated checks
  • All existing and new tests passed
  • I have updated the testing document
  • I have tested the functionality with amplify link

… 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.
@bhabalan bhabalan marked this pull request as ready for review April 6, 2026 06:15
@bhabalan bhabalan requested a review from a team as a code owner April 6, 2026 06:15
@bhabalan bhabalan added the validated Indicates that the PR is ready for actions label Apr 6, 2026
@aws-amplify-us-east-2
Copy link
Copy Markdown

This pull request is automatically being deployed by Amplify Hosting (learn more).

Access this pull request here: https://pr-667.d1b38q61t1z947.amplifyapp.com

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 bhabalan changed the title fix(cc-task): guard onRecordingToggle callback and fix event cleanup mismatch fix(cc-task): fix hold/resume not working after recording pause/resume Apr 7, 2026
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

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

💡 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".

Comment on lines +455 to +457
} catch {
// findHoldStatus may fail if task data is incomplete
}
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 Reset hold state when hold-status sync fails

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
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

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

💡 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]);
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 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 👍 / 👎.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

validated Indicates that the PR is ready for actions

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant