feat: init plugin framework + object storage plugin#2198
feat: init plugin framework + object storage plugin#2198viktormarinho merged 26 commits intomainfrom
Conversation
Release OptionsShould a new version be published when this PR is merged? React with an emoji to vote on the release type:
Current version: Deployment
|
🧪 BenchmarkShould we run the MCP Gateway benchmark for this PR? React with 👍 to run the benchmark.
Benchmark will run on the next push after you react. |
There was a problem hiding this comment.
6 issues found across 14 files
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="packages/bindings/package.json">
<violation number="1" location="packages/bindings/package.json:12">
P2: `@types/react` should be in `devDependencies`, not `dependencies`. Type definitions are only needed at compile time, not runtime.</violation>
</file>
<file name="apps/mesh/src/web/plugins.ts">
<violation number="1" location="apps/mesh/src/web/plugins.ts:2">
P2: Using a deep relative path with file extension is fragile and inconsistent with the workspace dependency pattern used elsewhere in this project. Consider adding `"mesh-plugin-store": "workspace:*"` to the mesh app's package.json dependencies and importing as `import { storePlugin } from "mesh-plugin-store"`.</violation>
</file>
<file name="packages/mesh-plugin-store/.gitignore">
<violation number="1" location="packages/mesh-plugin-store/.gitignore:15">
P2: This pattern `_.log` appears to be a typo and should be `*.log` to match all log files. Currently it only matches a file literally named `_.log`.</violation>
<violation number="2" location="packages/mesh-plugin-store/.gitignore:16">
P2: This pattern uses `_` instead of `*` wildcards. Should be `report.[0-9]*.[0-9]*.[0-9]*.[0-9]*.json` to properly match Node.js diagnostic report files.</violation>
</file>
<file name="apps/mesh/src/web/index.tsx">
<violation number="1" location="apps/mesh/src/web/index.tsx:201">
P2: `Array.prototype.forEach()` always returns `undefined`, so `plugins` will be `undefined`. If the export is needed, use `.map()` and return the plugin. If only side effects are needed, remove the assignment and export.</violation>
</file>
<file name="packages/mesh-plugin-store/package.json">
<violation number="1" location="packages/mesh-plugin-store/package.json:7">
P2: Avoid using `"latest"` for dependencies. Use a versioned range like `"^1.3.5"` to match the workspace convention (see `packages/runtime/package.json`) and ensure reproducible builds.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
There was a problem hiding this comment.
7 issues found across 71 files (changes from recent commits).
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="packages/mesh-plugin-store/layout.tsx">
<violation number="1" location="packages/mesh-plugin-store/layout.tsx:11">
P2: Relative import crosses package boundary into app internals. This tightly couples the plugin package to the mesh app's internal file structure. Consider exporting `PluginLayout` from the mesh app as a proper entry point (e.g., via a shared package or explicit app export) rather than using relative path traversal.</violation>
</file>
<file name="packages/mesh-sdk/src/context/project-context.tsx">
<violation number="1" location="packages/mesh-sdk/src/context/project-context.tsx:74">
P2: Context value object is recreated on every render, causing unnecessary re-renders of all consumers. Wrap the value in `useMemo` to maintain referential equality when `org` and `project` haven't changed.</violation>
</file>
<file name="packages/mesh-sdk/package.json">
<violation number="1" location="packages/mesh-sdk/package.json:12">
P2: `@types/react` should be in `devDependencies` instead of `dependencies`. Type definition packages are only needed at compile-time, and other packages in this workspace (e.g., `packages/ui`) correctly place them in `devDependencies`.</violation>
</file>
<file name="packages/ui/src/lib/github.ts">
<violation number="1" location="packages/ui/src/lib/github.ts:68">
P2: Links with existing `target` attribute won't get `rel="noopener noreferrer"` added. When the function detects an existing `target=`, it only replaces the target value but skips adding the `rel` attribute, which is needed for security when using `target="_blank"`.</violation>
</file>
<file name="packages/mesh-sdk/src/hooks/use-mcp.ts">
<violation number="1" location="packages/mesh-sdk/src/hooks/use-mcp.ts:82">
P2: The fetch calls should use the abort signal from React Query to support proper cancellation. Without it, requests continue even when the component unmounts or the query is cancelled.</violation>
</file>
<file name="packages/mesh-plugin-store/components/empty-state.tsx">
<violation number="1" location="packages/mesh-plugin-store/components/empty-state.tsx:34">
P2: Condition `image !== null` doesn't handle `undefined`. When `image` is not passed, the wrapper div will still render. Use `image != null` (loose equality) or `image &&` (truthy check) to properly handle both `null` and `undefined`.</violation>
</file>
<file name="packages/mesh-sdk/src/lib/tool-caller.ts">
<violation number="1" location="packages/mesh-sdk/src/lib/tool-caller.ts:70">
P1: Content-Type check uses strict equality which will fail when charset is included (e.g., `application/json; charset=utf-8`). Use `.includes()` or `.startsWith()` instead.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
There was a problem hiding this comment.
7 issues found across 60 files (changes from recent commits).
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="packages/mesh-plugin-object-storage/components/file-browser.tsx">
<violation number="1" location="packages/mesh-plugin-object-storage/components/file-browser.tsx:155">
P1: Selection state persists across folder navigation. When user navigates to a different folder, `selectedKeys` should be cleared to prevent accidentally deleting files from the previous folder.</violation>
</file>
<file name="apps/mesh/src/web/providers/project-context-provider.tsx">
<violation number="1">
P2: Context value should be memoized to prevent unnecessary consumer re-renders. Currently, a new object is created on every render, causing all `useProjectContext` consumers to re-render even when props haven't changed.</violation>
</file>
<file name="apps/mesh/src/web/components/store/readme-viewer.tsx">
<violation number="1">
P2: When `target` attribute already exists, the replacement adds `target="_blank"` but fails to add `rel="noopener noreferrer"`, leaving a potential security vulnerability where opened pages could access `window.opener`.</violation>
</file>
<file name="apps/mesh/src/web/hooks/use-tool-call.ts">
<violation number="1">
P2: Remove debug `console.log` statement from production code. If logging is needed, use a proper logging framework or remove entirely.</violation>
</file>
<file name="apps/mesh/src/web/hooks/use-mcp.ts">
<violation number="1">
P2: State calculation doesn't account for `enabled=false` case. When query is disabled but URL exists, state incorrectly shows "ready" instead of "disconnected". Consider checking `enabled` in the state derivation.</violation>
</file>
<file name="packages/mesh-plugin-object-storage/lib/utils.ts">
<violation number="1" location="packages/mesh-plugin-object-storage/lib/utils.ts:39">
P2: Array index out of bounds for files >= 1 PB. The `units` array has 5 elements but `i` can be 5+ for very large values, causing `units[i]` to be `undefined`.</violation>
</file>
<file name="packages/mesh-plugin-object-storage/layout.tsx">
<violation number="1" location="packages/mesh-plugin-object-storage/layout.tsx:9">
P1: Package should not import from app. This relative import from `packages/` to `apps/` violates the monorepo dependency hierarchy. If plugins need `PluginLayout`, it should be exported from a shared package like `@decocms/mesh-sdk` rather than importing directly from the app.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
ba78334 to
5f87c8e
Compare
There was a problem hiding this comment.
1 issue found across 10 files (changes from recent commits).
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="apps/mesh/src/web/hooks/use-store-discovery.ts">
<violation number="1">
P2: Search value used in conditions should be trimmed to match the check. If `search` is `" foo "`, the trim check passes but the actual query uses the untrimmed value with leading/trailing spaces.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
- Move store components from mesh app to mesh-plugin-store package
- Create @decocms/mesh-sdk package with shared hooks (useConnection, useToolCall, useMcp, etc.)
- Add PluginContext system with typed tool caller support
- Add PluginLayout for consistent plugin rendering with connection selection
- Support partial context for empty states via usePluginContext({ partial: true })
- Move ReadmeViewer and GitHub utilities to @deco/ui
- Clean up unused store routes and components from mesh app
Part 1: Revert Store Changes - Move store components back from mesh-plugin-store to apps/mesh/ - Restore store routes in mesh app router - Delete packages/mesh-plugin-store package - Keep plugin infrastructure (PluginContext, PluginLayout, mesh-sdk) Part 2: Create Object Storage Plugin - Add OBJECT_STORAGE_BINDING in packages/bindings with tools: - LIST_OBJECTS, GET_OBJECT_METADATA, GET_PRESIGNED_URL - PUT_PRESIGNED_URL, DELETE_OBJECT, DELETE_OBJECTS - Create packages/mesh-plugin-object-storage with: - File browser UI with list view - Breadcrumb navigation - Upload/download via presigned URLs - Single and batch delete - Infinite scroll pagination - Register plugin in apps/mesh/src/web/plugins.ts
- Add Store back to sidebar navigation - Restore use-gateway-system-prompt.ts, use-system.ts, theme-provider.tsx - Restore public-config.ts API route - Restore ThemeConfig in core/config.ts - Add gatewaySystemPrompts to LOCALSTORAGE_KEYS - Add publicConfig to KEYS
de702c2 to
5ee3c93
Compare
There was a problem hiding this comment.
1 issue found across 14 files (changes from recent commits).
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="apps/mesh/src/web/routes/orgs/settings/plugins.tsx">
<violation number="1" location="apps/mesh/src/web/routes/orgs/settings/plugins.tsx:24">
P2: Local state `enabledPlugins` is initialized once from `orgSettings` but won't sync if the query refetches (e.g., after staleTime expires or concurrent updates). Consider adding a `useEffect` to sync state when `orgSettings?.enabled_plugins` changes, or reset state only when there are no pending changes.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
There was a problem hiding this comment.
1 issue found across 4 files (changes from recent commits).
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="packages/mesh-plugin-object-storage/components/file-browser.tsx">
<violation number="1" location="packages/mesh-plugin-object-storage/components/file-browser.tsx:162">
P2: Selection state is not cleared when navigating via browser history (back/forward buttons). Since `prefix` is now URL-derived, but `selectedKeys` is only cleared in `setPrefix()`, browser history navigation will leave stale selections. Add a `useEffect` to clear selection when `prefix` changes.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
|
|
||
| const [selectedKeys, setSelectedKeys] = useState<Set<string>>(new Set()); | ||
|
|
||
| const setPrefix = (newPath: string) => { |
There was a problem hiding this comment.
P2: Selection state is not cleared when navigating via browser history (back/forward buttons). Since prefix is now URL-derived, but selectedKeys is only cleared in setPrefix(), browser history navigation will leave stale selections. Add a useEffect to clear selection when prefix changes.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/mesh-plugin-object-storage/components/file-browser.tsx, line 162:
<comment>Selection state is not cleared when navigating via browser history (back/forward buttons). Since `prefix` is now URL-derived, but `selectedKeys` is only cleared in `setPrefix()`, browser history navigation will leave stale selections. Add a `useEffect` to clear selection when `prefix` changes.</comment>
<file context>
@@ -151,8 +153,16 @@ function Breadcrumb({ prefix, onNavigate }: BreadcrumbProps) {
+
const [selectedKeys, setSelectedKeys] = useState<Set<string>>(new Set());
+
+ const setPrefix = (newPath: string) => {
+ setSelectedKeys(new Set()); // Clear selection when navigating folders
+ navigate({ to: "/", search: { path: newPath || undefined } });
</file context>
There was a problem hiding this comment.
1 issue found across 7 files (changes from recent commits).
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="packages/mesh-plugin-object-storage/components/image-preview.tsx">
<violation number="1" location="packages/mesh-plugin-object-storage/components/image-preview.tsx:34">
P1: IntersectionObserver setup runs during render phase instead of useEffect. On first render, `ref.current` is null (DOM not attached yet), so the observer won't be created. This also causes a memory leak if the component unmounts before intersection. Use useEffect to properly set up and clean up the observer.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| const hasTriggered = useRef(false); | ||
|
|
||
| // Set up the observer | ||
| if (typeof window !== "undefined" && ref.current && !hasTriggered.current) { |
There was a problem hiding this comment.
P1: IntersectionObserver setup runs during render phase instead of useEffect. On first render, ref.current is null (DOM not attached yet), so the observer won't be created. This also causes a memory leak if the component unmounts before intersection. Use useEffect to properly set up and clean up the observer.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/mesh-plugin-object-storage/components/image-preview.tsx, line 34:
<comment>IntersectionObserver setup runs during render phase instead of useEffect. On first render, `ref.current` is null (DOM not attached yet), so the observer won't be created. This also causes a memory leak if the component unmounts before intersection. Use useEffect to properly set up and clean up the observer.</comment>
<file context>
@@ -0,0 +1,112 @@
+ const hasTriggered = useRef(false);
+
+ // Set up the observer
+ if (typeof window !== "undefined" && ref.current && !hasTriggered.current) {
+ const observer = new IntersectionObserver(([entry]) => {
+ if (entry?.isIntersecting && !hasTriggered.current) {
</file context>
✅ Addressed in 3ea580f
There was a problem hiding this comment.
1 issue found across 1 file (changes from recent commits).
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="packages/mesh-plugin-object-storage/components/image-preview.tsx">
<violation number="1" location="packages/mesh-plugin-object-storage/components/image-preview.tsx:34">
P2: The `setRef` callback is recreated on every render, causing unnecessary IntersectionObserver disconnection/recreation until intersection is triggered. Wrap with `useCallback` to maintain a stable reference.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| const hasTriggered = useRef(false); | ||
|
|
||
| // Callback ref - called when element mounts (with node) or unmounts (with null) | ||
| const setRef = (node: HTMLElement | null) => { |
There was a problem hiding this comment.
P2: The setRef callback is recreated on every render, causing unnecessary IntersectionObserver disconnection/recreation until intersection is triggered. Wrap with useCallback to maintain a stable reference.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/mesh-plugin-object-storage/components/image-preview.tsx, line 34:
<comment>The `setRef` callback is recreated on every render, causing unnecessary IntersectionObserver disconnection/recreation until intersection is triggered. Wrap with `useCallback` to maintain a stable reference.</comment>
<file context>
@@ -19,46 +19,53 @@ interface ImagePreviewProps {
- }
- }, options);
+ // Callback ref - called when element mounts (with node) or unmounts (with null)
+ const setRef = (node: HTMLElement | null) => {
+ // Cleanup any existing observer
+ if (observerRef.current) {
</file context>
There was a problem hiding this comment.
@cubic-dev-ai Our codebase uses react-compiler, so we actually don't use useMemo/useCallback anymore. Would be good if you saved this into your memory.
There was a problem hiding this comment.
Thanks for the feedback! I've saved this as a new learning to improve future reviews.
Summary by cubic
Introduces a plugin framework with typed routing/context and ships the Object Storage (S3) plugin. Plugins mount at /$org/$pluginId, can be enabled per org, and only enabled plugins appear in the sidebar.
New Features
Refactors / Dependencies
Written for commit 358e05e. Summary will update on new commits.