merge: audit-config query fix + rig live-probe seed (register rows 46, 48) — ValueTuple serialized as {} fixed with AuditLogQueryResult record + ITuple guard; docker/seed-live-probe.sh

2026-08-15 12:00:32 -04:00
parent 4785f3cbef b5e6821caa
commit cde4af77fd
7 changed files with 323 additions and 3 deletions
@@ -87,6 +87,7 @@ docker/
 ├── build.sh                            # Build Docker image
 ├── deploy.sh                           # Build + deploy all containers
 ├── seed-sites.sh                       # Create test sites with Akka + gRPC addresses
+├── seed-live-probe.sh                  # Seed one deployable live-data instance on site-a (stream traffic)
 ├── teardown.sh                         # Stop and remove containers
 ├── central-node-a/
 │   ├── appsettings.Central.json        # Central node A configuration
@@ -160,6 +161,41 @@ docker/deploy.sh

 # 3. Seed test sites (first-time only, after cluster is healthy)
 docker/seed-sites.sh
+
+# 4. Seed one deployable live-data instance on site-a, so the rig has real
+#    stream traffic for live probes (see "Live-data probe instance" below)
+docker/seed-live-probe.sh
+```
+
+### Live-data probe instance
+
+`seed-live-probe.sh` creates the minimum working set the rig needs for any live probe
+of the data or streaming path: a `RigProbe` template with two attributes bound to
+**self-changing** opc-plc nodes (`ns=3;s=SpikeData`, `ns=3;s=RandomSignedInt32`, both
+~1 Hz) plus a 5 s heartbeat script, and a deployed `rig-probe-1` instance on site-a.
+
+Two traps it exists to avoid:
+
+- **The custom tags in `infra/opcua/nodes.json` are declared but not simulated.**
+  `Motor.*`, `Tank.*`, `Valve.*`, `TestChildObject.*` read 0 / false / epoch forever.
+  Binding to them gives a `Connected` data connection with `Good` quality and *zero*
+  event traffic, which looks healthy and probes nothing. Use the built-in simulated
+  nodes (`SpikeData`, `RandomSignedInt32`, `StepUp`, `FastUInt1..5`, `SlowUInt1..5`).
+- **`scadabridge_site_connection_up` does not mean what its name suggests.** It counts
+  open central→site gRPC *subscriber streams*, not OPC UA data connections, so it sits
+  at 0 on a perfectly healthy site with nobody watching. For data-connection health read
+  `dataConnectionEndpoints` / `dataConnectionTagQuality` off `scadabridge health site`.
+
+Verify (metrics are on container port 8084; the containers ship no curl/wget):
+
+```bash
+scadabridge debug snapshot --id <instance-id>
+scadabridge health site --identifier site-a
+scadabridge debug stream --id <instance-id>   # open a stream, then in another shell:
+
+docker exec scadabridge-site-a-a bash -c \
+  'exec 3<>/dev/tcp/localhost/8084 && printf "GET /metrics HTTP/1.0\r\nHost: localhost\r\n\r\n" >&3 && cat <&3' \
+  | grep -E 'site_connection_up|stream_batch_size'
 ```

 ### After Code Changes
@@ -0,0 +1,120 @@
+#!/bin/bash
+set -euo pipefail
+
+# Seed ONE deployable, live-data instance on site-a so the rig always has real
+# stream traffic for live probes.
+#
+# Why this exists: the rig repeatedly ends up with instances that cannot deploy
+# (deferred-work register row 48 — the seven `soak-*` rows fail pre-deployment
+# validation because their bound data connections no longer exist on the sites).
+# With nothing deployed, `debug snapshot` returns `instanceNotFound`, no gRPC site
+# stream is ever opened, and the R2 batching histogram
+# `scadabridge.site.stream.batch_size` has nothing to record — so any live probe of
+# the streaming path is unverifiable. This script rebuilds the minimum working set
+# from scratch, using only the CLI (no DB edits).
+#
+# What it creates (all on site-a / site id 1, against data connection id 1
+# "OPC PLC Simulator" -> opc.tcp://scadabridge-opcua:50000):
+#   - Template `RigProbe`
+#       * Spike      (Double) <- ns=3;s=SpikeData          — opc-plc built-in, changes ~1/s
+#       * RandomInt  (Int32)  <- ns=3;s=RandomSignedInt32  — opc-plc built-in, changes ~1/s
+#       * Heartbeat  (Int32)  — static, bumped every 5s by the Heartbeat script
+#       * Heartbeat script — Interval 5000ms, trust-policy-clean
+#   - Instance `rig-probe-1`, bound + deployed
+#
+# IMPORTANT — pick self-changing tags. The custom tags in `infra/opcua/nodes.json`
+# (Motor.*, Tank.*, Valve.*, TestChildObject.*) are declared but NOT simulated by
+# opc-plc: they read 0 / false / epoch and never change on their own, so binding to
+# them yields a healthy connection with zero event traffic. The built-in simulated
+# nodes are the ones that move: SpikeData, RandomSignedInt32, StepUp, FastUInt1..5
+# (1s) and SlowUInt1..5 (10s).
+#
+# Prerequisites:
+#   - Infrastructure services running (infra/docker-compose.yml — needs scadabridge-opcua)
+#   - Application containers running (docker/deploy.sh)
+#   - Sites seeded (docker/seed-sites.sh) and central healthy
+#       curl -sf http://localhost:9000/health/ready
+#
+# Usage:
+#   docker/seed-live-probe.sh
+#
+# Idempotency: this script is NOT idempotent — template and instance names must be
+# unique, so a second run fails on the create steps. To re-seed, delete the previous
+# instance and template first:
+#   scadabridge instance delete --id <id> && scadabridge template delete --id <id>
+
+SCRIPT_DIR="$(cd "$(dirname "$0")" && pwd)"
+PROJECT_ROOT="$(cd "$SCRIPT_DIR/.." && pwd)"
+CLI="dotnet run --project $PROJECT_ROOT/src/ZB.MOM.WW.ScadaBridge.CLI --"
+AUTH="--username multi-role --password password"
+URL="--url http://localhost:9000"
+
+SITE_ID=1              # site-a
+CONNECTION_ID=1        # "OPC PLC Simulator" on site-a
+TEMPLATE_NAME="RigProbe"
+INSTANCE_NAME="rig-probe-1"
+
+# Pulls the numeric "id" out of the CLI's JSON reply without needing jq.
+json_id() { python3 -c 'import json,sys; print(json.load(sys.stdin)["id"])'; }
+
+echo "=== Seeding live-data probe instance on site-a ==="
+
+echo ""
+echo "1/6 Creating template $TEMPLATE_NAME..."
+TEMPLATE_ID=$($CLI $URL $AUTH template create \
+    --name "$TEMPLATE_NAME" \
+    --description "Minimal live-traffic probe: two OPC UA sim tags that change on their own + a 5s heartbeat script" \
+    | json_id)
+echo "  template id = $TEMPLATE_ID"
+
+echo ""
+echo "2/6 Adding data-bound attributes..."
+$CLI $URL $AUTH template attribute add \
+    --template-id "$TEMPLATE_ID" --name Spike --data-type Double \
+    --data-source "ns=3;s=SpikeData" \
+    --description "opc-plc SpikeData — Double, changes every ~1s on its own" > /dev/null
+$CLI $URL $AUTH template attribute add \
+    --template-id "$TEMPLATE_ID" --name RandomInt --data-type Int32 \
+    --data-source "ns=3;s=RandomSignedInt32" \
+    --description "opc-plc RandomSignedInt32 — Int32, changes every ~1s on its own" > /dev/null
+
+echo ""
+echo "3/6 Adding the static heartbeat attribute + its interval script..."
+$CLI $URL $AUTH template attribute add \
+    --template-id "$TEMPLATE_ID" --name Heartbeat --data-type Int32 --value 0 \
+    --description "Static attribute bumped every 5s by the Heartbeat script — script-sourced traffic" > /dev/null
+$CLI $URL $AUTH template script add \
+    --template-id "$TEMPLATE_ID" --name Heartbeat \
+    --trigger-type Interval --trigger-config '{"intervalMs":5000}' \
+    --code 'var current = await Instance.GetAttribute("Heartbeat"); var next = (int.TryParse(current?.ToString(), out var n) ? n : 0) + 1; await Instance.SetAttribute("Heartbeat", next.ToString());' > /dev/null
+
+echo ""
+echo "4/6 Creating instance $INSTANCE_NAME..."
+INSTANCE_ID=$($CLI $URL $AUTH instance create \
+    --name "$INSTANCE_NAME" --template-id "$TEMPLATE_ID" --site-id "$SITE_ID" | json_id)
+echo "  instance id = $INSTANCE_ID"
+
+echo ""
+echo "5/6 Binding the data-sourced attributes to connection $CONNECTION_ID..."
+$CLI $URL $AUTH instance set-bindings --id "$INSTANCE_ID" \
+    --bindings "[[\"Spike\",$CONNECTION_ID],[\"RandomInt\",$CONNECTION_ID]]" > /dev/null
+
+echo ""
+echo "6/6 Deploying..."
+$CLI $URL $AUTH instance deploy --id "$INSTANCE_ID"
+
+echo ""
+echo "=== Done. Verify with: ==="
+echo "  scadabridge debug snapshot --id $INSTANCE_ID"
+echo "  scadabridge health site --identifier site-a     # dataConnectionEndpoints / dataConnectionTagQuality"
+echo "  scadabridge debug stream --id $INSTANCE_ID      # opens a gRPC site stream; ~2 events/s"
+echo ""
+echo "While a stream is open, the site node serving it reports both signals"
+echo "(metrics are on container port 8084, and the containers have no curl/wget):"
+echo "  docker exec scadabridge-site-a-a bash -c 'exec 3<>/dev/tcp/localhost/8084 && printf \"GET /metrics HTTP/1.0\\r\\nHost: localhost\\r\\n\\r\\n\" >&3 && cat <&3' \\"
+echo "    | grep -E 'site_connection_up|stream_batch_size'"
+echo ""
+echo "NOTE: scadabridge_site_connection_up counts OPEN CENTRAL->SITE gRPC SUBSCRIBER"
+echo "STREAMS, not OPC UA data connections. It reads 0 on a perfectly healthy site"
+echo "with no subscriber attached — use the health report's dataConnectionEndpoints"
+echo "for data-connection health."
@@ -241,6 +241,13 @@ warning; migrate scripts to `audit-config`.
 scadabridge audit-config query [--user <username>] [--entity-type <type>] [--action <action>] [--from <date>] [--to <date>] [--page <n>] [--page-size <n>]
 ```

