docs: document the repository settings behind the human gates - #169
docs: document the repository settings behind the human gates#169alejandro-llanes wants to merge 1 commit into
Conversation
Facility claims agents never approve, never merge, and never push to protected branches. Part of that is enforced by the product; the rest is GitHub repository configuration, and the docs never said which was which. A repository could run the whole loop, look correct, and still let an automated actor satisfy a human gate. Add a section to the security reference, immediately after the invariants it qualifies, stating per setting what it must be, whether Facility or the setting enforces it, and which advertised gate a skip breaks. It covers Actions approving pull requests, the default GITHUB_TOKEN scope, default-branch protection and required checks, and who may merge or bypass. Record which settings are readable through the API, and note what `facility doctor --github` does not check yet: the branch-protection response it already fetches is never parsed, and a branch protected by a ruleset is reported as a failure. Link it from kickstart and the existing-repo guide before the first agent run, and from the CLI reference where `doctor` is described. Closes theam#48
adrian-lorenzo
left a comment
There was a problem hiding this comment.
Thanks for putting this together!
The checklist is useful, but the API section cannot currently verify the bypass guarantee it asks operators to enforce. Under classic branch protection, restrictions covers who can push; pull-request bypass actors live in required_pull_request_reviews.bypass_pull_request_allowances. For rulesets, /rules/branches/{branch} returns active rules but not the ruleset’s bypass_actors; those require fetching each ruleset, and GitHub only exposes them when the caller has sufficient access.
Please point readers to the correct fields and endpoints, make the result explicitly unknown when permissions hide bypass actors, and clarify that admin:org is the classic-token requirement—fine-grained and App tokens use the organization Administration permission.
Once those distinctions are reflected in the guidance, this will be ready to approve.
What changes
apps/docs/docs/reference/security.mdgains a Repository settings that back the gatessection, placed immediately after The invariants that never move — the paragraph it
qualifies. For each setting it states the required value, whether Facility or the setting
enforces it, and which advertised gate a skip breaks.
It covers the four areas the issue asks for — Actions approving pull requests, the default
GITHUB_TOKENscope, default-branch protection and required checks, and who may merge orbypass — plus stale-approval dismissal, which matters here specifically because the
address-review agent pushes commits to a pull request branch after a human has reviewed it.
Two subsections follow: which settings are readable through the GitHub API (endpoint and
field), and what
facility doctor --githubdoes not check yet, recorded as a follow-uprather than implemented here.
Three guides link to it before the first agent run:
guides/kickstart.md/architectparagraphguides/existing-repo.mdreference/cli.mddoctor --run-guards --githubis described, to state the boundary of what it checksDocumentation only. No code, no behaviour change.
Why
Closes #48.
The gap is easy to confirm: grepping the repository for
approve pull requests,can_approve,approving_review_count,enforce_adminsorrestrictionsreturns nothing.The setting Allow GitHub Actions to create and approve pull requests is not mentioned
anywhere, and the existing guidance ("protect the default branch, require a human review")
names no setting and assigns no ownership.
That matters because the invariants lean on repository configuration more than the docs
admit:
facility-review.ymlstates it "never approves or merges" — but that is a prompt, andthe job runs with
pull-requests: write. The hard stop iscan_approve_pull_request_reviews: false.facility-address-review.ymlandfacility-doctor.ymlholdcontents: write.Contents: Read and write, so a bypass entry onrequired pull requests converts that into merge authority.
So a repository can run the whole loop, look correct, and still let an automated actor
satisfy a human gate — which is exactly what the issue describes.
Verification
Every API claim in the new section was probed against the live GitHub API rather than
written from memory:
GET /repos/{owner}/{repo}/actions/permissions/workflow{"default_workflow_permissions":"read","can_approve_pull_request_reviews":false}GET /orgs/{org}/actions/permissions/workflow403without org admin and theadmin:orgscopeGET /repos/{owner}/{repo}/branches/{branch}/protection404 "Branch not protected"as an admin on an unprotected branch, but404 "Not Found"without admin — two different meanings, same status codeGET /repos/{owner}/{repo}/rules/branches/{branch}Those last two are why the
doctorfollow-up list is specific: the command currently treatsany non-zero exit as a failure, so a branch protected by a ruleset and a caller who simply
lacks admin both report the same way as a genuinely unprotected branch — and the protection
response it already fetches is never parsed.
Commands run:
The docs build runs under
onBrokenLinks: "throw", so the four new relative links areverified rather than assumed. Because the guides link to an anchor and
onBrokenAnchorsonly warns, I checked the built output directly instead of trusting the slug rule:
id="repository-settings-that-back-the-gates"is present inreference/security/index.html,and
guides/kickstart,guides/existing-repoandreference/cliall emit links pointing atit.
On
pnpm verifyIt completed Lint, Typecheck, the clean cache-disabled workspace build, isolated Postgres,
both test-database recreations, and the critical integration tests, then exited
1in thefinal stage on a single test:
This is pre-existing and unrelated to this pull request. I confirmed it rather than
assuming: checking out unmodified
mainatae68401and running that file alone reproducesthe same failure, same test, same timeout —
It looks environment-dependent (a Unix-socket upgrade handshake that never receives its
101), so it may well be green in CI — flagging it only so the failing run below isn'tmistaken for something this branch introduced. Nothing in this diff is reachable from
runner/; the change is four Markdown files underapps/docs/.pnpm verifypasses locally — runs green except the pre-existingdocker-proxytimeout documented above, which also fails on cleanmain