feat(internal-plugin-task): support task management by vskygk · Pull Request #4453 · webex/webex-js-sdk

vskygk · 2025-08-26T07:51:09Z

COMPLETES #< https://jira-eng-gpk2.cisco.com/jira/browse/WEBEX-436262 >

This pull request addresses

Refer to above Epic, we need to implement a new plugin to support new task management feature

by making the following changes

Add a new plugin internal-plugin-task

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

Manually tested the API calls to task service
Write UT to cover code logic

The GAI Coding Policy And Copyright Annotation Best Practices

I certified that

I have read and followed contributing guidelines
I discussed changes with code owners prior to submitting this pull request
I have not skipped any automated checks
All existing and new tests passed
I have updated the documentation accordingly

Make sure to have followed the contributing guidelines before submitting.

arun3528 · 2025-08-28T13:54:42Z

+import WebexCore from '@webex/webex-core';
+
+const webex = new WebexCore();
+webex.internal.task.WHATEVER;


add more info on the usage

OK, I will add more usage to README.

arun3528 · 2025-08-28T13:55:58Z

@@ -0,0 +1 @@
+module.exports = {browser: true};


why would we need this , it should also be able to run on node env , i see lot of use cases for this

I just followed other internal plugin's way to introduce this file, I am not sure if there are any potential issues if missing it.

arun3528 · 2025-08-28T13:57:22Z

