From 8af781ce32ac776a91499668956701a4d1ad225e Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 9 Oct 2026 07:34:59 +0000 Subject: [PATCH] Match whole segments when marking a build as updating, and count shared request keys [patch] IsBuildUpdating tested request keys with a bare prefix, so a build named "Build" turned cyan while "Build and Release" in the same repository was being polled. It now matches the build key exactly or followed by "/", via a pure IsRequestForBuild. ActiveRequests tracked each key once, so two concurrent run polls of one GitHub workflow (which share a key unless run-name is set) dropped the "updating" state when the first finished. It now counts requests per key and removes the key only when the last one finishes. Fixes #351 Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_019FkGdWB8Ko3mTn9JiBH973 --- BuildMonitor.Test/ActiveRequestTests.cs | 52 +++++++++++++++++++++++++ BuildMonitor/BuildMonitor.cs | 41 ++++++++++++++++--- 2 files changed, 88 insertions(+), 5 deletions(-) create mode 100644 BuildMonitor.Test/ActiveRequestTests.cs diff --git a/BuildMonitor.Test/ActiveRequestTests.cs b/BuildMonitor.Test/ActiveRequestTests.cs new file mode 100644 index 0000000..fc09cc2 --- /dev/null +++ b/BuildMonitor.Test/ActiveRequestTests.cs @@ -0,0 +1,52 @@ +// Copyright (c) 2023-2026 ktsu-dev contributors + +namespace ktsu.BuildMonitor.Test; + +using Microsoft.VisualStudio.TestTools.UnitTesting; + +/// +/// Covers how in-flight requests mark a build as updating (ktsu-dev/BuildMonitor#351): a build's key +/// has to match whole segments of a request key, and a key shared by two concurrent requests has to +/// stay until both finish. +/// +[TestClass] +public sealed class ActiveRequestTests +{ + private const string BuildKey = "GitHub/ktsu-dev/BuildMonitor/Build"; + + [TestMethod] + public void ABuildMatchesItsOwnPollAndItsRunsPolls() + { + Assert.IsTrue(BuildMonitor.IsRequestForBuild(BuildKey, BuildKey)); + Assert.IsTrue(BuildMonitor.IsRequestForBuild($"{BuildKey}/Build #42", BuildKey)); + } + + [TestMethod] + public void ABuildDoesNotMatchAnotherBuildItsNameIsAPrefixOf() + { + Assert.IsFalse(BuildMonitor.IsRequestForBuild($"{BuildKey} and Release", BuildKey)); + Assert.IsFalse(BuildMonitor.IsRequestForBuild($"{BuildKey} and Release/Build and Release", BuildKey)); + Assert.IsFalse(BuildMonitor.IsRequestForBuild($"{BuildKey}Docs", BuildKey)); + } + + [TestMethod] + public async Task AKeySharedByTwoRequestsStaysUntilBothFinish() + { + // A unique key, because ActiveRequests is process-wide and other tests may run alongside. + string key = $"{BuildKey}/{Guid.NewGuid():N}"; + TaskCompletionSource first = new(TaskCreationOptions.RunContinuationsAsynchronously); + TaskCompletionSource second = new(TaskCreationOptions.RunContinuationsAsynchronously); + + Task firstRequest = BuildMonitor.MakeRequestAsync(key, () => first.Task); + Task secondRequest = BuildMonitor.MakeRequestAsync(key, () => second.Task); + Assert.IsTrue(BuildMonitor.ActiveRequests.ContainsKey(key)); + + first.SetResult(); + await firstRequest.ConfigureAwait(false); + Assert.IsTrue(BuildMonitor.ActiveRequests.ContainsKey(key), "The second request is still in flight."); + + second.SetResult(); + await secondRequest.ConfigureAwait(false); + Assert.IsFalse(BuildMonitor.ActiveRequests.ContainsKey(key)); + } +} diff --git a/BuildMonitor/BuildMonitor.cs b/BuildMonitor/BuildMonitor.cs index 2137822..fdcabc0 100644 --- a/BuildMonitor/BuildMonitor.cs +++ b/BuildMonitor/BuildMonitor.cs @@ -33,7 +33,12 @@ internal static class BuildMonitor private static Task UpdateTask { get; set; } = Task.CompletedTask; - internal static ConcurrentDictionary ActiveRequests { get; set; } = []; + /// + /// Gets or sets the in-flight requests, each key with the number of requests running under it. + /// Two ongoing runs of one GitHub workflow poll under the same key, so a key stays while any of + /// its requests is still running. + /// + internal static ConcurrentDictionary ActiveRequests { get; set; } = []; private static void Main() { @@ -1410,10 +1415,19 @@ internal static bool RenderErrorsColumn(Run run, Build build, BranchName branch) private static bool IsBuildUpdating(Build build) { - string prefix = $"{build.Repository.Owner.BuildProvider.Name}/{build.Owner.Name}/{build.Repository.Name}/{build.Name}"; - return ActiveRequests.Keys.Any(k => k.StartsWithOrdinal(prefix)); + string buildKey = $"{build.Repository.Owner.BuildProvider.Name}/{build.Owner.Name}/{build.Repository.Name}/{build.Name}"; + return ActiveRequests.Keys.Any(k => IsRequestForBuild(k, buildKey)); } + /// + /// Whether a request key is the build's own poll or one of its runs' polls. Request keys end in + /// /{build.Name} or /{build.Name}/{run.Name}, so the build key has to match a whole + /// segment: a bare prefix test let a build named Build light up while + /// Build and Release was being polled. + /// + internal static bool IsRequestForBuild(string requestKey, string buildKey) => + string.Equals(requestKey, buildKey, StringComparison.Ordinal) || requestKey.StartsWithOrdinal(buildKey + "/"); + private static ImColor GetStatusColor(RunStatus status) { return status switch @@ -1688,14 +1702,31 @@ private static void SaveSettingsIfRequired() internal static async Task MakeRequestAsync(string name, Func action) { - _ = ActiveRequests.TryAdd(name, DateTimeOffset.UtcNow); + _ = ActiveRequests.AddOrUpdate(name, 1, (_, count) => count + 1); try { await action.Invoke().ConfigureAwait(false); } finally { - _ = ActiveRequests.TryRemove(name, out _); + ReleaseRequest(name); + } + } + + private static void ReleaseRequest(string name) + { + // Compare-and-swap, so a request starting under the same key between the read and the write + // is never lost: the last one to finish is the one that removes the key. + while (ActiveRequests.TryGetValue(name, out int count)) + { + bool released = count <= 1 + ? ActiveRequests.TryRemove(KeyValuePair.Create(name, count)) + : ActiveRequests.TryUpdate(name, count - 1, count); + + if (released) + { + return; + } } } }