Complete ModbusLab remediation and recovery hardening#2
Merged
Conversation
There was a problem hiding this comment.
Pull request overview
This PR hardens ModbusLab’s runtime behaviors (startup memory, response delays, simulation arming, and reaction semantics) and introduces a unified domain validation layer shared by the WPF UI and the MCP surface. It also upgrades project persistence to a versioned, atomic/verified save format with guided corrupt-project recovery, and adds a Windows CI baseline plus expanded test/docs coverage.
Changes:
- Add versioned project format (v2), atomic verified saves, and guided recovery paths for corrupt or unsupported project files.
- Centralize validation rules in a new
DomainValidator, and apply them consistently across UI editors and MCP tools (including read-only validation responses). - Remediate runtime semantics (response delay placement/cancellation, simulation requested state, reaction increment behavior, startup memory application), with new xUnit coverage and Windows CI.
Reviewed changes
Copilot reviewed 42 out of 76 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| scripts/prepare-release.ps1 | Run build with warnings-as-errors and run tests during release prep. |
| README.md | Update build/test instructions and document pinned SDK/global.json usage. |
| ModbusLab/Windows/SimulationEditorWindow.xaml.cs | Apply shared domain validation before accepting simulation rule edits. |
| ModbusLab/Windows/ReactionEditorWindow.xaml.cs | Apply shared domain validation and normalize reaction name on save. |
| ModbusLab/Windows/FaultRuleEditorWindow.xaml.cs | Add function code 22 and validate fault rules before saving. |
| ModbusLab/Views/MainWindow.xaml.cs | Accept ProjectLoadResult, defer autosave/MCP start until recovery confirmation. |
| ModbusLab/Views/MainWindow.xaml | Add watch grid row edit handling hook. |
| ModbusLab/Views/MainWindow.Watch.xaml.cs | Validate watches on add/edit, enforce unique default watch naming, and tighten selection-change normalization. |
| ModbusLab/Views/MainWindow.UiState.xaml.cs | Allow UI state capture without forcing an immediate save. |
| ModbusLab/Views/MainWindow.Simulation.xaml.cs | Move simulation “requested/armed” state into runtime/viewmodel instead of local HashSet. |
| ModbusLab/Views/MainWindow.Servers.xaml.cs | Gate startup behaviors on recovery confirmation; validate server settings and surface recovery UI. |
| ModbusLab/Views/MainWindow.Live.xaml.cs | Surface save failures alongside runtime errors in the status area. |
| ModbusLab/Views/MainWindow.Diagnostics.xaml.cs | Include validation/save issues in diagnostics summary and self-test output. |
| ModbusLab/ViewModels/MainWindowViewModel.cs | Add project warnings/save error tracking, centralize simulation requested state, make SaveNow return results, and apply startup memory via runtime store. |
| ModbusLab/Validation/ValidationResult.cs | New validation result/issue primitives and a domain validation exception type. |
| ModbusLab/Validation/DomainValidator.cs | New centralized validator for MCP settings, server settings, watches, rules, projects, and watch writes. |
| ModbusLab/Runtime/SimulationEngine.cs | Write address simulations as block writes and invoke local reactions once per range. |
| ModbusLab/Runtime/ServerRuntime.cs | Add persisted runtime flag for simulation requested/armed state. |
| ModbusLab/Runtime/ModbusTcpServer.Reactions.cs | Improve increment semantics to support configured steps and typed watches. |
| ModbusLab/Runtime/ModbusTcpServer.cs | Harden startup/stop lifecycle and allow pre-accept initialization after bind. |
| ModbusLab/Runtime/ModbusTcpServer.ClientLoop.cs | Apply global response delay early in the request path. |
| ModbusLab/Runtime/ModbusDataStore.cs | Add snapshot, clear, and apply-memory helpers with synchronization. |
| ModbusLab/Models/WatchItem.cs | Implement IEditableObject for DataGrid edits; stop auto-normalizing fields at setter level. |
| ModbusLab/Models/ProjectStoreResults.cs | New load/save result types and recovery state enums. |
| ModbusLab/Models/ProjectStore.cs | Introduce format versioning, migrations, atomic verified save, backup search/recovery, and blocked/unsupported startup paths. |
| ModbusLab/Models/ProjectModel.cs | Add FormatVersion for versioned persistence. |
| ModbusLab/Models/McpSettings.cs | Make mutation permission default to read-only (AllowMutations default false). |
| ModbusLab/Mcp/ModbusLabMcpTools.cs | Mark MCP settings update tool as destructive and document mutation escalation rules. |
| ModbusLab/Mcp/ModbusLabMcpFacade.cs | Centralize MCP validation, enforce domain validation for mutations, and improve diagnostics payloads. |
| ModbusLab/App.xaml.cs | Block startup on unsupported/blocked projects; pass load result into main window. |
| ModbusLab.Tests/RuntimeRemediationTests.cs | New regression tests for startup memory, delays, simulation arming, and reaction steps. |
| ModbusLab.Tests/RuntimeAcceptanceTests.cs | New acceptance-level tests for delay semantics, simulation/reaction behavior, and snapshot consistency. |
| ModbusLab.Tests/ProjectStoreTests.cs | New tests for versioning, migrations, atomic save, backups, and recovery flows. |
| ModbusLab.Tests/McpValidationTests.cs | New tests for MCP read-only defaults and validation/mutation behaviors. |
| ModbusLab.Tests/DomainValidatorTests.cs | New unit tests for validator semantics across domain objects. |
| global.json | Pin .NET SDK used for builds/CI. |
| docs/user-guide/settings.md | Document startup memory and response delay semantics; expand project storage details. |
| docs/user-guide/mcp.md | Document mutation default-off and structured validation responses. |
| docs/troubleshooting.md | Add recovery/save failure guidance and update build-from-source steps. |
| docs/release-notes.md | Add release notes for persistence/validation/runtime hardening and CI/test additions. |
| docs/assets/screenshot-seed/ModbusLab.screenshots.modbuslab.json | Update seeded project to format v2 and new settings defaults. |
| .github/workflows/build.yml | Add Windows CI workflow to restore/build/test with pinned SDK. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+192
to
+211
| var current = ReadWatchValue(watch); | ||
| var step = GetReactionIncrementStep(rule.Value); | ||
| if (watch.DataType is WatchDataTypes.Float32 or WatchDataTypes.Double) | ||
| { | ||
| if (!TryParseFlexibleDouble(current, out var currentValue)) | ||
| { | ||
| throw new FormatException($"Watch '{watch.Name}' does not contain a numeric value."); | ||
| } | ||
|
|
||
| WriteWatchValue(watch, (currentValue + step).ToString("G17", CultureInfo.InvariantCulture)); | ||
| } | ||
| else | ||
| { | ||
| if (!TryParseFlexibleDecimal(current, out var currentValue)) | ||
| { | ||
| throw new FormatException($"Watch '{watch.Name}' does not contain an integral value."); | ||
| } | ||
|
|
||
| WriteWatchValue(watch, (currentValue + (decimal)step).ToString(CultureInfo.InvariantCulture)); | ||
| } |
Comment on lines
+223
to
+224
| var rawStep = (ushort)Math.Clamp(Math.Round(GetReactionIncrementStep(rule.Value)), ushort.MinValue, ushort.MaxValue); | ||
| WriteReactionValue(rule.TargetArea, rule.TargetAddress, unchecked((ushort)(value + rawStep))); |
Comment on lines
+78
to
+82
| if (_profile.ResponseDelayMs > 0) | ||
| { | ||
| await Task.Delay(_profile.ResponseDelayMs, token); | ||
| } | ||
|
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Verification