docs(ai-docs): task refactor migration overview (PR 1/4)#644
docs(ai-docs): task refactor migration overview (PR 1/4)#644akulakum wants to merge 18 commits intowebex:nextfrom
Conversation
![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: 439effd4b9
ℹ️ 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".
| | `TASK_MEDIA` | `handleTaskMedia` | **Keep** (browser WebRTC setup) | | ||
| | `TASK_UI_CONTROLS_UPDATED` | **NEW** — `handleUIControlsUpdated` | **Add** — trigger widget re-renders | | ||
| | `TASK_WRAPUP` | `handleWrapup` | **Simplify** — no need to refresh | | ||
| | `TASK_WRAPPEDUP` | `handleWrappedup` | **Simplify** — no need to refresh | |
There was a problem hiding this comment.
This mapping introduces TASK_WRAPPEDUP, but the current task event enum exposes AGENT_WRAPPEDUP (and this same doc’s code examples already use AGENT_WRAPPEDUP). If implementers follow the table literally, they will subscribe to a non-existent event and wrap-up callbacks will not fire, which breaks post-call UI updates tied to wrap-up completion; the same incorrect name is also repeated in 009-types-and-constants-migration.md.
Useful? React with 👍 / 👎.
|
|
||
| ### UIControlConfig Is Built by SDK (Not by Widgets) | ||
|
|
||
| Widgets do NOT need to provide `UIControlConfig`. The SDK builds it internally from agent profile, `callProcessingDetails`, media type, and voice variant. This means `deviceType`, `featureFlags`, `agentId`, and `conferenceEnabled` **can be removed** from `useCallControlProps` — they are only used for `getControlsVisibility()` which is being eliminated. |
There was a problem hiding this comment.
This line states agentId can be removed because it is only used for getControlsVisibility, but useCallControl also relies on agentId for participant/timer derivation and this migration set still documents timer helpers with an agentId parameter (Doc 004). If agentId is removed without explicitly documenting the replacement source, call/consult timer behavior will regress during migration.
Useful? React with 👍 / 👎.
akulakum
added
the
run_e2e
|
This pull request is automatically being deployed by Amplify Hosting (learn more). |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4a534dbf4d
ℹ️ 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".
| | `pauseResumeRecording` | `recording` | **Rename** | | ||
| | `recordingIndicator` | `recording` | **Merge** with recording control | | ||
| | `mergeConference` | `mergeToConference` | **Rename** | | ||
| | `consultTransferConsult` | — | **Remove** (use `transfer`) | |
There was a problem hiding this comment.
This mapping removes consultTransferConsult in favor of controls.transfer, but conference transfer is modeled as a separate control (controls.transferConference) in this same migration set (see the later CallControl example). In conferencing states transfer is not shown, so implementing this table literally would hide the conference-transfer affordance and block the task.transferConference() path for active conference calls.
Useful? React with 👍 / 👎.
| | `muteUnmute` | `controls.mute` | **Renamed** + nested | | ||
| | `holdResume` | `controls.hold` | **Renamed** + nested | | ||
| | `pauseResumeRecording` | `controls.recording` | **Renamed** + nested | | ||
| | `recordingIndicator` | `controls.recording` | **Merged** with recording | |
There was a problem hiding this comment.
The table merges recordingIndicator into controls.recording, but these are not equivalent behaviors in the current widgets: the indicator is used as a persistent recording status badge, while the pause/resume recording button has stricter visibility rules. If migration follows this merge, consult/conference scenarios can lose the recording status indicator when the toggle control is hidden, causing a visible regression in call-state feedback.
Useful? React with 👍 / 👎.
rsarika
removed
the
run_e2e
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 45a1bd66e3
ℹ️ 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".
| controls: TaskUIControls, // NEW type from SDK | ||
| ...handlers | ||
| ): CallControlButton[] => { | ||
| const isHeld = controls.hold.isVisible && !controls.hold.isEnabled; |
There was a problem hiding this comment.
The proposed isHeld computation (controls.hold.isVisible && !controls.hold.isEnabled) is not a reliable held-state signal: hold can be disabled in consult/transition states even when the main call is not held, and held flows still need resume semantics. Implementing this derivation in button-building logic will misclassify state and can drive incorrect hold/resume UI behavior (icon/tooltip/toggle direction) in core call-control flows.
Useful? React with 👍 / 👎.
| ...useCallControl({ | ||
| currentTask, onHoldResume, onEnd, onWrapUp, onRecordingToggle, onToggleMute, | ||
| logger, isMuted | ||
| // REMOVED: deviceType, featureFlags, conferenceEnabled, agentId |
There was a problem hiding this comment.
This After example removes agentId from the useCallControl call, but the current timer path still depends on agent identity for participant lookup (calculateStateTimerData/calculateConsultTimerData in task/src/Utils/timer-utils.ts). If migration follows this snippet without specifying a replacement source, consult/hold/wrap-up timer behavior will regress due to missing participant context. Fresh evidence: this file’s CallControlCAD migration section still omits agentId even though other migration docs now retain it.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 28397c24b4
ℹ️ 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".
| | `TASK_MEDIA` | `handleTaskMedia` | **Keep** (browser WebRTC setup) | | ||
| | `TASK_UI_CONTROLS_UPDATED` | **NEW** — `handleUIControlsUpdated` | **Add** — trigger widget re-renders | | ||
| | `TASK_WRAPUP` | `handleWrapup` | **Simplify** — no need to refresh | | ||
| | `AGENT_WRAPPEDUP` | `handleWrappedup` | **Simplify** — no need to refresh | |
There was a problem hiding this comment.
This row drops refreshTaskList() for AGENT_WRAPPEDUP, but the migration set’s own wrap-up flow shows task:end happening at wrap-up entry and final cleanup happening later; without a refresh (or an explicit replacement like a cleanup/removal handler), wrapped tasks can remain in store.taskList/currentTask after completion and keep stale call UI visible.
Useful? React with 👍 / 👎.
| 4. SDK: computeUIControls → recording: { isVisible: true, isEnabled: false } | ||
| (visible but disabled = recording paused) |
There was a problem hiding this comment.
Documenting paused recording as recording: { isVisible: true, isEnabled: false } conflicts with the migration’s button wiring that uses isEnabled for interactivity; if implementers follow this literally, the pause/resume button becomes disabled right after pause and agents lose the path to manually resume recording.
Useful? React with 👍 / 👎.
| |----------|-------------------|-----------------| | ||
| | Widget hooks (`task/src/`) | 4 | 1 (`index.ts`) | | ||
| | Widget utils (`task/src/Utils/`) | 4 | 0 | | ||
| | Widget entry points (`task/src/*/index.tsx`) | 3 | 2 (`OutdialCall`, `IncomingTask`) | |
There was a problem hiding this comment.
This summary marks IncomingTask entrypoint as unaffected, but the migration docs also require useIncomingTask to switch from deviceType/isBrowser gating to task.uiControls, which requires updating that entry file’s props wiring. Leaving it in the unaffected list can cause teams to skip a required migration step and keep old visibility plumbing in place.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3bd613f97f
ℹ️ 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".
| | `TASK_CONSULT_*` | Fire callback only | **Simplify** — SDK manages state | | ||
| | `TASK_CONFERENCE_*` | Fire callback only | **Simplify** — SDK manages state | |
There was a problem hiding this comment.
This mapping reduces all TASK_CONSULT_*/TASK_CONFERENCE_* listeners to callback-only behavior, but current store handlers do more than refreshTaskList(): handleConsultEnd, handleConsultQueueCancelled, and handleConferenceStarted reset isQueueConsultInProgress, currentConsultQueueId, and consultStartTimeStamp (store/src/storeEventsWrapper.ts). If migration follows this table literally, those flags can stay stale and useCallControl.endConsultCall may send an incorrect queueId payload (task/src/helper.ts), breaking queue-consult cleanup flows.
Useful? React with 👍 / 👎.
| // NOTE: Do NOT derive held state from controls.hold.isEnabled — hold can be | ||
| // disabled in consult/transition states even when call is not held. | ||
| // Use task data instead: | ||
| if (findHoldStatus(task, 'mainCall', agentId)) return 'Held'; |
There was a problem hiding this comment.
The After example uses findHoldStatus(task, 'mainCall', agentId) even though getTaskStatus was changed to accept only task, so implementers cannot reproduce held-state detection from this snippet. In current code, task status is agent-specific (getTaskStatus(task, agentId) in store/src/task-utils.ts), so dropping agentId here leads to either a compile error or loss of correct Held status during migration.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 606dcbfd21
ℹ️ 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".
| 1. Update `calculateStateTimerData(task, controls, agentId)` signature | ||
| 2. Replace `controlVisibility.isConsultInitiatedOrAccepted` → `controls.endConsult.isVisible` | ||
| 3. Replace `controlVisibility.isHeld` → derive from `controls.hold` | ||
| 4. Update `calculateConsultTimerData(task, controls, agentId)` similarly |
There was a problem hiding this comment.
This step instructs replacing controlVisibility.isHeld with controls.hold, but in current logic the hold control can be disabled for consult/conference transitions even when the main call is not actually held (getHoldResumeButtonVisibility disables hold when isConsultInProgress in task/src/Utils/task-util.ts). If this guidance is followed, migrated code will misclassify hold state and render incorrect hold/resume behavior (icon, tooltip, and related timer/status behavior) during consult flows.
Useful? React with 👍 / 👎.
| | # | File | Old References | Migration Doc | | ||
| |---|------|---------------|--------------| | ||
| | 8 | `cc-components/.../CallControl/call-control.tsx` | Receives `controlVisibility` as prop, passes to `buildCallControlButtons()`, `createConsultButtons()`, `filterButtonsForConsultation()` | [Doc 010](./010-component-layer-migration.md) | | ||
| | 9 | `cc-components/.../CallControlCustom/call-control-consult.tsx` | Receives `controlVisibility` from `CallControlConsultComponentsProps`, passes to `createConsultButtons()` | [Doc 010](./010-component-layer-migration.md) | | ||
| | 10 | `cc-components/.../CallControlCustom/consult-transfer-popover.tsx` | Receives `isConferenceInProgress` prop | [Doc 010](./010-component-layer-migration.md) | |
There was a problem hiding this comment.
This component inventory omits cc-components/src/components/task/CallControlCAD/call-control-cad.tsx and cc-components/src/wc.ts, both of which still depend on old call-control props (controlVisibility, recordingIndicator, isHeld). Because this document is used as the migration checklist, teams can miss these files and end up with type/runtime mismatches after moving hooks/components to TaskUIControls.
Useful? React with 👍 / 👎.
- Fix isHeld derivation in execution plan to use findHoldStatus(task) instead of controls.hold - Add CallControlCAD/call-control-cad.tsx and wc.ts to file inventory and migration checklist - Update cross-reference matrix with call-control-cad.tsx references Made-with: Cursor
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 23c1fab1bf
ℹ️ 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".
| | `switchToConsult` | `task.hold(mainMediaId)` + `task.resume(consultMediaId)` | None | | ||
| | `switchToMainCall` | `task.hold(consultMediaId)` + `task.resume(mainMediaId)` | None | |
There was a problem hiding this comment.
This table documents switchToConsult/switchToMainCall as two-call sequences (hold(...) + resume(...)), but the current hook implementations use a single SDK call per action (switchToConsult calls hold(mainCall) and switchToMainCall calls resume(consult)). If migration work follows this mapping literally, it will introduce extra state transitions and can break consult leg switching behavior during active consult flows.
Useful? React with 👍 / 👎.
| 14. SDK: State: WRAPPING_UP → COMPLETED | ||
| 15. SDK: computeUIControls(COMPLETED) → all controls disabled | ||
| 16. SDK: emitTaskWrappedup action → cleanupResources action | ||
| 17. SDK emits: task:wrappedup, task:ui-controls-updated, task:cleanup |
There was a problem hiding this comment.
The flow still says wrap-up completion emits task:wrappedup, but widget callbacks are wired to AGENT_WRAPPEDUP; implementing this event name as written would prevent wrap-up completion callbacks from firing and leave post-wrap-up UI/state transitions stale. Fresh evidence: this same sequence expects AGENT_WRAPPEDUP in step 19 immediately after listing task:wrappedup in step 17.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b35a8c3e25
ℹ️ 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".
| ### After (all above replaced by `task.uiControls`) | ||
| ```typescript | ||
| // task-util.ts — DELETED or reduced to: | ||
| export { findHoldTimestamp } from './task-util'; // Only keep for timer display |
There was a problem hiding this comment.
The "After" snippet instructs implementers to keep task-util.ts by writing export { findHoldTimestamp } from './task-util';, which re-exports from the same module and creates a circular self-import instead of preserving an implementation. Following this literally will either fail TypeScript compilation or effectively drop the timer helper export that useHoldTimer relies on, blocking migration work.
Useful? React with 👍 / 👎.
| | `getConferenceParticipantsCount(task)` | Used only for control visibility computation | SDK computes max participant check internally | | ||
| | `getIsCustomerInCall(task)` | Used only for control visibility computation | SDK computes internally | | ||
| | `getIsConsultInProgress(task)` | Used only for control visibility computation | SDK computes internally | | ||
| | `findHoldStatus(task, mType, agentId)` | Used for control visibility | SDK tracks hold state in context | |
There was a problem hiding this comment.
This row marks findHoldStatus as removable, but the migration docs still require it for held-state/status derivation (the same document’s getTaskStatus example and Doc 010’s hold-button mapping both depend on findHoldStatus(...)). If teams remove it per this table, they will either hit compile errors or lose correct Held detection in migrated call-control/task-list flows.
Useful? React with 👍 / 👎.
| | `cc-components/.../TaskList/task-list.utils.ts` | Task list data formatting | | ||
| | `cc-components/.../OutdialCall/outdial-call.tsx` | No task controls | | ||
| | `cc-components/.../IncomingTask/incoming-task.utils.tsx` | Incoming task display utils | |
There was a problem hiding this comment.
This inventory classifies TaskList/task-list.utils.ts and IncomingTask/incoming-task.utils.tsx as unaffected, but both files still derive accept/decline behavior from legacy inputs like isBrowser and isDeclineButtonEnabled instead of task.uiControls. Using this checklist as-is can cause teams to skip those files and leave old visibility/disablement logic in place after migration.
Useful? React with 👍 / 👎.
- Fix self-reexport in 008: replace circular export with clear retention note for findHoldTimestamp and findHoldStatus - Move findHoldStatus from REMOVE to KEEP in 008: still needed for getTaskStatus() and component layer isHeld - Move task-list.utils.ts and incoming-task.utils.tsx from unaffected to affected in 013: both use isBrowser/isDeclineButtonEnabled - Update cross-reference matrix with isBrowser and isDeclineButtonEnabled entries Made-with: Cursor
Remove files moved to separate PRs: - PR 2 (Store Layer): 003, 008 - PR 3 (Widget Layer): 004, 005, 006, 007, 010 - PR 4 (Planning & Reference): 011, 012, 013 This PR now contains only the foundation documents: - 001: Migration overview (master index) - 002: UI controls migration (core old→new mapping) - 009: Types and constants migration - 014: Task code scan report Made-with: Cursor
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0024a37302
ℹ️ 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".
|
|
||
| | Old State Flag | Replacement | | ||
| |----------------|-------------| | ||
| | `isConferenceInProgress` | Derive from `task.uiControls.exitConference.isVisible` if needed | |
There was a problem hiding this comment.
isConferenceInProgress cannot be reliably reconstructed from task.uiControls.exitConference.isVisible. In current widget logic, getExitConferenceButtonVisibility only shows that control when isConferenceInProgress && !isConsultInitiatedOrAccepted, so conference sessions with an active consult are still in conference state while the exit-conference control is hidden; following this replacement would misclassify those flows and break conference-state-dependent behavior.
Useful? React with 👍 / 👎.
|
|
||
| | Old (CC Widgets Store) | New (CC SDK) | | ||
| |------------------------|--------------| | ||
| | `TASK_STATE_CONSULT` | `TaskState.CONSULTING` | |
There was a problem hiding this comment.
This mapping collapses TASK_STATE_CONSULT into TaskState.CONSULTING, but store/src/task-utils.ts#getConsultMPCState returns TASK_STATE_CONSULT specifically for CONSULT_STATE_INITIATED. Migrating with this table will blur initiated-vs-connected consult phases and can move state-based UI/timer logic onto the wrong SDK state branch.
Useful? React with 👍 / 👎.
|
|
||
| ### useIncomingTask (lines 147–281) | ||
| - **setTaskCallback usage:** TASK_ASSIGNED, TASK_CONSULT_ACCEPTED, TASK_END, TASK_REJECT, TASK_CONSULT_END | ||
| - **removeTaskCallback:** Same events in cleanup |
There was a problem hiding this comment.
The report says cleanup removes the same events, but useIncomingTask registers TASK_ASSIGNED with an inline callback and removes it with taskAssignCallback, which is a different function reference in task/src/helper.ts. Marking cleanup as symmetric here hides a real listener leak/duplicate-callback risk, so migration work guided by this scan can miss an actionable bug.
Useful? React with 👍 / 👎.
akulakum
changed the title
…ivation, consult state mapping, and callback mismatch
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: abbbaf35b1
ℹ️ 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".
| | Old State Flag | Replacement | | ||
| |----------------|-------------| | ||
| | `isConferenceInProgress` | **Do NOT derive from `exitConference.isVisible`** — exit-conference is hidden during conference + active consult (`isConferenceInProgress && !isConsultInitiatedOrAccepted`). Use `task.data.isConferenceInProgress` from task data instead, or check if SDK exposes this as a dedicated flag | | ||
| | `isConsultInitiated` | Derive from `task.uiControls.endConsult.isVisible` if needed | |
There was a problem hiding this comment.
This row maps isConsultInitiated to task.uiControls.endConsult.isVisible, but that control does not represent an initiated-only phase: current logic shows end-consult for broader consult states (getEndConsultButtonVisibility(..., isConsultInitiatedOrAccepted) in packages/contact-center/task/src/Utils/task-util.ts), while initiated-only semantics are still used for behavior like consult timer labeling (calculateConsultTimerData in packages/contact-center/task/src/Utils/timer-utils.ts). Following this mapping will collapse initiated vs accepted consult phases and produce incorrect consult-state UI behavior.
Useful? React with 👍 / 👎.
| ### 5. Store Task Flow | ||
| - refreshTaskList → cc.taskManager.getAllTasks() | ||
| - setCurrentTask uses isIncomingTask(task, agentId) to skip incoming | ||
| - handleTaskRemove unregisters all task listeners |
There was a problem hiding this comment.
The summary says handleTaskRemove unregisters all task listeners, but registerTaskEventListeners adds TASK_REJECT and TASK_OUTDIAL_FAILED with inline lambdas while handleTaskRemove calls off with different inline lambdas in packages/contact-center/store/src/storeEventsWrapper.ts, so those handlers are not actually removed. Keeping this statement as-is can cause migration work to miss a real duplicate-callback/listener-leak path.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: eabc80db49
ℹ️ 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".
|
|
||
| 2. **`TaskState.CONSULT_INITIATING` vs `CONSULTING`:** The SDK has `CONSULT_INITIATING` (consult requested, async in-progress) and `CONSULTING` (consult accepted, actively consulting) as distinct states. The old widget constant `TASK_STATE_CONSULT` ('consult') maps to `CONSULT_INITIATING`, NOT `CONSULTING`. `TaskState.CONSULT_INITIATED` exists in the enum but is marked "NOT IMPLEMENTED". | ||
|
|
||
| 3. **Recording control:** `recording.isEnabled = true` when recording is in progress (allows pause/resume toggle). `recording.isEnabled = false` when recording is not active (allows starting). This means paused recordings show `{ isVisible: true, isEnabled: true }` to allow resumption. |
There was a problem hiding this comment.
This guidance says recording.isEnabled = false when recording is not active while also saying that state "allows starting", which conflicts with widget button semantics where isEnabled drives interactivity (disabled: !...isEnabled in cc-components/src/components/task/CallControl/call-control.utils.ts). If implementers follow this literally, the recording toggle becomes non-clickable in non-recording states and agents lose the manual start/resume path; fresh evidence is that this migration set also maps toggle interactivity to recording.isEnabled in Doc 002.
Useful? React with 👍 / 👎.
| | `isConsultInitiatedAndAccepted` | No longer needed — SDK handles via controls | | ||
| | `isConsultReceived` | No longer needed — SDK handles via controls | | ||
| | `isConsultInitiatedOrAccepted` | No longer needed — SDK handles via controls | | ||
| | `isHeld` | Derive from `task.uiControls.hold` state or SDK task state | |
There was a problem hiding this comment.
The table recommends deriving isHeld from task.uiControls.hold, but hold-control enablement is not a reliable held-state signal in consult/conference flows. In current logic, getHoldResumeButtonVisibility disables hold when consult is in progress even when the main call is not held (task/src/Utils/task-util.ts, lines 164–177), so migrating with this mapping will misclassify hold state and drive incorrect hold/resume icon/tooltip behavior.
Useful? React with 👍 / 👎.
| ```typescript | ||
| // REMOVE these (no longer needed for control computation): | ||
| // TASK_STATE_CONSULT, TASK_STATE_CONSULTING, TASK_STATE_CONSULT_COMPLETED | ||
| // INTERACTION_STATE_WRAPUP, POST_CALL, CONNECTED, CONFERENCE |
There was a problem hiding this comment.
This removal checklist names POST_CALL, CONNECTED, and CONFERENCE, but store/src/constants.ts exports INTERACTION_STATE_POST_CALL, INTERACTION_STATE_CONNECTED, and INTERACTION_STATE_CONFERENCE instead (lines 8–10). Teams following this list literally will not touch the actual constants that still feed task-status derivation, which can leave stale old-state logic in place during migration.
Useful? React with 👍 / 👎.
…rding semantics, isHeld, constants, and listener leak
Note to reviewers on Codex review commentsCodex does not have access to the CC SDK All Codex comments have been cross-verified against the SDK source code ( See the "CC SDK Task-Refactor Branch Reference" section in this PR description for key SDK architectural decisions that inform the migration guidance. Key SDK behaviors that differ from old widget logic (verified in
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6e50dc5fc8
ℹ️ 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".
| | Old State Flag | Replacement | | ||
| |----------------|-------------| | ||
| | `isConferenceInProgress` | **Caution with `exitConference.isVisible`** — In the old widget code, exit-conference was hidden during conference + active consult. In the **new SDK** (`uiControlsComputer.ts`), `exitConference` is `VISIBLE_DISABLED` (not hidden) during consulting-from-conference, making `isVisible` more reliable. However, for consulted agents not in conferencing state, `exitConference` is `DISABLED`. If you need a definitive conference-in-progress flag independent of agent role, use `task.data` (e.g., `getIsConferenceInProgress(taskData)` which the SDK itself uses internally) rather than relying solely on `exitConference.isVisible` | | ||
| | `isConsultInitiated` | **Do NOT derive from `endConsult.isVisible`** — SDK shows `endConsult` for all consulting states (`CONSULT_INITIATING`, `CONSULTING`, `CONF_INITIATING`), not just initiated. If initiated-only semantics are needed (e.g., consult timer labeling in `calculateConsultTimerData`), use SDK `TaskState` directly: `state === TaskState.CONSULT_INITIATING` | |
There was a problem hiding this comment.
This migration step tells implementers to derive initiated consult state with TaskState.CONSULT_INITIATING, but the same commit states in 009-types-and-constants-migration.md that TaskState is internal and not exported to widget consumers. In environments following the documented public SDK surface, this instruction is not implementable and can stall/derail migration of consult timer labeling logic unless the doc provides an exported alternative source of initiated state.
Useful? React with 👍 / 👎.
| - TASK_PARTICIPANT_LEFT → handleConferenceEnded | ||
| - TASK_PARTICIPANT_LEFT_FAILED → refreshTaskList | ||
| - TASK_CONFERENCE_STARTED → handleConferenceStarted | ||
| - TASK_CONFERENCE_TRANSFERRED → handleConferenceEnded |
There was a problem hiding this comment.
The scan report says TASK_CONFERENCE_TRANSFERRED is wired to handleConferenceEnded, but in storeEventsWrapper.ts#registerTaskEventListeners it is actually registered with refreshTaskList while cleanup uses off(..., handleConferenceEnded). Documenting the wrong registration target masks a real callback-reference mismatch, so migration work guided by this report is likely to miss a listener-leak/duplicate-callback path for conference-transfer flows.
Useful? React with 👍 / 👎.
… conference handler mismatch
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0d08a09773
ℹ️ 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".
| | `cc-components/src/components/task/task.types.ts` | Align `ControlProps` with new control names | | ||
| | `cc-components/src/components/task/CallControl/call-control.tsx` | Update prop names (`holdResume` → `hold`, etc.) | | ||
| | `cc-components/src/components/task/CallControl/call-control.utils.ts` | Simplify/remove old control derivation | | ||
| | `store/src/task-utils.ts` | Remove `getConsultStatus`, `findHoldStatus` if no longer consumed | |
There was a problem hiding this comment.
This document tells implementers to derive isHeld from findHoldStatus(task, 'mainCall', agentId) earlier in the same mapping, but this checklist item then advises removing findHoldStatus from store/src/task-utils.ts when pruning old helpers. If teams follow the removal guidance before wiring the new held-state path, hold/resume state derivation loses its documented source and migrated call-control UI can show incorrect hold status.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d57667611d
ℹ️ 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".
| # Migration Doc 014: Task-Related Code Scan Report | ||
|
|
||
| **Generated:** 2025-03-09 | ||
| **Scope:** Complete scan of CC Widgets repository for task-related code |
There was a problem hiding this comment.
This report claims a complete repository-wide task scan, but it omits task-related files that still carry the legacy control contract (for example, packages/contact-center/cc-components/src/components/task/CallControlCAD/call-control-cad.tsx uses controlVisibility.isConferenceInProgress, and the WC sample wiring in widgets-samples/cc/samples-cc-wc-app/app.js is not inventoried). If migration planning relies on this “complete” scan, teams can skip required CAD/Web Component updates and ship mismatched props after moving to task.uiControls.
Useful? React with 👍 / 👎.
| SDK computes task.uiControls automatically → | ||
| SDK emits task:ui-controls-updated → | ||
| Widgets consume task.uiControls directly → | ||
| Components receive {isVisible, isEnabled} per control |
There was a problem hiding this comment.
We might need to update this statement and point towards the new object structure that is being maintained in SDK. Saying that we receive {isVisible, isEnabled} per control is vague considering the change that has been done in SDK
|
|
||
| ## Repo Paths Reference | ||
|
|
||
| ### CC Widgets (this repo) |
There was a problem hiding this comment.
This information might already be present in existing specs that we have. Any particular reason we are adding this here. Also is adding SDK path here,
| @@ -0,0 +1,202 @@ | |||
| # Migration Doc 001: Task Refactor Migration Overview | |||
There was a problem hiding this comment.
We might need to update the root agents.md folder to refer to these specs. Also location of this folder should be inside contact-center widgets folder as this migration is specific to contact center
|
|
||
| | # | Document | Scope | Risk | Priority | | ||
| |---|----------|-------|------|----------| | ||
| | 002 | [002-ui-controls-migration.md](./002-ui-controls-migration.md) | Replace `getControlsVisibility()` with `task.uiControls` | **High** (core UX) | P0 | |
There was a problem hiding this comment.
Why do we need to have Priority column here ? How does it help in migration changes ?
| | # | Document | Scope | Risk | Priority | | ||
| |---|----------|-------|------|----------| | ||
| | 002 | [002-ui-controls-migration.md](./002-ui-controls-migration.md) | Replace `getControlsVisibility()` with `task.uiControls` | **High** (core UX) | P0 | | ||
| | 003 | [003-store-event-wiring-migration.md](./003-store-event-wiring-migration.md) | Refactor store event handlers to leverage state machine events | **Medium** | P1 | |
There was a problem hiding this comment.
I cannot see this file anymore but in this file, change is more about how we don't need to do refreshTaskList anymore so description here should be updated to say somehting like update store event handlers to accommodate task update being taken care in SDK.
Because events being recieved are not state machine events, they are same events but being emitted via state machine
| | 004 | [004-call-control-hook-migration.md](./004-call-control-hook-migration.md) | Refactor `useCallControl` hook, timer utils, fix bugs | **High** (largest widget) | P0 | | ||
| | 005 | [005-incoming-task-migration.md](./005-incoming-task-migration.md) | Refactor `useIncomingTask` for state-machine offer/assign flow | **Low** | P2 | | ||
| | 006 | [006-task-list-migration.md](./006-task-list-migration.md) | Refactor `useTaskList` for per-task `uiControls` | **Low** | P2 | | ||
| | 007 | [007-outdial-call-migration.md](./007-outdial-call-migration.md) | No changes needed (CC-level, not task-level) | **Low** | P3 | |
There was a problem hiding this comment.
If no changes needed then why do we have .md file for this ?
|
|
||
| --- | ||
|
|
||
| ## Key Architectural Shift |
There was a problem hiding this comment.
This is just a suggestion. Migration docs are generally supposed to talking about straightforward changes. Also this documentation is going to be one time use. So adding details here like problems, benefits etc might not be really required. And adding a refernce digram of old arhcitecture and new arhcitecture from widgets perspective along with descriptions of specificallyu those archietctural changes might be better I feel.
For example, Old appraoch section, new approach section talking about the changes are more than enough
|
|
||
| ## SDK Version Requirements | ||
|
|
||
| The CC Widgets migration depends on the CC SDK `task-refactor` branch being merged and released. Key new APIs: |
There was a problem hiding this comment.
Why is this information relevant for doing the migration ?
|
|
||
| --- | ||
|
|
||
| ## Pre-existing Bugs Found During Analysis |
There was a problem hiding this comment.
This should be part of this doc
|
|
||
| --- | ||
|
|
||
| ## Critical Migration Notes |
There was a problem hiding this comment.
Any particular reason to highlights these 3 points here ?
…move migration docs to contact-center scope
Addressing @Kesari3008's Review CommentsAll feedback has been implemented. Here's the summary: 1. "Saying that we receive {isVisible, isEnabled} per control is vague" Updated. Replaced the vague 2. "This information might already be present in existing specs... Also is adding SDK path here" Agreed. Removed the SDK paths table (those are already covered in the "CC SDK Task-Refactor Branch Reference" section). Renamed the section to "CC Widgets Files Affected" and kept only the widget-side file paths that are directly impacted by this migration. 3. "Update the root agents.md to refer to these specs. Also location of this folder should be inside contact-center widgets folder" Done both:
4. "Why do we need to have Priority column here?" Removed the Priority and Risk columns from the index table. The table now only has 5. "change is more about how we don't need to do refreshTaskList anymore... events are same events but being emitted via state machine" Updated the 003 scope description to: "Update store event handlers — same events, now emitted via state machine; task state updates handled by SDK (e.g., remove 6. "If no changes needed then why do we have .md file for this?" Removed the 007 row from the index table. The actual 7. "Migration docs are generally supposed to talk about straightforward changes... Old approach section, new approach section are more than enough" Agreed. Removed the "Problems" and "Benefits" bullet lists. The section is now just "Old Approach" and "New Approach" with data flow diagrams and concise descriptions, plus the concrete 8. "Why is this information relevant for doing the migration?" (SDK Version Requirements) Removed the "SDK Version Requirements" section. The key API info ( 9. "This should be part of this doc" (Pre-existing Bugs) Acknowledged — keeping the Pre-existing Bugs section as-is. Removed the 10. "Any particular reason to highlight these 3 points here?" (Critical Migration Notes) Simplified and renamed the section from "Critical Migration Notes" to "Migration Notes". Removed the verbose sample app code block. Kept 3 concise bullet points that are implementation-specific gotchas implementers need to be aware of (UIControlConfig source, timer utils signature change, wrapup timing). 11. "Why is fixing bugs and fixing findHoldTimestamp part of this migration doc?" Agreed — updated doc 004 and 008 scope descriptions to focus only on task-refactor changes. Removed |
Replaced 4 files (001-migration-overview, 002-ui-controls-migration, 009-types-and-constants-migration, 014-task-code-scan-report) with a single migration-overview.md. Reordered sections for spec-driven execution flow. Moved detailed constants, event mappings, and gotchas to PR 2/3 docs where they are actionable. Made-with: Cursor
akulakum
changed the title
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 91dd40532f
ℹ️ 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".
|
|
||
| | Order | Document | What to Do | | ||
| |-------|----------|------------| | ||
| | 1 | [store-event-wiring-migration.md](./store-event-wiring-migration.md) | Simplify 30+ event handlers — remove `refreshTaskList()`, add `TASK_UI_CONTROLS_UPDATED` subscription | |
There was a problem hiding this comment.
The execution-order table links to ./store-event-wiring-migration.md and five similar files that are not present in this commit, so readers following the "single entry point" guidance hit dead links immediately and cannot continue the migration workflow. I verified the scope under packages/contact-center/ai-docs/migration in this tree and only migration-overview.md exists, so these references need to be updated (or clearly marked as pending) to avoid blocking migration work.
Useful? React with 👍 / 👎.
3 participants
COMPLETES #https://jira-eng-sjc12.cisco.com/jira/browse/CAI-7714
This pull request addresses
The CC SDK task-refactor branch has completed a major architectural shift — replacing ad-hoc task state management with a state-machine-driven architecture. The SDK now computes UI control states (
task.uiControls) internally and exposes them via a getter and atask:ui-controls-updatedevent, eliminating the need for widgets to derive control visibility from raw task data.This is PR 1 of 4 in the task refactor migration documentation series. It provides the single entry-point overview document — the architectural comparison, execution order, SDK prerequisites, key types, files affected, and SDK reference.
by making the following changes
Added 1 consolidated migration overview document under
packages/contact-center/ai-docs/migration/:TaskUIControlsstructure, execution order (6 migration docs to follow in sequence), SDK pending exports, key types from SDK, CC Widgets files affected, and CC SDK referenceAlso updated:
migration-overview.mdCC SDK Task-Refactor Branch Reference
Repo: webex/webex-js-sdk (task-refactor)
Key SDK files:
uiControlsComputer.tsTaskUIControlsfromTaskState+TaskContextTask.tstask.uiControlsgetter andtask:ui-controls-updatedeventconstants.tsTaskStateandTaskEventenumsChange Type
Related PRs
The following scenarios were tested
The GAI Coding Policy And Copyright Annotation Best Practices
Checklist before merging