diff --git a/src/Backend/AutopilotMonitor.Functions.Tests/AdminConfigurationSessionDeletionKillSwitchTests.cs b/src/Backend/AutopilotMonitor.Functions.Tests/AdminConfigurationSessionDeletionKillSwitchTests.cs index fbeb4b9f..e93a533b 100644 --- a/src/Backend/AutopilotMonitor.Functions.Tests/AdminConfigurationSessionDeletionKillSwitchTests.cs +++ b/src/Backend/AutopilotMonitor.Functions.Tests/AdminConfigurationSessionDeletionKillSwitchTests.cs @@ -4,9 +4,9 @@ using Xunit; namespace AutopilotMonitor.Functions.Tests; /// -/// Default-value + round-trip tests for the cascade-delete kill-switch + per-tenant flag -/// (Plan §1 P8). Both flags default to false so the new V2 cascade pipeline is opt-in; -/// flipping the kill-switch is the global emergency stop. +/// Default-value tests for the cascade-delete global kill-switch (Plan §1 P8 / §9). +/// Default-off is the contract: a freshly created AdminConfiguration row must NOT +/// halt the cascade subsystem out of the box. /// public class AdminConfigurationSessionDeletionKillSwitchTests { @@ -21,8 +21,7 @@ public class AdminConfigurationSessionDeletionKillSwitchTests public void SessionDeletionKillSwitch_defaults_to_false_on_CreateDefault() { // CreateDefault() is called when no admin-config row exists yet — if the kill-switch - // somehow defaulted to true, the cascade subsystem (and the legacy delete path) would - // be 503'd out of the box. Default-off is the contract. + // somehow defaulted to true, the cascade subsystem would be 503'd out of the box. var cfg = AdminConfiguration.CreateDefault(); Assert.False(cfg.SessionDeletionKillSwitch); } @@ -33,25 +32,4 @@ public class AdminConfigurationSessionDeletionKillSwitchTests var cfg = new AdminConfiguration { SessionDeletionKillSwitch = true }; Assert.True(cfg.SessionDeletionKillSwitch); } - - [Fact] - public void EnableCascadeDeleteV2_defaults_to_false_on_new_tenant_config() - { - var cfg = new TenantConfiguration(); - Assert.False(cfg.EnableCascadeDeleteV2); - } - - [Fact] - public void EnableCascadeDeleteV2_defaults_to_false_on_CreateDefault() - { - var cfg = TenantConfiguration.CreateDefault("00000000-0000-0000-0000-000000000000"); - Assert.False(cfg.EnableCascadeDeleteV2); - } - - [Fact] - public void EnableCascadeDeleteV2_true_persists_on_the_config() - { - var cfg = new TenantConfiguration { EnableCascadeDeleteV2 = true }; - Assert.True(cfg.EnableCascadeDeleteV2); - } } diff --git a/src/Backend/AutopilotMonitor.Functions.Tests/DeleteSessionCasGuardTests.cs b/src/Backend/AutopilotMonitor.Functions.Tests/DeleteSessionCasGuardTests.cs deleted file mode 100644 index 508c0cbc..00000000 --- a/src/Backend/AutopilotMonitor.Functions.Tests/DeleteSessionCasGuardTests.cs +++ /dev/null @@ -1,175 +0,0 @@ -using System.Collections.Generic; -using System.Threading; -using System.Threading.Tasks; -using Azure; -using Azure.Data.Tables; -using AutopilotMonitor.Functions.Services; -using AutopilotMonitor.Shared; -using AutopilotMonitor.Shared.Models.Deletion; -using Microsoft.Extensions.Logging.Abstractions; -using Moq; -using Xunit; - -namespace AutopilotMonitor.Functions.Tests; - -/// -/// SDK-mock tests for the CAS guard added to -/// — Plan §5 PR5 finding 2. The legacy direct-delete path must refuse to tombstone a session -/// whose V2 cascade is in flight; without the guard, a scaled-out instance with stale tenant -/// config could race past the function-level pre-read and orphan every side-table row the -/// cascade was supposed to clean up. -/// -public class DeleteSessionCasGuardTests -{ - private const string TenantId = "11111111-1111-1111-1111-111111111111"; - private const string SessionId = "22222222-2222-2222-2222-222222222222"; - - [Fact] - public async Task DeleteSessionAsync_refuses_when_DeletionState_is_Queued() - { - // Sessions row is locked by an in-flight V2 cascade. The legacy path MUST NOT tombstone. - var harness = new Harness(); - harness.SetSessionEntity(deletionState: SessionDeletionState.Queued); - - var result = await harness.Sut.DeleteSessionAsync(TenantId, SessionId); - - Assert.False(result); - // Neither the Sessions DeleteEntity nor the SessionsIndex DeleteEntity may have run. - harness.Sessions.Verify(t => t.DeleteEntityAsync( - It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny()), - Times.Never); - harness.SessionsIndex.Verify(t => t.DeleteEntityAsync( - It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny()), - Times.Never); - } - - [Theory] - [InlineData(SessionDeletionState.Preparing)] - [InlineData(SessionDeletionState.Queued)] - [InlineData(SessionDeletionState.Running)] - [InlineData(SessionDeletionState.Poisoned)] - public async Task DeleteSessionAsync_refuses_for_every_locked_state(string state) - { - // The lock guard fires for every non-None state in SessionDeletionState.IsLocked. - var harness = new Harness(); - harness.SetSessionEntity(deletionState: state); - - var result = await harness.Sut.DeleteSessionAsync(TenantId, SessionId); - - Assert.False(result); - harness.Sessions.Verify(t => t.DeleteEntityAsync( - It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny()), - Times.Never); - } - - [Fact] - public async Task DeleteSessionAsync_returns_false_when_etag_cas_loses_to_concurrent_producer() - { - // Read passes (DeletionState=None), but between our read and DeleteEntity a parallel V2 - // producer CAS-set DeletionState=Preparing → Azure returns 412. Legacy must abort. - var harness = new Harness(); - harness.SetSessionEntity(deletionState: SessionDeletionState.None); - harness.Sessions - .Setup(t => t.DeleteEntityAsync(TenantId, SessionId, It.IsAny(), It.IsAny())) - .ThrowsAsync(new RequestFailedException(412, "Precondition Failed")); - - var result = await harness.Sut.DeleteSessionAsync(TenantId, SessionId); - - Assert.False(result); - // SessionsIndex must NOT be deleted when the Sessions tombstone CAS lost — the row is - // still owned by the V2 cascade that just claimed it. - harness.SessionsIndex.Verify(t => t.DeleteEntityAsync( - It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny()), - Times.Never); - } - - [Fact] - public async Task DeleteSessionAsync_passes_captured_etag_to_delete() - { - var harness = new Harness(); - harness.SetSessionEntity(deletionState: SessionDeletionState.None, etag: "0xMYETAG"); - harness.Sessions - .Setup(t => t.DeleteEntityAsync(TenantId, SessionId, It.IsAny(), It.IsAny())) - .ReturnsAsync(Mock.Of()); - harness.SessionsIndex - .Setup(t => t.DeleteEntityAsync(It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny())) - .ReturnsAsync(Mock.Of()); - - var result = await harness.Sut.DeleteSessionAsync(TenantId, SessionId); - - Assert.True(result); - // Verify the ETag captured from the GetEntity read flows through to DeleteEntity — this - // is what makes the CAS guard actually atomic at the storage layer. - harness.Sessions.Verify(t => t.DeleteEntityAsync( - TenantId, SessionId, It.Is(e => e.ToString() == "0xMYETAG"), It.IsAny()), - Times.Once); - } - - [Fact] - public async Task DeleteSessionAsync_returns_true_when_row_already_404() - { - // Idempotency: re-delete of an already-tombstoned session is a no-op success. - var harness = new Harness(); - harness.Sessions - .Setup(t => t.GetEntityAsync(TenantId, SessionId, It.IsAny>(), It.IsAny())) - .ThrowsAsync(new RequestFailedException(404, "Not Found")); - - var result = await harness.Sut.DeleteSessionAsync(TenantId, SessionId); - - Assert.True(result); - } - - [Fact] - public async Task DeleteSessionAsync_treats_None_state_as_writable_happy_path() - { - // Empty / None / null DeletionState all mean "no cascade in flight" → legacy proceeds. - var harness = new Harness(); - harness.SetSessionEntity(deletionState: SessionDeletionState.None); - harness.Sessions - .Setup(t => t.DeleteEntityAsync(TenantId, SessionId, It.IsAny(), It.IsAny())) - .ReturnsAsync(Mock.Of()); - harness.SessionsIndex - .Setup(t => t.DeleteEntityAsync(It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny())) - .ReturnsAsync(Mock.Of()); - - Assert.True(await harness.Sut.DeleteSessionAsync(TenantId, SessionId)); - - // Both deletes ran in the right order. - harness.Sessions.Verify(t => t.DeleteEntityAsync( - TenantId, SessionId, It.IsAny(), It.IsAny()), Times.Once); - harness.SessionsIndex.Verify(t => t.DeleteEntityAsync( - TenantId, It.IsAny(), It.IsAny(), It.IsAny()), Times.AtMostOnce); - } - - // ============================================================ Harness ==== - - private sealed class Harness - { - public Mock Sessions { get; } - public Mock SessionsIndex { get; } - public TableStorageService Sut { get; } - - public Harness() - { - Sessions = new Mock(); - SessionsIndex = new Mock(); - var mockServiceClient = new Mock(); - mockServiceClient.Setup(s => s.GetTableClient(Constants.TableNames.Sessions)).Returns(Sessions.Object); - mockServiceClient.Setup(s => s.GetTableClient(Constants.TableNames.SessionsIndex)).Returns(SessionsIndex.Object); - Sut = new TableStorageService(mockServiceClient.Object, NullLogger.Instance); - } - - public void SetSessionEntity(string deletionState, string etag = "0xORIGINAL") - { - var entity = new TableEntity(TenantId, SessionId) - { - ["IndexRowKey"] = "9999999999999999_22222222-2222-2222-2222-222222222222", - ["DeletionState"] = deletionState, - }; - entity.ETag = new ETag(etag); - Sessions - .Setup(t => t.GetEntityAsync(TenantId, SessionId, It.IsAny>(), It.IsAny())) - .ReturnsAsync(Response.FromValue(entity, Mock.Of())); - } - } -} diff --git a/src/Backend/AutopilotMonitor.Functions.Tests/DeleteSessionFunctionTests.cs b/src/Backend/AutopilotMonitor.Functions.Tests/DeleteSessionFunctionTests.cs index b55d6e2e..3014927a 100644 --- a/src/Backend/AutopilotMonitor.Functions.Tests/DeleteSessionFunctionTests.cs +++ b/src/Backend/AutopilotMonitor.Functions.Tests/DeleteSessionFunctionTests.cs @@ -8,16 +8,11 @@ using Xunit; namespace AutopilotMonitor.Functions.Tests; /// -/// Thin HTTP-layer tests for (plan §5 PR5). Mirrors the -/// pattern: verifies the policy-catalog registration -/// and the public status-mapping / body-shape helpers without touching HttpRequestData. -/// -/// The end-to-end coordination (kill-switch → 503, locked → 409, flag toggle → V2/legacy) -/// is covered by the producer test suite (PR3) for the V2 outcomes, the worker tests (PR4) -/// for cascade behaviour, and the §18.4 internal-tenant integration run before any tenant -/// flag flip. There is intentionally NO HttpRequestData mock here — the production code is -/// the same shell wrapped around . -/// +/// HTTP-layer tests for . Verifies the policy-catalog +/// registration, the public status-mapping / body-shape helpers, and the single pre-dispatch +/// kill-switch gate without touching HttpRequestData. Everything else (existence check, +/// lock-state mapping, recovery resume) is owned by the producer and covered by +/// tests. /// public class DeleteSessionFunctionTests { @@ -137,6 +132,31 @@ public class DeleteSessionFunctionTests Assert.False(HasProperty(body, "manifestId")); } + // ── EvaluateAdminDeleteGates (kill-switch short-circuit) ────────────────────────────── + + [Fact] + public void Gates_killSwitch_active_short_circuits_to_503() + { + var gate = DeleteSessionFunction.EvaluateAdminDeleteGates(killSwitchActive: true); + + Assert.NotNull(gate); + Assert.Equal(HttpStatusCode.ServiceUnavailable, gate!.Value.Status); + AssertProperty(gate.Value.Body, "hint", "kill_switch_active"); + // The kill-switch body must NOT carry a manifestId so the UI doesn't render a + // "track this cascade" link for a request that was refused. + Assert.False(HasProperty(gate.Value.Body, "manifestId")); + } + + [Fact] + public void Gates_killSwitch_inactive_returns_null_so_producer_is_invoked() + { + // The happy path: producer handles existence (404), lock-state (409), and recovery + // resume (202 for Queued/Preparing+Snapshot). Function must NOT short-circuit those. + var gate = DeleteSessionFunction.EvaluateAdminDeleteGates(killSwitchActive: false); + + Assert.Null(gate); + } + // ── Anonymous-object reflection helpers ─────────────────────────────────────────────── private static void AssertProperty(object body, string propertyName, object? expected) diff --git a/src/Backend/AutopilotMonitor.Functions.Tests/SessionDeletionMaintenanceFunctionTests.cs b/src/Backend/AutopilotMonitor.Functions.Tests/SessionDeletionMaintenanceFunctionTests.cs index e9eab7df..b5c2a941 100644 --- a/src/Backend/AutopilotMonitor.Functions.Tests/SessionDeletionMaintenanceFunctionTests.cs +++ b/src/Backend/AutopilotMonitor.Functions.Tests/SessionDeletionMaintenanceFunctionTests.cs @@ -324,7 +324,6 @@ public class SessionDeletionMaintenanceFunctionTests Fanout = new Mock( maintRepo.Object, - Mock.Of(), new Mock(Mock.Of(), NullLogger.Instance, memCache).Object, Mock.Of(), AdminConfig.Object, diff --git a/src/Backend/AutopilotMonitor.Functions.Tests/SessionRetentionFanoutServiceTests.cs b/src/Backend/AutopilotMonitor.Functions.Tests/SessionRetentionFanoutServiceTests.cs index 6689dfc4..eebb6272 100644 --- a/src/Backend/AutopilotMonitor.Functions.Tests/SessionRetentionFanoutServiceTests.cs +++ b/src/Backend/AutopilotMonitor.Functions.Tests/SessionRetentionFanoutServiceTests.cs @@ -15,8 +15,7 @@ using Xunit; namespace AutopilotMonitor.Functions.Tests; /// -/// Per-tenant retention dispatch + rate-limit tests for -/// (Plan §5 PR6 step 7). +/// Per-tenant retention dispatch + rate-limit tests for . /// public class SessionRetentionFanoutServiceTests { @@ -24,16 +23,15 @@ public class SessionRetentionFanoutServiceTests private const string TenantB = "22222222-2222-2222-2222-222222222222"; [Fact] - public async Task RunAsync_dispatches_V2_path_when_EnableCascadeDeleteV2_is_true() + public async Task RunAsync_enqueues_v2_cascade_for_old_sessions() { var harness = new Harness(); - harness.WithTenant(TenantA, retentionDays: 30, v2: true, + harness.WithTenant(TenantA, retentionDays: 30, sessions: new[] { Old("s1", 60), Old("s2", 45) }); var result = await harness.Sut.RunAsync(CancellationToken.None); Assert.Equal(2, result.SessionsEnqueued); - Assert.Equal(0, result.SessionsLegacyDeleted); harness.Enqueuer.Verify( e => e.EnqueueAsync(TenantA, "s1", "retention_cutoff", It.Is(a => a.Type == "maintenance"), It.IsAny(), It.IsAny()), Times.Once); @@ -42,52 +40,14 @@ public class SessionRetentionFanoutServiceTests Times.Once); } - [Fact] - public async Task RunAsync_falls_back_to_legacy_direct_delete_when_EnableCascadeDeleteV2_is_false() - { - var harness = new Harness(); - harness.WithTenant(TenantA, retentionDays: 30, v2: false, - sessions: new[] { Old("s1", 60) }); - harness.MaintenanceRepo - .Setup(m => m.DeleteSessionEventsAsync(TenantA, "s1")) - .ReturnsAsync(17); - harness.SessionRepo - .Setup(r => r.DeleteSessionAsync(TenantA, "s1")) - .ReturnsAsync(true); - - var result = await harness.Sut.RunAsync(CancellationToken.None); - - Assert.Equal(0, result.SessionsEnqueued); - Assert.Equal(1, result.SessionsLegacyDeleted); - harness.Enqueuer.Verify( - e => e.EnqueueAsync(It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny()), - Times.Never); - harness.SessionRepo.Verify(r => r.DeleteSessionAsync(TenantA, "s1"), Times.Once); - } - - [Fact] - public async Task RunAsync_legacy_path_counts_DeleteSessionAsync_false_as_skip_not_delete() - { - // PR5 finding 2: DeleteSessionAsync returns false when a V2 cascade has claimed the row. - // The fanout must record that as a skip so the next maintenance tick retries. - var harness = new Harness(); - harness.WithTenant(TenantA, retentionDays: 30, v2: false, sessions: new[] { Old("s1", 60) }); - harness.SessionRepo.Setup(r => r.DeleteSessionAsync(TenantA, "s1")).ReturnsAsync(false); - - var result = await harness.Sut.RunAsync(CancellationToken.None); - - Assert.Equal(0, result.SessionsLegacyDeleted); - Assert.Equal(1, result.SessionsSkipped); - } - [Fact] public async Task RunAsync_handles_two_tenants_with_different_retentions() { var harness = new Harness(); // Tenant A: 30d retention → eligible at 45d - harness.WithTenant(TenantA, retentionDays: 30, v2: true, sessions: new[] { Old("a1", 45) }); + harness.WithTenant(TenantA, retentionDays: 30, sessions: new[] { Old("a1", 45) }); // Tenant B: 120d retention → 45d-old session is NOT eligible (only 200d-old is) - harness.WithTenant(TenantB, retentionDays: 120, v2: true, sessions: new[] { Old("b1", 45), Old("b2", 200) }); + harness.WithTenant(TenantB, retentionDays: 120, sessions: new[] { Old("b1", 45), Old("b2", 200) }); // The repo only returns sessions older than each cutoff. Wire each tenant's mock to honor that. harness.MaintenanceRepo.Setup(m => m.GetSessionsOlderThanAsync(TenantA, It.IsAny())) @@ -112,7 +72,7 @@ public class SessionRetentionFanoutServiceTests var many = new List(); for (int i = 0; i < SessionRetentionFanoutService.MaxEnqueuesPerTenantPerRun + 25; i++) many.Add(Summary(TenantA, $"s{i:000}")); - harness.WithTenantOverride(TenantA, retentionDays: 30, v2: true, sessions: many); + harness.WithTenantOverride(TenantA, retentionDays: 30, sessions: many); var result = await harness.Sut.RunAsync(CancellationToken.None); @@ -128,62 +88,15 @@ public class SessionRetentionFanoutServiceTests public async Task RunAsync_skips_tenant_with_DataRetentionDays_zero() { var harness = new Harness(); - harness.WithTenant(TenantA, retentionDays: 0, v2: true, sessions: new[] { Old("s1", 365) }); + harness.WithTenant(TenantA, retentionDays: 0, sessions: new[] { Old("s1", 365) }); var result = await harness.Sut.RunAsync(CancellationToken.None); Assert.Equal(0, result.SessionsEnqueued); - Assert.Equal(0, result.SessionsLegacyDeleted); // GetSessionsOlderThanAsync must not have been called for that tenant. harness.MaintenanceRepo.Verify(m => m.GetSessionsOlderThanAsync(TenantA, It.IsAny()), Times.Never); } - // ────────────────────────────────────────────────────────────────────────── PR6 follow-up F1 ─ - // Codex-followup: the ordering changed from "side-tables first, Sessions-CAS last" to - // "Sessions-CAS first, side-tables after". DeleteSessionAsync's internal ETag-CAS is now the - // single arbiter against in-flight V2 cascades — if it returns false (locked / 412 / 404), - // side-tables stay untouched. This closes the TOCTOU window the pre-read couldn't. - - [Fact] - public async Task RunAsync_legacy_path_skips_side_tables_when_DeleteSessionAsync_refuses_locked_row() - { - // DeleteSessionAsync detects the V2 cascade lock internally and returns false. The fanout - // must record SessionsSkipped + leave side-tables intact so the V2 worker owns cleanup. - var harness = new Harness(); - harness.WithTenant(TenantA, retentionDays: 30, v2: false, sessions: new[] { Old("s1", 60) }); - harness.SessionRepo.Setup(r => r.DeleteSessionAsync(TenantA, "s1")).ReturnsAsync(false); - - var result = await harness.Sut.RunAsync(CancellationToken.None); - - Assert.Equal(1, result.SessionsSkipped); - Assert.Equal(0, result.SessionsLegacyDeleted); - harness.SessionRepo.Verify(r => r.DeleteSessionAsync(TenantA, "s1"), Times.Once); - // Side-tables MUST stay intact when DeleteSessionAsync refused. - harness.MaintenanceRepo.Verify(m => m.DeleteSessionEventsAsync(It.IsAny(), It.IsAny()), Times.Never); - harness.MaintenanceRepo.Verify(m => m.DeleteSessionRuleResultsAsync(It.IsAny(), It.IsAny()), Times.Never); - harness.MaintenanceRepo.Verify(m => m.DeleteSessionAppInstallSummariesAsync(It.IsAny(), It.IsAny()), Times.Never); - } - - [Fact] - public async Task RunAsync_legacy_path_runs_side_tables_only_after_DeleteSessionAsync_succeeds() - { - // Sessions-first ordering: side-table helpers fire ONLY after DeleteSessionAsync returns - // true. No producer can claim a missing Sessions row, so side-table cleanup is race-free. - var harness = new Harness(); - harness.WithTenant(TenantA, retentionDays: 30, v2: false, sessions: new[] { Old("s1", 60) }); - harness.SessionRepo.Setup(r => r.DeleteSessionAsync(TenantA, "s1")).ReturnsAsync(true); - harness.MaintenanceRepo.Setup(m => m.DeleteSessionEventsAsync(TenantA, "s1")).ReturnsAsync(7); - - var result = await harness.Sut.RunAsync(CancellationToken.None); - - Assert.Equal(1, result.SessionsLegacyDeleted); - Assert.Equal(0, result.SessionsSkipped); - harness.SessionRepo.Verify(r => r.DeleteSessionAsync(TenantA, "s1"), Times.Once); - harness.MaintenanceRepo.Verify(m => m.DeleteSessionEventsAsync(TenantA, "s1"), Times.Once); - harness.MaintenanceRepo.Verify(m => m.DeleteSessionRuleResultsAsync(TenantA, "s1"), Times.Once); - harness.MaintenanceRepo.Verify(m => m.DeleteSessionAppInstallSummariesAsync(TenantA, "s1"), Times.Once); - } - // ────────────────────────────────────────────────────────────────────────── PR6 follow-up F2 ─ [Fact] @@ -194,7 +107,7 @@ public class SessionRetentionFanoutServiceTests var harness = new Harness(); var manySessions = new List(); for (int i = 0; i < 5; i++) manySessions.Add(Summary(TenantA, $"s{i}")); - harness.WithTenantOverride(TenantA, retentionDays: 30, v2: true, sessions: manySessions); + harness.WithTenantOverride(TenantA, retentionDays: 30, sessions: manySessions); // First 2 calls return false (kill-switch off), then true → fanout aborts at the 3rd iteration. var calls = 0; @@ -223,8 +136,8 @@ public class SessionRetentionFanoutServiceTests { // Kill-switch flips on between tenant A and tenant B → tenant B is never started. var harness = new Harness(); - harness.WithTenant(TenantA, retentionDays: 30, v2: true, sessions: new[] { Old("a1", 60) }); - harness.WithTenant(TenantB, retentionDays: 30, v2: true, sessions: new[] { Old("b1", 60) }); + harness.WithTenant(TenantA, retentionDays: 30, sessions: new[] { Old("a1", 60) }); + harness.WithTenant(TenantB, retentionDays: 30, sessions: new[] { Old("b1", 60) }); var calls = 0; harness.AdminConfig.Setup(a => a.IsSessionDeletionKillSwitchActiveAsync()) @@ -244,13 +157,87 @@ public class SessionRetentionFanoutServiceTests harness.Enqueuer.Verify(e => e.EnqueueAsync(TenantB, It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny()), Times.Never); } + [Fact] + public async Task RunAsync_aborts_when_producer_reports_KillSwitchActive_outcome() + { + // Race scenario: pre-check (per-tenant + per-session) returns false because the + // operator hadn't flipped yet, the producer's own Step 0 check returns true because + // the flip landed in between. The fanout must NOT keep pushing the rest of the + // tenant's backlog through a producer that's already 503'ing every call. + var harness = new Harness(); + var sessions = new List(); + for (int i = 0; i < 4; i++) sessions.Add(Summary(TenantA, $"s{i}")); + harness.WithTenantOverride(TenantA, retentionDays: 30, sessions: sessions); + + // First enqueue returns Enqueued, second returns KillSwitchActive (producer flipped). + var enqueueCalls = 0; + harness.Enqueuer.Setup(e => e.EnqueueAsync(TenantA, It.IsAny(), It.IsAny(), + It.IsAny(), It.IsAny(), It.IsAny())) + .ReturnsAsync(() => + { + enqueueCalls++; + return enqueueCalls == 1 + ? new SessionDeletionEnqueueResult { Outcome = SessionDeletionEnqueueOutcome.Enqueued, ManifestId = "M" } + : new SessionDeletionEnqueueResult { Outcome = SessionDeletionEnqueueOutcome.KillSwitchActive }; + }); + + var result = await harness.Sut.RunAsync(CancellationToken.None); + + Assert.True(result.AbortedByKillSwitch); + Assert.Equal(1, result.SessionsEnqueued); + Assert.Equal(1, result.SessionsSkipped); // the KillSwitchActive session counts as skipped + // s2 and s3 must NOT be enqueued — inner loop broke at s1. + harness.Enqueuer.Verify(e => e.EnqueueAsync(TenantA, "s2", It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny()), Times.Never); + harness.Enqueuer.Verify(e => e.EnqueueAsync(TenantA, "s3", It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny()), Times.Never); + } + + [Fact] + public async Task RunAsync_aborts_outer_loop_when_producer_reports_KillSwitchActive_for_tenant_A() + { + // Two-tenant regression: tenant A's second enqueue returns KillSwitchActive. The + // outer loop must NOT advance to tenant B — that would push at least one more + // request through the producer despite the authoritative fail-closed outcome. + var harness = new Harness(); + harness.WithTenantOverride(TenantA, retentionDays: 30, sessions: new List + { + Summary(TenantA, "a0"), + Summary(TenantA, "a1"), + }); + harness.WithTenant(TenantB, retentionDays: 30, sessions: new[] { Old("b0", 60) }); + + // Tenant A: first enqueue Enqueued, second KillSwitchActive. + // Tenant B: would-be Enqueued (default) — must NEVER be called. + var aCalls = 0; + harness.Enqueuer.Setup(e => e.EnqueueAsync(TenantA, It.IsAny(), It.IsAny(), + It.IsAny(), It.IsAny(), It.IsAny())) + .ReturnsAsync(() => + { + aCalls++; + return aCalls == 1 + ? new SessionDeletionEnqueueResult { Outcome = SessionDeletionEnqueueOutcome.Enqueued, ManifestId = "M" } + : new SessionDeletionEnqueueResult { Outcome = SessionDeletionEnqueueOutcome.KillSwitchActive }; + }); + + var result = await harness.Sut.RunAsync(CancellationToken.None); + + Assert.True(result.AbortedByKillSwitch); + Assert.Equal(1, result.TenantsProcessed); // tenant A completed its RunForTenantAsync; B never started + Assert.Equal(1, result.SessionsEnqueued); + + // Tenant B must NEVER have been touched — neither config-read nor enqueue. + harness.Enqueuer.Verify(e => e.EnqueueAsync(TenantB, It.IsAny(), It.IsAny(), + It.IsAny(), It.IsAny(), It.IsAny()), + Times.Never); + harness.MaintenanceRepo.Verify(m => m.GetSessionsOlderThanAsync(TenantB, It.IsAny()), Times.Never); + } + // ─────────────────────────────────────────────────────────────────────────────────────────────── [Fact] public async Task RunAsync_counts_AlreadyInFlight_outcome_as_skip() { var harness = new Harness(); - harness.WithTenant(TenantA, retentionDays: 30, v2: true, sessions: new[] { Old("s1", 60) }); + harness.WithTenant(TenantA, retentionDays: 30, sessions: new[] { Old("s1", 60) }); // Another producer already claimed this session — fanout must not double-count. harness.Enqueuer.Setup(e => e.EnqueueAsync(TenantA, "s1", It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny())) .ReturnsAsync(new SessionDeletionEnqueueResult @@ -285,7 +272,6 @@ public class SessionRetentionFanoutServiceTests private sealed class Harness { public Mock MaintenanceRepo { get; } - public Mock SessionRepo { get; } public Mock TenantConfig { get; } public Mock Enqueuer { get; } public Mock AdminConfig { get; } @@ -296,18 +282,11 @@ public class SessionRetentionFanoutServiceTests public Harness() { MaintenanceRepo = new Mock(); - SessionRepo = new Mock(); var memCache = new MemoryCache(new MemoryCacheOptions()); TenantConfig = new Mock( Mock.Of(), NullLogger.Instance, memCache); Enqueuer = new Mock(); - // PR6 follow-up F1: legacy path now pre-reads the Sessions row via ISessionRepository - // to check DeletionState. Default: row exists with DeletionState=None so legacy proceeds. - SessionRepo.Setup(r => r.GetSessionAsync(It.IsAny(), It.IsAny())) - .Returns((tenant, sid) => Task.FromResult( - new SessionSummary { TenantId = tenant, SessionId = sid, DeletionState = SessionDeletionState.None })); - // PR6 follow-up F2: per-session kill-switch check inside the fanout loop. AdminConfig = new Mock( Mock.Of(), NullLogger.Instance, memCache); @@ -332,26 +311,26 @@ public class SessionRetentionFanoutServiceTests // Use internal ctor to inject a no-op throttle so the rate-limit test runs in real-time // (50ms × 100 = 5s otherwise). Sut = new SessionRetentionFanoutService( - MaintenanceRepo.Object, SessionRepo.Object, TenantConfig.Object, Enqueuer.Object, + MaintenanceRepo.Object, TenantConfig.Object, Enqueuer.Object, AdminConfig.Object, NullLogger.Instance, throttle: (_, _) => Task.CompletedTask); } - public void WithTenant(string tenantId, int retentionDays, bool v2, SessionSummary[] sessions) + public void WithTenant(string tenantId, int retentionDays, SessionSummary[] sessions) { _tenantIds.Add(tenantId); TenantConfig.Setup(t => t.GetConfigurationAsync(tenantId)) - .ReturnsAsync(new TenantConfiguration { TenantId = tenantId, DataRetentionDays = retentionDays, EnableCascadeDeleteV2 = v2 }); + .ReturnsAsync(new TenantConfiguration { TenantId = tenantId, DataRetentionDays = retentionDays }); MaintenanceRepo.Setup(m => m.GetSessionsOlderThanAsync(tenantId, It.IsAny())) .ReturnsAsync(new List(WithTenantId(tenantId, sessions))); } - public void WithTenantOverride(string tenantId, int retentionDays, bool v2, List sessions) + public void WithTenantOverride(string tenantId, int retentionDays, List sessions) { _tenantIds.Add(tenantId); TenantConfig.Setup(t => t.GetConfigurationAsync(tenantId)) - .ReturnsAsync(new TenantConfiguration { TenantId = tenantId, DataRetentionDays = retentionDays, EnableCascadeDeleteV2 = v2 }); + .ReturnsAsync(new TenantConfiguration { TenantId = tenantId, DataRetentionDays = retentionDays }); foreach (var s in sessions) s.TenantId = tenantId; MaintenanceRepo.Setup(m => m.GetSessionsOlderThanAsync(tenantId, It.IsAny())).ReturnsAsync(sessions); } diff --git a/src/Backend/AutopilotMonitor.Functions/DataAccess/TableStorage/TableConfigRepository.cs b/src/Backend/AutopilotMonitor.Functions/DataAccess/TableStorage/TableConfigRepository.cs index c8ee2f77..3a24a468 100644 --- a/src/Backend/AutopilotMonitor.Functions/DataAccess/TableStorage/TableConfigRepository.cs +++ b/src/Backend/AutopilotMonitor.Functions/DataAccess/TableStorage/TableConfigRepository.cs @@ -387,7 +387,6 @@ namespace AutopilotMonitor.Functions.DataAccess.TableStorage { "ValidateDeviceAssociation", config.ValidateDeviceAssociation }, { "AllowInsecureAgentRequests", config.AllowInsecureAgentRequests }, { "DataRetentionDays", config.DataRetentionDays }, - { "EnableCascadeDeleteV2", config.EnableCascadeDeleteV2 }, { "SessionTimeoutHours", config.SessionTimeoutHours }, { "MaxNdjsonPayloadSizeMB", config.MaxNdjsonPayloadSizeMB }, { "EnablePerformanceCollector", config.EnablePerformanceCollector }, @@ -470,7 +469,6 @@ namespace AutopilotMonitor.Functions.DataAccess.TableStorage ValidateDeviceAssociation = entity.GetBoolean("ValidateDeviceAssociation") ?? false, AllowInsecureAgentRequests = entity.GetBoolean("AllowInsecureAgentRequests") ?? false, DataRetentionDays = entity.GetInt32("DataRetentionDays") ?? 90, - EnableCascadeDeleteV2 = entity.GetBoolean("EnableCascadeDeleteV2") ?? false, SessionTimeoutHours = entity.GetInt32("SessionTimeoutHours") ?? 5, MaxNdjsonPayloadSizeMB = entity.GetInt32("MaxNdjsonPayloadSizeMB") ?? 5, EnablePerformanceCollector = entity.GetBoolean("EnablePerformanceCollector") ?? false, diff --git a/src/Backend/AutopilotMonitor.Functions/DataAccess/TableStorage/TableMaintenanceRepository.cs b/src/Backend/AutopilotMonitor.Functions/DataAccess/TableStorage/TableMaintenanceRepository.cs index a3c3bd75..9d3fe260 100644 --- a/src/Backend/AutopilotMonitor.Functions/DataAccess/TableStorage/TableMaintenanceRepository.cs +++ b/src/Backend/AutopilotMonitor.Functions/DataAccess/TableStorage/TableMaintenanceRepository.cs @@ -65,9 +65,6 @@ namespace AutopilotMonitor.Functions.DataAccess.TableStorage public Task DeleteSessionRuleResultsAsync(string tenantId, string sessionId) => _storage.DeleteSessionRuleResultsAsync(tenantId, sessionId); - public Task DeleteSessionAppInstallSummariesAsync(string tenantId, string sessionId) - => _storage.DeleteSessionAppInstallSummariesAsync(tenantId, sessionId); - public Task BackfillSessionIndexAsync() => _storage.BackfillSessionIndexAsync(); diff --git a/src/Backend/AutopilotMonitor.Functions/DataAccess/TableStorage/TableSessionRepository.cs b/src/Backend/AutopilotMonitor.Functions/DataAccess/TableStorage/TableSessionRepository.cs index 4fdd1ae9..577de65b 100644 --- a/src/Backend/AutopilotMonitor.Functions/DataAccess/TableStorage/TableSessionRepository.cs +++ b/src/Backend/AutopilotMonitor.Functions/DataAccess/TableStorage/TableSessionRepository.cs @@ -52,9 +52,6 @@ namespace AutopilotMonitor.Functions.DataAccess.TableStorage public Task GetAllSessionStatsAsync(string? tenantIdFilter, int days) => _storage.GetAllSessionStatsAsync(tenantIdFilter, days); - public Task DeleteSessionAsync(string tenantId, string sessionId) - => _storage.DeleteSessionAsync(tenantId, sessionId); - public Task UpdateSessionStatusAsync( string tenantId, string sessionId, SessionStatus status, EnrollmentPhase? currentPhase = null, string? failureReason = null, diff --git a/src/Backend/AutopilotMonitor.Functions/Functions/Config/UpdateTenantConfigurationFunction.cs b/src/Backend/AutopilotMonitor.Functions/Functions/Config/UpdateTenantConfigurationFunction.cs index 75ddf0cf..695ed665 100644 --- a/src/Backend/AutopilotMonitor.Functions/Functions/Config/UpdateTenantConfigurationFunction.cs +++ b/src/Backend/AutopilotMonitor.Functions/Functions/Config/UpdateTenantConfigurationFunction.cs @@ -95,8 +95,7 @@ namespace AutopilotMonitor.Functions.Functions.Config config.CustomRateLimitRequestsPerMinute != existingConfig.CustomRateLimitRequestsPerMinute || config.RateLimitRequestsPerMinute != existingConfig.RateLimitRequestsPerMinute || config.Disabled != existingConfig.Disabled || - config.ValidateDeviceAssociation != existingConfig.ValidateDeviceAssociation || - config.EnableCascadeDeleteV2 != existingConfig.EnableCascadeDeleteV2) + config.ValidateDeviceAssociation != existingConfig.ValidateDeviceAssociation) { _logger.LogWarning( "Tenant Admin {User} attempted to modify GA-only fields for tenant {TenantId}", @@ -117,9 +116,6 @@ namespace AutopilotMonitor.Functions.Functions.Config // RequestContext + TenantConfigurationService). Tracked in // memory/project_devprep_followups.md. config.ValidateDeviceAssociation = existingConfig.ValidateDeviceAssociation; - // V2 cascade-delete pipeline activation is a rollout decision; tenant - // admins cannot self-opt-in (or out) of the snapshot+restore pathway. - config.EnableCascadeDeleteV2 = existingConfig.EnableCascadeDeleteV2; } // Safety: if GA gate is off, force UnrestrictedMode to false diff --git a/src/Backend/AutopilotMonitor.Functions/Functions/Maintenance/SessionDeletionMaintenanceFunction.cs b/src/Backend/AutopilotMonitor.Functions/Functions/Maintenance/SessionDeletionMaintenanceFunction.cs index b99ff8c8..e9dedece 100644 --- a/src/Backend/AutopilotMonitor.Functions/Functions/Maintenance/SessionDeletionMaintenanceFunction.cs +++ b/src/Backend/AutopilotMonitor.Functions/Functions/Maintenance/SessionDeletionMaintenanceFunction.cs @@ -21,10 +21,10 @@ namespace AutopilotMonitor.Functions.Functions.Maintenance /// Manifest-blob-TTL sweep — defence-in-depth against a misconfigured Lifecycle policy. /// Stale-Preparing GC — clear rows stuck in Preparing for > 1h with no progress blob. /// Stranded-Queued detection — alert (no auto-clear) when a queued envelope hasn't been picked up in > 30min. - /// Retention fanout — per-tenant DataRetentionDays sweep, dispatched to V2 or legacy via . + /// Retention fanout — per-tenant DataRetentionDays sweep via . /// /// - /// Independent cadence from the legacy 2h Maintenance function so cascade-lifecycle + /// Independent cadence from the 2h Maintenance function so cascade-lifecycle /// work cannot starve rule-stats aggregation / cert-expiry checks, and operators get a /// dedicated OpsEvent stream ( /// + siblings) wired to Telegram via the standard alert-rules UI. @@ -167,7 +167,7 @@ namespace AutopilotMonitor.Functions.Functions.Maintenance { fanoutResult = await _fanout.RunAsync(cancellationToken); tenantsProcessedSoFar = fanoutResult.TenantsProcessed; - sessionsEnqueuedSoFar = fanoutResult.SessionsEnqueued + fanoutResult.SessionsLegacyDeleted; + sessionsEnqueuedSoFar = fanoutResult.SessionsEnqueued; } // (5) Completion OpsEvent — same shape regardless of kill-switch state, so dashboards @@ -176,7 +176,6 @@ namespace AutopilotMonitor.Functions.Functions.Maintenance killSwitchActive: killSwitchActive, tenantsProcessed: fanoutResult?.TenantsProcessed ?? 0, sessionsEnqueued: fanoutResult?.SessionsEnqueued ?? 0, - sessionsLegacyDeleted: fanoutResult?.SessionsLegacyDeleted ?? 0, sessionsSkipped: fanoutResult?.SessionsSkipped ?? 0, rateLimitedTenants: fanoutResult?.RateLimitedTenants ?? 0, blobsTtlGced: blobsTtlGced, @@ -186,11 +185,10 @@ namespace AutopilotMonitor.Functions.Functions.Maintenance abortedByKillSwitch: fanoutResult?.AbortedByKillSwitch ?? false); _logger.LogInformation( - "SessionDeletionMaintenance: completed in {Ms}ms — killSwitch={KillSwitch} tenants={Tenants} enqueued={Enqueued} legacy={Legacy} skipped={Skipped} blobs={Blobs} preparing={Preparing} stranded={Stranded} tombstones={Tombstones}", + "SessionDeletionMaintenance: completed in {Ms}ms — killSwitch={KillSwitch} tenants={Tenants} enqueued={Enqueued} skipped={Skipped} blobs={Blobs} preparing={Preparing} stranded={Stranded} tombstones={Tombstones}", sw.ElapsedMilliseconds, killSwitchActive, fanoutResult?.TenantsProcessed ?? 0, fanoutResult?.SessionsEnqueued ?? 0, - fanoutResult?.SessionsLegacyDeleted ?? 0, fanoutResult?.SessionsSkipped ?? 0, blobsTtlGced, preparingRowsCleared, strandedQueuedDetected, tombstonesPruned); } diff --git a/src/Backend/AutopilotMonitor.Functions/Functions/Sessions/DeleteSessionFunction.cs b/src/Backend/AutopilotMonitor.Functions/Functions/Sessions/DeleteSessionFunction.cs index ec422a8d..97866513 100644 --- a/src/Backend/AutopilotMonitor.Functions/Functions/Sessions/DeleteSessionFunction.cs +++ b/src/Backend/AutopilotMonitor.Functions/Functions/Sessions/DeleteSessionFunction.cs @@ -2,7 +2,6 @@ using System.Net; using AutopilotMonitor.Functions.Helpers; using AutopilotMonitor.Functions.Services; using AutopilotMonitor.Functions.Services.Deletion; -using AutopilotMonitor.Shared.DataAccess; using AutopilotMonitor.Shared.Models.Deletion; using Microsoft.Azure.Functions.Worker; using Microsoft.Azure.Functions.Worker.Http; @@ -11,37 +10,30 @@ using Microsoft.Extensions.Logging; namespace AutopilotMonitor.Functions.Functions.Sessions { /// - /// Admin-triggered session deletion. Plan §5 PR5 wires this function as a flag-gated - /// dispatcher between the legacy direct-delete path (today's default) and the V2 cascade - /// path (). + /// Admin-triggered session deletion. Always dispatches to the V2 cascade producer. /// - /// Order matters: kill-switch + DeletionState != None are checked before the - /// per-tenant feature flag — both paths must respect them so a kill-switch flip or a - /// half-completed V2 cascade is never stepped on by a legacy delete (plan §1 P8 / §6). + /// One HTTP gate runs before the producer: the global kill-switch (returns 503 uniformly + /// so no data-state info leaks while the operator override is active). The producer itself + /// owns the existence check (SessionMissing → 404), lock-state mapping + /// (AlreadyInFlight/Poisoned → 409), AND the intentional recovery resume + /// for stranded Queued / Preparing+Snapshot rows (→ 202). Routing those + /// through the producer is required so admin retries can recover crashed cascades — + /// blocking them at an HTTP gate strands the session until retention age. /// /// public class DeleteSessionFunction { private readonly ILogger _logger; - private readonly ISessionRepository _sessionRepo; - private readonly IMaintenanceRepository _maintenanceRepo; private readonly AdminConfigurationService _adminConfig; - private readonly TenantConfigurationService _tenantConfig; private readonly ISessionDeletionEnqueuer _enqueuer; public DeleteSessionFunction( ILogger logger, - ISessionRepository sessionRepo, - IMaintenanceRepository maintenanceRepo, AdminConfigurationService adminConfig, - TenantConfigurationService tenantConfig, ISessionDeletionEnqueuer enqueuer) { _logger = logger; - _sessionRepo = sessionRepo; - _maintenanceRepo = maintenanceRepo; _adminConfig = adminConfig; - _tenantConfig = tenantConfig; _enqueuer = enqueuer; } @@ -57,77 +49,29 @@ namespace AutopilotMonitor.Functions.Functions.Sessions // Authentication + TenantAdminOrGA authorization enforced by PolicyEnforcementMiddleware. // Tenant scoping is TenantScoping.QueryParam (catalog) so middleware validates the // optional ?tenantId=... against the caller's role and writes the resolved tenant - // into RequestContext.TargetTenantId. For non-GA users the query param is constrained - // to their own JWT tenant; GAs may target any tenant. The old TenantHelper.GetTenantId - // path read the JWT tenant only, which made every Global-Admin DELETE silently target - // the GA's home tenant instead of the requested one (Codex-followup F1). + // into RequestContext.TargetTenantId. var ctx = req.GetRequestContext(); var tenantId = ctx.TargetTenantId; var userIdentifier = ctx.UserPrincipalName; _logger.LogInformation($"Deleting session {sessionId} for tenant {tenantId} by user {userIdentifier}"); - // ── Step 1: global kill-switch — applied to BOTH paths (plan §1 P8 / §9). ───────── - // PR5 finding 1: must NOT go through the 5-minute-cached GetConfigurationAsync, - // otherwise a flip-ON is not honored across scaled-out instances until each one's - // cache expires independently. IsSessionDeletionKillSwitchActiveAsync is the - // direct (uncached, fail-closed) read. - if (await _adminConfig.IsSessionDeletionKillSwitchActiveAsync()) + // Gate — Kill-switch. Uncached, fail-closed read so a flip-ON is honored across + // scaled-out instances within seconds (not up to 5 min cache TTL). Runs before + // the producer so 503 wins over any data-state response and no info leaks. + var gateResult = EvaluateAdminDeleteGates( + killSwitchActive: await _adminConfig.IsSessionDeletionKillSwitchActiveAsync()); + if (gateResult.HasValue) { _logger.LogWarning( "DeleteSession rejected: SessionDeletionKillSwitch is active. tenant={TenantId} session={SessionId}", tenantId, sessionId); - return await WriteResponse(req, HttpStatusCode.ServiceUnavailable, new - { - success = false, - message = "Session deletion is temporarily disabled by global kill-switch.", - hint = "kill_switch_active", - }); + return await WriteResponse(req, gateResult.Value.Status, gateResult.Value.Body); } - // ── Step 2: existence + cascade-lock check — applied to BOTH paths (plan §6). ──── - // Reads the Sessions row once to (a) detect 404 and (b) check DeletionState so the - // legacy path can NEVER step on a half-completed V2 cascade (plan §1 P8 — last - // sentence: "legacy direct-delete path **also** respects DeletionState != None"). - var session = await _sessionRepo.GetSessionAsync(tenantId, sessionId); - if (session == null) - { - _logger.LogWarning($"Session {sessionId} not found"); - return await WriteResponse(req, HttpStatusCode.NotFound, new - { - success = false, - message = $"Session {sessionId} not found", - }); - } - - if (SessionDeletionState.IsLocked(session.DeletionState)) - { - _logger.LogWarning( - "DeleteSession rejected: session already in cascade state {State} (manifestId={ManifestId}). tenant={TenantId} session={SessionId}", - session.DeletionState, session.PendingDeletionManifestId, tenantId, sessionId); - - // Poisoned needs a different hint — operator must use the restore endpoint. - var poisonedHint = string.Equals(session.DeletionState, SessionDeletionState.Poisoned, StringComparison.Ordinal); - return await WriteResponse(req, HttpStatusCode.Conflict, new - { - success = false, - message = poisonedHint - ? "Cascade is poisoned; recover via POST /api/global/sessions/{id}/restore before retrying delete." - : "Session is already being deleted by a cascade.", - deletionState = session.DeletionState, - manifestId = session.PendingDeletionManifestId, - hint = poisonedHint ? "cascade_poisoned_use_restore" : "cascade_already_in_flight", - }); - } - - // ── Step 3: per-tenant V2 cascade flag — selects path. ────────────────────────── - var tenantCfg = await _tenantConfig.GetConfigurationAsync(tenantId); - if (tenantCfg.EnableCascadeDeleteV2) - { - return await RunV2CascadePathAsync(req, tenantId, sessionId, userIdentifier); - } - - return await RunLegacyDirectDeletePathAsync(req, tenantId, sessionId, userIdentifier); + // Gate passed — hand off to the V2 cascade producer (handles 404, locked states, + // and the Queued/Preparing+Snapshot recovery resume paths). + return await RunV2CascadePathAsync(req, tenantId, sessionId, userIdentifier); } catch (Exception ex) { @@ -141,10 +85,35 @@ namespace AutopilotMonitor.Functions.Functions.Sessions } } + /// + /// Pure pre-dispatch gate evaluation. Returns the kill-switch 503 short-circuit, or + /// null to hand off to the producer. + /// + /// Everything else — 404 SessionMissing, 409 AlreadyInFlight, 409 Poisoned, and the + /// intentional Queued / Preparing+Snapshot → 202 Resume recovery paths — + /// is owned by the producer. Adding an HTTP-level lock-state gate here would block the + /// producer's recovery and strand admin-triggered cascades that crashed mid-flight. + /// + /// + internal static (HttpStatusCode Status, object Body)? EvaluateAdminDeleteGates(bool killSwitchActive) + { + if (killSwitchActive) + { + return (HttpStatusCode.ServiceUnavailable, new + { + success = false, + message = "Session deletion is temporarily disabled by global kill-switch.", + hint = "kill_switch_active", + }); + } + + return null; + } + /// /// V2 cascade path — enqueue a manifest-driven cascade and let the worker drain it. - /// Returns 202 Accepted on success; translates the producer's enqueue outcomes to - /// HTTP statuses per plan §5 PR5. + /// Returns 202 Accepted on success; translates the producer's enqueue outcomes to HTTP + /// statuses via . /// private async Task RunV2CascadePathAsync( HttpRequestData req, string tenantId, string sessionId, string userIdentifier) @@ -183,9 +152,8 @@ namespace AutopilotMonitor.Functions.Functions.Sessions } /// - /// Pure status mapping for the V2 enqueue outcomes (plan §5 PR5). Exposed as a - /// public static so tests can verify the contract without HTTP plumbing — same - /// pattern as . + /// Pure status mapping for the V2 enqueue outcomes. Exposed as a public static so tests + /// can verify the contract without HTTP plumbing. /// public static HttpStatusCode MapEnqueueOutcomeToStatus(SessionDeletionEnqueueOutcome outcome) => outcome switch { @@ -251,102 +219,6 @@ namespace AutopilotMonitor.Functions.Functions.Sessions }, }; - /// - /// Legacy direct-delete path — preserved for the flag-OFF default. Removed by - /// plan §5 PR7 once V2 has been stable on all tenants for at least two weeks. - /// - /// Ordering (Sessions-first): the ETag-CAS-guarded DeleteSessionAsync runs - /// before any side-table delete. This closes the TOCTOU window where a V2 - /// cascade producer could CAS-claim DeletionState=Preparing after our pre-read - /// but before our side-table mutations — leaving the producer's manifest snapshot - /// inconsistent against an already-half-purged side-table state. Side-table cleanup - /// after the Sessions row is tombstoned is safe: no producer can claim a missing row. - /// - /// - private async Task RunLegacyDirectDeletePathAsync( - HttpRequestData req, string tenantId, string sessionId, string userIdentifier) - { - // Sessions-first: tombstone (and release the writer-lock semantics) before any - // side-table mutation. DeleteSessionAsync is ETag-CAS-guarded (PR5 finding 2) - // against in-flight V2 cascades. - var sessionDeleted = await _sessionRepo.DeleteSessionAsync(tenantId, sessionId); - - if (!sessionDeleted) - { - // DeleteSessionAsync returned false. Either 404 (already gone) or a concurrent - // V2 producer CAS-claimed the row (ETag 412 / non-None DeletionState). Re-read - // to disambiguate and pick the right status code. - var refreshed = await _sessionRepo.GetSessionAsync(tenantId, sessionId); - if (refreshed == null) - { - _logger.LogWarning($"Session {sessionId} not found at final delete (race with concurrent caller)"); - return await WriteResponse(req, HttpStatusCode.NotFound, new - { - success = false, - message = $"Session {sessionId} not found", - }); - } - if (SessionDeletionState.IsLocked(refreshed.DeletionState)) - { - _logger.LogWarning( - "Legacy delete refused: V2 cascade owns the session. " + - "tenant={TenantId} session={SessionId} state={State}", - tenantId, sessionId, refreshed.DeletionState); - var poisonedHint = string.Equals(refreshed.DeletionState, SessionDeletionState.Poisoned, StringComparison.Ordinal); - return await WriteResponse(req, HttpStatusCode.Conflict, new - { - success = false, - message = poisonedHint - ? "Cascade is poisoned; recover via POST /api/global/sessions/{id}/restore before retrying delete." - : "A V2 cascade owns this session; retry later or use the cascade-delete path.", - deletionState = refreshed.DeletionState, - manifestId = refreshed.PendingDeletionManifestId, - hint = poisonedHint ? "cascade_poisoned_use_restore" : "cascade_already_in_flight", - }); - } - _logger.LogError($"Legacy delete failed for session {sessionId} without an identifiable race cause"); - return await WriteResponse(req, HttpStatusCode.InternalServerError, new - { - success = false, - message = "Internal server error", - }); - } - - // Sessions row is gone — no V2 producer can claim it now. Safe to cascade-clean - // side tables without ordering races. - var eventsDeleted = await _maintenanceRepo.DeleteSessionEventsAsync(tenantId, sessionId); - _logger.LogInformation($"Deleted {eventsDeleted} events for session {sessionId}"); - - var ruleResultsDeleted = await _maintenanceRepo.DeleteSessionRuleResultsAsync(tenantId, sessionId); - _logger.LogInformation($"Deleted {ruleResultsDeleted} rule results for session {sessionId}"); - - var appSummariesDeleted = await _maintenanceRepo.DeleteSessionAppInstallSummariesAsync(tenantId, sessionId); - _logger.LogInformation($"Deleted {appSummariesDeleted} app install summaries for session {sessionId}"); - - await _maintenanceRepo.LogAuditEntryAsync( - tenantId, - "DELETE", - "Session", - sessionId, - userIdentifier, - new Dictionary - { - { "EventsDeleted", eventsDeleted.ToString() }, - { "RuleResultsDeleted", ruleResultsDeleted.ToString() }, - { "AppInstallSummariesDeleted", appSummariesDeleted.ToString() } - }); - - _logger.LogInformation($"Successfully deleted session {sessionId}"); - return await WriteResponse(req, HttpStatusCode.OK, new - { - success = true, - message = $"Session {sessionId} deleted successfully", - eventsDeleted, - ruleResultsDeleted, - appInstallSummariesDeleted = appSummariesDeleted, - }); - } - private static async Task WriteResponse(HttpRequestData req, HttpStatusCode status, object body) { var response = req.CreateResponse(status); diff --git a/src/Backend/AutopilotMonitor.Functions/Services/Deletion/SessionDeletionWorker.cs b/src/Backend/AutopilotMonitor.Functions/Services/Deletion/SessionDeletionWorker.cs index 0af95d97..2d9324ae 100644 --- a/src/Backend/AutopilotMonitor.Functions/Services/Deletion/SessionDeletionWorker.cs +++ b/src/Backend/AutopilotMonitor.Functions/Services/Deletion/SessionDeletionWorker.cs @@ -37,13 +37,6 @@ namespace AutopilotMonitor.Functions.Services.Deletion /// The Sessions row's DeletionState is NOT auto-cleared — operator action via /// restore-from-poisoned (§13, PR4b) is required. /// - /// - /// The worker is NOT registered in Program.cs in PR4 — that wires up in PR5 - /// alongside the flag-gated DeleteSessionFunction changes. Without the producer - /// emitting envelopes (gated by TenantConfiguration.EnableCascadeDeleteV2, default - /// false), having the worker idle on an empty queue is harmless; we hold off so the worker's - /// poll-loop doesn't appear in production telemetry until the cutover. - /// /// public sealed class SessionDeletionWorker : BackgroundService { diff --git a/src/Backend/AutopilotMonitor.Functions/Services/Deletion/SessionRetentionFanoutService.cs b/src/Backend/AutopilotMonitor.Functions/Services/Deletion/SessionRetentionFanoutService.cs index b9b3233f..39a9c26f 100644 --- a/src/Backend/AutopilotMonitor.Functions/Services/Deletion/SessionRetentionFanoutService.cs +++ b/src/Backend/AutopilotMonitor.Functions/Services/Deletion/SessionRetentionFanoutService.cs @@ -10,25 +10,17 @@ using Microsoft.Extensions.Logging; namespace AutopilotMonitor.Functions.Services.Deletion { /// - /// Per-tenant retention fanout for cascade-delete (plan §5 PR6 / §16 R14). Replaces the - /// session-retention loop previously embedded in - /// MaintenanceService.CleanupOldDataAsync: each tenant's - /// DataRetentionDays is read; sessions older than the cutoff are dispatched per the - /// per-tenant flag. - /// - /// Flag ON: with - /// reason="retention_cutoff". The producer's own CAS-then-build-then-enqueue - /// handles the DeletionState != None case (returns AlreadyInFlight). - /// Flag OFF: legacy direct-delete via the existing - /// + - /// helpers. PR5 finding 2 built the CAS guard into DeleteSessionAsync itself, so - /// a half-completed V2 cascade is never overrun even on the legacy path. - /// + /// Per-tenant retention fanout for cascade-delete. Replaces the session-retention loop + /// previously embedded in MaintenanceService.CleanupOldDataAsync: each tenant's + /// DataRetentionDays is read; sessions older than the cutoff are dispatched to + /// with reason="retention_cutoff". + /// The producer's own CAS-then-build-then-enqueue handles the DeletionState != None + /// case (returns AlreadyInFlight). /// - /// Rate limit (plan §5 PR6): at most dispatches - /// per tenant per invocation, with between successive - /// dispatches. Bounds the cost of a maintenance fan-out when a tenant has months of backlog - /// behind a freshly-shortened retention setting. + /// Rate limit: at most dispatches per tenant + /// per invocation, with between successive dispatches. + /// Bounds the cost of a maintenance fan-out when a tenant has months of backlog behind a + /// freshly-shortened retention setting. /// /// public class SessionRetentionFanoutService @@ -37,7 +29,6 @@ namespace AutopilotMonitor.Functions.Services.Deletion public static readonly TimeSpan EnqueueThrottleDelay = TimeSpan.FromMilliseconds(50); private readonly IMaintenanceRepository _maintenanceRepo; - private readonly ISessionRepository _sessionRepo; private readonly TenantConfigurationService _tenantConfig; private readonly ISessionDeletionEnqueuer _enqueuer; private readonly AdminConfigurationService _adminConfig; @@ -46,12 +37,11 @@ namespace AutopilotMonitor.Functions.Services.Deletion public SessionRetentionFanoutService( IMaintenanceRepository maintenanceRepo, - ISessionRepository sessionRepo, TenantConfigurationService tenantConfig, ISessionDeletionEnqueuer enqueuer, AdminConfigurationService adminConfig, ILogger logger) - : this(maintenanceRepo, sessionRepo, tenantConfig, enqueuer, adminConfig, logger, throttle: Task.Delay) + : this(maintenanceRepo, tenantConfig, enqueuer, adminConfig, logger, throttle: Task.Delay) { } @@ -61,7 +51,6 @@ namespace AutopilotMonitor.Functions.Services.Deletion /// internal SessionRetentionFanoutService( IMaintenanceRepository maintenanceRepo, - ISessionRepository sessionRepo, TenantConfigurationService tenantConfig, ISessionDeletionEnqueuer enqueuer, AdminConfigurationService adminConfig, @@ -69,7 +58,6 @@ namespace AutopilotMonitor.Functions.Services.Deletion Func throttle) { _maintenanceRepo = maintenanceRepo; - _sessionRepo = sessionRepo; _tenantConfig = tenantConfig; _enqueuer = enqueuer; _adminConfig = adminConfig; @@ -84,11 +72,10 @@ namespace AutopilotMonitor.Functions.Services.Deletion public sealed class FanoutResult { public int TenantsProcessed { get; set; } - public int SessionsEnqueued { get; set; } // V2 path: enqueued for cascade - public int SessionsLegacyDeleted { get; set; } // flag-OFF path: direct-delete succeeded + public int SessionsEnqueued { get; set; } // enqueued for cascade public int SessionsSkipped { get; set; } // already locked / poisoned / kill-switch / etc. public int RateLimitedTenants { get; set; } // tenants that hit MaxEnqueuesPerTenantPerRun - public bool AbortedByKillSwitch { get; set; } // PR6 follow-up F2: kill-switch flipped mid-run + public bool AbortedByKillSwitch { get; set; } // kill-switch flipped mid-run } /// @@ -106,8 +93,8 @@ namespace AutopilotMonitor.Functions.Services.Deletion { cancellationToken.ThrowIfCancellationRequested(); - // PR6 follow-up F2: per-tenant kill-switch check so a flip-ON mid-run halts the - // remaining tenants. The maintenance function gates entry; this gates iteration. + // Per-tenant kill-switch check so a flip-ON mid-run halts the remaining tenants. + // The maintenance function gates entry; this gates iteration. if (await _adminConfig.IsSessionDeletionKillSwitchActiveAsync().ConfigureAwait(false)) { _logger.LogWarning( @@ -130,6 +117,17 @@ namespace AutopilotMonitor.Functions.Services.Deletion { _logger.LogError(ex, "SessionRetentionFanout failed for tenant {TenantId} — continuing with next tenant", tenantId); } + + // The inner loop sets this when the producer's Step 0 reports KillSwitchActive + // mid-tenant. That's the authoritative fail-closed outcome — stop touching the + // remaining tenants instead of relying on the next pre-check to catch the flip. + if (result.AbortedByKillSwitch) + { + _logger.LogWarning( + "SessionRetentionFanout: aborting outer loop after tenant {TenantId} reported KillSwitchActive from producer", + tenantId); + break; + } } return result; @@ -155,11 +153,8 @@ namespace AutopilotMonitor.Functions.Services.Deletion return; } - var useV2Cascade = config?.EnableCascadeDeleteV2 == true; - int processed = 0; int enqueued = 0; - int legacyDeleted = 0; int skipped = 0; foreach (var session in oldSessions) @@ -175,9 +170,9 @@ namespace AutopilotMonitor.Functions.Services.Deletion break; } - // PR6 follow-up F2: per-session kill-switch check so an emergency flip-ON halts - // immediately instead of after the rest of this tenant's backlog. Uncached read - // (PR5 F1) — uniform behaviour across scaled-out instances within seconds. + // Per-session kill-switch check so an emergency flip-ON halts immediately instead + // of after the rest of this tenant's backlog. Uncached read — uniform behaviour + // across scaled-out instances within seconds. if (await _adminConfig.IsSessionDeletionKillSwitchActiveAsync().ConfigureAwait(false)) { _logger.LogWarning( @@ -187,29 +182,35 @@ namespace AutopilotMonitor.Functions.Services.Deletion break; } - if (useV2Cascade) + var outcome = await EnqueueAsync(session, cancellationToken).ConfigureAwait(false); + + // The producer's own kill-switch check (Step 0) flipped between our pre-check + // and the producer call. Abort the inner loop AND mark the fanout aborted so + // the outer loop also stops — otherwise we'd keep pushing sessions through a + // producer that's already 503'ing every call. + if (outcome == SessionDeletionEnqueueOutcome.KillSwitchActive) { - var outcome = await EnqueueV2Async(session, cancellationToken).ConfigureAwait(false); - if (outcome == SessionDeletionEnqueueOutcome.Enqueued) enqueued++; - else skipped++; - } - else - { - if (await LegacyDeleteAsync(session, cancellationToken).ConfigureAwait(false)) legacyDeleted++; - else skipped++; + _logger.LogWarning( + "SessionRetentionFanout: producer reported KillSwitchActive — aborting at session {SessionId} of {TenantId}", + session.SessionId, tenantId); + skipped++; + result.AbortedByKillSwitch = true; + break; } + if (outcome == SessionDeletionEnqueueOutcome.Enqueued) enqueued++; + else skipped++; + processed++; - // PR6 R14: rate-limit pacing — bounds the cost of a fanout when a tenant has months - // of backlog (e.g. retention freshly shortened). Throttle is injected so unit tests + // Rate-limit pacing — bounds the cost of a fanout when a tenant has months of + // backlog (e.g. retention freshly shortened). Throttle is injected so unit tests // can exercise the loop without waiting real time. if (processed < oldSessions.Count && processed < MaxEnqueuesPerTenantPerRun) await _throttle(EnqueueThrottleDelay, cancellationToken).ConfigureAwait(false); } result.SessionsEnqueued += enqueued; - result.SessionsLegacyDeleted += legacyDeleted; result.SessionsSkipped += skipped; await _maintenanceRepo.LogAuditEntryAsync( @@ -220,27 +221,26 @@ namespace AutopilotMonitor.Functions.Services.Deletion "System.Maintenance", new Dictionary { - { "Path", useV2Cascade ? "V2Cascade" : "LegacyDirectDelete" }, { "RetentionDays", retentionDays.ToString() }, { "CutoffUtc", cutoffUtc.ToString("o") }, { "Eligible", oldSessions.Count.ToString() }, { "Enqueued", enqueued.ToString() }, - { "LegacyDeleted", legacyDeleted.ToString() }, { "Skipped", skipped.ToString() }, { "AbortedByKillSwitch", result.AbortedByKillSwitch.ToString() }, }).ConfigureAwait(false); _logger.LogInformation( - "Tenant {TenantId}: retention fanout — path={Path} cutoff={Cutoff:o} eligible={Eligible} enqueued={Enqueued} legacy={Legacy} skipped={Skipped}", - tenantId, useV2Cascade ? "V2Cascade" : "LegacyDirectDelete", cutoffUtc, oldSessions.Count, enqueued, legacyDeleted, skipped); + "Tenant {TenantId}: retention fanout — cutoff={Cutoff:o} eligible={Eligible} enqueued={Enqueued} skipped={Skipped}", + tenantId, cutoffUtc, oldSessions.Count, enqueued, skipped); } /// - /// V2 enqueue path. The producer handles CAS-Preparing, manifest build, blob upload, and - /// queue send; we just translate the outcome into a counter bump (or skip) and log the - /// reason. Maintenance-side audits (deletion_started) are written by the producer. + /// Enqueue a cascade for one session. The producer handles CAS-Preparing, manifest build, + /// blob upload, and queue send; we just translate the outcome into a counter bump (or + /// skip) and log the reason. Maintenance-side audits (deletion_started) are + /// written by the producer. /// - private async Task EnqueueV2Async(SessionSummary session, CancellationToken cancellationToken) + private async Task EnqueueAsync(SessionSummary session, CancellationToken cancellationToken) { var actor = new DeletionActor { Type = "maintenance", Actor = "System.Maintenance" }; var result = await _enqueuer.EnqueueAsync( @@ -284,52 +284,5 @@ namespace AutopilotMonitor.Functions.Services.Deletion return result.Outcome; } } - - /// - /// Legacy direct-delete path — invoked when the tenant has not yet been migrated to the V2 - /// cascade (per-tenant flag OFF). Sequence: - /// - /// Pre-read the Sessions row (PR6 follow-up F1). If DeletionState != None - /// the V2 cascade owns the session — return false without touching ANY side table. - /// Without this gate, we would delete Events / RuleResults / AppInstallSummaries - /// and only discover the lock at the tombstone CAS, leaving the V2 cascade with a - /// half-eaten session. - /// Delete events / rule-results / app-summaries. - /// Delete the Sessions row via the CAS-guarded - /// (PR5 finding 2) — second-line - /// protection against the T1-T3 race where another instance CAS-claimed the row - /// after our pre-read but before our side-table mutations. - /// - /// - private async Task LegacyDeleteAsync(SessionSummary session, CancellationToken cancellationToken) - { - cancellationToken.ThrowIfCancellationRequested(); - var tenantId = session.TenantId; - var sessionId = session.SessionId; - - // Sessions-first: the ETag-CAS-guarded DeleteSessionAsync (PR5 finding 2) is the lock - // arbiter against in-flight V2 cascades. Running it BEFORE side-table mutations means - // a concurrent V2 producer either claims the row first (we get false → skip side - // tables, V2 owns the cascade) or we win (Sessions gone → no producer can claim → - // side-tables safe to cascade-clean). The previous side-tables-first ordering left - // a TOCTOU window where V2's manifest snapshot landed on top of an already-half- - // purged side-table state. - var deleted = await _sessionRepo.DeleteSessionAsync(tenantId, sessionId).ConfigureAwait(false); - if (!deleted) - { - _logger.LogInformation( - "Retention fanout (legacy): session {SessionId} not tombstoned — either already gone or owned by a V2 cascade. Side-tables left intact; V2 worker (or next retention run) will absorb", - sessionId); - return false; - } - - // Sessions row is gone — V2 producers cannot claim a missing row. Side-table cleanup - // is now race-free. - _ = await _maintenanceRepo.DeleteSessionEventsAsync(tenantId, sessionId).ConfigureAwait(false); - _ = await _maintenanceRepo.DeleteSessionRuleResultsAsync(tenantId, sessionId).ConfigureAwait(false); - _ = await _maintenanceRepo.DeleteSessionAppInstallSummariesAsync(tenantId, sessionId).ConfigureAwait(false); - - return true; - } } } diff --git a/src/Backend/AutopilotMonitor.Functions/Services/MaintenanceService.Aggregation.cs b/src/Backend/AutopilotMonitor.Functions/Services/MaintenanceService.Aggregation.cs index 94964926..3a0a4338 100644 --- a/src/Backend/AutopilotMonitor.Functions/Services/MaintenanceService.Aggregation.cs +++ b/src/Backend/AutopilotMonitor.Functions/Services/MaintenanceService.Aggregation.cs @@ -189,10 +189,9 @@ namespace AutopilotMonitor.Functions.Services // Plan §5 PR6 / §16 R14: the session retention loop that previously lived here is now // owned by SessionDeletionMaintenanceFunction (12h cadence, dedicated watchdog OpsEvents, - // V2 cascade vs legacy direct-delete dispatch via SessionRetentionFanoutService). The - // non-session tail of this method (UserUsageLog + RuleStats cleanup) was already a - // separate method (CleanupOldUsageDataAsync) and is now called directly from RunAllAsync - // and RunManualAsync. + // cascade-delete dispatch via SessionRetentionFanoutService). The non-session tail of + // this method (UserUsageLog + RuleStats cleanup) was already a separate method + // (CleanupOldUsageDataAsync) and is now called directly from RunAllAsync and RunManualAsync. /// /// Reconciles rule stats for a given date by computing global aggregate rows diff --git a/src/Backend/AutopilotMonitor.Functions/Services/OpsEventService.cs b/src/Backend/AutopilotMonitor.Functions/Services/OpsEventService.cs index 4a7371e0..52a0fbd4 100644 --- a/src/Backend/AutopilotMonitor.Functions/Services/OpsEventService.cs +++ b/src/Backend/AutopilotMonitor.Functions/Services/OpsEventService.cs @@ -153,13 +153,13 @@ namespace AutopilotMonitor.Functions.Services /// silently failed because the AuditLogs schema requires a non-null PartitionKey (tenantId). /// public Task RecordSessionDeletionMaintenanceCompletedAsync( - bool killSwitchActive, int tenantsProcessed, int sessionsEnqueued, int sessionsLegacyDeleted, + bool killSwitchActive, int tenantsProcessed, int sessionsEnqueued, int sessionsSkipped, int rateLimitedTenants, int blobsTtlGced, int preparingRowsCleared, int strandedQueuedDetected, int durationMs, bool abortedByKillSwitch) => WriteAsync(OpsEventCategory.Maintenance, "SessionDeletionMaintenanceCompleted", OpsEventSeverity.Info, - $"SessionDeletionMaintenance completed in {durationMs}ms — tenants={tenantsProcessed} enqueued={sessionsEnqueued} legacy={sessionsLegacyDeleted} skipped={sessionsSkipped} blobsTtlGced={blobsTtlGced} preparingCleared={preparingRowsCleared} stranded={strandedQueuedDetected} killSwitch={killSwitchActive}", + $"SessionDeletionMaintenance completed in {durationMs}ms — tenants={tenantsProcessed} enqueued={sessionsEnqueued} skipped={sessionsSkipped} blobsTtlGced={blobsTtlGced} preparingCleared={preparingRowsCleared} stranded={strandedQueuedDetected} killSwitch={killSwitchActive}", null, "System.Maintenance", new { - killSwitchActive, tenantsProcessed, sessionsEnqueued, sessionsLegacyDeleted, + killSwitchActive, tenantsProcessed, sessionsEnqueued, sessionsSkipped, rateLimitedTenants, blobsTtlGced, preparingRowsCleared, strandedQueuedDetected, durationMs, abortedByKillSwitch, }); diff --git a/src/Backend/AutopilotMonitor.Functions/Services/TableStorageService.Deletion.cs b/src/Backend/AutopilotMonitor.Functions/Services/TableStorageService.Deletion.cs index d8c931ac..e3e8cd23 100644 --- a/src/Backend/AutopilotMonitor.Functions/Services/TableStorageService.Deletion.cs +++ b/src/Backend/AutopilotMonitor.Functions/Services/TableStorageService.Deletion.cs @@ -507,8 +507,8 @@ namespace AutopilotMonitor.Functions.Services /// Enumerates every row in with /// PartitionKey == {tenantId}_{sessionId} and deletes them via /// . Used for PK_BY_SESSION-class cascade - /// targets when the caller does not yet have a manifest in hand (e.g. legacy direct-delete - /// paths or test fixtures); the §1 P6 cascade worker passes manifest keys to + /// targets when the caller does not yet have a manifest in hand (e.g. test fixtures); + /// the §1 P6 cascade worker passes manifest keys to /// directly. /// private async Task DeletePkBySessionAsync( diff --git a/src/Backend/AutopilotMonitor.Functions/Services/TableStorageService.Maintenance.cs b/src/Backend/AutopilotMonitor.Functions/Services/TableStorageService.Maintenance.cs index ab7b46f6..7c110175 100644 --- a/src/Backend/AutopilotMonitor.Functions/Services/TableStorageService.Maintenance.cs +++ b/src/Backend/AutopilotMonitor.Functions/Services/TableStorageService.Maintenance.cs @@ -594,20 +594,6 @@ namespace AutopilotMonitor.Functions.Services return await DeleteByFilterInBatchesAsync(tableClient, filter, $"rule results for session {sessionId}"); } - /// - /// Deletes all app install summaries for a session - /// - public async Task DeleteSessionAppInstallSummariesAsync(string tenantId, string sessionId) - { - SecurityValidator.EnsureValidGuid(tenantId, nameof(tenantId)); - SecurityValidator.EnsureValidGuid(sessionId, nameof(sessionId)); - - var tableClient = _tableServiceClient.GetTableClient(Constants.TableNames.AppInstallSummaries); - // AppInstallSummaries PK=tenantId, so all filtered rows still share the same PK → batch-tx-safe. - var filter = $"PartitionKey eq '{tenantId}' and SessionId eq '{sessionId}'"; - return await DeleteByFilterInBatchesAsync(tableClient, filter, $"app install summaries for session {sessionId}"); - } - /// /// Deletes all entities matching the filter from the given table. /// Uses projected query (PK/RK only) to minimize payload, and submits 100-entity diff --git a/src/Backend/AutopilotMonitor.Functions/Services/TableStorageService.Sessions.cs b/src/Backend/AutopilotMonitor.Functions/Services/TableStorageService.Sessions.cs index 91444378..5b4a2801 100644 --- a/src/Backend/AutopilotMonitor.Functions/Services/TableStorageService.Sessions.cs +++ b/src/Backend/AutopilotMonitor.Functions/Services/TableStorageService.Sessions.cs @@ -172,29 +172,6 @@ namespace AutopilotMonitor.Functions.Services } } - /// - /// Deletes a session entry from the SessionsIndex table. - /// - private async Task DeleteSessionIndexAsync(string tenantId, string? indexRowKey) - { - if (string.IsNullOrEmpty(indexRowKey)) - return; - - try - { - var indexTableClient = _tableServiceClient.GetTableClient(Constants.TableNames.SessionsIndex); - await indexTableClient.DeleteEntityAsync(tenantId, indexRowKey); - } - catch (RequestFailedException ex) when (ex.Status == 404) - { - // Already gone — fine - } - catch (Exception ex) - { - _logger.LogWarning(ex, "Failed to delete session index for {TenantId}/{IndexRowKey}", tenantId, indexRowKey); - } - } - /// /// Maps an index entity (from SessionsIndex table) to SessionSummary. /// The key difference from MapToSessionSummary: SessionId comes from a stored property @@ -2033,93 +2010,6 @@ namespace AutopilotMonitor.Functions.Services } } - /// - /// Deletes a session from storage (legacy direct-delete tombstone, PR5 finding 2). - /// - /// CAS-guarded against the V2 cascade lock: the read selects DeletionState + - /// captures the ETag; if the row is in Preparing/Queued/Running/Poisoned the - /// delete is refused (returns false → caller surfaces 409). The subsequent - /// DeleteEntityAsync passes the captured ETag so a concurrent V2 producer that - /// CAS-set DeletionState=Preparing between our read and our write loses the race - /// (412 → returns false). Without this guard, scaled-out instances with stale - /// tenant-config cache could legacy-delete a session whose V2 cascade is already in - /// flight, orphaning every side-table row the cascade was supposed to clean up. - /// - /// - /// The V2 worker's tombstone uses DeleteByExactKeysInBatchesAsync (a different - /// code path), so this CAS guard does NOT block the worker — only legacy callers - /// (DeleteSessionFunction flag-OFF path + MaintenanceService retention - /// fanout). - /// - /// - public async Task DeleteSessionAsync(string tenantId, string sessionId) - { - try - { - var tableClient = _tableServiceClient.GetTableClient(Constants.TableNames.Sessions); - - // Read IndexRowKey + DeletionState + ETag in one go. PR5 finding 2: we need - // ALL three to (a) cascade-delete the SessionsIndex row, (b) refuse if locked, - // (c) ETag-CAS on the actual delete to close the read-write race. - string? indexRowKey; - string? deletionState; - ETag etag; - try - { - var entity = await tableClient.GetEntityAsync(tenantId, sessionId, - select: new[] { "IndexRowKey", "DeletionState" }); - indexRowKey = entity.Value.GetString("IndexRowKey"); - deletionState = entity.Value.GetString("DeletionState"); - etag = entity.Value.ETag; - } - catch (RequestFailedException ex) when (ex.Status == 404) - { - // Session already gone — nothing to delete - return true; - } - - if (SessionDeletionState.IsLocked(deletionState)) - { - _logger.LogWarning( - "Legacy DeleteSessionAsync refused: session is locked by a V2 cascade. " + - "tenant={TenantId} session={SessionId} state={State}", - tenantId, sessionId, deletionState); - return false; - } - - // ETag-conditional delete: closes the T2-T4 race where a parallel V2 producer - // CAS-set DeletionState=Preparing between our DeletionState read and this write. - try - { - await tableClient.DeleteEntityAsync(tenantId, sessionId, etag); - } - catch (RequestFailedException ex) when (ex.Status == 412) - { - _logger.LogWarning( - "Legacy DeleteSessionAsync ETag-CAS lost: Sessions row was mutated by a concurrent writer " + - "(likely a V2 cascade producer CAS-claiming DeletionState=Preparing). tenant={TenantId} session={SessionId}", - tenantId, sessionId); - return false; - } - catch (RequestFailedException ex) when (ex.Status == 404) - { - // Lost a race with another deleter that also got 200 — treat as idempotent success. - indexRowKey = null; - } - - // Dual-write: delete from SessionsIndex - await DeleteSessionIndexAsync(tenantId, indexRowKey); - - _logger.LogInformation($"Deleted session {sessionId} for tenant {tenantId}"); - return true; - } - catch (Exception ex) - { - _logger.LogError(ex, $"Failed to delete session {sessionId}"); - return false; - } - } - // ===== SESSION/EVENT MAPPING HELPERS ===== /// diff --git a/src/Shared/AutopilotMonitor.Shared/DataAccess/IMaintenanceRepository.cs b/src/Shared/AutopilotMonitor.Shared/DataAccess/IMaintenanceRepository.cs index 14d23249..889b8abf 100644 --- a/src/Shared/AutopilotMonitor.Shared/DataAccess/IMaintenanceRepository.cs +++ b/src/Shared/AutopilotMonitor.Shared/DataAccess/IMaintenanceRepository.cs @@ -54,7 +54,6 @@ namespace AutopilotMonitor.Shared.DataAccess // --- Cleanup --- Task DeleteSessionEventsAsync(string tenantId, string sessionId); Task DeleteSessionRuleResultsAsync(string tenantId, string sessionId); - Task DeleteSessionAppInstallSummariesAsync(string tenantId, string sessionId); // --- Index Maintenance --- Task BackfillSessionIndexAsync(); diff --git a/src/Shared/AutopilotMonitor.Shared/DataAccess/ISessionRepository.cs b/src/Shared/AutopilotMonitor.Shared/DataAccess/ISessionRepository.cs index 3ee2e64d..bc9a1ae1 100644 --- a/src/Shared/AutopilotMonitor.Shared/DataAccess/ISessionRepository.cs +++ b/src/Shared/AutopilotMonitor.Shared/DataAccess/ISessionRepository.cs @@ -61,8 +61,6 @@ namespace AutopilotMonitor.Shared.DataAccess /// Task GetAllSessionStatsAsync(string? tenantIdFilter, int days); - Task DeleteSessionAsync(string tenantId, string sessionId); - // --- Session Updates --- Task UpdateSessionStatusAsync( string tenantId, string sessionId, SessionStatus status, diff --git a/src/Shared/AutopilotMonitor.Shared/Models/Config/AdminConfiguration.cs b/src/Shared/AutopilotMonitor.Shared/Models/Config/AdminConfiguration.cs index d097c081..101b3aaf 100644 --- a/src/Shared/AutopilotMonitor.Shared/Models/Config/AdminConfiguration.cs +++ b/src/Shared/AutopilotMonitor.Shared/Models/Config/AdminConfiguration.cs @@ -409,15 +409,12 @@ namespace AutopilotMonitor.Shared.Models /// Global emergency kill-switch for the cascade-deletion subsystem (Plan §1 P8 / §9). /// When true: /// - /// V2 cascade producers return 503 Service Unavailable; - /// legacy direct-delete paths also return 503 (so the switch is truly global); - /// the cascade worker (PR4) returns its message to the queue on entry without + /// cascade producers return 503 Service Unavailable; + /// the cascade worker returns its message to the queue on entry without /// processing. /// - /// Independent of per-tenant : - /// flag selects path, kill-switch halts everything. Round-tripped via the 4-file web - /// chain (memory feedback_admin_config_ui_roundtrip) so admin saves preserve it. - /// Default false. + /// Round-tripped via the 4-file web chain (memory feedback_admin_config_ui_roundtrip) + /// so admin saves preserve it. Default false. /// public bool SessionDeletionKillSwitch { get; set; } = false; diff --git a/src/Shared/AutopilotMonitor.Shared/Models/Config/TenantConfiguration.cs b/src/Shared/AutopilotMonitor.Shared/Models/Config/TenantConfiguration.cs index 6f66b370..ba021b41 100644 --- a/src/Shared/AutopilotMonitor.Shared/Models/Config/TenantConfiguration.cs +++ b/src/Shared/AutopilotMonitor.Shared/Models/Config/TenantConfiguration.cs @@ -137,16 +137,6 @@ namespace AutopilotMonitor.Shared.Models /// public int DataRetentionDays { get; set; } = 90; - /// - /// Per-tenant rollout flag for the V2 cascade-delete pipeline (Plan §1 P8 / §5 PR3 / - /// §9). When true, admin-delete and retention-cleanup paths build a manifest + - /// enqueue to the session-deletion queue; the worker (PR4) executes the cascade. - /// When false (default), the legacy direct-delete path runs for this tenant. - /// Per-tenant flag selects which PATH; the global - /// halts BOTH paths. - /// - public bool EnableCascadeDeleteV2 { get; set; } = false; - /// /// Session timeout in hours /// Sessions in "InProgress" status longer than this will be marked as "Failed - Timed Out" diff --git a/src/Web/autopilot-monitor-web/app/admin/components/AdminConfigSettingsSection.tsx b/src/Web/autopilot-monitor-web/app/admin/components/AdminConfigSettingsSection.tsx index 8c87dc9f..ca74544a 100644 --- a/src/Web/autopilot-monitor-web/app/admin/components/AdminConfigSettingsSection.tsx +++ b/src/Web/autopilot-monitor-web/app/admin/components/AdminConfigSettingsSection.tsx @@ -246,13 +246,10 @@ export function AdminConfigSettingsSection({ Cascade-Delete Kill-Switch (emergency)

