Pull-request target builds: `trigger.onPullRequestTarget` runs every open, non-draft pull request once per head commit, on a fresh checkout of its base. The definition is host-only: a branch can neither add, change nor switch off such a build, so the pull request under review cannot fake the status or reach the secrets. The status goes to the head SHA; the build gets `WERKATOR_PR_*` and `WERKATOR_GITEA_*`. `export:forgejo` maps it to `pull_request_target`. Motivation: an AI review gate (werkrevisor, PR-Agent based) as a merge condition. Details: the PR-doc under `docs/prs/` (still with the placeholder PR#33, renamed in a follow-up commit). 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: mhoennig <michael@hoennig.de> Reviewed-on: #117
201 lines
14 KiB
Markdown
201 lines
14 KiB
Markdown
# PR#117: Pull-Request Target Builds
|
|
|
|
> **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
|
|
|
|
An AI code review (the separate project werkrevisor) is to run for every pull request of a repository.
|
|
It needs secrets — a Gitea bot token and an API key of the model provider — and its verdict is a merge gate.
|
|
|
|
Werkator had no build kind for that.
|
|
Every build definition comes with the branch: the branch's committed `.werkator.yml` may change `buildCommand` and `werkdock.env`/`docker.env` of any definition.
|
|
A pull request could replace the review command by `exit 0` and fake the gate, or print the secrets.
|
|
|
|
Forgejo and GitHub solve this with `on: pull_request_target`: the workflow comes from the base side, never from the pull request.
|
|
|
|
## Non-Goals
|
|
|
|
- No new build when only the base branch moves; protect the base branch with Gitea's "Block merge if pull request is outdated" instead.
|
|
- No reaction to review approvals or comments.
|
|
- No AI-specific logic: the feature is a generic pull-request target build.
|
|
- No pull requests from forks, as for the pull-request links.
|
|
|
|
## The Scenarios
|
|
|
|
### Feature: a build for every pull request, defined by the host alone
|
|
|
|
#### Background
|
|
|
|
- A *pull-request target build* is a build definition with `trigger.onPullRequestTarget: true`.
|
|
- The *primary configuration* is the instance defaults, the checkout's `.werkator.yml`, the applied fragment and the machine config; the *branch layer* is the `.werkator.yml` committed on a branch.
|
|
- A *draft* is a pull request Gitea marks as one, by its `draft` flag or by the `WIP:`/`[WIP]` title prefix.
|
|
|
|
#### Scenario#117.01: every open pull request is built once per head commit
|
|
|
|
So that every change gets exactly one verdict, and a new head gets a new one.
|
|
|
|
- **Given** the primary configuration defines the pull-request target build `review`
|
|
- **and** pull request #7 from `feature/x` into `main` is open, with head `h1`
|
|
- **When** the watcher polls
|
|
- **Then** it enqueues `review` for #7 at head `h1`
|
|
- **and** it does not enqueue it again in a later poll, whatever the outcome of that build
|
|
- **When** a new head `h2` is pushed to `feature/x`
|
|
- **Then** `review` is enqueued for #7 at `h2`
|
|
- **And** a base the forge reports ahead of this cycle's fetch waits for the next cycle, and a failed read of the pull requests decides nothing
|
|
|
|
##### Verified by
|
|
|
|
- [PullRequestTargetsUnitTest](../../src/test/kotlin/de/hoennig/werkator/watcher/PullRequestTargetsUnitTest.kt) `an open pull request whose head was never built is due for every pull-request target definition`, `a head built before is not due again, whatever its outcome, nor while it is queued`, `a new head of a pull request built before is due again`, `a head built by another definition, or by a build without a pull request, does not count`, `a base the forge reports ahead of this cycle's fetch waits for the next cycle`
|
|
- [WatcherPullRequestTargetUnitTest](../../src/test/kotlin/de/hoennig/werkator/watcher/WatcherPullRequestTargetUnitTest.kt) `a poll enqueues the due pull-request target build once per head`
|
|
- [PullRequestIndexUnitTest](../../src/test/kotlin/de/hoennig/werkator/gitea/PullRequestIndexUnitTest.kt) `a refresh answers with this read alone, so a failed read never hands out an older list`
|
|
- [PullRequestTargetBuildIntegrationTest](../../src/test/kotlin/de/hoennig/werkator/build/PullRequestTargetBuildIntegrationTest.kt) `a head already queued or running is not queued twice`
|
|
|
|
#### Scenario#117.02: drafts and pull requests from forks are not built
|
|
|
|
So that a work in progress spends no secrets, and a fork's branch is never mistaken for one of the repository.
|
|
|
|
- **Given** an open pull request that is a draft
|
|
- **When** the watcher polls
|
|
- **Then** no build is enqueued for it
|
|
- **When** the pull request is marked ready
|
|
- **Then** its current head is due
|
|
- **And** a pull request from a fork is never due
|
|
|
|
##### Verified by
|
|
|
|
- [PullRequestTargetsUnitTest](../../src/test/kotlin/de/hoennig/werkator/watcher/PullRequestTargetsUnitTest.kt) `a draft is never built, and its head is due as soon as it is ready`, `a pull request from a fork is left out`
|
|
- [GiteaClientIntegrationTest](../../src/test/kotlin/de/hoennig/werkator/gitea/GiteaClientIntegrationTest.kt) `reads head and base of every open pull request, forks and drafts marked`
|
|
|
|
#### Scenario#117.03: the build runs the base and reports on the head
|
|
|
|
So that no file of the pull request is executed with the secrets, while the verdict belongs to the change.
|
|
|
|
- **Given** `review` is enqueued for #7 with head `h1` on base `b1`
|
|
- **When** it runs
|
|
- **Then** a fresh workspace has `b1` checked out, without any file of the pull request and without anything an earlier build left
|
|
- **and** the build sees `WERKATOR_PR_NUMBER`, `WERKATOR_PR_HEAD_SHA`, `WERKATOR_PR_HEAD_REF`, `WERKATOR_PR_BASE_REF`, `WERKATOR_PR_BASE_SHA`, `WERKATOR_GITEA_BASE_URL`, `WERKATOR_GITEA_OWNER` and `WERKATOR_GITEA_REPO`
|
|
- **and** every Gitea status of the build is published on `h1` under its `statusContext`, none on `b1`
|
|
- **and** the result is recorded as `feature/x@review` at commit `b1`, with the pull request beside it
|
|
- **And** the definition started without a pull request, e.g. by `werkator build`, fails without running anything
|
|
|
|
##### Verified by
|
|
|
|
- [PullRequestTargetBuildIntegrationTest](../../src/test/kotlin/de/hoennig/werkator/build/PullRequestTargetBuildIntegrationTest.kt) `runs the base, reports on the head, and tells the build which pull request it judges`, `a pull-request target definition never runs without its pull request`
|
|
- [GitWorktreeWorkspacesIntegrationTest](../../src/test/kotlin/de/hoennig/werkator/build/GitWorktreeWorkspacesIntegrationTest.kt) `a pull-request target build gets a fresh workspace apart from the branch's, without anything left behind`
|
|
- [BuildSettingsSourceUnitTest](../../src/test/kotlin/de/hoennig/werkator/build/BuildSettingsSourceUnitTest.kt) `a pull-request target definition without a pull request is refused, and pull-request data on any other`
|
|
|
|
#### Scenario#117.04: a branch cannot change, add or switch off a pull-request target build
|
|
|
|
So that the pull request under review can neither fake its gate nor reach the secrets.
|
|
|
|
- **Given** the host defines `review` with `buildCommand: tools/review` and a secret in `werkdock.env`
|
|
- **When** a branch's `.werkator.yml` redefines `review` with `buildCommand: exit 0` and its own `werkdock.env`
|
|
- **or** rewrites `builds.default`, from which `review` inherits
|
|
- **or** sets `review: null`
|
|
- **or** defines a pull-request target build of its own, even with a quoted `"true"`
|
|
- **Then** `review` runs exactly as the host defines it, and only the host's pull-request target builds exist
|
|
|
|
##### Verified by
|
|
|
|
- [PullRequestTargetConfigUnitTest](../../src/test/kotlin/de/hoennig/werkator/config/PullRequestTargetConfigUnitTest.kt) `a branch that redefines the pull-request target build changes nothing about it`, `a branch cannot switch the host's pull-request target build off`, `a branch cannot add a pull-request target build, however it writes the key`
|
|
- [BuildSettingsSourceUnitTest](../../src/test/kotlin/de/hoennig/werkator/build/BuildSettingsSourceUnitTest.kt) `a pull-request target build takes its settings from the primary configuration, never through the branch`
|
|
- [WatcherPullRequestTargetUnitTest](../../src/test/kotlin/de/hoennig/werkator/watcher/WatcherPullRequestTargetUnitTest.kt) `the definitions are the primary configuration's, never a branch's`
|
|
|
|
The three `PullRequestTargetConfigUnitTest` tests fail with the branch-layer pinning switched off; the first one fails already when only the re-resolution against a branch-rewritten `builds.default` is switched off.
|
|
|
|
#### Scenario#117.05: a pull-request target build stands alone
|
|
|
|
So that it never runs for a branch at its head, and nothing runs after it on the base under the head's name.
|
|
|
|
- **Given** a definition with `trigger.onPullRequestTarget: true`
|
|
- **When** it also says `onPush`, `atTimes`, `branches`, `activeWithin`, `afterSuccessOf` or `requirePullRequest`, in one file or spread over several
|
|
- **or** it is `builds.default`
|
|
- **or** another definition follows it
|
|
- **or** the key is written outside the trigger block
|
|
- **Then** the start is refused with a message naming the definition
|
|
- **And** a green pull-request target build triggers no follow-up
|
|
|
|
##### Verified by
|
|
|
|
- [PullRequestTargetConfigUnitTest](../../src/test/kotlin/de/hoennig/werkator/config/PullRequestTargetConfigUnitTest.kt) `a pull-request target build stands alone; every combination is refused at start, naming the definition`, `keys spread over the project and the machine config are refused together`, `builds.default cannot be a pull-request target build, nor can a build follow one`, `onPullRequestTarget written outside the trigger block is refused like every trigger key`
|
|
- [FollowUpTriggerUnitTest](../../src/test/kotlin/de/hoennig/werkator/watcher/FollowUpTriggerUnitTest.kt) `a green pull-request target build triggers nothing: its branch is the head, its commit the base`
|
|
|
|
#### Scenario#117.06: without a forge connection the build never runs, and the watcher says so once
|
|
|
|
So that a missing token is visible instead of a gate that silently never turns green.
|
|
|
|
- **Given** a pull-request target build, but no `git.token` or no complete `gitea` section
|
|
- **When** the watcher polls, again and again
|
|
- **Then** it enqueues nothing and logs one warning naming the builds
|
|
- **and** logs it once more only after the connection was there in between
|
|
|
|
##### Verified by
|
|
|
|
- [PullRequestTargetsUnitTest](../../src/test/kotlin/de/hoennig/werkator/watcher/PullRequestTargetsUnitTest.kt) `without a token or a gitea section there is no forge, and a failed read decides nothing`
|
|
- [WatcherPullRequestTargetUnitTest](../../src/test/kotlin/de/hoennig/werkator/watcher/WatcherPullRequestTargetUnitTest.kt) `without a forge the watcher says once that the builds never run, and again after it came back`
|
|
|
|
#### Scenario#117.07: a re-run builds the recorded pull request
|
|
|
|
So that a restart repeats the verdict it is about, and never asks the branch.
|
|
|
|
- **Given** a pull-request target build of #7 at head `h1` was interrupted, or failed
|
|
- **When** the startup recovery, a UI restart (also with `atOriginHead`) or `werkator retry` runs it again
|
|
- **Then** it runs for #7 at head `h1` on its recorded base, even when `feature/x` has moved on
|
|
- **And** a results file from before this change still loads
|
|
|
|
##### Verified by
|
|
|
|
- [WatcherPullRequestTargetUnitTest](../../src/test/kotlin/de/hoennig/werkator/watcher/WatcherPullRequestTargetUnitTest.kt) `the startup recovery re-runs an interrupted pull-request target build with the pull request it recorded`
|
|
- [BuildsApiControllerRestTest](../../src/test/kotlin/de/hoennig/werkator/server/BuildsApiControllerRestTest.kt) `restart of a pull-request target build re-runs the pull request it recorded, even at origin head`
|
|
- [FileBuildResultRepositoryUnitTest](../../src/test/kotlin/de/hoennig/werkator/build/FileBuildResultRepositoryUnitTest.kt) `round-trips the pull request of a pull-request target build, and loads results without one`
|
|
|
|
#### Scenario#117.08: the build converts to a Forgejo pull_request_target workflow
|
|
|
|
So that the key keeps a workflow counterpart (ADR 0010).
|
|
|
|
- **Given** a committed `review` with `trigger.onPullRequestTarget: true`
|
|
- **When** `export:forgejo` converts it
|
|
- **Then** the workflow runs `on: pull_request_target` for `opened`, `reopened`, `synchronize` and `ready_for_review`, without `workflow_dispatch`
|
|
- **and** its job skips drafts, checks out `github.event.pull_request.base.sha` and maps the `WERKATOR_PR_*`/`WERKATOR_GITEA_*` variables to the event and repository context
|
|
- **and** the differences are listed as not converted: the base commit, the re-run on `reopened`, the secrets
|
|
|
|
##### Verified by
|
|
|
|
- [ForgejoWorkflowExporterUnitTest](../../src/test/kotlin/de/hoennig/werkator/forgejo/ForgejoWorkflowExporterUnitTest.kt) `a pull-request target build becomes a pull_request_target workflow on the base, skipping drafts`
|
|
|
|
## The Solution
|
|
|
|
`TriggerConfig.onPullRequestTarget` is the new trigger key.
|
|
|
|
The pinning stays in `ConfigLoader`.
|
|
`stripPinned` drops every pull-request target definition from the branch layer: the host's, whole, and the branch's own, with a warning.
|
|
Stripping alone is not enough, because the merged definition still inherits from a `builds.default` the branch may have rewritten.
|
|
So `enforcePinnedPullRequestTargets` replaces these definitions in the branch-layered result with the primary configuration's bound ones.
|
|
`checkPullRequestTargets` refuses the combinations at start.
|
|
|
|
The executor resolves the settings of such a build from the primary configuration alone (`BuildSettingsSource`) and refuses the definition without its pull request, so a manual build cannot run the host's command with the secrets on the head's files.
|
|
The build gets a fresh worktree under `.git/werkator/pull-request-worktrees/`, because the head branch's reused worktree holds ignored files an earlier build of the head wrote, compiled build logic included.
|
|
`RunningBuild.statusCommit` is the head: every Gitea status and the duplicate check of the queue use it, while `commit` stays the checked-out base.
|
|
`BuildResult.pullRequest` records the pull request, so re-runs never ask the branch.
|
|
|
|
`GiteaClient.readOpenPullRequests` returns every open pull request with head, base, draft and fork marks; the pull-request links derive from that list as before.
|
|
`PullRequestIndex.refresh` returns this cycle's answer, and the watcher reads it before the enqueue instead of at the end of the cycle.
|
|
`PullRequestTargets` decides what is due; it waits while the forge's base commit differs from the fetched origin head of the base, since that commit may not be fetched yet.
|
|
|
|
`ForgejoWorkflowExporter` maps the key to `pull_request_target` (ADR 0010).
|
|
|
|
## Open Questions
|
|
|
|
- A head is built once per definition, whatever the outcome; a failed review is repeated by a restart, not by the watcher.
|
|
- A pull request reopened with an unchanged head is not built again; Forgejo's workflow runs on `reopened`.
|
|
- Gitea's `draft` field is read where Gitea sends it; older versions are judged by the title prefix `WIP:`/`[WIP]` alone, without the instance's own `WORK_IN_PROGRESS_PREFIXES`.
|
|
- `requirePullRequest` inherited from `builds.default` is ignored for a pull-request target build; only the definition's own key is refused.
|
|
- The pull request appears in the result record, not yet in the UI or the API DTOs.
|
|
|
|
## Additional Changes
|
|
|
|
- The release notes of v1.35.0 lacked their closing `</ul>`.
|