From 482165cbaccd21fcb72515dc99a5ca141d921273 Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Sat, 15 Aug 2026 12:43:25 -0400 Subject: [PATCH] fix(siteruntime): startup reconcile starts the healed InstanceActor (register row 67) SiteReconciliationActor's node-STARTUP fetch healed the config FILE but never the RUNTIME. Instance Actors are created from exactly two paths -- the Deployment Manager's boot-time SQLite load and an explicit deploy/enable command -- and the reconcile pass runs AFTER that boot load has already read storage. So a config reconcile restored started no actor: an enabled, successfully-deployed instance whose config was missing at boot (lost volume, first boot after a restore, rehomed data dir) stayed dead until the NEXT restart, whose load finally picked it up. Proven live on site-a 2026-08-15: "Reconcile: fetched + stored config" with no "InstanceActor started" after it, and a debug snapshot answering instanceNotFound. The pass now raises a fire-and-forget EnsureInstanceStartedCommand (new Commons Deployment message) for every ENABLED instance it healed, routed through the deployment-manager singleton PROXY so Instance Actors stay under the singleton no matter which node's reconcile found the gap. The handler is deliberately advisory: - idempotent -- no-op when the actor is already running, which both nodes' reconcile actors legitimately trigger for the same instance; - enabled-state-aware -- it re-reads the durable row, which is the authority on both the config body and enabled/disabled, so a disable landing after the fetch still wins and the instance stays stopped; - it defers to any in-flight deploy/redeploy/warm for the instance, which keeps the deploy gate's ordering guarantees and the all-or-nothing-per-instance rule intact -- this never applies a config, it only starts one already persisted; - a missing row is a quiet no-op, the normal case on an unreplicated site whose peer node healed its own file. The heal is no longer silent: a successful reconcile start emits a Warning log line and an operational `deployment` site event, because reaching it means the node booted without a config for an instance that should have been running. --- .../EnsureInstanceStartedCommand.cs | 29 ++ .../Actors/AkkaHostedService.cs | 8 +- .../Actors/DeploymentManagerActor.cs | 138 +++++++ .../Actors/SiteReconciliationActor.cs | 76 +++- .../Actors/ReconcileStartsInstanceTests.cs | 384 ++++++++++++++++++ 5 files changed, 626 insertions(+), 9 deletions(-) create mode 100644 src/ZB.MOM.WW.ScadaBridge.Commons/Messages/Deployment/EnsureInstanceStartedCommand.cs create mode 100644 tests/ZB.MOM.WW.ScadaBridge.SiteRuntime.Tests/Actors/ReconcileStartsInstanceTests.cs diff --git a/src/ZB.MOM.WW.ScadaBridge.Commons/Messages/Deployment/EnsureInstanceStartedCommand.cs b/src/ZB.MOM.WW.ScadaBridge.Commons/Messages/Deployment/EnsureInstanceStartedCommand.cs new file mode 100644 index 00000000..43973aa3 --- /dev/null +++ b/src/ZB.MOM.WW.ScadaBridge.Commons/Messages/Deployment/EnsureInstanceStartedCommand.cs @@ -0,0 +1,29 @@ +namespace ZB.MOM.WW.ScadaBridge.Commons.Messages.Deployment; + +/// +/// Site-internal request to make sure an already-persisted instance is actually RUNNING. +/// Routes via the deployment-manager singleton proxy so the Instance Actor is only ever +/// created under the singleton, whichever node raised the request. +/// +/// Emitted by SiteReconciliationActor after its node-STARTUP pass fetches and stores +/// a config the node was missing. The reconcile heal path writes the FILE; without this the +/// runtime was left untouched, so an instance whose config was absent at boot (lost volume, +/// first boot after a restore, rehomed data dir) stayed dead until the NEXT restart, whose +/// SQLite load finally started it. +/// +/// +/// Fire-and-forget and idempotent. There is no reply: the handler no-ops when the +/// instance is already running, when an in-flight deploy will create it anyway, or when the +/// durable row says the instance is disabled. The stored row — not this message — is the +/// authority on both the config body and enabled/disabled state, so a disable that landed +/// after the reconcile fetch still wins. +/// +/// +/// Unique name of the instance that should be running. +/// +/// Short human-readable provenance for the request (e.g. "startup reconcile"), carried +/// verbatim into the log line and the operational site event so the heal is not silent. +/// +public record EnsureInstanceStartedCommand( + string InstanceUniqueName, + string Reason); diff --git a/src/ZB.MOM.WW.ScadaBridge.Host/Actors/AkkaHostedService.cs b/src/ZB.MOM.WW.ScadaBridge.Host/Actors/AkkaHostedService.cs index c6e7f9bc..4f4aee45 100644 --- a/src/ZB.MOM.WW.ScadaBridge.Host/Actors/AkkaHostedService.cs +++ b/src/ZB.MOM.WW.ScadaBridge.Host/Actors/AkkaHostedService.cs @@ -1024,6 +1024,11 @@ akka {{ // guarded-writes it (orphans are logged, never deleted). Requires the HTTP // config fetcher; if it is somehow absent the self-heal is skipped (best-effort — // replication remains the primary path and the next restart retries). + // The dmProxy is passed so the pass heals the RUNTIME too (register row 67): each + // enabled instance it restores gets an EnsureInstanceStartedCommand routed to the + // singleton, which starts the Instance Actor if the boot-time load missed it. Going + // through the PROXY (not a local ActorOf) keeps Instance Actors under the singleton + // regardless of which node's reconcile pass found the gap. if (deploymentConfigFetcher != null) { var reconcileLogger = _serviceProvider.GetRequiredService() @@ -1037,7 +1042,8 @@ akka {{ _nodeOptions.NodeName, reconcileLogger, null, - null)), + null, + dmProxy)), "site-reconciliation"); _logger.LogInformation( "SiteReconciliationActor created (per-node startup self-heal) for site {SiteId} node {Node}", diff --git a/src/ZB.MOM.WW.ScadaBridge.SiteRuntime/Actors/DeploymentManagerActor.cs b/src/ZB.MOM.WW.ScadaBridge.SiteRuntime/Actors/DeploymentManagerActor.cs index 8495149f..693ba34c 100644 --- a/src/ZB.MOM.WW.ScadaBridge.SiteRuntime/Actors/DeploymentManagerActor.cs +++ b/src/ZB.MOM.WW.ScadaBridge.SiteRuntime/Actors/DeploymentManagerActor.cs @@ -255,6 +255,13 @@ public class DeploymentManagerActor : ReceiveActor, IWithTimers Receive(HandleRefreshFetched); Receive(HandleRefreshFetchFailed); + // Reconcile self-heal: a config the node was missing at boot has just been + // fetched + stored by SiteReconciliationActor, which heals the FILE only. Start the + // Instance Actor too, so a missing-config boot no longer needs a SECOND restart to + // run. Idempotent and enabled-state-aware; the durable row is the authority. + Receive(HandleEnsureInstanceStarted); + Receive(HandleEnsureInstanceStartedResult); + // Query-the-site-before-redeploy idempotency. // Central asks for the instance's currently-applied deployment identity // before re-sending a deployment whose prior record is stuck InProgress @@ -1283,6 +1290,129 @@ public class DeploymentManagerActor : ReceiveActor, IWithTimers _logger.LogInformation("Instance {Instance} enabled", instanceName); } + /// + /// Reconcile self-heal (deferred register row 67): makes sure an instance whose config was + /// (re)stored out-of-band is actually RUNNING. + /// + /// + /// Why this exists. Instance Actors were created from exactly two paths — the + /// boot-time SQLite load () and an explicit + /// deploy/enable command. SiteReconciliationActor's node-STARTUP fetch runs AFTER + /// that boot load has already read storage, so a config it healed into the file started no + /// actor: the instance stayed dead until the next restart's load finally picked it up. + /// This handler closes that gap without touching the deploy pipeline. + /// + /// + /// + /// Guards. The command is advisory and fire-and-forget, so every reason to do + /// nothing is a silent return: the actor is already running (idempotent — both nodes' + /// reconcile actors legitimately raise this for the same instance), or an in-flight + /// deploy/redeploy owns the instance and will create the actor itself. Deferring to the + /// deploy paths is what keeps the deploy gate's ordering guarantees and the + /// all-or-nothing-per-instance rule intact — this never applies a config, it only starts + /// one that is already fully persisted. + /// + /// + private void HandleEnsureInstanceStarted(EnsureInstanceStartedCommand command) + { + var instanceName = command.InstanceUniqueName; + + if (!ShouldAttemptEnsureStart(instanceName)) + return; + + // Re-read the durable row off the actor thread. The stored row — NOT the requesting + // message — is the authority on the config body AND on enabled/disabled state, so a + // disable that landed after the reconcile fetch still wins and the instance stays + // stopped. + Task.Run(async () => + { + var configs = await _storage.GetAllDeployedConfigsAsync(); + var config = configs.FirstOrDefault(c => c.InstanceUniqueName == instanceName); + return new EnsureInstanceStartedResult(command, config, null); + }).ContinueWith(t => t.IsCompletedSuccessfully + ? t.Result + : new EnsureInstanceStartedResult(command, null, t.Exception?.GetBaseException().Message)) + .PipeTo(Self); + } + + /// + /// Shared guard for the two halves of the ensure-started round trip. Re-evaluated after + /// the storage read because the instance's state can change while that read is in flight + /// (a deploy or delete may have arrived on the mailbox in between). + /// + private bool ShouldAttemptEnsureStart(string instanceName) => + !_instanceActors.ContainsKey(instanceName) && + !_deployWarms.ContainsKey(instanceName) && + !_terminatingActorsByName.ContainsKey(instanceName) && + !_inFlightDeployments.ContainsKey(instanceName); + + /// + /// Actor-thread half of the reconcile self-heal: creates the Instance Actor for a config + /// that is present and enabled in storage but has no running actor, and SURFACES the heal + /// (log + operational site event) so "config stored, actor not running" is no longer + /// silent. + /// + private void HandleEnsureInstanceStartedResult(EnsureInstanceStartedResult result) + { + var instanceName = result.Command.InstanceUniqueName; + var reason = result.Command.Reason; + + if (result.Error != null) + { + _logger.LogWarning( + "Ensure-started ({Reason}) for {Instance} could not read the deployed config: {Error}", + reason, instanceName, result.Error); + return; + } + + // No stored row: nothing to start. Normal when the config was superseded/removed + // between the reconcile fetch and this read, and on an UNREPLICATED site whose peer + // node healed its own file — this singleton's storage legitimately has no such row. + if (result.Config == null) + { + _logger.LogDebug( + "Ensure-started ({Reason}) for {Instance}: no deployed config on this node — nothing to start", + reason, instanceName); + return; + } + + // Disabled instances stay stopped. The heal restores the config row either way; a + // disabled instance having a row but no actor is the correct steady state. + if (!result.Config.IsEnabled) + { + _logger.LogDebug( + "Ensure-started ({Reason}) for {Instance}: instance is disabled — leaving it stopped", + reason, instanceName); + return; + } + + // Re-check the guards: a deploy/delete for this instance may have arrived while the + // storage read was in flight, and it is authoritative over this advisory start. + if (!ShouldAttemptEnsureStart(instanceName)) + { + _logger.LogDebug( + "Ensure-started ({Reason}) for {Instance}: already running or superseded by an in-flight deploy", + reason, instanceName); + return; + } + + CreateInstanceActor(instanceName, result.Config.ConfigJson); + _deployedInstanceNames.Add(instanceName); + UpdateInstanceCounts(); + + // Surface the heal. Warning (not Info) on purpose: reaching here means the node + // booted WITHOUT a config for an enabled, successfully-deployed instance — the + // condition is repaired, but it is abnormal and worth an operator's attention. + _logger.LogWarning( + "Instance {Instance} started by {Reason} self-heal — its config was missing at boot " + + "and has been restored; the instance was NOT running until now", + instanceName, reason); + LogDeploymentEvent("Warning", instanceName, + $"Instance {instanceName} started by {reason} self-heal", + "Config was missing at node boot and was restored by reconciliation; " + + "the Instance Actor was not running and has now been started."); + } + /// /// Deletes an instance: stops the actor and removes config from SQLite. /// Note: store-and-forward messages are NOT cleared per design decision. @@ -2418,6 +2548,14 @@ public class DeploymentManagerActor : ReceiveActor, IWithTimers internal record BatchState(List Configs, int NextIndex); internal record EnableResult( EnableInstanceCommand Command, DeployedInstance? Config, string? Error, IActorRef OriginalSender); + + /// + /// Carries the durable row read for an back onto + /// the actor thread. No original sender: the ensure-started path is fire-and-forget, so + /// there is nobody to answer. + /// + internal record EnsureInstanceStartedResult( + EnsureInstanceStartedCommand Command, DeployedInstance? Config, string? Error); internal record DeployPersistenceResult( string DeploymentId, string InstanceName, bool Success, string? Error, IActorRef OriginalSender, bool IsRedeploy); diff --git a/src/ZB.MOM.WW.ScadaBridge.SiteRuntime/Actors/SiteReconciliationActor.cs b/src/ZB.MOM.WW.ScadaBridge.SiteRuntime/Actors/SiteReconciliationActor.cs index 557569b8..be5827fe 100644 --- a/src/ZB.MOM.WW.ScadaBridge.SiteRuntime/Actors/SiteReconciliationActor.cs +++ b/src/ZB.MOM.WW.ScadaBridge.SiteRuntime/Actors/SiteReconciliationActor.cs @@ -14,7 +14,9 @@ namespace ZB.MOM.WW.ScadaBridge.SiteRuntime.Actors; /// read the node's local deployed inventory from SQLite, /// report it to central via the SiteCommunicationActor Ask /// ( → ), -/// fetch each gap item's config over HTTP and guarded-write it, and +/// fetch each gap item's config over HTTP and guarded-write it, +/// ask the Deployment Manager singleton to START each enabled instance it just +/// healed, and /// LOG (never delete) any orphan the node still has but central no longer deploys. /// /// @@ -36,6 +38,17 @@ namespace ZB.MOM.WW.ScadaBridge.SiteRuntime.Actors; /// The site does NOT carry the central fetch base URL in its own config — it uses /// from central's reply. /// +/// +/// The pass heals the runtime, not just the file (deferred register row 67). Storing a +/// config starts nothing by itself: the Deployment Manager creates Instance Actors from the +/// boot-time SQLite load, which has already run by the time this pass fires. So for every +/// ENABLED instance whose config this pass stored, an +/// is sent to the Deployment Manager singleton, which starts the actor if it is not already +/// running. Without it, an instance whose config was missing at boot (lost volume, first boot +/// after a restore, rehomed data dir) needed a SECOND restart before it ran again. +/// The command is advisory, idempotent, and deliberately fire-and-forget — a site with no +/// Deployment Manager ref configured simply keeps the old file-only behavior. +/// /// public sealed class SiteReconciliationActor : ReceiveActor, IWithTimers { @@ -44,6 +57,7 @@ public sealed class SiteReconciliationActor : ReceiveActor, IWithTimers private readonly SiteStorageService _storage; private readonly IDeploymentConfigFetcher _configFetcher; private readonly IActorRef _siteCommunicationActor; + private readonly IActorRef? _deploymentManager; private readonly string _siteIdentifier; private readonly string _nodeId; private readonly ILogger _logger; @@ -71,6 +85,14 @@ public sealed class SiteReconciliationActor : ReceiveActor, IWithTimers /// Defaults to 5 seconds. /// /// Round-trip timeout for the reconcile Ask to central. Defaults to 30 seconds. + /// + /// The Deployment Manager singleton PROXY. Every enabled instance this pass heals is + /// followed by a fire-and-forget so the runtime + /// heals with the file (register row 67). Routing through the proxy — rather than starting + /// anything locally — is what keeps Instance Actors under the singleton no matter which + /// node's reconcile pass found the gap. Optional: when null the pass stores configs and + /// starts nothing, exactly as it did before. + /// public SiteReconciliationActor( SiteStorageService storage, IDeploymentConfigFetcher configFetcher, @@ -79,11 +101,13 @@ public sealed class SiteReconciliationActor : ReceiveActor, IWithTimers string nodeId, ILogger logger, TimeSpan? initialDelay = null, - TimeSpan? askTimeout = null) + TimeSpan? askTimeout = null, + IActorRef? deploymentManager = null) { _storage = storage; _configFetcher = configFetcher; _siteCommunicationActor = siteCommunicationActor; + _deploymentManager = deploymentManager; _siteIdentifier = siteIdentifier; _nodeId = nodeId; _logger = logger; @@ -153,6 +177,9 @@ public sealed class SiteReconciliationActor : ReceiveActor, IWithTimers var fetched = 0; var failed = 0; + // Enabled instances healed by this pass, to be started via the Deployment Manager once + // the pass reports back on the actor thread (row 67). + var healedEnabled = new List(); // Fetch + guarded-write each gap item. Per-item failure must not abort the rest. foreach (var item in response.Gap) @@ -168,6 +195,12 @@ public sealed class SiteReconciliationActor : ReceiveActor, IWithTimers .ConfigureAwait(false); fetched++; + // Only ENABLED instances are candidates to start; a disabled one correctly + // keeps a config row with no running actor. The Deployment Manager re-checks + // this against the durable row anyway — this is just the cheap filter. + if (item.IsEnabled) + healedEnabled.Add(item.InstanceUniqueName); + _logger.LogInformation( "Reconcile: fetched + stored config for {Instance} (deployment {DeploymentId}, rev {Revision})", item.InstanceUniqueName, item.DeploymentId, item.RevisionHash); @@ -198,7 +231,7 @@ public sealed class SiteReconciliationActor : ReceiveActor, IWithTimers name); } - return ReconcilePassResult.Completed(fetched, failed, response.OrphanNames.Count); + return ReconcilePassResult.Completed(fetched, failed, response.OrphanNames.Count, healedEnabled); } private void HandleReconcilePassResult(ReconcilePassResult result) @@ -216,6 +249,23 @@ public sealed class SiteReconciliationActor : ReceiveActor, IWithTimers _logger.LogInformation( "Reconcile pass for site {Site} node {Node} complete: {Fetched} fetched, {Failed} failed, {Orphans} orphan(s)", _siteIdentifier, _nodeId, result.Fetched, result.Failed, result.Orphans); + + // Heal the RUNTIME, not just the file (row 67). Storing a config starts nothing: the + // Deployment Manager's boot-time SQLite load has already run by now, so without this + // an instance missing at boot would stay dead until the next restart. Fire-and-forget + // through the singleton proxy — the handler is idempotent, re-checks the durable row's + // enabled state, and defers to any in-flight deploy. + if (_deploymentManager == null || result.HealedEnabled.Count == 0) + return; + + foreach (var instanceName in result.HealedEnabled) + { + _logger.LogInformation( + "Reconcile: requesting start of healed instance {Instance} on site {Site}", + instanceName, _siteIdentifier); + _deploymentManager.Tell( + new EnsureInstanceStartedCommand(instanceName, "startup reconcile")); + } } // ── Internal messages ── @@ -227,21 +277,31 @@ public sealed class SiteReconciliationActor : ReceiveActor, IWithTimers private RunReconcile() { } } - /// Summary of one reconcile pass, piped to Self for logging. - private sealed record ReconcilePassResult(int Fetched, int Failed, int Orphans, Exception? Error) + /// + /// Summary of one reconcile pass, piped to Self for logging and for the + /// ensure-started dispatch. is built off the actor thread but + /// only ever read on it, so no synchronization is needed. + /// + private sealed record ReconcilePassResult( + int Fetched, int Failed, int Orphans, IReadOnlyList HealedEnabled, Exception? Error) { /// Creates a result for a reconcile pass that ran to completion. /// Number of instances successfully fetched/reconciled. /// Number of instances that failed to reconcile. /// Number of local instances no longer deployed at central. + /// + /// Names of the ENABLED instances whose config this pass stored — each gets an + /// so the runtime heals with the file. + /// /// A completed with no error. - public static ReconcilePassResult Completed(int fetched, int failed, int orphans) - => new(fetched, failed, orphans, null); + public static ReconcilePassResult Completed( + int fetched, int failed, int orphans, IReadOnlyList healedEnabled) + => new(fetched, failed, orphans, healedEnabled, null); /// Creates a result for a reconcile pass that failed before completing. /// The exception that caused the pass to fail. /// A faulted carrying the error. public static ReconcilePassResult Faulted(Exception error) - => new(0, 0, 0, error); + => new(0, 0, 0, [], error); } } diff --git a/tests/ZB.MOM.WW.ScadaBridge.SiteRuntime.Tests/Actors/ReconcileStartsInstanceTests.cs b/tests/ZB.MOM.WW.ScadaBridge.SiteRuntime.Tests/Actors/ReconcileStartsInstanceTests.cs new file mode 100644 index 00000000..13788bd7 --- /dev/null +++ b/tests/ZB.MOM.WW.ScadaBridge.SiteRuntime.Tests/Actors/ReconcileStartsInstanceTests.cs @@ -0,0 +1,384 @@ +using System.Collections.Concurrent; +using System.Text.Json; +using Akka.Actor; +using Akka.TestKit.Xunit2; +using Microsoft.Extensions.Logging.Abstractions; +using ZB.MOM.WW.ScadaBridge.Commons.Messages.DebugView; +using ZB.MOM.WW.ScadaBridge.Commons.Messages.Deployment; +using ZB.MOM.WW.ScadaBridge.Commons.Messages.Lifecycle; +using ZB.MOM.WW.ScadaBridge.Commons.Types.Enums; +using ZB.MOM.WW.ScadaBridge.Commons.Types.Flattening; +using ZB.MOM.WW.ScadaBridge.SiteRuntime.Actors; +using ZB.MOM.WW.ScadaBridge.SiteRuntime.Deployment; +using ZB.MOM.WW.ScadaBridge.SiteRuntime.Persistence; +using ZB.MOM.WW.ScadaBridge.SiteRuntime.Scripts; +using ZB.MOM.WW.ScadaBridge.SiteRuntime.Tests.TestSupport; +using ZB.MOM.WW.ScadaBridge.TestSupport; + +namespace ZB.MOM.WW.ScadaBridge.SiteRuntime.Tests.Actors; + +/// +/// Regression tests for deferred register row 67 — startup reconcile healed the FILE but not +/// the RUNTIME. +/// +/// +/// Instance Actors are created from exactly two paths: the Deployment Manager's boot-time SQLite +/// load, and an explicit deploy/enable command. 's +/// node-STARTUP fetch runs AFTER that boot load has already read storage, so a config it healed +/// into the file started nothing — the enabled, successfully-deployed instance stayed dead until +/// the NEXT restart, whose load finally picked it up. Proven live on site-a 2026-08-15: +/// "Reconcile: fetched + stored config" with no "InstanceActor started" after it, and a debug +/// snapshot answering instanceNotFound. +/// +/// +/// +/// The fix is an raised by the reconcile pass and +/// handled by the Deployment Manager singleton. These tests pin both halves plus the seam +/// between them. +/// +/// +public class ReconcileStartsInstanceTests : TestKit, IDisposable +{ + private const string SiteIdentifier = "site-1"; + private const string NodeId = "node-a"; + + private readonly SiteStorageService _storage; + private readonly ScriptCompilationService _compilationService; + private readonly SharedScriptLibrary _sharedScriptLibrary; + private readonly TestLocalDb _localDb; + + public ReconcileStartsInstanceTests() + { + _localDb = TestLocalDb.CreateTemp("reconcile-start-test"); + _storage = new SiteStorageService( + _localDb.Db, NullLogger.Instance); + _storage.InitializeAsync().GetAwaiter().GetResult(); + _compilationService = new ScriptCompilationService( + NullLogger.Instance); + _sharedScriptLibrary = new SharedScriptLibrary( + _compilationService, NullLogger.Instance); + } + + void IDisposable.Dispose() + { + // Shut the actors down BEFORE the ILocalDb they write through is disposed. + Shutdown(); + var path = _localDb.Path; + _localDb.Dispose(); + TestLocalDb.DeleteFiles(path); + } + + private IActorRef CreateDeploymentManager(IServiceProvider? serviceProvider = null) => + ActorOf(Props.Create(() => new DeploymentManagerActor( + _storage, + _compilationService, + _sharedScriptLibrary, + null, + new SiteRuntimeOptions(), + NullLogger.Instance, + dclManager: null, + healthCollector: null, + serviceProvider: serviceProvider, + loggerFactory: null, + configFetcher: null))); + + private IActorRef CreateReconciliationActor( + IActorRef siteCommunicationActor, + IDeploymentConfigFetcher fetcher, + IActorRef? deploymentManager = null) => + ActorOf(Props.Create(() => new SiteReconciliationActor( + _storage, + fetcher, + siteCommunicationActor, + SiteIdentifier, + NodeId, + NullLogger.Instance, + TimeSpan.FromMilliseconds(50), // initial delay — fast for tests + TimeSpan.FromSeconds(5), // ask timeout + deploymentManager))); + + private static string MakeConfigJson(string instanceName) + { + var config = new FlattenedConfiguration + { + InstanceUniqueName = instanceName, + Attributes = + [ + new ResolvedAttribute { CanonicalName = "TestAttr", Value = "42", DataType = "Int32" } + ] + }; + return JsonSerializer.Serialize(config); + } + + /// + /// Asks the Deployment Manager for a debug snapshot — the exact probe that answered + /// instanceNotFound on the live rig. InstanceNotFound == false means an + /// Instance Actor is running and routable. + /// + private async Task InstanceIsRunningAsync(IActorRef deploymentManager, string instanceName) + { + var snapshot = await deploymentManager.Ask( + new SubscribeDebugViewRequest(instanceName, Guid.NewGuid().ToString()), + TimeSpan.FromSeconds(5)); + return !snapshot.InstanceNotFound; + } + + // ── The reproduction ── + + /// + /// THE row-67 reproduction, end to end over one shared storage file: the node boots with no + /// config for an enabled instance (so the Deployment Manager's startup load starts nothing), + /// then the startup reconcile pass fetches and stores that config. The instance must be + /// RUNNING afterwards — not merely present in SQLite waiting for a second restart. + /// + /// + /// Fails before the fix at the final assert: the config row is stored, but the debug + /// snapshot still answers InstanceNotFound = true because nothing ever created the + /// Instance Actor. + /// + /// + [Fact] + public async Task StartupReconcile_StoringConfigForEnabledInstance_StartsTheInstanceActor() + { + // Boot the Deployment Manager against EMPTY storage — this is the missing-config boot + // (lost volume / first boot after a restore / rehomed data dir). + var dm = CreateDeploymentManager(); + await Task.Delay(500); + + // Baseline: the instance is genuinely not running. + Assert.False(await InstanceIsRunningAsync(dm, "HealedPump")); + + // Now the startup reconcile pass runs and central reports the instance as a gap. + const string instanceName = "HealedPump"; + var configJson = MakeConfigJson(instanceName); + var fetcher = new FakeConfigFetcher(_ => Task.FromResult(configJson)); + var commProbe = CreateTestProbe(); + + CreateReconciliationActor(commProbe, fetcher, dm); + + commProbe.ExpectMsg(TimeSpan.FromSeconds(5)); + commProbe.Reply(new ReconcileSiteResponse( + [new ReconcileGapItem(instanceName, "dep-healed", "rev-healed", true, "tok-healed")], + [], + "http://central:9000")); + + // The file heals... + await AwaitAssertAsync(async () => + { + var configs = await _storage.GetAllDeployedConfigsAsync(); + Assert.Contains(configs, c => c.InstanceUniqueName == instanceName); + }, TimeSpan.FromSeconds(5)); + + // ...and so must the RUNTIME. This is the assertion that fails pre-fix. + await AwaitAssertAsync(async () => + Assert.True( + await InstanceIsRunningAsync(dm, instanceName), + "Reconcile stored the config but the InstanceActor was never started " + + "— the instance would stay dead until the next restart (register row 67)"), + TimeSpan.FromSeconds(10)); + } + + /// + /// The heal must not be silent: starting a reconcile-healed instance emits an operational + /// deployment site event naming the instance, so an operator can see that a node + /// booted without a config and self-repaired. + /// + [Fact] + public async Task ReconcileStartedInstance_EmitsASiteEvent() + { + var siteLog = new FakeSiteEventLogger(); + var dm = CreateDeploymentManager(new SingleServiceProvider(siteLog)); + await Task.Delay(500); + + const string instanceName = "SurfacedPump"; + await _storage.StoreDeployedConfigAsync( + instanceName, MakeConfigJson(instanceName), "dep-s", "rev-s", true); + + dm.Tell(new EnsureInstanceStartedCommand(instanceName, "startup reconcile")); + + await AwaitAssertAsync(() => + { + Assert.Contains(siteLog.OfType("deployment"), r => + r.InstanceId == instanceName && + r.Source == "DeploymentManagerActor" && + r.Message.Contains("self-heal", StringComparison.OrdinalIgnoreCase)); + return Task.CompletedTask; + }, TimeSpan.FromSeconds(10)); + } + + // ── The reconcile half: which instances get a start request ── + + /// + /// An ENABLED instance healed by the pass produces an ensure-started request to the + /// Deployment Manager proxy. + /// + [Fact] + public void ReconcilePass_RequestsStart_ForHealedEnabledInstance() + { + var fetcher = new FakeConfigFetcher(_ => Task.FromResult("{\"instanceUniqueName\":\"B\"}")); + var commProbe = CreateTestProbe(); + var dmProbe = CreateTestProbe(); + + CreateReconciliationActor(commProbe, fetcher, dmProbe); + + commProbe.ExpectMsg(TimeSpan.FromSeconds(5)); + commProbe.Reply(new ReconcileSiteResponse( + [new ReconcileGapItem("B", "depB", "rev2", true, "tok-b")], + [], + "http://central:9000")); + + var ensure = dmProbe.ExpectMsg(TimeSpan.FromSeconds(10)); + Assert.Equal("B", ensure.InstanceUniqueName); + Assert.Contains("reconcile", ensure.Reason, StringComparison.OrdinalIgnoreCase); + } + + /// + /// A DISABLED instance healed by the pass must NOT be started — a disabled instance + /// legitimately has a config row and no running actor. + /// + [Fact] + public void ReconcilePass_DoesNotRequestStart_ForHealedDisabledInstance() + { + var fetcher = new FakeConfigFetcher(_ => Task.FromResult("{\"instanceUniqueName\":\"D\"}")); + var commProbe = CreateTestProbe(); + var dmProbe = CreateTestProbe(); + + CreateReconciliationActor(commProbe, fetcher, dmProbe); + + commProbe.ExpectMsg(TimeSpan.FromSeconds(5)); + commProbe.Reply(new ReconcileSiteResponse( + [new ReconcileGapItem("D", "depD", "revD", false, "tok-d")], + [], + "http://central:9000")); + + dmProbe.ExpectNoMsg(TimeSpan.FromSeconds(2)); + } + + /// + /// An item that failed to fetch was never stored, so it must not be started either. + /// + [Fact] + public void ReconcilePass_DoesNotRequestStart_ForFailedFetch() + { + var fetcher = new FakeConfigFetcher(_ => + Task.FromException(new DeploymentConfigFetchException("boom", isSuperseded: false))); + var commProbe = CreateTestProbe(); + var dmProbe = CreateTestProbe(); + + CreateReconciliationActor(commProbe, fetcher, dmProbe); + + commProbe.ExpectMsg(TimeSpan.FromSeconds(5)); + commProbe.Reply(new ReconcileSiteResponse( + [new ReconcileGapItem("Bad", "depBad", "revBad", true, "tok-bad")], + [], + "http://central:9000")); + + dmProbe.ExpectNoMsg(TimeSpan.FromSeconds(2)); + } + + // ── The Deployment Manager half: the ensure-started contract ── + + /// + /// The command is idempotent: both site nodes' reconcile actors legitimately raise it for + /// the same instance, and a repeat must not disturb (or replace) the running actor. + /// + [Fact] + public async Task EnsureInstanceStarted_IsIdempotent_WhenAlreadyRunning() + { + var dm = CreateDeploymentManager(); + await Task.Delay(500); + + const string instanceName = "IdempotentPump"; + dm.Tell(new DeployInstanceCommand( + "dep-idem", instanceName, "sha256:abc", + MakeConfigJson(instanceName), "admin", DateTimeOffset.UtcNow)); + ExpectMsg(TimeSpan.FromSeconds(5)); + await Task.Delay(1000); + + var before = await ResolveInstanceChildAsync(dm, instanceName); + + // Two redundant ensure-started requests, as the two nodes would send. + dm.Tell(new EnsureInstanceStartedCommand(instanceName, "startup reconcile")); + dm.Tell(new EnsureInstanceStartedCommand(instanceName, "startup reconcile")); + await Task.Delay(1000); + + // Same actor ref — nothing was restarted or replaced, and it is still routable. + var after = await ResolveInstanceChildAsync(dm, instanceName); + Assert.Equal(before, after); + Assert.True(await InstanceIsRunningAsync(dm, instanceName)); + } + + /// + /// The durable row is the authority on enabled/disabled state: a DISABLED instance stays + /// stopped even if an ensure-started request arrives for it (e.g. a disable landed between + /// the reconcile fetch and the start request). + /// + [Fact] + public async Task EnsureInstanceStarted_DisabledInstance_StaysStopped() + { + var dm = CreateDeploymentManager(); + await Task.Delay(500); + + const string instanceName = "DisabledPump"; + await _storage.StoreDeployedConfigAsync( + instanceName, MakeConfigJson(instanceName), "dep-d", "rev-d", false); + + dm.Tell(new EnsureInstanceStartedCommand(instanceName, "startup reconcile")); + await Task.Delay(1500); + + Assert.False(await InstanceIsRunningAsync(dm, instanceName)); + } + + /// + /// An ensure-started request for an instance with no stored config on THIS node is a quiet + /// no-op — the normal case on an unreplicated site whose peer node healed its own file. + /// The Deployment Manager must not crash or invent an instance. + /// + [Fact] + public async Task EnsureInstanceStarted_UnknownInstance_IsAQuietNoOp() + { + var dm = CreateDeploymentManager(); + await Task.Delay(500); + + dm.Tell(new EnsureInstanceStartedCommand("NeverHeardOf", "startup reconcile")); + await Task.Delay(1500); + + Assert.False(await InstanceIsRunningAsync(dm, "NeverHeardOf")); + + // The manager is still alive and serving other work. + dm.Tell(new DeployInstanceCommand( + "dep-after", "AfterPump", "sha256:abc", + MakeConfigJson("AfterPump"), "admin", DateTimeOffset.UtcNow)); + Assert.Equal( + DeploymentStatus.Success, + ExpectMsg(TimeSpan.FromSeconds(5)).Status); + } + + /// + /// Resolves the Instance Actor child of the Deployment Manager by path — the direct + /// "the InstanceActor exists" check, complementing the debug-snapshot probe. + /// + private async Task ResolveInstanceChildAsync(IActorRef deploymentManager, string instanceName) => + await Sys.ActorSelection(deploymentManager.Path / instanceName) + .ResolveOne(TimeSpan.FromSeconds(5)); + + /// + /// In-test fake : runs a per-deploymentId behavior and + /// records every call thread-safely (the fetch runs on a pool thread). + /// + private sealed class FakeConfigFetcher : IDeploymentConfigFetcher + { + private readonly Func> _behavior; + public ConcurrentQueue<(string BaseUrl, string DeploymentId, string Token)> Calls { get; } = new(); + + public FakeConfigFetcher(Func> behavior) => _behavior = behavior; + + public async Task FetchAsync( + string centralFetchBaseUrl, string deploymentId, string token, CancellationToken ct) + { + Calls.Enqueue((centralFetchBaseUrl, deploymentId, token)); + await Task.Yield(); + return await _behavior(deploymentId); + } + } +}