Fix test merges resetting in auto-update

If an error calling GitHub ocurred the job would stop after merging the HEAD branch and continue to deploy. Implement two measures to prevent this:

- If the repository update job fails, do not proceed with code deployment.
- Do not make changes to the repository before calling GitHub.

src/Tgstation.Server.Host/Components/Instance.cs

@@ -382,6 +382,17 @@ namespace Tgstation.Server.Host.Components
 					// build current commit data if it's missing
 					await UpdateRevInfo(repo.Head, false, null);
 
+					var preserveTestMerges = repositorySettings.AutoUpdatesKeepTestMerges.Value;
+					var remoteDeploymentManager = remoteDeploymentManagerFactory.CreateRemoteDeploymentManager(
+						metadata,
+						repo.RemoteGitProvider.Value);
+
+					var updatedTestMerges = await remoteDeploymentManager.RemoveMergedTestMerges(
+						repo,
+						repositorySettings,
+						currentRevInfo,
+						cancellationToken);
+
 					var result = await repo.MergeOrigin(
 						NextProgressReporter("Merge Origin"),
 						repositorySettings.CommitterName,
@@ -389,21 +400,10 @@ namespace Tgstation.Server.Host.Components
 						true,
 						cancellationToken);
 
-					var preserveTestMerges = repositorySettings.AutoUpdatesKeepTestMerges.Value;
-					var remoteDeploymentManager = remoteDeploymentManagerFactory.CreateRemoteDeploymentManager(
-						metadata,
-						repo.RemoteGitProvider.Value);
-
 					// take appropriate auto update actions
 					var shouldSyncTracked = false;
 					if (result.HasValue)
 					{
-						var updatedTestMerges = await remoteDeploymentManager.RemoveMergedTestMerges(
-							repo,
-							repositorySettings,
-							currentRevInfo,
-							cancellationToken);
-
 						if (updatedTestMerges.Count == 0)
 						{
 							logger.LogTrace("All test merges have been merged on remote");
@@ -517,7 +517,12 @@ namespace Tgstation.Server.Host.Components
 							RepositoryAutoUpdateJob,
 							cancellationToken);
 
-						await jobManager.WaitForJobCompletion(repositoryUpdateJob, null, cancellationToken, cancellationToken);
+						var repoUpdateJobResult = await jobManager.WaitForJobCompletion(repositoryUpdateJob, null, cancellationToken, cancellationToken);
+						if (repoUpdateJobResult == false)
+						{
+							logger.LogWarning("Aborting auto-update due to repository update error!");
+							continue;
+						}
 
 						Job compileProcessJob;
 						using (var repo = await RepositoryManager.LoadRepository(cancellationToken))

src/Tgstation.Server.Host/Jobs/IJobManager.cs

@@ -33,8 +33,8 @@ namespace Tgstation.Server.Host.Jobs
 		/// <param name="canceller">The <see cref="User"/> to cancel the <paramref name="job"/>. If <see langword="null"/> the TGS user will be used.</param>
 		/// <param name="jobCancellationToken">A <see cref="CancellationToken"/> that will cancel the <paramref name="job"/>.</param>
 		/// <param name="cancellationToken">The <see cref="CancellationToken"/> for the operation.</param>
-		/// <returns>A <see cref="Task"/> representing the <see cref="Job"/>.</returns>
-		Task WaitForJobCompletion(Job job, User canceller, CancellationToken jobCancellationToken, CancellationToken cancellationToken);
+		/// <returns>A <see cref="Task{TResult}"/> representing the <see cref="Job"/>. Results in <see langword="true"/> if the <see cref="Job"/> completed without errors, <see langword="false"/> if errors occurred, or <see langword="null"/> if the job isn't registered.</returns>
+		Task<bool?> WaitForJobCompletion(Job job, User canceller, CancellationToken jobCancellationToken, CancellationToken cancellationToken);
 
 		/// <summary>
 		/// Cancels a give <paramref name="job"/>.

src/Tgstation.Server.Host/Jobs/JobHandler.cs

@@ -30,20 +30,20 @@ namespace Tgstation.Server.Host.Jobs
 		readonly CancellationTokenSource cancellationTokenSource;
 
 		/// <summary>
-		/// A <see cref="Func{T, TResult}"/> taking a <see cref="CancellationToken"/> and returning a <see cref="Task"/> that the <see cref="JobHandler"/> will wrap.
+		/// A <see cref="Func{T, TResult}"/> taking a <see cref="CancellationToken"/> and returning a <see cref="Task{TResult}"/> that the <see cref="JobHandler"/> will wrap.
 		/// </summary>
-		readonly Func<CancellationToken, Task> jobActivator;
+		readonly Func<CancellationToken, Task<bool>> jobActivator;
 
 		/// <summary>
 		/// The <see cref="Task"/> being run.
 		/// </summary>
-		Task task;
+		Task<bool> task;
 
 		/// <summary>
 		/// Initializes a new instance of the <see cref="JobHandler"/> class.
 		/// </summary>
 		/// <param name="jobActivator">The value of <see cref="jobActivator"/>.</param>
-		public JobHandler(Func<CancellationToken, Task> jobActivator)
+		public JobHandler(Func<CancellationToken, Task<bool>> jobActivator)
 		{
 			this.jobActivator = jobActivator ?? throw new ArgumentNullException(nameof(jobActivator));
 			cancellationTokenSource = new CancellationTokenSource();
@@ -56,8 +56,8 @@ namespace Tgstation.Server.Host.Jobs
 		/// Wait for <see cref="task"/> to complete.
 		/// </summary>
 		/// <param name="cancellationToken">The <see cref="CancellationToken"/> for the operation.</param>
-		/// <returns>A <see cref="Task"/> representing the running operation.</returns>
-		public Task Wait(CancellationToken cancellationToken)
+		/// <returns>A <see cref="Task{TResult}"/> representing the job. Results in <see langword="true"/> if the job completed without errors, <see langword="false"/> otherwise.</returns>
+		public Task<bool> Wait(CancellationToken cancellationToken)
 		{
 			if (task == null)
 				throw new InvalidOperationException("Job not started!");

src/Tgstation.Server.Host/Jobs/JobService.cs

@@ -247,7 +247,7 @@ namespace Tgstation.Server.Host.Jobs
 		}
 
 		/// <inheritdoc />
-		public async Task WaitForJobCompletion(Job job, User canceller, CancellationToken jobCancellationToken, CancellationToken cancellationToken)
+		public async Task<bool?> WaitForJobCompletion(Job job, User canceller, CancellationToken jobCancellationToken, CancellationToken cancellationToken)
 		{
 			ArgumentNullException.ThrowIfNull(job);
 
@@ -259,7 +259,7 @@ namespace Tgstation.Server.Host.Jobs
 			lock (synchronizationLock)
 			{
 				if (!jobs.TryGetValue(job.Id.Value, out handler))
-					return;
+					return null;
 
 				noMoreJobsShouldStart = this.noMoreJobsShouldStart;
 			}
@@ -268,11 +268,14 @@ namespace Tgstation.Server.Host.Jobs
 				await Extensions.TaskExtensions.InfiniteTask.WaitAsync(cancellationToken);
 
 			Task cancelTask = null;
+			bool result;
 			using (jobCancellationToken.Register(() => cancelTask = CancelJob(job, canceller, true, cancellationToken)))
-				await handler.Wait(cancellationToken);
+				result = await handler.Wait(cancellationToken);
 
 			if (cancelTask != null)
 				await cancelTask;
+
+			return result;
 		}
 
 		/// <inheritdoc />
@@ -291,12 +294,14 @@ namespace Tgstation.Server.Host.Jobs
 		/// <param name="operation">The <see cref="JobEntrypoint"/> for the <paramref name="job"/>.</param>
 		/// <param name="cancellationToken">The <see cref="CancellationToken"/> for the operation.</param>
 		/// <returns>A <see cref="Task"/> representing the running operation.</returns>
-		async Task RunJob(Job job, JobEntrypoint operation, CancellationToken cancellationToken)
+		async Task<bool> RunJob(Job job, JobEntrypoint operation, CancellationToken cancellationToken)
 		{
 			using (LogContext.PushProperty(SerilogContextHelper.JobIdContextProperty, job.Id))
 				try
 				{
 					void LogException(Exception ex) => logger.LogDebug(ex, "Job {jobId} exited with error!", job.Id);
+
+					var result = false;
 					try
 					{
 						var oldJob = job;
@@ -334,6 +339,7 @@ namespace Tgstation.Server.Host.Jobs
 							cancellationToken);
 
 						logger.LogDebug("Job {jobId} completed!", job.Id);
+						result = true;
 					}
 					catch (OperationCanceledException ex)
 					{
@@ -368,6 +374,8 @@ namespace Tgstation.Server.Host.Jobs
 						// DCT: Cancellation token is for job, operation should always run
 						await databaseContext.Save(CancellationToken.None);
 					});
+
+					return result;
 				}
 				finally
 				{

tests/Tgstation.Server.Host.Tests/Components/Chat/Providers/TestDiscordProvider.cs

@@ -39,7 +39,7 @@ namespace Tgstation.Server.Host.Components.Chat.Providers.Tests
 				.Returns(Task.CompletedTask);
 			mockSetup
 				.Setup(x => x.WaitForJobCompletion(It.IsNotNull<Job>(), It.IsAny<User>(), It.IsAny<CancellationToken>(), It.IsAny<CancellationToken>()))
-				.Returns(Task.CompletedTask);
+				.Returns(Task.FromResult<bool?>(true));
 			mockJobManager = mockSetup.Object;
 		}
 

tests/Tgstation.Server.Host.Tests/Components/Chat/Providers/TestIrcProvider.cs

@@ -76,7 +76,7 @@ namespace Tgstation.Server.Host.Components.Chat.Providers.Tests
 				.Returns(Task.CompletedTask);
 			mockSetup
 				.Setup(x => x.WaitForJobCompletion(It.IsNotNull<Job>(), It.IsAny<User>(), It.IsAny<CancellationToken>(), It.IsAny<CancellationToken>()))
-				.Returns(Task.CompletedTask);
+				.Returns(Task.FromResult<bool?>(true));
 			var mockJobManager = mockSetup.Object;
 			await using var provider = new IrcProvider(mockJobManager, new AsyncDelayer(), loggerFactory.CreateLogger<IrcProvider>(), Mock.Of<IAssemblyInformationProvider>(), new ChatBot
 			{

tests/Tgstation.Server.Host.Tests/Jobs/TestJobHandler.cs

@@ -11,10 +11,11 @@ namespace Tgstation.Server.Host.Jobs.Tests
 		Task currentWaitTask;
 		bool cancelled;
 
-		async Task TestJob(CancellationToken cancellationToken)
+		async Task<bool> TestJob(CancellationToken cancellationToken)
 		{
 			await currentWaitTask;
 			cancelled = cancellationToken.IsCancellationRequested;
+			return true;
 		}
 
 		[TestMethod]