feat(build): every running build knows its repository — die laufenden Builds und das Worktree-Aufräumen unterscheiden Repositories
Der letzte Übertrag aus Sitzung C: `RunningBuild` trug kein Repository, der Executor ist aber instanzweit. Zwei Folgen, beide gemessen und jetzt behoben: - Das Worktree-Aufräumen schützte die Worktrees ALLER Repositories. Ein laufender Build eines anderen Repositories auf einem gleichnamigen Branch hielt hier einen Worktree fest, dessen Branch längst von origin weg war. - Die Current-Builds-Ansicht und `/api/builds/current` zeigten die Builds aller Repositories, schlugen ihren Status aber in den Ergebnissen NUR dieses Repositories nach — ein fremder Build fiel auf RUNNING zurück und zeigte einen Zustand, den niemand aufgezeichnet hat. Dasselbe beim Live-Log: der Schlüssel eines fremden Builds wurde beantwortet. `RunningBuild.repo` ist der `RepoContext` selbst, verglichen wird per Referenz — der Kontext ist die Identität (ADR 0009), und der Executor hat ihn in `startBuild` ohnehin zur Hand. Watcher, API und UI filtern damit auf ihr eigenes Repository. Drei neue Tests, Gegenprobe per Mutation gezogen: Nimmt man die drei Filter wieder heraus, fallen genau diese drei und sonst keiner. 493 Tests grün, ktlint sauber. Architektur-Skill und Plan (docs/plan/22-multi-repo.md) nachgezogen — die Aussage „RunningBuild carries no repository" stimmte nicht mehr. Was von Sitzung D bleibt: die repo-bezogenen Routen und die Oberfläche, die weiterhin nur `registry.current()` bedienen. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 5
parent
6fba43c627
commit
6ca67a6a37
@@ -63,7 +63,7 @@ Three places must stay in sync when config keys change: the `WerkatorConfig` dat
|
|||||||
|
|
||||||
## Repository Context
|
## Repository Context
|
||||||
|
|
||||||
Everything repository-scoped goes through a `RepoContext` (`repo` package, ADR 0009): the primary checkout (`workingDir`), the repository's `BuildResultRepository` (`.git/werkator/build-results.json`), its `ArtifactStore` (keyed by the repository path), and a short `name` defaulting to the directory basename — the future route segment. `RepoContexts.open(dir)` builds one (running the pre-rename state-dir migration for that repository on the way); `RepoRegistry` opens one per entry of the instance configuration's `repositories` — or the current directory without a registry — lazily on first use and loudly: a non-repository entry or a duplicate name aborts the start naming the home file, a repository whose config must not be read (`ConfigException`) is skipped with an error. `RepoConfiguration` provides `registry.current()` (the cwd when served, else the first entry) as the `RepoContext` bean for the still-unscoped controllers, and the `BuildResultRepository`/`ArtifactStore` beans are that context's. The `--repo` mixin (`RepoOption`) selects by name in `build`, `retry`, and `status`. Git access and config loading stay path-based services taking `repo.workingDir`; the instance configuration (`~/.werkator.yml`, `ConfigLoader.homeDir`/`WERKATOR_HOME`, bound as `InstanceConfig`) is folded in by `ConfigLoader.loadRaw` itself — its `defaults` below every repository layer, its `server`/`executor`/`watcher.pollInterval` overlaid on top and stripped from the repository files with one warning — so every consumer of `load(dir)` sees the instance values without knowing the file. The context object is the identity (executor pools, watcher memory are keyed by it), so exactly one is opened per repository. Not yet repository-scoped: `RunningBuild` carries no repository, so `currentBuilds()` and the worktree pruning cannot tell repositories apart (session D, with the routes).
|
Everything repository-scoped goes through a `RepoContext` (`repo` package, ADR 0009): the primary checkout (`workingDir`), the repository's `BuildResultRepository` (`.git/werkator/build-results.json`), its `ArtifactStore` (keyed by the repository path), and a short `name` defaulting to the directory basename — the future route segment. `RepoContexts.open(dir)` builds one (running the pre-rename state-dir migration for that repository on the way); `RepoRegistry` opens one per entry of the instance configuration's `repositories` — or the current directory without a registry — lazily on first use and loudly: a non-repository entry or a duplicate name aborts the start naming the home file, a repository whose config must not be read (`ConfigException`) is skipped with an error. `RepoConfiguration` provides `registry.current()` (the cwd when served, else the first entry) as the `RepoContext` bean for the still-unscoped controllers, and the `BuildResultRepository`/`ArtifactStore` beans are that context's. The `--repo` mixin (`RepoOption`) selects by name in `build`, `retry`, and `status`. Git access and config loading stay path-based services taking `repo.workingDir`; the instance configuration (`~/.werkator.yml`, `ConfigLoader.homeDir`/`WERKATOR_HOME`, bound as `InstanceConfig`) is folded in by `ConfigLoader.loadRaw` itself — its `defaults` below every repository layer, its `server`/`executor`/`watcher.pollInterval` overlaid on top and stripped from the repository files with one warning — so every consumer of `load(dir)` sees the instance values without knowing the file. The context object is the identity (executor pools, watcher memory are keyed by it), so exactly one is opened per repository — `RunningBuild` carries it too, so `currentBuilds()` says which repository a running build belongs to: the current-builds view and API serve only the served repository's builds, and the watcher's worktree pruning is protected by its own repository's builds alone. Not yet repository-scoped: the routes and the UI, which still serve `registry.current()` only (session D).
|
||||||
|
|
||||||
## Build Execution
|
## Build Execution
|
||||||
|
|
||||||
|
|||||||
@@ -60,7 +60,7 @@ The pinning model is untouched: pinned keys still come from each repo's machine
|
|||||||
- ~~Startup recovery per repo; auto-build slots stay in each repo's `.git/werkator/`.~~ — done: `start(repos)` recovers each in its own guard; slots unchanged.
|
- ~~Startup recovery per repo; auto-build slots stay in each repo's `.git/werkator/`.~~ — done: `start(repos)` recovers each in its own guard; slots unchanged.
|
||||||
- ~~CLI commands gain an optional repo selector and default to the current working directory, so `werkator status` inside a repo behaves as today.~~ — done: `--repo <name>` (`RepoOption` mixin) on `build`, `retry`, `status`; default is the cwd when served, else the first registered repository.
|
- ~~CLI commands gain an optional repo selector and default to the current working directory, so `werkator status` inside a repo behaves as today.~~ — done: `--repo <name>` (`RepoOption` mixin) on `build`, `retry`, `status`; default is the cwd when served, else the first registered repository.
|
||||||
- Also done: the pre-rename state-dir migration runs per opened repository; the metrics page's repository size sums the registered repositories (the disk metric is the first one's file store).
|
- Also done: the pre-rename state-dir migration runs per opened repository; the metrics page's repository size sums the registered repositories (the disk metric is the first one's file store).
|
||||||
- Carried over to session D: `RunningBuild` still carries no repository (the "current builds" view and the worktree pruning cannot tell repositories apart); the controllers still serve `registry.current()` only; `docs/deployment.md` gets the registry setup with session E.
|
- Carried over to session D: ~~`RunningBuild` still carries no repository (the "current builds" view and the worktree pruning cannot tell repositories apart)~~ — done 2026-09-03: `RunningBuild.repo` is the context (identity comparison), the current-builds view and API filter to the served repository, and the worktree pruning is protected by its own repository's running builds alone; the controllers still serve `registry.current()` only; `docs/deployment.md` gets the registry setup with session E.
|
||||||
|
|
||||||
### D — Server, API, and UI scoping
|
### D — Server, API, and UI scoping
|
||||||
|
|
||||||
|
|||||||
@@ -97,6 +97,7 @@ class BuildExecutor(
|
|||||||
val stagingDir = Files.createTempDirectory("werkator-build-")
|
val stagingDir = Files.createTempDirectory("werkator-build-")
|
||||||
val runningBuild =
|
val runningBuild =
|
||||||
RunningBuild(
|
RunningBuild(
|
||||||
|
repo = repo,
|
||||||
branch = branch,
|
branch = branch,
|
||||||
build = build,
|
build = build,
|
||||||
commit = commit,
|
commit = commit,
|
||||||
|
|||||||
@@ -1,11 +1,19 @@
|
|||||||
package de.hoennig.werkator.build
|
package de.hoennig.werkator.build
|
||||||
|
|
||||||
import de.hoennig.werkator.config.BuildDefinition
|
import de.hoennig.werkator.config.BuildDefinition
|
||||||
|
import de.hoennig.werkator.repo.RepoContext
|
||||||
import java.nio.file.Path
|
import java.nio.file.Path
|
||||||
import java.time.Instant
|
import java.time.Instant
|
||||||
|
|
||||||
/** Handle to a build accepted by the [BuildExecutor]; log paths become valid once the build runs. */
|
/** Handle to a build accepted by the [BuildExecutor]; log paths become valid once the build runs. */
|
||||||
data class RunningBuild(
|
data class RunningBuild(
|
||||||
|
/**
|
||||||
|
* The repository this build belongs to; the context object is the identity
|
||||||
|
* (ADR 0009), so it compares by reference. Without it neither the current-builds
|
||||||
|
* view nor the watcher's worktree pruning could tell two repositories apart —
|
||||||
|
* both would see every repository's running builds as their own.
|
||||||
|
*/
|
||||||
|
val repo: RepoContext,
|
||||||
/** The git branch being built. */
|
/** The git branch being built. */
|
||||||
val branch: String,
|
val branch: String,
|
||||||
/** The build definition (job) this build runs; its settings are resolved from config at run time. */
|
/** The build definition (job) this build runs; its settings are resolved from config at run time. */
|
||||||
|
|||||||
@@ -55,11 +55,17 @@ class BuildsApiController(
|
|||||||
|
|
||||||
private fun BuildResult.isLatestGreen(): Boolean = repository.latestGreenFor(name)?.artifactKey == artifactKey
|
private fun BuildResult.isLatestGreen(): Boolean = repository.latestGreenFor(name)?.artifactKey == artifactKey
|
||||||
|
|
||||||
/** The currently executing builds — several are possible, up to `executor.maxConcurrent`. */
|
/**
|
||||||
|
* The currently executing builds of the served repository — several are possible,
|
||||||
|
* up to `executor.maxConcurrent`. The executor is instance-global and returns the
|
||||||
|
* builds of every registered repository, so this view filters: its [repository]
|
||||||
|
* holds only this repository's results, and a foreign build looked up in them
|
||||||
|
* would fall back to RUNNING and show a status nobody recorded.
|
||||||
|
*/
|
||||||
@GetMapping("/api/builds/current")
|
@GetMapping("/api/builds/current")
|
||||||
fun current(): List<CurrentBuildDto> {
|
fun current(): List<CurrentBuildDto> {
|
||||||
val results = repository.history()
|
val results = repository.history()
|
||||||
return buildExecutor.currentBuilds().map { build ->
|
return buildExecutor.currentBuilds().filter { it.repo === repo }.map { build ->
|
||||||
CurrentBuildDto(
|
CurrentBuildDto(
|
||||||
branch = build.branch,
|
branch = build.branch,
|
||||||
name = build.name,
|
name = build.name,
|
||||||
@@ -82,7 +88,7 @@ class BuildsApiController(
|
|||||||
@RequestParam(defaultValue = "0") offset: Long,
|
@RequestParam(defaultValue = "0") offset: Long,
|
||||||
): ResponseEntity<Any> {
|
): ResponseEntity<Any> {
|
||||||
val build =
|
val build =
|
||||||
buildExecutor.currentBuilds().firstOrNull { it.artifactKey == artifactKey }
|
buildExecutor.currentBuilds().firstOrNull { it.repo === repo && it.artifactKey == artifactKey }
|
||||||
?: return notFound("no running build with artifact key '$artifactKey'")
|
?: return notFound("no running build with artifact key '$artifactKey'")
|
||||||
return ResponseEntity.ok(readLogTail(artifactKey, build.liveLogFile, offset))
|
return ResponseEntity.ok(readLogTail(artifactKey, build.liveLogFile, offset))
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -106,7 +106,7 @@ class UiController(
|
|||||||
val links = baseModel(model, view = "current", pageTitle = "Current Builds")
|
val links = baseModel(model, view = "current", pageTitle = "Current Builds")
|
||||||
val results = repository.history()
|
val results = repository.history()
|
||||||
val currentBuilds =
|
val currentBuilds =
|
||||||
buildExecutor.currentBuilds().map { build ->
|
buildExecutor.currentBuilds().filter { it.repo === repo }.map { build ->
|
||||||
CurrentBuildView(
|
CurrentBuildView(
|
||||||
branch = build.branch,
|
branch = build.branch,
|
||||||
name = build.name,
|
name = build.name,
|
||||||
|
|||||||
@@ -506,7 +506,10 @@ class Watcher(
|
|||||||
}
|
}
|
||||||
val keep = originBranches.map { ArtifactKeys.branchKey(it) }.toMutableSet()
|
val keep = originBranches.map { ArtifactKeys.branchKey(it) }.toMutableSet()
|
||||||
// never delete under a build that is still queued or executing
|
// never delete under a build that is still queued or executing
|
||||||
buildExecutor.currentBuilds().forEach { keep += ArtifactKeys.branchKey(it.branch) }
|
buildExecutor
|
||||||
|
.currentBuilds()
|
||||||
|
.filter { it.repo === repo }
|
||||||
|
.forEach { keep += ArtifactKeys.branchKey(it.branch) }
|
||||||
repo.results
|
repo.results
|
||||||
.latestPerName()
|
.latestPerName()
|
||||||
.filter { it.status == BuildStatus.PENDING || it.status == BuildStatus.RUNNING }
|
.filter { it.status == BuildStatus.PENDING || it.status == BuildStatus.RUNNING }
|
||||||
|
|||||||
@@ -34,6 +34,7 @@ class ConsoleBuildRunnerTest : FunSpec() {
|
|||||||
|
|
||||||
private fun runningBuild(stagingDir: Path) =
|
private fun runningBuild(stagingDir: Path) =
|
||||||
RunningBuild(
|
RunningBuild(
|
||||||
|
repo = repo,
|
||||||
branch = "main",
|
branch = "main",
|
||||||
commit = "0123456789abcdef",
|
commit = "0123456789abcdef",
|
||||||
artifactKey = "main-key",
|
artifactKey = "main-key",
|
||||||
|
|||||||
@@ -12,6 +12,7 @@ import de.hoennig.werkator.repo.RepoContext
|
|||||||
import io.kotest.core.spec.style.FunSpec
|
import io.kotest.core.spec.style.FunSpec
|
||||||
import io.mockk.clearMocks
|
import io.mockk.clearMocks
|
||||||
import io.mockk.every
|
import io.mockk.every
|
||||||
|
import io.mockk.mockk
|
||||||
import io.mockk.verify
|
import io.mockk.verify
|
||||||
import org.springframework.beans.factory.annotation.Autowired
|
import org.springframework.beans.factory.annotation.Autowired
|
||||||
import org.springframework.boot.webmvc.test.autoconfigure.WebMvcTest
|
import org.springframework.boot.webmvc.test.autoconfigure.WebMvcTest
|
||||||
@@ -68,6 +69,7 @@ class BuildsApiControllerTest : FunSpec() {
|
|||||||
|
|
||||||
private fun runningBuild(liveLogFile: Path) =
|
private fun runningBuild(liveLogFile: Path) =
|
||||||
RunningBuild(
|
RunningBuild(
|
||||||
|
repo = repo,
|
||||||
branch = "main",
|
branch = "main",
|
||||||
commit = successResult.commit,
|
commit = successResult.commit,
|
||||||
artifactKey = "main-abc123-running",
|
artifactKey = "main-abc123-running",
|
||||||
@@ -133,6 +135,35 @@ class BuildsApiControllerTest : FunSpec() {
|
|||||||
.andExpect(jsonPath("$[0].logSize").value(5))
|
.andExpect(jsonPath("$[0].logSize").value(5))
|
||||||
}
|
}
|
||||||
|
|
||||||
|
test("current answers only the served repository's builds") {
|
||||||
|
val liveLogFile = Files.writeString(tempDir.resolve("mine.log"), "12345")
|
||||||
|
val mine = runningBuild(liveLogFile)
|
||||||
|
val foreign =
|
||||||
|
runningBuild(liveLogFile).copy(
|
||||||
|
repo = mockk<RepoContext>(),
|
||||||
|
artifactKey = "other-repo-running",
|
||||||
|
)
|
||||||
|
every { buildExecutor.currentBuilds() } returns listOf(mine, foreign)
|
||||||
|
every { repository.history() } returns
|
||||||
|
listOf(successResult.copy(status = BuildStatus.RUNNING, artifactKey = mine.artifactKey))
|
||||||
|
|
||||||
|
mockMvc
|
||||||
|
.perform(get("/api/builds/current"))
|
||||||
|
.andExpect(status().isOk)
|
||||||
|
.andExpect(jsonPath("$.length()").value(1))
|
||||||
|
.andExpect(jsonPath("$[0].artifactKey").value(mine.artifactKey))
|
||||||
|
}
|
||||||
|
|
||||||
|
test("current log of a build in another repository answers 404") {
|
||||||
|
val liveLogFile = Files.writeString(tempDir.resolve("foreign.log"), "hello world")
|
||||||
|
val foreign = runningBuild(liveLogFile).copy(repo = mockk<RepoContext>())
|
||||||
|
every { buildExecutor.currentBuilds() } returns listOf(foreign)
|
||||||
|
|
||||||
|
mockMvc
|
||||||
|
.perform(get("/api/builds/current/${foreign.artifactKey}/log"))
|
||||||
|
.andExpect(status().isNotFound)
|
||||||
|
}
|
||||||
|
|
||||||
test("current log answers the tail from the requested offset") {
|
test("current log answers the tail from the requested offset") {
|
||||||
val liveLogFile = Files.writeString(tempDir.resolve("tail.log"), "hello world")
|
val liveLogFile = Files.writeString(tempDir.resolve("tail.log"), "hello world")
|
||||||
val build = runningBuild(liveLogFile)
|
val build = runningBuild(liveLogFile)
|
||||||
|
|||||||
@@ -246,6 +246,7 @@ class UiControllerTest : FunSpec() {
|
|||||||
test("current view renders a card per running build with cancel button and started-at attribute") {
|
test("current view renders a card per running build with cancel button and started-at attribute") {
|
||||||
val build =
|
val build =
|
||||||
RunningBuild(
|
RunningBuild(
|
||||||
|
repo = repo,
|
||||||
branch = "main",
|
branch = "main",
|
||||||
commit = successResult.commit,
|
commit = successResult.commit,
|
||||||
artifactKey = "main-abc123-running",
|
artifactKey = "main-abc123-running",
|
||||||
|
|||||||
@@ -94,7 +94,7 @@ class WatcherTest : FunSpec() {
|
|||||||
val branch = secondArg<String>()
|
val branch = secondArg<String>()
|
||||||
val commit = thirdArg<String>()
|
val commit = thirdArg<String>()
|
||||||
startedBuilds += branch to commit
|
startedBuilds += branch to commit
|
||||||
runningBuild(branch, commit)
|
runningBuild(repo, branch, commit)
|
||||||
}
|
}
|
||||||
every { artifactStore.prune(any()) } returns emptyList()
|
every { artifactStore.prune(any()) } returns emptyList()
|
||||||
}
|
}
|
||||||
@@ -131,11 +131,13 @@ class WatcherTest : FunSpec() {
|
|||||||
}
|
}
|
||||||
|
|
||||||
private fun runningBuild(
|
private fun runningBuild(
|
||||||
|
repo: RepoContext,
|
||||||
branch: String,
|
branch: String,
|
||||||
commit: String,
|
commit: String,
|
||||||
): RunningBuild {
|
): RunningBuild {
|
||||||
val stagingDir = Files.createTempDirectory("werkator-watcher-staging")
|
val stagingDir = Files.createTempDirectory("werkator-watcher-staging")
|
||||||
return RunningBuild(
|
return RunningBuild(
|
||||||
|
repo = repo,
|
||||||
branch = branch,
|
branch = branch,
|
||||||
commit = commit,
|
commit = commit,
|
||||||
artifactKey = ArtifactKeys.buildKey(branch, Instant.now()),
|
artifactKey = ArtifactKeys.buildKey(branch, Instant.now()),
|
||||||
@@ -272,7 +274,7 @@ class WatcherTest : FunSpec() {
|
|||||||
test("a poll cycle completes while a build is running and still enqueues other branches") {
|
test("a poll cycle completes while a build is running and still enqueues other branches") {
|
||||||
val harness = Harness()
|
val harness = Harness()
|
||||||
harness.seed("main", BuildStatus.RUNNING, commit = "commit-1")
|
harness.seed("main", BuildStatus.RUNNING, commit = "commit-1")
|
||||||
every { harness.buildExecutor.currentBuilds() } returns listOf(runningBuild("main", "commit-1"))
|
every { harness.buildExecutor.currentBuilds() } returns listOf(runningBuild(harness.repo, "main", "commit-1"))
|
||||||
every { harness.gitService.originBranches(any()) } returns listOf("main", "feature/other")
|
every { harness.gitService.originBranches(any()) } returns listOf("main", "feature/other")
|
||||||
every { harness.gitService.localBranches(any()) } returns listOf("main")
|
every { harness.gitService.localBranches(any()) } returns listOf("main")
|
||||||
every { harness.gitService.hasNewCommits("main", any()) } returns true
|
every { harness.gitService.hasNewCommits("main", any()) } returns true
|
||||||
@@ -767,6 +769,28 @@ class WatcherTest : FunSpec() {
|
|||||||
Files.exists(busyWorktree).shouldBeTrue()
|
Files.exists(busyWorktree).shouldBeTrue()
|
||||||
}
|
}
|
||||||
|
|
||||||
|
test("a running build of another repository does not keep this repository's worktree") {
|
||||||
|
val harness = Harness()
|
||||||
|
harness.seed("gone", BuildStatus.SUCCESS, commit = "commit-1")
|
||||||
|
val goneWorktree = harness.worktreeDir("gone")
|
||||||
|
val otherDir = Files.createTempDirectory("werkator-watcher-other-running")
|
||||||
|
val other =
|
||||||
|
RepoContext(
|
||||||
|
"other",
|
||||||
|
otherDir,
|
||||||
|
FileBuildResultRepository(otherDir.resolve(".git/werkator/build-results.json")),
|
||||||
|
harness.artifactStore,
|
||||||
|
)
|
||||||
|
// the other repository builds a branch of the same name — its build must not
|
||||||
|
// protect this repository's worktree, whose branch is gone from origin
|
||||||
|
every { harness.buildExecutor.currentBuilds() } returns listOf(runningBuild(other, "gone", "commit-other"))
|
||||||
|
every { harness.gitService.originBranches(any()) } returns emptyList()
|
||||||
|
|
||||||
|
harness.watcher.poll(harness.repo)
|
||||||
|
|
||||||
|
Files.exists(goneWorktree).shouldBeFalse()
|
||||||
|
}
|
||||||
|
|
||||||
test("one repository's unreachable origin neither stops nor silences the other") {
|
test("one repository's unreachable origin neither stops nor silences the other") {
|
||||||
val harness = Harness()
|
val harness = Harness()
|
||||||
val otherDir = Files.createTempDirectory("werkator-watcher-other")
|
val otherDir = Files.createTempDirectory("werkator-watcher-other")
|
||||||
|
|||||||
Reference in New Issue
Block a user