feat(webexconnect): bringing Webex Connect SDK into webex-js-sdk monorepo - #5201
feat(webexconnect): bringing Webex Connect SDK into webex-js-sdk monorepo#5201riag23 wants to merge 1 commit into
Conversation
️✅ There are no secrets present in this pull request anymore.If these secrets were true positive and are still valid, we highly recommend you to revoke them. 🦉 GitGuardian detects secrets in your source code to help developers and security teams secure the modern development process. You are seeing this because you or someone else with access to this repository has authorized GitGuardian to scan your pull request. |
![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: a0d1030262
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| ev.waitUntil(async () => { | ||
| self.clients.claim(); | ||
| }); |
There was a problem hiding this comment.
When the service worker first activates, waitUntil receives an async function object rather than a promise, so that function is never invoked and clients.claim() does not run. The activation event can also throw a TypeError, leaving already-open pages uncontrolled until their next navigation; pass the promise returned by self.clients.claim() directly to waitUntil.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
This PR is Phase 1 only: bring the Webex Connect SDK into the monorepo as a private package, with the SDK source copied unmodified.
| @@ -0,0 +1,57 @@ | |||
| (async function include_sdk_source_files() { | |||
| let appConfig = await fetch("app.config.json").then(response => response.json()); | |||
| let env = appConfig["sourceCodeType"] || IMISourceCodeType.Source; | |||
There was a problem hiding this comment.
When app.config.json omits or empties sourceCodeType, the intended default path evaluates IMISourceCodeType.Source while the later const IMISourceCodeType is still in its temporal dead zone. This throws a ReferenceError and prevents the SDK scripts from loading, so the enum must be initialized before this fallback expression.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
This PR is Phase 1 only: bring the Webex Connect SDK into the monorepo as a private package, with the SDK source copied unmodified.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d30b1a393e
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| var sw = navigator.serviceWorker.controller; | ||
| if (sw) { | ||
| sw.postMessage(msg); | ||
| return; |
There was a problem hiding this comment.
When an integration page is already controlled by another service worker, navigator.serviceWorker.controller refers to that host worker, not the Webex Connect worker registered under root + 'sw/' + appId + '/'. This branch posts REGISTER_APP_CLIENT to the unrelated worker and returns without using the scoped registration, leaving the SDK worker's client map empty; its filtered clients.matchAll() then excludes the page from message delivery and notification-click routing. Resolve the registration by scope instead of preferring the page controller.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
This PR is Phase 1 only: bring the Webex Connect SDK into the monorepo as a private package, with the SDK source copied unmodified.
| event.waitUntil(Promise.all([clickReadDeliveryFetchPromise, clientsPromise]) | ||
| .then(async ([resp, clientList]) => { |
There was a problem hiding this comment.
When the tracking endpoint is unreachable, such as while the user is offline, this Promise.all rejects before the callback that focuses an existing client or opens the notification URL runs. The notification is already closed, so clicking it appears to do nothing; navigation should proceed even if the best-effort read-receipt request fails.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
This PR is Phase 1 only: bring the Webex Connect SDK into the monorepo as a private package, with the SDK source copied unmodified.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 460a54323b
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| if (!messaging) { | ||
|
|
There was a problem hiding this comment.
When the token endpoint returns the standard expires_in lifetime (for example, 3600 seconds), storing it directly as expiresAt makes _isTokenValid() compare the current Unix timestamp against 3600, so every newly minted token is immediately considered expired. In token-exchange mode _getAjaxHeader() consequently omits Authorization, including from the registration request; compute the expiry as the current Unix time plus expires_in.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
This PR is Phase 1 only: bring the Webex Connect SDK into the monorepo as a private package, with the SDK source copied unmodified.
| if (self.appId) { | ||
| swScope = root + 'sw/' + self.appId + '/'; |
There was a problem hiding this comment.
When the integration page is served at the configured root, as in the included sample, the child scope root + 'sw/' + appId + '/' does not contain that page. Service-worker fetch events are selected by the requesting client's URL rather than the request URL, so the fetch listener in sw/sw.js never sees the page's /deliveryupdate?rr=... requests and its duplicate read-receipt suppression is inactive; use a scope that covers the integration page or move that interception out of this worker.
Useful? React with 👍 / 👎.
| setRoot: function (folder) { | ||
| self.root = folder; |
There was a problem hiding this comment.
When a caller invokes setRoot() after constructing ICConfig, self resolves to the browser global rather than the constructor-local variable, so this writes window.root while getRoot() continues returning the old instance value. Service-worker URLs and scopes therefore still use the previous root; assign through this here and in the sibling setAssetPath() and setShouldRequestNotificationPermission() setters.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
This PR is Phase 1 only: bring the Webex Connect SDK into the monorepo as a private package, with the SDK source copied unmodified.
| (async function include_sdk_source_files() { | ||
| let appConfig = await fetch("app.config.json").then(response => response.json()); |
There was a problem hiding this comment.
When app.config.json is slower than the page's other resources, this async IIFE yields at fetch() and allows parsing and the window load event to finish before the SDK script is even appended. api-demo.html starts IMI.IMIconnect from its load handler without listening for IMISdkReady, so that timing produces IMI is not defined and the demo never starts; expose/await the loader promise or make every startup path wait for the ready event.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
This PR is Phase 1 only bring the Webex Connect SDK into the monorepo as a private package, with the SDK source copied unmodified.
riag23
changed the title
riag23
changed the title
…repo Add a private @webex/webexconnect package with the canonical SDK source, sample app, and build workflow. Sample credentials are placeholders, and bundled min.js/source maps are not committed. Co-authored-by: Cursor <cursoragent@cursor.com>
460a543 to
df44ba9
Compare
github-actions
Bot
removed
the
validated
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: df44ba942c
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| "test": "yarn test:style && yarn test:unit", | ||
| "test:style": "eslint 'src/**/*.js'", | ||
| "test:unit": "jest" |
There was a problem hiding this comment.
On a clean checkout, running the documented yarn workspace @webex/webexconnect run test command reaches Jest without first creating dist/webex-connect-sdk.min.js; dist/ is gitignored, and smoke.test.js immediately reads that file, so the test command fails with ENOENT unless the developer happens to have built previously. Add a pre-test build step or make the test generate the bundle it evaluates.
Useful? React with 👍 / 👎.
| module.exports = { | ||
| rootDir: '.', | ||
| testEnvironment: 'node', | ||
| testMatch: ['<rootDir>/test/**/*.test.js'], |
There was a problem hiding this comment.
The added UA-routing suite lives at docs/samples/webexconnect/tests/push-routing.test.js, but this Jest pattern only discovers files under packages/webexconnect/test/. The PR unit workflow invokes this workspace's test:unit script, and a repo-wide search finds no node --test invocation or other reference to the routing test, so all seven routing cases are silently skipped in CI; add an explicit command for this Node test or move it into a discovered suite.
Useful? React with 👍 / 👎.
2 participants
COMPLETES <CAI-8541>
This pull request addresses
The Webex Connect JavaScript SDK (
IMIClient,aes,mqttws31, service worker)previously lived in the separate
wxconnect-js-sdkrepository, and the Webex Connectsample app under
docs/samples/webexconnect/vendored its own drifted copy of thatsource. There was no single canonical copy of the SDK in this monorepo, no shared
build/test pipeline for it, and no wiring to surface the sample from the docs index.
by making the following changes
@webex/webexconnect(packages/webexconnect/): the SDKsource (
src/IMIClient.js,src/aes.js,src/mqttws31.js,sw/sw.js) moved inbyte-for-byte unmodified as the single canonical copy. Marked
"private": trueso it is never published to npm.
webpack.config.js(production, source-mapped) that emitsdist/webex-connect-sdk.min.jsanddist/sw.min.js, preserving the reserved globalnames (
IMI,CryptoJS, MQTT/Paho, etc.) and injecting theJS_SDK_VERSIONbanner.scripts/sync-sample.jsruns afterbuild:srcand copiesthe four build artifacts into
docs/samples/webexconnect/, so the sample now consumesthe canonical build instead of its own vendored SDK source. Added the Webex Connect
sample and linked it from
docs/index.htmlanddocs/samples/index.html..github/workflows/webexconnect-deploy.yml): builds, tests, andassembles the SDK bundles + source maps as artifacts on
workflow_dispatch/ tag push.It intentionally never triggers on push to
nextand the publish step isdeliberately omitted (left as a TODO) pending security/legal review and sign-off.
.github/workflows/deploy.yml): added an explicitsedfilter so
@webex/webexconnectcan never leak into the deploy/publish matrix even ifprivate: truewere ever accidentally removed../packages/webexconnectin the root workspace list.Change Type
The following scenarios were tested
yarn workspace @webex/webexconnect run build:srcproducesdist/webex-connect-sdk.min.js,dist/sw.min.js, and their source maps, and syncsthem into
docs/samples/webexconnect/.yarn workspace @webex/webexconnect run test— ESLint stylecheck plus the Jest smoke test, which evaluates the built bundle and asserts the
IMIglobal and core public constructors (
ICConfig,ICMessage,ICDeviceProfile,IMIconnect) are exposed.docs/samples/webexconnect/in minified mode against the freshlybuilt canonical bundle and verified registration/connection and messaging flows.
src/**/*.js) is byte-for-byte identical to theoriginals and that the private package is excluded from the deploy matrix.
The GAI Coding Policy And Copyright Annotation Best Practices
I certified that