Add runtime version detection to prevent version mismatch errors#477
Conversation
- Update handleRuntimeGetUsersInfo to capture X-OpenPLC-Runtime-Version header - Add runtimeVersion field to IPC bridge return type - Add validateRuntimeVersion utility function to check version compatibility - Show error modal when selected runtime version doesn't match connected runtime - Helps users identify when they're connecting to wrong runtime version Co-Authored-By: Thiago Alves <thiagoralves@gmail.com>
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
|
Important Review skippedBot user detected. To trigger a single review, invoke the You can disable this status message by setting the Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. Comment |
There was a problem hiding this comment.
Pull request overview
This PR implements runtime version detection to prevent users from connecting to an incompatible OpenPLC runtime version. It extracts the runtime version from HTTP response headers and validates it against the selected board target (e.g., "OpenPLC Runtime v3" vs "OpenPLC Runtime v4").
Key changes:
- Added utility functions to extract expected runtime version from board target names and validate against detected versions
- Modified the connection flow to capture and validate the
X-OpenPLC-Runtime-Versionheader from runtime API responses - Display error modal when version mismatch is detected or when runtime version cannot be determined
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| src/utils/device.ts | Added getExpectedRuntimeVersion and validateRuntimeVersion utility functions for version extraction and validation |
| src/renderer/components/_features/[workspace]/editor/device/configuration/board.tsx | Integrated version validation into connection flow with error modal display |
| src/main/modules/ipc/renderer.ts | Updated return type for runtimeGetUsersInfo to include optional runtimeVersion field |
| src/main/modules/ipc/main.ts | Captured X-OpenPLC-Runtime-Version header from HTTP responses and included in return values |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if (normalizedDetected !== normalizedExpected) { | ||
| return { | ||
| isValid: false, | ||
| errorMessage: `Runtime version mismatch: Selected "${boardTarget}" but connected to a ${detectedVersion.toUpperCase()} runtime. Please update your device configuration to match the connected runtime.`, |
There was a problem hiding this comment.
The error message uses detectedVersion.toUpperCase() but detectedVersion could be undefined at this point in the code flow. While the undefined check exists at line 61, TypeScript's type narrowing may not carry through here. Consider using normalizedDetected.toUpperCase() instead, which is guaranteed to be defined at line 75.
| errorMessage: `Runtime version mismatch: Selected "${boardTarget}" but connected to a ${detectedVersion.toUpperCase()} runtime. Please update your device configuration to match the connected runtime.`, | |
| errorMessage: `Runtime version mismatch: Selected "${boardTarget}" but connected to a ${normalizedDetected.toUpperCase()} runtime. Please update your device configuration to match the connected runtime.`, |
|
|
||
| // Normalize versions for comparison (both should be lowercase like "v3", "v4") | ||
| const normalizedDetected = detectedVersion.toLowerCase() | ||
| const normalizedExpected = expectedVersion.toLowerCase() |
There was a problem hiding this comment.
The expectedVersion is already normalized to lowercase by getExpectedRuntimeVersion (line 39), making the second .toLowerCase() call on line 70 redundant. Consider removing it or adding a comment explaining why double-normalization is intentional.
| const normalizedExpected = expectedVersion.toLowerCase() | |
| const normalizedExpected = expectedVersion |
- Use normalizedDetected.toUpperCase() instead of detectedVersion.toUpperCase() for safer code - Remove redundant .toLowerCase() on expectedVersion since getExpectedRuntimeVersion already returns lowercase Co-Authored-By: Thiago Alves <thiagoralves@gmail.com>
… option - Modify validateRuntimeVersion to return status: 'ok' | 'missing' | 'mismatch' - Show warning dialog for older runtimes without version header - Add 'Continue Anyway' and 'Cancel' buttons to allow user to proceed - Update message to suggest updating runtime to latest version Co-Authored-By: Thiago Alves <thiagoralves@gmail.com>
When the debugger-message modal's onResponse callback opens another modal, the subsequent closeModal() call was closing ALL modals including the newly opened one. Fix by capturing the callback, closing the modal first, then calling the callback. This fixes the issue where clicking 'Continue Anyway' on the older runtime warning dialog would not show the login/create-user dialog. Co-Authored-By: Thiago Alves <thiagoralves@gmail.com>
|
Cool thanx. Will test. |
Pull request info
References
This PR is part of a coordinated change across three repositories to implement runtime version detection:
Link to Devin run
Devin session
Requested by: Thiago Alves (@thiagoralves)
Description of the changes proposed
X-OpenPLC-Runtime-VersionHTTP header from runtime API responses inhandleRuntimeGetUsersInforuntimeVersionfield to the IPC bridge return type forruntimeGetUsersInfogetExpectedRuntimeVersionandvalidateRuntimeVersioninsrc/utils/device.tsThis prevents users from accidentally connecting to the wrong runtime version (e.g., selecting "OpenPLC Runtime v4" but connecting to a v3 runtime), while still allowing connections to older runtimes that don't report version.
Updates since last revision
Fixed race condition in debugger-message-modal:
onResponsecallback opens another modal (e.g., login dialog), the subsequentcloseModal()was closing ALL modals including the newly opened onePrevious updates:
validateRuntimeVersionnow returnsstatus: 'ok' | 'missing' | 'mismatch'instead of justisValid: booleannormalizedDetected.toUpperCase()instead ofdetectedVersion.toUpperCase()for safer code.toLowerCase()call onexpectedVersionImportant review notes
Companion PRs merged: The runtime repositories (OpenPLC_v3 and openplc-runtime) now send the
X-OpenPLC-Runtime-Versionheader.Button order for warning dialog: Buttons are ordered as
['Continue Anyway', 'Cancel']so that Cancel (index 1) is triggered when closing the modal via Esc or clicking outside.Modal callback order change: The
DebuggerMessageModalnow closes before callingonResponse. This is necessary to allow callbacks to open new modals. Other existing uses of this modal (e.g.,showDebuggerMessagein workspace-activity-bar) just resolve a Promise and are unaffected.Regex pattern: The version extraction uses
/OpenPLC Runtime (v\d+)/i- verify this matches actual board target names inhals.json.Suggested review checklist
hals.jsonDOD checklist