Skip to content

Claude/mep electrical plumbing yn5y00#453

Open
renanem wants to merge 2 commits into
pascalorg:mainfrom
renanem:claude/mep-electrical-plumbing-yn5y00
Open

Claude/mep electrical plumbing yn5y00#453
renanem wants to merge 2 commits into
pascalorg:mainfrom
renanem:claude/mep-electrical-plumbing-yn5y00

Conversation

@renanem

@renanem renanem commented Jun 30, 2026

Copy link
Copy Markdown

What does this PR do?

How to test

Screenshots / screen recording

Checklist

  • I've tested this locally with bun dev
  • My code follows the existing code style (run bun check to verify)
  • I've updated relevant documentation (if applicable)
  • This PR targets the main branch

Note

High Risk
Large surface area touching authentication, authorization, and admin-only data mutations; middleware and RLS must be correct before production. Client-side audit inserts and admin UI calling Supabase directly assume policies and env keys are configured properly.

Overview
Adds Supabase-backed sign-in with middleware that redirects unauthenticated users to /login and restricts /admin to user_profiles.role === 'admin'. Client bootstrap initializes auth and reloads group-based parameter/custom-field write permissions on session changes.

Introduces an Admin Panel (/admin) with tabs for users (roles, invites, active flag), groups (membership), per-group parametric write matrix from the node registry, custom fields with group write toggles, and a filterable audit log viewer. Ships supabase/schema.sql (profiles, groups, permissions, audit_log, RLS) plus client/server Supabase helpers and Zustand useAuth / usePermissions.

Scene edits are recorded via useAuditLogger on useScene (create/update/delete → audit_log). ParametricInspector gains optional canEditField to render disallowed fields read-only.

MEP expansion: new node kinds water-line, electrical-conduit, and electrical-device (schemas, tools, geometry, floorplan, registry). Build tab MEP sub-grid is split into HVAC / Plumbing / Electrical sections (including water line and electrical tools). Adds typed MEP_CATALOG for picker/MCP defaults.

Reviewed by Cursor Bugbot for commit 3d8be30. Bugbot is set up for automated code reviews on this repo. Configure here.

claude added 2 commits June 30, 2026 02:01
Introduces three new MEP node kinds following the NodeDefinition pattern
established by pipe-segment and pipe-trap:

**water-line** — pressurized water supply polyline
- Schema: path, diameter (0.25–4in), pipeMaterial (pvc/cpvc/pex/copper),
  system (cold-water/hot-water)
- Geometry: cylinder sections colored by system/material with sphere hubs
- Floorplan: solid colored polyline (blue=cold, red=hot) with endpoint handles
- Tool: two-click placement, H to toggle cold/hot, [/] to step diameter

**electrical-conduit** — EMT/PVC/flex conduit polyline
- Schema: path, diameter (0.5–4in), conduitMaterial, system (power/lighting/data)
- Geometry: metallic/matte cylinder sections per material
- Floorplan: dashed polyline colored by system (amber/violet/green)
- Tool: two-click placement, E to cycle systems, [/] to step size

**electrical-device** — point-placed outlet/switch/light/junction-box/panel
- Schema: position, rotation, deviceType, mounting (wall/ceiling/floor), voltage
- Geometry: simple box meshes with material-appropriate coloring
- Floorplan: labeled circle symbol (O/S/L/JB/P) colored by device type
- Tool: click-to-place, D to cycle device type, R/T to rotate ±45°

Also:
- Registers all three kinds in AnyNode discriminated union and core schema exports
- Adds NodeEvents entries in the event bus for each new kind
- Extends StructureTool type in use-editor with the three new tool kinds
- Registers all three definitions in builtinPlugin.nodes
- Reorganizes Build tab MEP sub-grid into HVAC / Plumbing / Electrical sections
- Adds packages/nodes/src/mep/catalog.ts with typed MEP families reference data

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SA83aem53vTNBjCr27LSHj
- Supabase schema (7 tables): user_profiles, groups, group_members,
  parameter_permissions, custom_fields, custom_field_permissions, audit_log
- Auth: browser + server Supabase clients, useAuth Zustand store, Next.js
  middleware protecting /admin and redirecting unauthenticated users to /login
