Update pull requests when their base moves, merge them on approval #18

Merged

larandar merged 2 commits from agent.teyla/auto-update-and-merge into main

2026-08-10 17:42:30 +00:00

AGit

agent.teyla commented

2026-08-10 10:19:46 +00:00

Contributor

Closes #17. Stacked on #16 — merge that first; this branch is based on it, so the diff against main shows both commits. Only 3bb4156 belongs to this PR.

Two rules the substrate already supported and nobody had written. Both are Datalog in src/policy.rs, both declare effects and perform nothing.

Merging on approval

On an approving review of a non-draft pull request, declare the merge — rebase when it carries the curated label, squash when it does not.

Ting/Ting#4 curates history into logical commits before the PR, so a curated branch deserves to be replayed: the commits that were curated are the commits that land. A branch without the label is exploratory session work, and replaying it publishes exactly the noise curation exists to remove, so it is folded into one. The label is the author asserting they did the work; the steward reads it, it does not award it.

head_commit_id is the load-bearing field, not a detail. An approval is of a commit, not of a branch name. Without it, a push landing between the review and the merge is merged unreviewed — and the steward would have been the thing that asked for it. With it, Forgejo refuses rather than quietly taking the wrong tree. There is a test whose entire job is that property.

Drafts are excluded: a draft is not finished being written, and an approval on one is not a request to land it.

Keeping pull requests current

On a push to a base branch, declare that open pull requests based on it be updated — by rebase, never merge, because updating by merge writes a merge commit into the branch whose history the estate curates and whose merge styles #5 turns off.

Which pull requests those are is not in the payload and does not need to be: the steward declares what must become true; finding them is the executor's work. This fires alongside the protected-push denial when the base is main, and both conclusions are true at once — who pushed is one question, what must follow is another, and resolution is the substrate's job.

Two declarations that cannot disagree

nix/policy.nix now takes mergeStyles from nix/forgejo.nix instead of restating it, so the styles the rules may declare cannot fall outside the styles the estate's repos allow. Restating them would let #5 turn a style off for the repos while the steward went on asking for it — and the failure would surface at the forge, on a real pull request, at merge time. Forgejo's Do values and its per-repo allow flags are two vocabularies for one thing, and allowFlagFor is the only place they meet.

That is why this PR is stacked: mergeStyles arrives in #16.

What I could not verify, and what I did about it

Forgejo's wire name for an approving review is unverified from this estate. There is no webhook on any repo to read the delivery header off, and the swagger does not enumerate hook event names. So it is a crystallized parameter, not rule logic — approval_kinds lists both plausible spellings, and getting it wrong costs a Nix eval rather than a rebuild. The same reasoning covers the label name and both styles.

The hook-config spelling (pull_request_review_approved, added to the declared events) is a third name and is unverified for the same reason. Confirm both against a real delivery when the webhook is first provisioned, and delete whichever approval_kinds entry never arrives.

In scope

merge_with / approved / pr_curated / pr_draft / pr_head rules and the two conclusions that carry effects.
base_refs, approval_kinds, curated_label, merge_style_curated, merge_style_uncurated as crystallized parameters; policy schema 2 → 3.
pull_request_review_approved on the declared webhook.

Out of scope

Performing either effect. The steward holds no Forgejo token and checks.module-evaluates fails the build if its unit grows one. Execution is the extension this repo has named since PR #2.
Deciding whether a review is authoritative. Branch protection already owns required approvals and who may give them; re-deciding it here would put the answer in two places.
Awarding the curated label.
Update-then-merge sequencing. Each rule declares its own effect from its own event; chaining is an executor's concern.

Boundary

A schema bump means a deployed steward running v2 parameters refuses to start rather than silently gating less — which is the intent, and is already covered by a_future_schema_version_is_refused.

The update effect names a base branch, not a list of pull requests. That is deliberate (the payload has no such list), but it does mean the effect is coarser than the others: an executor acting on it must enumerate, and enumerating is where it could act on a PR nobody asked about. Worth keeping in view when the executor is built.

Acceptance

cargo test — 31 pass (was 19), 12 new. ✅
cargo clippy --all-targets -- -D warnings clean; cargo fmt --check clean. ✅
nix flake check — green, 10 checks, including the policy-drift check against the regenerated policy/ting.json. ✅
Curated + approved → one merge effect, Do: rebase, naming repository and index 16. ✅
Uncurated + approved → Do: squash. ✅
A near-miss label (curated-ish) does not count as curation. ✅
The merge names the approved head commit. ✅
Approving a draft declares no merge. ✅
A non-approval event with the same payload declares no merge. ✅
Push to refs/heads/fastlane → one update effect, style: rebase, naming repository and base; push to refs/heads/tango/17-something → nothing. ✅
Mutation-tested: allow_squash_merge = false in nix/forgejo.nix makes nix build .#policy fail with Ting's policy would declare merge style(s) the estate's repos do not allow: squash. — then reverted to green. ✅

