Add integration name tracking for collection collision detection#274
Open
Fryuni wants to merge 1 commit into
Open
Add integration name tracking for collection collision detection#274Fryuni wants to merge 1 commit into
Fryuni wants to merge 1 commit into
Conversation
…ting with Track integration names alongside collection entrypoints so error messages identify which integration injected a conflicting collection. Also detect inter-integration collisions when two different integrations inject the same collection key. Closes #97 https://claude.ai/code/session_016xmmzrXmsoxx2d1EJuVAhK
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
WalkthroughThis PR enhances integration collision detection for content collection injection. It tracks integration names alongside collection entrypoints, detects and reports collisions with specific owner information, and improves error messaging to indicate which integrations are conflicting when multiple integrations attempt to inject the same collection keys. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 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 |
@inox-tools/aik-mod
@inox-tools/aik-route-config
@inox-tools/astro-tests
@inox-tools/astro-when
@inox-tools/content-utils
@inox-tools/custom-routing
@inox-tools/cut-short
@inox-tools/inline-mod
@inox-tools/modular-station
@inox-tools/portal-gun
@inox-tools/request-nanostores
@inox-tools/request-state
@inox-tools/runtime-logger
@inox-tools/server-islands
@inox-tools/sitemap-ext
@inox-tools/star-warp
@inox-tools/utils
@inox-tools/velox-luna
commit: |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/content-utils/src/integration/injectorPlugin.ts`:
- Around line 49-52: The generated import injects raw entrypoint strings into
single-quoted code, which can break or allow injection when entrypoint contains
quotes/escapes; in injectorPlugin.ts where you build import lines (the loop over
entrypoints and the lines.push that creates `import {collections as
__collections${i}} from '...';`), replace the raw specifier insertion with an
escaped specifier using JSON.stringify(entrypoints[i].entrypoint) so the module
specifier is properly quoted/escaped before being concatenated into the
generated code.
ℹ️ Review info
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
AGENTS.mdpackages/content-utils/src/integration/index.tspackages/content-utils/src/integration/injectorPlugin.tspackages/content-utils/src/integration/state.tspackages/content-utils/src/integration/utilities.tspackages/content-utils/src/runtime/injector.tspackages/content-utils/tests/injector-runtime.test.tspackages/content-utils/virtual.d.ts
Comment on lines
+49
to
+52
| for (let i = 0; i < entrypoints.length; i++) { | ||
| lines.push( | ||
| `import {collections as __collections${i}} from '${entrypoints[i].entrypoint}';` | ||
| ); |
There was a problem hiding this comment.
Escape entrypoint specifiers in generated import statements.
Line 51 injects a raw module specifier into single-quoted code. If the entrypoint contains quotes or escapes,
the generated module can break (or be injection-prone). Use JSON.stringify(...) for the specifier.
🔧 Proposed fix
- lines.push(
- `import {collections as __collections${i}} from '${entrypoints[i].entrypoint}';`
- );
+ lines.push(
+ `import { collections as __collections${i} } from ${JSON.stringify(entrypoints[i].entrypoint)};`
+ );📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| for (let i = 0; i < entrypoints.length; i++) { | |
| lines.push( | |
| `import {collections as __collections${i}} from '${entrypoints[i].entrypoint}';` | |
| ); | |
| for (let i = 0; i < entrypoints.length; i++) { | |
| lines.push( | |
| `import { collections as __collections${i} } from ${JSON.stringify(entrypoints[i].entrypoint)};` | |
| ); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/content-utils/src/integration/injectorPlugin.ts` around lines 49 -
52, The generated import injects raw entrypoint strings into single-quoted code,
which can break or allow injection when entrypoint contains quotes/escapes; in
injectorPlugin.ts where you build import lines (the loop over entrypoints and
the lines.push that creates `import {collections as __collections${i}} from
'...';`), replace the raw specifier insertion with an escaped specifier using
JSON.stringify(entrypoints[i].entrypoint) so the module specifier is properly
quoted/escaped before being concatenated into the generated code.
Comment on lines
+69
to
+83
| lines.push(' if (__key in injectedCollections) {'); | ||
| lines.push(' const __prevOwner = collectionSources[__key];'); | ||
| lines.push(` const __curOwner = ${nameExpr};`); | ||
| lines.push( | ||
| ' if (__prevOwner !== undefined && __curOwner !== undefined && __prevOwner !== __curOwner) {' | ||
| ); | ||
| lines.push( | ||
| ' throw new Error(`Content collection "${__key}" is injected by both "${__prevOwner}" and "${__curOwner}". Only one integration may inject a given collection key.`);' | ||
| ); | ||
| lines.push(' }'); | ||
| lines.push(' }'); | ||
| lines.push(' injectedCollections[__key] = __value;'); | ||
| lines.push( | ||
| ` if (${nameExpr} !== undefined) collectionSources[__key] = ${nameExpr};` | ||
| ); |
There was a problem hiding this comment.
Collision detection misses conflicts when one integration name is unavailable.
At Line 73, duplicates only throw when both owners are defined and different.
That allows unnamed-vs-named collisions to silently overwrite injectedCollections (Line 80), defeating early collision detection and potentially leaving stale owner attribution.
🛠️ Proposed fix (owner-id based collision check)
lines.push('');
lines.push('export const collectionSources = {};');
lines.push('export const injectedCollections = {};');
+ lines.push('const __collectionOwners = {};');
@@
lines.push(
`for (const [__key, __value] of Object.entries(__collections${i})) {`
);
lines.push(' if (__key in injectedCollections) {');
lines.push(' const __prevOwner = collectionSources[__key];');
lines.push(` const __curOwner = ${nameExpr};`);
- lines.push(
- ' if (__prevOwner !== undefined && __curOwner !== undefined && __prevOwner !== __curOwner) {'
- );
+ lines.push(' const __prevOwnerId = __collectionOwners[__key];');
+ lines.push(` const __curOwnerId = ${JSON.stringify(entrypoints[i].entrypoint)};`);
+ lines.push(' if (__prevOwnerId !== __curOwnerId) {');
+ lines.push(
+ ' const __prevLabel = __prevOwner !== undefined ? `"${__prevOwner}"` : "an integration";'
+ );
+ lines.push(
+ ' const __curLabel = __curOwner !== undefined ? `"${__curOwner}"` : "an integration";'
+ );
lines.push(
- ' throw new Error(`Content collection "${__key}" is injected by both "${__prevOwner}" and "${__curOwner}". Only one integration may inject a given collection key.`);'
+ ' throw new Error(`Content collection "${__key}" is injected by both ${__prevLabel} and ${__curLabel}. Only one integration may inject a given collection key.`);'
);
lines.push(' }');
lines.push(' }');
lines.push(' injectedCollections[__key] = __value;');
+ lines.push(` __collectionOwners[__key] = ${JSON.stringify(entrypoints[i].entrypoint)};`);
lines.push(
` if (${nameExpr} !== undefined) collectionSources[__key] = ${nameExpr};`
);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| lines.push(' if (__key in injectedCollections) {'); | |
| lines.push(' const __prevOwner = collectionSources[__key];'); | |
| lines.push(` const __curOwner = ${nameExpr};`); | |
| lines.push( | |
| ' if (__prevOwner !== undefined && __curOwner !== undefined && __prevOwner !== __curOwner) {' | |
| ); | |
| lines.push( | |
| ' throw new Error(`Content collection "${__key}" is injected by both "${__prevOwner}" and "${__curOwner}". Only one integration may inject a given collection key.`);' | |
| ); | |
| lines.push(' }'); | |
| lines.push(' }'); | |
| lines.push(' injectedCollections[__key] = __value;'); | |
| lines.push( | |
| ` if (${nameExpr} !== undefined) collectionSources[__key] = ${nameExpr};` | |
| ); | |
| lines.push(' if (__key in injectedCollections) {'); | |
| lines.push(' const __prevOwner = collectionSources[__key];'); | |
| lines.push(` const __curOwner = ${nameExpr};`); | |
| lines.push(' const __prevOwnerId = __collectionOwners[__key];'); | |
| lines.push(` const __curOwnerId = ${JSON.stringify(entrypoints[i].entrypoint)};`); | |
| lines.push(' if (__prevOwnerId !== __curOwnerId) {'); | |
| lines.push( | |
| ' const __prevLabel = __prevOwner !== undefined ? `"${__prevOwner}"` : "an integration";' | |
| ); | |
| lines.push( | |
| ' const __curLabel = __curOwner !== undefined ? `"${__curOwner}"` : "an integration";' | |
| ); | |
| lines.push( | |
| ' throw new Error(`Content collection "${__key}" is injected by both ${__prevLabel} and ${__curLabel}. Only one integration may inject a given collection key.`);' | |
| ); | |
| lines.push(' }'); | |
| lines.push(' }'); | |
| lines.push(' injectedCollections[__key] = __value;'); | |
| lines.push(` __collectionOwners[__key] = ${JSON.stringify(entrypoints[i].entrypoint)};`); | |
| lines.push( | |
| ` if (${nameExpr} !== undefined) collectionSources[__key] = ${nameExpr};` | |
| ); |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
This PR enhances collection injection error reporting by tracking which integration injected each collection, enabling more informative error messages when collections collide or are overridden.
Key Changes
Virtual module generation: Refactored the injector plugin to generate runtime code that tracks collection sources alongside the collections themselves. The generated code now:
collectionSourcesmap to record which integration owns each collectionRuntime error reporting: Updated
injectCollections()to use the tracked integration names in error messages:Type system updates:
InjectedCollectionEntryinterface to track both entrypoint and integration nameintegrationNameoptional field toInjectCollectionOptionscollectionSourcesfrom the virtual injector moduleUtility enhancement: The
injectCollections()utility now automatically populatesintegrationNamefrom the integration's logger label if not explicitly providedTest coverage: Added tests verifying:
Implementation Details
The collision detection happens at two levels:
injectCollections()is called, with proper attribution to the original integrationThis approach ensures early detection of conflicts while maintaining clear error messages that help developers identify the source of the problem.
https://claude.ai/code/session_016xmmzrXmsoxx2d1EJuVAhK
Summary by CodeRabbit
Improvements