Canonicalize Slack webhook paths in core - #257
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 19 minutes and 23 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 ignored due to path filters (1)
📒 Files selected for processing (10)
✨ 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 |
![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 implements Slack webhook envelope path canonicalization, ensuring provider-relative channel paths and /slack/... paths are treated equivalently, raw channel IDs are resolved to existing aliases, and out-of-provider paths are ignored. It also adds a comprehensive test suite for these changes. Feedback on the changes highlights a performance bottleneck in the Slack channel alias resolution logic, where calling storage.listFiles() and mapping/splitting every file path in large workspaces can cause significant CPU overhead; a more efficient prefix-filtering approach is suggested to resolve this.
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.
| const candidates = files | ||
| .map((file) => normalizePath(file.path).slice(1).split("/")) | ||
| .filter( | ||
| (fileParts) => | ||
| fileParts.length >= 3 && | ||
| fileParts[0] === "slack" && | ||
| fileParts[1] === "channels" && | ||
| fileParts[2]?.startsWith(`${channelSegment}__`), | ||
| ) | ||
| .map((fileParts) => fileParts[2] as string) | ||
| .sort(); |
There was a problem hiding this comment.
Performance Bottleneck in Channel Alias Resolution
Calling storage.listFiles() returns all files in the workspace. In large workspaces, this array can contain thousands of files.
The current implementation maps, slices, and splits the path of every single file in the workspace before filtering. This creates significant CPU overhead and garbage collection pressure on every webhook event application.
By filtering the normalized paths using startsWith first, we can avoid splitting and slicing the paths of unrelated files.
const prefix = `/slack/channels/${channelSegment}__`;
const candidates = files
.map((file) => normalizePath(file.path))
.filter((p) => p.startsWith(prefix))
.map((p) => p.slice(1).split("/")[2] as string)
.sort();
Relayfile Eval ReviewRun: Passed: 4 | Needs human: 0 | Reviewable: 0 | Missing output: 0 | Failed: 0 | Skipped: 0 Human Review CasesNo reviewable human-review cases captured Relayfile output. |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
3 issues found across 5 files
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
|
Your free trial PR review limit of 300 PRs has been reached. Please upgrade your plan to continue using CodeAnt AI. |
|
✅ pr-reviewer applied fixes — committed and pushed Fixed two issues in packages/core/src/webhooks.ts:
Local validation passed:
I did not print |
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
| if (normalizeProvider(provider) !== "slack") { | ||
| return path; | ||
| } | ||
| return canonicalizeSlackChannelAliasPath(storage.listFiles(), path); |
There was a problem hiding this comment.
🚩 storage.listFiles() called on every Slack envelope application
canonicalizeExistingProviderAliasPath at packages/core/src/webhooks.ts:687 calls storage.listFiles() to scan ALL files in the workspace every time applyWebhookEnvelope processes a Slack envelope. For large workspaces with thousands of files, this is O(n) per webhook. Then canonicalizeSlackChannelAliasPath at line 701-711 further splits and filters every file path. This is correct but potentially expensive. Consider adding a narrower query method to StorageAdapter (e.g., listFilesByPrefix(prefix)) or caching the channel alias map rather than scanning all files on every webhook.
Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
1 issue found across 6 files (changes from recent commits).
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
|
Your free trial PR review limit of 300 PRs has been reached. Please upgrade your plan to continue using CodeAnt AI. |
|
Addressed the remaining PR-review storage-scan concern in commit 1aa330c. Also checked the SDK failure reported in Validation run locally:
|
|
Your free trial PR review limit of 300 PRs has been reached. Please upgrade your plan to continue using CodeAnt AI. |
|
Follow-up for the failing Validation after the change:
|
|
Implemented two additional fixes for PR #257:
Validation passed locally:
I could not run local Go commands or the root |
There was a problem hiding this comment.
2 issues found across 3 files (changes from recent commits).
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
|
Your free trial PR review limit of 300 PRs has been reached. Please upgrade your plan to continue using CodeAnt AI. |
|
Addressed the follow-up dependency alignment note in e52dfc0: Validation after the dependency alignment:
|
|
Your free trial PR review limit of 300 PRs has been reached. Please upgrade your plan to continue using CodeAnt AI. |
|
Addressed the two latest issues identified by cubic in c5a3f28:
Validation after this follow-up:
|
|
✅ pr-reviewer applied fixes — committed and pushed Fixed the PR issue I found: packages/sdk/typescript/package.json now depends on Verified locally:
I installed Go 1.22.12 temporarily under |
1 participant
Summary
@relayfile/core/channels/C123/...to/slack/channels/C123/...channelId__namealiases before file writes/events@relayfile/coreto0.8.17for publishValidation
npm run test --workspace=packages/corenpm run build --workspace=packages/corenpm pack --workspace=packages/core --dry-rungit diff --checkPublish note
npm whoamiiskhaliqgant, andnpm access list collaborators @relayfile/core --jsonreportskhaliqgant: read-write, so I should be able to publish@relayfile/core@0.8.17after merge.