From 77d23a8d12656650d2fb6f66b5eb41274dac21b2 Mon Sep 17 00:00:00 2001 From: Sall Date: Fri, 24 Jul 2026 09:37:46 +0100 Subject: [PATCH 1/4] docs: add branch-protection runbook from src/zsh-eza audit findings Adds runbooks/branch-protection.md, the checklist for provisioning and auditing repository settings and rulesets on next -> main repositories (decisions/0008-branching-model.md). Written from an audit of z-shell/src and z-shell/zsh-eza that found the same three gaps in both, independently: renovate.json missing baseBranches (Renovate bypassing next), no required check blocking direct-to-main PRs, and delete_branch_on_merge deleting the next branch itself after a promotion merge -- the ruleset deletion rule did not stop it under an admin bypass. Cross-links from dependency-management.md, new-repository.md, and ADR-0008's Consequences/References. Registers runbook-branch-protection in .github/instruction-surfaces.json. Closes #476 --- .github/instruction-surfaces.json | 12 ++++ decisions/0008-branching-model.md | 34 +++++----- runbooks/branch-protection.md | 107 ++++++++++++++++++++++++++++++ runbooks/dependency-management.md | 7 ++ runbooks/new-repository.md | 13 +++- 5 files changed, 157 insertions(+), 16 deletions(-) create mode 100644 runbooks/branch-protection.md diff --git a/.github/instruction-surfaces.json b/.github/instruction-surfaces.json index fc60e229a..258f8f273 100644 --- a/.github/instruction-surfaces.json +++ b/.github/instruction-surfaces.json @@ -423,6 +423,18 @@ "review_owner": "z-shell maintainers", "canonical_for": ["repository-bootstrap"] }, + { + "id": "runbook-branch-protection", + "path": "runbooks/branch-protection.md", + "kind": "runbook", + "authority": "canonical-detail", + "consumers": ["codex", "claude-code", "copilot", "gemini-cli", "human"], + "tasks": ["repository-bootstrap", "branch-protection"], + "file_patterns": ["**"], + "required": true, + "review_owner": "z-shell maintainers", + "canonical_for": ["branch-protection"] + }, { "id": "runbook-onboarding", "path": "runbooks/onboarding.md", diff --git a/decisions/0008-branching-model.md b/decisions/0008-branching-model.md index 494fdb189..e747967ed 100644 --- a/decisions/0008-branching-model.md +++ b/decisions/0008-branching-model.md @@ -18,7 +18,7 @@ inconsistency is real, not cosmetic: The meta-workspace catalog (`workspace/repos.yml`) had drifted from this reality and had to be reconciled by inspecting live remotes. The root cause is that no -decision says *which class of repo uses a `next` branch and which does not*, so +decision says _which class of repo uses a `next` branch and which does not_, so each repo's model is discovered empirically rather than governed. `zsh-lint` recently gained a `next` branch during its Go reboot, which re-surfaced the ambiguity. @@ -31,7 +31,7 @@ than decided per repo, so the catalog stops drifting at the source. The **canonical per-repo table below is the authoritative source** for branch model, and `workspace/repos.yml` derives from it. The ADR-0007 repository class -is an *input* to the choice (it sets the publication boundary and a default), but +is an _input_ to the choice (it sets the publication boundary and a default), but it does **not** by itself determine the branch model — repo churn/scale does. Reality confirms this: within class 1, `wiki`/`src` run `next` → `main` while `zd` is trunk-only; within class 2, `zsh-lint` uses `next` while `zunit` is @@ -41,19 +41,19 @@ requires amending this ADR (or a superseding one), not creating a branch. ### Canonical branch model -| Repo | Class | Branch model | Development branch | Publication boundary | -| ---------------------- | ----- | ---------------- | ------------------ | --------------------------- | -| `wiki` | 1 | `next` → `main` | `next` | merge to `main` (deploy) | -| `src` | 1 | `next` → `main` | `next` | merge to `main` (deploy) | -| `zd` | 1 | trunk on `main` | `main` | push to `main` (image) | -| `zunit` | 2 | trunk on `main` | `main` | `vX.Y.Z` tag | -| `zsh-lint` | 2 | `next` → `main` | `next` | `vX.Y.Z` tag | -| packaged `zsh` | 2 | trunk on `main` | `main` | `vX.Y.Z` tag (deferred) | -| `zi` | 3 | `next` → `main` | `next` | `main` is consumable ref | -| `zsh-eza` | 3 | `next` → `main` | `next` | `main` is consumable ref | -| `z-a-meta-plugins` | 3 | trunk on `main` | `main` | `main` is consumable ref | -| `zsh-fancy-completions`| 3 | trunk on `main` | `main` | `main` is consumable ref | -| `.github` | 4 | trunk on `main` | `main` | n/a | +| Repo | Class | Branch model | Development branch | Publication boundary | +| ----------------------- | ----- | --------------- | ------------------ | ------------------------ | +| `wiki` | 1 | `next` → `main` | `next` | merge to `main` (deploy) | +| `src` | 1 | `next` → `main` | `next` | merge to `main` (deploy) | +| `zd` | 1 | trunk on `main` | `main` | push to `main` (image) | +| `zunit` | 2 | trunk on `main` | `main` | `vX.Y.Z` tag | +| `zsh-lint` | 2 | `next` → `main` | `next` | `vX.Y.Z` tag | +| packaged `zsh` | 2 | trunk on `main` | `main` | `vX.Y.Z` tag (deferred) | +| `zi` | 3 | `next` → `main` | `next` | `main` is consumable ref | +| `zsh-eza` | 3 | `next` → `main` | `next` | `main` is consumable ref | +| `z-a-meta-plugins` | 3 | trunk on `main` | `main` | `main` is consumable ref | +| `zsh-fancy-completions` | 3 | trunk on `main` | `main` | `main` is consumable ref | +| `.github` | 4 | trunk on `main` | `main` | n/a | ### How the class informs the default @@ -90,6 +90,9 @@ catalog in the same change. - Promotion from `next` to `main` is a publication boundary only for class 1 (deploy) repos; for other classes the merge validates but does not mint a release (consistent with ADR-0007). +- This ADR sets the policy; `runbooks/branch-protection.md` covers the + repository-settings and ruleset provisioning that enforces it (added after + an audit found `src` and `zsh-eza` both missing parts of it). ## Alternatives considered @@ -107,3 +110,4 @@ catalog in the same change. - `decisions/0007-release-publication-flow.md` — repository classes this builds on. - `workspace/repos.yml` (meta-workspace) — per-repo branch model catalog. - `decisions/0003-conventional-commits.md` — commit/branch naming conventions. +- `runbooks/branch-protection.md` — enforcement checklist for this model. diff --git a/runbooks/branch-protection.md b/runbooks/branch-protection.md new file mode 100644 index 000000000..c8e1b5a0b --- /dev/null +++ b/runbooks/branch-protection.md @@ -0,0 +1,107 @@ +# Runbook — Branch Protection for `next` -> `main` Repositories + +Use this runbook when provisioning or auditing branch rulesets and repository +settings for a repository that uses the `next` -> `main` branch model +(`decisions/0008-branching-model.md`). It exists because `z-shell/src` and +`z-shell/zsh-eza` were both found, independently, with the same three gaps +during an audit — none of them were visible from the ruleset UI alone. + +## Why this exists + +A repository can have a correctly configured `main`/`next` ruleset pair and +still let `main` and `next` diverge, or lose the `next` branch outright, +because the gaps are in _repository settings_ and _automation defaults_ that +rulesets do not cover. Two incidents motivated this runbook: + +1. `src` and `zsh-eza` both had `renovate.json` with no `baseBranches` + override. Renovate defaulted to the repository's default branch (`main`), + opening routine dependency-update PRs that bypassed `next` entirely. Over + time, enough of these (plus a few manually-merged feature branches) landed + directly on `main` to diverge it from `next` by more than a dozen commits + in `zsh-eza`'s case, with real merge conflicts to resolve before `next` + could be promoted again. `dependency-management.md` already documents the + `baseBranches` override as an example — the actual gap was that nothing + audited whether a `next`-model repository had actually applied it. +2. Promoting `zsh-eza`'s `next` into `main` via a PR merge (`next` as the PR's + head branch) triggered GitHub's "Automatically delete head branches" repo + setting, which deleted `next` — the repository's persistent development + branch, not a disposable feature branch — immediately after the merge. + **The ruleset's `deletion` rule did not stop this.** The merge itself ran + under an organization-admin ruleset bypass (needed because the sole + `CODEOWNERS` entry was also the PR author, so the required code-owner + review could never be satisfied by anyone else), and the automatic + post-merge deletion inherited that same bypass context. + +## Checklist + +Run every item below for a repository whose `decisions/0008-branching-model.md` +row is `next` -> `main`. Skip repositories that are trunk-on-`main`. + +- [ ] **`delete_branch_on_merge` is `false` at the repository level.** + `gh api repos// --jq .delete_branch_on_merge`. If `true`, + any PR that uses `next` as its head branch (i.e. every `next` -> `main` + promotion) risks GitHub deleting `next` right after merge, regardless of + the ruleset's `deletion` rule. Disable it: + `gh api -X PATCH repos// --field delete_branch_on_merge=false`. + This is the single highest-value check in this runbook — it is the one + that caused actual data loss (recovered from the merge commit's known + SHA in this case, but that is luck, not a safety net). +- [ ] **`renovate.json` has `"baseBranches": ["next"]`** if the repository + uses Renovate. See `dependency-management.md` for the full config + example. Check `.github/dependabot.yml`'s `target-branch` too — it is + easy to fix Dependabot's target and assume Renovate inherited the same + fix; they are independent configs. +- [ ] **`main` and `next` are each governed by exactly one Repository Ruleset**, + not a mix of a ruleset and legacy classic branch protection. Classic + protection and rulesets both apply when both are present, and their + settings can silently contradict each other (observed: classic + protection allowing force-pushes while the ruleset's `non_fast_forward` + rule blocked them — harmless only because the stricter rule wins, but + confusing to audit and a sign the branch was migrated incompletely). + List them with `gh api repos///rulesets`; check for lingering + classic protection with `gh api repos///branches//protection` + (a `404` means none exists, which is correct). +- [ ] **A required status check blocks PRs into `main` whose head is not + `next` or `hotfix-*`.** Rulesets have no native "restrict PR source + branch" condition, so this has to be a CI check wired in as + `required_status_checks`. See `.github/workflows/main-branch-guard.yml` + in `z-shell/src` or `z-shell/zsh-eza` for the reference implementation + (a single `run:` step reading `github.head_ref`, no third-party + actions needed). The check must run at least once on a real PR against + `main` before GitHub will accept its context name in + `required_status_checks`. + +## Squash-merge trailers + +When squash-merging a `next` -> `main` promotion PR without an explicit +`--body`, GitHub synthesizes one by aggregating the squashed commits' +trailers — which reliably reintroduces `Co-authored-by` and `Signed-off-by` +trailers even when no individual commit you authored had one, violating the +organization's no-trailer policy. Always pass both `--subject` and an explicit +one-line `--body` (e.g. `gh pr merge --squash --body "..."`) to suppress +the synthesized body. Verify with +`gh api repos///commits/ --jq .commit.message` before +considering the promotion done. + +## Reference ruleset shape + +Both `main` and `next` should be a single Repository Ruleset each, scoped by +`refs/heads/`, with `bypass_actors` granting `OrganizationAdmin` and +the repository's admin/maintain/write roles `bypass_mode: always` (self-review +deadlock is expected and intentional: the only `CODEOWNERS` entry is often +also the person merging, so bypass is how promotions and fixes actually land; +`gh pr merge --admin` is the normal path here, not an escape hatch). + +`main`: `deletion`, `required_linear_history`, `pull_request` (code-owner +review required, approving-review count `0`), `non_fast_forward`, +`copilot_code_review`, `required_status_checks` (the guard workflow above). + +`next`: `deletion`, `non_fast_forward`, `required_signatures`, `pull_request`. +`required_linear_history` is deliberately absent from `next` — ordinary merge +commits (not squashes) are the convention for feature/fix PRs landing there. + +## See also + +- `decisions/0008-branching-model.md` +- `runbooks/dependency-management.md` +- `runbooks/new-repository.md` diff --git a/runbooks/dependency-management.md b/runbooks/dependency-management.md index a42b77e04..6c447b68c 100644 --- a/runbooks/dependency-management.md +++ b/runbooks/dependency-management.md @@ -64,6 +64,12 @@ exception, such as a non-default target branch: Custom managers and specialized package grouping also belong in the repository that needs them. +For any repository whose `decisions/0008-branching-model.md` row is `next` -> +`main`, this `baseBranches` override is not optional: without it Renovate +opens routine update PRs against `main` directly, bypassing `next`. This was +found live (not theoretical) in two repositories — see +`runbooks/branch-protection.md` for the full audit checklist. + ## Migrating a repository Do not remove Dependabot version updates until Renovate coverage is proven. @@ -121,3 +127,4 @@ If Renovate cannot access or process a repository: - `decisions/0012-hybrid-dependency-management.md` - `renovate-config.json` - `runbooks/new-repository.md` +- `runbooks/branch-protection.md` diff --git a/runbooks/new-repository.md b/runbooks/new-repository.md index 7da81de45..b056a3424 100644 --- a/runbooks/new-repository.md +++ b/runbooks/new-repository.md @@ -115,9 +115,19 @@ Follow `runbooks/dependency-management.md`: 3. Enable the dependency graph, Dependabot alerts, and Dependabot security updates in GitHub settings. 4. Add `renovate.json` only for a repository-specific exception such as a - `next` target branch. + `next` target branch. If `decisions/0008-branching-model.md` assigns this + repository the `next` -> `main` model, this exception is mandatory, not + optional — see `runbooks/branch-protection.md`. 5. Do not add `.github/dependabot.yml` for routine version updates. +## Step 5a — Provision branch rulesets + +If this repository uses the `next` -> `main` model +(`decisions/0008-branching-model.md`), follow `runbooks/branch-protection.md` +in full before opening the bootstrap pull request. Trunk-on-`main` +repositories still need a `main` ruleset, but can skip the `next`-specific +items (the guard workflow, `renovate.json` override). + ## Step 6 — Verify before publication Before opening the bootstrap pull request: @@ -146,6 +156,7 @@ Reusable screenshot and terminal-demo generation is tracked separately in - `AGENTS.md` - `PATTERNS.md` +- `runbooks/branch-protection.md` - `runbooks/dependency-management.md` - `runbooks/labels.md` - `runbooks/project-tracker.md` From a0e7dbfeb4eb51f12a0fa85be03fa08665ce0e9d Mon Sep 17 00:00:00 2001 From: Sal <59910950+ss-o@users.noreply.github.com> Date: Fri, 24 Jul 2026 23:04:53 +0100 Subject: [PATCH 2/4] Apply suggestions from code review Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> Signed-off-by: Sal <59910950+ss-o@users.noreply.github.com> --- runbooks/branch-protection.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/runbooks/branch-protection.md b/runbooks/branch-protection.md index c8e1b5a0b..78452f32f 100644 --- a/runbooks/branch-protection.md +++ b/runbooks/branch-protection.md @@ -78,7 +78,7 @@ When squash-merging a `next` -> `main` promotion PR without an explicit trailers — which reliably reintroduces `Co-authored-by` and `Signed-off-by` trailers even when no individual commit you authored had one, violating the organization's no-trailer policy. Always pass both `--subject` and an explicit -one-line `--body` (e.g. `gh pr merge --squash --body "..."`) to suppress +one-line `--body` (e.g. `gh pr merge --squash --subject "..." --body "..."`) to suppress the synthesized body. Verify with `gh api repos///commits/ --jq .commit.message` before considering the promotion done. From 8d312953fe6dbbeaaea2f37f493bcc7cb40c1ae4 Mon Sep 17 00:00:00 2001 From: Sall Date: Fri, 24 Jul 2026 23:44:32 +0100 Subject: [PATCH 3/4] docs(runbooks): use consistent next -> main arrow glyph Copilot flagged 8 spots using ASCII -> instead of the Unicode -> already used by ADR-0008 and the rest of the runbook set. --- runbooks/branch-protection.md | 10 +++++----- runbooks/dependency-management.md | 2 +- runbooks/new-repository.md | 4 ++-- 3 files changed, 8 insertions(+), 8 deletions(-) diff --git a/runbooks/branch-protection.md b/runbooks/branch-protection.md index 78452f32f..a1b356258 100644 --- a/runbooks/branch-protection.md +++ b/runbooks/branch-protection.md @@ -1,7 +1,7 @@ -# Runbook — Branch Protection for `next` -> `main` Repositories +# Runbook — Branch Protection for `next` → `main` Repositories Use this runbook when provisioning or auditing branch rulesets and repository -settings for a repository that uses the `next` -> `main` branch model +settings for a repository that uses the `next` → `main` branch model (`decisions/0008-branching-model.md`). It exists because `z-shell/src` and `z-shell/zsh-eza` were both found, independently, with the same three gaps during an audit — none of them were visible from the ruleset UI alone. @@ -35,11 +35,11 @@ rulesets do not cover. Two incidents motivated this runbook: ## Checklist Run every item below for a repository whose `decisions/0008-branching-model.md` -row is `next` -> `main`. Skip repositories that are trunk-on-`main`. +row is `next` → `main`. Skip repositories that are trunk-on-`main`. - [ ] **`delete_branch_on_merge` is `false` at the repository level.** `gh api repos// --jq .delete_branch_on_merge`. If `true`, - any PR that uses `next` as its head branch (i.e. every `next` -> `main` + any PR that uses `next` as its head branch (i.e. every `next` → `main` promotion) risks GitHub deleting `next` right after merge, regardless of the ruleset's `deletion` rule. Disable it: `gh api -X PATCH repos// --field delete_branch_on_merge=false`. @@ -73,7 +73,7 @@ row is `next` -> `main`. Skip repositories that are trunk-on-`main`. ## Squash-merge trailers -When squash-merging a `next` -> `main` promotion PR without an explicit +When squash-merging a `next` → `main` promotion PR without an explicit `--body`, GitHub synthesizes one by aggregating the squashed commits' trailers — which reliably reintroduces `Co-authored-by` and `Signed-off-by` trailers even when no individual commit you authored had one, violating the diff --git a/runbooks/dependency-management.md b/runbooks/dependency-management.md index 6c447b68c..abd263e04 100644 --- a/runbooks/dependency-management.md +++ b/runbooks/dependency-management.md @@ -64,7 +64,7 @@ exception, such as a non-default target branch: Custom managers and specialized package grouping also belong in the repository that needs them. -For any repository whose `decisions/0008-branching-model.md` row is `next` -> +For any repository whose `decisions/0008-branching-model.md` row is `next` → `main`, this `baseBranches` override is not optional: without it Renovate opens routine update PRs against `main` directly, bypassing `next`. This was found live (not theoretical) in two repositories — see diff --git a/runbooks/new-repository.md b/runbooks/new-repository.md index b056a3424..212bbcc98 100644 --- a/runbooks/new-repository.md +++ b/runbooks/new-repository.md @@ -116,13 +116,13 @@ Follow `runbooks/dependency-management.md`: updates in GitHub settings. 4. Add `renovate.json` only for a repository-specific exception such as a `next` target branch. If `decisions/0008-branching-model.md` assigns this - repository the `next` -> `main` model, this exception is mandatory, not + repository the `next` → `main` model, this exception is mandatory, not optional — see `runbooks/branch-protection.md`. 5. Do not add `.github/dependabot.yml` for routine version updates. ## Step 5a — Provision branch rulesets -If this repository uses the `next` -> `main` model +If this repository uses the `next` → `main` model (`decisions/0008-branching-model.md`), follow `runbooks/branch-protection.md` in full before opening the bootstrap pull request. Trunk-on-`main` repositories still need a `main` ruleset, but can skip the `next`-specific From 74715cdd048c4d6536087565f7dd9b7096d61606 Mon Sep 17 00:00:00 2001 From: Sall Date: Sat, 25 Jul 2026 00:13:05 +0100 Subject: [PATCH 4/4] docs(runbooks): scope the trailer warning to the actual policy Copilot review: AGENTS.md only disallows Co-authored-by; there is no blanket no-trailer policy and Signed-off-by is not itself prohibited. --- runbooks/branch-protection.md | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/runbooks/branch-protection.md b/runbooks/branch-protection.md index a1b356258..c1b191717 100644 --- a/runbooks/branch-protection.md +++ b/runbooks/branch-protection.md @@ -76,8 +76,10 @@ row is `next` → `main`. Skip repositories that are trunk-on-`main`. When squash-merging a `next` → `main` promotion PR without an explicit `--body`, GitHub synthesizes one by aggregating the squashed commits' trailers — which reliably reintroduces `Co-authored-by` and `Signed-off-by` -trailers even when no individual commit you authored had one, violating the -organization's no-trailer policy. Always pass both `--subject` and an explicit +trailers even when no individual commit you authored had one. Only +`Co-authored-by` is organization-disallowed (`AGENTS.md`); letting a +synthesized body reintroduce it violates that policy regardless of which +squashed commit it came from. Always pass both `--subject` and an explicit one-line `--body` (e.g. `gh pr merge --squash --subject "..." --body "..."`) to suppress the synthesized body. Verify with `gh api repos///commits/ --jq .commit.message` before