docs(ai-docs): add docs for meetings widget#642
docs(ai-docs): add docs for meetings widget#642Riteshfyi wants to merge 8 commits intowebex:nextfrom
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ede33494ce
ℹ️ 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".
|
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: 3b4fdafe3d
ℹ️ 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".
| All control `action()` methods take a **destructured object**, not a plain string. | ||
|
|
There was a problem hiding this comment.
Document control action signatures as positional args
This section says every meeting control action() takes a destructured object, but the bundled @webex/sdk-component-adapter implementation used by this package defines positional signatures (for example JoinControl.action(meetingID) and SwitchCameraControl.action(meetingID, cameraID)). Following this guidance will lead maintainers to pass the wrong argument shape and can drop join/device parameters when wiring controls, so the documented call contract should match the actual adapter API.
Useful? React with 👍 / 👎.
akulakum
left a comment
There was a problem hiding this comment.
Thorough Review of ai-docs for Meetings Widget
I've cross-referenced both files (AGENTS.md and ARCHITECTURE.md) against:
- The actual meetings widget source code (
WebexMeetings.jsx,index.js) - The
@webex/sdk-component-adapterimplementation (control action signatures) - The
@webex/componentshooks (useMeetingControl) - The established ai-docs conventions from cc-widgets packages (
station-login,cc-widgets,store,cc-components, etc.)
Overall the documentation is well-written, comprehensive, and clearly structured with excellent Mermaid diagrams and thorough API coverage. However, there are several factual inaccuracies and convention gaps that need addressing.
Summary of Findings
| Category | Count |
|---|---|
| Critical (must fix) | 3 |
| Structural/Convention (should fix) | 6 |
| Content Accuracy (should fix) | 5 |
| Minor (nice to fix) | 6 |
See inline comments for details on each issue.
Structural/Convention Issues (not attached inline)
1. Missing "AI Agent Routing Instructions" section (AGENTS.md)
All existing cc-widget ai-docs for individual widgets include an "AI Agent Routing Instructions" section at the top that directs AI agents to start from the root AGENTS.md. This is missing. Follow the convention from station-login/ai-docs/AGENTS.md.
2. Missing "Installation" section (AGENTS.md)
The cc-widgets ai-docs include an explicit Installation section with yarn add / npm install commands. Should add:
## Installation
yarn add @webex/widgets
3. Missing _Last Updated: YYYY-MM-DD_ footer (both files)
All cc-widgets ai-docs end with _Last Updated: 2025-11-26_. Both files are missing this.
4. Missing **Version:** See [package.json](../package.json) line (AGENTS.md)
The cc-widgets convention includes a version reference in the Overview section.
5. Missing "File Structure" section (ARCHITECTURE.md)
CC-widgets ARCHITECTURE.md files include a directory tree showing the package structure. Should add the packages/@webex/widgets/ tree.
6. Missing "Related Documentation" links to patterns (ARCHITECTURE.md)
CC-widgets ARCHITECTURE.md files include links to relevant patterns files. Currently only links back to its own AGENTS.md.
Minor Issues (not attached inline)
1. Missing @webex/component-adapter-interfaces in Runtime Dependencies table — listed in package.json but not in the doc.
2. Missing prop-types and webex in Peer Dependencies table — package.json lists both as peer deps.
3. Component Table "Source" column points to external repos — Should clarify these are @webex/components package paths, not paths within this repo.
4. Missing documentation of the accessibility workaround code — WebexMeetings.jsx has ~140 lines of accessibility code (componentDidMount) with MutationObserver, focus management, and arrow key navigation. This is significant widget-specific behavior worth mentioning.
5. Missing "Testing" section (ARCHITECTURE.md) — The widget has E2E tests (tests/WebexMeeting.e2e.js) using WebdriverIO but no unit tests. CC-widgets ARCHITECTURE.md files document testing.
6. Widget is a class component — not called out — Unlike cc-widgets (functional components with hooks), this uses a class component with lifecycle methods. This architectural distinction should be noted.
|
|
||
| ### Control Action Parameters | ||
|
|
||
| All control `action()` methods take a **destructured object**, not a plain string. |
There was a problem hiding this comment.
CRITICAL: Control action signatures are WRONG
This entire section is factually incorrect. The statement:
All control
action()methods take a destructured object, not a plain string.
...is the opposite of reality. I verified in the actual @webex/sdk-component-adapter source code — all control action() methods take positional arguments, NOT destructured objects:
| Control | Actual Signature |
|---|---|
AudioControl |
action(meetingID) |
VideoControl |
action(meetingID) |
ShareControl |
async action(meetingID) |
JoinControl |
async action(meetingID) |
ExitControl |
async action(meetingID) |
SwitchCameraControl |
async action(meetingID, cameraID) |
SwitchMicrophoneControl |
async action(meetingID, microphoneID) |
SwitchSpeakerControl |
async action(meetingID, speakerID) |
RosterControl |
async action(meetingID) |
SettingsControl |
action(meetingID) |
Additionally, the useMeetingControl hook in @webex/components calls control.action(meetingID, value) with positional arguments.
The Codex bot also flagged this same issue. This table and the assertion must be corrected to show positional args.
| #### 4. Screen Sharing | ||
|
|
||
| The share button triggers the browser's native screen picker. The SDK handles `getDisplayMedia()` and negotiates the share stream with the backend. | ||
|
|
There was a problem hiding this comment.
Incorrect: SwitchCameraControl.action({ meetingID, cameraId })
This should be SwitchCameraControl.action(meetingID, cameraId) — positional arguments, not a destructured object. The actual adapter implementation uses async action(meetingID, cameraID) with positional params.
Same correction needed for SettingsControl.action() mentioned above.
|
|
||
|
|
||
| --- | ||
|
|
There was a problem hiding this comment.
Missing Peer Dependencies
The package.json lists prop-types: ^15.7.2 and webex: 2.60.2 as peer dependencies, but they're missing from this table. Should add:
| Package | Purpose |
|---|---|
prop-types |
React prop type checking |
webex |
Core Webex SDK (peer) |
Also missing @webex/component-adapter-interfaces from the Runtime Dependencies table above.
|
|
||
| ### Outbound (User Action → Backend) | ||
|
|
||
| ``` |
There was a problem hiding this comment.
Incorrect: Control.action({ meetingID })
The outbound data flow shows:
→ Control.action({ meetingID })
This should be:
→ Control.action(meetingID)
All control action() methods in @webex/sdk-component-adapter use positional arguments, not destructured objects. The useMeetingControl hook calls control.action(meetingID, value) with positional params.
| This is the real shape emitted by `adapter.meetingsAdapter.getMeeting(ID)`: | ||
|
|
||
| ``` | ||
| { |
There was a problem hiding this comment.
Missing 'ASKING' permission state
The localAudio.permission and localVideo.permission fields document:
permission: string | null // 'ALLOWED' | 'ERROR' | null
But the widget's actual render logic (WebexMeetings.jsx lines 188-191) critically depends on the 'ASKING' state:
if (audioPermission === 'ASKING') {
content = <WebexMediaAccess meetingID={meeting.ID} media="microphone" ... />;
} else if (videoPermission === 'ASKING') {
content = <WebexMediaAccess meetingID={meeting.ID} media="camera" ... />;
}Should be: // 'ALLOWED' | 'ASKING' | 'ERROR' | null
| participant Backend | ||
|
|
||
| User->>Component: Mount widget with accessToken | ||
| Component->>SDK: Webex.init({ credentials: { access_token } }) |
There was a problem hiding this comment.
Inaccurate: Webex.init()
The sequence diagram shows:
Component->>SDK: Webex.init({ credentials: { access_token } })
But the actual source code (WebexMeetings.jsx line 260) uses the constructor form:
const webex = new Webex({ credentials: { access_token: props.accessToken }, ... });Should be new Webex({ credentials: { access_token } }) — this also appears in the Three-Repository Architecture table in AGENTS.md (line 168) where it says Webex.init() under Key Exports Used.
| Note over User: User selects a different camera | ||
|
|
||
| User->>Component: Select new camera from dropdown | ||
| Component->>Adapter: SwitchCameraControl.action({ meetingID, cameraId }) |
There was a problem hiding this comment.
Incorrect: SwitchCameraControl.action({ meetingID, cameraId })
Same issue as in AGENTS.md — should use positional args:
Component->>Adapter: SwitchCameraControl.action(meetingID, cameraId)
Not:
Component->>Adapter: SwitchCameraControl.action({ meetingID, cameraId })
|
|
||
| - React strict mode causing double initialization | ||
| - Missing cleanup on prop changes | ||
| - Missing dependency array in useEffect |
There was a problem hiding this comment.
Troubleshooting #5 references useEffect but widget is a class component
This troubleshooting item mentions:
- "Missing dependency array in useEffect"
- "Implement proper cleanup in useEffect return"
But WebexMeetingsWidget is a class component — it uses componentDidMount and componentWillUnmount, not hooks. These references to useEffect would confuse developers. Either:
- Rewrite to reference class lifecycle methods, or
- Clarify this applies to consumer/wrapper code, not the widget itself.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1a32332f38
ℹ️ 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".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5baf0e3727
ℹ️ 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".
|
|
||
| Do **not** use this file as your main entry point for reasoning or code generation. | ||
|
|
||
| - **First step:** Locate and begin with the closest parent `AGENTS.md` file in the `ai-docs` directory structure. For example, the root `ai-docs/AGENTS.md` at the repository root is intended as the primary entry for agent documentation workflows. |
There was a problem hiding this comment.
Point routing instructions to existing root AGENTS.md
The first-step guidance tells agents to start from ai-docs/AGENTS.md at repository root, but this commit’s tree only contains a root-level AGENTS.md (no ai-docs/AGENTS.md). Following this instruction sends automation to a non-existent file and skips the actual top-level guidance, so the path needs to be corrected to the real root AGENTS location.
Useful? React with 👍 / 👎.
COMPLETES #https://jira-eng-gpk2.cisco.com/jira/browse/SPARK-783362
This pull request addresses
adds ai-docs for meetings widget repo
by making the following changes
created AGENTS.md & ARCHITECTURE.md
Change Type
The following scenarios were tested
< ENUMERATE TESTS PERFORMED, WHETHER MANUAL OR AUTOMATED >
The GAI Coding Policy And Copyright Annotation Best Practices
Checklist before merging
Make sure to have followed the contributing guidelines before submitting.