diff --git a/src/ZB.MOM.WW.ScadaBridge.Commons/Messages/Instance/WaitForAttribute.cs b/src/ZB.MOM.WW.ScadaBridge.Commons/Messages/Instance/WaitForAttribute.cs
index 39409759..6617952b 100644
--- a/src/ZB.MOM.WW.ScadaBridge.Commons/Messages/Instance/WaitForAttribute.cs
+++ b/src/ZB.MOM.WW.ScadaBridge.Commons/Messages/Instance/WaitForAttribute.cs
@@ -39,19 +39,27 @@ public record WaitForAttributeRequest(
///
/// Reply to a . Exactly one of
/// / is set on the happy paths;
-/// is populated only on the defensive cap-exceeded path.
+/// is populated on the failure paths (per-instance
+/// waiter cap exceeded, or the match predicate threw).
///
/// Echoes the request's correlation id.
/// True when the attribute reached the target/predicate within the timeout.
/// The matched value (null on timeout / error).
-/// The attribute quality at match time (empty on timeout / error).
+///
+/// The attribute quality at match time; on the non-match
+/// paths (timeout / error / cap-exceeded), matching the nullable
+/// convention.
+///
/// True when the timeout fired before a match.
-/// Non-null only when the wait was refused (e.g. per-instance waiter cap exceeded).
+///
+/// Non-null only when the wait failed/refused — the per-instance waiter cap was
+/// exceeded, or the match predicate threw ("Wait predicate threw: …").
+///
public record WaitForAttributeResponse(
string CorrelationId,
bool Matched,
object? Value,
- string Quality,
+ string? Quality,
bool TimedOut,
string? ErrorMessage = null);
diff --git a/src/ZB.MOM.WW.ScadaBridge.SiteRuntime/Actors/InstanceActor.cs b/src/ZB.MOM.WW.ScadaBridge.SiteRuntime/Actors/InstanceActor.cs
index 59643389..91043e83 100644
--- a/src/ZB.MOM.WW.ScadaBridge.SiteRuntime/Actors/InstanceActor.cs
+++ b/src/ZB.MOM.WW.ScadaBridge.SiteRuntime/Actors/InstanceActor.cs
@@ -571,12 +571,35 @@ public class InstanceActor : ReceiveActor
}
// Fast path: the current value already satisfies the test → reply now.
- if (_attributes.TryGetValue(req.AttributeName, out var current) && test(current))
+ // A script-supplied predicate (or the codec-equality lambda) runs on the
+ // actor thread; guard it so a throwing predicate cannot crash the actor or
+ // leak a never-resolved waiter. On throw: reply non-matched + ErrorMessage
+ // and return WITHOUT registering (no timeout scheduled).
+ if (_attributes.TryGetValue(req.AttributeName, out var current))
{
- _attributeQualities.TryGetValue(req.AttributeName, out var quality);
- replyer.Tell(new WaitForAttributeResponse(
- req.CorrelationId, Matched: true, current, quality ?? "Good", TimedOut: false));
- return;
+ bool fastMatch;
+ try
+ {
+ fastMatch = test(current);
+ }
+ catch (Exception ex)
+ {
+ _logger.LogWarning(ex,
+ "WaitForAttribute predicate threw on the fast-path for {Instance}.{Attribute}; refusing the wait",
+ _instanceUniqueName, req.AttributeName);
+ replyer.Tell(new WaitForAttributeResponse(
+ req.CorrelationId, Matched: false, null, null, TimedOut: false,
+ ErrorMessage: "Wait predicate threw: " + ex.Message));
+ return;
+ }
+
+ if (fastMatch)
+ {
+ _attributeQualities.TryGetValue(req.AttributeName, out var quality);
+ replyer.Tell(new WaitForAttributeResponse(
+ req.CorrelationId, Matched: true, current, quality ?? "Good", TimedOut: false));
+ return;
+ }
}
// Defensive cap: refuse rather than register if the instance already has
@@ -584,7 +607,7 @@ public class InstanceActor : ReceiveActor
if (_attributeWaiters.Count >= MaxAttributeWaiters)
{
replyer.Tell(new WaitForAttributeResponse(
- req.CorrelationId, Matched: false, null, "", TimedOut: false,
+ req.CorrelationId, Matched: false, null, null, TimedOut: false,
ErrorMessage: "Too many concurrent attribute waiters on this instance"));
return;
}
@@ -607,7 +630,7 @@ public class InstanceActor : ReceiveActor
if (_attributeWaiters.Remove(msg.CorrelationId, out var pending))
{
pending.Replyer.Tell(new WaitForAttributeResponse(
- msg.CorrelationId, Matched: false, null, "", TimedOut: true));
+ msg.CorrelationId, Matched: false, null, null, TimedOut: true));
}
}
@@ -648,9 +671,14 @@ public class InstanceActor : ReceiveActor
_attributeQualities[attrName] = "Bad";
_attributeTimestamps[attrName] = update.Timestamp;
var currentValue = _attributes.GetValueOrDefault(attrName);
+ // WaitForAttribute (spec §4.2): quality-only republish — the
+ // stored value is UNCHANGED (we publish the OLD currentValue, only
+ // the quality flips to Bad). Do NOT evaluate waiters, or an
+ // "any-change" / unchanged-value-equality waiter would fire on a
+ // non-change.
PublishAndNotifyChildren(new AttributeValueChanged(
_instanceUniqueName, update.TagPath, attrName,
- currentValue, "Bad", update.Timestamp));
+ currentValue, "Bad", update.Timestamp), evaluateWaiters: false);
}
continue;
}
@@ -1000,7 +1028,17 @@ public class InstanceActor : ReceiveActor
/// Publishes attribute change to stream and notifies child Script/Alarm actors.
/// WP-22: Tell for attribute notifications (fire-and-forget, never blocks).
///
- private void PublishAndNotifyChildren(AttributeValueChanged changed)
+ /// The attribute change to publish.
+ ///
+ /// WaitForAttribute (spec §4.2): when true (the default), registered
+ /// Attributes.WaitAsync waiters on this attribute are re-evaluated against
+ /// 's value. Pass false on republish/quality-only
+ /// paths that do NOT assign a new value to _attributes[name] (e.g. the
+ /// List-coerce-failure Bad-quality republish, which publishes the OLD value) —
+ /// otherwise an "any-change" waiter (or a waiter whose target equals the unchanged
+ /// value) would spuriously fire even though nothing actually changed.
+ ///
+ private void PublishAndNotifyChildren(AttributeValueChanged changed, bool evaluateWaiters = true)
{
// WP-23: Publish to site-wide stream
_streamManager?.PublishAttributeValueChanged(changed);
@@ -1017,15 +1055,19 @@ public class InstanceActor : ReceiveActor
alarmActor.Tell(changed);
}
- // WaitForAttribute (spec §4.2): re-evaluate any waiters on THIS attribute.
- // PublishAndNotifyChildren is THE single choke point for every value change
- // — both the DCL ingest path (HandleAttributeValueChanged) and the static
- // write path (HandleSetStaticAttributeCore) call it AFTER updating
- // _attributes, so changed.Value is the just-applied current value. Iterate a
- // snapshot so satisfied waiters can be removed during the loop; each match
- // cancels its scheduled timeout (so no stray WaitForAttributeTimeout follows)
- // and replies Matched=true.
- ResolveMatchedWaiters(changed);
+ // WaitForAttribute (spec §4.2): re-evaluate any waiters on THIS attribute —
+ // but ONLY when this publish reflects a real value change (evaluateWaiters).
+ // The genuine value-change paths (HandleAttributeValueChanged, the scalar
+ // DCL update path, HandleSetStaticAttributeCore) call it AFTER assigning
+ // _attributes[name], so changed.Value is the just-applied current value.
+ // Republish/quality-only paths (List-coerce-failure Bad-quality, which
+ // publishes the OLD value) pass evaluateWaiters:false so an "any-change" or
+ // unchanged-value-equality waiter does not spuriously fire (spec §4.2).
+ // Iterate a snapshot so satisfied waiters can be removed during the loop;
+ // each match cancels its scheduled timeout (so no stray WaitForAttributeTimeout
+ // follows) and replies Matched=true.
+ if (evaluateWaiters)
+ ResolveMatchedWaiters(changed);
}
///
@@ -1033,19 +1075,50 @@ public class InstanceActor : ReceiveActor
/// 's attribute whose test now passes against the
/// just-applied value — cancelling its timeout, replying Matched, and removing
/// it from the registry. A no-op when there are no waiters.
+ ///
+ ///
+ /// Each waiter's match test runs inside a per-waiter try/catch: a throwing
+ /// script-supplied predicate (or codec lambda) must NOT abort the loop and
+ /// strand sibling waiters on the same attribute, nor leave the throwing waiter
+ /// registered with a live scheduled timeout. On throw we cancel that waiter's
+ /// timeout, reply non-matched + ErrorMessage, remove it, and continue.
+ ///
///
private void ResolveMatchedWaiters(AttributeValueChanged changed)
{
if (_attributeWaiters.Count == 0)
return;
- var matched = _attributeWaiters
- .Where(kvp => kvp.Value.AttributeName == changed.AttributeName
- && kvp.Value.Test(changed.Value))
+ // Snapshot the candidate waiters on THIS attribute. Iterating a snapshot
+ // (and NOT evaluating the test inside the LINQ filter) keeps removal mid-loop
+ // safe and ensures one throwing test cannot abort materialization for siblings.
+ var candidates = _attributeWaiters
+ .Where(kvp => kvp.Value.AttributeName == changed.AttributeName)
.ToList();
- foreach (var (cid, pending) in matched)
+ foreach (var (cid, pending) in candidates)
{
+ bool matched;
+ try
+ {
+ matched = pending.Test(changed.Value);
+ }
+ catch (Exception ex)
+ {
+ _logger.LogWarning(ex,
+ "WaitForAttribute predicate threw while resolving waiter {CorrelationId} on {Instance}.{Attribute}; evicting it",
+ cid, _instanceUniqueName, changed.AttributeName);
+ pending.Timeout.Cancel();
+ pending.Replyer.Tell(new WaitForAttributeResponse(
+ cid, Matched: false, null, null, TimedOut: false,
+ ErrorMessage: "Wait predicate threw: " + ex.Message));
+ _attributeWaiters.Remove(cid);
+ continue;
+ }
+
+ if (!matched)
+ continue;
+
pending.Timeout.Cancel();
pending.Replyer.Tell(new WaitForAttributeResponse(
cid, Matched: true, changed.Value, changed.Quality, TimedOut: false));
diff --git a/src/ZB.MOM.WW.ScadaBridge.SiteRuntime/Scripts/ScopeAccessors.cs b/src/ZB.MOM.WW.ScadaBridge.SiteRuntime/Scripts/ScopeAccessors.cs
index 165a446f..f2388396 100644
--- a/src/ZB.MOM.WW.ScadaBridge.SiteRuntime/Scripts/ScopeAccessors.cs
+++ b/src/ZB.MOM.WW.ScadaBridge.SiteRuntime/Scripts/ScopeAccessors.cs
@@ -81,9 +81,24 @@ public class AttributeAccessor
/// false on timeout (no throw). Honors the script's execution-timeout token.
/// Scope/composition path resolution () is applied just like
/// / .
+ ///
+ ///
+ /// Quality-agnostic by default (spec §4.2): matching tests the VALUE, not
+ /// the quality — a value arriving at Bad quality still satisfies the wait. A
+ /// quality-gated ("Good"-only) mode is a planned enhancement, deferred per spec §4.2.
+ ///
+ ///
+ ///
+ /// Passing a null means "match on any change":
+ /// the wait then matches the next value the attribute receives — and matches
+ /// IMMEDIATELY (fast-path) if the attribute already holds any value at registration.
+ ///
///
/// The attribute key (scope-resolved before the wait is registered).
- /// The value to wait for (codec-encoded for comparison).
+ ///
+ /// The value to wait for (codec-encoded for comparison); null means
+ /// "match on any change" (matches immediately if the attribute already has a value).
+ ///
/// How long to wait before returning false.
/// true on match within the timeout; false on timeout.
public Task WaitAsync(string key, object? targetValue, TimeSpan timeout)
@@ -95,6 +110,13 @@ public class AttributeAccessor
/// value, bounded by . Site-local only (the predicate
/// is an in-process delegate). Returns true if matched within the timeout,
/// false on timeout (no throw). Scope/composition path resolution applies.
+ ///
+ ///
+ /// Quality-agnostic by default (spec §4.2): the predicate is tested against
+ /// the VALUE, regardless of quality — a value arriving at Bad quality still
+ /// satisfies the wait if the predicate passes. A quality-gated ("Good"-only) mode
+ /// is a planned enhancement, deferred per spec §4.2.
+ ///
///
/// The attribute key (scope-resolved before the wait is registered).
/// The site-local predicate tested against the current value.
diff --git a/src/ZB.MOM.WW.ScadaBridge.SiteRuntime/Scripts/ScriptRuntimeContext.cs b/src/ZB.MOM.WW.ScadaBridge.SiteRuntime/Scripts/ScriptRuntimeContext.cs
index d37a91a7..025794f0 100644
--- a/src/ZB.MOM.WW.ScadaBridge.SiteRuntime/Scripts/ScriptRuntimeContext.cs
+++ b/src/ZB.MOM.WW.ScadaBridge.SiteRuntime/Scripts/ScriptRuntimeContext.cs
@@ -399,6 +399,21 @@ public class ScriptRuntimeContext
/// so the InstanceActor's own scheduled timeout reply is the authoritative path
/// for the false/timed-out outcome, not the Ask deadline.
///
+ ///
+ ///
+ /// Quality-agnostic by default (spec §4.2): a value arriving at Bad
+ /// quality still satisfies the wait — the match tests the value, not the quality.
+ /// A quality-gated ("Good"-only) mode is a planned enhancement, deferred per spec §4.2.
+ ///
+ ///
+ ///
+ /// Never throws on timeout. An
+ /// (the pathological case where the InstanceActor's authoritative timeout reply
+ /// never arrives — actor stopped/restarted) is caught and surfaced as false,
+ /// matching the timeout contract. An /
+ /// from the script-deadline token is NOT caught
+ /// — it propagates to abort the script (intended §4.3 behaviour).
+ ///
///
/// The scope-resolved attribute name to wait on.
///
@@ -415,10 +430,24 @@ public class ScriptRuntimeContext
var req = new WaitForAttributeRequest(
cid, _instanceName, name, targetValueEncoded, predicate, timeout, DateTimeOffset.UtcNow);
- var resp = await _instanceActor.Ask(
- req, timeout + _askTimeout, _scriptTimeoutToken);
+ try
+ {
+ var resp = await _instanceActor.Ask(
+ req, timeout + _askTimeout, _scriptTimeoutToken);
- return resp.Matched;
+ return resp.Matched;
+ }
+ catch (AskTimeoutException)
+ {
+ // Pathological: the InstanceActor's own scheduled timeout reply never
+ // arrived (e.g. the actor stopped/restarted under us). The helper's
+ // contract is "false on timeout, never throw" — so swallow and return
+ // false rather than leaking the Ask exception to the script.
+ // OperationCanceledException / TaskCanceledException from the
+ // script-deadline token are deliberately NOT caught here: they must
+ // propagate to abort the script (§4.3).
+ return false;
+ }
}
///
diff --git a/tests/ZB.MOM.WW.ScadaBridge.SiteRuntime.Tests/Actors/InstanceActorWaitForAttributeTests.cs b/tests/ZB.MOM.WW.ScadaBridge.SiteRuntime.Tests/Actors/InstanceActorWaitForAttributeTests.cs
index c9db26dc..8745d53a 100644
--- a/tests/ZB.MOM.WW.ScadaBridge.SiteRuntime.Tests/Actors/InstanceActorWaitForAttributeTests.cs
+++ b/tests/ZB.MOM.WW.ScadaBridge.SiteRuntime.Tests/Actors/InstanceActorWaitForAttributeTests.cs
@@ -5,6 +5,7 @@ using Microsoft.Extensions.Logging.Abstractions;
using ZB.MOM.WW.ScadaBridge.Commons.Interfaces.Protocol;
using ZB.MOM.WW.ScadaBridge.Commons.Messages.DataConnection;
using ZB.MOM.WW.ScadaBridge.Commons.Messages.Instance;
+using ZB.MOM.WW.ScadaBridge.Commons.Messages.Streaming;
using ZB.MOM.WW.ScadaBridge.Commons.Types.Flattening;
using ZB.MOM.WW.ScadaBridge.SiteRuntime.Actors;
using ZB.MOM.WW.ScadaBridge.SiteRuntime.Persistence;
@@ -293,10 +294,12 @@ public class InstanceActorWaitForAttributeTests : TestKit, IDisposable
///
/// Spec §4.1: a null TargetValueEncoded + null Predicate means "wait for any
- /// change" — the next value update on that attribute matches.
+ /// change" (test _ => true). When the attribute ALREADY holds a value at
+ /// registration, the fast-path matches IMMEDIATELY — there is no need to wait for
+ /// a subsequent update. (A separate test covers the absent-at-registration case.)
///
[Fact]
- public void WaitForAttribute_AnyChange_MatchesOnNextUpdate()
+ public void WaitForAttribute_AnyChange_MatchesImmediatelyWhenAttributePresent()
{
const string tag = "ns=3;s=Speed";
var config = new FlattenedConfiguration
@@ -338,4 +341,227 @@ public class InstanceActorWaitForAttributeTests : TestKit, IDisposable
Assert.True(response.Matched);
Assert.False(response.TimedOut);
}
+
+ ///
+ /// Spec §4.1 (companion to the immediate-match case): when the attribute is
+ /// ABSENT at registration (no entry in _attributes), the "any change"
+ /// waiter does NOT fast-path — it registers, and a later value update on that
+ /// attribute is the first thing that satisfies it.
+ ///
+ [Fact]
+ public void WaitForAttribute_AnyChange_AttributeAbsent_MatchesOnLaterSet()
+ {
+ var config = new FlattenedConfiguration
+ {
+ InstanceUniqueName = "Pump1",
+ Attributes =
+ [
+ new ResolvedAttribute { CanonicalName = "Known", Value = "x", DataType = "String" }
+ ]
+ };
+
+ var actor = CreateInstanceActor("Pump1", config);
+
+ // "Ghost" is not a configured attribute, so _attributes has no entry — the
+ // fast-path TryGetValue misses and the waiter registers rather than matching.
+ actor.Tell(new WaitForAttributeRequest(
+ "wfa-absent", "Pump1", "Ghost",
+ null, null, TimeSpan.FromSeconds(30), DateTimeOffset.UtcNow));
+
+ ExpectNoMsg(TimeSpan.FromMilliseconds(300));
+
+ // A direct AttributeValueChanged for "Ghost" populates _attributes and
+ // re-evaluates the waiter; the any-change test now matches the new value.
+ actor.Tell(new AttributeValueChanged(
+ "Pump1", "Ghost", "Ghost", "appeared", "Good", DateTimeOffset.UtcNow));
+
+ var response = ExpectMsg(TimeSpan.FromSeconds(5));
+ Assert.True(response.Matched);
+ Assert.False(response.TimedOut);
+ Assert.Equal("wfa-absent", response.CorrelationId);
+ Assert.Equal("appeared", response.Value);
+ }
+
+ // ── 7. CRITICAL 1: no spurious match on a quality-only republish ─────────
+
+ ///
+ /// CRITICAL 1 regression: the List-coerce-failure Bad-quality path republishes
+ /// the OLD value (quality flipped to Bad) WITHOUT changing _attributes, so
+ /// it passes evaluateWaiters:false — registered waiters are NOT re-evaluated
+ /// on this non-change republish, must NOT spuriously fire, and must STILL resolve
+ /// on the next genuine value change.
+ ///
+ ///
+ /// We register an "any-change" waiter (which correctly fast-path matches the
+ /// present value and is drained) plus a pending predicate waiter that does not yet
+ /// match, then drive the Bad-quality republish and assert NO match is delivered for
+ /// the pending waiter, and that a subsequent REAL change resolves it. (Note: the
+ /// purest "any-change fires on a non-change republish" symptom is not directly
+ /// reproducible — an any-change waiter against a present attribute always fast-path
+ /// matches and so never stays pending across a republish; this test guards the
+ /// republish path against double-firing / stranding waiters and against the
+ /// predicate being re-evaluated on the non-change republish.)
+ ///
+ ///
+ [Fact]
+ public void WaitForAttribute_BadQualityRepublish_NoValueChange_DoesNotMatch()
+ {
+ const string tag = "ns=3;s=Items";
+ var config = new FlattenedConfiguration
+ {
+ InstanceUniqueName = "Pump1",
+ Attributes =
+ [
+ new ResolvedAttribute
+ {
+ // Static default {1,2}: a real list value is present from
+ // construction so the Bad-quality republish has an OLD value to
+ // republish. The waiter below targets a DIFFERENT value so it is
+ // genuinely pending (no fast-path match) when the republish fires.
+ CanonicalName = "Items", Value = "[1,2]", DataType = "List",
+ ElementDataType = "Int32",
+ DataSourceReference = tag, BoundDataConnectionName = "PLC"
+ }
+ ]
+ };
+
+ var dcl = CreateTestProbe();
+ var actor = ActorOf(Props.Create(() => new InstanceActor(
+ "Pump1",
+ JsonSerializer.Serialize(config),
+ _storage,
+ _compilationService,
+ _sharedScriptLibrary,
+ null,
+ _options,
+ NullLogger.Instance,
+ dcl.Ref)));
+
+ dcl.ExpectMsg(TimeSpan.FromSeconds(5));
+
+ // A predicate waiter that matches a list of length >= 3. Current value is
+ // {1,2} (length 2) so it does NOT fast-path match — it registers and stays
+ // pending. Crucially, the Bad-quality republish below carries the SAME OLD
+ // value {1,2} (length 2); with the bug (evaluateWaiters always true) the
+ // predicate would be re-evaluated against {1,2} → still false, so this probe
+ // also guards the predicate-isolation contract on the republish path.
+ Func