fix: Visitor "lead capture" failing when there was no email/phone already registered #36835
Conversation
|
Looks like this PR is ready to merge! 🎉 |
🦋 Changeset detectedLatest commit: 09f0bc7 The changes in this PR will be included in the next version bump. This PR includes changesets to release 41 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 |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #36835 +/- ##
===========================================
- Coverage 68.14% 67.06% -1.08%
===========================================
Files 3365 3418 +53
Lines 115815 117981 +2166
Branches 20958 21611 +653
===========================================
+ Hits 78918 79128 +210
- Misses 34204 36154 +1950
- Partials 2693 2699 +6
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull Request Overview
This PR fixes a bug where visitor "lead capture" functionality fails when capturing email/phone numbers from messages if the visitor doesn't already have email or phone data stored. The root cause was that MongoDB's $addToSet operator fails when the target field doesn't exist or isn't an array.
Key changes:
- Replace
$addToSetwith aggregation pipeline using$setUnionand$ifNullto handle missing fields - Add comprehensive test coverage for lead capture scenarios
- Improve regex handling with safe matching and deduplication
Reviewed Changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/models/src/models/LivechatVisitors.ts | Replace $addToSet with aggregation pipeline to handle missing email/phone arrays |
| apps/meteor/app/livechat/server/hooks/leadCapture.ts | Add safe regex matching and deduplication of captured data |
| apps/meteor/tests/end-to-end/api/livechat/09-visitors.ts | Add comprehensive test suite for lead capture functionality |
| apps/meteor/tests/data/livechat/rooms.ts | Add utility function to create visitors without email/phone data |
| .changeset/fair-dolls-trade.md | Document the bug fix in changeset |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
KevLehman
added
the
stat: QA assured
dionisio-bot
bot
added
the
stat: ready to merge
dionisio-bot
bot
removed
the
stat: ready to merge
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughAdds safe regex matching and deduplication in lead-capture, skips DB writes when no matches, fixes visitor merge logic to handle null email/phone fields, and introduces test helpers plus end-to-end tests validating email/phone extraction and related scenarios. Changes
Sequence Diagram(s)sequenceDiagram
participant Visitor
participant Room as Livechat Room
participant Hook as leadCapture Hook
participant SafeMatch as safeMatch
participant Model as LivechatVisitors.saveGuestEmailPhoneById
Visitor->>Room: Send message (may contain email/phone)
Room->>Hook: Trigger leadCapture callback
Hook->>SafeMatch: Run email regex (setting)
SafeMatch-->>Hook: matchedEmails (or empty)
Hook->>SafeMatch: Run phone regex (setting)
SafeMatch-->>Hook: matchedPhones (or empty)
alt matchedEmails or matchedPhones
Hook->>Model: saveGuestEmailPhoneById(matchedEmails, matchedPhones)
Model->>Model: Early-return if both empty (guard)
Model-->>Hook: Update visitor record (merge via $setUnion/$ifNull)
else no matches
Note over Hook: Skip DB update
end
Hook-->>Room: leadCapture complete
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (4 passed)
✨ 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 |
![coderabbitai[bot]](https://waps.l3s.uni-hannover.de/live/im_/https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/models/src/models/LivechatVisitors.ts (1)
387-410: Remove commented code block.This substantial commented block (24 lines) should be removed to improve code maintainability. Version control preserves this history if needed for reference.
Apply this diff to remove the commented code:
if (!saveEmail.length && !savePhone.length) { return Promise.resolve(); } - - // const updatePipeline: Document[] = []; - - // if (saveEmail.length) { - // updatePipeline.push({ - // $set: { - // visitorEmails: { - // $setUnion: [{ $ifNull: ['$visitorEmails', []] }, saveEmail], - // }, - // }, - // }); - // } - - // if (savePhone.length) { - // updatePipeline.push({ - // $set: { - // phone: { - // $setUnion: [{ $ifNull: ['$phone', []] }, savePhone], - // }, - // }, - // }); - // } - - // return this.updateOne({ _id }, updatePipeline); const update: UpdateFilter<ILivechatVisitor> = {
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (5)
.changeset/fair-dolls-trade.md(1 hunks)apps/meteor/app/livechat/server/hooks/leadCapture.ts(1 hunks)apps/meteor/tests/data/livechat/rooms.ts(1 hunks)apps/meteor/tests/end-to-end/api/livechat/09-visitors.ts(3 hunks)packages/models/src/models/LivechatVisitors.ts(1 hunks)
🧰 Additional context used
🧠 Learnings (11)
📚 Learning: 2025-09-16T22:08:51.490Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-09-16T22:08:51.490Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Use descriptive test names that clearly communicate expected behavior
Applied to files:
apps/meteor/tests/end-to-end/api/livechat/09-visitors.tsapps/meteor/tests/data/livechat/rooms.ts
📚 Learning: 2025-09-16T22:08:51.490Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-09-16T22:08:51.490Z
Learning: Applies to apps/meteor/tests/e2e/**/*.{ts,tsx,js,jsx} : Follow DRY by extracting reusable logic into helper functions or page objects
Applied to files:
apps/meteor/tests/end-to-end/api/livechat/09-visitors.ts
📚 Learning: 2025-09-16T22:08:51.490Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-09-16T22:08:51.490Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Maintain test isolation between test cases
Applied to files:
apps/meteor/tests/end-to-end/api/livechat/09-visitors.tsapps/meteor/tests/data/livechat/rooms.ts
📚 Learning: 2025-09-16T22:08:51.490Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-09-16T22:08:51.490Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Utilize Playwright fixtures (test, page, expect) consistently
Applied to files:
apps/meteor/tests/end-to-end/api/livechat/09-visitors.ts
📚 Learning: 2025-09-16T22:08:51.490Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-09-16T22:08:51.490Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Ensure tests run reliably in parallel without shared state conflicts
Applied to files:
apps/meteor/tests/end-to-end/api/livechat/09-visitors.ts
📚 Learning: 2025-09-16T22:08:51.490Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-09-16T22:08:51.490Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Prefer web-first assertions (e.g., toBeVisible, toHaveText)
Applied to files:
apps/meteor/tests/end-to-end/api/livechat/09-visitors.ts
📚 Learning: 2025-09-16T22:08:51.490Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-09-16T22:08:51.490Z
Learning: Applies to apps/meteor/tests/e2e/**/*.{ts,tsx} : Store commonly used locators in variables/constants for reuse
Applied to files:
apps/meteor/tests/end-to-end/api/livechat/09-visitors.ts
📚 Learning: 2025-09-16T22:08:51.490Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-09-16T22:08:51.490Z
Learning: Applies to apps/meteor/tests/e2e/**/*.{ts,tsx} : Implement proper wait strategies for dynamic content
Applied to files:
apps/meteor/tests/end-to-end/api/livechat/09-visitors.ts
📚 Learning: 2025-09-16T22:08:51.490Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-09-16T22:08:51.490Z
Learning: Applies to apps/meteor/tests/e2e/**/*.{ts,tsx} : Follow the Page Object Model pattern consistently
Applied to files:
apps/meteor/tests/end-to-end/api/livechat/09-visitors.ts
📚 Learning: 2025-09-16T22:08:51.490Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-09-16T22:08:51.490Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Use expect matchers (toEqual, toContain, toBeTruthy, toHaveLength, etc.) instead of assert statements
Applied to files:
apps/meteor/tests/end-to-end/api/livechat/09-visitors.ts
📚 Learning: 2025-10-28T16:53:42.761Z
Learnt from: ricardogarim
Repo: RocketChat/Rocket.Chat PR: 37205
File: ee/packages/federation-matrix/src/FederationMatrix.ts:296-301
Timestamp: 2025-10-28T16:53:42.761Z
Learning: In the Rocket.Chat federation-matrix integration (ee/packages/federation-matrix/), the createRoom method from rocket.chat/federation-sdk will support a 4-argument signature (userId, roomName, visibility, displayName) in newer versions. Code using this 4-argument call is forward-compatible with planned library updates and should not be flagged as an error.
Applied to files:
apps/meteor/tests/data/livechat/rooms.ts
🧬 Code graph analysis (3)
apps/meteor/app/livechat/server/hooks/leadCapture.ts (2)
packages/mongo-adapter/src/common.ts (1)
isTruthy(148-148)packages/models/src/index.ts (1)
LivechatVisitors(178-178)
apps/meteor/tests/end-to-end/api/livechat/09-visitors.ts (3)
packages/core-typings/src/ILivechatVisitor.ts (1)
ILivechatVisitor(28-55)apps/meteor/tests/data/livechat/rooms.ts (5)
createVisitorWithCustomData(58-105)createLivechatRoom(22-34)sendMessage(337-354)sendAgentMessage(372-392)createVisitor(107-141)apps/meteor/tests/data/api-data.ts (1)
credentials(39-42)
apps/meteor/tests/data/livechat/rooms.ts (3)
packages/core-typings/src/ILivechatVisitor.ts (1)
ILivechatVisitor(28-55)apps/meteor/tests/data/livechat/users.ts (1)
getRandomVisitorToken(28-28)apps/meteor/tests/data/api-data.ts (1)
credentials(39-42)
🪛 ast-grep (0.39.7)
apps/meteor/app/livechat/server/hooks/leadCapture.ts
[warning] 47-47: Regular expression constructed from variable input detected. This can lead to Regular Expression Denial of Service (ReDoS) attacks if the variable contains malicious patterns. Use libraries like 'recheck' to validate regex safety or use static patterns.
Context: new RegExp(pattern, flags)
Note: [CWE-1333] Inefficient Regular Expression Complexity [REFERENCES]
- https://owasp.org/www-community/attacks/Regular_expression_Denial_of_Service_-_ReDoS
- https://cwe.mitre.org/data/definitions/1333.html
(regexp-from-variable)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: 📦 Build Packages
- GitHub Check: CodeQL-Build
- GitHub Check: CodeQL-Build
🔇 Additional comments (10)
.changeset/fair-dolls-trade.md (1)
1-6: LGTM!The changeset correctly documents the fix with appropriate patch-level version bumps for both affected packages.
packages/models/src/models/LivechatVisitors.ts (1)
383-385: LGTM! Core fix for the lead capture issue.The early return when both email and phone arrays are empty prevents the MongoDB $addToSet operation from failing when the visitor record lacks these fields.
apps/meteor/app/livechat/server/hooks/leadCapture.ts (3)
43-53: Defensive regex handling prevents failures from invalid patterns.The
safeMatchhelper correctly handles invalid regex patterns by returning an empty array. The static analysis ReDoS warning can be safely disregarded here since regex patterns are admin-controlled via settings, not user input.However, consider adding error logging to aid debugging when admins configure invalid patterns, as noted in the past review comment.
55-58: LGTM! Clean deduplication and matching logic.The
uniqhelper combined withsafeMatchcorrectly extracts and deduplicates lead data from visitor messages.
60-63: Core fix: Conditional save prevents unnecessary operations.Only invoking
saveGuestEmailPhoneByIdwhen matches exist aligns with the model's early return logic and prevents MongoDB $addToSet failures on non-existent fields.apps/meteor/tests/data/livechat/rooms.ts (1)
58-105: LGTM! Well-designed test utility.The
createVisitorWithCustomDatafunction provides flexible visitor creation for test scenarios, properly handling visitor reuse and conditional field inclusion. The implementation is clean and consistent with existing patterns in the file.apps/meteor/tests/end-to-end/api/livechat/09-visitors.ts (4)
1441-1455: LGTM! Proper test setup for lead capture scenarios.The test suite correctly creates a visitor without email/phone data to validate the fix for lead capture when these fields are missing.
1457-1551: LGTM! Comprehensive test coverage for lead capture.The test cases thoroughly validate email and phone capture, including multiple matches, duplicate prevention, agent message filtering, and combined capture scenarios.
1553-1580: LGTM! Important edge case testing.This test block correctly validates that lead capture can be disabled via settings and properly restores the default regex patterns afterward.
1582-1607: LGTM! Critical test for existing visitor data.This test validates that lead capture correctly merges new data with existing email/phone arrays, ensuring the fix doesn't break the normal case when fields are already present.
dionisio-bot
bot
removed
the
stat: ready to merge
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
apps/meteor/tests/end-to-end/api/livechat/09-visitors.ts (1)
1487-1493: Minor: Typo in test descriptions.The test descriptions use "thats" which should be "that's" or "that is" for grammatical correctness.
- it('should not add an email thats already registered', async () => { + it('should not add an email that is already registered', async () => {- it('should not add a phone number thats already registered', async () => { + it('should not add a phone number that is already registered', async () => {Also applies to: 1523-1529
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
apps/meteor/tests/end-to-end/api/livechat/09-visitors.ts(3 hunks)
🧰 Additional context used
🧠 Learnings (10)
📚 Learning: 2025-09-16T22:08:51.490Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-09-16T22:08:51.490Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Use descriptive test names that clearly communicate expected behavior
Applied to files:
apps/meteor/tests/end-to-end/api/livechat/09-visitors.ts
📚 Learning: 2025-09-16T22:08:51.490Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-09-16T22:08:51.490Z
Learning: Applies to apps/meteor/tests/e2e/**/*.{ts,tsx,js,jsx} : Follow DRY by extracting reusable logic into helper functions or page objects
Applied to files:
apps/meteor/tests/end-to-end/api/livechat/09-visitors.ts
📚 Learning: 2025-09-16T22:08:51.490Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-09-16T22:08:51.490Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Utilize Playwright fixtures (test, page, expect) consistently
Applied to files:
apps/meteor/tests/end-to-end/api/livechat/09-visitors.ts
📚 Learning: 2025-09-16T22:08:51.490Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-09-16T22:08:51.490Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Maintain test isolation between test cases
Applied to files:
apps/meteor/tests/end-to-end/api/livechat/09-visitors.ts
📚 Learning: 2025-09-16T22:08:51.490Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-09-16T22:08:51.490Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Ensure tests run reliably in parallel without shared state conflicts
Applied to files:
apps/meteor/tests/end-to-end/api/livechat/09-visitors.ts
📚 Learning: 2025-09-16T22:08:51.490Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-09-16T22:08:51.490Z
Learning: Applies to apps/meteor/tests/e2e/**/*.{ts,tsx} : Implement proper wait strategies for dynamic content
Applied to files:
apps/meteor/tests/end-to-end/api/livechat/09-visitors.ts
📚 Learning: 2025-09-16T22:08:51.490Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-09-16T22:08:51.490Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Ensure a clean state for each test execution
Applied to files:
apps/meteor/tests/end-to-end/api/livechat/09-visitors.ts
📚 Learning: 2025-09-16T22:08:51.490Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-09-16T22:08:51.490Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Use test.beforeAll() and test.afterAll() for setup and teardown
Applied to files:
apps/meteor/tests/end-to-end/api/livechat/09-visitors.ts
📚 Learning: 2025-09-16T22:08:51.490Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-09-16T22:08:51.490Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Use test.step() to organize complex test scenarios
Applied to files:
apps/meteor/tests/end-to-end/api/livechat/09-visitors.ts
📚 Learning: 2025-09-16T22:08:51.490Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-09-16T22:08:51.490Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Use expect matchers (toEqual, toContain, toBeTruthy, toHaveLength, etc.) instead of assert statements
Applied to files:
apps/meteor/tests/end-to-end/api/livechat/09-visitors.ts
🧬 Code graph analysis (1)
apps/meteor/tests/end-to-end/api/livechat/09-visitors.ts (3)
packages/core-typings/src/ILivechatVisitor.ts (1)
ILivechatVisitor(28-55)apps/meteor/tests/data/livechat/rooms.ts (7)
createVisitorWithCustomData(58-105)createLivechatRoom(22-34)createAgent(247-261)closeOmnichannelRoom(417-423)sendMessage(337-354)sendAgentMessage(372-392)createVisitor(107-141)apps/meteor/tests/data/api-data.ts (1)
credentials(39-42)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: CodeQL-Build
- GitHub Check: CodeQL-Build
🔇 Additional comments (4)
apps/meteor/tests/end-to-end/api/livechat/09-visitors.ts (4)
16-18: LGTM! Imports support the new lead capture test scenarios.The imported functions are appropriately used throughout the new test block to create visitors without initial email/phone data and to send messages for capture testing.
1641-1666: Good test isolation in this nested block.This nested describe block properly creates its own visitor and room with existing email/phone data, ensuring it doesn't depend on state from other tests. The assertions (lengthOf 2) correctly reflect adding 1 new item to the 1 existing item. This demonstrates good test isolation practices.
Based on learnings.
1445-1666: Excellent edge case coverage for lead capture functionality.The test suite comprehensively covers:
- Visitors without initial email/phone (directly addresses PR objective)
- Multiple captures in single message
- Duplicate prevention
- Agent vs visitor message distinction
- Empty/broken regex configurations
- Merging with existing visitor data
This thorough coverage validates the fix for the MongoDB
$addToSetissue when fields are missing or not arrays.
1463-1463: Good use of optional chaining and defensive coding.The tests consistently use optional chaining (
?.) and fallback patterns (|| []) to safely access potentially missing fields. This prevents brittle tests and aligns with TypeScript best practices.Also applies to: 1472-1472, 1509-1509, 1518-1518, 1575-1575, 1578-1578, 1609-1609, 1632-1632
sampaiodiego
added
the
stat: ready to merge
dionisio-bot
bot
removed
the
stat: ready to merge
sampaiodiego
added
the
stat: ready to merge
…eady registered (#36835) Co-authored-by: Diego Sampaio <8591547+sampaiodiego@users.noreply.github.com>
|
/backport 7.10.4 |
…eady registered (#36835) Co-authored-by: Diego Sampaio <8591547+sampaiodiego@users.noreply.github.com>
|
/backport 7.11.1 |
|
/patch |
|
Pull request #37434 added to Project: "Patch 7.10.4" |
…eady registered (#36835) Co-authored-by: Diego Sampaio <8591547+sampaiodiego@users.noreply.github.com>
…eady registered (#36835) Co-authored-by: Diego Sampaio <8591547+sampaiodiego@users.noreply.github.com>
|
Pull request #37435 added to Project: "Patch 7.11.1" |
|
Pull request #37436 added to Project: "Patch 7.12.1" |
Proposed changes (including videos or screenshots)
Issue(s)
https://rocketchat.atlassian.net/browse/CTZ-313
Steps to test or reproduce
Further comments
Basically, $addToSet doesnt work when there's no array. So if the field you're trying to apply the action doens't exist or it's not an array, it will fail.
it should not happen anymore :)
Summary by CodeRabbit
Bug Fixes
Tests
Chores