diff --git a/.github/PULL_REQUEST_TEMPLATE.md b/.github/PULL_REQUEST_TEMPLATE.md index 9e865a5b..7fae0958 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 5f35581e..2cbfa40b 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 a2dcc14c..937c0ed8 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 cf09d486..82f0f011 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 384edd5f..21144ec5 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 4785ea8d..7eb50421 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 79461112..08522f29 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