+The response is an `AuditLogQueryResult` — `{ "entries": [ … ], "totalCount": <n> }`, where
+`totalCount` is the unpaged match count. The result type is a named record specifically
+because the handler must not return the repository's `(Entries, TotalCount)` ValueTuple:
+`System.Text.Json` serializes properties only, so a tuple result renders as the literal
+`{}` and every query looks empty regardless of how many rows matched. `ManagementActor.SerializeResult`
+now rejects tuple results outright so the failure mode cannot recur silently.
+
 The new centralized Audit Log component (#23) is exposed via the `scadabridge audit` group below.

 ### Centralized Audit Commands
@@ -1802,6 +1802,28 @@ scadabridge --url <url> audit-config query [options]
 | `--page` | no | `1` | Page number |
 | `--page-size` | no | `50` | Results per page |

+Output shape:
+
+```json
+{
+  "entries": [
+    {
+      "id": 412,
+      "user": "multi-role",
+      "action": "Created",
+      "entityType": "Template",
+      "entityId": "7",
+      "entityName": "PumpTemplate",
+      "timestamp": "2026-08-15T12:00:00+00:00"
+    }
+  ],
+  "totalCount": 137
+}
+```
+
+`totalCount` is the unpaged number of rows matching the filters, so a client can page
+without a second call.
+
 ---

 ### `shared-script` — Manage shared scripts
@@ -1,6 +1,25 @@
+using ZB.MOM.WW.ScadaBridge.Commons.Entities.Audit;
+
 namespace ZB.MOM.WW.ScadaBridge.Commons.Messages.Management;

 public record QueryAuditLogCommand(
    string? User = null, string? EntityType = null, string? Action = null,
    DateTimeOffset? From = null, DateTimeOffset? To = null,
    int Page = 1, int PageSize = 50);
+
+/// <summary>
+/// Result wrapper for <see cref="QueryAuditLogCommand"/>.
+/// </summary>
+/// <remarks>
+/// This record exists specifically so the handler does not return the repository's
+/// <c>(IReadOnlyList&lt;AuditLogEntry&gt; Entries, int TotalCount)</c> ValueTuple directly.
+/// A ValueTuple exposes <c>Item1</c>/<c>Item2</c> as public <em>fields</em>, and
+/// <see cref="System.Text.Json"/> serializes only properties unless
+/// <c>JsonSerializerOptions.IncludeFields</c> is set — so a tuple result serialized to
+/// the literal string <c>{}</c> and every <c>audit-config query</c> looked empty no
+/// matter how many rows the query matched (deferred-work register row 46). Mirrors the
+/// <c>SecuredWriteListResult</c> shape used by the other paged management query.
+/// </remarks>
+/// <param name="Entries">The page of configuration-change audit entries.</param>
+/// <param name="TotalCount">Total number of rows matching the filters (unpaged).</param>
+public record AuditLogQueryResult(IReadOnlyList<AuditLogEntry> Entries, int TotalCount);
@@ -90,10 +90,35 @@ public class ManagementActor : ReceiveActor
    /// Serializes a command result to JSON using <see cref="System.Text.Json"/> — the same
    /// serializer the HTTP endpoint uses — with cycle-safe settings.
    /// </summary>
+    /// <remarks>
+    /// Rejects tuple results outright. <see cref="System.Text.Json"/> writes only
+    /// properties by default, and a <c>ValueTuple</c> exposes its members as public
+    /// <em>fields</em> — so a handler that returns a repository's
+    /// <c>(rows, totalCount)</c> tuple serializes to the literal <c>{}</c>, silently
+    /// handing every caller an empty result with no error anywhere in the stack. That is
+    /// exactly how <c>audit-config query</c> shipped broken (deferred-work register row
+    /// 46). Failing loudly here converts the whole class of bug into an immediate,
+    /// obvious error the first time the handler is exercised; the fix is always to
+    /// project the tuple into a named result record before returning it.
+    /// </remarks>
    /// <param name="result">The command result object to serialize, or null.</param>
    /// <returns>The JSON string representation of <paramref name="result"/>, or <c>"null"</c> when null.</returns>
-    public static string SerializeResult(object? result) =>
-        JsonSerializer.Serialize(result, ResultSerializerOptions);
+    /// <exception cref="InvalidOperationException">
+    /// Thrown when <paramref name="result"/> is a tuple, which cannot round-trip through
+    /// the JSON contract.
+    /// </exception>
+    public static string SerializeResult(object? result)
+    {
+        if (result is System.Runtime.CompilerServices.ITuple)
+        {
+            throw new InvalidOperationException(
+                $"Management command results must be named types, not tuples — "
+                + $"System.Text.Json serializes '{result.GetType().Name}' to '{{}}' because tuple "
+                + "members are fields, not properties. Project the tuple into a result record.");
+        }
+
+        return JsonSerializer.Serialize(result, ResultSerializerOptions);
+    }

    private void HandleEnvelope(ManagementEnvelope envelope)
    {
@@ -2578,12 +2603,24 @@ public class ManagementActor : ReceiveActor
    // Audit Log handler
    // ========================================================================

+    /// <summary>
+    /// Queries the configuration-change audit trail and projects the repository's
+    /// ValueTuple into <see cref="AuditLogQueryResult"/>.
+    /// </summary>
+    /// <remarks>
+    /// The projection is load-bearing, not cosmetic: returning the repository tuple
+    /// directly made <see cref="SerializeResult"/> emit the literal <c>{}</c> (ValueTuple
+    /// exposes <c>Item1</c>/<c>Item2</c> as fields, which System.Text.Json skips), so
+    /// <c>audit-config query</c> reported an empty result for every filter combination
+    /// regardless of how many rows matched — deferred-work register row 46.
+    /// </remarks>
    private static async Task<object?> HandleQueryAuditLog(IServiceProvider sp, QueryAuditLogCommand cmd)
    {
        var repo = sp.GetRequiredService<ICentralUiRepository>();
-        return await repo.GetAuditLogEntriesAsync(
+        var (entries, totalCount) = await repo.GetAuditLogEntriesAsync(
            cmd.User, cmd.EntityType, cmd.Action, cmd.From, cmd.To,
            page: cmd.Page, pageSize: cmd.PageSize);
+        return new AuditLogQueryResult(entries, totalCount);
    }

    // ========================================================================
@@ -238,6 +238,85 @@ public class ManagementActorTests : TestKit, IDisposable
        Assert.Equal(envelope.CorrelationId, response.CorrelationId);
    }

+    // ========================================================================
+    // 2b. Configuration-change audit query -- result must not serialize to "{}"
+    // ========================================================================
+
+    /// <summary>
+    /// Deferred-work register row 46: <c>audit-config query</c> returned the literal
+    /// <c>{}</c> for every filter combination against the rig even though the
+    /// <c>AuditLogEntries</c> rows were present. The handler returned the repository's
+    /// <c>(Entries, TotalCount)</c> ValueTuple straight to <c>SerializeResult</c>, and
+    /// System.Text.Json writes properties only — a tuple's <c>Item1</c>/<c>Item2</c> are
+    /// fields, so the whole payload vanished with no error anywhere. Pins that the rows
+    /// and the total actually reach the wire.
+    /// </summary>
+    [Fact]
+    public void QueryAuditLogCommand_ReturnsEntriesAndTotalCount()
+    {
+        var uiRepo = Substitute.For<ICentralUiRepository>();
+        var entries = new List<Commons.Entities.Audit.AuditLogEntry>
+        {
+            new("alice", "Created", "Template", "7", "PumpTemplate")
+            {
+                Id = 1,
+                Timestamp = new DateTimeOffset(2026, 8, 15, 12, 0, 0, TimeSpan.Zero)
+            }
+        };
+        uiRepo.GetAuditLogEntriesAsync(
+                Arg.Any<string?>(), Arg.Any<string?>(), Arg.Any<string?>(),
+                Arg.Any<DateTimeOffset?>(), Arg.Any<DateTimeOffset?>(),
+                Arg.Any<string?>(), Arg.Any<string?>(), Arg.Any<Guid?>(),
+                Arg.Any<int>(), Arg.Any<int>(), Arg.Any<CancellationToken>())
+            .Returns(((IReadOnlyList<Commons.Entities.Audit.AuditLogEntry>)entries, 137));
+        _services.AddScoped(_ => uiRepo);
+
+        var actor = CreateActor();
+        var envelope = Envelope(new QueryAuditLogCommand(PageSize: 25), "Administrator");
+
+        actor.Tell(envelope);
+
+        var response = ExpectMsg<ManagementSuccess>(TimeSpan.FromSeconds(5));
+        Assert.NotEqual("{}", response.JsonData);
+        Assert.Contains("PumpTemplate", response.JsonData);
+        Assert.Contains("\"entries\"", response.JsonData);
+        Assert.Contains("\"totalCount\":137", response.JsonData);
+    }
+
+    /// <summary>
+    /// Guard for the whole class of bug behind row 46: any handler that returns a tuple
+    /// silently serializes to <c>{}</c>. <see cref="ManagementActor.SerializeResult"/>
+    /// now rejects tuples outright so the next occurrence is a loud failure rather than
+    /// an empty response.
+    /// </summary>
+    [Fact]
+    public void SerializeResult_TupleResult_ThrowsInsteadOfEmittingEmptyObject()
+    {
+        var ex = Assert.Throws<InvalidOperationException>(
+            () => ManagementActor.SerializeResult((new[] { "a" }, 1)));
+
+        Assert.Contains("tuple", ex.Message, StringComparison.OrdinalIgnoreCase);
+    }
+
+    /// <summary>
+    /// Pins the positive half of the guard: the named result record the handler now
+    /// returns serializes to a populated, camelCased object.
+    /// </summary>
+    [Fact]
+    public void SerializeResult_AuditLogQueryResult_EmitsEntriesAndTotalCount()
+    {
+        var json = ManagementActor.SerializeResult(new AuditLogQueryResult(
+            new List<Commons.Entities.Audit.AuditLogEntry>
+            {
+                new("bob", "Deleted", "Instance", "3", "Pump1")
+            },
+            42));
+
+        Assert.Contains("\"entries\"", json);
+        Assert.Contains("\"totalCount\":42", json);
+        Assert.Contains("Pump1", json);
+    }
+
    // ========================================================================
    // 3. Template list test -- verify data flows through
    // ========================================================================