From 06eafc80105f6413b63eb81e44e43195dd3d81a1 Mon Sep 17 00:00:00 2001 From: mhoennig Date: Fri, 4 Sep 2026 18:12:47 +0200 Subject: [PATCH] Add trigger.afterSuccessOf, the follow-up build's predecessor (PR#23, point 1) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A build definition may name the build it follows. The key lives in the trigger block, so it is never inherited and refused when written flat; a definition with only a predecessor counts as triggered. An unknown predecessor refuses the start of the primary configuration — a deployment that silently never runs is the failure the flat-key check exists to prevent — while a branch layer that dropped the name only loses its follow-up with a warning, and an instance fragment is not checked alone since its predecessor may live in the project config. A cycle is refused wherever it appears. Co-Authored-By: Claude Fable 5.1 --- .../werkator/config/BuildDefinition.kt | 17 ++- .../hoennig/werkator/config/ConfigLoader.kt | 87 ++++++++++++-- .../werkator/config/ConfigLoaderTest.kt | 111 ++++++++++++++++++ 3 files changed, 205 insertions(+), 10 deletions(-) diff --git a/src/main/kotlin/de/hoennig/werkator/config/BuildDefinition.kt b/src/main/kotlin/de/hoennig/werkator/config/BuildDefinition.kt index 2baaa5d..973d01c 100644 --- a/src/main/kotlin/de/hoennig/werkator/config/BuildDefinition.kt +++ b/src/main/kotlin/de/hoennig/werkator/config/BuildDefinition.kt @@ -89,8 +89,9 @@ data class BuildDefinition( * When a build runs and for which branches — the `trigger` block of a build definition, * and the one part of it that is never inherited from `builds.default`. * - * A definition with neither [onPush] nor [atTimes] never triggers automatically; that is - * how `builds.default` is written when it is meant as a settings base only. + * A definition with neither [onPush] nor [atTimes] nor [afterSuccessOf] never triggers + * automatically; that is how `builds.default` is written when it is meant as a settings + * base only. */ data class TriggerConfig( /** Build every new commit of the selected branches. */ @@ -112,7 +113,19 @@ data class TriggerConfig( * empty applies no age filter. Combines with [branches] as an intersection. */ val activeWithin: String = "", + /** + * Name of another definition of this configuration — the *predecessor*: this build + * runs on the predecessor's branch at the predecessor's commit whenever a run of it + * ends with `SUCCESS`, whatever started that run (PR#23). Empty means none. Sits in + * the trigger block because it says *when* this build runs, so it is never inherited; + * and it is host-pinned, because a follow-up build is the host's way to hand + * real-world effects — deployment targets, credentials — to a green commit. + */ + val afterSuccessOf: String = "", ) { + /** True when this build follows another one, see [afterSuccessOf]. */ + fun isFollowUp(): Boolean = afterSuccessOf.isNotBlank() + /** True when [branch] matches the [branches] patterns (or none are configured) and none excludes it. */ fun selectsByName(branch: String): Boolean { val (excluding, including) = branches.partition { it.startsWith(EXCLUDE_PREFIX) } diff --git a/src/main/kotlin/de/hoennig/werkator/config/ConfigLoader.kt b/src/main/kotlin/de/hoennig/werkator/config/ConfigLoader.kt index 9d8b471..c6c4027 100644 --- a/src/main/kotlin/de/hoennig/werkator/config/ConfigLoader.kt +++ b/src/main/kotlin/de/hoennig/werkator/config/ConfigLoader.kt @@ -122,15 +122,33 @@ class ConfigLoader( // must never stop the server or hold up the branches that are fine checkVersion(branchLayer, "the committed .werkator.yml of this branch", BRANCH_HINT) checkTriggerBlocks(branchLayer, "the committed .werkator.yml of this branch", BRANCH_HINT) - return toConfig(deepMerge(loadRaw(workingDir), stripPinned(branchLayer))) + return toConfig(deepMerge(loadRaw(workingDir), stripPinned(branchLayer)), MissingPredecessor.WARN) } - private fun toConfig(raw: Map): WerkatorConfig { + /** + * What a follow-up whose predecessor no definition has means for this load, see + * [checkFollowUps]. + */ + private enum class MissingPredecessor { + /** The primary configuration: a deployment that could never fire refuses the start. */ + REFUSE, + + /** A branch layer on top: the branch renamed or dropped the build the host's trigger names, and only loses its follow-up. */ + WARN, + + /** A fragment checked on its own: the predecessor may well live in the project config it is merged with later. */ + SKIP, + } + + private fun toConfig( + raw: Map, + missingPredecessor: MissingPredecessor = MissingPredecessor.REFUSE, + ): WerkatorConfig { val config = if (raw.isEmpty()) { WerkatorConfig() } else { - yaml.convertValue(resolveBuildSections(dropNonDefinitionBuilds(raw)), WerkatorConfig::class.java) + yaml.convertValue(resolveBuildSections(dropNonDefinitionBuilds(raw), missingPredecessor), WerkatorConfig::class.java) } return defaultPublicBaseUrl(config) } @@ -225,12 +243,16 @@ class ConfigLoader( * an empty docker policy and run natively on the host, which is exactly the escape * the pinned keys exist to prevent. */ - private fun resolveBuildSections(raw: Map): Map { + private fun resolveBuildSections( + raw: Map, + missingPredecessor: MissingPredecessor, + ): Map { @Suppress("UNCHECKED_CAST") val definitions = raw["builds"] as? Map ?: emptyMap() if (definitions.isEmpty()) { return mergeBranchDefaults(raw) } + checkFollowUps(definitions, missingPredecessor) if (raw.containsKey("branches") && warnedSections.add(LEGACY_BRANCHES_WARNING)) { log.warn( "ignoring the branches section: this configuration defines builds, and a build definition " + @@ -252,13 +274,59 @@ class ConfigLoader( return } if (warnedSections.add(NO_TRIGGER_WARNING)) { - log.warn("no build defines onPush or atTimes; the watcher will never start a build on its own") + log.warn("no build defines onPush, atTimes, or afterSuccessOf; the watcher will never start a build on its own") } } private fun isTriggered(definition: Any?): Boolean { val trigger = (definition as? Map<*, *>)?.get("trigger") as? Map<*, *> ?: return false - return trigger["onPush"] == true || (trigger["atTimes"] as? List<*>)?.isNotEmpty() == true + return trigger["onPush"] == true || + (trigger["atTimes"] as? List<*>)?.isNotEmpty() == true || + predecessorOf(definition) != null + } + + private fun predecessorOf(definition: Any?): String? { + val trigger = (definition as? Map<*, *>)?.get("trigger") as? Map<*, *> ?: return null + return (trigger["afterSuccessOf"] as? String)?.takeIf { it.isNotBlank() } + } + + /** + * A follow-up build that could never fire must not exist, for the same reason a flat + * trigger key is refused: a deployment that silently never runs is worse than a + * configuration that refuses to load (PR#23). Refused are a predecessor no definition + * has, and a cycle of follow-ups (a build following itself included) — a cycle is a + * configuration error whoever wrote it, while a missing predecessor depends on what + * is being loaded ([MissingPredecessor]). + */ + private fun checkFollowUps( + definitions: Map, + missingPredecessor: MissingPredecessor, + ) { + val predecessors = definitions.mapValues { (_, definition) -> predecessorOf(definition) } + val effective = definitions.keys + BuildDefinition.DEFAULT + for ((name, predecessor) in predecessors) { + if (predecessor == null || predecessor in effective) continue + val message = "builds.$name follows '$predecessor' (trigger.afterSuccessOf), but no build of that name is defined" + when (missingPredecessor) { + MissingPredecessor.REFUSE -> throw ConfigFormatException("$message. Name an existing build, or remove the follow-up.") + MissingPredecessor.WARN -> log.warn("$message on this branch; the follow-up will not run for it") + MissingPredecessor.SKIP -> {} + } + } + for (start in predecessors.keys) { + val path = mutableListOf(start) + var current = predecessors[start] + while (current != null && current !in path) { + path += current + current = predecessors[current] + } + if (current == start) { + throw ConfigFormatException( + "builds.$start follows itself through trigger.afterSuccessOf (${path.joinToString(" -> ")} -> $start); " + + "a follow-up build cannot wait for its own success.", + ) + } + } } /** @@ -426,7 +494,10 @@ class ConfigLoader( checkVersion(raw, fragment.toString(), ROLLBACK_HINT) checkTriggerBlocks(raw, fragment.toString(), ROLLBACK_HINT) try { - strictYaml.convertValue(resolveBuildSections(dropNonDefinitionBuilds(raw)), WerkatorConfig::class.java) + strictYaml.convertValue( + resolveBuildSections(dropNonDefinitionBuilds(raw), MissingPredecessor.SKIP), + WerkatorConfig::class.java, + ) } catch (e: IllegalArgumentException) { throw IllegalArgumentException( "instance fragment $fragment does not match the configuration schema: ${e.message}", @@ -610,7 +681,7 @@ class ConfigLoader( private val TRIGGER_KEYS = setOf("trigger") /** The keys that moved into [TRIGGER_KEYS]; still writing them flat is refused, not ignored. */ - private val FLAT_TRIGGER_KEYS = setOf("onPush", "atTimes", "branches", "activeWithin") + private val FLAT_TRIGGER_KEYS = setOf("onPush", "atTimes", "branches", "activeWithin", "afterSuccessOf") /** * Top-level sections owned by the instance once a home config exists (ADR 0009): diff --git a/src/test/kotlin/de/hoennig/werkator/config/ConfigLoaderTest.kt b/src/test/kotlin/de/hoennig/werkator/config/ConfigLoaderTest.kt index 46b29ed..b8ab53b 100644 --- a/src/test/kotlin/de/hoennig/werkator/config/ConfigLoaderTest.kt +++ b/src/test/kotlin/de/hoennig/werkator/config/ConfigLoaderTest.kt @@ -739,6 +739,117 @@ class ConfigLoaderTest : FunSpec() { .shouldBeTrue() } + test("afterSuccessOf must name an existing definition and must not form a cycle") { + val dir = Files.createTempDirectory("werkator-test") + val project = dir.resolve(".werkator.yml").toFile() + project.writeText( + """ + builds: + default: + trigger: + onPush: true + deploy: + trigger: + afterSuccessOf: default + branches: ["main"] + buildCommand: scripts/deploy.sh + """.trimIndent(), + ) + // a follow-up with nothing but its predecessor is a triggered build, and it binds + val deploy = loader.load(dir).buildDefinitions.getValue("deploy") + deploy.trigger.afterSuccessOf shouldBe "default" + deploy.trigger.isFollowUp().shouldBeTrue() + deploy.trigger.onPush.shouldBeFalse() + + project.writeText( + """ + builds: + deploy: + trigger: + afterSuccessOf: frontend + """.trimIndent(), + ) + shouldThrow { loader.load(dir) }.message.let { + it.shouldContain("builds.deploy") + it.shouldContain("frontend") + } + + project.writeText( + """ + builds: + a: + trigger: + afterSuccessOf: b + b: + trigger: + afterSuccessOf: a + """.trimIndent(), + ) + shouldThrow { loader.load(dir) }.message.shouldContain("a -> b -> a") + + project.writeText( + """ + builds: + a: + trigger: + afterSuccessOf: a + """.trimIndent(), + ) + shouldThrow { loader.load(dir) }.message.shouldContain("builds.a follows itself") + } + + test("a branch whose layer lacks the predecessor loses only its follow-up") { + val dir = Files.createTempDirectory("werkator-test") + dir.resolve(".werkator.yml").toFile().writeText( + """ + builds: + frontend: + trigger: + onPush: true + deploy: + trigger: + afterSuccessOf: frontend + """.trimIndent(), + ) + // the branch renamed the predecessor: a warning, not a failed load — the branch's + // own builds must keep running, its follow-up simply never fires for it + val config = + loader.loadWithBranchLayer( + dir, + """ + builds: + frontend: null + ui: + trigger: + onPush: true + """.trimIndent(), + ) + config.buildDefinitions + .getValue("ui") + .trigger.onPush + .shouldBeTrue() + config.buildDefinitions + .getValue("deploy") + .trigger.afterSuccessOf shouldBe "frontend" + } + + test("afterSuccessOf written flat is refused like every trigger key") { + val dir = Files.createTempDirectory("werkator-test") + dir.resolve(".werkator.yml").toFile().writeText( + """ + builds: + default: + trigger: + onPush: true + deploy: + afterSuccessOf: default + """.trimIndent(), + ) + val thrown = shouldThrow { loader.load(dir) } + thrown.message.shouldContain("builds.deploy: afterSuccessOf") + thrown.message.shouldContain("trigger:") + } + test("builds.default is the base of every other build, but never its trigger") { val dir = Files.createTempDirectory("werkator-test") dir.resolve(".werkator.yml").toFile().writeText(