ScadaBridge/tests/ZB.MOM.WW.ScadaBridge.DeploymentManager.Tests/DeploymentServiceTests.cs

using System.Diagnostics.Metrics;
using Akka.Actor;
using Akka.TestKit.Xunit2;
using Microsoft.Extensions.Logging.Abstractions;
using Microsoft.Extensions.Options;
using NSubstitute;
using ZB.MOM.WW.ScadaBridge.Commons.Entities.Deployment;
using ZB.MOM.WW.ScadaBridge.Commons.Entities.Instances;
using ZB.MOM.WW.ScadaBridge.Commons.Entities.Sites;
using ZB.MOM.WW.ScadaBridge.Commons.Interfaces.Repositories;
using ZB.MOM.WW.ScadaBridge.Commons.Interfaces.Services;
using ZB.MOM.WW.ScadaBridge.Commons.Interfaces.Transport;
using ZB.MOM.WW.ScadaBridge.Commons.Messages.Deployment;
using ZB.MOM.WW.ScadaBridge.Commons.Messages.Lifecycle;
using ZB.MOM.WW.ScadaBridge.Commons.Observability;
using ZB.MOM.WW.ScadaBridge.Commons.Types;
using ZB.MOM.WW.ScadaBridge.Commons.Types.Enums;
using ZB.MOM.WW.ScadaBridge.Commons.Types.Flattening;
using ZB.MOM.WW.ScadaBridge.Communication;
using ZB.MOM.WW.ScadaBridge.TemplateEngine.Flattening;
using ZB.MOM.WW.ScadaBridge.Commons.Types.Templates;

namespace ZB.MOM.WW.ScadaBridge.DeploymentManager.Tests;

/// <summary>
/// WP-1/2/4/5/6/8/16: Tests for central-side DeploymentService.
/// </summary>
public class DeploymentServiceTests : TestKit
{
    private readonly IDeploymentManagerRepository _repo;
    private readonly IFlatteningPipeline _pipeline;
    private readonly CommunicationService _comms;
    private readonly OperationLockManager _lockManager;
    private readonly IAuditService _audit;
    private readonly DeploymentService _service;

    public DeploymentServiceTests()
    {
        _repo = Substitute.For<IDeploymentManagerRepository>();
        _pipeline = Substitute.For<IFlatteningPipeline>();
        _comms = new CommunicationService(
            Options.Create(new CommunicationOptions()),
            NullLogger<CommunicationService>.Instance);
        _lockManager = new OperationLockManager();
        _audit = Substitute.For<IAuditService>();

        var options = Options.Create(new DeploymentManagerOptions
        {
            OperationLockTimeout = TimeSpan.FromSeconds(5)
        });

        var siteRepo = CreateSiteRepoStub();
        _service = new DeploymentService(
            _repo, siteRepo, _pipeline, _comms, _lockManager, _audit,
            new DiffService(),
            new RevisionHashService(),
            new DeploymentStatusNotifier(NullLogger<DeploymentStatusNotifier>.Instance),
            Substitute.For<IStaleInstanceProbe>(),
            options,
            DeployCommOptions(),
            NullLogger<DeploymentService>.Instance);
    }

    /// <summary>
    /// Notify-and-fetch (Task 6): the central HTTP base URL carried in the
    /// <c>RefreshDeploymentCommand</c>. Must be non-empty for the deploy path to
    /// clear the fail-fast configuration guard.
    /// </summary>
    private const string TestFetchBaseUrl = "https://central.test:9000";

    /// <summary>How long a staged <c>PendingDeployment</c> stays valid in these tests.</summary>
    private static readonly TimeSpan TestPendingTtl = TimeSpan.FromMinutes(5);

    /// <summary>
    /// Builds the <see cref="CommunicationOptions"/> the notify-and-fetch deploy
    /// path reads: a non-empty <see cref="CommunicationOptions.CentralFetchBaseUrl"/>
    /// (so the fail-fast guard passes) and a fixed
    /// <see cref="CommunicationOptions.PendingDeploymentTtl"/>. Pass an empty base
    /// URL to exercise the fail-fast path.
    /// </summary>
    private static IOptions<CommunicationOptions> DeployCommOptions(
        string centralFetchBaseUrl = TestFetchBaseUrl) =>
        Options.Create(new CommunicationOptions
        {
            CentralFetchBaseUrl = centralFetchBaseUrl,
            PendingDeploymentTtl = TestPendingTtl
        });

    // ── WP-1: Deployment flow ──

    [Fact]
    public async Task DeployInstanceAsync_InstanceNotFound_ReturnsFailure()
    {
        _repo.GetInstanceByIdAsync(1).Returns((Instance?)null);

        var result = await _service.DeployInstanceAsync(1, "admin");

        Assert.True(result.IsFailure);
        Assert.Contains("not found", result.Error);
    }

    [Fact]
    public async Task DeployInstanceAsync_ValidationFails_ReturnsFailure()
    {
        var instance = new Instance("TestInst") { Id = 1, SiteId = 1, State = InstanceState.NotDeployed };
        _repo.GetInstanceByIdAsync(1).Returns(instance);

        var validationResult = new ValidationResult
        {
            Errors = [ValidationEntry.Error(ValidationCategory.ScriptCompilation, "Compile error")]
        };
        _pipeline.FlattenAndValidateAsync(1, Arg.Any<CancellationToken>())
            .Returns(Result<FlatteningPipelineResult>.Success(
                new FlatteningPipelineResult(new FlattenedConfiguration(), "hash1", validationResult)));

        var result = await _service.DeployInstanceAsync(1, "admin");

        Assert.True(result.IsFailure);
        Assert.Contains("validation failed", result.Error);
    }

    [Fact]
    public async Task DeployInstanceAsync_FlatteningFails_ReturnsFailure()
    {
        var instance = new Instance("TestInst") { Id = 1, SiteId = 1, State = InstanceState.NotDeployed };
        _repo.GetInstanceByIdAsync(1).Returns(instance);

        _pipeline.FlattenAndValidateAsync(1, Arg.Any<CancellationToken>())
            .Returns(Result<FlatteningPipelineResult>.Failure("Template chain empty"));

        var result = await _service.DeployInstanceAsync(1, "admin");

        Assert.True(result.IsFailure);
        Assert.Contains("Validation failed", result.Error);
    }

    // ── DeploymentManager-021: missing Site row -> hard failure, no silent fabrication ──

    [Fact]
    public async Task DeployInstanceAsync_SiteRowMissing_FailsLoudlyInsteadOfSilentlySubstituting()
    {
        // DeploymentManager-021 regression: previously ResolveSiteIdentifierAsync
        // silently returned the numeric siteId rendered as a string when the
        // site row was missing (FK was deleted, race with admin delete, DB
        // inconsistency). That bogus identifier then surfaced downstream as a
        // confusing "unknown site" routing error that hid the real cause.
        //
        // After the fix the resolver throws InvalidOperationException naming
        // the unresolved id; on the deploy path the existing try/catch turns
        // it into a Failed deployment record whose error message reflects the
        // actual problem.
        var instance = new Instance("OrphanInst")
        {
            Id = 99, SiteId = 42, State = InstanceState.NotDeployed
        };
        _repo.GetInstanceByIdAsync(99, Arg.Any<CancellationToken>()).Returns(instance);
        SetupValidPipeline(99, "OrphanInst", "sha256:abc");

        // Build a fresh service whose siteRepo explicitly returns null for the
        // instance's SiteId (the helper above seeds every id, so we shadow it
        // for SiteId=42 only).
        var siteRepo = CreateSiteRepoStub();
        siteRepo.GetSiteByIdAsync(42, Arg.Any<CancellationToken>()).Returns((Site?)null);

        var service = new DeploymentService(
            _repo, siteRepo, _pipeline, _comms, _lockManager, _audit,
            new DiffService(),
            new RevisionHashService(),
            new DeploymentStatusNotifier(NullLogger<DeploymentStatusNotifier>.Instance),
            Substitute.For<IStaleInstanceProbe>(),
            Options.Create(new DeploymentManagerOptions { OperationLockTimeout = TimeSpan.FromSeconds(5) }),
            DeployCommOptions(),
            NullLogger<DeploymentService>.Instance);

        var result = await service.DeployInstanceAsync(99, "admin");

        Assert.True(result.IsFailure);
        // The descriptive message names the unresolved id so the operator sees
        // the actual problem (missing site row), not a downstream routing error.
        Assert.Contains("42", result.Error);
        Assert.Contains("not found", result.Error, StringComparison.OrdinalIgnoreCase);
    }

    // ── DeploymentManager-022: no transient Pending -> single InProgress insert ──

    [Fact]
    public async Task DeployInstanceAsync_NoTransientPendingWrite_RecordCreatedDirectlyInProgress()
    {
        // DeploymentManager-022 regression: previously the deploy path wrote
        // the record as Pending, then immediately updated it to InProgress
        // with no work in between — an extra SaveChangesAsync round-trip, an
        // extra notifier push, and a Pending->InProgress flicker in the
        // CentralUI deployment-status page. After the fix the record is
        // inserted directly in InProgress (one Add + one notify); no
        // intermediate Pending row is ever persisted or notified.
        var instance = new Instance("DirectInProgressInst")
        {
            Id = 200, SiteId = 1, State = InstanceState.NotDeployed
        };
        _repo.GetInstanceByIdAsync(200, Arg.Any<CancellationToken>()).Returns(instance);
        SetupValidPipeline(200, "DirectInProgressInst", "sha256:dp22");

        // The catch path later flips the same record reference to Failed, so
        // snapshot the Status at insert time rather than reading the live
        // reference at assertion time.
        DeploymentStatus? statusAtInsert = null;
        await _repo.AddDeploymentRecordAsync(
            Arg.Do<DeploymentRecord>(r => statusAtInsert = r.Status), Arg.Any<CancellationToken>());

        // The communication actor is unset so the call throws after the insert;
        // we only care about the status the insert was made with.
        await _service.DeployInstanceAsync(200, "admin");

        // The single Add happens with the record already in InProgress.
        Assert.NotNull(statusAtInsert);
        Assert.Equal(DeploymentStatus.InProgress, statusAtInsert!.Value);

        // No Pending update was issued — the resolver never wrote the
        // intermediate Pending row.
        await _repo.DidNotReceive().UpdateDeploymentRecordAsync(
            Arg.Is<DeploymentRecord>(r => r.Status == DeploymentStatus.Pending),
            Arg.Any<CancellationToken>());
    }

    // ── WP-2: Deployment identity ──

    [Fact]
    public async Task DeployInstanceAsync_CreatesUniqueDeploymentId()
    {
        var instance = new Instance("TestInst") { Id = 1, SiteId = 1, State = InstanceState.NotDeployed };
        _repo.GetInstanceByIdAsync(1).Returns(instance);

        // Pipeline succeeds
        var config = new FlattenedConfiguration { InstanceUniqueName = "TestInst" };
        var validResult = ValidationResult.Success();
        _pipeline.FlattenAndValidateAsync(1, Arg.Any<CancellationToken>())
            .Returns(Result<FlatteningPipelineResult>.Success(
                new FlatteningPipelineResult(config, "sha256:abc", validResult)));

        // Capture the deployment record
        DeploymentRecord? captured = null;
        await _repo.AddDeploymentRecordAsync(Arg.Do<DeploymentRecord>(r => captured = r), Arg.Any<CancellationToken>());

        // CommunicationService will throw because actor not set -- this tests the flow up to that point
        try
        {
            await _service.DeployInstanceAsync(1, "admin");
        }
        catch (InvalidOperationException)
        {
            // Expected -- CommunicationService not initialized
        }

        Assert.NotNull(captured);
        Assert.False(string.IsNullOrEmpty(captured!.DeploymentId));
        Assert.Equal(32, captured.DeploymentId.Length); // GUID without hyphens
        Assert.Equal("sha256:abc", captured.RevisionHash);
    }

    // ── WP-4: State transition validation ──

