Clear only the credential GitHub rejected, and stop treating a plain 403 as one [patch]

.gitignore

@@ -203,6 +203,11 @@ PublishScripts/
 **/[Pp]ackages/*
 # except build/, which is used as an MSBuild target.
 !**/[Pp]ackages/build/
+# and except a Unity project's Packages/, which is source: Unity's package manifest and its
+# resolved lock file are both meant to be committed, and a NuGet restore folder never contains
+# a file by either name.
+!**/[Pp]ackages/manifest.json
+!**/[Pp]ackages/packages-lock.json
 # Uncomment if necessary however generally it will be regenerated when needed
 #!**/[Pp]ackages/repositories.config
 # NuGet v3's project.json files produces more ignorable files
@@ -651,3 +656,16 @@ Temporary Items
 
 # ImGui.ini files
 imgui.ini
+
+# Game engine projects
+#
+# Godot: the import cache, and the mono/temp bin+obj a C# build writes.
+.godot/
+
+# Unity: .meta files are source, not the Visual Studio C++ build artifact that the `*.meta` rule
+# further up targets. Unity generates one per asset and it carries the GUID that scenes, prefabs
+# and serialized references point at, so ignoring them gives every clone fresh GUIDs and silently
+# breaks those references - including for a plug-in whose .dll is itself a build output. This
+# negation has to come after that rule to win, and is scoped to the asset tree so the Visual
+# Studio artifact stays ignored everywhere else.
+!**/[Aa]ssets/**/*.meta

BuildMonitor.Test/GitHubRequestOutcomeTests.cs

@@ -104,7 +104,7 @@ public async Task ARateLimited403ReportsFailure()
 	}
 
 	/// <summary>
-	/// The other headline case: a plain 403, which is what a just-revoked token looks like.
+	/// The other headline case: a plain 403, a request the token is not permitted to make.
 	/// </summary>
 	[TestMethod]
 	public async Task APlain403ReportsFailure()
@@ -114,9 +114,76 @@ public async Task APlain403ReportsFailure()
 		bool succeeded = await RunFailing(provider, ApiFailure(HttpStatusCode.Forbidden)).ConfigureAwait(false);
 
 		Assert.IsFalse(succeeded, "A request refused for lack of authorization did not happen, and must not report success.");
+		Assert.AreEqual(ProviderStatus.Error, provider.Status);
+	}
+
+	/// <summary>
+	/// A 403 that is not a rate limit refuses one request -- a workflow dispatch on a repository the
+	/// token lacks <c>actions: write</c> for, say. The token still works for everything else, so the
+	/// refusal must not erase it along with every owner that depends on it.
+	/// </summary>
+	[TestMethod]
+	public async Task APlain403LeavesTheCredentialsInPlace()
+	{
+		GitHub provider = new();
+		SetProviderToken(provider, "provider-pat");
+		Owner owner = OwnerWithToken(provider);
+
+		bool succeeded = await provider.MakeGitHubRequestAsync(
+			RequestName, () => Task.FromException(ApiFailure(HttpStatusCode.Forbidden)), owner).ConfigureAwait(false);
+
+		Assert.IsFalse(succeeded);
+		Assert.AreEqual("provider-pat", ProviderToken(provider));
+		Assert.AreEqual("alpha-pat", owner.Token.ToString());
+	}
+
+	/// <summary>
+	/// A request sent with an owner's override token that GitHub rejects condemns that token, not
+	/// the provider's. Clearing the provider token took every other owner offline, while the owner
+	/// token that actually failed stayed in place and was rejected again every cycle.
+	/// </summary>
+	[TestMethod]
+	public async Task ARejectedOwnerTokenClearsOnlyThatOwnersToken()
+	{
+		GitHub provider = new();
+		SetProviderToken(provider, "provider-pat");
+		Owner owner = OwnerWithToken(provider);
+		AuthorizationException unauthorized = new(new FakeResponse(HttpStatusCode.Unauthorized, new Dictionary<string, string>()));
+
+		bool succeeded = await provider.MakeGitHubRequestAsync(
+			RequestName, () => Task.FromException(unauthorized), owner).ConfigureAwait(false);
+
+		Assert.IsFalse(succeeded);
+		Assert.IsFalse(owner.HasToken, "The owner token GitHub rejected must not be sent again.");
+		Assert.AreEqual("provider-pat", ProviderToken(provider), "The provider token was not the one rejected.");
+		Assert.AreEqual(ProviderStatus.AuthFailed, provider.Status);
+	}
+
+	/// <summary>
+	/// An owner without its own token uses the provider's, so a rejection there still clears the
+	/// provider credentials as before.
+	/// </summary>
+	[TestMethod]
+	public async Task ARejectedProviderTokenStillClearsTheProviderCredentials()
+	{
+		GitHub provider = new();
+		SetProviderToken(provider, "provider-pat");
+		Owner owner = provider.CreateOwner(OwnerName.Create<OwnerName>("beta"));
+		AuthorizationException unauthorized = new(new FakeResponse(HttpStatusCode.Unauthorized, new Dictionary<string, string>()));
+
+		bool succeeded = await provider.MakeGitHubRequestAsync(
+			RequestName, () => Task.FromException(unauthorized), owner).ConfigureAwait(false);
+
+		Assert.IsFalse(succeeded);
+		Assert.AreEqual(string.Empty, ProviderToken(provider));
 		Assert.AreEqual(ProviderStatus.AuthFailed, provider.Status);
 	}
 
