docs(ai-docs): task refactor migration — store layer (PR 2/4)#646
docs(ai-docs): task refactor migration — store layer (PR 2/4)#646akulakum wants to merge 17 commits intowebex:nextfrom
Conversation
Add store layer migration documentation: - 003: Store event wiring migration (30+ event handlers in storeEventsWrapper.ts) - 008: Store task-utils migration (~15 utility functions, findHoldStatus/findHoldTimestamp retention) Made-with: Cursor
![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: 08e96feca6
ℹ️ 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".
| // REMOVED from store/task-utils.ts — SDK tracks hold state in TaskContext: | ||
| // - task.uiControls.hold.isEnabled indicates holdable | ||
| // - task.uiControls.switchToConsult.isVisible indicates consult call is held | ||
| // No widget-side derivation needed. |
There was a problem hiding this comment.
This "After" snippet says findHoldStatus is removed and that hold state should be inferred from task.uiControls, but the same document elsewhere says findHoldStatus must be kept and explicitly warns that controls.hold.isEnabled is not a reliable held-state signal. Following this removal guidance would cause held calls to be misclassified during consult/transition states, which can regress TaskList status and held-state UI behavior.
Useful? React with 👍 / 👎.
| | `handleConsulting` | Refresh + callbacks | **Simplify** — callbacks only | | ||
| | `handleConsultAccepted` | Refresh + callbacks | **Simplify** — callbacks only | | ||
| | `handleConsultOffer` | Refresh + callbacks | **Simplify** — callbacks only | | ||
| | `handleConferenceStarted` | Refresh + callbacks | **Simplify** — callbacks only | | ||
| | `handleConferenceEnded` | Refresh + callbacks | **Simplify** — callbacks only | |
There was a problem hiding this comment.
The mapping marks handleConsulting and handleConferenceStarted as callback-only simplifications, but this conflicts with the earlier event table in the same doc that keeps these handlers to update consultStartTimeStamp, isQueueConsultInProgress, and currentConsultQueueId. If implementers follow this row, consult/conference transitions will stop resetting these store fields, breaking timer/reset behavior after those events.
Useful? React with 👍 / 👎.
|
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: 566a06c19d
ℹ️ 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 Handler | Old Action | New Action | | ||
| |-------------|-----------|------------| | ||
| | `refreshTaskList` (15+ events) | Re-fetch ALL tasks from SDK | **Remove** — task.data is live; fire callbacks only | |
There was a problem hiding this comment.
This mapping says refreshTaskList can be removed for the whole 15+ event group, but AGENT_WRAPPEDUP is currently one of those registrations in storeEventsWrapper.ts and is used to clear wrapped-up tasks from store state. If migration work follows this row literally, wrap-up completion can leave stale interactions in taskList/currentTask until another full refresh path runs.
Useful? React with 👍 / 👎.
| | `handleConsultOffer` | Refresh + callbacks | **Simplify** — callbacks only | | ||
| | `handleConferenceStarted` | Refresh + callbacks | **Simplify** — callbacks only | | ||
| | `handleConferenceEnded` | Refresh + callbacks | **Simplify** — callbacks only | | ||
| | `handleAutoAnswer` | Refresh + callbacks | **Simplify** — callbacks only | |
There was a problem hiding this comment.
Marking handleAutoAnswer as callback-only drops behavior that currently sets isDeclineButtonEnabled in the store; that flag is read by TaskList disable logic for auto-answering calls (disableDecline depends on !store.isDeclineButtonEnabled). Following this guidance would keep Decline disabled when auto-answer is active, changing user-facing call controls.
Useful? React with 👍 / 👎.
Move migration docs from ai-docs/migration/ to packages/contact-center/ai-docs/migration/ per reviewer feedback — this migration is specific to contact center. Made-with: Cursor
Remove findHoldTimestamp signature mismatch section from 008 — not a task-refactor change. Fix double --- separators in 003 and 008. Made-with: Cursor
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c05d0ccba0
ℹ️ 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".
| | `handleTaskAssigned` | Set current task | **Keep** — still need list management | | ||
| | `handleConsultCreated` | Refresh + callbacks | **Simplify** — callbacks only | | ||
| | `handleConsulting` | Refresh + callbacks | **Simplify** — callbacks only | | ||
| | `handleConsultAccepted` | Refresh + callbacks | **Simplify** — callbacks only | |
There was a problem hiding this comment.
This row marks handleConsultAccepted as callback-only, but store/src/storeEventsWrapper.ts::handleConsultAccepted currently performs essential state work beyond refreshes, including attaching TASK_MEDIA for browser devices (validated by store/tests/storeEventsWrapper.ts around the handleConsultAccepted media-listener tests). If migration follows this mapping literally, consult-accepted browser flows can lose media-track setup and regress call audio behavior.
Useful? React with 👍 / 👎.
| | `refreshTaskList` (15+ events) | Re-fetch ALL tasks from SDK | **Remove** — task.data is live; fire callbacks only | | ||
| | `handleTaskEnd` | Remove from list + cleanup | **Keep** — still need list management | | ||
| | `handleTaskAssigned` | Set current task | **Keep** — still need list management | | ||
| | `handleConsultCreated` | Refresh + callbacks | **Simplify** — callbacks only | |
There was a problem hiding this comment.
Mapping handleConsultCreated to callback-only drops the current consultStartTimeStamp update performed in store/src/storeEventsWrapper.ts::handleConsultCreated (and explicitly asserted in store/tests/storeEventsWrapper.ts), so AGENT_CONSULT_CREATED would no longer initialize consult timing state. That can leave consult timers stale or unset until a later event arrives, which changes user-visible consult timing behavior.
Useful? React with 👍 / 👎.
Removed serial numbers from filenames. Added event name rename table, constants to delete/keep, CONSULT_INITIATING gotcha, and fixed findHoldStatus contradiction in store-task-utils doc. Made-with: Cursor
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 81c581c128
ℹ️ 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".
| | Delete | Reason | | ||
| |--------|--------| | ||
| | Local `TASK_EVENTS` enum (`store/src/store.types.ts`) | SDK exports this — delete local copy | | ||
| | `TASK_STATE_CONSULT`, `TASK_STATE_CONSULTING`, `TASK_STATE_CONSULT_COMPLETED` | SDK handles via state machine | |
There was a problem hiding this comment.
This row says TASK_STATE_CONSULT, TASK_STATE_CONSULTING, and TASK_STATE_CONSULT_COMPLETED should be deleted, but the same migration document also says findHoldStatus is kept with unchanged implementation, and that implementation in store/src/task-utils.ts still depends on those constants (via isConsultOnHoldMPC and consult-completed checks). If implementers delete these constants following this guidance before rewriting hold-state logic, the store layer will either fail to compile or regress held-state derivation.
Useful? React with 👍 / 👎.
| | `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` | **Keep refresh or add explicit task removal** — task must be removed from `taskList`/`currentTask` after wrapup completion to prevent stale UI | |
There was a problem hiding this comment.
The document first maps AGENT_WRAPPEDUP to SDK TASK_WRAPPEDUP and tells readers to import TASK_EVENTS from SDK, but this “new registration” table still uses AGENT_WRAPPEDUP. In a migration that actually switches to SDK enums/events, this mismatch can leave wrap-up completion unsubscribed (or non-compiling), which in turn risks stale taskList/currentTask cleanup after wrap-up.
Useful? React with 👍 / 👎.
Cross-validated against both CC Widgets and CC SDK task-refactor repos: - Fix event count (27 not 30+), add all handler detail (state mutations, side effects) - Document handleTaskRemove cleanup, pre-existing bugs (dead handleConsultEnd, listener leak) - Add store-only enum members to delete, new SDK events to evaluate - Fix contradictions between proposed and mapping tables (consolidated into single table) - Correct renamed event usage in After code examples - Fix function counts (remove 5 not 6), add ConsultStatus enum to deletions - Add participant role constants to keep list, findHoldTimestamp dual-signature note - Replace simplified getTaskStatus Before code with actual implementation - Add all 22 get*ButtonVisibility functions to deletion scope - Add review function dependencies, test file impact, barrel export notes Made-with: Cursor
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: af98aa7af8
ℹ️ 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.on(TASK_EVENTS.TASK_UI_CONTROLS_UPDATED, (uiControls) => { | ||
| this.fireTaskCallbacks(TASK_EVENTS.TASK_UI_CONTROLS_UPDATED, interactionId, uiControls); | ||
| }); |
There was a problem hiding this comment.
This on registration uses an inline arrow function, but the cleanup section later removes TASK_UI_CONTROLS_UPDATED with this.handleUIControlsUpdated; those are different function references, so off(...) will not detach the listener. If implementers follow this pattern, removed/switched tasks can keep stale listeners and fire duplicate UI-control callbacks on future updates.
Useful? React with 👍 / 👎.
| this.fireTaskCallbacks(TASK_EVENTS.TASK_CONFERENCE_STARTED, interactionId); | ||
| this.fireTaskCallbacks(TASK_EVENTS.TASK_PARTICIPANT_JOINED, interactionId); | ||
| }; |
There was a problem hiding this comment.
This shared handler fires both TASK_CONFERENCE_STARTED and TASK_PARTICIPANT_JOINED callbacks unconditionally, while the registration table binds the same handler to each event type; as a result, one incoming event can produce two different callback notifications and trigger widget side effects twice or at the wrong phase. The same duplication pattern appears in handleConferenceEnded for ended/participant-left callbacks.
Useful? React with 👍 / 👎.
Add callout note explaining getControlsVisibility lives in task/src/Utils/task-util.ts (hook layer), not the store. It appears in the store utils doc because it is the primary consumer of the 5 store functions being removed. Made-with: Cursor
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 850b0bacdd
ℹ️ 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.on(TASK_EVENTS.TASK_AUTO_ANSWERED, this.handleAutoAnswer); | ||
|
|
||
| // KEEP: Wrapup completion — task must be removed from list | ||
| task.on(TASK_EVENTS.TASK_WRAPPEDUP, (data) => { // renamed from AGENT_WRAPPEDUP |
There was a problem hiding this comment.
This TASK_WRAPPEDUP registration uses an inline arrow function, but the same document’s cleanup pattern relies on off(event, handler) matching the original handler reference; with an inline callback, handleTaskRemove cannot reliably detach the listener. If implementers follow this snippet, removed/re-hydrated tasks can retain stale wrapup listeners and trigger duplicate refreshTaskList()/callback executions on later wrapup events.
Useful? React with 👍 / 👎.
| this.fireTaskCallbacks(TASK_EVENTS.TASK_CONFERENCE_STARTED, interactionId); | ||
| this.fireTaskCallbacks(TASK_EVENTS.TASK_PARTICIPANT_JOINED, interactionId); |
There was a problem hiding this comment.
This handler fires callbacks with interactionId, but the shown method signature does not receive interactionId and the snippet defines it as a standalone class handler. Implementing this literally leaves interactionId unresolved at runtime, so conference events can fail callback dispatch (or dispatch with the wrong task id), which breaks conference-related widget updates.
Useful? React with 👍 / 👎.
1. Add ordering constraint for consult state constants — TASK_STATE_CONSULT, TASK_STATE_CONSULTING, TASK_STATE_CONSULT_COMPLETED must not be deleted until findHoldStatus/isConsultOnHoldMPC are rewritten to use SDK TaskState. 2. Use stable named handler (handleUIControlsUpdated) for TASK_UI_CONTROLS_UPDATED event instead of inline arrow, so handleTaskRemove can correctly .off() it. 3. Fix conference handlers to accept eventType parameter and fire only the triggering event's callback, preventing duplicate notifications when both TASK_CONFERENCE_STARTED and TASK_PARTICIPANT_JOINED are wired to the same handler. Made-with: Cursor
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0df8e9c70b
ℹ️ 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 interactionId = this.store.currentTask?.data.interactionId; | ||
| if (interactionId) { |
There was a problem hiding this comment.
handleUIControlsUpdated derives interactionId from this.store.currentTask, so when TASK_UI_CONTROLS_UPDATED fires for a non-selected task (multi-task/consult scenarios), callbacks are dispatched under the wrong task id. That can prevent the emitting task’s widgets from re-rendering and can incorrectly notify the current task instead; this handler should use the interaction id of the task that raised the event, not global current-task state.
Useful? React with 👍 / 👎.
|
|
||
| // KEEP: Conference state management (remove refreshTaskList, keep consult state reset) | ||
| // Pass event type so handler fires only the correct callback (not both) | ||
| task.on(TASK_EVENTS.TASK_PARTICIPANT_JOINED, () => this.handleConferenceStarted(TASK_EVENTS.TASK_PARTICIPANT_JOINED)); |
There was a problem hiding this comment.
This registration uses a new inline function, but the cleanup section requires off(event, handler) to receive the exact same handler reference. Without storing these wrapper callbacks, TASK_PARTICIPANT_JOINED/STARTED/ENDED/LEFT listeners cannot be detached on task removal, which can leave stale listeners and produce duplicate conference callbacks after task switches.
Useful? React with 👍 / 👎.
…sues All 4 issues stem from the tension between needing per-task context (interactionId) and stable handler references for .off() cleanup: 1. TASK_WRAPPEDUP inline arrow — now a bound handler stored in map 2. Conference handlers unresolved interactionId — now passed as parameter from bound handler wrappers 3. handleUIControlsUpdated wrong task ID — no longer reads currentTask; uses bound interactionId from the emitting task's registration closure 4. Conference inline wrappers not detachable — replaced with stored bound handlers that handleTaskRemove retrieves from the map Pattern: taskBoundHandlers Map<interactionId, Record<string, Function>> - Created per-task in registerTaskEventListeners - Retrieved in handleTaskRemove for exact .off() reference matching - Deleted after cleanup to prevent memory leaks Made-with: Cursor
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5d29b58c40
ℹ️ 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".
| | `INTERACTION_STATE_WRAPUP` | `store/src/constants.ts` | SDK handles via `TaskState.WRAPPING_UP` | | ||
| | `INTERACTION_STATE_POST_CALL` | `store/src/constants.ts` | SDK handles via `TaskState.POST_CALL` | | ||
| | `INTERACTION_STATE_CONNECTED` | `store/src/constants.ts` | SDK handles via `TaskState.CONNECTED` | | ||
| | `INTERACTION_STATE_CONFERENCE` | `store/src/constants.ts` | SDK handles via `TaskState.CONFERENCING` | |
There was a problem hiding this comment.
These rows schedule INTERACTION_STATE_WRAPUP/POST_CALL/CONNECTED/CONFERENCE for deletion, but the same document keeps findHoldStatus and only marks getConsultMPCState for later review. In the current store code, findHoldStatus still reaches getConsultMPCState, which still uses these interaction-state constants, so following this deletion guidance before rewriting that chain will break compilation and held-state derivation during migration.
Useful? React with 👍 / 👎.
| task.on(TASK_EVENTS.TASK_HOLD, () => { | ||
| this.fireTaskCallbacks(TASK_EVENTS.TASK_HOLD, interactionId); | ||
| }); | ||
| task.on(TASK_EVENTS.TASK_RESUME, () => { | ||
| this.fireTaskCallbacks(TASK_EVENTS.TASK_RESUME, interactionId); |
There was a problem hiding this comment.
This "After" snippet reintroduces inline callbacks for TASK_HOLD/TASK_RESUME, but the cleanup strategy in this same doc depends on removing listeners via stored handler references from taskBoundHandlers. If implementers copy this snippet literally, off(...) will not match these inline functions, leaving stale listeners attached and causing duplicate callback firing after task removal/re-registration.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b91d8b78ea
ℹ️ 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".
| // KEEP: Task lifecycle events that need store-level management (class methods — no interactionId needed) | ||
| task.on(TASK_EVENTS.TASK_END, this.handleTaskEnd); | ||
| task.on(TASK_EVENTS.TASK_ASSIGNED, this.handleTaskAssigned); | ||
| task.on(TASK_EVENTS.TASK_REJECT, this.handleTaskReject); |
There was a problem hiding this comment.
This migration snippet wires TASK_REJECT directly to this.handleTaskReject, but the current store handler takes (task, reason) and today is intentionally registered with a task-bound wrapper ((reason) => this.handleTaskReject(task, reason) in store/src/storeEventsWrapper.ts, with tests asserting that task is forwarded). If implementers follow this line literally, reject events provide only the reason and lose the emitting task context, which can misfire onTaskRejected and remove/update the wrong interaction in multi-task flows.
Useful? React with 👍 / 👎.
| | `CONSULT_STATE_INITIATED` | `store/src/constants.ts` | SDK handles via context | | ||
| | `CONSULT_STATE_COMPLETED` | `store/src/constants.ts` | SDK handles via context | | ||
| | `CONSULT_STATE_CONFERENCING` | `store/src/constants.ts` | SDK handles via context | |
There was a problem hiding this comment.
These rows instruct deleting CONSULT_STATE_INITIATED, CONSULT_STATE_COMPLETED, and CONSULT_STATE_CONFERENCING without any ordering constraint, but store/src/task-utils.ts still imports and branches on all three inside getConsultMPCState (and getTaskStatus’s consult-completed path). Following this table as written before rewriting those helpers will break compilation and regress consult/hold derivation; it needs the same explicit "delete only after rewrite" guard used for other state constants in this doc.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 307891e442
ℹ️ 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 controls = currentTask?.uiControls ?? getDefaultUIControls(); | ||
| // All 17 controls come pre-computed from SDK. Zero store util calls needed. |
There was a problem hiding this comment.
This migration step bypasses getControlsVisibility and reads raw task.uiControls, but the existing control logic also applies widget-level gates from featureFlags and the conferenceEnabled prop in task/src/Utils/task-util.ts. Because those inputs are not part of task state, SDK-computed uiControls cannot enforce them, so following this guidance can make controls (for example conference/transfer/end-consult) visible or enabled even when an integrator has explicitly disabled them.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 708fad0bf4
ℹ️ 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 (controls.wrapup.isVisible) return 'Wrap Up'; | ||
| if (controls.endConsult.isVisible) return 'Consulting'; | ||
| if (controls.exitConference.isVisible) return 'Conference'; |
There was a problem hiding this comment.
This rewrite changes getTaskStatus to return display labels ('Wrap Up', 'Consulting', 'Conference', etc.) instead of the existing machine-readable state values, which is a breaking API change for any downstream consumer of @webex/cc-store that compares status strings. Since this same document calls out that task-utils is barrel-exported, migration guidance should keep the existing return contract (or introduce a separate display-status helper) to avoid silent behavior regressions outside this repo.
Useful? React with 👍 / 👎.
| | `store/src/storeEventsWrapper.ts` | Refactor `registerTaskEventListeners` (see definitive table), update `handleTaskRemove` (fix listener mismatches + add `TASK_UI_CONTROLS_UPDATED`), simplify handlers (remove `refreshTaskList()` from all except `TASK_WRAPPEDUP`), wire `handleConsultEnd` to `TASK_CONSULT_END` | | ||
| | `store/src/store.ts` | No changes expected (observables stay) | | ||
| | `store/src/store.types.ts` | Delete local `TASK_EVENTS` enum; import from SDK (which includes `TASK_UI_CONTROLS_UPDATED`) | |
There was a problem hiding this comment.
This section instructs replacing the local TASK_EVENTS enum with SDK TASK_EVENTS but only lists store files, which is incomplete: task-layer consumers still use legacy members (AGENT_WRAPPEDUP, CONTACT_RECORDING_PAUSED, CONTACT_RECORDING_RESUMED in packages/contact-center/task/src/helper.ts). If implementers follow this file list literally, those references become invalid and the build breaks, so this step needs to explicitly include (or clearly sequence) the required task-package updates.
Useful? React with 👍 / 👎.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7364f9d112
ℹ️ 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".
| | 3 | `TASK_REJECT` | `handleTaskReject` | **Keep** | Remove from task list | | ||
| | 4 | `TASK_OUTDIAL_FAILED` | `handleOutdialFailed` | **Keep** | Remove from task list | | ||
| | 5 | `TASK_MEDIA` | `handleTaskMedia` | **Keep** | Browser-only WebRTC setup (conditional registration) | | ||
| | 6 | `TASK_UI_CONTROLS_UPDATED` | `handleUIControlsUpdated` | **Add new** | Fire callbacks to trigger widget re-renders | |
There was a problem hiding this comment.
The "Definitive New Event Registration" section labels TASK_UI_CONTROLS_UPDATED with handleUIControlsUpdated, but this doc later explains that a class-level handler is incorrect in multi-task/consult scenarios because it can resolve the wrong interactionId from currentTask; since this table is presented as the single authoritative mapping, readers can implement the wrong wiring and misroute callbacks. The table entry should explicitly point to the per-task bound handler pattern used later in the document.
Useful? React with 👍 / 👎.
|
|
||
| --- | ||
|
|
||
| _Parent: [migration-overview.md](./migration-overview.md)_ |
There was a problem hiding this comment.
This file ends with a _Parent: [migration-overview.md] link, but migration-overview.md is not present in this commit tree, so navigation from the new migration docs is broken. Because this is part of a documentation series, shipping broken parent links makes the migration flow harder to follow and should be fixed by adding the referenced file or updating the link target.
Useful? React with 👍 / 👎.
|
|
||
| | # | Function | Reason | SDK Replacement | Impact on Other Functions | | ||
| |---|----------|--------|-----------------|--------------------------| | ||
| | 1 | `getConsultStatus(task, agentId)` | Primary consumer `getControlsVisibility` is deleted | `task.uiControls` encodes all consult control states | `getTaskStatus()` calls this — must be updated (see After code below) | |
There was a problem hiding this comment.
The removal table says getTaskStatus() calls getConsultStatus(), but the actual relationship is the reverse (getConsultStatus calls getTaskStatus, including in the existing implementation and in the later snippet in this same doc). Keeping this dependency backwards can mislead migration sequencing and make reviewers look for the wrong call-site updates when deleting getConsultStatus.
Useful? React with 👍 / 👎.
…tatus dependency direction
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d575a7b690
ℹ️ 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".
| export function getTaskStatus(task: ITask, agentId: string): string { | ||
| const interaction = task.data.interaction; | ||
| const controls = task.uiControls; | ||
| if (!controls) return interaction?.state ?? ''; |
There was a problem hiding this comment.
The proposed getTaskStatus rewrite returns immediately when task.uiControls is absent, which skips the legacy EP-DN and consult-completed branches shown just below and changes behavior for tasks that arrive before controls are populated (for example hydration/race windows). In that case status falls back to raw interaction.state instead of the existing TASK_STATE_CONSULTING/TASK_STATE_CONSULT_COMPLETED derivation, so downstream timers and state-dependent UI can regress even though the same task worked under the pre-migration logic.
Useful? React with 👍 / 👎.
|
|
||
| // Map from uiControls to same constant values as old getConsultMPCState / getTaskStatus | ||
| if (controls.wrapup.isVisible) return INTERACTION_STATE_WRAPUP; | ||
| if (controls.endConsult.isVisible) return TASK_STATE_CONSULTING; |
There was a problem hiding this comment.
Mapping controls.endConsult.isVisible directly to TASK_STATE_CONSULTING collapses consult-initiating and consulting phases, despite this same doc’s gotcha that CONSULT_INITIATING and CONSULTING must stay distinct. The legacy visibility logic (task/src/Utils/task-util.ts getEndConsultButtonVisibility) treats end-consult visibility as valid for both initiated and accepted consult, so this mapping can misclassify initiated consults and break consult timer/state transitions that depend on the TASK_STATE_CONSULT vs TASK_STATE_CONSULTING distinction.
Useful? React with 👍 / 👎.
…nsultMPCState for endConsult
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: df026d84d5
ℹ️ 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".
| | 4 | `TASK_OUTDIAL_FAILED` | `handleOutdialFailed` | **Keep** | Remove from task list | | ||
| | 5 | `TASK_MEDIA` | `handleTaskMedia` | **Keep** | Browser-only WebRTC setup (conditional registration) | | ||
| | 6 | `TASK_UI_CONTROLS_UPDATED` | `bound.uiControlsUpdated` (per-task; see Pattern 1 & 3) | **Add new** | Fire callbacks to trigger widget re-renders. Do **not** use a class-level handler — it would resolve the wrong `interactionId` in multi-task scenarios. | | ||
| | 7 | `TASK_WRAPPEDUP` | `handleWrappedup` | **Keep + rename** | Was `AGENT_WRAPPEDUP`. Keep `refreshTaskList()` — task must be removed from list after wrapup. Fire callback. | |
There was a problem hiding this comment.
This section says the table is the “single authoritative” registration map, but the TASK_WRAPPEDUP row names handleWrappedup while the implementation patterns and cleanup logic rely on bound.wrappedup (the closure that carries interactionId and is removable via off(...)). If someone follows the table literally, they can wire a class handler that diverges from the documented bound-handler cleanup strategy and regress callback routing/teardown for wrapup events.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 585851545d
ℹ️ 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".
| > | `featureFlags.webRtcEnabled` | accept, decline, muteUnmute, conference, muteUnmuteConsult, + telephony support (holdResume, endConsult) | Hide control when `webRtcEnabled` is `false` and channel is voice in browser | | ||
| > | `featureFlags.isEndCallEnabled` | end | Hide end button when `isEndCallEnabled` is `false` (phone device only) | | ||
| > | `featureFlags.isEndConsultEnabled` | endConsult | Hide end-consult when `isEndConsultEnabled` is `false` | | ||
| > | `conferenceEnabled` (widget prop) | conference, exitConference, mergeConference, consultTransferConsult | Hide all conference-related controls when `conferenceEnabled` is `false` | |
There was a problem hiding this comment.
The new applyFeatureGates table is still incomplete: it lists conferenceEnabled gating for conference/exitConference/mergeConference/consultTransferConsult, but omits mergeConferenceConsult, which is currently gated by conferenceEnabled in task/src/Utils/task-util.ts (getMergeConferenceConsultButtonVisibility). The webRtcEnabled row also omits controls like transfer that are currently hidden via isTransferVisibility. If implementers follow this table literally, integrator-disabled controls can reappear in production after migration.
Useful? React with 👍 / 👎.
1 participant
COMPLETES #https://jira-eng-sjc12.cisco.com/jira/browse/CAI-7714
This pull request addresses
This is PR 2 of 4 in the task refactor migration documentation series. It covers the store layer changes — event wiring simplification and store utility function migration.
by making the following changes
Added 2 store layer migration documents under
packages/contact-center/ai-docs/migration/:store-event-wiring-migration.md: Documents how 30+ event handler subscriptions in
storeEventsWrapper.tschange:refreshTaskList()calls becomefireTaskCallbacks()(SDK keeps task.data in sync)TASK_UI_CONTROLS_UPDATEDsubscription added to trigger widget re-rendersstore.types.tsregisterTaskEventListenersstore-task-utils-migration.md: Identifies ~15 utility functions and their migration fate:
getConsultStatus,getIsConferenceInProgress,getConferenceParticipantsCount,getIsCustomerInCall,getIsConsultInProgressisIncomingTask,getTaskStatus,getConferenceParticipants,isInteractionOnHold,findMediaResourceId,findHoldTimestamp,findHoldStatusTaskState.CONSULT_INITIATINGvsCONSULTINGdistinctiongetTaskStatusenhancementCC SDK Task-Refactor Branch Reference
Change Type
Related PRs
The following scenarios were tested
The GAI Coding Policy And Copyright Annotation Best Practices
Checklist before merging