From bc8916994610613e188153fecb75df19d03b3500 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 3 Nov 2026 20:38:42 +0000 Subject: [PATCH 1/3] Fetch a re-run's errors again when the new attempt fails too [patch] A GitHub re-run keeps the run id. When attempt 2 also failed and the app never saw it pending or running, the stored run was already a failure with attempt 1's errors, so ShouldFetchRunErrors returned false and the row kept showing the previous attempt's errors next to the new start time. UpdateRunFromWorkflowAsync now compares the stored start time with the run's run_started_at before overwriting it. A changed start time is a new attempt: the old errors are dropped and the jobs are fetched again if it failed. An unchanged failed run still isn't refetched on every update (#311). Fixes ktsu-dev/BuildMonitor#394 Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_017vfJW5KENnPmXEat6KQ92F --- BuildMonitor.Test/GitHubRunErrorsTests.cs | 42 +++++++++++++++++++++++ BuildMonitor/Providers/GitHub.cs | 30 ++++++++++++---- 2 files changed, 66 insertions(+), 6 deletions(-) diff --git a/BuildMonitor.Test/GitHubRunErrorsTests.cs b/BuildMonitor.Test/GitHubRunErrorsTests.cs index 3cd3e6f..19ea650 100644 --- a/BuildMonitor.Test/GitHubRunErrorsTests.cs +++ b/BuildMonitor.Test/GitHubRunErrorsTests.cs @@ -66,6 +66,48 @@ public void ASecondUpdateOfAFailedRunDoesNotFetchItsJobsAgain() Assert.IsFalse(fetchAgain); } + /// + /// A re-run keeps the run id. When attempt 2 also failed and was never seen running, the stored run + /// was already a failure with attempt 1's errors, so its jobs were never fetched again + /// (ktsu-dev/BuildMonitor#394). + /// + [TestMethod] + public void AFailedReRunOfAFailedRunFetchesItsJobsAgain() + { + // Arrange + DateTimeOffset firstAttempt = new(2026, 11, 3, 9, 0, 0, TimeSpan.Zero); + DateTimeOffset secondAttempt = firstAttempt.AddHours(1); + IReadOnlyList attemptOneErrors = ["[build] OLD: error CS1002 from attempt 1"]; + + // Act + bool newAttempt = GitHub.IsNewAttempt(firstAttempt, secondAttempt); + bool fetch = GitHub.ShouldFetchRunErrors(RunStatus.Failure, RunStatus.Failure, attemptOneErrors, newAttempt); + + // Assert + Assert.IsTrue(newAttempt, "A later start time on the same run id is a new attempt"); + Assert.IsTrue(fetch, "The new attempt's failure should be read from its jobs"); + } + + [TestMethod] + public void AnUnchangedFailedRunIsNotANewAttemptAndDoesNotRefetch() + { + // Arrange + DateTimeOffset started = new(2026, 11, 3, 9, 0, 0, TimeSpan.Zero); + IReadOnlyList errors = ["[build] error CS1002: ; expected"]; + + // Act + bool newAttempt = GitHub.IsNewAttempt(started, started); + bool fetch = GitHub.ShouldFetchRunErrors(RunStatus.Failure, RunStatus.Failure, errors, newAttempt); + + // Assert + Assert.IsFalse(newAttempt); + Assert.IsFalse(fetch, "The #311 guarantee: an unchanged failed run is not refetched on every update"); + } + + [TestMethod] + public void ARunReadForTheFirstTimeIsNotANewAttempt() => + Assert.IsFalse(GitHub.IsNewAttempt(default, new DateTimeOffset(2026, 11, 3, 9, 0, 0, TimeSpan.Zero))); + [TestMethod] [DataRow(WorkflowJobConclusion.Failure, true)] [DataRow(WorkflowJobConclusion.TimedOut, true)] diff --git a/BuildMonitor/Providers/GitHub.cs b/BuildMonitor/Providers/GitHub.cs index d71259c..e83717f 100644 --- a/BuildMonitor/Providers/GitHub.cs +++ b/BuildMonitor/Providers/GitHub.cs @@ -515,6 +515,15 @@ internal static RunStatus MapWorkflowRunStatus(StringEnum run private async Task UpdateRunFromWorkflowAsync(Run run, WorkflowRun gitHubRun) { + // A re-run keeps the run id but starts a new attempt with its own start time. The previous + // attempt's errors no longer describe the run, so they are dropped and fetched afresh + // (ktsu-dev/BuildMonitor#394). + bool newAttempt = IsNewAttempt(run.Started, gitHubRun.RunStartedAt); + if (newAttempt) + { + run.Errors = []; + } + run.Started = gitHubRun.RunStartedAt; run.LastUpdated = gitHubRun.UpdatedAt; run.Branch = gitHubRun.HeadBranch.As(); @@ -540,7 +549,7 @@ private async Task UpdateRunFromWorkflowAsync(Run run, WorkflowRun gitHubRun) } // Fetch error details if the run just failed or is a failure without errors - if (ShouldFetchRunErrors(run.Status, previousStatus, run.Errors)) + if (ShouldFetchRunErrors(run.Status, previousStatus, run.Errors, newAttempt)) { await FetchRunErrorsAsync(GitHubJobs, run, gitHubRun.Conclusion).ConfigureAwait(false); } @@ -556,13 +565,22 @@ private async Task UpdateRunFromWorkflowAsync(Run run, WorkflowRun gitHubRun) } /// - /// Whether a run's jobs should be fetched for error details: when it has just failed, or is a - /// failure whose errors have not been fetched yet. A completed fetch always leaves at least one - /// error behind (see ), so it is not repeated on every update + /// Whether a run's jobs should be fetched for error details: when it has just failed, is a failure + /// whose errors have not been fetched yet, or is a failure of a new attempt of a run that had + /// already failed. A completed fetch always leaves at least one error behind (see + /// ), so it is not repeated on every update /// (ktsu-dev/BuildMonitor#311). /// - internal static bool ShouldFetchRunErrors(RunStatus status, RunStatus previousStatus, IReadOnlyList errors) => - status == RunStatus.Failure && (previousStatus != RunStatus.Failure || errors.Count == 0); + internal static bool ShouldFetchRunErrors(RunStatus status, RunStatus previousStatus, IReadOnlyList errors, bool newAttempt = false) => + status == RunStatus.Failure && (newAttempt || previousStatus != RunStatus.Failure || errors.Count == 0); + + /// + /// Whether a run read from GitHub is a later attempt than the one stored: a re-run keeps the run + /// id and gets a new start time. A run that has never been read has no stored start time, and is + /// not a re-run. + /// + internal static bool IsNewAttempt(DateTimeOffset storedStarted, DateTimeOffset started) => + storedStarted != default && started != storedStarted; /// /// Whether a job's conclusion is one that fails its run. A job that hit its timeout ends as From 9f2c4f87917a90500352d6756b24186db7d6912c Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 3 Nov 2026 20:47:42 +0000 Subject: [PATCH 2/3] Move the new-attempt bookkeeping into a tested helper SonarCloud's quality gate failed at 33% coverage on new code because the attempt check lived in the private UpdateRunFromWorkflowAsync. StartAttempt now records the start time, drops a previous attempt's errors and reports whether the attempt is new, and is covered directly. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_017vfJW5KENnPmXEat6KQ92F --- BuildMonitor.Test/GitHubRunErrorsTests.cs | 36 +++++++++++++++++++++++ BuildMonitor/Providers/GitHub.cs | 29 +++++++++++------- 2 files changed, 55 insertions(+), 10 deletions(-) diff --git a/BuildMonitor.Test/GitHubRunErrorsTests.cs b/BuildMonitor.Test/GitHubRunErrorsTests.cs index 19ea650..3eeb3f9 100644 --- a/BuildMonitor.Test/GitHubRunErrorsTests.cs +++ b/BuildMonitor.Test/GitHubRunErrorsTests.cs @@ -104,6 +104,42 @@ public void AnUnchangedFailedRunIsNotANewAttemptAndDoesNotRefetch() Assert.IsFalse(fetch, "The #311 guarantee: an unchanged failed run is not refetched on every update"); } + [TestMethod] + public void StartingANewAttemptDropsThePreviousAttemptsErrors() + { + // Arrange + DateTimeOffset firstAttempt = new(2026, 11, 3, 9, 0, 0, TimeSpan.Zero); + Run run = FailedRun(); + run.Started = firstAttempt; + run.Errors = ["[build] OLD: error CS1002 from attempt 1"]; + + // Act + bool newAttempt = GitHub.StartAttempt(run, firstAttempt.AddHours(1)); + + // Assert + Assert.IsTrue(newAttempt); + Assert.IsEmpty(run.Errors, "Attempt 1's errors don't describe attempt 2"); + Assert.AreEqual(firstAttempt.AddHours(1), run.Started); + Assert.IsTrue(GitHub.ShouldFetchRunErrors(RunStatus.Failure, RunStatus.Failure, run.Errors, newAttempt)); + } + + [TestMethod] + public void RereadingTheSameAttemptKeepsItsErrors() + { + // Arrange + DateTimeOffset started = new(2026, 11, 3, 9, 0, 0, TimeSpan.Zero); + Run run = FailedRun(); + run.Started = started; + run.Errors = ["[build] error CS1002: ; expected"]; + + // Act + bool newAttempt = GitHub.StartAttempt(run, started); + + // Assert + Assert.IsFalse(newAttempt); + AssertErrors(run, "[build] error CS1002: ; expected"); + } + [TestMethod] public void ARunReadForTheFirstTimeIsNotANewAttempt() => Assert.IsFalse(GitHub.IsNewAttempt(default, new DateTimeOffset(2026, 11, 3, 9, 0, 0, TimeSpan.Zero))); diff --git a/BuildMonitor/Providers/GitHub.cs b/BuildMonitor/Providers/GitHub.cs index e83717f..22e23a6 100644 --- a/BuildMonitor/Providers/GitHub.cs +++ b/BuildMonitor/Providers/GitHub.cs @@ -515,16 +515,7 @@ internal static RunStatus MapWorkflowRunStatus(StringEnum run private async Task UpdateRunFromWorkflowAsync(Run run, WorkflowRun gitHubRun) { - // A re-run keeps the run id but starts a new attempt with its own start time. The previous - // attempt's errors no longer describe the run, so they are dropped and fetched afresh - // (ktsu-dev/BuildMonitor#394). - bool newAttempt = IsNewAttempt(run.Started, gitHubRun.RunStartedAt); - if (newAttempt) - { - run.Errors = []; - } - - run.Started = gitHubRun.RunStartedAt; + bool newAttempt = StartAttempt(run, gitHubRun.RunStartedAt); run.LastUpdated = gitHubRun.UpdatedAt; run.Branch = gitHubRun.HeadBranch.As(); @@ -582,6 +573,24 @@ internal static bool ShouldFetchRunErrors(RunStatus status, RunStatus previousSt internal static bool IsNewAttempt(DateTimeOffset storedStarted, DateTimeOffset started) => storedStarted != default && started != storedStarted; + /// + /// Records the start time GitHub reports for a run, and returns whether it began a new attempt. A + /// re-run keeps the run id but starts a new attempt with its own start time, and the previous + /// attempt's errors no longer describe the run, so they are dropped to be fetched afresh + /// (ktsu-dev/BuildMonitor#394). + /// + internal static bool StartAttempt(Run run, DateTimeOffset started) + { + bool newAttempt = IsNewAttempt(run.Started, started); + if (newAttempt) + { + run.Errors = []; + } + + run.Started = started; + return newAttempt; + } + /// /// Whether a job's conclusion is one that fails its run. A job that hit its timeout ends as /// timed_out, not failure, and is just as much the reason the run failed. From 14736ba2a7c4e74c1dcdfc985414a68d0f2e306c Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 3 Nov 2026 20:56:52 +0000 Subject: [PATCH 3/3] Let dropped errors trigger the refetch instead of a separate flag Clearing a new attempt's stale errors already makes ShouldFetchRunErrors fetch them, since a failure with no errors is always fetched. The extra newAttempt parameter is gone, StartAttempt just records the start time and drops the old errors, and UpdateRunFromWorkflowAsync changes by one line. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_017vfJW5KENnPmXEat6KQ92F --- BuildMonitor.Test/GitHubRunErrorsTests.cs | 59 +++++++---------------- BuildMonitor/Providers/GitHub.cs | 32 ++++++------ 2 files changed, 33 insertions(+), 58 deletions(-) diff --git a/BuildMonitor.Test/GitHubRunErrorsTests.cs b/BuildMonitor.Test/GitHubRunErrorsTests.cs index 3eeb3f9..2300207 100644 --- a/BuildMonitor.Test/GitHubRunErrorsTests.cs +++ b/BuildMonitor.Test/GitHubRunErrorsTests.cs @@ -73,39 +73,6 @@ public void ASecondUpdateOfAFailedRunDoesNotFetchItsJobsAgain() /// [TestMethod] public void AFailedReRunOfAFailedRunFetchesItsJobsAgain() - { - // Arrange - DateTimeOffset firstAttempt = new(2026, 11, 3, 9, 0, 0, TimeSpan.Zero); - DateTimeOffset secondAttempt = firstAttempt.AddHours(1); - IReadOnlyList attemptOneErrors = ["[build] OLD: error CS1002 from attempt 1"]; - - // Act - bool newAttempt = GitHub.IsNewAttempt(firstAttempt, secondAttempt); - bool fetch = GitHub.ShouldFetchRunErrors(RunStatus.Failure, RunStatus.Failure, attemptOneErrors, newAttempt); - - // Assert - Assert.IsTrue(newAttempt, "A later start time on the same run id is a new attempt"); - Assert.IsTrue(fetch, "The new attempt's failure should be read from its jobs"); - } - - [TestMethod] - public void AnUnchangedFailedRunIsNotANewAttemptAndDoesNotRefetch() - { - // Arrange - DateTimeOffset started = new(2026, 11, 3, 9, 0, 0, TimeSpan.Zero); - IReadOnlyList errors = ["[build] error CS1002: ; expected"]; - - // Act - bool newAttempt = GitHub.IsNewAttempt(started, started); - bool fetch = GitHub.ShouldFetchRunErrors(RunStatus.Failure, RunStatus.Failure, errors, newAttempt); - - // Assert - Assert.IsFalse(newAttempt); - Assert.IsFalse(fetch, "The #311 guarantee: an unchanged failed run is not refetched on every update"); - } - - [TestMethod] - public void StartingANewAttemptDropsThePreviousAttemptsErrors() { // Arrange DateTimeOffset firstAttempt = new(2026, 11, 3, 9, 0, 0, TimeSpan.Zero); @@ -114,17 +81,16 @@ public void StartingANewAttemptDropsThePreviousAttemptsErrors() run.Errors = ["[build] OLD: error CS1002 from attempt 1"]; // Act - bool newAttempt = GitHub.StartAttempt(run, firstAttempt.AddHours(1)); + GitHub.StartAttempt(run, firstAttempt.AddHours(1)); // Assert - Assert.IsTrue(newAttempt); Assert.IsEmpty(run.Errors, "Attempt 1's errors don't describe attempt 2"); Assert.AreEqual(firstAttempt.AddHours(1), run.Started); - Assert.IsTrue(GitHub.ShouldFetchRunErrors(RunStatus.Failure, RunStatus.Failure, run.Errors, newAttempt)); + Assert.IsTrue(GitHub.ShouldFetchRunErrors(RunStatus.Failure, RunStatus.Failure, run.Errors), "The new attempt's failure should be read from its jobs"); } [TestMethod] - public void RereadingTheSameAttemptKeepsItsErrors() + public void RereadingAnUnchangedFailedRunKeepsItsErrorsAndDoesNotRefetch() { // Arrange DateTimeOffset started = new(2026, 11, 3, 9, 0, 0, TimeSpan.Zero); @@ -133,16 +99,27 @@ public void RereadingTheSameAttemptKeepsItsErrors() run.Errors = ["[build] error CS1002: ; expected"]; // Act - bool newAttempt = GitHub.StartAttempt(run, started); + GitHub.StartAttempt(run, started); // Assert - Assert.IsFalse(newAttempt); AssertErrors(run, "[build] error CS1002: ; expected"); + Assert.IsFalse(GitHub.ShouldFetchRunErrors(RunStatus.Failure, RunStatus.Failure, run.Errors), "The #311 guarantee: an unchanged failed run is not refetched on every update"); } [TestMethod] - public void ARunReadForTheFirstTimeIsNotANewAttempt() => - Assert.IsFalse(GitHub.IsNewAttempt(default, new DateTimeOffset(2026, 11, 3, 9, 0, 0, TimeSpan.Zero))); + public void ARunReadForTheFirstTimeKeepsWhateverItHas() + { + // Arrange + Run run = FailedRun(); + DateTimeOffset started = new(2026, 11, 3, 9, 0, 0, TimeSpan.Zero); + + // Act + GitHub.StartAttempt(run, started); + + // Assert + Assert.AreEqual(started, run.Started); + Assert.IsFalse(GitHub.IsNewAttempt(default, started)); + } [TestMethod] [DataRow(WorkflowJobConclusion.Failure, true)] diff --git a/BuildMonitor/Providers/GitHub.cs b/BuildMonitor/Providers/GitHub.cs index 22e23a6..ddfbaf7 100644 --- a/BuildMonitor/Providers/GitHub.cs +++ b/BuildMonitor/Providers/GitHub.cs @@ -515,7 +515,7 @@ internal static RunStatus MapWorkflowRunStatus(StringEnum run private async Task UpdateRunFromWorkflowAsync(Run run, WorkflowRun gitHubRun) { - bool newAttempt = StartAttempt(run, gitHubRun.RunStartedAt); + StartAttempt(run, gitHubRun.RunStartedAt); run.LastUpdated = gitHubRun.UpdatedAt; run.Branch = gitHubRun.HeadBranch.As(); @@ -540,7 +540,7 @@ private async Task UpdateRunFromWorkflowAsync(Run run, WorkflowRun gitHubRun) } // Fetch error details if the run just failed or is a failure without errors - if (ShouldFetchRunErrors(run.Status, previousStatus, run.Errors, newAttempt)) + if (ShouldFetchRunErrors(run.Status, previousStatus, run.Errors)) { await FetchRunErrorsAsync(GitHubJobs, run, gitHubRun.Conclusion).ConfigureAwait(false); } @@ -556,14 +556,14 @@ private async Task UpdateRunFromWorkflowAsync(Run run, WorkflowRun gitHubRun) } /// - /// Whether a run's jobs should be fetched for error details: when it has just failed, is a failure - /// whose errors have not been fetched yet, or is a failure of a new attempt of a run that had - /// already failed. A completed fetch always leaves at least one error behind (see - /// ), so it is not repeated on every update - /// (ktsu-dev/BuildMonitor#311). + /// Whether a run's jobs should be fetched for error details: when it has just failed, or is a + /// failure whose errors have not been fetched yet. A completed fetch always leaves at least one + /// error behind (see ), so it is not repeated on every update + /// (ktsu-dev/BuildMonitor#311). A new attempt of a failed run has its errors dropped by + /// , so it is fetched again too (ktsu-dev/BuildMonitor#394). /// - internal static bool ShouldFetchRunErrors(RunStatus status, RunStatus previousStatus, IReadOnlyList errors, bool newAttempt = false) => - status == RunStatus.Failure && (newAttempt || previousStatus != RunStatus.Failure || errors.Count == 0); + internal static bool ShouldFetchRunErrors(RunStatus status, RunStatus previousStatus, IReadOnlyList errors) => + status == RunStatus.Failure && (previousStatus != RunStatus.Failure || errors.Count == 0); /// /// Whether a run read from GitHub is a later attempt than the one stored: a re-run keeps the run @@ -574,21 +574,19 @@ internal static bool IsNewAttempt(DateTimeOffset storedStarted, DateTimeOffset s storedStarted != default && started != storedStarted; /// - /// Records the start time GitHub reports for a run, and returns whether it began a new attempt. A - /// re-run keeps the run id but starts a new attempt with its own start time, and the previous - /// attempt's errors no longer describe the run, so they are dropped to be fetched afresh - /// (ktsu-dev/BuildMonitor#394). + /// Records the start time GitHub reports for a run. A re-run keeps the run id but starts a new + /// attempt with its own start time, and the previous attempt's errors no longer describe the run, + /// so they are dropped, which makes fetch them afresh if the new + /// attempt failed too (ktsu-dev/BuildMonitor#394). /// - internal static bool StartAttempt(Run run, DateTimeOffset started) + internal static void StartAttempt(Run run, DateTimeOffset started) { - bool newAttempt = IsNewAttempt(run.Started, started); - if (newAttempt) + if (IsNewAttempt(run.Started, started)) { run.Errors = []; } run.Started = started; - return newAttempt; } ///