Verified with --override-input jostoph-rs path:…, as #13 has not merged.

Closes #17. **Stacked on #16** — merge that first; this branch is based on it, so the diff against `main` shows both commits. Only `3bb4156` belongs to this PR. Two rules the substrate already supported and nobody had written. Both are Datalog in `src/policy.rs`, both declare effects and perform nothing. ## Merging on approval On an approving review of a non-draft pull request, declare the merge — **`rebase` when it carries the `curated` label, `squash` when it does not**. Ting/Ting#4 curates history into logical commits *before* the PR, so a curated branch deserves to be **replayed**: the commits that were curated are the commits that land. A branch without the label is exploratory session work, and replaying it publishes exactly the noise curation exists to remove, so it is folded into one. The label is the author asserting they did the work; the steward reads it, it does not award it. **`head_commit_id` is the load-bearing field, not a detail.** An approval is of a *commit*, not of a branch name. Without it, a push landing between the review and the merge is merged unreviewed — and the steward would have been the thing that asked for it. With it, Forgejo refuses rather than quietly taking the wrong tree. There is a test whose entire job is that property. Drafts are excluded: a draft is not finished being written, and an approval on one is not a request to land it. ## Keeping pull requests current On a push to a base branch, declare that open pull requests based on it be updated — by **`rebase`**, never `merge`, because updating by merge writes a merge commit into the branch whose history the estate curates and whose merge styles #5 turns off. Which pull requests those are is not in the payload and does not need to be: the steward declares what must become true; finding them is the executor's work. This fires alongside the protected-push denial when the base is `main`, and both conclusions are true at once — who pushed is one question, what must follow is another, and resolution is the substrate's job. ## Two declarations that cannot disagree `nix/policy.nix` now takes `mergeStyles` from `nix/forgejo.nix` instead of restating it, so the styles the rules may **declare** cannot fall outside the styles the estate's repos **allow**. Restating them would let #5 turn a style off for the repos while the steward went on asking for it — and the failure would surface at the forge, on a real pull request, at merge time. Forgejo's `Do` values and its per-repo allow flags are two vocabularies for one thing, and `allowFlagFor` is the only place they meet. That is why this PR is stacked: `mergeStyles` arrives in #16. ## What I could not verify, and what I did about it **Forgejo's wire name for an approving review is unverified from this estate.** There is no webhook on any repo to read the delivery header off, and the swagger does not enumerate hook event names. So it is a crystallized *parameter*, not rule logic — `approval_kinds` lists both plausible spellings, and getting it wrong costs a Nix eval rather than a rebuild. The same reasoning covers the label name and both styles. The hook-config spelling (`pull_request_review_approved`, added to the declared `events`) is a *third* name and is unverified for the same reason. Confirm both against a real delivery when the webhook is first provisioned, and delete whichever `approval_kinds` entry never arrives. ## In scope - `merge_with` / `approved` / `pr_curated` / `pr_draft` / `pr_head` rules and the two conclusions that carry effects. - `base_refs`, `approval_kinds`, `curated_label`, `merge_style_curated`, `merge_style_uncurated` as crystallized parameters; policy schema **2 → 3**. - `pull_request_review_approved` on the declared webhook. ## Out of scope - **Performing either effect.** The steward holds no Forgejo token and `checks.module-evaluates` fails the build if its unit grows one. Execution is the extension this repo has named since PR #2. - **Deciding whether a review is authoritative.** Branch protection already owns required approvals and who may give them; re-deciding it here would put the answer in two places. - **Awarding the curated label.** - **Update-then-merge sequencing.** Each rule declares its own effect from its own event; chaining is an executor's concern. ## Boundary A schema bump means a deployed steward running v2 parameters refuses to start rather than silently gating less — which is the intent, and is already covered by `a_future_schema_version_is_refused`. The update effect names a base branch, not a list of pull requests. That is deliberate (the payload has no such list), but it does mean the effect is coarser than the others: an executor acting on it must enumerate, and enumerating is where it could act on a PR nobody asked about. Worth keeping in view when the executor is built. ## Acceptance - `cargo test` — **31 pass** (was 19), 12 new. ✅ - `cargo clippy --all-targets -- -D warnings` clean; `cargo fmt --check` clean. ✅ - `nix flake check` — green, 10 checks, including the policy-drift check against the regenerated `policy/ting.json`. ✅ - Curated + approved → one merge effect, `Do: rebase`, naming repository and index `16`. ✅ - Uncurated + approved → `Do: squash`. ✅ - A near-miss label (`curated-ish`) does **not** count as curation. ✅ - The merge names the approved head commit. ✅ - Approving a draft declares no merge. ✅ - A non-approval event with the same payload declares no merge. ✅ - Push to `refs/heads/fastlane` → one update effect, `style: rebase`, naming repository and base; push to `refs/heads/tango/17-something` → nothing. ✅ - **Mutation-tested**: `allow_squash_merge = false` in `nix/forgejo.nix` makes `nix build .#policy` fail with `Ting's policy would declare merge style(s) the estate's repos do not allow: squash.` — then reverted to green. ✅ Verified with `--override-input jostoph-rs path:…`, as #13 has not merged.