    [Fact]
    public async Task DeployInstanceAsync_EnabledInstance_AllowsDeploy()
    {
        var instance = new Instance("TestInst") { Id = 1, SiteId = 1, State = InstanceState.Enabled };
        _repo.GetInstanceByIdAsync(1).Returns(instance);

        var config = new FlattenedConfiguration { InstanceUniqueName = "TestInst" };
        _pipeline.FlattenAndValidateAsync(1, Arg.Any<CancellationToken>())
            .Returns(Result<FlatteningPipelineResult>.Success(
                new FlatteningPipelineResult(config, "hash", ValidationResult.Success())));

        // Will fail at communication layer, but passes state validation
        try { await _service.DeployInstanceAsync(1, "admin"); } catch (InvalidOperationException) { }

        // If we got past state validation, the deployment record was created
        await _repo.Received().AddDeploymentRecordAsync(Arg.Any<DeploymentRecord>(), Arg.Any<CancellationToken>());
    }

    // ── DeploymentManager-001: unexpected exception must not leave record InProgress ──

    [Fact]
    public async Task DeployInstanceAsync_CommunicationThrowsUnexpectedException_RecordMarkedFailed()
    {
        var instance = new Instance("TestInst") { Id = 1, SiteId = 1, State = InstanceState.NotDeployed };
        _repo.GetInstanceByIdAsync(1).Returns(instance);

        var config = new FlattenedConfiguration { InstanceUniqueName = "TestInst" };
        _pipeline.FlattenAndValidateAsync(1, Arg.Any<CancellationToken>())
            .Returns(Result<FlatteningPipelineResult>.Success(
                new FlatteningPipelineResult(config, "sha256:abc", ValidationResult.Success())));

        // Capture the deployment record so we can inspect its final state.
        DeploymentRecord? captured = null;
        await _repo.AddDeploymentRecordAsync(
            Arg.Do<DeploymentRecord>(r => captured = r), Arg.Any<CancellationToken>());

        // _comms has no actor set, so DeployInstanceAsync throws
        // InvalidOperationException -- a non-timeout, non-cancellation exception.
        var result = await _service.DeployInstanceAsync(1, "admin");

        // The exception must be handled, not escape.
        Assert.True(result.IsFailure);
        Assert.Contains("Deployment failed", result.Error);

        // The record must not be left stuck in InProgress.
        Assert.NotNull(captured);
        Assert.Equal(DeploymentStatus.Failed, captured!.Status);
        Assert.NotNull(captured.ErrorMessage);
        Assert.NotNull(captured.CompletedAt);
    }

    // ── DeploymentManager-002: failure write must not use a cancelled token ──

    [Fact]
    public async Task DeployInstanceAsync_FailureWrite_UsesNonCancellableToken()
    {
        var instance = new Instance("TestInst") { Id = 1, SiteId = 1, State = InstanceState.NotDeployed };
        _repo.GetInstanceByIdAsync(Arg.Any<int>(), Arg.Any<CancellationToken>()).Returns(instance);

        var config = new FlattenedConfiguration { InstanceUniqueName = "TestInst" };
        _pipeline.FlattenAndValidateAsync(Arg.Any<int>(), Arg.Any<CancellationToken>())
            .Returns(Result<FlatteningPipelineResult>.Success(
                new FlatteningPipelineResult(config, "sha256:abc", ValidationResult.Success())));

        DeploymentRecord? captured = null;
        await _repo.AddDeploymentRecordAsync(
            Arg.Do<DeploymentRecord>(r => captured = r), Arg.Any<CancellationToken>());

        // Simulate a repository that rejects already-cancelled tokens (the
        // real EF Core behaviour when the operation token is cancelled). If the
        // catch block passes the operation's cancelled token, the Failed-status
        // write throws and the record stays InProgress -- the exact bug.
        _repo.UpdateDeploymentRecordAsync(
                Arg.Is<DeploymentRecord>(r => r.Status == DeploymentStatus.Failed),
                Arg.Is<CancellationToken>(ct => ct.IsCancellationRequested))
            .Returns<Task>(_ => throw new OperationCanceledException());
        _repo.SaveChangesAsync(Arg.Is<CancellationToken>(ct => ct.IsCancellationRequested))
            .Returns<Task<int>>(_ => throw new OperationCanceledException());

        // The communication call fails (no actor set). The catch block must
        // persist the Failed status with a non-cancellable token, so cleanup
        // succeeds even when the caller's token is cancelled.
        var result = await _service.DeployInstanceAsync(1, "admin");

        Assert.True(result.IsFailure);
        Assert.NotNull(captured);
        Assert.Equal(DeploymentStatus.Failed, captured!.Status);

        // The Failed-status write happened with a non-cancelled token.
        await _repo.Received().UpdateDeploymentRecordAsync(
            Arg.Is<DeploymentRecord>(r => r.Status == DeploymentStatus.Failed),
            Arg.Is<CancellationToken>(ct => !ct.IsCancellationRequested));
    }

    // ── AskTimeoutException: Akka's AskTimeoutException must be classified as a deploy timeout ──

    [Fact]
    public async Task DeployInstanceAsync_AskTimeoutException_ClassifiedAsTimeout()
    {
        // Arrange: first-time deploy with an actor that drops every message, so
        // the RefreshDeploymentAsync Ask never receives a reply and throws
        // AskTimeoutException (which does NOT derive from System.TimeoutException).
        // Prior to the fix this fell through to the generic "Deployment error:"
        // branch instead of the timeout "Communication failure:" branch, which
        // broke the DeploymentManager-006 reconciliation that keys off that prefix.
        var instance = new Instance("AskTimeoutInst") { Id = 99, SiteId = 1, State = InstanceState.NotDeployed };
        _repo.GetInstanceByIdAsync(99, Arg.Any<CancellationToken>()).Returns(instance);
        SetupValidPipeline(99, "AskTimeoutInst", "sha256:target");
        _repo.GetCurrentDeploymentStatusAsync(99, Arg.Any<CancellationToken>())
            .Returns((DeploymentRecord?)null);

        DeploymentRecord? captured = null;
        await _repo.AddDeploymentRecordAsync(
            Arg.Do<DeploymentRecord>(r => captured = r), Arg.Any<CancellationToken>());

        // A 50 ms DeploymentTimeout forces AskTimeoutException immediately
        // when the actor silently drops every message (never replies).
        var droppingActor = Sys.ActorOf(Props.Create<DroppingActor>());
        var comms = new CommunicationService(
            Options.Create(new CommunicationOptions { DeploymentTimeout = TimeSpan.FromMilliseconds(50) }),
            NullLogger<CommunicationService>.Instance);
        comms.SetCommunicationActor(droppingActor);

        var siteRepo = CreateSiteRepoStub();
        var service = new DeploymentService(
            _repo, siteRepo, _pipeline, comms, _lockManager, _audit,
            new DiffService(),
            new RevisionHashService(),
            new DeploymentStatusNotifier(NullLogger<DeploymentStatusNotifier>.Instance),
            Substitute.For<IStaleInstanceProbe>(),
            Options.Create(new DeploymentManagerOptions { OperationLockTimeout = TimeSpan.FromSeconds(5) }),
            DeployCommOptions(),
            NullLogger<DeploymentService>.Instance);

        // Act
        var result = await service.DeployInstanceAsync(99, "admin");

        // Assert: AskTimeoutException must route through the isTimeout branch.
        Assert.True(result.IsFailure);
        // Result error must reflect a timeout, not a generic deploy failure.
        Assert.StartsWith("Deployment timed out:", result.Error);
        Assert.DoesNotContain("Deployment failed:", result.Error);

        // The deployment record must be Failed with the timeout-failure prefix
        // (which the DeploymentManager-006 reconciliation check keys off).
        Assert.NotNull(captured);
        Assert.Equal(DeploymentStatus.Failed, captured!.Status);
        Assert.NotNull(captured.ErrorMessage);
        Assert.StartsWith("Communication failure:", captured.ErrorMessage);
        Assert.DoesNotContain("Deployment error:", captured.ErrorMessage);
    }

    // ── WP-6: Lifecycle commands ──

    [Fact]
    public async Task DisableInstanceAsync_InstanceNotFound_ReturnsFailure()
    {
        _repo.GetInstanceByIdAsync(1).Returns((Instance?)null);

        var result = await _service.DisableInstanceAsync(1, "admin");

        Assert.True(result.IsFailure);
        Assert.Contains("not found", result.Error);
    }

    [Fact]
    public async Task DisableInstanceAsync_WhenDisabled_ReturnsTransitionError()
    {
        var instance = new Instance("TestInst") { Id = 1, SiteId = 1, State = InstanceState.Disabled };
        _repo.GetInstanceByIdAsync(1).Returns(instance);

        var result = await _service.DisableInstanceAsync(1, "admin");

        Assert.True(result.IsFailure);
        Assert.Contains("not allowed", result.Error);
    }

    [Fact]
    public async Task EnableInstanceAsync_WhenEnabled_ReturnsTransitionError()
    {
        var instance = new Instance("TestInst") { Id = 1, SiteId = 1, State = InstanceState.Enabled };
        _repo.GetInstanceByIdAsync(1).Returns(instance);

        var result = await _service.EnableInstanceAsync(1, "admin");

        Assert.True(result.IsFailure);
        Assert.Contains("not allowed", result.Error);
    }

    [Fact]
    public async Task DeleteInstanceAsync_InstanceNotFound_ReturnsFailure()
    {
        _repo.GetInstanceByIdAsync(1).Returns((Instance?)null);

        var result = await _service.DeleteInstanceAsync(1, "admin");

        Assert.True(result.IsFailure);
        Assert.Contains("not found", result.Error);
    }

    [Fact]
    public async Task DeleteInstanceAsync_NotDeployed_RemovesRecordWithoutSiteRoundTrip()
    {
        // #05-T18: a NotDeployed instance has no live actor at any site (Transport-
        // imported instances always land NotDeployed, often against an uncommissioned
        // or unreachable site). Delete must succeed as a central-side record cleanup
        // WITHOUT contacting the site. The default _service's CommunicationService has
        // no actor wired, so any attempted site round-trip throws uncaught -- a Success
        // result therefore proves the site was never contacted (Times.Never, enforced
        // structurally).
        var instance = new Instance("ImportedInst") { Id = 70, SiteId = 1, State = InstanceState.NotDeployed };
        _repo.GetInstanceByIdAsync(70, Arg.Any<CancellationToken>()).Returns(instance);

        var result = await _service.DeleteInstanceAsync(70, "admin");

        Assert.True(result.IsSuccess);
        await _repo.Received(1).DeleteInstanceAsync(70, Arg.Any<CancellationToken>());
    }

    // ── DeploymentManager-004: site-success but central-delete-failure must not escape uncaught ──

    [Fact]
    public async Task DeleteInstanceAsync_SiteSucceeds_CentralDeleteFails_ReturnsDistinctFailure()
    {
        // The site destroys the Instance Actor and removes its config (response
        // Success), but the central record removal throws. The exception must
        // NOT propagate uncaught -- it must be surfaced as a distinct failure so
        // an operator can reconcile the orphaned central record.
        var instance = new Instance("OrphanInst") { Id = 30, SiteId = 1, State = InstanceState.Enabled };
        _repo.GetInstanceByIdAsync(30, Arg.Any<CancellationToken>()).Returns(instance);

        _repo.DeleteInstanceAsync(30, Arg.Any<CancellationToken>())
            .Returns<Task>(_ => throw new InvalidOperationException("db unavailable"));

        var counters = new ReconcileProbeCounters();
        var commActor = Sys.ActorOf(Props.Create(() =>
            new ReconcileProbeActor(counters, siteHash: "sha256:x", failQuery: false)));
        var service = CreateServiceWithCommActor(commActor);

        var result = await service.DeleteInstanceAsync(30, "admin");

        // The failure is surfaced (not thrown) and clearly says the site
        // succeeded but the central record could not be removed.
        Assert.True(result.IsFailure);
        Assert.Contains("site", result.Error, StringComparison.OrdinalIgnoreCase);
        Assert.Contains("central", result.Error, StringComparison.OrdinalIgnoreCase);
    }

    // ── WP-8: Deployment comparison ──

    [Fact]
    public async Task GetDeploymentComparisonAsync_NoSnapshot_ReturnsFailure()
    {
        _repo.GetDeployedSnapshotByInstanceIdAsync(1).Returns((DeployedConfigSnapshot?)null);

        var result = await _service.GetDeploymentComparisonAsync(1);

        Assert.True(result.IsFailure);
        Assert.Contains("No deployed snapshot", result.Error);
    }

