diff --git a/build.gradle.kts b/build.gradle.kts index 7669afe..f5e7b85 100644 --- a/build.gradle.kts +++ b/build.gradle.kts @@ -12,7 +12,7 @@ group = "de.hoennig" // bump at least the patch version for every deployment — and only then, not per commit — // so the UI footer (BuildProperties), --version and the release notes identify what is // actually running; a deployment bundles whatever was committed since the last one -version = "1.0.0" +version = "1.0.1" java { toolchain { diff --git a/docs/prs/2026-08-31-PR#000-build-current-head-from-the-branches-view.md b/docs/prs/2026-08-31-PR#000-build-current-head-from-the-branches-view.md new file mode 100644 index 0000000..2c58e45 --- /dev/null +++ b/docs/prs/2026-08-31-PR#000-build-current-head-from-the-branches-view.md @@ -0,0 +1,105 @@ +> **WARNING:** This document describes only the change applied in this PR. +> It may already be outdated once the next PR is merged. +> Historic PR-documentation is not maintained along with new PRs — treat it as a snapshot, not as current documentation. + +## The Problem + +The restart button repeats the commit a build was recorded on. +On the History and Latest views that is exactly right: a row there is a past run, and repeating it means that run. + +On the Branches view it is wrong, because a row there is not a run — it is a branch, listed whether it was ever built or not. +Pressing restart on `master` therefore rebuilt whatever commit master happened to point at the last time it was built, not what master is now. + +Repeating an overtaken commit answers a question nobody asked, and it can be worse than useless. +A build gate that compares the built commit against origin cannot pass on a superseded commit at all: hs.hsadmin.ng's `prQuickCheck` asserts that HEAD contains `origin/master`, which an overtaken master commit can never do. +Observed in production on 2026-08-31: a restart of master rebuilt a commit from two days earlier and failed, and every further press produced the same red result, because pressing it again cannot change the commit. + +## Non-Goals + +- No change to the Latest and History views: a row there stands for a recorded run, and repeating it is the point. +- No change to which build definition a restart uses, and no way to pick one — the row's own definition is re-run either way. +- No change to the watcher, which decides on its own what to build and when. +- No new button: the existing one changes what it does on one view, and says so. + +## The Scenarios + +### Feature: a restart on the Branches view builds the branch as it is now + +#### Background + +- A row on `/branches` stands for a branch of origin, with its latest build or an `unknown` row. +- A row on `/` (Latest) and `/history` stands for a recorded build. +- A build *name* is the pool: the branch itself for the default build, `@` for a named one. + +#### Scenario#000.01: The Branches view builds the branch's current origin head + +So that a restart answers "build this branch as it is", which is what a branch row means. + +- **Given** a branch whose last recorded build ran on an overtaken commit +- **When** the restart button on the Branches view is pressed +- **Then** the branch's current origin head is built + - **and** the recorded commit is not built + +##### Verified by + +- [BuildsApiControllerTest: "restart with atOriginHead builds the branch as it is now, not the recorded commit"](../../src/test/kotlin/de/hoennig/werkator/server/BuildsApiControllerTest.kt) +- [UiControllerTest: "the branches view restarts at the branch's origin head, the latest view repeats the run"](../../src/test/kotlin/de/hoennig/werkator/server/UiControllerTest.kt) + +#### Scenario#000.02: The row keeps its build definition and its real branch + +So that restarting a named build does not silently turn it into a different build. + +- **Given** a row for a named build such as `main@pitest` +- **When** it is restarted from the Branches view +- **Then** the new commit is built under that same definition + - **and** on the branch the record names, not on the pool name + +##### Verified by + +- [BuildsApiControllerTest: "restart with atOriginHead keeps the recorded build definition and its real branch"](../../src/test/kotlin/de/hoennig/werkator/server/BuildsApiControllerTest.kt) + +#### Scenario#000.03: A branch that is gone from origin is refused by name + +So that a restart cannot quietly fall back to a commit the user did not ask for. + +- **Given** a row for a branch that no longer exists on origin +- **When** it is restarted from the Branches view +- **Then** the request is refused, naming the branch + - **and** no build is started + +##### Verified by + +- [BuildsApiControllerTest: "restart with atOriginHead of a branch gone from origin is refused by name"](../../src/test/kotlin/de/hoennig/werkator/server/BuildsApiControllerTest.kt) + +#### Scenario#000.04: Latest and History still repeat the recorded run + +So that the one view whose rows are runs keeps the behavior that fits them. + +- **Given** a row on the Latest view +- **When** its restart button is pressed +- **Then** the recorded commit is built, as before + +##### Verified by + +- [UiControllerTest: "the branches view restarts at the branch's origin head, the latest view repeats the run"](../../src/test/kotlin/de/hoennig/werkator/server/UiControllerTest.kt) +- [BuildsApiControllerTest: "restart enqueues the branch's last recorded commit, also for branch names with slashes"](../../src/test/kotlin/de/hoennig/werkator/server/BuildsApiControllerTest.kt) + +## The Solution + +**One flag, decided by the view, not by the button.** +`/api/builds/restart` takes `atOriginHead`; with it the commit comes from `GitService.originHeadCommit` instead of the recorded build. +`UiController` sets `restartAtOriginHead` per view — true for Branches, false for Latest and History — and the template and `werkator.js` carry it to the button. +The decision therefore lives where the meaning of a row is decided, and there is one endpoint, one token check, one place to change. + +**The button says what it does.** +On the Branches view it reads "Build current head" instead of "Restart build", server-rendered and in the JavaScript alike. +A button whose label promises a repeat while it builds something else would be the same class of quiet surprise this change removes. + +**A missing branch is an error, not a fallback.** +Falling back to the recorded commit would produce exactly the behavior the user asked to leave behind, at the moment they can least expect it. +The message names the branch. + +## Additional Changes + +- The endpoint's parameter is still called `branch` although the views pass a build *name* (`main@pitest`). + It resolves correctly — the recorded branch is used to build — but the name is misleading and is left for a separate change, to keep this one reviewable. diff --git a/src/main/kotlin/de/hoennig/werkator/server/BuildsApiController.kt b/src/main/kotlin/de/hoennig/werkator/server/BuildsApiController.kt index 0fb408b..e91cd8a 100644 --- a/src/main/kotlin/de/hoennig/werkator/server/BuildsApiController.kt +++ b/src/main/kotlin/de/hoennig/werkator/server/BuildsApiController.kt @@ -86,28 +86,43 @@ class BuildsApiController( } /** - * Re-enqueues the last recorded commit of the build name [branch] — or the origin - * head for a branch never built, so the branches view can trigger first builds like - * legacy. A restarted build re-runs its recorded build definition, with the - * settings from the current configuration. + * Re-enqueues the build name [branch] — or the origin head for a branch never + * built, so the branches view can trigger first builds like legacy. A restarted + * build re-runs its recorded build definition, with the settings from the current + * configuration. + * + * With [atOriginHead] the branch's current origin head is built instead of the + * recorded commit. That is what the branches view asks for: a row there stands for + * a branch, not for a past run, and repeating an overtaken commit answers a + * question nobody asked — a build gate comparing against origin cannot even pass + * on it. Latest and history mean the recorded run, and keep repeating it. + * * The name is a parameter, not a path variable, because branch names may contain * slashes (Tomcat rejects encoded slashes in the path by default). */ @PostMapping("/api/builds/restart") fun restart( @RequestParam branch: String, + @RequestParam(defaultValue = "false") atOriginHead: Boolean, @RequestHeader(name = TOKEN_HEADER, required = false) headerToken: String?, ): ResponseEntity { rejectBadToken(headerToken)?.let { return it } val latest = repository.latestFor(branch) + // the name may be a pool like `main@pitest`; the branch to build is the recorded one + val branchName = latest?.branch ?: branch val commit = - latest?.commit - ?: gitService.originHeadCommit(branch, workingDir) - ?: return notFound("branch '$branch' has no recorded build and no origin counterpart") + if (atOriginHead) { + gitService.originHeadCommit(branchName, workingDir) + ?: return notFound("branch '$branchName' is not on origin") + } else { + latest?.commit + ?: gitService.originHeadCommit(branchName, workingDir) + ?: return notFound("branch '$branch' has no recorded build and no origin counterpart") + } // a restarted build re-runs its recorded build definition (settings from the current config) val running = buildExecutor.startBuild( - branch = latest?.branch ?: branch, + branch = branchName, commit = commit, build = latest?.build ?: BuildDefinition.DEFAULT, ) diff --git a/src/main/kotlin/de/hoennig/werkator/server/UiController.kt b/src/main/kotlin/de/hoennig/werkator/server/UiController.kt index d198c9c..925ed1d 100644 --- a/src/main/kotlin/de/hoennig/werkator/server/UiController.kt +++ b/src/main/kotlin/de/hoennig/werkator/server/UiController.kt @@ -62,6 +62,7 @@ class UiController( model.addAttribute("rows", repository.latestPerName().map { BuildRowView.from(it, links, permanentUrlOf(it)) }) model.addAttribute("apiPath", "/api/builds/latest") model.addAttribute("allowRestart", true) + model.addAttribute("restartAtOriginHead", false) model.addAttribute("emptyMessage", "No builds recorded yet.") return "builds" } @@ -73,6 +74,8 @@ class UiController( model.addAttribute("rows", branchListing.branches(workingDir).map { BuildRowView.from(it, links) }) model.addAttribute("apiPath", "/api/branches") model.addAttribute("allowRestart", true) + // a row here stands for a branch, not for a past run + model.addAttribute("restartAtOriginHead", true) model.addAttribute("emptyMessage", "No branches found on origin.") return "builds" } @@ -83,6 +86,7 @@ class UiController( model.addAttribute("rows", repository.history().map { BuildRowView.from(it, links, permanentUrlOf(it)) }) model.addAttribute("apiPath", "/api/builds/history") model.addAttribute("allowRestart", false) + model.addAttribute("restartAtOriginHead", false) model.addAttribute("emptyMessage", "No builds archived yet.") return "builds" } diff --git a/src/main/resources/static/werkator.js b/src/main/resources/static/werkator.js index 74253ad..f3b2be9 100644 --- a/src/main/resources/static/werkator.js +++ b/src/main/resources/static/werkator.js @@ -336,7 +336,7 @@ function actionButton(symbol, title, className, dataset) { // ---- latest/history table -------------------------------------------------- -function renderBuildRow(build, allowRestart) { +function renderBuildRow(build, allowRestart, restartAtOriginHead) { const row = document.createElement("tr"); const displayName = build.name || build.branch; row.dataset.artifactKey = build.artifactKey || ""; @@ -413,7 +413,15 @@ function renderBuildRow(build, allowRestart) { const actionsCell = elem("td", "actions-cell"); const actions = elem("div", "actions"); if (allowRestart) { - actions.appendChild(actionButton("↻", "Restart build", null, { action: "restart", branch: displayName })); + // the branches view builds the branch as it is now, the other views repeat a run + const restartTitle = restartAtOriginHead ? "Build current head" : "Restart build"; + actions.appendChild( + actionButton("↻", restartTitle, null, { + action: "restart", + branch: displayName, + atOriginHead: restartAtOriginHead ? "true" : "false", + }), + ); } if (build.artifactKey) { actions.appendChild( @@ -439,6 +447,7 @@ function initBuildsTable() { } const tbody = document.getElementById("build-rows"); const allowRestart = table.dataset.allowRestart === "true"; + const restartAtOriginHead = table.dataset.restartAtOriginHead === "true"; async function refresh() { const builds = await fetchJson(table.dataset.api); @@ -449,7 +458,7 @@ function initBuildsTable() { tbody.appendChild(elem("tr")).appendChild(cell); return; } - builds.forEach((build) => tbody.appendChild(renderBuildRow(build, allowRestart))); + builds.forEach((build) => tbody.appendChild(renderBuildRow(build, allowRestart, restartAtOriginHead))); } startPolling(refresh, TABLE_POLL_MS); @@ -680,7 +689,8 @@ document.addEventListener("click", async (event) => { button.disabled = true; try { if (action === "restart") { - await sendAction("/api/builds/restart?branch=" + encodeURIComponent(button.dataset.branch), "POST"); + const atOriginHead = button.dataset.atOriginHead === "true" ? "&atOriginHead=true" : ""; + await sendAction("/api/builds/restart?branch=" + encodeURIComponent(button.dataset.branch) + atOriginHead, "POST"); } else if (action === "cancel") { await sendAction(`/api/builds/${encodeURIComponent(button.dataset.artifactKey)}/cancel`, "POST"); } else if (action === "delete") { diff --git a/src/main/resources/templates/builds.html b/src/main/resources/templates/builds.html index 6056fe1..777e42d 100644 --- a/src/main/resources/templates/builds.html +++ b/src/main/resources/templates/builds.html @@ -7,7 +7,7 @@
+ th:attr="data-api=${apiPath},data-allow-restart=${allowRestart},data-restart-at-origin-head=${restartAtOriginHead},data-empty-message=${emptyMessage}"> @@ -65,8 +65,9 @@
Status
+ th:attr="data-branch=${row.name},data-at-origin-head=${restartAtOriginHead}" + th:title="${restartAtOriginHead} ? 'Build current head' : 'Restart build'" + th:aria-label="${restartAtOriginHead} ? 'Build current head' : 'Restart build'">↻ diff --git a/src/main/resources/templates/releases.html b/src/main/resources/templates/releases.html index 6022580..5b19486 100644 --- a/src/main/resources/templates/releases.html +++ b/src/main/resources/templates/releases.html @@ -7,6 +7,19 @@
+