agent.teyla added 2 commits

2026-08-10 10:19:46 +00:00

forgejo: declare the repos the estate wants, and what holds on them bbf39e7b5c

#3 (push mirrors) and #5 (merge styles) both ask for per-repo forge
settings that must not drift. Declared in nix/forgejo.nix beside the
webhook, in Forgejo's own field names — and not reconciled here, for the
reason already at the top of that file.

#3 left "provider or steward?" open. A push mirror is the hook case: it
is addressed by a remote_name the forge invents (CreatePushMirrorOption
has no field for it), so its identity is state, and state is the
provider's requireState case. Read off the live forge, not assumed.

This is desired state, not an inventory. Repos are declared by org, and
a repo or mirror target that does not exist yet is a thing to create,
not a reason to leave it undeclared — midgard/modron is declared for
exactly that reason. The converse does not hold: a repo on the forge and
absent here is a finding for a person, never a deletion.

The instance defaults are upstreamed from lar.ad/flake-ops, where they
are applied. They are the same policy as the per-repo settings, split
across two mechanisms by Forgejo rather than by intent; kept in separate
files they can disagree silently, so they are rendered into one document
and the check compares them. DEFAULT_* only reaches repos created after
it is set — it reached none of ours, which is why the per-repo half has
to exist and why delete-branch-after-merge is declared twice.

Both ends of a mirror are declared. A midgard repo has shed the fastlane
flow: main only, no pull requests — a PR against a copy a mirror
overwrites on a schedule produces work the next sync discards — and the
source mirror's public key as a WRITABLE deploy key, since a read-only
one cannot be pushed with. Public faces are derived from the same pairs
as the mirrors, so neither can exist without the other, and the public
org cannot be written into by hand.

branch_filter is main, and that is the field that matters. The hand-made
mirror on lar.ad/jostoph-rs carries an empty filter — every branch — and
a development branch created on the canonical was published to the
public repo, where deleting it from the source did not retract it.

Closes #3
Closes #5

policy: update pull requests when their base moves, merge them on approval 3bb4156142

Two rules the substrate already supported and nobody had written.

On an approving review of a non-draft pull request, declare the merge:
rebase when it carries the curated label, squash when it does not.
Ting/Ting#4 curates history into logical commits BEFORE the PR, so a
curated branch is replayed and an uncurated one is folded into one
commit rather than publishing the session's false starts. The label is
the author's assertion; the steward reads it, it does not award it.

The merge names head_commit_id. An approval is of a commit, not of a
branch name: without it a push landing between the review and the merge
gets merged unreviewed, and the steward would have asked for it.

On a push to a base branch, declare that open pull requests on it be
updated, by rebase — updating by merge writes a merge commit into the
branch whose history the estate curates. Which pull requests those are
is not in the payload and does not need to be: the steward says what
must become true, finding them is the executor's work. This fires
alongside the protected-push denial on main, and both are true at once.

Declared, never performed — the same posture forgejo.comment has had
since PR #2.

The merge styles come from nix/forgejo.nix's allow-set rather than being
restated, so the steward cannot ask for a style #5 turned off; policy
fails at eval time if it ever could. The approval event kind, the label
and the styles are all crystallized parameters, not rule logic: Forgejo's
wire name for a review could not be verified from here — no webhook
exists to read it off and the swagger does not enumerate hook events —
so a wrong guess must cost an eval, not a rebuild.

Closes #17