    [Fact]
    public async Task GetDeploymentComparisonAsync_SameHash_NotStale()
    {
        // I-1: the deployed-side staleness hash is now recomputed from the
        // (List-normalized) deserialized snapshot, so the snapshot's stored
        // ConfigurationJson and the current re-flatten must describe the SAME
        // config to be not-stale. Use a faithfully-serialized config on both
        // sides; recompute the current hash with the real RevisionHashService so
        // the recomputed deployed hash equals it.
        var deployedConfig = new FlattenedConfiguration
        {
            InstanceUniqueName = "TestInst",
            Attributes = [new ResolvedAttribute { CanonicalName = "Setpoint", Value = "5", DataType = "Int32" }]
        };
        var sameHash = new RevisionHashService().ComputeHash(deployedConfig);
        var snapshot = new DeployedConfigSnapshot(
            "dep1", sameHash, System.Text.Json.JsonSerializer.Serialize(deployedConfig))
        {
            InstanceId = 1,
            DeployedAt = DateTimeOffset.UtcNow
        };
        _repo.GetDeployedSnapshotByInstanceIdAsync(1).Returns(snapshot);

        var currentConfig = deployedConfig with { };
        _pipeline.FlattenAndValidateAsync(1, Arg.Any<CancellationToken>(), Arg.Any<bool>())
            .Returns(Result<FlatteningPipelineResult>.Success(
                new FlatteningPipelineResult(currentConfig, sameHash, ValidationResult.Success())));

        var result = await _service.GetDeploymentComparisonAsync(1);

        Assert.True(result.IsSuccess);
        Assert.False(result.Value.IsStale);
    }

    [Fact]
    public async Task GetDeploymentComparisonAsync_DifferentHash_IsStale()
    {
        // I-1: a genuinely different current config still reports stale. The
        // deployed-side hash is recomputed from the snapshot; the current hash is
        // the (different) re-flatten hash, so the two differ.
        var deployedConfig = new FlattenedConfiguration
        {
            InstanceUniqueName = "TestInst",
            Attributes = [new ResolvedAttribute { CanonicalName = "Setpoint", Value = "5", DataType = "Int32" }]
        };
        var deployedHash = new RevisionHashService().ComputeHash(deployedConfig);
        var snapshot = new DeployedConfigSnapshot(
            "dep1", deployedHash, System.Text.Json.JsonSerializer.Serialize(deployedConfig))
        {
            InstanceId = 1,
            DeployedAt = DateTimeOffset.UtcNow
        };
        _repo.GetDeployedSnapshotByInstanceIdAsync(1).Returns(snapshot);

        // The current re-flatten changed the attribute value.
        var currentConfig = new FlattenedConfiguration
        {
            InstanceUniqueName = "TestInst",
            Attributes = [new ResolvedAttribute { CanonicalName = "Setpoint", Value = "9", DataType = "Int32" }]
        };
        var currentHash = new RevisionHashService().ComputeHash(currentConfig);
        _pipeline.FlattenAndValidateAsync(1, Arg.Any<CancellationToken>(), Arg.Any<bool>())
            .Returns(Result<FlatteningPipelineResult>.Success(
                new FlatteningPipelineResult(currentConfig, currentHash, ValidationResult.Success())));

        var result = await _service.GetDeploymentComparisonAsync(1);

        Assert.True(result.IsSuccess);
        Assert.True(result.Value.IsStale);
    }

    // ── PLAN-05 Task 16: read-only paths skip Roslyn script compilation ──

    [Fact]
    public async Task GetDeploymentComparisonAsync_RequestsValidateScriptsFalse()
    {
        // The comparison page only needs the revision hash for staleness — the
        // expensive Roslyn compile must be skipped (validateScripts: false).
        var deployedConfig = new FlattenedConfiguration
        {
            InstanceUniqueName = "TestInst",
            Attributes = [new ResolvedAttribute { CanonicalName = "Setpoint", Value = "5", DataType = "Int32" }]
        };
        var hash = new RevisionHashService().ComputeHash(deployedConfig);
        var snapshot = new DeployedConfigSnapshot(
            "dep1", hash, System.Text.Json.JsonSerializer.Serialize(deployedConfig))
        {
            InstanceId = 1,
            DeployedAt = DateTimeOffset.UtcNow
        };
        _repo.GetDeployedSnapshotByInstanceIdAsync(1).Returns(snapshot);
        _pipeline.FlattenAndValidateAsync(1, Arg.Any<CancellationToken>(), Arg.Any<bool>())
            .Returns(Result<FlatteningPipelineResult>.Success(
                new FlatteningPipelineResult(deployedConfig with { }, hash, ValidationResult.Success())));

        var result = await _service.GetDeploymentComparisonAsync(1);

        Assert.True(result.IsSuccess);
        await _pipeline.Received(1).FlattenAndValidateAsync(1, Arg.Any<CancellationToken>(), false);
        await _pipeline.DidNotReceive().FlattenAndValidateAsync(1, Arg.Any<CancellationToken>(), true);
    }

    [Fact]
    public async Task DeployInstanceAsync_RequestsValidateScriptsTrue()
    {
        // The deploy gate stays the authoritative compile — validateScripts defaults to true.
        var instance = new Instance("TestInst") { Id = 1, SiteId = 1, State = InstanceState.NotDeployed };
        _repo.GetInstanceByIdAsync(1).Returns(instance);

        var validationResult = new ValidationResult
        {
            Errors = [ValidationEntry.Error(ValidationCategory.ScriptCompilation, "Compile error")]
        };
        _pipeline.FlattenAndValidateAsync(1, Arg.Any<CancellationToken>(), Arg.Any<bool>())
            .Returns(Result<FlatteningPipelineResult>.Success(
                new FlatteningPipelineResult(new FlattenedConfiguration(), "hash1", validationResult)));

        await _service.DeployInstanceAsync(1, "admin");

        await _pipeline.Received().FlattenAndValidateAsync(1, Arg.Any<CancellationToken>(), true);
    }

    [Fact]
    public async Task StaleInstanceProbe_RequestsValidateScriptsFalse()
    {
        // The Transport stale-instance probe also only needs the hash — same skip.
        // The probe memoises per-instance against the graph watermark, so start
        // from a clean memo or a sibling test's entry would satisfy the probe
        // without a flatten at all.
        StaleInstanceProbe.ClearMemos();

        _pipeline.CreateSession().Returns(_ => new FlattenSession(new TemplateGraphWatermark()));
        _pipeline.FlattenAndValidateAsync(5, Arg.Any<CancellationToken>(), Arg.Any<bool>(), Arg.Any<FlattenSession?>())
            .Returns(Result<FlatteningPipelineResult>.Success(
                new FlatteningPipelineResult(new FlattenedConfiguration(), "sha256:probe", ValidationResult.Success())));
        var probe = new StaleInstanceProbe(_pipeline, new TemplateGraphWatermark());

        var hash = await probe.GetCurrentRevisionHashAsync(5);

        Assert.Equal("sha256:probe", hash);
        await _pipeline.Received(1).FlattenAndValidateAsync(5, Arg.Any<CancellationToken>(), false, Arg.Any<FlattenSession?>());
    }

    // ── DeploymentManager-007: comparison must produce a structured diff ──

    [Fact]
    public async Task GetDeploymentComparisonAsync_ProducesStructuredDiff()
    {
        // The deployed snapshot has one attribute; the current template-derived
        // config has a different attribute. The comparison must surface a real
        // Added/Removed diff via the TemplateEngine DiffService, not just a
        // boolean staleness flag.
        var deployedConfig = new FlattenedConfiguration
        {
            InstanceUniqueName = "DiffInst",
            Attributes = [new ResolvedAttribute { CanonicalName = "OldAttr", DataType = "Int" }]
        };
        var snapshot = new DeployedConfigSnapshot(
            "dep1", "sha256:old", System.Text.Json.JsonSerializer.Serialize(deployedConfig))
        {
            InstanceId = 40,
            DeployedAt = DateTimeOffset.UtcNow
        };
        _repo.GetDeployedSnapshotByInstanceIdAsync(40, Arg.Any<CancellationToken>()).Returns(snapshot);

        var currentConfig = new FlattenedConfiguration
        {
            InstanceUniqueName = "DiffInst",
            Attributes = [new ResolvedAttribute { CanonicalName = "NewAttr", DataType = "Int" }]
        };
        _pipeline.FlattenAndValidateAsync(40, Arg.Any<CancellationToken>(), Arg.Any<bool>())
            .Returns(Result<FlatteningPipelineResult>.Success(
                new FlatteningPipelineResult(currentConfig, "sha256:new", ValidationResult.Success())));

        var result = await _service.GetDeploymentComparisonAsync(40);

        Assert.True(result.IsSuccess);
        Assert.True(result.Value.IsStale);

        // A structured diff is present with the added and removed attributes.
        Assert.NotNull(result.Value.Diff);
        Assert.True(result.Value.Diff!.HasChanges);
        Assert.Contains(result.Value.Diff.AttributeChanges,
            c => c.CanonicalName == "NewAttr" && c.ChangeType == DiffChangeType.Added);
        Assert.Contains(result.Value.Diff.AttributeChanges,
            c => c.CanonicalName == "OldAttr" && c.ChangeType == DiffChangeType.Removed);
    }

    // ── I-1 (latent): old-form List snapshot must not show a spurious stale/diff ──

    [Fact]
    public async Task GetDeploymentComparisonAsync_OldFormListValue_NormalizedToNative_NotStaleNoDiff()
    {
        // I-1 regression: a List attribute deployed in the OLD quoted form
        // (e.g. ["10","20","30"]) was frozen into the snapshot with a RevisionHash
        // computed over that old form. The current flatten emits the SAME list in
        // native form ([10,20,30]) with a different hash — so a naive comparison
        // would report a spurious stale flag AND a spurious Changed attribute.
        // After the fix the deserialized snapshot's List value is normalized via
        // AttributeValueCodec Decode→Encode before BOTH the staleness hash and the
        // diff, so the comparison reports equal.
        var hasher = new RevisionHashService();

        var oldFormConfig = new FlattenedConfiguration
        {
            InstanceUniqueName = "ListInst",
            Attributes =
            [
                new ResolvedAttribute
                {
                    CanonicalName = "Thresholds",
                    DataType = "List",
                    ElementDataType = "Int32",
                    Value = "[\"10\",\"20\",\"30\"]" // OLD quoted form
                }
            ]
        };
        // The frozen hash reflects the OLD form — it differs from the native hash,
        // which is exactly what would have produced the spurious stale flag.
        var frozenOldFormHash = hasher.ComputeHash(oldFormConfig);
        var snapshot = new DeployedConfigSnapshot(
            "dep1", frozenOldFormHash, System.Text.Json.JsonSerializer.Serialize(oldFormConfig))
        {
            InstanceId = 50,
            DeployedAt = DateTimeOffset.UtcNow
        };
        _repo.GetDeployedSnapshotByInstanceIdAsync(50, Arg.Any<CancellationToken>()).Returns(snapshot);

        // The current re-flatten carries the SAME list in native form.
        var nativeConfig = new FlattenedConfiguration
        {
            InstanceUniqueName = "ListInst",
            Attributes =
            [
                new ResolvedAttribute
                {
                    CanonicalName = "Thresholds",
                    DataType = "List",
                    ElementDataType = "Int32",
                    Value = "[10,20,30]" // native form
                }
            ]
        };
        var nativeHash = hasher.ComputeHash(nativeConfig);
        _pipeline.FlattenAndValidateAsync(50, Arg.Any<CancellationToken>(), Arg.Any<bool>())
            .Returns(Result<FlatteningPipelineResult>.Success(
                new FlatteningPipelineResult(nativeConfig, nativeHash, ValidationResult.Success())));

        // Sanity: the OLD-form frozen hash genuinely differs from the native hash —
        // without normalization the comparison WOULD be spuriously stale.
        Assert.NotEqual(frozenOldFormHash, nativeHash);

        var result = await _service.GetDeploymentComparisonAsync(50);

        Assert.True(result.IsSuccess);
        // Normalization collapses old-form → native: not stale, no attribute diff.
        Assert.False(result.Value.IsStale);
        Assert.NotNull(result.Value.Diff);
        Assert.DoesNotContain(result.Value.Diff!.AttributeChanges,
            c => c.CanonicalName == "Thresholds");
        Assert.False(result.Value.Diff.HasChanges);
        // The recomputed deployed hash equals the native current hash.
        Assert.Equal(nativeHash, result.Value.DeployedRevisionHash);
    }