- Permissions: usePermissions store loads per-group write-access at login;
  ParametricInspector accepts canEditField prop to render fields read-only
- Audit trail: useAuditLogger hook diffs Zustand scene state on every
  change and batch-inserts create/update/delete events to audit_log
- Admin panel (/admin): tabbed UI for Users, Groups, Permissions, Custom
  Fields, and paginated Audit Log with filters
- Custom fields: admin-created free-form fields stored in node.metadata;
  group-level write permissions per field

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SA83aem53vTNBjCr27LSHj

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes using high effort and found 9 potential issues.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Want reviews to match your repository better? Bugbot Learning can learn team-specific rules from PR activity. A team admin can enable Learning in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 3d8be30. Configure here.

onClick={() => {
setPage((p) => p + 1)
load(false)
}}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Load more duplicates audit rows

Medium Severity

Clicking "Load more" calls setPage and then load(false). Because setPage updates state asynchronously, load reads the old page value, causing it to refetch and append the same page of data.

Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 3d8be30. Configure here.

old_value: null,
new_value: node as unknown as AuditInsert['new_value'],
})
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Scene load floods audit log

High Severity

The useAuditLogger hook incorrectly logs "create" actions for existing nodes when the entire scene graph is replaced (e.g., on initial load or sync). This misinterprets existing nodes as new creations, leading to excessive and inaccurate audit entries, potentially under the wrong user's account.

Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 3d8be30. Configure here.

Comment thread apps/editor/middleware.ts
const loginUrl = new URL('/login', request.url)
loginUrl.searchParams.set('next', pathname)
return NextResponse.redirect(loginUrl)
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Disabled users still access app

High Severity

The middleware doesn't check user_profiles.is_active, allowing accounts marked as disabled by an admin to access protected routes while their session is valid.

Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 3d8be30. Configure here.

} else {
set({ user: null, profile: null })
}
})

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Auth init stacks listeners

Medium Severity

The init function in useAuth registers an onAuthStateChange listener on each invocation without unsubscribing previous ones. This can lead to multiple active listeners, particularly when init is called repeatedly (like in React Strict Mode), resulting in redundant profile state updates and potential race conditions.

Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 3d8be30. Configure here.

children.push({ kind: 'move-handle', point: [cx, cy] })
}

return { kind: 'group', children }

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Floor plan ignores device yaw

Medium Severity

The buildElectricalDeviceFloorplan function's JSDoc states symbols rotate with device yaw, but node.rotation isn't applied. This causes the 2D floorplan representation to not match the device's 3D orientation.

Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 3d8be30. Configure here.

useEffect(() => {
const unsubscribe = useScene.subscribe((state, prevState) => {
const { user, profile } = useAuth.getState()
if (!user) return

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remote edits misattributed in audit

High Severity

Live scene updates apply another session’s graph locally, but the audit hook still runs and attributes those node diffs to the current user. Collaborator changes can appear in the audit log as if this user created, updated, or deleted them.

Additional Locations (1)
Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 3d8be30. Configure here.

footer?: React.ReactNode
nodeId?: AnyNodeId
onClose?: () => void
canEditField?: (fieldKey: string) => boolean

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Field permissions not enforced

Medium Severity

The inspector supports canEditField for read-only parametrics, and the app loads group permissions into usePermissions, but nothing passes a canEditField callback from the editor shell. Configured parameter permissions do not affect the scene UI.

Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 3d8be30. Configure here.

.eq('node_kind', nodeKind)
setPermissions((prev) =>
prev.filter((p) => !(p.group_id === selectedGroup && p.node_kind === nodeKind)),
)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Revoke all leaves wildcards

Medium Severity

Revoke all for a node kind deletes rows matching that exact node_kind only. Permissions with node_kind or parameter_key set to * stay in the database, so isGranted can still show access after a full revoke.

Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 3d8be30. Configure here.

setLoading(false)
} else {
router.replace(next)
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Login success leaves loading set

Low Severity

After a successful signIn, the form calls router.replace(next) but never clears loading. If navigation is slow or fails, the submit button stays disabled and shows Signing in….

Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 3d8be30. Configure here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants