docs(archreview): mark Critical 4 (TwinCAT notification replay / STAB-2) done

All 4 Criticals complete. crit4 branch af318fb4: replayable-intent store +
reconnect-replay + probe-recycle, 4 unit guards + 174/174 (unit-only, no TC3 fixture).
Next: systemic guards (task 6 forwarding test first) + Critical-1 failover harness (task 9).
This commit is contained in:
Joseph Doherty
2026-07-08 17:21:13 -04:00
parent fcd36bd53d
commit 26c9d28b69
2 changed files with 16 additions and 13 deletions
+6 -5
View File
@@ -1,10 +1,11 @@
# Design + Implementation Plan — 05 Protocol Drivers
> **Status (2026-07-08):** **STAB-1 (S7 reconnect, Critical, task 4) DONE** — `fix/archreview-crit3-s7-reconnect`
> `25c0c6f6`; `IS7Plc`/`IS7PlcFactory` seam + lazy `EnsureConnectedAsync`; 4 unit guards + LIVE-verified
> against real snap7 (container bounce → recovers, no redeploy). **STAB-2 (TwinCAT orphaned ADS subs,
> Critical, task 5) NEXT** — unit-only (no TwinCAT docker fixture). Then STAB-4/5 (AbCip lock, FOCAS caches)
> + Part B shared-template consolidation. See [`STATUS.md`](STATUS.md).
> **Status (2026-07-08):** **BOTH Criticals DONE.** STAB-1 (S7 reconnect, task 4) — `fix/archreview-crit3-s7-reconnect`
> `25c0c6f6`; `IS7Plc`/`IS7PlcFactory` seam + lazy `EnsureConnectedAsync`; 4 unit guards + LIVE-verified against real
> snap7 (container bounce → recovers, no redeploy). STAB-2 (TwinCAT orphaned ADS subs, task 5) —
> `fix/archreview-crit4-twincat-replay` `af318fb4`; replayable-intent store + reconnect replay + probe-recycle;
> 4 unit guards + 174/174 (unit-only — no TwinCAT docker fixture). **Remaining (non-Critical):** STAB-4/5 (AbCip lock,
> FOCAS caches) + Part B shared-template consolidation. See [`STATUS.md`](STATUS.md).
**Source report:** `archreview/05-protocol-drivers.md`
**Review commit:** `9cad9ed0` (== current HEAD, so no drift)
+10 -8
View File
@@ -12,6 +12,7 @@ truth for remediation progress. Update it as work lands.
| `fix/archreview-crit1-split-brain-resolver` | `a81dea10` | Critical 1 (03/S1) activation + guard test + Redundancy.md |
| `fix/archreview-crit2-vt-timeout` | `7fd44f0f` | Critical 2 (02/U2 + 02/U3) + tests |
| `fix/archreview-crit3-s7-reconnect` | `25c0c6f6` | Critical 3 (05/STAB-1) IS7Plc seam + lazy reconnect + unit guards + live bounce test |
| `fix/archreview-crit4-twincat-replay` | `af318fb4` | Critical 4 (05/STAB-2) native-notification replay + probe-recycle + unit guards |
**Decision (user):** one branch per Critical, each off `master`. The plans are only on the docs branch,
so from a critical branch read them via `git show docs/archreview-plans-and-drift-fixes:archreview/plans/0X-<name>.md`.
@@ -26,6 +27,7 @@ so from a critical branch read them via `git show docs/archreview-plans-and-drif
| **Doc drift — KNOWN LIMITATION 2 stale** | 03/U1 = 06/U-1 | `docs/…` `9fadead6` (CLAUDE.md) | historized-ref feed IS wired; rewrote to "value-capture code-complete, live-verify pending". |
| **Doc drift — ScriptAnalysis policy** | theme #5 | same `9fadead6` (CLAUDE.md) | corrected `FleetAdmin``Administrator,Designer` roles. |
| **Critical 3 — S7 lazy reconnect** | 05/STAB-1 | `fix/archreview-crit3-s7-reconnect` `25c0c6f6` | New `IS7Plc`/`IS7PlcFactory` seam + `EnsureConnectedAsync` (dispose-dead-handle → reopen on next data call); connection-fatal faults (socket/`ErrorCode.ConnectionError`) mark the handle dead, data-address errors do NOT; probe loop routes through it as a backstop. 4 unit guards + full S7 suite 234/234; **LIVE-verified** against real python-snap7 — bounced the container (`docker restart`), driver observed the outage and resumed Good reads with no redeploy (`S7_1500ReconnectTests`, double-gated on sim + `S7_RECONNECT_BOUNCE_CMD`). Also closes the S7 TEST-1 gap. |
| **Critical 4 — TwinCAT native-notification replay** | 05/STAB-2 | `fix/archreview-crit4-twincat-replay` `af318fb4` | Native ADS notifications were opaque handles (no replayable intent) → silently orphaned on a client swap. Now `NativeRegistration` (symbol/type/bit/interval/onChange + swappable handle) hangs off `DeviceState`; `EnsureConnectedAsync` split into a gate wrapper + under-gate core that **replays every intent onto a freshly-built client** (register+replay both under `ConnectGate`); probe-failure `RecycleClientAsync` disposes+nulls the wire-dead-but-port-open client so the next tick rebuilds+replays. 4 unit guards + full TwinCAT suite 174/174. **Unit-only — no TwinCAT docker fixture exists** (integration needs a real TC3 XAR), so the fake-client tests are authoritative (no live gate like S7 had). |
## Task list (TaskCreate IDs)
@@ -35,7 +37,7 @@ so from a critical branch read them via `git show docs/archreview-plans-and-drif
| 2 | Critical 2a — VT timeout | ✅ completed |
| 3 | Critical 2b — CompiledScriptCache | ✅ completed |
| 4 | Critical 3 — S7 reconnect (05/STAB-1) | ✅ completed |
| 5 | Critical 4 — TwinCAT ADS re-registration (05/STAB-2) | ⬜ pending |
| 5 | Critical 4 — TwinCAT ADS re-registration (05/STAB-2) | ✅ completed |
| 6 | Guard — DeferredAddressSpaceSink reflection forwarding test (03/U2) | ⬜ pending — **must precede** 03/P1 surgical-adds + any new sink capability |
| 7 | Guard — wire OTOPCUA0001 analyzer + triage (07/C-1) | ⬜ pending — triage before Warning severity; C-1 triage → C-2 |
| 8 | Guard — CI to all unit suites + fail-on-skip (07/S-1,S-2, pair S-4) | ⬜ pending |
@@ -43,13 +45,13 @@ so from a critical branch read them via `git show docs/archreview-plans-and-drif
## Suggested next (from 00-INDEX execution order)
1. **Critical 4 (05/STAB-2)** — TwinCAT ADS notification re-registration after reconnect. Store a
replayable registration intent (symbol/type/bitIndex/interval/onChange) per subscribed tag and replay
it on client swap; the fake `ITwinCATClientFactory`/`ITwinCATClient` already exist, so a
`TwinCATReconnectReplayTests` can be deterministic. **No TwinCAT docker fixture exists** (13 integration
tests need a real TC3 target), so unit-only — no live gate available like S7 had. New branch off master.
*(Critical 3 (05/STAB-1) DONE — see Completed table, LIVE-verified against real snap7.)*
2. **Systemic guards** — task 7 (analyzer) then task 8 (CI); task 6 (forwarding test) before any P1/P2 work.
**All 4 Criticals are DONE** (see Completed table). Remaining work is the systemic guards + failure-visibility Highs:
1. **Systemic guards** — task 6 (`DeferredAddressSpaceSink` reflection forwarding test) **first**, before any P1/P2
surgical-add work (else new sink caps ship inert — F10b/PR#423 class); then task 7 (wire OTOPCUA0001 analyzer,
triage before Warning severity) → task 8 (CI to all unit suites + fail-on-skip, pair the S-4 fixed-sleep flakes).
2. **Critical 1 follow-up (task 9)** — the hard-kill failover integration test on the 2-node harness (the effort-M
harness half; SBR activation itself is done+verified).
3. **Failure-visibility Highs** — 01/S-1 (AddressSpaceApplyOutcome failure field), 06/S-1 (Galaxy fail-closed),
03/S4 (primary-gate default-deny).