From 94b316478545bbd835b82cc58fc6e1f675894547 Mon Sep 17 00:00:00 2001 From: Shevchik Igor Date: Wed, 19 Aug 2026 14:05:20 +0000 Subject: [PATCH] docs(release): document the CI approval, the `revert:` subject, and correct the pipeline's own account of itself MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Five review passes found the first version of this change wrong in ways that mattered, including one place where it told the reader the opposite of what they would be looking at. This is the corrected version, and it fixes the false claims the pipeline makes about itself rather than only adding true ones beside them. **The symptom is stated correctly now.** The release PR sits at `blocked` while its `ci` check is **green** — the green one is the dispatched run; a separate `pull_request` run is held at `action_required`, creates no jobs and therefore no check run, and is what branch protection is waiting on. The first version said "blocked with no `ci` check", which is what the API contradicts and which would have sent a reader to the wrong remedy, since a green check with a refused merge is also what a stale branch looks like. Both causes are now listed, in the order to check them, with the literal error string a person will search on. **The click is documented as a procedure with an order.** Actions → the held run is the primary route; the Checks tab is not, because a held run renders nothing there. Approval attaches to a run and a run to a commit, so any push to `main` strands it — up to nine held runs in a day here — which makes approve-then-merge a race, not a sequence, and merges to `main` have to pause while it runs. **Two workflow comments asserted the mechanism this document refutes.** Both `ci.yml` and `release-please.yml` claimed a `GITHUB_TOKEN` PR fires no pull_request event and that the dispatched run satisfies the required check. Neither is true, and they are the files a person opens first. Corrected and pointed at the new section, as `release-watchdog.yml` already does for the cadence commitment. **The revert guidance now names where the subject is actually set.** Retitling the PR is not sufficient: a revert PR is always a single commit, and for those the squash subject may come from the commit rather than the title. The squash-merge dialog is the route that works either way. Recorded too: the parse failure prints in the Release Please job log (it is invisible, not absent), a `Revert "…"` that is the only commit since the last tag opens no release PR at all, and a `revert:` does not cancel the `feat:` it undoes — both ship, and the release is still a minor. The two `Revert "…"` commits in this history are no longer offered as evidence of harm. They predate `bootstrap-sha` and the CHANGELOG's first entry, which the #435 commit message already said and this one had dropped. The sample under release-please is zero; that is now stated plainly. **The template is where the trap is catchable.** GitHub's revert button opens the PR form with `Revert "…"` already in the title box, so the instruction goes there as well as in the guides nobody has open at that moment. The watchdog's nudge — which reaches a maintainer exactly when their release PR is stuck — now links the section instead of only saying "merge it". Also corrected in passing: the file claimed under Optional hardening that merging is "the only human step", contradicting its own opening; the `severity:crash` paragraph existed twice; a cross-reference pointed at the wrong promise; two paragraphs about release cadence had been orphaned under a heading about repository settings; branch protection was missing from the list of settings the pipeline depends on; and the break-glass omitted `dev:prepare` and `--no-git-checks`, without which it ships a tarball missing a file `package.json` lists. The "no workflow-file fix" claim is softened to "none worth taking", because one does work: a workflow can `POST /statuses/{sha}` with `context: ci`. It is refused on purpose — it fabricates the signal the gate carries — and the reason is now recorded next to the trick. `merge_group` and the Actions approval setting are listed as unexplored rather than impossible. Closes #353. --- .github/PULL_REQUEST_TEMPLATE.md | 6 + .github/contributing/releasing.md | 184 +++++++++++++++--- .github/scripts/watchdog-collect.sh | 2 +- .github/workflows/ci.yml | 9 +- .github/workflows/release-please.yml | 12 +- AGENTS.md | 4 +- .../docs/1.getting-started/4.contribution.md | 1 + 7 files changed, 184 insertions(+), 34 deletions(-) diff --git a/.github/PULL_REQUEST_TEMPLATE.md b/.github/PULL_REQUEST_TEMPLATE.md index 9e865a5ba..7fae0958d 100644 --- a/.github/PULL_REQUEST_TEMPLATE.md +++ b/.github/PULL_REQUEST_TEMPLATE.md @@ -1,5 +1,10 @@ ### Linked issue @@ -15,6 +20,7 @@ PR title should follow conventional commits (https://conventionalcommits.org) - [ ] Enhancement (improving an existing functionality) - [ ] New feature (a non-breaking change that adds functionality) - [ ] Chore (updates to the build process or auxiliary tools and libraries) +- [ ] Revert (undoing a merged change — retitle this PR `revert(Scope): ...`) - [ ] Breaking change (fix or feature that would cause existing functionality to change) ### Description diff --git a/.github/contributing/releasing.md b/.github/contributing/releasing.md index 5f35581ee..2cbfa40bd 100644 --- a/.github/contributing/releasing.md +++ b/.github/contributing/releasing.md @@ -1,7 +1,17 @@ # Releasing -Releases are automated. Nothing here has to be run by hand in the normal case, -and none of it requires repository settings to be configured first. +Releases are automated up to two human actions, in this order: **approve the +release PR's held CI run, then merge it.** The merge is the decision this +pipeline exists to leave with a person. The approval is not a decision at all — +it is a click GitHub requires and nothing announces. + +**If the release PR is sitting at `blocked` while its `ci` check is green, that +is what you are looking at.** See +[Approving the release PR's CI](#approving-the-release-prs-ci). + +Three repository settings have to be right for any of this to work, and none of +them fails loudly. They are listed under +[Repository settings this depends on](#repository-settings-this-depends-on). ## The normal path @@ -13,11 +23,14 @@ and none of it requires repository settings to be configured first. the commits since the last tag. Every push to `main` rewrites that same PR — fifteen merged fixes update it fifteen times, they do not cut fifteen releases. -3. **Merging the release PR is the release.** It tags the commit, publishes the - GitHub Release with those notes, and dispatches `npm-publish.yml` against the - tag. Merge it the way every other PR here is merged — squash. release-please - keys off the merged PR's `merge_commit_sha` from the API, not off the shape of - the commit graph, so squashing does not orphan anything. +3. **Approve the held CI run, then merge — in that order.** Merging the release + PR is the release: it tags the commit, publishes the GitHub Release with those + notes, and dispatches `npm-publish.yml` against the tag. It will not be + mergeable until you approve its held `ci` run; see + [Approving the release PR's CI](#approving-the-release-prs-ci). Merge it the + way every other PR here is merged — squash. release-please keys off the merged + PR's `merge_commit_sha` from the API, not off the shape of the commit graph, + so squashing does not orphan anything. 4. **`npm-publish.yml` publishes** after asserting the commit is merged `main` history and waiting for CI to be green on that exact SHA. Publishing goes through npm trusted publishing (OIDC) — there is no long-lived token in the @@ -29,8 +42,9 @@ workflow's filename (`npm-publish.yml`) and optionally an environment name. Renaming either breaks publishing with no signal from here — if you rename the file or the `npm-publish` environment, update npm to match in the same change. -So the only decision a human makes is *when to merge the release PR*. Its -description is the changelog you are about to ship; read it and merge. +So the only *decision* a human makes is **when to merge the release PR**. Its +description is the changelog you are about to ship: read it, approve the held CI +run, and merge. **The commitment is two weeks.** An open release PR should not outlive it, and `release-watchdog.yml` reports one that does. The number lives in that workflow's @@ -40,13 +54,95 @@ While the PR is open it doubles as a live list of what is fixed on `main` and no yet on npm. That is deliberate — issue #315 exists because a crash fix sat unreleased for five weeks with nothing surfacing the fact. +## Approving the release PR's CI + +**The symptom:** the release PR sits at `blocked` while its `ci` check is +**green**, and merging is refused with + +``` +Required status check "ci" is expected +``` + +Two things cause that combination. Check them in this order. + +### Cause 1 — a run is held, awaiting approval + +The green `ci` you can see is the run `release-please.yml` dispatches. Separately +GitHub creates a `pull_request` run for the PR and holds it at +`action_required`. A held run creates no jobs, and therefore no check run, so it +is invisible on the *Checks* tab — but branch protection is still waiting on it. + +This is specific to the bot's branch: ordinary PRs start their CI unattended, and +across the last 30 `pull_request` runs every one on `release-please--**` was held +while none of the others were. + +**What to do — immediately before you merge, not earlier.** Go to **Actions →** +the held `ci` run on the `release-please--…` branch and approve it. GitHub also +surfaces the control on the PR itself — *Files changed* → **Awaiting approval**, +or the merge box on *Conversation* — and has labelled the button both +**Approve and run** and **Approve workflows to run**. The run takes about eight +minutes. Then merge. + +**Why the ordering.** An approval attaches to a run, and a run attaches to a +commit. Any push to `main` regenerates the release PR onto a new head, stranding +the approval on a commit that is no longer the head while a fresh held run +appears on the new one — up to nine of them in a day here. So approving early to +"have it ready" is wasted within the hour, and **if `main` moves while your run +is in flight, approve the new held run and start the eight minutes over**. On a +busy day that means pausing merges to `main` until the release is out. + +### Cause 2 — the branch is behind + +If the head commit carries a green `ci` and nothing is held, the PR is simply out +of date with `main`. Update the branch and let `ci` re-run on the new head. + +### Why there is no workflow-file fix worth taking + +Recorded so nobody re-derives it (#353). Skip unless you are about to try one. + +- `branches` and `branches-ignore` are mutually exclusive for one event, and for + `pull_request` the filter matches the *base* branch — so a head-branch pattern + matches nothing. +- A job-level `if` on `github.head_ref`, which is GitHub's own prescribed way to + branch on the head, never evaluates: a held run creates no jobs at all. +- **Do not drop `ci` from required checks.** It would work, and it is a bad + trade — it removes the merge gate from every PR into `main` to accommodate one + bot PR, and the publish gate does not compensate, because it protects npm, not + `main`. +- **Do not have a workflow post a commit status named `ci`.** `GITHUB_TOKEN` can + `POST /repos/{owner}/{repo}/statuses/{sha}`, and a commit status satisfies a + required check exactly as a check run does. It works — which is what makes it + dangerous, because it fabricates the signal the gate exists to carry. Refused + on purpose; written down so the next person finds the reason and not just the + trick. + +Two routes are unexplored rather than ruled out: adding `merge_group:` to +`ci.yml` behind a merge queue, and the repository or organisation setting that +governs when Actions demands approval at all. Neither has been tried here. + +## Repository settings this depends on + +None of these is in version control, and none fails loudly. + +- **Settings → Actions → General → Workflow permissions →** *Allow GitHub + Actions to create and approve pull requests*. Without it release-please cannot + open the release PR at all; the run fails with `GitHub Actions is not + permitted to create or approve pull requests`. This one gates the pipeline + existing. +- **Branch protection on `main` requiring the `ci` check.** This is what makes a + held run block the merge, and therefore what makes the approval above + necessary. Deliberate — read the trade above before changing it. +- **Settings → Labels → `severity:crash`.** This one gates nothing in the release + path; it only arms the watchdog's 48-hour threshold, and it fails silently when + absent. See [What is watched automatically](#what-is-watched-automatically). + ## How the version is chosen | commits since the last tag | next version | | --- | --- | | any `BREAKING CHANGE` / `!` | major | -| at least one `feat:` | minor | -| otherwise (`fix:`, `docs:`, `ci:`, …) | patch | +| at least one `feat:` (or its alias `feature:`) | minor | +| otherwise (`fix:`, `revert:`, `docs:`, `ci:`, …) | patch | The highest wins, and it applies once — fifteen `fix:` commits produce one patch release, not fifteen. To force a specific version, put `Release-As: 2.12.0` in a @@ -61,9 +157,11 @@ themselves. #301 is the reference case: every SPA on Nuxt ≥ 4.5.1 died on boot When one is confirmed: 1. Label the issue `severity:crash`. -2. Fix it, and **merge the release PR within 48 hours** — not on the next - convenient cadence. The fix reaching `main` is not the goal; the fix reaching - npm is. +2. Fix it, then **approve the release PR's held CI run and merge it within 48 + hours** — not on the next convenient cadence. The fix reaching `main` is not + the goal; the fix reaching npm is. Landing the fix regenerates the release PR, + so the approval has to come after it and immediately before the merge; see + [Approving the release PR's CI](#approving-the-release-prs-ci). 3. Say so in the issue when it is published, with the version. The fast lane ships whatever else is on `main` alongside the fix. That is @@ -82,6 +180,39 @@ patch, re-land it afterwards. This is smaller than it sounds, and the reason is worth saying out loud at 2am — **unreleased work on `main` has no users**. Reverting it costs nobody anything, and the re-land is a rebase. +**Title the revert `revert(Scope): …`.** GitHub's revert button produces +`Revert ""`, which release-please's parser rejects outright. It +never becomes a changelog line — and if it is the only commit since the last tag, +no release PR is opened at all. + +``` +Revert "feat(Modal): add fullscreen prop" -> vanishes +revert(Modal): drop the fullscreen prop -> lands under Reverts +``` + +**Set that subject in the squash-merge dialog, not only in the PR title.** A +revert PR is always a single commit, and for a single-commit PR GitHub may take +the squash subject from the commit rather than from the PR title, depending on +the repository's squash-merge settings. Editing the message box on the merge +button is the route that works either way. + +The parse failure is not silent, though nothing surfaces it where you would +look: the Release Please job log carries `commit could not be parsed: +Revert "…"` on every run. That line is the diagnosis when a changelog entry is +missing. + +The `Reverts` section exists (#435), but only a conventional `revert:` subject +reaches it. Two reverts in this repository's history took the `Revert "…"` form; +both predate `bootstrap-sha` and the CHANGELOG's first entry, so nothing +published is missing anything. The sample of reverts under release-please is +zero, which is why this is written down before the first one rather than after. + +**A revert does not cancel the commit it undoes.** release-please pairs nothing: +a `feat:` and its `revert:` in the same release window both appear, and the +release is still a minor. Reverting unreleased work therefore produces a +changelog that announces and retracts the same feature — usually fine, but not a +surprise you want at 2am. + If CI is red for something that is not a revertable commit — a flaky suite, a broken runner image, a playground build that only fails in `npm-publish.yml` — there is nothing to revert, and **there is no bypass in the pipeline**. Say it @@ -97,8 +228,13 @@ So the options are the honest two. **Fix the failure** — for a flake, re-runni window needs no further action. **Or accept the delay** and say so on the issue. If neither is acceptable — a crash-class bug, CI broken for reasons nobody can -fix quickly — the break-glass is a maintainer publishing by hand: `pnpm build` -then `pnpm publish` from a machine logged in to npm. It is deliberately ugly. +fix quickly — the break-glass is a maintainer publishing by hand from a machine +logged in to npm, running the same sequence `npm-publish.yml` does: +`pnpm install --frozen-lockfile`, `pnpm run dev:prepare`, `pnpm build`, then +`pnpm publish --no-git-checks`. `dev:prepare` is not optional — it emits +`.nuxt/b24ui.static.css`, which `package.json` lists in `files` — and without +`--no-git-checks` the publish is refused outright from a detached checkout. It +is deliberately ugly. It skips the merged-history assert, the CI gate and npm's provenance, and it needs a human account with publish rights rather than the repository's OIDC identity. Do it only as the last step, and open an issue afterwards recording @@ -118,11 +254,10 @@ true: with the tag selected as the ref; re-runs are safe, an already-published version ends as a green no-op. -The `severity:crash` half of that first line needs the label to exist in -Settings → Labels. Nothing creates it, and GitHub returns an empty list rather -than an error for a label that was never created — so if it is missing the -threshold silently stays at 14 days and the 48-hour promise has nothing behind -it after all. +The `severity:crash` half of that first line needs the label to exist, and +nothing creates it. It is listed under +[Repository settings this depends on](#repository-settings-this-depends-on) with +the reason it fails silently. Actions → **Release watchdog** → *Run workflow* takes a day-threshold override. It is the only way to exercise the reporting path without waiting out the real @@ -132,6 +267,7 @@ threshold: dispatching with `0` reports whatever release PR is currently open. `npm-publish.yml` declares `environment: npm-publish`. GitHub auto-creates a referenced environment **without** protection rules, so by default the job runs -unprotected and the release PR merge is the only human step. Adding required -reviewers to that environment in Settings → Environments turns on a second -approval before anything reaches npm, with no code change. +unprotected and the two actions above — approving the held `ci` run and merging +the release PR — are the only human ones. Adding required reviewers to that +environment in Settings → Environments turns on a further approval before +anything reaches npm, with no code change. diff --git a/.github/scripts/watchdog-collect.sh b/.github/scripts/watchdog-collect.sh index a2dcc14ca..937c0ed83 100755 --- a/.github/scripts/watchdog-collect.sh +++ b/.github/scripts/watchdog-collect.sh @@ -75,7 +75,7 @@ if [ -n "$pr" ]; then if [ "$age" -ge "$threshold" ]; then # Referenced by number, never by title: the title is attacker-supplied text # and this body tells a human to run the publish workflow. - printf -- '- **Release PR #%s has been open for %s days.** Everything in it is fixed on `main` and not on npm. Merging it tags, releases and publishes.\n' \ + printf -- '- **Release PR #%s has been open for %s days.** Everything in it is fixed on `main` and not on npm. Approve its held `ci` run, then merge — see [releasing.md](../blob/main/.github/contributing/releasing.md#approving-the-release-prs-ci).\n' \ "$number" "$age" >> findings.md fi elif [ "$pr_checked" = true ]; then diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index cf09d4867..82f0f0118 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -5,9 +5,12 @@ on: branches: [ main ] pull_request: branches: [ main ] - # Dispatched by release-please.yml onto its release-PR branch: PRs opened - # with GITHUB_TOKEN fire no pull_request event, and a dispatched run on the - # same head SHA is what satisfies a required `ci` status check. + # Dispatched by release-please.yml onto its release-PR branch. The bot's PR + # does fire a pull_request event, but GitHub holds that run at + # `action_required` awaiting manual approval, so the dispatched run is the only + # `ci` that actually executes there. It does NOT satisfy the required check — + # branch protection waits on the held run, which a human has to approve. See + # .github/contributing/releasing.md#approving-the-release-prs-ci. workflow_dispatch: concurrency: diff --git a/.github/workflows/release-please.yml b/.github/workflows/release-please.yml index 384edd5fe..21144ec5e 100644 --- a/.github/workflows/release-please.yml +++ b/.github/workflows/release-please.yml @@ -33,10 +33,14 @@ jobs: config-file: release-please-config.json manifest-file: .release-please-manifest.json - # A PR opened with GITHUB_TOKEN fires no pull_request event, so the - # release PR would sit with no `ci` check — unmergeable wherever that - # check is required. A dispatched run lands on the same head SHA, which - # is what status checks are keyed by. + # Gives the release PR a `ci` run that actually executes. Its own + # pull_request run is created and then held at `action_required`, so + # without this the PR carries no CI result at all. + # + # This does NOT make the PR mergeable: branch protection waits on the held + # run, and a human has to approve that one before merging. Documented in + # .github/contributing/releasing.md#approving-the-release-prs-ci — keep the + # two in sync. - name: Run CI on the release PR if: steps.release.outputs.prs_created == 'true' env: diff --git a/AGENTS.md b/AGENTS.md index 4785ea8d3..7eb504214 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -65,8 +65,8 @@ Options: ## Key Conventions -- **Conventional commits**: All commit messages must follow [conventional commits](https://conventionalcommits.org) (e.g. `fix(Button): resolve hover state`, `feat(Modal): add fullscreen prop`). -- **Releases are automated**: merging work into `main` publishes nothing; release-please keeps one release PR open and merging *that* tags, releases and publishes to npm. Version arithmetic (`feat` -> minor, `fix` -> patch), the release cadence commitment and the `severity:crash` hotfix policy live in [releasing.md](.github/contributing/releasing.md). Filed here rather than in the References table below, which is scoped to `src/` and `test/` work. +- **Conventional commits**: All commit messages must follow [conventional commits](https://conventionalcommits.org) (e.g. `fix(Button): resolve hover state`, `feat(Modal): add fullscreen prop`). **A revert must be titled `revert(Scope): …`** — GitHub's revert button produces `Revert "…"`, which release-please rejects outright, so the revert reaches no changelog and, if it is the only commit since the last tag, opens no release PR at all. +- **Releases are automated**: merging work into `main` publishes nothing; release-please keeps one release PR open and merging *that* tags, releases and publishes to npm. Version arithmetic (`feat`/`feature` -> minor, `fix` -> patch), the release cadence commitment, the `severity:crash` hotfix policy, the `revert:` subject a revert PR must carry, and the manual CI approval the release PR needs before it can merge all live in [releasing.md](.github/contributing/releasing.md). Filed here rather than in the References table below, which is scoped to `src/` and `test/` work. - **Semantic colors**: Use `text-description`, `bg-elevated`, etc. — never raw Tailwind palette colors like `text-gray-500`. - **Dependency pins are not ours to move**: `reka-ui` and `vaul-vue` are exact-pinned in `package.json` because upstream pins them at those exact versions; they change through a port, never through a local bump. And `vue` is declared as a required peer (`^3.5.0`) even though upstream does not declare it — `src/` uses `useTemplateRef` and `useId`, both Vue 3.5. Both facts are invariants in [.sync/PORTING.md](.sync/PORTING.md) §2 and guarded by `test/utils/peer-dependencies.spec.ts`. - **`Soon` badge on docs headings**: PRs that introduce a new feature or fix often add `:badge{label="Soon" class="align-text-top"}` to the relevant docs heading. This is intentional: the docs site redeploys on merge, but the feature only ships on the next npm release — the badge bridges that gap. Do NOT flag this as inconsistent in reviews. See [documentation.md](.github/contributing/documentation.md) for details. diff --git a/docs/content/docs/1.getting-started/4.contribution.md b/docs/content/docs/1.getting-started/4.contribution.md index 79461112b..08522f29c 100644 --- a/docs/content/docs/1.getting-started/4.contribution.md +++ b/docs/content/docs/1.getting-started/4.contribution.md @@ -174,6 +174,7 @@ We use [Conventional Commits](https://www.conventionalcommits.org/) for commit m - Use `fix` and `feat` for code changes that affect functionality or logic - Use `docs` for documentation changes and `chore` for maintenance tasks +- Use `revert(Scope): …` when undoing a merged change, and write that subject yourself — GitHub's revert button titles the PR `Revert "…"`, which is not a conventional commit and is dropped from the changelog silently ### Making a Pull Request