fix(rules): retry-safe sunset GC + idempotent point-deletes

Codex review of the sunset orphan-GC introduced in dff5ba65 surfaced three
issues; all three are valid and now fixed:

(Medium) The previous sequence was "delete global rule first, then GC
per-tenant RuleStates". If the GC failed partially (per-row error) or
entirely (cross-partition enumeration error), the global rule was already
gone — the sunset diff went empty on the next seed cycle and the surviving
orphan RuleStates would linger indefinitely. Worst case for a future
default-ENABLED sunset rule: tenants that had explicitly disabled it would
lose the disable, silently re-enabling something they didn't want.

Three coordinated fixes:

1. Storage methods made idempotent: DeleteAnalyzeRuleAsync and
   DeleteRuleStateAsync now treat Azure 404 (row already gone) as success
   instead of returning false. The 404-on-individual-row case inside the
   cross-tenant GC enumeration is also counted as success — a row that
   was raced-deleted between the query and our delete is a clean state,
   not a failure.

2. Cross-tenant GC return shape changed from `int` to `(int deleted,
   int failed)`:
     - `deleted` counts fresh deletes AND 404s (idempotent retries)
     - `failed > 0` signals at least one per-row delete threw a non-404
     - `failed == -1` signals the cross-partition enumeration itself
       failed — we genuinely don't know how many orphans are still out
       there
   Callers must check `failed != 0` before proceeding to the global
   delete. The integer-only return shape couldn't distinguish "GC found
   nothing" from "GC failed silently".

3. Caller sequence inverted to GC-FIRST in all three sunset code paths:
     - AnalyzeRuleService.EnsureBuiltInRulesSeededAsync (on-startup seed)
     - AnalyzeRuleService.ReseedBuiltInRulesAsync (explicit reseed)
     - ReseedFromGitHubFunction.ReseedAnalyzeAsync (admin UI button)
   GC is attempted first. The global rule is deleted ONLY when GC returns
   `failed == 0`. On partial GC failure the rule stays in the global
   catalog so the next seed/reseed cycle sees it in the sunset diff
   again and re-attempts the cleanup. End state is eventually consistent
   even across transient Azure errors.

(Low) The GitHub-reseed HTTP response shape was extended from
`{ deleted, written }` to `{ deleted, written, orphanStatesGcd,
sunsetSkipped }` so operators can see the cleanup count + any rules
that got skipped for the retry loop.

Tests:
- 3 new sunset-mechanism tests covering partial GC failure, enumeration
  failure (-1 sentinel), and the still-living-rule no-touch invariant.
- Existing ReseedBuiltIn_returns_orphanGc_count_in_tuple updated to the
  new helper shape.
- ReseedBuiltIn_skips_global_delete_on_partial_GC_failure pins the
  retry-safe semantics in the explicit reseed path.

Coverage: backend 2076/2076 (+3 sunset edge-case tests).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

src/Backend/AutopilotMonitor.Functions.Tests/AnalyzeRuleServiceSunsetTests.cs

@@ -26,19 +26,50 @@ public class AnalyzeRuleServiceSunsetTests
         // can't easily inject a fake catalog, so we use a ruleId that the real catalog
         // doesn't contain). Plus the rule has TWO per-tenant RuleState overrides that
         // would normally keep it firing.
-        var (ruleRepo, deleteAnalyzeCalls, gcCalls) = BuildRepoWithSunsetRule(SunsetRuleId, orphanCount: 2);
+        var (ruleRepo, deleteAnalyzeCalls, gcCalls) = BuildRepoWithSunsetRule(SunsetRuleId, gcDeleted: 2, gcFailed: 0);
 
         var service = new AnalyzeRuleService(ruleRepo.Object, NullLogger<AnalyzeRuleService>.Instance);
 
         // Act: a regular GetAllRulesForTenantAsync triggers EnsureBuiltInRulesSeededAsync.
         await service.GetAllRulesForTenantAsync("a1b2c3d4-e5f6-7890-abcd-ef1234567890");
 
-        // Assert: the sunset rule's global row got deleted AND its per-tenant orphan
+        // Assert: the sunset rule's per-tenant orphan RuleStates got GC'd AND its
-        // RuleStates got GC'd via the cross-tenant cleanup method.
+        // global row got deleted. The order matters (GC first), but for a clean GC
+        // the end-state is the same: both call sites recorded.
         Assert.Contains(SunsetRuleId, deleteAnalyzeCalls);
         Assert.Contains(SunsetRuleId, gcCalls);
     }
 
+    [Fact]
+    public async Task EnsureSeed_partial_GC_failure_keeps_global_row_so_retry_works()
+    {
+        // Codex review (Medium): if GC reports >0 failed rows, the global delete must NOT
+        // run — otherwise the rule falls out of the sunset-diff and the remaining orphan
+        // RuleState rows are unreachable. With the global row kept, the next startup will
+        // see the rule in the diff again and re-attempt the GC.
+        var (ruleRepo, deleteAnalyzeCalls, gcCalls) = BuildRepoWithSunsetRule(SunsetRuleId, gcDeleted: 1, gcFailed: 1);
+
+        var service = new AnalyzeRuleService(ruleRepo.Object, NullLogger<AnalyzeRuleService>.Instance);
+        await service.GetAllRulesForTenantAsync("a1b2c3d4-e5f6-7890-abcd-ef1234567890");
+
+        Assert.Contains(SunsetRuleId, gcCalls);             // GC was attempted
+        Assert.DoesNotContain(SunsetRuleId, deleteAnalyzeCalls); // global delete SKIPPED on partial GC
+    }
+
+    [Fact]
+    public async Task EnsureSeed_GC_enumeration_failure_keeps_global_row_too()
+    {
+        // The gcFailed == -1 sentinel signals "we couldn't enumerate the RuleStates at
+        // all". Same contract as partial failure: do NOT delete the global row.
+        var (ruleRepo, deleteAnalyzeCalls, gcCalls) = BuildRepoWithSunsetRule(SunsetRuleId, gcDeleted: 0, gcFailed: -1);
+
+        var service = new AnalyzeRuleService(ruleRepo.Object, NullLogger<AnalyzeRuleService>.Instance);
+        await service.GetAllRulesForTenantAsync("a1b2c3d4-e5f6-7890-abcd-ef1234567890");
+
+        Assert.Contains(SunsetRuleId, gcCalls);
+        Assert.DoesNotContain(SunsetRuleId, deleteAnalyzeCalls);
+    }
+
     [Fact]
     public async Task EnsureSeed_does_not_GC_RuleStates_for_rules_still_in_catalog()
     {
@@ -46,7 +77,7 @@ public class AnalyzeRuleServiceSunsetTests
         // its RuleState rows even if tenants have overrides — those are legitimate opt-out
         // / opt-in settings.
         var stillLiveRuleId = BuiltInAnalyzeRules.GetAll().First().RuleId;
-        var (ruleRepo, deleteAnalyzeCalls, gcCalls) = BuildRepoWithSunsetRule(stillLiveRuleId, orphanCount: 1);
+        var (ruleRepo, deleteAnalyzeCalls, gcCalls) = BuildRepoWithSunsetRule(stillLiveRuleId, gcDeleted: 1, gcFailed: 0);
 
         var service = new AnalyzeRuleService(ruleRepo.Object, NullLogger<AnalyzeRuleService>.Instance);
 
@@ -54,8 +85,6 @@ public class AnalyzeRuleServiceSunsetTests
 
         // Live rule's RuleState must NOT be GC'd.
         Assert.DoesNotContain(stillLiveRuleId, gcCalls);
-        // Live rule may have been overwritten by version-bump update logic — that's
-        // fine, only the GC path matters for this invariant.
     }
 
     [Fact]
@@ -64,7 +93,7 @@ public class AnalyzeRuleServiceSunsetTests
         // The new ReseedBuiltInRulesAsync return shape is (deleted, written, orphanStatesGcd)
         // — pin that so external callers (admin UI, future scripts) can surface the
         // cleanup count to the operator.
-        var (ruleRepo, _, _) = BuildRepoWithSunsetRule(SunsetRuleId, orphanCount: 3);
+        var (ruleRepo, _, _) = BuildRepoWithSunsetRule(SunsetRuleId, gcDeleted: 3, gcFailed: 0);
 
         var service = new AnalyzeRuleService(ruleRepo.Object, NullLogger<AnalyzeRuleService>.Instance);
         var (deleted, written, orphanStatesGcd) = await service.ReseedBuiltInRulesAsync();
@@ -74,6 +103,23 @@ public class AnalyzeRuleServiceSunsetTests
         Assert.Equal(3, orphanStatesGcd);
     }
 
+    [Fact]
+    public async Task ReseedBuiltIn_skips_global_delete_on_partial_GC_failure()
+    {
+        // Same invariant as the EnsureSeed test — the explicit Reseed path must also
+        // refuse to drop the global row when GC didn't go all the way through.
+        var (ruleRepo, deleteAnalyzeCalls, gcCalls) = BuildRepoWithSunsetRule(SunsetRuleId, gcDeleted: 2, gcFailed: 1);
+
+        var service = new AnalyzeRuleService(ruleRepo.Object, NullLogger<AnalyzeRuleService>.Instance);
+        var (deleted, written, orphanStatesGcd) = await service.ReseedBuiltInRulesAsync();
+
+        Assert.Contains(SunsetRuleId, gcCalls);
+        Assert.DoesNotContain(SunsetRuleId, deleteAnalyzeCalls);
+        // orphanStatesGcd still surfaces the partial successes (2/3 rows deleted before
+        // the failure) so operators can see progress was made.
+        Assert.Equal(2, orphanStatesGcd);
+    }
+
     // ===== Helpers =====
 
     /// <summary>
@@ -81,14 +127,14 @@ public class AnalyzeRuleServiceSunsetTests
     /// (a) returns the full built-in catalog PLUS one extra rule with the given
     ///     <paramref name="extraRuleId"/> on the "global" partition — so the sunset
     ///     detection has something to find when extraRuleId is not in the catalog;
-    /// (b) reports the configured <paramref name="orphanCount"/> from the cross-tenant
+    /// (b) reports the configured GC result via
-    ///     RuleState GC method.
+    ///     <see cref="IRuleRepository.DeleteRuleStatesForRuleIdAcrossTenantsAsync"/>.
     /// Returns the mock plus two recorder lists capturing which ruleIds got
     /// DeleteAnalyzeRuleAsync and which got DeleteRuleStatesForRuleIdAcrossTenantsAsync
     /// invoked.
     /// </summary>
     private static (Mock<IRuleRepository> repo, List<string> deleteAnalyzeCalls, List<string> gcCalls)
-        BuildRepoWithSunsetRule(string extraRuleId, int orphanCount)
+        BuildRepoWithSunsetRule(string extraRuleId, int gcDeleted, int gcFailed)
     {
         var deleteAnalyzeCalls = new List<string>();
         var gcCalls = new List<string>();
@@ -122,7 +168,7 @@ public class AnalyzeRuleServiceSunsetTests
             .ReturnsAsync(true);
         ruleRepo.Setup(r => r.DeleteRuleStatesForRuleIdAcrossTenantsAsync(It.IsAny<string>()))
             .Callback<string>(ruleId => gcCalls.Add(ruleId))
-            .ReturnsAsync(orphanCount);
+            .ReturnsAsync((gcDeleted, gcFailed));
 
         return (ruleRepo, deleteAnalyzeCalls, gcCalls);
     }

src/Backend/AutopilotMonitor.Functions/DataAccess/TableStorage/TableRuleRepository.cs

@@ -48,7 +48,7 @@ namespace AutopilotMonitor.Functions.DataAccess.TableStorage
         public Task<bool> DeleteRuleStateAsync(string tenantId, string ruleId)
             => _storage.DeleteRuleStateAsync(tenantId, ruleId);
 
-        public Task<int> DeleteRuleStatesForRuleIdAcrossTenantsAsync(string ruleId)
+        public Task<(int deleted, int failed)> DeleteRuleStatesForRuleIdAcrossTenantsAsync(string ruleId)
             => _storage.DeleteRuleStatesForRuleIdAcrossTenantsAsync(ruleId);
 
         public Task<bool> StoreAnalyzeRuleAsync(AnalyzeRule rule, string tenantId = "global")

src/Backend/AutopilotMonitor.Functions/Functions/Admin/ReseedFromGitHubFunction.cs

@@ -64,7 +64,7 @@ namespace AutopilotMonitor.Functions.Functions.Admin
                 _logger.LogInformation($"Reseed from GitHub triggered by Global Admin {upn}, type={typeParam}");
 
                 var gatherResult = new { deleted = 0, written = 0 };
-                var analyzeResult = new { deleted = 0, written = 0 };
+                var analyzeResult = new { deleted = 0, written = 0, orphanStatesGcd = 0, sunsetSkipped = 0 };
                 var imeResult = new { deleted = 0, written = 0 };
 
                 if (typeParam == "all" || typeParam == "gather")
