docs(raw): close the #468 follow-up — raw modals now feed pickers live merged config
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.
This commit is contained in:
@@ -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
|
||||
|
||||
@@ -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)
|
||||
|
||||
|
||||
@@ -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`.
|
||||
|
||||
---
|
||||
|
||||
|
||||
@@ -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.
|
||||
|
||||
@@ -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"
|
||||
}
|
||||
Reference in New Issue
Block a user