    [Fact]
    public async Task GetDeploymentComparisonAsync_GenuinelyChangedListValue_StillStaleAndDiffs()
    {
        // I-1 negative: a List value that genuinely changed (not just a form
        // difference) must still report stale and a Changed attribute. The
        // deployed snapshot is native [10,20,30]; the current re-flatten is
        // [10,20,40] — normalization does not mask a real change.
        var hasher = new RevisionHashService();

        var deployedConfig = new FlattenedConfiguration
        {
            InstanceUniqueName = "ListInst",
            Attributes =
            [
                new ResolvedAttribute
                {
                    CanonicalName = "Thresholds",
                    DataType = "List",
                    ElementDataType = "Int32",
                    Value = "[10,20,30]"
                }
            ]
        };
        var deployedHash = hasher.ComputeHash(deployedConfig);
        var snapshot = new DeployedConfigSnapshot(
            "dep1", deployedHash, System.Text.Json.JsonSerializer.Serialize(deployedConfig))
        {
            InstanceId = 51,
            DeployedAt = DateTimeOffset.UtcNow
        };
        _repo.GetDeployedSnapshotByInstanceIdAsync(51, Arg.Any<CancellationToken>()).Returns(snapshot);

        var currentConfig = new FlattenedConfiguration
        {
            InstanceUniqueName = "ListInst",
            Attributes =
            [
                new ResolvedAttribute
                {
                    CanonicalName = "Thresholds",
                    DataType = "List",
                    ElementDataType = "Int32",
                    Value = "[10,20,40]" // genuinely different element
                }
            ]
        };
        var currentHash = hasher.ComputeHash(currentConfig);
        _pipeline.FlattenAndValidateAsync(51, Arg.Any<CancellationToken>(), Arg.Any<bool>())
            .Returns(Result<FlatteningPipelineResult>.Success(
                new FlatteningPipelineResult(currentConfig, currentHash, ValidationResult.Success())));

        var result = await _service.GetDeploymentComparisonAsync(51);

        Assert.True(result.IsSuccess);
        Assert.True(result.Value.IsStale);
        Assert.NotNull(result.Value.Diff);
        Assert.Contains(result.Value.Diff!.AttributeChanges,
            c => c.CanonicalName == "Thresholds" && c.ChangeType == DiffChangeType.Changed);
    }

    // ── WP-2: GetDeploymentStatusAsync ──

    [Fact]
    public async Task GetDeploymentStatusAsync_ReturnsRecordByDeploymentId()
    {
        var record = new DeploymentRecord("dep1", "admin")
        {
            Status = DeploymentStatus.Success
        };
        _repo.GetDeploymentByDeploymentIdAsync("dep1").Returns(record);

        var result = await _service.GetDeploymentStatusAsync("dep1");

        Assert.NotNull(result);
        Assert.Equal("dep1", result!.DeploymentId);
        Assert.Equal(DeploymentStatus.Success, result.Status);
    }

    // ── Audit logging ──

    [Fact]
    public async Task DeployInstanceAsync_FlatteningFails_DoesNotReachAudit()
    {
        // DeploymentManager-011: this test previously asserted nothing. A
        // flatten failure returns before any site communication, so no audit
        // entry is written.
        var instance = new Instance("TestInst") { Id = 1, SiteId = 1, State = InstanceState.NotDeployed };
        _repo.GetInstanceByIdAsync(1).Returns(instance);

        _pipeline.FlattenAndValidateAsync(1, Arg.Any<CancellationToken>())
            .Returns(Result<FlatteningPipelineResult>.Failure("Error"));

        await _service.DeployInstanceAsync(1, "admin");

        await _audit.DidNotReceive().LogAsync(
            Arg.Any<string>(), Arg.Any<string>(), Arg.Any<string>(), Arg.Any<string>(),
            Arg.Any<string>(), Arg.Any<object>(), Arg.Any<CancellationToken>());
    }

    [Fact]
    public async Task DeployInstanceAsync_SiteSucceeds_WritesDeployAuditEntry()
    {
        // DeploymentManager-011: a successful deployment must write a "Deploy"
        // audit entry referencing the deployed instance.
        var instance = new Instance("AuditInst") { Id = 50, SiteId = 1, State = InstanceState.NotDeployed };
        _repo.GetInstanceByIdAsync(50, Arg.Any<CancellationToken>()).Returns(instance);
        SetupValidPipeline(50, "AuditInst", "sha256:target");
        _repo.GetCurrentDeploymentStatusAsync(50, Arg.Any<CancellationToken>())
            .Returns((DeploymentRecord?)null);

        var counters = new ReconcileProbeCounters();
        var commActor = Sys.ActorOf(Props.Create(() =>
            new ReconcileProbeActor(counters, siteHash: "sha256:target", failQuery: false)));
        var service = CreateServiceWithCommActor(commActor);

        var result = await service.DeployInstanceAsync(50, "admin");

        Assert.True(result.IsSuccess);
        await _audit.Received().LogAsync(
            "admin", "Deploy", "Instance", "50", "AuditInst",
            Arg.Any<object>(), Arg.Any<CancellationToken>());
    }

    [Fact]
    public async Task DeployInstanceAsync_DeployAuditWriteThrows_KeepsCommittedSuccess()
    {
        // #05-T17: the post-success "Deploy" audit write is best-effort. If it
        // throws, the already-committed Success record must NOT be flipped to
        // Failed by the outer catch — "audit-write failure NEVER aborts the
        // user-facing action".
        var instance = new Instance("AuditFaultInst") { Id = 52, SiteId = 1, State = InstanceState.NotDeployed };
        _repo.GetInstanceByIdAsync(52, Arg.Any<CancellationToken>()).Returns(instance);
        SetupValidPipeline(52, "AuditFaultInst", "sha256:target");
        _repo.GetCurrentDeploymentStatusAsync(52, Arg.Any<CancellationToken>())
            .Returns((DeploymentRecord?)null);

        // The Deploy audit LogAsync faults.
        _audit.When(a => a.LogAsync(
                Arg.Any<string>(), "Deploy", Arg.Any<string>(), Arg.Any<string>(),
                Arg.Any<string>(), Arg.Any<object>(), Arg.Any<CancellationToken>()))
            .Do(_ => throw new InvalidOperationException("audit pipeline down"));

        var counters = new ReconcileProbeCounters();
        var commActor = Sys.ActorOf(Props.Create(() =>
            new ReconcileProbeActor(counters, siteHash: "sha256:target", failQuery: false)));
        var service = CreateServiceWithCommActor(commActor);

        var result = await service.DeployInstanceAsync(52, "admin");

        Assert.True(result.IsSuccess);
        Assert.Equal(DeploymentStatus.Success, result.Value.Status);
    }

    // ── Telemetry follow-on: scadabridge.deployments.applied on deploy success ──

    [Fact]
    public async Task DeployInstanceAsync_SiteSucceeds_EmitsDeploymentsAppliedCounterOnce()
    {
        // A successful deployment must increment the
        // scadabridge.deployments.applied counter exactly once — one
        // DeployInstanceAsync deploys one instance to one site, so the unit is
        // one increment per successful deploy operation.
        var instance = new Instance("MetricInst") { Id = 55, SiteId = 1, State = InstanceState.NotDeployed };
        _repo.GetInstanceByIdAsync(55, Arg.Any<CancellationToken>()).Returns(instance);
        SetupValidPipeline(55, "MetricInst", "sha256:target");
        _repo.GetCurrentDeploymentStatusAsync(55, Arg.Any<CancellationToken>())
            .Returns((DeploymentRecord?)null);

        var counters = new ReconcileProbeCounters();
        var commActor = Sys.ActorOf(Props.Create(() =>
            new ReconcileProbeActor(counters, siteHash: "sha256:target", failQuery: false)));
        var service = CreateServiceWithCommActor(commActor);

        long applied = 0;
        using var listener = new MeterListener
        {
            InstrumentPublished = (instrument, l) =>
            {
                if (instrument.Meter.Name == ScadaBridgeTelemetry.MeterName
                    && instrument.Name == "scadabridge.deployments.applied")
                {
                    l.EnableMeasurementEvents(instrument);
                }
            }
        };
        listener.SetMeasurementEventCallback<long>((_, measurement, _, _) =>
            Interlocked.Add(ref applied, measurement));
        listener.Start();

        var result = await service.DeployInstanceAsync(55, "admin");

        listener.Dispose();

        Assert.True(result.IsSuccess);
        // Fresh first-time deploy applied -> exactly one increment.
        Assert.Equal(1, Interlocked.Read(ref applied));
    }

    [Fact]
    public async Task DeployInstanceAsync_Reconciled_DoesNotEmitDeploymentsAppliedCounter()
    {
        // The reconciliation path recovers a PRIOR timed-out apply rather than
        // performing a fresh one; counting it would risk double-counting the
        // original apply, so scadabridge.deployments.applied must NOT increment
        // on a reconciled (no re-deploy) success.
        var instance = new Instance("MetricReconcileInst")
        {
            Id = 56, SiteId = 1, State = InstanceState.NotDeployed
        };
        _repo.GetInstanceByIdAsync(56, Arg.Any<CancellationToken>()).Returns(instance);
        SetupValidPipeline(56, "MetricReconcileInst", "sha256:target");

        var prior = new DeploymentRecord("dep-prior-56", "admin")
        {
            InstanceId = 56,
            Status = DeploymentStatus.InProgress,
            RevisionHash = "sha256:target"
        };
        _repo.GetCurrentDeploymentStatusAsync(56, Arg.Any<CancellationToken>()).Returns(prior);
        _repo.GetDeployedSnapshotByInstanceIdAsync(56, Arg.Any<CancellationToken>())
            .Returns((DeployedConfigSnapshot?)null);

        var counters = new ReconcileProbeCounters();
        var commActor = Sys.ActorOf(Props.Create(() =>
            new ReconcileProbeActor(counters, siteHash: "sha256:target", failQuery: false)));
        var service = CreateServiceWithCommActor(commActor);

        long applied = 0;
        using var listener = new MeterListener
        {
            InstrumentPublished = (instrument, l) =>
            {
                if (instrument.Meter.Name == ScadaBridgeTelemetry.MeterName
                    && instrument.Name == "scadabridge.deployments.applied")
                {
                    l.EnableMeasurementEvents(instrument);
                }
            }
        };
        listener.SetMeasurementEventCallback<long>((_, measurement, _, _) =>
            Interlocked.Add(ref applied, measurement));
        listener.Start();

        var result = await service.DeployInstanceAsync(56, "admin");

        listener.Dispose();

        Assert.True(result.IsSuccess);
        // Reconciled — no fresh deploy was sent, so no increment.
        Assert.Equal(0, counters.DeployCount);
        Assert.Equal(0, Interlocked.Read(ref applied));
    }

    // ── DeploymentManager-011: lifecycle success paths ──

    [Fact]
    public async Task DisableInstanceAsync_SiteSucceeds_SetsDisabledStateAndAudits()
    {
        var instance = new Instance("DisInst") { Id = 51, SiteId = 1, State = InstanceState.Enabled };
        _repo.GetInstanceByIdAsync(51, Arg.Any<CancellationToken>()).Returns(instance);

        var counters = new ReconcileProbeCounters();
        var commActor = Sys.ActorOf(Props.Create(() =>
            new ReconcileProbeActor(counters, siteHash: "x", failQuery: false)));
        var service = CreateServiceWithCommActor(commActor);

        var result = await service.DisableInstanceAsync(51, "admin");

        Assert.True(result.IsSuccess);
        Assert.Equal(InstanceState.Disabled, instance.State);
        await _repo.Received().UpdateInstanceAsync(instance, Arg.Any<CancellationToken>());
        await _audit.Received().LogAsync(
            "admin", "Disable", "Instance", "51", "DisInst",
            Arg.Any<object>(), Arg.Any<CancellationToken>());
    }

