diff --git a/.claude/skills/ship/SKILL.md b/.claude/skills/ship/SKILL.md deleted file mode 100644 index 7f337ae..0000000 --- a/.claude/skills/ship/SKILL.md +++ /dev/null @@ -1,142 +0,0 @@ ---- -name: ship -description: >- - End-to-end PR lifecycle: update docs, commit, create PR, monitor CI, - address CodeRabbit and Copilot reviews, and merge. Use this skill - whenever the user says "ship it", "send a PR", "commit and merge", - "push this", or wants to finalize and land their changes. Pass a PR - number to resume monitoring an existing PR. -disable-model-invocation: true -user-invocable: true -argument-hint: "[optional PR number to resume monitoring]" ---- - -# Ship — Commit, Monitor, Fix, Merge - -End-to-end workflow: update documentation, commit, create PR, monitor CI -and code reviews, address feedback, and merge when everything passes. - -If `$ARGUMENTS` contains a PR number, skip to the monitoring phase for -that PR. - -## Phase 1 — Update Documentation - -Before committing, review all staged and unstaged changes to understand -what was modified, then update: - -1. **CLAUDE.md** — If any CI/CD pipelines, hooks, skills, conventions, - or project structure changed, update the relevant sections. -2. **README.md** — If scripts, settings, architecture, or tooling - changed, update the relevant sections. -3. **docs/adr/** — If a significant architectural decision was made - (new pattern, new tool, structural change), create or update an ADR. -4. **MEMORY.md** — Update project memory at - `$HOME/.claude/projects/-Users-gamaware-Documents-Repos-personal-github-org-settings/memory/MEMORY.md` - if there are new gotchas, patterns, or preferences learned. - -Only update files where changes are actually needed. Do not update docs -for trivial changes. - -## Phase 2 — Commit and Push - -1. Stage all changes (including doc updates from Phase 1). -2. Write a conventional commit message summarizing all changes. -3. Push to the current feature branch. -4. Create a PR if one does not exist yet. Use the commit message as - the PR title. Include a summary and test plan in the body. - -## Phase 3 — Monitor CI - -Poll CI status using `gh pr checks ` every 30 seconds until -all checks complete (pass, fail, or skip). Report the final status. - -If any check fails: - -1. Read the failure logs with `gh run view --log-failed`. -2. Diagnose and fix the issue. -3. Commit and push the fix. -4. Return to monitoring. - -## Phase 4 — Monitor Code Reviews - -Check for review comments from CodeRabbit and Copilot: - -```bash -gh api repos/{owner}/{repo}/pulls/{number}/comments -gh api repos/{owner}/{repo}/issues/{number}/comments -``` - -If CodeRabbit is rate-limited, wait for the timeout period then trigger -with `gh pr comment --body "@coderabbitai review"`. - -For each review comment: - -1. Read the comment carefully. -2. Check if the comment is stale (already fixed in a later commit) by - reading the current file state. Dismiss stale comments. -3. If the comment is valid, fix the issue, commit, and push. -4. After fixing, re-monitor CI (return to Phase 3). - -After all fixes are pushed and the incremental review passes, resolve -stale CodeRabbit threads in bulk: - -```bash -gh pr comment --body "@coderabbitai resolve" -``` - -For Copilot threads, resolve them via the GraphQL API: - -```bash -gh api graphql -f query='{ - repository(owner: "gamaware", name: "github-org-settings") { - pullRequest(number: ) { - reviewThreads(first: 100) { nodes { id isResolved } } - } - } -}' -``` - -Then for each unresolved thread ID: - -```bash -gh api graphql -f query='mutation { - resolveReviewThread(input: {threadId: ""}) { - thread { isResolved } - } -}' -``` - -## Phase 5 — Merge - -Once ALL of the following are true: - -- All CI checks pass -- CodeRabbit review has no unaddressed comments -- Copilot review has no unaddressed comments - -Then merge: - -```bash -gh pr merge --squash --admin --delete-branch -``` - -Pull main locally after merge: - -```bash -git checkout main && git pull -``` - -Report the merge commit and confirm the branch was deleted. - -## Rules - -- Never suppress lint violations — fix them. -- No AI attribution in commits. -- Conventional commit messages required. -- CodeRabbit may hit hourly rate limits — wait and retry. -- Copilot comments may be stale after fix commits — verify current file state. -- CodeRabbit auto-reviews incrementally on every push (up to 5 commits, - then pauses). Use `@coderabbitai review` to resume after pause. -- CodeRabbit does NOT auto-resolve its threads — use `@coderabbitai resolve` - after fixes are confirmed. -- Use `--admin` to bypass branch protection for merge. diff --git a/.github/workflows/auto-merge-bot-prs.yml b/.github/workflows/auto-merge-bot-prs.yml new file mode 100644 index 0000000..5663717 --- /dev/null +++ b/.github/workflows/auto-merge-bot-prs.yml @@ -0,0 +1,203 @@ +# ----------------------------------------------------------------- +# Auto-merge bot PRs +# +# Squash-merges Dependabot PRs and the weekly pre-commit hook update +# PRs once every check on the PR is green and none is pending. +# +# Why admin bypass: the pre-commit PRs are authored with the repo +# owner's PAT, and GitHub refuses self-approval, so a review-based +# auto-merge can never satisfy the CODEOWNERS rule. Branch protection +# is not enforced on admins, so the owner's PAT merges directly. +# +# Safety: never touches drafts, fork PRs, PRs from other authors, +# PRs with conflicts, PRs with no registered checks, or PRs with any +# failing or pending check. A PR that is behind main is updated and +# picked up on the next run. +# +# Requires: PRE_COMMIT_PAT secret (fine-grained PAT of the repo owner +# with contents, pull requests and workflows write access). +# ----------------------------------------------------------------- +name: Auto-merge bot PRs +run-name: "Auto-merge bot PRs (${{ github.event_name }})" + +on: + schedule: + - cron: "17 * * * *" + workflow_dispatch: + +permissions: {} + +concurrency: + group: auto-merge-bot-prs + cancel-in-progress: false + +jobs: + merge: + name: Merge green bot PRs + runs-on: ubuntu-latest + timeout-minutes: 15 + # zizmor's secrets-outside-env audit requires secrets to be read inside a + # job environment. "automation" carries no protection rules and GitHub + # creates it on first use. + environment: automation + steps: + - name: Merge eligible PRs + env: + GH_TOKEN: ${{ secrets.PRE_COMMIT_PAT }} + REPO: ${{ github.repository }} + OWNER: ${{ github.repository_owner }} + run: | + set -euo pipefail + + if [ -z "$GH_TOKEN" ]; then + echo "::error::PRE_COMMIT_PAT secret is not set on this repository" + exit 1 + fi + + # Candidate PRs: Dependabot, or the weekly pre-commit update branch + # authored by the repo owner (the PAT owner). Fork PRs are excluded + # because anyone can name a fork branch to match the prefix. + # Capture the list first so a gh failure aborts the run instead of + # being mistaken for "no open PRs". + pr_list=$( + gh pr list --repo "$REPO" --state open --limit 100 \ + --json number,author,headRefName,baseRefName,headRefOid,isDraft,isCrossRepository,headRepositoryOwner \ + --jq '.[] + | select(.isDraft | not) + | select(.isCrossRepository | not) + | select(.headRepositoryOwner.login == env.OWNER) + | select(.baseRefName == "main") + | select( + ((.author.login | sub("^app/"; "")) == "dependabot") + or ( + (.headRefName | startswith("chore/update-pre-commit-hooks")) + and (.author.login == env.OWNER) + ) + ) + | "\(.number) \(.headRefOid)"' + ) + mapfile -t candidates <<< "$pr_list" + if [ "${#candidates[@]}" -eq 1 ] && [ -z "${candidates[0]}" ]; then + candidates=() + fi + + if [ "${#candidates[@]}" -eq 0 ]; then + echo "No eligible bot PRs open." + exit 0 + fi + + merged=0 + skipped=0 + + for entry in "${candidates[@]}"; do + number="${entry%% *}" + head_sha="${entry##* }" + if [[ ! "$head_sha" =~ ^[0-9a-f]{40}$ ]]; then + echo "::error::PR #$number: missing head SHA in '$entry', skipping" + skipped=$((skipped + 1)) + continue + fi + + if ! state_line=$(gh pr view "$number" --repo "$REPO" \ + --json mergeStateStatus,reviewDecision \ + --jq '"\(.mergeStateStatus) \(.reviewDecision)"'); then + echo "PR #$number: could not read merge state, skipping" + skipped=$((skipped + 1)) + continue + fi + state="${state_line%% *}" + review="${state_line##* }" + + # Admin bypass must never override a reviewer who asked for changes. + if [ "$review" = "CHANGES_REQUESTED" ]; then + echo "PR #$number: changes requested by a reviewer, skipping" + skipped=$((skipped + 1)) + continue + fi + + case "$state" in + DIRTY) + echo "PR #$number: merge conflicts, skipping" + skipped=$((skipped + 1)) + continue + ;; + BEHIND) + echo "PR #$number: behind base, requesting branch update" + gh pr update-branch "$number" --repo "$REPO" \ + || echo "::warning::PR #$number: branch update failed, will retry next run" + skipped=$((skipped + 1)) + continue + ;; + CLEAN|BLOCKED|UNSTABLE|HAS_HOOKS) + ;; + *) + # UNKNOWN (mergeability still being computed) or anything new. + echo "PR #$number: merge state $state, skipping" + skipped=$((skipped + 1)) + continue + ;; + esac + + # gh pr checks exits non-zero on API errors or when no checks exist + # (with --json it exits 0 for failing or pending checks). Keep the + # output, drop the exit code, and treat empty output as "no checks" + # so the guard below skips. + checks_err="${RUNNER_TEMP:-/tmp}/pr-checks-$number.err" + summary=$(gh pr checks "$number" --repo "$REPO" --json bucket \ + --jq '[ + length, + ([.[] | select(.bucket == "fail" or .bucket == "cancel")] | length), + ([.[] | select(.bucket == "pending")] | length), + ([.[] | select(.bucket == "pass")] | length) + ] | @tsv' 2>"$checks_err") || true + if [ -z "$summary" ]; then + # Distinguish a real API/auth failure from "no checks reported". + if [ -s "$checks_err" ] && ! grep -qi "no checks" "$checks_err"; then + echo "::warning::PR #$number: gh pr checks failed: $(head -n 1 "$checks_err")" + fi + summary=$'0\t0\t0\t0' + fi + IFS=$'\t' read -r total fail pending pass <<< "$summary" + + if [ "$fail" -gt 0 ]; then + echo "PR #$number: $fail failing check(s), skipping" + skipped=$((skipped + 1)) + continue + fi + if [ "$pending" -gt 0 ]; then + echo "PR #$number: $pending pending check(s), skipping" + skipped=$((skipped + 1)) + continue + fi + # Never merge on an empty check list: a brand-new PR may not have + # registered its checks yet, and a held fork run has none at all. + if [ "$total" -eq 0 ]; then + echo "PR #$number: no checks registered, skipping" + skipped=$((skipped + 1)) + continue + fi + # Skipped checks are not failures, but they are not evidence either: + # require at least one check that actually ran and passed. + if [ "$pass" -eq 0 ]; then + echo "PR #$number: $total check(s) registered but none passed, skipping" + skipped=$((skipped + 1)) + continue + fi + + echo "PR #$number: $pass of $total check(s) passed, none failing or pending, merging" + # Bind the merge to the head captured before the checks were + # inspected; if the head moved since then, the merge is refused. + if gh pr merge "$number" --repo "$REPO" --squash --admin --delete-branch \ + --match-head-commit "$head_sha"; then + merged=$((merged + 1)) + # main has moved: stop here so every remaining candidate is + # re-evaluated against the new base on the next run. + echo "Merged one PR; remaining candidates are re-evaluated next run." + break + else + echo "::warning::PR #$number: merge failed" + skipped=$((skipped + 1)) + fi + done + + echo "Merged: $merged | Skipped: $skipped" diff --git a/.github/workflows/auto-merge-dependabot.yml b/.github/workflows/auto-merge-dependabot.yml deleted file mode 100644 index a2b0d20..0000000 --- a/.github/workflows/auto-merge-dependabot.yml +++ /dev/null @@ -1,42 +0,0 @@ -# ----------------------------------------------------------------- -# Dependabot & Pre-commit Auto-merge -# -# Auto-approves and enables squash auto-merge for: -# - Dependabot PRs (all ecosystems) -# - Pre-commit hook update PRs (chore/update-pre-commit-hooks branch) -# -# Requires: PRE_COMMIT_PAT secret (fine-grained PAT with PR write) -# to satisfy CODEOWNERS approval requirement. -# -# Gate: CI must pass before GitHub executes the auto-merge. -# ----------------------------------------------------------------- -name: Auto-merge Dependabot & Pre-commit -run-name: "Auto-merge: PR #${{ github.event.pull_request.number }}" - -on: - pull_request_target: - types: [opened, synchronize, reopened] - -permissions: - contents: write - pull-requests: write - -jobs: - auto-merge: - name: Auto-approve and merge - runs-on: ubuntu-latest - if: >- - github.actor == 'dependabot[bot]' || - startsWith(github.head_ref, 'chore/update-pre-commit-hooks') - steps: - - name: Approve PR (as repo owner via PAT) - run: gh pr review --approve "$PR_URL" - env: - PR_URL: ${{ github.event.pull_request.html_url }} - GH_TOKEN: ${{ secrets.PRE_COMMIT_PAT }} - - - name: Enable auto-merge (squash) - run: gh pr merge --auto --squash "$PR_URL" - env: - PR_URL: ${{ github.event.pull_request.html_url }} - GH_TOKEN: ${{ secrets.PRE_COMMIT_PAT }} diff --git a/.github/workflows/sync-settings.yml b/.github/workflows/sync-settings.yml index 7f4112a..26979d9 100644 --- a/.github/workflows/sync-settings.yml +++ b/.github/workflows/sync-settings.yml @@ -88,16 +88,29 @@ jobs: See the [workflow run]($RUN_URL) for the full report." # Create new issue first, then close old ones - if ! gh issue create --title "$TITLE" --body "$BODY" --label "settings-drift"; then + if ! NEW_URL=$(gh issue create --title "$TITLE" --body "$BODY" --label "settings-drift"); then echo "::error::Failed to create drift issue" exit 1 fi - - # Close previous drift issues (all except the one just created) - LATEST=$(gh issue list --label "settings-drift" --state open --json number --jq '.[0].number') - gh issue list --label "settings-drift" --state open --json number --jq '.[].number' | while read -r num; do - if [ "$num" != "$LATEST" ]; then - gh issue close "$num" --comment "Superseded by new sync run." + NEW_NUMBER="${NEW_URL##*/}" + case "$NEW_NUMBER" in + ''|*[!0-9]*) + echo "::error::Could not parse issue number from '$NEW_URL'" + exit 1 + ;; + esac + echo "Created drift issue #$NEW_NUMBER" + + # Close every other open drift issue. The search index can lag + # behind issue creation, so compare against the number returned + # by 'gh issue create' instead of trusting list order. + # Capture the list first so a failing gh call fails the step + # instead of silently iterating zero times. + OPEN_NUMBERS=$(gh issue list --label "settings-drift" --state open --limit 100 --json number --jq '.[].number') + for num in $OPEN_NUMBERS; do + if [ "$num" != "$NEW_NUMBER" ]; then + gh issue close "$num" --comment "Superseded by #$NEW_NUMBER." \ + || echo "::warning::Could not close superseded drift issue #$num" fi done @@ -106,9 +119,21 @@ jobs: env: GH_TOKEN: ${{ secrets.ORG_SETTINGS_PAT }} run: | - gh issue list --label "settings-drift" --state open --json number --jq '.[].number' | while read -r num; do - gh issue close "$num" --comment "All repositories are now compliant." + # Capture the list first so a failing gh call fails the step + # instead of silently iterating zero times. Close every issue + # even if one close fails, then fail the step so the leftover + # is visible. + OPEN_NUMBERS=$(gh issue list --label "settings-drift" --state open --limit 100 --json number --jq '.[].number') + close_failed=0 + for num in $OPEN_NUMBERS; do + if ! gh issue close "$num" --comment "All repositories are now compliant."; then + echo "::warning::Could not close drift issue #$num" + close_failed=1 + fi done + if [ "$close_failed" -ne 0 ]; then + exit 1 + fi new-repo-check: name: Discover New Repositories diff --git a/CLAUDE.md b/CLAUDE.md index abbb858..49e0285 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -71,6 +71,10 @@ General, secrets, shell, markdown, prose (Vale), GitHub Actions - `security.yml` — Semgrep SAST + Trivy SCA (via composite action) - `update-pre-commit-hooks.yml` — weekly auto-update via PR (via composite action) +- `auto-merge-bot-prs.yml` — hourly scheduled job that squash-merges + Dependabot and pre-commit PRs with admin bypass once every check is + green (no approval step; GitHub rejects self-approval with the + owner's PAT) ## Composite Actions @@ -85,8 +89,9 @@ General, secrets, shell, markdown, prose (Vale), GitHub Actions - `/audit` — run a dry-run settings check across all repos - `/add-repo-override` — add a per-repo exception to overrides.json - `/exclude-repo` — exclude a repository from governance -- `/ship [PR-number]` — end-to-end PR lifecycle: update docs, commit, - create PR, monitor CI, address reviews (CodeRabbit + Copilot), merge +- `/ship-it [PR-number]` — end-to-end PR lifecycle: update docs, commit, + create PR, monitor CI, address reviews (CodeRabbit + Copilot), merge, + clean up stale branches. Uses global skill. ## Code Review diff --git a/README.md b/README.md index 57e93e5..2dd809f 100644 --- a/README.md +++ b/README.md @@ -179,6 +179,7 @@ github-org-settings/ │ │ └── update-pre-commit-composite/ # Composite: hook updates │ │ └── action.yml │ ├── workflows/ +│ │ ├── auto-merge-bot-prs.yml # Hourly admin merge of green bot PRs │ │ ├── sync-settings.yml # Weekly settings sync + reports │ │ ├── quality-checks.yml # PR/push linting and validation │ │ ├── security.yml # SAST + SCA scanning @@ -306,6 +307,7 @@ Add repo names to the `excluded` array in `config/overrides.json`: | `quality-checks.yml` | PR + push to main | Markdown, YAML, shell, structure | | `security.yml` | PR + push to main | Semgrep SAST + Trivy SCA | | `update-pre-commit-hooks.yml` | Weekly + manual | Auto-update hook versions | +| `auto-merge-bot-prs.yml` | Hourly + manual | Squash-merge green Dependabot and pre-commit PRs | ## Author diff --git a/docs/architecture.md b/docs/architecture.md index bb11f4c..b19d832 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -78,6 +78,7 @@ Reusable building blocks consumed by workflows: | `quality-checks.yml` | PR + push | Linting and validation | | `security.yml` | PR + push | SAST + SCA | | `update-pre-commit-hooks.yml` | Weekly Sunday 00:00 UTC | Hook version updates | +| `auto-merge-bot-prs.yml` | Hourly | Squash-merge green Dependabot and pre-commit PRs | ### Reporting diff --git a/scripts/sync-repo-settings.sh b/scripts/sync-repo-settings.sh index 55eb7d3..4ae6a84 100755 --- a/scripts/sync-repo-settings.sh +++ b/scripts/sync-repo-settings.sh @@ -14,8 +14,11 @@ REPORT_FILE="${REPORT_FILE:-$ROOT_DIR/reports/sync-report.md}" mkdir -p "$(dirname "$REPORT_FILE")" +# Log to stderr: every sync_* function's stdout is captured by main() as +# that repo's drift text, so log lines on stdout would land in the report +# and make every repo look drifted. log() { - echo "[$(date '+%Y-%m-%d %H:%M:%S')] $*" + echo "[$(date '+%Y-%m-%d %H:%M:%S')] $*" >&2 } # Collect all repos for the owner, excluding archived and excluded repos @@ -70,10 +73,24 @@ sync_repo_settings() { fi done + # GitHub returns 422 invalid_squash_commit_setting_combo when + # squash_merge_commit_message is sent without squash_merge_commit_title + # (or vice versa), so always send the pair from the effective baseline. + if echo "$patch" | jq -e 'has("squash_merge_commit_title") or has("squash_merge_commit_message")' > /dev/null; then + local squash_pair + squash_pair=$(echo "$effective" | jq '.repo_settings | {squash_merge_commit_title, squash_merge_commit_message} | with_entries(select(.value != null))') + patch=$(echo "$patch" | jq --argjson s "$squash_pair" '. + $s') + fi + if [ "$changes" != "" ]; then if [ "$MODE" = "--apply" ]; then - gh api -X PATCH "repos/$OWNER/$repo" --input <(echo "$patch") > /dev/null 2>&1 - log "APPLIED repo settings for $repo" + local output + if output=$(gh api -X PATCH "repos/$OWNER/$repo" --input <(echo "$patch") 2>&1); then + log "APPLIED repo settings for $repo" + else + log "ERROR: could not apply repo settings for $repo: $(echo "$output" | head -n 1)" + changes="${changes}- ERROR: settings not applied (see log)\n" + fi else log "DRIFT detected in repo settings for $repo" fi @@ -115,7 +132,8 @@ sync_security() { changes="- Secret scanning: \`$current_scanning\` -> \`$desired_scanning_status\`\n" changes="${changes}- Push protection: \`$current_push\` -> \`$desired_push_status\`\n" if [ "$MODE" = "--apply" ]; then - gh api -X PATCH "repos/$OWNER/$repo" --input <(cat < /dev/null 2>&1 || log "WARN: Could not update security settings for $repo (may require admin)" - log "APPLIED security settings for $repo" + ) 2>&1); then + log "APPLIED security settings for $repo" + else + log "ERROR: could not update security settings for $repo: $(echo "$security_out" | head -n 1)" + changes="${changes}- ERROR: security settings not applied (see log)\n" + fi else log "DRIFT detected in security settings for $repo" fi @@ -185,14 +207,34 @@ sync_branch_protection() { branch=$(echo "$effective" | jq -r '.branch_protection.branch') local current - current=$(gh api "repos/$OWNER/$repo/branches/$branch/protection" 2>/dev/null) || { + if ! current=$(gh api "repos/$OWNER/$repo/branches/$branch/protection" 2>&1); then + # Private repos on the Free plan cannot have branch protection at + # all, so this is not drift the sync can ever resolve. + if echo "$current" | grep -qE "Upgrade to GitHub Pro|\(HTTP 403\)"; then + log "SKIP: branch protection unavailable on this plan for $repo" + echo "" + return + fi + # Only a confirmed "Branch not protected" (404) means nothing is + # configured. Any other failure (auth, rate limit, outage) must not + # be mistaken for missing protection: the create path issues a PUT + # that would overwrite whatever is really there. + if ! echo "$current" | grep -qE "Branch not protected|\(HTTP 404\)"; then + log "ERROR: could not read branch protection for $repo: $(echo "$current" | head -n 1)" + echo -e "- ERROR: branch protection could not be read (see log)\n" + return + fi changes="- Branch protection: **not configured** -> will be created\n" if [ "$MODE" = "--apply" ]; then - apply_branch_protection "$repo" "$branch" "$effective" + if apply_branch_protection "$repo" "$branch" "$effective"; then + log "APPLIED branch protection for $repo" + else + changes="${changes}- ERROR: branch protection not applied (see log)\n" + fi fi echo -e "$changes" return - } + fi # Check each protection setting local current_reviews desired_reviews @@ -255,8 +297,11 @@ sync_branch_protection() { if [ "$drift" = "true" ]; then if [ "$MODE" = "--apply" ]; then - apply_branch_protection "$repo" "$branch" "$effective" - log "APPLIED branch protection for $repo" + if apply_branch_protection "$repo" "$branch" "$effective"; then + log "APPLIED branch protection for $repo" + else + changes="${changes}- ERROR: branch protection not applied (see log)\n" + fi else log "DRIFT detected in branch protection for $repo" fi @@ -308,7 +353,8 @@ apply_branch_protection() { local deletions deletions=$(echo "$effective" | jq -r '.branch_protection.allow_deletions') - gh api -X PUT "repos/$OWNER/$repo/branches/$branch/protection" --input <(cat < /dev/null 2>&1 + ) 2>&1); then + return 0 + fi + log "ERROR: could not apply branch protection for $repo: $(echo "$output" | head -n 1)" + return 1 } # Sync standard labels across repos.