@@ -0,0 +1,50 @@
+const _decryptTextProp = (ctx, name, key, object) => {


the mercury already decrypts text you dont need this

i am not sure about that @arun3528 . i think each plugin still needs to manage decrypting mercury events

@arun3528 It is not for mercury event, it is for decrypting API response.

@robstax i was looking throguh it the reason is because the each plugin can have there own intercepter they dont have to keep adding the encrypt and decrypy logic in all the place , they just have to add it in once place

they just have to do this

https://github.com/webex/webex-js-sdk/blob/next/packages/%40webex/internal-plugin-conversation/src/index.js

@vskygk i would add a comments saying not a standard way and use intercepter , so that others dont copy the same

@arun3528 got, thank you very much!

arun3528 · 2025-08-28T13:58:55Z

@@ -0,0 +1,45 @@
+import {isArray} from 'lodash';


lets check on this as well , i think the SDk also does encrypt the text check how the conversaion plugin is setup , that would help

its all done via intercepters

@arun3528 the decrypt/encrypt flow in the SDK plugins is pretty convoluted and pretty unfriendly to dev. different plugins seem to have adapted other ways of doing it. this task plugin seems to be heavily copying calendar plugin (see: packages/@webex/internal-plugin-calendar/src/calendar.decrypt.helper.js). also ai-assistant plugin did decryption different and just called await this.webex.internal.encryption.decryptText(encryptionKeyUrl, value) wherever it was needed rather than the payload transformer flow.

@arun3528 @robstax, yes, I followed calendar plugin's way to encrypt/decrypt data, I ever considered using payload transformer, but it seems too magic and hard to deal with special logic if we have in future.

arun3528 · 2025-08-28T13:59:34Z

+
+registerInternalPlugin('task', Task, {
+  config,
+  payloadTransformer: {


Here is where the logic for encryption and decryption will go

I ever meet a issue before, the encrypt/decrypt rules configured in one plugin's payloadTransformer will affect another plugin, or conflict with each other, there is a example:

{
name: 'transformMeetingParticipants',
direction: 'inbound',
test(ctx, response) {
return Promise.resolve(
has(response, 'body.encryptedParticipants') &&
!(
response.options &&
response.options.service === 'calendar' &&
response.options.method === 'GET' &&
response.options.resource === 'schedulerData'
)
);
},

We have to add such codes to resolve conflict.

arun3528 · 2025-08-28T14:00:19Z

+    // operation.
+    this.listenToOnce(this.webex, 'ready', () => {
+      // Pre-fetch a KMS encryption key url to improve performance
+      this.webex.internal.encryption.kms.createUnboundKeys({count: 1}).then((keys) => {


OK, I will make it more promise:)

arun3528 · 2025-08-28T14:01:43Z

+
+    if (this.registered) {
+      this.logger.info('Task->register#INFO, Calendar plugin already registered');
+


should you trigger raindrop register event here as well , if its already registred

No need here, if this.registered is true, it should have triggered the register event here:

return this.webex.internal.device
.register()
.then(() => this.webex.internal.mercury.connect())
.then(() => {
this.listenForEvents();
this.trigger(RAINDROP_REGISTERED);
this.registered = true;
})

arun3528 · 2025-08-28T14:03:00Z

+
+    this.stopListeningForEvents();
+
+    return this.webex.internal.mercury


why do you want to un register socket when you un register for task

this seems to be a pattern that has emerged. i am not totally sure of it's consequences, but it's done in meetings, ai-assistant, dss, device, and calendar plugins. all of them disconnect from mercury on unregister

As I know, the calendar plugin calls unregister() on beforeunload to release connections when running in site web pages, likely as a protection mechanism.

This is going to cause issues @robstax , if someone does a un register on task but they need meetings or other the socket would get disconnected

webex.logout whould dergister device and socket

can we check once , all the other plugin might have un register but might not be calling it , launch the web app , in console do task.unregister and see what happens

@arun3528 you are right, it is not safe to disconnect the socket, I have removed related logic and only stop listening the events.

robstax · 2025-08-28T16:17:18Z

+
+    this.stopListeningForEvents();
+
+    return this.webex.internal.mercury


this seems to be a pattern that has emerged. i am not totally sure of it's consequences, but it's done in meetings, ai-assistant, dss, device, and calendar plugins. all of them disconnect from mercury on unregister

robstax · 2025-08-28T16:19:17Z

+    // operation.
+    this.listenToOnce(this.webex, 'ready', () => {
+      // Pre-fetch a KMS encryption key url to improve performance
+      this.webex.internal.encryption.kms.createUnboundKeys({count: 1}).then((keys) => {


i don't think we should do this. we only do it in calendar plugin and i am not sure why. seems strange

@robstax Caching an encryption key URL here was intended to improve performance. However, after reviewing the createUnboundKeys implementation, I found it simply connects to Mercury to fetch a key, which should be very fast. I’ll remove this part of the code and run some tests.

robstax · 2025-08-28T16:20:56Z

+  },
+
+  /**
+   * Explicitly tears down the calendar plugin by de-registering


are these bad copy/pastes in the comments? i see lots of references to calendar plugin in this file in the comments. i don't think that's correct?

Yes, I will correct them.

robstax · 2025-08-28T16:22:11Z

@@ -0,0 +1,50 @@
+const _decryptTextProp = (ctx, name, key, object) => {


i am not sure about that @arun3528 . i think each plugin still needs to manage decrypting mercury events

robstax · 2025-08-28T16:23:51Z

@@ -0,0 +1,45 @@
+import {isArray} from 'lodash';


@arun3528 the decrypt/encrypt flow in the SDK plugins is pretty convoluted and pretty unfriendly to dev. different plugins seem to have adapted other ways of doing it. this task plugin seems to be heavily copying calendar plugin (see: packages/@webex/internal-plugin-calendar/src/calendar.decrypt.helper.js). also ai-assistant plugin did decryption different and just called await this.webex.internal.encryption.decryptText(encryptionKeyUrl, value) wherever it was needed rather than the payload transformer flow.

arun3528 · 2025-10-13T02:28:32Z

+import '@webex/internal-plugin-conversation';
+import testUsers from '@webex/test-helper-test-users';
+
+describe.skip('plugin-task', function () {


why is this skipped ?

I will enable it to check if it can pass, because when running locally, it always returning error:

WARN: 'http POST to https://cig-service-intb.ciscospark.com/cig-service/api/v1/users/test_users_s result: 403'
ERROR: '###Test error: Forbidden: Forbidden. Retrying test in 1000 seconds'

After I enable this integration test, it is still blocked by following error:

Google Chrome is not currently installed; installing it
Preparing Chrome installation for Debian-based systems
https://dl.google.com/linux/chrome/deb/pool/main/g/google-chrome-stable/google-chrome-stable_114.0.5735.90-1_amd64.deb:
2025-10-14 03:20:41 ERROR 404: Not Found.
/bin/bash: line 129: google-chrome-stable: command not found
Google Chrome v114.0.5735.90 (stable) failed to install.

Exited with code exit status 1

adhmenon · 2025-10-28T11:10:02Z

@vskygk can you change the target branch to next. That is the main active branch for this repo.
It might be the reason why the integration tests are failing for you.

vskygk requested review from a team as code owners August 26, 2025 07:51

arun3528 requested changes Aug 28, 2025

View reviewed changes

robstax reviewed Aug 28, 2025

View reviewed changes

arun3528 reviewed Oct 13, 2025

View reviewed changes

adhmenon added the validated If the pull request is validated for automation. label Oct 28, 2025

vskygk changed the base branch from master to next November 4, 2025 01:23

vskygk changed the base branch from next to master November 4, 2025 01:26

vskygk force-pushed the master branch from d31bc44 to e44d817 Compare December 3, 2025 01:10

vskygk closed this Dec 12, 2025

vskygk force-pushed the master branch from e44d817 to a012ca3 Compare December 12, 2025 03:41

		@@ -0,0 +1,50 @@
		const _decryptTextProp = (ctx, name, key, object) => {


		if (this.registered) {
		this.logger.info('Task->register#INFO, Calendar plugin already registered');


		this.stopListeningForEvents();

		return this.webex.internal.mercury

Conversation

vskygk commented Aug 26, 2025

COMPLETES #< https://jira-eng-gpk2.cisco.com/jira/browse/WEBEX-436262 >

This pull request addresses

by making the following changes

Change Type

The following scenarios were tested

The GAI Coding Policy And Copyright Annotation Best Practices

I certified that

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

vskygk Aug 29, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

adhmenon commented Oct 28, 2025

Uh oh!

vskygk Aug 29, 2025 •

edited

Loading