    [Fact]
    public async Task EnableInstanceAsync_SiteSucceeds_SetsEnabledStateAndAudits()
    {
        var instance = new Instance("EnInst") { Id = 52, SiteId = 1, State = InstanceState.Disabled };
        _repo.GetInstanceByIdAsync(52, Arg.Any<CancellationToken>()).Returns(instance);

        var counters = new ReconcileProbeCounters();
        var commActor = Sys.ActorOf(Props.Create(() =>
            new ReconcileProbeActor(counters, siteHash: "x", failQuery: false)));
        var service = CreateServiceWithCommActor(commActor);

        var result = await service.EnableInstanceAsync(52, "admin");

        Assert.True(result.IsSuccess);
        Assert.Equal(InstanceState.Enabled, instance.State);
        await _repo.Received().UpdateInstanceAsync(instance, Arg.Any<CancellationToken>());
        await _audit.Received().LogAsync(
            "admin", "Enable", "Instance", "52", "EnInst",
            Arg.Any<object>(), Arg.Any<CancellationToken>());
    }

    [Fact]
    public async Task DeleteInstanceAsync_SiteSucceeds_RemovesRecordAndAudits()
    {
        var instance = new Instance("DelInst") { Id = 53, SiteId = 1, State = InstanceState.Enabled };
        _repo.GetInstanceByIdAsync(53, Arg.Any<CancellationToken>()).Returns(instance);

        var counters = new ReconcileProbeCounters();
        var commActor = Sys.ActorOf(Props.Create(() =>
            new ReconcileProbeActor(counters, siteHash: "x", failQuery: false)));
        var service = CreateServiceWithCommActor(commActor);

        var result = await service.DeleteInstanceAsync(53, "admin");

        Assert.True(result.IsSuccess);
        await _repo.Received().DeleteInstanceAsync(53, Arg.Any<CancellationToken>());
        await _audit.Received().LogAsync(
            "admin", "Delete", "Instance", "53", "DelInst",
            Arg.Any<object>(), Arg.Any<CancellationToken>());
    }

    [Fact]
    public async Task DeployInstanceAsync_SameInstance_OperationLockSerializesConcurrentDeploys()
    {
        // DeploymentManager-011: two concurrent deploys of the SAME instance
        // must be serialized by the per-instance operation lock — the site sees
        // them one at a time, never overlapping.
        var instance = new Instance("LockInst") { Id = 54, SiteId = 1, State = InstanceState.Enabled };
        _repo.GetInstanceByIdAsync(54, Arg.Any<CancellationToken>()).Returns(instance);
        SetupValidPipeline(54, "LockInst", "sha256:target");
        _repo.GetCurrentDeploymentStatusAsync(54, Arg.Any<CancellationToken>())
            .Returns((DeploymentRecord?)null);

        var serializationCounters = new SerializationProbeCounters();
        var commActor = Sys.ActorOf(Props.Create(() =>
            new SerializationProbeActor(serializationCounters)));
        var service = CreateServiceWithCommActor(commActor);

        var deploy1 = service.DeployInstanceAsync(54, "admin");
        var deploy2 = service.DeployInstanceAsync(54, "admin");
        var results = await Task.WhenAll(deploy1, deploy2);

        Assert.True(results[0].IsSuccess);
        Assert.True(results[1].IsSuccess);
        // The probe records the maximum concurrency observed; the lock must
        // keep it at 1 for a single instance.
        Assert.Equal(1, serializationCounters.MaxConcurrent);
    }

    // ── DeploymentManager-006: query-the-site-before-redeploy idempotency ──

    /// <summary>
    /// Builds a DeploymentService whose CommunicationService is backed by the
    /// supplied actor, so the site query and deploy notifies can be observed.
    /// <paramref name="centralFetchBaseUrl"/> seeds the DeploymentService's own
    /// <see cref="CommunicationOptions.CentralFetchBaseUrl"/>; pass "" to drive
    /// the notify-and-fetch fail-fast guard.
    /// </summary>
    private DeploymentService CreateServiceWithCommActor(
        IActorRef commActor, string centralFetchBaseUrl = TestFetchBaseUrl)
    {
        var comms = new CommunicationService(
            Options.Create(new CommunicationOptions
            {
                QueryTimeout = TimeSpan.FromSeconds(5),
                DeploymentTimeout = TimeSpan.FromSeconds(5)
            }),
            NullLogger<CommunicationService>.Instance);
        comms.SetCommunicationActor(commActor);

        var siteRepo = CreateSiteRepoStub();
        return new DeploymentService(
            _repo, siteRepo, _pipeline, comms, _lockManager, _audit,
            new DiffService(),
            new RevisionHashService(),
            new DeploymentStatusNotifier(NullLogger<DeploymentStatusNotifier>.Instance),
            Substitute.For<IStaleInstanceProbe>(),
            Options.Create(new DeploymentManagerOptions { OperationLockTimeout = TimeSpan.FromSeconds(5) }),
            DeployCommOptions(centralFetchBaseUrl),
            NullLogger<DeploymentService>.Instance);
    }

    private void SetupValidPipeline(int instanceId, string instanceName, string revisionHash)
    {
        var config = new FlattenedConfiguration { InstanceUniqueName = instanceName };
        _pipeline.FlattenAndValidateAsync(instanceId, Arg.Any<CancellationToken>())
            .Returns(Result<FlatteningPipelineResult>.Success(
                new FlatteningPipelineResult(config, revisionHash, ValidationResult.Success())));
    }

    /// <summary>
    /// DeploymentManager-021 test helper: returns an <see cref="ISiteRepository"/>
    /// substitute that resolves <see cref="ISiteRepository.GetSiteByIdAsync"/>
    /// for ANY integer id to a stub <see cref="Site"/> whose
    /// <c>SiteIdentifier</c> is <c>"site-{id}"</c>. Prior to the
    /// DeploymentManager-021 fix the production `ResolveSiteIdentifierAsync`
    /// silently substituted the numeric id when the site row was missing, so
    /// these tests passed without seeding any Sites. After the fix a missing
    /// site throws — every test that drives a deploy/lifecycle path needs a
    /// real-shaped <see cref="Site"/> back, and this helper centralises that
    /// arrangement so individual tests don't repeat the boilerplate.
    /// </summary>
    private static ISiteRepository CreateSiteRepoStub()
    {
        var siteRepo = Substitute.For<ISiteRepository>();
        siteRepo.GetSiteByIdAsync(Arg.Any<int>(), Arg.Any<CancellationToken>())
            .Returns(callInfo =>
            {
                var id = callInfo.ArgAt<int>(0);
                return new Site($"Test Site {id}", $"site-{id}") { Id = id };
            });
        return siteRepo;
    }

    [Fact]
    public async Task DeployInstanceAsync_PriorInProgressRecord_SiteHasTargetHash_MarksSuccessWithoutRedeploy()
    {
        // Prior record stuck InProgress -> site is queried. The site reports it
        // already has the TARGET revision hash, so the prior record is marked
        // Success and NO new DeployInstanceCommand is sent.
        var instance = new Instance("RedeployInst") { Id = 7, SiteId = 1, State = InstanceState.Enabled };
        _repo.GetInstanceByIdAsync(7, Arg.Any<CancellationToken>()).Returns(instance);
        SetupValidPipeline(7, "RedeployInst", "sha256:target");

        var prior = new DeploymentRecord("dep-prior", "admin")
        {
            InstanceId = 7,
            Status = DeploymentStatus.InProgress,
            RevisionHash = "sha256:target"
        };
        _repo.GetCurrentDeploymentStatusAsync(7, Arg.Any<CancellationToken>()).Returns(prior);

        var counters = new ReconcileProbeCounters();
        var commActor = Sys.ActorOf(Props.Create(() =>
            new ReconcileProbeActor(counters, siteHash: "sha256:target", failQuery: false)));
        var service = CreateServiceWithCommActor(commActor);

        var result = await service.DeployInstanceAsync(7, "admin");

        Assert.True(result.IsSuccess);
        Assert.Equal(DeploymentStatus.Success, prior.Status);
        // The site query was issued, but no new deploy command was sent.
        Assert.Equal(1, counters.QueryCount);
        Assert.Equal(0, counters.DeployCount);
        // No new deployment record was created — the prior one was reconciled.
        await _repo.DidNotReceive().AddDeploymentRecordAsync(
            Arg.Any<DeploymentRecord>(), Arg.Any<CancellationToken>());
    }

    [Fact]
    public async Task DeployInstanceAsync_PriorInProgressRecord_SiteHasDifferentHash_ProceedsWithDeploy()
    {
        // Prior record stuck InProgress -> site is queried. The site has a
        // DIFFERENT revision hash, so the normal deploy proceeds.
        var instance = new Instance("RedeployInst2") { Id = 8, SiteId = 1, State = InstanceState.Enabled };
        _repo.GetInstanceByIdAsync(8, Arg.Any<CancellationToken>()).Returns(instance);
        SetupValidPipeline(8, "RedeployInst2", "sha256:target");

        var prior = new DeploymentRecord("dep-prior2", "admin")
        {
            InstanceId = 8,
            Status = DeploymentStatus.InProgress,
            RevisionHash = "sha256:old"
        };
        _repo.GetCurrentDeploymentStatusAsync(8, Arg.Any<CancellationToken>()).Returns(prior);

        var counters = new ReconcileProbeCounters();
        var commActor = Sys.ActorOf(Props.Create(() =>
            new ReconcileProbeActor(counters, siteHash: "sha256:old", failQuery: false)));
        var service = CreateServiceWithCommActor(commActor);

        var result = await service.DeployInstanceAsync(8, "admin");

        Assert.True(result.IsSuccess);
        Assert.Equal(1, counters.QueryCount);
        // The normal deploy proceeded — a new command was sent.
        Assert.Equal(1, counters.DeployCount);
        await _repo.Received().AddDeploymentRecordAsync(
            Arg.Any<DeploymentRecord>(), Arg.Any<CancellationToken>());
    }

    [Fact]
    public async Task DeployInstanceAsync_PriorFailedTimeoutRecord_QueriesSite()
    {
        // A prior record Failed due to a timeout also triggers the site query.
        var instance = new Instance("TimedOutInst") { Id = 9, SiteId = 1, State = InstanceState.Enabled };
        _repo.GetInstanceByIdAsync(9, Arg.Any<CancellationToken>()).Returns(instance);
        SetupValidPipeline(9, "TimedOutInst", "sha256:target");

        var prior = new DeploymentRecord("dep-prior3", "admin")
        {
            InstanceId = 9,
            Status = DeploymentStatus.Failed,
            RevisionHash = "sha256:target",
            ErrorMessage = "Communication failure: deployment Ask timed out"
        };
        _repo.GetCurrentDeploymentStatusAsync(9, Arg.Any<CancellationToken>()).Returns(prior);

        var counters = new ReconcileProbeCounters();
        var commActor = Sys.ActorOf(Props.Create(() =>
            new ReconcileProbeActor(counters, siteHash: "sha256:target", failQuery: false)));
        var service = CreateServiceWithCommActor(commActor);

        var result = await service.DeployInstanceAsync(9, "admin");

        Assert.True(result.IsSuccess);
        Assert.Equal(1, counters.QueryCount);
        Assert.Equal(0, counters.DeployCount);
        Assert.Equal(DeploymentStatus.Success, prior.Status);
    }

    [Fact]
    public async Task DeployInstanceAsync_PriorSuccessRecord_SkipsSiteQuery()
    {
        // A clean prior Success record must NOT trigger the extra round-trip.
        var instance = new Instance("CleanInst") { Id = 10, SiteId = 1, State = InstanceState.Enabled };
        _repo.GetInstanceByIdAsync(10, Arg.Any<CancellationToken>()).Returns(instance);
        SetupValidPipeline(10, "CleanInst", "sha256:target");

        var prior = new DeploymentRecord("dep-clean", "admin")
        {
            InstanceId = 10,
            Status = DeploymentStatus.Success,
            RevisionHash = "sha256:old"
        };
        _repo.GetCurrentDeploymentStatusAsync(10, Arg.Any<CancellationToken>()).Returns(prior);

        var counters = new ReconcileProbeCounters();
        var commActor = Sys.ActorOf(Props.Create(() =>
            new ReconcileProbeActor(counters, siteHash: "sha256:target", failQuery: false)));
        var service = CreateServiceWithCommActor(commActor);

        var result = await service.DeployInstanceAsync(10, "admin");

        Assert.True(result.IsSuccess);
        // No site query — the prior deploy completed cleanly.
        Assert.Equal(0, counters.QueryCount);
        Assert.Equal(1, counters.DeployCount);
    }

