Commit Graph

2 Commits

Author SHA1 Message Date
Joseph Doherty 41a5ecf14a feat(external-systems): deploy per-system retry policy to sites + CLI/management surface (register row 27)
An external system's MaxRetries/RetryDelay existed on the central entity but never
reached a site and had no management surface, so every ExternalSystem.CachedCall
buffered with the S&F engine default (DefaultMaxRetries 50 x DefaultRetryInterval
30s ~= 25 min to park) whatever central had configured. The sibling
DatabaseConnectionArtifact had always carried its equivalents.

The consumption end was already wired: ExternalSystemClient.CachedCallAsync already
read system.MaxRetries/RetryDelay and passed them to EnqueueAsync with the 0-as-unset
convention. Only the transport and persistence were missing.

Commons
- ExternalSystemArtifact gains MaxRetries/RetryDelay as optional positional params
  (0 / TimeSpan.Zero = unset), matching DatabaseConnectionArtifact rather than going
  nullable, so it maps 1:1 onto the entity and the proto3 scalars.
- Create/UpdateExternalSystemCommand gain nullable MaxRetries/RetryDelay, following
  the existing TimeoutSeconds "omit to preserve" convention. Both additive.

Central
- ManagementActor create maps both; update preserves-if-null.
- ArtifactDeploymentService projects them onto the artifact.

Wire
- site_command.proto ExternalSystemArtifactDto gains max_retries=7 /
  retry_delay=8 (google.protobuf.Duration), mirroring DatabaseConnectionArtifactDto.
  Additive: a pre-existing site decodes them as 0/absent, which IS the unset value,
  so old behaviour is preserved. Vendored C# regenerated via docker/regen-proto.sh
  sitecommand; csproj Protobuf item re-commented, no message types added or removed.

Site
- external_systems gains max_retries / retry_delay_ms via AddColumnIfMissing,
  DEFAULT 0 = unset. Storage upsert, repository SELECT/mapping (new ordinals 6/7
  appended so existing ones are untouched), apply path, and the legacy migrator's
  explicit column list all carry them.

Replicated-table decision: external_systems IS in SiteLocalDbSetup.ReplicatedTables,
so this was checked against the AckTime/native_alarm_state precedent. Real columns
are correct here, not a JSON bag: the table has no general-purpose config column
(auth_configuration and method_definitions are both semantically specific),
timeout_seconds already set the additive-column precedent on this exact table, and
sibling database_connections stores the same two values as plain columns. Verified in
LocalDb source that RegisterReplicated re-reads PRAGMA table_info every boot and its
install script is DROP-then-CREATE, so capture triggers rebuild over the new column
set automatically, and SiteStorageSchema.Apply runs before registration on the same
boot. The only cost is the ReplicatedTable.ComputeDigest change, which the sync
handshake compares fail-closed - a mixed-version pair refuses to sync loudly instead
of diverging silently. Both-nodes-together is already the standing site-pair rule, so
no new operational constraint; recorded in topology-guide.md.

Also fixed: ArtifactDiff.CompareExternalSystem never compared MaxRetries/RetryDelay,
so a bundle import that changed a system's retry budget previewed as "Identical" -
harmless while the values were inert, a live behaviour change now.

No EF migration: the central ExternalSystemDefinitions columns already exist
(InitialSchema). Central UI already had both form fields.

Tests: site storage round-trip + unset-default, site apply path, central artifact
projection, proto round-trip (via the shared sample), management create/preserve,
CLI option parse + alias, and diff reporting. Fail-before verified on the two core
tests (expected 7 got 0 on the read path; expected 9 got 0 on the apply path).

Docs: Component-ExternalSystemGateway, Component-StoreAndForward, Component-CLI,
components/StoreAndForward, components/SiteRuntime, deployment/topology-guide,
CLI README (which was also missing --timeout-seconds entirely).
2026-08-15 12:56:12 -04:00
Joseph Doherty 59b13d317b feat(grpc): T1B.1 — site_command.proto + SiteCommandDtoMapper + round-trip goldens
Phase 1B's contract slice: the wire shape and the canonical translation for the
28 central→site commands that leave ClusterClient. No behaviour changes yet —
SiteCommunicationActor and CentralCommunicationActor are untouched; the
dispatcher refactor (T1B.2) and the central transport seam (T1B.3) consume this.

Protos/site_command.proto (package scadabridge.sitecommand.v1, service
SiteCommandService): six domain RPCs, each with a `oneof` request/reply
envelope. The grouping is what carries deadline policy — every command inside a
group shares a CommunicationOptions timeout class today, so one RPC per group
keeps the deadline choice in one place on the client and one dispatch switch on
the server, while the oneof keeps each command individually typed:
ExecuteLifecycle(6) · ExecuteOpcUa(8) · ExecuteQuery(4) · ExecuteParked(5) ·
ExecuteRoute(4) · TriggerFailover(1). IntegrationCallRequest — the 29th entry on
SiteCommunicationActor's receive table — is deliberately excluded as dead code
(2026-07-22-integration-call-routing-is-dead-code.md).

Contract decisions worth knowing:

- Nullable COLLECTIONS ride in per-collection wrapper messages
  (DeployArtifactsCommand's six artifact lists, CertTrustResult.Certs,
  RouteToCallRequest.Parameters). proto3 repeated/map collapses null into empty,
  and that distinction is live at the site — the same silent-data-loss class the
  transport round-trip guard exposed in PLAN-05 T8. Goldens cover null, empty
  and populated for each.
- Nullable strings use the empty-string-means-null convention already set by
  AuditEventDtoMapper, with ONE exception: RouteToWaitForAttributeRequest's
  TargetValueEncoded, where "wait for the empty string" is a real target, so it
  carries a StringValue wrapper. Both behaviours are asserted, not assumed.
- Nullable enums ride in one-field messages (proto3 enums have no presence and
  no stock wrapper). Enum translation is an explicit switch in both directions —
  never by ordinal — so reordering a C# enum cannot re-map the wire; every wire
  enum reserves 0 for _UNSPECIFIED and decodes to a documented safe default
  rather than faulting a command from a version-skewed peer.
- New LooseValueCodec carries the surviving `object?` members (script params and
  return values, attribute values, tag read/write values) as a type-tagged union
  so a boxed value keeps its runtime CLR type, as it does today under Akka's
  type-preserving JSON serializer. Dates ride as invariant round-trip strings,
  not Timestamp, which would silently normalise away DateTime.Kind and
  DateTimeOffset.Offset. Lists/maps recurse; anything outside the tagged set
  falls back to JSON and is documented as CLR-type-lossy.
- DebugViewSnapshot gets its own full-fidelity alarm/attribute messages rather
  than reusing sitestream's AlarmStateUpdate, which flattens values to display
  strings — right for a live stream, lossy for a snapshot the UI treats as
  authoritative. The encoder omits an AlarmStateChanged.Condition that already
  equals the record's derived default, so computed alarms round-trip exactly
  (record equality compares the nullable backing field, not the property).

Tests are reflection-driven so the coverage cannot drift: the round-trip theory
enumerates the mapper's own ToProto overloads, the envelope guards enumerate the
generated oneof descriptors, and a missing golden fails the build. 216 new tests
green (Communication 532 total, Commons 684 total, solution build 0/0).

Codegen is checked in under SiteCommandGrpc/ per the sitestream recipe; the
<Protobuf> ItemGroup stays commented out (an active one segfaults protoc in the
linux_arm64 Docker image). docker/regen-proto.sh now handles every proto in that
ItemGroup instead of just sitestream, and re-comments idempotently.
2026-07-22 20:04:23 -04:00