Skip to content

feat(checkchange): bypass on publish_<ts> branches for repo admins#7537

Open
janechu wants to merge 3 commits into
mainfrom
users/janechu/manual-bump-tolerance
Open

feat(checkchange): bypass on publish_<ts> branches for repo admins#7537
janechu wants to merge 3 commits into
mainfrom
users/janechu/manual-bump-tolerance

Conversation

@janechu
Copy link
Copy Markdown
Collaborator

@janechu janechu commented May 28, 2026

Description

Replaces the bare beachball check invocation in npm run checkchange with a thin Node wrapper (build/scripts/checkchange.mjs) that skips the change-file requirement only when both of these hold for the current run:

  1. Branch name matches ^publish_\d+$ — beachball's own publish-branch convention, generated as 'publish_' + String(new Date().getTime()) in beachball/lib/commands/publish.js.
  2. Actor has the admin role on microsoft/fast — verified live via the GitHub REST API: GET /repos/microsoft/fast/collaborators/<login>/permission. The wrapper resolves the actor login from $GITHUB_ACTOR in CI (falling back to gh api user --jq .login locally) and reads the API token from $GITHUB_TOKEN$GH_TOKENgh auth token.

Maintainership lives in the GitHub repo settings — adding or revoking the bypass for someone happens by changing their role on microsoft/fast, not by editing this script.

When the bypass fires, the wrapper writes a multi-line banner to stdout naming the branch, the actor, the resolved role, and the source of each so reviewers can spot the bypass in CI logs and verify the diff really is a version bump.

This PR supersedes the now-closed #7536, which used diff-content inspection ("the only changed line is "version"") to auto-exclude packages. That approach was silently coupled to file contents and was abandoned in favor of the explicit, declarative branch + role check here.