@@ -77,8 +77,8 @@ namespace AutopilotMonitor.Functions.Functions.Admin
                 if (typeParam == "all" || typeParam == "analyze")
                 {
                     var rules = await _gitHubRepo.FetchAnalyzeRulesAsync();
-                    var (d, w) = await ReseedAnalyzeAsync(rules);
+                    var (d, w, gcd, skipped) = await ReseedAnalyzeAsync(rules);
-                    analyzeResult = new { deleted = d, written = w };
+                    analyzeResult = new { deleted = d, written = w, orphanStatesGcd = gcd, sunsetSkipped = skipped };
                 }
 
                 if (typeParam == "all" || typeParam == "ime")
@@ -159,29 +159,59 @@ namespace AutopilotMonitor.Functions.Functions.Admin
             return (deleted, rules.Count);
         }
 
-        private async Task<(int deleted, int written)> ReseedAnalyzeAsync(List<AutopilotMonitor.Shared.Models.AnalyzeRule> rules)
+        private async Task<(int deleted, int written, int orphanStatesGcd, int sunsetSkipped)> ReseedAnalyzeAsync(List<AutopilotMonitor.Shared.Models.AnalyzeRule> rules)
         {
             var existing = await _ruleRepo.GetAnalyzeRulesAsync("global");
-
-            // Identify sunset rules BEFORE the delete pass so we can fan out the
-            // per-tenant RuleState orphan GC after the catalog is rebuilt. A rule is
-            // "sunset" if it existed as built-in / community in the global partition but
-            // isn't in the new GitHub-fetched catalog. Mirrors the sunset handling in
-            // AnalyzeRuleService.EnsureBuiltInRulesSeededAsync — kept symmetric so
-            // every reseed path leaves the same DB invariants.
             var newCatalogIds = rules.Select(r => r.RuleId).ToHashSet(System.StringComparer.Ordinal);
-            var sunsetRuleIds = existing
+
-                .Where(r => (r.IsBuiltIn || r.IsCommunity) && !newCatalogIds.Contains(r.RuleId))
+            // Split existing built-in / community rows into survivors (in new catalog
-                .Select(r => r.RuleId)
+            // → re-imported below) and sunsets (NOT in new catalog → orphan-state GC
-                .ToList();
+            // first, global delete only on clean GC). The sunset ordering mirrors
+            // AnalyzeRuleService.EnsureBuiltInRulesSeededAsync — kept symmetric so every
+            // reseed path leaves the same DB invariants and partial failures retry.
+            var survivors = existing.Where(r => (r.IsBuiltIn || r.IsCommunity) && newCatalogIds.Contains(r.RuleId)).ToList();
+            var sunset = existing.Where(r => (r.IsBuiltIn || r.IsCommunity) && !newCatalogIds.Contains(r.RuleId)).ToList();
 
             var deleted = 0;
-            foreach (var rule in existing.Where(r => r.IsBuiltIn || r.IsCommunity))
+
+            // Survivor path: delete then re-write (legacy behaviour).
+            foreach (var rule in survivors)
             {
                 await _ruleRepo.DeleteAnalyzeRuleAsync("global", rule.RuleId);
                 deleted++;
             }
 
+            // Sunset path: GC-first ordering. If GC fails partially, leave the global
+            // row in place so the next reseed cycle sees the rule in the sunset diff
+            // again and retries.
+            var orphanStatesGcd = 0;
+            var sunsetSkipped = 0;
+            foreach (var sunsetRule in sunset)
+            {
+                var (gcDeleted, gcFailed) = await _ruleRepo.DeleteRuleStatesForRuleIdAcrossTenantsAsync(sunsetRule.RuleId);
+                orphanStatesGcd += gcDeleted;
+                if (gcFailed != 0)
+                {
+                    _logger.LogWarning(
+                        "GitHub reseed sunset GC partial failure for {RuleId} (deleted={Deleted}, failed={Failed}); keeping global row to retry on next reseed",
+                        sunsetRule.RuleId, gcDeleted, gcFailed);
+                    sunsetSkipped++;
+                    continue;
+                }
+                var globalDeleted = await _ruleRepo.DeleteAnalyzeRuleAsync("global", sunsetRule.RuleId);
+                if (globalDeleted)
+                {
+                    deleted++;
+                }
+                else
+                {
+                    _logger.LogWarning(
+                        "GitHub reseed sunset GC clean but global delete failed for {RuleId}; will retry on next reseed",
+                        sunsetRule.RuleId);
+                    sunsetSkipped++;
+                }
+            }
+
             foreach (var rule in rules)
             {
                 // Community rules keep IsCommunity=true from JSON; all others are built-in
@@ -192,16 +222,10 @@ namespace AutopilotMonitor.Functions.Functions.Admin
                 await _ruleRepo.StoreAnalyzeRuleAsync(rule, "global");
             }
 
-            var orphanStatesGcd = 0;
-            foreach (var sunsetRuleId in sunsetRuleIds)
-            {
-                orphanStatesGcd += await _ruleRepo.DeleteRuleStatesForRuleIdAcrossTenantsAsync(sunsetRuleId);
-            }
-
             _logger.LogInformation(
-                "GitHub reseed analyze: {Deleted} deleted, {Written} written, {OrphanCount} orphan per-tenant RuleState(s) cleaned for {SunsetCount} sunset rule(s)",
+                "GitHub reseed analyze: {Deleted} deleted, {Written} written, {OrphanCount} orphan per-tenant RuleState(s) cleaned across {SunsetTotal} sunset rule(s) ({Skipped} skipped on failure for retry)",
-                deleted, rules.Count, orphanStatesGcd, sunsetRuleIds.Count);
+                deleted, rules.Count, orphanStatesGcd, sunset.Count, sunsetSkipped);
-            return (deleted, rules.Count);
+            return (deleted, rules.Count, orphanStatesGcd, sunsetSkipped);
         }
 
         private async Task<(int deleted, int written)> ReseedImeAsync(List<AutopilotMonitor.Shared.Models.ImeLogPattern> patterns)

