diff --git a/.claude/skills/architecture/SKILL.md b/.claude/skills/architecture/SKILL.md index c976dd7..5d67090 100644 --- a/.claude/skills/architecture/SKILL.md +++ b/.claude/skills/architecture/SKILL.md @@ -63,7 +63,7 @@ Three places must stay in sync when config keys change: the `WerkatorConfig` dat ## 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 diff --git a/docs/plan/22-multi-repo.md b/docs/plan/22-multi-repo.md index 6cb91d6..65c8efd 100644 --- a/docs/plan/22-multi-repo.md +++ b/docs/plan/22-multi-repo.md @@ -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. - ~~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 ` (`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). -- 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 diff --git a/src/main/kotlin/de/hoennig/werkator/build/BuildExecutor.kt b/src/main/kotlin/de/hoennig/werkator/build/BuildExecutor.kt index 927783f..ed7e8cd 100644 --- a/src/main/kotlin/de/hoennig/werkator/build/BuildExecutor.kt +++ b/src/main/kotlin/de/hoennig/werkator/build/BuildExecutor.kt @@ -97,6 +97,7 @@ class BuildExecutor( val stagingDir = Files.createTempDirectory("werkator-build-") val runningBuild = RunningBuild( + repo = repo, branch = branch, build = build, commit = commit, diff --git a/src/main/kotlin/de/hoennig/werkator/build/RunningBuild.kt b/src/main/kotlin/de/hoennig/werkator/build/RunningBuild.kt index d1178f2..dc81a83 100644 --- a/src/main/kotlin/de/hoennig/werkator/build/RunningBuild.kt +++ b/src/main/kotlin/de/hoennig/werkator/build/RunningBuild.kt @@ -1,11 +1,19 @@ package de.hoennig.werkator.build import de.hoennig.werkator.config.BuildDefinition +import de.hoennig.werkator.repo.RepoContext import java.nio.file.Path import java.time.Instant /** Handle to a build accepted by the [BuildExecutor]; log paths become valid once the build runs. */ 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. */ val branch: String, /** The build definition (job) this build runs; its settings are resolved from config at run time. */ diff --git a/src/main/kotlin/de/hoennig/werkator/server/BuildsApiController.kt b/src/main/kotlin/de/hoennig/werkator/server/BuildsApiController.kt index ce4c5da..84c8ceb 100644 --- a/src/main/kotlin/de/hoennig/werkator/server/BuildsApiController.kt +++ b/src/main/kotlin/de/hoennig/werkator/server/BuildsApiController.kt @@ -55,11 +55,17 @@ class BuildsApiController( 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") fun current(): List { val results = repository.history() - return buildExecutor.currentBuilds().map { build -> + return buildExecutor.currentBuilds().filter { it.repo === repo }.map { build -> CurrentBuildDto( branch = build.branch, name = build.name, @@ -82,7 +88,7 @@ class BuildsApiController( @RequestParam(defaultValue = "0") offset: Long, ): ResponseEntity { 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 ResponseEntity.ok(readLogTail(artifactKey, build.liveLogFile, offset)) } diff --git a/src/main/kotlin/de/hoennig/werkator/server/UiController.kt b/src/main/kotlin/de/hoennig/werkator/server/UiController.kt index f5cfe76..196527e 100644 --- a/src/main/kotlin/de/hoennig/werkator/server/UiController.kt +++ b/src/main/kotlin/de/hoennig/werkator/server/UiController.kt @@ -106,7 +106,7 @@ class UiController( val links = baseModel(model, view = "current", pageTitle = "Current Builds") val results = repository.history() val currentBuilds = - buildExecutor.currentBuilds().map { build -> + buildExecutor.currentBuilds().filter { it.repo === repo }.map { build -> CurrentBuildView( branch = build.branch, name = build.name, diff --git a/src/main/kotlin/de/hoennig/werkator/watcher/Watcher.kt b/src/main/kotlin/de/hoennig/werkator/watcher/Watcher.kt index 2f1fcd1..bdb627d 100644 --- a/src/main/kotlin/de/hoennig/werkator/watcher/Watcher.kt +++ b/src/main/kotlin/de/hoennig/werkator/watcher/Watcher.kt @@ -506,7 +506,10 @@ class Watcher( } val keep = originBranches.map { ArtifactKeys.branchKey(it) }.toMutableSet() // 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 .latestPerName() .filter { it.status == BuildStatus.PENDING || it.status == BuildStatus.RUNNING } diff --git a/src/test/kotlin/de/hoennig/werkator/commands/ConsoleBuildRunnerTest.kt b/src/test/kotlin/de/hoennig/werkator/commands/ConsoleBuildRunnerTest.kt index 8558f39..01ac78e 100644 --- a/src/test/kotlin/de/hoennig/werkator/commands/ConsoleBuildRunnerTest.kt +++ b/src/test/kotlin/de/hoennig/werkator/commands/ConsoleBuildRunnerTest.kt @@ -34,6 +34,7 @@ class ConsoleBuildRunnerTest : FunSpec() { private fun runningBuild(stagingDir: Path) = RunningBuild( + repo = repo, branch = "main", commit = "0123456789abcdef", artifactKey = "main-key", diff --git a/src/test/kotlin/de/hoennig/werkator/server/BuildsApiControllerTest.kt b/src/test/kotlin/de/hoennig/werkator/server/BuildsApiControllerTest.kt index 9dbd121..e5f9d78 100644 --- a/src/test/kotlin/de/hoennig/werkator/server/BuildsApiControllerTest.kt +++ b/src/test/kotlin/de/hoennig/werkator/server/BuildsApiControllerTest.kt @@ -12,6 +12,7 @@ import de.hoennig.werkator.repo.RepoContext import io.kotest.core.spec.style.FunSpec import io.mockk.clearMocks import io.mockk.every +import io.mockk.mockk import io.mockk.verify import org.springframework.beans.factory.annotation.Autowired import org.springframework.boot.webmvc.test.autoconfigure.WebMvcTest @@ -68,6 +69,7 @@ class BuildsApiControllerTest : FunSpec() { private fun runningBuild(liveLogFile: Path) = RunningBuild( + repo = repo, branch = "main", commit = successResult.commit, artifactKey = "main-abc123-running", @@ -133,6 +135,35 @@ class BuildsApiControllerTest : FunSpec() { .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(), + 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()) + 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") { val liveLogFile = Files.writeString(tempDir.resolve("tail.log"), "hello world") val build = runningBuild(liveLogFile) diff --git a/src/test/kotlin/de/hoennig/werkator/server/UiControllerTest.kt b/src/test/kotlin/de/hoennig/werkator/server/UiControllerTest.kt index 6dcfd43..c2bbbc7 100644 --- a/src/test/kotlin/de/hoennig/werkator/server/UiControllerTest.kt +++ b/src/test/kotlin/de/hoennig/werkator/server/UiControllerTest.kt @@ -246,6 +246,7 @@ class UiControllerTest : FunSpec() { test("current view renders a card per running build with cancel button and started-at attribute") { val build = RunningBuild( + repo = repo, branch = "main", commit = successResult.commit, artifactKey = "main-abc123-running", diff --git a/src/test/kotlin/de/hoennig/werkator/watcher/WatcherTest.kt b/src/test/kotlin/de/hoennig/werkator/watcher/WatcherTest.kt index be58238..218d798 100644 --- a/src/test/kotlin/de/hoennig/werkator/watcher/WatcherTest.kt +++ b/src/test/kotlin/de/hoennig/werkator/watcher/WatcherTest.kt @@ -94,7 +94,7 @@ class WatcherTest : FunSpec() { val branch = secondArg() val commit = thirdArg() startedBuilds += branch to commit - runningBuild(branch, commit) + runningBuild(repo, branch, commit) } every { artifactStore.prune(any()) } returns emptyList() } @@ -131,11 +131,13 @@ class WatcherTest : FunSpec() { } private fun runningBuild( + repo: RepoContext, branch: String, commit: String, ): RunningBuild { val stagingDir = Files.createTempDirectory("werkator-watcher-staging") return RunningBuild( + repo = repo, branch = branch, commit = commit, 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") { val harness = Harness() 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.localBranches(any()) } returns listOf("main") every { harness.gitService.hasNewCommits("main", any()) } returns true @@ -767,6 +769,28 @@ class WatcherTest : FunSpec() { 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") { val harness = Harness() val otherDir = Files.createTempDirectory("werkator-watcher-other")