- Global emergency stop for the V2 cascade-deletion subsystem. When on: the cascade producer - returns 503 Service Unavailable, - the legacy direct-delete path also returns 503 (so the switch is truly global, not just V2), + Global emergency stop for the cascade-deletion subsystem. When on: the cascade producer + returns 503 Service Unavailable and the cascade worker pauses on entry. Flip on if a runaway cascade or storage incident requires - halting all session-delete activity across all tenants. Independent of the per-tenant - EnableCascadeDeleteV2 flag — - that selects the path, this halts both paths. Default off. + halting all session-delete activity across all tenants. Default off.

diff --git a/src/Web/autopilot-monitor-web/app/admin/components/TenantManagementSection.tsx b/src/Web/autopilot-monitor-web/app/admin/components/TenantManagementSection.tsx index dee78e75..67d4d4c6 100644 --- a/src/Web/autopilot-monitor-web/app/admin/components/TenantManagementSection.tsx +++ b/src/Web/autopilot-monitor-web/app/admin/components/TenantManagementSection.tsx @@ -21,7 +21,6 @@ export interface TenantConfiguration { bootstrapTokenEnabled?: boolean; unrestrictedModeEnabled?: boolean; dataRetentionDays: number; - enableCascadeDeleteV2?: boolean; sessionTimeoutHours: number; planTier?: string; } @@ -352,14 +351,6 @@ export function TenantManagementSection({ Ready )} - {tenant.enableCascadeDeleteV2 && ( - - Cascade V2 - - )}