    [Fact]
    public async Task DeployInstanceAsync_FreshFirstTimeDeploy_SkipsSiteQuery()
    {
        // No prior record at all -> fresh deploy, no extra round-trip.
        var instance = new Instance("FreshInst") { Id = 11, SiteId = 1, State = InstanceState.NotDeployed };
        _repo.GetInstanceByIdAsync(11, Arg.Any<CancellationToken>()).Returns(instance);
        SetupValidPipeline(11, "FreshInst", "sha256:target");
        _repo.GetCurrentDeploymentStatusAsync(11, Arg.Any<CancellationToken>())
            .Returns((DeploymentRecord?)null);

        var counters = new ReconcileProbeCounters();
        var commActor = Sys.ActorOf(Props.Create(() =>
            new ReconcileProbeActor(counters, siteHash: "sha256:target", failQuery: false)));
        var service = CreateServiceWithCommActor(commActor);

        var result = await service.DeployInstanceAsync(11, "admin");

        Assert.True(result.IsSuccess);
        Assert.Equal(0, counters.QueryCount);
        Assert.Equal(1, counters.DeployCount);
    }

    [Fact]
    public async Task DeployInstanceAsync_PriorInProgressRecord_QueryFails_FallsThroughToDeploy()
    {
        // The site query fails (unreachable / times out). The deploy must NOT
        // abort — it falls through to a normal deploy and relies on site-side
        // stale-rejection as the safety net.
        var instance = new Instance("UnreachableInst") { Id = 12, SiteId = 1, State = InstanceState.Enabled };
        _repo.GetInstanceByIdAsync(12, Arg.Any<CancellationToken>()).Returns(instance);
        SetupValidPipeline(12, "UnreachableInst", "sha256:target");

        var prior = new DeploymentRecord("dep-prior5", "admin")
        {
            InstanceId = 12,
            Status = DeploymentStatus.InProgress,
            RevisionHash = "sha256:target"
        };
        _repo.GetCurrentDeploymentStatusAsync(12, Arg.Any<CancellationToken>()).Returns(prior);

        // The probe drops the query (no reply) -> the Ask times out.
        var counters = new ReconcileProbeCounters();
        var commActor = Sys.ActorOf(Props.Create(() =>
            new ReconcileProbeActor(counters, siteHash: "sha256:target", failQuery: true)));
        var service = CreateServiceWithCommActor(commActor);

        var result = await service.DeployInstanceAsync(12, "admin");

        // Did not abort — the deploy proceeded after the failed query.
        Assert.True(result.IsSuccess);
        Assert.Equal(1, counters.QueryCount);
        Assert.Equal(1, counters.DeployCount);
    }

    // ── Task 6: notify-and-fetch send path (stage PendingDeployment + send RefreshDeploymentCommand) ──

    [Fact]
    public async Task DeployInstanceAsync_Success_StagesPendingConfigThenSendsRefreshNotify()
    {
        // The central send path must stage the flattened config in a
        // PendingDeployment row and then notify the site with a small
        // RefreshDeploymentCommand (NOT ship the fat config inline), so the site
        // fetches the config over HTTP. The staged token + fetch base URL +
        // deployment id must travel in the notify.
        var instance = new Instance("NotifyFetchInst")
        {
            Id = 80, SiteId = 1, State = InstanceState.NotDeployed
        };
        _repo.GetInstanceByIdAsync(80, Arg.Any<CancellationToken>()).Returns(instance);
        SetupValidPipeline(80, "NotifyFetchInst", "sha256:target");
        _repo.GetCurrentDeploymentStatusAsync(80, Arg.Any<CancellationToken>())
            .Returns((DeploymentRecord?)null);

        PendingDeployment? staged = null;
        await _repo.AddPendingDeploymentAsync(
            Arg.Do<PendingDeployment>(p => staged = p), Arg.Any<CancellationToken>());

        var counters = new ReconcileProbeCounters();
        var commActor = Sys.ActorOf(Props.Create(() =>
            new ReconcileProbeActor(counters, siteHash: "sha256:target", failQuery: false)));
        var service = CreateServiceWithCommActor(commActor);

        var result = await service.DeployInstanceAsync(80, "admin");

        Assert.True(result.IsSuccess);

        // A PendingDeployment was staged with the deploy's identity + config.
        Assert.NotNull(staged);
        Assert.Equal(80, staged!.InstanceId);
        Assert.Equal("sha256:target", staged.RevisionHash);
        Assert.False(string.IsNullOrEmpty(staged.DeploymentId));
        Assert.False(string.IsNullOrEmpty(staged.Token));
        Assert.False(string.IsNullOrEmpty(staged.ConfigurationJson));
        // The TTL window is createdAt + CommunicationOptions.PendingDeploymentTtl.
        Assert.Equal(TestPendingTtl, staged.ExpiresAtUtc - staged.CreatedAtUtc);

        // The site was notified via RefreshDeploymentCommand (the deploy path no
        // longer ships DeployInstanceCommand), carrying the SAME deployment id,
        // the configured fetch base URL, and the staged fetch token.
        Assert.Equal(1, counters.DeployCount);
        Assert.NotNull(counters.LastRefresh);
        Assert.Equal(staged.DeploymentId, counters.LastRefresh!.DeploymentId);
        Assert.Equal("NotifyFetchInst", counters.LastRefresh.InstanceUniqueName);
        Assert.Equal("sha256:target", counters.LastRefresh.RevisionHash);
        Assert.Equal(TestFetchBaseUrl, counters.LastRefresh.CentralFetchBaseUrl);
        Assert.Equal(staged.Token, counters.LastRefresh.FetchToken);

        // The staged config was committed before/with the notify.
        await _repo.Received().AddPendingDeploymentAsync(
            Arg.Any<PendingDeployment>(), Arg.Any<CancellationToken>());
    }

    [Fact]
    public async Task DeployInstanceAsync_Success_DoesNotDeletePendingRow()
    {
        // Cleanup is TTL-based only: deleting the pending row on success would
        // 404 a standby node whose HTTP fetch is still in flight after a
        // central-side Ask timeout. The row must be left for TTL purge.
        var instance = new Instance("NoDeleteInst")
        {
            Id = 81, SiteId = 1, State = InstanceState.NotDeployed
        };
        _repo.GetInstanceByIdAsync(81, Arg.Any<CancellationToken>()).Returns(instance);
        SetupValidPipeline(81, "NoDeleteInst", "sha256:target");
        _repo.GetCurrentDeploymentStatusAsync(81, Arg.Any<CancellationToken>())
            .Returns((DeploymentRecord?)null);

        var counters = new ReconcileProbeCounters();
        var commActor = Sys.ActorOf(Props.Create(() =>
            new ReconcileProbeActor(counters, siteHash: "sha256:target", failQuery: false)));
        var service = CreateServiceWithCommActor(commActor);

        var result = await service.DeployInstanceAsync(81, "admin");

        Assert.True(result.IsSuccess);
        // The pending row is NOT deleted on success — TTL purge owns cleanup.
        await _repo.DidNotReceive().DeletePendingDeploymentByIdAsync(
            Arg.Any<string>(), Arg.Any<CancellationToken>());
    }

    [Fact]
    public async Task DeployInstanceAsync_CentralFetchBaseUrlEmpty_FailsFastWithoutNotifyOrStaging()
    {
        // Notify-and-fetch requires CentralFetchBaseUrl. With it empty the deploy
        // must fail fast with a clear configuration error BEFORE staging a
        // pending row, creating a deployment record, or notifying the site — so
        // the operator never sees a confusing downstream site-fetch error.
        var instance = new Instance("NoFetchUrlInst")
        {
            Id = 82, SiteId = 1, State = InstanceState.NotDeployed
        };
        _repo.GetInstanceByIdAsync(82, Arg.Any<CancellationToken>()).Returns(instance);
        SetupValidPipeline(82, "NoFetchUrlInst", "sha256:target");
        _repo.GetCurrentDeploymentStatusAsync(82, Arg.Any<CancellationToken>())
            .Returns((DeploymentRecord?)null);

        var counters = new ReconcileProbeCounters();
        var commActor = Sys.ActorOf(Props.Create(() =>
            new ReconcileProbeActor(counters, siteHash: "sha256:target", failQuery: false)));
        var service = CreateServiceWithCommActor(commActor, centralFetchBaseUrl: "");

        var result = await service.DeployInstanceAsync(82, "admin");

        Assert.True(result.IsFailure);
        Assert.Contains("CentralFetchBaseUrl", result.Error);

        // No site notify, no staged config, no deployment record.
        Assert.Equal(0, counters.DeployCount);
        Assert.Null(counters.LastRefresh);
        await _repo.DidNotReceive().AddPendingDeploymentAsync(
            Arg.Any<PendingDeployment>(), Arg.Any<CancellationToken>());
        await _repo.DidNotReceive().AddDeploymentRecordAsync(
            Arg.Any<DeploymentRecord>(), Arg.Any<CancellationToken>());
    }

    // ── DeploymentManager-015: reconciliation must perform the normal success side effects ──

    [Fact]
    public async Task DeployInstanceAsync_Reconciled_SetsInstanceEnabledAndStoresSnapshot()
    {
        // A prior deploy timed out; the site actually applied the target
        // revision. Reconciliation marks the prior record Success WITHOUT
        // re-sending -- but it must still perform the same side effects as the
        // normal success path: set the instance State to Enabled and store the
        // deployed-config snapshot. Otherwise central and site diverge.
        var instance = new Instance("ReconcileSideEffects")
        {
            Id = 70, SiteId = 1, State = InstanceState.NotDeployed
        };
        _repo.GetInstanceByIdAsync(70, Arg.Any<CancellationToken>()).Returns(instance);
        SetupValidPipeline(70, "ReconcileSideEffects", "sha256:target");

        var prior = new DeploymentRecord("dep-prior-70", "admin")
        {
            InstanceId = 70,
            Status = DeploymentStatus.InProgress,
            RevisionHash = "sha256:target"
        };
        _repo.GetCurrentDeploymentStatusAsync(70, Arg.Any<CancellationToken>()).Returns(prior);
        _repo.GetDeployedSnapshotByInstanceIdAsync(70, Arg.Any<CancellationToken>())
            .Returns((DeployedConfigSnapshot?)null);

        DeployedConfigSnapshot? storedSnapshot = null;
        await _repo.AddDeployedSnapshotAsync(
            Arg.Do<DeployedConfigSnapshot>(s => storedSnapshot = s), Arg.Any<CancellationToken>());

        var counters = new ReconcileProbeCounters();
        var commActor = Sys.ActorOf(Props.Create(() =>
            new ReconcileProbeActor(counters, siteHash: "sha256:target", failQuery: false)));
        var service = CreateServiceWithCommActor(commActor);

        var result = await service.DeployInstanceAsync(70, "admin");

        Assert.True(result.IsSuccess);
        // No re-deploy was sent -- this was reconciled.
        Assert.Equal(1, counters.QueryCount);
        Assert.Equal(0, counters.DeployCount);

        // DeploymentManager-015: the instance State must reflect the deployed
        // config the site is actually running.
        Assert.Equal(InstanceState.Enabled, instance.State);
        await _repo.Received().UpdateInstanceAsync(instance, Arg.Any<CancellationToken>());

        // DeploymentManager-015: a deployed-config snapshot must be stored so
        // GetDeploymentComparisonAsync has something to compare against.
        Assert.NotNull(storedSnapshot);
        Assert.Equal(70, storedSnapshot!.InstanceId);
        Assert.Equal("sha256:target", storedSnapshot.RevisionHash);
    }

    // ── DeploymentManager-018: reconciliation must preserve an intentional Disabled state ──

