fix(integrations): redeliver Slack content after blind events#163
Conversation
|
Your free trial PR review limit of 300 PRs has been reached. Please upgrade your plan to continue using CodeAnt AI. |
|
Warning Review limit reached
More reviews will be available in 30 minutes and 47 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
✨ 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 |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
No issues found across 2 files
You’re at about 93% of the monthly reviewed-line limit. You may want to disable incremental reviews to conserve quota. Reviews will continue until that limit is exceeded. If you need help avoiding interruptions, please contact contact@cubic.dev.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request modifies the deduplication logic in IntegrationEventBridge to allow the first content-bearing replay of a blind claim to go through, and adds a corresponding integration test. The reviewer identified a critical lifecycle bug where the blind event's success or failure could prematurely commit or accidentally release the provisional content hashes of in-flight replays, potentially causing permanent suppression or duplicate deliveries. To resolve this, the reviewer recommends decoupling the blind event's lifecycle from the content hashes in commitDedupeKey and releaseDedupeKey.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| if (provisional) existing.provisionalContentHashes.add(contentHash) | ||
| else existing.committedContentHashes.add(contentHash) | ||
| existing.expiresAt = now + ttlMs | ||
| return { claimed: false } | ||
| return { claimed: true, contentHash } |
There was a problem hiding this comment.
Critical Deduplication Lifecycle Bug
While letting the first content-bearing replay through (claimed: true) is correct, it introduces a critical bug in how commitDedupeKey and releaseDedupeKey manage the lifecycle of provisionalContentHashes and committedContentHashes.
In the previous implementation, content-bearing replays were suppressed/dropped immediately, so their hashes were "learned" but they never had independent delivery lifecycles. Now that they are actually delivered, they have their own independent success/failure lifecycles.
Because of this, the blind event's commit/release lifecycle should no longer touch the content hashes:
- Premature Commit Bug: If the blind event succeeds,
commitDedupeKeycurrently moves allprovisionalContentHashestocommittedContentHashes. If an in-flight content-bearing replay subsequently fails,releaseDedupeKeywon't be able to release its hash because it has already been committed. This results in the message being permanently suppressed and unable to be retried. - Accidental Release Bug: If the blind event fails,
releaseDedupeKeycurrently clearsprovisionalContentHashes. This wipes out the hashes of any in-flight content-bearing replays, meaning they will never be committed even if they succeed, allowing duplicate replays to bypass deduplication.
Recommended Fix
To resolve this, update commitDedupeKey and releaseDedupeKey (located further down in this file) so that the blind event's lifecycle only manages the blind flags and does not touch the content hashes:
// In commitDedupeKey (around line 3026):
} else if (entry.provisionalBlind) {
entry.provisionalBlind = false
entry.committedBlind = true
// Remove the loop moving provisionalContentHashes to committedContentHashes
}// In releaseDedupeKey (around line 3070):
} else {
entry.provisionalBlind = false
// Remove entry.provisionalContentHashes.clear()
}
|
Reviewed PR #163 against Addressed comments
Validation run:
I did not verify GitHub CI status or mergeability from GitHub, per the “don’t use git or gh CLI” instruction. |
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
|
Your free trial PR review limit of 300 PRs has been reached. Please upgrade your plan to continue using CodeAnt AI. |
|
Addressed the Gemini lifecycle finding in What changed:
Regression coverage added:
These cover the two failure classes Gemini flagged:
Validation:
Devin note:
|
|
ℹ️ pr-reviewer: review only — no file changes were applied to the PR (nothing to commit after review). The notes below are advisory and were not pushed. Reviewed PR #163 against the diff and current checkout. I did not make edits: Gemini’s actionable finding is already fixed in the current code, and the regression tests are present. Validation run: GitHub API status at final poll: Addressed comments
|
1 participant
Summary
Message: unavailabledeliveryValidation
node --experimental-strip-types --no-warnings --test src/main/__tests__/integration-event-bridge.test.ts(71/71 pass in source worktree)node --experimental-strip-types --no-warnings --test --test-name-pattern 'slack blind thread-reply delivery does not suppress a later content-bearing replay|slack context does not inject sparse relayfile pointer fallback as message content|slack unchanged-content replay is suppressed after injected delivery commits' src/main/__tests__/integration-event-bridge.test.ts(3/3 pass in source worktree)\n\n## Notes\n- This is the Pear bridge follow-on to the relayfile mount read-not-ready retry fix.\n- No cloud-side change is required for this incident.