From fe1f9216b57c7e16637d4795b5c6e8fd9e599477 Mon Sep 17 00:00:00 2001 From: Sebastian Mendel Date: Sat, 23 May 2026 07:37:31 +0200 Subject: [PATCH 1/8] feat(scripts): add init-branch-protection.sh for new-repo bootstrap MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add a runnable script + JSON template that operators invoke immediately after `gh repo create`, before pushing the first commit or opening the first PR. The script applies the Netresearch baseline branch protection: - required_conversation_resolution: true (load-bearing) - required_approving_review_count: 1 - allow_force_pushes: false - allow_deletions: false - required_linear_history: false The template deliberately omits required_signatures (so the script never resets a repo that has already enabled signing) and ships enforce_admins as false to accommodate solo-maintainer repos. Operators tighten both per-repo via the documented one-liners in the script header. Two-step flow: 1. bash init-branch-protection.sh OWNER/REPO (applies baseline; no required status checks yet) 2. bash init-branch-protection.sh OWNER/REPO --from-current-checks (after first CI run completes, captures check-run names as required contexts with strict=true) Idempotent: a second run on an already-compliant repo reports drift on opinionated fields only (or 'already compliant') and exits 0. Live-tested against netresearch/snipe-it-docker-compose-stack — correctly reports already-compliant. shellcheck + bash -n clean. Closes the enforcement gap demonstrated in netresearch/snipe-it-docker-compose-stack#17 where 3 of 8 merged PRs shipped with unresolved bot-reviewer threads (including a HIGH-severity token-leak flagged by both Copilot and gemini-code-assist) because branch protection was never applied at repo creation time. Signed-off-by: Sebastian Mendel --- .../assets/branch-protection.json.template | 17 ++ .../scripts/init-branch-protection.sh | 286 ++++++++++++++++++ 2 files changed, 303 insertions(+) create mode 100644 skills/github-project/assets/branch-protection.json.template create mode 100755 skills/github-project/scripts/init-branch-protection.sh diff --git a/skills/github-project/assets/branch-protection.json.template b/skills/github-project/assets/branch-protection.json.template new file mode 100644 index 0000000..e880e0f --- /dev/null +++ b/skills/github-project/assets/branch-protection.json.template @@ -0,0 +1,17 @@ +{ + "required_status_checks": null, + "enforce_admins": false, + "required_pull_request_reviews": { + "required_approving_review_count": 1, + "dismiss_stale_reviews": false, + "require_code_owner_reviews": false, + "require_last_push_approval": false + }, + "restrictions": null, + "required_linear_history": false, + "allow_force_pushes": false, + "allow_deletions": false, + "required_conversation_resolution": true, + "lock_branch": false, + "allow_fork_syncing": false +} diff --git a/skills/github-project/scripts/init-branch-protection.sh b/skills/github-project/scripts/init-branch-protection.sh new file mode 100755 index 0000000..08eaa8a --- /dev/null +++ b/skills/github-project/scripts/init-branch-protection.sh @@ -0,0 +1,286 @@ +#!/usr/bin/env bash +# init-branch-protection.sh +# Apply Netresearch standard branch protection to a GitHub repository. +# +# This is the REQUIRED first step after `gh repo create`, before pushing the +# first commit or opening the first PR. The structural enforcement applied +# here (required_conversation_resolution + min-1-approver) is what makes the +# unresolved-threads workflow rule actually safe — relying only on operator +# discipline has demonstrably failed (see netresearch/snipe-it-docker-compose-stack#17). +# +# Usage: +# bash init-branch-protection.sh / +# Apply baseline protection (no required status checks yet — for new +# repos with no CI history). Idempotent: a second run on an already- +# compliant repo reports drift (or "already compliant") and exits 0. +# +# bash init-branch-protection.sh / --from-current-checks +# Follow-up after the first successful CI run. Reads the check-run names +# from the most recent completed workflow run on the default branch and +# PATCHes them in as required status contexts with strict=true. +# +# Baseline applied (see assets/branch-protection.json.template): +# required_conversation_resolution: true <- the load-bearing field +# required_approving_review_count: 1 +# allow_force_pushes: false +# allow_deletions: false +# required_linear_history: false (must be false for merge-commit +# strategy needed by signed commits) +# +# Deliberately NOT in the template (per Netresearch org policy 2026-05): +# enforce_admins: shipped as false; tighten per-repo via: +# gh api repos/OWNER/REPO/branches//protection/enforce_admins -X POST +# required_signatures: omitted entirely (not set to false) so this script +# never resets a repo that has already enabled signing. Tighten per-repo: +# gh api repos/OWNER/REPO/branches//protection/required_signatures -X POST +# +# Exit codes: +# 0 - applied or already compliant +# 1 - drift detected (reported, not auto-corrected on second run) +# 2 - invalid arguments / template missing +# 3 - repo not found or no access +# 4 - default branch does not yet exist (empty repo — push initial commit first) +# 5 - --from-current-checks: no completed workflow run found on default branch +# +# SPDX-License-Identifier: MIT +# Copyright (c) Netresearch DTT GmbH + +set -euo pipefail + +# ---------- output helpers ---------- +RED=$'\033[0;31m' +GREEN=$'\033[0;32m' +YELLOW=$'\033[1;33m' +BLUE=$'\033[0;34m' +NC=$'\033[0m' + +err() { printf '%s\n' "${RED}error:${NC} $*" >&2; } +warn() { printf '%s\n' "${YELLOW}warn:${NC} $*" >&2; } +info() { printf '%s\n' "${BLUE}info:${NC} $*" >&2; } +ok() { printf '%s\n' "${GREEN}ok:${NC} $*" >&2; } + +usage() { + cat >&2 <<'EOF' +Usage: + init-branch-protection.sh / + init-branch-protection.sh / --from-current-checks + +See script header comment for full documentation. +EOF + exit 2 +} + +# ---------- arg parsing ---------- +[[ $# -ge 1 && $# -le 2 ]] || usage +[[ "${1:-}" == "-h" || "${1:-}" == "--help" ]] && usage + +SLUG="$1" +MODE="${2:-apply}" + +if [[ "$MODE" != "apply" && "$MODE" != "--from-current-checks" ]]; then + err "unknown second argument: $MODE" + usage +fi + +if [[ ! "$SLUG" =~ ^[A-Za-z0-9_.-]+/[A-Za-z0-9_.-]+$ ]]; then + err "expected /, got: $SLUG" + exit 2 +fi + +OWNER="${SLUG%/*}" +REPO="${SLUG#*/}" + +# ---------- locate template ---------- +# Script lives at skills/github-project/scripts/init-branch-protection.sh +# Template lives at skills/github-project/assets/branch-protection.json.template +SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" +TEMPLATE="${SCRIPT_DIR}/../assets/branch-protection.json.template" + +if [[ ! -f "$TEMPLATE" ]]; then + err "template not found at: $TEMPLATE" + exit 2 +fi + +# ---------- prerequisites ---------- +command -v gh >/dev/null 2>&1 || { err "gh CLI not installed"; exit 2; } +command -v jq >/dev/null 2>&1 || { err "jq not installed"; exit 2; } + +# ---------- discover default branch ---------- +info "fetching repo metadata for $SLUG ..." +REPO_JSON="$(gh api "repos/$OWNER/$REPO" 2>&1)" || { + err "cannot access repo $SLUG — not found or no permission" + printf '%s\n' "$REPO_JSON" >&2 + exit 3 +} + +DEFAULT_BRANCH="$(jq -r '.default_branch' <<<"$REPO_JSON")" +if [[ -z "$DEFAULT_BRANCH" || "$DEFAULT_BRANCH" == "null" ]]; then + err "could not determine default branch for $SLUG" + exit 4 +fi +info "default branch: $DEFAULT_BRANCH" + +# Verify the default branch actually exists (an empty repo has a `default_branch` +# field set, but the ref does not exist yet — protection PUT would fail with 404). +if ! gh api "repos/$OWNER/$REPO/branches/$DEFAULT_BRANCH" --silent 2>/dev/null; then + err "default branch '$DEFAULT_BRANCH' does not exist yet (empty repo)" + err "push an initial commit first, then re-run this script." + exit 4 +fi + +PROTECTION_URL="repos/$OWNER/$REPO/branches/$DEFAULT_BRANCH/protection" + +# ---------- --from-current-checks mode ---------- +if [[ "$MODE" == "--from-current-checks" ]]; then + info "discovering required status checks from latest workflow run on $DEFAULT_BRANCH ..." + + # Find the most recent completed run on the default branch. + RUN_ID="$(gh api \ + "repos/$OWNER/$REPO/actions/runs?branch=$DEFAULT_BRANCH&status=completed&per_page=1" \ + --jq '.workflow_runs[0].id // empty' 2>/dev/null || true)" + + if [[ -z "$RUN_ID" ]]; then + err "no completed workflow run found on $DEFAULT_BRANCH" + err "trigger and complete at least one CI run, then re-run with --from-current-checks." + exit 5 + fi + info "using run id: $RUN_ID" + + # Collect successful check-run names from that run. + mapfile -t CHECK_NAMES < <(gh api --paginate \ + "repos/$OWNER/$REPO/actions/runs/$RUN_ID/jobs" \ + --jq '.jobs[] | select(.conclusion == "success") | .name') + + if [[ ${#CHECK_NAMES[@]} -eq 0 ]]; then + err "no successful jobs found in latest run; nothing to require." + exit 5 + fi + + info "discovered ${#CHECK_NAMES[@]} required check(s):" + for n in "${CHECK_NAMES[@]}"; do printf ' - %s\n' "$n" >&2; done + + # Build a JSON body that updates only required_status_checks. We PATCH the + # protection endpoint by re-PUTting the merged document: GitHub's API for + # branch protection does not accept partial bodies, so we read existing + # protection, splice the new contexts in, and PUT back. + EXISTING="$(gh api "$PROTECTION_URL" 2>/dev/null || echo '{}')" + + # If protection has not been initialized yet, fall back to the template. + if [[ "$EXISTING" == "{}" ]] || [[ -z "$(jq -r '.url // empty' <<<"$EXISTING")" ]]; then + warn "no existing protection — applying baseline first then adding checks" + EXISTING="$(cat "$TEMPLATE")" + fi + + # Normalize the existing protection into a PUT-compatible body (the GET + # response includes embedded `url` fields and nested envelopes that the + # PUT endpoint rejects). + PUT_BODY="$(jq \ + --argjson checks "$(printf '%s\n' "${CHECK_NAMES[@]}" | jq -R . | jq -s .)" \ + '{ + required_status_checks: { + strict: true, + contexts: $checks + }, + enforce_admins: (.enforce_admins.enabled // false), + required_pull_request_reviews: ( + if .required_pull_request_reviews then { + required_approving_review_count: (.required_pull_request_reviews.required_approving_review_count // 1), + dismiss_stale_reviews: (.required_pull_request_reviews.dismiss_stale_reviews // false), + require_code_owner_reviews: (.required_pull_request_reviews.require_code_owner_reviews // false), + require_last_push_approval: (.required_pull_request_reviews.require_last_push_approval // false) + } else { + required_approving_review_count: 1, + dismiss_stale_reviews: false, + require_code_owner_reviews: false, + require_last_push_approval: false + } end + ), + restrictions: null, + required_linear_history: (.required_linear_history.enabled // false), + allow_force_pushes: (.allow_force_pushes.enabled // false), + allow_deletions: (.allow_deletions.enabled // false), + required_conversation_resolution: (.required_conversation_resolution.enabled // true), + lock_branch: (.lock_branch.enabled // false), + allow_fork_syncing: (.allow_fork_syncing.enabled // false) + }' <<<"$EXISTING")" + + info "PUT $PROTECTION_URL (with required checks)" + if RESP="$(gh api -X PUT "$PROTECTION_URL" --input - <<<"$PUT_BODY" 2>&1)"; then + ok "required status checks applied (${#CHECK_NAMES[@]} contexts, strict=true)" + exit 0 + else + err "PUT failed:" + printf '%s\n' "$RESP" >&2 + exit 1 + fi +fi + +# ---------- apply mode ---------- +TEMPLATE_BODY="$(cat "$TEMPLATE")" + +# Check whether protection already exists. +EXISTING="$(gh api "$PROTECTION_URL" 2>/dev/null || echo '')" + +if [[ -n "$EXISTING" ]] && [[ -n "$(jq -r '.url // empty' <<<"$EXISTING" 2>/dev/null)" ]]; then + info "protection already exists — checking for drift against template baseline" + + # Compare the load-bearing fields from the template against current state. + # We only flag drift on fields the template OPINIONATES on; fields the + # template intentionally omits (e.g. required_signatures) are out of scope. + DRIFT="" + check_field() { + local label="$1" expected="$2" actual="$3" + if [[ "$expected" != "$actual" ]]; then + DRIFT+=" ${label}: expected=${expected} actual=${actual}"$'\n' + fi + } + + EXP_RCR="$(jq -r '.required_conversation_resolution' <<<"$TEMPLATE_BODY")" + ACT_RCR="$(jq -r '.required_conversation_resolution.enabled // false' <<<"$EXISTING")" + check_field "required_conversation_resolution" "$EXP_RCR" "$ACT_RCR" + + EXP_APR="$(jq -r '.required_pull_request_reviews.required_approving_review_count' <<<"$TEMPLATE_BODY")" + ACT_APR="$(jq -r '.required_pull_request_reviews.required_approving_review_count // 0' <<<"$EXISTING")" + check_field "required_approving_review_count" "$EXP_APR" "$ACT_APR" + + EXP_AFP="$(jq -r '.allow_force_pushes' <<<"$TEMPLATE_BODY")" + ACT_AFP="$(jq -r '.allow_force_pushes.enabled // false' <<<"$EXISTING")" + check_field "allow_force_pushes" "$EXP_AFP" "$ACT_AFP" + + EXP_AD="$(jq -r '.allow_deletions' <<<"$TEMPLATE_BODY")" + ACT_AD="$(jq -r '.allow_deletions.enabled // false' <<<"$EXISTING")" + check_field "allow_deletions" "$EXP_AD" "$ACT_AD" + + EXP_LH="$(jq -r '.required_linear_history' <<<"$TEMPLATE_BODY")" + ACT_LH="$(jq -r '.required_linear_history.enabled // false' <<<"$EXISTING")" + check_field "required_linear_history" "$EXP_LH" "$ACT_LH" + + if [[ -z "$DRIFT" ]]; then + ok "$SLUG already compliant with template baseline (no drift on opinionated fields)" + exit 0 + fi + + warn "drift detected vs template baseline:" + printf '%s' "$DRIFT" >&2 + warn "not auto-correcting — re-run without --from-current-checks intentionally," + warn "or PATCH specific fields by hand. Aborting to avoid clobbering admin choices." + exit 1 +fi + +# No protection yet — apply the template. +info "no existing protection on $DEFAULT_BRANCH — applying template" +if RESP="$(gh api -X PUT "$PROTECTION_URL" --input - <<<"$TEMPLATE_BODY" 2>&1)"; then + ok "branch protection applied to $SLUG on $DEFAULT_BRANCH" + ok "required_conversation_resolution: true" + ok "required_approving_review_count: 1" + info "next steps:" + info " 1. push at least one CI run on $DEFAULT_BRANCH" + info " 2. re-run with --from-current-checks to capture required status checks" + info " 3. (optional) enforce admins: gh api $PROTECTION_URL/enforce_admins -X POST" + info " 4. (optional) require signed commits: gh api $PROTECTION_URL/required_signatures -X POST" + exit 0 +else + err "PUT failed:" + printf '%s\n' "$RESP" >&2 + exit 1 +fi From e1b722272690464e34fbd8ef8baa10f92edb67b9 Mon Sep 17 00:00:00 2001 From: Sebastian Mendel Date: Sat, 23 May 2026 07:38:43 +0200 Subject: [PATCH 2/8] docs(skill): require branch-protection init step before first PR/commit MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Promote the new init-branch-protection.sh to a first-class surface in the skill: - 'When to Use' now leads with 'After gh repo create, before first commit/PR' (REQUIRED). - New 'Required First Step After gh repo create' section documents the two-step flow (apply baseline, then --from-current-checks after first CI run), links the snipe-it#17 incident as the pain that triggered this, and points at /assess github-project's GH-31 checkpoint for read-only verification. - 'Security & Compliance Quick Checks' gets a one-liner that prints whether required_conversation_resolution is enabled on the repo's default branch. - 'Running Scripts' lists init-branch-protection.sh alongside the existing verify-github-project.sh. The structural enforcement (required_conversation_resolution=true plus min-1-approver) is what makes the 'abort merge if unresolved threads' memory rule actually safe. Documentation without a runnable apply path demonstrably wasn't enough — operator discipline failed in snipe-it#17. Signed-off-by: Sebastian Mendel --- skills/github-project/SKILL.md | 36 ++++++++++++++++++++++++++++++++++ 1 file changed, 36 insertions(+) diff --git a/skills/github-project/SKILL.md b/skills/github-project/SKILL.md index 8be4594..b3c3b45 100644 --- a/skills/github-project/SKILL.md +++ b/skills/github-project/SKILL.md @@ -16,6 +16,7 @@ GitHub repository configuration, troubleshooting, and collaboration workflow bes ## When to Use +- **After `gh repo create`, before first commit/PR** — apply branch protection (REQUIRED, see below) - PR won't merge, BLOCKED, or unresolved threads - Auto-merge fails for Dependabot/Renovate - Solo maintainer needs auto-approve @@ -27,6 +28,30 @@ GitHub repository configuration, troubleshooting, and collaboration workflow bes - CODEOWNERS, issue/PR templates, release labels - Fork PR merge base (too many commits) +## Required First Step After `gh repo create` + +After creating any new Netresearch repository, **before pushing the first commit or opening the first PR**, you MUST apply branch protection. Without this, the unresolved-threads workflow rule is unenforceable — operator discipline alone has demonstrably failed (see [snipe-it-docker-compose-stack#17](https://github.com/netresearch/snipe-it-docker-compose-stack/pull/17): 3 of 8 merged PRs shipped unresolved bot-reviewer threads, including a HIGH-severity token leak that both Copilot and gemini-code-assist had flagged). + +Two-step flow: + +```bash +# 1. Immediately after `gh repo create` and the first push: +bash /skills/github-project/scripts/init-branch-protection.sh OWNER/REPO +# Applies: required_conversation_resolution=true, 1 approver, +# no force-push, no deletions, no required checks yet. + +# 2. After the first CI run completes on the default branch: +bash /skills/github-project/scripts/init-branch-protection.sh OWNER/REPO --from-current-checks +# Captures the now-known check-run names as required status contexts +# with strict=true. +``` + +The script is idempotent: re-running on an already-compliant repo reports `already compliant` and exits 0. Drift on opinionated fields exits 1 with a per-field diff (no silent clobber of admin choices). + +Baseline applied is intentionally minimal — `enforce_admins` and `required_signatures` are NOT in the template (per-repo decision; tighten via the one-liners in the script header). The load-bearing field is `required_conversation_resolution: true`, which is what makes the "abort merge if unresolved threads" memory rule structurally safe rather than discipline-dependent. + +You can also invoke `/assess github-project` against a repo to verify (read-only) that the baseline is in place — checkpoint `GH-31` fails with `severity: error` if `required_conversation_resolution` is not enabled. + ## Quick Diagnostics ### PR Won't Merge @@ -70,6 +95,11 @@ gh run rerun RUN_ID --repo OWNER/REPO ### Security & Compliance Quick Checks ```bash +# REQUIRED baseline (one-liner): is required_conversation_resolution on? +gh api repos/OWNER/REPO/branches/$(gh api repos/OWNER/REPO --jq .default_branch)/protection \ + --jq '.required_conversation_resolution.enabled // false' +# If false, run scripts/init-branch-protection.sh OWNER/REPO + gh api repos/OWNER/REPO/branches/main/protection --jq '.enforce_admins.enabled' gh api repos/OWNER/REPO/code-scanning/default-setup --jq '.state' gh api graphql -f query='query($owner:String!,$repo:String!,$pr:Int!){ @@ -86,6 +116,12 @@ See `references/auto-merge-guide.md` for: rebase-merge-with-signed-commits fixes ## Running Scripts ```bash +# Apply baseline branch protection to a new repo (REQUIRED post-`gh repo create`) +scripts/init-branch-protection.sh OWNER/REPO +# After first CI run completes: +scripts/init-branch-protection.sh OWNER/REPO --from-current-checks + +# Audit an existing local checkout against GitHub-platform best practices scripts/verify-github-project.sh /path/to/repository ``` From 6c68991f1c88f4b92c1b2657e0bbd6bc3d0cd6d2 Mon Sep 17 00:00:00 2001 From: Sebastian Mendel Date: Sat, 23 May 2026 07:39:19 +0200 Subject: [PATCH 3/8] feat(checkpoints): clarify required_conversation_resolution failure mode in GH-31 Severity was already 'error' (verified). This expands the desc to name the concrete user-visible failure mode that motivated promoting this to a hard checkpoint in the first place: - PR merges may silently include unresolved bot-reviewer feedback - Including security findings like token leakage in build logs - Concrete incident: netresearch/snipe-it-docker-compose-stack#17 The desc now also points operators at the new init script as the apply path, so /assess findings have an actionable remediation embedded in the failure message rather than a generic 'enable it'. Signed-off-by: Sebastian Mendel --- skills/github-project/checkpoints.yaml | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/skills/github-project/checkpoints.yaml b/skills/github-project/checkpoints.yaml index e73ef30..18722c1 100644 --- a/skills/github-project/checkpoints.yaml +++ b/skills/github-project/checkpoints.yaml @@ -344,10 +344,16 @@ mechanical: json_path: ".required_conversation_resolution.enabled" severity: error desc: >- - required_conversation_resolution must be enabled — combined with - enforce_admins, ensures unresolved review threads block ALL merges - including admins (audited by GH-32 llm_review; runner skips because - gh API access requires auth). + required_conversation_resolution must be enabled. Without it, PR + merges may silently include unresolved bot-reviewer feedback — + including security issues like token leakage in build logs (see + netresearch/snipe-it-docker-compose-stack#17 where this gap let a + HIGH-severity Copilot/gemini-code-assist finding ship to main). + Apply via skills/github-project/scripts/init-branch-protection.sh + OWNER/REPO immediately after `gh repo create`. Combined with + enforce_admins, also ensures unresolved threads block ALL merges + including admins (audited by GH-32 llm_review; runner skips + because gh API access requires auth). llm_reviews: # === BRANCH PROTECTION + MERGE QUEUE COMPATIBILITY === From da0b180cc971c1765ca1716cd20ef702bea1df5e Mon Sep 17 00:00:00 2001 From: Sebastian Mendel Date: Sat, 23 May 2026 07:39:51 +0200 Subject: [PATCH 4/8] docs(readme): announce required init step at top of README 5-line callout at the top of the repo README pointing operators at scripts/init-branch-protection.sh. The full two-step flow + rationale lives in skills/github-project/SKILL.md; this is just the discovery surface for someone landing on the repo page. Signed-off-by: Sebastian Mendel --- README.md | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/README.md b/README.md index 7d7a5df..8db0461 100644 --- a/README.md +++ b/README.md @@ -7,6 +7,13 @@ description: "GitHub repository setup and platform-specific features. This skill GitHub platform configuration and repository management patterns. This skill focuses exclusively on **GitHub-specific features**. +> **New repo? Run this BEFORE the first PR:** +> `bash skills/github-project/scripts/init-branch-protection.sh OWNER/REPO` +> Applies `required_conversation_resolution: true` + 1-approver baseline so the +> "abort merge if unresolved threads" rule is structurally enforced, not just +> documented. See `skills/github-project/SKILL.md` → *Required First Step +> After `gh repo create`* for the two-step flow. + ## 🔌 Compatibility This is an **Agent Skill** following the [open standard](https://agentskills.io) originally developed by Anthropic and released for cross-platform use. From dc7f8522070a6f2ab29c32cda090d7d90c12748f Mon Sep 17 00:00:00 2001 From: Sebastian Mendel Date: Sat, 23 May 2026 07:43:17 +0200 Subject: [PATCH 5/8] fix(scripts): use check-runs API (not jobs API) for --from-current-checks GitHub's required_status_checks.contexts are matched against the check-run *display name*, which includes the 'workflow / job' prefix for matrix and called-workflow jobs (e.g. 'container-lint / hadolint'). The /actions/runs/{id}/jobs endpoint returns just the bare job name ('hadolint'), which would never match. Live-verified against netresearch/snipe-it-docker-compose-stack: the check-runs API returns names that align character-for-character with that repo's existing required_status_checks.contexts list (10 of 12 contexts present in the latest run on main; the other 2 are conditionally-triggered build matrix jobs not exercised in that run). Switched source to: GET /repos/{owner}/{repo}/commits/{default_branch}/check-runs ?per_page=100 (paginated, deduped) Caught by review before any live --from-current-checks invocation. Signed-off-by: Sebastian Mendel --- .../scripts/init-branch-protection.sh | 36 +++++++++++-------- 1 file changed, 21 insertions(+), 15 deletions(-) diff --git a/skills/github-project/scripts/init-branch-protection.sh b/skills/github-project/scripts/init-branch-protection.sh index 08eaa8a..a2c9d62 100755 --- a/skills/github-project/scripts/init-branch-protection.sh +++ b/skills/github-project/scripts/init-branch-protection.sh @@ -132,27 +132,33 @@ PROTECTION_URL="repos/$OWNER/$REPO/branches/$DEFAULT_BRANCH/protection" # ---------- --from-current-checks mode ---------- if [[ "$MODE" == "--from-current-checks" ]]; then - info "discovering required status checks from latest workflow run on $DEFAULT_BRANCH ..." - - # Find the most recent completed run on the default branch. - RUN_ID="$(gh api \ - "repos/$OWNER/$REPO/actions/runs?branch=$DEFAULT_BRANCH&status=completed&per_page=1" \ - --jq '.workflow_runs[0].id // empty' 2>/dev/null || true)" - - if [[ -z "$RUN_ID" ]]; then - err "no completed workflow run found on $DEFAULT_BRANCH" - err "trigger and complete at least one CI run, then re-run with --from-current-checks." + info "discovering required status checks from latest commit on $DEFAULT_BRANCH ..." + + # GitHub's required_status_checks.contexts are matched against the + # check-run *display name* (which includes the "workflow / job" prefix + # for matrix and called-workflow jobs, e.g. "container-lint / hadolint"). + # The /actions/runs/{id}/jobs endpoint returns the bare job name + # ("hadolint") — wrong for context matching. We use /commits/{sha}/check-runs + # against the default branch's HEAD which returns the canonical + # check-run names that align with what shows up in the PR check UI and + # what GitHub compares against required_status_checks.contexts. + HEAD_SHA="$(gh api "repos/$OWNER/$REPO/commits/$DEFAULT_BRANCH" --jq '.sha // empty' 2>/dev/null || true)" + if [[ -z "$HEAD_SHA" ]]; then + err "could not resolve HEAD sha of $DEFAULT_BRANCH" exit 5 fi - info "using run id: $RUN_ID" + info "using $DEFAULT_BRANCH @ ${HEAD_SHA:0:8}" - # Collect successful check-run names from that run. + # Collect successful check-run names for that commit, deduped. mapfile -t CHECK_NAMES < <(gh api --paginate \ - "repos/$OWNER/$REPO/actions/runs/$RUN_ID/jobs" \ - --jq '.jobs[] | select(.conclusion == "success") | .name') + "repos/$OWNER/$REPO/commits/$HEAD_SHA/check-runs?per_page=100" \ + --jq '.check_runs[] | select(.conclusion == "success") | .name' \ + | sort -u) if [[ ${#CHECK_NAMES[@]} -eq 0 ]]; then - err "no successful jobs found in latest run; nothing to require." + err "no successful check-runs found on $DEFAULT_BRANCH @ ${HEAD_SHA:0:8}" + err "trigger and complete at least one CI run on the default branch," + err "then re-run with --from-current-checks." exit 5 fi From 3654b0b3b511c81c32a005f3f9a3ee5d7f780333 Mon Sep 17 00:00:00 2001 From: Sebastian Mendel Date: Sat, 23 May 2026 07:49:25 +0200 Subject: [PATCH 6/8] fix(skill): keep SKILL.md under 500-word cap; extract bootstrap to reference CI's Skill Validation enforces a hard 500-word cap on SKILL.md (see netresearch/skill-repo-skill validate-skill.sh:101-107). The docs(skill) commit pushed it to 810 words and CI rejected. Resolution: - New skills/github-project/references/repo-bootstrap.md captures the full two-step flow, per-knob rationale (why enforce_admins and required_signatures are intentionally NOT in the template), verification commands, exit-code table, and the not-closed pending-reviewer gap acknowledgement. - SKILL.md keeps a single one-line REQUIRED-callout right above Quick Diagnostics that points at the script + the new reference. - Compressed several dense GraphQL blocks in Quick Diagnostics to gh-pr-view equivalents (semantically identical, far fewer words). - Tightened references table column-2 descriptions. - Slimmed front-matter description while preserving the 'Use when' prefix the validator requires (validate-skill.sh:89). Net: 496 words (was 500 on origin/main; was 810 after the docs commit this fix resolves). All other CI checks were already green; this brings Skill Validation green too. Signed-off-by: Sebastian Mendel --- skills/github-project/SKILL.md | 110 ++++++------------ .../references/repo-bootstrap.md | 67 +++++++++++ 2 files changed, 102 insertions(+), 75 deletions(-) create mode 100644 skills/github-project/references/repo-bootstrap.md diff --git a/skills/github-project/SKILL.md b/skills/github-project/SKILL.md index b3c3b45..1f5d91f 100644 --- a/skills/github-project/SKILL.md +++ b/skills/github-project/SKILL.md @@ -1,6 +1,6 @@ --- name: github-project -description: "Use when PRs won't merge or show BLOCKED (Copilot-review race), AI reviewer pushback, auto-approve/auto-merge fails for Dependabot/Renovate, branch protection/rulesets need configuring, CI fails, authoring reusable workflows or composite actions, harden-runner setup, or CODEOWNERS / PR templates." +description: "Use when bootstrapping a repo (apply branch protection before first PR), PRs won't merge or BLOCKED, AI reviewer pushback, auto-merge fails for Dependabot/Renovate, branch protection or rulesets, CI fails, authoring reusable workflows, harden-runner, or CODEOWNERS/PR templates." license: "(MIT AND CC-BY-SA-4.0). See LICENSE-MIT and LICENSE-CC-BY-SA-4.0" compatibility: "Requires gh CLI, git." metadata: @@ -16,53 +16,27 @@ GitHub repository configuration, troubleshooting, and collaboration workflow bes ## When to Use -- **After `gh repo create`, before first commit/PR** — apply branch protection (REQUIRED, see below) +- **Post `gh repo create`, before first commit/PR** — apply branch protection (REQUIRED, see below) - PR won't merge, BLOCKED, or unresolved threads - Auto-merge fails for Dependabot/Renovate -- Solo maintainer needs auto-approve +- Solo maintainer auto-approve - Branch protection, rulesets, `enforce_admins` - GHA failures or permission issues - Signed commit merge (rebase can't auto-sign) - CodeQL default vs custom workflows -- OpenSSF Scorecard (token perms, pinned deps) -- CODEOWNERS, issue/PR templates, release labels -- Fork PR merge base (too many commits) +- Scorecard (token perms, pinned deps) +- CODEOWNERS, templates, release labels +- Fork PR merge base -## Required First Step After `gh repo create` - -After creating any new Netresearch repository, **before pushing the first commit or opening the first PR**, you MUST apply branch protection. Without this, the unresolved-threads workflow rule is unenforceable — operator discipline alone has demonstrably failed (see [snipe-it-docker-compose-stack#17](https://github.com/netresearch/snipe-it-docker-compose-stack/pull/17): 3 of 8 merged PRs shipped unresolved bot-reviewer threads, including a HIGH-severity token leak that both Copilot and gemini-code-assist had flagged). - -Two-step flow: - -```bash -# 1. Immediately after `gh repo create` and the first push: -bash /skills/github-project/scripts/init-branch-protection.sh OWNER/REPO -# Applies: required_conversation_resolution=true, 1 approver, -# no force-push, no deletions, no required checks yet. - -# 2. After the first CI run completes on the default branch: -bash /skills/github-project/scripts/init-branch-protection.sh OWNER/REPO --from-current-checks -# Captures the now-known check-run names as required status contexts -# with strict=true. -``` - -The script is idempotent: re-running on an already-compliant repo reports `already compliant` and exits 0. Drift on opinionated fields exits 1 with a per-field diff (no silent clobber of admin choices). - -Baseline applied is intentionally minimal — `enforce_admins` and `required_signatures` are NOT in the template (per-repo decision; tighten via the one-liners in the script header). The load-bearing field is `required_conversation_resolution: true`, which is what makes the "abort merge if unresolved threads" memory rule structurally safe rather than discipline-dependent. - -You can also invoke `/assess github-project` against a repo to verify (read-only) that the baseline is in place — checkpoint `GH-31` fails with `severity: error` if `required_conversation_resolution` is not enabled. +> **REQUIRED post `gh repo create`:** `scripts/init-branch-protection.sh OWNER/REPO` — see `references/repo-bootstrap.md` (closes [snipe-it#17](https://github.com/netresearch/snipe-it-docker-compose-stack/pull/17) class). ## Quick Diagnostics ### PR Won't Merge ```bash -gh api graphql -f query='query($owner:String!,$repo:String!,$pr:Int!){ - repository(owner:$owner,name:$repo){pullRequest(number:$pr){ - mergeStateStatus reviewDecision mergeable - reviewThreads(first:100){nodes{isResolved comments(first:1){nodes{body}}}} - }} -}' -f owner=OWNER -f repo=REPO -F pr=NUMBER --jq '.data.repository.pullRequest' +gh pr view PR --repo OWNER/REPO \ + --json mergeStateStatus,reviewDecision,mergeable,reviewThreads ``` ### Solo Maintainer: PRs Stuck on REVIEW_REQUIRED @@ -71,15 +45,12 @@ Use `assets/pr-quality.yml.template` for auto-approve with `required_approving_r ### Auto-merge Setup -Requirements: `allow_auto_merge` on repo, `pull_request_target` trigger (not `pull_request`), check `user.login` (not `github.actor`), `gh pr merge --auto` with dynamic strategy. +Requires `allow_auto_merge`, `pull_request_target` trigger, `user.login` bot detection, `gh pr merge --auto` with dynamic strategy. See `references/auto-merge-guide.md`. ### Auto-merge Not Working ```bash -gh api graphql -f query='query{repository(owner:"OWNER",name:"REPO"){ - pullRequest(number:PR){autoMergeRequest{enabledBy{login}}} -}}' --jq '.data.repository.pullRequest.autoMergeRequest' - +gh pr view PR --repo OWNER/REPO --json autoMergeRequest --jq .autoMergeRequest gh api repos/OWNER/REPO/branches/main/protection/required_pull_request_reviews \ --jq '.bypass_pull_request_allowances.apps[].slug' ``` @@ -95,61 +66,50 @@ gh run rerun RUN_ID --repo OWNER/REPO ### Security & Compliance Quick Checks ```bash -# REQUIRED baseline (one-liner): is required_conversation_resolution on? -gh api repos/OWNER/REPO/branches/$(gh api repos/OWNER/REPO --jq .default_branch)/protection \ - --jq '.required_conversation_resolution.enabled // false' -# If false, run scripts/init-branch-protection.sh OWNER/REPO - -gh api repos/OWNER/REPO/branches/main/protection --jq '.enforce_admins.enabled' +gh api repos/OWNER/REPO/branches/main/protection \ + --jq '{rcr: .required_conversation_resolution.enabled, admins: .enforce_admins.enabled}' gh api repos/OWNER/REPO/code-scanning/default-setup --jq '.state' -gh api graphql -f query='query($owner:String!,$repo:String!,$pr:Int!){ - repository(owner:$owner,name:$repo){pullRequest(number:$pr){ - reviewThreads(first:100){nodes{id isResolved}} - }} -}' -f owner=OWNER -f repo=REPO -F pr=NUMBER +gh pr view PR --repo OWNER/REPO --json reviewThreads --jq '.reviewThreads' ``` ### Merge Strategy Issues -See `references/auto-merge-guide.md` for: rebase-merge-with-signed-commits fixes, workflow-file PR manual merges, and the Copilot-review auto-approve race. +See `references/auto-merge-guide.md` (signed-commit rebase fixes, workflow-file PRs, Copilot auto-approve race). ## Running Scripts ```bash -# Apply baseline branch protection to a new repo (REQUIRED post-`gh repo create`) -scripts/init-branch-protection.sh OWNER/REPO -# After first CI run completes: -scripts/init-branch-protection.sh OWNER/REPO --from-current-checks - -# Audit an existing local checkout against GitHub-platform best practices -scripts/verify-github-project.sh /path/to/repository +scripts/init-branch-protection.sh OWNER/REPO # baseline (post gh repo create) +scripts/init-branch-protection.sh OWNER/REPO --from-current-checks # after first CI +scripts/verify-github-project.sh /path/to/repository # local-checkout audit ``` ## References | Topic | Reference | |-------|-----------| +| Repo bootstrap (post `gh repo create`) | `references/repo-bootstrap.md` | | Repository file layout | `references/repository-structure.md` | -| Branch migration (master to main) | `references/branch-migration.md` | -| Dependabot/Renovate configuration | `references/dependency-management.md` | +| Branch migration | `references/branch-migration.md` | +| Dependabot/Renovate | `references/dependency-management.md` | | Auto-approve + auto-merge | `references/auto-merge-guide.md` | -| Merge strategy for signed commits | `references/merge-strategy.md` | -| Sub-issues and issue hierarchy | `references/sub-issues.md` | -| Release labeling automation | `references/release-labeling.md` | +| Merge strategy (signed commits) | `references/merge-strategy.md` | +| Sub-issues | `references/sub-issues.md` | +| Release labeling | `references/release-labeling.md` | | gh CLI commands | `references/gh-cli-reference.md` | -| Go, TYPO3, polyglot CI checklists | `references/repo-setup-guide.md` | -| OpenSSF Scorecard, CodeQL, security | `references/security-config.md` | -| Workflow linting (actionlint) | `references/actionlint-guide.md` | -| Bash pitfalls in workflow `run:` steps | `references/workflow-bash-patterns.md` | -| PR shows too many commits (fork merge base) | `references/pr-commit-cleanup.md` | +| Polyglot CI checklists | `references/repo-setup-guide.md` | +| Scorecard, CodeQL, security | `references/security-config.md` | +| actionlint | `references/actionlint-guide.md` | +| Workflow bash pitfalls | `references/workflow-bash-patterns.md` | +| Fork merge base | `references/pr-commit-cleanup.md` | | Multi-repo batch ops | `references/multi-repo-operations.md` | -| Reusable workflow supply-chain trust + SHA pinning | `references/reusable-workflow-security.md` | -| Reusable workflow pitfalls (composite actions, ref caching, permissions) | `references/reusable-workflow-pitfalls.md` | -| Org-level security settings (SHA pinning) | `references/org-security-settings.md` | -| Tag validation (defense-in-depth) | `references/tag-validation.md` | -| AI reviewer pushback patterns | `references/ai-reviewer-pushback.md` | +| Reusable workflow security | `references/reusable-workflow-security.md` | +| Reusable workflow pitfalls | `references/reusable-workflow-pitfalls.md` | +| Org security settings | `references/org-security-settings.md` | +| Tag validation | `references/tag-validation.md` | +| AI reviewer pushback | `references/ai-reviewer-pushback.md` | | Agentic workflows | `references/agentic-workflows.md` | --- -> **Contributing:** https://github.com/netresearch/github-project-skill +> Contributing: https://github.com/netresearch/github-project-skill diff --git a/skills/github-project/references/repo-bootstrap.md b/skills/github-project/references/repo-bootstrap.md new file mode 100644 index 0000000..e679ec3 --- /dev/null +++ b/skills/github-project/references/repo-bootstrap.md @@ -0,0 +1,67 @@ +# Repository Bootstrap — Required First Step After `gh repo create` + +After creating any new Netresearch repository, **before pushing the first commit or opening the first PR**, you MUST apply branch protection. Without this, the unresolved-threads workflow rule is unenforceable — operator discipline alone has demonstrably failed. + +**Concrete incident:** [netresearch/snipe-it-docker-compose-stack#17](https://github.com/netresearch/snipe-it-docker-compose-stack/pull/17). The repo was created mid-session, branch protection was never applied, and three of the next eight merged PRs shipped with unresolved bot-reviewer threads — including a HIGH-severity token leak that both Copilot and gemini-code-assist had flagged. The structural enforcement (`required_conversation_resolution: true`) would have blocked those merges. The skill had the docs; nothing prompted the apply. + +## Two-step flow + +```bash +# 1. Immediately after `gh repo create` and the first push: +bash /skills/github-project/scripts/init-branch-protection.sh OWNER/REPO +# Applies the baseline: +# required_conversation_resolution: true (load-bearing) +# required_approving_review_count: 1 +# allow_force_pushes: false +# allow_deletions: false +# required_linear_history: false (needed for merge-commit +# signing strategy) +# Required status checks are intentionally NOT set yet — a brand-new repo +# has no CI history to discover context names from. + +# 2. After the first CI run completes on the default branch: +bash /skills/github-project/scripts/init-branch-protection.sh OWNER/REPO --from-current-checks +# Discovers successful check-run names from /commits/{default}/check-runs +# and PATCHes them in as required contexts with strict=true. +``` + +The script is idempotent: re-running on an already-compliant repo reports `already compliant` and exits 0. Drift on opinionated fields exits 1 with a per-field diff (no silent clobber of admin choices). + +## What the baseline deliberately omits + +- **`enforce_admins`** — shipped as `false`. Solo-maintainer Netresearch repos (snipe-it-docker-compose-stack, ldap-selfservice-…, usercentrics-widgets, etc.) need admin bypass for emergency response. Tighten per-repo once the team validates the workflow: + ```bash + gh api repos/OWNER/REPO/branches/DEFAULT/protection/enforce_admins -X POST + ``` +- **`required_signatures`** — omitted entirely (not set to `false`). The template would otherwise reset repos that have already opted into signing. Tighten per-repo: + ```bash + gh api repos/OWNER/REPO/branches/DEFAULT/protection/required_signatures -X POST + ``` + +Both knobs flip from absent/`false` → `true` only after the team has signing infrastructure for bot accounts (Dependabot, Renovate) so those PRs don't immediately get blocked. + +## Verification + +Read-only audit of an existing repo: + +```bash +gh api repos/OWNER/REPO/branches/$(gh api repos/OWNER/REPO --jq .default_branch)/protection \ + --jq '.required_conversation_resolution.enabled // false' +``` + +Or invoke `/assess github-project` — checkpoint `GH-31` fails with `severity: error` if `required_conversation_resolution` is not enabled, with a `desc:` that names this exact failure mode. + +## Gap NOT closed by the baseline + +GitHub branch protection cannot block on *requested-but-pending* reviews (Copilot is mid-review, you merge anyway). The baseline closes the **unresolved-threads** class (which is what snipe-it#17 slipped through), not the **pending-reviewer** class. The pending-reviewer gap is a workflow-discipline rule audited via the GraphQL `reviewRequests` check before any merge — see `references/security-config.md` § "Required Reviews from All Requested Reviewers". + +## Script exit codes + +| Code | Meaning | +|------|---------| +| 0 | Applied, or already compliant | +| 1 | Drift detected on opinionated fields (per-field diff printed); script refuses to clobber | +| 2 | Invalid arguments / template missing | +| 3 | Repo not found or no API access | +| 4 | Default branch ref does not yet exist (empty repo — push first) | +| 5 | `--from-current-checks`: no completed CI run on default branch | From a85aa53922b26dcf8ffec9e19c71fc14a1c247df Mon Sep 17 00:00:00 2001 From: Sebastian Mendel Date: Sat, 23 May 2026 07:55:26 +0200 Subject: [PATCH 7/8] refactor(scripts): PATCH required_status_checks subresource; fix header inconsistencies MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Addresses 7 of 10 review threads on PR #77. Architectural fix (Copilot thread on line 214): --from-current-checks previously rebuilt the entire branch-protection document and PUT it back, which silently dropped any fields the hardcoded jq filter didn't enumerate (bypass_pull_request_allowances, dismissal_restrictions, lock_branch settings, anything GitHub adds later). Switched to PATCH /branches/{b}/protection/required_status_checks — the dedicated subresource endpoint that accepts a partial body ({strict, contexts}) and leaves all other protection fields untouched. Now also requires baseline protection to already exist before --from-current-checks runs (errors with exit 1 if not), making the apply-then-discover flow explicit. Success-conditional gap (Copilot thread on line 156): --from-current-checks previously captured 'success' check-runs from the latest commit regardless of the commit's combined status. If the commit's overall status was 'failure' or 'pending', we'd silently require an incomplete subset of checks. Now we read the combined /commits/{sha}/status and warn (not abort) if it's not 'success', surfacing exactly which contexts were/weren't captured. Wording inconsistencies (Copilot threads on lines 9, 15, 267): - Header now says 'after gh repo create + initial push, BEFORE first PR' instead of 'before first commit' (script needs the default branch ref to exist — exits 4 on empty repos, was contradictory). - 'Exits 0 (or drift)' confusion clarified: exits 0 only on already- compliant, exits 1 with per-field diff on drift. - Drift-warning message no longer mentions '--from-current-checks' as the remedy (apply mode and --from-current-checks both refuse to clobber); now points at the manual gh-api PUT against the template. - Header notes enforce_admins IS in the template (was previously claimed not to be — Copilot thread on SKILL.md). Not changed (deliberate): gemini-code-assist HIGH on missing 'jq -r': false positive. 'gh api --jq' uses raw output by default (verified by xxd on actual output); the standalone-jq quoting rule doesn't apply to gh's embedded jq. Live-verified against snipe-it-docker-compose-stack — apply-mode still reports 'already compliant', shellcheck + bash -n clean. Signed-off-by: Sebastian Mendel --- .../scripts/init-branch-protection.sh | 139 +++++++++--------- 1 file changed, 69 insertions(+), 70 deletions(-) diff --git a/skills/github-project/scripts/init-branch-protection.sh b/skills/github-project/scripts/init-branch-protection.sh index a2c9d62..2ddcf87 100755 --- a/skills/github-project/scripts/init-branch-protection.sh +++ b/skills/github-project/scripts/init-branch-protection.sh @@ -2,22 +2,29 @@ # init-branch-protection.sh # Apply Netresearch standard branch protection to a GitHub repository. # -# This is the REQUIRED first step after `gh repo create`, before pushing the -# first commit or opening the first PR. The structural enforcement applied -# here (required_conversation_resolution + min-1-approver) is what makes the -# unresolved-threads workflow rule actually safe — relying only on operator -# discipline has demonstrably failed (see netresearch/snipe-it-docker-compose-stack#17). +# REQUIRED step after `gh repo create` + initial push, BEFORE opening the +# first PR. (The default branch ref must exist — push your initial commit +# first; this script exits 4 on empty repos.) The structural enforcement +# applied here (required_conversation_resolution + min-1-approver) is what +# makes the unresolved-threads workflow rule actually safe — operator +# discipline alone has demonstrably failed (see +# netresearch/snipe-it-docker-compose-stack#17). # # Usage: # bash init-branch-protection.sh / # Apply baseline protection (no required status checks yet — for new -# repos with no CI history). Idempotent: a second run on an already- -# compliant repo reports drift (or "already compliant") and exits 0. +# repos with no CI history). Idempotent: a second run reports +# "already compliant" and exits 0 if no drift, or exits 1 with a +# per-field diff if drift is present on opinionated fields. The +# script never auto-corrects drift — it refuses to clobber explicit +# admin choices. # # bash init-branch-protection.sh / --from-current-checks -# Follow-up after the first successful CI run. Reads the check-run names -# from the most recent completed workflow run on the default branch and -# PATCHes them in as required status contexts with strict=true. +# Follow-up after the first successful CI run. Reads check-run names +# from /commits/{default_branch}/check-runs and PATCHes them in via +# the .../protection/required_status_checks subresource (so other +# branch-protection fields — bypass_pull_request_allowances, +# dismissal_restrictions, etc. — are untouched). # # Baseline applied (see assets/branch-protection.json.template): # required_conversation_resolution: true <- the load-bearing field @@ -26,21 +33,25 @@ # allow_deletions: false # required_linear_history: false (must be false for merge-commit # strategy needed by signed commits) +# enforce_admins: false (explicit; see template comment) # -# Deliberately NOT in the template (per Netresearch org policy 2026-05): -# enforce_admins: shipped as false; tighten per-repo via: +# Deliberately tighter-than-default knobs (template ships permissive; raise +# per-repo once your team's signing infra and admin policy are settled): +# +# Make admins bound by branch protection: # gh api repos/OWNER/REPO/branches//protection/enforce_admins -X POST -# required_signatures: omitted entirely (not set to false) so this script -# never resets a repo that has already enabled signing. Tighten per-repo: +# +# Require GPG/SSH-signed commits (not in template — script never resets it): # gh api repos/OWNER/REPO/branches//protection/required_signatures -X POST # # Exit codes: -# 0 - applied or already compliant -# 1 - drift detected (reported, not auto-corrected on second run) +# 0 - applied successfully, or already compliant +# 1 - drift detected on opinionated fields (per-field diff printed), +# or a PUT/PATCH failed # 2 - invalid arguments / template missing # 3 - repo not found or no access -# 4 - default branch does not yet exist (empty repo — push initial commit first) -# 5 - --from-current-checks: no completed workflow run found on default branch +# 4 - default branch ref does not yet exist (empty repo — push first) +# 5 - --from-current-checks: no completed CI run on default branch # # SPDX-License-Identifier: MIT # Copyright (c) Netresearch DTT GmbH @@ -132,6 +143,17 @@ PROTECTION_URL="repos/$OWNER/$REPO/branches/$DEFAULT_BRANCH/protection" # ---------- --from-current-checks mode ---------- if [[ "$MODE" == "--from-current-checks" ]]; then + # Baseline protection MUST already exist — we PATCH the + # required_status_checks subresource only. This avoids clobbering + # fields the apply-mode template does not enumerate (e.g. + # bypass_pull_request_allowances, dismissal_restrictions, or any + # field GitHub adds later). + if ! gh api "$PROTECTION_URL" --silent 2>/dev/null; then + err "no existing branch protection on $SLUG" + err "run without --from-current-checks first to apply the baseline." + exit 1 + fi + info "discovering required status checks from latest commit on $DEFAULT_BRANCH ..." # GitHub's required_status_checks.contexts are matched against the @@ -139,15 +161,27 @@ if [[ "$MODE" == "--from-current-checks" ]]; then # for matrix and called-workflow jobs, e.g. "container-lint / hadolint"). # The /actions/runs/{id}/jobs endpoint returns the bare job name # ("hadolint") — wrong for context matching. We use /commits/{sha}/check-runs - # against the default branch's HEAD which returns the canonical - # check-run names that align with what shows up in the PR check UI and - # what GitHub compares against required_status_checks.contexts. + # against the default branch's HEAD, which returns the canonical + # check-run names that align with what GitHub compares against + # required_status_checks.contexts. HEAD_SHA="$(gh api "repos/$OWNER/$REPO/commits/$DEFAULT_BRANCH" --jq '.sha // empty' 2>/dev/null || true)" if [[ -z "$HEAD_SHA" ]]; then err "could not resolve HEAD sha of $DEFAULT_BRANCH" exit 5 fi - info "using $DEFAULT_BRANCH @ ${HEAD_SHA:0:8}" + + # Sanity-check the commit's overall combined status: if it's not 'success' + # we may be capturing an incomplete set of checks (e.g., a failing run + # where some jobs never executed). Warn rather than abort — operator + # may intentionally be onboarding partial coverage. + COMBINED="$(gh api "repos/$OWNER/$REPO/commits/$HEAD_SHA/status" --jq '.state // "unknown"' 2>/dev/null || echo unknown)" + info "using $DEFAULT_BRANCH @ ${HEAD_SHA:0:8} (combined status: $COMBINED)" + if [[ "$COMBINED" != "success" ]]; then + warn "combined status is '$COMBINED' (not 'success') — only check-runs that" + warn "actually completed successfully will be captured. Other contexts that" + warn "did not run on this commit will NOT be required. Re-run after a fully" + warn "green CI run on $DEFAULT_BRANCH for complete coverage." + fi # Collect successful check-run names for that commit, deduped. mapfile -t CHECK_NAMES < <(gh api --paginate \ @@ -165,57 +199,21 @@ if [[ "$MODE" == "--from-current-checks" ]]; then info "discovered ${#CHECK_NAMES[@]} required check(s):" for n in "${CHECK_NAMES[@]}"; do printf ' - %s\n' "$n" >&2; done - # Build a JSON body that updates only required_status_checks. We PATCH the - # protection endpoint by re-PUTting the merged document: GitHub's API for - # branch protection does not accept partial bodies, so we read existing - # protection, splice the new contexts in, and PUT back. - EXISTING="$(gh api "$PROTECTION_URL" 2>/dev/null || echo '{}')" - - # If protection has not been initialized yet, fall back to the template. - if [[ "$EXISTING" == "{}" ]] || [[ -z "$(jq -r '.url // empty' <<<"$EXISTING")" ]]; then - warn "no existing protection — applying baseline first then adding checks" - EXISTING="$(cat "$TEMPLATE")" - fi - - # Normalize the existing protection into a PUT-compatible body (the GET - # response includes embedded `url` fields and nested envelopes that the - # PUT endpoint rejects). - PUT_BODY="$(jq \ + # PATCH only the required_status_checks subresource. This endpoint + # accepts a partial body and leaves all other branch-protection fields + # untouched — the safe way to add required checks without enumerating + # (and potentially dropping) other settings. + SUBRES="$PROTECTION_URL/required_status_checks" + PATCH_BODY="$(jq -n \ --argjson checks "$(printf '%s\n' "${CHECK_NAMES[@]}" | jq -R . | jq -s .)" \ - '{ - required_status_checks: { - strict: true, - contexts: $checks - }, - enforce_admins: (.enforce_admins.enabled // false), - required_pull_request_reviews: ( - if .required_pull_request_reviews then { - required_approving_review_count: (.required_pull_request_reviews.required_approving_review_count // 1), - dismiss_stale_reviews: (.required_pull_request_reviews.dismiss_stale_reviews // false), - require_code_owner_reviews: (.required_pull_request_reviews.require_code_owner_reviews // false), - require_last_push_approval: (.required_pull_request_reviews.require_last_push_approval // false) - } else { - required_approving_review_count: 1, - dismiss_stale_reviews: false, - require_code_owner_reviews: false, - require_last_push_approval: false - } end - ), - restrictions: null, - required_linear_history: (.required_linear_history.enabled // false), - allow_force_pushes: (.allow_force_pushes.enabled // false), - allow_deletions: (.allow_deletions.enabled // false), - required_conversation_resolution: (.required_conversation_resolution.enabled // true), - lock_branch: (.lock_branch.enabled // false), - allow_fork_syncing: (.allow_fork_syncing.enabled // false) - }' <<<"$EXISTING")" - - info "PUT $PROTECTION_URL (with required checks)" - if RESP="$(gh api -X PUT "$PROTECTION_URL" --input - <<<"$PUT_BODY" 2>&1)"; then + '{strict: true, contexts: $checks}')" + + info "PATCH $SUBRES" + if RESP="$(gh api -X PATCH "$SUBRES" --input - <<<"$PATCH_BODY" 2>&1)"; then ok "required status checks applied (${#CHECK_NAMES[@]} contexts, strict=true)" exit 0 else - err "PUT failed:" + err "PATCH failed:" printf '%s\n' "$RESP" >&2 exit 1 fi @@ -268,7 +266,8 @@ if [[ -n "$EXISTING" ]] && [[ -n "$(jq -r '.url // empty' <<<"$EXISTING" 2>/dev/ warn "drift detected vs template baseline:" printf '%s' "$DRIFT" >&2 - warn "not auto-correcting — re-run without --from-current-checks intentionally," + warn "not auto-correcting — apply the template manually with" + warn " gh api -X PUT $PROTECTION_URL --input /assets/branch-protection.json.template" warn "or PATCH specific fields by hand. Aborting to avoid clobbering admin choices." exit 1 fi From fa66bd5432286ccb30aef3eba8f2b6d433e1c461 Mon Sep 17 00:00:00 2001 From: Sebastian Mendel Date: Sat, 23 May 2026 07:55:36 +0200 Subject: [PATCH 8/8] docs: align skill+reference+checkpoint wording with empty-repo precondition MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Addresses 3 of 10 review threads on PR #77 (Copilot threads on SKILL.md, checkpoints.yaml line 356, and the enforce_admins description inside repo-bootstrap.md). The init script requires the default branch ref to exist (exits 4 on empty repos). Earlier wording said 'before pushing the first commit' which is operationally impossible — the script needs a ref to protect. Corrected uniformly across: - SKILL.md 'When to Use' bullet: 'post gh repo create + initial push, before first PR' (was 'before first commit/PR') - references/repo-bootstrap.md intro paragraph: explicit three-step flow (create -> initial push -> protect, then open PRs) - checkpoints.yaml GH-31 desc: 'right after gh repo create + initial push, before opening the first PR' (was 'immediately after gh repo create') Also corrected repo-bootstrap.md's 'baseline deliberately omits' section: the template DOES include enforce_admins=false (explicit permissive default). Only required_signatures is omitted entirely. Section renamed to 'Deliberately permissive knobs' to match reality. Signed-off-by: Sebastian Mendel --- skills/github-project/SKILL.md | 2 +- skills/github-project/checkpoints.yaml | 10 ++++++---- skills/github-project/references/repo-bootstrap.md | 10 +++++----- 3 files changed, 12 insertions(+), 10 deletions(-) diff --git a/skills/github-project/SKILL.md b/skills/github-project/SKILL.md index 1f5d91f..4b23588 100644 --- a/skills/github-project/SKILL.md +++ b/skills/github-project/SKILL.md @@ -16,7 +16,7 @@ GitHub repository configuration, troubleshooting, and collaboration workflow bes ## When to Use -- **Post `gh repo create`, before first commit/PR** — apply branch protection (REQUIRED, see below) +- **Post `gh repo create` + initial push, before first PR** — apply branch protection (REQUIRED, see below) - PR won't merge, BLOCKED, or unresolved threads - Auto-merge fails for Dependabot/Renovate - Solo maintainer auto-approve diff --git a/skills/github-project/checkpoints.yaml b/skills/github-project/checkpoints.yaml index 18722c1..ae803f9 100644 --- a/skills/github-project/checkpoints.yaml +++ b/skills/github-project/checkpoints.yaml @@ -350,10 +350,12 @@ mechanical: netresearch/snipe-it-docker-compose-stack#17 where this gap let a HIGH-severity Copilot/gemini-code-assist finding ship to main). Apply via skills/github-project/scripts/init-branch-protection.sh - OWNER/REPO immediately after `gh repo create`. Combined with - enforce_admins, also ensures unresolved threads block ALL merges - including admins (audited by GH-32 llm_review; runner skips - because gh API access requires auth). + OWNER/REPO right after `gh repo create` + initial push, before + opening the first PR (script requires the default branch ref to + exist; exits 4 on empty repos). Combined with enforce_admins, + also ensures unresolved threads block ALL merges including admins + (audited by GH-32 llm_review; runner skips because gh API access + requires auth). llm_reviews: # === BRANCH PROTECTION + MERGE QUEUE COMPATIBILITY === diff --git a/skills/github-project/references/repo-bootstrap.md b/skills/github-project/references/repo-bootstrap.md index e679ec3..0c89f57 100644 --- a/skills/github-project/references/repo-bootstrap.md +++ b/skills/github-project/references/repo-bootstrap.md @@ -1,6 +1,6 @@ # Repository Bootstrap — Required First Step After `gh repo create` -After creating any new Netresearch repository, **before pushing the first commit or opening the first PR**, you MUST apply branch protection. Without this, the unresolved-threads workflow rule is unenforceable — operator discipline alone has demonstrably failed. +After creating any new Netresearch repository — `gh repo create`, push your initial commit, **then before opening the first PR** — you MUST apply branch protection. (The default branch ref must exist; the script exits 4 on empty repos.) Without this, the unresolved-threads workflow rule is unenforceable — operator discipline alone has demonstrably failed. **Concrete incident:** [netresearch/snipe-it-docker-compose-stack#17](https://github.com/netresearch/snipe-it-docker-compose-stack/pull/17). The repo was created mid-session, branch protection was never applied, and three of the next eight merged PRs shipped with unresolved bot-reviewer threads — including a HIGH-severity token leak that both Copilot and gemini-code-assist had flagged. The structural enforcement (`required_conversation_resolution: true`) would have blocked those merges. The skill had the docs; nothing prompted the apply. @@ -27,18 +27,18 @@ bash /skills/github-project/scripts/init-branch-protection.sh OWNER/ The script is idempotent: re-running on an already-compliant repo reports `already compliant` and exits 0. Drift on opinionated fields exits 1 with a per-field diff (no silent clobber of admin choices). -## What the baseline deliberately omits +## Deliberately permissive knobs -- **`enforce_admins`** — shipped as `false`. Solo-maintainer Netresearch repos (snipe-it-docker-compose-stack, ldap-selfservice-…, usercentrics-widgets, etc.) need admin bypass for emergency response. Tighten per-repo once the team validates the workflow: +- **`enforce_admins`** — explicitly `false` in the template. Solo-maintainer Netresearch repos (snipe-it-docker-compose-stack, ldap-selfservice-…, usercentrics-widgets, etc.) need admin bypass for emergency response. Tighten per-repo once the team validates the workflow: ```bash gh api repos/OWNER/REPO/branches/DEFAULT/protection/enforce_admins -X POST ``` -- **`required_signatures`** — omitted entirely (not set to `false`). The template would otherwise reset repos that have already opted into signing. Tighten per-repo: +- **`required_signatures`** — *omitted entirely* from the template (not set to `false`). PUTting the template would otherwise reset repos that have already opted into signing. The script never touches this field. Tighten per-repo: ```bash gh api repos/OWNER/REPO/branches/DEFAULT/protection/required_signatures -X POST ``` -Both knobs flip from absent/`false` → `true` only after the team has signing infrastructure for bot accounts (Dependabot, Renovate) so those PRs don't immediately get blocked. +Both knobs flip to `true` only after the team has signing infrastructure for bot accounts (Dependabot, Renovate) so those PRs don't immediately get blocked. ## Verification