diff --git a/src/TeamsISO.Engine/Discovery/NdiDiscoveryService.cs b/src/TeamsISO.Engine/Discovery/NdiDiscoveryService.cs index 0ec626c..67dda9c 100644 --- a/src/TeamsISO.Engine/Discovery/NdiDiscoveryService.cs +++ b/src/TeamsISO.Engine/Discovery/NdiDiscoveryService.cs @@ -127,34 +127,17 @@ public sealed class NdiDiscoveryService RebuildFinder("operator request"); lastRebuildAt = now; } - // Auto-healing rebuilds. Only one path can fire per tick. + // Auto-healing rebuilds — see ShouldAutoRebuild. else if (_previous.Count == 0) { - var sinceStart = now - startedAt; - var sinceRebuild = now - lastRebuildAt; - if (lastSeenAt is null) + var decision = ShouldAutoRebuild( + sinceStart: now - startedAt, + sinceLastSeen: lastSeenAt is { } seen ? now - seen : (TimeSpan?)null, + sinceLastRebuild: now - lastRebuildAt); + if (decision is { } reason) { - // Never seen a source. After 5s of empty results, try a rebuild - // every 5s. This handles the cold-start case where the initial - // finder didn't bind to the right network interface. - if (sinceStart > TimeSpan.FromSeconds(5) && sinceRebuild > TimeSpan.FromSeconds(5)) - { - RebuildFinder("auto-heal: never saw a source"); - lastRebuildAt = now; - } - } - else - { - // We've seen sources before but currently see nothing. After - // 15s of empty results, try a rebuild every 10s. This handles - // the "Teams stopped broadcasting then started again but our - // finder didn't pick up the new advertisements" case. - var sinceLastSeen = now - lastSeenAt.Value; - if (sinceLastSeen > TimeSpan.FromSeconds(15) && sinceRebuild > TimeSpan.FromSeconds(10)) - { - RebuildFinder("auto-heal: source set went empty 15s ago"); - lastRebuildAt = now; - } + RebuildFinder(reason); + lastRebuildAt = now; } } @@ -170,6 +153,39 @@ public sealed class NdiDiscoveryService } } + /// + /// Pure-function decision for whether the discovery loop should rebuild the + /// NDI finder on the current tick. Returns a non-null reason string when + /// the rebuild should fire (which is also logged); null means "leave the + /// finder alone." Caller is responsible for tracking the timestamps and + /// updating lastRebuildAt after the rebuild. + /// + /// Public + static for unit-testability — the time-based rules are easy to + /// regress and hard to spot in integration testing. + /// + /// Rules: + /// + /// Never seen a source ( is null): + /// rebuild when sinceStart > 5s AND sinceLastRebuild > 5s. + /// Used to see sources, now empty: rebuild when sinceLastSeen + /// > 15s AND sinceLastRebuild > 10s. + /// + /// Both rules back off the rebuild cadence to avoid churn during legitimate + /// empty periods (no meeting active, all participants left, etc.). + /// + public static string? ShouldAutoRebuild(TimeSpan sinceStart, TimeSpan? sinceLastSeen, TimeSpan sinceLastRebuild) + { + if (sinceLastSeen is null) + { + if (sinceStart > TimeSpan.FromSeconds(5) && sinceLastRebuild > TimeSpan.FromSeconds(5)) + return "auto-heal: never saw a source"; + return null; + } + if (sinceLastSeen.Value > TimeSpan.FromSeconds(15) && sinceLastRebuild > TimeSpan.FromSeconds(10)) + return "auto-heal: source set went empty 15s ago"; + return null; + } + /// /// Dispose the current finder and create a fresh one against the cached /// discovery groups. Clears the seen-set so all currently-visible sources diff --git a/src/tests/TeamsISO.Engine.Tests/Discovery/NdiDiscoveryServiceTests.cs b/src/tests/TeamsISO.Engine.Tests/Discovery/NdiDiscoveryServiceTests.cs index 54ca337..a8527aa 100644 --- a/src/tests/TeamsISO.Engine.Tests/Discovery/NdiDiscoveryServiceTests.cs +++ b/src/tests/TeamsISO.Engine.Tests/Discovery/NdiDiscoveryServiceTests.cs @@ -66,4 +66,78 @@ public class NdiDiscoveryServiceTests while (reader.TryRead(out var ev)) list.Add(ev); return list; } + + // ============================================================ + // ShouldAutoRebuild — pure function gating the auto-heal path + // ============================================================ + // + // Two rules under test: + // (a) Never seen a source AND sinceStart>5s AND sinceLastRebuild>5s -> rebuild + // (b) Used to see sources, now empty AND sinceLastSeen>15s AND sinceLastRebuild>10s -> rebuild + // + // Both rules back off to avoid churn during legitimate empty periods. + + [Fact] + public void ShouldAutoRebuild_NeverSeenSource_BeforeWarmup_ReturnsNull() + { + // 3s after startup is well inside the "give cold start a chance" window. + NdiDiscoveryService.ShouldAutoRebuild( + sinceStart: TimeSpan.FromSeconds(3), + sinceLastSeen: null, + sinceLastRebuild: TimeSpan.FromSeconds(99)) + .Should().BeNull(); + } + + [Fact] + public void ShouldAutoRebuild_NeverSeenSource_AfterWarmup_TriggersRebuild() + { + NdiDiscoveryService.ShouldAutoRebuild( + sinceStart: TimeSpan.FromSeconds(6), + sinceLastSeen: null, + sinceLastRebuild: TimeSpan.FromSeconds(6)) + .Should().Contain("never saw a source"); + } + + [Fact] + public void ShouldAutoRebuild_NeverSeenSource_RecentRebuild_HoldsOff() + { + // sinceStart qualifies, but the last rebuild was 2s ago — back off. + NdiDiscoveryService.ShouldAutoRebuild( + sinceStart: TimeSpan.FromSeconds(20), + sinceLastSeen: null, + sinceLastRebuild: TimeSpan.FromSeconds(2)) + .Should().BeNull(); + } + + [Fact] + public void ShouldAutoRebuild_HadSources_NowEmpty_LongAgo_TriggersRebuild() + { + NdiDiscoveryService.ShouldAutoRebuild( + sinceStart: TimeSpan.FromMinutes(5), + sinceLastSeen: TimeSpan.FromSeconds(20), + sinceLastRebuild: TimeSpan.FromSeconds(30)) + .Should().Contain("source set went empty"); + } + + [Fact] + public void ShouldAutoRebuild_HadSources_NowEmpty_Recently_HoldsOff() + { + // 10s since last source seen — still inside the 15s grace window. + NdiDiscoveryService.ShouldAutoRebuild( + sinceStart: TimeSpan.FromMinutes(5), + sinceLastSeen: TimeSpan.FromSeconds(10), + sinceLastRebuild: TimeSpan.FromSeconds(30)) + .Should().BeNull(); + } + + [Fact] + public void ShouldAutoRebuild_HadSources_NowEmpty_RecentRebuild_HoldsOff() + { + // Grace window expired, but we just rebuilt 8s ago — back off. + NdiDiscoveryService.ShouldAutoRebuild( + sinceStart: TimeSpan.FromMinutes(5), + sinceLastSeen: TimeSpan.FromSeconds(30), + sinceLastRebuild: TimeSpan.FromSeconds(8)) + .Should().BeNull(); + } }