[cursor]

Summary

On upgrade, the webapp clears the plugin's components and loads the new bundle. But initialize awaits getConnected before registering anything. An initialize that is still waiting on that request when the next bundle loads (overlapping upgrade events on a slow connection) resumes and registers its components on top of the new ones. That is how the duplicate GitHub buttons end up in the team sidebar. The plugin also implemented only the deprecated deinitialize, which the webapp skips when one bundle replaces another (it calls uninitialize there), so nothing cleaned up.

Changes in webapp/src/index.js:

• Record the ID of every component registered in initialize, and unregister them all in a new uninitialize. This replaces deinitialize and still removes the click listener. initialize also calls it first, so re-initializing replaces components instead of adding more.
• Add a generation counter, so an initialize that was uninitialized or superseded while getConnected was in flight returns before registering anything.

Websocket handlers and the reconnect handler are keyed by plugin ID, so they can't duplicate; they are unchanged.

Servers from 12.0 on also drop registrations from a replaced bundle script, so this mostly shows up on older servers like the one in the report. The plugin-side guard covers both.

Testing

• New unit tests in webapp/src/index.test.js cover unregistering on uninitialize, skipping registration when uninitialized or re-initialized mid-connect, and replacing components on re-initialize. All four fail against the old code.
• Reproduced on Mattermost 11.4 with the plugin at the revision current when the ticket was filed: three back-to-back plugin upgrades with 1.5s of emulated browser latency left 2 GitHub team-sidebar buttons. The same steps with this fix applied left 1, across 2 runs.
• On Mattermost 12.1, with this branch and a connected user, one button group remains after upgrades and after disabling and re-enabling the plugin. The Playwright connect, sidebar and smoke specs pass (11 tests).
• npm run lint, npm run check-types and the full jest suite (37 tests) pass.

Before	After

Ticket Link

https://mattermost.atlassian.net/browse/MM-70322

  

Change Impact: Medium 🟡

Changes Breakdown:

+32 -12 Production Changes (24.9%)
+133 -0 Test Changes (75.1%)

Regression Risk: The change alters the plugin lifecycle contract by renaming deinitialize to uninitialize, so cleanup may not run on hosts that still call only deinitialize. The change is limited to webapp/src/index.js, and the new tests cover uninitialize during a pending getConnected, overlapping initialization, and reinitialization.

Generated by CodeRabbitAI

[mattermost-build]

Hello @cursor[bot],

Thanks for your pull request! A Core Committer will review your pull request soon. For code contributions, you can learn more about the review process here.

[coderabbitai]

No actionable comments were generated in the recent review.

Walkthrough

The plugin now tracks registered component IDs and uses initialization generations to prevent stale connection requests from registering components. Uninitialization invalidates pending initialization, removes the click listener, and unregisters tracked components.

Changes: Plugin component lifecycle

Layer / File(s)	Summary
Guard initialization and track registrations webapp/src/index.js	Initialization uninitializes prior state, checks whether its generation changed while getConnected was pending, and tracks component registration IDs.
Uninitialize and validate lifecycle webapp/src/index.js, webapp/src/index.test.js	uninitialize invalidates pending initialization, removes the click listener, unregisters tracked components, and clears their IDs. Tests cover cleanup and overlapping initialization.

Estimated code review effort: 2 (Simple) | ~12 minutes

Sequence Diagram(s)

sequenceDiagram
  participant PluginClass
  participant getConnected
  participant Registry
  PluginClass->>PluginClass: uninitialize
  PluginClass->>getConnected: request connection
  getConnected-->>PluginClass: return registry
  PluginClass->>PluginClass: check initialization generation
  PluginClass->>Registry: register components if generation is unchanged
  PluginClass->>Registry: unregister tracked components on uninitialize

Loading

Merge Risk: Minimal · up to 626c5

The change prevents duplicate GitHub sidebar components when the plugin is re-initialized by cleaning up registered components and skipping stale initialization. No actionable merge-blocking risk was identified.

Pre-merge checks: 4

Check name	Status	Explanation
Description Check		Check skipped - CodeRabbit’s high-level summary is enabled.
Title check		The title names the primary change: preventing duplicate GitHub sidebar components when the plugin is re-initialized. The raw summary confirms the changes target this. Initialization generations, trac…
Linked Issues check		Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check		Check skipped because no linked issues were found for this pull request.

Full details: Title check

Explanation

The title names the primary change: preventing duplicate GitHub sidebar components when the plugin is re-initialized. The raw summary confirms the changes target this. Initialization generations, tracked component IDs, and unregistration on uninitialize all address duplicate registration on re-initialization. The title is specific and concise. It uses the ticket ID in brackets, which is common in this repository. It does not mention the rename from deinitialize to uninitialize, but that detail is secondary.

Finishing Touches

Generate unit tests (beta)

• Commit to this branch
• Create a new PR

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Recent review info

Run configuration

• Source mode: Auto
• Repository: Unavailable
• Branch: Unavailable
• File: No configuration file from this repository applied
• Commit: Unavailable
• Layers: Organization UI
• Review profile: CHILL
• Plan: Essentials
• Run ID: 610b4cdf-5881-47af-afdc-c30ccb589ba8

Commits

Reviewing files that changed from the base of the PR and between bcbfe13 and 626c5bf.

Files selected for processing: 2

• webapp/src/index.js
• webapp/src/index.test.js

Included review availability: This review used your included allowance. Your plan provides up to 5 included reviews per hour; 1 remain after this review.

[nang2049]

LGTM

[mattermost-code]

@marianunez - checking in - we've reached the required number of approvals here - if this is good to merge, please click the button!