    [Fact]
    public async Task DeployInstanceAsync_Reconciled_DisabledInstance_PreservesDisabledState()
    {
        // DeploymentManager-018: after a central failover, the in-memory
        // OperationLockManager is lost (by design — in-progress treated as
        // failed). The prior deployment record remains InProgress in the DB.
        // The operator can legitimately invoke Disable on the instance between
        // the timed-out deploy and the redeploy. Disable does not change the
        // deployed config, so the site still reports the target revision hash.
        // When the operator retries the deploy, the reconciliation branch must
        // NOT silently overwrite Instance.State back to Enabled — that would
        // undo the explicit operator action with no audit trail.
        var instance = new Instance("ReconcileDisabled")
        {
            Id = 72, SiteId = 1, State = InstanceState.Disabled
        };
        _repo.GetInstanceByIdAsync(72, Arg.Any<CancellationToken>()).Returns(instance);
        SetupValidPipeline(72, "ReconcileDisabled", "sha256:target");

        var prior = new DeploymentRecord("dep-prior-72", "admin")
        {
            InstanceId = 72,
            Status = DeploymentStatus.InProgress,
            RevisionHash = "sha256:target"
        };
        _repo.GetCurrentDeploymentStatusAsync(72, Arg.Any<CancellationToken>()).Returns(prior);
        _repo.GetDeployedSnapshotByInstanceIdAsync(72, Arg.Any<CancellationToken>())
            .Returns((DeployedConfigSnapshot?)null);

        var counters = new ReconcileProbeCounters();
        var commActor = Sys.ActorOf(Props.Create(() =>
            new ReconcileProbeActor(counters, siteHash: "sha256:target", failQuery: false)));
        var service = CreateServiceWithCommActor(commActor);

        var result = await service.DeployInstanceAsync(72, "admin");

        // The reconciliation still succeeds and the prior record is marked
        // Success — central and site agree on the applied config.
        Assert.True(result.IsSuccess);
        Assert.Equal(DeploymentStatus.Success, prior.Status);
        Assert.Equal(1, counters.QueryCount);
        Assert.Equal(0, counters.DeployCount);

        // DeploymentManager-018: the operator's explicit Disable must survive
        // the reconciliation — Instance.State stays Disabled, not silently
        // flipped to Enabled.
        Assert.Equal(InstanceState.Disabled, instance.State);
    }

    /// <summary>
    /// The counterpart to the bulk path's Disabled skip (arch-review P7-2): a
    /// SINGLE-instance deploy of a Disabled instance still deploys AND enables it.
    /// That is the documented "Deploy on a Disabled instance also enables it"
    /// transition and the explicit operator action bulk deploy now refuses to
    /// perform implicitly, so it must keep working exactly as before.
    /// </summary>
    [Fact]
    public async Task DeployInstanceAsync_DisabledInstance_StillDeploysAndEnables()
    {
        var instance = new Instance("SingleDisabled")
        {
            Id = 73, SiteId = 1, State = InstanceState.Disabled
        };
        _repo.GetInstanceByIdAsync(73, Arg.Any<CancellationToken>()).Returns(instance);
        SetupValidPipeline(73, "SingleDisabled", "sha256:target");
        _repo.GetCurrentDeploymentStatusAsync(73, Arg.Any<CancellationToken>())
            .Returns((DeploymentRecord?)null);

        var counters = new ReconcileProbeCounters();
        var commActor = Sys.ActorOf(Props.Create(() =>
            new ReconcileProbeActor(counters, siteHash: "sha256:target", failQuery: false)));

        var result = await CreateServiceWithCommActor(commActor).DeployInstanceAsync(73, "admin");

        Assert.True(result.IsSuccess);
        Assert.Equal(1, counters.DeployCount);
        Assert.Equal(InstanceState.Enabled, instance.State);
    }

    // ── DeploymentManager-016: reconciled record must carry the target revision hash ──

    [Fact]
    public async Task DeployInstanceAsync_Reconciled_PriorRecordRevisionHashUpdatedToTarget()
    {
        // The prior record carries a stale revision hash (R1), but the site
        // reports it has the freshly-flattened target revision (R2). After
        // reconciliation the prior record's RevisionHash must agree with the
        // site's applied revision -- not keep the stale R1.
        var instance = new Instance("ReconcileStaleHash")
        {
            Id = 71, SiteId = 1, State = InstanceState.Enabled
        };
        _repo.GetInstanceByIdAsync(71, Arg.Any<CancellationToken>()).Returns(instance);
        SetupValidPipeline(71, "ReconcileStaleHash", "sha256:target");

        var prior = new DeploymentRecord("dep-prior-71", "admin")
        {
            InstanceId = 71,
            Status = DeploymentStatus.InProgress,
            RevisionHash = "sha256:stale-R1"
        };
        _repo.GetCurrentDeploymentStatusAsync(71, Arg.Any<CancellationToken>()).Returns(prior);
        _repo.GetDeployedSnapshotByInstanceIdAsync(71, Arg.Any<CancellationToken>())
            .Returns((DeployedConfigSnapshot?)null);

        var counters = new ReconcileProbeCounters();
        var commActor = Sys.ActorOf(Props.Create(() =>
            new ReconcileProbeActor(counters, siteHash: "sha256:target", failQuery: false)));
        var service = CreateServiceWithCommActor(commActor);

        var result = await service.DeployInstanceAsync(71, "admin");

        Assert.True(result.IsSuccess);
        Assert.Equal(DeploymentStatus.Success, prior.Status);
        // DeploymentManager-016: the persisted record's RevisionHash must agree
        // with the site's applied revision, not keep the stale R1.
        Assert.Equal("sha256:target", prior.RevisionHash);
    }

    // ── DeploymentManager-020: reconciliation audit attributes to the CURRENT user, not the prior deployer ──

    [Fact]
    public async Task DeployInstanceAsync_Reconciled_AuditAttributesCurrentUserNotPriorDeployer()
    {
        // DeploymentManager-020: a redeploy that reconciles a timed-out prior
        // record must be audited as the action of the user driving THIS
        // redeploy — not the user who originally issued the now-reconciled
        // deployment. The prior deployer is preserved in the detail object so
        // forensics still see who started the rescued run.
        var instance = new Instance("ReconcileAuditUser")
        {
            Id = 73, SiteId = 1, State = InstanceState.NotDeployed
        };
        _repo.GetInstanceByIdAsync(73, Arg.Any<CancellationToken>()).Returns(instance);
        SetupValidPipeline(73, "ReconcileAuditUser", "sha256:target");

        var prior = new DeploymentRecord("dep-prior-73", "originalUser")
        {
            InstanceId = 73,
            Status = DeploymentStatus.InProgress,
            RevisionHash = "sha256:target"
        };
        _repo.GetCurrentDeploymentStatusAsync(73, Arg.Any<CancellationToken>()).Returns(prior);
        _repo.GetDeployedSnapshotByInstanceIdAsync(73, Arg.Any<CancellationToken>())
            .Returns((DeployedConfigSnapshot?)null);

        var counters = new ReconcileProbeCounters();
        var commActor = Sys.ActorOf(Props.Create(() =>
            new ReconcileProbeActor(counters, siteHash: "sha256:target", failQuery: false)));
        var service = CreateServiceWithCommActor(commActor);

        var result = await service.DeployInstanceAsync(73, "currentUser");

        Assert.True(result.IsSuccess);
        // DeploymentManager-020: audit row's actor is the current user.
        await _audit.Received().LogAsync(
            "currentUser", "DeployReconciled", "Instance", "73", "ReconcileAuditUser",
            Arg.Any<object>(), Arg.Any<CancellationToken>());
        // And the prior deployer was NOT used as the actor.
        await _audit.DidNotReceive().LogAsync(
            "originalUser", "DeployReconciled", "Instance", "73", "ReconcileAuditUser",
            Arg.Any<object>(), Arg.Any<CancellationToken>());
    }

    // ── DeploymentManager-012: LifecycleCommandTimeout must actually bound lifecycle commands ──

    [Fact]
    public async Task DisableInstanceAsync_SiteUnresponsive_LifecycleCommandTimeoutBoundsTheWait()
    {
        // The site never replies to the DisableInstanceCommand. A short
        // LifecycleCommandTimeout must abort the wait quickly -- if the option
        // is dead code the call would instead hang until CommunicationOptions
        // .LifecycleTimeout (much longer) elapses.
        var instance = new Instance("StuckInst") { Id = 60, SiteId = 1, State = InstanceState.Enabled };
        _repo.GetInstanceByIdAsync(60, Arg.Any<CancellationToken>()).Returns(instance);

        // Probe drops every message -> no reply ever arrives.
        var commActor = Sys.ActorOf(Props.Create(() => new SilentProbeActor()));

        var comms = new CommunicationService(
            Options.Create(new CommunicationOptions
            {
                // Long communication-layer timeout: if LifecycleCommandTimeout
                // were dead, the test would wait this long.
                LifecycleTimeout = TimeSpan.FromSeconds(30)
            }),
            NullLogger<CommunicationService>.Instance);
        comms.SetCommunicationActor(commActor);

        var siteRepo = CreateSiteRepoStub();
        var service = new DeploymentService(
            _repo, siteRepo, _pipeline, comms, _lockManager, _audit,
            new DiffService(),
            new RevisionHashService(),
            new DeploymentStatusNotifier(NullLogger<DeploymentStatusNotifier>.Instance),
            Substitute.For<IStaleInstanceProbe>(),
            Options.Create(new DeploymentManagerOptions
            {
                OperationLockTimeout = TimeSpan.FromSeconds(5),
                LifecycleCommandTimeout = TimeSpan.FromMilliseconds(300)
            }),
            DeployCommOptions(),
            NullLogger<DeploymentService>.Instance);

        var sw = System.Diagnostics.Stopwatch.StartNew();
        var result = await service.DisableInstanceAsync(60, "admin");
        sw.Stop();

        Assert.True(result.IsFailure);
        // The 300ms LifecycleCommandTimeout bounded the wait well under the
        // 30s communication-layer timeout.
        Assert.True(sw.Elapsed < TimeSpan.FromSeconds(10),
            $"Lifecycle command was not bounded by LifecycleCommandTimeout (took {sw.Elapsed}).");
    }

    /// <summary>Stand-in actor that never replies to anything.</summary>
    private class SilentProbeActor : ReceiveActor
    {
        public SilentProbeActor() => ReceiveAny(_ => { });
    }

    // ── DeploymentManager-019: lifecycle timeouts must write an audit entry ──

    /// <summary>
    /// DeploymentManager-019: when a Disable times out at the site, the
    /// operator's attempted action must still be recorded in the audit log
    /// with the documented <c>DisableTimedOut</c> action — pre-fix nothing
    /// was written and the audit trail was silent about the attempt.
    /// </summary>
    [Fact]
    public async Task DisableInstanceAsync_LifecycleTimeout_WritesDisableTimedOutAuditEntry()
    {
        var instance = new Instance("TimeoutAuditInst")
        {
            Id = 61,
            SiteId = 1,
            State = InstanceState.Enabled,
        };
        _repo.GetInstanceByIdAsync(61, Arg.Any<CancellationToken>()).Returns(instance);

        var commActor = Sys.ActorOf(Props.Create(() => new SilentProbeActor()));

        var comms = new CommunicationService(
            Options.Create(new CommunicationOptions { LifecycleTimeout = TimeSpan.FromSeconds(30) }),
            NullLogger<CommunicationService>.Instance);
        comms.SetCommunicationActor(commActor);

        var siteRepo = CreateSiteRepoStub();
        var deadline = TimeSpan.FromMilliseconds(300);
        var service = new DeploymentService(
            _repo, siteRepo, _pipeline, comms, _lockManager, _audit,
            new DiffService(),
            new RevisionHashService(),
            new DeploymentStatusNotifier(NullLogger<DeploymentStatusNotifier>.Instance),
            Substitute.For<IStaleInstanceProbe>(),
            Options.Create(new DeploymentManagerOptions
            {
                OperationLockTimeout = TimeSpan.FromSeconds(5),
                LifecycleCommandTimeout = deadline,
            }),
            DeployCommOptions(),
            NullLogger<DeploymentService>.Instance);

        var result = await service.DisableInstanceAsync(61, "operator-jane");

        Assert.True(result.IsFailure);

        // The DisableTimedOut audit entry must have been written. Pre-fix the
        // catch block returned without calling _auditService at all.
        await _audit.Received(1).LogAsync(
            "operator-jane",
            "DisableTimedOut",
            "Instance",
            "61",
            instance.UniqueName,
            Arg.Any<object?>(),
            Arg.Any<CancellationToken>());
    }

    /// <summary>
    /// DeploymentManager-019: same audit guarantee for the Enable path.
    /// </summary>
    [Fact]
    public async Task EnableInstanceAsync_LifecycleTimeout_WritesEnableTimedOutAuditEntry()
    {
        var instance = new Instance("EnableTimeoutInst")
        {
            Id = 62,
            SiteId = 1,
            State = InstanceState.Disabled,
        };
        _repo.GetInstanceByIdAsync(62, Arg.Any<CancellationToken>()).Returns(instance);

        var commActor = Sys.ActorOf(Props.Create(() => new SilentProbeActor()));
        var comms = new CommunicationService(
            Options.Create(new CommunicationOptions { LifecycleTimeout = TimeSpan.FromSeconds(30) }),
            NullLogger<CommunicationService>.Instance);
        comms.SetCommunicationActor(commActor);

        var siteRepo = CreateSiteRepoStub();
        var service = new DeploymentService(
            _repo, siteRepo, _pipeline, comms, _lockManager, _audit,
            new DiffService(),
            new RevisionHashService(),
            new DeploymentStatusNotifier(NullLogger<DeploymentStatusNotifier>.Instance),
            Substitute.For<IStaleInstanceProbe>(),
            Options.Create(new DeploymentManagerOptions
            {
                OperationLockTimeout = TimeSpan.FromSeconds(5),
                LifecycleCommandTimeout = TimeSpan.FromMilliseconds(300),
            }),
            DeployCommOptions(),
            NullLogger<DeploymentService>.Instance);

        var result = await service.EnableInstanceAsync(62, "operator-jane");

        Assert.True(result.IsFailure);
        await _audit.Received(1).LogAsync(
            "operator-jane",
            "EnableTimedOut",
            "Instance",
            "62",
            instance.UniqueName,
            Arg.Any<object?>(),
            Arg.Any<CancellationToken>());
    }

    /// <summary>
    /// DeploymentManager-019: same audit guarantee for the Delete path.
    /// </summary>
    [Fact]
    public async Task DeleteInstanceAsync_LifecycleTimeout_WritesDeleteTimedOutAuditEntry()
    {
        var instance = new Instance("DeleteTimeoutInst")
        {
            Id = 63,
            SiteId = 1,
            State = InstanceState.Enabled,
        };
        _repo.GetInstanceByIdAsync(63, Arg.Any<CancellationToken>()).Returns(instance);

        var commActor = Sys.ActorOf(Props.Create(() => new SilentProbeActor()));
        var comms = new CommunicationService(
            Options.Create(new CommunicationOptions { LifecycleTimeout = TimeSpan.FromSeconds(30) }),
            NullLogger<CommunicationService>.Instance);
        comms.SetCommunicationActor(commActor);

        var siteRepo = CreateSiteRepoStub();
        var service = new DeploymentService(
            _repo, siteRepo, _pipeline, comms, _lockManager, _audit,
            new DiffService(),
            new RevisionHashService(),
            new DeploymentStatusNotifier(NullLogger<DeploymentStatusNotifier>.Instance),
            Substitute.For<IStaleInstanceProbe>(),
            Options.Create(new DeploymentManagerOptions
            {
                OperationLockTimeout = TimeSpan.FromSeconds(5),
                LifecycleCommandTimeout = TimeSpan.FromMilliseconds(300),
            }),
            DeployCommOptions(),
            NullLogger<DeploymentService>.Instance);

        var result = await service.DeleteInstanceAsync(63, "operator-jane");

        Assert.True(result.IsFailure);
        await _audit.Received(1).LogAsync(
            "operator-jane",
            "DeleteTimedOut",
            "Instance",
            "63",
            instance.UniqueName,
            Arg.Any<object?>(),
            Arg.Any<CancellationToken>());
    }

    // ── DeploymentManager-003: post-success persistence must commit the Success status ──

    [Fact]
    public async Task DeployInstanceAsync_SiteSucceeds_SnapshotWriteFails_RecordStillCommittedSuccess()
    {
        // The site applies the deployment (response Success), but storing the
        // deployed-config snapshot afterwards throws. The deployment record's
        // Success status MUST still be durably committed -- otherwise central
        // and site diverge: the site runs the new config while central shows a
        // non-Success record forever.
        var instance = new Instance("SnapFailInst") { Id = 20, SiteId = 1, State = InstanceState.NotDeployed };
        _repo.GetInstanceByIdAsync(20, Arg.Any<CancellationToken>()).Returns(instance);
        SetupValidPipeline(20, "SnapFailInst", "sha256:target");
        _repo.GetCurrentDeploymentStatusAsync(20, Arg.Any<CancellationToken>())
            .Returns((DeploymentRecord?)null);

        DeploymentRecord? captured = null;
        await _repo.AddDeploymentRecordAsync(
            Arg.Do<DeploymentRecord>(r => captured = r), Arg.Any<CancellationToken>());

        // The snapshot store throws.
        _repo.GetDeployedSnapshotByInstanceIdAsync(20, Arg.Any<CancellationToken>())
            .Returns((DeployedConfigSnapshot?)null);
        _repo.AddDeployedSnapshotAsync(Arg.Any<DeployedConfigSnapshot>(), Arg.Any<CancellationToken>())
            .Returns<Task>(_ => throw new InvalidOperationException("snapshot store unavailable"));

        var counters = new ReconcileProbeCounters();
        var commActor = Sys.ActorOf(Props.Create(() =>
            new ReconcileProbeActor(counters, siteHash: "sha256:target", failQuery: false)));
        var service = CreateServiceWithCommActor(commActor);

        var result = await service.DeployInstanceAsync(20, "admin");

        // The site succeeded -> the deployment is reported successful.
        Assert.True(result.IsSuccess);
        Assert.NotNull(captured);
        Assert.Equal(DeploymentStatus.Success, captured!.Status);

        // The Success status was committed (a SaveChanges happened with the
        // record in Success state) BEFORE the snapshot write was attempted.
        await _repo.Received().UpdateDeploymentRecordAsync(
            Arg.Is<DeploymentRecord>(r => r.Status == DeploymentStatus.Success),
            Arg.Any<CancellationToken>());
    }

    /// <summary>
    /// Per-test counters for <see cref="SerializationProbeActor"/>. DeploymentManager-024:
    /// each test owns its own instance, passed into the actor's constructor, so
    /// counters are no longer shared static state that races under parallel
    /// test execution.
    /// </summary>
    private sealed class SerializationProbeCounters
    {
        public int MaxConcurrent;
        public int Current;
        public readonly object Gate = new();
    }

    /// <summary>
    /// Stand-in CentralCommunicationActor that measures deploy concurrency. It
    /// defers each deploy reply via the scheduler, so if two deploys for the
    /// same instance were NOT serialized by the operation lock their windows
    /// would overlap and <c>MaxConcurrent</c> would exceed 1.
    ///
    /// Notify-and-fetch (Task 6): the deploy path now sends a
    /// <see cref="RefreshDeploymentCommand"/> instead of <c>DeployInstanceCommand</c>.
    /// </summary>
    private class SerializationProbeActor : ReceiveActor, IWithTimers
    {
        public ITimerScheduler Timers { get; set; } = null!;

        public SerializationProbeActor(SerializationProbeCounters counters)
        {
            Receive<SiteEnvelope>(env =>
            {
                if (env.Message is RefreshDeploymentCommand d)
                {
                    lock (counters.Gate)
                    {
                        counters.Current++;
                        if (counters.Current > counters.MaxConcurrent)
                            counters.MaxConcurrent = counters.Current;
                    }

                    var replyTo = Sender;
                    // Defer the reply so the deploy "window" stays open long
                    // enough for a non-serialized second deploy to overlap.
                    Timers.StartSingleTimer(
                        d.DeploymentId,
                        new CompleteDeploy(d, replyTo),
                        TimeSpan.FromMilliseconds(150));
                }
                else if (env.Message is DeploymentStateQueryRequest q)
                {
                    Sender.Tell(new DeploymentStateQueryResponse(
                        q.CorrelationId, q.InstanceUniqueName, false, null, null, DateTimeOffset.UtcNow));
                }
            });

            Receive<CompleteDeploy>(c =>
            {
                lock (counters.Gate)
                {
                    counters.Current--;
                }
                c.ReplyTo.Tell(new DeploymentStatusResponse(
                    c.Command.DeploymentId, c.Command.InstanceUniqueName,
                    DeploymentStatus.Success, null, DateTimeOffset.UtcNow));
            });
        }

        private sealed record CompleteDeploy(RefreshDeploymentCommand Command, IActorRef ReplyTo);
    }

    /// <summary>
    /// Per-test counters for <see cref="ReconcileProbeActor"/>. DeploymentManager-024:
    /// each test owns its own instance so counter assertions cannot race across
    /// tests running in parallel.
    /// </summary>
    private sealed class ReconcileProbeCounters
    {
        public int QueryCount;
        public int DeployCount;

        /// <summary>
        /// Notify-and-fetch (Task 6): the most recent
        /// <see cref="RefreshDeploymentCommand"/> the probe received, so a test
        /// can assert the deploy path notified the site with the staged token,
        /// fetch base URL, and deployment id.
        /// </summary>
        public RefreshDeploymentCommand? LastRefresh;
    }

    /// <summary>
    /// Stand-in CentralCommunicationActor for reconciliation tests. Counts the
    /// site queries and deploy notifies it receives (into a per-test
    /// <see cref="ReconcileProbeCounters"/> instance), answers queries with a
    /// configurable applied revision hash, and (optionally) drops the query to
    /// simulate an unreachable site so the central Ask times out.
    ///
    /// Notify-and-fetch (Task 6): the deploy path now sends a
    /// <see cref="RefreshDeploymentCommand"/> (not the fat
    /// <c>DeployInstanceCommand</c>), so the probe counts that message as a
    /// "deploy" and records it for assertion.
    /// </summary>
    private class ReconcileProbeActor : ReceiveActor
    {
        public ReconcileProbeActor(ReconcileProbeCounters counters, string siteHash, bool failQuery)
        {
            Receive<SiteEnvelope>(env =>
            {
                switch (env.Message)
                {
                    case DeploymentStateQueryRequest q:
                        Interlocked.Increment(ref counters.QueryCount);
                        if (!failQuery)
                        {
                            Sender.Tell(new DeploymentStateQueryResponse(
                                q.CorrelationId, q.InstanceUniqueName, true,
                                "dep-applied", siteHash, DateTimeOffset.UtcNow));
                        }
                        // failQuery: drop the message -> caller's Ask times out.
                        break;

                    case RefreshDeploymentCommand d:
                        Interlocked.Increment(ref counters.DeployCount);
                        counters.LastRefresh = d;
                        Sender.Tell(new DeploymentStatusResponse(
                            d.DeploymentId, d.InstanceUniqueName,
                            DeploymentStatus.Success, null, DateTimeOffset.UtcNow));
                        break;

                    case DisableInstanceCommand dis:
                        Sender.Tell(new InstanceLifecycleResponse(
                            dis.CommandId, dis.InstanceUniqueName, true, null, DateTimeOffset.UtcNow));
                        break;

                    case EnableInstanceCommand en:
                        Sender.Tell(new InstanceLifecycleResponse(
                            en.CommandId, en.InstanceUniqueName, true, null, DateTimeOffset.UtcNow));
                        break;

                    case DeleteInstanceCommand del:
                        Sender.Tell(new InstanceLifecycleResponse(
                            del.CommandId, del.InstanceUniqueName, true, null, DateTimeOffset.UtcNow));
                        break;
                }
            });
        }
    }

    /// <summary>
    /// Akka actor that silently discards every message, causing any Ask call
    /// directed at it to time out with <see cref="Akka.Actor.AskTimeoutException"/>.
    /// Used by <c>DeployInstanceAsync_AskTimeoutException_ClassifiedAsTimeout</c>
    /// to verify that the deploy catch block treats
    /// <see cref="Akka.Actor.AskTimeoutException"/> as a timeout.
    /// </summary>
    private class DroppingActor : ReceiveActor { }
}