v1.0.1 — 2026-08-31

+
    +
  • The restart button on the Branches view builds the branch's current head + instead of repeating the commit its last build ran on, and says so: it reads + "Build current head" there. A row on that page is a branch, not a past run — and + repeating an overtaken commit can be worse than useless, because a build gate + comparing the built commit against origin can never pass on it. The row keeps its + build definition, and a branch that is gone from origin is refused by name instead + of quietly falling back to the old commit.
    • +
  • Latest and History are unchanged: a row there is a recorded run, + and restarting it means that commit.
    • +
+

v1.0.0 — 2026-08-31

  • GitTally is now Werkator. The old name already belongs to another diff --git a/src/test/kotlin/de/hoennig/werkator/server/BuildsApiControllerTest.kt b/src/test/kotlin/de/hoennig/werkator/server/BuildsApiControllerTest.kt index b296bb5..aa706e3 100644 --- a/src/test/kotlin/de/hoennig/werkator/server/BuildsApiControllerTest.kt +++ b/src/test/kotlin/de/hoennig/werkator/server/BuildsApiControllerTest.kt @@ -186,6 +186,58 @@ class BuildsApiControllerTest : FunSpec() { verify { buildExecutor.startBuild("main", successResult.commit, build = "pitest") } } + test("restart with atOriginHead builds the branch as it is now, not the recorded commit") { + val liveLogFile = tempDir.resolve("head-restart.log") + every { repository.latestFor("main") } returns successResult + every { gitService.originHeadCommit("main", any()) } returns "newhead1" + every { buildExecutor.startBuild("main", "newhead1") } returns runningBuild(liveLogFile) + + mockMvc + .perform( + post("/api/builds/restart") + .param("branch", "main") + .param("atOriginHead", "true") + .header(BuildsApiController.TOKEN_HEADER, "secret"), + ).andExpect(status().isAccepted) + + // the recorded commit is deliberately not used: a branches row stands for a branch + verify { buildExecutor.startBuild("main", "newhead1") } + verify(exactly = 0) { buildExecutor.startBuild("main", successResult.commit) } + } + + test("restart with atOriginHead keeps the recorded build definition and its real branch") { + val liveLogFile = tempDir.resolve("head-named.log") + every { repository.latestFor("main@pitest") } returns successResult.copy(build = "pitest", name = "main@pitest") + every { gitService.originHeadCommit("main", any()) } returns "newhead2" + every { buildExecutor.startBuild("main", "newhead2", build = "pitest") } returns + runningBuild(liveLogFile).copy(build = "pitest", name = "main@pitest") + + mockMvc + .perform( + post("/api/builds/restart") + .param("branch", "main@pitest") + .param("atOriginHead", "true") + .header(BuildsApiController.TOKEN_HEADER, "secret"), + ).andExpect(status().isAccepted) + + verify { buildExecutor.startBuild("main", "newhead2", build = "pitest") } + } + + test("restart with atOriginHead of a branch gone from origin is refused by name") { + every { repository.latestFor("gone") } returns successResult.copy(branch = "gone", name = "gone") + every { gitService.originHeadCommit("gone", any()) } returns null + + mockMvc + .perform( + post("/api/builds/restart") + .param("branch", "gone") + .param("atOriginHead", "true") + .header(BuildsApiController.TOKEN_HEADER, "secret"), + ).andExpect(status().isNotFound) + + verify(exactly = 0) { buildExecutor.startBuild(any(), any(), any(), any()) } + } + test("restart of a never-built branch enqueues its origin head commit") { val liveLogFile = tempDir.resolve("first-build.log") every { repository.latestFor("fresh") } returns null diff --git a/src/test/kotlin/de/hoennig/werkator/server/UiControllerTest.kt b/src/test/kotlin/de/hoennig/werkator/server/UiControllerTest.kt index f78aeb8..f35baa6 100644 --- a/src/test/kotlin/de/hoennig/werkator/server/UiControllerTest.kt +++ b/src/test/kotlin/de/hoennig/werkator/server/UiControllerTest.kt @@ -177,6 +177,24 @@ class UiControllerTest : FunSpec() { .andExpect(content().string(containsString("Permanent link"))) } + test("the branches view restarts at the branch's origin head, the latest view repeats the run") { + every { branchListing.branches(any()) } returns + listOf(BranchDto.from("main", "ignored-head", successResult, isLatestGreen = true)) + every { repository.latestPerName() } returns listOf(successResult) + + // a row on /branches stands for a branch, so its button builds the branch as it is now + mockMvc + .perform(get("/branches")) + .andExpect(content().string(containsString("""data-restart-at-origin-head="true""""))) + .andExpect(content().string(containsString("Build current head"))) + + // a row on / stands for a recorded run, and repeating it means that commit + mockMvc + .perform(get("/")) + .andExpect(content().string(containsString("""data-restart-at-origin-head="false""""))) + .andExpect(content().string(containsString("Restart build"))) + } + test("the permanent link shows on the branch's latest green build only, the live link while it runs") { val running = successResult.copy(status = BuildStatus.RUNNING, duration = null, artifactKey = "running-key") every { repository.history() } returns listOf(running, successResult)