Reviewer notes

  • Why both conditions? Either alone is too easy to trigger. Branch-name alone would let any contributor bypass by renaming their branch. Role-only would bypass every PR from a maintainer, including normal source-code changes. The AND is the safety net.
  • Why ^publish_\d+$ and not a broader pattern? Matching beachball's exact convention keeps the bypass surface narrow. Maintainers create the branch explicitly with git checkout -b "publish_$(node -p 'Date.now()')". Patterns like ^publish[_/] would catch plausible feature branches by accident.
  • Why admin (and not maintain/write)? The bypass disables one of the few automated guards on bump correctness; only people empowered to publish should be able to skip it.
  • Why GitHub API instead of a hardcoded allowlist? A list of logins in the script drifts as the team changes. The API check sources the truth from the repo's collaborator settings, where it's auditable in GitHub and managed through normal access reviews.
  • Fail-closed everywhere. Missing token, missing login, API error, non-admin response, network timeout — all paths fall through to beachball check and log why. The bypass is granted only when the API responds with "admin".
  • Workflow change: .github/workflows/ci-validate-pr.yml now exposes GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} to the npm run checkchange step. The /collaborators/.../permission endpoint requires push access to the repo, which the default workflow token has for same-repo PRs but not for fork PRs — fork PRs will silently fall through to beachball (and contributors don't have admin anyway, so behavior is identical).
  • What's NOT changed: Beachball's ignorePatterns, the postbump Cargo-sync hook, Lefthook, and biome configs are all untouched.

Manual verification

Run from a fresh clone of this branch with npm ci:

# Setup Expected Result
A Clean tree, no env vars (branch != publish_*) Short-circuit, beachball runs, exits 0
B Edit packages/fast-element/package.json version + GITHUB_HEAD_REF=publish_<n> GITHUB_ACTOR=janechu (admin) Bypass banner, exits 0
C Same edit + GITHUB_HEAD_REF=publish_<n> GITHUB_ACTOR=chrisdholt (admin) Bypass banner, exits 0
D Same edit + GITHUB_HEAD_REF=publish_<n> GITHUB_ACTOR=dependabot (no role) Wrapper logs "role 'none'", beachball runs, exits 1
E Same edit + publish_<n> + admin actor + no token anywhere Wrapper logs "no token available", beachball runs, exits 1
F Same edit + publish_<n> + admin actor + invalid $GITHUB_TOKEN Wrapper logs "HTTP 401", beachball runs, exits 1
G Local fallback: branch = publish_<n> checked out, no env vars, gh auth authenticated as janechu Wrapper resolves login + token via gh, API returns admin, bypass banner, exits 0

Live API probe used during design (verifying expected roles):

gh api repos/microsoft/fast/collaborators/janechu/permission     -> admin
gh api repos/microsoft/fast/collaborators/chrisdholt/permission  -> admin
gh api repos/microsoft/fast/collaborators/dependabot/permission  -> none

Documentation

  • CONTRIBUTING.md — new Manual version bumps section spelling out the bypass conditions (branch pattern + admin role), the three use cases (full bump PR, hotfix override, paired Rust/npm sync recovery), the recommended branch-naming recipe, and reviewer expectations.
  • CONTRIBUTING.md (Publishing section) — the bump-PR walkthrough now uses publish_$(node -p 'Date.now()') so the existing flow benefits from the bypass.
  • .github/skills/shipping/SKILL.mdWhen change files are NOT required now references the admin-role check.
  • .github/workflows/ci-validate-pr.yml — passes GITHUB_TOKEN to the checkchange step.

Checklist

  • Code changes meet the contributor guide.
  • Changes are based on the main branch.
  • Documentation has been added/updated.
  • A change/ file describing the changes has been added (N/A — this PR doesn't touch packages/* and is itself a tooling change; the wrapper script lives under build/scripts/).

janechu and others added 3 commits May 28, 2026 13:52
…ntainers

Wrap the `checkchange` npm script with a new
`build/scripts/checkchange.mjs` that skips `beachball check` only
when BOTH of the following hold for the current run:

  1. The branch name matches `^publish_\d+$` — beachball's own
     publish-branch convention from
     `beachball/lib/commands/publish.js` (`'publish_' +
     String(new Date().getTime())`).
  2. The actor ($GITHUB_ACTOR in CI; HEAD commit author email
     locally) is on a short maintainer allowlist defined inline in
     the wrapper.

Both conditions are deliberately opt-in: a contributor renaming
their branch `publish_1234567890` is not enough, nor is being an
allowlisted maintainer on a regular feature branch. When the
bypass fires, the wrapper writes a multi-line banner naming the
branch, actor, and source of each so reviewers can spot it in CI
logs.

Documentation:
- CONTRIBUTING.md: new 'Manual version bumps' section spelling out
  the bypass conditions, hotfix/Rust-sync use cases, and reviewer
  expectations. The existing bump-PR flow now also uses the
  `publish_<timestamp>` branch naming so it benefits from the
  bypass.
- .github/skills/shipping/SKILL.md: 'When change files are NOT
  required' now mentions the bypass with a cross-reference.
- beachball.config.js: short comment pointing maintainers at the
  wrapper.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Keep only the GitHub noreply addresses; the corporate email isn't
needed for the bypass and reduces the public footprint.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…role

Replace the inline allowlist (hardcoded GitHub logins + git emails) in
`build/scripts/checkchange.mjs` with a live GitHub REST API check:
the wrapper only bypasses `beachball check` when the actor has the
`admin` role on `microsoft/fast`, verified via
`GET /repos/microsoft/fast/collaborators/<login>/permission`.

Wrapper behavior:

- Short-circuits if the branch does not match `^publish_\d+$` so
  the API is never called on regular feature branches.
- Resolves the actor login: $GITHUB_ACTOR in CI; falls back to
  `gh api user --jq .login` locally.
- Resolves the API token: $GITHUB_TOKEN -> $GH_TOKEN ->
  `gh auth token`.
- Fail-closed on every short-circuit before the bypass (no login,
  no token, network error, non-admin role) — the wrapper logs the
  reason and falls through to `beachball check`.
- Bypasses only when the API responds with `"admin"`, then prints
  a multi-line banner naming branch, actor, resolved role, and the
  source of each.

Workflow:

- `.github/workflows/ci-validate-pr.yml` exposes
  `GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}` to the
  `npm run checkchange` step so the wrapper can call the API.

Documentation:

- `CONTRIBUTING.md > Manual version bumps` describes the admin-role
  check, token sources, and that maintainer changes happen in repo
  settings rather than via PR.
- `.github/skills/shipping/SKILL.md` updated to match.

Maintenance: adding or removing a bypass-eligible maintainer happens
by changing their role on `microsoft/fast` (Settings ->
Collaborators and teams); no edit to this script required.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@janechu janechu changed the title feat(checkchange): bypass on publish_<ts> branches by allowlisted maintainers feat(checkchange): bypass on publish_<ts> branches for repo admins May 28, 2026
@janechu janechu marked this pull request as ready for review May 28, 2026 21:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant