From 9dacd079ddab8a1c3d095bcbe2ff84168c79c1d2 Mon Sep 17 00:00:00 2001 From: Mr-RedHat-fb Date: Mon, 13 Jul 2026 09:25:31 +0200 Subject: [PATCH 1/2] feat(ci): dead-branch sweeper (dry-run by default, merged-only delete) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Central, org-wide job (register-skill.yml's shape, not a per-repo workflow_call) that deletes only branches fully merged into their repo's default branch — checked both by commit-containment and by merged-PR history, so squash/rebase merges are still recognised as merged. Never touches the default branch, the mnab set (main/next/before/after), anything GitHub reports as protected, or any branch with an open PR. Unmerged branches idle > stale_days (default 90) with no open PR are report-only in the job summary + JSON artifact — the "stalled but maybe-real work" signal the legibility doctrine says to surface, not erase. Ships inert: scheduled runs stay dry-run unless the operator sets BRANCH_SWEEP_ARMED=true; manual dispatch also defaults dry_run=true. Token model documented in docs/branch-sweep.md: interim bot-owned fine-grained PAT (Contents: write, Pull requests: read, least privilege), target-state migration path to the already-decided-but-unregistered aifred-maintenance App noted for when that App exists. Governance note: no DECISIONS.md entry currently authorizes org-wide auto-delete of merged branches. This PR does not add one — it ships inert (dry-run) and flags that a short DECISIONS.md post should land before BRANCH_SWEEP_ARMED is ever set to true anywhere. See docs/branch-sweep.md "Governance note" section. Shellcheck-clean (scripts/dead-branch-sweep.sh), YAML-validated. Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_01JGpSmSMHtG3rP13yq5sU1g --- .github/workflows/dead-branch-sweep.yml | 107 ++++++++++++++ README.md | 3 + docs/branch-sweep.md | 118 +++++++++++++++ scripts/dead-branch-sweep.sh | 183 ++++++++++++++++++++++++ 4 files changed, 411 insertions(+) create mode 100644 .github/workflows/dead-branch-sweep.yml create mode 100644 docs/branch-sweep.md create mode 100755 scripts/dead-branch-sweep.sh diff --git a/.github/workflows/dead-branch-sweep.yml b/.github/workflows/dead-branch-sweep.yml new file mode 100644 index 0000000..4dc349b --- /dev/null +++ b/.github/workflows/dead-branch-sweep.yml @@ -0,0 +1,107 @@ +name: dead-branch-sweep + +# Central, org-wide dead-branch sweeper. Lives here (not as a per-repo +# workflow_call reusable) because "dead branches across the org" is a single +# cross-repo job, not something each consumer repo needs to opt into +# separately — same shape as register-skill.yml. +# +# SAFETY MODEL (see scripts/dead-branch-sweep.sh for the full contract): +# - Only ever DELETES branches that are fully MERGED (checked two ways — +# commit-containment AND merged-PR history, so squash/rebase merges are +# still recognised as merged). +# - NEVER touches the default branch, the mnab set (main/next/before/ +# after), anything GitHub itself reports as protected, or any branch +# with an open PR. +# - Unmerged + stale (no open PR, no activity > stale_days) branches are +# REPORT-ONLY in the job summary + JSON artifact — never deleted. That +# silence-plus-no-PR signal is exactly the "stalled but maybe real work" +# case the legibility doctrine says to surface, not erase. +# - Defaults to dry-run. The scheduled (cron) run only arms (actually +# deletes) if the repo variable BRANCH_SWEEP_ARMED is literally "true" — +# an explicit, separate, operator-controlled step outside this PR. A +# manual workflow_dispatch run defaults its own dry_run input to true as +# well, so nothing deletes anything until someone deliberately says so +# twice over (once to arm the schedule, once per manual run if used). +# +# SCOPE NOTE: this reaches only repos the BRANCH_SWEEP_TOKEN can see — i.e. +# alfred-intelligence-owned repos reachable by the fleet identity that owns +# the token. It is not an org-universal or cross-owner sweep; GeGGe01/ +# SAVANTERNA/kebab-it-owned repos are out of scope by construction (see +# docs/branch-sweep.md). + +on: + schedule: + # Weekly, off-peak, off-the-hour: Sunday 03:17 UTC. + - cron: "17 3 * * 0" + workflow_dispatch: + inputs: + dry_run: + description: "Dry run — report what WOULD be deleted without deleting." + required: true + default: true + type: boolean + stale_days: + description: "Days of inactivity before an unmerged branch is reported (never deleted)." + required: false + default: "90" + type: string + repos_override: + description: "Optional comma-separated owner/repo list. Empty = enumerate the whole org." + required: false + default: "" + type: string + +permissions: + contents: read # only for checking out this repo's own script + +jobs: + sweep: + runs-on: ubuntu-latest + steps: + - name: Verify configuration + env: + SWEEP_TOKEN_SET: ${{ secrets.BRANCH_SWEEP_TOKEN != '' }} + run: | + if [[ "$SWEEP_TOKEN_SET" != "true" ]]; then + echo "::error::missing secret BRANCH_SWEEP_TOKEN — see docs/branch-sweep.md" + exit 1 + fi + + - uses: actions/checkout@v4 + + - name: Resolve dry-run mode + id: mode + env: + ARMED_VAR: ${{ vars.BRANCH_SWEEP_ARMED }} + IS_SCHEDULE: ${{ github.event_name == 'schedule' }} + DISPATCH_DRY_RUN: ${{ inputs.dry_run }} + run: | + # Schedule runs are dry-run unless the operator has explicitly set + # the BRANCH_SWEEP_ARMED repo/org variable to "true". Manual + # dispatch runs use the input as-is (itself defaulting to true). + if [[ "$IS_SCHEDULE" == "true" ]]; then + if [[ "$ARMED_VAR" == "true" ]]; then + echo "dry_run=false" >> "$GITHUB_OUTPUT" + else + echo "dry_run=true" >> "$GITHUB_OUTPUT" + fi + else + echo "dry_run=${DISPATCH_DRY_RUN}" >> "$GITHUB_OUTPUT" + fi + + - name: Sweep + env: + GH_TOKEN: ${{ secrets.BRANCH_SWEEP_TOKEN }} + ORG: alfred-intelligence + DRY_RUN: ${{ steps.mode.outputs.dry_run }} + STALE_DAYS: ${{ inputs.stale_days || '90' }} + REPOS_OVERRIDE: ${{ inputs.repos_override || '' }} + OUTPUT_JSON: ${{ runner.temp }}/sweep-summary.json + run: bash scripts/dead-branch-sweep.sh | tee -a "$GITHUB_STEP_SUMMARY" + + - name: Upload JSON summary + uses: actions/upload-artifact@v4 + with: + name: dead-branch-sweep-summary + path: ${{ runner.temp }}/sweep-summary.json + retention-days: 90 diff --git a/README.md b/README.md index 15bd348..4443b56 100644 --- a/README.md +++ b/README.md @@ -17,6 +17,9 @@ alfred-intelligence ecosystem. | [`.github/workflows/dependabot-automerge.yml`](.github/workflows/dependabot-automerge.yml) | Reusable (`workflow_call`) Dependabot auto-merge: minor/patch bumps queue `gh pr merge --auto` once required checks are green; majors are left for a human. Policy: `DECISIONS.md` (S-konservoppnaren). | | [`scripts/gh-app-installation-token.sh`](scripts/gh-app-installation-token.sh) | Mint a GitHub App installation token outside a workflow (local agent use, e.g. Governator's org-ruleset sweep). | | [`docs/aifred-governance-app.md`](docs/aifred-governance-app.md) | The read-only `aifred-governance` App: permissions, credential location, how to invoke the mint script. | +| [`.github/workflows/dead-branch-sweep.yml`](.github/workflows/dead-branch-sweep.yml) | Central, org-wide sweep that deletes only fully-merged branches; reports (never deletes) stale unmerged ones. Dry-run by default. | +| [`scripts/dead-branch-sweep.sh`](scripts/dead-branch-sweep.sh) | The sweep logic, callable standalone for local testing. | +| [`docs/branch-sweep.md`](docs/branch-sweep.md) | Safety guarantees, token scope, arming procedure for the dead-branch sweep. | ## Quick start diff --git a/docs/branch-sweep.md b/docs/branch-sweep.md new file mode 100644 index 0000000..a8140ef --- /dev/null +++ b/docs/branch-sweep.md @@ -0,0 +1,118 @@ +# Dead-branch sweep + +`dead-branch-sweep.yml` is a central, org-wide job (not a per-repo +`workflow_call` reusable) that finds and removes dead branches across +`alfred-intelligence`. It lives in this repo the same way +`register-skill.yml` does: one cross-repo job, deployed once, rather than +something each consumer repo opts into separately. + +## Safety guarantees (read this before arming it) + +- **Deletes ONLY fully-merged branches.** "Merged" is decided two ways, + OR-ed together: + 1. Commit-containment: `compare ...` reports `identical` + or `behind` — the branch has no commits the default branch lacks. + 2. Merged-PR history: a closed PR exists for that branch head with + `merged_at` set — this is what catches **squash and rebase merges**, + which (1) alone would miss because the base branch gets a *new* commit + SHA, not the branch's original commits. +- **Never deletes:** the repo's own default branch, the mnab set + (`main`, `next`, `before`, `after`) by name, any branch GitHub's API + itself reports as `protected` (covers release branches and anything + branch-protection-covered beyond the mnab set), or any branch with an + **open** pull request — regardless of merge state. +- **Stale unmerged branches are report-only.** A branch with no open PR, + not merged, and no commit activity in `stale_days` (default 90) is listed + in the job summary and JSON artifact for human triage — never deleted. + Per the legibility doctrine, an unmerged branch nobody opened a PR for is + exactly the "stalled but maybe-real work" signal, not garbage. +- **Dry-run by default, twice over.** The scheduled (cron) run only deletes + anything if the repo variable `BRANCH_SWEEP_ARMED` is literally `true` — + an explicit step outside this workflow's own PR. A manual + `workflow_dispatch` run's `dry_run` input also defaults to `true`. Nothing + deletes until an operator deliberately flips one of those. + +## What it does not cover + +- **Scope is the token's reach, not the whole org's repos in a legal + sense.** The sweep enumerates repos via `GET /orgs/alfred-intelligence/ + repos` using `BRANCH_SWEEP_TOKEN` — it only sees repos that token can + read/write. It never crosses to `GeGGe01`, `SAVANTERNA`, `kebab-it`, or + any other owner; those are out of scope by construction, not by choice + each run. +- It does not touch tags, releases, or anything outside `refs/heads/*`. +- It does not open issues. Report output is the job summary (markdown + tables, human-readable in the Actions run) plus a JSON artifact + (`dead-branch-sweep-summary`, 90-day retention) for anything that wants + to consume it programmatically (e.g. a future lumberjack ingest). + +## Configuration + +| Name | Type | Scope | Purpose | +|---|---|---|---| +| `BRANCH_SWEEP_TOKEN` | secret | org (recommended) or repo | Token used for every cross-repo API call: list repos/branches, compare, list PRs, delete refs. | +| `BRANCH_SWEEP_ARMED` | variable | org or repo | Must be exactly `true` for the **scheduled** run to actually delete anything. Absent/anything else = dry-run. | + +`workflow_dispatch` inputs (`dry_run`, `stale_days`, `repos_override`) are +per-run and don't need any variable set — useful for a supervised first +pass before touching `BRANCH_SWEEP_ARMED` at all. + +## Token scope — current state vs. target state + +**Current (this PR):** `BRANCH_SWEEP_TOKEN` is a fine-grained PAT owned by +the fleet bot account (`alfred-int-bot`), following the same pattern as +`MARKETPLACE_PAT` (see [`docs/auth.md`](auth.md)) — a bot-owned token kept +in the org (or repo) secret store, least-privilege repository permissions: + +- `Contents: Read and write` (branch read/delete) +- `Pull requests: Read` (open-PR check, merged-PR history for squash/rebase + detection — write is never needed) + +Set while signed in as the bot account, resource owner = the bot account, +repository access = **All repositories** in `alfred-intelligence` (needed +to reach the whole org from one token; requires org-admin approval for a +member-owned fine-grained PAT with org-wide repo access — an operator +step, same class as any fine-grained-PAT-across-many-repos grant). + +**Target state:** this exact permission profile — +`Contents: write` + `Pull requests: read`, org-wide — is inside the +already-decided but not-yet-registered `aifred-maintenance` App's scope +(`.github-private/strategy/meta-apps.md`: "Stale-issue triage, +dependency-bump PRs, lint-fix PRs" — a dead-branch sweep is the same class +of maintenance chore). Once that App is registered (blocked on an +operator-only GitHub UI step, same as `aifred-governance`'s), migrate this +workflow to `actions/create-github-app-token` with +`permission-contents: write` / `permission-pull-requests: read` +(sub-setting the App's own broader declared permissions, per the "Token +issuance pattern" in `meta-apps.md`) instead of a standing PAT. This is a +mechanical swap of the auth step only — the sweep logic +(`scripts/dead-branch-sweep.sh`) does not change. + +## Governance note — flagged, not decided here + +`DECISIONS.md` has no existing entry authorizing "merged branches get +auto-deleted org-wide." The zero-cost-review-stack and mnab-CI-gate-gradient +decisions govern *what runs and what's required*, not branch lifecycle — +this sweep doesn't fight either (it never touches protected branches or +required-check config, and dry-run-by-default keeps it inert until an +operator arms it). But "delete X automatically, org-wide" is exactly the +shape of decision the governance doc says lives in `DECISIONS.md`, not +something a workflow silently decides for itself. **This PR does not add +that entry** — it ships the mechanism inert (dry-run) and flags that the +operator/governance-owner should add a short `DECISIONS.md` post before +`BRANCH_SWEEP_ARMED` is ever set to `true` anywhere. + +## Local testing + +The script is standalone and callable outside Actions: + +```bash +GH_TOKEN=$(gh auth token) \ +ORG=alfred-intelligence \ +DRY_RUN=true \ +STALE_DAYS=90 \ + bash scripts/dead-branch-sweep.sh +``` + +Use `REPOS_OVERRIDE=owner/repo1,owner/repo2` to scope a test run to one or +two repos before trusting it against the whole org. diff --git a/scripts/dead-branch-sweep.sh b/scripts/dead-branch-sweep.sh new file mode 100755 index 0000000..c79d395 --- /dev/null +++ b/scripts/dead-branch-sweep.sh @@ -0,0 +1,183 @@ +#!/usr/bin/env bash +# dead-branch-sweep.sh — org-wide dead-branch sweeper for alfred-intelligence. +# +# WHY THIS EXISTS: branches accumulate. Most are dead weight (merged and +# forgotten); a few are silently-stalled real work (unmerged, no open PR, +# untouched for months) — per the "everything-must-work"/legibility doctrine, +# THOSE are a triage signal, not garbage. This script tells the two apart and +# only ever deletes the first kind. +# +# SAFE BY DESIGN: +# - Deletes ONLY branches that are fully MERGED into the repo's default +# branch. "Merged" is checked two ways or-ed together, because neither +# alone is reliable: +# (a) commit-containment via `compare base...head` (status identical +# or behind) — catches merge-commit and fast-forward merges. +# (b) a closed PR for that branch head with merged_at set — catches +# squash/rebase merges, where (a) alone would say "diverged" +# even though the PR genuinely merged. +# - NEVER deletes: the default branch, any name in PROTECTED_BRANCHES +# (main/next/before/after — the mnab set — by default), any branch +# GitHub itself reports as protected (covers release branches and +# anything branch-protection-covered beyond the mnab set), or any +# branch with an OPEN pull request — regardless of merge state. +# - Unmerged branches with no open PR and no activity in STALE_DAYS are +# REPORT-ONLY: listed for triage, never touched. +# - DRY_RUN=true (the default) computes and reports every action it WOULD +# take without calling the delete API at all. +# +# Usage: dead-branch-sweep.sh +# Reads all configuration from environment variables (see below) so the +# calling workflow stays a thin wrapper. Prints progress to stderr, writes +# the two triage tables (would-)deleted / stale-unmerged as markdown to +# stdout, and a machine-readable JSON summary to OUTPUT_JSON. +# +# Required env: +# GH_TOKEN - token gh CLI uses; scoped to contents:write + +# pull-requests:read on the target repos. +# Optional env: +# ORG - org login to sweep (default: alfred-intelligence) +# REPOS_OVERRIDE - comma-separated owner/repo list; skips org +# enumeration when set (mainly for local testing) +# DRY_RUN - "true" (default) or "false" +# STALE_DAYS - report threshold for unmerged branches (default 90) +# PROTECTED_BRANCHES - comma-separated names never touched, in addition +# to each repo's own default branch and anything +# GitHub reports as `protected` (default: +# "main,next,before,after" — the mnab set) +# OUTPUT_JSON - path for the JSON summary (default: ./sweep-summary.json) +set -euo pipefail + +: "${GH_TOKEN:?GH_TOKEN is required}" +ORG="${ORG:-alfred-intelligence}" +DRY_RUN="${DRY_RUN:-true}" +STALE_DAYS="${STALE_DAYS:-90}" +PROTECTED_BRANCHES="${PROTECTED_BRANCHES:-main,next,before,after}" +OUTPUT_JSON="${OUTPUT_JSON:-./sweep-summary.json}" + +for bin in gh jq date; do + command -v "$bin" >/dev/null 2>&1 || { + echo "dead-branch-sweep: missing required tool: $bin" >&2 + exit 1 + } +done + +log() { printf '%s\n' "$*" >&2; } + +is_protected_name() { + local branch="$1" name + IFS=',' read -ra names <<<"$PROTECTED_BRANCHES" + for name in "${names[@]}"; do + [[ "$branch" == "$name" ]] && return 0 + done + return 1 +} + +# Resolve the repo list. +repos=() +if [[ -n "${REPOS_OVERRIDE:-}" ]]; then + IFS=',' read -ra repos <<<"$REPOS_OVERRIDE" +else + log "enumerating non-archived repos in org: $ORG" + mapfile -t repos < <( + gh api "orgs/${ORG}/repos" --paginate -q '.[] | select(.archived == false and .disabled == false) | .full_name' + ) +fi +log "repos to sweep: ${#repos[@]}" + +deleted_rows=() +stale_rows=() +now_epoch=$(date -u +%s) + +for repo in "${repos[@]}"; do + owner="${repo%%/*}" + log "== $repo ==" + + default_branch=$(gh api "repos/${repo}" -q '.default_branch') + + mapfile -t branches < <(gh api "repos/${repo}/branches" --paginate -q '.[].name') + + for branch in "${branches[@]}"; do + [[ "$branch" == "$default_branch" ]] && continue + is_protected_name "$branch" && continue + + protected=$(gh api "repos/${repo}/branches/${branch}" -q '.protected') + [[ "$protected" == "true" ]] && continue + + prs_json=$(gh api "repos/${repo}/pulls?head=${owner}:${branch}&state=all" 2>/dev/null || echo '[]') + + has_open_pr=$(jq -r 'any(.[]; .state == "open")' <<<"$prs_json") + [[ "$has_open_pr" == "true" ]] && continue + + has_merged_pr=$(jq -r 'any(.[]; .merged_at != null)' <<<"$prs_json") + + compare_status=$(gh api "repos/${repo}/compare/${default_branch}...${branch}" -q '.status' 2>/dev/null || echo "unknown") + merged_by_commits="false" + [[ "$compare_status" == "identical" || "$compare_status" == "behind" ]] && merged_by_commits="true" + + branch_info=$(gh api "repos/${repo}/branches/${branch}") + last_commit_date=$(jq -r '.commit.commit.committer.date' <<<"$branch_info") + + if [[ "$merged_by_commits" == "true" || "$has_merged_pr" == "true" ]]; then + if [[ "$DRY_RUN" == "true" ]]; then + action="would-delete" + else + action="deleted" + gh api -X DELETE "repos/${repo}/git/refs/heads/${branch}" + fi + log " $action: $branch (merged, last commit $last_commit_date)" + deleted_rows+=("$(jq -nc --arg repo "$repo" --arg branch "$branch" --arg action "$action" \ + --arg last_commit "$last_commit_date" \ + '{repo:$repo, branch:$branch, action:$action, last_commit:$last_commit}')") + continue + fi + + # Unmerged, no open PR — staleness check only. Never deleted. + commit_epoch=$(date -u -d "$last_commit_date" +%s) + age_days=$(((now_epoch - commit_epoch) / 86400)) + if ((age_days > STALE_DAYS)); then + log " stale-unmerged: $branch (${age_days}d since last commit)" + stale_rows+=("$(jq -nc --arg repo "$repo" --arg branch "$branch" --argjson age_days "$age_days" \ + --arg last_commit "$last_commit_date" \ + '{repo:$repo, branch:$branch, age_days:$age_days, last_commit:$last_commit}')") + fi + done +done + +# ---- JSON summary (machine-readable, feeds lumberjack / operator triage) ---- +deleted_json="[]" +if ((${#deleted_rows[@]} > 0)); then + deleted_json=$(printf '%s\n' "${deleted_rows[@]}" | jq -s '.') +fi +stale_json="[]" +if ((${#stale_rows[@]} > 0)); then + stale_json=$(printf '%s\n' "${stale_rows[@]}" | jq -s '.') +fi +jq -n --argjson deleted "$deleted_json" --argjson stale "$stale_json" \ + --arg dry_run "$DRY_RUN" --arg stale_days "$STALE_DAYS" \ + '{dry_run: ($dry_run == "true"), stale_days: ($stale_days | tonumber), merged: $deleted, stale_unmerged: $stale}' \ + >"$OUTPUT_JSON" +log "wrote summary: $OUTPUT_JSON" + +# ---- Markdown tables (job summary) ---- +mode_label="DRY RUN — nothing deleted" +[[ "$DRY_RUN" == "false" ]] && mode_label="ARMED — branches deleted" + +printf '## Dead-branch sweep — %s\n\n' "$mode_label" + +printf '### Merged branches (%s)\n\n' "$([[ "$DRY_RUN" == "true" ]] && echo "would delete" || echo "deleted")" +if ((${#deleted_rows[@]} == 0)); then + printf '_none_\n\n' +else + printf '| repo | branch | last commit |\n|---|---|---|\n' + jq -r '.[] | "| \(.repo) | \(.branch) | \(.last_commit) |"' <<<"$deleted_json" + printf '\n' +fi + +printf '### Stale unmerged branches — report only, never deleted (>%s days idle, no open PR)\n\n' "$STALE_DAYS" +if ((${#stale_rows[@]} == 0)); then + printf '_none_\n' +else + printf '| repo | branch | age (days) | last commit |\n|---|---|---|---|\n' + jq -r '.[] | "| \(.repo) | \(.branch) | \(.age_days) | \(.last_commit) |"' <<<"$stale_json" +fi From ae4c8842baf039c833857074f2c7f3ec9f1cf073 Mon Sep 17 00:00:00 2001 From: alfred-int-bot <273560732+alfred-int-bot@users.noreply.github.com> Date: Mon, 13 Jul 2026 13:09:58 +0200 Subject: [PATCH 2/2] fix(ci): close dead-branch-sweep security review findings (F1/F2/F5) Security review (inquisitor) + evidence pass (referent) found blockers on PR#6 before merge, fixed here: - F1 [CRITICAL] data-loss: has_merged_pr only counted a closed PR's merged_at, matched by branch NAME. GitHub keeps closed-PR records forever, so a reused branch name (force-pushed with new unmerged commits after an earlier PR on that name merged, or deleted+recreated) would still read as merged via the stale record even though compare correctly reported diverged -> genuinely unmerged work could get deleted. Now requires the merged PR's head.sha to equal the branch's current tip before trusting it; a genuine squash/rebase merge of the current tip still matches, since the tip hasn't moved since GitHub recorded it. - F2 [HIGH] query-param injection: branch names can legally contain '&'/'='/'#' (git ref rules don't forbid them) and were spliced raw into `pulls?head=...&state=all`, so a crafted branch name could inject a second head= param and read a different branch's PR history. Switched to `gh api -f head=... -f state=all` (gh encodes the query for us) and percent-encode every branch name used in a REST path segment (branches/, compare/..., git/refs/heads/). - F5 [MED] SHA-pin actions/checkout + actions/upload-artifact in dead-branch-sweep.yml. Repo convention (72f78e9) deliberately leaves first-party actions/* on floating tags; this workflow is an explicit exception because the Sweep step shares a job with BRANCH_SWEEP_TOKEN (org-wide contents:write) -- noted inline in the workflow why this one diverges from the general convention. - Reconciled against current main (referent's regression finding): PR#6's branch predated main's CodeQL "Code injection" fix to mnab-gate.yml (build-command allowlist). Rebased onto origin/main -- git recognised the branch's own mnab-gate.yml/pin-SHA/light-gates-docs commits as already-applied-equivalent patches and dropped them automatically, so the only real conflict was a README.md table-row merge. Diffed clean afterward: dead-branch-sweep's diff against main is now only the three new sweep files, no stale mnab-gate.yml copy. docs/branch-sweep.md: added a second-gate note -- BRANCH_SWEEP_ARMED must also confirm the head.sha-match fix is still present in the script, not just that a DECISIONS.md authorization entry exists (F1 fix must stay landed, per inquisitor O1). Does not merge -- operator's review gate. Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_01JGpSmSMHtG3rP13yq5sU1g --- .github/workflows/dead-branch-sweep.yml | 16 +++++- docs/branch-sweep.md | 25 ++++++++-- scripts/dead-branch-sweep.sh | 65 ++++++++++++++++++++----- 3 files changed, 90 insertions(+), 16 deletions(-) diff --git a/.github/workflows/dead-branch-sweep.yml b/.github/workflows/dead-branch-sweep.yml index 4dc349b..ff7b7c8 100644 --- a/.github/workflows/dead-branch-sweep.yml +++ b/.github/workflows/dead-branch-sweep.yml @@ -28,6 +28,18 @@ name: dead-branch-sweep # the token. It is not an org-universal or cross-owner sweep; GeGGe01/ # SAVANTERNA/kebab-it-owned repos are out of scope by construction (see # docs/branch-sweep.md). +# +# ACTION PINNING NOTE: this repo's established convention (72f78e9) leaves +# GitHub-owned actions (actions/checkout et al) on floating major tags — +# only third-party actions get SHA-pinned — because CodeQL's unpinned-tag +# query itself trusts first-party actions/* actions. This workflow is a +# deliberate exception: actions/checkout and actions/upload-artifact are +# SHA-pinned here even though they're first-party, because the Sweep step +# runs with BRANCH_SWEEP_TOKEN (an org-wide contents:write PAT) in the same +# job — a floating tag on a step that shares a job with that token is a +# bigger blast radius than the convention was written for. Bump the SHA +# deliberately (not by just re-adding @v4) when a new checkout/upload- +# artifact major version is needed. on: schedule: @@ -67,7 +79,7 @@ jobs: exit 1 fi - - uses: actions/checkout@v4 + - uses: actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5 # v4 - name: Resolve dry-run mode id: mode @@ -100,7 +112,7 @@ jobs: run: bash scripts/dead-branch-sweep.sh | tee -a "$GITHUB_STEP_SUMMARY" - name: Upload JSON summary - uses: actions/upload-artifact@v4 + uses: actions/upload-artifact@ea165f8d65b6e75b540449e92b4886f43607fa02 # v4 with: name: dead-branch-sweep-summary path: ${{ runner.temp }}/sweep-summary.json diff --git a/docs/branch-sweep.md b/docs/branch-sweep.md index a8140ef..a4b7ab8 100644 --- a/docs/branch-sweep.md +++ b/docs/branch-sweep.md @@ -13,9 +13,17 @@ something each consumer repo opts into separately. 1. Commit-containment: `compare ...` reports `identical` or `behind` — the branch has no commits the default branch lacks. 2. Merged-PR history: a closed PR exists for that branch head with - `merged_at` set — this is what catches **squash and rebase merges**, - which (1) alone would miss because the base branch gets a *new* commit - SHA, not the branch's original commits. + `merged_at` set **and** that PR's `head.sha` equals the branch's + *current* tip commit — this is what catches **squash and rebase + merges**, which (1) alone would miss because the base branch gets a + *new* commit SHA, not the branch's original commits. The head-SHA match + is load-bearing, not decorative: GitHub keeps closed-PR records + forever, so without it a *reused* branch name (force-pushed with new, + genuinely-unmerged commits after an earlier PR on that name merged, or + deleted and recreated) would still read as merged off the stale PR + record even though commit-containment correctly says diverged — that + was a real data-loss bug (security review, pre-merge) fixed before this + PR shipped. - **Never deletes:** the repo's own default branch, the mnab set (`main`, `next`, `before`, `after`) by name, any branch GitHub's API itself reports as `protected` (covers release branches and anything @@ -102,6 +110,17 @@ that entry** — it ships the mechanism inert (dry-run) and flags that the operator/governance-owner should add a short `DECISIONS.md` post before `BRANCH_SWEEP_ARMED` is ever set to `true` anywhere. +**Second gate, not just the first:** the `DECISIONS.md` entry authorizing +org-wide auto-delete is not the *only* precondition for arming. It must ALSO +confirm the merged-branch decision in `scripts/dead-branch-sweep.sh` still +requires a merged PR's `head.sha` to match the branch's current tip (the +fix described above) before `BRANCH_SWEEP_ARMED` is set anywhere — a future +edit to that script that drops the SHA-match check (e.g. "simplify" back to +`any(.[]; .merged_at != null)`) reopens the reused-branch-name data-loss +path even with a clean DECISIONS.md authorization in place. Whoever reviews +the arming request should diff the live script against this contract, not +just check that a governance entry exists. + ## Local testing The script is standalone and callable outside Actions: diff --git a/scripts/dead-branch-sweep.sh b/scripts/dead-branch-sweep.sh index c79d395..42a4d65 100755 --- a/scripts/dead-branch-sweep.sh +++ b/scripts/dead-branch-sweep.sh @@ -13,9 +13,24 @@ # alone is reliable: # (a) commit-containment via `compare base...head` (status identical # or behind) — catches merge-commit and fast-forward merges. -# (b) a closed PR for that branch head with merged_at set — catches +# (b) a closed PR for that branch head with merged_at set AND +# head.sha == the branch's CURRENT tip commit — catches # squash/rebase merges, where (a) alone would say "diverged" -# even though the PR genuinely merged. +# even though the PR genuinely merged. The head.sha match is +# load-bearing: GitHub keeps closed-PR records forever, so a +# branch NAME that gets reused (force-pushed with new unmerged +# commits after its old PR merged, or deleted+recreated) would +# otherwise still show a "merged" PR for that name even though +# the branch's current content was never merged. Requiring the +# merged PR's head.sha to equal the live branch tip means a +# stale record for a reused name can't fire the OR. +# - Branch names can legally contain `&`, `=`, `#` (git ref rules forbid +# space/`..`/`~`/`^`/`:`/`?`/`*`/`[`/`\`, but not those) — every branch +# name that reaches a GitHub API call is percent-encoded first, and the +# PR-history query uses `gh api -f`/`-f` (which encodes for us) rather +# than hand-spliced query-string interpolation, so a branch named e.g. +# `x&head=owner:other-branch` can't inject a second `head=` param and +# read a different branch's PR history. # - NEVER deletes: the default branch, any name in PROTECTED_BRANCHES # (main/next/before/after — the mnab set — by default), any branch # GitHub itself reports as protected (covers release branches and @@ -64,6 +79,14 @@ done log() { printf '%s\n' "$*" >&2; } +# Percent-encode a single path segment (branch names can contain characters +# that are structurally meaningful in a URL — '&', '=', '#' — even though +# git ref rules forbid the classic shell-hostile set). Used for every branch +# name spliced into a REST path. jq's @uri is RFC 3986 percent-encoding. +urlencode() { + jq -rn --arg s "$1" '$s | @uri' +} + is_protected_name() { local branch="$1" name IFS=',' read -ra names <<<"$PROTECTED_BRANCHES" @@ -101,29 +124,49 @@ for repo in "${repos[@]}"; do [[ "$branch" == "$default_branch" ]] && continue is_protected_name "$branch" && continue - protected=$(gh api "repos/${repo}/branches/${branch}" -q '.protected') + encoded_branch=$(urlencode "$branch") + + # One branches/ fetch serves both the protected flag and the + # current tip SHA / last-commit date used below — avoids a second + # identical call and gives FIX 1 (see below) the live tip to compare + # against before the branches/ call happens a second time. + branch_info=$(gh api "repos/${repo}/branches/${encoded_branch}") + protected=$(jq -r '.protected' <<<"$branch_info") [[ "$protected" == "true" ]] && continue - prs_json=$(gh api "repos/${repo}/pulls?head=${owner}:${branch}&state=all" 2>/dev/null || echo '[]') + current_sha=$(jq -r '.commit.sha' <<<"$branch_info") + last_commit_date=$(jq -r '.commit.commit.committer.date' <<<"$branch_info") + + # gh api -f/-f builds the query string itself (percent-encodes each + # value), so a branch name containing '&'/'='/'#' can't splice in an + # extra query param and redirect this lookup at a different branch's + # PR history. + prs_json=$(gh api -f head="${owner}:${branch}" -f state=all "repos/${repo}/pulls" 2>/dev/null || echo '[]') has_open_pr=$(jq -r 'any(.[]; .state == "open")' <<<"$prs_json") [[ "$has_open_pr" == "true" ]] && continue - has_merged_pr=$(jq -r 'any(.[]; .merged_at != null)' <<<"$prs_json") - - compare_status=$(gh api "repos/${repo}/compare/${default_branch}...${branch}" -q '.status' 2>/dev/null || echo "unknown") + # A closed PR's merged_at record is only trusted when its head.sha + # matches the branch's CURRENT tip. Without this, a reused branch name + # (force-pushed with new unmerged commits after an earlier merge, or + # deleted+recreated) would still read as "merged" off the stale PR + # record even though compare correctly reports diverged — this is the + # data-loss path this check exists to close. A genuine squash/rebase + # merge of the branch's current tip still matches, since the tip hasn't + # moved since GitHub recorded the merge. + has_merged_pr=$(jq -r --arg sha "$current_sha" \ + 'any(.[]; .merged_at != null and .head.sha == $sha)' <<<"$prs_json") + + compare_status=$(gh api "repos/${repo}/compare/$(urlencode "$default_branch")...${encoded_branch}" -q '.status' 2>/dev/null || echo "unknown") merged_by_commits="false" [[ "$compare_status" == "identical" || "$compare_status" == "behind" ]] && merged_by_commits="true" - branch_info=$(gh api "repos/${repo}/branches/${branch}") - last_commit_date=$(jq -r '.commit.commit.committer.date' <<<"$branch_info") - if [[ "$merged_by_commits" == "true" || "$has_merged_pr" == "true" ]]; then if [[ "$DRY_RUN" == "true" ]]; then action="would-delete" else action="deleted" - gh api -X DELETE "repos/${repo}/git/refs/heads/${branch}" + gh api -X DELETE "repos/${repo}/git/refs/heads/${encoded_branch}" fi log " $action: $branch (merged, last commit $last_commit_date)" deleted_rows+=("$(jq -nc --arg repo "$repo" --arg branch "$branch" --arg action "$action" \