refactor(drivers): delete the dead discovered-node injection path (§8.2, #507)

#507 was filed as "injection inert in v3, re-migrate onto the raw subtree".
Reading it, the retained code is not revivable: it resolves equipment from
EquipmentNode.DriverInstanceId UNION EquipmentTags, and BOTH are structurally
empty in v3 (AddressSpaceComposer always constructs EquipmentNode with a null
DriverInstanceId; DeploymentArtifact hard-codes an empty EquipmentTags set).
Removing the guard would have changed a log line and injected nothing. So it
is deleted rather than fixed, and #507 closes as superseded by /raw
browse-commit.

Removed: HandleDiscoveredNodes, PartitionDiscoveredByDeviceHost,
ShouldWarnPartition, PlansRoutingEqual, ApplyDiscoveredPlansForDriver,
_discoveredByDriver, the redeploy re-inject tail, DiscoveredNodeMapper,
DiscoveredInjection, AddressSpaceApplier.MaterialiseDiscoveredNodes,
OpcUaPublishActor.MaterialiseDiscoveredNodes, and the Runtime-local copies of
CapturingAddressSpaceBuilder/DiscoveredNode.

The connect-time discovery loop goes too (StartDiscovery, RediscoverTick,
HandleRediscoverAsync, DiscoveredNodesReady, TriggerRediscovery). With
injection gone it had no consumer, and leaving it would either dead-letter or
keep browsing real devices up to ~15x per connect to drop the result.
ITagDiscovery itself stays — the /raw browse picker drives it through
Commons/Browsing/DiscoveryDriverBrowser, which never went through the actor,
so the picker is unaffected.

deferment.md §4.4 called the 18 DiscoveryInjectionDormantV3 tests "Real —
blocked on #507". They are not: they assert an equipment-rooted graft
(EquipmentRootNodeId == "EQ-1") that v3 cannot produce, so they could never
have unskipped as written. Deleted along with DiscoveredNodeMapperTests, the
CapturingAddressSpaceBuilder tests, and the MaterialiseDiscoveredNodes tests
in AddressSpaceApplierTests/OpcUaPublishActorTests. Runtime.Tests skipped:
31 -> 13.

IHostConnectivityProbe is deliberately NOT resolved here. GetHostStatuses()
has no production call site and nothing writes a DriverHostStatus row, but the
table was re-created in the v3 initial migration, so deleting on inference
would be wrong. Split to Gitea #521 with both options costed.

CLAUDE.md, docs/drivers/{Galaxy,TwinCAT,MTConnect}.md updated — they described
the seam as dead.

Build clean; Runtime.Tests 476 passed / 13 skipped; OpcUaServer.Tests 362
passed / 4 skipped.
This commit is contained in:
Joseph Doherty
2026-07-27 18:57:09 -04:00
parent 09a401b881
commit adce27e7fa
22 changed files with 117 additions and 4116 deletions
+48 -2
View File
@@ -347,7 +347,9 @@ appear in §7.1 as well.
`docs/ReadWriteOperations.md` first; it is the one that could mislead a security review.~~
**Done** — decided *make non-enforcement explicit*; wire-up tracked as Gitea **#520**. See §9.
(`docs/ReadWriteOperations.md` was **not** the sharpest one — `docs/security.md` was.)
2. **#518 + #507 together** — neither fix is observable alone.
2. ~~**#518 + #507 together** — neither fix is observable alone.~~**Done.** The framing was right
that they had to be handled together, but wrong about the resolution: **#507 was not revivable**,
so it was deleted rather than fixed. See §9.
3. **#516** — silent config discard on 5 drivers; then re-scope #489, which it partly subsumes.
4. **G-4** — extend the existing picker-parity test to `DriverConfigModal` + `DeviceModal`; it would
have caught G-1 and G-2 for free, and this class has now recurred twice.
@@ -366,7 +368,7 @@ the work, so this register never disagrees with the tree.
| §8 item | Status | Commit |
|---|---|---|
| 1. ACL enforcement decision | ✅ **Done** — decided *make non-enforcement explicit*; Gitea **#520** tracks the wire-up | `d1e88dc4` |
| 2. #518 + #507 | ⏳ Not started — decided *signal, not mutation* | — |
| 2. #518 + #507 | **Done**`IRediscoverable` consumed as a re-browse prompt; #507's injection path **deleted**; `IHostConnectivityProbe` half split to Gitea **#521** | `09a401b8`, `97c9f4b4` |
| 3. #516 config discard | ⏳ Not started — decided *both* (per-driver re-parse **and** seam respawn) | — |
| 4. G-4 dispatch-map parity | ⏳ Not started | — |
| 5. Bookkeeping sweep | ⏳ Not started | — |
@@ -415,6 +417,50 @@ Also recorded in #520 and not previously known: every existing evaluator unit te
is effectively untested; and `NodeAcl.ScopeId` has no FK or existence check, so scope ids dangle
silently.
### 2026-07-27 — §8.2 rediscovery as a signal; injection deleted
Decision: **signal, not mutation.**
`DriverInstanceActor` now consumes `IRediscoverable.OnRediscoveryNeeded` (attach in `PreStart`, detach
in `PostStop`) and carries it on the driver-health snapshot to `/hosts` as a **re-browse chip**. It is
advisory — the served address space is unchanged, because v3 authors raw tags through `/raw`
browse-commit and a runtime graft would materialise nodes nobody approved.
**#507's injection path was deleted, not fixed.** §2 recorded it as a "deferred (deliberate guard)".
That was too generous: its two inputs — `EquipmentNode.DriverInstanceId` and `EquipmentTags` — are
*structurally empty* in v3, so removing the guard would have changed a log line and injected nothing.
Deleted with it: `HandleDiscoveredNodes`, `PartitionDiscoveredByDeviceHost`,
`ApplyDiscoveredPlansForDriver`, the redeploy re-inject tail, `DiscoveredNodeMapper`,
`AddressSpaceApplier.MaterialiseDiscoveredNodes`, `OpcUaPublishActor.MaterialiseDiscoveredNodes`, the
connect-time discovery loop, and 4 files' worth of tests. `ITagDiscovery` stays — the `/raw` browse
picker drives it through `Commons/Browsing/DiscoveryDriverBrowser`, which never went through the actor.
**§4.4's "18 `DiscoveryInjectionDormantV3` — Real, blocked on #507" was wrong.** They were v2
characterization tests asserting an equipment-rooted graft (`EquipmentRootNodeId == "EQ-1"`) that v3
cannot produce. They could never have unskipped as written; they are deleted. Skipped tests in
`Runtime.Tests` went 31 → 13.
**The planned drift detector was deliberately NOT built.** The plan proposed diffing each connect-time
discovery pass against the authored raw tags. Reading the drivers killed it: **Modbus, S7, MQTT,
AbLegacy and Sql all echo authored config from `DiscoverAsync`** rather than browsing the device, so
the diff is permanently empty for them. It would have been another plausible-looking inert seam —
precisely the class this register exists to remove. That also removed the last consumer of the
connect-time loop, which is why the loop went too: it was browsing real devices up to ~15× per connect
and dropping the result.
Two traps worth keeping:
- `PublishHealthSnapshot` dedups on a health fingerprint, and a rediscovery raise changes none of the
four fields it hashed. The timestamp had to join the tuple or the dedup swallows the signal.
**Verified by reverting the one line — 3 of the 5 new tests go red.**
- The shared `StubDriver.GetHealth()` returns `DateTime.UtcNow`, so its fingerprint differs every call
and the dedup never engages. A dedup test built on it would pass whether or not the fix is present;
the new tests use a stub with stable health.
`IHostConnectivityProbe` was **not** resolved — it is a keep-or-delete decision, and the
`DriverHostStatus` table was re-created deliberately in the v3 initial migration, so deleting on
inference would be wrong. Split to Gitea **#521** with both options costed.
---
*Generated 2026-07-27 against `master` @ `90bdaa44` by five parallel source-verification agents.