Conversation
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 29 minutes and 26 seconds. ⌛ 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: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (12)
📝 WalkthroughWalkthroughThis pull request introduces a new Flow plugin that enables users to create, manage, and visualize interactive system flow diagrams. The implementation adds backend CRUD endpoints with role-based access control, a comprehensive diagram editor frontend component, and integrates the Flow entity kind throughout the application UI. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Browser as Web Client
participant API as Flow API<br/>(handlers)
participant DB as Database
participant EventBus as Event System
User->>Browser: Creates new Flow entity
Browser->>API: POST /plugins/flow/entities<br/>(entity spec)
API->>API: Validate write access<br/>(check editorRole permission)
API->>API: Validate entity structure<br/>(Kind="Flow")
API->>DB: Create Flow entity
API->>EventBus: Publish EntityCreated event
API->>DB: Write audit log<br/>(entity state, client IP)
API-->>Browser: HTTP 201 + entity
Browser->>Browser: Update local flows list<br/>Reload saved flow draft
User->>Browser: Modifies Flow diagram<br/>(adds nodes/edges)
Browser->>API: PUT /plugins/flow/entities/{name}<br/>(updated spec)
API->>API: Validate write access
API->>DB: Fetch previous state<br/>(for audit)
API->>DB: Update Flow entity
API->>EventBus: Publish EntityUpdated event
API->>DB: Write audit log<br/>(before/after states)
API-->>Browser: HTTP 200 + entity
User->>Browser: Requests Flow settings
Browser->>API: GET /plugins/flow/settings
API->>DB: Load Flow plugin config
API->>API: Compute canEdit permission<br/>(editorRole vs caller role)
API-->>Browser: HTTP 200 + settings<br/>(showInSidebar, editorRole, canEdit)
Browser->>Browser: Conditionally render<br/>editor vs. read-only view
sequenceDiagram
participant User
participant Canvas as Flow Editor<br/>Canvas
participant StateStore as Draft<br/>State
participant API as Flow API
participant DB as Database
User->>Canvas: Drag entity node
Canvas->>StateStore: Update node position
StateStore->>StateStore: Mark draft dirty
Canvas->>Canvas: Re-render canvas
User->>Canvas: Click "Seed Edges"
Canvas->>Canvas: Analyze existing entity nodes<br/>(dependsOn, consumesApis, etc.)
Canvas->>Canvas: Generate relationship edges
Canvas->>StateStore: Add edges to draft
Canvas->>Canvas: Re-render with new edges
User->>Canvas: Click "Save"
Canvas->>API: POST or PUT /plugins/flow/entities<br/>(full FlowSpec + metadata)
API->>API: Validate against schema
API->>DB: Persist Flow entity
API-->>Canvas: HTTP 200/201 + persisted entity
Canvas->>StateStore: Clear dirty flag
Canvas->>Canvas: Reload draft from server
Canvas->>User: Show success notice
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes 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 |
Go2Engle
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 10
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
web/src/pages/EntityDetail.tsx (1)
365-397:⚠️ Potential issue | 🟠 MajorGeneric entity endpoints must enforce Flow editor permissions.
The generic
PUT /entities/Flow/{name}andDELETE /entities/Flow/{name}endpoints bypass Flow'seditorRolerestriction. These handlers are gated only byRequireRole("developer"), allowing any developer to edit or delete Flow entities even if they lack the Flow editor role. AddensureFlowWriteAccess()check toUpdateEntityandDeleteEntitywhenkind == "Flow".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/src/pages/EntityDetail.tsx` around lines 365 - 397, Update the generic entity handlers so Flow edits/deletes require Flow editor permissions: in the UpdateEntity and DeleteEntity handlers, when entity.kind === "Flow" call ensureFlowWriteAccess() (and abort/return a forbidden/error response if it fails) before performing the update or delete; ensure this check runs in both UpdateEntity and DeleteEntity code paths and gates any downstream logic that modifies or removes Flow entities.
🧹 Nitpick comments (3)
internal/api/handlers/flow.go (1)
41-50: Reusingentity.ErrEntityNotFoundfor a missing plugin conflates two concepts.
getFlowPluginreturnsentity.ErrEntityNotFoundwhen theflowrow is missing from the plugins table. Callers then translate that back to "flow plugin not installed" (lines 72-74, 94-96). Consider a dedicated sentinel (e.g.ErrPluginNotInstalledin thepluginspackage) to avoid future mis-detection ifErrEntityNotFoundever leaks through a different code path.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/api/handlers/flow.go` around lines 41 - 50, Replace use of the generic entity.ErrEntityNotFound in getFlowPlugin with a dedicated sentinel error in the plugins package (e.g., plugins.ErrPluginNotInstalled); create that error in the plugins package, update getFlowPlugin to return plugins.ErrPluginNotInstalled when h.DB.GetPlugin returns a nil plugin, and update any callers of getFlowPlugin (the handlers that currently interpret entity.ErrEntityNotFound) to check for plugins.ErrPluginNotInstalled instead of entity.ErrEntityNotFound so the “flow plugin not installed” case is unambiguous.web/src/components/FlowTab.tsx (1)
25-181: Significant duplication withweb/src/pages/Flow.tsx.
ensureFlowSpec,isMockNode,nodeColor,withAlpha,mockFoldFill,estimateWrappedLines,getNodeDimensions,renderMockNodeShell,mockContentClasses,mockContentStyle,edgePath,edgeLabelPosition,edgeOffsetTransform,kindColors,MOCK_SHAPE_OPTIONS, and the size constants are copy-pasted between this file andweb/src/pages/Flow.tsx(lines ~60-368). Any future tweak to shape geometry/sizing will need to be made in two places and will quietly drift (e.g. the existing divergence inmockShapeLabelreturn strings: "Mock Pill" vs "Pill"). Consider extracting a shared module likeweb/src/lib/flow.tsthat both the preview and editor import.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/src/components/FlowTab.tsx` around lines 25 - 181, Duplicate flow utilities and constants (ensureFlowSpec, isMockNode, nodeColor, withAlpha, mockFoldFill, estimateWrappedLines, getNodeDimensions, mockShapeLabel, plus MOCK_SHAPE_OPTIONS, kindColors and size constants) exist in both FlowTab and Flow; extract them into a single shared module and import it from both places. Create a new shared module that exports the listed functions and constants, move the implementations (keeping the canonical strings like mockShapeLabel returns consistent), update FlowTab to import the utilities instead of redefining them, and update the other file (Flow) to import the same module so all geometry/sizing/labels are maintained in one place. Ensure exported types (FlowSpec, FlowNode, FlowMockNode, FlowEntityNode, FlowEdge) are available or re-exported so existing type usages in ensureFlowSpec and getNodeDimensions continue to compile.web/src/pages/Flow.tsx (1)
989-989: Use functionalsetDirtyto avoid stale closure ondirty.
setDirty(added > 0 || dirty)capturesdirtyat render time. If this handler is ever triggered while another update is already pending,dirtywill be stale. Safer to write:- setDirty(added > 0 || dirty); + setDirty((prev) => added > 0 || prev);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/src/pages/Flow.tsx` at line 989, The current call setDirty(added > 0 || dirty) uses the captured dirty value and can suffer from a stale-closure race; change it to use the functional updater form of the state setter (i.e., pass a function that receives the previous dirty value and returns added > 0 || previous) so the update always composes with any pending state changes—update the code around the setDirty call in the handler that references added and dirty (look for the setDirty call) to use the prev-state functional pattern.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@internal/api/handlers/flow.go`:
- Around line 115-119: The CreateFlowEntity and UpdateFlowEntity handlers
currently call json.NewDecoder(r.Body).Decode(&e) without limiting body size;
wrap r.Body with http.MaxBytesReader before creating the decoder (e.g., maxBytes
:= int64(1<<20) or a shared const) and use
json.NewDecoder(http.MaxBytesReader(w, r.Body, maxBytes)). After decoding,
detect if the error is due to exceeding the limit and return
http.StatusRequestEntityTooLarge (413), otherwise return the existing bad
request error; update both CreateFlowEntity and UpdateFlowEntity and keep
references to json.NewDecoder, r.Body, and http.MaxBytesReader.
In `@internal/api/handlers/plugins.go`:
- Around line 194-203: The Flow editor role is being validated after trimming
but the trimmed value isn't written back, so whitespace like " developer "
passes validation but is stored with spaces; in the name == "flow" block update
merged["editorRole"] to the trimmed role (after computing role :=
strings.TrimSpace(...)), ensure you set merged["editorRole"] = "developer" when
role == "" and otherwise, after auth.IsValidRole(role) returns true, persist
merged["editorRole"] = role; use the merged map and auth.IsValidRole references
to locate the change.
In `@internal/entity/schemas/flow.json`:
- Around line 18-20: The schema currently allows zero/negative sizes; update the
JSON schema to require strictly positive values by adding "exclusiveMinimum": 0
to the "zoom" property and to the "width" and "height" properties (both
occurrences referenced in this file), ensuring these numeric fields are
validated as > 0 rather than allowing 0 or negatives.
In `@internal/plugins/bundled/registry.json`:
- Around line 861-862: Update the plugin registry entry so the plugin advertises
its Flow detail tab by populating the entityPanels array instead of leaving it
empty; specifically, in the same plugin object that currently has
"entityPanels": [] and "actionTypes": [], set "entityPanels" to include the Flow
entity kind (e.g., "Flow") so registry-driven metadata is aware the plugin
provides a Flow detail tab.
In `@web/src/components/EntityCard.tsx`:
- Line 22: Replace the hard-coded Tailwind "sky-*" classes used for the Flow
badge in EntityCard (the Flow entry in the badge mapping) with styles that
reference Gantry CSS variables (e.g., --gantry-bg-*, --gantry-text-*,
--gantry-border, --gantry-accent as appropriate). Update the Flow badge
rendering (look for the Flow key in the badge/class mapping or the JSX that
applies that class string) to use either a class that consumes those CSS
variables or inline style attributes that set backgroundColor, color and any
dark-mode variants via the matching Gantry vars, ensuring only the allowed
variables (--gantry-bg-primary/secondary/tertiary,
--gantry-text-primary/secondary, --gantry-border, --gantry-accent,
--gantry-accent-hover, --gantry-danger) are referenced.
In `@web/src/lib/api.ts`:
- Around line 209-215: The updateFlow call in updateFlow (web/src/lib/api.ts)
does not propagate the passed namespace into the request body, so the server may
read metadata.namespace and default to "default"; modify updateFlow to merge the
passed namespace into the Entity metadata before sending (e.g., set
metadata.namespace = namespace when namespace is provided and not
empty/undefined, leaving existing metadata.namespace otherwise) so the request
body contains the correct namespace that the UpdateFlowEntity handler will use.
In `@web/src/pages/Catalog.tsx`:
- Around line 47-50: Replace the hardcoded Tailwind color in the Flow kind
definition by using the Gantry CSS variable; specifically update the Flow
object's color property (the one currently set to 'text-sky-500') to reference
the appropriate --gantry-* variable (e.g., --gantry-accent or the theme color
used for kinds) so the icon color stays theme-compatible across the app.
In `@web/src/pages/Flow.tsx`:
- Around line 1259-1290: Replace the hardcoded hex stroke colors in the edge
rendering paths with the theme CSS variables so edges respect dark mode;
specifically update the stroke attributes used in the single-path branch and the
twoWay forward/reverse branches (the path elements rendering with stroke={...}
in Flow.tsx) to use var(--gantry-text-primary) for the selected/active strokes
and var(--gantry-text-secondary) for the unselected/inactive strokes (or
dedicated tokens if available), keeping existing strokeWidth, strokeDasharray,
markerStart/markerEnd and animation logic intact.
- Around line 481-553: The effect watching flowBounds causes an infinite
re-render because getFlowBounds returns a new object each render; memoize the
result by replacing the direct call to getFlowBounds(flowSpec) with a useMemo
that computes flowBounds and depends on the true inputs (e.g., flowSpec and any
values used inside getFlowBounds) so the reference stabilizes; keep the existing
useEffect that calls setRenderBounds(flowBounds) but use the memoized flowBounds
variable (functions: getFlowBounds, setRenderBounds, useEffect, and the
flowBounds constant) to prevent the dependency loop.
- Around line 1191-1195: The onWheelCapture synthetic handler is ineffective at
preventing page scroll because React registers passive wheel listeners; replace
it by adding a native wheel listener on viewportRef.current inside a useEffect
that attaches with { passive: false }, calls event.preventDefault() and
event.stopPropagation(), computes the new zoom using canvasZoom and ZOOM_STEP
and invokes applyZoomAtClientPoint with event.clientX/event.clientY; ensure the
effect adds the listener when viewportRef.current exists, updates when
canvasZoom or applyZoomAtClientPoint change, and removes the listener in the
cleanup.
---
Outside diff comments:
In `@web/src/pages/EntityDetail.tsx`:
- Around line 365-397: Update the generic entity handlers so Flow edits/deletes
require Flow editor permissions: in the UpdateEntity and DeleteEntity handlers,
when entity.kind === "Flow" call ensureFlowWriteAccess() (and abort/return a
forbidden/error response if it fails) before performing the update or delete;
ensure this check runs in both UpdateEntity and DeleteEntity code paths and
gates any downstream logic that modifies or removes Flow entities.
---
Nitpick comments:
In `@internal/api/handlers/flow.go`:
- Around line 41-50: Replace use of the generic entity.ErrEntityNotFound in
getFlowPlugin with a dedicated sentinel error in the plugins package (e.g.,
plugins.ErrPluginNotInstalled); create that error in the plugins package, update
getFlowPlugin to return plugins.ErrPluginNotInstalled when h.DB.GetPlugin
returns a nil plugin, and update any callers of getFlowPlugin (the handlers that
currently interpret entity.ErrEntityNotFound) to check for
plugins.ErrPluginNotInstalled instead of entity.ErrEntityNotFound so the “flow
plugin not installed” case is unambiguous.
In `@web/src/components/FlowTab.tsx`:
- Around line 25-181: Duplicate flow utilities and constants (ensureFlowSpec,
isMockNode, nodeColor, withAlpha, mockFoldFill, estimateWrappedLines,
getNodeDimensions, mockShapeLabel, plus MOCK_SHAPE_OPTIONS, kindColors and size
constants) exist in both FlowTab and Flow; extract them into a single shared
module and import it from both places. Create a new shared module that exports
the listed functions and constants, move the implementations (keeping the
canonical strings like mockShapeLabel returns consistent), update FlowTab to
import the utilities instead of redefining them, and update the other file
(Flow) to import the same module so all geometry/sizing/labels are maintained in
one place. Ensure exported types (FlowSpec, FlowNode, FlowMockNode,
FlowEntityNode, FlowEdge) are available or re-exported so existing type usages
in ensureFlowSpec and getNodeDimensions continue to compile.
In `@web/src/pages/Flow.tsx`:
- Line 989: The current call setDirty(added > 0 || dirty) uses the captured
dirty value and can suffer from a stale-closure race; change it to use the
functional updater form of the state setter (i.e., pass a function that receives
the previous dirty value and returns added > 0 || previous) so the update always
composes with any pending state changes—update the code around the setDirty call
in the handler that references added and dirty (look for the setDirty call) to
use the prev-state functional pattern.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 05058585-5fba-47fd-a5eb-1d8d69a410c3
📒 Files selected for processing (19)
internal/api/handlers/flow.gointernal/api/handlers/plugins.gointernal/api/handlers/topology.gointernal/api/server.gointernal/entity/kinds.gointernal/entity/schemas/flow.jsoninternal/plugins/bundled/registry.jsonweb/src/App.tsxweb/src/components/CommandPalette.tsxweb/src/components/EntityCard.tsxweb/src/components/FlowTab.tsxweb/src/components/Sidebar.tsxweb/src/lib/api.tsweb/src/lib/types.tsweb/src/pages/Catalog.tsxweb/src/pages/Dashboard.tsxweb/src/pages/EntityDetail.tsxweb/src/pages/Flow.tsxweb/src/pages/Plugins.tsx
Comment on lines
+194
to
+203
| if name == "flow" { | ||
| role, _ := merged["editorRole"].(string) | ||
| role = strings.TrimSpace(role) | ||
| if role == "" { | ||
| merged["editorRole"] = "developer" | ||
| } else if !auth.IsValidRole(role) { | ||
| writeError(w, http.StatusBadRequest, "invalid flow editor role") | ||
| return | ||
| } | ||
| } |
There was a problem hiding this comment.
Persist the normalized Flow editor role.
A value like " developer " is validated as developer but saved with whitespace, which can break downstream role checks. Store the trimmed value after validation.
🐛 Proposed fix
if name == "flow" {
role, _ := merged["editorRole"].(string)
role = strings.TrimSpace(role)
if role == "" {
merged["editorRole"] = "developer"
} else if !auth.IsValidRole(role) {
writeError(w, http.StatusBadRequest, "invalid flow editor role")
return
+ } else {
+ merged["editorRole"] = role
}
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@internal/api/handlers/plugins.go` around lines 194 - 203, The Flow editor
role is being validated after trimming but the trimmed value isn't written back,
so whitespace like " developer " passes validation but is stored with spaces; in
the name == "flow" block update merged["editorRole"] to the trimmed role (after
computing role := strings.TrimSpace(...)), ensure you set merged["editorRole"] =
"developer" when role == "" and otherwise, after auth.IsValidRole(role) returns
true, persist merged["editorRole"] = role; use the merged map and
auth.IsValidRole references to locate the change.
This was referenced Apr 20, 2026
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.
This pull request introduces a new "Flow" plugin and entity type, enabling users to create, edit, and manage interactive system flow diagrams that are backed by catalog entities. The implementation includes backend API endpoints, schema and registry updates, access control, and frontend integration, ensuring that "Flow" entities are first-class citizens in the system and UI. Additionally, the UI and command palette are updated to support and display the new entity type.
Flow plugin and entity support:
Flowentity kind to the catalog, including its JSON schema (flow.json) for validation and a corresponding entry in the plugin registry, describing its features and configuration options. [1] [2] [3]Flowentities, including role-based access control, auditing, and event publishing. These endpoints are registered in the API server and include logic for editor role validation and sidebar visibility. [1] [2] [3] [4]Flowentities from topology graphs, as flows are meant to be diagrammatic and not part of the inferred system topology.Frontend and UI integration:
/flowroute in the web frontend, with a suspense fallback, and included the Flow page in the application routes. [1] [2] [3]Flowkind using a new icon and color scheme. [1] [2] [3] [4] [5]