feat: Register definition issue checkers for plugin schema validation#834
Merged
kingston merged 14 commits intoMar 19, 2026
Merged
Conversation
🦋 Changeset detectedLatest commit: 1d14322 The changes in this PR will be included in the next version bump. This PR includes changesets to release 23 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 |
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, 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 have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (2)
📒 Files selected for processing (1)
📝 WalkthroughWalkthroughAdds global plugin-level schema issue checkers (auth, better-auth, rate-limit, storage) that detect model drift and offer fix actions. Exposes previewIssueFix in project-builder-lib to compute fix diffs. Updates web UI to preview diffs and apply fixes. Reorganizes example auth/users features and adjusts import paths and project definitions. Changes
Sequence DiagramsequenceDiagram
actor PluginInit as Plugin Initialization
participant IC as IssueCheckers
participant SC as SchemaChecker
participant PD as ProjectDefinition/Container
participant Diff as Diff Engine
participant UI as Web UI
PluginInit->>IC: receive issueCheckers dependency
PluginInit->>IC: register checker<br/>issueCheckers.checkers.set(pluginKey, checker)
SC->>PD: load plugin config & plugin definition
SC->>Diff: diff(current definition, partial definition)
alt diff found
Diff-->>SC: diff result (changed labels)
SC->>IC: emit warning issue with fix action
else no diff
Diff-->>SC: no changes
end
UI->>IC: list issues
UI->>SC: previewIssueFix(issue, container)
SC->>PD: apply fix to draft & serialize before/after
SC->>Diff: diff(serialized before, serialized after)
Diff-->>UI: return DefinitionDiff
UI->>UI: display expandable diff
UI->>IC: apply fix action
IC-->>PD: applyMergedDefinition -> update project definition
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
🚥 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)
📝 Coding Plan
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 |
Deploying baseplate-storybook with
|
| Latest commit: |
994e60b
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://a783e085.baseplate-storybook.pages.dev |
| Branch Preview URL: | https://kingston-eng-1074-show-plugi.baseplate-storybook.pages.dev |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
plugins/plugin-auth/src/local-auth/core/components/local-auth-definition-editor.tsx (1)
160-161:⚠️ Potential issue | 🟡 MinorFix typographical errors in environment variable names.
The description references
INTIIAL_USER_EMAILandINTIIAL_USER_PASSWORDwhich should beINITIAL_USER_EMAILandINITIAL_USER_PASSWORD.📝 Proposed fix
add a seed script to create the initial user provided a - .seed.env exists with the INTIIAL_USER_EMAIL and - INTIIAL_USER_PASSWORD variables. + .seed.env exists with the INITIAL_USER_EMAIL and + INITIAL_USER_PASSWORD variables.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugins/plugin-auth/src/local-auth/core/components/local-auth-definition-editor.tsx` around lines 160 - 161, In the LocalAuthDefinitionEditor component text that currently mentions ".seed.env exists with the INTIIAL_USER_EMAIL and INTIIAL_USER_PASSWORD variables", correct the typographical errors by replacing INTIIAL_USER_EMAIL with INITIAL_USER_EMAIL and INTIIAL_USER_PASSWORD with INITIAL_USER_PASSWORD so the displayed documentation strings in local-auth-definition-editor.tsx are accurate.
🧹 Nitpick comments (3)
packages/project-builder-web/src/components/definition-warning-dialog/definition-warning-dialog.tsx (1)
43-44: Consider resettingexpandedIndexwhen warnings change.When a fix is applied and
warningsarray updates (lines 61-64), theexpandedIndexmay reference a stale or invalid warning. This could cause visual glitches if a different warning now occupies that index.♻️ Proposed fix using useEffect
const [applyingFixIndex, setApplyingFixIndex] = useState<number | null>(null); const [expandedIndex, setExpandedIndex] = useState<number | null>(null); + +// Reset expanded index when warnings change +React.useEffect(() => { + setExpandedIndex(null); +}, [warnings]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/project-builder-web/src/components/definition-warning-dialog/definition-warning-dialog.tsx` around lines 43 - 44, The expandedIndex state can point to a stale index after the warnings array changes; add a useEffect that watches the warnings array (and optionally applyingFixIndex) and resets expandedIndex via setExpandedIndex(null) whenever warnings change (or when a fix completes) so the UI cannot reference an invalid warning; locate the expandedIndex/setExpandedIndex declarations and the component that updates warnings to add this effect.plugins/plugin-rate-limit/src/rate-limit/core/schema/schema-issue-checker.ts (1)
35-37: Silent catch may mask configuration issues.The empty
catchblock silently swallows exceptions fromgetFeaturePathById. While returning[]is safe, this could hide legitimate configuration errors (e.g., corrupted definition) that would be useful for debugging.Consider logging or at least distinguishing "feature not found" from unexpected errors if the utility supports it.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugins/plugin-rate-limit/src/rate-limit/core/schema/schema-issue-checker.ts` around lines 35 - 37, The empty catch in the block that calls getFeaturePathById silently swallows all errors; change it to catch the error (e) and distinguish a "not found" case from unexpected errors: if getFeaturePathById explicitly indicates missing feature return [] as before, otherwise log the unexpected error (include feature id and error message) via the module logger (or rethrow if appropriate) before returning []; update the try/catch around getFeaturePathById in schema-issue-checker.ts to capture and report the error instead of ignoring it.packages/ui-components/src/components/ui/button/button.tsx (1)
30-33: Use the exportedButtonPropsinterface instead of duplicating the type inline.The function parameter type at lines 30-33 duplicates the
ButtonPropsinterface defined at lines 9-10. SinceButtonPrimitive.Propsalready includes therenderprop, the explicitrenderproperty in the inline type is redundant.♻️ Proposed fix
function Button({ className, variant, size, justify, ...props -}: ButtonPrimitive.Props & - VariantProps<typeof buttonVariants> & { - render?: ButtonPrimitive.Props['render']; - }): React.ReactElement { +}: ButtonProps): React.ReactElement {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/ui-components/src/components/ui/button/button.tsx` around lines 30 - 33, Replace the inline union type on the Button component parameter with the exported ButtonProps interface: use ButtonProps instead of "ButtonPrimitive.Props & VariantProps<typeof buttonVariants> & { render?: ButtonPrimitive.Props['render'] }". Remove the redundant explicit render declaration since ButtonPrimitive.Props already includes it; update the function signature to accept (props: ButtonProps) and keep using buttonVariants and ButtonPrimitive where referenced.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In
`@plugins/plugin-auth/src/local-auth/core/components/local-auth-definition-editor.tsx`:
- Around line 160-161: In the LocalAuthDefinitionEditor component text that
currently mentions ".seed.env exists with the INTIIAL_USER_EMAIL and
INTIIAL_USER_PASSWORD variables", correct the typographical errors by replacing
INTIIAL_USER_EMAIL with INITIAL_USER_EMAIL and INTIIAL_USER_PASSWORD with
INITIAL_USER_PASSWORD so the displayed documentation strings in
local-auth-definition-editor.tsx are accurate.
---
Nitpick comments:
In
`@packages/project-builder-web/src/components/definition-warning-dialog/definition-warning-dialog.tsx`:
- Around line 43-44: The expandedIndex state can point to a stale index after
the warnings array changes; add a useEffect that watches the warnings array (and
optionally applyingFixIndex) and resets expandedIndex via setExpandedIndex(null)
whenever warnings change (or when a fix completes) so the UI cannot reference an
invalid warning; locate the expandedIndex/setExpandedIndex declarations and the
component that updates warnings to add this effect.
In `@packages/ui-components/src/components/ui/button/button.tsx`:
- Around line 30-33: Replace the inline union type on the Button component
parameter with the exported ButtonProps interface: use ButtonProps instead of
"ButtonPrimitive.Props & VariantProps<typeof buttonVariants> & { render?:
ButtonPrimitive.Props['render'] }". Remove the redundant explicit render
declaration since ButtonPrimitive.Props already includes it; update the function
signature to accept (props: ButtonProps) and keep using buttonVariants and
ButtonPrimitive where referenced.
In
`@plugins/plugin-rate-limit/src/rate-limit/core/schema/schema-issue-checker.ts`:
- Around line 35-37: The empty catch in the block that calls getFeaturePathById
silently swallows all errors; change it to catch the error (e) and distinguish a
"not found" case from unexpected errors: if getFeaturePathById explicitly
indicates missing feature return [] as before, otherwise log the unexpected
error (include feature id and error message) via the module logger (or rethrow
if appropriate) before returning []; update the try/catch around
getFeaturePathById in schema-issue-checker.ts to capture and report the error
instead of ignoring it.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: a357ecc8-c5ed-46da-99f5-a6271c09e652
⛔ Files ignored due to path filters (9)
examples/todo-with-better-auth/apps/backend/baseplate/generated/src/modules/accounts/auth/authorizers/user.authorizer.tsis excluded by!**/generated/**,!**/generated/**examples/todo-with-better-auth/apps/backend/baseplate/generated/src/modules/accounts/auth/authorizers/user.query-filter.tsis excluded by!**/generated/**,!**/generated/**examples/todo-with-better-auth/apps/backend/baseplate/generated/src/modules/accounts/auth/index.tsis excluded by!**/generated/**,!**/generated/**examples/todo-with-better-auth/apps/backend/baseplate/generated/src/modules/accounts/auth/schema/user-role.object-type.tsis excluded by!**/generated/**,!**/generated/**examples/todo-with-better-auth/apps/backend/baseplate/generated/src/modules/accounts/auth/schema/user.mutations.tsis excluded by!**/generated/**,!**/generated/**examples/todo-with-better-auth/apps/backend/baseplate/generated/src/modules/accounts/auth/schema/user.object-type.tsis excluded by!**/generated/**,!**/generated/**examples/todo-with-better-auth/apps/backend/baseplate/generated/src/modules/accounts/auth/schema/user.queries.tsis excluded by!**/generated/**,!**/generated/**examples/todo-with-better-auth/apps/backend/baseplate/generated/src/modules/accounts/auth/services/user.data-service.tsis excluded by!**/generated/**,!**/generated/**examples/todo-with-better-auth/apps/backend/baseplate/generated/src/modules/accounts/users/index.tsis excluded by!**/generated/**,!**/generated/**
📒 Files selected for processing (28)
.changeset/plugin-schema-issue-checkers.mdexamples/todo-with-better-auth/apps/backend/baseplate/file-id-map.jsonexamples/todo-with-better-auth/apps/backend/src/modules/accounts/auth/authorizers/user.authorizer.tsexamples/todo-with-better-auth/apps/backend/src/modules/accounts/auth/authorizers/user.query-filter.tsexamples/todo-with-better-auth/apps/backend/src/modules/accounts/auth/index.tsexamples/todo-with-better-auth/apps/backend/src/modules/accounts/auth/schema/user-role.object-type.tsexamples/todo-with-better-auth/apps/backend/src/modules/accounts/auth/schema/user.mutations.tsexamples/todo-with-better-auth/apps/backend/src/modules/accounts/auth/schema/user.object-type.tsexamples/todo-with-better-auth/apps/backend/src/modules/accounts/auth/schema/user.queries.tsexamples/todo-with-better-auth/apps/backend/src/modules/accounts/auth/services/user.data-service.tsexamples/todo-with-better-auth/apps/backend/src/modules/accounts/users/index.tsexamples/todo-with-better-auth/baseplate/project-definition.jsonpackages/project-builder-lib/src/parser/definition-issue-utils.tspackages/project-builder-lib/src/parser/index.tspackages/project-builder-lib/src/schema/models/models.tspackages/project-builder-lib/src/web/components/definition-diff-alert.tsxpackages/project-builder-web/src/app/project-sync-modal/project-sync-modal.tsxpackages/project-builder-web/src/components/definition-warning-dialog/definition-warning-dialog.tsxpackages/ui-components/src/components/ui/button/button.tsxplugins/plugin-auth/src/better-auth/core/common.tsplugins/plugin-auth/src/better-auth/core/schema/schema-issue-checker.tsplugins/plugin-auth/src/local-auth/core/common.tsplugins/plugin-auth/src/local-auth/core/components/local-auth-definition-editor.tsxplugins/plugin-auth/src/local-auth/core/schema/schema-issue-checker.tsplugins/plugin-rate-limit/src/rate-limit/core/common.tsplugins/plugin-rate-limit/src/rate-limit/core/schema/schema-issue-checker.tsplugins/plugin-storage/src/storage/core/common.tsplugins/plugin-storage/src/storage/core/schema/schema-issue-checker.ts
💤 Files with no reviewable changes (1)
- examples/todo-with-better-auth/apps/backend/src/modules/accounts/users/index.ts
kingston
deleted the
kingston/eng-1074-show-plugin-schema-merger-errors-in-issues
branch
Merged
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
1 participant
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 by CodeRabbit
New Features
Bug Fixes
Chores