feat: include featured extensions #912
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the 📝 WalkthroughWalkthroughThis PR introduces a "featured" boolean field for extensions across the stack—including database schema, backend types, service logic, and frontend types—along with new API key management endpoints. The featured field is propagated through extension listings with dedicated sorting to prioritize featured extensions, and new OpenAPI schemas document the API key management endpoints. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
api/internal/features/extension/service/service.go (1)
95-177: Critical: Inconsistent featured status handling in fork paths.When forking with YAML override (line 132),
Featuredis explicitly set tofalse. However, when forking without YAML override (lines 154-177), the fork inherits all fields from the parent viafork := *srcat line 104, including theFeaturedstatus. This creates inconsistent behavior where:
- Fork with override:
Featured = false✓- Fork without override:
Featured = <inherited from parent>✗Featured status should not be inherited by forks (similar to
IsVerifiedat line 131).🔧 Proposed fix
fork := *src fork.ID = uuid.UUID{} fork.ParentExtensionID = &src.ID fork.Name = src.Name + " (Fork)" // ensure unique extension_id for fork fork.ExtensionID = src.ExtensionID + "-fork-" + time.Now().Format("20060102150405") fork.CreatedAt = time.Now() fork.UpdatedAt = time.Now() fork.DeletedAt = nil + fork.IsVerified = false + fork.Featured = false if yamlOverride != "" {
🤖 Fix all issues with AI agents
In @api/migrations/extensions/043_add_featured_column_up.sql:
- Line 4: The migration currently creates an unused index
`idx_extensions_featured` on `extensions(featured)` which harms write
performance; remove the `CREATE INDEX IF NOT EXISTS idx_extensions_featured ON
extensions(featured);` statement from migration
`043_add_featured_column_up.sql`, or instead remove it here and add a new,
separate migration that creates `idx_extensions_featured` only when code
introduces queries that filter/sort by `featured`; ensure no other migration or
schema tool recreates it implicitly so writes aren’t penalized until it is
actually needed.
🧹 Nitpick comments (3)
api/doc/openapi.json (3)
1346-1587: API key schemas are consistent; consider clarifying one‑time key exposure and future paginationThe CreateAPIKeyRequest/CreateAPIKeyResponse and ListAPIKeysResponse shapes look consistent with existing patterns (timestamps, nested org/user,
prefixvs fullkey). The fact that only CreateAPIKeyResponse includeskeywhile ListAPIKeysResponse does not is a good security posture.Two follow‑ups you may want to consider:
- Add a short description (via struct tags in the Go types) that the
keyfield is only returned at creation time and must be stored client‑side, to avoid client misuse.- If you expect many API keys per org/user, think about a paginated ListAPIKeysResponse shape (page/page_size/total like other list endpoints) to keep responses bounded.
Also applies to: 3462-3685
2492-2494: Featured flag is cleanly propagated; ensure it’s always set server‑sideThe new
featured: booleanproperty is added consistently across all extension representations (single extension, executions, list, and nested extension objects). This matches the PR goal of surfacing featured extensions.To make client handling simpler and avoid tri‑state behavior, ensure the server always serializes
featured(defaulting tofalse) rather than omitting it; otherwise some generators will treat it as optional/undefined.Also applies to: 2668-2670, 2790-2792, 2959-2961, 4932-4934, 5115-5117
9385-9607: New API key endpoints look correct; polish summaries for readabilityThe new
/api/v1/auth/api-keysGET/POST and/api/v1/auth/api-keys/{id}DELETE paths are wired to the expected controllers and reuse the standard HTTPError/MessageResponse patterns, which is consistent with the rest of the API.Two small polish items:
- The summaries (
"list a p i keys","create a p i key","revoke a p i key") read a bit oddly; consider updating route metadata so they render as “List API keys”, “Create API key”, etc.- Optionally add a brief human‑readable description noting that these endpoints are authenticated and scoped to the current user/org, to set expectations for consumers.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
api/doc/openapi.jsonapi/internal/features/extension/service/service.goapi/internal/features/extension/storage/storage.goapi/internal/features/extension/tools/list_extensions.goapi/internal/features/extension/tools/types.goapi/internal/types/extension.goapi/migrations/extensions/043_add_featured_column_down.sqlapi/migrations/extensions/043_add_featured_column_up.sqlview/redux/types/extension.ts
🧰 Additional context used
📓 Path-based instructions (8)
api/**/*.go
📄 CodeRabbit inference engine (.cursor/rules/backend.mdc)
api/**/*.go: Search the codebase before writing new code for existing implementations ininternal/utils/,internal/types/, existing storage patterns, repository interfaces, validation logic, and middleware
Follow Single Responsibility Principle: Controllers handle HTTP request/response only, Services handle business logic and orchestration, Storage handles database operations only, Validation handles request validation only, Types define data structures and domain errors
Use early returns and flat code structure instead of nested conditions for improved code readability
Always log errors usingc.logger.Log(logger.Error, err.Error(), additionalContext)before returning fuego.HTTPError responses
Use fuego.HTTPError with Err and Status fields for API error responses, providing appropriate HTTP status codes (400 for validation, 401 for unauthorized, 404 for not found, 500 for server errors)
Success responses should use shared_types.Response struct with Status set to "success", appropriate Message, and Data containing the result
Use import aliases for clarity:shared_storagefor internal/storage,shared_typesfor internal/types, and standard imports before custom imports
Use common utilities:utils.GetUser(w, r)for authenticated user retrieval,utils.SendErrorResponse()andutils.SendJSONResponse()for non-Fuego handlers,c.logger.Log()for logging
Use lowercase single-word package names, noun/noun phrase names for interfaces and structs (e.g., DomainRepository, DomainController), verb/verb phrase names for functions (e.g., NewDomainController, GetItems), and Err prefix for error variables
Include only essential comments: function documentation with purpose and return behavior, explanations for complex logic. Avoid obvious comments, unused imports, commented-out code, and unused variables
Before committing code, verify: early returns used (no deep nesting), all errors logged before returning, transactions used for mutations, soft delete implemented, ...
Files:
api/internal/features/extension/tools/types.goapi/internal/types/extension.goapi/internal/features/extension/tools/list_extensions.goapi/internal/features/extension/storage/storage.goapi/internal/features/extension/service/service.go
api/internal/types/*.go
📄 CodeRabbit inference engine (.cursor/rules/backend.mdc)
All model structs should use
bun.BaseModelfor ORM integration, include UUID primary keys withbun:"id,pk,type:uuid", and include soft delete support with nullable DeletedAt timestamp field
Files:
api/internal/types/extension.go
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/frontend.mdc)
**/*.{ts,tsx}: Before writing any new logic, search the codebase for existing implementations inview/hooks/,view/lib/utils.ts,view/components/ui/, andview/redux/services/before creating custom elements
Extract repeated patterns into custom hooks or shared components
Use early returns and flat structure instead of nested conditions for improved code readability
Always use typed hooksuseAppDispatchanduseAppSelectorfrom@/redux/hooksinstead of untypeduseDispatchanduseSelector
Never useanytype; always use explicit types for function parameters and return values
Remove unused imports immediately and delete commented-out code
Keep files focused and minimal by removing unused variables and functions
In comments, explain WHY code works a certain way, not WHAT it does
Never disable lint rules without strong justification
Files:
view/redux/types/extension.ts
view/**/*.{tsx,ts}
📄 CodeRabbit inference engine (.cursor/rules/frontend.mdc)
view/**/*.{tsx,ts}: Each component file should export one main component
Never write plain HTML for interactive elements; always use shadcn components like Button, Card, Badge, Skeleton
Use thecn()utility function for conditional Tailwind classes
No hardcoded user-facing strings; always use i18n with theuseTranslationhook
Handle loading states by checkingisLoadingfrom RTK Query and returning skeleton loaders or loading components
Handle error states by checking error values from RTK Query and displaying error messages using i18n
Show empty state UI when data is empty using i18n messages
Always provide skeleton loaders for async content instead of blank/white space
Use semantic HTML elements for accessibility
Provide proper ARIA labels where needed for accessibility
Memoize expensive computations withuseMemo
Prevent unnecessary re-renders withuseCallback
Use React Query's caching effectively to avoid redundant API calls
Lazy load heavy components when appropriate
Ensure responsive design works on all screen sizes
Files:
view/redux/types/extension.ts
view/redux/types/**/*.ts
📄 CodeRabbit inference engine (.cursor/rules/frontend.mdc)
Place API response types in
view/redux/types/[domain].ts, component props types inline or co-located with component, and shared types in appropriatetypes/directories
Files:
view/redux/types/extension.ts
view/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/frontend.mdc)
view/**/*.{ts,tsx}: Respect ESLint rules configured inview/eslint.config.mjs
Use Prettier for consistent formatting and fix all lint errors before committing
Files:
view/redux/types/extension.ts
api/internal/features/*/storage/*.go
📄 CodeRabbit inference engine (.cursor/rules/backend.mdc)
api/internal/features/*/storage/*.go: Define a repository interface for storage operations to enable mocking in tests, with methods like CreateItem, GetItemByID, GetItemsByUserID, UpdateItem, DeleteItem
Use transactions with BeginTx, defer Rollback, and Commit for all mutation operations (CreateItem, UpdateItem, DeleteItem) to ensure atomicity
Implement soft deletes by setting deleted_at timestamp and excluding soft-deleted records in SELECT queries withWHERE ... AND deleted_at IS NULLcondition
Files:
api/internal/features/extension/storage/storage.go
api/internal/features/*/service/*.go
📄 CodeRabbit inference engine (.cursor/rules/backend.mdc)
api/internal/features/*/service/*.go: Service struct must include store (*shared_storage.Store), ctx context.Context, logger, and storage (repository interface) fields
Service methods should return empty slices instead of nil for list operations, and log errors usings.logger.Log(logger.Error, err.Error(), userID)before returning them
Files:
api/internal/features/extension/service/service.go
🧠 Learnings (5)
📚 Learning: 2025-12-27T03:02:06.464Z
Learnt from: CR
Repo: raghavyuva/nixopus PR: 0
File: .cursor/rules/backend.mdc:0-0
Timestamp: 2025-12-27T03:02:06.464Z
Learning: Applies to api/**/*.go : Follow Single Responsibility Principle: Controllers handle HTTP request/response only, Services handle business logic and orchestration, Storage handles database operations only, Validation handles request validation only, Types define data structures and domain errors
Applied to files:
api/doc/openapi.json
📚 Learning: 2025-12-27T03:02:06.464Z
Learnt from: CR
Repo: raghavyuva/nixopus PR: 0
File: .cursor/rules/backend.mdc:0-0
Timestamp: 2025-12-27T03:02:06.464Z
Learning: Applies to api/internal/features/*/controller/*.go : Controller struct must include fields for store (*shared_storage.Store), validator (*validation.Validator), service (*[Domain]Service), ctx context.Context, logger, and notification (*notification.NotificationManager)
Applied to files:
api/doc/openapi.json
📚 Learning: 2025-12-27T03:02:06.464Z
Learnt from: CR
Repo: raghavyuva/nixopus PR: 0
File: .cursor/rules/backend.mdc:0-0
Timestamp: 2025-12-27T03:02:06.464Z
Learning: Applies to api/internal/routes/*.go : Register domain routes using fuego.Get, fuego.Post, fuego.Put, fuego.Delete with appropriate HTTP methods and path parameters like `/{id}`
Applied to files:
api/doc/openapi.json
📚 Learning: 2025-12-27T03:02:06.464Z
Learnt from: CR
Repo: raghavyuva/nixopus PR: 0
File: .cursor/rules/backend.mdc:0-0
Timestamp: 2025-12-27T03:02:06.464Z
Learning: Applies to api/internal/features/*/controller/*.go : In all handlers, retrieve authenticated user via `utils.GetUser(w, r)` before processing requests and return `fuego.HTTPError{Status: http.StatusUnauthorized}` if user is nil
Applied to files:
api/doc/openapi.json
📚 Learning: 2025-12-27T03:02:06.464Z
Learnt from: CR
Repo: raghavyuva/nixopus PR: 0
File: .cursor/rules/backend.mdc:0-0
Timestamp: 2025-12-27T03:02:06.464Z
Learning: Applies to api/internal/features/*/types/*.go : Define domain-specific request types (CreateItemRequest, UpdateItemRequest) and domain-specific error variables prefixed with Err (ErrMissingName, ErrItemNotFound, ErrPermissionDenied)
Applied to files:
api/doc/openapi.json
🧬 Code graph analysis (1)
api/internal/features/extension/storage/storage.go (1)
api/internal/types/extension.go (1)
SortDirectionDesc(184-184)
🔇 Additional comments (7)
api/migrations/extensions/043_add_featured_column_up.sql (1)
1-2: LGTM!The column addition is correct with appropriate type, NOT NULL constraint, sensible default value, and idempotent IF NOT EXISTS clause.
api/migrations/extensions/043_add_featured_column_down.sql (1)
1-4: LGTM!The down migration correctly drops the index before the column with proper IF EXISTS clauses for idempotency. The order of operations is correct.
view/redux/types/extension.ts (1)
48-48: LGTM!The
featuredboolean field correctly aligns with the backend type (SQL BOOLEAN/Go bool) and follows the established naming conventions. The field is appropriately non-optional since the database column is NOT NULL.api/internal/features/extension/tools/list_extensions.go (1)
123-123: LGTM!The
Featuredfield mapping is correct and consistent with the other field mappings in theconvertToMCPExtensionfunction.api/internal/features/extension/tools/types.go (1)
37-37: LGTM! Featured field properly added.The
Featuredfield is correctly typed and tagged for JSON serialization. This change is backward compatible as JSON unmarshaling will default tofalsefor existing API consumers.api/internal/types/extension.go (1)
69-69: LGTM! Featured field properly defined with appropriate constraints.The
Featuredfield is correctly defined with:
notnullconstraint for data integritydefault:falseproviding a sensible default for existing extensions- Proper JSON serialization tag
This aligns with the database migration adding the column with a NOT NULL constraint and default value.
api/internal/features/extension/storage/storage.go (1)
174-180: The sorting logic is sound and performance is already optimized.The featured column is properly indexed via
idx_extensions_featuredin the database migration (api/migrations/extensions/043_add_featured_column_up.sql), so theORDER BY featured DESCapplied to all extension list queries will not cause performance degradation. The sorting logic correctly prioritizes featured extensions before applying the user's sort preference.
Issue
NIL
Description
Scope of Change
Select all applicable areas impacted by this PR:
Screenshot / Video / GIF (if applicable)
Related PRs (if any)
Link any related or dependent PRs across repos.
Additional Notes for Reviewers (optional)
Anything reviewers should know before testing or merging (e.g., environment variables, setup steps).
Developer Checklist
To be completed by the developer who raised the PR.
Reviewer Checklist
To be completed by the reviewer before merge.