From fd326f03fc027e97cdbe4a52de5b48bab4afb13b Mon Sep 17 00:00:00 2001 From: Michael Hoennig Date: Wed, 8 Jul 2026 15:50:45 +0200 Subject: [PATCH] made the pull-request gate configurable: added `watcher.pullRequestGate` (default true); set false for plain-git origins without pull-request refs, so `requirePullRequest` gates are ignored and gated branches build on new commits; added the PR-doc Co-Authored-By: Claude Fable 5 --- .claude/skills/architecture/SKILL.md | 2 +- docs/configuration.md | 7 + ...PR#000-build-only-pull-request-branches.md | 141 ++++++++++++++++++ .../hoennig/gittally/commands/InitCommand.kt | 3 + .../hoennig/gittally/config/GitTallyConfig.kt | 7 + .../de/hoennig/gittally/watcher/Watcher.kt | 9 +- .../hoennig/gittally/watcher/WatcherTest.kt | 18 +++ 7 files changed, 184 insertions(+), 3 deletions(-) create mode 100644 docs/prs/2026-07-08-PR#000-build-only-pull-request-branches.md diff --git a/.claude/skills/architecture/SKILL.md b/.claude/skills/architecture/SKILL.md index 1024a32..04a383f 100644 --- a/.claude/skills/architecture/SKILL.md +++ b/.claude/skills/architecture/SKILL.md @@ -60,7 +60,7 @@ The runtime is selected per branch behind the `BuildRunner` interface: `Dispatch ## Watcher -`Watcher` replaces the legacy blocking main loop with a non-blocking fixed-delay poll cycle: fetch origin, enqueue due branches (changed local, recent new origin, due auto-build slots) via `BuildExecutor`, then prune results, artifacts, and stale worktrees. Branches with `branches..requirePullRequest` are enqueued only while their head commit matches a pull-request head, detected without an API token by listing `refs/pull/*/head` via `git ls-remote` (lazily, at most once per poll cycle); manual `build` commands bypass this gate. Nothing is scheduled until `Watcher.start()` is called explicitly (server/watch mode) — CLI commands and tests never start the loop. "Already built" is tracked via the result repository, not by moving local branch refs. Auto-build slot state lives in `.git/gittally/auto-builds.json`; watcher health is exposed via `Watcher.state()`. +`Watcher` replaces the legacy blocking main loop with a non-blocking fixed-delay poll cycle: fetch origin, enqueue due branches (changed local, recent new origin, due auto-build slots) via `BuildExecutor`, then prune results, artifacts, and stale worktrees. Branches with `branches..requirePullRequest` are enqueued only while their head commit matches a pull-request head, detected without an API token by listing `refs/pull/*/head` via `git ls-remote` (lazily, at most once per poll cycle); manual `build` commands bypass this gate, and `watcher.pullRequestGate: false` disables it globally for plain-git origins without pull-request refs. Nothing is scheduled until `Watcher.start()` is called explicitly (server/watch mode) — CLI commands and tests never start the loop. "Already built" is tracked via the result repository, not by moving local branch refs. Auto-build slot state lives in `.git/gittally/auto-builds.json`; watcher health is exposed via `Watcher.state()`. ## System Metrics diff --git a/docs/configuration.md b/docs/configuration.md index 567a97e..65680b7 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -64,6 +64,10 @@ watcher: pollInterval: 10s # max commit age for new origin branches to be pulled automatically newBranchMaxAge: 5d + # Honor the branches..requirePullRequest gates (see notes below). + # Set false for a plain git origin without pull-request refs (no Gitea/GitHub); + # gated branches then build on new commits like any other branch. + pullRequestGate: true # Per-branch build configuration. # Use "default" as the fallback for all branches not listed explicitly. @@ -142,6 +146,9 @@ branches: Without the `main` override, direct pushes and merges to `main` would never build — merge commits do not match any pull-request head. +A plain git origin (no Gitea/GitHub) serves no `refs/pull/*/head` at all, so gated branches would never build there. +For such origins, disable all gates globally with `watcher.pullRequestGate: false` — typically in the machine-specific `.git/gittally/.gittally.yml`, so the committed configuration keeps the gates for forge-backed environments. + ### Notes on `branches..docker` With `docker.enabled`, GitTally shells out to the `docker` CLI; the `docker` command must be on the `PATH`. diff --git a/docs/prs/2026-07-08-PR#000-build-only-pull-request-branches.md b/docs/prs/2026-07-08-PR#000-build-only-pull-request-branches.md new file mode 100644 index 0000000..b3ee73b --- /dev/null +++ b/docs/prs/2026-07-08-PR#000-build-only-pull-request-branches.md @@ -0,0 +1,141 @@ +# Build Only Branches with a Pull Request + +> **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 watcher builds every changed or recently created origin branch. +On repositories with many work-in-progress branches this wastes build capacity on branches nobody asked to be verified. +The desired policy is: only branches with a pull request get watcher builds. +Detecting pull requests must not require a Gitea API token, so it must work for anonymous read access. + +## Non-Goals + +- Distinguishing open from closed pull requests (requires the Gitea API, see Open Questions). +- Gating manual `gittally build ` invocations; an explicit command always builds. +- Building the merge preview commit (`refs/pull//merge`) instead of the branch head. + +## The Scenarios + +### Feature: watcher builds only pull-request branches + +#### Background + +- Gitea (like GitHub) exposes each pull request to plain git clients as a ref `refs/pull//head` pointing at the PR's head commit. +- A branch "has a pull request" when its origin head commit equals one of those pull-request head commits. +- The gate is configured per branch via `branches..requirePullRequest` (default `false`); setting it under `branches.default` applies it to all branches. + +#### Scenario#000.01: A gated branch with a pull request is built! + +So that pull-request branches get their commit status verified as before. + +- **Given** `requirePullRequest: true` applies to branch `feature/pr` + - **and** `feature/pr` has new commits on origin + - **and** a ref `refs/pull//head` on origin points at the head commit of `feature/pr` +- **When** the watcher polls +- **Then** a build of `feature/pr` at that head commit is enqueued + +##### Verified by + +- [WatcherTest: "a branch requiring a pull request is only built when its head matches a pull-request head"](../../src/test/kotlin/de/hoennig/gittally/watcher/WatcherTest.kt) + +#### Scenario#000.02: A gated branch without a pull request is not built! + +So that work-in-progress branches do not consume build capacity. + +- **Given** `requirePullRequest: true` applies to branch `feature/no-pr` + - **and** `feature/no-pr` has new commits on origin + - **and** no `refs/pull//head` points at its head commit +- **When** the watcher polls +- **Then** no build is enqueued + - **and** the skip is logged + +##### Verified by + +- [WatcherTest: "a branch requiring a pull request is only built when its head matches a pull-request head"](../../src/test/kotlin/de/hoennig/gittally/watcher/WatcherTest.kt) + +#### Scenario#000.03: Pull-request detection works without an API token! + +So that GitTally needs no Gitea credentials for this feature. + +- **Given** a git remote that serves `refs/pull//head` refs +- **When** pull-request heads are queried +- **Then** they are read via `git ls-remote origin "refs/pull/*/head"` using the existing git authentication (or none) + +##### Verified by + +- [GitServiceTest: "pullRequestHeads returns the head commits of the remote's pull-request refs"](../../src/test/kotlin/de/hoennig/gittally/git/GitServiceTest.kt) + +#### Scenario#000.04: Ungated setups make no extra remote calls! + +So that existing installations see no new network traffic. + +- **Given** no due branch has `requirePullRequest` set +- **When** the watcher polls +- **Then** `refs/pull/*/head` is not queried at all + +##### Verified by + +- [WatcherTest: "pull-request refs are not queried when no due branch requires a pull request"](../../src/test/kotlin/de/hoennig/gittally/watcher/WatcherTest.kt) + +#### Scenario#000.05: A branch entry overrides the default! + +So that permanent branches like `main` keep building after merges, whose merge commits never match a pull-request head. + +- **Given** `branches.default.requirePullRequest: true` + - **and** `branches.main.requirePullRequest: false` +- **When** `main` has new commits and the watcher polls +- **Then** `main` is built without any pull-request check + +##### Verified by + +- [WatcherTest: "a branch entry overrides requirePullRequest from the default entry"](../../src/test/kotlin/de/hoennig/gittally/watcher/WatcherTest.kt) + +#### Scenario#000.06: Auto builds respect the gate! + +So that scheduled rebuilds follow the same policy as push-triggered builds. + +- **Given** a branch with `autoBuild.enabled` and `requirePullRequest: true` + - **and** its head commit matches no pull-request head +- **When** an auto-build slot becomes due +- **Then** no build is enqueued + - **and** the slot stays untriggered, so it is retried on later poll cycles + +##### Verified by + +- [WatcherTest: "an auto build requiring a pull request is skipped and its slot stays untriggered"](../../src/test/kotlin/de/hoennig/gittally/watcher/WatcherTest.kt) + +#### Scenario#000.07: The gate can be disabled globally for plain git origins! + +So that the same committed configuration works on environments whose origin is plain git without pull-request refs (no Gitea/GitHub), e.g. a local test server. + +- **Given** `requirePullRequest: true` applies to branch `feature/no-pr` + - **and** `watcher.pullRequestGate: false` is set (typically in the machine-specific `.git/gittally/.gittally.yml`) +- **When** the watcher polls and `feature/no-pr` has new commits +- **Then** a build is enqueued like for an ungated branch + - **and** `refs/pull/*/head` is not queried at all + +##### Verified by + +- [WatcherTest: "a disabled pull-request gate builds gated branches on plain-git origins without querying pull-request refs"](../../src/test/kotlin/de/hoennig/gittally/watcher/WatcherTest.kt) + +## The Solution + +`GitService.pullRequestHeads()` lists `refs/pull/*/head` on origin via `git ls-remote` and returns the set of head commit SHAs. +The refs are read remotely instead of being fetched, because the default fetch refspec only covers `refs/heads/*` and mirroring pull refs locally would bloat every clone. +The watcher's `startBuildIfDue` gate compares the branch's origin head commit against that set when the resolved branch config has `requirePullRequest`. +The set is computed lazily and shared across one poll cycle, so `ls-remote` runs at most once per cycle and only when a gated branch is otherwise due. +The new config key lives in `BranchConfig` with default `false`, so the feature is opt-in and existing configurations behave unchanged. +`watcher.pullRequestGate` (default `true`) turns all `requirePullRequest` gates off globally, because a plain git origin serves no `refs/pull/*/head` and gated branches would otherwise never build there; being a property of the environment, it is typically set in the machine-specific `.git/gittally/.gittally.yml`. +Config reference, `init` templates, and `CLAUDE.md` were updated in sync, including the recommended setup (`default: true`, `main: false`). + +## Open Questions + +- Closed pull requests whose head ref still equals the branch head also pass the gate, because matching is by commit id only; distinguishing open from closed would require the Gitea API. Currently implemented: any matching `refs/pull//head` counts. +- A due auto-build slot of a gated branch without a pull request is re-checked every poll cycle for the rest of the day instead of being marked as skipped. Currently implemented: the slot stays untriggered, mirroring the behavior while a build is still running. + +## Follow-up PRs + +- Optionally use the Gitea API (when a token is configured) to restrict the gate to *open* pull requests. diff --git a/src/main/kotlin/de/hoennig/gittally/commands/InitCommand.kt b/src/main/kotlin/de/hoennig/gittally/commands/InitCommand.kt index 46e9102..2c9aa08 100644 --- a/src/main/kotlin/de/hoennig/gittally/commands/InitCommand.kt +++ b/src/main/kotlin/de/hoennig/gittally/commands/InitCommand.kt @@ -152,6 +152,9 @@ class InitCommand( pollInterval: 10s # max commit age for new origin branches to be pulled automatically newBranchMaxAge: 5d + # honor branches..requirePullRequest; set false for a plain git origin + # without pull-request refs (refs/pull/*/head) — gated branches then build on new commits + pullRequestGate: true # Per-branch build configuration. # Use "default" as the fallback for all branches not listed explicitly. diff --git a/src/main/kotlin/de/hoennig/gittally/config/GitTallyConfig.kt b/src/main/kotlin/de/hoennig/gittally/config/GitTallyConfig.kt index b43cc1c..bb9bcb4 100644 --- a/src/main/kotlin/de/hoennig/gittally/config/GitTallyConfig.kt +++ b/src/main/kotlin/de/hoennig/gittally/config/GitTallyConfig.kt @@ -49,6 +49,13 @@ data class WatcherConfig( /** Delay between poll cycles, e.g. `10s` or `1m`. */ val pollInterval: String = "10s", val newBranchMaxAge: String = "5d", + /** + * Honor the `branches..requirePullRequest` gates. Set false for a plain git + * origin without pull-request refs (no Gitea/GitHub) — gated branches then build + * on new commits like any other branch. Typically overridden per machine in + * `.git/gittally/.gittally.yml` when the committed config enables the gates. + */ + val pullRequestGate: Boolean = true, ) data class BranchConfig( diff --git a/src/main/kotlin/de/hoennig/gittally/watcher/Watcher.kt b/src/main/kotlin/de/hoennig/gittally/watcher/Watcher.kt index c4dcd3e..80ce62a 100644 --- a/src/main/kotlin/de/hoennig/gittally/watcher/Watcher.kt +++ b/src/main/kotlin/de/hoennig/gittally/watcher/Watcher.kt @@ -175,7 +175,9 @@ class Watcher( * repository, not by resetting the local ref like legacy. A new commit for a * branch that is still pending/running waits for a later cycle (queue-behind). * With `requirePullRequest`, the branch head must match a pull-request head on - * origin (`refs/pull//head`); manual `build` commands bypass this gate. + * origin (`refs/pull//head`); manual `build` commands bypass this gate, and + * `watcher.pullRequestGate: false` disables it globally for plain-git origins + * without pull-request refs. */ private fun startBuildIfDue( branch: String, @@ -192,7 +194,10 @@ class Watcher( if (!allowSameCommit && latest?.commit == commit) { return false } - if (branchConfig(config, branch).requirePullRequest && commit !in pullRequestHeads.value) { + if (config.watcher.pullRequestGate && + branchConfig(config, branch).requirePullRequest && + commit !in pullRequestHeads.value + ) { log.info("not enqueueing branch {}: no pull request has head commit {}", branch, commit) return false } diff --git a/src/test/kotlin/de/hoennig/gittally/watcher/WatcherTest.kt b/src/test/kotlin/de/hoennig/gittally/watcher/WatcherTest.kt index 2cce0be..1f5b55b 100644 --- a/src/test/kotlin/de/hoennig/gittally/watcher/WatcherTest.kt +++ b/src/test/kotlin/de/hoennig/gittally/watcher/WatcherTest.kt @@ -256,6 +256,24 @@ class WatcherTest : FunSpec() { verify(exactly = 0) { harness.gitService.pullRequestHeads(any()) } } + test("a disabled pull-request gate builds gated branches on plain-git origins without querying pull-request refs") { + val harness = + Harness( + GitTallyConfig( + watcher = WatcherConfig(pullRequestGate = false), + branches = mapOf("default" to BranchConfig(requirePullRequest = true)), + ), + ) + every { harness.gitService.originBranches(any()) } returns listOf("feature/no-pr") + every { harness.gitService.newOriginBranches(any(), any()) } returns listOf("feature/no-pr") + every { harness.gitService.originHeadCommit("feature/no-pr", any()) } returns "commit-solo" + + harness.watcher.poll(harness.workingDir) + + harness.startedBuilds shouldContainExactly listOf("feature/no-pr" to "commit-solo") + verify(exactly = 0) { harness.gitService.pullRequestHeads(any()) } + } + test("a branch entry overrides requirePullRequest from the default entry") { val harness = Harness(