src/Backend/AutopilotMonitor.Functions/Services/AnalyzeRuleService.cs

@@ -215,11 +215,16 @@ namespace AutopilotMonitor.Functions.Services
         }
 
         /// <summary>
-        /// Re-imports all built-in analyze rules into the global partition. Deletes old
+        /// Re-imports all built-in analyze rules into the global partition. The previous
-        /// global built-in rules and writes current code definitions. For rules that
+        /// "delete all built-ins, then re-import" approach is preserved for the rules
-        /// existed before but are NOT in the new catalog (sunset rules), this also GCs
+        /// that survive the new catalog; for SUNSET rules (existed in DB as built-in but
-        /// the per-tenant <c>RuleState</c> rows that would otherwise linger as orphan
+        /// no longer in <see cref="BuiltInAnalyzeRules"/>) the sequence is inverted —
-        /// state. Tenant RuleStates for rules that are still in the catalog are preserved.
+        /// orphan-state GC first, global delete only on a clean GC — so a partial
+        /// failure leaves the rule in the diff to be retried on the next seed cycle.
+        /// <para>
+        /// Returns <c>(deleted, written, orphanStatesGcd)</c> so the admin endpoint can
+        /// surface the cleanup count to the operator.
+        /// </para>
         /// </summary>
         public async Task<(int deleted, int written, int orphanStatesGcd)> ReseedBuiltInRulesAsync()
         {
@@ -229,22 +234,55 @@ namespace AutopilotMonitor.Functions.Services
             var builtInRules = BuiltInAnalyzeRules.GetAll();
             var newCatalogIds = builtInRules.Select(r => r.RuleId).ToHashSet(StringComparer.Ordinal);
 
-            // Identify sunset rule IDs BEFORE the delete pass so we can fan out the
+            // Survivors: existing built-ins that ARE in the new catalog — these get the
-            // per-tenant RuleState GC after the catalog is rebuilt. A rule is "sunset"
+            // legacy "delete then re-write" treatment (preserves UpdatedAt etc.).
-            // if it existed in the global partition (as built-in) but isn't in the
+            // Sunset:   existing built-ins NOT in the new catalog — these get the
-            // new catalog.
+            // retry-safe "GC first, global delete on clean GC" treatment.
-            var sunsetRuleIds = existingGlobalRules
+            var survivors = existingGlobalRules.Where(r => r.IsBuiltIn && newCatalogIds.Contains(r.RuleId)).ToList();
-                .Where(r => r.IsBuiltIn && !newCatalogIds.Contains(r.RuleId))
+            var sunset = existingGlobalRules.Where(r => r.IsBuiltIn && !newCatalogIds.Contains(r.RuleId)).ToList();
-                .Select(r => r.RuleId)
-                .ToList();
 
             var deleted = 0;
-            foreach (var rule in existingGlobalRules.Where(r => r.IsBuiltIn))
+
+            // Survivor path: same as before — delete then write.
+            foreach (var rule in survivors)
             {
                 await _ruleRepo.DeleteAnalyzeRuleAsync("global", rule.RuleId);
                 deleted++;
             }
-            _logger.LogInformation($"Deleted {deleted} old global built-in analyze rules");
+
+            // Sunset path: GC-first ordering.
+            var orphanStatesGcd = 0;
+            var sunsetSkippedOnGcFailure = 0;
+            var sunsetSkippedOnGlobalDeleteFailure = 0;
+            foreach (var sunsetRule in sunset)
+            {
+                var (gcDeleted, gcFailed) = await _ruleRepo.DeleteRuleStatesForRuleIdAcrossTenantsAsync(sunsetRule.RuleId);
+                orphanStatesGcd += gcDeleted;
+                if (gcFailed != 0)
+                {
+                    _logger.LogWarning(
+                        "Reseed sunset GC partial failure for {RuleId} (deleted={Deleted}, failed={Failed}); keeping global row to retry next cycle",
+                        sunsetRule.RuleId, gcDeleted, gcFailed);
+                    sunsetSkippedOnGcFailure++;
+                    continue;
+                }
+                var globalDeleted = await _ruleRepo.DeleteAnalyzeRuleAsync("global", sunsetRule.RuleId);
+                if (globalDeleted)
+                {
+                    deleted++;
+                }
+                else
+                {
+                    _logger.LogWarning(
+                        "Reseed sunset GC clean but global delete failed for {RuleId}; will retry next cycle",
+                        sunsetRule.RuleId);
+                    sunsetSkippedOnGlobalDeleteFailure++;
+                }
+            }
+
+            _logger.LogInformation(
+                "Reseed analyze rules: {Deleted} old rows deleted, {Survivors} survivors, {Sunset} sunset (GC skip={GcSkip}, delete skip={DelSkip}, orphan states removed={OrphanGcd})",
+                deleted, survivors.Count, sunset.Count, sunsetSkippedOnGcFailure, sunsetSkippedOnGlobalDeleteFailure, orphanStatesGcd);
 
             foreach (var rule in builtInRules)
             {
@@ -252,21 +290,6 @@ namespace AutopilotMonitor.Functions.Services
             }
             _logger.LogInformation($"Written {builtInRules.Count} built-in analyze rules from code");
 
-            // Orphan-RuleState GC for sunset rules — see method-level XML doc for the
-            // motivation. Best-effort: failures on individual rows are logged inside the
-            // storage method, not surfaced here.
-            var orphanStatesGcd = 0;
-            foreach (var sunsetRuleId in sunsetRuleIds)
-            {
-                orphanStatesGcd += await _ruleRepo.DeleteRuleStatesForRuleIdAcrossTenantsAsync(sunsetRuleId);
-            }
-            if (orphanStatesGcd > 0)
-            {
-                _logger.LogInformation(
-                    "Cleaned up {Count} orphan per-tenant RuleState row(s) for {SunsetCount} sunset built-in rule(s)",
-                    orphanStatesGcd, sunsetRuleIds.Count);
-            }
-
             _seeded = false;
 
             return (deleted, builtInRules.Count, orphanStatesGcd);
@@ -330,6 +353,13 @@ namespace AutopilotMonitor.Functions.Services
                 // Sunset detection: rules that existed as built-in but are no longer in
                 // the code catalog. Delete the global row AND fan out the per-tenant
                 // RuleState GC so opt-in overrides don't linger as dead state.
+                //
+                // Order matters and is deliberate: orphan-GC FIRST, then global delete.
+                // If GC fails partially we leave the global row in place so the next
+                // startup sees the rule in the sunset diff again and retries the cleanup.
+                // The previous order (global-delete-first) lost retry-ability because
+                // once the global row is gone the diff goes empty and any remaining
+                // orphan RuleStates would linger indefinitely.
                 var sunsetRules = existingRules
                     .Where(r => r.IsBuiltIn && !newCatalogIds.Contains(r.RuleId))
                     .ToList();
@@ -337,14 +367,42 @@ namespace AutopilotMonitor.Functions.Services
                 if (sunsetRules.Count > 0)
                 {
                     var orphanStatesGcd = 0;
+                    var skippedOnGcFailure = 0;
+                    var skippedOnGlobalDeleteFailure = 0;
                     foreach (var sunset in sunsetRules)
                     {
-                        await _ruleRepo.DeleteAnalyzeRuleAsync("global", sunset.RuleId);
+                        var (gcDeleted, gcFailed) = await _ruleRepo.DeleteRuleStatesForRuleIdAcrossTenantsAsync(sunset.RuleId);
-                        orphanStatesGcd += await _ruleRepo.DeleteRuleStatesForRuleIdAcrossTenantsAsync(sunset.RuleId);
+                        if (gcFailed != 0)
+                        {
+                            // Either per-row error (failed > 0) or enumeration failure (-1).
+                            // Either way we cannot safely drop the global row — retry next
+                            // startup. Orphan-State rows that DID get deleted in this pass
+                            // are gone for good, but that's fine — the per-row delete is
+                            // already 404-idempotent.
+                            _logger.LogWarning(
+                                "Sunset GC partial failure for {RuleId} (deleted={Deleted}, failed={Failed}); keeping global row to retry on next seed cycle",
+                                sunset.RuleId, gcDeleted, gcFailed);
+                            skippedOnGcFailure++;
+                            orphanStatesGcd += gcDeleted;
+                            continue;
+                        }
+
+                        var globalDeleted = await _ruleRepo.DeleteAnalyzeRuleAsync("global", sunset.RuleId);
+                        if (!globalDeleted)
+                        {
+                            // GC was clean but the global delete itself failed (transient
+                            // Azure error). The orphan states are already gone; the global
+                            // row will be retried on next startup (GC will then no-op).
+                            _logger.LogWarning(
+                                "Sunset GC clean but global delete failed for {RuleId}; will retry next seed cycle",
+                                sunset.RuleId);
+                            skippedOnGlobalDeleteFailure++;
+                        }
+                        orphanStatesGcd += gcDeleted;
                     }
                     _logger.LogInformation(
-                        "Sunset {RuleCount} built-in analyze rule(s) no longer in the code catalog; GC'd {OrphanCount} orphan per-tenant RuleState row(s)",
+                        "Sunset {RuleCount} built-in analyze rule(s); GC'd {OrphanCount} orphan per-tenant RuleState row(s); skipped {GcSkip} on GC failure, {DelSkip} on global-delete failure",
-                        sunsetRules.Count, orphanStatesGcd);
+                        sunsetRules.Count, orphanStatesGcd, skippedOnGcFailure, skippedOnGlobalDeleteFailure);
                 }
             }
 

