Repository navigation
Conversation
|
This pull request is automatically being deployed by Amplify Hosting (learn more). |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![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: 46833d8cab
ℹ️ 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".
github-actions
Bot
removed
the
validated
akulakum
left a comment
akulakum
left a comment
There was a problem hiding this comment.
Two inline comments covering published-content verification and the consumer fixture's supported peer version.
| "dependencies": { | ||
| "@momentum-ui/core": "19.16.0", | ||
| "@momentum-ui/icons": "8.28.5", | ||
| "@momentum-ui/react-collaboration": "26.197.0", |
There was a problem hiding this comment.
[Nonblocking] Use a supported Momentum UI peer version in the fixture
The fixture pins @momentum-ui/react-collaboration to 26.197.0, but @webex/cc-station-login declares a minimum of 26.201.9. Because both install commands use --legacy-peer-deps, the checks run with this unsupported combination. Could we update the pin and lockfile to a version satisfying all tested packages’ peer requirements?
There was a problem hiding this comment.
Fixed in fbfb39d1. The fixture now pins @momentum-ui/react-collaboration to 26.201.9, and its npm lockfile was regenerated. I checked the fixture pins against every tested Contact Center package's direct peer requirements, including cc-station-login; all ranges are satisfied. The fixture manifest and resolved lockfile versions also match.
There was a problem hiding this comment.
The requested peer pin and lockfile update are present. The supported-peer fork matrix exposed an upstream CSS error: MeetingRowContent.style.scss emits ::part(icon) path, which Vite and Parcel reject. Webpack, Rollup, and esbuild passed.
I also tried published 26.214.0 locally; its nested Sass emits the same selector and both checks failed. Sampled intermediate versions contain the same block. The fixture keeps the minimum supported 26.201.9, and the CSS failures remain visible in the gate. I am leaving this thread open to track that dependency blocker. Full evidence and current reruns.
There was a problem hiding this comment.
The requested direction for this PR is to keep the Momentum version already used by Contact Center. Both the React sample and Web Component sample pin 26.197.0.
dec8f305 keeps that fixture pin and lockfile, enables all five bundlers, and adds a test that checks both sample pins and the lockfile. All five local production builds and Chromium startup checks passed. All five also passed in the current fork run; its reports match the same nine archives used locally.
This is an explicit exception to the requested peer-range alignment. Station Login still declares >=26.201.9; 21 of the 22 direct peer requirements are satisfied, with that single mismatch. The standing doc and PR description state the limit and the use of --legacy-peer-deps. Updating to 26.201.9 reproduced vendor CSS failures in Vite and Parcel, and 26.214.0 also failed both local checks. No published package peer range is changed here. I am leaving this nonblocking thread open for review of the version choice. Evidence.
github-actions
Bot
removed
the
validated
Fork test evidence and review fixesCurrent head: All five bundlers passed locally and in fork CI. The fork unit/lint run also passed. The fork test merge and PR head have the same Git tree.
All five CI reports and all five local results match the same nine candidate archive hashes and package versions. The downloaded archives passed SHA-256 and embedded identity checks. Build logs, results, and candidate manifest are available as Actions artifacts for seven days; the verified hashes are also recorded below. Contact Center UI versionThe fixture keeps Momentum UI This is below Station Login's declared Review fixes
Validation
Earlier dependency checksThe run with Momentum UI Verified candidate SHA-256 valuesAll nine packages use version
No packages were published. These checks cover the fixture and pinned versions, without an authenticated SDK session, media playback, or CSP validation. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: fbfb39d13f
ℹ️ 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".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0e33daae36
ℹ️ 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".
rsarika
left a comment
rsarika
left a comment
There was a problem hiding this comment.
Looks good to me. One minor follow-up below.
| env: {...process.env, NODE_OPTIONS: '--max-old-space-size=8192', PARCEL_WORKERS: '2'}, | ||
| stdio: ['ignore', log, log], | ||
| }); | ||
| execute('npm', ['ci', '--legacy-peer-deps', '--no-audit', '--no-fund']); |
There was a problem hiding this comment.
Minor follow-up: let's align the fixture with Station Login's declared Momentum peer range once the upstream CSS issue is fixed. With --legacy-peer-deps, the gate can pass without checking that dependency contract. The exception is documented, so I'm fine with handling this separately.
COMPLETES https://jira-eng-sjc12.cisco.com/jira/browse/CAI-8661
Adds the requested configurable consumer bundler release checks. Related regression fix: #751, now included through merges from
next. No separate issue was supplied.This pull request addresses
The fixture uses Momentum UI
26.197.0, matching both Contact Center sample apps, as requested. All five bundlers are enabled. This version is below Station Login's declared>=26.201.9peer minimum; that existing mismatch is documented rather than changing a package contract in this workflow PR. Fork evidence and current runs.A package can pass the repository's Webpack build and fail when a host app uses another bundler. We need consumer checks before publishing, with an opt out for each bundler.
This revision preserves the existing release tooling:
package-tools sync/increment,build:prod, and workspacedeploy:npm. The custom archive publisher was removed.by making the following changes
nextand for release checks.tooling/consumer-builds/config.jsonfile for both paths. Webpack, Vite, Parcel, Rollup, and esbuild are enabled by default. Set a bundler'senabledflag tofalseto omit its check. Invalid flags and disabling all five fail validation.deploy:npmcommand with the changed package selection and branch tag. A missing build cache or manifest patch conflict stops publishing.@webex/test-fixtures. Keep the existing version outputs, documentation, tags, and release comments.YARN_NPM_AUTH_TOKENonly in the deploy step. Preparation and consumer jobs have no publishing credentials.@momentum-ui/react-collaborationat the Contact Center samples'26.197.0. Add a regression test that checks the fixture and lockfile against both sample pins. Document the Station Login peer-range exception and the newer Momentum CSS failures.ai-docs/CONSUMER_BUILDS.mdand its spec index links.Change Type
The following scenarios were tested
The testing is done with the amplify link
Immutable Yarn install and development and production builds on the current PR branch.
76 consumer gate tests passed, within all 100 root tooling tests. Helper line coverage exceeds 90%. Tests cover bundler opt outs, invalid configuration, archive integrity and identity, build and browser failures, manifest patch restoration and conflicts, empty patches, release job dependencies, the existing deploy command, and deployment failure propagation.
A separate local fixture used real Yarn 4.5.1 with harmless workspace deploy scripts. Prepared versions, package selection, forwarded access/tag arguments,
test-fixturesexclusion, and failed deployment handling passed. No publish command ran.Yarn accepted the deploy step's auth environment setting with a dummy value.
actionlint, formatting, and the repository styles check. Styles reported zero errors and six existing warnings in the unchanged Web Component sample.All five current-head fork consumer builds and Chromium startup checks passed. All five local checks also passed. The ten result reports match the same nine candidate archive hashes.
Real Yarn 4.5.1 verification with all nine production archives passed for matching workspaces, failed with exit code 1 after modifying
cc-widgets/dist/index.js, and passed after restoration. Candidate archives stayed unchanged.On the final head, changing only
parcel.enabledtofalsepassed all 100 tooling tests and produced a four-bundler matrix. A direct Parcel command failed before installation while disabled. The five-bundler default config was restored.Ten shell tests execute the actual workflow configuration steps, including invalid flags, missing config, all-disabled config, and valid opt-outs.
Full pre-commit hook passed without a bypass: 1,915 passed test executions across 98 suite executions, with seven existing skips. The root script runs the meetings suite twice. Styles passed with zero errors and six existing warnings.
Staged secret-pattern scan and
git diff --check.dec8f305The full hook passed on the final commit. An earlier local attempt had a Jest worker
SIGSEGVin IncomingTask; its four tests passed on isolated retry, and the complete hook passed on retry without a bypass.The original baseline at
c298215cfailed the Vite check on the inline wellness loader. The branch includes #751 through its merges fromnext. The earlier fork run passed all five tools with the same Contact Center UI pin. Updating to26.201.9caused Vite and Parcel to reject vendor CSS; local26.214.0checks reproduced it. The fixture therefore keeps the Contact Center samples' version by request. Both install commands use--legacy-peer-deps; this evidence does not validate Station Login's higher declared minimum.The current head is
dec8f3058056edd7882de8b9b54a404158a85200. The consumer run passed all six jobs: package preparation and all five bundlers. The unit/lint run passed, including all 100 tooling tests and 1,915 passed test executions, with seven existing skips. The fork test merge and PR head have the same Git tree. All five CI reports and all five local results match the same nine archive hashes.These checks cover the fixture and pinned versions. They do not cover all consumer settings, development servers, media playback, CSP policies, or authenticated agent sessions.
Testing and configuration: Consumer build release checks.
Rollback: revert this workflow change. To omit one bundler check, change its config flag.
The GAI Coding Policy And Copyright Annotation Best Practices
Checklist before merging
Make sure to have followed the contributing guidelines before submitting.