fix(core): union slack trigger catalog sources#130
Conversation
|
Warning Review limit reached
More reviews will be available in 51 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 (4)
✨ 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 updates the trigger catalog generator to merge webhook mapping events with supported events instead of letting the mapping events override them. Consequently, the generated catalog files have been updated with additional Slack events, and the test suite has been adjusted to reflect these changes. A review comment suggests using supportedEvents.provider instead of adapterPackage.provider when processing mapping events to prevent potential provider name mismatches and ensure consistency.
| entries.push({ | ||
| provider: supportedEvents.provider, | ||
| events: supportedEvents.events, | ||
| provider: adapterPackage.provider, | ||
| events: mappingEvents.events, | ||
| }); | ||
| sources.push({ | ||
| packageName: adapterPackage.packageName, | ||
| packagePath: adapterPackage.relativePath, | ||
| provider: supportedEvents.provider, | ||
| source: "supportedEvents", | ||
| provider: adapterPackage.provider, | ||
| source: "mapping.webhooks", | ||
| }); |
There was a problem hiding this comment.
Using adapterPackage.provider here can lead to a mismatch if the adapter's canonical provider name (defined by supportedEvents.provider, which resolves to instance.name) differs from the package/directory-derived slug (adapterPackage.provider). To ensure consistency and prevent duplicate or split entries in the generated catalog, we should use supportedEvents.provider instead.
| entries.push({ | |
| provider: supportedEvents.provider, | |
| events: supportedEvents.events, | |
| provider: adapterPackage.provider, | |
| events: mappingEvents.events, | |
| }); | |
| sources.push({ | |
| packageName: adapterPackage.packageName, | |
| packagePath: adapterPackage.relativePath, | |
| provider: supportedEvents.provider, | |
| source: "supportedEvents", | |
| provider: adapterPackage.provider, | |
| source: "mapping.webhooks", | |
| }); | |
| entries.push({ | |
| provider: supportedEvents.provider, | |
| events: mappingEvents.events, | |
| }); | |
| sources.push({ | |
| packageName: adapterPackage.packageName, | |
| packagePath: adapterPackage.relativePath, | |
| provider: supportedEvents.provider, | |
| source: "mapping.webhooks", | |
| }); |
khaliqgant
force-pushed
the
codex/fix-slack-trigger-catalog-union
branch
from
3930303 to
824b173
Compare
|
Addressed Gemini's consistency note: when core mapping webhook events are merged after supportedEvents(), the generator now uses the supportedEvents provider name if one was resolved, falling back to the package provider only when supportedEvents is absent. Re-ran:\n\n- node --import tsx packages/core/src/cli.ts triggers generate\n- node --import tsx packages/core/src/cli.ts triggers check\n- node --import tsx --test packages/core/tests/triggers/catalog-generator.test.ts\n- git diff --check |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
1 issue found across 4 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="packages/core/src/triggers/catalog-generator.ts">
<violation number="1" location="packages/core/src/triggers/catalog-generator.ts:82">
P2: Use the same canonical provider key when merging mapping events with supported events; using `adapterPackage.provider` here can split a single adapter’s triggers into separate catalog entries if it differs from the adapter’s runtime provider name.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| entries.push({ | ||
| provider: supportedEvents.provider, | ||
| events: supportedEvents.events, | ||
| provider: adapterPackage.provider, |
There was a problem hiding this comment.
P2: Use the same canonical provider key when merging mapping events with supported events; using adapterPackage.provider here can split a single adapter’s triggers into separate catalog entries if it differs from the adapter’s runtime provider name.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/core/src/triggers/catalog-generator.ts, line 82:
<comment>Use the same canonical provider key when merging mapping events with supported events; using `adapterPackage.provider` here can split a single adapter’s triggers into separate catalog entries if it differs from the adapter’s runtime provider name.</comment>
<file context>
@@ -57,35 +57,41 @@ export async function generateTriggerCatalog(repoRoot?: string): Promise<Trigger
entries.push({
- provider: supportedEvents.provider,
- events: supportedEvents.events,
+ provider: adapterPackage.provider,
+ events: mappingEvents.events,
});
</file context>
| provider: adapterPackage.provider, | |
| provider: supportedEvents.provider, |
|
Cubic's provider-key finding was on the pre-amend commit (). It is addressed in current head : core mapping events now use when supportedEvents resolved a runtime provider name, and fall back to only when supportedEvents is absent. CI is green on the amended head. |
1 participant
Summary
messageandreaction_addedVerification
npm run build --workspace @relayfile/adapter-corenode --import tsx packages/core/src/cli.ts triggers generatenode --import tsx packages/core/src/cli.ts triggers checknode --import tsx --test packages/core/tests/triggers/catalog-generator.test.tsnpm test --workspace @relayfile/adapter-core -- --test-name-pattern='trigger catalog|provider-specific'git diff --checkFixes #127