Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
45 changes: 37 additions & 8 deletions .github/settings.yml
Original file line number Diff line number Diff line change
Expand Up @@ -24,15 +24,12 @@ repository:
delete_branch_on_merge: true
allow_update_branch: true

# Branch protection applied to the default branch of every repo. Maps to
# PUT /repos/{owner}/{repo}/branches/{name}/protection.
# Branch protection for public repos is handled via GitHub Rulesets (see `rulesets:` below),
# which support bypass actors so org owners and repo admins can force-push without extra steps
# — just `git push origin main --force`. GitHub records the bypass in the audit log.
#
# `enforce_admins: false` is deliberate: with a single admin/code-owner you
# would otherwise deadlock on your own PRs. Admins can bypass; the rules
# apply to everyone else (and to agents).
#
# `require_code_owner_reviews: true` is a no-op on repos that don't have a
# CODEOWNERS file — harmless. It enforces real review on swimblocks/.github.
# Legacy branch protection (this `branches:` block) is still attempted for private repos
# but is expected to fail on GitHub Free; the reconciler skips it gracefully.
branches:
- name: main
protection:
Expand All @@ -47,3 +44,35 @@ branches:
allow_force_pushes: false
allow_deletions: false
required_conversation_resolution: true

# Rulesets for public repos. GitHub Free supports repository-level rulesets on public repos.
# apply-settings.py creates/updates these and removes the legacy branch protection when the
# repo is public. See https://github.com/swimblocks/.github/issues/11
rulesets:
- name: swimblocks-default
target: branch
enforcement: active
bypass_actors:
# Org owners and repo admins bypass all rules — break-glass for history rewrites etc.
# No extra steps: just `git push origin main --force`. Recorded in the audit log.
- actor_id: 1
actor_type: OrganizationAdmin
bypass_mode: always
- actor_id: 5 # Admin (built-in repository role ID)
actor_type: RepositoryRole
bypass_mode: always
conditions:
ref_name:
include: ["~DEFAULT_BRANCH"]
exclude: []
rules:
- type: deletion
- type: non_fast_forward # prevents force push for non-admins
- type: required_linear_history
- type: pull_request
parameters:
required_approving_review_count: 1
require_code_owner_review: true
dismiss_stale_reviews_on_push: true
require_last_push_approval: false
required_review_thread_resolution: true
20 changes: 14 additions & 6 deletions AGENTS.md
Original file line number Diff line number Diff line change
Expand Up @@ -57,35 +57,43 @@ For **every** change:
Do not push directly to `main`. Do not use `--no-verify` or otherwise skip hooks. Create new
commits rather than amending already-pushed ones.

## 4. Quality gates (Python repos)
## 4. Code comments

