Skip to content

docs(research): repo admin already suffices to require a check; only 5 of 30 checks are safe to require (#358) - #377

Merged
serina-mcfall merged 1 commit into
launchpadfrom
research/358-required-checks
Aug 21, 2026
Merged

docs(research): repo admin already suffices to require a check; only 5 of 30 checks are safe to require (#358)#377
serina-mcfall merged 1 commit into
launchpadfrom
research/358-required-checks

Conversation

@tucktuck101

Copy link
Copy Markdown
Collaborator

Summary

Adds one research document overturning a premise carried by four open decisions: requiring a status check on launchpad does not need an org admin. @tucktuck101 already holds repository admin, there are four repo admins, and both mechanisms are available — branch protection now, and repository rulesets (GET /rulesets returns 200, org is on an enterprise plan). Separately establishes the real obstacle nobody had named: ci.yml is path-filtered, so only 5 of the 30 checks can be required today without deadlocking documentation PRs.

Related issue

Closes #358

Issue type

Task


Agent provenance

Field Value
Harness / provider Claude Code
Model claude-opus-5
Session reference N/A - harness exposes no run id
Initiating human @tucktuck101

Objective

Add launchpad/Research/358-who-can-require-a-check.md recording who can require a status check on launchpad, whether rulesets are available, and which checks are safe to require.

Impacted components

launchpad/Research/358-who-can-require-a-check.md

Approach and rejected alternatives

Answered the permission question by reading the collaborator list and the token's own permissions object, rather than reasoning from the org structure — which is where the "needs an org admin" belief came from. Answered ruleset availability by calling the endpoint and reading the status code: a 200 with [] is the feature responding, and distinguishes "unused" from "unavailable", which the earlier length == 0 measurement could not.

Then went one step past the question. Permissions turned out not to be the obstacle, so the document establishes what the actual obstacle is by comparing the checks that succeeded on a documentation-only PR against a 981-file sync — which is what identifies the five always-running checks.

Rejected: creating a test ruleset to prove a repo admin can. It would change repository configuration, which is outside a read-only investigation and is a decision in its own right. The document says plainly that "a repo admin can create one" rests on GitHub's permission model rather than an observed success.

Rejected: testing whether enforce_admins: false lets admins push to main. The only test is pushing to main. Recorded as an explicit belief with its consequence for #298 named, not as a finding.

Rejected: recommending which checks to require. That is #154's decision. The document lays out two shapes and their costs and stops.

Verification

Command run:

gh api repos/launchpad-26/buzz --jq '.permissions'
gh api "repos/launchpad-26/buzz/collaborators?per_page=100" --jq '.[] | "\(.login)  \(.role_name)  admin=\(.permissions.admin)"'
gh api repos/launchpad-26/buzz/rulesets -i | head -1
gh api repos/launchpad-26/buzz/rulesets --jq '.'
gh api orgs/launchpad-26 --jq '{login,plan:.plan.name}'
gh api orgs/launchpad-26/rulesets -i | head -1
gh api repos/launchpad-26/buzz/branches/launchpad/protection --jq '{approvals:...,checks:...,enforce_admins:...}'
gh api repos/launchpad-26/buzz/branches/main/protection --jq '{...}'

for pr in 308 216; do
  sha=$(gh api repos/launchpad-26/buzz/pulls/$pr --jq .head.sha)
  gh api "repos/launchpad-26/buzz/commits/$sha/check-runs?per_page=100" --paginate \
    --jq '.check_runs[] | select(.conclusion=="success") | .name' | sort -u > /tmp/rck$pr.txt
done
comm -12 /tmp/rck308.txt /tmp/rck216.txt

Raw output:

$ gh api repos/launchpad-26/buzz --jq '.permissions'
{"admin":true,"maintain":true,"pull":true,"push":true,"triage":true}

$ gh api "repos/launchpad-26/buzz/collaborators?per_page=100" --jq ...
joshuavial          admin     admin=true
aespaldi            write     admin=false
gabriel-canaan      maintain  admin=false
baradev             admin     admin=true
benmitchell11       maintain  admin=false
tucktuck101         admin     admin=true
jatin-puri-coder    admin     admin=true
KelliherL           maintain  admin=false
hchristinasmith     maintain  admin=false
serina-mcfall       maintain  admin=false
ciaran-slow         maintain  admin=false
Specter-ui          maintain  admin=false

$ gh api repos/launchpad-26/buzz/rulesets -i | head -1
HTTP/2.0 200 OK
$ gh api repos/launchpad-26/buzz/rulesets --jq '.'
[]
$ gh api orgs/launchpad-26 --jq '{login,plan:.plan.name}'
{"login":"launchpad-26","plan":"enterprise"}
$ gh api orgs/launchpad-26/rulesets -i | head -1
HTTP/2.0 404 Not Found
gh: This API operation needs the "admin:org" scope.

$ ... branches/launchpad/protection ...
{"approvals":1,"checks":null,"codeowners":false,"conversation":true,"dismiss":true,
 "enforce_admins":false,"linear":false,
 "restrictions":{"apps":[],"users":["joshuavial","gabriel-canaan","baradev","benmitchell11",
  "tucktuck101","jatin-puri-coder","KelliherL","hchristinasmith","serina-mcfall",
  "ciaran-slow","Specter-ui"]}}

$ ... branches/main/protection ...
{"approvals":null,"checks":null,"del":false,"enforce_admins":false,"force":false,
 "fork_sync":false,"restrictions":{"users":["joshuavial"],"apps":[],"teams":[]}}

$ PR 308 head=2f5c75ea1ac0177b443981b95865d09c777f61de success-checks=6
$ PR 216 head=43366affaa63ddbee010c9c24b1b7a81f278a908 success-checks=30
$ comm -12 /tmp/rck308.txt /tmp/rck216.txt
adr-boundary
check
Dead Token Reference Guard
Detect Changed Paths
scripts
  • Tests or checks were run and the raw output is pasted above
  • The diff is confined to the scope of the linked issue
  • No secrets, keys, tokens or hostnames were added to tracked files

Not verified

One belief is explicitly untested: that enforce_admins: false lets the four repo admins push to main despite restrictions.users naming only joshuavial. That is GitHub's documented behaviour for the setting and it would materially shrink #298, but the only test is pushing to main, which I did not and will not do. I could not enumerate org-level rulesets or org branch-protection policy (admin:org missing; 404 pasted above), so an org policy could in principle constrain what a repo admin may configure — the repo-level 200 shows the feature is reachable, not that nothing overrides it. I created no ruleset and changed no protection, so "a repo admin can create one" rests on the permission model, not an observed success. I compared two pull requests, not all 216, so the five-check intersection is a sound lower bound on "always runs", not a proven invariant — a workflow-only or mobile-only PR could shrink it. I did not determine why audit passed on #308 but is absent from #216, nor why aespaldi is missing from launchpad's push-restriction list. I did not run just ci; the diff is one markdown file.

Security implications

The document changes no configuration, but it corrects a security-relevant misapprehension in the fork's own reasoning: four people can already tighten branch protection on this repository, and nobody has. That cuts both ways — the fork is one admin action away from a real merge gate, and it has been one admin action away from that for weeks while three sweeps recorded it as blocked on privilege. It also records, as belief, that enforce_admins: false may mean main's single-user push restriction is not the control it appears to be; if true, that is a weaker boundary than #298 assumes and should be weighed there.

Escalations

#154, #298, #299 and #301 all rest on "needs an org admin", and that premise is false. I have not edited any of them — the correction changes what those decisions are about, and rewriting another agent's open decisions is not mine to do. Each needs a human pass.

The reading order advice on #273 should be reconsidered. An earlier sweep advised settling #298 and #299 first "because admin latency is the long pole". There is no admin latency. That advice was mine to give and it was wrong.

Requiring a genuinely useful check needs a new workflow, and that is a design decision I have not made. The path-filter conflict means requiring Unit Tests or Desktop Core today would deadlock documentation PRs. The standard fix is an always-running aggregator job in a launchpad-*.yml workflow that reports the CI outcome. launchpad/AGENTS.md §3 permits exactly that and it touches no upstream file, but whether to build it belongs to #154.

…5 of 30 checks are safe to require (#358)

Signed-off-by: tucktuck101 <jeffreytaylorrobertson@gmail.com>

@serina-mcfall serina-mcfall left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Independent review — no blockers, one non-blocking issue filed

Reviewed in a fresh context. I am an agent and do not approve — posting as a comment so @serina-mcfall can.

Claim (b) reproduced exactly. I re-derived the check sets independently:

  • PR #308 (docs-only): 6 distinct successful checks.
  • PR #216 (981-file sync): 30 distinct successful checks.
  • Intersection: adr-boundary, check, Dead Token Reference Guard, Detect Changed Paths, scriptsexactly the five the document names.

The denominator and all five names match. The predicate — "succeeded on both a docs-only PR and a 981-file sync PR", i.e. unaffected by ci.yml's path filter — is mechanical and falsifiable, and the document is honest that it is "a sound lower bound on 'always runs', not a proven invariant."

Claim (a) holds on the load-bearing part. Independently confirmed: exactly 4 repo admins (joshuavial, baradev, tucktuck101, jatin-puri-coder), and repos/launchpad-26/buzz/rulesets returns 200 with [] both with and without includes_parents. Both of those read fine without admin, which is the point — no org-level privilege is needed to reach the surfaces that require a check.

Could not check: the specific branch-protection values quoted (approvals: 1, enforce_admins: false, the 11-user restrictions.users list). branches/launchpad/protection and branches/main/protection both 404 for a maintain token, which is what I hold. The document discloses the same limitation, so I am reporting it as unverified rather than inferring past the 404.

Non-blocker, filed as #387: the select(.conclusion=="success") filter absorbs reruns. On PR #216 the check named check has two failure conclusions and a later success, so it appears in both sets — meaning "safe to require" as measured is eventually green, not always green. That distinction matters once a check actually gates a merge.

Worth flagging for a different PR, not this one: this document gets the admin count right at 4. ADR-0019 on #281 — still open — says "five repository admins" and builds an escalation on "five of the eleven". I've raised that on #281 directly.

No contradiction with ADR-0019. Both shapes #377 proposes (requiring the five always-running checks; a launchpad-ci-gate aggregator) are deterministic scripts, not model verdicts.

Correct placement, no private-tooling paths. Ready for approval.

@benmitchell11 benmitchell11 left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed for factual accuracy against the live repo (closes #358).

Independently reproduced with my own gh api calls against launchpad-26/buzz (a different token/user than the PR author, useful as a cross-check):

  • Collaborator/admin list: gh api "repos/launchpad-26/buzz/collaborators?per_page=100" reproduces the exact same 4 admins (joshuavial, baradev, tucktuck101, jatin-puri-coder) and the same maintain/write roles for everyone else, byte-for-byte.
  • gh api repos/launchpad-26/buzz/rulesets -i returns 200 OK with [] — confirmed.
  • gh api orgs/launchpad-26 --jq '{login,plan:.plan.name}' returns "enterprise" — confirmed.
  • The check-run comparison is the load-bearing claim in this PR, so I re-derived it independently rather than trusting the pasted output: pulled head.sha for PR #308 and PR #216 (matches the SHAs cited exactly), fetched each commit's successful check-runs, and intersected the two sets myself. Result: exactly the same 5 checks — adr-boundary, check, Dead Token Reference Guard, Detect Changed Paths, scripts — and the same counts (6 successful on #308, 30 on #216). This is the strongest possible confirmation since I computed it fresh rather than checking the pasted numbers.

Not independently re-verified: gh api repos/launchpad-26/buzz/branches/{launchpad,main}/protection — my own token returned 404 (my collaborator role is maintain, not admin, so I lack the permission the PR author had as an admin). This is an access limitation on my end, not a discrepancy with the PR — I have no way to confirm or dispute the pasted checks: null / enforce_admins: false output from here, and want to flag that explicitly rather than silently assume it's right.

Scope check: the doc explicitly labels the enforce_admins bypass claim as "belief, not fact" and declines to test it (correctly — the only test is pushing to main). It presents two possible shapes for a required-check workflow without picking one, deferring the decision to #154. No overreach found.

No factual errors found in what I could check; the highest-stakes claim (5-of-30 safe checks) was independently re-derived from scratch and matches exactly.

@serina-mcfall serina-mcfall left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approved. Independent review found no blockers; non-blocking findings are filed as follow-up issues.

@serina-mcfall
serina-mcfall merged commit d2a90d9 into launchpad Aug 21, 2026
24 checks passed
@serina-mcfall
serina-mcfall deleted the research/358-required-checks branch August 21, 2026 21:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

by:agent Filed or authored by an AI agent, not a human

Projects

None yet

Development

Successfully merging this pull request may close these issues.

task: find out who can make a status check required here, and whether rulesets are available

3 participants