lmxopcua/docs/plans/2026-08-05-raw-modal-picker-live-config-design.md

Raw-modal address pickers: pass live merged config to browse — design

Status: DONE 2026-08-05 — unit suite green (AdminUI 1064/1064) and the live /run gate §4.2 PASSED (all three legs; see §7). 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; gate evidence + the defect it uncovered (Gitea #539) in §7.

1. Problem

The #468 first-gate follow-up named AbCipDriverPage/TwinCATDriverPage/FOCASDriverPage as hosting *AddressPickerBody without passing DriverType/GetConfigJson. Those page shells were deleted when v3 Batch 2 retired the routed /clusters/{id}/drivers flow — but the defect pattern survived the migration into the two v3 hosts of the typed tag editors:

• Components/Shared/Raw/RawTagModal.razor (edit an existing raw tag) — BuildEditorParameters() passes ["GetDriverConfigJson"] = (Func<string>)(() => "{}").
• Components/Shared/Raw/RawManualTagEntryModal.razor (manual multi-row tag entry) — same stub, per row.

The editors (AbCipTagConfigEditor, TwinCATTagConfigEditor, FocasTagConfigEditor) forward that Func to their *AddressPickerBody, whose browse gate is BrowserService.CanBrowse(DriverType, GetConfigJson()). DiscoveryDriverBrowser.CanBrowse creates a throwaway driver from the JSON — and "{}" parses fine into default options for AbCip / TwinCAT / FOCAS, all ITagDiscovery { SupportsOnlineDiscovery: true }. So the "Browse controller" button renders enabled, and every click captures against an empty config (no endpoint) — a guaranteed failure chip. The affordance is a lie in exactly the hosts where an operator would re-point a tag.

2. Decision — wire it through (not hide)

The #468 note offered "wire the live form config through, or hide the Browse affordance". Wire it through:

• Both hosts already hold the device context. RawManualTagEntryModal takes DeviceId as a parameter; RawTagModal receives RawTagEditDto.DeviceId from LoadTagForEditAsync.
• The resolver already exists and is proven: IRawTreeService.LoadMergedProbeConfigAsync(deviceId) → (DriverType, MergedConfigJson)? — the same Driver+Device merge the Test-connect probe and RawBrowseModal dial (endpoint lives in DeviceConfig in v3, so the merged blob is the only JSON a browse capture can succeed against).
• Hiding would delete a legitimate workflow (browse while editing/creating a tag) whose whole supporting machinery shipped with Wave 0.

3. Design

Per modal, identically:

1. Resolve on open. When the modal becomes visible with a device in hand (RawTagModal: after LoadTagForEditAsync returns the dto; RawManualTagEntryModal: on open, from the DeviceId parameter), call LoadMergedProbeConfigAsync(deviceId) and cache the result in a nullable field _mergedConfigJson. Reset the cache when the modal closes/reopens for a different target.
2. Forward it. ["GetDriverConfigJson"] = (Func<string>)(() => _mergedConfigJson ?? "{}"). The Func is synchronous by contract (the picker body calls it inline), which is why the 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 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.

Out of scope

• The MQTT/Sparkplug and OpcUaClient pickers (bespoke browsers with their own config paths — not hosted through these two stubs' editors, or already receiving config).
• Making the resolve lazy/async in the picker body (Func<string> contract change) — YAGNI.
• The two documented MQTT browse-UI gaps (tracked in docs/drivers/Mqtt.md).

4. Testing

The three MQTT live gates proved this defect class (hand-maintained AdminUI wiring left behind by a driver-side feature) is invisible to green unit suites — so test at two layers:

1. Unit (falsifiable against today's code). The AdminUI csproj has InternalsVisibleTo("ZB.MOM.WW.OtOpcUa.AdminUI.Tests") and precedent for testing razor component classes directly (RawDriverTypeDialogParityTests). Make each modal's editor-parameter construction internal-testable and assert the GetDriverConfigJson entry returns the cached merged JSON when one is loaded — this test fails against the current stub. Also assert the null-resolve fallback returns "{}".
2. Live /run gate (docker-dev). Against the patched AbCip fixture (10.100.0.35:44818, the #468 second-gate ab_server that serves @tags):
   • Open the edit modal for an existing AbCip raw tag → "Browse controller" → the nine-tag tree renders (was: error chip).
   • Open manual entry for the same device → same result on a row's picker.
   • Negative: a device whose endpoint is unreachable still shows the graceful error chip, and the modal itself stays usable.

5. Bookkeeping

On completion, close the loop in the two places that record the follow-up: the "Residual follow-up" bullet in docs/plans/2026-07-24-driver-expansion-tracking.md (Wave 0 section) and the "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 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:

• 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.
  • Ported 2026-08-09. The _openGeneration counter now guards every post-await write in ResetAndResolveAsync: the merged-config write, the group-context write (_groupPrefix/_effectiveDeviceId — a second await the siblings do not have), the catch's error banner, and the finally's _resolving + StateHasChanged. The finally is the one departure from the sibling shape and it is load-bearing: this modal resolves inside a try/finally, so a stale return still runs the tail and would drop the live open's spinner mid-resolve. Pinned by four facts in RawBrowseModalStaleOpenTests (A_stale_open_never_feeds_the_browser_another_devices_config, A_stale_group_resolve_never_retargets_the_displayed_open, A_stale_open_completing_late_never_clears_the_live_opens_loading_state, A_stale_resolve_failure_never_paints_its_error_over_the_live_open), each observed red before the guard and each re-falsified afterwards by reverting only its own guard. Driving this modal as a class needs one thing the siblings did not: it calls StateHasChanged on the open path, which throws without a render handle, so the test attaches an inert Renderer subclass (file-local #pragma warning disable BL0006).
• 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. 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.

7. Live /run gate — PASSED (2026-08-05)

Run by the main session against the local docker-dev rig (http://localhost:9200, login disabled) with both central containers rebuilt from master d88e42b8 and verified on the same image (sha256:6b1ddb47…) — the previously-running image predated the merge by three minutes, and :9200 round-robins central-1/central-2, so per-container image parity is the precondition for reading anything into the result. Backend: the #468 patched ab_server (otopcua-ab-server-controllogix, 10.100.0.35:44818), authored as docker-dev / abcip-browse / Device1.

Leg	Result
Edit modal — Device1/Motor_Run → Tag config → Build → Browse controller	✅ BROWSER OPEN, tree AbCip → Device1 → Discovered with all nine fixture tags (Motor_Run, Line1_Count, Pump_1_Speed, TestArray, TestString, TestSINT, TestBOOL, TestREAL, TestDINT). Selecting TestREAL landed it in the Tag-path field.
Manual entry — device → Add tags (manual) → row Configure → Build → Browse controller	✅ Same nine-tag tree, same Discovered shape.
Negative — a device whose endpoint is unreachable	✅ Button still renders; the click yields the graceful ERRORTIMEOUT chip, the grid stays usable, and the row still saved (a tag was created after the failed browse).

The tree's second level is labelled with the device's own name (Device1, and DeadDevice for the throwaway negative-leg device) — direct evidence that the per-device merged config, not a stub, is what the picker captures against. Under the old "{}" stub no endpoint reached the capture at all. The probe device + its tag were deleted afterwards; the rig is back to its pre-gate state.

The gate found a real defect — Gitea #539 (AbCip discovery drops the authored port)

The negative leg was first run against ab://10.100.0.35:44999/1,0 (nothing listening on 44999) and browsed successfully, returning the fixture's nine tags — while Test Connect on the same merged config correctly failed CONNECT FAILED: CONNECTIONREFUSED. That divergence is not this change: the AbCip discovery path passes libplctag the bare host, so libplctag substitutes its own 44818 default — Gitea #534's defect surviving in LibplctagTagEnumerator.cs:33 and LibplctagTemplateReader.cs:24 (the data path was fixed and uses LibplctagGateway). Consequence: browsing a device on a non-default port shows whatever answers on 44818, which on a plant network is another PLC.

Re-pointing the device at ab://127.0.0.1/1,0 produced the expected failure, which is the run recorded in the table above. Two things worth carrying forward:

• ⚠️ A "bad endpoint" chosen by changing only the port is not a valid negative control for anything libplctag-backed — the port may never reach the wire. Change the host.
• The fixture sitting on the default 44818 is exactly why #534's discovery half survived its own fix: on 44818 the bug is invisible by construction, so no existing test could see it.