proposal: lexicon/knot/appview: add branch rules #454

Proposed Change#

Add a new lexicon record for branch rules (sh.tangled.repo.branchRule ?)
Add xrpc CRUD endpoints for managing branch rules
Create user flow for editing branch rules in repo settings (///settings?tab=branchRules)
Update branch management xrpc calls to enforce branch rules

Motivation#

Github/Codeberg/Gitlab support branch rules for configuring who's allowed to merge/force push to which branches or how many approvals are required from which users. These rules give users more confidence that their default branch is in a good state and make it easier for repo owners to delegate permissions.

Implementation Plan#

Create lexicons/repo/branchRule.json with:

{
  "lexicon": 1,
  "id": "sh.tangled.repo.branchRule",
  "defs": {
    "main": {
      "type": "record",
      "key": "tid",
      "record": {
        "required": ["repo", "name", "status", "branchPatterns", "blockedActions",  "createdAt"],
        "properties": {
          "repo": {
            "type": "string",
            "format": "at-uri",
            "description": "Repo to apply rule to"
          },
          "name": {
            "type": "string",
            "description": "Descriptive name for branch rule"
          },
          "status": {
            "type": "string",
            "enum": ["enabled", "disabled"],
            "description": "Whether this rule is actively being applied to the repo or not"
          },
          "branchPatterns": {
            "type": "array",
            "items": {
              "type": "string"
            },
            "description": "Branch names matching these patterns will have the rule applied to them"
          },
          "blockedActions": {
            "type": "array",
            "items": {
              "type": "string"
            },
           "description": "Actions that are blocked by the rule"
          },
          "excludedDids": {
            "type": "array",
            "items": {
              "type": "string",
              "format": "did"
            },
            "description": "Rule does not impact actions by these DIDs"
          }
        }
      }
    }
  }
}

Create xrpc crud endpoints on knot
- knotserver/xrpc/list_branch_rule.go
- knotserver/xrpc/create_repo_branch_rule.go
- knotserver/xrpc/delete_repo_branch_rule.go
- knotserver/xrpc/change_status_repo_branch_rule.go (not sure if this should be two files)
Create flow for creating branch rules in settings
- appview/pages/templates/repo/settings/branch_rules.html
- Update appview/repo/settings.go to create new tab
  - Probably just copy most of the hooks UX, so a list and a button that creates a modal with a form
Update delete/merge xrpc calls to respect branch rules
- Not 100% on this but I think after the x.Enforcer.IsPushAllowed call we need another helper for checking if branch rules allow this?
- RBAC stuff seems to be in sqlite, but assuming branch rules are in PDSs we need to index them in the app view/sqllite and retrieve from there?

Open Questions#

Not sure exactly where this data should live. This proposal adds them as atproto records but they could just as easily be in markdown files under .tangled/branch_rules/*. Maybe that has weird edge cases where the owner can lock themselves out and prevent them from making further changes?

Assuming the atproto record is the way to go, I'm not really confident that the above lexicon is correct.

Also unsure about DIDs for excluded users, emails or keys could be more robust.

Haven't contributed before so hopefully this is sufficient, If so, I can take a stab at implementing. This is mostly an attempt at fleshing out issue 432, so if it's better to keep things organized I can close this and add this as a comment there.

create table if not exists branch_rules ( repo text not null, name text not null, active bool not null, branch_patterns text not null, -- json array or tab separated blocked_actions text not null -- json array or tab separated excluded_dids text -- json array or tab separated )

I think they could fit into the repo record. The record is kinda light at the moment, so having a few rules would probably dominate the size of the record. That's likely fine but I assumed these separate concepts should be split out.

nixpkgs has ~10k contributors on github, but I don't think all those users have been added to branch rules. Likely there's just a rule that requires at least one member of some core maintainer list to approve merges. Some implementations allow rules to apply to groups, so if group member records was separated, that could also keep the rules small.

My original thought of a separate branchRule record was because sh.tangled.repo.collaborator and sh.tangled.repo.setDefaultBranch are separate from the main repo record and branch rules felt similar.

I don't have strong opinions about choosing between:

Adding them to the repo record
New record type
Data lives in knot db