Merge branch 'main' into 20-build-duration-tracking

This commit is contained in:
Michael Hönnig
2026-08-31 13:58:24 +02:00
committed by GitHub
9 changed files with 234 additions and 16 deletions
+1 -1
View File
@@ -12,7 +12,7 @@ group = "de.hoennig"
// bump at least the patch version for every deployment — and only then, not per commit — // 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 // 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 // actually running; a deployment bundles whatever was committed since the last one
version = "1.0.0" version = "1.0.1"
java { java {
toolchain { toolchain {
@@ -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, `<branch>@<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.
@@ -86,28 +86,43 @@ class BuildsApiController(
} }
/** /**
* Re-enqueues the last recorded commit of the build name [branch] — or the origin * Re-enqueues the build name [branch] — or the origin head for a branch never
* head for a branch never built, so the branches view can trigger first builds like * built, so the branches view can trigger first builds like legacy. A restarted
* legacy. A restarted build re-runs its recorded build definition, with the * build re-runs its recorded build definition, with the settings from the current
* settings from the current configuration. * 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 * The name is a parameter, not a path variable, because branch names may contain
* slashes (Tomcat rejects encoded slashes in the path by default). * slashes (Tomcat rejects encoded slashes in the path by default).
*/ */
@PostMapping("/api/builds/restart") @PostMapping("/api/builds/restart")
fun restart( fun restart(
@RequestParam branch: String, @RequestParam branch: String,
@RequestParam(defaultValue = "false") atOriginHead: Boolean,
@RequestHeader(name = TOKEN_HEADER, required = false) headerToken: String?, @RequestHeader(name = TOKEN_HEADER, required = false) headerToken: String?,
): ResponseEntity<Any> { ): ResponseEntity<Any> {
rejectBadToken(headerToken)?.let { return it } rejectBadToken(headerToken)?.let { return it }
val latest = repository.latestFor(branch) 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 = val commit =
latest?.commit if (atOriginHead) {
?: gitService.originHeadCommit(branch, workingDir) gitService.originHeadCommit(branchName, workingDir)
?: return notFound("branch '$branch' has no recorded build and no origin counterpart") ?: 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) // a restarted build re-runs its recorded build definition (settings from the current config)
val running = val running =
buildExecutor.startBuild( buildExecutor.startBuild(
branch = latest?.branch ?: branch, branch = branchName,
commit = commit, commit = commit,
build = latest?.build ?: BuildDefinition.DEFAULT, build = latest?.build ?: BuildDefinition.DEFAULT,
) )
@@ -62,6 +62,7 @@ class UiController(
model.addAttribute("rows", repository.latestPerName().map { BuildRowView.from(it, links, permanentUrlOf(it)) }) model.addAttribute("rows", repository.latestPerName().map { BuildRowView.from(it, links, permanentUrlOf(it)) })
model.addAttribute("apiPath", "/api/builds/latest") model.addAttribute("apiPath", "/api/builds/latest")
model.addAttribute("allowRestart", true) model.addAttribute("allowRestart", true)
model.addAttribute("restartAtOriginHead", false)
model.addAttribute("emptyMessage", "No builds recorded yet.") model.addAttribute("emptyMessage", "No builds recorded yet.")
return "builds" return "builds"
} }
@@ -73,6 +74,8 @@ class UiController(
model.addAttribute("rows", branchListing.branches(workingDir).map { BuildRowView.from(it, links) }) model.addAttribute("rows", branchListing.branches(workingDir).map { BuildRowView.from(it, links) })
model.addAttribute("apiPath", "/api/branches") model.addAttribute("apiPath", "/api/branches")
model.addAttribute("allowRestart", true) 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.") model.addAttribute("emptyMessage", "No branches found on origin.")
return "builds" return "builds"
} }
@@ -83,6 +86,7 @@ class UiController(
model.addAttribute("rows", repository.history().map { BuildRowView.from(it, links, permanentUrlOf(it)) }) model.addAttribute("rows", repository.history().map { BuildRowView.from(it, links, permanentUrlOf(it)) })
model.addAttribute("apiPath", "/api/builds/history") model.addAttribute("apiPath", "/api/builds/history")
model.addAttribute("allowRestart", false) model.addAttribute("allowRestart", false)
model.addAttribute("restartAtOriginHead", false)
model.addAttribute("emptyMessage", "No builds archived yet.") model.addAttribute("emptyMessage", "No builds archived yet.")
return "builds" return "builds"
} }
+14 -4
View File
@@ -336,7 +336,7 @@ function actionButton(symbol, title, className, dataset) {
// ---- latest/history table -------------------------------------------------- // ---- latest/history table --------------------------------------------------
function renderBuildRow(build, allowRestart) { function renderBuildRow(build, allowRestart, restartAtOriginHead) {
const row = document.createElement("tr"); const row = document.createElement("tr");
const displayName = build.name || build.branch; const displayName = build.name || build.branch;
row.dataset.artifactKey = build.artifactKey || ""; row.dataset.artifactKey = build.artifactKey || "";
@@ -413,7 +413,15 @@ function renderBuildRow(build, allowRestart) {
const actionsCell = elem("td", "actions-cell"); const actionsCell = elem("td", "actions-cell");
const actions = elem("div", "actions"); const actions = elem("div", "actions");
if (allowRestart) { 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) { if (build.artifactKey) {
actions.appendChild( actions.appendChild(
@@ -439,6 +447,7 @@ function initBuildsTable() {
} }
const tbody = document.getElementById("build-rows"); const tbody = document.getElementById("build-rows");
const allowRestart = table.dataset.allowRestart === "true"; const allowRestart = table.dataset.allowRestart === "true";
const restartAtOriginHead = table.dataset.restartAtOriginHead === "true";
async function refresh() { async function refresh() {
const builds = await fetchJson(table.dataset.api); const builds = await fetchJson(table.dataset.api);
@@ -449,7 +458,7 @@ function initBuildsTable() {
tbody.appendChild(elem("tr")).appendChild(cell); tbody.appendChild(elem("tr")).appendChild(cell);
return; return;
} }
builds.forEach((build) => tbody.appendChild(renderBuildRow(build, allowRestart))); builds.forEach((build) => tbody.appendChild(renderBuildRow(build, allowRestart, restartAtOriginHead)));
} }
startPolling(refresh, TABLE_POLL_MS); startPolling(refresh, TABLE_POLL_MS);
@@ -680,7 +689,8 @@ document.addEventListener("click", async (event) => {
button.disabled = true; button.disabled = true;
try { try {
if (action === "restart") { 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") { } else if (action === "cancel") {
await sendAction(`/api/builds/${encodeURIComponent(button.dataset.artifactKey)}/cancel`, "POST"); await sendAction(`/api/builds/${encodeURIComponent(button.dataset.artifactKey)}/cancel`, "POST");
} else if (action === "delete") { } else if (action === "delete") {
+4 -3
View File
@@ -7,7 +7,7 @@
<div th:replace="~{fragments :: nav(${view})}"></div> <div th:replace="~{fragments :: nav(${view})}"></div>
<div class="table-wrap"> <div class="table-wrap">
<table id="builds-table" <table id="builds-table"
th:attr="data-api=${apiPath},data-allow-restart=${allowRestart},data-empty-message=${emptyMessage}"> th:attr="data-api=${apiPath},data-allow-restart=${allowRestart},data-restart-at-origin-head=${restartAtOriginHead},data-empty-message=${emptyMessage}">
<thead> <thead>
<tr> <tr>
<th>Status</th> <th>Status</th>
@@ -65,8 +65,9 @@
<td class="actions-cell"> <td class="actions-cell">
<div class="actions"> <div class="actions">
<button th:if="${allowRestart}" class="action-button" type="button" data-action="restart" <button th:if="${allowRestart}" class="action-button" type="button" data-action="restart"
th:attr="data-branch=${row.name}" title="Restart build" th:attr="data-branch=${row.name},data-at-origin-head=${restartAtOriginHead}"
aria-label="Restart build"></button> th:title="${restartAtOriginHead} ? 'Build current head' : 'Restart build'"
th:aria-label="${restartAtOriginHead} ? 'Build current head' : 'Restart build'"></button>
<button th:if="${row.artifactKey != ''}" class="action-button delete-button" type="button" <button th:if="${row.artifactKey != ''}" class="action-button delete-button" type="button"
data-action="delete" th:attr="data-artifact-key=${row.artifactKey}" data-action="delete" th:attr="data-artifact-key=${row.artifactKey}"
title="Delete stored build" aria-label="Delete stored build">×</button> title="Delete stored build" aria-label="Delete stored build">×</button>
@@ -7,6 +7,19 @@
<div th:replace="~{fragments :: nav(${view})}"></div> <div th:replace="~{fragments :: nav(${view})}"></div>
<div class="panel release-notes"> <div class="panel release-notes">
<h2>v1.0.1 <span class="muted">— 2026-08-31</span></h2>
<ul>
<li>The restart button on the <em>Branches</em> 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.</li>
<li><em>Latest</em> and <em>History</em> are unchanged: a row there is a recorded run,
and restarting it means that commit.</li>
</ul>
<h2>v1.0.0 <span class="muted">— 2026-08-31</span></h2> <h2>v1.0.0 <span class="muted">— 2026-08-31</span></h2>
<ul> <ul>
<li>GitTally is now <strong>Werkator</strong>. The old name already belongs to another <li>GitTally is now <strong>Werkator</strong>. The old name already belongs to another
@@ -186,6 +186,58 @@ class BuildsApiControllerTest : FunSpec() {
verify { buildExecutor.startBuild("main", successResult.commit, build = "pitest") } 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") { test("restart of a never-built branch enqueues its origin head commit") {
val liveLogFile = tempDir.resolve("first-build.log") val liveLogFile = tempDir.resolve("first-build.log")
every { repository.latestFor("fresh") } returns null every { repository.latestFor("fresh") } returns null
@@ -177,6 +177,24 @@ class UiControllerTest : FunSpec() {
.andExpect(content().string(containsString("Permanent link"))) .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") { 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") val running = successResult.copy(status = BuildStatus.RUNNING, duration = null, artifactKey = "running-key")
every { repository.history() } returns listOf(running, successResult) every { repository.history() } returns listOf(running, successResult)