feat(checkchange): bypass on publish_<ts> branches for repo admins#7537
Open
janechu wants to merge 3 commits into
Open
feat(checkchange): bypass on publish_<ts> branches for repo admins#7537janechu wants to merge 3 commits into
janechu wants to merge 3 commits into
Conversation
…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>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Replaces the bare
beachball checkinvocation innpm run checkchangewith a thin Node wrapper (build/scripts/checkchange.mjs) that skips the change-file requirement only when both of these hold for the current run:^publish_\d+$— beachball's own publish-branch convention, generated as'publish_' + String(new Date().getTime())inbeachball/lib/commands/publish.js.adminrole onmicrosoft/fast— verified live via the GitHub REST API:GET /repos/microsoft/fast/collaborators/<login>/permission. The wrapper resolves the actor login from$GITHUB_ACTORin CI (falling back togh api user --jq .loginlocally) and reads the API token from$GITHUB_TOKEN→$GH_TOKEN→gh 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
^publish_\d+$and not a broader pattern? Matching beachball's exact convention keeps the bypass surface narrow. Maintainers create the branch explicitly withgit checkout -b "publish_$(node -p 'Date.now()')". Patterns like^publish[_/]would catch plausible feature branches by accident.admin(and notmaintain/write)? The bypass disables one of the few automated guards on bump correctness; only people empowered to publish should be able to skip it.beachball checkand log why. The bypass is granted only when the API responds with"admin"..github/workflows/ci-validate-pr.ymlnow exposesGITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}to thenpm run checkchangestep. The/collaborators/.../permissionendpoint 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).ignorePatterns, thepostbumpCargo-sync hook, Lefthook, and biome configs are all untouched.Manual verification
Run from a fresh clone of this branch with
npm ci:publish_*)packages/fast-element/package.jsonversion +GITHUB_HEAD_REF=publish_<n> GITHUB_ACTOR=janechu(admin)GITHUB_HEAD_REF=publish_<n> GITHUB_ACTOR=chrisdholt(admin)GITHUB_HEAD_REF=publish_<n> GITHUB_ACTOR=dependabot(no role)publish_<n>+ admin actor + no token anywherepublish_<n>+ admin actor + invalid$GITHUB_TOKENpublish_<n>checked out, no env vars,gh authauthenticated asjanechugh, API returnsadmin, bypass banner, exits 0Live API probe used during design (verifying expected roles):
Documentation
CONTRIBUTING.md— newManual version bumpssection 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 usespublish_$(node -p 'Date.now()')so the existing flow benefits from the bypass..github/skills/shipping/SKILL.md—When change files are NOT requirednow references the admin-role check..github/workflows/ci-validate-pr.yml— passesGITHUB_TOKENto thecheckchangestep.Checklist
mainbranch.change/file describing the changes has been added (N/A — this PR doesn't touchpackages/*and is itself a tooling change; the wrapper script lives underbuild/scripts/).