From 4ee366ffeba71a28f98352c8a2b1dece6e2d358f Mon Sep 17 00:00:00 2001 From: Chris0Jeky Date: Fri, 3 Apr 2026 20:54:07 +0100 Subject: [PATCH 1/5] Add domain-level proposal lifecycle edge case tests (#708) Cover expiry timing boundaries, double-apply prevention, comprehensive state machine violations, dismissal edge cases, and operation mutation guards after state transitions. --- ...utomationProposalLifecycleEdgeCaseTests.cs | 553 ++++++++++++++++++ 1 file changed, 553 insertions(+) create mode 100644 backend/tests/Taskdeck.Domain.Tests/Entities/AutomationProposalLifecycleEdgeCaseTests.cs diff --git a/backend/tests/Taskdeck.Domain.Tests/Entities/AutomationProposalLifecycleEdgeCaseTests.cs b/backend/tests/Taskdeck.Domain.Tests/Entities/AutomationProposalLifecycleEdgeCaseTests.cs new file mode 100644 index 000000000..ff9040747 --- /dev/null +++ b/backend/tests/Taskdeck.Domain.Tests/Entities/AutomationProposalLifecycleEdgeCaseTests.cs @@ -0,0 +1,553 @@ +using FluentAssertions; +using Taskdeck.Domain.Entities; +using Taskdeck.Domain.Exceptions; +using Xunit; + +namespace Taskdeck.Domain.Tests.Entities; + +/// +/// Edge-case tests for the AutomationProposal state machine, covering +/// expiry timing boundaries, double-apply prevention, state machine violations, +/// and dismissal logic. Addresses issue #708 (TST-41). +/// +public class AutomationProposalLifecycleEdgeCaseTests +{ + private readonly Guid _userId = Guid.NewGuid(); + private readonly Guid _boardId = Guid.NewGuid(); + + #region Expiry Timing Boundaries + + [Fact] + public void Approve_ShouldThrow_WhenExpiresAtIsExactlyNow() + { + // Arrange: proposal whose expiry has just passed + var proposal = CreateProposal(); + SetExpiresAt(proposal, DateTime.UtcNow.AddMilliseconds(-1)); + + // Act + var act = () => proposal.Approve(Guid.NewGuid()); + + // Assert + act.Should().Throw() + .WithMessage("Cannot approve expired proposal"); + } + + [Fact] + public void IsExpired_ShouldBeTrue_WhenPastExpiry() + { + var proposal = CreateProposal(); + SetExpiresAt(proposal, DateTime.UtcNow.AddMinutes(-1)); + + proposal.IsExpired.Should().BeTrue(); + } + + [Fact] + public void IsExpired_ShouldBeFalse_WhenBeforeExpiry() + { + var proposal = CreateProposal(); + // Default expiry is 1440 minutes in the future + proposal.IsExpired.Should().BeFalse(); + } + + [Fact] + public void Expire_ShouldSucceed_EvenWhenExpiresAtIsInFuture() + { + // The Expire() method is a manual override that does not check ExpiresAt, + // it only requires PendingReview status. + var proposal = CreateProposal(); + + proposal.Expire(); + + proposal.Status.Should().Be(ProposalStatus.Expired); + } + + [Fact] + public void Approve_ShouldSucceed_WhenNotYetExpired() + { + var proposal = CreateProposal(); + // ExpiresAt defaults to 1440 minutes from now, well in the future + + proposal.Approve(Guid.NewGuid()); + + proposal.Status.Should().Be(ProposalStatus.Approved); + } + + #endregion + + #region Double-Apply Prevention + + [Fact] + public void MarkAsApplied_ShouldThrow_WhenAlreadyApplied() + { + var proposal = CreateProposal(); + proposal.Approve(Guid.NewGuid()); + proposal.MarkAsApplied(); + + var act = () => proposal.MarkAsApplied(); + + act.Should().Throw() + .WithMessage("Only approved proposals can be marked as applied"); + } + + [Fact] + public void MarkAsFailed_ShouldThrow_WhenAlreadyApplied() + { + var proposal = CreateProposal(); + proposal.Approve(Guid.NewGuid()); + proposal.MarkAsApplied(); + + var act = () => proposal.MarkAsFailed("Too late"); + + act.Should().Throw() + .WithMessage("Only approved proposals can be marked as failed"); + } + + [Fact] + public void MarkAsFailed_ShouldThrow_WhenAlreadyFailed() + { + var proposal = CreateProposal(); + proposal.Approve(Guid.NewGuid()); + proposal.MarkAsFailed("First failure"); + + var act = () => proposal.MarkAsFailed("Second failure"); + + act.Should().Throw() + .WithMessage("Only approved proposals can be marked as failed"); + } + + [Fact] + public void MarkAsApplied_ShouldThrow_WhenAlreadyFailed() + { + var proposal = CreateProposal(); + proposal.Approve(Guid.NewGuid()); + proposal.MarkAsFailed("Something broke"); + + var act = () => proposal.MarkAsApplied(); + + act.Should().Throw() + .WithMessage("Only approved proposals can be marked as applied"); + } + + #endregion + + #region State Machine Violations — Comprehensive Coverage + + [Fact] + public void Approve_ShouldThrow_WhenExpired() + { + var proposal = CreateProposal(); + proposal.Expire(); + + var act = () => proposal.Approve(Guid.NewGuid()); + + act.Should().Throw() + .WithMessage("Cannot approve proposal in status Expired"); + } + + [Fact] + public void Reject_ShouldThrow_WhenExpired() + { + var proposal = CreateProposal(); + proposal.Expire(); + + var act = () => proposal.Reject(Guid.NewGuid(), "Reason"); + + act.Should().Throw() + .WithMessage("Cannot reject proposal in status Expired"); + } + + [Fact] + public void Expire_ShouldThrow_WhenApproved() + { + var proposal = CreateProposal(); + proposal.Approve(Guid.NewGuid()); + + var act = () => proposal.Expire(); + + act.Should().Throw() + .WithMessage("Cannot expire proposal in status Approved"); + } + + [Fact] + public void Expire_ShouldThrow_WhenRejected() + { + var proposal = CreateProposal(); + proposal.Reject(Guid.NewGuid()); + + var act = () => proposal.Expire(); + + act.Should().Throw() + .WithMessage("Cannot expire proposal in status Rejected"); + } + + [Fact] + public void Expire_ShouldThrow_WhenAlreadyExpired() + { + var proposal = CreateProposal(); + proposal.Expire(); + + var act = () => proposal.Expire(); + + act.Should().Throw() + .WithMessage("Cannot expire proposal in status Expired"); + } + + [Fact] + public void Expire_ShouldThrow_WhenApplied() + { + var proposal = CreateProposal(); + proposal.Approve(Guid.NewGuid()); + proposal.MarkAsApplied(); + + var act = () => proposal.Expire(); + + act.Should().Throw() + .WithMessage("Cannot expire proposal in status Applied"); + } + + [Fact] + public void Expire_ShouldThrow_WhenFailed() + { + var proposal = CreateProposal(); + proposal.Approve(Guid.NewGuid()); + proposal.MarkAsFailed("Failure"); + + var act = () => proposal.Expire(); + + act.Should().Throw() + .WithMessage("Cannot expire proposal in status Failed"); + } + + [Fact] + public void Approve_ShouldThrow_WhenApplied() + { + var proposal = CreateProposal(); + proposal.Approve(Guid.NewGuid()); + proposal.MarkAsApplied(); + + var act = () => proposal.Approve(Guid.NewGuid()); + + act.Should().Throw() + .WithMessage("Cannot approve proposal in status Applied"); + } + + [Fact] + public void Approve_ShouldThrow_WhenFailed() + { + var proposal = CreateProposal(); + proposal.Approve(Guid.NewGuid()); + proposal.MarkAsFailed("Failure"); + + var act = () => proposal.Approve(Guid.NewGuid()); + + act.Should().Throw() + .WithMessage("Cannot approve proposal in status Failed"); + } + + [Fact] + public void Reject_ShouldThrow_WhenApplied() + { + var proposal = CreateProposal(); + proposal.Approve(Guid.NewGuid()); + proposal.MarkAsApplied(); + + var act = () => proposal.Reject(Guid.NewGuid(), "Too late"); + + act.Should().Throw() + .WithMessage("Cannot reject proposal in status Applied"); + } + + [Fact] + public void Reject_ShouldThrow_WhenFailed() + { + var proposal = CreateProposal(); + proposal.Approve(Guid.NewGuid()); + proposal.MarkAsFailed("Failure"); + + var act = () => proposal.Reject(Guid.NewGuid(), "Too late"); + + act.Should().Throw() + .WithMessage("Cannot reject proposal in status Failed"); + } + + [Fact] + public void MarkAsApplied_ShouldThrow_WhenPending() + { + var proposal = CreateProposal(); + + var act = () => proposal.MarkAsApplied(); + + act.Should().Throw() + .WithMessage("Only approved proposals can be marked as applied"); + } + + [Fact] + public void MarkAsFailed_ShouldThrow_WhenPending() + { + var proposal = CreateProposal(); + + var act = () => proposal.MarkAsFailed("Reason"); + + act.Should().Throw() + .WithMessage("Only approved proposals can be marked as failed"); + } + + [Fact] + public void MarkAsFailed_ShouldThrow_WhenExpired() + { + var proposal = CreateProposal(); + proposal.Expire(); + + var act = () => proposal.MarkAsFailed("Reason"); + + act.Should().Throw() + .WithMessage("Only approved proposals can be marked as failed"); + } + + [Fact] + public void MarkAsApplied_ShouldThrow_WhenExpired() + { + var proposal = CreateProposal(); + proposal.Expire(); + + var act = () => proposal.MarkAsApplied(); + + act.Should().Throw() + .WithMessage("Only approved proposals can be marked as applied"); + } + + #endregion + + #region Dismissal Edge Cases + + [Fact] + public void Dismiss_ShouldSucceed_WhenApplied() + { + var proposal = CreateProposal(); + proposal.Approve(Guid.NewGuid()); + proposal.MarkAsApplied(); + + proposal.Dismiss(); + + proposal.Status.Should().Be(ProposalStatus.Dismissed); + } + + [Fact] + public void Dismiss_ShouldSucceed_WhenFailed() + { + var proposal = CreateProposal(); + proposal.Approve(Guid.NewGuid()); + proposal.MarkAsFailed("Something failed"); + + proposal.Dismiss(); + + proposal.Status.Should().Be(ProposalStatus.Dismissed); + } + + [Fact] + public void Dismiss_ShouldSucceed_WhenRejected() + { + var proposal = CreateProposal(); + proposal.Reject(Guid.NewGuid()); + + proposal.Dismiss(); + + proposal.Status.Should().Be(ProposalStatus.Dismissed); + } + + [Fact] + public void Dismiss_ShouldThrow_WhenAlreadyDismissed() + { + var proposal = CreateProposal(); + proposal.Expire(); + proposal.Dismiss(); + + var act = () => proposal.Dismiss(); + + act.Should().Throw() + .WithMessage("Cannot dismiss proposal in status Dismissed"); + } + + [Fact] + public void CanBeDismissed_ShouldBeFalse_ForPendingReview() + { + var proposal = CreateProposal(); + proposal.CanBeDismissed.Should().BeFalse(); + } + + [Fact] + public void CanBeDismissed_ShouldBeFalse_ForApprovedNotExpired() + { + var proposal = CreateProposal(); + proposal.Approve(Guid.NewGuid()); + + proposal.CanBeDismissed.Should().BeFalse(); + } + + [Fact] + public void CanBeDismissed_ShouldBeTrue_ForApprovedAndExpired() + { + var proposal = CreateProposal(); + proposal.Approve(Guid.NewGuid()); + SetExpiresAt(proposal, DateTime.UtcNow.AddMinutes(-5)); + + proposal.CanBeDismissed.Should().BeTrue(); + } + + [Theory] + [InlineData("Applied")] + [InlineData("Rejected")] + [InlineData("Failed")] + [InlineData("Expired")] + public void CanBeDismissed_ShouldBeTrue_ForTerminalStatuses(string targetState) + { + var proposal = CreateProposal(); + + switch (targetState) + { + case "Applied": + proposal.Approve(Guid.NewGuid()); + proposal.MarkAsApplied(); + break; + case "Rejected": + proposal.Reject(Guid.NewGuid()); + break; + case "Failed": + proposal.Approve(Guid.NewGuid()); + proposal.MarkAsFailed("Error"); + break; + case "Expired": + proposal.Expire(); + break; + } + + proposal.CanBeDismissed.Should().BeTrue(); + } + + #endregion + + #region Operation Mutation Guards After State Transitions + + [Fact] + public void AddOperation_ShouldThrow_WhenExpired() + { + var proposal = CreateProposal(); + proposal.Expire(); + var operation = CreateOperation(proposal.Id); + + var act = () => proposal.AddOperation(operation); + + act.Should().Throw() + .WithMessage("Cannot add operations after proposal has been decided"); + } + + [Fact] + public void AddOperation_ShouldThrow_WhenRejected() + { + var proposal = CreateProposal(); + proposal.Reject(Guid.NewGuid()); + var operation = CreateOperation(proposal.Id); + + var act = () => proposal.AddOperation(operation); + + act.Should().Throw() + .WithMessage("Cannot add operations after proposal has been decided"); + } + + [Fact] + public void AddOperation_ShouldThrow_WhenApplied() + { + var proposal = CreateProposal(); + proposal.Approve(Guid.NewGuid()); + proposal.MarkAsApplied(); + var operation = CreateOperation(proposal.Id); + + var act = () => proposal.AddOperation(operation); + + act.Should().Throw() + .WithMessage("Cannot add operations after proposal has been decided"); + } + + [Fact] + public void SetDiffPreview_ShouldThrow_WhenApproved() + { + var proposal = CreateProposal(); + proposal.Approve(Guid.NewGuid()); + + var act = () => proposal.SetDiffPreview("diff"); + + act.Should().Throw() + .WithMessage("Cannot update diff preview after proposal has been decided"); + } + + [Fact] + public void SetDiffPreview_ShouldThrow_WhenExpired() + { + var proposal = CreateProposal(); + proposal.Expire(); + + var act = () => proposal.SetDiffPreview("diff"); + + act.Should().Throw() + .WithMessage("Cannot update diff preview after proposal has been decided"); + } + + [Fact] + public void SetValidationIssues_ShouldThrow_WhenApproved() + { + var proposal = CreateProposal(); + proposal.Approve(Guid.NewGuid()); + + var act = () => proposal.SetValidationIssues("issues"); + + act.Should().Throw() + .WithMessage("Cannot update validation issues after proposal has been decided"); + } + + [Fact] + public void SetValidationIssues_ShouldThrow_WhenExpired() + { + var proposal = CreateProposal(); + proposal.Expire(); + + var act = () => proposal.SetValidationIssues("issues"); + + act.Should().Throw() + .WithMessage("Cannot update validation issues after proposal has been decided"); + } + + #endregion + + #region Helpers + + private AutomationProposal CreateProposal(RiskLevel riskLevel = RiskLevel.Low) + { + return new AutomationProposal( + ProposalSourceType.Queue, + _userId, + "Test proposal", + riskLevel, + Guid.NewGuid().ToString(), + _boardId); + } + + private static AutomationProposalOperation CreateOperation(Guid proposalId, int sequence = 0) + { + return new AutomationProposalOperation( + proposalId, + sequence, + "create", + "card", + "{\"title\":\"Test\"}", + Guid.NewGuid().ToString()); + } + + private static void SetExpiresAt(AutomationProposal proposal, DateTime expiresAt) + { + var property = typeof(AutomationProposal).GetProperty( + nameof(AutomationProposal.ExpiresAt)); + property!.SetValue(proposal, expiresAt); + } + + #endregion +} From cf31b53ce0ba862fba23548f21f5a93c8f265341 Mon Sep 17 00:00:00 2001 From: Chris0Jeky Date: Fri, 3 Apr 2026 20:54:13 +0100 Subject: [PATCH 2/5] Add policy engine edge case tests (#708) Cover expiry enforcement during validation, operation limits, duplicate sequences, oversized parameters, risk classification defaults, and null proposal handling. --- .../AutomationPolicyEngineEdgeCaseTests.cs | 271 ++++++++++++++++++ 1 file changed, 271 insertions(+) create mode 100644 backend/tests/Taskdeck.Application.Tests/Services/AutomationPolicyEngineEdgeCaseTests.cs diff --git a/backend/tests/Taskdeck.Application.Tests/Services/AutomationPolicyEngineEdgeCaseTests.cs b/backend/tests/Taskdeck.Application.Tests/Services/AutomationPolicyEngineEdgeCaseTests.cs new file mode 100644 index 000000000..bcd71b554 --- /dev/null +++ b/backend/tests/Taskdeck.Application.Tests/Services/AutomationPolicyEngineEdgeCaseTests.cs @@ -0,0 +1,271 @@ +using FluentAssertions; +using Moq; +using Taskdeck.Application.DTOs; +using Taskdeck.Application.Interfaces; +using Taskdeck.Application.Services; +using Taskdeck.Domain.Entities; +using Xunit; + +namespace Taskdeck.Application.Tests.Services; + +/// +/// Edge-case tests for AutomationPolicyEngine covering policy validation, +/// expiry enforcement during execution, and empty-policy defaults. +/// Addresses issue #708 (TST-41). +/// +public class AutomationPolicyEngineEdgeCaseTests +{ + private readonly Mock _unitOfWorkMock; + private readonly AutomationPolicyEngine _engine; + + public AutomationPolicyEngineEdgeCaseTests() + { + _unitOfWorkMock = new Mock(); + _engine = new AutomationPolicyEngine(_unitOfWorkMock.Object); + } + + #region ValidatePolicy — Expiry + + [Fact] + public void ValidatePolicy_ShouldFail_WhenProposalHasExpired() + { + var proposal = CreateProposalDto( + expiresAt: DateTime.UtcNow.AddMinutes(-10), + operations: new List { CreateOperationDto() }); + + var result = _engine.ValidatePolicy(proposal); + + result.IsSuccess.Should().BeFalse(); + result.ErrorMessage.Should().Contain("expired"); + } + + [Fact] + public void ValidatePolicy_ShouldSucceed_WhenProposalHasNotExpired() + { + var proposal = CreateProposalDto( + expiresAt: DateTime.UtcNow.AddMinutes(60), + operations: new List { CreateOperationDto() }); + + var result = _engine.ValidatePolicy(proposal); + + result.IsSuccess.Should().BeTrue(); + } + + #endregion + + #region ValidatePolicy — Operation Limits + + [Fact] + public void ValidatePolicy_ShouldFail_WhenOperationCountExceedsMax() + { + var operations = Enumerable.Range(0, 51) + .Select(i => CreateOperationDto(sequence: i)) + .ToList(); + var proposal = CreateProposalDto(operations: operations); + + var result = _engine.ValidatePolicy(proposal); + + result.IsSuccess.Should().BeFalse(); + result.ErrorMessage.Should().Contain("maximum operation count"); + } + + [Fact] + public void ValidatePolicy_ShouldFail_WhenNoOperations() + { + var proposal = CreateProposalDto(operations: new List()); + + var result = _engine.ValidatePolicy(proposal); + + result.IsSuccess.Should().BeFalse(); + result.ErrorMessage.Should().Contain("at least one operation"); + } + + [Fact] + public void ValidatePolicy_ShouldFail_WhenOperationSequencesAreDuplicated() + { + var operations = new List + { + CreateOperationDto(sequence: 0), + CreateOperationDto(sequence: 0) + }; + var proposal = CreateProposalDto(operations: operations); + + var result = _engine.ValidatePolicy(proposal); + + result.IsSuccess.Should().BeFalse(); + result.ErrorMessage.Should().Contain("unique"); + } + + [Fact] + public void ValidatePolicy_ShouldFail_WhenOperationSequenceIsNegative() + { + var operations = new List + { + CreateOperationDto(sequence: -1) + }; + var proposal = CreateProposalDto(operations: operations); + + var result = _engine.ValidatePolicy(proposal); + + result.IsSuccess.Should().BeFalse(); + result.ErrorMessage.Should().Contain("non-negative"); + } + + [Fact] + public void ValidatePolicy_ShouldFail_WhenParametersExceedMaxLength() + { + var longParams = new string('x', 10001); + var operations = new List + { + CreateOperationDto(parameters: longParams) + }; + var proposal = CreateProposalDto(operations: operations); + + var result = _engine.ValidatePolicy(proposal); + + result.IsSuccess.Should().BeFalse(); + result.ErrorMessage.Should().Contain("maximum length"); + } + + #endregion + + #region ClassifyRisk — Default Behavior + + [Fact] + public void ClassifyRisk_ShouldReturnLow_WhenNoOperations() + { + // GP-06: empty policy defaults to review-first (low risk = still requires review) + var risk = _engine.ClassifyRisk(Enumerable.Empty()); + + risk.Should().Be(RiskLevel.Low); + } + + [Fact] + public void ClassifyRisk_ShouldReturnCritical_WhenDeleteBoard() + { + var operations = new[] + { + CreateOperationDto(actionType: "delete", targetType: "board") + }; + + var risk = _engine.ClassifyRisk(operations); + + risk.Should().Be(RiskLevel.Critical); + } + + [Fact] + public void ClassifyRisk_ShouldReturnCritical_WhenManyOperations() + { + var operations = Enumerable.Range(0, 21) + .Select(i => CreateOperationDto(sequence: i, actionType: "create", targetType: "card")) + .ToList(); + + var risk = _engine.ClassifyRisk(operations); + + risk.Should().Be(RiskLevel.Critical); + } + + [Fact] + public void ClassifyRisk_ShouldReturnHigh_WhenDeleteCard() + { + var operations = new[] + { + CreateOperationDto(actionType: "delete", targetType: "card") + }; + + var risk = _engine.ClassifyRisk(operations); + + risk.Should().Be(RiskLevel.High); + } + + [Fact] + public void ClassifyRisk_ShouldReturnMedium_WhenArchiveOperation() + { + var operations = new[] + { + CreateOperationDto(actionType: "archive", targetType: "card") + }; + + var risk = _engine.ClassifyRisk(operations); + + risk.Should().Be(RiskLevel.Medium); + } + + [Fact] + public void ClassifyRisk_ShouldReturnLow_ForSimpleCreate() + { + var operations = new[] + { + CreateOperationDto(actionType: "create", targetType: "card") + }; + + var risk = _engine.ClassifyRisk(operations); + + risk.Should().Be(RiskLevel.Low); + } + + #endregion + + #region ValidatePolicy — Null Proposal + + [Fact] + public void ValidatePolicy_ShouldFail_WhenProposalIsNull() + { + var result = _engine.ValidatePolicy(null!); + + result.IsSuccess.Should().BeFalse(); + result.ErrorMessage.Should().Contain("null"); + } + + #endregion + + #region Helpers + + private static ProposalDto CreateProposalDto( + DateTime? expiresAt = null, + List? operations = null) + { + return new ProposalDto( + Id: Guid.NewGuid(), + SourceType: ProposalSourceType.Queue, + SourceReferenceId: null, + BoardId: Guid.NewGuid(), + RequestedByUserId: Guid.NewGuid(), + Status: ProposalStatus.Approved, + RiskLevel: RiskLevel.Low, + Summary: "Test proposal", + DiffPreview: null, + ValidationIssues: null, + CreatedAt: DateTimeOffset.UtcNow.AddMinutes(-30), + UpdatedAt: DateTimeOffset.UtcNow.AddMinutes(-30), + ExpiresAt: expiresAt ?? DateTime.UtcNow.AddMinutes(60), + DecidedAt: DateTime.UtcNow.AddMinutes(-5), + DecidedByUserId: Guid.NewGuid(), + AppliedAt: null, + FailureReason: null, + CorrelationId: Guid.NewGuid().ToString(), + Operations: operations ?? new List { CreateOperationDto() } + ); + } + + private static ProposalOperationDto CreateOperationDto( + int sequence = 0, + string actionType = "create", + string targetType = "card", + string parameters = "{\"title\":\"Test\"}") + { + return new ProposalOperationDto( + Id: Guid.NewGuid(), + ProposalId: Guid.NewGuid(), + Sequence: sequence, + ActionType: actionType, + TargetType: targetType, + TargetId: null, + Parameters: parameters, + IdempotencyKey: Guid.NewGuid().ToString(), + ExpectedVersion: null + ); + } + + #endregion +} From 2d4c8843f3feac50a8b949dc49e1896ff852362c Mon Sep 17 00:00:00 2001 From: Chris0Jeky Date: Fri, 3 Apr 2026 20:54:19 +0100 Subject: [PATCH 3/5] Add proposal service edge case tests (#708) Cover approve-after-expiry race, batch expiry via service, double-apply and double-fail prevention, dismiss batch behavior with mixed dismissable/non-dismissable proposals, and reject after expiry. --- .../AutomationProposalServiceEdgeCaseTests.cs | 286 ++++++++++++++++++ 1 file changed, 286 insertions(+) create mode 100644 backend/tests/Taskdeck.Application.Tests/Services/AutomationProposalServiceEdgeCaseTests.cs diff --git a/backend/tests/Taskdeck.Application.Tests/Services/AutomationProposalServiceEdgeCaseTests.cs b/backend/tests/Taskdeck.Application.Tests/Services/AutomationProposalServiceEdgeCaseTests.cs new file mode 100644 index 000000000..08d64b4ac --- /dev/null +++ b/backend/tests/Taskdeck.Application.Tests/Services/AutomationProposalServiceEdgeCaseTests.cs @@ -0,0 +1,286 @@ +using System.Reflection; +using FluentAssertions; +using Moq; +using Taskdeck.Application.DTOs; +using Taskdeck.Application.Interfaces; +using Taskdeck.Application.Services; +using Taskdeck.Domain.Common; +using Taskdeck.Domain.Entities; +using Taskdeck.Domain.Exceptions; +using Xunit; + +namespace Taskdeck.Application.Tests.Services; + +/// +/// Edge-case tests for AutomationProposalService covering expiry service flow, +/// approve-after-expiry race, dismiss batch behavior, and double-apply via service. +/// Addresses issue #708 (TST-41). +/// +public class AutomationProposalServiceEdgeCaseTests +{ + private readonly Mock _unitOfWorkMock; + private readonly Mock _proposalRepoMock; + private readonly Mock _notificationServiceMock; + private readonly AutomationProposalService _service; + + public AutomationProposalServiceEdgeCaseTests() + { + _unitOfWorkMock = new Mock(); + _proposalRepoMock = new Mock(); + _notificationServiceMock = new Mock(); + + _unitOfWorkMock.Setup(u => u.AutomationProposals).Returns(_proposalRepoMock.Object); + _notificationServiceMock + .Setup(s => s.PublishAsync(It.IsAny(), default)) + .ReturnsAsync(Result.Success(true)); + + _service = new AutomationProposalService(_unitOfWorkMock.Object, _notificationServiceMock.Object); + } + + #region Approve After Expiry Race + + [Fact] + public async Task ApproveProposalAsync_ShouldReturnFailure_WhenProposalIsExpired() + { + // Arrange: proposal whose ExpiresAt is in the past + var proposal = CreatePendingProposal(); + SetExpiresAt(proposal, DateTime.UtcNow.AddMinutes(-10)); + + _proposalRepoMock + .Setup(r => r.GetByIdAsync(proposal.Id, It.IsAny())) + .ReturnsAsync(proposal); + + // Act + var result = await _service.ApproveProposalAsync(proposal.Id, Guid.NewGuid()); + + // Assert + result.IsSuccess.Should().BeFalse(); + result.ErrorMessage.Should().Contain("expired"); + } + + [Fact] + public async Task ApproveProposalAsync_ShouldReturnFailure_WhenProposalAlreadyApproved() + { + var proposal = CreatePendingProposal(); + proposal.Approve(Guid.NewGuid()); + + _proposalRepoMock + .Setup(r => r.GetByIdAsync(proposal.Id, It.IsAny())) + .ReturnsAsync(proposal); + + var result = await _service.ApproveProposalAsync(proposal.Id, Guid.NewGuid()); + + result.IsSuccess.Should().BeFalse(); + result.ErrorMessage.Should().Contain("Approved"); + } + + [Fact] + public async Task ApproveProposalAsync_ShouldReturnNotFound_WhenProposalDoesNotExist() + { + _proposalRepoMock + .Setup(r => r.GetByIdAsync(It.IsAny(), It.IsAny())) + .ReturnsAsync((AutomationProposal?)null); + + var result = await _service.ApproveProposalAsync(Guid.NewGuid(), Guid.NewGuid()); + + result.IsSuccess.Should().BeFalse(); + result.ErrorCode.Should().Be(ErrorCodes.NotFound); + } + + #endregion + + #region ExpireProposalsAsync (Batch Expiry) + + [Fact] + public async Task ExpireProposalsAsync_ShouldExpireAll_WhenMultipleExpired() + { + var expired1 = CreatePendingProposal(); + var expired2 = CreatePendingProposal(); + + _proposalRepoMock + .Setup(r => r.GetExpiredAsync(It.IsAny())) + .ReturnsAsync(new List { expired1, expired2 }); + + var result = await _service.ExpireProposalsAsync(); + + result.IsSuccess.Should().BeTrue(); + result.Value.Should().Be(2); + expired1.Status.Should().Be(ProposalStatus.Expired); + expired2.Status.Should().Be(ProposalStatus.Expired); + } + + [Fact] + public async Task ExpireProposalsAsync_ShouldReturnZero_WhenNoneExpired() + { + _proposalRepoMock + .Setup(r => r.GetExpiredAsync(It.IsAny())) + .ReturnsAsync(new List()); + + var result = await _service.ExpireProposalsAsync(); + + result.IsSuccess.Should().BeTrue(); + result.Value.Should().Be(0); + _unitOfWorkMock.Verify(u => u.SaveChangesAsync(It.IsAny()), Times.Never); + } + + #endregion + + #region MarkAsApplied — Double Apply Prevention + + [Fact] + public async Task MarkAsAppliedAsync_ShouldReturnFailure_WhenAlreadyApplied() + { + var proposal = CreatePendingProposal(); + proposal.Approve(Guid.NewGuid()); + proposal.MarkAsApplied(); + + _proposalRepoMock + .Setup(r => r.GetByIdAsync(proposal.Id, It.IsAny())) + .ReturnsAsync(proposal); + + var result = await _service.MarkAsAppliedAsync(proposal.Id); + + result.IsSuccess.Should().BeFalse(); + result.ErrorMessage.Should().Contain("Only approved proposals"); + } + + [Fact] + public async Task MarkAsFailedAsync_ShouldReturnFailure_WhenAlreadyFailed() + { + var proposal = CreatePendingProposal(); + proposal.Approve(Guid.NewGuid()); + proposal.MarkAsFailed("First failure"); + + _proposalRepoMock + .Setup(r => r.GetByIdAsync(proposal.Id, It.IsAny())) + .ReturnsAsync(proposal); + + var result = await _service.MarkAsFailedAsync(proposal.Id, "Second failure"); + + result.IsSuccess.Should().BeFalse(); + result.ErrorMessage.Should().Contain("Only approved proposals"); + } + + #endregion + + #region DismissProposalsAsync — Edge Cases + + [Fact] + public async Task DismissProposalsAsync_ShouldReturnZero_WhenEmptyIdsList() + { + var result = await _service.DismissProposalsAsync([]); + + result.IsSuccess.Should().BeTrue(); + result.Value.Should().Be(0); + } + + [Fact] + public async Task DismissProposalsAsync_ShouldSkipNonDismissable_AndCountOnlyDismissed() + { + // One expired (dismissable), one pending (not dismissable) + var expired = CreatePendingProposal(); + expired.Expire(); + + var pending = CreatePendingProposal(); + + _proposalRepoMock + .Setup(r => r.GetByIdsAsync(It.IsAny>(), It.IsAny())) + .ReturnsAsync(new List { expired, pending }); + + var result = await _service.DismissProposalsAsync([expired.Id, pending.Id]); + + result.IsSuccess.Should().BeTrue(); + result.Value.Should().Be(1); + expired.Status.Should().Be(ProposalStatus.Dismissed); + pending.Status.Should().Be(ProposalStatus.PendingReview); + } + + [Fact] + public async Task DismissProposalsAsync_ShouldDismissAll_WhenAllDismissable() + { + var proposals = new[] + { + CreateExpiredProposal(), + CreateRejectedProposal(), + CreateFailedProposal() + }; + + _proposalRepoMock + .Setup(r => r.GetByIdsAsync(It.IsAny>(), It.IsAny())) + .ReturnsAsync(proposals.ToList()); + + var result = await _service.DismissProposalsAsync(proposals.Select(p => p.Id).ToList()); + + result.IsSuccess.Should().BeTrue(); + result.Value.Should().Be(3); + proposals.Should().AllSatisfy(p => p.Status.Should().Be(ProposalStatus.Dismissed)); + } + + #endregion + + #region Reject Edge Cases + + [Fact] + public async Task RejectProposalAsync_ShouldReturnFailure_WhenAlreadyExpired() + { + var proposal = CreatePendingProposal(); + proposal.Expire(); + + _proposalRepoMock + .Setup(r => r.GetByIdAsync(proposal.Id, It.IsAny())) + .ReturnsAsync(proposal); + + var result = await _service.RejectProposalAsync( + proposal.Id, Guid.NewGuid(), new UpdateProposalStatusDto("Reason")); + + result.IsSuccess.Should().BeFalse(); + result.ErrorMessage.Should().Contain("Expired"); + } + + #endregion + + #region Helpers + + private static AutomationProposal CreatePendingProposal() + { + return new AutomationProposal( + ProposalSourceType.Queue, + Guid.NewGuid(), + "Test proposal", + RiskLevel.Low, + Guid.NewGuid().ToString(), + Guid.NewGuid()); + } + + private static AutomationProposal CreateExpiredProposal() + { + var p = CreatePendingProposal(); + p.Expire(); + return p; + } + + private static AutomationProposal CreateRejectedProposal() + { + var p = CreatePendingProposal(); + p.Reject(Guid.NewGuid()); + return p; + } + + private static AutomationProposal CreateFailedProposal() + { + var p = CreatePendingProposal(); + p.Approve(Guid.NewGuid()); + p.MarkAsFailed("Error"); + return p; + } + + private static void SetExpiresAt(AutomationProposal proposal, DateTime expiresAt) + { + typeof(AutomationProposal).GetProperty( + nameof(AutomationProposal.ExpiresAt), + BindingFlags.Instance | BindingFlags.Public | BindingFlags.NonPublic)! + .SetValue(proposal, expiresAt); + } + + #endregion +} From 262a2b2c34a0e676ebc095bb5e8edc0bd80118f9 Mon Sep 17 00:00:00 2001 From: Chris0Jeky Date: Fri, 3 Apr 2026 20:54:25 +0100 Subject: [PATCH 4/5] Add housekeeping worker edge case tests (#708) Cover normal pending expiry, batch expiry of 50 proposals, mixed-state batch handling, database error propagation, worker-vs-manual-approval race condition, and ExecuteAsync cancellation behavior. --- ...ProposalHousekeepingWorkerEdgeCaseTests.cs | 354 ++++++++++++++++++ 1 file changed, 354 insertions(+) create mode 100644 backend/tests/Taskdeck.Api.Tests/ProposalHousekeepingWorkerEdgeCaseTests.cs diff --git a/backend/tests/Taskdeck.Api.Tests/ProposalHousekeepingWorkerEdgeCaseTests.cs b/backend/tests/Taskdeck.Api.Tests/ProposalHousekeepingWorkerEdgeCaseTests.cs new file mode 100644 index 000000000..8fa704089 --- /dev/null +++ b/backend/tests/Taskdeck.Api.Tests/ProposalHousekeepingWorkerEdgeCaseTests.cs @@ -0,0 +1,354 @@ +using System.Reflection; +using FluentAssertions; +using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.Logging; +using Taskdeck.Api.Workers; +using Taskdeck.Application.Interfaces; +using Taskdeck.Application.Services; +using Taskdeck.Tests.Support; +using Taskdeck.Domain.Entities; +using Taskdeck.Domain.Enums; +using Xunit; + +namespace Taskdeck.Api.Tests; + +/// +/// Edge-case tests for ProposalHousekeepingWorker covering batch expiry, +/// mixed-state proposals, cancellation, database error resilience, +/// and race conditions between worker and manual actions. +/// Addresses issue #708 (TST-41) scenario 20. +/// +public class ProposalHousekeepingWorkerEdgeCaseTests +{ + #region Normal Expiry of Pending Proposals + + [Fact] + public async Task ExpireStaleProposals_ShouldExpirePendingProposal_WhenPastExpiresAt() + { + var proposal = CreatePendingProposal(); + SetExpiresAt(proposal, DateTime.UtcNow.AddMinutes(-10)); + + var repository = new TrackingProposalRepository([proposal]); + var unitOfWork = new FakeUnitOfWork(repository); + var (worker, sp) = CreateWorkerWithProvider(unitOfWork); + using (sp) + { + await InvokeExpireStaleProposalsAsync(worker, CancellationToken.None); + } + + proposal.Status.Should().Be(ProposalStatus.Expired); + repository.SaveChangesCalled.Should().BeTrue(); + } + + [Fact] + public async Task ExpireStaleProposals_ShouldNotExpirePendingProposal_WhenNotYetExpired() + { + var proposal = CreatePendingProposal(); + + var repository = new TrackingProposalRepository([proposal]); + var unitOfWork = new FakeUnitOfWork(repository); + var (worker, sp) = CreateWorkerWithProvider(unitOfWork); + using (sp) + { + await InvokeExpireStaleProposalsAsync(worker, CancellationToken.None); + } + + proposal.Status.Should().Be(ProposalStatus.PendingReview); + repository.SaveChangesCalled.Should().BeFalse(); + } + + #endregion + + #region Batch Expiry with Mixed States + + [Fact] + public async Task ExpireStaleProposals_ShouldHandleMixedStates_ExpiringOnlyEligiblePending() + { + var expiredPending = CreatePendingProposal(); + SetExpiresAt(expiredPending, DateTime.UtcNow.AddMinutes(-5)); + + var freshPending = CreatePendingProposal(); + + var repository = new TrackingProposalRepository([expiredPending, freshPending]); + var unitOfWork = new FakeUnitOfWork(repository); + var (worker, sp) = CreateWorkerWithProvider(unitOfWork); + using (sp) + { + await InvokeExpireStaleProposalsAsync(worker, CancellationToken.None); + } + + expiredPending.Status.Should().Be(ProposalStatus.Expired); + freshPending.Status.Should().Be(ProposalStatus.PendingReview); + repository.SaveChangesCalled.Should().BeTrue(); + } + + [Fact] + public async Task ExpireStaleProposals_ShouldExpireAllExpiredInBatch() + { + var proposals = Enumerable.Range(0, 50) + .Select(_ => + { + var p = CreatePendingProposal(); + SetExpiresAt(p, DateTime.UtcNow.AddMinutes(-1)); + return p; + }) + .ToList(); + + var repository = new TrackingProposalRepository(proposals); + var unitOfWork = new FakeUnitOfWork(repository); + var (worker, sp) = CreateWorkerWithProvider(unitOfWork); + using (sp) + { + await InvokeExpireStaleProposalsAsync(worker, CancellationToken.None); + } + + proposals.Should().AllSatisfy(p => p.Status.Should().Be(ProposalStatus.Expired)); + repository.SaveChangesCalled.Should().BeTrue(); + } + + #endregion + + #region Database Error Resilience + + [Fact] + public async Task ExpireStaleProposals_ShouldPropagateDbError_WhenGetByStatusThrows() + { + var repository = new ThrowingProposalRepository(); + var unitOfWork = new FakeUnitOfWork(repository); + var logger = new InMemoryLogger(); + var (worker, sp) = CreateWorkerWithProvider(unitOfWork, logger); + using (sp) + { + // ExpireStaleProposalsAsync doesn't catch DB errors in the fetch path; + // the ExecuteAsync loop handles that. Verify it propagates cleanly. + var act = () => InvokeExpireStaleProposalsAsync(worker, CancellationToken.None); + await act.Should().ThrowAsync(); + } + } + + #endregion + + #region Race: Worker vs Manual Approval + + [Fact] + public async Task ExpireStaleProposals_ShouldLogWarning_WhenProposalWasApprovedBetweenFetchAndExpire() + { + // Simulate race: proposal fetched as PendingReview but was approved + // before the worker iterates to it. Approve first (while not expired), + // then set ExpiresAt to past (simulating time passing). + var proposal = CreatePendingProposal(); + proposal.Approve(Guid.NewGuid()); + SetExpiresAt(proposal, DateTime.UtcNow.AddMinutes(-5)); + + var repository = new TrackingProposalRepository([proposal]); + var logger = new InMemoryLogger(); + var unitOfWork = new FakeUnitOfWork(repository); + var (worker, sp) = CreateWorkerWithProvider(unitOfWork, logger); + using (sp) + { + await InvokeExpireStaleProposalsAsync(worker, CancellationToken.None); + } + + proposal.Status.Should().Be(ProposalStatus.Approved, "proposal should remain approved"); + logger.Entries.Should().ContainSingle(e => e.Level == LogLevel.Warning); + logger.Entries.Single(e => e.Level == LogLevel.Warning) + .Message.Should().Contain("Failed to expire proposal"); + } + + #endregion + + #region Worker ExecuteAsync Loop + + [Fact] + public async Task ExecuteAsync_ShouldStopWithinReasonableTime_WhenCancelled() + { + var repository = new TrackingProposalRepository([]); + var unitOfWork = new FakeUnitOfWork(repository); + var (worker, sp) = CreateWorkerWithProvider(unitOfWork); + using (sp) + { + using var cts = new CancellationTokenSource(TimeSpan.FromMilliseconds(100)); + + var executeMethod = typeof(ProposalHousekeepingWorker).GetMethod( + "ExecuteAsync", + BindingFlags.Instance | BindingFlags.NonPublic); + executeMethod.Should().NotBeNull(); + + // Task.Delay throws TaskCanceledException when the token fires + // during the sleep. This is expected behavior for BackgroundService. + var task = (Task)executeMethod!.Invoke(worker, [cts.Token])!; + try + { + await task.WaitAsync(TimeSpan.FromSeconds(5)); + } + catch (TaskCanceledException) + { + // Expected: Task.Delay cancellation propagates + } + + task.IsCompleted.Should().BeTrue("worker should stop promptly after cancellation"); + } + } + + #endregion + + #region Helpers + + private static AutomationProposal CreatePendingProposal() + { + return new AutomationProposal( + ProposalSourceType.Queue, + Guid.NewGuid(), + "Test proposal", + RiskLevel.Low, + Guid.NewGuid().ToString()); + } + + private static void SetExpiresAt(AutomationProposal proposal, DateTime expiresAt) + { + typeof(AutomationProposal).GetProperty( + nameof(AutomationProposal.ExpiresAt), + BindingFlags.Instance | BindingFlags.Public | BindingFlags.NonPublic)! + .SetValue(proposal, expiresAt); + } + + private static (ProposalHousekeepingWorker Worker, ServiceProvider ServiceProvider) CreateWorkerWithProvider( + IUnitOfWork unitOfWork, + ILogger? logger = null) + { + var sp = BuildServiceProvider(unitOfWork); + var worker = new ProposalHousekeepingWorker( + sp.GetRequiredService(), + new WorkerSettings(), + new WorkerHeartbeatRegistry(), + logger ?? new InMemoryLogger()); + return (worker, sp); + } + + private static ServiceProvider BuildServiceProvider(IUnitOfWork unitOfWork) + { + var services = new ServiceCollection(); + services.AddSingleton(unitOfWork); + return services.BuildServiceProvider(); + } + + private static async Task InvokeExpireStaleProposalsAsync( + ProposalHousekeepingWorker worker, + CancellationToken cancellationToken) + { + var method = typeof(ProposalHousekeepingWorker).GetMethod( + "ExpireStaleProposalsAsync", + BindingFlags.Instance | BindingFlags.NonPublic); + method.Should().NotBeNull(); + await (Task)method!.Invoke(worker, [cancellationToken])!; + } + + #endregion + + #region Test Doubles + + private sealed class TrackingProposalRepository : IAutomationProposalRepository + { + private readonly IReadOnlyList _proposals; + public bool SaveChangesCalled { get; set; } + + public TrackingProposalRepository(IReadOnlyList proposals) + { + _proposals = proposals; + } + + public Task> GetByStatusAsync( + ProposalStatus status, int limit = 100, CancellationToken cancellationToken = default) + => Task.FromResult>(_proposals.Take(limit).ToList()); + + public Task> GetByIdsAsync(IEnumerable ids, CancellationToken cancellationToken = default) => throw new NotSupportedException(); + public Task CountPendingReviewByUserIdAsync(Guid userId, CancellationToken cancellationToken = default) => throw new NotSupportedException(); + public Task HasReviewedByUserIdAsync(Guid userId, CancellationToken cancellationToken = default) => throw new NotSupportedException(); + public Task GetByIdAsync(Guid id, CancellationToken cancellationToken = default) => throw new NotSupportedException(); + public Task> GetAllAsync(CancellationToken cancellationToken = default) => throw new NotSupportedException(); + public Task AddAsync(AutomationProposal entity, CancellationToken cancellationToken = default) => throw new NotSupportedException(); + public Task UpdateAsync(AutomationProposal entity, CancellationToken cancellationToken = default) => throw new NotSupportedException(); + public Task DeleteAsync(AutomationProposal entity, CancellationToken cancellationToken = default) => throw new NotSupportedException(); + public Task> GetByBoardIdAsync(Guid boardId, int limit = 100, CancellationToken cancellationToken = default) => throw new NotSupportedException(); + public Task> GetByUserIdAsync(Guid userId, int limit = 100, CancellationToken cancellationToken = default) => throw new NotSupportedException(); + public Task> GetByRiskLevelAsync(RiskLevel riskLevel, int limit = 100, CancellationToken cancellationToken = default) => throw new NotSupportedException(); + public Task GetBySourceReferenceAsync(ProposalSourceType sourceType, string referenceId, CancellationToken cancellationToken = default) => throw new NotSupportedException(); + public Task GetByCorrelationIdAsync(string correlationId, CancellationToken cancellationToken = default) => throw new NotSupportedException(); + public Task GetLatestByOperationTargetAsync(string targetType, string targetId, CancellationToken cancellationToken = default) => throw new NotSupportedException(); + public Task GetLatestByOperationTargetAsync(string targetType, string targetId, string actionType, ProposalSourceType sourceType, CancellationToken cancellationToken = default) => throw new NotSupportedException(); + public Task> GetExpiredAsync(CancellationToken cancellationToken = default) => throw new NotSupportedException(); + } + + private sealed class ThrowingProposalRepository : IAutomationProposalRepository + { + public Task> GetByStatusAsync( + ProposalStatus status, int limit = 100, CancellationToken cancellationToken = default) + => throw new InvalidOperationException("Database connection failed"); + + public Task> GetByIdsAsync(IEnumerable ids, CancellationToken cancellationToken = default) => throw new NotSupportedException(); + public Task CountPendingReviewByUserIdAsync(Guid userId, CancellationToken cancellationToken = default) => throw new NotSupportedException(); + public Task HasReviewedByUserIdAsync(Guid userId, CancellationToken cancellationToken = default) => throw new NotSupportedException(); + public Task GetByIdAsync(Guid id, CancellationToken cancellationToken = default) => throw new NotSupportedException(); + public Task> GetAllAsync(CancellationToken cancellationToken = default) => throw new NotSupportedException(); + public Task AddAsync(AutomationProposal entity, CancellationToken cancellationToken = default) => throw new NotSupportedException(); + public Task UpdateAsync(AutomationProposal entity, CancellationToken cancellationToken = default) => throw new NotSupportedException(); + public Task DeleteAsync(AutomationProposal entity, CancellationToken cancellationToken = default) => throw new NotSupportedException(); + public Task> GetByBoardIdAsync(Guid boardId, int limit = 100, CancellationToken cancellationToken = default) => throw new NotSupportedException(); + public Task> GetByUserIdAsync(Guid userId, int limit = 100, CancellationToken cancellationToken = default) => throw new NotSupportedException(); + public Task> GetByRiskLevelAsync(RiskLevel riskLevel, int limit = 100, CancellationToken cancellationToken = default) => throw new NotSupportedException(); + public Task GetBySourceReferenceAsync(ProposalSourceType sourceType, string referenceId, CancellationToken cancellationToken = default) => throw new NotSupportedException(); + public Task GetByCorrelationIdAsync(string correlationId, CancellationToken cancellationToken = default) => throw new NotSupportedException(); + public Task GetLatestByOperationTargetAsync(string targetType, string targetId, CancellationToken cancellationToken = default) => throw new NotSupportedException(); + public Task GetLatestByOperationTargetAsync(string targetType, string targetId, string actionType, ProposalSourceType sourceType, CancellationToken cancellationToken = default) => throw new NotSupportedException(); + public Task> GetExpiredAsync(CancellationToken cancellationToken = default) => throw new NotSupportedException(); + } + + private sealed class FakeUnitOfWork : IUnitOfWork + { + private readonly TrackingProposalRepository? _trackingRepo; + + public FakeUnitOfWork(IAutomationProposalRepository repo) + { + AutomationProposals = repo; + _trackingRepo = repo as TrackingProposalRepository; + } + + public IBoardRepository Boards => null!; + public IColumnRepository Columns => null!; + public ICardRepository Cards => null!; + public ICardCommentRepository CardComments => null!; + public ILabelRepository Labels => null!; + public IUserRepository Users => null!; + public IBoardAccessRepository BoardAccesses => null!; + public IAuditLogRepository AuditLogs => null!; + public ILlmQueueRepository LlmQueue => null!; + public IAutomationProposalRepository AutomationProposals { get; } + public IArchiveItemRepository ArchiveItems => null!; + public IChatSessionRepository ChatSessions => null!; + public IChatMessageRepository ChatMessages => null!; + public ICommandRunRepository CommandRuns => null!; + public INotificationRepository Notifications => null!; + public INotificationPreferenceRepository NotificationPreferences => null!; + public IUserPreferenceRepository UserPreferences => null!; + public IOutboundWebhookSubscriptionRepository OutboundWebhookSubscriptions => null!; + public IOutboundWebhookDeliveryRepository OutboundWebhookDeliveries => null!; + public ILlmUsageRecordRepository LlmUsageRecords => null!; + public IAgentProfileRepository AgentProfiles => null!; + public IAgentRunRepository AgentRuns => null!; + public IKnowledgeDocumentRepository KnowledgeDocuments => null!; + public IKnowledgeChunkRepository KnowledgeChunks => null!; + public IExternalLoginRepository ExternalLogins => null!; + + public Task SaveChangesAsync(CancellationToken cancellationToken = default) + { + if (_trackingRepo != null) _trackingRepo.SaveChangesCalled = true; + return Task.FromResult(0); + } + + public Task BeginTransactionAsync(CancellationToken cancellationToken = default) => Task.CompletedTask; + public Task CommitTransactionAsync(CancellationToken cancellationToken = default) => Task.CompletedTask; + public Task RollbackTransactionAsync(CancellationToken cancellationToken = default) => Task.CompletedTask; + } + + #endregion +} From a9dbec49b59af8fe4571c62123d5917f142ffd06 Mon Sep 17 00:00:00 2001 From: Chris0Jeky Date: Fri, 3 Apr 2026 21:51:12 +0100 Subject: [PATCH 5/5] Fix review findings: clock sensitivity, misleading names, missing edge cases - Rename Approve_ShouldThrow_WhenExpiresAtIsExactlyNow to WhenJustPastExpiry and use -1 second instead of -1ms to avoid Windows clock-resolution flakiness - Replace string-based Theory with separate Facts for refactoring safety - Add BindingFlags to SetExpiresAt reflection helper for consistency - Add missing tests: Dismiss on PendingReview, Dismiss on non-expired Approved, Reject without reason for High/Critical risk, AddOperation on Approved - Fix worker race test name and comment to honestly describe what it exercises - Increase cancellation test timeout from 100ms to 1s for CI stability - Fix cancellation test comment to accurately describe TaskCanceledException propagation from Task.Delay outside the worker try-catch --- ...ProposalHousekeepingWorkerEdgeCaseTests.cs | 21 +-- ...utomationProposalLifecycleEdgeCaseTests.cs | 131 +++++++++++++----- 2 files changed, 112 insertions(+), 40 deletions(-) diff --git a/backend/tests/Taskdeck.Api.Tests/ProposalHousekeepingWorkerEdgeCaseTests.cs b/backend/tests/Taskdeck.Api.Tests/ProposalHousekeepingWorkerEdgeCaseTests.cs index 8fa704089..0728a0cbd 100644 --- a/backend/tests/Taskdeck.Api.Tests/ProposalHousekeepingWorkerEdgeCaseTests.cs +++ b/backend/tests/Taskdeck.Api.Tests/ProposalHousekeepingWorkerEdgeCaseTests.cs @@ -131,11 +131,12 @@ public async Task ExpireStaleProposals_ShouldPropagateDbError_WhenGetByStatusThr #region Race: Worker vs Manual Approval [Fact] - public async Task ExpireStaleProposals_ShouldLogWarning_WhenProposalWasApprovedBetweenFetchAndExpire() + public async Task ExpireStaleProposals_ShouldLogWarning_WhenProposalCannotBeExpired() { - // Simulate race: proposal fetched as PendingReview but was approved - // before the worker iterates to it. Approve first (while not expired), - // then set ExpiresAt to past (simulating time passing). + // Verifies the worker's error handling when Expire() throws on a + // non-PendingReview proposal (e.g., approved between fetch and expire + // in production). The fake repo intentionally ignores status filters + // to exercise this catch path directly. var proposal = CreatePendingProposal(); proposal.Approve(Guid.NewGuid()); SetExpiresAt(proposal, DateTime.UtcNow.AddMinutes(-5)); @@ -167,23 +168,25 @@ public async Task ExecuteAsync_ShouldStopWithinReasonableTime_WhenCancelled() var (worker, sp) = CreateWorkerWithProvider(unitOfWork); using (sp) { - using var cts = new CancellationTokenSource(TimeSpan.FromMilliseconds(100)); + // Use 1 second timeout to avoid flakiness in CI environments + using var cts = new CancellationTokenSource(TimeSpan.FromSeconds(1)); var executeMethod = typeof(ProposalHousekeepingWorker).GetMethod( "ExecuteAsync", BindingFlags.Instance | BindingFlags.NonPublic); executeMethod.Should().NotBeNull(); - // Task.Delay throws TaskCanceledException when the token fires - // during the sleep. This is expected behavior for BackgroundService. + // Task.Delay(stoppingToken) on line 50 of the worker is outside the + // try-catch, so TaskCanceledException propagates when the token fires + // during the sleep interval. This is normal BackgroundService behavior. var task = (Task)executeMethod!.Invoke(worker, [cts.Token])!; try { - await task.WaitAsync(TimeSpan.FromSeconds(5)); + await task.WaitAsync(TimeSpan.FromSeconds(10)); } catch (TaskCanceledException) { - // Expected: Task.Delay cancellation propagates + // Expected: Task.Delay cancellation propagates out of ExecuteAsync } task.IsCompleted.Should().BeTrue("worker should stop promptly after cancellation"); diff --git a/backend/tests/Taskdeck.Domain.Tests/Entities/AutomationProposalLifecycleEdgeCaseTests.cs b/backend/tests/Taskdeck.Domain.Tests/Entities/AutomationProposalLifecycleEdgeCaseTests.cs index ff9040747..d87a7110e 100644 --- a/backend/tests/Taskdeck.Domain.Tests/Entities/AutomationProposalLifecycleEdgeCaseTests.cs +++ b/backend/tests/Taskdeck.Domain.Tests/Entities/AutomationProposalLifecycleEdgeCaseTests.cs @@ -18,11 +18,12 @@ public class AutomationProposalLifecycleEdgeCaseTests #region Expiry Timing Boundaries [Fact] - public void Approve_ShouldThrow_WhenExpiresAtIsExactlyNow() + public void Approve_ShouldThrow_WhenJustPastExpiry() { - // Arrange: proposal whose expiry has just passed + // Arrange: proposal whose expiry has just passed. + // Use -1 second (not -1ms) to avoid clock-resolution flakiness on Windows (~15ms). var proposal = CreateProposal(); - SetExpiresAt(proposal, DateTime.UtcNow.AddMilliseconds(-1)); + SetExpiresAt(proposal, DateTime.UtcNow.AddSeconds(-1)); // Act var act = () => proposal.Approve(Guid.NewGuid()); @@ -394,39 +395,105 @@ public void CanBeDismissed_ShouldBeTrue_ForApprovedAndExpired() proposal.CanBeDismissed.Should().BeTrue(); } - [Theory] - [InlineData("Applied")] - [InlineData("Rejected")] - [InlineData("Failed")] - [InlineData("Expired")] - public void CanBeDismissed_ShouldBeTrue_ForTerminalStatuses(string targetState) - { - var proposal = CreateProposal(); - - switch (targetState) - { - case "Applied": - proposal.Approve(Guid.NewGuid()); - proposal.MarkAsApplied(); - break; - case "Rejected": - proposal.Reject(Guid.NewGuid()); - break; - case "Failed": - proposal.Approve(Guid.NewGuid()); - proposal.MarkAsFailed("Error"); - break; - case "Expired": - proposal.Expire(); - break; - } + [Fact] + public void CanBeDismissed_ShouldBeTrue_WhenApplied() + { + var proposal = CreateProposal(); + proposal.Approve(Guid.NewGuid()); + proposal.MarkAsApplied(); + + proposal.CanBeDismissed.Should().BeTrue(); + } + + [Fact] + public void CanBeDismissed_ShouldBeTrue_WhenRejected() + { + var proposal = CreateProposal(); + proposal.Reject(Guid.NewGuid()); + + proposal.CanBeDismissed.Should().BeTrue(); + } + + [Fact] + public void CanBeDismissed_ShouldBeTrue_WhenFailed() + { + var proposal = CreateProposal(); + proposal.Approve(Guid.NewGuid()); + proposal.MarkAsFailed("Error"); + + proposal.CanBeDismissed.Should().BeTrue(); + } + + [Fact] + public void CanBeDismissed_ShouldBeTrue_WhenExpired() + { + var proposal = CreateProposal(); + proposal.Expire(); proposal.CanBeDismissed.Should().BeTrue(); } #endregion - #region Operation Mutation Guards After State Transitions + [Fact] + public void Dismiss_ShouldThrow_WhenPendingReview() + { + var proposal = CreateProposal(); + + var act = () => proposal.Dismiss(); + + act.Should().Throw() + .WithMessage("Cannot dismiss proposal in status PendingReview"); + } + + [Fact] + public void Dismiss_ShouldThrow_WhenApprovedAndNotExpired() + { + var proposal = CreateProposal(); + proposal.Approve(Guid.NewGuid()); + + var act = () => proposal.Dismiss(); + + act.Should().Throw() + .WithMessage("Cannot dismiss proposal in status Approved"); + } + + [Fact] + public void Reject_ShouldThrow_WhenHighRisk_WithoutReason() + { + var proposal = CreateProposal(RiskLevel.High); + + var act = () => proposal.Reject(Guid.NewGuid()); + + act.Should().Throw() + .WithMessage("Rejection reason is required for High and Critical risk proposals"); + } + + [Fact] + public void Reject_ShouldThrow_WhenCriticalRisk_WithoutReason() + { + var proposal = CreateProposal(RiskLevel.Critical); + + var act = () => proposal.Reject(Guid.NewGuid()); + + act.Should().Throw() + .WithMessage("Rejection reason is required for High and Critical risk proposals"); + } + + [Fact] + public void AddOperation_ShouldThrow_WhenApproved() + { + var proposal = CreateProposal(); + proposal.Approve(Guid.NewGuid()); + var operation = CreateOperation(proposal.Id); + + var act = () => proposal.AddOperation(operation); + + act.Should().Throw() + .WithMessage("Cannot add operations after proposal has been decided"); + } + + #region Operation Mutation Guards After State Transitions [Fact] public void AddOperation_ShouldThrow_WhenExpired() @@ -545,7 +612,9 @@ private static AutomationProposalOperation CreateOperation(Guid proposalId, int private static void SetExpiresAt(AutomationProposal proposal, DateTime expiresAt) { var property = typeof(AutomationProposal).GetProperty( - nameof(AutomationProposal.ExpiresAt)); + nameof(AutomationProposal.ExpiresAt), + System.Reflection.BindingFlags.Instance | System.Reflection.BindingFlags.Public | System.Reflection.BindingFlags.NonPublic); + property.Should().NotBeNull("ExpiresAt property must exist on AutomationProposal"); property!.SetValue(proposal, expiresAt); }