feat: OPC-UA Server Configuration#546
Conversation
Add comprehensive design documentation for the OPC-UA server configuration feature including: - Design overview with architecture and configuration flow - UI screen specifications for all 5 configuration tabs - JSON configuration mapping between editor and runtime formats - Implementation phases with detailed deliverables The OPC-UA server configuration will allow users to expose PLC variables through the OPC-UA protocol with support for: - Multiple security profiles (None, Sign, SignAndEncrypt) - User authentication (password and certificate-based) - Role-based access control (viewer, operator, engineer) - Complex data types (structures, arrays, nested function blocks) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Add foundational OPC-UA server configuration support including:
- Add OPC-UA type definitions (security profiles, users, certificates,
permissions, address space nodes, server config) to open-plc.ts
- Add 'opcua' protocol to server schemas (PLCServerProtocolSchema,
editor types, tabs types)
- Add OPC-UA option to server creation dropdown in element-card
- Implement OPC-UA server initialization with default configuration
- Create OpcUaServerEditor component with 5-tab interface:
- General Settings (functional): server name, URIs, port, bind
address, endpoint path, cycle time, enable toggle
- Security Profiles (placeholder for Phase 2)
- Users (placeholder for Phase 3)
- Certificates (placeholder for Phase 4)
- Address Space (placeholder for Phase 5)
- Add updateOpcUaServerConfig action to project slice
- Style OPC-UA editor to match existing Modbus/S7Comm server editors
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Add security profile CRUD actions to project slice: - addOpcUaSecurityProfile: Add new profile with duplicate name validation - updateOpcUaSecurityProfile: Update existing profile - removeOpcUaSecurityProfile: Remove profile (prevents deleting last one) - Create SecurityProfileModal component with validation: - Anonymous auth only with "None" security policy - Security Mode must match Policy constraints - Profile names must be unique - At least one auth method required - Create SecurityProfilesTab component: - List profiles with enable/disable toggles - Add/Edit/Delete functionality - Warning for insecure profiles (None policy) - Prevents disabling last enabled profile - Update OpcUaServerEditor to use SecurityProfilesTab Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Users Tab: - Add user CRUD actions (addOpcUaUser, updateOpcUaUser, removeOpcUaUser) - UserModal with password/certificate authentication type selector - Password hashing using SHA-256 (Web Crypto API) - Role selector (Viewer/Operator/Engineer) with descriptions - Certificate user binding to trusted certificates - Validation for duplicate usernames and certificate bindings Certificates Tab: - Server certificate strategy (auto_self_signed/custom) - Custom certificate and private key upload with file browse - Trusted client certificates list with Add/Delete - Certificate ID validation and PEM format validation - Prevents deleting certificates bound to users Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Add variable selection and OPC-UA node configuration: - Add address space node actions to project slice: - updateOpcUaAddressSpaceNamespace - addOpcUaNode, updateOpcUaNode, removeOpcUaNode - Create use-project-variables hook to extract variables from POUs into a hierarchical tree structure - Create UI components: - VariableTree: displays project variables with expand/collapse - VariableConfigModal: configure OPC-UA node properties - SelectedVariablesList: shows configured OPC-UA nodes - AddressSpaceTab: split-pane layout with namespace URI input - Features: - Browse and select PLC variables from programs and GVL - Configure node ID, browse name, display name, description - Set initial values and per-role permissions (viewer/operator/engineer) - Support for variables, structures, arrays, and function blocks Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
…gration Phase 5 - Address Space Complex Types: - Add array element expansion in variable tree (up to 50 elements) - Implement parent selection behavior (select/deselect all children) - Add structure/array info display in configuration modal - Add field permissions table for structures and function blocks - Include arrayInfo and structureInfo metadata in tree nodes Phase 6 - Compiler Integration: - Create OPC-UA JSON generator utility (src/utils/opcua/) - Implement index resolution from debug.c for variables, structures, arrays - Integrate OPC-UA config generation into compiler pipeline - Add OpcUaConfigError class for detailed error reporting - Add unit tests for generator (15 tests passing) - Generate conf/opcua.json during compilation for Runtime v4 Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
…type support - Create shared utilities for debug variable index resolution: - debug-variable-finder.ts: Path building, variable lookup functions - pou-helpers.ts: FB/struct lookup, leaf variable finding - Enhance OPC-UA to support complex types (FBs, structures, arrays): - Make complex types selectable in variable tree - Recursively expand to leaf variables at compile time - Add debugTypeToIecType() conversion for runtime compatibility - Update compiler to pass instances to OPC-UA config generation - Fix TypeScript errors with ArrayData type definition - Update tests for new instance-based path resolution Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Comprehensive documentation explaining: - Problem statement and current code duplication - Analysis of debug path formats in debug.c - Current architecture of debugger and OPC-UA systems - Detailed 6-phase refactoring plan: 1. Consolidate debug file parsing 2. Unify path building 3. Unify variable index lookup 4. Unify tree building with visitor pattern 5. Update debugger polling 6. Remove deprecated code after verification - Migration strategy with backwards compatibility - Testing and rollback plans Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Add parseArrayType() helper to parse IEC array declarations like "ARRAY[1..10] OF INT" into proper type definitions with dimensions - Support both base types and user-defined types as array element types - Add extractArrayElementType() to resolve element types from array strings - Improve resolveArray() to properly determine datatype for OPC-UA nodes Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Improve extractArrayElementType regex to support namespaced types with colons and dots (e.g., NS1:MyType, System.Int32) - Make parseArrayType regex more specific to avoid capturing trailing whitespace, using proper identifier pattern - Rename ambiguous variable 'dim' to 'dimensionRange' for clarity Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Consolidate the fallback index resolution logic used by both the debugger and OPC-UA systems into shared utilities. This ensures consistent behavior when resolving variable indexes for user-defined types (which can be either function blocks or structures). Key changes: - Add findDebugVariableWithFallback() to debug-variable-finder.ts - Add getIndexFromMapWithFallback() for polling map lookups - Update resolve-indices.ts to use shared fallback functions - Update debug-tree-traversal.ts to use shared fallback for FB fields - Update workspace-activity-bar to use findVariableIndexWithFallback() - Update workspace-screen.tsx polling to use shared fallback lookups - Extract debug-parser.ts with canonical parsing logic - Remove duplicate parsing logic from parse-debug-file.ts - Remove duplicate logic from debug-tree-builder.ts The unified fallback strategy (FB-style path first, then struct-style path) ensures that if one system can find a variable's index, the other will too. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Move closeModal and submitAutocompletion declarations before useImperativeHandle and add them to the dependency array to ensure the triggerSubmit closure always captures the latest function references. Addresses Copilot review feedback on PR #545. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
…ities refactor: Unify debugger and OPC-UA variable index resolution
…upport feat: Add array type parsing support for OPC-UA configuration
|
Caution Review failedThe pull request is closed. WalkthroughAdds end-to-end OPC‑UA support: canonical debug.c parser, traversal and index-resolution utilities, opcua.json generator integrated into the compiler, extensive renderer UI (server editor, tabs, modals, variable tree), store actions/schemas for OPC‑UA, tests, and documentation; existing debugger parsing usage migrated to shared utilities. Changes
Sequence Diagram(s)sequenceDiagram
participant U as User (Editor)
participant E as OPC‑UA Editor UI
participant S as Redux Store
participant C as Compiler
participant P as Debug.c Parser
participant G as OPC‑UA Config Generator
participant FS as File System
U->>E: configure server, add nodes/profiles/users
E->>S: dispatch OPC‑UA actions (addNode/addUser/addProfile)
S->>S: persist opcuaServerConfig
U->>E: trigger build/compile
E->>C: start compilation
C->>P: read debug.c
P->>C: parseDebugFile -> DebugVariableEntry[]
C->>G: generateOpcUaConfig(servers, debugContent, instances)
G->>G: resolveVariableIndex / resolveStructureIndices / resolveArrayIndex
G->>G: build runtime JSON (opcua.json)
G->>C: return opcua.json
C->>FS: write conf/opcua.json
C->>E: compilation result / errors
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 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. ✨ Finishing touches
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 |
There was a problem hiding this comment.
Pull request overview
This PR adds comprehensive OPC-UA server configuration support to OpenPLC Editor, enabling users to configure OPC-UA servers for PLC runtime communication with advanced security, authentication, and complex type support.
Changes:
- OPC-UA server configuration UI with tabs for general settings, security profiles, users/certificates, and address space management
- Complex type support (structures, function blocks, arrays) with recursive field resolution
- Shared debug utilities refactoring using canonical debug.c parsing and visitor pattern for tree traversal
Reviewed changes
Copilot reviewed 44 out of 45 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| src/utils/pou-helpers.ts | Shared POU lookup utilities for variable iteration and type resolution |
| src/utils/opcua/types.ts | Type definitions for OPC-UA configuration and debug variables |
| src/utils/opcua/resolve-indices.ts | Debug variable index resolution for OPC-UA nodes with fallback strategies |
| src/utils/opcua/generate-opcua-config.ts | Runtime configuration generation and validation for OPC-UA server |
| src/utils/debug-parser.ts | Canonical debug.c parser shared between debugger and OPC-UA |
| src/utils/debug-variable-finder.ts | Shared debug variable index lookup with FB-style/struct-style fallback |
| src/utils/debug-tree-traversal.ts | Visitor pattern tree traversal for complex PLC types |
| src/renderer/utils/parse-debug-file.ts | Re-exports from canonical debug-parser for backwards compatibility |
| src/renderer/utils/debug-tree-builder.ts | Refactored to use shared traversal utilities |
| src/renderer/screens/workspace-screen.tsx | Updated to use shared debug variable lookup utilities |
| src/renderer/store/slices/project/slice.ts | Added OPC-UA server configuration actions and default config |
| src/renderer/components/.../opcua-server/index.tsx | Main OPC-UA server editor with tabbed interface |
| src/types/PLC/open-plc.ts | Added OPC-UA configuration type schemas |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| const baseCheck = baseTypeSchema.safeParse(parsedType.toLowerCase()) |
There was a problem hiding this comment.
The code parses array types before checking base types, but then checks base types again. This could lead to unnecessary parsing if the array check succeeds. Consider restructuring to avoid redundant checks.
| const closeModal = () => { | ||
| setAutocompleteFocus(false) | ||
| setSelectedVariable({ positionInArray: -1, variable: { id: '', name: '' } }) | ||
| if (setIsOpen) setIsOpen(false) | ||
| } | ||
|
|
||
| const submitAutocompletion = ({ variable }: { variable: { id: string; name: string } }) => { | ||
| closeModal() | ||
| submit({ variable }) | ||
| } |
There was a problem hiding this comment.
Functions closeModal and submitAutocompletion are defined inside the component but not wrapped in useCallback. The dependency array at line 124 includes these functions, which will cause the useImperativeHandle to re-run on every render. Wrap these functions in useCallback with proper dependencies.
| setPort(remoteDevice.modbusTcpConfig.port.toString()) | ||
| setTimeoutMs(remoteDevice.modbusTcpConfig.timeout.toString()) | ||
| setSlaveId((remoteDevice.modbusTcpConfig.slaveId ?? 1).toString()) | ||
| setSlaveId(String(remoteDevice.modbusTcpConfig.slaveId ?? 1)) |
There was a problem hiding this comment.
Using String() constructor for number-to-string conversion. For consistency with the codebase (lines 363-364 use .toString()), consider using (remoteDevice.modbusTcpConfig.slaveId ?? 1).toString() instead.
| setSlaveId(String(remoteDevice.modbusTcpConfig.slaveId ?? 1)) | |
| setSlaveId((remoteDevice.modbusTcpConfig.slaveId ?? 1).toString()) |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 14
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/utils/generate-iec-string-to-variables.ts (1)
136-176: Arrays of function blocks are currently misclassified as user-data-type.
The array branch returns before FB detection, soARRAY[...] OF <FB>ends up withbaseType.definition = 'user-data-type'instead of'derived'. That can break FB-field resolution downstream. Please classify array base types using the same FB detection logic you use for non-arrays.🐛 One way to align FB detection for arrays and scalars
- const parsedType = type.trim() - - // Check if it's an array type first - const arrayType = parseArrayType(parsedType) - if (arrayType) { - variables.push({ - name: name.trim(), - class: currentClass, - type: arrayType, - location: location ? location.trim() : '', - initialValue: initialValue ? initialValue.trim() : null, - documentation: documentation ? documentation.trim() : '', - debug: false, - }) - return - } - - const baseCheck = baseTypeSchema.safeParse(parsedType.toLowerCase()) - - const isUserFunctionBlock = pous?.some( - (pou) => pou.type === 'function-block' && pou.data.name.toLowerCase() === parsedType.toLowerCase(), - ) - - const isSystemFunctionBlock = libraries?.system.some((lib) => { - if (!hasLibraryPous(lib)) return false - return lib.pous.some( - (pou) => pou.type === 'function-block' && pou.name.toLowerCase() === parsedType.toLowerCase(), - ) - }) - - const isUserLibraryFunctionBlock = libraries?.user.some( - (lib) => lib.type === 'function-block' && lib.name.toLowerCase() === parsedType.toLowerCase(), - ) - - const isFunctionBlock = isUserFunctionBlock || isSystemFunctionBlock || isUserLibraryFunctionBlock + const parsedType = type.trim() + + const isFunctionBlockType = (typeName: string) => { + const normalized = typeName.toLowerCase() + const isUserFunctionBlock = pous?.some( + (pou) => pou.type === 'function-block' && pou.data.name.toLowerCase() === normalized, + ) + const isSystemFunctionBlock = libraries?.system.some((lib) => { + if (!hasLibraryPous(lib)) return false + return lib.pous.some( + (pou) => pou.type === 'function-block' && pou.name.toLowerCase() === normalized, + ) + }) + const isUserLibraryFunctionBlock = libraries?.user.some( + (lib) => lib.type === 'function-block' && lib.name.toLowerCase() === normalized, + ) + return isUserFunctionBlock || isSystemFunctionBlock || isUserLibraryFunctionBlock + } + + // Check if it's an array type first + const arrayType = parseArrayType(parsedType) + if (arrayType) { + if ( + arrayType.data.baseType.definition === 'user-data-type' && + isFunctionBlockType(arrayType.data.baseType.value) + ) { + arrayType.data.baseType = { + definition: 'derived' as const, + value: arrayType.data.baseType.value, + } + } + variables.push({ + name: name.trim(), + class: currentClass, + type: arrayType, + location: location ? location.trim() : '', + initialValue: initialValue ? initialValue.trim() : null, + documentation: documentation ? documentation.trim() : '', + debug: false, + }) + return + } + + const baseCheck = baseTypeSchema.safeParse(parsedType.toLowerCase()) + const isFunctionBlock = isFunctionBlockType(parsedType)
🤖 Fix all issues with AI agents
In `@docs/opcua-server-configuration/01-design-overview.md`:
- Around line 109-113: Update the documentation references that mention the old
parser path and parseDebugFile(): replace mentions of parseDebugFile() from
src/renderer/utils/parse-debug-file.ts with the canonical parser
`@root/utils/debug-parser` (src/utils/debug-parser.ts) and update the debugger
integration bullet and file-structure listing accordingly so the docs reference
the debug-parser module and its exported parser functions instead of the old
path; also apply the same change to the other occurrence around lines 356-358.
- Around line 7-43: The fenced diagram block in 01-design-overview.md is missing
a language tag causing markdownlint MD040; update the triple-backtick fence
surrounding the ASCII diagram to include the language identifier (use "text") so
it becomes ```text ... ``` to satisfy the linter and preserve the diagram
formatting.
In `@docs/opcua-server-configuration/02-ui-screen-specifications.md`:
- Around line 7-23: The fenced code blocks in the OPC-UA UI screen spec are
missing language tags (markdownlint MD040); update each triple-backtick fence
that contains the ASCII UI diagram to use ```text and any real code samples to
use ```typescript or ```json as appropriate so the diagram block shown in the
diff becomes ```text ... ``` and any code snippets are labeled to satisfy the
linter and improve readability.
In `@docs/opcua-server-configuration/03-json-configuration-mapping.md`:
- Around line 467-468: The docs snippets import the parser from the old module;
update all instances where parseDebugFile and DebugVariable are imported from
"@root/renderer/utils/parse-debug-file" to instead import from the shared module
"@root/utils/debug-parser" so the snippets use the current parser export names
(parseDebugFile, DebugVariable); change each import statement accordingly (also
update the other occurrence noted around the later snippet).
In `@docs/opcua-server-configuration/04-implementation-phases.md`:
- Around line 9-17: The markdown file contains fenced code blocks without
language identifiers; update each fenced block to include a language tag—use
"text" for diagram/ASCII-art blocks and "typescript" for code samples—so
markdownlint MD040 is satisfied; specifically add these tags to the blocks
immediately following the phases table and to the other fenced blocks noted in
the review (including the ones later in the document referenced by the
reviewer).
In
`@src/renderer/components/_features/`[workspace]/editor/server/opcua-server/components/certificate-modal.tsx:
- Around line 34-47: The validation fails on PEMs with CRLF because carriage
returns remain in base64Content; before the length check and calling atob()
remove CRLF characters (e.g., normalize base64Content =
base64Content.replace(/\r/g, '')) and then perform the length check and try
atob() on the normalized string; update the references to base64Content and the
atob() call in this block so all checks use the cleaned value.
In
`@src/renderer/components/_features/`[workspace]/editor/server/opcua-server/components/security-profile-modal.tsx:
- Around line 118-132: In handleSecurityPolicyChange, when switching from 'None'
to a secure policy the current logic filters out 'Anonymous' via
setAuthMethods(prev => prev.filter(...)) which can leave authMethods empty;
update that setter to check the filtered result and, if empty, return a default
auth method (e.g., ['Username'] or ['Certificate']) so the form remains valid,
and keep the setSecurityMode change (setSecurityMode(prev => (prev === 'None' ?
'SignAndEncrypt' : prev))) as-is; reference handleSecurityPolicyChange,
setAuthMethods and setSecurityMode to locate and change the logic accordingly.
In
`@src/renderer/components/_features/`[workspace]/editor/server/opcua-server/components/user-modal.tsx:
- Around line 117-122: The password validation in user-modal.tsx currently
enforces a 4-character minimum (checks inside the password handling block
referencing password, confirmPassword, and errors); update that check to require
at least 8 characters and add a simple complexity rule (e.g., require one
uppercase, one lowercase, one digit/symbol) before pushing errors, and update
the error messages accordingly (e.g., "Password must be at least 8 characters"
and "Password must include uppercase, lowercase, and a number/symbol"); ensure
the existing confirmPassword comparison (password !== confirmPassword) remains
after the new checks so users still must match.
- Around line 32-40: The current hashPassword function uses plain SHA-256
without salting or stretching; replace its implementation (hashPassword) to use
a purpose-built password hasher (preferably Argon2id via node-argon2) configured
with memory ≥ 19 MiB and timeCost ≈ 2 and tuned to produce ~100–500ms hashes on
target hardware, and ensure a unique salt is applied and stored with the hash;
if Argon2 is unavailable use bcrypt with cost >= 12 as a fallback; remove the
Web Crypto SHA-256 logic, make hashPassword async to call the chosen library,
and ensure you also provide a verify function that uses the library's verify
method to validate passwords against stored salted hashes.
In
`@src/renderer/components/_features/`[workspace]/editor/server/opcua-server/components/variable-config-modal.tsx:
- Around line 233-261: When initializing the modal in the useEffect that checks
isOpen && variable, handle legacy configs that have no existingConfig.fields by
backfilling defaults: if existingConfig exists but existingConfig.fields is
falsy or empty and isComplexType(variable), call
generateDefaultFieldConfigs(variable, defaultPerms) and pass the result to
setFieldConfigs instead of setting an empty array; otherwise use
existingConfig.fields as before. Reference the existing symbols setFieldConfigs,
existingConfig, isComplexType, generateDefaultFieldConfigs, variable and ensure
defaultPerms (OpcUaPermissions) uses the same viewer/operator/engineer values
you set for the parent config so permissions remain consistent.
In `@src/types/PLC/open-plc.ts`:
- Around line 409-410: Update the OpcUaSecurityPolicySchema/OpcUaSecurityPolicy
definition to address deprecated policies: either remove 'Basic128Rsa15' and
'Basic256' from the z.enum so only modern policies (e.g., 'None' and
'Basic256Sha256' or newer) remain, or if backward-compatibility is required add
a clear TODO/comment above OpcUaSecurityPolicySchema explaining why the
deprecated values are retained, their security implications per OPC UA 1.04
(SHA-1 vulnerabilities), and when they should be removed; ensure the comment
references OpcUaSecurityPolicySchema and OpcUaSecurityPolicy so future readers
know this is an intentional decision.
In `@src/utils/debug-tree-traversal.ts`:
- Around line 101-105: parseArrayDimension currently only accepts unsigned
digits; update it to trim input, accept optional whitespace around and between
bounds and allow an optional leading + or - sign for each bound (e.g. support
patterns like "-5 .. 10" or " 0..-3 "). Use a regex that captures signed
integers with optional spaces, parse each with parseInt(..., 10) (or Number) and
then validate the range (return null if parsing fails or start > end). Keep the
function name parseArrayDimension and ensure it still returns [number, number] |
null when invalid.
In `@src/utils/opcua/resolve-indices.ts`:
- Around line 19-29: The function debugTypeToIecType incorrectly checks for the
shorter suffix first so _P_ENUM/_O_ENUM are treated as _ENUM; update
debugTypeToIecType to check for the longer suffixes (“_P_ENUM” or “_O_ENUM”)
before checking “_ENUM”, and keep the existing slice lengths (-7 for _P/_O
suffixes, -5 for _ENUM) so inputs like INT_P_ENUM return INT rather than INT_P.
In `@src/utils/pou-helpers.ts`:
- Around line 146-199: findLeafVariables can recurse infinitely on circular type
references; add cycle detection by introducing a visited set (e.g., visited:
Set<string> = new Set()) as an additional optional parameter to
findLeafVariables, normalize type names when adding/checking (toLowerCase or
consistent casing), check if typeName is already in visited before recursing and
skip recursive calls if so, and add typeName to visited before performing
recursive calls (pass the same visited set down) so nested calls share the
history and cycles are prevented.
🧹 Nitpick comments (17)
src/utils/generate-iec-string-to-variables.ts (1)
45-47: Support sized STRING/WSTRING array base types in the array regex.
parseArrayTypeonly matches a bare identifier for the base type, soARRAY[1..10] OF STRING[80](orWSTRING(80)) won’t be recognized as an array and will fall through as a non-array type. If your IEC input includes sized string types, this will silently drop array metadata. Consider allowing an optional size suffix (and, if needed, normalizing the base type for baseTypeSchema).♻️ Suggested regex expansion
- const arrayMatch = typeStr.match(/^ARRAY\s*\[([^\]]+)\]\s+OF\s+([A-Za-z_][\w.]*)\s*$/i) + const arrayMatch = typeStr.match( + /^ARRAY\s*\[([^\]]+)\]\s+OF\s+([A-Za-z_][\w.]*)(?:\s*(?:\[(\d+)\]|\((\d+)\)))?\s*$/i, + )src/renderer/components/_features/[workspace]/editor/server/opcua-server/index.tsx (3)
1-8: Unused imports detected.
InputWithRef,Label, andSelectcomponents are imported but only used in theGeneralSettingsTabcomponent defined later in this file. While this works, consider extractingGeneralSettingsTabto its own file (similar to other tab components likeAddressSpaceTab,SecurityProfilesTab) to improve code organization and follow the pattern established by other tab components.
67-80: Potential issue:localConfignot reset when server changes.If the user switches between different OPC-UA servers,
localConfigis updated via theuseEffectwhenconfigchanges. However, if the server is removed or not found,configbecomesundefined, but the effect only runs whenconfigis truthy. This could leave stalelocalConfigstate.Consider handling the case where
configbecomesundefined:useEffect(() => { if (config) { setLocalConfig(config) + } else { + setLocalConfig(null) } }, [config])
235-255: Silent validation failures may confuse users.When port or cycle time validation fails, the code silently resets to the previous value without user feedback. Consider adding visual feedback (e.g., brief toast, input highlight, or inline error message) so users understand why their input was rejected.
src/renderer/components/_features/[workspace]/editor/server/opcua-server/components/address-space-tab.tsx (2)
37-40:selectedVariableIdsinitialization may go out of sync.The
useStateinitializer runs only once on mount. Ifconfig.addressSpace.nodeschanges after mount (e.g., external updates or undo/redo),selectedVariableIdswon't be updated. Consider syncing this state with the config using auseEffect:+ useEffect(() => { + setSelectedVariableIds(new Set(config.addressSpace.nodes.map((n) => `${n.pouName}-${n.variablePath}`))) + }, [config.addressSpace.nodes])Alternatively, derive
selectedVariableIdsdirectly withuseMemoif the Set doesn't need to be independently mutable.
185-191: Namespace URI updates on every keystroke.The
onChangehandler callshandleNamespaceChangedirectly, which updates the store and triggersonConfigChange()on every keystroke. This could cause excessive re-renders and store updates. Consider debouncing or using a blur handler pattern (similar toGeneralSettingsTab):+ const [localNamespaceUri, setLocalNamespaceUri] = useState(config.addressSpace.namespaceUri) + + const handleNamespaceBlur = useCallback(() => { + if (localNamespaceUri !== config.addressSpace.namespaceUri) { + handleNamespaceChange(localNamespaceUri) + } + }, [localNamespaceUri, config.addressSpace.namespaceUri, handleNamespaceChange]) <InputWithRef type='text' - value={config.addressSpace.namespaceUri} - onChange={(e) => handleNamespaceChange(e.target.value)} + value={localNamespaceUri} + onChange={(e) => setLocalNamespaceUri(e.target.value)} + onBlur={handleNamespaceBlur} ... />src/main/modules/compiler/compiler-module.ts (1)
1238-1243: Consider clarifying the fallback behavior.When
debug.ccannot be read, the code logs an error but continues with empty content. If the OPC-UA address space has nodes configured, this will subsequently throw anOpcUaConfigErroringenerateOpcUaConfig(as seen in the external snippet lines 399-405).The current warning message mentions "variable indices may not be resolved" but doesn't indicate that the generation will fail if nodes are configured. Consider making the message clearer:
- handleOutputData('Warning: Could not read debug.c file. OPC-UA variable indices may not be resolved.', 'error') + handleOutputData('Warning: Could not read debug.c file. OPC-UA configuration will fail if address space nodes are configured.', 'error')src/renderer/components/_features/[workspace]/editor/server/opcua-server/components/variable-tree.tsx (1)
68-96: Consider using the existing atomic Checkbox component.A
Checkboxcomponent exists atsrc/renderer/components/_atoms/checkbox/index.tsx. The local implementation here may be intentional for specific styling, but using the shared atomic component would improve consistency across the codebase.If the atomic component doesn't support the
indeterminatestate, consider extending it rather than duplicating.src/renderer/components/_features/[workspace]/editor/server/opcua-server/components/certificates-tab.tsx (1)
27-28: Consider syncing local state with config prop changes.The
customCertandcustomKeystates are initialized fromconfig.security.serverCertificateCustomandconfig.security.serverPrivateKeyCustom, but if these values change from an external source (e.g., undo/redo, external store update), the local state won't reflect those changes.If external synchronization is needed, consider using
useEffectto sync or deriving state from props where possible.src/renderer/components/_features/[workspace]/editor/server/opcua-server/hooks/use-project-variables.ts (1)
58-70: Unused function detected.The
_getDisplayTypefunction is defined but doesn't appear to be used anywhere in this file or exported. Consider removing it if it's not needed, or removing the underscore prefix if it should be used.src/utils/opcua/resolve-indices.ts (2)
74-90: Potential inconsistency in GVL detection logic.Line 75 checks for
node.pouName === 'GVL'and then separatelynode.pouName.toUpperCase() === 'GVL'. The first check is case-sensitive while the third is case-insensitive, but they'd match the same thing whenpouName === 'GVL'. Consider simplifying to a single case-insensitive check for consistency.Suggested simplification
- if (node.pouName === 'GVL' || node.pouName === 'CONFIG' || node.pouName.toUpperCase() === 'GVL') { + const pouNameUpper = node.pouName.toUpperCase() + if (pouNameUpper === 'GVL' || pouNameUpper === 'CONFIG') {
173-218: Non-null assertion oninstanceNameafter null-check branch.At lines 193, 206, and 207,
instanceName!is used with a non-null assertion. This is safe here because the null check at line 162 throws before reaching this code for non-GVL cases. However, the code structure relies on the early return/throw pattern—consider restructuring to make the type narrowing explicit or add a comment explaining the invariant.src/utils/pou-helpers.ts (2)
163-166: Arrays are explicitly skipped without documentation of the deferred handling.The comment says "handled separately" but doesn't indicate where or how. Consider adding a TODO or reference to the handling location for maintainability.
70-90: Type check inconsistency between standard and custom FBs.Line 75 uses
normalizeTypeString(pou.type) === 'functionblock'but line 83 also usesnormalizeTypeString(pou.type) === 'functionblock'followed by an additional checkpou.type === 'function-block'at line 85. The second check uses the literal'function-block'which wouldn't match after normalization removed the hyphen. This appears intentional for type narrowing but is confusing.Clarify the type guard logic
// Check project POUs (user-defined FBs) const customFB = projectPous.find( - (pou) => normalizeTypeString(pou.type) === 'functionblock' && pou.data.name.toUpperCase() === typeNameUpper, + (pou) => pou.type === 'function-block' && pou.data.name.toUpperCase() === typeNameUpper, ) - if (customFB && customFB.type === 'function-block') { + if (customFB) { return customFB.data.variables as PouVariable[] }src/renderer/utils/debug-tree-builder.ts (1)
134-134: Case-sensitive path lookup for external variables.Line 134 uses exact match:
debugVariables.find((dv) => dv.name === fullPath). However,buildGlobalDebugPathconverts to uppercase (line 130), and the sharedfindDebugVariablefunction uses case-insensitive comparison. Consider usingfindDebugVariablehere for consistency.Use consistent lookup
+import { findDebugVariable } from '@root/utils/debug-variable-finder' // ... if (variable.type.definition === 'base-type') { - const debugVar = debugVariables.find((dv) => dv.name === fullPath) + const debugVar = findDebugVariable(debugVariables, fullPath)src/renderer/components/_features/[workspace]/editor/server/opcua-server/components/user-modal.tsx (1)
251-257: Emoji icons for show/hide password may have accessibility issues.Using 🙈 and 👁 emojis for the password visibility toggle may not be screen-reader friendly. Consider using an icon component with appropriate aria-label.
Suggested accessibility improvement
<button type='button' onClick={() => setShowPassword(!showPassword)} className='absolute right-2 top-1/2 -translate-y-1/2 text-neutral-500 hover:text-neutral-700 dark:hover:text-neutral-300' + aria-label={showPassword ? 'Hide password' : 'Show password'} > {showPassword ? '🙈' : '👁'} </button>src/renderer/store/slices/project/slice.ts (1)
1621-1639: Consider protecting against prototype pollution in deep merge.The custom
mergeDeepfunction does not guard against prototype pollution via keys like__proto__,constructor, orprototype. While this is mitigated by Immer's structural sharing and the fact that the source comes from trusted UI code, adding a safeguard would be prudent for defense-in-depth.♻️ Suggested safeguard
const mergeDeep = (target: Record<string, unknown>, source: Record<string, unknown>) => { for (const key of Object.keys(source)) { + // Skip prototype pollution vectors + if (key === '__proto__' || key === 'constructor' || key === 'prototype') { + continue + } if (source[key] !== undefined) {
| ``` | ||
| ┌─────────────────────────────────────────────────────────────────────────┐ | ||
| │ OpenPLC Editor │ | ||
| │ ┌─────────────────────────────────────────────────────────────────┐ │ | ||
| │ │ OPC-UA Configuration UI │ │ | ||
| │ │ ┌──────────┐ ┌──────────┐ ┌──────────┐ ┌──────────────────┐ │ │ | ||
| │ │ │ General │ │ Security │ │ Users │ │ Address Space │ │ │ | ||
| │ │ │ Settings │ │ Profiles │ │ │ │ (Variables) │ │ │ | ||
| │ │ └──────────┘ └──────────┘ └──────────┘ └──────────────────┘ │ │ | ||
| │ └─────────────────────────────────────────────────────────────────┘ │ | ||
| │ │ │ | ||
| │ ▼ │ | ||
| │ ┌─────────────────────────────────────────────────────────────────┐ │ | ||
| │ │ Project State (Zustand Store) │ │ | ||
| │ │ - OPC-UA config stored WITHOUT indices │ │ | ||
| │ └─────────────────────────────────────────────────────────────────┘ │ | ||
| │ │ │ | ||
| │ ▼ (on compile) │ | ||
| │ ┌─────────────────────────────────────────────────────────────────┐ │ | ||
| │ │ Compiler Module │ │ | ||
| │ │ 1. xml2st generates debug.c (with variable indices) │ │ | ||
| │ │ 2. Parse debug.c to get index mapping │ │ | ||
| │ │ 3. Resolve variable references → indices │ │ | ||
| │ │ 4. Generate opcua.json with resolved indices │ │ | ||
| │ └─────────────────────────────────────────────────────────────────┘ │ | ||
| └─────────────────────────────────────────────────────────────────────────┘ | ||
| │ | ||
| ▼ | ||
| ┌─────────────────────────────────────────────────────────────────────────┐ | ||
| │ OpenPLC Runtime │ | ||
| │ ┌─────────────────────────────────────────────────────────────────┐ │ | ||
| │ │ OPC-UA Plugin │ │ | ||
| │ │ Reads opcua.json configuration │ │ | ||
| │ │ Exposes variables via OPC-UA protocol │ │ | ||
| │ └─────────────────────────────────────────────────────────────────┘ │ | ||
| └─────────────────────────────────────────────────────────────────────────┘ | ||
| ``` |
There was a problem hiding this comment.
Add language tags to fenced blocks (markdownlint MD040).
markdownlint flags fenced blocks without language identifiers. Please add text for diagrams (or the appropriate language for code) to satisfy linting.
Example fix
-```
+```text
...
-```
+```🧰 Tools
🪛 markdownlint-cli2 (0.18.1)
7-7: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 Prompt for AI Agents
In `@docs/opcua-server-configuration/01-design-overview.md` around lines 7 - 43,
The fenced diagram block in 01-design-overview.md is missing a language tag
causing markdownlint MD040; update the triple-backtick fence surrounding the
ASCII diagram to include the language identifier (use "text") so it becomes
```text ... ``` to satisfy the linter and preserve the diagram formatting.
| The OPC-UA plugin uses the **same debug variable indices** as the OpenPLC debugger. This means: | ||
|
|
||
| - We reuse `parseDebugFile()` from `src/renderer/utils/parse-debug-file.ts` | ||
| - We reuse the path matching logic from the debugger implementation | ||
| - Variable indices are consistent between debugger and OPC-UA access |
There was a problem hiding this comment.
Doc references old debug parser path.
The canonical parser now appears to be @root/utils/debug-parser / src/utils/debug-parser.ts. Please update the debugger integration bullet and file-structure listing so the docs match the current location.
Also applies to: 356-358
🤖 Prompt for AI Agents
In `@docs/opcua-server-configuration/01-design-overview.md` around lines 109 -
113, Update the documentation references that mention the old parser path and
parseDebugFile(): replace mentions of parseDebugFile() from
src/renderer/utils/parse-debug-file.ts with the canonical parser
`@root/utils/debug-parser` (src/utils/debug-parser.ts) and update the debugger
integration bullet and file-structure listing accordingly so the docs reference
the debug-parser module and its exported parser functions instead of the old
path; also apply the same change to the other occurrence around lines 356-358.
| ``` | ||
| ┌─────────────────────────────────────────────────────────────────────────┐ | ||
| │ OPC-UA Server Configuration [X] Close │ | ||
| ├─────────────────────────────────────────────────────────────────────────┤ | ||
| │ ┌──────────┬──────────┬──────────┬──────────┬───────────────────────┐ │ | ||
| │ │ General │ Security │ Users │ Certifi- │ Address Space │ │ | ||
| │ │ Settings │ Profiles │ │ cates │ │ │ | ||
| │ └──────────┴──────────┴──────────┴──────────┴───────────────────────┘ │ | ||
| │ ┌─────────────────────────────────────────────────────────────────┐ │ | ||
| │ │ │ │ | ||
| │ │ [Active Tab Content] │ │ | ||
| │ │ │ │ | ||
| │ │ │ │ | ||
| │ └─────────────────────────────────────────────────────────────────┘ │ | ||
| └─────────────────────────────────────────────────────────────────────────┘ | ||
| ``` | ||
|
|
There was a problem hiding this comment.
Add language tags to fenced blocks (markdownlint MD040).
markdownlint reports multiple fences without a language. Please label ASCII diagrams as text and code samples as typescript/json to satisfy linting and improve readability.
Example fix
-```
+```text
...
-```
+```📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| ``` | |
| ┌─────────────────────────────────────────────────────────────────────────┐ | |
| │ OPC-UA Server Configuration [X] Close │ | |
| ├─────────────────────────────────────────────────────────────────────────┤ | |
| │ ┌──────────┬──────────┬──────────┬──────────┬───────────────────────┐ │ | |
| │ │ General │ Security │ Users │ Certifi- │ Address Space │ │ | |
| │ │ Settings │ Profiles │ │ cates │ │ │ | |
| │ └──────────┴──────────┴──────────┴──────────┴───────────────────────┘ │ | |
| │ ┌─────────────────────────────────────────────────────────────────┐ │ | |
| │ │ │ │ | |
| │ │ [Active Tab Content] │ │ | |
| │ │ │ │ | |
| │ │ │ │ | |
| │ └─────────────────────────────────────────────────────────────────┘ │ | |
| └─────────────────────────────────────────────────────────────────────────┘ | |
| ``` |
🧰 Tools
🪛 markdownlint-cli2 (0.18.1)
7-7: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 Prompt for AI Agents
In `@docs/opcua-server-configuration/02-ui-screen-specifications.md` around lines
7 - 23, The fenced code blocks in the OPC-UA UI screen spec are missing language
tags (markdownlint MD040); update each triple-backtick fence that contains the
ASCII UI diagram to use ```text and any real code samples to use ```typescript
or ```json as appropriate so the diagram block shown in the diff becomes ```text
... ``` and any code snippets are labeled to satisfy the linter and improve
readability.
| import { parseDebugFile, DebugVariable } from '@root/renderer/utils/parse-debug-file' | ||
|
|
There was a problem hiding this comment.
Update parseDebugFile import path in snippets.
Snippets still import from @root/renderer/utils/parse-debug-file, but the shared parser is now @root/utils/debug-parser. Updating these references will keep the docs aligned with the current codebase.
Also applies to: 608-609
🤖 Prompt for AI Agents
In `@docs/opcua-server-configuration/03-json-configuration-mapping.md` around
lines 467 - 468, The docs snippets import the parser from the old module; update
all instances where parseDebugFile and DebugVariable are imported from
"@root/renderer/utils/parse-debug-file" to instead import from the shared module
"@root/utils/debug-parser" so the snippets use the current parser export names
(parseDebugFile, DebugVariable); change each import statement accordingly (also
update the other occurrence noted around the later snippet).
| | Phase | Focus | Deliverable | | ||
| |-------|-------|-------------| | ||
| | 1 | Foundation & Types | Type definitions, project integration, basic UI shell | | ||
| | 2 | General Settings & Security Profiles | First two tabs fully functional | | ||
| | 3 | Users & Certificates | Authentication and certificate management | | ||
| | 4 | Address Space - Basic Variables | Variable tree and simple variable selection | | ||
| | 5 | Address Space - Complex Types | Structures, arrays, nested FBs | | ||
| | 6 | Compiler Integration & Testing | JSON generation, end-to-end testing | | ||
|
|
There was a problem hiding this comment.
Add language tags to fenced blocks (markdownlint MD040).
markdownlint flags fenced blocks without a language identifier. Please add text for diagrams and typescript for code samples to satisfy lint rules.
Example fix
-```
+```text
...
-```
+```Also applies to: 365-367
🧰 Tools
🪛 markdownlint-cli2 (0.18.1)
9-9: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 Prompt for AI Agents
In `@docs/opcua-server-configuration/04-implementation-phases.md` around lines 9 -
17, The markdown file contains fenced code blocks without language identifiers;
update each fenced block to include a language tag—use "text" for
diagram/ASCII-art blocks and "typescript" for code samples—so markdownlint MD040
is satisfied; specifically add these tags to the blocks immediately following
the phases table and to the other fenced blocks noted in the review (including
the ones later in the document referenced by the reviewer).
| // Initialize form when modal opens or variable changes | ||
| useEffect(() => { | ||
| if (isOpen && variable) { | ||
| if (existingConfig) { | ||
| // Editing existing configuration | ||
| setNodeId(existingConfig.nodeId) | ||
| setBrowseName(existingConfig.browseName) | ||
| setDisplayName(existingConfig.displayName) | ||
| setDescription(existingConfig.description) | ||
| setInitialValue(String(existingConfig.initialValue)) | ||
| setViewerPerm(existingConfig.permissions.viewer) | ||
| setOperatorPerm(existingConfig.permissions.operator) | ||
| setEngineerPerm(existingConfig.permissions.engineer) | ||
| // Load existing field configs | ||
| setFieldConfigs(existingConfig.fields || []) | ||
| } else { | ||
| // New configuration - generate defaults | ||
| setNodeId(generateNodeId(variable.pouName, variable.variablePath)) | ||
| setBrowseName(generateBrowseName(variable.variablePath)) | ||
| setDisplayName(generateDisplayName(variable.variablePath)) | ||
| setDescription('') | ||
| setInitialValue(String(getDefaultInitialValue(variable.variableType))) | ||
| setViewerPerm('r') | ||
| setOperatorPerm('r') | ||
| setEngineerPerm('rw') | ||
| // Generate default field configs for complex types | ||
| const defaultPerms: OpcUaPermissions = { viewer: 'r', operator: 'r', engineer: 'rw' } | ||
| setFieldConfigs(isComplexType(variable) ? generateDefaultFieldConfigs(variable, defaultPerms) : []) | ||
| } |
There was a problem hiding this comment.
Backfill field configs when editing legacy complex nodes.
If a complex node’s existing config lacks fields, the permissions table disappears and saving persists an empty list. Consider regenerating defaults as a fallback for legacy/manual configs.
♻️ Proposed fix
if (existingConfig) {
// Editing existing configuration
setNodeId(existingConfig.nodeId)
setBrowseName(existingConfig.browseName)
setDisplayName(existingConfig.displayName)
setDescription(existingConfig.description)
setInitialValue(String(existingConfig.initialValue))
setViewerPerm(existingConfig.permissions.viewer)
setOperatorPerm(existingConfig.permissions.operator)
setEngineerPerm(existingConfig.permissions.engineer)
- // Load existing field configs
- setFieldConfigs(existingConfig.fields || [])
+ // Load existing field configs (fallback for legacy/empty complex configs)
+ if (isComplexType(variable) && (existingConfig.fields?.length ?? 0) === 0) {
+ setFieldConfigs(generateDefaultFieldConfigs(variable, existingConfig.permissions))
+ } else {
+ setFieldConfigs(existingConfig.fields || [])
+ }
} else {📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // Initialize form when modal opens or variable changes | |
| useEffect(() => { | |
| if (isOpen && variable) { | |
| if (existingConfig) { | |
| // Editing existing configuration | |
| setNodeId(existingConfig.nodeId) | |
| setBrowseName(existingConfig.browseName) | |
| setDisplayName(existingConfig.displayName) | |
| setDescription(existingConfig.description) | |
| setInitialValue(String(existingConfig.initialValue)) | |
| setViewerPerm(existingConfig.permissions.viewer) | |
| setOperatorPerm(existingConfig.permissions.operator) | |
| setEngineerPerm(existingConfig.permissions.engineer) | |
| // Load existing field configs | |
| setFieldConfigs(existingConfig.fields || []) | |
| } else { | |
| // New configuration - generate defaults | |
| setNodeId(generateNodeId(variable.pouName, variable.variablePath)) | |
| setBrowseName(generateBrowseName(variable.variablePath)) | |
| setDisplayName(generateDisplayName(variable.variablePath)) | |
| setDescription('') | |
| setInitialValue(String(getDefaultInitialValue(variable.variableType))) | |
| setViewerPerm('r') | |
| setOperatorPerm('r') | |
| setEngineerPerm('rw') | |
| // Generate default field configs for complex types | |
| const defaultPerms: OpcUaPermissions = { viewer: 'r', operator: 'r', engineer: 'rw' } | |
| setFieldConfigs(isComplexType(variable) ? generateDefaultFieldConfigs(variable, defaultPerms) : []) | |
| } | |
| // Initialize form when modal opens or variable changes | |
| useEffect(() => { | |
| if (isOpen && variable) { | |
| if (existingConfig) { | |
| // Editing existing configuration | |
| setNodeId(existingConfig.nodeId) | |
| setBrowseName(existingConfig.browseName) | |
| setDisplayName(existingConfig.displayName) | |
| setDescription(existingConfig.description) | |
| setInitialValue(String(existingConfig.initialValue)) | |
| setViewerPerm(existingConfig.permissions.viewer) | |
| setOperatorPerm(existingConfig.permissions.operator) | |
| setEngineerPerm(existingConfig.permissions.engineer) | |
| // Load existing field configs (fallback for legacy/empty complex configs) | |
| if (isComplexType(variable) && (existingConfig.fields?.length ?? 0) === 0) { | |
| setFieldConfigs(generateDefaultFieldConfigs(variable, existingConfig.permissions)) | |
| } else { | |
| setFieldConfigs(existingConfig.fields || []) | |
| } | |
| } else { | |
| // New configuration - generate defaults | |
| setNodeId(generateNodeId(variable.pouName, variable.variablePath)) | |
| setBrowseName(generateBrowseName(variable.variablePath)) | |
| setDisplayName(generateDisplayName(variable.variablePath)) | |
| setDescription('') | |
| setInitialValue(String(getDefaultInitialValue(variable.variableType))) | |
| setViewerPerm('r') | |
| setOperatorPerm('r') | |
| setEngineerPerm('rw') | |
| // Generate default field configs for complex types | |
| const defaultPerms: OpcUaPermissions = { viewer: 'r', operator: 'r', engineer: 'rw' } | |
| setFieldConfigs(isComplexType(variable) ? generateDefaultFieldConfigs(variable, defaultPerms) : []) | |
| } |
🤖 Prompt for AI Agents
In
`@src/renderer/components/_features/`[workspace]/editor/server/opcua-server/components/variable-config-modal.tsx
around lines 233 - 261, When initializing the modal in the useEffect that checks
isOpen && variable, handle legacy configs that have no existingConfig.fields by
backfilling defaults: if existingConfig exists but existingConfig.fields is
falsy or empty and isComplexType(variable), call
generateDefaultFieldConfigs(variable, defaultPerms) and pass the result to
setFieldConfigs instead of setting an empty array; otherwise use
existingConfig.fields as before. Reference the existing symbols setFieldConfigs,
existingConfig, isComplexType, generateDefaultFieldConfigs, variable and ensure
defaultPerms (OpcUaPermissions) uses the same viewer/operator/engineer values
you set for the parent config so permissions remain consistent.
| const OpcUaSecurityPolicySchema = z.enum(['None', 'Basic128Rsa15', 'Basic256', 'Basic256Sha256']) | ||
| type OpcUaSecurityPolicy = z.infer<typeof OpcUaSecurityPolicySchema> |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
OPC UA Basic128Rsa15 Basic256 security policies deprecated 2025
💡 Result:
Short answer: Yes — the OPC UA security policies Basic128Rsa15 and Basic256 are deprecated and should not be used in new or production deployments; they were marked deprecated in the OPC UA 1.04 specification and many SDKs/vendors now disable or warn about them (use Basic256Sha256, Aes128_Sha256_RsaOaep or Aes256_Sha256_RsaPss instead). [1][2][3]
Sources:
- OPC Foundation (Part 7 / Profiles, v1.04) — Basic128Rsa15 and Basic256 deprecated (SHA‑1). [1]
- Unified Automation SDK docs — Basic128Rsa15 / Basic256 listed as deprecated; must be explicitly enabled only for backward compatibility. [2]
- Prosys SDK release notes — Basic128Rsa15 and Basic256 deprecated and should be avoided; security issues noted. [3]
🏁 Script executed:
cat -n src/types/PLC/open-plc.ts | sed -n '400,420p'Repository: Autonomy-Logic/openplc-editor
Length of output: 1127
🏁 Script executed:
rg -B 5 -A 5 "OpcUaSecurityPolicy" src/types/PLC/open-plc.tsRepository: Autonomy-Logic/openplc-editor
Length of output: 1570
🏁 Script executed:
rg "OpcUaSecurityPolicy" --type ts --type js -A 3 -B 3Repository: Autonomy-Logic/openplc-editor
Length of output: 1804
Add documentation or remove deprecated OPC UA security policies.
OpcUaSecurityPolicySchema includes Basic128Rsa15 and Basic256, which are deprecated per OPC UA 1.04 specification due to SHA-1 vulnerabilities. They should not appear in this schema without explicit comments clarifying backward compatibility requirements. Either document why they're retained or remove them in favor of Basic256Sha256 and newer alternatives.
🤖 Prompt for AI Agents
In `@src/types/PLC/open-plc.ts` around lines 409 - 410, Update the
OpcUaSecurityPolicySchema/OpcUaSecurityPolicy definition to address deprecated
policies: either remove 'Basic128Rsa15' and 'Basic256' from the z.enum so only
modern policies (e.g., 'None' and 'Basic256Sha256' or newer) remain, or if
backward-compatibility is required add a clear TODO/comment above
OpcUaSecurityPolicySchema explaining why the deprecated values are retained,
their security implications per OPC UA 1.04 (SHA-1 vulnerabilities), and when
they should be removed; ensure the comment references OpcUaSecurityPolicySchema
and OpcUaSecurityPolicy so future readers know this is an intentional decision.
| function parseArrayDimension(dimension: string): [number, number] | null { | ||
| const match = dimension.match(/^(\d+)\.\.(\d+)$/) | ||
| if (!match) return null | ||
| return [parseInt(match[1], 10), parseInt(match[2], 10)] | ||
| } |
There was a problem hiding this comment.
Handle negative/whitespace bounds in array dimensions.
IEC arrays can use negative ranges or include whitespace; the current regex only accepts digits. Consider supporting signed bounds and guarding invalid ranges.
♻️ Proposed fix
-function parseArrayDimension(dimension: string): [number, number] | null {
- const match = dimension.match(/^(\d+)\.\.(\d+)$/)
- if (!match) return null
- return [parseInt(match[1], 10), parseInt(match[2], 10)]
-}
+function parseArrayDimension(dimension: string): [number, number] | null {
+ const match = dimension.trim().match(/^(-?\d+)\.\.(-?\d+)$/)
+ if (!match) return null
+ const start = parseInt(match[1], 10)
+ const end = parseInt(match[2], 10)
+ if (Number.isNaN(start) || Number.isNaN(end) || start > end) return null
+ return [start, end]
+}🤖 Prompt for AI Agents
In `@src/utils/debug-tree-traversal.ts` around lines 101 - 105,
parseArrayDimension currently only accepts unsigned digits; update it to trim
input, accept optional whitespace around and between bounds and allow an
optional leading + or - sign for each bound (e.g. support patterns like "-5 ..
10" or " 0..-3 "). Use a regex that captures signed integers with optional
spaces, parse each with parseInt(..., 10) (or Number) and then validate the
range (return null if parsing fails or start > end). Keep the function name
parseArrayDimension and ensure it still returns [number, number] | null when
invalid.
| const debugTypeToIecType = (debugType: string): string => { | ||
| // Remove _ENUM suffix if present | ||
| if (debugType.endsWith('_ENUM')) { | ||
| return debugType.slice(0, -5) | ||
| } | ||
| // Remove _P_ENUM or _O_ENUM suffix for pointer/output types | ||
| if (debugType.endsWith('_P_ENUM') || debugType.endsWith('_O_ENUM')) { | ||
| return debugType.slice(0, -7) | ||
| } | ||
| return debugType | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Search for ENUM suffix patterns in the codebase
rg -n '_ENUM' --type=ts -C2Repository: Autonomy-Logic/openplc-editor
Length of output: 7306
🏁 Script executed:
#!/bin/bash
# Look for any documentation or references about debug type suffixes
rg -n 'debug.*[Tt]ype|_ENUM|_P_ENUM|_O_ENUM|_F_ENUM' --type=ts --type=md -C1 | head -100Repository: Autonomy-Logic/openplc-editor
Length of output: 8120
🏁 Script executed:
#!/bin/bash
# Check for test files that might show expected type conversions
fd -e 'test\.' -o -e '\.spec\.' -o -e '__tests__' | head -20Repository: Autonomy-Logic/openplc-editor
Length of output: 189
🏁 Script executed:
#!/bin/bash
# Find and examine the resolve-indices.ts file more thoroughly
cat -n src/utils/opcua/resolve-indices.ts | head -40Repository: Autonomy-Logic/openplc-editor
Length of output: 1511
Fix condition order: _P_ENUM and _O_ENUM checks must come before _ENUM.
The current logic has a critical bug. Since _P_ENUM and _O_ENUM both end with _ENUM, the first condition catches them and removes only 5 characters instead of 7, yielding incorrect results. For example, INT_P_ENUM would return INT_P instead of INT. The second condition is unreachable. Swap the conditions to check for the longer suffixes first:
Correct order
const debugTypeToIecType = (debugType: string): string => {
// Remove _P_ENUM or _O_ENUM suffix for pointer/output types
if (debugType.endsWith('_P_ENUM') || debugType.endsWith('_O_ENUM')) {
return debugType.slice(0, -7)
}
// Remove _ENUM suffix if present
if (debugType.endsWith('_ENUM')) {
return debugType.slice(0, -5)
}
return debugType
}🤖 Prompt for AI Agents
In `@src/utils/opcua/resolve-indices.ts` around lines 19 - 29, The function
debugTypeToIecType incorrectly checks for the shorter suffix first so
_P_ENUM/_O_ENUM are treated as _ENUM; update debugTypeToIecType to check for the
longer suffixes (“_P_ENUM” or “_O_ENUM”) before checking “_ENUM”, and keep the
existing slice lengths (-7 for _P/_O suffixes, -5 for _ENUM) so inputs like
INT_P_ENUM return INT rather than INT_P.
thiagoralves
force-pushed
the
feat/opcua-server-configuration
branch
2 times, most recently
from
51deee5 to
bd99357
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In
`@src/renderer/components/_features/`[workspace]/editor/server/opcua-server/components/address-space-tab.tsx:
- Around line 156-170: handleRemoveNode currently only deletes the parent key
from selectedVariableIds, leaving descendant IDs selected; update
handleRemoveNode (the callback that calls removeOpcUaNode and updates
selectedVariableIds) to also remove all descendant IDs the same way
handleVariableSelect does: compute the parent key
`${node.pouName}-${node.variablePath}` and, when building the new Set, iterate
over config.addressSpace.nodes to remove any keys that represent descendants
(e.g., same pouName and variablePath starting with the parent's variablePath +
separator) so all child selections are cleared before calling onConfigChange.
🧹 Nitpick comments (1)
src/renderer/components/_features/[workspace]/editor/server/opcua-server/components/address-space-tab.tsx (1)
172-266: LGTM!The two-pane layout is well-structured with proper overflow handling. Dark mode styling is consistent throughout. The slightly different filter input styles (line 216) are reasonable for the smaller input context.
Consider extracting the filter input styles (line 216) into a named constant similar to
inputStylesfor clarity, especially since they differ slightly (e.g.,h-[28px]vsh-[30px],text-xsvstext-cp-sm).
thiagoralves
force-pushed
the
feat/opcua-server-configuration
branch
from
bd99357 to
efb2cd4
Compare
Add recursive field support to allow nested FB instances and structs to appear as hierarchical OPC-UA Object nodes instead of flat fields. Changes: - Update OpcUaFieldConfig schema to support recursive nested fields - Update ResolvedField interface to support nested fields with null index - Add resolveFieldRecursively helper for hierarchical field resolution - Update resolveStructure to convert nested fields to runtime format - Update tests with required datatype field Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Clarify that fields property can be undefined or empty for leaf fields - Add note explaining path construction is for new hierarchical configs only Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
feat: Add nested structures support for OPC-UA
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In
`@src/renderer/components/_features/`[workspace]/editor/server/opcua-server/components/variable-config-modal.tsx:
- Around line 32-35: In generateNodeId, the replacement cleanPath uses
.replace(/\./g, '.') which is a no-op; change it to replace dots with the
intended character (e.g., underscore) so node IDs are sanitized. Update the
cleanPath logic in the generateNodeId function (referencing pouName and
variablePath) to replace '.' with '_' (and keep the existing replacements for
'[' and ']') so the returned `PLC.${pouName}.${cleanPath}` produces a valid,
normalized node id.
In `@src/utils/opcua/generate-opcua-config.ts`:
- Around line 358-391: Replace the regex-based parser in parseDebugFile with a
call to the canonical parser: import parseDebugVariables (and DebugVariableEntry
if needed) from debug-parser.ts, call parseDebugVariables(content) inside
parseDebugFile, map each returned DebugVariableEntry to the DebugVariable shape
expected by callers (e.g., copy name/type/index fields or rename fields if
necessary), and remove the duplicated regex logic (entryRegex, debugVarsMatch,
etc.) so parseDebugFile simply delegates to parseDebugVariables and returns the
mapped array.
♻️ Duplicate comments (3)
src/renderer/components/_features/[workspace]/editor/server/opcua-server/components/variable-config-modal.tsx (1)
212-242: Backfill field configs for legacy complex nodes.When editing an existing configuration for a complex type, if
existingConfig.fieldsis empty or missing, the modal won't display the field permissions table. Consider regenerating defaults as a fallback.src/utils/opcua/resolve-indices.ts (1)
19-29: Fix condition order:_P_ENUMand_O_ENUMchecks must come before_ENUM.The current logic has a critical bug. Since
_P_ENUMand_O_ENUMboth end with_ENUM, the first condition catches them and removes only 5 characters instead of 7, yielding incorrect results. For example,INT_P_ENUMwould returnINT_Pinstead ofINT. The second condition is unreachable for these suffixes.Proposed fix
const debugTypeToIecType = (debugType: string): string => { - // Remove _ENUM suffix if present - if (debugType.endsWith('_ENUM')) { - return debugType.slice(0, -5) - } // Remove _P_ENUM or _O_ENUM suffix for pointer/output types if (debugType.endsWith('_P_ENUM') || debugType.endsWith('_O_ENUM')) { return debugType.slice(0, -7) } + // Remove _ENUM suffix if present + if (debugType.endsWith('_ENUM')) { + return debugType.slice(0, -5) + } return debugType }src/types/PLC/open-plc.ts (1)
408-410: Document retention of deprecated OPC UA security policies.
Basic128Rsa15andBasic256are deprecated per OPC UA 1.04 specification due to SHA-1 vulnerabilities. If retained for backward compatibility with legacy clients, add a comment explaining this decision and the security implications. Otherwise, consider removing them.Suggested documentation
// OPC-UA Security Policy Enumeration +// NOTE: 'Basic128Rsa15' and 'Basic256' are deprecated per OPC UA 1.04 (SHA-1 vulnerabilities). +// Retained for backward compatibility with legacy OPC-UA clients. Prefer 'Basic256Sha256' for new deployments. const OpcUaSecurityPolicySchema = z.enum(['None', 'Basic128Rsa15', 'Basic256', 'Basic256Sha256'])
🧹 Nitpick comments (3)
src/renderer/components/_features/[workspace]/editor/server/opcua-server/components/variable-config-modal.tsx (3)
280-292:applyPermissionsToAllFieldsonly updates top-level fields.For nested complex types (e.g., a struct containing another struct), this function won't propagate permissions to nested
fieldsarrays. If deep permission propagation is intended, consider a recursive update.♻️ Proposed recursive implementation
+const applyPermissionsRecursively = ( + fields: OpcUaFieldConfig[], + perms: OpcUaPermissions, +): OpcUaFieldConfig[] => + fields.map((field) => ({ + ...field, + permissions: { ...perms }, + fields: field.fields ? applyPermissionsRecursively(field.fields, perms) : undefined, + })) + const applyPermissionsToAllFields = useCallback(() => { + const perms = { viewer: viewerPerm, operator: operatorPerm, engineer: engineerPerm } setFieldConfigs((prev) => - prev.map((field) => ({ - ...field, - permissions: { - viewer: viewerPerm, - operator: operatorPerm, - engineer: engineerPerm, - }, - })), + applyPermissionsRecursively(prev, perms), ) }, [viewerPerm, operatorPerm, engineerPerm])
294-304:updateFieldPermissionwon't update nested fields.Similar to
applyPermissionsToAllFields, this only matches top-levelfieldPath. Nested fields with matching names in different parent structures could also cause incorrect updates if paths aren't unique.♻️ Proposed recursive implementation
+const updateFieldPermissionRecursively = ( + fields: OpcUaFieldConfig[], + fieldPath: string, + role: 'viewer' | 'operator' | 'engineer', + value: PermissionLevel, +): OpcUaFieldConfig[] => + fields.map((field) => { + if (field.fieldPath === fieldPath) { + return { ...field, permissions: { ...field.permissions, [role]: value } } + } + if (field.fields) { + return { ...field, fields: updateFieldPermissionRecursively(field.fields, fieldPath, role, value) } + } + return field + }) + const updateFieldPermission = useCallback( (fieldPath: string, role: 'viewer' | 'operator' | 'engineer', value: PermissionLevel) => { setFieldConfigs((prev) => - prev.map((field) => - field.fieldPath === fieldPath ? { ...field, permissions: { ...field.permissions, [role]: value } } : field, - ), + updateFieldPermissionRecursively(prev, fieldPath, role, value), ) }, [], )
463-604: Consider extracting the repeated permission Select blocks.The three permission selectors (Viewer, Operator, Engineer) share nearly identical markup (~45 lines each). Extracting a reusable component would reduce ~135 lines of duplication.
♻️ Suggested extraction
const RolePermissionSelect = ({ label, value, onChange, }: { label: string value: PermissionLevel onChange: (value: PermissionLevel) => void }) => ( <div className='flex flex-col gap-2'> <Label className='text-xs text-neutral-700 dark:text-neutral-300'>{label}</Label> <Select value={value} onValueChange={(v) => onChange(v as PermissionLevel)}> <SelectTrigger withIndicator placeholder='Select' className='flex h-[30px] w-full items-center justify-between gap-1 rounded-md border border-neutral-300 bg-white px-2 py-1 font-caption text-cp-sm font-medium text-neutral-850 outline-none data-[state=open]:border-brand-medium-dark dark:border-neutral-850 dark:bg-neutral-950 dark:text-neutral-300' /> <SelectContent className='...'> {(['r', 'w', 'rw'] as const).map((perm) => ( <SelectItem key={perm} value={perm} className='...'> <span className='...'> {perm === 'r' ? 'Read Only' : perm === 'w' ? 'Write Only' : 'Read/Write'} </span> </SelectItem> ))} </SelectContent> </Select> </div> )Then use:
<div className='grid grid-cols-3 gap-4'> <RolePermissionSelect label='Viewer' value={viewerPerm} onChange={setViewerPerm} /> <RolePermissionSelect label='Operator' value={operatorPerm} onChange={setOperatorPerm} /> <RolePermissionSelect label='Engineer' value={engineerPerm} onChange={setEngineerPerm} /> </div>
| const generateNodeId = (pouName: string, variablePath: string): string => { | ||
| const cleanPath = variablePath.replace(/\./g, '.').replace(/\[/g, '_').replace(/\]/g, '') | ||
| return `PLC.${pouName}.${cleanPath}` | ||
| } |
There was a problem hiding this comment.
No-op string replacement detected.
The regex .replace(/\./g, '.') replaces dots with dots, which has no effect. This appears to be a bug—likely intended to replace dots with underscores or another character for a valid node ID.
🐛 Proposed fix
const generateNodeId = (pouName: string, variablePath: string): string => {
- const cleanPath = variablePath.replace(/\./g, '.').replace(/\[/g, '_').replace(/\]/g, '')
+ const cleanPath = variablePath.replace(/\[/g, '_').replace(/\]/g, '')
return `PLC.${pouName}.${cleanPath}`
}If dots should be replaced (e.g., with underscores), update accordingly:
- const cleanPath = variablePath.replace(/\./g, '.').replace(/\[/g, '_').replace(/\]/g, '')
+ const cleanPath = variablePath.replace(/\./g, '_').replace(/\[/g, '_').replace(/\]/g, '')📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const generateNodeId = (pouName: string, variablePath: string): string => { | |
| const cleanPath = variablePath.replace(/\./g, '.').replace(/\[/g, '_').replace(/\]/g, '') | |
| return `PLC.${pouName}.${cleanPath}` | |
| } | |
| const generateNodeId = (pouName: string, variablePath: string): string => { | |
| const cleanPath = variablePath.replace(/\[/g, '_').replace(/\]/g, '') | |
| return `PLC.${pouName}.${cleanPath}` | |
| } |
🤖 Prompt for AI Agents
In
`@src/renderer/components/_features/`[workspace]/editor/server/opcua-server/components/variable-config-modal.tsx
around lines 32 - 35, In generateNodeId, the replacement cleanPath uses
.replace(/\./g, '.') which is a no-op; change it to replace dots with the
intended character (e.g., underscore) so node IDs are sanitized. Update the
cleanPath logic in the generateNodeId function (referencing pouName and
variablePath) to replace '.' with '_' (and keep the existing replacements for
'[' and ']') so the returned `PLC.${pouName}.${cleanPath}` produces a valid,
normalized node id.
| export const parseDebugFile = (content: string): DebugVariable[] => { | ||
| const variables: DebugVariable[] = [] | ||
|
|
||
| // Find the debug_vars[] array | ||
| const debugVarsMatch = content.match(/debug_vars\[\]\s*=\s*\{([\s\S]*?)\};/) | ||
|
|
||
| if (!debugVarsMatch) { | ||
| console.warn('Could not find debug_vars[] array in debug.c') | ||
| return [] | ||
| } | ||
|
|
||
| const arrayContent = debugVarsMatch[1] | ||
|
|
||
| // Parse each entry: { &(VARIABLE_PATH), TYPE } | ||
| const entryRegex = /\{\s*&\(([^)]+)\)\s*,\s*(\w+)\s*\}/g | ||
|
|
||
| let match | ||
| let index = 0 | ||
|
|
||
| while ((match = entryRegex.exec(arrayContent)) !== null) { | ||
| const fullPath = match[1].trim() | ||
| const type = match[2].trim() | ||
|
|
||
| variables.push({ | ||
| name: fullPath, | ||
| type, | ||
| index, | ||
| }) | ||
|
|
||
| index++ | ||
| } | ||
|
|
||
| return variables | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify parseDebugFile implementations and check for existing shared parser
echo "=== Checking for parseDebugFile implementations ==="
rg -n "export const parseDebugFile" --type=ts
echo ""
echo "=== Checking for debug-parser.ts ==="
fd "debug-parser.ts" --type=f
echo ""
echo "=== Checking parseDebugFile exports from index.ts ==="
rg -n "parseDebugFile" src/utils/opcua/index.ts
echo ""
echo "=== Checking for any parseDebugFile usage ==="
rg -n "parseDebugFile" --type=tsRepository: Autonomy-Logic/openplc-editor
Length of output: 2329
🏁 Script executed:
# Check the debug-parser.ts implementation
echo "=== debug-parser.ts parseDebugFile ==="
sed -n '62,/^}/p' src/utils/debug-parser.ts | head -60
echo ""
echo "=== Check ParsedDebugData type ==="
rg -n "type ParsedDebugData\|interface ParsedDebugData" src/utils/debug-parser.ts -A 5
echo ""
echo "=== Check DebugVariable type ==="
rg -n "type DebugVariable\|interface DebugVariable" src/utils/opcua/generate-opcua-config.ts -A 5Repository: Autonomy-Logic/openplc-editor
Length of output: 492
🏁 Script executed:
# Look for type definitions more broadly
echo "=== Searching for ParsedDebugData ==="
rg "ParsedDebugData" src/utils/debug-parser.ts -B 2 -A 3
echo ""
echo "=== Searching for DebugVariable ==="
rg "DebugVariable" src/utils/opcua/generate-opcua-config.ts -B 2 -A 3 | head -30
echo ""
echo "=== Get full context of debug-parser.ts file ==="
wc -l src/utils/debug-parser.ts
echo ""
echo "=== Check imports and types at top of debug-parser.ts ==="
head -70 src/utils/debug-parser.tsRepository: Autonomy-Logic/openplc-editor
Length of output: 3427
🏁 Script executed:
# Check imports in generate-opcua-config.ts
echo "=== Imports at top of generate-opcua-config.ts ==="
head -20 src/utils/opcua/generate-opcua-config.ts
echo ""
echo "=== Check DebugVariable type definition ==="
cat src/utils/opcua/types.ts | grep -A 8 "DebugVariable"Repository: Autonomy-Logic/openplc-editor
Length of output: 983
Remove duplicated parsing logic and use the canonical parseDebugVariables from debug-parser.ts.
The parseDebugFile implementation here duplicates the parsing logic from src/utils/debug-parser.ts::parseDebugVariables (lines 29–51). Since debug-parser.ts is designated as the "canonical debug.c parser," this function should import and use parseDebugVariables from that module instead. The DebugVariable type is structurally identical to DebugVariableEntry, making the migration straightforward.
🤖 Prompt for AI Agents
In `@src/utils/opcua/generate-opcua-config.ts` around lines 358 - 391, Replace the
regex-based parser in parseDebugFile with a call to the canonical parser: import
parseDebugVariables (and DebugVariableEntry if needed) from debug-parser.ts,
call parseDebugVariables(content) inside parseDebugFile, map each returned
DebugVariableEntry to the DebugVariable shape expected by callers (e.g., copy
name/type/index fields or rename fields if necessary), and remove the duplicated
regex logic (entryRegex, debugVarsMatch, etc.) so parseDebugFile simply
delegates to parseDebugVariables and returns the mapped array.
Use bcryptjs library with cost factor 10 for secure password hashing instead of SHA-256 via Web Crypto API. Bcrypt provides better protection against brute-force attacks through its adaptive cost factor. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
feat: Replace SHA-256 with bcrypt for OPC-UA user password hashing
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@package.json`:
- Line 57: The renderer currently performs CPU-heavy bcryptjs hashing inside
src/renderer/components/_features/[workspace]/editor/server/opcua-server/components/user-modal.tsx;
extract that logic into a main-process/worker helper module (create
src/utils/opcua/bcrypt-utils.ts) that exports async functions like
hashPassword(password: string, rounds?: number) and comparePassword(password:
string, hash: string), implement bcryptjs calls there, and expose these
functions to the renderer via IPC or the existing preload/API bridge; then
replace direct bcrypt imports/usages in user-modal.tsx with calls to the new
IPC-exposed hashPassword/comparePassword functions so hashing runs outside the
renderer thread.
In
`@src/renderer/components/_features/`[workspace]/editor/server/opcua-server/components/user-modal.tsx:
- Around line 249-255: The password visibility toggle button in the UserModal
lacks an accessible name/state; update the button in the user-modal component
(the element using onClick={() => setShowPassword(!showPassword)} and the
showPassword variable) to include an appropriate accessible label and state
(e.g., set aria-label to "Hide password" when showPassword is true and "Show
password" when false, and add aria-pressed={showPassword} or aria-expanded
accordingly), and optionally add a title attribute for tooltip parity so screen
readers and keyboard users get a clear, dynamic description.
♻️ Duplicate comments (1)
src/renderer/components/_features/[workspace]/editor/server/opcua-server/components/user-modal.tsx (1)
112-118: Increase minimum password length.A 4‑character minimum is too weak for password auth. Consider raising to at least 8 characters and update the message accordingly.
🔧 Proposed change
- } else if (password.length < 4) { - errors.push('Password must be at least 4 characters') + } else if (password.length < 8) { + errors.push('Password must be at least 8 characters') } else if (password !== confirmPassword) { errors.push('Passwords do not match') }
| "@tanstack/react-table": "^8.10.7", | ||
| "@xyflow/react": "^12.0.1", | ||
| "auto-zustand-selectors-hook": "^2.0.0", | ||
| "bcryptjs": "^3.0.3", |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
npm view bcryptjs versionRepository: Autonomy-Logic/openplc-editor
Length of output: 78
🏁 Script executed:
npm view bcryptjs versions --json | tail -20Repository: Autonomy-Logic/openplc-editor
Length of output: 282
🏁 Script executed:
rg -n 'bcryptjs' --type ts --type tsx --type js --type jsx -A 3 -B 3Repository: Autonomy-Logic/openplc-editor
Length of output: 100
🏁 Script executed:
rg -n 'bcryptjs' -A 3 -B 3Repository: Autonomy-Logic/openplc-editor
Length of output: 5254
🏁 Script executed:
npm view bcryptjsRepository: Autonomy-Logic/openplc-editor
Length of output: 753
Move bcryptjs hashing operations out of the renderer process.
The version 3.0.3 is current and has no known vulnerabilities. However, bcryptjs is currently imported directly in src/renderer/components/_features/[workspace]/editor/server/opcua-server/components/user-modal.tsx (the renderer process), where CPU-intensive operations like password hashing with high salt rounds will block the UI thread. Move hashing logic to a main process helper or worker thread. The documentation already outlines a plan for src/utils/opcua/bcrypt-utils.ts—implement this pattern instead of calling bcryptjs directly from renderer components.
🤖 Prompt for AI Agents
In `@package.json` at line 57, The renderer currently performs CPU-heavy bcryptjs
hashing inside
src/renderer/components/_features/[workspace]/editor/server/opcua-server/components/user-modal.tsx;
extract that logic into a main-process/worker helper module (create
src/utils/opcua/bcrypt-utils.ts) that exports async functions like
hashPassword(password: string, rounds?: number) and comparePassword(password:
string, hash: string), implement bcryptjs calls there, and expose these
functions to the renderer via IPC or the existing preload/API bridge; then
replace direct bcrypt imports/usages in user-modal.tsx with calls to the new
IPC-exposed hashPassword/comparePassword functions so hashing runs outside the
renderer thread.
| <button | ||
| type='button' | ||
| onClick={() => setShowPassword(!showPassword)} | ||
| className='absolute right-2 top-1/2 -translate-y-1/2 text-neutral-500 hover:text-neutral-700 dark:hover:text-neutral-300' | ||
| > | ||
| {showPassword ? '🙈' : '👁'} | ||
| </button> |
There was a problem hiding this comment.
Add accessible labels to the password visibility toggle.
The emoji-only button lacks an accessible name/state for screen readers.
♿ Proposed fix
- <button
+ <button
type='button'
+ aria-label={showPassword ? 'Hide password' : 'Show password'}
+ aria-pressed={showPassword}
onClick={() => setShowPassword(!showPassword)}
className='absolute right-2 top-1/2 -translate-y-1/2 text-neutral-500 hover:text-neutral-700 dark:hover:text-neutral-300'
>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <button | |
| type='button' | |
| onClick={() => setShowPassword(!showPassword)} | |
| className='absolute right-2 top-1/2 -translate-y-1/2 text-neutral-500 hover:text-neutral-700 dark:hover:text-neutral-300' | |
| > | |
| {showPassword ? '🙈' : '👁'} | |
| </button> | |
| <button | |
| type='button' | |
| aria-label={showPassword ? 'Hide password' : 'Show password'} | |
| aria-pressed={showPassword} | |
| onClick={() => setShowPassword(!showPassword)} | |
| className='absolute right-2 top-1/2 -translate-y-1/2 text-neutral-500 hover:text-neutral-700 dark:hover:text-neutral-300' | |
| > | |
| {showPassword ? '🙈' : '👁'} | |
| </button> |
🤖 Prompt for AI Agents
In
`@src/renderer/components/_features/`[workspace]/editor/server/opcua-server/components/user-modal.tsx
around lines 249 - 255, The password visibility toggle button in the UserModal
lacks an accessible name/state; update the button in the user-modal component
(the element using onClick={() => setShowPassword(!showPassword)} and the
showPassword variable) to include an appropriate accessible label and state
(e.g., set aria-label to "Hide password" when showPassword is true and "Show
password" when false, and add aria-pressed={showPassword} or aria-expanded
accordingly), and optionally add a title attribute for tooltip parity so screen
readers and keyboard users get a clear, dynamic description.
Replace bcrypt with PBKDF2-HMAC-SHA256 for password hashing in OPC-UA user configuration. This fixes authentication failures on Windows/MSYS2 where Python's bcrypt module cannot be built. Changes: - Replace bcryptjs with Web Crypto API PBKDF2 implementation - Use format compatible with OpenPLC Runtime: pbkdf2:sha256:600000$salt$hash - Remove bcryptjs dependency from package.json The runtime already supports both bcrypt and PBKDF2 hash verification, so this change maintains backward compatibility on Linux/macOS while enabling Windows/MSYS2 support. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
…-hashing fix: Use PBKDF2 instead of bcrypt for OPC-UA password hashing
Code reviewNo issues found. Checked for bugs and CLAUDE.md compliance. |
- Fix CRLF handling in PEM certificate parsing for Windows compatibility - Fix descendant cleanup when removing complex type nodes from address space - Add infinite recursion protection for circular type references in findLeafVariables - Fix nested field permission updates (applyPermissionsToAllFields and updateFieldPermission now handle nested fields recursively) - Remove no-op replace statement in generateNodeId - Ensure at least one auth method when switching from None to secure policy - Support negative array indices in parseArrayDimension (IEC 61131-3 compliance) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Code Review Fixes AppliedI've addressed the issues identified in the code review. Here's a summary of the fixes: Must Fix (All Addressed)
Should Fix (All Addressed)
Not Addressed (By Request)The following items were intentionally not addressed as per discussion:
All changes pass linting and formatting checks. |
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Fix all issues with AI agents
In
`@src/renderer/components/_features/`[workspace]/editor/server/opcua-server/components/address-space-tab.tsx:
- Around line 35-40: The selectedVariableIds state is only initialized once from
config.addressSpace.nodes and can become stale when config updates; add a
useEffect in the AddressSpaceTab component that watches
config.addressSpace.nodes (or config) and calls setSelectedVariableIds(new
Set(config.addressSpace.nodes.map(n => `${n.pouName}-${n.variablePath}`))) to
re-sync selection when the configuration changes, ensuring you preserve the same
Set<string> shape and avoid unnecessary updates by comparing lengths or
stringified IDs if desired.
In
`@src/renderer/components/_features/`[workspace]/editor/server/opcua-server/components/certificate-modal.tsx:
- Around line 139-145: The onchange handler for the file input (input.onchange)
currently calls file.text() without handling rejection; wrap the file read in a
try/catch (or add a .catch) and surface failures to the UI instead of silently
ignoring them — e.g., when reading in the handler that ultimately calls
setPemContent, catch errors from file.text(), call a state updater like
setPemError or setPemContent with a sentinel, and log the error (or show a
user-facing message) so permission/encoding/size failures produce visible
feedback.
In
`@src/renderer/components/_features/`[workspace]/editor/server/opcua-server/components/user-modal.tsx:
- Around line 179-202: The handleSave function currently calls hashPassword
(which can reject) without try/catch; wrap the password hashing and subsequent
user creation in a try/catch so hashing failures are caught, display an error to
the user (e.g., set a local error state or call the existing UI error/toast
mechanism) and return early without calling onSave/onClose on failure;
specifically modify handleSave to catch errors from hashPassword when authType
=== 'password' (and when preserving existingUser.passwordHash) and handle them
gracefully instead of letting the promise rejection go unhandled.
In
`@src/renderer/components/_features/`[workspace]/editor/server/opcua-server/components/variable-config-modal.tsx:
- Around line 100-133: The generateDefaultFieldConfigs function currently sets
fieldPath to child.name which can collide for nested fields; change it to use
the fully-qualified identifier (child.variablePath) when constructing fieldPath
for both leaf and complex branches so each nested field is unique, keep
displayName as child.name, and ensure the recursive fields array also uses
variablePath for each nested entry so updateFieldPermissionRecursively and React
keys match the fully-qualified path.
In `@src/utils/pou-helpers.ts`:
- Around line 171-179: The function findLeafVariables currently skips
array-typed fbVar entries (fbVar.type.definition === 'array'), causing
array-contained leaves to be dropped; update findLeafVariables to handle arrays
by using fbVar.type.data (the array element type) to build a representative
element path (e.g., append "[0]" or similar to varPath), then either treat that
element as a base-type leaf (push into leaves with appropriate typeName) or
recurse into it (call findLeafVariables with the element typeName, same
projectPous/dataTypes and new path) while preserving visited tracking; ensure
you reference fbVar, varPath, varTypeName, leaves and visited when adding the
array-handling branch so arrays are surfaced for OPC‑UA/debug index resolution.
🧹 Nitpick comments (3)
src/renderer/components/_features/[workspace]/editor/server/opcua-server/components/security-profile-modal.tsx (2)
11-13: Consider importing these types from the shared type definition.These local type aliases duplicate what should already be defined in
OpcUaSecurityProfile. Importing them from@root/types/PLC/open-plc(or re-exporting them alongside the interface) would ensure consistency and prevent drift if the shared type is updated.
197-215: Accessibility: Toggle switch lacks proper label association.The
<Label>at line 199 is visually associated but not programmatically linked to the checkbox. Screen readers won't announce "Enabled" when the toggle receives focus.♿ Proposed fix to improve accessibility
{/* Enable Toggle */} <div className='flex items-center gap-4'> - <Label className='text-xs font-semibold text-neutral-950 dark:text-white'>Enabled</Label> + <Label htmlFor='profile-enabled-toggle' className='text-xs font-semibold text-neutral-950 dark:text-white'>Enabled</Label> <label className='relative inline-flex cursor-pointer items-center'> <input type='checkbox' + id='profile-enabled-toggle' checked={enabled} onChange={(e) => setEnabled(e.target.checked)} className='peer sr-only' + aria-label='Enable profile' />src/utils/pou-helpers.ts (1)
6-86: Follow the established pattern for renderer imports in shared utils.
pou-helpers.tsuses a static import ofStandardFunctionBlocksfrom a renderer-only path (line 6). While currently only used in renderer code, this violates the pattern already established indebug-tree-traversal.ts, which uses dynamicrequire()with try-catch to safely handle renderer imports in shared utilities (lines 82–89). If main-process code ever importspou-helpers, it will fail at runtime. Refactor to use dynamic imports matching the pattern indebug-tree-traversal.ts.
| // Local state | ||
| const [filter, setFilter] = useState('') | ||
| const [selectedVariableIds, setSelectedVariableIds] = useState<Set<string>>(() => { | ||
| // Initialize with IDs of already configured nodes | ||
| return new Set(config.addressSpace.nodes.map((n) => `${n.pouName}-${n.variablePath}`)) | ||
| }) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Find AddressSpaceTab usage and context
rg -n "AddressSpaceTab" -g'*.tsx' -g'*.ts' src/renderer --headingRepository: Autonomy-Logic/openplc-editor
Length of output: 501
🏁 Script executed:
#!/bin/bash
# Check the parent component and how config is managed
sed -n '1,220p' src/renderer/components/_features/[workspace]/editor/server/opcua-server/index.tsx | cat -nRepository: Autonomy-Logic/openplc-editor
Length of output: 9905
🏁 Script executed:
#!/bin/bash
# Check the full AddressSpaceTab component to see if there's already a useEffect handling config changes
cat -n src/renderer/components/_features/[workspace]/editor/server/opcua-server/components/address-space-tab.tsxRepository: Autonomy-Logic/openplc-editor
Length of output: 12848
Add useEffect to sync selectedVariableIds when config changes.
The Set is initialized only once from config.addressSpace.nodes, but the config prop can change after mount when the server configuration updates. If config.addressSpace.nodes changes externally (reload, external update), the selection state drifts from the source of truth, causing UI inconsistencies (stale selections, missing configured nodes).
🔧 Proposed fix
-import { useCallback, useMemo, useState } from 'react'
+import { useCallback, useEffect, useMemo, useState } from 'react'
...
const [selectedVariableIds, setSelectedVariableIds] = useState<Set<string>>(() => {
// Initialize with IDs of already configured nodes
return new Set(config.addressSpace.nodes.map((n) => `${n.pouName}-${n.variablePath}`))
})
+ useEffect(() => {
+ setSelectedVariableIds(new Set(config.addressSpace.nodes.map((n) => `${n.pouName}-${n.variablePath}`)))
+ }, [config.addressSpace.nodes])🤖 Prompt for AI Agents
In
`@src/renderer/components/_features/`[workspace]/editor/server/opcua-server/components/address-space-tab.tsx
around lines 35 - 40, The selectedVariableIds state is only initialized once
from config.addressSpace.nodes and can become stale when config updates; add a
useEffect in the AddressSpaceTab component that watches
config.addressSpace.nodes (or config) and calls setSelectedVariableIds(new
Set(config.addressSpace.nodes.map(n => `${n.pouName}-${n.variablePath}`))) to
re-sync selection when the configuration changes, ensuring you preserve the same
Set<string> shape and avoid unnecessary updates by comparing lengths or
stringified IDs if desired.
| input.onchange = (e) => { | ||
| const file = (e.target as HTMLInputElement).files?.[0] | ||
| if (file) { | ||
| void file.text().then((content) => { | ||
| setPemContent(content) | ||
| }) | ||
| } |
There was a problem hiding this comment.
Add error handling for file read operation.
If file.text() fails (e.g., permission issues, large file, or encoding problems), the promise rejection is silently ignored, leaving the user without feedback.
🔧 Proposed fix
input.onchange = (e) => {
const file = (e.target as HTMLInputElement).files?.[0]
if (file) {
- void file.text().then((content) => {
- setPemContent(content)
- })
+ file.text()
+ .then((content) => {
+ setPemContent(content)
+ })
+ .catch(() => {
+ setPemError('Failed to read certificate file')
+ })
}
}🤖 Prompt for AI Agents
In
`@src/renderer/components/_features/`[workspace]/editor/server/opcua-server/components/certificate-modal.tsx
around lines 139 - 145, The onchange handler for the file input (input.onchange)
currently calls file.text() without handling rejection; wrap the file read in a
try/catch (or add a .catch) and surface failures to the UI instead of silently
ignoring them — e.g., when reading in the handler that ultimately calls
setPemContent, catch errors from file.text(), call a state updater like
setPemError or setPemContent with a sentinel, and log the error (or show a
user-facing message) so permission/encoding/size failures produce visible
feedback.
| const handleSave = useCallback(async () => { | ||
| if (!isValid) return | ||
|
|
||
| let passwordHash: string | null = null | ||
|
|
||
| if (authType === 'password' && password) { | ||
| passwordHash = await hashPassword(password) | ||
| } else if (authType === 'password' && isEditing && existingUser?.passwordHash) { | ||
| // Keep existing password hash if not changing | ||
| passwordHash = existingUser.passwordHash | ||
| } | ||
|
|
||
| const user: OpcUaUser = { | ||
| id: existingUser?.id ?? uuidv4(), | ||
| type: authType, | ||
| username: authType === 'password' ? username.trim() : null, | ||
| passwordHash: authType === 'password' ? passwordHash : null, | ||
| certificateId: authType === 'certificate' ? certificateId : null, | ||
| role, | ||
| } | ||
|
|
||
| onSave(user) | ||
| onClose() | ||
| }, [isValid, authType, username, password, certificateId, role, existingUser, isEditing, onSave, onClose]) |
There was a problem hiding this comment.
Add error handling for password hashing failure.
hashPassword uses Web Crypto API which can throw. If crypto.subtle.deriveBits fails, the promise rejection is unhandled (called via void handleSave() on line 423). Consider wrapping in try/catch to display an error to the user.
🛠️ Suggested fix
const handleSave = useCallback(async () => {
if (!isValid) return
+ try {
let passwordHash: string | null = null
if (authType === 'password' && password) {
passwordHash = await hashPassword(password)
} else if (authType === 'password' && isEditing && existingUser?.passwordHash) {
// Keep existing password hash if not changing
passwordHash = existingUser.passwordHash
}
const user: OpcUaUser = {
id: existingUser?.id ?? uuidv4(),
type: authType,
username: authType === 'password' ? username.trim() : null,
passwordHash: authType === 'password' ? passwordHash : null,
certificateId: authType === 'certificate' ? certificateId : null,
role,
}
onSave(user)
onClose()
+ } catch (error) {
+ console.error('Failed to save user:', error)
+ // Consider setting an error state to display to the user
+ }
}, [isValid, authType, username, password, certificateId, role, existingUser, isEditing, onSave, onClose])🤖 Prompt for AI Agents
In
`@src/renderer/components/_features/`[workspace]/editor/server/opcua-server/components/user-modal.tsx
around lines 179 - 202, The handleSave function currently calls hashPassword
(which can reject) without try/catch; wrap the password hashing and subsequent
user creation in a try/catch so hashing failures are caught, display an error to
the user (e.g., set a local error state or call the existing UI error/toast
mechanism) and return early without calling onSave/onClose on failure;
specifically modify handleSave to catch errors from hashPassword when authType
=== 'password' (and when preserving existingUser.passwordHash) and handle them
gracefully instead of letting the promise rejection go unhandled.
| const generateDefaultFieldConfigs = ( | ||
| node: VariableTreeNode, | ||
| parentPermissions: OpcUaPermissions, | ||
| ): OpcUaFieldConfig[] => { | ||
| if (!node.children || node.children.length === 0) { | ||
| return [] | ||
| } | ||
|
|
||
| return node.children.map((child) => { | ||
| // Check if this is a leaf variable (base type) or a complex type | ||
| const isLeaf = child.type === 'variable' && (!child.children || child.children.length === 0) | ||
|
|
||
| if (isLeaf) { | ||
| // Leaf field - no nested fields | ||
| return { | ||
| fieldPath: child.name, | ||
| displayName: child.name, | ||
| datatype: child.variableType || 'UNKNOWN', | ||
| initialValue: getDefaultInitialValue(child.variableType || 'UNKNOWN'), | ||
| permissions: { ...parentPermissions }, | ||
| } | ||
| } else { | ||
| // Complex type (FB, struct, array) - generate nested fields recursively | ||
| const nestedFields = generateDefaultFieldConfigs(child, parentPermissions) | ||
| return { | ||
| fieldPath: child.name, | ||
| displayName: child.name, | ||
| datatype: child.variableType || 'UNKNOWN', | ||
| initialValue: getDefaultInitialValue(child.variableType || 'UNKNOWN'), | ||
| permissions: { ...parentPermissions }, | ||
| fields: nestedFields.length > 0 ? nestedFields : undefined, | ||
| } | ||
| } | ||
| }) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify how fieldPath is consumed and used across the codebase
rg -n "fieldPath" src/renderer src/main -g'*.ts' -g'*.tsx' -B 2 -A 2Repository: Autonomy-Logic/openplc-editor
Length of output: 10447
🏁 Script executed:
#!/bin/bash
# Check how VariableTreeNode is built and if nested structures with duplicate names are possible
rg -n "VariableTreeNode" src/renderer -g'*.ts' -g'*.tsx' -B 3 -A 3 | head -100Repository: Autonomy-Logic/openplc-editor
Length of output: 13101
🏁 Script executed:
#!/bin/bash
# Find the definition of VariableTreeNode
rg -n "type VariableTreeNode\|interface VariableTreeNode" src/renderer -g'*.ts' -g'*.tsx' -A 15Repository: Autonomy-Logic/openplc-editor
Length of output: 55
🏁 Script executed:
#!/bin/bash
# Find use-project-variables hook where VariableTreeNode is likely defined
fd -n "use-project-variables" src/renderer -g'*.ts' -g'*.tsx'Repository: Autonomy-Logic/openplc-editor
Length of output: 303
🏁 Script executed:
#!/bin/bash
# Find use-project-variables hook where VariableTreeNode is likely defined
fd "use-project-variables" src/rendererRepository: Autonomy-Logic/openplc-editor
Length of output: 176
🏁 Script executed:
#!/bin/bash
# Read the use-project-variables hook to find VariableTreeNode definition
cat -n src/renderer/components/_features/[workspace]/editor/server/opcua-server/hooks/use-project-variables.ts | head -150Repository: Autonomy-Logic/openplc-editor
Length of output: 5421
🏁 Script executed:
#!/bin/bash
# Continue reading to understand how children are built
cat -n src/renderer/components/_features/[workspace]/editor/server/opcua-server/hooks/use-project-variables.ts | sed -n '150,300p'Repository: Autonomy-Logic/openplc-editor
Length of output: 5272
🏁 Script executed:
#!/bin/bash
# Check if VariableTreeNode has variablePath property that we should be using
rg -n "variablePath" src/renderer/components/_features/[workspace]/editor/server/opcua-server/components/variable-config-modal.tsx -B 2 -A 2Repository: Autonomy-Logic/openplc-editor
Length of output: 1954
🏁 Script executed:
#!/bin/bash
# Check how fieldPath is used in the modal - is it displayed or used for other purposes
rg -n "field.fieldPath\|\.fieldPath" src/renderer/components/_features/[workspace]/editor/server/opcua-server/components/variable-config-modal.tsx -B 1 -A 1Repository: Autonomy-Logic/openplc-editor
Length of output: 55
🏁 Script executed:
#!/bin/bash
# Get a broader view of how fieldConfigs are used throughout the file
cat -n src/renderer/components/_features/[workspace]/editor/server/opcua-server/components/variable-config-modal.tsx | sed -n '300,350p'Repository: Autonomy-Logic/openplc-editor
Length of output: 2211
🏁 Script executed:
#!/bin/bash
# Check what properties are available on the child node in generateDefaultFieldConfigs
cat -n src/renderer/components/_features/[workspace]/editor/server/opcua-server/components/variable-config-modal.tsx | sed -n '100,140p'Repository: Autonomy-Logic/openplc-editor
Length of output: 1713
🏁 Script executed:
#!/bin/bash
# Verify that child nodes have variablePath available
cat -n src/renderer/components/_features/[workspace]/editor/server/opcua-server/hooks/use-project-variables.ts | sed -n '170,210p'Repository: Autonomy-Logic/openplc-editor
Length of output: 1543
Use variablePath for fieldPath to ensure uniqueness across nested fields.
Nested structures can have duplicate field names at different levels (e.g., "device.temperature" and "sensor.temperature"). Using only child.name causes collisions where updateFieldPermissionRecursively (line 311) matches the first occurrence instead of the intended field. Additionally, non-unique fieldPath values break React reconciliation when used as keys (line 709).
Build fully-qualified paths from child.variablePath instead:
🔧 Proposed fix
const generateDefaultFieldConfigs = (
node: VariableTreeNode,
parentPermissions: OpcUaPermissions,
): OpcUaFieldConfig[] => {
if (!node.children || node.children.length === 0) {
return []
}
return node.children.map((child) => {
const isLeaf = child.type === 'variable' && (!child.children || child.children.length === 0)
if (isLeaf) {
return {
- fieldPath: child.name,
+ fieldPath: child.variablePath,
displayName: child.name,
datatype: child.variableType || 'UNKNOWN',
initialValue: getDefaultInitialValue(child.variableType || 'UNKNOWN'),
permissions: { ...parentPermissions },
}
} else {
const nestedFields = generateDefaultFieldConfigs(child, parentPermissions)
return {
- fieldPath: child.name,
+ fieldPath: child.variablePath,
displayName: child.name,
datatype: child.variableType || 'UNKNOWN',
initialValue: getDefaultInitialValue(child.variableType || 'UNKNOWN'),
permissions: { ...parentPermissions },
fields: nestedFields.length > 0 ? nestedFields : undefined,
}
}
})
}🤖 Prompt for AI Agents
In
`@src/renderer/components/_features/`[workspace]/editor/server/opcua-server/components/variable-config-modal.tsx
around lines 100 - 133, The generateDefaultFieldConfigs function currently sets
fieldPath to child.name which can collide for nested fields; change it to use
the fully-qualified identifier (child.variablePath) when constructing fieldPath
for both leaf and complex branches so each nested field is unique, keep
displayName as child.name, and ensure the recursive fields array also uses
variablePath for each nested entry so updateFieldPermissionRecursively and React
keys match the fully-qualified path.
| if (fbVar.type.definition === 'base-type' && isBaseType(varTypeName)) { | ||
| leaves.push({ relativePath: varPath, typeName: varTypeName.toUpperCase() }) | ||
| } else if (fbVar.type.definition === 'array' && fbVar.type.data) { | ||
| // For arrays, we need the first element's path - handled separately | ||
| // Skip for now as arrays need special handling | ||
| } else if (!isEnumerationType(varTypeName, dataTypes)) { | ||
| // Recurse into nested FBs or structures | ||
| const nestedLeaves = findLeafVariables(varTypeName, projectPous, dataTypes, varPath, new Set(visited)) | ||
| leaves.push(...nestedLeaves) |
There was a problem hiding this comment.
Arrays are currently skipped, which can drop leaf variables.
In findLeafVariables, array fields are explicitly ignored (Line 173–176). This means array leaves won’t be surfaced for OPC-UA/debug index resolution, which conflicts with the PR’s array support objective. Please confirm arrays are handled elsewhere; otherwise, this needs array expansion (even if it’s only representative element paths).
🤖 Prompt for AI Agents
In `@src/utils/pou-helpers.ts` around lines 171 - 179, The function
findLeafVariables currently skips array-typed fbVar entries
(fbVar.type.definition === 'array'), causing array-contained leaves to be
dropped; update findLeafVariables to handle arrays by using fbVar.type.data (the
array element type) to build a representative element path (e.g., append "[0]"
or similar to varPath), then either treat that element as a base-type leaf (push
into leaves with appropriate typeName) or recurse into it (call
findLeafVariables with the element typeName, same projectPous/dataTypes and new
path) while preserving visited tracking; ensure you reference fbVar, varPath,
varTypeName, leaves and visited when adding the array-handling branch so arrays
are surfaced for OPC‑UA/debug index resolution.
…elds Nested structures can have duplicate field names at different levels (e.g., "device.temperature" and "sensor.temperature"). Using only child.name caused collisions where updateFieldPermissionRecursively matched the first occurrence and React keys conflicted. Now uses child.variablePath for unique identification while keeping child.name for displayName. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Additional Fix AppliedUse
|
The OPC-UA resolver builds the full path by combining parentPath + fieldPath (resolve-indices.ts:148). Using variablePath caused path duplication because variablePath already contains the parent path. Example of the bug: - parentPath = "STATE_DISPLAY0" - fieldPath = "STATE_DISPLAY0.State" (using variablePath) - Result: "STATE_DISPLAY0.STATE_DISPLAY0.State" (WRONG - duplicated) The CodeRabbit concern about uniqueness was a false alarm - the hierarchical structure of fieldConfigs means fields with the same name at different levels are stored in different `fields` arrays, so there's no collision. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Reverted the
|
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In
`@src/renderer/components/_features/`[workspace]/editor/server/opcua-server/components/variable-config-modal.tsx:
- Around line 100-137: generateDefaultFieldConfigs currently recurses through
VariableTreeNode.children without cycle protection; add a visited Set parameter
(e.g., visited: Set<string>) and check the node's unique id (or
variablePath/name if unique) at the start of generateDefaultFieldConfigs to
return [] when already visited, mirror the pattern used in findLeafVariables,
mark nodes as visited before recursing into children, and pass the same visited
set into recursive calls so circular type references in VariableTreeNode (and
interactions with buildVariableNode) cannot cause infinite recursion/stack
overflow.
🧹 Nitpick comments (3)
src/renderer/components/_features/[workspace]/editor/server/opcua-server/components/variable-config-modal.tsx (3)
341-351: Consider usingparseIntfor integer types to preserve expected type semantics.
parseFloatis used for all numeric types including integers (int,word,byte). This means a user input of "3.7" would store3.7even for anINTvariable. Consider usingparseIntfor integer types to match IEC 61131-3 type semantics.♻️ Suggested improvement
} else if ( varType.includes('int') || varType.includes('real') || varType.includes('word') || varType.includes('byte') ) { - const num = parseFloat(initialValue) - parsedInitialValue = isNaN(num) ? 0 : num + const isInteger = varType.includes('int') || varType.includes('word') || varType.includes('byte') + const num = isInteger ? parseInt(initialValue, 10) : parseFloat(initialValue) + parsedInitialValue = isNaN(num) ? 0 : num }
491-632: Consider extracting repeated permission select blocks to reduce duplication.The three permission selects (Viewer, Operator, Engineer) share nearly identical JSX (~45 lines each). This could be refactored into a mapped structure or extracted component to improve maintainability.
♻️ Suggested approach
const permissionRoles = [ { key: 'viewer', label: 'Viewer', value: viewerPerm, setter: setViewerPerm }, { key: 'operator', label: 'Operator', value: operatorPerm, setter: setOperatorPerm }, { key: 'engineer', label: 'Engineer', value: engineerPerm, setter: setEngineerPerm }, ] as const // Then in JSX: <div className='grid grid-cols-3 gap-4'> {permissionRoles.map(({ key, label, value, setter }) => ( <div key={key} className='flex flex-col gap-2'> <Label className='text-xs text-neutral-700 dark:text-neutral-300'>{label}</Label> <PermissionSelectFull value={value} onChange={(v) => setter(v as PermissionLevel)} /> </div> ))} </div>
463-469: Consider adding amaxLengthconstraint to the description textarea.Unlike the other text inputs which have
maxLengthattributes (128 for nodeId/displayName, 64 for browseName), the description textarea has no length limit. Consider adding a reasonable limit to prevent excessively long values.♻️ Suggested fix
<textarea value={description} onChange={(e) => setDescription(e.target.value)} placeholder='Optional description for this node' rows={2} + maxLength={512} className={textareaStyles} />
| const generateDefaultFieldConfigs = ( | ||
| node: VariableTreeNode, | ||
| parentPermissions: OpcUaPermissions, | ||
| ): OpcUaFieldConfig[] => { | ||
| if (!node.children || node.children.length === 0) { | ||
| return [] | ||
| } | ||
|
|
||
| return node.children.map((child) => { | ||
| // Check if this is a leaf variable (base type) or a complex type | ||
| const isLeaf = child.type === 'variable' && (!child.children || child.children.length === 0) | ||
|
|
||
| if (isLeaf) { | ||
| // Leaf field - no nested fields | ||
| // Use name (not variablePath) as fieldPath - the resolver builds the full path | ||
| // by combining parent path + fieldPath. Using variablePath would duplicate the parent. | ||
| return { | ||
| fieldPath: child.name, | ||
| displayName: child.name, | ||
| datatype: child.variableType || 'UNKNOWN', | ||
| initialValue: getDefaultInitialValue(child.variableType || 'UNKNOWN'), | ||
| permissions: { ...parentPermissions }, | ||
| } | ||
| } else { | ||
| // Complex type (FB, struct, array) - generate nested fields recursively | ||
| const nestedFields = generateDefaultFieldConfigs(child, parentPermissions) | ||
| // Use name (not variablePath) - see comment above | ||
| return { | ||
| fieldPath: child.name, | ||
| displayName: child.name, | ||
| datatype: child.variableType || 'UNKNOWN', | ||
| initialValue: getDefaultInitialValue(child.variableType || 'UNKNOWN'), | ||
| permissions: { ...parentPermissions }, | ||
| fields: nestedFields.length > 0 ? nestedFields : undefined, | ||
| } | ||
| } | ||
| }) | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, find VariableTreeNode definition
rg -n "type VariableTreeNode|interface VariableTreeNode" src/renderer/ -A 5Repository: Autonomy-Logic/openplc-editor
Length of output: 2732
🏁 Script executed:
# Check the use-project-variables.ts file to see how the tree is built
fd -t f "use-project-variables.ts" src/renderer/Repository: Autonomy-Logic/openplc-editor
Length of output: 176
🏁 Script executed:
# Look for cycle/visited patterns in the codebase
rg -n "visited|circular|cycle" src/renderer/components/_features/\[workspace\]/editor/server/opcua-server/ -B 1 -A 1Repository: Autonomy-Logic/openplc-editor
Length of output: 4259
🏁 Script executed:
# Check pou-helpers.ts for the visited Set implementation mentioned in the review
fd -t f "pou-helpers.ts" src/renderer/ -exec cat -n {} \;Repository: Autonomy-Logic/openplc-editor
Length of output: 303
🏁 Script executed:
# Read the use-project-variables.ts file to understand tree construction
wc -l src/renderer/components/_features/\[workspace\]/editor/server/opcua-server/hooks/use-project-variables.tsRepository: Autonomy-Logic/openplc-editor
Length of output: 180
🏁 Script executed:
# Read use-project-variables.ts
cat -n src/renderer/components/_features/\[workspace\]/editor/server/opcua-server/hooks/use-project-variables.ts | head -100Repository: Autonomy-Logic/openplc-editor
Length of output: 3700
🏁 Script executed:
# Find pou-helpers.ts file
find src/renderer -name "pou-helpers.ts" -o -name "*pou*helper*"Repository: Autonomy-Logic/openplc-editor
Length of output: 55
🏁 Script executed:
# Check if findLeafVariables with visited Set exists
rg -n "findLeafVariables" src/renderer/ -A 10Repository: Autonomy-Logic/openplc-editor
Length of output: 55
🏁 Script executed:
# Continue reading use-project-variables.ts from line 100
cat -n src/renderer/components/_features/\[workspace\]/editor/server/opcua-server/hooks/use-project-variables.ts | sed -n '100,250p'Repository: Autonomy-Logic/openplc-editor
Length of output: 5458
🏁 Script executed:
# Find pou-helpers.ts using a simpler search
find src -name "*pou*helper*" -type fRepository: Autonomy-Logic/openplc-editor
Length of output: 97
🏁 Script executed:
# Search for visited Set pattern in pou-helpers
rg -n "visited" src -B 3 -A 3Repository: Autonomy-Logic/openplc-editor
Length of output: 6562
🏁 Script executed:
# Read the variable-config-modal.tsx file around the function in question
cat -n src/renderer/components/_features/\[workspace\]/editor/server/opcua-server/components/variable-config-modal.tsx | sed -n '95,150p'Repository: Autonomy-Logic/openplc-editor
Length of output: 2417
🏁 Script executed:
# Check if there are any cycle prevention mechanisms in use-project-variables tree building
cat -n src/renderer/components/_features/\[workspace\]/editor/server/opcua-server/hooks/use-project-variables.ts | sed -n '250,350p'Repository: Autonomy-Logic/openplc-editor
Length of output: 3446
🏁 Script executed:
# Search for MAX_DEPTH or recursion depth limits
rg -n "MAX.*DEPTH|depth.*limit|recursion.*limit" src/renderer/components/_features/\[workspace\]/editor/server/opcua-server/Repository: Autonomy-Logic/openplc-editor
Length of output: 55
🏁 Script executed:
# Check if there's any depth tracking in the tree building
rg -n "depth|maxDepth|parentPath" src/renderer/components/_features/\[workspace\]/editor/server/opcua-server/hooks/use-project-variables.ts | head -20Repository: Autonomy-Logic/openplc-editor
Length of output: 766
🏁 Script executed:
# Verify how the tree is used - check if it's sanitized before being passed to generateDefaultFieldConfigs
grep -n "generateDefaultFieldConfigs" src/renderer/components/_features/\[workspace\]/editor/server/opcua-server/components/variable-config-modal.tsx -B 5 -A 5Repository: Autonomy-Logic/openplc-editor
Length of output: 1550
🏁 Script executed:
# Check how the tree is initially built - see if there's validation preventing circular types
rg -n "buildProjectVariableTree\|createVariableTree" src/renderer/components/_features/\[workspace\]/editor/server/opcua-server/hooks/use-project-variables.ts -A 20Repository: Autonomy-Logic/openplc-editor
Length of output: 55
🏁 Script executed:
# Check if PLCDataType or PLCPou validation prevents circular definitions
rg -n "validate|circular|cycle" src/utils/ | grep -i "type\|pou"Repository: Autonomy-Logic/openplc-editor
Length of output: 1438
🏁 Script executed:
# Check if there are circular type definitions possible in the project structure
# Look at how data types are loaded/validated
rg -n "PLCDataType\|PLCPou" src/types/ -l | head -5Repository: Autonomy-Logic/openplc-editor
Length of output: 55
🏁 Script executed:
# Check if there's any validation when loading project data that prevents circular types
rg -n "circular\|cycle\|validate.*type" src/main/ -i | grep -E "load|import|validate" | head -10Repository: Autonomy-Logic/openplc-editor
Length of output: 55
🏁 Script executed:
# Let's trace back: if findLeafVariables has visited Set protection, it suggests circular types are possible
# Let's see if buildVariableNode is ever used with data that could be circular
grep -n "buildVariableNode\|buildVariableTree" src/renderer/components/_features/\[workspace\]/editor/server/opcua-server/hooks/use-project-variables.ts -B 2 -A 2Repository: Autonomy-Logic/openplc-editor
Length of output: 1720
Add recursion protection for circular type references in generateDefaultFieldConfigs.
This function recursively processes nested structures without safeguards against cycles. While findLeafVariables in pou-helpers.ts already demonstrates the team's awareness of this risk (using a visited Set to prevent infinite recursion on circular type references), the tree-building phase in buildVariableNode and this field-config generation function lack similar protection.
If VariableTreeNode structures ever contain circular references—whether through user-defined types, complex function blocks, or data type configurations—this function would cause a stack overflow. Add a visited Set parameter (similar to findLeafVariables) to track processed nodes by their unique identifier and prevent re-entering the same node during recursion.
🤖 Prompt for AI Agents
In
`@src/renderer/components/_features/`[workspace]/editor/server/opcua-server/components/variable-config-modal.tsx
around lines 100 - 137, generateDefaultFieldConfigs currently recurses through
VariableTreeNode.children without cycle protection; add a visited Set parameter
(e.g., visited: Set<string>) and check the node's unique id (or
variablePath/name if unique) at the start of generateDefaultFieldConfigs to
return [] when already visited, mirror the pattern used in findLeafVariables,
mark nodes as visited before recursing into children, and pass the same visited
set into recursive calls so circular type references in VariableTreeNode (and
interactions with buildVariableNode) cannot cause infinite recursion/stack
overflow.
- Standardize font sizes across all form fields (text-cp-sm → text-xs) - Replace emoji password icons (👁️/🙈) with proper SVG icons (ViewIcon/ViewHiddenIcon) - Add ViewHiddenIcon component for password visibility toggle - Disable password visibility toggle when field is empty (hashed passwords can't be shown) - Remove inconsistent "!" icon from insecure security profile warning - Update Modbus and S7Comm server editors for font consistency Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In
`@src/renderer/components/_features/`[workspace]/editor/server/opcua-server/components/certificates-tab.tsx:
- Around line 26-115: The file browse handlers (handleBrowseCertFile and
handleBrowseKeyFile) silently ignore file.text() rejections—add a local state
(e.g., fileReadError via useState) and update it on failure; wrap file.text()
with .then(...).catch(err => setFileReadError(String(err))) and clear the error
on success (e.g., inside handleCustomCertChange/handleCustomKeyChange or after
successful read), and render the error message in the CertificatesTab component
UI near the cert/key inputs so users see read failures; reference the functions
handleBrowseCertFile, handleBrowseKeyFile, handleCustomCertChange,
handleCustomKeyChange, and state setters setCustomCert/setCustomKey when making
these changes.
🧹 Nitpick comments (4)
src/renderer/assets/icons/interface/ViewHidden.tsx (2)
7-7: Consider using conventionalrestnaming.The destructured variable
resis unconventional. The standard pattern is...restfor remaining props spread.Suggested naming fix
- const { stroke, className, size = 'sm', ...res } = props + const { stroke, className, size = 'sm', ...rest } = propsAnd on line 16:
- {...res} + {...rest}
15-15: Simplify template literal.The template literal wrapping
sizeClassesis redundant since it's already a string.Suggested fix
- className={cn(`${sizeClasses}`, className)} + className={cn(sizeClasses, className)}src/renderer/components/_features/[workspace]/editor/server/opcua-server/components/variable-config-modal.tsx (2)
100-137: Consider adding recursion depth limit for defensive safety.While the tree-building phase in
use-project-variables.tsshould prevent circular references, this function has no safeguard against malformed input. A depth limit would provide defense-in-depth against stack overflow in edge cases.♻️ Optional defensive enhancement
const generateDefaultFieldConfigs = ( node: VariableTreeNode, parentPermissions: OpcUaPermissions, + depth: number = 0, ): OpcUaFieldConfig[] => { + const MAX_DEPTH = 20 // Reasonable limit for IEC 61131-3 structures + if (depth > MAX_DEPTH) { + console.warn('generateDefaultFieldConfigs: max depth exceeded, truncating') + return [] + } + if (!node.children || node.children.length === 0) { return [] } return node.children.map((child) => { const isLeaf = child.type === 'variable' && (!child.children || child.children.length === 0) if (isLeaf) { return { fieldPath: child.name, displayName: child.name, datatype: child.variableType || 'UNKNOWN', initialValue: getDefaultInitialValue(child.variableType || 'UNKNOWN'), permissions: { ...parentPermissions }, } } else { - const nestedFields = generateDefaultFieldConfigs(child, parentPermissions) + const nestedFields = generateDefaultFieldConfigs(child, parentPermissions, depth + 1) return { fieldPath: child.name, displayName: child.name, datatype: child.variableType || 'UNKNOWN', initialValue: getDefaultInitialValue(child.variableType || 'UNKNOWN'), permissions: { ...parentPermissions }, fields: nestedFields.length > 0 ? nestedFields : undefined, } } }) }
487-637: Consider extracting repeated permission select markup.The Viewer, Operator, and Engineer permission selects share nearly identical markup. The inline
PermissionSelectcomponent exists but isn't used here. Consider refactoring to reduce duplication.♻️ Optional: Use PermissionSelect component for main permissions
Create a variant of the existing
PermissionSelectthat supports the larger styling, or extract theSelectContentitems into a shared constant:const PERMISSION_OPTIONS = [ { value: 'r', label: 'Read Only' }, { value: 'w', label: 'Write Only' }, { value: 'rw', label: 'Read/Write' }, ] as const // Then map over PERMISSION_OPTIONS in each Select's content
| const [isModalOpen, setIsModalOpen] = useState(false) | ||
| const [customCert, setCustomCert] = useState(config.security.serverCertificateCustom ?? '') | ||
| const [customKey, setCustomKey] = useState(config.security.serverPrivateKeyCustom ?? '') | ||
|
|
||
| // Get existing certificate IDs for validation | ||
| const existingCertificateIds = useMemo( | ||
| () => config.security.trustedClientCertificates.map((c) => c.id), | ||
| [config.security.trustedClientCertificates], | ||
| ) | ||
|
|
||
| // Handle strategy change | ||
| const handleStrategyChange = useCallback( | ||
| (strategy: CertificateStrategy) => { | ||
| if (strategy === 'custom') { | ||
| updateOpcUaServerCertificateStrategy(serverName, strategy, customCert || null, customKey || null) | ||
| } else { | ||
| updateOpcUaServerCertificateStrategy(serverName, strategy, null, null) | ||
| } | ||
| onConfigChange() | ||
| }, | ||
| [serverName, customCert, customKey, updateOpcUaServerCertificateStrategy, onConfigChange], | ||
| ) | ||
|
|
||
| // Handle custom certificate change | ||
| const handleCustomCertChange = useCallback( | ||
| (cert: string) => { | ||
| setCustomCert(cert) | ||
| if (config.security.serverCertificateStrategy === 'custom') { | ||
| updateOpcUaServerCertificateStrategy(serverName, 'custom', cert || null, customKey || null) | ||
| onConfigChange() | ||
| } | ||
| }, | ||
| [ | ||
| serverName, | ||
| customKey, | ||
| config.security.serverCertificateStrategy, | ||
| updateOpcUaServerCertificateStrategy, | ||
| onConfigChange, | ||
| ], | ||
| ) | ||
|
|
||
| // Handle custom key change | ||
| const handleCustomKeyChange = useCallback( | ||
| (key: string) => { | ||
| setCustomKey(key) | ||
| if (config.security.serverCertificateStrategy === 'custom') { | ||
| updateOpcUaServerCertificateStrategy(serverName, 'custom', customCert || null, key || null) | ||
| onConfigChange() | ||
| } | ||
| }, | ||
| [ | ||
| serverName, | ||
| customCert, | ||
| config.security.serverCertificateStrategy, | ||
| updateOpcUaServerCertificateStrategy, | ||
| onConfigChange, | ||
| ], | ||
| ) | ||
|
|
||
| // Handle file browse for certificate | ||
| const handleBrowseCertFile = useCallback(() => { | ||
| const input = document.createElement('input') | ||
| input.type = 'file' | ||
| input.accept = '.pem,.crt,.cer' | ||
| input.onchange = (e) => { | ||
| const file = (e.target as HTMLInputElement).files?.[0] | ||
| if (file) { | ||
| void file.text().then((content) => { | ||
| handleCustomCertChange(content) | ||
| }) | ||
| } | ||
| } | ||
| input.click() | ||
| }, [handleCustomCertChange]) | ||
|
|
||
| // Handle file browse for key | ||
| const handleBrowseKeyFile = useCallback(() => { | ||
| const input = document.createElement('input') | ||
| input.type = 'file' | ||
| input.accept = '.pem,.key' | ||
| input.onchange = (e) => { | ||
| const file = (e.target as HTMLInputElement).files?.[0] | ||
| if (file) { | ||
| void file.text().then((content) => { | ||
| handleCustomKeyChange(content) | ||
| }) | ||
| } | ||
| } | ||
| input.click() | ||
| }, [handleCustomKeyChange]) |
There was a problem hiding this comment.
Surface file read failures when browsing cert/key files.
file.text() can reject (permissions/encoding/size). Right now failures are silent.
🛠️ Proposed fix
- const [isModalOpen, setIsModalOpen] = useState(false)
+ const [isModalOpen, setIsModalOpen] = useState(false)
+ const [fileReadError, setFileReadError] = useState<string | null>(null)
const [customCert, setCustomCert] = useState(config.security.serverCertificateCustom ?? '')
const [customKey, setCustomKey] = useState(config.security.serverPrivateKeyCustom ?? '')
...
input.onchange = (e) => {
const file = (e.target as HTMLInputElement).files?.[0]
if (file) {
- void file.text().then((content) => {
- handleCustomCertChange(content)
- })
+ file
+ .text()
+ .then((content) => {
+ setFileReadError(null)
+ handleCustomCertChange(content)
+ })
+ .catch(() => {
+ setFileReadError('Failed to read certificate file')
+ })
}
}
...
input.onchange = (e) => {
const file = (e.target as HTMLInputElement).files?.[0]
if (file) {
- void file.text().then((content) => {
- handleCustomKeyChange(content)
- })
+ file
+ .text()
+ .then((content) => {
+ setFileReadError(null)
+ handleCustomKeyChange(content)
+ })
+ .catch(() => {
+ setFileReadError('Failed to read private key file')
+ })
}
}{fileReadError && (
<span className='text-xs text-red-600 dark:text-red-400'>{fileReadError}</span>
)}🤖 Prompt for AI Agents
In
`@src/renderer/components/_features/`[workspace]/editor/server/opcua-server/components/certificates-tab.tsx
around lines 26 - 115, The file browse handlers (handleBrowseCertFile and
handleBrowseKeyFile) silently ignore file.text() rejections—add a local state
(e.g., fileReadError via useState) and update it on failure; wrap file.text()
with .then(...).catch(err => setFileReadError(String(err))) and clear the error
on success (e.g., inside handleCustomCertChange/handleCustomKeyChange or after
successful read), and render the error message in the CertificatesTab component
UI near the cert/key inputs so users see read failures; reference the functions
handleBrowseCertFile, handleBrowseKeyFile, handleCustomCertChange,
handleCustomKeyChange, and state setters setCustomCert/setCustomKey when making
these changes.
The @tailwindcss/forms plugin applies base font-size styling to form inputs that was overriding Tailwind utility classes. Using !text-xs ensures consistent font sizes across all server editor form fields. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Summary
This PR adds comprehensive OPC-UA server configuration support to OpenPLC Editor, enabling users to configure OPC-UA servers for PLC runtime communication.
Key Features
OPC-UA Server Configuration UI (Phases 1-4)
Complex Type Support (Phases 5-6)
Shared Debug Utilities Refactoring
Files Changed
New Modules
src/utils/opcua/- OPC-UA configuration generation and index resolutionsrc/utils/debug-parser.ts- Canonical debug.c parsingsrc/utils/debug-variable-finder.ts- Shared variable index lookup utilitiessrc/utils/debug-tree-traversal.ts- Visitor-pattern tree traversalsrc/utils/pou-helpers.ts- POU lookup utilitiesTest plan
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Documentation
Tests
Bug Fixes
✏️ Tip: You can customize this high-level summary in your review settings.