diff --git a/src/main/kotlin/de/hoennig/gittally/build/BuildExecutor.kt b/src/main/kotlin/de/hoennig/gittally/build/BuildExecutor.kt index 798c357..9329fdd 100644 --- a/src/main/kotlin/de/hoennig/gittally/build/BuildExecutor.kt +++ b/src/main/kotlin/de/hoennig/gittally/build/BuildExecutor.kt @@ -63,12 +63,24 @@ class BuildExecutor( * Persists a PENDING result and queues the build; returns immediately. * A build of the same branch waits until the branch's previous build finished; * builds of other branches run concurrently while slots are free. + * While a build of the same branch and commit is already queued or executing (and + * not cancel-requested), that build is returned instead of stacking a duplicate — + * a double-triggered UI restart must not queue the same commit twice. Re-running + * a *finished* build stays possible; this only guards the active queue. */ fun startBuild( branch: String, commit: String, workingDir: Path = Paths.get("."), ): RunningBuild { + val duplicate = + builds.values.firstOrNull { + !it.cancelled.get() && it.runningBuild.branch == branch && it.runningBuild.commit == commit + } + if (duplicate != null) { + log.info("build of branch {} at commit {} is already queued or running; not queueing a duplicate", branch, commit) + return duplicate.runningBuild + } val startedAt = Instant.now() val stagingDir = Files.createTempDirectory("gittally-build-") val runningBuild = diff --git a/src/main/kotlin/de/hoennig/gittally/build/BuildResultRepository.kt b/src/main/kotlin/de/hoennig/gittally/build/BuildResultRepository.kt index f184a1b..5e52538 100644 --- a/src/main/kotlin/de/hoennig/gittally/build/BuildResultRepository.kt +++ b/src/main/kotlin/de/hoennig/gittally/build/BuildResultRepository.kt @@ -43,7 +43,11 @@ interface BuildResultRepository { * cutoff are dropped even within the retention count — except each branch's newest entry, * so dormant branches keep their last status. With [keepLatestGreen], the newest SUCCESS * entry of each surviving branch is kept even beyond both limits, so the permanent - * `/branches/…` artifact links stay valid while newer builds fail. Returns the removed entries. + * `/branches/…` artifact links stay valid while newer builds fail. + * PENDING and RUNNING entries are never removed, regardless of all limits and even + * when their branch is gone from [originBranches] — a queued or executing build + * belongs to the executor, and pruning its result would make it invisible in UI + * and history. Returns the removed entries. */ fun prune( originBranches: Collection, diff --git a/src/main/kotlin/de/hoennig/gittally/build/FileBuildResultRepository.kt b/src/main/kotlin/de/hoennig/gittally/build/FileBuildResultRepository.kt index 0520f70..9baa398 100644 --- a/src/main/kotlin/de/hoennig/gittally/build/FileBuildResultRepository.kt +++ b/src/main/kotlin/de/hoennig/gittally/build/FileBuildResultRepository.kt @@ -129,26 +129,32 @@ class FileBuildResultRepository( synchronized(lock) { val results = load() val originBranchSet = originBranches.toSet() + // a queued or executing build belongs to the executor, never to retention: + // pruning its result would make the build invisible in UI and history — seen + // live when a merged branch was deleted from origin while its last build ran + val active = + results.filter { it.status == BuildStatus.PENDING || it.status == BuildStatus.RUNNING } val kept = - results - .filter { it.branch in originBranchSet } - .groupBy { it.branch } - .values - .flatMap { entries -> - val newest = - entries - .sortedByDescending { it.startedAt } - .take(retentionPerBranch.coerceAtLeast(0)) - .filterIndexed { index, entry -> - // the branch's newest entry is never age-pruned - index == 0 || retentionCutoff == null || !entry.startedAt.isBefore(retentionCutoff) - } - val latestGreen = - entries - .filter { keepLatestGreen && it.status == BuildStatus.SUCCESS } - .maxByOrNull { it.startedAt } - newest + listOfNotNull(latestGreen) - }.toSet() + active.toSet() + + results + .filter { it.branch in originBranchSet } + .groupBy { it.branch } + .values + .flatMap { entries -> + val newest = + entries + .sortedByDescending { it.startedAt } + .take(retentionPerBranch.coerceAtLeast(0)) + .filterIndexed { index, entry -> + // the branch's newest entry is never age-pruned + index == 0 || retentionCutoff == null || !entry.startedAt.isBefore(retentionCutoff) + } + val latestGreen = + entries + .filter { keepLatestGreen && it.status == BuildStatus.SUCCESS } + .maxByOrNull { it.startedAt } + newest + listOfNotNull(latestGreen) + }.toSet() val removed = results.filterNot { it in kept } if (removed.isNotEmpty()) { save(results.filter { it in kept }) diff --git a/src/main/kotlin/de/hoennig/gittally/watcher/Watcher.kt b/src/main/kotlin/de/hoennig/gittally/watcher/Watcher.kt index 8584986..b0a74ca 100644 --- a/src/main/kotlin/de/hoennig/gittally/watcher/Watcher.kt +++ b/src/main/kotlin/de/hoennig/gittally/watcher/Watcher.kt @@ -97,6 +97,11 @@ class Watcher( for (result in restartable) { val commit = gitService.originHeadCommit(result.branch, workingDir) if (commit == null) { + if (result.status == BuildStatus.PENDING) { + // a PENDING entry is prune-immune (it normally belongs to the executor); + // close this orphan out so the gone branch can be pruned + repository.updateByArtifactKey(result.artifactKey) { it.copy(status = BuildStatus.INTERRUPTED) } + } log.info("not restarting build of branch {}: branch is gone from origin", result.branch) continue } diff --git a/src/test/kotlin/de/hoennig/gittally/build/BuildExecutorTest.kt b/src/test/kotlin/de/hoennig/gittally/build/BuildExecutorTest.kt index be8f17c..ad0beb5 100644 --- a/src/test/kotlin/de/hoennig/gittally/build/BuildExecutorTest.kt +++ b/src/test/kotlin/de/hoennig/gittally/build/BuildExecutorTest.kt @@ -219,6 +219,31 @@ class BuildExecutorTest : FunSpec() { } } + test("startBuild returns the active build of the same branch and commit instead of stacking a duplicate") { + val h = harness("sleep 30") + + val first = h.executor.startBuild("main", "abc123", h.workingDir) + // a double-triggered UI restart: same branch, same commit, while queued or running + val duplicate = h.executor.startBuild("main", "abc123", h.workingDir) + duplicate.artifactKey shouldBe first.artifactKey + h.repository.history().map { it.artifactKey } shouldContainExactly listOf(first.artifactKey) + + // another commit of the branch is a distinct build, queued behind the first + val newerCommit = h.executor.startBuild("main", "abc124", h.workingDir) + newerCommit.artifactKey shouldNotBe first.artifactKey + + // a cancel-requested build no longer blocks re-queueing its commit + h.executor.cancel(first.artifactKey).shouldBeTrue() + val again = h.executor.startBuild("main", "abc123", h.workingDir) + again.artifactKey shouldNotBe first.artifactKey + + h.executor.cancel(newerCommit.artifactKey).shouldBeTrue() + h.executor.cancel(again.artifactKey).shouldBeTrue() + eventually(30.seconds) { + h.executor.currentBuilds().shouldBeEmpty() + } + } + test("a build cancelled while still queued records neither runningSince nor a duration") { val h = harness("sleep 30") diff --git a/src/test/kotlin/de/hoennig/gittally/build/FileBuildResultRepositoryTest.kt b/src/test/kotlin/de/hoennig/gittally/build/FileBuildResultRepositoryTest.kt index 2a0aad4..2c7c596 100644 --- a/src/test/kotlin/de/hoennig/gittally/build/FileBuildResultRepositoryTest.kt +++ b/src/test/kotlin/de/hoennig/gittally/build/FileBuildResultRepositoryTest.kt @@ -220,6 +220,31 @@ class FileBuildResultRepositoryTest : FunSpec() { ) } + test("prune never removes queued or running results, even of branches gone from origin") { + val repository = FileBuildResultRepository(newFile()) + // a merged branch, deleted from origin while its last build still runs + repository.append(result(branch = "merged", status = BuildStatus.FAILED, startedOffsetSeconds = 0)) + repository.append(result(branch = "merged", status = BuildStatus.RUNNING, startedOffsetSeconds = 60)) + // a queued build beyond the retention count of its branch + repository.append(result(branch = "main", status = BuildStatus.PENDING, startedOffsetSeconds = 0)) + repository.append(result(branch = "main", startedOffsetSeconds = 60)) + + val removed = + repository.prune( + originBranches = listOf("main"), + retentionPerBranch = 1, + retentionCutoff = baseTime.plusSeconds(30), + ) + + removed shouldContainExactly listOf(result(branch = "merged", status = BuildStatus.FAILED, startedOffsetSeconds = 0)) + repository.history() shouldContainExactlyInAnyOrder + listOf( + result(branch = "merged", status = BuildStatus.RUNNING, startedOffsetSeconds = 60), + result(branch = "main", status = BuildStatus.PENDING, startedOffsetSeconds = 0), + result(branch = "main", startedOffsetSeconds = 60), + ) + } + test("prune drops entries older than the retention cutoff even within the retention count") { val repository = FileBuildResultRepository(newFile()) repository.append(result(branch = "main", status = BuildStatus.FAILED, startedOffsetSeconds = 0)) diff --git a/src/test/kotlin/de/hoennig/gittally/watcher/WatcherTest.kt b/src/test/kotlin/de/hoennig/gittally/watcher/WatcherTest.kt index b56b5e1..4613c84 100644 --- a/src/test/kotlin/de/hoennig/gittally/watcher/WatcherTest.kt +++ b/src/test/kotlin/de/hoennig/gittally/watcher/WatcherTest.kt @@ -420,6 +420,20 @@ class WatcherTest : FunSpec() { harness.startedBuilds shouldContainExactly listOf("main" to "commit-2") } + test("startup recovery closes out an orphaned PENDING build of a branch gone from origin") { + val harness = Harness() + val orphan = harness.seed("gone", BuildStatus.PENDING, commit = "commit-1") + + harness.watcher.recoverOnStartup(harness.workingDir) + + // PENDING is prune-immune; left as-is, the gone branch could never be pruned + harness.startedBuilds.shouldBeEmpty() + harness.repository + .history() + .first { it.artifactKey == orphan.artifactKey } + .status shouldBe BuildStatus.INTERRUPTED + } + test("poll prunes results, artifacts, and worktrees of branches gone from origin") { val harness = Harness() harness.seed("main", BuildStatus.SUCCESS, commit = "commit-1")