From e698aee401e679e5066d41753d41366522bdf126 Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Wed, 5 Aug 2026 03:21:23 -0400 Subject: [PATCH 1/8] test(raw): FakeRawTreeService for raw-modal component tests --- .../Raw/FakeRawTreeService.cs | 127 ++++++++++++++++++ 1 file changed, 127 insertions(+) create mode 100644 tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/Raw/FakeRawTreeService.cs diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/Raw/FakeRawTreeService.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/Raw/FakeRawTreeService.cs new file mode 100644 index 00000000..9c11df75 --- /dev/null +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/Raw/FakeRawTreeService.cs @@ -0,0 +1,127 @@ +using ZB.MOM.WW.OtOpcUa.AdminUI.Uns; + +namespace ZB.MOM.WW.OtOpcUa.AdminUI.Tests.Raw; + +/// +/// Minimal fake for component-class tests (AdminUI has no bUnit, so the +/// raw modals are exercised as plain classes with this injected). Only the raw modals' open-path +/// members — and — are +/// configurable, via the two settable delegates; every other member throws, so an unexpected call +/// fails the test instead of passing vacuously. The two delegates themselves default to throwing for +/// the same reason: a test must opt in to the calls it expects. +/// +internal sealed class FakeRawTreeService : IRawTreeService +{ + /// The stand-in; throws until a test sets it. + public Func OnLoadTagForEdit { get; set; } = + _ => throw new NotImplementedException(); + + /// The stand-in; throws until a test sets it. + public Func OnLoadMergedProbeConfig { get; set; } = + _ => throw new NotImplementedException(); + + /// + public Task LoadTagForEditAsync(string tagId, CancellationToken ct = default) + => Task.FromResult(OnLoadTagForEdit(tagId)); + + /// + public Task<(string DriverType, string MergedConfigJson)?> LoadMergedProbeConfigAsync( + string deviceId, CancellationToken ct = default) + => Task.FromResult(OnLoadMergedProbeConfig(deviceId)); + + /// + public Task> LoadRootsAsync(CancellationToken ct = default) + => throw new NotImplementedException(); + + /// + public Task> LoadChildrenAsync(RawNode parent, CancellationToken ct = default) + => throw new NotImplementedException(); + + /// + public Task CreateFolderAsync(string clusterId, string? parentFolderId, string name, CancellationToken ct = default) + => throw new NotImplementedException(); + + /// + public Task RenameFolderAsync(string rawFolderId, string newName, byte[] rowVersion, CancellationToken ct = default) + => throw new NotImplementedException(); + + /// + public Task DeleteFolderAsync(string rawFolderId, byte[] rowVersion, CancellationToken ct = default) + => throw new NotImplementedException(); + + /// + public Task MoveDriverToFolderAsync(string driverInstanceId, string? targetFolderId, byte[] rowVersion, CancellationToken ct = default) + => throw new NotImplementedException(); + + /// + public Task CreateDriverAsync(string clusterId, string? folderId, string name, string driverType, string driverConfigJson, CancellationToken ct = default) + => throw new NotImplementedException(); + + /// + public Task RenameDriverAsync(string driverInstanceId, string newName, byte[] rowVersion, CancellationToken ct = default) + => throw new NotImplementedException(); + + /// + public Task SetDriverEnabledAsync(string driverInstanceId, bool enabled, byte[] rowVersion, CancellationToken ct = default) + => throw new NotImplementedException(); + + /// + public Task DeleteDriverAsync(string driverInstanceId, byte[] rowVersion, CancellationToken ct = default) + => throw new NotImplementedException(); + + /// + public Task CreateDeviceAsync(string driverInstanceId, string name, string deviceConfigJson, CancellationToken ct = default) + => throw new NotImplementedException(); + + /// + public Task RenameDeviceAsync(string deviceId, string newName, byte[] rowVersion, CancellationToken ct = default) + => throw new NotImplementedException(); + + /// + public Task DeleteDeviceAsync(string deviceId, byte[] rowVersion, CancellationToken ct = default) + => throw new NotImplementedException(); + + /// + public Task CreateTagGroupAsync(string deviceId, string? parentGroupId, string name, CancellationToken ct = default) + => throw new NotImplementedException(); + + /// + public Task RenameTagGroupAsync(string tagGroupId, string newName, byte[] rowVersion, CancellationToken ct = default) + => throw new NotImplementedException(); + + /// + public Task DeleteTagGroupAsync(string tagGroupId, byte[] rowVersion, CancellationToken ct = default) + => throw new NotImplementedException(); + + /// + public Task DeleteTagAsync(string tagId, byte[] rowVersion, CancellationToken ct = default) + => throw new NotImplementedException(); + + /// + public Task CreateTagAsync(string deviceId, string? tagGroupId, RawTagInput input, CancellationToken ct = default) + => throw new NotImplementedException(); + + /// + public Task UpdateTagAsync(string tagId, RawTagInput input, byte[] rowVersion, CancellationToken ct = default) + => throw new NotImplementedException(); + + /// + public Task ImportTagsAsync(string deviceId, IReadOnlyList rows, CancellationToken ct = default) + => throw new NotImplementedException(); + + /// + public Task LoadDriverForEditAsync(string driverInstanceId, CancellationToken ct = default) + => throw new NotImplementedException(); + + /// + public Task UpdateDriverAsync(string driverInstanceId, string name, string driverConfigJson, string? resilienceConfig, byte[] rowVersion, CancellationToken ct = default) + => throw new NotImplementedException(); + + /// + public Task LoadDeviceForEditAsync(string deviceId, CancellationToken ct = default) + => throw new NotImplementedException(); + + /// + public Task UpdateDeviceAsync(string deviceId, string name, string deviceConfigJson, bool enabled, byte[] rowVersion, CancellationToken ct = default) + => throw new NotImplementedException(); +} From abb9f8a1cb1565707ef7a77928d441d111adf4d9 Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Wed, 5 Aug 2026 03:25:13 -0400 Subject: [PATCH 2/8] fix(raw): tag edit modal feeds the picker the merged device config, not '{}' (#468 follow-up) --- .../Components/Shared/Raw/RawTagModal.razor | 30 +++- .../Raw/RawTagModalMergedConfigTests.cs | 140 ++++++++++++++++++ 2 files changed, 167 insertions(+), 3 deletions(-) create mode 100644 tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/Raw/RawTagModalMergedConfigTests.cs diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Shared/Raw/RawTagModal.razor b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Shared/Raw/RawTagModal.razor index 4f6ed3f2..8e6ad6e1 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Shared/Raw/RawTagModal.razor +++ b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Shared/Raw/RawTagModal.razor @@ -3,7 +3,8 @@ passed in and drives the typed TagConfig editor dispatch (TagConfigEditorMap; raw-JSON fallback for unmapped drivers, e.g. Galaxy). Edit-only: the host opens it for an existing tree node (TagId), it loads via LoadTagForEditAsync, and saves via UpdateTagAsync. Visibility is @bind-Visible; a successful - save raises OnSaved so the coordinator can refresh the tree. *@ + save raises OnSaved so the coordinator can refresh the tree. The editor also receives the merged + Driver+Device probe config, which is what its address picker dials when browsing the live device. *@ @using System.ComponentModel.DataAnnotations @using Microsoft.AspNetCore.Components.Forms @using ZB.MOM.WW.OtOpcUa.AdminUI.Uns @@ -140,11 +141,17 @@ // reload + clobber in-progress edits. Null while closed. private string? _loadedKey; + // Merged Driver+Device config for the typed editor's address picker — the same blob Test-connect and + // RawBrowseModal dial, resolved once per open. Null (→ "{}") when the resolve failed: the picker's + // browse then degrades to its graceful error chip. + private string? _mergedConfigJson; + protected override async Task OnParametersSetAsync() { if (!Visible) { _loadedKey = null; // closed → next open reloads fresh + _mergedConfigJson = null; return; } @@ -180,14 +187,31 @@ TagConfig = string.IsNullOrWhiteSpace(d.TagConfig) ? "{}" : d.TagConfig, }; _rowVersion = d.RowVersion; + + _mergedConfigJson = null; + try + { + var merged = await Svc.LoadMergedProbeConfigAsync(d.DeviceId); + if (merged is { } m) + { + _mergedConfigJson = m.MergedConfigJson; + } + } + catch + { + // Fail-soft: the modal must open and save regardless of the probe-config resolve. Only the + // browse affordance degrades, and it reports its own error when used. + } } - private IDictionary BuildEditorParameters() => new Dictionary + /// Parameters handed to the typed TagConfig editor. Internal so tests can assert them. + internal IDictionary BuildEditorParameters() => new Dictionary { ["ConfigJson"] = _form.TagConfig, ["ConfigJsonChanged"] = EventCallback.Factory.Create(this, v => _form.TagConfig = v), ["DriverType"] = DriverType ?? "", - ["GetDriverConfigJson"] = (Func)(() => "{}"), + // Live accessor, not a snapshot: the picker reads it when the operator clicks Browse. + ["GetDriverConfigJson"] = (Func)(() => _mergedConfigJson ?? "{}"), }; private async Task SaveAsync() diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/Raw/RawTagModalMergedConfigTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/Raw/RawTagModalMergedConfigTests.cs new file mode 100644 index 00000000..3ca5dd50 --- /dev/null +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/Raw/RawTagModalMergedConfigTests.cs @@ -0,0 +1,140 @@ +using System.Reflection; +using Shouldly; +using Xunit; +using ZB.MOM.WW.OtOpcUa.AdminUI.Components.Shared.Raw; +using ZB.MOM.WW.OtOpcUa.AdminUI.Uns; +using ZB.MOM.WW.OtOpcUa.Configuration.Enums; + +namespace ZB.MOM.WW.OtOpcUa.AdminUI.Tests.Raw; + +/// +/// Pins the #468 follow-up: the /raw tag edit modal must hand its typed TagConfig editor the +/// merged Driver+Device config, not the historical "{}" stub. +/// +/// +/// +/// The AbCip / TwinCAT / FOCAS tag editors embed an address picker whose "Browse controller" +/// affordance dials the GetDriverConfigJson accessor as its connect config. While that +/// accessor returned a literal "{}" the browse captured against an empty config and could +/// never succeed — a defect no offline test could see, because the accessor was well-formed JSON. +/// +/// +/// AdminUI has no bUnit, so the component is driven directly as a class: the injected +/// Svc property and the protected OnParametersSetAsync lifecycle hook are reached by +/// reflection, and the editor parameters are read back through the internal +/// BuildEditorParameters. +/// +/// +// BL0005 forbids setting a [Parameter] from outside the component, which is right for production code +// and impossible to honour here: with no bUnit there is no renderer to supply parameters, so driving the +// component as a class means assigning them directly. +#pragma warning disable BL0005 +public sealed class RawTagModalMergedConfigTests +{ + private const string MergedConfig = """{"endpoint":"10.0.0.1:44818","slot":0}"""; + + [Fact] + public async Task The_editor_receives_the_merged_device_config_not_a_stub() + { + var svc = new FakeRawTreeService + { + OnLoadTagForEdit = tagId => TagOn(tagId, "dev-1"), + OnLoadMergedProbeConfig = deviceId => + deviceId == "dev-1" ? ("AbCip", MergedConfig) : null, + }; + + var modal = NewModal(svc, "tag-1"); + await InvokeParametersSetAsync(modal); + + DriverConfigJson(modal).ShouldBe(MergedConfig); + } + + [Fact] + public async Task A_failed_resolve_degrades_to_the_empty_stub_and_does_not_throw() + { + var svc = new FakeRawTreeService + { + OnLoadTagForEdit = tagId => TagOn(tagId, "dev-1"), + OnLoadMergedProbeConfig = _ => throw new InvalidOperationException("device is gone"), + }; + + var modal = NewModal(svc, "tag-1"); + + // The modal must still open and remain saveable — only the browse affordance degrades, and it + // reports its own error when used. + await Should.NotThrowAsync(() => InvokeParametersSetAsync(modal)); + + DriverConfigJson(modal).ShouldBe("{}"); + } + + [Fact] + public async Task The_accessor_is_live_not_a_captured_snapshot() + { + var svc = new FakeRawTreeService + { + OnLoadTagForEdit = tagId => TagOn(tagId, tagId == "tag-1" ? "dev-1" : "dev-2"), + OnLoadMergedProbeConfig = _ => null, + }; + + var modal = NewModal(svc, "tag-1"); + await InvokeParametersSetAsync(modal); + + var firstAccessor = (Func)modal.BuildEditorParameters()["GetDriverConfigJson"]; + firstAccessor().ShouldBe("{}"); + + // Close, then reopen on a different tag whose device now resolves. The per-open cache must have + // been reset and re-resolved, and the accessor must read the field at call time rather than + // having closed over the value it saw when it was built. + modal.Visible = false; + await InvokeParametersSetAsync(modal); + + svc.OnLoadMergedProbeConfig = deviceId => + deviceId == "dev-2" ? ("AbCip", MergedConfig) : null; + modal.Visible = true; + modal.TagId = "tag-2"; + await InvokeParametersSetAsync(modal); + + DriverConfigJson(modal).ShouldBe(MergedConfig); + firstAccessor().ShouldBe(MergedConfig); + } + + private static RawTagEditDto TagOn(string tagId, string deviceId) => new( + tagId, + deviceId, + TagGroupId: null, + Name: "Speed", + DataType: "Float", + AccessLevel: TagAccessLevel.Read, + WriteIdempotent: false, + PollGroupId: null, + TagConfig: "{}", + RowVersion: []); + + private static RawTagModal NewModal(IRawTreeService svc, string tagId) + { + var modal = new RawTagModal { Visible = true, TagId = tagId, DriverType = "AbCip" }; + + // @inject generates a private property, so the fake goes in by reflection. + var injected = typeof(RawTagModal).GetProperty( + "Svc", + BindingFlags.NonPublic | BindingFlags.Instance); + injected.ShouldNotBeNull("RawTagModal no longer injects IRawTreeService as 'Svc'."); + injected!.SetValue(modal, svc); + + return modal; + } + + private static Task InvokeParametersSetAsync(RawTagModal modal) + { + var hook = typeof(RawTagModal).GetMethod( + "OnParametersSetAsync", + BindingFlags.NonPublic | BindingFlags.Instance); + hook.ShouldNotBeNull("RawTagModal no longer overrides OnParametersSetAsync."); + + return (Task)hook!.Invoke(modal, null)!; + } + + private static string DriverConfigJson(RawTagModal modal) + => ((Func)modal.BuildEditorParameters()["GetDriverConfigJson"])(); +} +#pragma warning restore BL0005 From a2093b440007ee443c795d8328b2dd6a7fade5b8 Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Wed, 5 Aug 2026 03:25:13 -0400 Subject: [PATCH 3/8] fix(raw): manual tag entry feeds each row's picker the merged device config (#468 follow-up) --- .../Shared/Raw/RawManualTagEntryModal.razor | 28 +++- ...RawManualTagEntryModalMergedConfigTests.cs | 132 ++++++++++++++++++ 2 files changed, 157 insertions(+), 3 deletions(-) create mode 100644 tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/Raw/RawManualTagEntryModalMergedConfigTests.cs diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Shared/Raw/RawManualTagEntryModal.razor b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Shared/Raw/RawManualTagEntryModal.razor index 3f4b5c9d..aa96d398 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Shared/Raw/RawManualTagEntryModal.razor +++ b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Shared/Raw/RawManualTagEntryModal.razor @@ -1,7 +1,8 @@ @* Bulk manual tag-entry grid for the v3 /raw tree: add MANY new tags at once under a Device or a TagGroup. Each row carries Name / DataType / AccessLevel / WriteIdempotent / PollGroup plus a per-row driver-typed TagConfig editor (revealed in an expandable full-width sub-row — the grid stays - compact). The owning DriverType (fixed by the device) drives the typed-editor dispatch. On commit each + compact), each fed the merged Driver+Device probe config so its address picker can browse the live + device. The owning DriverType (fixed by the device) drives the typed-editor dispatch. On commit each row is created via CreateTagAsync; committed rows drop off, failed rows stay with their inline error so nothing is lost silently. Visibility is @bind-Visible; a successful commit raises OnSaved. *@ @using Microsoft.AspNetCore.Components.Forms @@ -159,11 +160,17 @@ private bool _summaryOk; private bool _open; // guards re-seeding rows on unrelated re-renders - protected override void OnParametersSet() + // Merged Driver+Device config for each row's address picker — the same blob Test-connect and + // RawBrowseModal dial, resolved once per open; null (→ "{}") when the resolve failed: the picker's + // browse degrades to its graceful error chip. + private string? _mergedConfigJson; + + protected override async Task OnParametersSetAsync() { if (!Visible) { _open = false; + _mergedConfigJson = null; return; } if (_open) { return; } // same open, re-render → preserve in-progress rows @@ -172,6 +179,21 @@ _rows.Add(new RowModel()); _summary = null; _summaryOk = false; + + _mergedConfigJson = null; + if (!string.IsNullOrEmpty(DeviceId)) + { + try + { + var merged = await Svc.LoadMergedProbeConfigAsync(DeviceId); + if (merged is { } m) { _mergedConfigJson = m.MergedConfigJson; } + } + catch + { + // Fail-soft: the grid must stay usable regardless — only the browse affordance inside each + // row's picker degrades, and it reports its own error on use. + } + } } private IDictionary BuildEditorParameters(RowModel row) => new Dictionary @@ -179,7 +201,7 @@ ["ConfigJson"] = row.TagConfig, ["ConfigJsonChanged"] = EventCallback.Factory.Create(this, v => row.TagConfig = v), ["DriverType"] = DriverType, - ["GetDriverConfigJson"] = (Func)(() => "{}"), + ["GetDriverConfigJson"] = (Func)(() => _mergedConfigJson ?? "{}"), }; private void AddRow() => _rows.Add(new RowModel()); diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/Raw/RawManualTagEntryModalMergedConfigTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/Raw/RawManualTagEntryModalMergedConfigTests.cs new file mode 100644 index 00000000..b09d9f6a --- /dev/null +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/Raw/RawManualTagEntryModalMergedConfigTests.cs @@ -0,0 +1,132 @@ +using System.Reflection; +using Shouldly; +using Xunit; +using ZB.MOM.WW.OtOpcUa.AdminUI.Components.Shared.Raw; + +namespace ZB.MOM.WW.OtOpcUa.AdminUI.Tests.Raw; + +/// +/// Pins that the bulk manual-entry grid hands each row's typed tag editor the merged +/// Driver+Device config, not a stub. +/// +/// +/// +/// The AbCip / TwinCAT / FOCAS editors' address pickers dial the GetDriverConfigJson Func as +/// their browse connect config. passed a literal "{}", +/// so "Browse controller" always connected against an empty config and could never reach the +/// device — the picker was authorable but unusable, and no offline test reached it (AdminUI has no +/// bUnit). +/// +/// +/// Unlike the single-tag modal, the device here is a direct [Parameter] — no tag load +/// precedes it — so the resolve keys off DeviceId on the open path. The accessor must stay +/// live (reading the field, not a captured snapshot), because the resolve is awaited after +/// the rows are seeded. Exercised as a plain class: AdminUI has no bUnit, so the injected service +/// and the lifecycle/parameter-building members are reached by reflection. +/// +/// +public sealed class RawManualTagEntryModalMergedConfigTests +{ + [Fact] + public async Task Each_rows_editor_receives_the_merged_device_config_not_a_stub() + { + const string merged = """{"endpoint":"10.0.0.1:44818","slot":0}"""; + + var svc = new FakeRawTreeService + { + OnLoadMergedProbeConfig = deviceId => + deviceId == "dev-1" ? ("AbCip", merged) : null, + }; + + var modal = new RawManualTagEntryModal(); + SetParameters(modal, visible: true, deviceId: "dev-1", driverType: "AbCip"); + InjectService(modal, svc); + + await OpenAsync(modal); + + GetDriverConfigJson(modal).ShouldBe(merged); + } + + [Fact] + public async Task A_failed_resolve_degrades_to_the_empty_stub_and_does_not_throw() + { + var svc = new FakeRawTreeService + { + OnLoadMergedProbeConfig = _ => throw new InvalidOperationException("device unreachable"), + }; + + var modal = new RawManualTagEntryModal(); + SetParameters(modal, visible: true, deviceId: "dev-1", driverType: "AbCip"); + InjectService(modal, svc); + + await Should.NotThrowAsync(() => OpenAsync(modal)); + + GetDriverConfigJson(modal).ShouldBe("{}"); + } + + /// + /// Sets the modal's [Parameter]s. By reflection because BL0005 forbids assigning component + /// parameters from outside the component, and this project treats warnings as errors. + /// + private static void SetParameters( + RawManualTagEntryModal modal, bool visible, string deviceId, string driverType) + { + Set("Visible", visible); + Set("DeviceId", deviceId); + Set("DriverType", driverType); + + void Set(string name, object value) + { + var property = typeof(RawManualTagEntryModal).GetProperty(name); + property.ShouldNotBeNull($"the [Parameter] {name} was renamed or removed."); + property!.SetValue(modal, value); + } + } + + /// Sets the @inject-generated private Svc property. + private static void InjectService(RawManualTagEntryModal modal, FakeRawTreeService svc) + { + var property = typeof(RawManualTagEntryModal).GetProperty( + "Svc", + BindingFlags.NonPublic | BindingFlags.Instance); + + property.ShouldNotBeNull("the injected IRawTreeService property is no longer named Svc."); + + property!.SetValue(modal, svc); + } + + /// Drives the open path — the protected lifecycle method the resolve hangs off. + private static async Task OpenAsync(RawManualTagEntryModal modal) + { + var method = typeof(RawManualTagEntryModal).GetMethod( + "OnParametersSetAsync", + BindingFlags.NonPublic | BindingFlags.Instance); + + method.ShouldNotBeNull("the open path moved off OnParametersSetAsync — retarget this driver."); + + await (Task)method!.Invoke(modal, null)!; + } + + /// Invokes the GetDriverConfigJson Func the first seeded row's editor is handed. + private static string GetDriverConfigJson(RawManualTagEntryModal modal) + { + var rowsField = typeof(RawManualTagEntryModal).GetField( + "_rows", + BindingFlags.NonPublic | BindingFlags.Instance); + + rowsField.ShouldNotBeNull("the manual-entry grid's row list is no longer named _rows."); + + var rows = ((System.Collections.IEnumerable)rowsField!.GetValue(modal)!).Cast().ToList(); + rows.ShouldNotBeEmpty("the open path no longer seeds a row, so no editor parameters exist to check."); + + var build = typeof(RawManualTagEntryModal).GetMethod( + "BuildEditorParameters", + BindingFlags.NonPublic | BindingFlags.Instance); + + build.ShouldNotBeNull("BuildEditorParameters was renamed — this test reads it by name."); + + var parameters = (IDictionary)build!.Invoke(modal, [rows[0]])!; + + return ((Func)parameters["GetDriverConfigJson"])(); + } +} From c1c21f3ff86b5b2aec51ac98f2b58c2f0189674e Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Wed, 5 Aug 2026 03:33:13 -0400 Subject: [PATCH 4/8] test(raw): pin the manual-entry accessor as live, not a captured snapshot --- ...RawManualTagEntryModalMergedConfigTests.cs | 37 ++++++++++++++++++- 1 file changed, 35 insertions(+), 2 deletions(-) diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/Raw/RawManualTagEntryModalMergedConfigTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/Raw/RawManualTagEntryModalMergedConfigTests.cs index b09d9f6a..b81c38d8 100644 --- a/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/Raw/RawManualTagEntryModalMergedConfigTests.cs +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/Raw/RawManualTagEntryModalMergedConfigTests.cs @@ -64,6 +64,36 @@ public sealed class RawManualTagEntryModalMergedConfigTests GetDriverConfigJson(modal).ShouldBe("{}"); } + [Fact] + public async Task The_accessor_is_live_not_a_captured_snapshot() + { + const string merged = """{"endpoint":"10.0.0.1:44818","slot":0}"""; + + (string DriverType, string MergedConfigJson)? answer = null; + var svc = new FakeRawTreeService { OnLoadMergedProbeConfig = _ => answer }; + + var modal = new RawManualTagEntryModal(); + SetParameters(modal, visible: true, deviceId: "dev-1", driverType: "AbCip"); + InjectService(modal, svc); + + await OpenAsync(modal); + + // The delegate an already-rendered editor is holding. It must read the field on every call, not a + // value captured when the parameters were built — the resolve is awaited AFTER the rows are + // seeded, so a snapshot taken at build time is permanently "{}". + var accessor = GetAccessor(modal); + accessor().ShouldBe("{}"); + + SetParameters(modal, visible: false, deviceId: "dev-1", driverType: "AbCip"); + await OpenAsync(modal); + + answer = ("AbCip", merged); + SetParameters(modal, visible: true, deviceId: "dev-1", driverType: "AbCip"); + await OpenAsync(modal); + + accessor().ShouldBe(merged); + } + /// /// Sets the modal's [Parameter]s. By reflection because BL0005 forbids assigning component /// parameters from outside the component, and this project treats warnings as errors. @@ -108,7 +138,10 @@ public sealed class RawManualTagEntryModalMergedConfigTests } /// Invokes the GetDriverConfigJson Func the first seeded row's editor is handed. - private static string GetDriverConfigJson(RawManualTagEntryModal modal) + private static string GetDriverConfigJson(RawManualTagEntryModal modal) => GetAccessor(modal)(); + + /// The GetDriverConfigJson Func itself, so a test can retain and re-invoke it. + private static Func GetAccessor(RawManualTagEntryModal modal) { var rowsField = typeof(RawManualTagEntryModal).GetField( "_rows", @@ -127,6 +160,6 @@ public sealed class RawManualTagEntryModalMergedConfigTests var parameters = (IDictionary)build!.Invoke(modal, [rows[0]])!; - return ((Func)parameters["GetDriverConfigJson"])(); + return (Func)parameters["GetDriverConfigJson"]; } } From 4e3a6e8497afafb77dbe2bc63207f4dfba33fe7a Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Wed, 5 Aug 2026 03:36:04 -0400 Subject: [PATCH 5/8] fix(raw): a stale open can no longer feed the picker another device's config --- .../Components/Shared/Raw/RawTagModal.razor | 7 ++++ .../Raw/FakeRawTreeService.cs | 16 +++++++-- .../Raw/RawTagModalMergedConfigTests.cs | 34 +++++++++++++++++++ 3 files changed, 55 insertions(+), 2 deletions(-) diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Shared/Raw/RawTagModal.razor b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Shared/Raw/RawTagModal.razor index 8e6ad6e1..1337dc8e 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Shared/Raw/RawTagModal.razor +++ b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Shared/Raw/RawTagModal.razor @@ -170,6 +170,11 @@ } var dto = await Svc.LoadTagForEditAsync(TagId); + // Two opens can overlap on one modal instance (the tree's Edit command is synchronous and does + // not disable while this resolves), so every post-await write is discarded unless this open is + // still the displayed one. Last-completion-wins would point the picker at another device. + if (_loadedKey != key || !Visible) { return; } + // RawTagEditDto is a record struct → a missing tag comes back as default (all-zero), not null. if (dto is not { } d || string.IsNullOrEmpty(d.TagId)) { @@ -192,6 +197,8 @@ try { var merged = await Svc.LoadMergedProbeConfigAsync(d.DeviceId); + if (_loadedKey != key || !Visible) { return; } // superseded open → never install its device + if (merged is { } m) { _mergedConfigJson = m.MergedConfigJson; diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/Raw/FakeRawTreeService.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/Raw/FakeRawTreeService.cs index 9c11df75..484c39ea 100644 --- a/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/Raw/FakeRawTreeService.cs +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/Raw/FakeRawTreeService.cs @@ -20,14 +20,26 @@ internal sealed class FakeRawTreeService : IRawTreeService public Func OnLoadMergedProbeConfig { get; set; } = _ => throw new NotImplementedException(); + /// + /// Optional asynchronous override of . The sync delegates above + /// complete inline, which cannot express two opens overlapping on one modal instance — set this to + /// gate a load on a and interleave the second open. + /// + public Func>? OnLoadTagForEditAsync { get; set; } + + /// Optional asynchronous override of ; see . + public Func>? OnLoadMergedProbeConfigAsync { get; set; } + /// public Task LoadTagForEditAsync(string tagId, CancellationToken ct = default) - => Task.FromResult(OnLoadTagForEdit(tagId)); + => OnLoadTagForEditAsync is { } async ? async(tagId) : Task.FromResult(OnLoadTagForEdit(tagId)); /// public Task<(string DriverType, string MergedConfigJson)?> LoadMergedProbeConfigAsync( string deviceId, CancellationToken ct = default) - => Task.FromResult(OnLoadMergedProbeConfig(deviceId)); + => OnLoadMergedProbeConfigAsync is { } async + ? async(deviceId) + : Task.FromResult(OnLoadMergedProbeConfig(deviceId)); /// public Task> LoadRootsAsync(CancellationToken ct = default) diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/Raw/RawTagModalMergedConfigTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/Raw/RawTagModalMergedConfigTests.cs index 3ca5dd50..2fd06818 100644 --- a/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/Raw/RawTagModalMergedConfigTests.cs +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/Raw/RawTagModalMergedConfigTests.cs @@ -33,6 +33,9 @@ public sealed class RawTagModalMergedConfigTests { private const string MergedConfig = """{"endpoint":"10.0.0.1:44818","slot":0}"""; + /// A second device's config — the blob a superseded open must never install. + private const string StaleConfig = """{"endpoint":"10.0.0.9:44818","slot":3}"""; + [Fact] public async Task The_editor_receives_the_merged_device_config_not_a_stub() { @@ -98,6 +101,37 @@ public sealed class RawTagModalMergedConfigTests firstAccessor().ShouldBe(MergedConfig); } + [Fact] + public async Task A_stale_open_never_feeds_the_picker_another_devices_config() + { + // RawTree's OnEditTag is synchronous and does not disable while the modal resolves, so a rapid + // double-click across two tag rows overlaps two opens on ONE modal instance. Every write after an + // await must therefore be discarded if the open it belongs to is no longer the displayed one: + // last-completion-wins would silently point a live browse at the wrong controller. + var gate = new TaskCompletionSource<(string DriverType, string MergedConfigJson)?>(); + var svc = new FakeRawTreeService + { + OnLoadTagForEdit = tagId => TagOn(tagId, tagId == "tag-1" ? "dev-A" : "dev-B"), + OnLoadMergedProbeConfigAsync = deviceId => deviceId == "dev-A" + ? gate.Task + : Task.FromResult<(string DriverType, string MergedConfigJson)?>(("AbCip", MergedConfig)), + }; + + var modal = NewModal(svc, "tag-1"); + var firstOpen = InvokeParametersSetAsync(modal); + + // The second open supersedes the first while the first is still parked on dev-A's resolve. + modal.TagId = "tag-2"; + await InvokeParametersSetAsync(modal); + + gate.SetResult(("AbCip", StaleConfig)); + await firstOpen; + + DriverConfigJson(modal).ShouldBe( + MergedConfig, + "the superseded open's dev-A config overwrote the displayed tag's dev-B config"); + } + private static RawTagEditDto TagOn(string tagId, string deviceId) => new( tagId, deviceId, From 2c19a50bf951073468f793eb884c40fa4a9c711a Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Wed, 5 Aug 2026 03:38:25 -0400 Subject: [PATCH 6/8] fix(raw): a superseded manual-entry open can no longer feed the pickers another device's config --- .../Shared/Raw/RawManualTagEntryModal.razor | 8 +++++ ...RawManualTagEntryModalMergedConfigTests.cs | 36 +++++++++++++++++++ 2 files changed, 44 insertions(+) diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Shared/Raw/RawManualTagEntryModal.razor b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Shared/Raw/RawManualTagEntryModal.razor index aa96d398..1042c96e 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Shared/Raw/RawManualTagEntryModal.razor +++ b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Shared/Raw/RawManualTagEntryModal.razor @@ -160,6 +160,9 @@ private bool _summaryOk; private bool _open; // guards re-seeding rows on unrelated re-renders + // Identifies WHICH open a post-await write belongs to; _open alone cannot tell two opens apart. + private int _openGeneration; + // Merged Driver+Device config for each row's address picker — the same blob Test-connect and // RawBrowseModal dial, resolved once per open; null (→ "{}") when the resolve failed: the picker's // browse degrades to its graceful error chip. @@ -175,6 +178,7 @@ } if (_open) { return; } // same open, re-render → preserve in-progress rows _open = true; + var generation = ++_openGeneration; _rows.Clear(); _rows.Add(new RowModel()); _summary = null; @@ -186,6 +190,10 @@ try { var merged = await Svc.LoadMergedProbeConfigAsync(DeviceId); + // A close-reopen onto another device can complete while this resolve is in flight, so a + // superseded open must never install its device's config under the displayed grid. + if (generation != _openGeneration || !Visible) { return; } + if (merged is { } m) { _mergedConfigJson = m.MergedConfigJson; } } catch diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/Raw/RawManualTagEntryModalMergedConfigTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/Raw/RawManualTagEntryModalMergedConfigTests.cs index b81c38d8..042e313e 100644 --- a/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/Raw/RawManualTagEntryModalMergedConfigTests.cs +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/Raw/RawManualTagEntryModalMergedConfigTests.cs @@ -94,6 +94,42 @@ public sealed class RawManualTagEntryModalMergedConfigTests accessor().ShouldBe(merged); } + [Fact] + public async Task A_stale_open_never_feeds_the_picker_another_devices_config() + { + const string deviceA = """{"endpoint":"10.0.0.1:44818","slot":0}"""; + const string deviceB = """{"endpoint":"10.0.0.2:44818","slot":1}"""; + + var deviceAResolve = new TaskCompletionSource(); + var svc = new FakeRawTreeService + { + OnLoadMergedProbeConfigAsync = async deviceId => + { + if (deviceId == "dev-A") { await deviceAResolve.Task; return ("AbCip", deviceA); } + return ("AbCip", deviceB); + }, + }; + + var modal = new RawManualTagEntryModal(); + InjectService(modal, svc); + + // Quick close-reopen onto a different device while the first resolve is still in flight — the + // grid shows dev-B, so dev-A's late continuation must not install its config underneath it. + SetParameters(modal, visible: true, deviceId: "dev-A", driverType: "AbCip"); + var staleOpen = OpenAsync(modal); + + SetParameters(modal, visible: false, deviceId: "dev-A", driverType: "AbCip"); + await OpenAsync(modal); + + SetParameters(modal, visible: true, deviceId: "dev-B", driverType: "AbCip"); + await OpenAsync(modal); + + deviceAResolve.SetResult(); + await staleOpen; + + GetDriverConfigJson(modal).ShouldBe(deviceB); + } + /// /// Sets the modal's [Parameter]s. By reflection because BL0005 forbids assigning component /// parameters from outside the component, and this project treats warnings as errors. From c1e8762cd4868384cc18d8b6497dc4b6cf3037ab Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Wed, 5 Aug 2026 03:45:30 -0400 Subject: [PATCH 7/8] =?UTF-8?q?docs(raw):=20close=20the=20#468=20follow-up?= =?UTF-8?q?=20=E2=80=94=20raw=20modals=20now=20feed=20pickers=20live=20mer?= =?UTF-8?q?ged=20config?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Records the outcome in the design doc §6 (including the stale-open race the review found and fixed in both modals), closes the follow-up in the tracking doc and the universal-browser plan, and registers RawBrowseModal's shared race shape in deferment.md §0.1 as the deliberate leftover. --- deferment.md | 1 + ...versal-discovery-browser-implementation.md | 13 +++-- .../2026-07-24-driver-expansion-tracking.md | 14 +++-- ...-05-raw-modal-picker-live-config-design.md | 38 ++++++++++++- ...raw-modal-picker-live-config.md.tasks.json | 56 ++++++++++++++++--- 5 files changed, 102 insertions(+), 20 deletions(-) diff --git a/deferment.md b/deferment.md index c6425d2c..d0499579 100644 --- a/deferment.md +++ b/deferment.md @@ -60,6 +60,7 @@ So everything below is **untracked work**. Nothing in it is a data-loss or outag | **1 genuinely unexecuted plan** — `2026-05-29-adminui-followups`, **19 pending / 0 completed** | The only substantial engineering left. ⚠️ Its first step is an **overlap audit**, not code: several items look superseded by v3's `TagConfigEditorMap`. | §6.2 | | **11 other open plan tasks** — 5 akka-hosting partials (likely v3-superseded), 4+1 followups-batch (open *by choice*), fixedtree Task 11, 1 manual smoke checklist | Small, and two of the groups are decisions already taken. | §6.2 | | **Review-coverage gap** — 10 shipped projects never reviewed (Calculation, Historian.Gateway, Mqtt +Browser/Contracts, MTConnect +Contracts, Sql +Browser/Contracts) while **3 modules still review the retired Wonderware backend** | Re-verified 2026-08-04 against `code-reviews/`. Unchanged since the audit. | §6.3 | +| **`RawBrowseModal` stale-open race** (added 2026-08-05) — `OnParametersSetAsync` never re-checks staleness after its awaits, so a superseded open's late resolve can install the wrong device's browse config. The identical shape was found by review and **fixed** in `RawTagModal`/`RawManualTagEntryModal` (post-await staleness re-check; see `docs/plans/2026-08-05-raw-modal-picker-live-config-design.md` §6); the browse modal was deliberately left out of that branch's scope. | Lower stakes than the fixed pair (the modal resolves the device it was opened for and shows its own error banner), but the fix pattern is proven and cheap to port. | design doc §6 | ⚠️ **One correction worth carrying forward.** archreview **03/S4** — "primary gate default-allows on unknown role" — is the *same shape* as a decision deliberately taken on 2026-08-04 in diff --git a/docs/plans/2026-07-15-universal-discovery-browser-implementation.md b/docs/plans/2026-07-15-universal-discovery-browser-implementation.md index 470e93bb..4d970582 100644 --- a/docs/plans/2026-07-15-universal-discovery-browser-implementation.md +++ b/docs/plans/2026-07-15-universal-discovery-browser-implementation.md @@ -1154,11 +1154,14 @@ neither agreed with a controller. Fixed, with the layout now pinned by a capture ### Follow-ups discovered during the first gate 1. ~~**Full tree-render verification is fixture-blocked.**~~ **CLOSED** by the second gate above. -2. **Driver-page address pickers pass no live config.** `AbCipDriverPage`/`TwinCATDriverPage`/`FOCASDriverPage` - host the same `*AddressPickerBody` but do **not** pass `DriverType`/`GetConfigJson`, so their (now-visible) - Browse button would capture against `"{}"` (no device). Out of Task 14's scope (which listed only the - TagEditor pass-through); wire the live form config through there too, or hide the Browse affordance on the - driver-config picker. +2. ~~**Driver-page address pickers pass no live config.**~~ — **CLOSED 2026-08-05.** The named + `*DriverPage` shells were deleted by v3 Batch 2 before this was ever actioned, but the defect + pattern survived into the v3 tag-editor hosts (`RawTagModal` / `RawManualTagEntryModal`, both + passing a `"{}"` stub as `GetDriverConfigJson`). Fixed by resolving the merged Driver+Device + config (`IRawTreeService.LoadMergedProbeConfigAsync` — the same blob Test-connect and + `RawBrowseModal` dial) once per open and forwarding a live accessor; fail-soft on resolve + failure. See `2026-08-05-raw-modal-picker-live-config-design.md` for the design and outcome + (including the stale-open race the review found and closed). ## Out of scope (explicitly) diff --git a/docs/plans/2026-07-24-driver-expansion-tracking.md b/docs/plans/2026-07-24-driver-expansion-tracking.md index 9265409e..ca662e05 100644 --- a/docs/plans/2026-07-24-driver-expansion-tracking.md +++ b/docs/plans/2026-07-24-driver-expansion-tracking.md @@ -104,10 +104,16 @@ marginal cost. 3. AbCip Test Connect misclassified the no-authored-tags probe **on real hardware too** — the reachable-exception matcher looked for native C status spellings libplctag.NET never emits (plus a sim-side 0x03/0x04 CIP status mismatch). -- **Residual follow-up (open):** the retired-flow driver pages (`AbCipDriverPage`/`TwinCATDriverPage`/ - `FOCASDriverPage`) host the picker body without passing `DriverType`/`GetConfigJson`, so their - Browse button captures against `"{}"` — wire the live form config through or hide the affordance - there. Recorded in the implementation plan's first-gate follow-ups. +- **Residual follow-up — CLOSED 2026-08-05.** The named `*DriverPage` shells were already deleted + when v3 Batch 2 retired the routed `/clusters/{id}/drivers` flow, but the defect pattern had + survived into the v3 hosts: `RawTagModal` and `RawManualTagEntryModal` passed a `"{}"` stub as + `GetDriverConfigJson`, so the AbCip/TwinCAT/FOCAS pickers' "Browse controller" captured against + no endpoint. Fixed by resolving `LoadMergedProbeConfigAsync` per open and forwarding a live + accessor (design + outcome: `2026-08-05-raw-modal-picker-live-config-design.md`). Review of the + fix surfaced and closed a further stale-open race (a superseded open could install another + device's config); `RawBrowseModal` shares that pre-existing race shape and is recorded in + `deferment.md` §0.1, not fixed here. Pinned by `RawTagModalMergedConfigTests` + + `RawManualTagEntryModalMergedConfigTests`. --- diff --git a/docs/plans/2026-08-05-raw-modal-picker-live-config-design.md b/docs/plans/2026-08-05-raw-modal-picker-live-config-design.md index 572a50e3..d3825919 100644 --- a/docs/plans/2026-08-05-raw-modal-picker-live-config-design.md +++ b/docs/plans/2026-08-05-raw-modal-picker-live-config-design.md @@ -1,7 +1,8 @@ # Raw-modal address pickers: pass live merged config to browse — design -**Status:** design, 2026-08-05. Closes the last dangling follow-up from the #468 universal-browser -live gate ("driver-page address pickers pass no live config"). +**Status:** IMPLEMENTED 2026-08-05 (unit suite green, AdminUI 1064/1064; live `/run` gate §4.2 +pending). Closes the last dangling follow-up from the #468 universal-browser live gate +("driver-page address pickers pass no live config"). Outcome notes in §6. ## 1. Problem @@ -50,7 +51,9 @@ Per modal, identically: resolve must be prefetched, not lazy. 3. **Fail soft.** A null resolve (device gone, service error) leaves the stub in place — the picker degrades to today's behaviour (graceful error chip on click), and the modal itself never blocks - or errors on the resolve. Wrap the call in try/catch; log at debug. + or errors on the resolve. Wrap the call in a swallowing try/catch carrying a constraint comment + (as built — neither modal injects a logger, matching the sibling `RawBrowseModal` pattern; the + picker reports its own error on use). No changes to the picker bodies, the editors, `IRawTreeService`, or the browse services — the seam already exists end-to-end; only the two stubs change. @@ -88,3 +91,32 @@ bullet in `docs/plans/2026-07-24-driver-expansion-tracking.md` (Wave 0 section) "Follow-ups discovered during the first gate" item 2 in `docs/plans/2026-07-15-universal-discovery-browser-implementation.md` — both should note the pages were already deleted and the surviving instance (the raw modals' stubs) is fixed. + +## 6. Outcome (2026-08-05) + +Shipped across five commits (`e698aee4` fake, `abb9f8a1` + `4e3a6e84` RawTagModal, `a2093b44` + +`c1c21f3f` + `2c19a50b` RawManualTagEntryModal), each TDD'd with observed-red evidence, and each +through a spec review + code review. Things the reviews changed or decided: + +- **The review found a real race this design missed, and it was fixed in both modals.** + `OnParametersSetAsync` runs overlapped across rapid open/reopen (Blazor does not serialize it + against its own outstanding task), and nothing re-checked staleness after the awaits — so a + superseded open's late resolve could install **another device's** merged config while a different + tag/grid was displayed, and Browse would silently dial the wrong controller. Fixed with a + post-await staleness re-check before every write: `RawTagModal` re-checks its existing + `_loadedKey` (after BOTH awaits — the dto write had the same shape, previously masked only by the + EF concurrency token downstream), `RawManualTagEntryModal` a new per-open `_openGeneration` + counter (its `_open` bool cannot tell two opens apart). Both pinned by + `A_stale_open_never_feeds_the_picker_another_devices_config` facts, each observed red pre-guard; + `FakeRawTreeService` gained optional Task-returning delegates so tests can express overlap. +- ⚠️ **`RawBrowseModal` shares the same pre-existing race shape and was deliberately NOT touched** + (out of this branch's scope). Recorded in `deferment.md` §0.1. Lower stakes there — the browse + modal resolves the device it was opened for and shows its own error banner — but the shape is + identical. +- **Component-class tests must not set `[Parameter]` properties directly**: analyzer BL0005 is an + error under `TreatWarningsAsErrors`. The two test files legitimately solve it two ways (file-local + pragma vs a reflection `SetParameters` helper); the reflection helper is the preferred convention + going forward. +- The liveness of the forwarded `Func` is load-bearing (the pickers call it at click time, after + the async resolve) and is pinned in both files by retained-delegate facts, each falsified against + a deliberate capture-by-value implementation before being trusted. diff --git a/docs/plans/2026-08-05-raw-modal-picker-live-config.md.tasks.json b/docs/plans/2026-08-05-raw-modal-picker-live-config.md.tasks.json index 393b8299..5ca0c219 100644 --- a/docs/plans/2026-08-05-raw-modal-picker-live-config.md.tasks.json +++ b/docs/plans/2026-08-05-raw-modal-picker-live-config.md.tasks.json @@ -1,12 +1,52 @@ { "planPath": "docs/plans/2026-08-05-raw-modal-picker-live-config.md", "tasks": [ - {"id": 0, "subject": "Task 0: FakeRawTreeService test helper", "status": "pending"}, - {"id": 1, "subject": "Task 1: RawTagModal — resolve and forward merged config", "status": "pending", "blockedBy": [0]}, - {"id": 2, "subject": "Task 2: RawManualTagEntryModal — resolve and forward merged config", "status": "pending", "blockedBy": [0]}, - {"id": 3, "subject": "Task 3: Full-suite verification", "status": "pending", "blockedBy": [1, 2]}, - {"id": 4, "subject": "Task 4: Documentation bookkeeping", "status": "pending", "blockedBy": [3]}, - {"id": 5, "subject": "Task 5: Live /run gate (main session)", "status": "pending", "blockedBy": [4]} + { + "id": 0, + "subject": "Task 0: FakeRawTreeService test helper", + "status": "completed" + }, + { + "id": 1, + "subject": "Task 1: RawTagModal \u2014 resolve and forward merged config", + "status": "completed", + "blockedBy": [ + 0 + ] + }, + { + "id": 2, + "subject": "Task 2: RawManualTagEntryModal \u2014 resolve and forward merged config", + "status": "completed", + "blockedBy": [ + 0 + ] + }, + { + "id": 3, + "subject": "Task 3: Full-suite verification", + "status": "completed", + "blockedBy": [ + 1, + 2 + ] + }, + { + "id": 4, + "subject": "Task 4: Documentation bookkeeping", + "status": "completed", + "blockedBy": [ + 3 + ] + }, + { + "id": 5, + "subject": "Task 5: Live /run gate (main session)", + "status": "pending", + "blockedBy": [ + 4 + ] + } ], - "lastUpdated": "2026-08-05T00:00:00Z" -} + "lastUpdated": "2026-08-05T18:00:00Z" +} \ No newline at end of file From 4b88dc6f9abae3ab3d507000a5b45d5540fe105b Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Wed, 5 Aug 2026 03:51:30 -0400 Subject: [PATCH 8/8] =?UTF-8?q?docs(raw):=20final-review=20nits=20?= =?UTF-8?q?=E2=80=94=20commit=20count,=20closed=20cross-reference,=20force?= =?UTF-8?q?d=20accessibility=20asymmetry?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- docs/plans/2026-07-24-driver-expansion-tracking.md | 4 ++-- .../plans/2026-08-05-raw-modal-picker-live-config-design.md | 6 ++++-- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/docs/plans/2026-07-24-driver-expansion-tracking.md b/docs/plans/2026-07-24-driver-expansion-tracking.md index ca662e05..0d06c642 100644 --- a/docs/plans/2026-07-24-driver-expansion-tracking.md +++ b/docs/plans/2026-07-24-driver-expansion-tracking.md @@ -274,8 +274,8 @@ Both CI-simulatable on the shared docker host, no hardware. ## Next actions 1. **Wave 0 is done.** The live gate (Gitea #468) CLOSED 2026-08-01 (`c2c3c522`) — browse - verification is no longer a blocker for BACnet. Residual follow-up: the driver-page picker - config pass-through (see the Wave 0 section above). + verification is no longer a blocker for BACnet. The residual follow-up (the picker config + pass-through) CLOSED 2026-08-05 — see the Wave 0 section above. 2. **Wave 1 is done.** Both deliverables shipped and live-gated: **Modbus RTU-over-TCP** (merged `0f38f486` / #495) and **SQL poll** (merged `4ad54037`; follow-ups #496/#497/#498 closed in `28c28667`). Remaining SQL work is optional follow-on — Postgres/ODBC dialects behind diff --git a/docs/plans/2026-08-05-raw-modal-picker-live-config-design.md b/docs/plans/2026-08-05-raw-modal-picker-live-config-design.md index d3825919..fbe224b9 100644 --- a/docs/plans/2026-08-05-raw-modal-picker-live-config-design.md +++ b/docs/plans/2026-08-05-raw-modal-picker-live-config-design.md @@ -94,7 +94,7 @@ were already deleted and the surviving instance (the raw modals' stubs) is fixed ## 6. Outcome (2026-08-05) -Shipped across five commits (`e698aee4` fake, `abb9f8a1` + `4e3a6e84` RawTagModal, `a2093b44` + +Shipped across six commits (`e698aee4` fake, `abb9f8a1` + `4e3a6e84` RawTagModal, `a2093b44` + `c1c21f3f` + `2c19a50b` RawManualTagEntryModal), each TDD'd with observed-red evidence, and each through a spec review + code review. Things the reviews changed or decided: @@ -116,7 +116,9 @@ through a spec review + code review. Things the reviews changed or decided: - **Component-class tests must not set `[Parameter]` properties directly**: analyzer BL0005 is an error under `TreatWarningsAsErrors`. The two test files legitimately solve it two ways (file-local pragma vs a reflection `SetParameters` helper); the reflection helper is the preferred convention - going forward. + going forward. A related asymmetry: `RawTagModal.BuildEditorParameters` went `internal` for its + test while the manual modal's stayed `private` (its `RowModel` parameter is a private nested type, + so `internal` would not compile) — the accessibility difference is forced, not stylistic. - The liveness of the forwarded `Func` is load-bearing (the pickers call it at click time, after the async resolve) and is pinned in both files by retained-delegate facts, each falsified against a deliberate capture-by-value implementation before being trusted.