-
Notifications
You must be signed in to change notification settings - Fork 13k
fix: slash command list is incomplete during startup #38267
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
🦋 Changeset detectedLatest commit: ac11d41 The changes in this PR will be included in the next version bump. This PR includes changesets to release 40 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
WalkthroughThe PR addresses a race condition where clients receive incomplete slash command lists during rolling workspace updates. It adds an Changes
Sequence DiagramsequenceDiagram
participant Client as Client Hook
participant API as Commands.list API
participant Apps as Apps System
rect rgba(100, 150, 200, 0.5)
Note over Client,Apps: Initial Request (Apps Still Loading)
Client->>API: GET /v1/commands.list
API->>Apps: Check if loaded
Apps-->>API: Not loaded
API-->>Client: HTTP 202, appsLoaded: false, empty list
end
rect rgba(150, 150, 100, 0.5)
Note over Client,Apps: Retry with Exponential Backoff
Client->>Client: Throw error to trigger retry
Client->>Client: Wait (randomized backoff)
end
rect rgba(100, 200, 150, 0.5)
Note over Client,Apps: Retry Request (Apps Loaded)
Client->>API: GET /v1/commands.list (retry)
API->>Apps: Check if loaded
Apps-->>API: Loaded
API-->>Client: HTTP 200, appsLoaded: true, commands list
Client->>Client: Process complete commands
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Looks like this PR is ready to merge! 🎉 |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #38267 +/- ##
===========================================
+ Coverage 70.71% 70.73% +0.02%
===========================================
Files 3142 3142
Lines 108925 108932 +7
Branches 19577 19612 +35
===========================================
+ Hits 77022 77054 +32
+ Misses 29902 29879 -23
+ Partials 2001 1999 -2
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
2efc62a to
685851b
Compare
5a9a3ab to
ac11d41
Compare
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No issues found across 5 files
Proposed changes (including videos or screenshots)
Fixes an issue where the slashcommand list may remain stale during a rolling restart in multi instances environments (microservices or HA).
This affects especially multi instances environments because the web client might start a websocket connection to one instance and try to fetch the list from another instance where apps are not loaded yet. This causes a problem because the server that has the websocket connection will not emit an
apps/command.addedevent that prompts the client to make a new request to get new slashcommands.One situation that I couldn't cover is: if the slash command list is refreshed while the user has the popup box open over the composer, then the items rendered will only be updated if the user interacts with the composer (a couple of keystrokes, sending a message, changing rooms, page reload)
Issue(s)
ARCH-1921
Steps to test or reproduce
Further comments
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.