From 7bcd3944f0a10fd30c92f6386e33282386cb640e Mon Sep 17 00:00:00 2001 From: ciotlosm Date: Sat, 11 Jul 2026 18:00:45 +0300 Subject: [PATCH] fix(docs): align release-bot standards with v0.2.0+ PR-based flow While validating the release-bot README against these standards (PR n3ary/release-bot#14), two drift items surfaced in standards/org-automation.md and one in standards/version-management.md. All three described the v0.1.0 direct-push design, NOT the v0.2.0+ PR-based flow that's been in main since n3ary/release-bot#1. ## org-automation.md 1. "What the bot does NOT do" - the "Open a Release PR" bullet was inverted from reality. The bot DOES open a `chore(release): ` PR; v0.1.0 was the design that got dropped. Replaced with a real anti-pattern: bypassing the review/checks path. No org-level bypass-actor rule exists. 2. "Where it lives" file tree: - `src/config.ts` does not exist (env-loading is in `src/auth.ts`). - `test/` has `commit.test.ts` and `helpers.ts` too, not just `bump.test.ts`. - Added `pnpm-workspace.yaml` (the pnpm 11 allowBuilds: list, landed in #12) and `vitest.config.ts`. 3. Install step 3 - events list said `pull_request, push`; actual `app.yml` only subscribes to `pull_request` (the push subscription was removed in #6 because the bot's own version-bump auto-merge produces a push event, which would loop). 4. "Pushing the commit" - heading and body described pushing to `main` directly. Replaced with "Committing to the branch" and the actual flow: bot commits to a `release/calver-` branch, the PR + auto-merge lands it on `main`. The multi-file PATCH endpoint is now `refs/heads/`, not `refs/heads/main`. 5. "Concurrency" - was about retrying on 409 Conflict when pushing to `main`. The actual code uses idempotency guards: `createBranch` returns "exists" on 422 from a duplicate ref, and `findOpenReleasePR` short-circuits on an existing open release PR. No retries needed for the common races. 6. "Secret rotation" - listed 3 secrets; the README and on-call runbook use a 4th (`ADMIN_TOKEN` for the `/test/bump` endpoint). Added. 7. "What this is NOT - Not release-please" - the contrast said the n3ary bot "pushes the version bump directly to main with no human review". Wrong on both counts: it opens a PR (so a human COULD review), and auto-merge handles it (so they don't have to). Reworded to reflect the actual trade-off. ## version-management.md 1. The "Bump on merge" rule at the top said the bot "pushes a chore(release) commit to main". Reworded to describe the PR + auto-merge flow. 2. All four "What this looks like" examples used "bot pushes ... to main". Updated each to "bot opens PR chore(release): , auto-merge fires". 3. "Two PRs merge in quick succession" - the concurrency group story was wrong (the Worker doesn't have one). Updated to describe the actual `findOpenReleasePR` short-circuit. 4. "Don't bump on the PR branch" anti-pattern - reworded to mention the bot's own `release/calver-` branch (the bot doesn't touch the dev's branch). 5. "Implementation reference" - removed `config.ts` (not a real file). Added `pr.ts` and `auth.ts`. Removed the "One org-level branch-protection rule allows the bot's identity to bypass" sentence - the PR-based flow doesn't need a bypass. No code change. ASCII only. --- standards/org-automation.md | 41 +++++++++++++++++++++++---------- standards/version-management.md | 33 +++++++++++++++----------- 2 files changed, 48 insertions(+), 26 deletions(-) diff --git a/standards/org-automation.md b/standards/org-automation.md index 18150ec..d68f093 100644 --- a/standards/org-automation.md +++ b/standards/org-automation.md @@ -34,9 +34,9 @@ The bot is a Cloudflare Worker. Source: `n3ary/release-bot/src/`. Deploy: `wrang - **Bump on the PR branch.** The PR branch is the dev's; the bot has no business there. The whole point of the bot is the bump happens on `main`, not on the PR. - **Bump on a tag, on a schedule, or on `push` to `main` directly.** Those are anti-patterns. See [version-management.md](version-management.md). -- **Open a Release PR.** The bot pushes the bump commit directly to `main`. No human review of the version itself, because the version is mechanically derived from the merge timestamp — there's nothing to review. +- **Bypass the review / checks path.** The version-bump PR goes through the normal review + status-checks path with the bot acting as a contributor. No org-level bypass-actor rule, no admin override, no direct-push privilege. The audit trail is the same as any other contributor PR. - **Publish to npm, push a container image, or deploy.** Those are per-consumer workflows with repo-specific secrets. The bot is version-management only. -- **Resolve merge conflicts.** If the bot's push fails (e.g. main advanced under it), the bot retries with the latest `main` and re-computes. Bounded retries; after N failures it surfaces an alert. +- **Resolve merge conflicts.** If the bot's commit fails (e.g. the branch ref was updated by a concurrent run), the bot retries with the latest state. Bounded retries; after N failures it surfaces an alert. ## Where it lives @@ -47,15 +47,19 @@ n3ary/release-bot/ │ ├── index.ts # Worker entry, route dispatch │ ├── webhook.ts # webhook signature verification, event handling │ ├── bump.ts # CalVer arithmetic: nextCalVer(current, now, tz) -│ ├── commit.ts # discoverAndOpenPR: discover, compute, branch, commit, PR, auto-merge +│ ├── commit.ts # discoverAndOpenPR: discover, skip rules, branch, commit, PR, auto-merge │ ├── pr.ts # createBranch, openPullRequest, enableAutoMerge, findOpenReleasePR -│ ├── config.ts # loads env (timezone, app ID, private key, webhook secret) +│ ├── auth.ts # JWT signing + installation token exchange │ └── types.ts # shared types ├── test/ -│ └── bump.test.ts # unit tests for the CalVer logic +│ ├── bump.test.ts # CalVer unit tests +│ ├── commit.test.ts # per-file idempotency tests (skip rules, isPackageTouched) +│ └── helpers.ts # test helpers ├── wrangler.toml # Cloudflare Worker config +├── pnpm-workspace.yaml # pnpm 11 allowBuilds: for esbuild + sharp + workerd ├── package.json ├── tsconfig.json +├── vitest.config.ts ├── .gitignore └── README.md # install + deploy + on-call runbook ``` @@ -70,7 +74,7 @@ The app is registered via a manifest-based flow: 1. Go to `https://github.com/organizations/n3ary/settings/apps/new` (or the equivalent for the org). 2. Paste the contents of `n3ary/release-bot/app.yml` as the manifest. -3. Confirm the app's name (`n3ary-release-bot`), webhook URL (the Cloudflare Worker URL), and events (`pull_request`, `push`). +3. Confirm the app's name (`n3ary-release-bot`), webhook URL (the Cloudflare Worker URL), and the `pull_request` event. 4. GitHub creates the app and provides an **App ID** and a **private key** (PEM). Download the private key; it does not get shown again. ### 2. Set the Cloudflare Worker secrets @@ -152,15 +156,27 @@ Before writing a new version, the bot reads the version at `HEAD~1` of `main` (t For multi-package repos, the check is per-`package.json` file. A dev who edits `libs/spec/package.json#version` does not affect the root `package.json#version`, so the root still gets its own bump. -### Pushing the commit +### Committing to the branch -The bot uses the GitHub API (`PUT /repos/{owner}/{repo}/contents/{path}`) to update each `package.json`. The commit is auto-created by the API; the bot sets the commit author to the app's identity (`n3ary-release-bot[bot] <[email protected]>`) and the commit message to `chore(release): `. +The bot commits the version bump to its own branch (`release/calver-`), not to `main`. The PR + auto-merge is what lands the commit on `main`. -For multi-file bumps, the bot uses the Git Data API (`POST /repos/{owner}/{repo}/git/trees`, then `POST /repos/{owner}/{repo}/git/commits`, then `PATCH /repos/{owner}/{repo}/git/refs/heads/main`) to create a single commit that updates all `package.json` files atomically. +For each bumped `package.json`: + +- **Single-file bump**: `PUT /repos/{owner}/{repo}/contents/{path}` with the new file content. The API auto-creates the commit on the bot's branch. +- **Multi-file bump**: Git Data API — `POST /repos/{owner}/{repo}/git/blobs` per file, then `POST /repos/{owner}/{repo}/git/trees` (with `base_tree` set to the merge commit's tree and only the changed entries passed), then `POST /repos/{owner}/{repo}/git/commits`, then `PATCH /repos/{owner}/{repo}/git/refs/heads/` to update the branch ref. + +In both cases, the commit author is the app's identity (`n3ary-release-bot[bot] <[email protected]>`) and the commit message is `chore(release): ` (single) or `chore(release): , , ...` (multi-file, one entry per bumped `package.json`). + +The bot does NOT touch `main` directly. The PR + auto-merge is what lands the commit on `main`. ### Concurrency -Two PRs that merge in quick succession can fire two bot invocations. The Cloudflare Worker is stateless, so there's no built-in mutex. The bot's push is retried on `409 Conflict` (the "update was rejected because the tip of the ref advanced under you" error). On conflict, the bot re-fetches `main`, re-computes, retries. Bounded at 3 retries. After 3 failures, the bot surfaces an alert (TBD: which channel — for now, the Worker logs the failure and the next PR merge re-triggers the bump for the missed one). +Two PRs that merge in quick succession can fire two bot invocations. The Cloudflare Worker is stateless, so there's no built-in mutex. The bot's idempotency guards are: + +- **`createBranch` is idempotent** (`src/pr.ts`): if the branch already exists (e.g. from a previous attempt for the same version), the function returns `"exists"` instead of erroring. The bot continues with the commit step on the existing ref. +- **`findOpenReleasePR` short-circuits** (`src/pr.ts`): if a previous webhook run already created a `release/calver-*` PR that's still open, the new event no-ops and the existing PR is the one that lands. Surfaced in the log as `Idempotency: open release PR #N already exists; no-op`. + +These two guards cover the common races. Transient GitHub API errors (5xx) get a standard bounded retry. After N failures, the Worker logs the failure and the next PR merge re-triggers the bump for the missed one. ## On-call runbook @@ -183,17 +199,18 @@ When the bot fails: ## Secret rotation -The bot has three secrets, all stored as Cloudflare Worker secrets: +The bot has four secrets, all stored as Cloudflare Worker secrets: - `GITHUB_APP_ID` — the app's numeric ID. Stable; rotate only if the app is recreated. - `GITHUB_APP_PRIVATE_KEY` — the app's PEM private key. Rotate annually. The old key remains valid until you remove it from the GH UI; the new key works immediately. No downtime. - `GITHUB_WEBHOOK_SECRET` — a strong random string used to sign webhooks. Rotate annually, or immediately if a leak is suspected. To rotate, set a new value via `wrangler secret put`, then update the app's "Webhook secret" in the GH UI to match. +- `ADMIN_TOKEN` — bearer token for the manual `/test/bump` endpoint (used in the on-call runbook). Rotate annually, or immediately if a leak is suspected. After rotating any secret, verify with a test PR merge that the bot still fires. ## What this is NOT -- **Not release-please.** Google's release-please opens a Release PR with a CHANGELOG and a version bump; humans review the Release PR before merge. The n3ary bot pushes the version bump directly to `main` with no human review. The trade-off: less ceremony, no CHANGELOG. CalVer's "date is in the version" is the only human-readable signal needed. +- **Not release-please.** Google's release-please opens a Release PR with a CHANGELOG and a version bump; humans review the Release PR before merge. The n3ary bot opens a `chore(release): ` PR and enables auto-merge — humans don't have to click, but the commit still goes through the same review + status-checks path as any contributor. The trade-off vs. release-please: less ceremony, no CHANGELOG. CalVer's "date is in the version" is the only human-readable signal needed. - **Not semantic-release.** semantic-release analyzes commit messages to decide patch/minor/major. The n3ary bot uses CalVer + daily counter, with no commit-message parsing. - **Not a generic CI runner.** The bot is version-management only. Builds, tests, deploys, and publishes are per-consumer workflows with repo-specific secrets. diff --git a/standards/version-management.md b/standards/version-management.md index 392a0c7..f8d0804 100644 --- a/standards/version-management.md +++ b/standards/version-management.md @@ -10,7 +10,7 @@ Every shipped change to any n3ary repo produces a new version in that repo's `pa - The same day, the second release is `26.7.11-2`, the third is `26.7.11-3`, and so on. - The next day, the counter resets to `1`: `26.7.12-1`. - **Timezone: `Europe/Bucharest`.** The day boundary is midnight in the org's local timezone. This is the only place "today" is defined for the bot. -- **Bump on merge, by the org-level release bot.** The bot is a GitHub App installed on the n3ary org. On every `pull_request: closed` event with `merged == true`, it pushes a `chore(release): ` commit to `main` with the next CalVer version. No per-consumer workflow file references the bot. +- **Bump on merge, by the org-level release bot.** The bot is a GitHub App installed on the n3ary org. On every `pull_request: closed` event with `merged == true`, it opens a pull request titled `chore(release): ` with the version bump on a `release/calver-` branch, and enables auto-merge on the PR. With 0 required reviews (the n3ary branch protection standard), the version lands on `main` as soon as the required status checks pass. No per-consumer workflow file references the bot. - **Skip when the merge already changed the version.** If the merge commit's `package.json#version` differs from its parent's, the dev manually edited the version (e.g. for a tagged build, a one-off release, or a backport). The bot's idempotency check sees this and no-ops. The dev's edit wins. - **Multi-package repos are handled per `package.json`.** The bot discovers every `package.json` under the repo tree (root, `libs/spec/`, `adapters/*/`, etc.) and bumps each one independently. The `skip-if-already-touched` rule applies per file. @@ -41,13 +41,15 @@ main is at 26.7.11-1 (from an earlier merge today) 11:30 PR #142 "fix: handle missing stop times" merges → bot receives webhook, reads main = 26.7.11-1 → bot computes next = 26.7.11-2 (same day, counter+1) - → bot pushes chore(release): 26.7.11-2 to main + → bot opens PR "chore(release): 26.7.11-2" against main + → auto-merge fires as soon as required checks pass (~10-30 s) → main is now 26.7.11-2 15:45 PR #143 "chore: bump deps" merges → bot receives webhook, reads main = 26.7.11-2 → bot computes next = 26.7.11-3 (same day, counter+1) - → bot pushes chore(release): 26.7.11-3 to main + → bot opens PR "chore(release): 26.7.11-3" against main + → auto-merge fires → main is now 26.7.11-3 ``` @@ -59,7 +61,8 @@ main is at 26.7.11-3 (from 11 July) 10:00 PR #144 "feat: support multiple agencies" merges → bot receives webhook, reads main = 26.7.11-3 → bot computes next = 26.7.12-1 (new day, counter resets to 1) - → bot pushes chore(release): 26.7.12-1 to main + → bot opens PR "chore(release): 26.7.12-1" against main + → auto-merge fires → main is now 26.7.12-1 ``` @@ -73,7 +76,7 @@ main is at 26.7.11-2 14:30 PR #145 merges → bot receives webhook, reads main's libs/spec/package.json#version = 26.7.12-1 → bot reads main's libs/spec/package.json#version at HEAD~1 = 26.7.11-2 - → bot sees they differ → no-op + → bot sees they differ → no-op on libs/spec/package.json → main is now 26.7.12-1, exactly as the dev intended ``` @@ -82,13 +85,14 @@ main is at 26.7.11-2 ``` main is at 26.7.11-1 -11:30:00 PR-A merges → bot triggers -11:30:02 PR-B merges → bot triggers (queued behind PR-A's bump) -11:30:05 PR-A's bump runs → reads 26.7.11-1, pushes 26.7.11-2 -11:30:08 PR-B's bump runs (dequeued) → reads 26.7.11-2, pushes 26.7.11-3 +11:30:00 PR-A merges → bot webhook fires +11:30:02 PR-B merges → bot webhook fires +11:30:05 PR-A's webhook runs first → reads 26.7.11-1, opens PR "chore(release): 26.7.11-2" +11:30:08 PR-B's webhook runs → calls findOpenReleasePR, sees PR-A's PR is still open, no-ops +11:30:35 PR-A's PR auto-merges → main is now 26.7.11-2 ``` -The two bumps serialize via a `concurrency: group` on the bot's Cloudflare Worker, so they don't race on the read-then-write of main's version. The final state is `26.7.11-3`, with two distinct `chore(release)` commits in `main`'s log. Clean. +The two webhooks don't race. The second one short-circuits on `findOpenReleasePR` and lets PR-A's PR land first. Once main is at 26.7.11-2, the next PR merge in any repo will trigger the bot again for the 26.7.11-3 bump on the original repo (not on PR-B's repo — each bot run is scoped to the repo that fired the webhook). ## What ships in `package.json` @@ -98,7 +102,7 @@ For multi-package repos (e.g. `gtfs-publisher` with `libs/spec/`, `gtfs-adapters ## Anti-patterns to avoid -- **Don't bump on the PR branch.** The PR branch is the dev's; the bot has no business there. The whole point of this model is the bot touches `main`, not the PR. +- **Don't bump on the PR branch.** The PR branch is the dev's; the bot has no business there. The bot creates its own `release/calver-` branch and opens a PR — the dev's branch is never touched. - **Don't bump on a tag.** We don't tag releases. The version in `package.json` is the only version string we publish. If a tag is ever needed, the release bot's commit message is the natural anchor. - **Don't bump on a schedule.** Schedule-based bumps cause version drift between the source and the published bundle. The bump should always accompany a code change. - **Don't hand-edit `package.json#version` in a PR unless you mean it.** The bot's `skip-if-already-touched` rule treats any version change in the merge as intentional. If the version change is a mistake (e.g. you ran `npm version` locally and committed it), the bot will silently no-op on the bump and your mistake becomes the version. @@ -110,10 +114,11 @@ The bump is implemented as the `n3ary/release-bot` Cloudflare Worker, deployed v - `webhook.ts` — receives GitHub webhooks, verifies signature, dispatches. - `bump.ts` — CalVer arithmetic, the `nextCalVer(current, now, tz)` function. -- `commit.ts` — pushes the `chore(release)` commit via the GitHub API. -- `config.ts` — loads env (timezone, app ID, private key, webhook secret). +- `commit.ts` — discovers `package.json` files, applies the per-file skip rules, creates the `release/calver-` branch, commits the version bump, opens the PR, enables auto-merge. +- `pr.ts` — branch + PR operations (`createBranch` is idempotent; `findOpenReleasePR` short-circuits on existing open release PRs). +- `auth.ts` — JWT signing + installation token exchange. -The bot is a GitHub App registered with the manifest in `n3ary/release-bot/app.yml`, installed on the n3ary org. One org-level branch-protection rule allows the bot's identity to bypass the "no direct push to `main`" requirement. +The bot is a GitHub App registered with the manifest in `n3ary/release-bot/app.yml`, installed on the n3ary org. The bot is a contributor, not a privileged actor — no org-level bypass-actor rule is required. The `n3ary/actions/.github/actions/version-bump` composite action **remains** for the `version-input` publish path (used by `gtfs-publisher`'s `release-gtfs-spec.yml` and `gtfs-adapters`'s `release-gtfs-adapter.yml` to publish a specific version). Its `pr-bump` mode is deprecated; no consumer calls it.