src/Backend/AutopilotMonitor.Functions/Services/TableStorageService.Rules.cs

@@ -318,7 +318,9 @@ namespace AutopilotMonitor.Functions.Services
         }
 
         /// <summary>
-        /// Deletes the rule state for a tenant (resets to rule's default enabled state)
+        /// Deletes the rule state for a tenant (resets to rule's default enabled state).
+        /// Idempotent: a 404 (the row was already gone) counts as success so callers can
+        /// retry safely without flipping their happy-path on the missing-row condition.
         /// </summary>
         public async Task<bool> DeleteRuleStateAsync(string tenantId, string ruleId)
         {
@@ -330,6 +332,10 @@ namespace AutopilotMonitor.Functions.Services
                 await tableClient.DeleteEntityAsync(tenantId, ruleId);
                 return true;
             }
+            catch (Azure.RequestFailedException ex) when (ex.Status == 404)
+            {
+                return true;
+            }
             catch (Exception ex)
             {
                 _logger.LogError(ex, $"Failed to delete rule state {ruleId} for {tenantId}");
@@ -342,13 +348,21 @@ namespace AutopilotMonitor.Functions.Services
         /// RowKey matches <paramref name="ruleId"/>. Used as orphan-GC when a built-in
         /// rule is removed from the global catalog — without this, per-tenant
         /// <c>RuleState{Enabled=true}</c> overrides survive the sunset and would re-fire
-        /// the moment the rule was ever re-introduced under the same ruleId. Best-effort:
+        /// the moment the rule was ever re-introduced under the same ruleId.
-        /// failures on individual rows are logged and skipped (no transactionality across
+        /// <para>
-        /// partitions). Returns the actual number of rows deleted.
+        /// Returns a tuple of <c>(deleted, failed)</c>. <c>deleted</c> counts both fresh
+        /// deletes and 404 (the row was already gone — idempotent retry). <c>failed</c>
+        /// is non-zero if any individual delete threw a non-404 error, or
+        /// <c>-1</c> if the cross-partition enumeration itself failed (we can't tell how
+        /// many rows are still out there). Callers must check <c>failed</c> before
+        /// proceeding with the global rule delete — otherwise a partial GC followed by a
+        /// successful catalog delete leaves orphan state with no way to re-enter the diff
+        /// on a later seed cycle.
+        /// </para>
         /// </summary>
-        public async Task<int> DeleteRuleStatesForRuleIdAcrossTenantsAsync(string ruleId)
+        public async Task<(int deleted, int failed)> DeleteRuleStatesForRuleIdAcrossTenantsAsync(string ruleId)
         {
-            if (string.IsNullOrWhiteSpace(ruleId)) return 0;
+            if (string.IsNullOrWhiteSpace(ruleId)) return (0, 0);
             try
             {
                 var tableClient = _tableServiceClient.GetTableClient(Constants.TableNames.RuleStates);
@@ -358,6 +372,7 @@ namespace AutopilotMonitor.Functions.Services
                 var query = tableClient.QueryAsync<TableEntity>(filter: $"RowKey eq '{ruleId.Replace("'", "''")}'");
 
                 var deleted = 0;
+                var failed = 0;
                 await foreach (var entity in query)
                 {
                     try
@@ -365,26 +380,35 @@ namespace AutopilotMonitor.Functions.Services
                         await tableClient.DeleteEntityAsync(entity.PartitionKey, entity.RowKey);
                         deleted++;
                     }
+                    catch (Azure.RequestFailedException ex) when (ex.Status == 404)
+                    {
+                        // Row was deleted between the query and our delete — idempotent.
+                        deleted++;
+                    }
                     catch (Exception ex)
                     {
                         _logger.LogWarning(
                             ex,
                             "Failed to delete orphan RuleState {RuleId} for tenant {TenantId}",
                             ruleId, entity.PartitionKey);
+                        failed++;
                     }
                 }
-                if (deleted > 0)
+                if (deleted > 0 || failed > 0)
                 {
                     _logger.LogInformation(
-                        "Deleted {Count} orphan RuleState row(s) for sunset rule {RuleId}",
+                        "Orphan RuleState GC for {RuleId}: {Deleted} deleted, {Failed} failed",
-                        deleted, ruleId);
+                        ruleId, deleted, failed);
                 }
-                return deleted;
+                return (deleted, failed);
             }
             catch (Exception ex)
             {
                 _logger.LogError(ex, "Failed to enumerate RuleStates for orphan GC of {RuleId}", ruleId);
-                return 0;
+                // -1 signals "we don't know" — caller must NOT proceed to delete the
+                // global catalog row, otherwise the rule falls out of the diff and any
+                // orphan state we couldn't enumerate lingers indefinitely.
+                return (0, -1);
             }
         }
 