- **Aspirational / TODO comments must link to a GitHub issue.** A note that says "out of
scope" or "should add X later" is just a wish. File an issue and reference it inline:
`# See https://github.com/swimblocks/<repo>/issues/N — the cheap stopgap below covers …`.
Reviewers will push back on bare TODOs. Full rationale in
[CONTRIBUTING.md](CONTRIBUTING.md#code-comments).

## 5. Quality gates (Python repos)

- Lint: `ruff check .` (config in each repo's `pyproject.toml`). Auto-fix with `--fix`.
- Tests: `pytest -q`. Add coverage for new behaviour and regressions.
- CI: each repo's `ci.yml` calls
[`swimblocks/.github/.github/workflows/reusable-python-ci.yml@main`](.github/workflows/reusable-python-ci.yml).

## 5. Dependencies
## 6. Dependencies

- `requirements.txt`: direct runtime + test deps only, UTF-8, `>=` minimums. No full `pip
freeze` output, no transitive pins.
- `requirements-dev.txt`: layered on top, adds dev-only tools (e.g. `ruff`).
- Dependabot config lives **per repo** at `.github/dependabot.yml` (no org-wide inheritance).

## 6. Secrets, data, and PII
## 7. Secrets, data, and PII

- Never commit `.env`, credentials, or service-account keys (all gitignored).
- Never commit personal data — officials' names/emails, club contact lists, REMS exports.
Treat such CSV/PDF as local sample data and gitignore it, or scrub before commit.
- Report any exposure privately via the repo's Security tab, not a public issue.

## 7. Creating a new repo
## 8. Creating a new repo

Use [`scripts/create-repo.sh`](scripts/create-repo.sh) — **never** the GitHub UI. It applies
the canonical settings from [`.github/settings.yml`](.github/settings.yml) automatically.
Drift on existing repos is healed by the scheduled
[`reconcile-repo-defaults.yml`](.github/workflows/reconcile-repo-defaults.yml) workflow.

## 8. Other agent-specific files
## 9. Other agent-specific files

Each repo carries thin pointer files so any tool finds the right context:

Expand All @@ -95,7 +103,7 @@ Each repo carries thin pointer files so any tool finds the right context:

Edit `AGENTS.md`. The pointer files don't need changes.

## 9. When in doubt
## 10. When in doubt

- **Don't widen scope.** If an issue says "fix X," fix X. Don't refactor unrelated code.
- **Don't generalise away the Canada specifics.** They are load-bearing.
Expand Down
68 changes: 56 additions & 12 deletions CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,11 @@ explicitly overrides it.
- **Docs travel with code.** User-facing changes update the README / docs in the same PR.
New code carries clear docstrings.
- **Simplicity.** If a change adds complexity, consider refactoring for clarity instead.
- **Capture review-derived rules here.** When a code review (or any maintainer conversation)
yields a generalizable rule about how SwimBlocks repos should be developed, fold it into
this file via a PR closing a tracking issue. A verbal "I'll remember" doesn't bind future
contributors — agents especially. The rules below grew this way; new ones should land the
same way.

## Workflow: issue → branch → PR → squash-merge

Expand All @@ -35,9 +40,37 @@ explicitly overrides it.
deletes the branch. The squash commit message should carry the meaningful detail, not just
the PR title.

### Solo-admin merge path

Until a second code owner exists, the author satisfying `require_code_owner_reviews` on their
own PR is mathematically impossible (GitHub blocks self-approval). Two options, both relying
on admin bypass in the repo ruleset (see [`settings.yml`](.github/settings.yml)):

- **Web UI:** on the PR page, scroll past the standard "Squash and merge" button to the
"Merge without waiting for requirements to be met (bypass branch protections)" link, and
confirm.
- **CLI:** `gh pr merge <N> --repo swimblocks/<repo> --admin --squash --delete-branch`.

This is the deliberate steady state for a single-admin org. Revisit when a second code owner
joins.

Direct pushes to `main` are discouraged; go through a PR. Never use `--no-verify` or bypass
signing. Create new commits rather than amending already-pushed ones.

### Code comments

- **TODO / aspirational comments must link to a GitHub issue.** A comment that says "out of
scope for now" or "should add X later" is just a wish; the next reader (human or agent)
has no way to act on it. Open a tracking issue and reference it inline:

```python
# See https://github.com/swimblocks/<repo>/issues/42 for the proper solution;
# the regex below is the cheap stopgap.
```

Same rule for shell, YAML, etc. If you don't have an issue number yet, file one before the
PR lands. Reviewers will (and have!) push back on bare TODOs.

## Quality gates

- **Lint:** Python repos use [ruff](https://docs.astral.sh/ruff/) with `select = ["E","F","I","W"]`.
Expand Down Expand Up @@ -85,20 +118,31 @@ of truth**; the table below is a human-readable summary.

### Branch protection (same source of truth)

`settings.yml` also carries a `branches:` section that the reconciler applies to every repo's
default branch via `PUT /repos/.../branches/main/protection`. Today that's:
**Public repos** use GitHub Rulesets (`rulesets:` block in `settings.yml`). Rulesets support
bypass actors, so org owners and repo admins can force-push when genuinely needed (see
[Force-push break-glass](#force-push-break-glass) below). The ruleset enforces:

- Pull request required (1 approving review, code-owner review required, stale reviews
dismissed on new pushes).
- Linear history required (i.e. squash-only — pairs with the merge-method settings above).
- No force pushes, no deletions, no unresolved conversations at merge time.
- `enforce_admins: false` — the admin (you) keeps an override so a single-admin org doesn't
deadlock approving its own PRs. Tighten this once another code owner exists.

> **Private-repo caveat:** GitHub Free does not allow branch protection on private repositories.
> The reconciler will skip and report this as a known limitation; the repo remains aligned on
> all the merge-method fields. Either upgrade the plan or flip the repo to public to enable
> protection.
dismissed on new pushes, all threads resolved).
- Linear history required (squash-only — pairs with the merge-method settings above).
- No force pushes or deletions for non-admins.

**Private repos** fall back to the legacy `branches:` protection block. GitHub Free does not
allow branch protection on private repositories, so the reconciler skips it and reports it as a
known limitation; the repo remains aligned on all the merge-method fields. Either upgrade the
plan or flip the repo public to enable protection.

### Force-push break-glass

Org owners and repo admins are listed as bypass actors in the ruleset, so a force-push
(needed for a history rewrite, purging a file that shouldn't have been committed, etc.) is just:

```bash
git push origin main --force
```

No UI changes, no disabling protection first. GitHub records the bypass in the org audit log.
This only works on public repos (rulesets); private repos have no protection at all on Free.

### How it's enforced

Expand Down
116 changes: 94 additions & 22 deletions scripts/apply-settings.py
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,58 @@ def verify(repo: str, repo_block: dict) -> bool:
}


def get_repo_visibility(repo: str) -> str:
"""Return 'public' or 'private' for the repo."""
out = subprocess.run(
["gh", "api", f"repos/{repo}", "--jq", ".visibility"],
check=True, capture_output=True, text=True,
).stdout.strip().lower()
return out


def list_rulesets(repo: str) -> list[dict]:
out = subprocess.run(
["gh", "api", f"repos/{repo}/rulesets"],
check=True, capture_output=True, text=True,
).stdout
return json.loads(out)


def apply_ruleset(repo: str, ruleset: dict) -> None:
"""Create or update a named ruleset (idempotent by name)."""
existing = list_rulesets(repo)
match = next((r for r in existing if r["name"] == ruleset["name"]), None)
if match:
cmd = ["gh", "api", "-X", "PUT",
f"repos/{repo}/rulesets/{match['id']}",
"--input", "-"]
else:
cmd = ["gh", "api", "-X", "POST",
f"repos/{repo}/rulesets",
"--input", "-"]
subprocess.run(cmd, input=json.dumps(ruleset), text=True,
check=True, stdout=subprocess.DEVNULL)


def verify_ruleset(repo: str, ruleset_name: str) -> bool:
existing = list_rulesets(repo)
found = any(r["name"] == ruleset_name for r in existing)
marker = "OK " if found else "FAIL"
print(f" {marker} ruleset '{ruleset_name}': {'present' if found else 'missing'}")
return found


def delete_legacy_protection(repo: str, branch: str) -> None:
"""Remove legacy branch protection (superseded by ruleset on public repos)."""
result = subprocess.run(
["gh", "api", "-X", "DELETE",
f"repos/{repo}/branches/{branch}/protection"],
capture_output=True, text=True,
)
if result.returncode == 0:
print(f" OK removed legacy branch protection on '{branch}' (superseded by ruleset)")


def apply_branch_protection(repo: str, branch: str, protection: dict) -> None:
"""PUT /repos/{owner}/{repo}/branches/{branch}/protection."""
payload = {k: v for k, v in protection.items() if k in PROTECTION_KEYS}
Expand Down Expand Up @@ -168,34 +220,54 @@ def main(argv: list[str]) -> int:
branches_block = (settings.get("branches") or [])
args = patch_args(repo_block)

rulesets_block = settings.get("rulesets") or []

failures: list[str] = []
for repo in argv[1:]:
print(f"=== {repo} ===")
apply(repo, args)
if not verify(repo, repo_block):
failures.append(repo)
# Branch protection (applied per branch entry — currently just `main`).
# Failures here are not fatal to the loop: a private repo on a Free
# plan, a default branch with a different name, or a tier upgrade
# required are all real, recoverable conditions we want to surface
# without abandoning the rest of the run.
for entry in branches_block:
branch = entry.get("name")
protection = entry.get("protection") or {}
if not branch or not protection:
continue
try:
apply_branch_protection(repo, branch, protection)
except subprocess.CalledProcessError as e:
print(f" SKIP branches.{branch}: PUT failed (exit {e.returncode}). "
f"Likely cause: private repo on a plan without branch "
f"protection, or the branch doesn't exist yet.")
if repo not in failures:
failures.append(repo)
continue
if not verify_branch_protection(repo, branch, protection):
if repo not in failures:
failures.append(repo)

is_public = get_repo_visibility(repo) == "public"

if is_public and rulesets_block:
# Public repos: rulesets with bypass actors for admin force-push break-glass.
# Legacy branch protection is removed so it can't silently override the ruleset.
for ruleset in rulesets_block:
try:
apply_ruleset(repo, ruleset)
if not verify_ruleset(repo, ruleset["name"]):
if repo not in failures:
failures.append(repo)
except subprocess.CalledProcessError as e:
print(f" SKIP ruleset '{ruleset.get('name')}': apply failed "
f"(exit {e.returncode}).")
if repo not in failures:
failures.append(repo)
for entry in branches_block:
branch = entry.get("name")
if branch:
delete_legacy_protection(repo, branch)
else:
# Private repos: attempt legacy branch protection (expected to fail on Free plan).
for entry in branches_block:
branch = entry.get("name")
protection = entry.get("protection") or {}
if not branch or not protection:
continue
try:
apply_branch_protection(repo, branch, protection)
except subprocess.CalledProcessError as e:
print(f" SKIP branches.{branch}: PUT failed (exit {e.returncode}). "
f"Likely cause: private repo on a plan without branch "
f"protection, or the branch doesn't exist yet.")
if repo not in failures:
failures.append(repo)
continue
if not verify_branch_protection(repo, branch, protection):
if repo not in failures:
failures.append(repo)
if failures:
print(f"\nFAIL: drift remains on {', '.join(failures)}", file=sys.stderr)
return 1
Expand Down