dohertj2/scadalink-design
1
0
Fork 0
Code Issues Pull Requests 1 Actions Packages Projects Releases Wiki Activity

fix(site-runtime): resolve SiteRuntime-001/002/003 — route data-sourced writes to DCL, real per-attribute API results, race-free redeploy

Browse Source
This commit is contained in:
Joseph Doherty
2026-05-16 19:57:28 -04:00
parent 1ae11d1135
commit 09b4bd5dfa
9 changed files with 575 additions and 52 deletions
Download Patch File Download Diff File Expand all files Collapse all files

36
code-reviews/SiteRuntime/findings.md
View File

@@ -8,7 +8,7 @@
| Last reviewed | 2026-05-16 |
| Reviewer | claude-agent |
| Commit reviewed | `9c60592` |
| Open findings | 16 |
| Open findings | 13 |
## Summary
@@ -51,7 +51,7 @@ actor, and the repositories are untested.
|--|--|
| Severity | High |
| Category | Design-document adherence |
| Status | Open |
| Status | Resolved |
| Location | `src/ScadaLink.SiteRuntime/Scripts/ScriptRuntimeContext.cs:106`, `src/ScadaLink.SiteRuntime/Actors/InstanceActor.cs:204` |
**Description**
@@ -84,7 +84,15 @@ or branching inside the handler.
**Resolution**
_Unresolved._
Resolved 2026-05-16 (`<pending>`): `InstanceActor.HandleSetStaticAttribute` now resolves
the target attribute's data binding from `_configuration`. Data-sourced attributes are
routed via a new `HandleSetDataAttribute` that Asks the DCL with a `WriteTagRequest` and
pipes the device-write outcome back to the caller as a `SetStaticAttributeResponse` —
no override is persisted and `_attributes` is not optimistically mutated. Static
attributes keep the override path and now also reply with a `SetStaticAttributeResponse`.
`ScriptRuntimeContext.SetAttribute` is now `async Task` and Asks the Instance Actor,
throwing `InvalidOperationException` on a failed device write so scripts get the failure
synchronously.
### SiteRuntime-002 — `RouteInboundApiSetAttributes` always treats writes as static overrides
@@ -92,7 +100,7 @@ _Unresolved._
|--|--|
| Severity | High |
| Category | Correctness & logic bugs |
| Status | Open |
| Status | Resolved |
| Location | `src/ScadaLink.SiteRuntime/Actors/DeploymentManagerActor.cs:632` |
**Description**
@@ -115,7 +123,13 @@ response path.
**Resolution**
_Unresolved._
Resolved 2026-05-16 (`<pending>`): `RouteInboundApiSetAttributes` now Asks the Instance
Actor per attribute (instead of fire-and-forget Tell) and aggregates the
`SetStaticAttributeResponse` results. Because the Instance Actor handler is the
SiteRuntime-001 corrected handler, data-sourced attributes now reach the DCL and the
`RouteToSetAttributesResponse` reflects the real per-attribute outcome — a non-existent
attribute or a failed device write is reported as failure rather than an unconditional
optimistic `true`.
### SiteRuntime-003 — Redeployment relies on a fixed 500 ms reschedule and can collide on the child actor name
@@ -123,7 +137,7 @@ _Unresolved._
|--|--|
| Severity | High |
| Category | Akka.NET conventions |
| Status | Open |
| Status | Resolved |
| Location | `src/ScadaLink.SiteRuntime/Actors/DeploymentManagerActor.cs:222` |
**Description**
@@ -148,7 +162,15 @@ instance) until termination completes.
**Resolution**
_Unresolved._
Resolved 2026-05-16 (`<pending>`): `HandleDeploy` no longer uses a fixed 500 ms
reschedule. When a redeployment targets a running instance, the existing Instance Actor
is `Context.Watch`-ed and stopped, and the in-flight `DeployInstanceCommand` is buffered
in a `_pendingRedeploys` map keyed by the terminating actor ref. A new `Terminated`
handler recreates the Instance Actor only after the predecessor (and its whole subtree)
has fully stopped, eliminating the `InvalidActorNameException` race and the
unconditional redeploy-latency penalty. The shared `ApplyDeployment` helper also skips
the `_totalDeployedCount` increment for redeployments, so the deployed-instance count no
longer drifts (this additionally addresses the root cause behind SiteRuntime-004).
### SiteRuntime-004 — `_totalDeployedCount` is incremented on redeployment of an existing instance