+	private static void SetProviderToken(GitHub provider, string token) =>
+		Assert.IsTrue(TokenStorage.Write(provider.TokenPersona, BuildProviderToken.Create<BuildProviderToken>(token)));
+
+	private static string ProviderToken(GitHub provider) => TokenStorage.Read(provider.TokenPersona).ToString();
+
 	[TestMethod]
 	public async Task A429ReportsFailure()
 	{

BuildMonitor/Providers/GitHub.cs

@@ -93,7 +93,7 @@
 
 	private Owner? OwnerPendingTokenPopup { get; set; }
 
 	internal override void ShowMenu()
 	{
 		if (Hexa.NET.ImGui.ImGui.BeginMenu(Name))
 		{
@@ -219,7 +219,7 @@
 		}).ConfigureAwait(false);
 	}
 
 	internal override async Task UpdateRepositoriesAsync(Owner owner)
 	{
 		if (!HasValidCredentials(owner))
 		{
@@ -247,7 +247,7 @@
 				// Filter to only repos owned by this user (GetAllForCurrent returns repos from all orgs the user has access to)
 				IReadOnlyList<Octokit.Repository> currentUserRepos = await GitHubRepository.GetAllForCurrent().ConfigureAwait(false);
 				int totalCount = currentUserRepos.Count;
 				foreach (Octokit.Repository repo in currentUserRepos)
 				{
 					if (repo.Owner.Login.Equals(owner.Name.ToString(), StringComparison.OrdinalIgnoreCase))
 					{
@@ -268,7 +268,7 @@
 				catch (NotFoundException)
 				{
 					// Owner might be an org-only account, try org repos instead
 					userRepositories = [];
 				}
 
 				// Try to get organization repositories - this only works for orgs
@@ -281,7 +281,7 @@
 				catch (NotFoundException)
 				{
 					// Owner is not an organization, that's fine
 					organizationRepositories = [];
 				}
 			}
 
@@ -373,7 +373,7 @@
 
 					// Get existing build or create new one
 					bool isNew = false;
 					Build build = repository.Builds.GetOrAdd(buildId, _ =>
 					{
 						isNew = true;
 						return repository.CreateBuild(buildName, buildId);
@@ -680,6 +680,9 @@
 			// concurrent requests use different owner tokens
 			SetCurrentClient(owner);
 
+			// Which credential this request is sent with decides which one a rejection condemns.
+			Owner? tokenOwner = owner?.HasToken == true ? owner : null;
+
 			// Use smart waiting: if rate limited with a known reset time, wait until reset
 			TimeSpan waitTime = GetRateLimitWaitTime();
 			if (waitTime > BaseRequestDelay)
@@ -701,7 +704,15 @@
 			catch (AuthorizationException)
 			{
 				Log.Error($"{Name}: AuthorizationException for request '{name}'");
-				OnAuthenticationFailure();
+				if (tokenOwner is not null)
+				{
+					OnOwnerAuthenticationFailure(tokenOwner);
+				}
+				else
+				{
+					OnAuthenticationFailure();
+				}
+
 				return false;
 			}
 			catch (ApiException e)
@@ -718,8 +729,11 @@
 						}
 						else
 						{
+							// A token that is not allowed to make this one request -- a workflow
+							// dispatch without actions: write, say -- is still a working token.
+							// The refusal belongs to this request, not to the credentials.
 							Log.Error($"{Name}: 403 Forbidden for request '{name}' - {e.Message}");
-							OnAuthenticationFailure();
+							SetStatus(ProviderStatus.Error, $"{Strings.Forbidden}: {name}");
 						}
 						break;
 					case System.Net.HttpStatusCode.TooManyRequests:
@@ -746,6 +760,23 @@
 		}
 	}
 
+	/// <summary>
+	/// Clears an owner's override token after GitHub rejected it.
+	/// </summary>
+	/// <param name="owner">The owner whose token the rejected request was sent with.</param>
+	/// <remarks>
+	/// The provider credentials were not used for the request, so they are left alone: clearing them
+	/// took every other owner offline while the token that actually failed stayed in place and kept
+	/// being sent, and rejected, every cycle. Clearing the owner token instead lets that owner fall
+	/// back to the provider token until a new one is set.
+	/// </remarks>
+	private void OnOwnerAuthenticationFailure(Owner owner)
+	{
+		Log.Error($"{Name}: Authentication failed for owner '{owner.Name}' - owner token cleared");
+		owner.Token = new();
+		SetStatus(ProviderStatus.AuthFailed, $"{Strings.AuthFailedMessage} ({owner.Name})");
+	}
+
 	/// <summary>
 	/// Updates the rate limit budget from the last API response.
 	/// Uses Octokit's GetLastApiInfo() to retrieve rate limit headers.

BuildMonitor/Strings.cs

@@ -44,6 +44,7 @@ internal static class Strings
 	internal static string RateLimitedMessage { get; } = "Rate limited.";
 	internal static string AuthFailed { get; } = nameof(AuthFailed).Titleize();
 	internal static string AuthFailedMessage { get; } = "Authentication failed. Please update credentials.";
+	internal static string Forbidden { get; } = "Forbidden";
 	internal static string ConnectionError { get; } = nameof(ConnectionError).Titleize();
 	internal static string ConnectionErrorMessage { get; } = "Connection error.";
 	internal static string Delay { get; } = nameof(Delay).Titleize();