@@ -468,7 +492,10 @@ namespace AutopilotMonitor.Functions.Services
         }
 
         /// <summary>
-        /// Deletes an analyze rule
+        /// Deletes an analyze rule. Idempotent: a 404 (the rule was already gone) counts
+        /// as success so the sunset orchestration in
+        /// <see cref="AnalyzeRuleService.EnsureBuiltInRulesSeededAsync"/> can retry without
+        /// flipping its happy-path on the missing-row condition.
         /// </summary>
         public async Task<bool> DeleteAnalyzeRuleAsync(string tenantId, string ruleId)
         {
@@ -478,6 +505,10 @@ namespace AutopilotMonitor.Functions.Services
                 await tableClient.DeleteEntityAsync(tenantId, ruleId);
                 return true;
             }
+            catch (Azure.RequestFailedException ex) when (ex.Status == 404)
+            {
+                return true;
+            }
             catch (Exception ex)
             {
                 _logger.LogError(ex, $"Failed to delete analyze rule {ruleId}");

src/Shared/AutopilotMonitor.Shared/DataAccess/IRuleRepository.cs

@@ -28,9 +28,16 @@ namespace AutopilotMonitor.Shared.DataAccess
         /// <paramref name="ruleId"/>. Used as orphan-GC when a built-in / community rule
         /// is sunset (deleted from the global catalog): tenants that previously toggled
         /// the rule in the UI have a per-tenant <c>RuleState</c> row that would otherwise
-        /// linger as dead state. Returns the number of orphans deleted.
+        /// linger as dead state.
+        /// <para>
+        /// Returns <c>(deleted, failed)</c>. A 404 on an individual row is counted as
+        /// deleted (idempotent retry). <c>failed</c> is non-zero on per-row errors;
+        /// <c>-1</c> signals the cross-partition enumeration itself failed — callers
+        /// MUST then refuse to delete the global catalog row, or the rule falls out of
+        /// the sunset-diff and unreachable orphan state lingers forever.
+        /// </para>
         /// </summary>
-        Task<int> DeleteRuleStatesForRuleIdAcrossTenantsAsync(string ruleId);
+        Task<(int deleted, int failed)> DeleteRuleStatesForRuleIdAcrossTenantsAsync(string ruleId);
 
         // --- Analyze Rules ---
         Task<bool> StoreAnalyzeRuleAsync(AnalyzeRule rule, string tenantId = "global");